diff options
Diffstat (limited to 'packages/cli/src/ui/components')
6 files changed, 223 insertions, 155 deletions
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; |