feat(knowledge): read a PDF's text layer before paying for OCR - #6850
Conversation
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview When OCR is still needed, Azure Mistral follows the same chunked PDF path as Mistral through shared Opaque model-input safety ( Reviewed by Cursor Bugbot for commit 0dabda6. Configure here. |
Greptile SummaryThe PR reads an embedded PDF text layer before using external OCR and includes complete-document safeguards for local extraction and chunked OCR.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/knowledge/documents/document-processor.ts | Adds local PDF text-layer triage, moves the provenance assertion to the OCR egress boundary, and makes shared OCR chunk processing all-or-nothing. |
| apps/sim/lib/knowledge/documents/pdf-text-layer.ts | Defines text-layer usability checks for truncation, text density, CID escapes, and unreadable encoding. |
| apps/sim/lib/knowledge/documents/pdf-ocr-triage.test.ts | Covers local-versus-OCR routing, Azure page-cap chunking, malformed-PDF fallback, empty responses, and partial chunk failure. |
| apps/sim/lib/knowledge/documents/pdf-text-layer.test.ts | Exercises the text-layer classifier across ordinary, sparse, truncated, malformed, and multilingual content. |
| apps/sim/lib/knowledge/documents/document-processor-secret-provenance.test.ts | Updates provenance coverage to assert that refused opaque PDF bytes never reach the external OCR request. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
PDF[PDF in object storage] --> Local[Parse embedded text layer locally]
Local --> Assess{Text layer usable?}
Assess -->|Yes| Parse[Use file-parser content]
Assess -->|No| Guard[Verify opaque model input is safe]
Guard --> OCR{Configured OCR provider}
OCR --> Mistral[Mistral OCR]
OCR --> Azure[Azure Mistral OCR]
Mistral --> Split[Split PDF into bounded chunks]
Azure --> Split
Split --> Complete{Every chunk recovered?}
Complete -->|Yes| Stitch[Stitch chunks in page order]
Complete -->|No| Fail[Fail document for retry]
Parse --> Chunk[Chunk and embed document]
Stitch --> Chunk
Reviews (6): Last reviewed commit: "fix(knowledge): fail a PDF whose OCR onl..." | Re-trigger Greptile
… 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.
|
@cursor review |
…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.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 47cd6e1. Configure here.
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.
|
@cursor review |
…nest 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.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit fc8af91. Configure here.
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.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 0dabda6. Configure here.
Every PDF is sent to OCR — an external, per-document call — even though most carry an embedded text layer that costs nothing to read. This reads the text layer first and reserves OCR for the documents that actually need it.
The evidence
Across a real corpus of 2,693 PDFs that exist both ways (locally extracted and OCR'd):
Separately, of ~1,480 PDFs in that library only ~9% genuinely lacked a text layer. So the large majority of OCR calls were buying nothing.
One caveat stated plainly: local extraction returned more characters than OCR on average (116K vs 14K), which is not local winning — pdfjs emits positioning noise where OCR returns clean markdown. Character volume demonstrates coverage, not fidelity. That is why the threshold is tunable and the routing is logged.
How a text layer is judged
Three independent failure modes, none of which catches the others. Taken from prior art rather than invented — pdfmux documents a text-density check, and docling#2963 describes the corrupted-layer cases:
ToUnicodemap (/31 /8 /18 /12). This one matters here: it produces plenty of characters, so a length check reads it as healthy, and it is common in documents from older generators and subset fonts — much of the contract and procurement material that reaches a knowledge base.KB_PDF_MIN_CHARS_PER_PAGE(default 100) moves the balance toward cost or fidelity without a deploy. A typeset page carries roughly 1,500–3,000 characters and a scan carries none, so the gap is wide and the threshold does not need to be precise.Beyond cost
This narrows an availability dependency. Previously an OCR outage failed every PDF; now it only reaches the minority that cannot be read locally. That materially reduces the blast radius of the one failure mode this design deliberately does not paper over with a silent fallback.
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 we do not currently have. Called out rather than hidden — it is the natural next refinement.
One behavioural change worth review attention
assertKnowledgeOpaqueModelInputSafenow runs after the local read rather than before it, because the local read is what decides whether OCR is needed at all.The guarantee is unchanged: bytes never reach an external model when the projection is refused. The refusal simply fires at the outbound request instead of the storage read, and its test now asserts against
fetchaccordingly. This is consistent with the case directly above it in that file, which establishes that local parsing is not model egress. It is also the better ordering — a secret-bearing PDF with a usable text layer is now indexed locally instead of being refused for an external call that never happens.Testing
vitest run lib/knowledge/ lib/uploads/ lib/file-parsers/ connectors/ app/api/knowledge/— 2,020 passed (131 files)pdf-text-layer.test.ts— 9 cases: typeset document, scan, sparse text, CID escapes, replacement characters, prose containing slashes and digits, accented and non-Latin text, unknown page count, and threshold scaling with lengthpdf-ocr-triage.test.ts— 4 routing cases: text layer used with no OCR call, and fallthrough on scan, CID escapes, and an unparseable PDF. Verified the first fails when the triage is disabledbun run check:audits— 29/29tsgo --noEmit— no errors in changed filesChecklist