chore: version packages - #3186
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe changes update package versions, Rust dependency requirements, a validation script, and changelogs for the 4.0.1, 1.15.50, 4.3.1, and 6.0.1 patch releases. The diff contains no runtime code changes. ChangesRelease metadata
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (3 skipped: 3 unsupported.) Comment |
1bf70e9 to
f360a57
Compare
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 |
8da8e99 to
7a744a7
Compare
04427ec to
e893985
Compare
… gate that would have caught one #3182 merged from a commit taken before its review passes finished. The fix itself is correct and unchanged here; what did not land is the review, and three of its findings were claims the code does not support. 1. The changeset says `IfcDoorStyle` and `IfcWindowStyle` were affected and implies this change fixes them. It does not. Both carry `has_geometry: false` in `legacy_entities.rs`, and `styling/prepass.rs:177` plus its siblings in `gpu_meshes/prepass.rs`, `processing/shard_classes.rs` and `processing/processor/mod.rs` gate type-geometry candidates on a bare `IfcType::from_str(name).is_subtype_of(IfcTypeProduct)`, which is false for any keyword IFC4X3 dropped. Both are discarded before a job exists, so they never reach the corrected line. The changeset text ships as the changelog and #3186 has not published yet, so this is correctable in place. #3187 tracks the nine sites. 2. `batch.rs` justified the unvalidated `content.get(start..end)` with "`content[start..end]` is the record `decode_and_cache` just parsed". False on the cache-hit path: `decoder.rs:443` returns the cached `Arc` without reading `start`/`end`, so only the miss path bounds-checks them. Narrowed to the span the job carries, with the fail-soft behaviour stated. 3. A test docblock said the six #3172 entities were "deliberately absent" because #3172 "is still in review". #3172 merged; the sentence was true when written and false on merge. The gate that would have caught the first one, derived rather than transcribed: `check-legacy-entity-coverage.mjs` already extracts the legacy arm keys and the generated `from_str` names from source and never intersected them. An arm whose key `from_str` already resolves makes the `Unknown` short-circuit in `legacy_aware_ifc_type_from_record` skip the remap, reopening exactly the wasm-vs-native divergence #3179 was filed for. Now checked, with a mutation case proving it fires. Also from the review: a Rust test pinning the same invariant behaviourally (it calls `from_str` for real, where the gate reads source text); a contract test for `extract_entity_type_name` next to the function rather than only in its caller's tests; one shared `LEGACY_KEYS` so the two opposite-direction tests cannot drift; a dead `type_end <= type_start` guard removed (`paren_pos >= 1` always, the one reachable equality is the empty slice `is_empty` already rejects); and three dead or duplicated items in `test-wasm-contract.mjs`. Every new test mutation-verified in both directions.
… gate that would have caught one #3182 merged from a commit taken before its review passes finished. The fix itself is correct and unchanged here; what did not land is the review, and three of its findings were claims the code does not support. 1. The changeset says `IfcDoorStyle` and `IfcWindowStyle` were affected and implies this change fixes them. It does not. Both carry `has_geometry: false` in `legacy_entities.rs`, and `styling/prepass.rs:177` plus its siblings in `gpu_meshes/prepass.rs`, `processing/shard_classes.rs` and `processing/processor/mod.rs` gate type-geometry candidates on a bare `IfcType::from_str(name).is_subtype_of(IfcTypeProduct)`, which is false for any keyword IFC4X3 dropped. Both are discarded before a job exists, so they never reach the corrected line. The changeset text ships as the changelog and #3186 has not published yet, so this is correctable in place. #3187 tracks the nine sites. 2. `batch.rs` justified the unvalidated `content.get(start..end)` with "`content[start..end]` is the record `decode_and_cache` just parsed". False on the cache-hit path: `decoder.rs:443` returns the cached `Arc` without reading `start`/`end`, so only the miss path bounds-checks them. Narrowed to the span the job carries, with the fail-soft behaviour stated. 3. A test docblock said the six #3172 entities were "deliberately absent" because #3172 "is still in review". #3172 merged; the sentence was true when written and false on merge. The gate that would have caught the first one, derived rather than transcribed: `check-legacy-entity-coverage.mjs` already extracts the legacy arm keys and the generated `from_str` names from source and never intersected them. An arm whose key `from_str` already resolves makes the `Unknown` short-circuit in `legacy_aware_ifc_type_from_record` skip the remap, reopening exactly the wasm-vs-native divergence #3179 was filed for. Now checked, with a mutation case proving it fires. Also from the review: a Rust test pinning the same invariant behaviourally (it calls `from_str` for real, where the gate reads source text); a contract test for `extract_entity_type_name` next to the function rather than only in its caller's tests; one shared `LEGACY_KEYS` so the two opposite-direction tests cannot drift; a dead `type_end <= type_start` guard removed (`paren_pos >= 1` always, the one reachable equality is the empty slice `is_empty` already rejects); and three dead or duplicated items in `test-wasm-contract.mjs`. Every new test mutation-verified in both directions.
9c5f82b to
64c2fa5
Compare
|
Converted to draft to hold the release until #3191 lands. Reversible in one click, and here is the exact reason with the evidence. This PR consumes
Verified on this PR's own diff rather than assumed: the sentence lands as an added line in #3191 rewrites that changeset: the list becomes illustrative rather than exhaustive, and the two gated-out names are dropped. Once #3191 is on main the changesets action refreshes this branch with the corrected text and this can go back to ready. Nothing here is a merge decision. Sequencing is Louis's call, and if he wants the release to go first, undrafting takes one command. |
… gate that would have caught one itself is correct and unchanged here; what did not land is the review, and three of its findings were claims the code does not support. 1. The changeset says `IfcDoorStyle` and `IfcWindowStyle` were affected and implies this change fixes them. It does not. Both carry `has_geometry: false` in `legacy_entities.rs`, and `styling/prepass.rs:177` plus its siblings in `gpu_meshes/prepass.rs`, `processing/shard_classes.rs` and `processing/processor/mod.rs` gate type-geometry candidates on a bare `IfcType::from_str(name).is_subtype_of(IfcTypeProduct)`, which is false for any keyword IFC4X3 dropped. Both are discarded before a job exists, so they never reach the corrected line. The changeset text ships as the changelog and #3186 has not published yet, so this is correctable in place. #3187 tracks the nine sites. 2. `batch.rs` justified the unvalidated `content.get(start..end)` with "`content[start..end]` is the record `decode_and_cache` just parsed". False on the cache-hit path: `decoder.rs:443` returns the cached `Arc` without reading `start`/`end`, so only the miss path bounds-checks them. Narrowed to the span the job carries, with the fail-soft behaviour stated. 3. A test docblock said the six #3172 entities were "deliberately absent" because #3172 "is still in review". #3172 merged; the sentence was true when written and false on merge. The gate that would have caught the first one, derived rather than transcribed: `check-legacy-entity-coverage.mjs` already extracts the legacy arm keys and the generated `from_str` names from source and never intersected them. An arm whose key `from_str` already resolves makes the `Unknown` short-circuit in `legacy_aware_ifc_type_from_record` skip the remap, reopening exactly the wasm-vs-native divergence #3179 was filed for. Now checked, with a mutation case proving it fires. Also from the review: a Rust test pinning the same invariant behaviourally (it calls `from_str` for real, where the gate reads source text); a contract test for `extract_entity_type_name` next to the function rather than only in its caller's tests; one shared `LEGACY_KEYS` so the two opposite-direction tests cannot drift; a dead `type_end <= type_start` guard removed (`paren_pos >= 1` always, the one reachable equality is the empty slice `is_empty` already rejects); and three dead or duplicated items in `test-wasm-contract.mjs`. Every new test mutation-verified in both directions.
62f685b to
c6ba073
Compare
… gate that would have caught one (#3191) itself is correct and unchanged here; what did not land is the review, and three of its findings were claims the code does not support. 1. The changeset says `IfcDoorStyle` and `IfcWindowStyle` were affected and implies this change fixes them. It does not. Both carry `has_geometry: false` in `legacy_entities.rs`, and `styling/prepass.rs:177` plus its siblings in `gpu_meshes/prepass.rs`, `processing/shard_classes.rs` and `processing/processor/mod.rs` gate type-geometry candidates on a bare `IfcType::from_str(name).is_subtype_of(IfcTypeProduct)`, which is false for any keyword IFC4X3 dropped. Both are discarded before a job exists, so they never reach the corrected line. The changeset text ships as the changelog and #3186 has not published yet, so this is correctable in place. #3187 tracks the nine sites. 2. `batch.rs` justified the unvalidated `content.get(start..end)` with "`content[start..end]` is the record `decode_and_cache` just parsed". False on the cache-hit path: `decoder.rs:443` returns the cached `Arc` without reading `start`/`end`, so only the miss path bounds-checks them. Narrowed to the span the job carries, with the fail-soft behaviour stated. 3. A test docblock said the six #3172 entities were "deliberately absent" because #3172 "is still in review". #3172 merged; the sentence was true when written and false on merge. The gate that would have caught the first one, derived rather than transcribed: `check-legacy-entity-coverage.mjs` already extracts the legacy arm keys and the generated `from_str` names from source and never intersected them. An arm whose key `from_str` already resolves makes the `Unknown` short-circuit in `legacy_aware_ifc_type_from_record` skip the remap, reopening exactly the wasm-vs-native divergence #3179 was filed for. Now checked, with a mutation case proving it fires. Also from the review: a Rust test pinning the same invariant behaviourally (it calls `from_str` for real, where the gate reads source text); a contract test for `extract_entity_type_name` next to the function rather than only in its caller's tests; one shared `LEGACY_KEYS` so the two opposite-direction tests cannot drift; a dead `type_end <= type_start` guard removed (`paren_pos >= 1` always, the one reachable equality is the empty slice `is_empty` already rejects); and three dead or duplicated items in `test-wasm-contract.mjs`. Every new test mutation-verified in both directions.
c6ba073 to
53be899
Compare
|
@coderabbitai full review Requesting a real review. This PR currently has zero reviews of any kind: Context for whoever reads this next: the PR is drafted deliberately, and CodeRabbit does not auto-review drafts by default, so this is a manual trigger. If no review appears, re-trigger; do not undraft to get one. |
|
|
53be899 to
f13e97b
Compare
Review confirmed for real, and all 12 findings rejected with evidenceThis PR had zero reviews of any kind — It produced a real review: 12 findings, all severity major. Not a decline. All 12 are the same finding, and all 12 are wrong here. Every one says some variant of:
Applied to Rejected, because this PR IS the release tooling's output. The guideline is correct and is being applied outside its domain:
Acting on any of the 12 would mean reverting the release. A bot being confident is not evidence. What I verified myself instead, since the bot found nothing realVersion bumps match the changesets, all patch, no stray majors. Every changeset declares This is worth checking rather than assuming: on the v6.0.0 release nine packages were declared The changelog text is correct now, and was not before. This PR previously added a sentence claiming That ordering mattered because a published CHANGELOG entry cannot be corrected afterwards. CI is settled, not merely un-red: 31 SUCCESS, 6 SKIPPED, zero failures, zero checks without a verdict. One risk to state before merging, pre-existing and not introduced hereMain verifies the npm publish after the fact ( If it recurs, the remedy is proven: re-run the failed job. It is idempotent and fail-closed. The swallowed-tag-push defect is fixed and on main (#3208, |
…d the version it actually ships from (#3310) * docs(changeset): state the with_style_metadata break by signature, and the version it actually ships from The `rep-item-identity-across-boundary` changeset generates the Rust-API paragraph of the next `@ifc-lite/cache`, `geometry`, `wasm` and `server-client` CHANGELOG entries, so it is held to the code's accuracy bar. Two things in it were not. `6.0.0 → 6.1.0` was wrong when written and is wrong now: #3186 had already bumped the Cargo workspace to `6.0.1`, and that commit is an ancestor of #3210, which added this changeset. `Cargo.toml` on `main` reads `version = "6.0.1"`, so the minor this changeset causes ships `6.0.1 → 6.1.0`. `rust/export/src/usd/tests.rs` demonstrates only ONE of the two breaks. Its whole diff in #3210 is `+ material_id: None,` inside a `MeshData` struct literal. The arity break is demonstrated by `rust/processing/src/element.rs`, whose call went from `with_style_metadata(material_name, geometry_item_id)` to `with_style_metadata(material_name, source_id, id_is_material)`. Both are now named, each against the file that shows it. The "two arguments to three" claim itself is CORRECT and is kept, restated as the two signatures so it cannot be misread as counting `self`: CodeRabbit asked for it to be removed as false (#3227), and the diff of `rust/processing/src/types/mesh.rs` in 50895fb says otherwise. No behaviour change; changeset prose only. * docs(changeset): the "nothing gates this" claim is no longer true The last sentence of the BREAKING paragraph read "Nothing gates this: there is no `cargo-semver-checks` anywhere in the repo." That was accurate when the paragraph was written and stopped being accurate at 08:15 today, when #3298 merged `scripts/check-rust-semver.mjs` and its `Rust crate semver` lane; #3305 then added `rust-major-offset.json` at 14:56. `cargo-semver-checks` now appears 27 times across `.github/workflows/` and `scripts/` on main, so the sentence asserts the absence of something the reader can grep and find. Shipping it would put a false claim in the published changelog, in the one paragraph whose entire job is to be accurate about a break the changeset format cannot express -- and this PR exists only to make that paragraph accurate. Replaced with what the gate actually does, checked against the source rather than the PR description: it compares the required bump with the bump the derived version carries over the crate's latest crates.io release and fails on the smaller, its documented lint set covers BOTH breaks this paragraph names (a field added to a `pub` struct callers construct literally, and a changed argument count), it runs on PRs and again before publish, and the remedy for a Rust-only major is the committed offset. The two claims this PR does add both verify and are untouched: `with_style_metadata(self, material_name, source_id, id_is_material)` is the live signature at `rust/processing/src/types/mesh.rs:268`, three caller-supplied arguments; and 6.0.1 is the highest npm workspace version, so `6.0.1 -> 6.1.0` is the bump a `minor` here derives. Refs #3227
* Adding anonymizer-export for debug
* fix(parser): the last-resort schema scan folded Unicode, so `ıFC5` selected IFC5 (#3284) (#3315)
* Fold ASCII in the last-resort schema scan, and make its tests able to fail
Follow-up to #3297, which merged at the head I had pushed rather than the one
I had finished. Three commits did not make it, and one of them is a real fix
rather than polish, so this lands them.
THE FIX. `detectSchemaVersion`'s fallback uppercases the first 2000 bytes and
looks for `IFC5` / `IFC4X3` / `IFC4` / `IFC2X3` as substrings.
`'ı'.toUpperCase()` is `'I'`, so a FILE_DESCRIPTION mentioning `ıFC5` selects
IFC5 for a file that never said so. Still live on main at source-header.ts:294.
Same fold #3297 removed from the record scan, one function further down the
same file. The scan is deliberately loose -- it only runs when no FILE_SCHEMA
identifier resolves, and it already matches `IFC4` inside ordinary prose --
but loose is not a reason to accept a fold ISO 10303-21 does not use. A copy
is fine here where it was not in the record scan, because nothing takes
offsets from it.
THE TESTS THAT COULD NOT FAIL. My first two tests for this did not exercise
the fold at all: one asserted the trailing IFC4 default, which passes for any
implementation that fails to match, and the other fed input already
upper-case. An identity mutant on the helper was killed by ZERO tests across
the whole parser suite. There is now a case that drives the direction the fold
exists for, lower-case `ifc4x3` in prose, plus one for the subtler mutant that
DROPS non-ASCII rather than passing it through: deleting a character joins the
fragments either side, so `IFCı5` becomes `IFC5`, a match built from a
character that was never in the word.
Off-by-one bounds on the fold survive and are left alone deliberately. The
output is consumed only by `.includes()` on tokens whose letters are i, f, c
and x, so neither `a` nor `z` can appear in a match and nothing through the
public surface can distinguish them.
A FALSE CLAIM, replacing a stale one. #3297 rewrote a comment in
`schema-version-detection.test.ts` that wrongly said `detectSchemaVersion` is
module-private, and replaced it with a different wrong claim: that
`buildStep()` can never reach the last-resort scan. It always emits a
FILE_SCHEMA record but not always a RESOLVABLE one, and the `IFC2X2` case
falls through to the scan. Proven by putting a throw at the top of the scan
and watching only that test go red.
Also: `schema_detect.rs` uses the crate's SPDX one-line header like every
sibling, both changeset fences declare a language, and the changeset says the
`ıFC5` input falls through to the IFC4 default rather than "no longer selects
a schema", since `detectSchemaVersion` always returns one.
Verified by exit code: parser 849, rust export 0, typecheck 0, lint 0,
module-size 0. Mutation-verified: restoring `toUpperCase()` reddens the new
test and only it.
* Drop the license-header change, and say what the fold gives up
Preflight came back clean on the fix itself and raised two small things.
The SPDX header swap on `schema_detect.rs` has nothing to do with the ASCII
fold, so it is out. It was a CodeRabbit suggestion I took on the original
branch, and it is defensible -- 52 of 54 files in `rust/export/src` already
use the one-line form -- but `LICENSE_HEADER.md` still documents the block
comment as required for `.rs`, and `scripts/add-license-headers.mjs` matches
only that form. So the repo has an in-flight migration with a stale doc and a
stale script, and quietly adding one more file to the wrong side of it in a
parser fix is not the way to settle that. Filing it separately.
The changeset now says what the fold costs rather than only what it fixes: a
Turkish-locale `ıfc4x3` in free header prose used to resolve and no longer
does. It is the same character as the false positive being removed, pointed
the other way, and a reader of release notes should see both. ISO 10303-21
tokens are ASCII and this scan only runs for a file that declares no
resolvable schema, so the trade is worth making, but it is a trade.
Also `source-header.test.ts`'s own docstring claimed the file is direct
coverage for `parseSourceHeader`. It now tests `detectSchemaVersion` too, and
the sibling comment in `schema-version-detection.test.ts` -- rewritten in this
same work -- points at it for exactly that. The two now agree.
Verified by exit code: parser 849, typecheck 0, lint 0, module-size 0.
* Two corrections to what #3297 shipped, both found by the CLI after it merged
Neither blocked that merge; both are mine.
A DOC THAT SURVIVED ITS OWN MECHANISM. `find_unquoted`'s comment sends the
reader to `last_comment_close` for the linearity argument and describes the
closer search as HOISTED. I replaced that design mid-branch with a deferred
search and a `no_closer` memo on `Lex`, and deleted the function, but the
comment two files away still described the old shape. `grep -rn "fn
last_comment_close" rust/export/src/` returns nothing.
That is exactly the failure #3284 is about, committed by the fix for it: a
comment invalidated at a distance by a refactor, still confidently describing
a mechanism that no longer exists. Now it names the memo and says why the memo
is what makes the bound hold.
AN ASSERTION THAT COULD NOT TELL TWO ANSWERS APART.
assert!(h.is_none() || h.unwrap().schema_identifiers.is_empty());
passes whether the reader REJECTS the malformed `FILE_SCHEMA\u{00A0}(...)` or
ACCEPTS it and returns an empty list. The comment directly above claims the
first. So the test agreed with itself either way, and if the reader ever began
accepting that record it would still be green.
It returns None today, so that is what is pinned now. Mutation-verified rather
than assumed: teaching `skip_trivia` to treat 0xC2, the UTF-8 lead byte of
U+00A0, as whitespace makes the reader accept the record, and the tightened
assertion reddens where the old one did not.
This also gives the PR a new head event, which it needs for a second reason:
`gh run list --branch fix/3284-followup-ascii-fold` returned NOTHING, so
test.yml never fired when the branch was pushed and the PR opened. Seven lanes
registered, none of them a test lane, and the required aggregate absent. Same
shape as #3294, on my own PR, which is what #3313 exists to catch.
Verified by exit code: cargo test -p ifc-lite-export 0.
* fix(ci): close the path-filter holes, and gate the class that made them (#3312) (#3314)
* fix(ci): close the path-filter holes, and gate the class that made them (#3312)
A CI gate is only as good as the job that runs it, and that job only runs when
the path filter says so. When a gate's INPUT sits outside its own TRIGGER the
gate is not weak, it is unreachable: the PR that introduces the very mistake it
guards against is the PR the job skips, and a skipped job counts as success in
the aggregate `test` gate, so the required check goes green.
Four instances, each reproduced against a real merged PR or the wiring itself:
- `scripts/check-swallowed-push.mjs` declares its SCOPE to be
`.github/workflows/**` and ran in Node tests, which only `test.yml` and
`server-binaries.yml` could trigger -- unreachable on 11 of the 13 files it
guards. PR #3118 edited `release.yml` and `docker.yml`; Node tests SKIPPED.
- `pnpm test:integration` runs `tests/integration.test.ts`, which was in no
filter: the test could not trigger its own execution.
- `scripts/docs/generate-docs-sections.mjs --check` regenerates from
`tests/benchmark/baseline.json` and `apps/landing/app.jsx`, neither in any
filter. PR #1817 changed only `apps/landing/bench-data.json`; Node tests AND
Docs checks both skipped and the required check reported success.
- `apps/landing/**` was in no filter at all.
`tests/extensions/**` did NOT reproduce and is not fixed here: `sdk-canary.yml`
carries `tests/extensions/canaries/**` in its own `paths:`.
THE FIXES, each in the cheapest filter that reaches the gate:
frontend += `.github/workflows/**` (subsumes the two individual entries it
had), `tests/integration.test.ts`, `tests/tsconfig.json`
docs += `apps/landing/**`, `tests/benchmark/baseline.json`
`docs` rather than `frontend` for the last two on purpose: it reaches the same
`--check` through the free Docs-checks job -- one ubuntu-latest runner, three
node scripts, no build artifact and no Depot -- instead of dragging a
landing-copy edit through build + typecheck + lint + the viewer shards. Coverage
holds either way, because a PR that also touches frontend/rust makes Docs checks
skip itself and Node tests runs the same check. The workflow addition is the one
that costs: a workflow-only PR now runs the JS lane. Every job it adds is free
except `build`, which reaches Depot only when the WASM source has drifted from
the published release tag -- the same condition every frontend PR already pays.
THE DURABLE PART is `scripts/check-ci-path-coverage.mjs`, which derives gate
inputs from the gate scripts and fails when one is outside its trigger. For
every workflow it reads which `node scripts/...` gates each job runs, the globs
that can trigger that job, and the repo paths each gate reads out of its own
source; then it reports every path a gate reads and no glob can reach.
It fails closed. No workflows, no PR-triggered jobs, no gates, a filter block
that parses to nothing, a referenced gate script that is missing, a job gating
on an undefined filter, a missing allowlist, an exemption with no written
reason, an exemption that stopped matching, zero derived inputs -- each is a
NAMED failure, never a pass. Its own config is inside its own trigger, proved by
`assertSelfCoverage` rather than asserted in prose. `REQUIRED_COVERAGE` pins the
six specific facts above by name, because a count floor survives dropping the
one entry that matters.
`check-ci-path-coverage.test.mjs` is the executable proof: 27 tests covering
every fail-closed path, the glob and parsing semantics, and -- against a
symlink mirror of the real repo -- the removal of each of the four filter
entries, asserting the report names the specific file each time.
The 37 residue entries in the allowlist are each written out with a reason. The
one that is a trade rather than a technicality: the four gates that walk `apps/`
still cannot be triggered from `apps/landing`, because closing that needs
`apps/landing/**` in `frontend`, which the filter block already declines to do.
The walk matches zero files there today -- apps/landing ships unbuilt .jsx/.html
/.css with no TypeScript, no test file and no WASM handle. If TypeScript lands
there, the exemption is wrong and the lines say so.
Refs #3312
Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
* fix(ci): correct the workflow counts and re-file a non-hole in the path-coverage gate
Review findings on #3314, all accuracy rather than behaviour. A gate about
reachability has to be accurate about what it measured.
- `ls .github/workflows | wc -l` is 15, not 13. Two prose counts corrected:
"2 of the 13" -> "2 of the 15" in the gate's docblock, and "unreachable on
11 of the 13 files it guards" -> "13 of the 15" in test.yml (15 workflows,
2 named in the filter at the time, so 13 unreachable, not 11).
- The step scan matches only a literal `node scripts/*.mjs` in a `run:`, so a
gate invoked through a package script is outside the census: `pnpm lint`
runs four of them, and `check:vitest-timeout-audit` and `fixtures:check` run
one each. All six were walked by hand and none is outside its own trigger
today, so this is a stated LIMIT, not a fix. Recorded in the docblock rather
than left for the next reader to rediscover.
- Section 3 of the allowlist is headed "Real holes", and one entry was not one.
`check-report-numerals.mjs` carries
`relRaw.startsWith('scripts/') || relRaw.startsWith('docs/')`; the derivation
keeps the bare `scripts` and `docs` those normalise to. Its real roots are
`VISION_DIR = 'docs/vision'` and the bet directories under scripts/moonshot,
and moonshot.yml's `on.pull_request.paths` carries `docs/vision/**` and
`scripts/moonshot/**` -- so moonshot.yml has no hole here. Moved to section 1
(PREFIX FRAGMENTS), which is what it is.
Both entries still match (a stale exemption is a named failure), and deleting
them still reopens the 124-input report, so the re-filing is a relabel and not
a weakening.
Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
* fix(ci): the path-coverage verdict must be a function of the commit (#3312)
The new check passed on a clean checkout and failed in CI on the IDENTICAL
commit -- 15 findings, nine of them the single word `node_modules`. Its input
derivation and its tree walk both read the WORKING tree, so whatever happened
to be on disk changed the answer: `node_modules` after an install, a package's
`dist` after a build, the fetched `.ifc` corpus under `tests/models` after the
fixture cache warmed. None of those are committed, so none of them can ever be
what a `paths:` filter matches -- but all three were being reported as gate
inputs outside their own trigger.
The old skip set was the near miss: it filtered a walk's CHILDREN and never the
root node the walk was asked about, so `node_modules` as a derived input
enumerated the whole install.
The walk and the derivation now both exclude what `.gitignore` excludes, read
from the committed file rather than by shelling out to `git check-ignore`, so
the synthetic trees in the harness -- which are not repositories -- run the same
exclusion the real repository runs rather than a second behaviour nothing
tests. `gitignoreToGlobs` refuses negations and escapes instead of dropping
them, because a silently dropped pattern is a tree the walk wanders back into.
`.gitignore` is consequently an INPUT to this gate, and the gate said so on the
first run: an edit to it can turn a covered path into an uncovered one. Added
to the `frontend` filter, which is the cheapest one reaching Node tests.
Tests: five over `gitignoreToGlobs` (depth, anchoring, the zero-directory
`a/**/b` case that kept the corpus visible, trailing slash, negation refused),
one pinning the real ignore file translates and still admits `manifest.json`,
and two end-to-end -- an installed `node_modules` and a warmed fixture cache
must each leave the report BYTE-IDENTICAL. Mutation-checked: reverting the
derivation to the bare `exists` predicate turns the `node_modules` one red.
* fix(ci): the trigger parser must refuse a shape it cannot read (#3312)
CodeRabbit's finding on scripts/lib/ci-path-coverage.mjs, verified against the
branch rather than taken on the description. Both halves reproduce.
An INLINE list -- `paths: ['rust/**']` -- returned `{ paths: null }`. The block
matcher requires an empty tail after the colon, so an inline list fell through
to the "some other key" branch and left `paths` at its initial `null`. `null`
is not a degraded answer here, it is the OPPOSITE answer: the caller reads it
as "this workflow triggers on every path", the widest coverage claim there is,
asserted about a workflow that is in fact narrowly filtered. Every gate input
under such a workflow would look reachable. That is the precise defect class
this check exists to find, in the check itself.
An UNQUOTED entry inside a recognised block was silently omitted. That one errs
the safe way -- a short trigger list under-claims coverage and over-reports --
but a finding derived from a truncated list is indistinguishable from a real
hole, so it throws too.
Both now throw, and the checker catches at the workflow boundary and NAMES the
file, because these parsers throwing IS the check firing and an uncaught stack
trace leaves the reader to work out which of 26 workflows produced it.
Tests: the three refusals, plus one asserting the block, `paths-ignore` and
no-`paths` forms still parse -- the refusals must not have been bought by
refusing everything -- plus an end-to-end run over a mirror carrying an
inline-`paths` workflow, asserting the report names it and is not a stack.
Mutation-checked: disabling either refusal turns the matching test red.
* feat(export): native merged/federated IFC export at parity with the JS MergedExporter (#2951) (#2952)
* refactor(export): split merged.rs into a merged/ module
Move the monolithic merged.rs into merged/mod.rs and its tests into
merged/tests.rs (via the sibling mod tests; include the house pattern
uses), with no logic change. This creates the module directory the native
merged-export parity work (#2951) lands its submodules into.
* feat(export): native merged-export parity — GlobalId reconciliation, spatial merge, visibility (#2951)
Bring the native Rust merged exporter (rust/export/src/merged) up from the
id-offset-only "P1" to feature parity with the JS MergedExporter, so a native
consumer can federate models entirely in Rust without materializing the merge
in a webview JS heap (the OOM class this addresses).
- guid.rs: deterministic 22-char GlobalId minter (byte-identical to the JS
deterministicGlobalId, pinned against golden values) + rooted-entity
detection denylist + read/replace helpers. Duplicate GlobalIds are now
unified (same unit space) or re-stamped (relationships / federated), so a
merged file no longer carries duplicate GlobalIds.
- spatial.rs: match IfcSite / IfcBuilding / IfcBuildingStorey onto the first
model by name / elevation (single / by-name / by-elevation /
by-name-then-elevation, +-0.5-unit tolerance).
- plan.rs: per-model index, visibility forward-reference closure, reference
rewriting, and redundant-IfcRelAggregates pruning.
- units.rs: length-scale resolution + compatibility.
- mod.rs: orchestrator wiring project/infra unification, spatial merge,
GlobalId reconciliation, per-model visibility, and unit handling into
export_merged_models, plus extended MergedOptions / MergedStats.
Cross-unit rescaling (unitReconciliation 'normalize') is deferred: an
incompatible-unit model is federated (never silently mis-scaled) and
MergedStats.unit_rescale_required is set so the caller can gate that case to
the JS path — permitted as a first-iteration limitation by the spec.
cargo test -p ifc-lite-export and the workspace clippy gate are clean.
* test(export): add merge_ifc example harness for large federations
A runnable harness that reads several IFC files from disk, merges them
natively via export_merged_models, writes one .ifc, and self-checks the
result (duplicate GlobalIds, dangling references, unified IfcProject).
This is the native path a webview-embedding consumer would drive instead
of the JS MergedExporter, and the tool used to confirm a ~1.6 GB / 11-model
federation merges without the WebView2 out-of-memory crash (#2951).
* fix(export): address PR review on the native merged exporter (#2951)
Five reviewer findings on the merged export, verified against the code and fixed:
- Filtered canonical targets dangle (Greptile P1 / CR): canonical_project,
first_infra and spatial_lookup were derived from the COMPLETE first model, so
when models[0].included excludes its project / unit / a spatial container,
later models still redirected refs onto those never-emitted ids. Now the
first-model merge targets are filtered through resolve_included; an excluded
canonical simply isn't a target and later models keep their own.
- Schema conversion duplicates GlobalIds (Greptile P1): a downgrade with no
target type falls back to IFCPROXY with placeholder_guid(id). Two models
sharing a source-local id seeded the same GlobalId. Pass the OFFSET id so the
proxy guid is globally unique (and consistent with the line's offset #id).
- Within-model mint collision (CR): GuidMinter::mint only checked prior models'
emitted guids + its pending set, so a re-stamped guid could collide with an
unchanged guid in the SAME model. mint now takes the current model's
local_guids as an extra exclusion set.
- Type-aware GlobalId classification (CR): the harness/tests counted the first
22-char quoted value as a GlobalId, misclassifying non-rooted entities that
lead with a charset Name (IfcColourRgb, ...). Added those types to the
rooted-entity denylist and a public leading_rooted_global_id helper (single
source of truth with the merge's own extract_global_id_fast); the harness and
tests use it. Regression coverage added.
- Harness federated project count (CR): the self-check failed valid federated
output (projects <= 1); it now expects 1 + federated_model_count.
cargo test -p ifc-lite-export (36 merged tests) and the workspace clippy gate
pass.
* refactor(export): keep merged/mod.rs under the module-size ratchet (#2951)
The merged/ split left mod.rs at 425 lines, over the 400-line ratchet with no allowlist row (the failing rust-tests gate). Move the plan-building helpers (ModelPlan, PlanCtx, build_plan, reconcile_global_ids, model_salt) into plan.rs — their natural home beside ModelIndex/unify_spatial — dropping mod.rs to 303, and extract plan.rs's inline tests into a sibling plan_tests.rs (exempt via the _tests.rs suffix) so plan.rs stays at 374. Prefer splitting over allowlisting per AGENTS.md.
* fix(export): address second-round PR review on the native merged exporter (#2951)
Resolve the remaining CodeRabbit/Greptile findings on #2952:
- Schema-based rootedness: replace the hand-maintained non-rooted denylist with
`is_rooted_entity_type` (`legacy_aware_ifc_type(..).is_subtype_of(IfcRoot)`),
mirroring the JS exporter's IfcRoot inheritance check. A non-rooted resource
leading with a 22-char Name (IfcColourRgb, IfcMaterialLayer,
IfcRegularTimeSeries, IfcSimpleProperty) is no longer misread as a GlobalId.
- Within-model duplicate GlobalIds: reconcile every model (including the first)
and track seen local GlobalIds, so two rooted entities in one model sharing a
GlobalId re-stamp the later occurrence instead of emitting duplicates.
- Placeholder GUID collision: a schema-conversion IFCPROXY placeholder is minted
after reconciliation, so re-stamp it at emit time if it collides with an
already-emitted GlobalId.
- AssumeShared effective scale: store `primary_scale` (not the model's own), so
a later model's shared GlobalId unifies rather than failing the units gate.
- EXPRESS id overflow: guard the cumulative offset with checked_add (stop and
report `unmerged_model_count` instead of wrapping ids), and saturate ref-id
parsing in rewrite_refs so a malformed wide ref can't wrap onto a valid id.
- Tests: full project->site->building->storey aggregation in the fixture with a
remapped-endpoint assertion; per-unit-policy entity-count assertions; and
regressions for within-model dup GlobalIds, AssumeShared cross-unit unify,
the overflow guard, the schema rooted check, and the mint `also` collision.
* fix(export): tighten overflow bound and preserve source GlobalId on conversion (#2951)
Two further review findings on #2952:
- Capacity bound from VISIBLE entities: resolve `included` before the id-space
overflow check and bound `checked_add` on the largest visible id, not
`index.max_id`. An excluded near-max id no longer consumes id space or omits
a later model that would actually fit.
- Keep the source GlobalId reachable after schema conversion: when a rooted
entity is downgraded to an IFCPROXY placeholder, also map its SOURCE GlobalId
onto the final id (via `entry().or_insert`, never overwriting a re-stamped
duplicate), so a later compatible model carrying it unifies instead of emitting
a second proxy.
- Regressions for both: an excluded max id followed by a fitting model, and two
IFC4X3 models sharing an IfcAlignmentSegment GlobalId converted to IFC4.
* fix(export): cover legacy IFC2X3 rooted types the schema check misses (#2951)
is_rooted_entity_type used only `legacy_aware_ifc_type(..).is_subtype_of(IfcRoot)`,
which reaches `legacy_entities.rs` (21 element/geometry legacy names) then the
generated schema. But 38 rooted IFC2X3 resource types (IfcElectricalCircuit,
IfcCondition, IfcRelAssignsTasks, IfcServiceLife, IfcTimeSeriesSchedule, ...)
were dropped from IFC4X3 and are absent from both, so they resolve to
IfcType::Unknown and were classified as non-rooted. Their GlobalId then never
entered reconciliation and two models sharing one emitted it twice -- silent
GlobalId duplication, exactly the case the legacy rooted table exists to prevent
(reported on #2952; louistrue's "port both halves or neither").
Add a legacy IFC2X3 rooted-type fallback (`is_legacy_rooted_type`, the 54-entry
set kept in agreement with the JS exporter's IFC2X3 coverage), consulted only
when the schema does not recognise the type. The schema check stays primary, so
a type re-entering the generated schema simply stops reaching the fallback.
Split the guid.rs tests into a sibling guid_tests.rs (house pattern) so the
production module stays under the module-size ratchet. Regressions: unit-level
classification of six dropped rooted types, and an end-to-end merge of two
IFC2X3 models sharing an IfcElectricalCircuit GlobalId (emitted exactly once).
* test(export): re-adapt the two #3083 merge fixtures to the module's semantics
A second merge of origin/main re-took main's monolithic merged_tests.rs versions
of these two fixtures verbatim, undoing the adaptation from the first merge, so
they failed against the merged/ module (342 pass, 2 fail, reported on #2952).
Neither is a bug in the exporter -- both fixtures encoded main's old merged.rs
behaviour, which differs from this module:
- later_models_project_ref_redirects_to_the_first_models_project: the old
fixture related the later project to ITSELF (#7,(#7)); after the project is
unified BOTH endpoints point at it, so redundant-aggregation pruning correctly
drops the row -- which read as the redirect vanishing. Relating the project to
a DISTINCT wall keeps the row (only fully-unified aggregations are pruned) so
the redirect onto model A's project id stays observable. Verified directly:
the kept aggregation emits #1 (model A's project), not #7 nor its offset image.
- merge_mints_distinct_ids_for_collisions_within_the_same_model: this module
unifies the first cross-model duplicate (same unit space) and re-stamps only
the remaining within-model duplicates, so four rooted entities survive, not
five. The real invariant is unchanged -- every emitted GlobalId is distinct.
Full crate green, clippy clean.
---------
Co-authored-by: Louis Trümpler <78563314+louistrue@users.noreply.github.com>
* docs(changeset): state the with_style_metadata break by signature, and the version it actually ships from (#3310)
* docs(changeset): state the with_style_metadata break by signature, and the version it actually ships from
The `rep-item-identity-across-boundary` changeset generates the Rust-API
paragraph of the next `@ifc-lite/cache`, `geometry`, `wasm` and
`server-client` CHANGELOG entries, so it is held to the code's accuracy bar.
Two things in it were not.
`6.0.0 → 6.1.0` was wrong when written and is wrong now: #3186 had already
bumped the Cargo workspace to `6.0.1`, and that commit is an ancestor of #3210,
which added this changeset. `Cargo.toml` on `main` reads `version = "6.0.1"`,
so the minor this changeset causes ships `6.0.1 → 6.1.0`.
`rust/export/src/usd/tests.rs` demonstrates only ONE of the two breaks. Its
whole diff in #3210 is `+ material_id: None,` inside a `MeshData` struct
literal. The arity break is demonstrated by `rust/processing/src/element.rs`,
whose call went from `with_style_metadata(material_name, geometry_item_id)` to
`with_style_metadata(material_name, source_id, id_is_material)`. Both are now
named, each against the file that shows it.
The "two arguments to three" claim itself is CORRECT and is kept, restated as
the two signatures so it cannot be misread as counting `self`: CodeRabbit
asked for it to be removed as false (#3227), and the diff of
`rust/processing/src/types/mesh.rs` in 50895fb5b says otherwise.
No behaviour change; changeset prose only.
* docs(changeset): the "nothing gates this" claim is no longer true
The last sentence of the BREAKING paragraph read "Nothing gates this: there is
no `cargo-semver-checks` anywhere in the repo." That was accurate when the
paragraph was written and stopped being accurate at 08:15 today, when #3298
merged `scripts/check-rust-semver.mjs` and its `Rust crate semver` lane; #3305
then added `rust-major-offset.json` at 14:56. `cargo-semver-checks` now appears
27 times across `.github/workflows/` and `scripts/` on main, so the sentence
asserts the absence of something the reader can grep and find.
Shipping it would put a false claim in the published changelog, in the one
paragraph whose entire job is to be accurate about a break the changeset format
cannot express -- and this PR exists only to make that paragraph accurate.
Replaced with what the gate actually does, checked against the source rather
than the PR description: it compares the required bump with the bump the
derived version carries over the crate's latest crates.io release and fails on
the smaller, its documented lint set covers BOTH breaks this paragraph names
(a field added to a `pub` struct callers construct literally, and a changed
argument count), it runs on PRs and again before publish, and the remedy for a
Rust-only major is the committed offset.
The two claims this PR does add both verify and are untouched:
`with_style_metadata(self, material_name, source_id, id_is_material)` is the
live signature at `rust/processing/src/types/mesh.rs:268`, three
caller-supplied arguments; and 6.0.1 is the highest npm workspace version, so
`6.0.1 -> 6.1.0` is the bump a `minor` here derives.
Refs #3227
* fix(viewer-embed): apply ?hideAxis= and ?hideScale= instead of only parsing them (#3316)
* fix(viewer-embed): apply ?hideAxis= and ?hideScale= instead of only parsing them
`parseUrlParams` accepted `?hideAxis=true` and `?hideScale=true`, stored them
on `EmbedUrlParams`, and nothing ever read them: a grep for `urlParams.hideAxis`
/ `urlParams.hideScale` across `apps/viewer-embed` matched the parser and its
own test, nothing else. `ViewportOverlays` took a single `hideViewCube` prop and
drew the scale readout and the axis helper unconditionally, so an embed that
asked for a bare viewport still got both.
`hideViewCube` — the fourth sibling, and the one that was wired — is the pattern
followed here: a prop on `ViewportOverlays` guarding the JSX, passed from the
embed's single call site. Both flags default to `false`, so the standalone
viewer renders exactly as before.
The guards drop their own item only. `BasepointToggleButton` shares the same
bottom-left column and stays reachable with both flags set; the scale
subscription (`setOnScaleChange`) is still registered when `hideScale` is on,
matching `hideViewCube`, which likewise leaves `setOnCameraRotationChange` in
place. With `hideAxis` on, `axisHelperRef.current` stays null and the rotation
callback's `axisHelperRef.current?.updateRotation(...)` is a no-op.
The new test renders the REAL `ViewportOverlays` inside the real `EmbedViewer`
(the sibling URL-param test mocks the overlays out) and asserts on the DOM the
embed produces. Every case asserts BOTH directions — the other overlay is still
present — and the no-param case asserts both are, so an implementation that
hides them always fails rather than passes.
`?controls=` is left parse-only deliberately: its four values are not pinned to
observable behaviour anywhere in the repo or the SDK docs, and guessing one
would be inventing protocol. Refs #2934.
Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
* chore: retrigger CI
The required `parity (in-tree fixtures, committed reference)` check never
reported on c367123: its workflow run ended in `startup_failure` with zero
jobs during the GitHub Actions dispatch outage, and a startup failure cannot
be re-run. The branch is already current with main, and the quick parity job
is gated to `github.event_name == 'pull_request'`, so workflow_dispatch
cannot report the required context either.
Tree is byte-identical to c367123.
* fix(scripts): derive the test-wiring remedy from the workflow, and un-binary a gate script (#3319)
Two independent findings, both "a thing that looks fine because nobody can
see it".
1. check-test-wiring's 2b remedy line named `scripts/*.test.mjs` and
`scripts/lib/*.test.mjs` as the directories the workflow glob catch-all
reaches. That pair was correct when #3038 wrote it; the catch-all in
test.yml has since grown `scripts/fixtures/*.test.mjs` and
`scripts/docs/*.test.mjs`, and the sentence did not follow. A developer
whose new test was flagged was being told two of the four directories that
would have fixed it. The verdict was always right — only the advice drifted,
which is why nothing caught it.
The checker already computes the exact set (`testRunnerTargets` ->
`globDirs`) to decide the verdict. It now returns that set and the message
prints it, so the advice and the verdict read the same value and cannot
disagree again. An empty set (no catch-all anywhere) prints its own remedy
rather than an empty list. The header comment's directory list is likewise
marked as not being the source of truth.
Pinned by a regression case that gives the fixture a wider catch-all than
the hard-coded pair and asserts every covered directory appears in the
remedy; it reds against the old string.
2. scripts/moonshot/ci/check-report-numerals.mjs held two RAW NUL bytes, used
as a composite-key separator in `${token}<NUL>backed`. That is the whole of
what made git and grep classify the file as binary: `grep -c const` printed
nothing while `grep -ac const` printed 200, so an ordinary search of this
repo silently reported "not found" for anything in this file. Written as the
`\0` escape instead, the string built at runtime is the same string — the
source now differs from the old file only by that substitution, and the
script's 1880-line output is byte-identical before and after.
Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
* fix(ifcx): export each entity's own IFC class, and keep IFC4.3 facility levels in the spatial tree (#3318)
* fix(ifcx): export each entity's own IFC class, and keep IFC4.3 facility levels in the spatial tree
The IFCX writer decoded an entity's typeEnum through a 26-row enum->class
table written by hand. IfcTypeEnum has 128 members and its numbering had
moved on since the table was typed, so the table was both incomplete and
SHIFTED against the enum it claimed to decode: 14 of its 26 rows named a
different class than the id actually holds. Running the real writer over an
entity table built from IfcTypeEnumToString:
IfcStair -> IfcRoof
IfcMember -> IfcPile
IfcDistributionElement -> IfcOpeningElement
IfcFlowSegment -> (no class written)
IfcRoad -> (no class written)
That is a wrong value written into an exported file: bsi::ifc::class is the
node's IFC identity, and every reader takes the class from there and nowhere
else. 102 of the 128 ids had no row at all and lost the attribute entirely.
generatePath shares the lookup, so a GlobalId-less stair was also filed under
the path ifc:IfcRoof.7.
The class now comes from EntityTable.getTypeName, which resolves a type
override, then the enum, then the raw parsed class name — so IfcAirTerminal,
which the enum does not carry, keeps its own name too. IfcTypeEnumToString is
the fallback for structural table stubs with no getTypeName.
Second, the same shape in the same package: SPATIAL_TYPES, the set deciding
which classes are LEVELS of the IFCX spatial tree, held five names against the
shared authority's seventeen. It is also the stop condition in
collectElementIds, so a Site/Road/RoadPart/Wall tree did not merely lose its
facility levels — the site reported elements [road, roadPart, wall] and no
spatial children at all, and determineRelationshipType (a second hand-written
copy of the same list) called the Site->Road edge containment rather than
aggregation. Both call sites now derive from SPATIAL_STRUCTURE_TYPE_ENUMS in
@ifc-lite/data, the answer the parser and the viewer's hierarchy already use.
Tests derive their expectations from the enum and the authority rather than
restating a list, with an anti-vacuity floor on each, named required classes
so a regression names what it broke, and negative controls in both directions
(no class invented for an entity that has none; a physical element still
contained, not aggregated).
* chore(scripts): ratchet the ifcx writer.ts module-size row down to its new size
`packages/ifcx/src/writer.ts` shrank from 424 to 415 lines when its
hand-written enum->class table was replaced by a derivation, so the
recorded budget carried nine lines of headroom that no longer belongs to
anyone. `check-module-size` reports exactly that as a note and asks for
the row to be lowered; the allowlist lives under `scripts/`, which the
change that shrank the file could not reach.
Lowers the row to the measured 415 and re-pins the `packages/ifcx`
entry in `ALLOWLIST_DIGESTS` in the same commit, as the gate requires.
No other row moves, in either direction: the two remaining headroom
notes (`schema-converter.ts`, `parquet-tables.ts`) belong to `main` and
to files this branch does not touch, so tightening them here would put
an unrelated branch in the red for growth it is entitled to.
`node scripts/check-module-size.mjs` is green (309 rows, 0 new over
400) and `scripts/check-module-size.test.mjs` passes 29/29.
Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
* feat(ci): a review of an older commit has not reviewed this PR (#3312) (#3317)
* feat(ci): a review of an older commit has not reviewed this PR (#3312)
Issue #3312's third ask, and the one nobody had built. louistrue: "A review
whose `commit_id` is not the PR head has not reviewed the PR." His example is
#3276 -- head `1305f778`, `CodeRabbit :: success / Review completed` sitting on
it, and CodeRabbit's newest review event naming `c26e453d`, three commits back,
the last of which is real code nothing reviewed. Parts 1 and 2 both pass there:
the lanes ran, and "Review completed" matches no no-verdict phrase. Verified by
running the pre-change gate over #3276's real reviews and statuses -- exit 0,
two green lines, no mention of staleness.
Nothing in the free text of a status links back to a review EVENT, so this adds
the one API that carries the linkage, `pulls/{N}/reviews`, paginated with
`--paginate --slurp` because the NEWEST review is on the LAST page and a partial
walk would compare an older `commit_id` and report a CURRENT PR as stale.
WHICH REVIEWS COUNT IS A POLICY CALL AND IS NOT SETTLED HERE. It is
`staleReviewPolicy`, validated like `reviewVerdictSeverity` -- an unrecognised
value is BAD_CONFIG, never a silent downgrade. Both obvious scopings are wrong
against this repository's data, measured 2026-08-26:
- "ignore COMMENTED" would make the check a no-op. Every review event on
#3276, #3288 and #3227 is COMMENTED -- CodeRabbit's, cursor[bot]'s,
greptile's, codex's and the humans'. Not one APPROVED. It would drop #3276,
the example the issue is written around.
- "an author with no review at head is stale" would nag constantly. #3316,
#3205 and #3290 carry ZERO review events, and #3316 and #3205 still carry
`CodeRabbit :: success / Review completed`. Absence of a review is not
evidence of staleness, and part 3 never reports it. That is a STATED HOLE:
a reviewer that reviews without leaving a review event is invisible to a
`commit_id` comparison, and no scoping fixes it.
So the shipped default `claimed-verdict` is the narrowest rule that still
catches #3276: configured author, AND its context reports success on the head,
AND its newest review names a different commit. The middle clause is what keeps
this off a reviewer that is merely still working. Over the 12 open PRs of
2026-08-26 it fires on #3288, #3227 and #2952 and stays SILENT on #3315, #3309
and #2931, whose newest CodeRabbit review names the head exactly.
`configured-authors` drops the context clause; `all-authors` drops the identity
scope too and is the one that flags a human APPROVED across a rebase.
Severity `warn`, same @unwired-by-design ruling as part 2: whether a bot has
re-reviewed the newest push is transient GitHub state, not a fact about the diff.
Fail-closed, each with its own reason and its own test: NO_HEAD_SHA, NO_REVIEWS,
REVIEWS_TRUNCATED, EMPTY_REVIEW_AUTHORS, UNREADABLE_COMMIT_ID,
UNREADABLE_REVIEW_ID, plus BAD_CONFIG on both new knobs. `--state-file` passes
`reviews` and `headSha` STRAIGHT THROUGH rather than defaulting them, because
that mode quietly supplying a value the real path computes (`timedOut: false`)
was this file's last defect.
20 mutations run against the guards; all 20 caught, and two of them were caught
only after adding tests the sweep proved were missing -- the eager config-read
validation of `staleReviewPolicy` and `reviewAuthors` was masked by the lib's
own, so both now assert over an input where the lazy path cannot be the one
speaking. Every guard restored by inverse edit, byte-identity proved with diff.
Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
* fix(ci): the staleness premise is false for CodeRabbit, so ship it off (#3312)
Fifth-round review of #3317. The `claimed-verdict` rule false-positives on 2 of
its 4 claimed fires, INCLUDING THE FLAGSHIP #3276, and the cause is a premise
this repo's primary reviewer does not honour: CODERABBIT SUBMITS NO REVIEW EVENT
AT ALL WHEN A RUN FINDS NOTHING ACTIONABLE. So "no review object naming the
head" is not "the head was not reviewed". Measured live 2026-08-26 on all four:
#3276 head 1305f778 -- Review queued 14:09:52 -> in progress 14:09:55 ->
success 14:12:27. A real 155 s cycle ON THE HEAD, and the walkthrough comment
updated 14:12:25Z reads "No actionable comments were generated in the recent
review" over "changes between c26e453d and 1305f778": the head, including the
commit the rule called unreviewed. #3288 is identical (181 s, head named).
BOTH FALSE.
#3227 (14 s) and #2952 (9 s) are genuine -- their walkthroughs read "Reviews
paused ... under active development", and CodeRabbit published
`success / Review completed` regardless.
NOTHING IN THE STRUCTURED DATA SEPARATES THE TWO PAIRS. The status is
byte-identical across all four; CodeRabbit publishes no check RUN on any of
these heads, so there is no `conclusion` or `output.title` to read; and the
suggested narrowing -- "a completed review cycle on this head counts as review"
-- is not a narrowing but a deletion, because clause (b) already requires
`success` on the head and a `success` on the head IS a completed cycle, so it
silences #3227 and #2952 too. What is left is cycle DURATION, an unversioned
timing heuristic on a third party, and the reviewer's PROSE, which the config
rules out on purpose. It also contradicted this file's own stated hole: #3316
has success on its head, zero reviews, and is deliberately silent.
A rule that is wrong half the time cannot gate a PR and cannot be repaired with
a discriminator that does not exist, so the machinery, the three scopings and
the four worked examples all ship and `staleReviewPolicy` DEFAULTS TO `off`.
`off` is inert rather than merely silent -- it adjudicates nothing, so it
refuses nothing and does not pay for the paginated reviews walk -- and it NEVER
prints a pass: it prints `STALE_REVIEW not adjudicated` naming the knob.
#3227/#2952 stay catchable for whoever opts in. Verified live over #3276, #3288,
#3227, #2952, #3315, #3309, #2931 and #3316: `off` is silent on all eight, and
`claimed-verdict` still reproduces its 4-fire/4-silent table exactly.
A SUPPRESSED FINDING NO LONGER RENDERS AS A CLEAN PASS. With
`staleReviewSeverity: "fail"` and the shipped `reviewVerdictSeverity: "warn"`, a
rate-limited CodeRabbit with a stale review printed
`✅ No reviewer claims a verdict ... from a review of an older commit` and exited
0, while the same input under `configured-authors` printed `❌ STALE_REVIEW` and
exited 1: the `alreadyFlagged` dedup dropped the finding, so `stale.length === 0`
conflated "clean" with "suppressed" and the severity knob was inoperative.
`staleReviews` now returns the finding with `suppressedBy` set, and the caller
suppresses the SENTENCE, not the VERDICT -- one line naming what already
reported it, and the exit code still tracks the knob.
ORDERING IS `id` ALONE, and the old `(submitted_at, id)` was strictly worse: the
primary key was the one field that can be absent, so a review AT THE HEAD with
no timestamp sorted to `''`, lost to every dated review, and would have reported
a CURRENT PR as stale -- the finding the JSDoc promises is impossible. `id` is
always present (`UNREADABLE_REVIEW_ID` refuses otherwise) and removes the class
outright. `submitted_at` is still printed, no longer compared.
`fetchCheckRunDescriptions` now walks `--paginate --slurp` through
`flattenCheckRunPages`, which refuses a partial walk. It was not live (31 check
runs on the largest head measured, against a 100 page size) but the failure mode
was the bad one: under `claimed-verdict` a missing context is adjudicated by
SILENCE, so truncation was a false negative, not a failure.
And the gate's own unit tests now RUN. Neither test file was reached by any
workflow -- test.yml names its script tests one by one and this pair was never
added, and check-test-glob-coverage audits package globs, not `scripts/`.
10 mutations run against the guards; 10 of 10 caught, and the tenth only after
adding the WIRING test the sweep proved was missing: replacing
`flattenCheckRunPages(...)` with an inline `pages.flatMap(p => p.check_runs ?? [])`
survived the entire suite, because the helper's refusal was tested and its USE
was not. Every mutation restored by inverse edit, byte-identity asserted.
Refs #3312
Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
* fix(cache): carry the raw IFC class name through a cache round-trip (#3320)
`EntityTable` has a `rawTypeName` string column so `getTypeName()` can name
a class the hand-maintained `IfcTypeEnum` does not cover. Diffing that enum
against `packages/data/src/ifc-schema/generated/entities-ifc4.ts`: 101 of the
157 concrete `IfcProduct` subtypes have no enum member (`IfcPump`, `IfcValve`,
`IfcAirTerminal`, `IfcBoiler`, `IfcSurfaceFeature`, ...). The cache writer
never serialized the column, and the reader kept its own copy of the accessor
closures with no fallback in `getTypeName`, so every such element came back
from a cache hit as 'Unknown' while the same model parsed from source named
it correctly.
The column is now written — format v15, appended after the type-range triples
so a v14 section's bytes are unchanged and the read is version-gated — and
`readEntities` builds its table through `entityTableFromColumns`, the same
constructor the parser path uses, rather than a second copy of the closures.
The duplicate is what let the fallback exist on one side only.
Tests: a named list of IFC4 classes split by enum membership, asserted in both
directions (in-enum classes are the negative control, out-of-enum classes are
the regression), with the pre-cache table pinned first so a failure can only be
the cache losing the name; and a v14 section with trailing sentinel bytes,
asserting the reader stops exactly at the old section boundary.
Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
* fix(viewer): name a server-loaded class the IfcTypeEnum does not cover (#3322)
`buildEntityTable` answered `getTypeName` from `IfcTypeEnum` alone while
already holding the real class string from the server (`cols.typeName[idx]`,
handed straight to `CompactEntityIndexBuilder.add`). Any class outside the
128-member enum — IfcPump, IfcChiller, IfcBorehole, most IFC4.3-only
classes — therefore reported 'Unknown', and the hierarchy's By-Type tab
collapsed all of them into a single "Unknown" row for server-parsed models.
This is the third EntityTable implementation to need the same fix:
`entityTableFromColumns` in packages/data already carries a `rawTypeName`
column for exactly this, and the cache-restored table is being fixed
separately. The fallback here is the same mechanism, not a fourth one — an
interned raw-name column, canonicalised with `IFC_ENTITY_NAMES` the way
`EntityTableBuilder.add` and the `setTypeOverride` below it already do.
The six string getters collapse onto one shared column accessor, which is
what keeps the file at its recorded module-size budget.
Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
* fix(data): derive IFC_ENTITY_NAMES from the schema instead of hand-maintaining it (#3323)
* fix(data): derive IFC_ENTITY_NAMES from the schema instead of hand-maintaining it
The map was an 880-entry literal whose header named a regenerator,
`scripts/generate-entity-names.ts`, that has never existed in this
repository. The only thing pinning it was a test comparing it against
`IfcTypeEnum` — a 128-member subset of the ~1160-entity schema — so
everything outside that subset could go missing unnoticed, and 282 entries
had: `IfcWallElementedCase`, `IfcSlabElementedCase`, `IfcBuildingElement`,
`IfcDoorStyle`, `IfcWindowStyle` and the whole `*StandardCase` family among
them. Every caller doing `IFC_ENTITY_NAMES[upper] ?? upper` fell through to
the raw UPPERCASE STEP keyword for those.
It is now built at load from `ifc-schema/generated/entities-*.ts`, which
`generate:ifc-schema` regenerates from the buildingSMART schema dumps, so a
schema bump carries the names along and there is no second list to fall
behind. `IfcSolidStratum`, `IfcVoidStratum` and `IfcWaterStratum` are
reachable through `IfcTypeEnum` but absent from those dumps, so they stay
listed by name.
`ifc-entity-names.schema-parity.test.ts` re-derives the expectation
independently and checks both directions plus a named required list, so a
derivation that starts dropping entities — an `abstract` filter, a schema
left out of the loop — fails instead of degrading display names silently.
Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
* fix(data): emit IFC_ENTITY_NAMES at generate time instead of building it at load
Deriving the map at module load fixed the drift but kept all three
generated schema arrays alive in every bundle that touches a name lookup,
because a runtime loop over them is not something a bundler can
tree-shake. Measured with esbuild, minified, on an entry importing only
`EntityTableBuilder`: 49,405 bytes (12,932 gzipped) before the derivation,
681,999 (70,740) after. `@ifc-lite/data` is published, so a browser
consumer paid ~58 KB gzipped for a string map.
`scripts/emit-entity-names.ts` now writes the literal from the same
`entities-*.ts` tables, chained onto `generate:ifc-schema` so a schema
bump regenerates both in one command. The emitted map is identical to
what the load-time build produced — same 1162 keys, same values, same
insertion order — and the entry now costs 63,283 bytes (16,780 gzipped),
so the 282 recovered names cost ~3.8 KB gzipped rather than ~58 KB.
A committed artefact introduces one new failure mode, staleness, and
`ifc-entity-names.schema-parity.test.ts` is what closes it: it re-derives
the expectation from `entities-*.ts` and checks both directions, so an
`entity-names.ts` left behind by a schema bump fails there. Verified by
mutation — adding an entity to `entities-ifc4.ts` without regenerating
fails `schema → map`; dropping a key and inventing one fails four of the
five tests. The emitter refuses to write when a source array is empty,
which is the load-time module's silent-degradation case: it returned a
3-entry map without throwing.
The three `*STRATUM` names remain hand-added, in the emitter, with the
comment explaining that they are `IfcTypeEnum`-reachable but absent from
the buildingSMART dumps.
Changeset prose corrected on two points a reviewer raised: some of the
282 additions are defined types rather than entities (`IfcLengthMeasure`,
`IfcLabel`, `IfcBoolean`, `IfcGloballyUniqueId`), and `IfcWallStandardCase`
was already listed, so "the whole *StandardCase family" was overstated.
Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
* chore(scripts): drop the stale ifc-entity-names module-size row
`packages/data/src/ifc-entity-names.ts` is now 31 lines — the map it used to
carry inline is emitted into `src/ifc-schema/generated/entity-names.ts`, which
the ratchet excludes as generated. Its 907-line row is therefore pure slack,
and `check-module-size.mjs` prints a note asking for it to be deleted.
Deleted, with the `packages/data` scope re-pinned in `ALLOWLIST_DIGESTS` in the
same commit. Only that scope moved; no budget was raised and no row added —
309 rows to 308, one removal, verified against `upstream/main`.
Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436
* Adding anonymizer-export for debug
* fix(export): close the anonymized-export review findings (#3309)
Addresses every finding raised in the review of #3309.
Correctness and privacy:
- The CLI no longer copies the output path into the STEP header's
FILE_NAME, so `--out <project>.ifc` cannot reintroduce the name the
export exists to remove. It falls through to the neutral default.
- IfcSite/IfcBuilding georeferencing and address slots are blanked with
`$` instead of an empty string. RefLatitude/RefLongitude are a LIST OF
INTEGER, RefElevation a REAL, and the two address slots are entity
references, so `''` left the file unable to round-trip a strict reader.
- IfcPerson keeps FamilyName = 'Anonymous' rather than clearing every
slot, which violated the IdentifiablePersonName WHERE rule and let a
validator reject the file a bug report is meant to carry.
- Attribute slots now resolve against the source model's own schema.
Fixing the read side alone was not enough: `setAttribute` re-resolves
the name to a slot independently at serialize time against the pinned
IFC4 order, so on an IFC2X3 model the scrub wrote to the wrong slots
and left the real value untouched. The scrub writes positionally with
the already-resolved index.
Viewer:
- Only the trigger-less host instance answers the store flag. Both the
ViewerLayout mount and the toolbar-registered one used to open
together, each running preview isolation and each restoring shared
visibility state, which could strand the viewer in the temporary
isolation.
- The section-header checkbox writes `indeterminate` from an effect
rather than a ref callback, which React does not re-invoke on
re-render, so the mixed state went stale until a row remounted.
CLI surface:
- Relationship flags use exact IFC EXPRESS names, and IfcRelAggregates
and IfcRelNests are no longer collapsed into one switch.
Tests and docs:
- Entity-presence assertions parse the exported model instead of
matching serialized text, where 'IFCWALL' also matched IFCWALLTYPE and
IFCWALLSTANDARDCASE in both directions. The relationship tests assert
the relationship is gone rather than that a pseudonymized name is
absent.
- New coverage: an unselected spatial root is absent with none of its
values reachable and a selected one is scrubbed; the serialized
IFCSITE/IFCBUILDING lines; the dialog host gating; IFC2X3 slot
resolution.
- The CLI guide no longer promises a `--keep-*` flag for scrubs that
have no opt-out, and the export README's retained-field list matches
the code.
Module-size ratchet: the schema-derived type-set machinery moves out of
reference-collector.ts into entity-type-sets.ts, re-exported so no
caller and no public surface changes; the four viewer files come back
under budget by compressing prose. No budget was raised.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Louis Trümpler <78563314+louistrue@users.noreply.github.com>
Co-authored-by: Petru Conduraru <petru@bimvoice.com>
Co-authored-by: Yuri Isachenkov <69924139+Blogbotana@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This PR was opened by the Changesets release GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated.
Releases
@ifc-lite/cli@0.25.1
Patch Changes
#3143
22f4a1aThanks @BIMvoice! - Fix the CLIstatscommand's window-area total to sum every matching quantity, not silently drop to the first.sumQuantityinstats-aggregation.ts(introduced whenstats.tswas refactored to share aggregation logic across window area, floor area and material volumes) has nobreakafter adding a match — it sums everyArea/GrossArea/NetAreaetc. quantity across every quantity set on a ref. The original window-area loop it replaced had abreakafter the firstAreamatch inside each quantity set, so the two disagreed whenever a quantity set held more than one same-named quantity.Kept sum-all rather than adding a first-match flag: a quantity set with two same-named quantities is not valid IFC —
IfcElementQuantitycarries theUniqueQuantityNamesWHERE rule — so the divergence is only reachable on schema-non-compliant files, and four of the five call sites that fed the old loop already summed every match rather than taking the first.Updated dependencies [
66923ee,224386a,cf84055,cf0ad86,5b89621]:@ifc-lite/geometry@4.0.1
Patch Changes
#3176
66923eeThanks @louistrue! - Correct the call-site count ingeometry-class.ts's docblock and its test's: six files across three packages comparedgeometryClassagainst bare integers before the module existed, not five.The sixth is
apps/viewer/src/components/viewer/ViewportContainer.tsx:819, which read(meshes[i].geometryClass ?? 0) !== 0and now goes throughmeshIsNonOccurrence. It was converted on #3161 and named in that PR's changeset and merge subject, but the two doc comments kept the pre-audit number — and they are what a reader lands on when opening the module. Comment-only; the enumeration now lists all six.Updated dependencies [
224386a,cf84055,cf0ad86,5b89621]:@ifc-lite/ids@1.15.50
Patch Changes
224386a]:@ifc-lite/parser@4.3.1
Patch Changes
#3178
224386aThanks @louistrue! - Stop dropping six concrete IFC2X3 products from mesh and attribute export, and remove an alias row that named no entity.rust/core/src/legacy_entities.rsis the table every classification pass is told to consult instead of a bareIfcType::from_str. It held 21 arms. Diffing@ifc-lite/data's IFC2X3/IFC4 tables against the generated IFC4X3 enum — the methodmerged.rsalready documents — turns up six concreteIfcProductsubtypes that carry both a placement and a representation and were in neither:IfcElectricalElement,IfcElectricDistributionPoint,IfcChamferEdgeFeature,IfcRoundedEdgeFeature,IfcStructuralLinearActionVarying,IfcStructuralPlanarActionVarying.A name the table misses resolves to
IfcType::Unknown, andUnknownis a subtype of nothing. The attribute exporter keeps a row only if the type reachesIfcProduct, andhas_geometry_by_namerefusesUnknownoutright, so an IFC2X3 file containing one of these lost it from the attribute export and from meshing at once. The two passes agreed, on dropping it — which is why nothing looked wrong. Each new arm maps to its own supertype from the older schema rather than to a generic proxy.The
IfcElectricDistributionPointarm was spelledIFCELECTRICALDISTRIBUTIONPOINT, with an "AL" no IFC2X3 entity has. It could never match a real file, and a Rust test assertedhas_geometry_by_nameon the same misspelling, so the table and its test certified each other while describing nothing.That misspelling had spread. #2883 mirrored it into
@ifc-lite/parser'sENTITY_NAME_ALIASESon the stated premise that it was "real, deprecated IFC2X3 syntax", and two tests plus a comment in@ifc-lite/querywere then written against the mirror — five artifacts agreeing with each other about an entity that does not exist. The alias row is removed rather than respelled, because the correctly spelled name is inENTITIES_IFC2X3and already resolves throughIfcFlowControllertoIfcDistributionElementwith no alias at all; that is also exactly what the new Rust arm answers. The dependents now assert the real name, plus a negative on the misspelling so restoring the alias turns them red.Fixing the table exposed a second live defect. The construction-projection filter from #979 read
entity.ifc_type, which the decoder fills with a barefrom_str— so every legacy spelling of a feature element arrived asUnknownand passed straight through. Measured on AC20-FZK-Haus with its 17 openings respelled toIFCOPENINGSTANDARDCASE: 33 spurious void cross-sections in the floor plan before, none after.scripts/check-legacy-entity-coverage.mjsnow runs that diff on every PR, in both directions: a concrete legacy product with no arm fails, and so does an arm whose key names no entity in any bundled schema.Updated dependencies [
224386a,cf84055,cf0ad86,5b89621]:@ifc-lite/server-client@1.23.1
Patch Changes
#3123
cf0ad86Thanks @BIMvoice! - Bound the symbolic revisit budget across the whole extraction instead of resetting it per representation item.A drawing whose repeated traversal was spread across many top-level items was
previously unbounded: each item got a fresh budget, so a file of N items could
spend N times the intended limit. The budget now lives on the extraction and is
charged once per revisit wherever it happens.
Two consequences worth knowing before upgrading:
truncatedcan now appear on files that did not report it before, withreason
item-revisits. Nothing is dropped silently — that is the point ofreporting it — but a consumer that treats any
truncatedas an error willsee it more often. The bound's value was not re-sized for its wider scope, so
a large nested block import spread over many products can truncate where it
previously completed.
re-placing one library block many times is not counted as revisiting it.
@ifc-lite/wasm@6.0.1
Patch Changes
#3178
224386aThanks @louistrue! - Stop dropping six concrete IFC2X3 products from mesh and attribute export, and remove an alias row that named no entity.rust/core/src/legacy_entities.rsis the table every classification pass is told to consult instead of a bareIfcType::from_str. It held 21 arms. Diffing@ifc-lite/data's IFC2X3/IFC4 tables against the generated IFC4X3 enum — the methodmerged.rsalready documents — turns up six concreteIfcProductsubtypes that carry both a placement and a representation and were in neither:IfcElectricalElement,IfcElectricDistributionPoint,IfcChamferEdgeFeature,IfcRoundedEdgeFeature,IfcStructuralLinearActionVarying,IfcStructuralPlanarActionVarying.A name the table misses resolves to
IfcType::Unknown, andUnknownis a subtype of nothing. The attribute exporter keeps a row only if the type reachesIfcProduct, andhas_geometry_by_namerefusesUnknownoutright, so an IFC2X3 file containing one of these lost it from the attribute export and from meshing at once. The two passes agreed, on dropping it — which is why nothing looked wrong. Each new arm maps to its own supertype from the older schema rather than to a generic proxy.The
IfcElectricDistributionPointarm was spelledIFCELECTRICALDISTRIBUTIONPOINT, with an "AL" no IFC2X3 entity has. It could never match a real file, and a Rust test assertedhas_geometry_by_nameon the same misspelling, so the table and its test certified each other while describing nothing.That misspelling had spread. #2883 mirrored it into
@ifc-lite/parser'sENTITY_NAME_ALIASESon the stated premise that it was "real, deprecated IFC2X3 syntax", and two tests plus a comment in@ifc-lite/querywere then written against the mirror — five artifacts agreeing with each other about an entity that does not exist. The alias row is removed rather than respelled, because the correctly spelled name is inENTITIES_IFC2X3and already resolves throughIfcFlowControllertoIfcDistributionElementwith no alias at all; that is also exactly what the new Rust arm answers. The dependents now assert the real name, plus a negative on the misspelling so restoring the alias turns them red.Fixing the table exposed a second live defect. The construction-projection filter from #979 read
entity.ifc_type, which the decoder fills with a barefrom_str— so every legacy spelling of a feature element arrived asUnknownand passed straight through. Measured on AC20-FZK-Haus with its 17 openings respelled toIFCOPENINGSTANDARDCASE: 33 spurious void cross-sections in the floor plan before, none after.scripts/check-legacy-entity-coverage.mjsnow runs that diff on every PR, in both directions: a concrete legacy product with no arm fails, and so does an arm whose key names no entity in any bundled schema.#3124
cf84055Thanks @BIMvoice! - Stop merged STEP export from leaving duplicate GlobalIds on IFC4.3 stratum entities.export_mergedreconciles GlobalIds so that two federated models sharing an element emit that element's 22-character id once, not twice — a duplicate GlobalId is an IFC spec violation. Deciding whether a line's first attribute is a GlobalId means asking whether the entity type derives fromIfcRoot, and the Rust side asked with a bareIfcType::from_str.The generated enum is derived from IFC4X3 alone and models the three stratum leaves only by their abstract base:
IfcSolidStratum,IfcVoidStratumandIfcWaterStratumare all folded intoIfcGeotechnicalStratum.from_strtherefore answeredUnknownfor the names authoring tools actually write, andUnknownis a subtype of nothing, so reconciliation skipped them. Merging two infrastructure models that share a terrain or soil layer produced a file with the same GlobalId twice — while theIfcWallon the next line was reconciled correctly. The lookup now goes throughlegacy_aware_ifc_type, the same resolution every other classifying pass in the workspace is required to use.The JS classifier in
@ifc-lite/exportnever had the bug: it resolves those names throughENTITY_NAME_ALIASES, the mirror ofrust/core/src/legacy_entities.rs, and answered rooted all along. So this was a live cross-language disagreement, and both halves of the parity gate added in #3015 were green throughout — because the sweep's universe was built from Rust's own two tables, and a name known only to an alias table appears in neither. A universe that cannot name a type cannot compare it.Two structural changes close that, rather than three rows being added by hand. The sweep's universe now includes
ifc_lite_core::LEGACY_ENTITY_NAMES, a newly enumerable form of the legacy table whose contents are re-derived from the lookup's own source text and asserted equal, so an arm added without a name fails the test. Androoted_type_parity.rsnow asserts WHICH rows the fixture holds, not just how many: previously 31 rows could be deleted — including the three the unit tests call load-bearing — and both languages stayed green on the remainder at901 > 900, so a disagreement could be made to disappear by deleting the row that carried it.Measured across the whole 936-name universe, Rust and JS disagreed on 3 names before and 0 after.
#3123
cf0ad86Thanks @BIMvoice! - Bound the symbolic revisit budget across the whole extraction instead of resetting it per representation item.A drawing whose repeated traversal was spread across many top-level items was
previously unbounded: each item got a fresh budget, so a file of N items could
spend N times the intended limit. The budget now lives on the extraction and is
charged once per revisit wherever it happens.
Two consequences worth knowing before upgrading:
truncatedcan now appear on files that did not report it before, withreason
item-revisits. Nothing is dropped silently — that is the point ofreporting it — but a consumer that treats any
truncatedas an error willsee it more often. The bound's value was not re-sized for its wider scope, so
a large nested block import spread over many products can truncate where it
previously completed.
re-placing one library block many times is not counted as revisiting it.
#3182
5b89621Thanks @louistrue! - Label legacy IFC keywords with their resolved type in the browser, not"Unknown".The native pipeline resolves a legacy keyword through
legacy_entities.rsand labels the node with its real base type. The browser path did not: the jobs wire carries only(id, start, end), sobatch.rsrebuilt the type fromentity.ifc_type— the decoder's bareIfcType::from_str— and a legacy keyword that reached that path arrived asUnknownwith the Unknown default colour. The 22legacy_entities.rsarms that carry geometry are fixed here, among themIfcProxy, the eight*StandardCasevariants, both*ElementedCase,IfcEquipmentElement, the three IFC4X3 strata leaves and the six #3172 added. Type-exact visibility rules and styling consumers skipped them, and nothing threw.IfcDoorStyleandIfcWindowStyleare NOT fixed by this change. The pre-passes gate type-geometry candidates on a bareIfcType::from_str(name).is_subtype_of(IfcTypeProduct), which is false for any keyword IFC4X3 dropped, so both are discarded before a geometry job exists and never reach the corrected line. That is the same defect one layer up; #3187 enumerates the sites.It cannot be recovered from the decoded value:
IfcType::Unknownstores a CRC32 hash, not the name. It is recomputed from the record instead, which the batch already holds — a short scan to the first(, paid only by entities the decoder could not name.Fixing it surfaced a second defect.
extract_entity_type_namedid not trim, so[#71](https://github.com/LTplus-AG/ifc-lite/issues/71)= IFCCOLUMN(— legal STEP, and what buildingSMART's owncolumn-straight-rectangle-tessellation.ifcwrites on all 26 of its entity lines — yielded" IFCCOLUMN"with a leading space, matching no lookup. The function had no production caller, so its broken contract had never been exercised.extract_entity_type_nameispubinifc-lite-core, so that is a behaviour change on a published Rust surface: it now returns the trimmed name, andNonerather thanSome(" ")for a record with only whitespace between=and(.Summary by CodeRabbit
Bug Fixes
Documentation