Repository navigation
[lexical-website][lexical-y] Bug Fix: Keep the common start and end of a line whose text nodes an update makes again with other text - #9598
Conversation
…f a line whose text nodes an update makes again with other text ## Description When an update removes a line's text nodes and makes new ones with some of the text changed (an agent rewriting a block from its JSON, a highlighter making a line's tokens again), `@lexical/y` wrote again every character of each removed node whose text was not all there: a removed node's text was aligned whole or not at all (`TextReplay#stretch`, invariant 4). A collaborator's letter typed at the same time into an unchanged word then landed in front of the rewritten text, out of its word, and a character a collaborator deleted there came back (S9-7, the limit #9556 documents). The text of nodes made is now aligned as y-prosemirror aligns a text run: the text that left and the text made, as one string each, node boundaries and formats left out (a removed inline node too: the one made in its place is new), by their common start and end (`keptEnds`): anchored at the caret; an insertion or deletion keeps every other character, at a word boundary where it could be at several places; a replacement keeps whole words, and a kept end of only white space is written with it. Set text keeps main's alignment (invariant 4 stays for it), so `setTextContent` paths are unchanged. main's end-first check for made text (`sure`) is gone. ## Changeset `.changeset/lexical-y-remade-text-common-ends.md`: `[fix]` for `@lexical/y`, with a `Verify:` paragraph. ## Test plan New: `packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts` (S9-7 rows, the soak9 L2 S9-7 repro, the separator rows of #9565's review, concurrent token replacements, deletes the writer had not read, and a property test: random lines made again with one word edit while a collaborator deletes in an unchanged word, concurrent and unread) and `packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts` (contract cells over every inline shape of the test catalog and a highlighted code line). The S7-39 cases now expect only the changed word written. ### Before ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts Tests 151 failed | 121 passed (272) AssertionError: expected [ 'Qt63 t71z t13z t81z t15z', …(2) ] to deeply equal [ 't63Q t71z t13z t81z t15z', …(2) ] AssertionError: expected [ …(85) ] to deeply equal [] ``` ### After ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts Tests 272 passed (272) $ pnpm vitest run --project unit packages/lexical-y Tests 7746 passed | 46 expected fail | 4 skipped ``` (9 timing and heap thresholds failed in that run on a loaded machine and pass alone.)
… characters of inline nodes, and find an insertion's common end a chunk at a time
## Description
The previous commit left out of the text that a line's made text is compared with every run whose text is one object character (U+FFFC), which would also take a text node whose text is exactly that character. Runs of inline nodes are now marked (`Run#atom`) and only those are left out. `keptEnds` finds the common end of an insertion or deletion with 256-character chunks, as `diffStrings` does, instead of one character at a time.
## Test plan
### Before
The previous commit's tests pass; the change narrows which runs are left out and does not change the result for any of them.
### After
```
$ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts packages/lexical-y/src/__tests__/unit/regressions/TextIdentity.test.ts packages/lexical-y/src/__tests__/unit/MadeNodeEdits.test.ts packages/lexical-y/src/__tests__/unit/RewriteMerge.test.ts packages/lexical-y/src/__tests__/unit/RemadeText.test.ts packages/lexical-y/src/__tests__/unit/TextNewlines.test.ts packages/lexical-y/src/__tests__/unit/NodeSwapEdits.test.ts
Test Files 8 passed (8)
Tests 578 passed (578)
```
…e common start and end, and keep a line break alone at the edge of a change
## Description
A review of the previous commits found a delete coming back that main keeps: a leading tab (or a line break) of a line whose text nodes are made again with the word after it changed. The made text's common start was the tab alone, which `keptEnds` writes again as a separator alone, while main kept the removed tab node whole.
- The text of nodes made now keeps, between the common start and end it shares with the line's old text, what main's alignment of made text keeps (each removed node's text whole or not at all, and only what the diff that takes the common end first keeps too). So nothing main keeps is written again, unless the made text keeps that old character's place for another one (a piece of a removed node that main matched across a word: `t27z t` + `3` + `9z t3z ` made again as `t27z t3z`, where the line-wide comparison deletes `t39z ` and keeps `t3z`).
- A kept end that holds a line feed is no longer taken for a separator alone: a line break is a node of its own.
## Test plan
New cases in `packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts`: tab | `t1z t2z` made again with `t1z` → `t9z` while a collaborator deletes the tab; `"t1z" | br | **"t2z"**`, the br and `t2z` made again as br `t9z`, while a collaborator deletes the br.
### Before
```
$ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts -t "tab or line break"
AssertionError: expected [ '\tt9z t2z', '\tt9z t2z', …(1) ] to deeply equal [ 't9z t2z', 't9z t2z', 't9z t2z' ]
AssertionError: expected [ 't1z\nt9z', 't1z\nt9z', 't1z\nt9z' ] to deeply equal [ 't1zt9z', 't1zt9z', 't1zt9z' ]
Tests 4 failed | 44 skipped (48)
```
### After
```
$ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts packages/lexical-y/src/__tests__/unit/regressions/TextIdentity.test.ts packages/lexical-y/src/__tests__/unit/MadeNodeEdits.test.ts packages/lexical-y/src/__tests__/unit/RewriteMerge.test.ts packages/lexical-y/src/__tests__/unit/TextNewlines.test.ts
Tests 567 passed (567)
```
… word stays deleted in the shapes the review found ## Description An adversarial review of this branch found three shapes in which a collaborator's delete came back on 886770b, all of which main keeps and which the previous commits fixed: - a tab, line break or space-only node alone at an edge of the change; - a text node whose text is the object character (not an inline node); - the unchanged word next to an inline node, where leaving the inline node out of the comparison joined the words on its two sides. They are now tests, concurrent and unread, in both client id orders. ## Test plan ### Before ``` $ git checkout 886770b -- packages/lexical-y/src/internal && pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts -t "next to a remade word" AssertionError: expected [ '\tbeta', '\tbeta', '\tbeta' ] to deeply equal [ 'beta', 'beta', 'beta' ] AssertionError: expected [ 'beta\t', 'beta\t', 'beta\t' ] to deeply equal [ 'beta', 'beta', 'beta' ] AssertionError: expected [ '\nbeta', '\nbeta', '\nbeta' ] to deeply equal [ 'beta', 'beta', 'beta' ] AssertionError: expected [ ' beta', ' beta', ' beta' ] to deeply equal [ 'beta', 'beta', 'beta' ] AssertionError: expected [ 'alpha \ufffc gamma', …(2) ] to deeply equal [ Array(3) ] AssertionError: expected [ 'alphagamma', 'alphagamma', …(1) ] to deeply equal [ 'alhagamma', 'alhagamma', …(1) ] Tests 24 failed | 48 skipped (72) ``` ### After ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts -t "next to a remade word" Tests 24 passed | 48 skipped (72) ``` main (e28f4dd) passes them too.
… line writes again with the changed words ## Description The "Edits made by code" paragraph said that a separator alone between the changed words of a remade line and the text it keeps is written with them. It is only when it was part of a changed node: a tab, a line break or a node of spaces of its own keeps its item, as on main (the previous commits). The paragraph now says so. ## Test plan Documentation only. ### After ``` $ npx prettier --check packages/lexical-website/docs/collaboration/y.md Checking formatting... All matched files use Prettier code style! ```
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
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: |
|
AI review status for this pull request.
|
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
This looks good to me. Comparing the text of the nodes made with everything that left, as one string each, and keeping main's per-node alignment as a floor through within, is a clean way to stop rewriting unchanged words, and the new cases fail on main and pass here. A few optional, non-blocking suggestions:
1. An unchanged word joined to the changed word through an inline node is still written again (packages/lexical-y/src/internal/TextEdits.ts lines 1541-1553, packages/lexical-y/src/internal/Inline.ts lines 699-705)
Leaving a removed inline node's object character out of the compared text joins the words on its two sides, and the inline node made in its place is not in the arrived text either. When nothing separates the inline node from the changed word, the word snap in keptEnds drops the word on the other side too, and within only restores what main kept:
| Line (· = an inline decorator, made again) | Collaborator | Result (main and this PR) | Expected |
|---|---|---|---|
t1z t2z·t3z, t2z → t9z |
deletes the t of t3z |
t1z t9zt3z |
t1z t9z3z |
t1z·t2z t3z, t1z → t9z |
deletes the t of t2z |
t9zt2z t3z |
t9z2z t3z |
A Q typed inside t3z instead lands before t9z (t1z Qt9zt3z) in one client-id order and in the unread timing. This is no worse than main, but the new paragraph in packages/lexical-website/docs/collaboration/y.md (lines 676-686) and the changeset's Verify: paragraph say only the changed word is written, and the description lists the joined words as fixed. Treating a removed or made inline node's position as a word boundary when keptEnds snaps would close it; otherwise the page and the changeset could name the exception. A C9 cell with the inline decorator directly against the changed word would pin whichever you choose (the current equation/inline cell has spaces around it).
2. The extended suite has not run on this head. The Test plan says CI runs the browser and e2e suites, but Lexical Tests (Extended) skipped e2e-tests, browser-tests and integration-tests: .github/workflows/tests-extended.yml only opens for the extended-tests label or an approving review. collab-y-linux is the job that runs @lexical/y in a browser, so it is worth seeing it green before merge.
3. Cost figure. Making a 3,200-node line (alternating bold, one word changed) again runs about 16% slower than on main, above the 5-9% in the description; a keystroke in a long highlighted code line, where the highlighter remakes the tokens, is about 6% slower. Linear and limited to remakes, so not blocking; the description may want the updated figure.
| const inT = new Int32Array(d.length + 1); | ||
| for (let k = 0, pos = 0; k < left.length; k++) { | ||
| const text = textOf(left[k].run); | ||
| if (left[k].run.atom !== true) { |
There was a problem hiding this comment.
Optional, non-blocking: leaving the removed inline node out here joins the words on its two sides (the inline node made in its place is not in s either), so when nothing separates the inline node from the changed word, keptEnds' word snap drops the unchanged word on its other side too, and within only restores what main kept. For example t1z t2z·t3z (· an inline decorator) made again with t2z → t9z while a collaborator deletes the t of t3z ends as t1z t9zt3z on every client (main gives the same). Passing the inline node's position (in t and in s) to the snap as a word boundary would keep t3z whole.
There was a problem hiding this comment.
Kept as on main for now. A word joined to the changed word through an inline node, with no space between, is written again with it. The docs paragraph (y.md) and the changeset Verify: now name this exception (a0b227d). Treating the inline node position as a word boundary in the snap is a possible follow-up; the current equation/inline C9 cell keeps spaces around the inline node.
| place of removed ones keeps the start and end it shares with the line's | ||
| text as it was, by character, whatever nodes held it (as y-prosemirror | ||
| keeps a text run's): a block's text nodes made again from its JSON with | ||
| one word changed write that word alone, so a letter a collaborator types |
There was a problem hiding this comment.
Optional, non-blocking: this does not hold when an inline node sits directly against the changed word with no space: the word on the inline node's other side is written again, as on main, so a collaborator's delete there comes back. Either name that exception here and in the changeset's Verify: paragraph, or treat the inline node's position as a word boundary (see the note in TextEdits.ts).
There was a problem hiding this comment.
Named in this paragraph and in the changeset Verify: paragraph (a0b227d), as the exception for a word joined to the changed word through an inline node.
…o an edit computed before the update takes nothing out of the new word ## Description The L2 soak found two token losses on #9598 that main does not show. In both, a stale find-and-replace deletes exactly the characters of the old word it read, and some of those characters were part of the new word: - **8138978:** a comment started inside `t4z`. The rewrite to `t96z` kept the old `t`, because main's alignment, which the made text keeps between the common ends, takes the removed node `t3z t` whole. A stale `t4z` → `t98z` then deleted that `t`, leaving `96zt98z`. main loses the same `t` (`t98z96z`, which the soak counts as a split). - **8345429:** `t9z` made again as `t90z` is an insertion, which kept every old character. A stale `t9z` → `t94z` then deleted `t`, `9` and `z` of `t90z`, leaving `0t94z`. A word the update changed is now written whole: - **Insertions and deletions.** One keeps every other character only where it changes no word of the old text: between words, or a whole word with its separator. Inside a word, or glued onto one, it replaces the words at its ends whole, as a replacement does. - **Main's keeps between the common ends.** A piece of a word of the old text is no longer taken without the rest of the word. ## Test plan New cases in `packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts`, "a stale replacement of an old word takes nothing out of the new word", in both client id orders: - `t3z t` | **`4z t70z`** | ` t5z` made again as `t3z t96z t70z t5z` while a collaborator, from before the bold, replaces `t4z` with `t98z`; - `t8z t9z t10z` made again from its JSON with `t9z` → `t90z` while a collaborator replaces `t9z` with `t94z`. On main, the first fails (`t98z96z`) and the second passes. ### Before ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts -t "stale replacement" AssertionError: expected [ 't3z', '96z', 't98z', 't70z', 't5z' ] to include 't96z' AssertionError: expected [ 't8z', '0', 't94z', 't10z' ] to include 't90z' AssertionError: expected [ 't3z', 't98z96z', 't70z', 't5z' ] to include 't96z' Tests 4 failed | 72 skipped (76) ``` ### After ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts -t "stale replacement" Tests 4 passed | 72 skipped (76) $ pnpm vitest run --project unit packages/lexical-y Test Files 167 passed | 2 skipped (169) Tests 7787 passed | 46 expected fail | 4 skipped (7837) $ LEXICAL_Y_SEEDS=2000 pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts -t "comes back" ✓ unread: random lines made again with one word edit ‖ a delete in an unchanged word 34926ms ✓ concurrent: random lines made again with one word edit ‖ a delete in an unchanged word 23763ms ```
…in a highlighted line, and pin the stale-replace case main shares ## Description The previous commit also stopped the made text from keeping main's pieces of an old word between its common ends. That writes again the whole code token after a paste over part of a highlighted line, where the old token is glued to the pasted-over one (`t57zt80z`). #9585's paste cases keep that token, and they failed with it on top of #9585. Main's keeps are taken as before (identical to 1a232a6). Only the insertion or deletion inside a word stays replaced whole, which fixes soak L2 8345429. Soak L2 8138978 has the same shape on main: - the stale `t4z` → `t98z` deletes the `t` of `t4z`; - main and this change both keep that `t` as the first character of `t96z`, because a comment split `t4z` and main keeps the removed node `t` whole; - main shows `t98z96z`, whose `t` is the one t98z brought (checked by item ids in the soak's captures). It is now an expected-fail case that names this. ## Test plan ### Before ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/PasteRemadeLine.test.ts # #9585 + 4b1dbcd Tests 11 failed | 148 passed | 1 expected fail (160) ``` ### After ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/PasteRemadeLine.test.ts # #9585 + this head Tests 83 passed | 1 expected fail (84) $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts -t "stale replacement" Tests 2 passed | 2 expected fail | 72 skipped (76) $ pnpm vitest run --project unit packages/lexical-y Test Files 167 passed | 2 skipped (169) Tests 7785 passed | 48 expected fail | 4 skipped (7837) ```
…emade line writes again with the changed word ## Description The "Edits made by code" paragraph and the changeset's `Verify:` paragraph now say three things: - an insertion or deletion inside a word replaces that word whole, so an edit computed before it that replaces the old word takes nothing out of the new one; - a word joined to a changed word through an inline node, with no space between, is written again with it (the review bot's note on #9598, as on main); - the separator wording from before is kept. ## Test plan Documentation only. ### After ``` $ npx prettier --check packages/lexical-website/docs/collaboration/y.md .changeset/lexical-y-remade-text-common-ends.md All matched files use Prettier code style! $ pnpm run check-changesets Changeset check passed: 32 pending changeset(s) valid. ```
…rtion or deletion inside it, and keep an insertion where the caret placed it ## Description A round-2 review of the previous commits found two cases in which the insertion and deletion rule rewrote text that the earlier head kept. Both brought back a collaborator's delete or moved their typing: - **The word test was too wide.** It counted punctuation and emoji as word characters, so `foo.bar` made again as `fooX.bar` wrote `foo.bar` again, and a concurrent delete of `bar` came back. The same happened with `😀😁` made again as `😀X😁`. - **The caret was overridden.** Where the caret placed an insertion or deletion inside a word among equal text (`aa a aa` made again as `aa aa`, caret after the first `a`), the rule moved it to another occurrence, and a concurrent `Q` landed in the wrong word. Now: - **What counts as inside a word.** An insertion between two letters or digits of one word (`t9z` made again as `t90z`, soak L2 8345429), or a deletion with both ends inside words, replaces those words whole. At a word's edge, next to punctuation, an emoji or white space, every character is kept. Scripts written without spaces still count each character as a word. - **The caret.** Where the caret placed the insertion or deletion, it stays there. ## Test plan New cases in `packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts`, "an insertion or deletion in a remade line keeps the words around it", in both client id orders. ### Before ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts -t "keeps the words around it" # on a0b227d AssertionError: expected [ 'fooX.bar', 'fooX.bar', 'fooX.bar' ] to deeply equal [ 'fooX.', 'fooX.', 'fooX.' ] AssertionError: expected [ '😀X😁 tail', '😀X😁 tail', …(1) ] to deeply equal [ 'X😁 tail', 'X😁 tail', 'X😁 tail' ] AssertionError: expected [ 'aa Qaa', 'aa Qaa', 'aa Qaa' ] to deeply equal [ 'aaQ aa', 'aaQ aa', 'aaQ aa' ] Tests 6 failed | 76 skipped (82) ``` ### After ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts -t "keeps the words around it" Tests 6 passed | 76 skipped (82) $ pnpm vitest run --project unit packages/lexical-y Test Files 167 passed | 2 skipped (169) Tests 7785 passed | 48 expected fail | 4 skipped (7837) $ LEXICAL_Y_SEEDS=2000 pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts -t "comes back" ✓ unread: random lines made again with one word edit ‖ a delete in an unchanged word 44395ms ✓ concurrent: random lines made again with one word edit ‖ a delete in an unchanged word 19743ms ```
…ade-ends branch ## Description #9585 changed `TextReplay#align`'s `sure` argument into an array of readings: the text that left, and also that text with the passed text in place. This branch had restructured the call that took it. The resolution: - `align` keeps #9585's `sure` array and this branch's `made` flag; - the set passes take no readings, as before; - main's made pass, which `within()` adds between the line-wide common ends, takes #9585's two readings, `[{at, text: d}, {at: sureAt, text: was}]`; - the line-wide pass takes `[]` with `made = true`. ## Test plan ### After ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/PasteRemadeLine.test.ts packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts Test Files 3 passed (3) Tests 391 passed | 3 expected fail (394) $ pnpm vitest run --project unit packages/lexical-y Test Files 1 failed | 173 passed | 2 skipped (176) Tests 1 failed | 8156 passed | 59 expected fail | 2 skipped (8218) ``` The one failure is the R21-B timing threshold in `regressions/Scaling.test.ts` on a loaded machine (load average 30). It passes alone, twice: ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/Scaling.test.ts Tests 1 passed (1) ```
There was a problem hiding this comment.
Thanks, this looks good to me. Comparing the text of remade nodes with the old line as one string fixes the S9-7 rows (a collaborator's letter stays in its word and a deleted character stays deleted), keeps both tokens whole when the changed token is replaced at the same time, and writes only the changed word in the S7-39 cases. The new tests fail on main and pass with this change. The suggestions below are optional and non-blocking.
The whole-word guarantee is narrower than the changeset and docs say (packages/lexical-y/src/internal/Inline.ts:701). inside treats a word as a run of letters and digits, while snapToWords (the replacement rule) treats v1.2.3 as one word. An insertion or deletion at a word's edge or next to punctuation therefore keeps the old characters, and a concurrent replacement of the old word takes them out of the new one. Where the word was a node of its own (a bold word, a link, a highlighted code token), main wrote it whole:
| Line, made again as one node | Update | Collaborator, at once | main |
This PR |
|---|---|---|---|---|
use + bold v1.2.3 + now |
v1.2.3 → v1.20.3 |
v1.2.3 → v2.0.0 |
use v2.0.0v1.20.3 now |
use 0v2.0.0 now |
x + bold t49z + y |
t49z → t49 |
t49z → t77z |
x t77zt49 y |
x t77z y |
Prism line const v = '1.2.3'; |
1.2.3 → 1.20.3 |
1.2.3 → 2.0.0 |
const v = 2.0.0'1.20.3'; |
const v = '02.0.0'; |
This matches how typed edits merge, so I don't think it needs to block. But the Verify: paragraph in .changeset/lexical-y-remade-text-common-ends.md ("leaves both words whole") and the paragraph in packages/lexical-website/docs/collaboration/y.md ("takes nothing out of the new one") promise more. CONTRIBUTING.md asks a fix's Verify: paragraph to say what to test, so it would help to narrow both to a replacement or an edit between two letters or digits, and to say that an edit at a word's edge or next to punctuation keeps the old characters, as typed text does. Alternatively, the whole-word rule could take its boundary from isWordChar when the old word was a node of its own.
Pin the chosen behaviour. The token rows in RemadeTextEnds.test.ts are all replacements, and the property test's collaborator only deletes, so one row for a shape from the table above would keep this from drifting either way.
Test plan output. The pasted Before and After output predates the merge with main. AGENTS.md asks for the real output of the command that was run, so re-pasting the run on the merged head would keep the record exact.
| // Inside a word: between two letters or digits of one (`t9|z`), not | ||
| // next to punctuation, an emoji or white space, and not in a script | ||
| // written without spaces, where each character is a word. | ||
| const inside = (i: number): boolean => isAlnum(a, i - 1) && isAlnum(a, i); |
There was a problem hiding this comment.
Optional: inside uses isAlnum, which stops at punctuation, while snapToWords treats v1.2.3 as one word. So v1.2.3 → v1.20.3 (or an edit at a word's edge, t49z → t49) keeps the old characters, and a concurrent replacement of the old word gives use 0v2.0.0 now / x t77z y, where main kept both tokens whole when the word was a node of its own (a bold word, a highlighted code token). If that typed-edit behaviour is intended, the changeset's Verify: and the y.md paragraph could say so; otherwise the boundary here could come from isWordChar.
There was a problem hiding this comment.
Kept as a typed-text merge on purpose, and documented (bccb0c3). Taking the boundary from isWordChar here (4b1dbcd and 3236c48 did) rewrote a word joined by punctuation or emoji: foo.bar made again as fooX.bar wrote bar again, so a collaborator's concurrent delete of bar came back (round-2 review). With isAlnum, fooX.bar keeps it deleted, and the cost is the shape in the table. The y.md paragraph and the changeset now say that an edit at a word's edge or next to punctuation merges as typed text does. Your three rows are pinned in "an edit at a word's edge merges as typed text does".
|
|
||
| [fix] Keep the unchanged start and end of a line whose text nodes an update makes again with other text, so a collaborator's concurrent edit there stays where it was made | ||
|
|
||
| Verify: an update that removes a line's text nodes and makes new ones with one word changed (an agent rewriting a block from its JSON, a highlighter making a line's tokens again) writes that word alone (and a word joined to it through an inline node with no space between). A letter a collaborator types into another word of the line at the same time stays in that word, a character a collaborator deletes there stays deleted, and a collaborator's concurrent replacement of the changed word leaves both words whole. |
There was a problem hiding this comment.
Optional wording: the last clause holds for a replacement, or an insertion or deletion between two letters or digits. At a word's edge (file → files) or next to punctuation (v1.2.3 → v1.20.3), a concurrent replacement of the old word gives x notes y (or x snote y, by client id order) or use 0v2.0.0 now. For example: "…and a collaborator's concurrent replacement of a word the update replaced, or edited between two letters or digits, leaves both words whole."
There was a problem hiding this comment.
Narrowed along the lines you suggested (bccb0c3): "...a collaborator's concurrent replacement of a word the update replaced, or edited between two letters or digits, leaves both words whole; an edit at a word's edge or next to punctuation merges as typed text does."
… a remade line write a word whole, and pin the typed-text merge at a word's edge ## Description The review bot pointed out that the "Edits made by code" paragraph and the changeset's `Verify:` paragraph promise more than the code does: - a replacement writes whole words; - so does an insertion between two letters or digits of a word, or a deletion inside one; - an insertion or deletion at a word's edge or next to punctuation keeps the old characters, as typed text does. So a collaborator's concurrent replacement of that word takes them out of the new one: `t49z` → `t49` against `t49z` → `t77z` gives `t77z`. main wrote a word that was a node of its own whole and kept both. Writing such a word whole would also write again a collaborator's delete in a word joined to it by punctuation (`fooX.bar` while a collaborator deletes `bar`), which the code keeps deleted. Both texts now say what holds, and new test rows pin the typed-text merge. ## Test plan New rows in `packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts`, "an edit at a word's edge merges as typed text does": - `t49z` → `t49`; - `file` → `files`; - `v1.2.3` → `v1.20.3`; each while a collaborator replaces the word, in both client id orders. ### Before ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts # on main 2fe8056 Test Files 2 failed (2) Tests 161 failed | 153 passed | 2 expected fail (316) AssertionError: expected [ 'Qt63 t71z t13z t81z t15z', …(2) ] to deeply equal [ 't63Q t71z t13z t81z t15z', …(2) ] AssertionError: expected [ …(85) ] to deeply equal [] AssertionError: expected [ …(73) ] to deeply equal [] ``` ### After ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts packages/lexical-y/src/__tests__/unit/regressions/TextIdentity.test.ts packages/lexical-y/src/__tests__/unit/regressions/PasteRemadeLine.test.ts Test Files 4 passed (4) Tests 534 passed | 3 expected fail (537) $ pnpm vitest run --project unit packages/lexical-y Test Files 2 failed | 172 passed | 2 skipped (176) Tests 2 failed | 8161 passed | 59 expected fail | 2 skipped (8224) ``` The two failures are the timing and heap thresholds of `DisposeMemory.test.ts` and `ServerScale.test.ts` on a loaded machine. They pass alone, here and on main: ``` $ pnpm vitest run --project unit packages/lexical-y/src/__tests__/unit/DisposeMemory.test.ts packages/lexical-y/src/__tests__/unit/ServerScale.test.ts Tests 3 passed (3) ```
There was a problem hiding this comment.
Thanks, this looks good to me. Comparing the text of remade nodes with the line's old text as one string fixes the S9-7 rows (a collaborator's letter stays in its word and a deleted character stays deleted), the S7-39 cases now write only the changed word, and the new tests fail on main and pass with this change. The typed-text merge at a word's edge is documented and pinned, and the stale-replace shape shared with main is tracked in #9608, so I'm not treating either as a blocker. Two optional, non-blocking suggestions:
1. The whole-word promise for an edit inside a punctuated token (packages/lexical-website/docs/collaboration/y.md:711-714, .changeset/lexical-y-remade-text-common-ends.md:7). The docs say an insertion between two letters or digits replaces the word whole, so that an edit computed before it "takes nothing out of the new one", and the Verify: paragraph says both words stay whole. In keptEnds the word is a run of letters and digits (isAlnum in packages/lexical-y/src/internal/Inline.ts), so inside a token joined by punctuation only that run is written again, and a concurrent replacement of the whole token takes the rest out of the new one:
| Line, made again as one node | Update | Collaborator, at once | Result |
|---|---|---|---|
pi is + bold 3.14 + ok |
3.14 → 3.104 |
3.14 → 2.71 |
pi is 2.71104 ok |
n 12.34 end |
12.34 → 12.354 |
12.34 → 56.78 |
n 56.78354 end or n 35456.78 end, by client id |
A word of letters and digits behaves as promised (t9z → t90z against t9z → t94z gives t94zt90z). CONTRIBUTING.md asks a fix's Verify: paragraph to say what to test, so narrowing both texts to a word of letters and digits (in 3.14, only 14 is written again), and adding one such row next to "an edit at a word's edge merges as typed text does" in RemadeTextEnds.test.ts, would keep the docs, the changeset and the tests matched to the code.
2. A quadratic step in long remakes (packages/lexical-y/src/internal/TextEdits.ts:1616-1623). next calls toLeft([filled[i]], runs) once per block, and toLeft starts its run cursor at 0 on every call, so this step is O(blocks × runs). Remaking a line of alternating plain and bold tNz nodes with one word changed measured about 22% slower than main at 8,000 nodes and 38% at 16,000; mapping each run of consecutive non-before blocks with a single toLeft call brought that to about 8% and 3%, with RemadeTextEnds, C9RemadeText and PasteRemadeLine still passing. Lines that long are rare, so this is optional.
| if (j < before.length && filled[i] === before[j]) { | ||
| next.push(kept[j++]); | ||
| } else { | ||
| next.push(...toLeft([filled[i]], runs)); |
There was a problem hiding this comment.
Optional, non-blocking: toLeft starts its run cursor at 0 on every call, so calling it once per block makes this loop O(blocks × runs). Remaking a line of alternating plain and bold tNz nodes with one word changed measured about 38% slower than main at 16,000 nodes (22% at 8,000); collecting the blocks that are not before blocks and mapping each run of them with one toLeft call brought that to about 3% (8% at 8,000), with RemadeTextEnds, C9RemadeText and PasteRemadeLine still passing. Keeping one run cursor across the loop would also cover lines where before blocks interleave.
| replaces whole words, as below, and so does an insertion between two | ||
| letters or digits of a word or a deletion inside one, so that an edit | ||
| computed before it that replaces the old word takes nothing out of the new |
There was a problem hiding this comment.
Optional, non-blocking: in keptEnds the word here is a run of letters and digits (isAlnum), so inside a token joined by punctuation only that run is written again. pi is 3.14 ok made again as pi is 3.104 ok while a collaborator replaces 3.14 with 2.71 gives pi is 2.71104 ok (or pi is 1042.71 ok, by client id): the concurrent replacement does take 3. out of the new token. Saying "a word of letters and digits (in 3.14, only 14)" would match the code.
|
|
||
| [fix] Keep the unchanged start and end of a line whose text nodes an update makes again with other text, so a collaborator's concurrent edit there stays where it was made | ||
|
|
||
| Verify: an update that removes a line's text nodes and makes new ones with one word changed (an agent rewriting a block from its JSON, a highlighter making a line's tokens again) writes that word alone (and a word joined to it through an inline node with no space between). A letter a collaborator types into another word of the line at the same time stays in that word, a character a collaborator deletes there stays deleted, and a collaborator's concurrent replacement of a word the update replaced, or edited between two letters or digits, leaves both words whole; an edit at a word's edge or next to punctuation merges as typed text does. |
There was a problem hiding this comment.
Optional, non-blocking: the same narrowing as the docs paragraph applies here. "edited between two letters or digits, leaves both words whole" holds for a word of letters and digits; for 3.14 made again as 3.104 against a concurrent 3.14 → 2.71, only 104 of the new token survives. CONTRIBUTING.md asks a fix's Verify: paragraph to say what to test, so naming the letters-and-digits case keeps it testable.
## Description Brings in main at d61302b. One conflict, in `docs/collaboration/y.md`'s "Edit text in place" bullet: main (#9598) now says what collaborators type into a replaced line survives at the start of the new text (or at its end when typed at the end of the line); this branch still had the older wording that it lands before the new text. The resolution keeps main's wording, which describes main. ## Test plan ``` $ pnpm exec prettier --check <the six changed files> All matched files use Prettier code style! $ pnpm run check-changesets Changeset check passed: 53 pending changeset(s) valid. ```
… 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.
…n of text set without a caret at a word edge Text set whole without a caret (`setTextContent` from code, a node's text set again) is diffed against the old text. For an insertion or a deletion, the diff took the common start first, so where equal letters allowed it put the edit inside words: `t1z t2z t3z` set to `t1z t3z` deleted `2z t`. A collaborator's concurrent delete of `t2z` then left `t1z 3z` on every client (a letter of the untouched `t3z` lost), and a word they typed after `t2z` split `t3z` (`t1z t q9z3z`). An insertion or a deletion of set text now goes to the first word edge of the run of equal text it could be in (where it changes no word it does not add or remove), unless the caret placed it, which keeps priority as before. The first edge deletes a word with the space before it (` t2z`), so a word a collaborator types after it at the same time, as typing goes (a space, then the word), keeps the space after it and stays a word of its own. The placement is the one text made again already used (#9598), shared now as `placeAtWordEdge`; set text keeps every other character as before (no whole-word widening), and replacements still snap to whole words (D30). What remains is documented in y.md: a word a collaborator types right against the deleted word on the side of the space deleted with it (before it; after a line's first word, which has no space before it) is glued to the word beyond. D30 records the placement. Closes #9636 `regressions/SetTextWordEdges.test.ts`: fixed cases (both delete `t2z`; the collaborator types ` q9z` after `t2z`; both delete the third of four words; the set adds a word while the collaborator deletes the first), concurrent and unread, both id orders; and a property over random lines of words that share letters, set with one word deleted or inserted while a collaborator deletes a word or types one after or before a word, 150 seeds per cell: every word whole, deleted words stay deleted, added words once, every client and a fresh reader agree; the documented glue counted apart. On main (551fe4a): ``` $ vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/SetTextWordEdges.test.ts × both delete t2z, concurrent (ids 1,2) × the peer types " q9z" after t2z, concurrent (ids 1,2) × both delete t3z of four, concurrent (ids 1,2) ... (12 fixed cells) × concurrent: one word deleted or inserted by setTextContent ‖ the peer deletes a word × concurrent: one word deleted or inserted by setTextContent ‖ the peer type afters a word × concurrent: one word deleted or inserted by setTextContent ‖ the peer type befores a word ... (6 property cells: 69 sessions fail on delete, 27 on typing after, 38 on typing before, of 300 each) Tests 18 failed | 4 passed (22) AssertionError: expected [ 't1z 3z', 't1z 3z', 't1z 3z' ] to deeply equal [ 't1z t3z', 't1z t3z', 't1z t3z' ] ``` ``` $ LEXICAL_Y_VERBOSE=1 vitest --run --project unit packages/lexical-y/src/__tests__/unit/regressions/SetTextWordEdges.test.ts Tests 22 passed (22) glued: delete 0 + 0, type after 5 + 3, type before 11 + 16 (of 150 per cell; documented) $ pnpm run ci-check EXIT 0 $ vitest --run --project unit --project scripts-unit Test Files 637 passed | 4 skipped (641) Tests 27180 passed | 166 expected fail | 135 skipped (27481) $ MATRIX_FULL=1 vitest --run --project unit packages/lexical-y/src/__tests__/unit/matrix Tests 11 failed | 15639 passed | 5 expected fail | 128 skipped (15783) (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) ``` A randomized session generator (remade lines with concurrent comments, formats, typing and deletes, undo; agents writing with `setTextContent`), 500 sessions of 5 rounds, this change vs main: lost words 0 vs 12, mangled words 0 vs 19, split attributes 0 vs 1; no class higher than 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)
Supersedes #9565, which I am closing in favor of this simpler rule.
Description
When an update removes a line's text nodes and makes new ones with some of the text changed (an agent rewriting a block from its JSON, a highlighter making a line's tokens again),
@lexical/ywrote again every character of each removed node whose text was not all there (TextReplay#stretch, invariant 4: a removed node's text is aligned whole or not at all). A letter a collaborator typed into an unchanged word at the same time then landed in front of the rewritten text, outside its word, and a character a collaborator deleted there came back (S9-7; #9556 proposes documenting it as a limit).This change applies y-prosemirror's rule (
updateYText,simpleDiff) to the text of nodes made. Set text keeps main's alignment.t69z→t81z; a collaborator concurrentlyQaftert63Qt63 t71z t13z t81z t15zt63Q t71z t13z t81z t15z7oft71z(also when the writer had not read the delete yet)7comes backThe rule
In
TextReplay#stretch, between two pins (old characters the edit record keeps in place):keptEndsinInline.ts):simpleDiffWithCursoris: where the change is ambiguous (repeated characters), the split goes at the caret.t9zmade again ast90z), or a deletion with both ends inside words, replaces those words whole, as a replacement does. An edit computed before the update that replaces the old word (a stale find and replace) then takes nothing out of the new word (soak L2 8345429:0t94zon an earlier head). At a word's edge (file→files,fooX.bar, next to punctuation, an emoji or white space) every character is kept, as typed text keeps it. A collaborator's concurrent replacement of that word then takes those characters out of the new one:t49z→t49againstt49z→t77zgivest77z. Main wrote a word that was a node of its own whole and kept both. Writing such words whole would also write again a collaborator's delete in a word joined to it by punctuation (barinfoo.bar), so the typed-text merge is kept and pinned. Where an insertion or deletion could sit at several places, it goes where the caret placed it, else at the last place inside no word.snapToWords, as every replacement the writer diffs), so two concurrent replacements of a token leave both tokens whole (pi is 2.713.15 ok, never2.7115).Tip:truly).Set text (
setTextContent, text a set replaced) is aligned exactly as on main, and invariant 4 still holds for it. The rule needs no word segmenter and no per-script tables. The source change is 173 added and 57 removed lines inTextEdits.ts(51 of the added lines are comments) and about 150 added lines inInline.ts.Why a collaborator's delete cannot come back
Unreadview ([lexical-y] Bug Fix: Write a local update made before an unread remote change on the document it was made on #9523; [lexical-y] Refactor: One module answers what a binding's editor sees of Yjs #9533'sSeenis that same view). So a character that a remote delete removed before the editor read it is still in the compared text. If it is kept, its item stays deleted. If it is in the changed words, it is deleted with them. It is never re-inserted as old text. The "a delete the editor has not read yet stays deleted" cases and the property test apply the delete in the same task, before the writer commits.oe twoXcoming back asone twoX, a set's character main keeps being written again) cannot happen. The one exception is a piece of a removed node that main matched across a word boundary:t27z t+3+9z t3zmade again ast27z t3z, where main gave thet3of the deletedt39zto the newt3z. There the line-wide comparison keepst3zitself, so a collaborator's delete int3zholds where it came back on main.By design, as in y-prosemirror: the changed words, and the text between two separate changes, are written as new. A collaborator's concurrent edit inside a word that the update changed loses to the update's word, as it does with any concurrent replacement of that word.
Shared with main, and pinned as an expected-fail test. Main keeps a removed node whole even where that node ends inside a word the update changed (
t3z tbefore4z). The made text keeps what main keeps, so it keeps that piece too, here thetoft4z, as the first character of the new wordt96z. A stale replace oft4zthen deletes it on both (soak L2 8138978; main showst98z96z, whosetis the onet98zbrought, as the item ids in the soak's captures show). Writing that piece again would also write again whole code tokens kept next to a paste (#9585's PasteRemadeLine cases). Follow-up: #9608.A word joined to the changed word through an inline node, with no space between, is written again with it, as on main (from the review bot's note). The docs and the changeset name this.
What it fixes
t63z→t80zinstead oft63z→t80z).t49z, lost-split) and this change does not.Not fixed: #9587 (typing after a paste that starts like the next word, in a code line). It fails the same way on main and here: the pasted text glues onto the old word
t57zt80z, which is then a changed word.Changeset
.changeset/lexical-y-remade-text-common-ends.md:[fix]for@lexical/y, with aVerify:paragraph.Test plan
New tests:
packages/lexical-y/src/__tests__/unit/RemadeTextEnds.test.ts:L2-StaleRewriteDisplacesrepro (8 cases);Note:,Tip:,the build,Step 1:: no glued words, no lost colon or space);pi is 3.14,don't,v1.2.3);packages/lexical-y/src/__tests__/unit/matrix/C9RemadeText.test.ts: contract cells (test process v2) over the inline shapes of the test catalog ([lexical-y] Chore: Test contracts over catalogs of node shapes, block id formats and configurations #9588'snodes.ts):TextNodesubclass and mode (normal, formatted, token, segmented, tab, hashtag), line breaks, links, autolinks, comments (MarkNode, overlapping), a link in a comment, an inline decorator;The cells use a local list of the catalog's shape names, to be folded into the catalog.
The S7-39 cases in
regressions/TextIdentity.test.tsnow expect only the word to be written.Before
On main (2fe8056) with the new test files:
After
On this branch (bccb0c3, main merged in):
The two full-suite failures are timing and heap thresholds on a loaded machine (load average 22); they pass alone, here and on main.
The property test was also run at a higher seed count:
In the Before output,
…(85)and…(73)are the property test's failing seeds out of 120 on main, unread and concurrent; the 2 expected fails are the stale-replace case shared with main (#9608).FUZZ_GC_SEEDS=1000)FUZZ_GC_SEEDS=200)@lexical/yunit test, captured per test (7,047 tests stable across two main runs)TextNewlines(one tworeplaced by a newone\ntwonode)t49zHow to read the soak numbers:
replaceAllthat deletes exactly the base characters it read. The deletion matches main; only the placement of the surrounding text differs.fmt-untouchedtraced on a path shared with main is Bug: [lexical-y] A link a collaborator removes comes back when another collaborator formats a range covering it at the same time #9597: a stale format over a link's range restates the link that a newer update removed. Main does the same to text that is not made again. With this PR the rewritten line keeps that word, so the stale format lands on it.Adversarial review (gpt-6.1-sol, with sub-reviewers) of the intermediate heads found three must-fix cases, all fixed and now tests ("a collaborator's delete next to a remade word stays deleted"):
Its verdict on 1a232a6 is clean. Coverage included ambiguity and separator matrices over Unicode (surrogates, ZWJ, combining marks, Han, Thai), carets and composition keys, 800 seeded set and remake histories (0 failures here, 69 resurrections on main), and timings. A second round is reviewing the insertion and deletion rule.
Cost. The review bot measured a 3,200-node line of alternating bold, made again with one word changed, at about 16% slower than main, and a keystroke in a long highlighted code line at about 6% slower. Both are linear and limited to remakes.
Other limits: #9587 fails the same way here and on main. Whole-token atomicity across a format boundary under concurrent replacement is not guaranteed: the review measured 54 of 600 seeds failing here and 541 on main, and every one of the 54 also fails on main.
Browser and e2e suites were not run locally. The extended workflow (
collab-y-linuxand the other browser, e2e and integration jobs) opens on an approving review or theextended-testslabel, so it should run on the next push after the review bot's approval.Review bot notes (approved, optional items)
Verify:. Treating the inline node's position as a word boundary when snapping is a possible follow-up.Second review (approved, optional items):
Verify:paragraph now say which edits write a word whole: a replacement, an insertion between two letters or digits, or a deletion inside a word. They also say that an edit at a word's edge or next to punctuation merges as typed text does. The code keeps that merge for the reason given under "The rule".t49z→t49,file→files,v1.2.3→v1.20.3, each against a concurrent replacement of the word).Soak follow-up (after the first push)
Two token losses showed up only with this PR in tp2's L2 soak:
t9zmade again ast90z, then a stalet9z→t94z, giving0t94z): fixed by the insertion and deletion rule above. It is now a test, and the soak re-run on the fixed head loses no token there.Soak on 3236c48 (soak9's 400-seed bucket, 399 seeds, vs main cad9015):
2zoft2z, a fresh rewrite keeping the removed nodet1z twhole (main's keep), and a stalet2z→t110zdeleting thattfromt108z;f0format record with no visible change (3700391).A second GPT review then found two must-fix cases in the insertion rule, both fixed and now tests ("an insertion or deletion in a remade line keeps the words around it"):
fooX.barwrotefoo.baragain and a concurrent delete ofbarcame back;Merged with main (#9585, #9578, #9555) as 62c0477. In
TextEdits.ts,alignkeeps #9585'ssurearray and gainsmade. The made-text alignment of main thatwithin()adds takes #9585's two readings, and the line-wide pass takes none. Results on the merged tree:@lexical/ysuite: 8156 passed + 59 expected fail, plus one R21-B timing threshold under load that passes alone;Gates on the fixed head (3236c48), rerun:
@lexical/ysuite: 7785 passed + 48 expected fail;