Keep the mask when the photograph is closed

A local adjustment survived until the session ended and then did not exist.
Nothing reported it, because nothing had failed.

The sidecar format has carried masks since they were added, `Version::apply`
restores them into the graph, and `Version::update` captures them — that work
landed complete and was never called. The autosave writes `copy_settings()`,
which is a `Preset`: a map from (operation, parameter) to a number. A mask is
not a parameter. It is a rule about *where*, with a chain of its own, so it
fell outside the only thing being written, and the read path had nothing to
read.

The save now carries the stack beside the preset, on both the local and the
remote path. Deliberately not merged but replaced wholesale: this is the stack
as it stands, so a layer the user deleted has to leave the file too. Merging
two devices' stacks is `Sidecar::merge`'s job and belongs to sync (FR-NC-9).

A paste still carries no masks, and the `Option` is how that is said. Settings
travel between photographs; a mask does not, because it is drawn against one
frame and describes nothing on another — and `Scope` cannot express that,
since it filters parameters and a mask is not one.

The test fails against the old save path with "the mask must come back with
the photograph", which is the whole of the defect: not a crash, not an error,
just an edit that was not there in the morning.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

ui/dr-ui/src/develop.rs

@@ -2233,6 +2233,17 @@ impl DevelopSession {
     /// Captured at full scope — framing included — because the decision about
     /// what travels is made when the preset is *applied*. Copying, then
     /// changing one's mind about the crop, must not mean copying again.
+    /// TRACES: FR-DEV-3 | FR-CAT-8
+    /// The local adjustment stack, for writing this image's edit back.
+    ///
+    /// Beside `copy_settings` rather than part of it: that returns a `Preset`,
+    /// which travels *between* photographs, and a mask must not — it is drawn
+    /// against one frame and describes nothing on another. The save path takes
+    /// both; the paste path takes only the preset.
+    pub fn masks(&self) -> &dr_pipeline::mask::MaskStack {
+        self.graph.masks()
+    }
+
     pub fn copy_settings(&self) -> Preset {
         Preset::capture(&self.graph)
     }

ui/dr-ui/src/library.rs

@@ -375,6 +375,23 @@ pub enum Amendment {
     Settings {
         preset: dr_pipeline::Preset,
         scope: dr_pipeline::Scope,
+        /// TRACES: FR-DEV-3 | FR-CAT-8
+        /// The local adjustments, when this is an image's own edit being
+        /// written back rather than a paste onto someone else's.
+        ///
+        /// A `Preset` is a parameter map, and a mask is not a parameter — it
+        /// is a rule about *where*, with a chain of its own. So a save that
+        /// carried only the preset wrote the sliders and silently dropped
+        /// every local adjustment: the sidecar format has stored masks since
+        /// they were added and `Version::apply` restores them, but nothing
+        /// ever put any there. The mask survived until the session ended and
+        /// then did not exist.
+        ///
+        /// `None` for a paste, which must not carry the source image's masks
+        /// onto the target: a mask is drawn against one photograph and means
+        /// nothing on another, and `Scope` cannot express that because it
+        /// filters parameters.
+        masks: Option<dr_pipeline::mask::MaskStack>,
     },
 }
 
@@ -591,8 +608,20 @@ fn amend(base: dr_pipeline::Sidecar, w: &SidecarWrite) -> dr_pipeline::Sidecar {
             version.rating = *rating;
             version.flag = *flag;
         }
-        Amendment::Settings { preset, scope } => {
+        Amendment::Settings {
+            preset,
+            scope,
+            masks,
+        } => {
             preset.amend(&mut version.params, *scope);
+            // Replaced wholesale rather than merged: this is the whole of the
+            // image's local adjustment stack as it stands, so a layer the user
+            // deleted has to leave the sidecar too. Cross-device merging of
+            // two stacks is `Sidecar::merge`'s job and happens on sync, not
+            // here (FR-NC-9).
+            if let Some(masks) = masks {
+                version.masks = masks.clone();
+            }
         }
     }
 

ui/dr-ui/src/library_ui.rs

@@ -2475,6 +2475,10 @@ fn collect_settings_writes(
             amendment: library::Amendment::Settings {
                 preset: preset.clone(),
                 scope,
+                // A paste carries no masks, and must not: a mask is drawn
+                // against one photograph and describes nothing on another.
+                // The target keeps whatever local adjustments it already had.
+                masks: None,
             },
         })
     });

