Skip to content

Commit cb6c842

Browse files
authored
feat(knowledge): read a PDF's text layer before paying for OCR (#6850)
* feat(knowledge): read a PDF's text layer before paying for OCR Every PDF went to OCR, an external per-document call, even though most carry an embedded text layer that costs nothing to read. Across a real corpus of 2,693 documents, local extraction produced text for every PDF that OCR could also read, so the great majority of those calls bought nothing. A PDF's text layer is now read first and used when it is good enough, leaving OCR for the documents that actually need it. Three ways a layer fails, none of which catches the others: there is no text at all (a scan), the text is too sparse to be the document, or there is plenty of text that is not language — a broken encoding, or the raw character ids a CID-keyed font emits with no ToUnicode map, which is common in exactly the contract and procurement material that reaches a knowledge base and which a length check alone reads as healthy. Beyond the cost, this narrows an availability dependency: an OCR outage no longer touches every PDF, only the minority that cannot be read locally. The threshold is env-tunable so the balance can be moved toward cost or fidelity without a deploy. Known limitation: the judgement is per document, so a file mixing typeset pages with scanned inserts can average above the threshold and keep its partial text. Per-page routing would catch it and needs per-page extraction this does not have. The opaque-input refusal now asserts against the outbound request rather than the storage read: local parsing is not model input, so bytes are read before the projection is checked and still never leave the worker when it refuses. * fix(knowledge): route a truncated PDF extraction to OCR, and drop the threshold env var Two corrections to the text-layer triage. A parser limit stops extraction partway and reports `truncated`. Such a result has plenty of text by volume, so every volume-based check read it as healthy and the document was indexed as a fragment with the remainder silently missing from search. Truncation is now judged before anything that measures volume, and sends the document to OCR, which reads it whole. The characters-per-page threshold is a plain constant again. It read `process.env` directly rather than going through the env module, and the tunable was not worth having: a typeset page carries roughly 1,500-3,000 characters and a scan carries none, so the value sits in a wide gap where no realistic tuning changes an outcome. A constant is one less piece of configuration that can be set wrong, and if the threshold is ever wrong the fix is to change it. * fix(knowledge): take the page count from the parse that produced the text The density check counted pages with a second, independent read of the file. The two could disagree: a count that failed reported no pages, the check fell back to treating the document as a single page, and a long scan carrying only a header looked dense enough to skip OCR and be indexed as that header. `parseBuffer` already reports the page count from the parse that produced the text, so the two can no longer diverge, and the redundant second open of the file goes away with it. * fix(knowledge): chunk a long PDF for Azure OCR instead of refusing it Both OCR providers cap how many pages a single request may carry, and both were handling that cap differently: one split the document to fit, the other rejected any document over it. A long PDF could therefore be ingested on one provider and not at all on the other, for a limit that belongs to a request rather than to a document. The splitting, concurrency, ordering and partial-failure rule now live in one place that both providers call, so they cannot drift apart again. A chunk that fails is dropped rather than failing the document — losing one section of a long document beats losing all of it — and every chunk failing still throws. Also drops the unpdf mock from the triage tests. It was masking real behaviour: the page count now comes from the parse metadata, so the mock was no longer needed, and while it was in place a test asserting the old page-cap refusal passed against both the old and new code. * fix(knowledge): keep an unsplittable PDF and an empty OCR response honest Two regressions from chunking the Azure path. Splitting loads the document, which an encrypted or malformed PDF refuses, and that failure was deciding whether the file reached OCR at all. Those are exactly the documents the triage routes here — no readable text layer — and the provider may well accept bytes a local parser will not, so a failed split now sends the document whole and leaves the page cap to the provider, as it did before it was chunked. An Azure response carrying no pages fell back to the raw API payload as content. Chunked, that payload counted as recovered text and was stitched into the document; unchunked, it satisfied the empty-content check written to catch this. No pages is now no content, so the chunk counts as failed and the document reports it. * fix(knowledge): fail a PDF whose OCR only partly came back A chunked OCR run dropped any chunk that failed and returned the rest as a normal success, so the document was marked complete with whole page ranges absent from search and nothing downstream could tell the difference. That contradicted the rule this change set already applies to a truncated text layer, which is sent to OCR precisely because indexing a fragment while reporting success is the failure being removed. A document is now indexed whole or not at all: any missing chunk fails it, leaving it visible with a reason and eligible for the stuck-document sweep, which can retry and produce a complete result. Each chunk has already exhausted its own retries, so a missing one is a real failure rather than a blip. The page-cap test mocked fetch with a single Response object, whose body can only be read once — the second chunk was failing on "Body already read" and the lenient path hid it. It now returns a fresh response per call.
1 parent 10ff622 commit cb6c842

