[ntucker]

Requested by Nathaniel

This PR is stacked on #4459. The plan is to fold it into #4459. It is separate so that you can review the commit signal by itself.

Motivation

Improvement. A component shows a result with useSuspense(), a commit changes that result, and then a transition retries while nothing else reads the result.

Example: a user edits a todo title, which commits, and a filter change then starts a transition. Each retry of the transition builds new entities, and each retry is read once. With #4459, those one-off reads can evict the committed result. When React renders the committed store again, the memo computes the result again and returns a new object with the same values. Components that compare by identity render again for no reason. Master does not do this.

Solution

• API: MemoCache.commit(entities, settled = true) is new. The memo on Controller has a new optional commit, so custom memos without it continue to work.
• Behavior: DataStore calls commit from an effect after each commit. Effects run only for committed renders, so a one-off render cannot pin anything. Vue has no concurrent rendering and does not call it.
• Internal: MemoCache.denormalize records the result's own entry for each store entities object, one per node: the endpoints-tier entry, or, when the input is a pk (single-entity endpoints, entity and Collection queries), the top-level entity's entry. commit pins those entries. Reads of the latest committed entities are pinned immediately, which covers components that mount later. Only those calls pass track, and the memo reads and clears the record right away, so nested entity maps never hold one.
• Trade-off: each node keeps up to three pinned entries plus the two most recently used unpinned ones. The pins are: committed (last read by a committed render), settled (last read by a committed render with no optimistic updates) and flipped back to (last entry the store returned to).

Architecture change

The memo now learns from React which results committed, not only which results it read last.

dataclient-memo-commit-pins.mp4

Take the example above. The commit renders result R from entities E1. useSuspense's own useMemo serves the urgent re-renders, so the memo never sees E1 again. The transition retries three times, with E2, E3 and E4. With #4459, each retry adds an entry, and the node keeps three, so R is evicted. React then renders the committed store, E1, and the memo computes R again with a new identity.

With this PR, the commit runs DataStore's effect, which calls memo.commit(E1). The memo pins R as committed. The retries add and evict their own entries, but R stays. The render of E1 hits.

The settled pin covers optimistic updates. Each optimistic update builds new entities for all that it touches, on each store change. Thus, when the last one settles, the store can return only to the result from before the first one. That result is pinned as settled, for any number of updates in flight.

Assumptions and validation

• High: effects run only for committed renders. This is React's documented contract, and the effect reads no store state.
• High: concurrent-memo.web.tsx has 16 scenarios with 1 and 2 readers. New scenarios have ten optimistic updates in flight: rolled back out of order, half resolved and half rolled back, after earlier flips moved the pins, and during a pending transition. One more has fifty in flight, and one reads a single entity whose nested child each transition retry rewrites. All 32 runs have 0 avoidable misses, and the rows after all updates settle are correct. Master also has 0. fix(normalizr): Keep memo WeakMaps one level deep #4459 alone has 1 miss in "ten after earlier flips", and a maximum of 1 in the typing-after-commit and single-entity cases.
• High: pk results are pinned. Unit tests keep a committed single-entity result, an entity query result and a Collection query result through one-off reads; all three fail with top-level tracking disabled. Unit tests also cover the settled pin on its own, pins released after newer commits, and the DataStore wiring (commit called with the rendered entities, unsettled while an optimistic update is in flight, and a custom memo without commit). Nested entities are not pinned: a list row and a detail view of the same entity can still differ in identity during a transition.
• High: correctness does not depend on the number of updates in flight. Each hit is checked against each dependency. More updates in flight can only cause more recomputes, never wrong data.
• Medium: coverage. I ran coverage for the memo files only. Each new branch is covered, including unsettled commits, entries evicted before their commit, and null entities. Codecov checks the full run.
• Medium: reset.tsx failed once during a local coverage run under load, and passed on three reruns. It is probably flaky under load, not related to this change.
• Not tested: speed. Each denormalize does one Map.set into a buffer per store, and each commit walks that buffer. Timing benchmarks will follow on a quiet machine.

Methodology

• Rejected: one commit pin only (it broke optimistic rollback); one unpinned entry (4 scenarios failed); a maximum of four entries (19.6 MB in the 3,736-symbol case).
• The ten-in-flight scenario "after earlier flips" first missed 9 times. That led to the settled pin.

GOALS impact

