fix(v1.4.1.0): /make-pdf — page numbers, entity escape, Linux fonts by garrytan · Pull Request #1098 · garrytan/gstack

garrytan · 2026-04-20T13:55:34Z

Summary

Three visible bugs in v1.4.0.0 made /make-pdf output look broken. This PR fixes all three and expands each fix beyond the obvious symptom:

Bug fixes:

Page footer showed "N of M" twice on every page (Chromium native + CSS stacking). Fixed by making CSS the single source of truth; --no-page-numbers now actually reaches the CSS layer; --footer-template cleanly replaces stock footer.
# Herbert & Garry rendered as Herbert &amp; Garry in <title>, cover, and TOC entries. Fixed by adding decodeTextEntities at both extractor call sites; covers named + numeric entities (© → ©).
Linux PDFs rendered in DejaVu Sans instead of Helvetica. Fixed by adding "Liberation Sans" to all four font stacks + explicit fonts-liberation install in Dockerfile.ci with fc-match verify.

Three bisectable commits. Each independently revertable. 17 new tests.

Test Coverage

58 tests pass in render.test.ts (up from 41). 350 tests pass in gen-skill-docs.test.ts. Full bun test exits 0.

Coverage diagram:

Bug 1 — pageNumbers/footerTemplate plumbing
  ├── printCss({pageNumbers: false})             [★★★ TESTED]
  ├── printCss({pageNumbers: true})              [★★★ TESTED]
  ├── render({pageNumbers: false})               [★★★ TESTED]
  ├── render({footerTemplate: "..."})            [★★★ TESTED]
  └── orchestrator → browseClient.pdf           [★   indirect via CSS end-to-end]

