Refactor: Add GeminiRespondingSpinner to make use of streamingState idiomatic (#583)

This commit is contained in:
Jacob Richman 2025-05-28 18:17:19 +00:00 committed by GitHub
parent 98dcf43214
commit 05a49702d8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 122 additions and 67 deletions

View File

@ -0,0 +1,34 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import React from 'react';
import { Text } from 'ink';
import Spinner from 'ink-spinner';
import type { SpinnerName } from 'cli-spinners';
import { useStreamingContext } from '../contexts/StreamingContext.js';
import { StreamingState } from '../types.js';
interface GeminiRespondingSpinnerProps {
/**
* Optional string to display when not in Responding state.
* If not provided and not Responding, renders null.
*/
nonRespondingDisplay?: string;
spinnerType?: SpinnerName;
}
export const GeminiRespondingSpinner: React.FC<
GeminiRespondingSpinnerProps
> = ({ nonRespondingDisplay, spinnerType = 'dots' }) => {
const { streamingState } = useStreamingContext();
if (streamingState === StreamingState.Responding) {
return <Spinner type={spinnerType} />;
} else if (nonRespondingDisplay) {
return <Text>{nonRespondingDisplay}</Text>;
}
return null;
};

View File

@ -15,9 +15,21 @@ import {
import { StreamingState } from '../types.js';
import { vi } from 'vitest';
// Mock ink-spinner
vi.mock('ink-spinner', () => ({
default: () => <Text>MockSpinner</Text>,
// Mock GeminiRespondingSpinner
vi.mock('./GeminiRespondingSpinner.js', () => ({
GeminiRespondingSpinner: ({
nonRespondingDisplay,
}: {
nonRespondingDisplay?: string;
}) => {
const { streamingState } = React.useContext(StreamingContext)!;
if (streamingState === StreamingState.Responding) {
return <Text>MockRespondingSpinner</Text>;
} else if (nonRespondingDisplay) {
return <Text>{nonRespondingDisplay}</Text>;
}
return null;
},
}));
const renderWithContext = (
@ -54,12 +66,12 @@ describe('<LoadingIndicator />', () => {
StreamingState.Responding,
);
const output = lastFrame();
expect(output).toContain('MockSpinner');
expect(output).toContain('MockRespondingSpinner');
expect(output).toContain('Loading...');
expect(output).toContain('(esc to cancel, 5s)');
});
it('should render phrase and time but no spinner when streamingState is WaitingForConfirmation', () => {
it('should render spinner (static), phrase but no time/cancel when streamingState is WaitingForConfirmation', () => {
const props = {
currentLoadingPhrase: 'Confirm action',
elapsedTime: 10,
@ -69,7 +81,7 @@ describe('<LoadingIndicator />', () => {
StreamingState.WaitingForConfirmation,
);
const output = lastFrame();
expect(output).not.toContain('MockSpinner');
expect(output).toContain('⠏'); // Static char for WaitingForConfirmation
expect(output).toContain('Confirm action');
expect(output).not.toContain('(esc to cancel)');
expect(output).not.toContain(', 10s');
@ -127,7 +139,7 @@ describe('<LoadingIndicator />', () => {
</StreamingContext.Provider>,
);
let output = lastFrame();
expect(output).toContain('MockSpinner');
expect(output).toContain('MockRespondingSpinner');
expect(output).toContain('Now Responding');
expect(output).toContain('(esc to cancel, 2s)');
@ -143,7 +155,7 @@ describe('<LoadingIndicator />', () => {
</StreamingContext.Provider>,
);
output = lastFrame();
expect(output).not.toContain('MockSpinner');
expect(output).toContain('⠏');
expect(output).toContain('Please Confirm');
expect(output).not.toContain('(esc to cancel)');
expect(output).not.toContain(', 15s');

View File

@ -6,10 +6,10 @@
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';
import { GeminiRespondingSpinner } from './GeminiRespondingSpinner.js';
interface LoadingIndicatorProps {
currentLoadingPhrase: string;
@ -30,11 +30,13 @@ export const LoadingIndicator: React.FC<LoadingIndicatorProps> = ({
return (
<Box marginTop={1} paddingLeft={0}>
{streamingState === StreamingState.Responding && (
<Box marginRight={1}>
<Spinner type="dots" />
<GeminiRespondingSpinner
nonRespondingDisplay={
streamingState === StreamingState.WaitingForConfirmation ? '⠏' : ''
}
/>
</Box>
)}
<Text color={Colors.AccentPurple}>
{currentLoadingPhrase}
{streamingState === StreamingState.WaitingForConfirmation

View File

@ -4,15 +4,29 @@
* SPDX-License-Identifier: Apache-2.0
*/
import React from 'react';
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';
import {
StreamingContext,
StreamingContextType,
} from '../../contexts/StreamingContext.js';
// Mock child components or utilities if they are complex or have side effects
vi.mock('ink-spinner', () => ({
default: () => <Text>MockSpinner</Text>,
vi.mock('../GeminiRespondingSpinner.js', () => ({
GeminiRespondingSpinner: ({
nonRespondingDisplay,
}: {
nonRespondingDisplay?: string;
}) => {
const { streamingState } = React.useContext(StreamingContext)!;
if (streamingState === StreamingState.Responding) {
return <Text>MockRespondingSpinner</Text>;
}
return nonRespondingDisplay ? <Text>{nonRespondingDisplay}</Text> : null;
},
}));
vi.mock('./DiffRenderer.js', () => ({
DiffRenderer: function MockDiffRenderer({
@ -33,12 +47,14 @@ vi.mock('../../utils/MarkdownDisplay.js', () => ({
const renderWithContext = (
ui: React.ReactElement,
streamingState: StreamingState,
) =>
render(
<StreamingContext.Provider value={{ streamingState }}>
) => {
const contextValue: StreamingContextType = { streamingState };
return render(
<StreamingContext.Provider value={contextValue}>
{ui}
</StreamingContext.Provider>,
);
};
describe('<ToolMessage />', () => {
const baseProps: ToolMessageProps = {
@ -110,8 +126,8 @@ describe('<ToolMessage />', () => {
<ToolMessage {...baseProps} status={ToolCallStatus.Executing} />,
StreamingState.Idle,
);
expect(lastFrame()).toContain('');
expect(lastFrame()).not.toContain('MockSpinner');
expect(lastFrame()).toContain('');
expect(lastFrame()).not.toContain('MockRespondingSpinner');
expect(lastFrame()).not.toContain('✔');
});
@ -120,17 +136,17 @@ describe('<ToolMessage />', () => {
<ToolMessage {...baseProps} status={ToolCallStatus.Executing} />,
StreamingState.WaitingForConfirmation,
);
expect(lastFrame()).toContain('');
expect(lastFrame()).not.toContain('MockSpinner');
expect(lastFrame()).toContain('');
expect(lastFrame()).not.toContain('MockRespondingSpinner');
expect(lastFrame()).not.toContain('✔');
});
it('shows MockSpinner for Executing status when streamingState is Responding', () => {
it('shows MockRespondingSpinner for Executing status when streamingState is Responding', () => {
const { lastFrame } = renderWithContext(
<ToolMessage {...baseProps} status={ToolCallStatus.Executing} />,
StreamingState.Responding, // Simulate app still responding
);
expect(lastFrame()).toContain('MockSpinner');
expect(lastFrame()).toContain('MockRespondingSpinner');
expect(lastFrame()).not.toContain('✔');
});
});

View File

@ -6,16 +6,11 @@
import React from 'react';
import { Box, Text } from 'ink';
import Spinner from 'ink-spinner';
import {
IndividualToolCallDisplay,
StreamingState,
ToolCallStatus,
} from '../../types.js';
import { IndividualToolCallDisplay, ToolCallStatus } from '../../types.js';
import { DiffRenderer } from './DiffRenderer.js';
import { Colors } from '../../colors.js';
import { MarkdownDisplay } from '../../utils/MarkdownDisplay.js';
import { useStreamingContext } from '../../contexts/StreamingContext.js';
import { GeminiRespondingSpinner } from '../GeminiRespondingSpinner.js';
const STATIC_HEIGHT = 1;
const RESERVED_LINE_COUNT = 5; // for tool name, status, padding etc.
@ -61,7 +56,6 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
return (
<Box paddingX={1} paddingY={0} flexDirection="column">
<Box minHeight={1}>
{/* Status Indicator */}
<ToolStatusIndicator status={status} />
<ToolInfo
name={name}
@ -107,22 +101,20 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
type ToolStatusIndicatorProps = {
status: ToolCallStatus;
};
const ToolStatusIndicator: React.FC<ToolStatusIndicatorProps> = ({
status,
}) => {
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="toggle" />
) : (
// Paused spinner to avoid flicker.
<Text></Text>
))}
{status === ToolCallStatus.Executing && (
<GeminiRespondingSpinner
spinnerType="toggle"
nonRespondingDisplay={'⊷'}
/>
)}
{status === ToolCallStatus.Success && (
<Text color={Colors.AccentGreen}></Text>
)}
@ -140,8 +132,7 @@ const ToolStatusIndicator: React.FC<ToolStatusIndicatorProps> = ({
</Text>
)}
</Box>
);
};
);
type ToolInfo = {
name: string;