mirror of
https://github.com/QwenLM/qwen-code.git
synced 2025-12-19 09:33:53 +00:00
feat(errors): Make errors more informative (#7133)
This commit is contained in:
10
package-lock.json
generated
10
package-lock.json
generated
@@ -3195,6 +3195,13 @@
|
||||
"@types/send": "*"
|
||||
}
|
||||
},
|
||||
"node_modules/@types/fast-levenshtein": {
|
||||
"version": "0.0.4",
|
||||
"resolved": "https://registry.npmjs.org/@types/fast-levenshtein/-/fast-levenshtein-0.0.4.tgz",
|
||||
"integrity": "sha512-tkDveuitddQCxut1Db8eEFfMahTjOumTJGPHmT9E7KUH+DkVq9WTpVvlfenf3S+uCBeu8j5FP2xik/KfxOEjeA==",
|
||||
"dev": true,
|
||||
"license": "MIT"
|
||||
},
|
||||
"node_modules/@types/fs-extra": {
|
||||
"version": "11.0.4",
|
||||
"resolved": "https://registry.npmjs.org/@types/fs-extra/-/fs-extra-11.0.4.tgz",
|
||||
@@ -6885,7 +6892,6 @@
|
||||
"version": "2.0.6",
|
||||
"resolved": "https://registry.npmjs.org/fast-levenshtein/-/fast-levenshtein-2.0.6.tgz",
|
||||
"integrity": "sha512-DCXu6Ifhqcks7TZKY3Hxp3y6qphY5SJZmrWMDrKcERSOXWQdMhU9Ig/PYrzyw/ul9jOIyh0N4M0tbC5hodg8dw==",
|
||||
"dev": true,
|
||||
"license": "MIT"
|
||||
},
|
||||
"node_modules/fast-safe-stringify": {
|
||||
@@ -14679,6 +14685,7 @@
|
||||
"chardet": "^2.1.0",
|
||||
"diff": "^7.0.0",
|
||||
"dotenv": "^17.1.0",
|
||||
"fast-levenshtein": "^2.0.6",
|
||||
"fast-uri": "^3.0.6",
|
||||
"fdir": "^6.4.6",
|
||||
"fzf": "^0.5.2",
|
||||
@@ -14701,6 +14708,7 @@
|
||||
"@google/gemini-cli-test-utils": "file:../test-utils",
|
||||
"@types/diff": "^7.0.2",
|
||||
"@types/dotenv": "^6.1.1",
|
||||
"@types/fast-levenshtein": "^0.0.4",
|
||||
"@types/minimatch": "^5.1.2",
|
||||
"@types/picomatch": "^4.0.1",
|
||||
"@types/ws": "^8.5.10",
|
||||
|
||||
@@ -48,6 +48,7 @@ vi.mock('@google/gemini-cli-core', async () => {
|
||||
|
||||
const mockToolRegistry = {
|
||||
getTool: vi.fn(),
|
||||
getAllToolNames: vi.fn(() => ['mockTool', 'anotherTool']),
|
||||
};
|
||||
|
||||
const mockConfig = {
|
||||
@@ -427,11 +428,17 @@ describe('useReactToolScheduler', () => {
|
||||
request,
|
||||
response: expect.objectContaining({
|
||||
error: expect.objectContaining({
|
||||
message: 'Tool "nonexistentTool" not found in registry.',
|
||||
message: expect.stringMatching(
|
||||
/Tool "nonexistentTool" not found in registry/,
|
||||
),
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
]);
|
||||
const errorMessage = onComplete.mock.calls[0][0][0].response.error.message;
|
||||
expect(errorMessage).toContain('Did you mean one of:');
|
||||
expect(errorMessage).toContain('"mockTool"');
|
||||
expect(errorMessage).toContain('"anotherTool"');
|
||||
expect(result.current[0]).toEqual([]);
|
||||
});
|
||||
|
||||
|
||||
@@ -33,12 +33,14 @@
|
||||
"@opentelemetry/sdk-node": "^0.203.0",
|
||||
"@types/glob": "^8.1.0",
|
||||
"@types/html-to-text": "^9.0.4",
|
||||
"@xterm/headless": "5.5.0",
|
||||
"ajv": "^8.17.1",
|
||||
"fast-uri": "^3.0.6",
|
||||
"ajv-formats": "^3.0.0",
|
||||
"chardet": "^2.1.0",
|
||||
"diff": "^7.0.0",
|
||||
"dotenv": "^17.1.0",
|
||||
"fast-levenshtein": "^2.0.6",
|
||||
"fast-uri": "^3.0.6",
|
||||
"fdir": "^6.4.6",
|
||||
"fzf": "^0.5.2",
|
||||
"glob": "^10.4.5",
|
||||
@@ -54,22 +56,22 @@
|
||||
"simple-git": "^3.28.0",
|
||||
"strip-ansi": "^7.1.0",
|
||||
"undici": "^7.10.0",
|
||||
"ws": "^8.18.0",
|
||||
"@xterm/headless": "5.5.0"
|
||||
"ws": "^8.18.0"
|
||||
},
|
||||
"optionalDependencies": {
|
||||
"@lydell/node-pty": "1.1.0",
|
||||
"node-pty": "^1.0.0",
|
||||
"@lydell/node-pty-darwin-arm64": "1.1.0",
|
||||
"@lydell/node-pty-darwin-x64": "1.1.0",
|
||||
"@lydell/node-pty-linux-x64": "1.1.0",
|
||||
"@lydell/node-pty-win32-arm64": "1.1.0",
|
||||
"@lydell/node-pty-win32-x64": "1.1.0"
|
||||
"@lydell/node-pty-win32-x64": "1.1.0",
|
||||
"node-pty": "^1.0.0"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@google/gemini-cli-test-utils": "file:../test-utils",
|
||||
"@types/diff": "^7.0.2",
|
||||
"@types/dotenv": "^6.1.1",
|
||||
"@types/fast-levenshtein": "^0.0.4",
|
||||
"@types/minimatch": "^5.1.2",
|
||||
"@types/picomatch": "^4.0.1",
|
||||
"@types/ws": "^8.5.10",
|
||||
|
||||
@@ -195,6 +195,42 @@ describe('CoreToolScheduler', () => {
|
||||
.calls[0][0] as ToolCall[];
|
||||
expect(completedCalls[0].status).toBe('cancelled');
|
||||
});
|
||||
|
||||
describe('getToolSuggestion', () => {
|
||||
it('should suggest the top N closest tool names for a typo', () => {
|
||||
// Create mocked tool registry
|
||||
const mockConfig = {
|
||||
getToolRegistry: () => mockToolRegistry,
|
||||
} as unknown as Config;
|
||||
const mockToolRegistry = {
|
||||
getAllToolNames: () => ['list_files', 'read_file', 'write_file'],
|
||||
} as unknown as ToolRegistry;
|
||||
|
||||
// Create scheduler
|
||||
const scheduler = new CoreToolScheduler({
|
||||
config: mockConfig,
|
||||
getPreferredEditor: () => 'vscode',
|
||||
onEditorClose: vi.fn(),
|
||||
});
|
||||
|
||||
// Test that the right tool is selected, with only 1 result, for typos
|
||||
// @ts-expect-error accessing private method
|
||||
const misspelledTool = scheduler.getToolSuggestion('list_fils', 1);
|
||||
expect(misspelledTool).toBe(' Did you mean "list_files"?');
|
||||
|
||||
// Test that the right tool is selected, with only 1 result, for prefixes
|
||||
// @ts-expect-error accessing private method
|
||||
const prefixedTool = scheduler.getToolSuggestion('github.list_files', 1);
|
||||
expect(prefixedTool).toBe(' Did you mean "list_files"?');
|
||||
|
||||
// Test that the right tool is first
|
||||
// @ts-expect-error accessing private method
|
||||
const suggestionMultiple = scheduler.getToolSuggestion('list_fils');
|
||||
expect(suggestionMultiple).toBe(
|
||||
' Did you mean one of: "list_files", "read_file", "write_file"?',
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('CoreToolScheduler with payload', () => {
|
||||
|
||||
@@ -33,6 +33,7 @@ import {
|
||||
} from '../tools/modifiable-tool.js';
|
||||
import * as Diff from 'diff';
|
||||
import { doesToolInvocationMatch } from '../utils/tool-utils.js';
|
||||
import levenshtein from 'fast-levenshtein';
|
||||
|
||||
export type ValidatingToolCall = {
|
||||
status: 'validating';
|
||||
@@ -504,6 +505,40 @@ export class CoreToolScheduler {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Generates a suggestion string for a tool name that was not found in the registry.
|
||||
* It finds the closest matches based on Levenshtein distance.
|
||||
* @param unknownToolName The tool name that was not found.
|
||||
* @param topN The number of suggestions to return. Defaults to 3.
|
||||
* @returns A suggestion string like " Did you mean 'tool'?" or " Did you mean one of: 'tool1', 'tool2'?", or an empty string if no suggestions are found.
|
||||
*/
|
||||
private getToolSuggestion(unknownToolName: string, topN = 3): string {
|
||||
const allToolNames = this.toolRegistry.getAllToolNames();
|
||||
|
||||
const matches = allToolNames.map((toolName) => ({
|
||||
name: toolName,
|
||||
distance: levenshtein.get(unknownToolName, toolName),
|
||||
}));
|
||||
|
||||
matches.sort((a, b) => a.distance - b.distance);
|
||||
|
||||
const topNResults = matches.slice(0, topN);
|
||||
|
||||
if (topNResults.length === 0) {
|
||||
return '';
|
||||
}
|
||||
|
||||
const suggestedNames = topNResults
|
||||
.map((match) => `"${match.name}"`)
|
||||
.join(', ');
|
||||
|
||||
if (topNResults.length > 1) {
|
||||
return ` Did you mean one of: ${suggestedNames}?`;
|
||||
} else {
|
||||
return ` Did you mean ${suggestedNames}?`;
|
||||
}
|
||||
}
|
||||
|
||||
schedule(
|
||||
request: ToolCallRequestInfo | ToolCallRequestInfo[],
|
||||
signal: AbortSignal,
|
||||
@@ -557,12 +592,14 @@ export class CoreToolScheduler {
|
||||
(reqInfo): ToolCall => {
|
||||
const toolInstance = this.toolRegistry.getTool(reqInfo.name);
|
||||
if (!toolInstance) {
|
||||
const suggestion = this.getToolSuggestion(reqInfo.name);
|
||||
const errorMessage = `Tool "${reqInfo.name}" not found in registry. Tools must use the exact names that are registered.${suggestion}`;
|
||||
return {
|
||||
status: 'error',
|
||||
request: reqInfo,
|
||||
response: createErrorResponse(
|
||||
reqInfo,
|
||||
new Error(`Tool "${reqInfo.name}" not found in registry.`),
|
||||
new Error(errorMessage),
|
||||
ToolErrorType.TOOL_NOT_REGISTERED,
|
||||
),
|
||||
durationMs: 0,
|
||||
|
||||
@@ -27,6 +27,7 @@ describe('executeToolCall', () => {
|
||||
|
||||
mockToolRegistry = {
|
||||
getTool: vi.fn(),
|
||||
getAllToolNames: vi.fn(),
|
||||
} as unknown as ToolRegistry;
|
||||
|
||||
mockConfig = {
|
||||
@@ -94,6 +95,10 @@ describe('executeToolCall', () => {
|
||||
prompt_id: 'prompt-id-2',
|
||||
};
|
||||
vi.mocked(mockToolRegistry.getTool).mockReturnValue(undefined);
|
||||
vi.mocked(mockToolRegistry.getAllToolNames).mockReturnValue([
|
||||
'testTool',
|
||||
'anotherTool',
|
||||
]);
|
||||
|
||||
const response = await executeToolCall(
|
||||
mockConfig,
|
||||
@@ -101,18 +106,20 @@ describe('executeToolCall', () => {
|
||||
abortController.signal,
|
||||
);
|
||||
|
||||
const expectedErrorMessage =
|
||||
'Tool "nonexistentTool" not found in registry. Tools must use the exact names that are registered. Did you mean one of: "testTool", "anotherTool"?';
|
||||
expect(response).toStrictEqual({
|
||||
callId: 'call2',
|
||||
error: new Error('Tool "nonexistentTool" not found in registry.'),
|
||||
error: new Error(expectedErrorMessage),
|
||||
errorType: ToolErrorType.TOOL_NOT_REGISTERED,
|
||||
resultDisplay: 'Tool "nonexistentTool" not found in registry.',
|
||||
resultDisplay: expectedErrorMessage,
|
||||
responseParts: [
|
||||
{
|
||||
functionResponse: {
|
||||
name: 'nonexistentTool',
|
||||
id: 'call2',
|
||||
response: {
|
||||
error: 'Tool "nonexistentTool" not found in registry.',
|
||||
error: expectedErrorMessage,
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -174,6 +174,24 @@ describe('ToolRegistry', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('getAllToolNames', () => {
|
||||
it('should return all registered tool names', () => {
|
||||
// Register tools with displayNames in non-alphabetical order
|
||||
const toolC = new MockTool('c-tool', 'Tool C');
|
||||
const toolA = new MockTool('a-tool', 'Tool A');
|
||||
const toolB = new MockTool('b-tool', 'Tool B');
|
||||
|
||||
toolRegistry.registerTool(toolC);
|
||||
toolRegistry.registerTool(toolA);
|
||||
toolRegistry.registerTool(toolB);
|
||||
|
||||
const toolNames = toolRegistry.getAllToolNames();
|
||||
|
||||
// Assert that the returned array contains all tool names
|
||||
expect(toolNames).toEqual(['c-tool', 'a-tool', 'b-tool']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getToolsByServer', () => {
|
||||
it('should return an empty array if no tools match the server name', () => {
|
||||
toolRegistry.registerTool(new MockTool());
|
||||
|
||||
@@ -437,6 +437,13 @@ export class ToolRegistry {
|
||||
return declarations;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns an array of all registered and discovered tool names.
|
||||
*/
|
||||
getAllToolNames(): string[] {
|
||||
return Array.from(this.tools.keys());
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns an array of all registered and discovered tool instances.
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user