Mirror: fix: treat maxReadFileLine=0 as unlimited (#5575)#36
Closed
jeremylongshore wants to merge 1 commit intomainfrom
Closed
Mirror: fix: treat maxReadFileLine=0 as unlimited (#5575)#36jeremylongshore wants to merge 1 commit intomainfrom
jeremylongshore wants to merge 1 commit intomainfrom
Conversation
Users setting maxReadFileLine to 0 expect it to mean 'no limit', similar to -1. This fix ensures 0 is treated as unlimited, preventing errors when using @file mentions with this setting. Fixes Kilo-Org#5535
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoTreat maxReadFileLine=0 as unlimited for backward compatibility
WalkthroughsDescription• Treat maxReadFileLine=0 as unlimited, same as -1 • Update validation logic to accept 0 as valid unlimited value • Modify test to verify 0 reads all file content • Update error message to reflect new behavior Diagramflowchart LR
A["maxReadFileLine parameter"] --> B{"Value check"}
B -->|"0 or -1"| C["Treat as unlimited"]
B -->|"positive integer"| D["Apply line limit"]
B -->|"invalid"| E["Throw error"]
C --> F["Read entire file"]
D --> G["Read up to limit"]
File Changes1. src/integrations/misc/extract-text.ts
|
|
Failed to generate code suggestions for PR |
Code Review by Qodo
1. No changeset for extractTextFromFile
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+66
to
+70
| // 0 is treated as unlimited (same as -1) for backward compatibility | ||
| if (maxReadFileLine !== undefined && maxReadFileLine !== -1 && maxReadFileLine !== 0) { | ||
| if (!Number.isInteger(maxReadFileLine) || maxReadFileLine < 1) { | ||
| throw new Error( | ||
| `Invalid maxReadFileLine: ${maxReadFileLine}. Must be a positive integer or -1 for unlimited.`, | ||
| `Invalid maxReadFileLine: ${maxReadFileLine}. Must be a positive integer, 0, or -1 for unlimited.`, |
There was a problem hiding this comment.
1. No changeset for extracttextfromfile 📘 Rule violation ⛯ Reliability
This PR makes a functional behavior change (treating maxReadFileLine=0 as unlimited) but does not include an accompanying .changeset/*.md entry. Without a changeset, the user-facing fix may be missing from release notes/versioning.
Agent Prompt
## Issue description
This PR introduces a functional behavior change but does not include a required Changesets entry under `.changeset/`.
## Issue Context
Per repo compliance, non-documentation/non-internal-tooling changes must include a changeset so the fix is included in release notes and versioning.
## Fix Focus Areas
- .changeset/[new-file].md[1-6]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+94
to
98
| // 0 is treated as unlimited (same as -1) for backward compatibility | ||
| if (maxReadFileLine !== undefined && maxReadFileLine !== -1 && maxReadFileLine !== 0) { | ||
| const totalLines = await countFileLines(filePath) | ||
| if (totalLines > maxReadFileLine) { | ||
| // Read only up to maxReadFileLine (endLine is 0-based and inclusive) |
There was a problem hiding this comment.
2. 0 semantics inconsistent 🐞 Bug ✓ Correctness
This PR makes extractTextFromFile treat maxReadFileLine=0 as unlimited, but other subsystems treat 0 as “definitions-only/0 lines”, so the same setting can produce contradictory behavior and unexpectedly inject full file contents into mentions (risking context exhaustion). Aligning semantics across mentions, read_file, and prompt/tool gating is required.
Agent Prompt
## Issue description
`maxReadFileLine` is a shared setting used by multiple features (mentions, read_file tool, system prompt/tool building). In this repo, `maxReadFileLine === 0` is already established as a **definitions-only / 0 lines** mode in the read_file tool, but this PR changes `extractTextFromFile` to treat `0` as **unlimited**, which can cause full file contents to be included in file mentions when the global setting is `0`.
## Issue Context
- `extractTextFromFile` is used by the mentions system with the global `maxReadFileLine` value.
- `ReadFileTool` and helper tests explicitly treat `0` as "definitions only mode".
- Tool/prompt gating treats `maxReadFileLine !== -1` as "partial reads enabled", which may also need updating if `0` is redefined.
## Fix Focus Areas
Decide on one consistent meaning and implement it across all call sites and docs.
- src/integrations/misc/extract-text.ts[64-109]
- src/core/mentions/index.ts[193-207]
- src/core/mentions/index.ts[287-307]
- src/core/tools/ReadFileTool.ts[482-505]
- src/core/task/build-tools.ts[161-172]
- src/core/task/Task.ts[4253-4257]
- src/core/tools/helpers/__tests__/truncateDefinitions.spec.ts[15-23]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Owner
Author
|
@greptileai review |
Owner
Author
Owner
Author
|
Merged into |
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
2 participants
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.
Mirror of Kilo-Org#5575
This PR mirrors the upstream change for multi-AI review analysis.
Bot Review Tracker
Links