test(geometry): count strict directed edge pairs beside the signed balance (#3397) - #3416
Conversation
…lance (#3397) `edge_stats` measured watertightness as a SIGNED per-edge balance: an undirected edge counted when its forward and reverse use counts differed. A face duplicated along with its opposite-wound twin adds one forward and one reverse use of each edge and cancels to zero, and so does a 2-forward / 2-reverse seam. `total unmatched edges` and `torn void hosts` both derive from that count, so a duplicate-face defect was invisible to the census in either direction. The repo already holds the stricter reading and says why: `touching_operand.rs` ("A net-signed tally is not enough"), the bowtie fixture in the boolean chain-cycle tests, and #3353's pin on `forward == 1 && reverse == 1`. Option 1 from the issue: a strict directed-pair count as a SECOND column on `HostRow`, not a replacement. Both readings come off ONE walk over ONE 1 mm snapped topology in `edge_stats`, so they cannot drift into two measurements of two meshes. `open`, `alt` and `pre` keep the signed reading, so the gated defect population and the `torn` / `torn_solid` / `total unmatched edges` totals are unmoved and no pre-existing non-manifold host is reclassified into a tear. The number the issue was filed to get, over the 1170 swept void hosts: 7 hosts watertight by the SIGNED balance and torn by the STRICT rule 116 hosts where the two readings disagree at all 1430 edges the signed balance cannot see (20453 signed, 21883 strict) A RISE in `strict` gates as a worsened count, so the pair that hides the hazard (`open` falling while `strict` rises) files as a regression rather than as the friendly re-tessellation verdict. A FALL is qualified through the same `fall_note` as a falling `open`, so one arm cannot print "(improved)" beside the other's "(geometry lost)". The golden was re-blessed through `IFCLITE_CENSUS_BLESS`. Diffed against a census run from the parent commit: columns 1-9 are byte-identical on all 1170 hosts, and only the appended column differs. `strict` is written LAST for exactly that reason, so `cut -f1-9` still reproduces the pre-#3397 row. The blind-spot note on `open_is_a_defect_count` / `open_is_comparable` documented this limitation as un-gated. It now says what is measured and what still is not: `alt` and `pre` stay signed, so a doubled sheet only one triangulator emits is still invisible to `diverged`, and neither reading sees a self-intersection or coincidence finer than the 1 mm snap.
…ng it twice /simplify: the `unit_cube()` + `[0, 3, 2, 0, 2, 3]` doubling was constructed in two tests, and the comment explaining why those six indices are a doubling rather than a hole sat only on the first copy. A drifted copy would leave `a_real_hole_moves_both_readings_and_strict_is_never_below_open` asserting `strict >= open` over some other mesh and still passing. Extracted `doubled_face_cube()` next to `unit_cube()` and moved the rationale onto it. No behaviour change.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_cac7a897-2552-47aa-a04d-18d7481bd68b) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. 📝 WalkthroughWalkthroughThe census now records signed and strict directed-edge violations. Host rows, corpus totals, classification, TSV serialization, golden checks, and topology fixtures validate both readings. ChangesStrict watertightness census
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: ⚪ Minimal · up to This test-only change adds strict directed-edge census coverage and updates verified golden data; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant CensusHost
participant edge_stats
participant CensusClassifier
participant CorpusTotals
participant GoldenTSV
CensusHost->>edge_stats: compute open and strict violations
edge_stats-->>CensusHost: return edge statistics
CensusHost->>CensusClassifier: provide HostRow
CensusClassifier->>CorpusTotals: classify host and accumulate totals
CorpusTotals->>GoldenTSV: render ten-column census row
GoldenTSV-->>CensusHost: parse and validate strict value
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The changes implement the linked issue's core requirements. They add a strict count beside the signed count, compute both from the topology walk, preserve existing signed measurements, serialize the tenth column, and add regression coverage. The corpus TSV is excluded from review by the !**/*.tsv path filter, so the golden-file update cannot be independently verified. Full details: Out of Scope Changes checkExplanation The reviewed changes are test-only and directly support the linked issue. They add strict census measurements, serialization, classification logic, fixtures, and validation without unrelated production API or changeset modifications.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
|
Review evidence, since the hosted bots are dark today. Same situation as #3415: the CodeRabbit CLI, run locally against this branch's merge-base: All three changed files named, 0 findings. A real review rather than a refusal — the CLI exits 0 either way, so naming its subject is the discriminator. The re-bless is the part worth an independent check before merging, and it was verified rather than trusted: strip the new Local gates: |
Viewer benchmark⚠ 2 metric(s) exceeded the regression threshold (advisory only, not blocking). 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 |
1 participant
Closes #3397.
edge_statsmeasured watertightness with a SIGNED per-edge balance (+1 forward, -1reverse, count non-zero). A duplicated face contributes one extra forward and one extra
reverse use of each edge and cancels to zero; so does a 2-forward / 2-reverse seam. Both
total unmatched edgesandtorn void hostsderive from that count, so a duplicate-facedefect was invisible to the gate in either direction.
The repo already held the stricter reading and said why —
touching_operand.rs("Anet-signed tally is not enough: an edge used twice forward and twice reverse cancels to
zero, so a non-manifold seam would be certified closed"), the bowtie fixture in
processors/boolean/chain_cycle_tests.rs, and #3388's pin checkingforward == 1 && reverse == 1per directed edge. The census used the weaker one.Option 1 as filed: a SECOND column, not a replacement
strictis added besideopen, both computed from the same walk over the same trianglesso they cannot drift. Replacing the signed count outright would change every number in the
golden and surface pre-existing non-manifold hosts as new tears; keeping both is what lets
the two readings be compared per host.
The number the issue was filed to obtain
Over 1170 corpus hosts:
strict < signedstrict == signedstrict > signedopen == 0,strict > 0)Both numbers matter and quoting either alone misleads. The two rules disagree on 116
hosts; on 109 of them both already call the host torn and
strictsimply counts moreunmatched edges, so the signed balance was under-reporting severity without changing the
verdict. The 7 are where the census would have called a host watertight and the strict
rule does not — the population the gate could have been wrong about.
strict < signedon zero hosts is the sanity property: a directed-pair rule can only findthe same defects or more, never fewer, so a single host below would mean one counter is
wrong rather than merely weaker.
The re-bless, verified rather than trusted
Regenerating a golden accepts every difference in it, including ones the change did not
cause. So the manifest was checked, not blessed and hoped for: 1170 data rows before and
after, 9 -> 10 columns, and stripping the new
strictcolumn reproducesorigin/main'srows byte-for-byte. No existing column moved on any host. Re-verified at the final head,
not only when first generated.
HEADERgained astrict:legend describing what it sees thatopencannot, so #3401'sbyte-for-byte golden assertion stays satisfied.
The blind-spot note on
open_is_a_defect_countdocumented this exact limitation and wouldotherwise have been a comment that was true when written and false afterwards; it now says
what is measured and what still is not.
Test-only: three files under
rust/geometry/tests/. No changeset, no API surface.Gates:
cargo test -p ifc-lite-geometry --no-fail-fastexit 0,cargo clippy --workspace --exclude ifc-lite-wasm --all-targets -- -D warningsexit 0.Summary by CodeRabbit
New Features
Bug Fixes
Tests