Skip to content

Review agent should suppress findings where its own analysis concludes the code is acceptable #1106

Description

@fullsend-ai-retro

What happened

In Review Run 3 on PR #6744 (commit b7a2791), the review agent raised 3 LOW findings where its own analysis immediately concluded the current code was acceptable:

  1. PR title prefix: Questioned whether refactor was correct, then noted COMMITS.md says it's appropriate — no change recommended.
  2. Test integrity: Noted test overlap after feature removal, then acknowledged the tests provide regression value — no change recommended.
  3. Naming convention: Questioned the NoDispatch suffix, then concluded it's appropriate for a full removal — no change recommended.

All three findings were self-dismissed within the same finding text. The human reviewer agreed with all three non-actions (one explicit reply, two implicit via approval). These findings added approximately 300 words of non-actionable content to the review.

What could go better

When the review agent's reasoning chain concludes that a potential issue is acceptable, appropriate, or follows established conventions, the finding should be suppressed rather than emitted as a LOW. On this PR, 3 of 6 distinct findings across the final review were self-dismissed LOWs — a 50% noise rate for that run. This dilutes the signal from genuinely valuable findings (like the HIGH stale-reference catch in Run 1) and adds reading burden for the human reviewer.

Confidence: moderate. This pattern was clear on this PR's final review, but I have not verified whether it recurs across other PRs. It's possible the review agent is intentionally showing its work for transparency. However, if the agent's own analysis concludes something is fine, emitting it as a finding creates a false action signal — the reviewer reads a finding, expects to act on it, then discovers no action is needed.

Proposed change

In the review agent's skill guidance or sub-agent definitions in fullsend-ai/agents, add a suppression rule: if the agent's analysis of a potential finding concludes the current code is acceptable, appropriate, or follows established conventions, it should not emit that item as a finding. Only findings where the analysis concludes a change is warranted should appear in the findings list.

Concretely, this could be an instruction in the review skill or the correctness sub-agent prompt along the lines of: "Before emitting a finding, verify your analysis recommends a specific change. If you considered an issue but concluded the current code is correct or appropriate, do not include it. Suppress rather than self-dismiss."

Validation criteria

On the next 10 review runs in fullsend-ai/fullsend, count findings where the finding body contains language indicating the current code is acceptable (e.g., 'appropriate here', 'acceptable', 'fine as-is', 'reasonable choice'). The rate of such self-dismissed findings should decrease compared to the baseline (3 out of 6 on PR #6744's final review). Ideally, all emitted findings should recommend a specific actionable change.


Generated by retro agent from fullsend-ai/fullsend#6744

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions