fix(document-api): subsume inline cell changes inside a whole-table tracked change

packages/super-editor/src/editors/v1/document-api-adapters/helpers/tracked-change-resolver.test.ts

@@ -6,6 +6,7 @@ import {
   TrackInsertMarkName,
 } from '../../extensions/track-changes/constants.js';
 import { getTrackChanges } from '../../extensions/track-changes/trackChangesHelpers/getTrackChanges.js';
+import { enumerateStructuralRowChanges } from '../../extensions/track-changes/trackChangesHelpers/structuralRowChanges.js';
 import {
   buildTrackedChangeCanonicalIdMap,
   groupTrackedChanges,
@@ -18,6 +19,10 @@ vi.mock('../../extensions/track-changes/trackChangesHelpers/getTrackChanges.js',
   getTrackChanges: vi.fn(),
 }));
 
+vi.mock('../../extensions/track-changes/trackChangesHelpers/structuralRowChanges.js', () => ({
+  enumerateStructuralRowChanges: vi.fn(() => []),
+}));
+
 function makeEditor(options: Record<string, unknown> = { trackedChanges: {} }): Editor {
   return {
     options,
@@ -305,6 +310,103 @@ describe('groupTrackedChanges', () => {
     const grouped = groupTrackedChanges(makeEditor());
     expect(grouped[0]?.from).toBeLessThan(grouped[1]?.from ?? 0);
   });
+
+  // A whole inserted/deleted table is ONE logical change. Inline cell marks
+  // inside its range (native authoring stamps these, and imported-with-text
+  // tables carry them) must be subsumed by the structural change, not returned
+  // as separate review items.
+  const wholeTableInsert = (overrides: Record<string, unknown> = {}) =>
+    ({
+      id: 'logical-1',
+      side: 'insertion',
+      subtype: 'table-insert',
+      tableFrom: 10,
+      tableTo: 40,
+      tablePos: 10,
+      wholeTable: true,
+      decidable: true,
+      rows: [],
+      author: 'Alice',
+      sourceId: '7',
+      ...overrides,
+    }) as never;
+
+  it('subsumes inline cell marks inside a decidable whole-table change', () => {
+    const cell = makeTrackMark(TrackInsertMarkName, 'inline-cell', { author: 'Alice' });
+    vi.mocked(getTrackChanges).mockReturnValue([
+      { ...cell, node: { text: 'Hi', marks: [cell.mark] }, from: 20, to: 25 },
+    ] as never);
+    vi.mocked(enumerateStructuralRowChanges).mockReturnValueOnce([wholeTableInsert()]);
+
+    const grouped = groupTrackedChanges(makeEditor());
+
+    // Only the structural change remains; the inline cell mark is owned by it.
+    expect(grouped).toHaveLength(1);
+    expect(grouped[0]?.id).toBe('word:structural:7');
+    expect(grouped.find((change) => change.rawId === 'inline-cell')).toBeUndefined();
+  });
+
+  it('does NOT subsume an inline change outside the whole-table range', () => {
+    const outside = makeTrackMark(TrackInsertMarkName, 'inline-outside', { author: 'Alice' });
+    vi.mocked(getTrackChanges).mockReturnValue([
+      { ...outside, node: { text: 'Out', marks: [outside.mark] }, from: 50, to: 55 },
+    ] as never);
+    vi.mocked(enumerateStructuralRowChanges).mockReturnValueOnce([wholeTableInsert()]);
+
+    const grouped = groupTrackedChanges(makeEditor());
+
+    // The structural change AND the unrelated inline change both surface.
+    expect(grouped.find((change) => change.id === 'word:structural:7')).toBeDefined();
+    expect(grouped.find((change) => change.rawId === 'inline-outside')).toBeDefined();
+  });
+
+  it('does NOT subsume inline marks for a NON-decidable (partial) structural shape', () => {
+    const cell = makeTrackMark(TrackInsertMarkName, 'inline-partial', { author: 'Alice' });
+    vi.mocked(getTrackChanges).mockReturnValue([
+      { ...cell, node: { text: 'Hi', marks: [cell.mark] }, from: 20, to: 25 },
+    ] as never);
+    // Partial / undecidable structural shape is not one logical change.
+    vi.mocked(enumerateStructuralRowChanges).mockReturnValueOnce([
+      wholeTableInsert({ wholeTable: false, decidable: false }),
+    ]);
+
+    const grouped = groupTrackedChanges(makeEditor());
+    expect(grouped.find((change) => change.rawId === 'inline-partial')).toBeDefined();
+  });
+
+  it('subsumes a single revision id whose disjoint segments each sit in a different whole table', () => {
+    // One imported id reused across two tables: marks at [20,25] (table A 10-40)
+    // and [70,75] (table B 60-90). The collapsed envelope [20,75] spans the gap
+    // between the tables, so an envelope-only check would wrongly keep it — the
+    // per-segment rule correctly subsumes it (every segment is table-owned).
+    const shared = makeTrackMark(TrackInsertMarkName, 'shared-across-tables', { author: 'Alice' });
+    vi.mocked(getTrackChanges).mockReturnValue([
+      { ...shared, node: { text: 'A', marks: [shared.mark] }, from: 20, to: 25 },
+      { ...shared, node: { text: 'B', marks: [shared.mark] }, from: 70, to: 75 },
+    ] as never);
+    vi.mocked(enumerateStructuralRowChanges).mockReturnValueOnce([
+      wholeTableInsert({ id: 'logical-a', sourceId: '7', tableFrom: 10, tableTo: 40, tablePos: 10 }),
+      wholeTableInsert({ id: 'logical-b', sourceId: '8', tableFrom: 60, tableTo: 90, tablePos: 60 }),
+    ]);
+
+    const grouped = groupTrackedChanges(makeEditor());
+    expect(grouped.find((change) => change.rawId === 'shared-across-tables')).toBeUndefined();
+    // Both structural tables survive.
+    expect(grouped.filter((change) => change.id?.startsWith('word:structural:'))).toHaveLength(2);
+  });
+
+  it('does NOT subsume an inline change whose segment straddles a table edge', () => {
+    // Segment [35,45] crosses the table's right edge (table 10-40): it is not
+    // wholly inside any whole-table range, so the change is kept.
+    const straddle = makeTrackMark(TrackInsertMarkName, 'edge-straddle', { author: 'Alice' });
+    vi.mocked(getTrackChanges).mockReturnValue([
+      { ...straddle, node: { text: 'edge', marks: [straddle.mark] }, from: 35, to: 45 },
+    ] as never);
+    vi.mocked(enumerateStructuralRowChanges).mockReturnValueOnce([wholeTableInsert()]);
+
+    const grouped = groupTrackedChanges(makeEditor());
+    expect(grouped.find((change) => change.rawId === 'edge-straddle')).toBeDefined();
+  });
 });
 
 describe('resolveTrackedChange', () => {

packages/super-editor/src/editors/v1/document-api-adapters/helpers/tracked-change-resolver.ts

@@ -315,6 +315,11 @@ export function groupTrackedChanges(editor: Editor): GroupedTrackedChange[] {
 
   const marks = getRawTrackedMarks(editor);
   const byRawId = new Map<string, GroupedTrackedChangeDraft>();
+  // Every underlying mark range per grouped id (not just the min/max envelope).
+  // A single revision id can have disjoint segments (Word reuses ids across the
+  // document), so whole-table subsumption below must test each segment, not the
+  // collapsed `from`/`to`.
+  const segmentsByRawId = new Map<string, Array<{ from: number; to: number }>>();
 
   for (const item of marks) {
     const attrs = item.mark?.attrs ?? {};
@@ -333,6 +338,10 @@ export function groupTrackedChanges(editor: Editor): GroupedTrackedChange[] {
     const excerptText = contributesToExcerpt ? getTrackedMarkText(editor, item) : '';
     const range: [number, number] = [item.from, item.to];
 
+    const priorSegments = segmentsByRawId.get(groupKey);
+    if (priorSegments) priorSegments.push({ from: item.from, to: item.to });
+    else segmentsByRawId.set(groupKey, [{ from: item.from, to: item.to }]);
+
     if (!existing) {
       byRawId.set(groupKey, {
         rawId: groupKey,
@@ -397,7 +406,38 @@ export function groupTrackedChanges(editor: Editor): GroupedTrackedChange[] {
   // their own grouped changes. Their `id` is the shared Word revision id; the
   // accept/reject command routes by id through the review graph which owns the
   // node-level mutation plan.
-  for (const structural of enumerateStructuralRowChanges(editor.state)) {
+  const structuralChanges = enumerateStructuralRowChanges(editor.state);
+
+  // Inline content inside a decidable whole-table structural change is OWNED by
+  // that change — a whole inserted/deleted table is ONE logical change, not the
+  // structural change plus a separate review item per tracked cell run. Native
+  // authoring (markInsertion/markDeletion on cell text) and imported-with-text
+  // tables both leave such inline marks, so drop the inline grouped entries
+  // whose content lives inside a whole-table range before the structural change
+  // is appended. Only decidable whole-table ranges suppress; a partial/
+  // undecidable structural shape is not one logical change, so its inline marks
+  // stay as their own items. Mirrors the comments-store range suppression
+  // (`isInlineRangeInsideTrackedTable`): an inline change is owned only when
+  // EVERY one of its segments sits inside some whole-table range. Testing each
+  // segment (not the collapsed `from`/`to` envelope) matters when one revision
+  // id has disjoint marks across two tracked tables — the envelope would span
+  // the gap between them, yet every segment is table-owned. A segment that
+  // straddles a table edge is (correctly) not owned, so the change stays.
+  const wholeTableRanges = structuralChanges
+    .filter((structural) => structural.decidable && structural.wholeTable)
+    .map((structural) => ({ from: structural.tableFrom, to: structural.tableTo }));
+  if (wholeTableRanges.length > 0) {
+    const segmentInsideSomeTable = (segment: { from: number; to: number }) =>
+      wholeTableRanges.some((range) => segment.from >= range.from && segment.to <= range.to);
+    for (let i = grouped.length - 1; i >= 0; i -= 1) {
+      const change = grouped[i];
+      const segments = segmentsByRawId.get(change.rawId) ?? [{ from: change.from, to: change.to }];
+      const ownedByTable = segments.every(segmentInsideSomeTable);
+      if (ownedByTable) grouped.splice(i, 1);
+    }
+  }
+
+  for (const structural of structuralChanges) {
     const excerpt = normalizeExcerpt(editor.state.doc.textBetween(structural.tableFrom, structural.tableTo, ' ', ''));
     // Public id must be stable across import → export → reopen. The logical
     // `structural.id` is a fresh UUID minted on each import, so derive the