fix: review feedback

Author: harbournick

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

@@ -210,16 +210,31 @@ const logClickStage = (_level: 'log' | 'warn' | 'error', _stage: string, _payloa
   // No-op in production. Enable for debugging click-to-position mapping.
 };
 
+const readSelectionDebugEnabled = (): boolean => {
+  if (typeof globalThis === 'undefined') return false;
+  return (globalThis as { __sdSelectionDebug?: boolean }).__sdSelectionDebug === true;
+};
+
 const SELECTION_DEBUG_ENABLED = false;
 const logSelectionDebug = (payload: Record<string, unknown>): void => {
-  if (!SELECTION_DEBUG_ENABLED) return;
+  const enabled = SELECTION_DEBUG_ENABLED || readSelectionDebugEnabled();
+  if (!enabled) return;
   try {
     console.log('[SELECTION-DEBUG]', JSON.stringify(payload));
   } catch {
     console.log('[SELECTION-DEBUG]', payload);
   }
 };
 
+const pushSelectionDebugSnapshot = (payload: Record<string, unknown>): void => {
+  if (typeof globalThis === 'undefined') return;
+  const target = globalThis as { __sdSelectionDebugLog?: Record<string, unknown>[] };
+  if (!Array.isArray(target.__sdSelectionDebugLog)) {
+    target.__sdSelectionDebugLog = [];
+  }
+  target.__sdSelectionDebugLog.push(payload);
+};
+
 /**
  * Debug flag for DOM and geometry position mapping.
  * Set to true to enable detailed logging of click-to-position operations.
@@ -636,7 +651,8 @@ export function selectionToRects(
             pageIndex,
           });
 
-          if (SELECTION_DEBUG_ENABLED) {
+          const selectionDebugEnabled = SELECTION_DEBUG_ENABLED || readSelectionDebugEnabled();
+          if (selectionDebugEnabled) {
             const runs = block.runs.slice(line.fromRun, line.toRun + 1).map((run: Run, idx: number) => {
               const isAtomic =
                 'src' in run ||
@@ -657,7 +673,7 @@ export function selectionToRects(
               };
             });
 
-            debugEntries.push({
+            const debugEntry = {
               pageIndex,
               blockId: block.id,
               lineIndex: index,
@@ -690,9 +706,18 @@ export function selectionToRects(
                 Math.max(charOffsetFrom, charOffsetTo),
               ),
               indent: (block.attrs as { indent?: unknown } | undefined)?.indent,
+              alignment: (block.attrs as { alignment?: unknown } | undefined)?.alignment,
               marker: measure.marker,
+              markerWidth,
+              isListItemFlag,
+              alignmentOverride,
               lineSegments: line.segments,
-            });
+              lineSpaceCount: (line as { spaceCount?: unknown }).spaceCount,
+              lineNaturalWidth: (line as { naturalWidth?: unknown }).naturalWidth,
+              lineMaxWidth: (line as { maxWidth?: unknown }).maxWidth,
+            };
+            debugEntries.push(debugEntry);
+            pushSelectionDebugSnapshot(debugEntry);
           }
         });
         return;

packages/super-editor/src/editors/v1/core/header-footer/HeaderFooterPerRidLayout.test.ts

@@ -129,4 +129,73 @@ describe('layoutPerRIdHeaderFooters', () => {
     expect(deps.headerLayoutsByRId.has('rId-header-first')).toBe(true);
     expect(deps.headerLayoutsByRId.has('rId-header-orphan')).toBe(false);
   });
+
+  it('lays out first-page header refs in multi-section documents with per-section constraints', async () => {
+    const headerBlocksByRId = new Map<string, FlowBlock[]>([
+      ['rId-header-default', [makeBlock('block-default')]],
+      ['rId-header-first', [makeBlock('block-first')]],
+      ['rId-header-section-1', [makeBlock('block-section-1')]],
+    ]);
+
+    const headerFooterInput = {
+      headerBlocksByRId,
+      footerBlocksByRId: undefined,
+      headerBlocks: undefined,
+      footerBlocks: undefined,
+      constraints: {
+        width: 400,
+        height: 80,
+        pageWidth: 600,
+        pageHeight: 800,
+        margins: {
+          top: 50,
+          right: 50,
+          bottom: 50,
+          left: 50,
+          header: 20,
+        },
+      },
+    };
+
+    const layout = {
+      pages: [
+        { number: 1, fragments: [], sectionIndex: 0 },
+        { number: 2, fragments: [], sectionIndex: 1 },
+      ],
+    } as unknown as Layout;
+
+    const sectionMetadata: SectionMetadata[] = [
+      {
+        sectionIndex: 0,
+        margins: { top: 50, right: 50, bottom: 50, left: 50, header: 20 },
+        headerRefs: {
+          default: 'rId-header-default',
+          first: 'rId-header-first',
+        },
+      },
+      {
+        sectionIndex: 1,
+        margins: { top: 55, right: 55, bottom: 55, left: 55, header: 20 },
+        headerRefs: {
+          default: 'rId-header-section-1',
+        },
+      },
+    ];
+
+    const deps = {
+      headerLayoutsByRId: new Map(),
+      footerLayoutsByRId: new Map(),
+    };
+
+    await layoutPerRIdHeaderFooters(headerFooterInput, layout, sectionMetadata, deps);
+
+    const laidOutBlockIds = new Set(
+      mockLayoutHeaderFooterWithCache.mock.calls.map((call) => call[0].default?.[0]?.id).filter(Boolean),
+    );
+
+    expect(laidOutBlockIds).toEqual(new Set(['block-default', 'block-first', 'block-section-1']));
+    expect(deps.headerLayoutsByRId.has('rId-header-default::s0')).toBe(true);
+    expect(deps.headerLayoutsByRId.has('rId-header-first::s0')).toBe(true);
+    expect(deps.headerLayoutsByRId.has('rId-header-section-1::s1')).toBe(true);
+  });
 });

packages/super-editor/src/editors/v1/core/header-footer/HeaderFooterPerRidLayout.ts

@@ -189,36 +189,6 @@ function collectReferencedRIdsBySection(effectiveRefsBySection: Map<number, Head
   return result;
 }
 
-/**
- * Resolve the default header/footer rId for each section.
- *
- * Multi-section layout has historically measured only the default variant with
- * section-specific constraints. Preserve that behavior to avoid changing
- * established rendering for documents that use first/even/odd variants.
- */
-function resolveDefaultRIdPerSection(
-  sectionMetadata: SectionMetadata[],
-  kind: 'header' | 'footer',
-): Map<number, string> {
-  const result = new Map<number, string>();
-  let inheritedDefaultRId: string | undefined;
-
-  for (const section of sectionMetadata) {
-    const refs = getRefsForKind(section, kind);
-    const explicitDefaultRId = refs?.default;
-
-    if (explicitDefaultRId) {
-      inheritedDefaultRId = explicitDefaultRId;
-    }
-
-    if (inheritedDefaultRId) {
-      result.set(section.sectionIndex, inheritedDefaultRId);
-    }
-  }
-
-  return result;
-}
-
 /**
  * Layout header/footer blocks per rId, respecting per-section margins.
  *
@@ -411,7 +381,7 @@ async function layoutWithPerSectionConstraints(
 ): Promise<void> {
   if (!blocksByRId) return;
 
-  const defaultRIdPerSection = resolveDefaultRIdPerSection(sectionMetadata, kind);
+  const effectiveRefsBySection = buildEffectiveRefsBySection(sectionMetadata, kind);
 
   // Extract table width specs per rId (SD-1837).
   // Word allows tables in headers/footers to extend beyond content margins.
@@ -429,36 +399,48 @@ async function layoutWithPerSectionConstraints(
   // Key: `${rId}::w${effectiveWidth}`, Value: { constraints, sections[] }
   const groups = new Map<
     string,
-    { sectionConstraints: Constraints; sectionIndices: number[]; rId: string; effectiveWidth: number }
+    { sectionConstraints: Constraints; sectionIndices: Set<number>; rId: string; effectiveWidth: number }
   >();
 
   for (const section of sectionMetadata) {
-    const rId = defaultRIdPerSection.get(section.sectionIndex);
-    if (!rId || !blocksByRId.has(rId)) continue;
-
-    // Resolve the minimum width needed for tables in this section.
-    // For pct tables, this depends on the section's content width.
-    const contentWidth = buildSectionContentWidth(section, fallbackConstraints);
-    const tableWidthSpec = tableWidthSpecByRId.get(rId);
-    const tableMinWidth = resolveTableMinWidth(tableWidthSpec, contentWidth);
-    const sectionConstraints = buildConstraintsForSection(section, fallbackConstraints, tableMinWidth || undefined);
-    const effectiveWidth = sectionConstraints.width;
-    // Include vertical geometry in the key so sections with different page heights,
-    // vertical margins, or header distance get separate layouts (page-relative anchors
-    // and header band origin resolve differently).
-    const groupKey = `${rId}::w${effectiveWidth}::ph${sectionConstraints.pageHeight ?? ''}::mt${sectionConstraints.margins?.top ?? ''}::mb${sectionConstraints.margins?.bottom ?? ''}::mh${sectionConstraints.margins?.header ?? ''}`;
-
-    let group = groups.get(groupKey);
-    if (!group) {
-      group = {
-        sectionConstraints,
-        sectionIndices: [],
-        rId,
-        effectiveWidth,
-      };
-      groups.set(groupKey, group);
+    const refs = effectiveRefsBySection.get(section.sectionIndex);
+    if (!refs) continue;
+
+    const uniqueRIds = new Set<string>();
+    for (const variant of HEADER_FOOTER_VARIANTS) {
+      const rId = refs[variant];
+      if (rId) {
+        uniqueRIds.add(rId);
+      }
+    }
+
+    for (const rId of uniqueRIds) {
+      if (!blocksByRId.has(rId)) continue;
+
+      // Resolve the minimum width needed for tables in this section.
+      // For pct tables, this depends on the section's content width.
+      const contentWidth = buildSectionContentWidth(section, fallbackConstraints);
+      const tableWidthSpec = tableWidthSpecByRId.get(rId);
+      const tableMinWidth = resolveTableMinWidth(tableWidthSpec, contentWidth);
+      const sectionConstraints = buildConstraintsForSection(section, fallbackConstraints, tableMinWidth || undefined);
+      const effectiveWidth = sectionConstraints.width;
+      // Include vertical geometry in the key so sections with different page heights,
+      // vertical margins, or header distance get separate layouts (page-relative anchors
+      // and header band origin resolve differently).
+      const groupKey = `${rId}::w${effectiveWidth}::ph${sectionConstraints.pageHeight ?? ''}::mt${sectionConstraints.margins?.top ?? ''}::mb${sectionConstraints.margins?.bottom ?? ''}::mh${sectionConstraints.margins?.header ?? ''}`;
+
+      let group = groups.get(groupKey);
+      if (!group) {
+        group = {
+          sectionConstraints,
+          sectionIndices: new Set<number>(),
+          rId,
+          effectiveWidth,
+        };
+        groups.set(groupKey, group);
+      }
+      group.sectionIndices.add(section.sectionIndex);
     }
-    group.sectionIndices.push(section.sectionIndex);
   }
 
   // Measure and layout each unique (rId, effectiveWidth) group

packages/super-editor/src/editors/v1/core/header-footer/HeaderFooterRegistry.test.ts

@@ -546,6 +546,61 @@ describe('HeaderFooterLayoutAdapter', () => {
     expect(options?.storyKey).toBe('hf:part:rId-header-default');
   });
 
+  it('passes tracked change render config through to header/footer flow blocks', () => {
+    const descriptor = { id: 'rId-header-default', kind: 'header', variant: 'default' };
+    const doc = { type: 'doc', content: [{ type: 'paragraph' }] };
+
+    const manager = {
+      rootEditor: {
+        converter: {
+          convertedXml: {},
+          numbering: {},
+          linkedStyles: {},
+        },
+      },
+      getDescriptors: (kind: string) => (kind === 'header' ? [descriptor] : []),
+      getDocumentJson: vi.fn(() => doc),
+    } as unknown as HeaderFooterEditorManager;
+
+    const adapter = new HeaderFooterLayoutAdapter(manager);
+    adapter.setTrackedChangesRenderConfig({ mode: 'final', enabled: false });
+
+    mockToFlowBlocks.mockClear();
+    adapter.getBatch('header');
+
+    const [, options] = mockToFlowBlocks.mock.calls[0] || [];
+    expect(options?.trackedChangesMode).toBe('final');
+    expect(options?.enableTrackedChanges).toBe(false);
+  });
+
+  it('invalidates cached header/footer flow blocks when tracked change render config changes', () => {
+    const descriptor = { id: 'rId-header-default', kind: 'header', variant: 'default' };
+    const doc = { type: 'doc', content: [{ type: 'paragraph' }] };
+
+    const manager = {
+      rootEditor: {
+        converter: {
+          convertedXml: {},
+          numbering: {},
+          linkedStyles: {},
+        },
+      },
+      getDescriptors: (kind: string) => (kind === 'header' ? [descriptor] : []),
+      getDocumentJson: vi.fn(() => doc),
+    } as unknown as HeaderFooterEditorManager;
+
+    const adapter = new HeaderFooterLayoutAdapter(manager);
+
+    mockToFlowBlocks.mockClear();
+    adapter.getBatch('header');
+    adapter.getBatch('header');
+    expect(mockToFlowBlocks).toHaveBeenCalledTimes(1);
+
+    adapter.setTrackedChangesRenderConfig({ mode: 'final', enabled: true });
+    adapter.getBatch('header');
+    expect(mockToFlowBlocks).toHaveBeenCalledTimes(2);
+  });
+
   it('returns undefined when no descriptors have FlowBlocks', () => {
     const manager = {
       getDescriptors: () => [{ id: 'missing', kind: 'header', variant: 'default' }],