LRC-LIB-MISSING: answer a check over changing photos with what still holds instead of refusing it

crates/engine/src/cmd/missing.rs

@@ -52,18 +52,35 @@ 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 {
+fn fingerprint_of<'a>(entries: impl Iterator<Item = (PhotoId, &'a str)>) -> u64 {
     use std::hash::{DefaultHasher, Hash, Hasher};
-    cat.photos().filter_map(|p| checked_path(p).map(|f| (p.id, f))).fold(0u64, |sum, entry| {
+    entries.fold(0u64, |sum, entry| {
         let mut h = DefaultHasher::new();
         entry.hash(&mut h);
         sum.wrapping_add(h.finish())
     })
 }
 
+/// What [`checked`] lists, as one number (the photos in scope and the file each one points at,
+/// whatever their order), without building the list. A caller that took `checked` and checked its
+/// files on another thread compares it with [`fingerprint_of_checked`] to know whether the photos
+/// changed meanwhile.
+pub fn fingerprint(cat: &Catalog) -> u64 {
+    fingerprint_of(cat.photos().filter_map(|p| checked_path(p).map(|f| (p.id, f))))
+}
+
+/// [`fingerprint`] of the list [`checked`] gave.
+pub fn fingerprint_of_checked(files: &[(PhotoId, String)]) -> u64 {
+    fingerprint_of(files.iter().map(|(id, f)| (*id, f.as_str())))
+}
+
+/// What of `gone` (a sweep's answer, of the catalog as it was) still holds in `cat` as it is now: the
+/// photos still in scope that still point at the file that was not found. A photo removed, moved to
+/// Recently Deleted or relinked since is not reported as missing; one added since was not checked.
+pub fn still_missing(cat: &Catalog, gone: Vec<(PhotoId, String)>) -> Vec<(PhotoId, String)> {
+    gone.into_iter().filter(|(id, file)| cat.photo(*id).and_then(|p| checked_path(p)) == Some(file.as_str())).collect()
+}
+
 /// How many files [`sweep`] checks between two reports of its progress.
 const PROGRESS_EVERY: usize = 64;
 

crates/ui-egui/src/missing_sweep.rs

@@ -84,7 +84,7 @@ pub fn ask(app: &mut LightcraftApp, reply: Sender<Value>) -> Outcome {
         return Outcome::Later;
     }
     let files = missing::checked(&app.session.catalog);
-    let snapshot = missing::fingerprint(&app.session.catalog);
+    let snapshot = missing::fingerprint_of_checked(&files);
     let total = files.len() as u64;
     let exists = app.session.media.availability.probe();
     let cancel = Arc::new(AtomicBool::new(false));
@@ -101,13 +101,12 @@ 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),
+            Ok(Some(gone)) if missing::fingerprint(&app.session.catalog) == snapshot => (control::success(missing::missing_json(gone)), true),
+            // (photos changed while the files were checked, as an import does: the answer is what of the
+            // check still holds, and is not kept, for the next request must see the photos as they are then)
+            Ok(Some(gone)) => (control::success(missing::missing_json(missing::still_missing(&app.session.catalog, gone))), false),
         };
         let received = app.missing.answer(library, &answer);
         if complete && !received {

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

@@ -172,28 +172,69 @@ fn a_check_that_panics_is_answered_with_an_error() {
     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.
+fn new_photo(id: u64, path: &str) -> Box<Photo> {
+    Box::new(Photo::new(PhotoId(id), Source::File { path: path.into() }, "x.jpg", "JPEG", 60, 40, "2026-01-01T00:00:00"))
+}
+
+/// Photos change while the files are being checked (auto-import adds some every few seconds, a share check
+/// takes minutes): the answer is the check's, kept to the photos that are still where it looked, so asking
+/// again never has to be the answer. A photo that was missing and has been removed, moved to Recently
+/// Deleted or relinked since is not reported; one added since was not checked and is not either.
 #[test]
-fn a_check_of_an_earlier_catalog_is_not_answered_as_current() {
+fn a_check_over_changing_photos_answers_what_still_holds() {
     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();
+    let cat = &mut h.app.session.catalog;
+    cat.apply(Op::RemovePhoto { id: PhotoId(1) }).unwrap();
+    cat.apply(Op::SetDeleted { id: PhotoId(2), deleted: true }).unwrap();
+    let relinked =
+        Op::Relink { id: PhotoId(3), file_name: "y.jpg".into(), source: Source::File { path: "/share/IMG_9998.jpg".into() }, format: None };
+    cat.apply(relinked).unwrap();
+    cat.apply(Op::AddPhoto { photo: new_photo(PHOTOS + 1, "/share/IMG_0000x.jpg") }).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_eq!(reply["ok"], true, "{reply}");
+    let ids: Vec<u64> = reply["result"].as_array().unwrap().iter().map(|r| r["id"].as_u64().unwrap()).collect();
+    assert_eq!(ids, (4..=10).collect::<Vec<_>>(), "{reply}");
+}
+
+/// A stop is a stop, whatever the photos did meanwhile.
+#[test]
+fn a_stopped_check_says_stopped_even_if_the_photos_changed() {
+    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::AddPhoto { photo: new_photo(PHOTOS + 1, "/share/IMG_9999.jpg") }).unwrap();
+    let id = missing_rows(&h)[0].id;
+    let r = h.request("engine.execute", json!({"command": "activity.cancel", "params": {"id": id}}), T);
+    assert_eq!(r["ok"], true, "{r}");
+    share.open.store(true, Ordering::SeqCst);
+    let reply = answer(&mut h, &rx);
+    assert!(reply["error"].as_str().is_some_and(|e| e.contains("stopped")), "{reply}");
+}
+
+/// A check nobody waited for, over photos that changed while it ran, is not kept: the next request
+/// checks afresh (and then sees the photo that came meanwhile).
+#[test]
+fn a_check_over_changed_photos_is_not_kept_for_a_later_request() {
+    let (mut h, share) = app_on_a_slow_share(Duration::from_secs(3));
+    drop(ask(&mut h));
+    for _ in 0..3 {
+        h.step();
+    }
+    h.app.session.catalog.apply(Op::AddPhoto { photo: new_photo(PHOTOS + 1, "/share/IMG_0000x.jpg") }).unwrap();
+    share.open.store(true, Ordering::SeqCst);
     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");
+    assert_eq!(reply["result"].as_array().map(Vec::len), Some(11), "{reply}");
+    assert!(share.calls.load(Ordering::SeqCst) > calls, "checked afresh");
 }
 
 /// An edit that leaves the photos' files alone (a rating) does not spoil the check.

docs/whats-new.md

@@ -56,9 +56,10 @@
   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. 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.
+  that check when it asks again within ten minutes, instead of another walk of the share, as long as the photos have
+  not changed since; otherwise the files are checked again. If photos change while the files are being checked (an
+  import adds some, others are removed, moved to Recently Deleted or relinked), the answer leaves out the photos that
+  no longer apply, a photo added meanwhile is not part of that check, and the answer is not kept for a later request.
 
 ### Choose where a Lightroom catalog goes
 - File ▸ Import Lightroom Catalog… now asks first: merge the catalog's photos, collections and edits into the