From bf397a9a753fad651ee3a488f2eb16ee9473287c Mon Sep 17 00:00:00 2001 From: Adam Leith Date: Wed, 3 Jun 2026 14:32:27 +0100 Subject: [PATCH 1/6] perf(code): throttle conversation rebuilds to one per frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Long threads lagged badly while streaming. ConversationView re-parsed the entire event history via buildConversationItems on every appended token — O(n) per token, O(n^2) per turn — and reconciled all-new objects each time. Coalesce the events array to at most one rebuild per animation frame with a new useFrameThrottledValue hook. Token bursts collapse into a single rebuild instead of dozens, and the value always settles on the exact latest state within a frame so end-of-stream output is never stale. Build/render semantics are otherwise unchanged — only the rebuild rate is capped. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../sessions/components/ConversationView.tsx | 13 ++- .../hooks/useFrameThrottledValue.test.ts | 103 ++++++++++++++++++ .../renderer/hooks/useFrameThrottledValue.ts | 41 +++++++ 3 files changed, 154 insertions(+), 3 deletions(-) create mode 100644 apps/code/src/renderer/hooks/useFrameThrottledValue.test.ts create mode 100644 apps/code/src/renderer/hooks/useFrameThrottledValue.ts diff --git a/apps/code/src/renderer/features/sessions/components/ConversationView.tsx b/apps/code/src/renderer/features/sessions/components/ConversationView.tsx index 85f72405d..e1c13f92f 100644 --- a/apps/code/src/renderer/features/sessions/components/ConversationView.tsx +++ b/apps/code/src/renderer/features/sessions/components/ConversationView.tsx @@ -11,6 +11,7 @@ import { } from "@features/sessions/stores/sessionStore"; import { useSettingsStore } from "@features/settings/stores/settingsStore"; import { SkillButtonActionMessage } from "@features/skill-buttons/components/SkillButtonActionMessage"; +import { useFrameThrottledValue } from "@hooks/useFrameThrottledValue"; import { ArrowDown, XCircle } from "@phosphor-icons/react"; import { WorkerPoolContextProvider } from "@pierre/diffs/react"; import WorkerUrl from "@pierre/diffs/worker/worker.js?worker&url"; @@ -82,7 +83,13 @@ export function ConversationView({ const [showScrollButton, setShowScrollButton] = useState(false); const debugLogsCloudRuns = useSettingsStore((s) => s.debugLogsCloudRuns); const showDebugLogs = debugLogsCloudRuns; - const contextUsage = useContextUsage(events); + + // Streaming appends one event per token, so `events` changes far faster than + // the screen can paint. Coalesce to one rebuild per frame — each rebuild + // re-parses the whole history, so capping the rate is what keeps long threads + // responsive while streaming. Settles on the exact final state within a frame. + const throttledEvents = useFrameThrottledValue(events); + const contextUsage = useContextUsage(throttledEvents); const { items: conversationItems, @@ -90,10 +97,10 @@ export function ConversationView({ isCompacting, } = useMemo( () => - buildConversationItems(events, isPromptPending, { + buildConversationItems(throttledEvents, isPromptPending, { showDebugLogs, }), - [events, isPromptPending, showDebugLogs], + [throttledEvents, isPromptPending, showDebugLogs], ); const firstUserMessageIdRef = useRef(undefined); diff --git a/apps/code/src/renderer/hooks/useFrameThrottledValue.test.ts b/apps/code/src/renderer/hooks/useFrameThrottledValue.test.ts new file mode 100644 index 000000000..0e8afcb82 --- /dev/null +++ b/apps/code/src/renderer/hooks/useFrameThrottledValue.test.ts @@ -0,0 +1,103 @@ +import { act, renderHook } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { useFrameThrottledValue } from "./useFrameThrottledValue"; + +describe("useFrameThrottledValue", () => { + let queue: Array<() => void>; + + beforeEach(() => { + queue = []; + vi.stubGlobal("requestAnimationFrame", (cb: FrameRequestCallback) => { + queue.push(() => cb(0)); + return queue.length; + }); + vi.stubGlobal("cancelAnimationFrame", (id: number) => { + queue[id - 1] = () => {}; + }); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + function flushFrame() { + const pending = queue; + queue = []; + act(() => { + for (const fn of pending) fn(); + }); + } + + it("returns the initial value synchronously", () => { + const { result } = renderHook(() => useFrameThrottledValue("initial")); + expect(result.current).toBe("initial"); + }); + + it("settles on the latest value after a frame", () => { + const { result, rerender } = renderHook( + ({ value }) => useFrameThrottledValue(value), + { initialProps: { value: "a" } }, + ); + + rerender({ value: "b" }); + expect(result.current).toBe("a"); + + flushFrame(); + expect(result.current).toBe("b"); + }); + + it("coalesces a burst of changes within one frame into a single update", () => { + const { result, rerender } = renderHook( + ({ value }) => useFrameThrottledValue(value), + { initialProps: { value: 0 } }, + ); + + // Many changes before the frame fires — only one rAF should be queued. + for (let i = 1; i <= 10; i++) rerender({ value: i }); + expect(queue.length).toBe(1); + expect(result.current).toBe(0); + + flushFrame(); + // Lands on the freshest value, skipping every intermediate one. + expect(result.current).toBe(10); + }); + + it("keeps streaming across multiple frames", () => { + const { result, rerender } = renderHook( + ({ value }) => useFrameThrottledValue(value), + { initialProps: { value: "t0" } }, + ); + + rerender({ value: "t1" }); + flushFrame(); + expect(result.current).toBe("t1"); + + rerender({ value: "t2" }); + flushFrame(); + expect(result.current).toBe("t2"); + }); + + it("does not schedule a frame when the value is unchanged", () => { + const { rerender } = renderHook( + ({ value }) => useFrameThrottledValue(value), + { initialProps: { value: "a" } }, + ); + flushFrame(); + queue = []; + + rerender({ value: "a" }); + expect(queue.length).toBe(0); + }); + + it("cancels a pending frame on unmount", () => { + const { result, rerender, unmount } = renderHook( + ({ value }) => useFrameThrottledValue(value), + { initialProps: { value: "a" } }, + ); + + rerender({ value: "b" }); + unmount(); + flushFrame(); + expect(result.current).toBe("a"); + }); +}); diff --git a/apps/code/src/renderer/hooks/useFrameThrottledValue.ts b/apps/code/src/renderer/hooks/useFrameThrottledValue.ts new file mode 100644 index 000000000..49e3437fe --- /dev/null +++ b/apps/code/src/renderer/hooks/useFrameThrottledValue.ts @@ -0,0 +1,41 @@ +import { useEffect, useRef, useState } from "react"; + +/** + * Coalesces rapid changes to `value` so dependents recompute at most once per + * animation frame. + * + * During token streaming the upstream events array changes on every chunk; + * without this each chunk drives a full O(n) conversation rebuild + reconcile, + * so a long thread does far more rebuilds per second than it can paint. This + * caps rebuilds to the display rate while always settling on the latest value + * within one frame of the final change — so the end-of-stream state is exact, + * never stale. + */ +export function useFrameThrottledValue(value: T): T { + const [throttled, setThrottled] = useState(value); + const latestRef = useRef(value); + const rafRef = useRef(null); + + useEffect(() => { + latestRef.current = value; + // A frame is already queued; it will read latestRef when it fires, so we + // don't schedule a second one. This is what collapses a burst of changes + // within the same frame into a single recompute. + if (rafRef.current !== null) return; + rafRef.current = requestAnimationFrame(() => { + rafRef.current = null; + setThrottled((prev) => + prev === latestRef.current ? prev : latestRef.current, + ); + }); + }, [value]); + + useEffect( + () => () => { + if (rafRef.current !== null) cancelAnimationFrame(rafRef.current); + }, + [], + ); + + return throttled; +} From ae16276c3d92344754fd8c74f2ba1a489b7e85cc Mon Sep 17 00:00:00 2001 From: Adam Leith Date: Wed, 3 Jun 2026 16:27:00 +0100 Subject: [PATCH 2/6] perf(code): parse conversation items incrementally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Throttling capped how often the conversation rebuilt, but each rebuild still re-parsed the entire event history — O(n) per frame, scaling with thread length. Long threads stayed heavy while streaming. Process each event exactly once into a persistent builder. Completed turns are reused by reference (so their memoized rows skip re-render) and only the active turn is re-derived per frame, dropping per-frame cost from O(thread) to O(active turn). buildConversationItems is refactored into a reusable processEvent/finalize/readLastTurnInfo split; the public function is unchanged. The incremental builder falls back to a full rebuild when the append-only fast path can't faithfully represent the state (idle, non-append event change, options change, or a progress card in an already-frozen turn being mutated). An equivalence test asserts output matches buildConversationItems at every prefix across local/cloud/tool/progress/shell scenarios, plus reference stability for completed turns and liveness for the active turn. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../sessions/components/ConversationView.tsx | 23 +- .../components/buildConversationItems.ts | 85 ++++- .../incrementalConversationItems.test.ts | 322 ++++++++++++++++++ .../incrementalConversationItems.ts | 162 +++++++++ .../sessions/hooks/useConversationItems.ts | 57 ++++ 5 files changed, 616 insertions(+), 33 deletions(-) create mode 100644 apps/code/src/renderer/features/sessions/components/incrementalConversationItems.test.ts create mode 100644 apps/code/src/renderer/features/sessions/components/incrementalConversationItems.ts create mode 100644 apps/code/src/renderer/features/sessions/hooks/useConversationItems.ts diff --git a/apps/code/src/renderer/features/sessions/components/ConversationView.tsx b/apps/code/src/renderer/features/sessions/components/ConversationView.tsx index e1c13f92f..f8ae75c7e 100644 --- a/apps/code/src/renderer/features/sessions/components/ConversationView.tsx +++ b/apps/code/src/renderer/features/sessions/components/ConversationView.tsx @@ -1,5 +1,6 @@ import { CHAT_CONTENT_MAX_WIDTH } from "@features/sessions/constants"; import { useContextUsage } from "@features/sessions/hooks/useContextUsage"; +import { useConversationItems } from "@features/sessions/hooks/useConversationItems"; import { useConversationSearch } from "@features/sessions/hooks/useConversationSearch"; import { SessionTaskIdProvider } from "@features/sessions/hooks/useSessionTaskId"; import { @@ -25,11 +26,7 @@ import { Box, Flex, Text } from "@radix-ui/themes"; import type { Task } from "@shared/types"; import type { AcpMessage } from "@shared/types/session-events"; import { memo, useCallback, useEffect, useMemo, useRef, useState } from "react"; -import { - buildConversationItems, - type ConversationItem, - type TurnContext, -} from "./buildConversationItems"; +import type { ConversationItem, TurnContext } from "./buildConversationItems"; import { ConversationSearchBar } from "./ConversationSearchBar"; import { GitActionMessage } from "./GitActionMessage"; import { GitActionResult } from "./GitActionResult"; @@ -85,9 +82,9 @@ export function ConversationView({ const showDebugLogs = debugLogsCloudRuns; // Streaming appends one event per token, so `events` changes far faster than - // the screen can paint. Coalesce to one rebuild per frame — each rebuild - // re-parses the whole history, so capping the rate is what keeps long threads - // responsive while streaming. Settles on the exact final state within a frame. + // the screen can paint. Coalesce to one parse per frame; the parse itself is + // incremental (completed turns reused by reference) so per-frame cost tracks + // the active turn, not the whole thread. Settles on the exact final state. const throttledEvents = useFrameThrottledValue(events); const contextUsage = useContextUsage(throttledEvents); @@ -95,13 +92,9 @@ export function ConversationView({ items: conversationItems, lastTurnInfo, isCompacting, - } = useMemo( - () => - buildConversationItems(throttledEvents, isPromptPending, { - showDebugLogs, - }), - [throttledEvents, isPromptPending, showDebugLogs], - ); + } = useConversationItems(throttledEvents, isPromptPending, { + showDebugLogs, + }); const firstUserMessageIdRef = useRef(undefined); if (firstUserMessageIdRef.current === undefined) { diff --git a/apps/code/src/renderer/features/sessions/components/buildConversationItems.ts b/apps/code/src/renderer/features/sessions/components/buildConversationItems.ts index fbd0d1ee4..9501c2bc8 100644 --- a/apps/code/src/renderer/features/sessions/components/buildConversationItems.ts +++ b/apps/code/src/renderer/features/sessions/components/buildConversationItems.ts @@ -79,6 +79,8 @@ interface ProgressCardState { steps: Step[]; isActive: boolean; }; + /** Index in `items` where this card sits. */ + itemIndex: number; } interface TurnState { @@ -94,9 +96,13 @@ interface TurnState { itemCount: number; } -interface ItemBuilder { +export interface ItemBuilder { items: ConversationItem[]; currentTurn: TurnState | null; + /** Index in `items` where the current turn's first item sits. Lets an + * incremental consumer treat everything before it (completed turns) as + * frozen and only re-derive the active turn. */ + currentTurnStartIndex: number; pendingPrompts: Map; shellExecutes: Map; isCompacting: boolean; @@ -106,18 +112,25 @@ interface ItemBuilder { * event for the same id mutates the same card, regardless of which turn is * currently active. */ progressCards: Map; + /** Lowest item index touched by a progress event since it was last reset. + * An incremental consumer resets this before feeding a batch of events and + * reads it after to detect a card being mutated inside an already frozen + * (completed) turn, which would otherwise go unseen. */ + lowestTouchedProgressIndex: number; } -function createItemBuilder(): ItemBuilder { +export function createItemBuilder(): ItemBuilder { let idCounter = 0; return { items: [], currentTurn: null, + currentTurnStartIndex: 0, pendingPrompts: new Map(), shellExecutes: new Map(), isCompacting: false, nextId: () => idCounter++, progressCards: new Map(), + lowestTouchedProgressIndex: Number.POSITIVE_INFINITY, }; } @@ -130,7 +143,7 @@ function isThoughtItem( ); } -function markThoughtCompletion(items: ConversationItem[]) { +export function markThoughtCompletion(items: ConversationItem[]) { const seenContexts = new Set(); for (let i = items.length - 1; i >= 0; i--) { @@ -172,23 +185,53 @@ export function buildConversationItems( const b = createItemBuilder(); for (const event of events) { - const msg = event.message; + processEvent(b, event, options); + } - if (isJsonRpcNotification(msg)) { - handleNotification(b, msg, event.ts, options); - continue; - } + finalizeBuilder(b, isPromptPending); - if (isJsonRpcRequest(msg) && msg.method === "session/prompt") { - handlePromptRequest(b, msg, event.ts); - continue; - } + const lastTurnInfo = readLastTurnInfo(b); - if (isJsonRpcResponse(msg) && b.pendingPrompts.has(msg.id)) { - handlePromptResponse(b, msg, event.ts); - } + return { items: b.items, lastTurnInfo, isCompacting: b.isCompacting }; +} + +/** + * Apply one raw event to the builder. This is the append-only core: it never + * runs end-of-stream finalization, so it is safe to call incrementally as new + * events arrive without corrupting prior state. + */ +export function processEvent( + b: ItemBuilder, + event: AcpMessage, + options?: BuildConversationOptions, +) { + const msg = event.message; + + if (isJsonRpcNotification(msg)) { + handleNotification(b, msg, event.ts, options); + return; } + if (isJsonRpcRequest(msg) && msg.method === "session/prompt") { + handlePromptRequest(b, msg, event.ts); + return; + } + + if (isJsonRpcResponse(msg) && b.pendingPrompts.has(msg.id)) { + handlePromptResponse(b, msg, event.ts); + } +} + +/** + * End-of-stream finalization: speculative completions that assume no further + * events arrive. Mutates the builder in place, so an incremental consumer must + * only apply it to a snapshot it is about to read, never to state it will keep + * feeding events into. + */ +export function finalizeBuilder( + b: ItemBuilder, + isPromptPending: boolean | null, +) { // Only mark unresolved prompts as cancelled when we actively track prompt // state (local sessions). For cloud sessions isPromptPending is // null, meaning that the response hasn't streamed "in" yet @@ -207,16 +250,16 @@ export function buildConversationItems( } markThoughtCompletion(b.items); +} - const lastTurnInfo: LastTurnInfo | null = b.currentTurn +export function readLastTurnInfo(b: ItemBuilder): LastTurnInfo | null { + return b.currentTurn ? { isComplete: b.currentTurn.isComplete, durationMs: b.currentTurn.durationMs, stopReason: b.currentTurn.stopReason, } : null; - - return { items: b.items, lastTurnInfo, isCompacting: b.isCompacting }; } function handlePromptRequest( @@ -250,6 +293,7 @@ function handlePromptRequest( turnComplete: false, }; + b.currentTurnStartIndex = b.items.length; b.currentTurn = { id: turnId, promptId: msg.id, @@ -460,6 +504,7 @@ function ensureProgressCardForGroup( const card: ProgressCardState = { steps: new Map(), renderItem, + itemIndex: b.items.length, }; b.progressCards.set(group, card); pushItem(b, renderItem); @@ -487,6 +532,9 @@ function handleProgress(b: ItemBuilder, rawParams: unknown, ts: number) { const status = normalizeStepStatus(params.status); const card = ensureProgressCardForGroup(b, params.group, ts); if (!card) return; + if (card.itemIndex < b.lowestTouchedProgressIndex) { + b.lowestTouchedProgressIndex = card.itemIndex; + } card.steps.set(params.step, { key: params.step, status, @@ -525,6 +573,7 @@ function markCompactingStatusComplete(b: ItemBuilder) { function ensureImplicitTurn(b: ItemBuilder, ts: number) { if (b.currentTurn) return; + b.currentTurnStartIndex = b.items.length; const turnId = `turn-${ts}-implicit`; const toolCalls = new Map(); const childItems = new Map(); diff --git a/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.test.ts b/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.test.ts new file mode 100644 index 000000000..0ce85c335 --- /dev/null +++ b/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.test.ts @@ -0,0 +1,322 @@ +import type { AcpMessage } from "@shared/types/session-events"; +import { describe, expect, it } from "vitest"; +import { + type BuildResult, + buildConversationItems, + type ConversationItem, + type TurnContext, +} from "./buildConversationItems"; +import { createIncrementalConversationBuilder } from "./incrementalConversationItems"; + +// --- event builders ------------------------------------------------------- + +function updateMsg(ts: number, update: unknown): AcpMessage { + return { + type: "acp_message", + ts, + message: { jsonrpc: "2.0", method: "session/update", params: { update } }, + }; +} + +function userPromptMsg(ts: number, id: number, text: string): AcpMessage { + return { + type: "acp_message", + ts, + message: { + jsonrpc: "2.0", + id, + method: "session/prompt", + params: { prompt: [{ type: "text", text }] }, + }, + }; +} + +function promptResponseMsg( + ts: number, + id: number, + stopReason = "end_turn", +): AcpMessage { + return { + type: "acp_message", + ts, + message: { jsonrpc: "2.0", id, result: { stopReason } }, + }; +} + +function turnCompleteMsg(ts: number, stopReason = "end_turn"): AcpMessage { + return { + type: "acp_message", + ts, + message: { + jsonrpc: "2.0", + method: "_posthog/turn_complete", + params: { sessionId: "s", stopReason }, + }, + }; +} + +function consoleMsg(ts: number, message: string, level = "info"): AcpMessage { + return { + type: "acp_message", + ts, + message: { + jsonrpc: "2.0", + method: "_posthog/console", + params: { level, message }, + }, + }; +} + +function progressMsg( + ts: number, + step: string, + status: string, + label: string, + group = "setup", +): AcpMessage { + return { + type: "acp_message", + ts, + message: { + jsonrpc: "2.0", + method: "_posthog/progress", + params: { step, status, label, group }, + }, + }; +} + +function shellExecuteMsg(ts: number, id: string, command: string): AcpMessage { + return { + type: "acp_message", + ts, + message: { + jsonrpc: "2.0", + method: "_array/user_shell_execute", + params: { id, command, cwd: "/repo" }, + }, + }; +} + +const agentChunk = (ts: number, text: string) => + updateMsg(ts, { + sessionUpdate: "agent_message_chunk", + content: { type: "text", text }, + }); + +const thoughtChunk = (ts: number, text: string) => + updateMsg(ts, { + sessionUpdate: "agent_thought_chunk", + content: { type: "text", text }, + }); + +const toolCallMsg = ( + ts: number, + toolCallId: string, + extra: Record = {}, +) => + updateMsg(ts, { + sessionUpdate: "tool_call", + toolCallId, + kind: "execute", + status: "pending", + title: toolCallId, + ...extra, + }); + +const toolUpdateMsg = ( + ts: number, + toolCallId: string, + extra: Record, +) => updateMsg(ts, { sessionUpdate: "tool_call_update", toolCallId, ...extra }); + +const childToolCallMsg = ( + ts: number, + toolCallId: string, + parentToolCallId: string, +) => + updateMsg(ts, { + sessionUpdate: "tool_call", + toolCallId, + kind: "read", + status: "pending", + title: toolCallId, + _meta: { claudeCode: { parentToolCallId } }, + }); + +// --- normalization (cycle-free, Map-resolved) ----------------------------- + +function normContext(ctx: TurnContext) { + return { + turnComplete: ctx.turnComplete, + turnCancelled: ctx.turnCancelled, + toolCalls: [...ctx.toolCalls.entries()].sort(byKey), + childItems: [...ctx.childItems.entries()] + .sort(byKey) + .map(([k, arr]) => [k, arr.map(normChild)]), + }; +} + +function byKey(a: [string, unknown], b: [string, unknown]) { + return a[0] < b[0] ? -1 : a[0] > b[0] ? 1 : 0; +} + +function normChild(item: ConversationItem) { + if (item.type === "session_update") { + const { turnContext: _drop, ...rest } = item; + return rest; + } + return item; +} + +function normItem(item: ConversationItem) { + if (item.type === "session_update") { + const { turnContext, ...rest } = item; + return { ...rest, turnContext: normContext(turnContext) }; + } + return item; +} + +function normalize(result: BuildResult) { + return { + items: result.items.map(normItem), + lastTurnInfo: result.lastTurnInfo, + isCompacting: result.isCompacting, + }; +} + +function assertEquivalentAcrossPrefixes( + events: AcpMessage[], + isPromptPending: boolean | null, +) { + const inc = createIncrementalConversationBuilder(); + for (let k = 1; k <= events.length; k++) { + const prefix = events.slice(0, k); + const incremental = inc.update(prefix, isPromptPending); + const full = buildConversationItems(prefix, isPromptPending); + expect(normalize(incremental)).toEqual(normalize(full)); + } +} + +// --- scenarios ------------------------------------------------------------ + +const SCENARIOS: Record = { + "single local turn": [ + userPromptMsg(1, 1, "hello"), + agentChunk(2, "hi "), + agentChunk(3, "there"), + promptResponseMsg(4, 1), + ], + "multi-turn with tools": [ + userPromptMsg(1, 1, "do a thing"), + thoughtChunk(2, "thinking..."), + agentChunk(3, "working "), + toolCallMsg(4, "t1"), + toolUpdateMsg(5, "t1", { + status: "completed", + content: [{ type: "content", content: { type: "text", text: "ok" } }], + }), + agentChunk(6, "done"), + promptResponseMsg(7, 1), + userPromptMsg(8, 2, "another"), + agentChunk(9, "second turn"), + promptResponseMsg(10, 2), + ], + "implicit cloud turn": [ + agentChunk(1, "streaming "), + agentChunk(2, "without "), + agentChunk(3, "a prompt"), + turnCompleteMsg(4), + ], + "parent tool with children": [ + userPromptMsg(1, 1, "spawn agent"), + toolCallMsg(2, "task1", { _meta: { claudeCode: { toolName: "Task" } } }), + childToolCallMsg(3, "child1", "task1"), + childToolCallMsg(4, "child2", "task1"), + toolUpdateMsg(5, "task1", { status: "completed" }), + promptResponseMsg(6, 1), + ], + "progress single group": [ + userPromptMsg(1, 1, "setup"), + progressMsg(2, "a", "in_progress", "Step A"), + progressMsg(3, "b", "in_progress", "Step B"), + progressMsg(4, "a", "completed", "Step A"), + progressMsg(5, "b", "completed", "Step B"), + agentChunk(6, "ready"), + promptResponseMsg(7, 1), + ], + "progress card updated across turn boundary": [ + userPromptMsg(1, 1, "first"), + progressMsg(2, "a", "in_progress", "Step A", "g1"), + promptResponseMsg(3, 1), + userPromptMsg(4, 2, "second"), + // late completion for g1 reaches back into the (now frozen) first turn + progressMsg(5, "a", "completed", "Step A", "g1"), + agentChunk(6, "ok"), + promptResponseMsg(7, 2), + ], + "console and shell execute": [ + consoleMsg(1, "boot"), + shellExecuteMsg(2, "sh1", "ls"), + userPromptMsg(3, 1, "go"), + consoleMsg(4, "running"), + agentChunk(5, "out"), + promptResponseMsg(6, 1), + ], +}; + +describe("createIncrementalConversationBuilder", () => { + for (const [name, events] of Object.entries(SCENARIOS)) { + for (const pending of [true, false, null] as const) { + it(`matches buildConversationItems at every prefix — ${name} (pending=${pending})`, () => { + assertEquivalentAcrossPrefixes(events, pending); + }); + } + } + + it("keeps completed-turn item references stable while the active turn streams", () => { + const inc = createIncrementalConversationBuilder(); + const base = [ + userPromptMsg(1, 1, "first"), + agentChunk(2, "answer one"), + promptResponseMsg(3, 1), + userPromptMsg(4, 2, "second"), + agentChunk(5, "A"), + ]; + const r1 = inc.update(base, true); + // [u1, a1, u2, a2] + expect(r1.items).toHaveLength(4); + + const next = [...base, agentChunk(6, "B")]; + const r2 = inc.update(next, true); + + // Completed turn 1: identical object references — no re-render. + expect(r2.items[0]).toBe(r1.items[0]); + expect(r2.items[1]).toBe(r1.items[1]); + // Active turn's static user message also stays referentially stable. + expect(r2.items[2]).toBe(r1.items[2]); + // Active streaming message is re-derived and its text has grown. + expect(r2.items[3]).not.toBe(r1.items[3]); + const active = r2.items[3]; + if (active.type !== "session_update" || !("content" in active.update)) { + throw new Error("expected an agent message item"); + } + expect((active.update.content as { text: string }).text).toBe("AB"); + }); + + it("re-derives the active turn's context each call so live updates surface", () => { + const inc = createIncrementalConversationBuilder(); + const base = [userPromptMsg(1, 1, "go"), toolCallMsg(2, "t1")]; + const r1 = inc.update(base, true); + const next = [...base, toolUpdateMsg(3, "t1", { status: "completed" })]; + const r2 = inc.update(next, true); + + const tool1 = r1.items.find((i) => i.type === "session_update"); + const tool2 = r2.items.find((i) => i.type === "session_update"); + expect(tool1?.type).toBe("session_update"); + // Same logical row, fresh object — its memoized view re-renders. + expect(tool2).not.toBe(tool1); + if (tool2?.type === "session_update") { + expect(tool2.turnContext.toolCalls.get("t1")?.status).toBe("completed"); + } + }); +}); diff --git a/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.ts b/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.ts new file mode 100644 index 000000000..9ef97a303 --- /dev/null +++ b/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.ts @@ -0,0 +1,162 @@ +import type { AcpMessage } from "@shared/types/session-events"; +import { + type BuildConversationOptions, + type BuildResult, + buildConversationItems, + type ConversationItem, + createItemBuilder, + type ItemBuilder, + markThoughtCompletion, + processEvent, + readLastTurnInfo, + type TurnContext, +} from "./buildConversationItems"; + +/** + * Incremental front end for `buildConversationItems`. + * + * A full rebuild re-parses every event on every streamed token — O(n) per + * token, O(n^2) per turn. This processes each event exactly once into a + * persistent builder and, on every call, freezes completed turns (their item + * objects keep identity, so memoized rows skip re-render) while re-deriving + * only the active turn. Per call the cost is proportional to the active turn, + * not the whole thread. + * + * Output is content-equivalent to `buildConversationItems` for every prefix of + * events — see incrementalConversationItems.test.ts. It falls back to a full + * rebuild whenever the append-only fast path can't faithfully represent the + * state (idle, non-append event change, options change, or a progress card in + * an already-frozen turn being mutated). + */ +export function createIncrementalConversationBuilder() { + let b: ItemBuilder | null = null; + let processedCount = 0; + let firstEventRef: AcpMessage | null = null; + let boundaryEventRef: AcpMessage | null = null; + let showDebugLogs: boolean | undefined; + + function reset() { + b = null; + processedCount = 0; + firstEventRef = null; + boundaryEventRef = null; + } + + function update( + events: AcpMessage[], + isPromptPending: boolean | null, + options?: BuildConversationOptions, + ): BuildResult { + const debug = options?.showDebugLogs; + + // Idle (not streaming): cheap to rebuild, and it sidesteps the speculative + // end-of-stream completions that only `buildConversationItems` resolves. + if (isPromptPending === false) { + reset(); + return buildConversationItems(events, isPromptPending, options); + } + + // The fast path is valid only when this call appends to the exact prefix we + // already processed (events is append-only during streaming, immer hands us + // a new array each push but keeps element identity). + const canAppend = + b !== null && + debug === showDebugLogs && + events.length >= processedCount && + (processedCount === 0 || events[0] === firstEventRef) && + (processedCount === 0 || events[processedCount - 1] === boundaryEventRef); + + if (!canAppend) { + b = createItemBuilder(); + processedCount = 0; + showDebugLogs = debug; + } + + const builder = b as ItemBuilder; + builder.lowestTouchedProgressIndex = Number.POSITIVE_INFINITY; + for (let i = processedCount; i < events.length; i++) { + processEvent(builder, events[i], options); + } + processedCount = events.length; + firstEventRef = events[0] ?? null; + boundaryEventRef = events[processedCount - 1] ?? null; + + const turn = builder.currentTurn; + const activeStart = + turn && !turn.isComplete + ? builder.currentTurnStartIndex + : builder.items.length; + + // A progress card living in the frozen region was mutated by this batch — + // an event reached back across a turn boundary. The append-only view can't + // show that, so rebuild fully this frame (the persistent builder stays + // valid for the next one). + if (builder.lowestTouchedProgressIndex < activeStart) { + return buildConversationItems(events, isPromptPending, options); + } + + // `buildConversationItems` always marks a trailing implicit turn complete. + // Replicate that on the live turn's context so thought-completion matches; + // it's safe to persist (a later real completion still flows through + // `completePromptTurn`, which gates on `isComplete`, left untouched here). + if (turn && turn.promptId === -1) { + turn.context.turnComplete = true; + } + + markThoughtCompletion(builder.items); + + return { + items: assembleItems(builder, activeStart), + lastTurnInfo: readLastTurnInfoForOutput(builder), + isCompacting: builder.isCompacting, + }; + } + + return { update, reset }; +} + +function assembleItems( + b: ItemBuilder, + activeStart: number, +): ConversationItem[] { + // Completed turns: reuse the builder's own objects. They aren't rebuilt + // across calls, so their identity is stable and memoized rows skip work. + const out = b.items.slice(0, activeStart); + if (activeStart >= b.items.length) return out; + + const turn = b.currentTurn; + // The active turn streams: clone its rows onto a fresh shared context each + // call so their memoized views re-render and read the latest tool/child + // state — matching the all-new-objects behavior a full rebuild gives the + // live turn. Non-update rows (the user message, git actions) never change, + // so pass them through by reference. + const activeContext: TurnContext | null = turn + ? { + toolCalls: turn.context.toolCalls, + childItems: turn.context.childItems, + turnCancelled: turn.context.turnCancelled, + turnComplete: turn.context.turnComplete, + } + : null; + + for (let i = activeStart; i < b.items.length; i++) { + const item = b.items[i]; + if (item.type === "session_update" && activeContext) { + out.push({ ...item, turnContext: activeContext }); + } else { + out.push(item); + } + } + return out; +} + +function readLastTurnInfoForOutput(b: ItemBuilder) { + const info = readLastTurnInfo(b); + if (!info) return null; + // A trailing implicit turn reports complete (no prompt response will arrive + // to flip it), mirroring `buildConversationItems`' finalization. + if (b.currentTurn?.promptId === -1) { + return { ...info, isComplete: true }; + } + return info; +} diff --git a/apps/code/src/renderer/features/sessions/hooks/useConversationItems.ts b/apps/code/src/renderer/features/sessions/hooks/useConversationItems.ts new file mode 100644 index 000000000..dd9fa9008 --- /dev/null +++ b/apps/code/src/renderer/features/sessions/hooks/useConversationItems.ts @@ -0,0 +1,57 @@ +import type { + BuildConversationOptions, + BuildResult, +} from "@features/sessions/components/buildConversationItems"; +import { createIncrementalConversationBuilder } from "@features/sessions/components/incrementalConversationItems"; +import type { AcpMessage } from "@shared/types/session-events"; +import { useRef } from "react"; + +interface Cache { + impl: ReturnType; + events: AcpMessage[] | null; + pending: boolean | null; + debug: boolean | undefined; + result: BuildResult | null; +} + +/** + * Builds conversation items incrementally — each event is parsed once and + * completed turns are reused by reference, so a streamed token costs work + * proportional to the active turn rather than the whole thread. The persistent + * builder lives in a ref; results are memoized on the (events, pending, debug) + * triple so unrelated re-renders don't re-derive. + */ +export function useConversationItems( + events: AcpMessage[], + isPromptPending: boolean | null, + options?: BuildConversationOptions, +): BuildResult { + const ref = useRef(null); + if (!ref.current) { + ref.current = { + impl: createIncrementalConversationBuilder(), + events: null, + pending: null, + debug: undefined, + result: null, + }; + } + const cache = ref.current; + const debug = options?.showDebugLogs; + + if ( + cache.result && + cache.events === events && + cache.pending === isPromptPending && + cache.debug === debug + ) { + return cache.result; + } + + const result = cache.impl.update(events, isPromptPending, options); + cache.events = events; + cache.pending = isPromptPending; + cache.debug = debug; + cache.result = result; + return result; +} From 4f0618eee4f54052259c8222efc14d1ed98a711e Mon Sep 17 00:00:00 2001 From: Adam Leith Date: Wed, 3 Jun 2026 20:45:07 +0100 Subject: [PATCH 3/6] fix(code): don't drop a sent user message while it swaps from optimistic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `replaceOptimisticWithEvent` clears the optimistic user-message placeholder in the same store update that appends the real prompt event. With the conversation parse fed by a purely trailing frame-throttle, that appended event wasn't visible to the parse until the next frame — so for the frames in between the placeholder was gone and the real message not yet derived, and the message flickered out (worse under streaming load, when the trailing frame is delayed). Give the throttle a leading edge via useLayoutEffect: the first change after an idle window applies synchronously before paint, so a structural change like a freshly sent message never drops a frame. Streaming bursts still coalesce on the trailing rAF. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../hooks/useFrameThrottledValue.test.ts | 33 +++++++++++-------- .../renderer/hooks/useFrameThrottledValue.ts | 30 +++++++++-------- 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/apps/code/src/renderer/hooks/useFrameThrottledValue.test.ts b/apps/code/src/renderer/hooks/useFrameThrottledValue.test.ts index 0e8afcb82..76c389525 100644 --- a/apps/code/src/renderer/hooks/useFrameThrottledValue.test.ts +++ b/apps/code/src/renderer/hooks/useFrameThrottledValue.test.ts @@ -33,47 +33,50 @@ describe("useFrameThrottledValue", () => { expect(result.current).toBe("initial"); }); - it("settles on the latest value after a frame", () => { + it("applies the first change after an idle window immediately (leading edge)", () => { const { result, rerender } = renderHook( ({ value }) => useFrameThrottledValue(value), { initialProps: { value: "a" } }, ); + // Let the mount's window close so the next change hits the leading edge. + flushFrame(); rerender({ value: "b" }); - expect(result.current).toBe("a"); - - flushFrame(); + // No frame needed — leading edge applied it before paint. expect(result.current).toBe("b"); }); - it("coalesces a burst of changes within one frame into a single update", () => { + it("coalesces a burst within one open window into a single trailing flush", () => { const { result, rerender } = renderHook( ({ value }) => useFrameThrottledValue(value), { initialProps: { value: 0 } }, ); + flushFrame(); - // Many changes before the frame fires — only one rAF should be queued. - for (let i = 1; i <= 10; i++) rerender({ value: i }); + // First change opens the window via the leading edge... + rerender({ value: 1 }); + expect(result.current).toBe(1); + // ...the rest collapse into the pending frame. + for (let i = 2; i <= 10; i++) rerender({ value: i }); + expect(result.current).toBe(1); expect(queue.length).toBe(1); - expect(result.current).toBe(0); flushFrame(); - // Lands on the freshest value, skipping every intermediate one. expect(result.current).toBe(10); }); - it("keeps streaming across multiple frames", () => { + it("keeps streaming across multiple windows", () => { const { result, rerender } = renderHook( ({ value }) => useFrameThrottledValue(value), { initialProps: { value: "t0" } }, ); + flushFrame(); rerender({ value: "t1" }); - flushFrame(); expect(result.current).toBe("t1"); + flushFrame(); rerender({ value: "t2" }); - flushFrame(); expect(result.current).toBe("t2"); }); @@ -94,10 +97,14 @@ describe("useFrameThrottledValue", () => { ({ value }) => useFrameThrottledValue(value), { initialProps: { value: "a" } }, ); + flushFrame(); + // Open a window, then queue a second change into it before unmounting. rerender({ value: "b" }); + rerender({ value: "c" }); unmount(); flushFrame(); - expect(result.current).toBe("a"); + // The trailing flush to "c" never ran. + expect(result.current).toBe("b"); }); }); diff --git a/apps/code/src/renderer/hooks/useFrameThrottledValue.ts b/apps/code/src/renderer/hooks/useFrameThrottledValue.ts index 49e3437fe..c63efe569 100644 --- a/apps/code/src/renderer/hooks/useFrameThrottledValue.ts +++ b/apps/code/src/renderer/hooks/useFrameThrottledValue.ts @@ -1,36 +1,40 @@ -import { useEffect, useRef, useState } from "react"; +import { useLayoutEffect, useRef, useState } from "react"; /** - * Coalesces rapid changes to `value` so dependents recompute at most once per - * animation frame. + * Coalesces rapid changes to `value` so dependents recompute at most ~once per + * animation frame, with a leading edge so the first change after a quiet period + * is never delayed. * * During token streaming the upstream events array changes on every chunk; - * without this each chunk drives a full O(n) conversation rebuild + reconcile, - * so a long thread does far more rebuilds per second than it can paint. This - * caps rebuilds to the display rate while always settling on the latest value - * within one frame of the final change — so the end-of-stream state is exact, - * never stale. + * without coalescing each chunk drives a fresh derive + reconcile, far more + * often than the screen can paint. But a purely trailing throttle drops a frame + * on structural changes — e.g. a sent user message whose optimistic placeholder + * is cleared in the same store update that appends the real event would flicker + * out until the trailing flush caught up. So the first change after the window + * is idle applies synchronously before paint (leading), while a burst of + * changes within an open window collapses into one trailing flush. */ export function useFrameThrottledValue(value: T): T { const [throttled, setThrottled] = useState(value); const latestRef = useRef(value); const rafRef = useRef(null); - useEffect(() => { + useLayoutEffect(() => { latestRef.current = value; - // A frame is already queued; it will read latestRef when it fires, so we - // don't schedule a second one. This is what collapses a burst of changes - // within the same frame into a single recompute. + // A coalescing window is already open; the pending frame flushes the latest. if (rafRef.current !== null) return; + // Leading edge: apply immediately (pre-paint) and open a window. + setThrottled(value); rafRef.current = requestAnimationFrame(() => { rafRef.current = null; + // Trailing edge: flush anything that arrived while the window was open. setThrottled((prev) => prev === latestRef.current ? prev : latestRef.current, ); }); }, [value]); - useEffect( + useLayoutEffect( () => () => { if (rafRef.current !== null) cancelAnimationFrame(rafRef.current); }, From 13f96dcb0c6a0c607f126274eee7c7218f02aa08 Mon Sep 17 00:00:00 2001 From: Adam Leith Date: Wed, 3 Jun 2026 20:52:37 +0100 Subject: [PATCH 4/6] fix(code): remove conversation frame-throttle that dropped sent messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frame-throttle delayed the parser's view of `events` by up to a frame. `replaceOptimisticWithEvent` clears the optimistic user-message placeholder in the same store commit that appends the real prompt event, so during the lag the placeholder was gone and the real message not yet derived — the sent message flickered out. A leading-edge tweak didn't fully close the window. The parse is now incremental (each event handled once, completed turns reused by reference), so per-token cost already tracks the active turn rather than the whole thread. The throttle's marginal benefit no longer justifies the timing risk, so feed `events` straight through and drop the hook. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../sessions/components/ConversationView.tsx | 15 ++- .../incrementalConversationItems.test.ts | 35 ++++++ .../hooks/useFrameThrottledValue.test.ts | 110 ------------------ .../renderer/hooks/useFrameThrottledValue.ts | 45 ------- 4 files changed, 42 insertions(+), 163 deletions(-) delete mode 100644 apps/code/src/renderer/hooks/useFrameThrottledValue.test.ts delete mode 100644 apps/code/src/renderer/hooks/useFrameThrottledValue.ts diff --git a/apps/code/src/renderer/features/sessions/components/ConversationView.tsx b/apps/code/src/renderer/features/sessions/components/ConversationView.tsx index f8ae75c7e..2a1d9e994 100644 --- a/apps/code/src/renderer/features/sessions/components/ConversationView.tsx +++ b/apps/code/src/renderer/features/sessions/components/ConversationView.tsx @@ -12,7 +12,6 @@ import { } from "@features/sessions/stores/sessionStore"; import { useSettingsStore } from "@features/settings/stores/settingsStore"; import { SkillButtonActionMessage } from "@features/skill-buttons/components/SkillButtonActionMessage"; -import { useFrameThrottledValue } from "@hooks/useFrameThrottledValue"; import { ArrowDown, XCircle } from "@phosphor-icons/react"; import { WorkerPoolContextProvider } from "@pierre/diffs/react"; import WorkerUrl from "@pierre/diffs/worker/worker.js?worker&url"; @@ -81,18 +80,18 @@ export function ConversationView({ const debugLogsCloudRuns = useSettingsStore((s) => s.debugLogsCloudRuns); const showDebugLogs = debugLogsCloudRuns; - // Streaming appends one event per token, so `events` changes far faster than - // the screen can paint. Coalesce to one parse per frame; the parse itself is - // incremental (completed turns reused by reference) so per-frame cost tracks - // the active turn, not the whole thread. Settles on the exact final state. - const throttledEvents = useFrameThrottledValue(events); - const contextUsage = useContextUsage(throttledEvents); + const contextUsage = useContextUsage(events); + // Streaming appends one event per token. The parse is incremental — each + // event is handled once and completed turns are reused by reference — so per + // token the work tracks the active turn, not the whole thread. We feed + // `events` directly (no frame-throttle) so a sent message's optimistic->real + // swap is never delayed past the frame the store commits it. const { items: conversationItems, lastTurnInfo, isCompacting, - } = useConversationItems(throttledEvents, isPromptPending, { + } = useConversationItems(events, isPromptPending, { showDebugLogs, }); diff --git a/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.test.ts b/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.test.ts index 0ce85c335..35529acd9 100644 --- a/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.test.ts +++ b/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.test.ts @@ -303,6 +303,41 @@ describe("createIncrementalConversationBuilder", () => { expect((active.update.content as { text: string }).text).toBe("AB"); }); + it("preserves a sent user message across idle -> streaming -> complete transitions", () => { + const inc = createIncrementalConversationBuilder(); + const userMessages = (r: BuildResult) => + r.items.filter((i) => i.type === "user_message").map((i) => i.content); + + const turn1 = [ + userPromptMsg(1, 1, "first"), + agentChunk(2, "answer one"), + promptResponseMsg(3, 1), + ]; + expect(userMessages(inc.update(turn1, false))).toEqual(["first"]); + + // User sends "second": the prompt echo is appended to events. + const withPrompt = [...turn1, userPromptMsg(4, 2, "second")]; + // Renders that may straddle the pending flip — message must persist through all. + expect(userMessages(inc.update(withPrompt, false))).toEqual([ + "first", + "second", + ]); + expect(userMessages(inc.update(withPrompt, true))).toEqual([ + "first", + "second", + ]); + + const withChunk = [...withPrompt, agentChunk(5, "answer two")]; + expect(userMessages(inc.update(withChunk, true))).toEqual([ + "first", + "second", + ]); + + const done = [...withChunk, promptResponseMsg(6, 2)]; + expect(userMessages(inc.update(done, true))).toEqual(["first", "second"]); + expect(userMessages(inc.update(done, false))).toEqual(["first", "second"]); + }); + it("re-derives the active turn's context each call so live updates surface", () => { const inc = createIncrementalConversationBuilder(); const base = [userPromptMsg(1, 1, "go"), toolCallMsg(2, "t1")]; diff --git a/apps/code/src/renderer/hooks/useFrameThrottledValue.test.ts b/apps/code/src/renderer/hooks/useFrameThrottledValue.test.ts deleted file mode 100644 index 76c389525..000000000 --- a/apps/code/src/renderer/hooks/useFrameThrottledValue.test.ts +++ /dev/null @@ -1,110 +0,0 @@ -import { act, renderHook } from "@testing-library/react"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { useFrameThrottledValue } from "./useFrameThrottledValue"; - -describe("useFrameThrottledValue", () => { - let queue: Array<() => void>; - - beforeEach(() => { - queue = []; - vi.stubGlobal("requestAnimationFrame", (cb: FrameRequestCallback) => { - queue.push(() => cb(0)); - return queue.length; - }); - vi.stubGlobal("cancelAnimationFrame", (id: number) => { - queue[id - 1] = () => {}; - }); - }); - - afterEach(() => { - vi.unstubAllGlobals(); - }); - - function flushFrame() { - const pending = queue; - queue = []; - act(() => { - for (const fn of pending) fn(); - }); - } - - it("returns the initial value synchronously", () => { - const { result } = renderHook(() => useFrameThrottledValue("initial")); - expect(result.current).toBe("initial"); - }); - - it("applies the first change after an idle window immediately (leading edge)", () => { - const { result, rerender } = renderHook( - ({ value }) => useFrameThrottledValue(value), - { initialProps: { value: "a" } }, - ); - // Let the mount's window close so the next change hits the leading edge. - flushFrame(); - - rerender({ value: "b" }); - // No frame needed — leading edge applied it before paint. - expect(result.current).toBe("b"); - }); - - it("coalesces a burst within one open window into a single trailing flush", () => { - const { result, rerender } = renderHook( - ({ value }) => useFrameThrottledValue(value), - { initialProps: { value: 0 } }, - ); - flushFrame(); - - // First change opens the window via the leading edge... - rerender({ value: 1 }); - expect(result.current).toBe(1); - // ...the rest collapse into the pending frame. - for (let i = 2; i <= 10; i++) rerender({ value: i }); - expect(result.current).toBe(1); - expect(queue.length).toBe(1); - - flushFrame(); - expect(result.current).toBe(10); - }); - - it("keeps streaming across multiple windows", () => { - const { result, rerender } = renderHook( - ({ value }) => useFrameThrottledValue(value), - { initialProps: { value: "t0" } }, - ); - flushFrame(); - - rerender({ value: "t1" }); - expect(result.current).toBe("t1"); - flushFrame(); - - rerender({ value: "t2" }); - expect(result.current).toBe("t2"); - }); - - it("does not schedule a frame when the value is unchanged", () => { - const { rerender } = renderHook( - ({ value }) => useFrameThrottledValue(value), - { initialProps: { value: "a" } }, - ); - flushFrame(); - queue = []; - - rerender({ value: "a" }); - expect(queue.length).toBe(0); - }); - - it("cancels a pending frame on unmount", () => { - const { result, rerender, unmount } = renderHook( - ({ value }) => useFrameThrottledValue(value), - { initialProps: { value: "a" } }, - ); - flushFrame(); - - // Open a window, then queue a second change into it before unmounting. - rerender({ value: "b" }); - rerender({ value: "c" }); - unmount(); - flushFrame(); - // The trailing flush to "c" never ran. - expect(result.current).toBe("b"); - }); -}); diff --git a/apps/code/src/renderer/hooks/useFrameThrottledValue.ts b/apps/code/src/renderer/hooks/useFrameThrottledValue.ts deleted file mode 100644 index c63efe569..000000000 --- a/apps/code/src/renderer/hooks/useFrameThrottledValue.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { useLayoutEffect, useRef, useState } from "react"; - -/** - * Coalesces rapid changes to `value` so dependents recompute at most ~once per - * animation frame, with a leading edge so the first change after a quiet period - * is never delayed. - * - * During token streaming the upstream events array changes on every chunk; - * without coalescing each chunk drives a fresh derive + reconcile, far more - * often than the screen can paint. But a purely trailing throttle drops a frame - * on structural changes — e.g. a sent user message whose optimistic placeholder - * is cleared in the same store update that appends the real event would flicker - * out until the trailing flush caught up. So the first change after the window - * is idle applies synchronously before paint (leading), while a burst of - * changes within an open window collapses into one trailing flush. - */ -export function useFrameThrottledValue(value: T): T { - const [throttled, setThrottled] = useState(value); - const latestRef = useRef(value); - const rafRef = useRef(null); - - useLayoutEffect(() => { - latestRef.current = value; - // A coalescing window is already open; the pending frame flushes the latest. - if (rafRef.current !== null) return; - // Leading edge: apply immediately (pre-paint) and open a window. - setThrottled(value); - rafRef.current = requestAnimationFrame(() => { - rafRef.current = null; - // Trailing edge: flush anything that arrived while the window was open. - setThrottled((prev) => - prev === latestRef.current ? prev : latestRef.current, - ); - }); - }, [value]); - - useLayoutEffect( - () => () => { - if (rafRef.current !== null) cancelAnimationFrame(rafRef.current); - }, - [], - ); - - return throttled; -} From 817e974f692703578b5ac657945de34c51a9414a Mon Sep 17 00:00:00 2001 From: Adam Leith Date: Wed, 3 Jun 2026 21:07:36 +0100 Subject: [PATCH 5/6] perf(code): memoize UserMessage to stop per-scroll markdown re-renders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A scroll-trace of a very long thread showed react-virtual flush-syncing a render on every scroll event, and the dominant cost was re-rendering visible user messages. Unlike agent messages — which sit under the memoized SessionUpdateRow and are themselves memoized — UserMessage is rendered directly by the conversation's renderItem with no memo, so every visible one re-ran MarkdownRenderer on every scroll-driven parent render. Wrap it in memo. Its props are referentially stable for completed turns (the incremental parser reuses their objects), so memo skips them while scrolling. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/session-update/UserMessage.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/code/src/renderer/features/sessions/components/session-update/UserMessage.tsx b/apps/code/src/renderer/features/sessions/components/session-update/UserMessage.tsx index 536bebff1..698f9255a 100644 --- a/apps/code/src/renderer/features/sessions/components/session-update/UserMessage.tsx +++ b/apps/code/src/renderer/features/sessions/components/session-update/UserMessage.tsx @@ -10,7 +10,7 @@ import { } from "@phosphor-icons/react"; import { Box, Flex, IconButton } from "@radix-ui/themes"; import { motion } from "framer-motion"; -import { useCallback, useEffect, useRef, useState } from "react"; +import { memo, useCallback, useEffect, useRef, useState } from "react"; import { hasFileMentions, MentionChip, @@ -43,7 +43,12 @@ function formatTimestamp(ts: number): string { }); } -export function UserMessage({ +// Rendered directly by the conversation's renderItem (no memoized wrapper, unlike +// agent messages which sit under SessionUpdateRow), so without memo every visible +// user message re-runs MarkdownRenderer on every parent render — and the +// virtualizer flushSync-renders on every scroll event. Props are referentially +// stable for completed turns (incremental parser), so memo skips them on scroll. +export const UserMessage = memo(function UserMessage({ content, timestamp, sourceUrl, @@ -175,4 +180,4 @@ export function UserMessage({ ); -} +}); From 4c7210c77306ba667585e583df1d019d63dffa8f Mon Sep 17 00:00:00 2001 From: Adam Leith Date: Wed, 3 Jun 2026 22:10:48 +0100 Subject: [PATCH 6/6] test(code): parameterize equivalence cases with it.each MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use Vitest's native it.each for the SCENARIOS × pending matrix so each case is a first-class, individually named test entry, per project convention. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../incrementalConversationItems.test.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.test.ts b/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.test.ts index 35529acd9..de4b8750c 100644 --- a/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.test.ts +++ b/apps/code/src/renderer/features/sessions/components/incrementalConversationItems.test.ts @@ -264,14 +264,17 @@ const SCENARIOS: Record = { ], }; +const EQUIVALENCE_CASES = Object.entries(SCENARIOS).flatMap(([name, events]) => + ([true, false, null] as const).map((pending) => ({ name, events, pending })), +); + describe("createIncrementalConversationBuilder", () => { - for (const [name, events] of Object.entries(SCENARIOS)) { - for (const pending of [true, false, null] as const) { - it(`matches buildConversationItems at every prefix — ${name} (pending=${pending})`, () => { - assertEquivalentAcrossPrefixes(events, pending); - }); - } - } + it.each(EQUIVALENCE_CASES)( + "matches buildConversationItems at every prefix — $name (pending=$pending)", + ({ events, pending }) => { + assertEquivalentAcrossPrefixes(events, pending); + }, + ); it("keeps completed-turn item references stable while the active turn streams", () => { const inc = createIncrementalConversationBuilder();