[ENHANCEMENT] Refactor read_file tool #10257
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dosubot
bot
added
size:XXL
Contributor
Re-review complete.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
hannesrudolph
force-pushed
the
feat/read-file-codex-api
branch
from
b153038 to
b0362be
Compare
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
hannesrudolph
added
the
Issue/PR - Triage
hannesrudolph
force-pushed
the
feat/read-file-codex-api
branch
from
7ab3bf4 to
91018ae
Compare
hannesrudolph
removed
the
Issue/PR - Triage
hannesrudolph
force-pushed
the
feat/read-file-codex-api
branch
from
c9fdfa8 to
4dce77f
Compare
hannesrudolph
changed the title
hannesrudolph
force-pushed
the
feat/read-file-codex-api
branch
from
e73c499 to
7bd1b7d
Compare
hannesrudolph
force-pushed
the
feat/read-file-codex-api
branch
from
d347945 to
ba91006
Compare
| lineNumber++ | ||
| records.push(createLineRecord(lineNumber, buffer)) | ||
| } | ||
| console.log( |
Contributor
There was a problem hiding this comment.
collectFileLines() has a console.log(...) on stream end; since this runs in the read_file execution path it will spam logs and potentially leak file contents into logs when reading sensitive files. This looks like leftover debugging and should be removed before merge.
Fix it with Roo Code or mention @roomote and request a fix.
…g modes Replace line_ranges with new offset/limit/mode API for improved file reading: - Slice mode: simple line-by-line reading with offset pagination - Indentation mode: smart code block extraction based on indentation - Add rich metadata for pagination awareness (hasMoreBefore/After, etc.) - Remove deprecated truncateDefinitions helper - Update tool definition with new parameters and examples - Default maxReadFileLine from 500 to 2000 lines
- Use consistent camelCase in error messages (anchorLine, maxLines instead of snake_case) - Accept both camelCase and snake_case for indentation config in NativeToolCallParser - Continue counting total lines after hitting limit in readSlice() for accurate metadata - Return empty content instead of throwing when offset exceeds file length - Disable strict mode in tool schema when partial reads enabled to allow truly optional params - Update tests to match new behavior
…indentation mode When totalLinesInFile is 0, there are no lines before the anchor point, so hasMoreBefore must be false regardless of the offset/anchorLine value.
The limit parameter was defined in FileEntry but intentionally ignored by ReadFileTool since the line limit is controlled by the maxReadFileLine setting. This removes the unused type field and parsing code to align the types with actual behavior. - Remove limit from FileEntry interface in tool-params.ts - Remove limit parsing from NativeToolCallParser.convertFileEntries() - Remove limit from FileResult interface in ReadFileTool.ts - Update comment to remove obsolete limit mention
…leLine setting Addresses review comment about FileEntry.limit being defined but ignored. The limit parameter was intentionally not exposed to models - it's controlled by the maxReadFileLine user setting instead. Updated the comment in NativeToolCallParser.ts to clarify this design decision.
Updates tests to match the new API that replaces line_ranges with: - offset: 1-indexed starting line number - mode: 'slice' or 'indentation' reading mode - indentation: configuration for indentation mode Also updates strict mode expectations since partialReadsEnabled (default true) now sets strict to false to allow optional parameters.
…test The ReadFileTool's execute method requires additional Task methods that were not mocked: - sayAndCreateMissingParamError: called when files array is empty/undefined - cwd: used for path resolution - apiConfiguration: used for protocol resolution - taskToolProtocol: used for protocol resolution - rooIgnoreController: used for access validation - fileContextTracker: used for tracking file context This fix ensures the test properly mocks all dependencies required by the ReadFileTool when invoked through presentAssistantMessage.
…y.limit and truncatedByLimit
- Fix MAX_LINE_BYTES to 500 (was 2000), rename from MAX_LINE_LENGTH for clarity - Implement byte-based UTF-8 line truncation at safe boundaries (safeUtf8Truncate) - Rename FALLBACK_LIMIT to DEFAULT_LINE_LIMIT for clarity - Add maxLines property to indentation schema in tool definition - Add test for UTF-8 boundary truncation with emoji
hannesrudolph
force-pushed
the
feat/read-file-codex-api
branch
from
1264e77 to
4b14363
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Implements a new, more intuitive API for the
read_filetool, inspired by OpenAI's Codex CLI read_file implementation. This replaces the previousline_rangesapproach with a cleaner offset/limit/mode-based system that provides better pagination and smarter code extraction capabilities.closes #10239
Changes
New API Parameters: Replace
line_rangeswith:offset: 1-indexed starting line number (default: 1)limit: Maximum lines to return (controlled by maxReadFileLine setting)mode: Eitherslice(simple reading) orindentation(smart block extraction)indentation: Configuration for indentation mode (anchorLine, maxLevels, includeSiblings, includeHeader)New
read-file-content.tsmodule: Core implementation with:readSlice(): Simple line-by-line reading with offset paginationreadIndentationBlock(): Smart extraction of code blocks based on indentation levelsEnhanced Model Awareness: Tool descriptions now include the configured line limit so models understand pagination constraints
Improved Default Limit: Changed default maxReadFileLine from 500 to 2000 lines for better out-of-box experience
Removed Deprecated Code:
truncateDefinitions.tshelper and its testsCredits
This implementation is inspired by and adapts concepts from OpenAI's Codex CLI, particularly their approach to file reading with offset-based pagination and indentation-aware code block extraction.
Important
Refactor
read_filetool to use offset/limit/mode parameters, adding slice and indentation modes, and update related tests and settings.line_rangeswithoffset,limit, andmodeparameters inread_filetool.sliceandindentationmodes for reading files.maxReadFileLinedefault from 500 to 2000.readSlice()andreadIndentationBlock()functions inread-file-content.ts.truncateDefinitions.tsand related code.readFileTool.spec.tsandread-file-content.spec.tsto cover new API.ContextManagementSettings.tsxto reflect newmaxReadFileLinedefault.This description was created by
for 1264e77. You can customize this summary. It will automatically update as commits are pushed.