From 017967b3ee1394d3116dd1d29873b415e37d9f4d Mon Sep 17 00:00:00 2001 From: Adam Weidman Date: Thu, 12 Feb 2026 13:05:26 -0500 Subject: [PATCH] refactor: remove legacy offset/limit from read_file and use 1-based ranges exclusively --- .../__snapshots__/read-file.test.ts.snap | 4 +- .../core/src/tools/definitions/coreTools.ts | 16 +--- packages/core/src/tools/read-file.test.ts | 75 ++----------------- packages/core/src/tools/read-file.ts | 53 +++---------- packages/core/src/utils/fileUtils.test.ts | 14 ++-- packages/core/src/utils/fileUtils.ts | 9 --- 6 files changed, 28 insertions(+), 143 deletions(-) diff --git a/packages/core/src/tools/__snapshots__/read-file.test.ts.snap b/packages/core/src/tools/__snapshots__/read-file.test.ts.snap index 2ae056975c..873c25efd7 100644 --- a/packages/core/src/tools/__snapshots__/read-file.test.ts.snap +++ b/packages/core/src/tools/__snapshots__/read-file.test.ts.snap @@ -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."`; diff --git a/packages/core/src/tools/definitions/coreTools.ts b/packages/core/src/tools/definitions/coreTools.ts index 50c7028b2e..4a75285cb0 100644 --- a/packages/core/src/tools/definitions/coreTools.ts +++ b/packages/core/src/tools/definitions/coreTools.ts @@ -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', }, }, diff --git a/packages/core/src/tools/read-file.test.ts b/packages/core/src/tools/read-file.test.ts index 9579b3483b..ce2ea4c33c 100644 --- a/packages/core/src/tools/read-file.test.ts +++ b/packages/core/src/tools/read-file.test.ts @@ -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; } ).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; - } - ).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); diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index 38d99826a1..ef9a3e30e7 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -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'; } diff --git a/packages/core/src/utils/fileUtils.test.ts b/packages/core/src/utils/fileUtils.test.ts index b6c5192139..393b50ae36 100644 --- a/packages/core/src/utils/fileUtils.test.ts +++ b/packages/core/src/utils/fileUtils.test.ts @@ -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'); diff --git a/packages/core/src/utils/fileUtils.ts b/packages/core/src/utils/fileUtils.ts index a70f901fdd..afb3e8a393 100644 --- a/packages/core/src/utils/fileUtils.ts +++ b/packages/core/src/utils/fileUtils.ts @@ -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 { @@ -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); }