Skip to content

Commit de79693

Browse files
committed
fix(footnote): allow extra dead-reserve when trial eliminates a split (SD-2656)
Second iteration on Vivienne's feedback. Previous candidate-filter fix landed the b89cc7aa page 16 case but page 9 (anchors=[2,3], fn3 spilling) still split because: * trial target=130 (full preferred) would eliminate the split (afterSplit=0, afterLines=1->6) but rejected for dead-reserve-bloat: 148 px doc-wide growth > 128 px threshold; * trial target=125 then passed globally-safe but didn't fix the split (afterSplit=1) — the user-visible bug stayed. The scorer was treating the dead-reserve threshold as absolute. But eliminating a cluster split is a direct user-visible win that's worth trading some downstream slack for. ## Fix In `scoreFootnoteWindow`, double the window and document dead-reserve allowance when the trial eliminates a cluster split in that scope: windowAllowance = eliminatesSplitInWindow ? base * 2 : base docAllowance = eliminatesSplitInDoc ? base * 2 : base All other accept criteria (page count, new cluster-spills, new mandatory-only pages, candidate rendered lines improved) stay strict. Trials that just shift dead reserve without removing a split still hit the original threshold. ## Verified - b89cc7aa.docx: 4 split pages -> 0 split pages. Page 9 now renders fn3 fully on the anchor page (actual=130 of preferred=130, lastL=6); page 10 is body-only, matching Word. - 086 Carlsbad.docx: 12 split pages unchanged. The remaining cases all reject for `page-count-grew` (bumping reserve pushes body to a new page) — that's a hard global guarantee unchanged by this fix. - IT-923: pages 50 unchanged; splits 16 -> 15 (slight improvement). - 1254 layout-bridge tests pass (1 new test for the relaxation, using b89cc7aa page 9 ledger values).
1 parent 6e46bd2 commit de79693

2 files changed

Lines changed: 66 additions & 5 deletions

File tree

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -365,13 +365,22 @@ export const scoreFootnoteWindow = (input: FootnoteWindowScoreInput): FootnoteWi
365365
if (hasNewId(afterDocumentDiagnostics.mandatoryOnlyAnchorIds, beforeDocumentDiagnostics.mandatoryOnlyAnchorIds)) {
366366
return { accept: false, reason: 'new-mandatory-only', before, after };
367367
}
368-
if (after.deadReserveSum > before.deadReserveSum + deadReserveBloatThresholdPx) {
368+
// SD-2656 (Vivienne feedback): a trial that ELIMINATES a cluster split is a
369+
// direct user-visible win. Trade a larger dead-reserve growth for fewer
370+
// footnotes splitting across pages. Without this relaxation the scorer
371+
// accepts a smaller partial bump that improves mandatory-only count but
372+
// leaves the split intact — the user sees no change.
373+
const eliminatesSplitInWindow = beforeWindowDiagnostics.clusterSplitCount > afterWindowDiagnostics.clusterSplitCount;
374+
const eliminatesSplitInDoc = beforeDocumentDiagnostics.clusterSplitCount > afterDocumentDiagnostics.clusterSplitCount;
375+
const windowDeadAllowance = eliminatesSplitInWindow ? deadReserveBloatThresholdPx * 2 : deadReserveBloatThresholdPx;
376+
const docDeadAllowance = eliminatesSplitInDoc
377+
? wholeDocumentDeadReserveBloatThresholdPx * 2
378+
: wholeDocumentDeadReserveBloatThresholdPx;
379+
380+
if (after.deadReserveSum > before.deadReserveSum + windowDeadAllowance) {
369381
return { accept: false, reason: 'dead-reserve-bloat', before, after };
370382
}
371-
if (
372-
afterDocumentDiagnostics.deadReserveSum >
373-
beforeDocumentDiagnostics.deadReserveSum + wholeDocumentDeadReserveBloatThresholdPx
374-
) {
383+
if (afterDocumentDiagnostics.deadReserveSum > beforeDocumentDiagnostics.deadReserveSum + docDeadAllowance) {
375384
return { accept: false, reason: 'dead-reserve-bloat', before, after };
376385
}
377386
if (!candidateRenderedLinesImproved(before, after)) {

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,58 @@ describe('SD-2656 footnote preferred-reserve scorer', () => {
235235
expect(result.reason).toBe('page-count-grew');
236236
});
237237

238+
it('allows extra dead-reserve growth when the trial eliminates a cluster split (Vivienne feedback)', () => {
239+
// SD-2656: a trial that removes a footnote-spanning split is a direct
240+
// user-visible win, so the scorer trades up to 2x the normal dead-reserve
241+
// growth allowance. Without this, the scorer rejected the full preferred
242+
// bump on b89cc7aa page 9 (148 px doc-wide dead-reserve > 128 threshold)
243+
// and accepted a smaller partial bump that left the split intact.
244+
const beforeLedger = [
245+
makeLedger(0, {
246+
anchorIds: ['2', '3'],
247+
mandatoryReservePx: 53,
248+
preferredReservePx: 130,
249+
actualBandHeightPx: 115,
250+
deadReservePx: 0,
251+
lastAnchorRenderedLines: 5,
252+
continuationOut: [{ id: '3', remainingRangeCount: 1, remainingHeightPx: 30 }],
253+
}),
254+
makeLedger(1, {
255+
anchorIds: [],
256+
deadReservePx: 0,
257+
continuationIn: [{ id: '3', remainingRangeCount: 1, remainingHeightPx: 30 }],
258+
}),
259+
];
260+
// After bumping page 0 to preferred: fn3 fully renders, split eliminated,
261+
// but 148 px of dead reserve appears doc-wide (over the 128 default).
262+
const afterLedger = [
263+
makeLedger(0, {
264+
anchorIds: ['2', '3'],
265+
mandatoryReservePx: 53,
266+
preferredReservePx: 130,
267+
actualBandHeightPx: 130,
268+
deadReservePx: 0,
269+
lastAnchorRenderedLines: 7,
270+
}),
271+
makeLedger(1, {
272+
anchorIds: [],
273+
deadReservePx: 148,
274+
}),
275+
];
276+
277+
const result = scoreFootnoteWindow({
278+
beforeLayout: makeLayout(2, beforeLedger),
279+
afterLayout: makeLayout(2, afterLedger),
280+
candidatePageIndex: 0,
281+
candidateAnchorId: '3',
282+
beforeLedger,
283+
afterLedger,
284+
});
285+
286+
expect(result.accept).toBe(true);
287+
expect(result.reason).toBe('globally-safe');
288+
});
289+
238290
it('accepts a direct candidate-line improvement without requiring unrelated pages to change', () => {
239291
const beforeLedger = [
240292
makeLedger(0, {

0 commit comments

Comments
 (0)