fix: stop reading big files that crash context #6667
Conversation
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
dosubot
bot
added
size:XL
liwilliam2021
changed the title
![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}
There was a problem hiding this comment.
Thank you for your contribution! I've reviewed the changes and the implementation looks solid overall. The approach to prevent infinite hanging on large files is well thought out. I've left some suggestions inline that could improve performance and robustness.
| * Conservative buffer percentage for file reading. | ||
| * We use a very conservative estimate to ensure files fit in context. | ||
| */ | ||
| const FILE_READ_BUFFER_PERCENTAGE = 0.4 // 40% buffer for safety |
There was a problem hiding this comment.
Is the 40% buffer intentionally this conservative? It might be worth making this configurable or adjusting based on model capabilities. Some models might handle closer-to-limit content better than others.
There was a problem hiding this comment.
It seems like we shouldn’t need to be so conservative here if the rest of the logic is working right
There was a problem hiding this comment.
yeah sorry-- I think I just picked a big number for the simple version
hannesrudolph
added
the
Issue/PR - Triage
- Fix Hindi translation punctuation - Fix race condition by checking stream.destroyed - Optimize newline counting with regex - Performance improvements for large file handling - Defensive programming for end parameter already in place
hannesrudolph
removed
the
Issue/PR - Triage
daniel-lxs
left a comment
daniel-lxs
left a comment
There was a problem hiding this comment.
Thank you @liwilliam2021 this finally defeated my file
daniel-lxs
moved this from PR [Needs Prelim Review]
to PR [Needs Review]
in Roo Code Roadmap
dosubot
bot
added
size:L
| const remainingTokens = contextWindow - currentlyUsed | ||
| const usableTokens = Math.floor(remainingTokens * (1 - FILE_READ_BUFFER_PERCENTAGE)) | ||
|
|
||
| // Reserve space for response (use 25% of remaining or 4096, whichever is smaller) |
There was a problem hiding this comment.
We should use the common logic for this
| // For large files or when approaching limits, always limit | ||
| if (fileSizeBytes > safeCharLimit || fileSizeBytes > LARGE_FILE_SIZE) { | ||
| // Use a very conservative limit | ||
| const finalLimit = Math.min(safeCharLimit, 100000) // Cap at 100K chars |
There was a problem hiding this comment.
I think this might annoy people who are trying to use a model with a large context window to read large files
There was a problem hiding this comment.
I think the plan for this PR was to do something stupid to fix the temporary error-- basically never reading big files
This PR has the full implementation and doesn't have that limit
#6319
daniel-lxs
moved this from PR [Needs Review]
to PR [Changes Requested]
in Roo Code Roadmap
hannesrudolph
moved this from PR [Changes Requested]
to PR [Needs Prelim Review]
in Roo Code Roadmap
|
Let's get this across the finish line |
hannesrudolph
added
PR - Needs Preliminary Review
and removed
PR - Changes Requested
labels
Implements a simple, token-budget based file reading system that prevents context window overflow and tokenizer crashes. Problem: - Files could fill entire context window causing issues - tiktoken crashes with 'unreachable' error on files >5MB - PR #6667's approach was too complex with magic numbers Solution - Multi-Layer Defense: 1. Fast path: Files <100KB skip validation (no overhead) 2. Token validation: 100KB-5MB files use real token counting - Budget: (contextWindow - currentTokens) * 0.6 - Smart truncation if exceeds budget 3. Preview mode: Files >5MB get 100KB preview (prevents crashes) 4. Error recovery: Catch tokenizer 'unreachable' errors gracefully Key Features: - No magic numbers - dynamic based on actual context - Real token counting using existing tokenizer - 100KB previews for large files (perfect size for structure visibility) - Graceful error handling prevents conversation crashes - Simple implementation (~160 lines vs complex heuristics) Testing: - 17 comprehensive tests covering all scenarios - All tests passing including edge cases and error conditions Files: - src/core/tools/helpers/fileTokenBudget.ts: Core validation logic - src/core/tools/helpers/__tests__/fileTokenBudget.spec.ts: Test suite - src/core/tools/readFileTool.ts: Integration into read file tool
github-project-automation
bot
moved this from PR [Needs Prelim Review]
to Done
in Roo Code Roadmap
Implemented a simple version of: #6319
We now always stop reading after a limited read. We also use simpler heuristics that do not call the tokenizer to validate while remaining conservative.
Goal: prevent infinite hanging on large files when partial reads are off. This may limit the ability of the model to read large files which is addressed in the next PR.
Important
Introduces file size validation in
readFileTool.tsto prevent crashes from large files by limiting reads based on context size.readFileTool.tsusingvalidateFileSizeForContextfrom newcontextValidator.ts.readFileTool.spec.tsto test new validation logic and partial read notices.maxCharsparameter inread-lines.spec.tsto ensure character limit handling.showingOnlyLinestranslation in multipletools.jsonfiles for different languages.This description was created by
for 75fb09a. You can customize this summary. It will automatically update as commits are pushed.