Skip to content

Commit e7a7eb1

Browse files
wesmclaude
andauthored
Fix queue cursor behavior when hide-addressed active and addressing review from review screen (#52)
The cursor was getting "swallowed" if that makes sense. Also adds an AGENTS.md file to help Codex work better. --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent aee0d2a commit e7a7eb1

File tree

3 files changed

+237
-0
lines changed

3 files changed

+237
-0
lines changed

AGENTS.md

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
# AGENTS.md
2+
3+
## Purpose
4+
5+
This repo hosts roborev, a local daemon + CLI for AI-assisted code review. Use this guide to
6+
stay aligned with project conventions when reviewing code or addressing feedback.
7+
8+
## Project Orientation
9+
10+
- CLI entry point: `cmd/roborev/main.go`
11+
- Daemon entry point: `cmd/roborevd/main.go`
12+
- HTTP API: `internal/daemon/server.go`
13+
- Worker pool + job processing: `internal/daemon/worker.go`
14+
- SQLite storage: `internal/storage/`
15+
- Agent interface + implementations: `internal/agent/`
16+
- Config loading/agent resolution: `internal/config/config.go`
17+
18+
Runtime:
19+
- Daemon listens on `127.0.0.1:7373` (auto-increment if busy)
20+
- Runtime info at `~/.roborev/daemon.json`
21+
- DB at `~/.roborev/reviews.db` (WAL mode)
22+
- Data dir override via `ROBOREV_DATA_DIR`
23+
24+
## Development Preferences
25+
26+
- Keep changes simple; avoid over-engineering.
27+
- Prefer Go stdlib over new dependencies.
28+
- No emojis in code or output (commit messages are fine).
29+
- Never amend commits; fixes should be new commits.
30+
- Never push/pull or change branches unless explicitly asked.
31+
- Release builds use `CGO_ENABLED=0` (SQLite requires CGO locally).
32+
33+
## Testing
34+
35+
- Tests should be fast and isolated; use `t.TempDir()`.
36+
- Use the `agent = "test"` path to avoid calling real AI agents.
37+
- Suggested commands: `go test ./...`, `go build ./...`, `make install`.
38+
39+
## Review/Refine Guidance
40+
41+
When reviewing or fixing issues:
42+
- Focus on correctness, concurrency safety, and error handling in daemon/worker code.
43+
- For storage changes, keep migrations minimal and validate schema/queries.
44+
- For API changes, preserve HTTP/JSON conventions (no gRPC).
45+
- When addressing review feedback, update tests if behavior changes.
46+
- If diffs are large or truncated, inspect with `git show <sha>`.
47+
48+
## Config + Runtime Notes
49+
50+
- Config priority: CLI flags → `.roborev.toml``~/.roborev/config.toml`.
51+
- Reasoning defaults: reviews = thorough, refine = standard.
52+
- `roborev refine` runs agents without sandboxing; only use on trusted code.

cmd/roborev/tui.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,25 @@ func (m *tuiModel) findPrevViewableJob() int {
686686
return -1
687687
}
688688

689+
// normalizeSelectionIfHidden adjusts selectedIdx/selectedJobID if the current
690+
// selection is hidden (e.g., marked addressed with hideAddressed filter active).
691+
// Call this when returning to queue view from review view.
692+
func (m *tuiModel) normalizeSelectionIfHidden() {
693+
if m.selectedIdx >= 0 && m.selectedIdx < len(m.jobs) && !m.isJobVisible(m.jobs[m.selectedIdx]) {
694+
nextIdx := m.findNextVisibleJob(m.selectedIdx)
695+
if nextIdx < 0 {
696+
nextIdx = m.findPrevVisibleJob(m.selectedIdx)
697+
}
698+
if nextIdx < 0 {
699+
nextIdx = m.findFirstVisibleJob()
700+
}
701+
if nextIdx >= 0 {
702+
m.selectedIdx = nextIdx
703+
m.updateSelectedJobID()
704+
}
705+
}
706+
}
707+
689708
// cancelJob sends a cancel request to the server
690709
func (m tuiModel) cancelJob(jobID int64, oldStatus storage.JobStatus, oldFinishedAt *time.Time) tea.Cmd {
691710
return func() tea.Msg {
@@ -952,6 +971,7 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
952971
m.currentView = tuiViewQueue
953972
m.currentReview = nil
954973
m.reviewScroll = 0
974+
m.normalizeSelectionIfHidden()
955975
return m, nil
956976
}
957977
if m.currentView == tuiViewPrompt {
@@ -1287,6 +1307,7 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
12871307
m.currentView = tuiViewQueue
12881308
m.currentReview = nil
12891309
m.reviewScroll = 0
1310+
m.normalizeSelectionIfHidden()
12901311
} else if m.currentView == tuiViewPrompt {
12911312
// Go back to where we came from
12921313
if m.promptFromQueue {

cmd/roborev/tui_test.go

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,170 @@ func TestTUIToggleAddressedNoReview(t *testing.T) {
189189
}
190190
}
191191

192+
func TestTUIAddressFromReviewViewWithHideAddressed(t *testing.T) {
193+
// When hideAddressed is on and user marks a review as addressed from review view,
194+
// selection should move to next visible job when returning to queue (on escape),
195+
// not immediately on 'a' press (so j/k navigation still works in review view)
196+
m := newTuiModel("http://localhost")
197+
m.currentView = tuiViewReview
198+
m.hideAddressed = true
199+
addr1, addr2, addr3 := false, false, false
200+
m.jobs = []storage.ReviewJob{
201+
{ID: 1, Status: storage.JobStatusDone, Addressed: &addr1},
202+
{ID: 2, Status: storage.JobStatusDone, Addressed: &addr2},
203+
{ID: 3, Status: storage.JobStatusDone, Addressed: &addr3},
204+
}
205+
m.selectedIdx = 1 // Currently viewing job 2
206+
m.selectedJobID = 2
207+
m.currentReview = &storage.Review{
208+
ID: 10,
209+
JobID: 2,
210+
Addressed: false,
211+
Job: &m.jobs[1],
212+
}
213+
214+
// Press 'a' to mark as addressed - selection should NOT move yet
215+
updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'a'}})
216+
m2 := updated.(tuiModel)
217+
218+
// Selection stays at index 1 so j/k navigation still works relative to current review
219+
if m2.selectedIdx != 1 {
220+
t.Errorf("After 'a': expected selectedIdx=1 (unchanged), got %d", m2.selectedIdx)
221+
}
222+
if m2.selectedJobID != 2 {
223+
t.Errorf("After 'a': expected selectedJobID=2 (unchanged), got %d", m2.selectedJobID)
224+
}
225+
226+
// Press escape to return to queue - NOW selection should move
227+
updated2, _ := m2.Update(tea.KeyMsg{Type: tea.KeyEscape})
228+
m3 := updated2.(tuiModel)
229+
230+
// Selection should have moved to next visible job (job 3, index 2)
231+
// since job 2 is now addressed and hidden
232+
if m3.selectedIdx != 2 {
233+
t.Errorf("After escape: expected selectedIdx=2 (next visible job), got %d", m3.selectedIdx)
234+
}
235+
if m3.selectedJobID != 3 {
236+
t.Errorf("After escape: expected selectedJobID=3, got %d", m3.selectedJobID)
237+
}
238+
if m3.currentView != tuiViewQueue {
239+
t.Errorf("After escape: expected view=queue, got %d", m3.currentView)
240+
}
241+
}
242+
243+
func TestTUIAddressFromReviewViewFallbackToPrev(t *testing.T) {
244+
// When at end of list and marking addressed, should fall back to previous visible job
245+
m := newTuiModel("http://localhost")
246+
m.currentView = tuiViewReview
247+
m.hideAddressed = true
248+
addr1, addr2, addr3 := false, false, false
249+
m.jobs = []storage.ReviewJob{
250+
{ID: 1, Status: storage.JobStatusDone, Addressed: &addr1},
251+
{ID: 2, Status: storage.JobStatusDone, Addressed: &addr2},
252+
{ID: 3, Status: storage.JobStatusDone, Addressed: &addr3},
253+
}
254+
m.selectedIdx = 2 // Currently viewing job 3 (last one)
255+
m.selectedJobID = 3
256+
m.currentReview = &storage.Review{
257+
ID: 10,
258+
JobID: 3,
259+
Addressed: false,
260+
Job: &m.jobs[2],
261+
}
262+
263+
// Press 'a' then escape
264+
updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'a'}})
265+
m2 := updated.(tuiModel)
266+
updated2, _ := m2.Update(tea.KeyMsg{Type: tea.KeyEscape})
267+
m3 := updated2.(tuiModel)
268+
269+
// Should fall back to previous visible job (job 2, index 1)
270+
if m3.selectedIdx != 1 {
271+
t.Errorf("Expected selectedIdx=1 (prev visible job), got %d", m3.selectedIdx)
272+
}
273+
if m3.selectedJobID != 2 {
274+
t.Errorf("Expected selectedJobID=2, got %d", m3.selectedJobID)
275+
}
276+
}
277+
278+
func TestTUIAddressFromReviewViewExitWithQ(t *testing.T) {
279+
// Same as TestTUIAddressFromReviewViewWithHideAddressed but exits with 'q'
280+
m := newTuiModel("http://localhost")
281+
m.currentView = tuiViewReview
282+
m.hideAddressed = true
283+
addr1, addr2, addr3 := false, false, false
284+
m.jobs = []storage.ReviewJob{
285+
{ID: 1, Status: storage.JobStatusDone, Addressed: &addr1},
286+
{ID: 2, Status: storage.JobStatusDone, Addressed: &addr2},
287+
{ID: 3, Status: storage.JobStatusDone, Addressed: &addr3},
288+
}
289+
m.selectedIdx = 1
290+
m.selectedJobID = 2
291+
m.currentReview = &storage.Review{
292+
ID: 10,
293+
JobID: 2,
294+
Addressed: false,
295+
Job: &m.jobs[1],
296+
}
297+
298+
// Press 'a' to mark as addressed
299+
updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'a'}})
300+
m2 := updated.(tuiModel)
301+
302+
// Press 'q' to return to queue - selection should normalize
303+
updated2, _ := m2.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'q'}})
304+
m3 := updated2.(tuiModel)
305+
306+
if m3.selectedIdx != 2 {
307+
t.Errorf("Expected selectedIdx=2 (next visible job), got %d", m3.selectedIdx)
308+
}
309+
if m3.selectedJobID != 3 {
310+
t.Errorf("Expected selectedJobID=3, got %d", m3.selectedJobID)
311+
}
312+
if m3.currentView != tuiViewQueue {
313+
t.Errorf("Expected view=queue, got %d", m3.currentView)
314+
}
315+
}
316+
317+
func TestTUIAddressFromReviewViewExitWithCtrlC(t *testing.T) {
318+
// Same as TestTUIAddressFromReviewViewExitWithQ but exits with ctrl+c
319+
m := newTuiModel("http://localhost")
320+
m.currentView = tuiViewReview
321+
m.hideAddressed = true
322+
addr1, addr2, addr3 := false, false, false
323+
m.jobs = []storage.ReviewJob{
324+
{ID: 1, Status: storage.JobStatusDone, Addressed: &addr1},
325+
{ID: 2, Status: storage.JobStatusDone, Addressed: &addr2},
326+
{ID: 3, Status: storage.JobStatusDone, Addressed: &addr3},
327+
}
328+
m.selectedIdx = 1
329+
m.selectedJobID = 2
330+
m.currentReview = &storage.Review{
331+
ID: 10,
332+
JobID: 2,
333+
Addressed: false,
334+
Job: &m.jobs[1],
335+
}
336+
337+
// Press 'a' to mark as addressed
338+
updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'a'}})
339+
m2 := updated.(tuiModel)
340+
341+
// Press ctrl+c to return to queue - selection should normalize
342+
updated2, _ := m2.Update(tea.KeyMsg{Type: tea.KeyCtrlC})
343+
m3 := updated2.(tuiModel)
344+
345+
if m3.selectedIdx != 2 {
346+
t.Errorf("Expected selectedIdx=2 (next visible job), got %d", m3.selectedIdx)
347+
}
348+
if m3.selectedJobID != 3 {
349+
t.Errorf("Expected selectedJobID=3, got %d", m3.selectedJobID)
350+
}
351+
if m3.currentView != tuiViewQueue {
352+
t.Errorf("Expected view=queue, got %d", m3.currentView)
353+
}
354+
}
355+
192356
// addressRequest is used to decode and validate POST body in tests
193357
type addressRequest struct {
194358
ReviewID int64 `json:"review_id"`

0 commit comments

Comments
 (0)