fix(selection): push quickly-made visual selections to Claude (#246)

[ThomasK33]

Summary

Fixes #246 — single-line visual selections often never reached Claude under passive selection tracking.

Investigation (the full triage is on the issue) showed this is not single-line-specific and not a Claude-CLI limitation — a real Claude CLI shows ⧉ 1 line selected whenever the plugin actually broadcasts a single-line selection. The fault was timing/extraction in lua/claudecode/selection.lua:

1. 100 ms debounce race — a visual selection made and released faster than debounce_ms was only captured if the debounced callback happened to fire while still in visual mode; otherwise it was never broadcast. Single-line selections (viw, vw, v$, mouse, V) are produced/released far faster than multi-line ones, so they lost this race constantly.
2. Demotion-to-empty for an external Claude — with claude in a separate terminal (the /ide flow) there is no in-Neovim Claude buffer, so the demotion grace window meant for "switching to the Claude terminal" could never apply, and the selection was wiped to an empty cursor ~150 ms after leaving visual mode.
3. Stale visual-mode misclassification — get_effective_visual_mode() trusted vim.fn.visualmode() (the last completed visual mode), so a single-line linewise V made right after a charwise selection was extracted charwise → broadcast a single character (or empty on an empty line).

What changed (lua/claudecode/selection.lua)

• Flush on visual-mode exit — on_mode_changed now broadcasts the selection synchronously when leaving visual mode, reading it from the '</'> marks (the live get_visual_selection() returns nil there because the mode is already normal). This closes the debounce race; it deduplicates against the in-visual debounce so a lingered selection is not sent twice.
• Skip mutating operators — the buffer's changedtick is recorded on visual-enter; the flush is skipped when it advanced, so consuming operators (d/c/x/…) don't broadcast stale, post-edit text as a phantom selection.
• Cursor-guarded demotion — a selection is demoted to an empty cursor only once the cursor actually moves (matching the official VS Code extension), instead of timing out — which is what made the external-Claude case lose it.
• Prefer the live mode in get_effective_visual_mode() over vim.fn.visualmode().

Verification

• Full suite green: 538 / 0 / 0, luacheck clean.
• 12 new unit tests covering the flush, dedup, operator-no-phantom, terminal-skip, cursor-guarded + pending-demotion, stale-mode extraction, blockwise approximation, and cross-buffer state cleanup.
• scripts/repro_issue_246.lua is a deterministic gate that reproduces on the unfixed code (exit 1) and passes on the fix (exit 0).
• Confirmed end-to-end against a real Claude CLI connected via /ide: the exact reported flow (viw + Esc) now shows ⧉ 1 line selected and persists, while moving the cursor still clears it.

Known limitations (documented in-code, out of scope here)

• Blockwise (CTRL-V) selections are sent as their contiguous charwise span — selection_changed carries a single range and cannot represent a rectangle (pre-existing; proper per-column support is a follow-up).
• In-place transforms (gU/~/J/=) made-and-left quickly are not flushed (they advance changedtick and can't be told apart from consuming operators by the marks alone); the previous behavior never broadcast them either.
• Select mode (s/S/<C-s>) is deliberately not tracked.

🤖 Generated with Claude Code

[ThomasK33]

@codex review

[ThomasK33]

@claude review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Hooray!

ℹ️ 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".

[claude]

🟡 When the MAXCOL clamp at lines 657-662 fires for non-linewise modes (e.g. <C-V>$ blockwise extending to end of line), it sets end_coords.col = #last_line + 1, which calculate_lsp_positions then uses verbatim as lsp_end_char for non-V modes — one past the LSP-exclusive-end position. Text extraction is correct (Lua string.sub clamps gracefully) and LSP clients typically truncate over-length characters, so practical impact is minimal — but the fix is trivial: gate the clamp on visual_mode == "V", or use min(end_coords.col, #last_line) for non-linewise modes in calculate_lsp_positions.

Extended reasoning...

What the bug is

In M.get_visual_selection_from_marks (lua/claudecode/selection.lua:657-662), the MAXCOL clamp is applied unconditionally, before the visual-mode dispatch:

local last_line = lines_content[#lines_content] or ""
if end_coords.col > #last_line + 1 then
  end_coords.col = #last_line + 1
end

For linewise V, the clamp is harmless because calculate_lsp_positions (lines 541-548) ignores end_coords.col and computes lsp_end_char from #lines_content[#lines_content]. But for charwise v and blockwise \22, line 551 uses end_coords.col directly as lsp_end_char. After clamping a MAXCOL '> to #last_line + 1, lsp_end_char = #last_line + 1 — one past the correct LSP-exclusive-end (#last_line).

Does it actually trigger?

The author's own comment is the key evidence — line 657 reads: "For linewise selections (and $), the '> column can be MAXCOL (2147483647)". The clamp was added specifically because the author believed $ can also produce a MAXCOL '> outside linewise mode. The concrete trigger is <C-V>$ (blockwise extending to end of line), where Neovim records a sentinel column in '>.

Some verifiers refuted this on the grounds that '> for non-linewise modes always holds the actual byte column, never MAXCOL — making the clamp condition (end_coords.col > #last_line + 1) unreachable for non-V. If that were true, the clamp would be pure dead code for non-V modes, with no behavioral consequence either way. But the author wrote this clamp deliberately, with a comment that names $ as a trigger — they clearly believe the path is live. Either way, the recommended fix is a one-line tightening that costs nothing if the path is in fact dead, and corrects the off-by-one if it isn't.

Step-by-step proof (assuming MAXCOL '> for <C-V>$ on hello)

1. vim.fn.visualmode() returns "\22" (CTRL-V).
2. getpos("'>") returns column 2147483647 (MAXCOL).
3. end_coords.col = 2147483647.
4. #last_line + 1 = 5 + 1 = 6; clamp fires: end_coords.col = 6.
5. Dispatch enters the "v" or "\22" branch.
6. calculate_lsp_positions line 551: lsp_end_char = end_coords.col = 6.
7. Correct value: for selecting all of "hello" (5 chars), LSP-exclusive end is 5.

Impact

Low. Text extraction returns "hello" correctly because Lua's string.sub("hello", 1, 6) clamps. The LSP Position spec says clients normalize character past line length down to the line length, so most consumers silently round to the right value. The affected case (blockwise + $) is also already documented in this PR as an approximation ("selection_changed carries a single range and cannot represent a rectangular block"). Worth fixing because the fix is trivial and the off-by-one is in code the author wrote intentionally for this case.

How to fix

Either gate the clamp on visual_mode == "V" (its documented use case), e.g.:

if visual_mode == "V" and end_coords.col > #last_line + 1 then
  end_coords.col = #last_line + 1
end

Or, in calculate_lsp_positions line 551, clamp at the LSP step:

lsp_end_char = math.min(end_coords.col, #lines_content[#lines_content] or 0)

The second form is more robust (the LSP boundary is the natural place to enforce LSP-exclusive-end semantics).

[claude]

🟡 Stale cursor_at_flush from a previous successful flush survives the changedtick early-return at line 730-732 (and is also not cleared on visual entry). If the cursor coincidentally lands on the prior flush position after a mutating operator (e.g. vd on a single character at the same column, or other cases where the post-mutation cursor matches), cursor_unmoved_since_flush returns true against stale state on the next handle_selection_demotion, suppressing demotion — so latest_selection keeps pointing at deleted/mutated text until the user moves the cursor. One-line fix: clear M.state.cursor_at_flush on the early-return paths in flush_visual_selection (or in the entering_visual branch of on_mode_changed).

Extended reasoning...

What the bug is

M.flush_visual_selection (lua/claudecode/selection.lua:703-755) has an early-return guard at line 730-732 that skips the flush when the buffer's changedtick advanced since visual entry (a consuming operator like d/c/x mutated the selection). The guard is intentionally conservative — its comment explains it treats this as 'a residual non-broadcast, not a regression'. But it forgets to invalidate M.state.cursor_at_flush left behind by a previous successful flush, and the visual-entry branch of on_mode_changed (line 168-176) does not clear it either.

The code path that triggers it

cursor_at_flush is set on every successful flush at line 758. It is read by cursor_unmoved_since_flush (line 24-31), which handle_selection_demotion uses at line 404 to suppress demotion while the cursor still sits on the previous flush position. Two paths exist that should invalidate it but do not:

1. The changedtick early-return at line 730-732 (flush_visual_selection aborts because an operator consumed the selection).
2. The entering_visual branch in on_mode_changed (line 173-175) — re-entering visual mode without first moving the cursor leaves the prior flush position intact.

Why existing code does not prevent it

handle_selection_demotion's Condition 3 (line 397-415) intentionally leaves cursor_at_flush in place when the cursor is unmoved, per the comment at line 398-403, so a later cursor move can re-arm demotion. That design is correct for the happy path (held selection, then real cursor move → demote). What is missing is invalidation on the abort paths, where the held selection has been consumed by a mutating operator but cursor_at_flush still references it.

Step-by-step proof (single-char vd reproducer)

Buffer line 1 = alpha. Cursor starts at {1, 4} (on 'a' at end via prior viw or $ motion).

1. User does viw + Esc selecting alpha. flush_visual_selection succeeds: broadcasts alpha, sets cursor_at_flush = {bufnr=1, pos={1,4}}, last_active_visual_selection = sel_alpha, latest_selection = sel_alpha.
2. update_selection debounce fires. demotion_timer is armed for buffer 1.
3. Timer fires within 50ms with cursor still at {1,4}. handle_selection_demotion enters Condition 3 at line 397, cursor_unmoved_since_flush(1) returns true (pos {1,4} == flush {1,4}), returns early at line 405 without clearing cursor_at_flush or last_active_visual_selection (intentional, per design).
4. User does vd at {1,4}: n→v fires on_mode_changed, sets visual_entry = {bufnr=1, tick=T} at line 174. cursor_at_flush is not touched.
5. d consumes the visual range, deleting 'a'. Buffer becomes alph. changedtick advances to T+1. v→n fires on_mode_changed → flush_visual_selection.
6. Line 730 sees T+1 != T, early-returns at line 732. cursor_at_flush is not cleared — still {bufnr=1, pos={1,4}} from step 1.
7. Cursor now at {1,4} (single-char delete: cursor stays at the same column on the next char, or here at the new end-of-line).
8. Debounce fires update_selection. Mode is normal, demotion_timer is nil. last_active_visual_selection exists and matches buffer 1 → elif branch at line 287-321 arms a new demotion timer.
9. Timer fires. handle_selection_demotion(1): Condition 3, cursor_unmoved_since_flush(1) returns true (current {1,4} == stale flush {1,4}). Returns early at line 405. Demotion suppressed.

Result: latest_selection keeps pointing at the now-deleted alpha. get_latest_selection and any ClaudeCodeSend calls return text that no longer exists in the buffer. Self-heals on the very next cursor move (the new cursor_at_flush check fails, demotion fires, bottom of handle_selection_demotion clears both pieces of state).

Impact

Narrow and self-resolving: the trigger requires the post-mutation cursor to coincidentally land at the prior flush column, and a single keystroke (any motion) recovers state. No spurious broadcast happens during the stuck window — has_selection_changed compares against latest_selection itself, so no message is sent. The user-visible effect is that Claude's view of the selection lags behind reality until the user moves the cursor. Worth flagging as a nit because the cursor-guarded demotion mechanism is new in this PR and the gap is a one-line oversight.

Fix

Clear M.state.cursor_at_flush on the early-return paths in flush_visual_selection (the term-buffer branches at line 716-727 and the changedtick branch at line 730-732), or clear it in the entering_visual branch of on_mode_changed at line 173-175. Either approach prevents stale state from outliving the selection it was tracking. Clearing on entering_visual is the most defensive — it invalidates the prior flush position whenever a new visual selection begins, regardless of how the prior one was resolved.