feat(undo): per-container UndoScope for UndoManager#1039
Open
edochi wants to merge 4 commits into
Open
Conversation
Introduces a new `UndoScope` enum that lets `UndoManager` filter local commits by the set of containers they touch. The default `UndoScope::Doc` preserves today's doc-wide behavior, so existing users see no change. When `UndoScope::Containers([cid, ...])` is set, local commits that touch zero in-scope containers follow the same path as origin-excluded commits: their effect is composed into both stacks via `compose_remote_event` (so cursor transforms remain accurate) and `next_counter` is advanced past them, keeping subsequent in-scope `CounterSpan`s tight. Two ergonomic forms are provided on `UndoManager`: - `set_scope(&self, scope)` for mutating an existing manager - `with_scope(self, scope) -> Self` for builder-style construction The scope check is inlined at the subscription-callback site so the optimizer can fold the `Doc` arm to a constant `false` and skip the iteration entirely in the default path. Bench (1000 commits/iter, n=50): default scope vs main: +45 ns/commit (~1.3%, near noise floor) scoped one container: identical to default within noise The replay-time half of the feature (masking the computed `DiffBatch` so mixed commits revert only their in-scope portion) follows in the next commit; this commit alone handles the simpler "all in or all out" case. Re-exports `UndoScope` from both `loro-internal` and the public `loro` crate, and adds `set_scope`/`with_scope` to the `loro::UndoManager` wrapper. Co-Authored-By: Claude <noreply@anthropic.com>
Adds the replay-time half of UndoScope. Without this, a single commit that touched both in-scope and out-of-scope containers would have its *entire* effect reverted at undo time — because the recorded CounterSpan covers the whole commit and `undo_internal` produces a diff covering every container in that range. The fix is small and local: after the diff is computed but before `_apply_diff` consumes it, mask `cid_to_events` and `order` to retain only entries whose ContainerID (after walking `container_remap`, to mirror `_apply_diff`'s own remap chain) is in the scope set. `_apply_diff` already iterates containers independently and tolerates missing entries, so dropping unwanted (cid, diff) pairs is structurally safe. To keep this non-breaking for any direct callers of `undo_internal`, the new behavior lives on a new method `undo_internal_with_scope` that takes an `Option<&FxHashSet<ContainerID>>`. The existing `undo_internal` is preserved as a thin `#[inline]` wrapper that delegates with `None`. `#[instrument]` lives on the new method only, so direct callers get exactly one tracing span (no duplicates). `UndoManager::perform()` snapshots the active scope (if any) per undo iteration and threads it through. Per-iteration snapshot is correct under any threading model in case `set_scope` is called concurrently. Bench: replay path is statistically identical to main (5.15s vs 5.17s for 1000 undo+redo cycles, well within Criterion's noise band). The mask is O(M) per undo where M is the number of containers in the diff — dominated by the existing O(n + m) checkout-and-diff cost. Co-Authored-By: Claude <noreply@anthropic.com>
Adds 9 integration tests in crates/loro-internal/tests/undo.rs:
- scope_default_is_doc_wide: UndoScope::Doc preserves historical behavior
- scope_excludes_out_of_scope_local_commits: filter at record time
- scope_redo_only_in_scope: redo respects scope symmetrically
- scope_out_of_scope_edits_do_not_corrupt_in_scope_stack: counter
bookkeeping stays correct when out-of-scope commits sit between
in-scope ones
- scope_mixed_commit_only_undoes_in_scope: replay-time mask works (the
killer case — without the mask, this would revert both containers)
- scope_mixed_commit_redo_only_restores_in_scope: redo of a mixed-undo
also stays in-scope
- scope_multiple_mixed_commits_chained: 3 chained mixed commits, undo
walks back through only the in-scope portion of each
- scope_mixed_commit_with_three_containers: scope = {a, c}, single
commit touches a + b + c, undo reverts a and c but leaves b
- scope_change_after_construction: set_scope mid-session; commits are
classified by the scope active at record time
All 16 tests in tests/undo.rs pass (7 baseline + 9 new). The 7 baseline
tests prove backward compatibility is preserved (default UndoScope::Doc
is a no-op).
Co-Authored-By: Claude <noreply@anthropic.com>
Adds a Criterion bench at crates/loro-internal/benches/undo.rs (gated on the test_utils feature, matching the existing benches in this crate) covering three groups: - record_local_commits — measures the subscription-callback hot path with no UndoManager, with a default-scope (Doc) UndoManager, and with a Containers-scoped UndoManager. - record_mixed_scope — interleaves in-scope and out-of-scope commits to measure how the compose-as-remote branch performs vs full recording. - replay_all — measures undo() + redo() of a full 1000-commit history, exercising undo_internal_with_scope end-to-end (with and without the scope mask block). Runs with: cargo bench -p loro-internal --bench undo --features test_utils Numbers on Apple Silicon (1000 commits/iter, n=50): | variant | time | per commit | |------------------------------------------|----------|------------| | no_manager | 2.26 ms | 2.26 µs | | undo_manager_default_scope (this branch) | 3.59 ms | 3.59 µs | | undo_manager_default_scope (main) | 3.54 ms | 3.54 µs | | undo_manager_scoped_one_container | 3.58 ms | 3.58 µs | | replay/doc_scope | 5.15 s | 2.57 ms/op | | replay/scoped_one_container | 5.18 s | 2.59 ms/op | The default-scope path is ~45 ns/commit slower than main (~1.3%), attributable to one extra match-arm dispatch on the new `scope` field in the subscription callback. Replay is statistically identical (scoped vs unscoped within Criterion's noise band) — the mask block is free relative to the surrounding O(n + m) checkout-and-diff cost. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an opt-in
UndoScopetoUndoManagerso a manager can be restricted to a set of containers.Ctrl+Zthen reverts only edits within that scope, even when a single commit touched containers both inside and outside it. The default isUndoScope::Doc, which is bit-for-bit today's behavior, so the change is purely additive and every existing undo test passes unchanged. Closes #981.Motivation
The use case is a collaborative multi-file workspace held in a single
LoroDoc: aLoroTreeof file nodes, each file's content aLoroText. We want per-file undo on that unified doc. Today the only per-scope knob isadd_exclude_origin_prefix, so per-file undo requires NUndoManagerinstances cross-excluded by origin string, which is O(N²) to register, can't handle a commit that touches two files at once, relies onset_next_commit_origincarrying through into the reverse opundo()emits internally, and pushes the same boilerplate onto every consumer.UndoScope::Containersreads as a generalization ofadd_exclude_origin_prefix: same record-time filter path, keyed onContainerIDmembership instead of an origin-string prefix.Design
Two filter points, mirroring how
undoalready works. At record time, an out-of-scope local commit is not pushed onto the undo stack; it's routed through the samecompose_remote_eventpath that handles real remote peers, so cursor transforms still apply and counter bookkeeping advances cleanly. The scope check is inlined so the optimizer folds theDocarm to a constant, leaving the default path to pay only a discriminant load. At replay time, a newundo_internal_with_scopemasks the computedDiffBatchto in-scope containers before_apply_diff, walkingcontainer_remapto mirror its own remap chain; if a commit touched A, B, C and only A is in scope, only A is reverted. That replay-time mask is what makes mixed commits correct, and it has no equivalent in the origin-filter approach. The oldundo_internalstays as a thin#[inline]wrapper delegating withscope_filter: None, so no existing caller changes.Tests
9 new integration tests in
crates/loro-internal/tests/undo.rs(16 total, all passing): default-is-doc-wide regression guard; out-of-scope exclusion and redo symmetry; out-of-scope edits not corrupting the in-scope stack; mixed-commit undo and redo (impossible with the origin workaround); chained mixed commits; three-container scope sets; and scope changed after construction.Benchmarks
crates/loro-internal/benches/undo.rs(criterion, 1000 commits/iter, n=50), main vs this branch:no_manager(baseline)default_scope(existing hot path)scoped_one_container(new opt-in)replayper undo+redoThe default-path regression is ~45 ns/commit; the opt-in scoped path is indistinguishable from default-scope, and the replay-time mask is free relative to the surrounding checkout-and-diff cost.
Compatibility
Purely additive;
UndoScope::Docis the default and preserves current behavior. Fits a minor bump.