fix(platform): restore pdf image detection on scanned files - #3103
Closed
Israeltheminer wants to merge 1 commit into
Closed
fix(platform): restore pdf image detection on scanned files#3103Israeltheminer wants to merge 1 commit into
Israeltheminer wants to merge 1 commit into
Conversation
Collaborator
Author
|
Superseded by #3170, which carries the same fix against the current tree. Both files this branch touches were deleted by #3125. The polyfill module itself was re-homed to #3018 stays open until #3170 lands. One thing worth recording from redoing it: Bun ships |
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scanned PDFs index thin or empty because image detection never runs. This restores it.
Closes #3018.
Why
pdfjs 5.x calls
Map.prototype.getOrInsertComputed, which Node 22 does not ship, so the operator-list read throws once per page:The damage is narrower than the log suggests, and worse where it lands. Text-layer extraction (
getTextContent) still runs, so an ordinary digital PDF indexes fine and only produces noise. But the operator list is what finds embedded images and decides "scanned page → OCR / describe". Without it a scanned or image-heavy PDF never reaches OCR, and indexes as thin or empty text with nothing but a warning to say why.What changed
One shim beside the existing ES2025 fills in
pdfjs_dom_polyfill.ts, guarded like the rest so a real implementation wins once the runtime moves to Node ≥24.Scope is
Maponly, from the library rather than from the proposal: every call site in both the main and worker builds —_intentStates,methodPromises,objs,_cachedBitmapsMap,popupToElements— is a plainMap. NoWeakMapcall exists, so none is shimmed.Deliberately NOT the legacy pdfjs build. That would resolve the same call, and the loader already avoids it for Convex bundle size.
Risk
Silencing the warning would have been the wrong fix. Suppressing it without restoring the operator list leaves scanned PDFs indexing badly with nothing to explain it — a quality loss that no longer announces itself.
Presence, not truthiness. A callback returning
undefinedstill inserts, so a second call does not recompute. Getting this wrong would be invisible until a cache entry legitimately heldundefined.The callable check runs before the lookup, as the proposal specifies. Without that ordering a present key returns happily and a bad callback goes unnoticed until some later call happens to miss.
Tests
Seven cases: the method is installed, compute-and-insert, an existing value returned without calling the callback,
undefinedstored as a real value, the computed value winning when the callback inserted the key itself, a non-callable callback rejected for both a present and an absent key, and a real implementation left alone.Four deliberate breakages, all caught. One survived the first round: dropping the callable check still threw, because calling a string throws
TypeErroranyway — the case that distinguishes them is a present key, which is now tested.The last case installs a stand-in for a native implementation and removes it in a
finally, because the guard correctly refuses to replace it and every later suite in that worker would otherwise inherit it.Scope
Does not change extraction, OCR or chunking. Scanned PDFs indexed before this reach the corpus with whatever thin text they produced; re-indexing them is a separate step.
Gate: repo-wide typecheck,
oxlint --type-aware, oxfmt, knip, SAST 0 findings, platform suite 75,819 passing.