From 7b2d7b25ea569ab477e0dec9a6e1e9aa013286f8 Mon Sep 17 00:00:00 2001
From: Matthew James Sivak <me@mattsivak.cz>
Date: Sun, 18 Oct 2026 18:16:20 +0200
Subject: [PATCH] LR-BEHAV-BG-TASKS: an older Reload can't overwrite a newer
 one; background Reload auto-writes sidecars

Review of #1998. Each reload plan takes a ticket; apply skips a photo a
later-started reload has already been applied to, so an older read that
finishes last no longer restores the previous dimensions and content hash.
The app's background completion now commits through Session::execute_fn,
so the XMP sidecar auto-write (and the save) happen as for the command.
Regression tests: an_older_reload_does_not_replace_a_newer_one,
a_background_reload_writes_the_sidecar_with_auto_write_on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 crates/engine/src/cmd/reload.rs     | 33 +++++++++---
 crates/engine/src/lib.rs            |  3 ++
 crates/ui-egui/src/reload.rs        | 10 ++--
 crates/ui-egui/src/tests_recheck.rs | 80 ++++++++++++++++++++++++++++-
 docs/background-tasks.md            |  2 +-
 docs/parity-checklist.md            |  4 +-
 6 files changed, 118 insertions(+), 14 deletions(-)

diff --git a/crates/engine/src/cmd/reload.rs b/crates/engine/src/cmd/reload.rs
index 9fc0a590f..0c1cb964c 100644
--- a/crates/engine/src/cmd/reload.rs
+++ b/crates/engine/src/cmd/reload.rs
@@ -3,6 +3,10 @@
 //! the command ran. [`plan`] lists the photos to re-read (no file-system calls), [`probe`] reads
 //! their files without the session (cancellable, with progress), and [`apply`] commits what came
 //! of it as one undo step. The command itself (agents, CLI, MCP) does all three at once.
+//!
+//! Reloads can overlap (the user picks Reload from Disk again while a first one still reads), so
+//! each [`plan`] takes a ticket and [`apply`] drops a photo's result when a reload planned later
+//! has already been applied to it: an older read never replaces a newer one.
 
 use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 
@@ -18,14 +22,28 @@ use crate::{Result, Session};
 pub struct Item {
     pub id: PhotoId,
     pub path: String,
+    /// The [`plan`] it came from, in the order reloads were started (see [`Tickets`]).
+    pub ticket: u64,
 }
 
-/// The photos `p` targets (`ids`, else the selection) that have a file. No file-system calls.
-pub fn plan(s: &Session, p: &Value) -> Result<Vec<Item>> {
-    Ok(s.targets(p)?
+/// Which reload each photo last took its file information from, so a reload started earlier but
+/// finishing later doesn't put back what its file was before (session memory, not saved).
+#[derive(Debug, Default)]
+pub struct Tickets {
+    last: u64,
+    applied: std::collections::BTreeMap<PhotoId, u64>,
+}
+
+/// The photos `p` targets (`ids`, else the selection) that have a file, under a new ticket. No
+/// file-system calls.
+pub fn plan(s: &mut Session, p: &Value) -> Result<Vec<Item>> {
+    let ids = s.targets(p)?;
+    s.reload_tickets.last = s.reload_tickets.last.saturating_add(1);
+    let ticket = s.reload_tickets.last;
+    Ok(ids
         .into_iter()
         .filter_map(|id| match s.catalog.photo(id).map(|ph| &ph.source) {
-            Some(Source::File { path }) => Some(Item { id, path: path.clone() }),
+            Some(Source::File { path }) => Some(Item { id, path: path.clone(), ticket }),
             _ => None,
         })
         .collect())
@@ -66,16 +84,19 @@ pub fn probe(
 
 /// Commit [`probe`]'s results as one undo step ("Reload Changed Files"): each photo and its virtual
 /// copies get what their file is now. A file that couldn't be read changes nothing; a photo removed
-/// or relinked since it was listed is skipped. Returns the photos that changed.
+/// or relinked since it was listed, or already given a newer reload's result, is skipped. Returns
+/// the photos that changed.
 pub fn apply(s: &mut Session, items: Vec<Item>, results: Vec<std::result::Result<ProbeInfo, String>>) -> Result<Vec<PhotoId>> {
     let mut ops = Vec::new();
     let mut reloaded = Vec::new();
     for (item, info) in items.into_iter().zip(results) {
         let Ok(info) = info else { continue };
         let current = s.catalog.photo(item.id).is_some_and(|ph| matches!(&ph.source, Source::File { path } if *path == item.path));
-        if !current {
+        let superseded = s.reload_tickets.applied.get(&item.id).is_some_and(|t| *t > item.ticket);
+        if !current || superseded {
             continue;
         }
+        s.reload_tickets.applied.insert(item.id, item.ticket);
         crate::cmd::convert::reload_ops(&s.catalog, item.id, &info, &mut ops, &mut reloaded);
     }
     for id in &reloaded {
diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs
index 25e6ef52f..ab6c048f9 100644
--- a/crates/engine/src/lib.rs
+++ b/crates/engine/src/lib.rs
@@ -240,6 +240,8 @@ pub struct Session {
     /// Set by a command whose change must not rewrite the photo's XMP sidecar even with auto-write on
     /// (a catalog-only edit of data the sidecar writer does not emit); consumed when the command ends.
     pub(crate) skip_auto_write: bool,
+    /// The order of Reload from Disk runs, so an older one finishing late is dropped ([`cmd::reload`]).
+    pub(crate) reload_tickets: cmd::reload::Tickets,
     /// Photos whose automatic XMP sidecar write failed (their folder offline, read-only or full), written again by
     /// [`Session::retry_unsaved_sidecars`] once their file is reachable.
     pub(crate) unsaved_sidecars: std::collections::BTreeSet<PhotoId>,
@@ -434,6 +436,7 @@ impl Session {
             pending_sidecar_repairs: Vec::new(),
             interaction: None,
             skip_auto_write: false,
+            reload_tickets: Default::default(),
             unsaved_sidecars: Default::default(),
             denoise: Default::default(),
             face_models_dir: None,
diff --git a/crates/ui-egui/src/reload.rs b/crates/ui-egui/src/reload.rs
index 32823cf16..94c9510ab 100644
--- a/crates/ui-egui/src/reload.rs
+++ b/crates/ui-egui/src/reload.rs
@@ -1,7 +1,8 @@
 //! Reload from Disk in the app: the files are read on a worker thread (a raw's decode takes a
 //! second or more, and a selection of them froze the window), with a row in the activity stack
-//! that counts the files and can stop. What came of it is applied back here, as one undo step,
-//! unless another library was opened meanwhile. Agents get the same with `wait: true`, which runs
+//! that counts the files and can stop. What came of it is applied back here through the session's
+//! command wrapper (one undo step, saved, sidecars auto-written), unless another library was opened
+//! meanwhile or a reload started later has already been applied to the photo. Agents get the same with `wait: true`, which runs
 //! the engine's command in place.
 
 use std::sync::Arc;
@@ -22,7 +23,7 @@ pub fn command(app: &mut LightcraftApp, p: &Value) -> Option<Result<Value, Strin
     if p.get("wait").and_then(Value::as_bool) == Some(true) {
         return None;
     }
-    let items = match reload::plan(&app.session, p) {
+    let items = match reload::plan(&mut app.session, p) {
         Ok(items) => items,
         Err(e) => return Some(Err(format!("photo.reload: {e}"))),
     };
@@ -47,7 +48,8 @@ pub fn command(app: &mut LightcraftApp, p: &Value) -> Option<Result<Value, Strin
         if app.session.library_generation() != library {
             return;
         }
-        if let Err(e) = reload::apply(&mut app.session, items, results) {
+        // as the command: one undo step, the durable save and the automatic XMP sidecar write
+        if let Err(e) = app.session.execute_fn("photo.reload", |s| reload::apply(s, items, results).map(|ids| reload::to_json(&ids))) {
             return app.toast_error(ctx, e.to_string());
         }
         let cleared = preview_only.iter().filter(|id| app.session.catalog.photo(**id).is_some_and(|ph| ph.preview_only.is_none())).count();
diff --git a/crates/ui-egui/src/tests_recheck.rs b/crates/ui-egui/src/tests_recheck.rs
index c343050fc..88286027b 100644
--- a/crates/ui-egui/src/tests_recheck.rs
+++ b/crates/ui-egui/src/tests_recheck.rs
@@ -132,7 +132,7 @@ fn reload_of_a_selection_runs_in_the_background() {
             preview_checked: "0.5.0".into(),
         })
         .unwrap();
-    let items = lightcraft_engine::cmd::reload::plan(&app.session, &json!({"ids": [id.0]})).unwrap();
+    let items = lightcraft_engine::cmd::reload::plan(&mut app.session, &json!({"ids": [id.0]})).unwrap();
     let probe = lightcraft_engine::cmd::reload::file_probe(&app.session);
     let cancelled = std::sync::atomic::AtomicBool::new(true);
     let results = lightcraft_engine::cmd::reload::probe(probe.as_ref(), &items, &cancelled, &|_| {});
@@ -150,3 +150,81 @@ fn reload_with_nothing_selected_starts_nothing() {
     assert!(app.tasks.is_empty());
     assert!(app.session.activity.list().iter().all(|r| r.kind != "reload"));
 }
+
+/// Two Reloads of the same photo overlap: the first is held after reading the original file, the
+/// file is replaced and a second Reload finishes. The first one finishing afterwards must not put
+/// back the old dimensions and content hash.
+#[test]
+fn an_older_reload_does_not_replace_a_newer_one() {
+    use std::sync::atomic::{AtomicUsize, Ordering};
+    use std::sync::{Arc, Condvar, Mutex};
+    let dir = Dir(std::env::temp_dir().join(format!("lc-ui-reload-order-{}", std::process::id())));
+    let (mut app, id) = app_with_stale_photo(&dir.0);
+    let ctx = egui::Context::default();
+    let native = app.session.media.file_probe.clone().unwrap();
+    let calls = Arc::new(AtomicUsize::new(0));
+    // (the first probe has read its file, the first probe may return)
+    let gate = Arc::new((Mutex::new((false, false)), Condvar::new()));
+    let (worker_gate, worker_calls) = (gate.clone(), calls.clone());
+    app.session.media.file_probe = Some(Arc::new(move |path| {
+        let info = native(path);
+        if worker_calls.fetch_add(1, Ordering::SeqCst) == 0 {
+            let mut state = worker_gate.0.lock().unwrap();
+            state.0 = true;
+            worker_gate.1.notify_all();
+            let _ = worker_gate.1.wait_timeout_while(state, Duration::from_secs(10), |s| !s.1).unwrap();
+        }
+        info
+    }));
+    app.run("photo.reload", json!({"ids": [id.0]})).unwrap();
+    let (state, _) = gate.1.wait_timeout_while(gate.0.lock().unwrap(), Duration::from_secs(5), |s| !s.0).unwrap();
+    assert!(state.0, "the first reload read the original file");
+    drop(state);
+
+    // an external editor saves new pixels and the user reloads again
+    let image = lightcraft_raster::Rgba8::from_fn(40, 20, |x, y| [(x * 3) as u8, (y * 5) as u8, 90, 255]);
+    let o = lightcraft_engine::export::ExportOptions { format: lightcraft_engine::export::ExportFormat::Png, ..Default::default() };
+    std::fs::write(dir.0.join("files/one.png"), lightcraft_engine::export::encode_image(&image, &o).unwrap()).unwrap();
+    app.run("photo.reload", json!({"ids": [id.0]})).unwrap();
+    let deadline = std::time::Instant::now() + Duration::from_secs(5);
+    while app.session.catalog.photo(id).is_some_and(|p| (p.width, p.height) != (40, 20)) {
+        crate::tasks::poll(&mut app, &ctx);
+        assert!(std::time::Instant::now() < deadline, "the second reload finishes while the first is held");
+        std::thread::sleep(Duration::from_millis(5));
+    }
+    let newer_hash = app.session.catalog.photo(id).unwrap().content_hash.clone();
+
+    // the first reload finishes last
+    gate.0.lock().unwrap().1 = true;
+    gate.1.notify_all();
+    assert!(crate::tasks::wait(&mut app, &ctx, Duration::from_secs(10)));
+    let after = app.session.catalog.photo(id).unwrap();
+    assert_eq!((after.width, after.height), (40, 20), "the older reload put back the old dimensions");
+    assert_eq!(after.content_hash, newer_hash);
+}
+
+/// A background Reload commits like the command: with automatic XMP writing on, metadata the file
+/// newly gives (here GPS) reaches the sidecar too.
+#[test]
+fn a_background_reload_writes_the_sidecar_with_auto_write_on() {
+    use std::sync::Arc;
+    let dir = Dir(std::env::temp_dir().join(format!("lc-ui-reload-xmp-{}", std::process::id())));
+    let (mut app, id) = app_with_stale_photo(&dir.0);
+    let ctx = egui::Context::default();
+    app.run("library.xmpPreferences", json!({"autoWrite": true})).unwrap();
+    app.run("photo.rate", json!({"ids": [id.0], "rating": 1})).unwrap();
+    let sidecar = dir.0.join("files/one.xmp");
+    assert!(!std::fs::read_to_string(&sidecar).unwrap().contains("GPSLatitude"));
+    let native = app.session.media.file_probe.clone().unwrap();
+    app.session.media.file_probe = Some(Arc::new(move |path| {
+        let mut info = native(path)?;
+        // GPS the catalog doesn't have yet (as for a photo catalogued by an older version)
+        info.meta.gps = Some((41.0, 2.0));
+        Ok(info)
+    }));
+    let r = app.run("photo.reload", json!({"ids": [id.0]})).unwrap();
+    assert_eq!(r["background"], true);
+    assert!(crate::tasks::wait(&mut app, &ctx, Duration::from_secs(10)));
+    assert_eq!(app.session.catalog.photo(id).unwrap().meta.gps, Some((41.0, 2.0)));
+    assert!(std::fs::read_to_string(&sidecar).unwrap().contains("GPSLatitude"), "the sidecar was not written");
+}
diff --git a/docs/background-tasks.md b/docs/background-tasks.md
index 58fd3d6c5..86deac24d 100644
--- a/docs/background-tasks.md
+++ b/docs/background-tasks.md
@@ -60,7 +60,7 @@ guard.detail(&file_name);
 | Lightroom catalog import | `crates/ui-egui/src/lightroom_import.rs` | stop being cancellable for a phase that must finish |
 | Face scan, AI Denoise | `crates/engine/src/cmd/face_recognize.rs`, `crates/engine/src/denoise.rs` | a continuous job with no ✕, kept in session state |
 | Find Missing Photos | `crates/ui-egui/src/tasks.rs` | a generic background task with a row (`kind: Some(..)`) |
-| Reload from Disk | `crates/ui-egui/src/reload.rs`, `crates/engine/src/cmd/reload.rs` | a command split into plan / probe / apply so the app reads the files on a worker thread (`tasks::spawn_tracked`: progress and `Cancel::Flag`); `wait: true` runs it in place |
+| Reload from Disk | `crates/ui-egui/src/reload.rs`, `crates/engine/src/cmd/reload.rs` | a command split into plan / probe / apply so the app reads the files on a worker thread (`tasks::spawn_tracked`: progress and `Cancel::Flag`); the result is committed through `Session::execute_fn` (undo step, save, XMP auto-write), and dropped for a photo a later-started reload has already updated; `wait: true` runs it in place |
 | Reading focus points | `crates/engine/src/focus_scan.rs` | a worker paced by the UI's per-frame pump (`focus.pump {pace}`); `Cancel::Flag`; commits in batches without undo steps |
 
 ## Testing
diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md
index 43756c276..eb240c7ed 100644
--- a/docs/parity-checklist.md
+++ b/docs/parity-checklist.md
@@ -1,6 +1,6 @@
 # Lightroom parity checklist
 
-> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-15 · **Change:** minor (LRC-PRINT-PAGE: Print Page Options) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6
+> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-18 · **Change:** minor (LR-BEHAV-BG-TASKS: Reload from Disk in the background) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6
 
 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we
 have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good
@@ -797,7 +797,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove
 
 | Date | Change | Summary |
 |---|---|---|
-| 2026-10-15 | minor | LR-BEHAV-BG-TASKS: Reload from Disk reads its files on a worker thread with a cancellable activity row instead of freezing the window on a selection of raws; status unchanged (✅) |
+| 2026-10-15 | minor | LR-BEHAV-BG-TASKS: Reload from Disk reads its files on a worker thread with a cancellable activity row instead of freezing the window on a selection of raws; its result is committed through the command wrapper (XMP sidecars auto-written as before) and an older reload finishing late no longer overwrites a newer one; status unchanged (✅) |
 | 2026-10-14 | patch | LR-EXP-TYPE: opaque 8-bit PNG/TIFF/WebP use RGB; transparent exports retain alpha (#1670). Statuses and feature counts unchanged. |
 | 2026-10-15 | minor | LRC-PRINT-PAGE: Page Options (Page Numbers, Page Info, Crop Marks) on Single Image and Contact Sheet pages, in the Print dialog, `print.set` and templates; still 🟡 (no identity plate, watermark, photo info or Cut Guides) |
 | 2026-10-15 | minor | LR-IMP-FORMATS: Hasselblad 3FR and FFF show their uncompressed RGB preview (no JPEG in these files) in the grid and loupe until developed, and as the stand-in when a file fails to decode; status unchanged (🟡) |