Merge pull request #3355 from superdoc-dev/caio/IT-1065-diffing-table-text

fix(diffing): gate paragraph identity on structural depth (SD-3174)

Author: caio-pizzol

packages/super-editor/src/editors/v1/extensions/diffing/algorithm/paragraph-diffing.test.js

@@ -159,6 +159,35 @@ describe('paragraphComparator', () => {
   it('returns false for paragraphs with different identity signals', () => {
     expect(paragraphComparator({ fullText: 'one' }, { fullText: 'two' })).toBe(false);
   });
+
+  it('rejects paraId match across different depths', () => {
+    // Regression: importers (e.g., Google Docs) renumber paraIds when structure
+    // changes, so a table-cell paragraph (deeper) can collide with a body
+    // paragraph (depth 0). Treat them as different paragraphs.
+    const oldCell = createParagraphInfo({
+      depth: 3,
+      fullText: 'Milestone',
+      node: createParagraphNode({ attrs: { paraId: '00000008' } }),
+    });
+    const newBody = createParagraphInfo({
+      depth: 0,
+      fullText: '',
+      node: createParagraphNode({ attrs: { paraId: '00000008' } }),
+    });
+    expect(paragraphComparator(oldCell, newBody)).toBe(false);
+  });
+
+  it('rejects content-signature match across different depths', () => {
+    const oldCell = createParagraphInfo({ depth: 3, fullText: '' });
+    const newBody = createParagraphInfo({ depth: 0, fullText: '' });
+    expect(paragraphComparator(oldCell, newBody)).toBe(false);
+  });
+
+  it('still matches by content signature at the same depth', () => {
+    const oldParagraph = createParagraphInfo({ depth: 0, fullText: 'shared content' });
+    const newParagraph = createParagraphInfo({ depth: 0, fullText: 'shared content' });
+    expect(paragraphComparator(oldParagraph, newParagraph)).toBe(true);
+  });
 });
 
 describe('paragraph diff builders', () => {
@@ -488,4 +517,13 @@ describe('canTreatAsModification', () => {
     const b = { node: { attrs: {} }, fullText: 'dolor sit' };
     expect(canTreatAsModification(a, b)).toBe(false);
   });
+
+  it('rejects similarity-based pairing across different depths', () => {
+    // Even with text similar enough to clear SIMILARITY_THRESHOLD, paragraphs
+    // at different structural depths should not be re-paired as a single
+    // modification - their replay anchor would land in the wrong context.
+    const a = createParagraphInfo({ depth: 3, fullText: 'lorem ipsum dolor' });
+    const b = createParagraphInfo({ depth: 0, fullText: 'lorem ipsum dolor!' });
+    expect(canTreatAsModification(a, b)).toBe(false);
+  });
 });

packages/super-editor/src/editors/v1/extensions/diffing/algorithm/paragraph-diffing.ts

@@ -145,8 +145,16 @@ export function shouldProcessEqualAsModification(
  *
  * The content signature covers both text and inline nodes (images, etc.),
  * so image-only paragraphs with different images are not falsely paired.
+ *
+ * Identity is gated on structural context (depth) first. Cross-depth pairing
+ * via paraId or content-signature alone produces false identity when DOCX
+ * importers renumber paraIds across structural changes, e.g. a table-cell
+ * paragraph and a body paragraph sharing a paraId or both being empty.
  */
 export function paragraphComparator(oldParagraph: ParagraphNodeInfo, newParagraph: ParagraphNodeInfo): boolean {
+  if (oldParagraph?.depth !== newParagraph?.depth) {
+    return false;
+  }
   const oldId = oldParagraph?.node?.attrs?.paraId;
   const newId = newParagraph?.node?.attrs?.paraId;
   if (oldId && newId && oldId === newId) {
@@ -223,6 +231,9 @@ export function buildModifiedParagraphDiff(
  * Decides whether a delete/insert pair should be reinterpreted as a modification to minimize noisy diff output.
  */
 export function canTreatAsModification(oldParagraph: ParagraphNodeInfo, newParagraph: ParagraphNodeInfo): boolean {
+  if (oldParagraph?.depth !== newParagraph?.depth) {
+    return false;
+  }
   if (paragraphComparator(oldParagraph, newParagraph)) {
     return true;
   }

packages/super-editor/src/editors/v1/extensions/diffing/replayDiffs.test.js

@@ -382,6 +382,11 @@ const getReplayFixturePairs = () => [
   ['diff_before7.docx', 'diff_after7.docx'],
   ['diff_before8.docx', 'diff_after8.docx'],
   ['diff_before9.docx', 'diff_after9.docx'],
+  // IT-1065: combined structural (table removal) + text change. Google Docs
+  // renumbers paraIds across the structural change, so cell paragraphs in
+  // before share paraIds with body paragraphs in after. Depth-gated identity
+  // prevents false matches.
+  ['diff_before_it1065.docx', 'diff_after_it1065.docx'],
 ];
 
 /**
@@ -453,6 +458,26 @@ describe('replayDifferences can()', () => {
   });
 });
 describe('replayDiffs tracked changes', runTrackedReplayDiffsSuite);
+describe('replayDiffs tracked structural replay', () => {
+  it('replays the IT-1065 table/text collision fixture with tracked changes enabled', async () => {
+    const testUser = { name: 'Test User', email: 'test@example.com' };
+    const beforeEditor = await getEditorFromFixture('diff_before_it1065.docx', testUser);
+    const afterEditor = await getEditorFromFixture('diff_after_it1065.docx');
+
+    try {
+      const diff = beforeEditor.commands.compareDocuments(afterEditor);
+      const success = beforeEditor.commands.replayDifferences(diff, { applyTrackedChanges: true });
+
+      expect(success).toBe(true);
+      expect(getTrackChanges(beforeEditor.state).length).toBeGreaterThan(0);
+      expect(beforeEditor.commands.acceptAllTrackedChanges()).toBe(true);
+      expect(beforeEditor.state.doc.textContent).toBe(afterEditor.state.doc.textContent);
+    } finally {
+      beforeEditor.destroy?.();
+      afterEditor.destroy?.();
+    }
+  });
+});
 describe('replayDiffs tracked-change ids', () => {
   it('keeps tracked mark ids populated for diff_before8 replay', async () => {
     await expectTrackedReplayMarksHaveIds('diff_before8.docx', 'diff_after8.docx');