Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
101 changes: 80 additions & 21 deletions internal/prompt/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,17 @@ responses from developers. Use this context to:
- Consider developer responses about why certain patterns exist
`

// InRangeReviewsHeader introduces per-commit reviews within a range review
const InRangeReviewsHeader = `
## Per-Commit Reviews in This Range

The following commits in this range have already been individually reviewed.
Issues found in earlier commits may have been fixed by later commits in the range.

Do not re-raise issues identified below unless they persist in the final code.
Focus on cross-commit interactions and problems not caught by per-commit reviews.
`

// ReviewContext holds a commit SHA and its associated review (if any) plus responses
type ReviewContext struct {
SHA string
Expand Down Expand Up @@ -607,6 +618,10 @@ func (b *Builder) buildRangePrompt(repoPath, rangeRef string, repoID int64, cont
return "", fmt.Errorf("get range commits: %w", err)
}

// Include per-commit reviews for commits in this range so the
// reviewer doesn't re-raise issues already found and addressed.
b.writeInRangeReviews(&optionalContext, commits)

// Commit range section
var currentRequired strings.Builder
currentRequired.WriteString("## Commit Range\n\n")
Expand Down Expand Up @@ -801,37 +816,81 @@ func (b *Builder) writePreviousAttemptsForGitRef(sb *strings.Builder, gitRef str
}
}

// getPreviousReviewContexts gets the N commits before the target and looks up their reviews and responses
func (b *Builder) getPreviousReviewContexts(repoPath, sha string, count int) ([]ReviewContext, error) {
// Get parent commits from git
parentSHAs, err := git.GetParentCommits(repoPath, sha, count)
if err != nil {
return nil, fmt.Errorf("get parent commits: %w", err)
// writeInRangeReviews writes per-commit reviews for commits within a range.
// This gives the range reviewer context about issues already found and
// addressed, preventing duplicate findings.
func (b *Builder) writeInRangeReviews(sb *strings.Builder, commits []string) {
if b.db == nil || len(commits) == 0 {
return
}

var contexts []ReviewContext
for _, parentSHA := range parentSHAs {
ctx := ReviewContext{SHA: parentSHA}
contexts := b.lookupReviewContexts(commits, true)
if len(contexts) == 0 {
return
}

// Try to look up review for this commit
review, err := b.db.GetReviewByCommitSHA(parentSHA)
if err == nil {
ctx.Review = review
sb.WriteString(InRangeReviewsHeader)
sb.WriteString("\n")

// Also fetch comments for this review's job
if review.JobID > 0 {
responses, err := b.db.GetCommentsForJob(review.JobID)
if err == nil {
ctx.Responses = responses
}
for _, ctx := range contexts {
shortSHA := git.ShortSHA(ctx.SHA)
verdict := storage.ParseVerdict(ctx.Review.Output)
verdictLabel := "unknown"
switch verdict {
case "P":
verdictLabel = "passed"
case "F":
verdictLabel = "failed"
}

fmt.Fprintf(sb, "--- Commit %s (%s, %s) ---\n",
shortSHA, ctx.Review.Agent, verdictLabel)
sb.WriteString(ctx.Review.Output)
sb.WriteString("\n")

if len(ctx.Responses) > 0 {
sb.WriteString("\nComments on this review:\n")
for _, resp := range ctx.Responses {
fmt.Fprintf(sb, "- %s: %q\n", resp.Responder, resp.Response)
}
}
// If no review found, ctx.Review stays nil
sb.WriteString("\n")
}
}

// lookupReviewContexts fetches review contexts for the given SHAs.
// When skipMissing is true, SHAs without reviews are omitted; when false,
// they are included with Review == nil (used by writePreviousReviews to
// show "No review available").
func (b *Builder) lookupReviewContexts(shas []string, skipMissing bool) []ReviewContext {
var contexts []ReviewContext
for _, sha := range shas {
review, err := b.db.GetReviewByCommitSHA(sha)
if err != nil {
if skipMissing {
continue
}
contexts = append(contexts, ReviewContext{SHA: sha})
continue
}
ctx := ReviewContext{SHA: sha, Review: review}
if review.JobID > 0 {
if responses, err := b.db.GetCommentsForJob(review.JobID); err == nil {
ctx.Responses = responses
}
}
contexts = append(contexts, ctx)
}
return contexts
}

return contexts, nil
// getPreviousReviewContexts gets the N commits before the target and looks up their reviews and responses
func (b *Builder) getPreviousReviewContexts(repoPath, sha string, count int) ([]ReviewContext, error) {
parentSHAs, err := git.GetParentCommits(repoPath, sha, count)
if err != nil {
return nil, fmt.Errorf("get parent commits: %w", err)
}
return b.lookupReviewContexts(parentSHAs, false), nil
}

// SystemPromptDesignReview is the base instruction for reviewing design documents.
Expand Down
67 changes: 67 additions & 0 deletions internal/prompt/prompt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1290,3 +1290,70 @@ func TestBuildAddressPromptSplitsResponses(t *testing.T) {
assert.Contains(t, p, "false positive")
assert.Contains(t, p, "alice")
}

func TestBuildRangePrompt_IncludesInRangeReviews(t *testing.T) {
r := newTestRepo(t)

// Create initial commit (will serve as range start / merge base)
require.NoError(t, os.WriteFile(filepath.Join(r.dir, "base.txt"), []byte("base\n"), 0o644))
r.git("add", "base.txt")
r.git("commit", "-m", "initial")
baseSHA := r.git("rev-parse", "HEAD")

// Create two commits on the "branch"
require.NoError(t, os.WriteFile(filepath.Join(r.dir, "base.txt"), []byte("change1\n"), 0o644))
r.git("add", "base.txt")
r.git("commit", "-m", "first feature commit")
commit1 := r.git("rev-parse", "HEAD")

require.NoError(t, os.WriteFile(filepath.Join(r.dir, "base.txt"), []byte("change2\n"), 0o644))
r.git("add", "base.txt")
r.git("commit", "-m", "second feature commit")
commit2 := r.git("rev-parse", "HEAD")

// Set up DB with repo and per-commit reviews
db := testutil.OpenTestDB(t)
repo, err := db.GetOrCreateRepo(r.dir)
require.NoError(t, err)

testutil.CreateCompletedReview(t, db, repo.ID, commit1, "test",
"Found bug: missing null check in handler\n\nVerdict: FAIL")
testutil.CreateCompletedReview(t, db, repo.ID, commit2, "test",
"No issues found.\n\nVerdict: PASS")

// Build range prompt
rangeRef := baseSHA + ".." + commit2
builder := NewBuilder(db)
prompt, err := builder.Build(r.dir, rangeRef, repo.ID, 0, "test", "")
require.NoError(t, err)

// Should contain the in-range reviews section
assert := assert.New(t)
assert.Contains(prompt, "Per-Commit Reviews in This Range")
assert.Contains(prompt, "Do not re-raise issues")
assert.Contains(prompt, "missing null check in handler")
assert.Contains(prompt, "No issues found.")
assert.Contains(prompt, "failed")
assert.Contains(prompt, "passed")
}

func TestBuildRangePrompt_NoInRangeReviewsWithoutDB(t *testing.T) {
r := newTestRepo(t)

require.NoError(t, os.WriteFile(filepath.Join(r.dir, "base.txt"), []byte("base\n"), 0o644))
r.git("add", "base.txt")
r.git("commit", "-m", "initial")
baseSHA := r.git("rev-parse", "HEAD")

require.NoError(t, os.WriteFile(filepath.Join(r.dir, "base.txt"), []byte("change\n"), 0o644))
r.git("add", "base.txt")
r.git("commit", "-m", "feature")
headSHA := r.git("rev-parse", "HEAD")

// Build without DB — should not include in-range reviews section
builder := NewBuilder(nil)
prompt, err := builder.Build(r.dir, baseSHA+".."+headSHA, 0, 0, "test", "")
require.NoError(t, err)

assert.NotContains(t, prompt, "Per-Commit Reviews in This Range")
}
3 changes: 2 additions & 1 deletion internal/skills/claude/roborev-refine/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ by interpolating dynamic text directly into a shell string. Review-derived
content may contain shell metacharacters that could cause unintended execution.

The comment should reference each finding by severity and file, state what was
fixed, and note any findings intentionally skipped. Keep it concise.
fixed, and note why any dismissed findings were skipped. These comments are
included in re-review prompts. Keep it concise.

#### 3d. Re-review

Expand Down
3 changes: 2 additions & 1 deletion internal/skills/codex/roborev-refine/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ by interpolating dynamic text directly into a shell string. Review-derived
content may contain shell metacharacters that could cause unintended execution.

The comment should reference each finding by severity and file, state what was
fixed, and note any findings intentionally skipped. Keep it concise.
fixed, and note why any dismissed findings were skipped. These comments are
included in re-review prompts. Keep it concise.

#### 3d. Re-review

Expand Down
Loading