Repository navigation
Restore PDF contents page numbers by measuring the print (#1309) - #1372
Conversation
There was a problem hiding this comment.
Pull request overview
Restores PDF table-of-contents (TOC) page numbers in the headless-Chromium PDF export by measuring destination pages from link annotations in a first print, re-printing with those numbers, and verifying the final PDF still matches before publishing (else falling back to an unnumbered TOC). Also fixes a pre-existing anchor-collision bug by ensuring heading IDs are unique across the entire document.
Changes:
- Add
TocPageNumbersto read TOC destination page numbers from printed PDFs (PdfPig link annotations) with refusal checks. - Update
PdfDocumentRendererto do measure → publish → verify, using the processIIoPoolfor PDF read-back work. - Update TOC HTML/CSS to reserve a fixed-width page-number column on both passes; add tests and docs to pin and explain the contract; make heading anchors unique per document.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Markdown.Export.Test/RendererOutputTests.cs | Adds end-to-end assertions that TOC prints correct page numbers and matches link destinations. |
| test/MeshWeaver.Markdown.Export.Test/PdfHeaderLogoTests.cs | Updates renderer construction to pass pool registry. |
| test/MeshWeaver.Markdown.Export.Test/DocumentPrintComposerTests.cs | Pins the two-pass TOC HTML/CSS contract (reserved fixed-width column, digits-only diff, count mismatch ignored). |
| test/MeshWeaver.Markdown.Export.Test/DocumentBuilderTests.cs | Adds regression tests for unique heading anchors (including empty/punctuation headings). |
| src/MeshWeaver.Markdown.Export/README.md | Updates feature list to reflect restored TOC page numbers and verification approach. |
| src/MeshWeaver.Markdown.Export/Pdf/TocPageNumbers.cs | New helper to resolve TOC entry destination pages from PDF link annotations, with refusal reasons. |
| src/MeshWeaver.Markdown.Export/Pdf/PdfDocumentRenderer.cs | Implements two-pass render + verify; logs and falls back when numbering can’t be trusted. |
| src/MeshWeaver.Markdown.Export/Pdf/DocumentPrintComposer.cs | Accepts optional TOC page numbers; emits a reserved-number column for measuring pass and digits for publish pass. |
| src/MeshWeaver.Markdown.Export/Pdf/DocumentPrint.css | Styles TOC entries as flex rows with a fixed-width right-aligned page-number column. |
| src/MeshWeaver.Markdown.Export/MeshWeaver.Markdown.Export.csproj | Adds direct PdfPig reference now that the project uses it explicitly. |
| src/MeshWeaver.Markdown.Export/Ast/DocumentBuilder.cs | Ensures heading anchors are unique across the full document (including chapters) with deterministic suffixing. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-13-the-contents-list-tells-you-the-page-again.md | New “Fix” entry describing restored TOC page numbers and verification. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-12-pdf-export-cover-contents-header.md | Updates prior note to mark the TOC-number regression as resolved and links to the new entry. |
| src/MeshWeaver.Documentation/Data/Architecture/PixelFaithfulExport.md | Documents the measure/verify approach, assumptions, refusal behavior, and cost. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
#1230 replaced QuestPDF with the headless browser and reached parity on everything but one item: contents entries kept their links and lost the `..... 12` column. Chromium implements no part of CSS `target-counter`, so nothing in the page can name the page a heading landed on (#1309). The print can. Every contents entry is a link annotation whose GoTo destination is the page the browser itself chose — not a model of the pagination but the pagination, recorded by the engine that performed it. TocPageNumbers reads it back with PdfPig, already in this project's graph via MeshWeaver.ContentCollections and already carried by the licence gate, so no dependency is added. #1309 rejected the two-pass because its failure mode is silently wrong numbers when the inserted digits reflow the list, and set the bar at failing LOUDLY. Both halves are answered: * The digits cannot reflow anything. The number column is emitted on BOTH prints at a CONSTANT width (flex: 0 0 40pt — the same 40pt right-aligned column the document model drew), so the measuring print and the published print are the same layout by construction. Sizing the column to its content is exactly how a naive two-pass goes wrong. * That argument is not trusted anyway. The renderer re-reads the PUBLISHED PDF and releases it only if every destination still equals the number printed beside it. Any refusal — count mismatch, destinations out of document order, a destination not after the contents list, or a verification disagreement — falls back to the numberless list of #1230 and logs the reason with both readings. A contents list that lies about a page is worse than one that is silent about it. Reading stops at the page that completes the count, so a 50-page export parses two pages and the read-back costs ~1 ms; the cost is one extra browser print (3.1 s vs 1.4 s measured), paid only by documents that have a contents list. The read-back is a CPU leaf and goes through IIoPool's Process pool — the same pool as the browser — so a burst of exports bounds its parsing too. Cold and reactive throughout: no Task, no FromAsync. Found by the new refusal check, and fixed at the root: two headings sharing a title slugified to the SAME anchor, so the second contents entry jumped to the first section — in the rendered page and in the PDF's link annotation alike. Anchors are now made unique per document (overview, overview-2), which is what every markdown renderer does. That defect predates #1230 and was invisible only because there was no number to expose it. Verified the way the parity table was: by rendering PDFs and looking at them, then by assertion. Rasterised and inspected a 10-page report (first entry, a section spanning three pages, a heading landing mid-page, the last entry on the last page), a wrapping title with duplicate section names, and a 46-entry contents list spanning two pages — every printed number matches the page the section starts on and the running footer. RendererOutputTests then pins it against an INDEPENDENT oracle: the page whose glyphs carry the heading, required to agree with both what the list prints and where the link jumps, so an error in the annotation reading cannot also produce the expectation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
8112455 to
c8163d0
Compare
…before pairing title with number Both from the Copilot review on #1372. * break-inside: avoid on .mw-toc-entry. A title caught by a page break fragments and Chromium emits one link annotation PER FRAGMENT, so one entry contributes two rects and the positional match in TocPageNumbers sees more links than entries — the contents list loses its numbers to an avoidable refusal. Measured rather than assumed: sliding a list past a break in 8pt steps, 10 of 15 layouts printed 15 annotations for 14 entries without the rule and exactly 14 with it. Reproducing it needs a title of FOUR or more lines — orphans/widows both default to 2, so a shorter entry cannot split at all — and the comment says so, because anyone who tests the rule with an ordinary wrapped title will see no difference and conclude it does nothing. * The title+number assertion added to the existing PDF test now strips whitespace, like the new boundary test already did, so it does not depend on whether the extractor puts a gap across the column between them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Both review findings addressed in 8d18077 — and the first one turned out to be a genuine correctness bug, not just a robustness nit.
One detail worth recording, and now in the CSS comment: reproducing it needs a title of four or more lines. Whitespace. Right — the assertion I added to the existing PDF test was inconsistent with the new boundary test, which already stripped. Both now go through a shared Local after the change: |
…w requires A semantic conflict the 3-way merge could not see. #1372 gave PdfDocumentRenderer an IoPoolRegistry parameter for the two-pass contents-page-number print; this branch's artefact test predates it and constructed the renderer with the browser alone, so the merged tree failed to compile (CS7036) while every text hunk merged clean. The test already owns a pools field for HeadlessChromiumPdfRenderer, so it passes that one — the same shape RendererOutputTests and PdfHeaderLogoTests use. The artefact assertion itself is untouched and still real: Chrome resolves on this host, so Pdf_prints_the_word_struck_but_not_bold printed and read the fonts back out rather than taking the no-browser branch. MeshWeaver.Markdown.Export.Test 135/135, MeshWeaver.Documentation.Test 20/20, -c Release -warnaserror clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #1309.
#1230replaced QuestPDF with the headless browser and reached parity on everything but one item: contents entries kept their links and lost the..... 12column. Chromium implements no part of CSStarget-counter, so nothing in the page can name the page a heading landed on.The print can. Every contents entry is a link annotation whose
GoTodestination is the page the browser itself chose — not a model of the pagination but the pagination, recorded by the engine that performed it.TocPageNumbersreads it back with PdfPig, which is already in this project's graph (viaMeshWeaver.ContentCollections) and already carried by the licence gate, so no dependency is added andcheck-licenses.pyis untouched.The objection in #1309, answered
#1309 rejected a two-pass because its failure mode is silently wrong numbers when the inserted digits reflow the list, and set the bar at failing loudly. Both halves:
flex: 0 0 40pt— the same 40pt right-aligned column the document model drew), so the measuring print and the published print are the same layout by construction. Sizing the column to its content (min-width,fit-content, a width derived from the page count) is precisely how a naive two-pass goes wrong.Any refusal — count mismatch, destinations out of document order, a destination not after the contents list, or a verification disagreement — falls back to the numberless list of #1230 and logs the reason with both readings. A contents list that lies about a page is worse than one that is silent about it.
Why not the other approaches
offsetTop/ content height)break-after: avoid,orphans/widows, a repeated<thead>and unbreakable figures all move a heading to a page no per-page constant predicts — plausible numbers, occasionally off by one, indistinguishable from correct ones by eye. Also needsscript-srcin a document whose CSP is deliberatelydefault-src 'none'.Cost
One extra browser print, paid only by documents that have a contents list (measured: 3.1 s vs 1.4 s for the same document without one). Reading stops at the page that completes the count, so a 50-page export parses two pages and the read-back costs ~1 ms. The read-back is a CPU leaf and goes through
IIoPool'sProcesspool — the same pool as the browser — so a burst of exports bounds its parsing as well as its browsers. Cold and reactive throughout: noTask, noObservable.FromAsync.A real bug the refusal check caught
Two headings sharing a title slugified to the same anchor, so the second contents entry jumped to the first section — in the rendered page and in the PDF's link annotation alike. It predates #1230 and was invisible only because there was no number to expose it; the "destinations must be in document order" check surfaced it as
3, 3, 4, 6, 4, 8, 11. Fixed at the root: anchors are made unique per document (overview,overview-2), which is what every markdown renderer does.Verification
Verified the way #1230's parity table was — by rendering PDFs and looking at them — and then by assertion.
By eye (rasterised, real renderer):
Overviewentries now read 4 and 7.By assertion:
RendererOutputTestspins it against an independent oracle — the page whose glyphs carry the heading — and requires all three to agree: what the list prints, where the heading is drawn, and where the link jumps. An error in the annotation reading therefore cannot also produce the expectation.DocumentPrintComposerTestspins the composer contract with no browser installed (column reserved on both prints, constant width, digits the only difference between the two documents, a mismatched list ignored outright).Local:
MeshWeaver.Markdown.Export.Test109/109,MeshWeaver.Documentation.Test20/20, monolith export tests 16/16,-c Release -warnaserrorclean on the touched projects and their dependents, licence gate green.Not included (from #1309's "Related, NOT fixed")
The two pre-existing defects that issue names —
ReadInlinesIntorendering~~strike~~as bold+strike, and the export.csxtemplates usingnode.Content is MarkdownContent— are unrelated to the contents column and are left for their own change so they are not buried in this one.🤖 Generated with Claude Code