Read a Copilot refusal as a refusal, not as coverage by ptr727 · Pull Request #584 · ptr727/ProjectTemplate

ptr727 · 2026-08-06T12:59:01Z

Fixes #581.

Copilot answers a pull request it will not take on with a formal review, state: COMMENTED, carrying the correct commit.oid and zero inline threads, whose whole body is a refusal. pr_review.py treated a review carrying the head as coverage, so the digest read

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

which is the clean pass byte for byte, and wait exited 0. Observed on a pull request of 301 changed files, one over the reviewer's limit of 300, which was one command from merging on a round that never ran.

The existing guard does not reach it. Exit 40 rests on a comment carrying no commit, so it satisfies no coverage check whatever it says. This carries the right commit. Only the delivery mechanism differs, and the mechanism is what the guard keys on.

The reading

A refusal is not coverage. reviewed_head and the digest's review_on_head both exclude it, so the state reports NO, and the summary line carries a refusal= field of its own, because rounds=1 review_on_head=NO is equally what a stale round looks like and the two want opposite responses. wait exits 41 and the body prints whole, since 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.

The same payload, rendered by the code in this branch:

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.

The reading is head-scoped, unlike a suppressed finding: a refusal is a statement about one commit that a push retires, and a genuine review of that same head outranks it. 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.

The unit is the body's opening line

Matching anywhere in the body reports this pull request as a refusal of itself, the same false positive the suppressed matcher had once. Two lines is not enough either: a review's first line is its heading and its second is the overview prose, which is exactly where a review describing this check states the wording. A case pins that, and it is the case that failed while every other one passed. The cost runs the other way, that a refusal introduced by a heading would sit below the opening and be missed, and answering that shape means telling a refusal from an overview rather than reading one line further.

The alternation is the runbook's, and a case asserts the script's pattern is the published one rather than a copy that can drift. The dot spans both spellings of the apostrophe, the typographic one Copilot writes and the ASCII one, and it also keeps the published --jq filter usable inside single quotes, which neither spelling survives.

Docs

Surface	Change
`.github/copilot-instructions.md`	The clean-pass rule gets its one exception, with the observed body, the published filter, and the reviewer-side fact an agent hitting this needs: the limit is 300 changed files and the remedy is to split the pull request. The coverage-confirmation paragraph now says to read the body of the review the SHA matched, not only the SHA.
`GOVERNANCE.md`	Merge-gate condition 2 and review-loop step 3 stated the same guarantee for the fleet, so both now say the matched review is read rather than only counted. Fleet-carried, so a re-vendor is owed.
`scripts/README.md`	The `41` paragraph, alongside `30`, `40` and `50`.

Checks

python3 -m unittest discover -s scripts (267 cases), scripts/prose_lint.py . --diff origin/develop, scripts/repo_gate.py, markdownlint and editorconfig-checker over the changed files, all clean. The published --jq filter was run over a crafted set of five review bodies and selected the two refusals while skipping the quotation, the clean pass, and the null body.

Not in scope

The issue's second point, --repo defaulting to this repository, was fixed in #575 and needs nothing here.

🤖 Generated with Claude Code

Copilot answers a pull request it will not take on with a formal review, `state: COMMENTED`, carrying the correct `commit.oid` and zero inline threads, whose whole body is a refusal. `pr_review.py` treated a review carrying the head as coverage, so the digest read pr=108 head=9f56a472 rounds=1 review_on_head=yes threads=0 unresolved=0 suppressed=0 answered_outside_review=no requested=no merge=CLEAN which is the clean pass byte for byte, and `wait` exited 0. Observed on a pull request of 301 changed files, one over the reviewer's limit of 300, which was one command from merging on a round that never ran. The existing guard does not reach it. Exit 40 rests on a comment carrying no commit, so it satisfies no coverage check whatever it says. This carries the right commit. Only the delivery mechanism differs, and the mechanism is what the guard keys on. ## The reading A refusal is not coverage. `reviewed_head` and the digest's `review_on_head` both exclude it, so the state reports as `NO`, and the summary line carries a `refusal=` field of its own, because `rounds=1 review_on_head=NO` is equally what a stale round looks like and the two want opposite responses. `wait` exits 41 and the body prints whole, since 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. Head-scoped, unlike a suppressed finding: a refusal is a statement about one commit that a push retires, and a genuine review of the same head outranks it. 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. ## The unit is the body's opening line Matching anywhere in the body reports this pull request as a refusal of itself, the same false positive the suppressed matcher had once. Two lines is not enough either: a review's first line is its heading and its second is the overview prose, which is exactly where a review describing this check states the wording, and a case pins that. The cost runs the other way, that a refusal introduced by a heading would be missed. The alternation is the runbook's, and a case asserts the script's pattern is the published one rather than a copy that can drift. The dot spans both spellings of the apostrophe and keeps the published `--jq` filter usable inside single quotes, which neither spelling survives. ## Docs The runbook's clean-pass rule and its coverage-confirmation paragraph both stated the guarantee this defeats, and `GOVERNANCE.md` merge-gate condition 2 and review-loop step 3 state it for the fleet. Each now says the matched review is read rather than only counted. The runbook also records the reviewer-side fact an agent hitting this needs: the limit is 300 changed files and the remedy is to split the pull request. The issue's second point, `--repo` defaulting to this repository, was fixed in #575 and needs nothing here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

