Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
1f7b0fe
fix: include user comments in fix prompts
Apr 1, 2026
eb0d1e6
refactor: split responses into tool attempts vs user comments
Apr 1, 2026
ae7fd8a
fix: split mixed responses in fix prompts and add SHA fallback
Apr 1, 2026
306be26
feat: include comments in daemon fix prompts, show --json, and fix sk…
Apr 1, 2026
3f29e50
fix: add GetAllCommentsForJob and apply legacy SHA fallback consistently
Apr 1, 2026
97eb6b8
fix: use commit_id instead of SHA for legacy comment lookup
Apr 1, 2026
9d8fc1d
fix: log errors when fetching comments for fix prompts
Apr 1, 2026
fd3f35f
docs: add sync notes to duplicated comment merge/dedup logic
Apr 1, 2026
0a400ab
docs: include TUI loadResponses in merge/dedup sync notes
Apr 1, 2026
36f7784
fix: add SHA fallback for legacy jobs without commit_id, strengthen t…
Apr 1, 2026
751b321
fix: use commit_id for legacy comment lookup in CLI and show paths
Apr 1, 2026
2ea423e
fix: use commit_id for legacy comment lookup in TUI loadResponses
Apr 1, 2026
8e3f3ec
fix: propagate legacy comment lookup errors, use deterministic test t…
Apr 1, 2026
6a74b0c
test: add error-path test for GetAllCommentsForJob legacy lookup
Apr 1, 2026
35abdec
test: assert exact job-linked comments on legacy lookup error path
Apr 1, 2026
8022a88
fix: validate gitRef as hex SHA before legacy comment fallback, warn …
Apr 1, 2026
69222a9
fix: warn on non-200 legacy comment fetch responses
Apr 1, 2026
4cbdfd5
fix: use merged comment lookup in refine command via HTTP client
Apr 1, 2026
5c1d3a0
fix: relax looksLikeSHA to accept mixed-case and 4+ char SHAs
Apr 3, 2026
4a1ada9
fix: address lint issues in looksLikeSHA and prompt tests
Apr 3, 2026
13bf6b0
fix: restore 7-char minimum for looksLikeSHA, keep mixed-case support
Apr 3, 2026
31b6946
refactor: consolidate looksLikeSHA into git.LooksLikeSHA
Apr 3, 2026
72345b6
refactor: extract MergeResponses to eliminate 5x duplicated dedup logic
Apr 5, 2026
822e2db
fix: clean up test assertions and unnecessary comments
Apr 5, 2026
bf1a325
fix: use --commenter in fix/refine skills for proper tool identification
Apr 5, 2026
b289a83
docs: simplify comment handling instruction in fix skills
Apr 5, 2026
8a5788a
merge: resolve conflict with origin/main (#624)
Apr 5, 2026
4ed45da
refactor: remove storage->git dependency, add CommitIDValue helper
Apr 5, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions cmd/roborev/compact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,9 +428,6 @@ func TestCompactWorktreeBranchResolution(t *testing.T) {
opts := compactOptions{quiet: true}
_ = runCompact(cmd, opts)

// API query should use the main repo path, not the worktree path.
// Branch filtering is handled client-side by filterReachableJobs.
require.NotEmpty(t, receivedRepo, "expected repo param to be sent")
assert.Equal(t, repo.Dir, receivedRepo,
"repo: want main repo path")
assert.Equal(t, repo.Dir, receivedRepo, "repo: want main repo path")
}
134 changes: 110 additions & 24 deletions cmd/roborev/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/roborev-dev/roborev/internal/config"
"github.com/roborev-dev/roborev/internal/daemon"
"github.com/roborev-dev/roborev/internal/git"
"github.com/roborev-dev/roborev/internal/prompt"
"github.com/roborev-dev/roborev/internal/storage"
"github.com/roborev-dev/roborev/internal/streamfmt"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -558,7 +559,7 @@ func jobReachable(
}

// SHA ref: check commit graph reachability.
if looksLikeSHA(ref) {
if git.LooksLikeSHA(ref) {
reachable, err := git.IsAncestor(worktreeRoot, ref, "HEAD")
if err != nil || reachable {
return true
Expand Down Expand Up @@ -588,21 +589,6 @@ func branchMatch(matchBranch, jobBranch string) bool {
return jobBranch == matchBranch
}

// looksLikeSHA returns true if s looks like a hex commit SHA (7-40
// hex characters). This avoids calling git merge-base on task labels
// and other non-commit refs.
func looksLikeSHA(s string) bool {
if len(s) < 7 || len(s) > 40 {
return false
}
for _, c := range []byte(s) {
if (c < '0' || c > '9') && (c < 'a' || c > 'f') {
return false
}
}
return true
}

func queryOpenJobs(
ctx context.Context,
repoRoot, branch string,
Expand Down Expand Up @@ -876,6 +862,18 @@ func fixSingleJob(cmd *cobra.Command, repoRoot string, jobID int64, opts fixOpti
}
}

// Fetch user comments for context (including legacy commit-based comments)
var commitID int64
var gitRef string
if job != nil {
commitID = job.CommitIDValue()
gitRef = job.GitRef
}
comments, commentsErr := fetchComments(ctx, addr, jobID, commitID, gitRef)
if commentsErr != nil && !opts.quiet {
cmd.Printf("Warning: could not fetch comments for job %d: %v\n", jobID, commentsErr)
}

if !opts.quiet {
cmd.Printf("Running fix agent (%s) to apply changes...\n\n", fixAgent.Name())
}
Expand All @@ -894,7 +892,7 @@ func fixSingleJob(cmd *cobra.Command, repoRoot string, jobID int64, opts fixOpti
RepoRoot: repoRoot,
Agent: fixAgent,
Output: out,
}, buildGenericFixPrompt(review.Output, minSev))
}, buildGenericFixPrompt(review.Output, minSev, comments))
if fmtr != nil {
fmtr.Flush()
}
Expand Down Expand Up @@ -952,9 +950,10 @@ func fixSingleJob(cmd *cobra.Command, repoRoot string, jobID int64, opts fixOpti

// batchEntry holds a fetched job and its review for batch processing.
type batchEntry struct {
jobID int64
job *storage.ReviewJob
review *storage.Review
jobID int64
job *storage.ReviewJob
review *storage.Review
comments []storage.Response
}

// runFixBatch discovers jobs (or uses provided IDs), splits them into batches
Expand Down Expand Up @@ -1041,7 +1040,17 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches,
}
continue
}
entries = append(entries, batchEntry{jobID: id, job: job, review: review})
var batchCommitID int64
var batchGitRef string
if job != nil {
batchCommitID = job.CommitIDValue()
batchGitRef = job.GitRef
}
comments, commentsErr := fetchComments(ctx, batchAddr, id, batchCommitID, batchGitRef)
if commentsErr != nil && !opts.quiet {
cmd.Printf("Warning: could not fetch comments for job %d: %v\n", id, commentsErr)
}
entries = append(entries, batchEntry{jobID: id, job: job, review: review, comments: comments})
}

if len(entries) == 0 {
Expand Down Expand Up @@ -1175,7 +1184,11 @@ const batchPromptFooter = "## Instructions\n\nPlease apply fixes for all the fin
// batchEntrySize returns the size of a single entry in the batch prompt.
// The index parameter is the 1-based position in the batch.
func batchEntrySize(index int, e batchEntry) int {
return len(fmt.Sprintf("## Review %d (Job %d — %s)\n\n%s\n\n", index, e.jobID, git.ShortSHA(e.job.GitRef), e.review.Output))
toolAttempts, userComments := prompt.SplitResponses(e.comments)
size := len(fmt.Sprintf("## Review %d (Job %d — %s)\n\n%s\n\n", index, e.jobID, git.ShortSHA(e.job.GitRef), e.review.Output))
size += len(prompt.FormatToolAttempts(toolAttempts))
size += len(prompt.FormatUserComments(userComments))
return size
}

// splitIntoBatches groups entries into batches respecting maxSize.
Expand Down Expand Up @@ -1215,6 +1228,7 @@ func splitIntoBatches(

// buildBatchFixPrompt creates a concatenated prompt from multiple reviews.
// When minSeverity is non-empty, a severity filtering instruction is injected.
// User comments attached to each entry are included inline.
func buildBatchFixPrompt(entries []batchEntry, minSeverity string) string {
var sb strings.Builder
sb.WriteString(batchPromptHeader)
Expand All @@ -1224,9 +1238,12 @@ func buildBatchFixPrompt(entries []batchEntry, minSeverity string) string {
}

for i, e := range entries {
toolAttempts, userComments := prompt.SplitResponses(e.comments)
fmt.Fprintf(&sb, "## Review %d (Job %d — %s)\n\n", i+1, e.jobID, git.ShortSHA(e.job.GitRef))
sb.WriteString(e.review.Output)
sb.WriteString("\n\n")
sb.WriteString(prompt.FormatToolAttempts(toolAttempts))
sb.WriteString(prompt.FormatUserComments(userComments))
}

sb.WriteString(batchPromptFooter)
Expand Down Expand Up @@ -1308,9 +1325,75 @@ func fetchReview(ctx context.Context, serverAddr string, jobID int64) (*storage.
})
}

// fetchComments retrieves comments/responses for a job, including legacy
// commit-based comments merged via storage.MergeResponses. Prefers commit_id
// (unambiguous) when available, falls back to SHA for legacy jobs.
func fetchComments(ctx context.Context, serverAddr string, jobID, commitID int64, gitRef string) ([]storage.Response, error) {
return withFixDaemonRetryContext(ctx, serverAddr, func(addr string) ([]storage.Response, error) {
client := getDaemonHTTPClient(30 * time.Second)

// Fetch by job ID
req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("%s/api/comments?job_id=%d", addr, jobID), nil)
if err != nil {
return nil, err
}

resp, err := client.Do(req)
if err != nil {
return nil, err
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("server error (%d): %s", resp.StatusCode, body)
}

var result struct {
Responses []storage.Response `json:"responses"`
}
if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
return nil, err
}
responses := result.Responses

// Also fetch legacy commit-based comments and merge.
// Prefer commit_id (unambiguous), fall back to SHA only when
// gitRef looks like a hex SHA (not a task label like "run").
var legacyURL string
if commitID > 0 {
legacyURL = fmt.Sprintf("%s/api/comments?commit_id=%d", addr, commitID)
} else if git.LooksLikeSHA(gitRef) {
legacyURL = fmt.Sprintf("%s/api/comments?sha=%s", addr, gitRef)
}
if legacyURL != "" {
legacyReq, err := http.NewRequestWithContext(ctx, "GET", legacyURL, nil)
if err == nil {
legacyResp, err := client.Do(legacyReq)
if err == nil {
defer legacyResp.Body.Close()
if legacyResp.StatusCode == http.StatusOK {
var legacyResult struct {
Responses []storage.Response `json:"responses"`
}
if json.NewDecoder(legacyResp.Body).Decode(&legacyResult) == nil {
responses = storage.MergeResponses(responses, legacyResult.Responses)
}
}
}
}
}

return responses, nil
})
}

// buildGenericFixPrompt creates a fix prompt without knowing the analysis type.
// When minSeverity is non-empty, a severity filtering instruction is prepended.
func buildGenericFixPrompt(analysisOutput, minSeverity string) string {
// Responses are split into tool attempts and user comments so each type
// receives appropriate framing in the prompt.
func buildGenericFixPrompt(analysisOutput, minSeverity string, responses []storage.Response) string {
toolAttempts, userComments := prompt.SplitResponses(responses)
var sb strings.Builder
sb.WriteString("# Fix Request\n\n")
if inst := config.SeverityInstruction(minSeverity); inst != "" {
Expand All @@ -1320,7 +1403,10 @@ func buildGenericFixPrompt(analysisOutput, minSeverity string) string {
sb.WriteString("An analysis was performed and produced the following findings:\n\n")
sb.WriteString("## Analysis Findings\n\n")
sb.WriteString(analysisOutput)
sb.WriteString("\n\n## Instructions\n\n")
sb.WriteString("\n\n")
sb.WriteString(prompt.FormatToolAttempts(toolAttempts))
sb.WriteString(prompt.FormatUserComments(userComments))
sb.WriteString("## Instructions\n\n")
sb.WriteString("Please apply the suggested changes from the analysis above. ")
sb.WriteString("Make the necessary edits to address each finding. ")
sb.WriteString("Focus on the highest priority items first.\n\n")
Expand Down
87 changes: 61 additions & 26 deletions cmd/roborev/fix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestBuildGenericFixPrompt(t *testing.T) {
- Long function in main.go:50
- Missing error handling`

prompt := buildGenericFixPrompt(analysisOutput, "")
prompt := buildGenericFixPrompt(analysisOutput, "", nil)

// Should include the analysis output
assert.Contains(t, prompt, "Issues Found")
Expand All @@ -100,6 +100,63 @@ func TestBuildGenericFixPrompt(t *testing.T) {
assert.Contains(t, prompt, "git commit")
}

func TestBuildGenericFixPromptWithComments(t *testing.T) {
comments := []storage.Response{
{Responder: "dev", Response: "Don't touch the helper function", CreatedAt: time.Date(2026, 3, 15, 10, 0, 0, 0, time.UTC)},
}
p := buildGenericFixPrompt("Found bug in foo.go", "", comments)

assert.Contains(t, p, "User Comments")
assert.Contains(t, p, "Don't touch the helper function")
assert.Contains(t, p, "Found bug in foo.go")
}

func TestBuildGenericFixPromptSplitsMixedResponses(t *testing.T) {
responses := []storage.Response{
{Responder: "roborev-fix", Response: "Fix applied (commit: abc123)", CreatedAt: time.Date(2026, 3, 15, 9, 0, 0, 0, time.UTC)},
{Responder: "alice", Response: "This is a false positive", CreatedAt: time.Date(2026, 3, 15, 10, 0, 0, 0, time.UTC)},
}
p := buildGenericFixPrompt("Found bug in foo.go", "", responses)

// Tool attempts should appear under "Previous Addressing Attempts"
assert.Contains(t, p, "Previous Addressing Attempts")
assert.Contains(t, p, "roborev-fix")

// User comments should appear under "User Comments"
assert.Contains(t, p, "User Comments")
assert.Contains(t, p, "false positive")
assert.Contains(t, p, "alice")
}

func TestBuildBatchFixPromptSplitsMixedResponses(t *testing.T) {
entries := []batchEntry{
{
jobID: 1,
job: &storage.ReviewJob{GitRef: "abc123"},
review: &storage.Review{
Output: "Found HIGH bug in foo.go",
},
comments: []storage.Response{
{Responder: "roborev-refine", Response: "Created commit def456", CreatedAt: time.Date(2026, 3, 15, 9, 0, 0, 0, time.UTC)},
{Responder: "alice", Response: "The foo.go finding is a false positive", CreatedAt: time.Date(2026, 3, 15, 10, 0, 0, 0, time.UTC)},
},
},
}
p := buildBatchFixPrompt(entries, "")

// Tool attempts should appear under "Previous Addressing Attempts"
assert.Contains(t, p, "Previous Addressing Attempts")
assert.Contains(t, p, "roborev-refine")

// User comments should appear under "User Comments"
assert.Contains(t, p, "User Comments")
assert.Contains(t, p, "false positive")
assert.Contains(t, p, "alice")

// Review output should be present
assert.Contains(t, p, "Found HIGH bug in foo.go")
}

func TestBuildGenericCommitPrompt(t *testing.T) {
prompt := buildGenericCommitPrompt()

Expand Down Expand Up @@ -2692,7 +2749,7 @@ func TestBuildGenericFixPromptMinSeverity(t *testing.T) {
output := "Found bug in foo.go"

t.Run("no filter", func(t *testing.T) {
p := buildGenericFixPrompt(output, "")
p := buildGenericFixPrompt(output, "", nil)
if strings.Contains(p, "Severity filter") {
assert.Condition(t, func() bool {
return false
Expand All @@ -2706,7 +2763,7 @@ func TestBuildGenericFixPromptMinSeverity(t *testing.T) {
})

t.Run("high filter", func(t *testing.T) {
p := buildGenericFixPrompt(output, "high")
p := buildGenericFixPrompt(output, "high", nil)
if !strings.Contains(p, "Severity filter") {
assert.Condition(t, func() bool {
return false
Expand All @@ -2725,7 +2782,7 @@ func TestBuildGenericFixPromptMinSeverity(t *testing.T) {
})

t.Run("low filter is no-op", func(t *testing.T) {
p := buildGenericFixPrompt(output, "low")
p := buildGenericFixPrompt(output, "low", nil)
if strings.Contains(p, "Severity filter") {
assert.Condition(t, func() bool {
return false
Expand Down Expand Up @@ -3019,28 +3076,6 @@ func TestFilterReachableJobsDetachedHead(t *testing.T) {
}
}

func TestLooksLikeSHA(t *testing.T) {
tests := []struct {
s string
want bool
}{
{"abc1234", true},
{"0000000000000000000000000000000000000000", true},
{"abcdef1234567890abcdef1234567890abcdef12", true},
{"abc123", false}, // too short (6 chars)
{"", false}, // empty
{"dirty", false}, // non-hex
{"run", false}, // task label
{"ABCDEF1", false}, // uppercase not valid
{"abc123..def456", false}, // range
}
for _, tt := range tests {
t.Run(tt.s, func(t *testing.T) {
assert.Equal(t, tt.want, looksLikeSHA(tt.s))
})
}
}

func TestRunFixOpenFiltersUnreachableJobs(t *testing.T) {
repo, worktreeDir := setupWorktree(t)

Expand Down
10 changes: 8 additions & 2 deletions cmd/roborev/refine.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,14 @@ func runRefine(ctx RunContext, opts refineOptions) error {
fmt.Printf("Addressing review (job %d)...\n", currentFailedReview.JobID)
}

// Get previous attempts for context
previousAttempts, err := client.GetCommentsForJob(currentFailedReview.JobID)
// Get previous attempts for context (including legacy commit-based)
var reviewCommitID int64
var reviewGitRef string
if currentFailedReview.Job != nil {
reviewCommitID = currentFailedReview.Job.CommitIDValue()
reviewGitRef = currentFailedReview.Job.GitRef
}
previousAttempts, err := client.GetAllCommentsForJob(currentFailedReview.JobID, reviewCommitID, reviewGitRef)
if err != nil {
return fmt.Errorf("fetch previous comments: %w", err)
}
Expand Down
Loading
Loading