Skip to content

Conversation

@carlos-algms
Copy link
Owner

@carlos-algms carlos-algms commented Jan 2, 2026

Show edit tool call diffs directly in the target buffer with:

  • Syntax highlighting preserved on virtual lines
  • Word-level diff highlighting for modifications
  • Support for new files, modifications, and deletions
  • Automatic buffer reload after file save to avoid W13 warning

Refactor acp_diff_handler to ui/tool_call_diff with opts object API.
Add chat_widget methods for finding/opening non-widget windows.

@coderabbitai
Copy link

coderabbitai bot commented Jan 2, 2026

📝 Walkthrough

Walkthrough

This pull request introduces a diff preview feature that visually renders code changes directly within buffers. The implementation adds a new DiffPreview module that uses extmarks and virtual lines to display edit operations, integrates diff preview hooks into the SessionManager's tool call workflow, refactors diff handling to use a unified ToolCallDiff namespace with updated API signatures, and introduces a new configuration option to enable or disable the feature. A minor improvement to the file system module resolves buffer reload behavior for newly created files.

Sequence Diagram

sequenceDiagram
    participant SessionManager
    participant Config
    participant DiffPreview
    participant Buffer as Neovim Buffer
    participant TreeSitter

    SessionManager->>Config: Check diff_preview.enabled
    
    rect rgb(0, 100, 0)
    Note over SessionManager,TreeSitter: Show Diff Preview
    SessionManager->>DiffPreview: show_diff(tool_call_id)
    DiffPreview->>Buffer: Resolve/create target buffer
    DiffPreview->>Buffer: Extract diff blocks
    DiffPreview->>TreeSitter: Build highlight map
    DiffPreview->>Buffer: Apply deletion highlights
    DiffPreview->>Buffer: Render additions via virt_lines
    DiffPreview->>Buffer: Scroll to first diff block
    end
    
    rect rgb(100, 0, 0)
    Note over SessionManager,DiffPreview: Clear Diff Preview
    SessionManager->>DiffPreview: clear_diff(tool_call_id)
    DiffPreview->>Buffer: Clear all DIFF extmarks
    end
Loading

Poem

🐰 Hops excitedly

Diffs now bloom within the editor's view,
Virtual lines dance with colors bright and true,
From edit tools to buffers they do leap,
Changes rendered where code runs deep!
Preview magic, clean and sleek.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main feature: showing diff previews directly in buffers, which is the primary objective of this PR.
Description check ✅ Passed The description is directly related to the changeset, detailing the specific features and refactoring involved in showing diff previews in buffers.
Docstring Coverage ✅ Passed Docstring coverage is 88.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 44c0b24 and 9984926.

📒 Files selected for processing (4)
  • lua/agentic/session_manager.lua
  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/message_writer.lua
  • lua/agentic/ui/tool_call_diff.lua
🧰 Additional context used
📓 Path-based instructions (2)
**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

Ensure all features are multi-tabpage safe with one instance per tabpage, managed via SessionRegistry mapping tab_page_id to SessionManager

Files:

  • lua/agentic/ui/message_writer.lua
  • lua/agentic/session_manager.lua
  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/tool_call_diff.lua
lua/**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

lua/**/*.lua: Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances instead
Use module-level namespace constants for extmarks (created via vim.api.nvim_create_namespace) as they are idempotent and buffer isolation comes from buffer-specific operations
Prefer buffer-local autocommands using vim.api.nvim_create_autocmd with buffer parameter; filter by tabpage in global autocommands if necessary
Keymaps must be buffer-local using BufHelpers.keymap_set(bufnr, mode, key, fn); never use global keymaps affecting all tabpages
Use visibility prefixes for class properties: _* for private (hidden from external consumers), __* for protected (visible to subclasses), no prefix for public (visible everywhere)
Set _index to self for inheritance in Lua classes; use setmetatable to create instances and always pass parent class explicitly when calling parent constructor
In LuaCATS annotations for @param and @field, use 'variable? type' format for optional types; for @return, @type, and @alias use explicit 'type|nil' union format
In fun() type declarations, use explicit 'type|nil' union format instead of 'variable? type' due to LuaLS limitations with optional parameters in function signatures
Include a space after --- in all LuaCATS annotations and descriptions
Always run both 'make luals' (type checking) and 'make luacheck' (style/syntax checking) after making any Lua file changes before completion
Minimize class properties to only those accessed by external code, used as public API, or needed by subclasses/mixins; perform regular cleanup of unused properties
Get tabpage ID correctly using self.tab_page_id in instance methods, from buffer via vim.api.nvim_win_get_tabpage(vim.fn.bufwinid(bufnr)), or current tabpage via vim.api.nvim_get_current_tabpage()
Each tabpage manages its own buffers and windows; never assume buffer/window exists globally using vim.api.nvim_tabpage
* APIs when needed
Use Neovim v0.11.0+ APIs and f...

Files:

  • lua/agentic/ui/message_writer.lua
  • lua/agentic/session_manager.lua
  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/tool_call_diff.lua
🧠 Learnings (4)
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/agentic/acp/acp_client.lua : Never remove critical type annotations in lua/agentic/acp/acp_client.lua; only update them when underlying types change

Applied to files:

  • lua/agentic/ui/message_writer.lua
  • lua/agentic/ui/tool_call_diff.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to **/*.lua : Ensure all features are multi-tabpage safe with one instance per tabpage, managed via SessionRegistry mapping tab_page_id to SessionManager

Applied to files:

  • lua/agentic/session_manager.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/agentic/acp/**/*.lua : Each ACP provider tabpage instance maintains its own session ID from the shared ACP provider; never mix session IDs between tabpages

Applied to files:

  • lua/agentic/session_manager.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : Use module-level namespace constants for extmarks (created via vim.api.nvim_create_namespace) as they are idempotent and buffer isolation comes from buffer-specific operations

Applied to files:

  • lua/agentic/ui/diff_preview.lua
🪛 GitHub Actions: PR Checks
lua/agentic/session_manager.lua

[warning] 580-580: Cannot assign { new: string[], old: string[], all: boolean? } to { new: string[], old: string[], all: boolean }. assign-type-mismatch. diff = tracker.diff

🔇 Additional comments (19)
lua/agentic/ui/message_writer.lua (2)

1-1: LGTM!

The import correctly references the renamed module under the ui namespace, which is a cleaner organization for diff-related UI utilities.


380-385: LGTM!

The function call correctly uses the renamed ToolCallDiff module with the same parameters. The diff extraction logic remains unchanged.

lua/agentic/session_manager.lua (4)

7-10: LGTM!

Import additions are correctly organized. DiffPreview is appropriately imported from the ui namespace to support the new diff preview functionality.


420-422: LGTM!

Pre-emptively clearing the diff preview on tool call updates is the correct approach. This ensures the preview is removed regardless of success or failure status.


443-457: LGTM!

The callback wrapping pattern correctly ensures diff cleanup after permission handling. The flow is well-structured: show diff → display permission request → user responds → clear diff.


585-595: LGTM!

The _clear_diff_in_buffer method has consistent guard logic and correctly passes the file path to DiffPreview.clear_diff.

lua/agentic/ui/diff_preview.lua (7)

1-9: LGTM!

Module setup follows conventions. The namespace constant NS_DIFF is correctly defined at module level per coding guidelines, as extmark namespaces are idempotent and buffer isolation comes from buffer-specific operations.


19-38: LGTM!

The function correctly scopes window lookup to the current tabpage using nvim_get_current_tabpage and nvim_tabpage_list_wins, ensuring multi-tabpage safety. The set-based lookup for widget windows is efficient.


40-49: LGTM!

The helper correctly creates a left-side split window using nvim_open_win. Setting win = -1 makes it relative to the current window.


74-108: LGTM!

Deletion highlighting correctly converts 1-indexed line numbers to 0-indexed for the Neovim API. The pcall wrapper with user notification provides appropriate error handling for edge cases like out-of-bounds lines.


110-137: LGTM!

Virtual line rendering for additions is correctly implemented. The anchor line calculation handles edge cases, and virt_lines_above = false ensures additions appear below the deleted content, providing a natural diff visualization.


140-154: LGTM!

Cursor positioning to the first diff block provides good UX. The pcall wrapper handles edge cases gracefully.


156-168: LGTM!

The clear_diff function correctly handles both buffer numbers and paths, with defensive pcall wrapping for the namespace clear operation.

lua/agentic/ui/tool_call_diff.lua (6)

1-8: LGTM!

The type definitions are correctly moved to the agentic.ui.ToolCallDiff namespace, providing better organization for UI-related diff utilities.


18-21: LGTM!

Return type annotation correctly uses the new namespace. The function signature and behavior remain unchanged.


69-73: LGTM!

Type annotations for _minimize_diff_blocks consistently use the new agentic.ui.ToolCallDiff.DiffBlock type for both parameters and return values.


160-165: LGTM!

Type annotations for _create_new_file_diff_block correctly updated to the new namespace.


194-205: LGTM!

Type annotations in _match_or_substring_fallback consistently updated. The return type and local block type both use the new namespace.


236-252: LGTM!

Type annotations in _find_substring_replacements correctly updated to the new namespace.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
lua/agentic/ui/diff_preview.lua (1)

59-63: Target window may not display the diff buffer.

When find_first_non_widget_window returns a window showing a different buffer, extmarks are applied to bufnr but the window continues displaying its current buffer. The user won't see the diff preview.

Set the buffer in the target window before applying extmarks:

🔎 Proposed fix
     local target_winid = find_first_non_widget_window(opts.widget_windows)

     if not target_winid then
         target_winid = open_left_window(bufnr)
+    else
+        -- Ensure the target window displays the diff buffer
+        vim.api.nvim_win_set_buf(target_winid, bufnr)
     end
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9984926 and 1443b67.

📒 Files selected for processing (2)
  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/message_writer.lua
🧰 Additional context used
📓 Path-based instructions (2)
**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

Ensure all features are multi-tabpage safe with one instance per tabpage, managed via SessionRegistry mapping tab_page_id to SessionManager

Files:

  • lua/agentic/ui/message_writer.lua
  • lua/agentic/ui/diff_preview.lua
