fix: keep root-time map values when checking out a shallow doc

.changeset/shallow-checkout-equal-value.md

@@ -0,0 +1,5 @@
+---
+"loro-crdt": patch
+---
+
+Fix a map value disappearing after importing a shallow snapshot and checking out its root. It happened when a key was rewritten after the root with the value it had at the root (for example by `revertTo`) and the op that wrote the root value was trimmed. Checking out a version also now updates the last editor of map entries and movable-list items whose value is equal at both versions, so `getLastEditor` is correct after a checkout, and shallow and state-only snapshots record the right writer in their root state. Snapshots exported by older versions can still carry the wrong writer in their root state; export them again from a full-history document to fix them.

context/internal-encoding.md

@@ -301,7 +301,11 @@ after the root, never concurrent with it (loro-dev/loro#1095). The meet of
 the requested heads is not enough: a branch merged later that
 forked below the requested version is concurrent with it. `StateOnly` retains
 history only up to its target, so it uses the target on both sides.
-`crates/loro/tests/shallow_root_critical.rs` checks the property op by op. The
+`crates/loro/tests/shallow_root_critical.rs` checks the property op by op.
+Because the root state is materialized by a checkout, it is only as correct as
+the checkout's winner metadata (lamport/peer), not just its values: see
+"Winner metadata, not just values" in
+[crates/loro-internal/docs/diff_calc.md](../crates/loro-internal/docs/diff_calc.md). The
 root is then moved past a rich-text StyleStart when necessary and clamped to
 an existing shallow root. The root state carries `fr`; a later state overlay
 does not. Import loads the root first and then either overlays the later state

crates/loro-internal/docs/diff_calc.md

@@ -78,6 +78,36 @@ Map diffs in `Checkout`/`Import` mode only look up the keys written inside the
 replayed span, and skip replayed ops that both versions already contain, so
 their cost follows the update instead of the map size.
 
+## Winner metadata, not just values
+
+Map and MovableList states store the winning op's lamport/peer (map entry,
+movable-list `value_id`) next to the value. A checkout must move that metadata
+even when both versions hold an equal value written by different ops (a value
+rewritten after the target, e.g. by `revert_to`).
+
+`MapDiffCalculator` (Checkout/Import) compares the winners at `from` and `to`
+by op id only (`MapHistoryCache::changed_winners_for_keys`): the same winner is
+skipped without reading any value, and a different winner is emitted with the
+`to` value. The `from` value is never fetched, because the state applying the
+diff is already at `from` (the isolated-import fast path diffs from the empty
+version, where every winner is new). `MapState` writes each entry with one
+`insert` and reports a change only when the previous value differs, so an
+equal-value entry is a silent metadata update and events still only report
+value changes. `MovableListState` does the same for element values. This keeps
+equal-value checkouts at the cost of `main` before the fix: the added state
+write per key is paid for by the dropped `from` value lookup (loro-dev/loro#1124;
+benchmark: `crates/loro/examples/map_equal_value_bench.rs`).
+
+Skipping those entries left the later op's metadata in the checked-out state.
+Shallow and state-only exports build their root state through such a checkout,
+so the root state carried a lamport/peer that is not in the root version. On
+import, `ensure_shallow_map_seeded` inserted that entry into the map checkout
+index, where it compared equal to the retained later op (the index is keyed by
+container, key, lamport and peer) and was dropped, so `checkout(root)` lost the
+key whenever its root-time writer was trimmed. `get_last_editor` after a
+checkout was wrong for the same reason. Regression tests:
+`crates/loro/tests/shallow_checkout_equal_value.rs`.
+
 ## Diff modes
 
 - `Checkout` is the general and slowest mode. It can move in either direction

crates/loro-internal/src/diff_calc.rs

@@ -672,48 +672,35 @@ impl DiffCalculatorTrait for MapDiffCalculator {
             DiffMode::Checkout | DiffMode::Import => oplog.with_history_cache(|h| {
                 h.ensure_shallow_map_seeded(self.container_idx);
                 let checkout_index = &h.get_checkout_index().map;
-                let mut changed = Vec::new();
                 let keys = std::mem::take(&mut self.changed);
-                let from_map = checkout_index.get_container_latest_op_at_vv_for_keys(
+                // Every key whose winning op differs is emitted, even when both
+                // winners carry the same value: the state keeps the winner's
+                // lamport/peer, and a shallow root state encoded from stale
+                // metadata collides with a retained op in this index and loses
+                // the root entry. The state is at `from` and already holds the
+                // `from` value, so `MapState` tells value changes from
+                // metadata-only updates in the same write, and the `from` value
+                // is never fetched here.
+                let changed = checkout_index.changed_winners_for_keys(
                     self.container_idx,
                     from_vv,
-                    keys.keys().cloned(),
-                    oplog,
-                );
-                let mut to_map = checkout_index.get_container_latest_op_at_vv_for_keys(
-                    self.container_idx,
                     to_vv,
                     keys.into_keys(),
                     oplog,
                 );
 
-                for (k, peek_from) in from_map.iter() {
-                    let peek_to = to_map.remove(k);
-                    match peek_to {
-                        None => changed.push((k.clone(), None)),
-                        Some(b) => {
-                            if peek_from.value != b.value {
-                                changed.push((k.clone(), Some(b)))
-                            }
-                        }
-                    }
-                }
-
-                for (k, peek_to) in to_map.into_iter() {
-                    changed.push((k, Some(peek_to)));
-                }
-
                 let mut updated =
                     FxHashMap::with_capacity_and_hasher(changed.len(), Default::default());
                 for (key, value) in changed {
                     let value = value.map(|v| {
-                        let value = v.value.clone();
-                        if let Some(LoroValue::Container(c)) = &value {
+                        // A normal container id is minted by one op, so a new
+                        // winner holding a container always changes the value.
+                        if let Some(LoroValue::Container(c)) = &v.value {
                             on_new_container(c);
                         }
 
                         MapValue {
-                            value,
+                            value: v.value,
                             lamp: v.lamport,
                             peer: v.peer,
                         }

crates/loro-internal/src/history_cache.rs

@@ -679,78 +679,84 @@ impl MapHistoryCache {
         });
     }
 
-    /// Resolve the winning op at `vv` for a restricted set of keys.
+    /// The entry of the op that wins `key` at `vv`, if any.
+    fn latest_entry_at_vv(
+        &self,
+        container: ContainerIdx,
+        key: u32,
+        vv: &VersionVector,
+    ) -> Option<&MapHistoryCacheEntry> {
+        let bound = |lamport, peer| MapHistoryCacheEntry {
+            container,
+            key,
+            lamport,
+            peer,
+            counter_or_value: Either::Left(0),
+        };
+        self.map
+            .range(bound(0, 0)..=bound(Lamport::MAX, PeerID::MAX))
+            .rev()
+            .find(|entry| match &entry.counter_or_value {
+                Either::Left(cnt) => vv.get(&entry.peer).copied().unwrap_or(0) > *cnt,
+                // Shallow-root entries are part of every version the doc can reach.
+                Either::Right(_) => true,
+            })
+    }
+
+    fn entry_info(entry: &MapHistoryCacheEntry, oplog: &OpLog) -> GroupedMapOpInfo {
+        let value = match &entry.counter_or_value {
+            Either::Left(cnt) => {
+                let op = oplog
+                    .get_op_that_includes(ID::new(entry.peer, *cnt))
+                    .unwrap();
+                debug_assert_eq!(op.atom_len(), 1);
+                match &op.content {
+                    InnerContent::Map(map) => map.value.clone(),
+                    _ => unreachable!(),
+                }
+            }
+            Either::Right(v) => (**v).clone(),
+        };
+        GroupedMapOpInfo {
+            value,
+            lamport: entry.lamport,
+            peer: entry.peer,
+        }
+    }
+
+    /// For each key, the op that wins it at `to_vv` when that differs from the
+    /// op that wins it at `from_vv`; `None` when no op sets the key at `to_vv`.
     ///
     /// Every key whose value can differ between two versions must have been
     /// written by an op inside the replayed span, so a diff only ever needs the
     /// candidate keys instead of the whole container. This keeps map diffing
     /// proportional to the update instead of to the size of the map.
-    pub fn get_container_latest_op_at_vv_for_keys(
+    ///
+    /// Winners are compared by op id only, so only the `to` value is ever
+    /// fetched. A different winner can still carry an equal value; the caller's
+    /// state has the `from` value and tells the two cases apart (see
+    /// `MapDiffCalculator::calculate_diff`).
+    pub fn changed_winners_for_keys(
         &self,
         container: ContainerIdx,
-        vv: &VersionVector,
+        from_vv: &VersionVector,
+        to_vv: &VersionVector,
         keys: impl Iterator<Item = InternalString>,
         oplog: &OpLog,
-    ) -> FxHashMap<InternalString, GroupedMapOpInfo> {
-        let mut ans = FxHashMap::default();
+    ) -> Vec<(InternalString, Option<GroupedMapOpInfo>)> {
+        let mut ans = Vec::new();
         for key in keys {
             let Some(key_idx) = self.keys.get(&key) else {
                 continue;
             };
-
             let key_idx = key_idx as u32;
-            let range = (
-                Bound::Included(MapHistoryCacheEntry {
-                    container,
-                    key: key_idx,
-                    lamport: 0,
-                    peer: 0,
-                    counter_or_value: Either::Left(0),
-                }),
-                Bound::Included(MapHistoryCacheEntry {
-                    container,
-                    key: key_idx,
-                    lamport: Lamport::MAX,
-                    peer: PeerID::MAX,
-                    counter_or_value: Either::Left(0),
-                }),
-            );
-
-            for entry in self.map.range(range).rev() {
-                match &entry.counter_or_value {
-                    Either::Left(cnt) => {
-                        if vv.get(&entry.peer).copied().unwrap_or(0) > *cnt {
-                            let id = ID::new(entry.peer, *cnt);
-                            let op = oplog.get_op_that_includes(id).unwrap();
-                            debug_assert_eq!(op.atom_len(), 1);
-                            match &op.content {
-                                InnerContent::Map(map) => {
-                                    ans.insert(
-                                        key,
-                                        GroupedMapOpInfo {
-                                            value: map.value.clone(),
-                                            lamport: entry.lamport,
-                                            peer: entry.peer,
-                                        },
-                                    );
-                                }
-                                _ => unreachable!(),
-                            }
-                            break;
-                        }
-                    }
-                    Either::Right(v) => {
-                        ans.insert(
-                            key,
-                            GroupedMapOpInfo {
-                                value: (**v).clone(),
-                                lamport: entry.lamport,
-                                peer: entry.peer,
-                            },
-                        );
-                        break;
-                    }
-                }
+            let from = self.latest_entry_at_vv(container, key_idx, from_vv);
+            let to = self.latest_entry_at_vv(container, key_idx, to_vv);
+            match (from, to) {
+                (None, None) => {}
+                (Some(_), None) => ans.push((key, None)),
+                (Some(a), Some(b)) if a.lamport == b.lamport && a.peer == b.peer => {}
+                (_, Some(b)) => ans.push((key, Some(Self::entry_info(b, oplog)))),
             }
         }
 

crates/loro-internal/src/state/map_state.rs

@@ -262,8 +262,12 @@ impl ContainerState for MapState {
 
             let mut changed = false;
             if force {
-                self.insert(key.clone(), value.clone());
-                changed = true;
+                let prev = self.insert(key.clone(), value.clone());
+                // A checkout diff carries every key whose winning op changed,
+                // including ones whose value did not (see `MapDiffCalculator`):
+                // those only refresh the lamport/peer and report no change.
+                changed = mode != DiffMode::Checkout
+                    || !matches!(prev, Some(prev) if prev.value == value.value);
             } else {
                 match self.map.get(&key) {
                     Some(old_value) if old_value > &value => {}
@@ -394,13 +398,21 @@ impl MapState {
 
     pub fn insert(&mut self, key: InternalString, value: MapValue) -> Option<MapValue> {
         let value_yes = value.value.is_some();
-        if let Some(LoroValue::Container(id)) = &value.value {
-            self.child_containers.insert(id.clone(), key.clone());
-        }
+        let new_child = match &value.value {
+            Some(LoroValue::Container(id)) => {
+                self.child_containers.insert(id.clone(), key.clone());
+                Some(id.clone())
+            }
+            _ => None,
+        };
 
         let result = self.map.insert(key.clone(), value);
         if let Some(Some(LoroValue::Container(c))) = result.as_ref().map(|x| &x.value) {
-            self.child_containers.remove(c);
+            // Re-inserting the same child (e.g. a metadata-only checkout update)
+            // must keep its parent edge.
+            if new_child.as_ref() != Some(c) {
+                self.child_containers.remove(c);
+            }
         }
 
         match (&result, value_yes) {

crates/loro-internal/src/state/movable_list_state.rs

@@ -1133,8 +1133,18 @@ impl ContainerState for MovableListState {
                 match self.inner.elements().get(&elem_id).cloned() {
                     Some(elem) => {
                         // Update value if needed
-                        if let Some(value_id) = value_id {
-                            if elem.value != value && (!need_compare || elem.value_id < value_id) {
+                        if let Some(value_id) =
+                            value_id.filter(|value_id| !need_compare || elem.value_id < *value_id)
+                        {
+                            if elem.value == value {
+                                // Only the winning set op may have changed (e.g. a
+                                // checkout past a later set of the same value):
+                                // keep the value id accurate without an event, as
+                                // for maps.
+                                if elem.value_id != value_id {
+                                    self.inner.update_value(elem_id, value, value_id);
+                                }
+                            } else {
                                 maybe_moved.remove(&elem_id);
                                 self.inner.update_value(elem_id, value.clone(), value_id);
                                 let index = self.get_index_of_elem(elem_id);