fix: correct spacing for paragraphs with borders/shades (#2586)

* fix: correct spacing for paragraphs with borders/shades

* fix(layout): overlap border expansion with paragraph spacing (SD-2106)

Per ECMA-376 §17.3.1.42, paragraph border space is measured from
the text edge "ignoring any spacing above/below". This means border
expansion should overlap with spacingBefore/After, not stack on top.

- Reduce top border expansion by the amount spacingBefore already covers
- Reduce effective spacingAfter by the bottom border expansion
- Subtract border expansion from sliceLines available height to prevent
  page-bottom overflow
- Include border expansion in keepLines height calculation

* fix(layout): correct cursorY mismatch and continuation fragment overlap

Three fixes from self-review:

- fragment.y now uses extraTop (overlap-adjusted) instead of full
  borderExpansion.top, so cursorY reaches the visual bottom exactly
- Track spacingAppliedThisFragment to avoid under-counting border
  expansion on continuation fragments after page breaks
- keepLines fitsOnBlankPage uses overlap model (max instead of add)
  to avoid unnecessary page breaks for bordered paragraphs

* fix(layout): hoist borderExpansion to function scope

borderExpansion was declared inside the while loop but referenced
in the spacingAfter block outside it, causing a ReferenceError.
Move it before the loop so it's accessible in both places.

* fix(layout,renderer): between-border group detection and spacing

Layout engine:
- Detect between-border groups by comparing adjacent paragraph border
  hashes. When borders match, suppress top expansion and reclaim
  previous paragraph's bottom expansion.
- Border expansion is additive with spacing for non-grouped paragraphs
  (matches Word behavior).
- Add lastParagraphBorderHash to PageState for group tracking.

Renderer (group-analysis.ts):
- Remove requirement for `between` border element to be present for
  grouping. Per ECMA-376 §17.3.1.5, grouping occurs when all border
  properties are identical — a between border is optional.
- When no between border is defined, suppress bottom/top borders
  and render as a single continuous box.

* test(painter-dom): update between-border test for grouping without between element

The test expected no grouping when `between` border is absent. Updated
to expect grouping with suppressBottomBorder per ECMA-376 §17.3.1.5:
identical borders form a group regardless of whether a between border
is defined.

* fix(layout): use local PX_PER_PT constant instead of pm-adapter import

layout-engine cannot import from pm-adapter (boundary removed in #2618).
Define PX_PER_PT locally, same as border-layer.ts does.

---------

Co-authored-by: Johannes Wilm <mail@johanneswilm.org>

Author: caio-pizzol

packages/layout-engine/layout-engine/src/layout-paragraph.ts

@@ -12,6 +12,7 @@ import type {
   DrawingBlock,
   DrawingMeasure,
   DrawingFragment,
+  ParagraphBorders,
 } from '@superdoc/contracts';
 import {
   computeFragmentPmRange,
@@ -24,6 +25,9 @@ import {
 import { computeAnchorX } from './floating-objects.js';
 import { getFragmentZIndex } from '@superdoc/contracts';
 
+/** Points → CSS pixels (96 dpi / 72 pt-per-inch). */
+const PX_PER_PT = 96 / 72;
+
 const spacingDebugEnabled = false;
 /**
  * Type definition for Word layout attributes attached to paragraph blocks.
@@ -89,6 +93,8 @@ type ParagraphBlockAttrs = {
   floatAlignment?: unknown;
   /** Keep all lines of the paragraph on the same page */
   keepLines?: boolean;
+  /** Border attributes for the paragraph */
+  borders?: ParagraphBorders;
 };
 
 const spacingDebugLog = (..._args: unknown[]): void => {
@@ -162,6 +168,39 @@ const asSafeNumber = (value: unknown): number => {
   return value;
 };
 
+/**
+ * Simple hash of paragraph borders for between-border group detection.
+ * Two paragraphs form a group when their border hashes match (ECMA-376 §17.3.1.5).
+ */
+const hashBorders = (borders?: ParagraphBorders): string | undefined => {
+  if (!borders) return undefined;
+  const side = (b?: { style?: string; width?: number; color?: string; space?: number }) =>
+    b ? `${b.style ?? ''},${b.width ?? 0},${b.color ?? ''},${b.space ?? 0}` : '';
+  return `${side(borders.top)}|${side(borders.right)}|${side(borders.bottom)}|${side(borders.left)}|${side(borders.between)}`;
+};
+
+/**
+ * Computes the vertical border expansion for a paragraph fragment.
+ * The border's `space` attribute (in points) plus the border width extends
+ * the visual box beyond the content area. This ensures cursorY accounts
+ * for the full visual height when paragraphs have borders with space.
+ */
+const computeBorderVerticalExpansion = (borders?: ParagraphBorders): { top: number; bottom: number } => {
+  if (!borders) return { top: 0, bottom: 0 };
+
+  // Top border: space (pts) + width (px)
+  const topSpace = (borders.top?.space ?? 0) * PX_PER_PT;
+  const topWidth = borders.top?.width ?? 0;
+  const top = topSpace + topWidth;
+
+  // Bottom border: space (pts) + width (px)
+  const bottomSpace = (borders.bottom?.space ?? 0) * PX_PER_PT;
+  const bottomWidth = borders.bottom?.width ?? 0;
+  const bottom = bottomSpace + bottomWidth;
+
+  return { top, bottom };
+};
+
 /**
  * Calculates the first line indent for list markers when remeasuring paragraphs.
  *
@@ -589,11 +628,33 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para
     }
   }
 
+  // Compute border expansion once per paragraph (constant across fragments).
+  // Border space overlaps with paragraph spacing per ECMA-376 §17.3.1.42:
+  // "the space above the text (ignoring any spacing above)"
+  const rawBorderExpansion = computeBorderVerticalExpansion(attrs?.borders);
+
+  // Between-border group detection (ECMA-376 §17.3.1.5): when adjacent paragraphs
+  // have identical borders, they form a group — top/bottom borders are suppressed
+  // between group members, so the layout engine should not reserve space for them.
+  const currentBorderHash = hashBorders(attrs?.borders);
+  const inBorderGroup = currentBorderHash != null && currentBorderHash === ensurePage().lastParagraphBorderHash;
+  const borderExpansion = {
+    top: inBorderGroup ? 0 : rawBorderExpansion.top,
+    bottom: rawBorderExpansion.bottom, // bottom suppression is handled when the NEXT paragraph joins the group
+  };
+
   // PHASE 2: Layout the paragraph with the remeasured lines
   while (fromLine < lines.length) {
     let state = ensurePage();
     if (state.trailingSpacing == null) state.trailingSpacing = 0;
 
+    // Reclaim the previous paragraph's bottom border expansion when joining a group.
+    // The previous paragraph already reserved space for its bottom border, but in a
+    // group that border is suppressed — so we move cursorY back to close the gap.
+    if (inBorderGroup && fromLine === 0) {
+      state.cursorY -= rawBorderExpansion.bottom;
+    }
+
     /**
      * Contextual Spacing Logic (OOXML w:contextualSpacing)
      *
@@ -635,12 +696,14 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para
      * We use baseSpacingBefore for the blank page check because on a new page there's no
      * previous trailing spacing to collapse with.
      */
+
     const keepLines = attrs?.keepLines === true;
     if (keepLines && fromLine === 0) {
       const prevTrailing = state.trailingSpacing ?? 0;
       const neededSpacingBefore = Math.max(spacingBefore - prevTrailing, 0);
       const pageContentHeight = state.contentBottom - state.topMargin;
-      const fullHeight = lines.reduce((sum, line) => sum + (line.lineHeight || 0), 0);
+      const linesHeight = lines.reduce((sum, line) => sum + (line.lineHeight || 0), 0);
+      const fullHeight = linesHeight + borderExpansion.top + borderExpansion.bottom;
       const fitsOnBlankPage = fullHeight + baseSpacingBefore <= pageContentHeight;
       const remainingHeightAfterSpacing = state.contentBottom - (state.cursorY + neededSpacingBefore);
       if (fitsOnBlankPage && state.page.fragments.length > 0 && fullHeight > remainingHeightAfterSpacing) {
@@ -774,7 +837,11 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para
       offsetX = narrowestOffsetX;
     }
 
-    const slice = sliceLines(lines, fromLine, state.contentBottom - state.cursorY);
+    // Reserve border expansion from available height so sliceLines doesn't accept
+    // lines that would overflow the page once border space is added.
+    const borderVertical = borderExpansion.top + borderExpansion.bottom;
+    const availableForSlice = Math.max(0, state.contentBottom - state.cursorY - borderVertical);
+    const slice = sliceLines(lines, fromLine, availableForSlice);
     const fragmentHeight = slice.height;
 
     // Apply negative indent adjustment to fragment position and width (similar to table indent handling).
@@ -785,14 +852,13 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para
     // Expand width: negative indents on both sides expand the fragment width
     // (e.g., -48px left + -72px right = 120px wider)
     const adjustedWidth = effectiveColumnWidth - negativeLeftIndent - negativeRightIndent;
-
     const fragment: ParaFragment = {
       kind: 'para',
       blockId: block.id,
       fromLine,
       toLine: slice.toLine,
       x: adjustedX,
-      y: state.cursorY,
+      y: state.cursorY + borderExpansion.top,
       width: adjustedWidth,
       ...computeFragmentPmRange(block, lines, fromLine, slice.toLine),
     };
@@ -838,9 +904,9 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para
         fragment.x = columnX(state.columnIndex) + offsetX + (effectiveColumnWidth - maxLineWidth) / 2;
       }
     }
-
     state.page.fragments.push(fragment);
-    state.cursorY += fragmentHeight;
+
+    state.cursorY += borderExpansion.top + fragmentHeight + borderExpansion.bottom;
     lastState = state;
     fromLine = slice.toLine;
   }
@@ -879,5 +945,6 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para
     }
     lastState.lastParagraphStyleId = styleId;
     lastState.lastParagraphContextualSpacing = contextualSpacing;
+    lastState.lastParagraphBorderHash = currentBorderHash;
   }
 }