5 files changed

Lines changed: 694 additions & 82 deletions

File tree

apps/sim/lib/knowledge/documents/document-processor-secret-provenance.test.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,17 @@ describe('knowledge document model-input provenance', () => {
9191
expect(fetchMock).not.toHaveBeenCalled()
9292
})
9393

94+
/**
95+
* The refusal guards egress to an external model, so it is asserted against the
96+
* outbound request rather than the storage read. A PDF is now parsed locally
97+
* first and only reaches OCR when it has no usable text layer — local parsing is
98+
* not model input, as the case above establishes — so the bytes are read before
99+
* the projection is checked, and never leave the worker when it refuses.
100+
*/
94101
it('rejects secret-bearing opaque document bytes before external OCR', async () => {
102+
const fetchMock = vi.fn()
103+
vi.stubGlobal('fetch', fetchMock)
104+
95105
await expect(
96106
runWithKnowledgeModelInputProvenance(
97107
undefined,
@@ -109,7 +119,7 @@ describe('knowledge document model-input provenance', () => {
109119
)
110120
).rejects.toThrow('Knowledge model input could not be safely projected')
111121

112-
expect(mockDownloadFileFromUrl).not.toHaveBeenCalled()
122+
expect(fetchMock).not.toHaveBeenCalled()
113123
})
114124

115125
it('attaches exact-empty provenance to the internal Mistral OCR request', async () => {

apps/sim/lib/knowledge/documents/document-processor.ts

Lines changed: 220 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
resolveParserExtension,
2323
resolveStoredArtifactExtension,
2424
} from '@/lib/knowledge/documents/parser-extension'
25+
import { assessPdfTextLayer } from '@/lib/knowledge/documents/pdf-text-layer'
2526
import { retryWithExponentialBackoff } from '@/lib/knowledge/documents/utils'
2627
import {
2728
assertKnowledgeOpaqueModelInputSafe,
@@ -295,6 +296,68 @@ async function getMistralApiKey(workspaceId?: string | null): Promise<string | n
295296
return env.MISTRAL_API_KEY || null
296297
}
297298

299+
/**
300+
* Reads a PDF's embedded text layer, returning it only when it is good enough to
301+
* index — otherwise `undefined`, leaving the caller to fall through to OCR.
302+
*
303+
* A failure to parse is not an error here: an encrypted or malformed PDF simply
304+
* has no usable layer, which is precisely a case for OCR. The document is fetched
305+
* again on that path, a second read from our own storage, which is a cheap price
306+
* for keeping the two extraction routes independent.
307+
*/
308+
async function readEmbeddedPdfText(
309+
fileUrl: string,
310+
filename: string,
311+
mimeType: string,
312+
userId?: string
313+
): Promise<
314+
| {
315+
content: string
316+
processingMethod: 'file-parser'
317+
cloudUrl?: string
318+
metadata?: FileParseMetadata
319+
}
320+
| undefined
321+
> {
322+
try {
323+
const buffer = await downloadFileWithTimeout(fileUrl, userId)
324+
const parsed = await parseBuffer(buffer, 'pdf')
325+
326+
/**
327+
* The page count comes from the same parse as the text, rather than a second
328+
* independent read of the file. Counting separately lets the two disagree: a
329+
* count that failed would report no pages, the density check would fall back to
330+
* treating the document as a single page, and a long scan carrying only a header
331+
* would look dense enough to skip OCR and be indexed as that header.
332+
*/
333+
const pageCount = parsed.metadata?.pageCount ?? 0
334+
const verdict = assessPdfTextLayer(parsed.content, pageCount, parsed.metadata?.truncated)
335+
if (!verdict.usable) {
336+
logger.info('PDF text layer not usable, routing to OCR', {
337+
filename,
338+
pageCount,
339+
reason: verdict.reason,
340+
})
341+
return undefined
342+
}
343+
344+
logger.info('Using embedded PDF text layer', { filename, pageCount })
345+
return {
346+
content: parsed.content,
347+
processingMethod: 'file-parser',
348+
cloudUrl: undefined,
349+
metadata: parsed.metadata,
350+
}
351+
} catch (error) {
352+
logger.info('Could not read PDF text layer, routing to OCR', {
353+
filename,
354+
mimeType,
355+
error: toError(error).message,
356+
})
357+
return undefined
358+
}
359+
}
360+
298361
async function parseDocument(
299362
fileUrl: string,
300363
filename: string,
@@ -319,14 +382,23 @@ async function parseDocument(
319382
MISTRAL_API_KEY: mistralApiKey,
320383
}).providerId
321384

322-
if (ocrProvider === 'azure-mistral') {
323-
assertKnowledgeOpaqueModelInputSafe()
324-
logger.info('Using Azure Mistral OCR')
325-
return parseWithAzureMistralOCR(fileUrl, filename, mimeType, userId)
326-
}
385+
if (ocrProvider === 'azure-mistral' || ocrProvider === 'mistral') {
386+
/**
387+
* Most PDFs carry a usable text layer, and reading it costs nothing. OCR is
388+
* a per-document call to an external service, so it is reserved for the
389+
* documents that actually need it — which also means everything else stops
390+
* depending on that service being reachable.
391+
*/
392+
const embedded = await readEmbeddedPdfText(fileUrl, filename, mimeType, userId)
393+
if (embedded) return embedded
327394

328-
if (ocrProvider === 'mistral') {
329395
assertKnowledgeOpaqueModelInputSafe()
396+
397+
if (ocrProvider === 'azure-mistral') {
398+
logger.info('Using Azure Mistral OCR')
399+
return parseWithAzureMistralOCR(fileUrl, filename, mimeType, userId)
400+
}
401+
330402
logger.info('Using Mistral OCR')
331403
return parseWithMistralOCR(fileUrl, filename, mimeType, userId, workspaceId, mistralApiKey)
332404
}
@@ -522,42 +594,19 @@ async function parseWithAzureMistralOCR(
522594

523595
const fileBuffer = await downloadFileForBase64(fileUrl, userId)
524596

525-
if (mimeType === 'application/pdf') {
526-
const pageCount = await getPdfPageCount(fileBuffer)
527-
if (pageCount > MISTRAL_MAX_PAGES) {
528-
throw new Error(
529-
`PDF has ${pageCount} pages, exceeding the Azure OCR limit of ${MISTRAL_MAX_PAGES}`
530-
)
531-
}
532-
logger.info('Azure Mistral OCR: PDF page count resolved', { pageCount })
533-
}
534-
535-
const base64Data = fileBuffer.toString('base64')
536-
const dataUri = `data:${mimeType};base64,${base64Data}`
537-
538597
try {
539-
const response = await retryWithExponentialBackoff(
540-
() =>
541-
makeOCRRequest(
542-
env.OCR_AZURE_ENDPOINT!,
543-
{
544-
'Content-Type': 'application/json',
545-
Authorization: `Bearer ${env.OCR_AZURE_API_KEY}`,
546-
},
547-
{
548-
model: env.OCR_AZURE_MODEL_NAME!,
549-
document: {
550-
type: 'document_url',
551-
document_url: dataUri,
552-
},
553-
include_image_base64: false,
554-
}
555-
),
556-
{ maxRetries: 3, initialDelayMs: 1000, maxDelayMs: 10000 }
557-
)
558-
559-
const ocrResult = (await response.json()) as AzureOCRResponse
560-
const content = extractPageContent(ocrResult.pages || []) || JSON.stringify(ocrResult, null, 2)
598+
/**
599+
* A PDF is chunked to the provider's page cap rather than refused for
600+
* exceeding it, matching the other OCR provider. Refusing meant a long
601+
* document could not be ingested at all, and the cap applies to a single
602+
* request, not to the document.
603+
*/
604+
const content =
605+
mimeType === 'application/pdf'
606+
? await ocrPdfInChunks(fileBuffer, 'azure-mistral', (chunk) =>
607+
recognizeWithAzureOCR(chunk.buffer, mimeType)
608+
)
609+
: await recognizeWithAzureOCR(fileBuffer, mimeType)
561610

562611
if (!content.trim()) {
563612
throw new Error('Azure Mistral OCR returned empty content')
@@ -573,6 +622,41 @@ async function parseWithAzureMistralOCR(
573622
}
574623
}
575624

625+
/** Sends one document to Azure Mistral OCR inline, as a base64 data URI. */
626+
async function recognizeWithAzureOCR(buffer: Buffer, mimeType: string): Promise<string> {
627+
const dataUri = `data:${mimeType};base64,${buffer.toString('base64')}`
628+
629+
const response = await retryWithExponentialBackoff(
630+
() =>
631+
makeOCRRequest(
632+
env.OCR_AZURE_ENDPOINT!,
633+
{
634+
'Content-Type': 'application/json',
635+
Authorization: `Bearer ${env.OCR_AZURE_API_KEY}`,
636+
},
637+
{
638+
model: env.OCR_AZURE_MODEL_NAME!,
639+
document: {
640+
type: 'document_url',
641+
document_url: dataUri,
642+
},
643+
include_image_base64: false,
644+
}
645+
),
646+
{ maxRetries: 3, initialDelayMs: 1000, maxDelayMs: 10000 }
647+
)
648+
649+
const ocrResult = (await response.json()) as AzureOCRResponse
650+
651+
/**
652+
* A response carrying no pages is no content. Returning the raw payload instead
653+
* would be indexed as though it were the document: stitched into a chunked run
654+
* as recovered text, and in a single-document run it would satisfy the
655+
* empty-content check that exists to catch exactly this.
656+
*/
657+
return extractPageContent(ocrResult.pages || [])
658+
}
659+
576660
async function parseWithMistralOCR(
577661
fileUrl: string,
578662
filename: string,
@@ -740,63 +824,118 @@ async function processChunk(
740824
}
741825
}
742826

743-
async function processMistralOCRInBatches(
744-
filename: string,
745-
apiKey: string,
827+
/**
828+
* Runs a PDF through OCR a chunk at a time and stitches the pages back together.
829+
*
830+
* A provider that caps how many pages one request may carry needs the document
831+
* split, and both providers cap at the same limit — so the splitting, the
832+
* concurrency, the ordering and the partial-failure rule live here once rather
833+
* than being restated per provider, where they had already drifted into one
834+
* provider chunking and the other refusing anything over the cap.
835+
*
836+
* A document is indexed whole or not at all: if any chunk fails, the document
837+
* fails, because a partial result reports success while page ranges are missing
838+
* and nothing downstream can tell.
839+
*/
840+
async function ocrPdfInChunks(
746841
pdfBuffer: Buffer,
747-
userId?: string,
748-
cloudUrl?: string
749-
): Promise<{
750-
content: string
751-
processingMethod: 'mistral-ocr'
752-
cloudUrl?: string
753-
}> {
842+
provider: string,
843+
recognize: (
844+
chunk: { buffer: Buffer; startPage: number; endPage: number },
845+
chunkIndex: number,
846+
totalChunks: number
847+
) => Promise<string | null>
848+
): Promise<string> {
754849
const totalPages = await getPdfPageCount(pdfBuffer)
755-
logger.info(`Splitting PDF into chunks`, { totalPages, maxPagesPerChunk: MISTRAL_MAX_PAGES })
756850

757-
const pdfChunks = await splitPdfIntoChunks(pdfBuffer, MISTRAL_MAX_PAGES)
758-
logger.info(
759-
`Split into ${pdfChunks.length} chunks, processing with concurrency ${MAX_CONCURRENT_CHUNKS}`
760-
)
851+
/**
852+
* Splitting has to load the document, which an encrypted or malformed PDF will
853+
* refuse. That must not decide whether the file reaches OCR at all: those are
854+
* exactly the documents with no readable text layer, so OCR is their only route,
855+
* and the provider may well accept bytes that a local parser would not. When the
856+
* split fails the document is sent whole and the page cap is left to the
857+
* provider — the behaviour before it was chunked.
858+
*/
859+
let pdfChunks: { buffer: Buffer; startPage: number; endPage: number }[]
860+
try {
861+
pdfChunks = await splitPdfIntoChunks(pdfBuffer, MISTRAL_MAX_PAGES)
862+
} catch (error) {
863+
logger.info('PDF could not be split for OCR, sending it whole', {
864+
provider,
865+
error: toError(error).message,
866+
})
867+
pdfChunks = [{ buffer: pdfBuffer, startPage: 0, endPage: Math.max(0, totalPages - 1) }]
868+
}
869+
870+
logger.info('Splitting PDF for OCR', {
871+
provider,
872+
totalPages,
873+
chunks: pdfChunks.length,
874+
maxPagesPerChunk: MISTRAL_MAX_PAGES,
875+
concurrency: MAX_CONCURRENT_CHUNKS,
876+
})
761877

762878
const results: { index: number; content: string | null }[] = []
763879

764880
for (let i = 0; i < pdfChunks.length; i += MAX_CONCURRENT_CHUNKS) {
765881
const batch = pdfChunks.slice(i, i + MAX_CONCURRENT_CHUNKS)
766-
const batchPromises = batch.map((chunk, batchIndex) =>
767-
processChunk(chunk, i + batchIndex, pdfChunks.length, filename, apiKey, userId)
768-
)
769-
770-
const batchResults = await Promise.all(batchPromises)
771-
for (const result of batchResults) {
772-
results.push(result)
773-
}
774-
775-
logger.info(
776-
`Completed batch ${Math.floor(i / MAX_CONCURRENT_CHUNKS) + 1}/${Math.ceil(pdfChunks.length / MAX_CONCURRENT_CHUNKS)}`
882+
const batchResults = await Promise.all(
883+
batch.map((chunk, batchIndex) => {
884+
const index = i + batchIndex
885+
return recognize(chunk, index, pdfChunks.length).then(
886+
(content) => ({ index, content }),
887+
(error) => {
888+
logger.warn('OCR chunk failed', {
889+
provider,
890+
chunk: index + 1,
891+
error: toError(error).message,
892+
})
893+
return { index, content: null }
894+
}
895+
)
896+
})
777897
)
898+
results.push(...batchResults)
778899
}
779900

780-
const sortedResults = results
901+
const recovered = results
781902
.sort((a, b) => a.index - b.index)
782-
.filter((r) => r.content !== null)
783-
.map((r) => r.content as string)
784-
785-
if (sortedResults.length === 0) {
903+
.map((r) => r.content)
904+
.filter((content): content is string => content !== null && content.trim().length > 0)
905+
906+
/**
907+
* Each chunk has already exhausted its own retries, so a missing one is a real
908+
* failure rather than a blip. Failing the document leaves it visible with a
909+
* reason and eligible for the stuck-document sweep, which can retry it and
910+
* produce a complete result — whereas indexing what came back would be
911+
* indistinguishable from a document that never had those pages.
912+
*/
913+
if (recovered.length < pdfChunks.length) {
786914
throw new Error(
787-
`OCR failed for all ${pdfChunks.length} chunks. ` +
788-
`Large PDFs require OCR - file parser fallback would produce poor results.`
915+
`OCR recovered ${recovered.length} of ${pdfChunks.length} chunks; ` +
916+
'indexing the document would omit the rest'
789917
)
790918
}
791919

792-
const combinedContent = sortedResults.join('\n\n')
793-
logger.info(`Successfully processed ${sortedResults.length}/${pdfChunks.length} chunks`)
920+
return recovered.join('\n\n')
921+
}
794922

795-
return {
796-
content: combinedContent,
797-
processingMethod: 'mistral-ocr',
798-
cloudUrl,
799-
}
923+
async function processMistralOCRInBatches(
924+
filename: string,
925+
apiKey: string,
926+
pdfBuffer: Buffer,
927+
userId?: string,
928+
cloudUrl?: string
929+
): Promise<{
930+
content: string
931+
processingMethod: 'mistral-ocr'
932+
cloudUrl?: string
933+
}> {
934+
const content = await ocrPdfInChunks(pdfBuffer, 'mistral', (chunk, index, total) =>
935+
processChunk(chunk, index, total, filename, apiKey, userId).then((r) => r.content)
936+
)
937+
938+
return { content, processingMethod: 'mistral-ocr', cloudUrl }
800939
}
801940

802941
/**

0 commit comments

Comments
 (0)