fix(super-converter): preserve cell-level SDT wrapping table cells (SD-3289)#3539
Conversation
…D-3289) Word date pickers and similar content controls that wrap a single table cell are stored in DOCX as `<w:tr><w:sdt><w:sdtContent><w:tc> ...</w:tc></w:sdtContent></w:sdt></w:tr>` (ECMA-376 Part 1 §17.5.2.32, CT_SdtCell). The row importer previously filtered to direct `w:tc` children only, silently dropping the wrapped cell on import and from any subsequent export. The row encoder now recognizes both direct `<w:tc>` and cell-level `<w:sdt>`, routes the inner cell through the existing tc-translator, and preserves `w:sdtPr`/`w:sdtEndPr` on a new `cellSdt` attr (rendered: false, declared on `tableCell` and `tableHeader`). The row decoder re-wraps the exported `<w:tc>` in `<w:sdt>` when the attr is present, so date metadata (fullDate, dateFormat, lid, calendar) round-trips. - Cell-level controls remain outside the content-controls Document API surface for v1. - Multi-cell `CT_SdtContentCell` wrappers import all cells defensively but drop wrapper metadata; exact multi-cell grouping is out of scope. - Adds 11 unit tests plus a real table import/export round-trip integration test covering the IT-1119 OOXML shape.
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
|
The MCP tools require permissions I don't have. I'll review based on the ECMA-376 spec from prior knowledge of the relevant sections. Status: PASS This PR correctly handles cell-level structured document tags. The key spec elements check out:
The implementation treats the preserved |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66f4433436
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
No issues found across 6 files
Linked issue analysis
Linked issue: SD-3289: Preserve date content controls that wrap table cells
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Import recognizes a cell-level wrapping a single , unwraps it and preserves the wrapper's w:sdtPr / w:sdtEndPr as per-cell metadata (cellSdt) | The row normalizer extracts sdtPr/sdtEndPr and attaches cellSdt to the encoded tableCell when the wrapper contains exactly one w:tc. |
| ✅ | Inner is routed through the existing tc-translator with correct extraParams (columnIndex/columnWidth) rather than being dropped or processed incorrectly | The inner cell is passed to the existing tc translator and unit tests assert the translator was called with the expected extraParams. |
| ✅ | Export re-wraps a table cell carrying cellSdt in a envelope, reconstructing w:sdtPr (and w:sdtEndPr when present) so date subtree metadata (e.g. w:date fullDate and child elements) round-trips | Decoder replaces exported with containing preserved sdtPr/sdtEndPr and sdtContent; integration test verifies w:date attributes survive export and wrapped contains the date text. |
| ✅ | The date text is preserved in the editor model and the exported tree contains exactly one cell with the date (no duplication or accidental drop) | Integration test asserts the imported PM table has the date text in the correct cell and the exported OOXML contains a single wrapped cell with the date text. |
| ✅ | Multi-cell SDT wrappers are handled defensively (do not claim to round-trip multi-cell grouping) — inner cells are imported but wrapper metadata is not incorrectly applied | The normalizer drops wrapper metadata when sdtContent contains multiple w:tc children, emitting each inner cell without cellSdt, matching the PR's stated conservative behavior. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…SD-3289) The row encoder unwraps `<w:sdt><w:sdtContent><w:tc/></w:sdtContent></w:sdt>` into real cells, but `findTableCellAtColumn` (called from `handleTableCellNode` to mark vMerge continuations) still filtered to direct `<w:tc>` children of `<w:tr>`. As a result, a vertical merge whose continuation cell lived inside a cell-level SDT was never marked `_vMergeConsumed`; the row encoder would then re-emit it as a real cell and break the merge round-trip. Extract the row-child normalization to a shared helper (`row-cell-children.js`) and route both the row encoder and the vMerge column lookup through it. The inner `<w:tc>` is now reachable from either side; vMerge marking propagates naturally because both sides see the same node reference. Caught by automated review on PR #3539. Adds a unit test covering a vMerge column with an SDT-wrapped continuation cell.
2 participants
Preserves Word date pickers (and other content controls that wrap a single table cell) on DOCX import/export. The shape is
<w:tr><w:sdt><w:sdtContent><w:tc>...</w:tc></w:sdtContent></w:sdt></w:tr>— ECMA-376 Part 1 §17.5.2.32, CT_SdtCell. The row importer previously filtered to directw:tcchildren only, silently dropping the wrapped cell on import and from any subsequent export. Reported on IT-1119.<w:tc>and cell-level<w:sdt>, routes the inner cell through the existing tc-translator, and preservesw:sdtPr/w:sdtEndPron a newcellSdtattr (rendered: false, declared on bothtableCellandtableHeader).<w:tc>in<w:sdt>when the attr is present, so thew:datesubtree round-trips (fullDate,dateFormat,lid,storeMappedDataAs,calendar).defaultNodeListHandlerandexportSchemaToJsonagainst the IT-1119 OOXML shape with no translator mocks.Out of scope, tracked as follow-ups: content-controls Document API exposure for cell-level controls (
target-resolution.tsonly findsstructuredContent/structuredContentBlock); exact multi-cellCT_SdtContentCellgrouping round-trip; analogous row-level SDT (CT_SdtRow) andw:customXml-wrapped row/cell drops at the same filter sites.Review: row import normalize helper, row decode re-wrap, schema attr on both
tableCellandtableHeader. Ignore: pre-existing TS errors elsewhere intr-translator.js(net -1 from this change).Verified: super-converter suite 2979/2979; tr/ handler dir 32/32 including the round-trip integration test; tsc errors in tr-translator.js 34 → 33.