Skip to content

Commit af067e5

Browse files
BIMvoicelouistrue
andauthored
fix(parser,ids): keep relationship-resolved classifications on server-parsed stores (#3951)
* fix(parser,ids): distinguish classified-but-unresolved from unclassified on server-parsed stores Closes #3948. extractClassificationsOnDemand and extractClassificationSystemsOnDemand (packages/parser/src/classification-resolver.ts) resolved classification ids via the relationship graph on server-parsed (source-empty) stores, then unconditionally discarded the result with `if (!store.source?.length) return [];` right after — a genuinely classified entity was byte-identical to an unclassified one to every caller, including the IDS bridge's classification facet. The classification's own attributes (system name, identification code, reference chain) need raw STEP bytes to read, and no precomputed table covers them on a server-parsed store the way the type-inherited property sets fix (#1795/#1787) had one to fall back to. So both functions now signal "classified, but unresolved" distinctly from "genuinely unclassified": extractClassificationsOnDemand returns one `{ unresolved: true }` entry per resolved id instead of `[]`, and extractClassificationSystemsOnDemand's return type changes from `string[]` to `{ names: string[]; unresolved: boolean }`. The IDS classification facet checker now passes a presence-only ("any classification") requirement correctly for a classified entity (was a false CLASSIFICATION_MISSING), and reports a new distinct CLASSIFICATION_UNRESOLVED failure for a system/value-constrained facet it cannot verify, instead of silently passing or failing on data it never read. The viewer's classification-systems panel (ModelMetadataPanel.tsx) shows an explicit "unavailable on this data source" state instead of a false "no classification systems". Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436 * fix(ids): surface CLASSIFICATION_UNRESOLVED as a clear message, not the raw enum Neither describeFailure (translation/service.ts) nor formatFailureReason (validation/validator.ts) had a case for CLASSIFICATION_UNRESOLVED (added earlier in this PR for #3948), so both fell through to their default branch and showed the raw internal enum to the user: "Validation failed: CLASSIFICATION_UNRESOLVED" That string reached the IDS panel (styled identically to a genuine violation) and the exported HTML/CSV report. Both formatters now say plainly that the entity is classified but its details could not be read from this data source, matching the length/voice of the neighbouring classification cases. ClassificationCard.tsx gets the same "unavailable on this data source" treatment ModelMetadataPanel.tsx already has, instead of rendering an empty "Classification / Unknown" card for an unresolved entry. No new UI severity was introduced: a fail result has no softer variant today (StatusIcon's not_applicable is a different, unrelated status), so a clear message is the fix. The optionality exclusion in validator.ts that keeps CLASSIFICATION_UNRESOLVED out of `missingFailures` is untouched — that conservative call (an optional facet that cannot be verified still fails, rather than silently passing) was deliberate. Trimmed two unrelated JSDoc blocks to single lines to keep translation/service.ts and validation/validator.ts within their checked-in module-size budgets after the new switch cases. Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436 * fix(mcp): forward classification `unresolved` through the playground's own IDS accessor The playground's `makeIdsAccessor().getClassifications` (used by the `ids_validate` tool, the chat agent's IDS check) is a separate reimplementation of `IFCDataAccessor` from `createDataAccessor` (packages/ids/src/bridge/data-accessor.ts) — the "two paths that must agree" shape this repo already caught once for materials. #3948/#3951 added a tri-state `unresolved` marker to `ClassificationInfo` so a classified-but-unreadable entity (server-parsed store) reads distinctly from a genuinely unclassified one. The canonical bridge forwards it; this accessor rebuilt a plain object from `m.bim.classifications(...)` and dropped the field, so a classified-but-unresolved entity looked to the IDS engine like a real classification with an empty system/value — reported as a fabricated CLASSIFICATION_SYSTEM_MISMATCH/_VALUE_MISMATCH instead of the honest CLASSIFICATION_UNRESOLVED the viewer's own IDS panel already reports. `ClassificationData` (@ifc-lite/sdk) gains the same optional `unresolved` field so the accessor can type-check while forwarding it. Verified against the real dispatcher + IDS engine: a fixture with a genuine IfcRelAssociatesClassification edge, forced into the server-parsed shape (no source bytes, no on-demand map) the same way #3951's own fixtures do. Reverting the fix reproduces CLASSIFICATION_SYSTEM_MISMATCH; with the fix, CLASSIFICATION_UNRESOLVED. Claude-Session: https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436 --------- Co-authored-by: Louis Trümpler <78563314+louistrue@users.noreply.github.com>
1 parent 4dc93be commit af067e5

24 files changed

Lines changed: 635 additions & 50 deletions
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
"@ifc-lite/parser": minor
3+
"@ifc-lite/ids": patch
4+
---
5+
6+
Fix a server-parsed (source-empty) store reporting a genuinely classified entity as unclassified (#3948). `extractClassificationsOnDemand` and `extractClassificationSystemsOnDemand` (`packages/parser/src/classification-resolver.ts`) resolved classification ids via the relationship graph on server-parsed stores, then unconditionally discarded the result with `if (!store.source?.length) return [];` — a classified entity was byte-identical to an unclassified one to every caller, including the IDS bridge.
7+
8+
The classification's own attributes (system name, identification code, reference chain) genuinely cannot be read without raw STEP bytes, and no equivalent precomputed table exists for them on a server-parsed store (unlike type-inherited property sets, fixed for the same shape of bug in #1795/#1787). So both functions now signal "classified, but unresolved" distinctly from "genuinely unclassified": `extractClassificationsOnDemand` returns one `{ unresolved: true }` entry per resolved id instead of `[]`, and `extractClassificationSystemsOnDemand`'s return type changes from `string[]` to `{ names: string[]; unresolved: boolean }` (a breaking signature change with no known external callers today).
9+
10+
The IDS classification facet checker (`packages/ids/src/facets/classification-facet.ts`) now treats presence-only facets correctly (a classified entity passes an "any classification" requirement instead of a false `CLASSIFICATION_MISSING`), and reports a new `CLASSIFICATION_UNRESOLVED` failure — distinct from `CLASSIFICATION_MISSING`/`CLASSIFICATION_VALUE_MISMATCH`/`CLASSIFICATION_SYSTEM_MISMATCH` — when a system/value-constrained facet cannot be verified because the matching classification's attributes are unreadable, instead of silently passing or failing on data it never read.
11+
12+
Both message formatters (`packages/ids/src/translation/service.ts` and `packages/ids/src/validation/validator.ts`) now have a case for `CLASSIFICATION_UNRESOLVED` — previously both fell through to their `default` branch and showed the raw enum ("Validation failed: CLASSIFICATION_UNRESOLVED") in the viewer's IDS panel and exported reports, indistinguishable from a genuine violation. The message now states plainly that the entity is classified but the details could not be read from this data source. `ClassificationCard.tsx` (properties panel) gets the same "unavailable on this data source" treatment already added to `ModelMetadataPanel.tsx`, instead of rendering an empty "Classification / Unknown" card for an unresolved entry.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@ifc-lite/sdk": patch
3+
---
4+
5+
`ClassificationData` (the SDK's public classification shape) gained an `unresolved?: boolean` field mirroring `@ifc-lite/parser`'s `ClassificationInfo.unresolved` (#3948/#3951), so a classified-but-unreadable entity (server-parsed store) can be told apart from a genuinely unclassified one through the SDK too.
6+
7+
The MCP playground's own `IFCDataAccessor` implementation (`apps/viewer/src/components/mcp/playground-dispatcher.ts`) built its `getClassifications` result from `m.bim.classifications(...)` (this same SDK shape) but dropped the `unresolved` marker — a second, independent reimplementation of the canonical bridge (`packages/ids/src/bridge/data-accessor.ts`) that the viewer's own IDS panel already uses correctly. Without the field, a classified-but-unresolved entity looked to `checkClassificationFacet` like a real classification with an empty system/value, so the agent's `ids_validate` tool reported a fabricated `CLASSIFICATION_SYSTEM_MISMATCH`/`CLASSIFICATION_VALUE_MISMATCH` instead of the honest `CLASSIFICATION_UNRESOLVED` the same fixture produces through the canonical bridge. Fixed by forwarding `unresolved` through the mapping.
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
4+
5+
/**
6+
* The playground's own `makeIdsAccessor().getClassifications` (used by the
7+
* `ids_validate` tool, the chat agent's IDS check) is a SEPARATE
8+
* reimplementation of `IFCDataAccessor` from `@ifc-lite/ids`'s
9+
* `createDataAccessor` (packages/ids/src/bridge/data-accessor.ts) — the same
10+
* "two paths that must agree" shape already caught once for materials (see
11+
* playground-dispatcher-materials.test.ts).
12+
*
13+
* Issue #3948 added a tri-state `unresolved` marker to `ClassificationInfo`
14+
* so a classified-but-unreadable entity (server-parsed / source-empty store)
15+
* can be told apart from a genuinely unclassified one. The canonical bridge
16+
* (`resolveClassifications` / `createDataAccessor`) forwards `unresolved`.
17+
* The playground's own `makeIdsAccessor` does not: it rebuilds a plain
18+
* object literal from `m.bim.classifications(...)` and drops the
19+
* `unresolved` field, so a classified-but-unresolved entity looks to the
20+
* IDS engine like a REAL classification with `system: ''` / `value: ''` —
21+
* which a system/value-constrained facet then reports as a genuine
22+
* CLASSIFICATION_SYSTEM_MISMATCH / CLASSIFICATION_VALUE_MISMATCH instead of
23+
* the honest CLASSIFICATION_UNRESOLVED the same fixture reports via the
24+
* canonical bridge (packages/ids/src/facets/classification-facet.server-parsed.test.ts).
25+
*
26+
* A normally-parsed (source-bearing) model never hits this: `source` is
27+
* always populated in that case, so this test simulates the server-parsed
28+
* shape the way #3948's own fixtures do — parse a real model (so the
29+
* relationship graph is genuine), then strip the fields a server-parsed
30+
* store never has (`source`, `onDemandClassificationMap`) so the resolver's
31+
* existing fallback path is what runs, not a hand-built assumption.
32+
*/
33+
34+
import { describe, it } from 'node:test';
35+
import assert from 'node:assert/strict';
36+
37+
import { dispatch, parsePlaygroundModel, type LoadedPlaygroundModel } from './playground-dispatcher.js';
38+
39+
function ifc4(body: string): string {
40+
return [
41+
'ISO-10303-21;', 'HEADER;', "FILE_DESCRIPTION((''),'2;1');",
42+
"FILE_NAME('','',(''),(''),'','','');", "FILE_SCHEMA(('IFC4'));", 'ENDSEC;',
43+
'DATA;', body, 'ENDSEC;', 'END-ISO-10303-21;', '',
44+
].join('\n');
45+
}
46+
47+
const CLASSIFIED_WALL = ifc4(`
48+
#100=IFCWALL('0Wall0000000000000001',$,'Wall A',$,$,$,$,$,$);
49+
#300=IFCCLASSIFICATION('Uniclass',$,$,'Uniclass 2015');
50+
#310=IFCCLASSIFICATIONREFERENCE('','Ss_25_10','Some Name',#300,$,$);
51+
#320=IFCRELASSOCIATESCLASSIFICATION('0RelCls00000000000001',$,$,$,(#100),#310);
52+
`);
53+
54+
const SYSTEM_IDS_XML = `<ids xmlns="http://standards.buildingsmart.org/IDS">
55+
<info><title>System check</title></info>
56+
<specifications>
57+
<specification name="Uniclass required" ifcVersion="IFC4" minOccurs="1" maxOccurs="unbounded">
58+
<applicability>
59+
<entity><name><simpleValue>IFCWALL</simpleValue></name></entity>
60+
</applicability>
61+
<requirements>
62+
<classification><system><simpleValue>Uniclass 2015</simpleValue></system></classification>
63+
</requirements>
64+
</specification>
65+
</specifications>
66+
</ids>`;
67+
68+
/** Force the parsed model's store into a server-parsed shape: no source
69+
* bytes, no on-demand classification map — the same condition that makes
70+
* `extractClassificationsOnDemand` fall back to the relationship graph and
71+
* report `unresolved: true` (issue #3948). The relationship edge itself
72+
* (`IfcRelAssociatesClassification`) is genuine — built by the real parser
73+
* from the fixture above — not fabricated. */
74+
function toServerParsedShape(model: LoadedPlaygroundModel): void {
75+
const store = model.store as unknown as { source?: Uint8Array; onDemandClassificationMap?: unknown };
76+
store.source = new Uint8Array(0);
77+
store.onDemandClassificationMap = undefined;
78+
}
79+
80+
async function runSystemCheck(): Promise<{ status: string; failureType?: string; failureReason?: string }> {
81+
const model = await parsePlaygroundModel(
82+
new TextEncoder().encode(CLASSIFIED_WALL).buffer as ArrayBuffer,
83+
'fixture.ifc',
84+
);
85+
toServerParsedShape(model);
86+
const result = await dispatch(model, 'ids_validate', { ids_xml: SYSTEM_IDS_XML });
87+
assert.equal(result.isError, false, `ids_validate should not error: ${result.text}`);
88+
const report = result.structured as {
89+
specificationResults: Array<{
90+
status: string;
91+
entityResults: Array<{ requirementResults: Array<{ status: string; failure?: { type: string }; failureReason?: string }> }>;
92+
}>;
93+
};
94+
const req = report.specificationResults[0]?.entityResults[0]?.requirementResults[0];
95+
return { status: req?.status ?? '(none)', failureType: req?.failure?.type, failureReason: req?.failureReason };
96+
}
97+
98+
describe('playground ids_validate — classification facet on a server-parsed-shaped store (#3948/#3951)', () => {
99+
it('reports CLASSIFICATION_UNRESOLVED, not a fabricated SYSTEM_MISMATCH, for a classified-but-unresolved entity', async () => {
100+
const { status, failureType, failureReason } = await runSystemCheck();
101+
assert.equal(status, 'fail', 'entity IS classified but unreadable here, so the facet cannot pass');
102+
assert.equal(
103+
failureType,
104+
'CLASSIFICATION_UNRESOLVED',
105+
`expected an honest "cannot verify" reason, got ${failureType} (${failureReason}) — ` +
106+
'the playground\'s own IDS accessor dropped the unresolved marker, making the entity ' +
107+
'look like a real classification with an empty system, which reads as a genuine mismatch',
108+
);
109+
assert.notEqual(failureType, 'CLASSIFICATION_SYSTEM_MISMATCH');
110+
assert.notEqual(failureType, 'CLASSIFICATION_MISSING');
111+
});
112+
});

apps/viewer/src/components/mcp/playground-dispatcher.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1655,8 +1655,7 @@ const IMPLS: Record<string, ToolImpl> = {
16551655
const t0 = Date.now();
16561656
const initial = v.getSelection();
16571657
if (initial.length > 0) {
1658-
// Already something selected — return immediately so the agent
1659-
// doesn't pointlessly stall.
1658+
// Already something selected — return immediately so the agent doesn't pointlessly stall.
16601659
return {
16611660
text: `Already selected ${initial.length} entit${initial.length === 1 ? 'y' : 'ies'}.`,
16621661
structured: { selection: initial, waitedMs: 0, timedOut: false },
@@ -1871,17 +1870,18 @@ function makeIdsAccessor(m: LoadedPlaygroundModel): import('@ifc-lite/ids').IFCD
18711870
}));
18721871
},
18731872
getClassifications(id) {
1873+
// Forward `unresolved` (#3948/#3951) — else a classified-but-unresolved entity reads as a fabricated empty match.
18741874
return m.bim.classifications(ref(id)).map((c) => ({
18751875
system: c.system ?? '',
18761876
value: c.identification ?? c.name ?? '',
18771877
name: c.name,
1878+
unresolved: c.unresolved,
18781879
}));
18791880
},
18801881
getMaterials(id) {
1881-
// Every variant via the same #1366 lens collector the material
1882-
// filter/list panels use. Previously only `mat.layers` and the
1883-
// top-level `mat.name` were checked, so a profile set, constituent
1884-
// set, or material list was invisible to IDS material requirements.
1882+
// Every variant via the same #1366 lens collector the material filter/list panels use.
1883+
// Previously only `mat.layers`/top-level `mat.name` were checked, so a profile set,
1884+
// constituent set, or material list was invisible to IDS material requirements.
18851885
return lensMaterialNames(m.bim.materials(ref(id))).map((name) => ({ name }));
18861886
},
18871887
getParent(id) {

apps/viewer/src/components/viewer/properties/ClassificationCard.tsx

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,23 @@ import { Tag } from 'lucide-react';
1111
import type { ClassificationInfo } from '@ifc-lite/parser';
1212

1313
export function ClassificationCard({ classification }: { classification: ClassificationInfo }) {
14+
// Matches the "unavailable on this data source" treatment in
15+
// ModelMetadataPanel: a server-parsed store (#3948) can prove the entity
16+
// is classified via the relationship graph without being able to read
17+
// any of the classification's own attributes. Without this branch the
18+
// card fell through to the general case below and rendered a
19+
// content-free "Classification / Unknown" card.
20+
if (classification.unresolved) {
21+
return (
22+
<div className="border-2 border-emerald-200 dark:border-emerald-800 bg-emerald-50/20 dark:bg-emerald-950/20 w-full max-w-full overflow-hidden flex items-center gap-2 p-2.5">
23+
<Tag className="h-3.5 w-3.5 text-emerald-600 dark:text-emerald-400 shrink-0" />
24+
<span className="text-xs text-zinc-500 dark:text-zinc-400">
25+
Classification present, but unavailable on this data source
26+
</span>
27+
</div>
28+
);
29+
}
30+
1431
const displayName = classification.identification || classification.name || 'Unknown';
1532
const systemName = classification.system;
1633

apps/viewer/src/components/viewer/properties/ModelMetadataPanel.tsx

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,12 @@ export function ModelMetadataPanel({ model }: { model: FederatedModel }) {
114114
// Classification systems used in THIS model (e.g. Uniclass, OmniClass, a
115115
// national system — a model can carry several at once). Walks only the
116116
// handful of IfcClassification entities via the byType index, so it's
117-
// cheap even on large models — not a per-element scan.
117+
// cheap even on large models — not a per-element scan. `unresolved` means
118+
// the model HAS classification systems but this store (server-parsed, no
119+
// source bytes) can't read their names — distinct from "genuinely none"
120+
// (#3948); `names` is always `[]` in that case.
118121
const classificationSystems = useMemo(() => {
119-
if (!dataStore) return [];
122+
if (!dataStore) return { names: [], unresolved: false };
120123
return extractClassificationSystemsOnDemand(dataStore as IfcDataStore);
121124
}, [dataStore]);
122125

@@ -301,13 +304,18 @@ export function ModelMetadataPanel({ model }: { model: FederatedModel }) {
301304
</h4>
302305
</div>
303306
<div className="divide-y divide-zinc-100 dark:divide-zinc-900">
304-
{classificationSystems.length === 0 ? (
307+
{classificationSystems.unresolved ? (
308+
<div className="flex items-center gap-3 px-3 py-2">
309+
<BookMarked className="h-3.5 w-3.5 text-zinc-400 shrink-0" />
310+
<span className="text-xs text-zinc-500">Classification systems present, but unavailable on this data source</span>
311+
</div>
312+
) : classificationSystems.names.length === 0 ? (
305313
<div className="flex items-center gap-3 px-3 py-2">
306314
<BookMarked className="h-3.5 w-3.5 text-zinc-400 shrink-0" />
307315
<span className="text-xs text-zinc-500">No classification systems</span>
308316
</div>
309317
) : (
310-
classificationSystems.map((system) => (
318+
classificationSystems.names.map((system) => (
311319
<div key={system} className="flex items-center gap-3 px-3 py-2">
312320
<BookMarked className="h-3.5 w-3.5 text-zinc-400 shrink-0" />
313321
<span className="text-xs font-mono text-zinc-900 dark:text-zinc-100">

packages/ids/src/bridge/classifications.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ interface ClassRecord {
1414
identification?: string;
1515
name?: string;
1616
path?: string[];
17+
unresolved?: boolean;
1718
}
1819

1920
/**
@@ -45,7 +46,7 @@ export function resolveClassifications(
4546
// even when the value is empty — so optional-cardinality value
4647
// mismatches register as a value mismatch rather than as a
4748
// missing-classification (which optional pardons).
48-
out.push({ system, value: baseValue, name: c.name });
49+
out.push({ system, value: baseValue, name: c.name, unresolved: c.unresolved });
4950
if (Array.isArray(c.path)) {
5051
for (const code of c.path) {
5152
if (code && code !== baseValue) {

0 commit comments

Comments
 (0)