From 47dfdd3c0eed9c20a5bfabf86be84190874eb142 Mon Sep 17 00:00:00 2001
From: Nathaniel Ledford <nate@nateledford.com>
Date: Fri, 9 Oct 2026 18:56:49 -0400
Subject: [PATCH] Engine: work out sidecar naming for many photos in one pass
 (review fix)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Session::sidecar_naming builds the catalog's sidecar owners (a pass over
every photo) for each photo it is asked about, so readying a synchronize
(on the UI thread) with M sidecar updates in an N-photo library cost
M·N — seconds for thousands of updates in a large library. The folder
scan had the same pattern. Session::sidecar_namings works them out once
for a list of photos; both use it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 crates/engine/src/sidecar.rs    | 12 ++++++++++++
 crates/engine/src/sync.rs       |  9 ++++++---
 crates/engine/src/tests_sync.rs | 24 ++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/crates/engine/src/sidecar.rs b/crates/engine/src/sidecar.rs
index f8127f68..830b7208 100644
--- a/crates/engine/src/sidecar.rs
+++ b/crates/engine/src/sidecar.rs
@@ -445,6 +445,18 @@ impl Session {
     /// catalogued file shares the stem (`IMG_0001.CR3` + `IMG_0001.JPG`) and owns the stem
     /// sidecar — a raw first, else the first by file name. The others use Full naming
     /// (`IMG_0001.JPG.xmp`), so their metadata never overwrites each other.
+    /// [`Session::sidecar_naming`] for many photos, the catalog's sidecar owners worked out once
+    /// (per photo that is a pass over the whole catalog).
+    pub fn sidecar_namings(&self, ids: &[PhotoId]) -> Vec<SidecarNaming> {
+        let owners = (self.xmp.naming == SidecarNaming::Stem).then(|| StemOwners::of(&self.catalog));
+        ids.iter()
+            .map(|id| match (self.catalog.photo(*id), &owners) {
+                (Some(p), Some(o)) => o.naming(p, self.xmp.naming),
+                _ => self.xmp.naming,
+            })
+            .collect()
+    }
+
     pub fn sidecar_naming(&self, id: PhotoId) -> SidecarNaming {
         match self.catalog.photo(id) {
             Some(p) if self.xmp.naming == SidecarNaming::Stem => StemOwners::of(&self.catalog).naming(p, self.xmp.naming),
diff --git a/crates/engine/src/sync.rs b/crates/engine/src/sync.rs
index d4f63895..b876aef8 100644
--- a/crates/engine/src/sync.rs
+++ b/crates/engine/src/sync.rs
@@ -174,7 +174,8 @@ impl SyncInput {
         if !disk && (lightcraft_catalog::folders::is_disk_root(path) || crate::cmd::library::covers_other_disks(s, path, &ids)) {
             return Err(invalid(format!("{path} is a whole disk or holds disks: pass `disk: true` to synchronize it")));
         }
-        let photos = ids.iter().filter_map(|id| s.catalog.photo(*id).map(|p| (Arc::clone(p), s.sidecar_naming(*id)))).collect();
+        let namings = s.sidecar_namings(&ids);
+        let photos = ids.iter().zip(namings).filter_map(|(id, naming)| s.catalog.photo(*id).map(|p| (Arc::clone(p), naming))).collect();
         let (scan, _) = ScanInput::new(s, &[path.to_string()]);
         Ok(SyncInput { folder: path.to_string(), scan, photos, labels: s.catalog.clone() })
     }
@@ -509,11 +510,13 @@ impl SyncJob {
         };
         let now = import.as_ref().map_or_else(|| (s.clock)(), |(j, _)| j.now().to_string());
         let sidecars = if choice.read_metadata {
+            let namings = s.sidecar_namings(&metadata.iter().map(|m| PhotoId(m.id)).collect::<Vec<_>>());
             metadata
                 .into_iter()
-                .filter_map(|m| {
+                .zip(namings)
+                .filter_map(|(m, naming)| {
                     let p = s.catalog.photo(PhotoId(m.id))?;
-                    Some((p.id, m.path, s.sidecar_naming(p.id), p.kind == lightcraft_catalog::MediaKind::Raw))
+                    Some((p.id, m.path, naming, p.kind == lightcraft_catalog::MediaKind::Raw))
                 })
                 .collect()
         } else {
diff --git a/crates/engine/src/tests_sync.rs b/crates/engine/src/tests_sync.rs
index 75d40d8c..b5601e71 100644
--- a/crates/engine/src/tests_sync.rs
+++ b/crates/engine/src/tests_sync.rs
@@ -597,3 +597,27 @@ fn an_undo_pressed_while_synchronizing_is_not_folded_into_it() {
     s.execute("edit.undo", &json!({})).unwrap();
     assert_eq!(s.catalog.photo(a).unwrap().rating, 4, "the earlier, unrelated step stays");
 }
+
+#[test]
+fn sidecar_namings_for_many_photos_agree_with_one_at_a_time() {
+    let dir = Scratch::new("namings");
+    let mut s = library(&dir);
+    // a raw beside its JPEG: the raw owns IMG_1.xmp, the JPEG's is IMG_1.JPG.xmp
+    for name in ["IMG_1.CR3", "IMG_1.JPG", "IMG_2.JPG"] {
+        let id = s.catalog.alloc_photo_id();
+        let p = lightcraft_catalog::Photo::new(
+            id,
+            lightcraft_catalog::Source::File { path: dir.path(&format!("trip/{name}")) },
+            name,
+            "X",
+            6,
+            4,
+            "2026-01-01",
+        );
+        s.catalog.apply(lightcraft_catalog::Op::AddPhoto { photo: Box::new(p) }).unwrap();
+    }
+    let ids: Vec<lightcraft_catalog::PhotoId> = s.catalog.photos().map(|p| p.id).collect();
+    let one_by_one: Vec<_> = ids.iter().map(|id| s.sidecar_naming(*id)).collect();
+    assert_eq!(s.sidecar_namings(&ids), one_by_one);
+    assert!(one_by_one.contains(&crate::sidecar::SidecarNaming::Full), "the case that matters is there");
+}