SD-3358 - tracked deletion is captured char by char in suggesting mode

packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.js

@@ -274,6 +274,81 @@ const findAdjacentInsertedSegment = (ctx, pos) => {
   );
 };
 
+/**
+ * Find the current user's own unresolved tracked deletion adjacent to the
+ * delete range [from, to], so contiguous keystroke deletions coalesce into one
+ * logical change. A Backspace run leaves the caret at the left edge of the
+ * prior deletion, so the next range's `to` meets that deletion's `from`
+ * (right-adjacent); forward Delete extends the other way (left-adjacent).
+ *
+ * Adjacency is checked exactly first (contiguous within a single run), then
+ * across a gap gated by `isCoalescibleDeletionGap`. Multi-run paragraphs —
+ * e.g. Google Docs exports that split "Open comment " and "from Google Docs."
+ * into separate runs — separate the prior deletion from this range by run
+ * open/close tokens, and Google Docs additionally anchors comments with
+ * zero-width marker nodes at those seams; without the gap-tolerant pass,
+ * deleting the space at the seam would mint a new change. `isCoalescibleDeletionGap`
+ * tolerates run boundaries AND zero-width review/anchor markers but still
+ * requires no live text in the gap, so a live character between two deletions
+ * splits them.
+ *
+ * This is analogous to the same-user insertion refinement in `compileTextInsert`
+ * but intentionally MORE permissive: that path's `findSegmentAcrossEmptyStructuralGap`
+ * rejects any inline leaf, so the insertion side still splits at comment-anchor
+ * seams. TC-EDIT-018 covers "deleted or inserted", so the insertion side is a
+ * known conformance gap to close in a follow-up — not a mirror of this logic.
+ *
+ * Known limitation (bridge case): when a single live character sits between two
+ * of the user's own deletions, deleting it matches `exactLeft` first and reuses
+ * the LEFT deletion's id; the right deletion is never consulted. Because span
+ * merging joins only same-id spans, the result is two touching logical
+ * deletions where Word shows one. This is strictly better than the
+ * pre-coalescing behavior (which minted a third id) but short of TC-EDIT-018's
+ * "one logical change"; bridging both sides under a single id (reassigning the
+ * right deletion to the left id) is left as a future refinement.
+ *
+ * @param {*} ctx
+ * @param {number} from
+ * @param {number} to
+ */
+const findAdjacentDeletedSegment = (ctx, from, to) => {
+  // Only coalesce into a PLAIN standalone deletion. A replacement's deleted
+  // side (replacementGroupId set) or an overlap child (overlapParentId set) is
+  // a structured change: reusing its id for an unrelated plain deletion would
+  // write a deletion mark with empty/mismatched replacement metadata under that
+  // id, widening or mis-typing the change and corrupting its accept/reject.
+  const sameUserDeleted = (segment) =>
+    segment.side === SegmentSide.Deleted &&
+    !segment.attrs?.replacementGroupId &&
+    !segment.attrs?.overlapParentId &&
+    isSameUserForRefinement(ctx, segment);
+
+  const exactLeft = ctx.graph.segments.find((segment) => sameUserDeleted(segment) && segment.to === from);
+  if (exactLeft) return exactLeft;
+  const exactRight = ctx.graph.segments.find((segment) => sameUserDeleted(segment) && segment.from === to);
+  if (exactRight) return exactRight;
+
+  let nearest = null;
+  let nearestDistance = Infinity;
+  for (const segment of ctx.graph.segments) {
+    if (!sameUserDeleted(segment)) continue;
+    if (segment.from > to) {
+      const distance = segment.from - to;
+      if (distance < nearestDistance && isCoalescibleDeletionGap(ctx, to, segment.from)) {
+        nearest = segment;
+        nearestDistance = distance;
+      }
+    } else if (segment.to < from) {
+      const distance = from - segment.to;
+      if (distance < nearestDistance && isCoalescibleDeletionGap(ctx, segment.to, from)) {
+        nearest = segment;
+        nearestDistance = distance;
+      }
+    }
+  }
+  return nearest;
+};
+
 const isEmptyStructuralGap = (ctx, from, to) => {
   if (to <= from) return false;
   if (!sharesTextblock(ctx.tr.doc, from, to)) return false;
@@ -291,6 +366,59 @@ const isEmptyStructuralGap = (ctx, from, to) => {
   return !hasInlineLeaf;
 };
 
+/**
+ * Zero-width review/anchor marker node types: inline ATOM nodes that occupy a
+ * document position but render no visible content (comment range start/end,
+ * comment reference, bookmark end, permission range start/end). A contiguous
+ * visible-text deletion spans them, so they must not block deletion coalescing
+ * — e.g. Google Docs anchors comments with inline marker nodes between runs.
+ *
+ * Membership rule: list an inline leaf here only if it is genuinely zero-width.
+ * Deliberate non-members:
+ *   - `bookmarkStart` is a content node (`content: 'inline*'`), not a leaf, so
+ *     it never reaches the leaf check in `isCoalescibleDeletionGap`; any text it
+ *     wraps is caught by the live-text guard. Listing it would be inert.
+ *   - `fieldAnnotation` is an inline atom but renders real content (a field /
+ *     form widget), so a deletion spanning one MUST split — it stays out.
+ */
+const ZERO_WIDTH_ANCHOR_NODE_NAMES = new Set([
+  'commentRangeStart',
+  'commentRangeEnd',
+  'commentReference',
+  'bookmarkEnd',
+  'permStart',
+  'permEnd',
+]);
+
+/**
+ * Whether [from, to] separates two same-author tracked deletions that should
+ * still coalesce: same textblock, no other tracked change in range, no live
+ * text, and any inline-leaf nodes present are zero-width anchors (comment /
+ * bookmark markers) — run-wrapper boundaries carry no inline leaf at all.
+ * Strictly more permissive than `isEmptyStructuralGap`, which rejects any
+ * inline leaf and so would split a deletion at a Google-Docs comment seam.
+ *
+ * @param {*} ctx
+ * @param {number} from
+ * @param {number} to
+ */
+const isCoalescibleDeletionGap = (ctx, from, to) => {
+  if (to <= from) return false;
+  if (!sharesTextblock(ctx.tr.doc, from, to)) return false;
+  if (ctx.graph.segmentsInRange(from, to).length) return false;
+  if (ctx.tr.doc.textBetween(from, to, '', '')) return false;
+
+  let blocked = false;
+  ctx.tr.doc.nodesBetween(from, to, (node, pos) => {
+    if (pos < from || pos >= to) return;
+    if (node.isInline && node.isLeaf && !ZERO_WIDTH_ANCHOR_NODE_NAMES.has(node.type.name)) {
+      blocked = true;
+      return false;
+    }
+  });
+  return !blocked;
+};
+
 const sharesTextblock = (doc, from, to) => {
   const left = textblockStart(doc, from);
   const right = textblockStart(doc, to);
@@ -543,16 +671,48 @@ const compileTextDelete = (ctx, intent) => {
     return failure('INVALID_TARGET', 'text-delete requires a non-empty range.');
   }
 
+  // Coalesce contiguous same-user keystroke deletions. When this delete range
+  // is immediately adjacent to the current user's own unresolved tracked
+  // deletion, reuse that change's id so a run deleted character-by-character
+  // (e.g. holding Backspace) surfaces as ONE logical tracked deletion rather
+  // than one review object per keystroke — mirroring the same-user insertion
+  // refinement in compileTextInsert (TC-EDIT-018). Replacement-driven deletes
+  // keep their caller-provided pairing id; preserve-review-state edits never
+  // fold into an existing change.
+  //
+  // Date semantics: the new run is marked with this edit's date while the
+  // existing runs keep theirs, so one changeId can span runs with mixed dates
+  // (the same as same-user insertion refinement, and as coalescing into an
+  // imported older-session same-author deletion — both intentional). This does
+  // not create ambiguity: the read model takes the change date from the first
+  // segment (review-graph buildLogicalChange `primary = segments[0]`), and on
+  // export mergeConsecutiveTrackedChanges joins the per-run w:del/w:ins wrappers
+  // by w:id and keeps the first wrapper's attributes — so one logical deletion
+  // exports as a single w:del with the first run's w:date, and the panel shows
+  // that same date.
+  const coalesceTarget =
+    intent.replacementGroupHint || intent.preserveExistingReviewState
+      ? null
+      : findAdjacentDeletedSegment(ctx, intent.from, intent.to);
+  const sharedDeletionId = intent.replacementGroupHint || coalesceTarget?.changeId || null;
+
   const result = applyTrackedDelete(ctx, intent.from, intent.to, {
     replacementGroupId: '',
     replacementSideId: '',
-    sharedDeletionId: intent.replacementGroupHint || null,
+    sharedDeletionId,
     recordSharedDeletionId: Boolean(intent.replacementGroupHint),
     reassignExistingDeletions:
       intent.source !== 'native' && !intent.preserveExistingReviewState ? 'different-user' : false,
   });
   if (result.ok === false) return result;
 
+  // Folding into an existing deletion is an update to that change, not a new
+  // one. applyTrackedDelete suppresses the created-id record when a shared id
+  // is supplied without recordSharedDeletionId, so surface it as updated here.
+  if (coalesceTarget && !ctx.updatedChangeIds.includes(coalesceTarget.changeId)) {
+    ctx.updatedChangeIds.push(coalesceTarget.changeId);
+  }
+
   // Caret at original `from`: matches Word's behavior where the cursor sits
   // at the left edge of a tracked deletion.
   return {

packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.test.js

@@ -271,6 +271,43 @@ describe('overlap-compiler: same-user own-insertion refinement (SD-486-adjacent
   });
 });
 
