fix(constraints): skipped conflict probes keep the allowance

Cargo.toml

@@ -37,7 +37,7 @@ clap_mangen = { version = "0.3.3", features = ["env"] }
 thiserror = "2"
 same-file = "1"
 serde = { version = "1", features = ["derive", "rc"] }
-serde_json = { version = "1", features = ["float_roundtrip", "raw_value"] }
+serde_json = { version = "1", features = ["float_roundtrip"] }
 log = "0.4"
 # All date/time conversion goes through chrono (no hand-rolled calendar code beyond the 400-year range extension; AGENTS.md).
 chrono = { version = "0.4.45", default-features = false, features = ["std"] }

apps/cadcraft/src/control_server.rs

@@ -221,17 +221,12 @@ fn read_bounded_line(reader: &mut impl BufRead, buf: &mut Vec<u8>, max: usize, h
     }
 }
 
-/// Longest request `id` (it is copied into the reply).
-const MAX_ID_BYTES: usize = 4096;
-
 /// One request line, parsed.
-enum Line<'a> {
+enum Line {
     Request {
         id: Value,
         method: String,
-        /// Left as text until the token is checked: decoding it first would let an unauthenticated
-        /// peer turn a flat array into many owned JSON values.
-        params: Option<&'a serde_json::value::RawValue>,
+        params: Value,
         token: Option<String>,
     },
     Blank,
@@ -240,7 +235,7 @@ enum Line<'a> {
 }
 
 /// Parse a UTF-8 JSON request and its optional token, distinguishing blank lines from rejected input.
