refactor(cli): address nuanced snapshot rendering scenarios through layout and padding refinements

- Refined height calculation logic in ToolGroupMessage to ensure consistent spacing between compact and standard tools.
- Adjusted padding and margins in StickyHeader, ToolConfirmationQueue, ShellToolMessage, and ToolMessage for visual alignment.
- Updated TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT to account for internal layout changes.
- Improved ToolResultDisplay height handling in alternate buffer mode.
- Updated test snapshots to reflect layout and spacing corrections.

refactor(cli): cleanup and simplify UI components

- Reduced UI refresh delay in AppContainer.tsx for a more responsive user experience.
- Reorder imports and hook definitions within AppContainer.tsx to reduce diff 'noise'.

refactor(cli): enhance compact output robustness and visual regression testing

Addressing automated review feedback to improve code maintainability and layout stability.

1. Robust File Extension Parsing:
- Introduced getFileExtension utility in packages/cli/src/ui/utils/fileUtils.ts using node:path for reliable extension extraction.
- Updated DenseToolMessage and DiffRenderer to use the new utility, replacing fragile string splitting.

2. Visual Regression Coverage:
- Added SVG snapshot tests to DenseToolMessage.test.tsx to verify semantic color rendering and layout integrity in compact mode.

fix(cli): resolve dense tool output code quality issues

