fix(versioning): diff structural changes made by the old Yjs binding

packages/core/src/y/extensions/blockMatchNodes.ts

@@ -136,6 +136,8 @@ function getTableDimensions(
  * @param b inserted (new) node
  * @returns whether `a` and `b` are the same node (diff in place) vs different (replace)
  */
+const replaceReshapedTables = false;
+
 export const blockMatchNodes = (
   a: schema.Unwrap<typeof $prosemirrorDelta>,
   b: schema.Unwrap<typeof $prosemirrorDelta>,
@@ -165,7 +167,16 @@ export const blockMatchNodes = (
     return false;
   }
 
-  if (childA?.name === "table" && childB?.name === "table") {
+  // Disabled: replacing a table that one edit reshapes both ways silently
+  // dropped concurrent edits to it, and version diffs compare all the changes
+  // between two versions, so concurrent one-way reshapes (a row and a column)
+  // replaced the whole table there. Reshapes merge in place, as one-way ones
+  // always did.
+  if (
+    replaceReshapedTables &&
+    childA?.name === "table" &&
+    childB?.name === "table"
+  ) {
     const dimA = getTableDimensions(childA);
     const dimB = getTableDimensions(childB);
     if (

packages/core/src/y/extensions/legacyYjsDocBinding.test.ts

@@ -458,8 +458,7 @@ describe("legacy Yjs document binding", () => {
     };
   }
 
-  // To be fixed by #3173.
-  it.fails.each(structuralChanges)(
+  it.each(structuralChanges)(
     "diffs $name made with the old binding like one made with the new binding",
     ({ blocks, change }) => {
       const { current, old } = diffsOfBothBindings(blocks, change);
@@ -468,8 +467,7 @@ describe("legacy Yjs document binding", () => {
     },
   );
 
-  // To be fixed by #3173.
-  it.fails("diffs a table resize made with the old binding like one made with the new binding", () => {
+  it("diffs a table resize made with the old binding like one made with the new binding", () => {
     const { current, old } = diffsOfBothBindings(
       [table(2, 2)],
       (editor) => editor.updateBlock("table", table(3, 3)),
@@ -496,8 +494,7 @@ describe("legacy Yjs document binding", () => {
     expect(Y.encodeStateAsUpdateV2(opened.doc)).toEqual(stored);
   });
 
-  // To be fixed by #3173.
-  it.fails("credits a type change made with the old binding only to whoever made it", () => {
+  it("credits a type change made with the old binding only to whoever made it", () => {
     const legacy = createLegacyEditor();
     legacy.editor.replaceBlocks(legacy.editor.document, [
       { id: "changed", type: "paragraph", content: "Text" },
@@ -529,6 +526,40 @@ describe("legacy Yjs document binding", () => {
     ]);
   });
 
+  it("credits text typed after an old-binding type change to whoever typed it", () => {
+    const legacy = createLegacyEditor();
+    legacy.editor.replaceBlocks(legacy.editor.document, [
+      { id: "changed", type: "paragraph", content: "Text" },
+    ]);
+    const before = Y1.encodeStateAsUpdateV2(legacy.doc);
+    legacy.editor.updateBlock("changed", { type: "heading" });
+    const byBob = Y1.encodeStateAsUpdateV2(legacy.doc);
+    legacy.editor.setTextCursorPosition("changed", "end");
+    legacy.editor.insertInlineContent(" by Alice");
+    const after = Y1.encodeStateAsUpdateV2(legacy.doc);
+    const opened = openWithNewBinding(after);
+
+    const changed = diffBlocks(
+      opened.editor,
+      opened.doc,
+      before,
+      after,
+      "all",
+      attributionsOfSteps(before, [
+        { state: byBob, user: "bob", time: 2000 },
+        { state: after, user: "alice", time: 3000 },
+      ]),
+    );
+    expect(
+      changed
+        .filter(({ type }) => type === "text")
+        .map(({ change, text, users }) => [change, text, users]),
+    ).toEqual([
+      ["y-attributed-insert", "Text", ["bob"]],
+      ["y-attributed-insert", " by Alice", ["alice"]],
+    ]);
+  });
+
   it("diffs a text edit made with the old binding in place", () => {
     const legacy = createLegacyEditor();
     legacy.editor.replaceBlocks(legacy.editor.document, [

packages/core/src/y/extensions/snapshotPreview.ts

@@ -2,11 +2,214 @@ import { configureYProsemirror } from "@y/prosemirror";
 import * as Y from "@y/y";
 
 import type { BlockNoteEditor } from "../../editor/BlockNoteEditor.js";
+import { findTypeInOtherYdoc } from "../utils.js";
+import { blockMatchNodes } from "./blockMatchNodes.js";
 import {
   decodeFragmentUpdate,
   destroyDecodedFragment,
 } from "./snapshotCodec.js";
 
+/** Whether the item with this id was already in `baseline`. */
+function inBaseline(baseline: Y.Doc, id: Y.ID): boolean {
+  const last = baseline.store.clients.get(id.client)?.at(-1);
+  return last !== undefined && id.clock < last.id.clock + last.length;
+}
+
+/**
+ * The attributions of what changed `block`'s structure since `baseline`: its
+ * content nodes and child groups that are new, or deleted, since then. Its
+ * other content (e.g. text typed in it) doesn't make the change.
+ */
+function structuralAttributions(
+  block: Y.Node,
+  baseline: Y.Doc,
+  attributions: Y.ContentMap,
+): { inserted: Y.ContentAttribute<any>[]; deleted: Y.ContentAttribute<any>[] } {
+  const inserted = new Map<string, Y.ContentAttribute<any>>();
+  const deleted = new Map<string, Y.ContentAttribute<any>>();
+  for (let item = block._start; item !== null; item = item.right) {
+    const isNew = !inBaseline(baseline, item.id);
+    if (!isNew && !item.deleted) {
+      continue;
+    }
+    const [found, map] = isNew
+      ? [inserted, attributions.inserts]
+      : [deleted, attributions.deletes];
+    for (const range of map.slice(item.id.client, item.id.clock, item.length)) {
+      for (const attr of range.attrs ?? []) {
+        found.set(`${attr.name}:${String(attr.val)}`, attr);
+      }
+    }
+  }
+  return { inserted: [...inserted.values()], deleted: [...deleted.values()] };
+}
+
+/**
+ * The units of a node's live content, in order: one per character or embed,
+ * and one per child node.
+ */
+function liveUnits(node: Y.Node): Array<{ id: Y.ID; node?: Y.Node }> {
+  const units: Array<{ id: Y.ID; node?: Y.Node }> = [];
+  for (let item = node._start; item !== null; item = item.right) {
+    if (item.deleted) {
+      continue;
+    }
+    if (item.content instanceof Y.ContentType) {
+      units.push({ id: item.id, node: item.content.type });
+      continue;
+    }
+    for (let i = 0; i < item.length; i++) {
+      units.push({ id: Y.createID(item.id.client, item.id.clock + i) });
+    }
+  }
+  return units;
+}
+
+/**
+ * Pair each unit and attribute of `original` with the same one in `copy`, its
+ * clone. Returns false if they don't line up, which a clone always should.
+ */
+function pairClone(
+  original: Y.Node,
+  copy: Y.Node,
+  pairs: Array<[Y.ID, Y.ID]>,
+): boolean {
+  for (const [key, item] of original._map) {
+    const cloned = copy._map.get(key);
+    if (!item.deleted) {
+      if (!cloned) {
+        return false;
+      }
+      pairs.push([item.id, cloned.id]);
+    }
+  }
+  const a = liveUnits(original);
+  const b = liveUnits(copy);
+  if (a.length !== b.length) {
+    return false;
+  }
+  return a.every((unit, i) => {
+    const other = b[i];
+    pairs.push([unit.id, other.id]);
+    return unit.node && other.node
+      ? pairClone(unit.node, other.node, pairs)
+      : !unit.node && !other.node;
+  });
+}
+
+/** Attributions as the other kind: `insert`/`insertAt` as `delete`/`deleteAt`. */
+function asKind(
+  attrs: Y.ContentAttribute<any>[],
+  kind: "insert" | "delete",
+): Y.ContentAttribute<any>[] {
+  return attrs.map((attr) =>
+    Y.createContentAttribute(
+      attr.name.replace(/^(insert|delete)/, kind),
+      attr.val,
+    ),
+  );
+}
+
+/**
+ * Replace each block that changed structurally since `baseline` with a fresh
+ * copy, where "structurally" is what `blockMatchNodes` treats as a different
+ * block (e.g. a type change). The old collaboration binding stored such changes
+ * inside the same container, which a diff renders as schema-invalid content
+ * that is then dropped. A fresh container diffs as a deleted block next to an
+ * inserted one, as the current binding stores it. The copy and the deletion
+ * are credited to whoever changed the block's structure (see
+ * {@link structuralAttributions}).
+ */
+function splitChangedBlocks(
+  node: Y.Node,
+  baseline: Y.Doc,
+  attributions: Y.ContentMap | undefined,
+  added: Y.ContentMap,
+): void {
+  for (let index = 0; index < node.length; index++) {
+    const child = node.get(index);
+    if (!(child instanceof Y.Node)) {
+      continue;
+    }
+    if (child.name === "blockContainer" && child._item) {
+      // A block created after the baseline has no previous version.
+      const before = inBaseline(baseline, child._item.id)
+        ? findTypeInOtherYdoc(child, baseline)
+        : undefined;
+      if (
+        before &&
+        !blockMatchNodes(before.toDeltaDeep(), child.toDeltaDeep())
+      ) {
+        const { inserted, deleted } = attributions
+          ? structuralAttributions(child, baseline, attributions)
+          : { inserted: [], deleted: [] };
+        // `node` is a decoded snapshot's, never the live document, so the split
+        // doesn't reach the stored document.
+        const doc = child.doc!;
+        // A change that only inserted (or only deleted) still credits both
+        // sides of the split, under that side's attribution kind.
+        const insertedAs = inserted.length
+          ? inserted
+          : asKind(deleted, "insert");
+        const deletedAs = deleted.length ? deleted : asKind(inserted, "delete");
+        // Copied content added after the baseline keeps its own author (e.g.
+        // text typed after the structural change).
+        const own: Y.IdMap<any> = Y.createIdMap();
+        const owned = Y.createIdSet();
+        function record(tr: Y.Transaction) {
+          if (insertedAs.length) {
+            Y.insertIntoIdMap(
+              added.inserts,
+              Y.mergeIdMaps([
+                Y.diffIdMap(
+                  Y.createIdMapFromIdSet(tr.insertSet, insertedAs),
+                  owned,
+                ),
+                own,
+              ]),
+            );
+            Y.insertIntoIdMap(
+              added.deletes,
+              Y.createIdMapFromIdSet(tr.deleteSet, deletedAs),
+            );
+          }
+        }
+        doc.on("beforeObserverCalls", record);
+        try {
+          doc.transact(() => {
+            const copy = child.clone();
+            node.insert(index + 1, [copy]);
+            // Paired while the original is still live.
+            const pairs: Array<[Y.ID, Y.ID]> = [];
+            if (attributions && pairClone(child, copy, pairs)) {
+              for (const [original, cloned] of pairs) {
+                const attrs = inBaseline(baseline, original)
+                  ? undefined
+                  : attributions.inserts.slice(
+                      original.client,
+                      original.clock,
+                      1,
+                    )[0]?.attrs;
+                if (attrs) {
+                  const ids = Y.createIdSet();
+                  ids.add(cloned.client, cloned.clock, 1);
+                  owned.add(cloned.client, cloned.clock, 1);
+                  Y.insertIntoIdMap(own, Y.createIdMapFromIdSet(ids, attrs));
+                }
+              }
+            }
+            node.delete(index);
+          });
+        } finally {
+          doc.off("beforeObserverCalls", record);
+        }
+        continue;
+      }
+    }
+    splitChangedBlocks(child, baseline, attributions, added);
+  }
+}
+
 /**
  * Decode a snapshot, diff it against a baseline if given, and render it.
  *
@@ -33,14 +236,32 @@ export function showSnapshotPreview(
   try {
     const snapshot = decodeFragmentUpdate(fragment, snapshotContent);
     try {
+      let renderAttributions = attributions;
+      if (baseline) {
+        const added = Y.createContentMap();
+        splitChangedBlocks(
+          snapshot.fragment,
+          baseline.doc,
+          attributions,
+          added,
+        );
+        if (attributions) {
+          renderAttributions = Y.createContentMap(
+            Y.mergeIdMaps([attributions.inserts, added.inserts]),
+            Y.mergeIdMaps([attributions.deletes, added.deletes]),
+          );
+        }
+      }
       editor.exec(
         configureYProsemirror({
           ytype: snapshot.fragment,
           renderer: baseline
             ? Y.createDiffRenderer(
                 baseline.doc,
                 snapshot.doc,
-                attributions ? { attributions } : undefined,
+                renderAttributions
+                  ? { attributions: renderAttributions }
+                  : undefined,
               )
             : undefined,
         }),

packages/core/src/y/extensions/tableReshape.test.ts

@@ -95,8 +95,7 @@ it("stores a table that one edit reshapes in two directions", () => {
   );
 });
 
-// To be fixed by #3173.
-it.fails("keeps a concurrent cell edit when a different user reshapes the table", () => {
+it("keeps a concurrent cell edit when a different user reshapes the table", () => {
   const { a, b, sync } = twoUsers();
   a.updateBlock("table", table(grown));
   b.updateBlock(