Skip to content

qodo-gate: require the first review, not a review of every head - #360

Merged
josephnef merged 2 commits into
masterfrom
qodo-gate-first-review
Aug 3, 2026
Merged

qodo-gate: require the first review, not a review of every head#360
josephnef merged 2 commits into
masterfrom
qodo-gate-first-review

Conversation

@josephnef

Copy link
Copy Markdown
Collaborator

Problem

The head-pinned gate rule makes every review-response push invalidate the review evidence: push a fix → summon /review → the bot re-scans the whole diff → it opens a fresh batch of findings → fix → push → repeat. On #359 this ran eleven rounds, converging on one-line style nits (float truncation vs ceil, max over key views) long after the substantive findings were exhausted. The re-review passes also re-anchor stale comment bodies and open re-litigations of already-dismissed findings, so the loop does not terminate on its own.

Change

The gate now requires:

  1. At least one Qodo review on the PR — the first, whole-diff pass, which is where the review value is concentrated (on ARQ end-to-end delivery bench: three-ledger harness; count spsc-fat pool-exhaustion drops #359: a SIGFPE, a data race, a missing file dependency, a host-wide pkill — all from the first two passes).
  2. All Qodo-authored threads resolved — unchanged. Every finding still must be addressed or explicitly dismissed with rationale in its thread before resolution; the accountability mechanism stays.

Dropped: the head-oid match and the update-in-place marker-comment fallback (and the issues: read permission only that fallback needed). Follow-up commits after the review are maintainer judgment — the same contract as a human reviewer who does not re-review every fixup. Thread pagination, any-comment bot attribution, and the skip-qodo-gate escape hatch are unchanged.

Validation

The workflow parses (same trigger set as the current file, which is the parser-accepted form from #357), and this PR itself runs under the new logic via the merge ref: its gate must see Qodo's first review of this PR plus resolved threads to go green — a live demonstration of exactly the intended flow.

🤖 Generated with Claude Code

The head-pinned rule turned every review-response push into a fresh summon,
and every summoned pass re-scans the diff and opens a new batch of
ever-smaller findings — an unbounded fix/re-review treadmill (eleven rounds
on a single PR, converging on one-line style nits). The first, whole-diff
review is where the value is; thread resolution already keeps each finding
accountable (address or dismiss with rationale in the thread); follow-up
commits are maintainer judgment, as with a human reviewer who does not
re-review every fixup.

The gate now requires (a) at least one Qodo review on the PR and (b) all
Qodo-authored threads resolved. The head-oid match and the update-in-place
marker-comment fallback are gone — and with the fallback goes the
`issues: read` permission it needed. Thread accounting (pagination,
any-comment bot attribution) and the skip-qodo-gate escape hatch are
unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

qodo-gate: require at least one Qodo PR review (not head-pinned) + resolved threads

⚙️ Configuration changes ✨ Enhancement 🕐 10-20 Minutes

Grey Divider

AI Description

• Change Qodo gate to require any Qodo review on the PR (not current head).
• Keep requirement that all Qodo-authored review threads are resolved.
• Remove marker-comment fallback and drop unneeded issues: read permission.
Diagram

graph TD
  A([PR / review / thread events]) --> B["qodo-gate.yml"] --> C["GraphQL: PR reviews + labels"] --> D{Has Qodo review?} --> E{All Qodo threads resolved?} --> F([PASS])
  E --> G([FAIL])
  D --> G
  subgraph Legend
    direction LR
    _evt([Trigger/Event]) ~~~ _wf["Workflow"] ~~~ _dec{Decision} ~~~ _out([Outcome])
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep head-pinned review requirement but add an explicit re-review cap
  • ➕ Maintains strict 'latest commit reviewed' invariant
  • ➕ Prevents unbounded review loops by policy
  • ➖ Still incentivizes repeated bot re-scans for small fixups
  • ➖ Hard to choose a cap that fits all PR sizes and risk profiles
  • ➖ More complicated UX and enforcement
2. Require first review + opt-in re-review via label/comment (manual escalation)
  • ➕ Preserves current PR’s simplified default path
  • ➕ Allows teams to demand re-review when changes are substantial
  • ➖ Adds another control surface and process to document
  • ➖ Still needs clear rules for when escalation is required
3. Use a timestamp-based freshness check (submittedAt vs last push time)
  • ➕ Approximates 'review is recent enough' without head OID matching
  • ➖ Can be spoofed/invalidated by rebases and commit metadata quirks
  • ➖ More edge cases than a simple 'at least one review exists' rule

Recommendation: The PR’s approach (require at least one Qodo review on the PR + all Qodo threads resolved) is the best default for a merge gate: it preserves accountability via thread resolution while eliminating the head-pinned treadmill and enabling permission reduction (dropping issues: read). If additional strictness is needed for high-risk repositories, consider an opt-in “force re-review” escalation mechanism rather than making re-review mandatory on every push.

Files changed (1) +26 / -50

Other (1) +26 / -50
qodo-gate.ymlRelax Qodo gate from head-pinned review to first-review presence +26/-50

Relax Qodo gate from head-pinned review to first-review presence

• Updates the gate logic to accept any Qodo-authored PR review (removing head OID matching) while keeping the unresolved Qodo-thread requirement. Deletes the marker-comment fallback path and removes the now-unnecessary 'issues: read' permission. Adjusts failure/success messaging and expands rationale in comments to document the policy shift.

.github/workflows/qodo-gate.yml

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 3, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Unpaginated review lookup ✓ Resolved 🐞 Bug ☼ Reliability
Description
The gate now accepts any Qodo review on the PR, but it only inspects the last 100 reviews; if Qodo’s
review falls outside that window, the job incorrectly fails with “no Qodo review”. This can block
merges on PRs with heavy review churn even though the required Qodo review exists.
Code

.github/workflows/qodo-gate.yml[R79-82]

+          reviewed=$(echo "$json" | jq --arg b "$BOT" \
          '[.data.repository.pullRequest.reviews.nodes[]
-              | select(.author.login == $b)
-              | select(.commit.oid == $oid)] | length')
-          if [ "$current" -eq 0 ]; then
-            # --paginate applies --jq PER PAGE, so a `| length` there emits
-            # one count per page ("0\n1"), breaking the integer test below.
-            # Emit matching comment ids instead and count lines across pages.
-            current=$(gh api "repos/$REPO_OWNER/$REPO_NAME/issues/$PR/comments" \
-              --paginate --jq ".[]
-                | select(.user.login == \"${BOT}[bot]\")
-                | select(.body | test(\"up to the latest commit\"))
-                | select(.body | contains(\"$head_oid\"))
-                | .id" | wc -l)
-          fi
-          if [ "$current" -eq 0 ]; then
-            echo "FAIL: no Qodo review of the current head ($head_oid) —"
-            echo "comment /review on the PR to summon one, then re-run this"
-            echo "check (or reply in a thread) once it answers. (Outage?"
+              | select(.author.login == $b)] | length')
+          if [ "$reviewed" -eq 0 ]; then
Evidence
The workflow fetches only reviews(last: 100) and then counts Qodo-authored reviews from that
limited node set, so any Qodo review outside the last 100 will not be seen. In contrast, the same
workflow explicitly paginates review threads to avoid truncated reads, showing the author already
considers pagination necessary for correctness in similar data sources.

.github/workflows/qodo-gate.yml[55-67]
.github/workflows/qodo-gate.yml[76-88]
.github/workflows/qodo-gate.yml[90-126]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The workflow determines whether Qodo has reviewed the PR by counting matches in `pullRequest.reviews.nodes`, but the GraphQL query requests only `reviews(last: 100)`. With the new semantics (“any Qodo review counts”), Qodo’s review can be older than the most recent 100 reviews, causing a false failure.
### Issue Context
The same workflow already paginates `reviewThreads` specifically to avoid truncated reads producing incorrect results; `reviews` should be treated similarly now that the check must be able to find older bot reviews.
### Fix Focus Areas
- .github/workflows/qodo-gate.yml[55-67]
- .github/workflows/qodo-gate.yml[76-88]
### Suggested fix approach
Paginate the `reviews` connection (GraphQL `pageInfo{hasNextPage,endCursor}` + a cursor loop like the `reviewThreads` loop) and stop early once a matching Qodo author is found, or use a REST endpoint for PR reviews with `--paginate` and robust counting.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread .github/workflows/qodo-gate.yml Outdated
Every inline reply wraps itself in a review object, so a busy PR exceeds 100
of them — and the bot's first review is the oldest, exactly what a last-100
window loses first. REST + --paginate, ids counted per line (per-page --jq),
prefix-matched login (REST adds the [bot] suffix GraphQL omits).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@josephnef
josephnef merged commit a4ce3ba into master Aug 3, 2026
22 checks passed
@josephnef
josephnef deleted the qodo-gate-first-review branch August 3, 2026 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant