Skip to content

[lexical-y] Bug Fix: A delete keeps every format item it leaves (D86) - #9678

Closed
zurfyx wants to merge 4 commits into
mainfrom
zurfyx/y-no-collapse
Closed

zurfyx wants to merge 4 commits into
mainfrom
zurfyx/y-no-collapse

Conversation

@zurfyx

@zurfyx zurfyx commented Oct 31, 2026 •

Copy link
Copy Markdown
Member

Description

DECISIONS.md D86, DECIDED (10-31). Lands together with #9679 (stacked on this one): on its own, this PR is worse than main on one gc-on downstream undo-fuzz session (below), which #9679 fixes.

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, which now points at D86). Cursor
loses collapseGap and the gap tracking it used (gapStart, gapFormats).

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. D51 (the #9575
spread) is marked replaced by D86; D65 is updated.

Tests changed where they encoded the collapse, each with the reason in a
comment:

Test plan

Landing tree: main cfcd3c4 + this PR (code identical from 8c57ef8 to
8ed68ce: later commits change docs, tests and one comment). Every
property, fuzz and soak lane ran with Yjs garbage collection on (what
products use) and off.

Before (main cfcd3c4)

  • Three-client view property test, 15,000 seeds, multi-value: 492 bad seeds
    with gc off, 511 with gc on.
  • Two-client undo-spread fuzz, 3,000 seeds: 221 spreading with gc on, 0 off.
  • pnpm run ci-check passes; units of every package: 0 failures with gc
    off; with gc on 15 Formats tests fail (gc-only, on main too); MATRIX_FULL:
    11 legacy-binding failures.

After (this PR)

  • View property test: 4 bad seeds gc off, 14 gc on; 0 seeds wrong where
    main is right
    in either; 601 (gc off) and 608 (gc on) main-wrong and
    this right.
  • Undo-spread fuzz 221 / 0 (same seeds), run-sequence fuzz 8 / 8 (same
    seeds), comment-run fuzz 0 / 0.
  • pnpm run ci-check passes. Units, all packages, gc off and gc on: no
    failure that main does not have (the same 15 gc-on Formats tests).
    MATRIX_FULL: the same 11.
  • Soak (L1B, L2 on identical op paths, gc off / on): 13,918 / 14,051
    sessions, 0 failing only here, 4 / 4 failing only on main. L1C (IME):
    10,988 sessions, 0 either way. L3 (hour-long sessions, reloads, offline
    gaps, clients with gc on and off in each session): 269 sessions, 6
    flagged only here and 13 only on main, in the same classes both ways.
  • A downstream document library (lib, two apps, its fuzz spike and settle
    probes) on preview builds: identical to main. Its corpus of 189 documents
    after 10 / 40 / 150 comment cycles: format items x1.003 / x1.049 / x1.26,
    bytes x1.000 / x1.012 / x1.051.
  • Jot (typecheck, lint, format, unit, e2e 146, fuzz 6,000 + 2,000):
    identical to main. Its undo-spread fuzz (gc on): plain 351 vs main 350:
    the extra seed (2810) spreads here and not on main with gc on, passes with
    gc off on both, and passes with [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, which adds it as a test
    (regressions/UndoSpreadSession2810.test.ts). Macro profile 433 = 433.
  • Reviews: an independent review (gpt-6.1-sol) is clean on 8ed68ce.

…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).
…to the collapse

## Description

KeepEndsSessions' 3645 cell spied on the removed `Cursor#collapseGap` inside `test.fails`, so it passed by throwing before its comparison. It now asserts what the session shows: c9 on every character of "wxq4xq56abcd". Test comments that pointed at `Cursor#collapseGap` now point at `Cursor#delete` or say the collapse is gone (D86); no test references `collapseGap`, `gapStart` or `gapFormats` any more.

## Test plan

KeepEndsSessions passes (3645 as a plain test). Units of lexical-y, lexical-mark, lexical, lexical-selection, lexical-list, lexical-table, lexical-html, lexical-extension and lexical-playground: 24,547 passed, 394 expected fail, 0 failed. `pnpm run ci-check` passes.
@vercel

vercel Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
lexical Ready Ready Preview Oct 31, 2026 10:05pm UTC
lexical-playground Ready Ready Preview Oct 31, 2026 10:05pm UTC

Request Review

@zurfyx
zurfyx requested a review from etrepum as a code owner October 31, 2026 21:19
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 31, 2026
@meta-code-review

meta-code-review Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Oct 31, 2026, 10:20 PM UTC

@pkg-pr-new

pkg-pr-new Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
lexical

npm i https://pkg.pr.new/lexical@9678

@lexical/a11y

npm i https://pkg.pr.new/@lexical/a11y@9678

@lexical/clipboard

npm i https://pkg.pr.new/@lexical/clipboard@9678

@lexical/code

npm i https://pkg.pr.new/@lexical/code@9678

@lexical/code-core

npm i https://pkg.pr.new/@lexical/code-core@9678

@lexical/code-prism

npm i https://pkg.pr.new/@lexical/code-prism@9678

@lexical/code-shiki

npm i https://pkg.pr.new/@lexical/code-shiki@9678

@lexical/compiler

npm i https://pkg.pr.new/@lexical/compiler@9678

@lexical/devtools-core

npm i https://pkg.pr.new/@lexical/devtools-core@9678

@lexical/dragon

npm i https://pkg.pr.new/@lexical/dragon@9678

@lexical/eslint-plugin

npm i https://pkg.pr.new/@lexical/eslint-plugin@9678

@lexical/extension

npm i https://pkg.pr.new/@lexical/extension@9678

@lexical/fast-check

npm i https://pkg.pr.new/@lexical/fast-check@9678

@lexical/file

npm i https://pkg.pr.new/@lexical/file@9678

@lexical/hashtag

npm i https://pkg.pr.new/@lexical/hashtag@9678

@lexical/headless

npm i https://pkg.pr.new/@lexical/headless@9678

@lexical/history

npm i https://pkg.pr.new/@lexical/history@9678

@lexical/html

npm i https://pkg.pr.new/@lexical/html@9678

@lexical/internal

npm i https://pkg.pr.new/@lexical/internal@9678

@lexical/link

npm i https://pkg.pr.new/@lexical/link@9678

@lexical/list

npm i https://pkg.pr.new/@lexical/list@9678

@lexical/mark

npm i https://pkg.pr.new/@lexical/mark@9678

@lexical/markdown

npm i https://pkg.pr.new/@lexical/markdown@9678

