Sketch: accept redundant but consistent constraints (#687) (#699)

A clover of same-size rectangles drawn around a shared corner was refused as over-constrained.

Constraints that remove no degree of freedom but still hold (zero residual) are now kept and shown dimmed as redundant. Conflicting ones are still refused. A redundant driving dimension is refused with "already determined by ...". Typed sizes become driven (reference) dimensions. Drawing no longer adds Horizontal/Vertical to a side that is already implied through two coincident corners. The solver reports redundant constraints using a Jacobian row-dependence test.

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

crates/engine/src/cmd/sketch.rs

@@ -106,7 +106,7 @@ pub static COMMANDS: &[CommandSpec] = &[
         .params(concat!("p0, p1: overall ends, width", connect_doc!(), created_doc!())),
     CommandSpec::new("sketch.point", "Point", draw_point).at("SKETCH", "CREATE").icon("point").enabled(in_sketch).params(concat!("point: [x,y], id?", connect_doc!(), created_doc!())),
     CommandSpec::new("sketch.dimension", "Sketch Dimension", dimension).at("SKETCH", "CREATE").icon("dimension").key("D").enabled(in_sketch).params(
-        "entities: [refs], type?: auto|distance|horizontal|vertical|length|radius|diameter|angle, value?: number or expression (default: current; a bare number is in the design's units, e.g. 2 means 2 in in an inch design; give a unit such as \"50 mm\" to override); text_at?: [x, y]. Two points or a line off the axes: with auto, text_at across from them is the distance between them, above or below them horizontal, beside them vertical. Two lines: the angle between them (acute, or 180° − it when the value is nearer that) unless text_at picks a sector. Returns measured in unit (mm or deg)",
+        "entities: [refs], type?: auto|distance|horizontal|vertical|length|radius|diameter|angle, value?: number or expression (default: current; a bare number is in the design's units, e.g. 2 means 2 in in an inch design; give a unit such as \"50 mm\" to override); text_at?: [x, y]. Two points or a line off the axes: with auto, text_at across from them is the distance between them, above or below them horizontal, beside them vertical. Two lines: the angle between them (acute, or 180° − it when the value is nearer that) unless text_at picks a sector. A dimension the sketch already determines is refused (naming what determines it); if_redundant?: \"driven\" places it as a driven (reference) dimension instead. Returns measured in unit (mm or deg)",
     ),
     CommandSpec::new("sketch.constraint.horizontal_vertical", "Horizontal/Vertical", c_horizontal_vertical)
         .at("SKETCH", "CONSTRAINTS")
@@ -401,6 +401,7 @@ pub(super) fn draw(
         let first = sk.points.len();
         let (curves, mut cons) = f(sk, doc)?;
         cons.extend(connect_points(sk, first, p)?);
+        drop_implied_axes(sk, &mut cons);
         Ok((curves, cons))
     })
 }
@@ -821,6 +822,74 @@ fn rect_constraints(sk: &mut Sketch, l: &[usize]) -> Result<Vec<String>> {
     Ok(cons)
 }
 
