pr_review.py reports a Copilot too-many-files refusal as a clean pass

[ptr727]

pr_review.py reports a Copilot refusal as a clean pass

pr_review.py status|wait treats a review carrying the head SHA as coverage. Copilot delivers its too-many-files refusal as exactly that: a formal review, state: COMMENTED, with the correct commit.oid, whose body is

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

and zero inline threads. So the digest reads:

pr=108 head=9f56a472 rounds=1 review_on_head=yes threads=0 unresolved=0 suppressed=0 answered_outside_review=no requested=no merge=CLEAN

That is byte-for-byte the shape of a genuine clean pass, and wait exits 0. Observed on a real pull request of 301 changed files, which was one file over the limit. The review never happened and the pull request was one command from merging on the strength of it.

Why the existing guard does not cover it

scripts/README.md reasons carefully about the adjacent case and exit 40: "a comment carries no commit, so it satisfies no coverage check whatever it says". That reasoning assumes a refusal arrives as a comment. This one arrives as a review, with a commit, so it satisfies every coverage check.

The 40 documentation also states the useful principle, that the test is the shape of the answer rather than its cause, and that a refusal is terminal because no review follows it and re-requesting does not clear it. All of that applies here: re-requesting is futile, because the file count has not changed. Only the delivery mechanism differs, and it is the mechanism the guard keys on.

It also interacts badly with the documented rule that "a review with no inline comments is still a completed review ... the clean-pass success case". That rule is correct and is exactly what makes this indistinguishable without reading the body.

Suggested handling

Treat a review whose body matches a refusal as a terminal error, not coverage. A distinct exit code alongside 40 would fit the existing vocabulary, and the digest should print the body whole, as 40 already does, because the wording is what separates a file-count refusal (fix by splitting the pull request) from a quota or rate-limit refusal (fix by waiting) from an ordinary empty review.

Matching on the body is unavoidable here, and the README's own guidance for the suppressed-findings matcher applies: match an alternation rather than one phrasing, and have a case assert the script's pattern is the one the runbook publishes rather than a copy that can drift.

Worth noting the reviewer-side fact for the docs too, since it is what an agent hitting this needs to know: the limit is 300 changed files, and the answer is to split the pull request rather than to retry. A repo committing binary data alongside code crosses it easily.

Second, smaller issue: --repo defaults to this repository

ap.add_argument('--repo', default='ptr727/ProjectTemplate'). A downstream run that omits it silently digests this repository's pull request of the same number and reports a state belonging to a different repo entirely. My first run reported threads=1 unresolved=0 while the repo I was actually working in had three unresolved threads, and nothing in the output indicated the mismatch.

Given the script's own standard, that a wrong answer is worse than a crash, defaulting to the current checkout's remote (or requiring the flag) would be safer than defaulting to a fixed repository.

[ptr727]

Fixed by #584, squashed to develop as e0c2a0a. Closing by hand, since a closing keyword only fires on a merge to the default branch and this landed on develop.

What changed

A refusal is no longer coverage. reviewed_head and the digest's review_on_head both exclude a review whose body opens by saying it did not review, so the state reports NO, wait exits 41, and the body prints whole, because the wording is the only thing separating a file-count refusal, cleared by splitting the pull request, from a quota one, cleared by waiting. The script reads neither cause, exactly as 40 reads none.

The digest this issue reported, rendered by the merged code against the same shape:

repo=ptr727/ProjectTemplate pr=108 head=9f56a472 rounds=1 review_on_head=NO refusal=YES threads=0 unresolved=0 suppressed=0 (on_head=0 earlier=0) answered_outside_review=no requested=no merge=CLEAN
  COPILOT REFUSED THIS ROUND: the review carrying the head says it did not review, so it covers nothing and re-requesting the same head repeats it, and the body below is what says which remedy applies
    Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

refusal= is a field of its own rather than left to inference, because rounds=1 review_on_head=NO is equally what a stale round looks like and the two want opposite responses, one a re-request and the other a split pull request. It is spent by coverage of the same head on both surfaces, the exit code and the field alike, or the line reads review_on_head=yes refusal=YES and tells a reader to split a pull request the reviewer has just reviewed. Copilot raised that one during the review of the fix.

The reading is head-scoped, unlike a suppressed finding, because a refusal is a statement about one commit that a push retires. The liveness query still carries no bodies, so a refusal reads there as ordinary coverage, which ends the wait, and the full read every wait finishes with is what tells the two apart. No exit code comes from the cheaper reading, so the per-poll cost is unchanged.

On the matching, which was the interesting part

Your suggestion to match an alternation and to pin it to the runbook was taken, and a case asserts the script's pattern is the published one rather than a copy that can drift. The alternation is wasn.t able to review|was not able to review|unable to review, where the dot spans both spellings of the apostrophe and also keeps the published --jq filter usable inside single quotes, which neither spelling survives.

The unit turned out to matter more than the pattern. Matching anywhere in the body reports the pull request that adds the check as a refusal of itself, which is the false positive the suppressed-block matcher already had once. Reading the opening two lines passed every case in the suite except that one, because a review's first line is its heading and its second is its overview prose, which is exactly where a review describing this check states the wording. The unit is one line. The cost runs the other way and is stated in scripts/README.md: a refusal introduced by a heading would sit below the opening and be missed.

Docs

The guarantee this defeats was stated on four surfaces, so each moved: the runbook's clean-pass rule and its coverage-confirmation paragraph, and GOVERNANCE.md merge-gate condition 2 and review-loop step 3, which now say the matched review is read rather than only counted. The runbook also records the reviewer-side fact you noted, that the limit is 300 changed files and the remedy is to split the pull request rather than to retry, since re-requesting cannot change the file count. GOVERNANCE.md is verbatim-carried, so a fleet re-vendor is owed for it.

The second issue

--repo defaulting to this repository was fixed in #575, before this issue was filed. It is now required and carries no default, the digest leads with repo=OWNER/NAME, and a value that is not OWNER/NAME is rejected by name. Nothing was needed here.