Skip to content

fetch_pr_feedback cannot tell an unfinished review from a clean one #292

Description

@thecodedrift

fetch_pr_feedback.py has no notion of a review that is still running, so a caller can read a placeholder as a verdict and conclude a pull request is clean when nothing has been reviewed yet.

What the reviewer actually does

It posts a comment immediately and edits it in place as it works. Captured live on #290:

created: 2026-09-07T01:54:04Z   updated: 2026-09-07T01:57:32Z
---
### Review in progress <img src="..." />

Review mode: incremental — read 0 prior review thread(s) before reviewing.

- [x] Read `.prior-review.json`
- [x] Understand the request (`/review`)
- [x] Gather full diff via `gh pr diff`
- [x] Manual pass over core logic ...

So three things that look like completion signals are not:

  • the comment existing. It appears within seconds of the trigger.
  • created_at. It stamps the placeholder. An agent today read it as the finish time and reported a review that ran 1m49s as having completed in 14 seconds. Its conclusion happened to be right; the reasoning was not.
  • the job concluding success. Observed on feat: publish the shapes our --json output is produced from #267 earlier: the run reported success while the body still read "Review in progress" with unchecked boxes. That one produced a partial review that had to be re-triggered.

The only reliable signal is the body no longer opening with Review in progress.

Why it matters here

fetch_pr_feedback.py buckets comments into high/medium/low/bot/resolved. An in-progress placeholder is just a claude[bot] comment to it, so a caller that fetches feedback promptly after triggering a review gets either an empty result or a partial checklist, and needs_attention: 0.

That is the exact silent-success shape the iterate-pr skill exists to avoid: a run that reports "no findings" because it looked too early, indistinguishable from one that found nothing. Four agents were driven through this workflow today and each had to be told in its brief to poll for the review rather than trust the first fetch, which is guidance that belongs in the tool rather than in every caller.

Suggested shape

Have fetch_pr_feedback.py recognise an unfinished review and surface it, rather than bucketing it as feedback. Something like a review_in_progress count in summary, so a caller can tell "reviewed, nothing found" from "not reviewed yet" without pattern-matching prose itself.

The detection is a body test, since nothing else is dependable. Worth writing it as "does the body still open with the in-progress marker" rather than a substring search anywhere in the body, or a review that legitimately quotes the phrase would read as unfinished forever.

The skill's step 4 already says to wait for review bots before proceeding. This is the same instruction, moved somewhere it cannot be skipped.

Found while running /iterate-pr across five pull requests today.

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

    AI friendlyWell defined bugs suitable for a PR from an AgentbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions