perf(bindx): memoize the dirty-entity scan per store write version (#65)

[matej21]

Addresses the memoization half of #65. Deliberately does not close it — see Scope.

Problem

ChangeRegistry.getDirtyEntities() runs a full-store scan: deepEqual of data vs serverData for every snapshot, a reachability walk, and a per-entity dirtyFields/dirtyRelations pass. usePersist feeds it to useSyncExternalStore, which React runs synchronously inside every store notification, and list reorder helpers emit one notification per reindexed item. A single delete in a large sortable list therefore performs O(N·M) full scans before React renders.

Why not key the memo on getVersion()

This is the interesting part. SnapshotStore.getVersion() is SubscriptionManager's globalVersion, bumped only inside the notifying paths — and several dirtiness-changing writes do not notify:

path	what it changes silently
SnapshotStore.createEntity	roots.register() runs after the last notification
SnapshotStore.registerParentChild	roots.unregister(), no notification
commitAllRelations / resetAllRelations	no notification at all — called by BatchPersister on every persist success and rollback
refreshServerData(…, skipNotify=true)	called from HasManyListHandle / HasOneHandle

The first one alone is fatal: usePersist subscribes globally, so React runs its getSnapshot synchronously inside createEntity's own notification — before the root is registered. A version-keyed memo would be populated with an empty dirty set at a version createEntity never bumps again, leaving the Save button permanently dead. Today's unconditional rescan hides this.

Change

getDirtyVersion() sums monotonic sub-store mutation counters — the pattern ReachabilityAnalyzer already uses:

entitySnapshots.getDataWriteVersion()   (new)
+ meta.getMutationVersion()
+ meta.getEditableWriteVersion()        (schedule/unscheduleForDeletion bump only this)
+ relations.getMutationVersion()
+ roots.getMutationVersion()            (closes the createEntity / registerParentChild holes)

A sum of monotonic counters is strictly increasing on any bump, so an unchanged sum proves no dirtiness-relevant write happened — independent of whether anything was notified. The new dataWriteVersion is required because neither existing counter suffices: mutationVersion misses value edits, and editableWriteVersion deliberately excludes server-baseline writes.

getDirtyEntitiesNotInFlight() keeps filtering on every call, since in-flight state changes without any store write.

Scope

Memoization only. The issue lists three composable fixes; the other two — coalescing the notification storm, and incremental dirty tracking — are out of scope and the issue should stay open for them. The win is a factor of M (the number of mounted persist hooks), not of N. An earlier version of this description claimed each notification becomes O(1); that was wrong and is corrected below.

The four missing notifications above are real bugs in their own right. This change is correct despite them by construction, and deliberately does not fix them — but any consumer relying on getVersion() or on a store notification to observe those state changes is still wrong.

Tests

tests/unit/persistence/dirtyEntitiesMemoization.test.ts — 7 cases, written failing-first (2 failed before the change: no memo at all, and one full scan per call). The key itself is pinned, not just the presence of a cache: swapping getDirtyVersion() back to getVersion() turns 3 tests red, including one that reproduces the createEntity hole directly.

Verified: tests/unit/store + tests/unit/persistence 283 pass, tests/react 326 pass, full CI suite 1545 pass, typecheck clean.

Not verified

The test counts scans, not wall-clock — it proves the walk runs once per write version and says nothing about latency; there is no benchmark here. The memo also returns the same array instance within a version; all current consumers treat it read-only and the type is readonly DirtyEntity[], but nothing enforces that at runtime.

The Browser Tests check is red for an unrelated known reason — CI installs agent-browser unpinned and the popover click behaviour changed in the 0.32.x line. The suite is 66/66 green locally on an older driver. Being fixed separately.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GiniFfaE4gb5EuQpQ3Ncee

---

Measured (this was previously asserted without measurement)

Issue #65's scenario: 100 blocks each with a rich-text JSON column, removeFromHasMany + 99 setFieldValue from the order repair, M global subscribers reading inside every notification. cpu-lease run -n 2 --no-smt, 200 rounds, both paths warmed, two independent runs:

M	reads	scans before	scans after	ms before	ms after	speedup
1	100	100	100	1972 / 1941	1854 / 1872	1.06× / 1.04×
2	200	200	100	3634 / 3615	2010 / 1841	1.81× / 1.96×
4	400	400	100	7365 / 7190	1850 / 1758	3.98× / 4.09×

Scans after are 100 regardless of M. The memo deduplicates the 2nd..Mth read within one notification and nothing else — every setFieldValue in the repair loop legitimately bumps dataWriteVersion, so the first read in each of the N notifications still scans. On a page with a single save button the win is ~1.0×, i.e. zero.

This is exactly why #65 stays open: the memo is a prerequisite for the notification-coalescing fix, which is where N actually goes away.

Follow-up commit: guard the key

The memo is only as correct as its key, and dataWriteVersion++ was hand-maintained across 11 sites. Forgetting one — a new method, or a new early-return branch — fails silently: a stale dirty set for the rest of the session, surfacing as a dead Save button.

• Writes now go through writeSnapshot / deleteSnapshot, which own the bump — the shape HasOneStore already uses for writeRelation / deleteRelation. bumpVersion is the one documented bypass (same data/serverData refs, so it cannot change dirtiness, and it runs once per ancestor per notification, so bumping there would keep the cache permanently cold).
• A guard test classifies every name on EntitySnapshotStore.prototype into mutating / non-mutating / internal, fails on anything unclassified, then asserts each mutating method moves the key and each non-mutating one does not. Both halves were verified to bite. This is what actually stops the regression; the chokepoint just makes it hard to reach.
• hasDirtyEntities() now shares the memo instead of running its own full scan.
• getDirtyVersion() is @internal and documents a real constraint: createEntitySnapshot freezes only the top level, so a nested value mutated in place changes dirtiness without touching any counter and the memo serves the pre-edit answer. Documented, not enforced — deep-freezing is a separate perf and BC decision.
• The memoization test no longer asserts that a mid-write read sees an empty dirty set. That pinned one of the four known missing-notification bugs as expected behaviour and would have handed a red test to whoever eventually fixes it.

One review suggestion declined

Reusing ReachabilityAnalyzer.graphVersion() instead of maintaining a second sum was suggested and not taken. It is key-equivalent (its bump set is a strict subset), but it would couple the dirty key to a reachability key that a future author could legitimately narrow, and the new guard test already covers the drift risk directly. Making graphVersion() public for this would also widen surface for no behavioural gain.

Independent review

Verified by a differential fuzz — 5000 seeds × 60 steps ≈ 296,000 operations including undo/redo with a live journal, transaction(), rekey, importPartialSnapshot and sweepUnreachableCreated, with a global subscriber calling getDirtyEntities() from inside every notification to poison the memo mid-write. Zero stale memos, zero non-monotonic key steps. Every counter confirmed ++-only, never reset, so the sum is strictly increasing and no two states collide.

Branch gated in isolation: typecheck clean, bun run test 1561 pass / 0 fail.