Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,65 @@ 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 an empty structural gap. 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
* with no intervening text; without the gap-tolerant pass, deleting the space
* at the run seam would mint a new change. The gap is bounded and gated on
* `isEmptyStructuralGap`, so a live character between two deletions still
* splits them. Mirrors the insertion refinement in `compileTextInsert`
* (`findAdjacentInsertedSegment` + `findSegmentAcrossEmptyStructuralGap`).
Comment thread
chittolinag marked this conversation as resolved.
Outdated
*
* @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;
Comment thread
chittolinag marked this conversation as resolved.

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;
Expand All @@ -291,6 +350,49 @@ const isEmptyStructuralGap = (ctx, from, to) => {
return !hasInlineLeaf;
};

/**
* Zero-width review/anchor marker node types. They occupy a document position
* but render no visible content, so a contiguous visible-text deletion spans
* them. They must not block deletion coalescing — e.g. Google Docs anchors
* comments with inline marker nodes that sit between runs.
*/
const ZERO_WIDTH_ANCHOR_NODE_NAMES = new Set([
'commentRangeStart',
'commentRangeEnd',
'commentReference',
'bookmarkStart',
'bookmarkEnd',
]);
Comment thread
chittolinag marked this conversation as resolved.

/**
* 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);
Expand Down Expand Up @@ -543,16 +645,37 @@ 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.
const coalesceTarget =
intent.replacementGroupHint || intent.preserveExistingReviewState
? null
: findAdjacentDeletedSegment(ctx, intent.from, intent.to);
const sharedDeletionId = intent.replacementGroupHint || coalesceTarget?.changeId || null;
Comment thread
harbournick marked this conversation as resolved.

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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
@@ -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.');
});
Loading