Skip to content

Run Copilot Review Once per Change at the Promotion - #3403

Merged
ptr727 merged 6 commits into
developfrom
feature/copilot-review-at-promotion-only
Oct 23, 2026
Merged

ptr727 merged 6 commits into
developfrom
feature/copilot-review-at-promotion-only

Conversation

@ptr727

@ptr727 ptr727 commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

Copilot's hosted review runs once per change, at the develop -> main promotion pull request, and not on feature pull requests into develop.

Why: measured on 2026-10-16 to 2026-10-23, the four most active fleet repositories ran 418 Copilot reviews and about 2,200 workflow runs for 382 merged pull requests, and a promotion re-reviews content its feature pull requests already carried. So the hosted review runs once per change, at the promotion, and the feature pull request carries the local pass and CI. Decision (ptr727, 2026-10-23) on #3391, item 4.

  • repo-config/develop.json: the copilot_code_review rule is removed. repo-config/main.json is unchanged and keeps it.
  • scripts/pr_review.py: round_before_head is removed from holds and local_cover, and the function itself is deleted since nothing else reads it. On a pull request whose base is not the repository's default branch, wait requests no Copilot review, and an attested local pass alone satisfies coverage on the head (review_on_head=local coverage=local), with the digest saying so. Exceptions kept as they were: a partial Copilot round on record, a review history past the 100-review window, or a request already pending. A pull request into the default branch is unchanged. Every exit code keeps its meaning: 49 is still "no round covers this head, none was requested, and no attestation vouches for it", now reachable on the first head too. The docstring, the --request help, the held note, the exit-49 message, and the COVERAGE IS READ FROM THE LOCAL PASS digest line are edited in place.
  • tests/test_pr_review.py: constructed fixtures for both base-branch cases. A feature pull request with no Copilot round is never requested (unattested exits 49, attested exits 0 with coverage=local), status reads review_on_head=local there, a promotion with no round reads review_on_head=NO and no local cover, and a promotion is still requested with or without an attestation. The tests that asserted the old first-round rule now assert the new one, and holds/local_cover are tested directly on both bases. No test calls the live API.
  • .agents/skills/drive-pr/SKILL.md step 3, plus its regenerated mirrors only: request no Copilot round on the feature PR, read the attestation as coverage=local, and what clears exit 49. The promotion steps are unchanged.
  • In-place sentence edits where a statement became false: repo-config/README.md "Rulesets", scripts/README.md (the pr_review.py review-moment paragraph), docs/pr-reviewer-reference.md, .github/copilot-instructions.md, and TODO.md.

Applying the Ruleset Change

configure.sh apply updates an existing ruleset with a PUT that replaces the whole document, so removing the rule from develop.json removes it from the live develop ruleset when apply runs, rather than only no longer asserting it. configure.sh check compares the rule-type set in both directions, so until apply runs it reports drift on every release-model repository, and the live develop ruleset keeps requesting a Copilot review when a feature pull request opens. Nothing here ran configure.sh against any repository. Per repo-config/README.md, run it from a hub checkout at main once this change is promoted. For one repository:

repo-config/configure.sh apply ptr727/ProjectTemplate release

Then repo-config/configure.sh check ptr727/ProjectTemplate release confirms it. Each other release-model repository takes the same command with its own name.

Follow-up: pr-review-conduct Merge Gate text

.agents/skills/pr-review-conduct/SKILL.md is held by another lane, so this pull request leaves it alone. Until the follow-up lands, it and GOVERNANCE.md "PR Review Etiquette" still state the first-round rule this change retires.

Merge Gate item 2. Replace the sentence that opens "On a pull request into a branch other than the default, a head after Copilot's first round can carry an attested local pass instead" with:

On a pull request into a branch other than the default, every head can carry an attested local pass instead, coverage=local in the digest, where the whole review history is in view and no round on record states or appears to state partial coverage. A promotion re-reviews content its feature pull requests already carried, so the hosted review runs once per change, at the promotion. A pull request into the default branch, a promotion among them, always carries a Copilot round on its head.

The same follow-up also owes these sentences, which state the same retired rule:

  • pr-review-conduct "Expected review loop" step 3, the first four sentences and the sentence beginning "status and wait never read", replaced by: "Request a Copilot round at one defined moment only: the head of a pull request into the default branch, a promotion among them. Auto-trigger is unreliable, so request it explicitly, which step 4's wait is what does. On a pull request into any other branch, wait requests nothing and reads step 1's attestation as coverage=local. Pass wait --request where a change deserves a Copilot round anyway." The sentences from "wait also skips the request" onward stay as they are.
  • pr-review-conduct "Which Reviewers a Repository Actually Has", the "Copilot's absence blocks, and is answered elsewhere." bullet: "Merge Gate item 2 requires Copilot's own coverage of the current head, or on a fix push into a branch other than the default an attested local pass," becomes "Merge Gate item 2 requires Copilot's own coverage of the current head, or on a pull request into a branch other than the default an attested local pass,". The rest of that bullet stays as it is.
  • GOVERNANCE.md "PR Review Etiquette", a verbatim-carried section: "Request a review at defined moments rather than on every push: when the pull request opens, and on the head of a pull request into the default branch, a promotion among them." becomes "Request a Copilot review at one defined moment rather than on every push: the head of a pull request into the default branch, a promotion among them." And "On a pull request into any other branch, a fix push after the first round is covered instead by the recorded local pass the push already owes, published to the pull request so the review status can read it." becomes "On a pull request into any other branch, every head is covered instead by the recorded local pass the push already owes, published to the pull request so the review status can read it."

Verification

Run from the worktree with COVERAGE_FILE, MYPY_CACHE_DIR, UV_CACHE_DIR, and RUFF_CACHE_DIR under a scratch cache directory, per OPERATIONS.md. Results, the full suite rerun at the final head as the second test row shows:

Command Result
uvx ruff@latest format --check . 257 files already formatted, exit 0
uvx ruff@latest check . All checks passed!, exit 0
uvx mypy@latest (the [tool.mypy] CI checker in pyproject.toml) Success: no issues found in 63 source files, exit 0
python3 -m unittest tests.test_pr_review Ran 958 tests in 23.203s / OK
uvx coverage@latest run --source=scripts,spec,host-setup -m unittest discover -s tests Ran 3059 tests in 206.598s / OK (skipped=7), exit 0
python3 -m unittest discover -s tests, rerun at the final head 5b5ae9ad Ran 3059 tests in 214.314s / OK (skipped=7)
spec/audit.py --selftest under coverage SELFTEST PASS, exit 0
spec/workflow_reuse.py --selftest under coverage workflow_reuse selftest OK, exit 0
gh-write-guard.py --selftest under coverage SELFTEST PASS, exit 0
python3 scripts/build_dist.py --check Generated skill distributions are current., exit 0
python3 scripts/canonical_review.py status exit 0
python3 scripts/repo_gate.py every check [ok ], exit 0
python3 scripts/prose_lint.py . --check charset --check semicolon --check dash --check dupword --check spelling --check comment-wrap --check comment-case --check home-path --check dead-path --check issue-ref scope: 378 file(s) read, whole tree, exit 0
python3 scripts/prose_lint.py --diff origin/develop scope: 12 of 378 file(s) read, 130 changed line(s), exit 0 (same result at the final head)
jq empty over registry/*.json spec/*.json repo-config/*.json no output, exit 0
python3 spec/validate.py Spec validation OK: 22 cataloged repos classify cleanly., exit 0

Not run: scripts/docker_lint.py, which needs Docker and repository-exposure approval this seat does not hold, the live-repository spec/audit.py, and repo-config/configure.sh check, which this change was briefed not to run against any repository. OPERATIONS.md names the last two for a change to repo-config/, so they are the maintainer's after the apply step above. The opt-in --check sentence-length run is listed under the local review below.

Local Review

Five recorded local-strict-review passes, at the frontier tier, against develop: 10 findings, then 4, then 2 before the first push, then 3 after Copilot's round, then 0, then 0 again after merging develop to clear a conflict in the generated drive-pr digest. Every introduced finding is fixed, so none remains open.

One first-round finding is answered here rather than in code: a release-model repository keeps its live Copilot rule on develop until configure.sh apply runs there, and the spec's branch.ruleset.develop check reports drift until then. "Applying the Ruleset Change" above names the command.

Copilot's round raised three findings. One, on docs/pr-reviewer-reference.md, is fixed. Two, on the drive-pr step and the Copilot instructions, are the known disagreement with pr-review-conduct and GOVERNANCE.md that the follow-up above carries, and are declined in their threads on that boundary.

Pre-existing findings, not filed

Per Decision (ptr727, 2026-10-23) on #3391, item 2, these are listed rather than filed. Each is a sentence over the opt-in 25-word sentence-length cap that this branch did not write and only trimmed, rewrapped, or left beside an edit:

  • .github/copilot-instructions.md:31: six sentences in the "GitHub Copilot Review Runbook" paragraph.
  • docs/pr-reviewer-reference.md:27: the 55-word and 28-word sentences after the edited ones.
  • repo-config/README.md:15: the 43-word opener of the operational bullet.
  • repo-config/README.md:17: the 38-word opener of the main paragraph.
  • scripts/README.md:181: five sentences in the review-moment paragraph.
  • TODO.md:140: the 42-word Settled bullet.
  • scripts/pr_review.py: the sentences at lines 100, 306 (the 58-word bot-id sentence), 310, and 6067.

🤖 Generated with Claude Code

ptr727 and others added 3 commits October 23, 2026 07:18
A promotion re-reviews content its feature pull requests already
carried, so the hosted review runs once per change, at the
develop -> main promotion, and a feature pull request carries the
attested local pass and CI. Measured 2026-10-16 to 2026-10-23, the four
most active fleet repositories ran 418 Copilot reviews and about 2,200
workflow runs for 382 merged pull requests.

Drop the copilot_code_review rule from the develop ruleset, keep it in
main. pr_review.py no longer needs a first Copilot round before an
attested local pass covers a head on a pull request into a non-default
branch, so wait requests nothing there and status reads coverage=local
on an attested head. A pull request into the default branch is
unchanged. drive-pr step 3 says so, and the script README, the reviewer
reference, and the Copilot instructions drop the old first-round rule.

Decision (ptr727, 2026-10-23) on #3391, item 4.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
State drive-pr's exceptions where wait still requests a round and what
clears exit 49, keep the holds and local_cover docstrings distinct, drop
the operational bullet's now-shared Copilot rule and the stale "both
ruleset payloads" in TODO.md, and split the sentences this change
rewrote.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings October 23, 2026 14:32
@ptr727

ptr727 commented Oct 23, 2026

Copy link
Copy Markdown
Owner Author

A recorded local strict-review pass covers head ceb6bd0c981741b5675070dc7ee4fb038c2130fe, the content this pull request carries at that commit against develop, and it recorded 2 findings.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

🟡 Changes recommended

Unresolved review findings remain in the review guidance and feature-head coverage messaging.

3 open issues
What changed in this PR

This pull request changes fleet review policy so feature pull requests rely on an attested local review and CI, while Copilot review runs at the develop → main promotion.

Changes:

  • Removes the Copilot review rule from the develop ruleset while retaining it for main.
  • Updates pr_review.py and its tests for local coverage on non-default branches and hosted review at promotion.
  • Synchronizes documentation, review guidance, and generated skill distributions.
File Description
TODO.md Updates the settled ruleset note.
tests/​test_pr_review.py Covers feature and promotion review behavior.
scripts/​README.md Documents the revised Copilot request flow.
scripts/​pr_review.py Implements local coverage for eligible feature heads.
repo-config/​README.md Documents the ruleset split and promotion review model.
repo-config/​develop.json Removes Copilot review from develop.
docs/​pr-reviewer-reference.md Updates operational reviewer guidance.
.github/​skills/​drive-pr/​SKILL.md Updates the generated drive-pr instructions.
.github/​copilot-instructions.md Updates the Copilot review runbook.
.claude-plugin/​fleet-skills/​skills/​drive-pr/​SKILL.md Regenerates the Claude skill mirror.
.claude-plugin/​fleet-skills/​.source-digests/​drive-pr Updates the generated skill digest.
.agents/​skills/​drive-pr/​SKILL.md Updates the canonical drive-pr workflow instructions.

Review effort: Lite

Comment thread .agents/skills/drive-pr/SKILL.md
Comment thread .github/copilot-instructions.md Outdated
Comment thread docs/pr-reviewer-reference.md Outdated
@codecov

codecov Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (develop@92875fb). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #3403   +/-   ##
==========================================
  Coverage           ?   81.60%           
==========================================
  Files              ?       19           
  Lines              ?    10231           
  Branches           ?        0           
==========================================
  Hits               ?     8349           
  Misses             ?     1882           
  Partials           ?        0           
Flag Coverage Δ
python-3.13 81.60% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ptr727 and others added 2 commits October 23, 2026 07:37
…ested

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ptr727

ptr727 commented Oct 23, 2026

Copy link
Copy Markdown
Owner Author

A recorded local strict-review pass covers head 5b5ae9ad9cbecfabe06105a3a3ef05adb33db82f, the content this pull request carries at that commit against develop, and it recorded 0 findings.

…view-at-promotion-only

# Conflicts:
#	.claude-plugin/fleet-skills/.source-digests/drive-pr
@ptr727

ptr727 commented Oct 23, 2026

Copy link
Copy Markdown
Owner Author

A recorded local strict-review pass covers head 3ad8c2ad27ed7642c07e8ffd51bed9426b4183bb, the content this pull request carries at that commit against develop, and it recorded 0 findings.

@ptr727
ptr727 merged commit 96d1396 into develop Oct 23, 2026
13 checks passed
@ptr727
ptr727 deleted the feature/copilot-review-at-promotion-only branch October 23, 2026 15:38
ptr727 added a commit that referenced this pull request Oct 23, 2026
## Summary

Brings the review rule text in line with two changes already on
`develop`: Copilot's hosted review runs once per change, at the
promotion, and a `pre-existing` finding is listed in the pull request
body rather than filed unless it is a correctness defect with a stated
failure. Prose only, no code.

Decision (ptr727, 2026-10-23) on #3391, items 2 and 4.

### Copilot reviews once per change, at the promotion

Applies the replacement text from #3403's "Follow-up: pr-review-conduct
Merge Gate text", in place:

- `.agents/skills/pr-review-conduct/SKILL.md` "Merge Gate" item 2: every
head on a pull request into a branch other than the default can carry an
attested local pass (`coverage=local`), and a pull request into the
default branch always carries a Copilot round.
- Same item 2, promotion evidence path: a constituent feature pull
request's full coverage comes from its attested local pass or a Copilot
round, and a Copilot table row on the promotion itself counts as
per-file evidence. The "lacked that coverage" example reads "no attested
local pass and no review round".
- "Expected review loop" step 3: one defined moment, the head of a pull
request into the default branch. Elsewhere `wait` reads the attestation
as `coverage=local` where Merge Gate item 2 allows that reading, and
requests a round only where a partial round or a truncated review
history is on record (the review found the dictated sentence dropped the
exception `pr_review.py` keeps).
- "Copilot's absence blocks, and is answered elsewhere": "on a fix push
into" becomes "on a pull request into".
- `GOVERNANCE.md` "PR Review Etiquette": the two first-round sentences,
a sentence naming the review wait's partial-history request, a pointer
to the Merge Gate for when the published pass counts as coverage, and
the following sentence's "for such a fix push", which referred to the
retired case and is recast around "on such a pull request".

### Class-sweep sibling takes the pre-existing outcome

- `GOVERNANCE.md` "Verification Discipline": a pre-existing sibling in
an untouched file stays out of the diff, and such a sibling takes the
outcome a `pre-existing` finding takes under `pr-review-conduct` outcome
4, by pointer.
- `pr-review-conduct` outcome 1: "filing the rest" becomes "recording
the rest as outcome 4 records a `pre-existing` finding".
- `pr-review-conduct` outcome 4: the sentence saying outcome 1 "keeps
its own filing rule" for a sweep sibling says instead that the sibling
takes those bullets as a `pre-existing` finding does.

`GOVERNANCE.md` "Design Before Code" is deliberately unchanged. Its
"Sites in files a change does not touch move in a change of their own,
filed as one issue naming each" is a consolidation rule rather than the
class-sweep sibling rule. Its bold lead says every site moves onto the
primitive, and `pr-review-conduct` "Triaging findings" files the other
sites as one issue to match. Sending those sites to outcome 4 would drop
a consolidation finding and leave the lead false, so that change is left
to the maintainer.

`agent-conduct` and `drive-pr` change only inside generated include
regions, regenerated with `python3 scripts/build_dist.py`, and the
`.github/skills/` and `.claude-plugin/` mirrors are regenerated.

## Verification

- `python3 scripts/build_dist.py --check`: `Generated skill
distributions are current.` (exit 0)
- `python3 scripts/prose_lint.py . --diff origin/develop`: exit 0,
`scope: 13 of 380 file(s) read, 95 changed line(s)` at 466adc1

## Local Review

Four recorded `local-strict-review` passes at the `frontier` tier, the
last over the pushed head with no findings. The two `introduced`
findings the third pass left open, the one-moment rule against `wait`'s
partial-history request and the implicit antecedent of the Merge Gate
pointer, were raised again by Copilot and are fixed in 466adc1.

Declined: `sentence-length` reports on edited sentences. The check is
opt-in, and most of those sentences are either the dictated replacement
text or pre-existing sentences with one clause changed.

## Pre-existing findings, not filed

- `pre-existing`, `scripts/pr_review.py` module docstring (`attest`) and
`scripts/README.md` `pr_review.py` section: "A promotion reads the
comment only in its Merge Gate's per-file fallback" is stale since
feature pull requests merge on `coverage=local`, so the promotion case's
first bullet also reads the attestation.
- `pre-existing`, `.agents/skills/pr-review-conduct/SKILL.md` "Which
Reviewers a Repository Actually Has": the bold lead "Copilot's absence
blocks, and is answered elsewhere" overstates the case for a pull
request into a non-default branch, where an attested head merges with
Copilot absent. The dictated replacement left the rest of that bullet as
it is.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
ptr727 added a commit that referenced this pull request Oct 23, 2026
## Summary

Promotes these pull requests from develop. Each carries a maintainer
decision recorded on #3391.

- [#3399](#3399): states
the write guard's threat model and documents two shapes it keeps
allowed, a quoted run-time `-C` value and an `xargs` replace string run
as a shell script (item 1).
- [#3401](#3401): lists a
pre-existing review finding in the pull request body instead of filing
an issue for it (item 2).
- [#3402](#3402): keeps
one standing handoff issue per track, edited in place each round, with a
`close` subcommand that ends a track (item 5).
- [#3400](#3400): adds a
weekly backlog and review-cost report script.
- [#3403](#3403): runs the
Copilot review once per change, at the promotion, with the attested
local pass covering a pull request into develop (item 4).
- [#3404](#3404): aligns
the remaining review rule text with the promotion-only Copilot review
(items 2 and 4).
- [#3406](#3406): fixes
the findings this promotion's first Copilot round raised on the content
above, in the `wait` note for a head no local pass covers, the
pr-review-conduct heading, and the backlog report's label line and cost
scoping.

Closes #3382
Closes #3385

## Pre-existing findings, not filed

- The [Copilot round on
4a77a1d](#3405 (review))
headline says documentation does not consistently describe current-head
Copilot refusal handling. `local_cover` in `scripts/pr_review.py`
refuses an attested head whose current Copilot round is a refusal, and
`wait` exits `41` there. The `wait` entry of the `pr_review.py`
docstring ("An attested head is covered at once.") and the
`scripts/README.md` paragraph on the defined review moments ("Where it
is attested, `wait` polls the head's checks instead.") leave that
exception out. `pr-review-conduct` Merge Gate item 2 states it ("A
refusal is not that coverage"). Both omissions are on `main` already, so
this promotion did not introduce them.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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