Repository navigation
[lexical-y] Chore: Add undo run-end, own-run restore end and run-sequence cases, with the ones main fails as expected failures - #9605
Merged
Conversation
…ones main fails as expected failures ## Description Tests only, no change to the binding. They come from #9561 (an undo keeps the run ends of the text it removes), parked as a draft, and from the randomized run-sequence sessions, so the cases stay in main whatever lands later. - `UndoRunEnds.test.ts`: undos of text typed inside a bold, italic, underlined, styled or commented run while a collaborator types next to it; a paragraph inserted at an Enter split; type, delete, undo, undo; the undo of the run such run ends restate; restarts; several inserts; the skipped-step path; token and segmented nodes; long pastes (link-read counts). The cases main fails are `test.fails`: they pass on #9561's branch. The concurrent-undo case that #9561 gets wrong (#9593) passes on main and is a plain test here. - `RunSequenceCases.test.ts`: two reduced run-sequence sessions. Seed 175 (one undo step that underlines a run, types and underlines what it typed, undone while a collaborator types inside the run) and seed 8500123 (a collaborator's undo of its text inside a run together with the writer's undo of the run's format, seen or not). Main passes both; #9561's branch fails the unseen variant of the second (#9593). `RunSequenceUndo.test.ts`, the 200-seed run-sequence fuzz, is in #9562. ## Test plan ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/UndoRunEnds.test.ts packages/lexical-y/src/__tests__/unit/RunSequenceCases.test.ts Tests 103 passed | 81 expected fail (184) ``` On #9561's branch (fbb792b) the 81 expected failures pass and the unseen variants of the seed-8500123 case fail. Without its seed-175 fix (282d1ecf) the seed-175 case fails. `pnpm run ci-check` passes.
|
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: |
This was referenced Oct 30, 2026
|
AI review status for this pull request.
|
… from #9562, with the ones main fails as expected failures ## Description Tests only. #9562 (a restore inside a run its own client opened gets a null of its own after the run's end) is parked; its cases move to main here, so they stay whatever lands later. - `OwnRunRestoreEnd.test.ts`: the cases where the writer opened the run itself (a comment's restore whose end a collaborator's delete drops; a comment written with a hole in it; the same after the writer wrote the value again; 64 comment ids on the run; 1, 16, 64 and 128 comment ids dropped with the word) fail on main and are `test.fails`, tracked in #9609. The cases #9562 regressed and main handles are plain tests: S9-12 and S9-12b, the review's cases of a restore in another client's run after that client wrote the value again, a later write's hole (comment and bold), and the #9589 tie where the author opened the run (a collaborator's bold starting at, or running past, the author's bold run's end stays). - `OwnCommentGapCollapse.test.ts`: one write inside a comment its author opened, undone while a collaborator who has seen it deletes past the comment's end across another comment; the comment ends where it ended. Main passes; #9562 fails it in both id orders. - `RunSequenceUndo.test.ts`: the 200-seed two-client run-sequence fuzz; only seed 87 fails, as on main (#9599). ## Test plan ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/OwnRunRestoreEnd.test.ts packages/lexical-y/src/__tests__/unit/OwnCommentGapCollapse.test.ts packages/lexical-y/src/__tests__/unit/RunSequenceUndo.test.ts packages/lexical-y/src/__tests__/unit/UndoRunEnds.test.ts packages/lexical-y/src/__tests__/unit/RunSequenceCases.test.ts Tests 124 passed | 97 expected fail (221) ``` The same on main at 2fe8056 (the three new files: 21 passed, 16 expected fail). On #9562's branch `OwnCommentGapCollapse` fails 4 of 4: `expected 'aa Kil[c1] bb[c1,c2] cc[c1] dd[c1]' not to match /(bb|cc|dd)\[c1/`. `pnpm run ci-check` passes.
zurfyx
changed the title
…lapse case
## Description
`OwnCommentGapCollapse.test.ts` checked only that `bb`, `cc` and `dd` carry
no `c1` and that the second line is untouched: losing the comment on `Kil`
(or `c2` on `bb`) passed too, so a mutation that skips A's comment of
`Kilo` passed all four cells. It now asserts the whole first line as main
gives it: `aa Kil[c1] bb[c2] cc dd` when B deletes `o` and the space,
`aa Kil[c1] cc dd` when it deletes `o` and ` bb`. Reported in review.
## Test plan
```
$ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/OwnCommentGapCollapse.test.ts
Tests 4 passed (4)
```
With A's comment of `Kilo` removed from the test, all four cells fail now:
```
AssertionError: expected 'aa Kil bb[c2] cc dd' to be 'aa Kil[c1] bb[c2] cc dd' // Object.is equality
AssertionError: expected 'aa Kil cc dd' to be 'aa Kil[c1] cc dd' // Object.is equality
```
On #9562's branch all four still fail. `pnpm run ci-check` passes.
…llapses spaces
## Description
The word model these tests compare (`words()`, `line()`) splits text on
spaces and joins the words with one space, so it cannot tell `aa Kil bb`
from `aa Kilbb`: in `OwnCommentGapCollapse`, leaving out B's second delete
(the space) still passed. The final checks now also assert the first
line's text exactly, as main gives it: `aa Kilbb cc dd` and `aa Kil cc dd`
there, and `aa Xbb cc`, `aa Ybb cc` and `aa Kilome cc dd` for the four
`OwnRunRestoreEnd` cases whose result is checked with `words()` after the
merge. Reported in review.
## Test plan
```
$ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/OwnRunRestoreEnd.test.ts packages/lexical-y/src/__tests__/unit/OwnCommentGapCollapse.test.ts
Tests 20 passed | 16 expected fail (36)
```
With B's second delete left out of the `o then space` rows, those two
cells fail now:
```
AssertionError: expected 'aa Kil bb cc dd' to be 'aa Kilbb cc dd' // Object.is equality
```
`pnpm run tsc-test`, `pnpm run prettier` and `pnpm run lint` pass.
zurfyx
marked this pull request as ready for review
October 30, 2026 18:54
zurfyx
requested review from
acywatson,
etrepum,
ivailop7 and
potatowagon
as code owners
October 30, 2026 18:54
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
The added cases look good to me: they converge on every peer, pass at this head (124 passed, 97 expected failures), and test.fails is a sensible way to keep the parked work's cases on main. A few optional, non-blocking suggestions:
UndoRunEnds.test.ts:663. The six cases of “the end a kept restore relies on stays ended when a collaborator deletes the run” fail on main only atexpect(hNulls.length).toBe(2), which counts the nulls #9561's undo keeps (main's undo deletes both). Without that line, the user-visible assertion on line 667 ('one three') already passes on main in all six id orders, but astest.failsthat outcome is not checked on main at all. Dropping the count, or leaving it to #9561, would let this run as a plaintest.eachnow.UndoRunEnds.test.ts:528-547. InboldAfterUndoneWord, neither the undo nor the redo writes a transaction with the binding's run-end origin on main, so the throwing listener never fires and the case passes on its content checks alone; the follow-up writes named at lines 549-568 do not happen on main either. A note that both depend on #9561, or a scenario where main'scloseRestoredRunswrites a follow-up, would make them exercise what their names say.- Comments that describe a parked branch.
UndoRunEnds.test.ts:1300-1304describes #9561's outcome (twoturns bold, #9593) above an assertion thattwoends plain, andOwnRunRestoreEnd.test.ts:250-256(S9-12) describes #9562's rule 5, which main does not apply to a run the writer opened (lines 48-50 and S9-12b say so). Prefixing them with “With #9561” and “With #9562” would remove the contradiction. - Run-sequence agreement.
RunSequenceCases.test.ts:149-153andRunSequenceUndo.test.ts:199-213compare only per-word format maps between the clients, which run withoutselfCheck. Comparing both editors' JSON (or using the harness'sexpectConverged) would also catch a text or structure divergence; all 200 seeds already agree on the full JSON on main. - Test plan. AGENTS.md asks for the real Before and After output in the Test plan; the results on #9561's and #9562's branches are described rather than pasted.
No changeset is needed: CONTRIBUTING.md and AGENTS.md exempt test-only changes.
| hNulls.push(it); | ||
| } | ||
| } | ||
| expect(hNulls.length).toBe(2); |
There was a problem hiding this comment.
Optional: on main all six id orders fail here (main's undo deletes both nulls, so the count is 0). Without this line, the assertion on line 667 ('one three') passes on main, but as test.fails that outcome is not checked on main at all. Dropping the count, or leaving it to #9561, would let this case run as a plain test.each.
| test('a listener of the follow-up write that throws leaves it undone with its step', async () => { | ||
| const t = await boldAfterUndoneWord(); | ||
| const listener = (tr: Y.Transaction) => { | ||
| if ((tr.origin as {runEnd?: boolean} | null)?.runEnd === true) { |
There was a problem hiding this comment.
Optional: in boldAfterUndoneWord on main, neither a.undo() nor the redo writes a transaction with the run-end origin, so this listener never throws and the case passes on its content checks alone. A note that it depends on #9561, or a scenario where main's closeRestoredRuns writes a follow-up, would make it exercise what its name says.
Comment on lines
+1300
to
+1304
| // F's undo keeps its null and the restore after " mid" (text typed | ||
| // next to " mid" would rely on them); A's undo, which has not seen | ||
| // F's, removes the bold's opener and end, so the restore gives " two" | ||
| // bold for everyone. Settling it needs a write after both are seen, | ||
| // which no client may make while it applies an update. |
Comment on lines
+250
to
+256
| // S9-12: one write of A types uncommented text at two places inside A's | ||
| // own comment, so both of its restores end at the comment's end. Rule 5 | ||
| // gives that end one null of A's own (not one per restore: the undo | ||
| // removes them together, and a second one would push the comment's end | ||
| // out of the last items a collaborator's collapse keeps). A undoes the | ||
| // write while B, which has not seen the undo, deletes the rest of the | ||
| // comment and then the text after it. |
Comment on lines
+208
to
+213
| if ( | ||
| changed.length > 0 || | ||
| JSON.stringify(after) !== JSON.stringify(afterB) | ||
| ) { | ||
| failing.push(seed); | ||
| } |
There was a problem hiding this comment.
Nit: agreement here is the per-word format maps only, and the clients run without selfCheck. Also comparing both editors' getEditorState().toJSON() would catch a text or structure divergence; all 200 seeds agree on the full JSON on main.
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
Tests only, no change to the binding. They come from #9561 (an undo keeps
the run ends of the text it removes), parked as a draft, and from the
randomized run-sequence sessions, so the cases stay in main whatever
lands later.
UndoRunEnds.test.ts: undos of text typed inside a bold, italic,underlined, styled or commented run while a collaborator types next to
it; a paragraph inserted at an Enter split; type, delete, undo, undo;
the undo of the run such run ends restate; restarts; several inserts;
the skipped-step path; token and segmented nodes; long pastes (link-read
counts). The cases main fails are
test.fails: they pass on [lexical-y] Bug Fix: Keep the run ends of text an undo removes, and end a value an undo ends past other writes' run ends #9561'sbranch. The concurrent-undo case that [lexical-y] Bug Fix: Keep the run ends of text an undo removes, and end a value an undo ends past other writes' run ends #9561 gets wrong (Bug: [lexical-y] Text a client typed into a format run and undid gives the format back to the rest of the run when a collaborator undoes that format at the same time #9593) passes on
main and is a plain test here.
RunSequenceCases.test.ts: two reduced run-sequence sessions. Seed 175(one undo step that underlines a run, types and underlines what it typed,
undone while a collaborator types inside the run) and seed 8500123 (a
collaborator's undo of its text inside a run together with the writer's
undo of the run's format, seen or not). Main passes both; [lexical-y] Bug Fix: Keep the run ends of text an undo removes, and end a value an undo ends past other writes' run ends #9561's branch
fails the unseen variant of the second (Bug: [lexical-y] Text a client typed into a format run and undid gives the format back to the rest of the run when a collaborator undoes that format at the same time #9593).
RunSequenceUndo.test.ts, the 200-seed run-sequence fuzz, is in #9562.Test plan
On #9561's branch (fbb792b) the 81 expected failures pass and the unseen
variants of the seed-8500123 case fail. Without its seed-175 fix (282d1ecf)
the seed-175 case fails.
pnpm run ci-checkpasses.Follow-up: #9562's cases
#9562 is parked as well; its cases are added in a second commit:
OwnRunRestoreEnd.test.ts(the own-run cases main fails astest.fails, tracked in #9609; the cases #9562 regressed as plain tests),OwnCommentGapCollapse.test.ts(one write inside the author's comment undone while a collaborator deletes past its end across another comment; main passes) andRunSequenceUndo.test.ts(200-seed run-sequence fuzz, only #9599's seed 87 fails, as on main).