fix(super-editor): keep comment range markers around unpaired tracked changes (SD-2528) (#3239)

* fix(super-editor): keep comment range markers as siblings of unpaired tracked changes (SD-2528)

mergeConsecutiveTrackedChanges greedily absorbed trailing w:commentRangeEnd
and w:r->w:commentReference elements into a w:del/w:ins wrapper even when no
same-id wrapper followed to actually merge into. This produced a lopsided
structure where w:commentRangeStart sat outside the wrapper but
w:commentRangeEnd ended up inside it, breaking comment round-trip on redlined
text.

Buffer comment markers during the forward scan and only commit them inside
the wrapper when a same-id merge actually happens. Otherwise emit them as
siblings, matching Word's expected OOXML.

SD-1519 merge behavior is unchanged and covered by the new tests.

* fix(super-editor): fold commentRangeStart into TC wrapper for round-trip (SD-2528)

The previous fix left commentRangeStart as a sibling of w:ins/w:del.
documentCommentsImporter.js' extractCommentRangesFromDocument only
associates a comment with a tracked change when commentRangeStart sits
inside the wrapper, so the sibling shape silently dropped the comment
to TC link on re-import.

Fold any leading commentRangeStart sibling into the immediately following
w:ins/w:del as its first child, matching the shape Word produces. The
existing SD-1519 same-id merge for trailing commentRangeEnd and
w:r/w:commentReference stays unchanged.

Adds an end-to-end test that loads the Google-Docs TC+comment fixture,
exports it, re-imports the exported XML, and asserts every comment that
was originally inside a tracked change still carries
trackedChangeParentId after the round-trip.

* fix: cascade accept/reject of a tracked change to its anchored comments (SD-2528)

A user comment anchored to a tracked change carries trackedChangeParentId
pointing at the TC. Two bugs broke the link end-to-end:

1. docxImporter built two tracked-change id maps independently
   (trackedChangeIdMap and trackedChangeIdMapsByPart), each minting fresh
   UUIDs for the same Word w:id. The comments importer used the global
   map; ins/del translators used the per-part map. The two never matched,
   so trackedChangeParentId on a comment never pointed at the actual TC
   mark id in the PM doc. Fix: build the per-part maps first and reuse
   the document.xml entry as the global map.

2. The comments-store resolve handler only resolved the TC's own
   redline-display entry. User comments with trackedChangeParentId === the
   resolved TC stayed open. Fix: after resolving the TC entity, iterate
   commentsList and resolve every comment whose trackedChangeParentId
   matches. Defer via Promise.resolve so the cascading resolveComment
   doesn't dispatch into a still-running accept/rejectTrackedChangeById
   loop and collide with the loop's mutable tr.

E2E browser repro on the real Google-Docs TC+comment fixture: accept TC
by id, both the TC and its anchored user comments resolve in one user
action. Same for reject. No mismatched-tr errors.

The export-side round-trip test also asserts the two id maps are
aligned and every comment trackedChangeParentId matches a real
tracked-change mark id in the PM doc.

* fix(super-editor): thread reply-to-TC under its tracked-change bubble on re-import (SD-2528)

A reply that the user typed under a tracked-change bubble has
parentCommentId pointing at the synthetic TC entity in the comments
store. On export the TC parent is filtered out of comments.xml
(TC entries are not real comments), so the reply lands in the file
without any paraIdParent. On re-import the reply gets trackedChangeParentId
via the document.xml walker (the commentRange wraps the TC text) but
parentCommentId was left undefined — the sidebar then renders the reply
as a separate top-level bubble next to the TC instead of nested under it,
matching the user-reported regression in image 1 of SD-2528.

Promote trackedChangeParentId to parentCommentId when no explicit
parent is set. CommentDialog already threads via direct
parentCommentId === trackedChangeId (line 321), so this is the
cheapest path to restore the live pre-export state.

Round-trip stable: re-export still filters TC parents but re-emits the
commentRange inside the wrapper, which gets re-detected on the next
import via extractCommentRangesFromDocument and re-establishes the
linkage.

* fix(comments): thread tracked-change replies regardless of file origin (SD-2528)

The UI guarded TC reply threading with isRangeThreadedComment, which is
true only when the source DOCX has no commentsExtended.xml (Google Docs
style). SuperDoc-exported DOCX files always write commentsExtended.xml,
so on re-import the guard short-circuited and the reply rendered as a
top-level bubble next to its TC instead of nested under it.

Drop the file-origin guard from the two sites that threaded TC replies:
collectTrackedChangeThread in CommentDialog.vue and
shouldThreadWithTrackedChange in comments-store.js.
trackedChangeParentId pointing at a tracked-change entity is sufficient
to thread; file origin should not change whether a comment threads under
its TC.

Reverts the earlier importer-side patch that promoted
trackedChangeParentId into parentCommentId. That patch violated the
comment-diffing contract (parentCommentId is diffed; trackedChangeParentId
is intentionally ignored because it is regenerated across imports) and
broke six existing tests. The UI-side change is surgical and breaks no
tests.

* fix(comments): preserve TC color on anchored comments + clean up IMPORTED/resolve gates (SD-2528)

Three visual round-trip regressions after the SD-2528 fix made TC replies
thread again:

1. CommentHighlightDecorator painted its pink (external) / green (internal)
   inline background on every element with the superdoc-comment-highlight
   class — including text that already carries a track-insert-dec /
   track-delete-dec decoration. The inline style won over the TC's own CSS
   class background, so a green trackInsert came back pink after re-import.
   Skip the BG override when the element is also a tracked-change decoration:
   the TC color (green for insert, red for delete) is the right signal for
   the user, and the comment range is still visually identified by its
   dashed border + sidebar bubble.

2. CommentHeader's IMPORTED tag fired whenever comment.origin or
   importedAuthor was set — including comments authored by the current user
   in a previous session. Round-tripping a file you exported then re-opened
   should not relabel your own comments as imported. Suppress the tag when
   the comment's creatorEmail matches the current user's email.

3. CommentHeader's allowResolve guard treated parentCommentId as the only
   marker of a child comment. A TC-anchored reply on re-import keeps the
   linkage through trackedChangeParentId only (parentCommentId is left
   undefined to preserve the comment-diffing contract). The resolve check
   affordance therefore appeared on re-imported replies even though the
   pre-export state had no parentCommentId either. Treat
   trackedChangeParentId as an equivalent child signal.

All three are surgical render-side gates — no converter / data-model
changes. 1369 super-editor presentation tests pass.

* fix(comments): scope cascade to active doc + tighten TC-anchored gates (SD-2528)

Addresses Codex's 3 P2 review findings from PR #3239:

P2 #1 — comments-store.js cascade scope
  The new cascade-resolve scan introduced in aa88a58 didn't honour the
  resolve event's documentId. findTrackedChangeById (line 591) correctly
  scopes its match by belongsToTrackedChangeSyncDocument; the cascade six
  lines lower did not. In multi-document sessions where imported
  tracked-change ids collide across files (each w:id space is local),
  accepting a change in document A would also resolve comments anchored on
  the same id in document B. Mirror the same per-document filter when a
  documentId is provided; single-document callers (no documentId on the
  event) keep the legacy global behaviour.

P2 #2 — CommentHighlightDecorator.ts visual gate
  The earlier suppression triggered on any of `track-insert-dec`,
  `track-delete-dec`, or `track-format-dec`. Per layout-engine styles.ts
  only `.track-insert-dec.highlighted` and `.track-delete-dec.highlighted`
  paint a background; `.track-format-dec` only paints a border-bottom, and
  the `.highlighted` modifier is only applied in "review" / All Markup mode
  (renderer.ts:909-928). In Original / Final modes, and on format-only
  changes, the suppression cleared the comment fill with nothing to replace
  it, making the bubble invisible. Tighten the gate to require both
  `.highlighted` and one of the bg-painting base classes.

P2 #3 — collectTrackedChangeThread parent shadowing
  documentCommentsImporter can produce a comment with BOTH a non-TC
  `parentCommentId` and a `trackedChangeParentId`: the comment's range
  lives inside a TC, but its conversational thread starts at a regular
  comment outside the TC. The previous unconditional pull on
  trackedChangeParentId placed such replies in both threads. Restrict the
  direct seed to roots (no parentCommentId) and let the BFS step pick up
  same-TC-anchored chains via parent links. Extract the helper to a
  sibling module so the BFS logic can be unit-tested in isolation —
  previously trapped inside CommentDialog.vue's <script setup>.

Verification
  - 8 new unit tests covering each P2 case (3 in
    collect-tracked-change-thread.test.js, 4+ in
    CommentHighlightDecorator.test.ts, 1 cross-doc + 1 single-doc
    regression in comments-store.test.js).
  - SD-2528 integration round-trip test still passes (1/1).
  - super-editor: 12 850 / 12 850 unit tests pass.
  - superdoc: 966 unit tests pass (1 pre-existing collab-server import
    failure, unrelated, present on main and other branches).
  - Browser repro on the corpus fixture: accepting an imported TC still
    cascade-resolves both anchored user comments end-to-end.

Author: tupizz

packages/super-editor/src/editors/v1/core/presentation-editor/dom/CommentHighlightDecorator.test.ts

@@ -352,6 +352,105 @@ describe('CommentHighlightDecorator', () => {
 
   // ── Edge cases ─────────────────────────────────────────────────────
 
+  // ── SD-2528 P2 #2: comment fill vs tracked-change background ───────
+  //
+  // Per layout-engine styles.ts, only `.track-insert-dec.highlighted` and
+  // `.track-delete-dec.highlighted` paint a background; `.track-format-dec`
+  // only paints a `border-bottom`. The `.highlighted` modifier is ONLY
+  // applied in "review" (All Markup) mode — Original and Final modes use
+  // `.hidden` / `.normal` / `.before` and paint no background.
+  //
+  // The decorator must therefore suppress its comment-fill repaint ONLY in
+  // the narrow case where the TC actually paints a competing background.
+  // Otherwise the comment highlight is cleared with nothing to replace it
+  // and the comment becomes invisible in Original / Final / format-only.
+  describe('tracked-change-anchored elements (SD-2528 P2 #2)', () => {
+    const commentClass = 'superdoc-comment-highlight';
+
+    function tcCommentSpan(opts: { commentIds: string[]; tcClasses: string[] }): HTMLSpanElement {
+      const el = commentSpan({ commentIds: opts.commentIds });
+      el.classList.add(...opts.tcClasses);
+      return el;
+    }
+
+    it('suppresses comment fill when element is track-insert-dec AND highlighted', () => {
+      const span = tcCommentSpan({ commentIds: ['c-1'], tcClasses: ['track-insert-dec', 'highlighted'] });
+      container.appendChild(span);
+
+      decorator.apply();
+
+      expect(span.style.backgroundColor).toBe('');
+      expect(span.classList.contains(commentClass)).toBe(true);
+    });
+
+    it('suppresses comment fill when element is track-delete-dec AND highlighted', () => {
+      const span = tcCommentSpan({ commentIds: ['c-1'], tcClasses: ['track-delete-dec', 'highlighted'] });
+      container.appendChild(span);
+
+      decorator.apply();
+
+      expect(span.style.backgroundColor).toBe('');
+    });
+
+    it('keeps comment fill when track-insert-dec is present but .highlighted is NOT (Original/Final mode)', () => {
+      // In Original/Final modes the painter applies `.hidden` / `.normal` /
+      // `.before` instead of `.highlighted`, so no green background is drawn.
+      // The comment fill must remain visible — otherwise the comment bubble
+      // disappears with no replacement paint.
+      const span = tcCommentSpan({ commentIds: ['c-1'], tcClasses: ['track-insert-dec', 'normal'] });
+      container.appendChild(span);
+
+      decorator.apply();
+
+      expect(span.style.backgroundColor).toBe(EXT);
+    });
+
+    it('keeps comment fill when track-delete-dec is present but .highlighted is NOT', () => {
+      const span = tcCommentSpan({ commentIds: ['c-1'], tcClasses: ['track-delete-dec', 'hidden'] });
+      container.appendChild(span);
+
+      decorator.apply();
+
+      expect(span.style.backgroundColor).toBe(EXT);
+    });
+
+    it('keeps comment fill on track-format-dec.highlighted — format changes paint only a border, not a background', () => {
+      const span = tcCommentSpan({ commentIds: ['c-1'], tcClasses: ['track-format-dec', 'highlighted'] });
+      container.appendChild(span);
+
+      decorator.apply();
+
+      expect(span.style.backgroundColor).toBe(EXT);
+    });
+
+    it('still suppresses comment fill in the active highlight branch when TC paints', () => {
+      const span = tcCommentSpan({ commentIds: ['c-1'], tcClasses: ['track-insert-dec', 'highlighted'] });
+      container.appendChild(span);
+
+      setActiveCommentAndApply(decorator, 'c-1');
+
+      expect(span.style.backgroundColor).toBe('');
+    });
+
+    it('still suppresses comment fill in the faded branch when TC paints and a different comment is active', () => {
+      const span = tcCommentSpan({ commentIds: ['c-1'], tcClasses: ['track-delete-dec', 'highlighted'] });
+      container.appendChild(span);
+
+      setActiveCommentAndApply(decorator, 'other');
+
+      expect(span.style.backgroundColor).toBe('');
+    });
+
+    it('keeps faded comment fill in Original mode (track-insert-dec without .highlighted)', () => {
+      const span = tcCommentSpan({ commentIds: ['c-1'], tcClasses: ['track-insert-dec', 'normal'] });
+      container.appendChild(span);
+
+      setActiveCommentAndApply(decorator, 'other');
+
+      expect(span.style.backgroundColor).toBe(EXT_FADED);
+    });
+  });
+
   describe('edge cases', () => {
     it('apply() is a no-op when no container is set', () => {
       const dec = new CommentHighlightDecorator();

packages/super-editor/src/editors/v1/core/presentation-editor/dom/CommentHighlightDecorator.ts

@@ -171,9 +171,39 @@ export class CommentHighlightDecorator {
       // Determine if primary (first) comment is internal — used for uniform/faded colors.
       const primaryIsInternal = internalIds.has(ids[0]);
 
+      // SD-2528: a comment anchored on tracked-change text shows the TC's
+      // own background (green for trackInsert, red for trackDelete) via
+      // `.track-insert-dec.highlighted` / `.track-delete-dec.highlighted`.
+      // The comment highlight stacking on top of that paints pink/green
+      // over the TC color, making an "insert" look pink after re-import.
+      // Leave the background alone in that case so the TC color wins
+      // (matches Word — comments anchored on a redline don't recolor the
+      // redline).
+      //
+      // AIDEV-NOTE: SD-2528 P2 #2. Suppress comment fill ONLY when the TC
+      // is actually painting a competing background. Per layout-engine
+      // styles.ts:270-294, that requires both:
+      //   - the base class `track-insert-dec` or `track-delete-dec` (not
+      //     `track-format-dec`, which only paints a `border-bottom`);
+      //   - the `.highlighted` modifier — only applied in "review" / All
+      //     Markup mode per renderer.ts:909-928. In Original/Final modes
+      //     the modifier is `hidden` / `normal` / `before` and no
+      //     background is painted.
+      // Without this narrower gate the comment highlight was cleared with
+      // nothing to replace it, making the bubble invisible in Original /
+      // Final modes and on format-only changes. Hover/focus affordances
+      // still come from the TC's own `.track-change-focused` class.
+      const isTrackedChangeAnchored =
+        el.classList.contains('highlighted') &&
+        (el.classList.contains('track-insert-dec') || el.classList.contains('track-delete-dec'));
+
       if (activeId == null) {
         // No active comment → uniform light highlight
-        applyBgColor(el, primaryIsInternal ? H.INT : H.EXT);
+        if (!isTrackedChangeAnchored) {
+          applyBgColor(el, primaryIsInternal ? H.INT : H.EXT);
+        } else {
+          el.style.backgroundColor = '';
+        }
         el.style.boxShadow = '';
         continue;
       }
@@ -184,7 +214,11 @@ export class CommentHighlightDecorator {
       if (matchedId != null) {
         // This element belongs to the active comment → bright highlight
         const matchIsInternal = internalIds.has(matchedId);
-        applyBgColor(el, matchIsInternal ? H.INT_ACTIVE : H.EXT_ACTIVE);
+        if (!isTrackedChangeAnchored) {
+          applyBgColor(el, matchIsInternal ? H.INT_ACTIVE : H.EXT_ACTIVE);
+        } else {
+          el.style.backgroundColor = '';
+        }
 
         // Nested comments: other IDs besides the active one
         const hasNested = ids.length > 1;
@@ -195,7 +229,11 @@ export class CommentHighlightDecorator {
         }
       } else {
         // Active comment is set but doesn't match this element → faded
-        applyBgColor(el, primaryIsInternal ? H.INT_FADED : H.EXT_FADED);
+        if (!isTrackedChangeAnchored) {
+          applyBgColor(el, primaryIsInternal ? H.INT_FADED : H.EXT_FADED);
+        } else {
+          el.style.backgroundColor = '';
+        }
         el.style.boxShadow = '';
       }
     }

packages/super-editor/src/editors/v1/core/super-converter/v2/importer/documentCommentsImporter.js

@@ -208,9 +208,6 @@ const generateCommentsWithExtendedData = ({ docx, comments, converter, threading
       }
     }
 
-    // Track the tracked change association but don't use it as parentCommentId
-    // This keeps comments and tracked changes as separate bubbles in the UI
-    // while preserving the relationship for export and visual purposes
     const trackedChangeParentId = isInsideTrackedChange ? trackedChangeParent.trackedChangeId : undefined;
 
     // Only use range-based parenting as fallback when:

packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.js

@@ -158,8 +158,16 @@ export const createDocumentJson = (docx, converter, editor) => {
     const trackedChangeIdMapOptions = {
       replacements: converter.trackedChangesOptions?.replacements ?? 'paired',
     };
-    converter.trackedChangeIdMap = buildTrackedChangeIdMap(docx, trackedChangeIdMapOptions);
+    // AIDEV-NOTE: SD-2528. The per-part map and the global map MUST share UUIDs
+    // for the same w:id, otherwise documentCommentsImporter (uses the global)
+    // and the ins/del translators (use the per-part) end up with two different
+    // UUIDs for the same tracked change — the comment's trackedChangeParentId
+    // never matches the tracked-change mark's id, breaking accept/reject
+    // cascading.
     converter.trackedChangeIdMapsByPart = buildTrackedChangeIdMapsByPart(docx, trackedChangeIdMapOptions);
+    converter.trackedChangeIdMap =
+      converter.trackedChangeIdMapsByPart.get('word/document.xml') ??
+      buildTrackedChangeIdMap(docx, trackedChangeIdMapOptions);
     const comments = importCommentData({ docx, nodeListHandler, converter, editor });
     const footnotes = importFootnoteData({ docx, nodeListHandler, converter, editor, numbering });
     const endnotes = importEndnoteData({ docx, nodeListHandler, converter, editor, numbering });

packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/p/helpers/translate-paragraph-node.js

@@ -1,72 +1,103 @@
 import { translateChildNodes } from '@converter/v2/exporter/helpers/index.js';
 import { generateParagraphProperties } from './generate-paragraph-properties.js';
 
+const isTrackedChangeWrapper = (el) => el?.name === 'w:ins' || el?.name === 'w:del';
+
+const isCommentMarker = (el) => {
+  if (!el) return false;
+  if (el.name === 'w:commentRangeStart' || el.name === 'w:commentRangeEnd') return true;
+  if (el.name === 'w:r' && el.elements?.length === 1 && el.elements[0]?.name === 'w:commentReference') return true;
+  return false;
+};
+
+// AIDEV-NOTE: SD-2528. The importer associates a comment with a tracked change
+// by walking document.xml and noting commentRangeStart elements that appear
+// inside a w:ins/w:del wrapper (see documentCommentsImporter.js'
+// extractCommentRangesFromDocument). Word always emits commentRangeStart inside
+// the wrapper; emitting it as a sibling silently loses the comment ↔ TC link
+// on re-import.
+function foldLeadingCommentStartsIntoTrackedChanges(elements) {
+  const result = [];
+  let i = 0;
+  while (i < elements.length) {
+    if (elements[i]?.name !== 'w:commentRangeStart') {
+      result.push(elements[i]);
+      i++;
+      continue;
+    }
+    const leadingStarts = [];
+    while (i < elements.length && elements[i]?.name === 'w:commentRangeStart') {
+      leadingStarts.push(elements[i]);
+      i++;
+    }
+    const next = elements[i];
+    if (isTrackedChangeWrapper(next)) {
+      result.push({ ...next, elements: [...leadingStarts, ...(next.elements || [])] });
+      i++;
+    } else {
+      result.push(...leadingStarts);
+    }
+  }
+  return result;
+}
+
 /**
- * Merge consecutive tracked change elements (w:ins/w:del) with the same ID.
- * Comment range markers between tracked changes with the same ID are included
- * inside the merged wrapper, matching Word's OOXML structure.
- *
- * See SD-1519 for details on the ECMA-376 spec compliance.
+ * Merge consecutive tracked change elements (w:ins/w:del) with the same ID,
+ * and fold any commentRangeStart that immediately precedes a tracked-change
+ * wrapper INTO the wrapper as its first child(ren). Trailing commentRangeEnd
+ * and w:r→w:commentReference stay as siblings and are only absorbed when a
+ * same-id successor wrapper triggers an SD-1519 merge.
  *
  * @param {Array} elements The translated paragraph elements
  * @returns {Array} Elements with consecutive tracked changes merged
  */
 function mergeConsecutiveTrackedChanges(elements) {
   if (!Array.isArray(elements) || elements.length === 0) return elements;
 
+  elements = foldLeadingCommentStartsIntoTrackedChanges(elements);
+
   const result = [];
   let i = 0;
 
   while (i < elements.length) {
     const current = elements[i];
 
-    // Check if this is a tracked change wrapper (w:ins or w:del)
-    if (current?.name === 'w:ins' || current?.name === 'w:del') {
+    if (isTrackedChangeWrapper(current)) {
       const tcId = current.attributes?.['w:id'];
       const tcName = current.name;
 
-      // Collect consecutive elements that belong to this tracked change
       const mergedElements = [...(current.elements || [])];
+      const pendingComments = [];
+      let didMerge = false;
       let j = i + 1;
 
       while (j < elements.length) {
         const next = elements[j];
 
-        // Include comment markers - they can sit inside tracked changes per ECMA-376
-        if (next?.name === 'w:commentRangeStart' || next?.name === 'w:commentRangeEnd') {
-          mergedElements.push(next);
+        if (isCommentMarker(next)) {
+          pendingComments.push(next);
           j++;
           continue;
         }
 
-        // Include comment references (w:r containing w:commentReference)
-        if (next?.name === 'w:r') {
-          const hasOnlyCommentRef = next.elements?.length === 1 && next.elements[0]?.name === 'w:commentReference';
-          if (hasOnlyCommentRef) {
-            mergedElements.push(next);
-            j++;
-            continue;
-          }
-        }
-
-        // Merge with next tracked change if same type and ID
         if (next?.name === tcName && next.attributes?.['w:id'] === tcId) {
-          mergedElements.push(...(next.elements || []));
+          mergedElements.push(...pendingComments, ...(next.elements || []));
+          pendingComments.length = 0;
+          didMerge = true;
           j++;
           continue;
         }
 
-        // Stop merging when we hit a different element
         break;
       }
 
-      // Create the merged wrapper
-      result.push({
-        name: tcName,
-        attributes: { ...current.attributes },
-        elements: mergedElements,
-      });
-
+      if (didMerge) {
+        result.push({ name: tcName, attributes: { ...current.attributes }, elements: mergedElements });
+        result.push(...pendingComments);
+      } else {
+        result.push(current);
+        result.push(...pendingComments);
+      }
       i = j;
     } else {
       result.push(current);