feat(versioning): diff nesting changes in place

examples/07-collaboration/14-suggestion-gallery/src/scenarios.ts

@@ -1021,7 +1021,7 @@ export const scenarios: SuggestionScenario[] = [
     feedback: [
       {
         severity: "low",
-        note: "Block N1 appears in two places. Previously this concurrency scenario would also not be correctly handled (one of the edits would be dropped).",
+        note: "N2 is lost: B's indent moves it into the original N1, which A's indent deletes. Versioning shows N2 deleted without an author (B only moved it, and A never saw it there). Before nesting changes diffed in place, N1 appeared twice instead. Needs move support.",
       },
     ],
     title: "Cascading indents",
@@ -1047,7 +1047,7 @@ export const scenarios: SuggestionScenario[] = [
     feedback: [
       {
         severity: "high",
-        note: "Q appears twice: B's indent moves a copy of Q under R, and A's nesting replaces the original Q with another copy holding B1–B3.",
+        note: "B1–B3 are lost. A's indent writes them into the original Q, which B's indent concurrently deletes (a move inserts a copy of Q, without them). Versioning shows them deleted without an author. Before nesting changes diffed in place, Q appeared twice instead. Needs move support.",
       },
     ],
     title: "Nest blocks into a block that is moved",
@@ -1103,8 +1103,8 @@ export const scenarios: SuggestionScenario[] = [
     id: "concurrent-move-into-deleted-block",
     feedback: [
       {
-        severity: "high",
-        note: "X is lost: B's move inserts a copy into Parent, which A deletes. Versioning shows X as deleted by B, though B only moved it, and doesn't show Child at all. To be fixed by #3166.",
+        severity: "info",
+        note: "X is lost: B's move inserts a copy into Parent, which A deletes. Versioning shows X deleted without an author: B only moved it, and A never saw it there.",
       },
     ],
     title: "Move a block into a block that is deleted",
@@ -1133,8 +1133,8 @@ export const scenarios: SuggestionScenario[] = [
     id: "concurrent-delete-parent-vs-child-type",
     feedback: [
       {
-        severity: "high",
-        note: "B's type change is lost with Parent. Versioning shows Parent as deleted by A, but doesn't show Child at all. To be fixed by #3166.",
+        severity: "info",
+        note: "B's type change is lost with Parent. Versioning shows Parent deleted by A, and Child deleted without an author: B's change replaced Child with a copy that was lost, and the diff can't tell who to credit.",
       },
     ],
     title: "Delete a parent vs change its child's type",

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

@@ -297,8 +297,7 @@ describe("AttributionExtension user loading", () => {
     });
   });
 
-  // To be fixed by #3166.
-  it.fails("does not show a change to a block's id", () => {
+  it("does not show a change to a block's id", () => {
     const { editor } = createEditor();
     editor.replaceBlocks(editor.document, [
       { type: "paragraph", content: "hello" },

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

@@ -230,19 +230,16 @@ export const AttributionExtension = createExtension(
             .map((id) => userStore.getUser(id)?.username ?? id)
             .filter((username) => username !== "");
 
-        // A stable identity string for a wrapper (empty if unattributed), used to
-        // (a) test whether a mark is attributed and (b) group adjacent marks with
-        // the *same* attribution under one tooltip. It's an internal grouping key,
-        // not the displayed text — that's composed in the view from `users` and
-        // the format label — so it's built from raw `data-*` (ids + format keys)
-        // and stays free of i18n/username resolution.
+        // A stable identity string for a mark wrapper, used to group nested marks
+        // with the *same* change under one tooltip. Every wrapper is a change;
+        // its author may be unknown (e.g. content removed along with a
+        // concurrently deleted block). It's an internal grouping key, not the
+        // displayed text, so it's built from raw `data-*` and stays free of
+        // i18n/username resolution.
         const attributionIdentity = (wrapper: HTMLElement) => {
           const ids = parseUserIds(wrapper.dataset["userIds"]);
-          if (ids.length === 0 && wrapper.dataset["attributes"] === undefined) {
-            return "";
-          }
           const format = parseFormatKeys(wrapper.dataset["format"]);
-          return `${wrapper.dataset["attributes"] ?? ""}:${format.join(",")}:${ids.join(",")}`;
+          return `${wrapper.tagName}:${wrapper.dataset["attributes"] ?? ""}:${format.join(",")}:${ids.join(",")}`;
         };
 
         // Build the tooltip state from a wrapper's `data-*` attributes. A
@@ -302,24 +299,6 @@ export const AttributionExtension = createExtension(
           store.setState(undefined);
         };
 
-        // The innermost attributed mark at or above `el`, skipping unattributed
-        // wrappers so an attributed ancestor still wins.
-        const innermostAttributed = (
-          el: Element | null,
-        ): HTMLElement | undefined => {
-          while (el) {
-            const wrapper = el.closest<HTMLElement>(ATTRIBUTION_MARK_SELECTOR);
-            if (!wrapper) {
-              return undefined;
-            }
-            if (attributionIdentity(wrapper)) {
-              return wrapper;
-            }
-            el = wrapper.parentElement;
-          }
-          return undefined;
-        };
-
         const nodeAttribution = (
           target: Element,
         ): { mark: HTMLElement; preview: Element } | undefined => {
@@ -356,17 +335,18 @@ export const AttributionExtension = createExtension(
           ) {
             return undefined;
           }
-          const mark = Array.from(
-            owner.querySelectorAll<HTMLElement>(ATTRIBUTION_MARK_SELECTOR),
-          ).find(attributionIdentity);
+          const mark = owner.querySelector<HTMLElement>(
+            ATTRIBUTION_MARK_SELECTOR,
+          );
           return mark ? { mark, preview: owner } : undefined;
         };
 
         const onPointerOver = (event: Event) => {
           const target = event.target instanceof Element ? event.target : null;
           const hoveredMark =
             target && dom.contains(target)
-              ? innermostAttributed(target)
+              ? (target.closest<HTMLElement>(ATTRIBUTION_MARK_SELECTOR) ??
+                undefined)
               : undefined;
           const fallback =
             target && !hoveredMark ? nodeAttribution(target) : undefined;
@@ -379,21 +359,18 @@ export const AttributionExtension = createExtension(
 
           const identity = attributionIdentity(innermost);
           // Anchor on the outermost ancestor with the *same* attribution so one
-          // tooltip covers the whole region; a differently-attributed ancestor
-          // breaks the chain, and unattributed ones are climbed past.
+          // tooltip covers the whole region; a different ancestor breaks the chain.
           let anchor = innermost;
           let el: Element | null = innermost.parentElement;
           while (el) {
             const ancestor = el.closest<HTMLElement>(ATTRIBUTION_MARK_SELECTOR);
             if (!ancestor) {
               break;
             }
-            const ancestorIdentity = attributionIdentity(ancestor);
-            if (ancestorIdentity === identity) {
-              anchor = ancestor;
-            } else if (ancestorIdentity) {
+            if (attributionIdentity(ancestor) !== identity) {
               break;
             }
+            anchor = ancestor;
             el = ancestor.parentElement;
           }
 

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

@@ -112,6 +112,16 @@ const createAttributionMarkView =
   ) =>
   ({ mark, inline }: { mark: PMMark; inline: boolean }) => {
     const editor = options?.editor;
+    // A block's id isn't content users edit, so a change to it isn't shown
+    // (e.g. a diff pairing a deleted block with the editor's empty one).
+    // Inline content may have an `id` prop of its own, which is.
+    const { id: _id, ...blockChanges } = getAttributeChanges(mark);
+    const changes = inline ? getAttributeChanges(mark) : blockChanges;
+    if (type === "attrs" && Object.keys(changes).length === 0) {
+      const dom = document.createElement(inline ? "span" : "div");
+      dom.style.display = "contents";
+      return { dom, contentDOM: dom };
+    }
     // `<ins>`/`<del>` are semantic elements. The modification mark has no
     // dedicated element, so it renders as a `<span>` inline or a `<div>` over a
     // block, matching its `parseDOM` rules.
@@ -131,7 +141,7 @@ const createAttributionMarkView =
     });
     if (type === "attrs") {
       dom.dataset["type"] = "attributes";
-      dom.dataset["attributes"] = JSON.stringify(getAttributeChanges(mark));
+      dom.dataset["attributes"] = JSON.stringify(changes);
     }
     if (type === "modification") {
       dom.dataset["type"] = "modification";

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

@@ -9,6 +9,7 @@ import {
   createExtension,
 } from "../../editor/BlockNoteExtension.js";
 import { blockMatchNodes } from "./blockMatchNodes.js";
+import { mergeBlockGroups } from "./mergeBlockGroups.js";
 import { docToBlocks } from "../../api/nodeConversions/nodeToBlock.js";
 import type {
   BlockSchema,
@@ -178,6 +179,8 @@ export const YSyncExtension = createExtension(
           // needed; `blockContainer` already whitelists the `y-attributed-*`
           // marks. See blockMatchNodes.ts.
           customCompare: blockMatchNodes,
+          // Nesting changes diff in place: show a block's groups as one group
+          transformers: [mergeBlockGroups],
           // Initial-empty gate: a single empty paragraph (any id/props) must
           // not seed an empty Y fragment — see isInitialBlockNoteDoc above
           // and "Initial-content gate" in ProsemirrorRdt's doc.

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

@@ -23,24 +23,6 @@ const firstChild = (
   return null;
 };
 
-/**
- * Whether a `blockContainer` delta carries a child `blockGroup` — i.e. the block
- * has nested children. A container's content is `blockContent blockGroup?`, so
- * this is what tells a leaf block apart from a parent.
- */
-const hasBlockGroup = (d: schema.Unwrap<typeof $prosemirrorDelta>): boolean => {
-  for (const op of (d as any).children) {
-    if (delta.$insertOp.check(op)) {
-      for (const it of op.insert) {
-        if (delta.$deltaAny.check(it) && it.name === "blockGroup") {
-          return true;
-        }
-      }
-    }
-  }
-  return false;
-};
-
 function getTableDimensions(
   d: schema.Unwrap<typeof $prosemirrorDelta>,
 ): { rows: number; cols: number } | null {
@@ -155,16 +137,6 @@ export const blockMatchNodes = (
     return false;
   }
 
-  // A change in nesting is structural too: if one container gains or loses a
-  // child `blockGroup`, diffing it in place would insert/delete the blockGroup as
-  // a sibling of the block content inside a single container — schema-invalid.
-  // Treat it as different so the whole container is replaced instead, same as a
-  // content-type change. Keeps concurrent nesting merges (e.g. two users nesting
-  // a block under the same parent) from producing a lopsided in-place result.
-  if (hasBlockGroup(a) !== hasBlockGroup(b)) {
-    return false;
-  }
-
   if (childA?.name === "table" && childB?.name === "table") {
     const dimA = getTableDimensions(childA);
     const dimB = getTableDimensions(childB);

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

@@ -383,8 +383,7 @@ describe("legacy Yjs document binding", () => {
     ]);
   });
 
-  // To be fixed by #3167.
-  it.fails.each(structuralChanges)(
+  it.each(structuralChanges)(
     "diffs $name made with the old binding like one made with the new binding",
     ({ blocks, change }) => {
       const legacy = createLegacyEditor();