composeTeardown's Object.is gate is per slice, so one moved field rewrites the rest as equal-but-new #3346

composeTeardown drops entries with Object.is, which cannot see through a rebuilt Set or Map. Two contributions work around that with a whole-slice touched pre-check, and because the gate is per slice rather than per key, one moved field still causes the others to be rewritten as equal-but-new collections.

This describes code on refactor/store-teardown-seam, not on main. It lands with the store-teardown-seam PR and is a follow-up to it.

What is wrong

apps/viewer/src/store/slices/visibilitySlice.teardown.ts computes one touched boolean over six fields, then returns all six rebuilt when any of them moved:

const touched =
  (priorHidden !== undefined && [...priorHidden].some(isStale)) ||
  ...
  state.hiddenEntitiesByModel?.has(modelId) === true ||
  state.isolatedEntitiesByModel?.has(modelId) === true;

if (!touched) return {};

So if only hiddenEntitiesByModel names the removed model, isolatedEntities and ghostExceptEntities are still rebuilt through nonEmptyOrNull into fresh Set objects with identical contents. Object.is will not drop them, so they are written, every subscriber of those keys re-renders, and both go through withVisibilityOwnershipInvalidation for a set that did not move.

That is the hazard the comment three lines above the guard says the guard prevents:

// Nothing of ours named the removed model. Returning {} rather than a set
// of equal-but-new collections is what keeps this scope idempotent

It is right about the all-or-nothing case and wrong about the mixed one.

apps/viewer/src/store/slices/selectionSlice.teardown.ts has the same per-slice gate shape.

Scope, corrected

An earlier note put this at ten files. It is two. scheduleSlice.ts also contains a touchedKeys local but it is undo bookkeeping, unrelated to teardown.

What this is not

It is not a performance problem, and that was measured rather than assumed. Replacing the pre-check with build-once-and-compare-size was benchmarked at N=10 models, M=100k ids:

nothing stale:   precheck+rebuild 2.65 ms   build-once 6.81 ms
0.01% stale:     precheck+rebuild 6.38 ms   build-once 5.59 ms
50% stale:       precheck+rebuild 4.85 ms   build-once 4.00 ms

The existing pre-check wins the common case. This issue is about correctness of the idempotence contract and about the contract living in two contribution bodies instead of in the seam.

Proposed fix

Give composeTeardown structural equality for Set, Map and array, compared per key rather than per slice. Then no contribution needs a touched boolean, both pre-checks and their comments go away, the mixed case stops writing unmoved keys, and idempotence becomes a property of the seam rather than a habit each contribution has to remember.

Cost: roughly 25 lines of compare in teardown.ts, run over collections the contribution already built.

How this was found

During an architecture review of the viewer store teardown seam. No code change for it has been made.