Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,42 @@ gh api repos/<owner>/<repo>/issues/<N>/comments --jq \

Coverage is confirmed when (1) exits 0, and **a formal review with no inline comments still satisfies path (1)**, because coverage is about the head SHA, not the comment count. The exception is the refusal above, which is a formal review on the head with no inline comments and covers nothing, so path (1) exits 0 over a round that never ran. Read the body of the review the SHA matched, not only the SHA. For issue comments (path 2), body content is the only reliable signal, and `created_at` is not: `git log -1 --format=%cI` is the **commit** timestamp, not the push timestamp, so amended or rebased commits can have an earlier timestamp and an older Copilot comment could satisfy a time check even though Copilot never saw the current head. Treat path (2) as confirmed only when the comment body explicitly refers to the current changes.

**Coverage of the head is not coverage of the diff, and the second one is stated in a line nothing above reads.** A review body says how many of the pull request's changed files it read, and a round that read fewer than the pull request changed is byte for byte the clean pass in everything else: the same `commit.oid`, the same absent threads, the same "generated no comments". Measured over 332 Copilot review bodies on this repository, five rounds across three pull requests reported reading fewer files than were changed, and all three merged. One of them changed three files, left one unread across **both** its rounds, and reported no comments each time. This is the third instance of the shape the refusal above and the suppressed block below are the first two, so read it the same way: **fail closed on a wording you do not recognize**, since a gate that allows whatever it does not recognize stops gating as the wording drifts, and both of those wordings have drifted once already.

Two spellings carry the count, and both are current rather than one superseding the other. Each opens its own line, which is what separates the round stating its coverage from prose mentioning changed files, that prose being what a review of a change to this rule looks like:

```text
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
- **Files reviewed:** 2/3 changed files
```

The sentence tail after the first spelling reports how many comments the round raised and appears in four wordings. It is not coverage, so it is not part of what has to be recognized, and the counts are. Read them into three verdicts and two exemptions:

- **Counts equal** - the round read the whole diff. This is the clean pass.
- **Counts unequal** - files in the diff have no review at all. Re-request on this head or split the pull request, rather than merging on it.
- **Coverage-shaped and unreadable** - the remedy is to fix the reader, not to read past it. The vetted spellings live in `scripts/pr_review.py` and here, and they stay in step because a case reads them out of this file.
- **Exempt: a body stating no coverage at all.** 28 of those 332 bodies are an overview and a change list and nothing more. That shape is current, interleaves with the counted one throughout, and one pull request carries both across its two rounds, so treating it as a failure cries wolf on about one review in twelve and teaches an agent to work around the gate. It reads as `coverage=unstated`, never as a pass and never as a failure.
- **Exempt: a refusal.** It carries no coverage line by design, and the refusal rule above has already classified it. Read it here as well and every refusal grows a spurious second failure on top of the one that names its remedy.

`scripts/pr_review.py status <N> --repo <owner>/<repo>` reports this as `coverage=full`, `coverage=PARTIAL`, `coverage=UNVETTED` or `coverage=unstated`, and exits `42` on a partial round. An unreadable wording exits `43` instead, as one of the unrecognized shapes below rather than as a case of its own, since both say the reader is what needs fixing. Read it by hand as:

```sh
gh pr view <N> --json reviews --jq \
'.reviews[] | select(.author.login=="copilot-pull-request-reviewer") | .body
| split("\n")[] | select(test("^(Copilot|[-*] \\*\\*Files reviewed:).*changed files?"))'
```

### A Shape Nothing Recognizes Blocks the Loop and Earns an Issue

**Every rule above keys on a marker in what Copilot sent, so a marker that changes spelling is a section the reader stops finding and reports as absent.** That is not a hypothetical: all three failures on record here have exactly that shape. The suppressed heading was reworded and the count went to zero. The suppressed section moved inside another wrapper and the count went to zero again. The coverage line was never read at all. Each one reported a clean pass over a review it had misread, and each was caught by the maintainer after it had already landed, rather than by the gate.

**So an unrecognized shape is a blocking outcome, and its remedy is an issue rather than a judgment call.** When any reader here meets a heading, a collapsed section, a metadata line, a coverage wording or a reviewer login it has no vetted spelling for, the review loop **does not close**, whatever else the digest says. Do not read past it, do not infer what the new wording probably means, and do not treat a body that looks clean as a clean review, because "looks clean" is precisely what a misread review looks like. Two things follow, in this order:

1. **File an issue on the hub, `ptr727/ProjectTemplate`**, which hosts `scripts/pr_review.py` and holds the vetted inventory. Name each unrecognized shape and quote the review body it came from, so the fix is made against the real wording rather than a paraphrase. The issue is filed even when the shape turns out to be cosmetic, since "cosmetic" is a conclusion drawn after reading the body and not before.
2. **The merge decision is the maintainer's**, not the agent's and not the script's. An unrecognized shape does not mean the pull request is bad, it means nothing here can vouch for the review of it. Report the state, hand it over, and stop.

`scripts/pr_review.py status <N> --repo <owner>/<repo>` reports this as `shapes=UNRECOGNIZED`, lists each shape under a marker naming the remedy, and exits `43`. `wait` carries the same code, so a wait cannot end on a clean zero over output nothing read. The vetted inventory lives in that script and is small on purpose: measured over 332 Copilot review bodies on this repository, with fenced blocks dropped and text reduced to ASCII, the whole corpus is seven headings, six `<summary>` texts and three metadata labels, and every body carries at least one of them. A body carrying none is itself the unrecognized shape, which is what catches a rewrite that changes everything at once, the refusal wording drifting among it.

### Bounded Retry Workflow

This path is only for a **genuinely missing** review, meaning no Copilot review (formal *or* issue comment) covers the current head SHA after polling. A review that covered the head but produced no comments is a clean pass, not a missing review, so do not enter this retry path for it.
Expand Down
7 changes: 4 additions & 3 deletions GOVERNANCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,10 @@ The repo runs a review loop on every PR: local agent iteration plus remote autom
**Do not merge, and do not enable auto-merge, unless ALL of these hold:**

1. Required status checks are green (`mergeStateStatus: CLEAN`), **and**
2. A Copilot review is confirmed on the **current head SHA** by matching the review's commit SHA to the head, not an earlier push, because a push makes required checks go green **before** the re-review lands, so a green merge-state can precede the current-head review and never signals readiness on its own, and the matched review is **read** rather than only counted, because Copilot declines a pull request it will not take on with a formal review carrying that same head SHA and no findings, which matches the SHA and covers nothing, **and**
2. A Copilot review is confirmed on the **current head SHA** by matching the review's commit SHA to the head, not an earlier push, because a push makes required checks go green **before** the re-review lands, so a green merge-state can precede the current-head review and never signals readiness on its own, and the matched review is **read** rather than only counted, because Copilot declines a pull request it will not take on with a formal review carrying that same head SHA and no findings, which matches the SHA and covers nothing, and because a review can carry the head SHA and still say it read only part of the diff, which is the same clean pass in everything a SHA match can see, so the body's own count of the files it read is checked against the files the pull request changed, **and**
3. **Every** Copilot finding on that head SHA is closed out, with all review threads resolved, **and** any issue-level Copilot comments (which have no resolve action) triaged and replied to, **and** the low-confidence findings collapsed in the review body investigated and answered, since those appear in no thread and a loop that polls threads alone reports a clean pass while they stand, so zero outstanding findings remain, **and**
4. The maintainer has given **explicit** permission to merge.
4. Nothing in the reviewer's output was a shape the review tooling could not read, since every reader keys on a marker and a marker that changes spelling is a section the reader stops finding and reports as absent, which is how three separate misreadings each reported a clean pass over a review they had not understood. An unrecognized shape blocks this gate on its own, and the remedy is an issue filed against the repository hosting the reader, naming the shape and quoting the body it came from, rather than a judgment about what the new wording probably meant, **and**
5. The maintainer has given **explicit** permission to merge.

`mergeStateStatus: CLEAN` reflects **only** required statuses, and never open bot review comments, so `CLEAN` alone is **never** sufficient to merge. A green/`CLEAN` PR with an unresolved Copilot finding fails this gate, so treat it as "not mergeable" no matter what the merge-state field says. The agent never merges on its own (consistent with "default to staging", and merging is maintainer-authorized).

Expand All @@ -283,7 +284,7 @@ The repo runs a review loop on every PR: local agent iteration plus remote autom
6. Reply to each thread and resolve what was addressed.
7. Re-run the loop after every fix push until no actionable findings remain.

Drive the loop to green, meaning a review confirmed on the latest head SHA and every actionable finding closed, then stop and apply the **Merge Gate** above: all four preconditions must hold, and `mergeStateStatus: CLEAN` alone never satisfies it.
Drive the loop to green, meaning a review confirmed on the latest head SHA and every actionable finding closed, then stop and apply the **Merge Gate** above: all five preconditions must hold, and `mergeStateStatus: CLEAN` alone never satisfies it.

For provider-specific mechanics (how to request review, query review state, post replies, resolve threads), see the **GitHub Copilot Review Runbook** in [.github/copilot-instructions.md](./.github/copilot-instructions.md). This file owns the contract, and that file owns the mechanics.

Expand Down
18 changes: 18 additions & 0 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,23 @@ One pull request routing the disproof record from the provider-agnostic contract
- **Settled** - The write side is where the gap bites rather than the read side, because an agent following the loop is already routed to the runbook for mechanics and an agent posting a decline is routed nowhere.
- **Settled** - "Durable Knowledge and Self-Improvement" already requires durable knowledge to reach a committed file, so this states where one class of it goes rather than adding an obligation.

### A Programmatic Reading of a Copilot Review

One pull request, after a survey, deciding whether anything stands between this fleet's review loop and the raw prose of a Copilot review. Today `scripts/pr_review.py` reads the review body as text and holds a vetted inventory of the headings, collapsed sections, metadata labels and coverage wordings it recognizes, blocking on anything it does not. That design is correct for a prose surface and it carries a cost the maintainer has accepted deliberately: a wording change at GitHub blocks every open pull request in the fleet at once, until the inventory is updated. The cost is worth paying against a reviewer silently missing a raised finding, which is the failure it replaces, but it is worth paying only for as long as prose is the only surface on offer.

**State** `measure`. **Touches** `scripts/pr_review.py` and the runbook section in [`.github/copilot-instructions.md`][copilot-instructions], once the survey says whether there is anything to move to. **Cost** a survey first, then either nothing or a rewrite of the reading layer, which is the larger of the two outcomes and the reason the survey comes first.

- **Find out whether GitHub publishes a structured form of a Copilot review, and decide whether to read that instead of the prose.** A schema, an API surface, a published payload, or a maintained library, anything that would make a wording change a non-event rather than a fleet-wide block.
- **Blocked by** - Nothing. The prose reader ships either way, so this decides what replaces it rather than whether the loop has a gate.
- **Issue** - None filed. The prose reader and its vetted inventory shipped under [#607][issue-607], which is the change this would supersede.
- **Checked** - `develop` at `20916ad` on 2026-08-07, reading the live GraphQL schema by introspection and one review over REST, against the reader in `scripts/pr_review.py`.
- **Open** - Whether `bodyHTML` is a better surface than the Markdown body, since it arrives as a rendered tree whose structure survives a change in Markdown syntax, while leaving the wording drift the inventory exists for exactly where it is.
- **Open** - Whether any third-party library tracks this output, and whether depending on one is acceptable at all, given that [`scripts/README.md`][scripts] holds these scripts to the standard library with no third-party packages.
- **Open** - Whether the review's own inline threads and their metadata carry enough to derive coverage and suppression without reading the body, which would narrow the prose surface rather than replace it.
- **Settled** - The public API carries no structured Copilot review as of the date above. GraphQL `PullRequestReview` exposes `body`, `bodyText` and `bodyHTML` and no field naming a finding, a file count, or a withheld section, and REST returns the same prose body beside its ids and its state.
- **Settled** - The only Copilot-named types in the GraphQL schema are `CopilotCodeReviewParameters` and its input form, which configure review-on-push inside a branch ruleset and describe nothing about a review that has run, so the schema search that looks promising by name answers a different question.
- **Settled** - A negative finding is the deliverable as much as a positive one, and it is recorded here rather than re-derived, since the reading layer's design rests on prose being the only surface and that premise is worth re-checking rather than assuming.

## Standalone Chores

Small work with no research to preserve, selectable one bullet at a time.
Expand Down Expand Up @@ -448,6 +465,7 @@ Each was checked against the tree and has nothing left to do anywhere. Closing i
[issue-580]: https://github.com/ptr727/ProjectTemplate/issues/580
[issue-585]: https://github.com/ptr727/ProjectTemplate/issues/585
[issue-597]: https://github.com/ptr727/ProjectTemplate/issues/597
[issue-607]: https://github.com/ptr727/ProjectTemplate/issues/607

<!-- Repo -->

Expand Down
Loading