diff --git a/.github/workflows/spec-review.yml b/.github/workflows/spec-review.yml index ced5d2d330..9e08b4955f 100644 --- a/.github/workflows/spec-review.yml +++ b/.github/workflows/spec-review.yml @@ -62,7 +62,9 @@ jobs: gh pr diff ${{ github.event.pull_request.number }} > /tmp/pr-diff.txt # Run Claude Code review - REVIEW=$(claude --print "Review this PR for OOXML spec compliance. + REVIEW=$(claude --print \ + --allowedTools "mcp__ecma-spec__search_ecma_spec,mcp__ecma-spec__get_section,mcp__ecma-spec__list_parts" \ + "Review this PR for OOXML spec compliance. Context: These are OOXML element handlers (XML ↔ ProseMirror JSON converters). diff --git a/.gitignore b/.gitignore index f51268092b..c80a1729e6 100644 --- a/.gitignore +++ b/.gitignore @@ -64,7 +64,7 @@ reports/ # Planning and progress tracking documents **/plan/progress.md - +wrangler/ codex.config.json perf-baseline-results.json .claude diff --git a/devtools/visual-testing/package.json b/devtools/visual-testing/package.json index aaae51e506..26aad745f3 100644 --- a/devtools/visual-testing/package.json +++ b/devtools/visual-testing/package.json @@ -5,6 +5,7 @@ "type": "module", "packageManager": "pnpm@10.25.0", "scripts": { + "postinstall": "pnpm exec playwright install", "dev": "pnpm --filter @superdoc-testing/harness dev", "build": "pnpm --filter @superdoc-testing/harness build", "preview": "pnpm --filter @superdoc-testing/harness preview", @@ -24,6 +25,8 @@ "get-corpus": "tsx scripts/get-corpus.ts", "get-docx": "tsx scripts/get-docx.ts", "upload": "tsx scripts/upload-corpus-doc.ts", + "r2:upload": "wrangler r2 object put", + "r2:sync-registry": "bash scripts/r2-sync-registry.sh", "filters": "tsx scripts/list-filters.ts", "clear:all": "read -p 'This will delete all baselines, screenshots, and results. Are you sure? (y/N) ' confirm && [ \"$confirm\" = 'y' ] && rm -rf baselines baselines-interactions screenshots results && echo 'Cleared!' || echo 'Aborted.'", "test": "vitest", diff --git a/devtools/visual-testing/pnpm-lock.yaml b/devtools/visual-testing/pnpm-lock.yaml index 7635a575e6..3a5de97f75 100644 --- a/devtools/visual-testing/pnpm-lock.yaml +++ b/devtools/visual-testing/pnpm-lock.yaml @@ -2062,8 +2062,8 @@ packages: resolution: {integrity: sha512-l63NF9y/cLROq/yqKXSLtcMeeyOfnSQlfMSlzFt/K73oIaD8DGaQWd7Z34X9GPiKqP5rbSh84Hl4bOlLcjiSrQ==} superdoc@file:../../packages/superdoc/superdoc.tgz: - resolution: {integrity: sha512-ujI4h2Ru+d9t8xIZmdJazSVhaDA6l2QS3IK2MuThzl0MqSCGjxAKDlfvTeyWA/7WLxQkQzxvY+V8/lCo0+ysqg==, tarball: file:../../packages/superdoc/superdoc.tgz} - version: 1.10.1-next.4 + resolution: {integrity: sha512-uv32JEnNC45h5pivFHQsOOP1rUnUucwHy9fGnWVwr483QNYDJkAXqbkKZVZPizj8+VBxX/ETiLP0tSpG4kXUfw==, tarball: file:../../packages/superdoc/superdoc.tgz} + version: 1.0.2 peerDependencies: '@hocuspocus/provider': ^2.13.6 pdfjs-dist: '>=4.3.136 <=4.6.82' diff --git a/devtools/visual-testing/scripts/r2-sync-registry.sh b/devtools/visual-testing/scripts/r2-sync-registry.sh new file mode 100755 index 0000000000..9fd4914b41 --- /dev/null +++ b/devtools/visual-testing/scripts/r2-sync-registry.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env bash +# Add an uploaded doc to the R2 corpus registry. +# +# Usage: +# pnpm r2:sync-registry tables/cell-shading.docx +# +# Prerequisites: +# wrangler login (one-time browser auth) +# jq (brew install jq) + +set -euo pipefail + +BUCKET="docx-test-corpus" +TMP_DIR="$(mktemp -d)" +REGISTRY_FILE="${TMP_DIR}/registry.json" + +cleanup() { rm -rf "$TMP_DIR"; } +trap cleanup EXIT + +KEY="${1:?Usage: pnpm r2:sync-registry (e.g. tables/cell-shading.docx)}" + +# Download current registry +echo "Fetching registry.json..." +if wrangler r2 object get "${BUCKET}/registry.json" --remote --file "$REGISTRY_FILE" 2>/dev/null; then + echo "Found existing registry." +else + echo "No existing registry — starting fresh." + echo '{"updated_at":"","docs":[]}' > "$REGISTRY_FILE" +fi + +# Derive fields from key +FILENAME="$(basename "$KEY")" +GROUP="$(dirname "$KEY")" +if [ "$GROUP" = "." ]; then + GROUP="" +fi +DOC_ID="$(echo "$KEY" | sed 's/\.docx$//' | tr '/' '-' | tr -c 'A-Za-z0-9._-' '-' | tr '[:upper:]' '[:lower:]' | sed 's/^-//;s/-$//')" + +# Update registry: add or replace entry matching this relative_path +UPDATED="$(jq \ + --arg key "$KEY" \ + --arg doc_id "$DOC_ID" \ + --arg filename "$FILENAME" \ + --arg group "$GROUP" \ + ' + .docs |= ( + [.[] | select(.relative_path != $key)] + + [{ + doc_id: $doc_id, + doc_rev: "manual", + filename: $filename, + group: (if $group == "" then null else $group end), + relative_path: $key + }] + | sort_by(.relative_path) + ) + | .updated_at = (now | todate) + ' "$REGISTRY_FILE")" + +echo "$UPDATED" | jq '.' > "$REGISTRY_FILE" + +DOC_COUNT="$(echo "$UPDATED" | jq '.docs | length')" +echo "Registry: ${DOC_COUNT} doc(s). Added: ${KEY} (doc_id=${DOC_ID})" + +# Upload updated registry +echo "Uploading registry.json..." +wrangler r2 object put "${BUCKET}/registry.json" --remote --file "$REGISTRY_FILE" --content-type "application/json" +echo "Done." diff --git a/packages/layout-engine/pm-adapter/src/converters/table.ts b/packages/layout-engine/pm-adapter/src/converters/table.ts index 1534d15c1d..00e944e4fe 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.ts @@ -41,7 +41,7 @@ import { applySdtMetadataToParagraphBlocks, applySdtMetadataToTableBlock, } from '../sdt/index.js'; -import { TableProperties } from '@superdoc/style-engine/ooxml'; +import { TableProperties, resolveTableCellProperties } from '@superdoc/style-engine/ooxml'; type TableParserDependencies = { nextBlockId: BlockIdGenerator; @@ -185,7 +185,17 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { // Table cells can contain paragraphs, images/drawings, structured content blocks, and nested tables. const blocks: (ParagraphBlock | ImageBlock | DrawingBlock | TableBlock)[] = []; + // Resolve table cell properties from the style cascade (wholeTable → bands → conditional → inline) + const inlineTcProps = cellNode.attrs?.tableCellProperties as Record | undefined; + const tableInfo = tableProperties ? { tableProperties, rowIndex, cellIndex, numCells, numRows } : undefined; + const resolvedTcProps = resolveTableCellProperties( + inlineTcProps as Parameters[0], + tableInfo, + context.converterContext?.translatedLinkedStyles, + ); + // Extract cell background color for auto text color resolution + // Priority: inline background attr > resolved style shading const cellBackground = cellNode.attrs?.background as { color?: string } | undefined; let cellBackgroundColor: string | undefined; if (cellBackground && typeof cellBackground.color === 'string') { @@ -198,6 +208,13 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { } } } + // Fall back to resolved style shading if no inline background + if (!cellBackgroundColor && resolvedTcProps?.shading?.fill) { + const fill = resolvedTcProps.shading.fill; + if (fill !== 'auto') { + cellBackgroundColor = fill.startsWith('#') ? fill : `#${fill}`; + } + } // Create enhanced converter context with table style paragraph props for the style cascade // This allows paragraphs inside table cells to inherit table style's pPr @@ -416,6 +433,9 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { if (background && typeof background.color === 'string') { const bgColor = background.color; cellAttrs.background = bgColor.startsWith('#') ? bgColor : `#${bgColor}`; + } else if (cellBackgroundColor) { + // Use resolved style background when no inline background is set + cellAttrs.background = cellBackgroundColor; } const tableCellProperties = cellNode.attrs?.tableCellProperties; diff --git a/packages/layout-engine/style-engine/src/cascade.ts b/packages/layout-engine/style-engine/src/cascade.ts index bc4c1fe77b..293dde176c 100644 --- a/packages/layout-engine/style-engine/src/cascade.ts +++ b/packages/layout-engine/style-engine/src/cascade.ts @@ -11,8 +11,9 @@ */ import { ParagraphProperties, RunFontFamilyProperties, RunProperties } from './ooxml/types'; +import type { TableCellProperties } from './ooxml/styles-types'; -export type PropertyObject = ParagraphProperties | RunProperties; +export type PropertyObject = ParagraphProperties | RunProperties | TableCellProperties; /** * Performs a deep merge on an ordered list of property objects. diff --git a/packages/layout-engine/style-engine/src/ooxml/index.test.ts b/packages/layout-engine/style-engine/src/ooxml/index.test.ts index d47944ae13..1fd091625d 100644 --- a/packages/layout-engine/style-engine/src/ooxml/index.test.ts +++ b/packages/layout-engine/style-engine/src/ooxml/index.test.ts @@ -6,6 +6,7 @@ import { resolveRunProperties, resolveParagraphProperties, resolveCellStyles, + resolveTableCellProperties, type OoxmlResolverParams, } from './index.js'; @@ -414,3 +415,131 @@ describe('ooxml - resolveCellStyles', () => { expect(result).toEqual([{ fontSize: 10 }, { fontSize: 50 }]); }); }); + +describe('ooxml - resolveTableCellProperties', () => { + const gridTable4Styles = { + ...emptyStyles, + styles: { + 'GridTable4-Accent1': { + type: 'table', + tableProperties: { tableStyleRowBandSize: 1, tableStyleColBandSize: 1 }, + tableStyleProperties: { + firstRow: { + tableCellProperties: { + shading: { val: 'clear', color: 'auto', fill: '156082' }, + borders: { top: { val: 'single', color: '156082', size: 4 } }, + }, + }, + band1Horz: { + tableCellProperties: { + shading: { val: 'clear', color: 'auto', fill: 'C1E4F5' }, + }, + }, + wholeTable: { + tableCellProperties: { + shading: { val: 'clear', color: 'auto', fill: 'EEEEEE' }, + }, + }, + }, + }, + }, + }; + + it('resolves firstRow shading from table style', () => { + const tableInfo = { + tableProperties: { + tableStyleId: 'GridTable4-Accent1', + tblLook: { firstRow: true, noHBand: false, noVBand: true }, + }, + rowIndex: 0, + cellIndex: 1, + numRows: 3, + numCells: 4, + }; + const result = resolveTableCellProperties(null, tableInfo, gridTable4Styles); + expect(result.shading).toEqual({ val: 'clear', color: 'auto', fill: '156082' }); + }); + + it('resolves band1Horz shading for data rows', () => { + const tableInfo = { + tableProperties: { + tableStyleId: 'GridTable4-Accent1', + tblLook: { firstRow: true, noHBand: false, noVBand: true }, + }, + rowIndex: 1, + cellIndex: 0, + numRows: 3, + numCells: 4, + }; + const result = resolveTableCellProperties(null, tableInfo, gridTable4Styles); + // band1Horz overrides wholeTable + expect(result.shading).toEqual({ val: 'clear', color: 'auto', fill: 'C1E4F5' }); + }); + + it('falls back to wholeTable when no band matches', () => { + const tableInfo = { + tableProperties: { + tableStyleId: 'GridTable4-Accent1', + tblLook: { firstRow: true, noHBand: true, noVBand: true }, + }, + rowIndex: 1, + cellIndex: 1, + numRows: 3, + numCells: 4, + }; + const result = resolveTableCellProperties(null, tableInfo, gridTable4Styles); + expect(result.shading).toEqual({ val: 'clear', color: 'auto', fill: 'EEEEEE' }); + }); + + it('inline cell shading overrides style shading', () => { + const tableInfo = { + tableProperties: { + tableStyleId: 'GridTable4-Accent1', + tblLook: { firstRow: true, noHBand: false, noVBand: true }, + }, + rowIndex: 0, + cellIndex: 0, + numRows: 3, + numCells: 4, + }; + const inlineProps = { shading: { val: 'clear', color: 'auto', fill: 'FF0000' } }; + const result = resolveTableCellProperties(inlineProps, tableInfo, gridTable4Styles); + expect(result.shading).toEqual({ val: 'clear', color: 'auto', fill: 'FF0000' }); + }); + + it('returns inline props when no table style exists', () => { + const tableInfo = { + tableProperties: {}, + rowIndex: 0, + cellIndex: 0, + numRows: 1, + numCells: 1, + }; + const inlineProps = { shading: { val: 'clear', fill: 'AABBCC' } }; + const result = resolveTableCellProperties(inlineProps, tableInfo, gridTable4Styles); + expect(result.shading).toEqual({ val: 'clear', fill: 'AABBCC' }); + }); + + it('returns empty object when no props available', () => { + const result = resolveTableCellProperties(null, null, null); + expect(result).toEqual({}); + }); + + it('merges borders from style and inline', () => { + const tableInfo = { + tableProperties: { + tableStyleId: 'GridTable4-Accent1', + tblLook: { firstRow: true, noHBand: false, noVBand: true }, + }, + rowIndex: 0, + cellIndex: 0, + numRows: 3, + numCells: 4, + }; + const inlineProps = { borders: { bottom: { val: 'double', color: '000000', size: 8 } } }; + const result = resolveTableCellProperties(inlineProps, tableInfo, gridTable4Styles); + // firstRow style provides top border, inline provides bottom border - both should be present + expect(result.borders?.top).toEqual({ val: 'single', color: '156082', size: 4 }); + expect(result.borders?.bottom).toEqual({ val: 'double', color: '000000', size: 8 }); + }); +}); diff --git a/packages/layout-engine/style-engine/src/ooxml/index.ts b/packages/layout-engine/style-engine/src/ooxml/index.ts index 16c6c97247..44dafd8bc6 100644 --- a/packages/layout-engine/style-engine/src/ooxml/index.ts +++ b/packages/layout-engine/style-engine/src/ooxml/index.ts @@ -9,7 +9,13 @@ import { combineIndentProperties, combineProperties, combineRunProperties } from import type { PropertyObject } from '../cascade.js'; import type { ParagraphProperties, RunProperties } from './types.ts'; import type { NumberingProperties } from './numbering-types.ts'; -import type { StylesDocumentProperties, TableStyleType, TableProperties, TableLookProperties } from './styles-types.ts'; +import type { + StylesDocumentProperties, + TableStyleType, + TableProperties, + TableLookProperties, + TableCellProperties, +} from './styles-types.ts'; export { combineIndentProperties, combineProperties, combineRunProperties }; export type { PropertyObject }; @@ -259,7 +265,7 @@ export function resolveStyleChain( return combineProperties(styleChain); } -export function getNumberingProperties( +export function getNumberingProperties( propertyType: 'paragraphProperties' | 'runProperties', params: OoxmlResolverParams, ilvl: number, @@ -356,7 +362,7 @@ export function resolveDocxFontFamily( } export function resolveCellStyles( - propertyType: 'paragraphProperties' | 'runProperties', + propertyType: 'paragraphProperties' | 'runProperties' | 'tableCellProperties', tableInfo: TableInfo | null | undefined, translatedLinkedStyles: StylesDocumentProperties, ): T[] { @@ -388,6 +394,41 @@ export function resolveCellStyles( return cellStyleProps; } +/** + * Resolve table cell properties (shading, borders, margins) by cascading + * conditional table style properties with inline cell properties. + * + * Cascade order (low → high priority): + * wholeTable → bands → firstRow/lastRow/firstCol/lastCol → corner cells → inline + */ +export function resolveTableCellProperties( + inlineProps: TableCellProperties | null | undefined, + tableInfo: TableInfo | null | undefined, + translatedLinkedStyles: StylesDocumentProperties | null | undefined, +): TableCellProperties { + if (!translatedLinkedStyles) { + return (inlineProps ?? {}) as TableCellProperties; + } + + const cellStyleProps = resolveCellStyles( + 'tableCellProperties', + tableInfo, + translatedLinkedStyles, + ); + + if (cellStyleProps.length === 0) { + return (inlineProps ?? {}) as TableCellProperties; + } + + // Cascade: style properties (low→high) then inline wins last + const chain: TableCellProperties[] = [...cellStyleProps]; + if (inlineProps && Object.keys(inlineProps).length > 0) { + chain.push(inlineProps); + } + + return combineProperties(chain, { fullOverrideProps: ['shading'] }); +} + function determineCellStyleTypes( tblLook: TableLookProperties | null | undefined, rowIndex: number, @@ -401,8 +442,13 @@ function determineCellStyleTypes( const normalizedRowBandSize = rowBandSize > 0 ? rowBandSize : 1; const normalizedColBandSize = colBandSize > 0 ? colBandSize : 1; - const rowGroup = Math.floor(rowIndex / normalizedRowBandSize); - const colGroup = Math.floor(cellIndex / normalizedColBandSize); + + // Per ECMA-376, banding excludes header/footer rows and first/last columns. + // Offset the index so the first data row/column starts at band1. + const bandRowIndex = Math.max(0, rowIndex - (tblLook?.firstRow ? 1 : 0)); + const bandColIndex = Math.max(0, cellIndex - (tblLook?.firstColumn ? 1 : 0)); + const rowGroup = Math.floor(bandRowIndex / normalizedRowBandSize); + const colGroup = Math.floor(bandColIndex / normalizedColBandSize); if (!tblLook?.noHBand) { if (rowGroup % 2 === 0) {