fix(comments-store): preserve tracked-change thread state during undo/redo operations

Author: palmer-cl

packages/superdoc/src/SuperDoc.vue

@@ -990,8 +990,6 @@ const onEditorTransaction = (payload = {}) => {
   // This could be extended to other listeners in the future
   if (inputType === 'historyUndo' || inputType === 'historyRedo') {
     const documentId = editor?.options?.documentId;
-    // separation is useful because functions have different inputs
-    // different side effects, and different reuse points.
     syncTrackedChangePositionsWithDocument({ documentId, editor });
     syncTrackedChangeComments({ superdoc: proxy.$superdoc, editor });
   }

packages/superdoc/src/stores/comments-store.js

@@ -29,6 +29,7 @@ export const useCommentsStore = defineStore('comments', () => {
 
   const isDebugging = false;
   const debounceTimers = {};
+  const trackedChangeResolutionSnapshots = new WeakMap();
 
   const COMMENT_EVENTS = comments_module_events;
   const hasInitializedComments = ref(false);
@@ -188,6 +189,17 @@ export const useCommentsStore = defineStore('comments', () => {
 
   const clearResolvedMetadata = (comment) => {
     if (!comment) return;
+    if (
+      comment.resolvedTime !== undefined ||
+      comment.resolvedByEmail !== undefined ||
+      comment.resolvedByName !== undefined
+    ) {
+      trackedChangeResolutionSnapshots.set(comment, {
+        resolvedTime: comment.resolvedTime ?? null,
+        resolvedByEmail: comment.resolvedByEmail ?? null,
+        resolvedByName: comment.resolvedByName ?? null,
+      });
+    }
     // Sets the resolved state to null so it can be restored in the comments sidebar
     comment.resolvedTime = null;
     comment.resolvedByEmail = null;
@@ -981,18 +993,18 @@ export const useCommentsStore = defineStore('comments', () => {
     // History replay can opt in to excluding resolved tracked-change threads so
     // undo/redo reopens them when their marks reappear. Initial import rebuilds
     // keep resolved IDs in the set so resolved DOCX threads do not reopen on load.
-    const existingUnresolvedIds = new Set();
+    const skipIds = new Set();
     commentsList.value.forEach((comment) => {
       if (!comment?.trackedChange) return;
       if (!belongsToDocument(comment, activeDocumentId)) return;
       if (comment.resolvedTime && !reopenResolved) {
-        if (comment.commentId != null) existingUnresolvedIds.add(String(comment.commentId));
-        if (comment.importedId != null) existingUnresolvedIds.add(String(comment.importedId));
+        if (comment.commentId != null) skipIds.add(String(comment.commentId));
+        if (comment.importedId != null) skipIds.add(String(comment.importedId));
         return;
       }
       if (comment.resolvedTime) return;
-      if (comment.commentId != null) existingUnresolvedIds.add(String(comment.commentId));
-      if (comment.importedId != null) existingUnresolvedIds.add(String(comment.importedId));
+      if (comment.commentId != null) skipIds.add(String(comment.commentId));
+      if (comment.importedId != null) skipIds.add(String(comment.importedId));
     });
 
     // Build a Map of change ID → tracked change entries for O(1) lookup per group.
@@ -1009,7 +1021,7 @@ export const useCommentsStore = defineStore('comments', () => {
     // Build comment params directly from grouped changes — no PM dispatch needed
     groupedChanges.forEach(({ insertedMark, deletionMark, formatMark }) => {
       const id = insertedMark?.mark.attrs.id || deletionMark?.mark.attrs.id || formatMark?.mark.attrs.id;
-      if (!id || existingUnresolvedIds.has(id)) return;
+      if (!id || skipIds.has(id)) return;
 
       const marks = {
         ...(insertedMark && { insertedMark: insertedMark.mark }),
@@ -1030,9 +1042,9 @@ export const useCommentsStore = defineStore('comments', () => {
 
       if (params) {
         handleTrackedChangeUpdate({ superdoc, params });
-        existingUnresolvedIds.add(String(id));
-        if (params.changeId != null) existingUnresolvedIds.add(String(params.changeId));
-        if (params.importedId != null) existingUnresolvedIds.add(String(params.importedId));
+        skipIds.add(String(id));
+        if (params.changeId != null) skipIds.add(String(params.changeId));
+        if (params.importedId != null) skipIds.add(String(params.importedId));
       }
     });
 
@@ -1096,6 +1108,15 @@ export const useCommentsStore = defineStore('comments', () => {
       const hasLiveImportedId = Boolean(importedId && liveTrackedChangeIds.has(importedId));
 
       if ((!commentId && !importedId) || hasLiveCommentId || hasLiveImportedId) return true;
+      if (comment.resolvedTime) return true;
+
+      const resolutionSnapshot = trackedChangeResolutionSnapshots.get(comment);
+      if (resolutionSnapshot) {
+        comment.resolvedTime = resolutionSnapshot.resolvedTime ?? Date.now();
+        comment.resolvedByEmail = resolutionSnapshot.resolvedByEmail ?? null;
+        comment.resolvedByName = resolutionSnapshot.resolvedByName ?? null;
+        return true;
+      }
 
       if (commentId) removedIds.add(commentId);
       if (importedId) removedIds.add(importedId);

packages/superdoc/src/stores/comments-store.test.js

@@ -940,6 +940,59 @@ describe('comments-store', () => {
     expect(editorDispatch).toHaveBeenCalledWith(tr);
   });
 
+  it('preserves tracked-change thread across accept undo redo undo history replay', () => {
+    const editorDispatch = vi.fn();
+    const tr = { setMeta: vi.fn() };
+    const superdoc = { emit: vi.fn() };
+    const editor = {
+      state: {},
+      view: { state: { tr }, dispatch: editorDispatch },
+      options: { documentId: 'doc-1' },
+    };
+
+    const rootComment = {
+      commentId: 'tc-history-replay',
+      trackedChange: true,
+      trackedChangeText: 'Existing',
+      resolvedTime: 123,
+      resolvedByEmail: 'reviewer@example.com',
+      resolvedByName: 'Reviewer',
+      fileId: 'doc-1',
+      getValues: vi.fn(() => ({ commentId: 'tc-history-replay' })),
+    };
+    const replyComment = {
+      commentId: 'tc-history-replay-reply',
+      parentCommentId: 'tc-history-replay',
+      fileId: 'doc-1',
+    };
+    store.commentsList = [rootComment, replyComment];
+
+    // undo: accepted mark returns, thread reopens
+    trackChangesHelpersMock.getTrackChanges.mockReturnValueOnce([{ mark: { attrs: { id: 'tc-history-replay' } } }]);
+    groupChangesMock.mockReturnValueOnce([{ insertedMark: { mark: { attrs: { id: 'tc-history-replay' } } } }]);
+    store.syncTrackedChangeComments({ superdoc, editor });
+
+    expect(rootComment.resolvedTime).toBeNull();
+    expect(store.commentsList).toHaveLength(2);
+
+    // redo: accepted mark removed again, thread should not be deleted
+    trackChangesHelpersMock.getTrackChanges.mockReturnValueOnce([]);
+    groupChangesMock.mockReturnValueOnce([]);
+    store.syncTrackedChangeComments({ superdoc, editor });
+
+    expect(store.commentsList).toHaveLength(2);
+    expect(store.commentsList.find((comment) => comment.commentId === 'tc-history-replay')).toBeTruthy();
+    expect(store.commentsList.find((comment) => comment.commentId === 'tc-history-replay-reply')).toBeTruthy();
+
+    // next undo: same original thread reopens, no rematerialized replacement thread
+    trackChangesHelpersMock.getTrackChanges.mockReturnValueOnce([{ mark: { attrs: { id: 'tc-history-replay' } } }]);
+    groupChangesMock.mockReturnValueOnce([{ insertedMark: { mark: { attrs: { id: 'tc-history-replay' } } } }]);
+    store.syncTrackedChangeComments({ superdoc, editor });
+
+    expect(store.commentsList.filter((comment) => comment.commentId === 'tc-history-replay')).toHaveLength(1);
+    expect(store.commentsList.filter((comment) => comment.commentId === 'tc-history-replay-reply')).toHaveLength(1);
+  });
+
   it('keeps tracked-change comments when importedId is live even if commentId differs', () => {
     const editorDispatch = vi.fn();
     const tr = { setMeta: vi.fn() };