Skip to content

Review agent should not flag scope-creep against ADR phase boundaries for human-authored PRs #2801

Description

@fullsend-ai-retro

What happened

On PR #2762, the review agent posted a [medium] inline comment on fix.yaml:56 flagging scope-creep: it argued that hardcoded config constants (MAX_RETRIES, TIMEOUT_SECONDS, etc.) being moved to env.sandbox weren't authorized by ADR 0055 Phase 2, which mentions 'simple passthrough vars.' The commit message explicitly stated: 'Move simple passthrough vars and hardcoded config values from fix-agent.env.' The human author (ralphbean) ignored the finding and merged the PR. The final review summary downgraded the finding to Low severity, creating an inconsistency with the Medium inline comment.

What could go better

The review agent treated ADR phase descriptions as strict scoping contracts rather than rough implementation plans. ADR phases describe planned increments — they are not hard constraints on what a PR may touch. When a human author explicitly describes their intent in the commit message, deviations from ADR phase boundaries are intentional design decisions, not scope-creep.

This is a low-impact issue — the finding didn't block the merge and was easy to ignore. However, it represents a class of false positives where the review agent applies normative documents too literally. Existing issues #2655 (suppress self-contradicting findings) and #2200 (cap severity for human PRs with descriptive bodies) are related but don't cover the specific pattern of over-literal ADR phase interpretation.

Confidence: Medium. This is based on a single instance. The pattern may not recur frequently enough to warrant dedicated handling.

Proposed change

In the review agent's scope-creep detection logic (likely in the intent-coherence sub-agent or similar), add guidance that ADR phase descriptions are non-binding implementation plans. Specifically:

  1. When the review agent detects a change that extends beyond what an ADR phase explicitly lists, it should check whether the commit message or PR description explicitly describes the broader scope.
  2. If the author has explicitly described the deviation, the finding should be suppressed or capped at info severity rather than medium.
  3. For human-authored PRs (not agent-authored), scope-creep findings based solely on ADR phase boundary interpretation should be capped at low severity.

This change would belong in the review agent's sub-agent definitions or the scope-creep detection prompt, likely under agents/ or skills/ in the fullsend repo.

Validation criteria

On the next 5 human-authored PRs that reference an ADR and intentionally extend beyond a phase boundary (as described in their commit message or PR body), the review agent should either not flag scope-creep or flag it at info/low severity. No medium or higher scope-creep findings should be emitted when the PR author explicitly describes the broader scope in the commit message.


Generated by retro agent from #2762

Metadata

Metadata

Assignees

No one assigned

    Labels

    ready-for-triageRetro-filed issue awaiting triage agent

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    Status
    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions