LR-EXP-BITDEPTH: judge a preset's bitDepth against the format the export writes

A preset without `format` lost its `bitDepth` before the call's `path` was merged in,
because it was judged against JPEG: `app.export {preset, path: a.png}` with a 16-bit
preset wrote an 8-bit PNG. Session::export_params now merges first and drops a preset's
depth only if the format written (with the caller's fallback) lacks it; a depth the call
passes is still validated. export.savePreset judges the depth at once only when the preset
has a `format`.

crates/engine/src/cmd/export.rs

@@ -17,6 +17,12 @@ impl Session {
     /// `app.export` params with a named `preset` expanded: the preset's params, overridden by the
     /// call's own (`preset` itself removed). Unknown preset names are an error.
     pub fn export_params(&self, p: &Value) -> std::result::Result<Value, String> {
+        self.export_params_or(p, crate::export::ExportFormat::default())
+    }
+
+    /// [`Session::export_params`] for a caller that writes `fallback` to a `path` naming no format
+    /// (see [`crate::export::ExportOptions::export_format`]).
+    pub fn export_params_or(&self, p: &Value, fallback: crate::export::ExportFormat) -> std::result::Result<Value, String> {
         let Some(name) = p.get("preset").and_then(Value::as_str) else { return Ok(p.clone()) };
         let (preset, _) = self
             .all_export_presets()
@@ -24,12 +30,17 @@ impl Session {
             .find(|(x, _)| x.name.eq_ignore_ascii_case(name.trim()))
             .ok_or_else(|| format!("unknown export preset `{name}` (see export.presets)"))?;
         // a preset saved by another version may hold keys this one doesn't know: not the caller's typo
-        let mut out = crate::export::ExportOptions::known_keys_only(&preset.params);
+        let mut out = crate::export::ExportOptions::known_keys(&preset.params);
         if let (Some(o), Some(own)) = (out.as_object_mut(), p.as_object()) {
             for (k, v) in own.iter().filter(|(k, _)| *k != "preset") {
                 o.insert(k.clone(), v.clone());
             }
         }
+        // the preset's depth is judged against the format written, known only now (`path`); a
+        // depth the call passes itself is the caller's, and validated as such
+        if !p.get("bitDepth").is_some_and(|d| !d.is_null()) {
+            crate::export::ExportOptions::drop_unfit_depth(&mut out, fallback);
+        }
         Ok(out)
     }
 }
@@ -55,8 +66,10 @@ fn save(s: &mut Session, p: &Value) -> Result<Value> {
     if let Some(o) = params.as_object_mut() {
         o.retain(|k, _| !NOT_SETTINGS.contains(&k.as_str()));
     }
-    // a preset that app.export would refuse is refused here, where the typo is on screen
-    crate::export::ExportOptions::validate(ID, &params)?;
+    // a preset that app.export would refuse is refused here, where the typo is on screen; without
+    // `format`, its `bitDepth` is judged when it exports, against the format written then
+    let format = params.get("format").and_then(Value::as_str).map(|_| crate::export::ExportOptions::export_format(&params, Default::default()));
+    crate::export::ExportOptions::validate_as(ID, &params, format)?;
     let preset = ExportPreset { name: name.to_string(), params };
     match s.export_presets.iter_mut().find(|x| x.name.eq_ignore_ascii_case(name)) {
         Some(x) => *x = preset,

crates/engine/src/export.rs

@@ -819,15 +819,17 @@ impl ExportOptions {
     /// for good after an update. A `bitDepth` the stored format doesn't have (written at the
     /// format's own depth before issue #1952) is dropped too.
     pub fn known_keys_only(p: &serde_json::Value) -> serde_json::Value {
+        let mut p = Self::known_keys(p);
+        Self::drop_unfit_depth(&mut p, ExportFormat::default());
+        p
+    }
+
+    /// [`ExportOptions::known_keys_only`] keeping `bitDepth`, for a preset whose format may only be
+    /// known once the call's own params (`path`) are merged in.
+    pub(crate) fn known_keys(p: &serde_json::Value) -> serde_json::Value {
         let mut p = p.clone();
         if let Some(o) = p.as_object_mut() {
             o.retain(|k, _| OPTION_PARAMS.contains(&k.as_str()) || TARGET_PARAMS.contains(&k.as_str()));
-            let format = o.get("format").and_then(serde_json::Value::as_str).map_or(Some(ExportFormat::default()), ExportFormat::parse);
-            if let (Some(format), Some(depth)) = (format, o.get("bitDepth").and_then(whole))
-                && !Self::bit_depths(format).iter().any(|d| i64::from(d.0) == depth)
-            {
-                o.remove("bitDepth");
-            }
             if let Some(w) = o.get_mut("watermark").and_then(serde_json::Value::as_object_mut) {
                 w.retain(|k, _| WATERMARK_PARAMS.contains(&k.as_str()));
             }
@@ -835,6 +837,18 @@ impl ExportOptions {
         p
     }
 
+    /// Drop a stored `bitDepth` the format `p` exports ([`ExportOptions::export_format`] with
+    /// `fallback`) doesn't have: it was written at the format's own depth before issue #1952.
+    pub(crate) fn drop_unfit_depth(p: &mut serde_json::Value, fallback: ExportFormat) {
+        let format = Self::export_format(p, fallback);
+        if let Some(o) = p.as_object_mut()
+            && let Some(depth) = o.get("bitDepth").and_then(whole)
+            && !Self::bit_depths(format).iter().any(|d| i64::from(d.0) == depth)
+        {
+            o.remove("bitDepth");
+        }
+    }
+
     /// Strict [`ExportOptions::from_json`] for a command that is about to export (issue #181): an
     /// unknown parameter or a value that cannot be read as what it is for is an error, never a
     /// silent default. `app.export` targets (`id`, `ids`, `path`, `dir`, `preset`, `background`)
@@ -847,7 +861,7 @@ impl ExportOptions {
     /// format; the format returned is [`ExportOptions::export_format`], the one written.
     pub fn from_params_or(p: &serde_json::Value, fallback: ExportFormat) -> crate::Result<Self> {
         let format = Self::export_format(p, fallback);
-        Self::validate_as("app.export", p, format)?;
+        Self::validate_as("app.export", p, Some(format))?;
         Ok(Self { format, ..Self::from_json(p) })
     }
 
@@ -868,11 +882,12 @@ impl ExportOptions {
     /// kind of thing its key takes, and a `watermark` object must hold only [`WATERMARK_PARAMS`]
     /// with its sizes in range (issue #183). `null` means "not given", as a dropped key does.
     pub fn validate(cmd: &str, p: &serde_json::Value) -> crate::Result<()> {
-        Self::validate_as(cmd, p, Self::export_format(p, ExportFormat::default()))
+        Self::validate_as(cmd, p, Some(Self::export_format(p, ExportFormat::default())))
     }
 
-    /// [`ExportOptions::validate`] with `bitDepth` checked against `format`, the one exported.
-    fn validate_as(cmd: &str, p: &serde_json::Value, format: ExportFormat) -> crate::Result<()> {
+    /// [`ExportOptions::validate`] with `bitDepth` checked against `format`, the one exported
+    /// (`None`: not known yet, as for a preset without `format`).
+    pub(crate) fn validate_as(cmd: &str, p: &serde_json::Value, format: Option<ExportFormat>) -> crate::Result<()> {
         use serde_json::Value;
         let bad = |msg: String| crate::cmd::bad(cmd, msg);
         let Some(obj) = p.as_object() else { return Err(bad("params must be an object".into())) };
@@ -981,7 +996,7 @@ impl ExportOptions {
             }
         }
         // issue #1952: a depth the format doesn't have would be written at the format's own depth
-        if let Some(depth) = obj.get("bitDepth").and_then(whole) {
+        if let (Some(format), Some(depth)) = (format, obj.get("bitDepth").and_then(whole)) {
             let mut depths: Vec<u8> = Self::bit_depths(format).iter().map(|d| d.0).collect();
             if !depths.iter().any(|d| i64::from(*d) == depth) {
                 depths.sort_unstable();

crates/engine/src/tests_export.rs

@@ -98,6 +98,28 @@ fn dng_export_rejects_non_raw_photos() {
     let _ = std::fs::remove_dir_all(&dir);
 }
 
+/// Review of #1966: a preset without `format` keeps its `bitDepth` until the format written is
+/// known. With `path: a.png` it writes the 16-bit PNG it asks for; a format without that depth
+/// gets its own, as stored settings do, while a depth the call itself passes is still refused.
+#[test]
+fn a_preset_depth_is_judged_against_the_format_written() {
+    use crate::export::ExportPreset;
+    let mut s = Session::with_demo();
+    let id = s.active().unwrap();
+    s.export_presets.push(ExportPreset { name: "Deep".into(), params: json!({"bitDepth": 16, "longEdge": 64}) });
+    let p = s.export_params(&json!({"preset": "deep", "path": "/out/a.png"})).unwrap();
+    let o = ExportOptions::from_params(&p).unwrap();
+    assert_eq!((o.format, o.bit_depth), (ExportFormat::Png, Some(16)));
+    assert_eq!(export_photo(&mut s, id, &o, 1).unwrap().bytes[24], 16, "a 16-bit PNG");
+    let p = s.export_params(&json!({"preset": "deep", "path": "/out/a.jpg"})).unwrap();
+    assert_eq!(ExportOptions::from_params(&p).unwrap().bit_depth, None);
+    let p = s.export_params(&json!({"preset": "deep", "path": "/out/a.jpg", "bitDepth": 16})).unwrap();
+    assert!(ExportOptions::from_params(&p).unwrap_err().to_string().contains("`bitDepth` must be 8 for jpeg"));
+    // such a preset can be saved: its depth is judged when it exports; with a format, at once
+    s.execute("export.savePreset", &json!({"name": "Deep 2", "params": {"bitDepth": 16}})).unwrap();
+    assert!(s.execute("export.savePreset", &json!({"name": "Deep 3", "params": {"format": "jpeg", "bitDepth": 16}})).is_err());
+}
+
 #[test]
 fn presets_conflicts_and_subfolders() {
     use crate::export::{Destination, export_batch};

crates/mcp/src/headless.rs

@@ -61,7 +61,7 @@ impl Headless {
     /// With `path` and no `format`, the format follows the path's extension.
     fn export(&mut self, p: &Value) -> Result<Value, String> {
         use lightcraft_engine::export::{Destination, ExportFormat, ExportOptions, Resize, prepare_batch, run_batch};
-        let p = &self.session.export_params(p)?;
+        let p = &self.session.export_params_or(p, ExportFormat::Png)?;
         // PNG for a `path` that names no format; `bitDepth` is checked against the format written
         let mut opts = ExportOptions::from_params_or(p, ExportFormat::Png).map_err(|e| e.to_string())?;
         if !ExportOptions::has_size_param(p) {

crates/mcp/src/tests.rs

@@ -169,6 +169,21 @@ fn export_checks_the_depth_of_the_format_a_bare_path_gets() {
     let _ = std::fs::remove_dir_all(&base);
 }
 
+/// Review of #1966: a preset's `bitDepth` is judged against the format this backend writes, PNG
+/// for a `path` that names none, so a 16-bit preset without `format` writes a 16-bit PNG.
+#[test]
+fn a_preset_depth_is_judged_against_the_format_written() {
+    let mut b = Headless::demo();
+    let base = std::env::temp_dir().join(format!("lc-mcp-preset-depth-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&base);
+    std::fs::create_dir_all(&base).unwrap();
+    b.session.export_presets.push(lightcraft_engine::export::ExportPreset { name: "Deep".into(), params: json!({"bitDepth": 16}) });
+    b.call("app.export", json!({"preset": "Deep", "path": base.join("output").to_string_lossy(), "longEdge": 32})).unwrap();
+    let bytes = std::fs::read(base.join("output")).unwrap();
+    assert!(bytes.starts_with(b"\x89PNG") && bytes[24] == 16, "a 16-bit PNG: {:?}", bytes.get(..25));
+    let _ = std::fs::remove_dir_all(&base);
+}
+
 /// Every option the `export` tool advertises reaches `app.export` (they were accepted, then dropped).
 #[test]
 fn export_tool_forwards_its_advertised_options() {