-
Notifications
You must be signed in to change notification settings - Fork 41
Review agent: detect issues introduced by fix commits #1530
Copy link
Copy link
Open
Labels
agent/reviewReview agentReview agentcomponent/harnessAgent harness, config, and skills loadingAgent harness, config, and skills loadingfeatureFeature-category issue awaiting human prioritizationFeature-category issue awaiting human prioritizationpriority/mediumNormal priority, plan for next cycleNormal priority, plan for next cycletriagedTriaged but awaiting human prioritizationTriaged but awaiting human prioritizationtype/featureNew capability requestNew capability request
Metadata
Metadata
Assignees
Labels
agent/reviewReview agentReview agentcomponent/harnessAgent harness, config, and skills loadingAgent harness, config, and skills loadingfeatureFeature-category issue awaiting human prioritizationFeature-category issue awaiting human prioritizationpriority/mediumNormal priority, plan for next cycleNormal priority, plan for next cycletriagedTriaged but awaiting human prioritizationTriaged but awaiting human prioritizationtype/featureNew capability requestNew capability request
Type
Fields
Give feedbackNo fields configured for issues without a type.
Projects
Status
Todo
What happened
PR #1178 went through 3 review rounds. In round 2 (May 21), the fix for the empty-Sanitized edge case introduced a dead-code guard (
if len(result.Findings) == 0inside aif len(result.Findings) > 0block). This was only caught in round 3 (May 26) by waynesun09's 4-agent review squad. The dead code itself was a logic error that could have masked a real sanitization bypass. Related existing issues: #1088 (self-review loop), #902 (circuit breaker for review-fix loop), #1368 (approval with residual findings).What could go better
When a review agent evaluates a fix commit that addresses prior review findings, it should specifically check whether the fix introduced new issues — particularly dead code, unreachable branches, or logic contradictions near the changed lines. Currently, each review round treats the diff as a fresh review without focused attention on fix-induced regressions. Medium confidence: this pattern (fix introduces new bug caught in next round) occurred once here but is architecturally predictable for multi-round reviews. The existing issues (#1088, #902) acknowledge the broader problem but don't specifically address fix-regression detection.
Proposed change
In the review agent's prompt or skill (likely in the
reusable-review.ymlworkflow or the review agent definition infullsend-ai/fullsend), add a review dimension for fix-induced regressions. When the review detects that the current push addresses prior review comments (e.g., by checking if the PR has prior CHANGES_REQUESTED reviews), the agent should specifically scan for: (1) dead code or unreachable branches in the modified regions, (2) logic contradictions between guard conditions, (3) whether the fix actually resolves the original finding or just moves the problem. This could be a focused sub-prompt added to the review skill's checklist rather than a new agent.Validation criteria
Over the next 10 PRs that go through 2+ review rounds, track whether fix-induced regressions are caught in the same round they're introduced (round N) rather than requiring an additional round (round N+1). Success: at least 80% of fix-induced issues caught in the same review round as the fix.
Generated by retro agent from #1178