Repository navigation
[lexical-website][lexical-y] Bug Fix: Keep every unchanged word of a line whose text nodes an update makes again with several words changed - #9625
Conversation
…line whose text nodes an update makes again with several words changed ## Description When an update removes a line's text nodes and makes new ones with other text (an agent rewriting a block from its JSON), the binding keeps the start and end the new text shares with the old (#9598) and writes everything between the first and the last changed word again as new text. With two or more changed words, a word a collaborator deleted between them at the same time came back on every client, and when the collaborator then undid its delete the word showed twice (`t1z t5zn9z t3z t4z t5z t6z n8z`). A letter a collaborator typed into such a word landed outside it, and a format it gave the word was lost. Between the common start and end, the text of the nodes made now keeps every run of unchanged whole words, with the white space around it that both texts share, found by a word-by-word shortest edit script (Myers) of the replaced middle. Only the changed words are written, so a collaborator's concurrent delete there stays deleted (and is restored once by its undo), typed letters stay in their word and formats stay on it. Words are those of the existing whole-word rule (D30): runs of non-space characters, each character of a script written without spaces, an inline node's object character, a line feed. White space alone is never kept between two changes, as at the ends. The script is bounded to 128 word edits per line, past which only the common start and end are kept, so the cost stays linear in the line's length (scaling test included). DECISIONS.md D29 now states the rule, and y.md's "Edits made by code" paragraph and its stale "Text nodes made again with other text" Limits entry describe what is kept and what is still written again (changed words, a word whose joined punctuation changed, a word joined to a changed word through an inline node, lines with more than 128 changed words). A stale format whose range reaches past a link the rewrite removes restates the link on kept text (#9597). With one changed word main already shows it on the kept end; keeping the unchanged words between two changes shows it there too. `RemadeTextStaleLink.test.ts` pins those 20 cells as expected failures by an exact predicate; #9601 (the #9597 fix) makes them pass (checked on this branch merged with #9601: 36/36), so the markers flip when it lands. Closes #9621 ## Test plan New tests: `regressions/RemadeTextStretches.test.ts` (the #9621 cases in both id orders, concurrent and unread, with and without the undo; other word shapes: a script without spaces, emoji, runs with different white space; a property over random lines of unique words in 1 to 4 text nodes, some bold or italic, made again with 2 to 4 word edits while a collaborator deletes, types into, deletes and undoes, types and undoes, or bolds an unchanged word, 120 seeds per cell; a long-line scaling test), 336 new cells in `matrix/C9RemadeText.test.ts` (every inline shape of the catalog, a word changed in the first and the last text node, the collaborator types into, deletes from, or deletes from and undoes `t2z` or `t3z` between them, concurrent and unread, both id orders), and `regressions/RemadeTextStaleLink.test.ts`. ### Before On main (fcea4ff): ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStretches.test.ts packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStaleLink.test.ts packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts ❯ unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStaleLink.test.ts (36 tests | 20 failed) ❯ unit packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts (564 tests | 336 failed) ❯ unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStretches.test.ts (34 tests | 30 failed) Tests 386 failed | 248 passed (634) AssertionError: expected [ 't1z n9z t3z t4z t5z t6z n8z', …(2) ] to deeply equal [ 't1z n9z t3z t4z t6z n8z', …(2) ] AssertionError: expected [ 't1z t5zn9z t3z t4z t5z t6z n8z', …(2) ] to deeply equal [ 't1z n9z t3z t4z t5z t6z n8z', …(2) ] "seed 1 ids 2,1 [\"t9\",\"38z t177z t158\",\"z t1\",\"99z\"] -> \"t215z, t938z t177z t158z\"; delete \"t177z\" at 1+2: \"t215z, t938z t177z t158z\": \"t7z\" missing, \"t177z\" came back" ``` The property fails 307 of its sessions on main (delete 103, type 96, delete and undo 81, bold 27). The 20 `RemadeTextStaleLink` failures on main are its expected-failure cells passing, because main writes the middle anew. ### After ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStretches.test.ts packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStaleLink.test.ts packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts ✓ unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStaleLink.test.ts (36 tests) ✓ unit packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts (564 tests) ✓ unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStretches.test.ts (34 tests) Tests 614 passed | 20 expected fail (634) $ pnpm run ci-check EXIT 0 $ vitest --run --project unit --project scripts-unit Test Files 637 passed | 4 skipped (641) Tests 26003 passed | 210 expected fail | 135 skipped (26348) ``` (main: 25617 passed | 190 expected fail | 135 skipped; the difference is the new tests.) ``` $ FUZZ_SEED_START=1 FUZZ_SEEDS=3000 vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts Tests 1 failed | 123 passed (124) AssertionError: expected [ 'seed 2277: dup' ] to deeply equal [] $ FUZZ_SEED_START=50000 FUZZ_SEEDS=2000 vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts Tests 7 failed | 77 passed (84) (seeds 50180, 50231, 50372, 51439, 51507, 51972, 51976) ``` The same seeds fail, with the same signatures, on main.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
There was a problem hiding this comment.
This looks good. Keeping the unchanged words between two changes fixes #9621 cleanly: the new matrix and property tests fail on main with the issue's symptoms and pass here. A few optional, non-blocking suggestions:
- Stale-link cells (#9597).
packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStaleLink.test.ts:110pins 20 cells where a link the rewrite removes comes back on the kept middle words. Those cells pass onmain, so this pattern is newly exposed until #9601 lands, and #9601 is still a draft. Applying #9601'sSchema.tschange on top of this branch makes all 20 pass. Consider landing the two together or this one after #9601, tested combined aspackages/lexical-y/AGENTS.mdasks for stacks. The alternative is a line in y.md's Limits naming #9597 for words kept between changes, since the same file asks for accepted limits to be documented there. - Bound wording.
packages/lexical-website/docs/collaboration/y.md:820and:1012say "more than 128 changed words".MAX_EDITS(Inline.ts:798) counts edits of the word diff, though, and a replaced word is a delete plus an insert, so a line with 65 replaced words already keeps only its start and end. D29's "128 word edits" is accurate; "more than 128 word edits (about 64 replaced words)" would match it. - Inline nodes (nit). Unchanged words joined to an inline node that the update makes again are written again too:
t2z abc⧉def t3z(⧉ an inline node), with changes at both ends, keepst2zandt3zbut notabcordef. The docs currently mention only words joined to a changed word. - Comment (nit). The
alignJSDoc inpackages/lexical-y/src/internal/TextEdits.ts(around line 1700) still says made text keeps "the common start and end ... (see keptEnds)". It could point tokeptStretchesnow.
| WORDS.indexOf(range[1]) + 1, | ||
| ).some(w => !link.includes(w)); | ||
| for (const order of ['rewrite first', 'format first']) { | ||
| (reachesPast ? test.fails : test)( |
There was a problem hiding this comment.
Optional: these 20 cells pass on main (the middle is written anew there), so keeping the middle words newly exposes #9597 until #9601 lands, and #9601 is still a draft. With #9601's Schema.ts change applied on top of this branch all 20 pass. Consider landing the two together or this after #9601, tested combined as packages/lexical-y/AGENTS.md asks, or noting #9597 in y.md's Limits meanwhile.
There was a problem hiding this comment.
| written again with it; text between two separate changes is written | ||
| again, and an inline node made there is new where it is put. Prefer | ||
| written again with it, and an inline node made between two changes is new | ||
| where it is put. Where a line has more than 128 changed words, only its |
There was a problem hiding this comment.
Optional wording fix: MAX_EDITS counts edits of the word diff, and a replaced word costs a delete plus an insert, so a line with 65 replaced words already keeps only its start and end. D29's "128 word edits" is accurate; something like "more than 128 word edits (about 64 replaced words)" would match here and at line 1012.
There was a problem hiding this comment.
Fixed in 456250b: y.md and D29 now say "more than 128 edits (about 64 replaced words, each a delete and an insert)", and state exactly which words are written again past the bound (that commit also pairs repeated words by occurrence there, so they are kept).
…dit-script bound, keep remade lines linear in their kept runs, and read an old line's inline nodes in the old state ## Description Follow-up to the first commit, from review: - Past 128 word edits, the diff of a remade line kept only its common start and end, so a collaborator's concurrent edit of an unchanged word came back, showed twice after an undo, landed at the start of the line or lost its format, as before this pull request. Now the words found once in both texts anchor the rest (the longest run of them in the same order, patience diff), and each stretch between two of them gets its own bounded edit script. Linear in the line's length up to a log factor. - A rewrite that changes only the white space between the words of a line of one alternately formatted text node per word made a kept run per word and a node per run, and two loops scanned every kept run per node (O(n^2), 5 s at 4,000 words). The loop over kept runs keeps a cursor, and runs are found by binary search. - Remaking a line holding an inline node whose class reads its latest version in `isInline()` (the playground's `EquationNode`) threw "Lexical node does not exist in active editor state", on main too: the old line's layout asked the removed node in the new state. It is now read in the previous state. ## Changeset `.changeset/lexical-y-remade-line-inline-node.md` for the throw; the cap and the cost are part of the first changeset's fix. ## Test plan ### Before On the first commit (225cdc6): ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStretches.test.ts -t 'one edit script covers|linear' × 65 words replaced ‖ the peer deletes an unchanged word (ids 1,2) ... (all 8 cells for 65 and for 200 words) × the white space between every two words, one node per word Tests 17 failed | 10 passed | 32 skipped (59) ``` On main, the catalog inline shapes: ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts -t 'keeps the shape' × equation/inline Error: Lexical node does not exist in active editor state (key 17, type equation). ❯ EquationNode.isInline packages/lexical-playground/src/nodes/EquationNode.tsx:142:17 ❯ kindOf packages/lexical-y/src/internal/TextEdits.ts:203:42 ❯ TextReplay.oldLayout packages/lexical-y/src/internal/TextEdits.ts:1200:5 Tests 1 failed | 22 passed | 564 skipped (587) ``` ### After ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStretches.test.ts packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStaleLink.test.ts packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts ✓ unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStaleLink.test.ts (36 tests) ✓ unit packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts (587 tests) ✓ unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStretches.test.ts (59 tests) Tests 662 passed | 20 expected fail (682) $ pnpm run ci-check EXIT 0 $ vitest --run --project unit --project scripts-unit Test Files 637 passed | 4 skipped (641) Tests 26051 passed | 210 expected fail | 135 skipped (26396) $ FUZZ_SEED_START=1 FUZZ_SEEDS=3000 vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts Tests 1 failed | 123 passed (124) AssertionError: expected [ 'seed 2277: dup' ] to deeply equal [] $ FUZZ_SEED_START=50000 FUZZ_SEEDS=2000 vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts Tests 7 failed | 77 passed (84) (seeds 50180, 50231, 50372, 51439, 51507, 51972, 51976, as on main) ``` Whitespace-only remake of one alternately formatted node per word, 500 / 1,000 / 2,000 / 4,000 words: 34 / 62 / 107 / 216 ms (the first commit: 114 / 353 / 1,258 / 5,162 ms; main: 70 / 70 / 108 / 210 ms).
There was a problem hiding this comment.
Thanks, this looks good. Keeping the unchanged words between changes fixes #9621 (a deleted word that came back, or showed twice after an undo). Reading removed inline nodes in the previous state fixes the EquationNode throw. The new regression and matrix cells fail on main and pass here, and they assert every peer plus a fresh reader.
Two optional, non-blocking suggestions:
- Landing order with #9601 (
packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStaleLink.test.ts:110). The 20 cells markedtest.failspass onmaintoday: the rewrite writes the middle again there, so the removed link stays removed. With this change, a stale format puts the link back on the kept words until #9601 lands. Pinning them withtest.failsfollowspackages/lexical-y/AGENTS.md. Consider landing #9601 first or alongside this PR. If this one lands first, add a line to y.md's Limits while #9601 is open, as that AGENTS.md asks for accepted limits. - Threshold wording in y.md (
packages/lexical-website/docs/collaboration/y.md:821and:1013). Both places say "more than 128 changed words", butMAX_EDITS(packages/lexical-y/src/internal/Inline.ts:798) caps word edits, and a replaced word counts as two. So the unique-word fallback starts at 65 replaced words, as DECISIONS.md D29 ("past 128 word edits") and the comment atRemadeTextStretches.test.ts:485already say. Suggested wording: "more than 128 word edits (about 64 replaced words)".
| again, and an inline node made there is new where it is put. Prefer | ||
| written again with it, and an inline node made between two changes is new | ||
| where it is put. The comparison stays linear in the line's length: where | ||
| a line has more than 128 changed words, the unchanged words found once in |
There was a problem hiding this comment.
Optional: MAX_EDITS (Inline.ts:798) caps word edits, and a replaced word is two (a delete and an insert), so this fallback starts at 65 replaced words. DECISIONS.md D29 ("past 128 word edits") and the comment at RemadeTextStretches.test.ts:485 say so. Suggested: "more than 128 word edits (about 64 replaced words)". The Limits entry at line 1013 has the same wording.
There was a problem hiding this comment.
Fixed in 456250b: y.md and D29 now say "more than 128 edits (about 64 replaced words, each a delete and an insert)", and state exactly which words are written again past the bound (that commit also pairs repeated words by occurrence there, so they are kept).
| WORDS.indexOf(range[1]) + 1, | ||
| ).some(w => !link.includes(w)); | ||
| for (const order of ['rewrite first', 'format first']) { | ||
| (reachesPast ? test.fails : test)( |
There was a problem hiding this comment.
Optional: these 20 cells pass on main today (the rewrite writes the middle again, so the removed link stays removed) and fail with this change until #9601 lands. Consider landing #9601 first or together with this PR, or adding a y.md Limits line while it is open, as packages/lexical-y/AGENTS.md asks for accepted limits.
There was a problem hiding this comment.
…of a remade line past the edit-script bound, and state when a word is written again ## Description Follow-up from review of 598c3c1. Past the 128-edit bound, the diff of a remade line anchored on words found once in both texts and, between two anchors that still needed more than 128 edits, kept only the common start and end of that stretch. A line with a word repeated between the changes and no unique word (`old echo old echo ...` made again as `new echo new echo ...`, more than 64 pairs), or such a stretch next to a unique word, wrote every `echo` again: a collaborator's concurrent delete there came back, showed twice after an undo, a letter typed into it went to the line's start, and a format on it was lost. Between two anchors past the bound, every word found in both stretches is now paired with itself by occurrence (its k-th occurrence in the old text with its k-th in the new), and the longest run of those pairs in the same order in both is kept, with a bounded edit script between them. What is kept is still a common subsequence of the two texts, found in O((n + m) * 128 + (n + m) log(n + m)). y.md and D29 state exactly which words are written again: a word the update moved past kept words, and, where the word diff needs more than 128 edits (about 64 replaced words), a repeated word whose occurrences the update added or removed between two words found once in both texts. y.md's Open issues lists #9597 (a stale format restating a removed link), which kept middle words now show as kept ends already did. ## Test plan New cells in `RemadeTextStretches.test.ts`: 64, 65 and 200 replacements for three line shapes (distinct words, a word repeated between them, the same with one unique word), with the collaborator deleting, deleting and undoing, typing into or bolding an unchanged word, both id orders; a scaling case for a repeated word between every two changed words; and an independent longest-common-subsequence check of `keptStretches` (2,000 random word sequences with repeats below the bound: kept words = LCS; 200 large ones above it: ranges are equal text in order). ### Before On 598c3c1: ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStretches.test.ts × a word repeated between them, 64 replaced ‖ the peer deletes an unchanged word (ids 1,2) × a word repeated between them, 64 replaced ‖ the peer delete, undos an unchanged word (ids 1,2) × a word repeated between them, 64 replaced ‖ the peer types an unchanged word (ids 1,2) × a word repeated between them, 64 replaced ‖ the peer bolds an unchanged word (ids 1,2) ... (48 cells: both repeated-word shapes, 64, 65 and 200 replaced, every edit, both id orders) × above it: equal runs in order, and every repeated word kept where its occurrences pair up Tests 49 failed | 61 passed (110) ``` ### After ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStretches.test.ts Tests 110 passed (110) $ pnpm run ci-check EXIT 0 $ vitest --run --project unit --project scripts-unit Test Files 637 passed | 4 skipped (641) Tests 26102 passed | 210 expected fail | 135 skipped (26447) $ MATRIX_FULL=1 vitest --run --project unit packages/lexical-y/src/__tests__/unit/matrix Tests 11 failed | 15974 passed | 29 expected fail | 128 skipped (16142) (the same 11 cells as main) $ FUZZ_SEED_START=1 FUZZ_SEEDS=3000 vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts Tests 1 failed | 123 passed (124) (seed 2277: dup, as on main) $ FUZZ_SEED_START=50000 FUZZ_SEEDS=2000 vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts Tests 7 failed | 77 passed (84) (seeds 50180, 50231, 50372, 51439, 51507, 51972, 51976, as on main) ``` Whitespace-only remake of one alternately formatted node per word, 500 / 1,000 / 2,000 / 4,000 words: 38 / 73 / 104 / 209 ms.
There was a problem hiding this comment.
This looks good. Keeping the unchanged words between two changes fixes #9621: a word a collaborator deleted between two changed words no longer comes back, and it no longer shows twice after the collaborator undoes the delete. Reading removed inline nodes in the previous state fixes the EquationNode throw. The new regression and matrix cells fail on main and pass here, and they assert every peer plus a fresh reader. y.md's 128-edit wording and its new #9597 Open issues entry match the code.
One optional, non-blocking suggestion:
Document the undo case for words that are still written again (packages/lexical-website/docs/collaboration/y.md:1019-1023). The "Text nodes made again with other text" Limits entry says what happens to a collaborator's typing and formatting in a changed or moved word, but not to a delete that is then undone. With two peers on t1z t2z t3z t4z t5z t6z t7z:
- A changes
t4zton4zwhile B deletest4zand undoes: every peer showsn1z t2z t3z t4zn4z t5z t6z n7z. - A moves
t2zaftert5zwhile B deletest2zand undoes: every peer showst2zn1z t3z t4z t5z t2z t6z n7z.
main brings the word back in both cases too (at the line start), so this is not a regression. packages/lexical-y/AGENTS.md asks for accepted limits to be documented in y.md's Limits, though, and the general Undo entry names only the case of two collaborators removing the same text. A sentence such as "A collaborator who deletes such a word at the same time and then undoes the delete gets the old word back next to the new one; a moved word then shows twice." would cover it.
The landing order with #9601 is already settled in the description: the 20 RemadeTextStaleLink cells stay test.fails, naming #9597 and #9601, until it lands.
| word at the same time is not kept. So is a word the update moved past | ||
| the words it kept, and, where the line's word diff needs more than 128 | ||
| edits (about 64 replaced words), a repeated word whose occurrences the | ||
| update added to or removed from between two words found once in both | ||
| texts. |
There was a problem hiding this comment.
Optional: this entry covers a collaborator's typing and formatting in a word that is written again, but not a delete that is then undone. A changed word comes back next to the new one (t4z changed to n4z while a collaborator deletes t4z and undoes gives n1z t2z t3z t4zn4z t5z t6z n7z), and a moved word shows twice (t2zn1z t3z t4z t5z t2z t6z n7z). main does the same, so this is not a regression, but packages/lexical-y/AGENTS.md asks for accepted limits to be documented here. Suggested sentence: "A collaborator who deletes such a word at the same time and then undoes the delete gets the old word back next to the new one; a moved word then shows twice."
… line with an exact linear-space diff, and document a word changed in part ## Description Follow-up from review of 456250b. Past 128 edits, the diff of a remade line's words fell back to heuristics (words found once in both texts as anchors, then repeated words paired by occurrence). Each left a case where an unchanged word was not kept: pairing the k-th occurrence of a word with its k-th occurrence lost a different, surviving `aa` when the update removed the first `aa` of `(old aa bb) x 65` and a collaborator deleted that same `aa`, and dropped an unchanged `bb` there, so the collaborator's delete of it came back. The kept words are now always a longest common subsequence of the two lines' words, found by Myers' linear-space diff (middle snake) over the words both texts have (a word only one text has is never kept, and leaving it out keeps the diff short), up to 2,048 edits among those: O((n + m) * 2048) time at most, about 30 ms for a 5,000-word line, O(n + m) memory. Past that bound (a line made again with most of its words moved), only the common start and end are kept, as before this pull request. The occurrence pairing and the anchors are gone. y.md and D29 state the bound, and that a word changed only in part is written whole (#9598's rule for an insertion between two letters of a word), so a collaborator's undone delete of its unchanged part shows twice: #9635, pinned as `test.fails` with a passing control of separate words. ## Test plan In `RemadeTextStretches.test.ts`: cells for lines of repeated words with an earlier occurrence removed (64, 65, 200, every collaborator edit, both id orders); the longest-common-subsequence check now over 1,100 random lines with repeated words up to 400 words long (kept words = LCS exactly); a line made again in reverse past the bound; the #9635 cells. ### Before On 456250b: ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStretches.test.ts × repeated words, an earlier occurrence of one removed, 64 replaced ‖ the peer deletes an unchanged word (ids 1,2) ... (12 cells: 64, 65 and 200 replaced, delete and type, both id orders) × a longest common run of words, with repeated words, short and long lines Tests 13 failed | 124 passed | 2 expected fail (139) ``` ### After ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStretches.test.ts Tests 137 passed | 2 expected fail (139) $ pnpm run ci-check EXIT 0 $ vitest --run --project unit --project scripts-unit Test Files 637 passed | 4 skipped (641) Tests 26129 passed | 212 expected fail | 135 skipped (26476) $ MATRIX_FULL=1 vitest --run --project unit packages/lexical-y/src/__tests__/unit/matrix Tests 11 failed | 15974 passed | 29 expected fail | 128 skipped (16142) (the same 11 cells as main) $ FUZZ_SEED_START=1 FUZZ_SEEDS=3000 vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts Tests 1 failed | 123 passed (124) (seed 2277: dup, as on main) $ FUZZ_SEED_START=50000 FUZZ_SEEDS=2000 vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts Tests 7 failed | 77 passed (84) (seeds 50180, 50231, 50372, 51439, 51507, 51972, 51976, as on main) ``` `keptStretches` on 5,000-word lines: every other word replaced 16 ms, repeated words with one removed 23 ms, 10 % of the words swapped 37 ms, made again in reverse (past the bound) 24 ms, 20-word vocabulary with half the words changed (past the bound) 25 ms.
There was a problem hiding this comment.
This looks good. Keeping the unchanged words between two changes instead of writing the whole middle again fixes #9621 cleanly, the word diff is bounded, and the new tests cover both client id orders, concurrent and unread edits, undo, and every catalog inline shape. The suggestions below are optional and non-blocking.
- Landing order with #9601 (
packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStaleLink.test.ts:110). As the description says, once the middle words are kept, a stale format that reaches past a link the rewrite removes restates the link on them (#9597), the same way main already does on the kept end. Landing #9601 first, or in the same merge-queue batch, avoids shipping that wider exposure on its own; thetest.failsmarkers flip when it lands. - Refresh the Test plan. The description still explains the previous commit's algorithm (a 128-edit bound with patience anchors and occurrence pairing) and pastes its run (110 tests in
RemadeTextStretches.test.ts, 713 passed and 20 expected fail). The head uses an exact linear-space diff capped at 2,048 edits (packages/lexical-y/src/internal/Inline.ts:830), and that test file now has 139 tests, including the two #9635test.failscells. AGENTS.md asks the Test plan's Before and After sections to carry the real output of the command that was run, so updating the paragraph, the pasted output and the timings would keep the description in step with the code. - Nit: the
isInline()fix is pinned only by theequation/inlinecell inpackages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts:451.packages/lexical-y/AGENTS.mdasks every bug fix to land with a regression test insrc/__tests__/unit/regressions/; a small case there (remake a line holding an inlineEquationNodefrom its JSON) would satisfy it.
## Description Merges main (2e2acb7, with #9625 and #9618). #9625's `RemadeTextStaleLink.test.ts` marks its 20 cells where a stale format reaches past the link as expected failures "(#9597, fixed by #9601)": they pass with this pull request, so they are plain tests now. The DECISIONS link list keeps both sides' references. ## Test plan ### Before On main, the 20 cells are expected failures. ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/RemadeTextStaleLink.test.ts Tests 36 passed (36) ```
…here the diff puts it inside a word, only between word edges, and never past a caret ## Description Follow-up from review of 09b721f. Whether the caret placed an insertion or a deletion in set text was inferred from where the diff stopped its common start (before the furthest place it could reach). A caret at the rightmost place of the run of equal text stops nothing, so its placement was taken for a guess and the edit moved to the first word edge: typing a space after two spaces (`a b` to `a b`, caret at 4) inserted it before them, and a collaborator's concurrent `Q` between the old spaces showed as `a Q b` instead of `a Q b`. `setTextEnds` now asks whether the caret anchors the edit (the text after it is unchanged: the condition diffStrings already uses, now one function, `caretAnchors`), and keeps the diff's placement whenever it does; only an edit no caret anchors goes to the first word edge. Second, an edit moved from where the diff put it was accepted with only one of its ends at a word edge, so it could take a piece of a word that stays: `a ba c` set to `a c` deleted `a b` (keeping the `a` of `ba` as the first word), and a collaborator's concurrent delete of `ba` left ` c`; `one someone end` set to `one end` lost `one` the same way, and the insertion mirror (`a c` set to `a ba c` while a collaborator deletes the old `a`) left `a b c`. A moved edit must now start and end at word edges of the longer text (` ba` here); where no place does, the diff's placement stays. Third, an edit already at word edges where the diff put it was moved too, to the first edge: an edit and a set in one update are composed without a caret, so a space added to two spaces (`Xa b` to `Xa b`) went before them, and a collaborator's `Q` between the old spaces showed after them. An edit now moves only where the diff puts it inside a word; everything else keeps main's placement. y.md and D30 say so, and that the glue left after a word deleted next to concurrent typing happens with any separator (a space, a tab, a comma, an emoji), as on main on the other side. ## Test plan New cells in `SetTextWordEdges.test.ts`: `setTextEnds` with a valid caret equals the caret-anchored diff (spaces, repeated letters, emoji, Han, punctuation, a word); a space typed after two spaces with the caret while a collaborator types between them, recorded and diffed by the writer, both id orders; a word set away (or added) next to a word with the same ending while a collaborator deletes one of them (`a ba c`, `one someone end`, `x ax ab`, and the insertions), both id orders; a letter typed and the text set in one update with a space added to two spaces while a collaborator types between them. Rebased on main with #9625. ### Before On 09b721f: ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/SetTextWordEdges.test.ts -t caret × "a b" set to "a b" with the caret at 4 × "aaa" set to "aaaa" with the caret at 4 × "😀😀" set to "😀😀😀" with the caret at 6 × "哈哈" set to "哈哈哈" with the caret at 3 × "a...b" set to "a....b" with the caret at 5 × a space typed after two spaces ‖ the peer types between them, recorded (ids 1,2) × a space typed after two spaces ‖ the peer types between them, diffed by the writer (ids 1,2) × a space typed after two spaces ‖ the peer types between them, recorded (ids 2,1) × a space typed after two spaces ‖ the peer types between them, diffed by the writer (ids 2,1) Tests 9 failed | 1 passed | 34 skipped (44) $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/SetTextWordEdges.test.ts -t 'same ending' Tests 10 failed | 34 skipped (44) $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/SetTextWordEdges.test.ts -t 'letter typed' Tests 2 failed | 42 skipped (44) ``` On main (551fe4a) the same-ending cells fail 2 of 10 (`x ax ab` set to `x ab` while a collaborator deletes `ax` leaves `x b`, the original #9636 shape). ### After ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/SetTextWordEdges.test.ts Tests 44 passed (44) $ pnpm run ci-check EXIT 0 $ vitest --run --project unit --project scripts-unit Test Files 639 passed | 4 skipped (643) Tests 27714 passed | 188 expected fail | 135 skipped (28037) $ MATRIX_FULL=1 vitest --run --project unit packages/lexical-y/src/__tests__/unit/matrix Tests 11 failed | 15998 passed | 5 expected fail | 128 skipped (16142) (the same 11 cells as main) $ FUZZ_SEED_START=1 FUZZ_SEEDS=3000 vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts Tests 1 failed | 123 passed (124) (seed 2277: dup, as on main) $ FUZZ_SEED_START=50000 FUZZ_SEEDS=2000 vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts Tests 7 failed | 77 passed (84) (seeds 50180, 50231, 50372, 51439, 51507, 51972, 51976, as on main)
…here the diff puts it inside a word, only between word edges, and never past a caret ## Description Follow-up from review of 09b721f. Whether the caret placed an insertion or a deletion in set text was inferred from where the diff stopped its common start (before the furthest place it could reach). A caret at the rightmost place of the run of equal text stops nothing, so its placement was taken for a guess and the edit moved to the first word edge: typing a space after two spaces (`a b` to `a b`, caret at 4) inserted it before them, and a collaborator's concurrent `Q` between the old spaces showed as `a Q b` instead of `a Q b`. `setTextEnds` now asks whether the caret anchors the edit (the text after it is unchanged: the condition diffStrings already uses, now one function, `caretAnchors`), and keeps the diff's placement whenever it does; only an edit no caret anchors goes to the first word edge. Second, an edit moved from where the diff put it was accepted with only one of its ends at a word edge, so it could take a piece of a word that stays: `a ba c` set to `a c` deleted `a b` (keeping the `a` of `ba` as the first word), and a collaborator's concurrent delete of `ba` left ` c`; `one someone end` set to `one end` lost `one` the same way, and the insertion mirror (`a c` set to `a ba c` while a collaborator deletes the old `a`) left `a b c`. A moved edit of set text must now start and end at word edges of the longer text (` ba` here); where no place does, the diff's placement stays. Text made again (#9598, #9625) keeps its own rule. Third, an edit already at word edges where the diff put it was moved too, to the first edge: an edit and a set in one update are composed without a caret, so a space added to two spaces (`Xa b` to `Xa b`) went before them, and a collaborator's `Q` between the old spaces showed after them. An edit now moves only where the diff puts it inside a word; everything else keeps main's placement. y.md and D30 say so, and that the glue left after a word deleted next to concurrent typing happens with any separator (a space, a tab, a comma, an emoji), as on main on the other side. ## Test plan New cells in `SetTextWordEdges.test.ts`: `setTextEnds` with a valid caret equals the caret-anchored diff (spaces, repeated letters, emoji, Han, punctuation, a word); a space typed after two spaces with the caret while a collaborator types between them, recorded and diffed by the writer, both id orders; a word set away (or added) next to a word with the same ending while a collaborator deletes one of them (`a ba c`, `one someone end`, `x ax ab`, and the insertions), both id orders; a letter typed and the text set in one update with a space added to two spaces while a collaborator types between them. Rebased on main with #9625. ### Before On 09b721f: ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/SetTextWordEdges.test.ts -t caret × "a b" set to "a b" with the caret at 4 × "aaa" set to "aaaa" with the caret at 4 × "😀😀" set to "😀😀😀" with the caret at 6 × "哈哈" set to "哈哈哈" with the caret at 3 × "a...b" set to "a....b" with the caret at 5 × a space typed after two spaces ‖ the peer types between them, recorded (ids 1,2) × a space typed after two spaces ‖ the peer types between them, diffed by the writer (ids 1,2) × a space typed after two spaces ‖ the peer types between them, recorded (ids 2,1) × a space typed after two spaces ‖ the peer types between them, diffed by the writer (ids 2,1) Tests 9 failed | 1 passed | 34 skipped (44) $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/SetTextWordEdges.test.ts -t 'same ending' Tests 10 failed | 34 skipped (44) $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/SetTextWordEdges.test.ts -t 'letter typed' Tests 2 failed | 42 skipped (44) ``` On main (551fe4a) the same-ending cells fail 2 of 10 (`x ax ab` set to `x ab` while a collaborator deletes `ax` leaves `x b`, the original #9636 shape). ### After ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/SetTextWordEdges.test.ts Tests 44 passed (44) $ pnpm run ci-check EXIT 0 $ vitest --run --project unit --project scripts-unit Test Files 644 passed | 4 skipped (648) Tests 27729 passed | 204 expected fail | 135 skipped (28068) $ MATRIX_FULL=1 vitest --run --project unit packages/lexical-y/src/__tests__/unit/matrix Tests 11 failed | 15998 passed | 5 expected fail | 128 skipped (16142) (the same 11 cells as main) $ FUZZ_SEED_START=1 FUZZ_SEEDS=3000 vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts Tests 1 failed | 123 passed (124) (seed 2277: dup, as on main) $ FUZZ_SEED_START=50000 FUZZ_SEEDS=2000 vitest --run --project unit packages/lexical-y/src/__tests__/unit/Fuzz.test.ts Tests 7 failed | 77 passed (84) (seeds 50180, 50231, 50372, 51439, 51507, 51972, 51976, as on main)
…edit cap of text made again counts, how repeated words are kept, and drop the 30 ms figure Follow-up to #9625's review notes on its docs (y.md "Edits made by code", D29, the `EXACT_EDITS` comment): - The cap of 2,048 word edits does not count words only added or only removed between a start and an end the shared words have in common (no search is needed there), so such a line keeps its words past the cap; the docs said only the start and end are kept past it. - "About 30 ms for a 5,000-word line" holds for short words only (5,000 words of 40 characters take longer); the docs now say the comparison stays linear in the line's length. - Where a word repeats next to a change, the docs now say which occurrence is kept and where a collaborator's concurrent edit of the old one lands (`aa bb` made again as `aa aa bb` keeps the first `aa`). No code change. The past-cap claim, against the code (all 33 shared words are kept although the shared words take 2,049 edits, all added between a shared start and end): ``` $ vitest --run --project unit <scratch check: keptStretches on that line, expecting 0 kept words> AssertionError: expected 33 to be +0 ``` The new wording matches that behavior; the repeated-word example, checked with the test harness (a collaborator types `X` in the old `aa` while `aa bb` is made again as `aa aa bb`, both id orders): ``` TIE [ 1, 2 ] aXa aa bb TIE [ 2, 1 ] aXa aa bb $ prettier --check packages/lexical-website/docs/collaboration/y.md packages/lexical-y/DECISIONS.md packages/lexical-y/src/internal/Inline.ts All matched files use Prettier code style! ```
Description
When an update removes a line's text nodes and makes new ones with other text (an agent rewriting a block from its JSON), the binding keeps the start and end the new text shares with the old (#9598) and writes everything between the first and the last changed word again as new text. With two or more changed words, a word a collaborator deleted between them at the same time came back on every client, and when the collaborator then undid its delete the word showed twice (
t1z t5zn9z t3z t4z t5z t6z n8z). A letter a collaborator typed into such a word landed outside it, and a format it gave the word was lost.Between the common start and end, the text of the nodes made now keeps every run of unchanged whole words, with the white space around it that both texts share, found by a word-by-word shortest edit script (Myers) of the replaced middle. Only the changed words are written, so a collaborator's concurrent delete there stays deleted (and is restored once by its undo), typed letters stay in their word and formats stay on it. Words are those of the existing whole-word rule (D30): runs of non-space characters, each character of a script written without spaces, an inline node's object character, a line feed. White space alone is never kept between two changes, as at the ends. The unchanged words kept are always a longest common subsequence of the two lines' words, found by Myers' linear-space diff (middle snake) over the words both texts have, up to 2,048 edits among those: O((n + m) * 2048) time at most, about 30 ms for a 5,000-word line, O(n + m) memory, and so does the writer's mapping of many kept runs onto many text nodes (scaling tests included). Past that bound (a line made again with most of its words moved), only the common start and end are kept, as before. A word is written again only where it is not kept: a word the update moved past kept words, and a word changed only in part (an insertion between two of its letters writes it whole, #9598), so a collaborator's undone delete of its unchanged part shows twice: #9635, documented and pinned as
test.fails.Remaking a line holding an inline node whose class reads its latest version in
isInline()(the playground'sEquationNode) threw "Lexical node does not exist in active editor state", on main too: the old line's layout asked the removed node in the new state. It is now read in the previous state (second changeset).DECISIONS.md D29 now states the rule, and y.md's "Edits made by code" paragraph and its stale "Text nodes made again with other text" Limits entry describe what is kept and what is still written again (changed words, a word whose joined punctuation changed, a word joined to a changed word through an inline node, an unchanged word that occurs more than once in a line with more than 128 changed words).
y.md's Open issues now lists #9597.
A stale format whose range reaches past a link the rewrite removes restates the link on kept text (#9597). With one changed word main already shows it on the kept end; keeping the unchanged words between two changes shows it there too.
RemadeTextStaleLink.test.tspins those 20 cells as expected failures by an exact predicate; #9601 (the #9597 fix) makes them pass (checked on this branch merged with #9601: 36/36), so the markers flip when it lands. #9601 should land first or together with this pull request (tested combined).Closes #9621
Changeset
.changeset/lexical-y-remade-text-unchanged-words.mdand.changeset/lexical-y-remade-line-inline-node.md(@lexical/ypatch,[fix], each with aVerify:paragraph).Test plan
New tests:
regressions/RemadeTextStretches.test.ts(the #9621 cases in both id orders, concurrent and unread, with and without the undo; other word shapes: a script without spaces, emoji, runs with different white space; a property over random lines of unique words in 1 to 4 text nodes, some bold or italic, made again with 2 to 4 word edits while a collaborator deletes, types into, deletes and undoes, types and undoes, or bolds an unchanged word, 120 seeds per cell; 64, 65 and 200 replaced words, in lines of distinct words, of a word repeated between the changes, of that with one unique word, and of repeated words with an earlier occurrence removed, with the same collaborator edits; an independent longest-common-subsequence check of the kept words over 1,100 random lines with repeated words up to 400 words long; a line made again in reverse past the bound; the #9635 cells; scaling tests for a long line with changes at both ends, a change every fourth word, and white space changed between every two words of one formatted node per word), 336 new cells inmatrix/C9RemadeText.test.ts(every inline shape of the catalog, a word changed in the first and the last text node, the collaborator types into, deletes from, or deletes from and undoest2zort3zbetween them, concurrent and unread, both id orders), 23 cells for every catalog inline shape made again from its JSON around changed words (JSON kept, no throw), andregressions/RemadeTextStaleLink.test.ts.Before
On main (fcea4ff):
The property fails 307 of its sessions on main (delete 103, type 96, delete and undo 81, bold 27). The 20
RemadeTextStaleLinkfailures on main are its expected-failure cells passing, because main writes the middle anew.After
The same fuzz seeds fail, with the same signatures, on main. Main: 25617 passed | 190 expected fail | 135 skipped; the difference is the new tests.
Whitespace-only remake of one alternately formatted node per word, 500 / 1,000 / 2,000 / 4,000 words: 46 / 89 / 113 / 256 ms (main: 70 / 70 / 108 / 210 ms).
keptStretchesalone on 5,000-word lines: 16 to 37 ms (every other word replaced, repeated words with one removed, 10 % of the words swapped, made again in reverse, a 20-word vocabulary with half the words changed).An adversarial review and downstream gates are pending and will be posted here.