fix(undo): ignore revived tree metadata in remote diffs

[camdenslade]

Tried my best to trace this one, think I ended up getting it but very open to criticism.

During import, tree checkout can emit Delete+Create for the same live node when reconciling from the LCA to the import frontier. That marks the node's associated metadata container as new and emits full-state bring-back diffs for the metadata subtree.

Those synthetic diffs look like real concurrent insertions to the undo transform, causing cursor positions to shift incorrectly and undo() to return false for nested text edits.

Detect same-target Delete+Create tree diffs, collect their associated metadata containers, and exclude those containers and path descendants from remote_diff composition.

Should fix #915.

[Leeeon233]

Hi @camdenslade Apologies for the very delayed reply.

I don’t think this is the most fundamental fix. For example, I had GPT find a test case that still fails.

import { describe, expect, test } from "vitest";
import { LoroDoc, LoroText, UndoManager } from "loro-crdt";

test("undo tree text after remote edit mixed with independent import", () => {
  // A: base doc with tree node meta text = "Hello"
  const docA = new LoroDoc();
  docA.setPeerId("1");

  const treeA = docA.getTree("tree");
  treeA.enableFractionalIndex(0);

  const nodeA = treeA.createNode(undefined, 0);
  nodeA.data.set("kind", "paragraph");

  const textA = nodeA.data.setContainer("content", new LoroText());
  textA.insert(0, "Hello");
  docA.commit();

  const baseVv = docA.version();
  const baseUpdates = docA.export({ mode: "update" });

  // A local undo target: append " world"
  const undoA = new UndoManager(docA, { mergeInterval: 0 });
  textA.insert(5, " world");
  docA.commit();

  expect(textA.toString()).toBe("Hello world");
  expect(undoA.canUndo()).toBe(true);

  // C branches from A's base and makes a real concurrent edit
  // under the same tree metadata subtree.
  const docC = new LoroDoc();
  docC.setPeerId("2");
  docC.import(baseUpdates);

  const textC = docC.getByPath("tree/0@1/content") as LoroText;
  textC.insert(0, "Hi ");
  docC.commit();

  // D is independent and creates a node in the same tree.
  // Importing D into C makes A's later import from C contain both:
  // - synthetic Delete+Create/revival for A's tree node metadata
  // - real remote edit: Insert "Hi "
  const docD = new LoroDoc();
  docD.setPeerId("3");

  const treeD = docD.getTree("tree");
  treeD.enableFractionalIndex(0);

  const nodeD = treeD.createNode(undefined, 0);
  nodeD.data.set("kind", "from-d");
  docD.commit();

  docC.import(docD.export({ mode: "update" }));

  // A imports C's updates since the shared base in one import call.
  docA.import(docC.export({ mode: "update", from: baseVv }));

  expect(textA.toString()).toBe("Hi Hello world");
  expect(undoA.canUndo()).toBe(true);

  const result = undoA.undo();

  expect(result).toBe(true);
  expect(textA.toString()).toBe("Hi Hello");   //   Hi Herld
});

Concurrent edits and tree metadata revival make things tricky, but I may have a rough idea of how to fix it.

[camdenslade]

Yeah I put a comment in the commit noting that as unhandled. From my understanding, the issue is that by the time the undo manager sees the ContainerDiff, the bring-back content and the real remote edit have already been composed together into a single diff in diffs_to_event. So excluding the container entirely drops the real edit as well, and not excluding it lets the synthetic positions corrupt the undo transform. The solution is an upstream one for sure but maybe the PR could give some added context or something.