From 54b0344fc5bf55758c23c1a2a59e3c1e0f13deff Mon Sep 17 00:00:00 2001 From: Jarrod Whelan <150866123+jwhelangoog@users.noreply.github.com> Date: Sat, 7 Mar 2026 11:04:22 -0800 Subject: [PATCH] fix(ui): unify Ctrl+O expansion hint experience across buffer modes (#21474) --- packages/cli/src/ui/AppContainer.test.tsx | 65 +++++- packages/cli/src/ui/AppContainer.tsx | 11 +- .../src/ui/components/FolderTrustDialog.tsx | 6 +- .../src/ui/components/ShowMoreLines.test.tsx | 6 +- .../cli/src/ui/components/ShowMoreLines.tsx | 3 - .../components/ShowMoreLinesLayout.test.tsx | 28 +++ .../src/ui/components/ToastDisplay.test.tsx | 2 +- .../cli/src/ui/components/ToastDisplay.tsx | 2 +- .../ui/components/ToolConfirmationQueue.tsx | 9 +- .../__snapshots__/MainContent.test.tsx.snap | 2 - .../ToolConfirmationQueue.test.tsx.snap | 1 + .../ui/components/messages/GeminiMessage.tsx | 20 +- .../messages/GeminiMessageContent.tsx | 10 +- .../messages/ToolGroupMessage.test.tsx | 197 ------------------ .../components/messages/ToolGroupMessage.tsx | 82 +------- .../ToolOverflowConsistencyChecks.test.tsx | 75 +++---- .../components/messages/ToolResultDisplay.tsx | 1 + .../ToolResultDisplayOverflow.test.tsx | 79 ------- .../src/ui/components/shared/Scrollable.tsx | 36 +++- 19 files changed, 184 insertions(+), 451 deletions(-) delete mode 100644 packages/cli/src/ui/components/messages/ToolResultDisplayOverflow.test.tsx diff --git a/packages/cli/src/ui/AppContainer.test.tsx b/packages/cli/src/ui/AppContainer.test.tsx index cfa81f7c2a..6cf81ca316 100644 --- a/packages/cli/src/ui/AppContainer.test.tsx +++ b/packages/cli/src/ui/AppContainer.test.tsx @@ -3465,6 +3465,63 @@ describe('AppContainer State Management', () => { unmount!(); }); + it('resets the hint timer when a new component overflows (overflowingIdsSize increases)', async () => { + let unmount: () => void; + await act(async () => { + const result = renderAppContainer(); + unmount = result.unmount; + }); + await waitFor(() => expect(capturedUIState).toBeTruthy()); + + // 1. Trigger first overflow + act(() => { + capturedOverflowActions.addOverflowingId('test-id-1'); + }); + + await waitFor(() => { + expect(capturedUIState.showIsExpandableHint).toBe(true); + }); + + // 2. Advance half the duration + act(() => { + vi.advanceTimersByTime(EXPAND_HINT_DURATION_MS / 2); + }); + expect(capturedUIState.showIsExpandableHint).toBe(true); + + // 3. Trigger second overflow (this should reset the timer) + act(() => { + capturedOverflowActions.addOverflowingId('test-id-2'); + }); + + // Advance by 1ms to allow the OverflowProvider's 0ms batching timeout to fire + // and flush the state update to AppContainer, triggering the reset. + act(() => { + vi.advanceTimersByTime(1); + }); + + await waitFor(() => { + expect(capturedUIState.showIsExpandableHint).toBe(true); + }); + + // 4. Advance enough that the ORIGINAL timer would have expired + // Subtracting 1ms since we advanced it above to flush the state. + act(() => { + vi.advanceTimersByTime(EXPAND_HINT_DURATION_MS / 2 + 100 - 1); + }); + // The hint should STILL be visible because the timer reset at step 3 + expect(capturedUIState.showIsExpandableHint).toBe(true); + + // 5. Advance to the end of the NEW timer + act(() => { + vi.advanceTimersByTime(EXPAND_HINT_DURATION_MS / 2 - 100); + }); + await waitFor(() => { + expect(capturedUIState.showIsExpandableHint).toBe(false); + }); + + unmount!(); + }); + it('toggles expansion state and resets the hint timer when Ctrl+O is pressed in Standard Mode', async () => { let unmount: () => void; let stdin: ReturnType['stdin']; @@ -3606,7 +3663,7 @@ describe('AppContainer State Management', () => { unmount!(); }); - it('does NOT set showIsExpandableHint when overflow occurs in Alternate Buffer Mode', async () => { + it('DOES set showIsExpandableHint when overflow occurs in Alternate Buffer Mode', async () => { const alternateSettings = mergeSettings({}, {}, {}, {}, true); const settingsWithAlternateBuffer = { merged: { @@ -3634,8 +3691,10 @@ describe('AppContainer State Management', () => { capturedOverflowActions.addOverflowingId('test-id'); }); - // Should NOT show hint because we are in Alternate Buffer Mode - expect(capturedUIState.showIsExpandableHint).toBe(false); + // Should NOW show hint because we are in Alternate Buffer Mode + await waitFor(() => { + expect(capturedUIState.showIsExpandableHint).toBe(true); + }); unmount!(); }); diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 41cc5dec3d..30ebe221f0 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -283,19 +283,18 @@ export const AppContainer = (props: AppContainerProps) => { * Manages the visibility and x-second timer for the expansion hint. * * This effect triggers the timer countdown whenever an overflow is detected - * or the user manually toggles the expansion state with Ctrl+O. We use a stable - * boolean dependency (hasOverflowState) to ensure the timer only resets on - * genuine state transitions, preventing it from infinitely resetting during - * active text streaming. + * or the user manually toggles the expansion state with Ctrl+O. + * By depending on overflowingIdsSize, the timer resets when *new* views + * overflow, but avoids infinitely resetting during single-view streaming. * * In alternate buffer mode, we don't trigger the hint automatically on overflow * to avoid noise, but the user can still trigger it manually with Ctrl+O. */ useEffect(() => { - if (hasOverflowState && !isAlternateBuffer) { + if (hasOverflowState) { triggerExpandHint(true); } - }, [hasOverflowState, isAlternateBuffer, triggerExpandHint]); + }, [hasOverflowState, overflowingIdsSize, triggerExpandHint]); const [defaultBannerText, setDefaultBannerText] = useState(''); const [warningBannerText, setWarningBannerText] = useState(''); diff --git a/packages/cli/src/ui/components/FolderTrustDialog.tsx b/packages/cli/src/ui/components/FolderTrustDialog.tsx index 5f154a4d1a..5bb748b28f 100644 --- a/packages/cli/src/ui/components/FolderTrustDialog.tsx +++ b/packages/cli/src/ui/components/FolderTrustDialog.tsx @@ -311,9 +311,5 @@ export const FolderTrustDialog: React.FC = ({ ); - return isAlternateBuffer ? ( - {content} - ) : ( - content - ); + return {content}; }; diff --git a/packages/cli/src/ui/components/ShowMoreLines.test.tsx b/packages/cli/src/ui/components/ShowMoreLines.test.tsx index 4a6829809a..dbdc8085a2 100644 --- a/packages/cli/src/ui/components/ShowMoreLines.test.tsx +++ b/packages/cli/src/ui/components/ShowMoreLines.test.tsx @@ -45,7 +45,7 @@ describe('ShowMoreLines', () => { }, ); - it('renders nothing in STANDARD mode even if overflowing', async () => { + it('renders message in STANDARD mode when overflowing', async () => { mockUseAlternateBuffer.mockReturnValue(false); mockUseOverflowState.mockReturnValue({ overflowingIds: new Set(['1']), @@ -55,7 +55,9 @@ describe('ShowMoreLines', () => { , ); await waitUntilReady(); - expect(lastFrame({ allowEmpty: true })).toBe(''); + expect(lastFrame().toLowerCase()).toContain( + 'press ctrl+o to show more lines', + ); unmount(); }); diff --git a/packages/cli/src/ui/components/ShowMoreLines.tsx b/packages/cli/src/ui/components/ShowMoreLines.tsx index 92acd2b29a..1af2befcd8 100644 --- a/packages/cli/src/ui/components/ShowMoreLines.tsx +++ b/packages/cli/src/ui/components/ShowMoreLines.tsx @@ -9,7 +9,6 @@ import { useOverflowState } from '../contexts/OverflowContext.js'; import { useStreamingContext } from '../contexts/StreamingContext.js'; import { StreamingState } from '../types.js'; import { theme } from '../semantic-colors.js'; -import { useAlternateBuffer } from '../hooks/useAlternateBuffer.js'; interface ShowMoreLinesProps { constrainHeight: boolean; @@ -20,7 +19,6 @@ export const ShowMoreLines = ({ constrainHeight, isOverflowing: isOverflowingProp, }: ShowMoreLinesProps) => { - const isAlternateBuffer = useAlternateBuffer(); const overflowState = useOverflowState(); const streamingState = useStreamingContext(); @@ -29,7 +27,6 @@ export const ShowMoreLines = ({ (overflowState !== undefined && overflowState.overflowingIds.size > 0); if ( - !isAlternateBuffer || !isOverflowing || !constrainHeight || !( diff --git a/packages/cli/src/ui/components/ShowMoreLinesLayout.test.tsx b/packages/cli/src/ui/components/ShowMoreLinesLayout.test.tsx index ede092976f..b5f8eb3b8b 100644 --- a/packages/cli/src/ui/components/ShowMoreLinesLayout.test.tsx +++ b/packages/cli/src/ui/components/ShowMoreLinesLayout.test.tsx @@ -64,4 +64,32 @@ describe('ShowMoreLines layout and padding', () => { unmount(); }); + + it('renders in Standard mode as well', async () => { + mockUseAlternateBuffer.mockReturnValue(false); // Standard mode + + const TestComponent = () => ( + + Top + + Bottom + + ); + + const { lastFrame, waitUntilReady, unmount } = render(); + await waitUntilReady(); + + const output = lastFrame({ allowEmpty: true }); + const lines = output.split('\n'); + + expect(lines).toEqual([ + 'Top', + ' Press Ctrl+O to show more lines', + '', + 'Bottom', + '', + ]); + + unmount(); + }); }); diff --git a/packages/cli/src/ui/components/ToastDisplay.test.tsx b/packages/cli/src/ui/components/ToastDisplay.test.tsx index 668f91c8d9..b1432caa9d 100644 --- a/packages/cli/src/ui/components/ToastDisplay.test.tsx +++ b/packages/cli/src/ui/components/ToastDisplay.test.tsx @@ -188,7 +188,7 @@ describe('ToastDisplay', () => { }); await waitUntilReady(); expect(lastFrame()).toContain( - 'Ctrl+O to show more lines of the last response', + 'Press Ctrl+O to show more lines of the last response', ); }); diff --git a/packages/cli/src/ui/components/ToastDisplay.tsx b/packages/cli/src/ui/components/ToastDisplay.tsx index 6fcef1667c..869139cb39 100644 --- a/packages/cli/src/ui/components/ToastDisplay.tsx +++ b/packages/cli/src/ui/components/ToastDisplay.tsx @@ -78,7 +78,7 @@ export const ToastDisplay: React.FC = () => { const action = uiState.constrainHeight ? 'show more' : 'collapse'; return ( - Ctrl+O to {action} lines of the last response + Press Ctrl+O to {action} lines of the last response ); } diff --git a/packages/cli/src/ui/components/ToolConfirmationQueue.tsx b/packages/cli/src/ui/components/ToolConfirmationQueue.tsx index 3fb1cc8c6f..b976bb3755 100644 --- a/packages/cli/src/ui/components/ToolConfirmationQueue.tsx +++ b/packages/cli/src/ui/components/ToolConfirmationQueue.tsx @@ -15,7 +15,6 @@ import type { ConfirmingToolState } from '../hooks/useConfirmingTool.js'; import { OverflowProvider } from '../contexts/OverflowContext.js'; import { ShowMoreLines } from './ShowMoreLines.js'; import { StickyHeader } from './StickyHeader.js'; -import { useAlternateBuffer } from '../hooks/useAlternateBuffer.js'; import type { SerializableConfirmationDetails } from '@google/gemini-cli-core'; import { useUIActions } from '../contexts/UIActionsContext.js'; @@ -43,7 +42,6 @@ export const ToolConfirmationQueue: React.FC = ({ }) => { const config = useConfig(); const { getPreferredEditor } = useUIActions(); - const isAlternateBuffer = useAlternateBuffer(); const { mainAreaWidth, terminalHeight, @@ -157,10 +155,5 @@ export const ToolConfirmationQueue: React.FC = ({ ); - return isAlternateBuffer ? ( - /* Shadow the global provider to maintain isolation in ASB mode. */ - {content} - ) : ( - content - ); + return {content}; }; 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 74acc6985d..c0043bf6f9 100644 --- a/packages/cli/src/ui/components/__snapshots__/MainContent.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/MainContent.test.tsx.snap @@ -18,7 +18,6 @@ AppHeader(full) │ Line 19 █ │ │ Line 20 █ │ ╰──────────────────────────────────────────────────────────────────────────────────────────────╯ - Press Ctrl+O to show more lines " `; @@ -40,7 +39,6 @@ AppHeader(full) │ Line 19 █ │ │ Line 20 █ │ ╰──────────────────────────────────────────────────────────────────────────────────────────────╯ - Press Ctrl+O to show more lines " `; diff --git a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap index a39d668825..6d9baba94f 100644 --- a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap @@ -16,6 +16,7 @@ exports[`ToolConfirmationQueue > calculates availableContentHeight based on avai │ 4. No, suggest changes (esc) │ │ │ ╰──────────────────────────────────────────────────────────────────────────────╯ + Press Ctrl+O to show more lines " `; diff --git a/packages/cli/src/ui/components/messages/GeminiMessage.tsx b/packages/cli/src/ui/components/messages/GeminiMessage.tsx index 481f0a8a0e..bc2246d23f 100644 --- a/packages/cli/src/ui/components/messages/GeminiMessage.tsx +++ b/packages/cli/src/ui/components/messages/GeminiMessage.tsx @@ -7,12 +7,9 @@ import type React from 'react'; import { Text, Box } from 'ink'; import { MarkdownDisplay } from '../../utils/MarkdownDisplay.js'; -import { ShowMoreLines } from '../ShowMoreLines.js'; import { theme } from '../../semantic-colors.js'; import { SCREEN_READER_MODEL_PREFIX } from '../../textConstants.js'; import { useUIState } from '../../contexts/UIStateContext.js'; -import { useAlternateBuffer } from '../../hooks/useAlternateBuffer.js'; -import { OverflowProvider } from '../../contexts/OverflowContext.js'; interface GeminiMessageProps { text: string; @@ -31,8 +28,7 @@ export const GeminiMessage: React.FC = ({ const prefix = '✦ '; const prefixWidth = prefix.length; - const isAlternateBuffer = useAlternateBuffer(); - const content = ( + return ( @@ -44,26 +40,14 @@ export const GeminiMessage: React.FC = ({ text={text} isPending={isPending} availableTerminalHeight={ - isAlternateBuffer || availableTerminalHeight === undefined + availableTerminalHeight === undefined ? undefined : Math.max(availableTerminalHeight - 1, 1) } terminalWidth={Math.max(terminalWidth - prefixWidth, 0)} renderMarkdown={renderMarkdown} /> - - - ); - - return isAlternateBuffer ? ( - /* Shadow the global provider to maintain isolation in ASB mode. */ - {content} - ) : ( - content - ); }; diff --git a/packages/cli/src/ui/components/messages/GeminiMessageContent.tsx b/packages/cli/src/ui/components/messages/GeminiMessageContent.tsx index f3ac6c7749..1aed5e1950 100644 --- a/packages/cli/src/ui/components/messages/GeminiMessageContent.tsx +++ b/packages/cli/src/ui/components/messages/GeminiMessageContent.tsx @@ -7,9 +7,7 @@ import type React from 'react'; import { Box } from 'ink'; import { MarkdownDisplay } from '../../utils/MarkdownDisplay.js'; -import { ShowMoreLines } from '../ShowMoreLines.js'; import { useUIState } from '../../contexts/UIStateContext.js'; -import { useAlternateBuffer } from '../../hooks/useAlternateBuffer.js'; interface GeminiMessageContentProps { text: string; @@ -31,7 +29,6 @@ export const GeminiMessageContent: React.FC = ({ terminalWidth, }) => { const { renderMarkdown } = useUIState(); - const isAlternateBuffer = useAlternateBuffer(); const originalPrefix = '✦ '; const prefixWidth = originalPrefix.length; @@ -41,18 +38,13 @@ export const GeminiMessageContent: React.FC = ({ text={text} isPending={isPending} availableTerminalHeight={ - isAlternateBuffer || availableTerminalHeight === undefined + availableTerminalHeight === undefined ? undefined : Math.max(availableTerminalHeight - 1, 1) } terminalWidth={Math.max(terminalWidth - prefixWidth, 0)} renderMarkdown={renderMarkdown} /> - - - ); }; diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx index 056e6a54b4..8971d488d3 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx @@ -6,7 +6,6 @@ import { renderWithProviders } from '../../../test-utils/render.js'; import { describe, it, expect, vi, afterEach } from 'vitest'; -import { act } from 'react'; import { ToolGroupMessage } from './ToolGroupMessage.js'; import type { HistoryItem, @@ -767,200 +766,4 @@ describe('', () => { }, ); }); - - describe('Manual Overflow Detection', () => { - it('detects overflow for string results exceeding available height', async () => { - const toolCalls = [ - createToolCall({ - resultDisplay: 'line 1\nline 2\nline 3\nline 4\nline 5', - }), - ]; - const { lastFrame, unmount, waitUntilReady } = renderWithProviders( - , - { - config: baseMockConfig, - settings: fullVerbositySettings, - useAlternateBuffer: true, - uiState: { - constrainHeight: true, - }, - }, - ); - await waitUntilReady(); - expect(lastFrame()?.toLowerCase()).toContain( - 'press ctrl+o to show more lines', - ); - unmount(); - }); - - it('detects overflow for array results exceeding available height', async () => { - // resultDisplay when array is expected to be AnsiLine[] - // AnsiLine is AnsiToken[] - const toolCalls = [ - createToolCall({ - resultDisplay: Array(5).fill([{ text: 'line', fg: 'default' }]), - }), - ]; - const { lastFrame, unmount, waitUntilReady } = renderWithProviders( - , - { - config: baseMockConfig, - settings: fullVerbositySettings, - useAlternateBuffer: true, - uiState: { - constrainHeight: true, - }, - }, - ); - await waitUntilReady(); - expect(lastFrame()?.toLowerCase()).toContain( - 'press ctrl+o to show more lines', - ); - unmount(); - }); - - it('respects ACTIVE_SHELL_MAX_LINES for focused shell tools', async () => { - const toolCalls = [ - createToolCall({ - name: 'run_shell_command', - status: CoreToolCallStatus.Executing, - ptyId: 1, - resultDisplay: Array(20).fill('line').join('\n'), // 20 lines > 15 (limit) - }), - ]; - const { lastFrame, unmount, waitUntilReady } = renderWithProviders( - , - { - config: baseMockConfig, - settings: fullVerbositySettings, - useAlternateBuffer: true, - uiState: { - constrainHeight: true, - activePtyId: 1, - embeddedShellFocused: true, - }, - }, - ); - await waitUntilReady(); - expect(lastFrame()?.toLowerCase()).toContain( - 'press ctrl+o to show more lines', - ); - unmount(); - }); - - it('does not show expansion hint when content is within limits', async () => { - const toolCalls = [ - createToolCall({ - resultDisplay: 'small result', - }), - ]; - const { lastFrame, unmount, waitUntilReady } = renderWithProviders( - , - { - config: baseMockConfig, - settings: fullVerbositySettings, - useAlternateBuffer: true, - uiState: { - constrainHeight: true, - }, - }, - ); - await waitUntilReady(); - expect(lastFrame()).not.toContain('Press Ctrl+O to show more lines'); - unmount(); - }); - - it('hides expansion hint when constrainHeight is false', async () => { - const toolCalls = [ - createToolCall({ - resultDisplay: 'line 1\nline 2\nline 3\nline 4\nline 5', - }), - ]; - const { lastFrame, unmount, waitUntilReady } = renderWithProviders( - , - { - config: baseMockConfig, - settings: fullVerbositySettings, - useAlternateBuffer: true, - uiState: { - constrainHeight: false, - }, - }, - ); - await waitUntilReady(); - expect(lastFrame()).not.toContain('Press Ctrl+O to show more lines'); - unmount(); - }); - - it('isolates overflow hint in ASB mode (ignores global overflow state)', async () => { - // In this test, the tool output is SHORT (no local overflow). - // We will inject a dummy ID into the global overflow state. - // ToolGroupMessage should still NOT show the hint because it calculates - // overflow locally and passes it as a prop. - const toolCalls = [ - createToolCall({ - resultDisplay: 'short result', - }), - ]; - const { lastFrame, unmount, waitUntilReady, capturedOverflowActions } = - renderWithProviders( - , - { - config: baseMockConfig, - settings: fullVerbositySettings, - useAlternateBuffer: true, - uiState: { - constrainHeight: true, - }, - }, - ); - await waitUntilReady(); - - // Manually trigger a global overflow - act(() => { - expect(capturedOverflowActions).toBeDefined(); - capturedOverflowActions!.addOverflowingId('unrelated-global-id'); - }); - - // The hint should NOT appear because ToolGroupMessage is isolated by its prop logic - expect(lastFrame()).not.toContain('Press Ctrl+O to show more lines'); - unmount(); - }); - }); }); diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx index 5ec2a18e06..05f9984d69 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx @@ -17,18 +17,12 @@ import { ToolMessage } from './ToolMessage.js'; import { ShellToolMessage } from './ShellToolMessage.js'; import { theme } from '../../semantic-colors.js'; import { useConfig } from '../../contexts/ConfigContext.js'; -import { isShellTool, isThisShellFocused } from './ToolShared.js'; +import { isShellTool } from './ToolShared.js'; import { shouldHideToolCall, CoreToolCallStatus, } from '@google/gemini-cli-core'; -import { ShowMoreLines } from '../ShowMoreLines.js'; import { useUIState } from '../../contexts/UIStateContext.js'; -import { useAlternateBuffer } from '../../hooks/useAlternateBuffer.js'; -import { - calculateShellMaxLines, - calculateToolContentMaxLines, -} from '../../utils/toolLayoutUtils.js'; import { getToolGroupBorderAppearance } from '../../utils/borderStyles.js'; import { useSettings } from '../../contexts/SettingsContext.js'; @@ -83,13 +77,11 @@ export const ToolGroupMessage: React.FC = ({ const config = useConfig(); const { - constrainHeight, activePtyId, embeddedShellFocused, backgroundShells, pendingHistoryItems, } = useUIState(); - const isAlternateBuffer = useAlternateBuffer(); const { borderColor, borderDimColor } = useMemo( () => @@ -149,72 +141,6 @@ export const ToolGroupMessage: React.FC = ({ const contentWidth = terminalWidth - TOOL_MESSAGE_HORIZONTAL_MARGIN; - /* - * ToolGroupMessage calculates its own overflow state locally and passes - * it as a prop to ShowMoreLines. This isolates it from global overflow - * reports in ASB mode, while allowing it to contribute to the global - * 'Toast' hint in Standard mode. - * - * Because of this prop-based isolation and the explicit mode-checks in - * AppContainer, we do not need to shadow the OverflowProvider here. - */ - const hasOverflow = useMemo(() => { - if (!availableTerminalHeightPerToolMessage) return false; - return visibleToolCalls.some((tool) => { - const isShellToolCall = isShellTool(tool.name); - const isFocused = isThisShellFocused( - tool.name, - tool.status, - tool.ptyId, - activePtyId, - embeddedShellFocused, - ); - - let maxLines: number | undefined; - - if (isShellToolCall) { - maxLines = calculateShellMaxLines({ - status: tool.status, - isAlternateBuffer, - isThisShellFocused: isFocused, - availableTerminalHeight: availableTerminalHeightPerToolMessage, - constrainHeight, - isExpandable, - }); - } - - // Standard tools and Shell tools both eventually use ToolResultDisplay's logic. - // ToolResultDisplay uses calculateToolContentMaxLines to find the final line budget. - const contentMaxLines = calculateToolContentMaxLines({ - availableTerminalHeight: availableTerminalHeightPerToolMessage, - isAlternateBuffer, - maxLinesLimit: maxLines, - }); - - if (!contentMaxLines) return false; - - if (typeof tool.resultDisplay === 'string') { - const text = tool.resultDisplay; - const hasTrailingNewline = text.endsWith('\n'); - const contentText = hasTrailingNewline ? text.slice(0, -1) : text; - const lineCount = contentText.split('\n').length; - return lineCount > contentMaxLines; - } - if (Array.isArray(tool.resultDisplay)) { - return tool.resultDisplay.length > contentMaxLines; - } - return false; - }); - }, [ - visibleToolCalls, - availableTerminalHeightPerToolMessage, - activePtyId, - embeddedShellFocused, - isAlternateBuffer, - constrainHeight, - isExpandable, - ]); - // If all tools are filtered out (e.g., in-progress AskUser tools, confirming tools), // only render if we need to close a border from previous // tool groups. borderBottomOverride=true means we must render the closing border; @@ -307,12 +233,6 @@ export const ToolGroupMessage: React.FC = ({ /> ) } - {(borderBottomOverride ?? true) && visibleToolCalls.length > 0 && ( - - )} ); diff --git a/packages/cli/src/ui/components/messages/ToolOverflowConsistencyChecks.test.tsx b/packages/cli/src/ui/components/messages/ToolOverflowConsistencyChecks.test.tsx index a82132d0d8..20b8d13459 100644 --- a/packages/cli/src/ui/components/messages/ToolOverflowConsistencyChecks.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolOverflowConsistencyChecks.test.tsx @@ -8,21 +8,19 @@ import { describe, it, expect } from 'vitest'; import { ToolGroupMessage } from './ToolGroupMessage.js'; import { renderWithProviders } from '../../../test-utils/render.js'; import { StreamingState, type IndividualToolCallDisplay } from '../../types.js'; -import { OverflowProvider } from '../../contexts/OverflowContext.js'; import { waitFor } from '../../../test-utils/async.js'; import { CoreToolCallStatus } from '@google/gemini-cli-core'; +import { useOverflowState } from '../../contexts/OverflowContext.js'; describe('ToolOverflowConsistencyChecks: ToolGroupMessage and ToolResultDisplay synchronization', () => { - it('should ensure explicit hasOverflow calculation is consistent with ToolResultDisplay truncation in Alternate Buffer (ASB) mode', async () => { + it('should ensure ToolGroupMessage correctly reports overflow to the global state in Alternate Buffer (ASB) mode', async () => { /** * Logic: - * 1. availableTerminalHeight(13) - staticHeight(3) = 10 lines per tool. - * 2. ASB mode reserves 1 + 6 = 7 lines. - * 3. Line budget = 10 - 7 = 3 lines. - * 4. 5 lines of output > 3 lines budget => hasOverflow should be TRUE. + * 1. availableTerminalHeight(13) - staticHeight(1) - ASB Reserved(6) = 6 lines per tool. + * 2. 10 lines of output > 6 lines budget => hasOverflow should be TRUE. */ - const lines = Array.from({ length: 5 }, (_, i) => `line ${i + 1}`); + const lines = Array.from({ length: 10 }, (_, i) => `line ${i + 1}`); const resultDisplay = lines.join('\n'); const toolCalls: IndividualToolCallDisplay[] = [ @@ -36,8 +34,15 @@ describe('ToolOverflowConsistencyChecks: ToolGroupMessage and ToolResultDisplay }, ]; - const { lastFrame } = renderWithProviders( - + let latestOverflowState: ReturnType; + const StateCapture = () => { + latestOverflowState = useOverflowState(); + return null; + }; + + const { unmount, waitUntilReady } = renderWithProviders( + <> + - , + , { uiState: { streamingState: StreamingState.Idle, @@ -55,24 +60,26 @@ describe('ToolOverflowConsistencyChecks: ToolGroupMessage and ToolResultDisplay }, ); - // In ASB mode, the hint should appear because hasOverflow is now correctly calculated. - await waitFor(() => - expect(lastFrame()?.toLowerCase()).toContain( - 'press ctrl+o to show more lines', - ), - ); + await waitUntilReady(); + + // To verify that the overflow state was indeed updated by the Scrollable component. + await waitFor(() => { + expect(latestOverflowState?.overflowingIds.size).toBeGreaterThan(0); + }); + + unmount(); }); - it('should ensure explicit hasOverflow calculation is consistent with ToolResultDisplay truncation in Standard mode', async () => { + it('should ensure ToolGroupMessage correctly reports overflow in Standard mode', async () => { /** * Logic: - * 1. availableTerminalHeight(13) - staticHeight(3) = 10 lines per tool. - * 2. Standard mode reserves 1 + 2 = 3 lines. - * 3. Line budget = 10 - 3 = 7 lines. - * 4. 9 lines of output > 7 lines budget => hasOverflow should be TRUE. + * 1. availableTerminalHeight(13) passed to ToolGroupMessage. + * 2. ToolGroupMessage subtracts its static height (2) => 11 lines available for tools. + * 3. ToolResultDisplay gets 11 lines, subtracts static height (1) and Standard Reserved (2) => 8 lines. + * 4. 15 lines of output > 8 lines budget => hasOverflow should be TRUE. */ - const lines = Array.from({ length: 9 }, (_, i) => `line ${i + 1}`); + const lines = Array.from({ length: 15 }, (_, i) => `line ${i + 1}`); const resultDisplay = lines.join('\n'); const toolCalls: IndividualToolCallDisplay[] = [ @@ -86,16 +93,14 @@ describe('ToolOverflowConsistencyChecks: ToolGroupMessage and ToolResultDisplay }, ]; - const { lastFrame } = renderWithProviders( - - - , + const { lastFrame, unmount, waitUntilReady } = renderWithProviders( + , { uiState: { streamingState: StreamingState.Idle, @@ -105,11 +110,11 @@ describe('ToolOverflowConsistencyChecks: ToolGroupMessage and ToolResultDisplay }, ); + await waitUntilReady(); + // Verify truncation is occurring (standard mode uses MaxSizedBox) await waitFor(() => expect(lastFrame()).toContain('hidden (Ctrl+O')); - // In Standard mode, ToolGroupMessage calculates hasOverflow correctly now. - // While Standard mode doesn't render the inline hint (ShowMoreLines returns null), - // the logic inside ToolGroupMessage is now synchronized. + unmount(); }); }); diff --git a/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx b/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx index 1c29407e91..05b94442db 100644 --- a/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx +++ b/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx @@ -236,6 +236,7 @@ export const ToolResultDisplay: React.FC = ({ maxHeight={maxLines ?? availableHeight} hasFocus={hasFocus} // Allow scrolling via keyboard (Shift+Up/Down) scrollToBottom={true} + reportOverflow={true} > {content} diff --git a/packages/cli/src/ui/components/messages/ToolResultDisplayOverflow.test.tsx b/packages/cli/src/ui/components/messages/ToolResultDisplayOverflow.test.tsx deleted file mode 100644 index 2dff7d25e7..0000000000 --- a/packages/cli/src/ui/components/messages/ToolResultDisplayOverflow.test.tsx +++ /dev/null @@ -1,79 +0,0 @@ -/** - * @license - * Copyright 2026 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import { describe, it, expect } from 'vitest'; -import { ToolGroupMessage } from './ToolGroupMessage.js'; -import { renderWithProviders } from '../../../test-utils/render.js'; -import { StreamingState, type IndividualToolCallDisplay } from '../../types.js'; -import { waitFor } from '../../../test-utils/async.js'; -import { CoreToolCallStatus } from '@google/gemini-cli-core'; - -describe('ToolResultDisplay Overflow', () => { - it('should display "press ctrl-o" hint when content overflows in ToolGroupMessage', async () => { - // Large output that will definitely overflow - const lines = []; - for (let i = 0; i < 50; i++) { - lines.push(`line ${i + 1}`); - } - const resultDisplay = lines.join('\n'); - - const toolCalls: IndividualToolCallDisplay[] = [ - { - callId: 'call-1', - name: 'test-tool', - description: 'a test tool', - status: CoreToolCallStatus.Success, - resultDisplay, - confirmationDetails: undefined, - }, - ]; - - const { lastFrame, waitUntilReady } = renderWithProviders( - , - { - uiState: { - streamingState: StreamingState.Idle, - constrainHeight: true, - }, - useAlternateBuffer: true, - }, - ); - - await waitUntilReady(); - - // In ASB mode the overflow hint can render before the scroll position - // settles. Wait for both the hint and the tail of the content so this - // snapshot is deterministic across slower CI runners. - await waitFor(() => { - const frame = lastFrame(); - expect(frame).toBeDefined(); - expect(frame?.toLowerCase()).toContain('press ctrl+o to show more lines'); - expect(frame).toContain('line 50'); - }); - - const frame = lastFrame(); - expect(frame).toBeDefined(); - if (frame) { - expect(frame.toLowerCase()).toContain('press ctrl+o to show more lines'); - // Ensure it's AFTER the bottom border - const linesOfOutput = frame.split('\n'); - const bottomBorderIndex = linesOfOutput.findLastIndex((l) => - l.includes('╰─'), - ); - const hintIndex = linesOfOutput.findIndex((l) => - l.toLowerCase().includes('press ctrl+o to show more lines'), - ); - expect(hintIndex).toBeGreaterThan(bottomBorderIndex); - expect(frame).toMatchSnapshot(); - } - }); -}); diff --git a/packages/cli/src/ui/components/shared/Scrollable.tsx b/packages/cli/src/ui/components/shared/Scrollable.tsx index 87ec6e72d6..a7227c7087 100644 --- a/packages/cli/src/ui/components/shared/Scrollable.tsx +++ b/packages/cli/src/ui/components/shared/Scrollable.tsx @@ -5,13 +5,22 @@ */ import type React from 'react'; -import { useState, useRef, useCallback, useMemo, useLayoutEffect } from 'react'; +import { + useState, + useRef, + useCallback, + useMemo, + useLayoutEffect, + useEffect, + useId, +} from 'react'; import { Box, ResizeObserver, type DOMElement } from 'ink'; import { useKeypress, type Key } from '../../hooks/useKeypress.js'; import { useScrollable } from '../../contexts/ScrollProvider.js'; import { useAnimatedScrollbar } from '../../hooks/useAnimatedScrollbar.js'; import { useBatchedScroll } from '../../hooks/useBatchedScroll.js'; import { keyMatchers, Command } from '../../keyMatchers.js'; +import { useOverflowActions } from '../../contexts/OverflowContext.js'; interface ScrollableProps { children?: React.ReactNode; @@ -22,6 +31,7 @@ interface ScrollableProps { hasFocus: boolean; scrollToBottom?: boolean; flexGrow?: number; + reportOverflow?: boolean; } export const Scrollable: React.FC = ({ @@ -33,10 +43,13 @@ export const Scrollable: React.FC = ({ hasFocus, scrollToBottom, flexGrow, + reportOverflow = false, }) => { const [scrollTop, setScrollTop] = useState(0); const viewportRef = useRef(null); const contentRef = useRef(null); + const overflowActions = useOverflowActions(); + const id = useId(); const [size, setSize] = useState({ innerHeight: typeof height === 'number' ? height : 0, scrollHeight: 0, @@ -52,6 +65,27 @@ export const Scrollable: React.FC = ({ scrollTopRef.current = scrollTop; }, [scrollTop]); + useEffect(() => { + if (reportOverflow && size.scrollHeight > size.innerHeight) { + overflowActions?.addOverflowingId?.(id); + } else { + overflowActions?.removeOverflowingId?.(id); + } + }, [ + reportOverflow, + size.scrollHeight, + size.innerHeight, + id, + overflowActions, + ]); + + useEffect( + () => () => { + overflowActions?.removeOverflowingId?.(id); + }, + [id, overflowActions], + ); + const viewportObserverRef = useRef(null); const contentObserverRef = useRef(null);