fix(viewer): composeTeardown's equality filter is now structural and per-key

[BIMvoice]

Summary

Fixes #3346.

composeTeardown dropped a contribution's key only when Object.is matched the live state. visibilitySlice.teardown.ts and selectionSlice.teardown.ts gate their model-removed arm with one touched boolean over several fields, so once any single field named the removed model, every field in that group was rebuilt — including a sibling Set/Map/array whose own content did not change. The rebuilt-but-equal collection is a fresh reference, so Object.is could not see through it and wrote it into the composed patch anyway, defeating any Object.is / memo check downstream.

Root cause, one sentence: the equality gate that decides whether a teardown key gets written was per-slice (a single touched boolean covering several fields) at the contribution level and Object.is-only at the composition level, so neither layer could catch a sibling field being rebuilt equal-but-new.

Fix: composeTeardown (apps/viewer/src/store/teardown.ts) now compares structurally — Set/Map/array, recursing into Map values — before dropping a key, so an untouched field keeps its identity regardless of which sibling field tripped the group's touched gate.

One consequence needed handling: NEVER_DROPPED forces isolatedEntities / ghostExceptEntities into the patch so withVisibilityOwnershipInvalidation keeps running even when their value hasn't changed. That's a real requirement for session-reset / all-models-cleared, whose contributions declare both keys unconditionally — but model-removed never made that promise (its own touched gate already decides presence), so forcing presence there just reproduced the reported bug for exactly those two keys. NEVER_DROPPED's exemption is now scoped to session-reset / all-models-cleared only (FORCED_PRESENCE_SCOPES).

Proof of RED, then GREEN

New file apps/viewer/src/store/teardown.object-is-gate.test.ts, two cases (visibilitySlice, selectionSlice), asserting presence in the composed patch for an untouched field — value-equality assertions can't see a reference change, so this asserts on the same signal an Object.is subscriber would.

Before the fix (source unmodified, only the new test added):

not ok 1 - visibilitySlice: hiddenEntitiesByModel naming the removed model must not rewrite
            isolatedEntities / ghostExceptEntities, which name only a survivor
  error: 'isolatedEntities did not change value (1050 survives in model B) and must not be
          rewritten as an equal-but-new Set just because a sibling field in the same slice moved'
not ok 2 - selectionSlice: selectedModelId naming the removed model must not rewrite
            selectedEntities / selectedEntitiesSet, which name only a survivor
  error: 'selectedEntities holds only a B-owned entry and must not be rewritten as an
          equal-but-new array just because selectedModelId, a sibling field, named the removed model'

After the fix:

$ npx tsx --import ./src/test/vite-module-hooks.mjs --test --test-timeout=120000 --test-concurrency=1 \
    src/store/teardown-registry.test.ts src/store/teardown.idempotence.test.ts src/store/teardown.object-is-gate.test.ts
# tests 9
# suites 3
# pass 9
# fail 0

Wider run (visibility ownership + slice tests) also green:

$ npx tsx --import ./src/test/vite-module-hooks.mjs --test --test-timeout=120000 --test-concurrency=1 \
    src/store/teardown-registry.test.ts src/store/teardown.idempotence.test.ts src/store/teardown.object-is-gate.test.ts \
    src/store/visibility-channel-invalidation.test.ts src/store/slices/visibilitySlice.test.ts \
    src/store/slices/selectionSlice.test.ts src/store/slices/modelSlice.test.ts
# tests 184
# pass 184
# fail 0

pnpm typecheck (root) and node scripts/check-module-size.mjs both pass; teardown.ts is 399 lines (comments trimmed to stay under the 400-line ratchet rather than allowlisting).

What didn't change, and why

• No owns list, no pin (PINNED_SESSION_RESET_KEYS / PINNED_ALL_MODELS_CLEARED_KEYS / PINNED_MODEL_REMOVED_KEYS / PINNED_OWNED_KEYS) moved. Those pins read raw entry.teardown(scope, state) output, bypassing composeTeardown entirely — this fix only changes which unchanged keys the composition drops, not what any contribution's body writes.
• visibilitySlice.teardown.ts / selectionSlice.teardown.ts themselves are unchanged: the issue's proposed fix (give the seam structural equality) is what's implemented, keeping the per-slice touched pre-check as a cheap common-case skip rather than removing it (the issue measured that removal as a net perf loss in the common case).

#3345 — reported, not fixed

Read #3345 ("a fourth teardown scope would silently no-op in 22 of 28 slice contributions"). It's a real, separate defect: the fix there is making TeardownScope's arms a required record per contribution instead of an if (scope.kind !== 'session-reset') return {} guard, so a fourth scope kind fails to compile in all 28 files rather than silently clearing nothing. That's a shape change to every *.teardown.ts file and to SliceTeardown's declaration surface — not something this PR's fix touches or depends on, and not inseparable from it. Leaving it for its own PR.

Overlap with in-flight teardown PRs

Per the task brief, checked all four open PRs against this mechanism (#3371, #3372, #3375, #3367) for textual overlap — none of them touch apps/viewer/src/store/teardown.ts (this PR's only source change) or the new test file, so there's no diff conflict to resolve. (#3372, #3375, #3367 all touch teardown-registry.test.ts, which this PR does not.)

🤖 Generated with Claude Code

https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 2 billable files and costs up to $0.50.}

Or wait 45 minutes for your next included review.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0b7821f8-b17c-41cd-a186-d21b2b326e48

📥 Commits

Reviewing files that changed from the base of the PR and between 5a431e5 and 1fb9898.

📒 Files selected for processing (2)

• apps/viewer/src/store/teardown.object-is-gate.test.ts
• apps/viewer/src/store/teardown.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Viewer benchmark

✅ No threshold regressions detected.

01_Snowdon_Towers_Sample_Structural(1).ifc

Baseline recorded 2026-07-01T20:31:05.538Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.

Metric	Current	Baseline	Delta	Threshold	Status
firstBatchWaitMs	1686ms	2905ms	-42.0%	+50%	✅
firstVisibleGeometryMs	2345ms	3652ms	-35.8%	+50%	✅
streamCompleteMs	2878ms	3598ms	-20.0%	+50%	✅
spatialReadyMs	1228ms	1032ms	+19.0%	+50%	✅
metadataCompleteMs	1816ms	3063ms	-40.7%	+50%	✅
totalWallClockMs	3200ms	3700ms	-13.5%	+50%	✅

AC20-FZK-Haus.ifc

Baseline recorded 2026-07-01T20:30:59.972Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.

Metric	Current	Baseline	Delta	Threshold	Status
firstBatchWaitMs	305ms	1075ms	-71.6%	+50%	✅
firstVisibleGeometryMs	821ms	1572ms	-47.8%	+50%	✅
streamCompleteMs	1104ms	1980ms	-44.2%	+50%	✅
spatialReadyMs	1130ms	915ms	+23.5%	+50%	✅
metadataCompleteMs	1222ms	1392ms	-12.2%	+50%	✅
totalWallClockMs	1600ms	3300ms	-51.5%	+50%	✅

Refresh the baseline from a CI run: dispatch the Benchmark workflow with record_baseline, download the benchmark-baseline artifact, and commit baseline.json (see tests/benchmark/README.md).