From 1798786d4e846e23582918aae645963a8ffed792 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 10 Sep 2026 21:52:21 -0700 Subject: [PATCH] perf(native-chat): mount only the transcript rows near the viewport (#19869) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(native-chat): share one row-content derivation between row and list Windowing needs the list and the row to agree on which messages draw nothing: a row the list counts but the row declines to render would reserve estimated height for an empty slot. Extracts the block derivation out of NativeChatMessageRow into a module cached on the block array, so a streaming turn pays for it once per revision rather than once per consumer. * refactor(native-chat): keep an opened tool run open past its row's lifetime A tool run, tool line or diff card the reader opened is state they created, but it lives in the component's own `useState`. That is fine while every row is mounted forever. It stops being fine the moment rows can be unmounted: the run silently re-collapses behind the reader's back. Rows now read their disclosure from a transcript-level map when one is provided and fall back to their own state when they are rendered standalone. The controls that re-sync a run — the toolbar's expand-all, a turn's disclosure, a diff reveal — are folded into the key the choice is remembered under, so a control flip reads as "nothing recorded yet" and the new default stands without a mid-render write to a map an ancestor owns. `ToolLine` moves to its own file; the run was over the line cap with it. * perf(native-chat): mount only the transcript rows near the viewport A settled transcript mounts every row it has ever loaded, so the cost of opening a conversation grows with its length even though only a screenful is legible. Rows near the viewport are now the only ones in the document; the rest are reserved as estimated height and measured when they arrive. Four things had to change for that to be safe: - `zoom` moves from the transcript column onto the scroll container. Item measurements are in the zoomed content's pixels while `scrollTop` is not, so with the two split across the boundary the window's arithmetic was off by exactly the font scale — correct at the top of a transcript and blank deep inside it. The column's padding moves to a new inner element to keep the layout it had. This does mean the scrollbar itself zooms with the text. - The three siblings that made up a row — the message, the turn status, the turn's diff rollup — move into one wrapper that carries the spacing they used to take from the column. The spacing between rows is the window's `gap`, never the height estimate, which would otherwise be counted twice. - Messages that draw nothing no longer take a slot. Counted but undrawn, each one would reserve estimated height for a row that never appears. - Paging in older history is driven by scroll events alone. Every row that resolves its real height moves the content and re-fires the size observers, so the old "am I near the top?" test would have asked for another page once per measurement. It now also requires the view to have moved upwards and requires new items since the last request. Anchoring is the virtualizer's: `anchorTo: 'end'` re-resolves the row at the current offset across a count change, which replaces the hand-rolled prepend anchor, and `followOnAppend` keeps a reader at the bottom pinned there. The document-level bottom pin stays, because the typing indicator, the activity line and the column's end padding all live past the last row. Revealing a diff from a turn rollup can target a row that isn't mounted, so that row is pinned into the window and the card still reports its own position — a turn that touched four files lands on the one that was asked for. * fix(native-chat): let a pinned row reach the mounted window Two faults the windowing tests turned up, plus the handles they needed. The virtualizer memoizes its mounted index list on the range extractor's identity. Holding that identity stable — which is right for the measurement memo, and was the reason it was written that way — meant a row pinned after the fact was never picked up: revealing a diff in a row the window had left behind pointed at a row that stayed unmounted. The extractor now changes identity with the pinned set, which is not a dependency of the measurement memo, so nothing expensive is rebuilt. The offset a row sits at is read off the `offsetParent` chain, with a rect-based fallback for the case where there is none. Using that fallback for the window's own scroll margin was wrong in kind: with no layout to measure, it returns the scroll position itself, so the margin tracked the offset and the window sat at the top of the transcript wherever the reader scrolled. The margin now takes the offset chain or nothing; the fallback stays where it belongs, on the reveal. The scroll root and the window's spacer are named, so measurement can find the scroll root without depending on which utility class makes it scroll, and so a test can tell a window from a whole transcript. * test(native-chat): cover the windowed transcript, and prove the window engaged The integration harness stubs `offsetHeight` — on the scroll root and on every row — because that is what the virtualizer measures with, and a DOM without layout answers zero to all of it. Rows report the height their own estimate predicted, which keeps the reserved totals exact no matter which rows have been mounted long enough to be measured. Every case reads the window through one helper that refuses to pass when there is no window. Without that, raising the usability gate would send all of them down the whole-transcript path, where "fewer rows mounted than messages" is false but every other assertion still holds — and they would go on reporting green while covering nothing. Reserved height is asserted as an exact total rather than "greater than zero", which a degenerate empty window also satisfies, and the mounted range is asserted to bracket the offset rather than merely to be smaller than the transcript. Covered: the window mounts a subset and moves with the reader; the newest row and a reveal's target stay mounted from outside it; an opened tool run is still open when its row comes back; a message that draws nothing takes no slot; and the scroll root with no usable height still renders every row as a direct child of the transcript column. What the environment cannot show is stated where it matters rather than faked: its ResizeObserver never fires and a scroll assignment emits no event, so measurement settling, the bottom pin under a streaming turn, prepend anchoring and smooth scrolling are covered as pure decisions — height estimation, the pinned set, range extraction, and whether a position should page in older history — and left to a real renderer as behaviour. * docs(native-chat): say that one offset path does read rects * test(native-chat): pin the window against a row that grows in place Whole-message appends were covered; a row being replaced by a taller version of itself — what a streaming reply is — was not. The existing windowing harness gains two things it needs to see that: a scroll root with a real document (a height, a viewport, and a scrollTop that clamps), and a resize observer that delivers when a target's height actually changed, since happy-dom's never fires and nothing re-measures without it. Frame by frame, while one row grows from 24px to 6358px: the view stays 0px from the bottom, the row stays mounted, and the reserved total tracks the measurement rather than the estimate. A reader who scrolls up mid growth keeps the exact offset they chose for the rest of it. * test(native-chat): guard history prepend anchoring * test(native-chat): strengthen prepend anchor contract * fix(native-chat): preserve provider tool call identity * fix(native-chat): harden transcript windowing lifecycle * test(native-chat): install virtualizer viewport for turn timing * fix(native-chat): reject blank tool call identities --------- Co-authored-by: Merge Sim --- .../claude-structured-item-translation.ts | 1 + ...ude-structured-journal-translation.test.ts | 3 + .../codex-structured-item-translation.test.ts | 13 + .../codex-structured-item-translation.ts | 4 + .../transcript-line-decoders-codex.ts | 5 +- ...anscript-reader-codex-history-mode.test.ts | 2 +- .../native-chat/transcript-reader.test.ts | 5 +- .../native-chat/transcript-record-blocks.ts | 3 +- .../native-chat/NativeChatDiffCard.tsx | 19 +- ...hatMessageList.stream-render.perf.test.tsx | 8 +- ...eChatMessageList.task-list-frames.test.tsx | 8 +- .../NativeChatMessageList.test.tsx | 8 +- ...eChatMessageList.tool-stream-cost.test.tsx | 8 +- .../native-chat/NativeChatMessageList.tsx | 383 +++++------ ...ativeChatMessageList.turn-history.test.tsx | 8 +- ...NativeChatMessageList.turn-timing.test.tsx | 11 +- .../NativeChatMessageList.windowing.test.tsx | 640 ++++++++++++++++++ .../native-chat/NativeChatMessageRow.tsx | 48 +- .../native-chat/NativeChatToolLine.tsx | 139 ++++ .../NativeChatToolRun.identity.test.tsx | 109 ++- .../native-chat/NativeChatToolRun.tsx | 193 ++---- .../native-chat/NativeChatTranscriptItems.tsx | 50 ++ .../native-chat/NativeChatTranscriptRow.tsx | 86 +++ .../native-chat-autoscroll.test.ts | 45 ++ .../native-chat/native-chat-autoscroll.ts | 35 + .../native-chat-disclosure-store.ts | 71 ++ .../native-chat-message-list-test-viewport.ts | 26 + .../native-chat-pinned-rows.test.ts | 46 ++ .../native-chat/native-chat-pinned-rows.ts | 57 ++ .../native-chat/native-chat-row-content.ts | 61 ++ .../native-chat-row-height-estimate.test.ts | 147 ++++ .../native-chat-row-height-estimate.ts | 126 ++++ .../native-chat-transcript-slots.test.ts | 111 +++ .../native-chat-transcript-slots.ts | 119 ++++ .../use-native-chat-transcript-scroll.ts | 145 ++++ ...ve-chat-transcript-window.options.test.tsx | 121 ++++ .../use-native-chat-transcript-window.ts | 230 +++++++ .../agent-session-journal-schemas.test.ts | 23 +- src/shared/agent-session-journal-schemas.ts | 7 + src/shared/agent-session-journal-types.ts | 2 + src/shared/native-chat-types.ts | 2 + ...tructured-agent-session-projection.test.ts | 1 + .../structured-agent-session-projection.ts | 1 + ...native-chat-history-prepend-anchor.spec.ts | 204 ++++++ 44 files changed, 2912 insertions(+), 422 deletions(-) create mode 100644 src/renderer/src/components/native-chat/NativeChatMessageList.windowing.test.tsx create mode 100644 src/renderer/src/components/native-chat/NativeChatToolLine.tsx create mode 100644 src/renderer/src/components/native-chat/NativeChatTranscriptItems.tsx create mode 100644 src/renderer/src/components/native-chat/NativeChatTranscriptRow.tsx create mode 100644 src/renderer/src/components/native-chat/native-chat-disclosure-store.ts create mode 100644 src/renderer/src/components/native-chat/native-chat-message-list-test-viewport.ts create mode 100644 src/renderer/src/components/native-chat/native-chat-pinned-rows.test.ts create mode 100644 src/renderer/src/components/native-chat/native-chat-pinned-rows.ts create mode 100644 src/renderer/src/components/native-chat/native-chat-row-content.ts create mode 100644 src/renderer/src/components/native-chat/native-chat-row-height-estimate.test.ts create mode 100644 src/renderer/src/components/native-chat/native-chat-row-height-estimate.ts create mode 100644 src/renderer/src/components/native-chat/native-chat-transcript-slots.test.ts create mode 100644 src/renderer/src/components/native-chat/native-chat-transcript-slots.ts create mode 100644 src/renderer/src/components/native-chat/use-native-chat-transcript-scroll.ts create mode 100644 src/renderer/src/components/native-chat/use-native-chat-transcript-window.options.test.tsx create mode 100644 src/renderer/src/components/native-chat/use-native-chat-transcript-window.ts create mode 100644 tests/e2e/native-chat-history-prepend-anchor.spec.ts diff --git a/src/main/claude/claude-structured-item-translation.ts b/src/main/claude/claude-structured-item-translation.ts index 1c1673b59cd..0fdb231f6ef 100644 --- a/src/main/claude/claude-structured-item-translation.ts +++ b/src/main/claude/claude-structured-item-translation.ts @@ -179,6 +179,7 @@ export function claudeToolBody(input: { kind: 'tool-call', name: input.tool.name, input: input.tool.input, + callId: input.tool.id, state: input.result ? (input.result.failed ? 'failed' : 'completed') : 'running', ...(input.result ? { output: boundInlineText(input.result.output, DEFAULT_JOURNAL_PAYLOAD_LIMITS).bounded } diff --git a/src/main/claude/claude-structured-journal-translation.test.ts b/src/main/claude/claude-structured-journal-translation.test.ts index 050fccc42b6..44d7e3a1251 100644 --- a/src/main/claude/claude-structured-journal-translation.test.ts +++ b/src/main/claude/claude-structured-journal-translation.test.ts @@ -528,6 +528,7 @@ describe('Claude structured journal translation', () => { expect(keyed.get('orca:claude-tool%3Aclaude-session%3Atool-1')).toMatchObject({ kind: 'tool-call', name: 'Bash', + callId: 'tool-1', state: 'completed', output: { head: 'a.ts\nb.ts', truncated: false } }) @@ -546,6 +547,7 @@ describe('Claude structured journal translation', () => { expect(state.items.at(-1)?.body).toMatchObject({ kind: 'tool-call', name: 'tool', + callId: 'tool-1', input: null, output: { head: 'done again' } }) @@ -606,6 +608,7 @@ describe('Claude structured journal translation', () => { ]) expect(state.items[0]?.body).toMatchObject({ kind: 'tool-call', + callId: 'tool-1', state: 'completed', output: { head: 'done' } }) diff --git a/src/main/codex/codex-structured-item-translation.test.ts b/src/main/codex/codex-structured-item-translation.test.ts index 201df58bc90..3f8ec524002 100644 --- a/src/main/codex/codex-structured-item-translation.test.ts +++ b/src/main/codex/codex-structured-item-translation.test.ts @@ -201,6 +201,7 @@ describe('codex item bodies', () => { expect(codexItemBody(LIVE_TURN[2] as CodexThreadItem)).toEqual({ kind: 'tool-call', name: 'shell', + callId: 'item-2', input: { command: 'ls', cwd: '/tmp' }, exitCode: 0, state: 'completed', @@ -229,6 +230,7 @@ describe('codex item bodies', () => { expect(body).toEqual({ kind: 'tool-call', name: 'read', + callId: 'item-read', // `name` is the target's basename, which `path` already carries and no // label ever reads, so it stays out of the bounded journal payload. input: { command: "sed -n '1,200p' notes.txt", cwd: '/repo', path: '/repo/notes.txt' }, @@ -257,6 +259,7 @@ describe('codex item bodies', () => { ).toEqual({ kind: 'tool-call', name: 'search', + callId: 'item-search', input: { command: 'rg -n --no-heading beta .', cwd: '/repo', query: 'beta', directory: '.' }, state: 'running' }) @@ -276,6 +279,7 @@ describe('codex item bodies', () => { ).toEqual({ kind: 'tool-call', name: 'search', + callId: 'item-search-bare', input: { command: 'rg beta', cwd: '/repo' }, exitCode: 0, state: 'completed' @@ -296,6 +300,7 @@ describe('codex item bodies', () => { expect(body).toEqual({ kind: 'tool-call', name: 'list', + callId: 'item-list', input: { command: 'ls', cwd: '/repo' }, exitCode: 0, state: 'completed' @@ -326,6 +331,7 @@ describe('codex item bodies', () => { ).toEqual({ kind: 'tool-call', name: 'shell', + callId: 'item-mixed', input: { command: 'cat a.txt && ls src', cwd: '/repo' }, exitCode: 0, state: 'completed' @@ -349,6 +355,7 @@ describe('codex item bodies', () => { ).toEqual({ kind: 'tool-call', name: 'read', + callId: 'item-two-reads', input: { command: 'cat a.ts && cat b.ts', cwd: '/repo' }, exitCode: 0, state: 'completed' @@ -424,6 +431,7 @@ describe('codex item bodies', () => { ).toEqual({ kind: 'tool-call', name: 'read', + callId: 'item-read-null', input: { command: 'cat', cwd: '/repo' }, exitCode: 0, state: 'completed' @@ -451,6 +459,7 @@ describe('codex item bodies', () => { const shellRow = { kind: 'tool-call', name: 'shell', + callId: 'item-fallback', input: { command: 'ls', cwd: '/tmp' }, exitCode: 0, state: 'completed' @@ -624,6 +633,7 @@ describe('codex item bodies', () => { // Server-qualified, and the arguments stay top level so the row label can // read `query`/`command`/`file_path` out of them. name: 'weather/get_forecast', + callId: 'mcp-1', mcpIdentity: { server: 'weather', tool: 'get_forecast' }, input: { city: 'Oslo' }, state: 'completed', @@ -672,6 +682,7 @@ describe('codex item bodies', () => { expect(codexItemBody({ type: 'mcpToolCall', id: 'm', tool: 't', arguments: {} })).toEqual({ kind: 'tool-call', name: 't', + callId: 'm', input: null, state: 'running' }) @@ -719,6 +730,7 @@ describe('codex item bodies', () => { expect(codexItemBody({ type: 'webSearch', id: 'w', query: '', action: null })).toEqual({ kind: 'tool-call', name: 'web_search', + callId: 'w', input: null, state: 'running' }) @@ -733,6 +745,7 @@ describe('codex item bodies', () => { ).toEqual({ kind: 'tool-call', name: 'web_search', + callId: 'w', input: { query: 'orca release notes', description: 'search', diff --git a/src/main/codex/codex-structured-item-translation.ts b/src/main/codex/codex-structured-item-translation.ts index 576f3fb19ec..52d7d5ae47f 100644 --- a/src/main/codex/codex-structured-item-translation.ts +++ b/src/main/codex/codex-structured-item-translation.ts @@ -93,6 +93,7 @@ function commandItem(item: CodexThreadItem): CodexJournalItem { body: { kind: 'tool-call', name: parsed?.name ?? 'shell', + callId: item.id, // Raw command and cwd stay so the expanded view still shows what ran. input: boundToolInput( { command: item.command ?? null, cwd: item.cwd ?? null, ...parsed?.fields }, @@ -120,6 +121,7 @@ function fileChangeItem(item: CodexThreadItem): CodexJournalItem { body: { kind: 'tool-call', name: 'apply_patch', + callId: item.id, input: boundToolInput({ changes: item.changes ?? null }, DEFAULT_JOURNAL_PAYLOAD_LIMITS), state: commandState(item) }, @@ -171,6 +173,7 @@ function mcpToolCallItem(item: CodexThreadItem): CodexJournalItem { body: { kind: 'tool-call', name: mcpToolCallName(item), + callId: item.id, ...(server && tool ? { mcpIdentity: { server, tool } } : {}), input: boundToolInput(mcpToolArguments(item.arguments), DEFAULT_JOURNAL_PAYLOAD_LIMITS), state: failure === null ? commandState(item) : 'failed', @@ -213,6 +216,7 @@ function webSearchItem(item: CodexThreadItem): CodexJournalItem { body: { kind: 'tool-call', name: 'web_search', + callId: item.id, ...(results.length > 0 ? { webSearchResults: results } : {}), input: boundToolInput(webSearchInput(item), DEFAULT_JOURNAL_PAYLOAD_LIMITS), state: item.action === null || item.action === undefined ? 'running' : 'completed', diff --git a/src/main/native-chat/transcript-line-decoders-codex.ts b/src/main/native-chat/transcript-line-decoders-codex.ts index 229ace2a461..ddc748dde03 100644 --- a/src/main/native-chat/transcript-line-decoders-codex.ts +++ b/src/main/native-chat/transcript-line-decoders-codex.ts @@ -92,10 +92,13 @@ function codexResponseItem( payload.type === 'custom_tool_call' ) { const name = extractString(payload.name) ?? 'tool' + const callId = extractString(payload.call_id) return { id, role: 'assistant', - blocks: [{ type: 'tool-call', name, input: codexCallInput(payload) }], + blocks: [ + { type: 'tool-call', name, input: codexCallInput(payload), ...(callId ? { callId } : {}) } + ], timestamp, source: 'transcript' } diff --git a/src/main/native-chat/transcript-reader-codex-history-mode.test.ts b/src/main/native-chat/transcript-reader-codex-history-mode.test.ts index 5d18bec8c85..9a9b2b76094 100644 --- a/src/main/native-chat/transcript-reader-codex-history-mode.test.ts +++ b/src/main/native-chat/transcript-reader-codex-history-mode.test.ts @@ -240,7 +240,7 @@ describe('Codex transcript history modes', () => { expect(call).toMatchObject({ id: 'call-1', role: 'assistant', - blocks: [{ type: 'tool-call', name: 'exec', input: 'pwd' }] + blocks: [{ type: 'tool-call', name: 'exec', input: 'pwd', callId: 'durable-call-1' }] }) expect(output).toMatchObject({ id: 'fallback-output', diff --git a/src/main/native-chat/transcript-reader.test.ts b/src/main/native-chat/transcript-reader.test.ts index ff48548804d..eb333cd8fda 100644 --- a/src/main/native-chat/transcript-reader.test.ts +++ b/src/main/native-chat/transcript-reader.test.ts @@ -67,7 +67,7 @@ describe('readNativeChatTranscript (claude)', () => { timestamp: '2026-06-01T10:05:00.000Z', message: { role: 'assistant', - content: [{ type: 'tool_use', name: 'Bash', input: { command: 'ls' } }] + content: [{ type: 'tool_use', id: 'tool-call-1', name: 'Bash', input: { command: 'ls' } }] } }) records.push({ @@ -98,7 +98,8 @@ describe('readNativeChatTranscript (claude)', () => { expect(toolCall?.blocks[0]).toEqual({ type: 'tool-call', name: 'Bash', - input: { command: 'ls' } + input: { command: 'ls' }, + callId: 'tool-call-1' }) const toolResult = result.messages.at(-1) diff --git a/src/main/native-chat/transcript-record-blocks.ts b/src/main/native-chat/transcript-record-blocks.ts index 6355df277e5..b82ff9672ca 100644 --- a/src/main/native-chat/transcript-record-blocks.ts +++ b/src/main/native-chat/transcript-record-blocks.ts @@ -81,7 +81,8 @@ function claudeContentBlock(record: Record): NativeChatBlock | } case 'tool_use': { const name = extractString(record.name) ?? 'tool' - return { type: 'tool-call', name, input: record.input } + const callId = extractString(record.id) + return { type: 'tool-call', name, input: record.input, ...(callId ? { callId } : {}) } } case 'tool_result': return toolResultBlock(record) diff --git a/src/renderer/src/components/native-chat/NativeChatDiffCard.tsx b/src/renderer/src/components/native-chat/NativeChatDiffCard.tsx index a69ae1252f1..91d113f15e0 100644 --- a/src/renderer/src/components/native-chat/NativeChatDiffCard.tsx +++ b/src/renderer/src/components/native-chat/NativeChatDiffCard.tsx @@ -1,4 +1,5 @@ -import { useLayoutEffect, useMemo, useRef, useState } from 'react' +import { useLayoutEffect, useMemo, useRef } from 'react' +import { useNativeChatDisclosure } from './native-chat-disclosure-store' import { ChevronRight, FilePlus2, FileMinus2, FilePen } from 'lucide-react' import { cn } from '@/lib/utils' import { translate } from '@/i18n/i18n' @@ -113,21 +114,29 @@ export function NativeChatDiffCard({ file, revealSignal, onReveal, - initiallyExpanded = false + initiallyExpanded = false, + disclosureKey }: { file: NativeChatEditFile revealSignal?: number onReveal?: (element: HTMLElement) => void initiallyExpanded?: boolean + /** Identity this card's open state is remembered under while it is unmounted. */ + disclosureKey?: string }): React.JSX.Element { - const [expanded, setExpanded] = useState(initiallyExpanded) + const { open: expanded, setOpen: setExpanded } = useNativeChatDisclosure( + disclosureKey, + initiallyExpanded + ) const cardRef = useRef(null) useLayoutEffect(() => { if (revealSignal && cardRef.current) { setExpanded(true) + // Reported from the card, not the row: a turn that touched four files must + // land on the one that was asked for, and only the card knows where it is. onReveal?.(cardRef.current) } - }, [revealSignal, onReveal]) + }, [revealSignal, onReveal, setExpanded]) // Joining every row to seed the copy button is the card's most expensive // work, and a collapsed card renders none of those rows. const copyText = useMemo(() => patchText(file.lines), [file.lines]) @@ -141,7 +150,7 @@ export function NativeChatDiffCard({
+
+
+ {hasMore ? ( +
+ +
+ ) : null} + + {showTurnStatus && + latestUserIndex === -1 && + turnStatuses.active && + showTypingIndicator ? ( + + ) : null} + {showTurnStatus && isWorking ? ( + + ) : null} + {!showTurnStatus && showTypingIndicator ? : null}
- ) : null} - {messages.map((message, index) => { - const turnKey = turnKeys[index] - const isCurrentTurn = currentTurnKey - ? turnKey === currentTurnKey - : turnKey === undefined - const status = - index === latestUserIndex - ? turnStatuses.active - : message.role === 'user' && turnKey - ? turnStatuses.completedByTurn[turnKey] - : undefined - const receipt = receipts.get(message.id) - const turnDiff = - turnKey && turnKeys[index + 1] !== turnKey ? turnDiffs.get(turnKey) : undefined - return ( - - {receipt ? ( - - ) : ( - - )} - {showTurnStatus && - status && - (index !== latestUserIndex || showTypingIndicator || !isWorking) ? ( - toggleExpandedTurn(turnKey) - : undefined - } - /> - ) : null} - {turnDiff ? ( - - ) : null} - - ) - })} - {showTurnStatus && - latestUserIndex === -1 && - turnStatuses.active && - showTypingIndicator ? ( - - ) : null} - {showTurnStatus && isWorking ? ( - - ) : null} - {!showTurnStatus && showTypingIndicator ? : null} +
+ {showJump ? ( + + ) : null} - {showJump ? ( - + {taskListState.list && taskListState.list.tasks.length > 0 ? ( +
+
+ +
+
) : null} - {taskListState.list && taskListState.list.tasks.length > 0 ? ( -
-
- -
-
- ) : null} - + ) } diff --git a/src/renderer/src/components/native-chat/NativeChatMessageList.turn-history.test.tsx b/src/renderer/src/components/native-chat/NativeChatMessageList.turn-history.test.tsx index 628d78b4c58..6447dfcbec0 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageList.turn-history.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageList.turn-history.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment happy-dom import '@testing-library/jest-dom/vitest' import { cleanup, fireEvent, render, screen } from '@testing-library/react' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest' import type { AgentJournalItemBody, AgentJournalRenderItem @@ -9,8 +9,14 @@ import type { import { projectStructuredItemsToNativeChat } from '../../../../shared/structured-agent-session-projection' import { NativeChatMessageList } from './NativeChatMessageList' import type { NativeChatLiveSession } from './use-native-chat-live-session' +import { installNativeChatMessageListTestViewport } from './native-chat-message-list-test-viewport' const scrollTo = vi.fn() +let restoreViewport = (): void => {} +beforeAll(() => { + restoreViewport = installNativeChatMessageListTestViewport() +}) +afterAll(() => restoreViewport()) afterEach(() => { cleanup() vi.restoreAllMocks() diff --git a/src/renderer/src/components/native-chat/NativeChatMessageList.turn-timing.test.tsx b/src/renderer/src/components/native-chat/NativeChatMessageList.turn-timing.test.tsx index 022750d83cc..2eacc13ed07 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageList.turn-timing.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageList.turn-timing.test.tsx @@ -3,12 +3,21 @@ import '@testing-library/jest-dom/vitest' import { cleanup, render, screen } from '@testing-library/react' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest' import type { NativeChatLiveSession } from './use-native-chat-live-session' import { NativeChatMessageList } from './NativeChatMessageList' +import { installNativeChatMessageListTestViewport } from './native-chat-message-list-test-viewport' afterEach(cleanup) +let restoreViewport = (): void => {} + +beforeAll(() => { + restoreViewport = installNativeChatMessageListTestViewport() +}) + +afterAll(() => restoreViewport()) + const session: NativeChatLiveSession = { messages: [ { diff --git a/src/renderer/src/components/native-chat/NativeChatMessageList.windowing.test.tsx b/src/renderer/src/components/native-chat/NativeChatMessageList.windowing.test.tsx new file mode 100644 index 00000000000..c4de79ceeed --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatMessageList.windowing.test.tsx @@ -0,0 +1,640 @@ +// @vitest-environment happy-dom + +import '@testing-library/jest-dom/vitest' + +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { + AgentJournalItemBody, + AgentJournalRenderItem +} from '../../../../shared/agent-session-journal-types' +import { projectStructuredItemsToNativeChat } from '../../../../shared/structured-agent-session-projection' +import type { NativeChatMessage } from '../../../../shared/native-chat-types' +import type { NativeChatLiveSession } from './use-native-chat-live-session' +import { NativeChatMessageList } from './NativeChatMessageList' +import { NATIVE_CHAT_BOTTOM_THRESHOLD_PX } from './native-chat-autoscroll' +import { + estimateNativeChatRowHeight, + NATIVE_CHAT_ROW_GAP_PX, + nativeChatRowContentMetrics +} from './native-chat-row-height-estimate' + +afterEach(cleanup) + +const VIEWPORT_PX = 600 +const TRANSCRIPT_LENGTH = 200 + +/** Everything the document holds below the last row: the transcript column's + * trailing chrome and the scroll root's bottom padding. Non-zero on purpose — + * the document's bottom sits past the window's last row, which is exactly where + * a pin computed from the virtualizer's totals and one computed from the + * document disagree. */ +const BELOW_TRANSCRIPT_PX = 24 + +/** Heights the stubbed layout reports per row index, when a case wants a row to + * measure as something other than its estimate. Empty means "every row at its + * estimate", which is what every non-growth case wants. */ +let measuredRowHeights: readonly number[] = [] + +function marker(index: number): NativeChatMessage { + return { + id: `message-${index}`, + role: 'assistant', + blocks: [{ type: 'text', text: `marker-${index}` }], + timestamp: index + 1, + source: 'transcript' + } +} + +const ROW_PX = estimateNativeChatRowHeight(nativeChatRowContentMetrics(marker(0)), { + hasReceipt: false, + hasStatus: false, + hasTurnDiff: false +}) +const ROW_PITCH_PX = ROW_PX + NATIVE_CHAT_ROW_GAP_PX + +/** Replace a layout property on every element, and hand back the undo. */ +function overrideLayoutProperty(name: string, descriptor: PropertyDescriptor): () => void { + const original = Object.getOwnPropertyDescriptor(HTMLElement.prototype, name) + Object.defineProperty(HTMLElement.prototype, name, { configurable: true, ...descriptor }) + return () => { + if (original) { + Object.defineProperty(HTMLElement.prototype, name, original) + } else { + Reflect.deleteProperty(HTMLElement.prototype, name) + } + } +} + +/** The spacer's reserved height, which is the transcript's whole rendered height: + * windowed rows are absolutely positioned inside it, so a row growing in place + * reaches the document only through the height the window reserves for it. */ +function reservedTranscriptHeight(root: ParentNode): number { + const spacer = root.querySelector('[data-native-chat-window]') + return spacer ? Number.parseFloat(spacer.style.height) || 0 : 0 +} + +// The virtualizer measures with `offsetHeight` — not `clientHeight`, not a +// bounding rect — so that is the one thing a DOM without layout has to answer +// for windowing to engage at all. Rows report the height their own estimate +// predicted, which keeps the totals exact and independent of which rows happen +// to have been mounted long enough to be measured; `measuredRowHeights` is how a +// case says a row measures as something else. +// +// `scrollGeometry` additionally gives the scroll root a document to scroll: a +// height, a viewport, and a `scrollTop` that clamps the way a real one does. +// Off by default, because a transcript with a real document opens pinned to its +// bottom and the cases above are about where the window sits, not where it lands. +function stubLayout({ + scrollGeometry = false, + viewportHeight = () => VIEWPORT_PX +}: { + scrollGeometry?: boolean + viewportHeight?: () => number +} = {}): () => void { + const scrollTops = new WeakMap() + const restores = [ + overrideLayoutProperty('offsetHeight', { + get(this: HTMLElement): number { + if (this.hasAttribute('data-native-chat-scroll')) { + return viewportHeight() + } + if (this.hasAttribute('data-native-chat-window')) { + return reservedTranscriptHeight(this.parentElement ?? this) + } + const index = this.dataset.index + if (index !== undefined) { + return measuredRowHeights[Number(index)] ?? ROW_PX + } + // The transcript column: as tall as the window it wraps, plus what sits + // under it. This is the element the list observes for streamed growth. + return this.classList.contains('max-w-4xl') + ? reservedTranscriptHeight(this) + BELOW_TRANSCRIPT_PX + : 0 + } + }) + ] + if (scrollGeometry) { + restores.push( + overrideLayoutProperty('clientHeight', { + get(this: HTMLElement): number { + return this.hasAttribute('data-native-chat-scroll') ? viewportHeight() : 0 + } + }), + overrideLayoutProperty('scrollHeight', { + get(this: HTMLElement): number { + return this.hasAttribute('data-native-chat-scroll') + ? reservedTranscriptHeight(this) + BELOW_TRANSCRIPT_PX + : 0 + } + }), + overrideLayoutProperty('scrollTop', { + get(this: HTMLElement): number { + return scrollTops.get(this) ?? 0 + }, + set(this: HTMLElement, value: number): void { + // A browser clamps; without this `scrollTop = scrollHeight` would park + // the view past the end and every distance-from-bottom would read 0. + const max = Math.max(0, this.scrollHeight - this.clientHeight) + scrollTops.set(this, Math.min(Math.max(0, value), max)) + } + }) + ) + } + return () => { + for (const restore of restores.toReversed()) { + restore() + } + } +} + +type FakeResizeObservation = { + callback: ResizeObserverCallback + /** Target -> height last delivered. -1 means "never", so the first flush + * delivers, the way a real observer's initial callback does. */ + observed: Map +} + +const resizeObservations = new Set() + +/** happy-dom's ResizeObserver never fires, so nothing that re-measures ever runs. + * This one records what production observes and delivers only when a target's + * height actually changed — the browser's own rule — and only when a test says + * a frame was painted. Entries carry no `borderBoxSize`, so the virtualizer + * falls back to `offsetHeight`, which is the path being modelled. */ +function stubResizeObserver(): () => void { + const original = window.ResizeObserver + class TestResizeObserver { + private readonly observation: FakeResizeObservation + constructor(callback: ResizeObserverCallback) { + this.observation = { callback, observed: new Map() } + resizeObservations.add(this.observation) + } + observe(target: Element): void { + this.observation.observed.set(target, -1) + } + unobserve(target: Element): void { + this.observation.observed.delete(target) + } + disconnect(): void { + this.observation.observed.clear() + resizeObservations.delete(this.observation) + } + } + window.ResizeObserver = TestResizeObserver as unknown as typeof ResizeObserver + return () => { + resizeObservations.clear() + window.ResizeObserver = original + } +} + +/** Deliver one round of resize callbacks; true when anything was delivered. */ +function deliverResizes(): boolean { + let delivered = false + // A copy: a callback may disconnect its own observer mid-delivery. + for (const observation of Array.from(resizeObservations)) { + const entries: ResizeObserverEntry[] = [] + for (const [target, lastHeight] of observation.observed) { + const height = (target as HTMLElement).offsetHeight + if (height !== lastHeight) { + observation.observed.set(target, height) + entries.push({ target } as unknown as ResizeObserverEntry) + } + } + if (entries.length > 0) { + delivered = true + observation.callback(entries, undefined as unknown as ResizeObserver) + } + } + return delivered +} + +function session(messages: NativeChatMessage[]): NativeChatLiveSession { + return { + messages, + status: 'ready', + sessionId: 'session-1', + agent: 'codex', + hasMore: false, + loadingEarlier: false, + loadEarlier: vi.fn(), + readPhase: 'ready' + } +} + +function list(messages: NativeChatMessage[]): React.JSX.Element { + return ( + + ) +} + +/** Reads the window, and refuses to pass if there is no window to read. + * + * Without this a change to the usability gate would quietly send every case + * below down the whole-transcript path, where "fewer rows than messages" is + * false but every other assertion still holds. */ +function windowState(container: HTMLElement): { totalSize: number; indexes: number[] } { + const spacer = container.querySelector('[data-native-chat-window]') + if (!spacer) { + throw new Error('transcript is not windowed: no spacer, every row is mounted') + } + const totalSize = Number.parseFloat(spacer.style.height) + if (!(totalSize > 0)) { + throw new Error(`transcript reserved no height (${spacer.style.height})`) + } + return { + totalSize, + indexes: Array.from(container.querySelectorAll('[data-index]')) + .map((row) => Number(row.dataset.index)) + .sort((left, right) => left - right) + } +} + +/** happy-dom fires no scroll event for an assignment to `scrollTop`. */ +function scrollTranscript(container: HTMLElement, top: number): void { + const scroller = container.querySelector('[data-native-chat-scroll]') + if (!scroller) { + throw new Error('no transcript scroll root') + } + scroller.scrollTop = top + fireEvent.scroll(scroller) +} + +describe('windowed transcript', () => { + let restoreLayout = (): void => {} + beforeEach(() => { + restoreLayout = stubLayout() + }) + afterEach(() => { + restoreLayout() + }) + + const transcript = Array.from({ length: TRANSCRIPT_LENGTH }, (_, index) => marker(index)) + + it('mounts a window over the transcript rather than all of it', () => { + const { container } = render(list(transcript)) + const { indexes } = windowState(container) + + expect(indexes.length).toBeGreaterThan(0) + expect(indexes.length).toBeLessThan(TRANSCRIPT_LENGTH / 4) + expect(indexes).toContain(0) + expect(screen.getByText('marker-0')).toBeInTheDocument() + expect(screen.queryByText(`marker-${TRANSCRIPT_LENGTH - 2}`)).toBeNull() + }) + + // One gap per pair of rows, and none after the last one. The other half of + // this — that a row's own reservation does not include the gap as well — is + // pinned on the estimate itself, where it can be seen without layout. + it('reserves each row once and one gap between each pair', () => { + const { container } = render(list(transcript)) + + expect(windowState(container).totalSize).toBe( + TRANSCRIPT_LENGTH * ROW_PX + (TRANSCRIPT_LENGTH - 1) * NATIVE_CHAT_ROW_GAP_PX + ) + }) + + it('moves the mounted rows to bracket the offset the reader scrolled to', () => { + const { container } = render(list(transcript)) + const offset = 5000 + scrollTranscript(container, offset) + const { indexes } = windowState(container) + const focused = Math.floor(offset / ROW_PITCH_PX) + + expect(indexes).toContain(focused) + expect(indexes[0]).toBeLessThanOrEqual(focused) + expect(indexes.at(-1)).toBeGreaterThanOrEqual(focused) + expect(indexes).not.toContain(0) + expect(indexes.length).toBeLessThan(TRANSCRIPT_LENGTH / 4) + }) + + // The live row announces a running tool through `aria-live`, which says nothing + // from a row that is not in the document. + it('keeps the newest row mounted after the reader scrolls away from it', () => { + const { container } = render(list(transcript)) + scrollTranscript(container, 5000) + + expect(windowState(container).indexes).toContain(TRANSCRIPT_LENGTH - 1) + }) + + it('gives no slot to a message that draws nothing', () => { + const withBlanks = Array.from({ length: TRANSCRIPT_LENGTH }, (_, index) => + index % 4 === 0 + ? { ...marker(index), blocks: [{ type: 'text' as const, text: '' }] } + : marker(index) + ) + const drawn = TRANSCRIPT_LENGTH - TRANSCRIPT_LENGTH / 4 + const { container } = render(list(withBlanks)) + const { totalSize, indexes } = windowState(container) + + expect(totalSize).toBe(drawn * ROW_PX + (drawn - 1) * NATIVE_CHAT_ROW_GAP_PX) + expect(indexes.at(-1)).toBeLessThanOrEqual(drawn - 1) + }) + + it('still has the tool run open when the row carrying it comes back', () => { + const withTool = [...transcript] + withTool[1] = { + ...marker(1), + blocks: [ + { type: 'text', text: 'marker-1' }, + { type: 'tool-call', name: 'shell', input: { command: 'ls' }, state: 'completed' } + ] + } + const { container } = render(list(withTool)) + + const header = screen.getByRole('button', { name: /1×/ }) + expect(header).toHaveAttribute('aria-expanded', 'false') + fireEvent.click(header) + expect(screen.getByRole('button', { name: /1×/ })).toHaveAttribute('aria-expanded', 'true') + + scrollTranscript(container, 5000) + expect(windowState(container).indexes).not.toContain(1) + expect(screen.queryByRole('button', { name: /1×/ })).toBeNull() + + scrollTranscript(container, 0) + expect(screen.getByRole('button', { name: /1×/ })).toHaveAttribute('aria-expanded', 'true') + }) +}) + +// The reveal chain runs message -> tool run -> diff card and lands on a card in +// a DIFFERENT, earlier message than the rollup that was clicked. Under windowing +// that message may not be mounted to be pointed at, so the reveal names it by id +// and the row is pinned into the window until the card can answer for itself. +describe('revealing a diff from a turn rollup', () => { + let restoreLayout = (): void => {} + beforeEach(() => { + restoreLayout = stubLayout() + }) + afterEach(() => { + restoreLayout() + vi.restoreAllMocks() + }) + + function journalItem(itemId: string, body: AgentJournalItemBody, sequence: number) { + return { itemId, body, sequence, observedAt: sequence * 1000, revision: 1 } + } + + const patch = '@@ -1 +1 @@\n-before\n+after' + const items: AgentJournalRenderItem[] = [ + journalItem( + 'user', + { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'Edit it' }] }, + 1 + ), + journalItem( + 'diff', + { + kind: 'diff', + path: 'src/a.ts', + patch: { head: patch, truncated: false, digest: 'fixture', byteLength: patch.length } + }, + 2 + ), + ...Array.from({ length: TRANSCRIPT_LENGTH }, (_, index) => + journalItem( + `tail-${index}`, + { kind: 'message', role: 'assistant', blocks: [{ type: 'text', text: `marker-${index}` }] }, + index + 3 + ) + ) + ] + + it('mounts the row a reveal names even when the window has left it behind', () => { + const scrollTo = vi.fn() + vi.spyOn(HTMLElement.prototype, 'scrollTo').mockImplementation(scrollTo) + const { container } = render( + + ) + // The rollup rides the turn's last row, which is pinned; the diff it points + // at is near the top and long gone from the window. + scrollTranscript(container, 4000) + expect(screen.queryByText('Edited file')).toBeNull() + const mountedBefore = windowState(container).indexes.length + + fireEvent.click(screen.getByRole('button', { name: /1 changed file/ })) + scrollTo.mockClear() + fireEvent.click(screen.getByRole('button', { name: /src\/a.ts/ })) + + expect(screen.getByText('Edited file')).toBeInTheDocument() + expect(screen.getByText('after')).toBeInTheDocument() + expect(scrollTo).toHaveBeenCalled() + // Pinned, not paged to: the window is still a window. + expect(windowState(container).indexes.length).toBeLessThanOrEqual(mountedBefore + 2) + }) +}) + +describe('transcript with a hidden scroll root', () => { + const transcript = Array.from({ length: 40 }, (_, index) => marker(index)) + + it('keeps the transcript bounded and rehydrates when the viewport becomes measurable', () => { + let viewportHeight = 0 + const restoreLayout = stubLayout({ viewportHeight: () => viewportHeight }) + const restoreResizeObserver = stubResizeObserver() + try { + const { container } = render(list(transcript)) + + expect(container.querySelector('[data-native-chat-window]')).toBeInTheDocument() + expect(container.querySelectorAll('[data-index]')).toHaveLength(0) + expect(screen.queryByText(/^marker-/)).toBeNull() + const column = container.querySelector('.max-w-4xl') + expect(column?.children).toHaveLength(1) + + viewportHeight = VIEWPORT_PX + act(() => { + deliverResizes() + }) + const { indexes } = windowState(container) + expect(indexes.length).toBeGreaterThan(0) + expect(indexes.length).toBeLessThan(transcript.length) + } finally { + restoreResizeObserver() + restoreLayout() + } + }) +}) + +// A row that grows in place: the same message id, more content, a taller measured +// box — what a streaming reply looks like to the window. Whole-message appends +// arrive at their final height and are a different case; this is the one where +// the row the reader is looking at keeps changing size underneath them. +// +// Two mechanisms are supposed to hold the pin, and both are exercised here: the +// list's own resize observer on the transcript column (which re-runs +// `scrollToBottom` against the document) and the virtualizer's end anchor (which +// compensates `scrollTop` by the growth when the view was already at the end). +describe('a row growing in place while the view is pinned to the bottom', () => { + const TAIL_INDEX = TRANSCRIPT_LENGTH - 1 + const GROWTH_STEPS = 24 + const LINES_PER_STEP = 12 + /** One wrapped prose line. Content and measured height grow from this one + * number, so a step that adds lines is a step that adds pixels. */ + const STREAM_LINE_PX = 22 + /** Every row but the growing one measures at its estimate, so the reserved + * total is arithmetic rather than a snapshot. */ + const BASE_TOTAL_PX = + (TRANSCRIPT_LENGTH - 1) * ROW_PX + (TRANSCRIPT_LENGTH - 1) * NATIVE_CHAT_ROW_GAP_PX + + /** Fixed so a re-render never restamps the turn and moves the status row. */ + const TURN_STARTED_AT = Date.now() + + const transcript = Array.from({ length: TRANSCRIPT_LENGTH }, (_, index) => marker(index)) + + function tailHeightAt(step: number): number { + return Math.max(ROW_PX, (1 + step * LINES_PER_STEP) * STREAM_LINE_PX) + } + + function transcriptAt(step: number): NativeChatMessage[] { + const lines = Array.from( + { length: step * LINES_PER_STEP }, + (_, index) => `streamed line ${index}` + ) + const next = [...transcript] + next[TAIL_INDEX] = { + ...marker(TAIL_INDEX), + blocks: [{ type: 'text', text: [`marker-${TAIL_INDEX}`, ...lines].join('\n') }] + } + return next + } + + function streamingList(step: number): React.JSX.Element { + return ( + + ) + } + + function scrollRoot(container: HTMLElement): HTMLElement { + const scroller = container.querySelector('[data-native-chat-scroll]') + if (!scroller) { + throw new Error('no transcript scroll root') + } + return scroller + } + + /** One painted frame, repeated to a fixed point: deliver the resize callbacks + * the growth caused, then fire the scroll event a browser fires for any + * `scrollTop` the code wrote itself. Refusing to settle is a failure in its + * own right — that is the view oscillating. */ + function paint(container: HTMLElement): void { + const scroller = scrollRoot(container) + let lastScrollTop = scroller.scrollTop + for (let pass = 0; pass < 12; pass += 1) { + let changed = false + act(() => { + changed = deliverResizes() + }) + if (scroller.scrollTop !== lastScrollTop) { + lastScrollTop = scroller.scrollTop + fireEvent.scroll(scroller) + changed = true + } + if (!changed) { + return + } + } + throw new Error('the transcript never settled: resize and scroll kept moving it') + } + + function distanceFromBottom(container: HTMLElement): number { + const scroller = scrollRoot(container) + return scroller.scrollHeight - scroller.clientHeight - scroller.scrollTop + } + + function setMeasuredTail(step: number): void { + const heights = Array.from({ length: TRANSCRIPT_LENGTH }, () => ROW_PX) + heights[TAIL_INDEX] = tailHeightAt(step) + measuredRowHeights = heights + } + + let restoreLayout = (): void => {} + let restoreResizeObserver = (): void => {} + beforeEach(() => { + restoreLayout = stubLayout({ scrollGeometry: true }) + restoreResizeObserver = stubResizeObserver() + setMeasuredTail(0) + }) + afterEach(() => { + restoreResizeObserver() + restoreLayout() + measuredRowHeights = [] + }) + + it('holds the pin, the mount and the reserved total at every frame of the growth', () => { + setMeasuredTail(0) + const { container, rerender } = render(streamingList(0)) + paint(container) + + expect(distanceFromBottom(container)).toBeLessThanOrEqual(NATIVE_CHAT_BOTTOM_THRESHOLD_PX) + expect(windowState(container).totalSize).toBe(BASE_TOTAL_PX + tailHeightAt(0)) + + const frames: { step: number; tail: number; total: number; distance: number }[] = [] + for (let step = 1; step <= GROWTH_STEPS; step += 1) { + setMeasuredTail(step) + rerender(streamingList(step)) + paint(container) + + const { totalSize, indexes } = windowState(container) + const distance = distanceFromBottom(container) + frames.push({ step, tail: tailHeightAt(step), total: totalSize, distance }) + + // Pinned: the reader is still looking at the bottom of the row. + expect(distance).toBeLessThanOrEqual(NATIVE_CHAT_BOTTOM_THRESHOLD_PX) + // Mounted: never swapped for reserved space while it is the live row. + expect(indexes).toContain(TAIL_INDEX) + expect(screen.getByText(/streamed line 0/)).toBeInTheDocument() + // Tracking: the reservation follows the measurement, not the estimate. + expect(totalSize).toBe(BASE_TOTAL_PX + tailHeightAt(step)) + // Still a window, not the whole transcript remounted by the growth. + expect(indexes.length).toBeLessThan(TRANSCRIPT_LENGTH / 4) + } + + expect(frames).toHaveLength(GROWTH_STEPS) + expect(frames.at(-1)?.tail).toBeGreaterThan(VIEWPORT_PX * 10) + expect(Math.max(...frames.map((frame) => frame.distance))).toBeLessThanOrEqual( + NATIVE_CHAT_BOTTOM_THRESHOLD_PX + ) + }) + + it('leaves a reader who scrolled up where they were, however far the row grows', () => { + setMeasuredTail(4) + const { container, rerender } = render(streamingList(4)) + paint(container) + + const readingAt = 2000 + scrollTranscript(container, readingAt) + paint(container) + expect(distanceFromBottom(container)).toBeGreaterThan(NATIVE_CHAT_BOTTOM_THRESHOLD_PX) + expect(screen.getByRole('button', { name: /jump to latest/i })).toBeInTheDocument() + + for (let step = 5; step <= GROWTH_STEPS; step += 1) { + setMeasuredTail(step) + rerender(streamingList(step)) + paint(container) + + const { totalSize, indexes } = windowState(container) + // Not yanked: the offset the reader chose is the offset they still have. + expect(scrollRoot(container).scrollTop).toBe(readingAt) + // The row is off screen but still measured, which is what keeps the + // reserved total — and so the scrollbar — honest while it grows. + expect(indexes).toContain(TAIL_INDEX) + expect(totalSize).toBe(BASE_TOTAL_PX + tailHeightAt(step)) + } + + expect(screen.getByRole('button', { name: /jump to latest/i })).toBeInTheDocument() + }) +}) diff --git a/src/renderer/src/components/native-chat/NativeChatMessageRow.tsx b/src/renderer/src/components/native-chat/NativeChatMessageRow.tsx index 773e6f2c2ff..d73772dfaf9 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageRow.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageRow.tsx @@ -1,23 +1,17 @@ -import { memo, useCallback, useMemo, useRef } from 'react' +import { memo, useCallback, useRef } from 'react' import CommentMarkdown, { type CommentMarkdownLinkClickHandler } from '@/components/sidebar/CommentMarkdown' import { cn } from '@/lib/utils' import { translate } from '@/i18n/i18n' -import { - isSubagentGroupFallbackText, - subagentGroupBlocks -} from '../../../../shared/native-chat-subagent-summary' -import { - isSubagentGroupBlock, - type NativeChatMessage, - type NativeChatToolCallBlock +import type { + NativeChatMessage, + NativeChatToolCallBlock } from '../../../../shared/native-chat-types' -import { splitNativeChatBlocks } from './native-chat-tool-fold' +import { deriveNativeChatRowContent } from './native-chat-row-content' import { NativeChatToolRun } from './NativeChatToolRun' import { NativeChatNoticeRow } from './NativeChatNoticeRow' import { NativeChatMessageTimestamp } from './NativeChatMessageTimestamp' -import { nativeChatProseToMarkdown } from './native-chat-prose' import { NativeChatAgentControls, NativeChatImageAttachments, @@ -62,32 +56,11 @@ export const MessageRow = memo(function MessageRow({ runtimeContext?: RuntimeFileOperationArgs | null }): React.JSX.Element | null { const rowRef = useRef(null) - // One pass per block set: a streaming turn re-renders this row on every frame, and these - // derivations used to re-run each time even though `message.blocks` had not changed. - const { hasImages, markdown, prose, subagentGroups, tools } = useMemo(() => { - const split = splitNativeChatBlocks(message.blocks) - const groups = subagentGroupBlocks(split.prose) - // A spawn-group row carries a plain-text twin so a client without the block - // type still reads the roster. This one draws the block, so the twin is - // dropped rather than printed beside it — only the twin, never the prose - // beside it: the block is provider-agnostic, so a lane that folds a roster - // into a message with real text must not lose that text here. - const prose = - groups.length === 0 - ? split.prose - : split.prose.filter( - (block) => - !isSubagentGroupBlock(block) && - !(block.type === 'text' && isSubagentGroupFallbackText(block.text)) - ) - return { - tools: split.tools, - prose, - subagentGroups: groups, - markdown: nativeChatProseToMarkdown(prose), - hasImages: prose.some((block) => block.type === 'image-ref') - } - }, [message.blocks]) + // One pass per block set, shared with the list that decides whether this row + // occupies a slot — so "draws nothing" means the same thing to both. + const { hasImages, markdown, prose, subagentGroups, tools } = deriveNativeChatRowContent( + message.blocks + ) const isUser = message.role === 'user' const isReasoning = message.role === 'reasoning' const isSystem = message.role === 'system' @@ -220,6 +193,7 @@ export const MessageRow = memo(function MessageRow({ expandOverride={activityExpandOverride} activeTurnIsWorking={activeTurnIsWorking} structuredActivityUi={structuredActivityUi} + disclosureId={message.id} /> ) : null} {showControls ? ( diff --git a/src/renderer/src/components/native-chat/NativeChatToolLine.tsx b/src/renderer/src/components/native-chat/NativeChatToolLine.tsx new file mode 100644 index 00000000000..883244d8f91 --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatToolLine.tsx @@ -0,0 +1,139 @@ +import type { CommentMarkdownLinkClickHandler } from '@/components/sidebar/CommentMarkdown' +import { ChevronRight } from 'lucide-react' +import { cn } from '@/lib/utils' +import { translate } from '@/i18n/i18n' +import { + isToolCallBlock, + isToolResultBlock, + type NativeChatBlock +} from '../../../../shared/native-chat-types' +import { + NativeChatCommandMetadata, + NativeChatSearchResults, + NativeChatToolName +} from './NativeChatToolAnnotations' +import { NativeChatToolIcon } from './NativeChatToolIcon' +import { NativeChatDiffView } from './NativeChatDiffView' +import { diffFromText, diffFromToolCall, type DiffLine } from './native-chat-diff' +import { useNativeChatDisclosure } from './native-chat-disclosure-store' +import { createToolInputDisplay, truncateToolDetail } from './native-chat-tool-summary' + +/** A single inline tool line — `▸ ToolName preview` — that expands in place to + * show the call's diff/input or the result's body. Tool calls read as flat + * lines in the conversation rather than boxed blocks (mobile parity). Lines only + * mount while the parent run is open and are individually collapsible. */ +export function NativeChatToolLine({ + block, + initiallyExpanded = true, + disclosureKey, + onLinkClick +}: { + block: NativeChatBlock + initiallyExpanded?: boolean + /** Identity this line's open state is remembered under while it is unmounted. */ + disclosureKey?: string + onLinkClick?: CommentMarkdownLinkClickHandler +}): React.JSX.Element | null { + const { open: expanded, setOpen: setExpanded } = useNativeChatDisclosure( + disclosureKey, + initiallyExpanded + ) + + let name: string + let preview: string + let diff: DiffLine[] | null = null + let body: { output: string; isError?: boolean } | null = null + let detail: string | null = null + let inputHasDetail = false + const isCall = isToolCallBlock(block) + + if (isCall) { + name = block.name + const inputDisplay = createToolInputDisplay(block.input) + preview = inputDisplay.label + inputHasDetail = inputDisplay.hasDetail + diff = expanded ? diffFromToolCall(block.name, block.input) : null + detail = expanded && !diff ? inputDisplay.formatDetail() : null + } else if (isToolResultBlock(block)) { + name = translate('components.native-chat.tool.result', 'Result') + preview = block.output.split('\n')[0]?.slice(0, 80) ?? '' + diff = expanded ? diffFromText(block.output) : null + body = { output: block.output, isError: block.isError } + } else { + return null + } + + const hasResults = isCall && (block.webSearchResults?.length ?? 0) > 0 + const hasDetail = diff !== null || body !== null || inputHasDetail || hasResults + + return ( +
+ + {hasDetail && expanded ? ( +
+ {isCall && hasResults ? ( + + ) : null} + {diff ? : null} + {!diff && body ? ( + 
+              {truncateToolDetail(body.output)}
+
+ ) : null} + {!diff && !body && detail ? ( + 
+              {detail}
+
+ ) : null} +
+ ) : null} +
+ ) +} diff --git a/src/renderer/src/components/native-chat/NativeChatToolRun.identity.test.tsx b/src/renderer/src/components/native-chat/NativeChatToolRun.identity.test.tsx index 2b83f86d14a..86d962ae81d 100644 --- a/src/renderer/src/components/native-chat/NativeChatToolRun.identity.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatToolRun.identity.test.tsx @@ -3,10 +3,24 @@ import { cleanup, fireEvent, render, screen } from '@testing-library/react' import { afterEach, describe, expect, it, vi } from 'vitest' import { NativeChatToolRun } from './NativeChatToolRun' import type { NativeChatToolCallBlock } from '../../../../shared/native-chat-types' +import { + NativeChatDisclosureContext, + useNativeChatDisclosures +} from './native-chat-disclosure-store' vi.mock('./NativeChatDiffCard', () => ({ NativeChatDiffCard: () => null })) vi.mock('./NativeChatDiffView', () => ({ NativeChatDiffView: () => null })) -afterEach(cleanup) + +const disclosureWrite = vi.fn() +const capturedDisclosures = { + read: (_key: string) => undefined, + write: (key: string, open: boolean) => disclosureWrite(key, open) +} + +afterEach(() => { + cleanup() + disclosureWrite.mockReset() +}) const shell: NativeChatToolCallBlock = { type: 'tool-call', @@ -17,7 +31,100 @@ const shell: NativeChatToolCallBlock = { durationMs: 400 } +function ToolRunDisclosureHarness({ expandOverride }: { expandOverride: boolean }) { + const disclosures = useNativeChatDisclosures() + return ( + + + + ) +} + describe('inline tool annotations', () => { + it('restores a per-run deviation when its turn returns to the same disclosure state', () => { + const { rerender } = render() + const run = screen.getByRole('button', { expanded: true }) + + fireEvent.click(run) + expect(run.getAttribute('aria-expanded')).toBe('false') + + rerender() + expect(screen.queryByRole('button')).toBeNull() + + rerender() + expect(screen.getByRole('button', { name: /1×/ }).getAttribute('aria-expanded')).toBe('false') + }) + + it('resynchronizes a standalone run when the toolbar signal flips', () => { + const { rerender } = render( + + ) + expect(screen.getByRole('button').getAttribute('aria-expanded')).toBe('false') + + rerender() + + expect(screen.getByRole('button', { name: /1×/ }).getAttribute('aria-expanded')).toBe('true') + }) + + it('uses provider call identities for byte-identical line disclosure keys', () => { + const blocks = [ + { ...shell, callId: 'call-a' }, + { ...shell, callId: 'call-b' } + ] + render( + + + + ) + + fireEvent.click(screen.getAllByRole('button')[2]!) + + expect(disclosureWrite).toHaveBeenCalledExactlyOnceWith('line:message-1:call:call-b', false) + }) + + it('keeps occurrence identity as the fallback for calls without provider IDs', () => { + render( + + + + ) + + fireEvent.click(screen.getAllByRole('button')[2]!) + + expect(disclosureWrite).toHaveBeenCalledExactlyOnceWith( + 'line:message-1:tool-call:shell:{"command":"missing-command"}:1', + false + ) + }) + + it('keeps occurrence identity for whitespace-only provider IDs', () => { + render( + + + + ) + + fireEvent.click(screen.getAllByRole('button')[2]!) + + expect(disclosureWrite).toHaveBeenCalledExactlyOnceWith( + 'line:message-1:tool-call:shell:{"command":"missing-command"}:1', + false + ) + }) + it('keeps command completion annotations on the collapsed tool line', () => { render( 0 - const hasDetail = diff !== null || body !== null || inputHasDetail || hasResults - - return ( -
- - {hasDetail && expanded ? ( -
- {isCall && hasResults ? ( - - ) : null} - {diff ? : null} - {!diff && body ? ( - 
-              {truncateToolDetail(body.output)}
-
- ) : null} - {!diff && !body && detail ? ( - 
-              {detail}
-
- ) : null} -
- ) : null} -
- ) -} - /** A run of a message's tool calls/results, collapsed to a one-line summary that - * expands to the individual inline tool lines. `expandSignal` lets the global - * toolbar toggle drive every run at once while still allowing per-run override. */ + * expands to the individual inline tool lines. */ export function NativeChatToolRun({ blocks, previousTodoWrite, @@ -170,6 +47,7 @@ export function NativeChatToolRun({ activeTurnIsWorking, expandOverride, structuredActivityUi = true, + disclosureId, onLinkClick }: { blocks: NativeChatBlock[] @@ -179,32 +57,28 @@ export function NativeChatToolRun({ onRevealDiff?: (element: HTMLElement) => void /** Spawn-group rosters that belong with this run's activity, one row each. */ subagentGroups?: NativeChatSubagentGroupBlock[] - /** Toolbar-driven desired open state. Each change re-syncs this run's state. */ + /** Legacy view-level default; production native-chat entry points pass false. */ expandSignal: boolean /** Per-turn disclosure state controlled by the completed turn status row. */ expandOverride?: boolean /** Structured lifecycle state, when available, keeps orphaned running calls from spinning. */ activeTurnIsWorking?: boolean structuredActivityUi?: boolean + /** Message this run belongs to. Windowing unmounts rows, so a run the reader + * opened has to be remembered somewhere that outlives the row. */ + disclosureId?: string onLinkClick?: CommentMarkdownLinkClickHandler }): React.JSX.Element | null { - const [open, setOpen] = useState(revealedDiff ? true : (expandOverride ?? expandSignal)) - const [controls, setControls] = useState({ expandOverride, expandSignal, revealedDiff }) - if ( - controls.expandOverride !== expandOverride || - controls.expandSignal !== expandSignal || - controls.revealedDiff !== revealedDiff - ) { - setControls({ expandOverride, expandSignal, revealedDiff }) - if (revealedDiff && controls.revealedDiff !== revealedDiff) { - setOpen(true) - } else if ( - controls.expandOverride !== expandOverride || - controls.expandSignal !== expandSignal - ) { - setOpen(expandOverride ?? expandSignal) - } - } + // A reader's deviation belongs to the controlling disclosure state, so returning + // to that state restores the same choice without writing to the store mid-render. + const runKey = + disclosureId === undefined + ? undefined + : `run:${disclosureId}:${expandOverride ?? '-'}:${expandSignal}:${revealedDiff?.requestId ?? '-'}` + const { open, setOpen } = useNativeChatDisclosure( + runKey, + revealedDiff ? true : (expandOverride ?? expandSignal) + ) // Childless groups are dropped so `subagentRows.length` stays an honest test of // "something will draw": the roster-only branch below returns a margin-bearing @@ -236,8 +110,7 @@ export function NativeChatToolRun({ : null const isSettled = latestActiveCall == null const hasRunningCall = blocks.some((block) => isToolCallBlock(block) && block.state === 'running') - // The turn caret opens the activity group, while each child tool remains - // collapsed. The global expand toolbar still opens child details together. + // The turn caret opens the activity group while each child tool stays collapsed. const expandToolLines = expandOverride === undefined ? open : false // Diffing every edit is the run's most expensive work, so a collapsed run — // which renders none of it — never pays for it. @@ -307,7 +180,7 @@ export function NativeChatToolRun({ {latestActiveCall ? (