`previous_reviews` omits the "do not re-raise" instruction `in_range_reviews` has, so a `fix` commit's review can ask to revert the fix

[hughdbrown]

roborev v0.64.0 · agent claude-code · macOS 26.4.1 arm64

Summary

A review of a commit produced by roborev fix receives the findings that commit closed, and is told to check whether they were addressed — but is never told not to re-raise them, and not to reverse what the fix introduced. The sibling prompt section for in-range reviews carries exactly that instruction. The omission lets a fix commit's review ask for the fix to be undone, which prevents refine from reaching a fixed point.

The defect

Both blocks are in internal/prompt/templates/prompt_sections.md.gotmpl:

{{define "previous_reviews"}}...
The following are reviews of recent commits in this repository. Use them as context
to understand ongoing work and to check if the current commit addresses previous feedback.

{{define "in_range_reviews"}}...
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.

previous_reviews has the checking instruction. Only in_range_reviews has the suppression instruction.

This section is populated for every single-commit review: buildSinglePrompt sets ctx.optional.PreviousReviews (internal/prompt/prompt.go:1106) from getPreviousReviewContexts (:1634), which resolves the reviewed commit's parent SHAs and loads each parent's stored review plus its comments. In a linear fix loop the parent is the commit whose review produced the findings, so the findings and the roborev-fix provenance comment are both in the prompt.

Evidence

Verifiable on any existing fix-commit review, no run required:

$ roborev show --prompt <job-id> | grep -nE '^## |Do not re-raise'
51:## Previous Reviews
61:## Review Findings      <- the parent commit's findings, verbatim
109:## Review Findings
141:## Current Commit

Do not re-raise matches nothing. The prior findings are present; the instruction not to re-litigate them is not.

The consequence, concretely. A commit introduced a moves_selection boolean to make a match exhaustive. That commit's own review had the originating findings in its prompt, and returned:

Problem: … The moves_selection boolean plus the if !moves_selection { return; } guard adds a level of indirection that the original returns expressed directly …
Fix: Keep the early returns in the three scroll arms …

That is a request to revert the change under review. Nothing in the prompt told it not to.

Reproduction

1. roborev init in a repo, commit a ~100-line source file with tests.
2. Let the post-commit review produce a finding, then roborev fix it.
3. roborev show --prompt <job-id-of-the-review-of-the-fix-commit> — confirm ## Previous Reviews contains the originating findings and that Do not re-raise is absent.
4. Read that review. It is free to re-raise the closed finding or object to the construct the fix introduced.

Suggested fix

Give previous_reviews the suppression language its sibling already has, e.g.:

Issues raised in these reviews may have been fixed by the current commit. Do not re-raise them unless they persist in the code as it now stands, and do not ask to undo a construct that one of these commits deliberately introduced to address earlier feedback.

One template change. No new data flow — the findings are already in the prompt.

Contributing defaults

Two defaults make the above easier to hit, both verifiable by inspection:

• roborev init writes no .roborev.toml, and review_guidelines defaults to '', so reviews run with no project standard.
• review_min_severity, refine_min_severity, fix_min_severity and ci.min_severity all default to '', so Low findings drive fix commits, each of which re-triggers review of the whole file.

I'd suggest treating these as secondary to the template fix. In a controlled comparison on one repo — same commit, same agent and model, guidelines toggled and everything else held constant — the guided run returned "No issues found." while the unguided run returned a Medium that was real (a test that hand-set its viewport instead of drawing, so the paging arithmetic was unpinned). Guidelines and severity floors bought termination at a cost in recall. The missing suppression line has no such tradeoff: it only stops the reviewer re-litigating what it was just told is closed.