LRC-LIB-MISSING: honour a stop during the last check; reject a check of an earlier catalog; fail only the library's own requests

crates/engine/src/cmd/missing.rs

@@ -52,6 +52,18 @@ pub fn checked(cat: &Catalog) -> Vec<(PhotoId, String)> {
     cat.photos().filter_map(|p| checked_path(p).map(|f| (p.id, f.to_string()))).collect()
 }
 
+/// What [`checked`] would list, as one number, without building the list: the photos in scope and
+/// the file each one points at, whatever their order. A caller that checked on its own thread
+/// compares it before and after to know the answer is still of the catalog it is given to.
+pub fn fingerprint(cat: &Catalog) -> u64 {
+    use std::hash::{DefaultHasher, Hash, Hasher};
+    cat.photos().filter_map(|p| checked_path(p).map(|f| (p.id, f))).fold(0u64, |sum, entry| {
+        let mut h = DefaultHasher::new();
+        entry.hash(&mut h);
+        sum.wrapping_add(h.finish())
+    })
+}
+
 /// How many files [`sweep`] checks between two reports of its progress.
 const PROGRESS_EVERY: usize = 64;
 
@@ -76,7 +88,8 @@ pub fn sweep(
             gone.push((id, file));
         }
     }
-    Some(gone)
+    // (a stop that came during the last check is honoured too)
+    (!stop()).then_some(gone)
 }
 
 /// `library.missing`'s answer: `[{id, path}]`.

crates/engine/src/tests/import.rs

@@ -1304,6 +1304,27 @@ fn missing_sweep_answers_what_the_command_does_and_can_stop() {
     assert_eq!((stopped, looked.load(Ordering::Relaxed)), (None, 10));
 }
 
+/// A stop that arrives while the last file is being checked is honoured: no answer.
+#[test]
+fn missing_sweep_honours_a_stop_during_the_last_check() {
+    use lightcraft_catalog::PhotoId;
+    use std::sync::atomic::{AtomicBool, Ordering};
+    let stop = AtomicBool::new(false);
+    let files = vec![(PhotoId(1), "/a".to_string()), (PhotoId(2), "/b".to_string())];
+    let swept = crate::cmd::missing::sweep(
+        files,
+        |f| {
+            if f == "/b" {
+                stop.store(true, Ordering::Relaxed);
+            }
+            false
+        },
+        || stop.load(Ordering::Relaxed),
+        |_| {},
+    );
+    assert_eq!(swept, None);
+}
+
 /// `library.browse` follows `library.source`'s rule for the active photo (issue #1447): a library
 /// photo the folder doesn't show gives way to the folder's first photo; one it shows stays.
 #[test]

crates/ui-egui/src/missing_sweep.rs

@@ -38,9 +38,33 @@ const KEEP: Duration = Duration::from_secs(10 * 60);
 #[derive(Default)]
 pub struct State {
     /// Each request with the library ([`lightcraft_engine::Session::library_generation`]) it asked about.
-    waiting: Vec<(u64, Sender<Value>)>,
-    /// A finished check's answer that no request received: its library and when it was kept.
-    kept: Option<(u64, Instant, Value)>,
+    pub(crate) waiting: Vec<(u64, Sender<Value>)>,
+    /// A finished check's answer that no request received.
+    kept: Option<Kept>,
+}
+
+/// An answer nobody received: its library, the photos it was of ([`missing::fingerprint`]), when it was kept.
+struct Kept {
+    library: u64,
+    snapshot: u64,
+    at: Instant,
+    answer: Value,
+}
+
+impl State {
+    /// Send `answer` to every request waiting on `library`; `true` if any caller was still there.
+    /// (a request whose caller gave up has no one to send to)
+    fn answer(&mut self, library: u64, answer: &Value) -> bool {
+        let (theirs, others): (Vec<_>, Vec<_>) = std::mem::take(&mut self.waiting).into_iter().partition(|(l, _)| *l == library);
+        self.waiting = others;
+        theirs.into_iter().fold(false, |received, (_, waiting)| waiting.send(answer.clone()).is_ok() || received)
+    }
+
+    /// A check for `library` could not start: fail its requests, and only those (another library's
+    /// check may be running for the others).
+    pub(crate) fn fail(&mut self, library: u64, error: &str) {
+        self.answer(library, &control::failure(error));
+    }
 }
 
 /// Answer `library.missing` from the control channel: give the answer kept from a check nobody
