SD-3352 - fix: refine deletion behavior for anonymous users in overlap-compiler

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

@@ -219,9 +219,11 @@ const classifySegment = (ctx, segment) => {
  * stored authorEmail matches the current user's normalized email — including
  * when both sides have no email at all (default unidentified user typing).
  *
- * Permission ownership and overlap parent decisions still require the
- * high-confidence `classifySegment` path; this helper is only for "is this
- * the same logical author for the purpose of coalescing contiguous edits".
+ * Permission ownership still requires the high-confidence `classifySegment`
+ * path; this helper answers "is this the same logical author for the purpose
+ * of refining their own pending edit" — coalescing contiguous typing,
+ * shrinking an own insertion on delete (SD-3352), and deciding that an own
+ * insertion is not a different-user overlap parent for a replacement.
  *
  * @param {*} ctx
  * @param {*} segment
@@ -791,8 +793,14 @@ const applyTrackedDelete = (
         change: getChangeAuthorIdentity(segmentAtPos?.attrs ?? insertMark.attrs),
       });
       const ownership = isSameUserHighConfidence(classification) ? 'same-user' : 'different-user';
+      // Deleting inside your own pending insertion must shrink that insertion,
+      // not stack a deletion on top of your own suggestion (SD-3352). Use the
+      // same permissive same-author gate as typing, so this also holds for
+      // users without an id/email (anonymous sessions).
+      const sameUserForRefinement =
+        ownership === 'same-user' || isSameUserForRefinement(ctx, segmentAtPos ?? { attrs: insertMark.attrs });
       const shouldCollapseOwnInsertion =
