Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { Text } from 'ink';
import {
CompactToolGroupDisplay,
buildToolSummary,
estimateCompactToolGroupHeight,
isCollapsibleTool,
} from './CompactToolGroupDisplay.js';
import { ToolCallStatus } from '../../types.js';
Expand Down Expand Up @@ -147,6 +148,20 @@ describe('<CompactToolGroupDisplay /> — summary label', () => {
);
expect(lastFrame()).toContain('Ran ls -la');
});

it('wraps long summaries instead of truncating them', () => {
const description =
'packages/cli/src/ui/components/messages/CompactToolGroupDisplay.tsx';
const tool = toolCall({ name: 'ReadFile', description });
const { lastFrame } = render(
<CompactToolGroupDisplay toolCalls={[tool]} contentWidth={30} />,
);
const frame = lastFrame()!;

expect(frame.split('\n').length).toBeGreaterThan(1);
expect(frame).not.toContain('…');
expect(frame.replace(/\s/g, '')).toContain(`Read${description}`);
});
});

describe('buildToolSummary', () => {
Expand Down Expand Up @@ -294,6 +309,67 @@ describe('buildToolSummary', () => {
});
});

describe('estimateCompactToolGroupHeight', () => {
it('returns 0 when there are no tool calls', () => {
expect(estimateCompactToolGroupHeight([], 80)).toBe(0);
Comment thread
han-dreamer marked this conversation as resolved.
});

it('returns 1 for summaries that fit on one line', () => {
expect(estimateCompactToolGroupHeight([toolCall()], 80)).toBe(1);
});

it('accounts for wrapped long summaries', () => {
const description =
'packages/cli/src/ui/components/messages/CompactToolGroupDisplay.tsx';
const tool = toolCall({ name: 'ReadFile', description });

expect(estimateCompactToolGroupHeight([tool], 30)).toBeGreaterThan(1);
});

it('reserves additional width for active summary status', () => {
const description =
'packages/cli/src/ui/components/messages/CompactToolGroupDisplay.tsx';
const completed = toolCall({ name: 'ReadFile', description });
const active = toolCall({
name: 'ReadFile',
description,
status: ToolCallStatus.Executing,
});

expect(estimateCompactToolGroupHeight([active], 30)).toBeGreaterThan(
estimateCompactToolGroupHeight([completed], 30),
);
Comment thread
han-dreamer marked this conversation as resolved.
});

it('reserves timeout label width for active shell summaries', () => {
const description =
'npm test -- --filter packages/cli/src/ui/components/messages';
const activeShell = shellTool({ description });
const activeShellWithTimeout = shellTool({
description,
resultDisplay: {
ansiOutput: [],
totalLines: 0,
totalBytes: 0,
timeoutMs: 30_000,
},
});

expect(
estimateCompactToolGroupHeight([activeShellWithTimeout], 55),
).toBeGreaterThan(estimateCompactToolGroupHeight([activeShell], 55));
});

it('uses terminal display width for wide characters', () => {
const tool = toolCall({
name: 'ReadFile',
description: '中文中文中文中文',
});

expect(estimateCompactToolGroupHeight([tool], 12)).toBe(3);
});
});

describe('isCollapsibleTool', () => {
it('returns true for read/search/list tools', () => {
expect(isCollapsibleTool('ReadFile')).toBe(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,30 @@

import type React from 'react';
import { Box, Text } from 'ink';
import stringWidth from 'string-width';
import wrapAnsi from 'wrap-ansi';
import type { IndividualToolCallDisplay } from '../../types.js';
import { ToolCallStatus } from '../../types.js';
import type { AnsiOutputDisplay } from '@qwen-code/qwen-code-core';
import { ToolDisplayNames } from '@qwen-code/qwen-code-core';
import { t } from '../../../i18n/index.js';
import { SHELL_COMMAND_NAME } from '../../constants.js';
import { ToolStatusIndicator } from '../shared/ToolStatusIndicator.js';
import {
STATUS_INDICATOR_WIDTH,
ToolStatusIndicator,
} from '../shared/ToolStatusIndicator.js';
import { ToolElapsedTime } from '../shared/ToolElapsedTime.js';
import { formatDuration } from '../../utils/formatters.js';

interface CompactToolGroupDisplayProps {
toolCalls: IndividualToolCallDisplay[];
contentWidth: number;
}

const COMPACT_GROUP_HORIZONTAL_PADDING = 2;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] COMPACT_GROUP_HORIZONTAL_PADDING = 2 correctly matches paddingX={1} in the JSX below, but the JSX uses a bare numeric literal — there is no structural link between the constant and the rendered value. If someone changes paddingX in the future, the height estimator silently underestimates, and tool results below get sized wrong. Consider referencing the same constant from the JSX (e.g. paddingX={COMPACT_GROUP_HORIZONTAL_PADDING / 2}) to make the coupling explicit.

— qwen3.7-max via Qwen Code /review

const ELAPSED_TIME_MARGIN_LEFT = 1;
const EXECUTING_ELAPSED_TIME_RESERVED_LABEL = '99h 59m 59s';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] EXECUTING_ELAPSED_TIME_RESERVED_LABEL reserves 13 display columns ('99h 59m 59s') for every executing tool without a timeout, but ToolElapsedTime renders bare elapsed time like '5s' (2–6 columns) and returns null for the first 3 seconds. This shrinks summaryWidth by ~10 unnecessary columns on narrow terminals, potentially causing the summary to wrap one line earlier than the render actually needs. The over-reservation is safe (never starves tool results), but a tighter cap (e.g. '59m 59s' or a dynamic reservation) would reduce unnecessary wrapping at narrow widths.

— qwen3.7-max via Qwen Code /review


// Priority: Confirming > Executing > Error > Canceled > Pending > Success
export function getOverallStatus(
toolCalls: IndividualToolCallDisplay[],
Expand Down Expand Up @@ -62,6 +72,32 @@ function getShellTimeoutMs(
return undefined;
}

function isToolGroupActive(status: ToolCallStatus): boolean {
return (
status === ToolCallStatus.Executing ||
status === ToolCallStatus.Pending ||
status === ToolCallStatus.Confirming
);
}

function getElapsedTimeReservedWidth(
tool: IndividualToolCallDisplay,
status: ToolCallStatus,
): number {
if (status !== ToolCallStatus.Executing) return 0;

const timeoutMs = getShellTimeoutMs(tool);
let label = EXECUTING_ELAPSED_TIME_RESERVED_LABEL;
if (timeoutMs != null && timeoutMs > 0) {
const maxElapsedStr = formatDuration(timeoutMs, {
hideTrailingZeros: true,
});
label = `(${maxElapsedStr} · timeout ${maxElapsedStr})`;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The timeout label format `(${maxElapsedStr} · timeout ${maxElapsedStr})` duplicates the format string from ToolElapsedTime. If the label format changes in one place, the estimator's width reservation silently drifts from the actual rendered width. Consider extracting the format template into a shared constant consumed by both the estimator and the component.

— qwen3.7-max via Qwen Code /review

}

return ELAPSED_TIME_MARGIN_LEFT + stringWidth(label);
}

type ToolCategory =
| 'read'
| 'edit'
Expand Down Expand Up @@ -312,24 +348,46 @@ export function buildToolSummary(
return parts.join(', ');
}

export function estimateCompactToolGroupHeight(
toolCalls: IndividualToolCallDisplay[],
contentWidth: number,
): number {
if (toolCalls.length === 0) return 0;

const overallStatus = getOverallStatus(toolCalls);
const activeTool = getActiveTool(toolCalls);
const isActive = isToolGroupActive(overallStatus);
const summary = `${buildToolSummary(toolCalls, isActive)}${isActive ? '…' : ''}`;
const summaryWidth = Math.max(
1,
contentWidth -
COMPACT_GROUP_HORIZONTAL_PADDING -
STATUS_INDICATOR_WIDTH -
getElapsedTimeReservedWidth(activeTool, overallStatus),
);
const wrappedSummary = wrapAnsi(summary, summaryWidth, {
hard: true,
trim: false,
});

return Math.max(1, wrappedSummary.split('\n').length);
}

export const CompactToolGroupDisplay: React.FC<
CompactToolGroupDisplayProps
> = ({ toolCalls, contentWidth }) => {
if (toolCalls.length === 0) return null;

const overallStatus = getOverallStatus(toolCalls);
const activeTool = getActiveTool(toolCalls);
const isActive =
overallStatus === ToolCallStatus.Executing ||
overallStatus === ToolCallStatus.Pending ||
overallStatus === ToolCallStatus.Confirming;
const isActive = isToolGroupActive(overallStatus);

return (
<Box flexDirection="column" width={contentWidth} paddingX={1} gap={0}>
<Box flexDirection="row">
<ToolStatusIndicator status={overallStatus} name={activeTool.name} />
<Box flexGrow={1}>
<Text wrap="truncate-end" bold>
<Text wrap="wrap" bold>
Comment thread
han-dreamer marked this conversation as resolved.
{buildToolSummary(toolCalls, isActive)}
{isActive && <Text key="ellipsis">…</Text>}
</Text>
Expand Down
34 changes: 34 additions & 0 deletions packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,40 @@ describe('<ToolGroupMessage />', () => {
);
expect(lastFrame()).toMatchSnapshot();
});

it('reserves wrapped compact summary height before sizing tool results', () => {
vi.mocked(ToolMessage).mockClear();
const toolCalls = [
createToolCall({
callId: 'read-long',
name: 'ReadFile',
description:
'packages/cli/src/ui/components/messages/CompactToolGroupDisplay.tsx',
status: ToolCallStatus.Success,
}),
createToolCall({
callId: 'shell-result',
name: 'Shell',
description: 'npm test',
status: ToolCallStatus.Success,
resultDisplay: 'shell output',
}),
];

renderWithProviders(
<ToolGroupMessage
{...baseProps}
contentWidth={30}
toolCalls={toolCalls}
availableTerminalHeight={12}
/>,
);

const call = vi
.mocked(ToolMessage)
.mock.calls.find((c) => c[0].callId === 'shell-result');
expect(call?.[0].availableTerminalHeight).toBe(8);
});
});

describe('Confirmation Handling', () => {
Expand Down
6 changes: 5 additions & 1 deletion packages/cli/src/ui/components/messages/ToolGroupMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { ToolMessage } from './ToolMessage.js';
import { ToolConfirmationMessage } from './ToolConfirmationMessage.js';
import {
CompactToolGroupDisplay,
estimateCompactToolGroupHeight,
isCollapsibleTool,
} from './CompactToolGroupDisplay.js';
import { InlineParallelAgentsDisplay } from './InlineParallelAgentsDisplay.js';
Expand Down Expand Up @@ -432,7 +433,10 @@ export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({
}

// Full expanded view for non-collapsible tools
const collapsibleSummaryHeight = collapsibleTools.length > 0 ? 1 : 0;
const collapsibleSummaryHeight = estimateCompactToolGroupHeight(
collapsibleTools,
contentWidth,
);
const memoryBadgeHeight = hasMemoryBadge ? 1 : 0;
const staticHeight =
/* marginBottom */ 1 + collapsibleSummaryHeight + memoryBadgeHeight;
Expand Down
Loading