@@ -48,17 +72,19 @@ pub struct State {
 /// `reply` when the check ends.
 pub fn ask(app: &mut LightcraftApp, reply: Sender<Value>) -> Outcome {
     let library = app.session.library_generation();
-    if let Some((kept_for, at, answer)) = app.missing.kept.take()
-        && kept_for == library
-        && at.elapsed() < KEEP
+    if let Some(kept) = app.missing.kept.take()
+        && kept.library == library
+        && kept.at.elapsed() < KEEP
+        && kept.snapshot == missing::fingerprint(&app.session.catalog)
     {
-        return Outcome::Done(answer);
+        return Outcome::Done(kept.answer);
     }
     app.missing.waiting.push((library, reply));
     if app.tasks.is_running(LABEL) && app.missing.waiting.iter().filter(|(l, _)| *l == library).count() > 1 {
         return Outcome::Later;
     }
     let files = missing::checked(&app.session.catalog);
+    let snapshot = missing::fingerprint(&app.session.catalog);
     let total = files.len() as u64;
     let exists = app.session.media.availability.probe();
     let cancel = Arc::new(AtomicBool::new(false));
@@ -75,22 +101,21 @@ pub fn ask(app: &mut LightcraftApp, reply: Sender<Value>) -> Outcome {
             _ if app.session.library_generation() != library => {
                 (control::failure("Missing Photos: another library was opened before the check ended"), false)
             }
+            // (photos added, removed or relinked while the files were checked: this is not the catalog's answer)
+            Ok(_) if missing::fingerprint(&app.session.catalog) != snapshot => {
+                (control::failure("Missing Photos: the photos changed while the files were checked, ask again"), false)
+            }
             Err(e) => (control::failure(e), false),
             Ok(None) => (control::failure("Missing Photos: the check was stopped"), false),
             Ok(Some(gone)) => (control::success(missing::missing_json(gone)), true),
         };
-        let (theirs, others): (Vec<_>, Vec<_>) = std::mem::take(&mut app.missing.waiting).into_iter().partition(|(l, _)| *l == library);
-        app.missing.waiting = others;
-        // (a request whose caller gave up has no one to send to)
-        let received = theirs.into_iter().fold(false, |received, (_, waiting)| waiting.send(answer.clone()).is_ok() || received);
+        let received = app.missing.answer(library, &answer);
         if complete && !received {
-            app.missing.kept = Some((library, Instant::now(), answer));
+            app.missing.kept = Some(Kept { library, snapshot, at: Instant::now(), answer });
         }
     };
     if let Err(e) = crate::tasks::spawn_tracked(app, LABEL, KIND, cancel, work, done) {
-        for (_, waiting) in std::mem::take(&mut app.missing.waiting) {
-            let _ = waiting.send(control::failure(&e));
-        }
+        app.missing.fail(library, &e);
     }
     Outcome::Later
 }

crates/ui-egui/src/tests/missing_sweep.rs

@@ -171,3 +171,74 @@ fn a_check_that_panics_is_answered_with_an_error() {
     let rx = ask(&mut h);
     assert_eq!(answer(&mut h, &rx)["result"], json!([]));
 }
