fix: replacement text in bubble (#2769)

* fix: replacement text in bubble

* test(comments): harden replacement tracked-change test coverage (SD-2509)

Add assertions for changeId, trackedChangeDisplayType, and deletedText
null-guards on pure-insertion path. Add Playwright spec for the
update-after-replace flow that triggered SD-2509.

* refactor(comments): rename isDeletionInsertion to isReplacement (SD-2509)

The old name read as "deletion then insertion" but actually means "both
marks co-exist for the same change id, i.e. a replacement." Every call
site had a comment re-explaining this. The rename makes the intent
self-evident and retires those comments.

---------

Co-authored-by: Caio Pizzol <caio@harbourshare.com>

Author: artem-harbour

packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js

@@ -988,13 +988,13 @@ const normalizeFormatAttrsForCommentText = (attrs = {}, nodes) => {
   };
 };
 
-const getTrackedChangeText = ({ nodes, mark, trackedChangeType, isDeletionInsertion }) => {
+const getTrackedChangeText = ({ nodes, mark, trackedChangeType, isReplacement }) => {
   let trackedChangeText = '';
   let deletionText = '';
   let trackedChangeDisplayType = null;
 
   // Extract deletion text first
-  if (trackedChangeType === TrackDeleteMarkName || isDeletionInsertion) {
+  if (trackedChangeType === TrackDeleteMarkName || isReplacement) {
     deletionText = nodes.reduce((acc, node) => {
       const hasDeleteMark = node.marks.find((nodeMark) => nodeMark.type.name === TrackDeleteMarkName);
       if (!hasDeleteMark) return acc;
@@ -1004,7 +1004,7 @@ const getTrackedChangeText = ({ nodes, mark, trackedChangeType, isDeletionInsert
     }, '');
   }
 
-  if (trackedChangeType === TrackInsertMarkName || isDeletionInsertion) {
+  if (trackedChangeType === TrackInsertMarkName || isReplacement) {
     trackedChangeText = nodes.reduce((acc, node) => {
       const hasInsertMark = node.marks.find((nodeMark) => nodeMark.type.name === TrackInsertMarkName);
       if (!hasInsertMark) return acc;
@@ -1064,17 +1064,14 @@ const createOrUpdateTrackedChangeComment = ({
   const { author, authorEmail, authorImage, date, importedAuthor } = attrs;
   const id = attrs.id;
 
-  // Check metadata first - this should be set correctly by groupChanges() in createCommentForTrackChanges
-  // for both newly created and imported tracked changes
-  let isDeletionInsertion = !!(marks.insertedMark && marks.deletionMark);
+  let isReplacement = !!(marks.insertedMark && marks.deletionMark);
 
-  // Fallback: If metadata doesn't indicate replacement (e.g., edge cases during import),
-  // check the document state directly to detect replacements by finding both marks with same ID
-  // This ensures robustness even if groupChanges() misses a replacement or metadata isn't set
-  if (!isDeletionInsertion) {
+  // Fallback: check the document for both mark types under the same ID
+  // (covers edge cases where transaction meta only carries one mark)
+  if (!isReplacement) {
     const hasInsertMark = trackedChangesWithId.some(({ mark }) => mark.type.name === TrackInsertMarkName);
     const hasDeleteMark = trackedChangesWithId.some(({ mark }) => mark.type.name === TrackDeleteMarkName);
-    isDeletionInsertion = hasInsertMark && hasDeleteMark;
+    isReplacement = hasInsertMark && hasDeleteMark;
   }
 
   // Collect nodes from the tracked changes found
@@ -1097,10 +1094,8 @@ const createOrUpdateTrackedChangeComment = ({
     });
   });
 
-  // For replacements, we need both insertion nodes and deletion nodes
-  // When isDeletionInsertion is true, nodesWithMark should contain both types
   let nodesToUse;
-  if (isDeletionInsertion) {
+  if (isReplacement) {
     // For replacements, prefer nodes found in the document to avoid duplicating text
     // when step.slice/deletionNodes include overlapping content.
     const hasInsertNode = nodesWithMark.some((node) =>
@@ -1128,7 +1123,7 @@ const createOrUpdateTrackedChangeComment = ({
     nodes: nodesToUse,
     mark: trackedMark,
     trackedChangeType,
-    isDeletionInsertion,
+    isReplacement,
     deletionNodes,
   });
 
@@ -1141,10 +1136,10 @@ const createOrUpdateTrackedChangeComment = ({
     type: 'trackedChange',
     documentId,
     changeId: id,
-    trackedChangeType: isDeletionInsertion ? 'both' : trackedChangeType,
+    trackedChangeType: isReplacement ? 'both' : trackedChangeType,
     trackedChangeText,
     trackedChangeDisplayType,
-    deletedText: marks.deletionMark ? deletionText : null,
+    deletedText: isReplacement || marks.deletionMark ? deletionText : null,
     author,
     authorEmail,
     ...(authorImage && { authorImage }),

packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js

@@ -1022,7 +1022,7 @@ describe('internal helper functions', () => {
       nodes: insertionNodes,
       mark: insertMark,
       trackedChangeType: TrackInsertMarkName,
-      isDeletionInsertion: false,
+      isReplacement: false,
     });
     expect(insertionResult.trackedChangeText).toBe('Added');
     expect(insertionResult.deletionText).toBe('');
@@ -1031,15 +1031,15 @@ describe('internal helper functions', () => {
       nodes: deletionNodes,
       mark: deleteMark,
       trackedChangeType: TrackDeleteMarkName,
-      isDeletionInsertion: false,
+      isReplacement: false,
     });
     expect(deletionResult.deletionText).toBe('Removed');
 
     const formatResult = getTrackedChangeText({
       nodes: [schema.text('Format', [formatMark])],
       mark: formatMark,
       trackedChangeType: TrackFormatMarkName,
-      isDeletionInsertion: false,
+      isReplacement: false,
     });
     expect(formatResult.trackedChangeText).toBe('italic, removed bold');
     expect(formatResult.trackedChangeDisplayType).toBeNull();
@@ -1053,7 +1053,7 @@ describe('internal helper functions', () => {
       nodes: [schema.text('Format', [deltaFormatMark])],
       mark: deltaFormatMark,
       trackedChangeType: TrackFormatMarkName,
-      isDeletionInsertion: false,
+      isReplacement: false,
     });
     expect(deltaFormatResult.trackedChangeText).toContain('bold');
     expect(deltaFormatResult.trackedChangeText).not.toContain('undefined');
@@ -1070,7 +1070,7 @@ describe('internal helper functions', () => {
       nodes: [schema.text('website', [hyperlinkFormatMark, schema.marks.link.create({ href: 'https://example.com' })])],
       mark: hyperlinkFormatMark,
       trackedChangeType: TrackFormatMarkName,
-      isDeletionInsertion: false,
+      isReplacement: false,
     });
     expect(hyperlinkFormatResult).toMatchObject({
       trackedChangeText: 'https://example.com',
@@ -1081,7 +1081,7 @@ describe('internal helper functions', () => {
       nodes: [...insertionNodes, ...deletionNodes],
       mark: insertMark,
       trackedChangeType: TrackInsertMarkName,
-      isDeletionInsertion: true,
+      isReplacement: true,
     });
     expect(combinedResult.deletionText).toBe('Removed');
   });
@@ -1124,6 +1124,49 @@ describe('internal helper functions', () => {
     expect(payload?.deletedText).toBe('original');
   });
 
+  it('preserves deletedText on replacement update when transaction meta only carries insertion mark', () => {
+    const schema = createCommentSchema();
+    const insertMark = schema.marks[TrackInsertMarkName].create({
+      id: 'replace-update-1',
+      author: 'Author',
+      authorEmail: 'author@example.com',
+      date: 'today',
+    });
+    const deleteMark = schema.marks[TrackDeleteMarkName].create({
+      id: 'replace-update-1',
+      author: 'Author',
+      authorEmail: 'author@example.com',
+      date: 'today',
+    });
+
+    const docInsertNode = schema.text('replacement', [insertMark]);
+    const docDeleteNode = schema.text('original', [deleteMark]);
+    const doc = schema.node('doc', null, [schema.node('paragraph', null, [docInsertNode, docDeleteNode])]);
+    const state = EditorState.create({ schema, doc });
+
+    // Simulate an update transaction where meta has only insertedMark, but the
+    // document still has both insert+delete marks under the same tracked-change id.
+    const payload = createOrUpdateTrackedChangeComment({
+      event: 'update',
+      marks: { insertedMark: insertMark, deletionMark: null, formatMark: null },
+      deletionNodes: [],
+      nodes: [schema.text('replacement', [insertMark])],
+      newEditorState: state,
+      documentId: 'doc-1',
+      trackedChangesForId: [
+        { mark: insertMark, from: 1, to: doc.content.size },
+        { mark: deleteMark, from: 1, to: doc.content.size },
+      ],
+    });
+
+    expect(payload?.event).toBe(comments_module_events.UPDATE);
+    expect(payload?.trackedChangeType).toBe('both');
+    expect(payload?.trackedChangeText).toBe('replacement');
+    expect(payload?.deletedText).toBe('original');
+    expect(payload?.changeId).toBe('replace-update-1');
+    expect(payload?.trackedChangeDisplayType).toBeNull();
+  });
+
   it('createOrUpdateTrackedChangeComment builds add and update payloads', () => {
     const schema = createCommentSchema();
     const insertMark = schema.marks[TrackInsertMarkName].create({
@@ -1152,9 +1195,11 @@ describe('internal helper functions', () => {
       changeId: 'create-1',
       trackedChangeText: 'Body',
     });
+    expect(addPayload.deletedText).toBeNull();
 
     const updatePayload = createOrUpdateTrackedChangeComment({ event: 'update', ...baseArgs });
     expect(updatePayload.event).toBe(comments_module_events.UPDATE);
+    expect(updatePayload.deletedText).toBeNull();
 
     const emptyState = EditorState.create({
       schema,

tests/behavior/tests/comments/sd-2509-replacement-update-preserves-deleted-text.spec.ts

@@ -0,0 +1,54 @@
+import { test, expect } from '../../fixtures/superdoc.js';
+import { assertDocumentApiReady, listTrackChanges } from '../../helpers/document-api.js';
+
+test.use({ config: { toolbar: 'full', comments: 'on', trackChanges: true } });
+
+test('SD-2509 replacement bubble preserves deleted text after follow-up edits', async ({ superdoc }) => {
+  await assertDocumentApiReady(superdoc.page);
+
+  // Type baseline text in editing mode
+  await superdoc.type('original');
+  await superdoc.waitForStable();
+
+  // Switch to suggesting mode and replace
+  await superdoc.setDocumentMode('suggesting');
+  await superdoc.waitForStable();
+
+  await superdoc.tripleClickLine(0);
+  await superdoc.waitForStable();
+  await superdoc.type('replacement');
+  await superdoc.waitForStable();
+
+  // Wait for the tracked change to appear
+  await expect
+    .poll(async () => (await listTrackChanges(superdoc.page, { type: 'insert' })).total)
+    .toBeGreaterThanOrEqual(1);
+
+  // The bubble should show both the deleted and inserted text
+  const dialog = superdoc.page.locator('.comment-placeholder .comments-dialog', {
+    has: superdoc.page.locator('.tracked-change-text'),
+  });
+  await expect(dialog).toBeVisible({ timeout: 10_000 });
+
+  const deletedText = dialog.locator('.tracked-change-text.is-deleted');
+  const insertedText = dialog.locator('.tracked-change-text.is-inserted');
+
+  // Both sides of the replacement must be visible
+  await expect(deletedText).toBeVisible();
+  await expect(deletedText).toContainText('original');
+  await expect(insertedText).toContainText('replacement');
+
+  // Type more inside the insertion to trigger update transactions with insert-only meta.
+  // This is the SD-2509 bug scenario: subsequent keystrokes fire updates where only
+  // insertedMark is in the transaction meta, but both marks still exist in the document.
+  await superdoc.page.keyboard.press('End');
+  await superdoc.page.keyboard.type(' extra');
+  await superdoc.waitForStable();
+
+  // The deleted text must still be visible after the update
+  await expect(deletedText).toBeVisible();
+  await expect(deletedText).toContainText('original');
+  await expect(insertedText).toContainText('replacement extra');
+
+  await superdoc.snapshot('sd-2509-replacement-update-preserves-deleted-text');
+});