Engine: the sketch store's scan also catches the active sketch taken as a value (#5425, review of #6336)

The scan only caught `active_sketch.ok_or`, so main's `let Some(id) = s.active_sketch else …`
could have come back unseen (0xlillith's review). It now also flags the value-read forms:
`) = s.active_sketch` (let-else and if let), `=> s.active_sketch` (a match arm),
`s.active_sketch.and_then(` and `s.active_sketch.map(`. State checks (`is_some`, `is_none`,
`== Some(`), assignments and snapshot copies stay allowed.

The wider scan found five commands that hand-rolled the store's `target` rule (a `sketch` param or
the sketch being edited). They now call `sketch_store::target`: sketch.inspect, sketch.glyphs,
sketch.export_dxf, inspect.curvature_comb and inspect.minimum_radius (their `curves` form).
Their refusals become target's:
- no `sketch` and none being edited: "no sketch is being edited" (was "no such sketch (and none
  is active)", or "no such sketch" for sketch.glyphs);
- an unknown name or id: "no sketch `X`" (an unknown id used to reach "that sketch is not
  evaluated");
- a feature that is not a sketch: "`X` is not a sketch";
- a `sketch` that is neither an id nor a name (true, a list, an object): refused,
  where it used to fall back to the sketch being edited. A null `sketch` still means none given
  (the scenario harness sends one): target now reads it that way for every sketch command
  (target_reads_a_null_sketch_as_none_given).
No test, doc or schema text named the old wording; the params docs ("sketch?: id|name (default
active)") still hold.

The two remaining value reads, the default sketch for a feature (features.rs) and the sketch a
measurement prefers (measure_sel.rs), go through `sketch_store::active`, with no change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/cmd/features.rs

@@ -290,7 +290,7 @@ pub(super) fn feature_sketch(s: &Session, p: &Value, cmd: &str) -> Result<u64> {
             _ => Err(bad(cmd, format!("`{key}` is not a sketch"))),
         };
     }
-    if let Some(a) = s.active_sketch {
+    if let Some(a) = super::sketch_store::active(s) {
         return Ok(a);
     }
     let limit = s.doc.marker.unwrap_or(usize::MAX);

crates/engine/src/cmd/inspect.rs

@@ -232,12 +232,7 @@ fn sketch_json(s: &Session, id: u64) -> Option<Value> {
 }
 
 fn sketch_inspect(s: &mut Session, p: &Value) -> Result<Value> {
-    let id = match p.get("sketch") {
-        Some(Value::Number(n)) => s.doc.find_feature(&n.to_string()).map(|f| f.id),
-        Some(Value::String(x)) => s.doc.find_feature(x).map(|f| f.id),
-        _ => s.active_sketch,
-    }
-    .ok_or_else(|| bad("sketch.inspect", "no such sketch (and none is active)"))?;
+    let id = super::sketch_store::target(s, p, "sketch.inspect")?;
     sketch_json(s, id).ok_or_else(|| EngineError::Other("that sketch is not evaluated (rolled back or not a sketch)".into()))
 }
 

crates/engine/src/cmd/measure_sel.rs

@@ -150,7 +150,7 @@ fn sketch_of<'a>(
     if let Some(id) = sketch {
         return st.sketch(id);
     }
-    if let Some(ss) = s.active_sketch.and_then(|id| st.sketch(id)).filter(|ss| has(&ss.sketch)) {
+    if let Some(ss) = super::sketch_store::active(s).and_then(|id| st.sketch(id)).filter(|ss| has(&ss.sketch)) {
         return Some(ss);
     }
     st.sketches.iter().rev().map(|ss| &**ss).find(|ss| has(&ss.sketch))

crates/engine/src/cmd/sketch_constraints.rs

@@ -460,12 +460,7 @@ fn anchor(sk: &Sketch, k: &ConstraintKind) -> Option<Vec2> {
 }
 
 fn glyphs(s: &mut Session, p: &Value) -> Result<Value> {
-    let id = match p.get("sketch") {
-        Some(Value::Number(n)) => n.as_u64(),
-        Some(Value::String(x)) => s.doc.find_feature(x).map(|f| f.id),
-        _ => s.active_sketch,
-    }
-    .ok_or_else(|| bad("sketch.glyphs", "no such sketch"))?;
+    let id = sketch_store::target(s, p, "sketch.glyphs")?;
     let st = s.model.state();
     let ss = st.sketch(id).ok_or_else(|| EngineError::Other("that sketch is not evaluated".into()))?;
     let failing = &ss.report.failing;

crates/engine/src/cmd/sketch_import.rs

@@ -178,12 +178,7 @@ fn insert_svg(s: &mut Session, p: &Value) -> Result<Value> {
 
 fn export_dxf(s: &mut Session, p: &Value) -> Result<Value> {
     let cmd = "sketch.export_dxf";
-    let id = match p.get("sketch") {
-        Some(Value::Number(n)) => n.as_u64(),
-        Some(Value::String(x)) => s.doc.find_feature(x).map(|f| f.id),
-        _ => s.active_sketch,
-    }
-    .ok_or_else(|| bad(cmd, "no such sketch (and none is active)"))?;
+    let id = sketch_store::target(s, p, cmd)?;
     let st = s.model.state();
     let ss = st.sketch(id).ok_or_else(|| EngineError::Other("that sketch is not evaluated".into()))?;
     let sk = &ss.sketch;

crates/engine/src/cmd/sketch_inspect.rs

@@ -79,12 +79,7 @@ fn curvature_comb(s: &mut Session, p: &Value) -> Result<Value> {
     let mut combs: Vec<(String, Vec<(Vec3, f64, Vec3)>)> = Vec::new();
     let curves = string_list(p, "curves");
     if !curves.is_empty() {
-        let id = match p.get("sketch") {
-            Some(Value::Number(n)) => n.as_u64(),
-            Some(Value::String(x)) => s.doc.find_feature(x).map(|f| f.id),
-            _ => s.active_sketch,
-        }
-        .ok_or_else(|| bad(cmd, "no such sketch (and none is active)"))?;
+        let id = super::sketch_store::target(s, p, cmd)?;
         let ss = st.sketch(id).ok_or_else(|| EngineError::Other("that sketch is not evaluated".into()))?;
         for c in curves.iter().take(1000) {
             let ci = ss.sketch.curve_index(c).ok_or_else(|| bad(cmd, format!("unknown curve `{c}`")))?;
@@ -230,12 +225,7 @@ fn minimum_radius(s: &mut Session, p: &Value) -> Result<Value> {
     let mut items: Vec<(String, Option<(f64, Vec3)>)> = Vec::new();
     let curves = string_list(p, "curves");
     if !curves.is_empty() {
-        let id = match p.get("sketch") {
-            Some(Value::Number(n)) => n.as_u64(),
-            Some(Value::String(x)) => s.doc.find_feature(x).map(|f| f.id),
-            _ => s.active_sketch,
-        }
-        .ok_or_else(|| bad(cmd, "no such sketch (and none is active)"))?;
+        let id = super::sketch_store::target(s, p, cmd)?;
         let ss = st.sketch(id).ok_or_else(|| EngineError::Other("that sketch is not evaluated".into()))?;
         for c in curves.iter().take(1000) {
             let ci = ss.sketch.curve_index(c).ok_or_else(|| bad(cmd, format!("unknown curve `{c}`")))?;

crates/engine/src/cmd/sketch_store.rs

@@ -11,9 +11,10 @@ use solidcraft_sketch::Sketch;
 use crate::params::bad;
 use crate::{EngineError, Result, Session};
 
-/// The sketch a command acts on: its `sketch` param (id or name), or the sketch being edited.
+/// The sketch a command acts on: its `sketch` param (id or name), or the sketch being edited
+/// (also when `sketch` is null, as callers send an optional value they don't have).
 pub(super) fn target(s: &Session, p: &Value, cmd: &str) -> Result<u64> {
-    if let Some(v) = p.get("sketch") {
+    if let Some(v) = p.get("sketch").filter(|v| !v.is_null()) {
         let key = match v {
             Value::Number(n) => n.to_string(),
             Value::String(x) => x.clone(),
@@ -63,12 +64,41 @@ pub(super) fn checked(sk: &Sketch) -> Result<()> {
 
 #[cfg(test)]
 mod tests {
+    use serde_json::json;
+
+    use crate::Session;
+
+    /// No `sketch`, or a null one, is the sketch being edited; anything else that isn't an id or
+    /// a name is refused (#6336: sketch.inspect's callers send null for "none given").
+    #[test]
+    fn target_reads_a_null_sketch_as_none_given() {
+        let mut s = Session::default();
+        s.execute("sketch.create", &json!({"plane": "XY"})).unwrap();
+        let active = s.active_sketch.unwrap();
+        for p in [json!({}), json!({"sketch": null})] {
+            assert_eq!(super::target(&s, &p, "test").unwrap(), active, "{p}");
+        }
+        assert!(super::target(&s, &json!({"sketch": true}), "test").is_err());
+    }
+
     /// Each sketch command finds and keeps its sketch through this store (W38.0). Outside it, the
     /// command sources (non-test code) reach a sketch directly only on a line marked
     /// `// design-only:` in the files that work on the design's own sketches.
     #[test]
     fn each_sketch_command_goes_through_the_store() {
-        const PATTERNS: [&str; 4] = [".sketch_mut(", "doc.sketch(", " d.sketch(", "active_sketch.ok_or"];
+        // Sketches read or written, and the sketch being edited taken as a value (let-else, if let,
+        // a match arm, `and_then`, `map`, `ok_or`); its state checks, assignments and snapshot copies
+        // stay allowed.
+        const PATTERNS: [&str; 8] = [
+            ".sketch_mut(",
+            "doc.sketch(",
+            " d.sketch(",
+            "active_sketch.ok_or",
+            ") = s.active_sketch",
+            "=> s.active_sketch",
+            "s.active_sketch.and_then(",
+            "s.active_sketch.map(",
+        ];
         // A motion trace's wire, a joint's curve and Redefine Sketch Plane: the design's sketches.
         const DESIGN_ONLY: [&str; 3] = ["browser.rs", "joints_inspect.rs", "motion_trace.rs"];
         let mut stack = vec![std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/cmd")];