fix: nested replacement tracked-change decisions (#3557)

Author: harbournick

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<string, string>} 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,29 +127,40 @@ 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.
     if (!idMap.has(wordId)) {
       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
     // UUID here would overwrite the earlier entry and break any replacement
     // 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;
 }
 

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,
     };
   }

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<string>([
   '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<string, unknown> | 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',

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<TextOffsetModel, 'raw' | 'visible'>;
+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,12 +106,15 @@ 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: 'visible',
+    searchModel,
   });
 
   if (!Array.isArray(rawResult)) {
@@ -114,9 +123,9 @@ export function executeTextSelector(
       'Editor search command returned an unexpected result format.',
     );
   }
-  const allMatches = rawResult as SearchMatch[];
 
   const scopeRange = scope.range;
+  const allMatches = rawResult as SearchMatch[];
   const matches = scopeRange
     ? allMatches.filter((m) => m.from >= scopeRange.start && m.to <= scopeRange.end)
     : allMatches;
@@ -133,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));
 
@@ -144,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);

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' });
 }

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 {