Skip to content

useAgentChat: the clientToolResults cleanup effect dispatches setState once per streamed chunk, accumulating nestedUpdateCount to the React #185 throw #2217

Closed

Description

@Konan69

Summary

useAgentChat's clientToolResults stale-entry cleanup effect calls setClientToolResults unconditionally on every chatMessages change. Because the dispatch is not actually free, it consumes one unit of React's nested-update budget per streamed chunk, and any sufficiently long answer ends in Maximum update depth exceeded (React #185).

This is a distinct dispatch source from the ones covered by #1361 / #1732 / #1913 (per-frame setMessages, replay merging). It also explains why experimental_throttle — the guidance given on #1732 — reduced the failure rate for people without closing it.

packages/agents/src/chat/react.tsx, unchanged on main @ ec93caf6 and in the published agents@0.22.0:

useEffect(() => {
  const currentToolCallIds = new Set<string>();
  // ...collect ids from chatMessages...

  // Use functional update to check and clean stale entries atomically
  setClientToolResults((prev) => {
    if (prev.size === 0) return prev;
    // ...
    if (!hasStaleEntries) return prev;   // <-- believed to be a bailout
    // ...
  });
}, [chatMessages]);

Why returning prev is not a bailout

React's useState dispatch only takes the eager-bailout path when the fiber has no pending work. During a streamed turn the next chunk has usually already scheduled an update, so the eager path is skipped, the update is queued, and the component re-renders even though the map is identical.

That alone would only be wasteful. The failure comes from which lane it lands on:

  • This is a passive effect, and a SyncLane commit flushes passive effects inside the commit itself (0 !== (pendingEffectsLanes & 3) && flushPendingEffects()).
  • So the dispatch schedules a DefaultLane update while root.pendingLanes is still non-empty.
  • That is exactly the condition under which React increments nestedUpdateCount instead of resetting it. The reset only happens on a commit that ends with no SyncLane | InputContinuousLane | DefaultLane pending.

nestedUpdateCount is a monotonic accumulator, not a loop detector. There is no render loop here — each streamed chunk contributes exactly one count, the counter never resets while the turn is live, and at 51 React throws from getRootForUpdatedFiber. The throw therefore lands on whatever update happens to come next, which in an AI SDK v6 setup is the ReactChatState store notification inside Chat.makeRequest's try/catch — so it is swallowed and reassigned to chat error, with no error boundary, no component stack, and nothing in server logs. The server-side turn completes normally.

Measurement

Reproduced against agents@0.21.0 + @ai-sdk/react@3.0.204 + React 19.2.6, with an instrumented react-dom-client.development.js logging the owner fiber, lane, and stack in scheduleUpdateOnFiber whenever isFlushingPassiveEffects, plus every nestedUpdateCount++:

  • 353 dispatches from this effect in a single answer
  • all on lane 32 (DefaultLane), owner fiber = the transcript component
  • nestedUpdateCount climbing monotonically, remaining=32 on every increment
  • reproduced on long answers, never on short ones — consistent with an accumulator rather than a loop

Fix

Compute staleness from a ref mirror of the map before dispatching, so a turn that prunes nothing schedules no update at all. The updater still recomputes from prev, so a concurrent write between the check and the update is not clobbered.

Branch and commit on my fork, since PR creation against this repo 404s for external accounts (same as #1363, #2181, #2182):

pnpm --filter agents typecheck, oxlint, and oxfmt --check pass on the change. Happy to add a regression test in packages/agents/src/react-tests/ if you'd like one in the same shape as default-throttle.test.tsx — it needs a counted-dispatch assertion rather than a render count, so I'd rather match whatever you prefer there.

Verified downstream

Carried as a pnpm patch in our app against 0.21.0. Long streamed answers that reliably produced the banner before now complete cleanly (3/3), with no change in tool-result rendering behaviour.

One gotcha worth recording for anyone reproducing: with Vite, agents/chat/react gets inlined into the pre-bundled dep chunk, so rewriting node_modules does not invalidate it. Clear the dep cache and restart, or you will silently test the unpatched code.

Activity

  1. Konan69 commented on Sep 4, 2026

    @Konan69
    ContributorAuthor

    Added a regression test, and it turned up something about the fix that you should weigh before taking it.

    Test: packages/agents/src/react-tests/tool-result-prune-updates.test.tsx — Konan69@97039d94

    It streams a turn chunk-per-task the way a live socket delivers one, rather than replaying a whole turn in a single task like default-throttle.test.tsx. Against the unfixed effect it fails on every attempt including all 3 configured retries, with the throw surfacing through forceStoreRerender from the AI SDK's store subscriber — the same stack as production. With the fix it passes first try.

    Two details are load-bearing, and I only found them by calibration — dropping either makes the test pass against the unfixed code, i.e. silently stop being a regression test:

    1. The component has to cost something to render. With a trivial component React drains its queue between chunks, every commit ends idle, and nestedUpdateCount resets instead of accumulating. My first attempt was a trivial component and it passed against the bug.
    2. The frames have to be queued as separate tasks up front rather than awaited one at a time. Awaiting between chunks hands React a clear main thread and has the same masking effect.

    The part you should weigh

    The fix makes resume-replay-burst.test.tsx > "delivers live chunks after a replayed burst in order" fail. Controlled comparison, same machine, same command, only react.tsx differing:

    react.tsx result
    unfixed 6 passed
    fixed 1 failed (all 4 attempts), 5 passed

    The failing assertion is the one at line 306, after await sleep(20):

    // Replayed prefix is applied at the boundary, before any live chunk.
    expect(h.read("chars")).toBe(String(expectedChars(80)));   // got "0"

    No content is lost. Every other assertion in that file sits behind await sleep(60) and all of them still pass with the fix — including the ones that check the full replayed prefix. It is only the single sleep(20) assertion that fails.

    My reading: the unconditional setClientToolResults dispatch was accidentally load-bearing. It forced an extra render immediately after the replay boundary, which flushed the batched messagesUpdate to the DOM sooner than it would otherwise commit. Removing it means the replayed prefix paints somewhere between 20ms and 60ms instead of under 20ms.

    So the choice is yours, and I did not want to make it by quietly editing your regression test:

    • If that assertion means "the replayed prefix is applied at the boundary, before any live chunk" — a sequencing claim — then vi.waitFor is the right shape and the fixed timeout was incidental.
    • If it means "the replayed prefix must be on screen within 20ms" — a latency claim — then this fix regresses it and wants a deliberate flush at replayComplete instead. That flush must not go through a passive-effect setState, or it re-enters the accumulation problem this issue is about.

    Happy to push whichever you prefer. Everything else in the react project passes with the fix (136/137 on the run where this was the only failure).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions