feat(assets): PDF page rendering via raw: true

[aliasunder]

Summary

• When vault_read_asset is called with raw: true on a PDF, each page is rendered as an image via unpdf's renderPageAsImage + @napi-rs/canvas (prebuilt Skia, no system deps) and returned as MCP image content blocks
• The model sees actual page layout, formatting, diagrams, and tables — everything text extraction loses
• Scanned/image-only PDFs that error on text extraction work in raw mode
• New MAX_PDF_RENDER_PAGES env var (default 5) — per-page byte budget is MAX_IMAGE_OUTPUT_BYTES divided evenly across rendered pages

Changes

• Dependency: @napi-rs/canvas added (^0.1.69, ~40-50MB prebuilt Skia binary)
• Config: maxPdfRenderPages via MAX_PDF_RENDER_PAGES env var, requireNonZeroBytes → requireNonZero rename
• Core: renderPdfPages function (sequential rendering at 2× scale, per-page fitImageToByteBudget, individual page failures logged and skipped), AssetReadResult extended with kind: "pages" variant
• Handler: maps kind: "pages" to metadata text block + per-page image/text block pairs
• Tool description: PDF bullet updated with raw: true page rendering, new error entry, raw param description updated, returns section updated
• Tests: 10 new tests (page rendering, budget division, cap enforcement, partial failures, proxy cleanup, option passing, text extraction unchanged)
• Deploy surfaces: env var in all 7 compose/env files + CLI sync
• Docs: ARCHITECTURE.md, README.md config table, DOCKERHUB.md regenerated

Test plan

• npm run build — TypeScript compiles clean
• npm run lint — 0 errors
• npm test — 2027 tests pass (10 new)
• Test deploy via test_deploy.yml → live verification with a real PDF (raw: true + raw: false)
• Ship-check pipeline

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added support for reading PDFs in raw mode as rendered page images, including scanned and image-only PDFs.
  • Added a configurable limit for rendered PDF pages, set to 5 by default.
  • Preserved image size budgets by distributing them across rendered pages.

• Documentation

  • Updated usage and configuration guidance for PDF rendering and page limits.
  • Clarified PDF metadata, page rendering, and image-quality behavior.

• Bug Fixes

  • Improved handling of individual page-rendering failures while continuing when possible.

[sourcery-ai]

Sorry @aliasunder, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@​napi-rs/​canvas@​0.1.100

View full report

