Greptile Summary

Wires up standardFontDataUrl in the getDocument() call inside extractPdfContent, pointing to the bundled pdfjs-dist/standard_fonts/ directory resolved via createRequire. This fixes a breakage where PDFs referencing any of the 14 standard PDF fonts produced empty text extraction and downstream model errors. The path resolution is wrapped in a try/catch with a module-level cache, falling back to the previous undefined behavior if pdfjs-dist is somehow unreachable.

Confidence Score: 5/5

Safe to merge — single-file change with correct trailing-slash handling, graceful fallback on resolution failure, and no new dependencies or public API surface changes.

The fix is minimal and correct: path.join preserves the trailing slash so pathToFileURL produces a proper directory URL (file:///…/standard_fonts/), the module-level cache prevents repeated require.resolve overhead on every PDF, and the catch block restores prior behavior on failure. No P0 or P1 issues found.

No files require special attention.

_{Reviews (1): Last reviewed commit: "Media: fix PDF text extraction by settin..." | Re-trigger Greptile}

Update — verified end-to-end in a clean build sandbox

Followed up on the "did not run pnpm test/build locally" note in the original PR body. Built and tested the branch in a clean Ubuntu 24.04 LXC (Node v22.22.2, pnpm 10.33.0, fresh clone, no host pollution) and verified the fix end-to-end. Two follow-up commits were needed; both are now on the branch.

Follow-up commits

1. 126cbc3c00 — Media: type pdf.js getDocument params to allow standardFontDataUrl

   • pnpm build initially failed with TS2353: 'standardFontDataUrl' does not exist in type '{ data; disableWorker? }'. The legacy pdfjs-dist/legacy/build/pdf.mjs ships narrower .d.ts than the runtime accepts — DocumentInitParameters has standardFontDataUrl but the legacy build's inline types only declare { data, disableWorker }.
   • Fix: extend the inferred parameter type structurally with Parameters<typeof getDocument>[0] & { standardFontDataUrl?: string | URL } and pass it as a typed local. No any cast — respects the repo's "prefer real types" rule.

2. 6624cb128e — Media: rename pdf-extract local var to avoid shadowing function param

   • First fix introduced a params local that shadowed the function's own params argument (Identifier 'params' has already been declared). Renamed the local to getDocumentParams. Pure rename, no behavior change.

Verification on the final commit (6624cb128e)

Build (pnpm build):

• pdf-extract.ts compiles cleanly. Runtime tsdown emits dist/pdf-extract-Bm_cn2JH.js.
• Two pre-existing errors on main reproduced both before and after my branch and are unrelated to this PR:
  • src/config/talk.ts(150,8): error TS2304: Cannot find name 'isPlainObject'
  • src/commands/doctor-state-integrity.ts(464,8): error TS2304: Cannot find name 'isRecord'
  • I verified by stashing my changes, checking out main, and rerunning pnpm build / pnpm tsgo — both errors are present on main. Not touching them in this PR.

Typecheck (pnpm tsgo):

• Zero new TypeScript errors introduced by this PR.
• The two pre-existing errors above are the only failures.

Targeted tests (pnpm vitest run --config vitest.media.config.ts):

• 24 test files, 207 tests, all passing in 3.29s. No regressions in the media pipeline.

Real-PDF runtime verification:

• Downloaded Mozilla pdf.js's canonical test fixture tracemonkey.pdf (the standard PDF used by the pdf.js project itself for regression testing — 6 pages, version 1.4, ~1 MB, references the standard fonts that originally triggered the bug).
• Imported extractPdfContent directly from the freshly-built dist/pdf-extract-Bm_cn2JH.js and ran it against the PDF inside the build sandbox.
• Result:

  {
    "textLength": 37975,
    "imageCount": 0,
    "textPreview": "Trace-based Just-in-Time Type Specialization for Dynamic Languages Andreas Gal ∗ + , Brendan Eich ∗ , Mike Shaver ∗ , David Anderson ∗ , David Mandelin ∗ , Mohammad R. Haghighat $ , Blake Kaplan ∗ , Graydon Hoare ∗ , Boris Zbarsky ∗ , Jason Orendorff",
    "fontWarnings": 0,
    "totalWarnings": 0,
    "warningSample": []
  }

• 37,975 characters extracted, zero standardFontDataUrl warnings, zero warnings of any kind. Compare with the original failure mode in the PR body where the same code path emitted Warning: UnknownErrorException: Ensure that the standardFontDataUrl API parameter is provided. once per page and yielded empty text.

Updated checklist

• pnpm build — succeeds for pdf-extract.ts (pre-existing unrelated talk.ts / doctor-state-integrity.ts errors only)
• pnpm tsgo — zero new errors from this PR
• pnpm vitest run --config vitest.media.config.ts — 207/207 passing
• Runtime PDF extraction verified against Mozilla pdf.js's canonical tracemonkey.pdf test fixture
• Fixture-based unit test — still happy to add src/media/pdf-extract.test.ts with tracemonkey.pdf (or another small fixture you'd prefer) if you'd like that included before merge.

Sandbox details for reproducibility: Ubuntu 24.04 LXC, Node v22.22.2, pnpm 10.32.1 (auto-bumped from 10.33.0 by the repo's engines resolution), fresh git clone of the fork, no global state.

Update 2 — second runtime bug caught + fixture test added

Two more commits on the branch. Sandbox testing turned up a real runtime bug in the original fix that no static check would have caught, and the new fixture test now locks in the regression so it cannot return.

Follow-up commits

3. 2535fa86cd — Media: pass pdf.js standardFontDataUrl as filesystem path on Node

   When I went to write the fixture test, I noticed pdf.js was still emitting one warning per call:

   Warning: UnknownErrorException: Unable to load font data at: file:///home/.../node_modules/pdfjs-dist/standard_fonts/LiberationSans-Regular.ttf
   

   The font file existed and was readable, so something was wrong with how pdf.js was resolving the URL. Tracing it down: pdf.js's Node-side standard-font fetcher reads via fs.promises.readFile and only handles plain filesystem paths with a trailing separator — it does not accept file:// URLs even though the public type allows string | URL. The original fix used pathToFileURL(fontDir).href which produced file:///... and silently failed at the font-load step.

   Fix: drop pathToFileURL, return the resolved directory as a plain path with the platform separator appended (join(dirname(pkgPath), "standard_fonts") + sep). The function is now getStandardFontDataPath and the cache variable is named accordingly. Renamed to make the path-vs-URL distinction explicit at the call site, and added an IMPORTANT: comment explaining the trap so future maintainers don't reintroduce it. This module is server-side only (the gateway runs in Node), so no environment branching is needed.

   This is the kind of bug a build sandbox catches that CI alone would not — pnpm build and pnpm tsgo were both green with the URL form because the type signature accepted it; the failure only surfaced at runtime with a real PDF.

4. a07a7a3a6f — Media: add pdf-extract test for standard-font Helvetica PDF

   New src/media/pdf-extract.test.ts colocated per repo convention, runs under vitest.media.config.ts. Two cases:

   • extracts text from a PDF that uses a standard font (Helvetica) — The exact regression. Embeds a 518-byte minimal valid PDF as a base64 constant (no binary fixture in the repo). The PDF declares Helvetica via /Type/Font/Subtype/Type1/BaseFont/Helvetica so it exercises the standard-font code path that originally failed. Asserts the extracted text contains "Hello PDF World", no images were emitted (so the canvas/image fallback path was not hit), and zero warnings mentioning standardFontDataUrl were captured during the call.
   • returns the extracted text even if the buffer is reused across calls — Sanity check that the cached standard-font path is reusable across multiple invocations and does not leak state between calls.

   The base64 PDF was generated by a deterministic builder and verified end-to-end against the freshly built dist/pdf-extract-*.js before being inlined into the test, so it's known to round-trip cleanly through pdf.js.

Verification on the final commit (a07a7a3a6f)

Build: pnpm build succeeds for pdf-extract.ts. Same two pre-existing unrelated errors on main (talk.ts:150, doctor-state-integrity.ts:464) — neither involves my files.

Targeted test: pnpm vitest run --config vitest.media.config.ts src/media/pdf-extract.test.ts

✓ media src/media/pdf-extract.test.ts (2 tests) 78ms

Test Files  1 passed (1)
     Tests  2 passed (2)

Full media suite: pnpm vitest run --config vitest.media.config.ts

Test Files  25 passed (25)
     Tests  209 passed (209)
  Duration  2.57s

(Was 24 / 207 before this PR; +1 file / +2 tests from my new test, no regressions.)

Updated checklist

• pnpm build — succeeds for pdf-extract.ts (only pre-existing unrelated errors remain)
• pnpm tsgo — zero new errors from this PR
• New colocated src/media/pdf-extract.test.ts — 2 tests, both passing
• Full vitest.media.config.ts suite — 209/209 passing, no regressions
• Runtime PDF extraction verified against the embedded fixture and Mozilla pdf.js's canonical tracemonkey.pdf

Final commit list on the branch (4 commits)

1. 18d98711e6 — Media: fix PDF text extraction by setting standardFontDataUrl
2. 126cbc3c00 — Media: type pdf.js getDocument params to allow standardFontDataUrl
3. 6624cb128e — Media: rename pdf-extract local var to avoid shadowing function param
4. 2535fa86cd — Media: pass pdf.js standardFontDataUrl as filesystem path on Node
5. a07a7a3a6f — Media: add pdf-extract test for standard-font Helvetica PDF

Happy to squash if maintainers prefer a single commit on landing. I kept them separate so the review history shows both bugs the sandbox testing caught (TS narrowing on the legacy build's .d.ts, and the silent file:// URL trap in pdf.js's Node font fetcher).

CI note: the current red checks on this PR are unrelated to the PDF change.

I pulled the failed run for commit a07a7a3, and the failures are in shared baseline areas:

• check-additional / checks-fast-contracts-protocol: boundary guard failure from extensions/acpx/src/acpx-runtime.d.ts importing ../../../src/acp/runtime/types.js
• checks-node-channels: acp-bindings.test.ts mocks ../../plugins/runtime.js but does not provide requireActivePluginChannelRegistry
• checks-node-test: three Slack threading tests time out in extensions/slack/src/monitor.threading.missing-thread-ts.test.ts

This PR only touches src/media/pdf-extract.ts and src/media/pdf-extract.test.ts.

I also checked the current main CI, and it is failing in the same boundary/contract area, so this looks like repo baseline breakage rather than a regression from this branch. Once the base is green again, I can rebase and rerun.

Update — rebased on main (2026-04-20)

Merged upstream/main into the branch (6f3986486f) to clear the 2-week conflict drift. Single conflict in src/media/pdf-extract.ts resolved as follows:

• Kept upstream's hand-rolled CanvasLike/PdfPage/PdfDocument/PdfJsModule interfaces and CANVAS_MODULE/PDFJS_MODULE constants from the recent type-system refactor.
• Extended upstream's PdfJsModule.getDocument signature to include standardFontDataUrl?: string, then dropped my previous Parameters<typeof getDocument>[0] & {...} widening trick at the call site — no longer needed now that we own the type locally.
• One incidental cleanup: String(err?.stack ?? "") → err?.stack ?? "" to satisfy the newly-added typescript-eslint/no-unnecessary-type-conversion rule.

Verified in a clean Ubuntu 24.04 LXC: pnpm install → pnpm vitest run src/media/pdf-extract.test.ts → 2/2 green. Full CI now clean on this branch.

Happy to squash to a single commit on landing, as originally offered.

Closing as stale. PDF extraction was refactored into extensions/document-extract/document-extractor.ts after this branch was opened, so the diff no longer applies. The underlying bug is still present (the new extractor at extensions/document-extract/document-extractor.ts:142 calls getDocument({ data, disableWorker: true }) with no standardFontDataUrl), but two follow-up attempts (#11427, #51465) were also closed without merge, so a fourth attempt against the same surface seems unproductive right now.