You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
createAppShellSessionUiStateController is an external store with per-session, per-slice maps, but useAppShellSessionUiState exposes it through a single forceRender(). Every write to any slice — including one per streaming token — re-renders all of AppShellContent, including subtrees that cannot have changed.
Split out of #1979, which fixed one instance (idle event-health polling). This issue is the shared root cause: the store's granularity and the subscription's granularity don't match.
Line numbers below are against 75e085354, i.e. after #1988 merged.
Evidence
app-shell-session-ui-state.ts:112-118,161-172 — replaceState calls a single onChange, wired to one useReducer bump. No selector, no per-slice subscription.
app-shell-session-events.ts:65-74 — setLiveTurnBySession is written per streaming delta. Each write is a fresh map → passes the reference check → forceRender().
app-shell-effects.ts:478-484 — shell-run updates arrive on their own high-rate IPC channel and write the second rendered map at tool-output rate.
use-app-shell-session-workspace.ts:70-82 is called from AppShellContent (app-shell.tsx:277), whose return renders the entire shell (2663-line file).
app-shell.tsx:312-321 destructures the whole store at the top of the component; :549 reads the full active projection; :606-625 feeds it to useShellLiveTurn.
app-shell.tsx:2135 — SessionListPanel takes ~25 props including inline lambdas (onWidthChange, onSelectSession), so it re-renders on every shell render and would keep doing so even if wrapped in memo.
Net effect during a stream: the session list, composer, and any open non-chat surface re-render at token rate, though none of them read liveTurnBySession.
What the first framing got wrong
Three corrections, each of which changes the fix:
subscribe + useSyncExternalStore + selector is necessary but not sufficient. The component holding the selector isAppShellContent (app-shell.tsx:312-321,549,606-625). As long as it selects the full projection, it re-renders on every delta and the whole subtree goes with it — zero renders saved. The subscription fix only pays off once the reader boundary moves.
The cheap form of "moving the boundary" is not to relocate every read: the top level can keep subscribing, as long as it selects a low-entropy semantic snapshot (phase, is-streaming, settled messageId, has-in-flight-tool). Those values are invariant across consecutive text deltas. Only the chat transcript genuinely needs the full projection, and only that read has to sink.
JSX reads are not the only consumers.app-shell.tsx:1787-1790 is an effect whose dep array contains activeLiveTurn, so it runs per token; the terminal-settle fallback below it, the health polling, and homeSurfaceActive (app-shell.tsx:2034) also derive from it. Any sinking plan must give these a home — an effect-only return null subscriber can run per delta without owning a subtree.
Second shallow-compare killer.use-shell-live-turn.ts:57-62 builds streamingSessionIds as a fresh Set on every liveTurnBySession change and passes it to SessionListPanel. It is not a trigger today — it is a by-product of the parent render — but it defeats any future shallow comparison at that boundary. Note its membership is not turn-invariant: the predicate is text?.text && !text.complete, so a session drops out between a settled step and the next text step. It needs a value-equality-cached selector, not just a useMemo.
Also worth downgrading: session rows are already memoized (packages/ui/src/session-history-list.tsx:332,622, locked by ui-render-memo-boundary-contract.test.ts:50), so the waste does not scale into per-row internals. It is AppShellContent itself, the panel/list traversal, and the composer.
Non-fixes
rAF / microtask coalescing in the store. React 19 + createRoot already batches the multiple forceRenders inside one IPC listener, so microtask coalescing is redundant and cannot merge across separate IPC tasks. rAF caps whole-tree renders at min(R, F) — near-zero benefit at typical token rates below 60 Hz, at the cost of a frame of latency and background-window throttling. It also does not meet the acceptance bar. Do not put a scheduler in the external-store notify path: it would delay the terminal handoff.
Blanket memo. The inline-lambda props defeat it, and fixing that means stabilizing ~25 props per panel. Once AppShellContent stops re-rendering, the children are never re-invoked and no memo is needed.
Fix direction
Extend the existing seam. The controller is already an external store; it only lacks per-subscriber notification.
Add a synchronous subscribe(listener), notified immediately after the state swap.
Drop the aggregate useReducer subscription. The top level subscribes to the low-frequency maps' raw references plus the live-turn semantic snapshot — so consecutive deltas change nothing it reads.
Sink the full liveTurnBySession[activeId] and the active shell-run record into the ChatMessageSurface desktop wrapper.
Move the per-delta reconcile / terminal-handoff effects into an effect-only subscriber that renders nothing.
Sidebar gets a value-equality-cached membership selector; the composer consumes only the semantic snapshot, so it re-renders at turn boundaries, not at token rate.
Correctness constraints
getSnapshot must not build a fresh Set/array/object per call. React compares with Object.is and requires a stable snapshot when nothing changed, or it throws "getSnapshot should be cached" / loops. Return the raw per-session record and do the Object.values inside the component.
activeId goes into the selector directly — the first render after a switch must read the new session's snapshot, not copy it in via an Effect.
Preserve the synchronous activeIdRef write order (use-app-shell-session-workspace.ts:23-35) and the layout-effect cleanup of the previous session's subscription (app-shell-effects.ts:414).
useSettledSessionTransientReconcile (app-shell-effects.ts:604) still has to clear settled background projections; sinking the read must not delete it.
Severity
P1. The waste scales with token rate and lands on the app's hottest path — the whole shell re-renders for every delta of a stream that only the chat surface reads. Perceived responsiveness during streaming is a primary quality bar, so this is not deferrable polish.
Acceptance
A streaming session re-renders the chat surface but not SessionListPanel / Composer.
Render-count contract test, using the existing client-commit harness (createRoot + act + fake DOM) at apps/desktop/src/main/__tests__/ui-render-memo-boundary-contract.test.ts:255 — no new test framework, and not a controller-only notification count, which would not pin the component boundary. Cover: a second text_delta bumps the chat render count and nothing else; activeId switch; terminal complete.
Summary
createAppShellSessionUiStateControlleris an external store with per-session, per-slice maps, butuseAppShellSessionUiStateexposes it through a singleforceRender(). Every write to any slice — including one per streaming token — re-renders all ofAppShellContent, including subtrees that cannot have changed.Split out of #1979, which fixed one instance (idle event-health polling). This issue is the shared root cause: the store's granularity and the subscription's granularity don't match.
Line numbers below are against
75e085354, i.e. after #1988 merged.Evidence
app-shell-session-ui-state.ts:112-118,161-172—replaceStatecalls a singleonChange, wired to oneuseReducerbump. No selector, no per-slice subscription.app-shell-session-events.ts:65-74—setLiveTurnBySessionis written per streaming delta. Each write is a fresh map → passes the reference check →forceRender().app-shell-effects.ts:478-484— shell-run updates arrive on their own high-rate IPC channel and write the second rendered map at tool-output rate.app-shell-effects.ts:384-392still writes event-health on every session event, but perf(desktop): stop idle-session event-health polling from re-rendering the whole shell #1988 moved that map into a controller-owned ref (app-shell-session-ui-state.ts:106-110,144), so it no longer notifies React. That is why this issue now has two token-rate rendered slices rather than three — worth stating explicitly, because it is the reason a measurement taken before perf(desktop): stop idle-session event-health polling from re-rendering the whole shell #1988 does not reproduce.use-app-shell-session-workspace.ts:70-82is called fromAppShellContent(app-shell.tsx:277), whose return renders the entire shell (2663-line file).app-shell.tsx:312-321destructures the whole store at the top of the component;:549reads the full active projection;:606-625feeds it touseShellLiveTurn.app-shell.tsx:2135—SessionListPaneltakes ~25 props including inline lambdas (onWidthChange,onSelectSession), so it re-renders on every shell render and would keep doing so even if wrapped inmemo.Net effect during a stream: the session list, composer, and any open non-chat surface re-render at token rate, though none of them read
liveTurnBySession.What the first framing got wrong
Three corrections, each of which changes the fix:
subscribe+useSyncExternalStore+ selector is necessary but not sufficient. The component holding the selector isAppShellContent(app-shell.tsx:312-321,549,606-625). As long as it selects the full projection, it re-renders on every delta and the whole subtree goes with it — zero renders saved. The subscription fix only pays off once the reader boundary moves.The cheap form of "moving the boundary" is not to relocate every read: the top level can keep subscribing, as long as it selects a low-entropy semantic snapshot (
phase, is-streaming, settledmessageId, has-in-flight-tool). Those values are invariant across consecutive text deltas. Only the chat transcript genuinely needs the full projection, and only that read has to sink.JSX reads are not the only consumers.
app-shell.tsx:1787-1790is an effect whose dep array containsactiveLiveTurn, so it runs per token; the terminal-settle fallback below it, the health polling, andhomeSurfaceActive(app-shell.tsx:2034) also derive from it. Any sinking plan must give these a home — an effect-onlyreturn nullsubscriber can run per delta without owning a subtree.Second shallow-compare killer.
use-shell-live-turn.ts:57-62buildsstreamingSessionIdsas a freshSeton everyliveTurnBySessionchange and passes it toSessionListPanel. It is not a trigger today — it is a by-product of the parent render — but it defeats any future shallow comparison at that boundary. Note its membership is not turn-invariant: the predicate istext?.text && !text.complete, so a session drops out between a settled step and the next text step. It needs a value-equality-cached selector, not just auseMemo.Also worth downgrading: session rows are already memoized (
packages/ui/src/session-history-list.tsx:332,622, locked byui-render-memo-boundary-contract.test.ts:50), so the waste does not scale into per-row internals. It isAppShellContentitself, the panel/list traversal, and the composer.Non-fixes
createRootalready batches the multipleforceRenders inside one IPC listener, so microtask coalescing is redundant and cannot merge across separate IPC tasks. rAF caps whole-tree renders atmin(R, F)— near-zero benefit at typical token rates below 60 Hz, at the cost of a frame of latency and background-window throttling. It also does not meet the acceptance bar. Do not put a scheduler in the external-store notify path: it would delay the terminal handoff.memo. The inline-lambda props defeat it, and fixing that means stabilizing ~25 props per panel. OnceAppShellContentstops re-rendering, the children are never re-invoked and nomemois needed.Fix direction
Extend the existing seam. The controller is already an external store; it only lacks per-subscriber notification.
subscribe(listener), notified immediately after the state swap.useReducersubscription. The top level subscribes to the low-frequency maps' raw references plus the live-turn semantic snapshot — so consecutive deltas change nothing it reads.liveTurnBySession[activeId]and the active shell-run record into theChatMessageSurfacedesktop wrapper.Correctness constraints
getSnapshotmust not build a freshSet/array/object per call. React compares withObject.isand requires a stable snapshot when nothing changed, or it throws "getSnapshot should be cached" / loops. Return the raw per-session record and do theObject.valuesinside the component.activeIdgoes into the selector directly — the first render after a switch must read the new session's snapshot, not copy it in via an Effect.activeIdRefwrite order (use-app-shell-session-workspace.ts:23-35) and the layout-effect cleanup of the previous session's subscription (app-shell-effects.ts:414).useSettledSessionTransientReconcile(app-shell-effects.ts:604) still has to clear settled background projections; sinking the read must not delete it.Severity
P1. The waste scales with token rate and lands on the app's hottest path — the whole shell re-renders for every delta of a stream that only the chat surface reads. Perceived responsiveness during streaming is a primary quality bar, so this is not deferrable polish.
Acceptance
SessionListPanel/Composer.createRoot+act+ fake DOM) atapps/desktop/src/main/__tests__/ui-render-memo-boundary-contract.test.ts:255— no new test framework, and not a controller-only notification count, which would not pin the component boundary. Cover: a secondtext_deltabumps the chat render count and nothing else;activeIdswitch; terminal complete.Related