Add PDF export to Docs#168
Conversation
Builds on top of DOCX export by adding a vector PDF output path. Reuses paginateLayout() for page/line/table coordinates instead of re-implementing layout, keeping a single source of truth with the on-screen renderer. Vector PDF (pdf-lib + fontkit) is preferred over a Canvas raster because the resulting file is searchable, copy/paste-able, and small. Korean text is supported by lazily fetching Noto Sans/Serif KR and embedding subset fonts; subsequent exports use IndexedDB cache. Module split (pdf-exporter / painter / fonts / style-map / table-painter / image-painter) mirrors the existing view/ split between doc-canvas and table-renderer, so file sizes stay bounded and units are testable in isolation. Frontend integration extracts shared image fetcher/download helpers from docx-actions and lazy-loads the PDF module via dynamic import to keep the initial bundle unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
7-phase TDD plan that walks an engineer from zero context through every task with exact file paths, code blocks, and verification commands. Phases: foundation (deps + fonts), text + inline styles, pages + header/footer, tables, images, PDF-native features (metadata + outline + links), frontend integration. Plan complements docs/design/docs/docs-pdf-export.md. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
These two packages enable vector PDF generation with embedded subset fonts, used by the upcoming PdfExporter for Docs PDF export. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Subsetted Noto Sans KR Regular (~68 KB, 516 glyphs) downloaded via Google Fonts CSS API. Tests inject this font into PdfFonts via DI so they don't need network or IndexedDB. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Phase 1 milestone: a minimum exporter that embeds a Korean font via pdf-lib + fontkit and writes one paragraph onto a single page. The integration test re-loads the produced PDF with pdf-lib to confirm it is a valid document with at least one page. Phase 2 will replace the body with the full layout+paginate+paint pipeline. Wraps the loaded font as a Uint8Array before embedFont because the jsdom test env exposes a different ArrayBuffer realm than Node, which causes pdf-lib's instanceof ArrayBuffer check to fail. Uint8Array's realm matches, and pdf-lib accepts both.
PdfPainter consumes a LayoutPage from view/pagination.ts and emits pdf-lib draw calls. Splits each run on CJK boundaries, resolves the embedded font via resolveFontKey, and computes the alphabetic baseline using the same formula as the canvas renderer (lineY + (lineHeight + fontSizePx * 0.8) / 2). Image runs and the underline/strikethrough/superscript/italic-shim/hyperlink layers are deferred to subsequent tasks. LayoutLine has no baseline field, so the painter derives it per-run from the layout's lineY + line.height + the run's font size. Fonts are subset-embedded once via PdfPainter.embedAllFonts and indexed by PdfFontKey for the rest of the export.
Layer per-segment background rectangle (drawn first), underline (one pixel below baseline), and strikethrough (half-ascent above baseline) on top of the existing text draw. Approximates ascent/descent as 0.8/0.2 of the font size to match the canvas renderer's implicit metrics, so PDF output mirrors what users see on screen for these inline styles.
Noto Sans/Serif KR have no italic variant, so resolveFontKey returns the regular Korean font for italic CJK runs. To still convey italic visually, the painter now wraps such draws in a graphics-state push/pop that applies a ~12 degree forward skew CTM around the run baseline, then draws at (0, 0). Background, underline, and strikethrough remain outside the transform so they stay upright.
Replace the hello-world body of PdfExporter.export with the full layout
+ paginate + paint pipeline. Fixture-driven tests cover a plain ASCII
paragraph and a mixed Korean/English paragraph; both round-trip through
pdf-lib and produce a single-page PDF.
Adds a jsdom-safe measurement context fallback (8 px per character)
since jsdom's HTMLCanvasElement.getContext('2d') is unimplemented and
real measureText is unavailable in unit tests.
Stubs pdf-image-painter.ts so future Phase 5 image embedding can drop
in without exporter changes.
Inline runs whose style.href is set now produce a Link annotation covering the drawn glyph box so PDF viewers render them as clickable hyperlinks. The annotation is built directly via pdf-lib's low-level PDFContext API and appended to the page's Annots array (which pdf-lib auto-creates as empty).
Headers and footers are computed once via computeLayout (independent of body pagination) and reused per page in PdfPainter. Each region is painted as synthetic PageLines that flow through the existing paintLine pipeline so font-shim italic, hyperlinks, and decorations all work uniformly. Page-local Y is derived directly — header at marginFromEdge, footer at pageHeight - marginFromEdge - layout.totalHeight — since pdf-lib pages have no Theme.pageGap. paintRun substitutes run text with the current page number when style.pageNumber is set, mirroring the canvas renderer's renderRunWithPageNumber.
Render bullet/number markers for list-item blocks during PDF export. Ordered counters are computed once over the body block list (matching the canvas pipeline) and threaded through PaintContext; unordered markers come from UNORDERED_MARKERS cycled by listLevel. Markers are drawn only on each block's first line, in the gutter to the left of the body text, using sans-regular regardless of the body run's font.
Move pure-geometry helpers out of table-renderer.ts (and a copy of computeTableRangeForPageLine out of doc-canvas.ts) into a new view/table-geometry.ts module so the upcoming PDF table painter can share them with the Canvas renderer. No behavior change: table-renderer.ts re-exports the moved symbols for existing call sites, table-layout.ts now imports computeMergedCellLineLayouts directly from table-geometry, and doc-canvas.ts uses the shared computeTableRangeForPageLine. All 661 docs tests still pass.
PdfPainter now detects table blocks in paintPage and delegates a contiguous span of PageLines to a new pdf-table-painter module. The painter walks the same row range computeTableRangeForPageLine produces for the canvas renderer, draws each non-merged cell's backgroundColor, then draws the four border edges (Top/Bottom/Left/Right). Cell content (recursive paint of inner blocks) is intentionally left as a no-op callback; Task 4.3 will fill it in. Reuses styleColor and the existing px-to-pt conversion so chrome lands at byte-identical coordinates with the canvas renderer's renderTableBackgrounds + border pass.
Task 4.2 left the cell-content callback as a no-op so the PDF only
carried table chrome (backgrounds + borders). This wires the callback
through to a new PdfPainter.paintCellContent that mirrors the canvas-
side renderTableContent text path: vertical alignment, merged-cell line
redistribution for top-aligned rowSpan>1 cells, and per-block list
markers.
The callback signature now also passes LayoutTableCell, LayoutTable,
and the row/col indices so paintCellContent can reuse
computeMergedCellLineLayouts without a second layout pass — the cell
layout is already in layoutTable.cells[r][c].
with-table.json fixture cells are populated with text so the new
size-delta test can prove text was actually drawn (cell.blocks=[] vs
cell.blocks=[paragraph("Hello")]).
Adds a 3x3 fixture exercising both colSpan (horizontal merge) and rowSpan (vertical merge), and a PdfExporter test that asserts the document exports to a single-page PDF without erroring. Confirms the table-geometry helpers (isCellCovered, cellOriginPx) handle merged cells end-to-end through the PDF pipeline; no source changes were required.
When `paginateLayout` splits a tall table row across pages, each fragment carries `rowSplitOffset` / `rowSplitHeight`. The PDF table painter previously ignored both, so every page redrew the entire row (content from off-page line indices ended up duplicated on every page). Mirror the Canvas renderer's `ctx.clip()` pass: install a clip path sized to the fragment band and shift the table origin up by `rowSplitOffset` so a row's table-logical Y of `rowSplitOffset` maps to `pl.y` on the page. Backgrounds, content and borders that fall outside the band get clipped, matching `renderTableBackgrounds` / `renderTableContent` behaviour for split rows.
Walk the document for image inlines, fetch each unique src via the caller-supplied imageFetcher, embed via pdf-lib (PNG/JPEG natively; GIF/WebP/BMP via Canvas re-encode), and draw them during paintRun. Matches the canvas renderer's bottom-aligned image placement so text and images share the same baseline row. Throws when image inlines exist but no fetcher is supplied so callers cannot silently drop visual content.
Wire opts.metadata (title/author/subject/keywords) into pdf-lib
setters and stamp Producer/Creator as "Wafflebase Docs". Use
PDFDocument.create({ updateMetadata: false }) so pdf-lib does not
overwrite Producer/Creator with its own default during save().
Walk the body block list during the per-page paint loop to record the first page on which each block appears, then build a flat /Outlines sibling chain from the headings. Each item carries a Title (UTF-16 BE hex string for non-ASCII safety), Parent, Dest = [pageRef /Fit], and Prev/Next refs allocated up front via ctx.nextRef() so siblings can reference each other before they are written. Phase 1 is intentionally flat — nesting by headingLevel is a follow-up.
Move the image fetcher/uploader, downloadBlob, pickFile, and safeFilename helpers out of docx-actions.ts into a new export-utils.ts so the upcoming PDF export action can reuse them. docx-actions.ts now keeps only the DOCX-specific import/export wrappers and re-exports docxImageUploader/docxImageFetcher as back-compat aliases for existing callers.
Re-export PdfExporter, PdfFonts, and related types from the docs package main entry, then add a frontend pdf-actions module that lazy-loads PdfExporter via dynamic import. This keeps pdf-lib and fontkit (~200 KB gzipped) out of the initial app bundle so they only load when the user clicks Export PDF.
The desktop toolbar's single Export button is replaced with a Radix DropdownMenu offering Word (.docx) and PDF (.pdf) entries. The mobile overflow menu gains a matching Export label with both formats so PDF export is reachable on every viewport. Click handlers mirror the existing DOCX path: shared exporting state, toast on failure, and lazy import via exportPdfAndDownload.
All 28 tasks complete on feature/pdf-export. 26 commits cover: deps + fonts (Phase 1), text + inline styles (Phase 2), pages + header/footer + lists (Phase 3), tables incl. row-split (Phase 4), images (Phase 5), metadata + outline (Phase 6), frontend integration + dropdown menu (Phase 7). Lessons file captures pdf-lib quirks (jsdom realm, updateMetadata, PDFHexString for non-ASCII titles), layout type shapes the plan got wrong, and follow-ups (self-host Noto KR, nested cell tables, heading-level outline nesting). Final test count: 674 docs + 177 frontend, all passing. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The first iteration loaded all 12 PDF font keys via PdfFonts.load(), which required injected sources for every key. With DEFAULT_URLS empty, English- only documents threw "no source for sans-regular" before drawing a single glyph. Switch Latin keys (sans-* / serif-*) to pdf-lib's StandardFonts so they embed by reference (no binary, no network). Korean keys (kr-*) only fetch when scanFontsUsed flags Korean text. Sources now default to the Google Fonts CSS API, which resolves a stable woff2 URL and is amortized by the existing IDB cache. Self-hosting can replace the default later. scanFontsUsed forces needsKR=true for unordered list-items so the U+25xx bullet glyphs (●○■) — outside Helvetica's WinAnsi coverage — route through the embedded Korean font. paintListMarker chooses kr-sans-regular for unordered markers and sans-regular for ordered (ASCII). Test thresholds dropped from 2000 to 800 because Latin StandardFonts no longer add embedded binary; renderWithStyle passes a fully-on FontUsage so the painter has Korean glyphs available for italic-shim and mixed-script test paths. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The previous CJK detection only matched U+3000-U+9FFF + U+AC00-U+D7AF + U+FF00-U+FFEF — Hangul and CJK Unified Ideographs. East Asian punctuation that lives outside those blocks (※ U+203B, 「」 U+300C-D, dingbats, geometric shapes outside the bullet range) was classified as Latin, then crashed pdf-lib's StandardFont encoder with "WinAnsi cannot encode". Replace the narrow CJK regex with the inverse: a Latin-safe character class covering basic Latin + Latin-1 Supplement + the WinAnsi specials Helvetica encodes (smart quotes, em/en dashes, euro, bullet, etc.). Any character outside that set routes through the embedded Korean font. This makes the split-and-route logic match what pdf-lib can actually encode, not what's nominally "Korean" — so newly-encountered exotic characters fail gracefully (covered by Noto KR's broader Unicode coverage) rather than throwing at draw time. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Two PDF rendering bugs surfaced during local testing. Empty Hangul glyphs: Google Fonts CSS API splits Noto KR into many unicode-range chunks and our regex only fetched the first (Latin) one. Pass `text=` with the document's actual non-Latin characters so the API returns a single subsetted file containing the glyphs we need. Cache keyed by hash of the canonical sorted-deduped subset so two documents with the same character set share IDB cache. Invisible table borders: the canvas renderer falls back to DEFAULT_BORDER_STYLE (1px black solid) when a cell omits a per-edge border definition. The PDF painter previously skipped undefined edges entirely, so default-inserted tables appeared without grid. Mirror the canvas fallback so borders match. Also strip C0 control characters (\n, \r, \t, etc.) from text segments before drawText. WinAnsi has no encoding for them; they're paste-time artifacts that have no visual representation at draw time. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The Google Fonts CSS2 API serves WOFF2 to modern user-agents, and @pdf-lib/fontkit doesn't include a Brotli decoder — loading WOFF2 as raw bytes rendered Hangul as garbled shapes (visible as "weird symbols" even though the layout positioned them correctly). Switch to direct OTF URLs from notofonts/noto-cjk via jsdelivr's GitHub mirror. Each subset OTF (~5-7 MB) downloads once per variant, gets cached in IDB, and pdf-lib's fontkit subsets it further at embed time so the final PDF only carries the glyphs the document actually uses. This also lets us drop the text-subset complexity that was added in the previous commit — the OTF source contains the full Korean character set, so per-document subset parameters aren't needed. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@pdf-lib/fontkit's CFF subset encoder throws "value argument is out of bounds" on larger CJK fonts (CFFSubset.encode -> NumberT.encode). Noto Sans/Serif KR ship as CFF-flavored OTF, so subsetting them with the existing pdf-lib pipeline produced the fontkit RangeError. Switch the Korean embeds to subset: false. PDFs grow by the full font size (~5-7 MB per variant when the document uses Korean), but the output renders reliably. A follow-up could swap in TrueType-outline Korean fonts (variable wght axis), which fontkit subsets without the CFF-specific encoder path. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The previous commit disabled subsetting (`subset: false`) for Korean embeds because @pdf-lib/fontkit's CFF subset encoder threw "value argument is out of bounds" on larger CFF/OTF fonts. The unsubsetted embeds added ~5-7 MB per Korean variant to every export. google/fonts ships Noto Sans/Serif KR as TrueType-outline variable fonts via raw.githubusercontent.com; fontkit subsets TrueType outlines cleanly. Switch DEFAULT_URLS to those TTFs and re-enable subset: true. PDFs containing Korean now embed only the glyphs the document uses. Caveat: variable fonts carry a `wght` axis (100-900). fontkit instantiates at the axis default (400), so kr-sans-bold currently renders identical to kr-sans-regular — the bold key just points at the same TTF until we instantiate at wght=700 explicitly. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
This reverts commit 04bcd71.
Records the four font-source configurations tried during local testing and why only one (Noto Sans/Serif KR OTF with subset: false) produces correct glyphs reliably. Variable-axis TTFs subsetted by fontkit dropped characters mid-string (saw "오픈소스" rendered as just "소"), so the revert to OTF + subset: false is the current accepted state. Trade-off, future paths, and updated follow-up list captured so the next person looking at the size of a Korean PDF knows what's been ruled out. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The PDF export feature pulls in pdf-lib + fontkit via dynamic import. Vite/Rollup groups dynamic imports into a `pending-imports-*` chunk that lands at ~1280 kB, well above the global 710 kB chunk-size budget the harness gate enforces. Bumping the global limit would let the rest of the bundle bloat unchecked. Add an `overrides` array to harness.config.json instead, so this one chunk can be ~1500 kB while every other chunk stays at 710 kB. The verify-frontend-chunks script now matches each chunk against the overrides regex list and falls back to the global limit when nothing matches; reasoning for the override is captured inline in the config. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
📝 WalkthroughWalkthroughAdds end-to-end PDF export for the Docs editor: design docs, core exporter/painter/fonts/image/table modules, frontend export utilities and UI integration, shared table-geometry refactor, test fixtures/suites, and small config/dependency updates to support pdf-lib and fontkit. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Frontend UI
participant PA as pdf-actions
participant EU as export-utils
participant PdfExp as PdfExporter
participant PdfF as PdfFonts
participant ImgF as ImageFetcher
participant PdfPainter as PdfPainter
participant PDFLib as pdf-lib
UI->>PA: exportPdfAndDownload(doc, title)
PA->>PdfExp: dynamic import & call export(doc, { imageFetcher, metadata })
PdfExp->>PdfF: scanFontsUsed(doc)
PdfF->>PdfF: check memory / IndexedDB cache
alt cache miss
PdfF->>PdfF: fetch font binaries & persist to IDB
end
PdfExp->>ImgF: collectAndEmbedImages(doc, pdfDoc)
ImgF->>PDFLib: embed images into PDFDocument
PdfExp->>PdfExp: paginateLayout(doc)
PdfExp->>PdfPainter: embedAllFonts(pdfDoc, fonts)
loop per page
PdfExp->>PdfPainter: paintPage(page, layoutPage, fonts, ctx)
PdfPainter->>PDFLib: draw text, images, tables, annotations
end
PdfExp->>PdfExp: build outlines & set metadata
PdfExp->>PA: return PDF Blob
PA->>EU: downloadBlob(blob, filename)
EU->>UI: browser download triggered
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
# Conflicts: # docs/tasks/archive/README.md # packages/docs/src/view/table-layout.ts
Verification: verify:selfResult: ✅ PASS in 131.0s
Verification: verify:integrationResult: ✅ PASS |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (5)
packages/docs/test/export/pdf-fonts.test.ts (1)
3-4: ⚡ Quick winAvoid
anyinparahelper to keep test types meaningful.Use a typed style input (e.g.,
Partial<InlineStyle>) so invalid style keys are still caught in tests.As per coding guidelines: `**/*.{ts,tsx}`: Use type annotations in TypeScript to improve code clarity and catch errors early.Suggested fix
-import type { Document } from '../../src/model/types.js'; +import type { Document, InlineStyle } from '../../src/model/types.js'; @@ -const para = (text: string, style: any = {}, fontFamily?: string): Document => ({ +const para = (text: string, style: Partial<InlineStyle> = {}, fontFamily?: string): Document => ({Also applies to: 16-16
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/docs/test/export/pdf-fonts.test.ts` around lines 3 - 4, The test helper `para` currently uses `any`, which weakens type checks; change its parameter type to a typed style input such as `Partial<InlineStyle>` (import `InlineStyle` from the same module where types live) so invalid style keys are caught by the compiler; update the `para` helper signature and any callsites in this test file (`pdf-fonts.test.ts`) to use the new typed parameter while keeping existing logic that uses `DEFAULT_BLOCK_STYLE` and `generateBlockId`.packages/docs/test/export/pdf-exporter.test.ts (1)
322-383: ⚡ Quick winAdd a non-ASCII outline-title regression test.
All of the outline assertions use ASCII headings, so they would still pass if bookmark title encoding regressed and Hangul titles were mangled. A small fixture with Korean headings would protect one of the core promises of this export path.
As per coding guidelines, "Write tests for critical business logic and edge cases".
packages/docs/test/export/pdf-painter.test.ts (1)
193-217: ⚡ Quick winAssert the emitted annotation is actually a URI link.
This currently passes for any non-empty annotation array, so a malformed annotation or wrong subtype would still look green. Since clickable hyperlinks are a user-visible feature here, it would be safer to assert the first annotation resolves to a link action for
https://example.com.As per coding guidelines, "Write tests for critical business logic and edge cases".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/docs/test/export/pdf-painter.test.ts` around lines 193 - 217, The test currently only checks Annots is non-empty; change it to dereference the first annotation and assert it's a URI link to https://example.com: get the Annots array via page.node.get(PDFName.of('Annots')), obtain the first annotation reference, use reloaded.context.lookup(firstAnnotRef) to get the PDFDict, then assert the Subtype equals PDFName.of('Link') and that the A (action) dictionary has S === PDFName.of('URI') and the URI entry equals 'https://example.com'; update the assertions after the PdfExporter.export call so the test fails for malformed or non-URI annotations.packages/docs/src/export/pdf-exporter.ts (1)
76-77: 💤 Low valueMinor: creation and modification dates may differ by milliseconds.
Two separate
new Date()calls could produce slightly different timestamps. Consider capturing the date once if exact equality is desired.Suggested fix
+ const now = new Date(); - pdfDoc.setCreationDate(new Date()); - pdfDoc.setModificationDate(new Date()); + pdfDoc.setCreationDate(now); + pdfDoc.setModificationDate(now);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/docs/src/export/pdf-exporter.ts` around lines 76 - 77, The creation and modification timestamps are set with two separate new Date() calls which can differ; capture a single Date instance and pass that same variable to both pdfDoc.setCreationDate(...) and pdfDoc.setModificationDate(...) so the two timestamps are identical.docs/design/docs/docs-pdf-export.md (1)
61-79: 💤 Low valueAdd language specifiers to fenced code blocks for better syntax highlighting.
Several ASCII-art diagram and path-reference code blocks lack a language identifier. While markdown renderers will display them, adding a language (e.g.,
textorplaintext) silences linter warnings and improves accessibility for screen readers.Affected blocks: lines 61, 95, 106, 164, 344, 353, 419.
Example fix for line 61
-``` +```text ┌──────────────┐ Document ┌──────────────┐ Blob ┌──────────────┐🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design/docs/docs-pdf-export.md` around lines 61 - 79, Add language specifiers to the fenced code blocks in the PDF export design doc that contain ASCII-art diagrams and path references so the linter and screen readers treat them as plain text; update each triple-backtick fence (e.g., the ASCII diagram starting with the DocStore → PdfExporter diagram and the other diagram/path blocks later in the file) to use ```text or ```plaintext, ensuring every fenced block listed in the review (the ASCII-art and path-reference blocks) is changed consistently to include the language specifier.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/tasks/archive/2026/04/20260430-pdf-export-lessons.md`:
- Around line 135-147: The "Things we deferred to manual verification" checklist
shows all items unchecked despite the archive noting Task 7.5.1 was completed;
update the document by either marking the specific completed checklist items as
checked (tick the boxes for items validated in Task 7.5.1) or change the section
heading and copy to clarify this is a reusable template (e.g., rename to "Manual
verification checklist (template)" and add a note that completed items are
recorded in Task 7.5.1). Locate the "Things we deferred to manual verification"
section and edit the checklist or heading text accordingly to reflect completion
or template intent.
In `@docs/tasks/archive/2026/04/20260430-pdf-export-todo.md`:
- Around line 5-7: Update the broken archive links: change the "Spec" link
target to an absolute/repo-root link that points to the design spec file
(docs-pdf-export.md) so it resolves correctly from the archived location, and
update the "Lessons" link to point to the archived lessons file created during
execution (move the reference from the active tasks location to the lessons file
in the archive folder) so both links work when this file lives under the archive
directory.
In `@packages/docs/src/export/pdf-fonts.ts`:
- Around line 98-102: DEFAULT_URLS currently points to moving-target URLs
(gh/notofonts/noto-cjk@main) for keys like 'kr-sans-regular', 'kr-sans-bold',
'kr-serif-regular', and 'kr-serif-bold'; update those values to immutable
references by replacing the `@main` paths with a specific commit SHA or tagged
release (or switch to self-hosted/internal CDN URLs) so production PDF font
assets are pinned; ensure the change is applied to the DEFAULT_URLS object (type
PdfFontKey) so all referenced keys use the pinned URIs before merging.
In `@packages/docs/src/export/pdf-image-painter.ts`:
- Around line 45-56: The code currently relies on blob.type and falls back to
embedAsPng unnecessarily; add a small byte-sniff helper (e.g.,
sniffImageMime(bytes: Uint8Array): string|undefined) that checks PNG (0x89 0x50
0x4E 0x47) and JPEG (0xFF 0xD8 0xFF) signatures, call it after creating
buf/Uint8Array from fetcher(src) and use its result to choose pdfDoc.embedPng or
pdfDoc.embedJpg before falling back to embedAsPng; update the branch around
fetcher, buf, img, pdfDoc.embedPng/pdfDoc.embedJpg/embedAsPng to prefer the
sniffed mime when blob.type is empty or unreliable.
In `@packages/docs/src/export/pdf-painter.ts`:
- Around line 337-351: The table-cell marker drawing uses fonts['sans-regular']
for unordered markers but those glyphs live in the Korean embedded font; update
the table-cell marker font selection to mirror the body logic by using
fonts['kr-sans-regular'] whenever block.listKind === 'unordered' (replace the
fonts['sans-regular'] used for unordered markers in the table cell
marker-drawing code with fonts['kr-sans-regular'] so the marker variable
rendered by page.drawText gets the correct font).
In `@packages/docs/src/export/pdf-table-painter.ts`:
- Around line 103-180: The split-row clipping currently uses the original
full-cell geometry (in the isSplit block using pl.rowSplitOffset/splitHeight and
rectangle/clip) which removes synthetic borders on continuation pages; instead,
compute the page-local fragment rectangle like the Canvas renderer (recompute
visibleStart/visibleEnd for the fragment using
cellOriginPx/layoutTable/tableData and pl.rowSplitOffset/splitHeight) and either
expand the clip to include border thickness or—preferably—draw a new four-sided
fragment rectangle for that slice before applying the clip so borders are
rendered; update the clipping logic around isSplit and the rendering sequence
(affecting paintCellContent and drawCellBorders calls) to use the fragment-local
rect when painting borders for split rows.
In `@packages/frontend/src/app/docs/export-utils.ts`:
- Around line 11-15: The function resolveImageUrl currently only preserves
http/https absolute URLs and will rewrite other valid absolute schemes (e.g.,
data:, blob:, chrome-extension:) when BACKEND_BASE is set; update
resolveImageUrl to first detect any absolute URL scheme by checking the URL
against a general scheme regex (for example /^[a-z][a-z0-9+.-]*:/i) and return
the url unchanged if it matches, otherwise continue to apply the existing
BACKEND_BASE logic; reference the resolveImageUrl function and the BACKEND_BASE
usage to implement this check before the existing /^https?:\/\// test or replace
that test with the general-scheme check.
- Around line 115-118: The sanitization in safeFilename can produce an empty
string when title contains only invalid chars, resulting in names like ".pdf";
update safeFilename to check the sanitized variable (safe) after replace/trim
and, if it's empty, set it to a fallback like "document" before applying
toLowerCase/extension logic so the returned filename is never just an extension.
In `@scripts/verify-frontend-chunks.mjs`:
- Around line 179-181: Update the over-budget header string in
scripts/verify-frontend-chunks.mjs to indicate a per-chunk limit instead of a
single global limit: change the message that currently uses chunkLimitKb (the
string starting with "[verify:frontend:chunks] Found chunk(s) above the allowed
limit of ") to reference "per-chunk limit" or "their per-chunk limit" and, when
available, surface each failing chunk's specific limit/override (use the failing
chunk's override property or its computed limit alongside chunkLimitKb) so the
header accurately reflects per-chunk overrides.
- Around line 75-80: The RegExp creation using new RegExp(entry.pattern) inside
the mapping that returns { regex, maxKb } can throw for invalid patterns; wrap
the RegExp construction in a try/catch (around new RegExp(entry.pattern)) and on
catch throw or return a consistent override error (the same style used by
parsePositiveNumber/validation) that includes the offending pattern and the
overrides index so the error is reported as a `[verify:frontend:chunks]`
override error; ensure the error message and throwing behavior match existing
validation flow so invalid regexes produce the same user-facing override error.
---
Nitpick comments:
In `@docs/design/docs/docs-pdf-export.md`:
- Around line 61-79: Add language specifiers to the fenced code blocks in the
PDF export design doc that contain ASCII-art diagrams and path references so the
linter and screen readers treat them as plain text; update each triple-backtick
fence (e.g., the ASCII diagram starting with the DocStore → PdfExporter diagram
and the other diagram/path blocks later in the file) to use ```text or
```plaintext, ensuring every fenced block listed in the review (the ASCII-art
and path-reference blocks) is changed consistently to include the language
specifier.
In `@packages/docs/src/export/pdf-exporter.ts`:
- Around line 76-77: The creation and modification timestamps are set with two
separate new Date() calls which can differ; capture a single Date instance and
pass that same variable to both pdfDoc.setCreationDate(...) and
pdfDoc.setModificationDate(...) so the two timestamps are identical.
In `@packages/docs/test/export/pdf-fonts.test.ts`:
- Around line 3-4: The test helper `para` currently uses `any`, which weakens
type checks; change its parameter type to a typed style input such as
`Partial<InlineStyle>` (import `InlineStyle` from the same module where types
live) so invalid style keys are caught by the compiler; update the `para` helper
signature and any callsites in this test file (`pdf-fonts.test.ts`) to use the
new typed parameter while keeping existing logic that uses `DEFAULT_BLOCK_STYLE`
and `generateBlockId`.
In `@packages/docs/test/export/pdf-painter.test.ts`:
- Around line 193-217: The test currently only checks Annots is non-empty;
change it to dereference the first annotation and assert it's a URI link to
https://example.com: get the Annots array via
page.node.get(PDFName.of('Annots')), obtain the first annotation reference, use
reloaded.context.lookup(firstAnnotRef) to get the PDFDict, then assert the
Subtype equals PDFName.of('Link') and that the A (action) dictionary has S ===
PDFName.of('URI') and the URI entry equals 'https://example.com'; update the
assertions after the PdfExporter.export call so the test fails for malformed or
non-URI annotations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5fa1a9be-a30b-4c22-bae6-0a53b1fdbab8
⛔ Files ignored due to path filters (3)
packages/docs/test/export/fixtures/fonts/test-cjk.ttfis excluded by!**/*.ttfpackages/docs/test/export/fixtures/pdf/test-image.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (38)
docs/design/README.mddocs/design/docs/docs-pdf-export.mddocs/tasks/README.mddocs/tasks/archive/2026/04/20260430-pdf-export-lessons.mddocs/tasks/archive/2026/04/20260430-pdf-export-todo.mddocs/tasks/archive/README.mdharness.config.jsonpackages/docs/package.jsonpackages/docs/src/export/pdf-exporter.tspackages/docs/src/export/pdf-fonts.tspackages/docs/src/export/pdf-image-painter.tspackages/docs/src/export/pdf-painter.tspackages/docs/src/export/pdf-style-map.tspackages/docs/src/export/pdf-table-painter.tspackages/docs/src/index.tspackages/docs/src/view/doc-canvas.tspackages/docs/src/view/table-geometry.tspackages/docs/src/view/table-layout.tspackages/docs/src/view/table-renderer.tspackages/docs/test/export/fixtures/fonts/README.mdpackages/docs/test/export/fixtures/pdf/mixed-korean-english.jsonpackages/docs/test/export/fixtures/pdf/simple-paragraph.jsonpackages/docs/test/export/fixtures/pdf/with-header-footer-pagenumber.jsonpackages/docs/test/export/fixtures/pdf/with-headings-and-links.jsonpackages/docs/test/export/fixtures/pdf/with-image.jsonpackages/docs/test/export/fixtures/pdf/with-list.jsonpackages/docs/test/export/fixtures/pdf/with-merged-cells.jsonpackages/docs/test/export/fixtures/pdf/with-split-row.jsonpackages/docs/test/export/fixtures/pdf/with-table.jsonpackages/docs/test/export/pdf-exporter.test.tspackages/docs/test/export/pdf-fonts.test.tspackages/docs/test/export/pdf-painter.test.tspackages/docs/test/export/pdf-style-map.test.tspackages/frontend/src/app/docs/docs-formatting-toolbar.tsxpackages/frontend/src/app/docs/docx-actions.tspackages/frontend/src/app/docs/export-utils.tspackages/frontend/src/app/docs/pdf-actions.tsscripts/verify-frontend-chunks.mjs
Five inline review findings rolled into one commit since they're each small and orthogonal: - pdf-painter: table-cell unordered markers now use kr-sans-regular to match the body painter. Helvetica's WinAnsi encoding has no glyph for ●○■, so cell-level lists were silently broken. - pdf-table-painter: when a row splits across pages, recompute the cell rect to the fragment band (top = pl.y, height = splitHeight) before drawing borders. The natural rect lives outside the PDF clip path, so without this the top border on the continuation page and the bottom border on the previous page get clipped away. - verify-frontend-chunks: wrap `new RegExp(entry.pattern)` in a try/catch so an invalid regex in harness.config.json fails with a clear error instead of an uncaught throw. - export-utils.resolveImageUrl: preserve any absolute URL (data:, blob:, file:, etc.), not only http(s). The previous regex routed data: URLs through the backend prefix when VITE_BACKEND_API_URL was set, breaking inline image fetches. - export-utils.safeFilename: fall back to "document" when the title reduces to nothing after sanitization. Previously a blank or fully-stripped title produced a hidden file like ".pdf". Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
- pdf-fonts: pin Noto KR URLs to release tags (Sans2.004 / Serif2.003) instead of @main. Upstream main is a moving target — first-time exports could change behavior or break without any local code change. IDB cache only protects returning users. - pdf-image-painter: when fetched image Blob lacks Content-Type, sniff PNG (89 50 4E 47) and JPEG (FF D8 FF) magic bytes before falling back to the Canvas conversion path. Servers that omit the header used to send real PNG/JPEG payloads through embedAsPng, which only works in the browser. - archived todo links: spec/lessons references no longer use relative paths that broke after the move under archive/2026/04/. Match the rest of the archive's convention by using repo-root paths. - lessons checklist: tick items locally verified during Korean document testing (Preview rendering, CJK punctuation, list markers, table borders) so the section reflects what was actually checked. - verify-frontend-chunks: rephrase the over-budget header to reflect per-chunk limits, since the chunk-loop already prints each effective limit. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
scripts/verify-frontend-chunks.mjs (1)
189-191:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the over-budget header to reflect per-chunk limits.
The header still says “allowed limit of
${chunkLimitKb},” but each chunk may use an override limit, so this message is now inaccurate.Proposed fix
console.error( - "[verify:frontend:chunks] Found chunk(s) above the allowed limit of " + - `${chunkLimitKb} kB:`, + "[verify:frontend:chunks] Found chunk(s) above their allowed per-chunk limit:", );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/verify-frontend-chunks.mjs` around lines 189 - 191, Update the over-budget header string in scripts/verify-frontend-chunks.mjs so it accurately describes that limits are applied per chunk (some chunks may have overrides) rather than a single global "allowed limit of ${chunkLimitKb}"; locate the message that constructs the header (the one starting with "[verify:frontend:chunks] Found chunk(s) above the allowed limit of " and the variable chunkLimitKb) and change the wording to indicate a per-chunk/per-file limit or mention "per-chunk limit (overrides may apply)" so the header reflects that each chunk may have its own limit.packages/docs/src/export/pdf-table-painter.ts (1)
103-120:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftClip/fragment math needs to be per visible row, not per initial
PageLine.
paintPage()callspaintTablePageRange()once for the table block and then skips the rest of that block’sPageLines. When the first line on the page is a continued split row, the clip installed here is onlysplitHeighttall, but the loops below still paint every row inrange.renderStartRow..endRowIndex, so later rows on the same page get clipped away. The inverse case is also wrong: if a later row is the split fragment, it never gets fragment-local borders becauser === pl.lineIndexonly matches the first line. Please derive fragment bounds from each visible tablePageLine, not fromplalone.Also applies to: 181-184
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/docs/src/export/pdf-table-painter.ts` around lines 103 - 120, The clip region is computed only once from the initial PageLine variable pl, which incorrectly clips other visible rows; change the logic to compute and install the PDF clip per visible table PageLine inside the loop that iterates range.renderStartRow..endRowIndex (the same loop that checks r === pl.lineIndex), using each row's own PageLine values (x, y, splitHeight/rowSplitOffset and totalWidth) to compute clipXpt/clipYpt/clipWpt/clipHpt and call page.pushOperators(...) for that row; apply the same per-PageLine fix to the equivalent clip code block around the lines referenced also (the second occurrence at 181-184).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/docs/src/export/pdf-painter.ts`:
- Around line 102-151: splitMixedScript classifies only CJK vs non-CJK causing
non-CJK-but-non-WinAnsi text to be drawn with StandardFonts (sans-*/serif-*) and
crash when paintRun calls font.widthOfTextAtSize or page.drawText; update the
text routing logic so that any segment containing code points outside WinAnsi
triggers a Unicode-capable fallback (e.g. resolveFontKey to use the embedded KR
fonts: 'kr-sans-regular'/'kr-sans-bold' or 'kr-serif-regular'/'kr-serif-bold')
or, if you prefer rejection, validate segments in splitMixedScript/paintRun and
throw a clear export error indicating unsupported characters before calling
font.widthOfTextAtSize or page.drawText.
In `@scripts/verify-frontend-chunks.mjs`:
- Around line 60-62: The code currently treats a non-array
config?.frontend?.chunkBudgets?.overrides as if no overrides exist; change the
logic so that after assigning const raw =
config?.frontend?.chunkBudgets?.overrides you return [] only if raw ===
undefined, but if raw is present and not Array.isArray(raw) then throw an error
(or call process.exit with a clear config error message) instead of silently
returning []; keep the subsequent raw.map(...) unchanged for the valid array
case.
---
Duplicate comments:
In `@packages/docs/src/export/pdf-table-painter.ts`:
- Around line 103-120: The clip region is computed only once from the initial
PageLine variable pl, which incorrectly clips other visible rows; change the
logic to compute and install the PDF clip per visible table PageLine inside the
loop that iterates range.renderStartRow..endRowIndex (the same loop that checks
r === pl.lineIndex), using each row's own PageLine values (x, y,
splitHeight/rowSplitOffset and totalWidth) to compute
clipXpt/clipYpt/clipWpt/clipHpt and call page.pushOperators(...) for that row;
apply the same per-PageLine fix to the equivalent clip code block around the
lines referenced also (the second occurrence at 181-184).
In `@scripts/verify-frontend-chunks.mjs`:
- Around line 189-191: Update the over-budget header string in
scripts/verify-frontend-chunks.mjs so it accurately describes that limits are
applied per chunk (some chunks may have overrides) rather than a single global
"allowed limit of ${chunkLimitKb}"; locate the message that constructs the
header (the one starting with "[verify:frontend:chunks] Found chunk(s) above the
allowed limit of " and the variable chunkLimitKb) and change the wording to
indicate a per-chunk/per-file limit or mention "per-chunk limit (overrides may
apply)" so the header reflects that each chunk may have its own limit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c1f2aa3-b478-4f39-8c2b-a7952de58246
📒 Files selected for processing (4)
packages/docs/src/export/pdf-painter.tspackages/docs/src/export/pdf-table-painter.tspackages/frontend/src/app/docs/export-utils.tsscripts/verify-frontend-chunks.mjs
✅ Files skipped from review due to trivial changes (1)
- packages/frontend/src/app/docs/export-utils.ts
Two findings from CodeRabbit's second pass. The misleading `isCJK` field on ScriptSegment classifies any character outside WinAnsi-encoded StandardFonts (Latin-1 + a few specials), not just CJK. The reviewer read the field name in isolation and assumed Cyrillic/Arabic/emoji would crash on Helvetica — they actually route through the embedded Korean font already. Rename to `needsCustomFont` with a JSDoc spelling out the semantics so the next reader doesn't make the same mistake. Behavior unchanged; tests adapted. When `harness.config.json` has `frontend.chunkBudgets.overrides` set to something other than an array, the verify-chunks script silently treated it as no overrides. Configuration mistakes now exit with a clear message instead of letting the budget gate run on stale config. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Internal code review (superpowers:code-reviewer) flagged six items worth fixing before merge alongside two real-world bug risks; this commit addresses all of them. - pdf-painter: filter PDF link annotations through `isSafeUrl` (the same gate the editor uses on user input). Without it, an imported document or paste-time mishap could smuggle a `javascript:` / `data:` URI into a downloaded PDF and rely on the reader's protocol filter, which historically has had bypasses. - pdf-exporter: log a console.warn when getMeasurementCtx falls through from a real browser context to the 8-px-per-char mock. A font-loading race or browser bug would otherwise silently produce a PDF with mock metrics — wrong line breaks across the document — and ship without any signal to the developer. - pdf-fonts / pdf-painter: cross-reference the implicit contract that unordered list-items force kr-sans-regular to embed (set in scanFontsUsed.visitBlock, consumed in paintListMarker). The invariant lives in two files; the comments now point at each other so future changes stay in sync. - pdf-style-map: spell out that U+FFFC is intentionally NOT in the control-strip range. Image inlines carry it as placeholder text and any future "let me strip more control chars while I'm here" refactor would break image-adjacent width measurements. Added positive (LF/CR/TAB stripped) and negative (FFFC preserved) tests to lock the behavior. - pdf-image-painter: document the URL-string dedup limitation (same image at two URLs gets fetched twice) and drop empty-string src early so a malformed inline surfaces as "skipped" rather than the fetcher's "Failed to fetch". Error message for missing fetcher now names the offending src. - lessons: note that outline destinations are page-granular ([/Fit]) rather than Y-precise. Multiple headings on the same page jump to the same Y. Y-precise destinations via [/XYZ left top zoom] is Phase 2 work. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Summary
Adds vector PDF export to the Docs editor alongside the existing DOCX export. Selectable, searchable PDFs with embedded Korean fonts, clickable hyperlinks, heading bookmarks, and document metadata. Reuses
paginateLayout()for coordinates so on-screen and PDF pagination match.Design and lessons
docs/design/docs/docs-pdf-export.mddocs/tasks/archive/2026/04/20260430-pdf-export-lessons.mdTest plan
pnpm verify:fastclean (674 docs + 177 frontend tests pass)pnpm verify:frontend:chunksclean (PDF bundle override applied atharness.config.json)Known trade-offs
@pdf-lib/fontkit's CFF subset encoder bug. Lessons file documents the four font configurations tried.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Chores