pr_review.py: a PARTIAL coverage verdict can contradict the review's own file table

[ptr727]

pr_review.py status reads coverage from the reviewer's Copilot reviewed N out of M changed files line and reports coverage=PARTIAL when N < M, with a caveat block saying part of the diff has no review at all and that the merge decision belongs to the maintainer. That is the right default. In at least one case the same review body contradicts that line, and the script has no way to notice.

Observed on ptr727/Blog#60, round 1, and on its predecessor ptr727/Blog#59 across six consecutive rounds at an identical 5-of-6 ratio.

The header says:

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

The <details> "Pull request overview" table in the same review body then names all six changed files with a one-line description each. Measured rather than eyeballed — the pull request's changed-file list and the paths named in that table are identical sets:

changed files          table names
TODO.md                TODO.md
checks/README.md       checks/README.md
checks/build-redirects.py   checks/build-redirects.py
checks/check-url-parity.py  checks/check-url-parity.py
deploy/maps/slugs.map  deploy/maps/slugs.map
hugo.yaml              hugo.yaml

So on this evidence the 5 out of 6 count is a reporting artifact rather than a file that went unread, and the caveat's "files in the diff have no review at all" is stronger than what can be shown. That matters because the caveat is written to stop a merge, and the runbook's own note that a re-request never clears a partial is consistent with the count being wrong rather than with coverage genuinely being short — six rounds held the identical ratio while each round found a real defect in a different file.

Suggested change, in rough order of value:

1. When the review body carries the overview table, compare the paths it names against the pull request's changed files. If they match, report the mismatch between the count and the table rather than a bare PARTIAL — something like coverage=PARTIAL(count) table=full, so a caller can tell a counting artifact from missing coverage.
2. Where the table names fewer paths than the diff, name the missing ones. The current caveat says which file went unread "cannot be read from the API", and where the table is present it can.
3. Keep the maintainer-decision framing either way. This is about making the verdict checkable, not about clearing it automatically.

Happy to send the change if the shape above is wanted. Filed from ptr727/Blog, where the caveat came up on a real pull request and cost a round of investigation to bottom out.

[ptr727]

Second data point, at a different ratio, which makes the pattern harder to read as coincidence.

ptr727/Blog#61, a develop -> main promotion, round 1:

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Its overview table names all seven, and the two sets are identical again:

changed files                named by the table
OPERATIONS.md                OPERATIONS.md
TODO.md                      TODO.md
checks/README.md             checks/README.md
checks/build-redirects.py    checks/build-redirects.py
checks/check-url-parity.py   checks/check-url-parity.py
deploy/maps/slugs.map        deploy/maps/slugs.map
hugo.yaml                    hugo.yaml

So the shortfall is exactly one at both M = 6 (issue body) and M = 7 (here), with the table complete both times. Two pull requests, three distinct diffs, and the same off-by-one, which reads as the count excluding one file from its own total rather than as a file going unread.

This also lands on the case the status line calls out as having no remedy: it says splitting is the fix "where it applies, and it does not apply to a promotion". A promotion cannot be split, so on a promotion the verdict is unfalsifiable from the API alone today — and comparing the table against the changed files is exactly what would falsify it.

Nothing changes about the maintainer keeping the decision. It changes what they are deciding with.

[ptr727]

Measured before changing anything, and the measurement splits your report in two: the factual complaint is right and is fixed, and the inference the suggestion rests on does not survive the corpus. Shipping in #625.

The caveat did state something false. "The reviewer names no file list in these rounds, so which file went unread cannot be read from the API" is wrong. Over 348 Copilot review bodies on the newest 120 pull requests here and 121 on ptr727/Blog, 91 carry a | File | Description | table, and every table row in either corpus belongs to one of them. That sentence is gone from scripts/pr_review.py, scripts/README.md and the runbook.

Suggestion 1 cannot discriminate, and Blog is where that shows. On Blog the table names exactly the changed set on every round measured: all seven partials (#61, #60, #59, #51, #50, #36, #35) and every fully covered round alike. A signal that is identical under both hypotheses separates a counting artifact from a genuinely unread file exactly never, so table=full would read as exoneration while carrying no information. This repository has the converse too: #574 states 33 of 33 and its table names 32, omitting TODO.md, so a short table does not mean missing coverage either.

Here, the table tracks neither number:

round	states	table names
#476	10 of 11	all 11 changed
#592	2 of 3	all 3 changed
#479	16 of 17	16, omitting GOVERNANCE.md
#609	61 of 62	50 of 62
#606	7 of 7	names GOVENANCE.md, a path no diff carries

Suggestion 2 ships, narrowed to the one shape that carries evidence. #479 is the only round on record whose table locates the unread file, and its omission matches the count's shortfall exactly. That is also the only evidence anywhere that the count is real rather than an artifact, which cuts the other way on the framing. So status names omitted files only where the table is short by exactly what the counts leave unread and names nothing outside the diff, that second guard existing because #606's typo would otherwise push the real GOVERNANCE.md into the omissions and report it as the file nobody reviewed. Otherwise it reports what the table is, and takes no verdict from it:

#479       the reviewer's own file table omits exactly the 1 file the counts leave unread, naming GOVERNANCE.md. The table is prose the reviewer writes rather than a list from the API, so that is a lead to check rather than a verdict
Blog #60   the reviewer's own file table names all 6 changed files, which it also does on rounds stating full coverage, so it corroborates nothing and names no unread file

One thing worth carrying back to Blog: the comparison is head-scoped like the counts are. Three of the four partials here carry their table on the round before a push, describing the diff that push replaced, and comparing that against the current changed files would name a file unreviewed on a stale list. Those report as no table rather than as a comparison.

Exit 42 and the maintainer-decision framing are unchanged, which is your third point.

[ptr727]

Fixed on develop in a706ddb, merged as #625. Closing by hand rather than through the keyword, since Closes #623 sat on a pull request into develop and GitHub only acts on it when the commit reaches the default branch. The change is in develop today and the promotion to main follows on its own schedule.

What answers each of the three suggestions:

1. Report the count against the table rather than a bare PARTIAL. Shipped, in the form the corpus supports rather than as table=full. On ptr727/Blog the table names the whole changed set on all seven partial rounds and on every fully covered round, so it is identical under both outcomes and cannot separate a counting artifact from a file nobody read. status says so in as many words: the reviewer's own file table names all N changed files, which it also does on rounds stating full coverage, so it corroborates nothing and names no unread file.
2. Name the missing paths where the table is short. Shipped, narrowed to the shape that carries evidence: the table must be short by exactly what the counts leave unread and name nothing outside the diff. Promote the prose sweeps and the suppressed-findings digest to main #479 is the only round on record that qualifies, and it now reports omits exactly the 1 file the counts leave unread, naming GOVERNANCE.md. The second guard exists because one round here names GOVENANCE.md, a path no diff carries, which would otherwise push the real file into the omissions and report it as unreviewed.
3. Keep the maintainer-decision framing. Unchanged, and exit 42 is unchanged in every arm.

The factual complaint underneath the report was the most valuable part: "the reviewer names no file list in these rounds" was false, 91 of 348 bodies here carry one, and that sentence is gone from scripts/pr_review.py, scripts/README.md and the runbook. Thanks for filing it with the evidence attached.

One thing worth carrying back: the comparison is head-scoped like the counts are, because three of the four partials here carry their table on the round before a push, describing the diff that push replaced.