- Replaced manual string truncation with Ink's `wrap="truncate-end"` to adhere to UI guidelines.
- Added `isReadManyFilesResult` type guard to `packages/core/src/tools/tools.ts` to improve typing for structured tool results.
- Fixed an incomplete test case in `DenseToolMessage.test.tsx` to properly simulate expansion via context instead of missing mouse events.
This commit is contained in:
Jarrod Whelan
2026-03-25 16:07:59 -07:00
parent 7d786f5f58
commit 09a6667c35
24 changed files with 224 additions and 108 deletions
@@ -54,9 +54,8 @@ describe('DenseToolMessage', () => {
/>,
);
await waitUntilReady();
// Remove all whitespace to check the continuous string content truncation
const output = lastFrame()?.replace(/\s/g, '');
expect(output).toContain('A'.repeat(117) + '...');
const output = lastFrame();
expect(output).toContain('');
expect(lastFrame()).toMatchSnapshot();
});
@@ -503,7 +502,7 @@ describe('DenseToolMessage', () => {
expect(output).toMatchSnapshot();
});
it('shows diff content after clicking summary', async () => {
it('shows diff content when expanded via ToolActionsContext', async () => {
const { lastFrame, waitUntilReady } = await renderWithProviders(
<DenseToolMessage
{...defaultProps}
@@ -513,13 +512,65 @@ describe('DenseToolMessage', () => {
{
config: makeFakeConfig({ useAlternateBuffer: true }),
settings: createMockSettings({ ui: { useAlternateBuffer: true } }),
mouseEventsEnabled: true,
toolActions: {
isExpanded: () => true,
},
},
);
await waitUntilReady();
// Verify it's hidden initially
expect(lastFrame()).not.toContain('new line');
// Verify it shows the diff when expanded
expect(lastFrame()).toContain('new line');
});
});
describe('Visual Regression', () => {
it('matches SVG snapshot for an Accepted file edit with diff stats', async () => {
const diffResult: FileDiff = {
fileName: 'test.ts',
filePath: '/mock/test.ts',
fileDiff: '--- a/test.ts\n+++ b/test.ts\n@@ -1 +1 @@\n-old\n+new',
originalContent: 'old',
newContent: 'new',
diffStat: {
model_added_lines: 1,
model_removed_lines: 1,
model_added_chars: 3,
model_removed_chars: 3,
user_added_lines: 0,
user_removed_lines: 0,
user_added_chars: 0,
user_removed_chars: 0,
},
};
const renderResult = await renderWithProviders(
<DenseToolMessage
{...defaultProps}
name="edit"
description="Editing test.ts"
resultDisplay={diffResult as ToolResultDisplay}
status={CoreToolCallStatus.Success}
/>,
);
await renderResult.waitUntilReady();
await expect(renderResult).toMatchSvgSnapshot();
});
it('matches SVG snapshot for a Rejected tool call', async () => {
const renderResult = await renderWithProviders(
<DenseToolMessage
{...defaultProps}
name="read_file"
description="Reading important.txt"
resultDisplay="Rejected by user"
status={CoreToolCallStatus.Cancelled}
/>,
);
await renderResult.waitUntilReady();
await expect(renderResult).toMatchSvgSnapshot();
});
});
});
@@ -16,6 +16,7 @@ import {
hasSummary,
isGrepResult,
isListResult,
isReadManyFilesResult,
} from '@google/gemini-cli-core';
import { type IndividualToolCallDisplay, isTodoList } from '../../types.js';
import { useAlternateBuffer } from '../../hooks/useAlternateBuffer.js';
@@ -33,6 +34,7 @@ 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';
import { getFileExtension } from '../../utils/fileUtils.js';
interface DenseToolMessageProps extends IndividualToolCallDisplay {
terminalWidth?: number;
@@ -242,14 +244,10 @@ function getListResultData(
result: ListDirectoryResult | ReadManyFilesResult,
originalDescription?: string,
): ViewParts {
// Use 'include' to determine if this is a ReadManyFilesResult
if ('include' in result) {
if (isReadManyFilesResult(result)) {
return getReadManyFilesData(result);
}
return getListDirectoryData(
result as ListDirectoryResult,
originalDescription,
);
return getListDirectoryData(result, originalDescription);
}
function getGenericSuccessData(
@@ -268,8 +266,8 @@ function getGenericSuccessData(
if (typeof resultDisplay === 'string') {
const flattened = resultDisplay.replace(/\n/g, ' ').trim();
summary = (
<Text color={theme.text.accent} wrap="wrap">
{flattened.length > 120 ? flattened.slice(0, 117) + '...' : flattened}
<Text color={theme.text.accent} wrap="truncate-end">
{flattened}
</Text>
);
} else if (isGrepResult(resultDisplay)) {
@@ -406,8 +404,8 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
? resultDisplay.replace(/\n/g, ' ')
: 'Failed';
const errorSummary = (
<Text color={theme.status.error} wrap="wrap">
{text.length > 120 ? text.slice(0, 117) + '...' : text}
<Text color={theme.status.error} wrap="truncate-end">
{text}
</Text>
);
const descriptionText = originalDescription ? (
@@ -455,7 +453,9 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
.filter((line) => line.type === 'add')
.map((line) => line.content)
.join('\n');
const fileExtension = diff.fileName?.split('.').pop() || null;
const fileExtension = getFileExtension(diff.fileName);
return colorizeCode({
code: addedContent,
language: fileExtension,
@@ -13,6 +13,7 @@ import { MaxSizedBox } from '../shared/MaxSizedBox.js';
import { theme as semanticTheme } from '../../semantic-colors.js';
import type { Theme } from '../../themes/theme.js';
import { useSettings } from '../../contexts/SettingsContext.js';
import { getFileExtension } from '../../utils/fileUtils.js';
export interface DiffLine {
type: 'add' | 'del' | 'context' | 'hunk' | 'other';
@@ -150,7 +151,7 @@ export const DiffRenderer: React.FC<DiffRendererProps> = ({
.map((line) => line.content)
.join('\n');
// Attempt to infer language from filename, default to plain text if no filename
const fileExtension = filename?.split('.').pop() || null;
const fileExtension = getFileExtension(filename);
const language = fileExtension
? getLanguageFromExtension(fileExtension)
: null;
@@ -259,7 +260,7 @@ export const renderDiffLines = ({
);
const gutterWidth = Math.max(1, maxLineNumber.toString().length);
const fileExtension = filename?.split('.').pop() || null;
const fileExtension = getFileExtension(filename);
const language = fileExtension
? getLanguageFromExtension(fileExtension)
: null;
@@ -190,6 +190,7 @@ export const ShellToolMessage: React.FC<ShellToolMessageProps> = ({
borderLeft={true}
borderRight={true}
paddingX={1}
paddingTop={1}
flexDirection="column"
>
<ToolResultDisplay
@@ -414,7 +414,7 @@ describe('<ToolGroupMessage />', () => {
];
const item = createItem(toolCalls);
const { lastFrame, unmount } = await renderWithProviders(
<Scrollable height={10} hasFocus={true} scrollToBottom={true}>
<Scrollable height={12} hasFocus={true} scrollToBottom={true}>
<ToolGroupMessage {...baseProps} item={item} toolCalls={toolCalls} />
</Scrollable>,
{
@@ -222,38 +222,41 @@ export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({
!Array.isArray(prevGroup) &&
isCompactTool(prevGroup, isCompactModeEnabled);
if (Array.isArray(group)) {
const isAgentGroup = Array.isArray(group);
const isCompact =
!isAgentGroup && isCompactTool(group, isCompactModeEnabled);
if (isFirst) {
height += (borderTopOverride ?? false) ? 1 : 0;
} else if (isCompact && prevIsCompact) {
height += 0;
} else if (isCompact || prevIsCompact) {
height += 1;
} else {
// Gap is provided by StickyHeader's paddingTop=1
height += 0;
}
const isFirstProp = !!(isFirst
? (borderTopOverride ?? true)
: prevIsCompact);
if (isAgentGroup) {
// 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)
}
// Static overhead for standard tool header:
// 1 line for header text
// 1 line for dark separator
// 1 line for tool body internal paddingTop=1
// 1 line for top border (if isFirstProp) OR StickyHeader paddingTop=1 (if !isFirstProp)
height += 3;
height += isFirstProp ? 1 : 1; // Either top border or paddingTop
}
}
}
@@ -352,12 +355,14 @@ export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({
let marginTop = 0;
if (isFirst) {
// marginTop = (borderTopOverride ?? true) ? 1 : 0;
marginTop = (borderTopOverride ?? false) ? 1 : 0;
} else if (isCompact && prevIsCompact) {
marginTop = 0;
} else {
} else if (isCompact || prevIsCompact) {
marginTop = 1;
} else {
// Subsequent standard tools: StickyHeader's paddingTop=1 provides the gap.
marginTop = 0;
}
const isFirstProp = !!(isFirst
@@ -119,6 +119,7 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
borderLeft={true}
borderRight={true}
paddingX={1}
paddingTop={1}
flexDirection="column"
>
{status === CoreToolCallStatus.Executing && progress !== undefined && (
@@ -179,10 +179,16 @@ export const ToolResultDisplay: React.FC<ToolResultDisplayProps> = ({
// Final render based on session mode
if (isAlternateBuffer) {
// If availableTerminalHeight is undefined, we don't have a fixed budget,
// so if maxLines is also undefined, we shouldn't cap the height at all.
const effectiveMaxHeight =
maxLines ??
(availableTerminalHeight !== undefined ? availableHeight : undefined);
return (
<Scrollable
width={childWidth}
maxHeight={maxLines ?? availableHeight}
maxHeight={effectiveMaxHeight}
hasFocus={hasFocus} // Allow scrolling via keyboard (Shift+Up/Down)
scrollToBottom={true}
reportOverflow={true}
@@ -0,0 +1,11 @@
<svg xmlns="http://www.w3.org/2000/svg" width="920" height="37" viewBox="0 0 920 37">
<style>
text { font-family: Consolas, "Courier New", monospace; font-size: 14px; dominant-baseline: text-before-edge; white-space: pre; }
</style>
<rect width="920" height="37" fill="#000000" />
<g transform="translate(10, 10)">
<text x="18" y="2" fill="#ffffaf" textLength="9" lengthAdjust="spacingAndGlyphs" font-weight="bold">-</text>
<text x="45" y="2" fill="#ffffff" textLength="90" lengthAdjust="spacingAndGlyphs" font-weight="bold">read_file </text>
<text x="144" y="2" fill="#afafaf" textLength="189" lengthAdjust="spacingAndGlyphs">Reading important.txt</text>
</g>
</svg>

After

Width:  |  Height:  |  Size: 693 B

@@ -0,0 +1,33 @@
<svg xmlns="http://www.w3.org/2000/svg" width="920" height="88" viewBox="0 0 920 88">
<style>
text { font-family: Consolas, "Courier New", monospace; font-size: 14px; dominant-baseline: text-before-edge; white-space: pre; }
</style>
<rect width="920" height="88" fill="#000000" />
<g transform="translate(10, 10)">
<text x="18" y="2" fill="#d7ffd7" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="45" y="2" fill="#ffffff" textLength="45" lengthAdjust="spacingAndGlyphs" font-weight="bold">edit </text>
<text x="99" y="2" fill="#afafaf" textLength="63" lengthAdjust="spacingAndGlyphs">test.ts</text>
<text x="171" y="2" fill="#d7afff" textLength="90" lengthAdjust="spacingAndGlyphs">→ Accepted</text>
<text x="270" y="2" fill="#afafaf" textLength="9" lengthAdjust="spacingAndGlyphs">(</text>
<text x="279" y="2" fill="#d7ffd7" textLength="18" lengthAdjust="spacingAndGlyphs">+1</text>
<text x="297" y="2" fill="#afafaf" textLength="18" lengthAdjust="spacingAndGlyphs">, </text>
<text x="315" y="2" fill="#ff87af" textLength="18" lengthAdjust="spacingAndGlyphs">-1</text>
<text x="333" y="2" fill="#afafaf" textLength="9" lengthAdjust="spacingAndGlyphs">)</text>
<rect x="54" y="34" width="9" height="17" fill="#5f0000" />
<text x="54" y="36" fill="#afafaf" textLength="9" lengthAdjust="spacingAndGlyphs">1</text>
<rect x="63" y="34" width="9" height="17" fill="#5f0000" />
<rect x="72" y="34" width="9" height="17" fill="#5f0000" />
<text x="72" y="36" fill="#ff87af" textLength="9" lengthAdjust="spacingAndGlyphs">-</text>
<rect x="81" y="34" width="9" height="17" fill="#5f0000" />
<rect x="90" y="34" width="27" height="17" fill="#5f0000" />
<text x="90" y="36" fill="#e5e5e5" textLength="27" lengthAdjust="spacingAndGlyphs">old</text>
<rect x="54" y="51" width="9" height="17" fill="#005f00" />
<text x="54" y="53" fill="#afafaf" textLength="9" lengthAdjust="spacingAndGlyphs">1</text>
<rect x="63" y="51" width="9" height="17" fill="#005f00" />
<rect x="72" y="51" width="9" height="17" fill="#005f00" />
<text x="72" y="53" fill="#d7ffd7" textLength="9" lengthAdjust="spacingAndGlyphs">+</text>
<rect x="81" y="51" width="9" height="17" fill="#005f00" />
<rect x="90" y="51" width="27" height="17" fill="#005f00" />
<text x="90" y="53" fill="#0000ee" textLength="27" lengthAdjust="spacingAndGlyphs">new</text>
</g>
</svg>

After

Width:  |  Height:  |  Size: 2.4 KiB

@@ -13,6 +13,16 @@ exports[`DenseToolMessage > Toggleable Diff View (Alternate Buffer) > shows diff
"
`;
exports[`DenseToolMessage > Visual Regression > matches SVG snapshot for a Rejected tool call 1`] = `" - read_file Reading important.txt"`;
exports[`DenseToolMessage > Visual Regression > matches SVG snapshot for an Accepted file edit with diff stats 1`] = `
" ✓ edit test.ts → Accepted (+1, -1)
1 - old
1 + new
"
`;
exports[`DenseToolMessage > does not render result arrow if resultDisplay is missing 1`] = `
" o test-tool Test description
"
@@ -128,8 +138,6 @@ exports[`DenseToolMessage > renders generic output message for unknown object re
exports[`DenseToolMessage > truncates long string results 1`] = `
" ✓ test-tool Test description
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAA...
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA…
"
`;
@@ -4,7 +4,6 @@ exports[`<ShellToolMessage /> > Height Constraints > defaults to ACTIVE_SHELL_MA
"╭──────────────────────────────────────────────────────────────────────────────╮
│ ⊶ Shell Command A shell command │
│ │
│ Line 90 │
│ Line 91 │
│ Line 92 │
│ Line 93 │
@@ -129,7 +128,6 @@ exports[`<ShellToolMessage /> > Height Constraints > respects availableTerminalH
"╭──────────────────────────────────────────────────────────────────────────────╮
│ ⊶ Shell Command A shell command │
│ │
│ Line 94 │
│ Line 95 │
│ Line 96 │
│ Line 97 │
@@ -143,7 +141,6 @@ exports[`<ShellToolMessage /> > Height Constraints > stays constrained in altern
"╭──────────────────────────────────────────────────────────────────────────────╮
│ ✓ Shell Command A shell command │
│ │
│ Line 90 │
│ Line 91 │
│ Line 92 │
│ Line 93 │
@@ -161,7 +158,6 @@ exports[`<ShellToolMessage /> > Height Constraints > uses ACTIVE_SHELL_MAX_LINES
"╭──────────────────────────────────────────────────────────────────────────────╮
│ ⊶ Shell Command A shell command │
│ │
│ Line 90 │
│ Line 91 │
│ Line 92 │
│ Line 93 │
@@ -179,11 +175,10 @@ exports[`<ShellToolMessage /> > Height Constraints > uses full availableTerminal
"╭──────────────────────────────────────────────────────────────────────────────╮
│ ⊶ Shell Command A shell command (Shift+Tab to unfocus) │
│ │
│ Line 4 │
│ Line 5 │
│ Line 6 │
│ Line 7
│ Line 8
│ Line 7
│ Line 8
│ Line 9 █ │
│ Line 10 █ │
│ Line 11 █ │
@@ -32,7 +32,6 @@ exports[`<ToolGroupMessage /> > Border Color Logic > uses gray border when all t
│ ✓ test-tool A tool for testing │
│ │
│ Test result │
│ │
│ ✓ another-tool A tool for testing │
│ │
@@ -62,10 +61,12 @@ exports[`<ToolGroupMessage /> > Golden Snapshots > renders canceled tool calls >
exports[`<ToolGroupMessage /> > Golden Snapshots > renders empty tool calls array 1`] = `""`;
exports[`<ToolGroupMessage /> > 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…
│──────────────────────────────────────────────────────────────────────────│ ▄
line4
│ line5 │ █
│ │ █
│ ✓ tool-2 Description 2 │ █
│ │ █
│ line1 │ █
@@ -80,12 +81,10 @@ exports[`<ToolGroupMessage /> > 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 │
│ │
@@ -99,12 +98,10 @@ exports[`<ToolGroupMessage /> > 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 │
│ │
@@ -147,7 +144,6 @@ exports[`<ToolGroupMessage /> > 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 │
│ │
@@ -170,12 +166,10 @@ exports[`<ToolGroupMessage /> > 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 │
│ │
@@ -37,8 +37,7 @@ exports[`ToolResultDisplay > renders string result as plain text when renderOutp
`;
exports[`ToolResultDisplay > truncates very long string results 1`] = `
"... 249 hidden (Ctrl+O) ...
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
"... 250 hidden (Ctrl+O) ...
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
@@ -40,7 +40,7 @@ exports[`ToolMessage Sticky Header Regression > verifies that multiple ToolMessa
"│ │
│ ✓ tool-2 Description for tool-2 │
│────────────────────────────────────────────────────────────────────────│
│ c2-09
│ c2-10 │ ▀
│ c2-10 │
╰────────────────────────────────────────────────────────────────────────╯ █
"
`;