Persistent naming: sketches on faces store the face's name and find it by name first (an upstream feature that adds a nearer parallel face no longer moves the sketch); a vanished face falls back to the old geometric search with a warning

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01AS5Tn3xieewUSg3uxrMVdL

crates/doc/src/assembly.rs

@@ -132,7 +132,7 @@ fn map_plane(m: &Mat, p: &Plane) -> Plane {
 fn map_plane_ref(m: &Mat, p: &mut PlaneRef) {
     match p {
         PlaneRef::Custom { plane } => *plane = map_plane(m, plane),
-        PlaneRef::Face { plane, at } => {
+        PlaneRef::Face { plane, at, .. } => {
             *plane = map_plane(m, plane);
             *at = apply_point(m, *at);
         }

crates/doc/src/document.rs

@@ -45,8 +45,14 @@ pub enum PlaneRef {
     Construction { name: String },
     /// A planar body face: found again on each evaluation (the face with this plane's normal
     /// nearest `at`), so sketches on it follow when earlier features change; `plane` is the
-    /// plane as picked (its frame, and the fallback when the face is gone).
-    Face { plane: Plane, at: Vec3 },
+    /// plane as picked (its frame, and the fallback when the face is gone). `name` is the face's
+    /// persistent name (found first; see `naming`).
+    Face {
+        plane: Plane,
+        at: Vec3,
+        #[serde(default, skip_serializing_if = "Option::is_none")]
+        name: Option<String>,
+    },
 }
 
 /// Which closed profiles of a sketch a feature uses.

crates/doc/src/eval.rs

@@ -1729,8 +1729,38 @@ mod more;
 
 /// The plane of the body face a sketch sits on, found again: among planar faces with the
 /// picked plane's normal, the one nearest `at`. The picked frame moves along its normal onto it.
-fn face_plane(st: &ModelState, picked: &Plane, at: Vec3) -> (Plane, Option<String>) {
+fn face_plane(st: &ModelState, picked: &Plane, at: Vec3, name: Option<&str>) -> (Plane, Option<String>) {
     let n = picked.normal();
+    // By name first: the face (or a piece of it) with that name, facing the same way.
+    if let Some(name) = name {
+        let stem = crate::naming::strip_piece(name);
+        let mut found: Option<(f64, f64)> = None;
+        for b in st.bodies.iter().filter(|b| !b.body.is_mesh()) {
+            let names = crate::naming::face_names(b);
+            let tol = (b.body.size() * 1e-3).max(1e-3);
+            let Ok(faces) = b.body.faces(tol) else { continue };
+            for f in faces {
+                let hit = names.get(f.index).is_some_and(|x| crate::naming::strip_piece(x) == stem);
+                if let (true, Some(fnrm)) = (hit, f.plane_normal)
+                    && fnrm.dot(n) > 1.0 - 1e-6
+                {
+                    let d = f.centroid.dist(at);
+                    if found.is_none_or(|x| d < x.0) {
+                        found = Some((d, f.centroid.dot(n)));
+                    }
+                }
+            }
+        }
+        if let Some((_, off)) = found {
+            return (picked.offset(off - picked.origin.dot(n)), None);
+        }
+        let (pl, w) = face_plane(st, picked, at, None);
+        let note = format!(
+            "the face `{name}` the sketch was on no longer exists; {}",
+            if w.is_some() { "the sketch stays where it was" } else { "using the face at the picked point" }
+        );
+        return (pl, Some(note));
+    }
     let mut best: Option<(f64, f64)> = None; // (distance from `at`, plane offset along n)
     for b in &st.bodies {
         let tol = (b.body.size() * 1e-3).max(1e-3);
@@ -1781,7 +1811,7 @@ fn eval_feature(doc: &Document, vals: &BTreeMap<String, Value>, f: &Feature, st:
         | FeatureKind::Remove { .. } => more::eval(doc, vals, f, st),
         FeatureKind::Sketch { plane, sketch } => {
             let (plane, face_warning) = match plane {
-                PlaneRef::Face { plane: picked, at } => face_plane(st, picked, *at),
+                PlaneRef::Face { plane: picked, at, name } => face_plane(st, picked, *at, name.as_deref()),
                 other => (doc.resolve_plane(vals, other, 0)?, None),
             };
             let mut sk = sketch.clone();

crates/engine/src/cmd/sketch.rs

@@ -314,6 +314,7 @@ pub(super) fn plane_ref(s: &Session, p: &Value, cmd: &str) -> Result<PlaneRef> {
                 // A planar face of a body at this point.
                 let st = s.model.state();
                 let mut found = None;
+                let mut face_name = None;
                 for b in &st.bodies {
                     let tol = (b.body.size() * 1e-3).max(1e-3);
                     for f in b.body.faces(tol).unwrap_or_default() {
@@ -324,11 +325,12 @@ pub(super) fn plane_ref(s: &Session, p: &Value, cmd: &str) -> Result<PlaneRef> {
                             // Sketch origin: the model origin projected onto the face plane.
                             let d = (f.centroid + n * (fp - f.centroid).dot(n)).dot(n);
                             found = Plane::from_normal(n * d, n);
+                            face_name = solvecraft_doc::naming::face_names(b).get(f.index).cloned();
                         }
                     }
                 }
                 let pl = found.ok_or_else(|| bad(cmd, "no planar face at that point"))?;
-                PlaneRef::Face { plane: pl, at: fp }
+                PlaneRef::Face { plane: pl, at: fp, name: face_name }
             } else {
                 let origin = v.get("origin").and_then(vec3).unwrap_or(Vec3::ZERO);
                 let x = v.get("x_dir").and_then(vec3).ok_or_else(|| bad(cmd, "plane needs x_dir"))?;

crates/engine/src/tests.rs

@@ -1296,4 +1296,33 @@ mod naming {
             assert!(n.iter().any(|x| x.starts_with(want)), "{want} in {n:?}");
         }
     }
+
+    #[test]
+    fn a_sketch_on_a_face_stays_on_that_face() {
+        let mut s = Session::default();
+        run(&mut s, "SketchCreate", json!({"plane": "XY", "name": "S"}));
+        run(&mut s, "ShapeRectangleTwoPoint", json!({"p0": [0, 0], "p1": [40, 30]}));
+        run(&mut s, "SketchStop", json!({}));
+        run(&mut s, "Extrude", json!({"distance": 10, "name": "Block"}));
+        run(&mut s, "SketchCreate", json!({"plane": {"face": [30, 15, 10]}, "name": "Top"}));
+        run(&mut s, "CircleCenterRadius", json!({"center": [30, 15], "radius": 3}));
+        run(&mut s, "SketchStop", json!({}));
+        run(&mut s, "Extrude", json!({"sketch": "Top", "distance": 5, "operation": "join", "name": "Peg"}));
+        let top = |s: &mut Session| run(s, "MeasureCommand", json!({}))["bodies"][0]["bbox"]["max"][2].as_f64().unwrap_or(0.0);
+        assert!((top(&mut s) - 15.0).abs() < 1e-6);
+        // Upstream, a tall boss over the picked point: the nearest upward face is now its top,
+        // but the sketch keeps to the block's top face by name.
+        run(&mut s, "timeline.rollTo", json!({"feature": "Block"}));
+        run(&mut s, "SketchCreate", json!({"plane": "XY", "name": "B"}));
+        run(&mut s, "ShapeRectangleTwoPoint", json!({"p0": [8, -5], "p1": [45, 35]}));
+        run(&mut s, "SketchStop", json!({}));
+        run(&mut s, "Extrude", json!({"sketch": "B", "distance": 20, "operation": "join", "name": "Boss"}));
+        run(&mut s, "timeline.rollTo", json!({}));
+        // (Its projected face outline changed, which it says; its plane did not.)
+        assert!(!warning(&s, "Top").unwrap_or_default().contains("no longer exists"));
+        // The peg starts on the block's top (z = 10), inside the boss: the part stays 20 tall.
+        assert!((top(&mut s) - 20.0).abs() < 1e-6, "{}", top(&mut s));
+        let sk = s.model.state().sketch(s.doc.find_feature("Top").map(|f| f.id).unwrap_or(0)).map(|x| x.plane.origin.z);
+        assert_eq!(sk.map(|z| (z * 1e6).round() / 1e6), Some(10.0));
+    }
 }