packages/layout-engine/layout-engine/src/paginator.ts

@@ -18,6 +18,8 @@ export type PageState = {
   trailingSpacing: number;
   lastParagraphStyleId?: string;
   lastParagraphContextualSpacing: boolean;
+  /** Border hash of the last paragraph for between-border group detection. */
+  lastParagraphBorderHash?: string;
 };
 
 export type PaginatorOptions = {

packages/layout-engine/painters/dom/src/between-borders.test.ts

@@ -469,14 +469,22 @@ describe('computeBetweenBorderFlags', () => {
     expect(flags.size).toBe(2);
   });
 
-  it('does not flag when between border is not defined', () => {
+  it('groups identical borders even when between border is not defined', () => {
+    // ECMA-376 §17.3.1.5: grouping occurs when all border properties are identical.
+    // When no between border is defined, the group renders as a single box (no separator).
     const noBetween: ParagraphBorders = { top: { style: 'solid', width: 1 } };
     const b1 = makeParagraphBlock('b1', noBetween);
     const b2 = makeParagraphBlock('b2', noBetween);
     const lookup = buildLookup([{ block: b1 }, { block: b2 }]);
     const fragments: Fragment[] = [paraFragment('b1'), paraFragment('b2')];
 
-    expect(computeBetweenBorderFlags(fragments, lookup).size).toBe(0);
+    const flags = computeBetweenBorderFlags(fragments, lookup);
+    expect(flags.size).toBe(2);
+    // First fragment: bottom border suppressed (no between separator, single box)
+    expect(flags.get(0)?.suppressBottomBorder).toBe(true);
+    expect(flags.get(0)?.showBetweenBorder).toBe(false);
+    // Second fragment: top border suppressed
+    expect(flags.get(1)?.suppressTopBorder).toBe(true);
   });
 
   it('does not flag when border definitions do not match', () => {

packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts

@@ -108,9 +108,13 @@ const isBetweenBorderNone = (borders: ParagraphAttrs['borders']): boolean => {
  * 1. Both are para or list-item (not table/image/drawing)
  * 2. Neither is a page-split continuation
  * 3. They represent different logical paragraphs
- * 4. Both have a `between` border defined (not nil/none)
+ * 4. Both have border definitions
  * 5. Their full border definitions match (same border group)
  *
+ * Per ECMA-376 §17.3.1.5: grouping occurs when all border properties are
+ * identical. A `between` border is NOT required — when absent, the group
+ * is rendered as a single box without a separator line.
+ *
  * For each pair, the first fragment gets:
  * - showBetweenBorder: true — bottom border replaced with between definition
  * - gapBelow: px distance to extend border layer into spacing gap
@@ -134,9 +138,7 @@ export const computeBetweenBorderFlags = (
     if (frag.continuesOnNext) continue;
 
     const borders = getFragmentParagraphBorders(frag, blockLookup);
-    // Skip if no between element at all (no grouping intent).
-    // between: {style: 'none'} (nil/none) IS allowed — it signals grouping without a separator.
-    if (!borders?.between) continue;
+    if (!borders) continue;
 
     const next = fragments[i + 1];
     if (next.kind !== 'para' && next.kind !== 'list-item') continue;
@@ -151,15 +153,17 @@ export const computeBetweenBorderFlags = (
       continue;
 
     const nextBorders = getFragmentParagraphBorders(next, blockLookup);
-    if (!nextBorders?.between) continue;
-    if (hashParagraphBorders(borders!) !== hashParagraphBorders(nextBorders!)) continue;
+    if (!nextBorders) continue;
+    if (hashParagraphBorders(borders) !== hashParagraphBorders(nextBorders)) continue;
 
     // Skip fragments in different columns (different x positions)
     if (frag.x !== next.x) continue;
 
     pairFlags.add(i);
 
-    // Track nil/none between pairs — these get suppressBottomBorder instead of showBetweenBorder
+    // Track nil/none/absent between pairs — these get suppressBottomBorder instead of showBetweenBorder.
+    // Per ECMA-376 §17.3.1.5: grouping happens when ALL borders are identical.
+    // When no between border is defined, the group has no separator line.
     if (isBetweenBorderNone(borders) && isBetweenBorderNone(nextBorders)) {
       noBetweenPairs.add(i);
     }