From 30467182c2f4f63f53cfbbf40e61f0c5049c900c Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Thu, 28 May 2026 16:45:42 -0700 Subject: [PATCH 1/5] fix: nested replacement tracked-change decisions --- .../review-model/decision-engine.js | 106 +++++++++++++++++- .../review-model/decision-engine.test.js | 60 ++++++++++ .../review-model/overlap-compiler.js | 10 +- .../review-model/overlap-compiler.test.js | 39 +++---- 4 files changed, 184 insertions(+), 31 deletions(-) diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.js b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.js index 19d81f0346..b27a04d0e6 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.js @@ -461,7 +461,7 @@ const buildMutationPlan = ({ state, graph, selections, decision, replacements }) } else if (change.type === CanonicalChangeType.Deletion) { planDeletionDecision({ ops, change, selection, decision, removedRanges, retired }); } else if (change.type === CanonicalChangeType.Replacement) { - const repResult = planReplacementDecision({ ops, change, decision, removedRanges, retired }); + const repResult = planReplacementDecision({ ops, graph, change, decision, removedRanges, retired }); if (!repResult.ok) return { ok: false, failure: repResult.failure }; } else if (change.type === CanonicalChangeType.Formatting) { planFormattingDecision({ ops, change, decision, retired }); @@ -590,7 +590,7 @@ const planDeletionDecision = ({ ops, change, selection, decision, removedRanges, if (isFull) retired.add(change.id); }; -const planReplacementDecision = ({ ops, change, decision, removedRanges, retired }) => { +const planReplacementDecision = ({ ops, graph, change, decision, removedRanges, retired }) => { const inserted = change.insertedSegments; const deleted = change.deletedSegments; if (!inserted.length || !deleted.length) { @@ -618,6 +618,7 @@ const planReplacementDecision = ({ ops, change, decision, removedRanges, retired ops.push({ kind: 'removeContent', from: seg.from, to: seg.to, changeId: change.id, side: SegmentSide.Inserted }); removedRanges.push({ from: seg.from, to: seg.to, cause: `reject-replacement-inserted:${change.id}` }); } + const parentRestore = getParentRestoreContext({ graph, change }); for (const seg of deleted) { pushRemoveMarkOpsForSegment({ ops, @@ -625,12 +626,97 @@ const planReplacementDecision = ({ ops, change, decision, removedRanges, retired changeId: change.id, side: SegmentSide.Deleted, }); + if (parentRestore?.mark) { + pushAddMarkOpsForSegment({ + ops, + segment: seg, + changeId: parentRestore.mark.attrs?.id || change.parent || change.id, + side: parentRestore.mark.type.name === TrackInsertMarkName ? SegmentSide.Inserted : SegmentSide.Deleted, + mark: parentRestore.mark, + }); + } + } + for (const sibling of parentRestore?.siblingSegments ?? []) { + pushRemoveMarkOpsForSegment({ + ops, + segment: sibling, + changeId: sibling.changeId, + side: sibling.side, + }); + pushAddMarkOpsForSegment({ + ops, + segment: sibling, + changeId: parentRestore.mark.attrs?.id || sibling.changeId, + side: parentRestore.mark.type.name === TrackInsertMarkName ? SegmentSide.Inserted : SegmentSide.Deleted, + mark: parentRestore.mark, + }); + retired.add(sibling.changeId); } } retired.add(change.id); return { ok: true }; }; +const getParentRestoreContext = ({ graph, change }) => { + const explicit = getExplicitParentRestoreContext({ graph, change }); + if (explicit) return explicit; + return inferAdjacentParentRestoreContext({ graph, change }); +}; + +const getExplicitParentRestoreContext = ({ graph, change }) => { + if (!change.parent) return null; + const parent = graph.changes.get(change.parent); + if (!parent) return null; + if (parent.type === CanonicalChangeType.Insertion) { + const mark = parent.insertedSegments[0]?.mark ?? null; + return mark ? { mark, siblingSegments: [] } : null; + } + if (parent.type === CanonicalChangeType.Deletion) { + const mark = parent.deletedSegments[0]?.mark ?? null; + return mark ? { mark, siblingSegments: [] } : null; + } + return null; +}; + +const inferAdjacentParentRestoreContext = ({ graph, change }) => { + if (!change.deletedSegments.length || !change.segments.length) return null; + const from = Math.min(...change.segments.map((segment) => segment.from)); + const to = Math.max(...change.segments.map((segment) => segment.to)); + const before = nearestSegmentBefore({ graph, change, from }); + const after = nearestSegmentAfter({ graph, change, to }); + if (!before || !after) return null; + if (!areParentRestorePeers(before, after)) return null; + + return { + mark: before.mark, + siblingSegments: after.changeId === before.changeId ? [] : [after], + }; +}; + +const nearestSegmentBefore = ({ graph, change, from }) => { + return graph.segments + .filter((segment) => segment.changeId !== change.id && segment.to <= from) + .sort((a, b) => b.to - a.to || b.from - a.from)[0]; +}; + +const nearestSegmentAfter = ({ graph, change, to }) => { + return graph.segments + .filter((segment) => segment.changeId !== change.id && segment.from >= to) + .sort((a, b) => a.from - b.from || a.to - b.to)[0]; +}; + +const areParentRestorePeers = (left, right) => { + if (!left || !right) return false; + if (left.side !== right.side) return false; + if (left.side !== SegmentSide.Inserted && left.side !== SegmentSide.Deleted) return false; + if (left.markType !== right.markType) return false; + return ( + left.attrs.author === right.attrs.author && + left.attrs.authorEmail === right.attrs.authorEmail && + left.attrs.date === right.attrs.date + ); +}; + const planFormattingDecision = ({ ops, change, decision, retired }) => { for (const seg of change.formattingSegments) { if (decision === 'accept') { @@ -748,6 +834,22 @@ const pushRemoveMarkOpsForSegment = ({ ops, segment, changeId, side, from = segm } }; +const pushAddMarkOpsForSegment = ({ ops, segment, changeId, side, mark, from = segment.from, to = segment.to }) => { + for (const run of getSegmentMarkRuns(segment)) { + const clippedFrom = Math.max(from, run.from); + const clippedTo = Math.min(to, run.to); + if (clippedFrom >= clippedTo) continue; + ops.push({ + kind: 'addMark', + from: clippedFrom, + to: clippedTo, + changeId, + side, + mark, + }); + } +}; + const getSegmentMarkRuns = (segment) => { return segment.markRuns?.length ? segment.markRuns : [{ from: segment.from, to: segment.to, mark: segment.mark }]; }; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.test.js index cf14ff45cf..018a195122 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.test.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.test.js @@ -311,6 +311,66 @@ describe('decideTrackedChanges overlap behavior', () => { expect(state.apply(result.tr).doc.textContent).toBe('a OLD b'); }); + it('reject replacement between split same-author deletion fragments restores the inferred parent deletion', () => { + const schema = createReviewGraphTestSchema(); + const replacementAttrs = { + changeType: 'replacement', + replacementGroupId: 'rep-3', + }; + const parentLeft = deleteAttrs('parent-left', OTHER_USER, { date: '2026-05-20T14:08:00Z' }); + const parentRight = deleteAttrs('parent-right', OTHER_USER, { date: '2026-05-20T14:08:00Z' }); + const { state } = stateFromTrackedSpans({ + schema, + spans: [ + { text: 'a ' }, + { text: 'B', marks: [{ markType: TrackDeleteMarkName, attrs: parentLeft }] }, + { + text: 'B', + marks: [ + { + markType: TrackDeleteMarkName, + attrs: deleteAttrs('rep-3', SAME_USER, { + ...replacementAttrs, + replacementSideId: 'rep-3#deleted', + }), + }, + ], + }, + { + text: 'ZZ', + marks: [ + { + markType: TrackInsertMarkName, + attrs: insertAttrs('rep-3', SAME_USER, { + ...replacementAttrs, + replacementSideId: 'rep-3#inserted', + }), + }, + ], + }, + { text: 'B', marks: [{ markType: TrackDeleteMarkName, attrs: parentRight }] }, + { text: ' b' }, + ], + }); + + const result = decideTrackedChanges({ + state, + editor: editorFor(SAME_USER), + decision: 'reject', + target: { kind: 'id', id: 'rep-3' }, + }); + + expect(result.ok).toBe(true); + const next = state.apply(result.tr); + expect(next.doc.textContent).toBe('a BBB b'); + const graph = buildReviewGraph({ state: next }); + expect(graph.changes.size).toBe(1); + const parent = graph.changes.get('parent-left'); + expect(parent).toBeDefined(); + expect(parent.type).toBe('deletion'); + expect(parent.deletedSegments.map((segment) => segment.text).join('')).toBe('BBB'); + }); + it('formatting accept removes the trackFormat mark; reject restores the before snapshot', () => { const schema = createReviewGraphTestSchema(); const beforeSnap = [{ type: TrackInsertMarkName, attrs: insertAttrs('inner-ins') }]; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.js b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.js index 717e7ead95..9c83c5fe93 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.js @@ -880,12 +880,10 @@ const compileTextReplace = (ctx, intent) => { * @returns {TrackedEditResult} */ const compileOrdinaryTextReplace = (ctx, intent, sanitizedSlice, replacementParentId) => { - // In paired mode share one id between insert/delete sides so a top-level - // replacement projects as one logical graph change. A replacement nested - // inside another author's open review item must keep each side separately - // reviewable, so those child sides intentionally use distinct ids even when - // the caller's default replacement mode is paired. - const shouldPairReplacement = intent.replacements === 'paired' && !replacementParentId; + // In paired mode share one id between insert/delete sides so the replacement + // projects as one logical child change. The parent remains independently + // reviewable through overlapParentId. + const shouldPairReplacement = intent.replacements === 'paired'; const sharedId = shouldPairReplacement ? intent.replacementGroupHint || uuidv4() : null; const replacementGroupId = sharedId ?? ''; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.test.js index a1d00645bc..d10273538e 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.test.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.test.js @@ -688,7 +688,7 @@ describe('overlap-compiler: text-delete', () => { }); describe('overlap-compiler: text-replace inside named no-email insertion', () => { - it('preserves parent insertion and creates child replacement sides', () => { + it('preserves parent insertion and creates one child replacement', () => { const parentId = 'ins-alice-no-email'; const { state } = stateFromTrackedSpans({ schema, @@ -721,22 +721,17 @@ describe('overlap-compiler: text-replace inside named no-email insertion', () => expect(textOf(result.tr)).toBe('lazyquickly '); const graph = buildReviewGraph({ state: { doc: result.tr.doc } }); - expect(graph.changes.size).toBe(3); + expect(graph.changes.size).toBe(2); const parent = graph.changes.get(parentId); expect(parent).toBeDefined(); expect(parent.type).toBe(CanonicalChangeType.Insertion); - const childDelete = Array.from(graph.changes.values()).find( - (change) => change.type === CanonicalChangeType.Deletion, - ); - const childInsert = Array.from(graph.changes.values()).find( - (change) => change.type === CanonicalChangeType.Insertion && change.id !== parentId, - ); - expect(childDelete).toBeDefined(); - expect(childDelete.deletedSegments[0].text).toBe('lazy'); - expect(childDelete.deletedSegments[0].attrs.overlapParentId).toBe(parentId); - expect(childInsert).toBeDefined(); - expect(childInsert.insertedSegments.map((segment) => segment.text).join('')).toBe('quickly'); - expect(childInsert.insertedSegments[0].attrs.overlapParentId).toBe(parentId); + const childReplacement = Array.from(graph.changes.values()).find((change) => change.id !== parentId); + expect(childReplacement).toBeDefined(); + expect(childReplacement.type).toBe(CanonicalChangeType.Replacement); + expect(childReplacement.deletedSegments[0].text).toBe('lazy'); + expect(childReplacement.deletedSegments[0].attrs.overlapParentId).toBe(parentId); + expect(childReplacement.insertedSegments.map((segment) => segment.text).join('')).toBe('quickly'); + expect(childReplacement.insertedSegments[0].attrs.overlapParentId).toBe(parentId); }); }); @@ -815,7 +810,7 @@ describe('overlap-compiler: text-replace produces paired replacement metadata', expect(change.replacement?.deleted.length).toBeGreaterThan(0); }); - it('keeps both sides of a child replacement separately reviewable under an other-user insertion parent', () => { + it('keeps both sides of a child replacement under an other-user insertion parent', () => { const parentId = 'ins-bob'; const { state } = stateFromTrackedSpans({ schema, @@ -837,14 +832,12 @@ describe('overlap-compiler: text-replace produces paired replacement metadata', expect(result.ok).toBe(true); const graph = buildReviewGraph({ state: { doc: result.tr.doc }, replacementsMode: 'paired' }); const children = Array.from(graph.changes.values()).filter((change) => change.parent === parentId); - expect(children).toHaveLength(2); - expect(children.map((change) => change.type).sort()).toEqual([ - CanonicalChangeType.Deletion, - CanonicalChangeType.Insertion, - ]); - expect( - children.every((change) => change.segments.every((segment) => segment.attrs.overlapParentId === parentId)), - ).toBe(true); + expect(children).toHaveLength(1); + const childReplacement = children[0]; + expect(childReplacement.type).toBe(CanonicalChangeType.Replacement); + expect(childReplacement.deletedSegments.map((segment) => segment.text).join('')).toBe('or'); + expect(childReplacement.insertedSegments.map((segment) => segment.text).join('')).toBe('AR'); + expect(childReplacement.segments.every((segment) => segment.attrs.overlapParentId === parentId)).toBe(true); }); it('honors a provided logical id hint for paired document-api replacements', () => { From 285efcfbddb49d541a4966ddf53c008d2fc2d360 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Thu, 28 May 2026 17:10:31 -0700 Subject: [PATCH 2/5] chore: generate all From cfba04f6b6e36b15a0632244d8183f5a0952cba8 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Fri, 29 May 2026 14:55:51 -0700 Subject: [PATCH 3/5] fix: more fixes around overlapped changes --- .../v2/importer/trackedChangeIdMapper.js | 101 +++++++++++++++--- .../find/text-strategy.ts | 42 ++++---- .../review-model/overlap-compiler.js | 43 ++++---- .../trackChangesHelpers/replaceStep.js | 7 +- 4 files changed, 141 insertions(+), 52 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/trackedChangeIdMapper.js b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/trackedChangeIdMapper.js index 2710d1b6c0..ce9562724f 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/trackedChangeIdMapper.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/trackedChangeIdMapper.js @@ -3,8 +3,8 @@ import { v4 as uuidv4 } from 'uuid'; /** * @typedef {'paired' | 'independent'} TrackChangesReplacements - * @typedef {{ type: string, author: string, date: string, internalId: string }} TrackedChangeEntry - * @typedef {{ lastTrackedChange: TrackedChangeEntry | null, replacements: TrackChangesReplacements }} WalkContext + * @typedef {{ type: string, author: string, date: string, internalId?: string }} TrackedChangeEntry + * @typedef {{ beforeLastTrackedChange: TrackedChangeEntry | null, lastTrackedChange: TrackedChangeEntry | null, replacements: TrackChangesReplacements }} WalkContext */ const TRACKED_CHANGE_NAMES = new Set(['w:ins', 'w:del']); @@ -44,6 +44,66 @@ function isReplacementPair(previous, current) { return previous.type !== current.type && previous.author === current.author && previous.date === current.date; } +/** + * @param {object} element + * @returns {TrackedChangeEntry} + */ +function trackedChangeEntryFromElement(element) { + return { + type: element.name, + author: element.attributes?.['w:author'] ?? '', + date: element.attributes?.['w:date'] ?? '', + }; +} + +/** + * Returns the next sibling tracked-change element, skipping only non-content + * markers. Content-bearing elements terminate the sibling check because they + * break Word replacement adjacency. + * + * @param {Array} elements + * @param {number} startIndex + * @returns {TrackedChangeEntry | null} + */ +function findNextSiblingTrackedChange(elements, startIndex) { + if (!Array.isArray(elements)) return null; + + for (let i = startIndex; i < elements.length; i += 1) { + const element = elements[i]; + if (TRACKED_CHANGE_NAMES.has(element?.name)) { + return trackedChangeEntryFromElement(element); + } + if (!PAIRING_TRANSPARENT_NAMES.has(element?.name)) { + return null; + } + } + + return null; +} + +/** + * Word serializes a replacement selected inside another author's deletion as + * child insertion/deletion sides surrounded by the parent deletion fragments. + * In paired mode the generic adjacent-replacement heuristic would otherwise + * collapse the child sides into one replacement. Keep them independent when + * either side of the candidate pair touches a different-author deletion. + * + * @param {TrackedChangeEntry | null} beforePrevious + * @param {TrackedChangeEntry} previous + * @param {TrackedChangeEntry} current + * @param {TrackedChangeEntry | null} next + * @returns {boolean} + */ +function isChildReplacementInsideDeletion(beforePrevious, previous, current, next) { + if (!isReplacementPair(previous, current)) return false; + + const touchesDifferentAuthorDeletionBefore = + beforePrevious?.type === 'w:del' && beforePrevious.author !== previous.author; + const touchesDifferentAuthorDeletionAfter = next?.type === 'w:del' && next.author !== previous.author; + + return touchesDifferentAuthorDeletionBefore || touchesDifferentAuthorDeletionAfter; +} + /** * Assigns an internal UUID to a tracked change element. In paired mode, * adjacent replacement halves (w:del + w:ins with matching author/date) @@ -53,8 +113,9 @@ function isReplacementPair(previous, current) { * @param {Map} idMap Accumulates Word ID → internal UUID * @param {WalkContext} context Mutable walk state for replacement pairing * @param {boolean} insideTrackedChange Whether this element is nested in another tracked change + * @param {TrackedChangeEntry | null} nextTrackedChange */ -function assignInternalId(element, idMap, context, insideTrackedChange) { +function assignInternalId(element, idMap, context, insideTrackedChange, nextTrackedChange = null) { const wordId = String(element.attributes?.['w:id'] ?? ''); if (!wordId) return; @@ -66,15 +127,24 @@ function assignInternalId(element, idMap, context, insideTrackedChange) { return; } - const current = { - type: element.name, - author: element.attributes?.['w:author'] ?? '', - date: element.attributes?.['w:date'] ?? '', - }; + const current = trackedChangeEntryFromElement(element); const shouldPair = context.replacements === 'paired'; + const shouldKeepChildSides = + context.lastTrackedChange && + isChildReplacementInsideDeletion( + context.beforeLastTrackedChange, + context.lastTrackedChange, + current, + nextTrackedChange, + ); - if (shouldPair && context.lastTrackedChange && isReplacementPair(context.lastTrackedChange, current)) { + if ( + shouldPair && + context.lastTrackedChange && + !shouldKeepChildSides && + isReplacementPair(context.lastTrackedChange, current) + ) { // Second half of a replacement — share the first half's UUID, but only // if this w:id hasn't already been mapped. A reused id that was already // part of an earlier pair must keep its original mapping. @@ -82,6 +152,7 @@ function assignInternalId(element, idMap, context, insideTrackedChange) { idMap.set(wordId, context.lastTrackedChange.internalId); } context.lastTrackedChange = null; + context.beforeLastTrackedChange = null; } else { // Reuse an existing mapping when the same w:id appears more than once // (Word reuses tracked-change ids across the document). Minting a fresh @@ -89,6 +160,7 @@ function assignInternalId(element, idMap, context, insideTrackedChange) { // pair that was already recorded for this id. const internalId = idMap.get(wordId) ?? uuidv4(); idMap.set(wordId, internalId); + context.beforeLastTrackedChange = context.lastTrackedChange; context.lastTrackedChange = { ...current, internalId }; } } @@ -105,9 +177,11 @@ function assignInternalId(element, idMap, context, insideTrackedChange) { function walkElements(elements, idMap, context, insideTrackedChange = false) { if (!Array.isArray(elements)) return; - for (const element of elements) { + for (let index = 0; index < elements.length; index += 1) { + const element = elements[index]; if (TRACKED_CHANGE_NAMES.has(element.name)) { - assignInternalId(element, idMap, context, insideTrackedChange); + const nextTrackedChange = findNextSiblingTrackedChange(elements, index + 1); + assignInternalId(element, idMap, context, insideTrackedChange, nextTrackedChange); if (element.elements) { // Descend with an isolated context so content inside a tracked change @@ -116,7 +190,7 @@ function walkElements(elements, idMap, context, insideTrackedChange = false) { walkElements( element.elements, idMap, - { lastTrackedChange: null, replacements: context.replacements }, + { beforeLastTrackedChange: null, lastTrackedChange: null, replacements: context.replacements }, /* insideTrackedChange */ true, ); } @@ -125,6 +199,7 @@ function walkElements(elements, idMap, context, insideTrackedChange = false) { // markers (comment/bookmark/permission ranges) are transparent. if (!PAIRING_TRANSPARENT_NAMES.has(element.name)) { context.lastTrackedChange = null; + context.beforeLastTrackedChange = null; } if (element.elements) { @@ -150,7 +225,7 @@ function buildTrackedChangeIdMapForPart(part, options = {}) { const replacements = options.replacements === 'independent' ? 'independent' : 'paired'; const idMap = new Map(); - walkElements(root.elements, idMap, { lastTrackedChange: null, replacements }); + walkElements(root.elements, idMap, { beforeLastTrackedChange: null, lastTrackedChange: null, replacements }); return idMap; } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/find/text-strategy.ts b/packages/super-editor/src/editors/v1/document-api-adapters/find/text-strategy.ts index a24765be07..bc26479f97 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/find/text-strategy.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/find/text-strategy.ts @@ -101,25 +101,31 @@ export function executeTextSelector( const search = requireEditorCommand(editor.commands?.search, 'find (search)'); - const rawResult = search(pattern, { - highlight: false, - caseSensitive: selector.caseSensitive ?? false, - maxMatches: Infinity, - searchModel: 'visible', - }); - - if (!Array.isArray(rawResult)) { - throw new DocumentApiAdapterError( - 'CAPABILITY_UNAVAILABLE', - 'Editor search command returned an unexpected result format.', - ); - } - const allMatches = rawResult as SearchMatch[]; - const scopeRange = scope.range; - const matches = scopeRange - ? allMatches.filter((m) => m.from >= scopeRange.start && m.to <= scopeRange.end) - : allMatches; + const runSearch = (searchModel: 'visible' | 'raw'): SearchMatch[] => { + pattern.lastIndex = 0; + const rawResult = search(pattern, { + highlight: false, + caseSensitive: selector.caseSensitive ?? false, + maxMatches: Infinity, + searchModel, + }); + + if (!Array.isArray(rawResult)) { + throw new DocumentApiAdapterError( + 'CAPABILITY_UNAVAILABLE', + 'Editor search command returned an unexpected result format.', + ); + } + + const allMatches = rawResult as SearchMatch[]; + return scopeRange ? allMatches.filter((m) => m.from >= scopeRange.start && m.to <= scopeRange.end) : allMatches; + }; + + let matches = runSearch('visible'); + if (matches.length === 0) { + matches = runSearch('raw'); + } const textBlocks = index.candidates.filter(isTextBlockCandidate); const contexts: MatchContext[] = []; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.js b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.js index 9c83c5fe93..e7e263187b 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.js @@ -544,6 +544,7 @@ const compileTextDelete = (ctx, intent) => { replacementSideId: '', sharedDeletionId: intent.replacementGroupHint || null, recordSharedDeletionId: Boolean(intent.replacementGroupHint), + reassignExistingDeletions: intent.source !== 'native' && !intent.preserveExistingReviewState, }); if (result.ok === false) return result; @@ -647,12 +648,18 @@ const applyTrackedDelete = ( if (existingDelete) { const allExistingDeletes = node.marks.filter((m) => m.type.name === TrackDeleteMarkName); - if (reassignExistingDeletions) { + const deleteOwnership = classifyOwnership({ + currentUser: ctx.currentIdentity, + change: getChangeAuthorIdentity(existingDelete.attrs), + }); + const isDifferentUserDeletion = !isSameUserHighConfidence(deleteOwnership); + if (reassignExistingDeletions && isDifferentUserDeletion) { ops.push({ kind: 'reassign', from: segFrom, to: segTo, node, + parentId: existingDelete.attrs.id || existingDelete.attrs.overlapParentId || '', existingDeleteMarks: allExistingDeletes, }); return; @@ -696,7 +703,7 @@ const applyTrackedDelete = ( // deletion mark so the new replacement encloses the prior delete. const mark = makeDeleteMark(ctx, { id: deletionId, - overlapParentId: '', + overlapParentId: op.parentId || '', replacementGroupId, replacementSideId, }); @@ -880,21 +887,21 @@ const compileTextReplace = (ctx, intent) => { * @returns {TrackedEditResult} */ const compileOrdinaryTextReplace = (ctx, intent, sanitizedSlice, replacementParentId) => { - // In paired mode share one id between insert/delete sides so the replacement - // projects as one logical child change. The parent remains independently - // reviewable through overlapParentId. - const shouldPairReplacement = intent.replacements === 'paired'; + // In paired mode ordinary replacements share one id between insert/delete + // sides. Nested replacements inside another author's pending change must + // keep the child insertion and deletion as independently reviewable sides. + const shouldPairReplacement = intent.replacements === 'paired' && !replacementParentId; const sharedId = shouldPairReplacement ? intent.replacementGroupHint || uuidv4() : null; const replacementGroupId = sharedId ?? ''; // 1. Probe for adjacent tracked-delete span at intent.to - 1 (legacy // behavior). Only applies for single-step user actions — plan-engine // multi-step rewrites must not probe. - let positionTo = intent.to; + let positionTo = replacementParentId ? intent.from : intent.to; if (intent.from !== intent.to && intent.probeForDeletionSpan) { const probePos = Math.max(intent.from, intent.to - 1); const deletionSpan = findMarkPosition(ctx.tr.doc, probePos, TrackDeleteMarkName); - if (deletionSpan && deletionSpan.to > positionTo) positionTo = deletionSpan.to; + if (!replacementParentId && deletionSpan && deletionSpan.to > positionTo) positionTo = deletionSpan.to; } // 2. Build a temp insertion in a throwaway transaction so we can read the @@ -938,9 +945,8 @@ const compileOrdinaryTextReplace = (ctx, intent, sanitizedSlice, replacementPare if (insertion && insertion.insertedFrom !== insertion.insertedTo) { const { tempTr, insertedFrom, insertedTo } = insertion; // Use the legacy markInsertion primitive so id reuse / refinement matches - // existing behavior exactly. Compiler-specific overlap fields - // (overlapParentId, replacementGroupId, replacementSideId) are layered on - // afterward. + // existing behavior exactly for ordinary replacements. Nested replacements + // force a fresh child insertion side under the parent. const forcedInsertId = sharedId || (replacementParentId ? uuidv4() : undefined); insertedMark = markInsertion({ tr: tempTr, @@ -993,11 +999,10 @@ const compileOrdinaryTextReplace = (ctx, intent, sanitizedSlice, replacementPare insertedLength = insertedToAbs - insertedFromAbs; } - // 4. Apply tracked delete on the original range. The range positions are - // unaffected by the insertion (insertion happened at positionTo which is - // >= intent.to). The delete may collapse own insertions inside the - // range, shifting the doc — we map the inserted position through the - // delete-induced map after. + // 4. Apply tracked delete on the original range. Ordinary replacements + // insert at or after intent.to, so the original range is stable. Nested + // replacements insert at intent.from; in that case remap the selected + // original text past the inserted child side before marking deletion. /** @type {Array} */ let deletionMarks = []; /** @type {Array} */ @@ -1007,11 +1012,13 @@ const compileOrdinaryTextReplace = (ctx, intent, sanitizedSlice, replacementPare if (intent.from !== intent.to) { const stepsBefore = ctx.tr.steps.length; - const delResult = applyTrackedDelete(ctx, intent.from, intent.to, { + const deleteFrom = insertedLength > 0 && positionTo <= intent.from ? intent.from + insertedLength : intent.from; + const deleteTo = insertedLength > 0 && positionTo <= intent.from ? intent.to + insertedLength : intent.to; + const delResult = applyTrackedDelete(ctx, deleteFrom, deleteTo, { replacementGroupId, replacementSideId: sharedId ? `${sharedId}#deleted` : '', sharedDeletionId: sharedId, - reassignExistingDeletions: Boolean(sharedId), + reassignExistingDeletions: Boolean(sharedId) || Boolean(replacementParentId), }); if (delResult.ok === false) return delResult; deletionMarks = delResult.deletionMarks; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/replaceStep.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/replaceStep.js index e3134c7a8f..1e0e241fb5 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/replaceStep.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/replaceStep.js @@ -264,13 +264,14 @@ const tryCompileStep = ({ let intent; try { const preserveExistingReviewState = tr.getMeta('protectTrackedReviewState') === true; + const source = tr.getMeta('inputType') === 'programmatic' ? 'document-api' : 'native'; if (step.from === step.to && step.slice.content.size > 0) { intent = makeTextInsertIntent({ at: step.from, content: step.slice, user, date, - source: 'native', + source, preserveExistingReviewState, }); } else if (step.from !== step.to && step.slice.content.size === 0) { @@ -279,7 +280,7 @@ const tryCompileStep = ({ to: step.to, user, date, - source: 'native', + source, preserveExistingReviewState, }); } else if (step.from !== step.to && step.slice.content.size > 0) { @@ -290,7 +291,7 @@ const tryCompileStep = ({ replacements, user, date, - source: 'native', + source, preserveExistingReviewState, }); // Single-step user actions (text replace from one ReplaceStep) probe From 5bef4200f7eb0ddd811de6eeba33910b95d6b406 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Fri, 29 May 2026 17:32:19 -0700 Subject: [PATCH 4/5] chore: more fixes --- .../v1/document-api-adapters/find-adapter.ts | 4 +- .../v1/document-api-adapters/find/common.ts | 60 +++++++- .../find/text-strategy.ts | 55 +++---- .../document-api-adapters/get-text-adapter.ts | 2 +- .../helpers/adapter-utils.ts | 11 +- .../helpers/sd-projection.ts | 139 +++++++++++------- .../helpers/selection-target-resolver.ts | 13 +- .../helpers/text-offset-resolver.test.ts | 42 +++++- .../helpers/text-offset-resolver.ts | 72 ++++++++- .../helpers/text-with-tabs.ts | 8 + .../plan-engine/compiler.ts | 79 +++++++--- .../plan-engine/executor.ts | 4 +- .../plan-engine/query-match-adapter.ts | 25 ++-- .../plan-engine/style-resolver.test.ts | 23 +++ .../plan-engine/style-resolver.ts | 17 ++- .../tracked-rewrite.integration.test.ts | 91 ++++++++++++ .../review-model/overlap-compiler.js | 16 +- .../review-model/overlap-compiler.test.js | 44 +++--- 18 files changed, 553 insertions(+), 152 deletions(-) diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/find-adapter.ts b/packages/super-editor/src/editors/v1/document-api-adapters/find-adapter.ts index 54d1013680..27436d9e70 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/find-adapter.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/find-adapter.ts @@ -249,12 +249,12 @@ function projectMatchToSDNodeResult( const found = blockIndex.candidates.find((c) => c.nodeType === address.nodeType && c.nodeId === address.nodeId); if (!found) return null; return { - node: projectContentNode(found.node), + node: projectContentNode(found.node, { textModel: 'visible' }), address, }; } return { - node: projectContentNode(candidate.node), + node: projectContentNode(candidate.node, { textModel: 'visible' }), address, }; } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/find/common.ts b/packages/super-editor/src/editors/v1/document-api-adapters/find/common.ts index 974a961ae4..88e3327f50 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/find/common.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/find/common.ts @@ -8,7 +8,7 @@ import type { UnknownNodeDiagnostic, } from '@superdoc/document-api'; import { toId } from '../helpers/value-utils.js'; -import { getInlineIndex } from '../helpers/index-cache.js'; +import { getBlockIndex, getInlineIndex } from '../helpers/index-cache.js'; import { findBlockById, toBlockAddress, @@ -17,6 +17,7 @@ import { } from '../helpers/node-address-resolver.js'; import { findInlineByAnchor, isInlineQueryType } from '../helpers/inline-address-resolver.js'; import { findCandidateByPos } from '../helpers/adapter-utils.js'; +import { pmPositionToTextOffset, textContentInBlock, type TextOffsetOptions } from '../helpers/text-offset-resolver.js'; /** Characters of document text to include before and after a match in snippet context. */ const SNIPPET_PADDING = 30; @@ -49,6 +50,13 @@ const KNOWN_INLINE_PM_NODE_TYPES = new Set([ 'commentRangeEnd', ]); +function getCandidateText(editor: Editor, candidate: BlockCandidate, options?: TextOffsetOptions): string { + if (candidate.node.childCount > 0) { + return textContentInBlock(candidate.node, options); + } + return editor.state.doc.textBetween(candidate.pos + 1, candidate.end - 1, '\n', '\ufffc'); +} + function resolveUnknownBlockId(attrs: Record | undefined): string | undefined { if (!attrs) return undefined; return toId(attrs.paraId) ?? toId(attrs.sdBlockId) ?? toId(attrs.blockId) ?? toId(attrs.id) ?? toId(attrs.uuid); @@ -85,7 +93,46 @@ export function buildTextContext( matchFrom: number, matchTo: number, textRanges?: TextAddress[], + options?: TextOffsetOptions, ): MatchContext { + if (textRanges?.length) { + const index = getBlockIndex(editor); + const firstRange = textRanges[0]; + const lastRange = textRanges[textRanges.length - 1]; + const firstBlock = index.candidates.find((candidate) => candidate.nodeId === firstRange.blockId); + const lastBlock = index.candidates.find((candidate) => candidate.nodeId === lastRange.blockId); + + if (firstBlock && lastBlock) { + const matchText = textRanges + .map((range) => { + const block = index.candidates.find((candidate) => candidate.nodeId === range.blockId); + if (!block) return ''; + return getCandidateText(editor, block, options).slice(range.range.start, range.range.end); + }) + .join('\n'); + const firstText = getCandidateText(editor, firstBlock, options); + const lastText = getCandidateText(editor, lastBlock, options); + const leftContext = firstText.slice( + Math.max(0, firstRange.range.start - SNIPPET_PADDING), + firstRange.range.start, + ); + const rightContext = lastText.slice(lastRange.range.end, lastRange.range.end + SNIPPET_PADDING); + const snippet = `${leftContext}${matchText}${rightContext}`.replace(/ {2,}/g, ' '); + const prefix = leftContext.replace(/ {2,}/g, ' '); + const normalizedMatch = matchText.replace(/ {2,}/g, ' '); + + return { + address, + snippet, + highlightRange: { + start: prefix.length, + end: prefix.length + normalizedMatch.length, + }, + textRanges, + }; + } + } + const docSize = editor.state.doc.content.size; const snippetFrom = Math.max(0, matchFrom - SNIPPET_PADDING); const snippetTo = Math.min(docSize, matchTo + SNIPPET_PADDING); @@ -120,13 +167,20 @@ export function toTextAddress( editor: Editor, block: BlockCandidate, range: { from: number; to: number }, + options?: TextOffsetOptions, ): TextAddress | undefined { const blockStart = block.pos + 1; const blockEnd = block.end - 1; if (range.from < blockStart || range.to > blockEnd) return undefined; - const start = editor.state.doc.textBetween(blockStart, range.from, '\n', '\ufffc').length; - const end = editor.state.doc.textBetween(blockStart, range.to, '\n', '\ufffc').length; + const start = + block.node.childCount > 0 + ? pmPositionToTextOffset(block.node, block.pos, range.from, options) + : editor.state.doc.textBetween(blockStart, range.from, '\n', '\ufffc').length; + const end = + block.node.childCount > 0 + ? pmPositionToTextOffset(block.node, block.pos, range.to, options) + : editor.state.doc.textBetween(blockStart, range.to, '\n', '\ufffc').length; return { kind: 'text', diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/find/text-strategy.ts b/packages/super-editor/src/editors/v1/document-api-adapters/find/text-strategy.ts index bc26479f97..1b3bea1fd4 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/find/text-strategy.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/find/text-strategy.ts @@ -19,6 +19,7 @@ import { addDiagnostic, findCandidateByPos, paginate, resolveWithinScope } from import { buildTextContext, toTextAddress } from './common.js'; import { DocumentApiAdapterError } from '../errors.js'; import { requireEditorCommand } from '../helpers/mutation-helpers.js'; +import type { TextOffsetModel } from '../helpers/text-offset-resolver.js'; /** Shape returned by `editor.commands.search`. */ type SearchMatch = { @@ -30,6 +31,10 @@ type SearchMatch = { /** Maximum allowed pattern length to guard against ReDoS and excessive memory usage. */ const MAX_PATTERN_LENGTH = 1024; +export type TextSelectorSearchModel = Extract; +export type ExecuteTextSelectorOptions = { + searchModel?: TextSelectorSearchModel; +}; function compileRegex(selector: TextSelector, diagnostics: UnknownNodeDiagnostic[]): RegExp | null { if (selector.pattern.length > MAX_PATTERN_LENGTH) { @@ -81,6 +86,7 @@ export function executeTextSelector( index: BlockIndex, query: Query, diagnostics: UnknownNodeDiagnostic[], + options: ExecuteTextSelectorOptions = {}, ): QueryResult { if (query.select.type !== 'text') { addDiagnostic(diagnostics, `Text strategy received a non-text selector (type="${query.select.type}").`); @@ -100,32 +106,29 @@ export function executeTextSelector( if (!pattern) return { matches: [], total: 0 }; const search = requireEditorCommand(editor.commands?.search, 'find (search)'); + const searchModel = options.searchModel ?? 'visible'; + const textOffsetOptions = { textModel: searchModel }; + + pattern.lastIndex = 0; + const rawResult = search(pattern, { + highlight: false, + caseSensitive: selector.caseSensitive ?? false, + maxMatches: Infinity, + searchModel, + }); + + if (!Array.isArray(rawResult)) { + throw new DocumentApiAdapterError( + 'CAPABILITY_UNAVAILABLE', + 'Editor search command returned an unexpected result format.', + ); + } const scopeRange = scope.range; - const runSearch = (searchModel: 'visible' | 'raw'): SearchMatch[] => { - pattern.lastIndex = 0; - const rawResult = search(pattern, { - highlight: false, - caseSensitive: selector.caseSensitive ?? false, - maxMatches: Infinity, - searchModel, - }); - - if (!Array.isArray(rawResult)) { - throw new DocumentApiAdapterError( - 'CAPABILITY_UNAVAILABLE', - 'Editor search command returned an unexpected result format.', - ); - } - - const allMatches = rawResult as SearchMatch[]; - return scopeRange ? allMatches.filter((m) => m.from >= scopeRange.start && m.to <= scopeRange.end) : allMatches; - }; - - let matches = runSearch('visible'); - if (matches.length === 0) { - matches = runSearch('raw'); - } + const allMatches = rawResult as SearchMatch[]; + const matches = scopeRange + ? allMatches.filter((m) => m.from >= scopeRange.start && m.to <= scopeRange.end) + : allMatches; const textBlocks = index.candidates.filter(isTextBlockCandidate); const contexts: MatchContext[] = []; @@ -139,7 +142,7 @@ export function executeTextSelector( const block = findCandidateByPos(textBlocks, range.from); if (!block) return undefined; if (!source) source = block; - return toTextAddress(editor, block, range); + return toTextAddress(editor, block, range, textOffsetOptions); }) .filter((range): range is TextAddress => Boolean(range)); @@ -150,7 +153,7 @@ export function executeTextSelector( const address = toBlockAddress(source); addresses.push(address); - contexts.push(buildTextContext(editor, address, match.from, match.to, textRanges)); + contexts.push(buildTextContext(editor, address, match.from, match.to, textRanges, textOffsetOptions)); } const paged = paginate(addresses, query.offset, query.limit); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/get-text-adapter.ts b/packages/super-editor/src/editors/v1/document-api-adapters/get-text-adapter.ts index a9628e0ccd..e4dd3c8c2b 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/get-text-adapter.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/get-text-adapter.ts @@ -16,5 +16,5 @@ import { textBetweenWithTabs } from './helpers/text-with-tabs.js'; export function getTextAdapter(editor: Editor, input: GetTextInput): string { const runtime = resolveStoryRuntime(editor, input.in); const doc = runtime.editor.state.doc; - return textBetweenWithTabs(doc, 0, doc.content.size, '\n', '\n'); + return textBetweenWithTabs(doc, 0, doc.content.size, '\n', '\n', { textModel: 'visible' }); } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/adapter-utils.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/adapter-utils.ts index 5917cfd775..2f6167e99b 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/adapter-utils.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/adapter-utils.ts @@ -74,7 +74,7 @@ export function resolveTextTarget(editor: Editor, target: TextAddress): Resolved assertUnambiguous(matches, target.blockId); const block = matches[0]; if (!block) return null; - return resolveTextRangeInBlock(block.node, block.pos, target.range); + return resolveTextRangeInBlock(block.node, block.pos, target.range, { textModel: 'visible' }); } /** @@ -167,8 +167,13 @@ export function resolveDefaultInsertTarget(editor: Editor): DefaultInsertTarget for (let i = index.candidates.length - 1; i >= 0; i--) { const candidate = index.candidates[i]; if (topLevelPositions.has(candidate.pos) && isTextBlockCandidate(candidate)) { - const textLength = computeTextContentLength(candidate.node); - const range = resolveTextRangeInBlock(candidate.node, candidate.pos, { start: textLength, end: textLength }); + const textLength = computeTextContentLength(candidate.node, { textModel: 'visible' }); + const range = resolveTextRangeInBlock( + candidate.node, + candidate.pos, + { start: textLength, end: textLength }, + { textModel: 'visible' }, + ); if (!range) continue; return { diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/sd-projection.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/sd-projection.ts index c1dad5a706..452312e7b4 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/sd-projection.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/sd-projection.ts @@ -45,6 +45,18 @@ import type { ParagraphAttrs, TableAttrs, TableCellAttrs, ImageAttrs } from '../ import { getHeadingLevel } from './node-address-resolver.js'; import { parseTocInstruction } from '../../core/super-converter/field-references/shared/toc-switches.js'; import { resolveSectionProjections, type SectionProjection } from './sections-resolver.js'; +import { textContentInBlock, type TextOffsetOptions } from './text-offset-resolver.js'; +import { TrackDeleteMarkName } from '../../extensions/track-changes/constants.js'; + +type ProjectionOptions = TextOffsetOptions; + +function isVisibleProjection(options?: ProjectionOptions): boolean { + return options?.textModel === 'visible'; +} + +function hasTrackDeleteMark(pmNode: ProseMirrorNode): boolean { + return pmNode.marks?.some((mark) => mark.type.name === TrackDeleteMarkName) ?? false; +} // --------------------------------------------------------------------------- // Public API @@ -53,15 +65,15 @@ import { resolveSectionProjections, type SectionProjection } from './sections-re /** * Projects a single ProseMirror content node into an SDM/1 SDContentNode. */ -export function projectContentNode(pmNode: ProseMirrorNode): SDContentNode { - return projectBlock(pmNode); +export function projectContentNode(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDContentNode { + return projectBlock(pmNode, options); } /** * Projects a ProseMirror text node (with marks) into SDM/1 inline nodes. */ -export function projectInlineNode(pmNode: ProseMirrorNode): SDInlineNode { - return projectInline(pmNode); +export function projectInlineNode(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDInlineNode { + return projectInline(pmNode, options) ?? { kind: 'run', run: { text: '' } }; } /** @@ -163,9 +175,10 @@ export function resolveTextByBlockId( export function projectDocument(editor: Editor, options?: SDReadOptions): SDDocument { const doc = editor.state.doc; const body: SDContentNode[] = []; + const projectionOptions: ProjectionOptions = { textModel: 'visible' }; doc.forEach((child) => { - body.push(projectBlock(child)); + body.push(projectBlock(child, projectionOptions)); }); const sections = projectSections(editor); @@ -319,28 +332,28 @@ interface TranslatedLevel { // Block-level dispatch // --------------------------------------------------------------------------- -function projectBlock(pmNode: ProseMirrorNode): SDContentNode { +function projectBlock(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDContentNode { const typeName = pmNode.type.name; switch (typeName) { case 'paragraph': - return projectParagraphOrHeading(pmNode); + return projectParagraphOrHeading(pmNode, options); case 'heading': - return projectHeadingNode(pmNode); + return projectHeadingNode(pmNode, options); case 'table': - return projectTable(pmNode); + return projectTable(pmNode, options); case 'bulletList': case 'orderedList': - return projectList(pmNode, typeName === 'orderedList'); + return projectList(pmNode, typeName === 'orderedList', options); case 'listItem': - return projectListItemAsContent(pmNode); + return projectListItemAsContent(pmNode, options); case 'image': return projectBlockImage(pmNode); case 'tableOfContents': return projectToc(pmNode); case 'sdt': case 'structuredContentBlock': - return projectBlockSdt(pmNode); + return projectBlockSdt(pmNode, options); case 'sectionBreak': return projectSectionBreak(pmNode); case 'pageBreak': @@ -349,7 +362,7 @@ function projectBlock(pmNode: ProseMirrorNode): SDContentNode { case 'drawing': return projectBlockDrawing(pmNode); default: - return projectFallbackBlock(pmNode); + return projectFallbackBlock(pmNode, options); } } @@ -357,26 +370,30 @@ function projectBlock(pmNode: ProseMirrorNode): SDContentNode { // Paragraph / Heading // --------------------------------------------------------------------------- -function projectParagraphOrHeading(pmNode: ProseMirrorNode): SDParagraph | SDHeading { +function projectParagraphOrHeading(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDParagraph | SDHeading { const attrs = pmNode.attrs as ParagraphAttrs | undefined; const headingLevel = getHeadingLevel(attrs?.paragraphProperties?.styleId); if (headingLevel && headingLevel >= 1 && headingLevel <= 6) { - return buildHeading(pmNode, attrs, headingLevel as 1 | 2 | 3 | 4 | 5 | 6); + return buildHeading(pmNode, attrs, headingLevel as 1 | 2 | 3 | 4 | 5 | 6, options); } - return buildParagraph(pmNode, attrs); + return buildParagraph(pmNode, attrs, options); } -function projectHeadingNode(pmNode: ProseMirrorNode): SDHeading { +function projectHeadingNode(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDHeading { const attrs = pmNode.attrs as ParagraphAttrs | undefined; const rawLevel = (pmNode.attrs as any)?.level; const level = (rawLevel ?? getHeadingLevel(attrs?.paragraphProperties?.styleId) ?? 1) as 1 | 2 | 3 | 4 | 5 | 6; - return buildHeading(pmNode, attrs, level); + return buildHeading(pmNode, attrs, level, options); } -function buildParagraph(pmNode: ProseMirrorNode, attrs: ParagraphAttrs | undefined): SDParagraph { - const inlines = projectInlineChildren(pmNode); +function buildParagraph( + pmNode: ProseMirrorNode, + attrs: ParagraphAttrs | undefined, + options?: ProjectionOptions, +): SDParagraph { + const inlines = projectInlineChildren(pmNode, options); const result: SDParagraph = { kind: 'paragraph', id: resolveNodeId(pmNode), @@ -396,8 +413,9 @@ function buildHeading( pmNode: ProseMirrorNode, attrs: ParagraphAttrs | undefined, level: 1 | 2 | 3 | 4 | 5 | 6, + options?: ProjectionOptions, ): SDHeading { - const inlines = projectInlineChildren(pmNode); + const inlines = projectInlineChildren(pmNode, options); const result: SDHeading = { kind: 'heading', id: resolveNodeId(pmNode), @@ -417,14 +435,14 @@ function buildHeading( // Table // --------------------------------------------------------------------------- -function projectTable(pmNode: ProseMirrorNode): SDTable { +function projectTable(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDTable { const attrs = pmNode.attrs as TableAttrs | undefined; const pmAttrs = pmNode.attrs as Record; const rows: SDTableRow[] = []; pmNode.forEach((child) => { if (child.type.name === 'tableRow') { - rows.push(projectTableRow(child)); + rows.push(projectTableRow(child, options)); } }); @@ -464,11 +482,11 @@ function projectTable(pmNode: ProseMirrorNode): SDTable { return result; } -function projectTableRow(pmNode: ProseMirrorNode): SDTableRow { +function projectTableRow(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDTableRow { const cells: SDTableCell[] = []; pmNode.forEach((child) => { if (child.type.name === 'tableCell' || child.type.name === 'tableHeader') { - cells.push(projectTableCell(child)); + cells.push(projectTableCell(child, options)); } }); @@ -482,11 +500,11 @@ function projectTableRow(pmNode: ProseMirrorNode): SDTableRow { return row; } -function projectTableCell(pmNode: ProseMirrorNode): SDTableCell { +function projectTableCell(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDTableCell { const attrs = pmNode.attrs as TableCellAttrs | undefined; const content: SDContentNode[] = []; pmNode.forEach((child) => { - content.push(projectBlock(child)); + content.push(projectBlock(child, options)); }); const cell: SDTableCell = { content }; @@ -506,11 +524,11 @@ function projectTableCell(pmNode: ProseMirrorNode): SDTableCell { // List // --------------------------------------------------------------------------- -function projectList(pmNode: ProseMirrorNode, ordered: boolean): SDList { +function projectList(pmNode: ProseMirrorNode, ordered: boolean, options?: ProjectionOptions): SDList { const items: SDListItem[] = []; pmNode.forEach((child) => { if (child.type.name === 'listItem') { - items.push(projectListItem(child)); + items.push(projectListItem(child, options)); } }); @@ -529,10 +547,10 @@ function projectList(pmNode: ProseMirrorNode, ordered: boolean): SDList { return result; } -function projectListItem(pmNode: ProseMirrorNode): SDListItem { +function projectListItem(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDListItem { const content: SDContentNode[] = []; pmNode.forEach((child) => { - content.push(projectBlock(child)); + content.push(projectBlock(child, options)); }); const item: SDListItem = { @@ -544,8 +562,8 @@ function projectListItem(pmNode: ProseMirrorNode): SDListItem { } /** When a listItem appears at top-level (orphan), wrap it in a paragraph. */ -function projectListItemAsContent(pmNode: ProseMirrorNode): SDParagraph { - const inlines = projectInlineChildren(pmNode); +function projectListItemAsContent(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDParagraph { + const inlines = projectInlineChildren(pmNode, options); return { kind: 'paragraph', id: resolveNodeId(pmNode), @@ -616,10 +634,10 @@ function extractSdtMetadata(attrs: Record): Omit { - children.push(projectBlock(child)); + children.push(projectBlock(child, options)); }); return { @@ -633,8 +651,8 @@ function projectBlockSdt(pmNode: ProseMirrorNode): SDSdt { }; } -function projectInlineSdt(pmNode: ProseMirrorNode): SDSdt { - const inlines = projectInlineChildren(pmNode); +function projectInlineSdt(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDSdt { + const inlines = projectInlineChildren(pmNode, options); return { kind: 'sdt', @@ -690,8 +708,8 @@ function projectBlockDrawing(pmNode: ProseMirrorNode): SDContentNode { // Fallback block // --------------------------------------------------------------------------- -function projectFallbackBlock(pmNode: ProseMirrorNode): SDParagraph { - const inlines = projectInlineChildren(pmNode); +function projectFallbackBlock(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDParagraph { + const inlines = projectInlineChildren(pmNode, options); return { kind: 'paragraph', id: resolveNodeId(pmNode), @@ -703,23 +721,23 @@ function projectFallbackBlock(pmNode: ProseMirrorNode): SDParagraph { // Inline projection // --------------------------------------------------------------------------- -function projectInlineChildren(pmNode: ProseMirrorNode): SDInlineNode[] { +function projectInlineChildren(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDInlineNode[] { const inlines: SDInlineNode[] = []; pmNode.forEach((child) => { - const projected = projectInline(child); - inlines.push(projected); + const projected = projectInline(child, options); + if (projected) inlines.push(projected); }); return inlines; } -function projectInline(pmNode: ProseMirrorNode): SDInlineNode { +function projectInline(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDInlineNode | null { if (pmNode.isText) { - return projectTextRun(pmNode); + return projectTextRun(pmNode, options); } switch (pmNode.type.name) { case 'run': - return projectRunNode(pmNode); + return projectRunNode(pmNode, options); case 'image': return projectInlineImage(pmNode); case 'tab': @@ -736,9 +754,9 @@ function projectInline(pmNode: ProseMirrorNode): SDInlineNode { case 'field': return projectInlineField(pmNode); case 'structuredContent': - return projectInlineSdt(pmNode); + return projectInlineSdt(pmNode, options); default: - return projectInlineFallback(pmNode); + return projectInlineFallback(pmNode, options); } } @@ -746,10 +764,11 @@ function projectInline(pmNode: ProseMirrorNode): SDInlineNode { // Run node (SuperDoc schema: paragraph → run → text) // --------------------------------------------------------------------------- -function projectRunNode(pmNode: ProseMirrorNode): SDRun | SDHyperlink { +function projectRunNode(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDRun | SDHyperlink | null { const attrs = (pmNode.attrs ?? {}) as Record; const runProperties = attrs.runProperties as Record | undefined; - const text = pmNode.textContent; + const text = isVisibleProjection(options) ? textContentInBlock(pmNode, options) : pmNode.textContent; + if (isVisibleProjection(options) && text.length === 0) return null; // Check for hyperlink wrapping via link mark on children let linkMark: ProseMirrorMark | undefined; @@ -760,7 +779,7 @@ function projectRunNode(pmNode: ProseMirrorNode): SDRun | SDHyperlink { }); if (linkMark) { - return buildHyperlinkFromRunNode(pmNode, linkMark); + return buildHyperlinkFromRunNode(pmNode, linkMark, options); } const run: SDRun = { kind: 'run', run: { text } }; @@ -779,11 +798,17 @@ function projectRunNode(pmNode: ProseMirrorNode): SDRun | SDHyperlink { return run; } -function buildHyperlinkFromRunNode(pmNode: ProseMirrorNode, linkMark: ProseMirrorMark): SDHyperlink { +function buildHyperlinkFromRunNode( + pmNode: ProseMirrorNode, + linkMark: ProseMirrorMark, + options?: ProjectionOptions, +): SDHyperlink | null { const attrs = linkMark.attrs as Record; + const text = isVisibleProjection(options) ? textContentInBlock(pmNode, options) : pmNode.textContent; + if (isVisibleProjection(options) && text.length === 0) return null; const childRun: SDRun = { kind: 'run', - run: { text: pmNode.textContent }, + run: { text }, }; const runProperties = (pmNode.attrs as Record)?.runProperties; @@ -918,7 +943,9 @@ function extractRunPropsFromRunProperties(runProperties: Record | u // Text run (bare PM text nodes — used in schemas without the run node wrapper) // --------------------------------------------------------------------------- -function projectTextRun(pmNode: ProseMirrorNode): SDRun | SDHyperlink { +function projectTextRun(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDRun | SDHyperlink | null { + if (isVisibleProjection(options) && hasTrackDeleteMark(pmNode)) return null; + const marks = pmNode.marks; // Check if wrapped in a link mark → hyperlink @@ -1025,10 +1052,12 @@ function projectInlineField(pmNode: ProseMirrorNode): SDField { }; } -function projectInlineFallback(pmNode: ProseMirrorNode): SDRun { +function projectInlineFallback(pmNode: ProseMirrorNode, options?: ProjectionOptions): SDRun | null { + const text = isVisibleProjection(options) ? textContentInBlock(pmNode, options) : (pmNode.textContent ?? '\ufffc'); + if (isVisibleProjection(options) && text.length === 0) return null; return { kind: 'run', - run: { text: pmNode.textContent ?? '\ufffc' }, + run: { text }, }; } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-target-resolver.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-target-resolver.ts index 0e08690c56..6b4b7e8fbd 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-target-resolver.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-target-resolver.ts @@ -54,10 +54,15 @@ function resolveTextPoint( }); } - const resolved = resolveTextRangeInBlock(candidate.node, candidate.pos, { - start: point.offset, - end: point.offset, - }); + const resolved = resolveTextRangeInBlock( + candidate.node, + candidate.pos, + { + start: point.offset, + end: point.offset, + }, + { textModel: 'visible' }, + ); if (!resolved) { throw new DocumentApiAdapterError( 'INVALID_TARGET', diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/text-offset-resolver.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/text-offset-resolver.test.ts index bf372bea9a..eb1374ccc8 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/text-offset-resolver.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/text-offset-resolver.test.ts @@ -1,8 +1,14 @@ import type { Node as ProseMirrorNode } from 'prosemirror-model'; -import { computeTextContentLength, pmPositionToTextOffset, resolveTextRangeInBlock } from './text-offset-resolver.js'; +import { + computeTextContentLength, + pmPositionToTextOffset, + resolveTextRangeInBlock, + textContentInBlock, +} from './text-offset-resolver.js'; type NodeOptions = { text?: string; + marks?: Array<{ type: { name: string } }>; isInline?: boolean; isBlock?: boolean; isLeaf?: boolean; @@ -31,6 +37,7 @@ function createNode(typeName: string, children: ProseMirrorNode[] = [], options: inlineContent, isTextblock: inlineContent, isLeaf, + marks: options.marks ?? [], childCount: children.length, child(index: number) { return children[index]!; @@ -120,6 +127,17 @@ describe('resolveTextRangeInBlock', () => { expect(result).toEqual({ from: 5, to: 6 }); }); + + it('maps visible offsets across tracked deleted text without counting it', () => { + const textA = createNode('text', [], { text: 'A' }); + const deleted = createNode('text', [], { text: 'gone', marks: [{ type: { name: 'trackDelete' } }] }); + const textB = createNode('text', [], { text: 'B' }); + const paragraph = createNode('paragraph', [textA, deleted, textB], { isBlock: true, inlineContent: true }); + + const result = resolveTextRangeInBlock(paragraph, 0, { start: 1, end: 2 }, { textModel: 'visible' }); + + expect(result).toEqual({ from: 6, to: 7 }); + }); }); describe('computeTextContentLength', () => { @@ -175,6 +193,17 @@ describe('computeTextContentLength', () => { expect(computeTextContentLength(paragraph)).toBe(2); }); + + it('excludes tracked deleted text in the visible text model', () => { + const textA = createNode('text', [], { text: 'A' }); + const deleted = createNode('text', [], { text: 'gone', marks: [{ type: { name: 'trackDelete' } }] }); + const textB = createNode('text', [], { text: 'B' }); + const paragraph = createNode('paragraph', [textA, deleted, textB], { isBlock: true, inlineContent: true }); + + expect(computeTextContentLength(paragraph)).toBe(6); + expect(computeTextContentLength(paragraph, { textModel: 'visible' })).toBe(2); + expect(textContentInBlock(paragraph, { textModel: 'visible' })).toBe('AB'); + }); }); describe('pmPositionToTextOffset', () => { @@ -228,4 +257,15 @@ describe('pmPositionToTextOffset', () => { // Past-end PM positions clamp to block length. expect(pmPositionToTextOffset(paragraph, 0, 1000)).toBe(2); }); + + it('keeps PM positions inside tracked deletions at the surrounding visible offset', () => { + const textA = createNode('text', [], { text: 'A' }); + const deleted = createNode('text', [], { text: 'gone', marks: [{ type: { name: 'trackDelete' } }] }); + const textB = createNode('text', [], { text: 'B' }); + const paragraph = createNode('paragraph', [textA, deleted, textB], { isBlock: true, inlineContent: true }); + + expect(pmPositionToTextOffset(paragraph, 0, 3, { textModel: 'visible' })).toBe(1); + expect(pmPositionToTextOffset(paragraph, 0, 6, { textModel: 'visible' })).toBe(1); + expect(pmPositionToTextOffset(paragraph, 0, 7, { textModel: 'visible' })).toBe(2); + }); }); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/text-offset-resolver.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/text-offset-resolver.ts index 6cbfbe942a..5395f05ce1 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/text-offset-resolver.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/text-offset-resolver.ts @@ -1,4 +1,5 @@ import type { Node as ProseMirrorNode } from 'prosemirror-model'; +import { TrackDeleteMarkName } from '../../extensions/track-changes/constants.js'; export type TextOffsetRange = { start: number; @@ -10,6 +11,24 @@ export type ResolvedTextRange = { to: number; }; +export type TextOffsetModel = 'raw' | 'visible'; + +export type TextOffsetOptions = { + textModel?: TextOffsetModel; +}; + +function isVisibleTextModel(options?: TextOffsetOptions): boolean { + return options?.textModel === 'visible'; +} + +function hasTrackDeleteMark(node: ProseMirrorNode): boolean { + return node.marks?.some((mark) => mark.type.name === TrackDeleteMarkName) ?? false; +} + +function shouldSkipTextNode(node: ProseMirrorNode, options?: TextOffsetOptions): boolean { + return isVisibleTextModel(options) && hasTrackDeleteMark(node); +} + function resolveSegmentPosition( targetOffset: number, segmentStart: number, @@ -35,7 +54,12 @@ function resolveSegmentPosition( * (`run`, etc.) or leaf atoms, because PM positions include wrapper * boundary tokens that the flattened model does not. */ -export function pmPositionToTextOffset(blockNode: ProseMirrorNode, blockPos: number, pmPos: number): number { +export function pmPositionToTextOffset( + blockNode: ProseMirrorNode, + blockPos: number, + pmPos: number, + options?: TextOffsetOptions, +): number { const contentStart = blockPos + 1; if (pmPos <= contentStart) return 0; @@ -48,6 +72,10 @@ export function pmPositionToTextOffset(blockNode: ProseMirrorNode, blockPos: num if (node.isText) { const text = node.text ?? ''; const endPos = docPos + text.length; + if (shouldSkipTextNode(node, options)) { + if (pmPos < endPos) done = true; + return; + } if (pmPos >= endPos) { offset += text.length; } else { @@ -103,11 +131,12 @@ export function pmPositionToTextOffset(blockNode: ProseMirrorNode, blockPos: num * offset model as {@link resolveTextRangeInBlock}: text contributes its * length, leaf atoms contribute 1, block separators contribute 1. */ -export function computeTextContentLength(blockNode: ProseMirrorNode): number { +export function computeTextContentLength(blockNode: ProseMirrorNode, options?: TextOffsetOptions): number { let length = 0; const walk = (node: ProseMirrorNode): void => { if (node.isText) { + if (shouldSkipTextNode(node, options)) return; length += (node.text ?? '').length; return; } @@ -149,6 +178,7 @@ export function resolveTextRangeInBlock( blockNode: ProseMirrorNode, blockPos: number, range: TextOffsetRange, + options?: TextOffsetOptions, ): ResolvedTextRange | null { if (range.start < 0 || range.end < range.start) return null; @@ -192,7 +222,7 @@ export function resolveTextRangeInBlock( const walkNode = (node: ProseMirrorNode, docPos: number) => { if (node.isText) { const text = node.text ?? ''; - if (text.length > 0) { + if (text.length > 0 && !shouldSkipTextNode(node, options)) { advanceSegment(text.length, docPos, docPos + text.length); } return; @@ -219,3 +249,39 @@ export function resolveTextRangeInBlock( if (fromPos == null || toPos == null) return null; return { from: fromPos, to: toPos }; } + +export function textContentInBlock(blockNode: ProseMirrorNode, options?: TextOffsetOptions): string { + let text = ''; + + const walkNode = (node: ProseMirrorNode): void => { + if (node.isText) { + if (!shouldSkipTextNode(node, options)) { + text += node.text ?? ''; + } + return; + } + + if (node.isLeaf) { + text += '\ufffc'; + return; + } + + let isFirstChild = true; + for (let i = 0; i < node.childCount; i++) { + const child = node.child(i); + if (child.isBlock && !isFirstChild) text += '\n'; + walkNode(child); + isFirstChild = false; + } + }; + + let isFirstChild = true; + for (let i = 0; i < blockNode.childCount; i++) { + const child = blockNode.child(i); + if (child.isBlock && !isFirstChild) text += '\n'; + walkNode(child); + isFirstChild = false; + } + + return text; +} diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/text-with-tabs.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/text-with-tabs.ts index c69f17e653..3a44a8582f 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/text-with-tabs.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/text-with-tabs.ts @@ -7,6 +7,7 @@ import type { Schema, } from 'prosemirror-model'; import type { Transaction } from 'prosemirror-state'; +import { TrackDeleteMarkName } from '../../extensions/track-changes/constants.js'; /** * Build a text-or-fragment suitable for insertion, splitting on '\t' and @@ -79,6 +80,7 @@ export function textBetweenWithTabs( to: number, blockSeparator: string, leafFallback: string, + options: { textModel?: 'raw' | 'visible' } = {}, ): string { // Defensive path for mocked docs: when `nodesBetween` isn't available, fall // back to the legacy `textBetween` semantics with no tab handling. Real PM @@ -108,6 +110,12 @@ export function textBetweenWithTabs( return false; } if (node.isText) { + if ( + options.textModel === 'visible' && + node.marks?.some((mark: ProseMirrorMark) => mark.type.name === TrackDeleteMarkName) + ) { + return false; + } const start = Math.max(from, pos) - pos; const end = Math.min(to, pos + node.nodeSize) - pos; // In real PM, node.text is always a string of length nodeSize. Some tests diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/compiler.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/compiler.ts index ba6f2c8005..a9f0ecbbb8 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/compiler.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/compiler.ts @@ -40,7 +40,12 @@ import { type BlockCandidate, type BlockIndex, } from '../helpers/node-address-resolver.js'; -import { resolveTextRangeInBlock } from '../helpers/text-offset-resolver.js'; +import { + resolveTextRangeInBlock, + textContentInBlock, + type TextOffsetModel, + type TextOffsetOptions, +} from '../helpers/text-offset-resolver.js'; import { resolveSelectionTarget, resolveSelectionPointPosition } from '../helpers/selection-target-resolver.js'; import { expandDeleteSelection } from '../helpers/expand-delete-selection.js'; @@ -238,6 +243,18 @@ interface ResolvedAddress { text: string; marks: readonly unknown[]; blockPos: number; + textModel: TextOffsetModel; +} + +export interface CompilePlanOptions { + /** + * Text model used only for `where.by = "select"` text selectors. + * + * Public discovery refs and explicit SelectionTargets stay visible-offset + * based. Tracked authoring selectors can opt into raw/review text so they + * can address unresolved deletion text without changing public read APIs. + */ + selectTextModel?: TextOffsetModel; } // --------------------------------------------------------------------------- @@ -250,8 +267,9 @@ function resolveAbsoluteRange( from: number, to: number, stepId: string, + options?: TextOffsetOptions, ): { absFrom: number; absTo: number } { - const resolved = resolveTextRangeInBlock(candidate.node, candidate.pos, { start: from, end: to }); + const resolved = resolveTextRangeInBlock(candidate.node, candidate.pos, { start: from, end: to }, options); if (!resolved) { throw planError('INVALID_INPUT', `text offset [${from}, ${to}) out of range in block`, stepId); } @@ -381,10 +399,11 @@ function buildRangeTarget( addr: ResolvedAddress, candidate: Pick, ): CompiledRangeTarget { - const abs = resolveAbsoluteRange(editor, candidate, addr.from, addr.to, step.id); + const textOffsetOptions = { textModel: addr.textModel }; + const abs = resolveAbsoluteRange(editor, candidate, addr.from, addr.to, step.id, textOffsetOptions); const capturedStyle = step.op === 'text.rewrite' || step.op === 'format.apply' - ? captureRunsInRange(editor, candidate.pos, addr.from, addr.to) + ? captureRunsInRange(editor, candidate.pos, addr.from, addr.to, textOffsetOptions) : undefined; return { @@ -412,6 +431,7 @@ function buildSpanTarget( step: MutationStep, segments: Array<{ blockId: string; from: number; to: number }>, matchId: string, + textModel: TextOffsetModel = 'visible', ): CompiledSpanTarget { // Validate segment ordering and contiguity in document order validateSegmentOrder(editor, index, segments, step.id); @@ -426,7 +446,8 @@ function buildSpanTarget( throw planError('INVALID_INPUT', `block "${seg.blockId}" not found for span segment`, step.id); } - const abs = resolveAbsoluteRange(editor, candidate, seg.from, seg.to, step.id); + const textOffsetOptions = { textModel }; + const abs = resolveAbsoluteRange(editor, candidate, seg.from, seg.to, step.id, textOffsetOptions); compiledSegments.push({ blockId: seg.blockId, from: seg.from, @@ -435,11 +456,11 @@ function buildSpanTarget( absTo: abs.absTo, }); - const blockText = getBlockText(editor, candidate); + const blockText = getBlockText(editor, candidate, textOffsetOptions); textParts.push(blockText.slice(seg.from, seg.to)); if (step.op === 'text.rewrite' || step.op === 'format.apply') { - capturedStyles.push(captureRunsInRange(editor, candidate.pos, seg.from, seg.to)); + capturedStyles.push(captureRunsInRange(editor, candidate.pos, seg.from, seg.to, textOffsetOptions)); } } @@ -516,15 +537,17 @@ function resolveTextSelector( selector: TextSelector | NodeSelector, within: import('@superdoc/document-api').BlockNodeAddress | undefined, stepId: string, - options?: { allBlockTypes?: boolean }, + options?: { allBlockTypes?: boolean; textModel?: TextOffsetModel }, ): { addresses: ResolvedAddress[] } { + const textModel = options?.textModel ?? 'visible'; + const textOffsetOptions = { textModel }; if (selector.type === 'text') { const query = { select: selector, within: within as import('@superdoc/document-api').BlockNodeAddress | undefined, includeNodes: false, }; - const result = executeTextSelector(editor, index, query, []); + const result = executeTextSelector(editor, index, query, [], { searchModel: textModel }); const addresses: ResolvedAddress[] = []; @@ -536,9 +559,9 @@ function resolveTextSelector( const candidate = index.candidates.find((c) => c.nodeId === coalesced.blockId); if (!candidate) continue; - const blockText = getBlockText(editor, candidate); + const blockText = getBlockText(editor, candidate, textOffsetOptions); const matchText = blockText.slice(coalesced.from, coalesced.to); - const captured = captureRunsInRange(editor, candidate.pos, coalesced.from, coalesced.to); + const captured = captureRunsInRange(editor, candidate.pos, coalesced.from, coalesced.to, textOffsetOptions); addresses.push({ blockId: coalesced.blockId, @@ -547,6 +570,7 @@ function resolveTextSelector( text: matchText, marks: captured.runs.length > 0 ? captured.runs[0].marks : [], blockPos: candidate.pos, + textModel, }); } } @@ -573,7 +597,7 @@ function resolveTextSelector( if (!candidate) continue; if (isTextBlockCandidate(candidate)) { - const blockText = getBlockText(editor, candidate); + const blockText = getBlockText(editor, candidate, textOffsetOptions); addresses.push({ blockId: match.nodeId, from: 0, @@ -581,6 +605,7 @@ function resolveTextSelector( text: blockText, marks: [], blockPos: candidate.pos, + textModel, }); } else { // Non-text block (table, image wrapper, etc.): no text offsets needed. @@ -592,6 +617,7 @@ function resolveTextSelector( text: '', marks: [], blockPos: candidate.pos, + textModel, }); } } @@ -599,7 +625,14 @@ function resolveTextSelector( return { addresses }; } -function getBlockText(editor: Editor, candidate: { pos: number; end: number }): string { +function getBlockText( + editor: Editor, + candidate: { node?: BlockCandidate['node']; pos: number; end: number }, + options: TextOffsetOptions = { textModel: 'visible' }, +): string { + if (candidate.node && candidate.node.childCount > 0) { + return textContentInBlock(candidate.node, options); + } const blockStart = candidate.pos + 1; const blockEnd = candidate.end - 1; return editor.state.doc.textBetween(blockStart, blockEnd, '\n', '\ufffc'); @@ -655,6 +688,7 @@ function resolveV3TextRef(editor: Editor, index: BlockIndex, step: MutationStep, text: matchText, marks: [], blockPos: candidate.pos, + textModel: 'visible', }; const target = buildRangeTarget(editor, step, addr, candidate); @@ -663,7 +697,7 @@ function resolveV3TextRef(editor: Editor, index: BlockIndex, step: MutationStep, } // Multi-segment match refs → span target - return [buildSpanTarget(editor, index, step, segments, refData.matchId)]; + return [buildSpanTarget(editor, index, step, segments, refData.matchId, 'visible')]; } /** @@ -728,6 +762,7 @@ function resolveV4TextRef( text: matchText, marks: [], blockPos: candidate.pos, + textModel: 'visible', }; const target = buildRangeTarget(editor, step, addr, candidate); @@ -736,7 +771,7 @@ function resolveV4TextRef( } // Multi-segment → span target - return [buildSpanTarget(editor, index, step, segments, refData.matchId ?? `v4:${step.id}`)]; + return [buildSpanTarget(editor, index, step, segments, refData.matchId ?? `v4:${step.id}`, 'visible')]; } function resolveTextRef(editor: Editor, index: BlockIndex, step: MutationStep, ref: string): CompiledTarget[] { @@ -778,6 +813,7 @@ function resolveBlockRef(editor: Editor, index: BlockIndex, step: MutationStep, text: blockText, marks: [], blockPos: candidate.pos, + textModel: 'visible', }; return [buildRangeTarget(editor, step, addr, candidate)]; @@ -902,6 +938,7 @@ function buildWholeBlockRangeTarget( text: blockText, marks: [], blockPos: candidate.pos, + textModel: 'visible', }; return buildRangeTarget(editor, step, addr, candidate); } @@ -1010,7 +1047,12 @@ function captureStyleAtAbsoluteRange(editor: Editor, absFrom: number, absTo: num // Step target resolution // --------------------------------------------------------------------------- -function resolveStepTargets(editor: Editor, index: BlockIndex, step: MutationStep): CompiledTarget[] { +function resolveStepTargets( + editor: Editor, + index: BlockIndex, + step: MutationStep, + options: CompilePlanOptions = {}, +): CompiledTarget[] { const where = step.where; const refWhere = isRefWhere(where) ? where : undefined; const selectWhere = isSelectWhere(where) ? where : undefined; @@ -1030,6 +1072,7 @@ function resolveStepTargets(editor: Editor, index: BlockIndex, step: MutationSte const isStructuralOp = step.op === 'structural.insert' || step.op === 'structural.replace'; const resolved = resolveTextSelector(editor, index, selectWhere.select, selectWhere.within, step.id, { allBlockTypes: isStructuralOp, + textModel: options.selectTextModel ?? 'visible', }); targets = resolved.addresses.map((addr) => { const candidate = index.candidates.find((c) => c.nodeId === addr.blockId); @@ -1518,7 +1561,7 @@ function assertSingleStoryKey(steps: MutationStep[]): void { } } -export function compilePlan(editor: Editor, steps: MutationStep[]): CompiledPlan { +export function compilePlan(editor: Editor, steps: MutationStep[], options: CompilePlanOptions = {}): CompiledPlan { // D8: plan step limit if (steps.length > MAX_PLAN_STEPS) { throw planError('INVALID_INPUT', `plan contains ${steps.length} steps, maximum is ${MAX_PLAN_STEPS}`); @@ -1576,7 +1619,7 @@ export function compilePlan(editor: Editor, steps: MutationStep[]): CompiledPlan validateCreateStepPosition(step); } - const targets = resolveStepTargets(editor, index, step); + const targets = resolveStepTargets(editor, index, step, options); // Validate insertion context for create ops (B0 invariant 5) if (isCreateOp(step.op) && targets.length > 0) { diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/executor.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/executor.ts index 5a2330695a..20d0439a40 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/executor.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/executor.ts @@ -2380,7 +2380,9 @@ export function executePlan(editor: Editor, input: MutationsApplyInput): PlanRec throw planError('INVALID_INPUT', 'plan must contain at least one step'); } - const compiled = compilePlan(editor, input.steps); + const compiled = compilePlan(editor, input.steps, { + selectTextModel: input.changeMode === 'tracked' ? 'raw' : 'visible', + }); return executeCompiledPlan(editor, compiled, { changeMode: input.changeMode ?? 'direct', diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/query-match-adapter.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/query-match-adapter.ts index 8dec1af097..eb0742e9b9 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/query-match-adapter.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/query-match-adapter.ts @@ -26,6 +26,7 @@ import type { PageInfo, StoryLocator, } from '@superdoc/document-api'; +import type { Node as ProseMirrorNode } from 'prosemirror-model'; import { SNIPPET_MAX_LENGTH, SNIPPET_CONTEXT_CHARS, @@ -50,6 +51,7 @@ import type { OoxmlResolverParams, ParagraphProperties } from '@superdoc/style-e import { readTranslatedLinkedStyles } from '../../core/parts/adapters/styles-read.js'; import { resolveStoryRuntime } from '../story-runtime/resolve-story-runtime.js'; import { encodeV4Ref } from '../story-runtime/story-ref-codec.js'; +import { textContentInBlock } from '../helpers/text-offset-resolver.js'; // --------------------------------------------------------------------------- // V3 ref encoding (D6) @@ -142,6 +144,14 @@ export function buildSelectionTargetFromTextRanges(textRanges: TextAddress[], st return target; } +function readCandidateVisibleText(editor: Editor, candidate: { node?: unknown; pos: number; end: number }): string { + const maybeNode = candidate.node as { childCount?: number } | undefined; + if (maybeNode && typeof maybeNode.childCount === 'number' && maybeNode.childCount > 0) { + return textContentInBlock(maybeNode as ProseMirrorNode, { textModel: 'visible' }); + } + return editor.state.doc.textBetween(candidate.pos + 1, candidate.end - 1, '\n', '\ufffc'); +} + // --------------------------------------------------------------------------- // Block/run builders (D4, D5) // --------------------------------------------------------------------------- @@ -224,9 +234,7 @@ function buildMatchBlocks( } // Get block-level metadata - const blockStart = candidate.pos + 1; - const blockEnd = candidate.end - 1; - const blockText = doc.textBetween(blockStart, blockEnd, '\n', '\ufffc'); + const blockText = readCandidateVisibleText(editor, candidate); const matchedText = blockText.slice(from, to); const node = doc.nodeAt(candidate.pos); const nodeType = node?.type.name ?? 'paragraph'; @@ -243,7 +251,7 @@ function buildMatchBlocks( : undefined; // Capture PM runs within the matched range and coalesce (D4) - const captured = captureRunsInRange(editor, candidate.pos, from, to); + const captured = captureRunsInRange(editor, candidate.pos, from, to, { textModel: 'visible' }); const coalesced = coalesceRuns(captured.runs); // Project to contract MatchRun[] with V4 refs @@ -337,7 +345,6 @@ function buildBlocksSnippet( if (!editor.state?.doc || blocks.length === 0) return undefined; const index = getBlockIndex(editor); - const doc = editor.state.doc; // D11 step 1: join block match texts const matchText = blocks.map((b) => b.text).join('\n'); @@ -359,9 +366,7 @@ function buildBlocksSnippet( const firstBlock = blocks[0]; const firstCandidate = index.candidates.find((c) => c.nodeId === firstBlock.blockId); if (firstCandidate) { - const blockStart = firstCandidate.pos + 1; - const blockEnd = firstCandidate.end - 1; - const fullBlockText = doc.textBetween(blockStart, blockEnd, '\n', '\ufffc'); + const fullBlockText = readCandidateVisibleText(editor, firstCandidate); const contextStart = Math.max(0, firstBlock.range.start - contextEachSide); leftContext = fullBlockText.slice(contextStart, firstBlock.range.start); } @@ -371,9 +376,7 @@ function buildBlocksSnippet( const lastBlock = blocks[blocks.length - 1]; const lastCandidate = index.candidates.find((c) => c.nodeId === lastBlock.blockId); if (lastCandidate) { - const blockStart = lastCandidate.pos + 1; - const blockEnd = lastCandidate.end - 1; - const fullBlockText = doc.textBetween(blockStart, blockEnd, '\n', '\ufffc'); + const fullBlockText = readCandidateVisibleText(editor, lastCandidate); const contextEnd = Math.min(fullBlockText.length, lastBlock.range.end + contextEachSide); rightContext = fullBlockText.slice(lastBlock.range.end, contextEnd); } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/style-resolver.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/style-resolver.test.ts index 872089ef53..808e7b39a6 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/style-resolver.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/style-resolver.test.ts @@ -168,6 +168,29 @@ describe('captureRunsInRange', () => { expect(result.runs[0].marks.map((m) => m.type.name)).toEqual(['bold']); }); + it('excludes tracked deleted text from visible run capture without leaving offset gaps', () => { + const bold = mockMark('bold'); + const trackDelete = mockMark('trackDelete'); + + const paragraph = createNode( + 'paragraph', + [ + createNode('text', [], { text: 'A', marks: [bold] }), + createNode('text', [], { text: 'gone', marks: [trackDelete] }), + createNode('text', [], { text: 'B', marks: [bold] }), + ], + { isBlock: true, inlineContent: true }, + ); + const editor = makeEditor(0, paragraph); + + const result = captureRunsInRange(editor, 0, 0, 2, { textModel: 'visible' }); + + expect(result.runs).toHaveLength(2); + expect(result.runs[0]).toMatchObject({ from: 0, to: 1, charCount: 1 }); + expect(result.runs[1]).toMatchObject({ from: 1, to: 2, charCount: 1 }); + expect(coalesceRuns(result.runs)).toHaveLength(1); + }); + it('returns empty runs when the block node cannot be resolved', () => { const editor = makeEditor(0, null); const result = captureRunsInRange(editor, 0, 0, 5); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/style-resolver.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/style-resolver.ts index 75520cf494..e13d08e02f 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/style-resolver.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/style-resolver.ts @@ -10,6 +10,8 @@ import { MARK_KEYS } from '@superdoc/document-api'; import type { Editor } from '../../core/Editor.js'; import { planError } from './errors.js'; import { TOGGLE_MARK_SPECS, applyDirectiveToMarks } from './mark-directives.js'; +import type { TextOffsetOptions } from '../helpers/text-offset-resolver.js'; +import { TrackDeleteMarkName } from '../../extensions/track-changes/constants.js'; // --------------------------------------------------------------------------- // Run types — describes contiguous spans sharing identical marks within a block @@ -71,7 +73,13 @@ const METADATA_MARK_NAMES = new Set([ * to the block-relative `from`/`to` offsets, collecting each inline text node * as a run with its marks. */ -export function captureRunsInRange(editor: Editor, blockPos: number, from: number, to: number): CapturedStyle { +export function captureRunsInRange( + editor: Editor, + blockPos: number, + from: number, + to: number, + options?: TextOffsetOptions, +): CapturedStyle { const doc = editor.state.doc; const blockNode = doc.nodeAt(blockPos); if (!blockNode || from < 0 || to < from || from === to) { @@ -98,11 +106,14 @@ export function captureRunsInRange(editor: Editor, blockPos: number, from: numbe if (node.isText) { const text = node.text ?? ''; if (text.length > 0) { - const start = offset; - const end = offset + text.length; const marks = Array.isArray((node as { marks?: unknown }).marks) ? ((node as unknown as { marks: PmMark[] }).marks as readonly PmMark[]) : []; + if (options?.textModel === 'visible' && marks.some((mark) => mark.type.name === TrackDeleteMarkName)) { + return; + } + const start = offset; + const end = offset + text.length; maybePushRun(start, end, marks); offset = end; } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/tracked-rewrite.integration.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/tracked-rewrite.integration.test.ts index 6f1aadbb5f..ebcfa6c06f 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/tracked-rewrite.integration.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/tracked-rewrite.integration.test.ts @@ -48,6 +48,41 @@ function paragraphTexts(editor: any): string[] { return paragraphs; } +function findTextRange(editor: any, text: string): { from: number; to: number } { + let range: { from: number; to: number } | null = null; + editor.state.doc.descendants((node: any, pos: number) => { + if (!node.isText || !node.text) return true; + const index = node.text.indexOf(text); + if (index === -1) return true; + range = { from: pos + index, to: pos + index + text.length }; + return false; + }); + if (!range) throw new Error(`Could not find text "${text}"`); + return range; +} + +function markTextAsOtherUserDeletion(editor: any, text: string): void { + const range = findTextRange(editor, text); + const mark = editor.schema.marks[TrackDeleteMarkName].create({ + id: 'alice-delete', + author: 'Alice Reviewer', + authorEmail: 'alice@example.com', + date: '2024-01-01T00:00:00.000Z', + }); + editor.dispatch(editor.state.tr.addMark(range.from, range.to, mark)); +} + +function markedTextByAuthor(editor: any, markName: string, authorEmail: string): string { + const parts: string[] = []; + editor.state.doc.descendants((node: any) => { + if (!node.isText || !node.text) return; + if (node.marks.some((mark: any) => mark.type.name === markName && mark.attrs.authorEmail === authorEmail)) { + parts.push(node.text); + } + }); + return parts.join(''); +} + function compileSingleRewrite(editor: any, pattern: string, text: string) { const step = { id: 'rewrite-step', @@ -221,6 +256,62 @@ describe('doc.replace multi-paragraph integration', () => { expect(insertedTexts).toEqual(expect.arrayContaining(['Alpha', 'Beta'])); }); + it('resolves tracked rewrite selectors against unresolved deletion text without changing public query refs', () => { + editor = makeEditor(['The quick brown fox jumps over the lazy dog.']); + markTextAsOtherUserDeletion(editor, 'lazy '); + + const receipt = editor.doc.mutations.apply({ + atomic: true, + changeMode: 'tracked', + steps: [ + { + id: 'replace-inside-delete', + op: 'text.rewrite', + where: { + by: 'select', + select: { type: 'text', pattern: 'lazy' }, + require: 'first', + }, + args: { + replacement: { text: 'OO' }, + style: { inline: { mode: 'preserve' } }, + }, + }, + ], + }); + + expect(receipt.success).toBe(true); + expect(markedTextByAuthor(editor, TrackInsertMarkName, 'integration@example.com')).toContain('OO'); + expect(markedTextByAuthor(editor, TrackDeleteMarkName, 'integration@example.com')).toContain('lazy'); + expect(markedTextByAuthor(editor, TrackDeleteMarkName, 'alice@example.com')).toBe(' '); + }); + + it('resolves tracked delete selectors against unresolved deletion text as a child deletion', () => { + editor = makeEditor(['The quick brown fox jumps over the lazy dog.']); + markTextAsOtherUserDeletion(editor, 'lazy '); + + const receipt = editor.doc.mutations.apply({ + atomic: true, + changeMode: 'tracked', + steps: [ + { + id: 'delete-inside-delete', + op: 'text.delete', + where: { + by: 'select', + select: { type: 'text', pattern: 'lazy' }, + require: 'first', + }, + args: { behavior: 'exact' }, + }, + ], + }); + + expect(receipt.success).toBe(true); + expect(markedTextByAuthor(editor, TrackDeleteMarkName, 'integration@example.com')).toContain('lazy'); + expect(markedTextByAuthor(editor, TrackDeleteMarkName, 'alice@example.com')).toBe(' '); + }); + it('creates an empty paragraph before the replacement when text has a leading newline (direct)', () => { editor = makeEditor(); const receipt = editor.doc.replace( diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.js b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.js index e7e263187b..fd855d459f 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.js @@ -103,6 +103,10 @@ import { getLiveInlineMarksInRange } from '../trackChangesHelpers/getLiveInlineM const SUPPORTED_KINDS = new Set(['text-insert', 'text-delete', 'text-replace', 'format-apply', 'format-remove']); const EMPTY_STRUCTURAL_GAP_REFINEMENT_MAX_DISTANCE = 4; +/** + * @typedef {false|'different-user'|'all'} ExistingDeletionReassignMode + */ + /** * Compile a tracked edit against an accumulated transaction. * @@ -544,7 +548,8 @@ const compileTextDelete = (ctx, intent) => { replacementSideId: '', sharedDeletionId: intent.replacementGroupHint || null, recordSharedDeletionId: Boolean(intent.replacementGroupHint), - reassignExistingDeletions: intent.source !== 'native' && !intent.preserveExistingReviewState, + reassignExistingDeletions: + intent.source !== 'native' && !intent.preserveExistingReviewState ? 'different-user' : false, }); if (result.ok === false) return result; @@ -571,7 +576,7 @@ const compileTextDelete = (ctx, intent) => { * @param {*} ctx * @param {number} from * @param {number} to - * @param {{ replacementGroupId: string, replacementSideId: string, sharedDeletionId: string | null, recordSharedDeletionId?: boolean, recordCollapsedIds?: boolean, reassignExistingDeletions?: boolean }} options + * @param {{ replacementGroupId: string, replacementSideId: string, sharedDeletionId: string | null, recordSharedDeletionId?: boolean, recordCollapsedIds?: boolean, reassignExistingDeletions?: ExistingDeletionReassignMode }} options * @returns {{ ok: true, deletionMarks: import('prosemirror-model').Mark[], deletionNodes: import('prosemirror-model').Node[], deletionId: string, mintedThisCall: boolean } | TrackedEditFailure} */ const applyTrackedDelete = ( @@ -653,7 +658,10 @@ const applyTrackedDelete = ( change: getChangeAuthorIdentity(existingDelete.attrs), }); const isDifferentUserDeletion = !isSameUserHighConfidence(deleteOwnership); - if (reassignExistingDeletions && isDifferentUserDeletion) { + const shouldReassignExistingDeletion = + reassignExistingDeletions === 'all' || + (reassignExistingDeletions === 'different-user' && isDifferentUserDeletion); + if (shouldReassignExistingDeletion) { ops.push({ kind: 'reassign', from: segFrom, @@ -1018,7 +1026,7 @@ const compileOrdinaryTextReplace = (ctx, intent, sanitizedSlice, replacementPare replacementGroupId, replacementSideId: sharedId ? `${sharedId}#deleted` : '', sharedDeletionId: sharedId, - reassignExistingDeletions: Boolean(sharedId) || Boolean(replacementParentId), + reassignExistingDeletions: sharedId || replacementParentId ? 'all' : false, }); if (delResult.ok === false) return delResult; deletionMarks = delResult.deletionMarks; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.test.js index d10273538e..d56d6a1008 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.test.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/overlap-compiler.test.js @@ -688,7 +688,7 @@ describe('overlap-compiler: text-delete', () => { }); describe('overlap-compiler: text-replace inside named no-email insertion', () => { - it('preserves parent insertion and creates one child replacement', () => { + it('preserves parent insertion and creates child insertion/deletion sides', () => { const parentId = 'ins-alice-no-email'; const { state } = stateFromTrackedSpans({ schema, @@ -718,20 +718,24 @@ describe('overlap-compiler: text-replace inside named no-email insertion', () => }); const result = runCompile({ state, intent }); expect(result.ok).toBe(true); - expect(textOf(result.tr)).toBe('lazyquickly '); + expect(textOf(result.tr)).toBe('quicklylazy '); const graph = buildReviewGraph({ state: { doc: result.tr.doc } }); - expect(graph.changes.size).toBe(2); + expect(graph.changes.size).toBe(3); const parent = graph.changes.get(parentId); expect(parent).toBeDefined(); expect(parent.type).toBe(CanonicalChangeType.Insertion); - const childReplacement = Array.from(graph.changes.values()).find((change) => change.id !== parentId); - expect(childReplacement).toBeDefined(); - expect(childReplacement.type).toBe(CanonicalChangeType.Replacement); - expect(childReplacement.deletedSegments[0].text).toBe('lazy'); - expect(childReplacement.deletedSegments[0].attrs.overlapParentId).toBe(parentId); - expect(childReplacement.insertedSegments.map((segment) => segment.text).join('')).toBe('quickly'); - expect(childReplacement.insertedSegments[0].attrs.overlapParentId).toBe(parentId); + + const children = Array.from(graph.changes.values()).filter((change) => change.parent === parentId); + expect(children).toHaveLength(2); + const childDeletion = children.find((change) => change.type === CanonicalChangeType.Deletion); + const childInsertion = children.find((change) => change.type === CanonicalChangeType.Insertion); + expect(childDeletion).toBeDefined(); + expect(childInsertion).toBeDefined(); + expect(childDeletion.deletedSegments.map((segment) => segment.text).join('')).toBe('lazy'); + expect(childDeletion.deletedSegments.every((segment) => segment.attrs.overlapParentId === parentId)).toBe(true); + expect(childInsertion.insertedSegments.map((segment) => segment.text).join('')).toBe('quickly'); + expect(childInsertion.insertedSegments.every((segment) => segment.attrs.overlapParentId === parentId)).toBe(true); }); }); @@ -810,7 +814,7 @@ describe('overlap-compiler: text-replace produces paired replacement metadata', expect(change.replacement?.deleted.length).toBeGreaterThan(0); }); - it('keeps both sides of a child replacement under an other-user insertion parent', () => { + it('keeps child insertion and deletion sides under an other-user insertion parent', () => { const parentId = 'ins-bob'; const { state } = stateFromTrackedSpans({ schema, @@ -832,12 +836,18 @@ describe('overlap-compiler: text-replace produces paired replacement metadata', expect(result.ok).toBe(true); const graph = buildReviewGraph({ state: { doc: result.tr.doc }, replacementsMode: 'paired' }); const children = Array.from(graph.changes.values()).filter((change) => change.parent === parentId); - expect(children).toHaveLength(1); - const childReplacement = children[0]; - expect(childReplacement.type).toBe(CanonicalChangeType.Replacement); - expect(childReplacement.deletedSegments.map((segment) => segment.text).join('')).toBe('or'); - expect(childReplacement.insertedSegments.map((segment) => segment.text).join('')).toBe('AR'); - expect(childReplacement.segments.every((segment) => segment.attrs.overlapParentId === parentId)).toBe(true); + expect(children).toHaveLength(2); + const childDeletion = children.find((change) => change.type === CanonicalChangeType.Deletion); + const childInsertion = children.find((change) => change.type === CanonicalChangeType.Insertion); + expect(childDeletion).toBeDefined(); + expect(childInsertion).toBeDefined(); + expect(childDeletion.deletedSegments.map((segment) => segment.text).join('')).toBe('or'); + expect(childInsertion.insertedSegments.map((segment) => segment.text).join('')).toBe('AR'); + expect( + [...childDeletion.segments, ...childInsertion.segments].every( + (segment) => segment.attrs.overlapParentId === parentId, + ), + ).toBe(true); }); it('honors a provided logical id hint for paired document-api replacements', () => { From f6daa9df982c9e901f8b3154b3f1e41382b9f5f5 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Fri, 29 May 2026 19:58:04 -0700 Subject: [PATCH 5/5] chore: behavior fix --- ...er-multi-paragraph-tracked-changes.spec.ts | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/behavior/tests/comments/replace-over-multi-paragraph-tracked-changes.spec.ts b/tests/behavior/tests/comments/replace-over-multi-paragraph-tracked-changes.spec.ts index 20cdf7b6ee..d68e3496fd 100644 --- a/tests/behavior/tests/comments/replace-over-multi-paragraph-tracked-changes.spec.ts +++ b/tests/behavior/tests/comments/replace-over-multi-paragraph-tracked-changes.spec.ts @@ -130,9 +130,23 @@ test('replace over multi-paragraph tracked changes stays coherent', async ({ sup await superdoc.press('Backspace'); await superdoc.waitForStable(); - // Both words should still exist in PM (as tracked deletions, not truly removed) - await superdoc.assertTextContains('tailword2'); - await superdoc.assertTextContains('tailword3'); + // Public text is visible/effective text, so unresolved deletions are hidden. + await superdoc.assertTextNotContains('tailword2'); + await superdoc.assertTextNotContains('tailword3'); + + // Both words should still exist in PM as tracked deletions, not truly removed. + const deletedTextAfterStep2 = await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + let text = ''; + editor.state.doc.descendants((node: any) => { + if (!node.isText || !node.text) return; + const hasDeleteMark = (node.marks ?? []).some((mark: any) => mark.type?.name === 'trackDelete'); + if (hasDeleteMark) text += node.text; + }); + return text; + }); + expect(deletedTextAfterStep2).toContain('tailword2'); + expect(deletedTextAfterStep2).toContain('tailword3'); // Tracked delete marks should exist const deletionCountAfterStep2 = await superdoc.page.evaluate(() => {