From 20d156c8784d9c221f76d48f573ae33e31bd1122 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 10:39:11 -0400 Subject: [PATCH] Issue #34: Missing Photos checks library photos only Missing Photos (sidebar count, the view, library.missing, Find Missing) now shares one scope rule, cmd::missing::checked_path: non-deleted library photos backed by a file. Local browse records, which can outnumber the library by orders of magnitude, are never stat'ed. The Missing view checks only the photos its query kept instead of every catalog record. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/missing.rs | 40 ++++++++++++++++++++++++------- crates/engine/src/lib.rs | 5 ++-- crates/engine/src/tests_import.rs | 28 ++++++++++++++++++++++ crates/ui-egui/src/panels/left.rs | 19 +++++---------- docs/parity.md | 2 +- 5 files changed, 70 insertions(+), 24 deletions(-) diff --git a/crates/engine/src/cmd/missing.rs b/crates/engine/src/cmd/missing.rs index 8ec96355b..c7d1a2ec4 100644 --- a/crates/engine/src/cmd/missing.rs +++ b/crates/engine/src/cmd/missing.rs @@ -5,24 +5,48 @@ use std::collections::HashMap; use std::path::Path; -use lightcraft_catalog::{Op, PhotoId, Source}; +use lightcraft_catalog::{Catalog, Op, Photo, PhotoId, Source}; use serde_json::{Value, json}; use super::{CommandSpec, always, bad, cmd, str_param}; use crate::{Result, Session}; +/// The file Missing Photos checks for `p`, if it is in scope: a library photo (not in Recently +/// Deleted, not a Local-only record seen while browsing a folder) whose original is a file. +/// +/// Local browse records are out of scope on purpose: browsing can leave tens of thousands of +/// them in the catalog, they never show in library views (Missing Photos included), and a +/// folder that is gone is simply not browsed again. The sidebar count, the Missing Photos view, +/// `library.missing` and Find Missing Photos all use this one rule. +pub fn checked_path(p: &Photo) -> Option<&str> { + match &p.source { + Source::File { path } if p.in_library() => Some(path), + _ => None, + } +} + +/// Every in-scope photo's file ([`checked_path`]), without touching the disk. +pub fn candidates(cat: &Catalog) -> Vec { + cat.photos().filter_map(|p| checked_path(p).map(str::to_string)).collect() +} + +/// [`missing`] with the existence check supplied (tests count the file-system calls). +pub fn missing_with(cat: &Catalog, mut exists: impl FnMut(&str) -> bool) -> Vec<(PhotoId, String)> { + cat.photos().filter_map(|p| checked_path(p).filter(|f| !exists(f)).map(|f| (p.id, f.to_string()))).collect() +} + /// Library photos whose original file can't be found: (id, path). pub fn missing(s: &Session) -> Vec<(PhotoId, String)> { if cfg!(target_arch = "wasm32") { return Vec::new(); } - s.catalog - .photos() - .filter_map(|p| match &p.source { - Source::File { path } if !p.deleted && !Path::new(path).exists() => Some((p.id, path.clone())), - _ => None, - }) - .collect() + missing_with(&s.catalog, |f| Path::new(f).exists()) +} + +/// Whether photo `id` is in scope and its file is gone (the Missing Photos view checks only +/// the photos its query already narrowed to). +pub fn is_missing(cat: &Catalog, id: PhotoId) -> bool { + !cfg!(target_arch = "wasm32") && cat.photo(id).and_then(|p| checked_path(p)).is_some_and(|f| !Path::new(f).exists()) } fn relink_op(id: PhotoId, path: &str) -> Op { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index ce0404f23..192d90be5 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -507,8 +507,9 @@ impl Session { }); } if self.source == LibrarySource::Missing { - let missing: std::collections::HashSet = cmd::missing::missing(self).into_iter().map(|m| m.0).collect(); - self.visible.retain(|id| missing.contains(id)); + // only the photos the query kept (library photos, not Local browse records) are checked + let cat = &self.catalog; + self.visible.retain(|id| cmd::missing::is_missing(cat, *id)); } if self.source != LibrarySource::RecentlyDeleted { self.visible = self.catalog.arrange_stacks(&self.visible); diff --git a/crates/engine/src/tests_import.rs b/crates/engine/src/tests_import.rs index 2a1480ba1..b927dd1bb 100644 --- a/crates/engine/src/tests_import.rs +++ b/crates/engine/src/tests_import.rs @@ -585,3 +585,31 @@ fn smart_previews_folder_is_chosen_per_library() { assert_eq!(r["custom"], false, "{r}"); assert_eq!(r["path"], lib.join("Smart Previews").to_string_lossy().as_ref()); } + +/// Missing Photos covers library photos only: a small library next to many Local browse +/// records (and a deleted photo) checks — and lists, and counts — the library files alone. +#[test] +fn missing_photos_skip_local_browse_records() { + use lightcraft_catalog::{Op, Photo, PhotoId, Source}; + let gone = std::env::temp_dir().join(format!("lc-missing-scope-{}", std::process::id())); + let mut s = Session::new(); + let mut ops = Vec::new(); + for i in 0..205u64 { + let path = gone.join(format!("IMG_{i:04}.jpg")).to_string_lossy().to_string(); + let mut p = Photo::new(PhotoId(i + 1), Source::File { path }, &format!("IMG_{i:04}.jpg"), "JPEG", 60, 40, "2026-01-01T00:00:00"); + p.local = i >= 5; // 5 library photos, 200 seen while browsing + p.deleted = i == 4; + ops.push(Op::AddPhoto { photo: Box::new(p) }); + } + s.commit("Add", Op::Batch { ops }).unwrap(); + let mut checked = 0; + let lost = crate::cmd::missing::missing_with(&s.catalog, |_| { + checked += 1; + false + }); + assert_eq!((checked, lost.len()), (4, 4), "only the non-deleted library files are checked"); + assert_eq!(crate::cmd::missing::candidates(&s.catalog).len(), 4, "the sidebar count checks the same files"); + assert_eq!(s.execute("library.missing", &json!({})).unwrap().as_array().map(Vec::len), Some(4)); + s.execute("library.source", &json!({"kind": "missing"})).unwrap(); + assert_eq!(s.visible_cloned().len(), 4, "the view lists what the count counts"); +} diff --git a/crates/ui-egui/src/panels/left.rs b/crates/ui-egui/src/panels/left.rs index 725163226..9e68ac7a6 100644 --- a/crates/ui-egui/src/panels/left.rs +++ b/crates/ui-egui/src/panels/left.rs @@ -146,9 +146,10 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { }); } -/// How many library photos have no file. Checking stats every file, which on a network share -/// takes seconds, so it runs on a worker thread: the count shown is the last finished one, -/// refreshed at most every 5 s (or when the catalog changed). +/// How many library photos have no file (Local browse records are not checked; see +/// `cmd::missing::checked_path`). Checking stats every file, which on a network share takes +/// seconds, so it runs on a worker thread: the count shown is the last finished one, refreshed +/// at most every 5 s, and at once (after the running check) when the catalog changed. fn missing_count(app: &mut LightcraftApp, ui: &mut egui::Ui) -> usize { use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering::Relaxed}; #[derive(Clone, Default)] @@ -164,16 +165,8 @@ fn missing_count(app: &mut LightcraftApp, ui: &mut egui::Ui) -> usize { let mut job: Job = ui.data(|d| d.get_temp(id)).unwrap_or(Job { rev: u64::MAX, at: f64::MIN, ..Default::default() }); let fresh = now - job.at < 5.0 && job.rev == rev; if !fresh && !job.running.load(Relaxed) { - let paths: Vec = app - .session - .catalog - .photos() - .filter(|p| !p.deleted) - .filter_map(|p| match &p.source { - lightcraft_catalog::Source::File { path } => Some(path.clone()), - _ => None, - }) - .collect(); + // the same scope as the Missing Photos view: library photos only, never Local browse records + let paths = lightcraft_engine::cmd::missing::candidates(&app.session.catalog); job.running.store(true, Relaxed); job.at = now; job.rev = rev; diff --git a/docs/parity.md b/docs/parity.md index a33caa461..51edb8358 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -708,7 +708,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06- | LRC-LIB-PEOPLE | People view | P2 | ⬜ | | | | LRC-LIB-COMMENTS | Comments panel | P2 | ⬜ | | | | LRC-LIB-VISUALSEARCH | Find similar photos | P2 | ✅ | `cmd:library.findSimilar`, `crates/pipeline/src/cull.rs` (`signature`) | photo menu / Photo ▸ Find Similar Photos: look-alike photos (composition and tones), most similar first, as a filter (`only`) | -| LRC-LIB-MISSING | Missing files & relink | P1 | ✅ | `cmd:library.missing`, `cmd:photo.relink`, `cmd:library.findMissing`, `cmd:file.findMissing`, `cmd:photo.locate` | File → Find Missing Photos… (same name and size anywhere in a folder, one undo step), photo menu → Locate Missing File…; unreadable files show "!" in the grid and a reason in the loupe; undo never moves files. No missing-photos collection | +| LRC-LIB-MISSING | Missing files & relink | P1 | ✅ | `cmd:library.missing`, `cmd:photo.relink`, `cmd:library.findMissing`, `cmd:file.findMissing`, `cmd:photo.locate`, `crates/engine/src/cmd/missing.rs` (`checked_path`) | File → Find Missing Photos… (same name and size anywhere in a folder, one undo step), photo menu → Locate Missing File…; unreadable files show "!" in the grid and a reason in the loupe; undo never moves files. Missing Photos in the sidebar (counted on a worker thread); it covers library photos only — Local browse records are never checked, for the count, the view, `library.missing` and Find Missing alike | | LRC-LIB-CONVERT | Convert to DNG | P2 | ✅ | `cmd:photo.convertToDng`, `crates/engine/src/cmd/convert.rs` | Photo ▸ Convert to DNG: lossless DNG next to the raw with the settings embedded, photo (and its virtual copies) relinked; originals kept; undoable | | LRC-LIB-PREVIEWS | Build / discard previews | P1 | ✅ | `cmd:library.buildPreviews`, `cmd:library.previewProgress`, `cmd:library.cancelPreviews`, `cmd:library.clearPreviews`, `crates/engine/src/cmd/previews.rs`, `crates/preview/src/lib.rs` | File ▸ Previews: build standard-sized (Settings → Performance size) or 1:1 previews of the selected / visible photos in the background (progress toasts, stop), discard the cache; disk thumbnail + view cache; no smart previews (offline proxies) | | LRC-LIB-SLIDESHOW-IMPROMPTU | Impromptu slideshow | P2 | ✅ | `cmd:view.slideshow` | see LR-VIEW-SLIDESHOW |