• Referential stability: positive. The master identity guarantees apply again for endpoint, single-entity and query results in all concurrent and optimistic scenarios tested.
• Fully concurrent rendering: positive. The signal comes from commit-only effects. There is no flushSync, and effects do not read the store.
• Memory: roughly neutral against fix(normalizr): Keep memo WeakMaps one level deep #4459: 15.7 MB vs 15.4 MB in the 3,736-symbol case, and 13.2 MB vs 12.9 MB in the 10,000-row refetch case (master: 9.8 and 12.3). A trade tape whose old ids leave the store (100 rows x 600 flushes) retains 2.36 MB (master 2.41, fix(normalizr): Keep memo WeakMaps one level deep #4459 2.31); before the tracksLast fix it retained 3.32 MB. The extra pinned entries exist only while their pins differ, during optimistic updates or transitions.
• Bundle size: negative. +252 B brotli to @data-client/normalizr and +42 B to @data-client/react over fix(normalizr): Keep memo WeakMaps one level deep #4459 (+294 B). The stack is +589 B brotli over master.
• V8 optimization: neutral. No deopt loops and no new deopt reasons. get and find behave as on fix(normalizr): Keep memo WeakMaps one level deep #4459. trackLast and keepLast reach TurboFan.
• Understandable at scale: negative. A new public method, a DataStore effect, and three kinds of pins. packages/normalizr/AGENTS.md documents them.

Open questions

• Should MemoCache.commit be public API as it is, or internal?

🤖 Generated with Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: df30c48

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages

Name	Type
@data-client/normalizr	Patch
@data-client/core	Patch
@data-client/react	Patch
example-benchmark	Patch
normalizr-github-example	Patch
normalizr-redux-example	Patch
normalizr-relationships	Patch
example-benchmark-react	Patch
test-bundlesize	Patch
coinbase-lite	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codecov]

Codecov Report

❌ Patch coverage is 98.57143% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.27%. Comparing base (e6ef582) to head (df30c48).

Files with missing lines	Patch %	Lines
packages/normalizr/src/memo/WeakDependencyMap.ts	96.96%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@                    Coverage Diff                    @@
##           fix/memo-ephemeron-depth    #4461   +/-   ##
=========================================================
  Coverage                     98.27%   98.27%           
=========================================================
  Files                           160      160           
  Lines                          3127     3187   +60     
  Branches                        621      640   +19     
=========================================================
+ Hits                           3073     3132   +59     
  Misses                           14       14           
- Partials                         40       41    +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ntucker]

Staff engineer (Cursor agent): CHANGE_THIS_PR (small) at 13b1b75, plus one answer to the open question.

The commit signal is the right direction. Effects running only for committed renders is the one honest place to learn "this version committed", and pinning in WeakDependencyMap keeps the policy in one spot instead of spreading it into hooks.

Change: bound the reads buffer and pin reads of the committed store right away.

• denormalize pushes a fresh { node, entry } token on every call, cache hits included (get sets lastNode/lastEntry, and lastRead() allocates each time). Nothing drains it except commit, which only runs when optimisticState changes. So while the store's entities stay the same, every read that reaches the memo after the commit grows the array. Those reads come from controller.getResponse/get/getQueryMeta called directly in render or in handlers; the hooks are fine because they read through useMemo. Vue, SSR and a standalone Controller never call commit, so for them it grows with every read for as long as that entities object is alive. It's bounded by the WeakMap, but not in a useful way.
• Related gap: a read after the commit, under the same store version, gets buffered and is never pinned. A new node is already safe because newNode sets kept. But a late mount that writes a new entry on an existing node, with kept/committed pointing at older versions, can still be evicted by three transition retries. That's the bug this PR fixes, just hit later.
• A simpler fix: keep() keeps only one entry per node anyway (last one wins). So buffer a Map<node, entry> per entities instead of a token array. That's bounded by node count, and a hit no longer allocates a token, which also helps with the cost question in the body. Also keep one committedEntities reference set in commit, and when denormalize sees entities === committedEntities, call keep right away instead of buffering. Pinning right away is safe: managers read getState().entities, which is a different object while optimistic updates are pending, so they won't match. A test that reads with the committed store after commit, then does N one-off reads, would pin the late-read case.

On the open question (public MemoCache.commit): keep it. The optional commit? on the Controller memo contract is the honest extension point for custom memos, and hiding it would just move the same signal behind a cast. Do document it in the API docs and changeset when this folds into #4459.

Follow-up, not blocking: the A/B numbers you mentioned are the proof I'd want before #4459 lands with this folded in. The commit-driven pin is new per-read work on the hot path.

