From 0b2d79a2eaf2dedea33db8134f60d443761656b8 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Sat, 27 Sep 2025 12:40:09 -0700 Subject: [PATCH] fix(ui): stop truncating output from the model rendered in (#9972) --- packages/cli/src/test-utils/render.tsx | 25 +++- .../ui/components/HistoryItemDisplay.test.tsx | 100 +++++++++++-- .../src/ui/components/HistoryItemDisplay.tsx | 10 +- .../cli/src/ui/components/MainContent.tsx | 7 + .../PermissionsModifyTrustDialog.test.tsx | 5 - .../HistoryItemDisplay.test.tsx.snap | 137 ++++++++++++++++++ .../cli/src/ui/utils/MarkdownDisplay.test.tsx | 109 +++++--------- 7 files changed, 295 insertions(+), 98 deletions(-) create mode 100644 packages/cli/src/ui/components/__snapshots__/HistoryItemDisplay.test.tsx.snap diff --git a/packages/cli/src/test-utils/render.tsx b/packages/cli/src/test-utils/render.tsx index 90372a7d91..690d765d80 100644 --- a/packages/cli/src/test-utils/render.tsx +++ b/packages/cli/src/test-utils/render.tsx @@ -6,17 +6,30 @@ import { render } from 'ink-testing-library'; import type React from 'react'; +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'; +const mockSettings = new LoadedSettings( + { path: '', settings: {}, originalSettings: {} }, + { path: '', settings: {}, originalSettings: {} }, + { path: '', settings: {}, originalSettings: {} }, + { path: '', settings: {}, originalSettings: {} }, + true, + new Set(), +); + export const renderWithProviders = ( component: React.ReactElement, - { shellFocus = true } = {}, + { shellFocus = true, settings = mockSettings } = {}, ): ReturnType => render( - - - {component} - - , + + + + {component} + + + , ); diff --git a/packages/cli/src/ui/components/HistoryItemDisplay.test.tsx b/packages/cli/src/ui/components/HistoryItemDisplay.test.tsx index a5fc4558f6..d0c28fa11f 100644 --- a/packages/cli/src/ui/components/HistoryItemDisplay.test.tsx +++ b/packages/cli/src/ui/components/HistoryItemDisplay.test.tsx @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { render } from 'ink-testing-library'; import { describe, it, expect, vi } from 'vitest'; import { HistoryItemDisplay } from './HistoryItemDisplay.js'; import { type HistoryItem, ToolCallStatus } from '../types.js'; @@ -15,6 +14,7 @@ import type { ToolExecuteConfirmationDetails, } from '@google/gemini-cli-core'; import { ToolGroupMessage } from './messages/ToolGroupMessage.js'; +import { renderWithProviders } from '../../test-utils/render.js'; // Mock child components vi.mock('./messages/ToolGroupMessage.js', () => ({ @@ -37,7 +37,7 @@ describe('', () => { type: MessageType.USER, text: 'Hello', }; - const { lastFrame } = render( + const { lastFrame } = renderWithProviders( , ); expect(lastFrame()).toContain('Hello'); @@ -49,7 +49,7 @@ describe('', () => { type: MessageType.USER, text: '/theme', }; - const { lastFrame } = render( + const { lastFrame } = renderWithProviders( , ); expect(lastFrame()).toContain('/theme'); @@ -61,7 +61,7 @@ describe('', () => { type: MessageType.STATS, duration: '1s', }; - const { lastFrame } = render( + const { lastFrame } = renderWithProviders( , @@ -81,7 +81,7 @@ describe('', () => { gcpProject: 'test-project', ideClient: 'test-ide', }; - const { lastFrame } = render( + const { lastFrame } = renderWithProviders( , ); expect(lastFrame()).toContain('About Gemini CLI'); @@ -92,7 +92,7 @@ describe('', () => { ...baseItem, type: 'model_stats', }; - const { lastFrame } = render( + const { lastFrame } = renderWithProviders( , @@ -107,7 +107,7 @@ describe('', () => { ...baseItem, type: 'tool_stats', }; - const { lastFrame } = render( + const { lastFrame } = renderWithProviders( , @@ -123,7 +123,7 @@ describe('', () => { type: 'quit', duration: '1s', }; - const { lastFrame } = render( + const { lastFrame } = renderWithProviders( , @@ -138,7 +138,7 @@ describe('', () => { text: 'Hello, \u001b[31mred\u001b[0m world!', }; - const { lastFrame } = render( + const { lastFrame } = renderWithProviders( ', () => { ], }; - render( + renderWithProviders( ', () => { 'echo "\\u001b[31mhello\\u001b[0m"', ); }); + + const longCode = + '# Example code block:\n' + + '```python\n' + + Array.from({ length: 50 }, (_, i) => `Line ${i + 1}`).join('\n') + + '\n```'; + + it('should render a truncated gemini item', () => { + const item: HistoryItem = { + id: 1, + type: 'gemini', + text: longCode, + }; + const { lastFrame } = renderWithProviders( + , + ); + + expect(lastFrame()).toMatchSnapshot(); + }); + + it('should render a full gemini item when using availableTerminalHeightGemini', () => { + const item: HistoryItem = { + id: 1, + type: 'gemini', + text: longCode, + }; + const { lastFrame } = renderWithProviders( + , + ); + + expect(lastFrame()).toMatchSnapshot(); + }); + + it('should render a truncated gemini_content item', () => { + const item: HistoryItem = { + id: 1, + type: 'gemini_content', + text: longCode, + }; + const { lastFrame } = renderWithProviders( + , + ); + + expect(lastFrame()).toMatchSnapshot(); + }); + + it('should render a full gemini_content item when using availableTerminalHeightGemini', () => { + const item: HistoryItem = { + id: 1, + type: 'gemini_content', + text: longCode, + }; + const { lastFrame } = renderWithProviders( + , + ); + + expect(lastFrame()).toMatchSnapshot(); + }); }); diff --git a/packages/cli/src/ui/components/HistoryItemDisplay.tsx b/packages/cli/src/ui/components/HistoryItemDisplay.tsx index bfca4845b6..4322a05ab4 100644 --- a/packages/cli/src/ui/components/HistoryItemDisplay.tsx +++ b/packages/cli/src/ui/components/HistoryItemDisplay.tsx @@ -36,6 +36,7 @@ interface HistoryItemDisplayProps { commands?: readonly SlashCommand[]; activeShellPtyId?: number | null; embeddedShellFocused?: boolean; + availableTerminalHeightGemini?: number; } export const HistoryItemDisplay: React.FC = ({ @@ -47,6 +48,7 @@ export const HistoryItemDisplay: React.FC = ({ isFocused = true, activeShellPtyId, embeddedShellFocused, + availableTerminalHeightGemini, }) => { const itemForDisplay = useMemo(() => escapeAnsiCtrlCodes(item), [item]); @@ -63,7 +65,9 @@ export const HistoryItemDisplay: React.FC = ({ )} @@ -71,7 +75,9 @@ export const HistoryItemDisplay: React.FC = ({ )} diff --git a/packages/cli/src/ui/components/MainContent.tsx b/packages/cli/src/ui/components/MainContent.tsx index dfb0ba6e64..fc6c216fac 100644 --- a/packages/cli/src/ui/components/MainContent.tsx +++ b/packages/cli/src/ui/components/MainContent.tsx @@ -12,6 +12,12 @@ import { useUIState } from '../contexts/UIStateContext.js'; import { useAppContext } from '../contexts/AppContext.js'; import { AppHeader } from './AppHeader.js'; +// Limit Gemini messages to a very high number of lines to mitigate performance +// issues in the worst case if we somehow get an enormous response from Gemini. +// This threshold is arbitrary but should be high enough to never impact normal +// usage. +const MAX_GEMINI_MESSAGE_LINES = 65536; + export const MainContent = () => { const { version } = useAppContext(); const uiState = useUIState(); @@ -32,6 +38,7 @@ export const MainContent = () => { vi.fn()); const mockedLoadTrustedFolders = vi.hoisted(() => vi.fn()); const mockedIsWorkspaceTrusted = vi.hoisted(() => vi.fn()); -const mockedUseSettings = vi.hoisted(() => vi.fn()); // Mock the modules themselves vi.mock('node:process', async (importOriginal) => { @@ -40,10 +39,6 @@ vi.mock('../../config/trustedFolders.js', () => ({ }, })); -vi.mock('../contexts/SettingsContext.js', () => ({ - useSettings: mockedUseSettings, -})); - vi.mock('../hooks/usePermissionsModifyTrust.js'); describe('PermissionsModifyTrustDialog', () => { diff --git a/packages/cli/src/ui/components/__snapshots__/HistoryItemDisplay.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/HistoryItemDisplay.test.tsx.snap new file mode 100644 index 0000000000..de25e153e7 --- /dev/null +++ b/packages/cli/src/ui/components/__snapshots__/HistoryItemDisplay.test.tsx.snap @@ -0,0 +1,137 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[` > should render a full gemini item when using availableTerminalHeightGemini 1`] = ` +"✦ Example code block: + 1 Line 1 + 2 Line 2 + 3 Line 3 + 4 Line 4 + 5 Line 5 + 6 Line 6 + 7 Line 7 + 8 Line 8 + 9 Line 9 + 10 Line 10 + 11 Line 11 + 12 Line 12 + 13 Line 13 + 14 Line 14 + 15 Line 15 + 16 Line 16 + 17 Line 17 + 18 Line 18 + 19 Line 19 + 20 Line 20 + 21 Line 21 + 22 Line 22 + 23 Line 23 + 24 Line 24 + 25 Line 25 + 26 Line 26 + 27 Line 27 + 28 Line 28 + 29 Line 29 + 30 Line 30 + 31 Line 31 + 32 Line 32 + 33 Line 33 + 34 Line 34 + 35 Line 35 + 36 Line 36 + 37 Line 37 + 38 Line 38 + 39 Line 39 + 40 Line 40 + 41 Line 41 + 42 Line 42 + 43 Line 43 + 44 Line 44 + 45 Line 45 + 46 Line 46 + 47 Line 47 + 48 Line 48 + 49 Line 49 + 50 Line 50" +`; + +exports[` > should render a full gemini_content item when using availableTerminalHeightGemini 1`] = ` +" Example code block: + 1 Line 1 + 2 Line 2 + 3 Line 3 + 4 Line 4 + 5 Line 5 + 6 Line 6 + 7 Line 7 + 8 Line 8 + 9 Line 9 + 10 Line 10 + 11 Line 11 + 12 Line 12 + 13 Line 13 + 14 Line 14 + 15 Line 15 + 16 Line 16 + 17 Line 17 + 18 Line 18 + 19 Line 19 + 20 Line 20 + 21 Line 21 + 22 Line 22 + 23 Line 23 + 24 Line 24 + 25 Line 25 + 26 Line 26 + 27 Line 27 + 28 Line 28 + 29 Line 29 + 30 Line 30 + 31 Line 31 + 32 Line 32 + 33 Line 33 + 34 Line 34 + 35 Line 35 + 36 Line 36 + 37 Line 37 + 38 Line 38 + 39 Line 39 + 40 Line 40 + 41 Line 41 + 42 Line 42 + 43 Line 43 + 44 Line 44 + 45 Line 45 + 46 Line 46 + 47 Line 47 + 48 Line 48 + 49 Line 49 + 50 Line 50" +`; + +exports[` > should render a truncated gemini item 1`] = ` +"✦ Example code block: + ... first 41 lines hidden ... + 42 Line 42 + 43 Line 43 + 44 Line 44 + 45 Line 45 + 46 Line 46 + 47 Line 47 + 48 Line 48 + 49 Line 49 + 50 Line 50" +`; + +exports[` > should render a truncated gemini_content item 1`] = ` +" Example code block: + ... first 41 lines hidden ... + 42 Line 42 + 43 Line 43 + 44 Line 44 + 45 Line 45 + 46 Line 46 + 47 Line 47 + 48 Line 48 + 49 Line 49 + 50 Line 50" +`; diff --git a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx index 80a339b3b1..634888b07c 100644 --- a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx +++ b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx @@ -4,12 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { render } from 'ink-testing-library'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { MarkdownDisplay } from './MarkdownDisplay.js'; import { LoadedSettings } from '../../config/settings.js'; -import { SettingsContext } from '../contexts/SettingsContext.js'; import { EOL } from 'node:os'; +import { renderWithProviders } from '../../test-utils/render.js'; describe('', () => { const baseProps = { @@ -18,34 +17,21 @@ describe('', () => { availableTerminalHeight: 40, }; - const mockSettings = new LoadedSettings( - { path: '', settings: {}, originalSettings: {} }, - { path: '', settings: {}, originalSettings: {} }, - { path: '', settings: {}, originalSettings: {} }, - { path: '', settings: {}, originalSettings: {} }, - true, - new Set(), - ); - beforeEach(() => { vi.clearAllMocks(); }); it('renders nothing for empty text', () => { - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); it('renders a simple paragraph', () => { const text = 'Hello, world.'; - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); @@ -57,10 +43,8 @@ describe('', () => { ### Header 3 #### Header 4 `.replace(/\n/g, EOL); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); @@ -70,30 +54,24 @@ describe('', () => { /\n/g, EOL, ); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); it('renders a fenced code block without a language', () => { const text = '```\nplain text\n```'.replace(/\n/g, EOL); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); it('handles unclosed (pending) code blocks', () => { const text = '```typescript\nlet y = 2;'.replace(/\n/g, EOL); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); @@ -104,10 +82,8 @@ describe('', () => { * item B + item C `.replace(/\n/g, EOL); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); @@ -118,10 +94,8 @@ describe('', () => { * Level 2 * Level 3 `.replace(/\n/g, EOL); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); @@ -131,10 +105,8 @@ describe('', () => { 1. First item 2. Second item `.replace(/\n/g, EOL); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); @@ -147,10 +119,8 @@ World *** Test `.replace(/\n/g, EOL); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); @@ -162,10 +132,8 @@ Test | Cell 1 | Cell 2 | | Cell 3 | Cell 4 | `.replace(/\n/g, EOL); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); @@ -176,10 +144,8 @@ Some text before. | A | B | |---| | 1 | 2 |`.replace(/\n/g, EOL); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); @@ -188,10 +154,8 @@ Some text before. const text = `Paragraph 1. Paragraph 2.`.replace(/\n/g, EOL); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); @@ -211,10 +175,8 @@ some code Another paragraph. `.replace(/\n/g, EOL); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); }); @@ -234,10 +196,9 @@ Another paragraph. new Set(), ); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , + { settings }, ); expect(lastFrame()).toMatchSnapshot(); expect(lastFrame()).not.toContain(' 1 '); @@ -245,10 +206,8 @@ Another paragraph. it('shows line numbers in code blocks by default', () => { const text = '```javascript\nconst x = 1;\n```'.replace(/\n/g, EOL); - const { lastFrame } = render( - - - , + const { lastFrame } = renderWithProviders( + , ); expect(lastFrame()).toMatchSnapshot(); expect(lastFrame()).toContain(' 1 ');