lua/**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

lua/**/*.lua: Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances instead
Use module-level namespace constants for extmarks (created via vim.api.nvim_create_namespace) as they are idempotent and buffer isolation comes from buffer-specific operations
Prefer buffer-local autocommands using vim.api.nvim_create_autocmd with buffer parameter; filter by tabpage in global autocommands if necessary
Keymaps must be buffer-local using BufHelpers.keymap_set(bufnr, mode, key, fn); never use global keymaps affecting all tabpages
Use visibility prefixes for class properties: _* for private (hidden from external consumers), __* for protected (visible to subclasses), no prefix for public (visible everywhere)
Set _index to self for inheritance in Lua classes; use setmetatable to create instances and always pass parent class explicitly when calling parent constructor
In LuaCATS annotations for @param and @field, use 'variable? type' format for optional types; for @return, @type, and @alias use explicit 'type|nil' union format
In fun() type declarations, use explicit 'type|nil' union format instead of 'variable? type' due to LuaLS limitations with optional parameters in function signatures
Include a space after --- in all LuaCATS annotations and descriptions
Always run both 'make luals' (type checking) and 'make luacheck' (style/syntax checking) after making any Lua file changes before completion
Minimize class properties to only those accessed by external code, used as public API, or needed by subclasses/mixins; perform regular cleanup of unused properties
Get tabpage ID correctly using self.tab_page_id in instance methods, from buffer via vim.api.nvim_win_get_tabpage(vim.fn.bufwinid(bufnr)), or current tabpage via vim.api.nvim_get_current_tabpage()
Each tabpage manages its own buffers and windows; never assume buffer/window exists globally using vim.api.nvim_tabpage
* APIs when needed
Use Neovim v0.11.0+ APIs and f...

Files:

  • lua/agentic/ui/message_writer.lua
  • lua/agentic/ui/diff_preview.lua
🧠 Learnings (3)
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/agentic/acp/acp_client.lua : Never remove critical type annotations in lua/agentic/acp/acp_client.lua; only update them when underlying types change

Applied to files:

  • lua/agentic/ui/message_writer.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : Use module-level namespace constants for extmarks (created via vim.api.nvim_create_namespace) as they are idempotent and buffer isolation comes from buffer-specific operations

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/**/*.lua : Use Neovim v0.11.0+ APIs and features; verify that settings, functions, and API usage are compatible with this version or newer

Applied to files:

  • lua/agentic/ui/diff_preview.lua
🔇 Additional comments (7)
lua/agentic/ui/message_writer.lua (3)

1-1: LGTM!

The import of ToolCallDiff module correctly replaces the previous diff handler and aligns with the refactored diff extraction logic used in _prepare_block_lines.


23-32: LGTM!

The ToolCallDiff type definition is well-structured with proper LuaCATS annotations. The optional all field correctly uses the field? syntax for optional types as per coding guidelines.


385-390: LGTM!

The call to ToolCallDiff.extract_diff_blocks correctly passes all required parameters including the new diff.all flag for full-file diff mode.

lua/agentic/ui/diff_preview.lua (4)

19-38: LGTM!

The helper correctly scopes window discovery to the current tabpage, maintaining multi-tabpage safety. The set-based lookup for widget windows is efficient.


74-108: LGTM!

The deletion highlight loop correctly converts 1-indexed block lines to 0-indexed Neovim API calls. Error handling with pcall and vim.notify is appropriate.


110-137: LGTM with a note on edge case.

Virtual lines are correctly anchored after the deletion block. The anchor_line = block.end_line == 0 and 0 or block.end_line - 1 handles insertions at file start, placing new content after line 0 (first line). Verify this is the intended behavior for prepending content to a file.


156-168: LGTM!

The clear_diff function properly handles both buffer numbers and names, with appropriate guards against invalid buffers.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1443b67 and a0f8d1d.

📒 Files selected for processing (2)
  • lua/agentic/config_default.lua
  • lua/agentic/ui/diff_preview.lua
🧰 Additional context used
📓 Path-based instructions (3)
**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

Ensure all features are multi-tabpage safe with one instance per tabpage, managed via SessionRegistry mapping tab_page_id to SessionManager

Files:

  • lua/agentic/config_default.lua
  • lua/agentic/ui/diff_preview.lua
lua/**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

lua/**/*.lua: Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances instead
Use module-level namespace constants for extmarks (created via vim.api.nvim_create_namespace) as they are idempotent and buffer isolation comes from buffer-specific operations
Prefer buffer-local autocommands using vim.api.nvim_create_autocmd with buffer parameter; filter by tabpage in global autocommands if necessary
Keymaps must be buffer-local using BufHelpers.keymap_set(bufnr, mode, key, fn); never use global keymaps affecting all tabpages
Use visibility prefixes for class properties: _* for private (hidden from external consumers), __* for protected (visible to subclasses), no prefix for public (visible everywhere)
Set _index to self for inheritance in Lua classes; use setmetatable to create instances and always pass parent class explicitly when calling parent constructor
In LuaCATS annotations for @param and @field, use 'variable? type' format for optional types; for @return, @type, and @alias use explicit 'type|nil' union format
In fun() type declarations, use explicit 'type|nil' union format instead of 'variable? type' due to LuaLS limitations with optional parameters in function signatures
Include a space after --- in all LuaCATS annotations and descriptions
Always run both 'make luals' (type checking) and 'make luacheck' (style/syntax checking) after making any Lua file changes before completion
Minimize class properties to only those accessed by external code, used as public API, or needed by subclasses/mixins; perform regular cleanup of unused properties
Get tabpage ID correctly using self.tab_page_id in instance methods, from buffer via vim.api.nvim_win_get_tabpage(vim.fn.bufwinid(bufnr)), or current tabpage via vim.api.nvim_get_current_tabpage()
Each tabpage manages its own buffers and windows; never assume buffer/window exists globally using vim.api.nvim_tabpage
* APIs when needed
Use Neovim v0.11.0+ APIs and f...

Files:

  • lua/agentic/config_default.lua
  • lua/agentic/ui/diff_preview.lua
lua/agentic/config_default.lua

📄 CodeRabbit inference engine (AGENTS.md)

When adding or refactoring configuration options in lua/agentic/config_default.lua, update the README.md Configuration section with default values and documentation

Files:

  • lua/agentic/config_default.lua
🧠 Learnings (8)
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/agentic/config_default.lua : When adding or refactoring configuration options in lua/agentic/config_default.lua, update the README.md Configuration section with default values and documentation

Applied to files:

  • lua/agentic/config_default.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : Use module-level namespace constants for extmarks (created via vim.api.nvim_create_namespace) as they are idempotent and buffer isolation comes from buffer-specific operations

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/**/*.lua : Use Neovim v0.11.0+ APIs and features; verify that settings, functions, and API usage are compatible with this version or newer

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances instead

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/**/*.lua : Minimize class properties to only those accessed by external code, used as public API, or needed by subclasses/mixins; perform regular cleanup of unused properties

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : Set __index to self for inheritance in Lua classes; use setmetatable to create instances and always pass parent class explicitly when calling parent constructor

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/**/*.lua : Each tabpage manages its own buffers and windows; never assume buffer/window exists globally using vim.api.nvim_tabpage_* APIs when needed

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/**/*.lua : Get tabpage ID correctly using self.tab_page_id in instance methods, from buffer via vim.api.nvim_win_get_tabpage(vim.fn.bufwinid(bufnr)), or current tabpage via vim.api.nvim_get_current_tabpage()

Applied to files:

  • lua/agentic/ui/diff_preview.lua
🔇 Additional comments (1)
lua/agentic/ui/diff_preview.lua (1)

1-196: Run make luals and make luacheck locally before completion.

Per coding guidelines, both type checking and style/syntax checking must be run after making any Lua file changes. The required tools (lua-language-server and luacheck) are configured in the Makefile but must be executed in your local development environment where these tools are installed (typically via nvim with Mason).

Execute these commands in your repository:

make luals
make luacheck

Ensure both pass without errors before marking this PR as ready for merge.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a0f8d1d and f76eb83.

📒 Files selected for processing (2)
  • AGENTS.md
  • lua/agentic/ui/diff_preview.lua
💤 Files with no reviewable changes (1)
  • AGENTS.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

Ensure all features are multi-tabpage safe with one instance per tabpage, managed via SessionRegistry mapping tab_page_id to SessionManager

Files:

  • lua/agentic/ui/diff_preview.lua
lua/**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

lua/**/*.lua: Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances instead
Use module-level namespace constants for extmarks (created via vim.api.nvim_create_namespace) as they are idempotent and buffer isolation comes from buffer-specific operations
Prefer buffer-local autocommands using vim.api.nvim_create_autocmd with buffer parameter; filter by tabpage in global autocommands if necessary
Keymaps must be buffer-local using BufHelpers.keymap_set(bufnr, mode, key, fn); never use global keymaps affecting all tabpages
Use visibility prefixes for class properties: _* for private (hidden from external consumers), __* for protected (visible to subclasses), no prefix for public (visible everywhere)
Set _index to self for inheritance in Lua classes; use setmetatable to create instances and always pass parent class explicitly when calling parent constructor
In LuaCATS annotations for @param and @field, use 'variable? type' format for optional types; for @return, @type, and @alias use explicit 'type|nil' union format
In fun() type declarations, use explicit 'type|nil' union format instead of 'variable? type' due to LuaLS limitations with optional parameters in function signatures
Include a space after --- in all LuaCATS annotations and descriptions
Always run both 'make luals' (type checking) and 'make luacheck' (style/syntax checking) after making any Lua file changes before completion
Minimize class properties to only those accessed by external code, used as public API, or needed by subclasses/mixins; perform regular cleanup of unused properties
Get tabpage ID correctly using self.tab_page_id in instance methods, from buffer via vim.api.nvim_win_get_tabpage(vim.fn.bufwinid(bufnr)), or current tabpage via vim.api.nvim_get_current_tabpage()
Each tabpage manages its own buffers and windows; never assume buffer/window exists globally using vim.api.nvim_tabpage
* APIs when needed
Use Neovim v0.11.0+ APIs and f...

Files:

  • lua/agentic/ui/diff_preview.lua
🧠 Learnings (8)
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : Use module-level namespace constants for extmarks (created via vim.api.nvim_create_namespace) as they are idempotent and buffer isolation comes from buffer-specific operations

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/**/*.lua : Use Neovim v0.11.0+ APIs and features; verify that settings, functions, and API usage are compatible with this version or newer

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances instead

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : Set __index to self for inheritance in Lua classes; use setmetatable to create instances and always pass parent class explicitly when calling parent constructor

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/**/*.lua : Minimize class properties to only those accessed by external code, used as public API, or needed by subclasses/mixins; perform regular cleanup of unused properties

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/**/*.lua : Each tabpage manages its own buffers and windows; never assume buffer/window exists globally using vim.api.nvim_tabpage_* APIs when needed

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/**/*.lua : Get tabpage ID correctly using self.tab_page_id in instance methods, from buffer via vim.api.nvim_win_get_tabpage(vim.fn.bufwinid(bufnr)), or current tabpage via vim.api.nvim_get_current_tabpage()

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : Prefer buffer-local autocommands using vim.api.nvim_create_autocmd with buffer parameter; filter by tabpage in global autocommands if necessary

Applied to files:

  • lua/agentic/ui/diff_preview.lua
🔇 Additional comments (5)
lua/agentic/ui/diff_preview.lua (5)

1-9: LGTM: Clean module setup.

The module imports and namespace constant are correctly configured per guidelines. The NS_DIFF namespace constant is appropriately stored at module level (idempotent) while avoiding per-tabpage runtime state.


11-15: LGTM: Type annotations are well-formed.

The LuaCATS annotations follow the correct format with proper spacing and clear documentation.


19-38: LGTM: Window discovery logic is sound.

The function correctly identifies non-widget windows within the current tabpage using efficient set-based lookup. The reliance on the current tabpage context is appropriate for this module-level function.


43-58: LGTM: Window creation properly handles errors.

The pcall protection and error notification address the previous review feedback. The split window configuration is compatible with Neovim 0.11.0+.


61-187: LGTM: Comprehensive diff rendering with proper error handling.

The function correctly addresses all previous review concerns:

  • Buffer is set in target window with pcall protection (lines 88-95)
  • All extmark operations are wrapped in pcall (lines 122-139, 153-169)
  • Cursor positioning handles errors gracefully (lines 175-185)

The mode check (lines 67-70) appropriately prevents disrupting the user's workflow, and the virtual line positioning logic (line 144) correctly handles both mid-file and beginning-of-file insertions.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lua/agentic/ui/chat_widget.lua (1)

169-189: Fix variable shadowing in the loop.

The pipeline reports that the loop variable winid on line 176 shadows the winid variable defined on line 169. Use a different variable name for the loop iteration.

🔎 Proposed fix
     local winid = self:find_first_non_widget_window()

     if not winid then
         -- Fallback: create a new left window to avoid closing the last window error
         self:open_left_window()
     end

-    for name, winid in pairs(self.win_nrs) do
+    for name, win_id in pairs(self.win_nrs) do
         self.win_nrs[name] = nil
-        local ok = pcall(vim.api.nvim_win_close, winid, true)
+        local ok = pcall(vim.api.nvim_win_close, win_id, true)
         if not ok then
             Logger.debug(
                 string.format(
                     "Failed to close window '%s' with id: %d",
                     name,
-                    winid
+                    win_id
                 )
             )
         end
     end
♻️ Duplicate comments (1)
lua/agentic/ui/diff_preview.lua (1)

136-148: Document the return value.

The function returns the result of pcall (line 147), which is a boolean indicating success and an optional error value. Adding a @return annotation would improve API documentation for callers who might want to check the result.

🔎 Proposed addition
 --- Clears the diff highlights from the given buffer
 --- @param buf number|string Buffer number or name
+--- @return boolean success Whether the clear operation succeeded
+--- @return any|nil error Error message if the operation failed
 function M.clear_diff(buf)
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f76eb83 and 0de2213.

📒 Files selected for processing (3)
  • lua/agentic/session_manager.lua
  • lua/agentic/ui/chat_widget.lua
  • lua/agentic/ui/diff_preview.lua
🧰 Additional context used
📓 Path-based instructions (2)
**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

Ensure all features are multi-tabpage safe with one instance per tabpage, managed via SessionRegistry mapping tab_page_id to SessionManager

Files:

  • lua/agentic/ui/chat_widget.lua
  • lua/agentic/session_manager.lua
  • lua/agentic/ui/diff_preview.lua
lua/**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

lua/**/*.lua: Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances instead
Use module-level namespace constants for extmarks (created via vim.api.nvim_create_namespace) as they are idempotent and buffer isolation comes from buffer-specific operations
Prefer buffer-local autocommands using vim.api.nvim_create_autocmd with buffer parameter; filter by tabpage in global autocommands if necessary
Keymaps must be buffer-local using BufHelpers.keymap_set(bufnr, mode, key, fn); never use global keymaps affecting all tabpages
Use visibility prefixes for class properties: _* for private (hidden from external consumers), __* for protected (visible to subclasses), no prefix for public (visible everywhere)
Set _index to self for inheritance in Lua classes; use setmetatable to create instances and always pass parent class explicitly when calling parent constructor
In LuaCATS annotations for @param and @field, use 'variable? type' format for optional types; for @return, @type, and @alias use explicit 'type|nil' union format
In fun() type declarations, use explicit 'type|nil' union format instead of 'variable? type' due to LuaLS limitations with optional parameters in function signatures
Include a space after --- in all LuaCATS annotations and descriptions
Always run both 'make luals' (type checking) and 'make luacheck' (style/syntax checking) after making any Lua file changes before completion
Minimize class properties to only those accessed by external code, used as public API, or needed by subclasses/mixins; perform regular cleanup of unused properties
Get tabpage ID correctly using self.tab_page_id in instance methods, from buffer via vim.api.nvim_win_get_tabpage(vim.fn.bufwinid(bufnr)), or current tabpage via vim.api.nvim_get_current_tabpage()
Each tabpage manages its own buffers and windows; never assume buffer/window exists globally using vim.api.nvim_tabpage
* APIs when needed
Use Neovim v0.11.0+ APIs and f...

Files:

  • lua/agentic/ui/chat_widget.lua
  • lua/agentic/session_manager.lua
  • lua/agentic/ui/diff_preview.lua
🧠 Learnings (14)
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/**/*.lua : Each tabpage manages its own buffers and windows; never assume buffer/window exists globally using vim.api.nvim_tabpage_* APIs when needed

Applied to files:

  • lua/agentic/ui/chat_widget.lua
  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/**/*.lua : Get tabpage ID correctly using self.tab_page_id in instance methods, from buffer via vim.api.nvim_win_get_tabpage(vim.fn.bufwinid(bufnr)), or current tabpage via vim.api.nvim_get_current_tabpage()

Applied to files:

  • lua/agentic/ui/chat_widget.lua
  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to **/*.lua : Ensure all features are multi-tabpage safe with one instance per tabpage, managed via SessionRegistry mapping tab_page_id to SessionManager

Applied to files:

  • lua/agentic/session_manager.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : In fun() type declarations, use explicit 'type|nil' union format instead of 'variable? type' due to LuaLS limitations with optional parameters in function signatures

Applied to files:

  • lua/agentic/session_manager.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/agentic/acp/acp_client.lua : Never remove critical type annotations in lua/agentic/acp/acp_client.lua; only update them when underlying types change

Applied to files:

  • lua/agentic/session_manager.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : In LuaCATS annotations for param and field, use 'variable? type' format for optional types; for return, type, and alias use explicit 'type|nil' union format

Applied to files:

  • lua/agentic/session_manager.lua
📚 Learning: 2025-12-30T12:30:22.653Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: tests/AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:22.653Z
Learning: Applies to tests/**/*_spec.lua : Do not use custom error messages as a second argument to Busted assertions to avoid redundant-parameter warnings from LuaLS

Applied to files:

  • lua/agentic/session_manager.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/agentic/acp/**/*.lua : Each ACP provider tabpage instance maintains its own session ID from the shared ACP provider; never mix session IDs between tabpages

Applied to files:

  • lua/agentic/session_manager.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : Use module-level namespace constants for extmarks (created via vim.api.nvim_create_namespace) as they are idempotent and buffer isolation comes from buffer-specific operations

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/**/*.lua : Use Neovim v0.11.0+ APIs and features; verify that settings, functions, and API usage are compatible with this version or newer

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances instead

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/**/*.lua : Minimize class properties to only those accessed by external code, used as public API, or needed by subclasses/mixins; perform regular cleanup of unused properties

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : Set __index to self for inheritance in Lua classes; use setmetatable to create instances and always pass parent class explicitly when calling parent constructor

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/**/*.lua : Prefer buffer-local autocommands using vim.api.nvim_create_autocmd with buffer parameter; filter by tabpage in global autocommands if necessary

Applied to files:

  • lua/agentic/ui/diff_preview.lua
🪛 GitHub Actions: PR Checks
lua/agentic/ui/chat_widget.lua

[warning] 176-176: shadowing definition of variable 'winid' on line 176 (previously defined on line 169)

lua/agentic/ui/diff_preview.lua

[warning] 69-69: shadowing definition of variable 'ok' on line 69 (previously defined on line 36)


[warning] 69-69: shadowing definition of variable 'err' on line 69 (previously defined on line 36)


[warning] 100-100: shadowing definition of variable 'ok' on line 100 (previously defined on line 36)


[warning] 100-100: shadowing definition of variable 'err' on line 100 (previously defined on line 36)


[warning] 122-122: shadowing definition of variable 'ok' on line 122 (previously defined on line 36)


[warning] 122-122: shadowing definition of variable 'err' on line 122 (previously defined on line 36)

🔇 Additional comments (4)
lua/agentic/session_manager.lua (3)

417-421: Good integration of diff clearing on tool call updates.

Pre-emptively clearing the diff preview when tool call updates are received ensures that stale diffs are removed when the tool execution completes or fails. This prevents confusion from outdated previews.


440-458: Well-designed diff preview lifecycle for permission requests.

The integration correctly shows the diff preview before the permission request (line 456) and ensures cleanup after the user responds (line 446). The wrapped callback pattern guarantees the diff is cleared regardless of which permission option is chosen.


595-605: LGTM: Clean diff clearing logic.

The method correctly guards against invalid states and uses the file path to identify which buffer to clear. The logic is symmetric with _show_diff_in_buffer.

lua/agentic/ui/diff_preview.lua (1)

1-14: Good module structure and namespace usage.

The singleton module pattern is appropriate for this utility. Using a module-level NS_DIFF namespace constant follows the coding guidelines: "Use module-level namespace constants for extmarks (created via vim.api.nvim_create_namespace) as they are idempotent and buffer isolation comes from buffer-specific operations."

Based on coding guidelines.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Fix all issues with AI Agents 🤖
In @lua/agentic/ui/tool_call_diff.lua:
- Around line 79-81: The duplicated compatibility shim for resolving the diff
function appears in _minimize_diff_blocks and minimize_diff; remove the fallback
and use vim.text.diff directly (or, if you must retain compatibility, extract a
single module-level helper like local diff_fn = vim.text and vim.text.diff or
vim.diff and reference diff_fn from both _minimize_diff_blocks and
minimize_diff) so the resolution is not duplicated; update both functions to
call the shared helper or vim.text.diff directly and delete the duplicate local
diff_fn declarations.
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8c216fe and 58d515b.

📒 Files selected for processing (3)
  • lua/agentic/ui/message_writer.lua
  • lua/agentic/ui/tool_call_diff.lua
  • lua/agentic/utils/text_matcher.lua
🧰 Additional context used
📓 Path-based instructions (2)
**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

Ensure all features are multi-tabpage safe with one instance per tabpage, managed via SessionRegistry mapping tab_page_id to SessionManager

Files:

  • lua/agentic/ui/message_writer.lua
  • lua/agentic/utils/text_matcher.lua
  • lua/agentic/ui/tool_call_diff.lua
lua/**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

lua/**/*.lua: Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances instead
Use module-level namespace constants for extmarks (created via vim.api.nvim_create_namespace) as they are idempotent and buffer isolation comes from buffer-specific operations
Prefer buffer-local autocommands using vim.api.nvim_create_autocmd with buffer parameter; filter by tabpage in global autocommands if necessary
Keymaps must be buffer-local using BufHelpers.keymap_set(bufnr, mode, key, fn); never use global keymaps affecting all tabpages
Use visibility prefixes for class properties: _* for private (hidden from external consumers), __* for protected (visible to subclasses), no prefix for public (visible everywhere)
Set _index to self for inheritance in Lua classes; use setmetatable to create instances and always pass parent class explicitly when calling parent constructor
In LuaCATS annotations for @param and @field, use 'variable? type' format for optional types; for @return, @type, and @alias use explicit 'type|nil' union format
In fun() type declarations, use explicit 'type|nil' union format instead of 'variable? type' due to LuaLS limitations with optional parameters in function signatures
Include a space after --- in all LuaCATS annotations and descriptions
Always run both 'make luals' (type checking) and 'make luacheck' (style/syntax checking) after making any Lua file changes before completion
Minimize class properties to only those accessed by external code, used as public API, or needed by subclasses/mixins; perform regular cleanup of unused properties
Get tabpage ID correctly using self.tab_page_id in instance methods, from buffer via vim.api.nvim_win_get_tabpage(vim.fn.bufwinid(bufnr)), or current tabpage via vim.api.nvim_get_current_tabpage()
Each tabpage manages its own buffers and windows; never assume buffer/window exists globally using vim.api.nvim_tabpage
* APIs when needed
Use Neovim v0.11.0+ APIs and f...

Files:

  • lua/agentic/ui/message_writer.lua
  • lua/agentic/utils/text_matcher.lua
  • lua/agentic/ui/tool_call_diff.lua
🧠 Learnings (3)
📚 Learning: 2025-12-30T12:30:06.857Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.857Z
Learning: Applies to lua/agentic/acp/acp_client.lua : Never remove critical type annotations in lua/agentic/acp/acp_client.lua; only update them when underlying types change

Applied to files:

  • lua/agentic/ui/message_writer.lua
  • lua/agentic/ui/tool_call_diff.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/agentic/theme.lua : Highlight groups are global and defined once in lua/agentic/theme.lua; use namespaces to control WHERE highlights appear, not to isolate definitions

Applied to files:

  • lua/agentic/ui/message_writer.lua
📚 Learning: 2025-12-30T12:30:06.856Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:06.856Z
Learning: Applies to lua/agentic/theme.lua : When adding new highlight groups to lua/agentic/theme.lua, add the group to Theme.HL_GROUPS constant, define in Theme.setup(), and update README.md Customization section

Applied to files:

  • lua/agentic/ui/message_writer.lua
🔇 Additional comments (12)
lua/agentic/ui/message_writer.lua (5)

1-1: LGTM!

Import updated to use the new ToolCallDiff module, aligning with the refactored diff representation.


23-27: LGTM!

Type definition follows LuaCATS conventions. The all? boolean format is correct for @field annotations per coding guidelines.


32-32: LGTM!

Field type updated to reference the new local type definition.


385-388: LGTM!

The fallback to empty arrays (or {}) is defensive and handles cases where diff.old or diff.new might be nil, which aligns with the optional nature of the diff field.


398-456: LGTM!

The hunk-based iteration logic correctly handles:

  • New file case (old_count == 0)
  • Modification case (equal old/new counts)
  • Addition/deletion cases with proper highlight range mapping

The index lookups (hunk.old_lines[i], hunk.new_lines[i]) are safe within the iteration bounds.

lua/agentic/utils/text_matcher.lua (3)

4-6: LGTM!

Clean type definition with proper LuaCATS annotations for the match result structure.


12-38: LGTM!

The exact matching implementation is clean and correct:

  • Early exit for empty/invalid inputs at lines 15-21
  • Loop bounds prevent out-of-bounds access
  • Non-overlapping matches via skip-ahead logic

40-53: Consider using a visibility prefix consistent with guidelines.

Per coding guidelines, private helpers should use _ prefix. The function already uses _matches_at_position which is correct. The implementation is sound—line-by-line exact comparison returns false immediately on mismatch.

lua/agentic/ui/tool_call_diff.lua (4)

1-12: LGTM!

Type definitions are clean and follow LuaCATS conventions. The separation of DiffBlock (with file positioning) and Hunk (content-only) provides clear semantic distinction.


22-63: LGTM!

The refactored extract_diff_blocks correctly:

  • Uses TextMatcher.find_all_matches for exact matching
  • Respects replace_all flag to limit matches
  • Falls back gracefully with debug logging when no matches found

183-235: LGTM!

The new minimize_diff function correctly handles all edge cases:

  • Empty old_lines returns single hunk with all new content
  • Single-line pairs returned as-is
  • Falls back to full content when vim.diff returns no patches but content differs
  • Returns empty array when content is identical

This provides a clean API for MessageWriter to consume content-only hunks.


66-145: LGTM!

The _minimize_diff_blocks function correctly preserves file positioning information (start_line, end_line) while minimizing the diff content. The sort at the end ensures blocks are ordered by position.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Fix all issues with AI Agents 🤖
In @lua/agentic/ui/diff_preview.lua:
- Around line 138-142: The file currently calls vim.notify directly in several
places (e.g., the "Failed to set buffer in window" branch) which violates
guidelines; add a Logger require at the top of the module (e.g., add a local
Logger = require(...) to the module's requires) and replace every
vim.notify(...) call in this file with Logger.notify(...) preserving the same
message and log-level argument(s); ensure you update the instances around the
failing buffer branch and the other occurrences (previously at the other notify
sites) so all notifications use Logger.notify instead of vim.notify.
♻️ Duplicate comments (1)
lua/agentic/ui/diff_preview.lua (1)

236-248: Add return value documentation.

The function returns the result of pcall (success boolean and error value) but lacks a @return annotation. Document the return value for API clarity.

🔎 Proposed fix
 --- Clears the diff highlights from the given buffer
 --- @param buf number|string Buffer number or name
+--- @return boolean success Whether the clear operation succeeded
+--- @return any|nil error Error message if the operation failed
 function M.clear_diff(buf)

As per coding guidelines: Use explicit type|nil union syntax for return types.

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 62cbf4d and 5c59935.

📒 Files selected for processing (3)
  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/message_writer.lua
  • lua/agentic/ui/tool_call_diff.lua
🧰 Additional context used
📓 Path-based instructions (2)
**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

**/*.lua: Always run both Lua linters after making ANY Lua file changes: make luals for type checking and make luacheck for style/syntax checking
Use LuaCATS annotations with proper spacing: include a space after --- for both descriptions and annotations
For @param and @field annotations, use variable? type format where the ? goes AFTER the variable name to indicate optional parameters
For @return, @type, and @alias annotations, use explicit type|nil union instead of type? syntax to make optional nature explicit
In fun() type declarations, use explicit type|nil union syntax instead of optional variable? syntax because LuaLS ignores optional syntax in function declarations
Set __index = self for Lua classes to enable inheritance, use setmetatable to create instances, and return the instance from constructor
For class inheritance, call parent constructor explicitly with parent class as first argument: Parent.new(Parent, ...) and re-metatable instance to child class
Use visibility prefixes in Lua class properties: _* for private, __* for protected, no prefix for public

Files:

  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/message_writer.lua
  • lua/agentic/ui/tool_call_diff.lua
lua/agentic/**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

lua/agentic/**/*.lua: Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances only
Namespace IDs can be defined at module level (shared across tabpages) because extmarks are buffer-scoped and each tabpage has its own buffers
Always use buffer-local keymaps with BufHelpers.keymap_set(bufnr, 'n', 'key', fn) instead of global keymaps to ensure tabpage isolation
Prefer buffer-local autocommands using vim.api.nvim_create_autocmd(..., { buffer = bufnr }) instead of global autocommands for tabpage safety
Never use vim.notify directly; always use Logger.notify() to prevent 'fast context is active' errors in agentic.nvim

Files:

  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/message_writer.lua
  • lua/agentic/ui/tool_call_diff.lua
🧠 Learnings (9)
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Namespace IDs can be defined at module level (shared across tabpages) because extmarks are buffer-scoped and each tabpage has its own buffers

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances only

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : Set `__index = self` for Lua classes to enable inheritance, use `setmetatable` to create instances, and return the instance from constructor

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Prefer buffer-local autocommands using `vim.api.nvim_create_autocmd(..., { buffer = bufnr })` instead of global autocommands for tabpage safety

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Never use `vim.notify` directly; always use `Logger.notify()` to prevent 'fast context is active' errors in agentic.nvim

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Always use buffer-local keymaps with `BufHelpers.keymap_set(bufnr, 'n', 'key', fn)` instead of global keymaps to ensure tabpage isolation

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : For `return`, `type`, and `alias` annotations, use explicit `type|nil` union instead of `type?` syntax to make optional nature explicit

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/theme.lua : When adding new highlight groups in theme.lua, add the group to `Theme.HL_GROUPS` constant and update the README.md 'Customization (Ricing)' section

Applied to files:

  • lua/agentic/ui/message_writer.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/theme.lua : Highlight group definitions are global and should be defined once in theme.lua; highlight groups apply to all buffers/windows/tabpages

Applied to files:

  • lua/agentic/ui/message_writer.lua
🧬 Code graph analysis (1)
lua/agentic/ui/tool_call_diff.lua (2)
lua/agentic/utils/file_system.lua (2)
  • FileSystem.to_absolute_path (118-120)
  • FileSystem.read_from_buffer_or_disk (11-33)
lua/agentic/utils/logger.lua (1)
  • Logger.debug (48-54)
🔇 Additional comments (9)
lua/agentic/ui/message_writer.lua (2)

1-1: LGTM: Clean refactoring to ToolCallDiff.

The refactoring from ACPDiffHandler to ToolCallDiff is well-executed with proper type annotations and namespace updates.

Also applies to: 23-26, 32-32


385-390: LGTM: Named parameters improve API clarity.

The updated call to ToolCallDiff.extract_diff_blocks with named parameters aligns with the new ExtractOpts definition and improves readability.

lua/agentic/ui/diff_preview.lua (3)

14-58: LGTM: Robust treesitter highlight mapping.

The build_highlight_map function properly handles treesitter parsing failures with pcall and returns nil for edge cases. The row->col->hl_group mapping logic is correct.


64-109: LGTM: Well-structured virtual line generation.

The get_highlighted_virt_lines function correctly segments lines by highlight groups and combines syntax highlighting with diff backgrounds. Edge cases (empty lines, no highlights) are handled appropriately.


147-234: LGTM: Correct diff rendering with treesitter integration.

The diff rendering logic properly:

  • Uses strict=true to prevent fallback blocks in preview mode
  • Applies deletion highlights to existing lines
  • Generates syntax-highlighted virtual lines for additions
  • Jumps cursor to the first diff block

The variable reuse (ok, err) is acceptable in sequential operations with distinct error contexts.

lua/agentic/ui/tool_call_diff.lua (4)

1-13: LGTM: Clean API refactoring with named parameters.

The refactoring to use an opts table with ExtractOpts improves API clarity and extensibility. The strict option properly prevents fallback blocks when matching fails, which is essential for the diff preview feature.

Also applies to: 21-70


72-158: LGTM: Consistent type updates in minimization logic.

The _minimize_diff_blocks function has been properly updated with the new agentic.ui.ToolCallDiff.DiffBlock type annotations throughout. The diff minimization logic correctly uses vim.text.diff with a fallback for compatibility.


160-175: LGTM: New file diff block creation updated correctly.

The return type and internal type annotation have been properly updated to use the new agentic.ui.ToolCallDiff.DiffBlock namespace.


192-266: LGTM: Matching logic updated with consistent types.

The fuzzy matching and substring replacement functions have been properly updated with the new agentic.ui.ToolCallDiff.DiffBlock type annotations. The logic remains intact while maintaining type consistency.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Fix all issues with AI Agents 🤖
In @lua/agentic/ui/diff_preview.lua:
- Around line 246-262: Replace the direct call to vim.notify inside the pcall
error branch with Logger.notify() (do not call vim.notify in lua/agentic files);
pass the same error message built from tostring(err) and map the warning level
to Logger's notify level API, keeping the surrounding context (the pcall around
vim.api.nvim_buf_set_extmark, variables ok/err, bufnr, NS_DIFF, anchor_line,
virt_lines) intact so only the notification mechanism changes.
♻️ Duplicate comments (4)
lua/agentic/ui/diff_preview.lua (4)

282-294: Add missing @return annotation.

The function returns the result of pcall (boolean success, any error) on line 293, but the @return annotation is missing from the documentation.

🔎 Proposed fix
 --- Clears the diff highlights from the given buffer
 --- @param buf number|string Buffer number or name
+--- @return boolean success Whether the clear operation succeeded
+--- @return any error Error message if the operation failed
 function M.clear_diff(buf)

1-4: Add Logger import to replace vim.notify calls.

The file uses vim.notify directly on lines 188, 258, and 274, which violates the coding guidelines. Add the Logger module import here so those calls can be replaced with Logger.notify().

🔎 Proposed fix
 local DiffHighlighter = require("agentic.utils.diff_highlighter")
 local ToolCallDiff = require("agentic.ui.tool_call_diff")
 local Theme = require("agentic.theme")
+local Logger = require("agentic.utils.logger")

As per coding guidelines, Never use vim.notify directly in lua/agentic/**/*.lua files.


186-193: Replace vim.notify with Logger.notify().

This call uses vim.notify directly, which violates coding guidelines and can cause 'fast context is active' errors in agentic.nvim.

🔎 Proposed fix
     local ok, err = pcall(vim.api.nvim_win_set_buf, target_winid, bufnr)
     if not ok then
-        vim.notify(
+        Logger.notify(
             "Failed to set buffer in window: " .. tostring(err),
             vim.log.levels.WARN
         )
         return
     end

As per coding guidelines, Never use vim.notify directly in lua/agentic/**/*.lua files.


268-279: Replace vim.notify with Logger.notify().

This call uses vim.notify directly, which violates coding guidelines and can cause 'fast context is active' errors in agentic.nvim.

🔎 Proposed fix
         ok, err = pcall(
             vim.api.nvim_win_set_cursor,
             target_winid,
             { first_block.start_line, 0 }
         )
         if not ok then
-            vim.notify(
+            Logger.notify(
                 "Failed to jump to first diff block: " .. tostring(err),
                 vim.log.levels.WARN
             )
         end

As per coding guidelines, Never use vim.notify directly in lua/agentic/**/*.lua files.

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5c59935 and 7f08a31.

📒 Files selected for processing (2)
  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/utils/file_system.lua
🧰 Additional context used
📓 Path-based instructions (2)
**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

**/*.lua: Always run both Lua linters after making ANY Lua file changes: make luals for type checking and make luacheck for style/syntax checking
Use LuaCATS annotations with proper spacing: include a space after --- for both descriptions and annotations
For @param and @field annotations, use variable? type format where the ? goes AFTER the variable name to indicate optional parameters
For @return, @type, and @alias annotations, use explicit type|nil union instead of type? syntax to make optional nature explicit
In fun() type declarations, use explicit type|nil union syntax instead of optional variable? syntax because LuaLS ignores optional syntax in function declarations
Set __index = self for Lua classes to enable inheritance, use setmetatable to create instances, and return the instance from constructor
For class inheritance, call parent constructor explicitly with parent class as first argument: Parent.new(Parent, ...) and re-metatable instance to child class
Use visibility prefixes in Lua class properties: _* for private, __* for protected, no prefix for public

Files:

  • lua/agentic/utils/file_system.lua
  • lua/agentic/ui/diff_preview.lua
lua/agentic/**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

lua/agentic/**/*.lua: Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances only
Namespace IDs can be defined at module level (shared across tabpages) because extmarks are buffer-scoped and each tabpage has its own buffers
Always use buffer-local keymaps with BufHelpers.keymap_set(bufnr, 'n', 'key', fn) instead of global keymaps to ensure tabpage isolation
Prefer buffer-local autocommands using vim.api.nvim_create_autocmd(..., { buffer = bufnr }) instead of global autocommands for tabpage safety
Never use vim.notify directly; always use Logger.notify() to prevent 'fast context is active' errors in agentic.nvim

Files:

  • lua/agentic/utils/file_system.lua
  • lua/agentic/ui/diff_preview.lua
🧠 Learnings (7)
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Prefer buffer-local autocommands using `vim.api.nvim_create_autocmd(..., { buffer = bufnr })` instead of global autocommands for tabpage safety

Applied to files:

  • lua/agentic/utils/file_system.lua
  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Namespace IDs can be defined at module level (shared across tabpages) because extmarks are buffer-scoped and each tabpage has its own buffers

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances only

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : Set `__index = self` for Lua classes to enable inheritance, use `setmetatable` to create instances, and return the instance from constructor

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Never use `vim.notify` directly; always use `Logger.notify()` to prevent 'fast context is active' errors in agentic.nvim

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Always use buffer-local keymaps with `BufHelpers.keymap_set(bufnr, 'n', 'key', fn)` instead of global keymaps to ensure tabpage isolation

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : For `return`, `type`, and `alias` annotations, use explicit `type|nil` union instead of `type?` syntax to make optional nature explicit

Applied to files:

  • lua/agentic/ui/diff_preview.lua
🔇 Additional comments (4)
lua/agentic/utils/file_system.lua (1)

73-74: LGTM! Appropriate use of silent! edit! for buffer reload.

The silent! edit! command correctly handles the W13 warning scenario for newly created files. The ! modifier forces the reload from disk after writing, and window view preservation ensures the user's cursor position and scroll state are maintained.

The edit! command will discard any unsaved buffer modifications, but this is acceptable here since the function just wrote the intended content to disk, and this operation is plugin-controlled within the diff-preview workflow.

lua/agentic/ui/diff_preview.lua (3)

11-59: LGTM!

The function correctly uses pcall for treesitter API calls and has proper LuaCATS annotations with explicit |nil union syntax.


61-70: Fix LuaCATS annotation syntax for optional parameter.

Line 63 uses change? table format, but per coding guidelines, optional parameters should use explicit type|nil union syntax.

🔎 Proposed fix
 --- Get the diff highlight for a column position based on word-level change
 --- @param col integer 0-indexed column
---- @param change? table Change info from find_inline_change
+--- @param change table|nil Change info from find_inline_change
 --- @return string hl_group
 local function get_diff_hl_for_col(col, change)

As per coding guidelines, for @return, @type, and @alias annotations, use explicit type|nil union instead of type? syntax.

⛔ Skipped due to learnings
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : For `return`, `type`, and `alias` annotations, use explicit `type|nil` union instead of `type?` syntax to make optional nature explicit
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : In `fun()` type declarations, use explicit `type|nil` union syntax instead of optional `variable?` syntax because LuaLS ignores optional syntax in function declarations
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : For `param` and `field` annotations, use `variable? type` format where the `?` goes AFTER the variable name to indicate optional parameters
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : Use LuaCATS annotations with proper spacing: include a space after `---` for both descriptions and annotations
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/theme.lua : When adding new highlight groups in theme.lua, add the group to `Theme.HL_GROUPS` constant and update the README.md 'Customization (Ricing)' section

72-159: Fix LuaCATS annotation syntax for optional parameter.

Line 74 uses old_lines? string[] format, but per coding guidelines, optional parameters should use explicit type|nil union syntax.

🔎 Proposed fix
 --- Builds virt_lines with syntax highlighting and diff background
 --- @param new_lines string[]
---- @param old_lines? string[] Optional old lines for word-level diff
+--- @param old_lines string[]|nil Optional old lines for word-level diff
 --- @param lang string
 --- @return table virt_lines
 local function get_highlighted_virt_lines(new_lines, old_lines, lang)

As per coding guidelines, for @param annotations, use explicit type|nil union instead of type? syntax.

⛔ Skipped due to learnings
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : For `return`, `type`, and `alias` annotations, use explicit `type|nil` union instead of `type?` syntax to make optional nature explicit
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : In `fun()` type declarations, use explicit `type|nil` union syntax instead of optional `variable?` syntax because LuaLS ignores optional syntax in function declarations
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : For `param` and `field` annotations, use `variable? type` format where the `?` goes AFTER the variable name to indicate optional parameters
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : Use LuaCATS annotations with proper spacing: include a space after `---` for both descriptions and annotations

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI Agents
In @lua/agentic/ui/chat_widget.lua:
- Line 656: The fallback buffer is being created as a listed, listed-modifiable
buffer with vim.api.nvim_create_buf(true, false) — change this to create an
unlisted scratch buffer by calling vim.api.nvim_create_buf(false, true) where
bufnr is assigned in the fallback case in chat_widget.lua so the temporary
window doesn't appear in buffer lists and behaves as a scratch buffer.
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f08a31 and cb31cf1.

📒 Files selected for processing (5)
  • AGENTS.md
  • lua/agentic/session_manager.lua
  • lua/agentic/ui/chat_widget.lua
  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/utils/file_system.lua
💤 Files with no reviewable changes (1)
  • AGENTS.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

**/*.lua: Always run both Lua linters after making ANY Lua file changes: make luals for type checking and make luacheck for style/syntax checking
Use LuaCATS annotations with proper spacing: include a space after --- for both descriptions and annotations
For @param and @field annotations, use variable? type format where the ? goes AFTER the variable name to indicate optional parameters
For @return, @type, and @alias annotations, use explicit type|nil union instead of type? syntax to make optional nature explicit
In fun() type declarations, use explicit type|nil union syntax instead of optional variable? syntax because LuaLS ignores optional syntax in function declarations
Set __index = self for Lua classes to enable inheritance, use setmetatable to create instances, and return the instance from constructor
For class inheritance, call parent constructor explicitly with parent class as first argument: Parent.new(Parent, ...) and re-metatable instance to child class
Use visibility prefixes in Lua class properties: _* for private, __* for protected, no prefix for public

Files:

  • lua/agentic/utils/file_system.lua
  • lua/agentic/session_manager.lua
  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/chat_widget.lua
lua/agentic/**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

lua/agentic/**/*.lua: Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances only
Namespace IDs can be defined at module level (shared across tabpages) because extmarks are buffer-scoped and each tabpage has its own buffers
Always use buffer-local keymaps with BufHelpers.keymap_set(bufnr, 'n', 'key', fn) instead of global keymaps to ensure tabpage isolation
Prefer buffer-local autocommands using vim.api.nvim_create_autocmd(..., { buffer = bufnr }) instead of global autocommands for tabpage safety
Never use vim.notify directly; always use Logger.notify() to prevent 'fast context is active' errors in agentic.nvim

Files:

  • lua/agentic/utils/file_system.lua
  • lua/agentic/session_manager.lua
  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/chat_widget.lua
🧠 Learnings (9)
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Prefer buffer-local autocommands using `vim.api.nvim_create_autocmd(..., { buffer = bufnr })` instead of global autocommands for tabpage safety

Applied to files:

  • lua/agentic/utils/file_system.lua
  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Namespace IDs can be defined at module level (shared across tabpages) because extmarks are buffer-scoped and each tabpage has its own buffers

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances only

Applied to files:

  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : Set `__index = self` for Lua classes to enable inheritance, use `setmetatable` to create instances, and return the instance from constructor

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Never use `vim.notify` directly; always use `Logger.notify()` to prevent 'fast context is active' errors in agentic.nvim

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to lua/agentic/**/*.lua : Always use buffer-local keymaps with `BufHelpers.keymap_set(bufnr, 'n', 'key', fn)` instead of global keymaps to ensure tabpage isolation

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Applies to **/*.lua : For `return`, `type`, and `alias` annotations, use explicit `type|nil` union instead of `type?` syntax to make optional nature explicit

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.028Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.028Z
Learning: Every feature in agentic.nvim must be multi-tabpage safe with support for one instance per tabpage via SessionRegistry, shared ACP provider instance, and isolated ChatWidget per tabpage

Applied to files:

  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2025-12-30T12:30:22.653Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: tests/AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:22.653Z
Learning: Applies to tests/**/*_spec.lua : Tests should verify tabpage isolation to ensure no cross-contamination of session state across different tabpages

Applied to files:

  • lua/agentic/ui/chat_widget.lua
🧬 Code graph analysis (2)
lua/agentic/ui/diff_preview.lua (1)
lua/agentic/utils/logger.lua (1)
  • Logger.notify (50-64)
lua/agentic/ui/chat_widget.lua (1)
lua/agentic/utils/logger.lua (1)
  • Logger.notify (50-64)
🔇 Additional comments (10)
lua/agentic/utils/file_system.lua (1)

74-75: LGTM! Buffer reload strategy is appropriate.

The change from checktime to silent! edit! with the silent prefix properly avoids the W13 warning when the file is created after the buffer, which is common for newly created files during diff preview flows.

lua/agentic/ui/chat_widget.lua (2)

605-625: LGTM! Tabpage-safe window lookup.

The method correctly uses self.tab_page_id to retrieve windows from the instance's tabpage, ensuring multi-tabpage safety per coding guidelines.


169-174: LGTM! Fallback logic prevents closing last window.

The logic correctly finds or creates a non-widget window before closing widget windows, preventing the "Cannot close last window" error.

lua/agentic/session_manager.lua (3)

7-8: LGTM! Module imports support diff preview integration.

The addition of Config and DiffPreview imports enables the new diff preview functionality gated by configuration.


558-586: LGTM! Diff preview logic is well-guarded.

The method properly:

  • Gates on Config.diff_preview.enabled
  • Validates tracker existence, kind, and diff presence
  • Ensures a target window exists or creates one
  • Checks the nil return from open_left_window() before proceeding (addresses past review concern)

414-415: LGTM! Proactive diff cleanup on tool call updates.

Clearing the diff preview when a tool call update is received ensures the preview is removed once the operation completes or fails, maintaining a clean UI state.

lua/agentic/ui/diff_preview.lua (4)

16-60: LGTM! Treesitter-based syntax highlighting is well-implemented.

The build_highlight_map function correctly uses treesitter to parse code blocks and build a column-level highlight map for preserving syntax highlighting in virtual lines.


78-160: LGTM! Virtual line highlighting combines syntax and diff context.

The get_highlighted_virt_lines function elegantly combines treesitter syntax highlighting with word-level diff highlighting for modifications, providing rich visual feedback.


168-191: LGTM! Diff preview setup is robust.

The initial setup correctly:

  • Validates target window and mode
  • Resolves or creates the buffer
  • Wraps nvim_win_set_buf in pcall with proper error handling
  • Uses Logger.notify() per coding guidelines

1-290: Run both Lua linters on this new file.

Per coding guidelines for **/*.lua files, always run make luals for type checking and make luacheck for style/syntax checking after creating any Lua file changes.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @lua/agentic/session_manager.lua:
- Around line 571-582: The call to vim.api.nvim_win_set_buf inside
DiffPreview.show_diff's get_winid should be wrapped in pcall to match module
error-handling patterns; change the logic in the get_winid function (which
currently calls self.widget:find_first_non_widget_window and
vim.api.nvim_win_set_buf(winid, bufnr), falling back to
self.widget:open_left_window) so that the vim.api.nvim_win_set_buf call is
executed inside a pcall and on failure you fall back to opening/returning the
left window (i.e., call self.widget:open_left_window(bufnr) or handle the error
result) and log or ignore the pcall error consistently with other API calls in
this module.

In @lua/agentic/ui/chat_widget.lua:
- Around line 650-703: The fallback scratch buffer created in
ChatWidget:open_left_window is currently created as a listed, non-scratch buffer
via vim.api.nvim_create_buf(true, false); change this to create an unlisted
scratch buffer by calling vim.api.nvim_create_buf(false, true) (i.e.,
listed=false, scratch=true) so the temporary fallback window uses an unlisted
scratch buffer instead of a regular listed buffer.
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cb31cf1 and 71596da.

📒 Files selected for processing (3)
  • lua/agentic/session_manager.lua
  • lua/agentic/ui/chat_widget.lua
  • lua/agentic/ui/diff_preview.lua
🧰 Additional context used
📓 Path-based instructions (2)
**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

**/*.lua: Always run both Lua linters after making ANY Lua file changes: make luals for type checking and make luacheck for style/syntax checking
Use LuaCATS annotations with proper spacing: include a space after --- for both descriptions and annotations
For @param and @field annotations, use variable? type format where the ? goes AFTER the variable name to indicate optional parameters
For @return, @type, and @alias annotations, use explicit type|nil union instead of type? syntax to make optional nature explicit
In fun() type declarations, use explicit type|nil union syntax instead of optional variable? syntax because LuaLS ignores optional syntax in function declarations
Set __index = self for Lua classes to enable inheritance, use setmetatable to create instances, and return the instance from constructor
For class inheritance, call parent constructor explicitly with parent class as first argument: Parent.new(Parent, ...) and re-metatable instance to child class
Use visibility prefixes in Lua class properties: _* for private, __* for protected, no prefix for public

Files:

  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/session_manager.lua
  • lua/agentic/ui/chat_widget.lua
lua/agentic/**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

lua/agentic/**/*.lua: Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances only
Namespace IDs can be defined at module level (shared across tabpages) because extmarks are buffer-scoped and each tabpage has its own buffers
Always use buffer-local keymaps with BufHelpers.keymap_set(bufnr, 'n', 'key', fn) instead of global keymaps to ensure tabpage isolation
Prefer buffer-local autocommands using vim.api.nvim_create_autocmd(..., { buffer = bufnr }) instead of global autocommands for tabpage safety
Never use vim.notify directly; always use Logger.notify() to prevent 'fast context is active' errors in agentic.nvim

Files:

  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/session_manager.lua
  • lua/agentic/ui/chat_widget.lua
🧠 Learnings (9)
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/**/*.lua : Namespace IDs can be defined at module level (shared across tabpages) because extmarks are buffer-scoped and each tabpage has its own buffers

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/**/*.lua : Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances only

Applied to files:

  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to **/*.lua : Set `__index = self` for Lua classes to enable inheritance, use `setmetatable` to create instances, and return the instance from constructor

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/**/*.lua : Prefer buffer-local autocommands using `vim.api.nvim_create_autocmd(..., { buffer = bufnr })` instead of global autocommands for tabpage safety

Applied to files:

  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/**/*.lua : Never use `vim.notify` directly; always use `Logger.notify()` to prevent 'fast context is active' errors in agentic.nvim

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/**/*.lua : Always use buffer-local keymaps with `BufHelpers.keymap_set(bufnr, 'n', 'key', fn)` instead of global keymaps to ensure tabpage isolation

Applied to files:

  • lua/agentic/ui/diff_preview.lua
  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to **/*.lua : For `return`, `type`, and `alias` annotations, use explicit `type|nil` union instead of `type?` syntax to make optional nature explicit

Applied to files:

  • lua/agentic/ui/diff_preview.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Every feature in agentic.nvim must be multi-tabpage safe with support for one instance per tabpage via SessionRegistry, shared ACP provider instance, and isolated ChatWidget per tabpage

Applied to files:

  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2025-12-30T12:30:22.662Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: tests/AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:22.662Z
Learning: Applies to tests/**/*_spec.lua : Tests should verify tabpage isolation to ensure no cross-contamination of session state across different tabpages

Applied to files:

  • lua/agentic/ui/chat_widget.lua

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @lua/agentic/ui/chat_widget.lua:
- Around line 175-180: Rename the misleading variable last_winid from the call
to find_first_non_widget_window() to something like non_widget_winid or
fallback_winid, and after calling open_left_window() verify its return value is
not nil before proceeding; if open_left_window() returns nil handle the failure
(e.g., log/error/early return) to avoid proceeding with closing widget windows
when no safe non-widget window exists. Ensure you update all references to the
renamed variable and add a nil-check for the result of open_left_window() (the
function that can return nil).
- Around line 616-621: Add a LuaCATS type annotation above the
EXCLUDED_FILETYPES constant to document its shape and improve LSP support; place
a line like "--- @type table<string, boolean>" (or "--- @type {[string]:
boolean}") immediately before the EXCLUDED_FILETYPES declaration so the symbol
EXCLUDED_FILETYPES is annotated as a map from filetype string to boolean.
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 49573e7 and 7c3a60f.

📒 Files selected for processing (2)
  • lua/agentic/config_default.lua
  • lua/agentic/ui/chat_widget.lua
🧰 Additional context used
📓 Path-based instructions (3)
**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

**/*.lua: Always run both Lua linters after making ANY Lua file changes: make luals for type checking and make luacheck for style/syntax checking
Use LuaCATS annotations with proper spacing: include a space after --- for both descriptions and annotations
For @param and @field annotations, use variable? type format where the ? goes AFTER the variable name to indicate optional parameters
For @return, @type, and @alias annotations, use explicit type|nil union instead of type? syntax to make optional nature explicit
In fun() type declarations, use explicit type|nil union syntax instead of optional variable? syntax because LuaLS ignores optional syntax in function declarations
Set __index = self for Lua classes to enable inheritance, use setmetatable to create instances, and return the instance from constructor
For class inheritance, call parent constructor explicitly with parent class as first argument: Parent.new(Parent, ...) and re-metatable instance to child class
Use visibility prefixes in Lua class properties: _* for private, __* for protected, no prefix for public

Files:

  • lua/agentic/config_default.lua
  • lua/agentic/ui/chat_widget.lua
lua/agentic/**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

lua/agentic/**/*.lua: Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances only
Namespace IDs can be defined at module level (shared across tabpages) because extmarks are buffer-scoped and each tabpage has its own buffers
Always use buffer-local keymaps with BufHelpers.keymap_set(bufnr, 'n', 'key', fn) instead of global keymaps to ensure tabpage isolation
Prefer buffer-local autocommands using vim.api.nvim_create_autocmd(..., { buffer = bufnr }) instead of global autocommands for tabpage safety
Never use vim.notify directly; always use Logger.notify() to prevent 'fast context is active' errors in agentic.nvim

Files:

  • lua/agentic/config_default.lua
  • lua/agentic/ui/chat_widget.lua
lua/agentic/config_default.lua

📄 CodeRabbit inference engine (AGENTS.md)

When adding configuration options to config_default.lua, update the README.md 'Configuration' section with default values and documentation

Files:

  • lua/agentic/config_default.lua
🧠 Learnings (7)
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/config_default.lua : When adding configuration options to config_default.lua, update the README.md 'Configuration' section with default values and documentation

Applied to files:

  • lua/agentic/config_default.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/theme.lua : When adding new highlight groups in theme.lua, add the group to `Theme.HL_GROUPS` constant and update the README.md 'Customization (Ricing)' section

Applied to files:

  • lua/agentic/config_default.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/acp/adapters/*.lua : When adding a new ACP provider, update the provider documentation in AGENTS.md with adapter file path and configuration details

Applied to files:

  • lua/agentic/config_default.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/**/*.lua : Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances only

Applied to files:

  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Every feature in agentic.nvim must be multi-tabpage safe with support for one instance per tabpage via SessionRegistry, shared ACP provider instance, and isolated ChatWidget per tabpage

Applied to files:

  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/**/*.lua : Prefer buffer-local autocommands using `vim.api.nvim_create_autocmd(..., { buffer = bufnr })` instead of global autocommands for tabpage safety

Applied to files:

  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2025-12-30T12:30:22.662Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: tests/AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:22.662Z
Learning: Applies to tests/**/*_spec.lua : Tests should verify tabpage isolation to ensure no cross-contamination of session state across different tabpages

Applied to files:

  • lua/agentic/ui/chat_widget.lua
🧬 Code graph analysis (1)
lua/agentic/ui/chat_widget.lua (1)
lua/agentic/utils/logger.lua (1)
  • Logger.notify (50-64)
🔇 Additional comments (3)
lua/agentic/ui/chat_widget.lua (3)

623-647: Well-implemented tabpage-safe window search.

The method correctly uses self.tab_page_id for tabpage-safe operation and implements efficient exclusion filtering. The logic properly handles widget window identification and filetype exclusions.


649-659: Clean helper implementation.

The private helper correctly follows naming conventions (underscore prefix) and provides a clear utility for checking widget buffer membership.


661-714: No changes needed; oldfiles fallback is correctly implemented.

The implementation correctly handles all stated edge cases: vim.fn.filereadable() on line 682 validates file existence and readability (preventing stale entries), the loop breaks immediately upon finding a valid file (optimizing performance), and the multi-stage fallback is appropriate. Uses Logger.notify() correctly per guidelines and creates an unlisted scratch buffer as intended.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In @lua/agentic/ui/chat_widget.lua:
- Around line 630-654: The docstring for ChatWidget:find_first_non_widget_window
is misleading: it says "on the current tabpage" but the function actually uses
self.tab_page_id (the widget's tabpage). Update the documentation line and any
related comment to read "on this widget's tabpage" (or similar) to reflect the
actual behavior, and ensure the @return description remains accurate for when no
non-widget window is found; leave the implementation (use of self.tab_page_id,
widget_win_ids, EXCLUDED_FILETYPES) unchanged.
- Around line 623-628: Add a LuaCATS type annotation for EXCLUDED_FILETYPES by
placing a single-line annotation immediately above its declaration, e.g. use the
LuaCATS form with proper spacing: --- @type table<string, boolean>, then keep
the existing table literal as the value for EXCLUDED_FILETYPES so the constant
is typed as a map from filetype string to boolean.
- Around line 680-699: The prefix check using vim.startswith(filepath, cwd) can
falsely match sibling directories (e.g., "/home/user/proj" vs
"/home/user/project"); update the check to compare normalized absolute paths and
require a directory boundary: compute absolute paths for filepath and cwd (use
vim.fn.fnamemodify with ':p' or similar), ensure cwd ends with the path
separator, then test that filepath starts with cwd_with_sep (or equals cwd)
before accepting the file; keep the rest of the logic using bufnr, oldfiles,
vim.fn.filereadable, vim.fn.bufnr and vim.fn.bufadd unchanged.
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7c3a60f and 8790d5b.

📒 Files selected for processing (1)
  • lua/agentic/ui/chat_widget.lua
🧰 Additional context used
📓 Path-based instructions (2)
**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

**/*.lua: Always run both Lua linters after making ANY Lua file changes: make luals for type checking and make luacheck for style/syntax checking
Use LuaCATS annotations with proper spacing: include a space after --- for both descriptions and annotations
For @param and @field annotations, use variable? type format where the ? goes AFTER the variable name to indicate optional parameters
For @return, @type, and @alias annotations, use explicit type|nil union instead of type? syntax to make optional nature explicit
In fun() type declarations, use explicit type|nil union syntax instead of optional variable? syntax because LuaLS ignores optional syntax in function declarations
Set __index = self for Lua classes to enable inheritance, use setmetatable to create instances, and return the instance from constructor
For class inheritance, call parent constructor explicitly with parent class as first argument: Parent.new(Parent, ...) and re-metatable instance to child class
Use visibility prefixes in Lua class properties: _* for private, __* for protected, no prefix for public

Files:

  • lua/agentic/ui/chat_widget.lua
lua/agentic/**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

lua/agentic/**/*.lua: Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances only
Namespace IDs can be defined at module level (shared across tabpages) because extmarks are buffer-scoped and each tabpage has its own buffers
Always use buffer-local keymaps with BufHelpers.keymap_set(bufnr, 'n', 'key', fn) instead of global keymaps to ensure tabpage isolation
Prefer buffer-local autocommands using vim.api.nvim_create_autocmd(..., { buffer = bufnr }) instead of global autocommands for tabpage safety
Never use vim.notify directly; always use Logger.notify() to prevent 'fast context is active' errors in agentic.nvim

Files:

  • lua/agentic/ui/chat_widget.lua
🧠 Learnings (9)
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/**/*.lua : Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances only

Applied to files:

  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Every feature in agentic.nvim must be multi-tabpage safe with support for one instance per tabpage via SessionRegistry, shared ACP provider instance, and isolated ChatWidget per tabpage

Applied to files:

  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/**/*.lua : Prefer buffer-local autocommands using `vim.api.nvim_create_autocmd(..., { buffer = bufnr })` instead of global autocommands for tabpage safety

Applied to files:

  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2025-12-30T12:30:22.662Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: tests/AGENTS.md:0-0
Timestamp: 2025-12-30T12:30:22.662Z
Learning: Applies to tests/**/*_spec.lua : Tests should verify tabpage isolation to ensure no cross-contamination of session state across different tabpages

Applied to files:

  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/**/*.lua : Always use buffer-local keymaps with `BufHelpers.keymap_set(bufnr, 'n', 'key', fn)` instead of global keymaps to ensure tabpage isolation

Applied to files:

  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to **/*.lua : Use LuaCATS annotations with proper spacing: include a space after `---` for both descriptions and annotations

Applied to files:

  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to **/*.lua : For `return`, `type`, and `alias` annotations, use explicit `type|nil` union instead of `type?` syntax to make optional nature explicit

Applied to files:

  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to **/*.lua : In `fun()` type declarations, use explicit `type|nil` union syntax instead of optional `variable?` syntax because LuaLS ignores optional syntax in function declarations

Applied to files:

  • lua/agentic/ui/chat_widget.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to **/*.lua : For `param` and `field` annotations, use `variable? type` format where the `?` goes AFTER the variable name to indicate optional parameters

Applied to files:

  • lua/agentic/ui/chat_widget.lua
🧬 Code graph analysis (1)
lua/agentic/ui/chat_widget.lua (1)
lua/agentic/utils/logger.lua (1)
  • Logger.notify (50-64)
🔇 Additional comments (2)
lua/agentic/ui/chat_widget.lua (2)

175-187: LGTM! Robust fallback window handling.

The updated hide logic correctly implements the fallback window strategy with proper error handling. The variable naming is clear, and the early return prevents unsafe window closure when fallback creation fails.


656-666: LGTM! Clean helper method.

The implementation correctly identifies widget buffers and follows the private method naming convention with the underscore prefix.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @lua/agentic/config_default.lua:
- Around line 210-214: Add a new entry to the README.md Configuration section
documenting the diff_preview option: describe diff_preview.enabled (default:
true) and its purpose ("Show diff preview for edit tool calls in the buffer"),
include the default value and a short one-line usage/behavior note so README
matches the config_default.lua diff_preview setting.
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8790d5b and 509f18d.

📒 Files selected for processing (3)
  • lua/agentic/config_default.lua
  • lua/agentic/session_manager.lua
  • lua/agentic/utils/file_system.lua
🧰 Additional context used
📓 Path-based instructions (3)
**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

**/*.lua: Always run both Lua linters after making ANY Lua file changes: make luals for type checking and make luacheck for style/syntax checking
Use LuaCATS annotations with proper spacing: include a space after --- for both descriptions and annotations
For @param and @field annotations, use variable? type format where the ? goes AFTER the variable name to indicate optional parameters
For @return, @type, and @alias annotations, use explicit type|nil union instead of type? syntax to make optional nature explicit
In fun() type declarations, use explicit type|nil union syntax instead of optional variable? syntax because LuaLS ignores optional syntax in function declarations
Set __index = self for Lua classes to enable inheritance, use setmetatable to create instances, and return the instance from constructor
For class inheritance, call parent constructor explicitly with parent class as first argument: Parent.new(Parent, ...) and re-metatable instance to child class
Use visibility prefixes in Lua class properties: _* for private, __* for protected, no prefix for public

Files:

  • lua/agentic/config_default.lua
  • lua/agentic/utils/file_system.lua
  • lua/agentic/session_manager.lua
lua/agentic/**/*.lua

📄 CodeRabbit inference engine (AGENTS.md)

lua/agentic/**/*.lua: Never use module-level shared state for per-tabpage runtime data; store per-tabpage state in tabpage-scoped instances only
Namespace IDs can be defined at module level (shared across tabpages) because extmarks are buffer-scoped and each tabpage has its own buffers
Always use buffer-local keymaps with BufHelpers.keymap_set(bufnr, 'n', 'key', fn) instead of global keymaps to ensure tabpage isolation
Prefer buffer-local autocommands using vim.api.nvim_create_autocmd(..., { buffer = bufnr }) instead of global autocommands for tabpage safety
Never use vim.notify directly; always use Logger.notify() to prevent 'fast context is active' errors in agentic.nvim

Files:

  • lua/agentic/config_default.lua
  • lua/agentic/utils/file_system.lua
  • lua/agentic/session_manager.lua
lua/agentic/config_default.lua

📄 CodeRabbit inference engine (AGENTS.md)

When adding configuration options to config_default.lua, update the README.md 'Configuration' section with default values and documentation

Files:

  • lua/agentic/config_default.lua
🧠 Learnings (6)
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/config_default.lua : When adding configuration options to config_default.lua, update the README.md 'Configuration' section with default values and documentation

Applied to files:

  • lua/agentic/config_default.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/theme.lua : When adding new highlight groups in theme.lua, add the group to `Theme.HL_GROUPS` constant and update the README.md 'Customization (Ricing)' section

Applied to files:

  • lua/agentic/config_default.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/acp/adapters/*.lua : When adding a new ACP provider, update the provider documentation in AGENTS.md with adapter file path and configuration details

Applied to files:

  • lua/agentic/config_default.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/**/*.lua : Prefer buffer-local autocommands using `vim.api.nvim_create_autocmd(..., { buffer = bufnr })` instead of global autocommands for tabpage safety

Applied to files:

  • lua/agentic/utils/file_system.lua
  • lua/agentic/session_manager.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/**/*.lua : Never use `vim.notify` directly; always use `Logger.notify()` to prevent 'fast context is active' errors in agentic.nvim

Applied to files:

  • lua/agentic/session_manager.lua
📚 Learning: 2026-01-05T19:30:46.039Z
Learnt from: CR
Repo: carlos-algms/agentic.nvim PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T19:30:46.039Z
Learning: Applies to lua/agentic/**/*.lua : Always use buffer-local keymaps with `BufHelpers.keymap_set(bufnr, 'n', 'key', fn)` instead of global keymaps to ensure tabpage isolation

Applied to files:

  • lua/agentic/session_manager.lua
🔇 Additional comments (5)
lua/agentic/utils/file_system.lua (1)

104-105: LGTM! Buffer reload approach is appropriate for the diff preview flow.

The silent! edit! command correctly forces a buffer reload without triggering the W13 prompt, which is essential for newly created files where the buffer may have been opened before the file existed on disk. Since the content was just written via save_to_disk, the forced reload safely synchronizes the buffer with disk state while preserving the view.

lua/agentic/session_manager.lua (4)

408-412: LGTM! Defensive cleanup ensures diff preview is cleared on tool completion.

The pre-emptive diff clearing on tool call updates is the right approach. While this might create redundant clearing (since the permission callback also clears), the defensive cleanup ensures previews are removed regardless of the execution path.


434-448: LGTM! Proper diff lifecycle management around permission flow.

The implementation correctly:

  1. Shows the diff preview when permission is requested (line 447)
  2. Wraps the callback to ensure cleanup after user response (line 434-445)
  3. Clears the diff after the original callback completes (line 437)

This ensures users see the diff while reviewing the permission request and cleanup happens reliably after their decision.


555-588: LGTM! Robust implementation with excellent error handling.

The method correctly:

  • Guards against disabled config and invalid tool calls (lines 557-566)
  • Uses proper visibility prefix _ for private method
  • Implements defensive pcall around nvim_win_set_buf (lines 576-584)
  • Uses Logger.notify() instead of vim.notify() per coding guidelines to prevent 'fast context is active' errors
  • Employs a clean callback pattern for window management

590-600: LGTM! Clear method correctly mirrors show method's defensive checks.

The implementation appropriately:

  • Validates tracker state before clearing (lines 592-597)
  • Uses consistent tracker.argument for the file path
  • Omits the config check (intentional - clears regardless of current config state)
  • Uses proper visibility prefix _ for private method

Comment on lines +210 to +214
--- Show diff preview for edit tool calls in the buffer
--- @class agentic.UserConfig.DiffPreview
diff_preview = {
enabled = true,
},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

git ls-files README.md

Repository: carlos-algms/agentic.nvim

Length of output: 78


🏁 Script executed:

# Find README and check if it exists
fd -t f README.md

Repository: carlos-algms/agentic.nvim

Length of output: 94


🏁 Script executed:

# Search for Configuration section in README and look for diff_preview
rg -A 50 "Configuration" README.md | head -100

Repository: carlos-algms/agentic.nvim

Length of output: 4036


🏁 Script executed:

# Also check for diff_preview specifically anywhere in README
rg "diff_preview" README.md

Repository: carlos-algms/agentic.nvim

Length of output: 51


🏁 Script executed:

sed -n '205,220p' lua/agentic/config_default.lua

Repository: carlos-algms/agentic.nvim

Length of output: 500


🏁 Script executed:

# Check the Configuration section structure to understand how options are documented
rg -B 5 -A 10 "acp_providers" README.md | head -40

Repository: carlos-algms/agentic.nvim

Length of output: 1142


Update README.md with the new diff_preview configuration option.

The diff_preview.enabled option was added to config_default.lua but is not documented in the README.md Configuration section. Add documentation including the default value (true) and its purpose ("Show diff preview for edit tool calls in the buffer") to keep configuration documentation in sync.

🤖 Prompt for AI Agents
In @lua/agentic/config_default.lua around lines 210 - 214, Add a new entry to
the README.md Configuration section documenting the diff_preview option:
describe diff_preview.enabled (default: true) and its purpose ("Show diff
preview for edit tool calls in the buffer"), include the default value and a
short one-line usage/behavior note so README matches the config_default.lua
diff_preview setting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants