feat(track-changes): structural tracked changes for whole-table insert/delete#3614
Conversation
…t/delete (SD-3360) Support importing, listing, deciding, and exporting whole-table insert/delete tracked changes in v1. A whole inserted/deleted table is encoded in OOXML as <w:ins>/<w:del> inside each row's <w:trPr> (rows may carry distinct w:ids); v1 previously dropped these on import, never listed them, could not accept/reject them, and lost them on export. - Schema: add a `trackChange` revision slot to the tableRow node. - Import: tr-translator reads <w:ins>/<w:del> in <w:trPr> into the row attr. - Enumerate: group a table's rows into ONE structural change (kind.type='structural', subtype table-insert/table-delete) when every row is tracked and shares one side; ids may differ. Partial/mixed rows are surfaced but undecidable. - Decide: accept-insert/reject-delete clear the rows; reject-insert/ accept-delete remove the table; partial-range fails closed with INVALID_INPUT; non-whole-table shapes fail closed with CAPABILITY_UNAVAILABLE; inline changes inside a removed table are retired. Graph identity is table-scoped to avoid id collisions. - Export: tr-translator emits <w:ins>/<w:del> in <w:trPr> for roundtrip. - Contract: add 'structural' type + table-insert/table-delete subtype to the document-api surface; regenerate artifacts. Validated against the Labs oracle (sdk-v1): the eight failing-known STRUCT-TABLE conformance/roundtrip/safety cases now pass, zero regression. Deferred: visual paint, and row/column/cell + property structural changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
|
I wasn't able to run the live ECMA-376 lookups this session — the Status: FAIL The core idea is sound — a whole inserted/deleted table really is encoded as 1. 2. 3. Note: the If you can grant the |
Visual + review-UI layer for whole-table insert/delete tracked changes,
on top of the import/list/decide/export logic from the prior commit.
Rendering (both modes):
- Editor mode: trackChangesBasePlugin emits node decorations on tracked
<tr> rows; prosemirror.css styles inserted (green) / deleted
(red + strikethrough) rows.
- Layout-engine mode (presentation/playground view): TableRowAttrs gains
`trackedChange` (shared TrackedChangeMeta); the v1 layout-adapter
populates it, the contracts author-color stamper colors it, and the
DomPainter paints it on the row's cells via applyRowTrackedChangeToCell
— reusing inline tracked-change classes, author-color CSS variables,
and show-original/show-modified modes.
Right-rail bubble:
- Whole-table changes surface as a right-rail card ("Inserted table" /
"Deleted table"), reusing the inline TC bubble pipeline. Positioned by
anchoring on the table PM range via the tracked-change index, so it
works in layout-engine viewing mode. Accept/reject routes through
trackChanges.decide by the public structural id.
- Created during the initial import bootstrap (parity with inline/story
bubbles) so it shows on first render, not only after a later edit.
Coalescing (Word/Google-Docs model — one change per table):
- Decide cascade: deciding a whole-table change cascades the same decision
to inline tracked changes inside the table; accepting an inserted table
also accepts its cell text, rejecting removes the whole table.
Whole-object partial-range still fails closed.
- UI: inline tracked-change bubbles inside a tracked table are suppressed
so only the single structural bubble shows; real comments and inline
changes in non-tracked tables are unaffected.
Labs sdk-v1 unchanged at 363/147 with the structural cases green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… changes (SD-3360) Authoring: inserting a table in suggesting mode now produces a tracked whole-table insertion. A structural-insert branch in the tracked transaction (replaceStep.js `tryStructuralTableInsert`) inserts the table and stamps each row's `trackChange` (rowInsert) via `stampInsertedTableRows`, matching the imported model. The inline overlap-compiler can't represent an empty (text-free) table, so it fails closed and would otherwise land the table untracked. Guarded to the no-real-content case so replacing a non-empty selection never deletes content untracked. Subsumption: a whole tracked table is ONE review item. Cell text typed in a tracked-inserted row inherits the row's revision id, so the inline change and the structural change share an id. Fixes: - decide resolver prefers the structural change for a shared id, and the staying-table cascade dedups by OBJECT identity (not change.id) so accepting/rejecting the table clears the contained cell text in ONE action (no leftover marks, no second bubble). - comments-store suppresses the inline cell-text comment at creation (and prunes stale ones) so it never becomes a separate floating/active bubble; the structural "Inserted/Deleted table" bubble is exempt. The id-set match is gated on "no inline range" so an inline change outside the table that merely shares a row id is never wrongly suppressed. Reviewed by Codex; must-fix correctness items (untracked content deletion, id-set over-suppression, nested-table descend) addressed. super-editor and superdoc suites green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (SD-3360) Collapse three copies of the "inline change inside a tracked whole-table change is subsumed" logic into the single central guard in `handleTrackedChangeUpdate` — the chokepoint every comment-creation path (full resync, targeted resync, live comments-plugin handler) funnels through. Removes the duplicated suppress+prune blocks from `createCommentForTrackChanges` and `refreshTrackedChangeCommentsByIds` (~50 lines) with no behavior change. Also drops dead code in decision-engine (unused `Mapping` import and `replacements` param). super-editor track-changes (433) and superdoc (1294) suites green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deleting a whole table in suggesting mode is now a tracked deletion: the table stays VISIBLE (struck-through), every row gets `trackChange` rowDelete, cell text gets `trackDelete`, and one "Deleted table" bubble shows; Accept removes the table, Reject restores it. Empty tables are kept + tracked instead of being removed untracked; track-changes OFF deletes normally. A new structural-delete branch in `tryCompileStep` (replaceStep.js) intercepts an empty-slice deletion whose range brackets a whole table and routes it to `tryStructuralTableDelete`, which marks cell text via `markDeletion` and stamps rows via the new `stampDeletedTableRows` WITHOUT applying the removal — mirroring the insert authoring. Downstream (enumerate → decide cascade → paint → bubble → suppression) already handles rowDelete. Reviewed by Codex; must-fix edges addressed: a mixed selection (text + table) is guarded so it never shares one deletion id across inside/outside text (falls through), and the un-applied removal's positional effect is cancelled on the outer map so later steps in a multi-step transaction don't drift (no-op for single-step deleteTable). Track-changes + table suites green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove the SD-3360 task-number references from code comments and test descriptions added by the structural table tracked-changes work. Comments only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Fix range/cursor decide resolving the inline cell-text child instead of
the structural table change on a shared revision id (resolveLogicalChangeById)
- Drop hidden tracked rows from the layout in original/final view modes so an
inserted/deleted table reserves no blank space (was CSS-hidden only)
- Show the imported author on the structural bubble: pass importedAuthor as
{ name } to match the inline shape (was a raw string → "(Imported)")
- Rename the insert label "Inserted table" -> "Added table" to match the
existing "Added ..." insert vocabulary
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Behavior-preserving simplification of the structural whole-table code:
- Merge stampInsertedTableRows + stampDeletedTableRows into a single
stampTableRows({ type }) helper
- Extract collectWholeTablesInRange, the shared "whole tables fully bracketed
by [from,to)" walk, now used by stampTableRows, rangeContainsWholeTable, and
tryStructuralTableDelete
- Consolidate computeTrackedTableRangesForState + computeStructuralChangeIdsForState
into one cached computeTrackedTableSummaryForState returning { ranges, ids }
(single enumeration, single per-state memo)
~250 fewer lines of production code; no behavior change. Cross-reviewed by Codex.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t-structural-tracked-changes-for-tables-whole-table
The whole-table structural segment sets `structural: true`, but the TrackedSegment typedef did not declare it, so `tsc -b` failed with TS2353 during the build (unit tests don't typecheck, so it passed locally). Add the optional `structural?: boolean` property to the typedef. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The MCP `bun build` (and the Release MCP workflow) failed with "Could not resolve: @superdoc/word-layout" because its exports resolved to ./dist/index.js, which is emitted only by `tsc -b` (project references). The CI MCP job builds superdoc + SDK but never runs that step, so the dist is absent and bun cannot resolve it. This has kept main's MCP builds red since word-layout was introduced. Resolve word-layout from its TypeScript source instead — the same pattern the private, bundler-only `document-api` package uses. word-layout is `private: true` and consumed exclusively by bundlers (super-editor vite, MCP bun, layout-bridge), so there is no node consumer that needs prebuilt JS, and the hidden build-order dependency goes away. Verified with dist absent: MCP build + 37 MCP tests pass; check:types and build:superdoc remain green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The structural-row decoration work made trackChangesBasePlugin.js a public JSDoc surface, tripping the jsdoc-ratchet (new public file without // @ts-check). Add // @ts-check (the preferred resolution) and narrow the Replace-step access (slice/from/to live on ReplaceStep, not the base Step type) so the file typechecks clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…ddition Adding the structural `subtype` field to the track-changes output schema shifted the auto-generated example for trackChanges.get/list (subtype now occupies the example's optional-field budget, displacing pairedWithChangeId). The committed reference docs were regenerated accordingly, but the reference-docs-artifacts test still asserted the old `"pairedWithChangeId": null` example value. Keep every nullable type-label assertion (still valid) and move the "nullable primitive renders as null" example check to header-footers/get, which still surfaces a nullable `refId` in its generated example. Full document-api suite: 1481 pass; check:public:docapi green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chittolinag
left a comment
chittolinag
left a comment
There was a problem hiding this comment.
Great job here @andrii-harbour ! For this one I decided to focus on manual testing for sake of time. As a side note, please don't forget to update the labs specs around tables too.
Finding 1
- Create a new document and add a table
- Switch to suggesting mode
- Press
cmd + aand hit delete - Verify the tracked changed is add instead of delete
Finding 2
- Create a new document in Word
- Switch to suggesting mode
- Add a table
- Verify that in the right sidebar it shows the change dialog -> Formatted table
- Now, create a document the same way you did in Word, but in SuperDoc
- Export the document
- Open in Word
- Verify that the right sidebar doesn't show the change dialog.
See screenshot of expected dialog in Word:
Finding 3
- Create a new document in Word
- Switch to suggesting mode
- Create a list
- In one of its items, add a table
- Export the document & open in SuperDoc
- Verify that the table is not rendered
SuperDoc vs Word
Finding 4 (unrelated but affects testing)
This doesn't seem to be related to your PR but depending on how I try to select the entire table by keyboard or mouse, it doesn't work. Did you notice something similar?
Finding 5 (possibly out of scope)
- Delete a row in Word in suggesting mode
- Open in SuperDoc
- See the row is "red" as expected, but there's no tracked change in the right sidebar.
From the ticket description it seems like row/column insert/delete are out of scope, but I figured I'd flag just to confirm!
Final notes
From my POV the most important ones here are 1 and 3. For 4 I don't think it's related to your changes - but maybe a follow up ticket? And for 5 please confirm whether that's expected.
4 participants
No description provided.