Skip to content

fix: rich-text insert positions near style anchors, delete IDs, and delete events - #1135

Merged
zxch3n merged 2 commits into
mainfrom
fix/richtext-wasm-positions
Sep 28, 2026
Merged

zxch3n merged 2 commits into
mainfrom
fix/richtext-wasm-positions

Conversation

@zxch3n

@zxch3n zxch3n commented Sep 27, 2026 •

Copy link
Copy Markdown
Member

Summary

Three rich-text bugs found by the randomized loro.js ↔ Rust differential suite (#1136). Each has a regression test.

Bug Builds Symptom Fix
Insert-position cache skipped the anchor rules native and WASM get_entity_index_for_text_insert reused a cached cursor at a text-chunk boundary, so find_best_insert_pos did not run and the side of a style anchor that new text landed on depended on earlier lookups. E.g. insert "b", mark(0..1, em /*expand: before*/), unmark(0..1, bold) (writes nothing), insert(1, "a") gave ba all in em; without the no-op unmark, a is outside em. Reuse the cached cursor only where the anchor rules cannot move the insert (anchor_rules_may_move_insert): inside or between text chunks, at the text ends, or at the end of a text chunk followed by an anchor that new text stays in front of (typing at the end of bold). Elsewhere, fall back to find_best_insert_pos.
Delete start_id off by UTF-16 length WASM get_text_entity_ranges merged two chunks when start_id + event_len equaled the next ID. In WASM event_len is UTF-16, so after an astral character it could merge the wrong chunk: deleting a😀x in a😀xb (x typed between 😀 and b) recorded start_id 0 with length 3, naming a😀b. A full import applies deletes by position and gets the right text, but a shallow-snapshot import labels its placeholders with the recorded IDs and a cursor on the deleted x stops resolving, so both disagree with the full import (b! vs x! in #1149). Advance by the range's entity length. This fixes new ops only; deletes already written by loro-crdt ≤ 1.16.3 keep the wrong IDs: #1149.
Op/hint length mismatch panic WASM In a transaction whose events are built (any subscriber, or just an UndoManager), backspacing over an astral character merges the ops (Unicode lengths) but not the event hints (UTF-16 spans). change_to_diff could only split an op when a hint spanned several ops. E.g. insert "𝒳y", delete(2, 1), delete(0, 2), commit panicked. Let one op feed several hints by slicing it. change_to_diff now also debug-checks that each op matches its hint's kind and that every hint is consumed.

Rationale for the cache rule: context/richtext-insert-positions.md.

Performance

cargo run -p loro --release --example richtext_typing_bench -- <scenario> <commit|txn>: 100k-character text, then 100k single-character inserts at consecutive positions, committing after each insert (commit) or once (txn). Median ms of 15 interleaved rounds (fresh process per run, loaded machine):

scenario mode main first version now first vs main now vs main
plain commit 45.4 45.1 45.2 −0.7% −0.4%
plain txn 10.4 10.0 10.2 −3.8% −1.9%
in-bold commit 55.7 63.0 56.8 +13.1% +2.0%
in-bold txn 21.7 26.9 21.9 +24.0% +0.9%
bold-end commit 56.8 65.2 56.9 +14.8% +0.2%
bold-end txn 22.0 30.2 22.4 +37.3% +1.8%
marks-2000 commit 58.3 66.9 57.8 +14.8% −0.9%
marks-2000 txn 21.5 31.3 22.0 +45.6% +2.3%

"First version" is this PR's first commit, which only reused the cache strictly inside a text chunk. bold-end types at the end of a bold range: the text goes before bold's end anchor, which the cache now handles because the anchor rules stop at that anchor.

Validation

  • insert_position_near_style_anchors_does_not_depend_on_cursor_cache (crates/loro/tests/contracts/text_richtext_advanced.rs) fails without the fix.
  • cached_insert_position_matches_cold_lookup_near_style_anchors (crates/loro-internal/src/container/richtext/richtext_state/insert_pos_cache_test.rs): random documents with all four expand types, overlapping styles, null/false values, unmarks, empty and adjacent anchor pairs, and astral/ZWJ/flag/modifier text; after every edit it compares the cached lookup with a cold one at every position for all five PosTypes, from the edit's cursor and from the previous position's. 12 seeds (~4 s debug) in CI; 1,000 seeds pass in release both natively and with --features wasm (LORO_INSERT_CACHE_SEEDS=1000). It fails on main's cache rule and on three looser variants of the new one.
  • WASM tests in crates/loro-wasm/tests/richtext.test.ts for the delete IDs, the subscriber panic, and the UndoManager-only panic (UTF-16 only, so not reproducible natively).
  • cargo test -p loro -p loro-internal: 1,181 passed, 0 failed (15 ignored).
  • WASM (pnpm -C crates/loro-wasm build-dev, debug assertions on, so the new change_to_diff checks run on UTF-16 hints): vitest 381 passed in 29 files; tsc, deno, and bun tests pass.
  • pnpm check (clippy, all features, -D warnings) passes.
  • With this build as the reference, the loro.js differential suite passes 400 rich-text seeds × 160 steps (fix(loro.js): keep rich-text style anchors in the text sequence #1137).

Related

🤖 Generated with Claude Code

- `get_entity_index_for_text_insert` reused a cached cursor at a text-leaf
  boundary, skipping the style-anchor rules of `find_best_insert_pos`.
- `get_text_entity_ranges` advanced a range's start ID by its UTF-16 length.
- `change_to_diff` could not give one op to several event hints.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

WASM Size Report

  • Original size: 3211.32 KB
  • Gzipped size: 1064.26 KB
  • Brotli size: 744.75 KB

Review follow-up; rationale in context/richtext-insert-positions.md.

- Reuse the cached cursor wherever the anchor rules cannot move the
  insert, which brings typing in styled text back to main's speed.
- Test cached against cold insert lookups on random styled documents.
- Check that change_to_diff pairs ops with hints of the same kind and
  consumes every hint.
- The cache bug also affects native builds, the panic needs only an
  UndoManager, and old delete IDs stay wrong (#1149).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@zxch3n zxch3n changed the title fix: rich-text insert positions, delete IDs, and delete events in WASM fix: rich-text insert positions near style anchors, delete IDs, and delete events Sep 27, 2026

zxch3n commented Sep 27, 2026

Copy link
Copy Markdown
Member Author

Review follow-up, in 4749483 (the first commit is unchanged):

  1. Typing slowdown with styles: the cache is reused again wherever the anchor rules cannot move the insert (anchor_rules_may_move_insert): inside a text chunk, between two text chunks, at the text ends, and, going one step beyond the suggested rule, at the end of a text chunk followed by an anchor that new text stays in front of (insert_stays_before_anchor, now shared with find_best_insert_pos). The rules stop at that anchor, so the answer there is the cursor itself. This covers typing at the end of bold, which was the one case the suggested rule left slow. Numbers (same scenarios, interleaved main / first version / now) are in the PR body; everything is back to main's level, bold-end included.
  2. Cache equivalence test: cached_insert_position_matches_cold_lookup_near_style_anchors compares the cached and cold lookups at every position, for all five PosTypes, after every random edit (four expand types, overlapping styles, null/false values, unmarks, empty/adjacent anchor pairs, astral/ZWJ/flags). 12 seeds in CI (~4 s debug); 1,000 seeds pass in release natively and with --features wasm. It fails on main's cache rule and on three looser variants of the new rule.
  3. txn.rs: the tautological assert_eq! is gone. Running out of ops for a hint is an explicit unreachable!, and debug builds check that every op matches its hint's kind and that every hint is consumed.
  4. Wording: title, body and changeset now say the cache bug affects native Rust too and the panic needs only an UndoManager. They also say old wrong start_ids are not repaired: shallow-snapshot imports and cursors still read them. Issue Text deletes written by loro-crdt (WASM) ≤ 1.16.3 can record a wrong start_id around astral characters #1149 has a repro against the published loro-crdt@1.16.3 (full import b!, shallow import x!, cursor on the deleted x unresolvable). New context article: context/richtext-insert-positions.md.

Also added crates/loro/examples/richtext_typing_bench.rs (the benchmark above) and a WASM test for the UndoManager-only panic.

Validation: cargo test -p loro -p loro-internal (1,181 passed), pnpm check (clippy), WASM build-dev tests (vitest 381 passed, tsc, deno, bun), and 1,000-seed cache-equivalence runs (native and --features wasm).

Typing benchmark, median ms of 15 interleaved rounds (100k-char text, 100k inserts):

scenario mode main first version now
in-bold commit / txn 55.7 / 21.7 63.0 / 26.9 56.8 / 21.9
bold-end commit / txn 56.8 / 22.0 65.2 / 30.2 56.9 / 22.4
marks-2000 commit / txn 58.3 / 21.5 66.9 / 31.3 57.8 / 22.0
plain commit / txn 45.4 / 10.4 45.1 / 10.0 45.2 / 10.2

@zxch3n
zxch3n merged commit 27982a5 into main Sep 28, 2026
1 check passed
zxch3n added a commit that referenced this pull request Sep 28, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.

1 participant