fix(page-number): address PAGE field review feedback

Author: luccas-harbour

packages/layout-engine/layout-bridge/src/layoutHeaderFooter.ts

@@ -1,4 +1,11 @@
-import type { FlowBlock, HeaderFooterLayout, Measure, ParagraphBlock, TableBlock } from '@superdoc/contracts';
+import type {
+  FlowBlock,
+  HeaderFooterLayout,
+  ListBlock,
+  Measure,
+  ParagraphBlock,
+  TableBlock,
+} from '@superdoc/contracts';
 import { layoutHeaderFooter, type HeaderFooterConstraints } from '@superdoc/layout-engine';
 import { MeasureCache } from './cache';
 import { resolveHeaderFooterTokens, cloneHeaderFooterBlocks } from './resolveHeaderFooterTokens';
@@ -143,6 +150,11 @@ function hasPageTokens(blocks: FlowBlock[]): boolean {
   for (const block of blocks) {
     if (block.kind === 'paragraph') {
       if (paragraphHasPageToken(block as ParagraphBlock)) return true;
+    } else if (block.kind === 'list') {
+      const list = block as ListBlock;
+      for (const item of list.items ?? []) {
+        if (paragraphHasPageToken(item.paragraph)) return true;
+      }
     } else if (block.kind === 'table') {
       // SD-1332: PAGE fields can live inside table cells in headers/footers
       // (Word's typical layout). Skipping tables here would take the
@@ -168,6 +180,11 @@ function hasPageNumberTokensRequiringPerPageLayout(blocks: FlowBlock[]): boolean
   for (const block of blocks) {
     if (block.kind === 'paragraph') {
       if (paragraphRequiresPerPageLayout(block as ParagraphBlock)) return true;
+    } else if (block.kind === 'list') {
+      const list = block as ListBlock;
+      for (const item of list.items ?? []) {
+        if (paragraphRequiresPerPageLayout(item.paragraph)) return true;
+      }
     } else if (block.kind === 'table') {
       const table = block as TableBlock;
       for (const row of table.rows ?? []) {

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

@@ -89,6 +89,23 @@ const makePageTokenBlock = (id: string): FlowBlock => ({
   ],
 });
 
+const makeFormattedPageTokenBlock = (
+  id: string,
+  pageNumberFieldFormat: NonNullable<TextRun['pageNumberFieldFormat']>,
+): FlowBlock => ({
+  kind: 'paragraph',
+  id,
+  runs: [
+    {
+      text: '0',
+      token: 'pageNumber',
+      pageNumberFieldFormat,
+      fontFamily: 'Arial',
+      fontSize: 12,
+    } as TextRun,
+  ],
+});
+
 describe('getBucketForPageNumber', () => {
   it('should return d1 for single-digit page numbers (1-9)', () => {
     expect(getBucketForPageNumber(1)).toBe('d1');
@@ -440,6 +457,34 @@ describe('layoutHeaderFooterWithCache - Digit Bucketing (Large Docs)', () => {
     expect(measureBlock).toHaveBeenCalledTimes(3);
     expect((result.default?.layout.pages[0].blocks?.[0] as ParagraphBlock).runs[1].text).toBe('005');
   });
+
+  it.each([
+    ['decimal', { format: 'decimal' }],
+    ['numberInDash', { format: 'numberInDash' }],
+  ] as const)('should keep bucketing for %s run-local page number format', async (_name, pageNumberFieldFormat) => {
+    const sections = {
+      default: [makeFormattedPageTokenBlock(`header-${pageNumberFieldFormat.format}`, pageNumberFieldFormat)],
+    };
+
+    const pageResolver: PageResolver = (pageNum) => ({
+      displayText: String(pageNum),
+      displayNumber: pageNum,
+      totalPages: 150,
+    });
+
+    const measureBlock = vi.fn(async () => makeMeasure(20));
+    const result = await layoutHeaderFooterWithCache(
+      sections,
+      { width: 400, height: 80 },
+      measureBlock,
+      undefined,
+      undefined,
+      pageResolver,
+    );
+
+    expect(result.default?.layout.pages).toHaveLength(3);
+    expect(measureBlock).toHaveBeenCalledTimes(3);
+  });
 });
 
 describe('layoutHeaderFooterWithCache - Section-Aware Token Resolution', () => {

packages/layout-engine/painters/dom/src/renderer-page-context-patch.test.ts

@@ -0,0 +1,116 @@
+import { describe, expect, it } from 'vitest';
+import type { FlowBlock, Layout, Measure, TextRun } from '@superdoc/contracts';
+import { createTestPainter } from './_test-utils.js';
+
+const pageNumberBlock: FlowBlock = {
+  kind: 'paragraph',
+  id: 'page-number-block',
+  runs: [
+    {
+      text: '0',
+      token: 'pageNumber',
+      pageNumberFormat: 'upperRoman',
+      fontFamily: 'Arial',
+      fontSize: 12,
+    } as TextRun,
+  ],
+};
+
+const pageNumberMeasure: Measure = {
+  kind: 'paragraph',
+  lines: [
+    {
+      fromRun: 0,
+      fromChar: 0,
+      toRun: 0,
+      toChar: 1,
+      width: 10,
+      ascent: 8,
+      descent: 2,
+      lineHeight: 10,
+    },
+  ],
+  totalHeight: 10,
+};
+
+const staticBlock: FlowBlock = {
+  kind: 'paragraph',
+  id: 'static-block',
+  runs: [
+    {
+      text: 'Static',
+      fontFamily: 'Arial',
+      fontSize: 12,
+    },
+  ],
+};
+
+const staticMeasure: Measure = {
+  kind: 'paragraph',
+  lines: [
+    {
+      fromRun: 0,
+      fromChar: 0,
+      toRun: 0,
+      toChar: 6,
+      width: 40,
+      ascent: 8,
+      descent: 2,
+      lineHeight: 10,
+    },
+  ],
+  totalHeight: 10,
+};
+
+function makeLayout(displayNumber: number): Layout {
+  return {
+    pageSize: { w: 400, h: 500 },
+    pages: [
+      {
+        number: 1,
+        displayNumber,
+        fragments: [
+          {
+            kind: 'para',
+            blockId: 'page-number-block',
+            fromLine: 0,
+            toLine: 1,
+            x: 0,
+            y: 0,
+            width: 200,
+          },
+          {
+            kind: 'para',
+            blockId: 'static-block',
+            fromLine: 0,
+            toLine: 1,
+            x: 0,
+            y: 20,
+            width: 200,
+          },
+        ],
+      },
+    ],
+  };
+}
+
+describe('DomPainter page-number context patching', () => {
+  it('rebuilds token fragments when display page number changes during incremental patch', () => {
+    const mount = document.createElement('div');
+    document.body.appendChild(mount);
+
+    const painter = createTestPainter({
+      blocks: [pageNumberBlock, staticBlock],
+      measures: [pageNumberMeasure, staticMeasure],
+    });
+
+    painter.paint(makeLayout(5), mount);
+    expect(mount.textContent).toContain('V');
+    const staticFragment = mount.querySelector('[data-block-id="static-block"]');
+    expect(staticFragment).toBeTruthy();
+
+    painter.paint(makeLayout(8), mount);
+    expect(mount.textContent).toContain('VIII');
+    expect(mount.querySelector('[data-block-id="static-block"]')).toBe(staticFragment);
+  });
+});

packages/layout-engine/painters/dom/src/renderer.ts

@@ -38,6 +38,7 @@ import type {
   ResolvedDrawingItem,
   LayoutSourceIdentity,
   LayoutStoryLocator,
+  ListBlock,
 } from '@superdoc/contracts';
 import {
   LAYOUT_BOUNDARY_SCHEMA,
@@ -258,6 +259,59 @@ type PageDomState = {
   fragments: FragmentDomState[];
 };
 
+function pageContextSignature(context: FragmentRenderContext): string {
+  return [
+    context.pageNumber,
+    context.totalPages,
+    context.pageNumberText ?? '',
+    context.pageNumberDisplayNumber ?? '',
+  ].join('|');
+}
+
+function hasPageContextTokenInBlock(block: FlowBlock | undefined): boolean {
+  if (!block) return false;
+  if (block.kind === 'paragraph') {
+    for (const run of (block as ParagraphBlock).runs) {
+      if ('token' in run && (run.token === 'pageNumber' || run.token === 'totalPageCount')) {
+        return true;
+      }
+    }
+  } else if (block.kind === 'list') {
+    const list = block as ListBlock;
+    for (const item of list.items ?? []) {
+      if (hasPageContextTokenInBlock(item.paragraph)) {
+        return true;
+      }
+    }
+  } else if (block.kind === 'table') {
+    const table = block as TableBlock;
+    for (const row of table.rows ?? []) {
+      for (const cell of row.cells ?? []) {
+        const cellBlocks: FlowBlock[] = cell.blocks
+          ? (cell.blocks as FlowBlock[])
+          : cell.paragraph
+            ? [cell.paragraph]
+            : [];
+        if (cellBlocks.some(hasPageContextTokenInBlock)) {
+          return true;
+        }
+      }
+    }
+  }
+  return false;
+}
+
+function needsRebuildForPageContext(
+  currentContext: FragmentRenderContext,
+  nextContext: FragmentRenderContext,
+  resolvedItem: ResolvedPaintItem | undefined,
+): boolean {
+  const block = resolvedItem?.kind === 'fragment' && 'block' in resolvedItem ? resolvedItem.block : undefined;
+  return (
+    pageContextSignature(currentContext) !== pageContextSignature(nextContext) && hasPageContextTokenInBlock(block)
+  );
+}
+
 /**
  * Rendering context passed to fragment renderers containing page metadata.
  * Provides information about the current page position and section for dynamic content like page numbers.
@@ -2297,6 +2351,7 @@ export class DomPainter {
           (current.element.dataset.betweenBorder === 'true') !== (betweenInfo?.showBetweenBorder ?? false) ||
           (current.element.dataset.suppressTopBorder === 'true') !== (betweenInfo?.suppressTopBorder ?? false) ||
           (current.element.dataset.gapBelow ?? '') !== (betweenInfo?.gapBelow ? String(betweenInfo.gapBelow) : '');
+        const pageContextChanged = needsRebuildForPageContext(current.context, contextBase, resolvedItem);
         // Verify the position mapping is reliable: if mapping the old pmStart doesn't produce
         // the expected new pmStart, the mapping is degenerate (e.g. full-document paste) and
         // we must rebuild to get correct span position attributes.
@@ -2312,6 +2367,7 @@ export class DomPainter {
           current.signature !== resolvedSig ||
           sdtBoundaryMismatch ||
           betweenBorderMismatch ||
+          pageContextChanged ||
           mappingUnreliable;
 
         if (needsRebuild) {

packages/super-editor/src/editors/v1/core/super-converter/field-references/fld-preprocessors/page-instruction.js

@@ -1,6 +1,7 @@
 const PAGE_VALUE_FORMAT_SWITCHES = {
   Arabic: 'decimal',
   Roman: 'upperRoman',
+  ROMAN: 'upperRoman',
   roman: 'lowerRoman',
   ALPHABETIC: 'upperLetter',
   alphabetic: 'lowerLetter',

packages/super-editor/src/editors/v1/core/super-converter/field-references/fld-preprocessors/page-preprocessor.test.js

@@ -22,6 +22,7 @@ describe('preProcessPageInstruction', () => {
     ['PAGE', undefined],
     ['PAGE \\* roman', 'lowerRoman'],
     ['PAGE \\* Roman \\* MERGEFORMAT', 'upperRoman'],
+    ['PAGE \\* ROMAN', 'upperRoman'],
     ['page \\* Arabic', 'decimal'],
     ['PAGE \\* Unsupported \\* MERGEFORMAT', undefined],
   ])('preserves PAGE instruction and parses supported value format: %s', (instruction, pageNumberFormat) => {