refactor(tools): add offset/limit pagination to read_file tools#12471
Open
Bijit-Mondal wants to merge 4 commits into
Open
refactor(tools): add offset/limit pagination to read_file tools#12471Bijit-Mondal wants to merge 4 commits into
Bijit-Mondal wants to merge 4 commits into
Conversation
- Replace full-file loading with bounded range reads to reduce memoryusage. The read_file tool now accepts optional offset (1-based line) and limit parameters and returns a pagination hint when more content is available, so the LLM can continue reading without hitting contextlimits. - core: use ide.readRangeInFile() — only the requested line window is fetched, never the full file - cli: replace fs.readFileSync with fs.createReadStream + readline for true O(1) memory regardless of file size - apply 50 KB hard byte cap and 2000-char per-line truncation as secondary guards on output size - remove throwIfFileExceedsHalfOfContext from readFileRangeImpl — the range fetch is already bounded by definition - add getOptionalNumberArg utility to parseArgs
dosubot
Bot
added
the
size:XL
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
Author
|
@sestinj Please Review |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="extensions/cli/src/tools/readFile.ts">
<violation number="1" location="extensions/cli/src/tools/readFile.ts:52">
P2: `readline` buffers complete lines internally before emitting the `'line'` event. For single-line files (e.g., minified JS without newlines), the entire file is loaded into memory as `rawLine` before truncation to `MAX_LINE_LENGTH` is applied. This violates the documented O(output) memory guarantee and can cause memory spikes proportional to file size.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Clamp offset to ≥1 to avoid broken line numbering and non-advancing pagination, clamp limit to ≥1 to prevent immediate stream termination, and clamp effectiveLimit to ≥1 after parallel division to avoid 0-value limits that trigger infinite loops. Handle invalid input values like 0 or negative offset/limit.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
- Clamp offset to ≥ 1 to preserve 1-based line numbering and ensure nextOffset always advances - Clamp limit to ≥ 1 to prevent a zero effectiveLimit that caused the stream to return linesRead=0 and loop infinitely on the same offset - Stop dividing limit by parallelCount: limit is a per-call value, not a shared budget; only effectiveMaxBytes (the internal context cap) is divided, avoiding both quota under-delivery and the effectiveLimit=0 collapse when limit < parallelCount
Author
|
Issues - #12432 |
Replace the ambiguous `>= limit` heuristic with the N+1 sentinel pattern for reliable EOF detection in both core and CLI readFile implementations. When the returned line count exceeds the requested limit, there is unambiguously more content — no false positives on exact-boundary reads. Also introduce MIN_LIMIT (200) to clamp caller-supplied limits, preventing excessive pagination from very small limit values while ensuring meaningful chunks are always returned. Changes: - Request limit+1 lines from the IDE/stream; trim sentinel before output - Replace globalLineCount-based `more` heuristic with outputLines.length > limit - Remove buggy `cut = false` assignment in the line-limit early-stop branch - Add MIN_LIMIT = 200 constant, applied in both core and CLI
Author
|
@sestinj added video of the changes also |
Contributor
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
Adds offset and limit parameters to the read_file tool so the LLM can paginate through large files instead of hitting a hard failure. Replaces the old throw-on-large-file approach with bounded range reads that are O(1) in memory regardless of file size.
read_file): switched from ide.readFile() (full file load) to ide.readRangeInFile() — only the requested line window is fetched from the IDE layer, never the full file.read_file): replaced fs.readFileSync with fs.createReadStream + readline so lines are processed one at a time and the stream is destroyed the moment the byte cap is hit.Also removes throwIfFileExceedsHalfOfContext from readFileRangeImpl — the range fetch is already bounded by definition so the check wasredundant.
AI Code Review
@continue-reviewChecklist
Screen recording or screenshot
2nd one is what the PR does
export-1779467617979.1.1.mp4
Tests
N/A
Summary by cubic
Adds offset/limit pagination to the
read_fileandread_currently_open_filetools so large files are read in predictable, line-numbered windows with a next-offset hint. Core uses bounded range reads; the CLI streams lines with a ~50 KB cap to keep memory O(output).New Features
offset(1-based) andlimiton both tools; defaultsoffset=1,limit=2000with aMIN_LIMIT=200clamp.readFileuseside.readRangeInFileand the N+1 sentinel to detect EOF;readCurrentlyOpenFileslices the editor buffer.Bug Fixes
offset ≥ 1, enforcelimit ≥ 200; stop dividinglimitacross parallel tool calls—only the byte cap is divided—to prevent zero-length windows and infinite loops.throwIfFileExceedsHalfOfContextfrom range paths and related tests.Written for commit d3fadba. Summary will update on new commits. Review in cubic