From 637d00cebcd07bcae917539e4b762e3e1ef45c66 Mon Sep 17 00:00:00 2001 From: jinye Date: Thu, 9 Jul 2026 18:09:44 +0800 Subject: [PATCH] feat(core): render PDF pages to images when text extraction overflows or fails (#6585) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(core): render PDF pages to images when text overflows or fails Replace the PDF text-only 12000-token dead-end with a bounded page->image fallback for vision-capable models, mirroring claude-code's approach. - pdf.ts: add renderPDFPagesToImages / isPdftoppmAvailable (pdftoppm -jpeg -scale-to), with timeout, error mapping, and a total-bytes cap. - fileUtils.ts: text-first, then four ordered fallbacks on overflow/failure: (1) render to the vision main model (explicit read, <=20 pages), (2) render <=4 pages to the vision bridge (text-only model, scanned @-PDF), (3) reference (no-pages @-attach), (4) text too-large guidance / error. Widen ProcessedFileReadResult.llmContent to PartListUnion. - read-file.ts / readManyFiles.ts / pathReader.ts: handle the Part[] payload; pathReader now references large @-PDFs, aligning the two @ paths. WIP: tests not yet migrated. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * test(core): cover PDF page rendering and vision-bridge fallbacks - pdf.test.ts: renderPDFPagesToImages (success/sort, page range, Infinity last page, unavailable, password, corrupt, empty output, byte-cap). - fileUtils.test.ts: mock renderPDFPagesToImages and assert vision-model page/whole-doc/scanned rendering, >20-page guidance, truncation notes, renderer-unavailable fallback, and text-only bridge rendering (scanned @ PDF -> <=4 pages, page-count note, text-first reference, no-flag error). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(core): never drop PDF pages silently when page count is unknown Audit follow-up. Both image-render fallbacks could omit pages without a note when pdfinfo is unavailable (page count null): - vision no-pages render: when the size heuristic underestimates and the render fills the per-read page ceiling, add a note (previously only the byte-cap path was flagged). - vision-bridge render: when the page count is unknown and the render hits the image cap, flag it (previously required a known count). Soften the note wording to not over-promise a later-range read on the @ path. Adds regression tests for both unknown-page-count paths. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/core/src/tools/read-file.ts | 4 +- packages/core/src/utils/fileUtils.test.ts | 339 +++++++++++++++++++++- packages/core/src/utils/fileUtils.ts | 195 +++++++++++-- packages/core/src/utils/pathReader.ts | 20 +- packages/core/src/utils/pdf.test.ts | 165 +++++++++++ packages/core/src/utils/pdf.ts | 207 +++++++++++++ packages/core/src/utils/readManyFiles.ts | 9 +- 7 files changed, 899 insertions(+), 40 deletions(-) diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index 947a98b9ec..6a9b507516 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -12,7 +12,7 @@ import type { ToolInvocation, ToolLocation, ToolResult } from './tools.js'; import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js'; import { ToolNames, ToolDisplayNames } from './tool-names.js'; -import type { PartUnion } from '@google/genai'; +import type { PartListUnion } from '@google/genai'; import type { PermissionDecision } from '../permissions/types.js'; import { processSingleFileContent, @@ -277,7 +277,7 @@ class ReadFileToolInvocation extends BaseToolInvocation< }); } - let llmContent: PartUnion; + let llmContent: PartListUnion; if ( result.isTruncated && result.linesShown && diff --git a/packages/core/src/utils/fileUtils.test.ts b/packages/core/src/utils/fileUtils.test.ts index c0cef12435..44636590e4 100644 --- a/packages/core/src/utils/fileUtils.test.ts +++ b/packages/core/src/utils/fileUtils.test.ts @@ -40,7 +40,12 @@ import { LargeNonUtf8TextError } from './read-text-range.js'; import type { Config } from '../config/config.js'; import { StandardFileSystemService } from '../services/fileSystemService.js'; import { ToolErrorType } from '../tools/tool-error.js'; -import { resetPdftotextCache } from './pdf.js'; +import { + PDF_MAX_PAGES_PER_READ, + renderPDFPagesToImages, + resetPdftotextCache, +} from './pdf.js'; +import { VISION_BRIDGE_MAX_IMAGES } from '../services/visionBridge/vision-bridge-constants.js'; vi.mock('mime/lite', () => ({ default: { getType: vi.fn() }, @@ -81,8 +86,18 @@ vi.mock('node:child_process', async (importOriginal) => { }; }); +// Keep the real pdf.js (extractPDFText, page-count gates, etc. drive the +// text path via the mocked execFile above) but stub out the image renderer so +// tests don't shell out to poppler / touch the filesystem. pdf.test.ts covers +// renderPDFPagesToImages itself. +vi.mock('./pdf.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, renderPDFPagesToImages: vi.fn() }; +}); + const mockMimeGetType = mime.getType as Mock; const mockExecFile = vi.mocked(execFile); +const mockRender = vi.mocked(renderPDFPagesToImages); function mockExecResult(result: { stdout: string; @@ -1045,6 +1060,12 @@ describe('fileUtils', () => { describe('processSingleFileContent', () => { beforeEach(() => { + // Default: renderer unavailable, so PDF reads fall back to the text path + // unless a test opts into rendering. Set after the global resetAllMocks. + mockRender.mockResolvedValue({ + success: false, + error: 'pdftoppm unavailable (test default)', + }); // Ensure files exist for statSync checks before readFile might be mocked if (actualNodeFs.existsSync(testTextFilePath)) actualNodeFs.unlinkSync(testTextFilePath); @@ -1868,6 +1889,322 @@ describe('fileUtils', () => { expect(result.returnDisplay).toContain('Read pdf file'); }); + describe('PDF image rendering (vision fallback)', () => { + const visionConfig = { + ...mockConfig, + getContentGeneratorConfig: () => ({ modalities: { image: true } }), + } as unknown as Config; + const fakeImage = (data: string) => ({ data, mimeType: 'image/jpeg' }); + type MediaPart = { text?: string; inlineData?: { data: string } }; + + it('renders the requested page range when text overflows', async () => { + actualNodeFs.writeFileSync(testPdfFilePath, Buffer.from('%PDF-1.7')); + mockMimeGetType.mockReturnValue('application/pdf'); + mockExecResult({ stdout: '', stderr: 'pdftotext version', code: 0 }); + mockExecResult({ stdout: 'x'.repeat(80_000), stderr: '', code: 0 }); + mockRender.mockResolvedValue({ + success: true, + images: [fakeImage('AAA'), fakeImage('BBB')], + bytesTruncated: false, + }); + + const result = await processSingleFileContent( + testPdfFilePath, + visionConfig, + { pages: '1-2' }, + ); + + expect(result.error).toBeUndefined(); + expect(Array.isArray(result.llmContent)).toBe(true); + const parts = result.llmContent as MediaPart[]; + expect(parts).toHaveLength(2); + expect(parts[0]!.inlineData).toMatchObject({ + data: 'AAA', + mimeType: 'image/jpeg', + }); + expect(result.returnDisplay).toContain('image'); + expect(mockRender).toHaveBeenCalledWith(testPdfFilePath, { + firstPage: 1, + lastPage: 2, + }); + }); + + it('renders the whole document (up to the ceiling) for a no-pages overflow', async () => { + actualNodeFs.writeFileSync(testPdfFilePath, Buffer.from('%PDF-1.7')); + mockMimeGetType.mockReturnValue('application/pdf'); + mockExecResult({ stdout: 'Pages: 3\n', stderr: '', code: 0 }); + mockExecResult({ stdout: '', stderr: 'pdftotext version', code: 0 }); + mockExecResult({ stdout: 'x'.repeat(80_000), stderr: '', code: 0 }); + mockRender.mockResolvedValue({ + success: true, + images: [fakeImage('P1'), fakeImage('P2'), fakeImage('P3')], + bytesTruncated: false, + }); + + const result = await processSingleFileContent( + testPdfFilePath, + visionConfig, + ); + + expect(Array.isArray(result.llmContent)).toBe(true); + expect(result.llmContent).toHaveLength(3); + expect(mockRender).toHaveBeenCalledWith(testPdfFilePath, { + firstPage: 1, + lastPage: PDF_MAX_PAGES_PER_READ, + }); + }); + + it('renders images when extraction fails on a scanned PDF', async () => { + actualNodeFs.writeFileSync(testPdfFilePath, Buffer.from('%PDF-1.7')); + mockMimeGetType.mockReturnValue('application/pdf'); + mockExecResult({ stdout: 'Pages: 2\n', stderr: '', code: 0 }); + mockExecResult({ stdout: '', stderr: 'pdftotext version', code: 0 }); + mockExecResult({ stdout: ' ', stderr: '', code: 0 }); + mockRender.mockResolvedValue({ + success: true, + images: [fakeImage('S1'), fakeImage('S2')], + bytesTruncated: false, + }); + + const result = await processSingleFileContent( + testPdfFilePath, + visionConfig, + ); + + expect(Array.isArray(result.llmContent)).toBe(true); + expect(result.llmContent).toHaveLength(2); + }); + + it('still returns page guidance (no render) beyond the page ceiling', async () => { + actualNodeFs.writeFileSync(testPdfFilePath, Buffer.from('%PDF-1.7')); + mockMimeGetType.mockReturnValue('application/pdf'); + mockExecResult({ stdout: 'Pages: 42\n', stderr: '', code: 0 }); + mockExecResult({ stdout: '', stderr: 'pdftotext version', code: 0 }); + + const result = await processSingleFileContent( + testPdfFilePath, + visionConfig, + ); + + expect(result.errorType).toBe(ToolErrorType.FILE_TOO_LARGE); + expect(result.llmContent).toContain("Use the 'pages' parameter"); + expect(mockRender).not.toHaveBeenCalled(); + }); + + it('flags truncation (never drops pages silently)', async () => { + actualNodeFs.writeFileSync(testPdfFilePath, Buffer.from('%PDF-1.7')); + mockMimeGetType.mockReturnValue('application/pdf'); + mockExecResult({ stdout: '', stderr: 'pdftotext version', code: 0 }); + mockExecResult({ stdout: 'x'.repeat(80_000), stderr: '', code: 0 }); + mockRender.mockResolvedValue({ + success: true, + images: [fakeImage('ONLY')], + bytesTruncated: true, + }); + + const result = await processSingleFileContent( + testPdfFilePath, + visionConfig, + { pages: '1-5' }, + ); + + const parts = result.llmContent as MediaPart[]; + expect( + parts.some( + (p) => typeof p.text === 'string' && /omitted/.test(p.text), + ), + ).toBe(true); + }); + + it('notes the page ceiling when a no-pages render fills it (page count unknown)', async () => { + actualNodeFs.writeFileSync(testPdfFilePath, Buffer.from('%PDF-1.7')); + mockMimeGetType.mockReturnValue('application/pdf'); + // pdfinfo unavailable -> page count falls back to the size heuristic, + // which underestimates; the render then fills the 20-page ceiling. + mockExecResult({ stdout: '', stderr: 'pdfinfo missing', code: 1 }); + mockExecResult({ stdout: '', stderr: 'pdftotext version', code: 0 }); + mockExecResult({ stdout: 'x'.repeat(80_000), stderr: '', code: 0 }); + mockRender.mockResolvedValue({ + success: true, + images: Array.from({ length: PDF_MAX_PAGES_PER_READ }, (_, i) => + fakeImage(`P${i + 1}`), + ), + bytesTruncated: false, + }); + + const result = await processSingleFileContent( + testPdfFilePath, + visionConfig, + ); + + const parts = result.llmContent as MediaPart[]; + expect(parts.filter((p) => p.inlineData).length).toBe( + PDF_MAX_PAGES_PER_READ, + ); + expect( + parts.some( + (p) => + typeof p.text === 'string' && /per-read maximum/.test(p.text), + ), + ).toBe(true); + }); + + it('falls back to text guidance when the renderer is unavailable', async () => { + actualNodeFs.writeFileSync(testPdfFilePath, Buffer.from('%PDF-1.7')); + mockMimeGetType.mockReturnValue('application/pdf'); + mockExecResult({ stdout: '', stderr: 'pdftotext version', code: 0 }); + mockExecResult({ stdout: 'x'.repeat(80_000), stderr: '', code: 0 }); + // mockRender default = failure (renderer unavailable). + + const result = await processSingleFileContent( + testPdfFilePath, + visionConfig, + { pages: '1' }, + ); + + expect(result.errorType).toBe(ToolErrorType.FILE_TOO_LARGE); + expect(result.llmContent).toContain('too large to return safely'); + }); + }); + + describe('PDF vision-bridge rendering (text-only model)', () => { + const bridgeConfig = { + ...mockConfig, + getContentGeneratorConfig: () => ({ modalities: {} }), + } as unknown as Config; + const fakeImage = (data: string) => ({ data, mimeType: 'image/jpeg' }); + type MediaPart = { text?: string; inlineData?: { data: string } }; + + it('renders up to VISION_BRIDGE_MAX_IMAGES pages for a scanned @ PDF', async () => { + actualNodeFs.writeFileSync(testPdfFilePath, Buffer.from('%PDF-1.7')); + mockMimeGetType.mockReturnValue('application/pdf'); + mockExecResult({ stdout: 'Pages: 2\n', stderr: '', code: 0 }); + mockExecResult({ stdout: '', stderr: 'pdftotext version', code: 0 }); + mockExecResult({ stdout: ' ', stderr: '', code: 0 }); + mockExecResult({ stdout: 'Pages: 2\n', stderr: '', code: 0 }); + mockRender.mockResolvedValue({ + success: true, + images: [fakeImage('B1'), fakeImage('B2')], + bytesTruncated: false, + }); + + const result = await processSingleFileContent( + testPdfFilePath, + bridgeConfig, + { preserveUnsupportedImage: true, largePdfBehavior: 'reference' }, + ); + + expect(Array.isArray(result.llmContent)).toBe(true); + expect(mockRender).toHaveBeenCalledWith(testPdfFilePath, { + firstPage: 1, + lastPage: VISION_BRIDGE_MAX_IMAGES, + }); + const parts = result.llmContent as MediaPart[]; + expect(parts.filter((p) => p.inlineData).length).toBe(2); + }); + + it('notes how many pages were rendered when more remain', async () => { + actualNodeFs.writeFileSync(testPdfFilePath, Buffer.from('%PDF-1.7')); + mockMimeGetType.mockReturnValue('application/pdf'); + mockExecResult({ stdout: 'Pages: 10\n', stderr: '', code: 0 }); + mockExecResult({ stdout: '', stderr: 'pdftotext version', code: 0 }); + mockExecResult({ stdout: ' ', stderr: '', code: 0 }); + mockExecResult({ stdout: 'Pages: 10\n', stderr: '', code: 0 }); + mockRender.mockResolvedValue({ + success: true, + images: [ + fakeImage('1'), + fakeImage('2'), + fakeImage('3'), + fakeImage('4'), + ], + bytesTruncated: false, + }); + + const result = await processSingleFileContent( + testPdfFilePath, + bridgeConfig, + { preserveUnsupportedImage: true, largePdfBehavior: 'reference' }, + ); + + const parts = result.llmContent as MediaPart[]; + expect(parts.filter((p) => p.inlineData).length).toBe(4); + expect( + parts.some((p) => typeof p.text === 'string' && /of 10/.test(p.text)), + ).toBe(true); + }); + + it('notes truncation when the render fills the cap and the page count is unknown', async () => { + actualNodeFs.writeFileSync(testPdfFilePath, Buffer.from('%PDF-1.7')); + mockMimeGetType.mockReturnValue('application/pdf'); + // pdfinfo unavailable on both the pre-gate probe and the note probe. + mockExecResult({ stdout: '', stderr: 'pdfinfo missing', code: 1 }); + mockExecResult({ stdout: '', stderr: 'pdftotext version', code: 0 }); + mockExecResult({ stdout: ' ', stderr: '', code: 0 }); + mockExecResult({ stdout: '', stderr: 'pdfinfo missing', code: 1 }); + mockRender.mockResolvedValue({ + success: true, + images: Array.from({ length: VISION_BRIDGE_MAX_IMAGES }, (_, i) => + fakeImage(`B${i + 1}`), + ), + bytesTruncated: false, + }); + + const result = await processSingleFileContent( + testPdfFilePath, + bridgeConfig, + { preserveUnsupportedImage: true, largePdfBehavior: 'reference' }, + ); + + const parts = result.llmContent as MediaPart[]; + expect(parts.filter((p) => p.inlineData).length).toBe( + VISION_BRIDGE_MAX_IMAGES, + ); + // No exact count is known, so no "of N", but truncation is still noted. + const note = parts.find( + (p) => typeof p.text === 'string' && /not included/.test(p.text), + ); + expect(note).toBeDefined(); + expect(note!.text).not.toMatch(/ of \d/); + }); + + it('keeps text-heavy @ PDFs as reference (text-first, no render)', async () => { + actualNodeFs.writeFileSync(testPdfFilePath, Buffer.from('%PDF-1.7')); + mockMimeGetType.mockReturnValue('application/pdf'); + mockExecResult({ stdout: 'Pages: 2\n', stderr: '', code: 0 }); + mockExecResult({ stdout: '', stderr: 'pdftotext version', code: 0 }); + mockExecResult({ stdout: 'x'.repeat(80_000), stderr: '', code: 0 }); + + const result = await processSingleFileContent( + testPdfFilePath, + bridgeConfig, + { preserveUnsupportedImage: true, largePdfBehavior: 'reference' }, + ); + + expect(result.error).toBeUndefined(); + expect(result.returnDisplay).toContain('Referenced large PDF'); + expect(result.llmContent).toContain('too large to return safely'); + expect(mockRender).not.toHaveBeenCalled(); + }); + + it('does not render without the bridge flag (scanned stays an error)', async () => { + actualNodeFs.writeFileSync(testPdfFilePath, Buffer.from('%PDF-1.7')); + mockMimeGetType.mockReturnValue('application/pdf'); + mockExecResult({ stdout: 'Pages: 2\n', stderr: '', code: 0 }); + mockExecResult({ stdout: '', stderr: 'pdftotext version', code: 0 }); + mockExecResult({ stdout: ' ', stderr: '', code: 0 }); + + const result = await processSingleFileContent( + testPdfFilePath, + bridgeConfig, + ); + + expect(result.errorType).toBe(ToolErrorType.READ_CONTENT_FAILURE); + expect(result.llmContent).toContain('Cannot extract text from PDF'); + expect(mockRender).not.toHaveBeenCalled(); + }); + }); + it('should read an SVG file as text when under 1MB', async () => { const svgContent = ` diff --git a/packages/core/src/utils/fileUtils.ts b/packages/core/src/utils/fileUtils.ts index 6cb62bf205..0f77e19d7b 100644 --- a/packages/core/src/utils/fileUtils.ts +++ b/packages/core/src/utils/fileUtils.ts @@ -7,7 +7,7 @@ import fs from 'node:fs'; import fsPromises from 'node:fs/promises'; import path from 'node:path'; -import type { PartUnion } from '@google/genai'; +import type { Part, PartListUnion } from '@google/genai'; import mime from 'mime/lite'; import { iconvDecode, @@ -21,6 +21,7 @@ import { createDebugLogger } from './debugLogger.js'; import { getErrorMessage, isAbortError, isNodeError } from './errors.js'; import type { InputModalities } from '../core/contentGenerator.js'; import { detectEncodingFromBuffer } from './systemEncoding.js'; +import type { PDFRenderedImage } from './pdf.js'; import { buildLargePDFGuidance, buildPDFTextTooLargeGuidance, @@ -32,8 +33,10 @@ import { PDF_MAX_PAGES_PER_READ, PDF_TEXT_EXTRACTION_UNAVAILABLE_MESSAGE, PDF_TEXT_RESULT_MAX_TOKENS, + renderPDFPagesToImages, shouldRequirePDFPageRange, } from './pdf.js'; +import { VISION_BRIDGE_MAX_IMAGES } from '../services/visionBridge/vision-bridge-constants.js'; import { readNotebookWithMetadata } from './notebook.js'; import { readTextRange } from './read-text-range.js'; import { @@ -862,7 +865,10 @@ export async function detectFileType(filePath: string): Promise { } export interface ProcessedFileReadResult { - llmContent: PartUnion; // string for text, Part for image/pdf/unreadable binary + // string for text; a single Part for image / native-PDF / unreadable binary; + // an array of image Parts when a PDF is rendered page-by-page (vision + // fallback / bridge transcription). + llmContent: PartListUnion; returnDisplay: string; error?: string; // Optional error message for the LLM if file processing failed errorType?: ToolErrorType; // Structured error type @@ -1044,6 +1050,23 @@ export async function processSingleFileContent( const modalities: InputModalities = config.getContentGeneratorConfig?.()?.modalities ?? {}; + // Vision-capable main model, explicit read (not `@`-reference): when text + // extraction overflows or fails, render pages to images for the model + // itself. Deliberately keyed on `modalities.image` (not `!modalities.pdf`) + // so an image+pdf model's dense `pages` read can still be rescued; the + // native whole-PDF base64 branch runs earlier and is unaffected. + const willRenderPdfImages = + fileType === 'pdf' && + !!modalities.image && + largePdfBehavior !== 'reference'; + // Text-only main model on a bridge-capable `@` path: a scanned / no-text + // PDF is rendered to a few pages so the existing vision bridge can + // transcribe them. Only fires when text extraction genuinely fails (see + // the switch below); text-bearing PDFs stay text-first and fall to + // reference. + const renderForBridge = + fileType === 'pdf' && !modalities.image && preserveUnsupportedImage; + const fileSizeInMB = stats.size / (1024 * 1024); const normalizedPages = pages?.trim(); let pageRange: @@ -1112,10 +1135,17 @@ export async function processSingleFileContent( if (willExtractPdfText && !pageRange) { const pageCount = await getPDFPageCount(filePath); const requirement = shouldRequirePDFPageRange(pageCount, stats.size); + // A vision render can hold up to PDF_MAX_PAGES_PER_READ pages, so only + // require an explicit range past that ceiling; the text path keeps the + // tighter full-text limit. Below the ceiling we fall through and let the + // switch try text first, rendering only on overflow/failure. + const rangeRequired = willRenderPdfImages + ? requirement.effectivePageCount > PDF_MAX_PAGES_PER_READ + : requirement.required; debugLogger.debug( - `PDF full-text fallback gate: file=${relativePathForDisplay}, sizeMB=${fileSizeInMB.toFixed(2)}, pageCount=${pageCount ?? 'unknown'}, required=${requirement.required}, effectivePageCount=${requirement.effectivePageCount}, hadPdfInfo=${requirement.hadPdfInfo}, behavior=${largePdfBehavior}`, + `PDF full-text fallback gate: file=${relativePathForDisplay}, sizeMB=${fileSizeInMB.toFixed(2)}, pageCount=${pageCount ?? 'unknown'}, required=${requirement.required}, rangeRequired=${rangeRequired}, effectivePageCount=${requirement.effectivePageCount}, hadPdfInfo=${requirement.hadPdfInfo}, behavior=${largePdfBehavior}`, ); - if (requirement.required) { + if (rangeRequired) { if (largePdfBehavior === 'error' && !(await isPdftotextAvailable())) { return { llmContent: `[Cannot extract text from PDF: "${displayName}". ${PDF_TEXT_EXTRACTION_UNAVAILABLE_MESSAGE}]`, @@ -1385,46 +1415,149 @@ export async function processSingleFileContent( }; } - // Extract text via pdftotext (for pages parameter, or models without PDF support) + // Text-first: extract via pdftotext (for a pages parameter, or models + // without native PDF support). Only when the text overflows the token + // budget or extraction fails (scanned / no text layer) do we fall back + // to rendering pages as images. const pdfResult = await extractPDFText(filePath, pageRange); if (pdfResult.success) { const estimatedTokens = estimatePDFTextOutputTokens(pdfResult.text); - if (estimatedTokens > PDF_TEXT_RESULT_MAX_TOKENS) { - debugLogger.debug( - `PDF text extraction output exceeds token limit: file=${relativePathForDisplay}, pages=${normalizedPages ?? 'all'}, estimatedTokens=${estimatedTokens}, limit=${PDF_TEXT_RESULT_MAX_TOKENS}`, - ); - const guidance = buildPDFTextTooLargeGuidance( - displayName, - estimatedTokens, - normalizedPages, - ); - if (!pageRange && largePdfBehavior === 'reference') { - return { - llmContent: guidance, - returnDisplay: `Referenced large PDF: ${relativePathForDisplay}`, - stats, - }; - } + if (estimatedTokens <= PDF_TEXT_RESULT_MAX_TOKENS) { + const pagesLabel = normalizedPages + ? ` (pages ${normalizedPages})` + : ''; return { - llmContent: guidance, - returnDisplay: `PDF text too large: ${relativePathForDisplay}`, - error: guidance, - errorType: ToolErrorType.FILE_TOO_LARGE, + llmContent: pdfResult.text, + returnDisplay: `Read pdf as text${pagesLabel}: ${relativePathForDisplay}`, stats, }; } + } - const pagesLabel = normalizedPages - ? ` (pages ${normalizedPages})` - : ''; + // Extraction overflowed (dense text) or failed (scanned / no text + // layer). Both converge here, resolved by the ordered, mutually + // exclusive fallbacks below. Tag each rendered page with its source + // page number so the model / bridge can cite pages. + const toImageParts = ( + images: PDFRenderedImage[], + startPage: number, + ): Part[] => + images.map((image, index) => ({ + inlineData: { + data: image.data, + mimeType: image.mimeType, + displayName: `${displayName} (page ${startPage + index})`, + }, + })); + + // (1) Render to the vision main model itself (explicit read). With no + // page range, render from the start up to the per-read ceiling. + if (willRenderPdfImages) { + const startPage = pageRange?.firstPage ?? 1; + const render = await renderPDFPagesToImages( + filePath, + pageRange ?? { firstPage: 1, lastPage: PDF_MAX_PAGES_PER_READ }, + ); + if (render.success) { + const parts = toImageParts(render.images, startPage); + // Never drop pages silently. Two ways a no-page-range read can be + // partial: the byte cap kicked in, or the render filled the page + // ceiling (the page count was unknown/underestimated upstream, so + // more pages may follow). + if (render.bytesTruncated) { + parts.push({ + text: `[Rendered the first ${render.images.length} page(s) of "${displayName}"; later pages were omitted to stay within size limits. Use the 'pages' parameter to read a specific range.]`, + }); + } else if ( + !pageRange && + render.images.length >= PDF_MAX_PAGES_PER_READ + ) { + parts.push({ + text: `[Rendered the first ${render.images.length} page(s) (the per-read maximum) of "${displayName}". If the document has more pages, use the 'pages' parameter to read a later range.]`, + }); + } + return { + llmContent: parts, + returnDisplay: `Read pdf as ${render.images.length} image(s): ${relativePathForDisplay}`, + stats, + }; + } + // Render unavailable/failed — fall through to the text-based + // guidance / error below so the user still gets an actionable + // message (e.g. install poppler-utils). + debugLogger.debug( + `PDF image render failed, falling back to text outcome: file=${relativePathForDisplay}, error=${render.error}`, + ); + } + + // (2) Render to the vision bridge for a text-only main model — but only + // for scanned / no-text PDFs. Text-bearing PDFs stay text-first and + // fall through to reference. This must precede the reference branch: + // the `@` path sets both `reference` and the preserve flag, so + // checking reference first would starve the bridge. + if (renderForBridge && pdfResult.success === false) { + const render = await renderPDFPagesToImages(filePath, { + firstPage: 1, + lastPage: VISION_BRIDGE_MAX_IMAGES, + }); + if (render.success) { + const parts = toImageParts(render.images, 1); + const pageCount = await getPDFPageCount(filePath); + // Never drop pages silently: a known page count above what we + // rendered, or (when the count is unknown) a render that filled the + // page cap, both mean pages may be missing. + const mayHaveMore = + pageCount !== null + ? pageCount > render.images.length + : render.images.length >= VISION_BRIDGE_MAX_IMAGES; + if (mayHaveMore || render.bytesTruncated) { + const total = pageCount !== null ? ` of ${pageCount}` : ''; + parts.push({ + text: `[Rendered the first ${render.images.length}${total} page(s) of "${displayName}" for transcription; later pages were not included.]`, + }); + } + return { + llmContent: parts, + returnDisplay: `Rendered ${render.images.length} page(s) for transcription: ${relativePathForDisplay}`, + stats, + }; + } + debugLogger.debug( + `PDF bridge render failed, falling back to text outcome: file=${relativePathForDisplay}, error=${render.error}`, + ); + } + + if (pdfResult.success) { + // Overflowed text: guidance to narrow the range. + const guidance = buildPDFTextTooLargeGuidance( + displayName, + estimatePDFTextOutputTokens(pdfResult.text), + normalizedPages, + ); + debugLogger.debug( + `PDF text extraction output exceeds token limit: file=${relativePathForDisplay}, pages=${normalizedPages ?? 'all'}, limit=${PDF_TEXT_RESULT_MAX_TOKENS}`, + ); + // (3) Reference (no pages, `@`-attached): guidance without a failed read. + if (!pageRange && largePdfBehavior === 'reference') { + return { + llmContent: guidance, + returnDisplay: `Referenced large PDF: ${relativePathForDisplay}`, + stats, + }; + } + // (4) Text fallback: surface the too-large guidance as an error. return { - llmContent: pdfResult.text, - returnDisplay: `Read pdf as text${pagesLabel}: ${relativePathForDisplay}`, + llmContent: guidance, + returnDisplay: `PDF text too large: ${relativePathForDisplay}`, + error: guidance, + errorType: ToolErrorType.FILE_TOO_LARGE, stats, }; } - // pdftotext failed or not available — return helpful error + // pdftotext failed or not available and no render path handled it — + // return a helpful error (scanned PDF on a text-only model without an + // available bridge, or poppler entirely missing). return { llmContent: `[Cannot extract text from PDF: "${displayName}". ${pdfResult.error}]`, returnDisplay: `Failed to read pdf: ${relativePathForDisplay}`, diff --git a/packages/core/src/utils/pathReader.ts b/packages/core/src/utils/pathReader.ts index ca13212165..98af960e87 100644 --- a/packages/core/src/utils/pathReader.ts +++ b/packages/core/src/utils/pathReader.ts @@ -84,8 +84,16 @@ export async function readPathFromWorkspace( for (const filePath of finalFiles) { const relativePathForDisplay = path.relative(absolutePath, filePath); allParts.push({ text: `--- ${relativePathForDisplay} ---\n` }); - const result = await processSingleFileContent(filePath, config); - allParts.push(result.llmContent); + // `@`-attached reads reference large PDFs (guidance without a failed + // read), matching the readManyFiles `@` path. + const result = await processSingleFileContent(filePath, config, { + largePdfBehavior: 'reference', + }); + if (Array.isArray(result.llmContent)) { + allParts.push(...result.llmContent); + } else { + allParts.push(result.llmContent); + } allParts.push({ text: '\n' }); // Add a newline for separation } @@ -106,7 +114,11 @@ export async function readPathFromWorkspace( } // It's a single file, process it directly. - const result = await processSingleFileContent(absolutePath, config); - return [result.llmContent]; + const result = await processSingleFileContent(absolutePath, config, { + largePdfBehavior: 'reference', + }); + return Array.isArray(result.llmContent) + ? result.llmContent + : [result.llmContent]; } } diff --git a/packages/core/src/utils/pdf.test.ts b/packages/core/src/utils/pdf.test.ts index 6c09481001..795c083b84 100644 --- a/packages/core/src/utils/pdf.test.ts +++ b/packages/core/src/utils/pdf.test.ts @@ -15,14 +15,36 @@ import { estimatePDFTextOutputTokens, buildLargePDFGuidance, buildPDFTextTooLargeGuidance, + renderPDFPagesToImages, + resetPdftoppmCache, + PDF_RENDER_UNAVAILABLE_MESSAGE, } from './pdf.js'; vi.mock('node:child_process', () => ({ execFile: vi.fn(), })); +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, tmpdir: vi.fn(() => '/tmp') }; +}); + +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + mkdtemp: vi.fn(async () => '/tmp/pdf-render-test'), + readdir: vi.fn(), + readFile: vi.fn(), + rm: vi.fn(async () => undefined), + }; +}); + import { execFile } from 'node:child_process'; +import { readdir, readFile } from 'node:fs/promises'; const mockExecFile = vi.mocked(execFile); +const mockReaddir = vi.mocked(readdir); +const mockReadFile = vi.mocked(readFile); /** * Helper: make mockExecFile resolve with given stdout/stderr/code. @@ -96,6 +118,7 @@ describe('pdf utilities', () => { beforeEach(() => { vi.clearAllMocks(); resetPdftotextCache(); + resetPdftoppmCache(); }); describe('PDF budget policy helpers', () => { @@ -694,4 +717,146 @@ describe('pdf utilities', () => { } }); }); + + describe('renderPDFPagesToImages', () => { + // Queue the `pdftoppm -v` availability probe as successful. It runs once + // per render call because resetPdftoppmCache clears the cache each test. + const mockAvailable = () => + mockExecResult({ + stdout: '', + stderr: 'pdftoppm version 24.02.0', + code: 0, + }); + const asEntries = (names: string[]) => names as never; + + it('renders pages to base64 JPEG images, numerically sorted', async () => { + mockAvailable(); + mockExecResult({ stdout: '', stderr: '', code: 0 }); + // Returned out of order to prove numeric (not lexical) sorting. + mockReaddir.mockResolvedValue(asEntries(['page-2.jpg', 'page-1.jpg'])); + mockReadFile + .mockResolvedValueOnce(Buffer.from('page-one-bytes')) + .mockResolvedValueOnce(Buffer.from('page-two-bytes')); + + const result = await renderPDFPagesToImages('/test.pdf'); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.images).toEqual([ + { + data: Buffer.from('page-one-bytes').toString('base64'), + mimeType: 'image/jpeg', + }, + { + data: Buffer.from('page-two-bytes').toString('base64'), + mimeType: 'image/jpeg', + }, + ]); + expect(result.bytesTruncated).toBe(false); + } + const renderCall = mockExecFile.mock.calls[1]!; + expect(renderCall[0]).toBe('pdftoppm'); + expect(renderCall[1]).toEqual( + expect.arrayContaining(['-jpeg', '-scale-to']), + ); + }); + + it('forwards an explicit page range to pdftoppm', async () => { + mockAvailable(); + mockExecResult({ stdout: '', stderr: '', code: 0 }); + mockReaddir.mockResolvedValue(asEntries(['page-3.jpg'])); + mockReadFile.mockResolvedValue(Buffer.from('x')); + + await renderPDFPagesToImages('/test.pdf', { firstPage: 3, lastPage: 5 }); + + const args = mockExecFile.mock.calls[1]![1] as string[]; + expect(args[args.indexOf('-f') + 1]).toBe('3'); + expect(args[args.indexOf('-l') + 1]).toBe('5'); + }); + + it('omits -l for an open-ended (Infinity) last page', async () => { + mockAvailable(); + mockExecResult({ stdout: '', stderr: '', code: 0 }); + mockReaddir.mockResolvedValue(asEntries(['page-1.jpg'])); + mockReadFile.mockResolvedValue(Buffer.from('x')); + + await renderPDFPagesToImages('/test.pdf', { + firstPage: 1, + lastPage: Infinity, + }); + + const args = mockExecFile.mock.calls[1]![1] as string[]; + expect(args).toContain('-f'); + expect(args).not.toContain('-l'); + }); + + it('returns an install hint when pdftoppm is unavailable', async () => { + mockExecError(); // `-v` probe fails with ENOENT + const result = await renderPDFPagesToImages('/test.pdf'); + expect(result).toEqual({ + success: false, + error: PDF_RENDER_UNAVAILABLE_MESSAGE, + }); + // Only the availability probe ran; no render invocation followed. + expect(mockExecFile).toHaveBeenCalledTimes(1); + }); + + it('maps password-protected PDFs to a clear error', async () => { + mockAvailable(); + mockExecResult({ + stdout: '', + stderr: 'Command Line Error: Incorrect password', + code: 1, + }); + const result = await renderPDFPagesToImages('/test.pdf'); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain('password-protected'); + } + }); + + it('maps corrupt PDFs to a clear error', async () => { + mockAvailable(); + mockExecResult({ + stdout: '', + stderr: 'Syntax Error: Document is damaged', + code: 1, + }); + const result = await renderPDFPagesToImages('/test.pdf'); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain('corrupted'); + } + }); + + it('errors when pdftoppm produces no images', async () => { + mockAvailable(); + mockExecResult({ stdout: '', stderr: '', code: 0 }); + mockReaddir.mockResolvedValue(asEntries([])); + const result = await renderPDFPagesToImages('/test.pdf'); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain('no image output'); + } + }); + + it('caps total payload size and flags truncation instead of dropping silently', async () => { + mockAvailable(); + mockExecResult({ stdout: '', stderr: '', code: 0 }); + mockReaddir.mockResolvedValue(asEntries(['page-1.jpg', 'page-2.jpg'])); + // The first page alone (~27MB base64) already exceeds the 25MB cap, so + // the second page is dropped and the result is flagged. + mockReadFile + .mockResolvedValueOnce(Buffer.alloc(20 * 1024 * 1024)) + .mockResolvedValueOnce(Buffer.from('second-page')); + + const result = await renderPDFPagesToImages('/test.pdf'); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.images).toHaveLength(1); + expect(result.bytesTruncated).toBe(true); + } + }); + }); }); diff --git a/packages/core/src/utils/pdf.ts b/packages/core/src/utils/pdf.ts index 0921757e5f..2361db0b7a 100644 --- a/packages/core/src/utils/pdf.ts +++ b/packages/core/src/utils/pdf.ts @@ -5,6 +5,9 @@ */ import { execFile, type ExecFileOptions } from 'node:child_process'; +import { mkdtemp, readdir, readFile, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import { estimateTextTokens } from './request-tokenizer/textTokenizer.js'; const MAX_PDF_TEXT_OUTPUT_CHARS = 100000; @@ -16,6 +19,26 @@ const PDF_TEXT_RESULT_WRAPPER_TOKEN_CHARS = 64; const PDF_TEXT_RESULT_CHARS_PER_TOKEN = 4; export const PDF_TEXT_EXTRACTION_UNAVAILABLE_MESSAGE = 'pdftotext is not installed. Install poppler-utils to enable PDF text extraction (e.g. `apt-get install poppler-utils` or `brew install poppler`).'; +export const PDF_RENDER_UNAVAILABLE_MESSAGE = + 'pdftoppm is not installed. Install poppler-utils to enable PDF page rendering (e.g. `apt-get install poppler-utils` or `brew install poppler`).'; +/** + * Longest-edge pixel cap passed to `pdftoppm -scale-to`. Bounds each rendered + * page's JPEG size — and thus its base64 payload and vision-token cost — + * independently of the PDF's physical page dimensions. Mirrors claude-code's + * page-as-image rendering. NOTE: `-scale-to` overrides `-r`, so the two must + * never be combined. + */ +export const PDF_RENDER_SCALE_TO_PX = 1600; +/** + * Upper bound on the summed base64 size of all pages returned from one render + * call. Rendering up to PDF_MAX_PAGES_PER_READ pages at ~1-2 MB each could + * otherwise produce a tool result tens of MB large; once this is reached the + * remaining pages are dropped and the caller is told (never silently). Sized + * well above any single 1600px JPEG so the first page always survives. + */ +const PDF_RENDER_MAX_TOTAL_BASE64_BYTES = 25 * 1024 * 1024; +/** Timeout for a single pdftoppm render invocation. */ +const PDF_RENDER_TIMEOUT_MS = 120_000; // Upper bound on a page number we're willing to forward to pdftotext. // Sits well below Number.MAX_SAFE_INTEGER so arithmetic in validation // (e.g. lastPage - firstPage + 1) stays exact, and well above any real @@ -399,3 +422,187 @@ export async function extractPDFText( }; } } + +let pdftoppmAvailable: boolean | undefined; +let pdftoppmAvailablePromise: Promise | undefined; + +/** + * Check whether `pdftoppm` (from poppler-utils) is available. Mirrors + * {@link isPdftotextAvailable}: the result and the in-flight probe promise are + * cached for the process lifetime so concurrent render callers share one probe. + */ +export async function isPdftoppmAvailable(): Promise { + if (pdftoppmAvailable !== undefined) return pdftoppmAvailable; + if (pdftoppmAvailablePromise) return pdftoppmAvailablePromise; + + pdftoppmAvailablePromise = (async () => { + try { + const { code } = await execCommand('pdftoppm', ['-v'], { + timeout: 5000, + }); + return code === 0; + } catch { + return false; + } + })() + .then((result) => { + pdftoppmAvailable = result; + return result; + }) + .finally(() => { + pdftoppmAvailablePromise = undefined; + }); + + return pdftoppmAvailablePromise; +} + +/** + * Reset the pdftoppm availability cache. Used by tests only. + */ +export function resetPdftoppmCache(): void { + pdftoppmAvailable = undefined; + pdftoppmAvailablePromise = undefined; +} + +export interface PDFRenderedImage { + /** base64-encoded JPEG data (no `data:` URI prefix). */ + data: string; + mimeType: string; +} + +export type PDFRenderResult = + | { success: true; images: PDFRenderedImage[]; bytesTruncated: boolean } + | { success: false; error: string }; + +/** + * Compare two pdftoppm output filenames (e.g. "page-1.jpg", "page-10.jpg") by + * their trailing page number, so page 10 sorts after page 2 regardless of the + * zero-padding width pdftoppm chooses (which depends on the page count). + */ +function comparePdfPageFilenames(a: string, b: string): number { + const pageNumber = (name: string): number => { + const match = /(\d+)\.jpg$/i.exec(name); + return match ? parseInt(match[1]!, 10) : 0; + }; + return pageNumber(a) - pageNumber(b); +} + +/** + * Render PDF pages to JPEG images using `pdftoppm` (from poppler-utils). Each + * page becomes one base64 JPEG whose longest edge is capped at + * {@link PDF_RENDER_SCALE_TO_PX}, giving a bounded token cost per page + * regardless of text density — the fallback path when text extraction + * overflows or fails on a vision-capable model. + * + * @param filePath Path to the PDF file. + * @param options Optional 1-indexed inclusive page range. Omit `firstPage` to + * render from the start; an `Infinity` `lastPage` renders through the end. + */ +export async function renderPDFPagesToImages( + filePath: string, + options?: { firstPage?: number; lastPage?: number }, +): Promise { + const available = await isPdftoppmAvailable(); + if (!available) { + return { success: false, error: PDF_RENDER_UNAVAILABLE_MESSAGE }; + } + + let tempDir: string | undefined; + try { + tempDir = await mkdtemp(join(tmpdir(), 'pdf-render-')); + const outputPrefix = join(tempDir, 'page'); + + const args: string[] = [ + '-jpeg', + '-scale-to', + String(PDF_RENDER_SCALE_TO_PX), + ]; + if (options?.firstPage) { + args.push('-f', String(options.firstPage)); + } + if (options?.lastPage && options.lastPage !== Infinity) { + args.push('-l', String(options.lastPage)); + } + // `--` separates options from positional args so a filename starting with + // `-` isn't misread as an option by poppler's parser. + args.push('--', filePath, outputPrefix); + + const { stderr, code, timedOut } = await execCommand('pdftoppm', args, { + timeout: PDF_RENDER_TIMEOUT_MS, + }); + + if (timedOut) { + return { + success: false, + error: `pdftoppm timed out after ${Math.round( + PDF_RENDER_TIMEOUT_MS / 1000, + )}s. The PDF may be unusually large or complex; try the 'pages' parameter to narrow the range.`, + }; + } + + if (code !== 0) { + if (/password/i.test(stderr)) { + return { + success: false, + error: + 'PDF is password-protected. Please provide an unprotected version.', + }; + } + if (/damaged|corrupt|invalid/i.test(stderr)) { + return { success: false, error: 'PDF file is corrupted or invalid.' }; + } + return { + success: false, + error: `pdftoppm failed: ${stderr || '(no stderr)'}`, + }; + } + + // The temp dir is fresh and holds only this call's output, so reading and + // numerically sorting whatever pdftoppm produced is robust to its + // zero-padding width. + const entries = (await readdir(tempDir)) + .filter((name) => name.toLowerCase().endsWith('.jpg')) + .sort(comparePdfPageFilenames); + + if (entries.length === 0) { + return { + success: false, + error: + 'pdftoppm produced no image output. The PDF may be empty or the page range may be out of bounds.', + }; + } + + const images: PDFRenderedImage[] = []; + let totalBytes = 0; + let bytesTruncated = false; + for (const name of entries) { + const buffer = await readFile(join(tempDir, name)); + const data = buffer.toString('base64'); + // Always keep the first page; afterwards stop before exceeding the cap so + // one tool result can't balloon to tens of MB. + if ( + images.length > 0 && + totalBytes + data.length > PDF_RENDER_MAX_TOTAL_BASE64_BYTES + ) { + bytesTruncated = true; + break; + } + totalBytes += data.length; + images.push({ data, mimeType: 'image/jpeg' }); + } + + return { success: true, images, bytesTruncated }; + } catch (e: unknown) { + return { + success: false, + error: `pdftoppm execution failed: ${ + e instanceof Error ? e.message : String(e) + }`, + }; + } finally { + if (tempDir) { + // Best-effort cleanup; never let a cleanup failure mask the result. + await rm(tempDir, { recursive: true, force: true }).catch(() => {}); + } + } +} diff --git a/packages/core/src/utils/readManyFiles.ts b/packages/core/src/utils/readManyFiles.ts index 7d12b26dca..11800c3801 100644 --- a/packages/core/src/utils/readManyFiles.ts +++ b/packages/core/src/utils/readManyFiles.ts @@ -267,8 +267,13 @@ async function readFileContent( }; } - // For binary files (images, PDFs), add prefix text before the inlineData/fileData part - const contentParts: Part[] = [prefixText, fileReadResult.llmContent]; + // For binary files (images, PDFs), add prefix text before the media + // part(s). A page-rendered PDF yields an array of image parts (plus an + // optional truncation note), so flatten it after the prefix. + const mediaParts = fileReadResult.llmContent; + const contentParts: Part[] = Array.isArray(mediaParts) + ? [prefixText, ...(mediaParts as Part[])] + : [prefixText, mediaParts]; return { contentParts, info: {