[umm-actually]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Guard against division by zero for empty PDFs When totalPages is 0 (an empty or malformed PDF), pagesToRender becomes 0 and perPageBudget becomes Infinity (division by zero in integer context actually yields Infinity in JS, not 0). This Infinity value would then be passed as budgetBytes to fitImageToByteBudget, which may behave unexpectedly. Add a guard that throws or returns an appropriate error when totalPages === 0 before computing perPageBudget. src/vault-mcp/vault-operations/asset-operations.ts [361-366] if (raw) { // Page rendering mode — render each page as an image const pagesToRender = Math.min(totalPages, params.maxPdfRenderPages) + if (pagesToRender === 0) { + throw new Error( + `PDF page rendering failed: "${path}" exists ` + + `(${asset.bytes} bytes, ${totalPages} pages) but no ` + + `pages could be rendered`, + ) + } const perPageBudget = Math.floor( params.maxImageOutputBytes / pagesToRender, ) Suggestion importance[1-10]: 6 __ Why: Valid edge case: when totalPages is 0, pagesToRender becomes 0 and Math.floor(params.maxImageOutputBytes / 0) yields Infinity in JavaScript, which would be passed as budgetBytes to fitImageToByteBudget. The fix correctly adds a guard before the division. However, this is an unlikely edge case since a PDF with 0 pages would be malformed, and the existing error path for pages.length === 0 would still catch it downstream.	Low
General	Eliminate budget/page-count mismatch from re-derivation pagesToRender is computed inside renderPdfPages as Math.min(params.totalPages, params.maxPages), but the caller already computed pagesToRender = Math.min(totalPages, params.maxPdfRenderPages) and passes both totalPages and maxPdfRenderPages separately. This means the budget division in the caller uses pagesToRender but renderPdfPages re-derives it independently — if the two Math.min calls ever diverge (e.g. due to a refactor), the per-page budget will be computed for a different number of pages than are actually rendered, silently over- or under-allocating the budget. Pass pagesToRender directly instead of re-deriving it inside the helper. src/vault-mcp/vault-operations/asset-operations.ts [119-159] const renderPdfPages = async ( params: { proxy: ReturnType<typeof getDocumentProxy> extends Promise<infer P> ? P : never - totalPages: number - maxPages: number + pagesToRender: number perPageBudget: number }, logger: Logger, ): Promise< Array<{ pageNumber: number; fitted: FittedImage; originalBytes: number }> > => { - const pagesToRender = Math.min(params.totalPages, params.maxPages) - ... - for (let pageNumber = 1; pageNumber <= pagesToRender; pageNumber++) { + for (let pageNumber = 1; pageNumber <= params.pagesToRender; pageNumber++) { Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies that pagesToRender is computed twice — once in the caller and once inside renderPdfPages — which could lead to a mismatch if refactored. However, the existing_code snippet contains ... which doesn't match the actual PR diff exactly, and the current code is consistent since both use Math.min(totalPages, maxPdfRenderPages). This is a minor maintainability concern rather than a current bug.	Low
	Fix misleading test name vs. actual assertion The test name says "shows (untitled)" but the assertion only checks result.title is undefined — it doesn't verify how the tool handler formats the metadata line when title is absent (i.e. that titleSegment is "" and the metadata line omits the title). The test name implies a display-level check but only validates the data model. Either rename the test to "omits title field when PDF has no title" to match what it actually asserts, or add an assertion on the rendered metadata text to match the stated intent. src/vault-mcp/vault-operations/tests/asset-operations.test.ts [728-741] -it("shows (untitled) in pages result when title is absent", async () => { +it("omits title field when PDF has no title", async () => { setupPdfMocks({ numPages: 1 }) mockRenderPageAsImage.mockResolvedValue(new ArrayBuffer(1_000)) mockedFitImage.mockResolvedValue(buildFittedImage()) const result = await assetOperations.readAssetContent( { ...defaultParams, path: "notitle.pdf", raw: true }, logger, ) expect(result.kind).toBe("pages") if (result.kind !== "pages") throw new Error("unreachable") expect(result.title).toBeUndefined() }) Suggestion importance[1-10]: 2 __ Why: The suggestion correctly notes the test name "shows (untitled)" implies a display-level check but only validates result.title being undefined. Renaming to "omits title field when PDF has no title" is more accurate, but this is a very minor naming/documentation improvement with no functional impact.	Low

[aliasunder]

@CodeRabbit review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 15ed2172-676c-43b9-8783-8be3acb7e11d

📥 Commits

Reviewing files that changed from the base of the PR and between 3c7ddd8 and 834fb22.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (20)

• .env.example
• .github/workflows/trivy.yml
• AGENTS.md
• ARCHITECTURE.md
• DOCKERHUB.md
• README.md
• cli/src/env.ts
• deploy/local/.env.example
• deploy/local/docker-compose.yml
• deploy/remote/.env.example
• deploy/remote/docker-compose.yml
• docker-compose.local.yml
• docker-compose.yml
• package.json
• server.json
• src/vault-mcp/__tests__/config.test.ts
• src/vault-mcp/config.ts
• src/vault-mcp/mcp-core/tools/asset-tools.ts
• src/vault-mcp/vault-operations/__tests__/asset-operations.test.ts
• src/vault-mcp/vault-operations/asset-operations.ts

---

📝 Walkthrough

Walkthrough

Adds configurable PDF raw: true page rendering with byte-budget fitting, MCP page-image formatting, deployment configuration, tests, documentation updates, and revised Trivy SARIF categories.

Changes

PDF raw rendering

Layer / File(s)	Summary
PDF rendering configuration src/vault-mcp/config.ts, src/vault-mcp/__tests__/config.test.ts, .env.example, cli/src/env.ts, deploy/*, docker-compose*.yml, server.json	Adds and validates MAX_PDF_RENDER_PAGES, propagating its default and value through generated environments, Compose services, and runtime configuration.
PDF page rendering pipeline src/vault-mcp/vault-operations/asset-operations.ts, src/vault-mcp/vault-operations/__tests__/asset-operations.test.ts, package.json	Renders capped PDF pages sequentially, fits each page to a divided byte budget, skips individual failures, cleans up the PDF proxy, and returns page metadata with fitted images.
MCP result formatting src/vault-mcp/mcp-core/tools/asset-tools.ts	Passes the page limit to asset operations and formats image, page, and text results into MCP content blocks.
PDF behavior documentation ARCHITECTURE.md, DOCKERHUB.md, README.md	Documents raw PDF rendering, scanned-PDF handling, page limits, and per-page image budgeting.

CI maintenance

Layer / File(s)	Summary
CI scan and checklist references .github/workflows/trivy.yml, AGENTS.md	Updates Trivy SARIF upload categories and corrects the documented CI drift-test path.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• aliasunder/vault-cortex#89: Both update CLI-generated local and remote environment templates.
• aliasunder/vault-cortex#351: Earlier asset-tool PDF handling is extended with raw page rendering.
• aliasunder/vault-cortex#352: Both modify PDF handling in asset-operations.ts.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: adding PDF page rendering for assets when raw: true is used.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch worktree-pdf-page-rendering

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}