fix(algo): correct sequential multi-tool cuts on thin-walled solids

[andymai]

Sequential cuts on hollow thin-walled solids (honeycomb wall patterns, compartment grids) were erratic from cut 2 and silently no-op'd from cut ~13 (Ok returned, volume frozen). Root causes: (1) the analytic box classifier accepted arbitrary plane soups as axis-aligned boxes — after the first cut the cavity-side planes collapsed into a garbage inner box classifying recess points Inside, so phantom tool fragments survived; it now requires strictly axis-aligned planes with ≤2 offsets per axis, else ray-cast. (2) An acceptance-gate rejection path detailed in the commit. New regression: hollow 84×84×30 box (1.2mm walls), sequential disjoint hex-prism cuts with per-cut analytic volume deltas; no hang under timeout. Full gates green.

[github-actions]

WASM Binary Size

File	main	PR	Delta
brepkit_wasm.wasm	4.90 MB	4.90 MB	+.8 KB (+0%)

⚠️ Size increased slightly.

[Copilot]

Pull request overview

This PR fixes incorrect behavior in sequential boolean cuts on hollow thin-walled solids by tightening analytic classification (avoiding invalid “axis-aligned box” classifiers from arbitrary plane sets) and by strengthening boolean acceptance gates to correctly validate manifoldness across all shells (outer + inner cavity shells). It also adds a dedicated regression test that reproduces the historical “erratic from cut 2” and “silent no-op from ~cut 13” failure modes on a thin-walled honeycomb-like cut pattern.

Changes:

• Add a regression test covering sequential disjoint hex-prism cuts through a thin wall, validating per-cut analytic volume deltas across both short and full sequences.
• Restrict the analytic “box” classifier to true axis-aligned plane sets with ≤2 distinct offsets per axis; otherwise fall back to non-box classification behavior.
• Fix boolean result validation by counting edge uses across all shells, preventing cavity-shell manifold issues from being missed.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
crates/operations/tests/regress_hexwall_cuts.rs	Adds regression coverage for sequential thin-wall hex-prism cuts with per-cut volume-delta assertions.
crates/operations/src/boolean/mod.rs	Updates Euler/genus acceptance notes and ensures manifold/free-edge checks traverse all shells via shared edge-use counting.
crates/algo/src/classifier/analytic.rs	Hardens box-classifier construction to only accept truly axis-aligned “box” plane sets; rejects oblique/over-specified plane soups.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Greptile Summary

This PR fixes two root causes of silent no-ops and wrong volume deltas on sequential cuts into hollow thin-walled solids. The classifier fix tightens build_box to reject non-axis-aligned planes and enforce ≤2 distinct offsets per axis, preventing garbage inner-box classifiers from surviving subsequent booleans. The Euler-balance fix removes the surplus >= 0 lower bound to correctly accept high-genus shells (a wall with N through-holes has genus N and surplus 2 − 2N), paired with the expansion of has_free_edges to walk all shells of the solid.

• analytic.rs: build_box now requires strictly axis-aligned plane normals (|n_i| > 1 − 1e-9) and bails immediately on any oblique plane; the new sort_dedup helper additionally rejects more than 2 distinct offsets per axis.
• boolean/mod.rs: euler_balanced drops the surplus ≥ 0 floor to admit genus-N surfaces; a new solid_edge_use_counts helper replaces the old outer-shell-only edge walk in has_free_edges to catch open cavity shells.
• regress_hexwall_cuts.rs: new regression test drives 20 and 133 sequential hex-prism cuts through an 84×84×30mm hollow bin and asserts per-cut analytic volume deltas within 0.05 mm³.

Confidence Score: 5/5

Safe to merge. The two root-cause fixes are mathematically sound and backed by a well-constructed regression test; all acceptance-gate paths continue to require an independent closed-manifold check.

The analytic classifier tightening is clearly correct and conservative (it bails out to ray-cast on any doubt). The Euler-balance relaxation is paired at every call site with is_closed_manifold, which provides the per-shell guarantee that euler_balanced alone no longer enforces. The one note — pooled edge counts in has_free_edges can theoretically mask a cross-shell free edge — is covered by is_closed_manifold in every acceptance path and is a defence-in-depth concern rather than a reachable defect.

The edge-pooling logic in has_free_edges (boolean/mod.rs) is worth a second look, specifically the interaction with BooleanOp::Intersect on line 546 where it is the primary topology check.

Important Files Changed

Filename	Overview
crates/algo/src/classifier/analytic.rs	Tightened build_box to require strictly axis-aligned planes (
crates/operations/src/boolean/mod.rs	Two linked changes: euler_balanced drops the surplus >= 0 lower bound to accept genus-N surfaces; has_free_edges now pools edge counts across all shells via the new solid_edge_use_counts helper. The pooling can theoretically mask a free edge present once in each shell, but is_closed_manifold in all acceptance paths independently catches it per-shell.
crates/operations/tests/regress_hexwall_cuts.rs	New regression test covering sequential hex-prism cuts on a hollow thin-walled box. Volume-delta assertions are tight (0.05 mm³ tolerance). Existing threads cover the timeout invariant and open-top bin geometry.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
crates/operations/src/boolean/mod.rs:2351-2354
**Pooled edge counts can mask a free edge when the same edge index appears once in each shell.**

`solid_edge_use_counts` sums edge uses across every shell into a single map. If an edge index appears exactly once in the outer shell and exactly once in an inner cavity shell (a cross-shell edge, which is topologically invalid), its pooled count is 2 and `has_free_edges` returns false — the free edge is silently hidden. In practice this is caught by the independent `is_closed_manifold` call in every acceptance path (per-shell count of 1 ≠ 2 fails that check), so the gate as a whole remains sound. But `has_free_edges` no longer reliably detects free edges in the outer shell on its own, which weakens a defence-in-depth guard that is the only topology check for the `BooleanOp::Intersect` path (line 546).

Consider walking each shell's edge counts independently (the same pattern used in `shell_is_closed_manifold`) so the function's contract — "outer shell has a free edge" — holds without depending on a companion call.

_{Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile}

[greptile-apps]

The axis_tol value of 1e-9 is a magic constant sitting between f64::EPSILON (~2.2e-16) and tol.linear (1e-7). The intent — reject any normal that isn't essentially exactly axis-aligned — is clear from the comment, but the choice of 1e-9 vs. something like f64::EPSILON * 4096.0 (a few thousand ULPs) is unexplained and easy to accidentally bump. A named constant with a short rationale comment would make future adjustments deliberate rather than arbitrary.

Suggested change

	let axis_tol = 1e-9;
	// Allow at most ~4096 ULPs of deviation from a unit axis component;
	// this is far tighter than tol.linear (1e-7) so only truly axis-aligned
	// normals pass, while 30°/60° hex-prism faces (component ≈ 0.866) are
	// unambiguously rejected.
	let axis_tol = f64::EPSILON * 4096.0; // ≈ 9.1e-13

Prompt To Fix With AI

This is a comment left during a code review.
Path: crates/algo/src/classifier/analytic.rs
Line: 966

Comment:
The `axis_tol` value of `1e-9` is a magic constant sitting between `f64::EPSILON` (~2.2e-16) and `tol.linear` (1e-7). The intent — reject any normal that isn't essentially exactly axis-aligned — is clear from the comment, but the choice of `1e-9` vs. something like `f64::EPSILON * 4096.0` (a few thousand ULPs) is unexplained and easy to accidentally bump. A named constant with a short rationale comment would make future adjustments deliberate rather than arbitrary.

```suggestion
        // Allow at most ~4096 ULPs of deviation from a unit axis component;
        // this is far tighter than tol.linear (1e-7) so only truly axis-aligned
        // normals pass, while 30°/60° hex-prism faces (component ≈ 0.866) are
        // unambiguously rejected.
        let axis_tol = f64::EPSILON * 4096.0; // ≈ 9.1e-13
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

No timeout guard for the "must complete in seconds" invariant. The comment explicitly calls out the historical multi-minute mesh-fallback churn and says the test must complete in seconds, but there is no enforcement. If a future change silently re-introduces the fallback, the test would still pass — it would just be very slow. Consider wrapping the call with a deadline: e.g. spawn the heavy work on a thread and join with a Duration, or use a test harness timeout attribute. Without this, the performance invariant is documentation-only.

Prompt To Fix With AI

This is a comment left during a code review.
Path: crates/operations/tests/regress_hexwall_cuts.rs
Line: 107-113

Comment:
**No timeout guard for the "must complete in seconds" invariant.** The comment explicitly calls out the historical multi-minute mesh-fallback churn and says the test must complete in seconds, but there is no enforcement. If a future change silently re-introduces the fallback, the test would still pass — it would just be very slow. Consider wrapping the call with a deadline: e.g. spawn the heavy work on a thread and `join` with a `Duration`, or use a test harness timeout attribute. Without this, the performance invariant is documentation-only.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

make_hollow_bin produces an open-top bin, not a six-sided hollow box. The cavity is 30.0 mm tall (matching the outer box), translated by WALL_T = 1.2 mm in z, so it runs from z = 1.2 to z = 31.2. Since the outer box ceiling is at z = 30.0, the top face is fully removed by the boolean cut — the bin has no lid. The PR description says "hollow 84×84×30 box (1.2mm walls)", which implies closed walls on all sides. For the specific regression being tested (front-wall hex cuts) this doesn't affect correctness, but a comment noting the intentionally open top would prevent confusion, and a future test that relies on top-wall geometry would silently use the wrong shape.

Prompt To Fix With AI

This is a comment left during a code review.
Path: crates/operations/tests/regress_hexwall_cuts.rs
Line: 27-33

Comment:
**`make_hollow_bin` produces an open-top bin, not a six-sided hollow box.** The cavity is `30.0` mm tall (matching the outer box), translated by `WALL_T = 1.2` mm in z, so it runs from `z = 1.2` to `z = 31.2`. Since the outer box ceiling is at `z = 30.0`, the top face is fully removed by the boolean cut — the bin has no lid. The PR description says "hollow 84×84×30 box (1.2mm walls)", which implies closed walls on all sides. For the specific regression being tested (front-wall hex cuts) this doesn't affect correctness, but a comment noting the intentionally open top would prevent confusion, and a future test that relies on top-wall geometry would silently use the wrong shape.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!