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.
2026-04-02 17:31:05 -07:00 · 2026-03-26 01:04:34 -07:00
parent e036fc3bc2
commit db5d7ee1bd
7 changed files with 130 additions and 3 deletions
--- 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(
+        <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();
+    });
+  });
 });
--- 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<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,
--- 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<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;
--- 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
@@ -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>
--- 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
@@ -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>
--- 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
 "
--- a/packages/cli/src/ui/utils/fileUtils.ts
+++ 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;
+}