+describe('overlap-compiler: keystroke deletion coalescing excludes structured changes (PR #3610)', () => {
+  it('a plain deletion adjacent to a same-user replacement does NOT fold into the replacement id', () => {
+    // "ab": replace "b" -> "X" (paired), then Backspace-delete the adjacent
+    // live "a". The deletion must be its own logical change, not merged into
+    // the replacement's id (which would corrupt the replacement's accept/reject).
+    const { state } = stateFromTrackedSpans({ schema, spans: [{ text: 'ab' }] });
+
+    const afterReplace = runCompile({
+      state,
+      intent: makeTextReplaceIntent({
+        from: 2,
+        to: 3,
+        content: sliceFromText(schema, 'X'),
+        replacements: 'paired',
+        user: ALICE,
+        date: FIXED_DATE,
+        source: 'native',
+      }),
+    });
+    expect(afterReplace.ok).toBe(true);
+
+    const afterDelete = runCompile({
+      state: state.apply(afterReplace.tr),
+      intent: makeTextDeleteIntent({ from: 1, to: 2, user: ALICE, date: FIXED_DATE, source: 'native' }),
+    });
+    expect(afterDelete.ok).toBe(true);
+
+    const graph = buildReviewGraph({ state: { doc: afterDelete.tr.doc } });
+    const changes = Array.from(graph.changes.values());
+    // Two distinct logical changes: the replacement and the standalone deletion.
+    // The pre-fix coalescing folded the deletion into the replacement id (size 1).
+    expect(graph.changes.size).toBe(2);
+    expect(changes.some((c) => c.type === CanonicalChangeType.Replacement)).toBe(true);
+    expect(changes.some((c) => c.type === CanonicalChangeType.Deletion)).toBe(true);
+  });
+});
+
 describe('overlap-compiler: different-user child insertion inside other-user insertion', () => {
   it('mints a child id with overlapParentId set to the parent insertion id', () => {
     const parentId = 'ins-bob';

tests/behavior/tests/comments/sd-3358-keystroke-delete-coalesce.spec.ts

@@ -0,0 +1,97 @@
+import fs from 'node:fs';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { test, expect, type SuperDocFixture } from '../../fixtures/superdoc.js';
+
+// SD-3358 / TC-EDIT-018: a contiguous same-author deletion authored
+// keystroke-by-keystroke in suggesting mode must surface as ONE logical tracked
+// deletion, not one per character — and that must hold across run seams,
+// including Google-Docs comment-anchor markers that sit between runs.
+
+test.use({ config: { toolbar: 'full', comments: 'off', trackChanges: true } });
+
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
+const GD_DOC_PATH = path.resolve(__dirname, '../../test-data/comments-tcs/gd-open-comment.docx');
+
+const REVIEWER = { name: 'Guest Reviewer', email: 'track@example.com' };
+
+/** Group tracked-deletion text by logical change id. One id == one logical deletion. */
+const snapshotTrackDeletesById = (superdoc: SuperDocFixture): Promise<Record<string, string>> =>
+  superdoc.page.evaluate(() => {
+    const editor = (window as any).editor;
+    const deleteById: Record<string, string> = {};
+    editor.state.doc.descendants((node: any) => {
+      if (!node.isText || !node.text) return;
+      for (const mark of node.marks ?? []) {
+        if (mark.type?.name !== 'trackDelete') continue;
+        const id = String(mark.attrs?.id ?? '');
+        if (!id) continue;
+        deleteById[id] = (deleteById[id] ?? '') + node.text;
+      }
+    });
+    return deleteById;
+  });
+
+const setReviewer = (superdoc: SuperDocFixture): Promise<void> =>
+  superdoc.page.evaluate((user) => {
+    (window as any).editor.setOptions({ user });
+  }, REVIEWER);
+
+test('contiguous Backspace deletion in suggesting mode is ONE tracked deletion (single run)', async ({ superdoc }) => {
+  await setReviewer(superdoc);
+  // Author normal content in editing mode, then switch to suggesting so the
+  // deletion is tracked (deleting an own insertion would collapse instead).
+  await superdoc.type('Alpha Beta');
+  await superdoc.waitForStable();
+  await superdoc.setDocumentMode('suggesting');
+  await superdoc.waitForStable();
+
+  const pos = await superdoc.findTextPos('Beta');
+  await superdoc.setTextSelection(pos + 'Beta'.length);
+  await superdoc.waitForStable();
+
+  for (let i = 0; i < 'Beta'.length; i += 1) {
+    await superdoc.press('Backspace');
+    await superdoc.waitForStable();
+  }
+
+  const deleteById = await snapshotTrackDeletesById(superdoc);
+  const ids = Object.keys(deleteById);
+  expect(ids).toHaveLength(1);
+  expect(deleteById[ids[0]]).toBe('Beta');
+});
+
+test('Backspace deletion across a run seam (Google-Docs comment anchors) is ONE tracked deletion', async ({
+  superdoc,
+}) => {
+  test.skip(!fs.existsSync(GD_DOC_PATH), 'Corpus document not available — run pnpm corpus:pull');
+
+  // Imported Google-Docs paragraph "Open comment from Google Docs." is stored
+  // as two runs ("Open comment " + "from Google Docs.") with a comment anchored
+  // across the seam. Deleting back across that seam used to mint a second
+  // tracked deletion (SD-3358); it must stay one.
+  await superdoc.loadDocument(GD_DOC_PATH);
+  await superdoc.waitForStable();
+  await setReviewer(superdoc);
+  await superdoc.setDocumentMode('suggesting');
+  await superdoc.waitForStable();
+
+  const before = await snapshotTrackDeletesById(superdoc);
+  expect(Object.keys(before)).toHaveLength(0);
+
+  // Caret at the end of the paragraph, then Backspace back across the seam:
+  // 25 chars deletes "comment from Google Docs." and leaves "Open " live.
+  const docsPos = await superdoc.findTextPos('Docs.');
+  await superdoc.setTextSelection(docsPos + 'Docs.'.length);
+  await superdoc.waitForStable();
+
+  for (let i = 0; i < 'comment from Google Docs.'.length; i += 1) {
+    await superdoc.press('Backspace');
+    await superdoc.waitForStable();
+  }
+
+  const deleteById = await snapshotTrackDeletesById(superdoc);
+  const ids = Object.keys(deleteById);
+  expect(ids).toHaveLength(1);
+  expect(deleteById[ids[0]]).toBe('comment from Google Docs.');
+});