Repository navigation
[lexical-y] Chore: Pin #9632 and the follow-up reply loops in regression tests - #9642
Merged
Merged
Conversation
…ion tests ## Description Tests only, for #9632 (an editor's list numbering is stale after four follow-ups in a row that the binding reads again) and for the loops its fix has to keep cut off. Whichever way #9632 is decided, its tests are ready: - `RefusedFollowUpRefresh.test.ts` (25 cells) and `RefusedFollowUpFreshness.test.ts` (32 cells): follow-ups in a row that are read again, then one that empties the list item holding a nested list, checked against a fresh reader. The 6 + 24 cells where the fourth re-read in a row runs no refresh are `test.fails` naming #9632. Their exact predicates: the naming tag and 3 or more follow-ups before (Refresh); 3 or more follow-ups before (Freshness). - Guards that pass on main, for any fix of #9632: - `RefusedFollowUpLoops.test.ts` (16) and `RefusedFollowUpReadOnly.test.ts` (8): an app's listener that answers every refresh, or every collaboration-tagged commit, with a follow-up the binding does not write. Active and read-only clients, discrete and batched, one or both peers. - `RefusedFollowUpValueReplies*.test.ts` (8 + 6 + 6): a listener that answers a derived value (a nested list item at `value` 2) the same way, nested in the listener, in a microtask or in a timer. - A list whose own transform adds an item on every refresh: cut off after 4 runs, with one warning, and the editor shows the document. ## Test plan ### Before The files do not exist. ### After On main (551fe4a): ``` $ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUp Test Files 7 passed (7) Tests 71 passed | 30 expected fail (101) ``` The same on two more runs. With the markers removed, the 30 cells fail with `expected [ 1, 1 ] to deeply equal [ 1, 2 ]` (and `[ 7, 1 ]` for a list starting at 7). `pnpm run ci-check` passes.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
zurfyx
requested review from
acywatson,
etrepum,
ivailop7 and
potatowagon
as code owners
October 31, 2026 01:24
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: |
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks, this looks good. The 30 test.fails cells pin #9632 precisely: with the markers removed, exactly those cells fail with the numbers the issue reports, and the other 71 cells pass. They also follow packages/lexical-y/AGENTS.md (a known failure is a test.fails that names its issue). A few optional, non-blocking suggestions:
- The value-reply guards hold only while #9632 stands (
RefusedFollowUpValueReplies.test.ts:122-125, and the BothPeers and ReadOnly copies). These chains stop because the fourth re-read leaves the wrapper item'svaluestale: at the end of each listener cell the author shows[1, 1, 3]while a fresh reader of the same document shows[1, 2, 2]. A fix that keeps the value fresh keeps answering until the fuse, as #9634 describes for its head. LikewiseRefusedFollowUpRefresh.test.ts:173pinsexpect(runs).toBe(4), a count that exists because the last re-read runs no refresh, while #9634's copy of this test expects 5. So "guards that pass on main, for any fix of #9632" holds for #9634's options 1 and 3, not option 2. Consider saying so in those file headers, and bounding line 173 asRefusedFollowUpValueReplies.test.ts:179does (toBeLessThanOrEqual(5)) if it should survive any fix. both=trueinRefusedFollowUpLoops.test.tsnever runs B's listener (line 56). Loggingcountsgives[4, 0]in all 16 cells: after B's own edit nothing remote reaches B, because A's follow-ups are refused and write nothing. Giving both peers a remote change (offline concurrent edits, asRefusedFollowUpValueRepliesBothPeers.test.ts:81-87does) and assertingMath.min(...counts) > 0would make the header's "on one or both peers" true.- Assert that the listener ran in
RefusedFollowUpLoops.test.ts:95-102andRefusedFollowUpReadOnly.test.ts:108-111. Only upper bounds are checked there, so these 24 cells would stay green if the refresh or collaboration tags the listeners key on changed. The ValueReplies files already do this ('listener actually exercised'). - Minor: the ValueReplies cells assert
n < 20after a 100 ms wait whilenis 4 or 5; the<= 5bound inRefusedFollowUpLoops.test.ts:99-102does not depend on timer rounds fitting in the wait.RefusedFollowUpValueReplies.test.ts:13cites "#9632 review" where the review was on #9634.RefusedFollowUpFreshnessand theRefusedFollowUpRefreshmatrix could spy onconsole.warnas the other five files do, which keeps the binding's development warnings out of the test output.
No changeset is needed for a tests-only change (CONTRIBUTING.md, Changesets).
| looping = null; | ||
| // The first refresh and the one after each of the re-reads before the | ||
| // last (MAX_REREADS = 3); the last runs none. | ||
| expect(runs).toBe(4); |
There was a problem hiding this comment.
Optional: this exact count is the one that exists because the last re-read runs no refresh (the #9632 mechanism); #9634's copy of this test expects 5. If this cell is meant to guard any fix of #9632, an upper bound such as toBeLessThanOrEqual(5) (as RefusedFollowUpValueReplies.test.ts:179 uses) plus the warning and fresh-reader checks would survive either outcome.
| .mockImplementation(() => {}); | ||
| try { | ||
| t.peers.forEach((p, i) => { | ||
| if (i > 0 && !both) return; |
There was a problem hiding this comment.
Optional: with both=true, B's listener never runs. Logging counts gives [4, 0] in all 16 cells, because after B's own edit nothing remote reaches B (A's follow-ups are refused and write nothing). Offline concurrent edits on both peers, as RefusedFollowUpValueRepliesBothPeers.test.ts:81-87 does, plus expect(Math.min(...counts)).toBeGreaterThan(0) would exercise the second peer.
| expect( | ||
| Math.max(...counts), | ||
| 'same bound as MAX_REREADS plus initial read/refresh', | ||
| ).toBeLessThanOrEqual(5); |
There was a problem hiding this comment.
Optional: only upper bounds are checked here (and at RefusedFollowUpReadOnly.test.ts:108-111), so the cells would stay green if the refresh or collaboration tags the listener keys on changed and no loop were attempted. An expect(counts[0]).toBeGreaterThan(0), like the ValueReplies files' 'listener actually exercised', closes that.
| expect(t.a.doc.store.pendingDs).toBe(null); | ||
| expect(t.peers.map(p => p.errors)).toEqual([[], []]); | ||
| expect(n, 'listener actually exercised').toBeGreaterThan(0); | ||
| expect(n, 'no listener-driven loop; 20 is a test fuse').toBeLessThan( |
There was a problem hiding this comment.
Optional: this chain stops because the fourth re-read leaves the wrapper item's value stale; at this point the author shows [1, 1, 3] while a fresh reader of the same document shows [1, 2, 2]. So this guard holds under #9634's options 1 and 3 but not option 2, which may be worth saying in the header. Separately, n is 4 or 5 here, so toBeLessThanOrEqual(5) (as in RefusedFollowUpLoops.test.ts:99-102) would not depend on 20 timer rounds fitting in the 100 ms wait.
…uards
## Description
The value-reply guards waited a fixed 100 ms after the sync, then checked
that the chain of replies stopped before its fuse. A binding that never
stops the chain passed when each reply took longer: the check ran before
the chain reached the fuse (review). The guards now count replies
scheduled and not yet run, and wait until none is scheduled for three turns
of the event loop in a row, however long each takes. They require that every
scheduled reply ran.
## Test plan
### Before
With each reply made to take 80 ms (an `Atomics.wait` in the scheduled
task), on a binding that loops on these replies, the fixed-delay version
passed the timer and microtask cells.
### After
```
$ pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/RefusedFollowUpValueReplies
Tests 20 passed (20)
```
With each reply made to take 80 ms: 12 of 12 pass on main, and on a binding
that loops on these replies all 12 fail (`expected 20 to be less than 20`).
There was a problem hiding this comment.
Thanks, this looks good. The test.fails cells in RefusedFollowUpRefresh.test.ts and RefusedFollowUpFreshness.test.ts match the #9632 failures exactly: removing the markers fails just those 30 cells, each with the stale value. The guards check both peers' documents, a fresh reader and the client clocks, not only that the loop ends. A few optional, non-blocking suggestions:
- Say what the value-reply guards depend on. This applies to
RefusedFollowUpValueReplies.test.ts:10-15,RefusedFollowUpValueRepliesBothPeers.test.ts:9andRefusedFollowUpValueRepliesReadOnly.test.ts:9-10. These listeners answer only while the nested item showsvalue2 (RefusedFollowUpValueReplies.test.ts:77). Onmainthe chain stops because the fourth re-read in a row leaves that value at 1, which is the #9632 bug itself. A fix that refreshes numbering on the last re-read, as #9634 describes for its head, makes all 18 listener cells run to the fuse. So they hold for option 1 in #9634, not "for any fix of #9632". A header line saying so, revisited with the #9634 decision, would stop the next author from reading a fuse failure as a regression. If the guards are meant to hold whatever is decided,packages/lexical-y/AGENTS.md("Decisions") asks for the DECISIONS.md row in the same pull request. - Exercise peer B in the both-peers Loops cells (
RefusedFollowUpLoops.test.ts:36). B makes the only user edit (line 83), and A's follow-ups are never written, so B's listener never fires:countsis[4, 0]in every cell.RefusedFollowUpValueRepliesBothPeers.test.ts:107-126edits on both peers while offline and assertsMath.min(...counts) > 0. Doing the same here would cover "both peers". - Bound the run count (
RefusedFollowUpRefresh.test.ts:173).toBe(4)pins how many times the refresh runs onmain.toBeLessThanOrEqual(5), as inRefusedFollowUpValueReplies.test.ts:185, keeps the guard about the cut-off, and the JSON comparisons after it already check what the editor shows.
A tests-only change needs no changeset (AGENTS.md, "Changesets").
Comment on lines
+10
to
+15
| // An app's update listener that answers a collaboration-tagged commit, when | ||
| // a nested list item shows `value` 2, by emptying that item in a follow-up | ||
| // the binding does not write (nested in the listener, in a microtask or in a | ||
| // timer): the chain of replies stops (#9632 review). And a list whose own | ||
| // transform adds an item on every run keeps its numbering when its change is | ||
| // read again. |
There was a problem hiding this comment.
Optional: these listeners answer only while the nested item shows value 2 (line 77). On main the chain ends because the fourth re-read in a row leaves that value at 1, which is the #9632 bug the test.fails cells pin. A fix that refreshes numbering on the last re-read (the shape #9634 describes) makes all 18 listener cells here and in the BothPeers and ReadOnly files run to the fuse. Saying so in this header and the other two would stop them reading as fix-agnostic guards, for example:
// These stop on main because the last re-read leaves `value` stale (#9632);
// revisit them with the decision in #9634.| [1, 2], | ||
| [2, 1], | ||
| ]) | ||
| for (const both of [false, true]) |
There was a problem hiding this comment.
Optional: with both=true, peer B's listener never fires. B makes the only user edit (line 83), and A's follow-ups are never written, so nothing reaches B: counts stays [4, 0] in every cell. RefusedFollowUpValueRepliesBothPeers.test.ts:107-126 edits on both peers while offline and asserts Math.min(...counts) > 0. Doing the same here would make these 8 cells cover two answering peers.
| looping = null; | ||
| // The first refresh and the one after each of the re-reads before the | ||
| // last (MAX_REREADS = 3); the last runs none. | ||
| expect(runs).toBe(4); |
There was a problem hiding this comment.
Optional nit: toBe(4) pins how often the refresh runs the transform on main, and a refresh on the last re-read makes it 5. toBeLessThanOrEqual(5), as RefusedFollowUpValueReplies.test.ts:185 uses, keeps this guard about the loop being cut off. The JSON comparisons below already check what the editor shows.
zurfyx
added a commit
that referenced
this pull request
Oct 31, 2026
## Description Merges main at f59f74d (#9631, #9642 and #9641). There were no conflicts. ## Test plan ### Before The branch was on main 2e2acb7. ### After ``` $ pnpm exec vitest run --project unit --project scripts-unit Tests 30437 passed | 282 expected fail | 135 skipped (30854) $ MATRIX_FULL=1 pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/WrapperTextFields.test.ts Tests 9062 passed | 206 expected fail (9268) $ pnpm run ci-check Found 0 errors ```
This was referenced Nov 1, 2026
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).
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, for #9632 (an editor's list numbering is stale after four
follow-ups in a row that the binding reads again) and for the loops its
fix has to keep cut off. Whichever way #9632 is decided, its tests are
ready:
RefusedFollowUpRefresh.test.ts(25 cells) andRefusedFollowUpFreshness.test.ts(32 cells): follow-ups in a row thatare read again, then one that empties the list item holding a nested
list, checked against a fresh reader. The 6 + 24 cells where the fourth
re-read in a row runs no refresh are
test.failsnaming Bug: [lexical-y] After refused naming follow-ups, the author shows a number list's wrapper item with value 1 where its own document reads 2 #9632. Theirexact predicates: the naming tag and 3 or more follow-ups before
(Refresh); 3 or more follow-ups before (Freshness).
RefusedFollowUpLoops.test.ts(16) andRefusedFollowUpReadOnly.test.ts(8): an app's listener that answersevery refresh, or every collaboration-tagged commit, with a follow-up
the binding does not write. Active and read-only clients, discrete and
batched, one or both peers.
RefusedFollowUpValueReplies*.test.ts(8 + 6 + 6): a listener thatanswers a derived value (a nested list item at
value2) the same way,nested in the listener, in a microtask or in a timer. They wait until no
reply is scheduled (not a fixed delay), so a chain of slow replies is
checked once it ends.
4 runs, with one warning, and the editor shows the document.
Changeset
None: tests only.
Test plan
Before
The files do not exist.
After
On main (551fe4a):
The same on two more runs. With each reply made to take 80 ms, the value-reply guards still pass on main (12 of 12), and fail on a binding that loops on these replies (12 of 12,
expected 20 to be less than 20). With the markers removed, the 30 cells fail withexpected [ 1, 1 ] to deeply equal [ 1, 2 ](and[ 7, 1 ]for a liststarting at 7).
pnpm run ci-checkpasses.