Skip to content

Commit a12693f

Browse files
wesmclaude
andauthored
TUI improvements: commit viewer, help modal, navigation feedback (#123)
Fixes #95 ## Summary - **Commit message viewer** (`m` key): View commit messages from queue or review view - **Keyboard help modal** (`?` key): Toggle help overlay with available shortcuts - **Navigation feedback**: Flash notifications at queue/review boundaries - **Update notification**: Moved to line 3, only shown in queue view - **Terminology**: Renamed "respond" to "comment" throughout CLI and API ## Test plan - [x] Verify `m` shows commit messages, `?` toggles help modal - [x] Verify navigation shows "No newer/older review" at boundaries - [x] Run `go test ./...` 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 5d34079 commit a12693f

File tree

30 files changed

+1270
-388
lines changed

30 files changed

+1270
-388
lines changed

cmd/roborev/client_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package main
22

3-
// Tests for daemon client functions (getResponsesForJob, waitForReview, findJobForCommit)
3+
// Tests for daemon client functions (getCommentsForJob, waitForReview, findJobForCommit)
44

55
import (
66
"encoding/json"
@@ -14,10 +14,10 @@ import (
1414
"github.com/roborev-dev/roborev/internal/storage"
1515
)
1616

17-
func TestGetResponsesForJob(t *testing.T) {
17+
func TestGetCommentsForJob(t *testing.T) {
1818
t.Run("returns responses for job", func(t *testing.T) {
1919
_, cleanup := setupMockDaemon(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
20-
if r.URL.Path != "/api/responses" || r.Method != "GET" {
20+
if r.URL.Path != "/api/comments" || r.Method != "GET" {
2121
t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
2222
w.WriteHeader(http.StatusNotFound)
2323
return
@@ -36,7 +36,7 @@ func TestGetResponsesForJob(t *testing.T) {
3636
}))
3737
defer cleanup()
3838

39-
responses, err := getResponsesForJob(42)
39+
responses, err := getCommentsForJob(42)
4040
if err != nil {
4141
t.Fatalf("unexpected error: %v", err)
4242
}
@@ -51,7 +51,7 @@ func TestGetResponsesForJob(t *testing.T) {
5151
}))
5252
defer cleanup()
5353

54-
_, err := getResponsesForJob(42)
54+
_, err := getCommentsForJob(42)
5555
if err == nil {
5656
t.Fatal("expected error, got nil")
5757
}
Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package main
22

3-
// Tests for the respond command
3+
// Tests for the comment command
44

55
import (
66
"encoding/json"
@@ -12,15 +12,15 @@ import (
1212
"github.com/roborev-dev/roborev/internal/version"
1313
)
1414

15-
func TestRespondJobFlag(t *testing.T) {
15+
func TestCommentJobFlag(t *testing.T) {
1616
t.Run("--job forces job ID interpretation", func(t *testing.T) {
1717
var receivedJobID int64
1818
_, cleanup := setupMockDaemon(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1919
if r.URL.Path == "/api/status" {
2020
json.NewEncoder(w).Encode(map[string]interface{}{"version": version.Version})
2121
return
2222
}
23-
if r.URL.Path == "/api/respond" && r.Method == "POST" {
23+
if r.URL.Path == "/api/comment" && r.Method == "POST" {
2424
var req struct {
2525
JobID int64 `json:"job_id"`
2626
}
@@ -34,7 +34,7 @@ func TestRespondJobFlag(t *testing.T) {
3434
defer cleanup()
3535

3636
// "1234567" could be a SHA, but --job forces job ID interpretation
37-
cmd := respondCmd()
37+
cmd := commentCmd()
3838
cmd.SetArgs([]string{"--job", "1234567", "-m", "test message"})
3939
err := cmd.Execute()
4040
if err != nil {
@@ -52,7 +52,7 @@ func TestRespondJobFlag(t *testing.T) {
5252
}))
5353
defer cleanup()
5454

55-
cmd := respondCmd()
55+
cmd := commentCmd()
5656
cmd.SetArgs([]string{"--job", "abc123", "-m", "test"})
5757
err := cmd.Execute()
5858
if err == nil {

cmd/roborev/main.go

Lines changed: 44 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ func main() {
5454
rootCmd.AddCommand(reviewCmd())
5555
rootCmd.AddCommand(statusCmd())
5656
rootCmd.AddCommand(showCmd())
57-
rootCmd.AddCommand(respondCmd())
57+
rootCmd.AddCommand(commentCmd())
58+
rootCmd.AddCommand(respondCmd()) // hidden alias for backward compatibility
5859
rootCmd.AddCommand(addressCmd())
5960
rootCmd.AddCommand(installHookCmd())
6061
rootCmd.AddCommand(uninstallHookCmd())
@@ -1199,27 +1200,27 @@ Examples:
11991200
return cmd
12001201
}
12011202

1202-
func respondCmd() *cobra.Command {
1203+
func commentCmd() *cobra.Command {
12031204
var (
1204-
responder string
1205+
commenter string
12051206
message string
12061207
forceJobID bool
12071208
)
12081209

12091210
cmd := &cobra.Command{
1210-
Use: "respond <job_id|sha> [message]",
1211-
Short: "Add a response to a review",
1212-
Long: `Add a response or note to a review.
1211+
Use: "comment <job_id|sha> [message]",
1212+
Short: "Add a comment to a review",
1213+
Long: `Add a comment or note to a review.
12131214
12141215
The first argument can be either a job ID (numeric) or a commit SHA.
12151216
Using job IDs is recommended since they are displayed in the TUI.
12161217
12171218
Examples:
1218-
roborev respond 42 "Fixed the null pointer issue"
1219-
roborev respond 42 -m "Added missing error handling"
1220-
roborev respond abc123 "Addressed by refactoring"
1221-
roborev respond 42 # Opens editor for message
1222-
roborev respond --job 1234567 "msg" # Force numeric arg as job ID`,
1219+
roborev comment 42 "Fixed the null pointer issue"
1220+
roborev comment 42 -m "Added missing error handling"
1221+
roborev comment abc123 "Addressed by refactoring"
1222+
roborev comment 42 # Opens editor for message
1223+
roborev comment --job 1234567 "msg" # Force numeric arg as job ID`,
12231224
Args: cobra.RangeArgs(1, 2),
12241225
RunE: func(cmd *cobra.Command, args []string) error {
12251226
// Ensure daemon is running
@@ -1272,7 +1273,7 @@ Examples:
12721273
editor = "vim"
12731274
}
12741275

1275-
tmpfile, err := os.CreateTemp("", "roborev-response-*.md")
1276+
tmpfile, err := os.CreateTemp("", "roborev-comment-*.md")
12761277
if err != nil {
12771278
return fmt.Errorf("create temp file: %w", err)
12781279
}
@@ -1289,26 +1290,26 @@ Examples:
12891290

12901291
content, err := os.ReadFile(tmpfile.Name())
12911292
if err != nil {
1292-
return fmt.Errorf("read response: %w", err)
1293+
return fmt.Errorf("read comment: %w", err)
12931294
}
12941295
message = strings.TrimSpace(string(content))
12951296
}
12961297

12971298
if message == "" {
1298-
return fmt.Errorf("empty response, aborting")
1299+
return fmt.Errorf("empty comment, aborting")
12991300
}
13001301

1301-
if responder == "" {
1302-
responder = os.Getenv("USER")
1303-
if responder == "" {
1304-
responder = "anonymous"
1302+
if commenter == "" {
1303+
commenter = os.Getenv("USER")
1304+
if commenter == "" {
1305+
commenter = "anonymous"
13051306
}
13061307
}
13071308

13081309
// Build request with either job_id or sha
13091310
reqData := map[string]interface{}{
1310-
"responder": responder,
1311-
"response": message,
1311+
"commenter": commenter,
1312+
"comment": message,
13121313
}
13131314
if jobID != 0 {
13141315
reqData["job_id"] = jobID
@@ -1319,29 +1320,37 @@ Examples:
13191320
reqBody, _ := json.Marshal(reqData)
13201321

13211322
addr := getDaemonAddr()
1322-
resp, err := http.Post(addr+"/api/respond", "application/json", bytes.NewReader(reqBody))
1323+
resp, err := http.Post(addr+"/api/comment", "application/json", bytes.NewReader(reqBody))
13231324
if err != nil {
13241325
return fmt.Errorf("failed to connect to daemon: %w", err)
13251326
}
13261327
defer resp.Body.Close()
13271328

13281329
if resp.StatusCode != http.StatusCreated {
13291330
body, _ := io.ReadAll(resp.Body)
1330-
return fmt.Errorf("failed to add response: %s", body)
1331+
return fmt.Errorf("failed to add comment: %s", body)
13311332
}
13321333

1333-
fmt.Println("Response added successfully")
1334+
fmt.Println("Comment added successfully")
13341335
return nil
13351336
},
13361337
}
13371338

1338-
cmd.Flags().StringVar(&responder, "responder", "", "responder name (default: $USER)")
1339-
cmd.Flags().StringVarP(&message, "message", "m", "", "response message (opens editor if not provided)")
1339+
cmd.Flags().StringVar(&commenter, "commenter", "", "commenter name (default: $USER)")
1340+
cmd.Flags().StringVarP(&message, "message", "m", "", "comment message (opens editor if not provided)")
13401341
cmd.Flags().BoolVar(&forceJobID, "job", false, "force argument to be treated as job ID (not SHA)")
13411342

13421343
return cmd
13431344
}
13441345

1346+
// respondCmd returns an alias for commentCmd
1347+
func respondCmd() *cobra.Command {
1348+
cmd := commentCmd()
1349+
cmd.Use = "respond <job_id|sha> [message]"
1350+
cmd.Short = "Alias for 'comment' - add a comment to a review"
1351+
return cmd
1352+
}
1353+
13451354
func addressCmd() *cobra.Command {
13461355
var unaddress bool
13471356

@@ -1561,19 +1570,19 @@ func enqueueReview(repoPath, gitRef, agentName string) (int64, error) {
15611570
return job.ID, nil
15621571
}
15631572

1564-
// getResponsesForJob fetches responses for a job
1565-
func getResponsesForJob(jobID int64) ([]storage.Response, error) {
1573+
// getCommentsForJob fetches comments for a job
1574+
func getCommentsForJob(jobID int64) ([]storage.Response, error) {
15661575
addr := getDaemonAddr()
15671576
client := &http.Client{Timeout: 5 * time.Second}
15681577

1569-
resp, err := client.Get(fmt.Sprintf("%s/api/responses?job_id=%d", addr, jobID))
1578+
resp, err := client.Get(fmt.Sprintf("%s/api/comments?job_id=%d", addr, jobID))
15701579
if err != nil {
15711580
return nil, err
15721581
}
15731582
defer resp.Body.Close()
15741583

15751584
if resp.StatusCode != http.StatusOK {
1576-
return nil, fmt.Errorf("fetch responses: %s", resp.Status)
1585+
return nil, fmt.Errorf("fetch comments: %s", resp.Status)
15771586
}
15781587

15791588
var result struct {
@@ -2139,7 +2148,7 @@ func syncStatusCmd() *cobra.Command {
21392148
const maxPending = 1000
21402149
jobs, jobsErr := db.GetJobsToSync(machineID, maxPending)
21412150
reviews, reviewsErr := db.GetReviewsToSync(machineID, maxPending)
2142-
responses, responsesErr := db.GetResponsesToSync(machineID, maxPending)
2151+
responses, responsesErr := db.GetCommentsToSync(machineID, maxPending)
21432152

21442153
fmt.Println()
21452154
if jobsErr != nil || reviewsErr != nil || responsesErr != nil {
@@ -2153,7 +2162,7 @@ func syncStatusCmd() *cobra.Command {
21532162
}
21542163
return fmt.Sprintf("%d", count)
21552164
}
2156-
fmt.Printf("Pending push: %s jobs, %s reviews, %s responses\n",
2165+
fmt.Printf("Pending push: %s jobs, %s reviews, %s comments\n",
21572166
formatCount(len(jobs)), formatCount(len(reviews)), formatCount(len(responses)))
21582167

21592168
// Try to connect to PostgreSQL
@@ -2257,13 +2266,13 @@ func syncNowCmd() *cobra.Command {
22572266
totalJobs := getInt(msg, "total_jobs")
22582267
totalRevs := getInt(msg, "total_revs")
22592268
totalResps := getInt(msg, "total_resps")
2260-
fmt.Printf("\rPushing: batch %d (total: %d jobs, %d reviews, %d responses) ",
2269+
fmt.Printf("\rPushing: batch %d (total: %d jobs, %d reviews, %d comments) ",
22612270
batch, totalJobs, totalRevs, totalResps)
22622271
} else if phase == "pull" {
22632272
totalJobs := getInt(msg, "total_jobs")
22642273
totalRevs := getInt(msg, "total_revs")
22652274
totalResps := getInt(msg, "total_resps")
2266-
fmt.Printf("\rPulled: %d jobs, %d reviews, %d responses \n",
2275+
fmt.Printf("\rPulled: %d jobs, %d reviews, %d comments \n",
22672276
totalJobs, totalRevs, totalResps)
22682277
}
22692278
case "error":
@@ -2289,9 +2298,9 @@ func syncNowCmd() *cobra.Command {
22892298
}
22902299

22912300
fmt.Println("Sync completed")
2292-
fmt.Printf("Pushed: %d jobs, %d reviews, %d responses\n",
2301+
fmt.Printf("Pushed: %d jobs, %d reviews, %d comments\n",
22932302
finalPushed.Jobs, finalPushed.Reviews, finalPushed.Responses)
2294-
fmt.Printf("Pulled: %d jobs, %d reviews, %d responses\n",
2303+
fmt.Printf("Pulled: %d jobs, %d reviews, %d comments\n",
22952304
finalPulled.Jobs, finalPulled.Reviews, finalPulled.Responses)
22962305

22972306
return nil

cmd/roborev/main_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ func TestRefineNoChangeRetryLogic(t *testing.T) {
293293
}))
294294
defer cleanup()
295295

296-
responses, _ := getResponsesForJob(1)
296+
responses, _ := getCommentsForJob(1)
297297

298298
// Count no-change attempts
299299
noChangeAttempts := 0
@@ -350,7 +350,7 @@ func TestRunRefineSurfacesResponseErrors(t *testing.T) {
350350
json.NewEncoder(w).Encode(storage.Review{
351351
ID: 1, JobID: 1, Output: "**Bug found**: fail", Addressed: false,
352352
})
353-
case r.URL.Path == "/api/responses":
353+
case r.URL.Path == "/api/comments":
354354
w.WriteHeader(http.StatusInternalServerError)
355355
default:
356356
w.WriteHeader(http.StatusNotFound)
@@ -634,7 +634,7 @@ func createMockRefineHandler(state *mockRefineState) http.Handler {
634634
}
635635
json.NewEncoder(w).Encode(reviewCopy)
636636

637-
case r.URL.Path == "/api/responses" && r.Method == "GET":
637+
case r.URL.Path == "/api/comments" && r.Method == "GET":
638638
jobIDStr := r.URL.Query().Get("job_id")
639639
var jobID int64
640640
fmt.Sscanf(jobIDStr, "%d", &jobID)
@@ -648,7 +648,7 @@ func createMockRefineHandler(state *mockRefineState) http.Handler {
648648
"responses": responses,
649649
})
650650

651-
case r.URL.Path == "/api/respond" && r.Method == "POST":
651+
case r.URL.Path == "/api/comment" && r.Method == "POST":
652652
var req struct {
653653
JobID int64 `json:"job_id"`
654654
Responder string `json:"responder"`
@@ -807,7 +807,7 @@ func TestRefineLoopNoChangeRetryScenario(t *testing.T) {
807807
_, cleanup := setupMockDaemon(t, createMockRefineHandler(state))
808808
defer cleanup()
809809

810-
responses, err := getResponsesForJob(42)
810+
responses, err := getCommentsForJob(42)
811811
if err != nil {
812812
t.Fatalf("unexpected error: %v", err)
813813
}
@@ -834,7 +834,7 @@ func TestRefineLoopNoChangeRetryScenario(t *testing.T) {
834834
_, cleanup := setupMockDaemon(t, createMockRefineHandler(state))
835835
defer cleanup()
836836

837-
responses, _ := getResponsesForJob(42)
837+
responses, _ := getCommentsForJob(42)
838838
noChangeAttempts := countNoChangeAttempts(responses)
839839

840840
// Should not give up yet (first attempt)
@@ -856,7 +856,7 @@ func TestRefineLoopNoChangeRetryScenario(t *testing.T) {
856856
_, cleanup := setupMockDaemon(t, createMockRefineHandler(state))
857857
defer cleanup()
858858

859-
responses, _ := getResponsesForJob(42)
859+
responses, _ := getCommentsForJob(42)
860860
noChangeAttempts := countNoChangeAttempts(responses)
861861

862862
// Should only count the 1 response from roborev-refine, not the others
@@ -1068,7 +1068,7 @@ func TestRefineLoopStaysOnFailedFixChain(t *testing.T) {
10681068
}
10691069
json.NewEncoder(w).Encode(review)
10701070

1071-
case r.URL.Path == "/api/responses" && r.Method == http.MethodGet:
1071+
case r.URL.Path == "/api/comments" && r.Method == http.MethodGet:
10721072
jobIDStr := r.URL.Query().Get("job_id")
10731073
var jobID int64
10741074
fmt.Sscanf(jobIDStr, "%d", &jobID)
@@ -1080,7 +1080,7 @@ func TestRefineLoopStaysOnFailedFixChain(t *testing.T) {
10801080
"responses": responses,
10811081
})
10821082

1083-
case r.URL.Path == "/api/respond" && r.Method == http.MethodPost:
1083+
case r.URL.Path == "/api/comment" && r.Method == http.MethodPost:
10841084
var req struct {
10851085
JobID int64 `json:"job_id"`
10861086
Responder string `json:"responder"`

0 commit comments

Comments
 (0)