-        !ctx.intent.preserveExistingReviewState && (ownership === 'same-user' || isImportedOwnInsertion(insertMark));
+        !ctx.intent.preserveExistingReviewState && (sameUserForRefinement || isImportedOwnInsertion(insertMark));
       if (shouldCollapseOwnInsertion) {
         // Own insertion → collapse (remove proposed content).
         ops.push({ kind: 'collapse', from: segFrom, to: segTo, changeId: insertMark.attrs.id });
@@ -1231,8 +1239,14 @@ const clampToDocSize = (size, pos) => Math.max(0, Math.min(size, pos));
 
 const getSingleFullyCoveringOwnInsertedSegment = (ctx, segments, from, to) => {
   if (!segments.length) return null;
+  // Same permissive same-author gate as the delete half in applyTrackedDelete
+  // (SD-3352): replacing text wholly inside your own pending insertion must
+  // refine that insertion in place, including for anonymous sessions where the
+  // high-confidence classification cannot confirm identity. Otherwise the
+  // delete half collapses while the replacement text nests as a child
+  // insertion under the user's own suggestion.
   const inserted = segments.filter(
-    (s) => s.side === SegmentSide.Inserted && classifySegment(ctx, s) === 'same-user' && s.from <= from && s.to >= to,
+    (s) => s.side === SegmentSide.Inserted && isSameUserForRefinement(ctx, s) && s.from <= from && s.to >= to,
   );
   if (inserted.length !== segments.length) return null;
   const [first] = inserted;
@@ -1243,6 +1257,11 @@ const getSingleFullyCoveringOwnInsertedSegment = (ctx, segments, from, to) => {
 const getReplacementParentId = (ctx, segments) => {
   for (const segment of segments) {
     if (classifySegment(ctx, segment) !== 'different-user') continue;
+    // An inserted segment that matches the permissive same-author refinement
+    // gate (SD-3352, anonymous sessions) is the user's own pending content:
+    // the delete half collapses it in applyTrackedDelete, so it must not be
+    // classified as a different-user overlap parent for the inserted side.
+    if (segment.side === SegmentSide.Inserted && isSameUserForRefinement(ctx, segment)) continue;
     if (segment.attrs?.overlapParentId) return segment.attrs.overlapParentId;
     return segment.changeId || '';
   }

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

@@ -269,6 +269,101 @@ describe('overlap-compiler: same-user own-insertion refinement (SD-486-adjacent
     expect(change.type).toBe(CanonicalChangeType.Insertion);
     expect(result.updatedChangeIds).toContain(id);
   });
+
+  it('refines own anonymous-default-user insertion when replacing inside it (SD-3352)', () => {
+    // Replace-path counterpart of the SD-3352 delete tests: an anonymous
+    // session selecting text inside its own pending insertion and typing must
+    // refine that insertion in place — one logical change with the same id —
+    // not collapse the delete half while nesting the replacement text as a
+    // different-user child insertion under the user's own suggestion.
+    const parentId = 'ins-default-user';
+    const anonDefaultUser = { name: 'Default SuperDoc user', email: '' };
+    const { state } = stateFromTrackedSpans({
+      schema,
+      spans: [
+        {
+          text: 'ABCHELLOXYZ',
+          marks: [
+            insertMark({ id: parentId, author: anonDefaultUser.name, authorEmail: '', sourceId: '', date: FIXED_DATE }),
+          ],
+        },
+      ],
+    });
+    // Replace "HELLO" at [4, 9) — wholly inside the pending insertion.
+    const intent = makeTextReplaceIntent({
+      from: 4,
+      to: 9,
+      content: sliceFromText(schema, 'Q'),
+      replacements: 'paired',
+      user: anonDefaultUser,
+      date: FIXED_DATE,
+      source: 'native',
+    });
+    const result = runCompile({ state, intent });
+    expect(result.ok).toBe(true);
+    expect(textOf(result.tr)).toBe('ABCQXYZ');
+
+    const graph = buildReviewGraph({ state: { doc: result.tr.doc } });
+    expect(graph.changes.size).toBe(1);
+    const parent = graph.changes.get(parentId);
+    expect(parent).toBeDefined();
+    expect(parent.type).toBe(CanonicalChangeType.Insertion);
+    expect(parent.insertedSegments.map((segment) => segment.text).join('')).toBe('ABCQXYZ');
+    expect(parent.insertedSegments.every((segment) => !segment.attrs.overlapParentId)).toBe(true);
+    expect(result.updatedChangeIds).toContain(parentId);
+  });
+
+  it('replacement spanning own anonymous insertion and live text pairs like an identified user (SD-3352)', () => {
+    // Partial overlap goes through getReplacementParentId rather than the
+    // fully-covering refinement path. The user's own (by the permissive
+    // anonymous gate) insertion must not be classified as a different-user
+    // overlap parent: the result is an ordinary paired replacement — the
+    // covered insertion slice collapses, the live slice gets the paired
+    // deletion — exactly what an identified user gets.
+    const parentId = 'ins-default-user';
+    const anonDefaultUser = { name: 'Default SuperDoc user', email: '' };
+    const { state } = stateFromTrackedSpans({
+      schema,
+      spans: [
+        {
+          text: 'NEW',
+          marks: [
+            insertMark({ id: parentId, author: anonDefaultUser.name, authorEmail: '', sourceId: '', date: FIXED_DATE }),
+          ],
+        },
+        { text: 'live' },
+      ],
+    });
+    // Replace "EWli" at [2, 6): spans the insertion tail and live text head.
+    const intent = makeTextReplaceIntent({
+      from: 2,
+      to: 6,
+      content: sliceFromText(schema, 'Q'),
+      replacements: 'paired',
+      user: anonDefaultUser,
+      date: FIXED_DATE,
+      source: 'native',
+    });
+    const result = runCompile({ state, intent });
+    expect(result.ok).toBe(true);
+
+    const graph = buildReviewGraph({ state: { doc: result.tr.doc } });
+    // Two logical changes: the refined parent insertion ("N") and one paired
+    // replacement ("li" -> "Q"). No change is nested under the parent.
+    expect(graph.changes.size).toBe(2);
+    const parent = graph.changes.get(parentId);
+    expect(parent).toBeDefined();
+    expect(parent.type).toBe(CanonicalChangeType.Insertion);
+    expect(parent.insertedSegments.map((segment) => segment.text).join('')).toBe('N');
+
+    const replacement = Array.from(graph.changes.values()).find((change) => change.id !== parentId);
+    expect(replacement).toBeDefined();
+    expect(replacement.type).toBe(CanonicalChangeType.Replacement);
+    expect(replacement.deletedSegments.map((segment) => segment.text).join('')).toBe('li');
+    expect(replacement.insertedSegments.map((segment) => segment.text).join('')).toBe('Q');
+    expect(replacement.insertedSegments.every((segment) => !segment.attrs.overlapParentId)).toBe(true);
+    expect(replacement.deletedSegments.every((segment) => !segment.attrs.overlapParentId)).toBe(true);
+  });
 });
 
 describe('overlap-compiler: keystroke deletion coalescing excludes structured changes (PR #3610)', () => {
@@ -642,7 +737,54 @@ describe('overlap-compiler: text-delete', () => {
     expect(result.removedChangeIds).toEqual([parentId]);
   });
 
-  it('creates a child deletion inside a live anonymous no-email insertion', () => {
+  it('refines own anonymous-default-user insertion when deleting inside it (SD-3352)', () => {
+    // Integrators that pass no `user` run as the anonymous default identity
+    // (display name only, no id/email). Deleting inside your own pending
+    // insertion in the same session must refine the insertion in place — not
+    // mint an overlapping different-user child deletion. Word overlap case
+    // 11_same_insert_delete_inside_control is the oracle: one refined w:ins.
+    const parentId = 'ins-default-user';
+    const anonDefaultUser = { name: 'Default SuperDoc user', email: '' };
+    const { state } = stateFromTrackedSpans({
+      schema,
+      spans: [
+        {
+          text: 'ABCHELLOXYZ',
+          marks: [
+            insertMark({ id: parentId, author: anonDefaultUser.name, authorEmail: '', sourceId: '', date: FIXED_DATE }),
+          ],
+        },
+      ],
+    });
+    // Delete "HELLO" at [4, 9) — wholly inside the pending insertion.
+    const intent = makeTextDeleteIntent({
+      from: 4,
+      to: 9,
+      user: anonDefaultUser,
+      date: FIXED_DATE,
+      source: 'native',
+    });
+    const result = runCompile({ state, intent });
+    expect(result.ok).toBe(true);
+    expect(textOf(result.tr)).toBe('ABCXYZ');
+
+    const graph = buildReviewGraph({ state: { doc: result.tr.doc } });
+    expect(graph.changes.size).toBe(1);
+    const parent = graph.changes.get(parentId);
+    expect(parent).toBeDefined();
+    expect(parent.type).toBe(CanonicalChangeType.Insertion);
+    expect(parent.insertedSegments.map((segment) => segment.text).join('')).toBe('ABCXYZ');
+    expect(result.updatedChangeIds).toContain(parentId);
+    expect(result.deletionMarks).toHaveLength(0);
+  });
+
+  it('collapses inside a live unattributed insertion when the current user is equally anonymous (SD-3352 gate unification)', () => {
+    // Same permissive rule as typing (matchesSameUserRefinement): a truly
+    // unattributed live insertion deleted by an equally anonymous session
+    // refines in place. Before SD-3352 this minted a different-user child
+    // deletion while typing into the very same insertion coalesced — the
+    // delete path simply used the stricter gate. Named no-email authors stay
+    // protected (see the named no-email test above).
     const parentId = 'ins-live-anonymous';
     const { state } = stateFromTrackedSpans({
       schema,
@@ -662,20 +804,15 @@ describe('overlap-compiler: text-delete', () => {
     });
     const result = runCompile({ state, intent });
     expect(result.ok).toBe(true);
-    expect(textOf(result.tr)).toBe('live-review-comment');
+    expect(textOf(result.tr)).toBe('live--comment');
 
     const graph = buildReviewGraph({ state: { doc: result.tr.doc } });
-    expect(graph.changes.size).toBe(2);
+    expect(graph.changes.size).toBe(1);
     const parent = graph.changes.get(parentId);
     expect(parent).toBeDefined();
     expect(parent.type).toBe(CanonicalChangeType.Insertion);
-    expect(parent.insertedSegments.map((segment) => segment.text).join('')).toBe('live-review-comment');
-
-    const child = Array.from(graph.changes.values()).find((change) => change.id !== parentId);
-    expect(child).toBeDefined();
-    expect(child.type).toBe(CanonicalChangeType.Deletion);
-    expect(child.deletedSegments[0].text).toBe('review');
-    expect(child.deletedSegments[0].attrs.overlapParentId).toBe(parentId);
+    expect(parent.insertedSegments.map((segment) => segment.text).join('')).toBe('live--comment');
+    expect(result.updatedChangeIds).toContain(parentId);
   });
 
   it('no-ops when deleting inside own deletion', () => {

tests/behavior/tests/comments/sd-3352-same-author-delete-inside-insertion.spec.ts

@@ -0,0 +1,149 @@
+import { test, expect, type SuperDocFixture } from '../../fixtures/superdoc.js';
+
+// SD-3352 / TC-SAME-001: deleting text wholly inside your OWN pending tracked
+// insertion must refine the insertion in place — one shrunken suggestion, the
+// way Word handles it (overlap case 11_same_insert_delete_inside_control) —
+// not stack an overlapping deletion on top of your own insertion.
+//
+// The regression trigger was an anonymous session (integrator passes no
+// `user`): typing recognized the anonymous user as itself, but the delete
+// path classified the same user as a different reviewer and minted a nested
+// tracked deletion over their own suggestion.
+
+test.use({ config: { toolbar: 'full', comments: 'off', trackChanges: true } });
+
+// Mirror of SuperDoc's DEFAULT_USER when the integrator passes no `user`.
+const ANONYMOUS_DEFAULT = { id: null, name: 'Default SuperDoc user', email: null };
+const REVIEWER = { name: 'Guest Reviewer', email: 'track@example.com' };
+
+const setUser = (superdoc: SuperDocFixture, user: Record<string, unknown>): Promise<void> =>
+  superdoc.page.evaluate((u) => {
+    (window as any).editor.setOptions({ user: u });
+  }, user);
+
+/**
+ * Snapshot tracked-change state: text grouped by logical change id per mark
+ * kind, plus the public document-api read model.
+ */
+const snapshotTrackedState = (
+  superdoc: SuperDocFixture,
+): Promise<{
+  insertById: Record<string, string>;
+  deleteById: Record<string, string>;
+  docText: string;
+  listItems: Array<{ type: string; text: string }>;
+}> =>
+  superdoc.page.evaluate(async () => {
+    const editor = (window as any).editor;
+    const insertById: Record<string, string> = {};
+    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 === 'trackInsert') {
+          const id = String(mark.attrs?.id ?? '');
+          insertById[id] = (insertById[id] ?? '') + node.text;
+        } else if (mark.type?.name === 'trackDelete') {
+          const id = String(mark.attrs?.id ?? '');
+          deleteById[id] = (deleteById[id] ?? '') + node.text;
+        }
+      }
+    });
+    const list = await editor.doc.trackChanges.list();
+    return {
+      insertById,
+      deleteById,
+      docText: editor.state.doc.textContent,
+      listItems: (list?.items ?? []).map((item: any) => ({
+        type: String(item.type),
+        text: String(item.insertedText ?? item.deletedText ?? ''),
+      })),
+    };
+  });
+
+/** Type a pending insertion in suggesting mode, then delete "HELLO" inside it. */
+async function typeThenDeleteInsideOwnInsertion(superdoc: SuperDocFixture): Promise<void> {
+  await superdoc.setDocumentMode('suggesting');
+  await superdoc.waitForStable();
+  await superdoc.type('ABCHELLOXYZ');
+  await superdoc.waitForStable();
+
+  const pos = await superdoc.findTextPos('HELLO');
+  await superdoc.setTextSelection(pos, pos + 'HELLO'.length);
+  await superdoc.waitForStable();
+  await superdoc.press('Backspace');
+  await superdoc.waitForStable();
+}
+
+const expectSingleRefinedInsertion = (state: Awaited<ReturnType<typeof snapshotTrackedState>>) => {
+  expect(state.docText).toBe('ABCXYZ');
+  // No overlapping deletion was minted over the user's own suggestion.
+  expect(Object.keys(state.deleteById)).toHaveLength(0);
+  // One logical insertion remains, refined to the surviving text.
+  const insertIds = Object.keys(state.insertById);
+  expect(insertIds).toHaveLength(1);
+  expect(state.insertById[insertIds[0]]).toBe('ABCXYZ');
+  // Public read model agrees: exactly one reviewable insert item.
+  expect(state.listItems).toEqual([{ type: 'insert', text: 'ABCXYZ' }]);
+};
+
+test('anonymous session: deleting inside own pending insertion refines it in place (SD-3352)', async ({ superdoc }) => {
+  await setUser(superdoc, ANONYMOUS_DEFAULT);
+  await typeThenDeleteInsideOwnInsertion(superdoc);
+  expectSingleRefinedInsertion(await snapshotTrackedState(superdoc));
+});
+
+test('identified user: deleting inside own pending insertion refines it in place', async ({ superdoc }) => {
+  await setUser(superdoc, REVIEWER);
+  await typeThenDeleteInsideOwnInsertion(superdoc);
+  expectSingleRefinedInsertion(await snapshotTrackedState(superdoc));
+});
+
+test('anonymous session: typing over a selection inside own insertion refines it in place (SD-3352)', async ({
+  superdoc,
+}) => {
+  // Replace-path variant: selecting inside your own pending insertion and
+  // typing must refine the insertion under its original id — not collapse the
+  // deleted half while minting the replacement text as a nested child
+  // insertion (overlapParentId) under your own suggestion.
+  await setUser(superdoc, ANONYMOUS_DEFAULT);
+  await superdoc.setDocumentMode('suggesting');
+  await superdoc.waitForStable();
+  await superdoc.type('ABCHELLOXYZ');
+  await superdoc.waitForStable();
+
+  const pos = await superdoc.findTextPos('HELLO');
+  await superdoc.setTextSelection(pos, pos + 'HELLO'.length);
+  await superdoc.waitForStable();
+  await superdoc.type('Q');
+  await superdoc.waitForStable();
+
+  const state = await snapshotTrackedState(superdoc);
+  expect(state.docText).toBe('ABCQXYZ');
+  expect(Object.keys(state.deleteById)).toHaveLength(0);
+  const insertIds = Object.keys(state.insertById);
+  expect(insertIds).toHaveLength(1);
+  expect(state.insertById[insertIds[0]]).toBe('ABCQXYZ');
+  expect(state.listItems).toEqual([{ type: 'insert', text: 'ABCQXYZ' }]);
+});
+
+test('anonymous session: backspacing char-by-char through own insertion stays one refined insertion', async ({
+  superdoc,
+}) => {
+  await setUser(superdoc, ANONYMOUS_DEFAULT);
+  await superdoc.setDocumentMode('suggesting');
+  await superdoc.waitForStable();
+  await superdoc.type('ABCHELLOXYZ');
+  await superdoc.waitForStable();
+
+  // Caret after the final "O" of HELLO, then one Backspace per character.
+  const pos = await superdoc.findTextPos('HELLO');
+  await superdoc.setTextSelection(pos + 'HELLO'.length);
+  await superdoc.waitForStable();
+  for (let i = 0; i < 'HELLO'.length; i += 1) {
+    await superdoc.press('Backspace');
+    await superdoc.waitForStable();
+  }
+
+  expectSingleRefinedInsertion(await snapshotTrackedState(superdoc));
+});