fix(review): drop parity-allowlist entries whose divergences are now fixed

[BIMvoice]

Summary

The server/browser type-parity gate (#3979) shipped an allowlist naming four divergences as in-flight, two of them by their own text: "open PR #3973" and "open PR #3971". Both merged — #3973 promoted the contained (not aggregated) IfcSpatialZone/IfcMarinePart/IfcFacilityPartCommon types into their own hierarchy nodes on the Rust server, and #3971 added the server's IFCCOMPLEXPROPERTY arm — so the four entries now mute nothing, and #3979's staleness detector correctly refuses them:

[spatialTypes:IFCSPATIALZONE]        the type is now handled by BOTH
[spatialTypes:IFCMARINEPART]         sides -- the divergence this entry
[spatialTypes:IFCFACILITYPARTCOMMON] mutes is gone
[properties:IFCCOMPLEXPROPERTY]

Each of #3971/#3973 was green alone; only the merged tree (with #3979's detector also present) is red, because #3979 added the check and #3971/#3973 independently closed the gaps it was told to expect as still-open. This is the staleness mechanism working exactly as designed — a temporary mute expiring — not a bug in the checker, and it is currently blocking Node tests on every PR rebased onto main (confirmed on #4102, #4090, #4089, #4018's job logs).

• Removed the four stale ALLOWLIST entries; the remaining 7 (6 pending, 1 deliberate) are untouched.
• The gate's regression suite used two of those entries as fixtures for "an allowlisted divergence does not fail on its own". No spatialTypes or properties entry survives to point a fixture at, and suppression is concept-agnostic (one ALLOWLIST['${concept}:${type}'] lookup), so the existing RELATIONSHIPS (pending) and QUANTITIES (deliberate) tests already cover the mechanism for both allowlist statuses — the two now-pointless duplicates are deleted rather than repointed.

Per-entry evidence the divergence is closed (verified against current main):

• apps/server/src/services/data_model/spatial.rs now matches "IFCSPATIALZONE", "IFCFACILITYPARTCOMMON", "IFCMARINEPART" in is_spatial_type, matching packages/data/src/spatial-types.ts's SPATIAL_STRUCTURE_TYPE_ENUMS.
• apps/server/src/services/data_model/properties.rs now has an "IFCCOMPLEXPROPERTY" => { ... } arm, matching packages/parser/src/property-value-parser.ts's handling.

Does not touch the module-size ratchet or boolean/mod.rs — that's a separate, unrelated failure being addressed by #4101.

Test plan

• node scripts/check-server-browser-type-parity.mjs → OK, 7 remaining allowlist entries (6 pending, 1 deliberate) correctly counted
• node --test scripts/check-server-browser-type-parity.test.mjs → 42 passing (44 before this change, minus the two deleted duplicate fixtures)
• node scripts/check-module-size.mjs → OK
• node scripts/check-test-wiring.mjs → OK
• node scripts/check-source-text-assertions.mjs → OK

No changeset: this is a scripts/-only change to a lint's allowlist, not a published package; comparable prior PRs to this same checker (#3979, and the fix that added the staleness detector itself) carried none.

unqueued: there is no tracking issue for this — it's an emergent CI-unblock from #3979's staleness detector correctly firing after #3971/#3973 merged.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 2 billable files and costs up to $0.50.}

Or wait 7 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 5fec8781-faba-48b4-8bce-2537de013b1c

📥 Commits

Reviewing files that changed from the base of the PR and between 49edb1e and 65c60cd.

📒 Files selected for processing (2)

• scripts/check-server-browser-type-parity.mjs
• scripts/check-server-browser-type-parity.test.mjs

---

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

[BIMvoice]

This overlaps #4101 — you likely only need one of them, and #4101 is probably it.

I opened this to unblock Node tests, which is failing on every PR rebased onto today's main because #3979's new staleness detector correctly reports four expired mutes. Then I checked #4101's file list:

rust/geometry/src/processors/boolean/chain_cycle_tests.rs
rust/geometry/src/processors/boolean/mod.rs
rust/geometry/src/processors/boolean/router_impl.rs
scripts/check-server-browser-type-parity.mjs
scripts/check-server-browser-type-parity.test.mjs

It already carries the same prune. So #4101 clears both red gates on main by itself — the module-size ratchet (boolean/mod.rs 948 → 920 against a 936 budget) and this parity staleness — while this PR only clears the second.

I'd suggest closing this in favour of #4101 unless you specifically want the two fixes separated for review or revert granularity. I'm not closing it myself; it costs nothing to leave open if you'd rather land them independently.

For the record, the verification here stands on its own either way — each of the four entries was confirmed genuinely closed on both sides before deletion, rather than deleted to get green:

• spatialTypes:IFCSPATIALZONE / IFCMARINEPART / IFCFACILITYPARTCOMMON — apps/server/src/services/data_model/spatial.rs's is_spatial_type now matches all three, aligning with packages/data/src/spatial-types.ts's SPATIAL_STRUCTURE_TYPE_ENUMS. Closed by fix(server): promote a contained (not aggregated) spatial element into its own hierarchy node #3973.
• properties:IFCCOMPLEXPROPERTY — apps/server/src/services/data_model/properties.rs now has an "IFCCOMPLEXPROPERTY" => { ... } arm, matching the TS parser's existing handling. Closed by fix(server): extract IfcComplexProperty and stop dropping a partially-resolved pset #3971.

Checker after: OK, with 6 pending ... 1 deliberate remaining, each re-verified as still genuinely divergent. Tests 44 → 42, the difference being two allowlist fixtures that no longer pin anything now that no spatialTypes/properties entry survives — suppression is concept-agnostic and the surviving RELATIONSHIPS (pending) and QUANTITIES (deliberate) tests still cover both statuses.

Worth noting the detector earned its keep on day one: #3979 merged at 13:31, #3971 and #3973 merged seconds later and closed these divergences, and the mutes — still annotated open PR #3973 and open PR #3971 — went stale within minutes. Before #3979 nothing would have reported that.

[github-actions]

Claude review - no findings for 65c60cd60

Reviewed this diff and found nothing to flag.

[louistrue]

Heads-up before this goes further: #4101 already carries this exact fix, and it is further along.

Same two files, same four entries, same reasoning. #4101 was opened to fix a different red-main breakage (#3922 pushed rust/geometry/src/processors/boolean/mod.rs to 948 lines against its 936 ratchet budget), and when its CI came back the parity gate was red too, so the allowlist fix went on the same PR. All four required contexts on #4101 went green with both fixes in place.

Your analysis here is right and matches mine independently, which is worth something on its own. The reason to consolidate rather than land both is mechanical: whichever merges second conflicts with the first in the same two files.

My suggestion is to close this in favour of #4101, but I am not going to close someone else's PR. Two things I would rather you decide:

1. If you would prefer the parity fix to land as its own clean PR (which it arguably should be, since fix(geometry): split boolean/mod.rs back under its module-size ratchet #4101's stated purpose is the ratchet), say so and I will drop the parity commits from fix(geometry): split boolean/mod.rs back under its module-size ratchet #4101 and let this one carry it. fix(geometry): split boolean/mod.rs back under its module-size ratchet #4101 would then merge first for the ratchet, and this rebases cleanly on top.
2. Otherwise close this one once fix(geometry): split boolean/mod.rs back under its module-size ratchet #4101 lands.

One difference worth folding in either way. My first attempt at the test side repointed the two allowlist-suppression tests at surviving entries. Pre-flight review caught that the repointed tests were duplicates of tests already at check-server-browser-type-parity.test.mjs:131 and :236, which already covered both the pending and deliberate statuses. So the honest fix was deletion plus a note on the surviving test, not a repoint. If this PR repoints rather than deletes, it has the same redundancy.

Separately: #4102 is a genuinely good catch and is not duplicated by anything of mine. That one should land.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated
ifc-lite-dev	Ignored		Sep 7, 2026 4:01pm UTC
ifc-lite-viewer-embed	Ignored		Sep 7, 2026 4:01pm UTC

[github-actions]

Claude review - nothing to review for 9a54a3bab

Every changed path in this diff is excluded from review: lockfiles, generated
code, snapshots, fixtures and build output.

The reviewer was NOT run, so this is not a statement that the diff is fine -- it
is a statement that nothing here was read. Another reviewer must NOT stand down
on this head.

[louistrue]

Closing as superseded by #4101, which merged at 15:44 and carries this exact fix.

Credit where it is due: you diagnosed this independently and correctly. Same four entries, same root cause, same reading of why a stale allowlist row is worse than clutter. Two of us arriving at the same analysis from different directions is a good sign about the analysis, and your write-up named the mechanism precisely: #3979 shipped the gate with an allowlist of expected divergences, then #3971 and #3973 closed four of them, and the staleness detector correctly refused entries that were by then muting nothing.

The only reason yours is the one closing is sequencing. #4101 existed for a different red-main breakage (#3922 took rust/geometry/src/processors/boolean/mod.rs to 948 lines against its 936 ratchet budget), its CI came back red on the parity gate too, and the allowlist fix went onto that PR because main could not go green without both. Whichever landed second was always going to conflict in the same two files.

For the record, since it is worth having somewhere: the version that landed deletes the gate's two allowlist-suppression tests rather than repointing them. My first attempt repointed them at surviving entries, and the pre-flight review caught that the repointed tests were duplicates of tests already at check-server-browser-type-parity.test.mjs:131 and :236, which between them already covered both the pending and deliberate statuses. So the honest fix was deletion plus a note on the surviving test recording where the old fixtures went. If your version repoints, it carries that same redundancy.

Nothing here was wasted: #4102, your Python-toolchain fix for the revert-oracle lane, is genuinely yours and is not duplicated by anything of mine. That one should land.

Reopen if you disagree with the consolidation.