Repository navigation
test(selection): wait for the plugin to settle in selection_held_spec - #559
Merged
Merged
Conversation
A fixed wait() ends in the loop iteration that fires the timers, before their vim.schedule_wrap callbacks run, and cannot wait for a chain (the debounce arms the demotion). When the main loop stalls for about 300 ms under load, the next step reads a plugin that is not done. settle() waits until no debounce or demotion timer is pending and every callback queued before it has run.
…hat the debounce sent before d After an operator the plugin has held a cursor, wait_for_cursor lets the debounce and the demotion run for 400 ms again (it was 200), so a selection sent up to 400 ms late shows. The new spec lets the debounce send the selection in visual mode before d, so assert_is_the_selection and the check that the last message is a cursor run on every run, not only when the host is slow.
Member
Author
|
@codex review |
Member
Author
|
@codex security review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
This was referenced Nov 12, 2026
…nothing sent settle() returns when no timer is pending, so a send that a later timer makes would pass. The old fixed waits gave it 300 ms (1300 ms in one test). A mutant that sends the cursor of the terminal buffer 200 ms late failed 3 tests of the old spec and none of the new one. With wait(300) after settle() it fails the same 3 again.
Member
Author
|
@codex review |
Member
Author
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![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: 9a2d27938f
ℹ️ 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".
A timer that fires in the last iteration of the 300 ms wait has not run its vim.schedule_wrap callback when the wait returns. The three tests that expect nothing sent now call wait_for_late_send(), which waits 300 ms and then settles again.
Member
Author
|
@codex review |
Member
Author
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1de8f59664
ℹ️ 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".
… selection A timer that fires in the last iteration of the wait has not run its vim.schedule_wrap callback when the wait returns, so a phantom selection sent at the end of the window would still pass. wait_for_cursor now drains the queued callbacks after the 400 ms.
Member
Author
|
@codex review |
Member
Author
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
4 tasks
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 13, 2026
…oder#540) ## What a user sees You select text in visual mode, a plugin or a mapping edits another line, and your Esc is read before the plugin has checked that edit. The selection is untouched and its text is still there, but Claude gets the cursor and not the selection. coder#524 fixed the case in which the check ran before the key. Three ways to read the key first also failed on `main`, and this PR fixes all of them: 1. A mapping that edits and presses Esc in one key sequence, for example `<Cmd>lua edit_other_line()<CR><Esc>`. 2. A key that arrives while a callback runs, as a UI sends it (`nvim_input`): during the plugin's own timer callback, or during another one that was queued before the check. 3. An autocmd that edits when visual mode starts (a `ModeChanged` handler for `n:v`) with the Esc already in the typeahead (`ve<Esc>`). The replays of `verify-selection-tracking` print `BUG` on `main` for each of them, 3 of 3 runs on Neovim 0.8.0 and 0.12.3, and `OK` with this change. ## Cause The flush on visual exit skips a selection that has an edit nobody checked yet (`entry.consumed or entry.unchecked_edits > 0`). An operator (`d`, `c`, `>`) edits the text and then ends visual mode, and its flush runs before any check, so that skip is what stops a phantom selection. A plugin's edit leaves visual mode on. The check that tells the two apart (is visual mode still on?) ran only in `vim.schedule`, and a schedule runs after the command or callback that made the edit returns, and after any callback that was queued before it. When a key is read first, the flush takes the plugin's edit for an operator's. ## The fix `lua/claudecode/selection.lua`: - `check_visual_edits(entry)` is the old scheduled check as its own function. It judges every edit that is waiting at once, by the mode that they left (visual mode on: a plugin's edit, benign. Off: an operator consumed the selection). It does nothing for a stale entry or when nothing waits, so running it twice is safe. - `note_visual_edit` still counts the edit and schedules the check, and it now also sets a `vim.on_key` listener (one namespace, made on first use). Neovim calls the listener for the next key before it processes that key, also for a key that comes from a mapping or that a UI sends, so the Esc finds visual mode still on. Whichever of the schedule and the key comes first runs the check, and the check clears the listener. - An operator's flush still comes before both, so `d`, `c`, `>` and the other operators skip as before. A listener of a flushed selection does nothing at the next key and clears itself, so it can live until that key (README says "at the latest until the next key"). I checked the key listener on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.4 and 0.12.3 (a probe script, not the plugin): it fires for the `<Esc>` of a mapping before it is processed with the mode still `v`, a second `vim.on_key(fn, ns)` replaces the first, and `vim.on_key(nil, ns)` is safe inside the callback and for an unregistered namespace. I did not try the range of the edit (`on_lines` receives the changed lines) as the test for whether it touched the selected lines. By my reasoning, which I did not test, a plugin edit inside the selection would then count as an operator's although the selection is still on the screen, and the marks and the edit range can be in different coordinates once lines move. ## Proof - `tests/integration/selection_visual_edit_spec.lua` has eight new real-Neovim cases: a mapping edits another line and presses Esc, appends lines and presses Esc, edits twice and presses Esc, writes the file and presses Esc, edits and then `d` (nothing is sent after it), an operator followed by the next selection in one typeahead, an autocmd edit at a mode change with `ve<Esc>`, and a queued check that runs after visual mode ended and before the flush (a user `ModeChanged` handler for `*:n` that calls `vim.wait(0)`, defined before the plugin starts, and a mapping that edits and presses Esc). On `main`'s `selection.lua` five of them fail on Neovim 0.8.0 and 0.12.3 (the first three, the autocmd case and the new one), and the rest are guards. The spec passes 20 of 20 on 0.8.0, 0.9.5, 0.10.4, 0.11.4 and 0.12.3. The case for a key that arrives during a callback is a replay only: `vim.wait` gives the callback's key a main-loop turn first, so the check cannot lose the race in that spec. - `tests/selection_test.lua` has eleven new cases (the mock gets `vim.on_key` and one `nvim_create_namespace` id for each name, a new one for the empty name, as Neovim does): the key comes before the schedule, a check that runs after visual mode ended and before the flush judges nothing again, several edits are judged together, the edit of a mapping is not the operator that comes next, a stale check does not clear the listener of the next selection, the schedule clears the listener, several edits keep one listener, a stale listener clears itself, an operator's listener waits for a key, and (the last two) a detached listener counts as an operator on the visual entry and the detach of another buffer does not. I changed `selection.lua` in 14 ways, one at a time, and counted the failures in `tests/selection_test.lua` (158 cases): no key listener (9), a listener that only clears (4), one decrement for each check (1), never consumed (6), always consumed (6), no schedule (6), no `clear_visual_edit_keys()` in the key listener (2), no `clear_visual_edit_keys()` in `check_visual_edits` (1), a `clear_visual_edit_keys` that does nothing, which removes both (4), a namespace for each edit (3), no stale-entry guard (1), no `unchecked_edits == 0` guard (1), and no `entry.consumed = true` in `on_detach` (1). 13 are killed. One survives and I think it is equivalent, but I did not prove it: the key listener checks `M.state.visual_entry` instead of the entry it was set for. The listener always belongs to the last noted entry, and a different current entry has no unchecked edit, so the zero guard returns. The zero guard itself is not equivalent. A `ModeChanged` handler that runs before the plugin's and calls `vim.wait(0)` runs the queued check after visual mode ended and before the flush. Without the guard that check judges the edit a second time, takes it for an operator's, and Claude gets the cursor. The new unit case and the new integration case kill it (the integration case on Neovim 0.8.0 and 0.12.3). - `verify-selection-tracking` (merged in coder#477): `check.sh` passes (129,986 distinct states) and so does `verify-diff-lifecycle` (89,205), with both locks re-recorded: `selection.lua` changed, the selection skill has a new covered block (`check_visual_edits`), and the diff model reads no block or site of that file. All 16 replays print `OK` on Neovim 0.8.0 and 0.12.3 at the default delay and with `DELAY_MS=50`. The three that replay these bugs print `BUG` on `main`. - The skill is updated for this fix: `VisualCheckSparesPluginEdit` now reads `check_visual_edits` and both triggers in `note_visual_edit`, so removing the schedule, the key listener, the mode test or the `consumed` mark reads `FALSE` and nothing else (checked with five mutants of `selection.lua`; the model then breaks `ReleaseSent`). `ListensToSelectionBuffer` reads `check_soon()` in the body of `on_lines` (in the branch that holds the selection) and in the body of `on_reload`, each body on its own. So moving the call into the `elseif not entry` branch, removing it from `on_lines` or `on_reload`, or moving it from `on_reload` into `on_detach` reads `FALSE` and nothing else (five mutants). All 27 facts read `TRUE` on this tree. On the code before coder#524 exactly four facts read `FALSE` (checked at the previous push), and the old form of `on_lines` still reads `TRUE` for `ListensToSelectionBuffer` after this change. The replay header prints the Neovim version as numbers (it printed a table address on 0.8). - Gates on the final head: the full unit suite (`busted --shuffle`, seed 1794484841) 1748 successes, 0 failures, 28 pending. The integration runner (CI style, a private `HOME`, normal and `odd tmp 50%` `TMPDIR`): 535 tests in both runs on 0.12.3 and 398 in both runs on v0.8.0, 0 failures, and the real cache stays untouched. `scripts/tla/test-check-lib.sh` 44 of 44. The spec passes 20 of 20 on all five Neovim versions, `selection_test.lua` 158 of 158, `check.sh` of both skills passes, `luacheck` 0 warnings in 185 files, `treefmt --ci` 0 changes in 427 files, and `--hash-only` matches for all four `verify-*` skills (no re-record was needed after the merge of `main`). - `selection_held_spec.lua` is flaky under load on `main` (here 7 of 72 runs, on `origin/main` 14 of 72). This PR does not touch it. coder#559 fixes it with condition waits and shows the cause with traces. ## Notes for the reviewer - coder#553 is merged. This branch merges `main` and keeps the helpers of that PR at the outer scope of the spec, because the mapping cases use them. coder#559 also edits `selection_visual_edit_spec.lua` (`wait_for_cursor` waits 400 ms, and one new case at the end of the operator block), so whichever of the two lands second needs a small merge in that file. - Neovim 0.8 and 0.9 still need the scheduled check for a plugin edit whose callback ends before any key (their `on_lines` runs with visual mode off during the API call). The key listener adds the keys that come first. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 13, 2026
…e terminal (coder#562) ## What a user sees You select text and press Esc. Within the next 100 to 150 ms you enter the Claude terminal (a toggle mapping, or `:ClaudeCode` that opens it). You come back to the file and the cursor has not moved. Claude holds the cursor and not your selection. A slower switch, or a terminal that Neovim already knows, keeps the selection. With this change Claude keeps it in both cases. I found this while tracing the `selection_held_spec` flake (coder#559): a stall of the main loop moved the demotion into that window, and the plugin did what a person can also trigger. ## Cause At the default `visual_demotion_delay_ms` of 50, the debounce (100 ms) reads the release and arms the demotion timer, and the timer fires 50 ms later. When you enter a terminal that the plugin already knows, `reconcile_held_selection` cancels the demotion at `BufEnter`. A new terminal has no name and is not registered at `BufEnter`, so nothing cancels it, and the demotion fires while the terminal is the current buffer, before the debounce of that `BufEnter` can cancel it. Condition 1 of `handle_selection_demotion` handled that case. It returned, but first it set `last_active_visual_selection = nil`. When you came back, `update_selection` found no recent visual exit, took the "genuinely in normal mode" branch and sent the cursor. Why Condition 1 cleared it: `git log -L` on the block shows one commit, f1cad48 (May 2025, the commit that added `visual_demotion_delay_ms`). Its comment says the selection is preserved and the "pending" status of `last_active_visual_selection` is resolved. That was written before coder#246. At that commit Condition 3 demoted without the cursor-unmoved test, and a return to the file reported the cursor. coder#246 (coder#267, `0b245051`) added the test and made Condition 3 leave the record intact, so that a later cursor move re-arms the demotion. Condition 1 was the one branch that still cleared it. ARCHITECTURE.md already said the timer "does nothing in the Claude terminal", and the model had a step for Condition 1 and a `holdLoose` flag that allowed either outcome on the return (SKILL.md lists it as a case where the model allows both). ## The fix `lua/claudecode/selection.lua`: Condition 1 is now `if M.is_claude_terminal_buffer(current_buf) then return end`. The demotion does nothing in a Claude terminal and leaves the records alone, so the return keeps the selection while the cursor stands where you released it, as it does when you never left. It uses `is_claude_terminal_buffer`, the check that `update_selection` and the flush already use. Before, Condition 1 asked only for the registered terminal. For a terminal that only its name marks (`terminal.provider = "none"` with a `:terminal ... claude` buffer), the demotion fell through to the last block of the function, which cleared the same records, so that user lost the selection the same way. SKILL.md described that as "clears the held-state records and sends nothing". The narrow change (only the registered terminal) leaves that case broken, and the new name-only cases fail with it. ## Each case that Condition 1 served still behaves | Case | Result | Evidence | |---|---|---| | The demotion fires in the registered terminal: it sends nothing and Claude still holds the selection there | same as before | unit tests assert `last_broadcast == nil`, the integration spec asserts `alpha` in the terminal | | The "pending" status stays: the return arms a new demotion, which returns at Condition 3 while the cursor is unmoved | no loop, no cursor sent | unit tests call `settle()` after the return | | You come back and the cursor has not moved | Claude keeps the selection (was: the cursor) | new unit tests, new spec (delay 50) | | You come back and move the cursor | Claude gets the cursor at `CursorMoved` | new unit test (it fails on `main` earlier, at the assertion that the selection was kept) | | The terminal was known at `BufEnter`: the demotion is cancelled there | same as before | existing unit test "keeps the selection when its demotion is armed just before you enter the Claude terminal" | | Delay 200: the terminal's own debounce cancels the demotion first | keeps the selection | new spec (delay 200) | | A terminal that only its name marks (no provider) | keeps the selection (was: the cursor) | new unit test, new spec | | The text of the held selection changes while you are in the terminal | the cursor is reported, as before | the existing unit tests of `demote_if_selection_text_changed` pass | | You go to another file buffer, not a terminal | unchanged (Condition 3 or the last block, which still clears) | existing unit tests pass | ## Proof, test first - `tests/integration/selection_demotion_in_terminal_spec.lua` (new, 3 cases): release `alpha`, arm the demotion, open a terminal, let the demotion fire there, come back, and check that Claude holds `alpha`. It fails on `main`'s `selection.lua` for delay 50 with a registered terminal and for the name-only terminal (2 of 3, on Neovim 0.8.0 and 0.12.3). Delay 200 passes on both, because the debounce cancels the demotion. With the fix it passes 3 of 3 on 0.8.0, 0.9.5, 0.10.4, 0.11.4 and 0.12.3. It arms the demotion by calling what the debounce timer calls (`update_selection`), so that the demotion is pending when the terminal opens and a stall cannot end it first. - `tests/selection_test.lua` (3 new cases, 161 in the file on the final head): the new terminal, the name-only terminal, and the cursor move after the return. All 3 fail on `main`'s `selection.lua`. With only the registered terminal in Condition 1 (the narrow change), the name-only case fails and nothing else does. - Model: `holdLoose` is gone from `SelectionTracking.tla`, so `HeldUntilMoved` now applies to the return from the terminal. A new fact, `DemotionIgnoresTerminal`, reads the whole body of Condition 1 (the `if` and one `return`, nothing else), and `Demote` uses `clearVis` when it is `FALSE`. On this tree `check.sh` passes (95,938 distinct states, 129,986 before: the either-outcome branch is gone). With the old clear put back, `DemotionIgnoresTerminal` reads `FALSE` and `check.sh` fails with `main: BROKEN: HeldUntilMoved`. - `check.sh --mutants` on the merged tree (head `7ba852fa5e`): exit 0 in 3,774 s, 27 rows (26 before, one for the new fact), "every guard is caught". The new fact's row reads `DemotionIgnoresTerminal=FALSE caught by: HeldUntilMoved HeldInTerminal`. - Both locks are re-recorded (`verify-selection-tracking`, and `verify-diff-lifecycle` for the file hash of `selection.lua`: the diff model reads no block or site of it). `--hash-only` matches for all four `verify-*` skills. ## Gates - Full unit suite (`busted --shuffle`, seed 1794487328, before the last test was added): 1739 successes, 0 failures, 29 pending. `selection_test.lua` alone 161 of 161 on the final head. A verifier reran the full suite on the head (same seed): 1770 successes, 0 failures, 29 pending. - The integration runner (CI style, a private `HOME`, normal and `odd tmp 50%` `TMPDIR`): 530 tests in both runs on 0.12.3 and 393 in both runs on v0.8.0, 0 failures. - `selection_held_spec.lua` 14 of 14 and the new spec 3 of 3 on all five Neovim versions, on the final head. - `luacheck` 0 warnings in 186 files, `treefmt --ci` 0 changes, `shfmt` and `shellcheck` clean on `check.sh`. ## Notes for the reviewer - coder#540 is merged. This branch merged `main` after it. Only the two `covered.lock` files conflicted, and I re-recorded both (`--hash-only` matches for all four `verify-*` skills). The skill checks pass on the merged tree: `check.sh` 95,938 states for the selection skill, 89,205 (and 5,269 live) for the diff skill, 16 of 16 replays on Neovim 0.8.0 and 0.12.3, `selection_test.lua` 161 of 161. - The test-only PR coder#559 fixes the flaky waits in `selection_held_spec.lua`. This PR does not touch that file, so the two do not conflict. - A demotion in a terminal still lets the debounce of the terminal run later: `update_selection` returns in the terminal and only checks the held text, as before. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Result
selection_held_spec.luafailed in 24 of 144 runs under load. With this change it failed in 0 of 144 runs, on the same tree, in the same load windows.The load was 16 spec runs at the same time, 8 disk writers (
dd conv=fsyncloops), Neovim 0.12.3, three rounds that alternate the old and the new spec (round 1: 18 of 48 against 0 of 48, round 2: 1 against 0, round 3: 5 against 0). Without load the spec takes 9 s instead of 23 s, because it no longer sleeps.This PR is only test code (two spec files). It does not change the plugin.
Cause
Every wait in the spec was a fixed
vim.wait(N). A wait ends at its deadline. When the main loop stalls for 230 to 390 ms (a loaded disk or CPU), the wait can end in the loop iteration that fires the timers. Thevim.schedule_wrapcallbacks of those timers have not run yet. And a late debounce arms the demotion timer too late for the deadline. The next step then reads a plugin that is not done.I traced this with a copy of the spec that logs the plugin's calls, adds a 5 ms heartbeat timer (it reports every stall of the loop), and dumps the log when a test fails. Each failure below comes from a wait that ended too early. The last row also exposes a plugin gap (see "Found while tracing"). The traces are in the details below.
enew)selection.isEmptywasfalseupdate_selectionsent the empty cursor 0 ms after the assertionselection.isEmptywasfalseselection_changedforterm://x//1:claudewas sent"", not"alpha"The old spec failed these in 24 of 144 runs: 18 times the overflow test, 7 times the first-opens test, 2 times a wiped-buffer test (27 failed tests in 24 runs). The listener test failed in other runs: 12 of 60, 5 of 60 and 15 of 48 traced runs, and 2 of 36 untraced runs.
Traces (times in ms, from a traced copy of the old spec)
Listener test, a stall straddles the end of
wait(300):Wiped-buffer test (
nvim_buf_delete), the debounce runs late and its demotion misses the deadline:Overflow test. I reproduced it by putting a 220 ms sleep between two loop iterations of the wait after Esc (the natural failure has the same assertion message:
selection_changed,filePath = 'term://x//1:claude',text = ''):First-opens test (delay 50):
What changed
tests/integration/selection_held_spec.lua:settle(keep_demotion)waits until the plugin has no debounce timer and no demotion timer pending, and until everyvim.schedulecallback queued before the call has run (it queues a flag and waits for the flag, and the queue is first in, first out). Everywait(N)that waited for the plugin is nowsettle().select_and_release(keep_demotion)settles afterve(the debounce reads the selection) and after Esc (the debounce reads the release, and the demotion runs). The two specs that need a long demotion still pending (visual_demotion_delay_msof 1000) passkeep_demotion, and the helper asserts that the demotion is pending, so a stall cannot silently end that premise.wait(20)stays, with its comment: the popup is closed inside the debounce on purpose.tests/integration/selection_visual_edit_spec.lua(two follow-ups to #553, F1 and F2):wait_for_cursorwaits 400 ms after the cursor (it was 200) and then drains the queued callbacks, so a phantom selection sent up to 400 ms after the operator shows again.d. The two cases above it pressdwithin the debounce, so their early-selection checks usually have nothing to look at. This one checks the text and the range of the early selection and that the last message is a cursor, on every run.The tests still test the same things
One thing was lost in the first version and is back in round 2. The three tests that expect nothing to be sent (overflow, pending demotion, sends nothing in another terminal) lost the late-send window of their old fixed waits, because
settle()returns as soon as no timer is pending. The verifier's mutant sends the cursor of the terminal buffer 200 ms late withvim.defer_fn. It failed 3 tests of the old spec and none of the first version of the new spec. Each of the three tests now callswait_for_late_send(): it waits 300 ms aftersettle()and then settles again, because a timer that fires in the last iteration of that wait has not run its callback yet (Codex found this in round 2). The mutant fails the same 3 tests again, and the unchanged plugin passes all 14.I changed
selection.luaten ways (one at a time, in a scratch copy) and ran the old and the new spec. None of these ten is a late send. The set of failing tests is the same for nine of them, and the new spec also kills one that the old spec did not:selection.luaupdate_selectionfrom_terminalignores the terminal namecheck_sooninon_reloadcheck_sooninon_linescaf.leftmarkon_detachFor the #553 follow-ups, on #553's tree: removing the flush guard fails 5 tests of the old spec and 6 of the new one. A range off by one at the selection end (the text stays right) survives the old spec and fails the new F2 case. A phantom selection sent 350 ms after the operator fails 4 of the 12 tests of #553's spec with
wait(200)in every run of the independent verifier (I saw 0 failures in one run, so the old margin depends on timing), and 6 tests withwait(400).Found while tracing: a plugin bug that this PR does not fix
At the default
visual_demotion_delay_msof 50, Claude loses a held selection when you enter the Claude terminal 100 to 150 ms after Esc and then come back. The demotion timer fires while the terminal is the current buffer, and Condition 1 ofhandle_selection_demotionsetslast_active_visual_selection = nil. When you return,update_selectionfinds no recent visual exit and sends the cursor.A probe with no stall shows it: release
alpha, wait until the debounce arms the demotion, open the terminal at once, wait for the demotion,wincmd p. At delay 50 the held text after the return is"". At delay 200 it is"alpha"(the terminal's own debounce cancels the demotion). Removing thelast_active_visual_selection = nilblock in Condition 1 keeps"alpha"at both delays, and this spec stays 14 of 14. That change touches a file thatverify-selection-trackingcovers, so it is its own PR: #562. The old test only failed because a stall moved the demotion into that window.Gates
selection_held_spec.lua14 of 14 andselection_visual_edit_spec.lua13 of 13 on Neovim 0.8.0, 0.9.5, 0.10.4, 0.11.4 and 0.12.3 before the fix(selection): check a plugin's edit in visual mode at the next key #540 merge, and 21 of 21 on 0.8.0 and 0.12.3 after it (a verifier rerun). Under the load of the first table (16 runs at once, 8 disk writers) the spec with the added waits failed 0 of 48 runs.HOME, normal andodd tmp 50%TMPDIR): 0.12.3 528 of 528 in both runs, v0.8.0 391 of 391 in both runs, 0 failures.luacheck0 warnings,treefmt --ci0 changes. No unit test changes, so I did not run the unit suite.Notes for the reviewer
mainwithout a conflict, and F1 and F2 sit on the helpers that fix(selection): check a plugin's edit in visual mode at the next key #540 moved to the outer scope ofselection_visual_edit_spec.lua.Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh