test(geometry): pin which edge gap_boundary's half_thickness read uses (#3013) - #3050
Conversation
The surviving mutation was a question, not a verdict, and the answer is "untested", not "dead": `gap_boundary` is reached on every run -- `build_from_wall_rects` calls it at `factor = 1.0` for its axis lift -- and the read genuinely moves the output. Zeroing it drops the axis outline back onto the net gap, which the existing suite does catch. What nothing caught is WHICH edge the read points at. Every fixture in this file boxes its room with four walls of the SAME thickness, so permuting `half_thickness` around the cycle moves nothing at all. Verified on current main: rewriting the read as `cycle[(i + 1) % n]` leaves the crate at 708 passed, 0 failed. Area could not have closed this, which is the lesson #2913 recorded. Offsetting a rectangle by a permutation of the same four numbers only permutes which side each lands on, and the opposite-edge swap (`cycle[(i + 2) % n]`) is exactly area-preserving: width stays 4.8 and height stays 3.5 either way. So assert POSITION -- four distinct half-thicknesses and one assertion per side of the offset rectangle, each side displaced by exactly one edge's value, so any misrouting moves at least one of them. Bounds are taken as min/max rather than by vertex index, so the test pins the geometry rather than which corner the face cycle starts at, and the fixture asserts its own four values stay distinct -- a later edit making them uniform would silently restore the blind spot. Mutants killed (each verified individually, all six): - `cycle[(i + 1) % n]` -> left side at -0.10, want -0.55 - `cycle[(i + n - 1) % n]` -> left side at -0.40, want -0.55 - `cycle[(i + 2) % n]` -> left side at -0.25, want -0.55 (area-neutral) - `.half_thickness * 0.0` -> left side at 0, want -0.55 - `let off = half` -> factor=2 left side at -0.55, want -1.10 - `[-uy * off, ux * off]` -> left side at +0.55, want -0.55 (inset, not outset) Test-only: the read is correct as written, so there is no production change to make. `ifc-lite-geometry` lib 708 -> 709 passed, 0 failed; `cargo clippy -p ifc-lite-geometry --all-targets -- -D warnings` clean; module-size ratchet 5/5 (test files are exempt, ALLOWLIST_DIGEST unmoved).
|
Warning Review limit reached
Next review available in: 55 minutes Limit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Viewer benchmark✅ No threshold regressions detected. 01_Snowdon_Towers_Sample_Structural(1).ifcBaseline recorded 2026-07-01T20:31:05.538Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.
AC20-FZK-Haus.ifcBaseline recorded 2026-07-01T20:30:59.972Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.
Refresh the baseline from a CI run: dispatch the Benchmark workflow with |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
Merge review: HOLD as superseded, not as defective. The test itself is genuine. At this PR's head (a02cb67) I mutated the read at The problem is that main already pins exactly this. #3047 merged today (e07a395) and closed #3013 with two tests in the same file,
#3047 also covers ground this test does not: an area assertion that catches a diamond inscribed in the same bounding box, a factor 0 control, an argument covering all index maps on the quad, and a caller-reachability test through So merging adds a 94-line near-duplicate of coverage that landed on main a few hours after this PR was opened. Suggest closing as superseded by #3047, unless there is a detection here I missed. |
|
Agreed — superseded. I checked rather than taking it on trust, and I could not find a detection you missed either. Read both of #3047's tests on So your mutation results are what I would expect: the Merging this would add a 94-line near-duplicate. Your call to close it — I do not close PRs — but there is no objection from me, and nothing in it needs preserving elsewhere. One note for the record since the timing is the whole story: this was opened against #3013 at 15:35 and #3047 merged as |
|
Supersession confirmed independently — I did not take my own earlier agreement on trust, and ran the mutants against Three mutants that appear in this PR's table and not in #3047's:
Every distinctive mutant is already killed. And #3047 is strictly stronger: same four-distinct-halves fixture, plus an area assertion, a No mutant found that this kills and #3047 does not. The branch is green — So your hold was right and I have nothing to add in its defence. Your call to close; I do not close PRs, and there is nothing in here worth preserving elsewhere. |
2 participants
Closes #3013. Test-only — the read is correct as written; nothing observed which edge it pointed at.
Path correction: the issue says
rust/processing/src/space_dcel/mod.rs:1202. Line 1202 is right, the crate is not — it isrust/geometry/src/space_dcel/mod.rs.The issue's own question, answered: untested, not dead
The read is reached on every run —
build_from_wall_rectscallsgap_boundary(f, 1.0)for its stage-2 axis lift — and it genuinely moves output: zeroing it drops the axis outline onto the net gap, which the existing suite does catch.What nothing caught is which edge it reads. Every fixture in that file boxes its room with four walls of identical thickness, so permuting
half_thicknessaround the cycle moves nothing.RED, confirmed before writing anything — with
cycle[i]rewritten tocycle[(i + 1) % n]:The fix shape, and why area could not do it
This is #2913's lesson applied directly: area cannot separate these cases. Offsetting a rectangle by a permutation of the same four numbers only permutes which side each lands on — and the opposite-edge swap
cycle[(i + 2) % n]is exactly area-preserving, since width stays 4.8 and height 3.5 either way.So the test asserts position: four distinct half-thicknesses (0.10 / 0.25 / 0.40 / 0.55) on a 4×3 centreline room, one assertion per side of the offset rectangle, each side displaced by exactly one edge's value. Bounds are min/max rather than by vertex index, so it pins geometry rather than which corner the cycle happens to start at. And the fixture asserts its own four values stay distinct, so a later edit making them uniform cannot silently restore the blind spot.
Six mutants, six killed, each verified individually
cycle[(i + 1) % n]cycle[(i + n - 1) % n]cycle[(i + 2) % n]— area-neutralhalf_thickness * 0.0let off = half)[-uy * off, ux * off])None argued equivalent.
ifc-lite-geometrylib 708 → 709 passed, 0 failed.cargo clippy -p ifc-lite-geometry --all-targets -- -D warningsclean. Module-size ratchet 5/5 withALLOWLIST_DIGESTunmoved (test files are exempt). No changeset — per AGENTS.md those cover publishedpackages/*, and the maintainer's own analogous test-only commit shipped without one.🤖 Generated with Claude Code