Bound What the Merge Gate Owes an Out-of-Diff Prose Finding and Record the Reviewer Footing - #1333
Conversation
…d the Reviewer Footing The Merge Gate half of #1314. Item 2 records that the coverage the gate requires is Copilot's and that CodeRabbit and Qodo are advisory, citing the hub's reviewer evaluation doc rather than restating a roster. Item 3 makes a pre-existing finding on carried Markdown prose outcome 4 applied once per unit, gathered onto the unit's tracker issue on the hub, with a carrying repository routing the same finding by ownership, while every other finding keeps its own outcome. GOVERNANCE.md "Verification Discipline" binds the sibling sweep to a finding being fixed and to what the change touched or broke, filing a pre-existing sibling elsewhere rather than folding it in, and agent-conduct surfaces the same bound. Three whole-unit passes and one diff pass, read three times each under the two-round budget. The ledger records each unit at the digest its last reviewer read. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 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. Comment |
There was a problem hiding this comment.
🟡 Changes recommended
The new Merge Gate batching paragraph misstates carrier ownership/routing for intent-fidelity units, which could cause real carrier-owned defects to be incorrectly declined and routed to the hub tracker.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refines the review-loop governance around (1) what the Merge Gate requires from automated reviewers, and (2) how “pre-existing” out-of-diff prose findings should be dispositioned (batching to a per-unit tracker rather than one-by-one), while updating the canonical review ledger and regenerating skill mirrors.
Changes:
- Updates
pr-review-conductMerge Gate item 2 to explicitly treat Copilot as the required coverage signal, with CodeRabbit/Qodo as advisory (perdocs/pr-reviewer-evaluation.md). - Bounds sibling-sweep expectations in
GOVERNANCE.md“Verification Discipline” to the files the change touched (and to contradictions the change introduced). - Records new canonical-review ledger entries and regenerates the distributed skill mirrors and digest stamps.
File summaries
| File | Description |
|---|---|
| reports/canonical-review.json | Updates the canonical-review coverage ledger with new unit digests/stamps and adds the Merge Gate unit entry. |
| GOVERNANCE.md | Refines “fix the class” sweep guidance to be bounded to what the change touched/broke. |
| .agents/skills/pr-review-conduct/SKILL.md | Updates Merge Gate language for reviewer footing and adds batching rules for pre-existing prose findings. |
| .agents/skills/agent-conduct/SKILL.md | Mirrors the bounded sibling-sweep rule into agent-conduct guidance via a pointer to “Verification Discipline”. |
| .github/skills/pr-review-conduct/SKILL.md | Regenerated mirror of the pr-review-conduct skill for GitHub Copilot discovery. |
| .github/skills/agent-conduct/SKILL.md | Regenerated mirror of the agent-conduct skill for GitHub Copilot discovery. |
| .claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.md | Regenerated Claude plugin copy of pr-review-conduct. |
| .claude-plugin/fleet-skills/skills/agent-conduct/SKILL.md | Regenerated Claude plugin copy of agent-conduct. |
| .claude-plugin/fleet-skills/.source-digests/pr-review-conduct | Updates the plugin digest stamp for pr-review-conduct after regeneration. |
| .claude-plugin/fleet-skills/.source-digests/agent-conduct | Updates the plugin digest stamp for agent-conduct after regeneration. |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…he Merge Gate Batches Under The maintainer-granted round past the two-round budget, answering four findings the third read left open. A carrying repository routes a finding on a verbatim unit by fidelity, since only a verbatim unit's text is the hub's, and an intent unit's body stays the carrier's own to fix. Outcomes 2 and 4 are named by their section rather than by position. The gloss and the complement both name the style class, which is declined rather than gathered. The change that moves a unit key owes the tracker's retitle. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new Merge Gate batching text is ambiguous/partly self-conflicting about tracker lifecycle and how threadless (suppressed) findings are “answered,” and it needs clarification to be reliably followable across hub vs. carrier contexts.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 2
- Review effort level: Lite
…ng It the Carrier's Own One clause, maintainer-granted, answering the contradiction the previous round wrote. A finding on an intent unit is filed on the same hub tracker, the carrier adapting its own copy meanwhile, since the defect is still fixed at the source, which is what "Verification Discipline" already says of intent fidelity. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Newly added prose exceeds the repo’s 25-words-per-sentence cap for new agent-authored text and should be recast into shorter, clearer sentences.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
.agents/skills/pr-review-conduct/SKILL.md:65
- This newly added guidance contains multiple long sentences that exceed the 25-word cap, and it also mixes two routing criteria (class vs. fidelity) in a way that reads contradictory. Recast as short sentences and make the hub-vs-carrier routing rule explicit.
What closing a finding owes turns on whether it is `pre-existing`. A finding on text inside a
canonical Markdown unit, one the hub's `scripts/canonical_review.py list` names, classed
`pre-existing` by the classes `local-strict-review` "Disposing of Findings" defines for a
local pass, applied here to a PR-hosted finding, is outcome 4 of "Every finding ends in one
of five outcomes" below applied once per unit rather than once per finding: the round gathers
- Files reviewed: 10/10 changed files
- Comments generated: 3
- Review effort level: Lite
|
Answering the suppressed finding in Copilot's round 3 review on 2f1aaa8, Suppressed comments (1):
|
…sh the WORKFLOW.md Reshape (#1397) Closes #1205. Closes #1212. Closes #1240. Closes #1250. Closes #1267. Closes #1268. Closes #1271. Closes #1288. Closes #1305. Closes #1314. Sixteen commits, ten issues. Each was driven as its own feature pull request into `develop`, reviewed by the PR-hosted reviewers, and merged only with CI green and every finding disposed of by one of the five outcomes: fixed, declined on evidence, decided by the maintainer, deferred behind a filed issue, or fixed as a class. ## What this promotes **The one-home include mechanism and its first six classes** (#1317). `scripts/build_dist.py` gained include regions filled from a rule's home and checked by `--check` (#1378), so a Skill carries a rule's whole text without a copy that can drift. Classes 2 to 6 then converted the restatements: `agent-conduct`'s three conduct sections (#1382), `pr-review-conduct`'s five outcomes into `drive-pr` with every step-ref renamed to a heading (#1383), `backlog-burndown`'s two narrowing rows cut to the narrowing with fourteen restatements pointered (#1384), `WORKFLOW.md` section 4 into `workflow-ci-contract` (#1385), and section 2 cut to a pointer at `GOVERNANCE.md` "Workflow YAML Conventions" (#1388). **The `WORKFLOW.md` reshape** (#1311 step 14's six-pull-request sequence, now finished). The verdict clause aligned with section 5's Assessment (#1390), sections 3 and 5 carried into `workflow-ci-contract` as generated includes (#1392), 5A collapsed to a procedure and an evidence rule (#1394), and the preamble decisions settled alongside the reshape of section 4, section 6 and the YAML conventions (#1395). Section 4's two longest items shrank to their outcomes with the displaced knowledge moved rather than deleted, and section 6 now states only what each type adds, carrying no N/A list at all. **The review loop's stop rule and disposition policy** (#1330), rewriting disposal by deletion and committing the condition under which a whole-unit loop ends, which #1267 filed as missing. **The Merge Gate's bound on an out-of-diff prose finding**, with the reviewer footing recorded (#1333), and reviewer bots scoped away from the generated Skill mirrors (#1329) so a mirror's diff is never reviewed in place of its source. **The fleet label set**, declared and applied through `configure.sh` (#1334). **The review ledger and skills digest decoupled from the working tree** (#1328), so concurrent branches no longer conflict in a generated report that cannot be hand-merged. Plus one grouped Dependabot bump, `docker/setup-qemu-action` 4.2.0 to 4.3.0 (#1325). ## What is deliberately not closed `#1311`, `#1317`, `#1206`, `#1367`, `#1369`, `#1370`, `#1371`, `#1386` and `#1237` each still hold findings this work did not settle. #1317 stands at class 6 of fourteen, and #1311's step 17 comment records what the reshape filed rather than fixed. ## Owed on merge `spec/files.json` declares both edited `GOVERNANCE.md` sections at `verbatim` fidelity, so every downstream copy goes stale on this promotion and a fleet resync follows it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Part of #1311, the Merge Gate half of #1314. With #1329's mechanical half this settles that issue, so the
develop -> mainpromotion PR carriesCloses #1314.What changed
pr-review-conductMerge Gate item 2 records the reviewer footing: the coverage the gate requires is Copilot's, and CodeRabbit and Qodo are advisory, cited to the hub'sdocs/pr-reviewer-evaluation.md"Status" rather than restated as a roster. The issue's recommendation was advisory on prose paths only, and the recorded footing is advisory on every path, since that doc says no candidate is a required reviewer anywhere. Maintainer's call, flagged here rather than narrowed on my own.pre-existingunderlocal-strict-review's classes, on text inside a canonical Markdown unit, is outcome 4 applied once per unit, gathered onto the unit's tracker on the hub, an open issue whose title carries the unit key, and answered with that link. Every other finding keeps its own outcome. The five outcomes stay the one enumeration.GOVERNANCE.md"Verification Discipline" binds the sibling sweep to a finding being fixed and to what the change touched or broke, filing a pre-existing sibling elsewhere rather than folding it in. This is the tiebreak the Rewrite Disposing of Findings by Deletion and Commit the Stop Rule #1330 body listed as open (its third bullet).agent-conductsurfaces the same bound.Measurement, under the stop rule
introducedfindings over total per read. Reads 1 to 3 are the two-round budget, read 4 the maintainer-granted round on 2930660, read 5 the one clause granted on 2f1aaa8.The two lightly edited units converged in one or two reads. The unit carrying the new rule did not. Each of the two granted rounds fixed the false claim it was granted for and drew a narrower one in its place: the first made an intent unit the carrier's own, the second files every intent-unit finding at the source, which a per-repo intent section such as Repository Layout does not have. That is the pilot's curve, and the next clause would draw the next one.
Open
introducedfindings, reported rather than fixedThese stand for the maintainer. My recommendation is to merge with them recorded on #1331 and let #1317's one-home pass, which this rule now plainly needs, settle the carrier routing in one place.
GOVERNANCE.mdDevcontainer and Repository Layout, a repo's ownCODESTYLE.mdadditions, a "Disproved Claims" ledger) has no source defect, so "filed there too ... fixed at the source" files carrier-local noise on a hub tracker. The Verification Discipline qualifier it drops is "since every other carrier holds it too".pre-existingcondition the hub-side rule carries, so read literally every carrier finding on an intent unit goes to the tracker, against the closing sentence.pre-existing" is falsified by the carrier clause, where fidelity decides.spec/files.json, and covers verbatim and intent while the manifest also carriesverbatim-tree, which is how this skill itself reaches a carrier..github/skills/path that no unit key names, so the text gives it no mirror-to-source mapping to find the unit or its tracker.listnames without saying why. Deliberate, the issue's own "correct for code".backlog-burndownbounds a change to the units the issue names. Thebacklog-burndownhalf is P2 (decision): Decide backlog-burndown's Shape Before Its 23 Open Defects Are Worked One at a Time #1323's.pre-existingfinding "once" with no tracker, so the same defect gets an issue from the local pass and a tracker entry from a PR reviewer. A P1: Give Every Rule One Home, Replacing Restatements With Pointers or Generated Includes #1317 candidate.pre-existingfinding lands on the tracker unjudged. That is the issue's design, batch and move on, with judgment deferred to the tracker.comment-and-doc-style's 25-word cap for new prose, an opt-in check the tree's existing prose predates.Recorded as a coupling rather than a defect: item 2 hard-codes a reviewer footing the evaluation doc marks revisable, and its citation of a hub-only doc from a carried skill falls under no rule
carried-doc-referencesstates, which the diff pass flagged as uncovered rather than judged.Pre-existing findings, filed
.agents/skills/pr-review-conduct/SKILL.md > Merge Gate, check this before merging or enabling auto-merge#1331, the Merge Gate unit's tracker, 9 items..agents/skills/agent-conduct/SKILL.md > When a Failure Surfaces a Lesson#1332, the agent-conduct unit's tracker, 4 items.Verification
Full local gate set per
OPERATIONS.md"Local Verification", 1060 tests, all green. Three whole-unit passes and one diff pass, each read three times, recorded. Pre-push hook passed both checks.Relates #1311, #1314, #1315, #1317, #1327.
🤖 Generated with Claude Code