Repository navigation
Conversation
…ent run ends when a delete collapses deleted text's format items ## Description When a delete lands next to text deleted earlier, `Cursor#collapseGap` collapses the format items left in that run of deleted content. It kept a bounded number per family (the units around the delete, the first change, the last two units, the items a kept place needs, the value after the run) and deleted the rest. A collaborator who, before seeing the delete, typed text without a run's value inside the run (uncommented text inside a commented word) restored the value after their text and relied on the run's end to end it again. When the collapse deleted that end, the restored value ran on over every later word and line. Comments hit this because each comment id is its own key: a family's last units are other comments' items, so nothing ended the restore (#9609; also #9575, where the collapse kept the copies of an end that rule 5 writes and dropped the end itself, and the copies' writers then undid their writes). Now the collapse also keeps, of each key, the item that ends the key's last run in the stretch: an item that changes a value that is not null (a closer, a comment's or link's end, or a style's end that writes the text's own style again), or a null after content that had no value of the key (the end of a run whose opener an earlier collapse removed). A null right after another one with no content between them is a copy of the same end (rule 5) and still goes, so the run's own end is the one kept. Where nothing kept after that end gives the key its value after the run, the key's last item is kept too, so the content after the run keeps its value. Starts and the other redundant items still go: a start kept without its end would spread. Keeping an end changes nothing a reader shows: the run holds no live content, ending a value that is not open there is a no-op, and the value after the run stays. One end per key is enough: a restore before an earlier end is ended by the last one at the latest, so it can reach only text inserted into the run at the same time, never past it. The bound changes from a few items per family to one more item per key at a place; with comments (one key per id) that is one item per comment deleted at a place, and nothing per keystroke. The two growth tests now count that from a derivation in their comments. The decision and the rejected alternatives (a provenance marker; paired anchors / Peritext-style storage for comments, which adds another storage model on top of formats) are D65 and D79 in DECISIONS.md. y.md's Limits drop the #9575 sentence and say what a later delete keeps and what that costs. The `test.fails` cases in `OwnRunRestoreEnd.test.ts` stay expected failures: they delete the run's end with raw Yjs deletes, which no write of the binding makes now, and nothing else ends the restore once its end is gone. The same case with a real collapse passes. Closes #9609 ## Changeset `.changeset/lexical-y-collapse-keeps-run-ends.md`: `@lexical/y` patch. ## Test plan - `regressions/CollapseKeepsEnds.test.ts`: #9609 with a real collapse (a delete of the words around the commented word, then of the space after them), the end kept through a later collapse of the same run, 1/16/64/128 comment ids on the word, a kept end that must not end the comment before a later run of the same id (no peer writes for it), and bold and style controls. - `CollapseKeepsEndsProperty.test.ts`: random documents (comments with repeating ids, formats, styles, links), typing inside a word while the other peer deletes around it and next to it, compared character by character with the same session with no collapse at all: taking turns, everything matches; at the same time, the document's own text matches; no peer writes more. - `Races.test.ts`: #9575 flipped to a regular test. - `CursorFamily.test.ts`, `regressions/Formats.test.ts` F24A-1: growth counts from a derivation. `pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/CollapseKeepsEnds.test.ts packages/lexical-y/src/__tests__/unit/CollapseKeepsEndsProperty.test.ts packages/lexical-y/src/__tests__/unit/Races.test.ts` ### Before main d61302b with the new tests: ``` × #9575: the comment ends where it ended (1,2,3,4) × #9575: the comment ends where it ended (4,3,2,1) × #9609: a comment's restore ends where the comment ended (ids 1,2) × #9609: the comment's end stays through a later collapse of the same run (ids 1,2) × #9609: the restores of 1 comment ids on one word end where the comments ended (ids 1,2) × #9609: the restores of 16 comment ids on one word end where the comments ended (ids 1,2) × #9609: the restores of 64 comment ids on one word end where the comments ended (ids 1,2) × #9609: the restores of 128 comment ids on one word end where the comments ended (ids 1,2) × typing ‖ deletes: the document's own text shows as with no collapse (ids 1,2) (and the same for ids 2,1) AssertionError: expected 'xx X cc[c1,c3] dd[c1]' to be 'xx X cc[c3] dd' AssertionError: expected { seed: 53, view: [ 'X', …(35) ] } to deeply equal { seed: 53, view: [ 'X', …(35) ] } Tests 16 failed | 8 passed | 119 skipped (143) ``` ### After ``` Test Files 6 passed (6) Tests 358 passed | 23 expected fail (381) ``` (the three files above plus `CursorFamily`, `regressions/Formats` and `OwnRunRestoreEnd`). `pnpm exec vitest run --project unit packages/lexical-y packages/lexical-mark`: 219 files, 12856 passed, 299 expected fail. `pnpm run ci-check`: passed.
|
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: |
main added its own D79 (the Yjs guard), so the rejected-alternatives row for D65 is D80.
…ment spreading after deletes drop its end ## Description y.md listed, among known issues, a comment spreading onto later words when a client types uncommented text inside it offline while a collaborator's deletes drop the item that ended it (#9564). With the collapse keeping each key's last run end, a collaborator's deletes no longer drop that item, so the entry goes. ## Test plan Documentation only. The behaviour is covered by `regressions/CollapseKeepsEnds.test.ts` and `CollapseKeepsEndsProperty.test.ts` in this pull request; `pnpm run ci-check` passes.
![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: keeping the end of each key's last run is a tidy way to stop a restored comment from running on, and the new CollapseKeepsEnds cases fail on main and pass here, also with garbage collection on. The suggestions below are optional and non-blocking, though the first decides how far the #9575 fix reaches.
1. The copy check depends on garbage collection (packages/lexical-y/src/internal/Cursor.ts, lines 1024-1026, used at 1033-1037)
A null counts as a rule-5 copy when no content stands between it and the key's previous item, and content here is any item that is not a ContentFormat. With GC on, the createYDoc default that D23 keeps, Yjs replaces a deleted item's content with ContentDeleted, so a copy that an earlier collapse deleted counts as content on every client that collected it. Splitting #9575's deletes across two clients shows it:
- A comments
Kilometer; B types plainZinside it; C types plainX, thenY. - D1 deletes
ter; D2, having seen that, deletesbb. - B undoes; C undoes twice.
Taking turns (or with the undos offline), every client ends with aa |Kilome cc dd[c1] and tail line[c1]: D1's collapse deleted C's copy next to A's end, D2 counts that collected copy as content, keeps C's other copy as lastEnd and deletes A's end, and the undos then remove the copies. The same case passes with one deleter, or with gc: false, which is how every peer in packages/lexical-y/src/__tests__/utils/harness.ts (line 234) is created, so the suite cannot see it. main fails this case too, so it is not a regression. Counting it.countable instead (set when the item is created, so collection does not reset it) makes it pass with the changed tests still green, but an item a client receives already collected is not countable either; recognising a copy by its origin (rule 5 writes it with insertAfter(end)) may be sturdier. packages/lexical-y/AGENTS.md asks that every bug fix land with a regression test, and a case with gc-on peers would keep this one covered.
Repro sketch (peers created with GC on)
// five peers from utils/harness.ts, with createYDoc(null, {gc: true})
const [a, b, c, d1, d2] = t.peers;
a.update(() => void $addMarkID('c1', $selectIn([0], 3, 12)));
await t.sync();
typeAfter(b, 'K', 'Z');
await t.sync();
typeAfter(c, 'KZil', 'X');
await t.sync();
typeAfter(c, 'KZilXome', 'Y');
await t.sync();
deleteText(d1, 'ter'); // select the text, removeText()
await t.sync();
deleteText(d2, ' bb');
await t.sync();
b.undo();
await t.sync();
c.undo();
await t.sync();
c.undo();
await t.sync();
// expected: ['aa |Kilome[c1]| cc dd', 'tail line']
// received: ['aa |Kilome cc dd[c1]', 'tail line[c1]']typeAfter and words as in Races.test.ts.
2. Keep the Limits docs in step with the register (packages/lexical-y/DECISIONS.md line 111, packages/lexical-website/docs/collaboration/y.md)
packages/lexical-y/AGENTS.md asks that accepted limits also be documented in y.md's Limits. D51 now says "Replaced by D65" but stays an ACCEPTED LIMIT whose first case still needs the parked #9562, while this PR removes the y.md sentence that documented it. "Partly replaced by D65" and a narrowed y.md sentence (which could also name the two-deleter case above until it is fixed) would keep them consistent. y.md's open-issues list also drops #9564, which stays open and is not closed here: either close it with a regression test or keep the entry.
3. Nit: the description calls the rejected alternatives D79; in DECISIONS.md they are D80 (D79 is the Yjs guard limit from #9607).
| if (!isFormat(it)) { | ||
| contents++; | ||
| } |
There was a problem hiding this comment.
With GC on (the createYDoc default), a deleted format item's content becomes ContentDeleted, so this also counts a collected copy as content. A later copy then reads as a null after content and becomes lastEnd in place of the run's own end: #9575 with ter and bb deleted by two different clients still ends with the comment over the rest of the document. Counting it.countable passes that case, though an item received already collected is not countable; checking a copy's origin (rule 5 writes it with insertAfter(end)) may be sturdier. Optional, non-blocking.
| | D49 | **Delete wins:** text typed into, or a block moved into, a container a collaborator deletes at the same time is deleted with it. | Yjs deletes a parent's children; anything else would resurrect deleted content. | `ACCEPTED LIMIT` | 10-25 | [limits] | | ||
| | D50 | **Two clients removing the same text and both undoing show it twice.** | Plain `UndoManager` behaviour; avoiding it needs a layout feature. | `ACCEPTED LIMIT` | 10-23 | [limits] | | ||
| | D51 | **Rare undo and marker-cleanup timings can spread a comment or format onto untouched words** (two writes of one client both undone while a collaborator collapses markers; three authors in [#9575]). | Rare, no text lost. A provenance marker would need a layout version bump and about a week. | `ACCEPTED LIMIT` | 10-30 | [#9562], [#9575] | | ||
| | D51 | **Rare undo and marker-cleanup timings can spread a comment or format onto untouched words** (two writes of one client both undone while a collaborator collapses markers; three authors in [#9575]). | Rare, no text lost. A provenance marker would need a layout version bump and about a week. Replaced by D65 (10-30): the collapse keeps the run's own end, so [#9575] ends where it ended; the first case needed the own-run nulls of [#9562], which is parked. | `ACCEPTED LIMIT` | 10-30 | [#9562], [#9575] | |
There was a problem hiding this comment.
D51 stays an ACCEPTED LIMIT (its first case still needs #9562), so "Partly replaced by D65" may read more accurately. packages/lexical-y/AGENTS.md also asks for accepted limits to stay documented in y.md's Limits, and this PR removes the sentence that documented D51. Optional.
| format can run on past its end when writes two collaborators made inside | ||
| it are undone while another client deletes text across its end: it | ||
| carries on over the following words, possibly into the next line. No |
There was a problem hiding this comment.
With GC on, this still happens when the two deletes come from different clients (see the comment on Cursor.ts); consider keeping a narrowed version of this sentence until that is fixed. Optional.
…run where a later null ended a run that lost its opener ## Description The collapse kept, of each key, only its last end. Where that was a null ending a run whose opener an earlier collapse removed, the end of the value's own last run before it went, and text an undo brought back between the two took the value from before that end: a word typed without a comment and deleted came back with the comment of the word before it (reported by the gate on randomized sessions, seeds 2725 and 1861, and reduced by the review to a sequential session). Main kept that end there. Now the collapse keeps, of each key, the last item that ends a run of a value and, where it comes after that, the last null after content that had no value of the key; the key's last item stays where nothing kept after the later of the two, newly kept, gives the key its value after the run. Also: - The bound is stated as derived: per key used at a place, one more item (its end), or two where an earlier collapse there removed the opener of a later run of it, and up to two more for a key whose run starts again before the text after the run (its last item and another client's same-valued item before it); none per keystroke. The new growth assertion pins the three-delete case (c1 keeps [true, null, true, null], 9 items in the line, where main keeps 6). - "Changes nothing anyone shows" is narrowed to readers that have seen the delete: text an undo brings back into the run can take the values the kept ends give it. - y.md keeps its known issue for #9564 (comment spreads after deletes and undos still happen, #9620 and #9623). ## Test plan ### Before `pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/KeepEndsSessions.test.ts packages/lexical-y/src/__tests__/unit/regressions/KeepEndsUndoPlainWord.test.ts` on the previous head (f32b35c): ``` × restored text that never had a comment takes none (2725) × restored text takes no comment of the word before it (1861) ✓ a comment stays on text that deletes around it never touched (3645) ✓ a comment stays on text that deletes around it never touched (4694) × an undo brings a plain word back without the comment of the word before it (1,3,2) × an undo brings a plain word back without the comment of the word before it (3,1,2) ``` On main (d61302b) the first two and the last two pass, and 3645 and 4694 fail (main drops a comment from text no delete touched). ### After ``` ✓ |unit| packages/lexical-y/src/__tests__/unit/regressions/KeepEndsSessions.test.ts (4 tests) ✓ |unit| packages/lexical-y/src/__tests__/unit/regressions/KeepEndsUndoPlainWord.test.ts (2 tests) ``` The gate's three-peer view property, seeds 1 to 5,000, compared with main and with no collapse at all: 4,762 identical, 234 where this matches the document without collapse and main does not, 4 more that are closer to it than main, 0 where it is wrong and main is right (previous head: 1). The property test of this pull request, 4,000 more seeds in each id order, taking turns and concurrently: 0 failures (main: 116 concurrent failures per order). Fuzz golden corpus, 2,500 sessions: stored bytes identical to main.
…hange what the document shows, or empty its stack ## Description Yjs's `UndoManager` skips a step only when it changes no item. A step that restores or removes only format items no reader shows was taken alone, and the press showed nothing; the next press then undid what the first was meant to. On main that happens when a client formats text a collaborator then deletes (the bold's items stay around the deleted text, and undoing the bold removes only them). With the collapse keeping run ends (the previous commit), it also happened where the kept end was the undoer's own item: undoing a comment removal on text a collaborator deleted restored only the removal's kept null, and the underline before it needed another press (the gate's ON-prgate-3; in some sessions the press that did nothing was 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). DECISIONS.md D81 records it, with the rejected alternatives; 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 (d61302b): ``` × 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 ``` (the last fails on the previous commit, which keeps that end). `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) ✓ random sessions (ids 1,2) ✓ random sessions (ids 2,1) ``` with 200 sessions per id order.
There was a problem hiding this comment.
This looks good: keeping the end of each key's last run fixes #9609 and #9575, and making every undo press change what the document shows closes the gap that keeping those ends opens for undo. The new regression tests fail on main and pass here, and the #9575 case also holds when its two deletes come from different clients with garbage collection on. Everything below is optional and non-blocking.
1. Each undo or redo press now serializes the whole document twice (packages/lexical-y/src/internal/Binding.ts, lines 5233 and 5238)
visibleDigest(this._root) walks every item of every container, tombstones included, and JSON.stringifys each live item's content, before and after every step. Timing one typed character and its undo with the self-check off: 1,500 lines (142k characters) take 1.0 ms on main and 23.5 ms here; 3,000 lines (295k characters) 1.7 ms and 50 ms. A press that passes k invisible steps pays two digests per step it takes, 2(k+1). Two cheaper shapes: compute before once per press (it cannot change across steps that changed nothing), and digest only the containers that hold the stack item's items, which scanStackItem already walks.
2. YBinding lost its doc comment (packages/lexical-y/src/internal/Binding.ts, lines 624-642)
visibleDigest and contentDigest sit between YBinding's @experimental JSDoc and export class YBinding, so that block now documents nothing and the exported class ships without the @experimental tag. Moving the two helpers above the block restores it.
3. Describe the undo change in the pull request
AGENTS.md (Commits and Pull Requests) asks the Description to say what the change makes the code do and the Test plan to paste the real Before and After of the command that was run. The title and description cover only the collapse: the applyUndo change (D81, .changeset/lexical-y-undo-press-visible.md) and UndoPressProperty.test.ts, UndoInvisibleSteps.test.ts, KeepEndsSessions.test.ts and KeepEndsUndoPlainWord.test.ts are not mentioned, and the pasted After predates them. Since the pull request is squash-merged, that text becomes the commit message. The description also calls the rejected alternatives D79; DECISIONS.md records them as D80.
4. Keep y.md's open issues and D51 in step (packages/lexical-website/docs/collaboration/y.md, lines 1305-1307; packages/lexical-y/DECISIONS.md, line 112)
packages/lexical-y/AGENTS.md asks that accepted limits also be documented in y.md. The #9564 entry still says a comment spreads when "a collaborator's deletes there drop the item that ended it", which collapseGap no longer does: it always keeps a key's last run end (Cursor.ts, lines 1214-1216). If #9564 still reproduces through real deletes, a regression test would show how; otherwise the entry can go. D51 says "Replaced by D65" while staying an accepted limit whose first case still needs #9562; "Partly replaced" would say what is left.
5. Nit: every harness peer runs with gc: false (packages/lexical-y/src/__tests__/utils/harness.ts, line 234), while clients default to GC on and collapseGap reads a collected format item as content (Cursor.ts, lines 1036-1040). A GC-on variant of the two-deleter #9575 case would keep it covered.
| if ((undo ? um.undoStack : um.redoStack).length === 0) { | ||
| return any; | ||
| } | ||
| const before = visibleDigest(this._root); |
There was a problem hiding this comment.
Optional: this digests the whole document (every container, tombstones included, each live item's content stringified) before and after every step. Undoing one typed character went from 1.0 ms to 23.5 ms on a 142k-character document and from 1.7 ms to 50 ms at 295k. before can be computed once per press (it cannot change across steps that changed nothing), and the comparison can be limited to the containers that hold the stack item's items, which scanStackItem already walks.
| * or transaction. A handler that throws does not affect syncing. | ||
| */ | ||
|
|
||
| /** |
There was a problem hiding this comment.
Nit: these helpers now sit between YBinding's @experimental JSDoc (lines 624-634) and export class YBinding, so the exported class has no doc comment and the published declaration loses the @experimental tag. Moving visibleDigest and contentDigest above that block restores it.
…osed D86) ## Description Proposed for review: DECISIONS.md D86 is `OPEN` (the number may shift at landing). A delete collapsed the run of deleted content and format items it left (`Cursor#collapseGap`, D24's bound per format key): it kept a few items per key and deleted the rest. The ends it deleted were sometimes the ones a concurrent writer's restore, or an undo, relied on, and the value then ran on past its end: a comment over the following words and line (#9575), a comment's restore after a collaborator's plain text inside it (#9609), a comment its author undid (a #9620 case), the concurrent restore cases of #9640. Keeping the right ends inside the collapse (#9617) needed to know which write relies on which item, which the document does not store. This removes the collapse: a delete keeps every format item it leaves. Yjs's own format cleanup stays off (D24). `Cursor` loses `collapseGap` and the gap tracking it used (`gapStart`, `gapFormats`); comments that pointed at it now point at `Cursor#delete`. Cost: each formatted run (a format, a comment, a NodeState value) whose text is deleted keeps its two items, its opener and its closer. Plain typing and deleting add none, and neither does typing inside a formatted run. y.md gets a Limits entry (Deleted formatted text), and its Write rules paragraph and Undo paragraph lose the collapse's limits. Tests changed where they encoded the collapse, each with the reason in a comment: - `regressions/Formats.test.ts`, F24A-1, and `CursorFamily.test.ts`: words of a format, comment or set member typed and deleted again at one place leave exactly two items each (40, 80, 120 after 20, 40, 60 words) instead of a bound. - `Cursor.test.ts`: a format change then a delete on one cursor keeps all five items after the range. - `Races.test.ts`, #9575: the comment ends where it ended (was `test.fails`); the cell that pinned the spread "as documented" is gone. - `regressions/CollapseKeepsEnds.test.ts`, #9609 (12 cells), `regressions/KeepEndsSessions.test.ts` (4694), `regressions/UndoRestoredRunEnds.test.ts` (#9620, 2 cells) and `regressions/UndoRestoredRunEndExtents.test.ts` (the 10 concurrent empty restore cells): were `test.fails`, pass. - `CollapseKeepsEndsProperty.test.ts` is removed: it compared every session with a run of the same session with the collapse turned off, which is now the only run. ## Test plan Unit tests of lexical-y, lexical-mark, lexical, lexical-selection, lexical-list, lexical-table, lexical-html, lexical-extension and lexical-playground: 24,546 passed, 395 expected fail, 0 failed (`DisposeMemory.test.ts`'s heap bound failed once under a 16-worker run and passes alone and in reruns, here and on main). `pnpm run ci-check` passes. `MATRIX_FULL=1` matrix: see the body. Measured (the coordinator's runs and mine, same seeds, against main): - A three-client view fuzz, 15,000 seeds: 0 seeds where this is wrong and main is right; 631 where main spreads a value and this does not. - Growth (measured on 1853707 with the collapse turned off, the same rule): F24A-1 above; a comment added and its word deleted 3,000 times at one place: 6,002 live format items (main 6), 41 ms per edit (main 27); adding and removing a comment 1,000 times: 1 live item (as main); 1,000 keystrokes inside a commented run: none added. - Emily: x1.003 at 10 cycles, x1.26 at 150. Jot: x1.01 on its default profile; a concentrated recolor 438 items (main 345).
…osed D86) ## Description Proposed for review: DECISIONS.md D86 is `OPEN` (the number may shift at landing). A delete collapsed the run of deleted content and format items it left (`Cursor#collapseGap`, D24's bound per format key): it kept a few items per key and deleted the rest. The ends it deleted were sometimes the ones a concurrent writer's restore, or an undo, relied on, and the value then ran on past its end: a comment over the following words and line (#9575), a comment's restore after a collaborator's plain text inside it (#9609), a comment its author undid (a #9620 case), the concurrent restore cases of #9640. Keeping the right ends inside the collapse (#9617) needed to know which write relies on which item, which the document does not store. This removes the collapse: a delete keeps every format item it leaves. Yjs's own format cleanup stays off (D24). `Cursor` loses `collapseGap` and the gap tracking it used (`gapStart`, `gapFormats`); comments that pointed at it now point at `Cursor#delete`. Cost: each formatted run (a format, a comment, a NodeState value) whose text is deleted keeps its two items, its opener and its closer. Plain typing and deleting add none, and neither does typing inside a formatted run. y.md gets a Limits entry (Deleted formatted text), and its Write rules paragraph and Undo paragraph lose the collapse's limits. Tests changed where they encoded the collapse, each with the reason in a comment: - `regressions/Formats.test.ts`, F24A-1, and `CursorFamily.test.ts`: words of a format, comment or set member typed and deleted again at one place leave exactly two items each (40, 80, 120 after 20, 40, 60 words) instead of a bound. - `Cursor.test.ts`: a format change then a delete on one cursor keeps all five items after the range. - `Races.test.ts`, #9575: the comment ends where it ended (was `test.fails`); the cell that pinned the spread "as documented" is gone. - `regressions/CollapseKeepsEnds.test.ts`, #9609 (12 cells), `regressions/KeepEndsSessions.test.ts` (4694), `regressions/UndoRestoredRunEnds.test.ts` (#9620, 2 cells) and `regressions/UndoRestoredRunEndExtents.test.ts` (the 10 concurrent empty restore cells): were `test.fails`, pass. - `CollapseKeepsEndsProperty.test.ts` is removed: it compared every session with a run of the same session with the collapse turned off, which is now the only run. ## Test plan Unit tests of lexical-y, lexical-mark, lexical, lexical-selection, lexical-list, lexical-table, lexical-html, lexical-extension and lexical-playground: 24,546 passed, 395 expected fail, 0 failed (`DisposeMemory.test.ts`'s heap bound failed once under a 16-worker run and passes alone and in reruns, here and on main). `pnpm run ci-check` passes. `MATRIX_FULL=1` matrix: see the body. Measured (the coordinator's runs and mine, same seeds, against main): - A three-client view fuzz, 15,000 seeds: 0 seeds where this is wrong and main is right; 631 where main spreads a value and this does not. - Growth (measured on 1853707 with the collapse turned off, the same rule): F24A-1 above; a comment added and its word deleted 3,000 times at one place: 6,002 live format items (main 6), 41 ms per edit (main 27); adding and removing a comment 1,000 times: 1 live item (as main); 1,000 keystrokes inside a commented run: none added. - Emily: x1.003 at 10 cycles, x1.26 at 150. Jot: x1.01 on its default profile; a concentrated recolor 438 items (main 345).
…osed D86) ## Description Proposed for review: DECISIONS.md D86 is `OPEN` (the number may shift at landing). A delete collapsed the run of deleted content and format items it left (`Cursor#collapseGap`, D24's bound per format key): it kept a few items per key and deleted the rest. The ends it deleted were sometimes the ones a concurrent writer's restore, or an undo, relied on, and the value then ran on past its end: a comment over the following words and line (#9575), a comment's restore after a collaborator's plain text inside it (#9609), a comment its author undid (a #9620 case), the concurrent restore cases of #9640. Keeping the right ends inside the collapse (#9617) needed to know which write relies on which item, which the document does not store. This removes the collapse: a delete keeps every format item it leaves. Yjs's own format cleanup stays off (D24). `Cursor` loses `collapseGap` and the gap tracking it used (`gapStart`, `gapFormats`); comments that pointed at it now point at `Cursor#delete`. Cost: each formatted run (a format, a comment, a NodeState value) whose text is deleted keeps its two items, its opener and its closer. Plain typing and deleting add none, and neither does typing inside a formatted run. y.md gets a Limits entry (Deleted formatted text), and its Write rules paragraph and Undo paragraph lose the collapse's limits. Tests changed where they encoded the collapse, each with the reason in a comment: - `regressions/Formats.test.ts`, F24A-1, and `CursorFamily.test.ts`: words of a format, comment or set member typed and deleted again at one place leave exactly two items each (40, 80, 120 after 20, 40, 60 words) instead of a bound. - `Cursor.test.ts`: a format change then a delete on one cursor keeps all five items after the range. - `Races.test.ts`, #9575: the comment ends where it ended (was `test.fails`); the cell that pinned the spread "as documented" is gone. - `regressions/CollapseKeepsEnds.test.ts`, #9609 (12 cells), `regressions/KeepEndsSessions.test.ts` (4694), `regressions/UndoRestoredRunEnds.test.ts` (#9620, 2 cells) and `regressions/UndoRestoredRunEndExtents.test.ts` (the 10 concurrent empty restore cells): were `test.fails`, pass. - `CollapseKeepsEndsProperty.test.ts` is removed: it compared every session with a run of the same session with the collapse turned off, which is now the only run. ## Test plan Unit tests of lexical-y, lexical-mark, lexical, lexical-selection, lexical-list, lexical-table, lexical-html, lexical-extension and lexical-playground: 24,546 passed, 395 expected fail, 0 failed (`DisposeMemory.test.ts`'s heap bound failed once under a 16-worker run and passes alone and in reruns, here and on main). `pnpm run ci-check` passes. `MATRIX_FULL=1` matrix: see the body. Measured (the coordinator's runs and mine, same seeds, against main): - A three-client view fuzz, 15,000 seeds: 0 seeds where this is wrong and main is right; 631 where main spreads a value and this does not. - Growth (measured on 1853707 with the collapse turned off, the same rule): F24A-1 above; a comment added and its word deleted 3,000 times at one place: 6,002 live format items (main 6), 41 ms per edit (main 27); adding and removing a comment 1,000 times: 1 live item (as main); 1,000 keystrokes inside a commented run: none added. - Emily: x1.003 at 10 cycles, x1.26 at 150. Jot: x1.01 on its default profile; a concentrated recolor 438 items (main 345).
Parked (10/30)
This PR is parked as a draft. The branch and its tests stay as they are; #9609 stays open. The tests move to main in #9631, with the cases main fails as expected failures; the undo-press change (aa2e198) is #9630, a draft of its own.
Why. Keeping each key's run end in
Cursor#collapseGapfixes #9609 broadly: in a three-peer randomized view property (15,000 seeds), 673 to 678 sessions where main lets a comment run on over the rest of the document come out right. But every version of "which end to keep" changed what other code does with the extra live item, and a few sessions came out wrong where main is right:aa2e198 (an undo or redo press always changes what the document shows, D81) fixes the press that shows nothing, including main's own cases (undoing a format on text a collaborator deleted); it does not depend on keeping ends.
The problem. A kept end is a live item. Later collapses' rules (the first change, the last two units, a kept place's items, the value after the run), the cursor's position for an insert among deleted items, and what an undo restores all read it, so keeping it changes decisions that have nothing to do with the run it ends. Any rule that keeps an item main deletes moves these errors around rather than removing them.
Also tried: skip the collapse. Where the collapse would remove a run's end, leave that stretch as written (b13a570f on zurfyx/y-skip-collapse-ends, not pushed). Growth is unbounded with typing: five comment ids used in turn, a word typed and deleted at one place each time, leave 40 / 80 / 120 live format items after 20 / 40 / 60 words (main 9). With the undo change on all trees, the view property on 15,000 seeds still has 6 seeds wrong where main and a document without any collapse are right (keep ends: 6; main: 536 seeds where a comment spreads, no collapse at all: 8).
Description
When a delete lands next to text deleted earlier,
Cursor#collapseGapcollapses the format items left in that run of deleted content. It kept a bounded number per family (the units around the delete, the first change, the last two units, the items a kept place needs, the value after the run) and deleted the rest. A collaborator who, before seeing the delete, typed text without a run's value inside the run (uncommented text inside a commented word) restored the value after their text and relied on the run's end to end it again. When the collapse deleted that end, the restored value ran on over every later word and line. Comments hit this because each comment id is its own key: a family's last units are other comments' items, so nothing ended the restore (#9609; also #9575, where the collapse kept the copies of an end that rule 5 writes and dropped the end itself, and the copies' writers then undid their writes).Now the collapse also keeps, of each key, the item that ends the key's last run in the stretch: an item that changes a value that is not null (a closer, a comment's or link's end, or a style's end that writes the text's own style again), or a null after content that had no value of the key (the end of a run whose opener an earlier collapse removed). A null right after another one with no content between them is a copy of the same end (rule 5) and still goes, so the run's own end is the one kept. Where nothing kept after that end gives the key its value after the run, the key's last item is kept too, so the content after the run keeps its value. Starts and the other redundant items still go: a start kept without its end would spread.
Keeping an end changes nothing a reader shows: the run holds no live content, ending a value that is not open there is a no-op, and the value after the run stays. One end per key is enough: a restore before an earlier end is ended by the last one at the latest, so it can reach only text inserted into the run at the same time, never past it. The bound changes from a few items per family to one more item per key at a place; with comments (one key per id) that is one item per comment deleted at a place, and nothing per keystroke. The two growth tests now count that from a derivation in their comments.
The decision and the rejected alternatives (a provenance marker; paired anchors / Peritext-style storage for comments, which adds another storage model on top of formats) are D65 and D79 in DECISIONS.md. y.md's Limits drop the #9575 sentence and say what a later delete keeps and what that costs.
The
test.failscases inOwnRunRestoreEnd.test.tsstay expected failures: they delete the run's end with raw Yjs deletes, which no write of the binding makes now, and nothing else ends the restore once its end is gone. The same case with a real collapse passes.Closes #9609
Changeset
.changeset/lexical-y-collapse-keeps-run-ends.md:@lexical/ypatch.Test plan
regressions/CollapseKeepsEnds.test.ts: Bug: [lexical-y] A comment's restore inside a word its own client commented runs on over the rest of the document when a collaborator's delete drops the comment's end #9609 with a real collapse (a delete of the words around the commented word, then of the space after them), the end kept through a later collapse of the same run, 1/16/64/128 comment ids on the word, a kept end that must not end the comment before a later run of the same id (no peer writes for it), and bold and style controls.CollapseKeepsEndsProperty.test.ts: random documents (comments with repeating ids, formats, styles, links), typing inside a word while the other peer deletes around it and next to it, compared character by character with the same session with no collapse at all: taking turns, everything matches; at the same time, the document's own text matches; no peer writes more.Races.test.ts: Bug: [lexical-y] A comment spreads onto later words when two collaborators undo text they typed inside it while a third deletes past its end #9575 flipped to a regular test.CursorFamily.test.ts,regressions/Formats.test.tsF24A-1: growth counts from a derivation.pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/regressions/CollapseKeepsEnds.test.ts packages/lexical-y/src/__tests__/unit/CollapseKeepsEndsProperty.test.ts packages/lexical-y/src/__tests__/unit/Races.test.tsBefore
main d61302b with the new tests:
After
(the three files above plus
CursorFamily,regressions/FormatsandOwnRunRestoreEnd).pnpm exec vitest run --project unit packages/lexical-y packages/lexical-mark: 219 files, 12856 passed, 299 expected fail.pnpm run ci-check: passed.