diff options
17 files changed, 1190 insertions, 739 deletions
diff --git a/.vscode/launch.json b/.vscode/launch.json index 5a7c6522..b7976fd6 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -82,6 +82,46 @@ "console": "integratedTerminal", "internalConsoleOptions": "neverOpen", "skipFiles": ["<node_internals>/**"] + }, + { + "type": "node", + "request": "launch", + "name": "Debug useLoadingIndicator Test (CLI)", + "runtimeExecutable": "npm", + "runtimeArgs": [ + "run", + "test", + "-w", + "packages/cli", + "--", + "--inspect-brk=9229", + "--no-file-parallelism", + "${workspaceFolder}/packages/cli/src/ui/hooks/useLoadingIndicator.test.ts" + ], + "cwd": "${workspaceFolder}", + "console": "integratedTerminal", + "internalConsoleOptions": "neverOpen", + "skipFiles": ["<node_internals>/**"] + }, + { + "type": "node", + "request": "launch", + "name": "Debug LoadingIndicator Test (CLI)", + "runtimeExecutable": "npm", + "runtimeArgs": [ + "run", + "test", + "-w", + "packages/cli", + "--", + "--inspect-brk=9229", + "--no-file-parallelism", + "${workspaceFolder}/packages/cli/src/ui/components/LoadingIndicator.test.tsx" + ], + "cwd": "${workspaceFolder}", + "console": "integratedTerminal", + "internalConsoleOptions": "neverOpen", + "skipFiles": ["<node_internals>/**"] } ] } diff --git a/packages/cli/src/ui/App.tsx b/packages/cli/src/ui/App.tsx index 1453487f..5860e36e 100644 --- a/packages/cli/src/ui/App.tsx +++ b/packages/cli/src/ui/App.tsx @@ -41,6 +41,10 @@ import { useHistory } from './hooks/useHistoryManager.js'; import process from 'node:process'; import { getErrorMessage, type Config } from '@gemini-code/server'; import { useLogger } from './hooks/useLogger.js'; +import { + StreamingContext, + StreamingContextType, +} from './contexts/StreamingContext.js'; interface AppProps { config: Config; @@ -174,19 +178,15 @@ export const App = ({ handleSlashCommand, shellModeActive, ); - const isPausedForConfirmation = useMemo( - () => - pendingHistoryItems.some( - (item) => - item?.type === 'tool_group' && - item.tools.some((tool) => tool.status === 'Confirming'), - ), - [pendingHistoryItems], - ); - const { elapsedTime, currentLoadingPhrase, shouldShowSpinner } = - useLoadingIndicator(streamingState, isPausedForConfirmation); + const { elapsedTime, currentLoadingPhrase } = + useLoadingIndicator(streamingState); const showAutoAcceptIndicator = useAutoAcceptIndicator({ config }); + const streamingContextValue: StreamingContextType = useMemo( + () => ({ streamingState }), + [streamingState], + ); + const handleFinalSubmit = useCallback( (submittedValue: string) => { const trimmedValue = submittedValue.trim(); @@ -278,177 +278,176 @@ export const App = ({ }, [consoleMessages, config]); return ( - <Box flexDirection="column" marginBottom={1} width="90%"> - {/* - * The Static component is an Ink intrinsic in which there can only be 1 per application. - * Because of this restriction we're hacking it slightly by having a 'header' item here to - * ensure that it's statically rendered. - * - * Background on the Static Item: Anything in the Static component is written a single time - * to the console. Think of it like doing a console.log and then never using ANSI codes to - * clear that content ever again. Effectively it has a moving frame that every time new static - * content is set it'll flush content to the terminal and move the area which it's "clearing" - * down a notch. Without Static the area which gets erased and redrawn continuously grows. - */} - <Static - key={staticKey} - items={[ - <Box flexDirection="column" key="header"> - <Header /> - <Tips /> - </Box>, - ...history.map((h) => ( + <StreamingContext.Provider value={streamingContextValue}> + <Box flexDirection="column" marginBottom={1} width="90%"> + {/* + * The Static component is an Ink intrinsic in which there can only be 1 per application. + * Because of this restriction we're hacking it slightly by having a 'header' item here to + * ensure that it's statically rendered. + * + * Background on the Static Item: Anything in the Static component is written a single time + * to the console. Think of it like doing a console.log and then never using ANSI codes to + * clear that content ever again. Effectively it has a moving frame that every time new static + * content is set it'll flush content to the terminal and move the area which it's "clearing" + * down a notch. Without Static the area which gets erased and redrawn continuously grows. + */} + <Static + key={staticKey} + items={[ + <Box flexDirection="column" key="header"> + <Header /> + <Tips /> + </Box>, + ...history.map((h) => ( + <HistoryItemDisplay + availableTerminalHeight={availableTerminalHeight} + key={h.id} + item={h} + isPending={false} + /> + )), + ]} + > + {(item) => item} + </Static> + <Box ref={pendingHistoryItemRef}> + {pendingHistoryItems.map((item, i) => ( <HistoryItemDisplay + key={i} availableTerminalHeight={availableTerminalHeight} - key={h.id} - item={h} - isPending={false} - streamingState={streamingState} + // TODO(taehykim): It seems like references to ids aren't necessary in + // HistoryItemDisplay. Refactor later. Use a fake id for now. + item={{ ...item, id: 0 }} + isPending={true} /> - )), - ]} - > - {(item) => item} - </Static> - <Box ref={pendingHistoryItemRef}> - {pendingHistoryItems.map((item, i) => ( - <HistoryItemDisplay - key={i} - availableTerminalHeight={availableTerminalHeight} - // TODO(taehykim): It seems like references to ids aren't necessary in - // HistoryItemDisplay. Refactor later. Use a fake id for now. - item={{ ...item, id: 0 }} - isPending={true} - streamingState={streamingState} - /> - ))} - </Box> - {showHelp && <Help commands={slashCommands} />} - - <Box flexDirection="column" ref={mainControlsRef}> - {startupWarnings.length > 0 && ( - <Box - borderStyle="round" - borderColor={Colors.AccentYellow} - paddingX={1} - marginY={1} - flexDirection="column" - > - {startupWarnings.map((warning, index) => ( - <Text key={index} color={Colors.AccentYellow}> - {warning} - </Text> - ))} - </Box> - )} + ))} + </Box> + {showHelp && <Help commands={slashCommands} />} - {isThemeDialogOpen ? ( - <Box flexDirection="column"> - {themeError && ( - <Box marginBottom={1}> - <Text color={Colors.AccentRed}>{themeError}</Text> - </Box> - )} - <ThemeDialog - onSelect={handleThemeSelect} - onHighlight={handleThemeHighlight} - settings={settings} - /> - </Box> - ) : ( - <> - <LoadingIndicator - isLoading={streamingState === StreamingState.Responding} - showSpinner={shouldShowSpinner} - currentLoadingPhrase={currentLoadingPhrase} - elapsedTime={elapsedTime} - /> + <Box flexDirection="column" ref={mainControlsRef}> + {startupWarnings.length > 0 && ( <Box - marginTop={1} - display="flex" - justifyContent="space-between" - width="100%" + borderStyle="round" + borderColor={Colors.AccentYellow} + paddingX={1} + marginY={1} + flexDirection="column" > - <Box> - {process.env.GEMINI_SYSTEM_MD && ( - <Text color={Colors.AccentRed}>|⌐■_■| </Text> - )} - {geminiMdFileCount > 0 && ( - <Text color={Colors.SubtleComment}> - Using {geminiMdFileCount} GEMINI.md file - {geminiMdFileCount > 1 ? 's' : ''} - </Text> - )} - </Box> - <Box> - {showAutoAcceptIndicator && !shellModeActive && ( - <AutoAcceptIndicator /> - )} - {shellModeActive && <ShellModeIndicator />} - </Box> + {startupWarnings.map((warning, index) => ( + <Text key={index} color={Colors.AccentYellow}> + {warning} + </Text> + ))} </Box> + )} - {showErrorDetails && ( - <DetailedMessagesDisplay messages={filteredConsoleMessages} /> - )} - - {isInputActive && ( - <InputPrompt - widthFraction={0.9} - onSubmit={handleFinalSubmit} - userMessages={userMessages} - onClearScreen={handleClearScreen} - config={config} - slashCommands={slashCommands} - shellModeActive={shellModeActive} - setShellModeActive={setShellModeActive} + {isThemeDialogOpen ? ( + <Box flexDirection="column"> + {themeError && ( + <Box marginBottom={1}> + <Text color={Colors.AccentRed}>{themeError}</Text> + </Box> + )} + <ThemeDialog + onSelect={handleThemeSelect} + onHighlight={handleThemeHighlight} + settings={settings} + /> + </Box> + ) : ( + <> + <LoadingIndicator + currentLoadingPhrase={currentLoadingPhrase} + elapsedTime={elapsedTime} /> - )} - </> - )} + <Box + marginTop={1} + display="flex" + justifyContent="space-between" + width="100%" + > + <Box> + {process.env.GEMINI_SYSTEM_MD && ( + <Text color={Colors.AccentRed}>|⌐■_■| </Text> + )} + {geminiMdFileCount > 0 && ( + <Text color={Colors.SubtleComment}> + Using {geminiMdFileCount} GEMINI.md file + {geminiMdFileCount > 1 ? 's' : ''} + </Text> + )} + </Box> + <Box> + {showAutoAcceptIndicator && !shellModeActive && ( + <AutoAcceptIndicator /> + )} + {shellModeActive && <ShellModeIndicator />} + </Box> + </Box> - {initError && streamingState !== StreamingState.Responding && ( - <Box - borderStyle="round" - borderColor={Colors.AccentRed} - paddingX={1} - marginBottom={1} - > - {history.find( - (item) => item.type === 'error' && item.text?.includes(initError), - )?.text ? ( - <Text color={Colors.AccentRed}> - { - history.find( - (item) => - item.type === 'error' && item.text?.includes(initError), - )?.text - } - </Text> - ) : ( - <> - <Text color={Colors.AccentRed}> - Initialization Error: {initError} - </Text> + {showErrorDetails && ( + <DetailedMessagesDisplay messages={filteredConsoleMessages} /> + )} + + {isInputActive && ( + <InputPrompt + widthFraction={0.9} + onSubmit={handleFinalSubmit} + userMessages={userMessages} + onClearScreen={handleClearScreen} + config={config} + slashCommands={slashCommands} + shellModeActive={shellModeActive} + setShellModeActive={setShellModeActive} + /> + )} + </> + )} + + {initError && streamingState !== StreamingState.Responding && ( + <Box + borderStyle="round" + borderColor={Colors.AccentRed} + paddingX={1} + marginBottom={1} + > + {history.find( + (item) => + item.type === 'error' && item.text?.includes(initError), + )?.text ? ( <Text color={Colors.AccentRed}> - {' '} - Please check API key and configuration. + { + history.find( + (item) => + item.type === 'error' && item.text?.includes(initError), + )?.text + } </Text> - </> - )} - </Box> - )} + ) : ( + <> + <Text color={Colors.AccentRed}> + Initialization Error: {initError} + </Text> + <Text color={Colors.AccentRed}> + {' '} + Please check API key and configuration. + </Text> + </> + )} + </Box> + )} - <Footer - config={config} - debugMode={config.getDebugMode()} - debugMessage={debugMessage} - cliVersion={cliVersion} - corgiMode={corgiMode} - errorCount={errorCount} - showErrorDetails={showErrorDetails} - /> + <Footer + config={config} + debugMode={config.getDebugMode()} + debugMessage={debugMessage} + cliVersion={cliVersion} + corgiMode={corgiMode} + errorCount={errorCount} + showErrorDetails={showErrorDetails} + /> + </Box> </Box> - </Box> + </StreamingContext.Provider> ); }; diff --git a/packages/cli/src/ui/components/HistoryItemDisplay.tsx b/packages/cli/src/ui/components/HistoryItemDisplay.tsx index 21ffb5a1..39cc5308 100644 --- a/packages/cli/src/ui/components/HistoryItemDisplay.tsx +++ b/packages/cli/src/ui/components/HistoryItemDisplay.tsx @@ -5,7 +5,7 @@ */ import React from 'react'; -import type { HistoryItem, StreamingState } from '../types.js'; +import type { HistoryItem } from '../types.js'; import { UserMessage } from './messages/UserMessage.js'; import { UserShellMessage } from './messages/UserShellMessage.js'; import { GeminiMessage } from './messages/GeminiMessage.js'; @@ -20,14 +20,12 @@ interface HistoryItemDisplayProps { item: HistoryItem; availableTerminalHeight: number; isPending: boolean; - streamingState?: StreamingState; } export const HistoryItemDisplay: React.FC<HistoryItemDisplayProps> = ({ item, availableTerminalHeight, isPending, - streamingState, }) => ( <Box flexDirection="column" key={item.id}> {/* Render standard message types */} @@ -62,7 +60,6 @@ export const HistoryItemDisplay: React.FC<HistoryItemDisplayProps> = ({ toolCalls={item.tools} groupId={item.id} availableTerminalHeight={availableTerminalHeight} - streamingState={streamingState} /> )} </Box> diff --git a/packages/cli/src/ui/components/LoadingIndicator.test.tsx b/packages/cli/src/ui/components/LoadingIndicator.test.tsx index 6a8880d4..a03fb230 100644 --- a/packages/cli/src/ui/components/LoadingIndicator.test.tsx +++ b/packages/cli/src/ui/components/LoadingIndicator.test.tsx @@ -4,91 +4,158 @@ * SPDX-License-Identifier: Apache-2.0 */ +import React from 'react'; import { render } from 'ink-testing-library'; -import { Text } from 'ink'; // Import Text directly from ink +import { Text } from 'ink'; import { LoadingIndicator } from './LoadingIndicator.js'; +import { + StreamingContext, + StreamingContextType, +} from '../contexts/StreamingContext.js'; +import { StreamingState } from '../types.js'; +import { vi } from 'vitest'; +// Mock ink-spinner vi.mock('ink-spinner', () => ({ - default: function MockSpinner() { - return <Text>MockSpinner</Text>; - }, + default: () => <Text>MockSpinner</Text>, })); +const renderWithContext = ( + ui: React.ReactElement, + streamingStateValue: StreamingState, +) => { + const contextValue: StreamingContextType = { + streamingState: streamingStateValue, + }; + return render( + <StreamingContext.Provider value={contextValue}> + {ui} + </StreamingContext.Provider>, + ); +}; + describe('<LoadingIndicator />', () => { - it('should not render when isLoading is false', () => { - const { lastFrame } = render( - <LoadingIndicator - isLoading={false} - showSpinner={true} - currentLoadingPhrase="Loading..." - elapsedTime={0} - />, + const defaultProps = { + currentLoadingPhrase: 'Loading...', + elapsedTime: 5, + }; + + it('should not render when streamingState is Idle', () => { + const { lastFrame } = renderWithContext( + <LoadingIndicator {...defaultProps} />, + StreamingState.Idle, ); expect(lastFrame()).toBe(''); }); - it('should render spinner, phrase, and time when isLoading is true and showSpinner is true', () => { - const phrase = 'Processing data...'; - const time = 5; - const { lastFrame } = render( - <LoadingIndicator - isLoading={true} - showSpinner={true} - currentLoadingPhrase={phrase} - elapsedTime={time} - />, + it('should render spinner, phrase, and time when streamingState is Responding', () => { + const { lastFrame } = renderWithContext( + <LoadingIndicator {...defaultProps} />, + StreamingState.Responding, ); - const output = lastFrame(); - expect(output).toContain(phrase); - expect(output).toContain(`(esc to cancel, ${time}s)`); - // Check for spinner presence by looking for its characteristic characters or structure - // This is a bit fragile as it depends on Spinner's output. - // A more robust way would be to mock Spinner and check if it was rendered. - expect(output).toContain('MockSpinner'); // Check for the mocked spinner text + expect(output).toContain('MockSpinner'); + expect(output).toContain('Loading...'); + expect(output).toContain('(esc to cancel, 5s)'); }); - it('should render phrase and time but no spinner when isLoading is true and showSpinner is false', () => { - const phrase = 'Waiting for input...'; - const time = 10; - const { lastFrame } = render( - <LoadingIndicator - isLoading={true} - showSpinner={false} - currentLoadingPhrase={phrase} - elapsedTime={time} - />, + it('should render phrase and time but no spinner when streamingState is WaitingForConfirmation', () => { + const props = { + currentLoadingPhrase: 'Confirm action', + elapsedTime: 10, + }; + const { lastFrame } = renderWithContext( + <LoadingIndicator {...props} />, + StreamingState.WaitingForConfirmation, ); const output = lastFrame(); - expect(output).toContain(phrase); - expect(output).toContain(`(esc to cancel, ${time}s)`); - // Ensure spinner characters are NOT present expect(output).not.toContain('MockSpinner'); + expect(output).toContain('Confirm action'); + expect(output).not.toContain('(esc to cancel)'); + expect(output).not.toContain(', 10s'); }); it('should display the currentLoadingPhrase correctly', () => { - const specificPhrase = 'Almost there!'; - const { lastFrame } = render( - <LoadingIndicator - isLoading={true} - showSpinner={true} - currentLoadingPhrase={specificPhrase} - elapsedTime={3} - />, + const props = { + currentLoadingPhrase: 'Processing data...', + elapsedTime: 3, + }; + const { lastFrame } = renderWithContext( + <LoadingIndicator {...props} />, + StreamingState.Responding, + ); + expect(lastFrame()).toContain('Processing data...'); + }); + + it('should display the elapsedTime correctly when Responding', () => { + const props = { + currentLoadingPhrase: 'Working...', + elapsedTime: 8, + }; + const { lastFrame } = renderWithContext( + <LoadingIndicator {...props} />, + StreamingState.Responding, + ); + expect(lastFrame()).toContain('(esc to cancel, 8s)'); + }); + + it('should render rightContent when provided', () => { + const rightContent = <Text>Extra Info</Text>; + const { lastFrame } = renderWithContext( + <LoadingIndicator {...defaultProps} rightContent={rightContent} />, + StreamingState.Responding, ); - expect(lastFrame()).toContain(specificPhrase); + expect(lastFrame()).toContain('Extra Info'); }); - it('should display the elapsedTime correctly', () => { - const specificTime = 7; - const { lastFrame } = render( - <LoadingIndicator - isLoading={true} - showSpinner={true} - currentLoadingPhrase="Working..." - elapsedTime={specificTime} - />, + it('should transition correctly between states using rerender', () => { + const { lastFrame, rerender } = renderWithContext( + <LoadingIndicator {...defaultProps} />, + StreamingState.Idle, ); - expect(lastFrame()).toContain(`(esc to cancel, ${specificTime}s)`); + expect(lastFrame()).toBe(''); // Initial: Idle + + // Transition to Responding + rerender( + <StreamingContext.Provider + value={{ streamingState: StreamingState.Responding }} + > + <LoadingIndicator + currentLoadingPhrase="Now Responding" + elapsedTime={2} + /> + </StreamingContext.Provider>, + ); + let output = lastFrame(); + expect(output).toContain('MockSpinner'); + expect(output).toContain('Now Responding'); + expect(output).toContain('(esc to cancel, 2s)'); + + // Transition to WaitingForConfirmation + rerender( + <StreamingContext.Provider + value={{ streamingState: StreamingState.WaitingForConfirmation }} + > + <LoadingIndicator + currentLoadingPhrase="Please Confirm" + elapsedTime={15} + /> + </StreamingContext.Provider>, + ); + output = lastFrame(); + expect(output).not.toContain('MockSpinner'); + expect(output).toContain('Please Confirm'); + expect(output).not.toContain('(esc to cancel)'); + expect(output).not.toContain(', 15s'); + + // Transition back to Idle + rerender( + <StreamingContext.Provider + value={{ streamingState: StreamingState.Idle }} + > + <LoadingIndicator {...defaultProps} /> + </StreamingContext.Provider>, + ); + expect(lastFrame()).toBe(''); }); }); diff --git a/packages/cli/src/ui/components/LoadingIndicator.tsx b/packages/cli/src/ui/components/LoadingIndicator.tsx index b0c24f80..d24b6a56 100644 --- a/packages/cli/src/ui/components/LoadingIndicator.tsx +++ b/packages/cli/src/ui/components/LoadingIndicator.tsx @@ -8,36 +8,38 @@ import React from 'react'; import { Box, Text } from 'ink'; import Spinner from 'ink-spinner'; import { Colors } from '../colors.js'; +import { useStreamingContext } from '../contexts/StreamingContext.js'; +import { StreamingState } from '../types.js'; interface LoadingIndicatorProps { - isLoading: boolean; - showSpinner: boolean; currentLoadingPhrase: string; elapsedTime: number; rightContent?: React.ReactNode; } export const LoadingIndicator: React.FC<LoadingIndicatorProps> = ({ - isLoading, - showSpinner, currentLoadingPhrase, elapsedTime, rightContent, }) => { - if (!isLoading) { + const { streamingState } = useStreamingContext(); + + if (streamingState === StreamingState.Idle) { return null; } return ( <Box marginTop={1} paddingLeft={0}> - {showSpinner && ( + {streamingState === StreamingState.Responding && ( <Box marginRight={1}> <Spinner type="dots" /> </Box> )} <Text color={Colors.AccentPurple}> {currentLoadingPhrase} - {isLoading && ` (esc to cancel, ${elapsedTime}s)`} + {streamingState === StreamingState.WaitingForConfirmation + ? '' + : ` (esc to cancel, ${elapsedTime}s)`} </Text> <Box flexGrow={1}>{/* Spacer */}</Box> {rightContent && <Box>{rightContent}</Box>} diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx index c6c8b874..8bcde3bb 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx @@ -6,11 +6,7 @@ import React, { useMemo } from 'react'; import { Box } from 'ink'; -import { - IndividualToolCallDisplay, - StreamingState, - ToolCallStatus, -} from '../../types.js'; +import { IndividualToolCallDisplay, ToolCallStatus } from '../../types.js'; import { ToolMessage } from './ToolMessage.js'; import { ToolConfirmationMessage } from './ToolConfirmationMessage.js'; import { Colors } from '../../colors.js'; @@ -19,14 +15,12 @@ interface ToolGroupMessageProps { groupId: number; toolCalls: IndividualToolCallDisplay[]; availableTerminalHeight: number; - streamingState?: StreamingState; } // Main component renders the border and maps the tools using ToolMessage export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({ toolCalls, availableTerminalHeight, - streamingState, }) => { const hasPending = !toolCalls.every( (t) => t.status === ToolCallStatus.Success, @@ -78,7 +72,6 @@ export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({ ? 'low' : 'medium' } - streamingState={streamingState} /> </Box> {tool.status === ToolCallStatus.Confirming && diff --git a/packages/cli/src/ui/components/messages/ToolMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolMessage.test.tsx index 1c81cc36..10380ad4 100644 --- a/packages/cli/src/ui/components/messages/ToolMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolMessage.test.tsx @@ -8,6 +8,7 @@ import { render } from 'ink-testing-library'; import { ToolMessage, ToolMessageProps } from './ToolMessage.js'; import { StreamingState, ToolCallStatus } from '../../types.js'; import { Text } from 'ink'; +import { StreamingContext } from '../../contexts/StreamingContext.js'; // Mock child components or utilities if they are complex or have side effects vi.mock('ink-spinner', () => ({ @@ -28,6 +29,17 @@ vi.mock('../../utils/MarkdownDisplay.js', () => ({ }, })); +// Helper to render with context +const renderWithContext = ( + ui: React.ReactElement, + streamingState: StreamingState, +) => + render( + <StreamingContext.Provider value={{ streamingState }}> + {ui} + </StreamingContext.Provider>, + ); + describe('<ToolMessage />', () => { const baseProps: ToolMessageProps = { callId: 'tool-123', @@ -38,11 +50,13 @@ describe('<ToolMessage />', () => { availableTerminalHeight: 20, confirmationDetails: undefined, emphasis: 'medium', - streamingState: StreamingState.Idle, }; it('renders basic tool information', () => { - const { lastFrame } = render(<ToolMessage {...baseProps} />); + const { lastFrame } = renderWithContext( + <ToolMessage {...baseProps} />, + StreamingState.Idle, + ); const output = lastFrame(); expect(output).toContain('✔'); // Success indicator expect(output).toContain('test-tool'); @@ -52,73 +66,72 @@ describe('<ToolMessage />', () => { describe('ToolStatusIndicator rendering', () => { it('shows ✔ for Success status', () => { - const { lastFrame } = render( + const { lastFrame } = renderWithContext( <ToolMessage {...baseProps} status={ToolCallStatus.Success} />, + StreamingState.Idle, ); expect(lastFrame()).toContain('✔'); }); it('shows o for Pending status', () => { - const { lastFrame } = render( + const { lastFrame } = renderWithContext( <ToolMessage {...baseProps} status={ToolCallStatus.Pending} />, + StreamingState.Idle, ); expect(lastFrame()).toContain('o'); }); it('shows ? for Confirming status', () => { - const { lastFrame } = render( + const { lastFrame } = renderWithContext( <ToolMessage {...baseProps} status={ToolCallStatus.Confirming} />, + StreamingState.Idle, ); expect(lastFrame()).toContain('?'); }); it('shows - for Canceled status', () => { - const { lastFrame } = render( + const { lastFrame } = renderWithContext( <ToolMessage {...baseProps} status={ToolCallStatus.Canceled} />, + StreamingState.Idle, ); expect(lastFrame()).toContain('-'); }); it('shows x for Error status', () => { - const { lastFrame } = render( + const { lastFrame } = renderWithContext( <ToolMessage {...baseProps} status={ToolCallStatus.Error} />, + StreamingState.Idle, ); expect(lastFrame()).toContain('x'); }); - it('shows MockSpinner for Executing status when streamingState is Idle', () => { - const { lastFrame } = render( - <ToolMessage - {...baseProps} - status={ToolCallStatus.Executing} - streamingState={StreamingState.Idle} - />, + it('shows paused spiner for Executing status when streamingState is Idle', () => { + const { lastFrame } = renderWithContext( + <ToolMessage {...baseProps} status={ToolCallStatus.Executing} />, + StreamingState.Idle, ); - expect(lastFrame()).toContain('MockSpinner'); + expect(lastFrame()).toContain('⠇'); + expect(lastFrame()).not.toContain('MockSpinner'); expect(lastFrame()).not.toContain('✔'); }); - it('shows MockSpinner for Executing status when streamingState is undefined (default behavior)', () => { - const { lastFrame } = render( + it('shows paused spiner for Executing status when streamingState is WaitingForConfirmation', () => { + const { lastFrame } = renderWithContext( <ToolMessage {...baseProps} status={ToolCallStatus.Executing} />, + StreamingState.WaitingForConfirmation, ); - expect(lastFrame()).toContain('MockSpinner'); + expect(lastFrame()).toContain('⠇'); + expect(lastFrame()).not.toContain('MockSpinner'); expect(lastFrame()).not.toContain('✔'); }); - it('shows ✔ (paused/confirmed look) for Executing status when streamingState is Responding', () => { - // This is the key change from the commit: if the overall app is still responding - // (e.g., waiting for other tool confirmations), an already confirmed and executing tool - // should show a static checkmark to avoid spinner flicker. - const { lastFrame } = render( - <ToolMessage - {...baseProps} - status={ToolCallStatus.Executing} - streamingState={StreamingState.Responding} // Simulate app still responding - />, + it('shows MockSpinner for Executing status when streamingState is Responding', () => { + const { lastFrame } = renderWithContext( + <ToolMessage {...baseProps} status={ToolCallStatus.Executing} />, + StreamingState.Responding, // Simulate app still responding ); - expect(lastFrame()).toContain('✔'); // Should be a checkmark, not spinner - expect(lastFrame()).not.toContain('MockSpinner'); + expect(lastFrame()).toContain('MockSpinner'); + expect(lastFrame()).not.toContain('✔'); }); }); @@ -127,22 +140,25 @@ describe('<ToolMessage />', () => { fileDiff: '--- a/file.txt\n+++ b/file.txt\n@@ -1 +1 @@\n-old\n+new', fileName: 'file.txt', }; - const { lastFrame } = render( + const { lastFrame } = renderWithContext( <ToolMessage {...baseProps} resultDisplay={diffResult} />, + StreamingState.Idle, ); // Check that the output contains the MockDiff content as part of the whole message expect(lastFrame()).toMatch(/MockDiff:--- a\/file\.txt/); }); it('renders emphasis correctly', () => { - const { lastFrame: highEmphasisFrame } = render( + const { lastFrame: highEmphasisFrame } = renderWithContext( <ToolMessage {...baseProps} emphasis="high" />, + StreamingState.Idle, ); // Check for trailing indicator or specific color if applicable (Colors are not easily testable here) expect(highEmphasisFrame()).toContain('←'); // Trailing indicator for high emphasis - const { lastFrame: lowEmphasisFrame } = render( + const { lastFrame: lowEmphasisFrame } = renderWithContext( <ToolMessage {...baseProps} emphasis="low" />, + StreamingState.Idle, ); // For low emphasis, the name and description might be dimmed (check for dimColor if possible) // This is harder to assert directly in text output without color checks. 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; diff --git a/packages/cli/src/ui/constants.ts b/packages/cli/src/ui/constants.ts index 5924c7a5..6a0a9375 100644 --- a/packages/cli/src/ui/constants.ts +++ b/packages/cli/src/ui/constants.ts @@ -12,22 +12,4 @@ export const BOX_PADDING_X = 1; export const UI_WIDTH = EstimatedArtWidth + BOX_PADDING_X * 2 + BoxBorderWidth * 2; // ~63 -export const WITTY_LOADING_PHRASES = [ - 'Consulting the digital spirits...', - 'Reticulating splines...', - 'Warming up the AI hamsters...', - 'Asking the magic conch shell...', - 'Generating witty retort...', - 'Polishing the algorithms...', - "Don't rush perfection (or my code)...", - 'Brewing fresh bytes...', - 'Counting electrons...', - 'Engaging cognitive processors...', - 'Checking for syntax errors in the universe...', - 'One moment, optimizing humor...', - 'Shuffling punchlines...', - 'Untangling neural nets...', - 'Compiling brilliance...', -]; -export const PHRASE_CHANGE_INTERVAL_MS = 15000; export const STREAM_DEBOUNCE_MS = 100; diff --git a/packages/cli/src/ui/contexts/StreamingContext.tsx b/packages/cli/src/ui/contexts/StreamingContext.tsx new file mode 100644 index 00000000..6144ed70 --- /dev/null +++ b/packages/cli/src/ui/contexts/StreamingContext.tsx @@ -0,0 +1,26 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import React, { createContext } from 'react'; +import { StreamingState } from '../types.js'; + +export interface StreamingContextType { + streamingState: StreamingState; +} + +export const StreamingContext = createContext<StreamingContextType | undefined>( + undefined, +); + +export const useStreamingContext = (): StreamingContextType => { + const context = React.useContext(StreamingContext); + if (context === undefined) { + throw new Error( + 'useStreamingContext must be used within a StreamingContextProvider', + ); + } + return context; +}; diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index 45415f39..9dcb005b 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -42,7 +42,6 @@ import { useToolScheduler, mapToDisplay } from './useToolScheduler.js'; enum StreamProcessingStatus { Completed, - PausedForConfirmation, UserCancelled, Error, } @@ -103,6 +102,21 @@ export const useGeminiStream = ( config, ); + const streamingState = useMemo(() => { + if (toolCalls.some((t) => t.status === 'awaiting_approval')) { + return StreamingState.WaitingForConfirmation; + } + if ( + isResponding || + toolCalls.some( + (t) => t.status === 'executing' || t.status === 'scheduled', + ) + ) { + return StreamingState.Responding; + } + return StreamingState.Idle; + }, [isResponding, toolCalls]); + useEffect(() => { setInitError(null); if (!geminiClientRef.current) { @@ -123,85 +137,100 @@ export const useGeminiStream = ( } }); - const prepareQueryForGemini = async ( - query: PartListUnion, - userMessageTimestamp: number, - signal: AbortSignal, - ): Promise<{ queryToSend: PartListUnion | null; shouldProceed: boolean }> => { - if (typeof query === 'string' && query.trim().length === 0) { - return { queryToSend: null, shouldProceed: false }; - } + const prepareQueryForGemini = useCallback( + async ( + query: PartListUnion, + userMessageTimestamp: number, + signal: AbortSignal, + ): Promise<{ + queryToSend: PartListUnion | null; + shouldProceed: boolean; + }> => { + if (typeof query === 'string' && query.trim().length === 0) { + return { queryToSend: null, shouldProceed: false }; + } - let localQueryToSendToGemini: PartListUnion | null = null; + let localQueryToSendToGemini: PartListUnion | null = null; - if (typeof query === 'string') { - const trimmedQuery = query.trim(); - onDebugMessage(`User query: '${trimmedQuery}'`); - await logger?.logMessage(MessageSenderType.USER, trimmedQuery); + if (typeof query === 'string') { + const trimmedQuery = query.trim(); + onDebugMessage(`User query: '${trimmedQuery}'`); + await logger?.logMessage(MessageSenderType.USER, trimmedQuery); - // Handle UI-only commands first - const slashCommandResult = handleSlashCommand(trimmedQuery); - if (typeof slashCommandResult === 'boolean' && slashCommandResult) { - // Command was handled, and it doesn't require a tool call from here - return { queryToSend: null, shouldProceed: false }; - } else if ( - typeof slashCommandResult === 'object' && - slashCommandResult.shouldScheduleTool - ) { - // Slash command wants to schedule a tool call (e.g., /memory add) - const { toolName, toolArgs } = slashCommandResult; - if (toolName && toolArgs) { - const toolCallRequest: ToolCallRequestInfo = { - callId: `${toolName}-${Date.now()}-${Math.random().toString(16).slice(2)}`, - name: toolName, - args: toolArgs, - }; - schedule([toolCallRequest]); // schedule expects an array or single object + // Handle UI-only commands first + const slashCommandResult = handleSlashCommand(trimmedQuery); + if (typeof slashCommandResult === 'boolean' && slashCommandResult) { + // Command was handled, and it doesn't require a tool call from here + return { queryToSend: null, shouldProceed: false }; + } else if ( + typeof slashCommandResult === 'object' && + slashCommandResult.shouldScheduleTool + ) { + // Slash command wants to schedule a tool call (e.g., /memory add) + const { toolName, toolArgs } = slashCommandResult; + if (toolName && toolArgs) { + const toolCallRequest: ToolCallRequestInfo = { + callId: `${toolName}-${Date.now()}-${Math.random().toString(16).slice(2)}`, + name: toolName, + args: toolArgs, + }; + schedule([toolCallRequest]); // schedule expects an array or single object + } + return { queryToSend: null, shouldProceed: false }; // Handled by scheduling the tool } - return { queryToSend: null, shouldProceed: false }; // Handled by scheduling the tool - } - if (shellModeActive && handleShellCommand(trimmedQuery)) { - return { queryToSend: null, shouldProceed: false }; - } - - // Handle @-commands (which might involve tool calls) - if (isAtCommand(trimmedQuery)) { - const atCommandResult = await handleAtCommand({ - query: trimmedQuery, - config, - addItem, - onDebugMessage, - messageId: userMessageTimestamp, - signal, - }); - if (!atCommandResult.shouldProceed) { + if (shellModeActive && handleShellCommand(trimmedQuery)) { return { queryToSend: null, shouldProceed: false }; } - localQueryToSendToGemini = atCommandResult.processedQuery; + + // Handle @-commands (which might involve tool calls) + if (isAtCommand(trimmedQuery)) { + const atCommandResult = await handleAtCommand({ + query: trimmedQuery, + config, + addItem, + onDebugMessage, + messageId: userMessageTimestamp, + signal, + }); + if (!atCommandResult.shouldProceed) { + return { queryToSend: null, shouldProceed: false }; + } + localQueryToSendToGemini = atCommandResult.processedQuery; + } else { + // Normal query for Gemini + addItem( + { type: MessageType.USER, text: trimmedQuery }, + userMessageTimestamp, + ); + localQueryToSendToGemini = trimmedQuery; + } } else { - // Normal query for Gemini - addItem( - { type: MessageType.USER, text: trimmedQuery }, - userMessageTimestamp, - ); - localQueryToSendToGemini = trimmedQuery; + // It's a function response (PartListUnion that isn't a string) + localQueryToSendToGemini = query; } - } else { - // It's a function response (PartListUnion that isn't a string) - localQueryToSendToGemini = query; - } - if (localQueryToSendToGemini === null) { - onDebugMessage( - 'Query processing resulted in null, not sending to Gemini.', - ); - return { queryToSend: null, shouldProceed: false }; - } - return { queryToSend: localQueryToSendToGemini, shouldProceed: true }; - }; + if (localQueryToSendToGemini === null) { + onDebugMessage( + 'Query processing resulted in null, not sending to Gemini.', + ); + return { queryToSend: null, shouldProceed: false }; + } + return { queryToSend: localQueryToSendToGemini, shouldProceed: true }; + }, + [ + config, + addItem, + onDebugMessage, + handleShellCommand, + handleSlashCommand, + logger, + shellModeActive, + schedule, + ], + ); - const ensureChatSession = async (): Promise<{ + const ensureChatSession = useCallback(async (): Promise<{ client: GeminiClient | null; chat: Chat | null; }> => { @@ -224,7 +253,7 @@ export const useGeminiStream = ( } } return { client: currentClient, chat: chatSessionRef.current }; - }; + }, [addItem]); // --- UI Helper Functions (used by event handlers) --- const updateFunctionResponseUI = ( @@ -285,6 +314,7 @@ export const useGeminiStream = ( history.push({ role: 'model', parts: [functionResponse] }); } updateFunctionResponseUI(responseInfo, status); + if (pendingHistoryItemRef.current) { addItem(pendingHistoryItemRef.current, Date.now()); setPendingHistoryItem(null); @@ -293,138 +323,151 @@ export const useGeminiStream = ( } // --- Stream Event Handlers --- - const handleContentEvent = ( - eventValue: ContentEvent['value'], - currentGeminiMessageBuffer: string, - userMessageTimestamp: number, - ): string => { - let newGeminiMessageBuffer = currentGeminiMessageBuffer + eventValue; - if ( - pendingHistoryItemRef.current?.type !== 'gemini' && - pendingHistoryItemRef.current?.type !== 'gemini_content' - ) { + + const handleContentEvent = useCallback( + ( + eventValue: ContentEvent['value'], + currentGeminiMessageBuffer: string, + userMessageTimestamp: number, + ): string => { + let newGeminiMessageBuffer = currentGeminiMessageBuffer + eventValue; + if ( + pendingHistoryItemRef.current?.type !== 'gemini' && + pendingHistoryItemRef.current?.type !== 'gemini_content' + ) { + if (pendingHistoryItemRef.current) { + addItem(pendingHistoryItemRef.current, userMessageTimestamp); + } + setPendingHistoryItem({ type: 'gemini', text: '' }); + newGeminiMessageBuffer = eventValue; + } + // Split large messages for better rendering performance. Ideally, + // we should maximize the amount of output sent to <Static />. + const splitPoint = findLastSafeSplitPoint(newGeminiMessageBuffer); + if (splitPoint === newGeminiMessageBuffer.length) { + // Update the existing message with accumulated content + setPendingHistoryItem((item) => ({ + type: item?.type as 'gemini' | 'gemini_content', + text: newGeminiMessageBuffer, + })); + } else { + // This indicates that we need to split up this Gemini Message. + // Splitting a message is primarily a performance consideration. There is a + // <Static> component at the root of App.tsx which takes care of rendering + // content statically or dynamically. Everything but the last message is + // treated as static in order to prevent re-rendering an entire message history + // multiple times per-second (as streaming occurs). Prior to this change you'd + // see heavy flickering of the terminal. This ensures that larger messages get + // broken up so that there are more "statically" rendered. + const beforeText = newGeminiMessageBuffer.substring(0, splitPoint); + const afterText = newGeminiMessageBuffer.substring(splitPoint); + addItem( + { + type: pendingHistoryItemRef.current?.type as + | 'gemini' + | 'gemini_content', + text: beforeText, + }, + userMessageTimestamp, + ); + setPendingHistoryItem({ type: 'gemini_content', text: afterText }); + newGeminiMessageBuffer = afterText; + } + return newGeminiMessageBuffer; + }, + [addItem, pendingHistoryItemRef, setPendingHistoryItem], + ); + + const handleUserCancelledEvent = useCallback( + (userMessageTimestamp: number) => { if (pendingHistoryItemRef.current) { - addItem(pendingHistoryItemRef.current, userMessageTimestamp); + if (pendingHistoryItemRef.current.type === 'tool_group') { + const updatedTools = pendingHistoryItemRef.current.tools.map( + (tool) => + tool.status === ToolCallStatus.Pending || + tool.status === ToolCallStatus.Confirming || + tool.status === ToolCallStatus.Executing + ? { ...tool, status: ToolCallStatus.Canceled } + : tool, + ); + const pendingItem: HistoryItemToolGroup = { + ...pendingHistoryItemRef.current, + tools: updatedTools, + }; + addItem(pendingItem, userMessageTimestamp); + } else { + addItem(pendingHistoryItemRef.current, userMessageTimestamp); + } + setPendingHistoryItem(null); } - setPendingHistoryItem({ type: 'gemini', text: '' }); - newGeminiMessageBuffer = eventValue; - } - // Split large messages for better rendering performance. Ideally, - // we should maximize the amount of output sent to <Static />. - const splitPoint = findLastSafeSplitPoint(newGeminiMessageBuffer); - if (splitPoint === newGeminiMessageBuffer.length) { - // Update the existing message with accumulated content - setPendingHistoryItem((item) => ({ - type: item?.type as 'gemini' | 'gemini_content', - text: newGeminiMessageBuffer, - })); - } else { - // This indicates that we need to split up this Gemini Message. - // Splitting a message is primarily a performance consideration. There is a - // <Static> component at the root of App.tsx which takes care of rendering - // content statically or dynamically. Everything but the last message is - // treated as static in order to prevent re-rendering an entire message history - // multiple times per-second (as streaming occurs). Prior to this change you'd - // see heavy flickering of the terminal. This ensures that larger messages get - // broken up so that there are more "statically" rendered. - const beforeText = newGeminiMessageBuffer.substring(0, splitPoint); - const afterText = newGeminiMessageBuffer.substring(splitPoint); addItem( - { - type: pendingHistoryItemRef.current?.type as - | 'gemini' - | 'gemini_content', - text: beforeText, - }, + { type: MessageType.INFO, text: 'User cancelled the request.' }, userMessageTimestamp, ); - setPendingHistoryItem({ type: 'gemini_content', text: afterText }); - newGeminiMessageBuffer = afterText; - } - return newGeminiMessageBuffer; - }; + setIsResponding(false); + cancel(); + }, + [addItem, pendingHistoryItemRef, setPendingHistoryItem, cancel], + ); - const handleUserCancelledEvent = (userMessageTimestamp: number) => { - if (pendingHistoryItemRef.current) { - if (pendingHistoryItemRef.current.type === 'tool_group') { - const updatedTools = pendingHistoryItemRef.current.tools.map((tool) => - tool.status === ToolCallStatus.Pending || - tool.status === ToolCallStatus.Confirming || - tool.status === ToolCallStatus.Executing - ? { ...tool, status: ToolCallStatus.Canceled } - : tool, - ); - const pendingItem: HistoryItemToolGroup = { - ...pendingHistoryItemRef.current, - tools: updatedTools, - }; - addItem(pendingItem, userMessageTimestamp); - } else { + const handleErrorEvent = useCallback( + (eventValue: ErrorEvent['value'], userMessageTimestamp: number) => { + if (pendingHistoryItemRef.current) { addItem(pendingHistoryItemRef.current, userMessageTimestamp); + setPendingHistoryItem(null); } - setPendingHistoryItem(null); - } - addItem( - { type: MessageType.INFO, text: 'User cancelled the request.' }, - userMessageTimestamp, - ); - setIsResponding(false); - cancel(); - }; - - const handleErrorEvent = ( - eventValue: ErrorEvent['value'], - userMessageTimestamp: number, - ) => { - if (pendingHistoryItemRef.current) { - addItem(pendingHistoryItemRef.current, userMessageTimestamp); - setPendingHistoryItem(null); - } - addItem( - { type: MessageType.ERROR, text: `[API Error: ${eventValue.message}]` }, - userMessageTimestamp, - ); - }; + addItem( + { type: MessageType.ERROR, text: `[API Error: ${eventValue.message}]` }, + userMessageTimestamp, + ); + }, + [addItem, pendingHistoryItemRef, setPendingHistoryItem], + ); - const processGeminiStreamEvents = async ( - stream: AsyncIterable<GeminiEvent>, - userMessageTimestamp: number, - ): Promise<StreamProcessingStatus> => { - let geminiMessageBuffer = ''; - const toolCallRequests: ToolCallRequestInfo[] = []; - for await (const event of stream) { - if (event.type === ServerGeminiEventType.Content) { - geminiMessageBuffer = handleContentEvent( - event.value, - geminiMessageBuffer, - userMessageTimestamp, - ); - } else if (event.type === ServerGeminiEventType.ToolCallRequest) { - toolCallRequests.push(event.value); - } else if (event.type === ServerGeminiEventType.UserCancelled) { - handleUserCancelledEvent(userMessageTimestamp); - cancel(); - return StreamProcessingStatus.UserCancelled; - } else if (event.type === ServerGeminiEventType.Error) { - handleErrorEvent(event.value, userMessageTimestamp); - return StreamProcessingStatus.Error; + const processGeminiStreamEvents = useCallback( + async ( + stream: AsyncIterable<GeminiEvent>, + userMessageTimestamp: number, + ): Promise<StreamProcessingStatus> => { + let geminiMessageBuffer = ''; + const toolCallRequests: ToolCallRequestInfo[] = []; + for await (const event of stream) { + if (event.type === ServerGeminiEventType.Content) { + geminiMessageBuffer = handleContentEvent( + event.value, + geminiMessageBuffer, + userMessageTimestamp, + ); + } else if (event.type === ServerGeminiEventType.ToolCallRequest) { + toolCallRequests.push(event.value); + } else if (event.type === ServerGeminiEventType.UserCancelled) { + handleUserCancelledEvent(userMessageTimestamp); + cancel(); + return StreamProcessingStatus.UserCancelled; + } else if (event.type === ServerGeminiEventType.Error) { + handleErrorEvent(event.value, userMessageTimestamp); + return StreamProcessingStatus.Error; + } } - } - schedule(toolCallRequests); - return StreamProcessingStatus.Completed; - }; - - const streamingState: StreamingState = - isResponding || - toolCalls.some( - (t) => t.status === 'awaiting_approval' || t.status === 'executing', - ) - ? StreamingState.Responding - : StreamingState.Idle; + schedule(toolCallRequests); + return StreamProcessingStatus.Completed; + }, + [ + handleContentEvent, + handleUserCancelledEvent, + cancel, + handleErrorEvent, + schedule, + ], + ); const submitQuery = useCallback( async (query: PartListUnion) => { - if (isResponding) return; + if ( + streamingState === StreamingState.Responding || + streamingState === StreamingState.WaitingForConfirmation + ) + return; const userMessageTimestamp = Date.now(); setShowHelp(false); @@ -458,10 +501,7 @@ export const useGeminiStream = ( userMessageTimestamp, ); - if ( - processingStatus === StreamProcessingStatus.PausedForConfirmation || - processingStatus === StreamProcessingStatus.UserCancelled - ) { + if (processingStatus === StreamProcessingStatus.UserCancelled) { return; } @@ -484,19 +524,16 @@ export const useGeminiStream = ( setIsResponding(false); } }, - // eslint-disable-next-line react-hooks/exhaustive-deps [ - isResponding, setShowHelp, - handleSlashCommand, - shellModeActive, - handleShellCommand, - config, addItem, - onDebugMessage, - refreshStatic, setInitError, - logger, + ensureChatSession, + prepareQueryForGemini, + processGeminiStreamEvents, + setPendingHistoryItem, + pendingHistoryItemRef, + streamingState, ], ); diff --git a/packages/cli/src/ui/hooks/useLoadingIndicator.test.ts b/packages/cli/src/ui/hooks/useLoadingIndicator.test.ts index 496e13d3..c20ded88 100644 --- a/packages/cli/src/ui/hooks/useLoadingIndicator.test.ts +++ b/packages/cli/src/ui/hooks/useLoadingIndicator.test.ts @@ -4,161 +4,124 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { renderHook, act } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { act, renderHook } from '@testing-library/react'; import { useLoadingIndicator } from './useLoadingIndicator.js'; import { StreamingState } from '../types.js'; import { WITTY_LOADING_PHRASES, PHRASE_CHANGE_INTERVAL_MS, -} from '../constants.js'; +} from './usePhraseCycler.js'; describe('useLoadingIndicator', () => { beforeEach(() => { vi.useFakeTimers(); + vi.clearAllMocks(); }); afterEach(() => { - vi.restoreAllMocks(); + vi.runOnlyPendingTimers(); + vi.useRealTimers(); // Restore real timers after each test }); - it('should initialize with default values when not responding', () => { + it('should initialize with default values when Idle', () => { const { result } = renderHook(() => - useLoadingIndicator(StreamingState.Idle, false), + useLoadingIndicator(StreamingState.Idle), ); expect(result.current.elapsedTime).toBe(0); expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[0]); - expect(result.current.shouldShowSpinner).toBe(true); }); - describe('when streamingState is Responding', () => { - it('should increment elapsedTime and cycle phrases when not paused', () => { - const { result } = renderHook(() => - useLoadingIndicator(StreamingState.Responding, false), - ); - expect(result.current.shouldShowSpinner).toBe(true); - expect(result.current.currentLoadingPhrase).toBe( - WITTY_LOADING_PHRASES[0], - ); + it('should reflect values when Responding', () => { + const { result } = renderHook(() => + useLoadingIndicator(StreamingState.Responding), + ); - act(() => { - vi.advanceTimersByTime(1000); - }); - expect(result.current.elapsedTime).toBe(1); + // Initial state before timers advance + expect(result.current.elapsedTime).toBe(0); + expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[0]); - act(() => { - vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS); - }); - expect(result.current.currentLoadingPhrase).toBe( - WITTY_LOADING_PHRASES[1], - ); - expect(result.current.elapsedTime).toBe( - 1 + PHRASE_CHANGE_INTERVAL_MS / 1000, - ); + act(() => { + vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS); }); + // Phrase should cycle if PHRASE_CHANGE_INTERVAL_MS has passed + // This depends on the actual implementation of usePhraseCycler + // For simplicity, we'll check it's one of the witty phrases + expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[1]); + }); - it('should pause elapsedTime, show specific phrase, and hide spinner when paused', () => { - const { result, rerender } = renderHook( - ({ isPaused }) => - useLoadingIndicator(StreamingState.Responding, isPaused), - { initialProps: { isPaused: false } }, - ); + it('should show waiting phrase and retain elapsedTime when WaitingForConfirmation', () => { + const { result, rerender } = renderHook( + ({ streamingState }) => useLoadingIndicator(streamingState), + { initialProps: { streamingState: StreamingState.Responding } }, + ); - act(() => { - vi.advanceTimersByTime(2000); - }); - expect(result.current.elapsedTime).toBe(2); - expect(result.current.shouldShowSpinner).toBe(true); + act(() => { + vi.advanceTimersByTime(60000); + }); + expect(result.current.elapsedTime).toBe(60); - rerender({ isPaused: true }); + rerender({ streamingState: StreamingState.WaitingForConfirmation }); - expect(result.current.currentLoadingPhrase).toBe( - 'Waiting for user confirmation...', - ); - expect(result.current.shouldShowSpinner).toBe(false); + expect(result.current.currentLoadingPhrase).toBe( + 'Waiting for user confirmation...', + ); + expect(result.current.elapsedTime).toBe(60); // Elapsed time should be retained - // Time should not advance while paused - const timeBeforePauseAdv = result.current.elapsedTime; - act(() => { - vi.advanceTimersByTime(3000); - }); - expect(result.current.elapsedTime).toBe(timeBeforePauseAdv); + // Timer should not advance further + act(() => { + vi.advanceTimersByTime(2000); + }); + expect(result.current.elapsedTime).toBe(60); + }); - // Unpause - rerender({ isPaused: false }); - expect(result.current.shouldShowSpinner).toBe(true); - // Phrase should reset to the beginning of witty phrases - expect(result.current.currentLoadingPhrase).toBe( - WITTY_LOADING_PHRASES[0], - ); + it('should reset elapsedTime and use initial phrase when transitioning from WaitingForConfirmation to Responding', () => { + const { result, rerender } = renderHook( + ({ streamingState }) => useLoadingIndicator(streamingState), + { initialProps: { streamingState: StreamingState.Responding } }, + ); - act(() => { - vi.advanceTimersByTime(1000); - }); - // Elapsed time should resume from where it left off - expect(result.current.elapsedTime).toBe(timeBeforePauseAdv + 1); + act(() => { + vi.advanceTimersByTime(5000); // 5s }); + expect(result.current.elapsedTime).toBe(5); - it('should reset timer and phrase when streamingState changes from Responding to Idle', () => { - const { result, rerender } = renderHook( - ({ streamingState }) => useLoadingIndicator(streamingState, false), - { initialProps: { streamingState: StreamingState.Responding } }, - ); - - act(() => { - vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS + 1000); - }); - expect(result.current.elapsedTime).toBe( - PHRASE_CHANGE_INTERVAL_MS / 1000 + 1, - ); - expect(result.current.currentLoadingPhrase).toBe( - WITTY_LOADING_PHRASES[1], - ); + rerender({ streamingState: StreamingState.WaitingForConfirmation }); + expect(result.current.elapsedTime).toBe(5); + expect(result.current.currentLoadingPhrase).toBe( + 'Waiting for user confirmation...', + ); - rerender({ streamingState: StreamingState.Idle }); + rerender({ streamingState: StreamingState.Responding }); + expect(result.current.elapsedTime).toBe(0); // Should reset + expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[0]); - expect(result.current.elapsedTime).toBe(0); - // When idle, the phrase interval should be cleared, but the last phrase might persist - // until the next "Responding" state. The important part is that the timer is reset. - // Depending on exact timing, it might be the last witty phrase or the first. - // For this test, we'll ensure it's one of them. - expect(WITTY_LOADING_PHRASES).toContain( - result.current.currentLoadingPhrase, - ); + act(() => { + vi.advanceTimersByTime(1000); }); + expect(result.current.elapsedTime).toBe(1); }); - it('should clear intervals on unmount', () => { - const { unmount } = renderHook(() => - useLoadingIndicator(StreamingState.Responding, false), - ); - const clearIntervalSpy = vi.spyOn(global, 'clearInterval'); - unmount(); - // Expecting two intervals (elapsedTime and phraseInterval) to be cleared. - expect(clearIntervalSpy).toHaveBeenCalledTimes(2); - }); - - it('should reset to initial witty phrase when unpaused', () => { + it('should reset timer and phrase when streamingState changes from Responding to Idle', () => { const { result, rerender } = renderHook( - ({ isPaused }) => - useLoadingIndicator(StreamingState.Responding, isPaused), - { initialProps: { isPaused: false } }, + ({ streamingState }) => useLoadingIndicator(streamingState), + { initialProps: { streamingState: StreamingState.Responding } }, ); - // Advance to the second witty phrase act(() => { - vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS); + vi.advanceTimersByTime(10000); // 10s }); - expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[1]); + expect(result.current.elapsedTime).toBe(10); - // Pause - rerender({ isPaused: true }); - expect(result.current.currentLoadingPhrase).toBe( - 'Waiting for user confirmation...', - ); + rerender({ streamingState: StreamingState.Idle }); - // Unpause - rerender({ isPaused: false }); + expect(result.current.elapsedTime).toBe(0); expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[0]); + + // Timer should not advance + act(() => { + vi.advanceTimersByTime(2000); + }); + expect(result.current.elapsedTime).toBe(0); }); }); diff --git a/packages/cli/src/ui/hooks/useLoadingIndicator.ts b/packages/cli/src/ui/hooks/useLoadingIndicator.ts index ac75986a..46754ac1 100644 --- a/packages/cli/src/ui/hooks/useLoadingIndicator.ts +++ b/packages/cli/src/ui/hooks/useLoadingIndicator.ts @@ -4,98 +4,54 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useState, useEffect, useRef } from 'react'; -import { - WITTY_LOADING_PHRASES, - PHRASE_CHANGE_INTERVAL_MS, -} from '../constants.js'; import { StreamingState } from '../types.js'; +import { useTimer } from './useTimer.js'; +import { usePhraseCycler } from './usePhraseCycler.js'; +import { useState, useEffect, useRef } from 'react'; // Added useRef -export const useLoadingIndicator = ( - streamingState: StreamingState, - isPaused: boolean, -) => { - const [elapsedTime, setElapsedTime] = useState(0); - const [currentLoadingPhrase, setCurrentLoadingPhrase] = useState( - WITTY_LOADING_PHRASES[0], - ); - const timerRef = useRef<NodeJS.Timeout | null>(null); - const phraseIntervalRef = useRef<NodeJS.Timeout | null>(null); - const currentPhraseIndexRef = useRef<number>(0); +export const useLoadingIndicator = (streamingState: StreamingState) => { + const [timerResetKey, setTimerResetKey] = useState(0); + const isTimerActive = streamingState === StreamingState.Responding; - const [shouldShowSpinner, setShouldShowSpinner] = useState(true); + const elapsedTimeFromTimer = useTimer(isTimerActive, timerResetKey); - useEffect(() => { - if (streamingState === StreamingState.Responding) { - if (!isPaused) { - if (!timerRef.current) { - // No specific action needed here if timer wasn't running and we are not paused. - // Elapsed time continues from where it left off or starts from 0 if it's a fresh start. - } - if (timerRef.current) clearInterval(timerRef.current); - timerRef.current = setInterval(() => { - setElapsedTime((prevTime) => prevTime + 1); - }, 1000); - } else { - if (timerRef.current) { - clearInterval(timerRef.current); - timerRef.current = null; - } - } - } else { - if (timerRef.current) { - clearInterval(timerRef.current); - timerRef.current = null; - } - setElapsedTime(0); - } + const isPhraseCyclingActive = streamingState === StreamingState.Responding; + const isWaiting = streamingState === StreamingState.WaitingForConfirmation; + const currentLoadingPhrase = usePhraseCycler( + isPhraseCyclingActive, + isWaiting, + ); - return () => { - if (timerRef.current) { - clearInterval(timerRef.current); - timerRef.current = null; - } - }; - }, [streamingState, isPaused]); + const [retainedElapsedTime, setRetainedElapsedTime] = useState(0); + const prevStreamingStateRef = useRef<StreamingState | null>(null); useEffect(() => { - if (streamingState === StreamingState.Responding) { - if (!isPaused) { - setShouldShowSpinner(true); - if (!phraseIntervalRef.current) { - currentPhraseIndexRef.current = 0; - setCurrentLoadingPhrase(WITTY_LOADING_PHRASES[0]); - phraseIntervalRef.current = setInterval(() => { - currentPhraseIndexRef.current = - (currentPhraseIndexRef.current + 1) % - WITTY_LOADING_PHRASES.length; - setCurrentLoadingPhrase( - WITTY_LOADING_PHRASES[currentPhraseIndexRef.current], - ); - }, PHRASE_CHANGE_INTERVAL_MS); - } - } else { - setShouldShowSpinner(false); - setCurrentLoadingPhrase('Waiting for user confirmation...'); - if (phraseIntervalRef.current) { - clearInterval(phraseIntervalRef.current); - phraseIntervalRef.current = null; - } - } - } else { - if (phraseIntervalRef.current) { - clearInterval(phraseIntervalRef.current); - phraseIntervalRef.current = null; - } + if ( + prevStreamingStateRef.current === StreamingState.WaitingForConfirmation && + streamingState === StreamingState.Responding + ) { + setTimerResetKey((prevKey) => prevKey + 1); + setRetainedElapsedTime(0); // Clear retained time when going back to responding + } else if ( + streamingState === StreamingState.Idle && + prevStreamingStateRef.current === StreamingState.Responding + ) { + setTimerResetKey((prevKey) => prevKey + 1); // Reset timer when becoming idle from responding + setRetainedElapsedTime(0); + } else if (streamingState === StreamingState.WaitingForConfirmation) { + // Capture the time when entering WaitingForConfirmation + // elapsedTimeFromTimer will hold the last value from when isTimerActive was true. + setRetainedElapsedTime(elapsedTimeFromTimer); } - return () => { - if (phraseIntervalRef.current) { - clearInterval(phraseIntervalRef.current); - phraseIntervalRef.current = null; - } - }; - }, [streamingState, isPaused]); + prevStreamingStateRef.current = streamingState; + }, [streamingState, elapsedTimeFromTimer]); - return { elapsedTime, currentLoadingPhrase, shouldShowSpinner }; + return { + elapsedTime: + streamingState === StreamingState.WaitingForConfirmation + ? retainedElapsedTime + : elapsedTimeFromTimer, + currentLoadingPhrase, + }; }; diff --git a/packages/cli/src/ui/hooks/usePhraseCycler.test.ts b/packages/cli/src/ui/hooks/usePhraseCycler.test.ts new file mode 100644 index 00000000..f5be12e9 --- /dev/null +++ b/packages/cli/src/ui/hooks/usePhraseCycler.test.ts @@ -0,0 +1,111 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { renderHook, act } from '@testing-library/react'; +import { + usePhraseCycler, + WITTY_LOADING_PHRASES, + PHRASE_CHANGE_INTERVAL_MS, +} from './usePhraseCycler.js'; + +describe('usePhraseCycler', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should initialize with the first witty phrase when not active and not waiting', () => { + const { result } = renderHook(() => usePhraseCycler(false, false)); + expect(result.current).toBe(WITTY_LOADING_PHRASES[0]); + }); + + it('should show "Waiting for user confirmation..." when isWaiting is true', () => { + const { result, rerender } = renderHook( + ({ isActive, isWaiting }) => usePhraseCycler(isActive, isWaiting), + { initialProps: { isActive: true, isWaiting: false } }, + ); + rerender({ isActive: true, isWaiting: true }); + expect(result.current).toBe('Waiting for user confirmation...'); + }); + + it('should not cycle phrases if isActive is false and not waiting', () => { + const { result } = renderHook(() => usePhraseCycler(false, false)); + act(() => { + vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS * 2); + }); + expect(result.current).toBe(WITTY_LOADING_PHRASES[0]); + }); + + it('should cycle through witty phrases when isActive is true and not waiting', () => { + const { result } = renderHook(() => usePhraseCycler(true, false)); + expect(result.current).toBe(WITTY_LOADING_PHRASES[0]); + + act(() => { + vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS); + }); + expect(result.current).toBe(WITTY_LOADING_PHRASES[1]); + + act(() => { + vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS); + }); + expect(result.current).toBe( + WITTY_LOADING_PHRASES[2 % WITTY_LOADING_PHRASES.length], + ); + }); + + it('should reset to the first phrase when isActive becomes true after being false (and not waiting)', () => { + const { result, rerender } = renderHook( + ({ isActive, isWaiting }) => usePhraseCycler(isActive, isWaiting), + { initialProps: { isActive: false, isWaiting: false } }, + ); + // Cycle to a different phrase + rerender({ isActive: true, isWaiting: false }); + act(() => { + vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS); + }); + expect(result.current).not.toBe(WITTY_LOADING_PHRASES[0]); + + // Set to inactive + rerender({ isActive: false, isWaiting: false }); + expect(result.current).toBe(WITTY_LOADING_PHRASES[0]); // Should reset to first phrase + + // Set back to active + rerender({ isActive: true, isWaiting: false }); + expect(result.current).toBe(WITTY_LOADING_PHRASES[0]); // Should start with the first phrase + }); + + it('should clear phrase interval on unmount when active', () => { + const { unmount } = renderHook(() => usePhraseCycler(true, false)); + const clearIntervalSpy = vi.spyOn(global, 'clearInterval'); + unmount(); + expect(clearIntervalSpy).toHaveBeenCalledOnce(); + }); + + it('should reset to the first witty phrase when transitioning from waiting to active', () => { + const { result, rerender } = renderHook( + ({ isActive, isWaiting }) => usePhraseCycler(isActive, isWaiting), + { initialProps: { isActive: true, isWaiting: false } }, + ); + + // Cycle to a different phrase + act(() => { + vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS); + }); + expect(result.current).toBe(WITTY_LOADING_PHRASES[1]); + + // Go to waiting state + rerender({ isActive: false, isWaiting: true }); + expect(result.current).toBe('Waiting for user confirmation...'); + + // Go back to active cycling + rerender({ isActive: true, isWaiting: false }); + expect(result.current).toBe(WITTY_LOADING_PHRASES[0]); + }); +}); diff --git a/packages/cli/src/ui/hooks/usePhraseCycler.ts b/packages/cli/src/ui/hooks/usePhraseCycler.ts new file mode 100644 index 00000000..96bad676 --- /dev/null +++ b/packages/cli/src/ui/hooks/usePhraseCycler.ts @@ -0,0 +1,84 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useState, useEffect, useRef } from 'react'; + +export const WITTY_LOADING_PHRASES = [ + 'Consulting the digital spirits...', + 'Reticulating splines...', + 'Warming up the AI hamsters...', + 'Asking the magic conch shell...', + 'Generating witty retort...', + 'Polishing the algorithms...', + "Don't rush perfection (or my code)...", + 'Brewing fresh bytes...', + 'Counting electrons...', + 'Engaging cognitive processors...', + 'Checking for syntax errors in the universe...', + 'One moment, optimizing humor...', + 'Shuffling punchlines...', + 'Untangling neural nets...', + 'Compiling brilliance...', +]; + +export const PHRASE_CHANGE_INTERVAL_MS = 15000; + +/** + * Custom hook to manage cycling through loading phrases. + * @param isActive Whether the phrase cycling should be active. + * @param isWaiting Whether to show a specific waiting phrase. + * @returns The current loading phrase. + */ +export const usePhraseCycler = (isActive: boolean, isWaiting: boolean) => { + const [currentLoadingPhrase, setCurrentLoadingPhrase] = useState( + WITTY_LOADING_PHRASES[0], + ); + const phraseIntervalRef = useRef<NodeJS.Timeout | null>(null); + const currentPhraseIndexRef = useRef<number>(0); + + useEffect(() => { + if (isWaiting) { + setCurrentLoadingPhrase('Waiting for user confirmation...'); + if (phraseIntervalRef.current) { + clearInterval(phraseIntervalRef.current); + phraseIntervalRef.current = null; + } + } else if (isActive) { + if (phraseIntervalRef.current) { + clearInterval(phraseIntervalRef.current); + } + // Reset to the first witty phrase when starting to respond + currentPhraseIndexRef.current = 0; + setCurrentLoadingPhrase(WITTY_LOADING_PHRASES[0]); + + phraseIntervalRef.current = setInterval(() => { + currentPhraseIndexRef.current = + (currentPhraseIndexRef.current + 1) % WITTY_LOADING_PHRASES.length; + setCurrentLoadingPhrase( + WITTY_LOADING_PHRASES[currentPhraseIndexRef.current], + ); + }, PHRASE_CHANGE_INTERVAL_MS); + } else { + // Idle or other states, clear the phrase interval + // and reset to the first phrase for next active state. + if (phraseIntervalRef.current) { + clearInterval(phraseIntervalRef.current); + phraseIntervalRef.current = null; + } + setCurrentLoadingPhrase(WITTY_LOADING_PHRASES[0]); + currentPhraseIndexRef.current = 0; + } + + return () => { + if (phraseIntervalRef.current) { + clearInterval(phraseIntervalRef.current); + phraseIntervalRef.current = null; + } + }; + }, [isActive, isWaiting]); + + return currentLoadingPhrase; +}; diff --git a/packages/cli/src/ui/hooks/useTimer.test.ts b/packages/cli/src/ui/hooks/useTimer.test.ts new file mode 100644 index 00000000..20d44d17 --- /dev/null +++ b/packages/cli/src/ui/hooks/useTimer.test.ts @@ -0,0 +1,120 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { renderHook, act } from '@testing-library/react'; +import { useTimer } from './useTimer.js'; + +describe('useTimer', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should initialize with 0', () => { + const { result } = renderHook(() => useTimer(false, 0)); + expect(result.current).toBe(0); + }); + + it('should not increment time if isActive is false', () => { + const { result } = renderHook(() => useTimer(false, 0)); + act(() => { + vi.advanceTimersByTime(5000); + }); + expect(result.current).toBe(0); + }); + + it('should increment time every second if isActive is true', () => { + const { result } = renderHook(() => useTimer(true, 0)); + act(() => { + vi.advanceTimersByTime(1000); + }); + expect(result.current).toBe(1); + act(() => { + vi.advanceTimersByTime(2000); + }); + expect(result.current).toBe(3); + }); + + it('should reset to 0 and start incrementing when isActive becomes true from false', () => { + const { result, rerender } = renderHook( + ({ isActive, resetKey }) => useTimer(isActive, resetKey), + { initialProps: { isActive: false, resetKey: 0 } }, + ); + expect(result.current).toBe(0); + + rerender({ isActive: true, resetKey: 0 }); + expect(result.current).toBe(0); // Should reset to 0 upon becoming active + + act(() => { + vi.advanceTimersByTime(1000); + }); + expect(result.current).toBe(1); + }); + + it('should reset to 0 when resetKey changes while active', () => { + const { result, rerender } = renderHook( + ({ isActive, resetKey }) => useTimer(isActive, resetKey), + { initialProps: { isActive: true, resetKey: 0 } }, + ); + act(() => { + vi.advanceTimersByTime(3000); // 3s + }); + expect(result.current).toBe(3); + + rerender({ isActive: true, resetKey: 1 }); // Change resetKey + expect(result.current).toBe(0); // Should reset to 0 + + act(() => { + vi.advanceTimersByTime(1000); + }); + expect(result.current).toBe(1); // Starts incrementing from 0 + }); + + it('should be 0 if isActive is false, regardless of resetKey changes', () => { + const { result, rerender } = renderHook( + ({ isActive, resetKey }) => useTimer(isActive, resetKey), + { initialProps: { isActive: false, resetKey: 0 } }, + ); + expect(result.current).toBe(0); + + rerender({ isActive: false, resetKey: 1 }); + expect(result.current).toBe(0); + }); + + it('should clear timer on unmount', () => { + const { unmount } = renderHook(() => useTimer(true, 0)); + const clearIntervalSpy = vi.spyOn(global, 'clearInterval'); + unmount(); + expect(clearIntervalSpy).toHaveBeenCalledOnce(); + }); + + it('should preserve elapsedTime when isActive becomes false, and reset to 0 when it becomes active again', () => { + const { result, rerender } = renderHook( + ({ isActive, resetKey }) => useTimer(isActive, resetKey), + { initialProps: { isActive: true, resetKey: 0 } }, + ); + + act(() => { + vi.advanceTimersByTime(3000); // Advance to 3 seconds + }); + expect(result.current).toBe(3); + + rerender({ isActive: false, resetKey: 0 }); + expect(result.current).toBe(3); // Time should be preserved when timer becomes inactive + + // Now make it active again, it should reset to 0 + rerender({ isActive: true, resetKey: 0 }); + expect(result.current).toBe(0); + act(() => { + vi.advanceTimersByTime(1000); + }); + expect(result.current).toBe(1); + }); +}); diff --git a/packages/cli/src/ui/hooks/useTimer.ts b/packages/cli/src/ui/hooks/useTimer.ts new file mode 100644 index 00000000..53c266dd --- /dev/null +++ b/packages/cli/src/ui/hooks/useTimer.ts @@ -0,0 +1,65 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useState, useEffect, useRef } from 'react'; + +/** + * Custom hook to manage a timer that increments every second. + * @param isActive Whether the timer should be running. + * @param resetKey A key that, when changed, will reset the timer to 0 and restart the interval. + * @returns The elapsed time in seconds. + */ +export const useTimer = (isActive: boolean, resetKey: unknown) => { + const [elapsedTime, setElapsedTime] = useState(0); + const timerRef = useRef<NodeJS.Timeout | null>(null); + const prevResetKeyRef = useRef(resetKey); + const prevIsActiveRef = useRef(isActive); + + useEffect(() => { + let shouldResetTime = false; + + if (prevResetKeyRef.current !== resetKey) { + shouldResetTime = true; + prevResetKeyRef.current = resetKey; + } + + if (prevIsActiveRef.current === false && isActive) { + // Transitioned from inactive to active + shouldResetTime = true; + } + + if (shouldResetTime) { + setElapsedTime(0); + } + prevIsActiveRef.current = isActive; + + // Manage interval + if (isActive) { + // Clear previous interval unconditionally before starting a new one + // This handles resetKey changes while active, ensuring a fresh interval start. + if (timerRef.current) { + clearInterval(timerRef.current); + } + timerRef.current = setInterval(() => { + setElapsedTime((prev) => prev + 1); + }, 1000); + } else { + if (timerRef.current) { + clearInterval(timerRef.current); + timerRef.current = null; + } + } + + return () => { + if (timerRef.current) { + clearInterval(timerRef.current); + timerRef.current = null; + } + }; + }, [isActive, resetKey]); + + return elapsedTime; +}; |