+/// Whether line `l` being horizontal (`horizontal`) or vertical already follows from another
+/// constraint than `own`: its ends coincide with the ends of a line constrained the same way (a
+/// side two rectangles share), or with two points constrained level (or plumb).
+fn axis_implied(sk: &Sketch, l: usize, horizontal: bool, own: &str) -> bool {
+    let Some(CurveKind::Line { a, b }) = sk.curves.get(l).map(|c| &c.kind) else { return false };
+    let (a, b) = (*a, *b);
+    // Coincident points as one (union-find over the coincident constraints).
+    let mut parent: Vec<usize> = (0..sk.points.len()).collect();
+    let root = |parent: &[usize], mut i: usize| {
+        for _ in 0..parent.len() {
+            match parent.get(i) {
+                Some(&up) if up != i => i = up,
+                _ => break,
+            }
+        }
+        i
+    };
+    for c in &sk.constraints {
+        if let ConstraintKind::Coincident { p, q } = c.kind {
+            let (rp, rq) = (root(&parent, p), root(&parent, q));
+            if rp != rq
+                && let Some(slot) = parent.get_mut(rp)
+            {
+                *slot = rq;
+            }
+        }
+    }
+    let (ra, rb) = (root(&parent, a), root(&parent, b));
+    sk.constraints.iter().filter(|c| c.id != own).any(|c| {
+        let ends = match (&c.kind, horizontal) {
+            (ConstraintKind::Horizontal { l: m }, true) | (ConstraintKind::Vertical { l: m }, false) if *m != l => {
+                match sk.curves.get(*m).map(|c| &c.kind) {
+                    Some(CurveKind::Line { a, b }) => Some((*a, *b)),
+                    _ => None,
+                }
+            }
+            (ConstraintKind::HorizontalPoints { p, q }, true) | (ConstraintKind::VerticalPoints { p, q }, false) => Some((*p, *q)),
+            _ => None,
+        };
+        ends.is_some_and(|(p, q)| {
+            let (rp, rq) = (root(&parent, p), root(&parent, q));
+            rp != rq && ((rp, rq) == (ra, rb) || (rp, rq) == (rb, ra))
+        })
+    })
+}
+
+/// Drawing adds horizontal/vertical only where it is not already implied: a rectangle drawn
+/// onto another's side shares that side's direction through the two coincident corners, so a
+/// second "vertical" there would only be redundant. Drops such constraints among `cons` (the
+/// drawing's own) from the sketch and the list.
+fn drop_implied_axes(sk: &mut Sketch, cons: &mut Vec<String>) {
+    let mut dropped = Vec::new();
+    for id in cons.iter() {
+        let axis = sk.constraints.iter().find(|c| &c.id == id).and_then(|c| match c.kind {
+            ConstraintKind::Horizontal { l } => Some((l, true)),
+            ConstraintKind::Vertical { l } => Some((l, false)),
+            _ => None,
+        });
+        if let Some((l, horizontal)) = axis
+            && axis_implied(sk, l, horizontal, id)
+        {
+            sk.constraints.retain(|c| &c.id != id);
+            dropped.push(id.clone());
+        }
+    }
+    cons.retain(|id| !dropped.contains(id));
+}
+
 /// Which side and corner of an axis-aligned rectangle is which, from its lines and corner
 /// points as `polygon_lines` made them (counter-clockwise from the bottom-left corner), so a
 /// caller can dimension it without inspecting the sketch first.
@@ -855,6 +924,7 @@ fn rect_two_point(s: &mut Session, p: &Value) -> Result<Value> {
         let mut cons = rect_constraints(sk, &l)?;
         mark_construction(sk, &l, p);
         cons.extend(connect_points(sk, first, p)?);
+        drop_implied_axes(sk, &mut cons);
         Ok(((ids_of(sk, &l), cons), rect_names(sk, &l, &pts)))
     })?;
     Ok(rect_result(out, names, info))
@@ -887,6 +957,7 @@ fn rect_center(s: &mut Session, p: &Value) -> Result<Value> {
         }
         mark_construction(sk, &l, p);
         cons.extend(connect_points(sk, first, p)?);
+        drop_implied_axes(sk, &mut cons);
         Ok(((ids_of(sk, &l), cons), rect_names(sk, &l, &pts)))
     })?;
     Ok(rect_result(out, names, info))
