refactor(cli): code review cleanup fix for tab+tab (#18967)

2026-04-04 18:31:36 -07:00 · 2026-02-17 07:16:37 -08:00
parent e5ff2023ad
commit 366f1df120
14 changed files with 334 additions and 197 deletions
--- a/packages/cli/src/ui/components/Composer.tsx
+++ b/packages/cli/src/ui/components/Composer.tsx
@@ -8,7 +8,7 @@ import { useState, useEffect, useMemo } from 'react';
 import { Box, Text, useIsScreenReaderEnabled } from 'ink';
 import {
  ApprovalMode,
-  tokenLimit,
+  checkExhaustive,
  CoreToolCallStatus,
 } from '@google/gemini-cli-core';
 import { LoadingIndicator } from './LoadingIndicator.js';
@@ -38,6 +38,7 @@ import { StreamingState, type HistoryItemToolGroup } from '../types.js';
 import { ConfigInitDisplay } from '../components/ConfigInitDisplay.js';
 import { TodoTray } from './messages/Todo.js';
 import { getInlineThinkingMode } from '../utils/inlineThinkingMode.js';
+import { isContextUsageHigh } from '../utils/contextUsage.js';
 import { theme } from '../semantic-colors.js';

 export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => {
@@ -114,30 +115,41 @@ export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => {
  const showApprovalIndicator =
    !uiState.shellModeActive && !hideUiDetailsForSuggestions;
  const showRawMarkdownIndicator = !uiState.renderMarkdown;
-  const modeBleedThrough =
-    showApprovalModeIndicator === ApprovalMode.YOLO
-      ? { text: 'YOLO', color: theme.status.error }
-      : showApprovalModeIndicator === ApprovalMode.PLAN
-        ? { text: 'plan', color: theme.status.success }
-        : showApprovalModeIndicator === ApprovalMode.AUTO_EDIT
-          ? { text: 'auto edit', color: theme.status.warning }
-          : null;
+  let modeBleedThrough: { text: string; color: string } | null = null;
+  switch (showApprovalModeIndicator) {
+    case ApprovalMode.YOLO:
+      modeBleedThrough = { text: 'YOLO', color: theme.status.error };
+      break;
+    case ApprovalMode.PLAN:
+      modeBleedThrough = { text: 'plan', color: theme.status.success };
+      break;
+    case ApprovalMode.AUTO_EDIT:
+      modeBleedThrough = { text: 'auto edit', color: theme.status.warning };
+      break;
+    case ApprovalMode.DEFAULT:
+      modeBleedThrough = null;
+      break;
+    default:
+      checkExhaustive(showApprovalModeIndicator);
+      modeBleedThrough = null;
+      break;
+  }
+
  const hideMinimalModeHintWhileBusy =
    !showUiDetails && (showLoadingIndicator || hasPendingActionRequired);
  const minimalModeBleedThrough = hideMinimalModeHintWhileBusy
    ? null
    : modeBleedThrough;
  const hasMinimalStatusBleedThrough = shouldShowToast(uiState);
-  const contextTokenLimit =
-    typeof uiState.currentModel === 'string' && uiState.currentModel.length > 0
-      ? tokenLimit(uiState.currentModel)
-      : 0;
+
  const showMinimalContextBleedThrough =
    !settings.merged.ui.footer.hideContextPercentage &&
-    typeof uiState.currentModel === 'string' &&
-    uiState.currentModel.length > 0 &&
-    contextTokenLimit > 0 &&
-    uiState.sessionStats.lastPromptTokenCount / contextTokenLimit > 0.6;
+    isContextUsageHigh(
+      uiState.sessionStats.lastPromptTokenCount,
+      typeof uiState.currentModel === 'string'
+        ? uiState.currentModel
+        : undefined,
+    );
  const hideShortcutsHintForSuggestions = hideUiDetailsForSuggestions;
  const showShortcutsHint =
    settings.merged.ui.showShortcutsHint &&
--- a/packages/cli/src/ui/components/ContextUsageDisplay.tsx
+++ b/packages/cli/src/ui/components/ContextUsageDisplay.tsx
@@ -6,7 +6,7 @@

 import { Text } from 'ink';
 import { theme } from '../semantic-colors.js';
-import { tokenLimit } from '@google/gemini-cli-core';
+import { getContextUsagePercentage } from '../utils/contextUsage.js';

 export const ContextUsageDisplay = ({
  promptTokenCount,
@@ -17,7 +17,7 @@ export const ContextUsageDisplay = ({
  model: string;
  terminalWidth: number;
 }) => {
-  const percentage = promptTokenCount / tokenLimit(model);
+  const percentage = getContextUsagePercentage(promptTokenCount, model);
  const percentageLeft = ((1 - percentage) * 100).toFixed(0);

  const label = terminalWidth < 100 ? '%' : '% context left';
--- a/packages/cli/src/ui/components/GeminiRespondingSpinner.test.tsx
+++ b/packages/cli/src/ui/components/GeminiRespondingSpinner.test.tsx
@@ -33,10 +33,15 @@ describe('GeminiRespondingSpinner', () => {
  const mockUseIsScreenReaderEnabled = vi.mocked(useIsScreenReaderEnabled);

  beforeEach(() => {
+    vi.useFakeTimers();
    vi.clearAllMocks();
    mockUseIsScreenReaderEnabled.mockReturnValue(false);
  });

+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
  it('renders spinner when responding', () => {
    mockUseStreamingContext.mockReturnValue(StreamingState.Responding);
    const { lastFrame } = render(<GeminiRespondingSpinner />);
--- a/packages/cli/src/ui/components/InputPrompt.tsx
+++ b/packages/cli/src/ui/components/InputPrompt.tsx
@@ -76,6 +76,7 @@ import { useMouse, type MouseEvent } from '../contexts/MouseContext.js';
 import { useUIActions } from '../contexts/UIActionsContext.js';
 import { useAlternateBuffer } from '../hooks/useAlternateBuffer.js';
 import { shouldDismissShortcutsHelpOnHotkey } from '../utils/shortcutsHelp.js';
+import { useRepeatedKeyPress } from '../hooks/useRepeatedKeyPress.js';

 /**
 * Returns if the terminal can be trusted to handle paste events atomically
@@ -227,10 +228,31 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
    shortcutsHelpVisible,
  } = useUIState();
  const [suppressCompletion, setSuppressCompletion] = useState(false);
-  const escPressCount = useRef(0);
-  const lastPlainTabPressTimeRef = useRef<number | null>(null);
+  const { handlePress: registerPlainTabPress, resetCount: resetPlainTabPress } =
+    useRepeatedKeyPress({
+      windowMs: DOUBLE_TAB_CLEAN_UI_TOGGLE_WINDOW_MS,
+    });
  const [showEscapePrompt, setShowEscapePrompt] = useState(false);
-  const escapeTimerRef = useRef<NodeJS.Timeout | null>(null);
+  const { handlePress: handleEscPress, resetCount: resetEscapeState } =
+    useRepeatedKeyPress({
+      windowMs: 500,
+      onRepeat: (count) => {
+        if (count === 1) {
+          setShowEscapePrompt(true);
+        } else if (count === 2) {
+          resetEscapeState();
+          if (buffer.text.length > 0) {
+            buffer.setText('');
+            resetCompletionState();
+          } else if (history.length > 0) {
+            onSubmit('/rewind');
+          } else {
+            coreEvents.emitFeedback('info', 'Nothing to rewind to');
+          }
+        }
+      },
+      onReset: () => setShowEscapePrompt(false),
+    });
  const [recentUnsafePasteTime, setRecentUnsafePasteTime] = useState<
    number | null
  >(null);
@@ -284,15 +306,6 @@ export const InputPrompt: React.FC<InputPromptProps> = ({

  const showCursor = focus && isShellFocused && !isEmbeddedShellFocused;

-  const resetEscapeState = useCallback(() => {
-    if (escapeTimerRef.current) {
-      clearTimeout(escapeTimerRef.current);
-      escapeTimerRef.current = null;
-    }
-    escPressCount.current = 0;
-    setShowEscapePrompt(false);
-  }, []);
-
  // Notify parent component about escape prompt state changes
  useEffect(() => {
    if (onEscapePromptChange) {
@@ -300,12 +313,9 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
    }
  }, [showEscapePrompt, onEscapePromptChange]);

-  // Clear escape prompt timer on unmount
+  // Clear paste timeout on unmount
  useEffect(
    () => () => {
-      if (escapeTimerRef.current) {
-        clearTimeout(escapeTimerRef.current);
-      }
      if (pasteTimeoutRef.current) {
        clearTimeout(pasteTimeoutRef.current);
      }
@@ -335,8 +345,8 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
      resetReverseSearchCompletionState();
    },
    [
-      onSubmit,
      buffer,
+      onSubmit,
      resetCompletionState,
      shellModeActive,
      shellHistory,
@@ -639,22 +649,16 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
        commandSearchActive;
      if (isPlainTab) {
        if (!hasTabCompletionInteraction) {
-          const now = Date.now();
-          const isDoubleTabPress =
-            lastPlainTabPressTimeRef.current !== null &&
-            now - lastPlainTabPressTimeRef.current <=
-              DOUBLE_TAB_CLEAN_UI_TOGGLE_WINDOW_MS;
-          if (isDoubleTabPress) {
-            lastPlainTabPressTimeRef.current = null;
+          if (registerPlainTabPress() === 2) {
            toggleCleanUiDetailsVisible();
+            resetPlainTabPress();
            return true;
          }
-          lastPlainTabPressTimeRef.current = now;
        } else {
-          lastPlainTabPressTimeRef.current = null;
+          resetPlainTabPress();
        }
      } else {
-        lastPlainTabPressTimeRef.current = null;
+        resetPlainTabPress();
      }

      if (key.name === 'paste') {
@@ -732,9 +736,7 @@ export const InputPrompt: React.FC<InputPromptProps> = ({

      // Reset ESC count and hide prompt on any non-ESC key
      if (key.name !== 'escape') {
-        if (escPressCount.current > 0 || showEscapePrompt) {
-          resetEscapeState();
-        }
+        resetEscapeState();
      }

      // Ctrl+O to expand/collapse paste placeholders
@@ -798,30 +800,7 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
          return true;
        }

-        // Handle double ESC
-        if (escPressCount.current === 0) {
-          escPressCount.current = 1;
-          setShowEscapePrompt(true);
-          if (escapeTimerRef.current) {
-            clearTimeout(escapeTimerRef.current);
-          }
-          escapeTimerRef.current = setTimeout(() => {
-            resetEscapeState();
-          }, 500);
-          return true;
-        }
-
-        // Second ESC
-        resetEscapeState();
-        if (buffer.text.length > 0) {
-          buffer.setText('');
-          resetCompletionState();
-          return true;
-        } else if (history.length > 0) {
-          onSubmit('/rewind');
-          return true;
-        }
-        coreEvents.emitFeedback('info', 'Nothing to rewind to');
+        handleEscPress();
        return true;
      }

@@ -1193,7 +1172,6 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
      reverseSearchCompletion,
      handleClipboardPaste,
      resetCompletionState,
-      showEscapePrompt,
      resetEscapeState,
      vimHandleInput,
      reverseSearchActive,
@@ -1205,16 +1183,17 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
      kittyProtocol.enabled,
      shortcutsHelpVisible,
      setShortcutsHelpVisible,
-      toggleCleanUiDetailsVisible,
      tryLoadQueuedMessages,
      setBannerVisible,
-      onSubmit,
      activePtyId,
      setEmbeddedShellFocused,
      backgroundShells.size,
      backgroundShellHeight,
-      history,
      streamingState,
+      handleEscPress,
+      registerPlainTabPress,
+      resetPlainTabPress,
+      toggleCleanUiDetailsVisible,
    ],
  );

--- a/packages/cli/src/ui/components/RewindViewer.test.tsx
+++ b/packages/cli/src/ui/components/RewindViewer.test.tsx
@@ -14,6 +14,10 @@ import type {
  MessageRecord,
 } from '@google/gemini-cli-core';

+vi.mock('./CliSpinner.js', () => ({
+  CliSpinner: () => 'MockSpinner',
+}));
+
 vi.mock('../utils/formatters.js', async (importOriginal) => {
  const original =
    await importOriginal<typeof import('../utils/formatters.js')>();
--- a/packages/cli/src/ui/components/messages/ToolMessage.test.tsx
+++ b/packages/cli/src/ui/components/messages/ToolMessage.test.tsx
@@ -13,6 +13,10 @@ import { type AnsiOutput, CoreToolCallStatus } from '@google/gemini-cli-core';
 import { renderWithProviders } from '../../../test-utils/render.js';
 import { tryParseJSON } from '../../../utils/jsonoutput.js';

+vi.mock('../GeminiRespondingSpinner.js', () => ({
+  GeminiRespondingSpinner: () => <Text>MockRespondingSpinner</Text>,
+}));
+
 vi.mock('../TerminalOutput.js', () => ({
  TerminalOutput: function MockTerminalOutput({
    cursor,
--- a/packages/cli/src/ui/components/messages/snapshots/ToolMessage.test.tsx.snap
+++ b/packages/cli/src/ui/components/messages/snapshots/ToolMessage.test.tsx.snap
@@ -26,7 +26,7 @@ exports[`<ToolMessage /> > ToolStatusIndicator rendering > shows - for Canceled

 exports[`<ToolMessage /> > ToolStatusIndicator rendering > shows MockRespondingSpinner for Executing status when streamingState is Responding 1`] = `
 "╭──────────────────────────────────────────────────────────────────────────────╮
-│ ⊶  test-tool A tool for testing                                              │
+│ MockRespondingSpinnertest-tool A tool for testing                            │
 │                                                                              │
 │ Test result                                                                  │"
 `;
@@ -40,14 +40,14 @@ exports[`<ToolMessage /> > ToolStatusIndicator rendering > shows o for Pending s

 exports[`<ToolMessage /> > ToolStatusIndicator rendering > shows paused spinner for Executing status when streamingState is Idle 1`] = `
 "╭──────────────────────────────────────────────────────────────────────────────╮
-│ ⊷  test-tool A tool for testing                                              │
+│ MockRespondingSpinnertest-tool A tool for testing                            │
 │                                                                              │
 │ Test result                                                                  │"
 `;

 exports[`<ToolMessage /> > ToolStatusIndicator rendering > shows paused spinner for Executing status when streamingState is WaitingForConfirmation 1`] = `
 "╭──────────────────────────────────────────────────────────────────────────────╮
-│ ⊷  test-tool A tool for testing                                              │
+│ MockRespondingSpinnertest-tool A tool for testing                            │
 │                                                                              │
 │ Test result                                                                  │"
 `;
--- a/packages/cli/src/ui/components/shared/ScrollableList.test.tsx
+++ b/packages/cli/src/ui/components/shared/ScrollableList.test.tsx
@@ -138,8 +138,10 @@ describe('ScrollableList Demo Behavior', () => {
    let listRef: ScrollableListRef<Item> | null = null;
    let lastFrame: () => string | undefined;

+    let result: ReturnType<typeof render>;
+
    await act(async () => {
-      const result = render(
+      result = render(
        <TestComponent
          onAddItem={(add) => {
            addItem = add;
@@ -192,6 +194,10 @@ describe('ScrollableList Demo Behavior', () => {
      expect(lastFrame!()).toContain('Count: 1003');
    });
    expect(lastFrame!()).not.toContain('Item 1003');
+
+    await act(async () => {
+      result.unmount();
+    });
  });

  it('should display sticky header when scrolled past the item', async () => {
@@ -243,8 +249,9 @@ describe('ScrollableList Demo Behavior', () => {
    };

    let lastFrame: () => string | undefined;
+    let result: ReturnType<typeof render>;
    await act(async () => {
-      const result = render(<StickyTestComponent />);
+      result = render(<StickyTestComponent />);
      lastFrame = result.lastFrame;
    });

@@ -286,6 +293,10 @@ describe('ScrollableList Demo Behavior', () => {
      expect(lastFrame!()).toContain('[Normal] Item 1');
    });
    expect(lastFrame!()).not.toContain('[STICKY] Item 1');
+
+    await act(async () => {
+      result.unmount();
+    });
  });

  describe('Keyboard Navigation', () => {
@@ -299,8 +310,9 @@ describe('ScrollableList Demo Behavior', () => {
        title: `Item ${i}`,
      }));

+      let result: ReturnType<typeof render>;
      await act(async () => {
-        const result = render(
+        result = render(
          <MouseProvider mouseEventsEnabled={false}>
            <KeypressProvider>
              <ScrollProvider>
@@ -378,6 +390,10 @@ describe('ScrollableList Demo Behavior', () => {
      await waitFor(() => {
        expect(listRef?.getScrollState()?.scrollTop).toBe(0);
      });
+
+      await act(async () => {
+        result.unmount();
+      });
    });
  });

@@ -386,8 +402,9 @@ describe('ScrollableList Demo Behavior', () => {
      const items = [{ id: '1', title: 'Item 1' }];
      let lastFrame: () => string | undefined;

+      let result: ReturnType<typeof render>;
      await act(async () => {
-        const result = render(
+        result = render(
          <MouseProvider mouseEventsEnabled={false}>
            <KeypressProvider>
              <ScrollProvider>
@@ -411,6 +428,10 @@ describe('ScrollableList Demo Behavior', () => {
      await waitFor(() => {
        expect(lastFrame()).toContain('Item 1');
      });
+
+      await act(async () => {
+        result.unmount();
+      });
    });
  });
 });