ui/dr-ui/src/presets.rs

@@ -177,7 +177,16 @@ pub fn load_local(image: &Path) -> Option<Sidecar> {
 /// Read first for the same reason the remote writer does: the file may already
 /// hold a rating, or an operation this build does not know about, and writing
 /// a fresh document containing only the current edit would delete both.
-pub fn save_local(image: &Path, preset: &Preset, scope: Scope) -> Result<(), String> {
+/// `masks` carries the local adjustments when this is the image's own edit,
+/// and is `None` for a paste, which must leave the target's masks alone — a
+/// mask is drawn against one photograph and describes nothing on another. See
+/// `library::Amendment::Settings` for the same distinction on the remote path.
+pub fn save_local(
+    image: &Path,
+    preset: &Preset,
+    scope: Scope,
+    masks: Option<&dr_pipeline::mask::MaskStack>,
+) -> Result<(), String> {
     let path = local_sidecar_path(image);
 
     // Distinguish "no sidecar yet" from "a sidecar this build cannot read".
@@ -217,6 +226,11 @@ pub fn save_local(image: &Path, preset: &Preset, scope: Scope) -> Result<(), Str
             });
 
     preset.amend(&mut version.params, scope);
+    // Wholesale, not merged: this is the stack as it stands, so a layer the
+    // user deleted must leave the file too.
+    if let Some(masks) = masks {
+        version.masks = masks.clone();
+    }
     version.revision = version.revision.saturating_add(1);
     version.modified = now_secs();
     sidecar.put(version);
@@ -252,7 +266,13 @@ pub fn save_open_edit(
     session: &Rc<RefCell<Option<DevelopSession>>>,
     library: &Rc<library_ui::LibraryController>,
 ) {
-    let Some(preset) = session.borrow().as_ref().map(|s| s.copy_settings()) else {
+    // Both taken in one borrow: they are one edit, and a mask stack captured
+    // from a session that had already moved on would be a different image's.
+    let Some((preset, masks)) = session
+        .borrow()
+        .as_ref()
+        .map(|s| (s.copy_settings(), s.masks().clone()))
+    else {
         return;
     };
 
@@ -265,7 +285,7 @@ pub fn save_open_edit(
             // `Everything`: this is the image's *own* edit being written back,
             // not a paste onto someone else's. Excluding framing here would
             // make a crop the one adjustment that never survived a restart.
-            if let Err(e) = save_local(path, &preset, Scope::Everything) {
+            if let Err(e) = save_local(path, &preset, Scope::Everything, Some(&masks)) {
                 log::warn!("saving {}: {e}", path.display());
             }
         }
@@ -277,6 +297,11 @@ pub fn save_open_edit(
                 amendment: library::Amendment::Settings {
                     preset,
                     scope: Scope::Everything,
+                    // The image's own edit, so the whole of it: `Everything`
+                    // already says framing travels, and the local adjustments
+                    // have to travel for the same reason. Without this the
+                    // sliders were written and every mask was dropped.
+                    masks: Some(masks),
                 },
             };
             library_ui::start_sidecar_writes(window, library, vec![write]);
@@ -521,6 +546,66 @@ mod tests {
         g
     }
 
+    /// The bug this exists to prevent: a local adjustment that survived until
+    /// the session ended and then did not exist.
+    ///
+    /// The sidecar format has stored masks since they were added, and
+    /// `Version::apply` restores them — but the save path wrote a `Preset`,
+    /// which is a parameter map, and a mask is not a parameter. So the sliders
+    /// came back and every mask was silently gone. Nothing reported it,
+    /// because nothing had failed.
+    #[test]
+    fn a_mask_survives_being_written_and_read_back() {
+        use dr_pipeline::mask::{MaskLayer, MaskSource};
+
+        let dir = tempdir("mask-roundtrip");
+        let image = dir.join("a.CR2");
+
+        let mut graph = edited();
+        let mut layer = MaskLayer::new(
+            "m1",
+            MaskSource::Radial {
+                centre: (0.4, 0.6),
+                radii: (0.2, 0.3),
+                angle: 0.0,
+                feather: 0.5,
+            },
+        );
+        layer.opacity = 0.75;
+        graph.masks_mut().push(layer);
+        assert_eq!(graph.masks().len(), 1, "the premise: the edit has a mask");
+
+        save_local(
+            &image,
+            &Preset::capture(&graph),
+            Scope::Everything,
+            Some(graph.masks()),
+        )
+        .unwrap();
+
+        // Read it back the way opening the photograph again does.
+        let text = std::fs::read_to_string(local_sidecar_path(&image)).unwrap();
+        let sidecar = Sidecar::parse(&text).unwrap();
+        let version = sidecar.default_version().expect("a default version");
+
+        let mut reopened = EditGraph::default_chain();
+        version.apply(&mut reopened);
+
+        assert_eq!(
+            reopened.masks().len(),
+            1,
+            "the mask must come back with the photograph"
+        );
+        let back = &reopened.masks().layers()[0];
+        assert_eq!(back.id, "m1");
+        assert!((back.opacity - 0.75).abs() < 1e-6, "and with its settings");
+        assert!(
+            matches!(back.source, MaskSource::Radial { .. }),
+            "and its kind: {:?}",
+            back.source.kind()
+        );
+    }
+
     #[test]
     fn a_sidecar_sits_beside_its_image_with_the_extension_replaced() {
         // Replaced, not appended, so a RAW and the JPEG beside it share one
@@ -537,7 +622,7 @@ mod tests {
         let dir = tempdir("round-trip");
         let image = dir.join("a.CR2");
 
-        save_local(&image, &Preset::capture(&edited()), Scope::Everything).unwrap();
+        save_local(&image, &Preset::capture(&edited()), Scope::Everything, None).unwrap();
 
         let sidecar = load_local(&image).expect("a sidecar was written");
         let mut restored = EditGraph::default_chain();
@@ -563,8 +648,8 @@ mod tests {
         let dir = tempdir("one-version");
         let image = dir.join("a.CR2");
 
-        save_local(&image, &Preset::capture(&edited()), Scope::Everything).unwrap();
-        save_local(&image, &Preset::capture(&edited()), Scope::Everything).unwrap();
+        save_local(&image, &Preset::capture(&edited()), Scope::Everything, None).unwrap();
+        save_local(&image, &Preset::capture(&edited()), Scope::Everything, None).unwrap();
 
         assert_eq!(load_local(&image).expect("a sidecar").versions.len(), 1);
     }
@@ -576,13 +661,13 @@ mod tests {
         let dir = tempdir("revision");
         let image = dir.join("a.CR2");
 
-        save_local(&image, &Preset::capture(&edited()), Scope::Everything).unwrap();
+        save_local(&image, &Preset::capture(&edited()), Scope::Everything, None).unwrap();
         let first = load_local(&image)
             .unwrap()
             .default_version()
             .unwrap()
             .revision;
-        save_local(&image, &Preset::capture(&edited()), Scope::Everything).unwrap();
+        save_local(&image, &Preset::capture(&edited()), Scope::Everything, None).unwrap();
         let second = load_local(&image)
             .unwrap()
             .default_version()
@@ -610,7 +695,7 @@ mod tests {
         });
         std::fs::write(local_sidecar_path(&image), sidecar.to_text()).unwrap();
 
-        save_local(&image, &Preset::capture(&edited()), Scope::Everything).unwrap();
+        save_local(&image, &Preset::capture(&edited()), Scope::Everything, None).unwrap();
 
         let back = load_local(&image).unwrap();
         let v = back.default_version().unwrap();
@@ -627,7 +712,7 @@ mod tests {
         let image = dir.join("a.CR2");
         std::fs::write(local_sidecar_path(&image), "drsc 99\n").unwrap();
 
-        assert!(save_local(&image, &Preset::capture(&edited()), Scope::Everything).is_err());
+        assert!(save_local(&image, &Preset::capture(&edited()), Scope::Everything, None).is_err());
         assert_eq!(
             std::fs::read_to_string(local_sidecar_path(&image)).unwrap(),
             "drsc 99\n",
@@ -642,6 +727,7 @@ mod tests {
             &dir.join("a.CR2"),
             &Preset::capture(&edited()),
             Scope::Everything,
+            None,
         )
         .unwrap();