From 898ecee27cbcb5586029368725b55800cebdb47b Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 9 Oct 2026 18:28:12 -0400 Subject: [PATCH] Synchronize Folder: relink renamed photos without a hash, and duplicates (review fixes) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moved-file matching only knew photos with a content hash, one per hash: a photo taken over from a Lightroom catalog (no hash on record) that was renamed still showed as missing with its file as new, so Remove missing + Import new swapped it for a bare copy; a Photo › Duplicate (hash ":dup") never matched; and of two missing files with the same content only the first was found. Hashes now match by the content they stand for, each missing file claiming one file; a photo with no hash matches by file size and dimensions, only when exactly one missing file and one unclaimed file agree. A Local record listed as new no longer carries its "duplicate"/"existing" marks in the scan. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/sync.rs | 81 ++++++++++++++++++++++++++------- crates/engine/src/tests_sync.rs | 32 +++++++++++++ 2 files changed, 97 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/sync.rs b/crates/engine/src/sync.rs index c91b4c7b3..5e74fb220 100644 --- a/crates/engine/src/sync.rs +++ b/crates/engine/src/sync.rs @@ -212,28 +212,15 @@ pub fn scan_with(input: SyncInput, progress: &SyncProgress) -> FolderChanges { Check::Nothing => {} } } - // a missing photo's content found under another name in the folder: the file was renamed or - // moved, and the photo (with its edits) follows it rather than going missing - let mut gone_by_hash: HashMap<&str, &str> = HashMap::new(); - for m in &changes.missing { - if let Some(h) = labels.photo(PhotoId(m.id)).and_then(|p| p.content_hash.as_deref()) { - gone_by_hash.entry(h).or_insert(m.path.as_str()); - } - } - let mut moved_from: HashMap = HashMap::new(); + let moved_from = match_moved(&changes.missing, &out.candidates, &out.probes, &labels); for c in out.candidates { - let hash = out.probes.get(&c.path).and_then(|i| i.content_hash.as_deref()); - if c.error.is_none() - && c.duplicate.as_deref() != Some("path") - && let Some(from) = hash.and_then(|h| gone_by_hash.remove(h)) - { - moved_from.insert(from.to_string(), c.path.clone()); + if moved_from.values().any(|to| *to == c.path) { continue; } let browsed = c.existing.and_then(|id| labels.photo(PhotoId(id))).is_some_and(|p| p.local && !p.deleted); match (c.duplicate.as_deref(), &c.error) { // a file only looked at in Local is not in the library yet: it is new - (Some("path"), _) if browsed => changes.new.push(c), + (Some("path"), _) if browsed => changes.new.push(ImportCandidate { duplicate: None, existing: None, ..c }), (Some("path"), _) => {} (Some(_), _) => changes.duplicates += 1, (None, Some(_)) => changes.unreadable.push(c), @@ -251,6 +238,68 @@ pub fn scan_with(input: SyncInput, progress: &SyncProgress) -> FolderChanges { changes } +/// The content a hash stands for: a Duplicate's own hash is the original's plus `:dup`. +fn base_hash(h: &str) -> &str { + h.split(':').next().unwrap_or(h) +} + +/// Missing photos' files found elsewhere in the folder (old path → new path): the file was +/// renamed or moved, and the photo (with its edits) follows it rather than going missing. +/// +/// By content first: each missing file's hash claims one file with that content (two missing +/// files with the same content claim two). A photo with no hash on record (taken over from a +/// Lightroom catalog) is matched by file size and dimensions, and only when exactly one missing +/// file and exactly one unclaimed file of the folder agree on them. +fn match_moved( + missing: &[FolderPhoto], + candidates: &[ImportCandidate], + probes: &HashMap, + labels: &Catalog, +) -> HashMap { + // distinct missing files (a photo's virtual copies share its file), with what is known of them + let mut files: Vec<(&str, Option<&Photo>)> = Vec::new(); + for m in missing { + if !files.iter().any(|(p, _)| *p == m.path) { + files.push((m.path.as_str(), labels.photo(PhotoId(m.id)).map(|p| &**p))); + } + } + let mut by_hash: HashMap<&str, std::collections::VecDeque<&str>> = HashMap::new(); + for (path, p) in &files { + if let Some(h) = p.and_then(|p| p.content_hash.as_deref()) { + by_hash.entry(base_hash(h)).or_default().push_back(path); + } + } + let usable = |c: &&ImportCandidate| c.error.is_none() && c.duplicate.as_deref() != Some("path"); + let mut moved: HashMap = HashMap::new(); + for c in candidates.iter().filter(usable) { + let hash = probes.get(&c.path).and_then(|i| i.content_hash.as_deref()); + if let Some(from) = hash.and_then(|h| by_hash.get_mut(base_hash(h))).and_then(std::collections::VecDeque::pop_front) { + moved.insert(from.to_string(), c.path.clone()); + } + } + let key = |size: u64, w: u32, h: u32| (size > 0 && w > 0 && h > 0).then_some((size, w, h)); + let mut hashless: HashMap<(u64, u32, u32), Vec<&str>> = HashMap::new(); + for (path, p) in &files { + if let Some(p) = p.filter(|p| p.content_hash.is_none()) + && let Some(k) = key(p.file_size, p.width, p.height) + { + hashless.entry(k).or_default().push(path); + } + } + let mut found: HashMap<(u64, u32, u32), Vec<&str>> = HashMap::new(); + for c in candidates.iter().filter(usable).filter(|c| !moved.values().any(|to| *to == c.path)) { + if let Some(k) = key(c.file_size, c.width, c.height) { + found.entry(k).or_default().push(c.path.as_str()); + } + } + for (k, from) in hashless { + if let ([from], Some([to])) = (from.as_slice(), found.get(&k).map(Vec::as_slice)) { + moved.insert(from.to_string(), to.to_string()); + } + } + moved +} + /// What checking one photo of the folder found. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] enum Check { diff --git a/crates/engine/src/tests_sync.rs b/crates/engine/src/tests_sync.rs index 3ac2030ee..616745120 100644 --- a/crates/engine/src/tests_sync.rs +++ b/crates/engine/src/tests_sync.rs @@ -419,3 +419,35 @@ fn a_sidecar_the_library_wrote_itself_is_no_update() { let r = scan(&mut s, &dir.path("trip")); assert!(paths(&r, "metadata").is_empty(), "{r}"); } + +#[test] +fn a_renamed_photo_with_no_content_hash_is_relinked_when_its_file_is_unmistakable() { + // as photos taken over from a Lightroom catalog are: no hash on record + let dir = Scratch::new("nohash"); + let mut s = library(&dir); + let a = s.catalog.photos().find(|p| p.file_name == "a.png").unwrap().id; + let mut p = (**s.catalog.photo(a).unwrap()).clone(); + p.content_hash = None; + s.catalog.apply(lightcraft_catalog::Op::RemovePhoto { id: a }).unwrap(); + s.catalog.apply(lightcraft_catalog::Op::AddPhoto { photo: Box::new(p) }).unwrap(); + std::fs::rename(dir.path("trip/a.png"), dir.path("trip/a-renamed.png")).unwrap(); + let r = scan(&mut s, &dir.path("trip")); + assert_eq!(r["moved"][0]["to"], dir.path("trip/a-renamed.png"), "{r}"); + assert!(paths(&r, "missing").is_empty() && paths(&r, "new").is_empty(), "{r}"); +} + +#[test] +fn renamed_duplicates_are_each_relinked() { + let dir = Scratch::new("dups"); + let mut s = library(&dir); + let a = s.catalog.photos().find(|p| p.file_name == "a.png").unwrap().id; + s.execute("photo.duplicate", &json!({"ids": [a.0]})).unwrap(); + assert!(Path::new(&dir.path("trip/a-copy.png")).exists()); + std::fs::rename(dir.path("trip/a.png"), dir.path("trip/one.png")).unwrap(); + std::fs::rename(dir.path("trip/a-copy.png"), dir.path("trip/two.png")).unwrap(); + let r = scan(&mut s, &dir.path("trip")); + assert_eq!(r["moved"].as_array().map(Vec::len), Some(2), "{r}"); + assert!(paths(&r, "missing").is_empty(), "{r}"); + let r = s.execute("folder.synchronize", &json!({"path": dir.path("trip")})).unwrap(); + assert_eq!(r["relinked"], 2, "{r}"); +}