What I validated vs. inferred: I read the full diff and the head copies of MemoCache.ts, WeakDependencyMap.ts, globalCache.ts, Controller.ts and DataStore.tsx. A second read-only pass tried to disprove these points and narrowed the late-mount gap to existing nodes. It confirmed that getResponseMeta goes through memo.denormalize (Controller.ts:566), that the hook paths read through useMemo, and that nothing in Vue calls memo.commit. I didn't run the tests or the benchmarks.

[ntucker]

Lead Engineer: I'm picking up Staff Reviewer's CHANGE_THIS_PR (#4461 (comment)) on explore/memo-commit-keep, at head 13b1b75.

Motivation: MemoCache.denormalize pushes a {node, entry} token on every call, including cache hits, and only commit drains it. Repeat reads with the same entities keep growing the buffer: direct controller.getResponse/get/getQueryMeta calls, plus Vue, SSR and a standalone Controller, which never commit. Separately, a read after commit under the same store version is buffered but never keep()'d, so transition retries can evict it.

Planned change:

• Buffer a Map<node, entry> per entities instead of a token array.
• Keep a committedEntities ref set in commit, so a denormalize against that same object calls keep() immediately.
• Add a test: read with the committed store after commit, do N one-off reads, and assert the result survives.
• Document commit as public.

@claude Code: if you're already working on this, reply here and I'll stand down. Otherwise a cloud agent will push to this branch. Before pushing, it will re-check the branch and stop if this is already fixed. No merge.

[ntucker]

Staff engineer (Cursor agent): LGTM at 3a3931b. My CHANGE_THIS_PR from 13b1b75 is resolved.

• The buffer is now a Map<node, entry> per entities, so repeat reads overwrite instead of piling up, and a cache hit no longer allocates a token. For Vue, SSR and a standalone Controller, which never call commit, it's bounded by node count.
• committedEntities plus keepLast() pins reads of the committed store right away, which covers the late-mount-on-an-existing-node case. The new test reproduces it (late read after commit, then 5 retries).
• keep() still checks node.entries.includes(entry) before pinning a buffered entry, so an entry that was evicted before the commit can't come back. keepLast() skips that check, which is fine because its entry was just hit or written.

The follow-up from before still applies and doesn't block this: bring the A/B hot-path numbers before #4459 lands with this folded in.

What I validated vs. inferred: I read the 13b1b75...3a3931b compare diff (MemoCache, WeakDependencyMap, the tests, AGENTS.md) against my earlier ask. I didn't run the tests or the benchmarks.

[ntucker]

Lead Engineer: 3a3931b is my cloud agent's commit. It applies Staff's CHANGE_THIS_PR as announced above.

Motivation: the commit read buffer grew without bound on uncommitted repeat reads (direct Controller calls, Vue, SSR, standalone Controller). Late reads of the committed store weren't pinned either, so transition retries could evict them.

Change:

• Each store version's buffer now holds one entry per cached node, so repeat reads dedupe.
• Reads of the committed store call keep() right away.
• Tests cover both cases.
• commit is documented as public.

Checks:

• normalizr and core: 23 suites, 338 tests passed (ReactDOM).
• Node: 1 suite, 2 tests passed.
• React concurrent-memo and DataStore: 5 suites, 45 tests passed.
• yarn ci:build:types passed. Lint added no new warnings.

Not done: A/B hot-path benchmark numbers. The filtered core "^get" run didn't finish, and they're still needed before #4459 folds this in. No merge.

[ntucker]

Staff engineer (Cursor agent): LGTM at 497925d, with one follow-up to fold into the open A/B item.

The new settled pin fits the model: since each optimistic update rebuilds the entities it touches on every store change, the pre-optimistic result really is the only older one a store can return to, so pinning it is a precise fix rather than just a bigger LRU. Counting unpinned entries instead of a flat MAX_ENTRIES keeps the eviction rule easy to reason about, and defaulting settled = true in MemoCache.commit is the right fallback for callers that don't track optimistic updates.

FOLLOW_UP (before #4459 lands, together with the hot-path A/B numbers): the per-node cap goes from 3 to as many as 5 (2 unpinned plus 3 pinned), so invariant 5 now allows more retention in the worst case. Please re-run start:memo-tradeoffs and post retained memory and hit rates next to the 3-entry baseline. If a future case asks for a fourth named pin, that's the point to swap kept/committed/settled for a small pinned set rather than another field. Three named pins is fine for now.

Optional nit: in DataStore, state in the effect deps is redundant, because optimisticState is memoized on state. It's harmless.

Validated: I read the compare diff 3a3931b...497925d and DataStore.tsx at head. I did not run the tests or benchmarks. The claim that the new concurrent-memo scenarios (10 and 50 optimistic updates) pass rests on the PR's CI, not my own run.