Bug 2 — decodeTextEntities
  ├── extractFirstHeading (<title>, cover)       [★★★ TESTED — 5 chars parameterized]
  ├── extractHeadings (TOC)                      [★★★ TESTED]
  ├── decimal numeric (&#169;)                   [★★  TESTED]
  ├── hex numeric (&#x2014;)                     [GAP]
  ├── amp-last ordering (&amp;lt;)               [GAP]
  └── smartypants interaction contract           [★★★ TESTED]

Bug 3 — Liberation Sans
  ├── body font stack                            [★★★ TESTED]
  ├── @page margin boxes (≥1)                    [★★★ TESTED]
  ├── ≥4 total occurrences (all 4 stacks)        [★★★ TESTED]
  └── Dockerfile.ci fc-match verify              [only at image build time]

Coverage: ~90%. Known gaps: hex entity path, amp-last ordering, SKILL.md content assertion.

Tests: 41 → 58 (+17 new)

Pre-Landing Review

Reviewed via /plan-eng-review (Claude, 1 finding → expanded scope) and /codex (outside voice, 11 findings → 11 incorporated). Cross-model agreement rate ~30% — Codex caught the data-flow gap Claude's eng review missed. All findings resolved before implementation.

Eval Results

No prompt-related files changed — evals skipped.

Plan Completion

Plan at ~/.claude/plans/system-instruction-you-are-working-whimsical-token.md — all items DONE or CHANGED (2 scope expansions, user approved both). Lake Score: 2/2.

Verification Results

No dev server detected — auto-verification skipped. Manual verification plan (Mac):

bun run dev make-pdf <file.md> --cover --toc -o /tmp/out.pdf → footer "N of M" once, title Herbert & Garry single-escaped, TOC entry Section © 2026 clean.
bun run dev make-pdf <file.md> --no-page-numbers -o /tmp/nop.pdf → NO page footer.
bun run dev make-pdf <file.md> --footer-template "<div>Custom</div>" -o /tmp/ft.pdf → ONLY custom footer.

TODOS

No TODOs matched this branch's work — no items to mark complete.

Test plan

58 make-pdf/test/render.test.ts tests pass
350 gen-skill-docs.test.ts tests pass
Full bun test exits 0 (3 pre-existing failures proven on main, unrelated)
Plan reviewed via /plan-eng-review + /codex
Manual visual check on Mac (see Verification Results above)
Manual visual check on Linux via Docker image build

🤖 Generated with Claude Code

@page

…mbers end-to-end Two page-number sources were stacking in every PDF: Chromium's native footer and our @page @bottom-center CSS. The CLI flag --page-numbers/--no-page-numbers also never reached the CSS layer, because RenderOptions didn't carry it. Passing --footer-template likewise dropped the "custom footer replaces stock footer" semantic. - orchestrator.ts: browseClient.pdf() gets pageNumbers:false unconditionally. CSS is the single source of truth. Chromium native numbering always off. - render.ts: RenderOptions gains pageNumbers + footerTemplate. render() computes showPageNumbers = pageNumbers !== false && !footerTemplate and passes to printCss(), preserving the prior footerTemplate-suppresses-stock semantic. - print-css.ts: PrintCssOptions.pageNumbers wraps @bottom-center in a conditional matching the existing showConfidential pattern. - types.ts: PreviewOptions.pageNumbers so preview path compiles and matches CLI. - render.test.ts: 7 regression tests covering printCss({pageNumbers}) in isolation AND the full render() data flow incl. footerTemplate path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…le-escape A markdown title like "# Herbert & Garry" rendered as "Herbert &amp; Garry" in <title>, cover block, and TOC entries. marked emits "&" (correct HTML), but extractFirstHeading and extractHeadings only stripTags — leaving the entity intact. That string then flows through escapeHtml, producing the double-encode. - render.ts: new decodeTextEntities helper, distinct from decodeTypographicEntities (which runs on in-pipeline HTML and intentionally preserves &). Covers named entities (lt/gt/quot/apos/39/x27/amp) AND numeric (decimal + hex) so inputs like "©" or "—" don't create the same partial-fix bug. Amp-last ordering prevents double-decode on "&lt;" et al. - Apply in both extractFirstHeading and extractHeadings. extractHeadings feeds buildTocBlock → escapeHtml, so the TOC site had the same bug. - render.test.ts: 8 tests covering the contract — parameterized across &, <, >, ©, — chars; single-escape in <title>/cover; TOC double-escape check; numeric entity decode; smartypants-interacts-with-quotes contract (no raw equality). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@page

On Linux (Docker, CI, servers), neither Helvetica nor Arial exist. Our CSS stacks were falling through to DejaVu Sans — wider letterforms that look like Verdana, not the intended Helvetica/Faber look. Liberation Sans is the standard metric-compatible Arial clone (SIL OFL 1.1, apt package fonts-liberation). - print-css.ts: all four font stacks (body + @top-center + @bottom-center + @bottom-right CONFIDENTIAL) gain "Liberation Sans" between Helvetica and Arial. File-header docblock updated to reflect the new stack. - .github/docker/Dockerfile.ci: explicit apt-get install fonts-liberation + fontconfig with retry, fc-cache -f, and a verify step that fails the build loud if the font disappears. Playwright's install-deps happens to pull this in today but the dep is implicit and could silently regress. - SKILL.md.tmpl: one-sentence note pointing Linux users at fonts-liberation. - SKILL.md: regenerated via bun run gen:skill-docs --host all (only make-pdf's generated file changed — verified clean diff scope). - render.test.ts: 2 assertions — Liberation Sans in body stack AND in at least one @page margin-box rule (proves all four intended stacks got touched, not just one). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions · 2026-04-20T14:00:14Z

E2E Evals: ✅ PASS

0/0 tests passed | $0 total cost | 12 parallel runners

Suite	Result	Status	Cost

12x ubicloud-standard-2 (Docker: pre-baked toolchain + deps) | wall clock ≈ slowest suite

- CHANGELOG + render.test.ts fixtures use "Faber & Faber" instead of a personal name. Same regression coverage (ampersand in <title>, cover, TOC, body), neutral subject. - make-pdf/SKILL.md.tmpl description drops the "send to a VC partner, a book agent, a judge, or Rick Rubin's team" line. "Not a draft artifact — a finished artifact" stands on its own without the audience posturing. - SKILL.md regenerated. No functional changes. All 58 make-pdf tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

# Conflicts: # CHANGELOG.md # VERSION # package.json

@page

…arrytan#1098) * fix(make-pdf): single-source page numbers via CSS, honor --no-page-numbers end-to-end Two page-number sources were stacking in every PDF: Chromium's native footer and our @page @bottom-center CSS. The CLI flag --page-numbers/--no-page-numbers also never reached the CSS layer, because RenderOptions didn't carry it. Passing --footer-template likewise dropped the "custom footer replaces stock footer" semantic. - orchestrator.ts: browseClient.pdf() gets pageNumbers:false unconditionally. CSS is the single source of truth. Chromium native numbering always off. - render.ts: RenderOptions gains pageNumbers + footerTemplate. render() computes showPageNumbers = pageNumbers !== false && !footerTemplate and passes to printCss(), preserving the prior footerTemplate-suppresses-stock semantic. - print-css.ts: PrintCssOptions.pageNumbers wraps @bottom-center in a conditional matching the existing showConfidential pattern. - types.ts: PreviewOptions.pageNumbers so preview path compiles and matches CLI. - render.test.ts: 7 regression tests covering printCss({pageNumbers}) in isolation AND the full render() data flow incl. footerTemplate path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(make-pdf): decode HTML entities in titles and TOC to prevent double-escape A markdown title like "# Herbert & Garry" rendered as "Herbert &amp; Garry" in <title>, cover block, and TOC entries. marked emits "&" (correct HTML), but extractFirstHeading and extractHeadings only stripTags — leaving the entity intact. That string then flows through escapeHtml, producing the double-encode. - render.ts: new decodeTextEntities helper, distinct from decodeTypographicEntities (which runs on in-pipeline HTML and intentionally preserves &). Covers named entities (lt/gt/quot/apos/39/x27/amp) AND numeric (decimal + hex) so inputs like "&garrytan#169;" or "—" don't create the same partial-fix bug. Amp-last ordering prevents double-decode on "&lt;" et al. - Apply in both extractFirstHeading and extractHeadings. extractHeadings feeds buildTocBlock → escapeHtml, so the TOC site had the same bug. - render.test.ts: 8 tests covering the contract — parameterized across &, <, >, ©, — chars; single-escape in <title>/cover; TOC double-escape check; numeric entity decode; smartypants-interacts-with-quotes contract (no raw equality). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(make-pdf): Liberation Sans font fallback for Linux rendering On Linux (Docker, CI, servers), neither Helvetica nor Arial exist. Our CSS stacks were falling through to DejaVu Sans — wider letterforms that look like Verdana, not the intended Helvetica/Faber look. Liberation Sans is the standard metric-compatible Arial clone (SIL OFL 1.1, apt package fonts-liberation). - print-css.ts: all four font stacks (body + @top-center + @bottom-center + @bottom-right CONFIDENTIAL) gain "Liberation Sans" between Helvetica and Arial. File-header docblock updated to reflect the new stack. - .github/docker/Dockerfile.ci: explicit apt-get install fonts-liberation + fontconfig with retry, fc-cache -f, and a verify step that fails the build loud if the font disappears. Playwright's install-deps happens to pull this in today but the dep is implicit and could silently regress. - SKILL.md.tmpl: one-sentence note pointing Linux users at fonts-liberation. - SKILL.md: regenerated via bun run gen:skill-docs --host all (only make-pdf's generated file changed — verified clean diff scope). - render.test.ts: 2 assertions — Liberation Sans in body stack AND in at least one @page margin-box rule (proves all four intended stacks got touched, not just one). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v1.4.1.0) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: anonymize test fixtures, drop VC-partner framing - CHANGELOG + render.test.ts fixtures use "Faber & Faber" instead of a personal name. Same regression coverage (ampersand in <title>, cover, TOC, body), neutral subject. - make-pdf/SKILL.md.tmpl description drops the "send to a VC partner, a book agent, a judge, or Rick Rubin's team" line. "Not a draft artifact — a finished artifact" stands on its own without the audience posturing. - SKILL.md regenerated. No functional changes. All 58 make-pdf tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@page

…arrytan#1098) * fix(make-pdf): single-source page numbers via CSS, honor --no-page-numbers end-to-end Two page-number sources were stacking in every PDF: Chromium's native footer and our @page @bottom-center CSS. The CLI flag --page-numbers/--no-page-numbers also never reached the CSS layer, because RenderOptions didn't carry it. Passing --footer-template likewise dropped the "custom footer replaces stock footer" semantic. - orchestrator.ts: browseClient.pdf() gets pageNumbers:false unconditionally. CSS is the single source of truth. Chromium native numbering always off. - render.ts: RenderOptions gains pageNumbers + footerTemplate. render() computes showPageNumbers = pageNumbers !== false && !footerTemplate and passes to printCss(), preserving the prior footerTemplate-suppresses-stock semantic. - print-css.ts: PrintCssOptions.pageNumbers wraps @bottom-center in a conditional matching the existing showConfidential pattern. - types.ts: PreviewOptions.pageNumbers so preview path compiles and matches CLI. - render.test.ts: 7 regression tests covering printCss({pageNumbers}) in isolation AND the full render() data flow incl. footerTemplate path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(make-pdf): decode HTML entities in titles and TOC to prevent double-escape A markdown title like "# Herbert & Garry" rendered as "Herbert &amp; Garry" in <title>, cover block, and TOC entries. marked emits "&" (correct HTML), but extractFirstHeading and extractHeadings only stripTags — leaving the entity intact. That string then flows through escapeHtml, producing the double-encode. - render.ts: new decodeTextEntities helper, distinct from decodeTypographicEntities (which runs on in-pipeline HTML and intentionally preserves &). Covers named entities (lt/gt/quot/apos/39/x27/amp) AND numeric (decimal + hex) so inputs like "&garrytan#169;" or "—" don't create the same partial-fix bug. Amp-last ordering prevents double-decode on "&lt;" et al. - Apply in both extractFirstHeading and extractHeadings. extractHeadings feeds buildTocBlock → escapeHtml, so the TOC site had the same bug. - render.test.ts: 8 tests covering the contract — parameterized across &, <, >, ©, — chars; single-escape in <title>/cover; TOC double-escape check; numeric entity decode; smartypants-interacts-with-quotes contract (no raw equality). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(make-pdf): Liberation Sans font fallback for Linux rendering On Linux (Docker, CI, servers), neither Helvetica nor Arial exist. Our CSS stacks were falling through to DejaVu Sans — wider letterforms that look like Verdana, not the intended Helvetica/Faber look. Liberation Sans is the standard metric-compatible Arial clone (SIL OFL 1.1, apt package fonts-liberation). - print-css.ts: all four font stacks (body + @top-center + @bottom-center + @bottom-right CONFIDENTIAL) gain "Liberation Sans" between Helvetica and Arial. File-header docblock updated to reflect the new stack. - .github/docker/Dockerfile.ci: explicit apt-get install fonts-liberation + fontconfig with retry, fc-cache -f, and a verify step that fails the build loud if the font disappears. Playwright's install-deps happens to pull this in today but the dep is implicit and could silently regress. - SKILL.md.tmpl: one-sentence note pointing Linux users at fonts-liberation. - SKILL.md: regenerated via bun run gen:skill-docs --host all (only make-pdf's generated file changed — verified clean diff scope). - render.test.ts: 2 assertions — Liberation Sans in body stack AND in at least one @page margin-box rule (proves all four intended stacks got touched, not just one). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v1.4.1.0) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: anonymize test fixtures, drop VC-partner framing - CHANGELOG + render.test.ts fixtures use "Faber & Faber" instead of a personal name. Same regression coverage (ampersand in <title>, cover, TOC, body), neutral subject. - make-pdf/SKILL.md.tmpl description drops the "send to a VC partner, a book agent, a judge, or Rick Rubin's team" line. "Not a draft artifact — a finished artifact" stands on its own without the audience posturing. - SKILL.md regenerated. No functional changes. All 58 make-pdf tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@page

…arrytan#1098) * fix(make-pdf): single-source page numbers via CSS, honor --no-page-numbers end-to-end Two page-number sources were stacking in every PDF: Chromium's native footer and our @page @bottom-center CSS. The CLI flag --page-numbers/--no-page-numbers also never reached the CSS layer, because RenderOptions didn't carry it. Passing --footer-template likewise dropped the "custom footer replaces stock footer" semantic. - orchestrator.ts: browseClient.pdf() gets pageNumbers:false unconditionally. CSS is the single source of truth. Chromium native numbering always off. - render.ts: RenderOptions gains pageNumbers + footerTemplate. render() computes showPageNumbers = pageNumbers !== false && !footerTemplate and passes to printCss(), preserving the prior footerTemplate-suppresses-stock semantic. - print-css.ts: PrintCssOptions.pageNumbers wraps @bottom-center in a conditional matching the existing showConfidential pattern. - types.ts: PreviewOptions.pageNumbers so preview path compiles and matches CLI. - render.test.ts: 7 regression tests covering printCss({pageNumbers}) in isolation AND the full render() data flow incl. footerTemplate path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(make-pdf): decode HTML entities in titles and TOC to prevent double-escape A markdown title like "# Herbert & Garry" rendered as "Herbert &amp; Garry" in <title>, cover block, and TOC entries. marked emits "&" (correct HTML), but extractFirstHeading and extractHeadings only stripTags — leaving the entity intact. That string then flows through escapeHtml, producing the double-encode. - render.ts: new decodeTextEntities helper, distinct from decodeTypographicEntities (which runs on in-pipeline HTML and intentionally preserves &). Covers named entities (lt/gt/quot/apos/39/x27/amp) AND numeric (decimal + hex) so inputs like "&garrytan#169;" or "—" don't create the same partial-fix bug. Amp-last ordering prevents double-decode on "&lt;" et al. - Apply in both extractFirstHeading and extractHeadings. extractHeadings feeds buildTocBlock → escapeHtml, so the TOC site had the same bug. - render.test.ts: 8 tests covering the contract — parameterized across &, <, >, ©, — chars; single-escape in <title>/cover; TOC double-escape check; numeric entity decode; smartypants-interacts-with-quotes contract (no raw equality). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(make-pdf): Liberation Sans font fallback for Linux rendering On Linux (Docker, CI, servers), neither Helvetica nor Arial exist. Our CSS stacks were falling through to DejaVu Sans — wider letterforms that look like Verdana, not the intended Helvetica/Faber look. Liberation Sans is the standard metric-compatible Arial clone (SIL OFL 1.1, apt package fonts-liberation). - print-css.ts: all four font stacks (body + @top-center + @bottom-center + @bottom-right CONFIDENTIAL) gain "Liberation Sans" between Helvetica and Arial. File-header docblock updated to reflect the new stack. - .github/docker/Dockerfile.ci: explicit apt-get install fonts-liberation + fontconfig with retry, fc-cache -f, and a verify step that fails the build loud if the font disappears. Playwright's install-deps happens to pull this in today but the dep is implicit and could silently regress. - SKILL.md.tmpl: one-sentence note pointing Linux users at fonts-liberation. - SKILL.md: regenerated via bun run gen:skill-docs --host all (only make-pdf's generated file changed — verified clean diff scope). - render.test.ts: 2 assertions — Liberation Sans in body stack AND in at least one @page margin-box rule (proves all four intended stacks got touched, not just one). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v1.4.1.0) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: anonymize test fixtures, drop VC-partner framing - CHANGELOG + render.test.ts fixtures use "Faber & Faber" instead of a personal name. Same regression coverage (ampersand in <title>, cover, TOC, body), neutral subject. - make-pdf/SKILL.md.tmpl description drops the "send to a VC partner, a book agent, a judge, or Rick Rubin's team" line. "Not a draft artifact — a finished artifact" stands on its own without the audience posturing. - SKILL.md regenerated. No functional changes. All 58 make-pdf tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

garrytan and others added 4 commits April 20, 2026 21:40

chore: bump version and changelog (v1.4.1.0)

7e98052

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

garrytan and others added 2 commits April 20, 2026 22:19

Merge remote-tracking branch 'origin/main' into garrytan/makepdf-patch

109994b

# Conflicts: # CHANGELOG.md # VERSION # package.json

garrytan merged commit e23ff28 into main Apr 20, 2026
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(v1.4.1.0): /make-pdf — page numbers, entity escape, Linux fonts#1098

fix(v1.4.1.0): /make-pdf — page numbers, entity escape, Linux fonts#1098
garrytan merged 6 commits into
mainfrom
garrytan/makepdf-patch

garrytan commented Apr 20, 2026

Uh oh!

github-actions Bot commented Apr 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

garrytan commented Apr 20, 2026

Summary

Test Coverage

Pre-Landing Review

Eval Results

Plan Completion

Verification Results

TODOS

Test plan

Uh oh!

github-actions Bot commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

E2E Evals: ✅ PASS

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Apr 20, 2026 •

edited

Loading