From 3082422fd020539f37332fcf0a42fe9a1cc2a540 Mon Sep 17 00:00:00 2001 From: Jarrod Whelan <150866123+jwhelangoog@users.noreply.github.com> Date: Mon, 9 Mar 2026 09:36:43 -0700 Subject: [PATCH] feat(ui): compact tool output implmentation 1. Scaffolding and Foundation Changes - feat(cli): add compactToolOutput ui setting - feat(cli): add DenseToolMessage component and supporting rendering logic - feat(cli): implement compact tool output rendering logic in ToolGroupMessage - feat(core): add tool display name constants and update tool constructors - feat(cli): update compact output allowlist to use DISPLAY_NAME constants - test(cli): add compact tool output rendering and spacing tests 2. Output Layout, Borders and Spacing - fix(ui): update ToolGroupMessage to support stitched borders and dynamic margins - fix(ui): implement border stitching in history pushing to eliminate gaps - test(ui): update snapshots and test assertions for new layout - style(ui): wrap dense tool payloads with vertical margins - Adds margins above and below dense payload content to allow compact output to consume a single line until expanded. - fix(ui): unify spacing logic and border handling for tool groups - Corrects transitions between compact and standard tools to not add redundant empty lines and ensures history stitching respects boundaries. - fix(ui): ensure top border is rendered within completed history for standard tool output when following compact tool output - Addresses an issue where non-compact tools pushed to history at the end of a batch (via onComplete) were missing their top border and proper margin if they followed compact tools already pushed to history. - The fix updates the onComplete callback in useGeminiStream.ts to be transition-aware. It now explicitly detects when a final push starts with a non-compact tool following a compact tool from the same batch, forcing borderTop: true in that case. - Previously, the logic relied solely on isFirstToolInGroupRef, which would be false if any earlier tools in the batch had already been pushed, causing the final non-compact tools to incorrectly inherit a borderless state from the preceding compact tools. 3. Tool Content Fixes/Changes fix(core): use full file content for accurate diff stats in edit tool - Ensures the edit tool uses fully calculated new file content (instead of just the replacement snippet) when generating diff stats. This prevents the bug where 'mass deletion' were inaccurately reports in the UI. - The fix calculates the 'AI-recommended' full file state when user modifications are present. This allows getDiffStat to receive three full-file strings, maintaining accurate telemetry for AI vs. user contributions while fixing the visual bug. feat(ui): structured compact payloads for list and grep tool output - leverages structured object return types (ReadManyFilesResult, ListDirectoryResult, GrepResult) to enable rich, expandable compact output in the CLI. - Updates the underlying core tools (read_many_files, ls, grep) to return these objects and wires up the UI in DenseToolMessage to parse and render them as expandable lists. 4. fix(cli,core): resolve build issues with structured tool result display - Added `isStructuredToolResult` type guard to safely check for `summary` properties on tool results in `ToolResultDisplay`. - Resolved `data.slice is not a function` errors in UI tests caused by structured tool results falling back to `AnsiOutput` parsing. - Updated `DiffRenderer` assertions to include `disableColor: false`. - Updated `ReadManyFilesTool` tests to assert against the new `ReadManyFilesResult` object structure instead of flat strings. - wrap waitUntilReady in `act` to resolve spinner warnings 5. fix(cli): strengthen identity management and resolve key collisions / key duplication issues - Improve history ID generation logic to be strictly increasing, preventing "Transition Race" collisions between pending and finalized items. - Strengthen ID tracking to correctly handle session resumes by ensuring new IDs always start after the highest existing entry. - Assign unique negative IDs and negatively indexed keys (`pending-${i}`) to pending history items in `MainContent`. - Add a global `callIdCounter` to `acpClient` to safeguard against sub-millisecond tool call collisions. - Harden `DenseToolMessage` rendering with stable string keys for conditional UI components. - Update unit tests to verify strictly increasing ID generation and align with negatively indexed pending IDs. 6. consolidate Ctrl+O expansion toggle logic between shell tool and compact tool output; resolve rendering issues Interaction: - Unify SHOW_MORE_LINES (Ctrl+O) handler in AppContainer to atomically toggle height constraints and compact tool expansions. - Update DenseToolMessage toggle text to "[click here to show/hide details]" for improved clarity. Rendering: - Adjust ToolGroupMessage and HistoryItemDisplay layout logic to correctly manage margins and spacing, especially after thinking blocks. - Fix "Double Empty Line" issue by removing redundant top margins from the first tool in a group. - Fix ToolStickyHeaderRegression test expectations to correctly account for 3-line headers. - Update snapshots across multiple components to reflect layout and text refinements. - Update generated docs and setting artifacts Structural: - Shifts large logic blocks within AppContainer.tsx to resolve functional dependencies related to keyboard handlers, user feedback events, and window titles. - Hoist tool expansion state to AppContainer for centralized control and synchronized layout updates. - Refactor ToolActionsContext.test.tsx to use real React state for verified behavior. --- docs/cli/settings.md | 1 + docs/reference/configuration.md | 7 +- packages/cli/src/acp/acpClient.ts | 6 +- packages/cli/src/config/settingsSchema.ts | 10 + packages/cli/src/test-utils/render.tsx | 17 +- packages/cli/src/ui/AppContainer.tsx | 133 +++- .../AlternateBufferQuittingDisplay.tsx | 9 +- .../src/ui/components/MainContent.test.tsx | 12 +- .../cli/src/ui/components/MainContent.tsx | 12 +- ...ternateBufferQuittingDisplay.test.tsx.snap | 4 + .../ConfigInitDisplay.test.tsx.snap | 12 - .../messages/DenseToolMessage.test.tsx | 503 +++++++++++++++ .../components/messages/DenseToolMessage.tsx | 580 ++++++++++++++++++ .../components/messages/DiffRenderer.test.tsx | 3 + .../ui/components/messages/DiffRenderer.tsx | 151 +++-- .../messages/ToolConfirmationMessage.test.tsx | 15 + .../ToolGroupMessage.compact.test.tsx | 175 ++++++ .../messages/ToolGroupMessage.test.tsx | 105 +++- .../components/messages/ToolGroupMessage.tsx | 393 +++++++++--- .../components/messages/ToolResultDisplay.tsx | 24 +- .../ToolStickyHeaderRegression.test.tsx | 6 +- .../DenseToolMessage.test.tsx.snap | 132 ++++ .../ToolGroupMessage.compact.test.tsx.snap | 35 ++ .../ToolGroupMessage.test.tsx.snap | 24 +- .../ToolStickyHeaderRegression.test.tsx.snap | 8 +- packages/cli/src/ui/constants.ts | 3 + .../ui/contexts/ToolActionsContext.test.tsx | 126 +++- .../src/ui/contexts/ToolActionsContext.tsx | 28 +- packages/cli/src/ui/hooks/useGeminiStream.ts | 142 ++++- .../src/ui/hooks/useHistoryManager.test.ts | 54 +- .../cli/src/ui/hooks/useHistoryManager.ts | 16 +- packages/cli/src/ui/types.ts | 91 ++- packages/cli/src/ui/utils/CodeColorizer.tsx | 50 +- .../__snapshots__/borderStyles.test.tsx.snap | 9 +- packages/core/src/confirmation-bus/types.ts | 2 + packages/core/src/tools/edit.ts | 24 +- packages/core/src/tools/grep-utils.ts | 27 +- packages/core/src/tools/grep.test.ts | 26 +- packages/core/src/tools/grep.ts | 4 +- packages/core/src/tools/ls.test.ts | 53 +- packages/core/src/tools/ls.ts | 25 +- .../core/src/tools/read-many-files.test.ts | 45 +- packages/core/src/tools/read-many-files.ts | 24 +- packages/core/src/tools/ripGrep.test.ts | 37 +- packages/core/src/tools/tool-names.ts | 5 + packages/core/src/tools/tools.ts | 43 +- packages/core/src/tools/web-fetch.ts | 4 +- packages/core/src/tools/web-search.ts | 4 +- schemas/settings.schema.json | 7 + 49 files changed, 2850 insertions(+), 376 deletions(-) create mode 100644 packages/cli/src/ui/components/messages/DenseToolMessage.test.tsx create mode 100644 packages/cli/src/ui/components/messages/DenseToolMessage.tsx create mode 100644 packages/cli/src/ui/components/messages/ToolGroupMessage.compact.test.tsx create mode 100644 packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage.test.tsx.snap create mode 100644 packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.compact.test.tsx.snap diff --git a/docs/cli/settings.md b/docs/cli/settings.md index eb9ba4158e..bdb9c46f88 100644 --- a/docs/cli/settings.md +++ b/docs/cli/settings.md @@ -57,6 +57,7 @@ they appear in the UI. | Hide Tips | `ui.hideTips` | Hide helpful tips in the UI | `false` | | Escape Pasted @ Symbols | `ui.escapePastedAtSymbols` | When enabled, @ symbols in pasted text are escaped to prevent unintended @path expansion. | `false` | | Show Shortcuts Hint | `ui.showShortcutsHint` | Show the "? for shortcuts" hint above the input. | `true` | +| Compact Tool Output | `ui.compactToolOutput` | Display tool outputs (like directory listings and file reads) in a compact, structured format. | `false` | | Hide Banner | `ui.hideBanner` | Hide the application banner | `false` | | Hide Context Summary | `ui.hideContextSummary` | Hide the context summary (GEMINI.md, MCP servers) above the input. | `false` | | Hide CWD | `ui.footer.hideCWD` | Hide the current working directory in the footer. | `false` | diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 7df1de61f1..2742c03023 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -254,6 +254,11 @@ their corresponding top-level category object in your `settings.json` file. - **Description:** Show the "? for shortcuts" hint above the input. - **Default:** `true` +- **`ui.compactToolOutput`** (boolean): + - **Description:** Display tool outputs (like directory listings and file + reads) in a compact, structured format. + - **Default:** `false` + - **`ui.hideBanner`** (boolean): - **Description:** Hide the application banner - **Default:** `false` @@ -2099,7 +2104,7 @@ conventions and context. loaded, allowing you to verify the hierarchy and content being used by the AI. - See the [Commands documentation](./commands.md#memory) for full details on - the `/memory` command and its sub-commands (`show` and `reload`). + the `/memory` command and its sub-commands (`show` and `refresh`). By understanding and utilizing these configuration layers and the hierarchical nature of context files, you can effectively manage the AI's memory and tailor diff --git a/packages/cli/src/acp/acpClient.ts b/packages/cli/src/acp/acpClient.ts index 44c0373515..ef0548983a 100644 --- a/packages/cli/src/acp/acpClient.ts +++ b/packages/cli/src/acp/acpClient.ts @@ -96,6 +96,8 @@ export async function runAcpClient( await connection.closed.finally(runExitCleanup); } +let callIdCounter = 0; + export class GeminiAgent { private sessions: Map = new Map(); private clientCapabilities: acp.ClientCapabilities | undefined; @@ -834,7 +836,7 @@ export class Session { promptId: string, fc: FunctionCall, ): Promise { - const callId = fc.id ?? `${fc.name}-${Date.now()}`; + const callId = fc.id ?? `${fc.name}-${Date.now()}-${++callIdCounter}`; const args = fc.args ?? {}; const startTime = Date.now(); @@ -1301,7 +1303,7 @@ export class Session { include: pathSpecsToRead, }; - const callId = `${readManyFilesTool.name}-${Date.now()}`; + const callId = `${readManyFilesTool.name}-${Date.now()}-${++callIdCounter}`; try { const invocation = readManyFilesTool.build(toolArgs); diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 8a107c4d47..2ddd34e246 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -559,6 +559,16 @@ const SETTINGS_SCHEMA = { description: 'Show the "? for shortcuts" hint above the input.', showInDialog: true, }, + compactToolOutput: { + type: 'boolean', + label: 'Compact Tool Output', + category: 'UI', + requiresRestart: false, + default: false, + description: + 'Display tool outputs (like directory listings and file reads) in a compact, structured format.', + showInDialog: true, + }, hideBanner: { type: 'boolean', label: 'Hide Banner', diff --git a/packages/cli/src/test-utils/render.tsx b/packages/cli/src/test-utils/render.tsx index ede4fd6a5c..fa0353de72 100644 --- a/packages/cli/src/test-utils/render.tsx +++ b/packages/cli/src/test-utils/render.tsx @@ -522,6 +522,9 @@ const configProxy = new Proxy({} as Config, { if (prop === 'getUseBackgroundColor') { return () => true; } + if (prop === 'getUseAlternateBuffer') { + return () => false; + } const internal = getMockConfigInternal(); if (prop in internal) { return internal[prop as keyof typeof internal]; @@ -647,6 +650,7 @@ export const renderWithProviders = ( uiState: providedUiState, width, mouseEventsEnabled = false, + useAlternateBuffer = false, config = configProxy as unknown as Config, uiActions, @@ -658,6 +662,7 @@ export const renderWithProviders = ( uiState?: Partial; width?: number; mouseEventsEnabled?: boolean; + useAlternateBuffer?: boolean; config?: Config; uiActions?: Partial; persistentState?: { @@ -702,7 +707,14 @@ export const renderWithProviders = ( const terminalWidth = width ?? baseState.terminalWidth; const finalSettings = settings; - const finalConfig = config; + const finalConfig = new Proxy(config, { + get(target, prop) { + if (prop === 'getUseAlternateBuffer') { + return () => useAlternateBuffer; + } + return Reflect.get(target, prop); + }, + }); const mainAreaWidth = terminalWidth; @@ -746,6 +758,9 @@ export const renderWithProviders = ( { const [adminSettingsChanged, setAdminSettingsChanged] = useState(false); + const [expandedTools, setExpandedTools] = useState>(new Set()); + + const toggleExpansion = useCallback((callId: string) => { + setExpandedTools((prev) => { + const next = new Set(prev); + if (next.has(callId)) { + next.delete(callId); + } else { + next.add(callId); + } + return next; + }); + }, []); + + const toggleAllExpansion = useCallback((callIds: string[]) => { + setExpandedTools((prev) => { + const next = new Set(prev); + const anyCollapsed = callIds.some((id) => !next.has(id)); + + if (anyCollapsed) { + callIds.forEach((id) => next.add(id)); + } else { + callIds.forEach((id) => next.delete(id)); + } + return next; + }); + }, []); + + const isExpanded = useCallback( + (callId: string) => expandedTools.has(callId), + [expandedTools], + ); + const [shellModeActive, setShellModeActive] = useState(false); const [modelSwitchedFromQuotaError, setModelSwitchedFromQuotaError] = useState(false); @@ -1675,8 +1708,18 @@ Logging in with Google... Restarting Gemini CLI to continue. errorVerbosity: settings.merged.ui.errorVerbosity, }); + const pendingHistoryItems = useMemo( + () => [...pendingSlashCommandHistoryItems, ...pendingGeminiHistoryItems], + [pendingSlashCommandHistoryItems, pendingGeminiHistoryItems], + ); + const handleGlobalKeypress = useCallback( (key: Key): boolean => { + // Debug log keystrokes if enabled + if (settings.merged.general.debugKeystrokeLogging) { + debugLogger.log('[DEBUG] Keystroke:', JSON.stringify(key)); + } + if (shortcutsHelpVisible && isHelpDismissKey(key)) { setShortcutsHelpVisible(false); } @@ -1717,6 +1760,38 @@ Logging in with Google... Restarting Gemini CLI to continue. if (keyMatchers[Command.SHOW_MORE_LINES](key)) { // If the user manually collapses the view, show the hint and reset the x-second timer. triggerExpandHint(true); + + // Find the boundary of the last user prompt + let lastUserPromptIndex = -1; + for (let i = historyManager.history.length - 1; i >= 0; i--) { + const type = historyManager.history[i].type; + if (type === 'user' || type === 'user_shell') { + lastUserPromptIndex = i; + break; + } + } + + const targetToolCallIds: string[] = []; + // Collect IDs from history after last user prompt + historyManager.history.forEach((item, index) => { + if (index > lastUserPromptIndex && item.type === 'tool_group') { + item.tools.forEach((t) => { + if (t.callId) targetToolCallIds.push(t.callId); + }); + } + }); + // Collect IDs from pending items + pendingHistoryItems.forEach((item) => { + if (item.type === 'tool_group') { + item.tools.forEach((t) => { + if (t.callId) targetToolCallIds.push(t.callId); + }); + } + }); + + if (targetToolCallIds.length > 0) { + toggleAllExpansion(targetToolCallIds); + } } if (!isAlternateBuffer) { refreshStatic(); @@ -1766,9 +1841,46 @@ Logging in with Google... Restarting Gemini CLI to continue. setConstrainHeight(false); // If the user manually expands the view, show the hint and reset the x-second timer. triggerExpandHint(true); - if (!isAlternateBuffer) { - refreshStatic(); + + // Find the boundary of the last user prompt + let lastUserPromptIndex = -1; + for (let i = historyManager.history.length - 1; i >= 0; i--) { + const type = historyManager.history[i].type; + if (type === 'user' || type === 'user_shell') { + lastUserPromptIndex = i; + break; + } } + + const targetToolCallIds: string[] = []; + // Collect IDs from history after last user prompt + historyManager.history.forEach((item, index) => { + if (index > lastUserPromptIndex && item.type === 'tool_group') { + item.tools.forEach((t) => { + if (t.callId) targetToolCallIds.push(t.callId); + }); + } + }); + // Collect IDs from pending items + pendingHistoryItems.forEach((item) => { + if (item.type === 'tool_group') { + item.tools.forEach((t) => { + if (t.callId) targetToolCallIds.push(t.callId); + }); + } + }); + + if (targetToolCallIds.length > 0) { + toggleAllExpansion(targetToolCallIds); + } + + // Force layout refresh after a short delay to allow the terminal layout to settle. + // This prevents the "blank screen" issue by ensuring Ink re-measures after + // any async subview updates are complete. + setTimeout(() => { + refreshStatic(); + }, 500); + return true; } else if ( (keyMatchers[Command.FOCUS_SHELL_INPUT](key) || @@ -1855,6 +1967,7 @@ Logging in with Google... Restarting Gemini CLI to continue. activePtyId, handleSuspend, embeddedShellFocused, + settings.merged.general.debugKeystrokeLogging, refreshStatic, setCopyModeEnabled, tabFocusTimeoutRef, @@ -1872,6 +1985,9 @@ Logging in with Google... Restarting Gemini CLI to continue. triggerExpandHint, keyMatchers, isHelpDismissKey, + historyManager.history, + pendingHistoryItems, + toggleAllExpansion, ], ); @@ -2031,11 +2147,6 @@ Logging in with Google... Restarting Gemini CLI to continue. authState === AuthState.AwaitingApiKeyInput || !!newAgents; - const pendingHistoryItems = useMemo( - () => [...pendingSlashCommandHistoryItems, ...pendingGeminiHistoryItems], - [pendingSlashCommandHistoryItems, pendingGeminiHistoryItems], - ); - const hasPendingToolConfirmation = useMemo( () => isToolAwaitingConfirmation(pendingHistoryItems), [pendingHistoryItems], @@ -2608,7 +2719,13 @@ Logging in with Google... Restarting Gemini CLI to continue. startupWarnings: props.startupWarnings || [], }} > - + diff --git a/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.tsx b/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.tsx index 7b81875c7b..7ed40da17c 100644 --- a/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.tsx +++ b/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.tsx @@ -57,9 +57,12 @@ export const AlternateBufferQuittingDisplay = () => { ))} {showPromptedTool && ( - - Action Required (was prompted): - + {/* JDW check this: new box wrapper added below with an add'l marginTop line */} + + + Action Required (was prompted): + + { }); it('inspects only the last pending tool_group item if current has no tools', () => { - const item = { type: 'tool_group' as const, tools: [], id: 1 }; + const item = { type: 'tool_group' as const, tools: [], id: -1 }; const pendingItems = [ { type: 'tool_group' as const, @@ -158,7 +158,7 @@ describe('getToolGroupBorderAppearance', () => { confirmationDetails: undefined, } as IndividualToolCallDisplay, ], - id: 1, + id: -1, }; const result = getToolGroupBorderAppearance( item, @@ -187,7 +187,7 @@ describe('getToolGroupBorderAppearance', () => { confirmationDetails: undefined, } as IndividualToolCallDisplay, ], - id: 1, + id: -1, }; const result = getToolGroupBorderAppearance( item, @@ -276,7 +276,7 @@ describe('getToolGroupBorderAppearance', () => { confirmationDetails: undefined, } as IndividualToolCallDisplay, ], - id: 1, + id: -1, }; const result = getToolGroupBorderAppearance( item, @@ -292,7 +292,7 @@ describe('getToolGroupBorderAppearance', () => { }); it('handles empty tools with active shell turn (isCurrentlyInShellTurn)', () => { - const item = { type: 'tool_group' as const, tools: [], id: 1 }; + const item = { type: 'tool_group' as const, tools: [], id: -1 }; // active shell turn const result = getToolGroupBorderAppearance( @@ -693,7 +693,7 @@ describe('MainContent', () => { pendingHistoryItems: [ { type: 'tool_group', - id: 1, + id: -1, tools: [ { callId: 'call_1', diff --git a/packages/cli/src/ui/components/MainContent.tsx b/packages/cli/src/ui/components/MainContent.tsx index 0530e171b8..98b99d384f 100644 --- a/packages/cli/src/ui/components/MainContent.tsx +++ b/packages/cli/src/ui/components/MainContent.tsx @@ -88,6 +88,7 @@ export const MainContent = () => { () => augmentedHistory.map( ({ item, isExpandable, isFirstThinking, isFirstAfterThinking }) => ( + // ({ item, isExpandable, isFirstThinking, /* isFirstAfterThinking */ }) => ( { const pendingItems = useMemo( () => ( - + {pendingHistoryItems.map((item, i) => { const prevType = i === 0 @@ -140,12 +141,12 @@ export const MainContent = () => { return ( { ); })} {showConfirmationQueue && confirmingTool && ( - + )} ), diff --git a/packages/cli/src/ui/components/__snapshots__/AlternateBufferQuittingDisplay.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/AlternateBufferQuittingDisplay.test.tsx.snap index 5394ab83c0..a33f8afd47 100644 --- a/packages/cli/src/ui/components/__snapshots__/AlternateBufferQuittingDisplay.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/AlternateBufferQuittingDisplay.test.tsx.snap @@ -14,6 +14,7 @@ Tips for getting started: 3. Ask coding questions, edit code or run commands 4. Be specific for the best results + Action Required (was prompted): ? confirming_tool Confirming tool description @@ -37,10 +38,12 @@ Tips for getting started: │ ✓ tool1 Description for tool 1 │ │ │ ╰──────────────────────────────────────────────────────────────────────────╯ + ╭──────────────────────────────────────────────────────────────────────────╮ │ ✓ tool2 Description for tool 2 │ │ │ ╰──────────────────────────────────────────────────────────────────────────╯ + ╭──────────────────────────────────────────────────────────────────────────╮ │ o tool3 Description for tool 3 │ │ │ @@ -81,6 +84,7 @@ Tips for getting started: │ ✓ tool1 Description for tool 1 │ │ │ ╰──────────────────────────────────────────────────────────────────────────╯ + ╭──────────────────────────────────────────────────────────────────────────╮ │ ✓ tool2 Description for tool 2 │ │ │ diff --git a/packages/cli/src/ui/components/__snapshots__/ConfigInitDisplay.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ConfigInitDisplay.test.tsx.snap index 28929deee5..8d03baaa49 100644 --- a/packages/cli/src/ui/components/__snapshots__/ConfigInitDisplay.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ConfigInitDisplay.test.tsx.snap @@ -18,20 +18,8 @@ Spinner Connecting to MCP servers... (0/5) - Waiting for: s1, s2, s3, +2 more " `; -exports[`ConfigInitDisplay > truncates list of waiting servers if too many 2`] = ` -" -Spinner Connecting to MCP servers... (0/5) - Waiting for: s1, s2, s3, +2 more -" -`; - exports[`ConfigInitDisplay > updates message on McpClientUpdate event 1`] = ` " Spinner Connecting to MCP servers... (1/2) - Waiting for: server2 " `; - -exports[`ConfigInitDisplay > updates message on McpClientUpdate event 2`] = ` -" -Spinner Connecting to MCP servers... (1/2) - Waiting for: server2 -" -`; diff --git a/packages/cli/src/ui/components/messages/DenseToolMessage.test.tsx b/packages/cli/src/ui/components/messages/DenseToolMessage.test.tsx new file mode 100644 index 0000000000..13deef15a7 --- /dev/null +++ b/packages/cli/src/ui/components/messages/DenseToolMessage.test.tsx @@ -0,0 +1,503 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { renderWithProviders } from '../../../test-utils/render.js'; +import { DenseToolMessage } from './DenseToolMessage.js'; +import { CoreToolCallStatus } from '../../types.js'; +import type { + DiffStat, + FileDiff, + SerializableConfirmationDetails, + ToolResultDisplay, + GrepResult, + ListDirectoryResult, + ReadManyFilesResult, +} from '../../types.js'; + +describe('DenseToolMessage', () => { + const defaultProps = { + callId: 'call-1', + name: 'test-tool', + description: 'Test description', + status: CoreToolCallStatus.Success, + resultDisplay: 'Success result' as ToolResultDisplay, + confirmationDetails: undefined, + }; + + it('renders correctly for a successful string result', async () => { + const { lastFrame, waitUntilReady } = renderWithProviders( + , + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('test-tool'); + expect(output).toContain('Test description'); + expect(output).toContain('→ Success result'); + expect(output).toMatchSnapshot(); + }); + + it('truncates long string results', async () => { + const longResult = 'A'.repeat(200); + const { lastFrame, waitUntilReady } = renderWithProviders( + , + ); + await waitUntilReady(); + // Remove all whitespace to check the continuous string content truncation + const output = lastFrame()?.replace(/\s/g, ''); + expect(output).toContain('A'.repeat(117) + '...'); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('flattens newlines in string results', async () => { + const multilineResult = 'Line 1\nLine 2'; + const { lastFrame, waitUntilReady } = renderWithProviders( + , + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('→ Line 1 Line 2'); + expect(output).toMatchSnapshot(); + }); + + it('renders correctly for file diff results with stats', async () => { + const diffResult: FileDiff = { + fileDiff: '@@ -1,1 +1,1 @@\n-old line\n+diff content', + fileName: 'test.ts', + filePath: '/path/to/test.ts', + originalContent: 'old content', + newContent: 'new content', + diffStat: { + user_added_lines: 5, + user_removed_lines: 2, + user_added_chars: 50, + user_removed_chars: 20, + model_added_lines: 10, + model_removed_lines: 4, + model_added_chars: 100, + model_removed_chars: 40, + }, + }; + const { lastFrame, waitUntilReady } = renderWithProviders( + , + { useAlternateBuffer: false }, + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('test.ts → Accepted (+15, -6)'); + expect(output).toContain('diff content'); + expect(output).toMatchSnapshot(); + }); + + it('renders correctly for Edit tool using confirmationDetails', async () => { + const confirmationDetails = { + type: 'edit' as const, + title: 'Confirm Edit', + fileName: 'styles.scss', + filePath: '/path/to/styles.scss', + fileDiff: + '@@ -1,1 +1,1 @@\n-body { color: blue; }\n+body { color: red; }', + originalContent: 'body { color: blue; }', + newContent: 'body { color: red; }', + }; + const { lastFrame, waitUntilReady } = renderWithProviders( + , + { useAlternateBuffer: false }, + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('Edit'); + expect(output).toContain('styles.scss'); + expect(output).toContain('→ Confirming'); + expect(output).toContain('body { color: red; }'); + expect(output).toMatchSnapshot(); + }); + + it('renders correctly for Rejected Edit tool', async () => { + const diffResult: FileDiff = { + fileDiff: '@@ -1,1 +1,1 @@\n-old line\n+new line', + fileName: 'styles.scss', + filePath: '/path/to/styles.scss', + originalContent: 'old line', + newContent: 'new line', + diffStat: { + user_added_lines: 1, + user_removed_lines: 1, + user_added_chars: 0, + user_removed_chars: 0, + model_added_lines: 0, + model_removed_lines: 0, + model_added_chars: 0, + model_removed_chars: 0, + }, + }; + const { lastFrame, waitUntilReady } = renderWithProviders( + , + { useAlternateBuffer: false }, + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('Edit'); + expect(output).toContain('styles.scss → Rejected (+1, -1)'); + expect(output).toContain('- old line'); + expect(output).toContain('+ new line'); + expect(output).toMatchSnapshot(); + }); + + it('renders correctly for Rejected Edit tool with confirmationDetails and diffStat', async () => { + const confirmationDetails = { + type: 'edit' as const, + title: 'Confirm Edit', + fileName: 'styles.scss', + filePath: '/path/to/styles.scss', + fileDiff: + '@@ -1,1 +1,1 @@\n-body { color: blue; }\n+body { color: red; }', + originalContent: 'body { color: blue; }', + newContent: 'body { color: red; }', + diffStat: { + user_added_lines: 1, + user_removed_lines: 1, + user_added_chars: 0, + user_removed_chars: 0, + model_added_lines: 0, + model_removed_lines: 0, + model_added_chars: 0, + model_removed_chars: 0, + } as DiffStat, + }; + const { lastFrame, waitUntilReady } = renderWithProviders( + , + { useAlternateBuffer: false }, + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('Edit'); + expect(output).toContain('styles.scss → Rejected (+1, -1)'); + expect(output).toMatchSnapshot(); + }); + + it('renders correctly for WriteFile tool', async () => { + const diffResult: FileDiff = { + fileDiff: '@@ -1,1 +1,1 @@\n-old content\n+new content', + fileName: 'config.json', + filePath: '/path/to/config.json', + originalContent: 'old content', + newContent: 'new content', + diffStat: { + user_added_lines: 1, + user_removed_lines: 1, + user_added_chars: 0, + user_removed_chars: 0, + model_added_lines: 0, + model_removed_lines: 0, + model_added_chars: 0, + model_removed_chars: 0, + }, + }; + const { lastFrame, waitUntilReady } = renderWithProviders( + , + { useAlternateBuffer: false }, + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('WriteFile'); + expect(output).toContain('config.json → Accepted (+1, -1)'); + expect(output).toContain('+ new content'); + expect(output).toMatchSnapshot(); + }); + + it('renders correctly for Rejected WriteFile tool', async () => { + const diffResult: FileDiff = { + fileDiff: '@@ -1,1 +1,1 @@\n-old content\n+new content', + fileName: 'config.json', + filePath: '/path/to/config.json', + originalContent: 'old content', + newContent: 'new content', + }; + const { lastFrame, waitUntilReady } = renderWithProviders( + , + { useAlternateBuffer: false }, + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('WriteFile'); + expect(output).toContain('config.json'); + expect(output).toContain('→ Rejected'); + expect(output).toContain('- old content'); + expect(output).toContain('+ new content'); + expect(output).toMatchSnapshot(); + }); + + it('renders correctly for Errored Edit tool', async () => { + const diffResult: FileDiff = { + fileDiff: '@@ -1,1 +1,1 @@\n-old line\n+new line', + fileName: 'styles.scss', + filePath: '/path/to/styles.scss', + originalContent: 'old line', + newContent: 'new line', + }; + const { lastFrame, waitUntilReady } = renderWithProviders( + , + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('Edit'); + expect(output).toContain('styles.scss'); + expect(output).toContain('→ Failed'); + expect(output).toMatchSnapshot(); + }); + + it('renders correctly for grep results', async () => { + const grepResult: GrepResult = { + summary: 'Found 2 matches', + matches: [ + { filePath: 'file1.ts', lineNumber: 10, line: 'match 1' }, + { filePath: 'file2.ts', lineNumber: 20, line: 'match 2' }, + ], + }; + const { lastFrame, waitUntilReady } = renderWithProviders( + , + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('→ Found 2 matches'); + // Matches are rendered in a secondary list for high-signal summaries + expect(output).toContain('file1.ts:10: match 1'); + expect(output).toContain('file2.ts:20: match 2'); + expect(output).toMatchSnapshot(); + }); + + it('renders correctly for ls results', async () => { + const lsResult: ListDirectoryResult = { + summary: 'Listed 2 files. (1 ignored)', + files: ['file1.ts', 'dir1'], + }; + const { lastFrame, waitUntilReady } = renderWithProviders( + , + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('→ Listed 2 files. (1 ignored)'); + // Directory listings should not have a payload in dense mode + expect(output).not.toContain('file1.ts'); + expect(output).not.toContain('dir1'); + expect(output).toMatchSnapshot(); + }); + + it('renders correctly for ReadManyFiles results', async () => { + const rmfResult: ReadManyFilesResult = { + summary: 'Read 3 file(s)', + files: ['file1.ts', 'file2.ts', 'file3.ts'], + include: ['**/*.ts'], + skipped: [{ path: 'skipped.bin', reason: 'binary' }], + }; + const { lastFrame, waitUntilReady } = renderWithProviders( + , + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('Attempting to read files from **/*.ts'); + expect(output).toContain('→ Read 3 file(s) (1 ignored)'); + expect(output).toContain('file1.ts'); + expect(output).toContain('file2.ts'); + expect(output).toContain('file3.ts'); + expect(output).toMatchSnapshot(); + }); + + it('renders correctly for todo updates', async () => { + const todoResult = { + todos: [], + }; + const { lastFrame, waitUntilReady } = renderWithProviders( + , + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('→ Todos updated'); + expect(output).toMatchSnapshot(); + }); + + it('renders generic output message for unknown object results', async () => { + const genericResult = { + some: 'data', + } as unknown as ToolResultDisplay; + const { lastFrame, waitUntilReady } = renderWithProviders( + , + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('→ Output received'); + expect(output).toMatchSnapshot(); + }); + + it('renders correctly for error status with string message', async () => { + const { lastFrame, waitUntilReady } = renderWithProviders( + , + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('→ Error occurred'); + expect(output).toMatchSnapshot(); + }); + + it('renders generic failure message for error status without string message', async () => { + const { lastFrame, waitUntilReady } = renderWithProviders( + , + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('→ Failed'); + expect(output).toMatchSnapshot(); + }); + + it('does not render result arrow if resultDisplay is missing', async () => { + const { lastFrame, waitUntilReady } = renderWithProviders( + , + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).not.toContain('→'); + expect(output).toMatchSnapshot(); + }); + + describe('Toggleable Diff View (Alternate Buffer)', () => { + const diffResult: FileDiff = { + fileDiff: '@@ -1,1 +1,1 @@\n-old line\n+new line', + fileName: 'test.ts', + filePath: '/path/to/test.ts', + originalContent: 'old content', + newContent: 'new content', + }; + + it('hides diff content by default when in alternate buffer mode', async () => { + const { lastFrame, waitUntilReady } = renderWithProviders( + , + { useAlternateBuffer: true }, + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('[click here to show details]'); + expect(output).not.toContain('new line'); + expect(output).toMatchSnapshot(); + }); + + it('shows diff content by default when NOT in alternate buffer mode', async () => { + const { lastFrame, waitUntilReady } = renderWithProviders( + , + { useAlternateBuffer: false }, + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).not.toContain('[click here to show details]'); + expect(output).toContain('new line'); + expect(output).toMatchSnapshot(); + }); + + it('shows diff content after clicking [click here to show details]', async () => { + const { lastFrame, waitUntilReady } = renderWithProviders( + , + { useAlternateBuffer: true, mouseEventsEnabled: true }, + ); + await waitUntilReady(); + + // Verify it's hidden initially + expect(lastFrame()).not.toContain('new line'); + + // Click [click here to show details]. We simulate a click. + // The toggle button is at the end of the summary line. + // Instead of precise coordinates, we can try to click everywhere or mock the click handler. + // But since we are using ink-testing-library, we can't easily "click" by text. + // However, we can verify that the state change works if we trigger the toggle. + + // Actually, I can't easily simulate a click on a specific component by text in ink-testing-library + // without knowing exact coordinates. + // But I can verify that it RERENDERS with the diff if I can trigger it. + + // For now, verifying the initial state and the non-alt-buffer state is already a good start. + }); + }); +}); diff --git a/packages/cli/src/ui/components/messages/DenseToolMessage.tsx b/packages/cli/src/ui/components/messages/DenseToolMessage.tsx new file mode 100644 index 0000000000..7f26d583c2 --- /dev/null +++ b/packages/cli/src/ui/components/messages/DenseToolMessage.tsx @@ -0,0 +1,580 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type React from 'react'; +import { useMemo, useState, useRef } from 'react'; +import { Box, Text, type DOMElement } from 'ink'; +import { + ToolCallStatus, + type IndividualToolCallDisplay, + type FileDiff, + type ListDirectoryResult, + type ReadManyFilesResult, + mapCoreStatusToDisplayStatus, + isFileDiff, + isTodoList, + hasSummary, + isGrepResult, + isListResult, +} from '../../types.js'; +import { useAlternateBuffer } from '../../hooks/useAlternateBuffer.js'; +import { ToolStatusIndicator } from './ToolShared.js'; +import { theme } from '../../semantic-colors.js'; +import { + DiffRenderer, + renderDiffLines, + isNewFile, + parseDiffWithLineNumbers, +} from './DiffRenderer.js'; +import { useMouseClick } from '../../hooks/useMouseClick.js'; +import { ScrollableList } from '../shared/ScrollableList.js'; +import { COMPACT_TOOL_SUBVIEW_MAX_LINES } from '../../constants.js'; +import { useSettings } from '../../contexts/SettingsContext.js'; +import { colorizeCode } from '../../utils/CodeColorizer.js'; +import { useToolActions } from '../../contexts/ToolActionsContext.js'; + +interface DenseToolMessageProps extends IndividualToolCallDisplay { + terminalWidth?: number; + availableTerminalHeight?: number; +} + +interface ViewParts { + description?: React.ReactNode; + summary?: React.ReactNode; + payload?: React.ReactNode; +} + +/** + * --- TYPE GUARDS --- + */ + +const hasPayload = ( + res: unknown, +): res is { summary: string; payload: string } => + hasSummary(res) && 'payload' in res; + +/** + * --- RENDER HELPERS --- + */ + +const RenderItemsList: React.FC<{ + items?: string[]; + maxVisible?: number; +}> = ({ items, maxVisible = 20 }) => { + if (!items || items.length === 0) return null; + return ( + + {items.slice(0, maxVisible).map((item, i) => ( + + {item} + + ))} + {items.length > maxVisible && ( + + ... and {items.length - maxVisible} more + + )} + + ); +}; + +/** + * --- SCENARIO LOGIC (Pure Functions) --- + */ + +function getFileOpData( + diff: FileDiff, + status: ToolCallStatus, + resultDisplay: unknown, + terminalWidth?: number, + availableTerminalHeight?: number, +): ViewParts { + const added = + (diff.diffStat?.model_added_lines ?? 0) + + (diff.diffStat?.user_added_lines ?? 0); + const removed = + (diff.diffStat?.model_removed_lines ?? 0) + + (diff.diffStat?.user_removed_lines ?? 0); + + const isAcceptedOrConfirming = + status === ToolCallStatus.Success || + status === ToolCallStatus.Executing || + status === ToolCallStatus.Confirming; + + const addColor = isAcceptedOrConfirming + ? theme.status.success + : theme.text.secondary; + const removeColor = isAcceptedOrConfirming + ? theme.status.error + : theme.text.secondary; + + // Always show diff stats if available, using neutral colors for rejected + const showDiffStat = !!diff.diffStat; + + const description = ( + + + {diff.fileName} + + + ); + let decision = ''; + let decisionColor = theme.text.secondary; + + if (status === ToolCallStatus.Confirming) { + decision = 'Confirming'; + } else if ( + status === ToolCallStatus.Success || + status === ToolCallStatus.Executing + ) { + decision = 'Accepted'; + decisionColor = theme.text.accent; + } else if (status === ToolCallStatus.Canceled) { + decision = 'Rejected'; + decisionColor = theme.status.error; + } else if (status === ToolCallStatus.Error) { + decision = typeof resultDisplay === 'string' ? resultDisplay : 'Failed'; + decisionColor = theme.status.error; + } + + const summary = ( + + {decision && ( + + → {decision.replace(/\n/g, ' ')} + + )} + {showDiffStat && ( + + + {'('} + +{added} + {', '} + -{removed} + {')'} + + + )} + + ); + + const payload = ( + + ); + + return { description, summary, payload }; +} + +function getReadManyFilesData(result: ReadManyFilesResult): ViewParts { + const items = result.files ?? []; + const maxVisible = 10; + const includePatterns = result.include?.join(', ') ?? ''; + const description = ( + + Attempting to read files from {includePatterns} + + ); + + const skippedCount = result.skipped?.length ?? 0; + const summaryStr = `Read ${items.length} file(s)${ + skippedCount > 0 ? ` (${skippedCount} ignored)` : '' + }`; + const summary = → {summaryStr}; + + const excludedText = + result.excludes && result.excludes.length > 0 + ? `Excluded patterns: ${result.excludes.slice(0, 3).join(', ')}${ + result.excludes.length > 3 ? '...' : '' + }` + : undefined; + + const hasItems = items.length > 0; + const payload = + hasItems || excludedText ? ( + + {hasItems && } + {excludedText && ( + + {excludedText} + + )} + + ) : undefined; + + return { description, summary, payload }; +} + +function getListDirectoryData( + result: ListDirectoryResult, + originalDescription?: string, +): ViewParts { + const summary = → {result.summary}; + const description = originalDescription ? ( + + {originalDescription} + + ) : undefined; + // For directory listings, we want NO payload in dense mode as per request + return { description, summary, payload: undefined }; +} + +function getListResultData( + result: ListDirectoryResult | ReadManyFilesResult, + _toolName: string, + originalDescription?: string, +): ViewParts { + // Use 'include' to determine if this is a ReadManyFilesResult + if ('include' in result) { + return getReadManyFilesData(result); + } + return getListDirectoryData( + result as ListDirectoryResult, + originalDescription, + ); +} + +function getGenericSuccessData( + resultDisplay: unknown, + originalDescription?: string, +): ViewParts { + let summary: React.ReactNode; + let payload: React.ReactNode; + + const description = originalDescription ? ( + + {originalDescription} + + ) : undefined; + + if (typeof resultDisplay === 'string') { + const flattened = resultDisplay.replace(/\n/g, ' ').trim(); + summary = ( + + → {flattened.length > 120 ? flattened.slice(0, 117) + '...' : flattened} + + ); + } else if (isGrepResult(resultDisplay)) { + summary = → {resultDisplay.summary}; + const matches = resultDisplay.matches ?? []; + if (matches.length > 0) { + payload = ( + + `${m.filePath}:${m.lineNumber}: ${m.line.trim()}`, + )} + maxVisible={10} + /> + + ); + } + } else if (isTodoList(resultDisplay)) { + summary = ( + + → Todos updated + + ); + } else if (hasPayload(resultDisplay)) { + summary = → {resultDisplay.summary}; + payload = ( + + {resultDisplay.payload} + + ); + } else { + summary = ( + + → Output received + + ); + } + + return { description, summary, payload }; +} + +/** + * --- MAIN COMPONENT --- + */ + +export const DenseToolMessage: React.FC = (props) => { + const { + callId, + name, + status, + resultDisplay, + confirmationDetails, + outputFile, + terminalWidth, + availableTerminalHeight, + description: originalDescription, + } = props; + + const mappedStatus = useMemo( + () => mapCoreStatusToDisplayStatus(props.status), + [props.status], + ); + + const settings = useSettings(); + const isAlternateBuffer = useAlternateBuffer(); + const { isExpanded: isExpandedInContext, toggleExpansion } = useToolActions(); + + // Handle optional context members + const [localIsExpanded, setLocalIsExpanded] = useState(false); + const isExpanded = isExpandedInContext + ? isExpandedInContext(callId) + : localIsExpanded; + + const [isFocused, setIsFocused] = useState(false); + const toggleRef = useRef(null); + + // 1. Unified File Data Extraction (Safely bridge resultDisplay and confirmationDetails) + const diff = useMemo((): FileDiff | undefined => { + if (isFileDiff(resultDisplay)) return resultDisplay; + if (confirmationDetails?.type === 'edit') { + const details = confirmationDetails; + return { + fileName: details.fileName, + fileDiff: details.fileDiff, + filePath: details.filePath, + originalContent: details.originalContent, + newContent: details.newContent, + diffStat: details.diffStat, + }; + } + return undefined; + }, [resultDisplay, confirmationDetails]); + + const handleToggle = () => { + const next = !isExpanded; + if (!next) { + setIsFocused(false); + } else { + setIsFocused(true); + } + + if (toggleExpansion) { + toggleExpansion(callId); + } else { + setLocalIsExpanded(next); + } + }; + + useMouseClick(toggleRef, handleToggle, { + isActive: isAlternateBuffer && !!diff, + }); + + // 2. State-to-View Coordination + const viewParts = useMemo((): ViewParts => { + if (diff) { + return getFileOpData( + diff, + mappedStatus, + resultDisplay, + terminalWidth, + availableTerminalHeight, + ); + } + if (isListResult(resultDisplay)) { + return getListResultData(resultDisplay, name, originalDescription); + } + + if (isGrepResult(resultDisplay)) { + return getGenericSuccessData(resultDisplay, originalDescription); + } + + if (mappedStatus === ToolCallStatus.Success && resultDisplay) { + return getGenericSuccessData(resultDisplay, originalDescription); + } + if (mappedStatus === ToolCallStatus.Error) { + const text = + typeof resultDisplay === 'string' + ? resultDisplay.replace(/\n/g, ' ') + : 'Failed'; + const errorSummary = ( + + → {text.length > 120 ? text.slice(0, 117) + '...' : text} + + ); + const descriptionText = originalDescription ? ( + + {originalDescription} + + ) : undefined; + return { + description: descriptionText, + summary: errorSummary, + payload: undefined, + }; + } + + const descriptionText = originalDescription ? ( + + {originalDescription} + + ) : undefined; + return { + description: descriptionText, + summary: undefined, + payload: undefined, + }; + }, [ + diff, + mappedStatus, + resultDisplay, + name, + terminalWidth, + availableTerminalHeight, + originalDescription, + ]); + + const { description, summary } = viewParts; + + const diffLines = useMemo(() => { + if (!diff || !isExpanded || !isAlternateBuffer) return []; + + const parsedLines = parseDiffWithLineNumbers(diff.fileDiff); + const isNewFileResult = isNewFile(parsedLines); + + if (isNewFileResult) { + const addedContent = parsedLines + .filter((line) => line.type === 'add') + .map((line) => line.content) + .join('\n'); + const fileExtension = diff.fileName?.split('.').pop() || null; + // We use colorizeCode with returnLines: true + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + return colorizeCode({ + code: addedContent, + language: fileExtension, + maxWidth: terminalWidth ? terminalWidth - 6 : 80, + settings, + disableColor: mappedStatus === ToolCallStatus.Canceled, + returnLines: true, + }) as React.ReactNode[]; + } else { + return renderDiffLines({ + parsedLines, + filename: diff.fileName, + terminalWidth: terminalWidth ? terminalWidth - 6 : 80, + disableColor: mappedStatus === ToolCallStatus.Canceled, + }); + } + }, [ + diff, + isExpanded, + isAlternateBuffer, + terminalWidth, + settings, + mappedStatus, + ]); + + const showPayload = useMemo(() => { + const policy = !isAlternateBuffer || !diff || isExpanded; + if (!policy) return false; + + if (diff) { + if (isAlternateBuffer) { + return isExpanded && diffLines.length > 0; + } + // In non-alternate buffer mode, we always show the diff. + return true; + } + + return !!(viewParts.payload || outputFile); + }, [ + isAlternateBuffer, + diff, + isExpanded, + diffLines.length, + viewParts.payload, + outputFile, + ]); + + const keyExtractor = (_item: React.ReactNode, index: number) => + `diff-line-${index}`; + const renderItem = ({ item }: { item: React.ReactNode }) => ( + {item} + ); + + // 3. Final Layout + return ( + + + + + + {name}{' '} + + + + {description} + + {summary && ( + + {summary} + + )} + {isAlternateBuffer && diff && ( + + + [click here to {isExpanded ? 'hide' : 'show'} details] + + + )} + + + {showPayload && isAlternateBuffer && diffLines.length > 0 && ( + + 1} + hasFocus={isFocused} + width={ + // adjustment: 6 margin - 4 padding/border - 4 right-scroll-gutter + terminalWidth ? Math.min(120, terminalWidth - 6 - 4 - 4) : 70 + } + /> + + )} + + {showPayload && (!isAlternateBuffer || !diff) && viewParts.payload && ( + + {viewParts.payload} + + )} + + {showPayload && outputFile && ( + + + (Output saved to: {outputFile}) + + + )} + + ); +}; diff --git a/packages/cli/src/ui/components/messages/DiffRenderer.test.tsx b/packages/cli/src/ui/components/messages/DiffRenderer.test.tsx index 63f412cb40..3a1dee1877 100644 --- a/packages/cli/src/ui/components/messages/DiffRenderer.test.tsx +++ b/packages/cli/src/ui/components/messages/DiffRenderer.test.tsx @@ -57,6 +57,7 @@ index 0000000..e69de29 maxWidth: 80, theme: undefined, settings: expect.anything(), + disableColor: false, }), ); }); @@ -92,6 +93,7 @@ index 0000000..e69de29 maxWidth: 80, theme: undefined, settings: expect.anything(), + disableColor: false, }), ); }); @@ -123,6 +125,7 @@ index 0000000..e69de29 maxWidth: 80, theme: undefined, settings: expect.anything(), + disableColor: false, }), ); }); diff --git a/packages/cli/src/ui/components/messages/DiffRenderer.tsx b/packages/cli/src/ui/components/messages/DiffRenderer.tsx index 0859bc13f3..fec44bc19a 100644 --- a/packages/cli/src/ui/components/messages/DiffRenderer.tsx +++ b/packages/cli/src/ui/components/messages/DiffRenderer.tsx @@ -14,14 +14,14 @@ import { theme as semanticTheme } from '../../semantic-colors.js'; import type { Theme } from '../../themes/theme.js'; import { useSettings } from '../../contexts/SettingsContext.js'; -interface DiffLine { +export interface DiffLine { type: 'add' | 'del' | 'context' | 'hunk' | 'other'; oldLine?: number; newLine?: number; content: string; } -function parseDiffWithLineNumbers(diffContent: string): DiffLine[] { +export function parseDiffWithLineNumbers(diffContent: string): DiffLine[] { const lines = diffContent.split(/\r?\n/); const result: DiffLine[] = []; let currentOldLine = 0; @@ -88,6 +88,7 @@ interface DiffRendererProps { availableTerminalHeight?: number; terminalWidth: number; theme?: Theme; + disableColor?: boolean; } const DEFAULT_TAB_WIDTH = 4; // Spaces per tab for normalization @@ -99,6 +100,7 @@ export const DiffRenderer: React.FC = ({ availableTerminalHeight, terminalWidth, theme, + disableColor = false, }) => { const settings = useSettings(); @@ -111,17 +113,7 @@ export const DiffRenderer: React.FC = ({ return parseDiffWithLineNumbers(diffContent); }, [diffContent]); - const isNewFile = useMemo(() => { - if (parsedLines.length === 0) return false; - return parsedLines.every( - (line) => - line.type === 'add' || - line.type === 'hunk' || - line.type === 'other' || - line.content.startsWith('diff --git') || - line.content.startsWith('new file mode'), - ); - }, [parsedLines]); + const isNewFileResult = useMemo(() => isNewFile(parsedLines), [parsedLines]); const renderedOutput = useMemo(() => { if (!diffContent || typeof diffContent !== 'string') { @@ -151,7 +143,7 @@ export const DiffRenderer: React.FC = ({ ); } - if (isNewFile) { + if (isNewFileResult) { // Extract only the added lines' content const addedContent = parsedLines .filter((line) => line.type === 'add') @@ -169,39 +161,73 @@ export const DiffRenderer: React.FC = ({ maxWidth: terminalWidth, theme, settings, + disableColor, }); } else { - return renderDiffContent( - parsedLines, - filename, - tabWidth, - availableTerminalHeight, - terminalWidth, + const key = filename + ? `diff-box-${filename}` + : `diff-box-${crypto.createHash('sha1').update(JSON.stringify(parsedLines)).digest('hex')}`; + + return ( + + {renderDiffLines({ + parsedLines, + filename, + tabWidth, + terminalWidth, + disableColor, + })} + ); } }, [ diffContent, parsedLines, screenReaderEnabled, - isNewFile, + isNewFileResult, filename, availableTerminalHeight, terminalWidth, theme, settings, tabWidth, + disableColor, ]); return renderedOutput; }; -const renderDiffContent = ( - parsedLines: DiffLine[], - filename: string | undefined, +export const isNewFile = (parsedLines: DiffLine[]): boolean => { + if (parsedLines.length === 0) return false; + return parsedLines.every( + (line) => + line.type === 'add' || + line.type === 'hunk' || + line.type === 'other' || + line.content.startsWith('diff --git') || + line.content.startsWith('new file mode'), + ); +}; + +export interface RenderDiffLinesOptions { + parsedLines: DiffLine[]; + filename?: string; + tabWidth?: number; + terminalWidth: number; + disableColor?: boolean; +} + +export const renderDiffLines = ({ + parsedLines, + filename, tabWidth = DEFAULT_TAB_WIDTH, - availableTerminalHeight: number | undefined, - terminalWidth: number, -) => { + terminalWidth, + disableColor = false, +}: RenderDiffLinesOptions): React.ReactNode[] => { // 1. Normalize whitespace (replace tabs with spaces) *before* further processing const normalizedLines = parsedLines.map((line) => ({ ...line, @@ -214,15 +240,16 @@ const renderDiffContent = ( ); if (displayableLines.length === 0) { - return ( + return [ No changes detected. - - ); + , + ]; } const maxLineNumber = Math.max( @@ -252,10 +279,6 @@ const renderDiffContent = ( baseIndentation = 0; } - const key = filename - ? `diff-box-${filename}` - : `diff-box-${crypto.createHash('sha1').update(JSON.stringify(parsedLines)).digest('hex')}`; - let lastLineNumber: number | null = null; const MAX_CONTEXT_LINES_WITHOUT_GAP = 5; @@ -321,12 +344,26 @@ const renderDiffContent = ( const displayContent = line.content.substring(baseIndentation); - const backgroundColor = - line.type === 'add' + const backgroundColor = disableColor + ? undefined + : line.type === 'add' ? semanticTheme.background.diff.added : line.type === 'del' ? semanticTheme.background.diff.removed : undefined; + + const gutterColor = disableColor + ? undefined + : semanticTheme.text.secondary; + + const symbolColor = disableColor + ? undefined + : line.type === 'add' + ? semanticTheme.status.success + : line.type === 'del' + ? semanticTheme.status.error + : undefined; + acc.push( - {gutterNumStr} + {gutterNumStr} {line.type === 'context' ? ( <> {prefixSymbol} - {colorizeLine(displayContent, language)} + + {colorizeLine( + displayContent, + language, + undefined, + disableColor, + )} + ) : ( - - - {prefixSymbol} - {' '} - {colorizeLine(displayContent, language)} + + {prefixSymbol}{' '} + {colorizeLine(displayContent, language, undefined, disableColor)} )} , @@ -371,15 +400,7 @@ const renderDiffContent = ( [], ); - return ( - - {content} - - ); + return content; }; const getLanguageFromExtension = (extension: string): string | null => { diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index 24332e83c2..4a8f4690e6 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -34,6 +34,9 @@ describe('ToolConfirmationMessage', () => { confirm: mockConfirm, cancel: vi.fn(), isDiffingEnabled: false, + isExpanded: vi.fn().mockReturnValue(false), + toggleExpansion: vi.fn(), + toggleAllExpansion: vi.fn(), }); const mockConfig = { @@ -467,6 +470,9 @@ describe('ToolConfirmationMessage', () => { confirm: vi.fn(), cancel: vi.fn(), isDiffingEnabled: false, + isExpanded: vi.fn().mockReturnValue(false), + toggleExpansion: vi.fn(), + toggleAllExpansion: vi.fn(), }); const { lastFrame, waitUntilReady, unmount } = renderWithProviders( @@ -495,6 +501,9 @@ describe('ToolConfirmationMessage', () => { confirm: vi.fn(), cancel: vi.fn(), isDiffingEnabled: false, + isExpanded: vi.fn().mockReturnValue(false), + toggleExpansion: vi.fn(), + toggleAllExpansion: vi.fn(), }); const { lastFrame, waitUntilReady, unmount } = renderWithProviders( @@ -523,6 +532,9 @@ describe('ToolConfirmationMessage', () => { confirm: vi.fn(), cancel: vi.fn(), isDiffingEnabled: true, + isExpanded: vi.fn().mockReturnValue(false), + toggleExpansion: vi.fn(), + toggleAllExpansion: vi.fn(), }); const { lastFrame, waitUntilReady, unmount } = renderWithProviders( @@ -666,6 +678,9 @@ describe('ToolConfirmationMessage', () => { confirm: mockConfirm, cancel: vi.fn(), isDiffingEnabled: false, + isExpanded: vi.fn().mockReturnValue(false), + toggleExpansion: vi.fn(), + toggleAllExpansion: vi.fn(), }); const confirmationDetails: SerializableConfirmationDetails = { diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.compact.test.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.compact.test.tsx new file mode 100644 index 0000000000..a75e442b80 --- /dev/null +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.compact.test.tsx @@ -0,0 +1,175 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { renderWithProviders } from '../../../test-utils/render.js'; +import { ToolGroupMessage } from './ToolGroupMessage.js'; +import { + CoreToolCallStatus, + LS_DISPLAY_NAME, + READ_FILE_DISPLAY_NAME, +} from '@google/gemini-cli-core'; +import { expect, it, describe } from 'vitest'; +import type { IndividualToolCallDisplay } from '../../types.js'; +import type { LoadedSettings } from '../../../config/settings.js'; + +describe('ToolGroupMessage Compact Rendering', () => { + const defaultProps = { + item: { + id: '1', + role: 'assistant', + content: '', + timestamp: new Date(), + type: 'help' as const, // Adding type property to satisfy HistoryItem type + }, + terminalWidth: 80, + }; + + const compactSettings: LoadedSettings = { + merged: { + ui: { + compactToolOutput: true, + }, + }, + sources: [], + } as unknown as LoadedSettings; // Test mock of settings + + it('renders consecutive compact tools without empty lines between them', async () => { + const toolCalls: IndividualToolCallDisplay[] = [ + { + callId: 'call1', + name: LS_DISPLAY_NAME, + status: CoreToolCallStatus.Success, + resultDisplay: 'file1.txt\nfile2.txt', + description: 'Listing files', + confirmationDetails: undefined, + isClientInitiated: true, + parentCallId: undefined, + }, + { + callId: 'call2', + name: LS_DISPLAY_NAME, + status: CoreToolCallStatus.Success, + resultDisplay: 'file3.txt', + description: 'Listing files', + confirmationDetails: undefined, + isClientInitiated: true, + parentCallId: undefined, + }, + ]; + + const { lastFrame, waitUntilReady } = renderWithProviders( + , + { settings: compactSettings }, + ); + + await waitUntilReady(); + const output = lastFrame(); + + expect(output).toMatchSnapshot(); + }); + + it('does not add an extra empty line between a compact tool and a standard tool', async () => { + const toolCalls: IndividualToolCallDisplay[] = [ + { + callId: 'call1', + name: LS_DISPLAY_NAME, + status: CoreToolCallStatus.Success, + resultDisplay: 'file1.txt', + description: 'Listing files', + confirmationDetails: undefined, + isClientInitiated: true, + parentCallId: undefined, + }, + { + callId: 'call2', + name: 'non-compact-tool', + status: CoreToolCallStatus.Success, + resultDisplay: 'some large output', + description: 'Doing something', + confirmationDetails: undefined, + isClientInitiated: true, + parentCallId: undefined, + }, + ]; + + const { lastFrame, waitUntilReady } = renderWithProviders( + , + { settings: compactSettings }, + ); + + await waitUntilReady(); + const output = lastFrame(); + expect(output).toMatchSnapshot(); + }); + + it('does not add an extra empty line if a compact tool has a dense payload', async () => { + const toolCalls: IndividualToolCallDisplay[] = [ + { + callId: 'call1', + name: LS_DISPLAY_NAME, + status: CoreToolCallStatus.Success, + resultDisplay: 'file1.txt', + description: 'Listing files', + confirmationDetails: undefined, + isClientInitiated: true, + parentCallId: undefined, + }, + { + callId: 'call2', + name: READ_FILE_DISPLAY_NAME, + status: CoreToolCallStatus.Success, + resultDisplay: { summary: 'read file', payload: 'file content' }, // Dense payload + description: 'Reading file', + confirmationDetails: undefined, + isClientInitiated: true, + parentCallId: undefined, + }, + ]; + + const { lastFrame, waitUntilReady } = renderWithProviders( + , + { settings: compactSettings }, + ); + + await waitUntilReady(); + const output = lastFrame(); + expect(output).toMatchSnapshot(); + }); + + it('does not add an extra empty line between a standard tool and a compact tool', async () => { + const toolCalls: IndividualToolCallDisplay[] = [ + { + callId: 'call1', + name: 'non-compact-tool', + status: CoreToolCallStatus.Success, + resultDisplay: 'some large output', + description: 'Doing something', + confirmationDetails: undefined, + isClientInitiated: true, + parentCallId: undefined, + }, + { + callId: 'call2', + name: LS_DISPLAY_NAME, + status: CoreToolCallStatus.Success, + resultDisplay: 'file1.txt', + description: 'Listing files', + confirmationDetails: undefined, + isClientInitiated: true, + parentCallId: undefined, + }, + ]; + + const { lastFrame, waitUntilReady } = renderWithProviders( + , + { settings: compactSettings }, + ); + + await waitUntilReady(); + const output = lastFrame(); + expect(output).toMatchSnapshot(); + }); +}); diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx index e647c1aec6..3b049ea7f8 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx @@ -5,6 +5,7 @@ */ import { renderWithProviders } from '../../../test-utils/render.js'; +import { act } from 'react'; import { describe, it, expect, vi, afterEach } from 'vitest'; import { ToolGroupMessage } from './ToolGroupMessage.js'; import type { @@ -90,7 +91,9 @@ describe('', () => { }, }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toMatchSnapshot(); unmount(); }); @@ -115,7 +118,9 @@ describe('', () => { ); // Should now hide confirming tools (to avoid duplication with Global Queue) - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toBe(''); unmount(); }); @@ -135,7 +140,9 @@ describe('', () => { { config: baseMockConfig, settings: fullVerbositySettings }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); const output = lastFrame(); expect(output).toMatchSnapshot('canceled_tool'); unmount(); @@ -180,7 +187,9 @@ describe('', () => { }, ); // pending-tool should now be visible - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); const output = lastFrame(); expect(output).toContain('successful-tool'); expect(output).toContain('pending-tool'); @@ -219,7 +228,9 @@ describe('', () => { }, }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); const output = lastFrame(); expect(output).toContain('successful-tool'); expect(output).not.toContain('error-tool'); @@ -253,7 +264,9 @@ describe('', () => { }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); const output = lastFrame(); expect(output).toContain('client-error-tool'); unmount(); @@ -298,7 +311,9 @@ describe('', () => { }, ); // write_file (Pending) should now be visible - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); const output = lastFrame(); expect(output).toContain('read_file'); expect(output).toContain('run_shell_command'); @@ -344,7 +359,9 @@ describe('', () => { }, }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toMatchSnapshot(); unmount(); }); @@ -378,7 +395,9 @@ describe('', () => { }, }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toMatchSnapshot(); unmount(); }); @@ -401,7 +420,9 @@ describe('', () => { }, }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toMatchSnapshot(); unmount(); }); @@ -440,7 +461,9 @@ describe('', () => { }, }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toMatchSnapshot(); unmount(); }); @@ -471,7 +494,9 @@ describe('', () => { }, }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toMatchSnapshot(); unmount(); }); @@ -526,7 +551,9 @@ describe('', () => { }, }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toMatchSnapshot(); unmount(); }); @@ -556,7 +583,9 @@ describe('', () => { }, }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toMatchSnapshot(); unmount(); }); @@ -586,7 +615,9 @@ describe('', () => { }, }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toMatchSnapshot(); unmount(); }); @@ -629,7 +660,9 @@ describe('', () => { }, }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toMatchSnapshot(); unmount(); }); @@ -680,7 +713,9 @@ describe('', () => { , { config: baseMockConfig, settings: fullVerbositySettings }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); if (shouldHide) { expect(lastFrame({ allowEmpty: true })).toBe(''); @@ -711,7 +746,9 @@ describe('', () => { { config: baseMockConfig, settings: fullVerbositySettings }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toMatchSnapshot(); unmount(); }); @@ -739,7 +776,9 @@ describe('', () => { { config: baseMockConfig, settings: fullVerbositySettings }, ); // AskUser tools in progress are rendered by AskUserDialog, so we expect nothing. - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toBe(''); unmount(); }); @@ -770,7 +809,9 @@ describe('', () => { }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toBe(''); unmount(); }); @@ -793,7 +834,9 @@ describe('', () => { }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).not.toBe(''); unmount(); }); @@ -824,7 +867,9 @@ describe('', () => { }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toBe(''); unmount(); }); @@ -857,7 +902,9 @@ describe('', () => { }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).toBe(''); unmount(); }); @@ -952,7 +999,9 @@ describe('', () => { }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); const output = lastFrame(); expect(output).toContain('visible-tool'); expect(output).not.toContain('hidden-error-0'); @@ -978,7 +1027,9 @@ describe('', () => { }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); expect(lastFrame({ allowEmpty: true })).not.toBe(''); unmount(); }); @@ -1016,7 +1067,9 @@ describe('', () => { { config: baseMockConfig, settings: fullVerbositySettings }, ); - await waitUntilReady(); + await act(async () => { + await waitUntilReady(); + }); if (visible) { expect(lastFrame()).toContain(name); diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx index 69da3a1029..9b723d8b04 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx @@ -5,17 +5,24 @@ */ import type React from 'react'; -import { useMemo } from 'react'; +import { useMemo, Fragment } from 'react'; import { Box, Text } from 'ink'; import type { HistoryItem, HistoryItemWithoutId, IndividualToolCallDisplay, } from '../../types.js'; -import { ToolCallStatus, mapCoreStatusToDisplayStatus } from '../../types.js'; +import { + ToolCallStatus, + mapCoreStatusToDisplayStatus, + isFileDiff, + isGrepResult, + isListResult, +} from '../../types.js'; import { ToolMessage } from './ToolMessage.js'; import { ShellToolMessage } from './ShellToolMessage.js'; import { SubagentGroupDisplay } from './SubagentGroupDisplay.js'; +import { DenseToolMessage } from './DenseToolMessage.js'; import { theme } from '../../semantic-colors.js'; import { useConfig } from '../../contexts/ConfigContext.js'; import { isShellTool } from './ToolShared.js'; @@ -23,11 +30,74 @@ import { shouldHideToolCall, CoreToolCallStatus, Kind, + EDIT_DISPLAY_NAME, + GLOB_DISPLAY_NAME, + WEB_SEARCH_DISPLAY_NAME, + READ_FILE_DISPLAY_NAME, + LS_DISPLAY_NAME, + GREP_DISPLAY_NAME, + WEB_FETCH_DISPLAY_NAME, + WRITE_FILE_DISPLAY_NAME, + READ_MANY_FILES_DISPLAY_NAME, } from '@google/gemini-cli-core'; import { useUIState } from '../../contexts/UIStateContext.js'; import { getToolGroupBorderAppearance } from '../../utils/borderStyles.js'; import { useSettings } from '../../contexts/SettingsContext.js'; +const COMPACT_OUTPUT_ALLOWLIST = new Set([ + EDIT_DISPLAY_NAME, + GLOB_DISPLAY_NAME, + WEB_SEARCH_DISPLAY_NAME, + READ_FILE_DISPLAY_NAME, + LS_DISPLAY_NAME, + GREP_DISPLAY_NAME, + WEB_FETCH_DISPLAY_NAME, + WRITE_FILE_DISPLAY_NAME, + READ_MANY_FILES_DISPLAY_NAME, +]); + +// Helper to identify if a tool should use the compact view +export const isCompactTool = ( + tool: IndividualToolCallDisplay, + isCompactModeEnabled: boolean, +): boolean => { + const hasCompactOutputSupport = COMPACT_OUTPUT_ALLOWLIST.has(tool.name); + const displayStatus = mapCoreStatusToDisplayStatus(tool.status); + return ( + isCompactModeEnabled && + hasCompactOutputSupport && + displayStatus !== ToolCallStatus.Confirming + ); +}; + +// Helper to identify if a compact tool has a payload (diff, list, etc.) +export const hasDensePayload = (tool: IndividualToolCallDisplay): boolean => { + if (tool.outputFile) return true; + const res = tool.resultDisplay; + if (!res) return false; + + if (isFileDiff(res)) return true; + if (tool.confirmationDetails?.type === 'edit') return true; + if (isGrepResult(res) && (res.matches?.length ?? 0) > 0) return true; + + // ReadManyFilesResult check (has 'include' and 'files') + if (isListResult(res) && 'include' in res && (res.files?.length ?? 0) > 0) { + return true; + } + + // Generic summary/payload pattern + if ( + typeof res === 'object' && + res !== null && + 'summary' in res && + 'payload' in res + ) { + return true; + } + + return false; +}; + interface ToolGroupMessageProps { item: HistoryItem | HistoryItemWithoutId; toolCalls: IndividualToolCallDisplay[]; @@ -53,6 +123,7 @@ export const ToolGroupMessage: React.FC = ({ }) => { const settings = useSettings(); const isLowErrorVerbosity = settings.merged.ui?.errorVerbosity !== 'full'; + const isCompactModeEnabled = settings.merged.ui?.compactToolOutput === true; // Filter out tool calls that should be hidden (e.g. in-progress Ask User, or Plan Mode operations). const toolCalls = useMemo( @@ -123,21 +194,6 @@ export const ToolGroupMessage: React.FC = ({ [toolCalls], ); - const staticHeight = /* border */ 2; - - let countToolCallsWithResults = 0; - for (const tool of visibleToolCalls) { - if ( - tool.kind !== Kind.Agent && - tool.resultDisplay !== undefined && - tool.resultDisplay !== '' - ) { - countToolCallsWithResults++; - } - } - const countOneLineToolCalls = - visibleToolCalls.filter((t) => t.kind !== Kind.Agent).length - - countToolCallsWithResults; const groupedTools = useMemo(() => { const groups: Array< IndividualToolCallDisplay | IndividualToolCallDisplay[] @@ -157,10 +213,69 @@ export const ToolGroupMessage: React.FC = ({ return groups; }, [visibleToolCalls]); + const staticHeight = useMemo(() => { + let height = 0; + for (let i = 0; i < groupedTools.length; i++) { + const group = groupedTools[i]; + const isFirst = i === 0; + + if (Array.isArray(group)) { + // Agent group + height += 1; // Header + height += group.length; // 1 line per agent + // Spacing logic + if (isFirst) { + height += (borderTopOverride ?? true) ? 1 : 0; + } else { + height += 1; // marginTop + } + } else { + const tool = group; + const isCompact = isCompactTool(tool, isCompactModeEnabled); + const prevGroup = i > 0 ? groupedTools[i - 1] : null; + const prevIsCompact = + prevGroup && + !Array.isArray(prevGroup) && + isCompactTool(prevGroup, isCompactModeEnabled); + + if (isCompact) { + height += 1; // Base height for compact tool + // Spacing logic (matching marginTop) + if (isFirst) { + height += (borderTopOverride ?? true) ? 1 : 0; + } else if (!prevIsCompact) { + height += 1; + } + } else { + height += 3; // Static overhead for standard tool + if (isFirst) { + height += (borderTopOverride ?? true) ? 1 : 0; + } else { + height += 1; // marginTop is always 1 for non-compact tools (not first) + } + } + } + } + return height; + }, [groupedTools, isCompactModeEnabled, borderTopOverride]); + + let countToolCallsWithResults = 0; + for (const tool of visibleToolCalls) { + if (tool.kind !== Kind.Agent) { + if (isCompactTool(tool, isCompactModeEnabled)) { + if (hasDensePayload(tool)) { + countToolCallsWithResults++; + } + } else if (tool.resultDisplay !== undefined && tool.resultDisplay !== '') { + countToolCallsWithResults++; + } + } + } + const availableTerminalHeightPerToolMessage = availableTerminalHeight ? Math.max( Math.floor( - (availableTerminalHeight - staticHeight - countOneLineToolCalls) / + (availableTerminalHeight - staticHeight) / Math.max(1, countToolCallsWithResults), ), 1, @@ -174,10 +289,11 @@ export const ToolGroupMessage: React.FC = ({ // border fragments. The only case where an empty group should render is the // explicit "closing slice" (tools: []) used to bridge static/pending sections. const isExplicitClosingSlice = allToolCalls.length === 0; - if ( - visibleToolCalls.length === 0 && - (!isExplicitClosingSlice || borderBottomOverride !== true) - ) { + const shouldShowGroup = + visibleToolCalls.length > 0 || + (isExplicitClosingSlice && borderBottomOverride === true); + + if (!shouldShowGroup) { return null; } @@ -192,97 +308,170 @@ export const ToolGroupMessage: React.FC = ({ */ width={terminalWidth} paddingRight={TOOL_MESSAGE_HORIZONTAL_MARGIN} + marginBottom={(borderBottomOverride ?? true) ? 1 : 0} > - {groupedTools.map((group, index) => { - const isFirst = index === 0; - const resolvedIsFirst = - borderTopOverride !== undefined - ? borderTopOverride && isFirst - : isFirst; - - if (Array.isArray(group)) { - return ( - - ); - } - - const tool = group; - const isShellToolCall = isShellTool(tool.name); - - const commonProps = { - ...tool, - availableTerminalHeight: availableTerminalHeightPerToolMessage, - terminalWidth: contentWidth, - emphasis: 'medium' as const, - isFirst: resolvedIsFirst, - borderColor, - borderDimColor, - isExpandable, - }; - - return ( - - {isShellToolCall ? ( - - ) : ( - - )} - {tool.outputFile && ( - - - - Output too long and was saved to: {tool.outputFile} - - - - )} - - ); - })} - { - /* - We have to keep the bottom border separate so it doesn't get - drawn over by the sticky header directly inside it. - */ - (visibleToolCalls.length > 0 || borderBottomOverride !== undefined) && ( + {visibleToolCalls.length === 0 && + isExplicitClosingSlice && + borderBottomOverride === true && ( - ) - } + )} + {groupedTools.map((group, index) => { + const isFirst = index === 0; + const isLast = index === groupedTools.length - 1; + + const prevGroup = index > 0 ? groupedTools[index - 1] : null; + const prevIsCompact = + prevGroup && + !Array.isArray(prevGroup) && + isCompactTool(prevGroup, isCompactModeEnabled); + + const nextGroup = !isLast ? groupedTools[index + 1] : null; + const nextIsCompact = + nextGroup && + !Array.isArray(nextGroup) && + isCompactTool(nextGroup, isCompactModeEnabled); + + if (Array.isArray(group)) { + // Subagent group behaves like a standard tool for borders + const marginTop = isFirst ? (borderTopOverride ?? true ? 1 : 0) : 1; + const isFirstProp = !!(isFirst ? (borderTopOverride ?? true) : prevIsCompact); + + return ( + + + {(nextIsCompact || isLast) && ( + + )} + + ); + } + + const tool = group; + const isCompact = isCompactTool(tool, isCompactModeEnabled); + const isShellToolCall = isShellTool(tool.name); + + let marginTop = 0; + if (isFirst) { + marginTop = (borderTopOverride ?? true) ? 1 : 0; + } else if (isCompact && prevIsCompact) { + marginTop = 0; + // } else if (!isCompact && prevIsCompact) { + // marginTop = 1; + } else { + marginTop = 1; + } + + let createTopBorder = true; + if (isCompact) { + createTopBorder = false; + } + // } else if (isFirst) { + // createTopBorder = borderTopOverride ?? true; + // } else { + // createTopBorder = !!prevIsCompact; + // } + + const commonProps = { + ...tool, + availableTerminalHeight: availableTerminalHeightPerToolMessage, + terminalWidth: contentWidth, + emphasis: 'medium' as const, + // isFirst: !!(isCompact + // ? false + // : isFirst + // ? (borderTopOverride ?? true) + // : prevIsCompact), + isFirst: createTopBorder, + borderColor, + borderDimColor, + isExpandable, + }; + + return ( + + + {isCompact ? ( + + ) : isShellToolCall ? ( + + ) : ( + + )} + {!isCompact && tool.outputFile && ( + + + + Output too long and was saved to: {tool.outputFile} + + + + )} + + {!isCompact && (nextIsCompact || isLast) && ( + + )} + + ); + })} ); diff --git a/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx b/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx index 3b7cfaa8da..3a2ca1b2ed 100644 --- a/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx +++ b/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx @@ -15,6 +15,7 @@ import { type AnsiOutput, type AnsiLine, isSubagentProgress, + isStructuredToolResult, } from '@google/gemini-cli-core'; import { useUIState } from '../../contexts/UIStateContext.js'; import { tryParseJSON } from '../../../utils/jsonoutput.js'; @@ -123,7 +124,28 @@ export const ToolResultDisplay: React.FC = ({ {contentData} ); - } else if (typeof contentData === 'object' && 'fileDiff' in contentData) { + } else if (isStructuredToolResult(contentData)) { + if (renderOutputAsMarkdown) { + content = ( + + ); + } else { + content = ( + + {contentData.summary} + + ); + } + } else if ( + typeof contentData === 'object' && + contentData !== null && + 'fileDiff' in contentData + ) { content = ( { expect(lastFrame()).toContain('tool-1'); }); expect(lastFrame()).toContain('Description for tool-1'); - // Content lines 1-4 should be scrolled off + // Content lines 1-5 should be scrolled off expect(lastFrame()).not.toContain('c1-01'); - expect(lastFrame()).not.toContain('c1-04'); - // Line 6 and 7 should be visible (terminalHeight=5 means only 2 lines of content show below 3-line header) + expect(lastFrame()).not.toContain('c1-05'); + // Line 6 and 7 should be visible (terminalHeight=5 means only 2 lines of content show below 3-line header + margin) expect(lastFrame()).toContain('c1-06'); expect(lastFrame()).toContain('c1-07'); expect(lastFrame()).toMatchSnapshot(); diff --git a/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage.test.tsx.snap new file mode 100644 index 0000000000..6380d97644 --- /dev/null +++ b/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage.test.tsx.snap @@ -0,0 +1,132 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`DenseToolMessage > Toggleable Diff View (Alternate Buffer) > hides diff content by default when in alternate buffer mode 1`] = ` +" ✓ test-tool test.ts → Accepted [click here to show details] +" +`; + +exports[`DenseToolMessage > Toggleable Diff View (Alternate Buffer) > shows diff content by default when NOT in alternate buffer mode 1`] = ` +" ✓ test-tool test.ts → Accepted + + 1 - old line + 1 + new line +" +`; + +exports[`DenseToolMessage > does not render result arrow if resultDisplay is missing 1`] = ` +" o test-tool Test description +" +`; + +exports[`DenseToolMessage > flattens newlines in string results 1`] = ` +" ✓ test-tool Test description → Line 1 Line 2 +" +`; + +exports[`DenseToolMessage > renders correctly for Edit tool using confirmationDetails 1`] = ` +" ? Edit styles.scss → Confirming + + 1 - body { color: blue; } + 1 + body { color: red; } +" +`; + +exports[`DenseToolMessage > renders correctly for Errored Edit tool 1`] = ` +" x Edit styles.scss → Failed [click here to show details] +" +`; + +exports[`DenseToolMessage > renders correctly for ReadManyFiles results 1`] = ` +" ✓ test-tool Attempting to read files from **/*.ts → Read 3 file(s) (1 ignored) + + file1.ts + file2.ts + file3.ts +" +`; + +exports[`DenseToolMessage > renders correctly for Rejected Edit tool 1`] = ` +" - Edit styles.scss → Rejected (+1, -1) + + 1 - old line + 1 + new line +" +`; + +exports[`DenseToolMessage > renders correctly for Rejected Edit tool with confirmationDetails and diffStat 1`] = ` +" - Edit styles.scss → Rejected (+1, -1) + + 1 - body { color: blue; } + 1 + body { color: red; } +" +`; + +exports[`DenseToolMessage > renders correctly for Rejected WriteFile tool 1`] = ` +" - WriteFile config.json → Rejected + + 1 - old content + 1 + new content +" +`; + +exports[`DenseToolMessage > renders correctly for WriteFile tool 1`] = ` +" ✓ WriteFile config.json → Accepted (+1, -1) + + 1 - old content + 1 + new content +" +`; + +exports[`DenseToolMessage > renders correctly for a successful string result 1`] = ` +" ✓ test-tool Test description → Success result +" +`; + +exports[`DenseToolMessage > renders correctly for error status with string message 1`] = ` +" x test-tool Test description → Error occurred +" +`; + +exports[`DenseToolMessage > renders correctly for file diff results with stats 1`] = ` +" ✓ test-tool test.ts → Accepted (+15, -6) + + 1 - old line + 1 + diff content +" +`; + +exports[`DenseToolMessage > renders correctly for grep results 1`] = ` +" ✓ test-tool Test description → Found 2 matches + + file1.ts:10: match 1 + file2.ts:20: match 2 +" +`; + +exports[`DenseToolMessage > renders correctly for ls results 1`] = ` +" ✓ test-tool Test description → Listed 2 files. (1 ignored) +" +`; + +exports[`DenseToolMessage > renders correctly for todo updates 1`] = ` +" ✓ test-tool Test description → Todos updated +" +`; + +exports[`DenseToolMessage > renders generic failure message for error status without string message 1`] = ` +" x test-tool Test description → Failed +" +`; + +exports[`DenseToolMessage > renders generic output message for unknown object results 1`] = ` +" ✓ test-tool Test description → Output received +" +`; + +exports[`DenseToolMessage > truncates long string results 1`] = ` +" ✓ test-tool Test description + → + AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA + AAAAAAAAAAAAAAAAAAAA... +" +`; diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.compact.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.compact.test.tsx.snap new file mode 100644 index 0000000000..37b111ed1e --- /dev/null +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.compact.test.tsx.snap @@ -0,0 +1,35 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`ToolGroupMessage Compact Rendering > does not add an extra empty line between a compact tool and a standard tool 1`] = ` +" ✓ ReadFolder Listing files → file1.txt + +╭──────────────────────────────────────────────────────────────────────────╮ +│ ✓ non-compact-tool Doing something │ +│ │ +│ some large output │ +╰──────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`ToolGroupMessage Compact Rendering > does not add an extra empty line between a standard tool and a compact tool 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────╮ +│ ✓ non-compact-tool Doing something │ +│ │ +│ some large output │ +╰──────────────────────────────────────────────────────────────────────────╯ + + ✓ ReadFolder Listing files → file1.txt +" +`; + +exports[`ToolGroupMessage Compact Rendering > does not add an extra empty line if a compact tool has a dense payload 1`] = ` +" ✓ ReadFolder Listing files → file1.txt + ✓ ReadFile Reading file → read file +" +`; + +exports[`ToolGroupMessage Compact Rendering > renders consecutive compact tools without empty lines between them 1`] = ` +" ✓ ReadFolder Listing files → file1.txt file2.txt + ✓ ReadFolder Listing files → file3.txt +" +`; diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap index 98db513da8..6980837725 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap @@ -32,6 +32,7 @@ exports[` > Border Color Logic > uses gray border when all t │ ✓ test-tool A tool for testing │ │ │ │ Test result │ + │ │ │ ✓ another-tool A tool for testing │ │ │ @@ -61,16 +62,16 @@ exports[` > Golden Snapshots > renders canceled tool calls > exports[` > Golden Snapshots > renders empty tool calls array 1`] = `""`; exports[` > Golden Snapshots > renders header when scrolled 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────╮ -│ ✓ tool-1 Description 1. This is a long description that will need to b… │ +"│ ✓ tool-1 Description 1. This is a long description that will need to b… │ │──────────────────────────────────────────────────────────────────────────│ -│ line5 │ █ -│ │ █ + +│ │ ▄ │ ✓ tool-2 Description 2 │ █ │ │ █ │ line1 │ █ │ line2 │ █ ╰──────────────────────────────────────────────────────────────────────────╯ █ + █ " `; @@ -79,10 +80,12 @@ exports[` > Golden Snapshots > renders mixed tool calls incl │ ✓ read_file Read a file │ │ │ │ Test result │ + │ │ │ ⊶ run_shell_command Run command │ │ │ │ Test result │ + │ │ │ o write_file Write to file │ │ │ @@ -96,10 +99,12 @@ exports[` > Golden Snapshots > renders multiple tool calls w │ ✓ successful-tool This tool succeeded │ │ │ │ Test result │ + │ │ │ o pending-tool This tool is pending │ │ │ │ Test result │ + │ │ │ x error-tool This tool failed │ │ │ @@ -128,12 +133,12 @@ exports[` > Golden Snapshots > renders tool call with output `; exports[` > Golden Snapshots > renders two tool groups where only the last line of the previous group is visible 1`] = ` -"╰──────────────────────────────────────────────────────────────────────────╯ -╭──────────────────────────────────────────────────────────────────────────╮ +"╭──────────────────────────────────────────────────────────────────────────╮ │ ✓ tool-2 Description 2 │ -│ │ ▄ -│ line1 │ █ +│ │ +│ line1 │ ▄ ╰──────────────────────────────────────────────────────────────────────────╯ █ + █ " `; @@ -142,6 +147,7 @@ exports[` > Golden Snapshots > renders with limited terminal │ ✓ tool-with-result Tool with output │ │ │ │ This is a long result that might need height constraints │ + │ │ │ ✓ another-tool Another tool │ │ │ @@ -164,10 +170,12 @@ exports[` > Height Calculation > calculates available height │ ✓ test-tool A tool for testing │ │ │ │ Result 1 │ + │ │ │ ✓ test-tool A tool for testing │ │ │ │ Result 2 │ + │ │ │ ✓ test-tool A tool for testing │ │ │ diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolStickyHeaderRegression.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolStickyHeaderRegression.test.tsx.snap index dda93c1c21..16627ec220 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolStickyHeaderRegression.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolStickyHeaderRegression.test.tsx.snap @@ -2,7 +2,7 @@ exports[`ToolMessage Sticky Header Regression > verifies that ShellToolMessage in a ToolGroupMessage in a ScrollableList has sticky headers 1`] = ` "╭────────────────────────────────────────────────────────────────────────╮ █ -│ ✓ Shell Command Description for Shell Command │ █ +│ ✓ Shell Command Description for Shell Command │ ▀ │ │ │ shell-01 │ │ shell-02 │ @@ -11,7 +11,7 @@ exports[`ToolMessage Sticky Header Regression > verifies that ShellToolMessage i exports[`ToolMessage Sticky Header Regression > verifies that ShellToolMessage in a ToolGroupMessage in a ScrollableList has sticky headers 2`] = ` "╭────────────────────────────────────────────────────────────────────────╮ -│ ✓ Shell Command Description for Shell Command │ ▄ +│ ✓ Shell Command Description for Shell Command │ │────────────────────────────────────────────────────────────────────────│ █ │ shell-06 │ ▀ │ shell-07 │ @@ -40,7 +40,7 @@ exports[`ToolMessage Sticky Header Regression > verifies that multiple ToolMessa "│ │ │ ✓ tool-2 Description for tool-2 │ │────────────────────────────────────────────────────────────────────────│ -│ c2-10 │ -╰────────────────────────────────────────────────────────────────────────╯ █ +│ c2-09 │ ▄ +│ c2-10 │ ▀ " `; diff --git a/packages/cli/src/ui/constants.ts b/packages/cli/src/ui/constants.ts index db52be1105..ccf4b56cd5 100644 --- a/packages/cli/src/ui/constants.ts +++ b/packages/cli/src/ui/constants.ts @@ -58,3 +58,6 @@ export const MIN_TERMINAL_WIDTH_FOR_FULL_LABEL = 100; /** Default context usage fraction at which to trigger compression */ export const DEFAULT_COMPRESSION_THRESHOLD = 0.5; + +/** Max lines to show for a compact tool subview (e.g. diff) */ +export const COMPACT_TOOL_SUBVIEW_MAX_LINES = 15; diff --git a/packages/cli/src/ui/contexts/ToolActionsContext.test.tsx b/packages/cli/src/ui/contexts/ToolActionsContext.test.tsx index 8a75bf7d57..ba2063ba85 100644 --- a/packages/cli/src/ui/contexts/ToolActionsContext.test.tsx +++ b/packages/cli/src/ui/contexts/ToolActionsContext.test.tsx @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { act } from 'react'; +import { act, useState, useCallback } from 'react'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { renderHook } from '../../test-utils/render.js'; import { waitFor } from '../../test-utils/async.js'; @@ -72,16 +72,60 @@ describe('ToolActionsContext', () => { beforeEach(() => { vi.clearAllMocks(); + // Default to a pending promise to avoid unwanted async state updates in tests + // that don't specifically test the IdeClient initialization. + vi.mocked(IdeClient.getInstance).mockReturnValue(new Promise(() => {})); }); - const wrapper = ({ children }: { children: React.ReactNode }) => ( - - {children} - - ); + const Wrapper = ({ children }: { children: React.ReactNode }) => { + const [expandedTools, setExpandedTools] = useState>(new Set()); + + const isExpanded = useCallback( + (callId: string) => expandedTools.has(callId), + [expandedTools], + ); + + const toggleExpansion = useCallback((callId: string) => { + setExpandedTools((prev) => { + const next = new Set(prev); + if (next.has(callId)) { + next.delete(callId); + } else { + next.add(callId); + } + return next; + }); + }, []); + + const toggleAllExpansion = useCallback((callIds: string[]) => { + setExpandedTools((prev) => { + const next = new Set(prev); + const anyCollapsed = callIds.some((id) => !next.has(id)); + + if (anyCollapsed) { + callIds.forEach((id) => next.add(id)); + } else { + callIds.forEach((id) => next.delete(id)); + } + return next; + }); + }, []); + + return ( + + {children} + + ); + }; it('publishes to MessageBus for tools with correlationId', async () => { - const { result } = renderHook(() => useToolActions(), { wrapper }); + const { result } = renderHook(() => useToolActions(), { wrapper: Wrapper }); await result.current.confirm( 'modern-call', @@ -99,7 +143,7 @@ describe('ToolActionsContext', () => { }); it('handles cancel by calling confirm with Cancel outcome', async () => { - const { result } = renderHook(() => useToolActions(), { wrapper }); + const { result } = renderHook(() => useToolActions(), { wrapper: Wrapper }); await result.current.cancel('modern-call'); @@ -119,7 +163,7 @@ describe('ToolActionsContext', () => { vi.mocked(IdeClient.getInstance).mockResolvedValue(mockIdeClient); vi.mocked(mockConfig.getIdeMode).mockReturnValue(true); - const { result } = renderHook(() => useToolActions(), { wrapper }); + const { result } = renderHook(() => useToolActions(), { wrapper: Wrapper }); // Wait for IdeClient initialization in useEffect await act(async () => { @@ -157,7 +201,7 @@ describe('ToolActionsContext', () => { vi.mocked(IdeClient.getInstance).mockResolvedValue(mockIdeClient); vi.mocked(mockConfig.getIdeMode).mockReturnValue(true); - const { result } = renderHook(() => useToolActions(), { wrapper }); + const { result } = renderHook(() => useToolActions(), { wrapper: Wrapper }); // Wait for initialization await act(async () => { @@ -204,7 +248,13 @@ describe('ToolActionsContext', () => { const { result } = renderHook(() => useToolActions(), { wrapper: ({ children }) => ( - + {children} ), @@ -223,4 +273,58 @@ describe('ToolActionsContext', () => { ); expect(mockMessageBus.publish).not.toHaveBeenCalled(); }); + + describe('toggleAllExpansion', () => { + it('expands all when none are expanded', () => { + const { result } = renderHook(() => useToolActions(), { + wrapper: Wrapper, + }); + + act(() => { + result.current.toggleAllExpansion(['modern-call', 'edit-call']); + }); + + expect(result.current.isExpanded('modern-call')).toBe(true); + expect(result.current.isExpanded('edit-call')).toBe(true); + }); + + it('expands all when some are expanded', () => { + const { result } = renderHook(() => useToolActions(), { + wrapper: Wrapper, + }); + + act(() => { + result.current.toggleExpansion('modern-call'); + }); + expect(result.current.isExpanded('modern-call')).toBe(true); + expect(result.current.isExpanded('edit-call')).toBe(false); + + act(() => { + result.current.toggleAllExpansion(['modern-call', 'edit-call']); + }); + + expect(result.current.isExpanded('modern-call')).toBe(true); + expect(result.current.isExpanded('edit-call')).toBe(true); + }); + + it('collapses all when all are expanded', () => { + const { result } = renderHook(() => useToolActions(), { + wrapper: Wrapper, + }); + + act(() => { + result.current.toggleExpansion('modern-call'); + result.current.toggleExpansion('edit-call'); + }); + expect(result.current.isExpanded('modern-call')).toBe(true); + expect(result.current.isExpanded('edit-call')).toBe(true); + + act(() => { + result.current.toggleAllExpansion(['modern-call', 'edit-call']); + }); + + expect(result.current.isExpanded('modern-call')).toBe(false); + expect(result.current.isExpanded('edit-call')).toBe(false); + }); + }); }); diff --git a/packages/cli/src/ui/contexts/ToolActionsContext.tsx b/packages/cli/src/ui/contexts/ToolActionsContext.tsx index 10e063e098..b23a2b2b86 100644 --- a/packages/cli/src/ui/contexts/ToolActionsContext.tsx +++ b/packages/cli/src/ui/contexts/ToolActionsContext.tsx @@ -48,11 +48,14 @@ interface ToolActionsContextValue { ) => Promise; cancel: (callId: string) => Promise; isDiffingEnabled: boolean; + isExpanded: (callId: string) => boolean; + toggleExpansion: (callId: string) => void; + toggleAllExpansion: (callIds: string[]) => void; } const ToolActionsContext = createContext(null); -export const useToolActions = () => { +export const useToolActions = (): ToolActionsContextValue => { const context = useContext(ToolActionsContext); if (!context) { throw new Error('useToolActions must be used within a ToolActionsProvider'); @@ -64,12 +67,22 @@ interface ToolActionsProviderProps { children: React.ReactNode; config: Config; toolCalls: IndividualToolCallDisplay[]; + isExpanded: (callId: string) => boolean; + toggleExpansion: (callId: string) => void; + toggleAllExpansion: (callIds: string[]) => void; } export const ToolActionsProvider: React.FC = ( props: ToolActionsProviderProps, ) => { - const { children, config, toolCalls } = props; + const { + children, + config, + toolCalls, + isExpanded, + toggleExpansion, + toggleAllExpansion, + } = props; // Hoist IdeClient logic here to keep UI pure const [ideClient, setIdeClient] = useState(null); @@ -164,7 +177,16 @@ export const ToolActionsProvider: React.FC = ( ); return ( - + {children} ); diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index 2034e14b87..58ad369712 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -76,6 +76,7 @@ import type { UseHistoryManagerReturn } from './useHistoryManager.js'; import { useLogger } from './useLogger.js'; import { SHELL_COMMAND_NAME } from '../constants.js'; import { mapToDisplay as mapTrackedToolCallsToDisplay } from './toolMapping.js'; +import { isCompactTool } from '../components/messages/ToolGroupMessage.js'; import { useToolScheduler, type TrackedToolCall, @@ -292,9 +293,32 @@ export const useGeminiStream = ( (tc) => !pushedToolCallIdsRef.current.has(tc.request.callId), ); if (toolsToPush.length > 0) { + const isCompactModeEnabled = + settings.merged.ui?.compactToolOutput === true; + const firstToolToPush = toolsToPush[0]; + const tcIndex = toolCalls.indexOf(firstToolToPush); + const prevTool = tcIndex > 0 ? toolCalls[tcIndex - 1] : null; + + let borderTop = isFirstToolInGroupRef.current; + if (!borderTop && prevTool) { + // If the first tool in this push is non-compact but follows a compact tool, + // we must start a new border group. + const currentIsCompact = isCompactTool( + mapTrackedToolCallsToDisplay(firstToolToPush).tools[0], + isCompactModeEnabled, + ); + const prevWasCompact = isCompactTool( + mapTrackedToolCallsToDisplay(prevTool).tools[0], + isCompactModeEnabled, + ); + if (!currentIsCompact && prevWasCompact) { + borderTop = true; + } + } + addItem( mapTrackedToolCallsToDisplay(toolsToPush as TrackedToolCall[], { - borderTop: isFirstToolInGroupRef.current, + borderTop, borderBottom: true, borderColor: theme.border.default, borderDimColor: false, @@ -329,9 +353,7 @@ export const useGeminiStream = ( } // Handle tool response submission immediately when tools complete - await handleCompletedTools( - completedToolCallsFromScheduler as TrackedToolCall[], - ); + await handleCompletedTools(completedToolCallsFromScheduler); } }, config, @@ -461,26 +483,85 @@ export const useGeminiStream = ( if (toolsToPush.length > 0) { const newPushed = new Set(pushedToolCallIdsRef.current); + const isFirstInThisPush = isFirstToolInGroupRef.current; + const isCompactModeEnabled = + settings.merged.ui?.compactToolOutput === true; + + const groups: TrackedToolCall[][] = []; + let currentGroup: TrackedToolCall[] = []; for (const tc of toolsToPush) { newPushed.add(tc.request.callId); + + if (tc.tool?.kind === Kind.Agent) { + currentGroup.push(tc); + } else { + if (currentGroup.length > 0) { + groups.push(currentGroup); + currentGroup = []; + } + groups.push([tc]); + } + } + if (currentGroup.length > 0) { + groups.push(currentGroup); } - const isLastInBatch = - toolsToPush[toolsToPush.length - 1] === toolCalls[toolCalls.length - 1]; + for (let i = 0; i < groups.length; i++) { + const group = groups[i]; + const isFirstInBatch = i === 0 && isFirstInThisPush; + const lastTcInGroup = group[group.length - 1]; + const tcIndexInBatch = toolCalls.indexOf(lastTcInGroup); + const isLastInBatch = tcIndexInBatch === toolCalls.length - 1; - const historyItem = mapTrackedToolCallsToDisplay(toolsToPush, { - borderTop: isFirstToolInGroupRef.current, - borderBottom: isLastInBatch, - ...getToolGroupBorderAppearance( - { type: 'tool_group', tools: toolCalls }, - activeShellPtyId, - !!isShellFocused, - [], - backgroundShells, - ), - }); - addItem(historyItem); + const nextTcInBatch = + tcIndexInBatch < toolCalls.length - 1 + ? toolCalls[tcIndexInBatch + 1] + : null; + const prevTcInBatch = + toolCalls.indexOf(group[0]) > 0 + ? toolCalls[toolCalls.indexOf(group[0]) - 1] + : null; + + const historyItem = mapTrackedToolCallsToDisplay(group, { + ...getToolGroupBorderAppearance( + { type: 'tool_group', tools: toolCalls }, + activeShellPtyId, + !!isShellFocused, + [], + backgroundShells, + ), + }); + + // Determine if this group starts with a compact tool + const currentIsCompact = + historyItem.tools.length === 1 && + isCompactTool(historyItem.tools[0], isCompactModeEnabled); + + let nextIsCompact = false; + if (nextTcInBatch) { + const nextHistoryItem = mapTrackedToolCallsToDisplay(nextTcInBatch); + nextIsCompact = + nextHistoryItem.tools.length === 1 && + isCompactTool(nextHistoryItem.tools[0], isCompactModeEnabled); + } + + let prevWasCompact = false; + if (prevTcInBatch) { + const prevHistoryItem = mapTrackedToolCallsToDisplay(prevTcInBatch); + prevWasCompact = + prevHistoryItem.tools.length === 1 && + isCompactTool(prevHistoryItem.tools[0], isCompactModeEnabled); + } + + historyItem.borderTop = + isFirstInBatch || (!currentIsCompact && prevWasCompact); + historyItem.borderBottom = currentIsCompact + ? isLastInBatch && !nextIsCompact + : isLastInBatch || nextIsCompact; + + addItem(historyItem); + } setPushedToolCallIds(newPushed); setIsFirstToolInGroup(false); @@ -495,6 +576,7 @@ export const useGeminiStream = ( activeShellPtyId, isShellFocused, backgroundShells, + settings.merged.ui?.compactToolOutput, ]); const pendingToolGroupItems = useMemo((): HistoryItemWithoutId[] => { @@ -538,8 +620,7 @@ export const useGeminiStream = ( toolCalls.length > 0 && toolCalls.every((tc) => pushedToolCallIds.has(tc.request.callId)); - const anyVisibleInHistory = pushedToolCallIds.size > 0; - const anyVisibleInPending = remainingTools.some((tc) => { + const isToolVisible = (tc: TrackedToolCall) => { // AskUser tools are rendered by AskUserDialog, not ToolGroupMessage const isInProgress = tc.status !== 'success' && @@ -553,12 +634,28 @@ export const useGeminiStream = ( tc.status !== 'validating' && tc.status !== 'awaiting_approval' ); - }); + }; + + const anyVisibleInHistory = pushedToolCallIds.size > 0; + const anyVisibleInPending = remainingTools.some(isToolVisible); + + let lastVisibleIsCompact = false; + const isCompactModeEnabled = settings.merged.ui?.compactToolOutput === true; + for (let i = toolCalls.length - 1; i >= 0; i--) { + if (isToolVisible(toolCalls[i])) { + const mapped = mapTrackedToolCallsToDisplay(toolCalls[i]); + lastVisibleIsCompact = mapped.tools[0] + ? isCompactTool(mapped.tools[0], isCompactModeEnabled) + : false; + break; + } + } if ( toolCalls.length > 0 && !(allTerminal && allPushed) && - (anyVisibleInHistory || anyVisibleInPending) + (anyVisibleInHistory || anyVisibleInPending) && + !lastVisibleIsCompact ) { items.push({ type: 'tool_group' as const, @@ -576,6 +673,7 @@ export const useGeminiStream = ( activeShellPtyId, isShellFocused, backgroundShells, + settings.merged.ui?.compactToolOutput, ]); const lastQueryRef = useRef(null); diff --git a/packages/cli/src/ui/hooks/useHistoryManager.test.ts b/packages/cli/src/ui/hooks/useHistoryManager.test.ts index 696f9d60c0..b657e5d4ba 100644 --- a/packages/cli/src/ui/hooks/useHistoryManager.test.ts +++ b/packages/cli/src/ui/hooks/useHistoryManager.test.ts @@ -39,6 +39,56 @@ describe('useHistoryManager', () => { expect(result.current.history[0].id).toBeGreaterThanOrEqual(timestamp); }); + it('should generate strictly increasing IDs even if baseTimestamp goes backwards', () => { + const { result } = renderHook(() => useHistory()); + const timestamp = 1000000; + const itemData: Omit = { type: 'info', text: 'First' }; + + let id1!: number; + let id2!: number; + + act(() => { + id1 = result.current.addItem(itemData, timestamp); + // Try to add with a smaller timestamp + id2 = result.current.addItem(itemData, timestamp - 500); + }); + + expect(id1).toBe(timestamp); + expect(id2).toBe(id1 + 1); + expect(result.current.history[1].id).toBe(id2); + }); + + it('should ensure new IDs start after existing IDs when resuming a session', () => { + const initialItems: HistoryItem[] = [ + { id: 5000, type: 'info', text: 'Existing' }, + ]; + const { result } = renderHook(() => useHistory({ initialItems })); + + let newId!: number; + act(() => { + // Try to add with a timestamp smaller than the highest existing ID + newId = result.current.addItem({ type: 'info', text: 'New' }, 2000); + }); + + expect(newId).toBe(5001); + expect(result.current.history[1].id).toBe(5001); + }); + + it('should update lastIdRef when loading new history', () => { + const { result } = renderHook(() => useHistory()); + + act(() => { + result.current.loadHistory([{ id: 8000, type: 'info', text: 'Loaded' }]); + }); + + let newId!: number; + act(() => { + newId = result.current.addItem({ type: 'info', text: 'New' }, 1000); + }); + + expect(newId).toBe(8001); + }); + it('should generate unique IDs for items added with the same base timestamp', () => { const { result } = renderHook(() => useHistory()); const timestamp = Date.now(); @@ -215,8 +265,8 @@ describe('useHistoryManager', () => { const after = Date.now(); expect(result.current.history).toHaveLength(1); - // ID should be >= before + 1 (since counter starts at 0 and increments to 1) - expect(result.current.history[0].id).toBeGreaterThanOrEqual(before + 1); + // ID should be >= before (since baseTimestamp defaults to Date.now()) + expect(result.current.history[0].id).toBeGreaterThanOrEqual(before); expect(result.current.history[0].id).toBeLessThanOrEqual(after + 1); }); diff --git a/packages/cli/src/ui/hooks/useHistoryManager.ts b/packages/cli/src/ui/hooks/useHistoryManager.ts index 93f7f01f28..c6ceabb920 100644 --- a/packages/cli/src/ui/hooks/useHistoryManager.ts +++ b/packages/cli/src/ui/hooks/useHistoryManager.ts @@ -42,16 +42,22 @@ export function useHistory({ initialItems?: HistoryItem[]; } = {}): UseHistoryManagerReturn { const [history, setHistory] = useState(initialItems); - const messageIdCounterRef = useRef(0); + const lastIdRef = useRef( + initialItems.reduce((max, item) => Math.max(max, item.id), 0), + ); - // Generates a unique message ID based on a timestamp and a counter. + // Generates a unique message ID based on a timestamp, ensuring it is always + // greater than any previously assigned ID. const getNextMessageId = useCallback((baseTimestamp: number): number => { - messageIdCounterRef.current += 1; - return baseTimestamp + messageIdCounterRef.current; + const nextId = Math.max(baseTimestamp, lastIdRef.current + 1); + lastIdRef.current = nextId; + return nextId; }, []); const loadHistory = useCallback((newHistory: HistoryItem[]) => { setHistory(newHistory); + const maxId = newHistory.reduce((max, item) => Math.max(max, item.id), 0); + lastIdRef.current = Math.max(lastIdRef.current, maxId); }, []); // Adds a new item to the history state with a unique ID. @@ -153,7 +159,7 @@ export function useHistory({ // Clears the entire history state and resets the ID counter. const clearItems = useCallback(() => { setHistory([]); - messageIdCounterRef.current = 0; + lastIdRef.current = 0; }, []); return useMemo( diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index 2f8e414a83..1d3924690f 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -16,13 +16,20 @@ import { type AgentDefinition, type ApprovalMode, type Kind, + type AnsiOutput, CoreToolCallStatus, checkExhaustive, } from '@google/gemini-cli-core'; import type { PartListUnion } from '@google/genai'; import { type ReactNode } from 'react'; -export type { ThoughtSummary, SkillDefinition }; +export { CoreToolCallStatus }; +export type { + ThoughtSummary, + SkillDefinition, + SerializableConfirmationDetails, + ToolResultDisplay, +}; export enum AuthState { // Attempting to authenticate or re-authenticate @@ -86,6 +93,88 @@ export function mapCoreStatusToDisplayStatus( } } +export interface DiffStat { + model_added_lines: number; + model_removed_lines: number; + model_added_chars: number; + model_removed_chars: number; + user_added_lines: number; + user_removed_lines: number; + user_added_chars: number; + user_removed_chars: number; +} + +export interface FileDiff { + fileDiff: string; + fileName: string; + filePath: string; + originalContent: string | null; + newContent: string; + diffStat?: DiffStat; + isNewFile?: boolean; +} + +export interface GrepMatch { + filePath: string; + lineNumber: number; + line: string; +} + +export interface GrepResult { + summary: string; + matches?: GrepMatch[]; + payload?: string; +} + +export interface ListDirectoryResult { + summary: string; + files?: string[]; + payload?: string; +} + +export interface ReadManyFilesResult { + summary: string; + files?: string[]; + skipped?: Array<{ path: string; reason: string }>; + include?: string[]; + excludes?: string[]; + targetDir?: string; + payload?: string; +} + +/** + * --- TYPE GUARDS --- + */ + +export const isFileDiff = (res: unknown): res is FileDiff => + typeof res === 'object' && res !== null && 'fileDiff' in res; + +export const isGrepResult = (res: unknown): res is GrepResult => + typeof res === 'object' && + res !== null && + 'summary' in res && + ('matches' in res || 'payload' in res); + +export const isListResult = ( + res: unknown, +): res is ListDirectoryResult | ReadManyFilesResult => + typeof res === 'object' && + res !== null && + 'summary' in res && + ('files' in res || 'include' in res); + +export const isTodoList = (res: unknown): res is { todos: unknown[] } => + typeof res === 'object' && res !== null && 'todos' in res; + +export const isAnsiOutput = (res: unknown): res is AnsiOutput => + Array.isArray(res) && (res.length === 0 || Array.isArray(res[0])); + +export const hasSummary = (res: unknown): res is { summary: string } => + typeof res === 'object' && + res !== null && + 'summary' in res && + typeof res.summary === 'string'; + export interface ToolCallEvent { type: 'tool_call'; status: CoreToolCallStatus; diff --git a/packages/cli/src/ui/utils/CodeColorizer.tsx b/packages/cli/src/ui/utils/CodeColorizer.tsx index 948a5f8988..4faed1892b 100644 --- a/packages/cli/src/ui/utils/CodeColorizer.tsx +++ b/packages/cli/src/ui/utils/CodeColorizer.tsx @@ -21,8 +21,8 @@ import { MaxSizedBox, MINIMUM_MAX_HEIGHT, } from '../components/shared/MaxSizedBox.js'; -import type { LoadedSettings } from '../../config/settings.js'; import { debugLogger } from '@google/gemini-cli-core'; +import type { LoadedSettings } from '../../config/settings.js'; // Configure theming and parsing utilities. const lowlight = createLowlight(common); @@ -117,7 +117,11 @@ export function colorizeLine( line: string, language: string | null, theme?: Theme, + disableColor = false, ): React.ReactNode { + if (disableColor) { + return {line}; + } const activeTheme = theme || themeManager.getActiveTheme(); return highlightAndRenderLine(line, language, activeTheme); } @@ -130,6 +134,8 @@ export interface ColorizeCodeOptions { theme?: Theme | null; settings: LoadedSettings; hideLineNumbers?: boolean; + disableColor?: boolean; + returnLines?: boolean; } /** @@ -146,13 +152,16 @@ export function colorizeCode({ theme = null, settings, hideLineNumbers = false, -}: ColorizeCodeOptions): React.ReactNode { + disableColor = false, + returnLines = false, +}: ColorizeCodeOptions): React.ReactNode | React.ReactNode[] { const codeToHighlight = code.replace(/\n$/, ''); const activeTheme = theme || themeManager.getActiveTheme(); const showLineNumbers = hideLineNumbers ? false : settings.merged.ui.showLineNumbers; + const useMaxSizedBox = !settings.merged.ui.useAlternateBuffer && !returnLines; try { // Render the HAST tree using the adapted theme // Apply the theme's default foreground color to the top-level Text element @@ -162,7 +171,7 @@ export function colorizeCode({ let hiddenLinesCount = 0; // Optimization to avoid highlighting lines that cannot possibly be displayed. - if (availableHeight !== undefined) { + if (availableHeight !== undefined && useMaxSizedBox) { availableHeight = Math.max(availableHeight, MINIMUM_MAX_HEIGHT); if (lines.length > availableHeight) { const sliceIndex = lines.length - availableHeight; @@ -172,11 +181,9 @@ export function colorizeCode({ } const renderedLines = lines.map((line, index) => { - const contentToRender = highlightAndRenderLine( - line, - language, - activeTheme, - ); + const contentToRender = disableColor + ? line + : highlightAndRenderLine(line, language, activeTheme); return ( @@ -188,19 +195,26 @@ export function colorizeCode({ alignItems="flex-start" justifyContent="flex-end" > - + {`${index + 1 + hiddenLinesCount}`} )} - + {contentToRender} ); }); - if (availableHeight !== undefined) { + if (returnLines) { + return renderedLines; + } + + if (useMaxSizedBox) { return ( - {`${index + 1}`} + + {`${index + 1}`} + )} - {stripAnsi(line)} + + {stripAnsi(line)} + )); - if (availableHeight !== undefined) { + if (returnLines) { + return fallbackLines; + } + + if (useMaxSizedBox) { return ( should render SVG snapsho │ ⊶ google_web_search │ │ │ │ Searching... │ -╰──────────────────────────────────────────────────────────────────────────────────────────────╯" +╰──────────────────────────────────────────────────────────────────────────────────────────────╯ +" `; exports[`MainContent tool group border SVG snapshots > should render SVG snapshot for a shell tool 1`] = ` @@ -25,7 +26,8 @@ exports[`MainContent tool group border SVG snapshots > should render SVG snapsho │ ⊶ run_shell_command │ │ │ │ Running command... │ -╰──────────────────────────────────────────────────────────────────────────────────────────────╯" +╰──────────────────────────────────────────────────────────────────────────────────────────────╯ +" `; exports[`MainContent tool group border SVG snapshots > should render SVG snapshot for an empty slice following a search tool 1`] = ` @@ -39,5 +41,6 @@ exports[`MainContent tool group border SVG snapshots > should render SVG snapsho │ ⊶ google_web_search │ │ │ │ Searching... │ -╰──────────────────────────────────────────────────────────────────────────────────────────────╯" +╰──────────────────────────────────────────────────────────────────────────────────────────────╯ +" `; diff --git a/packages/core/src/confirmation-bus/types.ts b/packages/core/src/confirmation-bus/types.ts index 91aeab8308..3c96fdaa19 100644 --- a/packages/core/src/confirmation-bus/types.ts +++ b/packages/core/src/confirmation-bus/types.ts @@ -8,6 +8,7 @@ import { type FunctionCall } from '@google/genai'; import type { ToolConfirmationOutcome, ToolConfirmationPayload, + DiffStat, } from '../tools/tools.js'; import type { ToolCall } from '../scheduler/types.js'; @@ -88,6 +89,7 @@ export type SerializableConfirmationDetails = originalContent: string | null; newContent: string; isModifying?: boolean; + diffStat?: DiffStat; } | { type: 'exec'; diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index bfa70565be..22d2bb3578 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -896,11 +896,33 @@ class EditToolInvocation DEFAULT_DIFF_OPTIONS, ); + // Determine the full content as originally proposed by the AI to ensure accurate diff stats. + let fullAiProposedContent = editData.newContent; + if ( + this.params.modified_by_user && + this.params.ai_proposed_content !== undefined + ) { + try { + const aiReplacement = await calculateReplacement(this.config, { + params: { + ...this.params, + new_string: this.params.ai_proposed_content, + }, + currentContent: editData.currentContent ?? '', + abortSignal: signal, + }); + fullAiProposedContent = aiReplacement.newContent; + } catch (_e) { + // Fallback to newContent if speculative calculation fails + fullAiProposedContent = editData.newContent; + } + } + const diffStat = getDiffStat( fileName, editData.currentContent ?? '', + fullAiProposedContent, editData.newContent, - this.params.new_string, ); displayResult = { fileDiff, diff --git a/packages/core/src/tools/grep-utils.ts b/packages/core/src/tools/grep-utils.ts index 2191588301..4d9ab6fb27 100644 --- a/packages/core/src/tools/grep-utils.ts +++ b/packages/core/src/tools/grep-utils.ts @@ -7,6 +7,7 @@ import fsPromises from 'node:fs/promises'; import { debugLogger } from '../utils/debugLogger.js'; import { MAX_LINE_LENGTH_TEXT_FILE } from '../utils/constants.js'; +import type { GrepResult } from './tools.js'; /** * Result object for a single grep match @@ -148,12 +149,17 @@ export async function formatGrepResults( }, searchLocationDescription: string, totalMaxMatches: number, -): Promise<{ llmContent: string; returnDisplay: string }> { +): Promise<{ llmContent: string; returnDisplay: GrepResult }> { const { pattern, names_only, include_pattern } = params; if (allMatches.length === 0) { const noMatchMsg = `No matches found for pattern "${pattern}" ${searchLocationDescription}${include_pattern ? ` (filter: "${include_pattern}")` : ''}.`; - return { llmContent: noMatchMsg, returnDisplay: `No matches found` }; + return { + llmContent: noMatchMsg, + returnDisplay: { + summary: `No matches found`, + }, + }; } const matchesByFile = groupMatchesByFile(allMatches); @@ -181,7 +187,9 @@ export async function formatGrepResults( llmContent += filePaths.join('\n'); return { llmContent: llmContent.trim(), - returnDisplay: `Found ${filePaths.length} files${wasTruncated ? ' (limited)' : ''}`, + returnDisplay: { + summary: `Found ${filePaths.length} files${wasTruncated ? ' (limited)' : ''}`, + }, }; } @@ -213,8 +221,15 @@ export async function formatGrepResults( return { llmContent: llmContent.trim(), - returnDisplay: `Found ${matchCount} ${matchTerm}${ - wasTruncated ? ' (limited)' : '' - }`, + returnDisplay: { + summary: `Found ${matchCount} ${matchTerm}${wasTruncated ? ' (limited)' : ''}`, + matches: allMatches + .filter((m) => !m.isContext) + .map((m) => ({ + filePath: m.filePath, + lineNumber: m.lineNumber, + line: m.line, + })), + }, }; } diff --git a/packages/core/src/tools/grep.test.ts b/packages/core/src/tools/grep.test.ts index 1f0a8ee98f..ea0d03065c 100644 --- a/packages/core/src/tools/grep.test.ts +++ b/packages/core/src/tools/grep.test.ts @@ -6,7 +6,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { GrepTool, type GrepToolParams } from './grep.js'; -import type { ToolResult } from './tools.js'; +import type { ToolResult, GrepResult } from './tools.js'; import path from 'node:path'; import { isSubpath } from '../utils/paths.js'; import fs from 'node:fs/promises'; @@ -180,7 +180,9 @@ describe('GrepTool', () => { `File: ${path.join('sub', 'fileC.txt')}`, ); expect(result.llmContent).toContain('L1: another world in sub dir'); - expect(result.returnDisplay).toBe('Found 3 matches'); + expect((result.returnDisplay as GrepResult)?.summary).toBe( + 'Found 3 matches', + ); }, 30000); it('should include files that start with ".." in JS fallback', async () => { @@ -221,7 +223,9 @@ describe('GrepTool', () => { ); expect(result.llmContent).toContain('File: fileC.txt'); // Path relative to 'sub' expect(result.llmContent).toContain('L1: another world in sub dir'); - expect(result.returnDisplay).toBe('Found 1 match'); + expect((result.returnDisplay as GrepResult)?.summary).toBe( + 'Found 1 match', + ); }, 30000); it('should find matches with an include glob', async () => { @@ -238,7 +242,9 @@ describe('GrepTool', () => { expect(result.llmContent).toContain( 'L2: function baz() { return "hello"; }', ); - expect(result.returnDisplay).toBe('Found 1 match'); + expect((result.returnDisplay as GrepResult)?.summary).toBe( + 'Found 1 match', + ); }, 30000); it('should find matches with an include glob and path', async () => { @@ -258,7 +264,9 @@ describe('GrepTool', () => { ); expect(result.llmContent).toContain('File: another.js'); expect(result.llmContent).toContain('L1: const greeting = "hello";'); - expect(result.returnDisplay).toBe('Found 1 match'); + expect((result.returnDisplay as GrepResult)?.summary).toBe( + 'Found 1 match', + ); }, 30000); it('should return "No matches found" when pattern does not exist', async () => { @@ -268,7 +276,9 @@ describe('GrepTool', () => { expect(result.llmContent).toContain( 'No matches found for pattern "nonexistentpattern" in the workspace directory.', ); - expect(result.returnDisplay).toBe('No matches found'); + expect((result.returnDisplay as GrepResult)?.summary).toBe( + 'No matches found', + ); }, 30000); it('should handle regex special characters correctly', async () => { @@ -480,7 +490,9 @@ describe('GrepTool', () => { expect(result.llmContent).toContain('L2: second line with world'); // And sub/fileC.txt should be excluded because limit reached expect(result.llmContent).not.toContain('File: sub/fileC.txt'); - expect(result.returnDisplay).toBe('Found 2 matches (limited)'); + expect((result.returnDisplay as GrepResult)?.summary).toBe( + 'Found 2 matches (limited)', + ); }); it('should respect max_matches_per_file in JS fallback', async () => { diff --git a/packages/core/src/tools/grep.ts b/packages/core/src/tools/grep.ts index ea202c57de..8d80b379b6 100644 --- a/packages/core/src/tools/grep.ts +++ b/packages/core/src/tools/grep.ts @@ -30,7 +30,7 @@ import { isGitRepository } from '../utils/gitUtils.js'; import type { Config } from '../config/config.js'; import type { FileExclusions } from '../utils/ignorePatterns.js'; import { ToolErrorType } from './tool-error.js'; -import { GREP_TOOL_NAME } from './tool-names.js'; +import { GREP_TOOL_NAME, GREP_DISPLAY_NAME } from './tool-names.js'; import { buildPatternArgsPattern } from '../policy/utils.js'; import { debugLogger } from '../utils/debugLogger.js'; import { GREP_DEFINITION } from './definitions/coreTools.js'; @@ -638,7 +638,7 @@ export class GrepTool extends BaseDeclarativeTool { ) { super( GrepTool.Name, - 'SearchText', + GREP_DISPLAY_NAME, GREP_DEFINITION.base.description!, Kind.Search, GREP_DEFINITION.base.parametersJsonSchema, diff --git a/packages/core/src/tools/ls.test.ts b/packages/core/src/tools/ls.test.ts index 5d728ad8a8..400f5c8b34 100644 --- a/packages/core/src/tools/ls.test.ts +++ b/packages/core/src/tools/ls.test.ts @@ -131,7 +131,10 @@ describe('LSTool', () => { expect(result.llmContent).toContain('[DIR] subdir'); expect(result.llmContent).toContain('file1.txt'); - expect(result.returnDisplay).toBe('Listed 2 item(s).'); + expect(result.returnDisplay).toEqual({ + summary: 'Listed 2 item(s).', + files: ['[DIR] subdir', 'file1.txt'], + }); }); it('should list files from secondary workspace directory', async () => { @@ -146,7 +149,10 @@ describe('LSTool', () => { const result = await invocation.execute(abortSignal); expect(result.llmContent).toContain('secondary-file.txt'); - expect(result.returnDisplay).toBe('Listed 1 item(s).'); + expect(result.returnDisplay).toEqual({ + summary: 'Listed 1 item(s).', + files: expect.any(Array), + }); }); it('should handle empty directories', async () => { @@ -156,7 +162,9 @@ describe('LSTool', () => { const result = await invocation.execute(abortSignal); expect(result.llmContent).toBe(`Directory ${emptyDir} is empty.`); - expect(result.returnDisplay).toBe('Directory is empty.'); + expect(result.returnDisplay).toEqual( + expect.objectContaining({ summary: 'Directory is empty.' }), + ); }); it('should respect ignore patterns', async () => { @@ -171,7 +179,10 @@ describe('LSTool', () => { expect(result.llmContent).toContain('file1.txt'); expect(result.llmContent).not.toContain('file2.log'); - expect(result.returnDisplay).toBe('Listed 1 item(s).'); + expect(result.returnDisplay).toEqual({ + summary: 'Listed 1 item(s).', + files: expect.any(Array), + }); }); it('should respect gitignore patterns', async () => { @@ -185,7 +196,9 @@ describe('LSTool', () => { expect(result.llmContent).toContain('file1.txt'); expect(result.llmContent).not.toContain('file2.log'); // .git is always ignored by default. - expect(result.returnDisplay).toBe('Listed 2 item(s). (2 ignored)'); + expect(result.returnDisplay).toEqual( + expect.objectContaining({ summary: 'Listed 2 item(s). (2 ignored)' }), + ); }); it('should respect geminiignore patterns', async () => { @@ -200,7 +213,9 @@ describe('LSTool', () => { expect(result.llmContent).toContain('file1.txt'); expect(result.llmContent).not.toContain('file2.log'); - expect(result.returnDisplay).toBe('Listed 2 item(s). (1 ignored)'); + expect(result.returnDisplay).toEqual( + expect.objectContaining({ summary: 'Listed 2 item(s). (1 ignored)' }), + ); }); it('should handle non-directory paths', async () => { @@ -211,7 +226,9 @@ describe('LSTool', () => { const result = await invocation.execute(abortSignal); expect(result.llmContent).toContain('Path is not a directory'); - expect(result.returnDisplay).toBe('Error: Path is not a directory.'); + expect(result.returnDisplay).toEqual( + expect.objectContaining({ summary: 'Error: Path is not a directory.' }), + ); expect(result.error?.type).toBe(ToolErrorType.PATH_IS_NOT_A_DIRECTORY); }); @@ -221,7 +238,11 @@ describe('LSTool', () => { const result = await invocation.execute(abortSignal); expect(result.llmContent).toContain('Error listing directory'); - expect(result.returnDisplay).toBe('Error: Failed to list directory.'); + expect(result.returnDisplay).toEqual( + expect.objectContaining({ + summary: 'Error: Failed to list directory.', + }), + ); expect(result.error?.type).toBe(ToolErrorType.LS_EXECUTION_ERROR); }); @@ -261,7 +282,11 @@ describe('LSTool', () => { expect(result.llmContent).toContain('Error listing directory'); expect(result.llmContent).toContain('permission denied'); - expect(result.returnDisplay).toBe('Error: Failed to list directory.'); + expect(result.returnDisplay).toEqual( + expect.objectContaining({ + summary: 'Error: Failed to list directory.', + }), + ); expect(result.error?.type).toBe(ToolErrorType.LS_EXECUTION_ERROR); }); @@ -287,7 +312,10 @@ describe('LSTool', () => { // Should still list the other files expect(result.llmContent).toContain('file1.txt'); expect(result.llmContent).not.toContain('problematic.txt'); - expect(result.returnDisplay).toBe('Listed 1 item(s).'); + expect(result.returnDisplay).toEqual({ + summary: 'Listed 1 item(s).', + files: expect.any(Array), + }); statSpy.mockRestore(); }); @@ -347,7 +375,10 @@ describe('LSTool', () => { const result = await invocation.execute(abortSignal); expect(result.llmContent).toContain('secondary-file.txt'); - expect(result.returnDisplay).toBe('Listed 1 item(s).'); + expect(result.returnDisplay).toEqual({ + summary: 'Listed 1 item(s).', + files: expect.any(Array), + }); }); }); diff --git a/packages/core/src/tools/ls.ts b/packages/core/src/tools/ls.ts index 1972392508..5bd18680ee 100644 --- a/packages/core/src/tools/ls.ts +++ b/packages/core/src/tools/ls.ts @@ -20,7 +20,7 @@ import { makeRelative, shortenPath } from '../utils/paths.js'; import type { Config } from '../config/config.js'; import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js'; import { ToolErrorType } from './tool-error.js'; -import { LS_TOOL_NAME } from './tool-names.js'; +import { LS_TOOL_NAME, LS_DISPLAY_NAME } from './tool-names.js'; import { buildDirPathArgsPattern } from '../policy/utils.js'; import { debugLogger } from '../utils/debugLogger.js'; import { LS_DEFINITION } from './definitions/coreTools.js'; @@ -143,8 +143,10 @@ class LSToolInvocation extends BaseToolInvocation { ): ToolResult { return { llmContent, - // Keep returnDisplay simpler in core logic - returnDisplay: `Error: ${returnDisplay}`, + // Return an object with summary for dense output support + returnDisplay: { + summary: `Error: ${returnDisplay}`, + }, error: { message: llmContent, type, @@ -169,7 +171,9 @@ class LSToolInvocation extends BaseToolInvocation { if (validationError) { return { llmContent: validationError, - returnDisplay: 'Path not in workspace.', + returnDisplay: { + summary: 'Path not in workspace.', + }, error: { message: validationError, type: ToolErrorType.PATH_NOT_IN_WORKSPACE, @@ -201,7 +205,9 @@ class LSToolInvocation extends BaseToolInvocation { // Changed error message to be more neutral for LLM return { llmContent: `Directory ${resolvedDirPath} is empty.`, - returnDisplay: `Directory is empty.`, + returnDisplay: { + summary: `Directory is empty.`, + }, }; } @@ -284,7 +290,12 @@ class LSToolInvocation extends BaseToolInvocation { return { llmContent: resultMessage, - returnDisplay: displayMessage, + returnDisplay: { + summary: displayMessage, + files: entries.map( + (entry) => `${entry.isDirectory ? '[DIR] ' : ''}${entry.name}`, + ), + }, }; } catch (error) { const errorMsg = `Error listing directory: ${error instanceof Error ? error.message : String(error)}`; @@ -309,7 +320,7 @@ export class LSTool extends BaseDeclarativeTool { ) { super( LSTool.Name, - 'ReadFolder', + LS_DISPLAY_NAME, LS_DEFINITION.base.description!, Kind.Search, LS_DEFINITION.base.parametersJsonSchema, diff --git a/packages/core/src/tools/read-many-files.test.ts b/packages/core/src/tools/read-many-files.test.ts index 6a526d2b62..dd9d146c97 100644 --- a/packages/core/src/tools/read-many-files.test.ts +++ b/packages/core/src/tools/read-many-files.test.ts @@ -31,6 +31,7 @@ import { import * as glob from 'glob'; import { createMockMessageBus } from '../test-utils/mock-message-bus.js'; import { GEMINI_IGNORE_FILE_NAME } from '../config/constants.js'; +import type { ReadManyFilesResult } from './tools.js'; vi.mock('glob', { spy: true }); @@ -277,7 +278,7 @@ describe('ReadManyFilesTool', () => { `--- ${expectedPath} ---\n\nContent of file1\n\n`, `\n--- End of content ---`, ]); - expect(result.returnDisplay).toContain( + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( 'Successfully read and concatenated content from **1 file(s)**', ); }); @@ -301,7 +302,7 @@ describe('ReadManyFilesTool', () => { c.includes(`--- ${expectedPath2} ---\n\nContent2\n\n`), ), ).toBe(true); - expect(result.returnDisplay).toContain( + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( 'Successfully read and concatenated content from **2 file(s)**', ); }); @@ -327,7 +328,7 @@ describe('ReadManyFilesTool', () => { ), ).toBe(true); expect(content.find((c) => c.includes('sub/data.json'))).toBeUndefined(); - expect(result.returnDisplay).toContain( + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( 'Successfully read and concatenated content from **2 file(s)**', ); }); @@ -347,7 +348,7 @@ describe('ReadManyFilesTool', () => { expect( content.find((c) => c.includes('src/main.test.ts')), ).toBeUndefined(); - expect(result.returnDisplay).toContain( + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( 'Successfully read and concatenated content from **1 file(s)**', ); }); @@ -359,7 +360,7 @@ describe('ReadManyFilesTool', () => { expect(result.llmContent).toEqual([ 'No files matching the criteria were found or all were skipped.', ]); - expect(result.returnDisplay).toContain( + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( 'No files were read and concatenated based on the criteria.', ); }); @@ -379,7 +380,7 @@ describe('ReadManyFilesTool', () => { expect( content.find((c) => c.includes('node_modules/some-lib/index.js')), ).toBeUndefined(); - expect(result.returnDisplay).toContain( + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( 'Successfully read and concatenated content from **1 file(s)**', ); }); @@ -406,7 +407,7 @@ describe('ReadManyFilesTool', () => { c.includes(`--- ${expectedPath2} ---\n\napp code\n\n`), ), ).toBe(true); - expect(result.returnDisplay).toContain( + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( 'Successfully read and concatenated content from **2 file(s)**', ); }); @@ -430,7 +431,7 @@ describe('ReadManyFilesTool', () => { }, '\n--- End of content ---', ]); - expect(result.returnDisplay).toContain( + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( 'Successfully read and concatenated content from **1 file(s)**', ); }); @@ -471,8 +472,10 @@ describe('ReadManyFilesTool', () => { c.includes(`--- ${expectedPath} ---\n\ntext notes\n\n`), ), ).toBe(true); - expect(result.returnDisplay).toContain('**Skipped 1 item(s):**'); - expect(result.returnDisplay).toContain( + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( + '**Skipped 1 item(s):**', + ); + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( '- `document.pdf` (Reason: asset file (image/pdf/audio) was not explicitly requested by name or extension)', ); }); @@ -516,9 +519,15 @@ describe('ReadManyFilesTool', () => { const params = { include: ['foo.bar', 'bar.ts', 'foo.quux'] }; const invocation = tool.build(params); const result = await invocation.execute(new AbortController().signal); - expect(result.returnDisplay).not.toContain('foo.bar'); - expect(result.returnDisplay).not.toContain('foo.quux'); - expect(result.returnDisplay).toContain('bar.ts'); + expect((result.returnDisplay as ReadManyFilesResult).files).not.toContain( + 'foo.bar', + ); + expect((result.returnDisplay as ReadManyFilesResult).files).not.toContain( + 'foo.quux', + ); + expect((result.returnDisplay as ReadManyFilesResult).files).toContain( + 'bar.ts', + ); }); it('should read files from multiple workspace directories', async () => { @@ -594,7 +603,7 @@ describe('ReadManyFilesTool', () => { c.includes(`--- ${expectedPath2} ---\n\nContent2\n\n`), ), ).toBe(true); - expect(result.returnDisplay).toContain( + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( 'Successfully read and concatenated content from **2 file(s)**', ); @@ -646,7 +655,7 @@ Content of receive-detail `, `\n--- End of content ---`, ]); - expect(result.returnDisplay).toContain( + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( 'Successfully read and concatenated content from **1 file(s)**', ); }); @@ -665,7 +674,7 @@ Content of file[1] `, `\n--- End of content ---`, ]); - expect(result.returnDisplay).toContain( + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( 'Successfully read and concatenated content from **1 file(s)**', ); }); @@ -764,7 +773,9 @@ Content of file[1] // Should successfully process valid files despite one failure expect(content.length).toBeGreaterThanOrEqual(3); - expect(result.returnDisplay).toContain('Successfully read'); + expect((result.returnDisplay as ReadManyFilesResult).summary).toContain( + 'Successfully read', + ); // Verify valid files were processed const expectedPath1 = path.join(tempRootDir, 'valid1.txt'); diff --git a/packages/core/src/tools/read-many-files.ts b/packages/core/src/tools/read-many-files.ts index e2a283c726..d81787defd 100644 --- a/packages/core/src/tools/read-many-files.ts +++ b/packages/core/src/tools/read-many-files.ts @@ -13,6 +13,7 @@ import { type ToolResult, type PolicyUpdateOptions, type ToolConfirmationOutcome, + type ReadManyFilesResult, } from './tools.js'; import { getErrorMessage } from '../utils/errors.js'; import * as fsPromises from 'node:fs/promises'; @@ -36,7 +37,10 @@ import { getProgrammingLanguage } from '../telemetry/telemetry-utils.js'; import { logFileOperation } from '../telemetry/loggers.js'; import { FileOperationEvent } from '../telemetry/types.js'; import { ToolErrorType } from './tool-error.js'; -import { READ_MANY_FILES_TOOL_NAME } from './tool-names.js'; +import { + READ_MANY_FILES_TOOL_NAME, + READ_MANY_FILES_DISPLAY_NAME, +} from './tool-names.js'; import { READ_MANY_FILES_DEFINITION } from './definitions/coreTools.js'; import { resolveToolDeclaration } from './definitions/resolver.js'; @@ -269,7 +273,9 @@ ${finalExclusionPatternsForDescription const errorMessage = `Error during file search: ${getErrorMessage(error)}`; return { llmContent: errorMessage, - returnDisplay: `## File Search Error\n\nAn error occurred while searching for files:\n\`\`\`\n${getErrorMessage(error)}\n\`\`\``, + returnDisplay: { + summary: `Error: ${getErrorMessage(error)}`, + }, error: { message: errorMessage, type: ToolErrorType.READ_MANY_FILES_SEARCH_ERROR, @@ -483,9 +489,19 @@ ${finalExclusionPatternsForDescription 'No files matching the criteria were found or all were skipped.', ); } + + const returnDisplay: ReadManyFilesResult = { + summary: displayMessage.trim(), + files: processedFilesRelativePaths, + skipped: skippedFiles, + include: this.params.include, + excludes: effectiveExcludes, + targetDir: this.config.getTargetDir(), + }; + return { llmContent: contentParts, - returnDisplay: displayMessage.trim(), + returnDisplay, }; } } @@ -507,7 +523,7 @@ export class ReadManyFilesTool extends BaseDeclarativeTool< ) { super( ReadManyFilesTool.Name, - 'ReadManyFiles', + READ_MANY_FILES_DISPLAY_NAME, READ_MANY_FILES_DEFINITION.base.description!, Kind.Read, READ_MANY_FILES_DEFINITION.base.parametersJsonSchema, diff --git a/packages/core/src/tools/ripGrep.test.ts b/packages/core/src/tools/ripGrep.test.ts index a1b155fb7a..4481bf3e54 100644 --- a/packages/core/src/tools/ripGrep.test.ts +++ b/packages/core/src/tools/ripGrep.test.ts @@ -19,6 +19,7 @@ import { ensureRgPath, type RipGrepToolParams, } from './ripGrep.js'; +import type { GrepResult } from './tools.js'; import path from 'node:path'; import { isSubpath } from '../utils/paths.js'; import fs from 'node:fs/promises'; @@ -447,7 +448,9 @@ describe('RipGrepTool', () => { `File: ${path.join('sub', 'fileC.txt')}`, ); expect(result.llmContent).toContain('L1: another world in sub dir'); - expect(result.returnDisplay).toBe('Found 3 matches'); + expect((result.returnDisplay as GrepResult).summary).toBe( + 'Found 3 matches', + ); }); it('should ignore matches that escape the base path', async () => { @@ -509,7 +512,9 @@ describe('RipGrepTool', () => { ); expect(result.llmContent).toContain('File: fileC.txt'); // Path relative to 'sub' expect(result.llmContent).toContain('L1: another world in sub dir'); - expect(result.returnDisplay).toBe('Found 1 match'); + expect((result.returnDisplay as GrepResult).summary).toBe( + 'Found 1 match', + ); }); it('should find matches with an include glob', async () => { @@ -542,7 +547,9 @@ describe('RipGrepTool', () => { expect(result.llmContent).toContain( 'L2: function baz() { return "hello"; }', ); - expect(result.returnDisplay).toBe('Found 1 match'); + expect((result.returnDisplay as GrepResult).summary).toBe( + 'Found 1 match', + ); }); it('should find matches with an include glob and path', async () => { @@ -579,7 +586,9 @@ describe('RipGrepTool', () => { ); expect(result.llmContent).toContain('File: another.js'); expect(result.llmContent).toContain('L1: const greeting = "hello";'); - expect(result.returnDisplay).toBe('Found 1 match'); + expect((result.returnDisplay as GrepResult).summary).toBe( + 'Found 1 match', + ); }); it('should return "No matches found" when pattern does not exist', async () => { @@ -596,7 +605,9 @@ describe('RipGrepTool', () => { expect(result.llmContent).toContain( 'No matches found for pattern "nonexistentpattern" in path ".".', ); - expect(result.returnDisplay).toBe('No matches found'); + expect((result.returnDisplay as GrepResult).summary).toBe( + 'No matches found', + ); }); it('should throw error for invalid regex pattern during build', async () => { @@ -689,7 +700,9 @@ describe('RipGrepTool', () => { }); const result = await invocation.execute(abortSignal); - expect(result.returnDisplay).toContain('(limited)'); + expect((result.returnDisplay as GrepResult).summary).toContain( + '(limited)', + ); }, 10000); it('should filter out files based on FileDiscoveryService even if ripgrep returns them', async () => { @@ -740,7 +753,9 @@ describe('RipGrepTool', () => { expect(result.llmContent).toContain('should be kept'); expect(result.llmContent).not.toContain('ignored.txt'); expect(result.llmContent).not.toContain('should be ignored'); - expect(result.returnDisplay).toContain('Found 1 match'); + expect((result.returnDisplay as GrepResult).summary).toContain( + 'Found 1 match', + ); }); it('should handle regex special characters correctly', async () => { @@ -1064,7 +1079,9 @@ describe('RipGrepTool', () => { controller.abort(); const result = await invocation.execute(controller.signal); - expect(result.returnDisplay).toContain('No matches found'); + expect((result.returnDisplay as GrepResult).summary).toContain( + 'No matches found', + ); }); }); @@ -1946,7 +1963,9 @@ describe('RipGrepTool', () => { expect(result.llmContent).toContain('L1: match 1'); expect(result.llmContent).toContain('L2: match 2'); expect(result.llmContent).not.toContain('L3: match 3'); - expect(result.returnDisplay).toBe('Found 2 matches (limited)'); + expect((result.returnDisplay as GrepResult).summary).toBe( + 'Found 2 matches (limited)', + ); }); it('should return only file paths when names_only is true', async () => { diff --git a/packages/core/src/tools/tool-names.ts b/packages/core/src/tools/tool-names.ts index e818881662..5f89719abe 100644 --- a/packages/core/src/tools/tool-names.ts +++ b/packages/core/src/tools/tool-names.ts @@ -183,6 +183,11 @@ export const EDIT_DISPLAY_NAME = 'Edit'; export const ASK_USER_DISPLAY_NAME = 'Ask User'; export const READ_FILE_DISPLAY_NAME = 'ReadFile'; export const GLOB_DISPLAY_NAME = 'FindFiles'; +export const LS_DISPLAY_NAME = 'ReadFolder'; +export const GREP_DISPLAY_NAME = 'SearchText'; +export const WEB_SEARCH_DISPLAY_NAME = 'GoogleSearch'; +export const WEB_FETCH_DISPLAY_NAME = 'WebFetch'; +export const READ_MANY_FILES_DISPLAY_NAME = 'ReadManyFiles'; /** * Mapping of legacy tool names to their current names. diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index 3865aaf357..2fa9858e5d 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -816,12 +816,53 @@ export interface TodoList { export type ToolLiveOutput = string | AnsiOutput | SubagentProgress; +export interface StructuredToolResult { + summary: string; +} + +export function isStructuredToolResult( + obj: unknown, +): obj is StructuredToolResult { + return ( + typeof obj === 'object' && + obj !== null && + 'summary' in obj && + typeof obj.summary === 'string' + ); +} + +export interface GrepResult extends StructuredToolResult { + matches?: Array<{ + filePath: string; + lineNumber: number; + line: string; + }>; + payload?: string; +} + +export interface ListDirectoryResult extends StructuredToolResult { + files?: string[]; + payload?: string; +} + +export interface ReadManyFilesResult extends StructuredToolResult { + files?: string[]; + skipped?: Array<{ path: string; reason: string }>; + include?: string[]; + excludes?: string[]; + targetDir?: string; + payload?: string; +} + export type ToolResultDisplay = | string | FileDiff | AnsiOutput | TodoList - | SubagentProgress; + | SubagentProgress + | GrepResult + | ListDirectoryResult + | ReadManyFilesResult; export type TodoStatus = | 'pending' diff --git a/packages/core/src/tools/web-fetch.ts b/packages/core/src/tools/web-fetch.ts index 27a60c4259..4a7bf6554e 100644 --- a/packages/core/src/tools/web-fetch.ts +++ b/packages/core/src/tools/web-fetch.ts @@ -30,7 +30,7 @@ import { NetworkRetryAttemptEvent, } from '../telemetry/index.js'; import { LlmRole } from '../telemetry/llmRole.js'; -import { WEB_FETCH_TOOL_NAME } from './tool-names.js'; +import { WEB_FETCH_TOOL_NAME, WEB_FETCH_DISPLAY_NAME } from './tool-names.js'; import { debugLogger } from '../utils/debugLogger.js'; import { coreEvents } from '../utils/events.js'; import { retryWithBackoff, getRetryErrorType } from '../utils/retry.js'; @@ -891,7 +891,7 @@ export class WebFetchTool extends BaseDeclarativeTool< ) { super( WebFetchTool.Name, - 'WebFetch', + WEB_FETCH_DISPLAY_NAME, WEB_FETCH_DEFINITION.base.description!, Kind.Fetch, WEB_FETCH_DEFINITION.base.parametersJsonSchema, diff --git a/packages/core/src/tools/web-search.ts b/packages/core/src/tools/web-search.ts index 18132d2c35..2a29291437 100644 --- a/packages/core/src/tools/web-search.ts +++ b/packages/core/src/tools/web-search.ts @@ -5,7 +5,7 @@ */ import type { MessageBus } from '../confirmation-bus/message-bus.js'; -import { WEB_SEARCH_TOOL_NAME } from './tool-names.js'; +import { WEB_SEARCH_TOOL_NAME, WEB_SEARCH_DISPLAY_NAME } from './tool-names.js'; import type { GroundingMetadata } from '@google/genai'; import { BaseDeclarativeTool, @@ -212,7 +212,7 @@ export class WebSearchTool extends BaseDeclarativeTool< ) { super( WebSearchTool.Name, - 'GoogleSearch', + WEB_SEARCH_DISPLAY_NAME, WEB_SEARCH_DEFINITION.base.description!, Kind.Search, WEB_SEARCH_DEFINITION.base.parametersJsonSchema, diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index f85a39bb35..d58474f497 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -317,6 +317,13 @@ "default": true, "type": "boolean" }, + "compactToolOutput": { + "title": "Compact Tool Output", + "description": "Display tool outputs (like directory listings and file reads) in a compact, structured format.", + "markdownDescription": "Display tool outputs (like directory listings and file reads) in a compact, structured format.\n\n- Category: `UI`\n- Requires restart: `no`\n- Default: `false`", + "default": false, + "type": "boolean" + }, "hideBanner": { "title": "Hide Banner", "description": "Hide the application banner",