-fn parse_line(bytes: &[u8]) -> Line<'_> {
+fn parse_line(bytes: &[u8]) -> Line {
     let Ok(text) = std::str::from_utf8(bytes) else {
         return Line::Reject("not a control request (invalid UTF-8); closing the connection".into());
     };
@@ -251,28 +246,23 @@ fn parse_line(bytes: &[u8]) -> Line<'_> {
     if is_http_request_line(text) {
         return Line::Reject("HTTP is not served here: this port takes JSON lines; closing the connection".into());
     }
-    // Checked without building values; then only the object's own keys are read, each value kept as text.
-    if let Err(e) = serde_json::from_str::<&serde_json::value::RawValue>(text) {
-        return Line::Reject(format!("bad JSON ({e}); expected one JSON request object per line; closing the connection"));
-    }
-    let Ok(mut obj) = serde_json::from_str::<std::collections::BTreeMap<String, &serde_json::value::RawValue>>(text) else {
+    let msg = match serde_json::from_str::<Value>(text) {
+        Ok(v) => v,
+        Err(e) => return Line::Reject(format!("bad JSON ({e}); expected one JSON request object per line; closing the connection")),
+    };
+    let Value::Object(mut obj) = msg else {
         return Line::Reject("not a control request (expected a JSON object); closing the connection".into());
     };
-    // `method` and `token` are decoded as strings only (anything else is no string); `id` is echoed back
-    // in replies, so it is decoded but bounded.
-    let string = |v: Option<&serde_json::value::RawValue>| v.and_then(|r| serde_json::from_str::<String>(r.get()).ok());
-    let Some(method) = string(obj.remove("method")) else {
+    let Some(Value::String(method)) = obj.remove("method") else {
         return Line::Reject("not a control request (missing string `method`); closing the connection".into());
     };
-    let id = match obj.remove("id") {
-        None => Value::Null,
-        Some(r) if r.get().len() <= MAX_ID_BYTES => match serde_json::from_str(r.get()) {
-            Ok(v) => v,
-            Err(e) => return Line::Reject(format!("bad `id` ({e}); closing the connection")),
-        },
-        Some(_) => return Line::Reject(format!("`id` is longer than {MAX_ID_BYTES} bytes; closing the connection")),
-    };
-    Line::Request { id, method, params: obj.remove("params"), token: string(obj.remove("token")) }
+    // Moved out of the parsed object, not cloned: `params` can be most of the line.
+    Line::Request {
+        id: obj.remove("id").unwrap_or(Value::Null),
+        method,
+        params: obj.remove("params").unwrap_or(json!({})),
+        token: obj.remove("token").and_then(|t| if let Value::String(t) = t { Some(t) } else { None }),
+    }
 }
 
 /// An HTTP/1 request line (`POST / HTTP/1.1`) or the HTTP/2 preface (`PRI * HTTP/2.0`).
@@ -315,16 +305,6 @@ fn serve(stream: TcpStream, tx: &SyncSender<ControlRequest>, ctx: &egui::Context
                         let _ = writeln!(out, "{r}");
                         break;
                     }
-                    // Absent `params` is `{}`; ones that cannot be decoded (an unpaired surrogate escape) are an
-                    // error, not a request run with defaults.
-                    let params = match params.map(|r| serde_json::from_str::<Value>(r.get())) {
-                        None => json!({}),
-                        Some(Ok(v)) => v,
-                        Some(Err(e)) => {
-                            let _ = writeln!(out, "{}", json!({"ok": false, "error": format!("bad params ({e}); closing the connection"), "id": id}));
-                            break;
-                        }
-                    };
                     let (req, pending) = ControlRequest::new(method, params);
                     let mut r = match tx.try_send(req) {
                         Ok(()) => {
@@ -392,22 +372,6 @@ mod tests {
         assert_eq!(input.len(), 100 - 11, "the rest of the line is left unread");
     }
 
-    /// The parameters stay text until the token is checked, and a request decodes as before.
-    #[test]
-    fn params_are_decoded_only_as_text_by_the_parser() {
-        let line = br#"{"id":7,"method":"m","token":"t","params":[1,2,{"a":[3]}],"extra":null}"#;
-        let Line::Request { id, method, params, token } = parse_line(line) else { panic!("a request") };
-        assert_eq!((id, method.as_str(), token.as_deref()), (json!(7), "m", Some("t")));
-        assert_eq!(params.map(|p| p.get()), Some(r#"[1,2,{"a":[3]}]"#));
-        assert!(matches!(parse_line(b"[1,2,3]"), Line::Reject(e) if e.contains("expected a JSON object")));
-        assert!(matches!(parse_line(b"{\"method\":"), Line::Reject(e) if e.contains("bad JSON")));
-        assert!(matches!(parse_line(br#"{"id":[1],"method":5}"#), Line::Reject(e) if e.contains("missing string `method`")));
-        let long_id = format!(r#"{{"id":"{}","method":"m"}}"#, "x".repeat(MAX_ID_BYTES));
-        assert!(matches!(parse_line(long_id.as_bytes()), Line::Reject(e) if e.contains("`id` is longer")));
-        assert!(matches!(parse_line(br#"{"id":"\ud800","method":"m"}"#), Line::Reject(e) if e.contains("bad `id`")));
-        assert!(matches!(parse_line(br#"{"id":1}"#), Line::Reject(e) if e.contains("missing string `method`")));
-    }
-
     fn held() -> Held {
         Held::new(Budget::new(usize::MAX))
     }

crates/constraints/src/auto.rs

@@ -8,6 +8,9 @@ use cadcraft_geom::{Line, Vec2};
 pub const MAX_OBJECTS: usize = 200;
 /// Most candidates returned.
 pub const MAX_CANDIDATES: usize = 2000;
+/// Residual-evaluation work all the solves of one AUTOCONSTRAIN may make together (four
+/// full-size solves; a few thousand small ones).
+const MAX_AUTO_WORK: f64 = 4.0 * crate::MAX_EVAL_WORK;
 
 /// Inference options.
 #[derive(Clone, Debug)]
@@ -309,11 +312,22 @@ fn on_arc(d: &Drawing, h: Handle, c: Vec2, p: Vec2) -> bool {
 /// Infer and apply constraints over `handles`; returns the ids added. Redundant or conflicting
 /// candidates are skipped.
 pub fn autoconstrain(d: &mut Drawing, handles: &[Handle], o: &InferOptions) -> Vec<u32> {
+    autoconstrain_report(d, handles, o).0
+}
+
+/// [`autoconstrain`], and whether it stopped early because the solver-work allowance of the command
+/// was spent (the remaining candidates were not tried).
+pub fn autoconstrain_report(d: &mut Drawing, handles: &[Handle], o: &InferOptions) -> (Vec<u32>, bool) {
     let mut added = Vec::new();
+    // One allowance of solver work for all the candidates: each is a full solve of its component.
+    let mut work = MAX_AUTO_WORK;
     for c in infer(d, handles, o) {
-        if let Ok(id) = crate::add(d, c, &crate::SolveOptions::default()) {
-            added.push(id);
+        match crate::add_within(d, c, &crate::SolveOptions::default(), &mut work) {
+            Ok(id) => added.push(id),
+            // The allowance is spent: what was added stays, the remaining candidates are not tried.
+            Err(crate::SolveError::TooLarge) if work < 0.0 => return (added, true),
+            Err(_) => {}
         }
     }
-    added
+    (added, false)
 }

crates/constraints/src/lib.rs

@@ -36,7 +36,7 @@ use cadcraft_doc::{Constraint, ConstraintKind, Drawing, GeomRef, Handle};
 use cadcraft_geom::Vec2;
 use serde::Serialize;
 
-pub use auto::{INFER_TYPES, InferOptions, autoconstrain, infer, infer_placed};
+pub use auto::{INFER_TYPES, InferOptions, autoconstrain, autoconstrain_report, infer, infer_placed};
 pub use dof::{DofReport, DofStatus, ObjectDof, dof};
 pub use expr::{Env, ExprError};
 pub use model::{ref_points, rigid_points, supported};
@@ -53,6 +53,14 @@ const MAX_ITER: usize = 200;
 /// Largest residual-evaluation work (including spline rebuilds) one numerical Jacobian may take,
 /// shared by solving, redundancy checks and DOF analysis.
 pub(crate) const MAX_EVAL_WORK: f64 = 2e9;
+/// Most multiply-adds (rows x columns squared) of the dense rank factorisations one Fix makes:
+/// about a thousand parameters locked at once is 1e9; 2e10 is a few seconds.
+const MAX_QR_WORK: f64 = 2e10;
+
+/// Whether a dense rank factorisation of a `rows` x `cols` matrix passes [`MAX_QR_WORK`].
+fn qr_too_large(rows: usize, cols: usize) -> bool {
+    (rows as f64) * (cols as f64) * (cols as f64) > MAX_QR_WORK
+}
 /// Residual evaluations a solver iteration makes besides its Jacobian (damped trial steps).
 const TRIAL_EVALS: f64 = 14.0;
 
@@ -335,8 +343,14 @@ impl Comp {
     }
     /// Constraint ids in conflict (each one whose removal makes the rest solvable).
     fn conflicts(&self, x_failed: &[f64]) -> Vec<u32> {
+        self.conflicts_probing(x_failed, true)
+    }
+
+    /// [`Comp::conflicts`]; `probe` false skips the re-solves that look for the constraints whose
+    /// removal lets the rest converge.
+    fn conflicts_probing(&self, x_failed: &[f64], probe: bool) -> Vec<u32> {
         let mut ids = Vec::new();
-        if self.built.len() <= MAX_CONFLICT_PROBES {
+        if probe && self.built.len() <= MAX_CONFLICT_PROBES {
             for i in 0..self.built.len() {
                 if let Ok(o) = self.run(Some(i))
                     && o.converged
@@ -741,7 +755,14 @@ pub fn purge_invalid(d: &mut Drawing) -> Vec<u32> {
 /// Add a constraint: assigns its id (and name/expression for dimensional constraints when
 /// empty), refuses redundant or conflicting constraints, solves and writes the geometry back.
 /// On error the drawing is unchanged.
-pub fn add(d: &mut Drawing, mut c: Constraint, opts: &SolveOptions) -> Result<u32, SolveError> {
+pub fn add(d: &mut Drawing, c: Constraint, opts: &SolveOptions) -> Result<u32, SolveError> {
+    let mut unlimited = f64::INFINITY;
+    add_within(d, c, opts, &mut unlimited)
+}
+
+/// [`add`] charging the solve's residual-evaluation work to `work`: `TooLarge` (nothing changed)
+/// once it would take more than is left.
+pub(crate) fn add_within(d: &mut Drawing, mut c: Constraint, opts: &SolveOptions, work: &mut f64) -> Result<u32, SolveError> {
     if d.constraints.len() >= MAX_CONSTRAINTS {
         return Err(SolveError::Other("too many constraints".into()));
     }
@@ -777,10 +798,24 @@ pub fn add(d: &mut Drawing, mut c: Constraint, opts: &SolveOptions) -> Result<u3
     let Some(comp) = comps.into_iter().find(|k| k.built.iter().any(|b| b.id == id)) else {
         return Err(SolveError::Invalid { id, msg: "could not be interpreted".into() });
     };
+    *work -= comp.eval_work(&comp.free(None), None);
+    if *work < 0.0 {
+        return Err(SolveError::TooLarge);
+    }
     let o = comp.run(None)?;
     if !o.converged {
         // The new constraint first, then the existing ones it fights with.
-        let mut ids = comp.conflicts(&o.x);
+        // Each probe is another solve (up to one per constraint): they are charged too, and skipped
+        // when the allowance cannot pay for them.
+        let probes = if comp.built.len() <= MAX_CONFLICT_PROBES { comp.built.len() as f64 } else { 0.0 };
+        let cost = probes * comp.eval_work(&comp.free(None), None);
+        // Paid for before it is spent: when the allowance cannot cover the probes they are skipped and
+        // it is left as it was for the candidates still to try.
+        let affordable = cost <= *work;
+        if affordable {
+            *work -= cost;
+        }
+        let mut ids = comp.conflicts_probing(&o.x, affordable);
         ids.retain(|i| *i != id);
         ids.insert(0, id);
         return Err(SolveError::Conflict { ids });
@@ -819,6 +854,10 @@ pub fn add(d: &mut Drawing, mut c: Constraint, opts: &SolveOptions) -> Result<u3
         let locked: Vec<usize> = before.iter().enumerate().filter(|(_, j)| !after.contains(j)).map(|(k, _)| k).collect();
         let rows_old = comp.rows(Some(pos));
         let nf = before.len();
+        // The synthetic lock rows enlarge the matrix after the solver's own limits were checked.
+        if qr_too_large(rows_old + locked.len(), nf) {
+            return Err(SolveError::TooLarge);
+        }
         let f = |x: &[f64], out: &mut Vec<f64>| comp.eval(x, out, Some(pos));
         let mut jac = lm::jacobian(&f, &o.x, &before, rows_old);
         let r_old = lm::rank_qr(&jac, rows_old, nf);

crates/constraints/src/tests.rs

@@ -516,3 +516,41 @@ fn next_name_skips_names_in_use() {
     });
     assert_eq!(next_name(&d, ConstraintKind::Distance(DistAxis::Aligned)), "d2");
 }
+
+#[test]
+fn a_fix_over_a_huge_component_is_refused_before_its_dense_factorisation() {
+    assert!(!super::qr_too_large(1_000, 1_000));
+    assert!(super::qr_too_large(3_000, 3_000));
+    assert!(super::qr_too_large(usize::MAX, 2));
+}
+
+#[test]
+fn a_spent_work_allowance_refuses_a_constraint_and_leaves_the_drawing() {
+    let mut d = Drawing::default();
+    let l = line(&mut d, (0.0, 0.0), (10.0, 0.01));
+    let before = d.clone();
+    let c = con(ConstraintKind::Horizontal, &[w(l)]);
+    assert!(matches!(add_within(&mut d, c.clone(), &SolveOptions::default(), &mut 0.0), Err(SolveError::TooLarge)));
+    assert_eq!(d, before);
+    let mut plenty = 1e9;
+    assert!(add_within(&mut d, c, &SolveOptions::default(), &mut plenty).is_ok());
+    assert!(plenty < 1e9, "the solve was charged");
+    let (_, stopped) = autoconstrain_report(&mut d, &[l], &InferOptions::default());
+    assert!(!stopped, "a small command never spends its allowance");
+}
+
+#[test]
+fn skipped_conflict_probes_do_not_spend_the_allowance() {
+    let mut d = Drawing::default();
+    let b = line(&mut d, (0.0, 0.0), (3.0, 4.0));
+    ok(&mut d, con(ConstraintKind::Fix, &[w(b)]));
+    let candidate = || con(ConstraintKind::Vertical, &[w(b)]);
+    // The whole cost of the conflicting candidate: its solve and its probes.
+    let mut rich = 1e12;
+    assert!(matches!(add_within(&mut d.clone(), candidate(), &SolveOptions::default(), &mut rich), Err(SolveError::Conflict { .. })));
+    let total = 1e12 - rich;
+    // Enough for the solve but not the probes: they are skipped, and what is left stays positive.
+    let mut work = total * 0.6;
+    assert!(matches!(add_within(&mut d, candidate(), &SolveOptions::default(), &mut work), Err(SolveError::Conflict { .. })));
+    assert!(work > 0.0 && work < total * 0.6, "{work} of {total}");
+}

crates/engine/src/cmd/constraints.rs

@@ -821,11 +821,15 @@ fn run_autoconstrain(s: &mut Session, p: &Value) -> Result<Value> {
         o.types = t.iter().filter_map(Value::as_str).map(|n| kind_from_name(n).map_or_else(|| n.to_string(), |k| k.name().to_string())).collect();
     }
     let mut d = s.doc()?.clone();
-    let ids = cs::autoconstrain(&mut d, &hs, &o);
+    let (ids, stopped) = cs::autoconstrain_report(&mut d, &hs, &o);
     if !ids.is_empty() {
         *s.doc_mut()? = d;
     }
-    Ok(json!({ "added": ids.len(), "ids": ids, "message": format!("{} constraint(s) applied to {} object(s)", ids.len(), hs.len()) }))
+    let mut message = format!("{} constraint(s) applied to {} object(s)", ids.len(), hs.len());
+    if stopped {
+        message.push_str("; stopped early: the rest need more solver work than one command may spend, so run AUTOCONSTRAIN on fewer objects");
+    }
+    Ok(json!({ "added": ids.len(), "ids": ids, "stopped": stopped, "message": message }))
 }
 
 fn run_delconstraint(s: &mut Session, p: &Value) -> Result<Value> {

crates/io/src/dxf_mline.rs

@@ -371,16 +371,7 @@ fn fill(store: &mut EntityStore, by_name: &HashMap<String, MLineStyle>) {
                 && let Some(st) = by_name.get(&key(&m.style))
                 && st.elements.len() == m.def.elements.len()
             {
-                // The description is the style table's; copying it into every multiline would multiply a long one.
-                m.def = MLineStyle {
-                    name: st.name.clone(),
-                    description: String::new(),
-                    elements: st.elements.clone(),
-                    start_caps: st.start_caps,
-                    end_caps: st.end_caps,
-                    joints: st.joints,
-                    fill: st.fill,
-                };
+                m.def = st.clone();
             }
         });
     }

crates/io/tests/it/mline_roundtrip.rs

@@ -59,8 +59,7 @@ fn mlines_and_styles_survive_save_and_reopen() {
     let back = cadcraft_io::read_dxf(saved.as_bytes()).unwrap();
     let [a, b] = mlines(&back).try_into().unwrap();
     // The geometry as drawn is the same, breaks included.
-    // The description lives in the style table (checked below), not copied into every multiline.
-    assert_eq!(a.def, MLineStyle { description: String::new(), ..wall() });
+    assert_eq!(a.def, wall());
     assert_eq!(
         (a.style.as_str(), a.justification, a.scale, a.closed, a.no_start_caps, a.no_end_caps),
         ("Wall", MLineJust::Zero, 2.0, false, false, true)
@@ -102,7 +101,7 @@ fn mlines_and_styles_survive_save_and_reopen() {
     // Saving again is stable.
     let again = cadcraft_io::read_dxf(cadcraft_io::write_dxf(&back).as_bytes()).unwrap();
     assert_eq!(mlines(&again).len(), 2);
-    assert_eq!(mlines(&again)[0].def, MLineStyle { description: String::new(), ..wall() });
+    assert_eq!(mlines(&again)[0].def, wall());
 }
 
 /// A multiline as another program writes it: style `Duo` (offsets 0.25 and -0.25, the first red)