From 690dc7b40f8898fac93f7800902f15469284dd04 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 7 Jul 2026 17:05:34 +1000 Subject: [PATCH 1/9] feat(sessions): add tool call view model Signed-off-by: Matt Toohey --- .../sessions/toolCallViewModel.test.ts | 286 +++++++++ .../features/sessions/toolCallViewModel.ts | 607 ++++++++++++++++++ 2 files changed, 893 insertions(+) create mode 100644 apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts create mode 100644 apps/staged/src/lib/features/sessions/toolCallViewModel.ts diff --git a/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts b/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts new file mode 100644 index 00000000..8fc20838 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts @@ -0,0 +1,286 @@ +import { describe, expect, it } from 'vitest'; +import type { SessionMessage } from '../../types'; +import type { RichToolItem, ToolStatus } from './acpTranscript'; +import { buildToolCallViewModel } from './toolCallViewModel'; + +function message( + overrides: Partial & Pick +): SessionMessage { + return { + sessionId: 'session-1', + content: overrides.content ?? '', + createdAt: overrides.id, + ...overrides, + }; +} + +function richTool(overrides: Partial = {}): RichToolItem { + const status = overrides.status ?? 'completed'; + return { + key: 'tool:tc-1', + call: message({ id: 1, role: 'tool_call', content: 'Custom tool' }), + result: null, + verb: 'Ran', + detail: '', + status, + statusLabel: statusLabel(status), + statusTone: statusTone(status), + toolCallId: 'tc-1', + toolKind: null, + rawInput: undefined, + rawOutput: undefined, + content: undefined, + locations: undefined, + ...overrides, + }; +} + +function statusLabel(status: ToolStatus): RichToolItem['statusLabel'] { + switch (status) { + case 'pending': + return 'Pending'; + case 'in_progress': + return 'In progress'; + case 'completed': + return 'Succeeded'; + case 'failed': + return 'Failed'; + case 'cancelled': + return 'Cancelled'; + } +} + +function statusTone(status: ToolStatus): RichToolItem['statusTone'] { + switch (status) { + case 'pending': + return 'muted'; + case 'in_progress': + return 'running'; + case 'completed': + return 'success'; + case 'failed': + return 'danger'; + case 'cancelled': + return 'cancelled'; + } +} + +describe('buildToolCallViewModel classification', () => { + it('classifies from normalized toolKind before the visible verb', () => { + const model = buildToolCallViewModel( + richTool({ + toolKind: 'Edit', + verb: 'Ran', + rawInput: { path: '/repo/src/App.svelte' }, + }), + '/repo' + ); + + expect(model.category).toBe('edit'); + expect(model.metadata.toolKind).toBe('edit'); + expect(model.metadata.targetPath).toBe('src/App.svelte'); + }); + + it('classifies from parsed tool titles and extracts parsed input', () => { + const model = buildToolCallViewModel( + richTool({ + verb: 'Processed', + call: message({ + id: 1, + role: 'tool_call', + content: JSON.stringify({ + name: 'Read /repo/src/App.svelte', + input: { file_path: '/repo/src/App.svelte' }, + }), + }), + }), + '/repo' + ); + + expect(model.category).toBe('read'); + expect(model.metadata.toolName).toBe('Read /repo/src/App.svelte'); + expect(model.metadata.targetPath).toBe('src/App.svelte'); + expect(model.metadata.inputText).toContain('file_path'); + }); + + it('classifies parsed_cmd metadata before falling back to display verbs', () => { + const model = buildToolCallViewModel( + richTool({ + verb: 'Ran', + call: message({ + id: 1, + role: 'tool_call', + content: JSON.stringify({ + name: 'Custom wrapper', + input: { + parsed_cmd: [ + { + type: 'search', + cmd: "rg -n 'needle' /repo/src", + query: 'needle', + path: '/repo/src', + }, + ], + }, + }), + }), + }), + '/repo' + ); + + expect(model.category).toBe('search'); + expect(model.metadata.query).toBe('needle'); + expect(model.metadata.targetPath).toBe('src'); + expect(model.metadata.parsedCommands[0]).toMatchObject({ + type: 'search', + cmd: "rg -n 'needle' /repo/src", + }); + }); + + it('falls back to ACP content blocks when tool identity is missing', () => { + const model = buildToolCallViewModel( + richTool({ + verb: 'Processed', + content: [ + { + type: 'diff', + path: '/repo/src/a.ts', + oldText: 'old line\n', + newText: 'new line\n', + }, + ], + }), + '/repo' + ); + + expect(model.category).toBe('edit'); + expect(model.metadata.diffs).toEqual([ + { + path: 'src/a.ts', + oldText: 'old line\n', + newText: 'new line\n', + kind: 'modified', + }, + ]); + expect(model.sections.some((section) => section.kind === 'diff')).toBe(true); + }); + + it('detects network-style unknown tools from raw input metadata', () => { + const model = buildToolCallViewModel( + richTool({ + verb: 'Processed', + rawInput: { method: 'post', url: 'https://example.test/api' }, + }) + ); + + expect(model.category).toBe('network'); + expect(model.metadata.method).toBe('POST'); + expect(model.metadata.url).toBe('https://example.test/api'); + }); + + it('keeps unknown tools generic with raw JSON fallback sections', () => { + const model = buildToolCallViewModel( + richTool({ + verb: 'Processed', + rawInput: { option: 'value' }, + rawOutput: { payload: { ok: true } }, + }) + ); + + expect(model.category).toBe('generic'); + expect(model.output.state).toBe('output'); + expect(model.sections).toEqual( + expect.arrayContaining([ + expect.objectContaining({ kind: 'input', text: expect.stringContaining('option') }), + expect.objectContaining({ kind: 'raw_output', text: expect.stringContaining('payload') }), + ]) + ); + }); +}); + +describe('buildToolCallViewModel output handling', () => { + it('promotes failed errors before raw output JSON', () => { + const model = buildToolCallViewModel( + richTool({ + status: 'failed', + rawOutput: { error: 'permission denied', exitCode: 1 }, + }) + ); + + expect(model.output.state).toBe('output'); + expect(model.output.errorText).toBe('permission denied'); + expect(model.output.exitCode).toBe(1); + + const errorIndex = model.sections.findIndex( + (section) => section.kind === 'output' && section.label === 'Error' + ); + const rawIndex = model.sections.findIndex((section) => section.kind === 'raw_output'); + expect(errorIndex).toBeGreaterThanOrEqual(0); + expect(rawIndex).toBeGreaterThan(errorIndex); + }); + + it('keeps structured stdout and stderr distinct for successful commands', () => { + const model = buildToolCallViewModel( + richTool({ + rawOutput: { stdout: 'tests passed', stderr: 'warning only', exitCode: 0 }, + }) + ); + + expect(model.output.stdout).toBe('tests passed'); + expect(model.output.stderr).toBe('warning only'); + expect(model.output.errorText).toBe(''); + const outputLabels = model.sections.flatMap((section) => + section.kind === 'output' ? [section.label] : [] + ); + expect(outputLabels).toEqual(['Stdout', 'Stderr']); + }); + + it('falls back to legacy tool_result content when ACP output is absent', () => { + const model = buildToolCallViewModel( + richTool({ + result: message({ + id: 2, + role: 'tool_result', + content: '```text\nlegacy output\n```', + }), + }) + ); + + expect(model.category).toBe('command'); + expect(model.output.primaryText).toBe('legacy output'); + expect(model.sections).toContainEqual({ + kind: 'output', + label: 'Output', + text: 'legacy output', + tone: 'normal', + }); + }); + + it('shows a waiting state for pending tools without output', () => { + const model = buildToolCallViewModel( + richTool({ + status: 'pending', + statusTone: 'muted', + result: null, + }) + ); + + expect(model.output.state).toBe('waiting'); + expect(model.output.emptyLabel).toBe('Waiting for output'); + expect(model.sections).toContainEqual({ kind: 'empty', label: 'Waiting for output' }); + }); + + it('shows a no-output state for completed tools without output', () => { + const model = buildToolCallViewModel( + richTool({ + status: 'completed', + result: null, + }) + ); + + expect(model.output.state).toBe('empty'); + expect(model.output.emptyLabel).toBe('No output'); + expect(model.hasDetails).toBe(true); + expect(model.sections).toContainEqual({ kind: 'empty', label: 'No output' }); + }); +}); diff --git a/apps/staged/src/lib/features/sessions/toolCallViewModel.ts b/apps/staged/src/lib/features/sessions/toolCallViewModel.ts new file mode 100644 index 00000000..d0945096 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/toolCallViewModel.ts @@ -0,0 +1,607 @@ +import type { DisplayRootInput } from './pathDisplayRoots'; +import type { RichToolItem, ToolStatus } from './acpTranscript'; +import { formatJson, terminalRefsFromAcpContent, textFromAcpContent } from './acpTranscript'; +import { makePathsRelative, parseToolCall, stripCodeFences } from './sessionModalHelpers'; + +export type ToolCallCategory = 'edit' | 'command' | 'read' | 'search' | 'network' | 'generic'; + +export type ToolCallOutputState = 'output' | 'waiting' | 'empty'; + +export type ToolCallDiffKind = 'created' | 'deleted' | 'modified' | 'unchanged'; + +export interface ToolCallDiff { + path: string; + oldText: string | null; + newText: string | null; + kind: ToolCallDiffKind; +} + +export interface ToolCallLocation { + path: string; + display: string; + line: number | null; + column: number | null; +} + +export interface ParsedToolCommand { + type: string | null; + cmd: string | null; + path: string | null; + query: string | null; + name: string | null; +} + +export interface ToolCallMetadata { + toolKind: string | null; + toolName: string | null; + input: Record | null; + inputText: string; + parsedCommands: ParsedToolCommand[]; + command: string | null; + workingDirectory: string | null; + targetPath: string | null; + query: string | null; + url: string | null; + method: string | null; + locations: ToolCallLocation[]; + diffs: ToolCallDiff[]; + terminalRefs: string[]; +} + +export interface ToolCallOutput { + state: ToolCallOutputState; + primaryText: string; + rawText: string; + errorText: string; + stdout: string; + stderr: string; + exitCode: number | null; + emptyLabel: 'Waiting for output' | 'No output' | null; +} + +export type ToolCallSection = + | { kind: 'locations'; locations: ToolCallLocation[] } + | { kind: 'input'; label: 'Input'; text: string } + | { kind: 'diff'; diff: ToolCallDiff } + | { kind: 'terminal_refs'; label: 'Terminal'; refs: string[] } + | { kind: 'output'; label: string; text: string; tone: 'normal' | 'danger' | 'cancelled' } + | { kind: 'raw_output'; label: 'Raw output'; text: string } + | { kind: 'empty'; label: 'Waiting for output' | 'No output' } + | { + kind: 'status'; + label: string; + status: ToolStatus; + tone: RichToolItem['statusTone']; + }; + +export interface ToolCallViewModel { + key: string; + category: ToolCallCategory; + verb: string; + detail: string; + status: ToolStatus; + statusLabel: string; + statusTone: RichToolItem['statusTone']; + metadata: ToolCallMetadata; + output: ToolCallOutput; + sections: ToolCallSection[]; + hasDetails: boolean; +} + +const EDIT_KINDS = new Set([ + 'applypatch', + 'apply_patch', + 'delete', + 'edit', + 'editfile', + 'editnotebook', + 'multi_edit', + 'multiedit', + 'patch', + 'replace', + 'strreplace', + 'str_replace', + 'update', + 'write', + 'writefile', +]); + +const COMMAND_KINDS = new Set([ + 'bash', + 'cmd', + 'command', + 'exec', + 'execute', + 'run', + 'shell', + 'terminal', +]); + +const READ_KINDS = new Set(['cat', 'load', 'open', 'read', 'readfile', 'view']); + +const SEARCH_KINDS = new Set([ + 'find', + 'glob', + 'grep', + 'list', + 'ls', + 'rg', + 'search', + 'semanticsearch', +]); + +const NETWORK_KINDS = new Set(['browser', 'curl', 'fetch', 'http', 'network', 'request', 'web']); + +const READ_VERBS = new Set(['Read', 'Reading']); +const SEARCH_VERBS = new Set(['Searched', 'Searching', 'Listed', 'Listing']); +const EDIT_VERBS = new Set(['Deleted', 'Deleting', 'Edited', 'Editing', 'Wrote', 'Writing']); +const COMMAND_VERBS = new Set(['Ran', 'Running']); + +const PATH_KEYS = ['file_path', 'filePath', 'path', 'filepath', 'filename', 'file']; +const QUERY_KEYS = ['query', 'pattern', 'glob', 'selector']; +const COMMAND_KEYS = ['command', 'cmd', 'script']; +const CWD_KEYS = ['cwd', 'workingDirectory', 'working_directory', 'workdir']; +const URL_KEYS = ['url', 'uri', 'href']; +const METHOD_KEYS = ['method', 'httpMethod', 'http_method']; +const NETWORK_KEYS = new Set([ + 'body', + 'headers', + 'method', + 'request', + 'response', + 'screenshot', + 'selector', + 'status', + 'statusCode', + 'status_code', + 'title', + 'url', +]); + +export function buildToolCallViewModel( + item: RichToolItem, + displayRoots?: DisplayRootInput +): ToolCallViewModel { + const metadata = extractToolCallMetadata(item, displayRoots); + const category = classifyToolCall(item, metadata); + const output = extractToolCallOutput(item); + const sections = buildToolCallSections(item, metadata, output); + + return { + key: item.key, + category, + verb: item.verb, + detail: item.detail, + status: item.status, + statusLabel: item.statusLabel, + statusTone: item.statusTone, + metadata, + output, + sections, + hasDetails: sections.length > 0, + }; +} + +export function extractToolCallMetadata( + item: RichToolItem, + displayRoots?: DisplayRootInput +): ToolCallMetadata { + const parsed = parseToolCall(item.call.content); + const parsedArgs = parsed ? recordOrNull(parsed.args) : null; + const rawInput = recordOrNull(item.rawInput); + const input = rawInput ?? parsedArgs; + const inputText = + item.rawInput !== undefined && item.rawInput !== null + ? formatJson(item.rawInput) + : parsedArgs && Object.keys(parsedArgs).length > 0 + ? formatJson(parsedArgs) + : ''; + const parsedCommands = [...parsedCommandsFrom(input), ...parsedCommandsFrom(parsedArgs)].filter( + uniqueParsedCommand + ); + const diffs = extractToolCallDiffs(item.content, displayRoots); + const locations = extractToolCallLocations(item.locations, displayRoots); + + return { + toolKind: normalizeToken(item.toolKind), + toolName: parsed?.name ?? null, + input, + inputText, + parsedCommands, + command: relativeValue( + firstString(input, COMMAND_KEYS) ?? firstCommandValue(parsedCommands), + displayRoots + ), + workingDirectory: relativeValue(firstString(input, CWD_KEYS), displayRoots), + targetPath: relativeValue( + firstString(input, PATH_KEYS) ?? + firstParsedCommandField(parsedCommands, 'path') ?? + firstParsedCommandField(parsedCommands, 'name') ?? + diffs[0]?.path ?? + locations[0]?.path ?? + null, + displayRoots + ), + query: firstString(input, QUERY_KEYS) ?? firstParsedCommandField(parsedCommands, 'query'), + url: firstString(input, URL_KEYS), + method: normalizeHttpMethod(firstString(input, METHOD_KEYS)), + locations, + diffs, + terminalRefs: terminalRefsFromAcpContent(item.content), + }; +} + +export function classifyToolCall( + item: RichToolItem, + metadata = extractToolCallMetadata(item) +): ToolCallCategory { + const toolKindCategory = categoryFromToken(item.toolKind); + if (toolKindCategory) return toolKindCategory; + + const parsedNameCategory = categoryFromTitle(metadata.toolName); + if (parsedNameCategory) return parsedNameCategory; + + const parsedCommandCategory = categoryFromParsedCommands(metadata.parsedCommands); + if (parsedCommandCategory) return parsedCommandCategory; + + const verbCategory = categoryFromVerb(item.verb); + if (verbCategory) return verbCategory; + + if (metadata.diffs.length > 0) return 'edit'; + if (metadata.terminalRefs.length > 0) return 'command'; + if (hasNetworkMetadata(metadata.input) || hasNetworkMetadata(item.rawOutput)) return 'network'; + + return 'generic'; +} + +export function extractToolCallOutput(item: RichToolItem): ToolCallOutput { + const rawRecord = recordOrNull(item.rawOutput); + const rawText = formatJson(item.rawOutput); + const contentText = textFromAcpContent(item.content); + const errorText = extractErrorText(rawRecord, item.status); + const stdout = valueText(firstValue(rawRecord, ['stdout', 'standardOutput', 'standard_output'])); + const stderr = valueText(firstValue(rawRecord, ['stderr', 'standardError', 'standard_error'])); + const exitCode = firstNumber(rawRecord, ['exitCode', 'exit_code', 'code']); + const structuredOutput = valueText( + firstValue(rawRecord, ['output', 'text', 'body', 'response', 'content', 'result']) + ); + const resultText = item.result?.content ? stripCodeFences(item.result.content) : ''; + const primaryText = + contentText || + (typeof item.rawOutput === 'string' ? item.rawOutput : '') || + structuredOutput || + stdout || + stderr || + resultText; + const hasAnyOutput = !!(primaryText || rawText || errorText || stdout || stderr); + const isWaiting = item.status === 'pending' || item.status === 'in_progress'; + + return { + state: hasAnyOutput ? 'output' : isWaiting ? 'waiting' : 'empty', + primaryText, + rawText, + errorText, + stdout, + stderr, + exitCode, + emptyLabel: hasAnyOutput ? null : isWaiting ? 'Waiting for output' : 'No output', + }; +} + +export function buildToolCallSections( + item: Pick, + metadata: ToolCallMetadata, + output: ToolCallOutput +): ToolCallSection[] { + const sections: ToolCallSection[] = []; + + if (metadata.locations.length > 0) { + sections.push({ kind: 'locations', locations: metadata.locations }); + } + + if (metadata.inputText) { + sections.push({ kind: 'input', label: 'Input', text: metadata.inputText }); + } + + for (const diff of metadata.diffs) { + sections.push({ kind: 'diff', diff }); + } + + if (metadata.terminalRefs.length > 0) { + sections.push({ kind: 'terminal_refs', label: 'Terminal', refs: metadata.terminalRefs }); + } + + const emittedOutputTexts = new Set(); + const outputTone = item.status === 'cancelled' ? 'cancelled' : 'normal'; + if (output.errorText) { + sections.push({ + kind: 'output', + label: 'Error', + text: output.errorText, + tone: 'danger', + }); + emittedOutputTexts.add(output.errorText); + } + + if (output.stdout) { + sections.push({ + kind: 'output', + label: 'Stdout', + text: output.stdout, + tone: outputTone, + }); + emittedOutputTexts.add(output.stdout); + } + + if (output.stderr && output.stderr !== output.errorText) { + sections.push({ + kind: 'output', + label: 'Stderr', + text: output.stderr, + tone: item.status === 'failed' ? 'danger' : outputTone, + }); + emittedOutputTexts.add(output.stderr); + } + + if ( + output.primaryText && + output.primaryText !== output.errorText && + output.primaryText !== output.stdout && + output.primaryText !== output.stderr + ) { + sections.push({ + kind: 'output', + label: 'Output', + text: output.primaryText, + tone: outputTone, + }); + emittedOutputTexts.add(output.primaryText); + } + + if (output.rawText && !emittedOutputTexts.has(output.rawText)) { + sections.push({ kind: 'raw_output', label: 'Raw output', text: output.rawText }); + } + + if (output.emptyLabel) { + sections.push({ kind: 'empty', label: output.emptyLabel }); + } + + sections.push({ + kind: 'status', + label: item.statusLabel, + status: item.status, + tone: item.statusTone, + }); + + return sections; +} + +export function extractToolCallDiffs( + content: unknown, + displayRoots?: DisplayRootInput +): ToolCallDiff[] { + if (!Array.isArray(content)) return []; + + const diffs: ToolCallDiff[] = []; + for (const block of content) { + if (stringProp(block, 'type') !== 'diff') continue; + const path = stringProp(block, 'path'); + if (!path) continue; + + const oldText = nullableStringProp(block, 'oldText'); + const newText = nullableStringProp(block, 'newText'); + if (oldText === null && newText === null) continue; + + diffs.push({ + path: makePathsRelative(path, displayRoots), + oldText, + newText, + kind: diffKind(oldText, newText), + }); + } + + return diffs; +} + +export function extractToolCallLocations( + locations: unknown, + displayRoots?: DisplayRootInput +): ToolCallLocation[] { + if (!Array.isArray(locations)) return []; + + return locations + .map((location) => { + const path = stringProp(location, 'path'); + if (!path) return null; + const displayPath = makePathsRelative(path, displayRoots); + const line = numberProp(location, 'line'); + const column = numberProp(location, 'column'); + const suffix = `${line !== null ? `:${line}` : ''}${column !== null ? `:${column}` : ''}`; + return { + path: displayPath, + display: `${displayPath}${suffix}`, + line, + column, + }; + }) + .filter((location): location is ToolCallLocation => location !== null); +} + +export function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +export function stringProp(value: unknown, key: string): string | null { + if (!isRecord(value)) return null; + const prop = value[key]; + return typeof prop === 'string' ? prop : null; +} + +function nullableStringProp(value: unknown, key: string): string | null { + if (!isRecord(value)) return null; + const prop = value[key]; + return typeof prop === 'string' ? prop : null; +} + +function numberProp(value: unknown, key: string): number | null { + if (!isRecord(value)) return null; + const prop = value[key]; + return typeof prop === 'number' && Number.isFinite(prop) ? prop : null; +} + +function recordOrNull(value: unknown): Record | null { + return isRecord(value) ? value : null; +} + +function normalizeToken(value: string | null | undefined): string | null { + if (!value) return null; + return value.trim().toLowerCase(); +} + +function compactToken(value: string | null | undefined): string | null { + const normalized = normalizeToken(value); + return normalized ? normalized.replace(/[^a-z0-9]/g, '') : null; +} + +function categoryFromToken(value: string | null | undefined): ToolCallCategory | null { + const normalized = normalizeToken(value); + const compact = compactToken(value); + if (!normalized || !compact) return null; + + if (EDIT_KINDS.has(normalized) || EDIT_KINDS.has(compact)) return 'edit'; + if (COMMAND_KINDS.has(normalized) || COMMAND_KINDS.has(compact)) return 'command'; + if (READ_KINDS.has(normalized) || READ_KINDS.has(compact)) return 'read'; + if (SEARCH_KINDS.has(normalized) || SEARCH_KINDS.has(compact)) return 'search'; + if (NETWORK_KINDS.has(normalized) || NETWORK_KINDS.has(compact)) return 'network'; + + return null; +} + +function categoryFromTitle(title: string | null): ToolCallCategory | null { + if (!title) return null; + const firstWord = title.trim().split(/\s+/, 1)[0]; + return categoryFromToken(title) ?? categoryFromToken(firstWord); +} + +function categoryFromParsedCommands(commands: ParsedToolCommand[]): ToolCallCategory | null { + for (const command of commands) { + const typeCategory = categoryFromToken(command.type); + if (typeCategory) return typeCategory; + if (command.cmd) return 'command'; + } + return null; +} + +function categoryFromVerb(verb: string): ToolCallCategory | null { + if (EDIT_VERBS.has(verb)) return 'edit'; + if (COMMAND_VERBS.has(verb)) return 'command'; + if (READ_VERBS.has(verb)) return 'read'; + if (SEARCH_VERBS.has(verb)) return 'search'; + return null; +} + +function parsedCommandsFrom(input: Record | null): ParsedToolCommand[] { + const parsedCmd = input?.parsed_cmd ?? input?.parsedCmd; + if (!Array.isArray(parsedCmd)) return []; + + return parsedCmd.filter(isRecord).map((command) => ({ + type: stringProp(command, 'type'), + cmd: stringProp(command, 'cmd') ?? stringProp(command, 'command'), + path: stringProp(command, 'path'), + query: stringProp(command, 'query'), + name: stringProp(command, 'name'), + })); +} + +function uniqueParsedCommand( + command: ParsedToolCommand, + index: number, + commands: ParsedToolCommand[] +): boolean { + return ( + commands.findIndex( + (candidate) => + candidate.type === command.type && + candidate.cmd === command.cmd && + candidate.path === command.path && + candidate.query === command.query && + candidate.name === command.name + ) === index + ); +} + +function firstString(input: Record | null, keys: string[]): string | null { + if (!input) return null; + for (const key of keys) { + const value = input[key]; + if (typeof value === 'string' && value.trim()) return value; + } + return null; +} + +function firstParsedCommandField( + commands: ParsedToolCommand[], + field: keyof ParsedToolCommand +): string | null { + for (const command of commands) { + const value = command[field]; + if (typeof value === 'string' && value.trim()) return value; + } + return null; +} + +function firstCommandValue(commands: ParsedToolCommand[]): string | null { + return firstParsedCommandField(commands, 'cmd'); +} + +function firstValue(input: Record | null, keys: string[]): unknown { + if (!input) return undefined; + for (const key of keys) { + if (input[key] !== undefined && input[key] !== null) return input[key]; + } + return undefined; +} + +function firstNumber(input: Record | null, keys: string[]): number | null { + if (!input) return null; + for (const key of keys) { + const value = input[key]; + if (typeof value === 'number' && Number.isFinite(value)) return value; + } + return null; +} + +function valueText(value: unknown): string { + if (value === undefined || value === null) return ''; + if (typeof value === 'string') return value; + if (typeof value === 'number' || typeof value === 'boolean') return String(value); + return formatJson(value); +} + +function extractErrorText(rawOutput: Record | null, status: ToolStatus): string { + const error = firstValue(rawOutput, ['error', 'errorMessage', 'error_message']); + if (error !== undefined && error !== null) return valueText(error); + + if (status !== 'failed') return ''; + const stderr = valueText(firstValue(rawOutput, ['stderr', 'standardError', 'standard_error'])); + return stderr; +} + +function normalizeHttpMethod(method: string | null): string | null { + return method ? method.toUpperCase() : null; +} + +function relativeValue(value: string | null, displayRoots?: DisplayRootInput): string | null { + return value ? makePathsRelative(value, displayRoots) : null; +} + +function hasNetworkMetadata(value: unknown): boolean { + if (!isRecord(value)) return false; + return Object.keys(value).some((key) => NETWORK_KEYS.has(key)); +} + +function diffKind(oldText: string | null, newText: string | null): ToolCallDiffKind { + if (oldText === null) return 'created'; + if (newText === null) return 'deleted'; + return oldText === newText ? 'unchanged' : 'modified'; +} From d99156d55149cfacd6cedfcbc9e8cdd98c5d77b1 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 7 Jul 2026 17:20:05 +1000 Subject: [PATCH 2/9] feat(sessions): render expanded tool call details Extract expanded tool call cards into Svelte components and dispatch edit, command, read/search, network, and generic renderers from the tool call view model. Signed-off-by: Matt Toohey --- .../features/sessions/SessionChatPane.svelte | 379 ++---------------- .../tool-calls/CommandToolDetails.svelte | 42 ++ .../tool-calls/EditToolDetails.svelte | 44 ++ .../tool-calls/GenericToolDetails.svelte | 48 +++ .../sessions/tool-calls/InlineToolDiff.svelte | 265 ++++++++++++ .../tool-calls/NetworkToolDetails.svelte | 183 +++++++++ .../sessions/tool-calls/OutputSections.svelte | 143 +++++++ .../tool-calls/ReadSearchToolDetails.svelte | 115 ++++++ .../sessions/tool-calls/ToolCallCard.svelte | 114 ++++++ .../tool-calls/ToolCallDetails.svelte | 264 ++++++++++++ .../sessions/tool-calls/ToolCallHeader.svelte | 113 ++++++ .../sessions/tool-calls/ToolStatusDot.svelte | 62 +++ .../sessions/toolCallViewModel.test.ts | 3 + 13 files changed, 1431 insertions(+), 344 deletions(-) create mode 100644 apps/staged/src/lib/features/sessions/tool-calls/CommandToolDetails.svelte create mode 100644 apps/staged/src/lib/features/sessions/tool-calls/EditToolDetails.svelte create mode 100644 apps/staged/src/lib/features/sessions/tool-calls/GenericToolDetails.svelte create mode 100644 apps/staged/src/lib/features/sessions/tool-calls/InlineToolDiff.svelte create mode 100644 apps/staged/src/lib/features/sessions/tool-calls/NetworkToolDetails.svelte create mode 100644 apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte create mode 100644 apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte create mode 100644 apps/staged/src/lib/features/sessions/tool-calls/ToolCallCard.svelte create mode 100644 apps/staged/src/lib/features/sessions/tool-calls/ToolCallDetails.svelte create mode 100644 apps/staged/src/lib/features/sessions/tool-calls/ToolCallHeader.svelte create mode 100644 apps/staged/src/lib/features/sessions/tool-calls/ToolStatusDot.svelte diff --git a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte index e494067f..f1cc61e5 100644 --- a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte +++ b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte @@ -35,18 +35,12 @@ import CircleStop from '@lucide/svelte/icons/circle-stop'; import Copy from '@lucide/svelte/icons/copy'; import Check from '@lucide/svelte/icons/check'; - import Clock from '@lucide/svelte/icons/clock'; - import CircleAlert from '@lucide/svelte/icons/circle-alert'; - import CircleCheck from '@lucide/svelte/icons/circle-check'; - import CircleDot from '@lucide/svelte/icons/circle-dot'; - import CircleSlash from '@lucide/svelte/icons/circle-slash'; import ChevronRight from '@lucide/svelte/icons/chevron-right'; import ChevronDown from '@lucide/svelte/icons/chevron-down'; import Zap from '@lucide/svelte/icons/zap'; import GitBranch from '@lucide/svelte/icons/git-branch'; import BookOpen from '@lucide/svelte/icons/book-open'; import FileText from '@lucide/svelte/icons/file-text'; - import ExternalLink from '@lucide/svelte/icons/external-link'; import ImagePlus from '@lucide/svelte/icons/image-plus'; import Spinner from '../../shared/Spinner.svelte'; import { isResumableReason } from '../../types'; @@ -108,22 +102,15 @@ import { hasXmlBlocks, sessionEndMessage, XML_BLOCK_TAGS } from './sessionModalHelpers'; import { buildAcpTranscriptGroups, - diffsFromAcpContent, - displayLocations, formatJson, groupRichToolsByVerb, isToolMetadataSettled, latestAvailableCommands, - simpleUnifiedDiff, stabilizeAcpTranscriptGroups, - terminalRefsFromAcpContent, - toolHasDetails, - toolResultText, transcriptGroupKey, type AcpCommand, type AcpTranscriptEvent, type AcpTranscriptGroup, - type RichToolItem, } from './acpTranscript'; import { displayRootKey, @@ -132,6 +119,8 @@ type DisplayRootInput, } from './pathDisplayRoots'; import PipelineSteps from './PipelineSteps.svelte'; + import ToolCallCard from './tool-calls/ToolCallCard.svelte'; + import ToolCallHeader from './tool-calls/ToolCallHeader.svelte'; import { highlightMatches, clearHighlights, scrollToMatch } from '../../shared/textHighlight'; import '../../shared/markdown/diagramStyles.css'; import { @@ -1739,131 +1728,11 @@ -{#snippet toolStatusDot(statusTone: RichToolItem['statusTone'])} - - {#if statusTone === 'running'} - - {:else if statusTone === 'success'} - - {:else if statusTone === 'danger'} - - {:else if statusTone === 'cancelled'} - - {:else} - - {/if} - -{/snippet} - -{#snippet richToolCard(item: RichToolItem, nested: boolean)} - {@const isExpanded = expandedTools.has(item.key)} - {@const hasDetails = toolHasDetails(item)} - {@const showSessionButton = !!(item.isPikchrDiagramTool && item.innerSessionId && onOpenSession)} - {@const showInlineDiagram = item.pikchrRenderSource !== null} -
- {#if showInlineDiagram} -
- {@html renderMarkdown(fencedDiagramMarkdown('pikchr', item.pikchrRenderSource!))} -
- {:else if showSessionButton} - - {:else} - - -
hasDetails && toggleTool(item.key)} - > - › - {@render toolStatusDot(item.statusTone)} - {item.verb} - {#if item.detail} - {item.detail} - {/if} -
- {/if} - {#if isExpanded && hasDetails && !showSessionButton && !showInlineDiagram} - - {@const resultText = toolResultText(item)} - {@const rawInputText = formatJson(item.rawInput)} - {@const rawOutputText = formatJson(item.rawOutput)} - {@const diffs = diffsFromAcpContent(item.content, displayRoots)} - {@const locations = displayLocations(item.locations, displayRoots)} - {@const terminalRefs = terminalRefsFromAcpContent(item.content)} -
- {#if (item.verb === 'Ran' || item.verb === 'Running') && item.detail} -
$ {item.detail}
- {/if} - {#if locations.length > 0} -
- {#each locations as location} - {location} - {/each} -
- {/if} - {#if rawInputText} -
Input
- 
{rawInputText}
- {/if} - {#each diffs as diff} -
{diff.path}
- 
{simpleUnifiedDiff(diff)}
- {/each} - {#if terminalRefs.length > 0} -
Terminal
-
- {#each terminalRefs as terminalRef} - {terminalRef} - {/each} -
- {/if} - {#if resultText} -
Output
- 
{resultText}
- {/if} - {#if rawOutputText && rawOutputText !== resultText} -
Raw output
- 
{rawOutputText}
- {/if} -
- {#if item.statusTone === 'success'} - - {/if} - {item.statusLabel} -
-
- {/if} + +{#snippet pikchrDiagram(source: string)} +
+ {@html renderMarkdown(fencedDiagramMarkdown('pikchr', source))}
{/snippet} @@ -2041,27 +1910,37 @@
{#each groupRichToolsByVerb(group.items) as verbGroup (verbGroup.key)} {#if verbGroup.items.length === 1} - {@render richToolCard(verbGroup.items[0], false)} + {:else} {@const isGroupExpanded = expandedTools.has(verbGroup.key)} -
- - -
toggleTool(verbGroup.key)} - > - › - {@render toolStatusDot(verbGroup.statusTone)} - {verbGroup.verb} - {verbGroup.summary} -
-
+ toggleTool(verbGroup.key)} + /> {#if isGroupExpanded}
{#each verbGroup.items as item (item.key)} - {@render richToolCard(item, true)} + {/each}
{/if} @@ -2606,61 +2485,6 @@ min-width: 0; } - .tool-card { - overflow: hidden; - min-width: 0; - } - - .tool-card-nested { - padding-left: 16px; - } - - .tool-header { - display: flex; - align-items: center; - gap: 4px; - padding: 2px 0; - background: none; - color: var(--text-muted); - font-size: var(--size-xs); - transition: background-color 0.1s; - cursor: default; - min-width: 0; - } - - .tool-header-expandable { - cursor: pointer; - } - - .tool-header-expandable:hover .tool-name { - text-decoration: underline; - } - - :global(.tool-session-button) { - height: 26px; - gap: 6px; - border-color: var(--border-muted); - padding: 0 8px; - color: var(--text-muted); - font-size: var(--size-xs); - font-weight: 500; - box-shadow: none; - } - - :global(.tool-session-button:hover) { - border-color: var(--border-emphasis); - background: var(--bg-hover); - color: var(--text-primary); - } - - /* A failed generate_pikchr call keeps its session button (the child session - records the failure); tint it so the error is visible without expanding. */ - :global(.tool-session-button-danger), - :global(.tool-session-button-danger:hover) { - border-color: var(--ui-danger); - color: var(--ui-danger); - } - /* Inline diagram from a successful render_pikchr call — rendered through the markdown pipeline, so it picks up the shared diagram styles (fullscreen zoom affordance included); only the message margins are tightened to sit @@ -2673,159 +2497,26 @@ display: inline-block; flex-shrink: 0; width: 8px; - font-size: var(--size-xs); color: var(--text-faint); - transition: transform 0.15s ease; + font-size: var(--size-xs); line-height: 1; + transition: transform 0.15s ease; } .tool-caret-expanded { transform: rotate(90deg); } - .tool-caret-hidden { - visibility: hidden; - } - - .tool-name { - flex-shrink: 0; - color: var(--text-muted); - font-size: var(--size-xs); - white-space: nowrap; - } - - .tool-status-dot { - display: inline-flex; - align-items: center; - justify-content: center; - width: 12px; - height: 12px; - flex-shrink: 0; - color: var(--text-faint); - } - - .tool-status-dot.status-running { - color: var(--ui-warning); - } - - .tool-status-dot.status-success { - color: var(--ui-success, var(--ui-accent)); - } - - .tool-status-dot.status-danger { - color: var(--ui-danger); - } - - .tool-status-dot.status-cancelled { - color: var(--text-muted); - } - .tool-args-preview { flex: 1; min-width: 0; overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; color: var(--text-faint); font-size: var(--size-xs); - } - - .tool-code-block { - background: color-mix(in srgb, var(--bg-chrome) 80%, black); - border-radius: 8px; - padding: 12px 14px; - margin-top: 4px; - max-height: 240px; - overflow-y: auto; - font-family: 'SF Mono', 'Menlo', 'Monaco', 'Courier New', monospace; - font-size: calc(var(--size-xs) * 0.9); - line-height: 1.5; - } - - .tool-code-command { - color: var(--text-primary); - font-weight: 500; - margin-bottom: 6px; - white-space: pre-wrap; - word-break: break-all; - } - - .tool-code-output { - margin: 0; - color: var(--text-muted); - white-space: pre-wrap; - word-break: break-word; - } - - .diff-output { - color: var(--text-primary); - } - - .tool-panel-label { - margin: 10px 0 4px; - color: var(--text-faint); - font-family: inherit; - font-size: calc(var(--size-xs) * 0.86); - font-weight: 600; - text-transform: uppercase; - letter-spacing: 0.04em; - } - - .tool-panel-label:first-child { - margin-top: 0; - } - - .tool-meta-row { - display: flex; - flex-wrap: wrap; - gap: 4px; - margin: 4px 0 8px; - } - - .tool-chip { - max-width: 100%; - overflow: hidden; text-overflow: ellipsis; - border: 1px solid var(--border-subtle); - border-radius: 6px; - padding: 2px 6px; - color: var(--text-muted); - font-family: inherit; - font-size: calc(var(--size-xs) * 0.88); white-space: nowrap; } - .tool-code-status { - display: flex; - align-items: center; - justify-content: flex-end; - gap: 3px; - margin-top: 8px; - font-size: calc(var(--size-xs) * 0.85); - color: var(--text-muted); - } - - .tool-code-status.status-danger { - color: var(--ui-danger); - } - - .tool-code-status.status-cancelled { - color: var(--text-faint); - } - - .tool-code-block::-webkit-scrollbar { - width: 4px; - } - - .tool-code-block::-webkit-scrollbar-track { - background: transparent; - } - - .tool-code-block::-webkit-scrollbar-thumb { - background: var(--scrollbar-thumb-transparent); - border-radius: 2px; - } - /* ACP metadata cards */ .acp-event-row { display: flex; diff --git a/apps/staged/src/lib/features/sessions/tool-calls/CommandToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/CommandToolDetails.svelte new file mode 100644 index 00000000..dabaae97 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/tool-calls/CommandToolDetails.svelte @@ -0,0 +1,42 @@ + + +
+
+
+ $ + {commandText} +
+
+ {#if viewModel.metadata.workingDirectory} + Directory + {viewModel.metadata.workingDirectory} + {/if} + {#if viewModel.output.exitCode !== null} + Exit + {viewModel.output.exitCode} + {/if} + Status + {viewModel.statusLabel} +
+
+ + {#if viewModel.metadata.terminalRefs.length > 0} +
+ {#each viewModel.metadata.terminalRefs as terminalRef} + {terminalRef} + {/each} +
+ {/if} + + +
diff --git a/apps/staged/src/lib/features/sessions/tool-calls/EditToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/EditToolDetails.svelte new file mode 100644 index 00000000..dbf61592 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/tool-calls/EditToolDetails.svelte @@ -0,0 +1,44 @@ + + +
+ {#if viewModel.metadata.targetPath} +
+ Path + {viewModel.metadata.targetPath} +
+ {/if} + + {#if viewModel.metadata.locations.length > 0} +
+ {#each viewModel.metadata.locations as location} + {location.display} + {/each} +
+ {/if} + + {#if viewModel.metadata.diffs.length > 0} + {#each viewModel.metadata.diffs as diff} +
+
{diff.path}
+ +
+ {/each} + {:else if viewModel.metadata.inputText} +
+
Input
+ 
{viewModel.metadata.inputText}
+
+ {/if} + + +
diff --git a/apps/staged/src/lib/features/sessions/tool-calls/GenericToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/GenericToolDetails.svelte new file mode 100644 index 00000000..7aba0746 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/tool-calls/GenericToolDetails.svelte @@ -0,0 +1,48 @@ + + +
+ {#if viewModel.metadata.locations.length > 0} +
+ {#each viewModel.metadata.locations as location} + {location.display} + {/each} +
+ {/if} + + {#if viewModel.metadata.inputText} +
+
Input
+ 
{viewModel.metadata.inputText}
+
+ {/if} + + {#each viewModel.metadata.diffs as diff} +
+
{diff.path}
+ +
+ {/each} + + {#if viewModel.metadata.terminalRefs.length > 0} +
+
Terminal
+
+ {#each viewModel.metadata.terminalRefs as terminalRef} + {terminalRef} + {/each} +
+
+ {/if} + + +
diff --git a/apps/staged/src/lib/features/sessions/tool-calls/InlineToolDiff.svelte b/apps/staged/src/lib/features/sessions/tool-calls/InlineToolDiff.svelte new file mode 100644 index 00000000..921dd266 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/tool-calls/InlineToolDiff.svelte @@ -0,0 +1,265 @@ + + +
+ {#if rows.length === 0} +
No changes
+ {:else} + {#each rows as row (row.key)} +
+ {lineNumber(row.oldLine)} + {lineNumber(row.newLine)} + {row.marker} + {@html highlightedLineHtml(row.text, row.highlights)} +
+ {/each} + {/if} +
+ + diff --git a/apps/staged/src/lib/features/sessions/tool-calls/NetworkToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/NetworkToolDetails.svelte new file mode 100644 index 00000000..a7034acb --- /dev/null +++ b/apps/staged/src/lib/features/sessions/tool-calls/NetworkToolDetails.svelte @@ -0,0 +1,183 @@ + + +
+ {#if hasNetworkPreview} +
+ {#if network.method || network.url || network.status} +
+ {#if network.method} + {network.method} + {/if} + {#if network.url} + {network.url} + {/if} + {#if network.status} + {network.status} + {/if} +
+ {/if} +
+ {#if network.title} + Title + {network.title} + {/if} + {#if network.selector} + Selector + {network.selector} + {/if} + {#if network.screenshot} + Screenshot + {network.screenshot} + {/if} +
+
+ + {#if network.requestHeaders} +
+
Request headers
+ 
{network.requestHeaders}
+
+ {/if} + {#if network.requestBody} +
+
Request body
+ 
{network.requestBody}
+
+ {/if} + {#if network.responseHeaders} +
+
Response headers
+ 
{network.responseHeaders}
+
+ {/if} + {#if network.responseBody} +
+
Response body
+ 
{network.responseBody}
+
+ {/if} + {/if} + + +
diff --git a/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte b/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte new file mode 100644 index 00000000..1004152c --- /dev/null +++ b/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte @@ -0,0 +1,143 @@ + + +{#each blocks as block} +
+
+
{block.label}
+ {#if copyable} + + {/if} +
+ 
{block.text}
+
+{/each} + +{#if viewModel.output.emptyLabel} +
{viewModel.output.emptyLabel}
+{/if} + +{#if includeStatus} +
+ {#if viewModel.statusTone === 'success'} + + {/if} + {viewModel.statusLabel} +
+{/if} diff --git a/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte new file mode 100644 index 00000000..7870ac39 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte @@ -0,0 +1,115 @@ + + +
+
+ {#if viewModel.metadata.targetPath} + Path + {viewModel.metadata.targetPath} + {/if} + {#if viewModel.metadata.query} + {viewModel.category === 'search' ? 'Query' : 'Selector'} + {viewModel.metadata.query} + {/if} +
+ + {#if viewModel.metadata.locations.length > 0} +
+ {#each viewModel.metadata.locations as location} + {location.display} + {/each} +
+ {/if} + + {#if matches.length > 0} +
+
Matches
+
+ {#each matches as match} +
+ {match.location} + {match.snippet} +
+ {/each} +
+
+ {/if} + + +
diff --git a/apps/staged/src/lib/features/sessions/tool-calls/ToolCallCard.svelte b/apps/staged/src/lib/features/sessions/tool-calls/ToolCallCard.svelte new file mode 100644 index 00000000..667c25ee --- /dev/null +++ b/apps/staged/src/lib/features/sessions/tool-calls/ToolCallCard.svelte @@ -0,0 +1,114 @@ + + +
+ {#if showInlineDiagram} + {@render diagram?.(item.pikchrRenderSource!)} + {:else if showSessionButton} + + {:else} + onToggle(item.key)} + /> + {/if} + {#if expanded && viewModel.hasDetails && !showInlineDiagram && !showSessionButton} +
+ +
+ {/if} +
+ + diff --git a/apps/staged/src/lib/features/sessions/tool-calls/ToolCallDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/ToolCallDetails.svelte new file mode 100644 index 00000000..961ee3db --- /dev/null +++ b/apps/staged/src/lib/features/sessions/tool-calls/ToolCallDetails.svelte @@ -0,0 +1,264 @@ + + +
+ {#if viewModel.category === 'edit'} + + {:else if viewModel.category === 'command'} + + {:else if viewModel.category === 'read' || viewModel.category === 'search'} + + {:else if viewModel.category === 'network'} + + {:else} + + {/if} +
+ + diff --git a/apps/staged/src/lib/features/sessions/tool-calls/ToolCallHeader.svelte b/apps/staged/src/lib/features/sessions/tool-calls/ToolCallHeader.svelte new file mode 100644 index 00000000..e3d987a0 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/tool-calls/ToolCallHeader.svelte @@ -0,0 +1,113 @@ + + + + + diff --git a/apps/staged/src/lib/features/sessions/tool-calls/ToolStatusDot.svelte b/apps/staged/src/lib/features/sessions/tool-calls/ToolStatusDot.svelte new file mode 100644 index 00000000..456e1663 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/tool-calls/ToolStatusDot.svelte @@ -0,0 +1,62 @@ + + + + {#if statusTone === 'running'} + + {:else if statusTone === 'success'} + + {:else if statusTone === 'danger'} + + {:else if statusTone === 'cancelled'} + + {:else} + + {/if} + + + diff --git a/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts b/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts index 8fc20838..9e165e8e 100644 --- a/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts +++ b/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts @@ -31,6 +31,9 @@ function richTool(overrides: Partial = {}): RichToolItem { rawOutput: undefined, content: undefined, locations: undefined, + isPikchrDiagramTool: false, + innerSessionId: null, + pikchrRenderSource: null, ...overrides, }; } From 2f8b160e1522e32e9ef975c669f5f27e7035b607 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 8 Jul 2026 17:41:57 +1000 Subject: [PATCH 3/9] fix(sessions): show raw tool output only as a fallback Stop rendering raw JSON alongside structured tool call details. Raw output now appears only when no structured blocks (error, stdout, stderr, output, network response) could be extracted, so typed tool calls render just their designed UI. Signed-off-by: Matt Toohey --- .../sessions/tool-calls/NetworkToolDetails.svelte | 15 ++++++++++++++- .../sessions/tool-calls/OutputSections.svelte | 9 +++------ .../features/sessions/toolCallViewModel.test.ts | 6 +++--- .../lib/features/sessions/toolCallViewModel.ts | 14 ++++++++------ 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/apps/staged/src/lib/features/sessions/tool-calls/NetworkToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/NetworkToolDetails.svelte index a7034acb..d2806c50 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/NetworkToolDetails.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/NetworkToolDetails.svelte @@ -25,6 +25,15 @@ let { item, viewModel }: Props = $props(); let network = $derived(extractNetworkInfo(viewModel, item)); + let hasStructuredResponse = $derived( + !!( + network.status || + network.title || + network.screenshot || + network.responseHeaders || + network.responseBody + ) + ); let hasNetworkPreview = $derived( !!( network.method || @@ -179,5 +188,9 @@ {/if} {/if} - +
diff --git a/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte b/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte index 1004152c..6766e794 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte @@ -50,18 +50,15 @@ options: Pick ): OutputBlock[] { const output = model.output; - const emitted = new Set(); const result: OutputBlock[] = []; const defaultTone = model.status === 'cancelled' ? 'cancelled' : 'normal'; if (output.errorText) { result.push({ key: 'error', label: 'Error', text: output.errorText, tone: 'danger' }); - emitted.add(output.errorText); } if (options.includeStreams && output.stdout) { result.push({ key: 'stdout', label: 'Stdout', text: output.stdout, tone: defaultTone }); - emitted.add(output.stdout); } if (options.includeStreams && output.stderr && output.stderr !== output.errorText) { @@ -71,7 +68,6 @@ text: output.stderr, tone: model.status === 'failed' ? 'danger' : defaultTone, }); - emitted.add(output.stderr); } if ( @@ -82,10 +78,11 @@ output.primaryText !== output.stderr ) { result.push({ key: 'output', label: 'Output', text: output.primaryText, tone: defaultTone }); - emitted.add(output.primaryText); } - if (options.includeRaw && output.rawText && !emitted.has(output.rawText)) { + // Raw JSON is a fallback for output we could not render structurally, + // never a companion to structured blocks. + if (options.includeRaw && output.rawText && result.length === 0) { result.push({ key: 'raw-output', label: 'Raw output', diff --git a/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts b/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts index 9e165e8e..bf3b9eb1 100644 --- a/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts +++ b/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts @@ -202,7 +202,7 @@ describe('buildToolCallViewModel classification', () => { }); describe('buildToolCallViewModel output handling', () => { - it('promotes failed errors before raw output JSON', () => { + it('suppresses raw output JSON when a structured error is shown', () => { const model = buildToolCallViewModel( richTool({ status: 'failed', @@ -217,9 +217,8 @@ describe('buildToolCallViewModel output handling', () => { const errorIndex = model.sections.findIndex( (section) => section.kind === 'output' && section.label === 'Error' ); - const rawIndex = model.sections.findIndex((section) => section.kind === 'raw_output'); expect(errorIndex).toBeGreaterThanOrEqual(0); - expect(rawIndex).toBeGreaterThan(errorIndex); + expect(model.sections.some((section) => section.kind === 'raw_output')).toBe(false); }); it('keeps structured stdout and stderr distinct for successful commands', () => { @@ -236,6 +235,7 @@ describe('buildToolCallViewModel output handling', () => { section.kind === 'output' ? [section.label] : [] ); expect(outputLabels).toEqual(['Stdout', 'Stderr']); + expect(model.sections.some((section) => section.kind === 'raw_output')).toBe(false); }); it('falls back to legacy tool_result content when ACP output is absent', () => { diff --git a/apps/staged/src/lib/features/sessions/toolCallViewModel.ts b/apps/staged/src/lib/features/sessions/toolCallViewModel.ts index d0945096..65d4d260 100644 --- a/apps/staged/src/lib/features/sessions/toolCallViewModel.ts +++ b/apps/staged/src/lib/features/sessions/toolCallViewModel.ts @@ -311,7 +311,7 @@ export function buildToolCallSections( sections.push({ kind: 'terminal_refs', label: 'Terminal', refs: metadata.terminalRefs }); } - const emittedOutputTexts = new Set(); + let hasStructuredOutput = false; const outputTone = item.status === 'cancelled' ? 'cancelled' : 'normal'; if (output.errorText) { sections.push({ @@ -320,7 +320,7 @@ export function buildToolCallSections( text: output.errorText, tone: 'danger', }); - emittedOutputTexts.add(output.errorText); + hasStructuredOutput = true; } if (output.stdout) { @@ -330,7 +330,7 @@ export function buildToolCallSections( text: output.stdout, tone: outputTone, }); - emittedOutputTexts.add(output.stdout); + hasStructuredOutput = true; } if (output.stderr && output.stderr !== output.errorText) { @@ -340,7 +340,7 @@ export function buildToolCallSections( text: output.stderr, tone: item.status === 'failed' ? 'danger' : outputTone, }); - emittedOutputTexts.add(output.stderr); + hasStructuredOutput = true; } if ( @@ -355,10 +355,12 @@ export function buildToolCallSections( text: output.primaryText, tone: outputTone, }); - emittedOutputTexts.add(output.primaryText); + hasStructuredOutput = true; } - if (output.rawText && !emittedOutputTexts.has(output.rawText)) { + // Raw JSON is a fallback for output we could not render structurally, + // never a companion to structured output sections. + if (output.rawText && !hasStructuredOutput) { sections.push({ kind: 'raw_output', label: 'Raw output', text: output.rawText }); } From 7e4a33181c567ecd6935da7215ec60c698d1ea42 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 9 Jul 2026 10:36:23 +1000 Subject: [PATCH 4/9] feat(sessions): polish per-type tool call details Give each tool call renderer a tighter, type-specific UI: - Diffs get a header with the file path, a created/deleted badge, and add/remove counts, and long unchanged runs collapse behind an expandable "N unchanged lines" row (logic extracted to inlineDiffRows.ts with tests). - Command cards drop the redundant status row (already in the header dot and footer) and show the exit code only when non-zero. - Edit and read/search cards stop repeating the same path across the field row, location chips, and diff label. - Search matches are labelled with their count, and read output is labelled Content. Signed-off-by: Matt Toohey --- .../tool-calls/CommandToolDetails.svelte | 30 +- .../tool-calls/EditToolDetails.svelte | 23 +- .../tool-calls/GenericToolDetails.svelte | 5 +- .../sessions/tool-calls/InlineToolDiff.svelte | 260 +++++++++--------- .../sessions/tool-calls/OutputSections.svelte | 15 +- .../tool-calls/ReadSearchToolDetails.svelte | 16 +- .../tool-calls/inlineDiffRows.test.ts | 104 +++++++ .../sessions/tool-calls/inlineDiffRows.ts | 201 ++++++++++++++ 8 files changed, 489 insertions(+), 165 deletions(-) create mode 100644 apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.test.ts create mode 100644 apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.ts diff --git a/apps/staged/src/lib/features/sessions/tool-calls/CommandToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/CommandToolDetails.svelte index dabaae97..048d08d7 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/CommandToolDetails.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/CommandToolDetails.svelte @@ -8,6 +8,12 @@ let { viewModel }: Props = $props(); let commandText = $derived((viewModel.metadata.command ?? viewModel.detail) || 'Command'); + // Status already shows in the header dot and the footer; only surface failure exits here. + let failureExitCode = $derived( + viewModel.output.exitCode !== null && viewModel.output.exitCode !== 0 + ? viewModel.output.exitCode + : null + );
@@ -16,18 +22,18 @@ $ {commandText}
-
- {#if viewModel.metadata.workingDirectory} - Directory - {viewModel.metadata.workingDirectory} - {/if} - {#if viewModel.output.exitCode !== null} - Exit - {viewModel.output.exitCode} - {/if} - Status - {viewModel.statusLabel} -
+ {#if viewModel.metadata.workingDirectory || failureExitCode !== null} +
+ {#if viewModel.metadata.workingDirectory} + Directory + {viewModel.metadata.workingDirectory} + {/if} + {#if failureExitCode !== null} + Exit + {failureExitCode} + {/if} +
+ {/if}
{#if viewModel.metadata.terminalRefs.length > 0} diff --git a/apps/staged/src/lib/features/sessions/tool-calls/EditToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/EditToolDetails.svelte index dbf61592..189c05c5 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/EditToolDetails.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/EditToolDetails.svelte @@ -8,19 +8,31 @@ } let { viewModel }: Props = $props(); + // The diff header already names the file; only surface path metadata it doesn't cover. + let showPath = $derived( + !!viewModel.metadata.targetPath && + !viewModel.metadata.diffs.some((diff) => diff.path === viewModel.metadata.targetPath) + ); + let locations = $derived( + viewModel.metadata.locations.filter( + (location) => + location.display !== viewModel.metadata.targetPath && + !viewModel.metadata.diffs.some((diff) => diff.path === location.display) + ) + );
- {#if viewModel.metadata.targetPath} + {#if showPath}
Path {viewModel.metadata.targetPath}
{/if} - {#if viewModel.metadata.locations.length > 0} + {#if locations.length > 0}
- {#each viewModel.metadata.locations as location} + {#each locations as location} {location.display} {/each}
@@ -28,10 +40,7 @@ {#if viewModel.metadata.diffs.length > 0} {#each viewModel.metadata.diffs as diff} -
-
{diff.path}
- -
+ {/each} {:else if viewModel.metadata.inputText}
diff --git a/apps/staged/src/lib/features/sessions/tool-calls/GenericToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/GenericToolDetails.svelte index 7aba0746..56d11693 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/GenericToolDetails.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/GenericToolDetails.svelte @@ -27,10 +27,7 @@ {/if} {#each viewModel.metadata.diffs as diff} -
-
{diff.path}
- -
+ {/each} {#if viewModel.metadata.terminalRefs.length > 0} diff --git a/apps/staged/src/lib/features/sessions/tool-calls/InlineToolDiff.svelte b/apps/staged/src/lib/features/sessions/tool-calls/InlineToolDiff.svelte index 921dd266..44e8cfdd 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/InlineToolDiff.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/InlineToolDiff.svelte @@ -1,18 +1,8 @@ +{#snippet diffRow(row: DiffRow)} +
+ {row.oldLine ?? ''} + {row.newLine ?? ''} + {row.marker} + {@html highlightedLineHtml(row.text, row.highlights)} +
+{/snippet} +
+
+ {diff.path} + {#if diff.kind === 'created'} + Created + {:else if diff.kind === 'deleted'} + Deleted + {/if} + {#if stats.added > 0 || stats.removed > 0} + + {#if stats.added > 0}+{stats.added}{/if} + {#if stats.removed > 0}−{stats.removed}{/if} + + {/if} +
{#if rows.length === 0}
No changes
{:else} - {#each rows as row (row.key)} -
- {lineNumber(row.oldLine)} - {lineNumber(row.newLine)} - {row.marker} - {@html highlightedLineHtml(row.text, row.highlights)} -
+ {#each blocks as block (block.key)} + {#if block.kind === 'visible' || expandedKeys.includes(block.key)} + {#each block.rows as row (row.key)} + {@render diffRow(row)} + {/each} + {:else} + + {/if} {/each} {/if}
@@ -204,11 +115,88 @@ background: var(--bg-primary); } + .inline-diff-header { + position: sticky; + left: 0; + display: flex; + align-items: center; + gap: 8px; + min-width: 0; + padding: 6px 10px; + border-bottom: 1px solid var(--border-subtle); + background: color-mix(in srgb, var(--bg-chrome) 60%, var(--bg-primary)); + } + + .diff-path { + flex: 1; + min-width: 0; + overflow-wrap: anywhere; + color: var(--text-muted); + font-weight: 600; + } + + .diff-kind-badge { + flex-shrink: 0; + border: 1px solid var(--border-subtle); + border-radius: 6px; + padding: 1px 6px; + font-size: calc(var(--size-xs) * 0.85); + } + + .diff-kind-badge.created { + border-color: color-mix(in srgb, var(--ui-success, var(--ui-accent)) 35%, var(--border-subtle)); + color: var(--ui-success, var(--ui-accent)); + } + + .diff-kind-badge.deleted { + border-color: color-mix(in srgb, var(--ui-danger) 35%, var(--border-subtle)); + color: var(--ui-danger); + } + + .diff-stats { + display: flex; + flex-shrink: 0; + gap: 6px; + } + + .diff-stat-added { + color: var(--ui-success, var(--ui-accent)); + } + + .diff-stat-removed { + color: var(--ui-danger); + } + .inline-diff-empty { padding: 8px 10px; color: var(--text-faint); } + .diff-collapse-row { + position: sticky; + left: 0; + display: flex; + align-items: center; + justify-content: center; + gap: 6px; + width: 100%; + border: 0; + border-top: 1px dashed var(--border-subtle); + border-bottom: 1px dashed var(--border-subtle); + padding: 3px 8px; + background: color-mix(in srgb, var(--bg-chrome) 40%, var(--bg-primary)); + color: var(--text-faint); + font: inherit; + font-size: calc(var(--size-xs) * 0.85); + cursor: pointer; + } + + .diff-collapse-row:hover, + .diff-collapse-row:focus-visible { + color: var(--text-muted); + outline: none; + } + .diff-row { display: grid; grid-template-columns: 3.5ch 3.5ch 1.5ch minmax(0, 1fr); diff --git a/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte b/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte index 6766e794..b26d97b3 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte @@ -19,6 +19,7 @@ includeRaw?: boolean; includeStatus?: boolean; includeStreams?: boolean; + primaryLabel?: string; } let { @@ -28,10 +29,13 @@ includeRaw = true, includeStatus = true, includeStreams = true, + primaryLabel = 'Output', }: Props = $props(); let copiedKey = $state(null); - let blocks = $derived(outputBlocks(viewModel, { includePrimary, includeRaw, includeStreams })); + let blocks = $derived( + outputBlocks(viewModel, { includePrimary, includeRaw, includeStreams, primaryLabel }) + ); async function copyOutput(text: string, key: string) { try { @@ -47,7 +51,7 @@ function outputBlocks( model: ToolCallViewModel, - options: Pick + options: Pick ): OutputBlock[] { const output = model.output; const result: OutputBlock[] = []; @@ -77,7 +81,12 @@ output.primaryText !== output.stdout && output.primaryText !== output.stderr ) { - result.push({ key: 'output', label: 'Output', text: output.primaryText, tone: defaultTone }); + result.push({ + key: 'output', + label: options.primaryLabel ?? 'Output', + text: output.primaryText, + tone: defaultTone, + }); } // Raw JSON is a fallback for output we could not render structurally, diff --git a/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte index 7870ac39..3c8b5bba 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte @@ -17,6 +17,12 @@ let { item, viewModel }: Props = $props(); let matches = $derived(extractMatches(item.rawOutput)); + // The Path field already names the file; only chip locations that add line info. + let locations = $derived( + viewModel.metadata.locations.filter( + (location) => location.display !== viewModel.metadata.targetPath + ) + ); function extractMatches(rawOutput: unknown): MatchRow[] { const candidates = matchCandidates(rawOutput); @@ -85,9 +91,9 @@ {/if}
- {#if viewModel.metadata.locations.length > 0} + {#if locations.length > 0}
- {#each viewModel.metadata.locations as location} + {#each locations as location} {location.display} {/each}
@@ -95,7 +101,10 @@ {#if matches.length > 0}
-
Matches
+
+ {matches.length} + {matches.length === 1 ? 'match' : 'matches'} +
{#each matches as match}
@@ -111,5 +120,6 @@ {viewModel} includePrimary={matches.length === 0} includeRaw={matches.length === 0} + primaryLabel={viewModel.category === 'read' ? 'Content' : 'Output'} />
diff --git a/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.test.ts b/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.test.ts new file mode 100644 index 00000000..13390986 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it } from 'vitest'; +import type { ToolCallDiff } from '../toolCallViewModel'; +import { buildDiffRows, diffRowStats, groupDiffRows, type DiffRow } from './inlineDiffRows'; + +function diff(oldText: string | null, newText: string | null): ToolCallDiff { + return { + path: 'src/example.ts', + oldText, + newText, + kind: oldText === null ? 'created' : newText === null ? 'deleted' : 'modified', + }; +} + +function lines(count: number, prefix = 'line'): string { + return Array.from({ length: count }, (_, index) => `${prefix} ${index + 1}`).join('\n'); +} + +describe('buildDiffRows', () => { + it('pairs modified lines as removed and added rows with line numbers', () => { + const rows = buildDiffRows(diff('const a = 1;\nshared\n', 'const a = 2;\nshared\n')); + + expect(rows[0]).toMatchObject({ + marker: '-', + tone: 'modified-removed', + oldLine: 1, + newLine: null, + text: 'const a = 1;', + }); + expect(rows[1]).toMatchObject({ + marker: '+', + tone: 'modified-added', + oldLine: null, + newLine: 1, + text: 'const a = 2;', + }); + expect(rows[2]).toMatchObject({ marker: ' ', tone: 'context', oldLine: 2, newLine: 2 }); + }); + + it('marks every line added for created files', () => { + const rows = buildDiffRows(diff(null, 'first\nsecond')); + + expect(rows).toHaveLength(2); + expect(rows.every((row) => row.marker === '+' && row.oldLine === null)).toBe(true); + }); +}); + +describe('diffRowStats', () => { + it('counts added and removed rows', () => { + const rows = buildDiffRows(diff('keep\nold 1\nold 2\n', 'keep\nnew 1\n')); + + expect(diffRowStats(rows)).toEqual({ added: 1, removed: 2 }); + }); +}); + +describe('groupDiffRows', () => { + function toneSummary(blocks: ReturnType): string[] { + return blocks.map((block) => + block.kind === 'collapsed' ? `collapsed:${block.rows.length}` : `visible:${block.rows.length}` + ); + } + + it('collapses a long unchanged run between changes, keeping context on both sides', () => { + const middle = lines(12, 'same'); + const rows = buildDiffRows( + diff(`start old\n${middle}\nend old`, `start new\n${middle}\nend new`) + ); + + // 2 change rows, 3 context, 6 collapsed, 3 context, 2 change rows. + expect(toneSummary(groupDiffRows(rows))).toEqual(['visible:5', 'collapsed:6', 'visible:5']); + }); + + it('trims leading context to the rows adjacent to the first change', () => { + const rows = buildDiffRows(diff(`${lines(10)}\nold tail`, `${lines(10)}\nnew tail`)); + + const blocks = groupDiffRows(rows); + expect(toneSummary(blocks)).toEqual(['collapsed:7', 'visible:5']); + expect(blocks[1].rows.map((row) => row.marker)).toEqual([' ', ' ', ' ', '-', '+']); + }); + + it('trims trailing context to the rows adjacent to the last change', () => { + const rows = buildDiffRows(diff(`old head\n${lines(10)}`, `new head\n${lines(10)}`)); + + expect(toneSummary(groupDiffRows(rows))).toEqual(['visible:5', 'collapsed:7']); + }); + + it('keeps short unchanged runs visible instead of collapsing them', () => { + const rows = buildDiffRows( + diff(`old\n${lines(8, 'same')}\nold end`, `new\n${lines(8, 'same')}\nnew end`) + ); + + // 8 context rows minus 3+3 kept leaves 2 hidden, below the collapse threshold. + expect(toneSummary(groupDiffRows(rows))).toEqual(['visible:12']); + }); + + it('keeps a fully unchanged diff visible as one block', () => { + const rows: DiffRow[] = buildDiffRows(diff(lines(10), lines(10))); + + expect(toneSummary(groupDiffRows(rows))).toEqual(['visible:10']); + }); + + it('returns no blocks for empty diffs', () => { + expect(groupDiffRows([])).toEqual([]); + }); +}); diff --git a/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.ts b/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.ts new file mode 100644 index 00000000..4386edcf --- /dev/null +++ b/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.ts @@ -0,0 +1,201 @@ +import { computeLineDiff, type CharHighlight } from '@builderbot/diff-viewer/utils'; +import type { ToolCallDiff } from '../toolCallViewModel'; + +export type DiffTone = 'context' | 'removed' | 'added' | 'modified-removed' | 'modified-added'; + +export interface DiffRow { + key: string; + oldLine: number | null; + newLine: number | null; + marker: ' ' | '-' | '+'; + text: string; + tone: DiffTone; + highlights: CharHighlight[]; +} + +export type DiffRowBlock = + | { kind: 'visible'; key: string; rows: DiffRow[] } + | { kind: 'collapsed'; key: string; rows: DiffRow[] }; + +export interface DiffRowStats { + added: number; + removed: number; +} + +export interface GroupDiffRowsOptions { + context?: number; + minHidden?: number; +} + +export function buildDiffRows(diff: ToolCallDiff): DiffRow[] { + const beforeLines = splitLines(diff.oldText); + const afterLines = splitLines(diff.newText); + const lineDiff = computeLineDiff(beforeLines, afterLines); + const modifiedByBefore = new Map( + lineDiff.modifiedPairs.map((pair) => [pair.beforeLineIndex, pair]) + ); + const modifiedByAfter = new Map( + lineDiff.modifiedPairs.map((pair) => [pair.afterLineIndex, pair]) + ); + const result: DiffRow[] = []; + let beforeIndex = 0; + let afterIndex = 0; + + while (beforeIndex < beforeLines.length || afterIndex < afterLines.length) { + const beforeClass = lineDiff.beforeLines[beforeIndex]; + const afterClass = lineDiff.afterLines[afterIndex]; + + if ( + beforeIndex < beforeLines.length && + afterIndex < afterLines.length && + beforeClass === 'unchanged' && + afterClass === 'unchanged' + ) { + result.push({ + key: `context:${beforeIndex}:${afterIndex}`, + oldLine: beforeIndex + 1, + newLine: afterIndex + 1, + marker: ' ', + text: beforeLines[beforeIndex], + tone: 'context', + highlights: [], + }); + beforeIndex += 1; + afterIndex += 1; + continue; + } + + const modifiedPair = modifiedByBefore.get(beforeIndex); + if (modifiedPair && modifiedPair.afterLineIndex === afterIndex) { + result.push({ + key: `mod-before:${beforeIndex}`, + oldLine: beforeIndex + 1, + newLine: null, + marker: '-', + text: beforeLines[beforeIndex], + tone: 'modified-removed', + highlights: modifiedPair.beforeHighlights, + }); + result.push({ + key: `mod-after:${afterIndex}`, + oldLine: null, + newLine: afterIndex + 1, + marker: '+', + text: afterLines[afterIndex], + tone: 'modified-added', + highlights: modifiedPair.afterHighlights, + }); + beforeIndex += 1; + afterIndex += 1; + continue; + } + + if (beforeClass === 'removed' || (beforeClass === 'modified' && !modifiedPair)) { + result.push({ + key: `removed:${beforeIndex}`, + oldLine: beforeIndex + 1, + newLine: null, + marker: '-', + text: beforeLines[beforeIndex], + tone: beforeClass === 'modified' ? 'modified-removed' : 'removed', + highlights: modifiedByBefore.get(beforeIndex)?.beforeHighlights ?? [], + }); + beforeIndex += 1; + continue; + } + + const afterPair = modifiedByAfter.get(afterIndex); + if (afterClass === 'added' || (afterClass === 'modified' && !afterPair)) { + result.push({ + key: `added:${afterIndex}`, + oldLine: null, + newLine: afterIndex + 1, + marker: '+', + text: afterLines[afterIndex], + tone: afterClass === 'modified' ? 'modified-added' : 'added', + highlights: modifiedByAfter.get(afterIndex)?.afterHighlights ?? [], + }); + afterIndex += 1; + continue; + } + + if (beforeIndex < beforeLines.length) { + beforeIndex += 1; + } + if (afterIndex < afterLines.length) { + afterIndex += 1; + } + } + + return result; +} + +export function diffRowStats(rows: DiffRow[]): DiffRowStats { + let added = 0; + let removed = 0; + for (const row of rows) { + if (row.marker === '+') added += 1; + else if (row.marker === '-') removed += 1; + } + return { added, removed }; +} + +export function groupDiffRows(rows: DiffRow[], options: GroupDiffRowsOptions = {}): DiffRowBlock[] { + const context = options.context ?? 3; + const minHidden = options.minHidden ?? 3; + + const segments: { isContext: boolean; rows: DiffRow[] }[] = []; + for (const row of rows) { + const isContext = row.tone === 'context'; + const last = segments[segments.length - 1]; + if (last && last.isContext === isContext) { + last.rows.push(row); + } else { + segments.push({ isContext, rows: [row] }); + } + } + + // A diff with no changed rows has nothing to anchor context around. + if (!segments.some((segment) => !segment.isContext)) { + return rows.length > 0 ? [{ kind: 'visible', key: `visible:${rows[0].key}`, rows }] : []; + } + + const blocks: DiffRowBlock[] = []; + const pushVisible = (visibleRows: DiffRow[]) => { + if (visibleRows.length === 0) return; + const last = blocks[blocks.length - 1]; + if (last?.kind === 'visible') { + last.rows = [...last.rows, ...visibleRows]; + } else { + blocks.push({ kind: 'visible', key: `visible:${visibleRows[0].key}`, rows: visibleRows }); + } + }; + + segments.forEach((segment, index) => { + if (!segment.isContext) { + pushVisible(segment.rows); + return; + } + + // Leading runs only pad the change below them, trailing runs the one above. + const head = index === 0 ? 0 : context; + const tail = index === segments.length - 1 ? 0 : context; + const hidden = segment.rows.length - head - tail; + if (hidden < minHidden) { + pushVisible(segment.rows); + return; + } + + pushVisible(segment.rows.slice(0, head)); + const hiddenRows = segment.rows.slice(head, head + hidden); + blocks.push({ kind: 'collapsed', key: `collapsed:${hiddenRows[0].key}`, rows: hiddenRows }); + pushVisible(segment.rows.slice(head + hidden)); + }); + + return blocks; +} + +function splitLines(text: string | null): string[] { + if (text === null || text === '') return []; + return text.split('\n'); +} From 833d06f1f82310f2427b3d7dc96de09b4fda1992 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 9 Jul 2026 17:18:06 +1000 Subject: [PATCH 5/9] fix(sessions): resolve tool call detail review findings Address the code review on the tool call detail work: - Fix buildDiffRows dropping both halves of a modified pair that straddles an LCS anchor (e.g. a line edited while moving across an unchanged line); each half now renders as its own removed/added row with its char highlights, with a regression test. - Render diff char highlights from segments with {#each} instead of {@html}, removing the XSS-sensitive path and the manual escaping. - Make OutputSections project viewModel.sections through its include options (via a new source discriminator on output sections) instead of re-deriving the error/stdout/stderr/raw suppression rules, so the logic lives only in buildToolCallSections. - Compute hasDetails from sections other than status and empty rows so the expand caret hides for tools with nothing to show. - Delete the dead acpTranscript exports the view model superseded (AcpDiff, diffsFromAcpContent, simpleUnifiedDiff, displayLocations, toolResultText). Signed-off-by: Matt Toohey --- .../features/sessions/acpTranscript.test.ts | 43 ------ .../lib/features/sessions/acpTranscript.ts | 128 +----------------- .../sessions/tool-calls/InlineToolDiff.svelte | 26 +--- .../sessions/tool-calls/OutputSections.svelte | 64 +++------ .../tool-calls/inlineDiffRows.test.ts | 13 ++ .../sessions/tool-calls/inlineDiffRows.ts | 44 +++++- .../sessions/toolCallViewModel.test.ts | 11 +- .../features/sessions/toolCallViewModel.ts | 18 ++- 8 files changed, 105 insertions(+), 242 deletions(-) diff --git a/apps/staged/src/lib/features/sessions/acpTranscript.test.ts b/apps/staged/src/lib/features/sessions/acpTranscript.test.ts index 0db0d2a4..47b0ee97 100644 --- a/apps/staged/src/lib/features/sessions/acpTranscript.test.ts +++ b/apps/staged/src/lib/features/sessions/acpTranscript.test.ts @@ -6,7 +6,6 @@ import { isToolMetadataSettled, latestAvailableCommands, stabilizeAcpTranscriptGroups, - toolHasDetails, type RichToolItem, } from './acpTranscript'; @@ -786,48 +785,6 @@ describe('stabilizeAcpTranscriptGroups', () => { }); }); -describe('toolHasDetails', () => { - it('matches the formatted-value emptiness checks', () => { - expect(toolHasDetails(richTool({ key: 'tool:1', verb: 'Ran' }))).toBe(false); - expect(toolHasDetails(richTool({ key: 'tool:1', verb: 'Ran', rawInput: '' }))).toBe(false); - expect(toolHasDetails(richTool({ key: 'tool:1', verb: 'Ran', rawInput: { a: 1 } }))).toBe(true); - expect(toolHasDetails(richTool({ key: 'tool:1', verb: 'Ran', rawOutput: 'ok' }))).toBe(true); - expect( - toolHasDetails( - richTool({ - key: 'tool:1', - verb: 'Ran', - content: [{ type: 'diff', path: '/repo/a.ts', newText: 'new' }], - }) - ) - ).toBe(true); - expect( - toolHasDetails( - richTool({ key: 'tool:1', verb: 'Ran', content: [{ type: 'terminal', terminalId: 't-1' }] }) - ) - ).toBe(true); - expect( - toolHasDetails( - richTool({ key: 'tool:1', verb: 'Ran', locations: [{ path: '/repo/a.ts', line: 3 }] }) - ) - ).toBe(true); - expect( - toolHasDetails( - richTool({ - key: 'tool:1', - verb: 'Ran', - result: message({ id: 9, role: 'tool_result', content: 'output' }), - }) - ) - ).toBe(true); - // Presence-only fields that the expanded card ignores stay hidden. - expect( - toolHasDetails(richTool({ key: 'tool:1', verb: 'Ran', content: [{ type: 'diff' }] })) - ).toBe(false); - expect(toolHasDetails(richTool({ key: 'tool:1', verb: 'Ran', locations: [{}] }))).toBe(false); - }); -}); - describe('isToolMetadataSettled', () => { it('treats terminal statuses as settled and everything else as mutable', () => { const row = (status?: string) => diff --git a/apps/staged/src/lib/features/sessions/acpTranscript.ts b/apps/staged/src/lib/features/sessions/acpTranscript.ts index cd0efa4f..5c29539a 100644 --- a/apps/staged/src/lib/features/sessions/acpTranscript.ts +++ b/apps/staged/src/lib/features/sessions/acpTranscript.ts @@ -1,12 +1,6 @@ import type { SessionMessage } from '../../types'; import type { DisplayRootInput } from './pathDisplayRoots'; -import { - formatToolDisplay, - makePathsRelative, - parseToolCall, - stripCodeFences, - verbGroupSummary, -} from './sessionModalHelpers'; +import { formatToolDisplay, parseToolCall, verbGroupSummary } from './sessionModalHelpers'; export type ToolStatus = 'pending' | 'in_progress' | 'completed' | 'failed' | 'cancelled'; @@ -308,29 +302,6 @@ export function textFromAcpContent(content: unknown): string { return parts.join('\n').trim(); } -export interface AcpDiff { - path: string; - oldText: string | null; - newText: string; -} - -export function diffsFromAcpContent(content: unknown, displayRoots?: DisplayRootInput): AcpDiff[] { - if (!Array.isArray(content)) return []; - const diffs: AcpDiff[] = []; - for (const item of content) { - if (stringProp(item, 'type') !== 'diff') continue; - const path = stringProp(item, 'path'); - const newText = stringProp(item, 'newText'); - if (!path || newText === null) continue; - diffs.push({ - path: makePathsRelative(path, displayRoots), - oldText: stringProp(item, 'oldText'), - newText, - }); - } - return diffs; -} - export function terminalRefsFromAcpContent(content: unknown): string[] { if (!Array.isArray(content)) return []; return content @@ -368,29 +339,6 @@ export function groupRichToolsByVerb(items: RichToolItem[]): RichToolVerbGroup[] })); } -export function simpleUnifiedDiff(diff: AcpDiff): string { - const oldLines = (diff.oldText ?? '').split('\n'); - const newLines = diff.newText.split('\n'); - if (diff.oldText === null) { - return newLines.map((line) => `+${line}`).join('\n'); - } - if (diff.oldText === diff.newText) return diff.newText; - - const output: string[] = []; - const max = Math.max(oldLines.length, newLines.length); - for (let i = 0; i < max; i++) { - const oldLine = oldLines[i]; - const newLine = newLines[i]; - if (oldLine === newLine) { - if (oldLine !== undefined) output.push(` ${oldLine}`); - } else { - if (oldLine !== undefined) output.push(`-${oldLine}`); - if (newLine !== undefined) output.push(`+${newLine}`); - } - } - return output.join('\n'); -} - function groupStatusTone(items: RichToolItem[]): RichToolItem['statusTone'] { if (items.some((item) => item.statusTone === 'danger')) return 'danger'; if (items.some((item) => item.statusTone === 'running')) return 'running'; @@ -870,77 +818,3 @@ function stringProp(value: unknown, key: string): string | null { const prop = (value as Record)[key]; return typeof prop === 'string' ? prop : null; } - -export function displayLocations(locations: unknown, displayRoots?: DisplayRootInput): string[] { - if (!Array.isArray(locations)) return []; - return locations - .map((location) => { - const path = stringProp(location, 'path'); - if (!path) return null; - const line = - location && typeof location === 'object' - ? (location as Record).line - : undefined; - const suffix = typeof line === 'number' ? `:${line}` : ''; - return `${makePathsRelative(path, displayRoots)}${suffix}`; - }) - .filter((location): location is string => !!location); -} - -export function toolResultText(item: RichToolItem): string { - const structuredText = textFromAcpContent(item.content); - if (structuredText) return structuredText; - if (typeof item.rawOutput === 'string') return item.rawOutput; - if (item.rawOutput !== undefined && item.rawOutput !== null) return formatJson(item.rawOutput); - if (item.result?.content) return stripCodeFences(item.result.content); - return ''; -} - -/** - * Whether an expanded tool card would have anything to show. Equivalent to - * checking the formatted values (`formatJson`, `toolResultText`, - * `diffsFromAcpContent`, …) for emptiness, but without building them — the - * formatted strings are only needed once a card is actually expanded, and - * pretty-printing large raw payloads for every collapsed card is what made - * live-transcript re-renders expensive. - */ -export function toolHasDetails(item: RichToolItem): boolean { - return ( - hasJsonValue(item.rawInput) || - hasJsonValue(item.rawOutput) || - acpContentHasDiff(item.content) || - acpContentHasTerminalRef(item.content) || - hasLocation(item.locations) || - !!toolResultText(item) - ); -} - -/** Mirrors `!!formatJson(value)`: only undefined/null/'' format to ''. */ -function hasJsonValue(value: unknown): boolean { - return value !== undefined && value !== null && value !== ''; -} - -/** Mirrors `diffsFromAcpContent(content).length > 0`. */ -function acpContentHasDiff(content: unknown): boolean { - if (!Array.isArray(content)) return false; - return content.some( - (item) => - stringProp(item, 'type') === 'diff' && - !!stringProp(item, 'path') && - stringProp(item, 'newText') !== null - ); -} - -/** Mirrors `terminalRefsFromAcpContent(content).length > 0`. */ -function acpContentHasTerminalRef(content: unknown): boolean { - if (!Array.isArray(content)) return false; - return content.some( - (item) => stringProp(item, 'type') === 'terminal' && !!stringProp(item, 'terminalId') - ); -} - -/** Mirrors `displayLocations(locations).length > 0`. */ -function hasLocation(locations: unknown): boolean { - if (!Array.isArray(locations)) return false; - return locations.some((location) => !!stringProp(location, 'path')); -} diff --git a/apps/staged/src/lib/features/sessions/tool-calls/InlineToolDiff.svelte b/apps/staged/src/lib/features/sessions/tool-calls/InlineToolDiff.svelte index 44e8cfdd..78602204 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/InlineToolDiff.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/InlineToolDiff.svelte @@ -41,24 +41,6 @@ } return result; } - - function highlightedLineHtml(text: string, highlights: CharHighlight[]): string { - return segments(text, highlights) - .map((segment) => { - const escaped = escapeHtml(segment.text); - return segment.highlighted ? `${escaped}` : escaped; - }) - .join(''); - } - - function escapeHtml(text: string): string { - return text - .replace(/&/g, '&') - .replace(//g, '>') - .replace(/"/g, '"') - .replace(/'/g, '''); - } {#snippet diffRow(row: DiffRow)} @@ -70,7 +52,11 @@ {row.oldLine ?? ''} {row.newLine ?? ''} {row.marker} - {@html highlightedLineHtml(row.text, row.highlights)} + {#each segments(row.text, row.highlights) as segment}{#if segment.highlighted}{segment.text}{:else}{segment.text}{/if}{/each}
{/snippet} @@ -246,7 +232,7 @@ white-space: pre-wrap; } - .inline-diff :global(.char-highlight) { + .char-highlight { border-radius: 3px; background: color-mix(in srgb, currentColor 18%, transparent); } diff --git a/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte b/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte index b26d97b3..86983d27 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte @@ -49,55 +49,35 @@ } } + // The view model's sections already encode which outputs to show (error / + // stdout / stderr precedence, raw JSON only as a fallback); this just + // projects them through the renderer's include options. function outputBlocks( model: ToolCallViewModel, options: Pick ): OutputBlock[] { - const output = model.output; const result: OutputBlock[] = []; const defaultTone = model.status === 'cancelled' ? 'cancelled' : 'normal'; - if (output.errorText) { - result.push({ key: 'error', label: 'Error', text: output.errorText, tone: 'danger' }); - } - - if (options.includeStreams && output.stdout) { - result.push({ key: 'stdout', label: 'Stdout', text: output.stdout, tone: defaultTone }); - } - - if (options.includeStreams && output.stderr && output.stderr !== output.errorText) { - result.push({ - key: 'stderr', - label: 'Stderr', - text: output.stderr, - tone: model.status === 'failed' ? 'danger' : defaultTone, - }); - } - - if ( - options.includePrimary && - output.primaryText && - output.primaryText !== output.errorText && - output.primaryText !== output.stdout && - output.primaryText !== output.stderr - ) { - result.push({ - key: 'output', - label: options.primaryLabel ?? 'Output', - text: output.primaryText, - tone: defaultTone, - }); - } - - // Raw JSON is a fallback for output we could not render structurally, - // never a companion to structured blocks. - if (options.includeRaw && output.rawText && result.length === 0) { - result.push({ - key: 'raw-output', - label: 'Raw output', - text: output.rawText, - tone: defaultTone, - }); + for (const section of model.sections) { + if (section.kind === 'output') { + if (!options.includeStreams && (section.source === 'stdout' || section.source === 'stderr')) + continue; + if (section.source === 'primary' && !options.includePrimary) continue; + result.push({ + key: section.source, + label: section.source === 'primary' ? (options.primaryLabel ?? 'Output') : section.label, + text: section.text, + tone: section.tone, + }); + } else if (section.kind === 'raw_output' && options.includeRaw) { + result.push({ + key: 'raw-output', + label: section.label, + text: section.text, + tone: defaultTone, + }); + } } return result; diff --git a/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.test.ts b/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.test.ts index 13390986..fa24dccd 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.test.ts +++ b/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.test.ts @@ -42,6 +42,19 @@ describe('buildDiffRows', () => { expect(rows).toHaveLength(2); expect(rows.every((row) => row.marker === '+' && row.oldLine === null)).toBe(true); }); + + it('renders both halves of a modified pair that straddles an unchanged line', () => { + // 'bar' is the LCS anchor, so the similar foo(1)/foo(2) lines pair up + // across it and never align during the row walk. + const rows = buildDiffRows(diff('foo(1)\nbar', 'bar\nfoo(2)')); + + expect(rows).toMatchObject([ + { marker: '-', tone: 'modified-removed', oldLine: 1, newLine: null, text: 'foo(1)' }, + { marker: ' ', tone: 'context', oldLine: 2, newLine: 1, text: 'bar' }, + { marker: '+', tone: 'modified-added', oldLine: null, newLine: 2, text: 'foo(2)' }, + ]); + expect(diffRowStats(rows)).toEqual({ added: 1, removed: 1 }); + }); }); describe('diffRowStats', () => { diff --git a/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.ts b/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.ts index 4386edcf..9877f167 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.ts +++ b/apps/staged/src/lib/features/sessions/tool-calls/inlineDiffRows.ts @@ -90,35 +90,67 @@ export function buildDiffRows(diff: ToolCallDiff): DiffRow[] { continue; } - if (beforeClass === 'removed' || (beforeClass === 'modified' && !modifiedPair)) { + if (beforeClass === 'removed') { result.push({ key: `removed:${beforeIndex}`, oldLine: beforeIndex + 1, newLine: null, marker: '-', text: beforeLines[beforeIndex], - tone: beforeClass === 'modified' ? 'modified-removed' : 'removed', - highlights: modifiedByBefore.get(beforeIndex)?.beforeHighlights ?? [], + tone: 'removed', + highlights: [], }); beforeIndex += 1; continue; } const afterPair = modifiedByAfter.get(afterIndex); - if (afterClass === 'added' || (afterClass === 'modified' && !afterPair)) { + if (afterClass === 'added') { + result.push({ + key: `added:${afterIndex}`, + oldLine: null, + newLine: afterIndex + 1, + marker: '+', + text: afterLines[afterIndex], + tone: 'added', + highlights: [], + }); + afterIndex += 1; + continue; + } + + // A modified pair can straddle an LCS anchor (e.g. a line edited while + // moving across an unchanged line), so it never aligns for the paired + // branch above. Render each half as its own row instead of dropping it. + if (beforeIndex < beforeLines.length && beforeClass !== 'unchanged') { + result.push({ + key: `removed:${beforeIndex}`, + oldLine: beforeIndex + 1, + newLine: null, + marker: '-', + text: beforeLines[beforeIndex], + tone: 'modified-removed', + highlights: modifiedPair?.beforeHighlights ?? [], + }); + beforeIndex += 1; + continue; + } + + if (afterIndex < afterLines.length && afterClass !== 'unchanged') { result.push({ key: `added:${afterIndex}`, oldLine: null, newLine: afterIndex + 1, marker: '+', text: afterLines[afterIndex], - tone: afterClass === 'modified' ? 'modified-added' : 'added', - highlights: modifiedByAfter.get(afterIndex)?.afterHighlights ?? [], + tone: 'modified-added', + highlights: afterPair?.afterHighlights ?? [], }); afterIndex += 1; continue; } + // Unreachable for well-formed line diffs; advance to guarantee progress. if (beforeIndex < beforeLines.length) { beforeIndex += 1; } diff --git a/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts b/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts index bf3b9eb1..b8187db2 100644 --- a/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts +++ b/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts @@ -253,6 +253,7 @@ describe('buildToolCallViewModel output handling', () => { expect(model.output.primaryText).toBe('legacy output'); expect(model.sections).toContainEqual({ kind: 'output', + source: 'primary', label: 'Output', text: 'legacy output', tone: 'normal', @@ -273,7 +274,7 @@ describe('buildToolCallViewModel output handling', () => { expect(model.sections).toContainEqual({ kind: 'empty', label: 'Waiting for output' }); }); - it('shows a no-output state for completed tools without output', () => { + it('is not expandable when only empty and status rows would show', () => { const model = buildToolCallViewModel( richTool({ status: 'completed', @@ -283,7 +284,13 @@ describe('buildToolCallViewModel output handling', () => { expect(model.output.state).toBe('empty'); expect(model.output.emptyLabel).toBe('No output'); - expect(model.hasDetails).toBe(true); + expect(model.hasDetails).toBe(false); expect(model.sections).toContainEqual({ kind: 'empty', label: 'No output' }); }); + + it('is expandable once any structured detail exists', () => { + const model = buildToolCallViewModel(richTool({ rawInput: { option: 'value' } })); + + expect(model.hasDetails).toBe(true); + }); }); diff --git a/apps/staged/src/lib/features/sessions/toolCallViewModel.ts b/apps/staged/src/lib/features/sessions/toolCallViewModel.ts index 65d4d260..2d4820d0 100644 --- a/apps/staged/src/lib/features/sessions/toolCallViewModel.ts +++ b/apps/staged/src/lib/features/sessions/toolCallViewModel.ts @@ -59,12 +59,20 @@ export interface ToolCallOutput { emptyLabel: 'Waiting for output' | 'No output' | null; } +export type ToolCallOutputSource = 'error' | 'stdout' | 'stderr' | 'primary'; + export type ToolCallSection = | { kind: 'locations'; locations: ToolCallLocation[] } | { kind: 'input'; label: 'Input'; text: string } | { kind: 'diff'; diff: ToolCallDiff } | { kind: 'terminal_refs'; label: 'Terminal'; refs: string[] } - | { kind: 'output'; label: string; text: string; tone: 'normal' | 'danger' | 'cancelled' } + | { + kind: 'output'; + source: ToolCallOutputSource; + label: string; + text: string; + tone: 'normal' | 'danger' | 'cancelled'; + } | { kind: 'raw_output'; label: 'Raw output'; text: string } | { kind: 'empty'; label: 'Waiting for output' | 'No output' } | { @@ -178,7 +186,9 @@ export function buildToolCallViewModel( metadata, output, sections, - hasDetails: sections.length > 0, + // Status and empty-state rows only echo what the collapsed header already + // shows, so they alone don't make a card worth expanding. + hasDetails: sections.some((section) => section.kind !== 'status' && section.kind !== 'empty'), }; } @@ -316,6 +326,7 @@ export function buildToolCallSections( if (output.errorText) { sections.push({ kind: 'output', + source: 'error', label: 'Error', text: output.errorText, tone: 'danger', @@ -326,6 +337,7 @@ export function buildToolCallSections( if (output.stdout) { sections.push({ kind: 'output', + source: 'stdout', label: 'Stdout', text: output.stdout, tone: outputTone, @@ -336,6 +348,7 @@ export function buildToolCallSections( if (output.stderr && output.stderr !== output.errorText) { sections.push({ kind: 'output', + source: 'stderr', label: 'Stderr', text: output.stderr, tone: item.status === 'failed' ? 'danger' : outputTone, @@ -351,6 +364,7 @@ export function buildToolCallSections( ) { sections.push({ kind: 'output', + source: 'primary', label: 'Output', text: output.primaryText, tone: outputTone, From da12e98f50e5aa22fb9dbd844781364382a894a5 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 30 Jul 2026 16:14:10 +1000 Subject: [PATCH 6/9] fix(sessions): finesse expanded read tool details Clean up the duplication and raw markdown in the expanded read/search tool card: - Stop showing the path twice: a location matching the Path field now folds into it as a ":line" suffix (path.rs:1952) instead of a chip repeating the full path. Remaining chips hide the path prefix when the card already names it, collapsing to "Line N"; edit cards get the same treatment for locations the diff header covers. - Strip wrapping markdown code fences from ACP content text before rendering, so read output no longer shows literal ``` lines around the file content (matches the existing legacy tool_result handling). - Run search match paths through makePathsRelative so they drop the workspace-root prefix like paths elsewhere in the chat UI. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- .../tool-calls/EditToolDetails.svelte | 22 +++--- .../tool-calls/ReadSearchToolDetails.svelte | 37 ++++++---- .../sessions/tool-calls/ToolCallCard.svelte | 2 +- .../tool-calls/ToolCallDetails.svelte | 6 +- .../sessions/toolCallViewModel.test.ts | 74 ++++++++++++++++++- .../features/sessions/toolCallViewModel.ts | 50 ++++++++++++- 6 files changed, 162 insertions(+), 29 deletions(-) diff --git a/apps/staged/src/lib/features/sessions/tool-calls/EditToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/EditToolDetails.svelte index 189c05c5..168c121e 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/EditToolDetails.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/EditToolDetails.svelte @@ -1,5 +1,5 @@ @@ -26,14 +26,16 @@ {#if showPath}
Path - {viewModel.metadata.targetPath} + {viewModel.metadata.targetPath}{locationSummary.pathSuffix}
{/if} - {#if locations.length > 0} + {#if locationSummary.chips.length > 0}
- {#each locations as location} - {location.display} + {#each locationSummary.chips as chip} + {chip} {/each}
{/if} diff --git a/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte index 3c8b5bba..55b1d405 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte @@ -1,7 +1,13 @@
@@ -21,7 +23,7 @@ {:else if viewModel.category === 'command'} {:else if viewModel.category === 'read' || viewModel.category === 'search'} - + {:else if viewModel.category === 'network'} {:else} diff --git a/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts b/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts index b8187db2..d88cd70b 100644 --- a/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts +++ b/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts @@ -1,7 +1,11 @@ import { describe, expect, it } from 'vitest'; import type { SessionMessage } from '../../types'; import type { RichToolItem, ToolStatus } from './acpTranscript'; -import { buildToolCallViewModel } from './toolCallViewModel'; +import { + buildToolCallViewModel, + summarizeToolCallLocations, + type ToolCallLocation, +} from './toolCallViewModel'; function message( overrides: Partial & Pick @@ -238,6 +242,22 @@ describe('buildToolCallViewModel output handling', () => { expect(model.sections.some((section) => section.kind === 'raw_output')).toBe(false); }); + it('strips wrapping markdown fences from ACP content text', () => { + const model = buildToolCallViewModel( + richTool({ + toolKind: 'read', + content: [ + { + type: 'content', + content: { type: 'text', text: '```\n1952\tasync fn list()\n1953\t{}\n```' }, + }, + ], + }) + ); + + expect(model.output.primaryText).toBe('1952\tasync fn list()\n1953\t{}'); + }); + it('falls back to legacy tool_result content when ACP output is absent', () => { const model = buildToolCallViewModel( richTool({ @@ -294,3 +314,55 @@ describe('buildToolCallViewModel output handling', () => { expect(model.hasDetails).toBe(true); }); }); + +describe('summarizeToolCallLocations', () => { + function location(overrides: Partial = {}): ToolCallLocation { + const path = overrides.path ?? 'src/a.ts'; + const line = overrides.line ?? null; + const column = overrides.column ?? null; + return { + path, + display: `${path}${line !== null ? `:${line}` : ''}${column !== null ? `:${column}` : ''}`, + line, + column, + ...overrides, + }; + } + + it('folds the target path location into a line suffix instead of a chip', () => { + const summary = summarizeToolCallLocations([location({ line: 1952 })], 'src/a.ts'); + + expect(summary.pathSuffix).toBe(':1952'); + expect(summary.chips).toEqual([]); + }); + + it('includes columns in the folded suffix', () => { + const summary = summarizeToolCallLocations([location({ line: 12, column: 4 })], 'src/a.ts'); + + expect(summary.pathSuffix).toBe(':12:4'); + }); + + it('collapses extra covered-path locations to line labels and keeps other paths in full', () => { + const summary = summarizeToolCallLocations( + [location({ line: 12 }), location({ line: 40 }), location({ path: 'src/b.ts', line: 3 })], + 'src/a.ts' + ); + + expect(summary.pathSuffix).toBe(':12'); + expect(summary.chips).toEqual(['Line 40', 'src/b.ts:3']); + }); + + it('drops covered locations that add no line info', () => { + const summary = summarizeToolCallLocations([location()], 'src/a.ts'); + + expect(summary.pathSuffix).toBe(''); + expect(summary.chips).toEqual([]); + }); + + it('keeps line labels for paths covered elsewhere when no path field is shown', () => { + const summary = summarizeToolCallLocations([location({ line: 12 })], null, ['src/a.ts']); + + expect(summary.pathSuffix).toBe(''); + expect(summary.chips).toEqual(['Line 12']); + }); +}); diff --git a/apps/staged/src/lib/features/sessions/toolCallViewModel.ts b/apps/staged/src/lib/features/sessions/toolCallViewModel.ts index 2d4820d0..9d50bee1 100644 --- a/apps/staged/src/lib/features/sessions/toolCallViewModel.ts +++ b/apps/staged/src/lib/features/sessions/toolCallViewModel.ts @@ -267,7 +267,9 @@ export function classifyToolCall( export function extractToolCallOutput(item: RichToolItem): ToolCallOutput { const rawRecord = recordOrNull(item.rawOutput); const rawText = formatJson(item.rawOutput); - const contentText = textFromAcpContent(item.content); + // ACP content text is markdown-formatted for display; agents (e.g. goose) + // wrap file/command output in code fences that a 
 would show literally.
+  const contentText = stripCodeFences(textFromAcpContent(item.content));
   const errorText = extractErrorText(rawRecord, item.status);
   const stdout = valueText(firstValue(rawRecord, ['stdout', 'standardOutput', 'standard_output']));
   const stderr = valueText(firstValue(rawRecord, ['stderr', 'standardError', 'standard_error']));
@@ -443,6 +445,52 @@ export function extractToolCallLocations(
     .filter((location): location is ToolCallLocation => location !== null);
 }
 
+export interface ToolCallLocationSummary {
+  /** ":line[:column]" from the first location matching the Path field's path. */
+  pathSuffix: string;
+  /** Labels for the remaining locations the card doesn't already name. */
+  chips: string[];
+}
+
+/**
+ * Fold locations into a card's Path field and chips without repeating paths
+ * the card already names. The first location matching `pathFieldPath` becomes
+ * a ":line" suffix on the Path field; the rest become chips — full
+ * "path:line" displays for new paths, bare "Line N" labels for covered paths,
+ * and dropped entirely when they add no line info.
+ */
+export function summarizeToolCallLocations(
+  locations: ToolCallLocation[],
+  pathFieldPath: string | null,
+  coveredPaths: Array = []
+): ToolCallLocationSummary {
+  const covered = new Set();
+  for (const path of [pathFieldPath, ...coveredPaths]) {
+    if (path) covered.add(path);
+  }
+
+  const suffixIndex = pathFieldPath
+    ? locations.findIndex((location) => location.path === pathFieldPath && location.line !== null)
+    : -1;
+
+  const chips = locations.flatMap((location, index) => {
+    if (index === suffixIndex) return [];
+    if (!covered.has(location.path)) return [location.display];
+    if (location.line === null) return [];
+    return [`Line ${location.line}${location.column !== null ? `:${location.column}` : ''}`];
+  });
+
+  return {
+    pathSuffix: suffixIndex >= 0 ? locationLineSuffix(locations[suffixIndex]) : '',
+    chips,
+  };
+}
+
+function locationLineSuffix(location: ToolCallLocation): string {
+  if (location.line === null) return '';
+  return `:${location.line}${location.column !== null ? `:${location.column}` : ''}`;
+}
+
 export function isRecord(value: unknown): value is Record {
   return typeof value === 'object' && value !== null && !Array.isArray(value);
 }

From 8444d7875f57a6f682357d10c3ce227895433f53 Mon Sep 17 00:00:00 2001
From: Matt Toohey 
Date: Thu, 30 Jul 2026 16:53:38 +1000
Subject: [PATCH 7/9] fix(sessions): drop redundant labels from tool call
 details

Trim titles from expanded tool call cards that only restate what the
card already conveys:

- Successful calls no longer render a "Succeeded" footer row; the green
  check already in the card header shows it. The status footer now
  appears only when it adds something (failed, cancelled, or still
  running).
- The primary output block renders without a header label ("Content"
  for reads, "Output" elsewhere), since it is obviously the tool's
  result. Stdout, stderr, error, and raw-output blocks keep their
  labels to tell them apart.

Co-Authored-By: Claude Fable 5 
Signed-off-by: Matt Toohey 
---
 .../sessions/tool-calls/OutputSections.svelte | 62 ++++++++++---------
 .../tool-calls/ReadSearchToolDetails.svelte   |  1 -
 2 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte b/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte
index 86983d27..36f2ae92 100644
--- a/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte
+++ b/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte
@@ -19,7 +19,6 @@
     includeRaw?: boolean;
     includeStatus?: boolean;
     includeStreams?: boolean;
-    primaryLabel?: string;
   }
 
   let {
@@ -29,13 +28,10 @@
     includeRaw = true,
     includeStatus = true,
     includeStreams = true,
-    primaryLabel = 'Output',
   }: Props = $props();
 
   let copiedKey = $state(null);
-  let blocks = $derived(
-    outputBlocks(viewModel, { includePrimary, includeRaw, includeStreams, primaryLabel })
-  );
+  let blocks = $derived(outputBlocks(viewModel, { includePrimary, includeRaw, includeStreams }));
 
   async function copyOutput(text: string, key: string) {
     try {
@@ -54,7 +50,7 @@
   // projects them through the renderer's include options.
   function outputBlocks(
     model: ToolCallViewModel,
-    options: Pick
+    options: Pick
   ): OutputBlock[] {
     const result: OutputBlock[] = [];
     const defaultTone = model.status === 'cancelled' ? 'cancelled' : 'normal';
@@ -66,7 +62,10 @@
         if (section.source === 'primary' && !options.includePrimary) continue;
         result.push({
           key: section.source,
-          label: section.source === 'primary' ? (options.primaryLabel ?? 'Output') : section.label,
+          // The primary block is the tool's main result — a "Content"/"Output"
+          // header just restates what the card already makes obvious, so it
+          // renders bare. Stdout/stderr/error keep labels to tell them apart.
+          label: section.source === 'primary' ? '' : section.label,
           text: section.text,
           tone: section.tone,
         });
@@ -85,25 +84,30 @@
 
 
 {#each blocks as block}
+  {@const copyLabel = block.label || 'output'}
   

-    

-      
{block.label}

-      {#if copyable}
-        
-      {/if}
-    

+    {#if block.label || copyable}
+      

+        {#if block.label}
+          
{block.label}

+        {/if}
+        {#if copyable}
+          
+        {/if}
+      

+    {/if}
     
{viewModel.output.emptyLabel}
{/if} -{#if includeStatus} + +{#if includeStatus && viewModel.statusTone !== 'success'}
- {#if viewModel.statusTone === 'success'} - - {/if} {viewModel.statusLabel}
{/if} diff --git a/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte index 55b1d405..2676b886 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/ReadSearchToolDetails.svelte @@ -129,6 +129,5 @@ {viewModel} includePrimary={matches.length === 0} includeRaw={matches.length === 0} - primaryLabel={viewModel.category === 'read' ? 'Content' : 'Output'} /> From 62703398420ec7a5007086d0614f65957877466d Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 30 Jul 2026 17:08:12 +1000 Subject: [PATCH 8/9] fix(sessions): tighten command tool call layout Stop the Run/Ran tool expansion from spending a whole row on the "$" prompt and another on the copy button: - The command now flows beside the prompt like a terminal line. The "$" sits in a fixed-width gutter (via padding + a negative-margin prefix) and the command wraps under itself with a hanging indent, instead of flex-wrapping the long command onto a line below a stranded "$". - A label-less output block (the primary command result) no longer renders a header row just to hold the copy button; the button tucks into the top-right corner of the output, with the block's own background masking any text it overlaps. Labelled blocks (stdout, stderr, error) keep their header row. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- .../tool-calls/CommandToolDetails.svelte | 3 +- .../sessions/tool-calls/OutputSections.svelte | 54 +++++++++++-------- .../tool-calls/ToolCallDetails.svelte | 45 ++++++++++++++-- 3 files changed, 76 insertions(+), 26 deletions(-) diff --git a/apps/staged/src/lib/features/sessions/tool-calls/CommandToolDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/CommandToolDetails.svelte index 048d08d7..73e2a920 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/CommandToolDetails.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/CommandToolDetails.svelte @@ -19,8 +19,7 @@
- $ - {commandText} + ${commandText}
{#if viewModel.metadata.workingDirectory || failureExitCode !== null}
diff --git a/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte b/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte index 36f2ae92..ef3b76e1 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/OutputSections.svelte @@ -83,35 +83,47 @@ } +{#snippet copyButton(block: OutputBlock, copyLabel: string)} + +{/snippet} + {#each blocks as block} {@const copyLabel = block.label || 'output'}
- {#if block.label || copyable} + {#if block.label}
- {#if block.label} -
{block.label}
- {/if} +
{block.label}
{#if copyable} - + {@render copyButton(block, copyLabel)} {/if}
{/if} - 
{block.text}
+
+ + {#if copyable && !block.label} +
+ {@render copyButton(block, copyLabel)} +
+ {/if} + 
{block.text}
+
{/each} diff --git a/apps/staged/src/lib/features/sessions/tool-calls/ToolCallDetails.svelte b/apps/staged/src/lib/features/sessions/tool-calls/ToolCallDetails.svelte index 6af30b45..1c2f57da 100644 --- a/apps/staged/src/lib/features/sessions/tool-calls/ToolCallDetails.svelte +++ b/apps/staged/src/lib/features/sessions/tool-calls/ToolCallDetails.svelte @@ -105,7 +105,6 @@ background: var(--bg-primary); } - .tool-code-block :global(.tool-command-line), .tool-code-block :global(.tool-network-line) { display: flex; flex-wrap: wrap; @@ -116,12 +115,29 @@ font-weight: 500; } + /* Lay the command out like a terminal line: the "$" sits in a fixed-width + gutter and the command flows beside it, wrapping under itself with a + hanging indent instead of stranding the prompt on its own row. */ + .tool-code-block :global(.tool-command-line) { + min-width: 0; + padding-left: 1.25em; + color: var(--text-primary); + font-weight: 500; + } + .tool-code-block :global(.tool-command-prefix) { - flex-shrink: 0; + display: inline-block; + width: 1.25em; + margin-left: -1.25em; color: var(--text-faint); + user-select: none; + } + + .tool-code-block :global(.tool-command-text) { + overflow-wrap: anywhere; + white-space: pre-wrap; } - .tool-code-block :global(.tool-command-text), .tool-code-block :global(.tool-network-url) { min-width: 0; overflow-wrap: anywhere; @@ -168,6 +184,18 @@ min-width: 0; } + .tool-code-block :global(.tool-output-body) { + position: relative; + min-width: 0; + } + + .tool-code-block :global(.tool-output-body-actions) { + position: absolute; + top: 4px; + right: 4px; + z-index: 1; + } + .tool-code-block :global(.tool-output-header) { display: flex; align-items: center; @@ -201,6 +229,17 @@ outline: none; } + /* The overlaid button sits over the top-right of the output, so give it the + block's own background to mask any text it covers. */ + .tool-code-block :global(.tool-output-body-actions .tool-copy-button) { + background: color-mix(in srgb, var(--bg-chrome) 82%, var(--bg-primary)); + } + + .tool-code-block :global(.tool-output-body-actions .tool-copy-button:hover), + .tool-code-block :global(.tool-output-body-actions .tool-copy-button:focus-visible) { + background: var(--bg-hover); + } + .tool-code-block :global(.tool-code-output) { margin: 0; max-height: 220px; From 548de22fbbe081aeceb7a33dcc3e39e717f5bec1 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 3 Aug 2026 10:23:18 +1000 Subject: [PATCH 9/9] fix(sessions): address tool call detail review findings Resolve the two Codex review comments on the tool call detail work: - Keep raw output/input JSON formatting lazy. buildToolCallViewModel runs on every live transcript rebuild to feed the collapsed card header, but it was eagerly formatJson-ing rawOutput (and raw input) even for collapsed cards and even when structured output made the raw fallback unused. inputText and rawText are now memoized getters gated by cheap hasInputText/hasRawText presence flags, so the pretty-printing only runs once an expanded card reads the value. - Stop generic status payloads from hijacking the network renderer. An unknown tool whose output merely contained a common key like status or title was classified as network, and NetworkToolDetails then treated that status as a structured response and suppressed the raw output, dropping the rest of the payload. Network detection now requires strong request/response evidence (url/method/request/response) rather than generic keys. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- .../sessions/toolCallViewModel.test.ts | 52 +++++++++++ .../features/sessions/toolCallViewModel.ts | 86 +++++++++++++------ 2 files changed, 112 insertions(+), 26 deletions(-) diff --git a/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts b/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts index d88cd70b..5359c9b2 100644 --- a/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts +++ b/apps/staged/src/lib/features/sessions/toolCallViewModel.test.ts @@ -203,6 +203,36 @@ describe('buildToolCallViewModel classification', () => { ]) ); }); + + it('does not treat generic status output as a network payload', () => { + const model = buildToolCallViewModel( + richTool({ + verb: 'Processed', + rawOutput: { status: 'ok', id: 42, title: 'Done' }, + }) + ); + + // A stray `status`/`title` key is not network evidence, so the payload must + // stay generic and keep its raw fallback instead of vanishing behind the + // network renderer's structured-response view. + expect(model.category).toBe('generic'); + expect(model.sections).toEqual( + expect.arrayContaining([ + expect.objectContaining({ kind: 'raw_output', text: expect.stringContaining('"id": 42') }), + ]) + ); + }); + + it('still classifies unknown tools as network on request/response evidence', () => { + const model = buildToolCallViewModel( + richTool({ + verb: 'Processed', + rawOutput: { response: { status: 200, body: 'ok' } }, + }) + ); + + expect(model.category).toBe('network'); + }); }); describe('buildToolCallViewModel output handling', () => { @@ -313,6 +343,28 @@ describe('buildToolCallViewModel output handling', () => { expect(model.hasDetails).toBe(true); }); + + it('defers raw output JSON formatting until the value is read', () => { + let reads = 0; + const rawOutput = { + get payload() { + reads += 1; + return 'value'; + }, + }; + + const model = buildToolCallViewModel(richTool({ verb: 'Processed', rawOutput })); + + // Building the view model for a collapsed card must not pretty-print the + // raw output; that only happens once an expanded card reads `rawText`. + expect(reads).toBe(0); + expect(model.output.hasRawText).toBe(true); + expect(model.hasDetails).toBe(true); + expect(reads).toBe(0); + + expect(model.output.rawText).toContain('value'); + expect(reads).toBe(1); + }); }); describe('summarizeToolCallLocations', () => { diff --git a/apps/staged/src/lib/features/sessions/toolCallViewModel.ts b/apps/staged/src/lib/features/sessions/toolCallViewModel.ts index 9d50bee1..3ff59273 100644 --- a/apps/staged/src/lib/features/sessions/toolCallViewModel.ts +++ b/apps/staged/src/lib/features/sessions/toolCallViewModel.ts @@ -35,7 +35,10 @@ export interface ToolCallMetadata { toolKind: string | null; toolName: string | null; input: Record | null; + /** Pretty-printed input JSON. Lazily formatted — read only when expanded. */ inputText: string; + /** Whether {@link inputText} would render anything, without formatting it. */ + hasInputText: boolean; parsedCommands: ParsedToolCommand[]; command: string | null; workingDirectory: string | null; @@ -51,7 +54,10 @@ export interface ToolCallMetadata { export interface ToolCallOutput { state: ToolCallOutputState; primaryText: string; + /** Pretty-printed raw output JSON. Lazily formatted — read only when expanded. */ rawText: string; + /** Whether {@link rawText} would render anything, without formatting it. */ + hasRawText: boolean; errorText: string; stdout: string; stderr: string; @@ -151,20 +157,12 @@ const COMMAND_KEYS = ['command', 'cmd', 'script']; const CWD_KEYS = ['cwd', 'workingDirectory', 'working_directory', 'workdir']; const URL_KEYS = ['url', 'uri', 'href']; const METHOD_KEYS = ['method', 'httpMethod', 'http_method']; -const NETWORK_KEYS = new Set([ - 'body', - 'headers', - 'method', - 'request', - 'response', - 'screenshot', - 'selector', - 'status', - 'statusCode', - 'status_code', - 'title', - 'url', -]); +// Only strong request/response evidence promotes an unknown tool to the +// network renderer. Generic keys like `status` or `title` show up on plenty of +// non-network payloads; classifying those as network routes the card through +// NetworkToolDetails, which treats the status as a structured response and +// hides the rest of the raw output. +const NETWORK_KEYS = new Set([...URL_KEYS, ...METHOD_KEYS, 'request', 'response']); export function buildToolCallViewModel( item: RichToolItem, @@ -200,12 +198,16 @@ export function extractToolCallMetadata( const parsedArgs = parsed ? recordOrNull(parsed.args) : null; const rawInput = recordOrNull(item.rawInput); const input = rawInput ?? parsedArgs; - const inputText = + // Formatting input to JSON is costly for large payloads and a collapsed card + // never shows it, so keep `inputText` behind a memoized getter and decide + // whether to render it from the cheap `hasInputText` flag instead. + const inputSource = item.rawInput !== undefined && item.rawInput !== null - ? formatJson(item.rawInput) + ? item.rawInput : parsedArgs && Object.keys(parsedArgs).length > 0 - ? formatJson(parsedArgs) - : ''; + ? parsedArgs + : null; + let inputTextCache: string | undefined; const parsedCommands = [...parsedCommandsFrom(input), ...parsedCommandsFrom(parsedArgs)].filter( uniqueParsedCommand ); @@ -216,7 +218,10 @@ export function extractToolCallMetadata( toolKind: normalizeToken(item.toolKind), toolName: parsed?.name ?? null, input, - inputText, + get inputText() { + return (inputTextCache ??= formatJson(inputSource)); + }, + hasInputText: hasFormattableJson(inputSource), parsedCommands, command: relativeValue( firstString(input, COMMAND_KEYS) ?? firstCommandValue(parsedCommands), @@ -266,7 +271,12 @@ export function classifyToolCall( export function extractToolCallOutput(item: RichToolItem): ToolCallOutput { const rawRecord = recordOrNull(item.rawOutput); - const rawText = formatJson(item.rawOutput); + // Pretty-printing the whole raw output is the expensive path a collapsed card + // must not pay, and it's discarded outright when structured output exists, so + // keep it lazy and gate its section on the cheap `hasRawText` flag. + const rawOutput = item.rawOutput; + const hasRawText = hasFormattableJson(rawOutput); + let rawTextCache: string | undefined; // ACP content text is markdown-formatted for display; agents (e.g. goose) // wrap file/command output in code fences that a 
 would show literally.
   const contentText = stripCodeFences(textFromAcpContent(item.content));
@@ -285,13 +295,16 @@ export function extractToolCallOutput(item: RichToolItem): ToolCallOutput {
     stdout ||
     stderr ||
     resultText;
-  const hasAnyOutput = !!(primaryText || rawText || errorText || stdout || stderr);
+  const hasAnyOutput = !!(primaryText || hasRawText || errorText || stdout || stderr);
   const isWaiting = item.status === 'pending' || item.status === 'in_progress';
 
   return {
     state: hasAnyOutput ? 'output' : isWaiting ? 'waiting' : 'empty',
     primaryText,
-    rawText,
+    get rawText() {
+      return (rawTextCache ??= formatJson(rawOutput));
+    },
+    hasRawText,
     errorText,
     stdout,
     stderr,
@@ -311,8 +324,16 @@ export function buildToolCallSections(
     sections.push({ kind: 'locations', locations: metadata.locations });
   }
 
-  if (metadata.inputText) {
-    sections.push({ kind: 'input', label: 'Input', text: metadata.inputText });
+  if (metadata.hasInputText) {
+    // `text` defers to the metadata's memoized getter so the caret-driven
+    // `hasDetails` check (which only reads `kind`) never triggers formatting.
+    sections.push({
+      kind: 'input',
+      label: 'Input',
+      get text() {
+        return metadata.inputText;
+      },
+    });
   }
 
   for (const diff of metadata.diffs) {
@@ -376,8 +397,14 @@ export function buildToolCallSections(
 
   // Raw JSON is a fallback for output we could not render structurally,
   // never a companion to structured output sections.
-  if (output.rawText && !hasStructuredOutput) {
-    sections.push({ kind: 'raw_output', label: 'Raw output', text: output.rawText });
+  if (output.hasRawText && !hasStructuredOutput) {
+    sections.push({
+      kind: 'raw_output',
+      label: 'Raw output',
+      get text() {
+        return output.rawText;
+      },
+    });
   }
 
   if (output.emptyLabel) {
@@ -642,6 +669,13 @@ function valueText(value: unknown): string {
   return formatJson(value);
 }
 
+/** Mirrors when {@link formatJson} yields a non-empty string, without formatting. */
+function hasFormattableJson(value: unknown): boolean {
+  if (value === undefined || value === null) return false;
+  if (typeof value === 'string') return value.length > 0;
+  return true;
+}
+
 function extractErrorText(rawOutput: Record | null, status: ToolStatus): string {
   const error = firstValue(rawOutput, ['error', 'errorMessage', 'error_message']);
   if (error !== undefined && error !== null) return valueText(error);