Repository navigation
Conversation
…do or redo press changes what the document shows, or empties its stack ## Description Yjs's `UndoManager` skips an undo step only when it changes no item. A step that restores or removes only format items no reader shows is taken alone, and the press shows nothing; the next press then undoes what the first was meant to. On main this happens when a client formats text a collaborator then deletes: the format's items stay around the deleted text, and undoing the format removes only them. Keeping run ends in the collapse (#9617, parked) made such presses more common, and in the randomized sessions of its gate the press that did nothing was sometimes the one meant to remove text. Now `YBinding#applyUndo` compares a digest of what any reader can show (the live content in order with its format values, the attributes and the nested types) before and after each step and, while it is unchanged and steps remain, takes the next step in the same press. A press a stack listener starts while an undo runs takes one step, as before (the running step is not done yet). This is a behaviour change to undo: a press now takes more than one step wherever a step showed nothing. DECISIONS.md D81 records it as `OPEN`, for a maintainer's call; y.md's undo paragraph says it. ## Test plan ### Before `pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/UndoInvisibleSteps.test.ts` on main (1853707): ``` × undoing a format on text a collaborator deleted takes the step before it too (1,2) × undoing a format on text a collaborator deleted takes the step before it too (2,1) ✓ undoing a comment removal on deleted text takes the format change before it too ``` `UNDO_PRESS_SEEDS=200 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/UndoPressProperty.test.ts` on main: ``` AssertionError: expected [ 'seed 2: undo press 0', …(9) ] to deeply equal [] ``` ### After ``` ✓ |unit| packages/lexical-y/src/__tests__/unit/regressions/UndoInvisibleSteps.test.ts (3 tests) ✓ |unit| packages/lexical-y/src/__tests__/unit/UndoPressProperty.test.ts (2 tests) ``` with 200 sessions per client id order, each press checked against a fresh read of the document.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
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: |
… what the document shows once per undo press
## Description
The review of the undo-press change noted that `visibleDigest` and `contentDigest` sat between `YBinding`'s `@experimental` comment and the class, so the exported class lost its doc comment; they now sit above it. A press also reads what the document shows once before its first step instead of before each step: a step that changed nothing visible leaves it as it was.
## Test plan
### Before
The class declaration had no doc comment; tests as before.
### After
`pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/UndoInvisibleSteps.test.ts packages/lexical-y/src/__tests__/unit/UndoPressProperty.test.ts packages/lexical-y/src/__tests__/unit/UndoRunEnds.test.ts packages/lexical-y/src/__tests__/unit/YjsContract.test.ts`:
```
Tests 144 passed | 81 expected fail (225)
```
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for the detailed description and the real before/after output. The loop in YBinding#applyUndo does what D81 describes: a step that only restores or removes format items around deleted text is now taken together with the next one, and the regression scenario fails on main and passes here. A few suggestions, then the one decision this needs from a maintainer.
Suggestions
- Per-press cost (
packages/lexical-y/src/internal/Binding.ts:5229and:5239).visibleDigestwalks the whole root before the first step and again after each step, so the common one-step press pays two full walks. In a quick benchmark undoing one typed character (self-check off), a press went from 0.42 ms to 11.74 ms on a 131,998-character document and from 0.33 ms to 24.11 ms at 273,898 characters, in line with the numbers in the description. The changed-items check the description proposes is the right fix;scanStackItemalready iterates the top stack item's inserts and deletes before the pop, which is a natural place for it. Until then,contentDigest(Binding.ts:674) could readc.strfor aContentStringinstead ofJSON.stringify(c.getContent()), which in@y/y14.0.0-rc.29 splits the string into one array entry per character. - Minimality in the property test (
packages/lexical-y/src/__tests__/unit/UndoPressProperty.test.ts:153). A press fails only when nothing changed and the stack is not empty, so an implementation that emptied the stack on every press would pass. SnapshottingY.encodeStateAsUpdateafter each step and requiring every non-final snapshot to read likebeforewould pin the other half of D81 (a press takes the next step only while nothing shows). That check passes on this head over 200 seeds per client-id order. - The command path. The new tests call
binding.undo()outside an update. A case that dispatchesUNDO_COMMAND/REDO_COMMANDon the regression scenario would cover the queued path (queueFirst) that a keyboard undo takes; it behaves correctly on this head. - Nit: where D81 is filed (
packages/lexical-y/DECISIONS.md:61). The row sits in the Storage model table betweenDECIDEDrows; the register's legend and its## Opensection are whereOPENrows live until they are decided (packages/lexical-y/AGENTS.mdasks for the row, which is here).
Question for maintainers (D81)
Should an undo or redo press in @lexical/y keep taking UndoManager steps until a fresh read of the document changes, rather than Yjs's one step per press where a press can show nothing? If yes, can the whole-document digest ship until the changed-items check replaces it, or should that check land in this pull request?
| try { | ||
| // What the document shows before the press: a step that changed | ||
| // nothing visible leaves it as it was. | ||
| const before = visibleDigest(this._root); |
There was a problem hiding this comment.
This digest and the one after each step walk the whole root, so the usual one-step press pays two full walks. Undoing one typed character (self-check off) went from 0.42 ms to 11.74 ms at 131,998 characters and from 0.33 ms to 24.11 ms at 273,898. The changed-items check from the description fixes it; until then, reading c.str for a ContentString in contentDigest (line 674) avoids getContent()'s per-character array.
| t.a.binding.redo(); | ||
| } | ||
| const after = await freshView(t.a); | ||
| if (after === before && stack.length > 0) { |
There was a problem hiding this comment.
This catches a press that shows nothing, but not one that takes a step too many: an applyUndo that emptied the stack on every press would pass. Snapshotting Y.encodeStateAsUpdate(t.a.doc) after each step and requiring every non-final snapshot to read like before would cover that half of D81.
| | D24 | **Yjs's format cleanup is off** in `@lexical/y` containers and on every relay that loads a state (`createYDoc`). A delete keeps the ends of each deleted formatted run, within a bound per format key. Not reported upstream. | The cleanup deletes format items a concurrent writer may still rely on. Upstream already knows that skipping it is the app's trade-off. | `DECIDED` | 10-28 | [#9400], [y.md: Servers][y-servers] | | ||
| | D25 | **Undo is Yjs's `UndoManager`**, scoped to the binding's root and this client's own edits (500 ms capture by default). It never deletes a container a collaborator has put content in (its `deleteFilter`). Do not combine it with `HistoryExtension`. | Collaborative undo must not revert other people's work; keeping a container whole once someone else wrote in it matches y-prosemirror's protected nodes and is cheap to maintain. The `UndoManager` was chosen on 10-23; keeping containers whole was confirmed on 10-30. | `DECIDED` | 10-30 | [#9400], [y.md: Limits][limits] | | ||
| | D26 | **Undo favours people taking turns over concurrent races.** Where a fix makes undo correct for sequential edits at the cost of rare races between an undo and a simultaneous edit of the same text, the sequential case wins. | Taking turns is the common case; a race with a simultaneous edit of the same text is rare. Picked on 10-28, confirmed on 10-30. | `DECIDED` | 10-30 | [#9400] | | ||
| | D81 | **An undo or redo press changes what a fresh read of the document shows, or takes every step there is.** After a step whose whole effect no reader shows, the same press takes the next step. A press that a listener starts while an undo runs takes one step, as before. | Yjs skips only a step that changes no item. A step that restores or removes only format items no reader shows (a format change on text a collaborator then deleted: the format's items stay around the deleted text, and undoing the format removes only them) changed items but nothing anyone sees, so the press did nothing visible and the next press undid what the first was meant to. What is visible is decided by a digest of what any reader can show (the live content in order with its format values, the attributes and the nested types), compared before and after each step. It changes how many steps a press takes wherever a step showed nothing, which is a behaviour change to undo: for a maintainer's call. The alternative is Yjs's rule as it is, a press that can show nothing. Keeping run ends in the collapse (#9617, parked) made such presses more common. | `OPEN` | 10-30 | [#9617] | |
There was a problem hiding this comment.
Nit: this OPEN row sits in the Storage model table among DECIDED rows; the register keeps OPEN rows under ## Open (Question / Where it stands) until they are decided.
zurfyx
added a commit
that referenced
this pull request
Oct 31, 2026
… exact ## Description Review of the test-only change found two expected failures that a real behaviour change could slip past: - The concurrent property test was one `test.fails` over 60 seeds. Since seed 53 fails on main, a change that broke a seed passing on main kept the whole test failing, so nothing noticed. Now one regular test runs every seed and checks everything (no errors, no more writes, the same characters, peers and a fresh reader agreeing); it skips the own-text comparison only on the seeds where main lets a comment spread (53). A `test.fails` per such seed checks only that comparison (#9609). - KeepEndsSessions 3645 shows main's drop of c9 only after the session's last undo. It also flips with #9630 (every undo press changes what the document shows), with no collapse change. Its name and comment now say so. ## Test plan ### Before `CollapseKeepsEndsProperty.test.ts` on main: 2 passed, 2 expected fail; one aggregate `test.fails` per id order. ### After `pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/CollapseKeepsEndsProperty.test.ts packages/lexical-y/src/__tests__/unit/regressions/KeepEndsSessions.test.ts` on main (1853707): ``` ✓ taking turns: every character shows as with no collapse (ids 1,2) ✓ typing ‖ deletes: the document's own text shows as with no collapse (ids 1,2) ✓ taking turns: every character shows as with no collapse (ids 2,1) ✓ typing ‖ deletes: the document's own text shows as with no collapse (ids 2,1) Tests 6 passed | 4 expected fail (10) ``` Seed 53 is the only one of 60 that fails on main, in both id orders, and only on the own text (its other checks pass). `npx tsc -p tsconfig.test.json --noEmit` and eslint pass.
Member
Author
|
Closing by decision: an undo press that shows nothing (a step whose only effect is on format items no reader shows) is an accepted limit, shared with every Yjs editor using Y.UndoManager. This change works, but it adds a second definition of "what a reader can show" that must track every stored shape the binding learns, costs a whole-document digest per press (23 to 50 ms at 140k to 300k characters), and changes how many steps one press takes for apps that listen to the undo stack. The benefit, one extra press in rare cases, does not justify that. D81 is recorded as an accepted limit; this draft stays for reference. |
zurfyx
added a commit
that referenced
this pull request
Nov 1, 2026
…ecisions on undo presses, list numbering, overlapping formats, rewritten words and provider ordering ## Description Records four maintainer decisions of 10/31 in DECISIONS.md and y.md: - **D81, accepted limit:** an undo press can show nothing when its step only restores or removes format items no reader shows; the next press undoes what the first was meant to. Every editor that uses Yjs's `UndoManager` behaves the same. Skipping such steps (#9630, closed) needed a per-press digest of what readers show. y.md: a sentence in Limits, Undo. - **D90, accepted limit (#9632):** after four refused follow-ups in a row that the binding reads again, the author can show a derived list number from before them until its next edit. The stop on the fourth re-read is the backstop that ends reply loops (#9634, closed, showed the three requirements conflict). y.md: a Limits entry. The #9642 pins (`RefusedFollowUpRefresh`, `RefusedFollowUpFreshness`) now name D90 and the limit; they stay `test.fails`. - **D91, accepted limit (#9626):** overlapping formats of one key from two writers can cancel, because a Yjs text keeps one format marker per key at a place (yjs/yjs#382); since #9601 this also reaches a link's edge and first word. #9601 is kept. y.md: Other races says why. - **D93, accepted limit (#9610):** a comment on a word a collaborator rewrites at the same moment can land on the new word (`setTextContent`: one client id order; a block rewrite: one or both orders), the same race as text typed at the start of a range a collaborator formats. y.md: a sentence in Other races. New `regressions/CommentOnRewrittenWord.test.ts` pins the 9 cells where it happens as `test.fails` (12 block-rewrite cells and 2 `setTextContent` cells); #9610 stays open. - **D92, decided (#9645):** providers deliver updates in causal order; a mesh or several relays need causal buffering. y.md: Servers states the requirement. #9645 stays open. The agent-edit guidance is unchanged. ## Test plan ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpRefresh.test.ts packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpFreshness.test.ts Tests 27 passed | 30 expected fail (57) ``` ### After ``` $ pnpm exec prettier --check packages/lexical-y/DECISIONS.md packages/lexical-website/docs/collaboration/y.md packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpRefresh.test.ts packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpFreshness.test.ts All matched files use Prettier code style! $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpRefresh.test.ts packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpFreshness.test.ts Tests 27 passed | 30 expected fail (57) $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/CommentOnRewrittenWord.test.ts Tests 5 passed | 9 expected fail (14) $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/CommentOnRewrittenWord.test.ts Tests 5 passed | 9 expected fail (14) ``` The two RefusedFollowUp files change only in comments; the same cells pass and fail as on main. With every `test.fails` in CommentOnRewrittenWord turned into `test`, exactly the 9 pinned cells fail (gc off and on).
zurfyx
added a commit
that referenced
this pull request
Nov 1, 2026
…ecisions on undo presses, list numbering, overlapping formats, rewritten words and provider ordering ## Description Records four maintainer decisions of 10/31 in DECISIONS.md and y.md: - **D81, accepted limit:** an undo press can show nothing when its step only restores or removes format items no reader shows; the next press undoes what the first was meant to. Every editor that uses Yjs's `UndoManager` behaves the same. Skipping such steps (#9630, closed) needed a per-press digest of what readers show. y.md: a sentence in Limits, Undo. - **D90, accepted limit (#9632):** after four refused follow-ups in a row that the binding reads again, the author can show a derived list number from before them until its next edit. The stop on the fourth re-read is the backstop that ends reply loops (#9634, closed, showed the three requirements conflict). y.md: a Limits entry. The #9642 pins (`RefusedFollowUpRefresh`, `RefusedFollowUpFreshness`) now name D90 and the limit; they stay `test.fails`. - **D91, accepted limit (#9626):** overlapping formats of one key from two writers can cancel, because a Yjs text keeps one format marker per key at a place (yjs/yjs#382); since #9601 this also reaches a link's edge and first word. #9601 is kept. y.md: Other races says why. - **D93, accepted limit (#9610):** a comment on a word a collaborator rewrites at the same moment can land on the new word (`setTextContent`: one client id order; a block rewrite: one or both orders), the same race as text typed at the start of a range a collaborator formats. y.md: a sentence in Other races. New `regressions/CommentOnRewrittenWord.test.ts` pins the 9 cells where it happens as `test.fails` (12 block-rewrite cells and 2 `setTextContent` cells); #9610 stays open. - **D92, decided (#9645):** providers deliver updates in causal order; a mesh or several relays need causal buffering. y.md: Servers states the requirement. #9645 stays open. Also, from the review of #9684: D74 and D64 link #9681 and #9682, and y.md's Open issues lists #9681 and #9683. The agent-edit guidance is unchanged. ## Test plan ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpRefresh.test.ts packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpFreshness.test.ts Tests 27 passed | 30 expected fail (57) ``` ### After ``` $ pnpm exec prettier --check packages/lexical-y/DECISIONS.md packages/lexical-website/docs/collaboration/y.md packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpRefresh.test.ts packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpFreshness.test.ts All matched files use Prettier code style! $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpRefresh.test.ts packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpFreshness.test.ts Tests 27 passed | 30 expected fail (57) $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/CommentOnRewrittenWord.test.ts Tests 5 passed | 9 expected fail (14) $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/CommentOnRewrittenWord.test.ts Tests 5 passed | 9 expected fail (14) ``` The two RefusedFollowUp files change only in comments; the same cells pass and fail as on main. With every `test.fails` in CommentOnRewrittenWord turned into `test`, exactly the 9 pinned cells fail (gc off and on).
zurfyx
added a commit
that referenced
this pull request
Nov 1, 2026
…ecisions on undo presses, list numbering, overlapping formats, rewritten words and provider ordering ## Description Records five maintainer decisions of 10/31 in DECISIONS.md and y.md: - **D81, accepted limit:** an undo press can show nothing when its step only restores or removes format items no reader shows, and several such steps take several presses, as with Yjs's `UndoManager` in any editor that uses it as it is. Skipping such steps (#9630, closed) needed a per-press digest of what readers show. y.md: a sentence in Limits, Undo. - **D90, accepted limit (#9632):** after four refused follow-ups in a row that the binding reads again, the author can show a derived list number from before them until that list is refreshed (an edit elsewhere does not do it); its fresh read, other clients and a reload are right. The stop on the fourth re-read is the backstop that ends reply loops (#9634, closed, showed the three requirements conflict). y.md: a Limits entry. The #9642 pins (`RefusedFollowUpRefresh`, `RefusedFollowUpFreshness`) now name D90 and the limit; they stay `test.fails`. - **D91, accepted limit (#9626):** overlapping formats of one key from two writers can cancel, because a Yjs text gives each place one value per key (yjs/yjs#382); since #9601 this also reaches a link's edge and first word. #9601 is kept. y.md: Other races says why. - **D93, accepted limit (#9610):** a comment on a word a collaborator rewrites at the same moment can land on the new word (`setTextContent`: one client id order; a block rewrite: one or both orders), the same race as text typed at the start of a range a collaborator formats. y.md: a sentence in Other races. New `regressions/CommentOnRewrittenWord.test.ts` pins the 9 cells where it happens as `test.fails` (12 block-rewrite cells and 2 `setTextContent` cells); #9610 stays open. D77 (the open question) says D93 replaced it. - **D92, decided (#9645):** providers deliver updates in causal order; a single relay that forwards each client's updates in the order it received them does; a mesh or several relays need causal buffering that tracks dependencies, deletions included. y.md: Servers states the requirement. #9645 stays open. Also, from the review of #9684: D74 and D64 link #9681 and #9682, and y.md's Open issues lists #9681 and #9683. The agent-edit guidance is unchanged. ## Test plan ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpRefresh.test.ts packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpFreshness.test.ts Tests 27 passed | 30 expected fail (57) ``` ### After ``` $ pnpm exec prettier --check packages/lexical-y/DECISIONS.md packages/lexical-website/docs/collaboration/y.md packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpRefresh.test.ts packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpFreshness.test.ts All matched files use Prettier code style! $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpRefresh.test.ts packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpFreshness.test.ts Tests 27 passed | 30 expected fail (57) $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/CommentOnRewrittenWord.test.ts Tests 5 passed | 9 expected fail (14) $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/CommentOnRewrittenWord.test.ts Tests 5 passed | 9 expected fail (14) ``` The two RefusedFollowUp files change only in comments; the same cells pass and fail as on main. With every `test.fails` in CommentOnRewrittenWord turned into `test`, exactly the 9 pinned cells fail (gc off and on).
zurfyx
added a commit
that referenced
this pull request
Nov 1, 2026
…ecisions on undo presses, list numbering, overlapping formats, rewritten words and provider ordering ## Description Records five maintainer decisions of 10/31 in DECISIONS.md and y.md: - **D81, accepted limit:** an undo press can show nothing when its step only restores or removes format items no reader shows, and several such steps take several presses, as with Yjs's `UndoManager` in any editor that uses it as it is. Skipping such steps (#9630, closed) needed a per-press digest of what readers show. y.md: a sentence in Limits, Undo. - **D90, accepted limit (#9632):** after four refused follow-ups in a row that the binding reads again, the author can show a derived list number from before them until that list is refreshed (adding an item does; an edit elsewhere or retyping an item does not); its fresh read, other clients and a reload are right. The stop on the fourth re-read is the backstop that ends reply loops (#9634, closed, showed the three requirements conflict). y.md: a Limits entry. The #9642 pins (`RefusedFollowUpRefresh`, `RefusedFollowUpFreshness`) now name D90 and the limit; they stay `test.fails`. - **D91, accepted limit (#9626):** overlapping formats of one key from two writers can cancel, because a Yjs text gives each place one value per key (yjs/yjs#382); since #9601 this also reaches a link's edge and first word. #9601 is kept. y.md: Other races says why. - **D93, accepted limit (#9610):** a comment on a word a collaborator rewrites at the same moment can land on the new word (`setTextContent`: one client id order; a block rewrite: one or both orders), the same race as text typed at the start of a range a collaborator formats. y.md: a sentence in Other races. New `regressions/CommentOnRewrittenWord.test.ts` pins the 9 cells where it happens as `test.fails` (12 block-rewrite cells and 2 `setTextContent` cells); #9610 stays open. D77 (the open question) says D93 replaced it. - **D92, decided (#9645):** providers deliver updates in causal order; a single relay that forwards every update to each client in the one order it received them does; a mesh or several relays need causal buffering that tracks dependencies, deletions included. y.md: Servers states the requirement. #9645 stays open. Also, from the review of #9684: D74 and D64 link #9681 and #9682, and y.md's Open issues lists #9681 and #9683. The agent-edit guidance is unchanged. ## Test plan ### Before ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpRefresh.test.ts packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpFreshness.test.ts Tests 27 passed | 30 expected fail (57) ``` ### After ``` $ pnpm exec prettier --check packages/lexical-y/DECISIONS.md packages/lexical-website/docs/collaboration/y.md packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpRefresh.test.ts packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpFreshness.test.ts All matched files use Prettier code style! $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpRefresh.test.ts packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpFreshness.test.ts Tests 27 passed | 30 expected fail (57) $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/CommentOnRewrittenWord.test.ts Tests 5 passed | 9 expected fail (14) $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/CommentOnRewrittenWord.test.ts Tests 5 passed | 9 expected fail (14) ``` The two RefusedFollowUp files change only in comments; the same cells pass and fail as on main. With every `test.fails` in CommentOnRewrittenWord turned into `test`, exactly the 9 pinned cells fail (gc off and on).
zurfyx
added a commit
that referenced
this pull request
Nov 1, 2026
…dit on the current state, then apply it there ## Description y.md's "Edits made by code" told integrators to apply an agent's edit to the state it was computed from and merge the result into the live document. That always converges, but for an edit addressed by text or by block id it converges to text nobody meant whenever the document changed under the edit. Measured on `@lexical/y` with `seedYState`, `editYState` and `readYState` (15 cases: a person's or a second agent's edit lands between the agent's read and its edit): | Case | Merging the edit made on the read | Check, then apply to the current state | | --- | --- | --- | | a collaborator replaced the same words | `The slowswift red fox` | refused (words changed) | | a collaborator made the same change | `swiftswift` | refused | | two agents, different words | `at risblocked` | refused | | two agents, the same edit | `4.2.2%` | refused | | a collaborator typed elsewhere in the block | right | right | | the words now also appear in another block | right | right (checked in the read's block) | | the line was moved | new text where the line was; the moved line keeps the old text | right (block found by id) | | words cut and pasted elsewhere | `The swift redfox jumps.` | refused | | a block rewrite as a new node while a collaborator types in it | their text moved to the start | refused | | a block delete while a collaborator types in it | their sentence glued to the next line | refused | | a count outside the edited words | wrong | wrong (only the agent can catch it) | Merging was wrong in 10 of 15; the check was wrong in 1 and refused 2 edits that would have merged well. This is how text-matching edit tools behave (Anthropic's `str_replace` and OpenAI's `apply_patch` match the current text and fail on a miss; Claude Code refuses a write to a file changed since its read; the Google Docs API's `requiredRevisionId` and TipTap's server toolkit refuse a stale write so the caller re-reads). Now: - y.md, "Edits made by code": **Check what an edit names, then apply it to the current state**: a replacement checks its own words in the block where it read them, found there by id (still there, exactly once); a rewrite or a delete checks the whole block (where it is, and its type, text, formats, style, NodeState and children, compared serialized, not only its text); an insert checks its anchor block. Every edit of a batch is checked before any is applied, then again, in order, as it is applied on the state the earlier ones leave: an edit an earlier one changes (its words changed or now appear twice, its block deleted or rewritten, a block named twice) refuses the whole batch, a conservative rule. On a refusal, apply none, and return every failure (the reason, the block's content) and a new read. On a pass, apply to the current state (blocks found by id there) and store with a compare-and-set; on a lost race, drop the state and the update computed on the older state and run the whole check and edit again on the newer one (bounded), or refuse back to the agent with a read of the state that won the last race: an update computed on an older state is never stored or sent. **Merge from the read only where there is nothing to check** (an append, a suggestion a person accepts, an edit by position): the fork-and-merge recipe stays for those cases. - y-server.md, "Edits by agents and tools" and "Edits by block id": the sketches check on the latest stored state and run `editYState` there, through `applyChecked` (the check; the edit, which checks each part again as it applies it and can refuse; the compare-and-set; up to 3 attempts, then a fresh load), instead of editing `seen` and merging. The word sketch takes a batch of replacements. `$findWords` finds a replacement's words by the block's id and requires them exactly once in that block's text, in one plain text node; `$blockAt` compares a target as a whole (its path and its serialized subtree); an answer by id is checked op by op and applied whole or not at all. "One edit at a time" says a checked edit starts over on a lost race instead of merging. `ServerDocsAgentEdits.test.ts` runs the sketches as written (they live in `__tests__/utils/agentEditSketch.ts`, and the first test checks that the page holds the same code), including targets changed since the read, a batch with one failing op, a batch whose first edit changes what the second names, and a store that loses the race. - block-ids.md, "Handles and collaboration": the handle is resolved and the whole block checked against what the model read on the stored state, and the edit is applied there by id with `applyChecked`; `BlockHandles.test.ts` follows it (a block the person typed in now goes back as `changed`). - DECISIONS.md: D94 (decided); D71 says D94 replaced which state the edit runs on. Also, from the reviews of #9685: - y.md, Servers: the per-client order paragraph now opens "Causal order includes each client's own order", and the json-joy comparison is dropped (y.md and D92). - D93 and y.md name the orders: one order when the comment ends at the rewritten word, both when it continues past it. - `regressions/UndoInvisibleSteps.test.ts` (from #9630) pins D81: the 3 cells that assert a press also taking the next step are `test.fails`. They fail on main with gc off and on. ## Test plan ### Before The new cells against weaker sketches: no uniqueness check, a target compared by its path and text only, and an update computed before a lost race merged into the newer state (the docs' previous recipe): ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/ServerDocsAgentEdits.test.ts × the sketches are the same in the docs and the tests 13ms × a replacement of words refuses words that appear twice in the block or across nodes, and finds the block read by id 10ms × step 3 refuses the whole answer when a target is gone, moved, replaced, typed in, formatted or styled since the read 42ms × a lost race starts over on the newer state: applied when the check still holds there, refused when not, never the older edit 20ms ⎯⎯⎯⎯⎯⎯⎯ Failed Tests 4 ⎯⎯⎯⎯⎯⎯⎯ AssertionError: expected '// Your storage of one document: its …' to be '// Your storage of one document: its …' // Object.is equality - Expected + Received AssertionError: expected undefined to deeply equal [ { …(3) } ] - Expected: + Received: AssertionError: formatted: expected [] to deeply equal [ [ 1, 'the block changed' ] ] - Expected + Received ``` An independent review of an earlier head of this branch composed the word check per edit into a batch: each check passed on the stored state, the first edit invalidated the second, and the batch was stored half done; after 3 lost races the read returned was the one that had lost: ``` FAIL > dependent word batch refuses as a whole when an earlier op invalidates a later target - "refused": true, "sent": 0, "stores": 0, "text": "red fox\n\nspacer" + "refused": false, "sent": 1, "stores": 1, "text": "blue fox\n\nspacer" FAIL > dependency that creates another occurrence cannot silently skip the second op - "text": "red fox and hen\n\nspacer" + "text": "red fox and red\n\nspacer" FAIL > retry exhaustion supplies a fresh read after the final known lost race AssertionError: expected 'red fox\n\nxxspacer' to be 'red fox\n\nxxxspacer' ``` Those cases are cells of `ServerDocsAgentEdits.test.ts` now. `UndoInvisibleSteps.test.ts` unpinned, on main: ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/UndoInvisibleSteps.test.ts × undoing a format on text a collaborator deleted takes the step before it too (1,2) × undoing a format on text a collaborator deleted takes the step before it too (2,1) × undoing a comment removal on deleted text takes the format change before it too AssertionError: expected 'one three X{0}[]' to be 'one three{0}[]' Tests 3 failed (3) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/ServerDocsAgentEdits.test.ts packages/lexical-y/src/__tests__/unit/BlockHandles.test.ts packages/lexical-y/src/__tests__/unit/regressions/UndoInvisibleSteps.test.ts packages/lexical-html/src/__tests__/unit/BlockIdDocs.test.ts Tests 28 passed | 3 expected fail (31) $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit (same files) Tests 28 passed | 3 expected fail (31) $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit packages/lexical-html/src/__tests__/unit/BlockIdDocs.test.ts Test Files 225 passed | 2 skipped (227) Tests 18549 passed | 306 expected fail | 130 skipped (18985) $ pnpm run ci-check Found 0 errors ``` With gc off, the same lexical-y run had one failure, `DisposeMemory.test.ts` ("40 editors bound, edited and disposed in a synchronous loop hold no document"), under 24 parallel workers; it passes 5 of 5 alone, and this change touches no code it runs (docs and tests only).
zurfyx
added a commit
that referenced
this pull request
Nov 1, 2026
…dit on the current state, then apply it there ## Description y.md's "Edits made by code" told integrators to apply an agent's edit to the state it was computed from and merge the result into the live document. That always converges, but for an edit addressed by text or by block id it converges to text nobody meant whenever the document changed under the edit. Measured on `@lexical/y` with `seedYState`, `editYState` and `readYState` (15 cases: a person's or a second agent's edit lands between the agent's read and its edit): | Case | Merging the edit made on the read | Check, then apply to the current state | | --- | --- | --- | | a collaborator replaced the same words | `The slowswift red fox` | refused (words changed) | | a collaborator made the same change | `swiftswift` | refused | | two agents, different words | `at risblocked` | refused | | two agents, the same edit | `4.2.2%` | refused | | a collaborator typed elsewhere in the block | right | right | | the words now also appear in another block | right | right (checked in the read's block) | | the line was moved | new text where the line was; the moved line keeps the old text | right (block found by id) | | words cut and pasted elsewhere | `The swift redfox jumps.` | refused | | a block rewrite as a new node while a collaborator types in it | their text moved to the start | refused | | a block delete while a collaborator types in it | their sentence glued to the next line | refused | | a count outside the edited words | wrong | wrong (only the agent can catch it) | Merging was wrong in 10 of 15; the check was wrong in 1 and refused 2 edits that would have merged well. This is how text-matching edit tools behave (Anthropic's `str_replace` and OpenAI's `apply_patch` match the current text and fail on a miss; Claude Code refuses a write to a file changed since its read; the Google Docs API's `requiredRevisionId` and TipTap's server toolkit refuse a stale write so the caller re-reads). Now: - y.md, "Edits made by code": **Check what an edit names, then apply it to the current state**: a replacement checks its own words in the block where it read them, found there by id (still there, exactly once); a rewrite or a delete checks the whole block (where it is, and its type, text, formats, style, NodeState and children, compared serialized, not only its text); an insert checks its anchor block. Every edit of a batch is checked before any is applied, then again, in order, as it is applied on the state the earlier ones leave: an edit an earlier one changes (its words changed or now appear twice, its block deleted or rewritten, a block named twice) refuses the whole batch, a conservative rule. On a refusal, apply none, and return every failure (which edit, the reason, and the block's content in the latest stored state, not in the discarded edit; edits after an in-order failure are still checked) and a new read. On a pass, apply to the current state (blocks found by id there) and store with a compare-and-set; on a lost race, drop the state and the update computed on the older state and run the whole check and edit again on the newer one (bounded), or refuse back to the agent with a read of the state that won the last race: an update computed on an older state is never stored or sent. **Merge from the read only where there is nothing to check** (an append, a suggestion a person accepts, an edit by position): the fork-and-merge recipe stays for those cases. - y-server.md, "Edits by agents and tools" and "Edits by block id": the sketches check on the latest stored state and run `editYState` there, through `applyChecked` (the check; the edit, which checks each part again as it applies it and can refuse; the compare-and-set; up to 3 attempts, then a fresh load), instead of editing `seen` and merging. The word sketch takes a batch of replacements. `$findWords` finds a replacement's words by the block's id and requires them exactly once in that block's text, in one plain text node; `$blockAt` compares a target as a whole (its path and its serialized subtree); an answer by id is checked op by op and applied whole or not at all. "One edit at a time" says a checked edit starts over on a lost race instead of merging. `ServerDocsAgentEdits.test.ts` runs the sketches as written (they live in `__tests__/utils/agentEditSketch.ts`, and the first test checks that the page holds the same code), including targets changed since the read, a batch with one failing op, a batch whose first edit changes what the second (and third) names, refusals that report the stored content, and a store that loses the race. - block-ids.md, "Handles and collaboration": the handle is resolved and the whole block checked against what the model read on the stored state, and the edit is applied there by id with `applyChecked`; `BlockHandles.test.ts` follows it (a block the person typed in now goes back as `changed`). - DECISIONS.md: D94 (decided); D71 says D94 replaced which state the edit runs on. Also, from the reviews of #9685: - y.md, Servers: the per-client order paragraph now opens "Causal order includes each client's own order", and the json-joy comparison is dropped (y.md and D92). - D93 and y.md name the orders: one order when the comment ends at the rewritten word, both when it continues past it. - `regressions/UndoInvisibleSteps.test.ts` (from #9630) pins D81: the 3 cells that assert a press also taking the next step are `test.fails`. They fail on main with gc off and on. ## Test plan ### Before The new cells against weaker sketches: no uniqueness check, a target compared by its path and text only, and an update computed before a lost race merged into the newer state (the docs' previous recipe): ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/ServerDocsAgentEdits.test.ts × the sketches are the same in the docs and the tests 13ms × a replacement of words refuses words that appear twice in the block or across nodes, and finds the block read by id 10ms × step 3 refuses the whole answer when a target is gone, moved, replaced, typed in, formatted or styled since the read 42ms × a lost race starts over on the newer state: applied when the check still holds there, refused when not, never the older edit 20ms ⎯⎯⎯⎯⎯⎯⎯ Failed Tests 4 ⎯⎯⎯⎯⎯⎯⎯ AssertionError: expected '// Your storage of one document: its …' to be '// Your storage of one document: its …' // Object.is equality - Expected + Received AssertionError: expected undefined to deeply equal [ { …(3) } ] - Expected: + Received: AssertionError: formatted: expected [] to deeply equal [ [ 1, 'the block changed' ] ] - Expected + Received ``` An independent review of an earlier head of this branch composed the word check per edit into a batch: each check passed on the stored state, the first edit invalidated the second, and the batch was stored half done; after 3 lost races the read returned was the one that had lost: ``` FAIL > dependent word batch refuses as a whole when an earlier op invalidates a later target - "refused": true, "sent": 0, "stores": 0, "text": "red fox\n\nspacer" + "refused": false, "sent": 1, "stores": 1, "text": "blue fox\n\nspacer" FAIL > dependency that creates another occurrence cannot silently skip the second op - "text": "red fox and hen\n\nspacer" + "text": "red fox and red\n\nspacer" FAIL > retry exhaustion supplies a fresh read after the final known lost race AssertionError: expected 'red fox\n\nxxspacer' to be 'red fox\n\nxxxspacer' ``` A second review found the refusal of such a batch reporting the block's text from the discarded edit (`blue fox and hen` where storage held `red fox and hen`), and only the first of two invalidated edits. Those cases are cells of `ServerDocsAgentEdits.test.ts` now. `UndoInvisibleSteps.test.ts` unpinned, on main: ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/UndoInvisibleSteps.test.ts × undoing a format on text a collaborator deleted takes the step before it too (1,2) × undoing a format on text a collaborator deleted takes the step before it too (2,1) × undoing a comment removal on deleted text takes the format change before it too AssertionError: expected 'one three X{0}[]' to be 'one three{0}[]' Tests 3 failed (3) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/ServerDocsAgentEdits.test.ts packages/lexical-y/src/__tests__/unit/BlockHandles.test.ts packages/lexical-y/src/__tests__/unit/regressions/UndoInvisibleSteps.test.ts packages/lexical-html/src/__tests__/unit/BlockIdDocs.test.ts Tests 28 passed | 3 expected fail (31) $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit (same files) Tests 28 passed | 3 expected fail (31) $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit packages/lexical-html/src/__tests__/unit/BlockIdDocs.test.ts Test Files 225 passed | 2 skipped (227) Tests 18549 passed | 306 expected fail | 130 skipped (18985) $ pnpm run ci-check Found 0 errors ``` With gc off, the same lexical-y run had one failure, `DisposeMemory.test.ts` ("40 editors bound, edited and disposed in a synchronous loop hold no document"), under 24 parallel workers; it passes 5 of 5 alone, and this change touches no code it runs (docs and tests only).
zurfyx
added a commit
that referenced
this pull request
Nov 1, 2026
…dit on the current state, then apply it there ## Description y.md's "Edits made by code" told integrators to apply an agent's edit to the state it was computed from and merge the result into the live document. That always converges, but for an edit addressed by text or by block id it converges to text nobody meant whenever the document changed under the edit. Measured on `@lexical/y` with `seedYState`, `editYState` and `readYState` (15 cases: a person's or a second agent's edit lands between the agent's read and its edit): | Case | Merging the edit made on the read | Check, then apply to the current state | | --- | --- | --- | | a collaborator replaced the same words | `The slowswift red fox` | refused (words changed) | | a collaborator made the same change | `swiftswift` | refused | | two agents, different words | `at risblocked` | refused | | two agents, the same edit | `4.2.2%` | refused | | a collaborator typed elsewhere in the block | right | right | | the words now also appear in another block | right | right (checked in the read's block) | | the line was moved | new text where the line was; the moved line keeps the old text | right (block found by id) | | words cut and pasted elsewhere | `The swift redfox jumps.` | refused | | a block rewrite as a new node while a collaborator types in it | their text moved to the start | refused | | a block delete while a collaborator types in it | their sentence glued to the next line | refused | | a count outside the edited words | wrong | wrong (only the agent can catch it) | Merging was wrong in 10 of 15; the check was wrong in 1 and refused 2 edits that would have merged well. This is how text-matching edit tools behave (Anthropic's `str_replace` and OpenAI's `apply_patch` match the current text and fail on a miss; Claude Code refuses a write to a file changed since its read; the Google Docs API's `requiredRevisionId` and TipTap's server toolkit refuse a stale write so the caller re-reads). Now: - y.md, "Edits made by code": **Check what an edit names, then apply it to the current state**: a replacement checks its own words in the block where it read them, found there by id (still there, exactly once); a rewrite or a delete checks the whole block (where it is, and its type, text, formats, style, NodeState and children, compared serialized, not only its text, as the checking schema declares them: run the check on a schema that still declares every field stored versions may hold, or a change to a dropped field goes unnoticed); an insert checks its anchor block. Every edit of a batch is checked before any is applied, then again, in order, as it is applied on the state the earlier ones leave: an edit an earlier one changes (its words changed or now appear twice, its block deleted or rewritten, a block named twice) refuses the whole batch, a conservative rule. On a refusal, apply none, and return every failure (which edit, the reason, and the block's content in the latest stored state, not in the discarded edit; edits after an in-order failure are still checked) and a new read. On a pass, apply to the current state (blocks found by id there) and store with a compare-and-set; on a lost race, drop the state and the update computed on the older state and run the whole check and edit again on the newer one (bounded), or refuse back to the agent with a read of the state that won the last race: an update computed on an older state is never stored or sent. **Merge from the read only where there is nothing to check** (an append, a suggestion a person accepts, an edit by position): the fork-and-merge recipe stays for those cases. - y-server.md, "Edits by agents and tools" and "Edits by block id": the sketches check on the latest stored state and run `editYState` there, through `applyChecked` (the check; the edit, which checks each part again as it applies it and can refuse; the compare-and-set; up to 3 attempts, then a fresh load), instead of editing `seen` and merging. The word sketch takes a batch of replacements. `$findWords` finds a replacement's words by the block's id and requires them exactly once in that block's text, in one plain text node; `$blockAt` compares a target as a whole (its path and its serialized subtree); an answer by id is checked op by op and applied whole or not at all. "One edit at a time" says a checked edit starts over on a lost race instead of merging. `ServerDocsAgentEdits.test.ts` runs the sketches as written (they live in `__tests__/utils/agentEditSketch.ts`, and the first test checks that the page holds the same code), including targets changed since the read, a batch with one failing op, a batch whose first edit changes what the second (and third) names, refusals that report the stored content, and a store that loses the race. - block-ids.md, "Handles and collaboration": the handle is resolved and the whole block checked against what the model read on the stored state, and the edit is applied there by id with `applyChecked`; `BlockHandles.test.ts` follows it (a block the person typed in now goes back as `changed`). - DECISIONS.md: D94 (decided); D71 says D94 replaced which state the edit runs on. Also, from the reviews of #9685: - y.md, Servers: the per-client order paragraph now opens "Causal order includes each client's own order", and the json-joy comparison is dropped (y.md and D92). - D93 and y.md name the orders: one order when the comment ends at the rewritten word, both when it continues past it. - `regressions/UndoInvisibleSteps.test.ts` (from #9630) pins D81: the 3 cells that assert a press also taking the next step are `test.fails`. They fail on main with gc off and on. ## Test plan ### Before The new cells against weaker sketches: no uniqueness check, a target compared by its path and text only, and an update computed before a lost race merged into the newer state (the docs' previous recipe): ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/ServerDocsAgentEdits.test.ts × the sketches are the same in the docs and the tests 13ms × a replacement of words refuses words that appear twice in the block or across nodes, and finds the block read by id 10ms × step 3 refuses the whole answer when a target is gone, moved, replaced, typed in, formatted or styled since the read 42ms × a lost race starts over on the newer state: applied when the check still holds there, refused when not, never the older edit 20ms ⎯⎯⎯⎯⎯⎯⎯ Failed Tests 4 ⎯⎯⎯⎯⎯⎯⎯ AssertionError: expected '// Your storage of one document: its …' to be '// Your storage of one document: its …' // Object.is equality - Expected + Received AssertionError: expected undefined to deeply equal [ { …(3) } ] - Expected: + Received: AssertionError: formatted: expected [] to deeply equal [ [ 1, 'the block changed' ] ] - Expected + Received ``` An independent review of an earlier head of this branch composed the word check per edit into a batch: each check passed on the stored state, the first edit invalidated the second, and the batch was stored half done; after 3 lost races the read returned was the one that had lost: ``` FAIL > dependent word batch refuses as a whole when an earlier op invalidates a later target - "refused": true, "sent": 0, "stores": 0, "text": "red fox\n\nspacer" + "refused": false, "sent": 1, "stores": 1, "text": "blue fox\n\nspacer" FAIL > dependency that creates another occurrence cannot silently skip the second op - "text": "red fox and hen\n\nspacer" + "text": "red fox and red\n\nspacer" FAIL > retry exhaustion supplies a fresh read after the final known lost race AssertionError: expected 'red fox\n\nxxspacer' to be 'red fox\n\nxxxspacer' ``` A second review found the refusal of such a batch reporting the block's text from the discarded edit (`blue fox and hen` where storage held `red fox and hen`), and only the first of two invalidated edits. Those cases are cells of `ServerDocsAgentEdits.test.ts` now, as is a third review's case, pinned as the documented scope: a field the server's schema dropped is not compared, so a rewrite passes over a collaborator's change to it. `UndoInvisibleSteps.test.ts` unpinned, on main: ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/UndoInvisibleSteps.test.ts × undoing a format on text a collaborator deleted takes the step before it too (1,2) × undoing a format on text a collaborator deleted takes the step before it too (2,1) × undoing a comment removal on deleted text takes the format change before it too AssertionError: expected 'one three X{0}[]' to be 'one three{0}[]' Tests 3 failed (3) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/ServerDocsAgentEdits.test.ts packages/lexical-y/src/__tests__/unit/BlockHandles.test.ts packages/lexical-y/src/__tests__/unit/regressions/UndoInvisibleSteps.test.ts packages/lexical-html/src/__tests__/unit/BlockIdDocs.test.ts Tests 29 passed | 3 expected fail (32) $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit (same files) Tests 29 passed | 3 expected fail (32) $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit packages/lexical-html/src/__tests__/unit/BlockIdDocs.test.ts Test Files 225 passed | 2 skipped (227) Tests 18549 passed | 306 expected fail | 130 skipped (18985) $ pnpm run ci-check Found 0 errors ``` With gc off, the same lexical-y run had one failure, `DisposeMemory.test.ts` ("40 editors bound, edited and disposed in a synchronous loop hold no document"), under 24 parallel workers; it passes 5 of 5 alone, and this change touches no code it runs (docs and tests only).
zurfyx
added a commit
that referenced
this pull request
Nov 1, 2026
…dit on the current state, then apply it there ## Description y.md's "Edits made by code" told integrators to apply an agent's edit to the state it was computed from and merge the result into the live document. That always converges, but for an edit addressed by text or by block id it converges to text nobody meant whenever the document changed under the edit. Measured on `@lexical/y` with `seedYState`, `editYState` and `readYState` (15 cases: a person's or a second agent's edit lands between the agent's read and its edit): | Case | Merging the edit made on the read | Check, then apply to the current state | | --- | --- | --- | | a collaborator replaced the same words | `The slowswift red fox` | refused (words changed) | | a collaborator made the same change | `swiftswift` | refused | | two agents, different words | `at risblocked` | refused | | two agents, the same edit | `4.2.2%` | refused | | a collaborator typed elsewhere in the block | right | right | | the words now also appear in another block | right | right (checked in the read's block) | | the line was moved | new text where the line was; the moved line keeps the old text | right (block found by id) | | words cut and pasted elsewhere | `The swift redfox jumps.` | refused | | a block rewrite as a new node while a collaborator types in it | their text moved to the start | refused | | a block delete while a collaborator types in it | their sentence glued to the next line | refused | | a count outside the edited words | wrong | wrong (only the agent can catch it) | Merging was wrong in 10 of 15; the check was wrong in 1 and refused 2 edits that would have merged well. This is how text-matching edit tools behave (Anthropic's `str_replace` and OpenAI's `apply_patch` match the current text and fail on a miss; Claude Code refuses a write to a file changed since its read; the Google Docs API's `requiredRevisionId` and TipTap's server toolkit refuse a stale write so the caller re-reads). Now: - y.md, "Edits made by code": **Check what an edit names, then apply it to the current state**: a replacement checks its own words in the block where it read them, found there by id (still there, exactly once); a rewrite or a delete checks the whole block (where it is, and its type, text, formats, style, NodeState and children, compared serialized, not only its text, as the checking schema declares them: run the check on a schema that still declares every field stored versions may hold, or a change to a dropped field goes unnoticed); a format or a comment on words the edit names checks those words like a replacement and is applied where they are found on the current state, never by offsets from the read; an insert checks its anchor block. Every edit of a batch is checked before any is applied, then again, in order, as it is applied on the state the earlier ones leave: an edit an earlier one changes (its words changed or now appear twice, its block deleted or rewritten, a block named twice) refuses the whole batch, a conservative rule. On a refusal, apply none, and return every failure (which edit, the reason, and the block's content in the latest stored state, not in the discarded edit; edits after an in-order failure are still checked) and a new read. On a pass, apply to the current state (blocks found by id there) and store with a compare-and-set; on a lost race, drop the state and the update computed on the older state and run the whole check and edit again on the newer one (bounded), or refuse back to the agent with a read of the state that won the last race: an update computed on an older state is never stored or sent. **Merge from the read only where there is nothing to check** (an append, a suggestion a person accepts, an edit by position): the fork-and-merge recipe stays for those cases. - y-server.md, "Edits by agents and tools" and "Edits by block id": the sketches check on the latest stored state and run `editYState` there, through `applyChecked` (the check; the edit, which checks each part again as it applies it and can refuse; the compare-and-set; up to 3 attempts, then a fresh load), instead of editing `seen` and merging. The word sketch takes a batch of replacements. `$findWords` finds a replacement's words by the block's id and requires them exactly once in that block's text, in one plain text node; `$blockAt` compares a target as a whole (its path and its serialized subtree); an answer by id is checked op by op and applied whole or not at all. "One edit at a time" says a checked edit starts over on a lost race instead of merging. `ServerDocsAgentEdits.test.ts` runs the sketches as written (they live in `__tests__/utils/agentEditSketch.ts`, and the first test checks that the page holds the same code), including targets changed since the read, a batch with one failing op, a batch whose first edit changes what the second (and third) names, refusals that report the stored content, a format on named words applied where they are now, and a store that loses the race. - block-ids.md, "Handles and collaboration": the handle is resolved and the whole block checked against what the model read on the stored state, and the edit is applied there by id with `applyChecked`; `BlockHandles.test.ts` follows it (a block the person typed in now goes back as `changed`). - DECISIONS.md: D94 (decided); D71 says D94 replaced which state the edit runs on. Also, from the reviews of #9685: - y.md, Servers: the per-client order paragraph now opens "Causal order includes each client's own order", and the json-joy comparison is dropped (y.md and D92). - D93 and y.md name the orders: one order when the comment ends at the rewritten word, both when it continues past it. - `regressions/UndoInvisibleSteps.test.ts` (from #9630) pins D81: the 3 cells that assert a press also taking the next step are `test.fails`. They fail on main with gc off and on. ## Test plan ### Before The new cells against weaker sketches: no uniqueness check, a target compared by its path and text only, and an update computed before a lost race merged into the newer state (the docs' previous recipe): ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/ServerDocsAgentEdits.test.ts × the sketches are the same in the docs and the tests 13ms × a replacement of words refuses words that appear twice in the block or across nodes, and finds the block read by id 10ms × step 3 refuses the whole answer when a target is gone, moved, replaced, typed in, formatted or styled since the read 42ms × a lost race starts over on the newer state: applied when the check still holds there, refused when not, never the older edit 20ms ⎯⎯⎯⎯⎯⎯⎯ Failed Tests 4 ⎯⎯⎯⎯⎯⎯⎯ AssertionError: expected '// Your storage of one document: its …' to be '// Your storage of one document: its …' // Object.is equality - Expected + Received AssertionError: expected undefined to deeply equal [ { …(3) } ] - Expected: + Received: AssertionError: formatted: expected [] to deeply equal [ [ 1, 'the block changed' ] ] - Expected + Received ``` An independent review of an earlier head of this branch composed the word check per edit into a batch: each check passed on the stored state, the first edit invalidated the second, and the batch was stored half done; after 3 lost races the read returned was the one that had lost: ``` FAIL > dependent word batch refuses as a whole when an earlier op invalidates a later target - "refused": true, "sent": 0, "stores": 0, "text": "red fox\n\nspacer" + "refused": false, "sent": 1, "stores": 1, "text": "blue fox\n\nspacer" FAIL > dependency that creates another occurrence cannot silently skip the second op - "text": "red fox and hen\n\nspacer" + "text": "red fox and red\n\nspacer" FAIL > retry exhaustion supplies a fresh read after the final known lost race AssertionError: expected 'red fox\n\nxxspacer' to be 'red fox\n\nxxxspacer' ``` A second review found the refusal of such a batch reporting the block's text from the discarded edit (`blue fox and hen` where storage held `red fox and hen`), and only the first of two invalidated edits. Those cases are cells of `ServerDocsAgentEdits.test.ts` now, as is a third review's case, pinned as the documented scope: a field the server's schema dropped is not compared, so a rewrite passes over a collaborator's change to it. `UndoInvisibleSteps.test.ts` unpinned, on main: ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/UndoInvisibleSteps.test.ts × undoing a format on text a collaborator deleted takes the step before it too (1,2) × undoing a format on text a collaborator deleted takes the step before it too (2,1) × undoing a comment removal on deleted text takes the format change before it too AssertionError: expected 'one three X{0}[]' to be 'one three{0}[]' Tests 3 failed (3) ``` ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/ServerDocsAgentEdits.test.ts packages/lexical-y/src/__tests__/unit/BlockHandles.test.ts packages/lexical-y/src/__tests__/unit/regressions/UndoInvisibleSteps.test.ts packages/lexical-html/src/__tests__/unit/BlockIdDocs.test.ts Tests 30 passed | 3 expected fail (33) $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit (same files) Tests 30 passed | 3 expected fail (33) $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit packages/lexical-html/src/__tests__/unit/BlockIdDocs.test.ts Test Files 225 passed | 2 skipped (227) Tests 18549 passed | 306 expected fail | 130 skipped (18985) $ pnpm run ci-check Found 0 errors ``` With gc off, the same lexical-y run had one failure, `DisposeMemory.test.ts` ("40 editors bound, edited and disposed in a synchronous loop hold no document"), under 24 parallel workers; it passes 5 of 5 alone, and this change touches no code it runs (docs and tests only).
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Yjs's
UndoManagerskips an undo step only when it changes no item. A step that restores or removes only format items no reader shows is taken alone, and the press shows nothing; the next press then undoes what the first was meant to. On main this happens when a client formats text a collaborator then deletes: the format's items stay around the deleted text, and undoing the format removes only them. Keeping run ends in the collapse (#9617, parked) made such presses more common, and in the randomized sessions of its gate the press that did nothing was sometimes the one meant to remove text.Now
YBinding#applyUndocompares a digest of what any reader can show (the live content in order with its format values, the attributes and the nested types) before and after each step and, while it is unchanged and steps remain, takes the next step in the same press. A press a stack listener starts while an undo runs takes one step, as before (the running step is not done yet). This is a behaviour change to undo: a press now takes more than one step wherever a step showed nothing. DECISIONS.md D81 records it asOPEN, for a maintainer's call; y.md's undo paragraph says it.Test plan
Before
pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/UndoInvisibleSteps.test.tson main (1853707):UNDO_PRESS_SEEDS=200 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/UndoPressProperty.test.tson main:After
with 200 sessions per client id order, each press checked against a fresh read of the document.
Open before this can leave draft
OPEN: a maintainer decides whether undo should take more than one step where a step shows nothing.KeepEndsSessions3645 flips with this change (the session shows main's dropped comment only after an undo that this change makes take more steps); remove itstest.failshere then.