@lexical/mdast

npm i https://pkg.pr.new/@lexical/mdast@9678

@lexical/offset

npm i https://pkg.pr.new/@lexical/offset@9678

@lexical/overflow

npm i https://pkg.pr.new/@lexical/overflow@9678

@lexical/plain-text

npm i https://pkg.pr.new/@lexical/plain-text@9678

@lexical/react

npm i https://pkg.pr.new/@lexical/react@9678

@lexical/rich-text

npm i https://pkg.pr.new/@lexical/rich-text@9678

@lexical/selection

npm i https://pkg.pr.new/@lexical/selection@9678

@lexical/table

npm i https://pkg.pr.new/@lexical/table@9678

@lexical/tailwind

npm i https://pkg.pr.new/@lexical/tailwind@9678

@lexical/text

npm i https://pkg.pr.new/@lexical/text@9678

@lexical/utils

npm i https://pkg.pr.new/@lexical/utils@9678

@lexical/y

npm i https://pkg.pr.new/@lexical/y@9678

@lexical/yjs

npm i https://pkg.pr.new/@lexical/yjs@9678

commit: 8ed68ce

@meta-code-review meta-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Removing Cursor#collapseGap makes a delete delete only text. That is always a valid Yjs operation, and it removes the class of spreads where a comment or format lost the end a concurrent restore or an undo relied on. The regression tests that now pass (#9575, #9609, #9620, the KeepEndsSessions seeds and the concurrent restore cells) fail against main's implementation in both client id orders, and they assert converged peers and comment extents rather than only checking that nothing throws. The storage trade-off is recorded as D86 and documented in y.md's Limits.

Optional, non-blocking suggestions:

  1. Decision register. packages/lexical-y/DECISIONS.md:58 (D24) still says a delete keeps the ends of deleted runs "within a bound per format key", and packages/lexical-y/src/internal/Cursor.ts:848 cites D24 for the new rule. The register's preamble asks a pull request that changes a recorded decision to update its row ("a replaced decision says what replaced it"). Consider pointing D24 at D86, citing D86 in the JSDoc, and updating D65 (:131). D65's reasoning relies on the growth bound this removes, and its #9609 cells in OwnRunRestoreEnd.test.ts (:209, :390, :567) are still test.fails.
  2. Test plan. AGENTS.md (Commits and Pull Requests) asks for ### Before and ### After with the command and its pasted output. The post-rebase gate results are still "to be added", and the 15,000-seed three-client comparison and the growth and timing figures are neither committed nor pasted, so nobody else can rerun them.
  3. Stale wording. regressions/Formats.test.ts:729 is still titled "leave a bounded number of format items" but now asserts linear growth. Present-tense descriptions of the collapse remain in UndoRunEnds.test.ts:648 and :677, OwnCommentGapCollapse.test.ts:60 and regressions/TypedTextAfterDeletedSpace.test.ts:10 (title at :54).
  4. Coverage. The deleted CollapseKeepsEndsProperty.test.ts also checked that both peers and a fresh reader agree over random typing ‖ delete sessions. That check never depended on the collapse, and keeping it as a convergence property would preserve the coverage.
  5. Changeset. One sentence saying that documents now grow with each deleted formatted run would help people upgrading (CONTRIBUTING.md: write the entry for someone who uses Lexical).

* items without live content that the delete leaves (see collapseGap).
* Delete `n` content units. The format items around and inside them stay:
* a concurrent writer that has not seen the delete relies on them, and an
* undo of the delete brings the content back between them (D24).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: this rule is D86. D24's row still says a delete keeps the ends "within a bound per format key", so citing D86 here (and adding "replaced by D86" to D24, as the register's preamble asks) would keep the two in step.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8ed68ce: Cursor#delete now cites D24 and D86, and D24 (since d318d9a) says D86 removed its bound.

