Composition: `null` deletes `children`/`inherits` but not `attributes`

## TL;DR

- A later layer's `children: {"x": null}` or `inherits: {"x": null}` deletes the entry
during composition, but `attributes: {"x": null}` is copied through as a literal `null`
forever — the key is never removed.
- The repo's own `Diff()` emits `null` markers for removed children and removed attributes
uniformly, so a `Diff()` output cannot be losslessly re-applied through
`Federate` + `LoadIfcxFile`: the child deletion takes effect, the attribute deletion
doesn't.
- The fix is small: give the two `attributes` loops in `compose.ts` the same
delete-on-`null` branch that `children`/`inherits` already have.

## Environment

- Commit: `02b0b21aaa4ab10354cdf931a6ec0d704d6e8d22` (current `main`)
- Node v25.9.0, CLI built per the repository's own instructions:
`cd src && npm install && npx esbuild ifcx-cli/ifcx-cli.ts --bundle --outfile=ifcx-cli.js --external:three --platform=node`

## The gap

`FlattenPathToPreCompositionNode` special-cases `null` for `inherits` (deletes the key),
and one composition stage later `AddDataFromPreComposition` does the same for `children`.
Neither function's `attributes` loop has an equivalent branch:

https://github.com/buildingSMART/IFC5-development/blob/02b0b21aaa4ab10354cdf931a6ec0d704d6e8d22/src/ifcx-core/composition/compose.ts#L52-L54

```ts
Object.keys(node.attributes).forEach((attrName) => {
compositionNode.attributes[attrName] = node.attributes[attrName];
})
```

The second loop (`AddDataFromPreComposition`, L177-179) is equally unconditional:
`node.attributes.set(attrID, attr)`. There is no code path anywhere in composition that
removes an attribute key when its value is `null`.

## Reproduction 1: attribute set in one layer, nulled in the next

base.ifcx + override-null.ifcx — a String-typed attribute set to "A", then to null in a later layer

`base.ifcx`:
```json
{
"header": {
"id": "repro/minimal/base.ifcx",
"ifcxVersion": "ifcx_alpha",
"dataVersion": "1.0.0",
"author": "repro",
"timestamp": "2026-07-22"
},
"imports": [],
"schemas": {
"test::rating": {
"uri": "https://example.com/repro/test-rating",
"value": { "dataType": "String" }
}
},
"data": [
{ "path": "wall-0001", "attributes": { "test::rating": "A" } }
]
}
```

`override-null.ifcx`:
```json
{
"header": {
"id": "repro/minimal/override-null.ifcx",
"ifcxVersion": "ifcx_alpha",
"dataVersion": "1.0.0",
"author": "repro",
"timestamp": "2026-07-22"
},
"imports": [],
"schemas": {},
"data": [
{ "path": "wall-0001", "attributes": { "test::rating": null } }
]
}
```

Composing these two layers (validation bypassed — see note below) produces:

```json
{
"node": "wall-0001",
"children": {},
"attributes": { "test::rating": null }
}
```

`test::rating` is retained with the literal value `null` — not deleted, not reverted to
`"A"`.

> **Note:** this can't be observed through the CLI directly, because `--no-validate`
> doesn't actually bypass validation — `IfcxLayerStack`'s constructor always runs
> `LoadIfcxFile` with the default `checkSchemas = true`, so the run dies first with
> `SchemaValidationError: Error validating ["wall-0001"].attributes["test::rating"]:
> Expected "null" to be of type string`. That plumbing bug is reported separately
> (see companion issue #133),
> and it is also what has kept this one invisible. The output above comes from a ~20-line
> helper that calls `Federate` + `LoadIfcxFile(federated, /*checkSchemas*/ false, true)`
> directly — the same functions `IfcxLayerStack` uses internally.

## Reproduction 2: `Diff()` output cannot be re-applied

A base file has a child (`opening-0001`) and an attribute (`test::rating`) on
`wall-0001`; an edited copy removes both. The repo's own `Diff(base, edited)` emits the
same `null` marker for both removals, as designed:

```json
{
"path": "wall-0001",
"children": { "opening-0001": null },
"inherits": {},
"attributes": { "test::rating": null }
}
```

Re-applying that diff through `Federate([base, diff])` + `LoadIfcxFile` (no validation):

- `children`: `opening-0001` is **gone** — deleted, as intended
- `attributes`: `test::rating` is **still present**, with value `null`

The composition engine disagrees with the tool's own diff output: attribute removals
round-trip lossily, child/inherit removals round-trip fine.

## Why I believe this is an omission, not a design choice

1. `f54b852` ("Deletes", 2025-03-21) added the delete-on-`null` branches for
`children`/`inherits` **and a test named "delete removes attributes in order"** in the
same commit — the intended semantics for `attributes` are stated right there. The test
has passed vacuously ever since: chai's `.to.not.exist` accepts a literal `null`, and
the test helpers can't express two layers (details below).
2. `DiffNodes`/`Collapse` (`00f89e9`, same day) treat `children`/`inherits`/`attributes`
uniformly: a removal becomes an explicit `null` marker for all three.
3. The internal types already agree: `CompositionInputNode` declares
`attributes: {[key: string]: any | null}` — the same nullable shape as
`children`/`inherits` (`node.ts` L3-9).
4. Nothing anywhere commits to "`null` is a legitimate attribute value": `Validate()`
rejects `null` for schema-declared attributes of every dataType (`Object` only
indirectly, via nested members), and none of the 47 example `.ifcx` files uses `null`
as an attribute value.

Full evidence: commit history, types, and why the existing tests can't catch this

### Commit history (one author, one day: 2025-03-21)

- `f54b852` ("Deletes", 10:16 UTC+1) added the delete-on-`null` branch for `children` and
`inherits` to the (then `docs/viewer/`-hosted) prototype of this same flatten function,
plus three tests: "delete removes children in order", "delete removes inherits in
order" — and **"delete removes attributes in order"**. The `attributes` loop in the
same commit is untouched — it already did, and still does, an unconditional assign:
https://github.com/buildingSMART/IFC5-development/commit/f54b852341124331e3dc467d951dfe4506fa1c99
- That prototype then migrated file-by-file into `src/ifcx-core/composition/compose.ts`:
`54ead4a` ("Rename files", 13:54 UTC+1) renamed it to `compose-alpha.ts`, and
`6fb2d05` → `0ae7574` → `156348b` (2025-06-07) moved it under `src/ifcx-core/` and
split it into today's layout. Across every commit in that chain, both `attributes`
loops are byte-for-byte unchanged (only surrounding type/function renames).
- `00f89e9` ("Diff workflow", 15:48 UTC+1) added `DiffNodes` and `Collapse` (at the time
still inside `docs/viewer/compose-alpha.ts`; the later file splits moved them into
`workflows.ts` unchanged), and there `attributes` gets exactly the same treatment as
`children` and `inherits`:

https://github.com/buildingSMART/IFC5-development/blob/02b0b21aaa4ab10354cdf931a6ec0d704d6e8d22/src/ifcx-core/workflows.ts#L66-L97

```ts
Object.keys(node1.attributes).forEach((name) => {
if (!DeepEqual(node1.attributes[name], node2.attributes[name]))
{
result.attributes![name] = node2.attributes[name] ? node2.attributes[name] : null;
}
})
```

Same file, same commit, `Collapse` (used by both `Diff` and `Federate`/`Prune`):
https://github.com/buildingSMART/IFC5-development/blob/02b0b21aaa4ab10354cdf931a6ec0d704d6e8d22/src/ifcx-core/workflows.ts#L179-L217

Flatten was written first and never revisited once `Diff`/`Collapse`, five and a half
hours later the same day, confirmed that `null` was meant to mean "delete" for all three
fields, not just two of them.

### Types

`CompositionInputNode` types all three fields the same way:

https://github.com/buildingSMART/IFC5-development/blob/02b0b21aaa4ab10354cdf931a6ec0d704d6e8d22/src/ifcx-core/composition/node.ts#L3-L9
```ts
export interface CompositionInputNode
{
path: string;
children: {[key: string]: string | null};
inherits: {[key: string]: string | null};
attributes: {[key: string]: any | null};
}
```

The public `.tsp` schema, by contrast, only spells out the `| null` case for
`children`/`inherits` and leaves `attributes` as an untyped `Record`:

https://github.com/buildingSMART/IFC5-development/blob/02b0b21aaa4ab10354cdf931a6ec0d704d6e8d22/schema/ifcx.tsp#L16-L18
```
children?: Record;
inherits?: Record;
attributes?: Record;
```

So the internal type already documents "attributes can be null, same as the other two" —
it's the runtime code and the public schema that never followed through.

### Validation tests and examples

https://github.com/buildingSMART/IFC5-development/blob/02b0b21aaa4ab10354cdf931a6ec0d704d6e8d22/src/test/schema-test.ts#L20-L46

`Boolean`, `String`, `DateTime`, `Enum`, `Integer`, `Real`, `Reference`, and `Array` are
each explicitly asserted to throw `SchemaValidationError` on a `null` value (`Object` is
the one exception — it is exercised with `false` at the top level, and with `null` only
for nested members via `example::optional_object`; 8 of the 9 dataTypes carry the direct
top-level `null` assertion). None of the 47 `.ifcx` files under `examples/` uses `null`
on an attribute either.

### Why the existing test can't catch this

`compose-test.ts`'s `AddAttribute`/`AddChild`/`AddInherits` helpers always write into
`nodes.get(path)![0]`, i.e. a single input node, so a test that calls them twice for the
same path/name is mutating one JS object in place, not modeling two composition layers:

https://github.com/buildingSMART/IFC5-development/blob/02b0b21aaa4ab10354cdf931a6ec0d704d6e8d22/src/test/compose-test.ts#L22-L47

And the assertion style doesn't distinguish the two outcomes either — `NodeToJSON` copies
whatever value is in the attributes map (including a literal `null`) straight into the
output object, and chai's `.to.not.exist` is satisfied by both "key absent" and "key
present with value `null`":

https://github.com/buildingSMART/IFC5-development/blob/02b0b21aaa4ab10354cdf931a6ec0d704d6e8d22/src/test/compose-test.ts#L208-L219
```ts
it("delete removes attributes in order", () => {
...
expect(root.attributes.a1).to.exist;
expect(root.attributes.a2).to.not.exist;
});
```
`root.attributes.a2` ends up `null` here (not deleted), and the test passes anyway. This
test would pass identically whether or not flatten ever deleted the key.

## Suggested fix

- Add a `null`-means-delete branch to the `attributes` loops in
`FlattenPathToPreCompositionNode` and `AddDataFromPreComposition` (`compose.ts`),
matching the existing `inherits`/`children` branches.
- Fix `compose-test.ts`'s `AddChild`/`AddInherits`/`AddAttribute` helpers to push a new
`CompositionInputNode` per call instead of overwriting `nodes.get(path)![0]`, so tests
can express "layer N sets X, layer N+1 sets X to null" — and strengthen the
delete-related assertions to check key absence (`Object.hasOwn` /
`.to.not.have.property`) instead of `.to.not.exist`, so a retained `null` can't pass as
"deleted" again.

Happy to send a PR once there's agreement on whether a deleted attribute should simply be
removed from the map, or whether that should also propagate to `Prune`'s `deleteEmpty`
handling.

(Side note: `DiffNodes`' truthiness check `node2.attributes[name] ? ... : null` also
degrades legitimate `0`/`false`/`""` values to `null` markers — probably worth fixing in
the same pass.)

Start in src/ifcx-core/composition/compose.ts at FlattenPathToPreCompositionNode and AddDataFromPreComposition, comparing their attributes loops with the existing children and inherits deletion branches. Then inspect src/test/compose-test.ts and run the composition tests, ensuring they model two layers and distinguish an absent key from a literal null. Done means null attribute markers are removed during composition and Diff output round-trips attribute deletions like child deletions.

Written by the indexing model from the issue text.