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 @@ -125,7 +125,7 @@ describe('<CompactToolGroupDisplay /> — summary label', () => {
const frame = lastFrame()!;
// CATEGORY_ORDER: search → read → list → ...
expect(frame).toContain('Searched search pattern');
expect(frame).toContain('read 2 files');
expect(frame).toContain('read a.ts, b.ts');
});

it('renders nothing for empty tool calls', () => {
Expand Down Expand Up @@ -163,7 +163,7 @@ describe('<CompactToolGroupDisplay /> — summary label', () => {
expect(frame.replace(/\s/g, '')).toContain(`Read${description}`);
});

it('shows the latest executing description while a batch is active', () => {
it('shows all descriptions inline when a batch is active with ≤ 3 tools', () => {
const tools = [
toolCall({
callId: 'c1',
Expand All @@ -188,9 +188,9 @@ describe('<CompactToolGroupDisplay /> — summary label', () => {
);
const frame = lastFrame()!;

expect(frame).toContain('Reading 3 files…');
expect(frame).toContain('⎿ current.ts');
expect(frame).not.toContain('queued.ts');
expect(frame).toContain('Reading completed.ts, current.ts, queued.ts…');
// No redundant hint line when descriptions are already inline.
expect(frame).not.toContain('');
});

it('hides the description hint when a batch completes', () => {
Expand All @@ -203,7 +203,7 @@ describe('<CompactToolGroupDisplay /> — summary label', () => {
);
const frame = lastFrame()!;

expect(frame).toContain('Read 2 files');
expect(frame).toContain('Read a.ts, b.ts');
expect(frame).not.toContain('⎿');
});

Expand Down Expand Up @@ -268,13 +268,14 @@ describe('<CompactToolGroupDisplay /> — summary label', () => {
);
const frame = lastFrame()!;

expect(frame.split('\n')).toHaveLength(2);
expect(frame).toContain('Reading 30 files…');
// Summary line wraps + hint line → at least 2 rows at 80 columns.
expect(frame.split('\n').length).toBeGreaterThanOrEqual(2);
expect(frame).toContain('... and 28 more');
expect(frame).toContain('⎿ packages/cli/src/ui/components/example-30.tsx');
expect(frame).not.toContain('example-01.tsx');
expect(frame).not.toContain('example-03.tsx');
});

it('truncates a long active hint to one row', () => {
it('wraps a long inline summary without a redundant hint', () => {
const currentPath =
'packages/cli/src/ui/components/messages/CompactToolGroupDisplay.tsx';
const tools = [
Expand All @@ -290,12 +291,12 @@ describe('<CompactToolGroupDisplay /> — summary label', () => {
<CompactToolGroupDisplay toolCalls={tools} contentWidth={30} />,
);
const frame = lastFrame()!;
const lines = frame.split('\n');

expect(lines).toHaveLength(2);
expect(lines[1]).toContain('⎿ packages/cli');
expect(lines[1]).toMatch(/…$/);
expect(frame).not.toContain(currentPath);
// Summary wraps across multiple lines but no hint row.
expect(frame).not.toContain('⎿');
// Both descriptions appear in the wrapped summary (may be split across lines).
expect(frame).toContain('a.ts');
expect(frame).toContain('Display.tsx');
});
});

Expand Down Expand Up @@ -324,21 +325,52 @@ describe('buildToolSummary', () => {
expect(buildToolSummary([make({})], true)).toBe('Reading a.ts');
});

it('multiple same-type tools use count format', () => {
it('multiple same-type tools show descriptions inline when ≤ 3', () => {
const tools = [
make({ callId: 'c1', description: 'a.ts' }),
make({ callId: 'c2', description: 'b.ts' }),
make({ callId: 'c3', description: 'c.ts' }),
];
expect(buildToolSummary(tools, false)).toBe('Read 3 files');
expect(buildToolSummary(tools, false)).toBe('Read a.ts, b.ts, c.ts');
});

it('multiple same-type tools use progressive verb when active', () => {
const tools = [
make({ callId: 'c1', description: 'a.ts' }),
make({ callId: 'c2', description: 'b.ts' }),
];
expect(buildToolSummary(tools, true)).toBe('Reading 2 files');
expect(buildToolSummary(tools, true)).toBe('Reading a.ts, b.ts');
});

it('multiple same-type tools show first 2 + "...and N more" when > 3', () => {
const tools = [
make({ callId: 'c1', description: 'a.ts' }),
make({ callId: 'c2', description: 'b.ts' }),
make({ callId: 'c3', description: 'c.ts' }),
make({ callId: 'c4', description: 'd.ts' }),
];
expect(buildToolSummary(tools, false)).toBe(
'Read a.ts, b.ts, ... and 2 more',
);
});

it('multiple same-type tools fall back to count when descriptions are missing', () => {
const tools = [
make({ callId: 'c1', description: 'a.ts' }),
make({ callId: 'c2', description: '' }),
make({ callId: 'c3', description: 'c.ts' }),
];
expect(buildToolSummary(tools, false)).toBe('Read 3 files');
});

it('more than 3 tools fall back to count when preview descriptions are missing', () => {
const tools = [
make({ callId: 'c1', description: '' }),
make({ callId: 'c2', description: '' }),
Comment on lines +366 to +369

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 > 3 tools fallback branch is only tested when both of the first two preview descriptions are missing (previewDescs.length === 0). The sub-case where exactly one of the first two has a usable description (previewDescs.length === 1) is never exercised. — Failure scenario: With 4+ same-category tools where descriptions are ['a.ts', '', 'c.ts', 'd.ts'], a future change loosening the condition to previewDescs.length > 0 would ship 'Read a.ts, ... and 3 more' with only one description shown, and no test would catch it.

Suggested change
it('more than 3 tools fall back to count when preview descriptions are missing', () => {
const tools = [
make({ callId: 'c1', description: '' }),
make({ callId: 'c2', description: '' }),
it('more than 3 tools fall back to count when preview descriptions are missing', () => {
const tools = [
make({ callId: 'c1', description: '' }),
make({ callId: 'c2', description: '' }),
make({ callId: 'c3', description: 'c.ts' }),
make({ callId: 'c4', description: 'd.ts' }),
];
expect(buildToolSummary(tools, false)).toBe('Read 4 files');
});
it('more than 3 tools fall back to count when only one preview description is available', () => {
const tools = [
make({ callId: 'c1', description: 'a.ts' }),
make({ callId: 'c2', description: '' }),
make({ callId: 'c3', description: 'c.ts' }),
make({ callId: 'c4', description: 'd.ts' }),
];
expect(buildToolSummary(tools, false)).toBe('Read 4 files');
});

— qwen3.7-max via Qwen Code /review

make({ callId: 'c3', description: 'c.ts' }),
make({ callId: 'c4', description: 'd.ts' }),
];
expect(buildToolSummary(tools, false)).toBe('Read 4 files');
});

it('mixed types joined with comma and lowercase verbs', () => {
Expand Down Expand Up @@ -441,13 +473,15 @@ describe('buildToolSummary', () => {
expect(buildToolSummary(tools, false)).toBe('Ran echo hello world');
});

it('mixed group uses count format per category', () => {
it('mixed group shows descriptions inline per category', () => {
const tools = [
make({ callId: 'c1', name: 'ReadFile', description: 'a.ts' }),
make({ callId: 'c2', name: 'ReadFile', description: 'b.ts' }),
make({ callId: 'c3', name: 'Shell', description: 'npm test' }),
];
expect(buildToolSummary(tools, false)).toBe('Read 2 files, ran npm test');
expect(buildToolSummary(tools, false)).toBe(
'Read a.ts, b.ts, ran npm test',
);
});

it('legacy display names map to correct categories', () => {
Expand Down Expand Up @@ -478,7 +512,23 @@ describe('estimateCompactToolGroupHeight', () => {
expect(estimateCompactToolGroupHeight([tool], 30)).toBeGreaterThan(1);
});

it('adds one row for an active batch description hint', () => {
it('adds one row for an active batch description hint when > 3 tools', () => {
const tools = [
toolCall({ callId: 'c1', name: 'ReadFile', description: 'a.ts' }),
toolCall({ callId: 'c2', name: 'ReadFile', description: 'b.ts' }),
toolCall({ callId: 'c3', name: 'ReadFile', description: 'c.ts' }),
toolCall({
callId: 'c4',
name: 'ReadFile',
description: 'd.ts',
status: ToolCallStatus.Executing,
}),
];

expect(estimateCompactToolGroupHeight(tools, 80)).toBe(2);
});

it('uses one row for an active batch with ≤ 3 tools (descriptions inline)', () => {
const tools = [
toolCall({ callId: 'c1', name: 'ReadFile', description: 'a.ts' }),
toolCall({
Expand All @@ -489,7 +539,7 @@ describe('estimateCompactToolGroupHeight', () => {
}),
];

expect(estimateCompactToolGroupHeight(tools, 80)).toBe(2);
expect(estimateCompactToolGroupHeight(tools, 80)).toBe(1);
});

it('uses one row for a completed batch', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,24 @@ function safeDescription(raw: string | undefined): string | undefined {
return cleaned || undefined;
}

/**
* Whether all tools in the given category have usable descriptions and the
* count is within the inline limit — meaning the summary already shows each
* description individually, so a separate hint line would be redundant.
*/
function categoryShowsDescriptionsInline(
toolCalls: IndividualToolCallDisplay[],
category: ToolCategory,
): boolean {
const sameCategory = toolCalls.filter(
(tc) => getToolCategory(tc.name) === category,
);
if (sameCategory.length > DESCRIPTION_INLINE_LIMIT) return false;
return sameCategory.every(
(tc) => safeDescription(tc.description) !== undefined,
);
}

function getActiveToolHint(
toolCalls: IndividualToolCallDisplay[],
): string | undefined {
Expand All @@ -305,25 +323,44 @@ function getActiveToolHint(
const tool = toolCalls[index];
if (tool.status === status) {
const category = getToolCategory(tool.name);
const usesCountSummary = toolCalls.some(
const hasCategoryPeers = toolCalls.some(
(candidate, candidateIndex) =>
candidateIndex !== index &&
getToolCategory(candidate.name) === category,
);
return usesCountSummary ? safeDescription(tool.description) : undefined;
if (!hasCategoryPeers) return undefined;
// Summary already shows descriptions inline → no hint needed.
if (categoryShowsDescriptionsInline(toolCalls, category))
return undefined;
return safeDescription(tool.description);
}
}
}

return undefined;
}

/**
* Maximum number of tools within one category whose individual descriptions
* are shown inline. Beyond this, the first `DESCRIPTION_PREVIEW_COUNT` are
* shown followed by "...and N more".
*/
const DESCRIPTION_INLINE_LIMIT = 3;

/**
* Number of descriptions shown as a preview when the category exceeds
* `DESCRIPTION_INLINE_LIMIT`.
*/
const DESCRIPTION_PREVIEW_COUNT = 2;

/**
* Build a semantic summary line from a batch of tool calls.
*
* Single tool (with description) → "Read a.ts" / "Ran ls -la"
* Single tool (no description) → "Read 1 file" / "Ran 1 command"
* Multi same → "Read 3 files"
* Multi ≤ 3 (with descriptions) → "Read a.ts, b.ts, c.ts"
* Multi ≤ 3 (no descriptions) → "Read 3 files"
* Multi > 3 → "Read a.ts, b.ts, ... and 3 more"
* Multi mixed → "Read 2 files, ran npm test"
*
* Uses past tense when all tools are done, present progressive when active.
Expand Down Expand Up @@ -352,25 +389,50 @@ export function buildToolSummary(
if (!tools || tools.length === 0) continue;

const template = CATEGORY_TEMPLATES[cat];
const verb = isActive ? template.activeVerb : template.pastVerb;
let part: string;
if (tools.length === 1) {
const safeDesc = safeDescription(tools[0].description);
if (safeDesc !== undefined) {
// Single tool with a concrete description: show it ("Read a.ts").
// Verb is English (see CategoryTemplate note) but the description is
// language-neutral, so the line reads correctly in every locale.
const verb = isActive ? template.activeVerb : template.pastVerb;
part = `${verb} ${safeDesc}`;
} else {
// No usable description → localized count phrase ("Read 1 file").
part = t(isActive ? template.active.one : template.past.one, {
count: '1',
});
}
} else if (tools.length <= DESCRIPTION_INLINE_LIMIT) {
// ≤ 3 tools: show all descriptions if available.
const descriptions = tools
.map((tc) => safeDescription(tc.description))
.filter((d): d is string => d !== undefined);
if (descriptions.length === tools.length) {
part = `${verb} ${descriptions.join(', ')}`;
} else {
// Not all tools have usable descriptions → count phrase.
const forms = isActive ? template.active : template.past;
part = t(forms.many, { count: String(tools.length) });
}
} else {
// Multiple tools of one category → localized plural count phrase.
const forms = isActive ? template.active : template.past;
part = t(forms.many, { count: String(tools.length) });
// > 3 tools: show first N descriptions + "...and M more".
const previewDescs = tools
.slice(0, DESCRIPTION_PREVIEW_COUNT)
.map((tc) => safeDescription(tc.description))
.filter((d): d is string => d !== undefined);
if (previewDescs.length === DESCRIPTION_PREVIEW_COUNT) {
const remaining = tools.length - DESCRIPTION_PREVIEW_COUNT;
const morePhrase = t('... and {{count}} more', {
count: String(remaining),
});
part = `${verb} ${previewDescs.join(', ')}, ${morePhrase}`;
} else {
// Not enough preview descriptions → count phrase.
const forms = isActive ? template.active : template.past;
part = t(forms.many, { count: String(tools.length) });
}
}
// Lowercase the leading character for every part after the first ("Read 3
// files, edited 2 files"). Operating on the first char only keeps already-
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ describe('<ToolGroupMessage />', () => {
const frame = lastFrame() ?? '';
// CATEGORY_ORDER: search first (capitalized), then read (lowercased)
expect(frame).toContain('Searched pattern');
expect(frame).toContain('read 2 files');
expect(frame).toContain('read a.ts, b.ts');
expect(frame).not.toContain('MockTool');
});

Expand Down Expand Up @@ -338,7 +338,7 @@ describe('<ToolGroupMessage />', () => {
/>,
);
const frame = lastFrame() ?? '';
expect(frame).toContain('Read 2 files');
expect(frame).toContain('Read a.ts, b.ts');
expect(frame).toContain('Recalled 1 memory');
});

Expand Down Expand Up @@ -1072,7 +1072,8 @@ describe('<ToolGroupMessage />', () => {
const call = vi
.mocked(ToolMessage)
.mock.calls.find((c) => c[0].callId === 'shell-result');
expect(call?.[0].availableTerminalHeight).toBe(9);
// 2 reads inline (no hint row) → summary is 1 row shorter than before.
expect(call?.[0].availableTerminalHeight).toBe(10);
});
});

Expand Down
Loading