From db5d7ee1bd1b8398f76ea89a811e74bab2577981 Mon Sep 17 00:00:00 2001 From: Jarrod Whelan <150866123+jwhelangoog@users.noreply.github.com> Date: Thu, 26 Mar 2026 01:04:34 -0700 Subject: [PATCH] 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. --- .../messages/DenseToolMessage.test.tsx | 50 +++++++++++++++++++ .../components/messages/DenseToolMessage.tsx | 5 +- .../ui/components/messages/DiffRenderer.tsx | 5 +- ...snapshot-for-a-Rejected-tool-call.snap.svg | 11 ++++ ...ccepted-file-edit-with-diff-stats.snap.svg | 33 ++++++++++++ .../DenseToolMessage.test.tsx.snap | 10 ++++ packages/cli/src/ui/utils/fileUtils.ts | 19 +++++++ 7 files changed, 130 insertions(+), 3 deletions(-) create mode 100644 packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage-DenseToolMessage-Visual-Regression-matches-SVG-snapshot-for-a-Rejected-tool-call.snap.svg create mode 100644 packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage-DenseToolMessage-Visual-Regression-matches-SVG-snapshot-for-an-Accepted-file-edit-with-diff-stats.snap.svg create mode 100644 packages/cli/src/ui/utils/fileUtils.ts diff --git a/packages/cli/src/ui/components/messages/DenseToolMessage.test.tsx b/packages/cli/src/ui/components/messages/DenseToolMessage.test.tsx index a4b3120b3f..221d702ef6 100644 --- a/packages/cli/src/ui/components/messages/DenseToolMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/DenseToolMessage.test.tsx @@ -522,4 +522,54 @@ describe('DenseToolMessage', () => { expect(lastFrame()).not.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( + , + ); + + await renderResult.waitUntilReady(); + await expect(renderResult).toMatchSvgSnapshot(); + }); + + it('matches SVG snapshot for a Rejected tool call', async () => { + const renderResult = await renderWithProviders( + , + ); + + await renderResult.waitUntilReady(); + await expect(renderResult).toMatchSvgSnapshot(); + }); + }); }); diff --git a/packages/cli/src/ui/components/messages/DenseToolMessage.tsx b/packages/cli/src/ui/components/messages/DenseToolMessage.tsx index 6dc9497d25..1c5f53d7a8 100644 --- a/packages/cli/src/ui/components/messages/DenseToolMessage.tsx +++ b/packages/cli/src/ui/components/messages/DenseToolMessage.tsx @@ -33,6 +33,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; @@ -455,7 +456,9 @@ export const DenseToolMessage: React.FC = (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, diff --git a/packages/cli/src/ui/components/messages/DiffRenderer.tsx b/packages/cli/src/ui/components/messages/DiffRenderer.tsx index fec44bc19a..2a0d5b39c4 100644 --- a/packages/cli/src/ui/components/messages/DiffRenderer.tsx +++ b/packages/cli/src/ui/components/messages/DiffRenderer.tsx @@ -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 = ({ .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; diff --git a/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage-DenseToolMessage-Visual-Regression-matches-SVG-snapshot-for-a-Rejected-tool-call.snap.svg b/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage-DenseToolMessage-Visual-Regression-matches-SVG-snapshot-for-a-Rejected-tool-call.snap.svg new file mode 100644 index 0000000000..96d89e7416 --- /dev/null +++ b/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage-DenseToolMessage-Visual-Regression-matches-SVG-snapshot-for-a-Rejected-tool-call.snap.svg @@ -0,0 +1,11 @@ + + + + + - + read_file + Reading important.txt + + \ No newline at end of file diff --git a/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage-DenseToolMessage-Visual-Regression-matches-SVG-snapshot-for-an-Accepted-file-edit-with-diff-stats.snap.svg b/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage-DenseToolMessage-Visual-Regression-matches-SVG-snapshot-for-an-Accepted-file-edit-with-diff-stats.snap.svg new file mode 100644 index 0000000000..7b21bd65a0 --- /dev/null +++ b/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage-DenseToolMessage-Visual-Regression-matches-SVG-snapshot-for-an-Accepted-file-edit-with-diff-stats.snap.svg @@ -0,0 +1,33 @@ + + + + + ✓ + edit + test.ts + → Accepted + ( + +1 + , + -1 + ) + + 1 + + + - + + + old + + 1 + + + + + + + new + + \ No newline at end of file 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 index e7cb9cb195..93c2dad9fd 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage.test.tsx.snap @@ -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 " diff --git a/packages/cli/src/ui/utils/fileUtils.ts b/packages/cli/src/ui/utils/fileUtils.ts new file mode 100644 index 0000000000..a1f3472aa4 --- /dev/null +++ b/packages/cli/src/ui/utils/fileUtils.ts @@ -0,0 +1,19 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as path from 'node:path'; + +/** + * Gets the file extension from a filename or path, excluding the leading dot. + * Returns null if no extension is found. + */ +export function getFileExtension( + filename: string | null | undefined, +): string | null { + if (!filename) return null; + const ext = path.extname(filename); + return ext ? ext.slice(1) : null; +}