diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index e4b15aa27e..e01a6f6338 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -1184,11 +1184,6 @@ "count": 1 } }, - "integrations/misc/__tests__/extract-text.spec.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 4 - } - }, "integrations/misc/__tests__/line-counter.spec.ts": { "@typescript-eslint/no-explicit-any": { "count": 2 diff --git a/src/integrations/misc/__tests__/extract-text-formats.integration.test.ts b/src/integrations/misc/__tests__/extract-text-formats.integration.test.ts new file mode 100644 index 0000000000..105c81a75a --- /dev/null +++ b/src/integrations/misc/__tests__/extract-text-formats.integration.test.ts @@ -0,0 +1,49 @@ +import fs from "fs/promises" +import path from "path" +import { createHash } from "node:crypto" +import { extractTextFromBuffer } from "../extract-text" + +// Keep real parser fixtures separate from the mocked parser unit suite. +describe("real document buffer decoding", () => { + afterEach(() => vi.restoreAllMocks()) + + it.each([ + { + filename: "approved.pdf", + sha256: "11b19aed8bc2e40b9eda9d18b7bfe36e0bbc41c4383d7e30141927992790f411", + }, + { + filename: "approved.docx", + sha256: "732d827ad38ff51c24983c773bcb964ba53ca29bb055ea73aa9fbed820ae3a9b", + }, + ])("matches the documented SHA-256 for $filename", async ({ filename, sha256 }) => { + const source = await fs.readFile(path.join(__dirname, "fixtures", filename)) + expect(createHash("sha256").update(source).digest("hex")).toBe(sha256) + }) + + it("extracts numbered PDF text from fixture bytes without reopening the metadata pathname", async () => { + const source = await fs.readFile(path.join(__dirname, "fixtures", "approved.pdf")) + const access = vi.spyOn(fs, "access").mockRejectedValue(new Error("Unexpected pathname access")) + const readFile = vi.spyOn(fs, "readFile").mockRejectedValue(new Error("Unexpected pathname read")) + + // pdf-parse prefixes each page with two newlines, which retain their line numbers. + expect(await extractTextFromBuffer(source, "metadata-only.PDF")).toBe( + "1 | \n2 | \n3 | Approved PDF first line\n4 | Approved PDF second line\n", + ) + expect(access).not.toHaveBeenCalled() + expect(readFile).not.toHaveBeenCalled() + }) + + it("extracts numbered DOCX paragraphs from fixture bytes without reopening the metadata pathname", async () => { + const source = await fs.readFile(path.join(__dirname, "fixtures", "approved.docx")) + const access = vi.spyOn(fs, "access").mockRejectedValue(new Error("Unexpected pathname access")) + const readFile = vi.spyOn(fs, "readFile").mockRejectedValue(new Error("Unexpected pathname read")) + + // Mammoth separates paragraphs with two newlines and decodes XML entities across runs. + expect(await extractTextFromBuffer(source, "metadata-only.DOCX")).toBe( + "1 | Approved DOCX first paragraph\n2 | \n3 | Approved DOCX second & final paragraph\n4 | \n", + ) + expect(access).not.toHaveBeenCalled() + expect(readFile).not.toHaveBeenCalled() + }) +}) diff --git a/src/integrations/misc/__tests__/extract-text-from-xlsx.test.ts b/src/integrations/misc/__tests__/extract-text-from-xlsx.test.ts index a3c46e30b7..b2f553d31c 100644 --- a/src/integrations/misc/__tests__/extract-text-from-xlsx.test.ts +++ b/src/integrations/misc/__tests__/extract-text-from-xlsx.test.ts @@ -1,9 +1,50 @@ import ExcelJS from "exceljs" -import { extractTextFromXLSX } from "../extract-text-from-xlsx" +import { extractTextFromXLSX, formatWorkbook } from "../extract-text-from-xlsx" +import { extractTextFromFile } from "../extract-text" + +describe("XLSX buffer decoding", () => { + it("preserves empty columns in a populated row from real XLSX bytes", async () => { + const workbook = new ExcelJS.Workbook() + const worksheet = workbook.addWorksheet("Sparse") + worksheet.getCell("A1").value = null + worksheet.getCell("B1").value = "middle" + worksheet.getCell("C1").value = undefined + worksheet.getCell("D1").value = "end" + + const expected = "--- Sheet: Sparse ---\n\tmiddle\t\tend" + expect(formatWorkbook(workbook)).toBe(expected) + const source = Buffer.from(await workbook.xlsx.writeBuffer()) + expect(await extractTextFromXLSX(source)).toBe(expected) + }) + + it("decodes only the selected buffer view when another workbook follows in the backing buffer", async () => { + const approved = new ExcelJS.Workbook() + const worksheet = approved.addWorksheet("Selected") + worksheet.getCell("A1").value = "approved cell" + worksheet.getCell("B1").value = 42 + const approvedBytes = Buffer.from(await approved.xlsx.writeBuffer()) + + const unapproved = new ExcelJS.Workbook() + unapproved.addWorksheet("Unapproved").getCell("A1").value = "outside selected bytes" + const unapprovedBytes = Buffer.from(await unapproved.xlsx.writeBuffer()) + const prefix = Buffer.from("unapproved prefix") + const backing = Buffer.concat([prefix, approvedBytes, unapprovedBytes]) + const source = backing.subarray(prefix.length, prefix.length + approvedBytes.length) + + // Passing source.buffer would expose the later, unapproved ZIP archive to the decoder. + expect(source.byteOffset).toBeGreaterThan(0) + expect(source.byteLength).toBeLessThan(source.buffer.byteLength) + expect(await extractTextFromXLSX(source)).toBe("--- Sheet: Selected ---\napproved cell\t42") + }) -describe("extractTextFromXLSX", () => { + it("rejects invalid workbook bytes instead of returning empty workbook text", async () => { + await expect(extractTextFromXLSX(Buffer.from("not an XLSX archive"))).rejects.toThrow() + }) +}) + +describe("formatWorkbook", () => { describe("basic functionality", () => { - it("should extract text with proper formatting", async () => { + it("should extract text with proper formatting", () => { const workbook = new ExcelJS.Workbook() const worksheet = workbook.addWorksheet("Sheet1") @@ -12,14 +53,14 @@ describe("extractTextFromXLSX", () => { worksheet.getCell("A2").value = "Test" worksheet.getCell("B2").value = 123 - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("--- Sheet: Sheet1 ---") expect(result).toContain("Hello\tWorld") expect(result).toContain("Test\t123") }) - it("should skip rows with no content", async () => { + it("should skip rows with no content", () => { const workbook = new ExcelJS.Workbook() const worksheet = workbook.addWorksheet("Sheet1") @@ -27,7 +68,7 @@ describe("extractTextFromXLSX", () => { // Row 2 is completely empty worksheet.getCell("A3").value = "Row 3" - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("Row 1") expect(result).toContain("Row 3") @@ -37,7 +78,7 @@ describe("extractTextFromXLSX", () => { }) describe("sheet handling", () => { - it("should process multiple sheets", async () => { + it("should process multiple sheets", () => { const workbook = new ExcelJS.Workbook() const sheet1 = workbook.addWorksheet("First Sheet") @@ -46,7 +87,7 @@ describe("extractTextFromXLSX", () => { const sheet2 = workbook.addWorksheet("Second Sheet") sheet2.getCell("A1").value = "Sheet 2 Data" - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("--- Sheet: First Sheet ---") expect(result).toContain("Sheet 1 Data") @@ -54,7 +95,7 @@ describe("extractTextFromXLSX", () => { expect(result).toContain("Sheet 2 Data") }) - it("should skip hidden sheets", async () => { + it("should skip hidden sheets", () => { const workbook = new ExcelJS.Workbook() const visibleSheet = workbook.addWorksheet("Visible Sheet") @@ -64,7 +105,7 @@ describe("extractTextFromXLSX", () => { hiddenSheet.getCell("A1").value = "Hidden Data" hiddenSheet.state = "hidden" - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("--- Sheet: Visible Sheet ---") expect(result).toContain("Visible Data") @@ -72,7 +113,7 @@ describe("extractTextFromXLSX", () => { expect(result).not.toContain("Hidden Data") }) - it("should skip very hidden sheets", async () => { + it("should skip very hidden sheets", () => { const workbook = new ExcelJS.Workbook() const visibleSheet = workbook.addWorksheet("Visible Sheet") @@ -82,7 +123,7 @@ describe("extractTextFromXLSX", () => { veryHiddenSheet.getCell("A1").value = "Very Hidden Data" veryHiddenSheet.state = "veryHidden" - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("--- Sheet: Visible Sheet ---") expect(result).toContain("Visible Data") @@ -92,7 +133,7 @@ describe("extractTextFromXLSX", () => { }) describe("formatCellValue logic", () => { - it("should handle null and undefined values", async () => { + it("should handle null and undefined values", () => { const workbook = new ExcelJS.Workbook() const worksheet = workbook.addWorksheet("Sheet1") @@ -101,7 +142,7 @@ describe("extractTextFromXLSX", () => { worksheet.getCell("A3").value = undefined worksheet.getCell("A4").value = "After" - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("Before") expect(result).toContain("After") @@ -111,30 +152,30 @@ describe("extractTextFromXLSX", () => { expect(dataLines).toHaveLength(2) // Only 'Before' and 'After' should create content }) - it("should format dates correctly", async () => { + it("should format dates correctly", () => { const workbook = new ExcelJS.Workbook() const worksheet = workbook.addWorksheet("Sheet1") const testDate = new Date("2023-12-25") worksheet.getCell("A1").value = testDate - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("2023-12-25") }) - it("should handle error values", async () => { + it("should handle error values", () => { const workbook = new ExcelJS.Workbook() const worksheet = workbook.addWorksheet("Sheet1") worksheet.getCell("A1").value = { error: "#DIV/0!" } - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("[Error: #DIV/0!]") }) - it("should handle rich text", async () => { + it("should handle rich text", () => { const workbook = new ExcelJS.Workbook() const worksheet = workbook.addWorksheet("Sheet1") @@ -142,12 +183,12 @@ describe("extractTextFromXLSX", () => { richText: [{ text: "Hello " }, { text: "World", font: { bold: true } }], } - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("Hello World") }) - it("should handle hyperlinks", async () => { + it("should handle hyperlinks", () => { const workbook = new ExcelJS.Workbook() const worksheet = workbook.addWorksheet("Sheet1") @@ -156,19 +197,19 @@ describe("extractTextFromXLSX", () => { hyperlink: "https://roocode.com/", } - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("Roo Code (https://roocode.com/)") }) - it("should handle formulas with and without results", async () => { + it("should handle formulas with and without results", () => { const workbook = new ExcelJS.Workbook() const worksheet = workbook.addWorksheet("Sheet1") worksheet.getCell("A1").value = { formula: "A2+A3", result: 30 } worksheet.getCell("A2").value = { formula: "SUM(B1:B10)" } - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("30") // Formula with result expect(result).toContain("[Formula: SUM(B1:B10)]") // Formula without result @@ -176,17 +217,30 @@ describe("extractTextFromXLSX", () => { }) describe("edge cases", () => { - it("should handle empty workbook", async () => { + it("includes row 50000, marks row 50001 as truncated, and still formats the next sheet", () => { + const workbook = new ExcelJS.Workbook() + const worksheet = workbook.addWorksheet("Large") + worksheet.getCell("A50000").value = "last included row" + worksheet.getCell("A50001").value = "excluded row content" + workbook.addWorksheet("Next").getCell("A1").value = "next sheet content" + + expect(formatWorkbook(workbook)).toBe( + "--- Sheet: Large ---\nlast included row\n[... truncated at row 50001 ...]\n\n" + + "--- Sheet: Next ---\nnext sheet content", + ) + }) + + it("should handle empty workbook", () => { const workbook = new ExcelJS.Workbook() workbook.addWorksheet("Empty Sheet") - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("--- Sheet: Empty Sheet ---") expect(result.trim()).toBe("--- Sheet: Empty Sheet ---") }) - it("should handle workbook with only empty cells", async () => { + it("should handle workbook with only empty cells", () => { const workbook = new ExcelJS.Workbook() const worksheet = workbook.addWorksheet("Sheet1") @@ -194,7 +248,7 @@ describe("extractTextFromXLSX", () => { worksheet.getCell("A1").value = "" worksheet.getCell("B1").value = "" - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("--- Sheet: Sheet1 ---") // Should not contain any data rows since empty strings don't count as content @@ -203,19 +257,19 @@ describe("extractTextFromXLSX", () => { }) }) - describe("function overloads", () => { - it("should work with workbook objects", async () => { + describe("workbook formatting and path adapter", () => { + it("should work with workbook objects", () => { const workbook = new ExcelJS.Workbook() const worksheet = workbook.addWorksheet("Test") worksheet.getCell("A1").value = "Test Data" - const result = await extractTextFromXLSX(workbook) + const result = formatWorkbook(workbook) expect(result).toContain("Test Data") }) it("should reject invalid file paths", async () => { - await expect(extractTextFromXLSX("/non/existent/file.xlsx")).rejects.toThrow() + await expect(extractTextFromFile("/non/existent/file.xlsx")).rejects.toThrow() }) }) }) diff --git a/src/integrations/misc/__tests__/extract-text.spec.ts b/src/integrations/misc/__tests__/extract-text.spec.ts index bb4b52fe93..1f6d2b61ad 100644 --- a/src/integrations/misc/__tests__/extract-text.spec.ts +++ b/src/integrations/misc/__tests__/extract-text.spec.ts @@ -1,5 +1,16 @@ +import fs from "fs/promises" +import os from "os" +import path from "path" +import * as binaryFile from "isbinaryfile" +import ExcelJS from "exceljs" +import mammoth from "mammoth" +import { DEFAULT_LINE_LIMIT } from "../../../core/prompts/tools/native-tools/read_file" import { addLineNumbers, + extractTextFromBuffer, + extractTextFromFile, + extractTextFromFileWithMetadata, + getSupportedBinaryFormats, everyLineHasLineNumbers, stripLineNumbers, truncateOutput, @@ -8,6 +19,290 @@ import { processBackspaces, } from "../extract-text" +const { parsePdf } = vi.hoisted(() => ({ + parsePdf: vi.fn<(source: Buffer) => Promise<{ text: string }>>(), +})) + +vi.mock("pdf-parse/lib/pdf-parse", () => ({ default: parsePdf })) + +describe("document decoders", () => { + afterEach(() => { + vi.restoreAllMocks() + parsePdf.mockReset() + }) + + it("lists supported document extensions without exposing the decoder registry", () => { + const formats = getSupportedBinaryFormats() + expect(formats).toEqual([".pdf", ".docx", ".ipynb", ".xlsx"]) + formats.pop() + expect(getSupportedBinaryFormats()).toEqual([".pdf", ".docx", ".ipynb", ".xlsx"]) + }) + + it.each([ + ["first page\nsecond page\n", "1 | first page\n2 | second page\n"], + ["", ""], + ])("formats PDF parser text %j from the exact supplied bytes", async (text, expected) => { + const source = Buffer.from("%PDF-1.7 approved document bytes") + parsePdf.mockResolvedValue({ text }) + const access = vi.spyOn(fs, "access").mockRejectedValue(new Error("Unexpected pathname access")) + const readFile = vi.spyOn(fs, "readFile").mockRejectedValue(new Error("Unexpected pathname read")) + + expect(await extractTextFromBuffer(source, "missing.PDF")).toBe(expected) + expect(parsePdf).toHaveBeenCalledExactlyOnceWith(source) + expect(parsePdf.mock.calls[0][0]).toBe(source) + expect(access).not.toHaveBeenCalled() + expect(readFile).not.toHaveBeenCalled() + }) + + it.each([ + ["first paragraph\n\nsecond paragraph\n", "1 | first paragraph\n2 | \n3 | second paragraph\n"], + ["", ""], + ])("formats DOCX parser text %j from the exact supplied bytes", async (value, expected) => { + const source = Buffer.from("PK approved document bytes") + const parser = vi.spyOn(mammoth, "extractRawText").mockResolvedValue({ value, messages: [] }) + const access = vi.spyOn(fs, "access").mockRejectedValue(new Error("Unexpected pathname access")) + const readFile = vi.spyOn(fs, "readFile").mockRejectedValue(new Error("Unexpected pathname read")) + + expect(await extractTextFromBuffer(source, "missing.DOCX")).toBe(expected) + expect(parser).toHaveBeenCalledExactlyOnceWith({ buffer: source }) + const input = parser.mock.calls[0][0] + expect("buffer" in input && input.buffer).toBe(source) + expect(access).not.toHaveBeenCalled() + expect(readFile).not.toHaveBeenCalled() + }) + + it("propagates PDF parser failures instead of treating document bytes as text", async () => { + const source = Buffer.from("%PDF corrupted document") + const failure = new Error("Invalid PDF document") + parsePdf.mockRejectedValue(failure) + const probe = vi.spyOn(binaryFile, "isBinaryFile") + + await expect(extractTextFromBuffer(source, "corrupted.pdf")).rejects.toBe(failure) + expect(parsePdf).toHaveBeenCalledExactlyOnceWith(source) + expect(probe).not.toHaveBeenCalled() + }) + + it("propagates DOCX parser failures instead of treating document bytes as text", async () => { + const source = Buffer.from("PK corrupted document") + const failure = new Error("Invalid DOCX archive") + const parser = vi.spyOn(mammoth, "extractRawText").mockRejectedValue(failure) + const probe = vi.spyOn(binaryFile, "isBinaryFile") + + await expect(extractTextFromBuffer(source, "corrupted.docx")).rejects.toBe(failure) + expect(parser).toHaveBeenCalledExactlyOnceWith({ buffer: source }) + expect(probe).not.toHaveBeenCalled() + }) +}) + +describe("buffer input", () => { + afterEach(() => vi.restoreAllMocks()) + + it("returns supplied ordinary-text bytes without pathname I/O", async () => { + const access = vi.spyOn(fs, "access").mockResolvedValue(undefined) + const readFile = vi.spyOn(fs, "readFile").mockResolvedValue("unapproved disk content") + const probe = vi.spyOn(binaryFile, "isBinaryFile").mockResolvedValue(false) + const source = Buffer.from("approved first line\napproved second line") + + const content = await extractTextFromBuffer(source, "metadata-only.txt") + + expect(content).toBe("1 | approved first line\n2 | approved second line") + expect(access).not.toHaveBeenCalled() + expect(readFile).not.toHaveBeenCalled() + expect(probe).not.toHaveBeenCalledWith("metadata-only.txt") + }) + + it("preserves empty-text formatting for an empty supplied buffer without pathname I/O", async () => { + const access = vi.spyOn(fs, "access").mockRejectedValue(new Error("Unexpected pathname access")) + const readFile = vi.spyOn(fs, "readFile").mockRejectedValue(new Error("Unexpected pathname read")) + const probe = vi.spyOn(binaryFile, "isBinaryFile") + const source = Buffer.alloc(0) + + const content = await extractTextFromBuffer(source, "missing.txt") + + expect(content).toBe("1 | ") + expect(access).not.toHaveBeenCalled() + expect(readFile).not.toHaveBeenCalled() + expect(probe).not.toHaveBeenCalledWith("missing.txt") + }) + + it("rejects unsupported binary bytes without pathname I/O", async () => { + const access = vi.spyOn(fs, "access").mockRejectedValue(new Error("Unexpected pathname access")) + const readFile = vi.spyOn(fs, "readFile").mockRejectedValue(new Error("Unexpected pathname read")) + const probe = vi.spyOn(binaryFile, "isBinaryFile") + const source = Buffer.from([0, 1, 2, 0, 255]) + + await expect(extractTextFromBuffer(source, "missing.BIN")).rejects.toThrow( + "Cannot read text for file type: .bin", + ) + + expect(access).not.toHaveBeenCalled() + expect(readFile).not.toHaveBeenCalled() + expect(probe).not.toHaveBeenCalledWith("missing.BIN") + }) + + it("falls back to supplied text bytes when binary detection fails", async () => { + const source = Buffer.from("approved text\nsecond line") + const probe = vi.spyOn(binaryFile, "isBinaryFile").mockRejectedValue(new Error("Detection failed")) + const access = vi.spyOn(fs, "access").mockRejectedValue(new Error("Unexpected pathname access")) + const readFile = vi.spyOn(fs, "readFile").mockRejectedValue(new Error("Unexpected pathname read")) + + expect(await extractTextFromBuffer(source, "missing.txt")).toBe("1 | approved text\n2 | second line") + expect(probe).toHaveBeenCalledExactlyOnceWith(source) + expect(access).not.toHaveBeenCalled() + expect(readFile).not.toHaveBeenCalled() + }) + + it("decodes notebook bytes with historical line numbering and trailing newline", async () => { + const access = vi.spyOn(fs, "access").mockRejectedValue(new Error("Unexpected pathname access")) + const readFile = vi.spyOn(fs, "readFile").mockRejectedValue(new Error("Unexpected pathname read")) + const source = Buffer.from( + JSON.stringify({ + cells: [ + { cell_type: "markdown", source: ["Heading"] }, + { cell_type: "code", source: ["first", "second"] }, + { cell_type: "raw", source: ["ignored"] }, + ], + }), + ) + + expect(await extractTextFromBuffer(source, "missing.IPYNB")).toBe("1 | Heading\n2 | first\n3 | second\n") + expect(access).not.toHaveBeenCalled() + expect(readFile).not.toHaveBeenCalled() + }) + + it("preserves empty notebook output without pathname I/O", async () => { + const access = vi.spyOn(fs, "access").mockRejectedValue(new Error("Unexpected pathname access")) + const readFile = vi.spyOn(fs, "readFile").mockRejectedValue(new Error("Unexpected pathname read")) + const source = Buffer.from(JSON.stringify({ cells: [] })) + + expect(await extractTextFromBuffer(source, "missing.ipynb")).toBe("") + expect(access).not.toHaveBeenCalled() + expect(readFile).not.toHaveBeenCalled() + }) + + it("decodes XLSX bytes and preserves visible-sheet formatting without pathname I/O", async () => { + const workbook = new ExcelJS.Workbook() + workbook.addWorksheet("Data").addRow(["approved cell", 42]) + const hidden = workbook.addWorksheet("Hidden", { state: "hidden" }) + hidden.addRow(["hidden cell"]) + const source = Buffer.from(await workbook.xlsx.writeBuffer()) + const access = vi.spyOn(fs, "access").mockRejectedValue(new Error("Unexpected pathname access")) + const readFile = vi.spyOn(fs, "readFile").mockRejectedValue(new Error("Unexpected pathname read")) + + expect(await extractTextFromBuffer(source, "missing.XLSX")).toBe("--- Sheet: Data ---\napproved cell\t42") + expect(access).not.toHaveBeenCalled() + expect(readFile).not.toHaveBeenCalled() + }) +}) + +describe("path input", () => { + afterEach(() => vi.restoreAllMocks()) + + it("rejects unsupported binary paths after probing without loading the whole file", async () => { + vi.spyOn(fs, "access").mockResolvedValue(undefined) + const readFile = vi.spyOn(fs, "readFile").mockRejectedValue(new Error("Unexpected whole-file read")) + const probe = vi.spyOn(binaryFile, "isBinaryFile").mockResolvedValue(true) + + await expect(extractTextFromFileWithMetadata("large.bin")).rejects.toThrow( + "Cannot read text for file type: .bin", + ) + + expect(probe).toHaveBeenCalledWith("large.bin") + expect(readFile).not.toHaveBeenCalled() + }) + + it("loads document bytes from the path and preserves document metadata", async () => { + vi.spyOn(fs, "access").mockResolvedValue(undefined) + const source = Buffer.from(JSON.stringify({ cells: [{ cell_type: "code", source: ["first", "second"] }] })) + const readFile = vi.spyOn(fs, "readFile").mockResolvedValue(source) + + expect(await extractTextFromFileWithMetadata("document.ipynb", 1)).toEqual({ + content: "1 | first\n2 | second\n", + totalLines: 3, + returnedLines: 3, + wasTruncated: false, + }) + expect(readFile).toHaveBeenCalledWith("document.ipynb") + }) +}) + +describe("text path adapters", () => { + let directory: string + + beforeEach(async () => { + directory = await fs.mkdtemp(path.join(os.tmpdir(), "extract-text-")) + }) + + afterEach(async () => { + vi.restoreAllMocks() + await fs.rm(directory, { recursive: true, force: true }) + }) + + it("returns truncation metadata and the actual included range for a text file", async () => { + const filePath = path.join(directory, "notes.txt") + await fs.writeFile(filePath, "first\nsecond\nthird") + + expect(await extractTextFromFileWithMetadata(filePath, 2)).toEqual({ + content: "1 | first\n2 | second", + totalLines: 3, + returnedLines: 2, + wasTruncated: true, + linesShown: [1, 2], + }) + }) + + it("preserves the single blank line and its metadata for an empty text file", async () => { + const filePath = path.join(directory, "empty.txt") + await fs.writeFile(filePath, "") + + expect(await extractTextFromFileWithMetadata(filePath)).toEqual({ + content: "1 | ", + totalLines: 1, + returnedLines: 1, + wasTruncated: false, + linesShown: [1, 1], + }) + }) + + it("keeps the legacy content-only path adapter and default line limit", async () => { + const filePath = path.join(directory, "large.txt") + const source = Buffer.from(Array.from({ length: DEFAULT_LINE_LIMIT + 1 }, (_, i) => `line ${i + 1}`).join("\n")) + await fs.writeFile(filePath, source) + + const content = await extractTextFromFile(filePath) + + expect(content.split("\n")).toHaveLength(DEFAULT_LINE_LIMIT) + expect(content).toContain(`${DEFAULT_LINE_LIMIT} | line ${DEFAULT_LINE_LIMIT}`) + expect(content).not.toContain(`line ${DEFAULT_LINE_LIMIT + 1}`) + expect(await extractTextFromBuffer(source, "metadata-only.txt")).toBe(content) + }) + + it("continues reading text when pathname binary detection fails", async () => { + const filePath = path.join(directory, "notes.txt") + await fs.writeFile(filePath, "first\nsecond") + const probe = vi.spyOn(binaryFile, "isBinaryFile").mockRejectedValue(new Error("Detection failed")) + + expect(await extractTextFromFileWithMetadata(filePath)).toEqual({ + content: "1 | first\n2 | second", + totalLines: 2, + returnedLines: 2, + wasTruncated: false, + linesShown: [1, 2], + }) + expect(probe).toHaveBeenCalledExactlyOnceWith(filePath) + }) + + it("reports a missing text path before probing or loading content", async () => { + const filePath = path.join(directory, "missing.txt") + const probe = vi.spyOn(binaryFile, "isBinaryFile") + const readFile = vi.spyOn(fs, "readFile") + + await expect(extractTextFromFileWithMetadata(filePath)).rejects.toThrow(`File not found: ${filePath}`) + expect(probe).not.toHaveBeenCalled() + expect(readFile).not.toHaveBeenCalled() + }) +}) + describe("addLineNumbers", () => { it("should add line numbers starting from 1 by default", () => { const input = "line 1\nline 2\nline 3" @@ -134,6 +429,19 @@ describe("stripLineNumbers", () => { expect(stripLineNumbers(input)).toBe(expected) }) + it.each([false, true])("preserves trailing LF and CRLF including blank lines (aggressive=%s)", (aggressive) => { + for (const newline of ["\n", "\r\n"]) { + expect(stripLineNumbers(`1 | first${newline}2 | ${newline}${newline}`, aggressive)).toBe( + `first${newline}${newline}${newline}`, + ) + expect(stripLineNumbers(newline, aggressive)).toBe(newline) + } + }) + + it("normalizes mixed line endings to CRLF while preserving the trailing blank line", () => { + expect(stripLineNumbers("1 | first\n2 | second\r\n")).toBe("first\r\nsecond\r\n") + }) + it("should handle content with varying line number widths", () => { const input = " 1 | line one\n 10 | line two\n100 | line three" const expected = "line one\nline two\nline three" @@ -502,8 +810,33 @@ describe("truncateOutput", () => { describe("applyRunLengthEncoding", () => { it("should handle empty input", () => { expect(applyRunLengthEncoding("")).toBe("") - expect(applyRunLengthEncoding(null as any)).toBe(null as any) - expect(applyRunLengthEncoding(undefined as any)).toBe(undefined as any) + }) + + it.each(["single line", "single line\n", "first\nsecond\nlast", "first\nsecond\nlast\n"])( + "preserves distinct lines and their final newline in %j", + (input) => { + expect(applyRunLengthEncoding(input)).toBe(input) + }, + ) + + it("flushes separate compressible runs without carrying their repeat counts across a divider", () => { + const first = "first long repeated log entry with enough text to compress\n" + const second = "second long repeated log entry with enough text to compress\n" + const input = first.repeat(3) + "divider\n" + second.repeat(4) + "last line" + + expect(applyRunLengthEncoding(input)).toBe( + first + + "\n" + + "divider\n" + + second + + "\n" + + "last line", + ) + }) + + it("retains a short repeated run before a different final line when compression would expand it", () => { + const input = "x\nx\nx\nfinal line" + expect(applyRunLengthEncoding(input)).toBe(input) }) it("should compress repeated single lines when beneficial", () => { @@ -524,6 +857,23 @@ describe("processCarriageReturns", () => { expect(processCarriageReturns(input)).toBe(input) }) + it("handles a terminal chunk ending with a high surrogate during a partial overwrite", () => { + // An emoji split across chunks can leave a high surrogate at the end of the replacement segment. + expect(processCarriageReturns("abcdef\r\ud83d")).toBe("\ud83d cdef") + }) + + it("preserves a complete emoji replacement and the remaining suffix", () => { + expect(processCarriageReturns("abcdef\r🚀")).toBe("🚀cdef") + }) + + it("detects a surrogate pair beginning at the overwrite boundary", () => { + expect(processCarriageReturns("a🚀tail\rb")).toBe("b \ude80tail") + }) + + it("does not treat private-use BMP characters at the overwrite boundary as surrogates", () => { + expect(processCarriageReturns("a\ue000tail\rb")).toBe("b\ue000tail") + }) + it("should process basic progress bar with carriage returns (\r)", () => { const input = "Progress: [===>---------] 30%\rProgress: [======>------] 60%\rProgress: [==========>] 100%" const expected = "Progress: [==========>] 100%%" diff --git a/src/integrations/misc/__tests__/fixtures/.gitattributes b/src/integrations/misc/__tests__/fixtures/.gitattributes new file mode 100644 index 0000000000..d5a03a01bc --- /dev/null +++ b/src/integrations/misc/__tests__/fixtures/.gitattributes @@ -0,0 +1,2 @@ +*.pdf binary +*.docx binary diff --git a/src/integrations/misc/__tests__/fixtures/FIXTURES.md b/src/integrations/misc/__tests__/fixtures/FIXTURES.md new file mode 100644 index 0000000000..a251ede1a6 --- /dev/null +++ b/src/integrations/misc/__tests__/fixtures/FIXTURES.md @@ -0,0 +1,27 @@ +# Document extraction fixtures + +These small, owned documents exercise the real parsers through the buffer-only extraction API. They contain no external resources and require no additional dependencies. + +- [approved.pdf](approved.pdf) is a one-page PDF 1.4 containing two Helvetica text lines: “Approved PDF first line” and “Approved PDF second line”. It was generated using Python's standard library with five objects (Catalog, Pages, Page, Font, Contents), byte-accurate stream length, cross-reference offsets, and trailer. The default PDF parser prefixes the page text with two newlines; the integration test preserves that historical numbering. +- [approved.docx](approved.docx) is a minimal OOXML document generated using Python's standard-library ZIP writer, fixed entry timestamps, and DEFLATE compression. It includes the content-type declarations, root office-document relationship, and main document part. Its two paragraphs read “Approved DOCX first paragraph” and “Approved DOCX second & final paragraph”; the second combines two runs and an escaped XML entity. Mammoth separates paragraphs with two newlines. + +The integration suite deliberately does not mock either document parser. It reads each fixture before guarding against pathname I/O, then passes a different, metadata-only pathname to the extraction API and asserts the full numbered output. + +## Regeneration and integrity + +[`generate-fixtures.py`](generate-fixtures.py) contains the complete document sources and prints each generated file's SHA-256 hash. Run it from the repository root: + +```sh +python3 src/integrations/misc/__tests__/fixtures/generate-fixtures.py +``` + +Without an argument it writes beside the script, regardless of the current working directory. Supply an output directory as the optional argument to regenerate elsewhere for comparison. The generator uses only Python 3's standard library; it is not invoked by the test suite, so running tests does not require Python. + +PDF object order and offsets, DOCX entry order, ZIP timestamps, Unix file permissions, and DEFLATE level 6 are fixed. The committed files have these sizes and SHA-256 hashes: + +| Fixture | Bytes | SHA-256 | +| ------------------------------ | ----: | ------------------------------------------------------------------ | +| [approved.pdf](approved.pdf) | 643 | `11b19aed8bc2e40b9eda9d18b7bfe36e0bbc41c4383d7e30141927992790f411` | +| [approved.docx](approved.docx) | 1002 | `732d827ad38ff51c24983c773bcb964ba53ca29bb055ea73aa9fbed820ae3a9b` | + +The integration suite checks these hashes as well as the real parsers' extracted text. If a fixture is intentionally changed, update its generator source, the documented hash, and the corresponding test expectations together. diff --git a/src/integrations/misc/__tests__/fixtures/approved.docx b/src/integrations/misc/__tests__/fixtures/approved.docx new file mode 100644 index 0000000000..653a1c8889 Binary files /dev/null and b/src/integrations/misc/__tests__/fixtures/approved.docx differ diff --git a/src/integrations/misc/__tests__/fixtures/approved.pdf b/src/integrations/misc/__tests__/fixtures/approved.pdf new file mode 100644 index 0000000000..5d13f49b4a Binary files /dev/null and b/src/integrations/misc/__tests__/fixtures/approved.pdf differ diff --git a/src/integrations/misc/__tests__/fixtures/generate-fixtures.py b/src/integrations/misc/__tests__/fixtures/generate-fixtures.py new file mode 100644 index 0000000000..7d98e0c5c1 --- /dev/null +++ b/src/integrations/misc/__tests__/fixtures/generate-fixtures.py @@ -0,0 +1,108 @@ +"""Regenerate the owned PDF/DOCX fixtures using only Python's standard library.""" + +import argparse +import hashlib +import io +from pathlib import Path +import zipfile + + +def generate_pdf() -> bytes: + stream = ( + b"BT\n/F1 12 Tf\n72 720 Td\n" + b"(Approved PDF first line) Tj\n0 -18 Td\n" + b"(Approved PDF second line) Tj\nET\n" + ) + objects = [ + b"<< /Type /Catalog /Pages 2 0 R >>", + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] " + b"/Resources << /Font << /F1 4 0 R >> >> /Contents 5 0 R >>", + b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>", + b"<< /Length %d >>\nstream\n" % len(stream) + stream + b"endstream", + ] + document = bytearray(b"%PDF-1.4\n%\xe2\xe3\xcf\xd3\n") + offsets = [] + for number, content in enumerate(objects, 1): + offsets.append(len(document)) + document += b"%d 0 obj\n" % number + content + b"\nendobj\n" + + xref_offset = len(document) + document += b"xref\n0 6\n0000000000 65535 f \n" + for offset in offsets: + document += b"%010d 00000 n \n" % offset + document += ( + b"trailer\n<< /Size 6 /Root 1 0 R >>\nstartxref\n%d\n%%%%EOF\n" + % xref_offset + ) + return bytes(document) + + +def generate_docx() -> bytes: + header = '\n' + parts = [ + ( + "[Content_Types].xml", + header + + '\n' + '\n' + '\n' + '\n' + '', + ), + ( + "_rels/.rels", + header + + '\n' + '\n' + '', + ), + ( + "word/document.xml", + header + + '\n' + '\n' + 'Approved DOCX first paragraph\n' + 'Approved DOCX second & ' + 'final paragraph\n' + '\n' + '\n', + ), + ] + + output = io.BytesIO() + with zipfile.ZipFile(output, "w") as archive: + for name, content in parts: + entry = zipfile.ZipInfo(name, (2020, 1, 1, 0, 0, 0)) + entry.compress_type = zipfile.ZIP_DEFLATED + entry.create_system = 3 + entry.external_attr = 0o100644 << 16 + archive.writestr(entry, content.encode("utf-8"), compresslevel=6) + return output.getvalue() + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "output_directory", + nargs="?", + type=Path, + default=Path(__file__).resolve().parent, + help="destination directory (defaults to this script's directory)", + ) + output_directory = parser.parse_args().output_directory + output_directory.mkdir(parents=True, exist_ok=True) + + for name, content in ( + ("approved.pdf", generate_pdf()), + ("approved.docx", generate_docx()), + ): + (output_directory / name).write_bytes(content) + print(f"{hashlib.sha256(content).hexdigest()} {name}") + + +if __name__ == "__main__": + main() diff --git a/src/integrations/misc/extract-text-from-xlsx.ts b/src/integrations/misc/extract-text-from-xlsx.ts index 82e1d4db9a..9ff3078a0e 100644 --- a/src/integrations/misc/extract-text-from-xlsx.ts +++ b/src/integrations/misc/extract-text-from-xlsx.ts @@ -1,4 +1,13 @@ import ExcelJS from "exceljs" +import type { Buffer as NodeBuffer } from "node:buffer" + +// ExcelJS 4.4.0 declares its Buffer as an ArrayBuffer, but its Node loader accepts +// Node Buffers directly. Correct that boundary without casts or copying the input. +declare module "exceljs" { + interface Xlsx { + load(buffer: NodeBuffer, options?: Partial): Promise + } +} const ROW_LIMIT = 50000 @@ -40,16 +49,14 @@ function formatCellValue(cell: ExcelJS.Cell): string { return value.toString() } -export async function extractTextFromXLSX(filePathOrWorkbook: string | ExcelJS.Workbook): Promise { - let workbook: ExcelJS.Workbook - let excelText = "" +export async function extractTextFromXLSX(source: Buffer): Promise { + const workbook = new ExcelJS.Workbook() + await workbook.xlsx.load(source) + return formatWorkbook(workbook) +} - if (typeof filePathOrWorkbook === "string") { - workbook = new ExcelJS.Workbook() - await workbook.xlsx.readFile(filePathOrWorkbook) - } else { - workbook = filePathOrWorkbook - } +export function formatWorkbook(workbook: ExcelJS.Workbook): string { + let excelText = "" workbook.eachSheet((worksheet, sheetId) => { if (worksheet.state === "hidden" || worksheet.state === "veryHidden") { diff --git a/src/integrations/misc/extract-text.ts b/src/integrations/misc/extract-text.ts index 1ff98d273b..c757733b46 100644 --- a/src/integrations/misc/extract-text.ts +++ b/src/integrations/misc/extract-text.ts @@ -8,20 +8,18 @@ import { extractTextFromXLSX } from "./extract-text-from-xlsx" import { readWithSlice } from "./indentation-reader" import { DEFAULT_LINE_LIMIT } from "../../core/prompts/tools/native-tools/read_file" -async function extractTextFromPDF(filePath: string): Promise { - const dataBuffer = await fs.readFile(filePath) - const data = await pdf(dataBuffer) +async function extractTextFromPDF(source: Buffer): Promise { + const data = await pdf(source) return addLineNumbers(data.text) } -async function extractTextFromDOCX(filePath: string): Promise { - const result = await mammoth.extractRawText({ path: filePath }) +async function extractTextFromDOCX(source: Buffer): Promise { + const result = await mammoth.extractRawText({ buffer: source }) return addLineNumbers(result.value) } -async function extractTextFromIPYNB(filePath: string): Promise { - const data = await fs.readFile(filePath, "utf8") - const notebook = JSON.parse(data) +async function extractTextFromIPYNB(source: Buffer): Promise { + const notebook = JSON.parse(source.toString("utf8")) let extractedText = "" for (const cell of notebook.cells) { @@ -91,7 +89,8 @@ export async function extractTextFromFileWithMetadata( const extractor = SUPPORTED_BINARY_FORMATS[fileExtension as keyof typeof SUPPORTED_BINARY_FORMATS] if (extractor) { // For binary formats, extract and count lines - const content = await extractor(filePath) + const source = await fs.readFile(filePath) + const content = await extractor(source) const lines = content.split("\n") return { content, @@ -113,7 +112,7 @@ export async function extractTextFromFileWithMetadata( totalLines: result.totalLines, returnedLines: result.returnedLines, wasTruncated: result.wasTruncated, - linesShown: result.includedRanges.length > 0 ? result.includedRanges[0] : undefined, + linesShown: result.includedRanges[0], } } else { throw new Error(`Cannot read text for file type: ${fileExtension}`) @@ -133,6 +132,24 @@ export async function extractTextFromFile(filePath: string): Promise { return result.content } +/** + * Extracts formatted text exclusively from supplied bytes, without pathname I/O. + * filePath is only format/diagnostic metadata, never a source of file contents. + */ +export async function extractTextFromBuffer(source: Buffer, filePath: string): Promise { + const fileExtension = path.extname(filePath).toLowerCase() + const extractor = SUPPORTED_BINARY_FORMATS[fileExtension as keyof typeof SUPPORTED_BINARY_FORMATS] + if (extractor) { + return extractor(source) + } + + if (await isBinaryFile(source).catch(() => false)) { + throw new Error(`Cannot read text for file type: ${fileExtension}`) + } + + return readWithSlice(source.toString("utf8"), 0, DEFAULT_LINE_LIMIT).content +} + export function addLineNumbers(content: string, startLine: number = 1): string { // If content is empty, return empty string - empty files should not have line numbers // If content is empty but startLine > 1, return "startLine | " because we know the file is not empty @@ -187,16 +204,8 @@ export function stripLineNumbers(content: string, aggressive: boolean = false): // Join back with original line endings (carriage return (\r) + line feed (\n) or just line feed (\n)) const lineEnding = content.includes("\r\n") ? "\r\n" : "\n" - let result = processedLines.join(lineEnding) - - // Preserve trailing newline if present in original content - if (content.endsWith(lineEnding)) { - if (!result.endsWith(lineEnding)) { - result += lineEnding - } - } - - return result + // split() preserves the trailing empty element, so join() also preserves the trailing newline. + return processedLines.join(lineEnding) } /** @@ -354,7 +363,7 @@ export function applyRunLengthEncoding(content: string): string { result += prevLine } } - } else if (prevLine !== null) { + } else { result += prevLine } @@ -499,7 +508,7 @@ function processLineWithCarriageReturns( } else { // Partial overwrite - need to check for multi-byte character boundary issues const potentialPartialChar = curLine.charAt(segment.length) - const segmentLastCharCode = segment.length > 0 ? segment.charCodeAt(segment.length - 1) : 0 + const segmentLastCharCode = segment.charCodeAt(segment.length - 1) const partialCharCode = potentialPartialChar.charCodeAt(0) // Simplified condition for multi-byte character detection