-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: prevent context window overflow for directory mentions with MiniMax #9132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…h @mentions - Add size limits for directory content (50KB total, ~12-15K tokens) - Limit to maximum 20 files per directory mention - Add 10KB per-file size limit when reading directory contents - Truncate large files and show warning messages - Add informative messages when content is limited This fixes the MiniMax context window exceeded error (2013) when using @ to reference directories with many files.
Review completed. Found 3 issues that need attention:
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
| folderContent += `${linePrefix}${displayName}\n` | ||
| if (!isIgnored) { | ||
| // Check if we've reached the file limit or size limit | ||
| if (filesIncluded >= MAX_FILES_TO_INCLUDE || totalContentSize >= MAX_DIR_CONTENT_SIZE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beware: the shared counters (filesIncluded, totalContentSize) are updated inside asynchronous functions, but the loop’s synchronous check (if (filesIncluded >= MAX_FILES_TO_INCLUDE || totalContentSize >= MAX_DIR_CONTENT_SIZE)) uses values that haven’t yet been updated. This may lead to a race condition where limits are not correctly enforced. Consider processing entries sequentially or otherwise synchronizing updates.
| let totalContentSize = 0 | ||
| let filesIncluded = 0 | ||
| let filesTruncated = 0 | ||
| let filesSkipped = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Race condition: totalContentSize, filesIncluded, filesTruncated, and filesSkipped are updated inside async promises that execute concurrently via Promise.all(). Multiple promises can read and update these shared counters simultaneously, leading to inaccurate counts. For example, two promises could both read totalContentSize as 40KB, each add 15KB of content, and both increment the counter, resulting in a final value of 55KB instead of the correct 70KB. This undermines the size limiting logic and could allow the context window to be exceeded.
Fix it with Roo Code or mention @roomote and request a fix.
| folderContent += `${linePrefix}${displayName}\n` | ||
| if (!isIgnored) { | ||
| // Check if we've reached the file limit or size limit | ||
| if (filesIncluded >= MAX_FILES_TO_INCLUDE || totalContentSize >= MAX_DIR_CONTENT_SIZE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent size limit enforcement: Line 323 checks totalContentSize >= MAX_DIR_CONTENT_SIZE before processing files, but line 351 checks totalContentSize + content.length > MAX_DIR_CONTENT_SIZE after reading. This means when totalContentSize equals or exceeds the limit, the code still attempts to read files (wasting I/O), only to skip them afterward. The pre-check should use the same logic as the post-check to avoid unnecessary file reads.
Fix it with Roo Code or mention @roomote and request a fix.
| const fileStats = await fs.stat(absoluteFilePath) | ||
| if (fileStats.size > MAX_FILE_SIZE) { | ||
| // For large files in directories, truncate the content | ||
| const content = await extractTextFromFile(absoluteFilePath, 50) // Read only first 50 lines | ||
| const truncatedContent = content.substring(0, MAX_FILE_SIZE) | ||
| filesTruncated++ | ||
| filesIncluded++ | ||
| totalContentSize += truncatedContent.length | ||
| return `<file_content path="${filePath.toPosix()}">\n${truncatedContent}\n[... File truncated. Showing first ${MAX_FILE_SIZE} bytes of ${fileStats.size} bytes ...]\n</file_content>` | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inefficient file size check: The code checks fileStats.size > MAX_FILE_SIZE at line 340, then calls extractTextFromFile(absoluteFilePath, 50) at line 342 to read 50 lines before truncating. For large files, extractTextFromFile performs expensive I/O operations (file stats, line counting, file reading) before the content is truncated. The file size check should happen before any extractTextFromFile call to avoid unnecessary disk reads on files that will be truncated anyway.
Fix it with Roo Code or mention @roomote and request a fix.
This PR attempts to address Issue #9131. Feedback and guidance are welcome.
Problem
When using @ to reference directories in a new task with the MiniMax model, users were encountering a "context window exceeded" error (2013). The MiniMax model has a 192K token context window, and when referencing directories with many files, all file contents were being included without any size limits, causing the overflow.
Solution
Added size limits for directory content when processing @ mentions to prevent context window overflow:
Changes
src/core/mentions/index.tsto add configurable size limits in thegetFileOrFolderContentfunctionTesting
npx vitest run core/mentions/__tests__/index.spec.tsnpx vitest run core/mentions/__tests__/processUserContentMentions.spec.tsImpact
Fixes #9131
Important
Adds size limits to directory mentions in
index.tsto prevent MiniMax context window overflow, with truncation and warnings for large files.getFileOrFolderContentinindex.tsto prevent context window overflow for directory mentions.npx vitest run core/mentions/__tests__/index.spec.tsandprocessUserContentMentions.spec.ts.This description was created by
for e69fd19. You can customize this summary. It will automatically update as commits are pushed.