fix(query)!: reject a non-IFC type name in ofType(), but not standard types the enum table omits - #3009
fix(query)!: reject a non-IFC type name in ofType(), but not standard types the enum table omits#3009BIMvoice wants to merge 3 commits into
Conversation
…of matching Unknown
IfcTypeEnumFromString falls back to IfcTypeEnum.Unknown for any type name it
does not recognize, so a caller's typo (ofType('IfcWal')) or a vendor-specific
type silently queried the Unknown bucket — every entity whose type the store
itself could not classify — instead of returning nothing. ofType() now throws
for an unrecognized name; the Unknown bucket is still reachable by passing the
literal string 'Unknown'.
The guard added in 33bda64 rejected every type string that mapped to IfcTypeEnum.Unknown. TYPE_STRING_TO_ENUM (packages/data/src/types.ts) is a curated subset of IFC, not the whole schema, so that rule also rejected standard buildingSMART types the table simply has no row for - IfcChiller, IfcActuator, IfcElectricAppliance, IfcBuildingSystem, IfcAudioVisualAppliance among them. Querying those returned the Unknown bucket before, which answers correctly in a file whose only unclassified entities are of that type; the guard turned that working query into a throw with no disclosure. Key the check on IFC_ENTITY_NAMES instead - the ~880-entry IFC4X3 entity-name table already exported from @ifc-lite/data. A string that is not an IFC entity name at all ('IfcWal') still throws; a real IFC name the enum table does not map falls through to Unknown exactly as before. 'Unknown' stays reachable by its literal string. RED: with the previous condition restored, the six new expectations covering the five standard types plus casing/whitespace fail; they pass with this one. packages/query 177 -> 185 pass, packages/data 148 pass, both 0 fail. The changeset is corrected from patch to major and now states the actual breaking case: a name that is not an IFC entity name - a typo, or a genuine vendor-specific type name - previously returned an EntityQuery over the Unknown bucket and now throws. @ifc-lite/query is 1.x, and this is a behaviour change on a published SDK export.
|
Warning Review limit reached
Next review available in: 55 minutes Limit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Viewer benchmark⚠ 1 metric(s) exceeded the regression threshold (advisory only, not blocking). 01_Snowdon_Towers_Sample_Structural(1).ifcBaseline recorded 2026-07-01T20:31:05.538Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.
AC20-FZK-Haus.ifcBaseline recorded 2026-07-01T20:30:59.972Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.
Refresh the baseline from a CI run: dispatch the Benchmark workflow with |
|
The guard rejects seven entity names that this repo's own parser ships as real IFC entities.
Run against the built Concrete failure: So the changeset's "Standard IFC types that this build's enum table does not map ... are not rejected" is not true as written, and the error text tells the user to fix a spelling that is already right. The oracle needs to cover the schemas the parser actually reads, not just IFC4X3. Widening it to the parser's schema registry (or adding the missing names to A cheap regression test: assert that every entity name in |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
2 participants
query.ofType()silently returned an empty result for a mistyped type name. It now throws — but only for strings that are not IFC entity names at all.Found on a never-raised branch; merges clean. The branch as written threw for any type absent from
TYPE_STRING_TO_ENUM, which is a curated 138-entry subset — so it also rejected standard buildingSMART types the table simply omits.What that would have broken
Confirmed by running
IfcTypeEnumFromString:IfcChiller,IfcActuator,IfcElectricAppliance,IfcBuildingSystemandIfcAudioVisualApplianceall resolve toUnknownand would have thrown. Those are standard IFC4 types, not the "typo … or vendor-specific type name" the original changeset described. Querying them previously reached the Unknown bucket — which, in a file whose only unclassified entities are chillers, worked.The fix keys on a real oracle
packages/data/src/ifc-entity-names.tsalready exportsIFC_ENTITY_NAMES, a ~880-entry IFC4X3 table, from the packageofType()already imports:An indexed-value check rather than
in, so the lookup cannot reachObject.prototype.Extending
TYPE_STRING_TO_ENUMinstead was considered and rejected as disproportionate: it would mean adding ~750 members toIfcTypeEnum, which is mirrored inrust/core/src/generated/type_ids.rs.Verified per type, by returned ids rather than by absence of a throw: all five standard types are in
IFC_ENTITY_NAMES(lines 26, 60, 99, 126, 261), absent fromTYPE_STRING_TO_ENUM, and now reach the Unknown bucket.IFCWALandIFCPROPRIETARYVENDORTHINGstill throw.RED with the branch's original condition restored: 6 of 10 fail — the five standard types plus the casing/whitespace case — with
Error: ofType(): "IFCCHILLER" is not an IFC entity name.The bump was wrong, and is corrected
patch→major.@ifc-lite/queryis 1.14.16, and throwing where the API previously returned anEntityQueryis breaking on a published export.The changeset now leads with "Breaking:", lists the five standard types as explicitly not rejected, explains why the check keys on
IFC_ENTITY_NAMESrather than the enum table, and states the real breaking case plainly: a genuine vendor-specific type name — which the original changeset cited as a reason to throw — previously reached the Unknown bucket and now throws, with'Unknown'as the migration path.packages/query177 → 185 (the 2-test file is replaced by 10);packages/data148 unchanged.'Unknown'escape hatch verified still working. api-surface unchanged at 4191 (the signature did not move), unused-locals, changesets, source-text-assertions, test-wiring and check-generated all pass. No baseline or ratchet touched.🤖 Generated with Claude Code