diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx index 13e7c7e8e7a..b7a0c7d78de 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx @@ -27,6 +27,7 @@ import { ZoomableImage } from '@/components/chat/zoomable-image' import { Button } from '@/components/ui/button' import { Codicon } from '@/components/ui/codicon' import { CopyButton } from '@/components/ui/copy-button' +import { DiffCount } from '@/components/ui/diff-count' import { DisclosureCaret } from '@/components/ui/disclosure-caret' import { FadeText } from '@/components/ui/fade-text' import { FileTypeIcon } from '@/components/ui/file-type-icon' @@ -63,6 +64,7 @@ import { type ToolStatus, type ToolTitleAction } from './fallback-model' +import { isToolCallPart, type RunSummary, summarizeToolRun } from './run-summary' // `true` when a ToolEntry is rendered inside an embedding wrapper that owns // the per-row chrome (timer / preview). The flat ToolGroupSlot sets this @@ -797,29 +799,89 @@ function useToolWindow(enabled: boolean) { return { contentRef, faded, onScroll, scrollRef } } +// The one grey line that stands in for a run of tool calls once it has +// settled — "Edited wiring.tsx, explored 3 files +6 −4". While the run is +// live the same line narrates it in the present tense and the rows stay +// visible below, so nothing the user is waiting on hides behind a chevron. +function ToolRunHeader({ + live, + onToggle, + open, + summary +}: { + live: boolean + onToggle?: () => void + open: boolean + summary: RunSummary +}) { + return ( +
+ + + + {live ? {summary.text} : summary.text} + + + + +
+ ) +} + +interface ToolRunState { + key: string + live: boolean + summary: RunSummary +} + +// assistant-ui compares selector results with `Object.is` and calls the +// selector on every store update, so returning a fresh object here would +// re-render the group on every text delta in the turn. The run only changes +// when a call arrives or one finishes; cache on exactly that. +function useToolRun(startIndex: number, endIndex: number): ToolRunState { + const cache = useRef(null) + + return useAuiState(state => { + const tools = state.message.parts.slice(Math.max(0, startIndex), endIndex + 1).filter(isToolCallPart) + const signature = tools.map(tool => `${tool.toolCallId}:${tool.result === undefined ? 0 : 1}`).join('|') + + if (cache.current?.signature !== signature) { + cache.current = { + signature, + value: { + key: tools[0]?.toolCallId ?? '', + live: tools.some(tool => tool.result === undefined), + summary: summarizeToolRun(tools) + } + } + } + + return cache.current.value + }) +} + /** - * Flat, Cursor-style tool list. assistant-ui hands us a *range* of - * consecutive tool-call parts, but how that range is sliced is unstable: a - * live stream interleaves narration/reasoning between calls (many tiny - * ranges), while the settled message reconstructs every tool_call back-to-back - * (one big range). Rendering a "Tool actions · N steps" group off that range - * therefore reshuffled the whole turn the instant it settled. + * A run of consecutive tool calls, headed by the line that summarizes it. * - * So we still never *label* the group: each tool is a standalone row on the - * tight `--tool-row-gap` rhythm. Once a run reaches `TOOL_GROUP_SCROLL_THRESHOLD` - * rows it collapses into a fixed-height, auto-scrolling window so a long run - * doesn't shove the reply off screen; shorter runs are byte-identical to before. - * The DOM shape is the same either way — only classes flip — so a run that - * crosses the threshold mid-stream never remounts a row. `ToolEmbedContext` is - * false so every row owns its own chrome (timer / preview / copy / approval). + * The run is identified by its FIRST tool call, never by its position: a live + * stream and the same turn rehydrated from history agree on which calls belong + * together, but not on the indices they land at, because rehydration folds a + * turn into one bubble that the live view spreads over several. Keying off the + * index is what made an earlier attempt at this reshuffle the moment a turn + * settled. `lib/tool-run-continuity.test.ts` locks that agreement down. + * + * A settled run collapses to its header; a live one keeps its rows on screen + * (and past `TOOL_GROUP_SCROLL_THRESHOLD` inside a fixed-height auto-scrolling + * window, so a long run can't shove the reply off screen). `ToolEmbedContext` + * is false so every row still owns its own chrome (timer / copy / approval). */ export const ToolGroupSlot: FC> = ({ children, endIndex, startIndex }) => { - const messageId = useAuiState(s => s.message.id) const messageRunning = useAuiState(selectMessageRunning) + const { key, live, summary } = useToolRun(startIndex, endIndex) const hasUnboundable = useAuiState(s => s.message.parts @@ -827,26 +889,49 @@ export const ToolGroupSlot: FC part.type === 'tool-call' && isUnboundableTool(part.toolName)) ) - const enterRef = useEnterAnimation(messageRunning, `tool-group:${messageId}:${startIndex}`) + const disclosureId = `tool-run:${key}` + const persistedOpen = useStore($toolDisclosureOpen(disclosureId)) + // A lone call is already its own one-line summary, so it never earns a + // header. Neither does a run holding an `UNBOUNDABLE_TOOLS` surface — a + // `clarify` asking the user a question must not end up behind a chevron. + const grouped = endIndex > startIndex && !hasUnboundable + const open = !grouped || live || (persistedOpen ?? false) - const bounded = shouldBoundToolGroup(Children.count(children), hasUnboundable) + const enterRef = useEnterAnimation(messageRunning, `tool-run:${key}`) + + const bounded = open && shouldBoundToolGroup(Children.count(children), hasUnboundable) const { contentRef, faded, onScroll, scrollRef } = useToolWindow(bounded) return ( -
-
-
- {children} +
+ {grouped && ( + setToolDisclosureOpen(disclosureId, !open)} + open={open} + summary={summary} + /> + )} + {open && ( +
+
+ {children} +
-
+ )}
) diff --git a/apps/desktop/src/components/assistant-ui/tool/approval-group.test.tsx b/apps/desktop/src/components/assistant-ui/tool/tool-group.test.tsx similarity index 71% rename from apps/desktop/src/components/assistant-ui/tool/approval-group.test.tsx rename to apps/desktop/src/components/assistant-ui/tool/tool-group.test.tsx index f6201153c98..25c46fbe3fd 100644 --- a/apps/desktop/src/components/assistant-ui/tool/approval-group.test.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/tool-group.test.tsx @@ -9,11 +9,12 @@ import { $toolDisclosureStates } from '@/store/tool-view' import { Thread } from '../thread' -// Regression coverage for the "approval must never be buried" bug. Tools now -// render as a flat list (no collapsible "N steps" group), so a pending tool's -// inline ApprovalBar is always in the visual flow — never inside a `hidden` -// body. These assert the bar shows only when an approval is live and is never -// trapped under a `hidden` ancestor. +// A run of tool calls collapses to a one-line summary once it has settled, but +// a run with anything still pending always renders its rows. That rule is what +// keeps the "approval must never be buried" bug fixed: an inline ApprovalBar +// only ever exists on a pending tool, and a pending tool's run is never behind +// a chevron. These cover both halves — the collapse itself, and the approval +// staying in the visual flow. const createdAt = new Date('2026-06-03T00:00:00.000Z') @@ -183,6 +184,42 @@ function failedOnlyMessage(): ThreadMessage { } as ThreadMessage } +// Two settled tools in a row — an edit plus a read — so the run earns a +// summary line and collapses. +function settledRunMessage(): ThreadMessage { + return { + id: 'assistant-settled-run', + role: 'assistant', + content: [ + { + type: 'tool-call', + toolCallId: 'patch-1', + toolName: 'patch', + args: { path: '/repo/src/wiring.tsx' }, + argsText: JSON.stringify({ path: '/repo/src/wiring.tsx' }), + result: { path: '/repo/src/wiring.tsx', inline_diff: '--- a\n+++ b\n+added line\n-removed line' } + }, + { + type: 'tool-call', + toolCallId: 'read-2', + toolName: 'read_file', + args: { path: '/repo/src/status.tsx' }, + argsText: JSON.stringify({ path: '/repo/src/status.tsx' }), + result: { content: 'export const Status = () => null' } + } + ], + status: { type: 'complete', reason: 'stop' }, + createdAt, + metadata: { + unstable_state: null, + unstable_annotations: [], + unstable_data: [], + steps: [], + custom: {} + } + } as ThreadMessage +} + function GroupHarness({ message }: { message: ThreadMessage }) { const runtime = useExternalStoreRuntime({ messages: [message], @@ -211,6 +248,55 @@ afterEach(() => { clearDismissedToolRows() }) +describe('settled tool run', () => { + it('collapses to a summary line naming the work and its diff', async () => { + const { container } = render() + + expect(await screen.findByText('Edited wiring.tsx, explored status.tsx')).toBeTruthy() + expect(container.querySelectorAll('[data-tool-row]')).toHaveLength(0) + expect(screen.getByText('1', { selector: '.text-\\(--ui-green\\) *' })).toBeTruthy() + }) + + it('expands to the underlying rows when the summary is clicked', async () => { + const { container } = render() + + fireEvent.click(await screen.findByText('Edited wiring.tsx, explored status.tsx')) + + await waitFor(() => { + expect(container.querySelectorAll('[data-tool-row]').length).toBeGreaterThan(0) + }) + }) + + it('leaves a lone tool call as its own row, with no summary above it', async () => { + const { container } = render() + + await waitFor(() => { + expect(container.querySelectorAll('[data-tool-row]').length).toBe(1) + }) + expect(container.querySelector('[data-tool-summary]')).toBeNull() + }) +}) + +describe('live tool run', () => { + it('keeps its rows on screen instead of hiding them behind the summary', async () => { + const { container } = render() + + await waitFor(() => { + expect(container.querySelectorAll('[data-tool-row]').length).toBeGreaterThan(0) + }) + }) + + it('cannot be collapsed while a tool is still running', async () => { + const { container } = render() + + await waitFor(() => { + expect(container.querySelector('[data-tool-summary]')).not.toBeNull() + }) + + expect(container.querySelector('[data-tool-summary] button[aria-expanded]')).toBeNull() + }) +}) + describe('flat tool list approval surfacing', () => { it('renders no inline approval bar when there is no live approval', async () => { const { container } = render() diff --git a/apps/desktop/src/styles.css b/apps/desktop/src/styles.css index 9fb935bbe08..c2a86f4960a 100644 --- a/apps/desktop/src/styles.css +++ b/apps/desktop/src/styles.css @@ -1421,6 +1421,7 @@ text-* variant utilities. */ .btn-arc { it impossible to keep one row lit (an open diff) while its siblings faded. With the fade per-row, each row hovers/focuses independently. */ [data-slot='aui_assistant-message-content'] > [data-slot='aui_thinking-disclosure'], +[data-slot='aui_assistant-message-content'] [data-tool-summary], [data-slot='aui_assistant-message-content'] [data-slot='tool-block'][data-tool-row] { opacity: 0.67; transition: opacity 120ms ease-out; @@ -1430,6 +1431,7 @@ text-* variant utilities. */ .btn-arc { focus a mouse click leaves on the disclosure toggle, which kept a row lit after you clicked to collapse it; `:has(:focus-visible)` excludes that. */ [data-slot='aui_assistant-message-content'] > [data-slot='aui_thinking-disclosure']:is(:hover, :has(:focus-visible)), +[data-slot='aui_assistant-message-content'] [data-tool-summary]:is(:hover, :has(:focus-visible)), [data-slot='aui_assistant-message-content'] [data-slot='tool-block'][data-tool-row]:is(:hover, :has(:focus-visible)) { opacity: 1; }