From 6a2637222b645c22272cf359bb2bb50c76faf6b9 Mon Sep 17 00:00:00 2001 From: Eric Lee Date: Wed, 12 Aug 2026 11:26:19 -0700 Subject: [PATCH 1/6] feat(tui): collapse tool runs into the original's brief line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The transcript listed every tool call on its own `⏺ Tool(args)` row, so a turn that read three files and ran three commands cost six rows plus their results. The original Claude Code transcript folds a run of calls into one dim summary line instead — "Read 3 files, listed 1 directory, ran 1 shell command" — and puts the full list back under ctrl+o. Ported that. domain/toolBrief.ts owns the vocabulary: which bucket a call falls into (shell calls bucket on the *command*, so `ls` reads as a listing and `grep` as a search), the clause order, and the two rules that carry the look — only the first clause is capitalized, and a live run uses the gerund with a trailing ellipsis ("Reading 3 files…"). Edits, delegations and questions stay on their own row: upstream only folds tools that declare a search/read shape, and here their detail rows carry a patch, the subagent tree anchor, and the user's own answers respectively — things a tally would destroy. briefRuns() preserves order, so a trail that interleaves them reads brief / row / brief. Two more parity fixes fall out: expanded blocks are now blank-line separated the way upstream renders them, and the ⎿ result gutter moved into a string literal — it is three columns wide, and a formatter had quietly collapsed the bare JSX whitespace to two. Verified against Claude Code 2.1.228 driven through a pty: same clause text, same ordering, same bold tallies, same gutter. Co-Authored-By: Claude Opus 5 --- ui-tui/src/__tests__/expandResults.test.ts | 14 +- ui-tui/src/__tests__/toolBrief.test.ts | 230 ++++++++++++++++++ ui-tui/src/__tests__/toolTranscript.test.ts | 41 ++-- ui-tui/src/components/thinking.tsx | 191 +++++++++++---- ui-tui/src/domain/toolBrief.ts | 246 ++++++++++++++++++++ ui-tui/src/lib/virtualHeights.ts | 43 +++- 6 files changed, 684 insertions(+), 81 deletions(-) create mode 100644 ui-tui/src/__tests__/toolBrief.test.ts create mode 100644 ui-tui/src/domain/toolBrief.ts diff --git a/ui-tui/src/__tests__/expandResults.test.ts b/ui-tui/src/__tests__/expandResults.test.ts index 256130b64..8d1fc43cb 100644 --- a/ui-tui/src/__tests__/expandResults.test.ts +++ b/ui-tui/src/__tests__/expandResults.test.ts @@ -233,20 +233,22 @@ describe('expanded rendering', () => { const trail = ['Bash(seq 6) :: 1\n2\n3\n… +3 lines (ctrl+o to expand) ✓'] const verboseTrail = ['Bash(seq 6) (0.2s) :: Result:\n1\n2\n3\n4\n5\n6 ✓'] - it('collapsed shows the summary; expanded swaps in the full result', () => { + it('collapsed folds the call into the brief; expanded swaps in the full result', () => { const collapsed = stripAnsi( renderToString( React.createElement(ToolTrail, { detailsMode: 'collapsed', t: DEFAULT_THEME, trail, verboseTrail }) ) ) - expect(collapsed).toContain('+3 lines (ctrl+o to expand)') - expect(collapsed).not.toContain('4') // verbose-only content stays hidden + // Collapsed is the brief line: the tally only — no call row, no result + // summary, and none of the verbose-only content. + expect(collapsed).toContain('Ran 1 shell command') + expect(collapsed).not.toContain('Bash(seq 6)') + expect(collapsed).not.toContain('+3 lines (ctrl+o to expand)') + expect(collapsed).not.toContain('4') const expanded = stripAnsi( - renderToString( - React.createElement(ToolTrail, { detailsMode: 'expanded', t: DEFAULT_THEME, trail, verboseTrail }) - ) + renderToString(React.createElement(ToolTrail, { detailsMode: 'expanded', t: DEFAULT_THEME, trail, verboseTrail })) ) expect(expanded).toContain('Result:') diff --git a/ui-tui/src/__tests__/toolBrief.test.ts b/ui-tui/src/__tests__/toolBrief.test.ts new file mode 100644 index 000000000..ede623383 --- /dev/null +++ b/ui-tui/src/__tests__/toolBrief.test.ts @@ -0,0 +1,230 @@ +import { EventEmitter } from 'node:events' +import { PassThrough } from 'node:stream' + +import { renderSync } from '@clawcodex/ink' +import React from 'react' +import { describe, expect, it, vi } from 'vitest' + +vi.mock('node:child_process', () => ({ spawn: () => new EventEmitter() })) + +vi.hoisted(() => { + process.env.FORCE_COLOR = '3' + process.env.COLORTERM = 'truecolor' + delete process.env.NO_COLOR +}) + +import { ToolTrail } from '../components/thinking.js' +import { + briefCallOfTrailLine, + briefClauses, + type BriefCounts, + briefRuns, + briefText, + classifyBriefTool, + emptyBriefCounts +} from '../domain/toolBrief.js' +import { buildToolTrailLine, stripAnsi } from '../lib/text.js' +import { DEFAULT_THEME } from '../theme.js' + +// ── classification ────────────────────────────────────────────────────────── + +describe('classifyBriefTool', () => { + it('buckets the file/search tools', () => { + expect(classifyBriefTool('Read(src/a.py)')).toBe('read') + expect(classifyBriefTool('Grep(TODO)')).toBe('search') + expect(classifyBriefTool('Glob(src/*.py)')).toBe('search') + }) + + it('buckets a shell call on its command, the way upstream does', () => { + expect(classifyBriefTool('Bash(ls src)')).toBe('list') + expect(classifyBriefTool('Bash(grep -rn def src)')).toBe('search') + expect(classifyBriefTool('Bash(cat docs/readme.md)')).toBe('read') + expect(classifyBriefTool('Bash(echo hello)')).toBe('bash') + // A command that merely *starts* with those letters is still a command. + expect(classifyBriefTool('Bash(lsof -i)')).toBe('bash') + expect(classifyBriefTool('Bash(catalog --build)')).toBe('bash') + }) + + it('keeps edits, delegations and questions out of the tally', () => { + expect(classifyBriefTool('Edit(a.py)')).toBe('edit') + expect(classifyBriefTool('Write(b.py)')).toBe('edit') + expect(classifyBriefTool('Delegate Task(audit)')).toBe('agent') + expect(classifyBriefTool('AskUserQuestion(pick one)')).toBe('ask') + }) + + it('falls back to the catch-all bucket', () => { + expect(classifyBriefTool('WebSearch(rust release)')).toBe('other') + expect(classifyBriefTool('Mcp Github List Prs(open)')).toBe('other') + }) + + it('ignores a legacy duration suffix on resumed trail lines', () => { + expect(classifyBriefTool('Read(a.py) (1.2s)')).toBe('read') + }) +}) + +// ── vocabulary ────────────────────────────────────────────────────────────── + +describe('briefText', () => { + const counts = (over: Partial): BriefCounts => ({ ...emptyBriefCounts(), ...over }) + + it('capitalizes only the first clause and joins the rest with commas', () => { + expect(briefText(counts({ bash: 1, list: 1, read: 3 }))).toBe( + 'Read 3 files, listed 1 directory, ran 1 shell command' + ) + }) + + it('orders clauses search → read → list → other → shell', () => { + expect(briefText(counts({ bash: 1, list: 1, other: 1, read: 1, search: 1 }))).toBe( + 'Searched for 1 pattern, read 1 file, listed 1 directory, called 1 tool, ran 1 shell command' + ) + }) + + it('uses the gerund and an ellipsis while the run is live', () => { + expect(briefText(counts({ read: 1 }), true)).toBe('Reading 1 file…') + expect(briefText(counts({ search: 2 }), true)).toBe('Searching for 2 patterns…') + }) + + it('pluralizes each noun independently', () => { + expect(briefText(counts({ list: 2 }))).toBe('Listed 2 directories') + expect(briefText(counts({ read: 1 }))).toBe('Read 1 file') + }) + + it('is empty when nothing collapsible ran', () => { + expect(briefText(counts({ edit: 3 }))).toBe('') + expect(briefClauses(counts({ agent: 1 }))).toEqual([]) + }) +}) + +// ── runs ──────────────────────────────────────────────────────────────────── + +describe('briefRuns', () => { + const id = (s: string) => s + + it('folds a consecutive stretch into one brief run', () => { + const runs = briefRuns(['Read(a)', 'Read(b)', 'Bash(echo hi)'], id) + + expect(runs).toHaveLength(1) + expect(runs[0]!.kind).toBe('brief') + expect(runs[0]!.items).toHaveLength(3) + }) + + it('breaks the run at a standalone call and keeps source order', () => { + const runs = briefRuns(['Read(a)', 'Edit(b)', 'Bash(echo hi)'], id) + + expect(runs.map(r => r.kind)).toEqual(['brief', 'flat', 'brief']) + expect(runs[1]!.items).toEqual(['Edit(b)']) + }) + + it('never merges two standalone calls into one block', () => { + const runs = briefRuns(['Edit(a)', 'Edit(b)'], id) + + expect(runs.map(r => r.kind)).toEqual(['flat', 'flat']) + }) +}) + +describe('briefCallOfTrailLine', () => { + it('recovers the call from a completed trail line', () => { + expect(briefCallOfTrailLine(buildToolTrailLine('Read', 'src/a.py', false, 'Read 8 lines'))).toBe('Read(src/a.py)') + }) + + it('recovers the label from a drafting line', () => { + expect(briefCallOfTrailLine('drafting Write…')).toBe('Write') + }) +}) + +// ── render ────────────────────────────────────────────────────────────────── + +const renderToString = (element: React.ReactElement): string => { + const stdout = new PassThrough() + const stdin = new PassThrough() + const stderr = new PassThrough() + let output = '' + + Object.assign(stdout, { columns: 100, isTTY: false, rows: 40 }) + Object.assign(stdin, { isTTY: false }) + stdout.on('data', (chunk: Buffer) => { + output += chunk.toString() + }) + + const instance = renderSync(element, { stderr: stderr as never, stdin: stdin as never, stdout: stdout as never }) + + instance.unmount() + + return output +} + +describe('ToolTrail brief render', () => { + const trail = [ + buildToolTrailLine('Read', 'src/alpha.py', false, 'Read 8 lines'), + buildToolTrailLine('Read', 'src/beta.py', false, 'Read 8 lines'), + buildToolTrailLine('Bash', 'ls src', false, 'alpha.py\nbeta.py'), + buildToolTrailLine('Bash', 'echo hello', false, 'hello') + ] + + it('collapses the whole run to one summary line', () => { + const out = stripAnsi( + renderToString(React.createElement(ToolTrail, { detailsMode: 'collapsed', t: DEFAULT_THEME, trail })) + ) + + expect(out).toContain('Read 2 files, listed 1 directory, ran 1 shell command') + expect(out).not.toContain('Read(src/alpha.py)') + expect(out).not.toContain('hello') + }) + + it('puts every call back under ctrl+o', () => { + const out = stripAnsi( + renderToString(React.createElement(ToolTrail, { detailsMode: 'expanded', t: DEFAULT_THEME, trail })) + ) + + expect(out).toContain('Read(src/alpha.py)') + expect(out).toContain('Read(src/beta.py)') + expect(out).toContain('Bash(echo hello)') + expect(out).not.toContain('Read 2 files,') + }) + + it('keeps a standalone edit visible while collapsed', () => { + const withEdit = [trail[0]!, buildToolTrailLine('Edit', 'src/gamma.py', false, 'Updated src/gamma.py')] + + const out = stripAnsi( + renderToString(React.createElement(ToolTrail, { detailsMode: 'collapsed', t: DEFAULT_THEME, trail: withEdit })) + ) + + expect(out).toContain('Read 1 file') + expect(out).toContain('Edit(src/gamma.py)') + }) + + it('separates expanded blocks with a blank line', () => { + const out = stripAnsi( + renderToString(React.createElement(ToolTrail, { detailsMode: 'expanded', t: DEFAULT_THEME, trail })) + ) + + const rows = out.split('\n') + const first = rows.findIndex(r => r.includes('Read(src/alpha.py)')) + const second = rows.findIndex(r => r.includes('Read(src/beta.py)')) + + // ⏺ call / ⎿ result / blank / ⏺ next call + expect(second - first).toBe(3) + expect(rows[second - 1]!.trim()).toBe('') + }) + + // The ⎿ result gutter is three columns wide (" ⎿ x"). It lives in a + // string literal because a formatter collapses bare JSX whitespace, and the + // looser /⎿\s+/ assertions elsewhere cannot see the difference. + it('keeps the result gutter three columns wide', () => { + const out = stripAnsi( + renderToString(React.createElement(ToolTrail, { detailsMode: 'expanded', t: DEFAULT_THEME, trail })) + ) + + expect(out.split('\n').find(row => row.includes('⎿'))).toMatch(/^ {2}⎿ {2}\S/) + }) + + it('renders the brief under a two-column gutter, like the ⏺ rows', () => { + const out = stripAnsi( + renderToString(React.createElement(ToolTrail, { detailsMode: 'collapsed', t: DEFAULT_THEME, trail })) + ) + + const row = out.split('\n').find(line => line.includes('Read 2 files')) + + expect(row).toMatch(/^ {2}Read 2 files/) + }) +}) diff --git a/ui-tui/src/__tests__/toolTranscript.test.ts b/ui-tui/src/__tests__/toolTranscript.test.ts index b03adc1b3..e1192a7f0 100644 --- a/ui-tui/src/__tests__/toolTranscript.test.ts +++ b/ui-tui/src/__tests__/toolTranscript.test.ts @@ -169,25 +169,34 @@ describe('formatToolResult', () => { // ── virtualHeights: multi-line tool entries count rendered rows ────────────── describe('estimatedMsgHeight with multi-line tool details', () => { - it('counts one row per rendered detail line, not per entry', () => { - const base = { - kind: 'trail' as const, - role: 'system' as const, - text: '', - tools: [buildToolTrailLine('Bash', 'ls', false, 'a\nb\nc')] - } - - const single = { - ...base, - tools: [buildToolTrailLine('Bash', 'ls', false, 'a')] - } - - const opts = { compact: false, details: true, leadGap: false } - const tall = estimatedMsgHeight(base, 80, opts) - const short = estimatedMsgHeight(single, 80, opts) + const base = { + kind: 'trail' as const, + role: 'system' as const, + text: '', + tools: [buildToolTrailLine('Bash', 'seq 3', false, 'a\nb\nc')] + } + + const single = { + ...base, + tools: [buildToolTrailLine('Bash', 'seq 1', false, 'a')] + } + + const opts = { compact: false, details: true, leadGap: false } + + it('counts one row per rendered detail line, not per entry, when expanded', () => { + const expanded = { ...opts, toolsExpanded: true } + const tall = estimatedMsgHeight(base, 80, expanded) + const short = estimatedMsgHeight(single, 80, expanded) expect(tall - short).toBe(2) // two extra detail rows }) + + it('collapses to the one-row brief regardless of detail length', () => { + // The brief renders the tally alone, so a 3-line result costs no more + // rows than a 1-line one — the estimate has to agree or the scrollbar + // and topSpacer math drift against the paint. + expect(estimatedMsgHeight(base, 80, opts)).toBe(estimatedMsgHeight(single, 80, opts)) + }) }) // ── ToolTrail render: CC anatomy ───────────────────────────────────────────── diff --git a/ui-tui/src/components/thinking.tsx b/ui-tui/src/components/thinking.tsx index f4d4667eb..5a064452e 100644 --- a/ui-tui/src/components/thinking.tsx +++ b/ui-tui/src/components/thinking.tsx @@ -4,6 +4,7 @@ import spinners, { type BrailleSpinnerName } from 'unicode-animations' import { THINKING_COT_MAX } from '../config/limits.js' import { sectionMode } from '../domain/details.js' +import { briefClauses, type BriefCounts, briefRuns, countBriefTools } from '../domain/toolBrief.js' import { buildSubagentTree, fmtTokens, @@ -264,6 +265,62 @@ function Chevron({ ) } +/** + * The collapsed tool row — the original's brief line. A run of tool calls + * reads as one dim sentence ("Read 3 files, listed 1 directory") with the + * tallies bold, under a 2-column gutter so it lines up with the `⏺ ` bullets + * the expanded view (ctrl+o) puts back. + * + * Two gutter states: a blinking bullet while the run is still executing (the + * original animates a dot there), and — a deliberate departure from upstream, + * which shows nothing — a red bullet when a call in the run failed, so a + * failure is never silently tallied away as a plain count. + */ +function BriefLine({ + blinkOn, + counts, + error, + live, + t +}: { + blinkOn: boolean + counts: BriefCounts + error: boolean + live: boolean + t: Theme +}) { + const clauses = briefClauses(counts, live) + + if (!clauses.length) { + return null + } + + const gutter = live ? ( + {blinkOn ? '⏺ ' : ' '} + ) : error ? ( + + ) : ( + {' '} + ) + + return ( + + + {gutter} + + + {clauses.map((clause, index) => ( + + {index > 0 ? ', ' : ''} + {clause.verb} {clause.count} {clause.noun} + + ))} + {live ? '…' : ''} + + + ) +} + function heatColor(node: SubagentNode, peak: number, theme: Theme): string | undefined { const palette = [theme.color.border, theme.color.accent, theme.color.primary, theme.color.warn, theme.color.error] const idx = hotnessBucket(node.aggregate.hotness, peak, palette.length) @@ -1060,63 +1117,95 @@ export const ToolTrail = memo(function ToolTrail({ // alternates glyph/space (original useBlink is 600ms — same read). const blinkOn = Math.floor(now / 500) % 2 === 0 - const toolsFlat = - hasTools && visible.tools !== 'hidden' ? ( - - {groups.map(group => { - const isDelegateGroup = group.label.startsWith('Delegate Task') - const bulletColor = group.error ? t.color.error : t.color.ok + // One full `⏺ Tool(args)` + `⎿ result` block. This is every row in the + // expanded (ctrl+o) view, and the shape STANDALONE tools — edits, + // delegations, questions — keep even while collapsed. `gap` opens the blank + // line upstream leaves between consecutive blocks. + const renderGroup = (group: Group, gap: boolean) => { + const isDelegateGroup = group.label.startsWith('Delegate Task') + const bulletColor = group.error ? t.color.error : t.color.ok - return ( - - - {group.live ? ( - // Running: dim blinking ⏺ — the off-frame renders two - // spaces matching the glyph+space width so the row never - // reflows mid-blink. - {blinkOn ? '⏺ ' : ' '} + return ( + + + {group.live ? ( + // Running: dim blinking ⏺ — the off-frame renders two + // spaces matching the glyph+space width so the row never + // reflows mid-blink. + {blinkOn ? '⏺ ' : ' '} + ) : ( + + )} + {toolLabel(group)} + {isDelegateGroup ? ( + + {' (/agents to monitor)'} + + ) : null} + + {group.details.map(detail => { + // Multi-line details (Bash 3-line summaries, error caps): + // first row carries the ⎿ connector, continuations align + // under the content column. + if (typeof detail.content === 'string' && detail.content.includes('\n')) { + return detail.content.split('\n').map((row, rowIdx) => ( + + {rowIdx === 0 ? ( + <> + {' '} + {/* String literal, not JSX text: the two trailing spaces are load-bearing + (the original's ⎿ gutter is 3 columns) and a formatter collapses + bare JSX whitespace. */} + {'⎿ '} + ) : ( - + ' ' )} - {toolLabel(group)} - {isDelegateGroup ? ( - - {' (/agents to monitor)'} - - ) : null} + {row || ' '} - {group.details.map(detail => { - // Multi-line details (Bash 3-line summaries, error caps): - // first row carries the ⎿ connector, continuations align - // under the content column. - if (typeof detail.content === 'string' && detail.content.includes('\n')) { - return detail.content.split('\n').map((row, rowIdx) => ( - - {rowIdx === 0 ? ( - <> - {' '} - - - ) : ( - ' ' - )} - {row || ' '} - - )) - } - - return ( - - {' '} - - {detail.content} - - ) - })} - {inlineDelegateKey === group.key ? renderSubagentList([]) : null} - + )) + } + + return ( + + {' '} + {/* String literal, not JSX text: the two trailing spaces are load-bearing + (the original's ⎿ gutter is 3 columns) and a formatter collapses + bare JSX whitespace. */} + {'⎿ '} + {detail.content} + ) })} + {inlineDelegateKey === group.key ? renderSubagentList([]) : null} + + ) + } + + // Collapsed (default) view: consecutive collapsible calls fold into one + // brief line, standalone calls keep their block, and order is preserved. + // Expanded (ctrl+o) view: every call keeps its block, blank-line separated. + const toolsFlat = + hasTools && visible.tools !== 'hidden' ? ( + + {toolsExpanded + ? groups.map((group, index) => renderGroup(group, index > 0)) + : briefRuns(groups, group => group.label).map((run, index) => + run.kind === 'flat' ? ( + + {run.items.map(group => renderGroup(group, false))} + + ) : ( + group.label))} + error={run.items.some(group => group.error)} + key={`run-${index}`} + live={run.items.some(group => group.live)} + t={t} + /> + ) + )} ) : null diff --git a/ui-tui/src/domain/toolBrief.ts b/ui-tui/src/domain/toolBrief.ts new file mode 100644 index 000000000..99a925613 --- /dev/null +++ b/ui-tui/src/domain/toolBrief.ts @@ -0,0 +1,246 @@ +import { parseToolTrailResultLine, splitToolDuration, toolTrailLabel } from '../lib/text.js' + +/** + * The original Claude Code transcript does not list every tool call on its own + * row. A run of calls collapses into a single dim summary line — the "brief": + * + * Read 3 files, listed 1 directory, ran 1 shell command + * + * and ctrl+o (details `expanded`) swaps it back for the full `⏺ Tool(args)` + * list. This module owns the brief's vocabulary: which bucket a call falls + * into, whether that bucket collapses at all, and which clauses it produces. + * + * Three rules carry most of the look: + * - only the FIRST clause is capitalized; the rest stay lowercase, joined + * by ", " ("Read 3 files, listed 1 directory"). + * - a still-running group uses the gerund and trails an ellipsis + * ("Reading 1 file…"); a settled one uses the past tense. + * - upstream only folds tools that describe themselves as a search/read + * shaped call (plus shell commands and MCP bridges). Everything else — + * edits, delegations, questions — keeps its own row, because its detail + * rows carry information a tally would destroy. See STANDALONE below. + */ +export type BriefBucket = 'agent' | 'ask' | 'bash' | 'edit' | 'list' | 'other' | 'read' | 'search' + +export type BriefCounts = Record + +/** + * Buckets that never fold into the brief — they render as their own + * `⏺ Tool(args)` block even in the collapsed view: + * + * edit — the patch itself is the point (upstream renders `⏺ Update(f)` + * with the diff under it, and never tallies it away). + * agent — the Delegate Task row anchors the inline subagent tree. + * ask — AskUserQuestion / clarify carry the ANSWERS in their detail + * rows; "called 1 tool" would delete them from the transcript. + */ +const STANDALONE: ReadonlySet = new Set(['agent', 'ask', 'edit']) + +export const isCollapsibleBucket = (bucket: BriefBucket): boolean => !STANDALONE.has(bucket) + +export const emptyBriefCounts = (): BriefCounts => ({ + agent: 0, + ask: 0, + bash: 0, + edit: 0, + list: 0, + other: 0, + read: 0, + search: 0 +}) + +const READ_TOOLS = new Set(['NotebookRead', 'Read']) +const SEARCH_TOOLS = new Set(['Glob', 'Grep']) +// `Update` is the label an Edit carries once the gateway resolved it to a +// patch; Write / NotebookEdit land here too. +const EDIT_TOOLS = new Set(['Edit', 'MultiEdit', 'NotebookEdit', 'Update', 'Write']) +const AGENT_TOOLS = new Set(['Agent', 'Delegate Task', 'Task']) +const ASK_TOOLS = new Set(['AskUserQuestion', 'Clarify']) + +/** `Read(src/a.py)` → `Read`. Legacy trail lines may still carry a `(1.2s)` + * duration suffix, so strip that before splitting on the arg paren. */ +export const briefToolName = (call: string): string => { + const { label } = splitToolDuration(call) + const paren = label.indexOf('(') + + return (paren > 0 ? label.slice(0, paren) : label).trim() +} + +/** `Bash(ls src)` → `ls src`. Empty when the call carries no args. */ +export const briefToolArgs = (call: string): string => { + const { label } = splitToolDuration(call) + const paren = label.indexOf('(') + + return paren > 0 ? label.slice(paren + 1).replace(/\)$/, '') : '' +} + +// Upstream buckets a shell call on the *command*, not the tool: `ls` reads as +// "listed 1 directory" and `grep`/`rg` as "searched for 1 pattern", so a +// transcript full of shell-driven exploration still reads as exploration. +// Anything else is a shell command. +const LIST_COMMAND = /^\s*(?:\$\s*)?(?:ls|tree)(?:\s|$)/ +const SEARCH_COMMAND = /^\s*(?:\$\s*)?(?:ag|ack|grep|egrep|fgrep|rg)(?:\s|$)/ +const READ_COMMAND = /^\s*(?:\$\s*)?(?:bat|cat|head|less|tail)(?:\s|$)/ + +export const classifyBriefTool = (call: string): BriefBucket => { + const name = briefToolName(call) + + if (READ_TOOLS.has(name)) { + return 'read' + } + + if (SEARCH_TOOLS.has(name)) { + return 'search' + } + + if (EDIT_TOOLS.has(name)) { + return 'edit' + } + + if (AGENT_TOOLS.has(name)) { + return 'agent' + } + + if (ASK_TOOLS.has(name)) { + return 'ask' + } + + if (name === 'Bash') { + const args = briefToolArgs(call) + + if (LIST_COMMAND.test(args)) { + return 'list' + } + + if (SEARCH_COMMAND.test(args)) { + return 'search' + } + + return READ_COMMAND.test(args) ? 'read' : 'bash' + } + + // Everything else — WebSearch/WebFetch, Skill, MCP bridges, task tools — is + // "called N tools", upstream's catch-all clause. (Upstream names the MCP + // *server* in a clause of its own; a trail line doesn't carry one, so MCP + // calls read as generic tool calls rather than an invented label.) + return 'other' +} + +/** + * Classification key for a RAW trail line: the `Tool(args)` call when the line + * is a completed tool result, the drafting label while one is still being + * written, and the line itself otherwise. Mirrors how ToolTrail derives + * `group.label`, so the renderer and the height estimator agree on where runs + * begin and end. + */ +export const briefCallOfTrailLine = (line: string): string => { + const parsed = parseToolTrailResultLine(line) + + if (parsed) { + return parsed.call + } + + if (line.startsWith('drafting ')) { + return toolTrailLabel(line.slice(9).replace(/…$/, '').trim()) + } + + return line +} + +export const countBriefTools = (calls: readonly string[]): BriefCounts => { + const counts = emptyBriefCounts() + + for (const call of calls) { + counts[classifyBriefTool(call)]++ + } + + return counts +} + +export const briefTotal = (counts: BriefCounts): number => Object.values(counts).reduce((sum, n: number) => sum + n, 0) + +export interface BriefClause { + /** The tally — rendered bold, between `verb` and `noun`. */ + count: number + key: BriefBucket + noun: string + /** Capitalized on the first clause only. */ + verb: string +} + +const plural = (n: number, one: string, many: string) => (n === 1 ? one : many) + +// Clause order is upstream's: the read/search band first, then the catch-all, +// with shell commands last. STANDALONE buckets never reach here. +const ORDER: { bucket: BriefBucket; live: string; noun: [string, string]; past: string }[] = [ + { bucket: 'search', live: 'searching for', noun: ['pattern', 'patterns'], past: 'searched for' }, + { bucket: 'read', live: 'reading', noun: ['file', 'files'], past: 'read' }, + { bucket: 'list', live: 'listing', noun: ['directory', 'directories'], past: 'listed' }, + { bucket: 'other', live: 'calling', noun: ['tool', 'tools'], past: 'called' }, + { bucket: 'bash', live: 'running', noun: ['shell command', 'shell commands'], past: 'ran' } +] + +export const briefClauses = (counts: BriefCounts, live = false): BriefClause[] => { + const out: BriefClause[] = [] + + for (const spec of ORDER) { + const count = counts[spec.bucket] + + if (count <= 0) { + continue + } + + const verb = live ? spec.live : spec.past + + out.push({ + count, + key: spec.bucket, + noun: plural(count, spec.noun[0], spec.noun[1]), + // Only the opening clause is capitalized — the rest read as a list. + verb: out.length === 0 ? verb[0]!.toUpperCase() + verb.slice(1) : verb + }) + } + + return out +} + +/** Plain-text form of the brief. The renderer bolds the counts; this is the + * same string without styling, for height estimation and tests. */ +export const briefText = (counts: BriefCounts, live = false): string => { + const clauses = briefClauses(counts, live) + + if (!clauses.length) { + return '' + } + + return `${clauses.map(c => `${c.verb} ${c.count} ${c.noun}`).join(', ')}${live ? '…' : ''}` +} + +/** + * Split an ordered tool list into render runs: each consecutive stretch of + * collapsible calls becomes one `brief` run, and each standalone call keeps + * its own `flat` run. Preserves order, so a Read → Delegate Task → Bash trail + * reads brief / delegate row / brief rather than reordering the turn. + */ +export interface BriefRun { + items: T[] + kind: 'brief' | 'flat' +} + +export const briefRuns = (items: readonly T[], callOf: (item: T) => string): BriefRun[] => { + const runs: BriefRun[] = [] + + for (const item of items) { + const kind = isCollapsibleBucket(classifyBriefTool(callOf(item))) ? 'brief' : 'flat' + const last = runs.at(-1) + + // Standalone rows never merge with each other — each keeps its own block. + if (last && last.kind === 'brief' && kind === 'brief') { + last.items.push(item) + } else { + runs.push({ items: [item], kind }) + } + } + + return runs +} diff --git a/ui-tui/src/lib/virtualHeights.ts b/ui-tui/src/lib/virtualHeights.ts index 1c5c914bd..b2293759f 100644 --- a/ui-tui/src/lib/virtualHeights.ts +++ b/ui-tui/src/lib/virtualHeights.ts @@ -1,4 +1,5 @@ import { TERMUX_TUI_MODE } from '../config/env.js' +import { briefCallOfTrailLine, briefRuns, briefText, countBriefTools } from '../domain/toolBrief.js' import type { Msg } from '../types.js' import { transcriptBodyWidth } from './inputMetrics.js' @@ -76,6 +77,39 @@ export const wrappedLines = (text: string, width: number, maxLines: number = MAX return n } +/** + * Rows a message's tool trail paints, matching ToolTrail's two layouts: + * + * expanded (ctrl+o) — every call keeps its `⏺ …` + `⎿ …` block, verbose + * sibling when one exists, and a blank line between consecutive blocks. + * collapsed (default) — consecutive collapsible calls fold to one brief + * line; standalone calls (edits, delegations, questions) keep their block. + * + * Both walk the same briefRuns() split the renderer uses, so the estimate and + * the paint can't disagree about where a run begins. + */ +const trailRows = (msg: Msg, bodyWidth: number, toolsExpanded: boolean) => { + const lines = msg.tools ?? [] + + if (toolsExpanded) { + const rows = lines.reduce((sum, line, i) => sum + (msg.toolsVerbose?.[i] || line).split('\n').length, 0) + + // Blank line between consecutive blocks. + return rows + Math.max(0, lines.length - 1) + } + + return briefRuns(lines, briefCallOfTrailLine).reduce((sum, run) => { + if (run.kind === 'flat') { + return sum + run.items.reduce((rows, line) => rows + line.split('\n').length, 0) + } + + // The brief renders under a 2-column gutter, so it wraps 2 narrower. + const text = briefText(countBriefTools(run.items.map(briefCallOfTrailLine))) + + return sum + (text ? wrappedLines(text, bodyWidth - 2) : 0) + }, 0) +} + export const estimatedMsgHeight = ( msg: Msg, cols: number, @@ -136,14 +170,7 @@ export const estimatedMsgHeight = ( // Tool entries can carry multi-line details (Bash 3-line summaries, // 10-line error caps) — count rendered rows, not entries, or off-screen // estimates under-count and the scrollbar/topSpacer math jumps. - const toolRows = hasVisibleTools - ? (msg.tools ?? []).reduce((sum, line, i) => { - // Expanded details render the verbose sibling when present. - const rendered = toolsExpanded && msg.toolsVerbose?.[i] ? msg.toolsVerbose[i]! : line - - return sum + rendered.split('\n').length - }, 0) - : 0 + const toolRows = hasVisibleTools ? trailRows(msg, bodyWidth, toolsExpanded) : 0 h += toolRows + (hasVisibleThinking ? wrappedLines(msg.thinking ?? '', bodyWidth) : 0) From 2974cd9b1c2d5da1f7ec56dabfa9cd42dcd3672b Mon Sep 17 00:00:00 2001 From: Eric Lee Date: Wed, 12 Aug 2026 11:55:35 -0700 Subject: [PATCH 2/6] fix(tui): brief line stayed bold, and the estimator disagreed with it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the brief-line port turned up one rendering defect and a cluster of height-estimate divergences. The brief painted bold from its first tally onward. Bold and faint share SGR close code 22, so wrapping bold counts in a `dim` parent rewrites each count's `\e[22m` into the parent's `\e[2m` — faint reopened, bold never cleared. `muted` alone carries the quiet now, and a test asserts on the bytes, which is the only place this is visible: stripAnsi cannot see it. The estimator was counting raw trail lines where the renderer counts painted blocks: it charged a blank-line gap for meta notes that render in the activity panel instead, never counted the `⏺` call row, wrapped the brief at the prose width (a trail has no role gutter, so it is three columns wider), and gave every trail the one-row text floor that only prose rows pay. A `trail` block's estimate could be double its paint. All four are fixed against a new test that renders five trail shapes in both modes and asserts the estimate equals the painted row count — the two implementations can no longer drift silently. Failures now break out of a run. Upstream tallies a failed call away like any other (a `cat` of a missing file still reads "Read 1 file"), but the error text lives only in that call's ⎿ row, so folding it put the one thing worth reading behind ctrl+o. The successes around it still fold. That also retires the red-bullet marker the previous commit used as a half-measure. Also: advisor / vision_analyze / ExitPlanMode join AskUserQuestion as standalone — their detail rows ARE the result; `/details tools expanded` selects the full rows again instead of silently meaning "brief"; blank lines now separate collapsed blocks the way they separate expanded ones; and the dead tool names (NotebookRead, MultiEdit, Update) and their untrue comment are gone, since this backend registers none of them. Co-Authored-By: Claude Opus 5 --- ui-tui/src/__tests__/toolBrief.test.ts | 160 ++++++++++++++++++++++++- ui-tui/src/app/useMainApp.ts | 7 +- ui-tui/src/components/thinking.tsx | 60 +++++----- ui-tui/src/domain/toolBrief.ts | 50 +++++--- ui-tui/src/lib/inputMetrics.ts | 9 ++ ui-tui/src/lib/virtualHeights.ts | 79 +++++++++--- 6 files changed, 298 insertions(+), 67 deletions(-) diff --git a/ui-tui/src/__tests__/toolBrief.test.ts b/ui-tui/src/__tests__/toolBrief.test.ts index ede623383..2339c6d57 100644 --- a/ui-tui/src/__tests__/toolBrief.test.ts +++ b/ui-tui/src/__tests__/toolBrief.test.ts @@ -24,7 +24,9 @@ import { emptyBriefCounts } from '../domain/toolBrief.js' import { buildToolTrailLine, stripAnsi } from '../lib/text.js' +import { estimatedMsgHeight } from '../lib/virtualHeights.js' import { DEFAULT_THEME } from '../theme.js' +import type { Msg } from '../types.js' // ── classification ────────────────────────────────────────────────────────── @@ -45,11 +47,15 @@ describe('classifyBriefTool', () => { expect(classifyBriefTool('Bash(catalog --build)')).toBe('bash') }) - it('keeps edits, delegations and questions out of the tally', () => { + it('keeps edits, delegations and answer-bearing tools out of the tally', () => { expect(classifyBriefTool('Edit(a.py)')).toBe('edit') expect(classifyBriefTool('Write(b.py)')).toBe('edit') expect(classifyBriefTool('Delegate Task(audit)')).toBe('agent') - expect(classifyBriefTool('AskUserQuestion(pick one)')).toBe('ask') + expect(classifyBriefTool('AskUserQuestion(pick one)')).toBe('answer') + // Labels, not wire names: toolTrailLabel() title-cases and de-snakes. + expect(classifyBriefTool('Advisor(is this sound?)')).toBe('answer') + expect(classifyBriefTool('Vision Analyze(shot.png)')).toBe('answer') + expect(classifyBriefTool('ExitPlanMode(plan)')).toBe('answer') }) it('falls back to the catch-all bucket', () => { @@ -120,6 +126,13 @@ describe('briefRuns', () => { expect(runs.map(r => r.kind)).toEqual(['flat', 'flat']) }) + + it('breaks a failing call out so its message survives the fold', () => { + const runs = briefRuns(['Read(a)', 'Bash(boom)', 'Read(c)'], id, item => item === 'Bash(boom)') + + expect(runs.map(r => r.kind)).toEqual(['brief', 'flat', 'brief']) + expect(runs[1]!.items).toEqual(['Bash(boom)']) + }) }) describe('briefCallOfTrailLine', () => { @@ -134,6 +147,22 @@ describe('briefCallOfTrailLine', () => { // ── render ────────────────────────────────────────────────────────────────── +// Ink brackets every frame in a synchronized-update pair (BSU … ESU). Into a +// PassThrough — which is not a TTY, so nothing gets overwritten — it flushes +// the same frame more than once and they concatenate, which turns any row +// count into 2n-1. Keep the last frame that painted something. +const BSU = '[?2026h' +const ESU = '[?2026l' + +const lastFrame = (output: string): string => { + const frames = output + .split(BSU) + .map(chunk => chunk.split(ESU)[0] ?? '') + .filter(frame => stripAnsi(frame).trim() !== '') + + return frames.at(-1) ?? '' +} + const renderToString = (element: React.ReactElement): string => { const stdout = new PassThrough() const stdin = new PassThrough() @@ -142,15 +171,22 @@ const renderToString = (element: React.ReactElement): string => { Object.assign(stdout, { columns: 100, isTTY: false, rows: 40 }) Object.assign(stdin, { isTTY: false }) + Object.assign(stderr, { isTTY: false }) stdout.on('data', (chunk: Buffer) => { output += chunk.toString() }) - const instance = renderSync(element, { stderr: stderr as never, stdin: stdin as never, stdout: stdout as never }) + const instance = renderSync(element, { + patchConsole: false, + stderr: stderr as never, + stdin: stdin as never, + stdout: stdout as never + }) instance.unmount() + instance.cleanup() - return output + return lastFrame(output) } describe('ToolTrail brief render', () => { @@ -227,4 +263,120 @@ describe('ToolTrail brief render', () => { expect(row).toMatch(/^ {2}Read 2 files/) }) + + it('breaks a failed call out of the brief so its error stays readable', () => { + const withError = [ + trail[0]!, + buildToolTrailLine('Bash', 'cat missing.txt', true, 'Error: No such file or directory'), + trail[3]! + ] + + const out = stripAnsi( + renderToString(React.createElement(ToolTrail, { detailsMode: 'collapsed', t: DEFAULT_THEME, trail: withError })) + ) + + expect(out).toContain('Bash(cat missing.txt)') + expect(out).toContain('Error: No such file or directory') + // The successes around it still fold. + expect(out).toContain('Read 1 file') + expect(out).toContain('Ran 1 shell command') + }) + + // Bold and faint share SGR close code 22, so a `dim` wrapper rewrites each + // bold tally's reset into `\e[2m` — faint re-opened, bold never cleared — + // and every column after the first tally paints bold. stripAnsi cannot see + // this, so assert on the bytes. + it('closes each bold tally without leaving the rest of the line bold', () => { + const out = renderToString(React.createElement(ToolTrail, { detailsMode: 'collapsed', t: DEFAULT_THEME, trail })) + const BOLD = '\u001b[1m' + const RESET_INTENSITY = '\u001b[22m' + const FAINT = '\u001b[2m' + + expect(out).toContain(`${BOLD}2${RESET_INTENSITY}`) + // No faint is opened anywhere, so no tally's reset can be rewritten into + // one — bold cannot survive past the tally that opened it. + expect(out).not.toContain(FAINT) + }) +}) + +// ── estimate vs paint ─────────────────────────────────────────────────────── + +// The virtualized transcript positions rows from estimatedMsgHeight before +// Yoga has measured anything, so an estimate that disagrees with the paint +// shows up as scrollbar drift and blank gaps. Assert the two agree on real +// trails rather than trusting the two implementations to stay in step. +describe('estimatedMsgHeight matches the painted trail', () => { + const paintedRows = (msg: Msg, detailsMode: 'collapsed' | 'expanded') => { + const rows = stripAnsi( + renderToString( + React.createElement(ToolTrail, { + detailsMode, + t: DEFAULT_THEME, + trail: msg.tools ?? [], + verboseTrail: msg.toolsVerbose ?? [] + }) + ) + ).split('\n') + + while (rows.length && rows[rows.length - 1]!.trim() === '') { + rows.pop() + } + + return rows.length + } + + const trailMsg = (tools: string[], toolsVerbose?: string[]): Msg => ({ + kind: 'trail', + role: 'system', + text: '', + tools, + ...(toolsVerbose ? { toolsVerbose } : {}) + }) + + const cases: [string, Msg][] = [ + ['a plain run', trailMsg([buildToolTrailLine('Read', 'a.py', false, 'Read 8 lines')])], + [ + 'a mixed run', + trailMsg([ + buildToolTrailLine('Read', 'a.py', false, 'Read 8 lines'), + buildToolTrailLine('Bash', 'ls src', false, 'a.py\nb.py'), + buildToolTrailLine('Bash', 'echo hi', false, 'hi') + ]) + ], + [ + 'a run split by a standalone edit', + trailMsg([ + buildToolTrailLine('Read', 'a.py', false, 'Read 8 lines'), + buildToolTrailLine('Edit', 'b.py', false, 'Updated b.py'), + buildToolTrailLine('Bash', 'echo hi', false, 'hi') + ]) + ], + [ + 'a run split by a failure', + trailMsg([ + buildToolTrailLine('Read', 'a.py', false, 'Read 8 lines'), + buildToolTrailLine('Bash', 'cat nope', true, 'Error: No such file'), + buildToolTrailLine('Bash', 'echo hi', false, 'hi') + ]) + ], + [ + 'a call with no result row', + trailMsg([ + buildToolTrailLine('Bash', 'true', false, ''), + buildToolTrailLine('Read', 'a.py', false, 'Read 1 line') + ]) + ] + ] + + for (const [name, msg] of cases) { + it(`agrees on ${name} (collapsed)`, () => { + expect(estimatedMsgHeight(msg, 100, { compact: false, details: true })).toBe(paintedRows(msg, 'collapsed')) + }) + + it(`agrees on ${name} (expanded)`, () => { + expect(estimatedMsgHeight(msg, 100, { compact: false, details: true, toolsExpanded: true })).toBe( + paintedRows(msg, 'expanded') + ) + }) + } }) diff --git a/ui-tui/src/app/useMainApp.ts b/ui-tui/src/app/useMainApp.ts index 9e1b8a6f8..bd22bc099 100644 --- a/ui-tui/src/app/useMainApp.ts +++ b/ui-tui/src/app/useMainApp.ts @@ -10,6 +10,7 @@ import { import { useStore } from '@nanostores/react' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import type { QuestionAnswers } from '../components/questionPrompt.js' import { STARTUP_RESUME_ID, TRANSCRIPT_COLOR } from '../config/env.js' import { MAX_HISTORY, WHEEL_SCROLL_STEP } from '../config/limits.js' import { hasLeadGap, prevRenderedMsg, showsInterTurnSeparator } from '../domain/blockLayout.js' @@ -26,7 +27,6 @@ import type { SessionCloseResponse, TerminalResizeResponse } from '../gatewayTypes.js' -import type { QuestionAnswers } from '../components/questionPrompt.js' import { useGitBranch } from '../hooks/useGitBranch.js' import { useVirtualHistory } from '../hooks/useVirtualHistory.js' import { composerPromptWidth } from '../lib/inputMetrics.js' @@ -353,7 +353,10 @@ export function useMainApp(gw: GatewayClient) { const [thinkingDetailsMode, toolsDetailsMode] = detailsLayoutKey.split(':') const thinkingDetailsVisible = thinkingDetailsMode !== 'hidden' const toolsDetailsVisible = toolsDetailsMode !== 'hidden' - const toolsDetailsExpanded = ui.detailsMode === 'expanded' + // Mirrors ToolTrail's `toolsExpanded` exactly (ctrl+o, or an explicit + // `/details tools expanded` pin) — it picks the flat per-call rows over the + // collapsed brief, so the estimate and the paint must derive it the same way. + const toolsDetailsExpanded = ui.detailsMode === 'expanded' || ui.sections?.tools === 'expanded' const detailsVisible = thinkingDetailsVisible || toolsDetailsVisible const userPromptWidth = composerPromptWidth(ui.theme.brand.prompt) const heightCacheKey = `${ui.sid ?? 'draft'}:${cols}:${userPromptWidth}:${ui.compact ? '1' : '0'}:${detailsLayoutKey}` diff --git a/ui-tui/src/components/thinking.tsx b/ui-tui/src/components/thinking.tsx index 5a064452e..d5928494a 100644 --- a/ui-tui/src/components/thinking.tsx +++ b/ui-tui/src/components/thinking.tsx @@ -267,25 +267,29 @@ function Chevron({ /** * The collapsed tool row — the original's brief line. A run of tool calls - * reads as one dim sentence ("Read 3 files, listed 1 directory") with the + * reads as one quiet sentence ("Read 3 files, listed 1 directory") with the * tallies bold, under a 2-column gutter so it lines up with the `⏺ ` bullets - * the expanded view (ctrl+o) puts back. + * the expanded view (ctrl+o) puts back. A blinking bullet fills the gutter + * while the run is still executing (the original animates a dot there). * - * Two gutter states: a blinking bullet while the run is still executing (the - * original animates a dot there), and — a deliberate departure from upstream, - * which shows nothing — a red bullet when a call in the run failed, so a - * failure is never silently tallied away as a plain count. + * The quiet comes from `muted`, NOT from `dim`: bold and faint share SGR + * close code 22, so wrapping bold tallies in a dim parent rewrites each + * tally's `\e[22m` into the parent's `\e[2m` — which re-opens faint without + * ever clearing bold, and every column after the first tally paints bold. + * (The ink fork's own Text props encode this as `dim?: never` alongside + * `bold`; nesting the two in separate elements evades the type, not the + * terminal.) Keep them apart. */ function BriefLine({ blinkOn, counts, - error, + gap, live, t }: { blinkOn: boolean counts: BriefCounts - error: boolean + gap: boolean live: boolean t: Theme }) { @@ -295,20 +299,12 @@ function BriefLine({ return null } - const gutter = live ? ( - {blinkOn ? '⏺ ' : ' '} - ) : error ? ( - - ) : ( - {' '} - ) - return ( - + - {gutter} + {live ? {blinkOn ? '⏺ ' : ' '} : {' '}} - + {clauses.map((clause, index) => ( {index > 0 ? ', ' : ''} @@ -846,10 +842,15 @@ export const ToolTrail = memo(function ToolTrail({ const meta: DetailRow[] = [] const pushDetail = (row: DetailRow) => (groups.at(-1)?.details ?? meta).push(row) - // Verbose swap keys on the GLOBAL details mode (ctrl+o / /details) — the - // tools *section* mode defaults to 'expanded' merely to make the flat - // trail visible, and must not force verbose output. - const toolsExpanded = detailsMode === 'expanded' + // "Show every call, with whatever raw output the gateway kept" — the + // opposite of the collapsed brief. Driven by ctrl+o (the global details + // mode) or by an EXPLICIT `/details tools expanded` pin. Reading the raw + // `sections` override, not the resolved mode, matters: the tools section + // defaults to 'expanded' just to keep the trail visible, and treating that + // default as a pin would disable the brief for everyone. + // useMainApp derives the height estimator's flag the same way — one signal + // for both, or the estimate drifts from the paint. + const toolsExpanded = detailsMode === 'expanded' || sections?.tools === 'expanded' for (const [i, compactLine] of trail.entries()) { // Expanded details render the verbose sibling (full Args/Result blocks) @@ -1184,22 +1185,27 @@ export const ToolTrail = memo(function ToolTrail({ // Collapsed (default) view: consecutive collapsible calls fold into one // brief line, standalone calls keep their block, and order is preserved. - // Expanded (ctrl+o) view: every call keeps its block, blank-line separated. + // Expanded (ctrl+o) view: every call keeps its block. Either way a blank + // line separates consecutive blocks, as upstream renders them. const toolsFlat = hasTools && visible.tools !== 'hidden' ? ( {toolsExpanded ? groups.map((group, index) => renderGroup(group, index > 0)) - : briefRuns(groups, group => group.label).map((run, index) => + : briefRuns( + groups, + group => group.label, + group => Boolean(group.error) + ).map((run, index) => run.kind === 'flat' ? ( - {run.items.map(group => renderGroup(group, false))} + {run.items.map((group, item) => renderGroup(group, index > 0 || item > 0))} ) : ( group.label))} - error={run.items.some(group => group.error)} + gap={index > 0} key={`run-${index}`} live={run.items.some(group => group.live)} t={t} diff --git a/ui-tui/src/domain/toolBrief.ts b/ui-tui/src/domain/toolBrief.ts index 99a925613..003c06750 100644 --- a/ui-tui/src/domain/toolBrief.ts +++ b/ui-tui/src/domain/toolBrief.ts @@ -20,7 +20,7 @@ import { parseToolTrailResultLine, splitToolDuration, toolTrailLabel } from '../ * edits, delegations, questions — keeps its own row, because its detail * rows carry information a tally would destroy. See STANDALONE below. */ -export type BriefBucket = 'agent' | 'ask' | 'bash' | 'edit' | 'list' | 'other' | 'read' | 'search' +export type BriefBucket = 'agent' | 'answer' | 'bash' | 'edit' | 'list' | 'other' | 'read' | 'search' export type BriefCounts = Record @@ -31,16 +31,18 @@ export type BriefCounts = Record * edit — the patch itself is the point (upstream renders `⏺ Update(f)` * with the diff under it, and never tallies it away). * agent — the Delegate Task row anchors the inline subagent tree. - * ask — AskUserQuestion / clarify carry the ANSWERS in their detail - * rows; "called 1 tool" would delete them from the transcript. + * answer — AskUserQuestion, clarify, advisor, vision_analyze, ExitPlanMode: + * the detail rows ARE the result (the user's own choices, the + * advisor's opinion, the plan). "Called 1 tool" would delete them + * from the transcript with no way to get them back. */ -const STANDALONE: ReadonlySet = new Set(['agent', 'ask', 'edit']) +const STANDALONE: ReadonlySet = new Set(['agent', 'answer', 'edit']) export const isCollapsibleBucket = (bucket: BriefBucket): boolean => !STANDALONE.has(bucket) export const emptyBriefCounts = (): BriefCounts => ({ agent: 0, - ask: 0, + answer: 0, bash: 0, edit: 0, list: 0, @@ -49,13 +51,13 @@ export const emptyBriefCounts = (): BriefCounts => ({ search: 0 }) -const READ_TOOLS = new Set(['NotebookRead', 'Read']) +// Labels, not wire names: the trail carries what toolTrailLabel() produced, so +// `vision_analyze` arrives as `Vision Analyze`. +const READ_TOOLS = new Set(['Read']) const SEARCH_TOOLS = new Set(['Glob', 'Grep']) -// `Update` is the label an Edit carries once the gateway resolved it to a -// patch; Write / NotebookEdit land here too. -const EDIT_TOOLS = new Set(['Edit', 'MultiEdit', 'NotebookEdit', 'Update', 'Write']) +const EDIT_TOOLS = new Set(['Edit', 'NotebookEdit', 'Write']) const AGENT_TOOLS = new Set(['Agent', 'Delegate Task', 'Task']) -const ASK_TOOLS = new Set(['AskUserQuestion', 'Clarify']) +const ANSWER_TOOLS = new Set(['Advisor', 'AskUserQuestion', 'Clarify', 'ExitPlanMode', 'Vision Analyze']) /** `Read(src/a.py)` → `Read`. Legacy trail lines may still carry a `(1.2s)` * duration suffix, so strip that before splitting on the arg paren. */ @@ -78,6 +80,13 @@ export const briefToolArgs = (call: string): string => { // "listed 1 directory" and `grep`/`rg` as "searched for 1 pattern", so a // transcript full of shell-driven exploration still reads as exploration. // Anything else is a shell command. +// +// Deliberately a prefix match on the bare command, so anything wrapped — +// `sudo ls`, `FOO=1 ls`, `cd x && ls`, a pipeline that greps midway — falls +// through to "shell command". Under-claiming reads fine; a `cat > file.py` +// heredoc counted as a read would not. The args this matches against are the +// compactPreview head, but every pattern is anchored, so truncation can never +// change a verdict. const LIST_COMMAND = /^\s*(?:\$\s*)?(?:ls|tree)(?:\s|$)/ const SEARCH_COMMAND = /^\s*(?:\$\s*)?(?:ag|ack|grep|egrep|fgrep|rg)(?:\s|$)/ const READ_COMMAND = /^\s*(?:\$\s*)?(?:bat|cat|head|less|tail)(?:\s|$)/ @@ -101,8 +110,8 @@ export const classifyBriefTool = (call: string): BriefBucket => { return 'agent' } - if (ASK_TOOLS.has(name)) { - return 'ask' + if (ANSWER_TOOLS.has(name)) { + return 'answer' } if (name === 'Bash') { @@ -157,8 +166,6 @@ export const countBriefTools = (calls: readonly string[]): BriefCounts => { return counts } -export const briefTotal = (counts: BriefCounts): number => Object.values(counts).reduce((sum, n: number) => sum + n, 0) - export interface BriefClause { /** The tally — rendered bold, between `verb` and `noun`. */ count: number @@ -221,17 +228,28 @@ export const briefText = (counts: BriefCounts, live = false): string => { * collapsible calls becomes one `brief` run, and each standalone call keeps * its own `flat` run. Preserves order, so a Read → Delegate Task → Bash trail * reads brief / delegate row / brief rather than reordering the turn. + * + * `standalone` forces an item out of the brief regardless of its bucket. The + * renderer passes failures through it: upstream tallies a failed call away + * like any other (a `cat` of a missing file still reads "Read 1 file"), but + * the error message lives only in that call's `⎿` row, so folding it would + * make the one thing worth reading unreachable without ctrl+o. */ export interface BriefRun { items: T[] kind: 'brief' | 'flat' } -export const briefRuns = (items: readonly T[], callOf: (item: T) => string): BriefRun[] => { +export const briefRuns = ( + items: readonly T[], + callOf: (item: T) => string, + standalone?: (item: T) => boolean +): BriefRun[] => { const runs: BriefRun[] = [] for (const item of items) { - const kind = isCollapsibleBucket(classifyBriefTool(callOf(item))) ? 'brief' : 'flat' + const collapsible = !standalone?.(item) && isCollapsibleBucket(classifyBriefTool(callOf(item))) + const kind = collapsible ? 'brief' : 'flat' const last = runs.at(-1) // Standalone rows never merge with each other — each keeps its own block. diff --git a/ui-tui/src/lib/inputMetrics.ts b/ui-tui/src/lib/inputMetrics.ts index ae3395405..30cdb53b7 100644 --- a/ui-tui/src/lib/inputMetrics.ts +++ b/ui-tui/src/lib/inputMetrics.ts @@ -191,6 +191,15 @@ export function transcriptBodyWidth(totalCols: number, role: Role, userPrompt: s return Math.max(20, available) } +/** Width a tool trail renders at. Unlike prose rows, a `kind: 'trail'` block + * has no role gutter — MessageLine hands ToolTrail the whole transcript + * interior — so it is `transcriptBodyWidth` WITHOUT the 3-column gutter. */ +export function transcriptTrailWidth(totalCols: number, termuxMode = false) { + const horizontalReserve = termuxMode ? 2 : 4 + + return Math.max(1, totalCols - horizontalReserve) +} + /** Scrollbar gutter beside the transcript: `marginLeft={1}` + `width={1}` on * the `TranscriptScrollbar` wrapper in appLayout. Always reserved — the * scrollbar renders a `width={1}` spacer even when it has nothing to show. */ diff --git a/ui-tui/src/lib/virtualHeights.ts b/ui-tui/src/lib/virtualHeights.ts index b2293759f..3ed8c69e4 100644 --- a/ui-tui/src/lib/virtualHeights.ts +++ b/ui-tui/src/lib/virtualHeights.ts @@ -2,7 +2,8 @@ import { TERMUX_TUI_MODE } from '../config/env.js' import { briefCallOfTrailLine, briefRuns, briefText, countBriefTools } from '../domain/toolBrief.js' import type { Msg } from '../types.js' -import { transcriptBodyWidth } from './inputMetrics.js' +import { transcriptBodyWidth, transcriptTrailWidth } from './inputMetrics.js' +import { parseToolTrailResultLine } from './text.js' const hashText = (text: string) => { let h = 5381 @@ -77,36 +78,75 @@ export const wrappedLines = (text: string, width: number, maxLines: number = MAX return n } +/** + * One entry per trail line that actually paints a `⏺ Tool(args)` block, with + * the rows it costs. Lines that produce no group — gateway meta notes, which + * ToolTrail routes to the (hidden-by-default) activity panel — drop out here, + * so both layouts below count blocks, never raw lines. + */ +interface TrailEntry { + call: string + error: boolean + rows: number +} + +const trailEntries = (msg: Msg, toolsExpanded: boolean): TrailEntry[] => { + const entries: TrailEntry[] = [] + + for (const [i, line] of (msg.tools ?? []).entries()) { + const rendered = (toolsExpanded && msg.toolsVerbose?.[i]) || line + const parsed = parseToolTrailResultLine(rendered) + + if (parsed) { + entries.push({ + call: parsed.call, + error: parsed.mark === '✗', + // The `⏺` call row, then one row per line of the `⎿` detail. + rows: 1 + (parsed.detail ? parsed.detail.split('\n').length : 0) + }) + } else if (line.startsWith('drafting ')) { + // Call row + the static "drafting..." detail row. + entries.push({ call: briefCallOfTrailLine(line), error: false, rows: 2 }) + } + } + + return entries +} + /** * Rows a message's tool trail paints, matching ToolTrail's two layouts: * * expanded (ctrl+o) — every call keeps its `⏺ …` + `⎿ …` block, verbose - * sibling when one exists, and a blank line between consecutive blocks. + * sibling when one exists. * collapsed (default) — consecutive collapsible calls fold to one brief - * line; standalone calls (edits, delegations, questions) keep their block. + * line; standalone calls (edits, delegations, questions) and failures + * keep their block. * - * Both walk the same briefRuns() split the renderer uses, so the estimate and - * the paint can't disagree about where a run begins. + * Both walk the same briefRuns() split the renderer uses, and both add the + * blank line it opens between consecutive blocks. */ -const trailRows = (msg: Msg, bodyWidth: number, toolsExpanded: boolean) => { - const lines = msg.tools ?? [] +const trailRows = (msg: Msg, trailWidth: number, toolsExpanded: boolean) => { + const entries = trailEntries(msg, toolsExpanded) if (toolsExpanded) { - const rows = lines.reduce((sum, line, i) => sum + (msg.toolsVerbose?.[i] || line).split('\n').length, 0) - - // Blank line between consecutive blocks. - return rows + Math.max(0, lines.length - 1) + return entries.reduce((sum, entry) => sum + entry.rows, 0) + Math.max(0, entries.length - 1) } - return briefRuns(lines, briefCallOfTrailLine).reduce((sum, run) => { + return briefRuns( + entries, + entry => entry.call, + entry => entry.error + ).reduce((sum, run, index) => { + const gap = index > 0 ? 1 : 0 + if (run.kind === 'flat') { - return sum + run.items.reduce((rows, line) => rows + line.split('\n').length, 0) + return sum + gap + run.items.reduce((rows, entry) => rows + entry.rows, 0) } - // The brief renders under a 2-column gutter, so it wraps 2 narrower. - const text = briefText(countBriefTools(run.items.map(briefCallOfTrailLine))) + // The brief sits under a 2-column gutter, so it wraps 2 narrower. + const text = briefText(countBriefTools(run.items.map(entry => entry.call))) - return sum + (text ? wrappedLines(text, bodyWidth - 2) : 0) + return sum + gap + (text ? wrappedLines(text, trailWidth - 2) : 0) }, 0) } @@ -150,7 +190,10 @@ export const estimatedMsgHeight = ( const bodyWidth = transcriptBodyWidth(cols, msg.role, userPrompt, TERMUX_TUI_MODE) const text = msg.text - let h = wrappedLines(text || ' ', bodyWidth) + // A `trail` block paints no text row at all (MessageLine hands it straight + // to ToolTrail), so it must not be charged the one-row floor every prose + // block gets — that alone doubled the estimate for a one-row brief. + let h = msg.kind === 'trail' ? 0 : wrappedLines(text || ' ', bodyWidth) if (!compact && msg.role === 'assistant') { // Paragraph gaps add up to 6 extra rows of breathing room. Slice @@ -170,7 +213,7 @@ export const estimatedMsgHeight = ( // Tool entries can carry multi-line details (Bash 3-line summaries, // 10-line error caps) — count rendered rows, not entries, or off-screen // estimates under-count and the scrollbar/topSpacer math jumps. - const toolRows = hasVisibleTools ? trailRows(msg, bodyWidth, toolsExpanded) : 0 + const toolRows = hasVisibleTools ? trailRows(msg, transcriptTrailWidth(cols, TERMUX_TUI_MODE), toolsExpanded) : 0 h += toolRows + (hasVisibleThinking ? wrappedLines(msg.thinking ?? '', bodyWidth) : 0) From b29c8f217f33310b4c66700ad5606e03a4130373 Mon Sep 17 00:00:00 2001 From: Eric Lee Date: Wed, 12 Aug 2026 12:12:23 -0700 Subject: [PATCH 3/6] fix(tui): estimator undercounted reasoning trails and wrapped rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second review pass. Removing the one-row text floor from `trail` blocks in the last commit was right for tool-only trails but wrong everywhere else: that floor had been silently paying for the `∴ Thinking…` header the reasoning panel paints and the estimator never counted. Since turnController emits a reasoning trail for every turn that thinks, that traded a doubling on one shape for a −1 on the common one. The header is charged explicitly now, at the rail-indented width the body actually wraps at. Trail rows were also counted as lines rather than wrapped rows. An Edit's detail is `Updated with N additions…` and buildToolTrailLine caps it at twelve LINES, never at columns, so it wraps on any ordinary terminal — est 2 / paint 4 at eighty columns. Call rows and detail rows now go through wrappedLines at their real gutters (2 and 5). The parity test could not have caught either: it rendered at the terminal width while the estimator modelled the trail width, which is four narrower, so it agreed only on shapes that never wrapped — and would have failed against correct code on any shape that did. It renders at transcriptTrailWidth() now, runs at 60 and 100 columns, and covers wrapping briefs, wrapping details, and reasoning trails. Measuring the prose path to settle whether the header row generalized turned up a third gap, older than this branch: a prose row wraps its details in a Box with marginBottom={1} that nothing counted, so every assistant message with reasoning or tools was one row short. Fixed, and the parity suite now renders MessageLine too, so prose chrome is covered by measurement instead of by a hand-written constant. That is what moves virtualHeights' own response-separator expectation from +3 to +5. Last: `/details tools expanded` collided in the height cache key. The key re-derived `detailsMode === 'expanded'` while the layout now also flips on an explicit tools pin, so pinning changed the paint without changing the key and served the brief's heights to the expanded layout. It keys on the same flag the renderer reads. Co-Authored-By: Claude Opus 5 --- ui-tui/src/__tests__/toolBrief.test.ts | 105 +++++++++++++++++--- ui-tui/src/__tests__/virtualHeights.test.ts | 6 +- ui-tui/src/app/useMainApp.ts | 22 ++-- ui-tui/src/components/thinking.tsx | 7 +- ui-tui/src/lib/virtualHeights.ts | 54 ++++++++-- 5 files changed, 161 insertions(+), 33 deletions(-) diff --git a/ui-tui/src/__tests__/toolBrief.test.ts b/ui-tui/src/__tests__/toolBrief.test.ts index 2339c6d57..307b593a2 100644 --- a/ui-tui/src/__tests__/toolBrief.test.ts +++ b/ui-tui/src/__tests__/toolBrief.test.ts @@ -13,6 +13,7 @@ vi.hoisted(() => { delete process.env.NO_COLOR }) +import { MessageLine } from '../components/messageLine.js' import { ToolTrail } from '../components/thinking.js' import { briefCallOfTrailLine, @@ -23,6 +24,7 @@ import { classifyBriefTool, emptyBriefCounts } from '../domain/toolBrief.js' +import { transcriptTrailWidth } from '../lib/inputMetrics.js' import { buildToolTrailLine, stripAnsi } from '../lib/text.js' import { estimatedMsgHeight } from '../lib/virtualHeights.js' import { DEFAULT_THEME } from '../theme.js' @@ -163,13 +165,13 @@ const lastFrame = (output: string): string => { return frames.at(-1) ?? '' } -const renderToString = (element: React.ReactElement): string => { +const renderToString = (element: React.ReactElement, columns = 100): string => { const stdout = new PassThrough() const stdin = new PassThrough() const stderr = new PassThrough() let output = '' - Object.assign(stdout, { columns: 100, isTTY: false, rows: 40 }) + Object.assign(stdout, { columns, isTTY: false, rows: 40 }) Object.assign(stdin, { isTTY: false }) Object.assign(stderr, { isTTY: false }) stdout.on('data', (chunk: Buffer) => { @@ -305,16 +307,24 @@ describe('ToolTrail brief render', () => { // Yoga has measured anything, so an estimate that disagrees with the paint // shows up as scrollbar drift and blank gaps. Assert the two agree on real // trails rather than trusting the two implementations to stay in step. +const READ_LINE = buildToolTrailLine('Read', 'a.py', false, 'Read 8 lines') + describe('estimatedMsgHeight matches the painted trail', () => { - const paintedRows = (msg: Msg, detailsMode: 'collapsed' | 'expanded') => { + // A `kind: 'trail'` block gets the transcript interior with no role gutter, + // which is narrower than the terminal. Render at that same width — pulled + // from the helper the estimator uses — or the comparison is against a paint + // the app never produces, and every wrapping case reads backwards. + const paintedRows = (msg: Msg, detailsMode: 'collapsed' | 'expanded', cols: number) => { const rows = stripAnsi( renderToString( React.createElement(ToolTrail, { detailsMode, + reasoning: msg.thinking ?? '', t: DEFAULT_THEME, trail: msg.tools ?? [], verboseTrail: msg.toolsVerbose ?? [] - }) + }), + transcriptTrailWidth(cols) ) ).split('\n') @@ -365,18 +375,87 @@ describe('estimatedMsgHeight matches the painted trail', () => { buildToolTrailLine('Bash', 'true', false, ''), buildToolTrailLine('Read', 'a.py', false, 'Read 1 line') ]) + ], + [ + // Edit details carry a full path and are capped in LINES, never columns, + // so this is the ordinary case on a normal-width terminal, not a corner. + 'a detail long enough to wrap', + trailMsg([ + buildToolTrailLine( + 'Edit', + 'src/components/transcript/toolTrail.tsx', + false, + 'Updated src/components/transcript/toolTrail.tsx with 3 additions and 1 removal' + ) + ]) + ], + [ + 'a brief long enough to wrap', + trailMsg([ + buildToolTrailLine('Grep', 'TODO', false, 'Found 2 lines'), + buildToolTrailLine('Read', 'a.py', false, 'Read 8 lines'), + buildToolTrailLine('Bash', 'ls src', false, 'a.py'), + buildToolTrailLine('WebSearch', 'rust', false, 'Did 1 search'), + buildToolTrailLine('Bash', 'echo hi', false, 'hi') + ]) + ], + ['a reasoning trail', { kind: 'trail', role: 'system', text: '', thinking: 'I should check the parser first.' }], + [ + 'reasoning plus tools', + { + kind: 'trail', + role: 'system', + text: '', + thinking: 'First line.\nSecond line.\nThird line.', + tools: [buildToolTrailLine('Read', 'a.py', false, 'Read 8 lines')] + } ] ] - for (const [name, msg] of cases) { - it(`agrees on ${name} (collapsed)`, () => { - expect(estimatedMsgHeight(msg, 100, { compact: false, details: true })).toBe(paintedRows(msg, 'collapsed')) - }) + // Prose rows carry the trail through MessageLine, which adds chrome the + // estimator has to model too: the details wrapper's margin, the `└─ + // Response` separator, and the reasoning panel's own header row. + describe('through MessageLine', () => { + const proseRows = (msg: Msg, cols: number) => { + const rows = stripAnsi( + renderToString(React.createElement(MessageLine, { cols, msg, t: DEFAULT_THEME }), cols) + ).split('\n') - it(`agrees on ${name} (expanded)`, () => { - expect(estimatedMsgHeight(msg, 100, { compact: false, details: true, toolsExpanded: true })).toBe( - paintedRows(msg, 'expanded') - ) - }) + while (rows.length && rows[rows.length - 1]!.trim() === '') { + rows.pop() + } + + return rows.length + } + + const proseCases: [string, Msg][] = [ + ['bare assistant prose', { role: 'assistant', text: 'ok' }], + ['prose with reasoning', { role: 'assistant', text: 'ok', thinking: 'plan' }], + ['prose with a tool brief', { role: 'assistant', text: 'ok', tools: [READ_LINE] }], + ['prose with both', { role: 'assistant', text: 'ok', thinking: 'plan', tools: [READ_LINE] }] + ] + + for (const [name, msg] of proseCases) { + it(`agrees on ${name}`, () => { + expect(estimatedMsgHeight(msg, 80, { compact: false, details: true })).toBe(proseRows(msg, 80)) + }) + } + }) + + // Narrow widths are where wrapping bites; 100 is the everyday case. + for (const cols of [60, 100]) { + for (const [name, msg] of cases) { + it(`agrees on ${name} (collapsed, cols=${cols})`, () => { + expect(estimatedMsgHeight(msg, cols, { compact: false, details: true })).toBe( + paintedRows(msg, 'collapsed', cols) + ) + }) + + it(`agrees on ${name} (expanded, cols=${cols})`, () => { + expect(estimatedMsgHeight(msg, cols, { compact: false, details: true, toolsExpanded: true })).toBe( + paintedRows(msg, 'expanded', cols) + ) + }) + } } }) diff --git a/ui-tui/src/__tests__/virtualHeights.test.ts b/ui-tui/src/__tests__/virtualHeights.test.ts index 011206608..d6ae23096 100644 --- a/ui-tui/src/__tests__/virtualHeights.test.ts +++ b/ui-tui/src/__tests__/virtualHeights.test.ts @@ -43,8 +43,12 @@ describe('virtual height estimates', () => { it('accounts for the response separator when assistant details are visible', () => { const msg: Msg = { role: 'assistant', text: 'ok', thinking: 'plan' } + // Measured against the real paint (see toolBrief.test.ts's parity suite): + // `∴ Thinking…` header, the reasoning body, the details wrapper's + // marginBottom, the `└─ Response` row, and its marginBottom — 5 rows above + // the bare `⏺ ok`. The header and the wrapper margin used to be missing. expect(estimatedMsgHeight(msg, 80, { compact: false, details: true })).toBe( - estimatedMsgHeight(msg, 80, { compact: false, details: false }) + 3 + estimatedMsgHeight(msg, 80, { compact: false, details: false }) + 5 ) }) diff --git a/ui-tui/src/app/useMainApp.ts b/ui-tui/src/app/useMainApp.ts index bd22bc099..5ff2dbb6b 100644 --- a/ui-tui/src/app/useMainApp.ts +++ b/ui-tui/src/app/useMainApp.ts @@ -340,23 +340,27 @@ export function useMainApp(gw: GatewayClient) { [cols, historyItems, messageId] ) + // Mirrors ToolTrail's `toolsExpanded` exactly (ctrl+o, or an explicit + // `/details tools expanded` pin) — it picks the flat per-call rows over the + // collapsed brief, so the estimate and the paint must derive it the same way. + const toolsDetailsExpanded = ui.detailsMode === 'expanded' || ui.sections?.tools === 'expanded' + const detailsLayoutKey = useMemo(() => { const thinking = sectionMode('thinking', ui.detailsMode, ui.sections, ui.detailsModeCommandOverride) const tools = sectionMode('tools', ui.detailsMode, ui.sections, ui.detailsModeCommandOverride) - // The global expanded toggle (ctrl+o) swaps which tool variant renders, - // so it must bucket the height cache too — section modes alone default - // to 'expanded' and would serve stale collapsed heights across toggles. - return `${thinking}:${tools}:${ui.detailsMode === 'expanded' ? 'x' : '-'}` - }, [ui.detailsMode, ui.detailsModeCommandOverride, ui.sections]) + // The expanded toggle swaps which tool variant renders (flat per-call rows + // vs the collapsed brief), so it must bucket the height cache too. Keyed on + // `toolsDetailsExpanded` itself, not on `detailsMode` a second time: a + // `/details tools expanded` pin flips the layout without moving the global + // mode, and the resolved section mode already reads 'expanded' by default — + // so re-deriving it here would hand the pinned layout the brief's heights. + return `${thinking}:${tools}:${toolsDetailsExpanded ? 'x' : '-'}` + }, [toolsDetailsExpanded, ui.detailsMode, ui.detailsModeCommandOverride, ui.sections]) const [thinkingDetailsMode, toolsDetailsMode] = detailsLayoutKey.split(':') const thinkingDetailsVisible = thinkingDetailsMode !== 'hidden' const toolsDetailsVisible = toolsDetailsMode !== 'hidden' - // Mirrors ToolTrail's `toolsExpanded` exactly (ctrl+o, or an explicit - // `/details tools expanded` pin) — it picks the flat per-call rows over the - // collapsed brief, so the estimate and the paint must derive it the same way. - const toolsDetailsExpanded = ui.detailsMode === 'expanded' || ui.sections?.tools === 'expanded' const detailsVisible = thinkingDetailsVisible || toolsDetailsVisible const userPromptWidth = composerPromptWidth(ui.theme.brand.prompt) const heightCacheKey = `${ui.sid ?? 'draft'}:${cols}:${userPromptWidth}:${ui.compact ? '1' : '0'}:${detailsLayoutKey}` diff --git a/ui-tui/src/components/thinking.tsx b/ui-tui/src/components/thinking.tsx index d5928494a..7c1f03490 100644 --- a/ui-tui/src/components/thinking.tsx +++ b/ui-tui/src/components/thinking.tsx @@ -1199,7 +1199,12 @@ export const ToolTrail = memo(function ToolTrail({ ).map((run, index) => run.kind === 'flat' ? ( - {run.items.map((group, item) => renderGroup(group, index > 0 || item > 0))} + {/* One gap per RUN, matching how the height estimator counts + them. briefRuns never merges two standalone calls, so a + flat run holds exactly one item today — keeping the gap + keyed on the run means the two stay in step even if that + ever changes. */} + {run.items.map(group => renderGroup(group, index > 0))} ) : ( { +// Gutters a `⏺ …` block reserves: 2 columns for the bullet on the call row, +// 5 for the ` ⎿ ` connector (and the matching pad on continuations) under it. +const CALL_GUTTER = 2 +const DETAIL_GUTTER = 5 + +const trailEntries = (msg: Msg, trailWidth: number, toolsExpanded: boolean): TrailEntry[] => { const entries: TrailEntry[] = [] + const detailRows = (detail: string) => + detail ? detail.split('\n').reduce((sum, row) => sum + wrappedLines(row, trailWidth - DETAIL_GUTTER), 0) : 0 + for (const [i, line] of (msg.tools ?? []).entries()) { + // Read both the call and the drafting marker off the SAME line the + // renderer will draw — the verbose sibling when one is being shown. const rendered = (toolsExpanded && msg.toolsVerbose?.[i]) || line const parsed = parseToolTrailResultLine(rendered) @@ -101,13 +111,20 @@ const trailEntries = (msg: Msg, toolsExpanded: boolean): TrailEntry[] => { entries.push({ call: parsed.call, error: parsed.mark === '✗', - // The `⏺` call row, then one row per line of the `⎿` detail. - rows: 1 + (parsed.detail ? parsed.detail.split('\n').length : 0) + // The `⏺` call row, then the `⎿` detail — both of which wrap: an Edit + // detail carries a full path and is capped in lines, never columns. + rows: wrappedLines(parsed.call, trailWidth - CALL_GUTTER) + detailRows(parsed.detail) }) - } else if (line.startsWith('drafting ')) { + } else if (rendered.startsWith('drafting ')) { // Call row + the static "drafting..." detail row. - entries.push({ call: briefCallOfTrailLine(line), error: false, rows: 2 }) + entries.push({ call: briefCallOfTrailLine(rendered), error: false, rows: 2 }) } + + // Anything else is a gateway meta note. ToolTrail routes those to the + // activity panel (hidden by default), so they paint no row here. The one + // transient it does fold onto the previous group, "analyzing tool + // output…", is filtered by isTransientTrailLine before a line ever + // reaches msg.tools, so it cannot appear in a settled trail. } return entries @@ -126,7 +143,7 @@ const trailEntries = (msg: Msg, toolsExpanded: boolean): TrailEntry[] => { * blank line it opens between consecutive blocks. */ const trailRows = (msg: Msg, trailWidth: number, toolsExpanded: boolean) => { - const entries = trailEntries(msg, toolsExpanded) + const entries = trailEntries(msg, trailWidth, toolsExpanded) if (toolsExpanded) { return entries.reduce((sum, entry) => sum + entry.rows, 0) + Math.max(0, entries.length - 1) @@ -210,13 +227,32 @@ export const estimatedMsgHeight = ( const hasVisibleDetails = hasVisibleTools || hasVisibleThinking if (hasVisibleDetails) { + const trailWidth = transcriptTrailWidth(cols, TERMUX_TUI_MODE) + // Tool entries can carry multi-line details (Bash 3-line summaries, // 10-line error caps) — count rendered rows, not entries, or off-screen // estimates under-count and the scrollbar/topSpacer math jumps. - const toolRows = hasVisibleTools ? trailRows(msg, transcriptTrailWidth(cols, TERMUX_TUI_MODE), toolsExpanded) : 0 - - h += toolRows + (hasVisibleThinking ? wrappedLines(msg.thinking ?? '', bodyWidth) : 0) + const toolRows = hasVisibleTools ? trailRows(msg, trailWidth, toolsExpanded) : 0 + + // The reasoning panel is its `∴ Thinking…` header row plus the body, + // which sits under a 3-column `└─ ` rail. The header used to be paid + // for by the one-row text floor above; trail blocks no longer get that + // floor, so charge it here where it is actually true. + // (Known gap, pre-dating the brief: `/details thinking collapsed` closes + // the body while this still counts it. The panel is expanded by default, + // so the common path is exact.) + const thinkingRows = hasVisibleThinking ? 1 + wrappedLines(msg.thinking ?? '', trailWidth - 3) : 0 + + h += toolRows + thinkingRows + + // A prose row wraps its details in a Box with marginBottom={1}. A trail + // block has no such wrapper — MessageLine hands ToolTrail straight + // through — so only prose pays this. + if (msg.kind !== 'trail') { + h++ + } + // "Response" separator row + its own marginBottom. if (msg.role === 'assistant' && /\S/.test(msg.text)) { h += 2 } From 5126c415df086be1452d19d0d97b3b0fb7890400 Mon Sep 17 00:00:00 2001 From: Eric Lee Date: Wed, 12 Aug 2026 12:14:00 -0700 Subject: [PATCH 4/6] test(tui): pin the estimator's remaining width and no-separator branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three cases the parity suite could not distinguish before: reasoning long enough to wrap (pins the `└─ ` rail's 3-column offset), an assistant row with details but no text (details margin without the Response separator), and a system row carrying details (neither separator nor assistant handling). Co-Authored-By: Claude Opus 5 --- ui-tui/src/__tests__/toolBrief.test.ts | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/ui-tui/src/__tests__/toolBrief.test.ts b/ui-tui/src/__tests__/toolBrief.test.ts index 307b593a2..833d7cb21 100644 --- a/ui-tui/src/__tests__/toolBrief.test.ts +++ b/ui-tui/src/__tests__/toolBrief.test.ts @@ -409,6 +409,19 @@ describe('estimatedMsgHeight matches the painted trail', () => { thinking: 'First line.\nSecond line.\nThird line.', tools: [buildToolTrailLine('Read', 'a.py', false, 'Read 8 lines')] } + ], + [ + // The reasoning body sits under a `└─ ` rail, so it wraps 3 narrower + // than the trail — this case is what pins that offset. + 'reasoning long enough to wrap', + { + kind: 'trail', + role: 'system', + text: '', + thinking: + 'The parser reads the header first, then walks each block until it hits a boundary, ' + + 'and only then does it decide whether the trailing bytes belong to the previous frame.' + } ] ] @@ -432,7 +445,12 @@ describe('estimatedMsgHeight matches the painted trail', () => { ['bare assistant prose', { role: 'assistant', text: 'ok' }], ['prose with reasoning', { role: 'assistant', text: 'ok', thinking: 'plan' }], ['prose with a tool brief', { role: 'assistant', text: 'ok', tools: [READ_LINE] }], - ['prose with both', { role: 'assistant', text: 'ok', thinking: 'plan', tools: [READ_LINE] }] + ['prose with both', { role: 'assistant', text: 'ok', thinking: 'plan', tools: [READ_LINE] }], + // No text means no `└─ Response` separator, but the details wrapper's + // margin is still paid — the two must not be conflated. + ['details with no prose', { role: 'assistant', text: '', tools: [READ_LINE] }], + // Details on a non-assistant row: no separator either, same margin. + ['a system row carrying details', { role: 'system', text: 'note', tools: [READ_LINE] }] ] for (const [name, msg] of proseCases) { From fec6ca28387ef2d2cf08d071d40b379c056975e9 Mon Sep 17 00:00:00 2001 From: Eric Lee Date: Wed, 12 Aug 2026 12:23:50 -0700 Subject: [PATCH 5/6] fix(tui): only the read/search band folds; reasoning wraps at the rail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Captured three more tool shapes from Claude Code 2.1.228 and one of them contradicted the classifier. A lone WebSearch does NOT fold — it keeps `⏺ Web Search("…")` with `⎿ Did 1 search in 2s` under it, exactly as a lone Agent keeps `⏺ Agent(…)` with `⎿ Done (2 tool uses · 48.0k tokens · 11s)`, while a lone Bash still folds to "Ran 1 shell command". So the catch-all bucket is standalone too, and "called N tools" is gone with it. That is the rule upstream's message type has been stating all along by name (`collapsed_read_search`): reading, searching, listing and shelling out are what bury a transcript, and they are all that folds. Everything else keeps its row and its result. Two estimator fixes, both measured rather than argued. The reasoning body sits under ` └─ ` — content column 5, not 3 — so it wraps at DETAIL_GUTTER like every other rail-indented row; the old `- 3` missed by a row for any body landing in the two-column band the wrong gutter opened. The parity suite could not see it because both reasoning cases used bodies that never wrapped, so there is now one sized to wrap at 91 and not at 93, built from a single unbreakable token so Ink's word wrap and the estimator's character wrap coincide and the gutter is the only variable. And the structured-diff branch brings its own wrapper with ToolTrail as a direct child, so charging it the prose details margin added a row it never paints. Documented, not fixed, in the same pass: wrappedLines is character math while Ink word-wraps, so a detail carrying one token longer than the content width — a deep file path — can paint one row more than we count. Post-mount measurement corrects it. Co-Authored-By: Claude Opus 5 --- ui-tui/src/__tests__/toolBrief.test.ts | 49 ++++++++++++++++++++++---- ui-tui/src/domain/toolBrief.ts | 41 ++++++++++++--------- ui-tui/src/lib/virtualHeights.ts | 22 ++++++++---- 3 files changed, 82 insertions(+), 30 deletions(-) diff --git a/ui-tui/src/__tests__/toolBrief.test.ts b/ui-tui/src/__tests__/toolBrief.test.ts index 833d7cb21..a2b6e3e22 100644 --- a/ui-tui/src/__tests__/toolBrief.test.ts +++ b/ui-tui/src/__tests__/toolBrief.test.ts @@ -22,7 +22,8 @@ import { briefRuns, briefText, classifyBriefTool, - emptyBriefCounts + emptyBriefCounts, + isCollapsibleBucket } from '../domain/toolBrief.js' import { transcriptTrailWidth } from '../lib/inputMetrics.js' import { buildToolTrailLine, stripAnsi } from '../lib/text.js' @@ -60,9 +61,10 @@ describe('classifyBriefTool', () => { expect(classifyBriefTool('ExitPlanMode(plan)')).toBe('answer') }) - it('falls back to the catch-all bucket', () => { + it('falls back to the catch-all bucket, which also stands alone', () => { expect(classifyBriefTool('WebSearch(rust release)')).toBe('other') expect(classifyBriefTool('Mcp Github List Prs(open)')).toBe('other') + expect(isCollapsibleBucket('other')).toBe(false) }) it('ignores a legacy duration suffix on resumed trail lines', () => { @@ -81,9 +83,9 @@ describe('briefText', () => { ) }) - it('orders clauses search → read → list → other → shell', () => { - expect(briefText(counts({ bash: 1, list: 1, other: 1, read: 1, search: 1 }))).toBe( - 'Searched for 1 pattern, read 1 file, listed 1 directory, called 1 tool, ran 1 shell command' + it('orders clauses search → read → list → shell', () => { + expect(briefText(counts({ bash: 1, list: 1, read: 1, search: 1 }))).toBe( + 'Searched for 1 pattern, read 1 file, listed 1 directory, ran 1 shell command' ) }) @@ -100,6 +102,8 @@ describe('briefText', () => { it('is empty when nothing collapsible ran', () => { expect(briefText(counts({ edit: 3 }))).toBe('') expect(briefClauses(counts({ agent: 1 }))).toEqual([]) + // WebSearch and friends keep their own row, so they never tally either. + expect(briefText(counts({ other: 2 }))).toBe('') }) }) @@ -411,8 +415,6 @@ describe('estimatedMsgHeight matches the painted trail', () => { } ], [ - // The reasoning body sits under a `└─ ` rail, so it wraps 3 narrower - // than the trail — this case is what pins that offset. 'reasoning long enough to wrap', { kind: 'trail', @@ -422,6 +424,15 @@ describe('estimatedMsgHeight matches the painted trail', () => { 'The parser reads the header first, then walks each block until it hits a boundary, ' + 'and only then does it decide whether the trailing bytes belong to the previous frame.' } + ], + [ + // The reasoning body sits under a ` └─ ` rail — content column 5, not + // 3. One unbreakable token so Ink's word wrap and the estimator's + // character wrap coincide and the gutter is the only variable, sized to + // land between the two candidate widths at cols=100: it wraps at 91 and + // does not at 93. A body that merely "is long" cannot tell them apart. + 'reasoning that wraps only at the rail column', + { kind: 'trail', role: 'system', text: '', thinking: 'x'.repeat(92) } ] ] @@ -458,6 +469,30 @@ describe('estimatedMsgHeight matches the painted trail', () => { expect(estimatedMsgHeight(msg, 80, { compact: false, details: true })).toBe(proseRows(msg, 80)) }) } + + // The structured-diff branch brings its own wrapper with ToolTrail as a + // direct child, so it must NOT be charged the details wrapper's margin. + // (Its absolute estimate is off for older reasons — it still counts + // msg.text for a markdown fallback the structured path never renders — so + // pin the delta rather than the number.) + it('does not charge the details margin to a structured diff', () => { + const diff: Msg = { + diffData: { filePath: 'a.py', hunks: [], kind: 'update' }, + kind: 'diff', + role: 'assistant', + text: 'patch', + tools: [READ_LINE] + } + + const withDetails = estimatedMsgHeight(diff, 80, { compact: false, details: true }) + const withoutDetails = estimatedMsgHeight(diff, 80, { compact: false, details: false }) + + // Turning details on costs the one brief row, plus the 2 the estimator + // charges for a `Response` separator this branch never paints (older + // divergence, left alone). The point is that it is 3 and not 4 — the + // details wrapper's marginBottom belongs to prose rows only. + expect(withDetails - withoutDetails).toBe(3) + }) }) // Narrow widths are where wrapping bites; 100 is the everyday case. diff --git a/ui-tui/src/domain/toolBrief.ts b/ui-tui/src/domain/toolBrief.ts index 003c06750..e13ab4d92 100644 --- a/ui-tui/src/domain/toolBrief.ts +++ b/ui-tui/src/domain/toolBrief.ts @@ -2,7 +2,8 @@ import { parseToolTrailResultLine, splitToolDuration, toolTrailLabel } from '../ /** * The original Claude Code transcript does not list every tool call on its own - * row. A run of calls collapses into a single dim summary line — the "brief": + * row. A run of calls collapses into a single muted summary line — the + * "brief": * * Read 3 files, listed 1 directory, ran 1 shell command * @@ -17,7 +18,7 @@ import { parseToolTrailResultLine, splitToolDuration, toolTrailLabel } from '../ * ("Reading 1 file…"); a settled one uses the past tense. * - upstream only folds tools that describe themselves as a search/read * shaped call (plus shell commands and MCP bridges). Everything else — - * edits, delegations, questions — keeps its own row, because its detail + * edits, delegations, answers — keeps its own row, because its detail * rows carry information a tally would destroy. See STANDALONE below. */ export type BriefBucket = 'agent' | 'answer' | 'bash' | 'edit' | 'list' | 'other' | 'read' | 'search' @@ -25,18 +26,27 @@ export type BriefBucket = 'agent' | 'answer' | 'bash' | 'edit' | 'list' | 'other export type BriefCounts = Record /** - * Buckets that never fold into the brief — they render as their own - * `⏺ Tool(args)` block even in the collapsed view: + * Only the read/search band and shell commands fold. Everything else renders + * as its own `⏺ Tool(args)` block even in the collapsed view — which is what + * upstream does, and the reason its collapsed message type is literally named + * `collapsed_read_search`. Captured from Claude Code 2.1.228: a lone `Bash` + * folds to "Ran 1 shell command", while a lone `WebSearch` keeps + * `⏺ Web Search("…")` + `⎿ Did 1 search in 2s`, and a lone `Agent` keeps + * `⏺ Agent(…)` + `⎿ Done (2 tool uses · 48.0k tokens · 11s)`. * - * edit — the patch itself is the point (upstream renders `⏺ Update(f)` - * with the diff under it, and never tallies it away). - * agent — the Delegate Task row anchors the inline subagent tree. + * edit — the patch itself is the point (upstream renders `⏺ Write(f)` + * with the content under it, and never tallies it away). + * agent — the row anchors the inline subagent tree. * answer — AskUserQuestion, clarify, advisor, vision_analyze, ExitPlanMode: * the detail rows ARE the result (the user's own choices, the - * advisor's opinion, the plan). "Called 1 tool" would delete them - * from the transcript with no way to get them back. + * advisor's opinion, the plan). + * other — WebSearch, WebFetch, Skill, MCP bridges: their `⎿` rows carry a + * result no tally can stand in for. + * + * The point of the brief is the high-volume, low-information calls — reading, + * searching, listing, shelling out. Those are what bury a transcript. */ -const STANDALONE: ReadonlySet = new Set(['agent', 'answer', 'edit']) +const STANDALONE: ReadonlySet = new Set(['agent', 'answer', 'edit', 'other']) export const isCollapsibleBucket = (bucket: BriefBucket): boolean => !STANDALONE.has(bucket) @@ -128,10 +138,8 @@ export const classifyBriefTool = (call: string): BriefBucket => { return READ_COMMAND.test(args) ? 'read' : 'bash' } - // Everything else — WebSearch/WebFetch, Skill, MCP bridges, task tools — is - // "called N tools", upstream's catch-all clause. (Upstream names the MCP - // *server* in a clause of its own; a trail line doesn't carry one, so MCP - // calls read as generic tool calls rather than an invented label.) + // Everything else — WebSearch/WebFetch, Skill, MCP bridges, task tools. + // STANDALONE, so each keeps its own row and its own result. return 'other' } @@ -177,13 +185,12 @@ export interface BriefClause { const plural = (n: number, one: string, many: string) => (n === 1 ? one : many) -// Clause order is upstream's: the read/search band first, then the catch-all, -// with shell commands last. STANDALONE buckets never reach here. +// Clause order is upstream's: the read/search band first, shell commands last. +// STANDALONE buckets never reach here. const ORDER: { bucket: BriefBucket; live: string; noun: [string, string]; past: string }[] = [ { bucket: 'search', live: 'searching for', noun: ['pattern', 'patterns'], past: 'searched for' }, { bucket: 'read', live: 'reading', noun: ['file', 'files'], past: 'read' }, { bucket: 'list', live: 'listing', noun: ['directory', 'directories'], past: 'listed' }, - { bucket: 'other', live: 'calling', noun: ['tool', 'tools'], past: 'called' }, { bucket: 'bash', live: 'running', noun: ['shell command', 'shell commands'], past: 'ran' } ] diff --git a/ui-tui/src/lib/virtualHeights.ts b/ui-tui/src/lib/virtualHeights.ts index 5546c19f7..666b9d73e 100644 --- a/ui-tui/src/lib/virtualHeights.ts +++ b/ui-tui/src/lib/virtualHeights.ts @@ -92,6 +92,12 @@ interface TrailEntry { // Gutters a `⏺ …` block reserves: 2 columns for the bullet on the call row, // 5 for the ` ⎿ ` connector (and the matching pad on continuations) under it. +// The reasoning body's ` └─ ` rail lands on the same column 5. +// +// Known residual: wrappedLines is character math while Ink word-wraps, so a +// detail carrying one token longer than the content width (a deep file path) +// can paint one row more than this counts. Post-mount measurement corrects it; +// it is a ±1 on long paths, not a structural divergence. const CALL_GUTTER = 2 const DETAIL_GUTTER = 5 @@ -235,20 +241,24 @@ export const estimatedMsgHeight = ( const toolRows = hasVisibleTools ? trailRows(msg, trailWidth, toolsExpanded) : 0 // The reasoning panel is its `∴ Thinking…` header row plus the body, - // which sits under a 3-column `└─ ` rail. The header used to be paid + // which sits under a ` └─ ` rail — content starts at column 5, so + // DETAIL_GUTTER is the right offset here too. The header used to be paid // for by the one-row text floor above; trail blocks no longer get that // floor, so charge it here where it is actually true. // (Known gap, pre-dating the brief: `/details thinking collapsed` closes // the body while this still counts it. The panel is expanded by default, // so the common path is exact.) - const thinkingRows = hasVisibleThinking ? 1 + wrappedLines(msg.thinking ?? '', trailWidth - 3) : 0 + const thinkingRows = hasVisibleThinking ? 1 + wrappedLines(msg.thinking ?? '', trailWidth - DETAIL_GUTTER) : 0 h += toolRows + thinkingRows - // A prose row wraps its details in a Box with marginBottom={1}. A trail - // block has no such wrapper — MessageLine hands ToolTrail straight - // through — so only prose pays this. - if (msg.kind !== 'trail') { + // A prose row wraps its details in a Box with marginBottom={1}. Trail + // blocks hand ToolTrail straight through, and the structured-diff branch + // brings its own wrapper with ToolTrail as a direct child — neither pays + // this. (The diff branch's estimate is off for older reasons too: it + // still counts msg.text for a markdown fallback the structured path + // never renders. Out of scope here; just don't add to it.) + if (msg.kind !== 'trail' && msg.kind !== 'diff') { h++ } From 34cab783be08824a563f441259bec2cbfc7d76df Mon Sep 17 00:00:00 2001 From: Eric Lee Date: Wed, 12 Aug 2026 12:31:55 -0700 Subject: [PATCH 6/6] test(tui): cover a standalone catch-all tool, and name the wrap case honestly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A lone WebSearch keeping its `⏺`/`⎿` block rested on one unit assertion about the bucket; it has a render case now, since that behaviour came from a capture and a fresh reading of upstream rather than from anything structural. The brief-wrap case stopped wrapping at cols=100 when `other` left the vocabulary — four clauses top out around 84 columns. Counts raised so it still wraps at 60, and the name says which arm actually exercises it. Co-Authored-By: Claude Opus 5 --- ui-tui/src/__tests__/toolBrief.test.ts | 30 ++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/ui-tui/src/__tests__/toolBrief.test.ts b/ui-tui/src/__tests__/toolBrief.test.ts index a2b6e3e22..523efd289 100644 --- a/ui-tui/src/__tests__/toolBrief.test.ts +++ b/ui-tui/src/__tests__/toolBrief.test.ts @@ -270,6 +270,22 @@ describe('ToolTrail brief render', () => { expect(row).toMatch(/^ {2}Read 2 files/) }) + // WebSearch and friends are NOT part of the read/search band upstream folds: + // captured from Claude Code 2.1.228, a lone Web Search keeps its row and its + // `⎿ Did 1 search in 2s`. A tally would delete the only interesting part. + it('keeps a catch-all tool standalone with its result', () => { + const withSearch = [trail[0]!, buildToolTrailLine('WebSearch', 'rust 1.90', false, 'Did 1 search in 2s')] + + const out = stripAnsi( + renderToString(React.createElement(ToolTrail, { detailsMode: 'collapsed', t: DEFAULT_THEME, trail: withSearch })) + ) + + expect(out).toContain('WebSearch(rust 1.90)') + expect(out).toContain('Did 1 search in 2s') + expect(out).toContain('Read 1 file') + expect(out).not.toContain('called 1 tool') + }) + it('breaks a failed call out of the brief so its error stays readable', () => { const withError = [ trail[0]!, @@ -394,13 +410,15 @@ describe('estimatedMsgHeight matches the painted trail', () => { ]) ], [ - 'a brief long enough to wrap', + // Four clauses with multi-digit tallies is the widest a brief can get + // now that `other` stands alone: ~84 columns, so it wraps at cols=60 and + // fits at cols=100. The cols=60 arm is the one exercising brief wrap. + 'a brief long enough to wrap at 60 columns', trailMsg([ - buildToolTrailLine('Grep', 'TODO', false, 'Found 2 lines'), - buildToolTrailLine('Read', 'a.py', false, 'Read 8 lines'), - buildToolTrailLine('Bash', 'ls src', false, 'a.py'), - buildToolTrailLine('WebSearch', 'rust', false, 'Did 1 search'), - buildToolTrailLine('Bash', 'echo hi', false, 'hi') + ...Array.from({ length: 12 }, (_, i) => buildToolTrailLine('Grep', `p${i}`, false, 'Found 2 lines')), + ...Array.from({ length: 13 }, (_, i) => buildToolTrailLine('Read', `f${i}.py`, false, 'Read 8 lines')), + ...Array.from({ length: 24 }, (_, i) => buildToolTrailLine('Bash', `ls d${i}`, false, 'a.py')), + ...Array.from({ length: 18 }, (_, i) => buildToolTrailLine('Bash', `echo ${i}`, false, 'hi')) ]) ], ['a reasoning trail', { kind: 'trail', role: 'system', text: '', thinking: 'I should check the parser first.' }],