SD-2343 - Table borders being converted twice

packages/layout-engine/pm-adapter/src/attributes/borders.test.ts

@@ -30,28 +30,28 @@ import {
 
 describe('convertBorderSpec', () => {
   describe('valid borders', () => {
-    it('should convert complete border with all properties', () => {
+    it('should treat already normalized pixel widths as-is', () => {
       const input = { val: 'single', size: 2, color: 'FF0000' };
       const result = convertBorderSpec(input);
       expect(result?.style).toBe('single');
       expect(result?.color).toBe('#FF0000');
-      expect(result?.width).toBeCloseTo(Math.max(0.5, (2 / 8) * (96 / 72)));
+      expect(result?.width).toBe(2);
     });
 
     it('should add # prefix to color if missing', () => {
       const input = { val: 'double', size: 4, color: '00FF00' };
       const result = convertBorderSpec(input);
       expect(result?.style).toBe('double');
       expect(result?.color).toBe('#00FF00');
-      expect(result?.width).toBeCloseTo(Math.max(0.5, (4 / 8) * (96 / 72)));
+      expect(result?.width).toBe(4);
     });
 
     it('should preserve # prefix if already present', () => {
       const input = { val: 'single', size: 1, color: '#0000FF' };
       const result = convertBorderSpec(input);
       expect(result?.style).toBe('single');
       expect(result?.color).toBe('#0000FF');
-      expect(result?.width).toBeCloseTo(Math.max(0.5, (1 / 8) * (96 / 72)));
+      expect(result?.width).toBe(1);
     });
 
     it('should default to black color for auto', () => {
@@ -72,16 +72,22 @@ describe('convertBorderSpec', () => {
       expect(result?.style).toBe('single');
     });
 
-    it('should handle fractional width', () => {
+    it('should handle fractional pixel width', () => {
       const input = { val: 'single', size: 1.5, color: 'FF0000' };
       const result = convertBorderSpec(input);
-      expect(result?.width).toBeCloseTo(Math.max(0.5, (1.5 / 8) * (96 / 72)));
+      expect(result?.width).toBe(1.5);
     });
 
-    it('should convert eighths-of-point sizes to pixels', () => {
-      const input = { val: 'single', size: 16, color: 'FF0000' };
+    it('converts eighth-point sizes when requested', () => {
+      const input = { val: 'single', size: 8, color: 'FF0000' }; // 1pt → 1.333px
+      const result = convertBorderSpec(input, { unit: 'eighthPoints' });
+      expect(result?.width).toBeCloseTo(1.3333, 4);
+    });
+
+    it('should clamp extremely large widths to a reasonable maximum', () => {
+      const input = { val: 'single', size: 2000, color: 'FF0000' };
       const result = convertBorderSpec(input);
-      expect(result?.width).toBeCloseTo((16 / 8) * (96 / 72));
+      expect(result?.width).toBeCloseTo(100);
     });
 
     it('should handle various border styles', () => {
@@ -176,12 +182,12 @@ describe('convertBorderSpec', () => {
 
 describe('convertTableBorderValue', () => {
   describe('valid borders', () => {
-    it('should convert complete border with all properties', () => {
+    it('should keep normalized pixel widths', () => {
       const input = { val: 'single', size: 2, color: 'FF0000' };
       const result = convertTableBorderValue(input);
       expect(result?.style).toBe('single');
       expect(result?.color).toBe('#FF0000');
-      expect(result?.width).toBeCloseTo(Math.max(0.5, (2 / 8) * (96 / 72)));
+      expect(result?.width).toBe(2);
     });
 
     it('should add # prefix to color if missing', () => {
@@ -196,10 +202,16 @@ describe('convertTableBorderValue', () => {
       expect(result?.color).toBe('#000000');
     });
 
-    it('should convert border size units to pixels', () => {
-      const input = { val: 'single', size: 24, color: 'FF0000' };
+    it('should clamp extremely large widths to prevent overflow', () => {
+      const input = { val: 'single', size: 1000, color: 'FF0000' };
       const result = convertTableBorderValue(input);
-      expect(result?.width).toBeCloseTo((24 / 8) * (96 / 72));
+      expect(result?.width).toBe(100);
+    });
+
+    it('converts eighth-point sizes for table borders when requested', () => {
+      const input = { val: 'double', size: 4, color: '00FF00' }; // 0.5pt → 0.666px
+      const result = convertTableBorderValue(input, { unit: 'eighthPoints' });
+      expect(result?.width).toBeCloseTo(0.6666, 3);
     });
   });
 
@@ -272,18 +284,18 @@ describe('extractTableBorders', () => {
   });
 
   describe('raw OOXML borders extraction', () => {
-    it('should convert raw OOXML borders to TableBorderValue format', () => {
+    it('should keep raw OOXML pixel sizes when converting', () => {
       const input = {
         top: { val: 'single', size: 2, color: 'FF0000' },
         bottom: { val: 'double', size: 4, color: '00FF00' },
       };
       const result = extractTableBorders(input);
       expect(result?.top?.style).toBe('single');
       expect(result?.top?.color).toBe('#FF0000');
-      expect(result?.top?.width).toBeCloseTo(Math.max(0.5, (2 / 8) * (96 / 72)));
+      expect(result?.top?.width).toBe(2);
       expect(result?.bottom?.style).toBe('double');
       expect(result?.bottom?.color).toBe('#00FF00');
-      expect(result?.bottom?.width).toBeCloseTo(Math.max(0.5, (4 / 8) * (96 / 72)));
+      expect(result?.bottom?.width).toBe(4);
     });
 
     it('should handle all six border sides from raw OOXML', () => {
@@ -299,6 +311,16 @@ describe('extractTableBorders', () => {
       expect(Object.keys(result!)).toHaveLength(6);
     });
 
+    it('converts eighth-point units when requested', () => {
+      const input = {
+        top: { val: 'single', size: 8 },
+        bottom: { val: 'single', size: 4 },
+      };
+      const result = extractTableBorders(input, { unit: 'eighthPoints' });
+      expect(result?.top?.width).toBeCloseTo(1.3333, 4);
+      expect(result?.bottom?.width).toBeCloseTo(0.6666, 3);
+    });
+
     it('should convert nil borders to {none: true}', () => {
       const input = {
         top: { val: 'single', size: 2 },
@@ -343,16 +365,16 @@ describe('extractCellBorders', () => {
       const result = extractCellBorders(input);
       expect(result?.top?.style).toBe('single');
       expect(result?.top?.color).toBe('#FF0000');
-      expect(result?.top?.width).toBeCloseTo(Math.max(0.5, (1 / 8) * (96 / 72)));
+      expect(result?.top?.width).toBe(1);
       expect(result?.right?.style).toBe('double');
       expect(result?.right?.color).toBe('#00FF00');
-      expect(result?.right?.width).toBeCloseTo(Math.max(0.5, (2 / 8) * (96 / 72)));
+      expect(result?.right?.width).toBe(2);
       expect(result?.bottom?.style).toBe('dashed');
       expect(result?.bottom?.color).toBe('#0000FF');
-      expect(result?.bottom?.width).toBeCloseTo(Math.max(0.5, (3 / 8) * (96 / 72)));
+      expect(result?.bottom?.width).toBe(3);
       expect(result?.left?.style).toBe('dotted');
       expect(result?.left?.color).toBe('#FFFF00');
-      expect(result?.left?.width).toBeCloseTo(Math.max(0.5, (4 / 8) * (96 / 72)));
+      expect(result?.left?.width).toBe(4);
     });
 
     it('should extract partial cell borders', () => {

packages/layout-engine/pm-adapter/src/attributes/borders.ts

@@ -23,6 +23,11 @@ const EIGHTHS_PER_POINT = 8;
 const MIN_BORDER_SIZE_PX = 0.5; // Minimum visible border
 const MAX_BORDER_SIZE_PX = 100; // Reasonable maximum
 
+type BorderConversionUnit = 'px' | 'eighthPoints';
+type BorderConversionOptions = {
+  unit?: BorderConversionUnit;
+};
+
 /**
  * Convert an OOXML border size (stored in eighths of a point) to pixels.
  *
@@ -32,7 +37,7 @@ const MAX_BORDER_SIZE_PX = 100; // Reasonable maximum
  *
  * Clamps results to reasonable bounds to prevent edge cases.
  */
-const borderSizeToPx = (size?: number): number | undefined => {
+export const borderSizeToPx = (size?: number): number | undefined => {
   if (!isFiniteNumber(size)) return undefined;
   if (size <= 0) return 0;
 
@@ -43,6 +48,16 @@ const borderSizeToPx = (size?: number): number | undefined => {
   return Math.min(MAX_BORDER_SIZE_PX, Math.max(MIN_BORDER_SIZE_PX, pixelValue));
 };
 
+const clampPixelBorderWidth = (width: number): number =>
+  Math.min(MAX_BORDER_SIZE_PX, Math.max(MIN_BORDER_SIZE_PX, width));
+
+const resolveBorderWidth = (size: number, unit: BorderConversionUnit): number | undefined => {
+  if (unit === 'eighthPoints') {
+    return borderSizeToPx(size);
+  }
+  return clampPixelBorderWidth(size);
+};
+
 /**
  * Normalizes a border/shading color with a default fallback.
  *
@@ -57,17 +72,18 @@ const normalizeColorWithDefault = (color?: string): string => {
 /**
  * Converts an OOXML border specification to layout engine BorderSpec format.
  *
- * Border sizes are assumed to be in eighths of a point (OOXML standard) if >= 8,
- * otherwise treated as already-converted pixel values. Nil/none borders return
- * a special BorderSpec with style 'none' and width 0.
+ * Border sizes emitted by the DOCX translator are already expressed in pixels,
+ * so the pm-adapter simply clamps them to a safe range instead of converting
+ * from eighths-of-a-point. Nil/none borders return a special BorderSpec with
+ * style 'none' and width 0.
  *
  * @param ooxmlBorder - Raw OOXML border object with optional val, size, and color properties
  * @returns BorderSpec with style, width (in pixels), and color, or undefined if invalid
  *
  * @example
  * ```typescript
- * convertBorderSpec({ val: 'single', size: 16, color: 'FF0000' });
- * // { style: 'single', width: 2.67, color: '#FF0000' }
+ * convertBorderSpec({ val: 'single', size: 4, color: 'FF0000' });
+ * // { style: 'single', width: 4, color: '#FF0000' }
  *
  * convertBorderSpec({ val: 'nil' });
  * // { style: 'none', width: 0 }
@@ -76,7 +92,7 @@ const normalizeColorWithDefault = (color?: string): string => {
  * // undefined
  * ```
  */
-export function convertBorderSpec(ooxmlBorder: unknown): BorderSpec | undefined {
+export function convertBorderSpec(ooxmlBorder: unknown, options?: BorderConversionOptions): BorderSpec | undefined {
   if (!ooxmlBorder || typeof ooxmlBorder !== 'object' || ooxmlBorder === null) {
     return undefined;
   }
@@ -101,11 +117,17 @@ export function convertBorderSpec(ooxmlBorder: unknown): BorderSpec | undefined
     return { style: 'none' as BorderStyle, width: 0 };
   }
 
-  const width = borderSizeToPx(sizeNumber);
-  if (width == null) return undefined;
+  if (!isFiniteNumber(sizeNumber)) return undefined;
+  const numericSize = sizeNumber as number;
+  if (numericSize <= 0) {
+    return { style: 'none' as BorderStyle, width: 0 };
+  }
 
   // Ensure color has # prefix
   const normalizedColor = normalizeColorWithDefault(colorString);
+  const unit: BorderConversionUnit = options?.unit ?? 'px';
+  const width = resolveBorderWidth(numericSize, unit);
+  if (width == null) return undefined;
 
   return {
     style: (val as BorderStyle) || 'single',
@@ -125,14 +147,17 @@ export function convertBorderSpec(ooxmlBorder: unknown): BorderSpec | undefined
  *
  * @example
  * ```typescript
- * convertTableBorderValue({ val: 'single', size: 16, color: 'FF0000' });
- * // { style: 'single', width: 2.67, color: '#FF0000' }
+ * convertTableBorderValue({ val: 'single', size: 4, color: 'FF0000' });
+ * // { style: 'single', width: 4, color: '#FF0000' }
  *
  * convertTableBorderValue({ val: 'nil' });
  * // { none: true }
  * ```
  */
-export function convertTableBorderValue(ooxmlBorder: unknown): TableBorderValue | undefined {
+export function convertTableBorderValue(
+  ooxmlBorder: unknown,
+  options?: BorderConversionOptions,
+): TableBorderValue | undefined {
   if (!ooxmlBorder || typeof ooxmlBorder !== 'object') return undefined;
 
   const border = ooxmlBorder as OoxmlBorder;
@@ -145,10 +170,14 @@ export function convertTableBorderValue(ooxmlBorder: unknown): TableBorderValue
     return { none: true };
   }
 
-  const width = borderSizeToPx(size);
-  if (width == null) return undefined;
+  if (!isFiniteNumber(size)) return undefined;
+  const numericSize = size as number;
+  if (numericSize <= 0) return { none: true };
 
   const normalizedColor = normalizeColorWithDefault(color);
+  const unit: BorderConversionUnit = options?.unit ?? 'px';
+  const width = resolveBorderWidth(numericSize, unit);
+  if (width == null) return undefined;
 
   return {
     style: (val as BorderStyle) || 'single',
@@ -204,7 +233,10 @@ function isTableBorderValue(value: unknown): value is TableBorderValue {
  * @param bordersInput - Record of border definitions for sides (top, left, right, etc.)
  * @returns TableBorders | undefined
  */
-export function extractTableBorders(bordersInput: Record<string, unknown> | undefined): TableBorders | undefined {
+export function extractTableBorders(
+  bordersInput: Record<string, unknown> | undefined,
+  options?: BorderConversionOptions,
+): TableBorders | undefined {
   if (!bordersInput || typeof bordersInput !== 'object') {
     return undefined;
   }
@@ -221,7 +253,7 @@ export function extractTableBorders(bordersInput: Record<string, unknown> | unde
       borders[side] = raw;
     } else {
       // Convert from OOXML
-      const converted = convertTableBorderValue(raw);
+      const converted = convertTableBorderValue(raw, options);
       if (converted !== undefined) {
         borders[side] = converted;
       }

packages/layout-engine/pm-adapter/src/attributes/index.ts

@@ -16,6 +16,7 @@ export {
   normalizeShadingColor,
   mapBorderStyle,
   normalizeBorderSide,
+  borderSizeToPx,
 } from './borders.js';
 
 // Spacing and indent

packages/layout-engine/pm-adapter/src/converters/table-styles.test.ts

@@ -5,6 +5,7 @@ import type { ConverterContext } from '../converter-context.js';
 import type { StylesDocumentProperties } from '@superdoc/style-engine/ooxml';
 
 const emptyStyles: StylesDocumentProperties = { docDefaults: {}, latentStyles: {}, styles: {} };
+const PX_PER_PT = 96 / 72;
 
 const buildContext = (styles?: StylesDocumentProperties): ConverterContext =>
   ({
@@ -58,7 +59,8 @@ describe('hydrateTableStyleAttrs', () => {
     } as unknown as PMNode;
 
     const result = hydrateTableStyleAttrs(table, buildContext(styles));
-    expect(result?.borders).toEqual({ top: { val: 'single', size: 8 } });
+    expect(result?.borders?.top?.style).toBe('single');
+    expect(result?.borders?.top?.width).toBeCloseTo((8 / 8) * PX_PER_PT);
     expect(result?.justification).toBe('center');
     expect(result?.cellPadding?.left).toBeCloseTo((72 / 1440) * 96);
     expect(result?.tableCellSpacing).toEqual({ value: 24, type: 'dxa' });
@@ -91,7 +93,8 @@ describe('hydrateTableStyleAttrs', () => {
 
     const result = hydrateTableStyleAttrs(table, buildContext(styles));
     // Inline borders win over style
-    expect(result?.borders).toEqual({ top: { val: 'single', size: 12 } });
+    expect(result?.borders?.top?.style).toBe('single');
+    expect(result?.borders?.top?.width).toBeCloseTo((12 / 8) * PX_PER_PT);
     // Inline justification wins over style
     expect(result?.justification).toBe('left');
   });
@@ -125,11 +128,15 @@ describe('hydrateTableStyleAttrs', () => {
 
     const result = hydrateTableStyleAttrs(table, buildContext(styles));
     // Inline top wins
-    expect(result?.borders?.top).toEqual({ val: 'double', size: 8 });
+    expect(result?.borders?.top?.style).toBe('double');
+    expect(result?.borders?.top?.width).toBeCloseTo((8 / 8) * PX_PER_PT);
     // Style fills other sides
-    expect(result?.borders?.bottom).toEqual({ val: 'single', size: 4 });
-    expect(result?.borders?.left).toEqual({ val: 'single', size: 4 });
-    expect(result?.borders?.right).toEqual({ val: 'single', size: 4 });
+    expect(result?.borders?.bottom?.style).toBe('single');
+    expect(result?.borders?.bottom?.width).toBeCloseTo((4 / 8) * PX_PER_PT);
+    expect(result?.borders?.left?.style).toBe('single');
+    expect(result?.borders?.left?.width).toBeCloseTo((4 / 8) * PX_PER_PT);
+    expect(result?.borders?.right?.style).toBe('single');
+    expect(result?.borders?.right?.width).toBeCloseTo((4 / 8) * PX_PER_PT);
   });
 
   it('per-side merge: partial inline cellPadding preserves style padding on other sides', () => {
@@ -225,7 +232,8 @@ describe('hydrateTableStyleAttrs', () => {
 
     const result = hydrateTableStyleAttrs(table, buildContext(styles));
     // From TableGrid
-    expect(result?.borders).toEqual({ top: { val: 'single', size: 4 } });
+    expect(result?.borders?.top?.style).toBe('single');
+    expect(result?.borders?.top?.width).toBeCloseTo((4 / 8) * PX_PER_PT);
     // Inherited from TableNormal via basedOn
     expect(result?.cellPadding?.left).toBeCloseTo((108 / 1440) * 96);
     expect(result?.justification).toBe('left');