Skip to content

fix: keep exact common ancestor when checking out to the past - #1071

Closed
lodystage[bot] wants to merge 3 commits into
mainfrom
fix/fork-at-full-history-replay
Closed

fix: keep exact common ancestor when checking out to the past#1071
lodystage[bot] wants to merge 3 commits into
mainfrom
fix/fork-at-full-history-replay

Conversation

@lodystage

@lodystage lodystage Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #1056forkAt(frontiers) hanging the browser with no error on large documents.

_find_common_ancestor discarded the ancestor it had just computed and returned empty
frontiers whenever a branch of its DAG walk reached a root without meeting the other
side. Checking out to a version already in the past — exactly what forkAt does —
always ends that way:

find_common_ancestor([1:10, 2:5], [1:10]) -> Frontiers([])   // should be [1:10]

The conservative fallback is now skipped when the computed ancestor is exactly the
target version.

Rationale

An empty replay base is not a constant-factor cost, it is document-sized and
multiplicative: the causal iterator walks the entire oplog, so every container in
the document gets a diff calculator, and each of those independently re-walks the whole
oplog to rebuild its tracker — O(containers x ops).

When ans == right, every id of right is reachable from left, so right is the
maximal common ancestor and there is nothing to be conservative about. It is also the
safe direction for the trackers: from_vv then strictly contains to_vv, so the
retreat is never empty and the richtext/list/movable-list trackers are still rebuilt
from CRDT ids rather than advanced incrementally.

The symmetric relaxation (ans == left, fast-forwarding past a concurrent branch) is
not safe — it has no retreat to force a rebuild, and undo_tree in crates/fuzz
fails under it. The code comment names that test so the next person does not retry it.

The regression came from b81abfce (#974) in 1.13.7, which is why 1.13.6 is unaffected.

Validation

  • Full gate cargo test --features=test_utils,jsonpath --no-fail-fast: 73 groups,
    1417 tests, 0 failures

  • crates/fuzz 128 tests including the undo_tree canary

  • New common_ancestor_of_subset_target_is_the_target_itself (dag unit test) and
    test_fork_at_with_concurrent_frontiers (fork / checkout / attach / re-import)

  • Perf harness crates/examples/examples/fork_at_many_containers.rs, 1600 containers /
    194k ops:

    1.13.6 1.13.7 this PR
    checkout 46.7ms 268ms 1.45ms
    forkAt 193ms 679ms 346ms

Known limitation

forkAt stays above 1.13.6 because it internally checks back to the original version,
which is the ans == left direction that cannot be relaxed safely. Separately, a single
large text container is still ~2x slower than 1.12 due to the unconditional tracker
rebuild b81abfce added. Both are recorded in context/CONTEXT-GAPS.md, along with
four relaxations of that rebuild that were measured and ruled out — including the trap
that an oracle checking only a single checkout wrongly reports the rebuild as redundant.

Background and the cost model live in the new context/diff-calc-replay-base.md.

🤖 Generated with Claude Code

zxch3n and others added 3 commits August 11, 2026 08:32
`_find_common_ancestor` discarded the ancestor it had just computed and
returned empty frontiers whenever a branch of its DAG walk ended without
meeting the other side. Checking out to a version already in the past --
what `fork_at(frontiers)` does -- always ends that way, so the diff
calculator replayed from the start of history, waking a calculator for
every container in the document. That is O(containers x ops) and hangs
large documents in the browser.

Skip the conservative fallback when the computed ancestor is exactly the
target: every id of the target is then reachable from the source, so it
is provably the maximal common ancestor. It is also the safe direction
for the trackers, since the retreat is never empty there.

Rationale and the measured cost model live in
context/diff-calc-replay-base.md.

Closes #1056

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four ways of relaxing the richtext/list `should_rebuild` were measured and
reverted while following up on #1056. Each looks obviously correct on
paper, so record the evidence that rules them out - including that an
oracle which only checks a single checkout's output wrongly concludes the
rebuild is unnecessary.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rebasing onto main brought in #1058, which reworks replay-base selection
around critical versions: `all_tips_covered_by_ancestors` tells a genuine
concurrent branch apart from a redundant path into a known ancestor, and
`latest_single_head_critical_version` retreats to the latest safe cut
instead of to empty frontiers. That subsumes the fix this branch carried,
so the conflicting hunk in `dag.rs` was resolved in favour of main.

The reasoning this branch used was also wrong, not merely redundant: it
treated the meet of the two versions as a safe base whenever the target
was already in the source's past. For `current = [10@1, 5@2]` with peer 2
branching at `3@1`, the meet `10@1` is not a critical version, because
`0@2..5@2` is concurrent with `4@1..10@1`. The unit test now asserts the
branch point `3@1`, and separately that the base is not empty, so it still
guards the regression it was written for.

Drop the changeset: no user-visible behaviour is left in this branch.
Rewrite the falsified section of the replay-base article to point at
`docs/critical-version-spec.md`, and note the bad argument so it is not
retried.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lodystage
lodystage Bot force-pushed the fix/fork-at-full-history-replay branch from cf583b5 to a4b6d20 Compare August 11, 2026 08:43
@lodystage

lodystage Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #1058, which reworked replay-base selection around critical versions — all_tips_covered_by_ancestors distinguishes a genuine concurrent branch from a redundant path into a known ancestor, and latest_single_head_critical_version retreats to the latest safe cut instead of to empty frontiers. That covers the #1056 hang this branch targeted.

The argument here was also unsound, not just redundant: it treated the meet of the two versions as a safe replay base whenever the target was already in the source's past. For current = [10@1, 5@2] with peer 2 branching at 3@1, the meet 10@1 is not a critical version, since 0@2..5@2 is concurrent with 4@1..10@1. It passed the fuzz corpus by luck rather than by correctness.

Closing rather than reworking. Nothing here is needed on top of #1058.

@lodystage lodystage Bot closed this Aug 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

WASM Size Report

  • Original size: 3104.95 KB
  • Gzipped size: 1031.21 KB
  • Brotli size: 722.44 KB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

forkAt breaks in 1.13.7, no error, browser just locks up when doing fork with frontiers

1 participant