From 77a5f3409b92f0e7fa0d23873a202ffe63c4d4d2 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. ============================================ Rebase Cleanup cleanup tasks - remove redundant tool alias; simplify core tool error paths and data structures - update default compact tool success summary text ("Returned") - refactor ToolGroupMessage logic & border transitions across Compact, Standard, and Subagent output types feat(ui): make result summary the *diff toggle target* and refine styling in dense output - Makes the entire result summary (action word + stats) the clickable trigger for diff visibility. - Styles the action word (e.g., "Accepted") with an underline to indicate interactivity. - Removes the redundant "[click to show/hide details]" text to clean up the UI. - Updates tests and snapshots to reflect the new interaction model. fix(core): derive and preserve *diffstat* info for rejected edit operations - Adds getDiffStatFromPatch utility to derive numeric stats from unified diff strings. - Updates ToolEditConfirmationDetails to include an optional diffStat property. - Modifies StateManager and CoreToolScheduler to populate diffStat when an edit/write is cancelled. - Ensures the UI can display rich information (e.g., "+5, -2") for rejected operations. - Adds a core test to verify correct stats derivation on tool rejection. <<-- INTEGRATION OF SIGNIFICANT SUBAGENT CHANGES -->> refactor(cli): consolidate pendingHistoryItems and remove shadowing in AppContainer refactor(cli): refine tool layout and modernize test infrastructure - fix(ui): remove default top margin (1 -> 0) for the first tool in a group - Aligns with dense output goal of reducing vertical whitespace. - Updates ToolGroupMessage.tsx to default borderTopOverride to false for margin calculation. - refactor(test): modernize renderWithProviders and update affected tests - Removes the legacy useAlternateBuffer prop from renderWithProviders. - Shifts alternate buffer configuration to standard config and settings mocks for better consistency. - Updates DenseToolMessage.test.tsx and ShellToolMessage.test.tsx to use the new pattern. - Wraps waitUntilReady and waitFor calls in act to resolve React state update warnings. - fix(ui): improve AskUserDialog layout and scroll visibility - Refines maxItemsToShow logic to properly account for available terminal height. - Ensures scroll arrows are correctly visible in alternate buffer mode. - test(ui): update snapshots and assertions for tool layout regression tests - Updates ToolStickyHeaderRegression.test.tsx assertions to match the new 0-line margin layout. - Regenerates snapshots for AskUserDialog, DenseToolMessage, and ToolStickyHeaderRegression. fix(ui): correct shell height calculation and modernize test utilities - Update TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT to 3 to match actual header overhead. - Fix renderWithProviders to correctly respect useAlternateBuffer and dynamically load config. - Ensure all tests await renderWithProviders for reliable component initialization. - Refactor height calculation in tests to use layout constants and fix off-by-one errors. - Add fake timer support to ShellToolMessage tests to resolve act() warnings from spinners. - Refresh snapshots for MainContent, ShellToolMessage, and ToolResultDisplay. refactor(cli): reduce code duplication while addressing /review-frontend feedback fix(cli): resolve IDE client race condition and memory leak within `useEffect` hook in `ToolActionsContext.tsx`. fix(cli): improve type safety for colorizeCode - Adds function overloads to `colorizeCode` in `CodeColorizer.tsx` to correctly type the return value when `returnLines` is enabled. - Removes unsafe `as React.ReactNode[]` cast and associated ESLint disable comment in `DenseToolMessage.tsx`. - Adds a unit test in `CodeColorizer.test.tsx` to verify the `returnLines: true` contract. --- 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 | 42 +- packages/cli/src/ui/AppContainer.tsx | 149 +++-- .../src/ui/components/AskUserDialog.test.tsx | 2 +- .../cli/src/ui/components/AskUserDialog.tsx | 10 +- .../src/ui/components/MainContent.test.tsx | 12 +- .../cli/src/ui/components/MainContent.tsx | 12 +- ...ternateBufferQuittingDisplay.test.tsx.snap | 3 + .../__snapshots__/AskUserDialog.test.tsx.snap | 27 +- .../__snapshots__/MainContent.test.tsx.snap | 9 +- .../messages/DenseToolMessage.test.tsx | 513 +++++++++++++++ .../components/messages/DenseToolMessage.tsx | 585 ++++++++++++++++++ .../components/messages/DiffRenderer.test.tsx | 3 + .../ui/components/messages/DiffRenderer.tsx | 151 +++-- .../messages/ShellToolMessage.test.tsx | 92 +-- .../messages/ToolConfirmationMessage.test.tsx | 15 + .../ToolGroupMessage.compact.test.tsx | 175 ++++++ .../components/messages/ToolGroupMessage.tsx | 443 +++++++++---- .../components/messages/ToolResultDisplay.tsx | 24 +- .../ToolStickyHeaderRegression.test.tsx | 6 +- .../DenseToolMessage.test.tsx.snap | 135 ++++ .../ShellToolMessage.test.tsx.snap | 15 +- .../ToolGroupMessage.compact.test.tsx.snap | 35 ++ .../ToolGroupMessage.test.tsx.snap | 24 +- .../ToolResultDisplay.test.tsx.snap | 3 +- .../ToolStickyHeaderRegression.test.tsx.snap | 8 +- packages/cli/src/ui/constants.ts | 3 + .../ui/contexts/ToolActionsContext.test.tsx | 126 +++- .../src/ui/contexts/ToolActionsContext.tsx | 50 +- 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 ++- .../cli/src/ui/utils/CodeColorizer.test.tsx | 24 + packages/cli/src/ui/utils/CodeColorizer.tsx | 56 +- .../__snapshots__/borderStyles.test.tsx.snap | 9 +- packages/cli/src/ui/utils/confirmingTool.ts | 6 +- packages/cli/src/ui/utils/historyUtils.ts | 83 +++ .../cli/src/ui/utils/toolLayoutUtils.test.ts | 20 +- packages/cli/src/ui/utils/toolLayoutUtils.ts | 2 +- packages/core/src/confirmation-bus/types.ts | 2 + packages/core/src/core/coreToolScheduler.ts | 7 + .../core/src/scheduler/state-manager.test.ts | 24 +- packages/core/src/scheduler/state-manager.ts | 3 + packages/core/src/tools/diffOptions.ts | 36 ++ 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 | 33 +- packages/core/src/tools/ls.ts | 12 +- .../core/src/tools/read-many-files.test.ts | 45 +- packages/core/src/tools/read-many-files.ts | 22 +- packages/core/src/tools/ripGrep.test.ts | 37 +- packages/core/src/tools/tool-names.ts | 7 +- packages/core/src/tools/tools.ts | 44 +- packages/core/src/tools/web-fetch.ts | 4 +- packages/core/src/tools/web-search.ts | 4 +- schemas/settings.schema.json | 7 + 62 files changed, 3031 insertions(+), 536 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 create mode 100644 packages/cli/src/ui/utils/historyUtils.ts diff --git a/docs/cli/settings.md b/docs/cli/settings.md index 9b08867cc4..3a65408d83 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 f57fd40747..05a494963c 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` @@ -2106,7 +2111,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 bd5a52f126..7815f9ba42 100644 --- a/packages/cli/src/acp/acpClient.ts +++ b/packages/cli/src/acp/acpClient.ts @@ -97,6 +97,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; @@ -837,7 +839,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(); @@ -1310,7 +1312,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 ea6b9f9239..a8a60981ea 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 7d298b120d..f417f5499a 100644 --- a/packages/cli/src/test-utils/render.tsx +++ b/packages/cli/src/test-utils/render.tsx @@ -16,7 +16,7 @@ import { vi } from 'vitest'; import stripAnsi from 'strip-ansi'; import type React from 'react'; import { act, useState } from 'react'; -import type { LoadedSettings } from '../config/settings.js'; +import { LoadedSettings } from '../config/settings.js'; import { KeypressProvider } from '../ui/contexts/KeypressContext.js'; import { SettingsContext } from '../ui/contexts/SettingsContext.js'; import { ShellFocusContext } from '../ui/contexts/ShellFocusContext.js'; @@ -604,16 +604,18 @@ export const renderWithProviders = async ( uiState: providedUiState, width, mouseEventsEnabled = false, + useAlternateBuffer: explicitUseAlternateBuffer, config, uiActions, persistentState, appState = mockAppState, }: { shellFocus?: boolean; - settings?: LoadedSettings; + settings?: LoadedSettings | Partial; uiState?: Partial; width?: number; mouseEventsEnabled?: boolean; + useAlternateBuffer?: boolean; config?: Config; uiActions?: Partial; persistentState?: { @@ -660,15 +662,34 @@ export const renderWithProviders = async ( const terminalWidth = width ?? baseState.terminalWidth; + const finalSettings = + settings instanceof LoadedSettings + ? settings + : createMockSettings(settings || {}); + if (!config) { config = await loadCliConfig( - settings.merged, + finalSettings.merged, 'random-session-id', - {} as unknown as CliArgs, + {} as CliArgs, { cwd: '/' }, ); } + const useAlternateBuffer = + explicitUseAlternateBuffer ?? + finalSettings.merged.ui?.useAlternateBuffer ?? + false; + + const finalConfig = new Proxy(config, { + get(target, prop) { + if (prop === 'getUseAlternateBuffer') { + return () => useAlternateBuffer; + } + return Reflect.get(target, prop); + }, + }); + const mainAreaWidth = terminalWidth; const finalUiState = { @@ -697,8 +718,8 @@ export const renderWithProviders = async ( const wrapWithProviders = (comp: React.ReactElement) => ( - - + + @@ -709,8 +730,11 @@ export const renderWithProviders = async ( ( wrapper?: React.ComponentType<{ children: React.ReactNode }>; // Options for renderWithProviders shellFocus?: boolean; - settings?: LoadedSettings; + settings?: LoadedSettings | Partial; uiState?: Partial; width?: number; mouseEventsEnabled?: boolean; @@ -864,7 +888,7 @@ export async function renderHookWithProviders( const Wrapper = options.wrapper || (({ children }) => <>{children}); - let renderResult: ReturnType; + let renderResult: Awaited>; await act(async () => { renderResult = await renderWithProviders( diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 9d05f54347..4190977c95 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -30,8 +30,6 @@ import { import { ConfigContext } from './contexts/ConfigContext.js'; import { type HistoryItem, - type HistoryItemWithoutId, - type HistoryItemToolGroup, AuthState, type ConfirmationRequest, type PermissionConfirmationRequest, @@ -81,7 +79,6 @@ import { type AgentsDiscoveredPayload, ChangeAuthRequestedError, ProjectIdRequiredError, - CoreToolCallStatus, buildUserSteeringHintPrompt, logBillingEvent, ApiKeyUpdatedEvent, @@ -171,29 +168,6 @@ import { useSuspend } from './hooks/useSuspend.js'; import { useRunEventNotifications } from './hooks/useRunEventNotifications.js'; import { isNotificationsEnabled } from '../utils/terminalNotifications.js'; -function isToolExecuting(pendingHistoryItems: HistoryItemWithoutId[]) { - return pendingHistoryItems.some((item) => { - if (item && item.type === 'tool_group') { - return item.tools.some( - (tool) => CoreToolCallStatus.Executing === tool.status, - ); - } - return false; - }); -} - -function isToolAwaitingConfirmation( - pendingHistoryItems: HistoryItemWithoutId[], -) { - return pendingHistoryItems - .filter((item): item is HistoryItemToolGroup => item.type === 'tool_group') - .some((item) => - item.tools.some( - (tool) => CoreToolCallStatus.AwaitingApproval === tool.status, - ), - ); -} - interface AppContainerProps { config: Config; startupWarnings?: StartupWarning[]; @@ -208,6 +182,12 @@ import { APPROVAL_MODE_REVEAL_DURATION_MS, } from './hooks/useVisibilityToggle.js'; import { useKeyMatchers } from './hooks/useKeyMatchers.js'; +import { + getLastTurnToolCallIds, + isToolExecuting, + isToolAwaitingConfirmation, + getAllToolCalls, +} from './utils/historyUtils.js'; /** * The fraction of the terminal width to allocate to the shell. @@ -259,6 +239,39 @@ export const AppContainer = (props: AppContainerProps) => { 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); @@ -1173,6 +1186,11 @@ Logging in with Google... Restarting Gemini CLI to continue. setIsBackgroundShellListOpenRef.current = setIsBackgroundShellListOpen; + const pendingHistoryItems = useMemo( + () => [...pendingSlashCommandHistoryItems, ...pendingGeminiHistoryItems], + [pendingSlashCommandHistoryItems, pendingGeminiHistoryItems], + ); + const lastOutputTimeRef = useRef(0); useEffect(() => { @@ -1222,10 +1240,6 @@ Logging in with Google... Restarting Gemini CLI to continue. cancelHandlerRef.current = useCallback( (shouldRestorePrompt: boolean = true) => { - const pendingHistoryItems = [ - ...pendingSlashCommandHistoryItems, - ...pendingGeminiHistoryItems, - ]; if (isToolAwaitingConfirmation(pendingHistoryItems)) { return; // Don't clear - user may be composing a follow-up message } @@ -1259,8 +1273,7 @@ Logging in with Google... Restarting Gemini CLI to continue. inputHistory, getQueuedMessagesText, clearQueue, - pendingSlashCommandHistoryItems, - pendingGeminiHistoryItems, + pendingHistoryItems, ], ); @@ -1296,10 +1309,7 @@ Logging in with Google... Restarting Gemini CLI to continue. const isIdle = streamingState === StreamingState.Idle; const isAgentRunning = streamingState === StreamingState.Responding || - isToolExecuting([ - ...pendingSlashCommandHistoryItems, - ...pendingGeminiHistoryItems, - ]); + isToolExecuting(pendingHistoryItems); if (isSlash && isAgentRunning) { const { commandToExecute } = parseSlashCommand( @@ -1361,8 +1371,7 @@ Logging in with Google... Restarting Gemini CLI to continue. isMcpReady, streamingState, messageQueue.length, - pendingSlashCommandHistoryItems, - pendingGeminiHistoryItems, + pendingHistoryItems, config, constrainHeight, setConstrainHeight, @@ -1684,6 +1693,11 @@ Logging in with Google... Restarting Gemini CLI to continue. 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,13 +1731,26 @@ Logging in with Google... Restarting Gemini CLI to continue. return true; } + const toggleLastTurnTools = () => { + // If the user manually collapses/expands the view, show the hint and reset the x-second timer. + triggerExpandHint(true); + + const targetToolCallIds = getLastTurnToolCallIds( + historyManager.history, + pendingHistoryItems, + ); + + if (targetToolCallIds.length > 0) { + toggleAllExpansion(targetToolCallIds); + } + }; + let enteringConstrainHeightMode = false; if (!constrainHeight) { enteringConstrainHeightMode = true; setConstrainHeight(true); 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); + toggleLastTurnTools(); } if (!isAlternateBuffer) { refreshStatic(); @@ -1771,11 +1798,15 @@ Logging in with Google... Restarting Gemini CLI to continue. !enteringConstrainHeightMode ) { setConstrainHeight(false); - // If the user manually expands the view, show the hint and reset the x-second timer. - triggerExpandHint(true); - if (!isAlternateBuffer) { + toggleLastTurnTools(); + + // 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) || @@ -1862,6 +1893,7 @@ Logging in with Google... Restarting Gemini CLI to continue. activePtyId, handleSuspend, embeddedShellFocused, + settings.merged.general.debugKeystrokeLogging, refreshStatic, setCopyModeEnabled, tabFocusTimeoutRef, @@ -1879,6 +1911,9 @@ Logging in with Google... Restarting Gemini CLI to continue. triggerExpandHint, keyMatchers, isHelpDismissKey, + historyManager.history, + pendingHistoryItems, + toggleAllExpansion, ], ); @@ -2022,11 +2057,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], @@ -2121,12 +2151,7 @@ Logging in with Google... Restarting Gemini CLI to continue. ]); const allToolCalls = useMemo( - () => - pendingHistoryItems - .filter( - (item): item is HistoryItemToolGroup => item.type === 'tool_group', - ) - .flatMap((item) => item.tools), + () => getAllToolCalls(pendingHistoryItems), [pendingHistoryItems], ); @@ -2291,11 +2316,7 @@ Logging in with Google... Restarting Gemini CLI to continue. newAgents, showIsExpandableHint, hintMode: - config.isModelSteeringEnabled() && - isToolExecuting([ - ...pendingSlashCommandHistoryItems, - ...pendingGeminiHistoryItems, - ]), + config.isModelSteeringEnabled() && isToolExecuting(pendingHistoryItems), hintBuffer: '', }), [ @@ -2597,7 +2618,13 @@ Logging in with Google... Restarting Gemini CLI to continue. startupWarnings: props.startupWarnings || [], }} > - + diff --git a/packages/cli/src/ui/components/AskUserDialog.test.tsx b/packages/cli/src/ui/components/AskUserDialog.test.tsx index 8ed240389c..8369f4ec49 100644 --- a/packages/cli/src/ui/components/AskUserDialog.test.tsx +++ b/packages/cli/src/ui/components/AskUserDialog.test.tsx @@ -288,7 +288,7 @@ describe('AskUserDialog', () => { }); describe.each([ - { useAlternateBuffer: true, expectedArrows: false }, + { useAlternateBuffer: true, expectedArrows: true }, { useAlternateBuffer: false, expectedArrows: true }, ])( 'Scroll Arrows (useAlternateBuffer: $useAlternateBuffer)', diff --git a/packages/cli/src/ui/components/AskUserDialog.tsx b/packages/cli/src/ui/components/AskUserDialog.tsx index b1d23885e6..f845c3dcfd 100644 --- a/packages/cli/src/ui/components/AskUserDialog.tsx +++ b/packages/cli/src/ui/components/AskUserDialog.tsx @@ -857,8 +857,14 @@ const ChoiceQuestionView: React.FC = ({ : undefined; const maxItemsToShow = - listHeight && questionHeightLimit - ? Math.max(1, Math.floor((listHeight - questionHeightLimit) / 2)) + listHeight && (!isAlternateBuffer || availableHeight !== undefined) + ? Math.min( + selectionItems.length, + Math.max( + 1, + Math.floor((listHeight - (questionHeightLimit ?? 0)) / 2), + ), + ) : selectionItems.length; return ( diff --git a/packages/cli/src/ui/components/MainContent.test.tsx b/packages/cli/src/ui/components/MainContent.test.tsx index b2c18aa7d8..66ebc772e9 100644 --- a/packages/cli/src/ui/components/MainContent.test.tsx +++ b/packages/cli/src/ui/components/MainContent.test.tsx @@ -97,7 +97,7 @@ describe('getToolGroupBorderAppearance', () => { }); 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..414523f367 100644 --- a/packages/cli/src/ui/components/__snapshots__/AlternateBufferQuittingDisplay.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/AlternateBufferQuittingDisplay.test.tsx.snap @@ -37,10 +37,12 @@ Tips for getting started: │ ✓ tool1 Description for tool 1 │ │ │ ╰──────────────────────────────────────────────────────────────────────────╯ + ╭──────────────────────────────────────────────────────────────────────────╮ │ ✓ tool2 Description for tool 2 │ │ │ ╰──────────────────────────────────────────────────────────────────────────╯ + ╭──────────────────────────────────────────────────────────────────────────╮ │ o tool3 Description for tool 3 │ │ │ @@ -81,6 +83,7 @@ Tips for getting started: │ ✓ tool1 Description for tool 1 │ │ │ ╰──────────────────────────────────────────────────────────────────────────╯ + ╭──────────────────────────────────────────────────────────────────────────╮ │ ✓ tool2 Description for tool 2 │ │ │ diff --git a/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap index 30caf0fb40..c0fae8bf19 100644 --- a/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap @@ -39,37 +39,14 @@ Enter to select · ↑/↓ to navigate · Esc to cancel exports[`AskUserDialog > Scroll Arrows (useAlternateBuffer: true) > shows scroll arrows correctly when useAlternateBuffer is true 1`] = ` "Choose an option +▲ ● 1. Option 1 Description 1 2. Option 2 Description 2 3. Option 3 Description 3 - 4. Option 4 - Description 4 - 5. Option 5 - Description 5 - 6. Option 6 - Description 6 - 7. Option 7 - Description 7 - 8. Option 8 - Description 8 - 9. Option 9 - Description 9 - 10. Option 10 - Description 10 - 11. Option 11 - Description 11 - 12. Option 12 - Description 12 - 13. Option 13 - Description 13 - 14. Option 14 - Description 14 - 15. Option 15 - Description 15 - 16. Enter a custom value +▼ Enter to select · ↑/↓ to navigate · Esc to cancel " diff --git a/packages/cli/src/ui/components/__snapshots__/MainContent.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/MainContent.test.tsx.snap index 785dc6b6f0..4a5813f814 100644 --- a/packages/cli/src/ui/components/__snapshots__/MainContent.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/MainContent.test.tsx.snap @@ -6,12 +6,11 @@ AppHeader(full) ╭──────────────────────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command Running a long command... │ │ │ -│ Line 9 │ │ Line 10 │ │ Line 11 │ │ Line 12 │ │ Line 13 │ -│ Line 14 █ │ +│ Line 14 │ │ Line 15 █ │ │ Line 16 █ │ │ Line 17 █ │ @@ -28,12 +27,11 @@ AppHeader(full) ╭──────────────────────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command Running a long command... │ │ │ -│ Line 9 │ │ Line 10 │ │ Line 11 │ │ Line 12 │ │ Line 13 │ -│ Line 14 █ │ +│ Line 14 │ │ Line 15 █ │ │ Line 16 █ │ │ Line 17 █ │ @@ -49,8 +47,7 @@ exports[`MainContent > MainContent Tool Output Height Logic > 'Normal mode - Con ╭──────────────────────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command Running a long command... │ │ │ -│ ... first 9 lines hidden (Ctrl+O to show) ... │ -│ Line 10 │ +│ ... first 10 lines hidden (Ctrl+O to show) ... │ │ Line 11 │ │ Line 12 │ │ Line 13 │ 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..342dcf60fd --- /dev/null +++ b/packages/cli/src/ui/components/messages/DenseToolMessage.test.tsx @@ -0,0 +1,513 @@ +/** + * @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'; + +import { makeFakeConfig } from '@google/gemini-cli-core'; +import { createMockSettings } from '../../../test-utils/settings.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 } = await 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 } = await 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 } = await 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 } = await renderWithProviders( + , + {}, + ); + 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 } = await renderWithProviders( + , + {}, + ); + 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 } = await renderWithProviders( + , + {}, + ); + 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 } = await renderWithProviders( + , + {}, + ); + 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 } = await renderWithProviders( + , + {}, + ); + 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 } = await renderWithProviders( + , + {}, + ); + 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', + 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 } = await renderWithProviders( + , + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('Edit'); + expect(output).toContain('styles.scss → Failed (+1, -1)'); + 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 } = await 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 } = await 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 } = await 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 } = await 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 } = await renderWithProviders( + , + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('→ Returned (possible empty result)'); + expect(output).toMatchSnapshot(); + }); + + it('renders correctly for error status with string message', async () => { + const { lastFrame, waitUntilReady } = await 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 } = await 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 } = await 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 } = await renderWithProviders( + , + { + config: makeFakeConfig({ useAlternateBuffer: true }), + settings: createMockSettings({ ui: { useAlternateBuffer: true } }), + }, + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('Accepted'); + 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 } = await renderWithProviders( + , + { + config: makeFakeConfig({ useAlternateBuffer: false }), + settings: createMockSettings({ ui: { useAlternateBuffer: false } }), + }, + ); + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('Accepted'); + expect(output).toContain('new line'); + expect(output).toMatchSnapshot(); + }); + + it('shows diff content after clicking summary', async () => { + const { lastFrame, waitUntilReady } = await renderWithProviders( + , + { + config: makeFakeConfig({ useAlternateBuffer: true }), + settings: createMockSettings({ ui: { useAlternateBuffer: true } }), + mouseEventsEnabled: true, + }, + ); + await waitUntilReady(); + + // Verify it's hidden initially + expect(lastFrame()).not.toContain('new line'); + }); + }); +}); 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..3aae27d7ea --- /dev/null +++ b/packages/cli/src/ui/components/messages/DenseToolMessage.tsx @@ -0,0 +1,585 @@ +/** + * @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 { + // brief description of action + description?: React.ReactNode; + // result summary or status text + summary?: React.ReactNode; + // detailed output, e.g. diff or command output + 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, + isClickable?: boolean, +): 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 resultSummary = ''; + let resultColor = theme.text.secondary; + + if (status === ToolCallStatus.Confirming) { + resultSummary = 'Confirming'; + } else if ( + status === ToolCallStatus.Success || + status === ToolCallStatus.Executing + ) { + resultSummary = 'Accepted'; + resultColor = theme.text.accent; + } else if (status === ToolCallStatus.Canceled) { + resultSummary = 'Rejected'; + resultColor = theme.status.error; + } else if (status === ToolCallStatus.Error) { + resultSummary = + typeof resultDisplay === 'string' ? resultDisplay : 'Failed'; + resultColor = theme.status.error; + } + + const summary = ( + + {resultSummary && ( + + →{' '} + + {resultSummary.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 description = originalDescription ? ( + + {originalDescription} + + ) : undefined; + const summary = → {result.summary}; + + // For directory listings, we want NO payload in dense mode + return { description, summary, payload: undefined }; +} + +function getListResultData( + result: ListDirectoryResult | ReadManyFilesResult, + 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 = ( + + → Returned (possible empty result) + + ); + } + + 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, + isAlternateBuffer, + ); + } + if (isListResult(resultDisplay)) { + return getListResultData(resultDisplay, 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, + terminalWidth, + availableTerminalHeight, + originalDescription, + isAlternateBuffer, + ]); + + 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; + return colorizeCode({ + code: addedContent, + language: fileExtension, + maxWidth: terminalWidth ? terminalWidth - 6 : 80, + settings, + disableColor: mappedStatus === ToolCallStatus.Canceled, + returnLines: true, + }); + } 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} + + )} + + + {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 46b0c0097c..5f75d6e009 100644 --- a/packages/cli/src/ui/components/messages/DiffRenderer.test.tsx +++ b/packages/cli/src/ui/components/messages/DiffRenderer.test.tsx @@ -55,6 +55,7 @@ index 0000000..e69de29 maxWidth: 80, theme: undefined, settings: expect.anything(), + disableColor: false, }), ); }); @@ -89,6 +90,7 @@ index 0000000..e69de29 maxWidth: 80, theme: undefined, settings: expect.anything(), + disableColor: false, }), ); }); @@ -119,6 +121,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/ShellToolMessage.test.tsx b/packages/cli/src/ui/components/messages/ShellToolMessage.test.tsx index 6135d3574e..3247d8fcba 100644 --- a/packages/cli/src/ui/components/messages/ShellToolMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ShellToolMessage.test.tsx @@ -19,8 +19,12 @@ import { renderWithProviders } from '../../../test-utils/render.js'; import { createMockSettings } from '../../../test-utils/settings.js'; import { makeFakeConfig } from '@google/gemini-cli-core'; import { waitFor } from '../../../test-utils/async.js'; -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { SHELL_COMMAND_NAME, ACTIVE_SHELL_MAX_LINES } from '../../constants.js'; +import { + SHELL_CONTENT_OVERHEAD, + TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT, +} from '../../utils/toolLayoutUtils.js'; describe('', () => { const baseProps: ShellToolMessageProps = { @@ -35,6 +39,7 @@ describe('', () => { isFirst: true, borderColor: 'green', borderDimColor: false, + isExpandable: false, config: { getEnableInteractiveShell: () => true, } as unknown as Config, @@ -52,6 +57,11 @@ describe('', () => { beforeEach(() => { vi.clearAllMocks(); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); }); describe('interactive shell focus', () => { @@ -59,14 +69,14 @@ describe('', () => { ['SHELL_COMMAND_NAME', SHELL_COMMAND_NAME], ['SHELL_TOOL_NAME', SHELL_TOOL_NAME], ])('clicks inside the shell area sets focus for %s', async (_, name) => { - const { lastFrame, simulateClick, unmount } = await renderWithProviders( - , - { uiActions, mouseEventsEnabled: true }, - ); + const { lastFrame, simulateClick, unmount, waitUntilReady } = + await renderWithProviders( + , + { uiActions, mouseEventsEnabled: true }, + ); - await waitFor(() => { - expect(lastFrame()).toContain('A shell command'); - }); + await waitUntilReady(); + expect(lastFrame()).toContain('A shell command'); await simulateClick(2, 2); @@ -75,6 +85,7 @@ describe('', () => { }); unmount(); }); + it('resets focus when shell finishes', async () => { let updateStatus: (s: CoreToolCallStatus) => void = () => {}; @@ -86,19 +97,21 @@ describe('', () => { return ; }; - const { lastFrame, unmount } = await renderWithProviders(, { - uiActions, - uiState: { - streamingState: StreamingState.Idle, - embeddedShellFocused: true, - activePtyId: 1, + const { lastFrame, unmount, waitUntilReady } = await renderWithProviders( + , + { + uiActions, + uiState: { + streamingState: StreamingState.Idle, + embeddedShellFocused: true, + activePtyId: 1, + }, }, - }); + ); // Verify it is initially focused - await waitFor(() => { - expect(lastFrame()).toContain('(Shift+Tab to unfocus)'); - }); + await waitUntilReady(); + expect(lastFrame()).toContain('(Shift+Tab to unfocus)'); // Now update status to Success await act(async () => { @@ -185,29 +198,33 @@ describe('', () => { [ 'respects availableTerminalHeight when it is smaller than ACTIVE_SHELL_MAX_LINES', 10, - 8, + 10 - TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT, // 7 (Header height is 3, but calculation uses reserved=3) false, true, + false, ], [ 'uses ACTIVE_SHELL_MAX_LINES when availableTerminalHeight is large', 100, - ACTIVE_SHELL_MAX_LINES - 3, + ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD, // 11 false, true, + false, ], [ 'uses full availableTerminalHeight when focused in alternate buffer mode', 100, - 98, + 100 - TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT, // 97 true, false, + false, ], [ 'defaults to ACTIVE_SHELL_MAX_LINES in alternate buffer when availableTerminalHeight is undefined', undefined, - ACTIVE_SHELL_MAX_LINES - 3, + ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD, // 11 false, + true, false, ], ])( @@ -218,6 +235,7 @@ describe('', () => { expectedMaxLines, focused, constrainHeight, + isExpandable, ) => { const { lastFrame, waitUntilReady, unmount } = await renderWithProviders( @@ -228,6 +246,7 @@ describe('', () => { availableTerminalHeight={availableTerminalHeight} ptyId={1} status={CoreToolCallStatus.Executing} + isExpandable={isExpandable} />, { uiActions, @@ -252,7 +271,7 @@ describe('', () => { ); it('fully expands in standard mode when availableTerminalHeight is undefined', async () => { - const { lastFrame, unmount } = await renderWithProviders( + const { lastFrame, unmount, waitUntilReady } = await renderWithProviders( ', () => { }, ); - await waitFor(() => { - const frame = lastFrame(); - // Should show all 100 lines - expect(frame.match(/Line \d+/g)?.length).toBe(100); - }); + await waitUntilReady(); + const frame = lastFrame(); + // Should show all 100 lines + expect(frame.match(/Line \d+/g)?.length).toBe(100); unmount(); }); @@ -296,11 +314,9 @@ describe('', () => { ); await waitUntilReady(); - await waitFor(() => { - const frame = lastFrame(); - // Should show all 100 lines because constrainHeight is false and isExpandable is true - expect(frame.match(/Line \d+/g)?.length).toBe(100); - }); + const frame = lastFrame(); + // Should show all 100 lines because constrainHeight is false and isExpandable is true + expect(frame.match(/Line \d+/g)?.length).toBe(100); expect(lastFrame()).toMatchSnapshot(); unmount(); }); @@ -326,11 +342,11 @@ describe('', () => { ); await waitUntilReady(); - await waitFor(() => { - const frame = lastFrame(); - // Should still be constrained to 12 (15 - 3) because isExpandable is false - expect(frame.match(/Line \d+/g)?.length).toBe(12); - }); + const frame = lastFrame(); + // Should still be constrained to 11 (15 - 4) because isExpandable is false + expect(frame.match(/Line \d+/g)?.length).toBe( + ACTIVE_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD, + ); expect(lastFrame()).toMatchSnapshot(); unmount(); }); diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index 5398f2c23f..2fc89a63e9 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 = { @@ -469,6 +472,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 } = await renderWithProviders( @@ -497,6 +503,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 } = await renderWithProviders( @@ -525,6 +534,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 } = await renderWithProviders( @@ -668,6 +680,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..f46037b8e1 --- /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 } = await 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 } = await 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 } = await 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 } = await renderWithProviders( + , + { settings: compactSettings }, + ); + + await waitUntilReady(); + const output = lastFrame(); + expect(output).toMatchSnapshot(); + }); +}); diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx index 69da3a1029..d16d3827a3 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,11 +123,13 @@ 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( + const visibleToolCalls = useMemo( () => allToolCalls.filter((t) => { + // Hide internal errors unless full verbosity if ( isLowErrorVerbosity && t.status === CoreToolCallStatus.Error && @@ -65,19 +137,34 @@ export const ToolGroupMessage: React.FC = ({ ) { return false; } + // Standard hiding logic (e.g. Plan Mode internal edits) + if ( + shouldHideToolCall({ + displayName: t.name, + status: t.status, + approvalMode: t.approvalMode, + hasResultDisplay: !!t.resultDisplay, + parentCallId: t.parentCallId, + }) + ) { + return false; + } - return !shouldHideToolCall({ - displayName: t.name, - status: t.status, - approvalMode: t.approvalMode, - hasResultDisplay: !!t.resultDisplay, - parentCallId: t.parentCallId, - }); + // We HIDE tools that are still in pre-execution states (Confirming, Pending) + // from the History log. They live in the Global Queue or wait for their turn. + // Only show tools that are actually running or finished. + const displayStatus = mapCoreStatusToDisplayStatus(t.status); + + // We hide Confirming tools from the history log because they are + // currently being rendered in the interactive ToolConfirmationQueue. + // We show everything else, including Pending (waiting to run) and + // Canceled (rejected by user), to ensure the history is complete + // and to avoid tools "vanishing" after approval. + return displayStatus !== ToolCallStatus.Confirming; }), [allToolCalls, isLowErrorVerbosity], ); - const config = useConfig(); const { activePtyId, embeddedShellFocused, @@ -85,6 +172,8 @@ export const ToolGroupMessage: React.FC = ({ pendingHistoryItems, } = useUIState(); + const config = useConfig(); + const { borderColor, borderDimColor } = useMemo( () => getToolGroupBorderAppearance( @@ -103,41 +192,6 @@ export const ToolGroupMessage: React.FC = ({ ], ); - // We HIDE tools that are still in pre-execution states (Confirming, Pending) - // from the History log. They live in the Global Queue or wait for their turn. - // Only show tools that are actually running or finished. - // We explicitly exclude Pending and Confirming to ensure they only - // appear in the Global Queue until they are approved and start executing. - const visibleToolCalls = useMemo( - () => - toolCalls.filter((t) => { - const displayStatus = mapCoreStatusToDisplayStatus(t.status); - // We hide Confirming tools from the history log because they are - // currently being rendered in the interactive ToolConfirmationQueue. - // We show everything else, including Pending (waiting to run) and - // Canceled (rejected by user), to ensure the history is complete - // and to avoid tools "vanishing" after approval. - return displayStatus !== ToolCallStatus.Confirming; - }), - - [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 +211,75 @@ 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; + const prevGroup = i > 0 ? groupedTools[i - 1] : null; + const prevIsCompact = + prevGroup && + !Array.isArray(prevGroup) && + isCompactTool(prevGroup, isCompactModeEnabled); + + if (Array.isArray(group)) { + // Agent group + height += 1; // Header + height += group.length; // 1 line per agent + const isFirstProp = isFirst + ? (borderTopOverride ?? true) + : prevIsCompact; + if (isFirstProp) height += 1; // Top border + + // Spacing logic + if (isFirst) { + height += (borderTopOverride ?? true) ? 1 : 0; + } else { + height += 1; // marginTop + } + } else { + const isCompact = isCompactTool(group, 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 +293,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 +312,158 @@ 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); + + const isAgentGroup = Array.isArray(group); + const isCompact = + !isAgentGroup && isCompactTool(group, isCompactModeEnabled); + + let marginTop = 0; + if (isFirst) { + // marginTop = (borderTopOverride ?? true) ? 1 : 0; + marginTop = (borderTopOverride ?? false) ? 1 : 0; + } else if (isCompact && prevIsCompact) { + marginTop = 0; + } else { + marginTop = 1; + } + + const isFirstProp = !!(isFirst + ? (borderTopOverride ?? true) + : prevIsCompact); + + const showClosingBorder = !isCompact && (nextIsCompact || isLast); + + if (isAgentGroup) { + return ( + + + {showClosingBorder && ( + + )} + + ); + } + + const tool = group; + const isShellToolCall = isShellTool(tool.name); + const commonProps = { + ...tool, + availableTerminalHeight: availableTerminalHeightPerToolMessage, + terminalWidth: contentWidth, + emphasis: 'medium' as const, + isFirst: isCompact ? false : isFirstProp, + borderColor, + borderDimColor, + isExpandable, + }; + + return ( + + + {isCompact ? ( + + ) : isShellToolCall ? ( + + ) : ( + + )} + {!isCompact && tool.outputFile && ( + + + + Output too long and was saved to: {tool.outputFile} + + + + )} + + {showClosingBorder && ( + + )} + + ); + })} ); 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 2 lines of content show below 3-line header) 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..e7cb9cb195 --- /dev/null +++ b/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage.test.tsx.snap @@ -0,0 +1,135 @@ +// 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 +" +`; + +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 (+1, -1) + + 1 - old line + 1 + new line +" +`; + +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 → Returned (possible empty result) +" +`; + +exports[`DenseToolMessage > truncates long string results 1`] = ` +" ✓ test-tool Test description + → + AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA + AAAAAAAAAAAAAAAAAAAA... +" +`; diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ShellToolMessage.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ShellToolMessage.test.tsx.snap index 1847b8ce67..967ea81e14 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ShellToolMessage.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/ShellToolMessage.test.tsx.snap @@ -4,7 +4,6 @@ exports[` > Height Constraints > defaults to ACTIVE_SHELL_MA "╭──────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command A shell command │ │ │ -│ Line 89 │ │ Line 90 │ │ Line 91 │ │ Line 92 │ @@ -14,7 +13,7 @@ exports[` > Height Constraints > defaults to ACTIVE_SHELL_MA │ Line 96 │ │ Line 97 │ │ Line 98 │ -│ Line 99 ▄ │ +│ Line 99 │ │ Line 100 █ │ " `; @@ -130,7 +129,6 @@ exports[` > Height Constraints > respects availableTerminalH "╭──────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command A shell command │ │ │ -│ Line 93 │ │ Line 94 │ │ Line 95 │ │ Line 96 │ @@ -145,7 +143,6 @@ exports[` > Height Constraints > stays constrained in altern "╭──────────────────────────────────────────────────────────────────────────────╮ │ ✓ Shell Command A shell command │ │ │ -│ Line 89 │ │ Line 90 │ │ Line 91 │ │ Line 92 │ @@ -155,7 +152,7 @@ exports[` > Height Constraints > stays constrained in altern │ Line 96 │ │ Line 97 │ │ Line 98 │ -│ Line 99 ▄ │ +│ Line 99 │ │ Line 100 █ │ " `; @@ -164,7 +161,6 @@ exports[` > Height Constraints > uses ACTIVE_SHELL_MAX_LINES "╭──────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command A shell command │ │ │ -│ Line 89 │ │ Line 90 │ │ Line 91 │ │ Line 92 │ @@ -174,7 +170,7 @@ exports[` > Height Constraints > uses ACTIVE_SHELL_MAX_LINES │ Line 96 │ │ Line 97 │ │ Line 98 │ -│ Line 99 ▄ │ +│ Line 99 │ │ Line 100 █ │ " `; @@ -183,10 +179,9 @@ exports[` > Height Constraints > uses full availableTerminal "╭──────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command A shell command (Shift+Tab to unfocus) │ │ │ -│ Line 3 │ │ Line 4 │ -│ Line 5 █ │ -│ Line 6 █ │ +│ Line 5 │ +│ Line 6 │ │ Line 7 █ │ │ Line 8 █ │ │ Line 9 █ │ 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__/ToolResultDisplay.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay.test.tsx.snap index 5e5c7ea2b0..e34e66cc48 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay.test.tsx.snap @@ -37,8 +37,7 @@ exports[`ToolResultDisplay > renders string result as plain text when renderOutp `; exports[`ToolResultDisplay > truncates very long string results 1`] = ` -"... 248 hidden (Ctrl+O) ... -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +"... 249 hidden (Ctrl+O) ... aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa 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..c6c8c2ebbe 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); @@ -77,24 +90,23 @@ export const ToolActionsProvider: React.FC = ( useEffect(() => { let isMounted = true; + let activeClient: IdeClient | null = null; + + const handleStatusChange = () => { + if (isMounted && activeClient) { + setIsDiffingEnabled(activeClient.isDiffingEnabled()); + } + }; + if (config.getIdeMode()) { IdeClient.getInstance() .then((client) => { if (!isMounted) return; + activeClient = client; setIdeClient(client); setIsDiffingEnabled(client.isDiffingEnabled()); - const handleStatusChange = () => { - if (isMounted) { - setIsDiffingEnabled(client.isDiffingEnabled()); - } - }; - client.addStatusChangeListener(handleStatusChange); - // Return a cleanup function for the listener - return () => { - client.removeStatusChangeListener(handleStatusChange); - }; }) .catch((error) => { debugLogger.error('Failed to get IdeClient instance:', error); @@ -102,6 +114,9 @@ export const ToolActionsProvider: React.FC = ( } return () => { isMounted = false; + if (activeClient) { + activeClient.removeStatusChangeListener(handleStatusChange); + } }; }, [config]); @@ -164,7 +179,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.test.tsx b/packages/cli/src/ui/utils/CodeColorizer.test.tsx index 2628a36d0a..b9d5c65877 100644 --- a/packages/cli/src/ui/utils/CodeColorizer.test.tsx +++ b/packages/cli/src/ui/utils/CodeColorizer.test.tsx @@ -82,4 +82,28 @@ describe('colorizeCode', () => { await expect(renderResult).toMatchSvgSnapshot(); renderResult.unmount(); }); + + it('returns an array of lines when returnLines is true', () => { + const code = 'line 1\nline 2\nline 3'; + const settings = new LoadedSettings( + { path: '', settings: {}, originalSettings: {} }, + { path: '', settings: {}, originalSettings: {} }, + { path: '', settings: {}, originalSettings: {} }, + { path: '', settings: {}, originalSettings: {} }, + true, + [], + ); + + const result = colorizeCode({ + code, + language: 'javascript', + maxWidth: 80, + settings, + hideLineNumbers: true, + returnLines: true, + }); + + expect(Array.isArray(result)).toBe(true); + expect(result).toHaveLength(3); + }); }); diff --git a/packages/cli/src/ui/utils/CodeColorizer.tsx b/packages/cli/src/ui/utils/CodeColorizer.tsx index 948a5f8988..94dda9501e 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; } /** @@ -138,6 +144,12 @@ export interface ColorizeCodeOptions { * @param options The options for colorizing the code. * @returns A React.ReactNode containing Ink elements for the highlighted code. */ +export function colorizeCode( + options: ColorizeCodeOptions & { returnLines: true }, +): React.ReactNode[]; +export function colorizeCode( + options: ColorizeCodeOptions & { returnLines?: false }, +): React.ReactNode; export function colorizeCode({ code, language = null, @@ -146,13 +158,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 +177,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 +187,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 +201,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/cli/src/ui/utils/confirmingTool.ts b/packages/cli/src/ui/utils/confirmingTool.ts index 86579f1d1f..c7edf8d790 100644 --- a/packages/cli/src/ui/utils/confirmingTool.ts +++ b/packages/cli/src/ui/utils/confirmingTool.ts @@ -6,10 +6,10 @@ import { CoreToolCallStatus } from '@google/gemini-cli-core'; import { - type HistoryItemToolGroup, type HistoryItemWithoutId, type IndividualToolCallDisplay, } from '../types.js'; +import { getAllToolCalls } from './historyUtils.js'; export interface ConfirmingToolState { tool: IndividualToolCallDisplay; @@ -23,9 +23,7 @@ export interface ConfirmingToolState { export function getConfirmingToolState( pendingHistoryItems: HistoryItemWithoutId[], ): ConfirmingToolState | null { - const allPendingTools = pendingHistoryItems - .filter((item): item is HistoryItemToolGroup => item.type === 'tool_group') - .flatMap((group) => group.tools); + const allPendingTools = getAllToolCalls(pendingHistoryItems); const confirmingTools = allPendingTools.filter( (tool) => tool.status === CoreToolCallStatus.AwaitingApproval, diff --git a/packages/cli/src/ui/utils/historyUtils.ts b/packages/cli/src/ui/utils/historyUtils.ts new file mode 100644 index 0000000000..ee607dca96 --- /dev/null +++ b/packages/cli/src/ui/utils/historyUtils.ts @@ -0,0 +1,83 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { CoreToolCallStatus } from '../types.js'; +import type { + HistoryItem, + HistoryItemWithoutId, + HistoryItemToolGroup, + IndividualToolCallDisplay, +} from '../types.js'; + +export function getLastTurnToolCallIds( + history: HistoryItem[], + pendingHistoryItems: HistoryItemWithoutId[], +): string[] { + const targetToolCallIds: string[] = []; + + // Find the boundary of the last user prompt + let lastUserPromptIndex = -1; + for (let i = history.length - 1; i >= 0; i--) { + const type = history[i].type; + if (type === 'user' || type === 'user_shell') { + lastUserPromptIndex = i; + break; + } + } + + // Collect IDs from history after last user prompt + 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); + }); + } + }); + + return targetToolCallIds; +} + +export function isToolExecuting( + pendingHistoryItems: HistoryItemWithoutId[], +): boolean { + return pendingHistoryItems.some((item) => { + if (item && item.type === 'tool_group') { + return item.tools.some( + (tool) => CoreToolCallStatus.Executing === tool.status, + ); + } + return false; + }); +} + +export function isToolAwaitingConfirmation( + pendingHistoryItems: HistoryItemWithoutId[], +): boolean { + return pendingHistoryItems + .filter((item): item is HistoryItemToolGroup => item.type === 'tool_group') + .some((item) => + item.tools.some( + (tool) => CoreToolCallStatus.AwaitingApproval === tool.status, + ), + ); +} + +export function getAllToolCalls( + historyItems: HistoryItemWithoutId[], +): IndividualToolCallDisplay[] { + return historyItems + .filter((item): item is HistoryItemToolGroup => item.type === 'tool_group') + .flatMap((group) => group.tools); +} diff --git a/packages/cli/src/ui/utils/toolLayoutUtils.test.ts b/packages/cli/src/ui/utils/toolLayoutUtils.test.ts index 57e1e3f190..768fccc111 100644 --- a/packages/cli/src/ui/utils/toolLayoutUtils.test.ts +++ b/packages/cli/src/ui/utils/toolLayoutUtils.test.ts @@ -9,6 +9,10 @@ import { calculateToolContentMaxLines, calculateShellMaxLines, SHELL_CONTENT_OVERHEAD, + TOOL_RESULT_STATIC_HEIGHT, + TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT, + TOOL_RESULT_ASB_RESERVED_LINE_COUNT, + TOOL_RESULT_MIN_LINES_SHOWN, } from './toolLayoutUtils.js'; import { CoreToolCallStatus } from '@google/gemini-cli-core'; import { @@ -48,7 +52,7 @@ describe('toolLayoutUtils', () => { availableTerminalHeight: 2, isAlternateBuffer: false, }, - expected: 3, + expected: TOOL_RESULT_MIN_LINES_SHOWN + 1, }, { desc: 'returns available space directly in constrained terminal (ASB mode)', @@ -56,7 +60,7 @@ describe('toolLayoutUtils', () => { availableTerminalHeight: 4, isAlternateBuffer: true, }, - expected: 3, + expected: TOOL_RESULT_MIN_LINES_SHOWN + 1, }, { desc: 'returns remaining space if sufficient space exists (Standard mode)', @@ -64,7 +68,10 @@ describe('toolLayoutUtils', () => { availableTerminalHeight: 20, isAlternateBuffer: false, }, - expected: 17, + expected: + 20 - + TOOL_RESULT_STATIC_HEIGHT - + TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT, }, { desc: 'returns remaining space if sufficient space exists (ASB mode)', @@ -72,7 +79,8 @@ describe('toolLayoutUtils', () => { availableTerminalHeight: 20, isAlternateBuffer: true, }, - expected: 13, + expected: + 20 - TOOL_RESULT_STATIC_HEIGHT - TOOL_RESULT_ASB_RESERVED_LINE_COUNT, }, ]; @@ -148,7 +156,7 @@ describe('toolLayoutUtils', () => { constrainHeight: true, isExpandable: false, }, - expected: 4, + expected: 6 - TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT, }, { desc: 'handles negative availableTerminalHeight gracefully', @@ -172,7 +180,7 @@ describe('toolLayoutUtils', () => { constrainHeight: false, isExpandable: false, }, - expected: 28, + expected: 30 - TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT, }, { desc: 'falls back to COMPLETED_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD for completed shells if space allows', diff --git a/packages/cli/src/ui/utils/toolLayoutUtils.ts b/packages/cli/src/ui/utils/toolLayoutUtils.ts index 9f391dca4e..1f140b9bc9 100644 --- a/packages/cli/src/ui/utils/toolLayoutUtils.ts +++ b/packages/cli/src/ui/utils/toolLayoutUtils.ts @@ -17,7 +17,7 @@ import { CoreToolCallStatus } from '@google/gemini-cli-core'; */ export const TOOL_RESULT_STATIC_HEIGHT = 1; export const TOOL_RESULT_ASB_RESERVED_LINE_COUNT = 6; -export const TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT = 2; +export const TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT = 3; export const TOOL_RESULT_MIN_LINES_SHOWN = 2; /** 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/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 1ecae4ef33..4b4e6e0c54 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -23,6 +23,7 @@ import { getToolSuggestion, isToolCallResponseInfo, } from '../utils/tool-utils.js'; +import { getDiffStatFromPatch } from '../tools/diffOptions.js'; import type { ToolConfirmationRequest } from '../confirmation-bus/types.js'; import { MessageBusType } from '../confirmation-bus/types.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; @@ -312,6 +313,12 @@ export class CoreToolScheduler { waitingCall.confirmationDetails.originalContent, newContent: waitingCall.confirmationDetails.newContent, filePath: waitingCall.confirmationDetails.filePath, + // Derive stats from the patch if they aren't already present + diffStat: + waitingCall.confirmationDetails.diffStat ?? + getDiffStatFromPatch( + waitingCall.confirmationDetails.fileDiff, + ), }; } } diff --git a/packages/core/src/scheduler/state-manager.test.ts b/packages/core/src/scheduler/state-manager.test.ts index dd5071c5bf..ff69e0d207 100644 --- a/packages/core/src/scheduler/state-manager.test.ts +++ b/packages/core/src/scheduler/state-manager.test.ts @@ -22,6 +22,7 @@ import { ToolConfirmationOutcome, type AnyDeclarativeTool, type AnyToolInvocation, + type FileDiff, } from '../tools/tools.js'; import { MessageBusType } from '../confirmation-bus/types.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; @@ -359,7 +360,7 @@ describe('SchedulerStateManager', () => { expect(active.confirmationDetails).toEqual(details); }); - it('should preserve diff when cancelling an edit tool call', () => { + it('should preserve diff and derive stats when cancelling an edit tool call', () => { const call = createValidatingCall(); stateManager.enqueue([call]); stateManager.dequeue(); @@ -369,9 +370,9 @@ describe('SchedulerStateManager', () => { title: 'Edit', fileName: 'test.txt', filePath: '/path/to/test.txt', - fileDiff: 'diff', - originalContent: 'old', - newContent: 'new', + fileDiff: '@@ -1,1 +1,1 @@\n-old line\n+new line', + originalContent: 'old line', + newContent: 'new line', onConfirm: vi.fn(), }; @@ -389,13 +390,14 @@ describe('SchedulerStateManager', () => { const completed = stateManager.completedBatch[0] as CancelledToolCall; expect(completed.status).toBe(CoreToolCallStatus.Cancelled); - expect(completed.response.resultDisplay).toEqual({ - fileDiff: 'diff', - fileName: 'test.txt', - filePath: '/path/to/test.txt', - originalContent: 'old', - newContent: 'new', - }); + const result = completed.response.resultDisplay as FileDiff; + expect(result.fileDiff).toBe(details.fileDiff); + expect(result.diffStat).toEqual( + expect.objectContaining({ + model_added_lines: 1, + model_removed_lines: 1, + }), + ); }); it('should ignore status updates for non-existent callIds', () => { diff --git a/packages/core/src/scheduler/state-manager.ts b/packages/core/src/scheduler/state-manager.ts index 428b7f87a8..093aaa7308 100644 --- a/packages/core/src/scheduler/state-manager.ts +++ b/packages/core/src/scheduler/state-manager.ts @@ -32,6 +32,7 @@ import { type SerializableConfirmationDetails, } from '../confirmation-bus/types.js'; import { isToolCallResponseInfo } from '../utils/tool-utils.js'; +import { getDiffStatFromPatch } from '../tools/diffOptions.js'; /** * Handler for terminal tool calls. @@ -473,6 +474,8 @@ export class SchedulerStateManager { filePath: details.filePath, originalContent: details.originalContent, newContent: details.newContent, + // Derive stats from the patch if they aren't already present + diffStat: details.diffStat ?? getDiffStatFromPatch(details.fileDiff), }; } } diff --git a/packages/core/src/tools/diffOptions.ts b/packages/core/src/tools/diffOptions.ts index b026b14f7c..0a0e0fa49e 100644 --- a/packages/core/src/tools/diffOptions.ts +++ b/packages/core/src/tools/diffOptions.ts @@ -76,3 +76,39 @@ export function getDiffStat( user_removed_chars: userStats.removedChars, }; } + +/** + * Extracts line and character stats from a unified diff patch string. + * This is useful for reconstructing stats for rejected or errored operations + * where the full strings may no longer be easily accessible. + */ +export function getDiffStatFromPatch(patch: string): DiffStat { + let addedLines = 0; + let removedLines = 0; + let addedChars = 0; + let removedChars = 0; + + const lines = patch.split('\n'); + for (const line of lines) { + // Only count lines that are additions or removals, + // excluding the diff headers (--- and +++) and metadata (\) + if (line.startsWith('+') && !line.startsWith('+++')) { + addedLines++; + addedChars += line.length - 1; + } else if (line.startsWith('-') && !line.startsWith('---')) { + removedLines++; + removedChars += line.length - 1; + } + } + + return { + model_added_lines: addedLines, + model_removed_lines: removedLines, + model_added_chars: addedChars, + model_removed_chars: removedChars, + user_added_lines: 0, + user_removed_lines: 0, + user_added_chars: 0, + user_removed_chars: 0, + }; +} 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..372de8e8a6 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 () => { @@ -171,7 +177,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 +194,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 +211,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 () => { @@ -287,7 +300,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 +363,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..b8e2e6a803 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,7 +143,6 @@ class LSToolInvocation extends BaseToolInvocation { ): ToolResult { return { llmContent, - // Keep returnDisplay simpler in core logic returnDisplay: `Error: ${returnDisplay}`, error: { message: llmContent, @@ -284,7 +283,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 +313,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..c92b608791 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,7 @@ ${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: `Error: ${getErrorMessage(error)}`, error: { message: errorMessage, type: ToolErrorType.READ_MANY_FILES_SEARCH_ERROR, @@ -483,9 +487,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 +521,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..3b55cef0a1 100644 --- a/packages/core/src/tools/tool-names.ts +++ b/packages/core/src/tools/tool-names.ts @@ -150,8 +150,6 @@ export { SKILL_PARAM_NAME, }; -export const LS_TOOL_NAME_LEGACY = 'list_directory'; // Just to be safe if anything used the old exported name directly - export const EDIT_TOOL_NAMES = new Set([EDIT_TOOL_NAME, WRITE_FILE_TOOL_NAME]); /** @@ -183,6 +181,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..c0aae63f80 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' @@ -869,6 +910,7 @@ export interface ToolEditConfirmationDetails { originalContent: string | null; newContent: string; isModifying?: boolean; + diffStat?: DiffStat; ideConfirmation?: Promise; } 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 2b528ad8dc..fc5cc561dc 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",