From b4c16d1f56f4e19fffd3d7608b410570f35045f9 Mon Sep 17 00:00:00 2001
From: Jacob Richman <jacob314@gmail.com>
Date: Sat, 24 May 2025 00:44:17 -0700
Subject: Code review comment fixes and some refactors. (#525)

No intentional different behavior aside for tweaks suggested from the code review of #506 Refactor: Extract console message logic to custom hook

This commit refactors the console message handling from App.tsx into a new custom hook useConsoleMessages.

This change improves the testability of the console message logic and declutters the main App component.

Created useConsoleMessages.ts to encapsulate console message state and update logic.
Updated App.tsx to utilize the new useConsoleMessages hook.
Added unit tests for useConsoleMessages.ts to ensure its functionality.
I deleted and started over on LoadingIndicator.test.tsx as I spent way too much time trying to fix it before just regenerating the tests as the code was easier to write tests for from scratch and the existing tests were not that good (I added them in the previous pull request).
---
 .../cli/src/ui/components/messages/ToolMessage.tsx | 77 ++++++++++------------
 1 file changed, 35 insertions(+), 42 deletions(-)

(limited to 'packages/cli/src/ui/components/messages/ToolMessage.tsx')

diff --git a/packages/cli/src/ui/components/messages/ToolMessage.tsx b/packages/cli/src/ui/components/messages/ToolMessage.tsx
index 49743190..775b4177 100644
--- a/packages/cli/src/ui/components/messages/ToolMessage.tsx
+++ b/packages/cli/src/ui/components/messages/ToolMessage.tsx
@@ -15,6 +15,7 @@ import {
 import { DiffRenderer } from './DiffRenderer.js';
 import { Colors } from '../../colors.js';
 import { MarkdownDisplay } from '../../utils/MarkdownDisplay.js';
+import { useStreamingContext } from '../../contexts/StreamingContext.js';
 
 const STATIC_HEIGHT = 1;
 const RESERVED_LINE_COUNT = 5; // for tool name, status, padding etc.
@@ -25,7 +26,6 @@ export type TextEmphasis = 'high' | 'medium' | 'low';
 export interface ToolMessageProps extends IndividualToolCallDisplay {
   availableTerminalHeight: number;
   emphasis?: TextEmphasis;
-  streamingState?: StreamingState;
 }
 
 export const ToolMessage: React.FC<ToolMessageProps> = ({
@@ -35,7 +35,6 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
   status,
   availableTerminalHeight,
   emphasis = 'medium',
-  streamingState,
 }) => {
   const contentHeightEstimate =
     availableTerminalHeight - STATIC_HEIGHT - RESERVED_LINE_COUNT;
@@ -63,7 +62,7 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
     <Box paddingX={1} paddingY={0} flexDirection="column">
       <Box minHeight={1}>
         {/* Status Indicator */}
-        <ToolStatusIndicator status={status} streamingState={streamingState} />
+        <ToolStatusIndicator status={status} />
         <ToolInfo
           name={name}
           status={status}
@@ -107,48 +106,42 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
 
 type ToolStatusIndicatorProps = {
   status: ToolCallStatus;
-  streamingState?: StreamingState;
 };
 const ToolStatusIndicator: React.FC<ToolStatusIndicatorProps> = ({
   status,
-  streamingState,
-}) => (
-  <Box minWidth={STATUS_INDICATOR_WIDTH}>
-    {status === ToolCallStatus.Pending && (
-      <Text color={Colors.AccentGreen}>o</Text>
-    )}
-    {status === ToolCallStatus.Executing &&
-      (streamingState === StreamingState.Responding ? (
-        // If the tool is responding that means the user has already confirmed
-        // this tool call, so we can show a checkmark. The call won't complete
-        // executing until all confirmations are done. Showing a spinner would
-        // be misleading as the task is not actually executing at the moment
-        // and also has flickering issues due to Ink rendering limitations.
-        // If this hack becomes a problem, we can always add an additional prop
-        // indicating that the tool was indeed confirmed. If the tool was not
-        // confirmed we could show a paused version of the spinner.
-        <Text color={Colors.Gray}>✔</Text>
-      ) : (
-        <Spinner type="dots" />
-      ))}
-    {status === ToolCallStatus.Success && (
-      <Text color={Colors.AccentGreen}>✔</Text>
-    )}
-    {status === ToolCallStatus.Confirming && (
-      <Text color={Colors.AccentYellow}>?</Text>
-    )}
-    {status === ToolCallStatus.Canceled && (
-      <Text color={Colors.AccentYellow} bold>
-        -
-      </Text>
-    )}
-    {status === ToolCallStatus.Error && (
-      <Text color={Colors.AccentRed} bold>
-        x
-      </Text>
-    )}
-  </Box>
-);
+}) => {
+  const { streamingState } = useStreamingContext();
+  return (
+    <Box minWidth={STATUS_INDICATOR_WIDTH}>
+      {status === ToolCallStatus.Pending && (
+        <Text color={Colors.AccentGreen}>o</Text>
+      )}
+      {status === ToolCallStatus.Executing &&
+        (streamingState === StreamingState.Responding ? (
+          <Spinner type="dots" />
+        ) : (
+          // Paused spinner to avoid flicker.
+          <Text>⠇</Text>
+        ))}
+      {status === ToolCallStatus.Success && (
+        <Text color={Colors.AccentGreen}>✔</Text>
+      )}
+      {status === ToolCallStatus.Confirming && (
+        <Text color={Colors.AccentYellow}>?</Text>
+      )}
+      {status === ToolCallStatus.Canceled && (
+        <Text color={Colors.AccentYellow} bold>
+          -
+        </Text>
+      )}
+      {status === ToolCallStatus.Error && (
+        <Text color={Colors.AccentRed} bold>
+          x
+        </Text>
+      )}
+    </Box>
+  );
+};
 
 type ToolInfo = {
   name: string;
-- 
cgit v1.2.3