From 5f16fc363929cb7c399e39566ce5135818958508 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Tue, 20 Oct 2026 20:12:14 -0700 Subject: [PATCH] Image Trace: imageTrace.make refuses unknown keys inside params imageTrace.make and imageTrace.makeAndExpand checked their own keys, but not the keys inside `params`. A misspelled setting such as `colours` was dropped, and the trace ran with the preset's value and no error. A key inside `params` that is not a trace setting is now an error that names the nearest setting, as for a command's own keys. Errors from imageTrace.makeAndExpand, including those for `enhanced` and a missing swatch library, name that command instead of imageTrace.make. --- crates/engine/src/cmd/buildcmds.rs | 38 +++++++++++++++++------------- crates/engine/src/cmd/params.rs | 2 +- crates/engine/src/tests_build.rs | 34 ++++++++++++++++++++++++++ docs/mcp.md | 3 ++- 4 files changed, 58 insertions(+), 19 deletions(-) diff --git a/crates/engine/src/cmd/buildcmds.rs b/crates/engine/src/cmd/buildcmds.rs index a441d3a07..e10679f4b 100644 --- a/crates/engine/src/cmd/buildcmds.rs +++ b/crates/engine/src/cmd/buildcmds.rs @@ -951,40 +951,44 @@ fn view_param(cmd: &str, p: &Value) -> Result> { v.as_str().and_then(TraceView::from_id).map(Some).ok_or_else(|| bad(cmd, format!("view must be one of {}", ids()))) } -fn trace_params(p: &Value) -> Result { +/// The parameters command `c` traces with: the preset's, with `params` over them. +fn trace_params(c: &str, p: &Value) -> Result { let name = str_param(p, "preset").unwrap_or("Default"); - let enhanced = switch_or(p, "enhanced", false, "imageTrace.make")?; + let enhanced = switch_or(p, "enhanced", false, c)?; // (A preset without an enhanced version is traced as it is.) let base = enhanced.then(|| tr::enhanced_preset(name)).flatten().or_else(|| tr::preset(name)); - let base = base.ok_or_else(|| bad("imageTrace.make", format!("unknown preset `{name}` (see imageTrace.presets)")))?; + let base = base.ok_or_else(|| bad(c, format!("unknown preset `{name}` (see imageTrace.presets)")))?; let Some(over) = p.get("params").filter(|v| !v.is_null()) else { return Ok(base) }; let mut v = serde_json::to_value(&base).map_err(|e| EngineError::Other(e.to_string()))?; - let (Some(o), Some(src)) = (v.as_object_mut(), over.as_object()) else { return Err(bad("imageTrace.make", "params must be an object")) }; + let (Some(o), Some(src)) = (v.as_object_mut(), over.as_object()) else { return Err(bad(c, "params must be an object")) }; + // A key that isn't one of the settings is an error, with the nearest setting. + if let Some(key) = src.keys().find(|k| !o.contains_key(k.as_str())) { + return Err(super::params::unknown(c, key, &o.keys().map(String::as_str).collect::>())); + } for (k, val) in src { o.insert(k.clone(), val.clone()); } - let params: tr::TraceParams = serde_json::from_value(v).map_err(|e| bad("imageTrace.make", e.to_string()))?; + let params: tr::TraceParams = serde_json::from_value(v).map_err(|e| bad(c, e.to_string()))?; if !params.fills && !params.strokes { - return Err(bad("imageTrace.make", "Create needs fills, strokes or both")); + return Err(bad(c, "Create needs fills, strokes or both")); } if !(params.stroke_width.is_finite() && params.stroke_width > 0.0) { - return Err(bad("imageTrace.make", "strokeWidth must be a positive number of pixels")); + return Err(bad(c, "strokeWidth must be a positive number of pixels")); } Ok(params) } /// Document Library in Color mode: look the library's colours up into `params.swatches` → each -/// one as traced (sRGB) with the colour to fill with. -fn trace_library(s: &Session, params: &mut tr::TraceParams) -> Result> { - const C: &str = "imageTrace.make"; +/// one as traced (sRGB) with the colour to fill with. Errors name command `c`. +fn trace_library(c: &str, s: &Session, params: &mut tr::TraceParams) -> Result> { if params.mode != tr::Mode::Color || params.palette != tr::Palette::DocumentLibrary { return Ok(vec![]); } let key = params.library.as_str(); let colors = super::swatchlib::library_colors(s, key) - .ok_or_else(|| bad(C, format!("no swatch library `{key}` (see swatch.library.list; \"document\": the document's swatches)")))?; + .ok_or_else(|| bad(c, format!("no swatch library `{key}` (see swatch.library.list; \"document\": the document's swatches)")))?; if colors.is_empty() { - return Err(bad(C, format!("`{key}` has no colours"))); + return Err(bad(c, format!("`{key}` has no colours"))); } let library: Vec<([u8; 3], Color)> = colors .into_iter() @@ -1088,9 +1092,9 @@ fn grouped(d: &mut Document, nodes: Vec, parents: &[Option]) -> Vec } fn trace_make(s: &mut Session, p: &Value, expand: bool) -> Result { - const C: &str = "imageTrace.make"; - let mut params = trace_params(p)?; - let view = view_param(C, p)?; + let c = if expand { "imageTrace.makeAndExpand" } else { "imageTrace.make" }; + let mut params = trace_params(c, p)?; + let view = view_param(c, p)?; let params_json = serde_json::to_value(¶ms).map_err(|e| EngineError::Other(e.to_string()))?; // The preset's name if the parameters are exactly that preset's or its enhanced version's, // else "Custom". @@ -1114,12 +1118,12 @@ fn trace_make(s: &mut Session, p: &Value, expand: bool) -> Result { _ => None, } }) - .ok_or_else(|| bad(C, "select an image (or an Image Trace group)"))? + .ok_or_else(|| bad(c, "select an image (or an Image Trace group)"))? }; let blob = s.doc()?.doc.images.get(&img.key).ok_or_else(|| EngineError::Other(format!("image data `{}` is missing", img.key)))?; // An image over the tracer's size is traced scaled down (the paths are scaled back below). let raster = tr::Raster::decode_to_fit(&blob.bytes).map_err(|e| EngineError::Other(e.to_string()))?; - let library = trace_library(s, &mut params)?; + let library = trace_library(c, s, &mut params)?; let res = tr::trace_within(&raster, ¶ms, MAX_TRACE_ANCHORS).map_err(|e| EngineError::Other(e.to_string()))?; let warnings = trace_warnings((img.width, img.height), (raster.width, raster.height)); // Pixel space of the decoded raster → the image object's pixel space → document. diff --git a/crates/engine/src/cmd/params.rs b/crates/engine/src/cmd/params.rs index a0b13db2e..dae1cb1a1 100644 --- a/crates/engine/src/cmd/params.rs +++ b/crates/engine/src/cmd/params.rs @@ -202,7 +202,7 @@ pub fn check_keys(id: &str, doc: &'static str, params: &Value) -> Result<()> { /// The parameter error of command `id` for `key`, which isn't one of `keys`: with the nearest of /// them, or all of them. -fn unknown(id: &str, key: &str, keys: &[&str]) -> crate::EngineError { +pub(crate) fn unknown(id: &str, key: &str, keys: &[&str]) -> crate::EngineError { bad( id, match nearest(key, keys) { diff --git a/crates/engine/src/tests_build.rs b/crates/engine/src/tests_build.rs index 3464c3c77..33fd63662 100644 --- a/crates/engine/src/tests_build.rs +++ b/crates/engine/src/tests_build.rs @@ -2161,6 +2161,40 @@ fn image_trace_gradients_fills_with_a_gradient() { assert!(flat.len() > 1 && flat.iter().all(|n| matches!(n.appearance.fill_paint(), Paint::Solid { .. }))); } +/// A key inside `params` that isn't a setting is refused with the nearest setting, the image is +/// left as it was, and the error names the command that was run. +#[test] +fn image_trace_refuses_an_unknown_setting() { + for id in ["imageTrace.make", "imageTrace.makeAndExpand"] { + let mut s = session(); + let img = image_doc(&mut s); + let e = s.execute(id, &json!({"preset": "6 Colors", "params": {"mode": "color", "colours": 3}})).unwrap_err().to_string(); + assert!(e.contains(&format!("`{id}`")) && e.contains("unknown parameter `colours`; did you mean `colors`?"), "{e}"); + assert!(matches!(node(&s, img).kind, NodeKind::Image(_))); + let e = s.execute(id, &json!({"params": {"zz": 1}})).unwrap_err().to_string(); + assert!(e.contains("unknown parameter `zz`; the command takes: ") && e.contains("colorDetail"), "{e}"); + let e = s.execute(id, &json!({"enhanced": "yes"})).unwrap_err().to_string(); + assert!(e.contains(&format!("`{id}`")), "{e}"); + let library = json!({"preset": "6 Colors", "params": {"palette": "documentLibrary", "library": "No Such Library"}}); + let e = s.execute(id, &library).unwrap_err().to_string(); + assert!(e.contains(&format!("`{id}`")) && e.contains("no swatch library"), "{e}"); + } +} + +/// Every preset's settings, as imageTrace.presets lists them, trace when passed back whole. +#[test] +fn image_trace_takes_every_preset_back_as_params() { + let list = session().execute("imageTrace.presets", &json!({})).unwrap(); + for preset in list["presets"].as_array().unwrap() { + for params in [&preset["params"], &preset["enhanced"]].into_iter().filter(|p| !p.is_null()) { + let mut s = session(); + image_doc(&mut s); + let r = s.execute("imageTrace.make", &json!({"preset": preset["name"], "params": params})); + assert!(r.is_ok(), "{}: {r:?}", preset["name"]); + } + } +} + /// A preset's enhanced version: the preset with Auto Grouping and Shapes, under the preset's name. #[test] fn image_trace_enhanced_presets() { diff --git a/docs/mcp.md b/docs/mcp.md index a0ef6e4b1..5024d376f 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1447,7 +1447,8 @@ already walks Image Trace. Short agent entry points: format's first extension (`png8` gives `.png`, `template` gives `.vctemplate`). A step without `format` keeps the source file's extension (`.vectorcraft` over data sets). - Image Trace: `imageTrace.presets`, `imageTrace.make {id?, preset?, params?, view?}` (or `imageTrace.makeAndExpand`), - `imageTrace.setView`, `imageTrace.expand`, `imageTrace.release`. + `imageTrace.setView`, `imageTrace.expand`, `imageTrace.release`. A key inside `params` that is not one of the trace + settings, such as `colours`, is an error that names it and the nearest setting (`colors`), as for a command's own keys. - Live Paint: `livePaint.make`, `livePaint.merge`, `livePaint.fill {group?, point}`, `livePaint.strokeEdge`, `livePaint.info {group}`, `livePaint.release`, `livePaint.expand`, and `livePaint.gapOptions {ids?, detect?, size?, color?, closeGaps?}` (Object › Live Paint › Gap Options). With `detect: true`, paint stops at the gaps that run