Skip to content

Commit c10ef1e

Browse files
committed
Many changes
1 parent 87c024a commit c10ef1e

22 files changed

+3640
-506
lines changed

AGENTS.md

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,23 @@
1313
## Learned while implementing old-code preview behavior
1414

1515
- `toggle_prev_code()` is a global review-mode toggle: it flips `show_old_code` and applies to all hunks in applied review buffers, not just the currently selected change block.
16+
- `ui/virtual.expand()` is used directly in tests and other call paths, so it must call `define_highlights()` itself; relying on `apply_mode_to_buffer()` or `toggle_review_mode()` leaves `NRVirtualDelete` undefined.
17+
- Plugin-owned highlight groups that need to refresh in the current session should not use `default = true`; stale existing groups otherwise survive and make UI changes appear to have no effect.
1618

1719
## Learned while implementing Ask walkthrough navigation
1820

1921
- `next_change`/`prev_change` route through `ui/walkthrough.lua` when Ask is open, so anchor-level behavior must live in `next_step`/`prev_step`; changing `ui/nav.lua` alone will not affect Ask navigation.
22+
- Ask walkthrough highlight changes need updates in both `lua/tests/plenary/ui/walkthrough_spec.lua` and `lua/tests/plenary/init_walkthrough_controls_spec.lua`; the former should also fix `vim.o.columns`/`vim.o.lines` so long file names do not wrap and create false-negative detail-pane assertions.
2023

2124
## Learned while implementing local diff noise filtering
2225

2326
- Filter `ReviewDiff` files before `state.set_local_review()` and before optional AI analysis so navigation, change counts, and AI walkthrough inputs stay consistent.
2427
- Basename-based skipping is sufficient for lockfiles: matching both exact path and `"/" .. basename` suffix excludes nested lockfiles without adding glob parsing complexity.
2528

29+
## Learned while extending noise filtering to PR reviews
30+
31+
- Reapply `review_diff` noise filtering in both `ReviewPR` fetch and PR `ReviewSync` before `state.set_review()`; otherwise the initial review, sync refresh, and AI analysis drift onto different file sets.
32+
2633
## Learned while implementing ReviewDiff target modes
2734

2835
- Local diff review now combines tracked diffs and untracked-file patches in the Rust CLI (`git diff ...` plus `git ls-files --others --exclude-standard`); keep `--tracked-only` and `--cached-only` semantics in CLI, then apply Lua noise filtering after fetch as usual.
@@ -47,6 +54,35 @@
4754
- Auto-sync timers are owned by `init.lua`; `state.clear_review()` should call `neo_reviewer._stop_autosync()` when available to avoid leaking periodic timers across teardown paths and tests.
4855
- To keep the AI walkthrough panel stable during sync, plumb `keep_ai_ui` through `state.clear_review()` and re-render with `ai_ui.open()` after rebuilding review state.
4956

57+
## Learned while implementing the Neo-tree review source and AI pane split
58+
59+
- `nix flake check` only sees git-tracked paths; if a Lua module exists only as a new untracked file, Nix builds/tests fail with `module not found`. For temporary or optional modules, registering them through `package.preload` inside a tracked file keeps checks honest without needing git writes.
60+
- Neo-tree custom sources must expose `components` on the root module or provide a tracked `mod_root .. ".components"` module; preloading only the source and commands still fails during `setup()`.
61+
- If a custom source should look like Neo-tree filesystem output, render `{ root }` instead of `root.children` and populate source state such as `git_status_lookup`; borrowing filesystem components alone is not enough.
62+
- `topleft {width}vsplit` from the AI walkthrough pane creates a full-height column across the whole tab, not a split inside the bottom walkthrough area. Use `leftabove {width}vsplit` from the detail window to keep the navigator inside the walkthrough pane; otherwise the main editor window collapses and stacked walkthrough windows fail with `E36`.
63+
- Transient loading panes need the same split-base exclusions as AI/Ask walkthrough panes; if `find_split_base_window()` can target `neo-reviewer-loading`, the final navigator/detail splits open relative to the loading scratch window instead of the editor layout.
64+
65+
## Learned while fixing preload registration startup ordering
66+
67+
- `plugin/neo_reviewer.vim` and user config can both hit `require("neo_reviewer")` during startup, so `init.lua` cannot trust `package.loaded["neo_reviewer.plugin"]` to be complete. If preload handlers are required before the module cache stabilizes, load the tracked `lua/neo_reviewer/plugin.lua` runtime file directly and run its top-level registration instead of waiting on `require()` state.
68+
- When both the repo checkout and an installed plugin copy are on `runtimepath`, `vim.api.nvim_get_runtime_file("lua/neo_reviewer/plugin.lua", false)` can resolve the wrong `plugin.lua`. For preload recovery, derive the sibling tracked `plugin.lua` from the active source file path instead of asking runtimepath to choose.
69+
70+
## Learned while improving the AI step navigator
71+
72+
- `step_list_width` works better as a preferred width than a fixed one: cap the navigator against the available split width and wrap step titles across multiple navigator lines, otherwise larger defaults just crush the detail pane and single-line truncation makes reviewer-oriented step titles unreadable.
73+
- For navigator overview text, keep the full content in the scratch buffer by pre-wrapping it into shorter lines instead of pre-truncating with ellipses. The window can still stay `nowrap`; buffer-level truncation is what makes the overview effectively unreadable in narrow layouts.
74+
- The navigator overview wraps against `window width - 2`, so a default `step_list_width = 52` is what produces roughly 50 characters of overview text when the layout can spare the space. If the wrap point changes, update both the default and `get_target_nav_width()`.
75+
76+
## Learned while simplifying AI PR analysis
77+
78+
- The second AI coverage pass adds a full extra model roundtrip for marginal cleanup value. With a prompt that already asks for exact change-block coverage, a better tradeoff is one AI response plus local placeholder steps for any uncovered blocks, and prompt guidance should explicitly ban filler phrases like `"This matters because"` so the detail pane reads tightly.
79+
80+
## Learned while smoothing ReviewSync UI refresh
81+
82+
- `neo_reviewer.neotree.is_open()` cannot rely on `manager.get_state("review").winid` alone; Neo-tree source state can point at a valid sidebar window whose visible buffer belongs to another source. Confirm `vim.b[bufnr].neo_tree_source == "review"` before deciding the review tree is already open.
83+
- `neo_reviewer.neotree.on_review_changed({ open = true })` should still refresh, not re-open, when the review source buffer is already visible; otherwise autosync re-runs the Neo-tree open command and disturbs the layout.
84+
- For AI review panes, re-rendering existing split buffers during sync should preserve current window sizes; reusing `open()` without a layout-preserving option causes background refreshes to resize walkthrough splits.
85+
5086
## Project Overview
5187

5288
`neo-reviewer` is a Neovim plugin for reviewing GitHub PRs. Hybrid architecture: Rust CLI for GitHub API/diff parsing, Lua plugin for UI.
@@ -97,8 +133,11 @@ terraform fmt -recursive repo/
97133
cargo build -p neo-reviewer --release
98134

99135
# Build via Nix
100-
nix build .#neo-reviewer
101-
nix build .#neo-reviewer-nvim
136+
nix build .#neo-reviewer --out-link result-cli
137+
nix build .#neo-reviewer-nvim --out-link result-nvim
138+
139+
# Run a single Nix check without rewriting ./result
140+
./scripts/build-check lua-tests
102141
```
103142

104143
## Key Patterns
@@ -164,6 +203,8 @@ Keep the docs in sync with `lua/neo_reviewer/init.lua` (commands/functions) and
164203
- `lua-tests` - Plenary test suite
165204
- `rust-tests` - Rust test suite
166205

206+
For a single check, prefer `./scripts/build-check <name>` or `nix build .#checks.<system>.<name> --no-link` so ad hoc check runs do not rewrite `./result`.
207+
167208
**Always run `nix flake check` yourself before considering any change complete.** Do not recommend it to the user—execute it and report results. All checks MUST pass. This includes:
168209
- Zero Lua diagnostics (warnings are errors)
169210
- Zero Clippy warnings
@@ -178,7 +219,7 @@ If diagnostics fail, fix them before moving on. Common Lua diagnostic fixes:
178219

179220
GitHub Actions use Nix for reproducible builds. When adding new CI jobs, prefer Nix over manual tool installation:
180221
- Use `DeterminateSystems/nix-installer-action` and `magic-nix-cache-action`
181-
- Add checks to `flake.nix` and run via `nix build .#checks...` or `nix flake check`
222+
- Add checks to `flake.nix` and run via `./scripts/build-check <name>` or `nix flake check`
182223

183224
## Lua Coding Standards
184225

doc/neo_reviewer.txt

Lines changed: 60 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ your editor. It provides:
2828
- Multi-line comments with visual range selection
2929
- Submit reviews (approve or request changes)
3030
- Telescope integration for file picker
31+
- Optional Neo-tree changed-files review source
3132
- Local git diff review (review uncommitted changes before pushing)
3233
- AI-powered review analysis (optional, via Codex CLI by default)
3334

@@ -37,6 +38,7 @@ your editor. It provides:
3738
- Neovim >= 0.9.0
3839
- plenary.nvim (required)
3940
- telescope.nvim (optional, for file picker)
41+
- neo-tree.nvim (optional, for changed-files review tree)
4042
- neo-reviewer CLI (Rust CLI tool)
4143
- GitHub authentication via `gh` CLI
4244

@@ -105,14 +107,19 @@ Call setup() with your options: >lua
105107
periodic_interval_ms = 120000, -- 2 minutes
106108
cooldown_ms = 1500, -- Min spacing between sync starts
107109
},
110+
neo_tree = {
111+
open_on_review = false, -- Auto-open Neo-tree review source
112+
position = "left", -- Neo-tree window position
113+
},
108114
ai = {
109115
enabled = false, -- Run AI analysis by default
110116
model = "gpt-5.3-codex", -- Model for Codex CLI (default)
111117
command = "codex", -- Command to invoke
112-
reasoning_effort = "medium", -- Reasoning effort hint (Codex CLI)
118+
reasoning_effort = "low", -- Reasoning effort hint (Codex CLI)
113119
walkthrough_window = {
114120
height = 0, -- Auto size to content (min height)
115121
focus_on_open = false, -- Keep focus on diff window
122+
step_list_width = 52, -- Preferred width of the step list pane
116123
},
117124
},
118125
})
@@ -138,11 +145,12 @@ input_window.keys ~
138145

139146
*neo_reviewer-config-review_diff*
140147
review_diff ~
141-
Configure local diff review file filtering.
148+
Configure review file filtering.
142149
- `skip_noise_files`: Skip common lock/noise files in |:ReviewDiff|
150+
and |:ReviewPR|
143151
(default: true).
144152
- `noise_files`: Basenames to skip when `skip_noise_files` is enabled.
145-
Any file with a matching basename is excluded from local diff review.
153+
Any file with a matching basename is excluded from PR/local diff review.
146154

147155
*neo_reviewer-config-sync*
148156
sync ~
@@ -160,6 +168,28 @@ sync ~
160168
Automatic save/periodic sync runs silently on success (no "Synced"
161169
notifications). Errors still notify.
162170

171+
*neo_reviewer-config-neo_tree*
172+
neo_tree ~
173+
Configure optional Neo-tree integration for changed review files.
174+
- `open_on_review`: Open the Neo-tree review source when a review starts
175+
(default: false).
176+
- `position`: Neo-tree window position for the review source
177+
(default: "left").
178+
179+
To enable the source in Neo-tree, add
180+
`"neo_reviewer.sources.review"` to your Neo-tree `sources` list. The
181+
source name inside Neo-tree is `review`, so you can open it manually with
182+
`:Neotree review` or include it in the source selector. Example: >
183+
require("neo-tree").setup({
184+
sources = {
185+
"filesystem",
186+
"buffers",
187+
"git_status",
188+
"neo_reviewer.sources.review",
189+
},
190+
})
191+
<
192+
163193
*neo_reviewer-config-ai*
164194
ai ~
165195
Configure AI-powered review analysis. Requires an AI CLI (Codex CLI by
@@ -168,16 +198,20 @@ ai ~
168198
- `model`: Model to use for analysis (default: "gpt-5.3-codex").
169199
- `command`: Command to invoke the AI CLI (default: "codex").
170200
- `reasoning_effort`: Reasoning effort hint for Codex CLI
171-
(default: "medium", ignored by other CLIs).
201+
(default: "low", ignored by other CLIs).
172202
- `walkthrough_window.height`: Minimum height of the walkthrough split
173203
(default: 0, auto-sized to content).
174204
- `walkthrough_window.focus_on_open`: Focus the walkthrough window when it opens (default: false).
205+
- `walkthrough_window.step_list_width`: Preferred width of the walkthrough
206+
step list pane (default: 52).
175207

176208
When AI analysis is enabled, neo-reviewer generates a plain-language
177-
overview and an ordered walkthrough of the PR with step-by-step
178-
explanations. The walkthrough opens automatically and updates as you
179-
navigate change blocks. Use |neo_reviewer.toggle_ai_feedback()| to open or
180-
re-open the walkthrough window.
209+
overview and an ordered walkthrough of the PR. While the AI is working, a
210+
scratch window shows loading progress. The walkthrough then opens in a
211+
two-pane layout: a compact step list on the left and the current step
212+
explanation on the right. It updates as you navigate change blocks. Use
213+
|neo_reviewer.toggle_ai_feedback()| to open or re-open the walkthrough
214+
window.
181215
For context, the AI is allowed to read git-tracked files from disk using
182216
repo-relative paths. Make sure your working tree matches the PR before
183217
running analysis. The diff sent to the AI uses placeholders for added and
@@ -187,7 +221,8 @@ ai ~
187221
blocks and appends any remaining uncovered change blocks as minimal
188222
"Uncovered change" steps. It also normalizes AI output so each change block
189223
is referenced by at most one walkthrough step while preserving grouped
190-
one-to-many steps.
224+
one-to-many steps. The prompt also pushes the AI to use reviewer-friendly
225+
step titles and thematic grouping instead of a raw file-by-file dump.
191226

192227
Codebase explorations reuse this AI configuration and the walkthrough
193228
window settings. See |:Ask| for a guided tour of the codebase.
@@ -208,6 +243,9 @@ These user commands are created when you call `setup()`.
208243

209244
Diff hunks are generated from local git using the PR base/head commits
210245
(`git diff -w base...head`), so whitespace-only changes are excluded.
246+
Common lock/noise files (for example `pnpm-lock.yaml`, `Cargo.lock`) are
247+
skipped by default. Configure this via `review_diff.skip_noise_files` and
248+
`review_diff.noise_files`.
211249
If required commits are missing locally, neo-reviewer first attempts to
212250
fetch the needed refs automatically before failing.
213251

@@ -255,9 +293,10 @@ These user commands are created when you call `setup()`.
255293
expects the AI tool to read files directly from disk.
256294
The AI decides whether to return a code-focused walkthrough or a more
257295
conceptual explanation.
258-
While the walkthrough is being generated, a temporary window shows a
259-
loading message. When ready, the first step jumps to its first anchor
260-
(if any).
296+
While the walkthrough is being generated, a temporary scratch window shows
297+
loading progress. When ready, the walkthrough opens in the same two-pane
298+
overview/details layout used for review AI and the first step jumps to its
299+
first anchor (if any).
261300
When the exploration window is open, |neo_reviewer.next_change()| and
262301
|neo_reviewer.prev_change()| move through anchors in the current step
263302
first, then move between exploration steps.
@@ -286,15 +325,17 @@ These user commands are created when you call `setup()`.
286325
Sync the current active review.
287326
- During PR review: fetches latest comments/metadata from GitHub and
288327
rebuilds diff changes from local git (same base/head commit range and
289-
whitespace filtering as |:ReviewPR|).
328+
whitespace filtering as |:ReviewPR|). Noise file filtering from
329+
`review_diff.*` is reapplied.
290330
- During local diff review: re-runs local `git diff` sourcing with the
291331
same target/selector options used by |:ReviewDiff| and reapplies markers.
292332
Preserves cursor position, expanded change block state, and AI summary when
293333
possible.
294334
By default, sync also runs automatically on save for reviewed files and as
295335
a periodic background refresh every 2 minutes. Save-triggered PR sync skips
296-
comment refresh (diff/highlights only). Automatic sync keeps the AI
297-
walkthrough window open if it is already open. Configure via `sync.*`.
336+
comment refresh (diff/highlights only). Automatic sync refreshes already-
337+
open Neo-tree/AI walkthrough windows in place instead of reopening them.
338+
Configure via `sync.*`.
298339

299340
==============================================================================
300341
6. FUNCTIONS *neo_reviewer-functions*
@@ -308,6 +349,8 @@ require("neo_reviewer").review_pr({arg}) *neo_reviewer.review_
308349

309350
PR diff hunks are computed from local git using `base...head` with
310351
whitespace ignored (`-w`).
352+
Common lock/noise files are skipped using `review_diff.skip_noise_files`
353+
and `review_diff.noise_files`.
311354

312355
require("neo_reviewer").review_diff({opts}) *neo_reviewer.review_diff()*
313356
Review local git diff before committing. By default this includes
@@ -440,7 +483,8 @@ require("neo_reviewer").sync() *neo_reviewer.s
440483
Preserves cursor position and expanded change block state when possible.
441484
Manual sync (`:ReviewSync`) always refreshes comments for PR reviews.
442485
Automatic save-triggered sync refreshes diff/highlights but skips comment
443-
refresh for PR reviews.
486+
refresh for PR reviews, while keeping open Neo-tree/AI review panes in
487+
place instead of reopening them.
444488
See |:ReviewSync|.
445489

446490
require("neo_reviewer").toggle_ai_feedback() *neo_reviewer.toggle_ai_feedback()*

0 commit comments

Comments
 (0)