refactor: remove legacy offset/limit from read_file and use 1-based ranges exclusively

This commit is contained in:
Adam Weidman
2026-02-12 13:05:26 -05:00
parent 1ad33d2465
commit 017967b3ee
6 changed files with 28 additions and 143 deletions
@@ -1,5 +1,5 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
exports[`ReadFileTool > getSchema > should return the base schema when no modelId is provided 1`] = `"Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. Handles text, images, audio, and PDF files. For text files, it can read specific line ranges."`;
exports[`ReadFileTool > getSchema > should return the base schema when no modelId is provided 1`] = `"Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'start_line' and 'end_line' parameters. Handles text, images, audio, and PDF files. For text files, it can read specific line ranges."`;
exports[`ReadFileTool > getSchema > should return the schema from the resolver when modelId is provided 1`] = `"Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. Handles text, images, audio, and PDF files. For text files, it can read specific line ranges."`;
exports[`ReadFileTool > getSchema > should return the schema from the resolver when modelId is provided 1`] = `"Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'start_line' and 'end_line' parameters. Handles text, images, audio, and PDF files. For text files, it can read specific line ranges."`;
@@ -22,7 +22,7 @@ export const WRITE_FILE_TOOL_NAME = 'write_file';
export const READ_FILE_DEFINITION: ToolDefinition = {
base: {
name: READ_FILE_TOOL_NAME,
description: `Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. For Gemini 3 models, use 'start_line' and 'end_line' for precise extraction. **Important:** For high token efficiency, avoid reading large files in their entirety. Use 'grep_search' to find symbols or 'run_shell_command' with 'sed' for surgical block extraction instead of broad file reads. Handles text, images, audio, and PDF files.`,
description: `Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'start_line' and 'end_line' parameters. **Important:** For high token efficiency, avoid reading large files in their entirety. Use 'grep_search' to find symbols or 'run_shell_command' with 'sed' for surgical block extraction instead of broad file reads. Handles text, images, audio, and PDF files.`,
parametersJsonSchema: {
type: 'object',
properties: {
@@ -30,24 +30,14 @@ export const READ_FILE_DEFINITION: ToolDefinition = {
description: 'The path to the file to read.',
type: 'string',
},
offset: {
description:
"Optional: For text files, the 0-based line number to start reading from. Requires 'limit' to be set. Use for paginating through large files.",
type: 'number',
},
limit: {
description:
"Optional: For text files, maximum number of lines to read. Use with 'offset' to paginate through large files. If omitted, reads the entire file (if feasible, up to a default limit).",
type: 'number',
},
start_line: {
description:
'Optional: The 1-based line number to start reading from. (Gemini 3+)',
'Optional: The 1-based line number to start reading from.',
type: 'number',
},
end_line: {
description:
'Optional: The 1-based line number to end reading at (inclusive). (Gemini 3+)',
'Optional: The 1-based line number to end reading at (inclusive).',
type: 'number',
},
},
+8 -67
View File
@@ -79,7 +79,7 @@ describe('ReadFileTool', () => {
});
describe('schema', () => {
it('should include pagination parameters for Gemini 2.5', () => {
it('should include line range parameters for all models', () => {
vi.mocked(tool['config'].getActiveModel).mockReturnValue(
'gemini-2.5-pro',
);
@@ -89,26 +89,17 @@ describe('ReadFileTool', () => {
properties: Record<string, unknown>;
}
).properties;
expect(properties).toHaveProperty('offset');
expect(properties).toHaveProperty('limit');
expect(schema.description).toContain('offset');
expect(schema.description).toContain('limit');
});
it('should include line range parameters for Gemini 3', () => {
vi.mocked(tool['config'].getActiveModel).mockReturnValue(
'gemini-3-pro-preview',
);
const schema = tool.schema;
const properties = (
schema.parametersJsonSchema as {
properties: Record<string, unknown>;
}
).properties;
expect(properties).toHaveProperty('start_line');
expect(properties).toHaveProperty('end_line');
expect(properties).not.toHaveProperty('offset');
expect(properties).not.toHaveProperty('limit');
});
it('should include surgical guidance in description for Gemini 3', () => {
vi.mocked(tool['config'].getActiveModel).mockReturnValue(
'gemini-3-pro-preview',
);
const schema = tool.schema;
expect(schema.description).toContain('grep_search');
expect(schema.description).toContain('sed');
});
@@ -167,26 +158,6 @@ describe('ReadFileTool', () => {
);
});
it('should throw error if offset is negative', () => {
const params: ReadFileToolParams = {
file_path: path.join(tempRootDir, 'test.txt'),
offset: -1,
};
expect(() => tool.build(params)).toThrow(
'Offset must be a non-negative number',
);
});
it('should throw error if limit is zero or negative', () => {
const params: ReadFileToolParams = {
file_path: path.join(tempRootDir, 'test.txt'),
limit: 0,
};
expect(() => tool.build(params)).toThrow(
'Limit must be a positive number',
);
});
it('should throw error if start_line is less than 1', () => {
const params: ReadFileToolParams = {
file_path: path.join(tempRootDir, 'test.txt'),
@@ -457,34 +428,6 @@ describe('ReadFileTool', () => {
expect(result.returnDisplay).toBe('');
});
it('should support offset and limit for text files', async () => {
const filePath = path.join(tempRootDir, 'paginated.txt');
const lines = Array.from({ length: 20 }, (_, i) => `Line ${i + 1}`);
const fileContent = lines.join('\n');
await fsp.writeFile(filePath, fileContent, 'utf-8');
const params: ReadFileToolParams = {
file_path: filePath,
offset: 5, // Start from line 6
limit: 3,
};
const invocation = tool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain(
'IMPORTANT: The file content has been truncated',
);
expect(result.llmContent).toContain(
'Status: Showing lines 6-8 of 20 total lines',
);
expect(result.llmContent).toContain('Line 6');
expect(result.llmContent).toContain('Line 7');
expect(result.llmContent).toContain('Line 8');
expect(result.returnDisplay).toBe(
'Read lines 6-8 of 20 from paginated.txt',
);
});
it('should support start_line and end_line for text files', async () => {
const filePath = path.join(tempRootDir, 'lines.txt');
const lines = Array.from({ length: 20 }, (_, i) => `Line ${i + 1}`);
@@ -520,8 +463,6 @@ describe('ReadFileTool', () => {
const params: ReadFileToolParams = {
file_path: filePath,
offset: 10, // Should be ignored
limit: 5, // Should be ignored
};
const invocation = tool.build(params);
+9 -44
View File
@@ -38,16 +38,6 @@ export interface ReadFileToolParams {
*/
file_path: string;
/**
* The line number to start reading from (optional, 0-based)
*/
offset?: number;
/**
* The number of lines to read (optional)
*/
limit?: number;
/**
* The line number to start reading from (optional, 1-based)
*/
@@ -90,7 +80,7 @@ class ReadFileToolInvocation extends BaseToolInvocation<
return [
{
path: this.resolvedPath,
line: this.params.start_line ?? this.params.offset,
line: this.params.start_line,
},
];
}
@@ -120,8 +110,6 @@ class ReadFileToolInvocation extends BaseToolInvocation<
this.resolvedPath,
this.config.getTargetDir(),
this.config.getFileSystemService(),
isGemini3 ? undefined : this.params.offset,
isGemini3 ? undefined : this.params.limit,
this.params.start_line,
this.params.end_line,
);
@@ -154,13 +142,10 @@ Action:
--- FILE CONTENT (truncated) ---
${result.llmContent}`;
} else {
const nextOffset = this.params.offset
? this.params.offset + end - start + 1
: end;
llmContent = `
IMPORTANT: The file content has been truncated.
Status: Showing lines ${start}-${end} of ${total} total lines.
Action: To read more of the file, you can use the 'offset' and 'limit' parameters in a subsequent 'read_file' call. For example, to read the next section of the file, use offset: ${nextOffset}.
Action: To read more of the file, you can use the 'start_line' and 'end_line' parameters in a subsequent 'read_file' call.
--- FILE CONTENT (truncated) ---
${result.llmContent}`;
@@ -237,36 +222,22 @@ export class ReadFileTool extends BaseDeclarativeTool<
description: 'The path to the file to read.',
type: 'string',
},
};
if (isGemini3) {
properties['start_line'] = {
start_line: {
description: 'Optional: The 1-based line number to start reading from.',
type: 'number',
};
properties['end_line'] = {
},
end_line: {
description:
'Optional: The 1-based line number to end reading at (inclusive).',
type: 'number',
};
} else {
properties['offset'] = {
description:
"Optional: For text files, the 0-based line number to start reading from. Requires 'limit' to be set. Use for paginating through large files.",
type: 'number',
};
properties['limit'] = {
description:
"Optional: For text files, maximum number of lines to read. Use with 'offset' to paginate through large files. If omitted, reads the entire file (if feasible, up to a default limit).",
type: 'number',
};
}
},
};
return {
name: this.name,
description: isGemini3
? `Reads a specific range of a file (up to ${DEFAULT_MAX_LINES_TEXT_FILE} lines). **Important:** For high token efficiency, avoid reading large files in their entirety. Use 'grep_search' to find symbols or 'run_shell_command' with 'sed' for surgical block extraction instead of broad file reads. Handles text, images, audio, and PDF files.`
: `Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. Handles text, images, audio, and PDF files. For text files, it can read specific line ranges.`,
? `Reads a specific range of a file (up to ${DEFAULT_MAX_LINES_TEXT_FILE} lines). **Important:** For high token efficiency, avoid reading large files in their entirety. Use 'grep_search' to find symbols or 'run_shell_command' with 'sed' for surgical block extraction instead of broad file reads. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files.`
: `Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'start_line' and 'end_line' parameters. Handles text, images, audio, and PDF files. For text files, it can read specific line ranges.`,
parametersJsonSchema: {
properties,
required: ['file_path'],
@@ -295,12 +266,6 @@ export class ReadFileTool extends BaseDeclarativeTool<
return validationError;
}
if (params.offset !== undefined && params.offset < 0) {
return 'Offset must be a non-negative number';
}
if (params.limit !== undefined && params.limit <= 0) {
return 'Limit must be a positive number';
}
if (params.start_line !== undefined && params.start_line < 1) {
return 'start_line must be at least 1';
}
+6 -8
View File
@@ -930,7 +930,7 @@ describe('fileUtils', () => {
expect(result.returnDisplay).toContain('Path is a directory');
});
it('should paginate text files correctly (offset and limit)', async () => {
it('should paginate text files correctly (start_line and end_line)', async () => {
const lines = Array.from({ length: 20 }, (_, i) => `Line ${i + 1}`);
actualNodeFs.writeFileSync(testTextFilePath, lines.join('\n'));
@@ -938,8 +938,8 @@ describe('fileUtils', () => {
testTextFilePath,
tempRootDir,
new StandardFileSystemService(),
5,
5,
6,
10,
); // Read lines 6-10
const expectedContent = lines.slice(5, 10).join('\n');
@@ -958,8 +958,6 @@ describe('fileUtils', () => {
testTextFilePath,
tempRootDir,
new StandardFileSystemService(),
undefined,
undefined,
5,
10,
); // Read lines 5-10 (1-based)
@@ -976,13 +974,13 @@ describe('fileUtils', () => {
const lines = Array.from({ length: 20 }, (_, i) => `Line ${i + 1}`);
actualNodeFs.writeFileSync(testTextFilePath, lines.join('\n'));
// Read from line 11 to 20. The start is not 0, so it's truncated.
// Read from line 11 to 20. The start is not 1, so it's truncated.
const result = await processSingleFileContent(
testTextFilePath,
tempRootDir,
new StandardFileSystemService(),
10,
10,
11,
20,
);
const expectedContent = lines.slice(10, 20).join('\n');
-9
View File
@@ -400,8 +400,6 @@ export interface ProcessedFileReadResult {
* @param filePath Absolute path to the file.
* @param rootDirectory Absolute path to the project root for relative path display.
* @param _fileSystemService Placeholder for backward compatibility.
* @param offset Optional offset for text files (0-based line number).
* @param limit Optional limit for text files (number of lines to read).
* @param startLine Optional 1-based line number to start reading from.
* @param endLine Optional 1-based line number to end reading at (inclusive).
* @returns ProcessedFileReadResult object.
@@ -411,8 +409,6 @@ export async function processSingleFileContent(
rootDirectory: string,
// TODO: remove unused vars from other areas
_fileSystemService?: FileSystemService,
offset?: number,
limit?: number,
startLine?: number,
endLine?: number,
): Promise<ProcessedFileReadResult> {
@@ -488,11 +484,6 @@ export async function processSingleFileContent(
sliceEnd = endLine
? Math.min(endLine, originalLineCount)
: originalLineCount;
} else if (offset !== undefined || limit !== undefined) {
sliceStart = offset || 0;
const effectiveLimit =
limit === undefined ? DEFAULT_MAX_LINES_TEXT_FILE : limit;
sliceEnd = Math.min(sliceStart + effectiveLimit, originalLineCount);
} else {
sliceEnd = Math.min(DEFAULT_MAX_LINES_TEXT_FILE, originalLineCount);
}