Comment thread packages/lexical-y/DECISIONS.md Outdated
| D76 | **A code-token-aware trim for remade text.** A stale replace of a word can take the first letter of the word a rewrite made from it, where a node boundary cut the old word. | Writing a changed word whole fixes it, but also rewrites highlighter tokens glued to a pasted-over token in a code line. A follow-up would apply the rule everywhere except at code highlight token boundaries (D29). | `OPEN` | 10-30 | [#9608] |
| D77 | **A stale comment on a word a collaborator rewrites** lands on the new word for one client id order only. | Likely the documented race where text typed at the start of a range a collaborator formats takes that format, with a comment instead of a format. Either the docs name comments and whole-word replacements, or a fix makes both orders agree. | `OPEN` | 10-30 | [#9610], [limits] |
| D78 | **A format spreading after one client's triple undo:** a client formats a run, types plain text, formats again and undoes all three while a collaborator types plain text inside the first run. | Reproduced on main in both client id orders; no design chosen. | `OPEN` | 10-30 | [#9599] |
| D86 | **A delete keeps every format item it leaves (no collapse).** The collapse of the run of deleted content and format items a delete leaves (`Cursor#collapseGap`, D24's bound per format key) is removed; D24's cleanup-off stays. | The collapse dropped ends that a concurrent writer's restore or an undo relied on, and the value ran on: #9575, #9609, a #9620 cell, the concurrent restore cells of #9640, a KeepEndsSessions seed. Every attempt to keep the right ends in the collapse (#9617) needed to know which write relies on which item. Measured against main, same seeds: a three-client view fuzz of 15,000 seeds: 0 seeds wrong where main is right, 631 where main spreads a value and this does not. Cost: each formatted run whose text is deleted keeps its two items, none per keystroke or plain delete (F24A-1: 40, 80, 120 after 20, 40, 60 words typed and deleted at one place); a comment added and its word deleted 3,000 times at one place: 6,002 live format items (main 6); Emily x1.003 at 10 cycles, x1.26 at 150; Jot x1.01 on its default profile, a concentrated recolor 438 items (main 345). Accepted: the linear growth with deleted formatted text, over the bound and its spreads. | `DECIDED` | 10-31 | [#9575], [#9609] |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: the preamble asks a pull request that changes a recorded decision to update its row. D24 (line 58) still describes the bound this removes, and D65 (line 131) still reasons from "growth bounds" and points at #9609, part of which this fixes. Its OwnRunRestoreEnd.test.ts #9609 cells are still test.fails.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D24 points at D86 (d318d9a). D65 is updated in 8ed68ce: it no longer argues from the growth bound, and says the #9609 cells where a collaborator's delete took the run's end pass while the OwnRunRestoreEnd.test.ts cells (a restore whose run has no end of its own) still fail, which is why those stay test.fails. D51 (the #9575 spread) is marked replaced by D86.

## Description

D86 (a delete keeps every format item it leaves) moves from `OPEN` to `DECIDED` (10-31): the linear growth with deleted formatted text is accepted over the bound and its spreads. The row moves from Open to Storage model, next to D24, whose bound per format key it removes; D24 says so.

## Test plan

Docs only: DECISIONS.md rows D24 and D86; `prettier --check` passes.

@meta-code-review meta-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Taking Cursor#collapseGap out of @lexical/y's delete path fixes the run-on cases at their source, and the cost is written down where people will look for it: D86, and the new "Deleted formatted text" entry in y.md's Limits. Every cell that moves from test.fails to test checks that the peers converge and then checks the exact words with their comment ids. Those cells fail against the previous cursor, so they really tell the two behaviors apart. Deferring #9623 (the gc-only class) to the stacked follow-up makes sense.

A few optional, non-blocking suggestions:

  1. Refresh DECISIONS.md D51 and D65 (packages/lexical-y/DECISIONS.md:112 and :132). D51 still lists the #9575 spread as an accepted limit "documented in Limits", but this PR removes that Limits paragraph and the #9575 cells now pass. D65 still argues that keeping the last nulls "breaks growth bounds", and D86 removes those bounds. packages/lexical-y/AGENTS.md (Decisions) asks a pull request that changes a recorded decision to update its row in the same pull request, and DECISIONS.md asks a replaced decision to say what replaced it. D65 can keep tracking the raw-delete #9609 cells in OwnRunRestoreEnd.test.ts.
  2. Keep a randomized convergence check. CollapseKeepsEndsProperty.test.ts was the only randomized session test where one peer types inside a commented word offline while another deletes around it. Its no-collapse comparison is moot now, but its checks still apply: no errors, and every peer and a fresh reader agree. packages/lexical-y/AGENTS.md (Testing) asks to test the input space; keeping run() with just those assertions would preserve that coverage.
  3. Stale wording in tests. Some test names and comments still describe the bound:
    • regressions/Formats.test.ts:729 is still named "leave a bounded number of format items" but now asserts [40, 80, 120].
    • regressions/CollapseKeepsEnds.test.ts:152-155 still talks about "a later collapse of the same run".
    • Comments at OwnRunRestoreEnd.test.ts:254 and :293, OwnCommentGapCollapse.test.ts:60, UndoRunEnds.test.ts:648 and :677, and regressions/TypedTextAfterDeletedSpace.test.ts:10 and :54.
  4. D86 measurement names (DECISIONS.md:59). The row cites one measurement corpus only by a name the register doesn't define. The PR body's wording ("a downstream document library's corpus") would make it readable.
  5. Changeset. The Verify: paragraph is good. One more sentence pointing to y.md's "Deleted formatted text" Limits entry would tell upgrading apps that stored documents now grow with deleted formatted text.
  6. Test plan. AGENTS.md (Commits and Pull Requests) asks for the real command output under ### Before and ### After. The rebased gate results still read "will be added".

A heads-up, not a request: the 3645 cell now asserts [c9] directly. The comment it replaces noted that #9630's undo-press change removes c9 in that session, so that draft will need to revisit the cell.

// As above, and B then deletes "cc " too: the first collapse removed
// the comment's opener and kept its end, which the second collapse
// must keep although, for B, nothing opens the comment before it.
test(`#9609: the comment's end stays through a later collapse of the same run (ids ${ids})`, async () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: this test name and the comment under it still describe "a later collapse of the same run", which no longer happens after D86. Something like "the comment's end stays through a later delete next to the same run" would match the new behavior.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8ed68ce: renamed to "the comment's end stays through a later delete next to the same run", with the comment saying the collapse is gone (D86).

Comment thread packages/lexical-y/DECISIONS.md Outdated
| D23 | **Garbage collection stays on**, as Yjs defaults; headless results are compacted as clients store them. Only temporary documents turn it off. Turning it off for history is an app's choice, on every client and server. | A server that skips compaction while clients compact is a trap, and undo and insert attribution work with GC on. | `DECIDED` | 10-23 | [#9437], [y-server.md][y-server] |
| D24 | **Yjs's format cleanup is off** in `@lexical/y` containers and on every relay that loads a state (`createYDoc`). A delete keeps the ends of each deleted formatted run, within a bound per format key. Not reported upstream. | The cleanup deletes format items a concurrent writer may still rely on. Upstream already knows that skipping it is the app's trade-off. | `DECIDED` | 10-28 | [#9400], [y.md: Servers][y-servers] |
| D24 | **Yjs's format cleanup is off** in `@lexical/y` containers and on every relay that loads a state (`createYDoc`). A delete keeps the ends of each deleted formatted run (within a bound per format key until D86 removed the bound: a delete keeps every format item it leaves). Not reported upstream. | The cleanup deletes format items a concurrent writer may still rely on. Upstream already knows that skipping it is the app's trade-off. | `DECIDED` | 10-28 | [#9400], [y.md: Servers][y-servers] |
| D86 | **A delete keeps every format item it leaves (no collapse).** The collapse of the run of deleted content and format items a delete leaves (`Cursor#collapseGap`, D24's bound per format key) is removed; D24's cleanup-off stays. | The collapse dropped ends that a concurrent writer's restore or an undo relied on, and the value ran on: #9575, #9609, a #9620 cell, the concurrent restore cells of #9640, a KeepEndsSessions seed. Every attempt to keep the right ends in the collapse (#9617) needed to know which write relies on which item. Measured against main, same seeds: a three-client view fuzz of 15,000 seeds: 0 seeds wrong where main is right, 631 where main spreads a value and this does not. Cost: each formatted run whose text is deleted keeps its two items, none per keystroke or plain delete (F24A-1: 40, 80, 120 after 20, 40, 60 words typed and deleted at one place); a comment added and its word deleted 3,000 times at one place: 6,002 live format items (main 6); Emily x1.003 at 10 cycles, x1.26 at 150; Jot x1.01 on its default profile, a concentrated recolor 438 items (main 345). Accepted: the linear growth with deleted formatted text, over the bound and its spreads. | `DECIDED` | 10-31 | [#9575], [#9609] |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: one of the measurement corpora in this row is cited only by a name the register doesn't define. The PR body's description ("a downstream document library's corpus") would make the row readable for others.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8ed68ce: the row now says "a downstream document library's corpus of 189 documents".

…keep the delete-next-to-typing convergence property

## Description

Review follow-ups for D86:

- DECISIONS.md: D51 (the #9575 spread) is replaced by D86, whose cells pass; D65 no longer argues from the growth bound D86 removed and says which #9609 cells still fail; D86 names its corpus measurement without an internal name. `Cursor#delete` cites D86.
- `DeleteNextToTyping.test.ts` keeps the randomized sessions of the removed `CollapseKeepsEndsProperty.test.ts` (one peer types inside a commented word while the other deletes around it, taking turns and at the same time) with the checks that never depended on the collapse: no errors, every peer and a fresh reader agree.
- Test names and comments that described the collapse in the present tense now say it is gone (D86).
- The changeset says documents keep two format items per formatted run whose text is deleted.

## Test plan

`DeleteNextToTyping.test.ts`: 4 passed (60 sessions typing ‖ deleting and 20 taking turns per client id order). The renamed tests pass. `prettier --check`, `eslint --no-cache` on the changed files and the changeset check pass.

@meta-code-review meta-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Taking Cursor#collapseGap out of @lexical/y's delete path means a delete removes only text. That fixes the cases where a comment or format lost the end a concurrent restore or an undo relied on and ran on over the following words and lines (#9575, #9609, #9620). The cells that move from test.fails to test check that the peers converge and then check the exact words and comment ids, and they fail against the previous cursor. The storage cost is written down where people will look for it: D86, and the new "Deleted formatted text" entry in y.md's Limits.

A few optional, non-blocking suggestions:

  1. #9564 in y.md's Open issues (packages/lexical-website/docs/collaboration/y.md:1349-1351). The bullet still says a comment spreads "while a collaborator's deletes there drop the item that ended it". After this PR, a delete never drops a format item. The OwnRunRestoreEnd.test.ts cells that stay test.fails reproduce the spread only by removing the comment's opener and end with raw Yjs deletes (dropCommentItems). That section says each listed issue "has a test that reproduces it", so rewording the bullet to the write that can still drop that end (or narrowing it if none remains) would keep it accurate. #9564 and #9609 describe the same mechanism.
  2. Edit cost in the new Limits entry (y.md:1099-1106). It explains the growth. One clause saying that edits at a place where formatted text was deleted many times also get slower would set expectations; the PR's own figure after 3,000 cycles is 41 ms against 27 ms per edit.
  3. Test plan. AGENTS.md (Commits and Pull Requests) asks for the command and its real output under ### Before and ### After. The rebased gate results still read "will be added here".
  4. Nits. The describe title at packages/lexical-y/src/__tests__/unit/regressions/CollapseKeepsEnds.test.ts:117 still says "a delete collapsing a run". A few new comment lines run past 80 columns, which Prettier does not rewrap (packages/lexical-y/src/internal/Cursor.ts:50, packages/lexical-y/src/internal/Write.ts:2459).

Comment on lines +1099 to +1106
- **Deleted formatted text.** A delete keeps every format item it leaves,
as a collaborator's concurrent edit there and an undo of the delete rely
on them: each formatted run (a format, a comment, a NodeState value)
whose text is deleted leaves its two items (its opener and its closer).
Plain typing and deleting add none, and neither does typing
inside a formatted run. So a document grows with the formatted text
deleted from it, not only with its content: most in a place where
formatted text is typed and deleted over and over.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: this explains the growth well. One clause saying that edits at a place where formatted text was deleted many times also get slower (the PR's 3,000-cycle figure is 41 ms against 27 ms per edit) would tell apps with such a spot what to expect.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #9679 (2b39655), which lands in the same batch as this PR: the Deleted formatted text limit now says edits at such a place get slower, with the 41 ms vs 27 ms figure. y.md's #9564 entry is reworded there too.

* Y.Node#applyDelta), but the items differ: a delete keeps every format item
* in the run of deleted content and format items it leaves, which Yjs removes
* as redundant and concurrent writers rely on (see delete), and a format
* change keeps the items that bound the content around its range, opens its value next to its content and closes it where

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this comment line is 122 columns, and Prettier does not rewrap comments. The same applies to Write.ts:2459.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewrapped in #9679 (2b39655), together with the Write.ts comment; both PRs land in one batch.

zurfyx added a commit that referenced this pull request Oct 31, 2026
…llection while they are undoable

## Description

With Yjs's default garbage collection, which products use, an undo could
not read the format items of its own change after a collaborator deleted
them: they were collected, so the undo guards (`isKeptByUndo`, `changeEnd`,
`closeRestoredRuns`) took them for other content, and the value an undo
brought back ran on over the rest of the document (#9623). The unit harness
built every peer with gc off, so most tests never saw it.

- `Binding`: the binding sets Yjs's own `Item.keep` on the format items its
  transactions and its undo manager's insert, and on the end of each run
  they delete. It releases them when both undo stacks are empty (after a
  clear, an undo that leaves nothing, or a write they did not capture) and
  when it is disposed, and then collects the ones a remote update deleted
  meanwhile (Yjs's `tryGc`, as a transaction collects its own deletes); one
  deleted locally is left as Yjs leaves it, since an `UndoManager` step on
  the same document may restore it. An item another binding or an
  `UndoManager` step also keeps stays kept. Nothing new is written; while an
  item is kept, this client's state still holds its deleted content, as for
  the items an `UndoManager` step deletes. `YjsContract` checks `Item#keep`,
  `tryGc` and the `Doc` members used.
- `Cursor#runOpener`: a restore's scan stops at content of another client,
  so a client's earlier end and later opener with a collaborator's text
  between them are not one restored run (#9599).
- `Cursor` rule 3: a range end that writes its own copy of another client's
  opener also ends it where that opener's run ended, so undoing that
  client's change leaves the text after the range alone.
- `Cursor`: a format write removes the null and the restore that this
  client's own earlier insert left inside its range, as Yjs's `formatText`
  does, so a collaborator's typing no longer relies on items the insert's
  undo deletes (#9671).
- Tests: `regressions/OwnFormatItems.test.ts` runs the #9623, #9599 and
  #9671 repros and a restated-opener case, gc on and off, every client id
  order. `regressions/OwnFormatItemsRelease.test.ts` checks the release:
  clears, an undo that empties both stacks, writes the stacks do not
  capture, dispose, a new binding on the same document, another binding's
  undo after this one releases, repeated cycles, and what a relay stores.
  `regressions/UndoSpreadSession2810.test.ts` replays a downstream app's
  undo-fuzz session where, with gc on, the parent commit put a comment on
  four paragraphs and main did not. The harness
  takes a `gc` option and `LEXICAL_Y_GC=1` runs every harness test with gc
  on. Pins that now pass are plain tests: `UndoRestoredRunEndExtents` (117
  cells), `UndoRestoredRunEnds` (20; the D64 cells stay pinned),
  `UndoSpreadFuzz` (5 seeds; its documents use gc on),
  `UndoCommentRemovalSpread` (4), `RunSequenceUndo` seed 87.
- DECISIONS.md: D88 (own format items kept from gc while undoable), D78
  decided (the #9599 class fixed).
- Docs for D86 (#9678 review follow-ups): y.md's #9564 entry no longer
  says a delete drops the item that ended the comment, and the Deleted
  formatted text limit says edits at such a place get slower; two comments
  rewrapped and a test title updated.

## Test plan

`OwnFormatItems.test.ts`: 72 passed; on the parent commit 44 fail (the
issue cells with gc on for #9623; #9599, #9671 and the restated-opener
color cells with gc on and off). `OwnFormatItemsRelease.test.ts`: 14 passed;
5 fail with the keep and without the release paths, and 4 more without
the remote-only collection and the uncaptured-write release.
`UndoSpreadSession2810.test.ts`: 4 passed; on the parent commit the 2 gc-on
cells fail (main passes all 4). The pins above flip
from expected-fail to pass and nothing else changes in the lexical-y unit
suite.
zurfyx added a commit that referenced this pull request Oct 31, 2026
…llection while they are undoable

## Description

With Yjs's default garbage collection, which products use, an undo could
not read the format items of its own change after a collaborator deleted
them: they were collected, so the undo guards (`isKeptByUndo`, `changeEnd`,
`closeRestoredRuns`) took them for other content, and the value an undo
brought back ran on over the rest of the document (#9623). The unit harness
built every peer with gc off, so most tests never saw it.

- `Binding`: the binding sets Yjs's own `Item.keep` on the format items its
  transactions and its undo manager's insert, and on the end of each run
  they delete. It releases them when both undo stacks are empty (after a
  clear, an undo that leaves nothing, or a write they did not capture) and
  when it is disposed. An item another binding or an `UndoManager` step
  also keeps stays kept. Releasing collects nothing, as the `UndoManager`
  collects nothing when it releases what its steps kept: a format item a
  collaborator deleted while kept keeps its deleted content in this
  client's document until it reloads. Nothing new is written. `YjsContract`
  checks `Item#keep` and `Transaction#local`.
- `Cursor#runOpener`: a restore's scan stops at content of another client,
  so a client's earlier end and later opener with a collaborator's text
  between them are not one restored run (#9599).
- `Cursor` rule 3: a range end that writes its own copy of another client's
  opener also ends it where that opener's run ended, so undoing that
  client's change leaves the text after the range alone.
- `Cursor`: a format write removes the null and the restore that this
  client's own earlier insert left inside its range, as Yjs's `formatText`
  does, so a collaborator's typing no longer relies on items the insert's
  undo deletes (#9671).
- Tests: `regressions/OwnFormatItems.test.ts` runs the #9623, #9599 and
  #9671 repros and a restated-opener case, gc on and off, every client id
  order. `regressions/OwnFormatItemsRelease.test.ts` checks the release:
  clears, an undo that empties both stacks, writes the stacks do not
  capture, dispose, a new binding on the same document, another binding's
  undo and an app `UndoManager` tracking a provider's updates after this
  one releases, repeated cycles, and what a relay stores.
  `regressions/UndoSpreadSession2810.test.ts` replays a downstream app's
  undo-fuzz session where, with gc on, the parent commit put a comment on
  four paragraphs and main did not. The harness
  takes a `gc` option and `LEXICAL_Y_GC=1` runs every harness test with gc
  on. Pins that now pass are plain tests: `UndoRestoredRunEndExtents` (117
  cells), `UndoRestoredRunEnds` (20; the D64 cells stay pinned),
  `UndoSpreadFuzz` (5 seeds; its documents use gc on),
  `UndoCommentRemovalSpread` (4), `RunSequenceUndo` seed 87.
- DECISIONS.md: D88 (own format items kept from gc while undoable), D78
  decided (the #9599 class fixed).
- Docs for D86 (#9678 review follow-ups): y.md's #9564 entry no longer
  says a delete drops the item that ended the comment, and the Deleted
  formatted text limit says edits at such a place get slower; two comments
  rewrapped and a test title updated.

## Test plan

`OwnFormatItems.test.ts`: 72 passed; on the parent commit 44 fail (the
issue cells with gc on for #9623; #9599, #9671 and the restated-opener
color cells with gc on and off). `OwnFormatItemsRelease.test.ts`: 16 passed;
without the release paths 5 fail, and collecting on release fails the
other binding's and the app `UndoManager`'s undo with gc on.
`UndoSpreadSession2810.test.ts`: 4 passed; on the parent commit the 2 gc-on
cells fail (main passes all 4). The pins above flip
from expected-fail to pass and nothing else changes in the lexical-y unit
suite.
zurfyx added a commit that referenced this pull request Oct 31, 2026
…llection while they are undoable

## Description

With Yjs's default garbage collection, which products use, an undo could
not read the format items of its own change after a collaborator deleted
them: they were collected, so the undo guards (`isKeptByUndo`, `changeEnd`,
`closeRestoredRuns`) took them for other content, and the value an undo
brought back ran on over the rest of the document (#9623). The unit harness
built every peer with gc off, so most tests never saw it.

- `Binding`: the binding sets Yjs's own `Item.keep` on the format items its
  transactions and its undo manager's insert, and on the end of each run
  they delete. It releases them, between transactions, when both undo
  stacks are empty (after a clear, an undo that leaves nothing, or a write
  they did not capture) and when it is disposed. An item another binding or an `UndoManager` step
  also keeps stays kept. Releasing collects nothing, as the `UndoManager`
  collects nothing when it releases what its steps kept: a format item a
  collaborator deleted while kept keeps its deleted content in this
  client's document until it reloads. Nothing new is written. `YjsContract`
  checks `Item#keep` and `Transaction#local`.
- `Cursor#runOpener`: a restore's scan stops at content of another client,
  so a client's earlier end and later opener with a collaborator's text
  between them are not one restored run (#9599).
- `Cursor` rule 3: a range end that writes its own copy of another client's
  opener also ends it where that opener's run ended, so undoing that
  client's change leaves the text after the range alone.
- `Cursor`: a format write removes the null and the restore that this
  client's own earlier insert left inside its range, as Yjs's `formatText`
  does, so a collaborator's typing no longer relies on items the insert's
  undo deletes (#9671).
- Tests: `regressions/OwnFormatItems.test.ts` runs the #9623, #9599 and
  #9671 repros and a restated-opener case, gc on and off, every client id
  order. `regressions/OwnFormatItemsRelease.test.ts` checks the release:
  clears, an undo that empties both stacks, writes the stacks do not
  capture, dispose, a new binding on the same document, another binding's
  undo and an app `UndoManager` tracking a provider's updates after this
  one releases (also from inside that `UndoManager`'s capture), repeated
  cycles, and what a relay stores.
  `regressions/UndoSpreadSession2810.test.ts` replays a downstream app's
  undo-fuzz session where, with gc on, the parent commit put a comment on
  four paragraphs and main did not. The harness
  takes a `gc` option and `LEXICAL_Y_GC=1` runs every harness test with gc
  on. Pins that now pass are plain tests: `UndoRestoredRunEndExtents` (117
  cells), `UndoRestoredRunEnds` (20; the D64 cells stay pinned),
  `UndoSpreadFuzz` (5 seeds; its documents use gc on),
  `UndoCommentRemovalSpread` (4), `RunSequenceUndo` seed 87.
- DECISIONS.md: D88 (own format items kept from gc while undoable), D78
  decided (the #9599 class fixed).
- Docs for D86 (#9678 review follow-ups): y.md's #9564 entry no longer
  says a delete drops the item that ended the comment, and the Deleted
  formatted text limit says edits at such a place get slower; two comments
  rewrapped and a test title updated.

## Test plan

`OwnFormatItems.test.ts`: 72 passed; on the parent commit 44 fail (the
issue cells with gc on for #9623; #9599, #9671 and the restated-opener
color cells with gc on and off). `OwnFormatItemsRelease.test.ts`: 18 passed;
without the release paths 5 fail, and collecting on release fails the
other binding's and the app `UndoManager`'s undo with gc on, as does a
release inside that `UndoManager`'s capture.
`UndoSpreadSession2810.test.ts`: 4 passed; on the parent commit the 2 gc-on
cells fail (main passes all 4). The pins above flip
from expected-fail to pass and nothing else changes in the lexical-y unit
suite.
zurfyx added a commit that referenced this pull request Oct 31, 2026
…llection while they are undoable

## Description

With Yjs's default garbage collection, which products use, an undo could
not read the format items of its own change after a collaborator deleted
them: they were collected, so the undo guards (`isKeptByUndo`, `changeEnd`,
`closeRestoredRuns`) took them for other content, and the value an undo
brought back ran on over the rest of the document (#9623). The unit harness
built every peer with gc off, so most tests never saw it.

- `Binding`: the binding sets Yjs's own `Item.keep` on the format items its
  transactions and its undo manager's insert, and on the end of each run
  they delete. It releases them, between transactions, when both undo
  stacks are empty (after a clear, an undo that leaves nothing, or a write
  they did not capture), and when it is disposed or its document destroyed
  (a queued release keeps the stronger request; a listener call under way
  at dispose keeps nothing). An item another binding or an `UndoManager` step
  also keeps stays kept. Releasing collects nothing, as the `UndoManager`
  collects nothing when it releases what its steps kept: a format item a
  collaborator deleted while kept keeps its deleted content in this
  client's document until it reloads. Nothing new is written. `YjsContract`
  checks `Item#keep` and `Transaction#local`.
- `Cursor#runOpener`: a restore's scan stops at content of another client,
  so a client's earlier end and later opener with a collaborator's text
  between them are not one restored run (#9599).
- `Cursor` rule 3: a range end that writes its own copy of another client's
  opener also ends it where that opener's run ended, so undoing that
  client's change leaves the text after the range alone.
- `Cursor`: a format write removes the null and the restore that this
  client's own earlier insert left inside its range, as Yjs's `formatText`
  does, so a collaborator's typing no longer relies on items the insert's
  undo deletes (#9671).
- Tests: `regressions/OwnFormatItems.test.ts` runs the #9623, #9599 and
  #9671 repros and a restated-opener case, gc on and off, every client id
  order. `regressions/OwnFormatItemsRelease.test.ts` checks the release:
  clears, an undo that empties both stacks, writes the stacks do not
  capture, dispose, a new binding on the same document, another binding's
  undo and an app `UndoManager` tracking a provider's updates after this
  one releases (also from inside that `UndoManager`'s capture), dispose or
  destroy while a release waits, dispose during the first capture,
  repeated cycles, and what a relay stores.
  `regressions/UndoSpreadSession2810.test.ts` replays a downstream app's
  undo-fuzz session where, with gc on, the parent commit put a comment on
  four paragraphs and main did not. The harness
  takes a `gc` option and `LEXICAL_Y_GC=1` runs every harness test with gc
  on. Pins that now pass are plain tests: `UndoRestoredRunEndExtents` (117
  cells), `UndoRestoredRunEnds` (20; the D64 cells stay pinned),
  `UndoSpreadFuzz` (5 seeds; its documents use gc on),
  `UndoCommentRemovalSpread` (4), `RunSequenceUndo` seed 87.
- DECISIONS.md: D88 (own format items kept from gc while undoable), D78
  decided (the #9599 class fixed).
- Docs for D86 (#9678 review follow-ups): y.md's #9564 entry no longer
  says a delete drops the item that ended the comment, and the Deleted
  formatted text limit says edits at such a place get slower; two comments
  rewrapped and a test title updated.

## Test plan

`OwnFormatItems.test.ts`: 72 passed; on the parent commit 44 fail (the
issue cells with gc on for #9623; #9599, #9671 and the restated-opener
color cells with gc on and off). `OwnFormatItemsRelease.test.ts`: 22 passed;
without the release paths 5 fail, and collecting on release fails the
other binding's and the app `UndoManager`'s undo with gc on, as does a
release inside that `UndoManager`'s capture; the 4 dispose and destroy
cells fail without the queued-release handling.
`UndoSpreadSession2810.test.ts`: 4 passed; on the parent commit the 2 gc-on
cells fail (main passes all 4). The pins above flip
from expected-fail to pass and nothing else changes in the lexical-y unit
suite.
zurfyx added a commit that referenced this pull request Nov 1, 2026
…llection while they are undoable

## Description

With Yjs's default garbage collection, which products use, an undo could
not read the format items of its own change after a collaborator deleted
them: they were collected, so the undo guards (`isKeptByUndo`, `changeEnd`,
`closeRestoredRuns`) took them for other content, and the value an undo
brought back ran on over the rest of the document (#9623). The unit harness
built every peer with gc off, so most tests never saw it.

- `Binding`: the binding sets Yjs's own `Item.keep` on the format items its
  transactions and its undo manager's insert, and on the end of each run
  they delete. It releases them, between transactions, when both undo
  stacks are empty (after a clear, an undo that leaves nothing, or a write
  they did not capture), and when it is disposed or its document destroyed
  (a queued release keeps the stronger request; a listener call under way
  at dispose keeps nothing; ownership is per listening lifetime, so a
  binding started again keeps what its new steps need). An item another binding or an `UndoManager` step
  also keeps stays kept. Releasing collects nothing, as the `UndoManager`
  collects nothing when it releases what its steps kept: a format item a
  collaborator deleted while kept keeps its deleted content in this
  client's document until it reloads. Nothing new is written. `YjsContract`
  checks `Item#keep` and `Transaction#local`.
- `Cursor#runOpener`: a restore's scan stops at content of another client,
  so a client's earlier end and later opener with a collaborator's text
  between them are not one restored run (#9599).
- `Cursor` rule 3: a range end that writes its own copy of another client's
  opener also ends it where that opener's run ended, so undoing that
  client's change leaves the text after the range alone.
- `Cursor`: a format write removes the null and the restore that this
  client's own earlier insert left inside its range, as Yjs's `formatText`
  does, so a collaborator's typing no longer relies on items the insert's
  undo deletes (#9671).
- Tests: `regressions/OwnFormatItems.test.ts` runs the #9623, #9599 and
  #9671 repros and a restated-opener case, gc on and off, every client id
  order. `regressions/OwnFormatItemsRelease.test.ts` checks the release:
  clears, an undo that empties both stacks, writes the stacks do not
  capture, dispose, a new binding on the same document, another binding's
  undo and an app `UndoManager` tracking a provider's updates after this
  one releases (also from inside that `UndoManager`'s capture), dispose or
  destroy while a release waits, dispose during the first capture, a
  restart before the old release ran,
  repeated cycles, and what a relay stores.
  `regressions/UndoSpreadSession2810.test.ts` replays a downstream app's
  undo-fuzz session where, with gc on, the parent commit put a comment on
  four paragraphs and main did not. The harness
  takes a `gc` option and `LEXICAL_Y_GC=1` runs every harness test with gc
  on. Pins that now pass are plain tests: `UndoRestoredRunEndExtents` (117
  cells), `UndoRestoredRunEnds` (20; the D64 cells stay pinned),
  `UndoSpreadFuzz` (5 seeds; its documents use gc on),
  `UndoCommentRemovalSpread` (4), `RunSequenceUndo` seed 87.
- DECISIONS.md: D88 (own format items kept from gc while undoable), D78
  decided (the #9599 class fixed).
- Docs for D86 (#9678 review follow-ups): y.md's #9564 entry no longer
  says a delete drops the item that ended the comment, and the Deleted
  formatted text limit says edits at such a place get slower; two comments
  rewrapped and a test title updated.

## Test plan

`OwnFormatItems.test.ts`: 72 passed; on the parent commit 44 fail (the
issue cells with gc on for #9623; #9599, #9671 and the restated-opener
color cells with gc on and off). `OwnFormatItemsRelease.test.ts`: 24 passed;
without the release paths 5 fail, and collecting on release fails the
other binding's and the app `UndoManager`'s undo with gc on, as does a
release inside that `UndoManager`'s capture; the 4 dispose and destroy
cells fail without the queued-release handling, and the restart cell
(gc on) with ownership per binding instead of per lifetime.
`UndoSpreadSession2810.test.ts`: 4 passed; on the parent commit the 2 gc-on
cells fail (main passes all 4). The pins above flip
from expected-fail to pass and nothing else changes in the lexical-y unit
suite.
zurfyx added a commit that referenced this pull request Nov 1, 2026
…llection while they are undoable

## Description

With Yjs's default garbage collection, which products use, an undo could
not read the format items of its own change after a collaborator deleted
them: they were collected, so the undo guards (`isKeptByUndo`, `changeEnd`,
`closeRestoredRuns`) took them for other content, and the value an undo
brought back ran on over the rest of the document (#9623). The unit harness
built every peer with gc off, so most tests never saw it.

- `Binding`: the binding sets Yjs's own `Item.keep` on the format items its
  transactions and its undo manager's insert, and on the end of each run
  they delete. It releases them, between transactions, when both undo
  stacks are empty (after a clear, an undo that leaves nothing, or a write
  they did not capture), and when it is disposed or its document destroyed
  (a queued release keeps the stronger request; a listener call under way
  at dispose keeps nothing; ownership is per listening lifetime, so a
  binding started again keeps what its new steps need). An item another binding or an `UndoManager` step
  also keeps stays kept. Releasing collects nothing, as the `UndoManager`
  collects nothing when it releases what its steps kept: a format item a
  collaborator deleted while kept keeps its deleted content in this
  client's document until it reloads. Nothing new is written. `YjsContract`
  checks `Item#keep` and `Transaction#local`.
- `Cursor#runOpener`: a restore's scan stops at content of another client,
  so a client's earlier end and later opener with a collaborator's text
  between them are not one restored run (#9599).
- `Cursor` rule 3: a range end that writes its own copy of another client's
  opener also ends it where that opener's run ended, so undoing that
  client's change leaves the text after the range alone.
- `Cursor`: a format write removes the null and the restore that this
  client's own earlier insert left inside its range, as Yjs's `formatText`
  does, so a collaborator's typing no longer relies on items the insert's
  undo deletes (#9671).
- Tests: `regressions/OwnFormatItems.test.ts` runs the #9623, #9599 and
  #9671 repros and a restated-opener case, gc on and off, every client id
  order. `regressions/OwnFormatItemsRelease.test.ts` checks the release:
  clears, an undo that empties both stacks, writes the stacks do not
  capture, dispose, a new binding on the same document, another binding's
  undo and an app `UndoManager` tracking a provider's updates after this
  one releases (also from inside that `UndoManager`'s capture), dispose or
  destroy while a release waits, dispose during the first capture, a
  restart before the old release ran,
  repeated cycles, and what a relay stores.
  `regressions/UndoSpreadSession2810.test.ts` replays a downstream app's
  undo-fuzz session where, with gc on, the parent commit put a comment on
  four paragraphs and main did not. The harness
  takes a `gc` option and `LEXICAL_Y_GC=1` runs every harness test with gc
  on. Pins that now pass are plain tests: `UndoRestoredRunEndExtents` (117
  cells), `UndoRestoredRunEnds` (20; the D64 cells stay pinned),
  `UndoSpreadFuzz` (5 seeds; its documents use gc on),
  `UndoCommentRemovalSpread` (4), `RunSequenceUndo` seed 87.
- DECISIONS.md: D88 (own format items kept from gc while undoable), D78
  decided (the #9599 class fixed).
- Docs for D86 (#9678 review follow-ups): y.md's #9564 entry no longer
  says a delete drops the item that ended the comment, and the Deleted
  formatted text limit says edits at such a place get slower; two comments
  rewrapped and a test title updated.

## Test plan

`OwnFormatItems.test.ts`: 72 passed; on the parent commit 44 fail (the
issue cells with gc on for #9623; #9599, #9671 and the restated-opener
color cells with gc on and off). `OwnFormatItemsRelease.test.ts`: 24 passed;
without the release paths 5 fail, and collecting on release fails the
other binding's and the app `UndoManager`'s undo with gc on, as does a
release inside that `UndoManager`'s capture; the 4 dispose and destroy
cells fail without the queued-release handling, and the restart cell
(gc on) with ownership per binding instead of per lifetime.
`UndoSpreadSession2810.test.ts`: 4 passed; on the parent commit the 2 gc-on
cells fail (main passes all 4). The pins above flip
from expected-fail to pass and nothing else changes in the lexical-y unit
suite.
zurfyx added a commit that referenced this pull request Nov 1, 2026
…llection while they are undoable

## Description

With Yjs's default garbage collection, which products use, an undo could
not read the format items of its own change after a collaborator deleted
them: they were collected, so the undo guards (`isKeptByUndo`, `changeEnd`,
`closeRestoredRuns`) took them for other content, and the value an undo
brought back ran on over the rest of the document (#9623). The unit harness
built every peer with gc off, so most tests never saw it.

- `Binding`: the binding sets Yjs's own `Item.keep` on the format items its
  transactions and its undo manager's insert, and on the end of each run
  they delete. It releases them, between transactions, when both undo
  stacks are empty (after a clear, an undo that leaves nothing, or a write
  they did not capture), and when it is disposed or its document destroyed
  (a queued release keeps the stronger request; a listener call under way
  at dispose keeps nothing; ownership is per listening lifetime, so a
  binding started again keeps what its new steps need). An item another binding or an `UndoManager` step
  also keeps stays kept. Releasing collects nothing, as the `UndoManager`
  collects nothing when it releases what its steps kept: a format item a
  collaborator deleted while kept keeps its deleted content in this
  client's document until it reloads. Nothing new is written. `YjsContract`
  checks `Item#keep` and `Transaction#local`.
- `Cursor#runOpener`: a restore's scan stops at content of another client,
  so a client's earlier end and later opener with a collaborator's text
  between them are not one restored run (#9599).
- `Cursor` rule 3: a range end that writes its own copy of another client's
  opener also ends it where that opener's run ended, so undoing that
  client's change leaves the text after the range alone.
- `Cursor`: a format write removes the null and the restore that this
  client's own earlier insert left inside its range, as Yjs's `formatText`
  does, so a collaborator's typing no longer relies on items the insert's
  undo deletes (#9671).
- Tests: `regressions/OwnFormatItems.test.ts` runs the #9623, #9599 and
  #9671 repros and a restated-opener case, gc on and off, every client id
  order. `regressions/OwnFormatItemsRelease.test.ts` checks the release:
  clears, an undo that empties both stacks, writes the stacks do not
  capture, dispose, a new binding on the same document, another binding's
  undo and an app `UndoManager` tracking a provider's updates after this
  one releases (also from inside that `UndoManager`'s capture), dispose or
  destroy while a release waits, dispose during the first capture, a
  restart before the old release ran,
  repeated cycles, and what a relay stores.
  `regressions/UndoSpreadSession2810.test.ts` replays a downstream app's
  undo-fuzz session where, with gc on, the parent commit put a comment on
  four paragraphs and main did not. The harness
  takes a `gc` option and `LEXICAL_Y_GC=1` runs every harness test with gc
  on. Pins that now pass are plain tests: `UndoRestoredRunEndExtents` (117
  cells), `UndoRestoredRunEnds` (20; the D64 cells stay pinned),
  `UndoSpreadFuzz` (5 seeds; its documents use gc on),
  `UndoCommentRemovalSpread` (4), `RunSequenceUndo` seed 87.
- DECISIONS.md: D88 (own format items kept from gc while undoable), D78
  decided (the #9599 class fixed). y.md's Limits states D88's local cost.
- Docs for D86 (#9678 review follow-ups): y.md's #9564 entry no longer
  says a delete drops the item that ended the comment, and the Deleted
  formatted text limit says edits at such a place get slower; two comments
  rewrapped and a test title updated.

## Test plan

`OwnFormatItems.test.ts`: 72 passed; on the parent commit 44 fail (the
issue cells with gc on for #9623; #9599, #9671 and the restated-opener
color cells with gc on and off). `OwnFormatItemsRelease.test.ts`: 24 passed;
without the release paths 5 fail, and collecting on release fails the
other binding's and the app `UndoManager`'s undo with gc on, as does a
release inside that `UndoManager`'s capture; the 4 dispose and destroy
cells fail without the queued-release handling, and the restart cell
(gc on) with ownership per binding instead of per lifetime.
`UndoSpreadSession2810.test.ts`: 4 passed; on the parent commit the 2 gc-on
cells fail (main passes all 4). The pins above flip
from expected-fail to pass and nothing else changes in the lexical-y unit
suite.
@zurfyx

zurfyx commented Nov 1, 2026

Copy link
Copy Markdown
Member Author

Landed as part of #9679 (squash 4a13388), which was stacked on this PR and gated with it as one landing tree. #9678 alone is worse than main on one gc-on seed that #9679 fixes, so the two landed together.

This branch was successfully deployed

2 active deployments
Preview – lexical — 8ed68cec Deployed Oct 31, 2026 by vercel[bot]
Preview – lexical-playground — 8ed68cec Deployed Oct 31, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant