From 69da115dcf338fb934498ce0d831eb57d5a7a11d Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Thu, 23 Apr 2026 08:52:37 +0800 Subject: [PATCH] feat(cli): combine elapsed + timeout in shell time indicator (#3512) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(cli): combine elapsed + timeout in shell time indicator Render shell tools that have an explicit timeout as `(elapsed · timeout N)` inline with the Running… status from t=0, instead of splitting the information across the right-aligned elapsed indicator and the ShellStatsBar row. - formatters: add a `hideTrailingZeros` option so whole seconds render as `5s` rather than `5.0s` while fractional values like `5.5s` stay intact - ToolElapsedTime: accept optional `timeoutMs`; when set, skip the 3s quiet threshold and render the combined `(elapsed · timeout N)` label - ToolMessage: extract `timeoutMs` from AnsiOutputDisplay and feed it to ToolElapsedTime - ShellStatsBar: drop its `timeoutMs` field (now inline); keeps `+N lines` and memory usage only - Unify both modes on `formatDuration` so hour-range output is consistent (`1h 2m 6s` across timeout and no-timeout paths) * feat(cli): thread shell timeoutMs through compact tool group display The combined `(elapsed · timeout N)` format introduced in the previous commit was only wired through the expanded ToolMessage path. Compact tool groups kept rendering ToolElapsedTime without timeoutMs, so shell tools displayed in compact mode silently dropped the timeout budget. - CompactToolGroupDisplay: add getShellTimeoutMs() to pull timeoutMs off the active tool's AnsiOutputDisplay result (same shape used by ToolMessage) and feed it to ToolElapsedTime - add CompactToolGroupDisplay.test.tsx covering the three paths: ansi display with timeoutMs, ansi display without timeoutMs, and non-ansi resultDisplay (string) --- packages/cli/src/ui/components/AnsiOutput.tsx | 7 +- .../messages/CompactToolGroupDisplay.test.tsx | 93 ++++++++++++ .../messages/CompactToolGroupDisplay.tsx | 20 +++ .../ui/components/messages/ToolMessage.tsx | 17 ++- .../shared/ToolElapsedTime.test.tsx | 141 ++++++++++++++++++ .../ui/components/shared/ToolElapsedTime.tsx | 53 ++++--- packages/cli/src/ui/utils/formatters.test.ts | 26 ++++ packages/cli/src/ui/utils/formatters.ts | 20 ++- 8 files changed, 344 insertions(+), 33 deletions(-) create mode 100644 packages/cli/src/ui/components/messages/CompactToolGroupDisplay.test.tsx create mode 100644 packages/cli/src/ui/components/shared/ToolElapsedTime.test.tsx diff --git a/packages/cli/src/ui/components/AnsiOutput.tsx b/packages/cli/src/ui/components/AnsiOutput.tsx index c13f4f606..1821aacee 100644 --- a/packages/cli/src/ui/components/AnsiOutput.tsx +++ b/packages/cli/src/ui/components/AnsiOutput.tsx @@ -11,7 +11,7 @@ import type { AnsiOutput, AnsiToken, } from '@qwen-code/qwen-code-core'; -import { formatDuration, formatMemoryUsage } from '../utils/formatters.js'; +import { formatMemoryUsage } from '../utils/formatters.js'; import { theme } from '../semantic-colors.js'; import { MaxSizedBox } from './shared/MaxSizedBox.js'; @@ -62,23 +62,18 @@ export const AnsiOutputText: React.FC = ({ export interface ShellStatsBarProps { totalLines?: number; totalBytes?: number; - timeoutMs?: number; displayHeight?: number; } export const ShellStatsBar: React.FC = ({ totalLines, totalBytes, - timeoutMs, displayHeight = DEFAULT_HEIGHT, }) => { const parts: string[] = []; if (totalLines && totalLines > displayHeight) { parts.push(`+${totalLines - displayHeight} lines`); } - if (timeoutMs) { - parts.push(`timeout ${formatDuration(timeoutMs)}`); - } if (totalBytes && totalBytes > 0) { parts.push(formatMemoryUsage(totalBytes)); } diff --git a/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.test.tsx b/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.test.tsx new file mode 100644 index 000000000..e0180c3d1 --- /dev/null +++ b/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.test.tsx @@ -0,0 +1,93 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render } from 'ink-testing-library'; +import { Text } from 'ink'; +import { CompactToolGroupDisplay } from './CompactToolGroupDisplay.js'; +import { ToolCallStatus } from '../../types.js'; +import type { IndividualToolCallDisplay } from '../../types.js'; + +// ToolStatusIndicator pulls in GeminiRespondingSpinner which requires +// StreamingContext; stub it out so we can test the elapsed/timeout +// plumbing in isolation. +vi.mock('../shared/ToolStatusIndicator.js', () => ({ + ToolStatusIndicator: () => •, + STATUS_INDICATOR_WIDTH: 2, +})); + +const NOW = 1_700_000_000_000; + +function shellTool( + overrides: Partial = {}, +): IndividualToolCallDisplay { + return { + callId: 'c1', + name: 'Shell', + description: 'sleep 10', + status: ToolCallStatus.Executing, + executionStartTime: NOW, + resultDisplay: undefined, + confirmationDetails: undefined, + ...overrides, + }; +} + +describe('', () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(NOW); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('surfaces shell timeoutMs inline via ToolElapsedTime', () => { + const tool = shellTool({ + resultDisplay: { + ansiOutput: [], + totalLines: 0, + totalBytes: 0, + timeoutMs: 30_000, + }, + }); + const { lastFrame } = render( + , + ); + expect(lastFrame()).toContain('(0s · timeout 30s)'); + }); + + it('falls back to quiet elapsed-only when no timeout is surfaced', () => { + const tool = shellTool({ + resultDisplay: { + ansiOutput: [], + totalLines: 0, + totalBytes: 0, + }, + }); + const { lastFrame } = render( + , + ); + // Sub-3s without a timeout budget → indicator is quiet. + expect(lastFrame()).not.toContain('timeout'); + expect(lastFrame()).not.toContain('0s'); + }); + + it('ignores non-ansi resultDisplay shapes', () => { + const tool = shellTool({ + resultDisplay: 'plain text output', + }); + const { lastFrame, rerender } = render( + , + ); + vi.advanceTimersByTime(5_000); + rerender(); + // No timeout in display → legacy 3s-threshold elapsed. + expect(lastFrame()).toContain('5s'); + expect(lastFrame()).not.toContain('timeout'); + }); +}); diff --git a/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.tsx b/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.tsx index 2514f8bec..063a0c283 100644 --- a/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.tsx +++ b/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.tsx @@ -8,6 +8,7 @@ import type React from 'react'; import { Box, Text } from 'ink'; import type { IndividualToolCallDisplay } from '../../types.js'; import { ToolCallStatus } from '../../types.js'; +import type { AnsiOutputDisplay } from '@qwen-code/qwen-code-core'; import { SHELL_COMMAND_NAME, SHELL_NAME } from '../../constants.js'; import { theme } from '../../semantic-colors.js'; import { t } from '../../../i18n/index.js'; @@ -47,6 +48,24 @@ function getActiveTool( ); } +// Pull the configured shell timeout off an AnsiOutputDisplay result so +// ToolElapsedTime can surface it inline (matches the expanded +// ToolMessage path). Non-ansi resultDisplay → undefined → legacy +// quiet-then-elapsed behavior. +function getShellTimeoutMs( + tool: IndividualToolCallDisplay, +): number | undefined { + const display = tool.resultDisplay; + if ( + typeof display === 'object' && + display !== null && + 'ansiOutput' in display + ) { + return (display as AnsiOutputDisplay).timeoutMs; + } + return undefined; +} + export const CompactToolGroupDisplay: React.FC< CompactToolGroupDisplayProps > = ({ toolCalls, contentWidth }) => { @@ -107,6 +126,7 @@ export const CompactToolGroupDisplay: React.FC< diff --git a/packages/cli/src/ui/components/messages/ToolMessage.tsx b/packages/cli/src/ui/components/messages/ToolMessage.tsx index 0068c0c35..64f616cfc 100644 --- a/packages/cli/src/ui/components/messages/ToolMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolMessage.tsx @@ -147,7 +147,6 @@ const useResultDisplayRenderer = ( stats: { totalLines: display.totalLines, totalBytes: display.totalBytes, - timeoutMs: display.timeoutMs, }, }; } @@ -314,6 +313,21 @@ export const ToolMessage: React.FC = ({ } }, [resultDisplay]); + // Shell tools surface their configured timeout via AnsiOutputDisplay as + // soon as streaming starts. Feed it into ToolElapsedTime so the budget is + // shown inline (`(elapsed · timeout N)`) instead of in a separate stats + // row. + const shellTimeoutMs = React.useMemo(() => { + if ( + typeof resultDisplay === 'object' && + resultDisplay !== null && + 'ansiOutput' in resultDisplay + ) { + return (resultDisplay as AnsiOutputDisplay).timeoutMs; + } + return undefined; + }, [resultDisplay]); + React.useEffect(() => { if (!lastUpdateTime) { return; @@ -410,6 +424,7 @@ export const ToolMessage: React.FC = ({ {emphasis === 'high' && } diff --git a/packages/cli/src/ui/components/shared/ToolElapsedTime.test.tsx b/packages/cli/src/ui/components/shared/ToolElapsedTime.test.tsx new file mode 100644 index 000000000..e4a157454 --- /dev/null +++ b/packages/cli/src/ui/components/shared/ToolElapsedTime.test.tsx @@ -0,0 +1,141 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render } from 'ink-testing-library'; +import { ToolCallStatus } from '../../types.js'; +import { ToolElapsedTime } from './ToolElapsedTime.js'; + +describe('', () => { + const NOW = 1_700_000_000_000; + + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(NOW); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('renders nothing for non-executing status', () => { + const { lastFrame } = render( + , + ); + expect(lastFrame()).toBe(''); + }); + + it('stays quiet for the first 3s when no timeout is set', () => { + const { lastFrame, rerender } = render( + , + ); + expect(lastFrame()).toBe(''); + + vi.advanceTimersByTime(2_000); + rerender( + , + ); + expect(lastFrame()).toBe(''); + }); + + it('shows elapsed seconds past the 3s threshold (no timeout)', () => { + const { lastFrame, rerender } = render( + , + ); + vi.advanceTimersByTime(5_000); + rerender( + , + ); + expect(lastFrame()).toContain('5s'); + }); + + it('renders combined (elapsed · timeout N) from t=0 when timeout is set', () => { + const { lastFrame } = render( + , + ); + expect(lastFrame()).toContain('(0s · timeout 30s)'); + }); + + it('keeps fractional timeout precision', () => { + const { lastFrame } = render( + , + ); + expect(lastFrame()).toContain('(0s · timeout 5.5s)'); + }); + + it('advances elapsed inside the combined format', () => { + const { lastFrame, rerender } = render( + , + ); + vi.advanceTimersByTime(7_000); + rerender( + , + ); + expect(lastFrame()).toContain('(7s · timeout 30s)'); + }); + + it('formats combined output once elapsed crosses into the minute range', () => { + const { lastFrame, rerender } = render( + , + ); + vi.advanceTimersByTime(65_000); + rerender( + , + ); + expect(lastFrame()).toContain('(1m 5s · timeout 5m)'); + }); + + it('ignores non-positive timeouts (falls back to elapsed-only mode)', () => { + const { lastFrame } = render( + , + ); + // With no effective timeout, sub-3s = quiet. + expect(lastFrame()).toBe(''); + }); +}); diff --git a/packages/cli/src/ui/components/shared/ToolElapsedTime.tsx b/packages/cli/src/ui/components/shared/ToolElapsedTime.tsx index d4740d89b..5c0e09875 100644 --- a/packages/cli/src/ui/components/shared/ToolElapsedTime.tsx +++ b/packages/cli/src/ui/components/shared/ToolElapsedTime.tsx @@ -9,39 +9,34 @@ import { useState, useEffect } from 'react'; import { Box, Text } from 'ink'; import { ToolCallStatus } from '../../types.js'; import { theme } from '../../semantic-colors.js'; - -/** - * Formats elapsed seconds as compact text. - * Under 60s: "3s", "45s". - * 60s+: "1m", "1m 30s", "2h 15m". - */ -export function formatElapsed(seconds: number): string { - if (seconds < 60) return `${seconds}s`; - const minutes = Math.floor(seconds / 60); - const remainingSeconds = seconds % 60; - if (minutes < 60) { - return remainingSeconds > 0 - ? `${minutes}m ${remainingSeconds}s` - : `${minutes}m`; - } - const hours = Math.floor(minutes / 60); - const remainingMinutes = minutes % 60; - return remainingMinutes > 0 ? `${hours}h ${remainingMinutes}m` : `${hours}h`; -} +import { formatDuration } from '../../utils/formatters.js'; interface ToolElapsedTimeProps { status: ToolCallStatus; executionStartTime?: number; + /** + * When provided, the elapsed indicator becomes a combined budget display: + * `(elapsed · timeout N)` visible from t=0 so the timeout is always on + * screen. When absent, the indicator keeps the 3-second quiet threshold + * and renders just the elapsed time. + */ + timeoutMs?: number; } /** - * Right-aligned elapsed-time indicator for an executing tool. Renders - * nothing until the tool has been running for at least 3 seconds, so quick - * tools stay visually quiet. + * Right-aligned elapsed-time indicator for an executing tool. + * + * Two modes: + * - no `timeoutMs`: suppressed for the first 3 seconds so fast tools stay + * visually quiet. + * - with `timeoutMs`: rendered as `(elapsed · timeout N)` from t=0 so the + * user can see both how long the tool has been running and how much + * budget remains. */ export const ToolElapsedTime: React.FC = ({ status, executionStartTime, + timeoutMs, }) => { const [elapsedSeconds, setElapsedSeconds] = useState(0); @@ -57,11 +52,21 @@ export const ToolElapsedTime: React.FC = ({ return () => clearInterval(interval); }, [status, executionStartTime]); - if (status !== ToolCallStatus.Executing || elapsedSeconds < 3) return null; + if (status !== ToolCallStatus.Executing) return null; + + const hasTimeout = timeoutMs != null && timeoutMs > 0; + if (!hasTimeout && elapsedSeconds < 3) return null; + + const elapsedStr = formatDuration(elapsedSeconds * 1000, { + hideTrailingZeros: true, + }); + const label = hasTimeout + ? `(${elapsedStr} · timeout ${formatDuration(timeoutMs, { hideTrailingZeros: true })})` + : elapsedStr; return ( - {formatElapsed(elapsedSeconds)} + {label} ); }; diff --git a/packages/cli/src/ui/utils/formatters.test.ts b/packages/cli/src/ui/utils/formatters.test.ts index 09173e10e..7ef73fd94 100644 --- a/packages/cli/src/ui/utils/formatters.test.ts +++ b/packages/cli/src/ui/utils/formatters.test.ts @@ -154,6 +154,32 @@ describe('formatters', () => { it('should handle negative durations', () => { expect(formatDuration(-100)).toBe('0s'); }); + + describe('with hideTrailingZeros', () => { + it('drops .0 suffix for whole seconds under a minute', () => { + expect(formatDuration(5000, { hideTrailingZeros: true })).toBe('5s'); + expect(formatDuration(10000, { hideTrailingZeros: true })).toBe('10s'); + expect(formatDuration(30000, { hideTrailingZeros: true })).toBe('30s'); + }); + + it('keeps fractional seconds under a minute', () => { + expect(formatDuration(5500, { hideTrailingZeros: true })).toBe('5.5s'); + expect(formatDuration(12345, { hideTrailingZeros: true })).toBe( + '12.3s', + ); + }); + + it('does not affect ms-range output', () => { + expect(formatDuration(500, { hideTrailingZeros: true })).toBe('500ms'); + }); + + it('does not affect multi-unit output', () => { + expect(formatDuration(123000, { hideTrailingZeros: true })).toBe( + '2m 3s', + ); + expect(formatDuration(3600000, { hideTrailingZeros: true })).toBe('1h'); + }); + }); }); describe('formatTokenCount', () => { diff --git a/packages/cli/src/ui/utils/formatters.ts b/packages/cli/src/ui/utils/formatters.ts index 38afaaa30..36ed878d4 100644 --- a/packages/cli/src/ui/utils/formatters.ts +++ b/packages/cli/src/ui/utils/formatters.ts @@ -65,7 +65,19 @@ export const formatTokenCount = (count: number): string => { return `${Math.floor(count / 1000)}k`; }; -export const formatDuration = (milliseconds: number): string => { +export interface FormatDurationOptions { + /** + * When true, drops a trailing `.0` in the sub-minute range so that whole + * seconds render as `5s` rather than `5.0s`. Non-integer values keep their + * decimal (e.g. `5.5s`). Matches Claude Code's `ShellTimeDisplay` style. + */ + hideTrailingZeros?: boolean; +} + +export const formatDuration = ( + milliseconds: number, + options?: FormatDurationOptions, +): string => { if (milliseconds <= 0) { return '0s'; } @@ -77,7 +89,11 @@ export const formatDuration = (milliseconds: number): string => { const totalSeconds = milliseconds / 1000; if (totalSeconds < 60) { - return `${totalSeconds.toFixed(1)}s`; + const formatted = totalSeconds.toFixed(1); + if (options?.hideTrailingZeros && formatted.endsWith('.0')) { + return `${formatted.slice(0, -2)}s`; + } + return `${formatted}s`; } const hours = Math.floor(totalSeconds / 3600);