feat: automatic merging for concurrent container inserts in maps by canadaduane · Pull Request #912 · loro-dev/loro

canadaduane · 2026-01-25T23:20:49Z

Completes #759

Summary of Changes

Core Logic (crates/loro-internal/src/handler.rs):
- Implemented get_or_create_mergeable_container and insert_mergeable_container_with_txn in MapHandler.
- Mechanism: Generates a deterministic Root Container ID using the format parent_id/key. This ensures that concurrent creations by different peers result in the same Container ID, allowing their operations to merge naturally.
- Parentage: Explicitly sets the parent of these "Root" containers in the Arena, allowing them to function as children of a Map.
Serialization (crates/loro-internal/src/state.rs):
- Updated get_value, get_deep_value, and get_deep_value_with_id to filter out these mergeable containers from the top-level root container list. They are identified by the reserved / character in their name. This ensures they only appear nested under their parent Map, not as detached root documents.
Rust API (crates/loro-internal/src/handler.rs):
- Added get_mergeable_list, get_mergeable_map, get_mergeable_movable_list, get_mergeable_text, and get_mergeable_tree to MapHandler.
WASM/JS API (crates/loro-wasm/src/lib.rs):
- Exposed the new methods on LoroMap:
  - getMergeableList(key)
  - getMergeableMap(key)
  - getMergeableMovableList(key)
  - getMergeableText(key)
  - getMergeableTree(key)
- Updated TypeScript definitions in crates/loro-wasm/src/lib.rs.
Testing (crates/loro-internal/tests/mergeable_container.rs):
- Created comprehensive integration tests covering:
  - Concurrent Merging: Verified that two peers creating a list at the same key results in a single merged list containing items from both.
  - Container Types: Verified support for List, Map, Text, Tree, and MovableList.
  - Deep Nesting: Verified that chains like Map -> MergeableMap -> MergeableMap -> MergeableList zipper together correctly.
  - Mixed Types: Verified that different container types at the same key do not merge (LWW applies to the Map entry).
  - Serialization: Verified that mergeable containers are correctly hidden from the root view.
  - TypeScript Tests (crates/loro-wasm/tests/mergeable.test.ts): Added tests verifying the JS API for concurrent merging, deep nesting, and correct JSON representation.

Key Observations

Resurrection: Deleting a mergeable container and re-creating it at the same key will restore its history, because the ID is deterministic. This is a feature of the design but should be documented for users.
Map-Only: This feature is strictly for LoroMap children, as it relies on stable keys for ID generation.
Safety: The implementation relies on the invariant that user-created Root Containers cannot contain /, creating a safe namespace for these system IDs.

Why `/` is better than `:`

Reserved Namespace:
- The Loro system enforces a rule for user-created Root Containers: their names cannot contain the / character (or \0). This is checked by check_root_container_name.
- The : character, however, is allowed in user-created root container names.
Preventing Collisions:
- If we used :, a user could theoretically create a root container with a name like cid:10@100:Map:myKey. If this matches our internal format, the system might confuse the user's root container with a mergeable child container, leading to unpredictable behavior.
- By using / (e.g., cid:10@100:Map/myKey), we are using a character that is strictly forbidden for users. This guarantees that our generated ID will never collide with any valid user-created root container. It effectively creates a private, reserved namespace for these internal containers.
Parsing Simplicity:
- When filtering these containers during serialization (to hide them from the top-level view), we can simply check name.contains('/'). Since users can't use /, any root container with a / in its name must be one of our internal mergeable containers.

In summary, using / leverages an existing system constraint to create a robust, collision-free identifier for this new feature.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f8571612a4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-01-25T23:26:27Z

crates/loro-internal/src/handler.rs

+        // Set Parent in Arena
+        let child_idx = ans.idx();
+        inner
+            .doc
+            .arena
+            .set_parent(child_idx, Some(inner.container_idx));


Treat mergeable Root IDs as parentless

This explicit parent assignment won’t be observable because mergeable containers use ContainerID::Root and SharedArena::get_parent short‑circuits all root IDs to None. As a result, path/ancestor logic (e.g. DocState::get_path used by getPathToContainer and event path) will still treat mergeable containers as top‑level roots even after this call, so a getMergeableList created under a map will report a root path like cid:root-…/key instead of the map key path. That’s a functional regression for path‑based lookups/subscriptions; consider updating get_parent (and depth/root handling) to honor is_mergeable() root IDs.

Useful? React with 👍 / 👎.

Addressed in latest push.

canadaduane · 2026-01-26T00:18:18Z

Should there also be a getMergeableCounter? Probably yes?

EDIT: Added getMergeableCounter

canadaduane · 2026-01-27T00:15:01Z

I've played around with this a bit now. Here are some questions that arose as an app developer using the new Loro mergeable containers:

Currently, map.get(key) returns a different container than map.getMergeableMap(key). Maybe map.get(key) should return the mergeable map if it exists?
What happens if I call map.setContainer("list", someList) and then map.getMergeableList("list")? Do I get two different containers at the same key?
Should there be a way to "upgrade" an existing non-mergeable container to a mergeable one?

Update: validating with some tests, it doesn't look like my map.get claim is holding up. I'll report back.

canadaduane · 2026-01-27T02:27:26Z

I found a bug, which explains the previous contradictory outcomes I was seeing.

The "root container" is now a type that can be encoded and decoded.

When encoding a LoroValue::Container, the code only stored the container type (List, Map, etc.), not the full container ID. When decoding, it always created a ContainerID::new_normal() using the operation's ID, losing the Root container ID information that mergeable containers use.

Now we have a RootContainerType variant (value 10) to distinguish Root containers from Normal containers during encoding/decoding.

zxch3n · 2026-01-29T16:24:17Z

crates/loro-internal/src/encoding/value.rs

    Map,
    ContainerType,
+    /// Root container type - used for mergeable containers
+    RootContainerType,


I don't think we need to change this. Changing this may break the compatibility with the exiting Loro

zxch3n · 2026-01-29T16:27:24Z

crates/loro-internal/tests/mergeable_container.rs

+
+    let list2 = map.get_mergeable_list("list").unwrap();
+    assert_eq!(list.id(), list2.id());
+    assert_eq!(


The tests here don't feel like test the real things.

In this test case, we should test map.get_deep_value() to include the 'list' entry.

zxch3n · 2026-01-29T16:28:04Z

crates/loro-internal/tests/mergeable_container.rs

+    let list1 = map1.get_mergeable_list("list").unwrap();
+    let list2 = map2.get_mergeable_list("list").unwrap();
+
+    assert_eq!(list1.id(), list2.id());


we should test that list1 is now actually a field inside map1

zxch3n · 2026-01-29T16:29:17Z

crates/loro-internal/src/arena.rs

            // whether the target is a root container
            return None;
        }



If it's root, and it's mergeable, it should return the parent here

zxch3n · 2026-01-29T16:32:33Z

crates/loro-internal/src/handler.rs

+                if let Some(c) = C::from_handler(h) {
+                    return Ok(c);
+                } else {
+                    unreachable!("Container type mismatch for same ID");


We should return an Err here, instead of calling unreachable.

zxch3n · 2026-01-29T16:33:49Z

crates/loro-wasm/tests/mergeable.test.ts

+    const list = level2.getMergeableList("list");
+
+    expect(list.length).toBe(2);
+    expect(list.toJSON()).toEqual(expect.arrayContaining(["A", "B"]));


The test cases are not testing the real things that could go wrong

zxch3n · 2026-01-29T16:36:34Z

When encoding a LoroValue::Container, the code only stored the container type (List, Map, etc.), not the full container ID. When decoding, it always created a ContainerID::new_normal() using the operation's ID, losing the Root container ID information that mergeable containers use.

This is not the case. Is it produced by Opus?

canadaduane · 2026-02-03T01:29:05Z

These are all great points, thanks @zxch3n . I'm taking a step back to re-consider this PR, and so I'll add comments to #759 in case this PR is abandoned.

canadaduane force-pushed the automatic-merging-concurrent-container-inserts branch from f857161 to 3395956 Compare January 25, 2026 23:21

chatgpt-codex-connector bot reviewed Jan 25, 2026

View reviewed changes

canadaduane force-pushed the automatic-merging-concurrent-container-inserts branch from 3395956 to 9348727 Compare January 26, 2026 01:06

canadaduane force-pushed the automatic-merging-concurrent-container-inserts branch from 9348727 to 6d1f86e Compare January 27, 2026 02:25

feat: automatic merging for concurrent container inserts in maps

ed17330

canadaduane force-pushed the automatic-merging-concurrent-container-inserts branch from 6d1f86e to ed17330 Compare January 27, 2026 04:51

zxch3n requested changes Jan 29, 2026

View reviewed changes

Uh oh!

Conversation

canadaduane commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Key Observations

Why / is better than :

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

canadaduane Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

canadaduane commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

canadaduane commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

canadaduane commented Jan 27, 2026

Uh oh!

zxch3n Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

zxch3n Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

zxch3n Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

zxch3n Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

zxch3n Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

zxch3n Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

zxch3n commented Jan 29, 2026

Uh oh!

canadaduane commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

canadaduane commented Jan 25, 2026 •

edited

Loading

Why `/` is better than `:`

canadaduane commented Jan 26, 2026 •

edited

Loading

canadaduane commented Jan 27, 2026 •

edited

Loading