fix: Strip comments for diffing (#13212)

* fix: Strip comments for diffing

* feedback

shared/editor/lib/ChangesetHelper.test.ts

@@ -166,6 +166,65 @@ describe("ChangesetHelper.getChangeset", () => {
     });
   });
 
+  describe("comment marks", () => {
+    /**
+     * Builds a paragraph where the given word carries a comment mark.
+     */
+    function commented(
+      before: string,
+      word: string,
+      after: string
+    ): ProsemirrorData {
+      return {
+        type: "doc",
+        content: [
+          {
+            type: "paragraph",
+            content: [
+              { type: "text", text: before },
+              {
+                type: "text",
+                text: word,
+                marks: [
+                  { type: "comment", attrs: { id: "comment-id", userId: "u" } },
+                ],
+              },
+              { type: "text", text: after },
+            ],
+          },
+        ],
+      };
+    }
+
+    it("ignores a comment mark added to otherwise unchanged text", () => {
+      const changes = changesFor(
+        commented("Hello ", "brave", " world"),
+        para("Hello brave world")
+      );
+
+      expect(changes).toHaveLength(0);
+    });
+
+    it("ignores a comment mark removed from otherwise unchanged text", () => {
+      const changes = changesFor(
+        para("Hello brave world"),
+        commented("Hello ", "brave", " world")
+      );
+
+      expect(changes).toHaveLength(0);
+    });
+
+    it("still reports text changes within commented text", () => {
+      const changes = changesFor(
+        commented("Hello ", "bold", " world"),
+        commented("Hello ", "brave", " world")
+      );
+
+      expect(changes).toHaveLength(1);
+      expect(deletedText(changes[0])).toBe("brave");
+    });
+  });
+
   it("does not affect a simple single-word change", () => {
     const changes = changesFor(
       para("Hello modified world"),

shared/editor/lib/ChangesetHelper.ts

@@ -1,5 +1,5 @@
 import type { Mark, Slice } from "prosemirror-model";
-import { Node, Schema } from "prosemirror-model";
+import { Fragment, Node, Schema } from "prosemirror-model";
 import type { Change, TokenEncoder } from "prosemirror-changeset";
 import { ChangeSet, simplifyChanges } from "prosemirror-changeset";
 import { ReplaceStep, type Step } from "prosemirror-transform";
@@ -157,6 +157,32 @@ function mergeInterleavedChanges<T extends { step: Step; slice: Slice | null }>(
   return result;
 }
 
+/**
+ * Marks that carry no document content and should not be surfaced as changes.
+ */
+const IGNORED_MARKS = ["comment"];
+
+/**
+ * Recursively removes marks that are irrelevant to a diff from a node, so that
+ * adding or removing one does not render as a change to the text it covers.
+ *
+ * @param node - The node to strip marks from.
+ * @returns an equivalent node without the ignored marks.
+ */
+function removeIgnoredMarks(node: Node): Node {
+  const marks = node.marks.filter(
+    (mark) => !IGNORED_MARKS.includes(mark.type.name)
+  );
+
+  if (node.isText || !node.childCount) {
+    return node.mark(marks);
+  }
+
+  const children: Node[] = [];
+  node.content.forEach((child) => children.push(removeIgnoredMarks(child)));
+  return node.copy(Fragment.fromArray(children)).mark(marks);
+}
+
 /**
  * Represents a modification (attribute change) in the document.
  */
@@ -268,8 +294,15 @@ export class ChangesetHelper {
       });
 
       // Parse documents from JSON (old = previous revision, new = current revision)
-      const docOld = Node.fromJSON(schema, previousRevision);
-      const docNew = Node.fromJSON(schema, revision);
+      const original = Node.fromJSON(schema, revision);
+
+      // Diffing runs against copies without the ignored marks. Stripping marks
+      // leaves every position unchanged, so the resulting changes still line up
+      // with the original document.
+      const docOld = removeIgnoredMarks(
+        Node.fromJSON(schema, previousRevision)
+      );
+      const docNew = removeIgnoredMarks(original);
 
       // Calculate the transform and changeset
       const tr = recreateTransform(docOld, docNew, {
@@ -421,7 +454,7 @@ export class ChangesetHelper {
 
       return {
         changes: extendedChanges,
-        doc: tr.doc,
+        doc: original,
       };
     } catch {
       return null;