Skip to content

Commit 6e46bd2

Browse files
committed
fix(footnote): broaden preferred-reserve candidate filter for partial splits (SD-2656)
Vivienne's feedback on the rendering-fidelity PR called out footnotes splitting across pages even when Word fits them on a single page. Repro fixtures: 086 Carlsbad and b89cc7aa. ## Root cause `isMandatoryOnlyFootnotePage` only flagged a page as a preferred-reserve trial candidate when: actual_band ≈ mandatory AND lastAnchorRenderedLines <= 1 The scorer therefore never considered pages where the last anchor rendered 2+ lines and the remainder still spilled. These "partial split" cases are the most common user-visible bug because the reader has to scroll to the next page mid-footnote. Repro on b89cc7aa.docx: page 16 — anchors=[4], mand=36, pref=82, actual=51, lastL=2, fn4 spilled Repro on 086 Carlsbad: page 26 — anchors=[24], mand=42, pref=150, actual=116, lastL=5, fn24 spilled page 34 — anchors=[36], mand=42, pref=187, actual=61, lastL=2, fn36 spilled None of these entered the trial set. ## Fix Adds `isSplitLastAnchorFootnotePage`: a page is also a candidate when its last anchor appears in continuationOut AND the preferred reserve is meaningfully bigger than current actual. `getPreferredReserveCandidates` unions both predicates. The scorer's accept criteria (no new cluster spills, no new mandatory-only pages, bounded dead-reserve growth, candidate rendered lines improved) stays unchanged — only the candidate filter widens. ## Verified - b89cc7aa.docx: 4 split pages -> 1 split page (Vivienne's screenshot case on page 16 now renders fn4 fully on the anchor page). - 086 Carlsbad.docx: 12 split pages unchanged (the remaining cases are multi-anchor with preferred deltas large enough that the scorer correctly rejects because of downstream cascade — same global protection as before). - IT-923 (NVCA Model COI): 50 pages unchanged. No regression. - 1253 layout-bridge tests pass (1 new test for the partial-split predicate, covering Vivienne's b89cc7aa page 16 and Carlsbad page 26 patterns plus a non-spilled counter-example). - 657 layout-engine, 1136 painter-dom pass.
1 parent d4a8353 commit 6e46bd2

2 files changed

Lines changed: 82 additions & 1 deletion

File tree

packages/layout-engine/layout-bridge/src/footnote-scorer.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,44 @@ export const isMandatoryOnlyFootnotePage = (
8181
);
8282
};
8383

84+
/**
85+
* SD-2656 (post-Vivienne-feedback): a page whose LAST anchor partially rendered
86+
* but spilled to a later page. The user-visible bug is a footnote split across
87+
* pages even when the preferred reserve would fit the whole anchor on the
88+
* anchor page (Word does keep it together).
89+
*
90+
* The "mandatory-only" predicate catches first-line-only splits; this predicate
91+
* catches partial splits (lastAnchorRenderedLines > 1 but the rest still spilled).
92+
* Both feed into the same scorer trial. The scorer's accept criteria
93+
* (no new cluster spills, no new mandatory-only pages, bounded dead-reserve
94+
* growth, candidate rendered lines improved) still gates whether the bump
95+
* actually lands.
96+
*/
97+
export const isSplitLastAnchorFootnotePage = (
98+
ledger: FootnotePageLedger,
99+
preferredDeltaThresholdPx = DEFAULT_PREFERRED_DELTA_THRESHOLD_PX,
100+
): boolean => {
101+
if (ledger.anchorIds.length === 0) return false;
102+
const lastAnchorId = ledger.anchorIds[ledger.anchorIds.length - 1];
103+
const lastAnchorSpilled = ledger.continuationOut.some((entry) => entry.id === lastAnchorId);
104+
if (!lastAnchorSpilled) return false;
105+
return (
106+
ledger.preferredReservePx - ledger.mandatoryReservePx > preferredDeltaThresholdPx &&
107+
ledger.actualBandHeightPx < ledger.preferredReservePx - preferredDeltaThresholdPx
108+
);
109+
};
110+
84111
export const getPreferredReserveCandidates = (
85112
ledgers: FootnotePageLedger[],
86113
preferredDeltaThresholdPx = DEFAULT_PREFERRED_DELTA_THRESHOLD_PX,
87114
mandatoryOnlyTolerancePx = DEFAULT_MANDATORY_ONLY_TOLERANCE_PX,
88115
): FootnotePreferredReserveCandidate[] => {
89116
return ledgers
90-
.filter((ledger) => isMandatoryOnlyFootnotePage(ledger, preferredDeltaThresholdPx, mandatoryOnlyTolerancePx))
117+
.filter(
118+
(ledger) =>
119+
isMandatoryOnlyFootnotePage(ledger, preferredDeltaThresholdPx, mandatoryOnlyTolerancePx) ||
120+
isSplitLastAnchorFootnotePage(ledger, preferredDeltaThresholdPx),
121+
)
91122
.map((ledger) => ({
92123
pageIndex: ledger.pageIndex,
93124
anchorIds: ledger.anchorIds.slice(),

packages/layout-engine/layout-bridge/test/footnoteScorer.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,56 @@ describe('SD-2656 footnote preferred-reserve scorer', () => {
7272
]);
7373
});
7474

75+
it('also flags pages where the last anchor partially rendered but spilled (Vivienne feedback)', () => {
76+
// SD-2656: a page is also a candidate when the last anchor rendered >1 line
77+
// yet still spilled to the next page. The legacy filter (lastAnchorRenderedLines<=1)
78+
// missed these "partial split" cases reported by Vivienne — footnotes splitting
79+
// across pages even when preferred reserve would fit them on the anchor page.
80+
const ledgers = [
81+
// mandatory-only first-line case (legacy candidate) — should still match.
82+
makeLedger(0, {
83+
anchorIds: ['1'],
84+
mandatoryReservePx: 36,
85+
preferredReservePx: 121,
86+
actualBandHeightPx: 36,
87+
lastAnchorRenderedLines: 1,
88+
continuationOut: [{ id: '1', remainingRangeCount: 1, remainingHeightPx: 80 }],
89+
}),
90+
// Vivienne b89cc7aa page 16 pattern: single anchor [4], mand=36, pref=82,
91+
// actual=51, lastL=2, fn4 spilled. Old filter missed this (lastL>1).
92+
makeLedger(1, {
93+
anchorIds: ['4'],
94+
mandatoryReservePx: 36,
95+
preferredReservePx: 82,
96+
actualBandHeightPx: 51,
97+
lastAnchorRenderedLines: 2,
98+
continuationOut: [{ id: '4', remainingRangeCount: 1, remainingHeightPx: 30 }],
99+
}),
100+
// Carlsbad page 26 pattern: single anchor [24], mand=42, pref=150, actual=116,
101+
// lastL=5, fn24 spilled. Old filter missed this.
102+
makeLedger(2, {
103+
anchorIds: ['24'],
104+
mandatoryReservePx: 42,
105+
preferredReservePx: 150,
106+
actualBandHeightPx: 116,
107+
lastAnchorRenderedLines: 5,
108+
continuationOut: [{ id: '24', remainingRangeCount: 2, remainingHeightPx: 30 }],
109+
}),
110+
// Counter-example: last anchor rendered fully (no spill). Must NOT be a candidate.
111+
makeLedger(3, {
112+
anchorIds: ['5'],
113+
mandatoryReservePx: 36,
114+
preferredReservePx: 96,
115+
actualBandHeightPx: 96,
116+
lastAnchorRenderedLines: 5,
117+
continuationOut: [],
118+
}),
119+
];
120+
121+
const candidates = getPreferredReserveCandidates(ledgers).map((c) => c.pageIndex);
122+
expect(candidates).toEqual([0, 1, 2]);
123+
});
124+
75125
it('summarizes only the candidate page window', () => {
76126
const ledgers = [
77127
makeLedger(0, {

0 commit comments

Comments
 (0)