digest() can still report refusal=YES (and print the refusal body) even when a later non-refusal Copilot review on the same head provides valid coverage, which contradicts the intended “genuine review outranks refusal” precedence and can mislead consumers.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR hardens the scripts/pr_review.py review-coverage logic to treat a Copilot “unable to review” response (delivered as a formal review on the head SHA with zero threads) as a refusal rather than as successful coverage, and documents the nuance in the runbook and governance text.

Changes:

Add refusal detection based on the opening non-empty line of the review body, and surface it in the digest (refusal=YES) plus a new wait exit code (41).
Extend the unit tests to cover refusal phrasing variants (including typographic apostrophes) and to assert the regex matches the runbook alternation.
Update documentation (runbook, governance, scripts README) to describe refusal handling, matching strategy, and the 300-file limit behavior.

File summaries

File	Description
scripts/pr_review.py	Implements refusal detection, updates digest output, and introduces `wait` exit `41` for refusal-on-head.
scripts/test_pr_review.py	Adds a `TestRefusal` suite and CLI tests for exit `41`, plus runbook-pattern pinning.
scripts/README.md	Documents exit `41`, refusal semantics, and the opening-line matching rationale.
GOVERNANCE.md	Updates merge-gate and loop steps to require reading the matched review body to detect refusals.
.github/copilot-instructions.md	Documents the refusal exception, includes the published `--jq` filter, and updates coverage confirmation guidance.

Review details

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

`digest()` read `refusing_review` unconditionally, so a refusal plus a later genuine review of the same head rendered `review_on_head=yes` and `refusal=YES` on one line, with the refusal body printed under it. That contradicts the precedence the rest of the change holds, that coverage which landed spends the refusal, which `main` already honors by returning 0 on `reviewed_head` before it can reach 41. The reading a reader takes from those two fields together is to split a pull request the reviewer has just reviewed, and an automated one takes `refusal=YES` as outstanding. The field is now reported only where nothing covers the head, so the digest and the exit code spend it alike. A case holds the pair, digest and exit code, over one payload carrying both rounds. `refusing_review`'s docstring said `reviewed_head` settles this ahead of it, which was true of one caller and not of the digest, so it now names both. Raised by Copilot on #584. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟢 Ready to approve

The refusal-vs-coverage behavior is implemented consistently across the script, tests, and docs, with only a minor comment-clarity nit noted.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

scripts/pr_review.py:42

The comment above REFUSAL is hard to parse ("every coverage check it fails to meet is met" reads self-contradictory). Rephrasing would make the intent clearer without changing behavior.

# A refusal declines the round as a formal review carrying the head and no threads.
# That is the clean pass byte for byte, so every coverage check it fails to meet is met.
# The alternation is the runbook's for the same reason the one above is.
# One phrasing is one rewording away from reading a refusal as a review.
# The dot covers the apostrophe in the typographic spelling and the ASCII one alike.
# It also keeps the published filter usable inside single quotes, which neither survives.

Files reviewed: 5/5 changed files
Comments generated: 0 new
Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

"Every coverage check it fails to meet is met" was written to mean that the checks a refusal ought to fail all pass over it. Read plainly it is self-contradictory, which is the opposite of a comment's job on the one line that explains why this whole check exists. It now states the consequence directly: every coverage check passes over a round that never ran. No behavior changes. Raised as a low-confidence finding by Copilot on #584. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

