fix(layout-bridge): keep footnote fragments within the reserved band (SD-1680) (#2881)

tupizz · web-flow · commit adf4ea62e22e · 2026-04-21T13:47:31.000-07:00
* fix(layout-bridge): keep footnote fragments within the reserved band (SD-1680)

When multiple footnotes competed for space on a single page, footnote
fragments could render past the bottom page margin — sometimes 150+ pixels
into the footer area or off the page entirely. Caused by three interacting
issues in `computeFootnoteLayoutPlan` + the multi-pass reserve loop:

1. Placement used `maxReserve` (~the whole page) as the height ceiling, so
   the plan happily placed more footnote content than the body layout had
   actually reserved at the bottom.

2. The multi-pass stabilization loop could oscillate when reducing the
   reserve shifted refs between pages, and exit with a `reserves` vector
   that didn't match what the plan placed — leaving body layout and
   footnote plan inconsistent.

3. The post-loop "reservesDiffer → relayout" branch could then reintroduce
   the mismatch by laying out with one reserve vector while the final plan
   operated on another.

Fixes:

- `computeFootnoteLayoutPlan`: when `baseReserve &gt; 0`, cap `placeFootnote`'s
  `availableHeight` to `baseReserve` (the band actually reserved by the
  body layout), not `maxReserve`. Excess footnotes are pushed to the next
  page via the existing `pendingByColumn` mechanism. Initial pass
  (`baseReserve === 0`) still uses `maxReserve` to seed a meaningful
  reserve for subsequent passes.

- Multi-pass loop: merge reserves element-wise with `max(prev, next)` each
  pass. Reserves are monotonically non-decreasing, so the loop is
  guaranteed to converge (bounded by `maxReserve` per page) and the old
  `[247] ↔ [394]` oscillation can't recur.

- Post-loop: drop the `reservesDiffer → relayout` branch. With the
  monotonic loop already converged and placement bounded by `reserves`,
  `finalPlan`'s slices always fit within the body's reserved band. The
  extra relayout was the mechanism that reintroduced inconsistency.

Visual validation (in-browser) on the 3 fixtures from the Linear issue:

  - reporter-test.docx: max overflow 156px → 0px
  - harvey-nvca.docx: overflow present → 0px
  - torke-carlsbad.docx: overflow present → 0px

Tests:

  - 2 new tests in footnoteBandOverflow.test.ts asserting the invariant
    "every footnote fragment's bottom-Y ≤ page's physical bottom limit"
    for both multi-footnote pressure and oversized-footnote cases.
  - 1168 layout-bridge tests pass, 604 layout-engine tests pass,
    1737 pm-adapter tests pass, 11385 super-editor tests pass.

Follow-up (Step 3, separate PR): footnote-aware body pagination — consult
footnote heights when deciding whether paragraph P fits on page N, rather
than reactively shrinking body via reserves. This would eliminate the
multi-pass loop entirely.

* fix(layout-bridge): drive footnote reserve from per-page demand (SD-1680)

Cap footnote placement at each page's measured demand (capped at maxReserve)
rather than the body's prior-pass reserve, and merge element-wise max across
oscillating reserve vectors before the post-loop relayout. Ensures every
page's reserve is large enough to hold its placed footnotes so fragments
cannot render past the bottom margin, and iterates the post-loop relayout
until the layout's reserve per page matches the final plan's demand.
diff --git a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts
@@ -1329,6 +1329,34 @@ export async function incrementalLayout(
           const columns = pageColumns.get(pageIndex);
           const columnCount = Math.max(1, Math.floor(columns?.count ?? 1));
 
+          // SD-1680: cap placement to the footnote demand on this page (capped by maxReserve).
+          // Demand = sum of measured heights of all footnote refs anchored here, plus the
+          // separator/padding/gap overhead they would incur when stacked. Capping placement
+          // at `min(demand, maxReserve)` (rather than `baseReserve`) decouples the plan's
+          // placement from the body's prior-pass reserve: the plan reports how much band
+          // the footnotes actually need, the body grows its reserve to match on the next
+          // pass, and placement never exceeds maxReserve so footnotes cannot render past
+          // the page's bottom margin.
+          let demand = 0;
+          for (let columnIndex = 0; columnIndex < columnCount; columnIndex += 1) {
+            const ids = idsByColumn.get(pageIndex)?.get(columnIndex) ?? [];
+            let columnDemand = 0;
+            ids.forEach((id, idx) => {
+              const ranges = rangesByFootnoteId.get(id) ?? [];
+              let rangesHeight = 0;
+              ranges.forEach((range) => {
+                const spacingAfter = 'spacingAfter' in range ? (range.spacingAfter ?? 0) : 0;
+                rangesHeight += range.height + spacingAfter;
+              });
+              columnDemand += rangesHeight + (idx > 0 ? safeGap : 0);
+            });
+            if (columnDemand > 0) {
+              columnDemand += safeSeparatorSpacingBefore + safeDividerHeight + safeTopPadding;
+            }
+            if (columnDemand > demand) demand = columnDemand;
+          }
+          const placementCeiling = demand > 0 ? Math.min(Math.ceil(demand), maxReserve) : maxReserve;
+
           const pendingForPage = new Map<number, Array<{ id: string; ranges: FootnoteRange[] }>>();
           pendingByColumn.forEach((entries, columnIndex) => {
             const targetIndex = columnIndex < columnCount ? columnIndex : Math.max(0, columnCount - 1);
@@ -1366,7 +1394,7 @@ export async function incrementalLayout(
                 : 0;
               const overhead = isFirstSlice ? separatorBefore + separatorHeight + safeTopPadding : 0;
               const gapBefore = !isFirstSlice ? safeGap : 0;
-              const availableHeight = Math.max(0, maxReserve - usedHeight - overhead - gapBefore);
+              const availableHeight = Math.max(0, placementCeiling - usedHeight - overhead - gapBefore);
               const { slice, remainingRanges } = fitFootnoteContent(
                 id,
                 ranges,
@@ -1375,7 +1403,7 @@ export async function incrementalLayout(
                 columnIndex,
                 isContinuation,
                 measuresById,
-                isFirstSlice && maxReserve > 0,
+                isFirstSlice && placementCeiling > 0,
               );
 
               if (slice.ranges.length === 0) {
@@ -1757,7 +1785,7 @@ export async function incrementalLayout(
       // so each page gets the correct reserve (avoids "too much" on one page and "not enough" on another).
       if (reserves.some((h) => h > 0)) {
         let reservesStabilized = false;
-        const seenReserveKeys = new Set<string>([reserves.join(',')]);
+        const seenReserveVectors: number[][] = [reserves.slice()];
         for (let pass = 0; pass < MAX_FOOTNOTE_LAYOUT_PASSES; pass += 1) {
           layout = relayout(reserves);
           ({ columns: pageColumns, idsByColumn } = resolveFootnoteAssignments(layout));
@@ -1773,13 +1801,33 @@ export async function incrementalLayout(
             reservesStabilized = true;
             break;
           }
-          // Detect oscillation: if we've produced a reserve vector we already tried,
-          // the loop will never converge. Break early to avoid wasted relayout passes.
+          // SD-1680: when reserves oscillate (typically between a state where all footnotes
+          // fit and a state where body packs tighter with some footnotes pushed off the
+          // page), prefer the element-wise max across all seen states. This matches Word's
+          // bias toward keeping footnotes on their ref's page rather than tight body
+          // packing, and avoids overflow from the body reserving less than the plan places.
           const nextKey = nextReserves.join(',');
-          if (seenReserveKeys.has(nextKey)) {
+          const seen = seenReserveVectors.some((v) => v.join(',') === nextKey);
+          if (seen) {
+            const allVectors = [...seenReserveVectors, nextReserves];
+            const mergedLength = Math.max(...allVectors.map((v) => v.length));
+            const merged = new Array<number>(mergedLength).fill(0);
+            for (const vec of allVectors) {
+              for (let i = 0; i < mergedLength; i += 1) {
+                if ((vec[i] ?? 0) > merged[i]) merged[i] = vec[i];
+              }
+            }
+            reserves = merged;
+            // Relayout with merged reserves so post-loop sees a layout consistent with the
+            // reserves we're about to apply — otherwise pages may collapse to the layout
+            // built with the smaller oscillating reserve.
+            layout = relayout(reserves);
+            ({ columns: pageColumns, idsByColumn } = resolveFootnoteAssignments(layout));
+            ({ measuresById } = await measureFootnoteBlocks(collectFootnoteIdsByColumn(idsByColumn)));
+            plan = computeFootnoteLayoutPlan(layout, idsByColumn, measuresById, reserves, pageColumns);
             break;
           }
-          seenReserveKeys.add(nextKey);
+          seenReserveVectors.push(nextReserves.slice());
           // Only update reserves when we will do another layout pass; otherwise layout
           // would be built with the previous reserves while reserves would be nextReserves,
           // and the plan/injection phase could place footnotes in the wrong band.
@@ -1804,15 +1852,31 @@ export async function incrementalLayout(
           reserves,
           finalPageColumns,
         );
-        const finalReserves = finalPlan.reserves;
         let reservesAppliedToLayout = reserves;
-        const reservesDiffer =
-          finalReserves.length !== reserves.length ||
-          finalReserves.some((h, i) => (reserves[i] ?? 0) !== h) ||
-          reserves.some((h, i) => (finalReserves[i] ?? 0) !== h);
-        if (reservesDiffer) {
-          layout = relayout(finalReserves);
-          reservesAppliedToLayout = finalReserves;
+        // SD-1680: the post-loop can still mismatch the body reserve and plan placement when
+        // relayouting with finalPlan.reserves shifts footnote refs between pages (the newly
+        // relaxed page now holds refs the old reserves didn't account for). Iterate a few
+        // times, each step taking the element-wise max of current reserves and the new plan's
+        // reserves, so the final layout's reservation on every page is at least as large as
+        // the demand from the final ref assignment. This guarantees placements stay inside
+        // the band and cannot render past the page's bottom margin.
+        const MAX_POST_PASSES = 3;
+        for (let postPass = 0; postPass < MAX_POST_PASSES; postPass += 1) {
+          const target = reservesAppliedToLayout.slice();
+          const planReserves = finalPlan.reserves;
+          const len = Math.max(target.length, planReserves.length);
+          let needsRelayout = false;
+          for (let i = 0; i < len; i += 1) {
+            const applied = target[i] ?? 0;
+            const needed = planReserves[i] ?? 0;
+            if (needed > applied) {
+              target[i] = needed;
+              needsRelayout = true;
+            }
+          }
+          if (!needsRelayout) break;
+          layout = relayout(target);
+          reservesAppliedToLayout = target;
           ({ columns: finalPageColumns, idsByColumn: finalIdsByColumn } = resolveFootnoteAssignments(layout));
           ({ blocks: finalBlocks, measuresById: finalMeasuresById } = await measureFootnoteBlocks(
             collectFootnoteIdsByColumn(finalIdsByColumn),
diff --git a/packages/layout-engine/layout-bridge/test/footnoteBandOverflow.test.ts b/packages/layout-engine/layout-bridge/test/footnoteBandOverflow.test.ts
@@ -0,0 +1,224 @@
+/**
+ * SD-1680: Footnotes render past the bottom page margin when multiple footnotes
+ * need more total height than the reserved band allows. Verifies that the plan
+ * output is internally consistent with the body layout's bottom margin — i.e.,
+ * no footnote fragment's bottom-Y exceeds the top of the physical bottom margin.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import type { FlowBlock, Measure, Fragment } from '@superdoc/contracts';
+import { incrementalLayout } from '../src/incrementalLayout';
+
+const makeParagraph = (id: string, text: string, pmStart: number): FlowBlock => ({
+  kind: 'paragraph',
+  id,
+  runs: [{ text, fontFamily: 'Arial', fontSize: 12, pmStart, pmEnd: pmStart + text.length }],
+});
+
+const makeSingleLineMeasure = (lineHeight: number, textLength: number): Measure => ({
+  kind: 'paragraph',
+  lines: [
+    {
+      fromRun: 0,
+      fromChar: 0,
+      toRun: 0,
+      toChar: textLength,
+      width: 200,
+      ascent: lineHeight * 0.8,
+      descent: lineHeight * 0.2,
+      lineHeight,
+    },
+  ],
+  totalHeight: lineHeight,
+});
+
+const makeMultiLineMeasure = (lineHeight: number, lineCount: number): Measure => {
+  const lines = Array.from({ length: lineCount }, (_, i) => ({
+    fromRun: 0,
+    fromChar: i,
+    toRun: 0,
+    toChar: i + 1,
+    width: 200,
+    ascent: lineHeight * 0.8,
+    descent: lineHeight * 0.2,
+    lineHeight,
+  }));
+  return { kind: 'paragraph', lines, totalHeight: lineCount * lineHeight };
+};
+
+const PAGE_PHYSICAL_BOTTOM_MARGIN = 72;
+const BODY_LINE_HEIGHT = 20;
+const FOOTNOTE_LINE_HEIGHT = 12;
+
+describe('SD-1680: Footnote band must not overflow the reserved area', () => {
+  /**
+   * Scenario: multiple medium-size footnotes compete for a single page's band.
+   * Page body fills to have all 4 refs on page 1. Each footnote is long enough that
+   * the total needed reserve exceeds what a single pass' body layout anticipated.
+   * Before the fix, footnotes stacked past the reserved band and rendered past the
+   * page's physical bottom margin. After the fix, excess footnotes must be
+   * pushed to a later page (or split) and the band must stay within its reserve.
+   */
+  it('keeps every footnote fragment within the band on multi-footnote pages', async () => {
+    // Realistic-ish scenario: ~20 body paragraphs across 2-3 pages, with 2 footnote
+    // refs per page. Each footnote is medium-sized (4 lines). Ample body slack so
+    // the layout converges within MAX_FOOTNOTE_LAYOUT_PASSES.
+    const BODY_PARAS = 20;
+    const FOOTNOTE_LINES = 4;
+    const FOOTNOTE_COUNT = 4;
+
+    let pos = 0;
+    const bodyBlocks: FlowBlock[] = [];
+    const refs: Array<{ id: string; pos: number }> = [];
+    const blocksById = new Map<string, FlowBlock[]>();
+
+    for (let i = 0; i < BODY_PARAS; i += 1) {
+      const text = `Body paragraph ${i + 1}.`;
+      const para = makeParagraph(`body-${i}`, text, pos);
+      bodyBlocks.push(para);
+
+      // Spread refs so 2 land on each page
+      if (refs.length < FOOTNOTE_COUNT && i % 3 === 1) {
+        const refId = String(refs.length + 1);
+        refs.push({ id: refId, pos: pos + 2 });
+        const fnBlock = makeParagraph(
+          `footnote-${refId}-0-paragraph`,
+          `Footnote ${refId} content spanning multiple lines.`,
+          0,
+        );
+        blocksById.set(refId, [fnBlock]);
+      }
+
+      pos += text.length + 1;
+    }
+
+    const measureBlock = vi.fn(async (block: FlowBlock) => {
+      if (block.id.startsWith('footnote-')) {
+        return makeMultiLineMeasure(FOOTNOTE_LINE_HEIGHT, FOOTNOTE_LINES);
+      }
+      return makeSingleLineMeasure(BODY_LINE_HEIGHT, 20);
+    });
+
+    // Page holds ~10 body paragraphs (200px). With footnote reserve, body shrinks
+    // enough that refs can move between pages, but everything must stay bounded.
+    const contentHeight = 10 * BODY_LINE_HEIGHT; // 200px
+    const pageHeight = contentHeight + 72 + PAGE_PHYSICAL_BOTTOM_MARGIN;
+
+    const result = await incrementalLayout(
+      [],
+      null,
+      bodyBlocks,
+      {
+        pageSize: { w: 612, h: pageHeight },
+        margins: { top: 72, right: 72, bottom: PAGE_PHYSICAL_BOTTOM_MARGIN, left: 72 },
+        footnotes: {
+          refs,
+          blocksById,
+          topPadding: 4,
+          dividerHeight: 2,
+        },
+      },
+      measureBlock,
+    );
+
+    const { layout } = result;
+    const pageH = pageHeight;
+
+    // INVARIANT: on every page, every footnote fragment's bottom edge must be ≤ the
+    // top of the physical bottom margin (pageH - PAGE_PHYSICAL_BOTTOM_MARGIN).
+    // Anything below that point has rendered past the reserved band into the
+    // footer area — the core SD-1680 bug.
+    const overflows: string[] = [];
+    for (const page of layout.pages) {
+      const pageBottomLimit = (page.size?.h ?? pageH) - PAGE_PHYSICAL_BOTTOM_MARGIN;
+      const footnoteFragments = page.fragments.filter(
+        (f: Fragment) =>
+          typeof f.blockId === 'string' && f.blockId.startsWith('footnote-') && !f.blockId.includes('separator'),
+      );
+      for (const f of footnoteFragments) {
+        // For paragraph fragments, height is computed from fromLine/toLine * lineHeight.
+        // The footnote measure's totalHeight is FOOTNOTE_LINES * FOOTNOTE_LINE_HEIGHT.
+        let fragHeight = 0;
+        if (f.kind === 'para' && typeof f.toLine === 'number' && typeof f.fromLine === 'number') {
+          fragHeight = (f.toLine - f.fromLine) * FOOTNOTE_LINE_HEIGHT;
+        } else if (typeof (f as { height?: number }).height === 'number') {
+          fragHeight = (f as { height: number }).height;
+        }
+        const fragBottom = (f.y ?? 0) + fragHeight;
+        if (fragBottom > pageBottomLimit + 1) {
+          overflows.push(
+            `page ${page.number}: fragment ${f.blockId} bottom=${fragBottom.toFixed(1)} exceeds limit ${pageBottomLimit.toFixed(1)} by ${(fragBottom - pageBottomLimit).toFixed(1)}px`,
+          );
+        }
+      }
+    }
+
+    expect(overflows).toEqual([]);
+  });
+
+  /**
+   * A single huge footnote (taller than the page's max reserve) must be split
+   * across pages with a continuation on the next page — never rendered as one
+   * monolithic block that extends off the page.
+   */
+  it('splits an oversized footnote across pages rather than overflowing', async () => {
+    const BIG_FOOTNOTE_LINES = 30; // way bigger than a single page's band
+    const refPos = 5;
+
+    const bodyBlocks: FlowBlock[] = [makeParagraph('body-0', 'Body before footnote ref.', 0)];
+    const footnoteBlock = makeParagraph(
+      'footnote-1-0-paragraph',
+      'A very long footnote that should not fit on a single page band.',
+      0,
+    );
+
+    const measureBlock = vi.fn(async (block: FlowBlock) => {
+      if (block.id.startsWith('footnote-')) {
+        return makeMultiLineMeasure(FOOTNOTE_LINE_HEIGHT, BIG_FOOTNOTE_LINES);
+      }
+      return makeSingleLineMeasure(BODY_LINE_HEIGHT, 26);
+    });
+
+    const contentHeight = 200;
+    const pageHeight = contentHeight + 72 + PAGE_PHYSICAL_BOTTOM_MARGIN;
+
+    const result = await incrementalLayout(
+      [],
+      null,
+      bodyBlocks,
+      {
+        pageSize: { w: 612, h: pageHeight },
+        margins: { top: 72, right: 72, bottom: PAGE_PHYSICAL_BOTTOM_MARGIN, left: 72 },
+        footnotes: {
+          refs: [{ id: '1', pos: refPos }],
+          blocksById: new Map([['1', [footnoteBlock]]]),
+          topPadding: 4,
+          dividerHeight: 2,
+        },
+      },
+      measureBlock,
+    );
+
+    const { layout } = result;
+    const pageBottomLimit = pageHeight - PAGE_PHYSICAL_BOTTOM_MARGIN;
+
+    // No single fragment may extend past the page's bottom limit.
+    for (const page of layout.pages) {
+      const pageBottomThisPage = (page.size?.h ?? pageHeight) - PAGE_PHYSICAL_BOTTOM_MARGIN;
+      const footnoteFragments = page.fragments.filter(
+        (f: Fragment) =>
+          typeof f.blockId === 'string' && f.blockId.startsWith('footnote-') && !f.blockId.includes('separator'),
+      );
+      for (const f of footnoteFragments) {
+        let fragHeight = 0;
+        if (f.kind === 'para' && typeof f.toLine === 'number' && typeof f.fromLine === 'number') {
+          fragHeight = (f.toLine - f.fromLine) * FOOTNOTE_LINE_HEIGHT;
+        } else if (typeof (f as { height?: number }).height === 'number') {
+          fragHeight = (f as { height: number }).height;
+        }
+        const fragBottom = (f.y ?? 0) + fragHeight;
+        expect(fragBottom).toBeLessThanOrEqual(pageBottomThisPage + 1);
+      }
+    }
+  });
+});