@@ -1174,19 +1245,24 @@ fn draw_point(s: &mut Session, p: &Value) -> Result<Value> {
 
 fn constrain(s: &mut Session, p: &Value, cmd: &str, f: impl FnOnce(&mut Sketch) -> Result<ConstraintKind>) -> Result<Value> {
     let mut before = None;
+    let mut residual = None;
     let (id, info) = edit(s, p, cmd, true, |sk, _| {
         let k = f(sk)?;
         let rep = solve(&mut sk.clone());
         before = Some(rep.dof);
         let order = entity_order(sk, p);
         let id = add_c(sk, k)?;
-        if let Some((first, second)) = order {
+        residual = residual_of(sk, &id);
+        // A constraint that already holds needs nothing moved (even when both entities are
+        // fully determined: it is then redundant, see `redundancy`).
+        let holds = residual.is_some_and(|r| r <= solidcraft_sketch::CONSISTENT);
+        if let Some((first, second)) = order.filter(|_| !holds) {
             apply_in_order(sk, &rep, &[first], second, cmd)?;
         }
         Ok(id)
     })?;
-    reject_redundant(before, &info, cmd)?;
-    Ok(json!({"constraint": id, "sketch": info}))
+    let by = redundancy(s, p, cmd, before, &info, residual, &id)?;
+    constraint_result(p, cmd, id, info, by, Value::Null)
 }
 
 /// A sketch entity a constraint is applied to.
@@ -1330,18 +1406,79 @@ pub(super) fn dof_now(sk: &Sketch) -> usize {
     solve(&mut sk.clone()).dof
 }
 
-/// A constraint that removes no degree of freedom over-constrains the sketch (Fusion refuses
-/// it); the caller's edit is rolled back by returning the error.
-pub(super) fn reject_redundant(before: Option<usize>, info: &Value, cmd: &str) -> Result<()> {
+/// A constraint just added that removes no degree of freedom (`before` → the edit's `info`) is
+/// redundant or a conflict. Redundant when it held where the sketch was before it was added
+/// (`residual`, see [`solidcraft_sketch::constraint_residual`]): what it asks for is already
+/// implied by other constraints, and the solver satisfies it with them. Returns the
+/// constraints that determine it (as "id (Name)"), or `None` when it removes a degree of
+/// freedom. A conflict is refused; the caller's edit is rolled back by returning the error.
+pub(super) fn redundancy(
+    s: &Session,
+    p: &Value,
+    cmd: &str,
+    before: Option<usize>,
+    info: &Value,
+    residual: Option<f64>,
+    cid: &str,
+) -> Result<Option<Vec<String>>> {
     let after = info.get("dof").and_then(Value::as_u64).map(|d| d as usize);
-    if let (Some(b), Some(a)) = (before, after)
-        && a >= b
-    {
-        return Err(bad(cmd, "that would over-constrain the sketch (it is already determined by other constraints)"));
+    let (Some(b), Some(a)) = (before, after) else { return Ok(None) };
+    if a < b {
+        return Ok(None);
+    }
+    if !residual.is_some_and(|r| r <= solidcraft_sketch::CONSISTENT) {
+        return Err(bad(cmd, "that would over-constrain the sketch (it conflicts with what the other constraints already determine)"));
+    }
+    Ok(Some(determined_by(s, p, cmd, cid)))
+}
+
+/// The constraints that already determine constraint `cid` of the target sketch, as
+/// "id (Name)".
+fn determined_by(s: &Session, p: &Value, cmd: &str, cid: &str) -> Vec<String> {
+    let Some(sk) = target_sketch(s, p, cmd).ok().and_then(|id| s.doc.sketch(id).ok()) else { return Vec::new() };
+    let Some(ci) = sk.constraints.iter().position(|c| c.id == cid) else { return Vec::new() };
+    solidcraft_sketch::determined_by(sk, ci)
+        .into_iter()
+        .filter_map(|k| sk.constraints.get(k))
+        .map(|c| format!("{} ({})", c.id, c.kind.name()))
+        .collect()
+}
+
+/// "k3 (Vertical), k9 (Vertical)", or "other constraints" when they are not known.
+pub(super) fn listed(by: &[String]) -> String {
+    if by.is_empty() { "other constraints".to_string() } else { by.join(", ") }
+}
+
+/// A redundant constraint is kept (and shown as redundant) unless the caller asked for
+/// `if_redundant: "refuse"` (drawing-time inferences, which only add what is not already there).
+pub(super) fn keep_redundant(p: &Value, cmd: &str, by: &[String]) -> Result<()> {
+    if str_(p, "if_redundant").is_some_and(|m| m.eq_ignore_ascii_case("refuse")) {
+        return Err(bad(cmd, format!("that would over-constrain the sketch (it is already determined by {})", listed(by))));
     }
     Ok(())
 }
 
+/// The result of a constraint command: the constraint, and whether it is redundant (and what
+/// determines it).
+pub(super) fn constraint_result(p: &Value, cmd: &str, id: String, info: Value, by: Option<Vec<String>>, extra: Value) -> Result<Value> {
+    let mut out = json!({"constraint": id, "sketch": info});
+    if let Some(by) = by {
+        keep_redundant(p, cmd, &by)?;
+        out["redundant"] = json!(true);
+        out["determined_by"] = json!(by);
+    }
+    if let (Some(o), Value::Object(e)) = (out.as_object_mut(), extra) {
+        o.extend(e);
+    }
+    Ok(out)
+}
+
+/// The residual of the constraint `id` where the sketch is now (see
+/// [`solidcraft_sketch::constraint_residual`]).
+pub(super) fn residual_of(sk: &Sketch, id: &str) -> Option<f64> {
+    sk.constraints.iter().position(|c| c.id == id).and_then(|ci| solidcraft_sketch::constraint_residual(sk, ci))
+}
+
 fn c_horizontal_vertical(s: &mut Session, p: &Value) -> Result<Value> {
     let cmd = "sketch.constraint.horizontal_vertical";
     let mode = str_(p, "mode").map(str::to_ascii_lowercase);
@@ -1610,6 +1747,8 @@ fn dimension(s: &mut Session, p: &Value) -> Result<Value> {
     let text_at = p.get("text_at").and_then(vec2);
     let driven = bool_(p, "driven").unwrap_or(false);
     let mut before = None;
+    let (mut residual, mut new_id) = (None, String::new());
+    let start = s.doc.clone();
     let ((param, kind_name, current), info) = edit(s, p, cmd, true, |sk, doc| {
         before = Some(dof_now(sk));
         let ent = |r: &str| -> Result<(Option<usize>, Option<usize>)> {
@@ -1766,8 +1905,13 @@ fn dimension(s: &mut Session, p: &Value) -> Result<Value> {
         {
             (k, cur) = (ka, ca);
         }
+        // How far the value is from what the geometry measures now: zero for a dimension that
+        // only restates what the sketch already has.
+        let want = vals.get(&pname).and_then(|v| v.to_kind(if is_angle { Kind::Angle } else { Kind::Length }).ok());
+        residual = want.map(|w| (w - cur).abs()).filter(|r| r.is_finite());
         let name = k.name();
         let id = sk.add_constraint(k, Some(pname.clone()))?;
+        new_id = id.clone();
         place_text(sk, &id, text_at);
         Ok((pname, name, measured(is_angle, cur)))
     })?;
@@ -1776,8 +1920,28 @@ fn dimension(s: &mut Session, p: &Value) -> Result<Value> {
     if driven {
         return Ok(json!({"param": Value::Null, "driven": true, "type": kind_name, "measured": current.0, "unit": current.1, "sketch": info}));
     }
-    if reject_redundant(before, &info, cmd).is_err() {
-        return Err(bad(cmd, "that dimension would over-constrain the sketch; add it as a driven (reference) dimension with driven: true"));
+    // A dimension the sketch already determines cannot drive it: it is refused, or with
+    // `if_redundant: "driven"` placed as a driven (reference) dimension instead.
+    let by = redundancy(s, p, cmd, before, &info, residual, &new_id)
+        .map_err(|_| bad(cmd, "that dimension conflicts with the sketch: other constraints already determine it with a different value"))?;
+    if let Some(by) = by {
+        if str_(p, "if_redundant").is_some_and(|m| m.eq_ignore_ascii_case("driven")) {
+            s.doc = start;
+            let mut q = p.clone();
+            q["driven"] = json!(true);
+            if let Some(o) = q.as_object_mut() {
+                o.remove("if_redundant");
+                o.remove("value");
+            }
+            let mut out = dimension(s, &q)?;
+            out["redundant"] = json!(true);
+            out["determined_by"] = json!(by);
+            return Ok(out);
+        }
+        return Err(bad(
+            cmd,
+            format!("that dimension is already determined by {}; add it as a driven (reference) dimension with driven: true", listed(&by)),
+        ));
     }
     let v = s.doc.param(&param).map(|p| p.expr.clone()).unwrap_or_default();
     Ok(json!({"param": param, "type": kind_name, "expression": v, "measured": current.0, "unit": current.1, "sketch": info}))
@@ -2156,3 +2320,6 @@ mod drag_live_tests;
 #[cfg(test)]
 #[path = "sketch_ids_tests.rs"]
 mod ids_tests;
+#[cfg(test)]
+#[path = "sketch_redundant_tests.rs"]
+mod redundant_tests;