From 113be1e68d9c8d794d672c51cc072fcccb53f864 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 07:18:57 +0000 Subject: [PATCH 1/2] fix(replace): cut untracked spans in place across nested wrappers (#16) An untracked replace whose match crossed a revision wrapper split it into two halves that both kept the wrapper's w:id, put the replacement between them as plain original text, and never descended a hyperlink nested inside, so the link's part of the match survived and the text scrambled. Untracked, the cross-boundary walker now recurses into an additive / and cuts it in place (nested hyperlinks/fields/revisions sliced at the right offsets, replacement where the match starts, no id duplicated); a subtractive wrapper stays whole. Tracked, the post-half of a split wrapper gets a fresh id from the allocator. rebuildContainer now starts at the container's own offset and counts nested wrappers, so a match in a later run of a multi-run ins or hyperlink cuts the right words. Triage-Routine: true Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019XWuv9YGyEx2uZMPbg5Egd --- src/core/find/replace-span.tsx | 286 ++++++++++++++++++------------- src/core/track-changes/CLAUDE.md | 4 +- tests/cli/replace.test.ts | 171 +++++++++++++++++- 3 files changed, 338 insertions(+), 123 deletions(-) diff --git a/src/core/find/replace-span.tsx b/src/core/find/replace-span.tsx index 0af52ad..0bbe95b 100644 --- a/src/core/find/replace-span.tsx +++ b/src/core/find/replace-span.tsx @@ -137,11 +137,9 @@ export function replaceSpanInParagraph( } if (allSameParent) { - const containerStart = - firstParent === paragraph ? 0 : firstSlot.offsetBefore; rebuildContainer( firstParent, - containerStart, + firstSlot.parentStart, span, replacement, inheritedProperties, @@ -166,6 +164,8 @@ export function replaceSpanInParagraph( type RunSlot = { parent: XmlNode; + /** Offset at which `parent`'s content starts (0 for the paragraph). */ + parentStart: number; run: XmlNode; offsetBefore: number; length: number; @@ -175,11 +175,13 @@ function collectRunSlots(paragraph: XmlNode, view: FindView): RunSlot[] { const slots: RunSlot[] = []; let offset = 0; function walk(parent: XmlNode, children: XmlNode[]): void { + const parentStart = offset; for (const child of children) { if (child.tag === "w:r") { const length = runTextLength(child); slots.push({ parent, + parentStart, run: child, offsetBefore: offset, length, @@ -256,7 +258,7 @@ function rebuildContainer( } continue; } - if (isParagraph && isWrapperVisibleInView(child.tag, view)) { + if (isWrapperVisibleInView(child.tag, view)) { offset += sumVisibleTextLength(wrapperContent(child), view); newChildren.push(child); continue; @@ -277,22 +279,59 @@ function rebuildAcrossBoundaries( view: FindView, formatting?: ReplacementFormatting, ): void { + const across: AcrossSpan = { + span, + replacement, + runProperties, + tracked, + view, + formatting, + placed: false, + }; + paragraph.children = rebuildChildrenAcrossSpan( + paragraph.children, + 0, + across, + true, + ); + if (!across.placed) { + paragraph.children.push(...placedReplacement(across, true)); + } +} + +/** One cross-boundary replace's shared state. `placed` is shared across the + * recursion so the replacement lands exactly once, in the container that + * holds the span's first character. */ +type AcrossSpan = { + span: Span; + replacement: string; + runProperties: XmlNode | null; + tracked: TrackedReplaceOptions | null; + view: FindView; + formatting: ReplacementFormatting | undefined; + placed: boolean; +}; + +/** The span removed from `children` (which start at offset `baseOffset`), + * with the replacement placed where the span starts. `isParagraph` gates the + * tracked replacement's own ``: only a paragraph-level replacement is + * wrapped (the recursion into a wrapper's content runs untracked only). */ +function rebuildChildrenAcrossSpan( + children: XmlNode[], + baseOffset: number, + across: AcrossSpan, + isParagraph: boolean, +): XmlNode[] { + const { span, tracked, view } = across; const newChildren: XmlNode[] = []; - let offset = 0; - let placed = false; + let offset = baseOffset; const placeReplacement = (): void => { - if (placed) return; - placed = true; - const runs = replacementRuns(runProperties, replacement, formatting); - if (tracked) { - newChildren.push({runs}); - return; - } - newChildren.push(...runs); + if (across.placed) return; + newChildren.push(...placedReplacement(across, isParagraph)); }; - for (const child of paragraph.children) { + for (const child of children) { if (child.tag === "w:r") { const length = runTextLength(child); const runStart = offset; @@ -326,41 +365,40 @@ function rebuildAcrossBoundaries( continue; } - // Tracked-change wrappers invisible in the chosen view pass through - // untouched — their inner text contributes nothing to the offset and - // the span never slices into them. - if ( - (child.tag === "w:ins" || - child.tag === "w:del" || - child.tag === "w:moveFrom" || - child.tag === "w:moveTo") && - !isWrapperVisibleInView(child.tag, view) - ) { + // Wrappers invisible in the chosen view (and any non-wrapper child) + // pass through untouched — their inner text contributes nothing to the + // offset and the span never slices into them. + if (!isWrapperVisibleInView(child.tag, view)) { newChildren.push(child); continue; } - if ( - child.tag === "w:ins" || - child.tag === "w:del" || - child.tag === "w:moveFrom" || - child.tag === "w:moveTo" - ) { - const innerLength = sumVisibleTextLength(wrapperContent(child), view); - const wrapperStart = offset; - const wrapperEnd = offset + innerLength; - offset = wrapperEnd; - - if (wrapperEnd <= span.start) { - newChildren.push(child); - continue; - } - if (wrapperStart >= span.end) { - placeReplacement(); - newChildren.push(child); + const innerLength = sumVisibleTextLength(wrapperContent(child), view); + const wrapperStart = offset; + const wrapperEnd = offset + innerLength; + offset = wrapperEnd; + + if (wrapperEnd <= span.start) { + newChildren.push(child); + continue; + } + if (wrapperStart >= span.end) { + placeReplacement(); + newChildren.push(child); + continue; + } + + if (isTrackedChangeWrapper(child.tag)) { + if (!tracked) { + cutRevisionInPlace( + child, + wrapperStart, + across, + newChildren, + placeReplacement, + ); continue; } - splitWrapperAcrossSpan( child, wrapperStart, @@ -373,35 +411,12 @@ function rebuildAcrossBoundaries( } if (child.tag === "w:hyperlink") { - const innerLength = sumVisibleTextLength(wrapperContent(child), view); - const wrapperStart = offset; - const wrapperEnd = offset + innerLength; - offset = wrapperEnd; - - if (wrapperEnd <= span.start) { - newChildren.push(child); - continue; - } - if (wrapperStart >= span.end) { - placeReplacement(); - newChildren.push(child); - continue; - } - splitHyperlinkAcrossSpan( child, wrapperStart, - span, - runProperties, - replacement, - tracked, - view, - formatting, + across, newChildren, placeReplacement, - () => { - placed = true; - }, ); continue; } @@ -412,52 +427,91 @@ function rebuildAcrossBoundaries( // fldSimple would technically duplicate the field instruction, but // Word re-evaluates fields on next render and any other behavior // would silently drop the user's replacement intent. - if (isRunBearingWrapper(child.tag)) { - const innerLength = sumVisibleTextLength(wrapperContent(child), view); - const wrapperStart = offset; - const wrapperEnd = offset + innerLength; - offset = wrapperEnd; - - if (wrapperEnd <= span.start) { - newChildren.push(child); - continue; - } - if (wrapperStart >= span.end) { - placeReplacement(); - newChildren.push(child); - continue; - } + splitTransparentWrapperAcrossSpan( + child, + wrapperStart, + span, + newChildren, + placeReplacement, + ); + } - splitTransparentWrapperAcrossSpan( - child, - wrapperStart, - span, - newChildren, - placeReplacement, - ); - continue; - } + return newChildren; +} - newChildren.push(child); +/** The replacement's runs, marking it placed. `wrapTracked` puts a tracked + * replacement in the editor's own `` (paragraph level, or inside a + * hyperlink it inherits); inside another revision's content it stays bare. */ +function placedReplacement( + across: AcrossSpan, + wrapTracked: boolean, +): XmlNode[] { + across.placed = true; + const runs = replacementRuns( + across.runProperties, + across.replacement, + across.formatting, + ); + if (across.tracked && wrapTracked) { + return [{runs}]; } + return runs; +} - if (!placed) placeReplacement(); - paragraph.children = newChildren; +function isTrackedChangeWrapper(tag: string): boolean { + return ( + tag === "w:ins" || + tag === "w:del" || + tag === "w:moveFrom" || + tag === "w:moveTo" + ); +} + +/** Untracked, a revision wrapper the span crosses is cut IN PLACE rather than + * split (issue #16): splitting it would put its `w:id` on both halves (an + * untracked replace has no revision-id allocator) and the replacement + * between them as plain original text. An additive wrapper (`` / + * ``) loses the span's text — descending nested hyperlinks, + * fields and revisions through the same walker — and keeps the replacement + * when the span starts inside it, credited to its author exactly as a match + * wholly inside it is; one left with no children is dropped. A subtractive + * wrapper's text is already deleted, so it stays whole and a replacement + * starting inside it lands right after it. */ +function cutRevisionInPlace( + wrapper: XmlNode, + wrapperStart: number, + across: AcrossSpan, + out: XmlNode[], + placeReplacement: () => void, +): void { + if (isSubtractiveTrackedChangeWrapper(wrapper.tag)) { + out.push(wrapper); + placeReplacement(); + return; + } + wrapper.children = rebuildChildrenAcrossSpan( + wrapper.children, + wrapperStart, + across, + false, + ); + if (wrapper.children.length > 0) out.push(wrapper); } function splitWrapperAcrossSpan( wrapper: XmlNode, wrapperStart: number, span: Span, - tracked: TrackedReplaceOptions | null, + tracked: TrackedReplaceOptions, out: XmlNode[], placeReplacement: () => void, ): void { // Subtractive wrappers (w:del, w:moveFrom) hold content that's already // considered deleted — the cut portion stays in the pre-half wrapper. - // Additive wrappers (w:ins, w:moveTo) hold "live" content; under tracking - // the cut needs a new wrapper nested inside, preserving the - // surrounding author's insert/move-to attribution. + // Additive wrappers (w:ins, w:moveTo) hold "live" content; the cut needs a + // new wrapper nested inside, preserving the surrounding author's + // insert/move-to attribution. (Known gap: a wrapper NESTED in this one — + // a hyperlink, another revision — rides the pre-half whole, uncounted.) const isSubtractive = isSubtractiveTrackedChangeWrapper(wrapper.tag); const preInner: XmlNode[] = []; const cutInner: XmlNode[] = []; @@ -494,7 +548,7 @@ function splitWrapperAcrossSpan( const preChildren = preInner.slice(); if (isSubtractive) { preChildren.push(...cutInner); - } else if (tracked && cutInner.length > 0) { + } else if (cutInner.length > 0) { for (const cutRun of cutInner) convertRunTextToDelText(cutRun); preChildren.push({cutInner}); } @@ -507,7 +561,13 @@ function splitWrapperAcrossSpan( placeReplacement(); if (postInner.length > 0) { - const postWrapper = new XmlNode(wrapper.tag, { ...wrapper.attributes }); + // Two revisions can't share a w:id: when the pre-half kept it, the + // post-half is a new revision with a fresh one. + const postAttributes = { ...wrapper.attributes }; + if (preChildren.length > 0 && postAttributes["w:id"] !== undefined) { + postAttributes["w:id"] = String(tracked.allocator.next()); + } + const postWrapper = new XmlNode(wrapper.tag, postAttributes); postWrapper.children = postInner; out.push(postWrapper); } @@ -566,16 +626,11 @@ function splitTransparentWrapperAcrossSpan( function splitHyperlinkAcrossSpan( wrapper: XmlNode, wrapperStart: number, - span: Span, - runProperties: XmlNode | null, - replacement: string, - tracked: TrackedReplaceOptions | null, - view: FindView, - formatting: ReplacementFormatting | undefined, + across: AcrossSpan, out: XmlNode[], placeReplacement: () => void, - markReplacementPlaced: () => void, ): void { + const { span, view } = across; const wrapperEnd = wrapperStart + sumVisibleTextLength(wrapper.children, view); const startsInside = span.start > wrapperStart && span.start < wrapperEnd; @@ -612,15 +667,10 @@ function splitHyperlinkAcrossSpan( } } - if (startsInside) { - // Replacement inherits the link: append it inside the pre-half. - const innerRuns = replacementRuns(runProperties, replacement, formatting); - if (tracked) { - preInner.push({innerRuns}); - } else { - preInner.push(...innerRuns); - } - markReplacementPlaced(); + // Replacement inherits the link when the span starts inside it: append it + // inside the pre-half (tracked, in the editor's own ). + if (startsInside && !across.placed) { + preInner.push(...placedReplacement(across, true)); } if (preInner.length > 0) { @@ -629,7 +679,7 @@ function splitHyperlinkAcrossSpan( out.push(preWrapper); } - if (!startsInside) placeReplacement(); + placeReplacement(); if (postInner.length > 0) { const postWrapper = new XmlNode("w:hyperlink", { ...wrapper.attributes }); diff --git a/src/core/track-changes/CLAUDE.md b/src/core/track-changes/CLAUDE.md index 158cb3a..d9e85cc 100644 --- a/src/core/track-changes/CLAUDE.md +++ b/src/core/track-changes/CLAUDE.md @@ -12,7 +12,7 @@ Five files behind the `@core/track-changes` barrel ([index.tsx](index.tsx)), plu ## Nested revisions — the all-ancestors visibility rule -Word nests one author's `` inside another author's still-pending `` (`……`), and the mirror (`` inside ``). The reader keeps the run's `trackedChange` as the INNERMOST wrapper (its `tcN` is what accept/reject targets) and records the enclosing chain in `TrackedChange.within` (see [ast/CLAUDE.md](../ast/CLAUDE.md)). **A run is visible in a view iff EVERY enclosing wrapper is visible in that view** — `accepted` hides `del`/`moveFrom`, `baseline` hides `ins`/`moveTo`, `current` shows all — because rejecting the outer insertion removes the nested deletion with it, and accepting the outer deletion removes the nested insertion. That is exactly what the reverse-pre-order `accept --all`/`reject --all` produces, so `read --baseline` ≡ `reject --all` and `read --accepted` ≡ `accept --all` (and `wc`/`find` agree). Classifying by the innermost wrapper alone showed the nested deletion's text in `--baseline`. The XML-side LENGTH/PLACEMENT walkers (`sumVisibleTextLength`/`isWrapperVisibleInView` in [find/replace-span.tsx](../find/replace-span.tsx), [find/replace-across.tsx](../find/replace-across.tsx) and [comments/markers.tsx](../comments/markers.tsx)) never descend a wrapper hidden in the view, so a wrapper nested inside a hidden one contributes nothing — the same rule, structurally; keep it that way. (Known gap, separate from views: `splitWrapperAcrossSpan` in replace-span.tsx does not recurse into a wrapper nested inside the wrapper it splits — a `replace` span that crosses a nested ``/`` inside a `` slices at the wrong offsets.) +Word nests one author's `` inside another author's still-pending `` (`……`), and the mirror (`` inside ``). The reader keeps the run's `trackedChange` as the INNERMOST wrapper (its `tcN` is what accept/reject targets) and records the enclosing chain in `TrackedChange.within` (see [ast/CLAUDE.md](../ast/CLAUDE.md)). **A run is visible in a view iff EVERY enclosing wrapper is visible in that view** — `accepted` hides `del`/`moveFrom`, `baseline` hides `ins`/`moveTo`, `current` shows all — because rejecting the outer insertion removes the nested deletion with it, and accepting the outer deletion removes the nested insertion. That is exactly what the reverse-pre-order `accept --all`/`reject --all` produces, so `read --baseline` ≡ `reject --all` and `read --accepted` ≡ `accept --all` (and `wc`/`find` agree). Classifying by the innermost wrapper alone showed the nested deletion's text in `--baseline`. The XML-side LENGTH/PLACEMENT walkers (`sumVisibleTextLength`/`isWrapperVisibleInView` in [find/replace-span.tsx](../find/replace-span.tsx), [find/replace-across.tsx](../find/replace-across.tsx) and [comments/markers.tsx](../comments/markers.tsx)) never descend a wrapper hidden in the view, so a wrapper nested inside a hidden one contributes nothing — the same rule, structurally; keep it that way. (Untracked, `rebuildAcrossBoundaries` in replace-span.tsx never splits a revision wrapper the span crosses (issue #16): an additive one is cut IN PLACE by recursing the same walker into its content — so a nested ``/field/revision is sliced at the right offsets, the replacement lands where the span starts (inside the ins, credited to its author, as for a match wholly inside it), and no `w:id` is duplicated; a subtractive one stays whole. Known gap, tracked only: `splitWrapperAcrossSpan` does not recurse into a wrapper nested inside the wrapper it splits, so a `--track` span that crosses a nested ``/`` inside a `` still slices at the wrong offsets — its post-half does get a FRESH id from the allocator.) ## TrackChanges lens API @@ -88,7 +88,7 @@ rPr equality is structural (XML-string compare on cloned nodes). The codebase's ## Tracked span replace inside another author's insertion (issue #13) -`replaceSpanInParagraph` (the engine behind `replace --track` and `edit --at pN:S-E --track`) has ONE early branch, dispatching to [core/find/replace-in-revision.tsx](../find/replace-in-revision.tsx). **The gate**: the replace is tracked, the span is non-empty, every overlapping run shares the same immediate parent, and that parent is a `` / `` sitting DIRECTLY under the paragraph. Everything else — untracked, empty spans, a span crossing the wrapper's edge, a wrapper nested in a hyperlink/smartTag/field, a match inside a `` under `--current`/`--baseline` — stays on the general walkers, byte-for-byte as before. Inside the gate, `w:author` decides (exact string match): **the wrapper is split the way Word does it** — it keeps its `w:id` plus the text before the span and the editor's `` of the cut nested inside it (a del-of-ins, so rejecting either author's revision restores the right text), the replacement rides the editor's own paragraph-level ``, and the content after the span rides a copy of the wrapper with a FRESH id from the shared allocator — unless that tail is pure markers (comment range end + reference run, bookmark end), which stay in the wrapper after the cut instead of minting an empty revision (`isPureMarker`). **The editor's own `` is rebuilt in place** (same id, same date, no ``, no split) — the replacement is just more of that same insertion — but every ZERO-width child of the cut runs (note/comment reference, drawing, text-box anchor — `inlineMarkerWidth === 0`; a tab/break/sym the offsets counted as a character goes with the text) is kept (`cutRemnants`), range starts ahead of the replacement and the rest behind it, so nothing anchored on the replaced words vanishes and a comment that opened inside the cut brackets the new words. A `` is split even by its own author: moved text is relocated original text, not new words. `revN` pairing in `cli/track-changes/groups.ts` requires both halves to share an author, so the other author's head ins never pairs with the editor's nested del. The new path computes the wrapper's start offset from the siblings before it, where the general `rebuildContainer` starts its walk at the first OVERLAPPING run's offset — correct only when the match sits in the wrapper's first run, which is the "deleted the wrong words" half of the bug (the nested `` in the issue is just what put the match in a later run; the same miscount still applies to an untracked multi-run wrapper and to hyperlinks, both deliberately untouched here). Nested non-run children (a third author's ``, bookmarks) move WHOLE, counted at their view-visible length (a `` is 0 wide in the accepted view), and are never wrapped in the new ``: one inside the span keeps its place, so the cut is deleted as one `` per contiguous run group. Known limitations, all deliberately left on the old paths: an insertion nested in a hyperlink still gets a bare run inside it; an empty span (`pN:S-S`) inserts a bare run inside the other author's wrapper; a span crossing out of the wrapper takes `rebuildAcrossBoundaries`, whose two halves still share the original `w:id`; a `--current` match inside another author's `` still lands the replacement inside that deletion. +`replaceSpanInParagraph` (the engine behind `replace --track` and `edit --at pN:S-E --track`) has ONE early branch, dispatching to [core/find/replace-in-revision.tsx](../find/replace-in-revision.tsx). **The gate**: the replace is tracked, the span is non-empty, every overlapping run shares the same immediate parent, and that parent is a `` / `` sitting DIRECTLY under the paragraph. Everything else — untracked, empty spans, a span crossing the wrapper's edge, a wrapper nested in a hyperlink/smartTag/field, a match inside a `` under `--current`/`--baseline` — stays on the general walkers, byte-for-byte as before. Inside the gate, `w:author` decides (exact string match): **the wrapper is split the way Word does it** — it keeps its `w:id` plus the text before the span and the editor's `` of the cut nested inside it (a del-of-ins, so rejecting either author's revision restores the right text), the replacement rides the editor's own paragraph-level ``, and the content after the span rides a copy of the wrapper with a FRESH id from the shared allocator — unless that tail is pure markers (comment range end + reference run, bookmark end), which stay in the wrapper after the cut instead of minting an empty revision (`isPureMarker`). **The editor's own `` is rebuilt in place** (same id, same date, no ``, no split) — the replacement is just more of that same insertion — but every ZERO-width child of the cut runs (note/comment reference, drawing, text-box anchor — `inlineMarkerWidth === 0`; a tab/break/sym the offsets counted as a character goes with the text) is kept (`cutRemnants`), range starts ahead of the replacement and the rest behind it, so nothing anchored on the replaced words vanishes and a comment that opened inside the cut brackets the new words. A `` is split even by its own author: moved text is relocated original text, not new words. `revN` pairing in `cli/track-changes/groups.ts` requires both halves to share an author, so the other author's head ins never pairs with the editor's nested del. The new path computes the wrapper's start offset from the siblings before it, where the general `rebuildContainer` starts its walk at the first OVERLAPPING run's offset — correct only when the match sits in the wrapper's first run, which is the "deleted the wrong words" half of the bug (the nested `` in the issue is just what put the match in a later run; `rebuildContainer` now starts at the container's own start offset — `RunSlot.parentStart` — so an untracked match in a later run of a multi-run wrapper cuts the right words too (#16)). Nested non-run children (a third author's ``, bookmarks) move WHOLE, counted at their view-visible length (a `` is 0 wide in the accepted view), and are never wrapped in the new ``: one inside the span keeps its place, so the cut is deleted as one `` per contiguous run group. Known limitations, all deliberately left on the old paths: an insertion nested in a hyperlink still gets a bare run inside it; an empty span (`pN:S-S`) inserts a bare run inside the other author's wrapper; a span crossing out of the wrapper takes `rebuildAcrossBoundaries` (its post-half gets a fresh `w:id` since #16); a `--current` match inside another author's `` still lands the replacement inside that deletion. ## Adding a new tracked-change kind diff --git a/tests/cli/replace.test.ts b/tests/cli/replace.test.ts index 03cf125..6873c18 100644 --- a/tests/cli/replace.test.ts +++ b/tests/cli/replace.test.ts @@ -1725,8 +1725,8 @@ describe("docx replace --track inside another author's insertion (#13)", () => { test("a span CROSSING out of the ins keeps the general walker's shape", async () => { // Only a match wholly inside the wrapper is gated; this one starts in the - // plain run before it. The expected string is main's output, pinned - // verbatim (its reuse of w:id="1" on both halves is pre-existing). + // plain run before it. The half of the ins after the cut is a second + // revision, so it gets a fresh id (it reused w:id="1" before #16). const docPath = await trackedReplace( SINGLE_RUN, "ins-crossing", @@ -1738,7 +1738,7 @@ describe("docx replace --track inside another author's insertion (#13)", () => { 'Client ' + 'disputed sum' + `${INS_A}may` + - `${INS_A} withhold a disputed amount.`, + ' withhold a disputed amount.', ); }); @@ -1759,3 +1759,168 @@ describe("docx replace --track inside another author's insertion (#13)", () => { ); }); }); + +// Issue #16: an UNTRACKED replace whose match starts in plain text inside +// another author's pending and ends inside a nested in +// that same ins. Main split the ins around the match (both halves kept +// w:id="1"), put the replacement BETWEEN the halves as plain original text, +// and never descended the nested link — so the link's part of the match +// survived and the order scrambled ("may [a disputed](#x)keep aamount."). +// Untracked, a revision wrapper is now cut IN PLACE (no split, so no id is +// duplicated) and the replacement lands where the match started — inside the +// ins, credited to its author, as it already did for a match wholly inside it. +describe("docx replace across a wrapper nested in another author's insertion (#16)", () => { + const INS_A = + ''; + // A directly under is what the issue reported, but the + // ECMA schema only allows the reverse nesting, so this body fails `validate` + // before any edit — those cases skip the schema check. + const INS_LINK = `The Client ${INS_A}may withhold a disputed amount.`; + + async function paragraphXml(docPath: string): Promise { + const xml = await readDocumentXml(docPath); + return xml.match(/.*?<\/w:p>/s)?.[0] ?? ""; + } + + function revisionIds(xml: string): string[] { + return [...xml.matchAll(/ match[1] ?? "", + ); + } + + async function replaceIn( + body: string, + label: string, + pattern: string, + replacement: string, + ...flags: string[] + ): Promise { + const docPath = await buildRawDoc(body, label); + const result = await runCli( + "replace", + docPath, + pattern, + replacement, + ...flags, + ); + expect(result.exitCode).toBe(0); + if (body !== INS_LINK) { + expect((await runCli("validate", docPath)).exitCode).toBe(0); + } + return docPath; + } + + test("the link's part of the match is cut and the replacement stays in the ins", async () => { + const docPath = await replaceIn( + INS_LINK, + "ins-link", + "withhold a", + "keep a", + ); + expect(await paragraphXml(docPath)).toBe( + 'The Client ' + + `${INS_A}may ` + + 'keep a' + + ' disputed' + + ' amount.', + ); + const read = await runCli("read", docPath); + expect(read.stdout).toContain( + "The Client may keep a[ disputed](#x) amount.", + ); + const ids = revisionIds(await paragraphXml(docPath)); + expect(new Set(ids).size).toBe(ids.length); + }); + + test("a match starting inside the nested link keeps the replacement in the link", async () => { + const docPath = await replaceIn( + INS_LINK, + "ins-link-start", + "disputed amount", + "sum", + ); + expect(await paragraphXml(docPath)).toBe( + 'The Client ' + + `${INS_A}may withhold ` + + 'a sum' + + '.', + ); + }); + + test("a match crossing OUT of the ins cuts it in place without duplicating its id", async () => { + const docPath = await replaceIn( + `The Client ${INS_A}may withhold a disputed amount.`, + "ins-out", + "withhold a", + "keep a", + ); + expect(await paragraphXml(docPath)).toBe( + 'The Client ' + + `${INS_A}may ` + + 'keep a' + + ' disputed amount.', + ); + }); + + test("an ins the match covers entirely is removed, not left empty", async () => { + const docPath = await replaceIn( + `The Client may ${INS_A}not pay.`, + "ins-covered", + "may not pay", + "must pay", + ); + expect(await paragraphXml(docPath)).toBe( + 'The Client ' + + 'must pay' + + '.', + ); + }); + + test("a match in a LATER run of a multi-run ins cuts the right words", async () => { + const docPath = await replaceIn( + `The Client ${INS_A}may withhold a disputed amount.`, + "ins-multirun", + "disputed", + "contested", + ); + const read = await runCli("read", docPath); + expect(read.stdout).toContain( + "The Client may withhold a contested amount.", + ); + }); + + test("a match in a later run of a multi-run hyperlink cuts the right words", async () => { + const docPath = await replaceIn( + 'alpha beta gamma', + "link-multirun", + "gamma", + "DELTA", + ); + const read = await runCli("read", docPath); + expect(read.stdout).toContain("[alpha beta DELTA](#x)"); + }); + + test("a run after a link nested in the ins keeps its offset", async () => { + const docPath = await buildRawDoc( + `${INS_A}alpha link beta gamma`, + "ins-link-after", + ); + expect((await runCli("replace", docPath, "gamma", "DELTA")).exitCode).toBe( + 0, + ); + const read = await runCli("read", docPath, "--accepted"); + expect(read.stdout).toContain("alpha [link ](#x)beta DELTA"); + }); + + test("tracked, the half of the ins after the match gets a fresh id", async () => { + const docPath = await replaceIn( + `The Client ${INS_A}may withhold a disputed amount.`, + "ins-tracked-out", + "Client may", + "Client must", + "--track", + ); + const ids = revisionIds(await paragraphXml(docPath)); + expect(new Set(ids).size).toBe(ids.length); + }); +}); From 130838a39705ad277986c6a40fd441e11e20d44a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 18:45:07 +0000 Subject: [PATCH 2/2] test(replace): pin untracked replace after a deletion nested in an insertion (#22) The #16 in-place cut already maps offsets past another author's nested ; this regression test fails on main (it cut "The Client may " instead of "disputed amount") and passes on this branch. Triage-Routine: true Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UKadSq4CY1JScFMncHeY4W --- tests/cli/replace.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/cli/replace.test.ts b/tests/cli/replace.test.ts index 6873c18..fb0b38e 100644 --- a/tests/cli/replace.test.ts +++ b/tests/cli/replace.test.ts @@ -1912,6 +1912,30 @@ describe("docx replace across a wrapper nested in another author's insertion (#1 expect(read.stdout).toContain("alpha [link ](#x)beta DELTA"); }); + // Issue #22: a match AFTER another author's deletion nested in the ins. The + // deletion's text is absent from the accepted view, so the offset mapping + // must skip it — main cut "The Client may " instead of the match. + test("a match after a deletion nested in the ins cuts the right words (#22)", async () => { + const DEL_B = + 'any '; + const docPath = await replaceIn( + `Fees are due. ${INS_A}The Client may withhold ${DEL_B}disputed amount.`, + "ins-del-after", + "disputed amount", + "disputed sum", + ); + expect(await paragraphXml(docPath)).toBe( + 'Fees are due. ' + + `${INS_A}The Client may withhold ${DEL_B}` + + 'disputed sum' + + '.', + ); + const read = await runCli("read", docPath, "--accepted"); + expect(read.stdout).toContain( + "Fees are due. The Client may withhold disputed sum.", + ); + }); + test("tracked, the half of the ins after the match gets a fresh id", async () => { const docPath = await replaceIn( `The Client ${INS_A}may withhold a disputed amount.`,