Skip to content

fix(geometry): carry the no-void probe reading through the census reclassification list (#3366) - #3374

Open
BIMvoice wants to merge 1 commit into
mainfrom
fix-3366-census-no-void
Open

fix(geometry): carry the no-void probe reading through the census reclassification list (#3366)#3374
BIMvoice wants to merge 1 commit into
mainfrom
fix-3366-census-no-void

Conversation

@BIMvoice

@BIMvoice BIMvoice commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Refs Census: a no-void probe that starts failing reads as an improvement and the lane stays green #3366: HostRow::is_torn_solid reads open, rep, far, and pre (the no-void probe reading). reclassifications — the list of is_torn_solid inputs that move the gated defect population without any count moving — carried rep and far, but not pre.
  • When a host's no-void probe starts or stops failing, pre flips Open(n) <-> Failed while open, tris, collapsed and alt all hold, so only the derived is_torn_solid predicate saw it move. A probe going dark landed in classify's "no longer a genuine watertightness defect" arm with nothing else to attribute it to, so the host filed under improved, requires_bless() stayed false, and the lane stayed green while the census lost the ability to distinguish "arrived torn" from "the boolean tore it" for that host.
  • Fix: carry pre in reclassifications, per the issue's suggested fix. Both directions of a no-void flip now route into changed, which requires a bless. The probe-recovers direction changes label too (previously regressed, now changed) — both are red and both require a bless, so this does not weaken the gate; it updates the pre-existing test (a_no_void_pass_that_stops_failing_makes_the_host_a_gated_defect, renamed) that encoded the old label.
  • Grepped for the same shape (a symmetric better/worse classify pair over a derived predicate) elsewhere in the repo's census/benchmark comparators; rust/geometry/tests/census_golden/mod.rs is the only occurrence.

Test plan

  • RED: added a_probe_that_starts_failing_is_a_reclassification_not_an_improvement, confirmed it fails against the pre-fix code (d.improved non-empty, requires_bless() == false).
  • GREEN after carrying pre in reclassifications.
  • Both directions asserted: probe going dark AND probe starting to run both require a bless (d.changed, not d.improved/d.regressed).
  • Updated the pre-existing test that had encoded the old (buggy) expectation for the probe-recovers direction.
  • Full census_golden suite green: cargo test -p ifc-lite-geometry --test triangulation_invariance --features triangulation-alt census_golden — 19 passed.
  • cargo clippy -p ifc-lite-geometry --tests --features triangulation-alt clean.
  • Test-only change in rust/geometry/tests/; no changeset (matches this repo's convention for test-only fixes, e.g. e2c67f0).

https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of no-void probe failures and recoveries by classifying them as population reclassifications.
    • Transition reports now provide clearer reasons when these changes occur.
    • Both failure and recovery transitions now require explicit approval instead of passing silently.
  • Tests

    • Added coverage to verify correct classification and approval requirements in both directions.

…lassification list (#3366)

`reclassifications` carried `rep` and `far` — both inputs to
`HostRow::is_torn_solid` that move the gated defect population without any
count moving — but not `pre`. When a host's no-void probe starts or stops
failing, `pre` flips `Open(n) <-> Failed` while `open`, `tris`, `collapsed`
and `alt` all hold, so only the derived `is_torn_solid` predicate saw it.
The dark-probe direction landed in `classify`'s "no longer a genuine
watertightness defect" arm with nothing else to attribute it to, so the
host filed under `improved`, `requires_bless()` stayed false, and the lane
stayed green while the census silently lost the ability to tell "arrived
torn" apart from "the boolean tore it" for that host.

Carrying `pre` in `reclassifications` routes both directions of a no-void
flip into `changed`, which requires a bless. This changes the label on the
probe-recovers direction too (previously `regressed`, now `changed`) — both
outcomes are red and both require a bless, so this does not weaken the
gate, but it does update the pre-existing test that encoded the old label.

Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
@BIMvoice
BIMvoice requested a review from louistrue as a code owner August 28, 2026 04:51
@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 354e1674-77a4-406f-8785-caf83c17787e

📥 Commits

Reviewing files that changed from the base of the PR and between 5a431e5 and 8e5c385.

📒 Files selected for processing (1)
  • rust/geometry/tests/census_golden/mod.rs

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The census golden tests now classify no-void probe failures and recoveries as PreVoid reclassifications. Both transition directions appear as changes and require blessing.

Changes

PreVoid reclassification

Layer / File(s) Summary
Record PreVoid transitions
rust/geometry/tests/census_golden/mod.rs
Reclassification reporting now includes HostRow.pre changes and documents their effect on the gated defect population.
Validate transition blessing
rust/geometry/tests/census_golden/mod.rs
Tests cover Open-to-Failed and Failed-to-Open transitions. Both are classified as changed, not improved or regressed, and require blessing.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 8e5c3

This localized test-only change corrects census handling for no-void probe state transitions, with both directions covered and the relevant test suite passing; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: louistrue

Poem

A rabbit checks the probe at dawn

From Open to Failed, the fear is gone
From Failed to Open, records align
Both changes need a blessing sign
The census hops along just fine 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the geometry fix and the no-void probe change in the census reclassification list. It accurately reflects the main change.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Viewer benchmark

✅ No threshold regressions detected.

01_Snowdon_Towers_Sample_Structural(1).ifc

Baseline recorded 2026-07-01T20:31:05.538Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.

Metric Current Baseline Delta Threshold Status
firstBatchWaitMs 2068ms 2905ms -28.8% +50%
firstVisibleGeometryMs 3178ms 3652ms -13.0% +50%
streamCompleteMs 4059ms 3598ms +12.8% +50%
spatialReadyMs 1411ms 1032ms +36.7% +50%
metadataCompleteMs 2128ms 3063ms -30.5% +50%
totalWallClockMs 4200ms 3700ms +13.5% +50%

AC20-FZK-Haus.ifc

Baseline recorded 2026-07-01T20:30:59.972Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.

Metric Current Baseline Delta Threshold Status
firstBatchWaitMs 313ms 1075ms -70.9% +50%
firstVisibleGeometryMs 2054ms 1572ms +30.7% +50%
streamCompleteMs 1091ms 1980ms -44.9% +50%
spatialReadyMs 1126ms 915ms +23.1% +50%
metadataCompleteMs 1206ms 1392ms -13.4% +50%
totalWallClockMs 2100ms 3300ms -36.4% +50%

Refresh the baseline from a CI run: dispatch the Benchmark workflow with record_baseline, download the benchmark-baseline artifact, and commit baseline.json (see tests/benchmark/README.md).

@louistrue

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants