Repository navigation
[lexical-y] Chore: Point the known-failure tests at open issues - #9684
Merged
Merged
Conversation
zurfyx
requested review from
acywatson,
ivailop7 and
potatowagon
as code owners
November 1, 2026 01:07
|
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: |
zurfyx
force-pushed
the
zurfyx/y-test-markers
branch
from
November 1, 2026 01:12
b7b5344 to
71dfc59
Compare
![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. Every known failure in @lexical/y's undo run-end tests now names an open issue instead of a parked draft, and the F30A-1 italic case is pinned to #9683 for the gc-on lane only, with the gc-off lane untouched.
The markers hold at this head. Each of the 81 cells marked in UndoRunEnds.test.ts still fails with the shape of the issue named above it, with and without LEXICAL_Y_GC=1:
- #9681: a collaborator's text takes the undone run's value.
- #9682: the value survives an undo past a restore the undo has seen.
The italic F30A-1 cells fail with gc on exactly as #9683 describes. As CONTRIBUTING.md says for tests, no changeset is needed.
Optional, non-blocking:
packages/lexical-y/AGENTS.mdonly requires aDECISIONS.mdrow update when a pull request changes a decision, and this one doesn't. Still, the new header cites D74 and D64, while those rows' Links cells (packages/lexical-y/DECISIONS.md:139and:133) don't list #9681 or #9682 yet. The Open issues list inpackages/lexical-website/docs/collaboration/y.mddoesn't name #9681 or #9683 either. Adding them, here or in a follow-up, would let a reader get from the register to the tests and back.
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).
## Description Two markers in `@lexical/y`'s tests named nothing a reader could follow, and one lane failed on a known gc-only case: - `UndoRunEnds.test.ts`: its 14 `test.fails` groups (81 cells) named only #9561, a pull request parked as a draft. Each now names the open issue that tracks it: #9681 (an undo of text typed inside a run deletes that text's run ends, so a collaborator's plain text next to it takes the run's value; D74) or #9682 (an undo of a run's value leaves it after a restore the undo has seen; D64, documented in y.md's Limits, Undo). The file's header says so and that #9561 fixes both. - `regressions/Formats.test.ts`, F30A-1: the italic case fails with gc on (`LEXICAL_Y_GC=1`, which #9679 added), for 3 client id orders, the same on main before #9679. It is now a `test.fails` naming #9683 with gc on only, so the gc-on lane is green. - `regressions/PasteRemadeLine.test.ts` names #9587, which GitHub had closed by mistake (#9598's "Not fixed: #9587" reads as a closing keyword); the repro still fails on main, and the issue is reopened. No change to the test. No behavior change; tests only. ## Test plan ### Before ``` $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts -t F30A-1 × italic over "b ", italic removed from "b"; the removal undone ‖ "b" italic again (ids 1,2,3) × italic over "b ", italic removed from "b"; the removal undone ‖ "b" italic again (ids 3,2,1) × italic over "b ", italic removed from "b"; the removal undone ‖ "b" italic again (ids 2,3,1) AssertionError: expected '"a" "b"[I] " cd ef" / "next"' to be '"a" "b "[I] "cd ef" / "next"' Tests 3 failed | 8 passed | 187 skipped (198) ``` `UndoRunEnds.test.ts` with every `test.fails` turned into `test`: 81 failed, 98 passed, on gc off and gc on (each of the 81 is one of the two issues' shapes; the issues carry standalone repros that fail on main). ### After ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/UndoRunEnds.test.ts packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts Tests 292 passed | 85 expected fail (377) $ LEXICAL_Y_GC=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/UndoRunEnds.test.ts packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts Tests 289 passed | 88 expected fail (377) ``` Whole `@lexical/y` unit project (`vitest run --project unit packages/lexical-y/`), gc off and on (`LEXICAL_Y_GC=1`): ``` Tests 1 failed | 18528 passed | 291 expected fail | 130 skipped (18950) Tests 18526 passed | 294 expected fail | 130 skipped (18950) ``` The one failure is `DisposeMemory.test.ts` (heap held above its 8 MB bound) with 24 workers. It passes alone and 12 at a time, and the same full-suite run fails it on main before #9679 (cfcd3c4, 9.4 MB): a heap bound that full-suite load can exceed, unrelated to this change.
zurfyx
force-pushed
the
zurfyx/y-test-markers
branch
from
November 1, 2026 01:36
71dfc59 to
be84cf8
Compare
There was a problem hiding this comment.
Looks good: every marker now names an open issue that matches the failure it pins, and the change stays test-only.
With the markers removed, the same 81 cells in UndoRunEnds.test.ts fail with garbage collection off and with it on, each group on one assertion. Each group fails the way the issue it names describes: for #9681, E's plain text takes the run's value after F's undo; for #9682, A's undo leaves the bold after F's restore. The three F30A-1 italic cells fail only with LEXICAL_Y_GC=1, as #9683 says.
Optional, non-blocking:
packages/lexical-y/src/__tests__/unit/regressions/Formats.test.ts:912: the unit job in.github/workflows/call-core-tests.ymldoes not setLEXICAL_Y_GC, so CI only runs this case with gc off and will not flag thetest.failsonce #9683 is fixed. A note in #9683 that the fix needs aLEXICAL_Y_GC=1run to drop the marker would keep it from going stale.- The Test plan describes the Before run of
UndoRunEnds.test.ts(81 failed, 98 passed with everytest.failsturned intotest) instead of pasting its output, which AGENTS.md ("Commits and Pull Requests") asks for. The numbers reproduce, so this only matters for the record.
No changeset is needed: CONTRIBUTING.md ("Changesets") exempts test-only changes.
| ]; | ||
| for (const [name, $first, $second, $again, expected] of cases) { | ||
| // #9683: with gc on, the italic case takes the italic off the space. | ||
| const knownGcFailure = WITH_GC && name.startsWith('italic'); |
There was a problem hiding this comment.
Optional, non-blocking: the unit job in .github/workflows/call-core-tests.yml does not set LEXICAL_Y_GC, so CI only runs this case with gc off and will not flag this test.fails once #9683 is fixed. A note in #9683 that the fix needs a LEXICAL_Y_GC=1 run to drop the marker would keep it from going stale.
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
…9694 (an editor stopped matching its own document in long sessions) Reverts #9679 whole, on top of current `main` (keeps #9684 and #9685). With #9679, a downstream app's long-note sessions end with an editor that no longer matches its own Yjs document (15 of 133 sessions; 0 of 132 on the commit before #9679). Reverting only part of it does not work: the delete rule (D86) alone is worse on a recorded session (2810), and the divergence was traced to one of its hunks (the end a restated run gets at a range's end, which could be linked into another container; #9693 fixes that at the source). This PR is the fallback if #9693 is not taken. Closes #9694 if merged instead of #9693. What this brings back, as before #9679: - A delete collapses the run of deleted content and format items it leaves to a bounded number per format key (D24's bound): the cases it fixed (#9575, #9609, a #9620 cell, #9640's concurrent restore cells) fail again and are pinned with their issues. - No format items kept from garbage collection: with gc on, an undo can again put a value back past its end (#9623). The harness keeps its `LEXICAL_Y_GC` switch (added by #9679), and the cells that fail only with gc on are pinned as `test.fails` with gc on, naming #9623. - The restore rules for #9671 and the restore scan for #9599 (D78) are gone: those cases are open again. `DECISIONS.md` keeps D86 and D88 (now `REVERTED`, with the reason) and reopens D78, rather than rewriting them. `y.md` goes back to the behavior before #9679. The two changesets of #9679 are removed: it was not part of a release, so there is nothing to announce. `UndoSpreadSession2810.test.ts` (added by #9679) is kept: it passes without D86 and D88 as well. `main` 60022ae: the downstream app's long-note sessions diverge in 15 of 133. ``` $ LEXICAL_Y_GC=0 vitest --run --project unit Test Files 1 failed | 648 passed | 4 skipped (653) Tests 1 failed | 28071 passed | 481 expected fail | 135 skipped (28688) $ LEXICAL_Y_GC=1 vitest --run --project unit Test Files 2 failed | 647 passed | 4 skipped (653) Tests 2 failed | 28055 passed | 496 expected fail | 135 skipped (28688) $ pnpm run ci-check exit 0 ``` The failures are load flakes with 40 workers: `DisposeMemory.test.ts` (heap bound; gc off and on) and `MarkdownEscaping.test.ts` (5 s timeout, gc on). Each passes alone (two runs each).
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
Two markers in
@lexical/y's tests named nothing a reader could follow,and one lane failed on a known gc-only case:
UndoRunEnds.test.ts: its 14test.failsgroups (81 cells) named only[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, a pull request parked as a draft. Each now names the open issue
that tracks it: Bug: [lexical-y] Undoing plain text typed into a format run gives the run's format to plain text a collaborator typed next to it #9681 (an undo of text typed inside a run deletes that
text's run ends, so a collaborator's plain text next to it takes the
run's value; D74) or Bug: [lexical-y] Undoing a format leaves it on the text after a collaborator's plain text inside the run, with no concurrent edits #9682 (an undo of a run's value leaves it after a
restore the undo has seen; D64, documented in y.md's Limits, Undo). The
file's header says so and 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 fixes both.
regressions/Formats.test.ts, F30A-1: the italic case fails with gc on(
LEXICAL_Y_GC=1, which [lexical-y] Bug Fix: A delete keeps every format item it leaves (D86), and a client keeps its own format items from garbage collection while they are undoable (D88) #9679 added), for 3 client id orders, the sameon main before [lexical-y] Bug Fix: A delete keeps every format item it leaves (D86), and a client keeps its own format items from garbage collection while they are undoable (D88) #9679. It is now a
test.failsnaming Bug: [lexical-y] With gc on, undoing a format removal while a collaborator applies the same format again takes it off the next character #9683 with gc ononly, so the gc-on lane is green.
regressions/PasteRemadeLine.test.tsnames Bug: [lexical-y] Typing inside the word after a pasted range lands before the paste when the pasted text starts like that word (in a line a transform makes again) #9587, which GitHub hadclosed by mistake ([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's "Not fixed: Bug: [lexical-y] Typing inside the word after a pasted range lands before the paste when the pasted text starts like that word (in a line a transform makes again) #9587" reads as a closing
keyword); the repro still fails on main, and the issue is reopened. No
change to the test.
No behavior change; tests only.
Test plan
Before
UndoRunEnds.test.tswith everytest.failsturned intotest: 81failed, 98 passed, on gc off and gc on (each of the 81 is one of the two
issues' shapes; the issues carry standalone repros that fail on main).
After
Whole
@lexical/yunit project (vitest run --project unit packages/lexical-y/),gc off and on (
LEXICAL_Y_GC=1):The one failure is
DisposeMemory.test.ts(heap held above its 8 MB bound)with 24 workers. It passes alone and 12 at a time, and the same full-suite
run fails it on main before #9679 (cfcd3c4, 9.4 MB): a heap bound that
full-suite load can exceed, unrelated to this change.