+
+/// Photos added while the files are being checked were not in that check: its answer is not given as
+/// the library's current one, and is not kept for a later request either.
+#[test]
+fn a_check_of_an_earlier_catalog_is_not_answered_as_current() {
+    let (mut h, share) = app_on_a_slow_share(Duration::from_secs(3));
+    let rx = ask(&mut h);
+    for _ in 0..3 {
+        h.step();
+    }
+    let p =
+        Photo::new(PhotoId(PHOTOS + 1), Source::File { path: "/share/IMG_9999.jpg".into() }, "IMG_9999.jpg", "JPEG", 60, 40, "2026-01-01T00:00:00");
+    h.app.session.catalog.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
+    share.open.store(true, Ordering::SeqCst);
+    let reply = answer(&mut h, &rx);
+    assert_eq!(reply["ok"], false, "{reply}");
+    assert!(reply["error"].as_str().is_some_and(|e| e.contains("changed")), "{reply}");
+    assert!(h.step_until(T, |h| h.app.tasks.is_empty()));
+    let calls = share.calls.load(Ordering::SeqCst);
+    let again = ask(&mut h);
+    let reply = answer(&mut h, &again);
+    assert_eq!(reply["ok"], true, "{reply}");
+    assert!(share.calls.load(Ordering::SeqCst) > calls, "checked afresh, not given the stale answer");
+}
+
+/// An edit that leaves the photos' files alone (a rating) does not spoil the check.
+#[test]
+fn an_edit_that_leaves_the_files_alone_does_not_spoil_the_check() {
+    let (mut h, share) = app_on_a_slow_share(Duration::from_secs(3));
+    let rx = ask(&mut h);
+    for _ in 0..3 {
+        h.step();
+    }
+    h.app.session.catalog.apply(Op::SetRating { id: PhotoId(1), rating: 3 }).unwrap();
+    share.open.store(true, Ordering::SeqCst);
+    assert_eq!(answer(&mut h, &rx)["ok"], true);
+}
+
+/// A check that can't start for one library fails only the requests for that library.
+#[test]
+fn a_failed_start_fails_only_that_librarys_requests() {
+    let mut state = crate::missing_sweep::State::default();
+    let (a_tx, a_rx) = std::sync::mpsc::channel();
+    let (b_tx, b_rx) = std::sync::mpsc::channel();
+    state.waiting.push((1, a_tx));
+    state.waiting.push((2, b_tx));
+    state.fail(2, "Missing Photos: could not start");
+    assert!(b_rx.try_recv().is_ok_and(|v| v["ok"] == false));
+    assert!(a_rx.try_recv().is_err(), "the other library's request keeps waiting");
+    assert_eq!(state.waiting.len(), 1);
+}
+
+/// An answer kept for a request that never came is not given once the photos have changed.
+#[test]
+fn a_kept_answer_is_not_given_after_the_photos_changed() {
+    let (mut h, share) = app_on_a_slow_share(Duration::from_secs(3));
+    drop(ask(&mut h));
+    for _ in 0..3 {
+        h.step();
+    }
+    share.open.store(true, Ordering::SeqCst);
+    assert!(h.step_until(T, |h| h.app.tasks.is_empty()));
+    let p =
+        Photo::new(PhotoId(PHOTOS + 1), Source::File { path: "/share/IMG_0000x.jpg".into() }, "IMG_0000x.jpg", "JPEG", 60, 40, "2026-01-01T00:00:00");
+    h.app.session.catalog.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
+    let calls = share.calls.load(Ordering::SeqCst);
+    let again = ask(&mut h);
+    let reply = answer(&mut h, &again);
+    assert_eq!(reply["result"].as_array().map(Vec::len), Some(11), "{reply}");
+    assert!(share.calls.load(Ordering::SeqCst) > calls);
+}

docs/whats-new.md

@@ -56,7 +56,9 @@
   minutes. The files are now checked in the background: a "Missing Photos" row in the activity stack counts them and
   its ✕ stops the check, and the request is answered with the same list when the check ends. A second request meanwhile
   shares the running check. A caller that stops waiting (the control server gives a request 60 s) gets the answer of
-  that check when it asks again within ten minutes, instead of another walk of the share.
+  that check when it asks again within ten minutes, instead of another walk of the share. If photos are added, removed
+  or relinked while the files are being checked, that answer is not given as current: the request is told to ask
+  again.
 
 ### Choose where a Lightroom catalog goes
 - File ▸ Import Lightroom Catalog… now asks first: merge the catalog's photos, collections and edits into the