diff --git a/src/core/mentions/__tests__/processUserContentMentions.spec.ts b/src/core/mentions/__tests__/processUserContentMentions.spec.ts new file mode 100644 index 0000000000..3aebd66e53 --- /dev/null +++ b/src/core/mentions/__tests__/processUserContentMentions.spec.ts @@ -0,0 +1,353 @@ +// npx vitest core/mentions/__tests__/processUserContentMentions.spec.ts + +import { describe, it, expect, vi, beforeEach } from "vitest" +import { processUserContentMentions } from "../processUserContentMentions" +import { parseMentions } from "../index" +import { UrlContentFetcher } from "../../../services/browser/UrlContentFetcher" +import { FileContextTracker } from "../../context-tracking/FileContextTracker" + +// Mock the parseMentions function +vi.mock("../index", () => ({ + parseMentions: vi.fn(), +})) + +describe("processUserContentMentions", () => { + let mockUrlContentFetcher: UrlContentFetcher + let mockFileContextTracker: FileContextTracker + let mockRooIgnoreController: any + + beforeEach(() => { + vi.clearAllMocks() + + mockUrlContentFetcher = {} as UrlContentFetcher + mockFileContextTracker = {} as FileContextTracker + mockRooIgnoreController = {} + + // Default mock implementation + vi.mocked(parseMentions).mockImplementation(async (text) => `parsed: ${text}`) + }) + + describe("maxReadFileLine parameter", () => { + it("should pass maxReadFileLine to parseMentions when provided", async () => { + const userContent = [ + { + type: "text" as const, + text: "Read file with limit", + }, + ] + + await processUserContentMentions({ + userContent, + cwd: "/test", + urlContentFetcher: mockUrlContentFetcher, + fileContextTracker: mockFileContextTracker, + rooIgnoreController: mockRooIgnoreController, + maxReadFileLine: 100, + }) + + expect(parseMentions).toHaveBeenCalledWith( + "Read file with limit", + "/test", + mockUrlContentFetcher, + mockFileContextTracker, + mockRooIgnoreController, + true, + true, // includeDiagnosticMessages + 50, // maxDiagnosticMessages + 100, + ) + }) + + it("should pass undefined maxReadFileLine when not provided", async () => { + const userContent = [ + { + type: "text" as const, + text: "Read file without limit", + }, + ] + + await processUserContentMentions({ + userContent, + cwd: "/test", + urlContentFetcher: mockUrlContentFetcher, + fileContextTracker: mockFileContextTracker, + rooIgnoreController: mockRooIgnoreController, + }) + + expect(parseMentions).toHaveBeenCalledWith( + "Read file without limit", + "/test", + mockUrlContentFetcher, + mockFileContextTracker, + mockRooIgnoreController, + true, + true, // includeDiagnosticMessages + 50, // maxDiagnosticMessages + undefined, + ) + }) + + it("should handle UNLIMITED_LINES constant correctly", async () => { + const userContent = [ + { + type: "text" as const, + text: "Read unlimited lines", + }, + ] + + await processUserContentMentions({ + userContent, + cwd: "/test", + urlContentFetcher: mockUrlContentFetcher, + fileContextTracker: mockFileContextTracker, + rooIgnoreController: mockRooIgnoreController, + maxReadFileLine: -1, + }) + + expect(parseMentions).toHaveBeenCalledWith( + "Read unlimited lines", + "/test", + mockUrlContentFetcher, + mockFileContextTracker, + mockRooIgnoreController, + true, + true, // includeDiagnosticMessages + 50, // maxDiagnosticMessages + -1, + ) + }) + }) + + describe("content processing", () => { + it("should process text blocks with tags", async () => { + const userContent = [ + { + type: "text" as const, + text: "Do something", + }, + ] + + const result = await processUserContentMentions({ + userContent, + cwd: "/test", + urlContentFetcher: mockUrlContentFetcher, + fileContextTracker: mockFileContextTracker, + }) + + expect(parseMentions).toHaveBeenCalled() + expect(result[0]).toEqual({ + type: "text", + text: "parsed: Do something", + }) + }) + + it("should process text blocks with tags", async () => { + const userContent = [ + { + type: "text" as const, + text: "Fix this issue", + }, + ] + + const result = await processUserContentMentions({ + userContent, + cwd: "/test", + urlContentFetcher: mockUrlContentFetcher, + fileContextTracker: mockFileContextTracker, + }) + + expect(parseMentions).toHaveBeenCalled() + expect(result[0]).toEqual({ + type: "text", + text: "parsed: Fix this issue", + }) + }) + + it("should not process text blocks without task or feedback tags", async () => { + const userContent = [ + { + type: "text" as const, + text: "Regular text without special tags", + }, + ] + + const result = await processUserContentMentions({ + userContent, + cwd: "/test", + urlContentFetcher: mockUrlContentFetcher, + fileContextTracker: mockFileContextTracker, + }) + + expect(parseMentions).not.toHaveBeenCalled() + expect(result[0]).toEqual(userContent[0]) + }) + + it("should process tool_result blocks with string content", async () => { + const userContent = [ + { + type: "tool_result" as const, + tool_use_id: "123", + content: "Tool feedback", + }, + ] + + const result = await processUserContentMentions({ + userContent, + cwd: "/test", + urlContentFetcher: mockUrlContentFetcher, + fileContextTracker: mockFileContextTracker, + }) + + expect(parseMentions).toHaveBeenCalled() + expect(result[0]).toEqual({ + type: "tool_result", + tool_use_id: "123", + content: "parsed: Tool feedback", + }) + }) + + it("should process tool_result blocks with array content", async () => { + const userContent = [ + { + type: "tool_result" as const, + tool_use_id: "123", + content: [ + { + type: "text" as const, + text: "Array task", + }, + { + type: "text" as const, + text: "Regular text", + }, + ], + }, + ] + + const result = await processUserContentMentions({ + userContent, + cwd: "/test", + urlContentFetcher: mockUrlContentFetcher, + fileContextTracker: mockFileContextTracker, + }) + + expect(parseMentions).toHaveBeenCalledTimes(1) + expect(result[0]).toEqual({ + type: "tool_result", + tool_use_id: "123", + content: [ + { + type: "text", + text: "parsed: Array task", + }, + { + type: "text", + text: "Regular text", + }, + ], + }) + }) + + it("should handle mixed content types", async () => { + const userContent = [ + { + type: "text" as const, + text: "First task", + }, + { + type: "image" as const, + source: { + type: "base64" as const, + media_type: "image/png" as const, + data: "base64data", + }, + }, + { + type: "tool_result" as const, + tool_use_id: "456", + content: "Feedback", + }, + ] + + const result = await processUserContentMentions({ + userContent, + cwd: "/test", + urlContentFetcher: mockUrlContentFetcher, + fileContextTracker: mockFileContextTracker, + maxReadFileLine: 50, + }) + + expect(parseMentions).toHaveBeenCalledTimes(2) + expect(result).toHaveLength(3) + expect(result[0]).toEqual({ + type: "text", + text: "parsed: First task", + }) + expect(result[1]).toEqual(userContent[1]) // Image block unchanged + expect(result[2]).toEqual({ + type: "tool_result", + tool_use_id: "456", + content: "parsed: Feedback", + }) + }) + }) + + describe("showRooIgnoredFiles parameter", () => { + it("should default showRooIgnoredFiles to true", async () => { + const userContent = [ + { + type: "text" as const, + text: "Test default", + }, + ] + + await processUserContentMentions({ + userContent, + cwd: "/test", + urlContentFetcher: mockUrlContentFetcher, + fileContextTracker: mockFileContextTracker, + }) + + expect(parseMentions).toHaveBeenCalledWith( + "Test default", + "/test", + mockUrlContentFetcher, + mockFileContextTracker, + undefined, + true, // showRooIgnoredFiles should default to true + true, // includeDiagnosticMessages + 50, // maxDiagnosticMessages + undefined, + ) + }) + + it("should respect showRooIgnoredFiles when explicitly set to false", async () => { + const userContent = [ + { + type: "text" as const, + text: "Test explicit false", + }, + ] + + await processUserContentMentions({ + userContent, + cwd: "/test", + urlContentFetcher: mockUrlContentFetcher, + fileContextTracker: mockFileContextTracker, + showRooIgnoredFiles: false, + }) + + expect(parseMentions).toHaveBeenCalledWith( + "Test explicit false", + "/test", + mockUrlContentFetcher, + mockFileContextTracker, + undefined, + false, + true, // includeDiagnosticMessages + 50, // maxDiagnosticMessages + undefined, + ) + }) + }) +}) diff --git a/src/core/mentions/index.ts b/src/core/mentions/index.ts index 7ce54b984e..d0d305d096 100644 --- a/src/core/mentions/index.ts +++ b/src/core/mentions/index.ts @@ -82,6 +82,7 @@ export async function parseMentions( showRooIgnoredFiles: boolean = true, includeDiagnosticMessages: boolean = true, maxDiagnosticMessages: number = 50, + maxReadFileLine?: number, ): Promise { const mentions: Set = new Set() let parsedText = text.replace(mentionRegexGlobal, (match, mention) => { @@ -149,7 +150,13 @@ export async function parseMentions( } else if (mention.startsWith("/")) { const mentionPath = mention.slice(1) try { - const content = await getFileOrFolderContent(mentionPath, cwd, rooIgnoreController, showRooIgnoredFiles) + const content = await getFileOrFolderContent( + mentionPath, + cwd, + rooIgnoreController, + showRooIgnoredFiles, + maxReadFileLine, + ) if (mention.endsWith("/")) { parsedText += `\n\n\n${content}\n` } else { @@ -212,6 +219,7 @@ async function getFileOrFolderContent( cwd: string, rooIgnoreController?: any, showRooIgnoredFiles: boolean = true, + maxReadFileLine?: number, ): Promise { const unescapedPath = unescapeSpaces(mentionPath) const absPath = path.resolve(cwd, unescapedPath) @@ -224,7 +232,7 @@ async function getFileOrFolderContent( return `(File ${mentionPath} is ignored by .rooignore)` } try { - const content = await extractTextFromFile(absPath) + const content = await extractTextFromFile(absPath, maxReadFileLine) return content } catch (error) { return `(Failed to read contents of ${mentionPath}): ${error.message}` @@ -264,7 +272,7 @@ async function getFileOrFolderContent( if (isBinary) { return undefined } - const content = await extractTextFromFile(absoluteFilePath) + const content = await extractTextFromFile(absoluteFilePath, maxReadFileLine) return `\n${content}\n` } catch (error) { return undefined diff --git a/src/core/mentions/processUserContentMentions.ts b/src/core/mentions/processUserContentMentions.ts index 0649c4bc3c..245a25b379 100644 --- a/src/core/mentions/processUserContentMentions.ts +++ b/src/core/mentions/processUserContentMentions.ts @@ -15,6 +15,7 @@ export async function processUserContentMentions({ showRooIgnoredFiles = true, includeDiagnosticMessages = true, maxDiagnosticMessages = 50, + maxReadFileLine, }: { userContent: Anthropic.Messages.ContentBlockParam[] cwd: string @@ -24,6 +25,7 @@ export async function processUserContentMentions({ showRooIgnoredFiles?: boolean includeDiagnosticMessages?: boolean maxDiagnosticMessages?: number + maxReadFileLine?: number }) { // Process userContent array, which contains various block types: // TextBlockParam, ImageBlockParam, ToolUseBlockParam, and ToolResultBlockParam. @@ -52,6 +54,7 @@ export async function processUserContentMentions({ showRooIgnoredFiles, includeDiagnosticMessages, maxDiagnosticMessages, + maxReadFileLine, ), } } @@ -71,6 +74,7 @@ export async function processUserContentMentions({ showRooIgnoredFiles, includeDiagnosticMessages, maxDiagnosticMessages, + maxReadFileLine, ), } } @@ -91,6 +95,7 @@ export async function processUserContentMentions({ showRooIgnoredFiles, includeDiagnosticMessages, maxDiagnosticMessages, + maxReadFileLine, ), } } diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 95d12f66aa..fe8fd0f68f 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1230,6 +1230,7 @@ export class Task extends EventEmitter { showRooIgnoredFiles = true, includeDiagnosticMessages = true, maxDiagnosticMessages = 50, + maxReadFileLine = -1, } = (await this.providerRef.deref()?.getState()) ?? {} const parsedUserContent = await processUserContentMentions({ @@ -1241,6 +1242,7 @@ export class Task extends EventEmitter { showRooIgnoredFiles, includeDiagnosticMessages, maxDiagnosticMessages, + maxReadFileLine, }) const environmentDetails = await getEnvironmentDetails(this, includeFileDetails) diff --git a/src/integrations/misc/__tests__/extract-text-large-files.spec.ts b/src/integrations/misc/__tests__/extract-text-large-files.spec.ts new file mode 100644 index 0000000000..fc2f7f54b6 --- /dev/null +++ b/src/integrations/misc/__tests__/extract-text-large-files.spec.ts @@ -0,0 +1,221 @@ +// npx vitest run integrations/misc/__tests__/extract-text-large-files.spec.ts + +import { describe, it, expect, vi, beforeEach, Mock } from "vitest" +import * as fs from "fs/promises" +import { extractTextFromFile } from "../extract-text" +import { countFileLines } from "../line-counter" +import { readLines } from "../read-lines" +import { isBinaryFile } from "isbinaryfile" + +// Mock all dependencies +vi.mock("fs/promises") +vi.mock("../line-counter") +vi.mock("../read-lines") +vi.mock("isbinaryfile") + +describe("extractTextFromFile - Large File Handling", () => { + // Type the mocks + const mockedFs = vi.mocked(fs) + const mockedCountFileLines = vi.mocked(countFileLines) + const mockedReadLines = vi.mocked(readLines) + const mockedIsBinaryFile = vi.mocked(isBinaryFile) + + beforeEach(() => { + vi.clearAllMocks() + // Set default mock behavior + mockedFs.access.mockResolvedValue(undefined) + mockedIsBinaryFile.mockResolvedValue(false) + }) + + it("should truncate files that exceed maxReadFileLine limit", async () => { + const largeFileContent = Array(150) + .fill(null) + .map((_, i) => `Line ${i + 1}: This is a test line with some content`) + .join("\n") + + mockedCountFileLines.mockResolvedValue(150) + mockedReadLines.mockResolvedValue( + Array(100) + .fill(null) + .map((_, i) => `Line ${i + 1}: This is a test line with some content`) + .join("\n"), + ) + + const result = await extractTextFromFile("/test/large-file.ts", 100) + + // Should only include first 100 lines with line numbers + expect(result).toContain(" 1 | Line 1: This is a test line with some content") + expect(result).toContain("100 | Line 100: This is a test line with some content") + expect(result).not.toContain("101 | Line 101: This is a test line with some content") + + // Should include truncation message + expect(result).toContain( + "[File truncated: showing 100 of 150 total lines. The file is too large and may exhaust the context window if read in full.]", + ) + }) + + it("should not truncate files within the maxReadFileLine limit", async () => { + const smallFileContent = Array(50) + .fill(null) + .map((_, i) => `Line ${i + 1}: This is a test line`) + .join("\n") + + mockedCountFileLines.mockResolvedValue(50) + mockedFs.readFile.mockResolvedValue(smallFileContent as any) + + const result = await extractTextFromFile("/test/small-file.ts", 100) + + // Should include all lines with line numbers + expect(result).toContain(" 1 | Line 1: This is a test line") + expect(result).toContain("50 | Line 50: This is a test line") + + // Should not include truncation message + expect(result).not.toContain("[File truncated:") + }) + + it("should handle files with exactly maxReadFileLine lines", async () => { + const exactFileContent = Array(100) + .fill(null) + .map((_, i) => `Line ${i + 1}`) + .join("\n") + + mockedCountFileLines.mockResolvedValue(100) + mockedFs.readFile.mockResolvedValue(exactFileContent as any) + + const result = await extractTextFromFile("/test/exact-file.ts", 100) + + // Should include all lines with line numbers + expect(result).toContain(" 1 | Line 1") + expect(result).toContain("100 | Line 100") + + // Should not include truncation message + expect(result).not.toContain("[File truncated:") + }) + + it("should handle undefined maxReadFileLine by not truncating", async () => { + const largeFileContent = Array(200) + .fill(null) + .map((_, i) => `Line ${i + 1}`) + .join("\n") + + mockedFs.readFile.mockResolvedValue(largeFileContent as any) + + const result = await extractTextFromFile("/test/large-file.ts", undefined) + + // Should include all lines with line numbers when maxReadFileLine is undefined + expect(result).toContain(" 1 | Line 1") + expect(result).toContain("200 | Line 200") + + // Should not include truncation message + expect(result).not.toContain("[File truncated:") + }) + + it("should handle empty files", async () => { + mockedFs.readFile.mockResolvedValue("" as any) + + const result = await extractTextFromFile("/test/empty-file.ts", 100) + + expect(result).toBe("") + expect(result).not.toContain("[File truncated:") + }) + + it("should handle files with only newlines", async () => { + const newlineOnlyContent = "\n\n\n\n\n" + + mockedCountFileLines.mockResolvedValue(6) // 5 newlines = 6 lines + mockedReadLines.mockResolvedValue("\n\n") + + const result = await extractTextFromFile("/test/newline-file.ts", 3) + + // Should truncate at line 3 + expect(result).toContain("[File truncated: showing 3 of 6 total lines") + }) + + it("should handle very large files efficiently", async () => { + // Simulate a 10,000 line file + mockedCountFileLines.mockResolvedValue(10000) + mockedReadLines.mockResolvedValue( + Array(500) + .fill(null) + .map((_, i) => `Line ${i + 1}: Some content here`) + .join("\n"), + ) + + const result = await extractTextFromFile("/test/very-large-file.ts", 500) + + // Should only include first 500 lines with line numbers + expect(result).toContain(" 1 | Line 1: Some content here") + expect(result).toContain("500 | Line 500: Some content here") + expect(result).not.toContain("501 | Line 501: Some content here") + + // Should show truncation message + expect(result).toContain("[File truncated: showing 500 of 10000 total lines") + }) + + it("should handle maxReadFileLine of 0 by throwing an error", 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.", + ) + }) + + it("should handle negative maxReadFileLine by treating as undefined", async () => { + const fileContent = "Line 1\nLine 2\nLine 3" + + mockedFs.readFile.mockResolvedValue(fileContent as any) + + const result = await extractTextFromFile("/test/file.ts", -1) + + // Should include all content with line numbers when negative + 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 preserve file content structure when truncating", async () => { + const structuredContent = [ + "function example() {", + " const x = 1;", + " const y = 2;", + " return x + y;", + "}", + "", + "// More code below", + ].join("\n") + + mockedCountFileLines.mockResolvedValue(7) + mockedReadLines.mockResolvedValue(["function example() {", " const x = 1;", " const y = 2;"].join("\n")) + + const result = await extractTextFromFile("/test/structured.ts", 3) + + // Should preserve the first 3 lines with line numbers + expect(result).toContain("1 | function example() {") + expect(result).toContain("2 | const x = 1;") + expect(result).toContain("3 | const y = 2;") + expect(result).not.toContain("4 | return x + y;") + + // Should include truncation info + expect(result).toContain("[File truncated: showing 3 of 7 total lines") + }) + + it("should handle binary files by throwing an error", async () => { + mockedIsBinaryFile.mockResolvedValue(true) + + await expect(extractTextFromFile("/test/binary.bin", 100)).rejects.toThrow( + "Cannot read text for file type: .bin", + ) + }) + + it("should handle file not found errors", async () => { + mockedFs.access.mockRejectedValue(new Error("ENOENT")) + + await expect(extractTextFromFile("/test/nonexistent.ts", 100)).rejects.toThrow( + "File not found: /test/nonexistent.ts", + ) + }) +}) diff --git a/src/integrations/misc/extract-text.ts b/src/integrations/misc/extract-text.ts index eb02f63b95..8231c609be 100644 --- a/src/integrations/misc/extract-text.ts +++ b/src/integrations/misc/extract-text.ts @@ -5,6 +5,8 @@ import mammoth from "mammoth" import fs from "fs/promises" import { isBinaryFile } from "isbinaryfile" import { extractTextFromXLSX } from "./extract-text-from-xlsx" +import { countFileLines } from "./line-counter" +import { readLines } from "./read-lines" async function extractTextFromPDF(filePath: string): Promise { const dataBuffer = await fs.readFile(filePath) @@ -48,7 +50,27 @@ export function getSupportedBinaryFormats(): string[] { return Object.keys(SUPPORTED_BINARY_FORMATS) } -export async function extractTextFromFile(filePath: string): Promise { +/** + * Extracts text content from a file, with support for various formats including PDF, DOCX, XLSX, and plain text. + * For large text files, can limit the number of lines read to prevent context exhaustion. + * + * @param filePath - Path to the file to extract text from + * @param maxReadFileLine - Maximum number of lines to read from text files. + * Use UNLIMITED_LINES (-1) or undefined for no limit. + * Must be a positive integer or UNLIMITED_LINES. + * @returns Promise resolving to the extracted text content with line numbers + * @throws {Error} If file not found, unsupported format, or invalid parameters + */ +export async function extractTextFromFile(filePath: string, maxReadFileLine?: number): Promise { + // Validate maxReadFileLine parameter + if (maxReadFileLine !== undefined && maxReadFileLine !== -1) { + if (!Number.isInteger(maxReadFileLine) || maxReadFileLine < 1) { + throw new Error( + `Invalid maxReadFileLine: ${maxReadFileLine}. Must be a positive integer or -1 for unlimited.`, + ) + } + } + try { await fs.access(filePath) } catch (error) { @@ -67,6 +89,20 @@ export async function extractTextFromFile(filePath: string): Promise { const isBinary = await isBinaryFile(filePath).catch(() => false) if (!isBinary) { + // Check if we need to apply line limit + if (maxReadFileLine !== undefined && maxReadFileLine !== -1) { + const totalLines = await countFileLines(filePath) + if (totalLines > maxReadFileLine) { + // Read only up to maxReadFileLine (endLine is 0-based and inclusive) + const content = await readLines(filePath, maxReadFileLine - 1, 0) + const numberedContent = addLineNumbers(content) + return ( + numberedContent + + `\n\n[File truncated: showing ${maxReadFileLine} of ${totalLines} total lines. The file is too large and may exhaust the context window if read in full.]` + ) + } + } + // Read the entire file if no limit or file is within limit return addLineNumbers(await fs.readFile(filePath, "utf8")) } else { throw new Error(`Cannot read text for file type: ${fileExtension}`)