feat(footnote): rendering fidelity (SD-2656)#3220
Draft
tupizz wants to merge 4 commits into
Draft
Conversation
Make the body paginator demand-aware so footnote-heavy documents pack
body content tight to the separator instead of letting the post-hoc
reserve loop leave visible blank space above the footnote band.
Measured on Harvey NVCA Model SPA (108 footnote refs):
- BEFORE: 57 pages
- AFTER: 53 pages
- Word baseline: 51 pages (within +5%)
Mechanism
---------
PageState gains two fields:
- pageFootnoteReserve : existing per-page reserve, now exposed
to the break decision
- footnoteDemandThisPage : accumulator of measured footnote body
heights for refs anchored on this page
Paragraph layout consults a new optional callback:
- getFootnoteDemandForBlockId(blockId): number
The break decision uses an effective bottom:
additionalDemand = max(0, footnoteDemandThisPage - pageFootnoteReserve)
effectiveBottom = state.contentBottom - additionalDemand
Once the convergence loop has set a correct reserve, additionalDemand is
0 and the new code is a no-op. On pass 1 (no reserve), it provides the
tight-packing signal that prevents the body from filling the page only
to be clawed back by a later reserve relayout.
A safety cap clamps additionalDemand so the page always has room for at
least one body line - otherwise an oversized footnote would drive
effectiveBottom below cursorY and the paginator would advanceColumn
indefinitely.
The per-block demand lookup is built once per layoutDocument call. It
walks the block tree, including table cells (rows[].cells[].blocks /
.paragraph), and resolves each ref's pos to the containing top-level
block. Table-cell refs are attributed to the table block, the unit the
body paginator places on a page.
layout-bridge populates bodyHeightById from measures via
refreshBodyHeights and pre-measures every footnote on every convergence
iteration so migrating refs do not drop from the lookup mid-loop.
Tests
-----
- footnoteBodyDemand.test.ts RED-then-GREEN for block-aware break
+ no-op invariant for non-footnote docs
- footnoteContinuationDemand converged layout reserves carry-forward
demand on the continuation page
- footnoteRefMigration determinism regression: repeated runs
produce identical page counts, reserves,
and ref to page assignments
Refs: SD-2656 SD-3049 SD-3050 SD-3051
Plan: docs/plans/sd-2656-footnote-rendering-fidelity.md
Report: docs/plans/sd-2656-implementation-report.md
…986 SD-2658) Inline footnote references and the leading marker inside the footnote body now honor the OOXML number format / start configured in w:settings/w:footnotePr. Custom-mark refs (customMarkFollows="1") emit an empty marker run so the literal symbol in the next OOXML run renders as the visible mark. Supported formats: decimal, upperRoman, lowerRoman, upperLetter, lowerLetter, numberInDash. Unknown formats fall back to decimal. Single source of truth between the inline ref and the leading marker: pm-adapter/src/footnote-formatting.ts -> formatFootnoteCardinal() Used by: pm-adapter/.../converters/inline-converters/footnote-reference.ts super-editor/.../layout/FootnotesBuilder.ts The formatter switch is intentionally inlined (not imported from @superdoc/layout-engine's formatPageNumber) because pm-adapter sits upstream of layout-engine in the package graph - see Guard C in layout-engine/tests/src/architecture-boundaries.test.ts. A drift detection parity test asserts the two helpers agree on every supported format for cardinals 1..100: layout-engine/tests/src/footnote-formatter-parity.test.ts Settings readers in super-editor/document-api-adapters/document-settings: readFootnoteNumberFormat(settingsRoot): string | null readEndnoteNumberFormat(settingsRoot): string | null readFootnoteNumberStart(settingsRoot): number | null readEndnoteNumberStart(settingsRoot): number | null PresentationEditor reads all four up-front and threads the values through ConverterContext.footnoteNumberFormat / .endnoteNumberFormat and the per-doc cardinal counter is seeded with the configured start. customMarkFollows handling preserves pmStart/pmEnd on the empty marker run so click and selection continue to work at the ref position. Refs: SD-2656 SD-2986 SD-2986/B1 SD-2986/B2 SD-2658 SD-2662
End-to-end documentation for the footnote rendering fidelity epic:
docs/superdoc-feature-reports/sd-2656-plan.md
Original implementation plan: ticket inventory across the epic,
OOXML grounding (§17.11), code surface map with line numbers,
surgical approach for each slice, RED test scaffolds, falsifiable
success criteria.
docs/superdoc-feature-reports/sd-2656-implementation-report.md
What shipped, with measurements:
- Harvey NVCA: 57 -> 53 pages (Word baseline 51, +5%)
- pnpm test:layout vs superdoc@1.32.0:
535/543 docs (98.5%) byte-identical
5 unique-change docs, all NVCA-style footnote-rich legal
templates (the intended scope)
- pnpm test:visual: "no visual differences found"
- 16,649 unit tests across 5 packages, all green
Slice-by-slice walkthrough (SD-3049 / 3050 / 3051 / 2986/B1+B2 /
2658 / 2662), architecture compliance (Guard C parity test),
pr-reviewer findings + resolutions, deferred work, repro commands.
Refs: SD-2656
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
w:settings/w:footnotePr/w:numFmtandw:numStart. Custom marks (customMarkFollows="1") emit an empty marker run so the literal symbol renders. Single source of truth between inline ref and leading marker.Full slice-by-slice walkthrough, measurements, architecture compliance notes, pr-reviewer findings, and repro commands in
docs/superdoc-feature-reports/sd-2656-implementation-report.md.Tickets covered
w:numFmtw:numStartcustomMarkFollows)beneathTextplacementHeadline results
harvey-problem-docs/NVCA Model SPA.docx(108 footnote refs)Layout-snapshot regression (
pnpm test:layoutvs superdoc@1.32.0)lineCount,textIndentPx)Pixel diff (
pnpm test:visual)Final tool verdict: "Pixel comparison complete. No visual differences found." Report at
devtools/visual-testing/results/2026-05-09-17-27-55-v.1.32.0/webkit/report.html.Test plan
pnpm --filter @superdoc/layout-bridge test --run— 1 211 tests (incl. 3 new footnote tests)pnpm --filter @superdoc/layout-engine test— 649 testspnpm --filter @superdoc/pm-adapter test --run— 1 796 tests (incl. customMarkFollows + position preservation)pnpm --filter @superdoc/super-editor test --run— 12 699 testspnpm --filter @superdoc/layout-tests test --run— 294 tests (architecture-boundaries + new formatter parity)pnpm test:layout --reference 1.32.0— 5 unique-change docs, all in target scopepnpm test:visual— no visual differencesmainTotal unit tests: 16 649, all green.
Architecture compliance
architecture-boundaries.test.tsenforced: pm-adapter does not import@superdoc/layout-engineat runtime.pm-adapter/src/footnote-formatting.tsinlines the format switch instead of importingformatPageNumber. A drift-detection parity test inlayout-engine/tests/src/footnote-formatter-parity.test.tsasserts the two implementations agree on cardinals 1–100 for every supported format.Commits
1bed2d066—feat(layout): footnote-aware body pagination (SD-3049/3050/3051)— paginator state, layout-paragraph break decision, layout-bridge demand map plumbing, 3 new test filesb3b89b6a7—feat(footnote): honor w:numFmt / w:numStart + customMarkFollows (SD-2986 SD-2658)— pm-adapter formatter + customMark suppression, super-editor settings readers, leading marker formattingcb1ca5af1—docs(footnote): sd-2656 plan + implementation report—docs/superdoc-feature-reports/Files changed
13 modified, 6 added. +635 / −43 LOC (production + tests, excluding the docs commit which adds the plan and report).
pr-reviewer findings (resolved before push)
pm-adapter/footnote-formatting.tsimported@superdoc/layout-engine(Guard C violation)@superdoc/layout-enginewas onlydevDependencyof pm-adapterspans.sort()in demand buildermeasureFootnoteBlockscallbodyHeightByIdfrom subset only — could oscillateallFootnoteIds; all measure calls now use full settable.rows[].cells[].blocks/.paragraphcustomMarkFollowsempty run preservespmStart/pmEndlowerRoman, falls back to decimal hereKnown limitations (documented in the report)
footnoteReservedByPageIndex. ExistingfootnoteColumnPlacement.test.tsensures correctness.lowerRoman: falls back to decimal if absent. One-line fix inPresentationEditor.tswhen corpus shows demand.w:numRestartper-page / per-section: out of scope; couples numbering to layout output. SD-2986 successor.Repro