[diffs/edit] Add undo-traversal seals and delete-direction stops to coalescing#988
[diffs/edit] Add undo-traversal seals and delete-direction stops to coalescing#988fat wants to merge 3 commits into
Conversation
Design decision: edits applied through applyEdits are not a separate semantic class — a mixed programmatic/local sequence must behave exactly like the all-local sequence. Previously updateHistory=false skipped the EditStack entirely, so history entries carried stale offsets after any programmatic edit: undo could corrupt text, redo replayed at wrong positions, and exhaustion never restored the original document. TextDocument.applyResolvedEdits now always creates/coalesces/pushes an entry; the updateHistory flag only gates caller-supplied selection and undo-boundary metadata. Editor.applyEdits always tracks (its flag is kept but deprecated). Undo/redo replay is untouched — it always wrote through the private buffer path, so no recursion. Composition previews never touched the document and are unaffected. Flips the seven history-equivalence pins to permanent regression tests (assertions unchanged — they encode the all-tracked reference), rewrites the two old-model pins, and updates the one bypass-model test elsewhere in the suite plus the public docs. Pinned known-bug count: 32 -> 25. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The pre-commit formatter globs cover js/ts but not .mdx, so the History section edit landed unformatted and failed root:format-check in CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oalescing Fixes the three pinned coalescing bugs: 1. Typing after undo fused into the newly exposed pre-undo entry. 2. An undo-boundary entry stopped shielding once undone. 3. Backspace then forward-delete at the same pivot merged into one step. Every undo/redo pop now stamps the exposed top entry with a durable `sealed` flag that rejects forward coalescing — traversal is an undo stop in both directions, covering bugs 1 and 2 with one mechanism. Delete entries additionally carry a `deleteDirection` classified from caret geometry; coalescing rejects only when both entries' directions are known and opposite, so single-labeled and unlabeled (range-based, untracked, multi-caret-partial) deletes keep today's geometry rules. Direction labels propagate through merges so a whole backspace run still stops a forward delete. The both-known rule matters: a single-sided check regressed consecutive outdent grouping (outdent deletes coincide with caret edges), caught in adversarial review and pinned by three new regression tests. One documented residual: a caret mid-indent outdent pair is byte-identical to backspace-then-forward-delete and keeps getting two undo steps; threading explicit direction from the command layer is the follow-up. Pins flipped with assertions unchanged; suite 1281 pass / 0 fail. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3adadf45a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| selection.start.line !== selection.end.line || | ||
| selection.start.character !== selection.end.character | ||
| ) { | ||
| return undefined; |
There was a problem hiding this comment.
When Backspace/Delete is executed through the real helper, applyDeleteCharacterToSelections builds non-collapsed deleteSelections and applyTextReplaceToSelections passes those ranges to applyResolvedEdits as selectionsBefore (selection.ts lines 1203-1234 and 787-792). Because those selections are the deleted ranges rather than the original carets, this collapsed-selection check returns undefined, so no deleteDirection is recorded and a real Backspace followed by forward Delete at the same pivot still coalesces into one undo step. The new regression test bypasses this path by calling TextDocument.applyEdits with caret selections directly.
Useful? React with 👍 / 👎.
1 participant
What
Fixes the three pinned EditStack coalescing bugs (stacked on #987, which it builds on):
How
popUndoToRedo/popRedoToUndostamps the newly exposed top undo entry with a durablesealedflag;shouldCoalesceEditStackEntryrejects a sealed previous entry. Traversal is an undo stop in both directions — typing after undo, after redo, or at any depth of ping-pong starts a fresh step.undoBoundary's paste/cut semantics are untouched.deleteDirectionlabel (backspace/delete); coalescing rejects only when both entries' labels are known and opposite. Labels propagate through merges, so a whole backspace run still stops a forward delete. Anything ambiguous (range selections, multi-caret partial eligibility, untracked edits) stays on today's geometry-only rules.The judgment call reviewers should look at
The both-known rule exists because a single-sided version regressed consecutive outdent grouping — outdent deletes coincidentally place carets on edit edges, and two Shift+Tabs started costing two undo steps. Caught by adversarial review probes, fixed, and pinned by three new regression tests. One documented residual: an outdent pair with the caret mid-indent is byte-identical to backspace-then-forward-delete, so it keeps getting two steps — no classifier can split those without breaking the pinned contract. Threading explicit direction from the command layer (delete handlers, indent dispatch) is the noted follow-up if we want editor-level precision.
Verification
Third PR of the burn-down (#986, #987).
🤖 Generated with Claude Code