Skip to content

Commit b7b0453

Browse files
committed
ReviewSync for diffs
1 parent b0e0981 commit b7b0453

File tree

6 files changed

+187
-24
lines changed

6 files changed

+187
-24
lines changed

AGENTS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@
3131

3232
- `fetch` builds PR change blocks from local git (`git diff -w <base_sha>...<head_sha>`) instead of GitHub file patches; when commits are missing locally it attempts a targeted `git fetch` of base/head refs before failing.
3333

34+
## Learned while implementing ReviewSync for local diffs
35+
36+
- Store `ReviewDiff` selector options (`target`, `--cached-only`, `--uncached-only`, `--merge-base`, `--tracked-only`) on the active local review so `ReviewSync` can re-run the same diff mode instead of silently falling back to default `diff`.
37+
3438
## Project Overview
3539

3640
`neo-reviewer` is a Neovim plugin for reviewing GitHub PRs. Hybrid architecture: Rust CLI for GitHub API/diff parsing, Lua plugin for UI.

doc/neo_reviewer.txt

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -260,11 +260,13 @@ These user commands are created when you call `setup()`.
260260
and pops any stashed changes.
261261

262262
:ReviewSync *:ReviewSync*
263-
Sync the current PR review with GitHub. Fetches the latest comments and
264-
rebuilds diff changes from local git (same base/head commit range and
265-
whitespace filtering as |:ReviewPR|). Preserves cursor position and
266-
expanded change block state.
267-
Only works during an active PR review (not local diff reviews).
263+
Sync the current active review.
264+
- During PR review: fetches latest comments/metadata from GitHub and
265+
rebuilds diff changes from local git (same base/head commit range and
266+
whitespace filtering as |:ReviewPR|).
267+
- During local diff review: re-runs local `git diff` sourcing with the
268+
same target/selector options used by |:ReviewDiff| and reapplies markers.
269+
Preserves cursor position and expanded change block state when possible.
268270

269271
==============================================================================
270272
6. FUNCTIONS *neo_reviewer-functions*
@@ -402,10 +404,12 @@ require("neo_reviewer").done() *neo_reviewer.d
402404
the previous branch and pops any stashed changes.
403405

404406
require("neo_reviewer").sync() *neo_reviewer.sync()*
405-
Sync the current PR review with GitHub. Re-fetches PR metadata/comments and
406-
rebuilds diff changes from local git (`base...head`, `-w`). Preserves
407-
cursor position, expanded change block state, and checkout state. Only
408-
works during an active PR review.
407+
Sync the current active review.
408+
- During PR review: re-fetches PR metadata/comments and rebuilds diff
409+
changes from local git (`base...head`, `-w`).
410+
- During local diff review: re-runs local diff sourcing with the same
411+
selection options used by |neo_reviewer.review_diff()|.
412+
Preserves cursor position and expanded change block state when possible.
409413
See |:ReviewSync|.
410414

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

lua/neo_reviewer/init.lua

Lines changed: 84 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,18 @@ local function parse_review_diff_args(args)
129129
return opts, nil
130130
end
131131

132+
---@param opts NRReviewDiffOpts
133+
---@return NRLocalDiffOpts
134+
local function to_local_diff_opts(opts)
135+
return {
136+
target = opts.target,
137+
cached_only = opts.cached_only,
138+
uncached_only = opts.uncached_only,
139+
merge_base = opts.merge_base,
140+
tracked_only = opts.tracked_only,
141+
}
142+
end
143+
132144
---@param review NRReview
133145
---@return boolean
134146
local function restore_previous_branch(review)
@@ -208,7 +220,7 @@ function M.setup(opts)
208220

209221
vim.api.nvim_create_user_command("ReviewSync", function()
210222
M.sync()
211-
end, { desc = "Sync PR review with GitHub" })
223+
end, { desc = "Sync active review" })
212224
end
213225

214226
---@param url_or_number? string|integer
@@ -414,14 +426,7 @@ function M.review_diff(opts)
414426
return
415427
end
416428

417-
---@type NRLocalDiffOpts
418-
local diff_opts = {
419-
target = opts.target,
420-
cached_only = opts.cached_only,
421-
uncached_only = opts.uncached_only,
422-
merge_base = opts.merge_base,
423-
tracked_only = opts.tracked_only,
424-
}
429+
local diff_opts = to_local_diff_opts(opts)
425430

426431
vim.notify("Getting local diff...", vim.log.levels.INFO)
427432

@@ -459,7 +464,7 @@ function M.review_diff(opts)
459464
}
460465

461466
state.clear_review()
462-
local review = state.set_local_review(filtered_data)
467+
local review = state.set_local_review(filtered_data, diff_opts)
463468

464469
local comments_file = require("neo_reviewer.ui.comments_file")
465470
comments_file.clear()
@@ -1115,6 +1120,75 @@ function M.sync()
11151120
return
11161121
end
11171122

1123+
if review.review_type == "local" then
1124+
local config = require("neo_reviewer.config")
1125+
local preserved = {
1126+
diff_opts = review.local_diff_opts or {},
1127+
expanded_changes = review.expanded_changes,
1128+
show_old_code = review.show_old_code,
1129+
}
1130+
local cursor_pos = vim.api.nvim_win_get_cursor(0)
1131+
1132+
vim.notify("Syncing local diff...", vim.log.levels.INFO)
1133+
1134+
cli.get_local_diff(preserved.diff_opts, function(data, err)
1135+
if err then
1136+
vim.notify("Failed to sync local diff: " .. err, vim.log.levels.ERROR)
1137+
return
1138+
end
1139+
1140+
local filtered_files, skipped_count = filter_local_diff_files(data.files, config.values.review_diff)
1141+
1142+
state.clear_review()
1143+
1144+
if #filtered_files == 0 then
1145+
if #data.files == 0 then
1146+
vim.notify("No changes to review", vim.log.levels.WARN)
1147+
else
1148+
vim.notify(
1149+
string.format("No reviewable changes after skipping %d noise files", skipped_count),
1150+
vim.log.levels.WARN
1151+
)
1152+
end
1153+
return
1154+
end
1155+
1156+
if skipped_count > 0 then
1157+
vim.notify(
1158+
string.format("[neo-reviewer] Skipped %d noise file(s) in local diff review", skipped_count),
1159+
vim.log.levels.INFO
1160+
)
1161+
end
1162+
1163+
---@type NRDiffData
1164+
local filtered_data = {
1165+
git_root = data.git_root,
1166+
files = filtered_files,
1167+
}
1168+
local new_review = state.set_local_review(filtered_data, preserved.diff_opts)
1169+
new_review.expanded_changes = preserved.expanded_changes
1170+
new_review.show_old_code = preserved.show_old_code
1171+
1172+
M.enable_overlay()
1173+
1174+
pcall(vim.api.nvim_win_set_cursor, 0, cursor_pos)
1175+
1176+
if skipped_count > 0 then
1177+
vim.notify(
1178+
string.format(
1179+
"Synced local diff (%d files changed, %d noise files skipped)",
1180+
#filtered_files,
1181+
skipped_count
1182+
),
1183+
vim.log.levels.INFO
1184+
)
1185+
else
1186+
vim.notify(string.format("Synced local diff (%d files changed)", #filtered_files), vim.log.levels.INFO)
1187+
end
1188+
end)
1189+
return
1190+
end
1191+
11181192
if review.review_type ~= "pr" or not review.url then
11191193
vim.notify("Sync only works for PR reviews", vim.log.levels.WARN)
11201194
return

lua/neo_reviewer/state.lua

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@
9292
---@field url? string PR URL (for PR reviews)
9393
---@field viewer? string Current authenticated user
9494
---@field git_root? string Git root directory (for local reviews)
95+
---@field local_diff_opts? NRLocalDiffOpts Local diff selector options used to fetch current review
9596
---@field files NRFile[] Changed files
9697
---@field files_by_path table<string, NRFile> Files indexed by path
9798
---@field comments NRComment[] Comments on the PR
@@ -163,8 +164,9 @@ function M.set_review(review_data, git_root)
163164
end
164165

165166
---@param diff_data NRDiffData
167+
---@param diff_opts? NRLocalDiffOpts
166168
---@return NRReview
167-
function M.set_local_review(diff_data)
169+
function M.set_local_review(diff_data, diff_opts)
168170
local files_by_path = {}
169171
for _, file in ipairs(diff_data.files) do
170172
if not file.change_blocks then
@@ -176,6 +178,7 @@ function M.set_local_review(diff_data)
176178
state.active_review = {
177179
review_type = "local",
178180
git_root = diff_data.git_root,
181+
local_diff_opts = diff_opts or {},
179182
files = diff_data.files,
180183
files_by_path = files_by_path,
181184
comments = {},

lua/tests/plenary/state_spec.lua

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,22 @@ describe("neo_reviewer.state", function()
532532
assert.are.equal(2, #review.files)
533533
end)
534534

535+
it("stores local diff selector options for sync", function()
536+
local data = helpers.deep_copy(fixtures.local_diff)
537+
state.set_local_review(data, {
538+
target = "main",
539+
cached_only = true,
540+
merge_base = true,
541+
})
542+
543+
local review = state.get_review()
544+
assert.are.same({
545+
target = "main",
546+
cached_only = true,
547+
merge_base = true,
548+
}, review.local_diff_opts)
549+
end)
550+
535551
it("normalizes missing change_blocks for local review", function()
536552
local data = {
537553
git_root = "/tmp/test-repo",

lua/tests/plenary/sync_spec.lua

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ describe("neo_reviewer.sync", function()
1818
cli = require("neo_reviewer.cli")
1919

2020
stub(cli, "fetch_pr")
21+
stub(cli, "get_local_diff")
2122

2223
neo_reviewer = require("neo_reviewer")
2324

@@ -26,6 +27,7 @@ describe("neo_reviewer.sync", function()
2627

2728
after_each(function()
2829
cli.fetch_pr:revert()
30+
cli.get_local_diff:revert()
2931

3032
notifications.restore()
3133
state.clear_review()
@@ -42,15 +44,75 @@ describe("neo_reviewer.sync", function()
4244
assert.are.equal(vim.log.levels.WARN, notifs[1].level)
4345
end)
4446

45-
it("warns when trying to sync local diff review", function()
47+
it("syncs local diff review by re-running diff with saved options", function()
48+
state.set_local_review(mock_data.local_diff, {
49+
target = "main",
50+
tracked_only = true,
51+
})
52+
53+
cli.get_local_diff.invokes(function(_, callback)
54+
callback({
55+
git_root = "/tmp/test",
56+
files = {
57+
{ path = "src/synced.lua", status = "modified", change_blocks = {} },
58+
},
59+
}, nil)
60+
end)
61+
62+
neo_reviewer.sync()
63+
64+
assert.stub(cli.get_local_diff).was_called(1)
65+
assert.stub(cli.get_local_diff).was_called_with({
66+
target = "main",
67+
tracked_only = true,
68+
}, match._)
69+
70+
local review = state.get_review()
71+
assert.is_not_nil(review)
72+
assert.are.equal("local", review.review_type)
73+
assert.are.equal(1, #review.files)
74+
assert.are.equal("src/synced.lua", review.files[1].path)
75+
end)
76+
77+
it("notifies user when local diff sync is in progress", function()
4678
state.set_local_review(mock_data.local_diff)
4779

4880
neo_reviewer.sync()
4981

5082
local notifs = notifications.get()
51-
assert.are.equal(1, #notifs)
52-
assert.matches("Sync only works for PR reviews", notifs[1].msg)
53-
assert.are.equal(vim.log.levels.WARN, notifs[1].level)
83+
local found = false
84+
for _, n in ipairs(notifs) do
85+
if n.msg:match("Syncing local diff") then
86+
found = true
87+
break
88+
end
89+
end
90+
assert.is_true(found, "Expected 'Syncing local diff...' notification")
91+
end)
92+
93+
it("ends local review when sync finds no changes", function()
94+
state.set_local_review(mock_data.local_diff)
95+
96+
cli.get_local_diff.invokes(function(_, callback)
97+
callback({
98+
git_root = "/tmp/test",
99+
files = {},
100+
}, nil)
101+
end)
102+
103+
neo_reviewer.sync()
104+
105+
assert.is_nil(state.get_review())
106+
107+
local notifs = notifications.get()
108+
local found = false
109+
for _, n in ipairs(notifs) do
110+
if n.msg:match("No changes to review") and n.level == vim.log.levels.WARN then
111+
found = true
112+
break
113+
end
114+
end
115+
assert.is_true(found, "Expected 'No changes to review' warning")
54116
end)
55117

56118
it("warns when PR review has no url", function()

0 commit comments

Comments
 (0)