diff --git a/webviews/src/agent-session/acpmux/App.tsx b/webviews/src/agent-session/acpmux/App.tsx index d55641b2de94..be66bd681018 100644 --- a/webviews/src/agent-session/acpmux/App.tsx +++ b/webviews/src/agent-session/acpmux/App.tsx @@ -193,6 +193,15 @@ import { createAcpmuxDebug, type AcpmuxDebug } from "./debug"; import { type AutomationCompose, pageAction } from "./automation"; import { acpWire } from "./wire"; import { acpmuxPerf, type MountedSpan } from "./perf"; +import { + drawsAsIs, + drewStreamingRow, + setStreamingRowBlocked, + streamingRow, + streamingRowBlocked, + textOnlyRow, + useStreamingRow, +} from "./streamingRow"; import { followTrace } from "./followTrace"; import { ScrollPacing } from "./pacing"; import { AdaptiveRenderRate, reportScrollPacing } from "./renderPacing"; @@ -2355,6 +2364,7 @@ const RowFrame = React.memo(function RowFrame({ children: React.ReactNode; }) { acpmuxPerf.span("row.renders", 1); + if (acpmuxPerf.enabled) acpmuxPerf.render(`row:${row.id}`); const t = useT(); const ref = useRef(null); const [entering] = useState(enter); @@ -2830,8 +2840,26 @@ function TranscriptContext({ ); } +/// The index of the streaming reply's row when `next` differs from `drawn` only in that row's +/// text (HOT PATH AUDIT area F, row W1), else -1. Every other field compares by identity: the client +/// keeps the same objects while they do not change (direct.ts `publishedSummary`, `NO_CATALOG`, +/// subagentFold.ts `views`), so this is O(fields + rows), with no serialization. Drawn only while +/// the transcript draws that row as the client holds it (`drawsAsIs`) and no view that reads the +/// pane's rows is open (`streamingRowBlocked`). +function streamedOnly(drawn: AcpmuxSnapshot, next: AcpmuxSnapshot): number { + const before = drawn as unknown as Record; + const after = next as unknown as Record; + const keys = Object.keys(after); + if (keys.length !== Object.keys(before).length) return -1; + for (const key of keys) if (key !== "rows" && !Object.is(before[key], after[key])) return -1; + if (streamingRowBlocked()) return -1; + const at = textOnlyRow(drawn.rows, next.rows); + return at >= 0 && drawsAsIs(drawn.rows[at]!) ? at : -1; +} + export function VirtualTranscript({ - rows, + rows: paneRows, + streams = false, earlier, promptCount, loadThrough, @@ -2871,6 +2899,9 @@ export function VirtualTranscript({ topInset = 0, }: { rows: readonly AcpmuxRow[]; + /// The pane's live transcript: it draws the streaming reply's row ahead of the pane's snapshot + /// (streamingRow.ts), so a streamed chunk re-renders this transcript and that row, not the pane. + streams?: boolean; /// The chat's prompts older than `rows` (the snapshot's `earlierPrompts`): the thread rail lays /// them out from the first frame, so it never grows or shifts as history pages in. earlier?: readonly AcpmuxRow[]; @@ -2963,6 +2994,44 @@ export function VirtualTranscript({ /// above the composer. 0 for a scroller below its header. topInset?: number; }) { + // The streaming reply's newer text, when the pane has not redrawn for it (streamingRow.ts): that + // row only, in place, with its version moved on by the chunks it is ahead. + const live = useStreamingRow(streams); + const rows = useMemo(() => { + if (!streams || !live) return paneRows; + for (let index = paneRows.length - 1; index >= 0; index -= 1) { + const row = paneRows[index]!; + if (row.id !== live.row.id) continue; + if (row.text !== live.drawnText) return paneRows; + const next = paneRows.slice(); + const version = row.version + live.ahead; + next[index] = { ...row, text: live.row.text, version }; + // The live line under the reply stays quiet while the reply's text moves (turns.ts + // `quietKey` names the reply's id and version): it moves with the newer version here. + const stale = `${row.id}:${row.version}`; + for (let below = index + 1; below < next.length; below += 1) { + const line = next[below]!; + if (line.kind === THINKING && line.quietKey === stale) + next[below] = { ...line, quietKey: `${row.id}:${version}`, version: line.version + live.ahead }; + } + return next; + } + return paneRows; + }, [paneRows, live, streams]); + // What this transcript drew of the pane's streaming reply: the pane takes the fast path only + // while that row is drawn as the client holds it. + useLayoutEffect(() => { + if (!streams) return; + for (let index = paneRows.length - 1; index >= 0; index -= 1) { + const row = paneRows[index]!; + if (row.kind === "assistant" && row.streaming) { + drewStreamingRow(row.id, row.text); + return; + } + if (row.kind === "user") break; + } + drewStreamingRow(undefined, undefined); + }, [paneRows, streams]); const registry = givenRegistry ?? (readOnly ? readOnlyRegistry : defaultRegistry); const t = useT(); // Debug measurement (acpmuxPerf): off until the first debug call. @@ -6168,6 +6237,7 @@ const NO_PROJECTS: Project[] = []; function AcpmuxPane({ initialChat }: { initialChat?: InitialChat }) { // Debug measurement (acpmuxPerf): the pane's own render, before its children. const paneRenderStart = acpmuxPerf.enabled ? performance.now() : 0; + acpmuxPerf.render("pane"); const t = useT(); const { scrollToBottomOnSend, newChatsIn } = useComposerSettings(); const scrollToBottomOnSendRef = useRef(scrollToBottomOnSend); @@ -6468,6 +6538,11 @@ function AcpmuxPane({ initialChat }: { initialChat?: InitialChat }) { : undefined, [artifact, artifactRows, snapshot.sessionId], ); + // An open artifact follows its fence in the pane's rows: chunks draw through the pane meanwhile. + useLayoutEffect(() => { + setStreamingRowBlocked(artifact !== undefined, "artifact"); + return () => setStreamingRowBlocked(false, "artifact"); + }, [artifact]); const liveArtifact = liveDocument?.artifact ?? artifact; // Render cards by id (cx-ncc.94): a render that names an earlier one's id shows in that card. const pages = useMemo(() => renderPages(snapshot.rows), [snapshot.rows]); @@ -9501,6 +9576,9 @@ function AcpmuxPane({ initialChat }: { initialChat?: InitialChat }) { void callNative(QUICK_MESSAGES.dismiss).catch(() => undefined); }); const rowsRef = useRef(initialChat?.snapshot.rows ?? []); + /// The direct client's snapshot the pane last drew; a newer one that only grows the streaming + /// reply's text goes to that row instead (streamingRow.ts). + const drawnSnapshot = useRef(undefined); /// The newest snapshot, for host requests that read it (pane.context). const snapshotRef = useRef(initialChat?.snapshot); /// The chat menu shows New side chat: the right-click menu then opens one from any turn that forks. @@ -11042,6 +11120,16 @@ function AcpmuxPane({ initialChat }: { initialChat?: InitialChat }) { showsNewTabRef.current, ); } + // Only the streaming reply's text grew: its row draws it (streamingRow.ts); the pane, + // the composer and Session Info keep the snapshot they drew. + const drawn = drawnSnapshot.current; + const at = drawn ? streamedOnly(drawn, next) : -1; + if (drawn && at >= 0) { + streamingRow.set(next.rows[at]!, drawn.rows[at]!); + return; + } + streamingRow.clear(); + drawnSnapshot.current = next; setSnapshot((previous) => { if ( next.canHandoff && @@ -13438,6 +13526,7 @@ function AcpmuxPane({ initialChat }: { initialChat?: InitialChat }) { key={chatPresentation} topInset={soloUnder ? tabRow.height : 0} rows={transcriptRows} + streams={!childRows} earlier={!childRows ? snapshot.earlierPrompts : undefined} promptCount={!childRows ? snapshot.promptCount : undefined} loadThrough={ diff --git a/webviews/src/agent-session/acpmux/Composer.tsx b/webviews/src/agent-session/acpmux/Composer.tsx index 0b2e613430cb..ecfe961dda58 100644 --- a/webviews/src/agent-session/acpmux/Composer.tsx +++ b/webviews/src/agent-session/acpmux/Composer.tsx @@ -1,3 +1,4 @@ +import { acpmuxPerf } from "./perf"; import React, { useCallback, useEffect, useImperativeHandle, useLayoutEffect, useMemo, useRef, useState } from "react"; import { createPortal } from "react-dom"; // Send's iMessage colors (--cmux-messages-blue); build-agent-pane-web.sh inlines the sheet. @@ -496,6 +497,7 @@ export function Composer({ placeholder, onOpenContext, }: Props) { + acpmuxPerf.render("composer"); const t = useT(); const chrome = useComposerChrome(); // Off (a queued row's ⋯ menu, queueing.ts), what is sent mid-turn steers on every key. diff --git a/webviews/src/agent-session/acpmux/conversation/ChatFindScope.tsx b/webviews/src/agent-session/acpmux/conversation/ChatFindScope.tsx index 6f06a9e318aa..17bb5597d84a 100644 --- a/webviews/src/agent-session/acpmux/conversation/ChatFindScope.tsx +++ b/webviews/src/agent-session/acpmux/conversation/ChatFindScope.tsx @@ -1,6 +1,7 @@ // Find in Chat's state (cx-ncc.86): it lives here, under the pane, so a keystroke in the find bar // renders the transcript alone, not the composer and header beside it. The pane's page actions // reach it through `findRef`. +import { setStreamingRowBlocked } from "../streamingRow"; import { type ReactNode, type RefObject, useEffect, useLayoutEffect } from "react"; import type { AcpmuxRow } from "../model"; import { type ChatFind, isFindRow, useChatFind } from "./chatFind"; @@ -24,6 +25,12 @@ export function ChatFindScope({ }) { const find = useChatFind(rows); findRef.current = find; + // While Find is open it searches the pane's rows, so a streamed chunk draws through the pane + // (streamingRow.ts) and new text is found as it comes. + useLayoutEffect(() => { + setStreamingRowBlocked(find.open, "find"); + return () => setStreamingRowBlocked(false, "find"); + }, [find.open]); // A view without the transcript (New Tab, replay) has no Find; the page actions then do nothing. useEffect( () => () => { diff --git a/webviews/src/agent-session/acpmux/direct.ts b/webviews/src/agent-session/acpmux/direct.ts index 3c8c01e3df26..e86f692f0dbc 100644 --- a/webviews/src/agent-session/acpmux/direct.ts +++ b/webviews/src/agent-session/acpmux/direct.ts @@ -255,6 +255,8 @@ type Reply = { }; type Notification = { method: string; params?: any }; type Listener = (snapshot: AcpmuxSnapshot) => void; +/// The snapshot's catalog: one empty array, so equal snapshots compare by identity. +const NO_CATALOG: AcpmuxSnapshot["catalog"] = Object.freeze([]) as unknown as AcpmuxSnapshot["catalog"]; /// A hint that a harness is likely next, so acpmux can warm its adapter (cmux-tui acpmux lane). export const PREWARM_METHOD = "_acpmux/prewarm"; @@ -1745,6 +1747,7 @@ export class AcpmuxDirectClient { private sessionEntriesOf?: { sessions: Session[]; entries: AcpmuxSessionEntry[] }; /// The snapshot summary's `usageStats`, kept while acpmux's numbers are the same (sameUsageStats). private summaryUsage?: SessionUsageStats; + private publishedSummaryOf?: { summary: unknown; usage: unknown; value: AcpmuxSnapshot["summary"] }; /// The usage read in flight (refreshUsage). private usageRead?: Promise; private selectedSessionId?: string; @@ -3330,9 +3333,11 @@ export class AcpmuxDirectClient { } private receive(raw: string, stale = false): void { - this.wire.received(raw); // A frame that is not a JSON object is dropped like one that is not JSON (`null` would throw at `in`). - const message = parseJsonObjectFrame(raw) as Reply | Notification | null; + // Parsed once: the wire log takes the parsed frame. + const parsed = parseJsonObjectFrame(raw); + this.wire.received(raw, parsed); + const message = parsed as Reply | Notification | null; if (!message) return; if ("id" in message && (typeof message.id === "number" || typeof message.id === "string")) { const request = this.pending.get(message.id); @@ -6738,6 +6743,73 @@ export class AcpmuxDirectClient { return entries; } + /// The snapshot's `summary`: the same object while the chat's summary and usage are the same + /// objects (every change assigns a new one; `effort` and `limits` are read from the summary), so + /// the pane compares it by identity. A streamed chunk changes neither (HOT PATH AUDIT area F, + /// row W1; review of #34004: comparing it by its JSON cost O(summary) per chunk). + private publishedSummary( + summary: Record | undefined, + effort: any, + limits: ReturnType, + ): AcpmuxSnapshot["summary"] { + const kept = this.publishedSummaryOf; + if (kept && kept.summary === summary && kept.usage === this.usage) return kept.value; + const value: AcpmuxSnapshot["summary"] = summary + ? { + sessionId: summary.sessionId, + cwd: summary.cwd, + turnCount: summary.turnCount, + usage: this.usage, + contextBreakdown: contextBreakdownOf(summary.contextBreakdown), + usageStats: (this.summaryUsage = sameUsageStats(summary.usageStats, this.summaryUsage)), + host: text(summary.host), + machine: text(summary.machine), + peer: text(summary.peer), + hostKind: hostKind(summary.hostKind), + // acpmux's session detail names the folder's branch `gitBranch` (when the session started). + branch: text(summary.branch) ?? text(summary.gitBranch), + worktree: text(summary.worktree), + title: summary.title, + name: summary.name, + goal: taskGoal(summary.goal), + harness: summary.harness, + family: typeof summary.family === "string" ? summary.family : undefined, + agentInfo: + summary.agentInfo && typeof summary.agentInfo === "object" && !Array.isArray(summary.agentInfo) + ? summary.agentInfo + : undefined, + authSource: text(summary.authSource), + baseUrl: text(summary.baseUrl), + route: text(summary.route), + settingsSources: summary.settingsSources, + usageLimits: usageLimitsOf(summary.usageLimits) ?? text(summary.usageLimits), + ...(summary.usageLimitsApply === false ? { usageLimitsApply: false } : {}), + // acpmux reports `model: null` for a harness with no model (one known only from its config). + model: text(summary.model), + effort: effort?.currentValue, + // `audio` only while the picked model hears it (takesAudio). + promptCapabilities: summary.agentCapabilities?.promptCapabilities && { + ...summary.agentCapabilities.promptCapabilities, + audio: takesAudio(summary.agentCapabilities.promptCapabilities, summary.configOptions), + }, + steering: summary.steering === true, + backgroundsWork: summary.backgroundsWork === true, + queuePaused: summary.queuePaused === true, + ...(text(summary.watching) ? { watching: text(summary.watching) } : {}), + ...(Array.isArray(summary.mcpServers) ? { mcpServers: summary.mcpServers } : {}), + ...(limits ? { subsessionLimits: limits } : {}), + status: summary.status, + enforcement: sessionEnforcement(summary.enforcement), + modes: summary.modes, + configOptions: summary.configOptions, + tools: chatTools(summary.tools), + toolsMutable: summary.toolsMutable === true, + } + : undefined; + this.publishedSummaryOf = { summary, usage: this.usage, value }; + return value; + } + private emitNow(connection: string): void { if (this.closed && !this.replayMode && !(this.fedMode && !this.disposed)) return; if (this.snapshotHold) { @@ -6786,58 +6858,7 @@ export class AcpmuxDirectClient { peerStates: this.peerStates, peerResources: this.peerResources, ...(this.forkedAtSeq !== undefined ? { forkedAtSeq: this.forkedAtSeq } : {}), - summary: summary - ? { - sessionId: summary.sessionId, - cwd: summary.cwd, - turnCount: summary.turnCount, - usage: this.usage, - contextBreakdown: contextBreakdownOf(summary.contextBreakdown), - usageStats: (this.summaryUsage = sameUsageStats(summary.usageStats, this.summaryUsage)), - host: text(summary.host), - machine: text(summary.machine), - peer: text(summary.peer), - hostKind: hostKind(summary.hostKind), - // acpmux's session detail names the folder's branch `gitBranch` (when the session started). - branch: text(summary.branch) ?? text(summary.gitBranch), - worktree: text(summary.worktree), - title: summary.title, - name: summary.name, - goal: taskGoal(summary.goal), - harness: summary.harness, - family: typeof summary.family === "string" ? summary.family : undefined, - agentInfo: - summary.agentInfo && typeof summary.agentInfo === "object" && !Array.isArray(summary.agentInfo) - ? summary.agentInfo - : undefined, - authSource: text(summary.authSource), - baseUrl: text(summary.baseUrl), - route: text(summary.route), - settingsSources: summary.settingsSources, - usageLimits: usageLimitsOf(summary.usageLimits) ?? text(summary.usageLimits), - ...(summary.usageLimitsApply === false ? { usageLimitsApply: false } : {}), - // acpmux reports `model: null` for a harness with no model (one known only from its config). - model: text(summary.model), - effort: effort?.currentValue, - // `audio` only while the picked model hears it (takesAudio). - promptCapabilities: summary.agentCapabilities?.promptCapabilities && { - ...summary.agentCapabilities.promptCapabilities, - audio: takesAudio(summary.agentCapabilities.promptCapabilities, summary.configOptions), - }, - steering: summary.steering === true, - backgroundsWork: summary.backgroundsWork === true, - queuePaused: summary.queuePaused === true, - ...(text(summary.watching) ? { watching: text(summary.watching) } : {}), - ...(Array.isArray(summary.mcpServers) ? { mcpServers: summary.mcpServers } : {}), - ...(limits ? { subsessionLimits: limits } : {}), - status: summary.status, - enforcement: sessionEnforcement(summary.enforcement), - modes: summary.modes, - configOptions: summary.configOptions, - tools: chatTools(summary.tools), - toolsMutable: summary.toolsMutable === true, - } - : undefined, + summary: this.publishedSummary(summary, effort, limits), connection, origin: this.origin, ...(this.listedSessions ? { listedSessions: true as const } : {}), @@ -6888,7 +6909,7 @@ export class AcpmuxDirectClient { : {}), secret: this.pendingSecret, asks: this.asks.list(), - catalog: [], + catalog: NO_CATALOG, commands: this.commands, canLoadOlder: this.seqHistoryLeft() || this.nativeBefore() !== undefined, ...(this.shownEarlierPrompts().length ? { earlierPrompts: this.shownEarlierPrompts() } : {}), diff --git a/webviews/src/agent-session/acpmux/loadDebug.ts b/webviews/src/agent-session/acpmux/loadDebug.ts index c4ec098b05cf..92ef4a89de8f 100644 --- a/webviews/src/agent-session/acpmux/loadDebug.ts +++ b/webviews/src/agent-session/acpmux/loadDebug.ts @@ -124,6 +124,7 @@ export async function loadRun( // A page still busy from the seed stretches the warmup frames; no display here is slower than 60 Hz. const nominal = Math.min(1000 / 60, Math.max(1, median(warmup.slice(1).map((time, index) => time - warmup[index]!)))); acpmuxPerf.resetFrames(); + acpmuxPerf.resetRenders(); const streaming = mock.streamLoadTurn(session, { seconds, chunkChars: Math.max(1, Math.floor(options.chunk_chars ?? 32)), @@ -136,6 +137,7 @@ export async function loadRun( const work: number[] = []; const late: number[] = []; let blank = 0; + let liveLineShown = 0; // The scroll: up from the bottom at `speed` until `depth` viewports above it, then back down to // the bottom, again and again, while the reply streams in below. const span = depth * scroller.clientHeight; @@ -159,6 +161,8 @@ export async function loadRun( } previous = now; if (!streamDone && requested()) scroller.scrollTop = scroller.scrollHeight - scroller.clientHeight - offset; + // The live line under a reply whose text streams stays quiet (Thinking.tsx `quietKey`). + if (!streamDone && document.querySelector(".cv-worked--live:not(.is-quiet)")) liveLineShown += 1; const blankNow = isBlank(acpmuxPerf.mounted, scroller.scrollTop, scroller.clientHeight, scroller.scrollHeight); if (blankNow) blank += 1; acpmuxPerf.markFrame(now, blankNow); @@ -167,6 +171,9 @@ export async function loadRun( if (streamDone) tail += 1; } const streamed = await streaming; + // Renders per component over the run (`acpmuxPerf.render`): the pane root, the composer and + // each row by id (`row:`), so a caller can tell what a streamed chunk re-rendered. + const renders = Object.fromEntries(acpmuxPerf.renders); const sorted = [...work].sort((a, b) => a - b); const stats = acpmuxPerf.stats(false) as Record; // The longest frames, in order, with what they spent where. @@ -199,10 +206,12 @@ export async function loadRun( over_60hz_budget: sorted.filter((ms) => ms > 1000 / 60).length, frames: work.length, blank_frames: blank, + live_line_shown_frames: liveLineShown, layout_ms: stats.layout, react_ms: stats.react, other_ms: stats.other, spans: stats.spans, + renders, longest, }; } diff --git a/webviews/src/agent-session/acpmux/perf.ts b/webviews/src/agent-session/acpmux/perf.ts index 5e14bead5bcd..1f8fac6352fe 100644 --- a/webviews/src/agent-session/acpmux/perf.ts +++ b/webviews/src/agent-session/acpmux/perf.ts @@ -225,6 +225,19 @@ export class AcpmuxPerf { } /** Adds `ms` to the named span of the current frame (client reduce, row derivation, a flush). */ + /** Renders per component since `resetRenders` (tests: a streamed chunk renders only its row). */ + readonly renders = new Map(); + + /** Counts one render of `name` while measuring. */ + render(name: string): void { + if (!this.enabled) return; + this.renders.set(name, (this.renders.get(name) ?? 0) + 1); + } + + resetRenders(): void { + this.renders.clear(); + } + span(name: string, ms: number): void { if (!this.enabled) return; this.spansSinceMark.set(name, (this.spansSinceMark.get(name) ?? 0) + ms); diff --git a/webviews/src/agent-session/acpmux/streamingRow.ts b/webviews/src/agent-session/acpmux/streamingRow.ts new file mode 100644 index 000000000000..9484dfaf4cdf --- /dev/null +++ b/webviews/src/agent-session/acpmux/streamingRow.ts @@ -0,0 +1,121 @@ +// The streaming reply's row as its own store (HOT PATH AUDIT area F, row W1, +// handoff/hot-path-audit/stream/AUDIT.md). A streamed chunk changes one thing: the text of the +// reply being written. Drawn through the pane's snapshot, each chunk re-rendered the whole pane +// (AcpmuxPane: ~20 memos over every row), the composer and Session Info, 5-11 ms of React per +// frame with 2,000 rows. Chrome's model: a change repaints what it damages. +// +// When a snapshot differs from the one the pane drew only in that row's text (`textOnly`), the +// pane keeps its snapshot and the row's newer value goes here; the transcript reads it +// (`useStreamingRow`) and draws that one row. Any other change goes through the pane as before and +// clears this store in the same turn, so the pane's own rows are the truth again. The store holds +// one value; nothing else owns it. +import { useSyncExternalStore } from "react"; +import type { AcpmuxRow } from "./model"; + +/// The reply's raw row (the client's), how many chunks it is ahead of the row the pane drew, and +/// that drawn row's text. +export type StreamingRow = { + readonly row: AcpmuxRow; + readonly ahead: number; + readonly drawnText: string; +}; + +let current: StreamingRow | undefined; +const listeners = new Set<() => void>(); + +function publish(next: StreamingRow | undefined): void { + if (current === next) return; + current = next; + for (const listener of listeners) listener(); +} + +export const streamingRow = { + subscribe(listener: () => void): () => void { + listeners.add(listener); + return () => void listeners.delete(listener); + }, + get: (): StreamingRow | undefined => current, + /// The streaming row moved on without the pane: `drawn` is the raw row the pane last drew. + set(row: AcpmuxRow, drawn: AcpmuxRow): void { + publish({ row, ahead: row.version - drawn.version, drawnText: drawn.text ?? "" }); + }, + /// The pane draws a new snapshot: its rows are the truth again. + clear(): void { + publish(undefined); + }, +}; + +/// The text the transcript last drew for the streaming reply, by row id (`drew`): the pane's +/// transcript shapes rows (conversation/turns.ts), so the fast path is only taken while the reply's +/// drawn row is the client's row as it is (its text unchanged by the shaping). +let drawn: { id: string; text: string } | undefined; + +export function drewStreamingRow(id: string | undefined, text: string | undefined): void { + drawn = id === undefined || text === undefined ? undefined : { id, text }; +} + +export function drawsAsIs(row: AcpmuxRow): boolean { + return drawn !== undefined && drawn.id === row.id && drawn.text === row.text; +} + +const none = (): undefined => undefined; +const noSubscribe = (): (() => void) => () => {}; + +/// The streaming row the transcript draws ahead of the pane's snapshot, if any. Only the pane's +/// live transcript (`active`) subscribes; a replay, guest or other transcript never re-renders for it. +export function useStreamingRow(active: boolean): StreamingRow | undefined { + return useSyncExternalStore( + active ? streamingRow.subscribe : noSubscribe, + active ? streamingRow.get : none, + active ? streamingRow.get : none, + ); +} + +/// Whether the pane may draw a streamed chunk through the row alone: false while a view that reads +/// the reply's text from the pane's snapshot is open (an artifact following an open fence, Find), +/// so that view stays exact (review of #34004). +const blockedBy = new Set(); + +export function setStreamingRowBlocked(blocked: boolean, by: string): void { + if (blocked) blockedBy.add(by); + else blockedBy.delete(by); +} + +export function streamingRowBlocked(): boolean { + return blockedBy.size > 0; +} + +/// Every own field of the two rows is the same, except `text` and `version`. +function sameButText(previous: AcpmuxRow, next: AcpmuxRow): boolean { + const before = previous as unknown as Record; + const after = next as unknown as Record; + const keys = Object.keys(after); + if (keys.length !== Object.keys(before).length) return false; + return keys.every((key) => key === "text" || key === "version" || Object.is(before[key], after[key])); +} + +/// The index of the one row `next` changes from `previous` when the change is only a streaming +/// reply's text growing, else -1. Unchanged rows keep their objects (direct.ts `OrderedRows`), so +/// the scan is one pointer compare per row. +export function textOnlyRow(previous: readonly AcpmuxRow[], next: readonly AcpmuxRow[]): number { + if (previous === next || previous.length !== next.length) return -1; + let changed = -1; + for (let index = next.length - 1; index >= 0; index -= 1) { + const before = previous[index]!; + const after = next[index]!; + if (before === after) continue; + if (changed >= 0) return -1; + if ( + after.kind !== "assistant" || + after.streaming !== true || + before.id !== after.id || + typeof before.text !== "string" || + typeof after.text !== "string" || + !after.text.startsWith(before.text) || + !sameButText(before, after) + ) + return -1; + changed = index; + } + return changed; +} diff --git a/webviews/src/agent-session/acpmux/subagents/subagentFold.ts b/webviews/src/agent-session/acpmux/subagents/subagentFold.ts index b148cbdff7a5..bdad1d0461ad 100644 --- a/webviews/src/agent-session/acpmux/subagents/subagentFold.ts +++ b/webviews/src/agent-session/acpmux/subagents/subagentFold.ts @@ -885,9 +885,19 @@ export class SubagentFold { markTranscriptSummaryVersion(drawn.rows, revisions.summary); views[id] = drawn.rows; } + // The same record while every view is the same array, so a snapshot that changes no subagent + // compares equal by identity (HOT PATH AUDIT area F, row W1). + const kept = this.publishedViews; + if (kept) { + const ids = Object.keys(views); + if (ids.length === Object.keys(kept).length && ids.every((id) => kept[id] === views[id])) return kept; + } + this.publishedViews = views; return views; } + private publishedViews: Record | undefined; + private record(id: string, entry: ChildEntry): void { const entries = this.own.get(id) ?? []; entries.push(entry); diff --git a/webviews/src/agent-session/acpmux/wire.ts b/webviews/src/agent-session/acpmux/wire.ts index c41f7bea274f..e398386fc76d 100644 --- a/webviews/src/agent-session/acpmux/wire.ts +++ b/webviews/src/agent-session/acpmux/wire.ts @@ -137,12 +137,20 @@ export class AcpWireLog { this.push({ dir: "out", kind: id === undefined ? "notification" : "request", method, id, ...this.body(logged) }); } - /** A message acpmux sent. */ - received(text: string): void { + /** + * A message acpmux sent. `parsed`: the frame as its receiver already parsed it (null when it is + * not a JSON object), so a frame is parsed once (HOT PATH AUDIT area F, row W2). + */ + received(text: string, parsed?: Record | null): void { let message: any; - try { - message = JSON.parse(text); - } catch { + if (parsed !== undefined) message = parsed; + else + try { + message = JSON.parse(text); + } catch { + message = null; + } + if (message === null || typeof message !== "object") { this.push({ dir: "in", kind: "invalid", ...this.body(text) }); return; } diff --git a/webviews/test/agent-pane-session-change-coalesce.test.ts b/webviews/test/agent-pane-session-change-coalesce.test.ts index 9ac051821f61..2163b4b462ea 100644 --- a/webviews/test/agent-pane-session-change-coalesce.test.ts +++ b/webviews/test/agent-pane-session-change-coalesce.test.ts @@ -427,3 +427,46 @@ test("bench: client time for 30 running chats changing 20 times (600 changes)", client.close(); } }); + +// Each frame from acpmux is parsed once (HOT PATH AUDIT area F, row W2, +// handoff/hot-path-audit/stream/AUDIT.md): the wire log and the client read one parse. Before, a +// streamed chunk's frame was parsed twice, once for the log and once for the client. +test("a streamed chunk's frame is parsed once", async () => { + const { socket, client } = await openPane(); + const parse = JSON.parse; + let parses = 0; + const frames = 50; + try { + const lines = Array.from({ length: frames }, (_, n) => + JSON.stringify({ + jsonrpc: "2.0", + method: "_acpmux/event", + params: { + sessionId: SELECTED, + seq: 1000 + n, + at: 1000 + n, + dir: "in", + kind: "agent_message_chunk", + msg: { + method: "session/update", + params: { + sessionId: SELECTED, + update: { sessionUpdate: "agent_message_chunk", content: { type: "text", text: `word${n} ` } }, + }, + }, + }, + }), + ); + JSON.parse = ((text: string, reviver?: (this: unknown, key: string, value: unknown) => unknown) => { + parses += 1; + return parse(text, reviver); + }) as typeof JSON.parse; + for (const data of lines) socket.onmessage?.({ data }); + JSON.parse = parse; + flushFrames(); + expect(parses).toBe(frames); + } finally { + JSON.parse = parse; + client.close(); + } +}); diff --git a/webviews/test/agent-pane-stream-renders.test.ts b/webviews/test/agent-pane-stream-renders.test.ts new file mode 100644 index 000000000000..16081c5b7030 --- /dev/null +++ b/webviews/test/agent-pane-stream-renders.test.ts @@ -0,0 +1,87 @@ +// cmux-web-test-timeout: 300 +// l10n-allow-file: the mock daemon's fixture turns, not shipped UI. +// A streamed chunk of the reply re-renders the reply's row and nothing else (HOT PATH AUDIT area F, +// row W1, handoff/hot-path-audit/stream/AUDIT.md). Measured on the pane with a 2,000-row chat +// (Chrome 156, production React): every chunk re-rendered the whole pane (AcpmuxPane, ~20 memos over +// all rows), the composer and Session Info, 5-11 ms of React per frame, frames dropped at 2,000 rows. +// +// The real pane (`?mock`: the real direct client against the in-page mock daemon), a chat of 700 +// heavy turns seeded through attach (`cmuxAcpmuxDebug.loadSeed`), then one text-only turn streamed +// at full speed (`loadRun`, no tool calls, no scrolling). Over the stream the pane root and the +// composer render no more often than the turn's own structural changes (its prompt, its start and +// its end), however many chunks land, and the reply's row renders once per drawn chunk. +import { afterAll, beforeAll, expect, setDefaultTimeout, test } from "bun:test"; +import path from "node:path"; +import { chromium, type Browser } from "playwright"; +import { requireBrowserLane } from "./support/requireBrowserLane"; +import { warmViteServer } from "./support/warmDevServer"; + +await requireBrowserLane("agent-pane-stream-renders.test.ts", async () => { + setDefaultTimeout(280_000); + const { createServer } = await import("vite"); + const webviews = path.resolve(import.meta.dir, ".."); + let vite: Awaited> | undefined; + let browser: Browser | undefined; + beforeAll(async () => { + vite = await createServer({ + configFile: path.join(webviews, "vite.config.acpmux-pane.mjs"), + server: { port: 0, host: "127.0.0.1", strictPort: false }, + logLevel: "error", + }); + await vite.listen(); + await warmViteServer(vite, "/?mock"); + browser = await chromium.launch({ headless: true }); + }); + afterAll(async () => { + await browser?.close(); + await vite?.close(); + }); + + /// Structural changes a streamed turn makes besides its chunks: its prompt, its start, its end, + /// the summary's status in and out, and the frame its reply's row first appears. Renders past + /// these are renders the chunks caused. + const STRUCTURAL = 12; + + test("a streamed chunk re-renders only its row: not the pane, not the composer", async () => { + const page = await browser!.newPage({ viewport: { width: 900, height: 760 } }); + const { port } = vite!.httpServer!.address() as { port: number }; + await page.goto(`http://127.0.0.1:${port}/?mock`); + await page.waitForFunction(() => "cmuxAcpmuxDebug" in window, null, { timeout: 60_000 }); + const seeded = (await page.evaluate(() => (window as any).cmuxAcpmuxDebug.loadSeed(700))) as Record; + expect(seeded.error).toBeUndefined(); + const run = (await page.evaluate(() => + (window as any).cmuxAcpmuxDebug.loadRun({ + seconds: 4, + chunk_chars: 24, + chunk_ms: 12, + tool_every: 0, + scroll_speed: 0, + }), + )) as Record; + await page.close(); + const renders = (run.renders ?? {}) as Record; + const rowRenders = Object.entries(renders).filter(([name]) => name.startsWith("row:")); + const busiest = rowRenders.sort((a, b) => b[1] - a[1])[0] ?? ["none", 0]; + const report = { + rows: run.rows, + deltas: run.deltas, + frames: run.frames, + pane: renders.pane ?? 0, + composer: renders.composer ?? 0, + rowsRendered: rowRenders.length, + streamingRow: busiest, + liveLineShown: run.live_line_shown_frames, + reactMs: run.react_ms, + workMs: run.work_ms, + }; + console.log(`STREAM-RENDERS ${JSON.stringify(report)}`); + expect(run.deltas).toBeGreaterThan(100); + // The reply's row drew the stream. + expect(busiest[1]).toBeGreaterThan(20); + expect(report.pane, "pane root renders during a text-only stream").toBeLessThanOrEqual(STRUCTURAL); + expect(report.composer, "composer renders during a text-only stream").toBeLessThanOrEqual(STRUCTURAL); + // The reply's text moves on every few frames, so the live line under it never shows while it + // streams (review of #34004: the line came back once the pane stopped re-rendering per chunk). + expect(report.liveLineShown, "frames with the live line shown while the reply streams").toBe(0); + }); +});