Fix construction-plane references in mirror, section, and sketches (#99)

Accept construction planes by name or feature ID, preserve parametric mirror references, and reject invalid plane inputs. Retain both mirror-plane and primitive-point regression test modules after resolving the conflict with main.

Validation: cargo xtask ci passed, including 282 engine tests, 138 UI tests, 62 UI scenarios, and WebAssembly.

crates/engine/src/cmd/features.rs

@@ -46,7 +46,7 @@ pub static COMMANDS: &[CommandSpec] = &[
         .at("SOLID", "CREATE")
         .icon("pattern_rect")
         .params("features: [names] | bodies: [names]; path_sketch: sketch; path: [curve ids in order]; count; spacing (between instances) | distance (first to last); orientation?: identical|path; flip?: bool"),
-    CommandSpec::new("solid.mirror", "Mirror", mirror).at("SOLID", "CREATE").icon("mirror").params("features: [names] | bodies: [names] (combine?: join with the original); plane: XY|XZ|YZ | {origin, x_dir, y_dir}"),
+    CommandSpec::new("solid.mirror", "Mirror", mirror).at("SOLID", "CREATE").icon("mirror").params("features: [names] | bodies: [names] (combine?: join with the original); plane: XY|XZ|YZ|construction plane name or id|{origin, normal}|{origin, x_dir, y_dir} (a construction plane keeps the mirror following it)"),
     CommandSpec::new("solid.hole", "Hole", hole)
         .at("SOLID", "CREATE")
         .icon("hole")
@@ -869,35 +869,35 @@ fn mirror(s: &mut Session, p: &Value) -> Result<Value> {
     let cmd = "solid.mirror";
     let bodies = string_list(p, "bodies");
     let features = if bodies.is_empty() { source_features(s, p, cmd)? } else { Vec::new() };
-    let plane = match p.get("plane") {
-        Some(Value::String(n)) if solvecraft_geom::Plane::named(n).is_some() => solvecraft_doc::PlaneRef::Origin { name: n.to_ascii_uppercase() },
-        Some(o @ Value::Object(_)) => {
-            let origin = o.get("origin").and_then(vec3).unwrap_or(Vec3::ZERO);
-            let pl = match (o.get("x_dir").and_then(vec3), o.get("y_dir").and_then(vec3), o.get("normal").and_then(vec3)) {
-                (Some(x), Some(y), _) => solvecraft_geom::Plane::new(origin, x, y),
-                (_, _, Some(n)) => solvecraft_geom::Plane::from_normal(origin, n),
-                _ => None,
-            };
-            solvecraft_doc::PlaneRef::Custom { plane: pl.ok_or_else(|| bad(cmd, "plane needs x_dir and y_dir, or normal"))? }
-        }
-        _ => return Err(bad(cmd, "`plane` must be XY, XZ, YZ or {origin, normal}")),
-    };
+    let plane = plane_param(s, p.get("plane"), cmd)?;
     let combine = bool_(p, "combine").unwrap_or(false);
     add_feature(s, p, FeatureKind::Mirror { features, plane, bodies, combine })
 }
 
-/// A plane reference from a parameter: an origin plane, a construction plane name, or an
-/// explicit plane `{origin, x_dir, y_dir}` / `{origin, normal}`.
+/// A plane reference from a parameter: an origin plane, a construction plane (by name or
+/// feature id; kept by name, so the feature follows the plane when it changes), or an
+/// explicit plane `{origin, x_dir, y_dir}` / `{origin, normal}`. Every command that takes a
+/// plane reads it here.
 pub(super) fn plane_param(s: &Session, v: Option<&Value>, cmd: &str) -> Result<solvecraft_doc::PlaneRef> {
     use solvecraft_doc::PlaneRef;
+    const WANT: &str = "XY, XZ, YZ, a construction plane name or id, or {origin, normal}";
+    let construction = |r: &str| match s.doc.find_feature(r) {
+        Some(f) if matches!(f.kind, FeatureKind::ConstructionPlane { .. }) => Ok(PlaneRef::Construction { name: f.name.clone() }),
+        Some(f) => Err(bad(cmd, format!("`{}` is not a construction plane (a plane is {WANT})", f.name))),
+        None => Err(bad(cmd, format!("no plane `{r}` (a plane is {WANT})"))),
+    };
     match v {
-        Some(Value::String(n)) if solvecraft_geom::Plane::named(n).is_some() => Ok(PlaneRef::Origin { name: n.to_ascii_uppercase() }),
-        Some(Value::String(n)) => match s.doc.find_feature(n).map(|f| &f.kind) {
-            Some(FeatureKind::ConstructionPlane { .. }) => Ok(PlaneRef::Construction { name: n.clone() }),
-            _ => Err(bad(cmd, format!("no plane `{n}`"))),
+        Some(Value::String(n)) if solvecraft_geom::Plane::named(n.trim()).is_some() => Ok(PlaneRef::Origin { name: n.trim().to_ascii_uppercase() }),
+        Some(Value::String(n)) => construction(n.trim()),
+        Some(Value::Number(id)) => match id.as_u64() {
+            Some(id) => construction(&id.to_string()),
+            None => Err(bad(cmd, format!("`plane` must be {WANT}"))),
         },
         Some(o @ Value::Object(_)) => {
-            let origin = o.get("origin").and_then(vec3).unwrap_or(Vec3::ZERO);
+            let origin = match o.get("origin") {
+                None => Vec3::ZERO,
+                Some(x) => vec3(x).ok_or_else(|| bad(cmd, "the plane's `origin` must be a point [x, y, z]"))?,
+            };
             let pl = match (o.get("x_dir").and_then(vec3), o.get("y_dir").and_then(vec3), o.get("normal").and_then(vec3)) {
                 (Some(x), Some(y), _) => solvecraft_geom::Plane::new(origin, x, y),
                 (_, _, Some(n)) => solvecraft_geom::Plane::from_normal(origin, n),
@@ -905,7 +905,7 @@ pub(super) fn plane_param(s: &Session, v: Option<&Value>, cmd: &str) -> Result<s
             };
             Ok(PlaneRef::Custom { plane: pl.ok_or_else(|| bad(cmd, "plane needs x_dir and y_dir, or normal"))? })
         }
-        _ => Err(bad(cmd, "`plane` must be XY, XZ, YZ, a construction plane name or {origin, normal}")),
+        _ => Err(bad(cmd, format!("`plane` must be {WANT}"))),
     }
 }
 

crates/engine/src/cmd/section.rs

@@ -10,7 +10,7 @@ use crate::{Result, Session};
 
 pub static COMMANDS: &[CommandSpec] =
     &[CommandSpec::new("inspect.section", "Section Analysis", section).at("SOLID", "INSPECT").icon("section").noundo().params(
-        "plane: XY|XZ|YZ | {origin, normal}; offset?: expr along the plane's normal (XY: +Z, XZ: +Y, YZ: +X); \
+        "plane: XY|XZ|YZ | construction plane name or id | {origin, normal}; offset?: expr along the plane's normal (XY: +Z, XZ: +Y, YZ: +X); \
          at?: expr, the cut's world coordinate on the named plane's axis (XY: Z, XZ: Y, YZ: X), instead of offset; \
          flip?: bool; or clear: true. Returns the cut's origin and normal, and for a named plane its axis and at",
     )];
@@ -24,18 +24,31 @@ fn section(s: &mut Session, p: &Value) -> Result<Value> {
     }
     // A named plane's world axis: which component of a point lies along its normal.
     let (origin, normal, axis) = match p.get("plane") {
-        Some(Value::String(n)) => {
-            let pl = Plane::named(n).ok_or_else(|| bad(cmd, format!("unknown plane `{n}` (XY, XZ, YZ)")))?;
+        Some(Value::String(n)) if Plane::named(n.trim()).is_some() => {
+            let pl = Plane::named(n.trim()).ok_or_else(|| bad(cmd, format!("unknown plane `{n}`")))?;
             let normal = pl.normal();
             let axis = [normal.x, normal.y, normal.z].iter().position(|c| c.abs() > 0.5);
             (pl.origin, normal, axis)
         }
+        // A construction plane (name or id): where it is now.
+        Some(v @ (Value::String(_) | Value::Number(_))) => {
+            let r = super::features::plane_param(s, Some(v), cmd)?;
+            let solvecraft_doc::PlaneRef::Construction { name } = &r else { return Err(bad(cmd, "`plane` must be a plane")) };
+            let st = s.model.state();
+            let pl = st
+                .construct
+                .iter()
+                .find(|c| &c.name == name)
+                .and_then(|c| if let solvecraft_doc::construct::ConstructGeom::Plane(pl) = &c.geom { Some(*pl) } else { None })
+                .ok_or_else(|| bad(cmd, format!("construction plane `{name}` is not built (rolled back, suppressed or failed)")))?;
+            (pl.origin, pl.normal(), None)
+        }
         Some(o @ Value::Object(_)) => {
             let origin = o.get("origin").and_then(vec3).unwrap_or(Vec3::ZERO);
             let n = o.get("normal").and_then(vec3).and_then(|n| n.normalized()).ok_or_else(|| bad(cmd, "the plane needs a non-zero `normal`"))?;
             (origin, n, None)
         }
-        _ => return Err(bad(cmd, "`plane` must be XY, XZ, YZ or {origin, normal}")),
+        _ => return Err(bad(cmd, "`plane` must be XY, XZ, YZ, a construction plane name or id, or {origin, normal}")),
     };
     let length = |key: &str| -> Result<Option<f64>> {
         match expr(p, key) {

crates/engine/src/cmd/sketch.rs

@@ -20,7 +20,7 @@ pub static COMMANDS: &[CommandSpec] = &[
     CommandSpec::new("sketch.create", "Create Sketch", create_sketch)
         .at("SOLID", "CREATE")
         .icon("sketch")
-        .params("plane: XY|XZ|YZ | {origin, x_dir, y_dir} | {face: [x,y,z]} (planar face at point), offset?: expr, name?"),
+        .params("plane: XY|XZ|YZ | construction plane name or id | {origin, x_dir, y_dir} | {face: [x,y,z]} (planar face at point), offset?: expr, name?"),
     CommandSpec::new("sketch.edit", "Edit Sketch", edit_sketch).icon("sketch").params("sketch: id or name"),
     CommandSpec::new("sketch.finish", "Finish Sketch", finish_sketch).at("SKETCH", "FINISH SKETCH").icon("finish").enabled(in_sketch).noundo(),
     CommandSpec::new("sketch.line", "Line", draw_line)
@@ -348,15 +348,8 @@ pub(super) fn result(sk_out: (Vec<String>, Vec<String>), info: Value) -> Value {
 pub(super) fn plane_ref(s: &Session, p: &Value, cmd: &str) -> Result<PlaneRef> {
     let base = match p.get("plane") {
         None => PlaneRef::Origin { name: "XY".into() },
-        Some(Value::String(n)) => {
-            if Plane::named(n).is_some() {
-                PlaneRef::Origin { name: n.to_ascii_uppercase() }
-            } else if matches!(s.doc.find_feature(n).map(|f| &f.kind), Some(FeatureKind::ConstructionPlane { .. })) {
-                PlaneRef::Construction { name: n.clone() }
-            } else {
-                return Err(bad(cmd, format!("unknown plane `{n}` (XY, XZ, YZ or a construction plane)")));
-            }
-        }
+        // An origin plane, or a construction plane by name or id.
+        Some(v @ (Value::String(_) | Value::Number(_))) => super::features::plane_param(s, Some(v), cmd)?,
         Some(v @ Value::Object(o)) => {
             if let Some(fp) = o.get("face").and_then(vec3) {
                 // A planar face of a body at this point, as the active component sees the model
@@ -388,7 +381,7 @@ pub(super) fn plane_ref(s: &Session, p: &Value, cmd: &str) -> Result<PlaneRef> {
                 PlaneRef::Custom { plane: Plane::new(origin, x, y).ok_or_else(|| bad(cmd, "degenerate plane"))? }
             }
         }
-        _ => return Err(bad(cmd, "`plane` must be XY, XZ, YZ or an object")),
+        _ => return Err(bad(cmd, "`plane` must be XY, XZ, YZ, a construction plane name or id, or an object")),
     };
     Ok(match expr(p, "offset") {
         Some(d) => PlaneRef::Offset { base: Box::new(base), distance: d },

crates/engine/src/tests.rs

@@ -36,6 +36,7 @@ fn join_extrude_from_a_chamfered_top_face() {
 }
 
 mod component_frames;
+mod mirror_planes;
 mod primitive_points;
 mod robustness;
 

crates/engine/src/tests/mirror_planes.rs

@@ -0,0 +1,65 @@
+//! Commands that take a plane accept construction planes by name or id, like the origin planes
+//! (user report: `solid.mirror` refused a construction plane although its docs list plane
+//! names).
+
+use serde_json::json;
+
+use super::run;
+use crate::*;
+
+fn min_x(s: &Session, body: &str) -> f64 {
+    let b = s.model.state().body(body).map(|b| b.body.clone()).unwrap();
+    solvecraft_kernel::measure(&b).unwrap().bbox.min.x
+}
+
+#[test]
+fn mirror_across_a_construction_plane_follows_it() {
+    let mut s = Session::default();
+    run(&mut s, "parameters.change", json!({"name": "w", "expression": "100 mm"}));
+    let pl = run(&mut s, "construct.plane.offset", json!({"base": "YZ", "offset": "w / 2", "name": "Mid"}));
+    run(&mut s, "solid.box", json!({"length": 10, "width": 10, "height": 10, "body_name": "Ledger"}));
+    let before = s.model.state().bodies.len();
+    // By name: the copy lands at x = 90…100.
+    run(&mut s, "solid.mirror", json!({"bodies": ["Ledger"], "plane": "Mid"}));
+    let st = s.model.state();
+    assert_eq!(st.bodies.len(), before + 1);
+    let copy = st.bodies.last().map(|b| b.name.clone()).unwrap();
+    assert!((min_x(&s, &copy) - 90.0).abs() < 1e-6, "{}", min_x(&s, &copy));
+    // Parametric: moving the plane moves the copy.
+    run(&mut s, "parameters.change", json!({"name": "w", "expression": "60 mm"}));
+    assert!((min_x(&s, &copy) - 50.0).abs() < 1e-6, "{}", min_x(&s, &copy));
+    // By id, and case-insensitively by name.
+    let id = pl["feature"].as_u64().unwrap();
+    run(&mut s, "solid.mirror", json!({"bodies": ["Ledger"], "plane": id.to_string()}));
+    run(&mut s, "solid.mirror", json!({"bodies": ["Ledger"], "plane": "mid"}));
+    let st = s.model.state();
+    assert_eq!(st.bodies.len(), before + 3);
+    for b in st.bodies.iter().skip(before) {
+        assert!((min_x(&s, &b.name) - 50.0).abs() < 1e-6, "{}", b.name);
+    }
+}
+
+#[test]
+fn unknown_mirror_planes_are_refused_with_the_choices() {
+    let mut s = Session::default();
+    run(&mut s, "solid.box", json!({"length": 10, "width": 10, "height": 10, "body_name": "B"}));
+    let e = s.execute("solid.mirror", &json!({"bodies": ["B"], "plane": "Nope"})).unwrap_err().to_string();
+    assert!(e.contains("Nope"), "{e}");
+    let e = s.execute("solid.mirror", &json!({"bodies": ["B"], "plane": 5})).unwrap_err().to_string();
+    assert!(e.contains("construction plane"), "{e}");
+    // A plane object with an unreadable origin is refused, not put at the world origin.
+    assert!(s.execute("solid.mirror", &json!({"bodies": ["B"], "plane": {"origin": ["x", 0, 0], "normal": [1, 0, 0]}})).is_err());
+    // A feature that is not a plane is no plane.
+    assert!(s.execute("solid.mirror", &json!({"bodies": ["B"], "plane": "Box1"})).is_err());
+    assert_eq!(s.model.state().bodies.len(), 1);
+}
+
+/// Section Analysis cuts along a construction plane too.
+#[test]
+fn section_by_a_construction_plane() {
+    let mut s = Session::default();
+    run(&mut s, "construct.plane.offset", json!({"base": "XY", "offset": 7, "name": "Cut"}));
+    let r = run(&mut s, "inspect.section", json!({"plane": "Cut"}));
+    assert_eq!(r["section"]["origin"][2].as_f64(), Some(7.0), "{r}");
+    assert!(s.execute("inspect.section", &json!({"plane": "Nope"})).is_err());
+}