Commit 8368339
authored
fix(query): resolve abstract EXPRESS supertypes to their schema descendants in byType() (#3701)
## Reviewer summary
- **Without this:** querying by an abstract IFC type (e.g. `IfcBuildingElement`, `IfcElement`) — across the CLI, MCP, and viewer SDK, which all share this code path — silently returns zero results, even on a model full of matching walls, slabs, and columns.
- **Evidence:** on a fixture with a wall, a wall-standard-case, a slab, and a column, `byType('IfcBuildingElement')` returned an entity set that fails `expected ['IFCBUILDINGELEMENT'] to include 'IFCWALL'` before the fix; after the fix it returns all 4 building elements.
- **Risk:** the descendant resolver is deliberately scoped to one schema version at a time (descendant sets differ, e.g. IFC4X3's `IfcBuiltElement`), not a cross-version union. Control: `byType('IfcSpace')`, a real type with 0 instances in the fixture, still returns 0 — no over-matching. IDS entity-facet matching is explicitly left unchanged, per the buildingSMART IDS spec's no-automatic-inheritance rule (cited in a new code comment).
- **Sequencing:** stacked on #3577 — merge that first. #3577 already carries the maintainer's own waiver.
---
Stacked on #3577 — merge that first.
## Defect
`expandTypes` — the type-expansion every `byType()` query surface shares (CLI, MCP, viewer SDK; all three now import it from one place, `@ifc-lite/parser`'s `query-backend-maps.ts`, per an earlier consolidation this branch already carries) — walked a fixed nine-entry `IFC_SUBTYPES` table that only aliased `*StandardCase`/`*ElementedCase` pairs. Asking for an abstract EXPRESS supertype (`IfcBuildingElement`, `IfcElement`) is never a literal STEP entity type, so that table had no row for it, and `byType('IfcBuildingElement')` silently matched **zero** entities on a model full of walls, slabs and columns.
## Site-by-site
| Site | Before | After |
|---|---|---|
| `packages/parser/src/query-backend-maps.ts` (`expandTypes`) | fixed 9-entry `IFC_SUBTYPES` table, no schema awareness | delegates to `@ifc-lite/data`'s `expandTypeNamesToDescendants`, takes the model's `schemaVersion` |
| `packages/cli/src/headless-backend.ts` | called `expandTypes(descriptor.types)` | passes `store.schemaVersion` |
| `packages/mcp/src/backend-query.ts` + `packages/mcp/src/tools/query.ts` (`count_entities`) | same | passes `store.schemaVersion` |
| `apps/viewer/src/sdk/adapters/query-adapter.ts` | same | passes `model.ifcDataStore.schemaVersion` |
| `packages/ids/src/facets/entity-facet.ts` | exact-match only | **unchanged** — documented, see below |
| `packages/query/src/fluent-api.ts` (`QueryBuilder.ofType`) | — | **unchanged**, confirmed dead code (no product import outside its own test) |
## Fix
New `expandTypeNamesToDescendants(types, schemaVersion)` in `packages/data/src/ifc-schema/descendants.ts`: a per-schema-version descendant-closure resolver over the bundled `ENTITIES_IFC2X3`/`ENTITIES_IFC4`/`ENTITIES_IFC4X3` tables (the same schema authority `getInheritanceChainFromSchemaUnion` in `@ifc-lite/parser` walks in the ancestor direction). Builds and caches a parent→children map per schema version, then walks down from the requested type. Deliberately scoped to ONE schema version at a time — not a union — because descendant sets differ by version (IFC4X3 renamed `IfcBuildingElement` to `IfcBuiltElement`; per the bundled generated tables, IFC4X3 in this repo still lists `IfcWallStandardCase`, unlike the final published spec — verified directly against `entities-ifc4x3.ts` rather than assumed).
Unknown/unrecognized schema version falls back to IFC4, matching what the old hardcoded table implicitly assumed.
## Counts, real fixture, real columnar parser (packages/cli, `headless-backend-supertype-descendants.test.ts`)
Fixture: `IfcWall` ×1, `IfcWallStandardCase` ×1, `IfcSlab` ×1, `IfcColumn` ×1 (4 building elements total).
| Query | Before | After |
|---|---|---|
| `byType('IfcBuildingElement')` | 0 | 4 |
| `byType('IfcElement')` | 0 | 5 (also covers `IfcFurnishingElement` in a second fixture — broader ancestor) |
| `byType('IfcWall')` | 2 | 2 (no regression) |
| `byType('IfcSpace')` — real type, 0 instances | 0 | 0 (no over-matching) |
IFC4X3 fixture (`IfcWall` ×1, `IfcCourse` ×1, both descendants of `IfcBuiltElement`): `byType('IfcBuiltElement')` → 2 (was 0; `IfcBuiltElement` isn't in the old table at all).
Same shape reproduced independently in `packages/mcp/src/backend-query-supertype-descendants.test.ts` and `apps/viewer/src/sdk/adapters/query-adapter.supertype-descendants.test.ts`.
## Out of scope, documented not changed
- `packages/ids/src/facets/entity-facet.ts` — added a comment on `checkEntityFacet` citing buildingSMART's IDS spec: *"There is no automatic inheritance in IDS entity facet interpretation. In other words, all the entities need to be listed explicitly. […] to create a requirement applicable to all IfcElement objects, one should list all IfcElement sub-entities, such as IfcWall, IfcDoor, etc. Also, the IfcElement should not be listed, as it is an abstract entity […] and would not appear in a model."* (`Documentation/UserManual/entity-facet.md`, buildingSMART/IDS). Matching logic itself is untouched.
- `packages/query/src/fluent-api.ts`'s `QueryBuilder.ofType` — confirmed dead code (no product code imports `QueryBuilder` from `@ifc-lite/query` outside that file's own test); intentionally left untouched, FYI only.
## Tests
- `packages/data/src/ifc-schema/descendants.test.ts` (new, 8 tests): IFC4 `IfcWall`/`IfcBuildingElement` descendant sets, IFC4X3 `IfcBuiltElement`, unknown-type fallback, case-insensitivity, dedup, unknown/undefined schema version fallback.
- `packages/parser/test/query-backend-maps.test.ts` (extended — still green): pins `expandTypes`'s exact behavior/order for the StandardCase cases.
- `packages/cli/src/commands/validate-subtypes.test.ts` (extended — still green): cross-checks `expandTypes` against `IFC_SUBTYPES` and the schema registry; IFC4's declared children for the 9 old table entries are identical to the new resolver's output, so no behavior change there.
- Three new call-site tests proving the fix against real fixtures parsed by the real parser (see counts table above): `packages/cli/src/headless-backend-supertype-descendants.test.ts`, `packages/mcp/src/backend-query-supertype-descendants.test.ts`, `apps/viewer/src/sdk/adapters/query-adapter.supertype-descendants.test.ts`.
RED verified by reasoning against the unfixed `IFC_SUBTYPES` table (no key for `IFCBUILDINGELEMENT`/`IFCELEMENT`/`IFCBUILTELEMENT`, so `expandTypes` returned only the literal requested name, which never appears as a raw STEP type since it's abstract) rather than by checking out the pre-fix commit, since this PR replaces that table's only consumer in one change.
## Gates (all foreground, real output)
- `pnpm exec turbo build typecheck test --filter=@ifc-lite/data --filter=@ifc-lite/parser --filter=@ifc-lite/cli --filter=@ifc-lite/mcp --filter=@ifc-lite/ids`: 39/39 tasks successful.
- data: 22 test files, 252 tests passed
- parser: 87 test files, 964 passed | 2 skipped
- cli: 60 test files, 590 passed | 8 skipped
- mcp: 36 test files, 353 passed
- ids: 27 test files, 843 passed
- `apps/viewer` typecheck (`tsc --noEmit`): clean. Viewer test file run directly via its `node:test` runner: 4/4 passed.
- `node scripts/check-module-size.mjs`: OK (2086 files measured, 305 allowlisted, 0 new over 400).
- `node scripts/check-unused-locals.mjs`: pre-existing `packages/embed-sdk` "does not compile standalone" noise, confirmed present on the unmodified base commit too (verified via `git stash`) — unrelated to this change.
- `node scripts/check-test-wiring.mjs`: OK (49 packages, 55 gate scripts, 65 scripts/ test files).
- `pnpm run check:vitest-timeout-audit`: exit 0.
- `git diff --stat -- Cargo.lock`: empty, no diff.
Did not run `cargo`/build `@ifc-lite/wasm` per instructions (another agent held the shared rust target-dir lock).
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit1 parent 21b131d commit 8368339
23 files changed
Lines changed: 1805 additions & 54 deletions
File tree
- .changeset
- apps/viewer/src/sdk/adapters
- packages
- cli/src
- commands
- data/src
- ifc-schema
- ids/src
- bridge
- facets
- mcp/src
- parser
- src
- test
- rust/export
- examples
- src
- scripts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
Lines changed: 126 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
195 | 195 | | |
196 | 196 | | |
197 | 197 | | |
198 | | - | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
199 | 205 | | |
200 | | - | |
| 206 | + | |
201 | 207 | | |
202 | 208 | | |
203 | 209 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| 22 | + | |
22 | 23 | | |
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
26 | | - | |
27 | | - | |
| 27 | + | |
| 28 | + | |
28 | 29 | | |
29 | 30 | | |
30 | 31 | | |
| |||
95 | 96 | | |
96 | 97 | | |
97 | 98 | | |
98 | | - | |
99 | | - | |
| 99 | + | |
| 100 | + | |
100 | 101 | | |
101 | 102 | | |
102 | 103 | | |
103 | | - | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
104 | 110 | | |
105 | 111 | | |
106 | 112 | | |
| |||
113 | 119 | | |
114 | 120 | | |
115 | 121 | | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
64 | 64 | | |
65 | 65 | | |
66 | 66 | | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
67 | 70 | | |
68 | 71 | | |
69 | 72 | | |
| |||
72 | 75 | | |
73 | 76 | | |
74 | 77 | | |
75 | | - | |
76 | | - | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
77 | 81 | | |
78 | | - | |
79 | | - | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
80 | 85 | | |
81 | 86 | | |
82 | 87 | | |
| |||
284 | 289 | | |
285 | 290 | | |
286 | 291 | | |
287 | | - | |
| 292 | + | |
288 | 293 | | |
289 | 294 | | |
290 | 295 | | |
| |||
303 | 308 | | |
304 | 309 | | |
305 | 310 | | |
306 | | - | |
| 311 | + | |
307 | 312 | | |
308 | 313 | | |
309 | 314 | | |
| |||
0 commit comments