Skip to content

Review agent should surface confirmatory reasoning in APPROVE comments, not just "Looks good to me" #280

Description

@fullsend-ai-retro

What happened

On PR #5245 (one-line bug fix + regression test), the review agent ran 3 sub-agents (correctness, style-conventions, intent-coherence). All returned zero findings. The review agent posted "Looks good to me" with no further commentary and submitted an APPROVE.

The human reviewer (waynesun09), reviewing the same 2-file diff, provided five specific confirmatory verifications:

  1. Fix matches the exact error string from the linked issue
  2. Grepped all other openshell call sites — no other instances of the same bug
  3. Traced pre-fix behavior to confirm the test is genuine (not trivially passing)
  4. Confirmed no injection surface (argv form, hardcoded literal)
  5. Verified test isolation (no t.Parallel, t.Setenv auto-restores, shell check targets only import invocation)

The review agent's correctness sub-agent performed similar verification internally (read sandbox.go, read sandbox_test.go, grepped for ImportProfile patterns) but none of this reasoning appeared in the posted comment. The human reviewer had to independently redo all of this work.

What could go better

When the review agent approves with zero above-threshold findings, the posted comment should include a brief summary of the key verifications the sub-agents performed. This serves two purposes:

  1. Reduces human reviewer effort. The human can see that specific checks were already done (e.g., "no other call sites affected", "test exercises the pre-fix failure path") and focus their time on what the agent couldn't check.
  2. Builds trust in agent reviews. An APPROVE with reasoning is more trustworthy than a bare "Looks good to me", making autonomy escalation easier to justify.

This is related to fullsend-ai/fullsend#2609 ("Surface low-severity findings in APPROVE summary comments") but distinct: #2609 is about findings that fall below the severity threshold, while this proposal is about surfacing the positive verification steps the sub-agents performed even when they found nothing wrong.

Uncertainty: The review agent may already sometimes include reasoning — this observation is from a single PR. The pattern should be validated across 5-10 recent APPROVE reviews to confirm it's systemic.

Proposed change

Modify the review agent's synthesis logic in the pr-review skill (or the review orchestrator agent definition at agents/review.md) to include a "Verification summary" section in APPROVE comments when all sub-agents return clean.

Concretely, each sub-agent's prompt should instruct it to return not just findings but also a brief list of key verifications performed (2-3 bullet points). The orchestrator's synthesis step should then format these as a collapsed details section in the APPROVE comment, e.g.:

Looks good to me

<details><summary>Verification summary</summary>

- **Correctness:** Verified fix adds `--file` flag matching the sibling `ImportProfiles` pattern. Grepped all `openshell` call sites — no other instances of bare positional args for profile paths.
- **Style:** Commit message follows conventional-commits format. Test follows existing fake-openshell pattern.
- **Intent:** Fix directly addresses the error string in #5241. Test exercises the pre-fix failure path.
</details>

The sub-agent schema (if using structured output) would need a new optional verifications field (array of strings). The orchestrator's synthesis prompt would need to format these into the comment.

Validation criteria

Sample 10 APPROVE reviews after the change ships. At least 8 of 10 should include a verification summary with 2+ specific checks per sub-agent dimension. Measure human reviewer feedback — do reviews that include verification summaries result in faster human approvals (compare median time-to-human-approval before and after, across 20+ PRs)?


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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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