What happened
On PR #6036, the human reviewer (waynesun09) verified fix commits against the specific findings they claimed to address, and discovered that 3 of the author's fix commits did not actually resolve the original finding: (1) commit 61b9bae claimed to fix partial-result printing but the human found "printed results still mismatch disk"; (2) commit 8029af1d claimed to fix declarative attribute names but the human found it "introduces new wrong attribute name"; (3) commit 509444e4 addressed the duplicate artifact upload but introduced a new ordering dependency. The review agent (fullsend-ai-review[bot], running fullsend-ai/agents@v0) re-reviewed after each of these pushes (run 32320590280 and subsequent runs) but did not catch that the fixes were ineffective. The agent's re-review produced largely similar LOW findings without verifying whether prior-round findings were actually resolved by the new code.
What could go better
The review agent's re-review mechanism (documented in agents/review.md as severity anchoring from prior review via PRIOR_REVIEW_SHA and prior-review.txt) focuses on maintaining severity consistency across rounds, not on verifying that claimed fixes actually work. When a push arrives after a review round, the correctness sub-agent should take each prior-round finding and verify the fix resolves the described condition — not just that the code near the finding changed. This is distinct from existing proposals: agents#343 (re-review scoping for efficiency), agents#685 (tracking resolved vs. open findings), agents#587 (fix completeness across similar sites), and agents#270 (side-effect analysis on fix commits). None of those address verifying that a fix actually resolves the original condition. Confidence: high — the evidence is clear (3 broken fixes went uncaught by the agent, caught only by human verification), and the gap is reproducible (any re-review round where the author claims to fix a finding but the fix is incomplete).
Proposed change
Modify the correctness sub-agent definition (skills/pr-review/sub-agents/correctness.md) to add a fix-efficacy verification pass during re-review. When prior-review.txt is present (indicating a re-review), the correctness sub-agent should: (1) extract the specific findings from the prior round, (2) identify which files/functions changed in the new push that correspond to each prior finding, (3) for each prior finding that has a corresponding code change, verify that the new code actually resolves the condition described in the finding (e.g., if the finding said "ParseTelemetryFile discards all traces on a single bad line," verify the new code handles single bad lines gracefully), (4) flag any finding where the fix does not resolve the original condition, or where the fix introduces a new related issue. This should be a distinct section in the correctness sub-agent's output, separate from new findings.
Validation criteria
On the next 5 re-review rounds (across any repo) where the prior review had MEDIUM or higher findings and the author pushes a fix commit, the review agent should include a fix-efficacy section that explicitly states whether each prior finding is resolved, partially resolved, or unresolved. Measure false-negative rate: if a human reviewer subsequently identifies a broken fix that the agent marked as resolved, that is a miss. Target: zero misses on straightforward fix verifications (e.g., the fix clearly doesn't change the relevant code path).
Generated by retro agent from fullsend-ai/fullsend#6036
What happened
On PR #6036, the human reviewer (waynesun09) verified fix commits against the specific findings they claimed to address, and discovered that 3 of the author's fix commits did not actually resolve the original finding: (1) commit
61b9baeclaimed to fix partial-result printing but the human found "printed results still mismatch disk"; (2) commit8029af1dclaimed to fix declarative attribute names but the human found it "introduces new wrong attribute name"; (3) commit509444e4addressed the duplicate artifact upload but introduced a new ordering dependency. The review agent (fullsend-ai-review[bot], running fullsend-ai/agents@v0) re-reviewed after each of these pushes (run 32320590280 and subsequent runs) but did not catch that the fixes were ineffective. The agent's re-review produced largely similar LOW findings without verifying whether prior-round findings were actually resolved by the new code.What could go better
The review agent's re-review mechanism (documented in
agents/review.mdas severity anchoring from prior review viaPRIOR_REVIEW_SHAandprior-review.txt) focuses on maintaining severity consistency across rounds, not on verifying that claimed fixes actually work. When a push arrives after a review round, the correctness sub-agent should take each prior-round finding and verify the fix resolves the described condition — not just that the code near the finding changed. This is distinct from existing proposals: agents#343 (re-review scoping for efficiency), agents#685 (tracking resolved vs. open findings), agents#587 (fix completeness across similar sites), and agents#270 (side-effect analysis on fix commits). None of those address verifying that a fix actually resolves the original condition. Confidence: high — the evidence is clear (3 broken fixes went uncaught by the agent, caught only by human verification), and the gap is reproducible (any re-review round where the author claims to fix a finding but the fix is incomplete).Proposed change
Modify the
correctnesssub-agent definition (skills/pr-review/sub-agents/correctness.md) to add a fix-efficacy verification pass during re-review. Whenprior-review.txtis present (indicating a re-review), the correctness sub-agent should: (1) extract the specific findings from the prior round, (2) identify which files/functions changed in the new push that correspond to each prior finding, (3) for each prior finding that has a corresponding code change, verify that the new code actually resolves the condition described in the finding (e.g., if the finding said "ParseTelemetryFile discards all traces on a single bad line," verify the new code handles single bad lines gracefully), (4) flag any finding where the fix does not resolve the original condition, or where the fix introduces a new related issue. This should be a distinct section in the correctness sub-agent's output, separate from new findings.Validation criteria
On the next 5 re-review rounds (across any repo) where the prior review had MEDIUM or higher findings and the author pushes a fix commit, the review agent should include a fix-efficacy section that explicitly states whether each prior finding is resolved, partially resolved, or unresolved. Measure false-negative rate: if a human reviewer subsequently identifies a broken fix that the agent marked as resolved, that is a miss. Target: zero misses on straightforward fix verifications (e.g., the fix clearly doesn't change the relevant code path).
Generated by retro agent from fullsend-ai/fullsend#6036