ptr727 · 2026-08-06T13:28:54Z

Answering the suppressed finding from round 2

That review's Suppressed comments (1) block carries one finding, and this answers it. It reached no thread, so there is nothing to reply on or resolve.

scripts/pr_review.py:42
The comment above REFUSAL is hard to parse ("every coverage check it fails to meet is met" reads self-contradictory). Rephrasing would make the intent clearer without changing behavior.

Fixed in 421c6e8. It is self-contradictory read plainly. The sentence was written to mean that the checks a refusal ought to fail all pass over it, which is the entire reason this check exists, so an unreadable line there is worse than none. It now states the consequence directly:

# That is the clean pass byte for byte, so every coverage check passes over a round that never ran.

No behavior change, and the suite, prose_lint --diff, repo_gate and editorconfig-checker are clean on it.

1 of 1 answered.

Copilot

🟢 Ready to approve

The refusal detection is narrowly scoped, well-covered by new tests (including precedence and wait-loop behavior), and documentation is updated consistently across the relevant runbook/governance surfaces.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Files reviewed: 5/5 changed files
Comments generated: 0 new
Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

…, and the operational pull-request trigger (#589) Promotes the five commits develop carried since #582: #584 (a Copilot refusal is not coverage), #586 (the reference-definition collation), #587 (CI on a pull request into an operational develop), #588 (hub-hosted tooling, de-vendoring configure.sh), and #590 (the two definition blocks #588 pushed out of order). Merge commit, no squash, develop not deleted. git merge-tree reported no conflict and wrote a tree byte-identical to origin/develop. GOVERNANCE.md is verbatim-carried and three of these commits change it, so a fleet re-vendor is owed, including an entirely new carried section. #588 also turns every downstream repo-config/configure.sh copy into a deletion rather than a convergence, and #587 leaves a one-line trigger change owed by four operational repos.

`mergeStateStatus` reports `BLOCKED` for a failed check, a required check nothing is running, an unresolved thread, and a missing approval alike, and the digest printed that word and stopped there. Found by running into it. A run polled `BLOCKED` for twenty-five minutes on a pull request whose only unfinished check was an aggregator job GitHub dispatched and never assigned a runner, and the cause came from the maintainer rather than from any field in the digest. That is the defect: the digest named a state it could not explain. This is the refusal defect of #584 one gate along. There, a review that covered nothing rendered as coverage. Here, a check nothing is running renders as patience, and both read as a pull request worth waiting on. ## What the digest says now Reconstructed from the observed job states of the run this hit, rather than described: ``` repo=ptr727/ProjectTemplate pr=590 head=aaaaaaaa rounds=1 review_on_head=yes refusal=no threads=0 unresolved=0 suppressed=0 (on_head=0 earlier=0) answered_outside_review=no requested=no merge=BLOCKED checks=2/3 stuck=NOT_PICKED_UP CHECK NOT PICKED UP ('Check pull request workflow status job', queued 15m with no runner assigned): nothing here starts it, because the runner pool is GitHub-hosted, so re-run the workflow or wait on that capacity ``` The rollup rides the existing full query, so reading the checks costs no extra round-trip, which is the reason this script exists at all. ## Three shapes, because each wants the opposite response - **`NOT_PICKED_UP`** is a job GitHub dispatched and assigned no runner. Read from the queued state rather than from a runner name, which GraphQL does not carry, and the state suffices because a job held behind a `needs:` dependency does not enter the rollup until that dependency finishes, so there is no dependency-blocked queue to mistake for a starved one. Nothing agent-side starts it, since the pool is hosted, so the remedy is a re-run or that capacity, and **not** a re-request, a rebase, or an empty commit. - **`RUNNING_LONG`** is deliberately the weaker reading and its wording says so, since duration alone cannot separate a hung job from a slow one. This repository's lint job legitimately runs nine to eleven minutes while its aggregator is a single shell conditional, so the threshold is generous, the elapsed time prints for the reader to judge against what the job costs, and nothing asserts a fault. - **`FAILED`** is a verdict rather than a stuck check, reported so no reader deduces a red check from `BLOCKED`. ## Exit 44, and the boundary that makes it mean something `wait` gains exit `44` for a review loop that closed against a check in one of those shapes, because `0` was saying the loop closing is the merge gate. A check **merely still running normally is not 44 and exits 0.** That boundary is the whole design: the wait returns the moment coverage lands, which on almost every pull request is mid-CI, so taking `42` for a pending check would make `42` the ordinary outcome, and a code that fires always carries nothing. The grace is the pickup grace's five minutes for the reason that one is, and the stall is thirty because a fleet repository building and testing .NET runs longer than this one. Both are flags, and a negative value for either is rejected by name rather than rendering a digest that reports every check stuck from its first read. ## Two defects this caught in itself - A finished check carrying **no conclusion yet** fell through to the unknown-conclusion branch and reported `FAILED`, inventing a red check out of a race in the API. Caught by the suite, which is also what said the first test helper was at fault rather than the code, since it failed eight cases at once. - **`EXPECTED`** was missing from the unstarted set, so a required status nobody has posted reached that same branch and reported that same false failure. Caught reading the diff back. Both have cases now. `SKIPPED` and `NEUTRAL` count as passes, since the fleet aggregator pattern skips the conditional jobs and four of the six checks on a green pull request here are skips. An unrecognized conclusion **is** reported, because a new enum member read as a pass is a red check rendering as a green digest. ## Docs The runbook's `BLOCKED` bullet said the cause is "usually" unresolved review threads, which is true and is exactly what misled the run. The qualifier stays and the counter-case joins it rather than replacing it. `GOVERNANCE.md` merge-gate condition 1 now requires the reason to be **read** rather than inferred, and a new paragraph states that `BLOCKED` is no more self-explaining than `CLEAN` is sufficient, including that hosted-runner capacity is not a reason to weaken a gate. `GOVERNANCE.md` is verbatim-carried, so a **fleet re-vendor is owed**, and the `TODO.md` re-vendor entry now names these sections. ## Verification 461 cases pass across the three `scripts/` suites, 217 of them in `test_pr_review.py`, which is 41 more than `develop` carries. The count read 291 when this was written and the suites have grown on both sides since, so it is restated against the merge rather than left as a number measured on a tree that no longer exists. `prose_lint.py --diff origin/develop`, `repo_gate.py`, `spec/validate.py`, and `markdownlint-cli2` over all 44 files are clean. A grep for restatements of the changed merge-gate wording across every markdown file found none stale, and the "all four preconditions" sentence still holds since condition 1 was amended rather than added to. The digest was also run live against a real pull request, where it reports `checks=6/6` and no stuck field. ## Renumbered from 42 to 44 on the forward-merge This branch was parked on 2026-08-06 pending the GitHub Actions outage, and `develop` took exit `42` in the meantime for a round that read fewer files than the pull request changed, with `43` for a shape the reader does not know, both from #607, #608 and #613. The check reading here is `44`, ranked under both: `43` says no field can be believed, `42` says part of the diff has no review, and only once those are clear is a wedged required check the thing worth reporting. `wait` returns `report_verdict`'s code where it has one and reads the checks only where it does not.

Copilot AI lite review requested due to automatic review settings August 6, 2026 12:59

Copilot started reviewing on behalf of ptr727 August 6, 2026 12:59 View session

Copilot AI reviewed Aug 6, 2026

View reviewed changes

Comment thread scripts/pr_review.py Outdated

Copilot AI review requested due to automatic review settings August 6, 2026 13:22

Copilot started reviewing on behalf of ptr727 August 6, 2026 13:22 View session

Copilot AI previously approved these changes Aug 6, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 6, 2026 13:28

Copilot started reviewing on behalf of ptr727 August 6, 2026 13:29 View session

Copilot AI dismissed their stale review, a newer Copilot review was requested August 6, 2026 13:30

Copilot AI approved these changes Aug 6, 2026

View reviewed changes

ptr727 merged commit e0c2a0a into develop Aug 6, 2026
7 checks passed

ptr727 deleted the feature/review-refusal-is-not-coverage branch August 6, 2026 13:34

Navigation Menu

Read a Copilot refusal as a refusal, not as coverage - #584

Read a Copilot refusal as a refusal, not as coverage#584
ptr727 merged 3 commits into
developfrom
feature/review-refusal-is-not-coverage

ptr727 commented Aug 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

ptr727 commented Aug 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ptr727 commented Aug 6, 2026

The reading

The unit is the body's opening line

Docs

Checks

Not in scope

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Ready to approve

Suppressed comments (1)

Uh oh!

ptr727 commented Aug 6, 2026

Answering the suppressed finding from round 2

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Ready to approve

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants