feat(track-changes): structural tracked changes for whole-table insert/delete#3614
Draft
andrii-harbour wants to merge 13 commits into
Draft
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>
Contributor
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
Contributor
|
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.