Skip to content

Mirror: fix: treat maxReadFileLine=0 as unlimited (#5575) #36

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.

Sign up for GitHub

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

Closed
jeremylongshore wants to merge 1 commit into from
Closed

Mirror: fix: treat maxReadFileLine=0 as unlimited (#5575) #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/integrations/misc/__tests__/extract-text-large-files.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,15 +152,19 @@ describe("extractTextFromFile - Large File Handling", () => {
expect(result).toContain("[File truncated: showing 500 of 10000 total lines")
})

it("should handle maxReadFileLine of 0 by throwing an error", async () => {
it("should handle maxReadFileLine of 0 by treating as unlimited", async () => {
const fileContent = "Line 1\nLine 2\nLine 3"

mockedFs.readFile.mockResolvedValue(fileContent as any)

// maxReadFileLine of 0 should throw an error
await expect(extractTextFromFile("/test/file.ts", 0)).rejects.toThrow(
"Invalid maxReadFileLine: 0. Must be a positive integer or -1 for unlimited.",
)
// maxReadFileLine of 0 should be treated as unlimited (same as -1)
const result = await extractTextFromFile("/test/file.ts", 0)

// Should include all content with line numbers
expect(result).toContain("1 | Line 1")
expect(result).toContain("2 | Line 2")
expect(result).toContain("3 | Line 3")
expect(result).not.toContain("[File truncated:")
})

it("should handle negative maxReadFileLine by treating as undefined", async () => {
Expand Down
8 changes: 5 additions & 3 deletions src/integrations/misc/extract-text.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,11 @@ export function getSupportedBinaryFormats(): string[] {
*/
export async function extractTextFromFile(filePath: string, maxReadFileLine?: number): Promise<string> {
// Validate maxReadFileLine parameter
if (maxReadFileLine !== undefined && maxReadFileLine !== -1) {
// 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.`,
Comment on lines +66 to +70
Copy link
Copy Markdown

@qodo-code-review qodo-code-review Bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

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

)
}
}
Expand All @@ -90,7 +91,8 @@ export async function extractTextFromFile(filePath: string, maxReadFileLine?: nu

if (!isBinary) {
// Check if we need to apply line limit
if (maxReadFileLine !== undefined && maxReadFileLine !== -1) {
// 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)
Comment on lines +94 to 98
Copy link
Copy Markdown

@qodo-code-review qodo-code-review Bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

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

Expand Down
Loading