From f0dda667bb3e505c75ea438eed41a25d62140c43 Mon Sep 17 00:00:00 2001 From: selune <35337584+0xlillith@users.noreply.github.com> Date: Mon, 12 Oct 2026 16:38:04 +0200 Subject: [PATCH] Report accepted Boolean retries on features and fuzz runs (#249) Co-authored-by: bherbruck Co-authored-by: Claude Opus 5.5 --- ROADMAP.md | 6 + apps/solvecraft-cli/src/fuzz.rs | 39 ++++ crates/doc/src/eval.rs | 41 +++- crates/engine/src/cmd/edit.rs | 1 + crates/engine/src/cmd/inspect.rs | 1 + crates/engine/src/fuzz.rs | 39 +++- crates/engine/src/tests.rs | 1 + .../engine/src/tests/boolean_diagnostics.rs | 52 +++++ crates/kernel/src/boolean_diagnostics.rs | 195 ++++++++++++++++++ crates/kernel/src/coplanar.rs | 11 +- crates/kernel/src/fuzz.rs | 19 +- crates/kernel/src/lib.rs | 2 + crates/kernel/src/ops.rs | 81 ++++++-- crates/kernel/src/tests.rs | 5 +- docs/robustness.md | 24 +++ 15 files changed, 491 insertions(+), 26 deletions(-) create mode 100644 crates/engine/src/tests/boolean_diagnostics.rs create mode 100644 crates/kernel/src/boolean_diagnostics.rs diff --git a/ROADMAP.md b/ROADMAP.md index 017b9183..596a0127 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -331,6 +331,12 @@ or edited values, including the Appearance panel. ## Kernel +- Successful Boolean retries report their strategy and numerical adjustments as + feature warnings and structured inspection data. Diagnostics survive cached + evaluation, undo/redo and cold reopening; the robustness runner counts retry + events by strategy and reports any bounded-log truncation. Existing geometry + acceptance checks remain in force. + - Boolean intersection-curve construction refuses point-only, collapsed and non-finite polylines without panicking. Direct constructor regressions retain valid open endpoints and closed spherical intersections; the original interactive trigger remains unidentified. diff --git a/apps/solvecraft-cli/src/fuzz.rs b/apps/solvecraft-cli/src/fuzz.rs index 7a985f64..0fb98fc3 100644 --- a/apps/solvecraft-cli/src/fuzz.rs +++ b/apps/solvecraft-cli/src/fuzz.rs @@ -33,6 +33,8 @@ pub fn one(args: &[String]) -> Result<(), String> { "steps": run.steps.iter().map(|s| s.kind).collect::>(), "unsupported": run.unsupported, "rejected": run.rejected, + "boolean_fallbacks": run.boolean_fallbacks, + "boolean_reports_truncated": run.boolean_reports_truncated, "panics": PANICS.swap(0, std::sync::atomic::Ordering::SeqCst), }); if run.outcome.is_bug() { @@ -179,9 +181,23 @@ fn summarise(results: &[Value], took: Duration) -> String { let mut boolean: BTreeMap = BTreeMap::new(); let mut at_step: BTreeMap = BTreeMap::new(); let mut unsupported: BTreeMap = BTreeMap::new(); + let mut fallbacks: BTreeMap = BTreeMap::new(); + let mut truncated = [0usize; 2]; let mut steps = 0; let (mut dp, mut bp) = (0, 0); for v in results { + for (i, group) in ["design", "boolean"].into_iter().enumerate() { + if v[group]["boolean_reports_truncated"].as_bool().unwrap_or(false) { + truncated[i] += 1; + } + if let Some(counts) = v[group]["boolean_fallbacks"].as_object() { + for (strategy, count) in counts { + if let Some(total) = fallbacks.entry(strategy.clone()).or_default().get_mut(i) { + *total = total.saturating_add(count.as_u64().unwrap_or(0)); + } + } + } + } dp += usize::from(v["design"]["panics"].as_u64().unwrap_or(0) > 0); bp += usize::from(v["boolean_panics"].as_u64().unwrap_or(0) > 0); let k = if v.get("timeout").is_some() { @@ -220,5 +236,28 @@ fn summarise(results: &[Value], took: Duration) -> String { for (k, c) in &boolean { s += &format!("- {k}: {}\n", pct(*c)); } + s += "\nSuccessful Boolean retries (events, including nested operations):\n"; + for (strategy, [designs, checks]) in &fallbacks { + s += &format!("- {strategy}: designs {designs}, boolean checks {checks}\n"); + } + s += &format!("- Capped reports: designs {}, boolean checks {}\n", truncated[0], truncated[1]); s } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn report_counts_retry_events_separately_and_marks_incomplete_logs() { + let results = [ + json!({"design":{"outcome":{"kind":"ok"},"boolean_fallbacks":{"shifted_operands":2}},"boolean":{"outcome":"ok","boolean_fallbacks":{"shifted_operands":3},"boolean_reports_truncated":true}}), + json!({"design":{"outcome":{"kind":"ok"},"boolean_fallbacks":{"shifted_operands":1}},"boolean":{"outcome":"ok"}}), + json!({"timeout":90}), + ]; + let text = summarise(&results, Duration::ZERO); + assert!(text.contains("shifted_operands: designs 3, boolean checks 3"), "{text}"); + assert!(text.contains("Capped reports: designs 0, boolean checks 1"), "{text}"); + assert!(text.contains("timeout: 1 (33.3 %)"), "{text}"); + } +} diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index fd76266a..c4fd05ce 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -352,6 +352,7 @@ pub struct EvalCache { struct CachedEval { error: Option, warning: Option, + boolean_diagnostics: kernel::BooleanDiagnostics, ms: f64, state: Arc, } @@ -395,6 +396,7 @@ pub struct FeatureResult { pub name: String, pub error: Option, pub warning: Option, + pub boolean_diagnostics: kernel::BooleanDiagnostics, pub ms: f64, pub skipped: bool, #[serde(skip)] @@ -635,6 +637,7 @@ impl Model { name: f.name.clone(), error: None, warning: None, + boolean_diagnostics: Default::default(), ms: 0.0, skipped: true, fingerprint: fp, @@ -656,6 +659,7 @@ impl Model { name: f.name.clone(), error: c.error, warning: c.warning, + boolean_diagnostics: c.boolean_diagnostics, ms: c.ms, skipped: false, fingerprint: fp, @@ -699,12 +703,22 @@ impl Model { }; // A parameter with an error fails the features using it, with that error. let bad_param = param_names(f).into_iter().find_map(|n| self.param_errors.get(&n).map(|e| format!("parameter `{n}`: {e}"))); - let r = match bad_param { + let (r, mut boolean_diagnostics) = kernel::capture_boolean_diagnostics(|| match bad_param { Some(e) => Err(kernel::KernelError::Failed(e)), None => kernel::guard(&f.name, || { eval_feature(doc, &vals, f, &mut next, &mut warning).map_err(|e| kernel::KernelError::Failed(e.to_string())) }), - }; + }); + if r.is_ok() { + if let Some(note) = boolean_diagnostics.warning() { + warning = Some(match warning { + Some(existing) => format!("{existing}; {note}"), + None => note, + }); + } + } else { + boolean_diagnostics = Default::default(); + } let ms = t0.elapsed().as_secs_f64() * 1000.0; let error = match r { Ok(()) => { @@ -730,9 +744,28 @@ impl Model { if !self.no_cache && let Ok(mut m) = self.cache.lock() { - m.put(key, CachedEval { error: error.clone(), warning: warning.clone(), ms, state: state.clone() }); + m.put( + key, + CachedEval { + error: error.clone(), + warning: warning.clone(), + boolean_diagnostics: boolean_diagnostics.clone(), + ms, + state: state.clone(), + }, + ); } - out.push(FeatureResult { id: f.id, name: f.name.clone(), error, warning, ms, skipped: false, fingerprint: fp, state: state.clone() }); + out.push(FeatureResult { + id: f.id, + name: f.name.clone(), + error, + warning, + boolean_diagnostics, + ms, + skipped: false, + fingerprint: fp, + state: state.clone(), + }); } self.results = out; self.last_recomputed = recomputed; diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index cbcfff75..74dc3379 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -261,6 +261,7 @@ fn inspect_feature(s: &mut Session, p: &Value) -> Result { "id": f.id, "name": f.name, "type": f.kind.type_name(), "suppressed": f.suppressed, "rolled_back": rolled_back, "definition": def, "omitted": omitted, "inputs": inputs, "error": r.and_then(|r| r.error.clone()), "warning": r.and_then(|r| r.warning.clone()), + "boolean_diagnostics": r.map(|r| &r.boolean_diagnostics), })) } diff --git a/crates/engine/src/cmd/inspect.rs b/crates/engine/src/cmd/inspect.rs index c83ae954..4cdd87fe 100644 --- a/crates/engine/src/cmd/inspect.rs +++ b/crates/engine/src/cmd/inspect.rs @@ -205,6 +205,7 @@ fn inspect(s: &mut Session, p: &Value) -> Result { json!({ "id": f.id, "name": f.name, "type": f.kind.type_name(), "suppressed": f.suppressed, "rolled_back": i >= marker, "error": r.and_then(|r| r.error.clone()), "warning": r.and_then(|r| r.warning.clone()), "ms": r.map(|r| r.ms), + "boolean_diagnostics": r.map(|r| &r.boolean_diagnostics), }) }) .collect(); diff --git a/crates/engine/src/fuzz.rs b/crates/engine/src/fuzz.rs index 6bbd870c..1e76388f 100644 --- a/crates/engine/src/fuzz.rs +++ b/crates/engine/src/fuzz.rs @@ -64,6 +64,8 @@ pub struct Run { pub unsupported: Vec<(String, String)>, /// Steps whose generated parameters a command refused. pub rejected: usize, + pub boolean_fallbacks: std::collections::BTreeMap, + pub boolean_reports_truncated: bool, } impl Run { @@ -289,7 +291,15 @@ pub fn run(seed: u64, max_steps: usize) -> Run { pub fn run_with(seed: u64, max_steps: usize, on_step: &mut dyn FnMut(&Step)) -> Run { let mut r = Rng::new(seed); let mut s = Session::default(); - let mut run = Run { seed, steps: Vec::new(), outcome: Outcome::ok(), unsupported: Vec::new(), rejected: 0 }; + let mut run = Run { + seed, + steps: Vec::new(), + outcome: Outcome::ok(), + unsupported: Vec::new(), + rejected: 0, + boolean_fallbacks: Default::default(), + boolean_reports_truncated: false, + }; let mut last_feature: Option = None; let n = 2 + r.below(max_steps.max(2) - 1); let mut tries = 0; @@ -314,6 +324,12 @@ pub fn run_with(seed: u64, max_steps: usize, on_step: &mut dyn FnMut(&Step)) -> continue; } let kind = st.kind; + for result in s.model.results.iter().skip(before).filter(|result| result.error.is_none()) { + run.boolean_reports_truncated |= result.boolean_diagnostics.truncated; + for event in &result.boolean_diagnostics.events { + *run.boolean_fallbacks.entry(event.strategy).or_default() += 1; + } + } run.steps.push(st); if out.kind != "ok" { run.outcome = out; @@ -329,14 +345,19 @@ pub fn run_with(seed: u64, max_steps: usize, on_step: &mut dyn FnMut(&Step)) -> /// Run fixed steps (a recorded or minimised design): the index and outcome of the first step /// that isn't ok (or the last step's, ok). pub fn replay(steps: &[Step]) -> (usize, Outcome) { + let (index, outcome, _) = replay_session(steps); + (index, outcome) +} + +fn replay_session(steps: &[Step]) -> (usize, Outcome, Session) { let mut s = Session::default(); for (i, st) in steps.iter().enumerate() { let out = apply(&mut s, st); if out.kind != "ok" { - return (i, out); + return (i, out, s); } } - (steps.len().saturating_sub(1), Outcome::ok()) + (steps.len().saturating_sub(1), Outcome::ok(), s) } /// The start of a message, to tell failures apart (numbers vary between versions of a case). @@ -367,6 +388,14 @@ pub fn minimise(run: &Run) -> Run { } } } - let (_, outcome) = replay(&steps); - Run { seed: run.seed, steps, outcome, unsupported: Vec::new(), rejected: 0 } + let (_, outcome, session) = replay_session(&steps); + let mut boolean_fallbacks = std::collections::BTreeMap::new(); + let mut boolean_reports_truncated = false; + for result in &session.model.results { + boolean_reports_truncated |= result.boolean_diagnostics.truncated; + for event in &result.boolean_diagnostics.events { + *boolean_fallbacks.entry(event.strategy).or_default() += 1; + } + } + Run { seed: run.seed, steps, outcome, unsupported: Vec::new(), rejected: 0, boolean_fallbacks, boolean_reports_truncated } } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 8cdc25a4..69b4f930 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -118,6 +118,7 @@ fn join_meeting_nothing_keeps_the_first_body() { assert!(rel(volume(&mut s), 1064.0) < 1e-6); } +mod boolean_diagnostics; mod component_frames; mod mirror_planes; mod primitive_points; diff --git a/crates/engine/src/tests/boolean_diagnostics.rs b/crates/engine/src/tests/boolean_diagnostics.rs new file mode 100644 index 00000000..eaa54288 --- /dev/null +++ b/crates/engine/src/tests/boolean_diagnostics.rs @@ -0,0 +1,52 @@ +use super::*; +use solvecraft_kernel::BooleanStrategy; + +#[test] +fn boolean_retry_warning_and_details_survive_cache_undo_and_reopen() { + let slope = 1f64.to_radians().tan(); + let mut session = Session::default(); + run(&mut session, "sketch.create", json!({"plane":"XZ","name":"Floor section"})); + run(&mut session, "sketch.line", json!({"points":[[0,0],[100,0],[100,-10.-slope*100.],[0,-10.]],"closed":true})); + run(&mut session, "sketch.finish", json!({})); + run(&mut session, "solid.extrude", json!({"sketch":"Floor section","distance":80,"body_name":"Floor"})); + run(&mut session, "solid.cylinder", json!({"base":[50,40,-5],"radius":10,"height":30,"operation":"cut"})); + run(&mut session, "sketch.create", json!({"plane":"XY","name":"Ring section"})); + run(&mut session, "sketch.circle.center", json!({"center":[50,40],"radius":20})); + run(&mut session, "sketch.circle.center", json!({"center":[50,40],"radius":10})); + run(&mut session, "sketch.finish", json!({})); + run( + &mut session, + "solid.extrude", + json!({"sketch":"Ring section","profiles":[{"point":[65,40]}],"start_offset":-5,"distance":7,"body_name":"Ring"}), + ); + let id = run(&mut session, "solid.combine", json!({"target":"Floor","tools":["Ring"],"operation":"join"}))["feature"].as_u64().unwrap(); + let check = |session: &Session| { + let result = session.model.result(id).unwrap(); + assert!(result.error.is_none()); + assert!(result.warning.as_ref().is_some_and(|warning| warning.contains("Boolean retry:")), "{result:?}"); + assert!( + result.boolean_diagnostics.events.iter().any(|event| event.strategy == BooleanStrategy::CommonBore), + "{:?}", + result.boolean_diagnostics + ); + result.boolean_diagnostics.clone() + }; + let original = check(&session); + let expected = (10. + slope * 50.) * (8000. - PI * 100.) + PI * 300. * 5.; + assert!(rel(volume(&mut session), expected) < 2e-5); + let inspected = run(&mut session, "timeline.inspect", json!({"feature":id})); + assert_eq!(inspected["boolean_diagnostics"], serde_json::to_value(&original).unwrap()); + session.refresh(); + assert_eq!(check(&session), original); + run(&mut session, "edit.undo", json!({})); + run(&mut session, "edit.redo", json!({})); + assert_eq!(check(&session), original); + let reopened_doc = solvecraft_doc::Document::from_json(&session.doc.to_json()).unwrap(); + let reopened = Session::new(reopened_doc); + assert_eq!(check(&reopened), original); + // A later independent feature must not inherit the earlier retry's warning. + run(&mut session, "solid.box", json!({"corner":[200,0,0],"length":5,"width":5,"height":5})); + let last = session.model.results.last().unwrap(); + assert!(last.warning.is_none()); + assert!(last.boolean_diagnostics.events.is_empty()); +} diff --git a/crates/kernel/src/boolean_diagnostics.rs b/crates/kernel/src/boolean_diagnostics.rs new file mode 100644 index 00000000..c33bb341 --- /dev/null +++ b/crates/kernel/src/boolean_diagnostics.rs @@ -0,0 +1,195 @@ +//! Scoped reports for successful Boolean retries. Discarded attempts never enter +//! their caller's report, and independent evaluations cannot inherit old events. + +use std::cell::RefCell; + +use serde::Serialize; + +const MAX_EVENTS: usize = 1024; + +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum BooleanStrategy { + RefinedTolerance, + ShiftedOperands, + CoplanarSeparation, + CommonBore, + PlanarFallback, + ToolNudge, + Piecewise, +} + +/// Numerical adjustments used by a successful retry, in millimetres/radians. +/// Operand shifts are reversed on the result; face shifts are temporary working +/// offsets. A tool nudge, in contrast, changes the relative operand placement. +#[derive(Clone, Debug, PartialEq, Serialize)] +pub struct BooleanFallback { + pub strategy: BooleanStrategy, + pub translation_mm: f64, + pub rotation_rad: f64, + pub face_shift_mm: f64, + pub tolerance_mm: f64, +} + +impl BooleanFallback { + pub(crate) fn new(strategy: BooleanStrategy) -> Self { + Self { strategy, translation_mm: 0., rotation_rad: 0., face_shift_mm: 0., tolerance_mm: 0. } + } + + fn description(&self) -> String { + match self.strategy { + BooleanStrategy::RefinedTolerance => format!("refined tolerance {:.6e} mm", self.tolerance_mm), + BooleanStrategy::ShiftedOperands => { + format!("shifted operands {:.6e} mm (result shifted back), tolerance {:.6e} mm", self.translation_mm, self.tolerance_mm) + } + BooleanStrategy::CoplanarSeparation => format!("temporary coincident-face separation up to {:.6e} mm per face", self.face_shift_mm), + BooleanStrategy::CommonBore => format!("temporary bore offset {:.6e} mm, followed by recutting the original bore", self.face_shift_mm), + BooleanStrategy::PlanarFallback => "exact planar fallback (no displacement)".into(), + BooleanStrategy::ToolNudge => format!("tool displacement {:.6e} mm, rotation {:.6e} rad", self.translation_mm, self.rotation_rad), + BooleanStrategy::Piecewise => "piecewise retry (no displacement)".into(), + } + } +} + +#[derive(Clone, Debug, Default, PartialEq, Serialize)] +pub struct BooleanDiagnostics { + pub events: Vec, + pub truncated: bool, +} + +impl BooleanDiagnostics { + pub(crate) fn push(&mut self, event: BooleanFallback) { + if self.events.len() < MAX_EVENTS { + self.events.push(event); + } else { + self.truncated = true; + } + } + + pub fn warning(&self) -> Option { + if self.events.is_empty() && !self.truncated { + return None; + } + let mut descriptions: Vec = self.events.iter().take(4).map(BooleanFallback::description).collect(); + if self.events.len() > 4 { + descriptions.push(format!("{} additional retries", self.events.len() - 4)); + } + if self.truncated { + descriptions.push(format!("report capped at {MAX_EVENTS} retries")); + } + Some(format!("Boolean retry: {}", descriptions.join("; "))) + } +} + +thread_local! { + static REPORT: RefCell> = const { RefCell::new(None) }; +} + +struct Scope { + previous: Option>, +} + +impl Drop for Scope { + fn drop(&mut self) { + if let Some(previous) = self.previous.take() { + REPORT.with(|cell| { + if let Ok(mut report) = cell.try_borrow_mut() { + *report = previous; + } + }); + } + } +} + +/// Capture only the events produced by this call. Nested callers explicitly merge +/// an accepted attempt; dropping a scope restores its parent even during unwinding. +pub fn capture_boolean_diagnostics(run: impl FnOnce() -> T) -> (T, BooleanDiagnostics) { + let previous = REPORT.with(|cell| cell.try_borrow_mut().ok().map(|mut report| report.replace(BooleanDiagnostics::default()))); + let scope = Scope { previous }; + let result = run(); + let report = REPORT.with(|cell| cell.try_borrow_mut().ok().and_then(|mut report| report.take())).unwrap_or_default(); + drop(scope); + (result, report) +} + +pub(crate) fn record(event: BooleanFallback) { + REPORT.with(|cell| { + if let Ok(mut report) = cell.try_borrow_mut() + && let Some(report) = report.as_mut() + { + report.push(event); + } + }); +} + +pub(crate) fn merge(report: BooleanDiagnostics) { + for event in report.events { + record(event); + } + if report.truncated { + REPORT.with(|cell| { + if let Ok(mut report) = cell.try_borrow_mut() + && let Some(report) = report.as_mut() + { + report.truncated = true; + } + }); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn nested_attempts_are_committed_explicitly_and_do_not_leak() { + let (_, report) = capture_boolean_diagnostics(|| { + let (_, discarded) = capture_boolean_diagnostics(|| record(BooleanFallback::new(BooleanStrategy::ToolNudge))); + assert_eq!(discarded.events.len(), 1); + let (_, accepted) = capture_boolean_diagnostics(|| record(BooleanFallback::new(BooleanStrategy::PlanarFallback))); + merge(accepted); + }); + assert_eq!(report.events.len(), 1); + assert_eq!(report.events[0].strategy, BooleanStrategy::PlanarFallback); + assert!(capture_boolean_diagnostics(|| {}).1.events.is_empty()); + record(BooleanFallback::new(BooleanStrategy::ToolNudge)); + assert!(capture_boolean_diagnostics(|| {}).1.events.is_empty()); + } + + #[test] + fn exact_operations_and_failed_calls_have_no_retry_report() { + use solvecraft_geom::Vec3; + let body = crate::box_solid(Vec3::ZERO, Vec3::new(10., 10., 10.)).unwrap(); + let other = crate::box_solid(Vec3::new(5., 0., 0.), Vec3::new(15., 10., 10.)).unwrap(); + let (result, diagnostics) = capture_boolean_diagnostics(|| crate::boolean(&body, &other, crate::BoolOp::Union)); + assert!(result.unwrap().is_some()); + assert!(diagnostics.warning().is_none(), "{diagnostics:?}"); + let mesh = crate::mesh_body(&[Vec3::ZERO, Vec3::X, Vec3::Y], &[[0, 1, 2]]).unwrap(); + let (result, diagnostics) = capture_boolean_diagnostics(|| crate::boolean(&body, &mesh, crate::BoolOp::Cut)); + assert!(result.is_err()); + assert!(diagnostics.warning().is_none(), "{diagnostics:?}"); + } + + #[test] + fn unwinding_restores_parent_and_reports_are_thread_local_and_bounded() { + let (_, report) = capture_boolean_diagnostics(|| { + let failed = crate::guard("diagnostic test", || { + capture_boolean_diagnostics(|| -> crate::Result<()> { + record(BooleanFallback::new(BooleanStrategy::ToolNudge)); + panic!("discard this attempt"); + }) + .0 + }); + assert!(failed.is_err()); + let separate = std::thread::spawn(|| capture_boolean_diagnostics(|| record(BooleanFallback::new(BooleanStrategy::ToolNudge))).1); + assert_eq!(separate.join().unwrap().events.len(), 1); + for _ in 0..MAX_EVENTS + 1 { + record(BooleanFallback::new(BooleanStrategy::PlanarFallback)); + } + }); + assert_eq!(report.events.len(), MAX_EVENTS); + assert!(report.truncated); + assert!(report.events.iter().all(|event| event.strategy == BooleanStrategy::PlanarFallback)); + assert!(report.warning().unwrap().contains("capped")); + } +} diff --git a/crates/kernel/src/coplanar.rs b/crates/kernel/src/coplanar.rs index 256ea52b..c7de53bf 100644 --- a/crates/kernel/src/coplanar.rs +++ b/crates/kernel/src/coplanar.rs @@ -557,8 +557,15 @@ pub(crate) fn bore_union(a: &Body, b: &Body, accept: &dyn Fn(&Body) -> bool) -> let cut = crate::build::cylinder(at(lo, 0.0, 0.0), c.axis, c.radius, hi - lo)?; crate::ops::boolean(&joined, &cut, BoolOp::Cut) }; - match run() { - Ok(Some(r)) if accept(&r) => return Some(r), + let (result, diagnostics) = crate::capture_boolean_diagnostics(run); + match result { + Ok(Some(r)) if accept(&r) => { + crate::boolean_diagnostics::merge(diagnostics); + let mut event = crate::BooleanFallback::new(crate::BooleanStrategy::CommonBore); + event.face_shift_mm = d; + crate::boolean_diagnostics::record(event); + return Some(r); + } _ => continue, } } diff --git a/crates/kernel/src/fuzz.rs b/crates/kernel/src/fuzz.rs index cd03b500..8f8af63f 100644 --- a/crates/kernel/src/fuzz.rs +++ b/crates/kernel/src/fuzz.rs @@ -99,6 +99,8 @@ pub struct BoolCase { /// "ok", or the kind of failure: "error", "invalid", "identity". pub outcome: String, pub detail: String, + pub boolean_fallbacks: std::collections::BTreeMap, + pub boolean_reports_truncated: bool, } fn vol(b: &Option) -> crate::Result { @@ -118,7 +120,15 @@ pub fn boolean_shapes(seed: u64) -> (Shape, Shape) { /// V(A∪B) + V(A∩B) = V(A) + V(B) and V(A−B) = V(A) − V(A∩B), within 0.2 %. pub fn boolean_case(seed: u64) -> BoolCase { let (a, b) = boolean_shapes(seed); - let mut case = BoolCase { seed, a: a.clone(), b: b.clone(), outcome: "ok".into(), detail: String::new() }; + let mut case = BoolCase { + seed, + a: a.clone(), + b: b.clone(), + outcome: "ok".into(), + detail: String::new(), + boolean_fallbacks: Default::default(), + boolean_reports_truncated: false, + }; let fail = |c: &mut BoolCase, kind: &str, d: String| { c.outcome = kind.into(); c.detail = d; @@ -129,7 +139,12 @@ pub fn boolean_case(seed: u64) -> BoolCase { }; let mut res = Vec::new(); for op in [BoolOp::Union, BoolOp::Intersect, BoolOp::Cut] { - match boolean(&ba, &bb, op) { + let (result, diagnostics) = crate::capture_boolean_diagnostics(|| boolean(&ba, &bb, op)); + case.boolean_reports_truncated |= diagnostics.truncated; + for event in diagnostics.events { + *case.boolean_fallbacks.entry(event.strategy).or_default() += 1; + } + match result { Ok(x) => { if let Some(body) = &x { let bad = body.validity(); diff --git a/crates/kernel/src/lib.rs b/crates/kernel/src/lib.rs index 5a3b45f6..d900e4bf 100644 --- a/crates/kernel/src/lib.rs +++ b/crates/kernel/src/lib.rs @@ -16,6 +16,7 @@ mod blend; mod blend_corner; mod blend_variable; mod body; +mod boolean_diagnostics; mod bore_guard; mod build; mod coplanar; @@ -62,6 +63,7 @@ pub use blend::{ChamferSide, chamfer, chamfer_sides, fillet}; pub use blend_corner::fillet_radii; pub use blend_variable::{fillet_chord, fillet_variable, fillet_variable_points}; pub use body::{Body, CapSignature, EdgeInfo, FaceInfo, FacePaint, FaceSource, Paint}; +pub use boolean_diagnostics::{BooleanDiagnostics, BooleanFallback, BooleanStrategy, capture_boolean_diagnostics}; pub use build::{PathSeg, box_solid, canonical_revolve_axis, cylinder, extrude, extrude_tapered, loft, revolve, sphere, sweep, torus}; pub use delete_face::delete_faces; pub use emboss::{cylinder_emboss_anchor, emboss_cylinder, emboss_cylinder_at, has_cylinder_emboss_wall, infer_cylinder_emboss}; diff --git a/crates/kernel/src/ops.rs b/crates/kernel/src/ops.rs index 8a0763b4..d0ebb211 100644 --- a/crates/kernel/src/ops.rs +++ b/crates/kernel/src/ops.rs @@ -5,6 +5,7 @@ use solvecraft_geom::Vec3; use truck_modeling as mt; use crate::body::{Body, Solid, from_p3}; +use crate::boolean_diagnostics::{BooleanFallback, BooleanStrategy, capture_boolean_diagnostics, merge, record}; use crate::{KernelError, Result, guard}; #[cfg(test)] @@ -192,7 +193,7 @@ pub(crate) fn narrowing_bores() -> bool { /// everything or an intersection of disjoint bodies. Results are checked against volume bounds /// and retried with shifted copies and other tolerances when they fail or look wrong. pub fn boolean(a: &Body, b: &Body, op: BoolOp) -> Result> { - let mut r = boolean_whole(a, b, op); + let (mut r, mut diagnostics) = capture_boolean_diagnostics(|| boolean_whole(a, b, op)); // Bodies in several pieces: when the whole failed (or came out empty), a piece at a time. let retry = match &r { Err(_) => true, @@ -201,10 +202,13 @@ pub fn boolean(a: &Body, b: &Body, op: BoolOp) -> Result> { }; if retry && (a.solid.boundaries().len() > 1 || b.solid.boundaries().len() > 1) { let (la, lb) = (a.lumps()?, b.lumps()?); - if (la.len() > 1 || lb.len() > 1) - && let Ok(Some(x)) = by_lumps(la, lb, op) - { - r = Ok(Some(x)); + if la.len() > 1 || lb.len() > 1 { + let (pieces, report) = capture_boolean_diagnostics(|| by_lumps(la, lb, op)); + if let Ok(Some(x)) = pieces { + r = Ok(Some(x)); + diagnostics = report; + diagnostics.push(BooleanFallback::new(BooleanStrategy::Piecewise)); + } } } if op == BoolOp::Union @@ -215,7 +219,11 @@ pub fn boolean(a: &Body, b: &Body, op: BoolOp) -> Result> { // Prefer the exact polygon result; reject an edge-only pinch if it cannot // form a closed solid without displacing either operand. r = match crate::polybool::planar_boolean(a, b, op) { - Ok(Some(exact)) if preserves_planar_surfaces(a, b, &exact) => Ok(Some(exact)), + Ok(Some(exact)) if preserves_planar_surfaces(a, b, &exact) => { + diagnostics = Default::default(); + diagnostics.events.push(BooleanFallback::new(BooleanStrategy::PlanarFallback)); + Ok(Some(exact)) + } _ => Err(KernelError::Invalid( "join cannot preserve the original faces; check for zero-thickness edge contact or add intentional overlap".into(), )), @@ -225,6 +233,9 @@ pub fn boolean(a: &Body, b: &Body, op: BoolOp) -> Result> { if matches!(op, BoolOp::Cut | BoolOp::Union) { r = r.and_then(|result| result.map(|body| body.with_preserved_boolean_planar_sources(a, b, op == BoolOp::Cut)).transpose()); } + if r.is_ok() { + merge(diagnostics); + } r } @@ -353,11 +364,15 @@ fn boolean_whole(a: &Body, b: &Body, op: BoolOp) -> Result> { } if !APART.with(|c| c.get()) && crate::coplanar::has_coincident_faces(a, b) { APART.with(|c| c.set(true)); - let r = crate::coplanar::boolean_apart(a, b, op); + let (r, diagnostics) = capture_boolean_diagnostics(|| crate::coplanar::boolean_apart(a, b, op)); APART.with(|c| c.set(false)); if let Some(Ok(Some(body))) = r { let (ok, _) = check(&body); if ok { + merge(diagnostics); + let mut event = BooleanFallback::new(BooleanStrategy::CoplanarSeparation); + event.face_shift_mm = size * 0.0229; + record(event); return Ok(Some(body)); } } @@ -365,13 +380,15 @@ fn boolean_whole(a: &Body, b: &Body, op: BoolOp) -> Result> { // Bores on a common cylinder (a pad's bore around the hole it is joined over). if op == BoolOp::Union && !BORES.with(|c| c.get()) && crate::coplanar::has_coincident_cylinders(a, b) { BORES.with(|c| c.set(true)); - let r = crate::coplanar::bore_union(a, b, &|body| check(body).0); + let (r, diagnostics) = capture_boolean_diagnostics(|| crate::coplanar::bore_union(a, b, &|body| check(body).0)); BORES.with(|c| c.set(false)); if let Some(body) = r { + merge(diagnostics); return Ok(Some(body)); } } let mut empty_votes = 0; + let mut empty_diagnostics = crate::BooleanDiagnostics::default(); let keep: Vec> = a.split_keep().iter().chain(b.split_keep()).cloned().collect(); for (attempt, j) in JITTER.iter().enumerate() { let shift = mt::Vector3::new(j[0], j[1], j[2]) * (size * 0.01); @@ -397,12 +414,29 @@ fn boolean_whole(a: &Body, b: &Body, op: BoolOp) -> Result> { match r { Ok(s) if s.boundaries().is_empty() || s.face_iter().next().is_none() => { empty_votes += 1; + if attempt > 0 || k != 2e-3 { + let mut event = + BooleanFallback::new(if attempt > 0 { BooleanStrategy::ShiftedOperands } else { BooleanStrategy::RefinedTolerance }); + event.translation_mm = Vec3::new(j[0], j[1], j[2]).len() * (size * 0.01); + event.tolerance_mm = tol; + empty_diagnostics.push(event); + } last = "empty result".into(); } Ok(s) => match guard("boolean result", || Body::new(crate::heal::heal_keep(s, size, &keep))) { Ok(body) => { let (ok, v) = check(&body); if ok { + if attempt > 0 || k != 2e-3 { + let mut event = BooleanFallback::new(if attempt > 0 { + BooleanStrategy::ShiftedOperands + } else { + BooleanStrategy::RefinedTolerance + }); + event.translation_mm = Vec3::new(j[0], j[1], j[2]).len() * (size * 0.01); + event.tolerance_mm = tol; + record(event); + } return Ok(Some(body)); } last = format!("implausible result volume {v:.4}"); @@ -413,18 +447,23 @@ fn boolean_whole(a: &Body, b: &Body, op: BoolOp) -> Result> { } } if empty_votes >= 2 && op != BoolOp::Union { + merge(empty_diagnostics); return Ok(None); } } // Coincident planar faces: push them apart (exactly) and try again. if !APART.with(|c| c.get()) { APART.with(|c| c.set(true)); - let r = crate::coplanar::boolean_apart(a, b, op); + let (r, diagnostics) = capture_boolean_diagnostics(|| crate::coplanar::boolean_apart(a, b, op)); APART.with(|c| c.set(false)); match r { Some(Ok(Some(body))) => { let (ok, v) = check(&body); if ok { + merge(diagnostics); + let mut event = BooleanFallback::new(BooleanStrategy::CoplanarSeparation); + event.face_shift_mm = size * 0.0229; + record(event); return Ok(Some(body)); } last = format!("{last}; faces pushed apart gave an implausible volume {v:.4}"); @@ -440,11 +479,15 @@ fn boolean_whole(a: &Body, b: &Body, op: BoolOp) -> Result> { Ok(Some(body)) => { let (ok, v) = check(&body); if ok { + record(BooleanFallback::new(BooleanStrategy::PlanarFallback)); return Ok(Some(body)); } last = format!("planar fallback gave an implausible volume {v:.4}"); } - Ok(None) => return Ok(None), + Ok(None) => { + record(BooleanFallback::new(BooleanStrategy::PlanarFallback)); + return Ok(None); + } Err(e) => last = format!("{last}; {e}"), } // Last resort: nudge the tool by a micron-scale screw motion (turned 2e-6 rad about a @@ -453,17 +496,31 @@ fn boolean_whole(a: &Body, b: &Body, op: BoolOp) -> Result> { if !APART.with(|c| c.get()) { APART.with(|c| c.set(true)); let c = b.solid.vertex_iter().fold(Vec3::ZERO, |acc, v| acc + from_p3(v.point())) * (1.0 / b.solid.vertex_iter().count().max(1) as f64); - let r = transform(b, Vec3::new(0.61, -0.37, 0.71) * (size * 2e-6), c, Vec3::new(0.27, 0.83, -0.49), 2e-6).and_then(|nb| boolean(a, &nb, op)); + let translation = Vec3::new(0.61, -0.37, 0.71) * (size * 2e-6); + let (r, diagnostics) = + capture_boolean_diagnostics(|| transform(b, translation, c, Vec3::new(0.27, 0.83, -0.49), 2e-6).and_then(|nb| boolean(a, &nb, op))); APART.with(|c| c.set(false)); match r { Ok(Some(body)) => { let (ok, v) = check(&body); if ok { + merge(diagnostics); + let mut event = BooleanFallback::new(BooleanStrategy::ToolNudge); + event.translation_mm = translation.len(); + event.rotation_rad = 2e-6; + record(event); return Ok(Some(body)); } last = format!("{last}; the nudged tool gave an implausible volume {v:.4}"); } - Ok(None) => return Ok(None), + Ok(None) => { + merge(diagnostics); + let mut event = BooleanFallback::new(BooleanStrategy::ToolNudge); + event.translation_mm = translation.len(); + event.rotation_rad = 2e-6; + record(event); + return Ok(None); + } Err(e) => last = format!("{last}; nudged: {e}"), } } diff --git a/crates/kernel/src/tests.rs b/crates/kernel/src/tests.rs index 0fb2578c..4a509c8e 100644 --- a/crates/kernel/src/tests.rs +++ b/crates/kernel/src/tests.rs @@ -324,7 +324,10 @@ fn drain_pads_flush_under_a_floor_with_a_hole() { // The ring's part inside the floor adds nothing: 5 mm of ring below it either way. let v_ring = PI * (400.0 - 100.0) * 5.0; for (z0, z1) in [(-5.0, 0.0), (-5.0, 2.0)] { - let u = boolean(&floor, &ring(z0, z1), BoolOp::Union).unwrap().unwrap(); + let (result, diagnostics) = capture_boolean_diagnostics(|| boolean(&floor, &ring(z0, z1), BoolOp::Union)); + let u = result.unwrap().unwrap(); + assert!(diagnostics.events.iter().any(|event| event.strategy == BooleanStrategy::CommonBore), "{diagnostics:?}"); + assert!(diagnostics.events.iter().all(|event| event.translation_mm.is_finite() && event.face_shift_mm.is_finite())); let v = measure(&u).unwrap().volume; assert!(rel(v, v_floor + v_ring) < 2e-5, "ring {z0}..{z1}: {v} vs {}", v_floor + v_ring); assert!(u.validity().is_empty(), "{:?}", u.validity()); diff --git a/docs/robustness.md b/docs/robustness.md index 4639aa10..0713bdbc 100644 --- a/docs/robustness.md +++ b/docs/robustness.md @@ -36,6 +36,30 @@ rejected before any jobs or output directories are created. The maximum seed - `cargo test` runs a fixed set of seeds that must stay clean (`crates/engine/src/tests/robustness.rs`), next to the minimised cases fixed so far. +## Successful Boolean retries + +When an accepted Boolean result needs a numerical retry, the feature shows a +warning in the timeline. `timeline.inspect` and `doc.inspect` expose its +`boolean_diagnostics`, including the strategy and the applied translation, +rotation, face offset and tessellation tolerance in millimetres/radians. + +A common shift of both operands is reversed on the result. Coincident-face +separation and bore offsets describe temporary working geometry; they are not +claims about the final model's error. A tool nudge changes the relative operand +placement and reports both its translation magnitude and rotation angle. +Exact planar and piecewise fallbacks report zero displacement. The existing +volume and spatial acceptance checks still apply to all results. + +Reports belong to the evaluated feature and survive cached rebuilds and +undo/redo. Cold reopening regenerates them from the embedded design. Failed or +discarded retry branches do not contribute successful-retry events. Reports +are capped at 1024 events per feature and explicitly identify truncation. + +Fuzz JSON and the text report count successful retry events by strategy for +designs and Boolean identity checks separately, including nested operations. +These are event counts, so a single Boolean or seed can contribute several. +Truncated reports are counted separately rather than implying complete totals. + ## Results 1000 seeds (0–999), 8 processes on a heavily loaded machine (load average 60–80), 90 s per