Carry the Five Outcomes Into drive-pr as a Generated Include and Name Siblings by Heading - #1383
Conversation
… Siblings by Heading Class 3 of #1317. drive-pr's "Disposing of Every Finding" restated pr-review-conduct's five outcomes as a condensed mapping that had already drifted (#1288). It now carries that section whole as an include region scripts/build_dist.py fills from `.agents/skills/pr-review-conduct/SKILL.md > Every finding ends in one of five outcomes`, the first include whose source is a sibling skill rather than a governance document. The source section is edited so it reads whole outside its own file: its one reference to a step of "Expected review loop" names that section by heading, and it ends with the sentence naming drive-pr as the skill that includes it, the shape skill-lifecycle "Included content" sets. The dangling "this" in outcome 1 now names the pass it meant. The nineteen step-refs from drive-pr, local-strict-review, and backlog-burndown into a sibling skill by step, item, or outcome number each become the sibling's document and heading plus what is needed from it. The five references into WORKFLOW.md D-numbers, the write guard's rule 6, and the agent-safety README's requirement 6 are not references between skills and stay. drive-pr's frontmatter no longer enumerates four of the five outcomes as if complete, names the seat that cannot reach the maintainer, and states the promotion end state as every Merge Gate item except the maintainer's explicit permission, the one item the drive's seat cannot reach. Step 8 of "The Drive Loop" states the same end state. Nine carried units were read whole at the fable tier, two rounds, and recorded in the ledger. The digest engine is unchanged, per the class 2 decision on #1317: the region's bytes stay in the including unit's digest. Closes on promotion: #1288 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
Two newly introduced instruction inconsistencies/ambiguities remain (an undefined "pass" referent in outcome 1, and a promotion-loop exit condition mismatch with drive-pr).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR reduces procedure drift by carrying pr-review-conduct's "Every finding ends in one of five outcomes" into drive-pr as a generated include, and by replacing cross-skill step-number references with sibling-skill + heading references.
Changes:
- Replace
drive-pr's hand-written outcome mapping with a generated include sourced frompr-review-conduct. - Update several sibling references (notably in
local-strict-reviewandbacklog-burndown) to name documents and headings instead of step numbers. - Refresh canonical review ledger entries and skill distribution artifacts (generated mirrors and source digests).
File summaries
| File | Description |
|---|---|
| reports/canonical-review.json | Updates recorded digests/findings metadata for multiple reviewed units. |
| .github/skills/pr-review-conduct/SKILL.md | Regenerated distribution copy reflecting the updated outcome section. |
| .github/skills/local-strict-review/SKILL.md | Regenerated distribution copy reflecting updated sibling references and wording. |
| .github/skills/drive-pr/SKILL.md | Regenerated distribution copy showing the new generated-include outcome rule. |
| .github/skills/backlog-burndown/SKILL.md | Regenerated distribution copy with updated sibling references and wording. |
| .claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.md | Regenerated plugin copy of pr-review-conduct. |
| .claude-plugin/fleet-skills/skills/local-strict-review/SKILL.md | Regenerated plugin copy of local-strict-review. |
| .claude-plugin/fleet-skills/skills/drive-pr/SKILL.md | Regenerated plugin copy of drive-pr. |
| .claude-plugin/fleet-skills/skills/backlog-burndown/SKILL.md | Regenerated plugin copy of backlog-burndown. |
| .claude-plugin/fleet-skills/.source-digests/pr-review-conduct | Updated source digest for the plugin distribution. |
| .claude-plugin/fleet-skills/.source-digests/local-strict-review | Updated source digest for the plugin distribution. |
| .claude-plugin/fleet-skills/.source-digests/drive-pr | Updated source digest for the plugin distribution. |
| .claude-plugin/fleet-skills/.source-digests/backlog-burndown | Updated source digest for the plugin distribution. |
| .agents/skills/pr-review-conduct/SKILL.md | Source change clarifying outcome 1 wording and adding include-consumer mention. |
| .agents/skills/local-strict-review/SKILL.md | Source change updating outcome reference phrasing and sibling references. |
| .agents/skills/drive-pr/SKILL.md | Source change to carry the outcomes via include and update promotion-loop exit condition language. |
| .agents/skills/backlog-burndown/SKILL.md | Source change updating sibling references and Merge Gate phrasing in multiple sections. |
Review details
- Files reviewed: 17/17 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…Outcome 1 Means The granted round on #1383. backlog-burndown "The Promotion Boundary" step 2 still said the promotion loop repeats until the pull request carries no open finding, the end state drive-pr step 8 stated before the previous commit, so one loop had two exit conditions. It now states the same one, every pr-review-conduct Merge Gate item except the maintainer's explicit permission to merge, and the bold span that wrapped across three lines is reflowed. Outcome 1 of "Every finding ends in one of five outcomes" said "the round that pass gets dropped on" where the unit never introduced a pass, so it now names the local-strict-review pass. The include in drive-pr is regenerated from it. Three units read whole and recorded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Two updated instructions in .agents/skills/backlog-burndown/SKILL.md introduce avoidable ambiguity that should be clarified before landing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
.agents/skills/backlog-burndown/SKILL.md:407
- "nothing in it terminates on its own" is ambiguous here because the nearest antecedents are "budget", "discipline", and "feature branch". Clarify that the thing that does not terminate is the promotion review loop.
`pr-review-conduct` Merge Gate item except the maintainer's explicit permission to merge, and
nothing in it terminates on its own, so when the budget is reached, stop and put the state to
the maintainer rather than continuing to spend the run's only forward gear on one pull request.
- Files reviewed: 17/17 changed files
- Comments generated: 1
- Review effort level: Lite
The granted one-word round on #1383. Step 2 of backlog-burndown "The Promotion Boundary" said "nothing in it terminates on its own" with "it" twenty words from "that loop" and two nearer nouns, which the diff pass and Copilot both read as ambiguous. It now says "nothing in that loop terminates on its own". One unit read whole and recorded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Answering the one suppressed finding (1) from the round on 5865970, which opened no thread.
Fixed in 52ff72e: the sentence now reads "nothing in that loop terminates on its own", binding to the promotion review loop the same sentence opens with. The recorded diff pass on this commit raised the same finding and confirmed the binding after the fix, under the round the maintainer granted for it. |
There was a problem hiding this comment.
🔵 Needs a closer look
A few updated cross-skill references still omit the sibling heading (reducing traceability), and one sentence in local-strict-review misstates the referenced code-review wording.
Review details
Suppressed comments (4)
Previously missed (2) — in code that hasn't changed since the last review.
.agents/skills/backlog-burndown/SKILL.md:214
- This bullet references
drive-pras "drive-pr's worktree isolation" without naming the specific section indrive-pr. Since this PR is standardizing cross-skill references to use the sibling's heading (for stable lookup without step numbers), prefer citingdrive-pr"The Drive Loop" here so readers can find the rule unambiguously.
This issue also appears in the following locations of the same file:
- line 217
- line 264
.agents/skills/local-strict-review/SKILL.md:136
- This sentence says
code-review"Review the Change" has a "rule to omit preferences", but the referenced text is narrower ("Omit pure preferences"). Using the exact wording matters here becauselocal-strict-reviewdistinguishes preference-only style findings from convention violations.
.agents/skills/backlog-burndown/SKILL.md:221
- This cleanup bullet mentions
drive-pr's "post-merge cleanup" without pointing to a stable location in the sibling skill. To keep cross-skill references discoverable (and consistent with other updates in this PR), citedrive-pr"The Drive Loop" rather than an unlabeled concept name.
- **The worker does no cleanup**, which is this skill's one stated override of `drive-pr`'s
post-merge cleanup and of `repo-worktree`'s post-merge procedure. Say so in the brief, because
a worker following either alone will clean up. The worker still performs the merge itself, and
what the override moves is the two cleanup halves `drive-pr` runs after it, the worktree
procedure and the verify-then-delete of the merged remote branch, **both** rather than only the
.agents/skills/backlog-burndown/SKILL.md:265
- This paragraph refers to
drive-pr's "post-merge cleanup" without naming where that behavior is specified indrive-pr. For consistency with the rest of the PR's cross-skill references, point todrive-pr"The Drive Loop" here.
cleanup step, while no worker is live in a tree it touches. It carries the remote half of `drive-pr`'s
post-merge cleanup too, verifying the merged branch's tip against the pull request's `headRefOid` before
- Files reviewed: 17/17 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Answering the four suppressed findings (4) from the round on 52ff72e, which opened no thread.
No change needed, for all three. The rule this branch applies is #1317's: a skill names a sibling by name and by what it needs from it, never by where in the sibling it sits. "
No change needed. The sentence declines a |
…r Its Fourteen Restatements (#1384) Class 4 of #1317, scoped to one file, `.agents/skills/backlog-burndown/SKILL.md`, plus its two generated mirrors and digest. ## What changed - **The two `narrowing` rows are cut to the narrowing itself** (#1305). "Scope" says the run is narrower than `GOVERNANCE.md` "Repository Boundaries and Write Safety" requires and no longer repeats what that section says about reads or about the owner bound. "The Two Seats" had no narrowing once the restated rule was cut, since a worker is exactly the dispatched task `AGENTS.md` "Session Scope" describes, so its worker bullet is a plain pointer. - **Thirteen of the fourteen other inventory rows become pointers**, each naming the home by document and heading and keeping only the skill's own consequence: the question-issue prompt in "Ranking", the failed-fetch stop in "Grouping and File Claims", the pre-push pass and the shared-checkout sentence in "Dispatching a Worker", the two tier bullets in "Choosing the Worker's Model Tier", the wait bound and the dirty-tree sentence in "Bounding the Wait on a Worker", the escalation bullet in "Raising a Blocked Question", the operational-model paragraph, step 3, and step 5's rebase sentence in "The Promotion Boundary", and the working-notes bullet in "Run State". - **One row is left as it stands**: the inventory's `AGENTS.md` "Where the Rules Live" condensation at "Dispatching a Worker" is the skill stating its own procedure, and the skill is the home of that. The restatement is `AGENTS.md`'s paragraph about this skill, which is class 8's deletion-and-pointer work. - **One edit outside the inventory's fourteen rows**, requested on #1323: the "Bounding a Prose Group" budget bullet is a pointer at `local-strict-review` "Disposing of Findings", since #1312 landed the number the bullet used to state a second time. ## What did not change The eight step-refs in this file were class 3 (#1383). The "grant is bounded by the session" and "Closing keywords go on the promotion pull request" statements have no home and stay, per the inventory's class 14 list. The pre-existing findings on #1337 are untouched, this change fixes none of them and states no new claim in their sentences. ## Verification - `python3 scripts/build_dist.py --check`, `python3 scripts/prose_lint.py . --diff origin/develop` with the CI check list plus `sentence-length`, `python3 spec/validate.py`: clean. - Carried-content pass: ten units of this file read whole at the `fable` tier, one reviewer per unit, recorded in `reports/canonical-review.json`. Counts are in the pull request's read record below. - Diff pass recorded per `local-strict-review`. ## Read record, `introduced` over total per read Ten units read whole at the `fable` tier, one reviewer per unit, before the first commit. Round 2 read only the two units round 1's introduced findings changed, round 3 read the one unit the first diff pass's finding changed, round 4, a one-round grant past the budget, read the one unit the second diff pass's finding changed, and round 5, a second deletion-only grant, read the two units the third diff pass's findings changed. | Read | R1 | R2 | R3 | R4 | R5 | | --- | --- | --- | --- | --- | --- | | Scope | 0/0 | | | | | | The Two Seats | 1/2 | 0/0 | | | | | Ranking | 0/1 | | | | | | Grouping and File Claims | 0/3 | | | 0/2 | | | Bounding a Prose Group | 0/1 | | | | | | Dispatching a Worker | 0/2 | | | | | | Bounding the Wait on a Worker | 0/3 | | | | 0/3 | | Raising a Blocked Question | 0/1 | | | | | | The Promotion Boundary | 1/7 | 0/6 | 0/4 | | 0/7 | | Run State | 0/1 | | | | | | Diff pass | | 1/1 | 1/1 | 2/2 | 0/0 | The six introduced findings: the worker bullet attributed the one-worktree rule to `AGENTS.md` "Session Scope", which never mentions worktrees, the operational paragraph's pointer left "What the model adds ... a direct push" without the clause that anchored it, step 2 of "The Promotion Boundary" still cited "Bounding a Prose Group" as the discipline that sets a budget after that section stopped setting one, the failed-fetch sentence's "pushed since" lost its time anchor when the restated clause was cut, the operational paragraph's "so confirm" drew a cadence question from a premise that no longer stated one, and "doing it by proxy is still doing it" lost its noun when "reaching into a tree" was cut. The first two were fixed before round 2, the third before round 3, which spent the budget, the fourth under the maintainer's first one-round grant before round 4, and the last two, both deletions, under a second grant before round 5. Five of the six are a reference or a connective left standing when the restated clause beside it was deleted, the class 3 curve again, and the third diff pass found two of them on sentences the first two diff passes had read. Every pre-existing finding is on #1337, items 28 to 46, and the repeats of items 3, 13, 15, 22, and 27 are not listed again. Closes on promotion: #1305 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…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)
Class 3 of #1317, one pull request under the stop rule. Settles #1288 in full.
What changes
drive-pr"Disposing of Every Finding" carriespr-review-conduct's five outcomes whole, as an include regionscripts/build_dist.pyfills from.agents/skills/pr-review-conduct/SKILL.md > Every finding ends in one of five outcomes. It is the first include whose source is a sibling skill rather than a governance document, which "The Pipeline" already permits. The condensed mapping it replaces had drifted on outcome 2's scope, outcome 3's trigger and ordering, and a step-ref into its own loop, the three defects drive-pr: outcome 2 is wider than its owner states, and three enumerations are incomplete #1288 names.pr-review-conduct"Expected review loop" by heading, it ends with the sentence namingdrive-pras the skill that includes it, perskill-lifecycle"Included content", and the dangling "this" in outcome 1 now names the pass it meant.drive-pr(six),local-strict-review(five), andbacklog-burndown(eight). The five references intoWORKFLOW.mdD-numbers, the write guard's rule 6, and the agent-safety README's requirement 6 are not references between skills and stay, per the issue's own scope line.drive-pr's frontmatter no longer lists four of five outcomes as if complete, names the seat that cannot reach the maintainer, and states the promotion end state as every Merge Gate item except the maintainer's explicit permission, the one item the drive's seat cannot satisfy. Step 8 of "The Drive Loop" states the same end state, since the diff pass found the description had been a faithful restatement of that step's gap.Measurement,
introducedover total per readNine units read whole at the
fabletier, one reviewer per unit, two rounds as the budget, plus a diff pass each round. HEAD was the merge base when the ledger was recorded, and the branch carries one commit.drive-pr(preamble)drive-prDisposing of Every Findingdrive-prThe Drive Looppr-review-conductEvery finding ends in one of five outcomeslocal-strict-reviewDisposing of Findingslocal-strict-reviewWhen to Run Itbacklog-burndownDispatching a Workerbacklog-burndownThe Promotion Boundarybacklog-burndownWhat Invoking This Skill AuthorizesThe four round 1
introducedfindings had three root causes, all in framing text this change wrote: a seat with no antecedent, an end state that dropped the Merge Gate's review-on-head item, and a sibling rule named without its heading. Round 2 introduced one and the first granted round's diff pass one more, both fixed under grants below. The pre-existing findings are gathered on the four file trackers (#1337, #1343, #1346, #1349), and thedrive-prtracker's item 5 is half settled here.The two granted rounds
Two
introducedfindings were open after the second round, both one-sentence edits and both also raised by Copilot's first round, and the maintainer granted one round for both. Commit 5865970 makesbacklog-burndown"The Promotion Boundary" step 2 state the end statedrive-prstep 8 states, and names thelocal-strict-reviewpass where outcome 1 said "that pass". Three units were read whole again and recorded, and neither edit drew a finding.The diff pass on that commit raised one
introducedfinding, again a pronoun: the rewritten step 2 said "nothing in it terminates on its own" with "it" twenty words from "that loop" and two new nouns nearer, and Copilot's round on the same head raised it as a suppressed finding. The maintainer granted the one-word fix, commit 52ff72e binds it to "that loop", the unit was read whole once more and the diff pass on that commit found nothing. The same passes read the section's opening sentence, "driven to green and left for the maintainer", as a looser statement of the end state, pre-existing, filed on #1337 with the round's other pre-existing findings.Two style notes are taken as they stand, each on a sentence already rewritten twice: "the promotion half of
drive-pr"The Drive Loop"" wheredrive-pritself says "promotion steps", and the one-word line the reflow leaves.One round 2 finding is declined on precedent: the include's closing sentence reads as a third-person statement about
drive-prfrom insidedrive-pr. That is the doc-side sentence the "Included content" shape places inside the section, the same asagent-conductover its threeGOVERNANCE.mdsections, and the price of a byte-identical include.Closes on promotion: #1288
🤖 Generated with Claude Code