Skip to content

Review correctness sub-agent should escalate CI coverage regressions to HIGH when they remove merge-gate signal #156

Description

@fullsend-ai-retro

What happened

On PR #145, the fullsend review agent's correctness sub-agent identified that the new select-eval-agents.sh does not select any agent for changes to shared eval infrastructure (eval/scripts/, .github/scripts/, config.yaml), whereas the old grep-based filter matched these paths and ran tests unconditionally. The agent rated this finding as medium severity.

A human reviewer (waynesun09's review squad) identified the same finding and rated it HIGH, noting that the coverage gap means infrastructure regressions are only caught after merge — i.e., the merge-queue gate loses signal for this class of changes.

The severity gap between medium and HIGH directly affected the verdict: with 3 medium findings, the agent chose comment (non-blocking). Per the agent's own criteria, a HIGH finding would have triggered request-changes (blocking). This is a recurrence of the pattern in issue #45.

What could go better

The correctness sub-agent should distinguish between coverage gaps that affect development convenience (medium) and coverage gaps that remove merge-gate signal (HIGH). When a change causes a class of PRs to bypass CI checks that previously ran at the merge-queue gate, the regression is HIGH because it creates a path for undetected regressions to land on main.

Confidence: High. The severity difference between the agent and human reviewer is clear and directly impacted the verdict. The agent's own verdict criteria state that HIGH findings trigger request-changes, and the human reviewer's HIGH rating was well-reasoned.

This is distinct from issue #45 (which focuses on verdict logic when medium findings are numerous). Here, the root cause is severity calibration: correctly rating this as HIGH would have produced the correct verdict without any change to verdict logic.

Proposed change

Add guidance to the correctness sub-agent prompt (skills/pr-review/sub-agents/correctness.md) in a new subsection under the existing coverage-related checks:

### CI coverage regression severity

When a change reduces which file paths trigger CI checks (e.g.,
replacing a broad path filter with a narrower selection mechanism),
evaluate the merge-gate impact:

- **HIGH** if the regression removes merge-queue or PR-gate signal for
  a class of changes that previously received it. Changes to shared
  infrastructure (CI scripts, test runners, config files) landing
  without CI signal is a HIGH-severity coverage regression.
- **Medium** if the regression only affects optional or advisory checks
  that do not gate merging.

The key question: "Can a PR touching these paths now land on main
without the CI checks that previously guarded them?" If yes, rate HIGH.

This calibration guidance addresses the root cause of the recurring verdict miscalibration in issue #45 for this class of finding.

Validation criteria

On the next PR that modifies CI path filters or test selection logic, the correctness sub-agent should rate coverage regressions affecting merge gates as HIGH (not medium). Verify by comparing the finding severity in the review comment against the merge-gate impact assessment. The next 3 PRs modifying CI workflow path filters should produce correctly calibrated severity ratings.


Generated by retro agent from #145

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions