Skip to content

State the Partial-Coverage Remedy the Record Supports - #613

Merged
ptr727 merged 1 commit into
developfrom
fix/partial-coverage-remedy-as-measured
Aug 8, 2026
Merged

State the Partial-Coverage Remedy the Record Supports#613
ptr727 merged 1 commit into
developfrom
fix/partial-coverage-remedy-as-measured

Conversation

@ptr727

@ptr727 ptr727 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Follows the #609 promotion, which went partial at 61 of 62 files and stayed there through two re-requests. That prompted measuring whether the remedy the gate recommends has ever worked.

It has not.

The measurement

Over the 332 Copilot review bodies on the newest 120 pull requests, read beside gh pr list --json number,changedFiles,additions,deletions:

Question Answer
Pull requests that went partial 4 - #476, #479, #592, and the #609 promotion
Rounds those four produced 7
Rounds that recovered 0 - every later round repeated the identical ratio
Files skipped, each time exactly 1
PR 592: round 1 = 2/3    round 2 = 2/3
PR 476: round 1 = 10/11  round 2 = 10/11
PR 479: round 1 = 16/17
PR 609: round 1 = 61/62  round 2 = 61/62   (after a re-request on a new head)

So the digest was telling a reader to spend a round on a retry that nothing has ever retried out of.

Two hypotheses tested and rejected, so they are not re-derived

Size does not predict it. The partials changed 502, 629 and 961 lines. Fully covered pull requests on this repository reach 33 files and 2,219 changed lines, well past all three.

The denominator is not itself lossy. I suspected the reviewer might drop a file before counting, which would make N out of N a lie. It does not: the stated denominator equals the API's own changedFiles on 103 of 104 pull requests, and the single exception is one whose branch shrank between rounds. The reviewer counts the file and does not read it.

What changed

The three places that stated the remedy now state what the record supports, swept by grepping the phrasing rather than by fixing the one the finding pointed at:

  • scripts/pr_review.py - the status=COVERAGE_IS_PARTIAL line, the digest's COVERAGE IS PARTIAL block, and the status exit-code docstring.
  • .github/copilot-instructions.md - the "Counts unequal" bucket.
  • scripts/README.md - the coverage paragraph, carrying the measurements.

The new wording says a re-request has never cleared one, that splitting is real for a feature branch and unavailable for a promotion whose head is develop, and that the merge decision is the maintainer's, taken knowing one file has no review. No exit code or verdict changes, since the gate was reporting the state correctly. Only the advice attached to it was wrong.

What is left open

TODO.md gains an entry under the programmatic-reading cluster, with the measurements attached: which file is skipped, and why. The reviewer names no file list in these rounds, so it cannot be recovered from the API, and the pull request page may be the only place it appears. Whether it is worth escalating to GitHub needs the file first.

🤖 Generated with Claude Code

The coverage reading shipped telling a reader to re-request or split. The
#609 promotion then went partial at 61 of 62, and re-requesting twice
returned the identical ratio both times, which prompted measuring the rest.

Over the 332 review bodies on the newest 120 pull requests: four pull
requests have gone partial, across seven rounds, and every later round
repeated the identical ratio. No round has ever recovered, so a re-request
is not the remedy it reads as, and telling an agent to spend a round on it
is telling it to retry into a state nothing retries out of.

Size does not predict the state either. The partials changed 502, 629 and
961 lines, where fully covered pull requests here reach 33 files and 2,219
lines. The stated denominator equals the API's own changed-file count on
103 of 104 pull requests, so the reviewer counts the file and does not read
it rather than losing it earlier, and it names no file list in these
rounds, so which file went unread cannot be read from the API at all.

The three places that stated the remedy now say what the record shows, and
the open question of which file and why is a backlog entry with its
measurements attached.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 8, 2026 02:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the documentation and user-facing output of scripts/pr_review.py to reflect measured behavior: partial file-coverage reviews have not been observed to recover via re-requesting, so the recommended remedy is no longer to “retry into” the state.

Changes:

  • Rewords the status=COVERAGE_IS_PARTIAL and digest messaging to treat partial coverage as a durable state to escalate to a maintainer decision.
  • Documents the measurement and open research question in TODO.md (which file is skipped, and why).
  • Aligns the runbook guidance in .github/copilot-instructions.md and adds supporting explanation in scripts/README.md.

Reviewed changes

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

File Description
TODO.md Records the measurements and follow-on questions about partial-coverage rounds.
scripts/README.md Documents the observed non-recovery of partial coverage and the resulting operational guidance for exit 42.
scripts/pr_review.py Updates exit-code documentation and COVERAGE_IS_PARTIAL messaging in status and digest outputs (no behavior/exit-code changes).
.github/copilot-instructions.md Updates the “Counts unequal” guidance to match the measured record (don’t recommend re-requesting as a remedy).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ptr727
ptr727 merged commit 676a2bd into develop Aug 8, 2026
7 checks passed
@ptr727
ptr727 deleted the fix/partial-coverage-remedy-as-measured branch August 8, 2026 02:40
ptr727 added a commit that referenced this pull request Aug 8, 2026
Fixes the finding Copilot raised on the promotion pull request [#615](#615), against code [#613](#613) shipped.

## The defect

The `status=COVERAGE_IS_PARTIAL` line ended with "taken knowing one file of the diff has no review". The code knows only that the reviewed and changed counts differ. It never computed the difference.

Every partial on record skipped exactly one file, over four pull requests and seven rounds, which is where the wording came from. That is a measurement of those rounds rather than a property the state carries, so a round skipping two files would have been reported as skipping one, on the single line whose whole purpose is to inform a merge decision.

## The fix

`report_verdict` now keeps the coverage line that decided `PARTIAL` and re-reads its counts, so the message reports the difference the run carries and agrees with the `coverage=PARTIAL` ratio printed beside it:

```text
... taken knowing 1 of the 3 changed files has no review
... taken knowing 4 of the 9 changed files have no review
```

Re-reading cannot return `None`, because `PARTIAL` is reached only where that same line parsed, and the comment says so rather than leaving a reader to derive it.

## Verification

- `python3 scripts/test_pr_review.py`, 175 tests, OK. The new case asserts both a one-file and a four-file gap, and it **fails against the previous wording**, confirmed by stashing the fix and re-running: `FAILED (failures=2)`.
- The rest of the CI gate list run locally and green: `spec/validate.py`, `test_prose_lint.py`, `test_repo_gate.py`, `spec/audit.py --selftest`, `gh-write-guard.py --selftest`, `repo_gate.py`, and both `prose_lint.py` invocations.
- `ruff` reports 22 findings across these two files, all pre-existing and none inside the changed hunks, and the hub's CI does not run it over `scripts/`.
ptr727 added a commit that referenced this pull request Aug 8, 2026
…-coverage remedy (#615)

Promotes `develop` to `main`, carrying five merged pull requests.

## What is being promoted

- **[#617](#617
`8515666`, the branch rename written `-M` rather than `--move --force`,
which git gained later, checked against git's own option tables at
`v1.5.4` and `v2.0.0`.
- **[#616](#616
`08cf819`, the partial-coverage status line counting the unread files
rather than asserting there is one.
- **[#614](#614
`f39e0c0`, the branch bootstrap step in [`STANDUP.md`](./STANDUP.md).
Section 0B states the sequence that keeps the exploratory standup off
`main` and `develop`, carries the procedure's only `git init` alongside
both of section 0's identity checks, and records why the post-hoc
cleanup is unavailable. It also carries one Disproved Claims entry and
the removal of the shipped backlog cluster.
- **[#613](#613
`676a2bd`, the partial-coverage remedy in `scripts/pr_review.py` and its
two documentation mirrors, corrected to what the record supports after
measuring 4 pull requests and 7 rounds with 0 recoveries.
- **[#612](#612
`1892d6f`, a Dependabot group bump of four actions.

#616 and #617 both answer findings raised by the review **on this
promotion**. A promotion's head is `develop`, so neither could be fixed
here, and each took its own pull request into `develop` and its own
review rounds before merging.

## Review state

Each of the five was reviewed and merged on its own pull request, so
this promotion carries no unreviewed change. #614 ran 13 rounds, 12
findings accepted and 1 declined and recorded under Disproved Claims.
#616 ran 3 rounds and #617 ran 2, all findings accepted.

## Merge shape

This is a promotion, so it merges as a **merge commit** rather than a
squash, per [`GOVERNANCE.md`](./GOVERNANCE.md) "Branching Model". Its
head is `develop` itself, so it must **not** be merged with
`--delete-branch`.
ptr727 added a commit that referenced this pull request Aug 8, 2026
The branch was parked on 2026-08-06 pending the GitHub Actions outage,
and develop moved 20 commits past it. Six regions conflicted, and one of
them was not a text collision.

Exit 42 now means two things. This branch defined it as the review loop
closed over a check in a shape no wait clears, and #607, #608 and #613
have since defined 42 as a round that read fewer files than the pull
request changed, with 43 as a shape the reader does not know.

The check reading becomes 44, ranked under both. 43 says no field here
can be believed, so it outranks everything. 42 says part of the diff has
no review. 44 says the review itself is sound and a required check is
wedged, which is only worth reporting once the two above are clear, so
`wait` returns report_verdict's code where it has one and reads the
checks only where it does not.

The digest prints both sets of blocks, with the unrecognized-shape block
still first, since it says how far the rest of them can be trusted.

Resolved from develop's text rather than this branch's wherever develop
had edited the same line, which the copilot-instructions bullet needed:
the prose batch replaced its semicolons, and taking this branch's older
copy would have reverted that and failed the gate that now checks it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727 added a commit that referenced this pull request Aug 8, 2026
`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.
ptr727 added a commit that referenced this pull request Aug 8, 2026
Adds the missing half of the selection procedure's integrity check, and amends one cluster with an upstream reference.

## The gap

Step 1 confirms every open **issue** appears in this file. Nothing asked the same of an open **pull request**, so a pull request whose blocker has passed is invisible to the one procedure that would catch it. Nothing selects it, nothing closes it, and `develop` moves underneath it.

[#591](#591) is the worked example and is carried in the step as its evidence. It was parked correctly on 2026-08-06, during a GitHub Actions major outage, with the reason written on the pull request. The reason then expired quietly. Three days later it was 20 commits behind `develop`, conflicting in six regions, and its central exit code had come to mean something else, because #607, #608 and #613 had taken `42` for a different reading in the meantime.

The failure is not specific to this repository. The same shape was reported on Blog, where two pull requests were left open through the same outage and a day of new work landed on top of them.

## The rule

Step 2 asks that every open pull request carries a **stated active blocker**: stated where the pull request itself carries it rather than held in a session that has ended, and active only while the thing it names is still true. A landed review round, a merged dependency and a passed outage each stop being one, and what they leave is a forgotten pull request rather than a parked one. The remedy is to finish it, close it, or write the current blocker down.

## Measured before writing, not after

The lesson this repository keeps relearning about checkers is to measure the live corpus before shipping a rule, so it flags what it is for rather than the routine traffic:

| Reading | Result |
| --- | --- |
| Open pull requests right now | **1** (#591) |
| Dependabot pull requests, open to merged | ~**1 minute** (#611 and #612 both `02:24` to `02:25`) |

The merge-bot takes bot traffic inside a minute, so it never sits long enough to owe a blocker, and the rule's working set is the handful of human pull requests that actually linger.

## Second disposition in this change

**Amends "A Programmatic Reading of a Copilot Review"**. That cluster's open question is whether GitHub publishes anything but prose to read a Copilot review from, and its `Settled` line records that the public API does not. The ask is now filed upstream as [GitHub community discussion 204320](https://github.com/orgs/community/discussions/204320), which requests a versioned machine-readable schema carrying severity, category, suggestion and resolution state. It is unanswered, so the entry records it as a place to watch rather than a dependency to wait on.

## Verification

`prose_lint.py` clean including `sentence-split`, `editorconfig-checker` exit 0, `markdownlint-cli2` 0 issues across 44 files, `TODO.md` at 497 of 497 CRLF lines. The renumbering was checked against the file's own cross-references, and the only one that names a position is step 1 calling itself first, which it still is.
ptr727 added a commit that referenced this pull request Aug 8, 2026
…request sweep (#619)

Promotes `develop` to `main`, carrying two merged pull requests.

## What is being promoted

- **[#591](#591
`07ed74a`, reading why a merge is blocked instead of reporting one word.
`scripts/pr_review.py` gains the check rollup, the four stuck shapes it
tells apart, and exit `44` for a review loop that closed against a check
no waiting clears, plus the [`GOVERNANCE.md`](./GOVERNANCE.md) and
runbook wording that says `BLOCKED` never names its own cause.
- **[#618](#618
`dd5fc90`, the open pull request sweep in [`TODO.md`](./TODO.md)'s
selection procedure, plus an amendment recording the upstream ask for a
machine-readable Copilot review schema.

## The exit code, since it changed meaning between branches

#591 was authored before the outage of 2026-08-06 and defined exit `42`
for its check reading. #607, #608 and #613 took `42` for a round that
read fewer files than the pull request changed, and `43` for a shape the
reader does not know, while it sat. The forward-merge renumbered the
check reading to **44** and ranked it 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 the coverage and shape verdict where it has one and reads
the checks only where it does not.

## Review state

Both were reviewed and merged on their own pull requests, so this
promotion carries no unreviewed change. #591 ran 18 rounds across its
life, 5 of them after the revival, and #618 ran 5. Every finding was
accepted except one on #614, which was declined with evidence and
recorded under "Disproved Claims" in
[`.github/copilot-instructions.md`](./.github/copilot-instructions.md).

## Why #591 was open long enough to need reviving

It was parked correctly during the GitHub Actions major outage, with the
reason written on the pull request, and the reason then expired quietly.
Three days later it was 20 commits behind `develop`, conflicting in six
regions, and carrying an exit code that meant something else. #618 is
the procedural answer to that, and it is in this same promotion.

## Merge shape

This is a promotion, so it merges as a **merge commit** rather than a
squash, per [`GOVERNANCE.md`](./GOVERNANCE.md) "Branching Model". Its
head is `develop` itself, so it must **not** be merged with
`--delete-branch`.
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.

2 participants