docs(agents): one Copilot review per PR, not one per push - #11216
Merged
Merged
Conversation
Review-on-push was turned off in the "Automated Copilot Code Review" ruleset of both repositories on 2026-09-03 (owner), so the bot runs once when a PR opens or leaves draft and a push triggers nothing. CLAUDE.md's PR follow-through step 3 now says that, and says not to re-request one per push: each request is a full re-read of the whole diff, and the bot then surfaces "previously missed" findings in files the push never touched — which draws another push, and another request. A fresh review is requested only after a substantive rework, and a PR that is green with no open threads needs none. agentic/commands/pull_request.md gains the same note at its own step 7, whose follow-up commit is exactly the push that used to look like a reason to re-request. The rule is not mirrored into .github/copilot-instructions.md: that file has no PR-follow-through block, and this is agent workflow rather than one of the rules the two guides share. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The review-cadence wording and sibling PR references need correction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates agent guidance to reflect Copilot’s one-automatic-review-per-PR cadence.
Changes:
- Documents the substantive-rework exception.
- Updates the pull-request workflow.
- Records the change in the changelog.
File summaries
| File | Review |
|---|---|
CLAUDE.md |
Nit (2 votes): Replace bare #406 with a repository-qualified link. |
CHANGELOG.md |
Nit (1 vote): Clarify the automatic-versus-manual review distinction and link the sibling PR explicitly. Nit (1 vote): Replace bare #406 with a qualified reference. |
agentic/commands/pull_request.md |
Nit (1 vote): Clarify that only the automatic review runs once; substantive rework may warrant another request. |
Review details
Suppressed comments (3)
CHANGELOG.md:232
- The bare
#406links to this repository's unrelated issue rather than the sibling pull request being described. Qualify the reference so the changelog does not send readers to the wrong record.
another push, and another request (the sibling repo's PR #406 collected ~15 in a day over a
one-line docstring fix). A fresh review is requested only after a substantive rework, and a PR
CHANGELOG.md:233
- This release note repeats the exact-one claim despite the documented substantive-rework exception, and bare
#406links readers to anyplot's unrelated issue rather than the sibling PR used as evidence. Preserve the automatic-versus-manual distinction and use an explicit cross-repository link.
so, and say not to re-request one per push: each request is a full re-read of the whole diff, and
the bot then surfaces "previously missed" findings in files the push never touched — which draws
another push, and another request (the sibling repo's PR #406 collected ~15 in a day over a
one-line docstring fix). A fresh review is requested only after a substantive rework, and a PR
that is green with no open threads needs no further round. (#11216)
agentic/commands/pull_request.md:61
- “Runs once” conflicts with the exception two lines later and can make an agent skip a warranted post-rework review. Clarify that the automatic review runs once, while an additional review is requested only for substantive rework.
8. **The Copilot review runs ONCE**, when the PR opens (or leaves draft) — review-on-push is off in
the "Automated Copilot Code Review" ruleset (owner, 2026-09-03), so step 7's follow-up commit and
every later push trigger nothing. Do not re-request a review per push; the rule and its one
exception (a substantive rework) are in `CLAUDE.md` § "PR Follow-Through", step 3.
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot: a bare #406 resolves to this repository's own issue 406, not the sibling PR the evidence lives in. Both mentions are now full links to MarkusNeusinger/kurrentschrift#406. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3
…adence # Conflicts: # CHANGELOG.md
6 tasks
MarkusNeusinger
added a commit
that referenced
this pull request
Sep 5, 2026
## Summary - Carries the session retro that landed in the sibling repository as [kurrentschrift#538](MarkusNeusinger/kurrentschrift#538) into this repository's own conventions and anchors. `CLAUDE.md` now says what the Edit/Write rule outranks (an auto-mode reminder that arrives later in the context and prescribes `sed`/heredoc editing), and the delegation rule adds the second line a brief has to carry: all `git` stays inside the agent's own worktree. - Adds `.claude/guardrails.md`, the long version of the working rules — three sections only, the ones that apply here: the two lines every delegate brief carries, the `! bash` runbook pattern for a prod pair the harness classifier refuses (this repository's own origin-gate README as the worked case, rollback as its own script), and why a one-token substitution is not an exemption from the Edit/Write rule. `tests/unit/test_agent_instructions.py` pins the split so a rule cannot end up living only where no session loads it. - Closes the gap [#11216](#11216) left in `/open-pr`: that PR wrote the `review_on_push: false` cadence into `CLAUDE.md` and `agentic/commands/pull_request.md`, but the skill still carried "Copilot reviews every push round" as a gotcha and had no merge conditions at all. Plus three smaller transfers — branch-named scratchpad files, `/verify-frontend` § 3b (measure the result, not the plan), and `/write-docs` on shortening a text that carries claims. ### What landed where | Rule from the retro | Landed in | |---|---| | Edit/Write outranks the auto-mode reminder | `CLAUDE.md` § Development Workflow | | A delegate's `git` stays in its own worktree | `CLAUDE.md` § Important Rules (delegation bullet) | | Rule → incident → recipe, off the hot path | `.claude/guardrails.md` (new) + pointer section in `CLAUDE.md` | | Merge-on-request conditions (four, on the head SHA) | `/open-pr` § 3e | | `mergeable` vs. `mergeStateStatus`, `DIRTY` gets no CI | `/open-pr` § 3e | | One Copilot review per PR, re-request only after a rework | `/open-pr` § 3b, and the contradicting gotcha corrected | | Commit-message / PR-body files named after the branch | `/open-pr` § 2 | | A numeric UI rule is measured, not planned | `/verify-frontend` § 3b (new) | | Shortening drops qualifiers first | `/write-docs` § "Shortening a text that carries claims" | **Not transferred, with the reason:** - **The `/verify-api` versioned-contract duty.** It presumes a tool that pushes shared state at the API while both sides must agree how the data is computed, with the SERVER answering `409` on a mismatch. This repository's shared-state writes go straight to Postgres from the pipeline (`sync_to_postgres`), and its admin surface is `/debug` reads plus a cache invalidate — the mechanism has nothing to attach to here. - **The sibling's measurement guardrails** (frozen rulers, pinned BLAS threads, rescue-path register). Domain-specific to that repository; skipped as instructed. - **`.github/copilot-instructions.md` was left untouched.** The Edit/Write rule and the delegation rule do not exist in it at all — that file deliberately carries a subset aimed at review, and the sync duty is "a rule that exists in both". Adding either would be introducing a rule, not mirroring one. Flagging it here rather than deciding it unilaterally. **Two judgement calls I made myself:** `/write-docs` drops the sibling's "regenerate `app/prerender/` in the same push" clause, because this site serves crawlers through the live `@seo_proxy` rather than a committed prerender directory; and `.claude/guardrails.md` attributes the incidents to the sibling repository where that is where they happened, rather than restating them as local history. ## Plan N/A — sibling transfer of an already-merged retro. ## Test plan - [x] `uv run --extra test pytest tests/unit/test_agent_instructions.py` — 30 passed, including the three new guardrails pins - [x] `uv run --extra test pytest tests/unit` — 1929 passed - [x] `uv run --extra dev ruff check .` and `ruff format --check .` — clean - [x] `uv run python -m tools.changelog check --base origin/main` — fragment well-formed, no `[Unreleased]` bullet added - [ ] Read `.claude/guardrails.md` end to end: every claim about `infra/cloudflare/README.md` (§ "Arming, in full", § "Rolling back") matches what that file actually says - [ ] `/open-pr` § 3e reads correctly against this PR's own head SHA — the check-runs dedupe query returns one row per check name --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Markus turned review-on-push off in the "Automated Copilot Code Review" ruleset of both repositories today (2026-09-03): the bot now runs once when a PR opens or leaves draft, and a push triggers nothing. The instructions still read as though a review followed every push, so an agent would keep asking for one.
CLAUDE.md§ "PR Follow-Through", step 3 now states the cadence and the reason not to force it:A fresh review is requested explicitly only after a substantive rework (new behaviour, a reworked mechanism); a round that yields no new inline comments but only carried-over items is the end of it; a PR that is green with no open threads needs none at all.
agentic/commands/pull_request.mdgains the same note as a new step 8 — placed there deliberately, because its step 7 (append the PR number to the changelog bullets and push) is exactly the push that used to look like a reason to re-request.Not mirrored into
.github/copilot-instructions.md: that file has no PR-follow-through block, and this is agent workflow rather than one of the rules the two guides share. If it should live there too, say so and it is one more bullet.Carried over from kurrentschrift's
CLAUDE.md, where the same guardrail has stood since 2026-08-23 (sibling-transfer rule).Verification
pytest tests/unit/test_agent_instructions.py tests/unit/agentic— 189 passed, so the backticked paths still resolve and the mirrored-rule pins still hold.🤖 Generated with Claude Code
https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3