abbyswag/qwen-code
1
0
Fork 0
mirror of https://github.com/QwenLM/qwen-code.git synced 2025-12-20 08:47:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Push tool calls to absolute paths (#1055) (#1057)

Browse Source 
Make several changes to guide the model to request absolute paths,
reducing frequent accidental relative path tool call failures.

- Switch the parameter name: path --> absolute_path.
- Update the tool definition to strongly require an absolute path.
- Update the system prompt to indicate absolute paths are required.
- Update the system prompt tool use examples to use absolute paths.

Test case:

Open GC in GC: "Locate the primary file calling genai"
- Expected: Model opens files with absolute path, successfully.
- Actual (pre-patch): Failure, attempts to read with relative path.
- Actual (post-patch): Success, attempts to read with absolute path.
This commit is contained in:
Keir Mierle
2025-06-14 21:16:11 -07:00
committed by GitHub
parent e30e650a77
commit 32dd298351
5 changed files with 82 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
packages/core/src/tools/read-file.test.ts
View File

@@ -57,14 +57,14 @@ describe('ReadFileTool', () => {
describe('validateToolParams', () => {
it('should return null for valid params (absolute path within root)', () => {
const params: ReadFileToolParams = {
path: path.join(tempRootDir, 'test.txt'),
absolute_path: path.join(tempRootDir, 'test.txt'),
};
expect(tool.validateToolParams(params)).toBeNull();
});
it('should return null for valid params with offset and limit', () => {
const params: ReadFileToolParams = {
path: path.join(tempRootDir, 'test.txt'),
absolute_path: path.join(tempRootDir, 'test.txt'),
offset: 0,
limit: 10,
};
@@ -72,7 +72,7 @@ describe('ReadFileTool', () => {
});
it('should return error for relative path', () => {
const params: ReadFileToolParams = { path: 'test.txt' };
const params: ReadFileToolParams = { absolute_path: 'test.txt' };
expect(tool.validateToolParams(params)).toMatch(
/File path must be absolute/,
);
@@ -80,7 +80,7 @@ describe('ReadFileTool', () => {
it('should return error for path outside root', () => {
const outsidePath = path.resolve(os.tmpdir(), 'outside-root.txt');
const params: ReadFileToolParams = { path: outsidePath };
const params: ReadFileToolParams = { absolute_path: outsidePath };
expect(tool.validateToolParams(params)).toMatch(
/File path must be within the root directory/,
);
@@ -88,7 +88,7 @@ describe('ReadFileTool', () => {
it('should return error for negative offset', () => {
const params: ReadFileToolParams = {
path: path.join(tempRootDir, 'test.txt'),
absolute_path: path.join(tempRootDir, 'test.txt'),
offset: -1,
limit: 10,
};
@@ -99,7 +99,7 @@ describe('ReadFileTool', () => {
it('should return error for non-positive limit', () => {
const paramsZero: ReadFileToolParams = {
path: path.join(tempRootDir, 'test.txt'),
absolute_path: path.join(tempRootDir, 'test.txt'),
offset: 0,
limit: 0,
};
@@ -107,7 +107,7 @@ describe('ReadFileTool', () => {
'Limit must be a positive number',
);
const paramsNegative: ReadFileToolParams = {
path: path.join(tempRootDir, 'test.txt'),
absolute_path: path.join(tempRootDir, 'test.txt'),
offset: 0,
limit: -5,
};
@@ -127,21 +127,21 @@ describe('ReadFileTool', () => {
describe('getDescription', () => {
it('should return a shortened, relative path', () => {
const filePath = path.join(tempRootDir, 'sub', 'dir', 'file.txt');
const params: ReadFileToolParams = { path: filePath };
const params: ReadFileToolParams = { absolute_path: filePath };
// Assuming tempRootDir is something like /tmp/read-file-tool-root-XXXXXX
// The relative path would be sub/dir/file.txt
expect(tool.getDescription(params)).toBe('sub/dir/file.txt');
});
it('should return . if path is the root directory', () => {
const params: ReadFileToolParams = { path: tempRootDir };
const params: ReadFileToolParams = { absolute_path: tempRootDir };
expect(tool.getDescription(params)).toBe('.');
});
});
describe('execute', () => {
it('should return validation error if params are invalid', async () => {
const params: ReadFileToolParams = { path: 'relative/path.txt' };
const params: ReadFileToolParams = { absolute_path: 'relative/path.txt' };
const result = await tool.execute(params, abortSignal);
expect(result.llmContent).toMatch(/Error: Invalid parameters provided/);
expect(result.returnDisplay).toMatch(/File path must be absolute/);
@@ -149,7 +149,7 @@ describe('ReadFileTool', () => {
it('should return error from processSingleFileContent if it fails', async () => {
const filePath = path.join(tempRootDir, 'error.txt');
const params: ReadFileToolParams = { path: filePath };
const params: ReadFileToolParams = { absolute_path: filePath };
const errorMessage = 'Simulated read error';
mockProcessSingleFileContent.mockResolvedValue({
llmContent: `Error reading file ${filePath}: ${errorMessage}`,
@@ -171,7 +171,7 @@ describe('ReadFileTool', () => {
it('should return success result for a text file', async () => {
const filePath = path.join(tempRootDir, 'textfile.txt');
const fileContent = 'This is a test file.';
const params: ReadFileToolParams = { path: filePath };
const params: ReadFileToolParams = { absolute_path: filePath };
mockProcessSingleFileContent.mockResolvedValue({
llmContent: fileContent,
returnDisplay: `Read text file: ${path.basename(filePath)}`,
@@ -195,7 +195,7 @@ describe('ReadFileTool', () => {
const imageData = {
inlineData: { mimeType: 'image/png', data: 'base64...' },
};
const params: ReadFileToolParams = { path: filePath };
const params: ReadFileToolParams = { absolute_path: filePath };
mockProcessSingleFileContent.mockResolvedValue({
llmContent: imageData,
returnDisplay: `Read image file: ${path.basename(filePath)}`,
@@ -217,7 +217,7 @@ describe('ReadFileTool', () => {
it('should pass offset and limit to processSingleFileContent', async () => {
const filePath = path.join(tempRootDir, 'paginated.txt');
const params: ReadFileToolParams = {
path: filePath,
absolute_path: filePath,
offset: 10,
limit: 5,
};
@@ -237,7 +237,7 @@ describe('ReadFileTool', () => {
it('should return error if path is ignored by a .geminiignore pattern', async () => {
const params: ReadFileToolParams = {
path: path.join(tempRootDir, 'foo.bar'),
absolute_path: path.join(tempRootDir, 'foo.bar'),
};
const result = await tool.execute(params, abortSignal);
expect(result.returnDisplay).toContain('foo.bar');