mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
fix(cli): show file path in compact tool summary for single collapsible tools (#6448)
* fix(cli): show file path in compact tool summary for single collapsible tools
buildToolSummary() previously discarded the description field from
collapsible tools (ReadFile, Grep, Glob, ListFiles), showing only
generic counts like 'Read 1 file'. Now shows the actual file path or
search pattern for single tools, while preserving count format for
batches of multiple tools of the same type. Falls back to count format
when description is unavailable.
Signed-off-by: Alex <alex.tech.lab@outlook.com>
* fix(cli): sanitize description in buildToolSummary for error args and ANSI
When a tool call errors, useReactToolScheduler sets description to
JSON.stringify(args) which produces '{...}' blobs. Strip ANSI escape
sequences and reject JSON-looking descriptions so the summary falls
back to the count format instead of rendering raw JSON.
Signed-off-by: Alex <alex.tech.lab@outlook.com>
* fix(cli): broaden ANSI stripping and replace newlines with spaces in summary
Strip all common ANSI escape sequences (OSC, charset, CSI, single-byte
ESC) instead of just CSI. Replace all C0 control characters including
newlines with spaces so embedded \n in shell descriptions does not
break the single-line compact summary layout.
Signed-off-by: Alex <alex.tech.lab@outlook.com>
* test(cli): update ToolGroupMessage expectations for description-based summary
The buildToolSummary change from 'Read 1 file' to 'Read a.ts' broke
4 assertions in ToolGroupMessage.test.tsx. Update all 5 occurrences
to use the new description-based format.
Signed-off-by: Alex <alex.tech.lab@outlook.com>
---------
Signed-off-by: Alex <alex.tech.lab@outlook.com>
This commit is contained in:
parent
e83d548cd9
commit
49aa4c8ab5
3 changed files with 125 additions and 26 deletions
|
|
@ -123,7 +123,7 @@ describe('<CompactToolGroupDisplay /> — summary label', () => {
|
|||
);
|
||||
const frame = lastFrame()!;
|
||||
// CATEGORY_ORDER: search → read → list → ...
|
||||
expect(frame).toContain('Searched 1 pattern');
|
||||
expect(frame).toContain('Searched search pattern');
|
||||
expect(frame).toContain('read 2 files');
|
||||
});
|
||||
|
||||
|
|
@ -145,7 +145,7 @@ describe('<CompactToolGroupDisplay /> — summary label', () => {
|
|||
const { lastFrame } = render(
|
||||
<CompactToolGroupDisplay toolCalls={tools} contentWidth={80} />,
|
||||
);
|
||||
expect(lastFrame()).toContain('Ran 1 command');
|
||||
expect(lastFrame()).toContain('Ran ls -la');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -166,12 +166,12 @@ describe('buildToolSummary', () => {
|
|||
expect(buildToolSummary([], false)).toBe('');
|
||||
});
|
||||
|
||||
it('single tool uses count format', () => {
|
||||
expect(buildToolSummary([make({})], false)).toBe('Read 1 file');
|
||||
it('single tool uses description format', () => {
|
||||
expect(buildToolSummary([make({})], false)).toBe('Read a.ts');
|
||||
});
|
||||
|
||||
it('single tool uses progressive verb when active', () => {
|
||||
expect(buildToolSummary([make({})], true)).toBe('Reading 1 file');
|
||||
expect(buildToolSummary([make({})], true)).toBe('Reading a.ts');
|
||||
});
|
||||
|
||||
it('multiple same-type tools use count', () => {
|
||||
|
|
@ -191,7 +191,7 @@ describe('buildToolSummary', () => {
|
|||
];
|
||||
// CATEGORY_ORDER: search → read → list → command → edit
|
||||
expect(buildToolSummary(tools, false)).toBe(
|
||||
'Read 1 file, ran 1 command, edited 1 file',
|
||||
'Read a.ts, ran npm test, edited b.ts',
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -201,14 +201,77 @@ describe('buildToolSummary', () => {
|
|||
make({ callId: 'c2', name: 'Shell', description: 'ls' }),
|
||||
];
|
||||
const result = buildToolSummary(tools, false);
|
||||
expect(result).toBe('Read 1 file, ran 1 command');
|
||||
expect(result).toBe('Read a.ts, ran ls');
|
||||
});
|
||||
|
||||
it('unknown tool names fall to other category', () => {
|
||||
const tools = [
|
||||
make({ callId: 'c1', name: 'UnknownTool', description: 'something' }),
|
||||
];
|
||||
expect(buildToolSummary(tools, false)).toBe('Used 1 tool');
|
||||
expect(buildToolSummary(tools, false)).toBe('Used something');
|
||||
});
|
||||
|
||||
it('falls back to count format when description is empty', () => {
|
||||
const tools = [make({ callId: 'c1', name: 'ReadFile', description: '' })];
|
||||
expect(buildToolSummary(tools, false)).toBe('Read 1 file');
|
||||
});
|
||||
|
||||
it('falls back to count format when description is undefined', () => {
|
||||
const tools = [
|
||||
make({
|
||||
callId: 'c1',
|
||||
name: 'ReadFile',
|
||||
description: undefined as unknown as string,
|
||||
}),
|
||||
];
|
||||
expect(buildToolSummary(tools, false)).toBe('Read 1 file');
|
||||
});
|
||||
|
||||
it('falls back to count format when description is JSON (error args)', () => {
|
||||
const tools = [
|
||||
make({
|
||||
callId: 'c1',
|
||||
name: 'ReadFile',
|
||||
description: '{"file_path":"/tmp/test.txt"}',
|
||||
}),
|
||||
];
|
||||
expect(buildToolSummary(tools, false)).toBe('Read 1 file');
|
||||
});
|
||||
|
||||
it('falls back to count format when description starts with array bracket', () => {
|
||||
const tools = [
|
||||
make({ callId: 'c1', name: 'Shell', description: '["ls", "-la"]' }),
|
||||
];
|
||||
expect(buildToolSummary(tools, false)).toBe('Ran 1 command');
|
||||
});
|
||||
|
||||
it('strips ANSI CSI escape sequences from description', () => {
|
||||
const tools = [
|
||||
make({
|
||||
callId: 'c1',
|
||||
name: 'ReadFile',
|
||||
description: '\x1b[32ma.ts\x1b[0m',
|
||||
}),
|
||||
];
|
||||
expect(buildToolSummary(tools, false)).toBe('Read a.ts');
|
||||
});
|
||||
|
||||
it('strips non-CSI ANSI sequences (charset, OSC) from description', () => {
|
||||
const tools = [
|
||||
make({ callId: 'c1', name: 'Shell', description: '\x1b(Bls -la\x1b[0m' }),
|
||||
];
|
||||
expect(buildToolSummary(tools, false)).toBe('Ran ls -la');
|
||||
});
|
||||
|
||||
it('replaces embedded newlines with spaces in description', () => {
|
||||
const tools = [
|
||||
make({
|
||||
callId: 'c1',
|
||||
name: 'Shell',
|
||||
description: 'echo hello\nworld',
|
||||
}),
|
||||
];
|
||||
expect(buildToolSummary(tools, false)).toBe('Ran echo hello world');
|
||||
});
|
||||
|
||||
it('mixed group with count per category', () => {
|
||||
|
|
@ -217,7 +280,7 @@ describe('buildToolSummary', () => {
|
|||
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 1 command');
|
||||
expect(buildToolSummary(tools, false)).toBe('Read 2 files, ran npm test');
|
||||
});
|
||||
|
||||
it('legacy display names map to correct categories', () => {
|
||||
|
|
@ -226,7 +289,7 @@ describe('buildToolSummary', () => {
|
|||
make({ callId: 'c2', name: 'ReadFolder', description: '/src' }),
|
||||
];
|
||||
expect(buildToolSummary(tools, false)).toBe(
|
||||
'Searched 1 pattern, listed 1 directory',
|
||||
'Searched pattern, listed /src',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -189,14 +189,43 @@ export function isCollapsibleTool(toolName: string): boolean {
|
|||
return COLLAPSIBLE_CATEGORIES.has(getToolCategory(toolName));
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip ANSI control sequences and reject JSON-looking error fallbacks.
|
||||
*
|
||||
* When a tool call errors, `useReactToolScheduler` sets description to
|
||||
* `JSON.stringify(request.args)` which produces `{...}` blobs. Return
|
||||
* `undefined` for those so the caller falls back to count format.
|
||||
*/
|
||||
function safeDescription(raw: string | undefined): string | undefined {
|
||||
if (!raw) return undefined;
|
||||
|
||||
/* eslint-disable no-control-regex */
|
||||
// Strip all common ANSI escape sequences: OSC, charset, CSI, and single-byte ESC
|
||||
const stripped = raw.replace(
|
||||
/\x1b\][^\x07]*\x07|\x1b[()][A-Z0-9]|\x1b\[[0-9;]*[a-zA-Z]|\x1b./g,
|
||||
'',
|
||||
);
|
||||
// Replace all C0 control characters (including \n, \r) with spaces
|
||||
const cleaned = stripped.replace(/[\x00-\x1f\x7f]/g, ' ').trim();
|
||||
/* eslint-enable no-control-regex */
|
||||
|
||||
// Reject JSON-looking blobs (error fallback from args)
|
||||
if (cleaned.startsWith('{') || cleaned.startsWith('[')) return undefined;
|
||||
|
||||
return cleaned || undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a semantic summary line from a batch of tool calls.
|
||||
*
|
||||
* Single tool → "Read 1 file" / "Ran 1 command"
|
||||
* Multi same → "Read 3 files"
|
||||
* Multi mixed → "Read 3 files, edited 2 files, ran 1 command"
|
||||
* 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 mixed → "Read a.ts, ran npm test, edited b.ts"
|
||||
*
|
||||
* Uses past tense when all tools are done, present progressive when active.
|
||||
* Falls back to count format when description is empty, contains control
|
||||
* characters, or looks like a JSON blob (e.g. error fallback from args).
|
||||
*/
|
||||
export function buildToolSummary(
|
||||
toolCalls: IndividualToolCallDisplay[],
|
||||
|
|
@ -204,28 +233,35 @@ export function buildToolSummary(
|
|||
): string {
|
||||
if (toolCalls.length === 0) return '';
|
||||
|
||||
// Group by category and count
|
||||
const counts = new Map<ToolCategory, number>();
|
||||
// Group by category to preserve tool references for description access
|
||||
const toolsByCategory = new Map<ToolCategory, IndividualToolCallDisplay[]>();
|
||||
|
||||
for (const tool of toolCalls) {
|
||||
const cat = getToolCategory(tool.name);
|
||||
counts.set(cat, (counts.get(cat) ?? 0) + 1);
|
||||
const arr = toolsByCategory.get(cat) ?? [];
|
||||
arr.push(tool);
|
||||
toolsByCategory.set(cat, arr);
|
||||
}
|
||||
|
||||
const parts: string[] = [];
|
||||
for (const cat of CATEGORY_ORDER) {
|
||||
const count = counts.get(cat);
|
||||
if (!count) continue;
|
||||
const tools = toolsByCategory.get(cat);
|
||||
if (!tools || tools.length === 0) continue;
|
||||
|
||||
const template = CATEGORY_TEMPLATES[cat];
|
||||
const verb = isActive ? template.activeVerb : template.pastVerb;
|
||||
const lower = parts.length > 0;
|
||||
const v = lower ? verb.toLowerCase() : verb;
|
||||
|
||||
if (count === 1) {
|
||||
parts.push(`${v} 1 ${template.singular}`);
|
||||
if (tools.length === 1) {
|
||||
const safeDesc = safeDescription(tools[0].description);
|
||||
if (safeDesc !== undefined) {
|
||||
parts.push(`${v} ${safeDesc}`);
|
||||
} else {
|
||||
parts.push(`${v} 1 ${template.singular}`);
|
||||
}
|
||||
} else {
|
||||
parts.push(`${v} ${count} ${template.plural}`);
|
||||
parts.push(`${v} ${tools.length} ${template.plural}`);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -185,7 +185,7 @@ describe('<ToolGroupMessage />', () => {
|
|||
);
|
||||
const frame = lastFrame() ?? '';
|
||||
// CATEGORY_ORDER: search first (capitalized), then read (lowercased)
|
||||
expect(frame).toContain('Searched 1 pattern');
|
||||
expect(frame).toContain('Searched pattern');
|
||||
expect(frame).toContain('read 2 files');
|
||||
expect(frame).not.toContain('MockTool');
|
||||
});
|
||||
|
|
@ -204,7 +204,7 @@ describe('<ToolGroupMessage />', () => {
|
|||
);
|
||||
const frame = lastFrame() ?? '';
|
||||
// Collapsible → summary line
|
||||
expect(frame).toContain('Read 1 file');
|
||||
expect(frame).toContain('Read a.ts');
|
||||
// Non-collapsible → individual ToolMessage
|
||||
expect(frame).toContain('MockTool[s1]');
|
||||
});
|
||||
|
|
@ -226,7 +226,7 @@ describe('<ToolGroupMessage />', () => {
|
|||
// All tools render individually — no summary line
|
||||
expect(frame).toContain('MockTool[r1]');
|
||||
expect(frame).toContain('MockTool[e1]');
|
||||
expect(frame).not.toContain('Read 1 file');
|
||||
expect(frame).not.toContain('Read a.ts');
|
||||
});
|
||||
|
||||
it('forceExpandAll passes forceShowResult to Success siblings in error group', () => {
|
||||
|
|
@ -276,7 +276,7 @@ describe('<ToolGroupMessage />', () => {
|
|||
);
|
||||
const frame = lastFrame() ?? '';
|
||||
// Successful ReadFile → summary line
|
||||
expect(frame).toContain('Read 1 file');
|
||||
expect(frame).toContain('Read a.ts');
|
||||
// Canceled ReadFile → individual ToolMessage (partial output visible)
|
||||
expect(frame).toContain('MockTool[r2]');
|
||||
});
|
||||
|
|
@ -306,7 +306,7 @@ describe('<ToolGroupMessage />', () => {
|
|||
const frame = lastFrame() ?? '';
|
||||
expect(frame).toContain('Recalled 2 memories');
|
||||
// Collapsible tool still summarized
|
||||
expect(frame).toContain('Read 1 file');
|
||||
expect(frame).toContain('Read config.yaml');
|
||||
// Non-collapsible tool rendered individually
|
||||
expect(frame).toContain('MockTool[s1]');
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue