Windows: treat separator-only path differences as the same file

Catalog paths may mix `/` and `\` on Windows, so raw string comparison saw
`N:/photos/IMG.jpg` and `N:/photos\IMG.jpg` as different files:

- Rename: planning, the move list, move_file_with and file_moves now compare
  as paths. A separator-only "rename" is a no-op: no file move, no catalog
  change and no undo entry, alone or batched with a genuine rename.
  Previously it attempted a plain rename onto the existing file, failed, and
  rolled back the rest of the batch. Case-only renames still work (#95).
- Import: the already-in-library index is keyed by PathBuf.
- Tests build expected paths with Path::join, and the CLI test escapes the
  device path with serde_json instead of string formatting.

Split from a327b98 (T0 thumbnail work), with the follow-up rename fix that
the committed version needed.

Co-authored-by: Codex <noreply@openai.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

apps/lightcraft-cli/tests/cli.rs

@@ -292,7 +292,7 @@ fn devices_are_listed_and_imported_from() {
     assert_eq!(dev["name"], "CARD", "{line}");
     // the import review of its DCIM folder finds the photo
     let dcim = dev["path"].as_str().unwrap().to_string();
-    let o = Command::new(BIN).args(["run", "library.importPreview", &format!("paths=[\"{dcim}\"]")]).output().unwrap();
+    let o = Command::new(BIN).args(["run", "library.importPreview", &format!("paths={}", serde_json::json!([dcim]))]).output().unwrap();
     let line: Value = serde_json::from_slice(o.stdout.split(|b| *b == b'\n').next().unwrap()).unwrap();
     assert_eq!(line["result"]["candidates"].as_array().map(Vec::len), Some(1), "{line}");
 }

crates/engine/src/import.rs

@@ -393,7 +393,7 @@ pub struct ScanInput {
     probe: Option<crate::media::FileProbe>,
     skip: Option<PathBuf>,
     /// path → (photo, its summary) for files already in the library.
-    by_path: HashMap<String, (PhotoId, ImportCandidate)>,
+    by_path: HashMap<PathBuf, (PhotoId, ImportCandidate)>,
     by_hash: HashMap<String, PhotoId>,
     cache: HashMap<String, ProbeInfo>,
 }
@@ -432,7 +432,7 @@ impl ScanInput {
                     existing: Some(p.id.0),
                     ..Default::default()
                 };
-                by_path.insert(path.clone(), (p.id, c));
+                by_path.insert(PathBuf::from(path), (p.id, c));
             }
             if let Some(h) = &p.content_hash {
                 by_hash.insert(h.clone(), p.id);
@@ -458,7 +458,7 @@ pub fn scan(s: &mut Session, paths: &[String]) -> Vec<ImportCandidate> {
 pub fn scan_with(mut input: ScanInput, paths: &[String], progress: &ScanProgress) -> ScanOutput {
     use std::sync::atomic::Ordering::Relaxed;
     let files = expand(paths, input.skip.as_deref());
-    let todo: Vec<String> = files.iter().filter(|f| !input.by_path.contains_key(f.as_str())).cloned().collect();
+    let todo: Vec<String> = files.iter().filter(|f| !input.by_path.contains_key(Path::new(f))).cloned().collect();
     progress.total.store(todo.len(), Relaxed);
     // probes from a preceding `scan` are reused when the file is unchanged (same size)
     let cached: Vec<Option<ProbeInfo>> = todo
@@ -480,7 +480,7 @@ pub fn scan_with(mut input: ScanInput, paths: &[String], progress: &ScanProgress
     let mut out = Vec::with_capacity(files.len());
     let mut kept = HashMap::new();
     for f in files {
-        if let Some((_, c)) = input.by_path.get(&f) {
+        if let Some((_, c)) = input.by_path.get(Path::new(&f)) {
             let mut c = c.clone();
             c.name = Path::new(&f).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| f.clone());
             out.push(c);

crates/engine/src/rename.rs

@@ -395,7 +395,12 @@ impl MoveFs for RealFs {
 /// `from` → `to` differs only in letter case and `to` is that very file (a case-insensitive
 /// volume): the rename must go through a temporary name.
 fn is_respelling(fs: &dyn MoveFs, from: &Path, to: &Path) -> bool {
-    from != to && from.to_string_lossy().to_lowercase() == to.to_string_lossy().to_lowercase() && fs.exists(to) && fs.same_file(from, to)
+    from != to && folded_path(&from.to_string_lossy()) == folded_path(&to.to_string_lossy()) && fs.exists(to) && fs.same_file(from, to)
+}
+
+/// Windows accepts both separators; spelling differences must not hide file identity.
+fn folded_path(path: &str) -> String {
+    if cfg!(windows) { path.replace('\\', "/").to_lowercase() } else { path.to_lowercase() }
 }
 
 /// Rename `a` to `b` without ever replacing a file; a change of letter case only goes through a
@@ -439,7 +444,7 @@ pub fn move_file(from: &str, to: &str) -> std::result::Result<(), MoveError> {
 
 /// [`move_file`] on the file system `fs`.
 pub(crate) fn move_file_with(fs: &dyn MoveFs, from: &str, to: &str) -> std::result::Result<(), MoveError> {
-    if from == to {
+    if Path::new(from) == Path::new(to) {
         return Ok(());
     }
     let fail = |message: String| MoveError { message, moved: false };
@@ -590,11 +595,11 @@ fn plan_rename_core(
                 loop {
                     let name = candidate(k);
                     let tp = dir.join(&name).to_string_lossy().to_string();
-                    let key = tp.to_lowercase();
+                    let key = folded_path(&tp);
                     // this very file, maybe spelled differently (case-insensitive volume)? By
                     // identity: on a case-sensitive volume `img_1.jpg` may be another photo
                     let (from, to) = (Path::new(path), Path::new(&tp));
-                    let same = tp == *path || (from != to && key == path.to_lowercase() && exists(to) && same_file(from, to));
+                    let same = from == to || (from != to && key == folded_path(path) && exists(to) && same_file(from, to));
                     // free: not claimed in this batch and not on disk (unless it is this very file).
                     // A file this batch moves away still counts as taken: simple and safe.
                     if !taken.contains(&key) && (same || !exists(to)) {
@@ -656,6 +661,7 @@ impl Session {
             };
             let Some(plan) = plan else { continue };
             let source = match &plan.to_path {
+                Some(t) if matches!(&p.source, Source::File { path } if Path::new(path) == Path::new(t)) => p.source.clone(),
                 Some(t) => Source::File { path: t.clone() },
                 None => p.source.clone(),
             };
@@ -668,7 +674,7 @@ impl Session {
         let moves: Vec<(String, String)> = plans
             .iter()
             .filter_map(|pl| match (&pl.from_path, &pl.to_path) {
-                (Some(a), Some(b)) if a != b => Some((a.clone(), b.clone())),
+                (Some(a), Some(b)) if Path::new(a) != Path::new(b) => Some((a.clone(), b.clone())),
                 _ => None,
             })
             .collect();
@@ -732,7 +738,7 @@ impl Session {
                 Op::Batch { ops } => ops.iter().for_each(|o| rec(s, o, v)),
                 Op::SetFile { id, source: Source::File { path: to }, .. } => {
                     if let Some(Source::File { path: from }) = s.catalog.photo(*id).map(|p| &p.source)
-                        && from != to
+                        && Path::new(from) != Path::new(to)
                         && !v.iter().any(|(f, _)| f == from)
                     {
                         v.push((from.clone(), to.clone()));
@@ -770,11 +776,14 @@ mod tests {
         }
         fn idx(&self, p: &Path) -> Option<usize> {
             let p = p.to_string_lossy();
-            self.files.borrow().iter().position(|(f, _)| if self.ci { f.to_lowercase() == p.to_lowercase() } else { *f == p })
+            self.files
+                .borrow()
+                .iter()
+                .position(|(f, _)| if self.ci { folded_path(f) == folded_path(&p) } else { Path::new(f) == Path::new(p.as_ref()) })
         }
         /// The listing: (exact path, contents), sorted.
         pub fn listing(&self) -> Vec<(String, String)> {
-            let mut v: Vec<_> = self.files.borrow().iter().map(|(p, c)| (p.clone(), String::from_utf8_lossy(c).to_string())).collect();
+            let mut v: Vec<_> = self.files.borrow().iter().map(|(p, c)| (p.replace('\\', "/"), String::from_utf8_lossy(c).to_string())).collect();
             v.sort();
             v
         }
@@ -829,6 +838,48 @@ mod tests {
         Photo::new(PhotoId(id), Source::File { path: path.into() }, &name, "JPG", 1, 1, "2026-01-01T00:00:00")
     }
 
+    #[cfg(windows)]
+    #[test]
+    fn mixed_separator_noop_rename_preserves_files_catalog_and_undo() {
+        let fs = FakeFs::new(true, &[("N:/photos/IMG.jpg", "image"), ("N:/photos/IMG.xmp", "edits"), ("N:/photos/other.jpg", "other")]);
+        // Any attempt to move the unchanged photo (including rollback) must fail.
+        fs.fail.borrow_mut().push("IMG".into());
+        let mut s = Session::new();
+        for (id, path) in [(1, "N:/photos/IMG.jpg"), (2, "N:/photos/other.jpg")] {
+            s.catalog.apply(Op::AddPhoto { photo: Box::new(file_photo(id, path)) }).unwrap();
+        }
+        let before = fs.listing();
+        let plans = s.plan_rename_with(&fs, &[PhotoId(1)], "{name}", 1);
+        assert_ne!(plans[0].from_path, plans[0].to_path, "fixture exercises different separator spelling");
+        assert_eq!(s.apply_rename_with(&fs, &plans).unwrap(), 0);
+        assert_eq!(fs.listing(), before);
+        assert_eq!(s.catalog.photo(PhotoId(1)).unwrap().source, Source::File { path: "N:/photos/IMG.jpg".into() });
+        assert!(s.undo.is_empty());
+        move_file_with(&fs, "N:/photos/IMG.jpg", "N:/photos\\IMG.jpg").unwrap();
+        let separator_only = Op::SetFile { id: PhotoId(1), file_name: "IMG.jpg".into(), source: Source::File { path: "N:/photos\\IMG.jpg".into() } };
+        assert!(s.file_moves(&separator_only).is_empty());
+
+        // A batch may combine that no-op with a genuine rename, without touching IMG or its sidecar.
+        let mut batch = plans;
+        batch.extend(s.plan_rename_with(&fs, &[PhotoId(2)], "renamed", 1));
+        assert_eq!(s.apply_rename_with(&fs, &batch).unwrap(), 1);
+        assert_eq!(
+            fs.listing(),
+            vec![
+                ("N:/photos/IMG.jpg".into(), "image".into()),
+                ("N:/photos/IMG.xmp".into(), "edits".into()),
+                ("N:/photos/renamed.jpg".into(), "other".into())
+            ]
+        );
+        let undo = &s.undo.last().unwrap().op;
+        let moves = s.file_moves(undo);
+        assert_eq!(moves.len(), 1);
+        assert_eq!(Path::new(&moves[0].1), Path::new("N:/photos/other.jpg"));
+        move_all(&fs, &moves).unwrap();
+        s.catalog.apply(undo.clone()).unwrap();
+        assert_eq!(fs.listing(), before);
+    }
+
     /// Issue #95: a case-only rename is a real change on a case-insensitive volume and never
     /// replaces a different file that differs only by case on a case-sensitive one.
     #[test]
@@ -884,7 +935,7 @@ mod tests {
         let plans = s.plan_rename_with(&fs, &ids, "Trip-{seq}", 1);
         // c can't be renamed (the share went away), and neither can Trip-1 be moved back
         fs.fail.borrow_mut().extend(["c.jpg".to_string(), "Trip-1".to_string()]);
-        let err = s.apply_rename_with(&fs, &plans).unwrap_err().to_string();
+        let err = s.apply_rename_with(&fs, &plans).unwrap_err().to_string().replace('\\', "/");
         assert!(err.contains("rename /p/c.jpg"), "{err}");
         assert!(err.contains("/p/Trip-1.jpg could not be moved back"), "{err}");
         assert!(err.contains("/p/a.jpg → /p/Trip-1.jpg"), "lists what stayed renamed: {err}");
@@ -899,7 +950,7 @@ mod tests {
         );
         // the catalog matches the disk
         let path = |s: &Session, id: u64| match &s.catalog.photo(PhotoId(id)).unwrap().source {
-            Source::File { path } => path.clone(),
+            Source::File { path } => path.replace('\\', "/"),
             Source::Demo { .. } => String::new(),
         };
         assert_eq!(path(&s, 1), "/p/Trip-1.jpg");
@@ -913,7 +964,7 @@ mod tests {
         fs.fail.borrow_mut().extend(["d.jpg.xmp".to_string(), "/p/e.jpg".to_string()]);
         let e = move_file_with(&fs, "/p/d.jpg", "/p/e.jpg").unwrap_err();
         assert!(e.moved, "{e}");
-        assert!(e.message.contains("it is now /p/e.jpg with its sidecar /p/e.xmp"), "{e}");
+        assert!(e.message.replace('\\', "/").contains("it is now /p/e.jpg with its sidecar /p/e.xmp"), "{e}");
         assert_eq!(fs.listing(), vec![("/p/d.jpg.xmp".into(), "Df".into()), ("/p/e.jpg".into(), "D".into()), ("/p/e.xmp".into(), "Ds".into())]);
         // … and when the file does go back, its sidecars follow it
         let fs = FakeFs::new(false, &[("/p/d.jpg", "D"), ("/p/d.xmp", "Ds"), ("/p/d.jpg.xmp", "Df")]);

crates/engine/src/tests_import.rs

@@ -181,8 +181,12 @@ fn find_missing_matches_renamed_files_by_content() {
     let to = |name: &str| {
         found.iter().find(|f| f["from"].as_str().unwrap().ends_with(name)).map(|f| (f["to"].as_str().unwrap().to_string(), f["by"].clone()))
     };
-    assert_eq!(to("a.png"), Some((dir.join("moved/Trip-001.png").to_string_lossy().to_string(), json!("content"))));
-    assert_eq!(to("b.png"), Some((dir.join("moved/real/Trip-002.png").to_string_lossy().to_string(), json!("content"))), "not the impostor");
+    assert_eq!(to("a.png"), Some((dir.join("moved").join("Trip-001.png").to_string_lossy().to_string(), json!("content"))));
+    assert_eq!(
+        to("b.png"),
+        Some((dir.join("moved").join("real").join("Trip-002.png").to_string_lossy().to_string(), json!("content"))),
+        "not the impostor"
+    );
     assert_eq!(r["missing"], 1);
     assert_eq!(r["ambiguous"][0]["candidates"].as_array().map(Vec::len), Some(2), "{r}");
     let _ = std::fs::remove_dir_all(&dir);
@@ -582,7 +586,7 @@ fn folders_rename_and_move_with_their_photos() {
     assert!(s.execute("folder.move", &json!({"path": b.to_string_lossy(), "into": b.join("deeper").to_string_lossy()})).is_err(), "not into itself");
     let r = s.execute("folder.move", &json!({"path": b.to_string_lossy(), "into": root.join("Archive").to_string_lossy()})).unwrap();
     assert_eq!(r["relinked"], 1);
-    assert_eq!(path(&s), root.join("Archive/Italy 2026/one.png").to_string_lossy());
+    assert_eq!(path(&s), root.join("Archive").join("Italy 2026").join("one.png").to_string_lossy());
     let _ = std::fs::remove_dir_all(&root);
 }
 

crates/ui-egui/src/headless.rs

@@ -857,8 +857,12 @@ mod tests {
             std::fs::create_dir_all(base.join(d)).unwrap();
         }
         let s = |p: std::path::PathBuf| p.to_string_lossy().to_string();
-        let (photos, day1, day2, other) =
-            (s(base.join("Photos")), s(base.join("Photos/2026/20260101")), s(base.join("Photos/2026/20260114")), s(base.join("Other")));
+        let (photos, day1, day2, other) = (
+            s(base.join("Photos")),
+            s(base.join("Photos").join("2026").join("20260101")),
+            s(base.join("Photos").join("2026").join("20260114")),
+            s(base.join("Other")),
+        );
         let exec = |h: &mut Headless, c: &str, p: Value| h.request("engine.execute", json!({"command": c, "params": p}), t);
         let rects = |h: &mut Headless| -> std::collections::HashMap<String, f64> {
             let w = h.request("ui.widgets", json!({"filter": "lc-ui-roots-"}), t);