From 75fb09af4fb49cfb0fcced5169836ceb0506e79e Mon Sep 17 00:00:00 2001 From: Will Li Date: Wed, 6 Aug 2025 09:22:46 -0700 Subject: [PATCH] collapse char reads into one file --- src/core/tools/__tests__/readFileTool.spec.ts | 56 +------ src/core/tools/readFileTool.ts | 10 +- .../misc/__tests__/read-lines.spec.ts | 74 ++++++++ src/integrations/misc/read-lines.ts | 41 ++++- src/integrations/misc/read-partial-content.ts | 158 ------------------ 5 files changed, 124 insertions(+), 215 deletions(-) delete mode 100644 src/integrations/misc/read-partial-content.ts diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts index 700bcd1574..af1dbaa707 100644 --- a/src/core/tools/__tests__/readFileTool.spec.ts +++ b/src/core/tools/__tests__/readFileTool.spec.ts @@ -4,7 +4,6 @@ import * as path from "path" import { countFileLines } from "../../../integrations/misc/line-counter" import { readLines } from "../../../integrations/misc/read-lines" -import { readPartialContent } from "../../../integrations/misc/read-partial-content" import { extractTextFromFile, addLineNumbers, getSupportedBinaryFormats } from "../../../integrations/misc/extract-text" import { parseSourceCodeDefinitionsForFile } from "../../../services/tree-sitter" import { isBinaryFile } from "isbinaryfile" @@ -35,17 +34,6 @@ vi.mock("../../../integrations/misc/line-counter") vi.mock("../../../integrations/misc/read-lines", () => ({ readLines: vi.fn().mockResolvedValue("mocked line content"), })) -vi.mock("../../../integrations/misc/read-partial-content", () => ({ - readPartialSingleLineContent: vi.fn().mockResolvedValue("mocked partial content"), - readPartialContent: vi.fn().mockResolvedValue({ - content: "mocked partial content", - charactersRead: 100, - totalCharacters: 1000, - linesRead: 5, - totalLines: 50, - lastLineRead: 5, - }), -})) vi.mock("../contextValidator") // Mock fs/promises readFile for image tests @@ -1379,15 +1367,8 @@ describe("read_file tool XML output structure", () => { reason: "This is a partial read - the remaining content cannot be accessed due to context limitations.", }) - // Mock readPartialContent to return truncated content - vi.mocked(readPartialContent).mockResolvedValue({ - content: "Line 1\nLine 2\n...truncated...", - charactersRead: 2000, - totalCharacters: 500000, - linesRead: 100, - totalLines: 10000, - lastLineRead: 100, - }) + // Mock readLines to return truncated content with maxChars + vi.mocked(readLines).mockResolvedValue("Line 1\nLine 2\n...truncated...") const result = await executeReadFileTool( { args: `large-file.ts` }, @@ -1430,15 +1411,8 @@ describe("read_file tool XML output structure", () => { reason: "This is a partial read - the remaining content cannot be accessed due to context limitations.", }) - // Mock readPartialContent to return truncated content for single-line file - vi.mocked(readPartialContent).mockResolvedValue({ - content: "const a=1;const b=2;...truncated", - charactersRead: 5000, - totalCharacters: 10000, - linesRead: 1, - totalLines: 1, - lastLineRead: 1, - }) + // Mock readLines to return truncated content for single-line file with maxChars + vi.mocked(readLines).mockResolvedValue("const a=1;const b=2;...truncated") const result = await executeReadFileTool( { args: `minified.js` }, @@ -1463,15 +1437,8 @@ describe("read_file tool XML output structure", () => { reason: "This is a partial read - the remaining content cannot be accessed due to context limitations.", }) - // Mock readPartialContent to return truncated content - vi.mocked(readPartialContent).mockResolvedValue({ - content: "Line 1\nLine 2\n...truncated...", - charactersRead: 50000, - totalCharacters: 250000, - linesRead: 1000, - totalLines: 5000, - lastLineRead: 1000, - }) + // Mock readLines to return truncated content with maxChars + vi.mocked(readLines).mockResolvedValue("Line 1\nLine 2\n...truncated...") const result = await executeReadFileTool( { args: `large-file.ts` }, @@ -1496,15 +1463,8 @@ describe("read_file tool XML output structure", () => { reason: "This is a partial read - the remaining content cannot be accessed due to context limitations.", }) - // Mock readPartialContent for single-line file - vi.mocked(readPartialContent).mockResolvedValue({ - content: "const a=1;const b=2;const c=3;", - charactersRead: 8000, - totalCharacters: 10000, - linesRead: 1, - totalLines: 1, - lastLineRead: 1, - }) + // Mock readLines for single-line file with maxChars + vi.mocked(readLines).mockResolvedValue("const a=1;const b=2;const c=3;") const result = await executeReadFileTool( { args: `semi-large.js` }, diff --git a/src/core/tools/readFileTool.ts b/src/core/tools/readFileTool.ts index 0834297852..946ee4bf41 100644 --- a/src/core/tools/readFileTool.ts +++ b/src/core/tools/readFileTool.ts @@ -12,7 +12,6 @@ import { isPathOutsideWorkspace } from "../../utils/pathUtils" import { getReadablePath } from "../../utils/path" import { countFileLines } from "../../integrations/misc/line-counter" import { readLines } from "../../integrations/misc/read-lines" -import { readPartialContent } from "../../integrations/misc/read-partial-content" import { extractTextFromFile, addLineNumbers, getSupportedBinaryFormats } from "../../integrations/misc/extract-text" import { parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter" import { parseXml } from "../../utils/xml" @@ -578,12 +577,15 @@ export async function readFileTool( // Handle files with validation limits (character-based reading) if (shouldApplyValidation) { - const result = await readPartialContent(fullPath, validation.safeContentLimit) + const partialContent = await readLines(fullPath, undefined, undefined, validation.safeContentLimit) + + // Count lines in the partial content + const linesRead = partialContent ? (partialContent.match(/\n/g) || []).length + 1 : 0 // Generate line range attribute based on what was read - const lineRangeAttr = result.linesRead === 1 ? ` lines="1"` : ` lines="1-${result.lastLineRead}"` + const lineRangeAttr = linesRead === 1 ? ` lines="1"` : ` lines="1-${linesRead}"` - const content = addLineNumbers(result.content, 1) + const content = addLineNumbers(partialContent, 1) let xmlInfo = `\n${content}\n` // Add simple notice about partial read diff --git a/src/integrations/misc/__tests__/read-lines.spec.ts b/src/integrations/misc/__tests__/read-lines.spec.ts index 14456d24f1..01130031ee 100644 --- a/src/integrations/misc/__tests__/read-lines.spec.ts +++ b/src/integrations/misc/__tests__/read-lines.spec.ts @@ -128,5 +128,79 @@ describe("nthline", () => { expect(lines).toEqual("\n\n\n") }) }) + + describe("maxChars parameter", () => { + it("should limit output to maxChars when reading entire file", async () => { + const content = await readLines(testFile, undefined, undefined, 20) + expect(content).toEqual("Line 1\nLine 2\nLine 3") + expect(content.length).toBe(20) + }) + + it("should limit output to maxChars when reading a range", async () => { + const content = await readLines(testFile, 5, 1, 15) + // When maxChars cuts off in the middle of a line, we get partial content + expect(content).toEqual("Line 2\nLine 3\nL") + expect(content.length).toBe(15) + }) + + it("should return empty string when maxChars is 0", async () => { + const content = await readLines(testFile, undefined, undefined, 0) + expect(content).toEqual("") + }) + + it("should handle maxChars smaller than first line", async () => { + const content = await readLines(testFile, undefined, undefined, 3) + expect(content).toEqual("Lin") + expect(content.length).toBe(3) + }) + + it("should handle maxChars that cuts off in the middle of a line", async () => { + const content = await readLines(testFile, 2, 0, 10) + expect(content).toEqual("Line 1\nLin") + expect(content.length).toBe(10) + }) + + it("should respect both line limits and maxChars", async () => { + // This should read lines 2-4, but stop at 25 chars + const content = await readLines(testFile, 3, 1, 25) + expect(content).toEqual("Line 2\nLine 3\nLine 4\n") + expect(content.length).toBeLessThanOrEqual(25) + }) + + it("should handle maxChars with single line file", async () => { + await withTempFile( + "single-line-maxchars.txt", + "This is a long single line of text", + async (filepath) => { + const content = await readLines(filepath, undefined, undefined, 10) + expect(content).toEqual("This is a ") + expect(content.length).toBe(10) + }, + ) + }) + + it("should handle maxChars with Unicode characters", async () => { + await withTempFile("unicode-maxchars.txt", "Hello 😀 World\nLine 2", async (filepath) => { + // Note: The emoji counts as 2 chars in JavaScript strings + const content = await readLines(filepath, undefined, undefined, 10) + expect(content).toEqual("Hello 😀 W") + expect(content.length).toBe(10) + }) + }) + + it("should handle maxChars larger than file size", async () => { + const content = await readLines(testFile, undefined, undefined, 1000) + const fullContent = await readLines(testFile) + expect(content).toEqual(fullContent) + }) + + it("should handle maxChars with empty lines", async () => { + await withTempFile("empty-lines-maxchars.txt", "Line 1\n\n\nLine 4\n", async (filepath) => { + const content = await readLines(filepath, undefined, undefined, 10) + expect(content).toEqual("Line 1\n\n\nL") + expect(content.length).toBe(10) + }) + }) + }) }) }) diff --git a/src/integrations/misc/read-lines.ts b/src/integrations/misc/read-lines.ts index 5a5eda9f83..1893f162bc 100644 --- a/src/integrations/misc/read-lines.ts +++ b/src/integrations/misc/read-lines.ts @@ -18,10 +18,11 @@ const outOfRangeError = (filepath: string, n: number) => { * @param filepath - Path to the file to read * @param endLine - Optional. The line number to stop reading at (inclusive). If undefined, reads to the end of file. * @param startLine - Optional. The line number to start reading from (inclusive). If undefined, starts from line 0. + * @param maxChars - Optional. Maximum number of characters to read. If specified, reading stops when this limit is reached. * @returns Promise resolving to a string containing the read lines joined with newlines * @throws {RangeError} If line numbers are invalid or out of range */ -export function readLines(filepath: string, endLine?: number, startLine?: number): Promise { +export function readLines(filepath: string, endLine?: number, startLine?: number, maxChars?: number): Promise { return new Promise((resolve, reject) => { // Reject if startLine is defined but not a number if (startLine !== undefined && typeof startLine !== "number") { @@ -52,11 +53,16 @@ export function readLines(filepath: string, endLine?: number, startLine?: number ) } - // Set up stream - const input = createReadStream(filepath) + // Set up stream - only add 'end' option when maxChars is specified to avoid reading entire file + const streamOptions = + maxChars !== undefined + ? { end: Math.min(maxChars * 2, maxChars + 1024 * 1024) } // Read at most 2x maxChars or maxChars + 1MB + : undefined + const input = createReadStream(filepath, streamOptions) let buffer = "" let lineCount = 0 let result = "" + let totalCharsRead = 0 // Handle errors input.on("error", reject) @@ -73,7 +79,24 @@ export function readLines(filepath: string, endLine?: number, startLine?: number while (nextNewline !== -1) { // If we're in the target range, add this line to the result if (lineCount >= effectiveStartLine && (endLine === undefined || lineCount <= endLine)) { - result += buffer.substring(pos, nextNewline + 1) // Include the newline + const lineToAdd = buffer.substring(pos, nextNewline + 1) // Include the newline + + // Check if adding this line would exceed maxChars (only if maxChars is specified) + if (maxChars !== undefined && totalCharsRead + lineToAdd.length > maxChars) { + // Add only the portion that fits within maxChars + const remainingChars = maxChars - totalCharsRead + if (remainingChars > 0) { + result += lineToAdd.substring(0, remainingChars) + } + input.destroy() + resolve(result) + return + } + + result += lineToAdd + if (maxChars !== undefined) { + totalCharsRead += lineToAdd.length + } } // Move position and increment line counter @@ -100,7 +123,15 @@ export function readLines(filepath: string, endLine?: number, startLine?: number // Process any remaining data in buffer (last line without newline) if (buffer.length > 0) { if (lineCount >= effectiveStartLine && (endLine === undefined || lineCount <= endLine)) { - result += buffer + // Check if adding this would exceed maxChars (only if maxChars is specified) + if (maxChars !== undefined && totalCharsRead + buffer.length > maxChars) { + const remainingChars = maxChars - totalCharsRead + if (remainingChars > 0) { + result += buffer.substring(0, remainingChars) + } + } else { + result += buffer + } } lineCount++ } diff --git a/src/integrations/misc/read-partial-content.ts b/src/integrations/misc/read-partial-content.ts deleted file mode 100644 index 34f3585f48..0000000000 --- a/src/integrations/misc/read-partial-content.ts +++ /dev/null @@ -1,158 +0,0 @@ -import { createReadStream } from "fs" -import * as fs from "fs/promises" -import { countFileLines } from "./line-counter" - -/** - * Result of a partial file read operation - */ -export interface PartialReadResult { - content: string - charactersRead: number - totalCharacters: number // from file stats - linesRead: number - totalLines: number // from line counter - lastLineRead: number // which line we stopped at -} - -/** - * Reads partial content from a file up to a specified character limit. - * Works for both single-line and multi-line files, tracking line numbers. - * Uses streaming to avoid loading the entire file into memory for very large files. - * - * @param filePath - Path to the file to read - * @param maxChars - Maximum number of characters to read - * @returns Promise resolving to the partial read result with metadata - */ -export async function readPartialContent(filePath: string, maxChars: number): Promise { - // Get file stats and line count - const [stats, totalLines] = await Promise.all([fs.stat(filePath), countFileLines(filePath)]) - - const totalCharacters = stats.size - - // Handle edge cases - if (maxChars <= 0 || totalCharacters === 0) { - return { - content: "", - charactersRead: 0, - totalCharacters, - linesRead: 0, - totalLines, - lastLineRead: 0, - } - } - - return new Promise((resolve, reject) => { - // Use smaller chunks and set end position to limit reading - const stream = createReadStream(filePath, { - encoding: "utf8", - highWaterMark: 16 * 1024, // Smaller 16KB chunks for better control - start: 0, - end: Math.max(0, Math.min(maxChars * 2, maxChars + 1024 * 1024)), // Read at most 2x maxChars or maxChars + 1MB buffer - }) - - let content = "" - let totalRead = 0 - let currentLine = 1 - let streamDestroyed = false - let hasContent = false - - stream.on("data", (chunk: string | Buffer) => { - // Check stream state immediately - if (streamDestroyed || stream.destroyed) { - return - } - - try { - const chunkStr = typeof chunk === "string" ? chunk : chunk.toString("utf8") - const remainingChars = maxChars - totalRead - - if (remainingChars <= 0) { - streamDestroyed = true - stream.destroy() - resolve({ - content, - charactersRead: totalRead, - totalCharacters, - linesRead: hasContent ? currentLine : 0, - totalLines, - lastLineRead: hasContent ? currentLine : 0, - }) - return - } - - let chunkToAdd: string - if (chunkStr.length <= remainingChars) { - chunkToAdd = chunkStr - totalRead += chunkStr.length - } else { - chunkToAdd = chunkStr.substring(0, remainingChars) - totalRead += remainingChars - } - - // Mark that we have content - if (chunkToAdd.length > 0) { - hasContent = true - } - - // Count newlines in the chunk we're adding - const newlineCount = (chunkToAdd.match(/\n/g) || []).length - currentLine += newlineCount - - content += chunkToAdd - - // Check if we've reached the character limit - if (totalRead >= maxChars) { - streamDestroyed = true - stream.destroy() - - // Ensure we don't exceed maxChars - if (content.length > maxChars) { - content = content.substring(0, maxChars) - // Recount lines in the final content - currentLine = 1 - hasContent = content.length > 0 - const finalNewlineCount = (content.match(/\n/g) || []).length - currentLine += finalNewlineCount - } - - resolve({ - content, - charactersRead: Math.min(totalRead, maxChars), - totalCharacters, - linesRead: hasContent ? currentLine : 0, - totalLines, - lastLineRead: hasContent ? currentLine : 0, - }) - } - } catch (error) { - streamDestroyed = true - stream.destroy() - reject(error) - } - }) - - stream.on("end", () => { - resolve({ - content, - charactersRead: totalRead, - totalCharacters, - linesRead: hasContent ? currentLine : 0, - totalLines, - lastLineRead: hasContent ? currentLine : 0, - }) - }) - - stream.on("error", (error: Error) => { - reject(error) - }) - }) -} - -/** - * Legacy function for backward compatibility. - * @deprecated Use readPartialContent instead - */ -export async function readPartialSingleLineContent(filePath: string, maxChars: number): Promise { - const result = await readPartialContent(filePath, maxChars) - return result.content -}