mirror of
https://github.com/QwenLM/qwen-code.git
synced 2025-12-20 16:57:46 +00:00
feat: Modify loading indicator to support a paused state (#506)
This commit is contained in:
@@ -5,7 +5,7 @@
|
||||
*/
|
||||
|
||||
import React from 'react';
|
||||
import type { HistoryItem } from '../types.js';
|
||||
import type { HistoryItem, StreamingState } from '../types.js';
|
||||
import { UserMessage } from './messages/UserMessage.js';
|
||||
import { UserShellMessage } from './messages/UserShellMessage.js';
|
||||
import { GeminiMessage } from './messages/GeminiMessage.js';
|
||||
@@ -19,12 +19,14 @@ 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 */}
|
||||
@@ -51,6 +53,7 @@ export const HistoryItemDisplay: React.FC<HistoryItemDisplayProps> = ({
|
||||
toolCalls={item.tools}
|
||||
groupId={item.id}
|
||||
availableTerminalHeight={availableTerminalHeight}
|
||||
streamingState={streamingState}
|
||||
/>
|
||||
)}
|
||||
</Box>
|
||||
|
||||
94
packages/cli/src/ui/components/LoadingIndicator.test.tsx
Normal file
94
packages/cli/src/ui/components/LoadingIndicator.test.tsx
Normal file
@@ -0,0 +1,94 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { render } from 'ink-testing-library';
|
||||
import { Text } from 'ink'; // Import Text directly from ink
|
||||
import { LoadingIndicator } from './LoadingIndicator.js';
|
||||
|
||||
vi.mock('ink-spinner', () => ({
|
||||
default: function MockSpinner() {
|
||||
return <Text>MockSpinner</Text>;
|
||||
},
|
||||
}));
|
||||
|
||||
describe('<LoadingIndicator />', () => {
|
||||
it('should not render when isLoading is false', () => {
|
||||
const { lastFrame } = render(
|
||||
<LoadingIndicator
|
||||
isLoading={false}
|
||||
showSpinner={true}
|
||||
currentLoadingPhrase="Loading..."
|
||||
elapsedTime={0}
|
||||
/>,
|
||||
);
|
||||
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}
|
||||
/>,
|
||||
);
|
||||
|
||||
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
|
||||
});
|
||||
|
||||
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}
|
||||
/>,
|
||||
);
|
||||
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');
|
||||
});
|
||||
|
||||
it('should display the currentLoadingPhrase correctly', () => {
|
||||
const specificPhrase = 'Almost there!';
|
||||
const { lastFrame } = render(
|
||||
<LoadingIndicator
|
||||
isLoading={true}
|
||||
showSpinner={true}
|
||||
currentLoadingPhrase={specificPhrase}
|
||||
elapsedTime={3}
|
||||
/>,
|
||||
);
|
||||
expect(lastFrame()).toContain(specificPhrase);
|
||||
});
|
||||
|
||||
it('should display the elapsedTime correctly', () => {
|
||||
const specificTime = 7;
|
||||
const { lastFrame } = render(
|
||||
<LoadingIndicator
|
||||
isLoading={true}
|
||||
showSpinner={true}
|
||||
currentLoadingPhrase="Working..."
|
||||
elapsedTime={specificTime}
|
||||
/>,
|
||||
);
|
||||
expect(lastFrame()).toContain(`(esc to cancel, ${specificTime}s)`);
|
||||
});
|
||||
});
|
||||
@@ -11,6 +11,7 @@ import { Colors } from '../colors.js';
|
||||
|
||||
interface LoadingIndicatorProps {
|
||||
isLoading: boolean;
|
||||
showSpinner: boolean;
|
||||
currentLoadingPhrase: string;
|
||||
elapsedTime: number;
|
||||
rightContent?: React.ReactNode;
|
||||
@@ -18,21 +19,25 @@ interface LoadingIndicatorProps {
|
||||
|
||||
export const LoadingIndicator: React.FC<LoadingIndicatorProps> = ({
|
||||
isLoading,
|
||||
showSpinner,
|
||||
currentLoadingPhrase,
|
||||
elapsedTime,
|
||||
rightContent,
|
||||
}) => {
|
||||
if (!isLoading) {
|
||||
return null; // Don't render anything if not loading
|
||||
return null;
|
||||
}
|
||||
|
||||
return (
|
||||
<Box marginTop={1} paddingLeft={0}>
|
||||
<Box marginRight={1}>
|
||||
<Spinner type="dots" />
|
||||
</Box>
|
||||
{showSpinner && (
|
||||
<Box marginRight={1}>
|
||||
<Spinner type="dots" />
|
||||
</Box>
|
||||
)}
|
||||
<Text color={Colors.AccentPurple}>
|
||||
{currentLoadingPhrase} (esc to cancel, {elapsedTime}s)
|
||||
{currentLoadingPhrase}
|
||||
{isLoading && ` (esc to cancel, ${elapsedTime}s)`}
|
||||
</Text>
|
||||
<Box flexGrow={1}>{/* Spacer */}</Box>
|
||||
{rightContent && <Box>{rightContent}</Box>}
|
||||
|
||||
@@ -6,7 +6,11 @@
|
||||
|
||||
import React, { useMemo } from 'react';
|
||||
import { Box } from 'ink';
|
||||
import { IndividualToolCallDisplay, ToolCallStatus } from '../../types.js';
|
||||
import {
|
||||
IndividualToolCallDisplay,
|
||||
StreamingState,
|
||||
ToolCallStatus,
|
||||
} from '../../types.js';
|
||||
import { ToolMessage } from './ToolMessage.js';
|
||||
import { ToolConfirmationMessage } from './ToolConfirmationMessage.js';
|
||||
import { Colors } from '../../colors.js';
|
||||
@@ -15,12 +19,14 @@ 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,
|
||||
@@ -72,6 +78,7 @@ export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({
|
||||
? 'low'
|
||||
: 'medium'
|
||||
}
|
||||
streamingState={streamingState}
|
||||
/>
|
||||
</Box>
|
||||
{tool.status === ToolCallStatus.Confirming &&
|
||||
|
||||
152
packages/cli/src/ui/components/messages/ToolMessage.test.tsx
Normal file
152
packages/cli/src/ui/components/messages/ToolMessage.test.tsx
Normal file
@@ -0,0 +1,152 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { render } from 'ink-testing-library';
|
||||
import { ToolMessage, ToolMessageProps } from './ToolMessage.js';
|
||||
import { StreamingState, ToolCallStatus } from '../../types.js';
|
||||
import { Text } from 'ink';
|
||||
|
||||
// Mock child components or utilities if they are complex or have side effects
|
||||
vi.mock('ink-spinner', () => ({
|
||||
default: () => <Text>MockSpinner</Text>,
|
||||
}));
|
||||
vi.mock('./DiffRenderer.js', () => ({
|
||||
DiffRenderer: function MockDiffRenderer({
|
||||
diffContent,
|
||||
}: {
|
||||
diffContent: string;
|
||||
}) {
|
||||
return <Text>MockDiff:{diffContent}</Text>;
|
||||
},
|
||||
}));
|
||||
vi.mock('../../utils/MarkdownDisplay.js', () => ({
|
||||
MarkdownDisplay: function MockMarkdownDisplay({ text }: { text: string }) {
|
||||
return <Text>MockMarkdown:{text}</Text>;
|
||||
},
|
||||
}));
|
||||
|
||||
describe('<ToolMessage />', () => {
|
||||
const baseProps: ToolMessageProps = {
|
||||
callId: 'tool-123',
|
||||
name: 'test-tool',
|
||||
description: 'A tool for testing',
|
||||
resultDisplay: 'Test result',
|
||||
status: ToolCallStatus.Success,
|
||||
availableTerminalHeight: 20,
|
||||
confirmationDetails: undefined,
|
||||
emphasis: 'medium',
|
||||
streamingState: StreamingState.Idle,
|
||||
};
|
||||
|
||||
it('renders basic tool information', () => {
|
||||
const { lastFrame } = render(<ToolMessage {...baseProps} />);
|
||||
const output = lastFrame();
|
||||
expect(output).toContain('✔'); // Success indicator
|
||||
expect(output).toContain('test-tool');
|
||||
expect(output).toContain('A tool for testing');
|
||||
expect(output).toContain('MockMarkdown:Test result');
|
||||
});
|
||||
|
||||
describe('ToolStatusIndicator rendering', () => {
|
||||
it('shows ✔ for Success status', () => {
|
||||
const { lastFrame } = render(
|
||||
<ToolMessage {...baseProps} status={ToolCallStatus.Success} />,
|
||||
);
|
||||
expect(lastFrame()).toContain('✔');
|
||||
});
|
||||
|
||||
it('shows o for Pending status', () => {
|
||||
const { lastFrame } = render(
|
||||
<ToolMessage {...baseProps} status={ToolCallStatus.Pending} />,
|
||||
);
|
||||
expect(lastFrame()).toContain('o');
|
||||
});
|
||||
|
||||
it('shows ? for Confirming status', () => {
|
||||
const { lastFrame } = render(
|
||||
<ToolMessage {...baseProps} status={ToolCallStatus.Confirming} />,
|
||||
);
|
||||
expect(lastFrame()).toContain('?');
|
||||
});
|
||||
|
||||
it('shows - for Canceled status', () => {
|
||||
const { lastFrame } = render(
|
||||
<ToolMessage {...baseProps} status={ToolCallStatus.Canceled} />,
|
||||
);
|
||||
expect(lastFrame()).toContain('-');
|
||||
});
|
||||
|
||||
it('shows x for Error status', () => {
|
||||
const { lastFrame } = render(
|
||||
<ToolMessage {...baseProps} status={ToolCallStatus.Error} />,
|
||||
);
|
||||
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}
|
||||
/>,
|
||||
);
|
||||
expect(lastFrame()).toContain('MockSpinner');
|
||||
expect(lastFrame()).not.toContain('✔');
|
||||
});
|
||||
|
||||
it('shows MockSpinner for Executing status when streamingState is undefined (default behavior)', () => {
|
||||
const { lastFrame } = render(
|
||||
<ToolMessage {...baseProps} status={ToolCallStatus.Executing} />,
|
||||
);
|
||||
expect(lastFrame()).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
|
||||
/>,
|
||||
);
|
||||
expect(lastFrame()).toContain('✔'); // Should be a checkmark, not spinner
|
||||
expect(lastFrame()).not.toContain('MockSpinner');
|
||||
});
|
||||
});
|
||||
|
||||
it('renders DiffRenderer for diff results', () => {
|
||||
const diffResult = {
|
||||
fileDiff: '--- a/file.txt\n+++ b/file.txt\n@@ -1 +1 @@\n-old\n+new',
|
||||
fileName: 'file.txt',
|
||||
};
|
||||
const { lastFrame } = render(
|
||||
<ToolMessage {...baseProps} resultDisplay={diffResult} />,
|
||||
);
|
||||
// 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(
|
||||
<ToolMessage {...baseProps} emphasis="high" />,
|
||||
);
|
||||
// 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(
|
||||
<ToolMessage {...baseProps} emphasis="low" />,
|
||||
);
|
||||
// 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.
|
||||
// We can at least ensure it doesn't have the high emphasis indicator.
|
||||
expect(lowEmphasisFrame()).not.toContain('←');
|
||||
});
|
||||
});
|
||||
@@ -7,7 +7,11 @@
|
||||
import React from 'react';
|
||||
import { Box, Text } from 'ink';
|
||||
import Spinner from 'ink-spinner';
|
||||
import { IndividualToolCallDisplay, ToolCallStatus } from '../../types.js';
|
||||
import {
|
||||
IndividualToolCallDisplay,
|
||||
StreamingState,
|
||||
ToolCallStatus,
|
||||
} from '../../types.js';
|
||||
import { DiffRenderer } from './DiffRenderer.js';
|
||||
import { Colors } from '../../colors.js';
|
||||
import { MarkdownDisplay } from '../../utils/MarkdownDisplay.js';
|
||||
@@ -21,6 +25,7 @@ export type TextEmphasis = 'high' | 'medium' | 'low';
|
||||
export interface ToolMessageProps extends IndividualToolCallDisplay {
|
||||
availableTerminalHeight: number;
|
||||
emphasis?: TextEmphasis;
|
||||
streamingState?: StreamingState;
|
||||
}
|
||||
|
||||
export const ToolMessage: React.FC<ToolMessageProps> = ({
|
||||
@@ -30,6 +35,7 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
|
||||
status,
|
||||
availableTerminalHeight,
|
||||
emphasis = 'medium',
|
||||
streamingState,
|
||||
}) => {
|
||||
const contentHeightEstimate =
|
||||
availableTerminalHeight - STATIC_HEIGHT - RESERVED_LINE_COUNT;
|
||||
@@ -57,7 +63,7 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
|
||||
<Box paddingX={1} paddingY={0} flexDirection="column">
|
||||
<Box minHeight={1}>
|
||||
{/* Status Indicator */}
|
||||
<ToolStatusIndicator status={status} />
|
||||
<ToolStatusIndicator status={status} streamingState={streamingState} />
|
||||
<ToolInfo
|
||||
name={name}
|
||||
status={status}
|
||||
@@ -99,15 +105,32 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
|
||||
);
|
||||
};
|
||||
|
||||
type ToolStatusIndicator = {
|
||||
type ToolStatusIndicatorProps = {
|
||||
status: ToolCallStatus;
|
||||
streamingState?: StreamingState;
|
||||
};
|
||||
const ToolStatusIndicator: React.FC<ToolStatusIndicator> = ({ status }) => (
|
||||
const ToolStatusIndicator: React.FC<ToolStatusIndicatorProps> = ({
|
||||
status,
|
||||
streamingState,
|
||||
}) => (
|
||||
<Box minWidth={STATUS_INDICATOR_WIDTH}>
|
||||
{status === ToolCallStatus.Pending && (
|
||||
<Text color={Colors.AccentGreen}>o</Text>
|
||||
)}
|
||||
{status === ToolCallStatus.Executing && <Spinner type="dots" />}
|
||||
{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>
|
||||
)}
|
||||
|
||||
Reference in New Issue
Block a user