Repository navigation
docs(diff): name the Neovim versions that LuaJIT #1066 affects - #550
Merged
Merged
Conversation
The comments on the workaround for LuaJIT issue #1066 said that Neovim 0.8.0 bundles the bug. Neovim 0.9.5 bundles LuaJIT 2.1.1692716794 (commit 03c31124), which is 70 commits older than the fix e86990f7f of 21 September 2023. On 0.9.5 the minimal repro aborts in 10 of 10 runs, and the spec without the call fails in 29 of 32 runs. Neovim 0.10.0 bundles 75e92777, 87 commits after the fix (by GitHub compare, not run). Say "older than September 2023 (bundled by Neovim 0.8 and 0.9)", so nobody deletes the call when the minimum version moves to 0.9. Comments only.
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. Swish! 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. |
hareki
pushed a commit
to hareki/claudecode.nvim
that referenced
this pull request
Nov 13, 2026
…split layouts (coder#551) ## Why When a user empties the proposal buffer (`ggdG` or `:%delete`) and saves it with `:w`, Claude must get what Neovim's own `:w` writes. Neovim writes no bytes for an emptied buffer. With `'eol'` on, the `vertical` and `horizontal` layouts answered `" "`, so Claude wrote a one-byte file where the user had asked for an empty one. The F14 verifier found this. PR coder#542 listed it as a known, pre-existing defect that this PR fixes. ## What changed `_resolve_diff_as_saved` in `lua/claudecode/diff.lua` saves the lines of the proposal buffer, joined with line breaks. It adds a final line break when the buffer's `'eol'` is on. A buffer with no lines at all (Neovim shows "--No lines in buffer--") still gives one empty string through `nvim_buf_get_lines`. So the save could not tell it from a buffer with one empty line, and it added the line break. The save now counts the bytes of the buffer with `vim.fn.wordcount().bytes`, run inside `nvim_buf_call`. The count is 0 for an empty buffer and at least 1 for a buffer with one empty line when `'eol'` is on. The line break is added only when the count is not 0 and `'eol'` is on. That replaces the check `#content_lines > 0`, which was always true. I tried `nvim_buf_get_offset` and `line2byte` first. Both gave wrong sizes for a buffer that `ggdG` had emptied (a stale line cache), so I did not use them. Other changes: - `tests/mocks/vim.lua` gets `wordcount()` and an `empty = true` flag for a mock buffer. `nvim_buf_call` in the mock now names the buffer that the callback acts on. - `tests/unit/diff_ui_cleanup_spec.lua` gets one fixture line. The mock's `nvim_buf_call` refuses a buffer that does not exist, and that spec saved a buffer that the mock never had. - `lua/claudecode/README.md`: one bullet replaces coder#542's bullet about the saved content of the split layouts. It states both rules, `'eol'` from the proposal and the empty buffer. It also says that a buffer with one empty line is saved as `"\n"` when `'eol'` is on and as `""` when it is off. - `.agents/skills/verify-diff-lifecycle/covered.lock` is re-recorded (see Blast Radius). ## Scope - The `vertical` and `horizontal` layouts share one code path, so one change fixes both. - The `unified` layout is unchanged. It answers with the proposal, not with the buffer, so what the user does to its buffer never reaches Claude. The new spec runs it as a control. - With `'eol'` on, a buffer with one empty line is still saved as `"\n"`. With `'eol'` off, it is saved as `""`, as before. A buffer with any line is saved as before. - On `main` (with coder#542), this PR changes the answer only for a buffer that the user emptied. It changes no answer for an unedited proposal: coder#542 already makes an empty proposal give `""`, and the new spec has that case as a control. ### Minor findings that I declined Two verifiers (Opus and GPT) found these. None of them changes an answer that a user reaches with the default settings. - **A buffer set to `bomb` (Opus F3).** With `ggdG`, then `:setlocal bomb`, then `:w`, Neovim writes the 3-byte BOM. Both `main` and this PR answer without the BOM (`main` answers `"\n"` when `'eol'` is on and `""` otherwise, this PR answers `""`). The plugin never sets `'bomb'` on its scratch proposal buffer, and no answer of the plugin ever holds a BOM, so a user has to set a non-default option on a scratch buffer first. I left it. - **The `nvim_buf_call` wrapper is covered by the mock spec only (Opus F4).** If I delete the wrapper, only the mock unit spec fails. In the paths that save (`BufWriteCmd` and `accept_current_diff`) the current buffer is always the proposal buffer, so the wrapper is defensive and no live answer changes. I kept the wrapper, because `wordcount()` reads the current buffer, and I added no live test for a case that no path reaches. - **Cost of `wordcount()` (Opus F5).** It adds a second pass over the buffer. For a 33 MB proposal that is about 0.1 to 0.2 s (0.12.3: +112 ms, 0.8.0: +216 ms), about the cost of `nvim_buf_get_lines` plus the join. I accept that for one save. ## Blast Radius - User-visible: a proposal buffer that the user emptied is accepted as an empty string. Every other buffer gives the same text as before. - `wordcount()` runs in the proposal buffer through `nvim_buf_call`. It does not change the user's current buffer or window. Both functions exist in Neovim 0.8. I ran the specs on Neovim 0.12.3 only. The CI job `integration-tests (v0.8.0)` runs the new spec on 0.8. - `covered.lock`: the change touches `_resolve_diff_as_saved`, a covered block. `check.sh` printed STALE for the `diff.lua` file hash and the `resolve_saved` block hash only. It printed no changed call site, and all 21 code facts stay TRUE. The guard that lets only a `pending` diff resolve is unchanged, and so are the status change to `saved` and the one call of the resolution callback. The change only decides whether the `text` string of the `FILE_SAVED` answer ends in a line break. The model stores which kind of answer a call got, not its text, so no modelled transition changes. The model passed (`main`: 89205 distinct states, `live-DisconnectedCallsAnswered`: 5269). `check.sh --record` wrote the new lock and `check.sh --hash-only` reports that it matches. ### Merge with coder#542, coder#539 and coder#535 coder#542 (F14) landed first, on 2026-11-12 at 04:36Z, and coder#539 (F12) at 04:46Z. coder#538 and coder#550 later changed `diff.lua` too, and coder#549 renamed "proposed buffer" to "proposal buffer" in the README. This PR merged `origin/main` (9bfc39c) with merge commits. It ran `check.sh --record` after each change to `diff.lua` and kept main's README wording in the last one. Only the `diff.lua` file hash and the `resolve_saved` block hash differ from the lock on `main`. The full `check.sh` passes and `check.sh --hash-only` matches. - One README bullet replaces this branch's bullet and coder#542's bullet, because the two said almost the same sentence and this branch's sentence about one empty line was false once `'eol'` can be off. - coder#535 (F11) is still open. It touches neither `diff.lua` nor this README bullet. Its current head (`a5dfc81b`, with `main` merged) merges with this PR without a conflict. ## Verification Red before the fix, green after (same specs, only `lua/claudecode/diff.lua` swapped, on the merged head, so the red column is `main` at 9bfc39c with coder#542): | Spec | `main` (with coder#542) | with the fix | | --- | --- | --- | | `tests/unit/diff_saved_text_spec.lua` (4 cases, mock) | 3 successes, 1 failure | 4 successes, 0 failures | | `tests/integration/diff_accept_emptied_buffer_spec.lua` (40 cases, real Neovim, `:write`) | 28 successes, 12 failures | 40 successes, 0 failures | Against the older merge base (5acc100, before coder#542), the 38-case spec of round a had 24 red cases. Today's 40-case spec has 26 there: the 24 emptied cases plus the new control for an unedited empty proposal in both layouts. coder#542 fixes the 12 cases from proposals without a final line break (it sets `'eol'` off), so 12 stay red on `main`. The integration spec opens a real `openDiff` and edits the proposal window. For the `vertical` and `horizontal` layouts it runs 6 proposals (two lines and a final line break, one line and a final line break, one blank line, one line with no final line break, two lines with no final line break, and no text) with 2 ways to empty the buffer (`ggdG` and `:%delete`). Each case compares the answer with what Neovim's own `:w` writes: the spec edits a real file buffer with the same text and commands, runs `:write`, and reads the file bytes, which are empty. Five controls per layout give the right text before and after (a blank line left in the buffer, no edit, an empty proposal without an edit, one line deleted, and text typed into the emptied buffer). In the `unified` layout, 6 controls check that the answer is the proposal. Gates on the final commit, which merges `origin/main` (9bfc39c) into the branch (private XDG folders): - `mise run all`: native exit 0. luacheck: 0 warnings / 0 errors in 183 files. busted: 1710 successes / 0 failures / 0 errors / 26 pending. treefmt: 0 files changed. - `mise run format-check`: native exit 0 (360 files, 0 changed). - Diff specs (`tests/unit/diff*_spec.lua`, `opendiff_blocking_spec`, `new_file_reject_then_reopen_spec`, and the `open_diff_mcp`, `close_tab` and `close_all_diff_tabs` specs): 226 successes, 0 failures. - `scripts/run_integration_tests_individually.sh`: 457 successes, 0 failed, 0 errors. The new spec gives 40 successes. - `.agents/skills/verify-diff-lifecycle/check.sh --hash-only`: covered.lock matches the code. Two independent verifiers (Opus and GPT) checked the code at `ef8be297`. Both found the code correct. Their blockers were the README sentence and the stale base, and this head fixes both. --- _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.
Comment-only follow-up of #538. No code line changes.
Why
The comments on the workaround for LuaJIT issue #1066 (the call before the rethrow in
_setup_blocking_diff, and the eager-JIT case indiff_failed_setup_guards_spec.lua) named Neovim 0.8.0 as the only affected version. That is too narrow. Someone who moves the minimum version to 0.9 would read "0.8.0", conclude the call is dead, and delete it.The comments now say "a LuaJIT older than September 2023 (bundled by Neovim 0.8 and 0.9)".
Evidence
v0.8.0-1210-gd367ed9b2)LuaJIT 2.1.1692716794)luajit1066b.lua: 10 abort of 10 runs. This spec on the product code without the call, 4 parallel loops of 8: 29 bad of 32. With the call: 0 bad of 32LuaJIT 2.1.1774638290The 0.9.5 runs used the official
nvim-linux64.tar.gzof v0.9.5. "Bad" means the spec printed no plenary summary. The 0.10 and 0.11 rows come from comparing the LuaJIT commit incmake.deps/deps.txtat each tag with e86990f7f. I did not run those releases.Checks
lua/claudecode/diff.luaand in the spec (a diff with comment lines removed is empty).covered.lockis re-recorded.check.shread STALE before the record (thesetup_blockingblock hash changed). Its model passes (main 89205 distinct states, live-DisconnectedCallsAnswered 5269) and no fact is FALSE.--hash-onlymatches after the record.luacheck lua/ tests/ --no-unused-args --no-max-line-length: 0 warnings, 0 errors in 181 files.stylua --check lua tests: clean.busted --shuffle --seed=8888): 1702 successes, 0 failures, 0 errors, 25 pending.scripts/run_integration_tests_individually.sh: Neovim 0.12.3 Total Success 417, Failed 0, Errors 0. Neovim 0.8.0 Total Success 309, Failed 0, Errors 0.Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh