Conversation
Research notes from analyzing the JetBrains Sweep AI plugin architecture, including context building strategies, prompt formats, and LSP integration patterns for designing a Neovim equivalent.
- Plugin entry point with commands (enable/disable/toggle/status/debug) - Configuration module with typed defaults for keymaps, context, server, UI - Test harness setup with plenary.test - Initial config_spec tests Targeting Neovim 0.10+ with plenary.nvim dependency.
Implement context module for sweep.nvim that extracts rich context: - LSP integration for definitions and type info via textDocument/definition and hover - Treesitter integration for scope detection (function, class, method) - Import/require statement extraction for multiple languages - Formatted output with prioritized sections for prompts - Async API with callbacks for LSP operations - Graceful handling of missing LSP/treesitter - Content truncation to configurable limits
This is the core orchestration module for sweep.nvim that ties together the FIM, HTTP, parser, and UI modules. Implements: - Debounced trigger mechanism using vim.loop timers - Request cancellation for in-flight HTTP requests - accept_full/accept_line/accept_word for partial completion acceptance - dismiss for clearing without insertion - Filetype exclusion support - Enable/disable/toggle functionality Tests cover debouncing, request management, text insertion, and error handling.
Implements keymaps for sweep.nvim that allow users to: - Trigger completion manually (insert mode) - Accept full, line, or word completions (insert mode, only when visible) - Dismiss completions (insert mode, only when visible) Uses expr mappings for conditional behavior that passes through the key when no completion is visible. Includes comprehensive TDD tests.
Configure sweep.nvim with keybindings matching previous llama.vim setup: - <C-c> trigger, <C-s> accept full, <C-l> accept line, <C-w> accept word - Same highlight color (#f2cdcd) for ghost text - Uses LLAMA_SERVER_URL env var with localhost:8012 default
📝 WalkthroughWalkthroughAdds sweep.nvim: a new Neovim AI-autocomplete plugin (replacing a commented-out llama.vim entry) with config, lifecycle, completion pipeline, context extraction, caching, HTTP client for llama.cpp, UI, autocmds/keymaps, debug pane, and extensive unit tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Editor as Neovim Editor
participant Autocmd as Autocmds
participant Completion as Completion
participant FIM as FIM Builder
participant Context as Context/Ring/Edits
participant HTTP as HTTP Client
participant Server as llama.cpp Server
participant Parser as Parser
participant UI as UI Module
Editor->>Autocmd: CursorMovedI / Keymap event
Autocmd->>Completion: trigger()
activate Completion
Completion->>Completion: debounce timer
Completion->>FIM: build_request(buf context)
FIM-->>Completion: fim request
Completion->>Context: gather context (LSP/treesitter/ring/edits)
Context-->>Completion: context payload
Completion->>HTTP: request(prompt + params)
deactivate Completion
activate HTTP
HTTP->>Server: POST /completion
Server->>Server: inference
Server-->>HTTP: response (json)
HTTP->>Parser: parse(response)
deactivate HTTP
activate Parser
Parser-->>Completion: parsed result (lines, tokens, timings)
deactivate Parser
activate Completion
Completion->>Cache: store(result)
Completion->>UI: show(lines, info)
deactivate Completion
UI->>Editor: set extmarks (virt_text/virt_lines)
Editor->>Completion: accept_full/accept_line/accept_word
Completion->>Editor: insert text / clear UI
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Fix SCOPE_NODE_TYPES duplicate keys bug (Lua table overwrites) - Remove unnecessary JSON encode/decode round-trip in completion - Make parser.parse() accept both string and table input - Remove dead code: unreachable cancellation check in http callback - Remove dead code: unused get_treesitter_parser function - Remove dead code: unused in_import_section variable - Remove dead code: empty vim.schedule wrapper in error handler - Simplify redundant nil check in parser - Add entry counter to cache for O(1) count operations - Reuse parser.first_word() in completion accept_word - Clean up unused variable captures in treesitter range extraction
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In `@sweep-ai-autocomplete-analysis.md`:
- Line 15: Add language identifiers to all fenced code blocks flagged by
markdownlint MD040; for each triple-backtick block (e.g., the "JetBrains IDE"
sample and the other fenced examples) replace ``` with a language tag like
```text, ```python, ```lua, or ```diff as appropriate so the blocks are
annotated; ensure each code fence in the document is updated consistently to
remove the MD040 warnings.
In `@sweep.nvim/lua/sweep/cache.lua`:
- Around line 97-135: The set() path can loop infinitely when config.max_entries
<= 0; update M.setup to ensure config.max_entries is clamped to a sensible
minimum (e.g., 1) or set a flag to disable caching, and modify M.set to
short‑circuit when config.max_entries <= 0 (or caching disabled) so it doesn't
enter the while entry_count() >= config.max_entries loop; reference M.setup to
clamp config.max_entries and M.set (and functions entry_count and evict_lru) to
guard/return early when max_entries is non‑positive.
In `@sweep.nvim/lua/sweep/completion.lua`:
- Around line 59-67: The cancel_debounce_timer function currently stops and nils
state.debounce_timer but doesn't close the libuv handle; update
cancel_debounce_timer to call state.debounce_timer:stop() (if active) and then
state.debounce_timer:close() before setting state.debounce_timer = nil so the
underlying vim.loop timer handle is released; ensure you check that
state.debounce_timer has a :close method (i.e., non-nil) and call close in all
paths where the timer is discarded.
- Around line 250-251: The code sets state.current_handle via http.request by
concatenating cfg.server.endpoint .. '/infill', which can produce a double slash
if cfg.server.endpoint ends with '/' (e.g., "http://localhost:8012/"); update
the URL construction used in the http.request call to normalize the endpoint
first (trim any trailing slash from cfg.server.endpoint or use a join that
ensures a single separator) before appending '/infill' so the request URL is
always valid; adjust the logic around state.current_handle assignment and the
http.request invocation accordingly.
In `@sweep.nvim/lua/sweep/config.lua`:
- Around line 32-46: The SweepConfig annotation and M.defaults disagree about
where show_info lives; either add a top-level show_info field to the SweepConfig
typedef or move/remove the top-level default and keep show_info under
SweepUIConfig (and update M.defaults accordingly). Locate the SweepConfig
annotation and the M.defaults table (symbols: SweepConfig, M.defaults) and
ensure the schema and defaults match: if you intend a top-level flag, add
"@@field show_info boolean" to SweepConfig and keep the key in M.defaults; if
you intend it under ui, remove the top-level show_info from M.defaults and
ensure SweepUIConfig and its "@field show_info boolean" exist and are documented
(also update the duplicate instance at the second occurrence around the other
defaults noted in the comment).
In `@sweep.nvim/lua/sweep/context.lua`:
- Around line 185-236: The get_scope function currently hardcodes column 0 when
calling vim.treesitter.get_node which can return an outer scope on indented
lines; update the signature of M.get_scope(bufnr, row, col, opts) to accept an
optional col (default 0) and use pos = { row, col } when calling
vim.treesitter.get_node, then update both callers M.get(...) and
M.get_async(...) to pass their col argument through to M.get_scope so the
correct node at the cursor column is used for scope detection.
- Around line 387-440: The hover handling in M.get_type_info doesn't handle
array-formatted HoverContents (MarkedString[]) and only inspects strings/tables,
so update the callback passed to vim.lsp.buf_request to detect when
result.contents is a list: if type(contents) == 'table' and
vim.tbl_islist(contents) then pick the first element (or join multiple) and
extract its string or .value/.kind similarly; otherwise keep existing
string/table handling—assign the extracted value to type_info and call
callback(type_info) once. Ensure you reference M.get_type_info, the local
variables contents and type_info, and the vim.lsp.buf_request response handler
when making this change.
In `@sweep.nvim/lua/sweep/http.lua`:
- Around line 25-156: The bug in M.request is that active_requests[request_id]
is removed before any vim.schedule callbacks run, so a cancel() after response
but before the scheduled on_success/on_error cannot suppress callbacks; change
the lifecycle so the request entry remains until scheduled callbacks finish by
adding and using a completed flag (e.g., active_requests[request_id] = { job =
job, cancelled = false, completed = false }), only mark completed = true and
then remove the entry inside the vim.schedule wrapper(s) after double-checking
cancelled (in both on_success and on_error paths), and ensure cancel() sets
cancelled = true but does not remove the entry until completed is true.
In `@sweep.nvim/lua/sweep/keymaps.lua`:
- Around line 30-121: The conditional callback currently returns termcodes via
vim.api.nvim_replace_termcodes (in make_conditional_callback), which disables
user remapped mappings on passthrough; change make_conditional_callback to
return the raw key string (do not call nvim_replace_termcodes) when completion
is not visible, and update the vim.keymap.set calls in setup_buffer_keymaps for
the conditional mappings (those using km.accept_full, km.accept_line,
km.accept_word, km.dismiss) to include remap = true in their options so returned
keys honor existing user mappings and snippets.
In `@sweep.nvim/lua/sweep/ring.lua`:
- Around line 72-79: M.setup currently assigns config.max_chunks and
config.chunk_size directly from opts which can allow zero or negative values
causing modulo-by-zero in M.add/M.get; clamp config.max_chunks to a safe minimum
of 1 (and similarly clamp config.chunk_size to at least 1) inside M.setup after
reading opts, e.g. enforce config.max_chunks = math.max(1, opts.max_chunks or
16) and config.chunk_size = math.max(1, opts.chunk_size or 64), then call
M.clear(); ensure any dependent logic in M.add and M.get can rely on these
invariants.
In `@sweep.nvim/README.md`:
- Line 136: Update the README entry for the edits module: replace the inaccurate
description "Applies accepted completions to buffer" with a concise, accurate
description such as "Tracks edit history and provides context for next-edit
prediction" (mentioning that actual completion application is handled by modules
like completion or ui); target the `edits` module line in the README and ensure
the wording distinguishes its role from `completion` / `ui`.
In `@sweep.nvim/tests/sweep/http_spec.lua`:
- Around line 342-434: The test's mock_curl.post creates a local job but never
assigns it to the global mock_job, so job1/job2 become nil and calling
_trigger_callback fails; modify the mock_curl.post in the second spec to capture
the created job (e.g., local job = create_mock_job() or create_mock_job({...}) )
and either assign mock_job = job or return that same job and set local job1/job2
= mock_job after calling http.request; specifically update the mock_curl.post
closure used in the "should not call any callbacks after cancel_all" test to set
mock_job (or return and store the created job) before wiring stored_callback so
job1._trigger_callback and job2._trigger_callback are valid.
🧹 Nitpick comments (12)
sweep.nvim/lua/sweep/edits.lua (1)
241-248:detachdoesn't actually detach fromnvim_buf_attach.This function clears local state but doesn't actually stop the
on_linescallback from firing. To properly detach, you'd need to returntruefromon_linesoron_bytesin a subsequent callback. Consider documenting this limitation or implementing proper detachment by storing a flag that causeson_linesto returntrueon next invocation.♻️ Optional: Implement proper detachment
---@type table<number, boolean> Track which buffers are attached local attached_buffers = {} + +---@type table<number, boolean> Track buffers pending detach +local pending_detach = {} -- In on_lines callback: + if pending_detach[buf] then + pending_detach[buf] = nil + return true -- Returning true detaches the callback + end --- Stop tracking a buffer ---@param bufnr number function M.detach(bufnr) attached_buffers[bufnr] = nil buffer_content[bufnr] = nil + pending_detach[bufnr] = true -- Note: nvim_buf_attach with on_detach handles cleanup automatically -- when buffer is deleted. Manual detach is for explicit stop tracking. endsweep.nvim/tests/minimal_init.lua (1)
7-24: Minor redundancy in plenary path fallbacks.Line 14 (
~/.local/share/nvim/lazy/plenary.nvim) typically resolves to the same path as line 8 (vim.fn.stdpath('data') .. '/lazy/plenary.nvim'). This is harmless but could be removed for clarity.♻️ Remove redundant path
local alt_paths = { - vim.fn.expand('~/.local/share/nvim/lazy/plenary.nvim'), vim.fn.expand('~/.local/share/nvim/site/pack/packer/start/plenary.nvim'), '/usr/share/nvim/site/pack/packer/start/plenary.nvim', }sweep.nvim/tests/sweep/ring_spec.lua (1)
65-66: Simplify the assertion for better readability.The assertion on line 65 uses a convoluted pattern. Consider using the simpler and more direct approach that's already used on line 66.
♻️ Simplify assertion
- assert.is_nil(vim.tbl_contains(contents, 'chunk1') and 'chunk1' or nil) + assert.is_false(vim.tbl_contains(contents, 'chunk1')) assert.is_true(vim.tbl_contains(contents, 'chunk5'))sweep.nvim/tests/sweep/edits_spec.lua (1)
406-411: Consider tightening the attach success assertion.The assertion
result == true or result == nil or resultis very permissive. Sinceattachusespcallinternally and the function is documented to returnboolean success, consider asserting strictlyassert.is_true(result)for clearer test intent.♻️ Tighten assertion
it('should return true when attach is called', function() -- attach returns success indicator local result = edits.attach(1) - assert.is_true(result == true or result == nil or result) + assert.is_true(result) end)sweep.nvim/tests/sweep/cache_spec.lua (1)
146-205: Consider stubbing time instead ofvim.waitin TTL tests.
Real sleeps can slow the suite and be flaky on busy CI. If the cache can accept a time provider (or expose anow_msoverride), you can advance time deterministically without waiting.sweep.nvim/lua/sweep/keymaps.lua (1)
160-167: Avoid mutatingbuffer_keymapsduring iteration.
Clearing entries inside thepairs()loop can skip buffers. Iterate over a snapshot of keys for predictable teardown.♻️ Safer iteration during teardown
- for bufnr, _ in pairs(buffer_keymaps) do - teardown_buffer_keymaps(bufnr) - end + for _, bufnr in ipairs(vim.tbl_keys(buffer_keymaps)) do + teardown_buffer_keymaps(bufnr) + endsweep.nvim/lua/sweep/fim.lua (1)
68-187: Avoid reading the entire buffer for each request.
nvim_buf_get_lines(bufnr, 0, -1, ...)pulls the whole file on every completion. For large buffers this is expensive. Usenvim_buf_line_countplus targetedget_linesfor just the prefix/suffix ranges.♻️ Example refactor
- -- Get all buffer lines - local total_lines = vim.api.nvim_buf_get_lines(bufnr, 0, -1, false) - local line_count = `#total_lines` + -- Get line count without loading entire buffer + local line_count = vim.api.nvim_buf_line_count(bufnr) @@ - local current_line = total_lines[row + 1] or '' + local current_line = (vim.api.nvim_buf_get_lines(bufnr, row, row + 1, false)[1]) or '' @@ - for i = prefix_start, row - 1 do - local line = total_lines[i + 1] - if line then - table.insert(prefix_parts, line) - end - end + local prefix_lines_buf = vim.api.nvim_buf_get_lines(bufnr, prefix_start, row, false) + for _, line in ipairs(prefix_lines_buf) do + table.insert(prefix_parts, line) + end @@ - for i = row + 1, suffix_end do - local line = total_lines[i + 1] - if line then - table.insert(suffix_parts, line) - end - end + local suffix_lines_buf = vim.api.nvim_buf_get_lines(bufnr, row + 1, suffix_end + 1, false) + for _, line in ipairs(suffix_lines_buf) do + table.insert(suffix_parts, line) + endsweep.nvim/lua/sweep/completion.lua (3)
149-153: Remove no-opvim.schedulecall.This
vim.schedulewraps an empty function body (the notify is commented out). Either remove it entirely or enable debug logging conditionally.Proposed fix
-- Update pending state state.pending = false - -- Log the error (optional) - vim.schedule(function() - -- Uncomment for debugging: - -- vim.notify('Sweep completion error: ' .. error_msg, vim.log.levels.DEBUG) - end) + -- Log error in debug mode + local cfg = config.get() + if cfg.debug then + vim.schedule(function() + vim.notify('Sweep completion error: ' .. error_msg, vim.log.levels.DEBUG) + end) + end
405-412: Word pattern may be too restrictive for code completion.The pattern
[%w_]+only matches alphanumerics and underscores. Common code tokens likefoo-bar(CSS),foo.bar(method chains), orfoo::bar(C++/Rust) would only capture the first segment. Consider if this is intentional or if a more permissive pattern would be preferable.
97-100: Remove unnecessary JSON re-encoding in parser call.The
responseparameter is already a parsed table (as documented in the function signature). Re-encoding it to JSON withvim.json.encode()only forparser.parse()to immediately decode it again withvim.json.decode()is wasteful. Either haveparser.parse()accept tables directly, or pass the raw JSON string from the HTTP layer instead.sweep.nvim/lua/sweep/init.lua (2)
24-29: Cache configuration is hardcoded.The ring buffer uses configurable values with fallback defaults, but the cache uses hardcoded values (
max_entries = 100,ttl_ms = 60000). Consider exposing these in the config for consistency.Proposed fix
-- Initialize cache local cache = require('sweep.cache') cache.setup({ - max_entries = 100, - ttl_ms = 60000, -- 1 minute TTL + max_entries = cfg.cache and cfg.cache.max_entries or 100, + ttl_ms = cfg.cache and cfg.cache.ttl_ms or 60000, })
79-88: Minor redundancy: UI cleared twice.
completion.disable()already callsM.cancel()which invokesui.clear(). The explicitui.clear()on line 88 is redundant, though harmless.Proposed fix
-- Disable completion module (cancels pending requests) local completion = require('sweep.completion') completion.disable() -- Tear down autocmds and keymaps require('sweep.autocmds').teardown() require('sweep.keymaps').teardown() - -- Clear UI - require('sweep.ui').clear() -
|
|
||
| Sweep AI uses a **custom inference engine** rather than calling external APIs like OpenAI/Anthropic: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks (MD040).
markdownlint flags multiple fenced blocks without a language tag; please annotate them (e.g., text, python, lua, diff).
✅ Example (apply similarly to other blocks)
-```
+```text
┌─────────────────────────────────────────────────────────────────┐
│ JetBrains IDE │
...
-```
+```Also applies to: 64-64, 104-104, 147-147, 158-158, 196-196, 222-222, 277-277, 290-290, 353-353
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
15-15: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@sweep-ai-autocomplete-analysis.md` at line 15, Add language identifiers to
all fenced code blocks flagged by markdownlint MD040; for each triple-backtick
block (e.g., the "JetBrains IDE" sample and the other fenced examples) replace
``` with a language tag like ```text, ```python, ```lua, or ```diff as
appropriate so the blocks are annotated; ensure each code fence in the document
is updated consistently to remove the MD040 warnings.
| --- Initialize the cache with options | ||
| ---@param opts? table | ||
| function M.setup(opts) | ||
| opts = opts or {} | ||
| config = vim.tbl_deep_extend('force', defaults, opts) | ||
|
|
||
| -- Clear cache on setup | ||
| entries = {} | ||
| access_order = {} | ||
| stats = { | ||
| hits = 0, | ||
| misses = 0, | ||
| evictions = 0, | ||
| } | ||
| end | ||
|
|
||
| --- Store a value in the cache | ||
| ---@param key string | ||
| ---@param value any | ||
| function M.set(key, value) | ||
| if value == nil then | ||
| M.remove(key) | ||
| return | ||
| end | ||
|
|
||
| local existing = entries[key] | ||
| local current_time = now_ms() | ||
|
|
||
| if existing then | ||
| -- Update existing entry | ||
| existing.value = value | ||
| existing.created_at = current_time | ||
| existing.accessed_at = current_time | ||
| touch(key) | ||
| else | ||
| -- Check if we need to evict | ||
| while entry_count() >= config.max_entries do | ||
| evict_lru() | ||
| end |
There was a problem hiding this comment.
Prevent infinite loop when max_entries <= 0.
set() can spin forever if max_entries is 0 or negative. Clamp to a valid minimum or short‑circuit caching in that case.
🔧 Suggested fix
function M.setup(opts)
opts = opts or {}
config = vim.tbl_deep_extend('force', defaults, opts)
+ if config.max_entries < 1 then
+ config.max_entries = 0
+ end
-- Clear cache on setup
entries = {}
access_order = {}
stats = {
@@
function M.set(key, value)
+ if config.max_entries == 0 then
+ return
+ end
if value == nil then
M.remove(key)
return
end🤖 Prompt for AI Agents
In `@sweep.nvim/lua/sweep/cache.lua` around lines 97 - 135, The set() path can
loop infinitely when config.max_entries <= 0; update M.setup to ensure
config.max_entries is clamped to a sensible minimum (e.g., 1) or set a flag to
disable caching, and modify M.set to short‑circuit when config.max_entries <= 0
(or caching disabled) so it doesn't enter the while entry_count() >=
config.max_entries loop; reference M.setup to clamp config.max_entries and M.set
(and functions entry_count and evict_lru) to guard/return early when max_entries
is non‑positive.
| --- Cancel the debounce timer if active | ||
| local function cancel_debounce_timer() | ||
| if state.debounce_timer then | ||
| if state.debounce_timer:is_active() then | ||
| state.debounce_timer:stop() | ||
| end | ||
| state.debounce_timer = nil | ||
| end | ||
| end |
There was a problem hiding this comment.
Timer handle not closed – potential resource leak.
In libuv (via vim.loop), timers should be closed with :close() after stopping to release the underlying handle. Repeatedly creating timers without closing them can leak resources over time.
Proposed fix
local function cancel_debounce_timer()
if state.debounce_timer then
if state.debounce_timer:is_active() then
state.debounce_timer:stop()
end
+ state.debounce_timer:close()
state.debounce_timer = nil
end
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --- Cancel the debounce timer if active | |
| local function cancel_debounce_timer() | |
| if state.debounce_timer then | |
| if state.debounce_timer:is_active() then | |
| state.debounce_timer:stop() | |
| end | |
| state.debounce_timer = nil | |
| end | |
| end | |
| --- Cancel the debounce timer if active | |
| local function cancel_debounce_timer() | |
| if state.debounce_timer then | |
| if state.debounce_timer:is_active() then | |
| state.debounce_timer:stop() | |
| end | |
| state.debounce_timer:close() | |
| state.debounce_timer = nil | |
| end | |
| end |
🤖 Prompt for AI Agents
In `@sweep.nvim/lua/sweep/completion.lua` around lines 59 - 67, The
cancel_debounce_timer function currently stops and nils state.debounce_timer but
doesn't close the libuv handle; update cancel_debounce_timer to call
state.debounce_timer:stop() (if active) and then state.debounce_timer:close()
before setting state.debounce_timer = nil so the underlying vim.loop timer
handle is released; ensure you check that state.debounce_timer has a :close
method (i.e., non-nil) and call close in all paths where the timer is discarded.
sweep.nvim/lua/sweep/completion.lua
Outdated
| state.current_handle = http.request({ | ||
| endpoint = cfg.server.endpoint .. '/infill', |
There was a problem hiding this comment.
Potential double-slash in endpoint URL.
If cfg.server.endpoint ends with a trailing slash (e.g., http://localhost:8012/), concatenating /infill produces http://localhost:8012//infill. Consider normalizing the endpoint.
Proposed fix
+ -- Normalize endpoint (remove trailing slash if present)
+ local base_endpoint = cfg.server.endpoint:gsub('/$', '')
+
-- Make HTTP request
state.current_handle = http.request({
- endpoint = cfg.server.endpoint .. '/infill',
+ endpoint = base_endpoint .. '/infill',
body = request_body,🤖 Prompt for AI Agents
In `@sweep.nvim/lua/sweep/completion.lua` around lines 250 - 251, The code sets
state.current_handle via http.request by concatenating cfg.server.endpoint ..
'/infill', which can produce a double slash if cfg.server.endpoint ends with '/'
(e.g., "http://localhost:8012/"); update the URL construction used in the
http.request call to normalize the endpoint first (trim any trailing slash from
cfg.server.endpoint or use a join that ensures a single separator) before
appending '/infill' so the request URL is always valid; adjust the logic around
state.current_handle assignment and the http.request invocation accordingly.
| ---@class SweepConfig | ||
| ---@field auto_enable boolean Auto-enable on setup | ||
| ---@field debounce_ms number Debounce delay in milliseconds | ||
| ---@field keymaps SweepKeymaps | ||
| ---@field context SweepContextConfig | ||
| ---@field server SweepServerConfig | ||
| ---@field ui SweepUIConfig | ||
| ---@field filetypes_exclude string[] Filetypes to exclude | ||
|
|
||
| ---@type SweepConfig | ||
| M.defaults = { | ||
| auto_enable = true, | ||
| debounce_ms = 100, | ||
| show_info = true, | ||
|
|
There was a problem hiding this comment.
Align SweepConfig schema with show_info usage.
There’s a top‑level show_info default but the SweepConfig annotation doesn’t include it, and ui.show_info exists too. Please consolidate or document the intended source of truth to avoid ambiguity.
💡 Minimal fix to keep annotations accurate
---@class SweepConfig
---@field auto_enable boolean Auto-enable on setup
---@field debounce_ms number Debounce delay in milliseconds
+---@field show_info boolean Show completion info (global)
---@field keymaps SweepKeymaps
---@field context SweepContextConfig
---@field server SweepServerConfig
---@field ui SweepUIConfig
---@field filetypes_exclude string[] Filetypes to excludeAlso applies to: 72-76
🤖 Prompt for AI Agents
In `@sweep.nvim/lua/sweep/config.lua` around lines 32 - 46, The SweepConfig
annotation and M.defaults disagree about where show_info lives; either add a
top-level show_info field to the SweepConfig typedef or move/remove the
top-level default and keep show_info under SweepUIConfig (and update M.defaults
accordingly). Locate the SweepConfig annotation and the M.defaults table
(symbols: SweepConfig, M.defaults) and ensure the schema and defaults match: if
you intend a top-level flag, add "@@field show_info boolean" to SweepConfig and
keep the key in M.defaults; if you intend it under ui, remove the top-level
show_info from M.defaults and ensure SweepUIConfig and its "@field show_info
boolean" exist and are documented (also update the duplicate instance at the
second occurrence around the other defaults noted in the comment).
| --- Make a completion request to llama.cpp server | ||
| ---@param opts table Request options | ||
| ---@param opts.endpoint string Server endpoint URL | ||
| ---@param opts.body table Request body to be JSON encoded | ||
| ---@param opts.timeout? number Request timeout in milliseconds (default: 5000) | ||
| ---@param opts.on_success function Callback for successful response: function(response) | ||
| ---@param opts.on_error function Callback for error: function(error_message) | ||
| ---@return table|nil handle Request handle for cancellation, or nil on error | ||
| function M.request(opts) | ||
| if not opts or not opts.endpoint or not opts.body then | ||
| if opts and opts.on_error then | ||
| opts.on_error('Missing required options: endpoint and body') | ||
| end | ||
| return nil | ||
| end | ||
|
|
||
| local request_id = generate_request_id() | ||
| local timeout = opts.timeout or DEFAULT_TIMEOUT | ||
|
|
||
| -- Encode body as JSON | ||
| local body_json | ||
| local ok, result = pcall(vim.json.encode, opts.body) | ||
| if not ok then | ||
| if opts.on_error then | ||
| opts.on_error('Failed to encode request body as JSON: ' .. tostring(result)) | ||
| end | ||
| return nil | ||
| end | ||
| body_json = result | ||
|
|
||
| -- Create the request entry before making the call | ||
| active_requests[request_id] = { | ||
| job = nil, | ||
| cancelled = false, | ||
| } | ||
|
|
||
| -- Make the async HTTP request | ||
| local job = curl.post({ | ||
| url = opts.endpoint, | ||
| body = body_json, | ||
| headers = { | ||
| ['Content-Type'] = 'application/json', | ||
| ['Accept'] = 'application/json', | ||
| }, | ||
| timeout = timeout, | ||
| callback = function(response) | ||
| -- Check if request was cancelled | ||
| local request = active_requests[request_id] | ||
| if not request or request.cancelled then | ||
| -- Request was cancelled, don't call callbacks | ||
| active_requests[request_id] = nil | ||
| return | ||
| end | ||
|
|
||
| -- Remove from active requests | ||
| active_requests[request_id] = nil | ||
|
|
||
| -- Handle response | ||
| if not response then | ||
| if opts.on_error then | ||
| vim.schedule(function() | ||
| opts.on_error('No response received from server') | ||
| end) | ||
| end | ||
| return | ||
| end | ||
|
|
||
| -- Check for connection errors (status 0 usually indicates connection failure) | ||
| if response.status == 0 then | ||
| local error_msg = 'Connection failed' | ||
| if response.exit then | ||
| if response.exit == 7 then | ||
| error_msg = 'Connection refused - is the server running?' | ||
| elseif response.exit == 28 then | ||
| error_msg = 'Request timed out' | ||
| else | ||
| error_msg = string.format('Connection error (exit code: %d)', response.exit) | ||
| end | ||
| end | ||
| if opts.on_error then | ||
| vim.schedule(function() | ||
| opts.on_error(error_msg) | ||
| end) | ||
| end | ||
| return | ||
| end | ||
|
|
||
| -- Check for HTTP error status | ||
| if response.status < 200 or response.status >= 300 then | ||
| if opts.on_error then | ||
| vim.schedule(function() | ||
| opts.on_error(string.format('HTTP error %d: %s', response.status, response.body or 'Unknown error')) | ||
| end) | ||
| end | ||
| return | ||
| end | ||
|
|
||
| -- Parse JSON response | ||
| local parse_ok, parsed = pcall(vim.json.decode, response.body) | ||
| if not parse_ok then | ||
| if opts.on_error then | ||
| vim.schedule(function() | ||
| opts.on_error('Failed to parse JSON response: ' .. tostring(parsed)) | ||
| end) | ||
| end | ||
| return | ||
| end | ||
|
|
||
| -- Success! | ||
| if opts.on_success then | ||
| vim.schedule(function() | ||
| -- Double-check cancellation in case it happened during schedule | ||
| local req = active_requests[request_id] | ||
| if req and req.cancelled then | ||
| return | ||
| end | ||
| opts.on_success(parsed) | ||
| end) | ||
| end | ||
| end, | ||
| }) | ||
|
|
||
| -- Store the job reference | ||
| if active_requests[request_id] then | ||
| active_requests[request_id].job = job | ||
| end | ||
|
|
||
| -- Return handle for cancellation | ||
| return { | ||
| id = request_id, | ||
| } | ||
| end |
There was a problem hiding this comment.
Late cancellation can’t prevent scheduled callbacks.
The request entry is removed before vim.schedule, so a cancel() after the response but before the scheduled callback can’t suppress stale on_success. Keep the entry until the scheduled callback completes (or track completed and only finalize afterward).
🛠️ Proposed fix
- -- Remove from active requests
- active_requests[request_id] = nil
+ -- Keep entry until callbacks finish (allow late cancellation)
+ local function finalize()
+ active_requests[request_id] = nil
+ end
@@
- if not response then
- if opts.on_error then
- vim.schedule(function()
- opts.on_error('No response received from server')
- end)
- end
- return
- end
+ if not response then
+ if opts.on_error then
+ vim.schedule(function()
+ if request.cancelled then return finalize() end
+ opts.on_error('No response received from server')
+ finalize()
+ end)
+ else
+ finalize()
+ end
+ return
+ end
@@
- if opts.on_success then
- vim.schedule(function()
- -- Double-check cancellation in case it happened during schedule
- local req = active_requests[request_id]
- if req and req.cancelled then
- return
- end
- opts.on_success(parsed)
- end)
- end
+ if opts.on_success then
+ vim.schedule(function()
+ if request.cancelled then return finalize() end
+ opts.on_success(parsed)
+ finalize()
+ end)
+ else
+ finalize()
+ end🤖 Prompt for AI Agents
In `@sweep.nvim/lua/sweep/http.lua` around lines 25 - 156, The bug in M.request is
that active_requests[request_id] is removed before any vim.schedule callbacks
run, so a cancel() after response but before the scheduled on_success/on_error
cannot suppress callbacks; change the lifecycle so the request entry remains
until scheduled callbacks finish by adding and using a completed flag (e.g.,
active_requests[request_id] = { job = job, cancelled = false, completed = false
}), only mark completed = true and then remove the entry inside the vim.schedule
wrapper(s) after double-checking cancelled (in both on_success and on_error
paths), and ensure cancel() sets cancelled = true but does not remove the entry
until completed is true.
| --- Create an expr mapping callback for conditional keymaps | ||
| --- Returns empty string if action taken, otherwise returns the key for passthrough | ||
| ---@param key string The key to pass through if completion not visible | ||
| ---@param action function The action to take when completion is visible | ||
| ---@return function | ||
| local function make_conditional_callback(key, action) | ||
| return function() | ||
| if get_ui().is_visible() then | ||
| action() | ||
| return '' | ||
| else | ||
| -- Pass through the original key | ||
| return vim.api.nvim_replace_termcodes(key, true, false, true) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| --- Create a simple callback (no visibility check) | ||
| ---@param action function The action to take | ||
| ---@return function | ||
| local function make_callback(action) | ||
| return function() | ||
| action() | ||
| return '' | ||
| end | ||
| end | ||
|
|
||
| --- Set up keymaps on the current buffer | ||
| ---@param bufnr? number Buffer number (defaults to current buffer) | ||
| local function setup_buffer_keymaps(bufnr) | ||
| bufnr = bufnr or vim.api.nvim_get_current_buf() | ||
|
|
||
| local cfg = get_config() | ||
| local km = cfg.keymaps | ||
|
|
||
| -- Track keymaps for this buffer | ||
| buffer_keymaps[bufnr] = buffer_keymaps[bufnr] or {} | ||
|
|
||
| -- Trigger keymap (no visibility check needed) | ||
| vim.keymap.set('i', km.trigger, make_callback(function() | ||
| get_completion().manual_trigger() | ||
| end), { | ||
| buffer = bufnr, | ||
| expr = true, | ||
| desc = 'Sweep: trigger completion', | ||
| silent = true, | ||
| }) | ||
| table.insert(buffer_keymaps[bufnr], { mode = 'i', lhs = km.trigger }) | ||
|
|
||
| -- Accept full keymap (only when visible) | ||
| vim.keymap.set('i', km.accept_full, make_conditional_callback(km.accept_full, function() | ||
| get_completion().accept_full() | ||
| end), { | ||
| buffer = bufnr, | ||
| expr = true, | ||
| desc = 'Sweep: accept full completion', | ||
| silent = true, | ||
| }) | ||
| table.insert(buffer_keymaps[bufnr], { mode = 'i', lhs = km.accept_full }) | ||
|
|
||
| -- Accept line keymap (only when visible) | ||
| vim.keymap.set('i', km.accept_line, make_conditional_callback(km.accept_line, function() | ||
| get_completion().accept_line() | ||
| end), { | ||
| buffer = bufnr, | ||
| expr = true, | ||
| desc = 'Sweep: accept first line', | ||
| silent = true, | ||
| }) | ||
| table.insert(buffer_keymaps[bufnr], { mode = 'i', lhs = km.accept_line }) | ||
|
|
||
| -- Accept word keymap (only when visible) | ||
| vim.keymap.set('i', km.accept_word, make_conditional_callback(km.accept_word, function() | ||
| get_completion().accept_word() | ||
| end), { | ||
| buffer = bufnr, | ||
| expr = true, | ||
| desc = 'Sweep: accept first word', | ||
| silent = true, | ||
| }) | ||
| table.insert(buffer_keymaps[bufnr], { mode = 'i', lhs = km.accept_word }) | ||
|
|
||
| -- Dismiss keymap (only when visible) | ||
| vim.keymap.set('i', km.dismiss, make_conditional_callback(km.dismiss, function() | ||
| get_completion().dismiss() | ||
| end), { | ||
| buffer = bufnr, | ||
| expr = true, | ||
| desc = 'Sweep: dismiss completion', | ||
| silent = true, | ||
| }) | ||
| table.insert(buffer_keymaps[bufnr], { mode = 'i', lhs = km.dismiss }) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Neovim nvim_replace_termcodes expr mapping noremap behavior return value
💡 Result:
vim.api.nvim_replace_termcodes(str, from_part, do_lt, special)only converts key-notation like<CR>,<Esc>, etc. into Neovim’s internal keycode bytes and returns a string. It does not execute/“type” anything by itself. Use it only when you need the converted bytes (or usevim.keycode()instead). [1]- In an
exprmapping, Neovim uses the return value of your Lua callback / Vimscript expression as the keys to be fed as if typed (so you typically justreturn '<C-u>zz').vim.keymap.set(..., { expr = true })does this behavior. [2] - For
exprmappings created via Lua,vim.keymap.set()makes this easier because{ replace_keycodes = true }is the default whenexpr = true, meaning returned strings like'<CR>'/'<C-u>'are automatically keycode-replaced (so you usually do not neednvim_replace_termcodes()yourself). [3] noremapvsremapbehavior: by defaultvim.keymap.set()is non-recursive (remap = false, i.e. like:noremap), so the produced{rhs}/ returned keys are not further remapped. If you want the result to be remapped through other mappings, set{ remap = true }. [2]- If you call
nvim_replace_termcodes()inside a Lua mapping but don’t return it (forexpr) and don’t send it with an input API, nothing will happen—conversion alone doesn’t feed keys. (That’s whyreturn ...matters forexpr, or otherwise you’d use an input/feed API.) [1]
Sources: Neovim API docs for nvim_replace_termcodes() return value/behavior [1]; Neovim Lua guide on expr mappings and remap default [2]; vim.keymap.set() docs on replace_keycodes default with expr [3].
🌐 Web query:
Neovim expr mapping remap vs noremap keycodes passthrough
💡 Result:
1) What an <expr> mapping actually does
With an <expr> mapping, Neovim evaluates an expression/callback and uses its resulting string as the keys to execute. Returning '' does nothing; returning "\<Ignore>" is “do nothing, but break out to the main loop”. Also, <expr> mappings are commonly used to “read one more key” (e.g. with getchar() / getcharstr()) and then return it. [1]
2) remap vs noremap (recursive vs non-recursive) for the returned keys
After your <expr> callback returns a string, Neovim feeds those keys back into the typeahead stream. Whether that returned key sequence is allowed to trigger other mappings depends on recursion:
- Non-recursive (
:noremap, ornoremap=true) ⇒ returned keys won’t be remapped. - Recursive (
:map, orremap=true) ⇒ returned keys can trigger mappings.
For Lua mappings:
vim.keymap.set(..., { remap = false })is the default and is the inverse ofnoremap. [3]- For the lower-level API,
nvim_set_keymap(..., { noremap = true })disables recursion. [2]
3) “keycodes passthrough” = replace_keycodes
This is the piece that decides whether strings like "<CR>", "<Tab>", "<C-w>", etc. inside the returned value are treated as literal characters or are converted to real keycodes (via nvim_replace_termcodes()):
replace_keycodes = true⇒"<CR>"becomes an actual Enter keycode.replace_keycodes = false⇒"<CR>"stays four characters:<,C,R,>.
Neovim documents this option specifically for expr maps. [2]
And vim.keymap.set() defaults replace_keycodes to true when expr = true. [3]
Practical combos
- Return special keys like
<CR>/<Esc>/<C-w>from an<expr>map:
Useexpr=trueand keep the defaultreplace_keycodes=true. [2][3] - Return a string that must stay literal (e.g. you’re generating text containing
<...>):
Useexpr=true, replace_keycodes=false. [2][3] - Passthrough but still allow other mappings to apply to what you return:
Useremap=true. [3] - Passthrough and guarantee “raw” behavior (don’t trigger other mappings):
Use non-recursive (remap=false/noremap=true). [2][3]
Sources: Neovim :help map-expression (expr behavior, <Ignore>, getchar usage) [1], :help nvim_set_keymap() (defines noremap, replace_keycodes) [2], :help vim.keymap.set() (Lua remap and replace_keycodes defaults) [3].
Pass-through keymaps bypass existing mappings.
When completion is not visible, the callback returns termcodes with the default remap = false, which prevents the returned keys from triggering other mappings (e.g., snippets on <Tab>). To allow user mappings to take effect on passthrough, return the raw key and enable remap = true on the conditional mappings.
🐛 Proposed fix to preserve existing mappings
local function make_conditional_callback(key, action)
return function()
if get_ui().is_visible() then
action()
return ''
else
- -- Pass through the original key
- return vim.api.nvim_replace_termcodes(key, true, false, true)
+ -- Pass through the original key (allow remap)
+ return key
end
end
end
@@
vim.keymap.set('i', km.accept_full, make_conditional_callback(km.accept_full, function()
get_completion().accept_full()
end), {
buffer = bufnr,
expr = true,
+ remap = true,
desc = 'Sweep: accept full completion',
silent = true,
})
@@
vim.keymap.set('i', km.accept_line, make_conditional_callback(km.accept_line, function()
get_completion().accept_line()
end), {
buffer = bufnr,
expr = true,
+ remap = true,
desc = 'Sweep: accept first line',
silent = true,
})
@@
vim.keymap.set('i', km.accept_word, make_conditional_callback(km.accept_word, function()
get_completion().accept_word()
end), {
buffer = bufnr,
expr = true,
+ remap = true,
desc = 'Sweep: accept first word',
silent = true,
})
@@
vim.keymap.set('i', km.dismiss, make_conditional_callback(km.dismiss, function()
get_completion().dismiss()
end), {
buffer = bufnr,
expr = true,
+ remap = true,
desc = 'Sweep: dismiss completion',
silent = true,
})🤖 Prompt for AI Agents
In `@sweep.nvim/lua/sweep/keymaps.lua` around lines 30 - 121, The conditional
callback currently returns termcodes via vim.api.nvim_replace_termcodes (in
make_conditional_callback), which disables user remapped mappings on
passthrough; change make_conditional_callback to return the raw key string (do
not call nvim_replace_termcodes) when completion is not visible, and update the
vim.keymap.set calls in setup_buffer_keymaps for the conditional mappings (those
using km.accept_full, km.accept_line, km.accept_word, km.dismiss) to include
remap = true in their options so returned keys honor existing user mappings and
snippets.
| --- Initialize the ring buffer with configuration | ||
| ---@param opts? RingConfig | ||
| function M.setup(opts) | ||
| opts = opts or {} | ||
| config.max_chunks = opts.max_chunks or 16 | ||
| config.chunk_size = opts.chunk_size or 64 | ||
| M.clear() | ||
| end |
There was a problem hiding this comment.
Guard against max_chunks <= 0 (modulo-by-zero).
If a user sets max_chunks to 0 or negative, add/get will crash on modulo. Clamp to a safe minimum (and optionally do the same for chunk_size).
🔧 Suggested fix
function M.setup(opts)
opts = opts or {}
- config.max_chunks = opts.max_chunks or 16
- config.chunk_size = opts.chunk_size or 64
+ config.max_chunks = math.max(1, opts.max_chunks or 16)
+ config.chunk_size = math.max(1, opts.chunk_size or 64)
M.clear()
end🤖 Prompt for AI Agents
In `@sweep.nvim/lua/sweep/ring.lua` around lines 72 - 79, M.setup currently
assigns config.max_chunks and config.chunk_size directly from opts which can
allow zero or negative values causing modulo-by-zero in M.add/M.get; clamp
config.max_chunks to a safe minimum of 1 (and similarly clamp config.chunk_size
to at least 1) inside M.setup after reading opts, e.g. enforce config.max_chunks
= math.max(1, opts.max_chunks or 16) and config.chunk_size = math.max(1,
opts.chunk_size or 64), then call M.clear(); ensure any dependent logic in M.add
and M.get can rely on these invariants.
| | `ui` | Ghost text rendering and info display | | ||
| | `keymaps` | Keymap setup and handlers | | ||
| | `autocmds` | Autocommand management | | ||
| | `edits` | Applies accepted completions to buffer | |
There was a problem hiding this comment.
Fix inaccurate module description.
The edits module description says "Applies accepted completions to buffer", but based on the implementation, it actually tracks edit history for next-edit prediction context. The actual completion application likely happens in completion or ui modules.
📝 Proposed fix
-| `edits` | Applies accepted completions to buffer |
+| `edits` | Tracks edit history for next-edit prediction |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `edits` | Applies accepted completions to buffer | | |
| | `edits` | Tracks edit history for next-edit prediction | |
🤖 Prompt for AI Agents
In `@sweep.nvim/README.md` at line 136, Update the README entry for the edits
module: replace the inaccurate description "Applies accepted completions to
buffer" with a concise, accurate description such as "Tracks edit history and
provides context for next-edit prediction" (mentioning that actual completion
application is handled by modules like completion or ui); target the `edits`
module line in the README and ensure the wording distinguishes its role from
`completion` / `ui`.
| describe('cancel_all', function() | ||
| it('should cancel all pending requests', function() | ||
| local jobs = {} | ||
|
|
||
| mock_curl.post = function(opts) | ||
| mock_curl._last_opts = opts | ||
| local job = create_mock_job({ pid = #jobs + 1 }) | ||
| table.insert(jobs, job) | ||
| return job | ||
| end | ||
|
|
||
| -- Make multiple requests | ||
| http.request({ | ||
| endpoint = 'http://localhost:8080/completion', | ||
| body = { prompt = 'test1' }, | ||
| on_success = function() end, | ||
| on_error = function() end, | ||
| }) | ||
|
|
||
| http.request({ | ||
| endpoint = 'http://localhost:8080/completion', | ||
| body = { prompt = 'test2' }, | ||
| on_success = function() end, | ||
| on_error = function() end, | ||
| }) | ||
|
|
||
| http.request({ | ||
| endpoint = 'http://localhost:8080/completion', | ||
| body = { prompt = 'test3' }, | ||
| on_success = function() end, | ||
| on_error = function() end, | ||
| }) | ||
|
|
||
| assert.are.equal(3, #jobs) | ||
|
|
||
| -- Cancel all | ||
| http.cancel_all() | ||
|
|
||
| -- All jobs should be cancelled | ||
| for _, job in ipairs(jobs) do | ||
| assert.is_true(job._cancelled) | ||
| end | ||
| end) | ||
|
|
||
| it('should not call any callbacks after cancel_all', function() | ||
| local callbacks_triggered = 0 | ||
|
|
||
| mock_curl.post = function(opts) | ||
| mock_curl._last_opts = opts | ||
| local job = create_mock_job() | ||
| local stored_callback = opts.callback | ||
| job._trigger_callback = function() | ||
| if stored_callback then | ||
| stored_callback({ | ||
| status = 200, | ||
| body = vim.json.encode({ content = 'test' }), | ||
| }) | ||
| end | ||
| end | ||
| return job | ||
| end | ||
|
|
||
| local handle1 = http.request({ | ||
| endpoint = 'http://localhost:8080/completion', | ||
| body = { prompt = 'test1' }, | ||
| on_success = function() callbacks_triggered = callbacks_triggered + 1 end, | ||
| on_error = function() callbacks_triggered = callbacks_triggered + 1 end, | ||
| }) | ||
|
|
||
| local job1 = mock_job | ||
|
|
||
| local handle2 = http.request({ | ||
| endpoint = 'http://localhost:8080/completion', | ||
| body = { prompt = 'test2' }, | ||
| on_success = function() callbacks_triggered = callbacks_triggered + 1 end, | ||
| on_error = function() callbacks_triggered = callbacks_triggered + 1 end, | ||
| }) | ||
|
|
||
| local job2 = mock_job | ||
|
|
||
| -- Cancel all | ||
| http.cancel_all() | ||
|
|
||
| -- Try to trigger callbacks | ||
| vim.schedule(function() | ||
| job1._trigger_callback() | ||
| job2._trigger_callback() | ||
| end) | ||
|
|
||
| vim.wait(100, function() return callbacks_triggered > 0 end, 10) | ||
|
|
||
| assert.are.equal(0, callbacks_triggered) | ||
| end) |
There was a problem hiding this comment.
Fix nil job handles in cancel_all callback test.
The overridden mock_curl.post never assigns mock_job, so job1/job2 can be nil and _trigger_callback() will error. Capture the created job (or set mock_job) before storing it.
🐛 Proposed fix
- mock_curl.post = function(opts)
- mock_curl._last_opts = opts
- local job = create_mock_job()
+ mock_curl.post = function(opts)
+ mock_curl._last_opts = opts
+ local job = create_mock_job()
+ mock_job = job
local stored_callback = opts.callback
job._trigger_callback = function()
if stored_callback then
stored_callback({
status = 200,
body = vim.json.encode({ content = 'test' }),
})
end
end
return job
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe('cancel_all', function() | |
| it('should cancel all pending requests', function() | |
| local jobs = {} | |
| mock_curl.post = function(opts) | |
| mock_curl._last_opts = opts | |
| local job = create_mock_job({ pid = #jobs + 1 }) | |
| table.insert(jobs, job) | |
| return job | |
| end | |
| -- Make multiple requests | |
| http.request({ | |
| endpoint = 'http://localhost:8080/completion', | |
| body = { prompt = 'test1' }, | |
| on_success = function() end, | |
| on_error = function() end, | |
| }) | |
| http.request({ | |
| endpoint = 'http://localhost:8080/completion', | |
| body = { prompt = 'test2' }, | |
| on_success = function() end, | |
| on_error = function() end, | |
| }) | |
| http.request({ | |
| endpoint = 'http://localhost:8080/completion', | |
| body = { prompt = 'test3' }, | |
| on_success = function() end, | |
| on_error = function() end, | |
| }) | |
| assert.are.equal(3, #jobs) | |
| -- Cancel all | |
| http.cancel_all() | |
| -- All jobs should be cancelled | |
| for _, job in ipairs(jobs) do | |
| assert.is_true(job._cancelled) | |
| end | |
| end) | |
| it('should not call any callbacks after cancel_all', function() | |
| local callbacks_triggered = 0 | |
| mock_curl.post = function(opts) | |
| mock_curl._last_opts = opts | |
| local job = create_mock_job() | |
| local stored_callback = opts.callback | |
| job._trigger_callback = function() | |
| if stored_callback then | |
| stored_callback({ | |
| status = 200, | |
| body = vim.json.encode({ content = 'test' }), | |
| }) | |
| end | |
| end | |
| return job | |
| end | |
| local handle1 = http.request({ | |
| endpoint = 'http://localhost:8080/completion', | |
| body = { prompt = 'test1' }, | |
| on_success = function() callbacks_triggered = callbacks_triggered + 1 end, | |
| on_error = function() callbacks_triggered = callbacks_triggered + 1 end, | |
| }) | |
| local job1 = mock_job | |
| local handle2 = http.request({ | |
| endpoint = 'http://localhost:8080/completion', | |
| body = { prompt = 'test2' }, | |
| on_success = function() callbacks_triggered = callbacks_triggered + 1 end, | |
| on_error = function() callbacks_triggered = callbacks_triggered + 1 end, | |
| }) | |
| local job2 = mock_job | |
| -- Cancel all | |
| http.cancel_all() | |
| -- Try to trigger callbacks | |
| vim.schedule(function() | |
| job1._trigger_callback() | |
| job2._trigger_callback() | |
| end) | |
| vim.wait(100, function() return callbacks_triggered > 0 end, 10) | |
| assert.are.equal(0, callbacks_triggered) | |
| end) | |
| describe('cancel_all', function() | |
| it('should cancel all pending requests', function() | |
| local jobs = {} | |
| mock_curl.post = function(opts) | |
| mock_curl._last_opts = opts | |
| local job = create_mock_job({ pid = `#jobs` + 1 }) | |
| table.insert(jobs, job) | |
| return job | |
| end | |
| -- Make multiple requests | |
| http.request({ | |
| endpoint = 'http://localhost:8080/completion', | |
| body = { prompt = 'test1' }, | |
| on_success = function() end, | |
| on_error = function() end, | |
| }) | |
| http.request({ | |
| endpoint = 'http://localhost:8080/completion', | |
| body = { prompt = 'test2' }, | |
| on_success = function() end, | |
| on_error = function() end, | |
| }) | |
| http.request({ | |
| endpoint = 'http://localhost:8080/completion', | |
| body = { prompt = 'test3' }, | |
| on_success = function() end, | |
| on_error = function() end, | |
| }) | |
| assert.are.equal(3, `#jobs`) | |
| -- Cancel all | |
| http.cancel_all() | |
| -- All jobs should be cancelled | |
| for _, job in ipairs(jobs) do | |
| assert.is_true(job._cancelled) | |
| end | |
| end) | |
| it('should not call any callbacks after cancel_all', function() | |
| local callbacks_triggered = 0 | |
| mock_curl.post = function(opts) | |
| mock_curl._last_opts = opts | |
| local job = create_mock_job() | |
| mock_job = job | |
| local stored_callback = opts.callback | |
| job._trigger_callback = function() | |
| if stored_callback then | |
| stored_callback({ | |
| status = 200, | |
| body = vim.json.encode({ content = 'test' }), | |
| }) | |
| end | |
| end | |
| return job | |
| end | |
| local handle1 = http.request({ | |
| endpoint = 'http://localhost:8080/completion', | |
| body = { prompt = 'test1' }, | |
| on_success = function() callbacks_triggered = callbacks_triggered + 1 end, | |
| on_error = function() callbacks_triggered = callbacks_triggered + 1 end, | |
| }) | |
| local job1 = mock_job | |
| local handle2 = http.request({ | |
| endpoint = 'http://localhost:8080/completion', | |
| body = { prompt = 'test2' }, | |
| on_success = function() callbacks_triggered = callbacks_triggered + 1 end, | |
| on_error = function() callbacks_triggered = callbacks_triggered + 1 end, | |
| }) | |
| local job2 = mock_job | |
| -- Cancel all | |
| http.cancel_all() | |
| -- Try to trigger callbacks | |
| vim.schedule(function() | |
| job1._trigger_callback() | |
| job2._trigger_callback() | |
| end) | |
| vim.wait(100, function() return callbacks_triggered > 0 end, 10) | |
| assert.are.equal(0, callbacks_triggered) | |
| end) |
🤖 Prompt for AI Agents
In `@sweep.nvim/tests/sweep/http_spec.lua` around lines 342 - 434, The test's
mock_curl.post creates a local job but never assigns it to the global mock_job,
so job1/job2 become nil and calling _trigger_callback fails; modify the
mock_curl.post in the second spec to capture the created job (e.g., local job =
create_mock_job() or create_mock_job({...}) ) and either assign mock_job = job
or return that same job and set local job1/job2 = mock_job after calling
http.request; specifically update the mock_curl.post closure used in the "should
not call any callbacks after cancel_all" test to set mock_job (or return and
store the created job) before wiring stored_callback so job1._trigger_callback
and job2._trigger_callback are valid.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@sweep.nvim/lua/sweep/completion.lua`:
- Around line 248-258: The callbacks passed to the HTTP request are being
wrapped in vim.schedule twice, causing extra latency; remove the outer
vim.schedule wrappers in the on_success and on_error handlers so they call
on_completion_success(response, cache_key) and on_completion_error(err) directly
(leave the existing on_completion_success, on_completion_error and cache_key
references intact) because http.lua already schedules those callbacks.
♻️ Duplicate comments (5)
sweep.nvim/lua/sweep/http.lua (1)
79-80: Race condition: request entry removed before callbacks complete.This is the same issue flagged in a previous review. The request entry is removed at line 80 before
vim.schedulecallbacks execute, so acancel()call after the response arrives but before the scheduled callback runs cannot suppress staleon_success/on_errorinvocations.sweep.nvim/lua/sweep/completion.lua (2)
59-67: Timer handle not closed — potential resource leak.This is the same issue flagged in a previous review. In libuv, timers should be closed with
:close()after stopping to release the underlying handle.
244-245: Potential double-slash in endpoint URL.This is the same issue flagged in a previous review. If
cfg.server.endpointends with a trailing slash, the concatenation produces a double-slash URL.sweep.nvim/lua/sweep/context.lua (2)
175-183: Scope detection hardcodes column 0.This is the same issue flagged in a previous review. The function signature doesn't accept a
colparameter and hardcodespos = { row, 0 }. On indented lines, column 0 may resolve to an outer scope node rather than the actual scope at the cursor.
404-416: Hover contents array handling missing.This is the same issue flagged in a previous review. Per the LSP specification,
textDocument/hoverresponsecontentscan beMarkedString[](an array/list), but the current code only handles single strings and tables with.valueproperties.
🧹 Nitpick comments (3)
sweep.nvim/lua/sweep/parser.lua (1)
110-117: Consider usingvim.list_slicefor cleaner line limiting.The manual loop for limiting lines can be simplified using Neovim's built-in utility.
Optional simplification
-- Apply max_lines limit if max_lines and max_lines > 0 and `#lines` > max_lines then - local limited_lines = {} - for i = 1, max_lines do - table.insert(limited_lines, lines[i]) - end - lines = limited_lines + lines = vim.list_slice(lines, 1, max_lines) content = table.concat(lines, '\n') endsweep.nvim/lua/sweep/http.lua (1)
199-205: Consider O(1) active count tracking.The current implementation iterates over all entries. If request volume is low this is fine, but for consistency with the commit message mentioning "entry counter to cache for O(1) count operations," you could maintain an
active_countvariable incremented/decremented on add/remove.sweep.nvim/lua/sweep/context.lua (1)
442-442: Line counting pattern is clever but may confuse readers.
select(2, str:gsub('\n', '\n'))returns the replacement count (number of newlines). Consider extracting this to a named helper for clarity.Optional: Extract line count helper
-- Add near top of file local function count_lines(str) local _, newlines = str:gsub('\n', '\n') return newlines + 1 end -- Then use: local scope_lines = count_lines(ctx.scope.content)
sweep.nvim/lua/sweep/completion.lua
Outdated
| on_success = function(response) | ||
| vim.schedule(function() | ||
| on_completion_success(response, cache_key) | ||
| end) | ||
| end, | ||
| on_error = function(err) | ||
| vim.schedule(function() | ||
| on_completion_error(err) | ||
| end) | ||
| end, | ||
| }) |
There was a problem hiding this comment.
Double vim.schedule wrapping adds unnecessary latency.
The http.lua module already wraps on_success and on_error callbacks in vim.schedule (see lines 85, 105, 115, 126, 135-137 in http.lua). Wrapping them again here is redundant and delays callback execution by an extra event loop cycle.
Proposed fix
on_success = function(response)
- vim.schedule(function()
- on_completion_success(response, cache_key)
- end)
+ on_completion_success(response, cache_key)
end,
on_error = function(err)
- vim.schedule(function()
- on_completion_error(err)
- end)
+ on_completion_error(err)
end,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| on_success = function(response) | |
| vim.schedule(function() | |
| on_completion_success(response, cache_key) | |
| end) | |
| end, | |
| on_error = function(err) | |
| vim.schedule(function() | |
| on_completion_error(err) | |
| end) | |
| end, | |
| }) | |
| on_success = function(response) | |
| on_completion_success(response, cache_key) | |
| end, | |
| on_error = function(err) | |
| on_completion_error(err) | |
| end, | |
| }) |
🤖 Prompt for AI Agents
In `@sweep.nvim/lua/sweep/completion.lua` around lines 248 - 258, The callbacks
passed to the HTTP request are being wrapped in vim.schedule twice, causing
extra latency; remove the outer vim.schedule wrappers in the on_success and
on_error handlers so they call on_completion_success(response, cache_key) and
on_completion_error(err) directly (leave the existing on_completion_success,
on_completion_error and cache_key references intact) because http.lua already
schedules those callbacks.
| local function split_lines(str) | ||
| if str == '' then | ||
| return { '' } | ||
| end | ||
| local lines = {} | ||
| for line in str:gmatch('([^\n]*)\n?') do | ||
| if line ~= '' or #lines == 0 then | ||
| table.insert(lines, line) | ||
| end | ||
| end | ||
| -- Remove trailing empty string from gmatch if present | ||
| if #lines > 1 and lines[#lines] == '' then | ||
| table.remove(lines) | ||
| end | ||
| return lines | ||
| end |
There was a problem hiding this comment.
split_lines may drop legitimate empty lines.
The condition on line 14 skips empty lines after the first, which means multi-line content like "foo\n\nbar" would lose the middle blank line, producing {"foo", "bar"} instead of {"foo", "", "bar"}. If preserving blank lines matters for completion display, this logic needs adjustment.
Proposed fix to preserve interior empty lines
local function split_lines(str)
if str == '' then
return { '' }
end
local lines = {}
for line in str:gmatch('([^\n]*)\n?') do
- if line ~= '' or `#lines` == 0 then
- table.insert(lines, line)
- end
+ table.insert(lines, line)
end
-- Remove trailing empty string from gmatch if present
if `#lines` > 1 and lines[`#lines`] == '' then
table.remove(lines)
end
return lines
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local function split_lines(str) | |
| if str == '' then | |
| return { '' } | |
| end | |
| local lines = {} | |
| for line in str:gmatch('([^\n]*)\n?') do | |
| if line ~= '' or #lines == 0 then | |
| table.insert(lines, line) | |
| end | |
| end | |
| -- Remove trailing empty string from gmatch if present | |
| if #lines > 1 and lines[#lines] == '' then | |
| table.remove(lines) | |
| end | |
| return lines | |
| end | |
| local function split_lines(str) | |
| if str == '' then | |
| return { '' } | |
| end | |
| local lines = {} | |
| for line in str:gmatch('([^\n]*)\n?') do | |
| table.insert(lines, line) | |
| end | |
| -- Remove trailing empty string from gmatch if present | |
| if `#lines` > 1 and lines[`#lines`] == '' then | |
| table.remove(lines) | |
| end | |
| return lines | |
| end |
- Fix 4: Attach edit tracking to buffers on BufEnter (autocmds.lua) - Skip special buffers and excluded filetypes - edits.attach() now called for normal file buffers - Fix 1: Wire edit tracking into completion flow (completion.lua) - Add edits module to lazy-loaded dependencies - Include edits.get_context() in prompt (first, as most relevant) - Fix 3: Implement completion deduplication (completion.lua) - Add deduplicate_completion() helper - Removes text that already exists after cursor - Prevents suggesting duplicate content - Fix 2: Fix LSP context async/sync mismatch (completion.lua) - Refactor make_request() to use context.get_async() when LSP enabled - Extract send_completion_request() helper for cleaner flow - Extract add_sync_context() for ring buffer and edit context - LSP definitions and type_info now properly included in prompts
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@sweep.nvim/lua/sweep/autocmds.lua`:
- Around line 36-57: The add_visible_to_ring function can capture the wrong
window's visible range on BufLeave; modify add_visible_to_ring to accept the
target bufnr (and optional winid), avoid using the implicit current window, and
compute the visible range for a window actually showing that buffer (use
vim.fn.win_findbuf(bufnr) to get windows and call vim.fn.line('w0', winid) /
vim.fn.line('w$', winid) for that window); then use vim.api.nvim_buf_get_lines
with the computed first/last and call ring.add(...) as before; ensure the
BufLeave autocmd passes the buffer (and preferably the winid) or invoke
add_visible_to_ring earlier (before the window switches) so the captured lines
come from the correct window/buffer.
- Around line 80-92: on_buf_leave currently takes bufnr but calls
add_visible_to_ring which uses nvim_get_current_buf() and can capture the wrong
buffer; change add_visible_to_ring to accept an optional bufnr parameter (use
the provided bufnr when present, otherwise fall back to nvim_get_current_buf()),
then update the caller on_buf_leave to call add_visible_to_ring(bufnr). Make
sure the add_visible_to_ring implementation and any other callers handle a nil
bufnr gracefully and continue to default to the current buffer.
In `@sweep.nvim/lua/sweep/completion.lua`:
- Around line 399-404: The debounce timer created in state.debounce_timer is
never closed when it fires; update the logic so the timer is closed in both
cancellation and successful fire. Modify cancel_debounce_timer() to stop and
then close state.debounce_timer (ensuring nil assignment after closing), and in
the vim.schedule_wrap callback (where make_request() is called) call
state.debounce_timer:close() (or reuse cancel_debounce_timer() after ensuring it
closes rather than only stopping) so the handle is always closed and cleaned up
after firing; reference symbols: state.debounce_timer, cancel_debounce_timer(),
cfg.debounce_ms, make_request().
- Around line 171-182: The async callback uses a captured bufnr and cursor
(row/col) without validating the buffer is still valid/current; before calling
deduplicate_completion(result.lines, bufnr, row, col) or ui.clear(), check
vim.api.nvim_buf_is_valid(bufnr) and that vim.api.nvim_get_current_buf() ==
bufnr (or otherwise bail), and if invalid/changed, call ui.clear() and return to
avoid operating on a stale buffer.
🧹 Nitpick comments (3)
sweep.nvim/lua/sweep/completion.lua (3)
102-128: Deduplication only handles exact prefix matches.The current logic only removes duplicates when the completion starts exactly with
text_after_cursor. It won't handle partial overlaps (e.g., completion"function"when"func"exists after cursor). This is a minor limitation that may result in redundant suggestions in edge cases.
264-275: Comment contradicts code ordering.The comment on line 271 states edit context "goes first in the prompt" but the code prepends
ring_contextfirst (line 267), then prependsedit_context(line 274). Since prepending reverses order, edit context does end up first—but the logic is confusing. Consider reordering for clarity.Proposed fix for clarity
local function add_sync_context(prefix) local full_prefix = prefix - -- Add ring buffer context - local ring_context = ring.get_context() - if ring_context and ring_context ~= '' then - full_prefix = ring_context .. '\n\n' .. full_prefix - end - -- Add edit tracking context (Sweep's next-edit format) -- This goes first in the prompt as it's most relevant for predicting next edits local edit_context = edits.get_context() if edit_context and edit_context ~= '' then full_prefix = edit_context .. '\n\n' .. full_prefix end + -- Add ring buffer context (after edit context) + local ring_context = ring.get_context() + if ring_context and ring_context ~= '' then + full_prefix = ring_context .. '\n\n' .. full_prefix + end + return full_prefix end
341-360: Async LSP callback may proceed with stale context.The callback at line 341 only checks
state.enabledbut doesn't verify that the buffer or cursor position hasn't changed while waiting for LSP. If the user continues typing, this request may complete with outdatedbase_prefix,input_suffix, andcache_key, potentially showing irrelevant completions.Consider capturing the cursor position/buffer at request time and comparing in the callback, or incrementing a request generation counter to detect staleness.
| local function add_visible_to_ring(source) | ||
| local ring = require('sweep.ring') | ||
| local bufnr = vim.api.nvim_get_current_buf() | ||
| local filename = vim.api.nvim_buf_get_name(bufnr) | ||
| local filetype = vim.bo[bufnr].filetype | ||
|
|
||
| -- Get visible line range | ||
| local first = vim.fn.line('w0') | ||
| local last = vim.fn.line('w$') | ||
|
|
||
| -- Get lines (nvim_buf_get_lines is 0-indexed) | ||
| local ok, lines = pcall(vim.api.nvim_buf_get_lines, bufnr, first - 1, last, false) | ||
| if not ok or #lines == 0 then | ||
| return | ||
| end | ||
|
|
||
| ring.add({ | ||
| content = table.concat(lines, '\n'), | ||
| filename = filename, | ||
| filetype = filetype, | ||
| source = source, | ||
| }) |
There was a problem hiding this comment.
Window/buffer mismatch possible on BufLeave.
vim.fn.line('w0') and vim.fn.line('w$') return the visible range of the current window, not necessarily the window showing the buffer being left. On BufLeave, the window may have already switched context, causing add_visible_to_ring to capture content from the wrong buffer or incorrect line ranges.
Consider passing bufnr to the function and using buffer-local methods, or calling this function before the buffer switch completes.
🤖 Prompt for AI Agents
In `@sweep.nvim/lua/sweep/autocmds.lua` around lines 36 - 57, The
add_visible_to_ring function can capture the wrong window's visible range on
BufLeave; modify add_visible_to_ring to accept the target bufnr (and optional
winid), avoid using the implicit current window, and compute the visible range
for a window actually showing that buffer (use vim.fn.win_findbuf(bufnr) to get
windows and call vim.fn.line('w0', winid) / vim.fn.line('w$', winid) for that
window); then use vim.api.nvim_buf_get_lines with the computed first/last and
call ring.add(...) as before; ensure the BufLeave autocmd passes the buffer (and
preferably the winid) or invoke add_visible_to_ring earlier (before the window
switches) so the captured lines come from the correct window/buffer.
| --- Handle BufLeave event - cancel completion and add to ring | ||
| ---@param bufnr number | ||
| local function on_buf_leave(bufnr) | ||
| local completion = require('sweep.completion') | ||
| local ui = require('sweep.ui') | ||
|
|
||
| -- Cancel any pending completion | ||
| completion.cancel() | ||
| ui.clear() | ||
|
|
||
| -- Add visible content to ring buffer | ||
| add_visible_to_ring('buffer_leave') | ||
| end |
There was a problem hiding this comment.
Unused bufnr parameter — may capture wrong buffer content.
on_buf_leave receives bufnr (the buffer being left) but add_visible_to_ring calls nvim_get_current_buf() internally, which might already be the new buffer. This could cause the wrong buffer's content to be added to the ring.
Proposed fix
Modify add_visible_to_ring to accept an optional bufnr parameter:
-local function add_visible_to_ring(source)
- local ring = require('sweep.ring')
- local bufnr = vim.api.nvim_get_current_buf()
+local function add_visible_to_ring(source, bufnr)
+ local ring = require('sweep.ring')
+ bufnr = bufnr or vim.api.nvim_get_current_buf()Then pass it from the caller:
- add_visible_to_ring('buffer_leave')
+ add_visible_to_ring('buffer_leave', bufnr)🤖 Prompt for AI Agents
In `@sweep.nvim/lua/sweep/autocmds.lua` around lines 80 - 92, on_buf_leave
currently takes bufnr but calls add_visible_to_ring which uses
nvim_get_current_buf() and can capture the wrong buffer; change
add_visible_to_ring to accept an optional bufnr parameter (use the provided
bufnr when present, otherwise fall back to nvim_get_current_buf()), then update
the caller on_buf_leave to call add_visible_to_ring(bufnr). Make sure the
add_visible_to_ring implementation and any other callers handle a nil bufnr
gracefully and continue to default to the current buffer.
| -- Get current cursor position | ||
| local cursor = vim.api.nvim_win_get_cursor(0) | ||
| local row = cursor[1] - 1 -- Convert to 0-indexed | ||
| local col = cursor[2] | ||
| local bufnr = vim.api.nvim_get_current_buf() | ||
|
|
||
| -- Deduplicate: remove text that already exists after cursor | ||
| local display_lines = deduplicate_completion(result.lines, bufnr, row, col) | ||
| if not display_lines or #display_lines == 0 then | ||
| ui.clear() | ||
| return | ||
| end |
There was a problem hiding this comment.
Validate buffer still exists before using it in async callback.
The callback fires asynchronously after the HTTP request completes. By that time, the buffer may have been closed or the user may have switched to a different buffer. Using a stale bufnr could cause errors or display completions in the wrong buffer.
Proposed fix
-- Get current cursor position
local cursor = vim.api.nvim_win_get_cursor(0)
local row = cursor[1] - 1 -- Convert to 0-indexed
local col = cursor[2]
local bufnr = vim.api.nvim_get_current_buf()
+ -- Validate buffer is still valid
+ if not vim.api.nvim_buf_is_valid(bufnr) then
+ return
+ end
+
-- Deduplicate: remove text that already exists after cursor🤖 Prompt for AI Agents
In `@sweep.nvim/lua/sweep/completion.lua` around lines 171 - 182, The async
callback uses a captured bufnr and cursor (row/col) without validating the
buffer is still valid/current; before calling
deduplicate_completion(result.lines, bufnr, row, col) or ui.clear(), check
vim.api.nvim_buf_is_valid(bufnr) and that vim.api.nvim_get_current_buf() ==
bufnr (or otherwise bail), and if invalid/changed, call ui.clear() and return to
avoid operating on a stale buffer.
| -- Create new debounce timer | ||
| state.debounce_timer = vim.loop.new_timer() | ||
| state.debounce_timer:start(cfg.debounce_ms, 0, vim.schedule_wrap(function() | ||
| cancel_debounce_timer() | ||
| make_request() | ||
| end)) |
There was a problem hiding this comment.
Timer not closed after successful fire.
When the debounce timer fires and executes the callback, cancel_debounce_timer() is called but the timer is no longer active, so only close() would be needed. However, as noted in lines 61-69, close() is missing entirely. Ensure the timer is properly closed both when cancelled and when it fires successfully.
🤖 Prompt for AI Agents
In `@sweep.nvim/lua/sweep/completion.lua` around lines 399 - 404, The debounce
timer created in state.debounce_timer is never closed when it fires; update the
logic so the timer is closed in both cancellation and successful fire. Modify
cancel_debounce_timer() to stop and then close state.debounce_timer (ensuring
nil assignment after closing), and in the vim.schedule_wrap callback (where
make_request() is called) call state.debounce_timer:close() (or reuse
cancel_debounce_timer() after ensuring it closes rather than only stopping) so
the handle is always closed and cleaned up after firing; reference symbols:
state.debounce_timer, cancel_debounce_timer(), cfg.debounce_ms, make_request().
2 participants
Research notes from analyzing the JetBrains Sweep AI plugin architecture,
including context building strategies, prompt formats, and LSP integration
patterns for designing a Neovim equivalent.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.