diff --git a/.changeset/add-element-preview-side-normals-inward.md b/.changeset/add-element-preview-side-normals-inward.md deleted file mode 100644 index f6a3ac4b4..000000000 --- a/.changeset/add-element-preview-side-normals-inward.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -"@ifc-lite/viewer": patch ---- - -Fix inward-facing normals on the "add element" instant-preview mesh's side faces. - -`buildBoxFromIfcCorners` draws the instant-preview box the moment a builder tool commits, and is fed by two callers that wind their corner rings in **opposite** directions: `buildAxisBox` (column / door / window) lists its bottom ring counter-clockwise seen from IFC +Z, `buildLinearBox` (wall / beam / member) lists it clockwise. Each side face's normal came from `faceNormal(corners, a, b, c)`, whose sign follows that winding — so one fixed argument order was outward for one family and inward for the other. Columns, doors and windows previewed with all 4 side faces lit backwards until the export+re-parse round-trip replaced the preview with real geometry. - -Fixed by resolving the side normal's sign against the box centre rather than against the ring order: the cross product still supplies the face's axis, and the direction that points away from the centre is chosen (valid for any winding, since the box is convex). Both families now light correctly, and a future caller gets outward normals whatever ring order it uses. Vertex positions, the index buffer, per-vertex entity ids and the hardcoded top/bottom normals are byte-identical to before for every currently reachable shape. diff --git a/.changeset/agreement-pairs-round-2.md b/.changeset/agreement-pairs-round-2.md deleted file mode 100644 index 5a9b77892..000000000 --- a/.changeset/agreement-pairs-round-2.md +++ /dev/null @@ -1,54 +0,0 @@ ---- -"@ifc-lite/bcf": patch -"@ifc-lite/parser": minor -"@ifc-lite/cli": minor -"@ifc-lite/mcp": minor ---- - -Four places where two things had to agree and nothing made them. - -**BCF `` read back none of what it wrote.** BCF 2.1 and 3.0 both model `OriginatingSystem` and `AuthoringToolId` as child ELEMENTS of `` — only `IfcGuid` is an attribute. `writeComponent` emits the element form and its docstring says so; `parseComponent` matched `AuthoringToolId="…"` and `OriginatingSystem="…"` as attributes, which the element form never produces. Both fields were dropped from every archive read, whether ifc-lite wrote it or another tool did. Worse, the guard `if (!ifcGuidMatch && !authoringToolIdMatch) return undefined` used a match that could never fire, so a component identified only by its authoring-tool id — legal, `IfcGuid` is optional — was discarded whole rather than losing one field. - -The existing writer tests could not see it: no fixture set either field, so the reader's `undefined` looked like a faithful round-trip of an empty input rather than a dropped value. A writer and a reader that only ever meet each other agree with each other, not with the format. The reader now reads the element form (unescaping entities, like every other element it parses) and still accepts the attribute spelling as a fallback, so files from tools that emit the non-spec form keep working. - -**`ifc-lite clash`'s "Top 20" was not the top 20.** The engine returns `result.clashes` in `byKeyThenRule` grouping order. Both cap sites sliced that directly — `slice(0, 20)` for the human summary, `slice(0, 1000)` for `--json` — under a header reading `Top N of M clashes`, so on any run above the cap the deepest penetrations could sit past the cut and never be printed. `@ifc-lite/clash` has exported `sortClashes(clashes, 'distance')` for this the whole time, and the viewer's clash panel uses it; the MCP `clash_check` tool had independently hit the same problem and grown a local copy of the sort, minus the deterministic id tie-break. All three now call the one helper, so "top N" means the same N rows on every surface and equal-distance rows stop reshuffling between runs. - -**`ifc-lite mcp --allow-origin ` loaded the origin as a model file.** The standalone `ifc-lite-mcp` binary reads a flag and consumes its value in one branch, so it cannot disagree with itself. The `ifc-lite mcp` subcommand only needs to know WHICH flags carry a value, so it can skip them while collecting positional `.ifc` paths — and it kept a hand-written copy of that list. The copy drifted: `--allow-origin` reached the binary and never the list, so the subcommand skipped the flag, failed to skip the origin after it, and called `resolve('https://…')` as a model path. The flag tables now live in `@ifc-lite/mcp/cli-args` next to the binary's parser, which a test drives against them, and the subcommand imports them. Flags the subcommand cannot act on (`--allow-origin`, `--federate`) are now reported on stderr instead of silently appearing to work. `parseArgs` also stopped calling `process.exit` for `--help`/`--version` — it reports them and the binary acts — so it can be tested at all. - -**Three query backends, three copies of the same two lookup tables.** `IFC_SUBTYPES`, `expandTypes` and the `related()` relationship map were byte-identical in the viewer's `query-adapter`, `@ifc-lite/cli`'s `HeadlessBackend` and `@ifc-lite/mcp`'s `backend-query`, behind one SDK query API. Only the CLI copy had tests, so the other two were free to drift: deleting `IFCSLABELEMENTEDCASE` from the MCP copy left all 272 of its tests green, meaning `byType('IfcSlab')` could answer differently depending on which surface a caller reached. They now come from `@ifc-lite/parser`, the same home PR #3009's `isProductType` move used, and are covered there rather than by one consumer; that mutation now fails. `@ifc-lite/cli` and `@ifc-lite/mcp` keep publishing `expandTypes` under its old name, so no consumer surface changes. - -Putting the SDK's five-entry relationship map next to the parser's eighteen-entry `REL_TYPE_MAP` also makes visible, for the first time, that `related()` exposes five of the relationships the parser indexes — previously that narrowing was invisible in all three copies. Behaviour is unchanged; widening it is now a deliberate edit to one table. - -Also documented a near-miss: `harvestUpdatePaths` in `@ifc-lite/collab-server` pre-creates four of the five `TOP` shared types, omitting `annotations`, and reads like an enumeration missing an entry — which would make an `annotations/…` path lock unenforceable. It is not: `Y.applyUpdate` registers any top-level type the update names and `topLevelKeyOf` scans `doc.share`, so the path is harvested regardless. Verified by running, and pinned by two tests so a later "tidy-up" into a fixed list cannot quietly create the hole. - -**A fifth pair, found reviewing the fourth: the `` splitter read two components as one.** Fixing the field parsing above made this reachable, so it belongs in the same change rather than after it. The splitter was `]*(?:\/>|>[\s\S]*?<\/Component>)`, and `[^>]*` is greedy: it eats the `/` of a self-closing tag, so the `\/>` branch can never fire. A uniform list still parsed, because the engine backtracks and gives the `/` back when no later `` exists. A MIXED list did not. - -`writeComponent` emits `` for a component with no child elements and the full form for one with them, so an ordinary selection holding one of each produces exactly that mixed list. The pair matched as ONE element spanning both, and the first component silently inherited the second's `AuthoringToolId` and `OriginatingSystem`. Before this change that was data loss; with the field parsing working it is misattribution, which nothing downstream can detect. - -Every fixture in the suite held one shape, which is the one shape the defect cannot reach. There is now one splitter instead of two identical copies, in `parseComponentElements`, with fixtures for the mixed selection, the mixed coloring entry, and a uniform control. - -**The attribute fallback did not decode entities.** `AuthoringToolId="A & B"` came back as the literal `A & B` while `A & B` came back as `A & B`. Which spelling a file happens to use is not supposed to change the value. All three attribute reads now decode the same way `extractElement` does. - -**`reader.ts` was split.** The component, visibility and colouring parsers move to `reader-components.ts` and the XML text helpers to `xml-text.ts`. That is what put one splitter where there were two, and it takes `reader.ts` from 1204 lines to 1045. The module-size gate was genuinely RED before it (1204 against a 1190 budget), and the freed budget is banked rather than left as slack: the row drops to 1045 in the same commit that shrank the file. 1045 is still far above the ~400-line house guideline, so this pays a gate, not the rule behind it. - -**Two smaller ones in `@ifc-lite/mcp`.** `--help`/`--version` set `process.exitCode` and return instead of calling `process.exit(0)`, which can truncate stdout when it is a pipe. That makes `ifc-lite-mcp` match its sibling binary, `packages/cli/src/index.ts`, which already returns rather than exits. The same write-then-exit shape survives at about ten sites in `@ifc-lite/cli`'s subcommands; widening to those changes control flow (several exit non-zero) in a package this change does not otherwise open, so they are deliberately left. And four user-facing strings advertised the top clashes "by |distance|" while the code sorts by signed distance. The file's own docstring already warned that an absolute-value sort inverts the hard-clash order, so the text contradicted both the implementation and the comment beside it. - -**Reviewing the splitter fix turned up four more in the same file, three of them the same shape.** Fixing them here rather than filing them, because they live in the function the split just moved and the remedy is the one already applied. - -`` is schema-legal, since `` and `` are both optional. Matching only the paired form returned `undefined` for the WHOLE `` block, dropping the selection and colouring with it. That is the same missing self-closing branch as the component splitter, twenty lines away. - -`DefaultVisibility` was matched against the entire `` string rather than the `` element, so the attribute on any earlier element won. A file whose `` says `true` with a `DefaultVisibility="false"` anywhere ahead of it hid every element: the exact opposite of what it asked for. - -Attribute fallbacks were read from the whole element rather than its opening tag, so `` reported the child's `x` as the component's own. They also lacked the `\b` name anchor that `reader.ts`'s own `extractAttr` has, so `XAuthoringToolId="sneaky"` satisfied a search for `AuthoringToolId`. - -And an EMPTY value now reads as absent whichever spelling carries it. `` returned `''`, which passed the "a component needs some identity" guard with no identity, and `writeComponent` then wrote it back as a bare `` that the reader discards. Three spellings of nothing disagreeing is the defect this changeset opens with. - -`IfcGuid` is now entity-decoded like every other field, matching `writeComponent`, which already escapes it. A real IFC GUID contains no `&`, which is why nothing reached it. - -Each of these is pinned by a fixture that fails without its fix; all six were checked by reverting the fix and watching the fixture go red. - -**`unescapeXml` decodes numeric character references**, not only the five named entities `escapeXml` writes. Other authoring tools emit `&` and `&`, both legal XML, and those stayed encoded in the data. - -It is now a single pass rather than a chain of five `replace` calls. The chain had to decode `&` last, or a literal `<` written as `&lt;` was corrupted into `<` by the earlier pass; adding numeric forms to that chain reintroduces the same hazard from a second direction, since `&lt;` decodes to `<` and would be swept again. A single pass never looks at its own output, so the ordering question stops existing. An unrecognised or out-of-range reference is left untouched, because losing a character from someone else's archive is worse than leaving one encoded. - -**`clash_review` asked for something the data could not support.** The prompt requested a top-20 list "ordered by severity", but `clash_matrix` selects `sampleClashes` with `sortClashes(clashes, 'distance')` and caps it, so a high-severity clash with a large distance is not in the sample at all. A severity-ranked list built from it would silently omit exactly the items it claims to rank. The prompt now orders by distance and points at `bySeverity` for the severity picture, which is a complete count over every clash. The tool's own description says which half is complete and which is capped, and `clashReview.description` no longer says "prioritize by severity". diff --git a/.changeset/ask-json-exit-code-on-recipe-failure.md b/.changeset/ask-json-exit-code-on-recipe-failure.md deleted file mode 100644 index b8e84a5af..000000000 --- a/.changeset/ask-json-exit-code-on-recipe-failure.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -"@ifc-lite/cli": patch ---- - -Fix `ifc-lite ask "" --json` always exiting 0, even when the matched recipe throws. - -The recipe-execution catch branched on `--json`: the non-JSON path called `fatal()`, which hard-exits 1, but the JSON path only printed `{ error }` and fell through without setting `process.exitCode` — a caller reading just the exit code (a build pipeline, a script) saw success on a question that could not be answered. The `--json` path now sets `process.exitCode = 1` in that catch, matching the non-JSON verdict. diff --git a/.changeset/bcf-30-viewpoints-comments-nesting.md b/.changeset/bcf-30-viewpoints-comments-nesting.md deleted file mode 100644 index 1ef8d3ff1..000000000 --- a/.changeset/bcf-30-viewpoints-comments-nesting.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -"@ifc-lite/bcf": patch ---- - -Fix BCF 3.0 markup.bcf writing and reading the wrong `Comments`/`Viewpoints` structure. - -buildingSMART's BCF 3.0 `markup.xsd` moves `Comments` and `Viewpoints` inside `` (each wrapped in its own plural container, with per-entry `` — capital P, distinct from the `` a `` uses to reference one), after `RelatedTopics`. BCF 2.1 instead keeps them as top-level `` siblings after ``, in schema order `Comment*` then `Viewpoints*`. - -The writer previously emitted the 2.1-shaped flat siblings — `Viewpoints` before `Comment` — unconditionally for both versions, which is schema-invalid at 3.0 and out of order at 2.1. The reader's markup lookup only matched the 2.1 top-level `` shape, so on a genuine 3.0 file the per-viewpoint snapshot filename was silently dropped and resolution fell back to guessing our own `Snapshot_` naming convention. Verified empirically against buildingSMART/BCF-XML's own release_3_0 conformance fixture (`Test Cases/v3.0/Visualization/Perspective camera`): before the reader fix, the snapshot referenced by that fixture's `markup.bcf` was not attached to the parsed viewpoint. diff --git a/.changeset/bcf-reader-drops-fields-the-writer-emits.md b/.changeset/bcf-reader-drops-fields-the-writer-emits.md deleted file mode 100644 index 5e5f69a5a..000000000 --- a/.changeset/bcf-reader-drops-fields-the-writer-emits.md +++ /dev/null @@ -1,13 +0,0 @@ ---- -"@ifc-lite/bcf": patch ---- - -Fix three BCF fields the writer emits correctly but the reader silently dropped. - -Each one was invisible to the existing round-trip tests because no fixture ever populated it: `parse(write(x)) === x` held only because both sides saw `undefined`. - -- **`ViewSetupHints`.** `visinfo.xsd` puts `SpacesVisible` / `SpaceBoundariesVisible` / `OpeningsVisible` on `Components`. The writer emits them; no reader path looked for them, so every hint was lost on read. An attribute the file omits now stays `undefined` rather than collapsing to `false`. -- **`BimSnippet` attribute order.** The reader's regex anchored `SnippetType` to the first attribute position — which our own writer always satisfies — so a spec-correct file that writes `IsExternal` first had its entire snippet dropped. XML attribute order is not semantically significant. `IsExternal` now also accepts the `xs:boolean` `1`/`0` forms, matching how the `Header`/`File` flag is already read. -- **A project `Name` containing XML metacharacters.** `project.bcfp` is written with `escapeXml` but was read back with a raw regex instead of the shared `extractElement` helper, so the escape had no inverse: `A & B` came back as the literal `A & B`, and each re-export escaped it again. - -Covered by round-trip tests that set every affected field, plus two tests that feed the reader third-party-shaped XML directly rather than our own writer's output. diff --git a/.changeset/bcf-validate-against-official-xsds.md b/.changeset/bcf-validate-against-official-xsds.md deleted file mode 100644 index 55f33ad9a..000000000 --- a/.changeset/bcf-validate-against-official-xsds.md +++ /dev/null @@ -1,26 +0,0 @@ ---- -"@ifc-lite/bcf": major ---- - -Validate real `.bcfzip` output against buildingSMART's published BCF XSDs. That check found eleven schema violations in our output; nine are fixed here and two are reported below as needing a decision this change cannot make. - -Every existing test in this package is `parse(write(x)) === x`. That check cannot see a field both sides get wrong the same way — the writer and the reader agree with each other, not with the format. buildingSMART's `markup.xsd`, `visinfo.xsd`, `project.xsd` and `version.xsd` are an authority independent of this codebase, so they can. They are now vendored verbatim under `src/__fixtures__/schemas/` (CC BY-ND 4.0, which permits unmodified redistribution with attribution; see the `UPSTREAM_LICENSE` beside them) and `src/schema-validation.test.ts` writes a maximal archive — every optional field set, a distinct value in every position, cameras on a schema boundary — and validates each entry through `xmllint-wasm`, a WebAssembly libxml2 build with no native dependencies and no network access. It is a `devDependency` pinned to an exact version; nothing ships in the published package. - -**BCF 2.1 — affects every archive ifc-lite writes by default,** since `createBCFProject` defaults to 2.1 and both `@ifc-lite/cli` and `@ifc-lite/mcp` use that default: - -- **Viewpoint bitmaps used a container that does not exist in BCF 2.1.** `visinfo.xsd` repeats `` directly under `` and names the per-entry format element `` as well (`PNG…`). The writer emitted the BCF 3.0 shape — a `` wrapper with `` children — for both versions, so no 2.1 archive we wrote had schema-valid bitmaps. The reader matched the same non-2.1 shape, and required the wrapper to be present at all (`if (!bitmapsMatch) return bitmaps`), so it dropped every bitmap from a conformant 2.1 file written by any other tool. Its inner match was also a plain non-greedy `…`, which on the real 2.1 shape terminates at the nested format tag's closing tag rather than the entry's. Verified against buildingSMART's own `release_2_1` conformance archives: before this change, `readBCF` recovered 0 bitmaps across all 124 of them; after, it recovers the 2 that the `v2.1/Markup/MaximumInformation` fixture contains. - -**BCF 3.0 — the `bcf.version`, `project.bcfp` and `.bcfv` writers ignored the version argument entirely** and emitted 2.1 shapes into 3.0 archives: - -- `` in 3.0's `version.xsd` has an empty content type, so `` is not allowed. It must be written self-closing: libxml2 reports even the whitespace inside a `…` pair as character content against an empty content model. -- 3.0's `project.xsd` renames the root element from `` to ``. -- 3.0's `markup.xsd` gives `` a single container holding `