Skip to content

pr-review-conduct: extend 'success check can't hide actionable findings' to CodeRabbit outside-diff-range comments and Qodo comment-only findings #1058

Description

@ptr727

Motivation

From the personal PR-reviewer evaluation's 2026-08-28 pass (kept outside this repo). Confirmed,
not hypothetical: pieter reads these findings himself, but the coding agents doing the actual PR
work routinely don't.

pr_review.py's Copilot support already has dedicated logic so an agent can't silently miss
Copilot's easy-to-miss surface — its collapsed "Suppressed comments" section never becomes a
blocking thread even when it repeats a real issue across several rounds, and pr_review.py
already reads for it. CodeRabbit and Qodo have their own equivalent gaps, and neither has the
same protection yet:

  • CodeRabbit's "outside diff range" comments don't create resolvable reviewThreads. This
    pass, two different repos were bitten by it: HomeAutomation-Config Disable auto-merge on maintainer push to bot PR #79's decisive round-4/5
    lock.sh findings (a real symlink/TOCTOU lock-file vulnerability) and Financial-Modeling Harden merge gate + prerelease guard, and de-prose comments (#188, #189) #190's
    payslip-conflict and insurance-double-count findings all landed as this comment type — correct
    findings that never became inline threads a pr_review.py-style coverage check, or an agent
    scanning reviewThreads, would ever see.
  • Qodo's findings live entirely in separate PR-level comments, not its formal review. Its
    review object body is always empty across every PR checked (confirmed on dozens of PRs this
    pass); the actual content is two comments ("PR Summary by Qodo" and "Code Review by Qodo," the
    latter carrying the numbered findings, each with its own ✓ Resolved/✗ Dismissed
    self-tracking badge). A reader that only checks reviews[].body sees nothing from Qodo, ever.

Proposal

Extend pr-review-conduct's existing "a success check cannot hide actionable findings" principle
explicitly to cover both of these, and give pr_review.py (or a new adapter) the equivalent
of its Copilot suppressed-comments handling:

  • Treat CodeRabbit's "outside diff range" PR comments as first-class findings requiring
    triage/disposition, not just its reviewThreads.
  • Read Qodo's "Code Review by Qodo" comment (not its review body) as the actual finding source,
    and consider its ✓ Resolved/✗ Dismissed badges as a fast pre-triage signal (spot-verify
    against gh pr diff rather than trusting outright, per the evaluation doc's own caveat).

Status: intentionally filed now, ahead of graduation

This is filed now as forward-looking work rather than waiting, per pieter's explicit go-ahead --
even though CodeRabbit/Qodo haven't yet met the First-Class Support Criteria that would make
either a required (non-advisory) reviewer. The underlying gap (agents missing real findings that
live outside resolvable threads) is real today regardless of that decision, since both are
already active advisory reviewers across most of the fleet's repos.

Related: #1056 (local-strict-review
skill proposal, where this was first flagged as related follow-on work).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions