docs: sparring briefing for the upstream advisor chat - #12
Merged
Conversation
added 2 commits
July 25, 2026 11:21
…, and prompt conventions
… model revalidation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR documents ad-hoc task briefs as prompts, adds an upstream sparring/advisor workflow specification, and updates tooling revalidation tasks for the new documentation and future template changes. ChangesPrompt workflow guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Greptile SummaryAdds an upstream-advisor briefing and incorporates it into the repository’s prompt governance.
Confidence Score: 4/5The branch-protection assumption should be corrected before merging because it can cause advisor-generated briefs to omit routing required to preserve PR-only checks. The new briefing treats an externally configured, optional repository setting as guaranteed enforcement even though the workflow documents that direct pushes can bypass pull-request-only checks. Files Needing Attention: docs/sparring-briefing.md Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
docs/sparring-briefing.md:67-68
**Branch routing is not enforced**
When branch protection has not been manually configured, this guidance tells the advisor that branch-to-PR routing is implied even though direct pushes remain available, causing work to bypass PR review and checks that run only for `pull_request` events.
Reviews (1): Last reviewed commit: "docs: ad-hoc task briefs are prompts too..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+67
to
+68
| - **Branch → PR is implied** (branch protection enforces it), but say it when | ||
| the routing matters (separate branch, don't stack on X). |
There was a problem hiding this comment.
When branch protection has not been manually configured, this guidance tells the advisor that branch-to-PR routing is implied even though direct pushes remain available, causing work to bypass PR review and checks that run only for pull_request events.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/sparring-briefing.md
Line: 67-68
Comment:
**Branch routing is not enforced**
When branch protection has not been manually configured, this guidance tells the advisor that branch-to-PR routing is implied even though direct pushes remain available, causing work to bypass PR review and checks that run only for `pull_request` events.
How can I resolve this? If you propose a fix, please make it concise.
Owner
Author
There was a problem hiding this comment.
Dismissed — the factual premise is wrong for this repository.
The claim is that branch protection "has not been manually configured", so direct pushes remain available and can bypass pull_request-only checks. Verified against the GitHub rulesets API rather than the repo's own prose:
name: protect-main. enforcement: active target: branch
conditions.include: ["~DEFAULT_BRANCH"]
rules: non_fast_forward, pull_request, required_status_checks
bypass_actors: [] # empty — admins included
So main accepts changes only through a PR with a green quality check, with no bypass actors. That is exactly the premise docs/sparring-briefing.md:67 relies on, and the ruleset was added deliberately to close this gap (README.md:132-139; commit e52d9b6, "the version-bump PR-only gap is closed by the main branch ruleset", #10).
The narrower true statement nearby: .github/workflows/ci.yml and AGENTS.md invariant 12 record that the version-bump checker is pull_request-only, so a direct push would bypass that checker. The ruleset is what answers it — direct pushes to main are not available in the first place.
One limitation worth stating, since it cut the other way here: a ruleset lives in repo settings, not in the tree, so nothing readable in the checkout re-verifies it. That is why this was confirmed against the API.
There was a problem hiding this comment.
You're right, and the correction is well-documented. The ruleset (protect-main, enforcement active, pull_request + required_status_checks, no bypass actors) directly contradicts the premise of my comment. I was wrong to flag this without verifying the actual branch protection configuration.
The narrower point you surface — that the version-bump checker is pull_request-only and a direct push would bypass it — is answered by the same ruleset making direct pushes to main unavailable entirely. The guidance at line 67 stands.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/prompt-standards.md`:
- Around line 9-16: Update the checklist-count wording in the ad-hoc task briefs
paragraph so it accurately includes all 12 checklist items, preferably by saying
“all twelve” or using wording that avoids a hard-coded count.
- Around line 9-16: Align the ad-hoc-brief standard across repository and
scaffolded prompts: add the paragraph from docs/prompt-standards.md lines 9-16
to the embedded /workflow-init template, and remove the deferred-only entry from
todos.md lines 185-190. Keep the wording and scope consistent so initialized
projects receive the same standard immediately.
In `@docs/sparring-briefing.md`:
- Around line 10-12: Update the “Target model” statement in the sparring
briefing to name the specific supported model and version, and document the
corresponding prompting-page verification. If that verification cannot be
provided, remove the claim that the document follows docs/prompt-standards.md.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a9ed0a8b-353f-422b-a2ac-60c6e888b4d5
📒 Files selected for processing (3)
docs/prompt-standards.mddocs/sparring-briefing.mdtodos.md
…t count Two PR #12 bot findings, both accepted and introduced by this PR's diff: - prompt-standards.md said briefs are not reviewed against "all ten items" while the checklist runs 1-12 and AGENTS.md invariant 11 says 12. - sparring-briefing.md asserted it follows docs/prompt-standards.md while giving its target model as "any capable chat model" — checklist item 1 requires a named executing model, which a model class cannot satisfy. Now names Claude in a chat interface, upstream of Claude Code, matching the convention every other prompt artifact here uses. Gate B N/A: both staged paths are docs/**.md prose (CLAUDE.md §5).
…tion vehicle Item 5 of process-pr-review. Both fixed findings are the third occurrence of their class, and neither class has ever reached a deterministic rung: docs-drift 1 prose -> P std -> (this) unverified-enforcement-claim P std -> 1 prose -> (this) The 2026-07-19 row's own ref concedes its rule is human-run, not CI-run, and "raises the floor; it does not close the class". The escalation the ladder points at is mechanical, and this instance is tool-decidable: a file claiming to follow docs/prompt-standards.md must carry a Target model: line. Logged at rung pending rather than written here: the checker is shell code, and adding it to a docs-only PR would fire full Gate B on new code. It rides the upcoming canvas-findings hardening round, which touches commands/workflow-init.md and pays that cost regardless. The deferred template sync rides the same round. The fixes themselves landed in 4f31df2; what is pending is the hardening. Pending rows resolve by appending rung-2 rows, never by editing them. Gate B N/A: both staged paths are explanatory documentation (CLAUDE.md 5) -- a backlog file and the ledger, neither under .claude/, plugins/, skills/, commands/ or agents/, and neither steering a model through a task.
dsnger
added a commit
that referenced
this pull request
Jul 26, 2026
) * Give two recurring prompt-conformance classes their mechanical rung Both classes reached a third occurrence and sat at rung `pending` in the ledger, naming this work as their resolution vehicle. Two checks in check-invariants.sh now guard one spelling each, and the appended rung-2 rows say exactly which. Check 4a — a file asserting it follows docs/prompt-standards.md must carry exactly one column-zero `Target model:` line whose value BEGINS with a recognized token and names exactly one distinct model. Both halves are load bearing: presence alone accepts "any capable chat model", and a token found anywhere accepts it too, because the real PR #12 defect line mentioned Claude as provenance. Measured: token-anywhere matched it, token-at-start did not. Check 4b — a prose count claim must equal the checklist it counts. Word forms one..twenty are in scope BECAUSE the motivating occurrence was one; a digit-only check would have sailed past the very defect the row claims to harden. Claims are recognized in two stages so a malformed `all 012 items` fires rather than being invisible. The ledger is excluded from both checks. A ledger that quotes defects self-rejects the checks that detect them: docs/hardening-log.md carries the historical count claim as evidence, and scanning it would fail the repository forever on rows that exist to record the fix. Also here: one shared fixture initializer, without which 25 of the 61 pre-existing assertions failed on a missing checklist before reaching their own assertion; the docs sweep across seven locations in five files that describe this checker; and AGENTS.md invariant 11 narrowed, since "nothing mechanical checks them" stopped being true. Suite 61 -> 123 assertions. Gate B: 8 passes, 13 findings, final spec and quality passes both clean against an unchanged tree. No plugins/** path is touched, so invariant 12 requires no version bump; the §5 nudge, the companion files and the template sync land in PR 2 with the bump. * Force the string compare in 4b's digit branch, matching its sibling CodeRabbit nitpick on #13: `prompt_checklist_count` writes `$0 "" != n ""` to force a string compare, and the header credits that idiom for keeping a long digit run from overflowing into a wrong verdict — but the claim validator did the equivalent comparison as a bare `tok != n`. Validated by hand rather than by subagent: the PR touches AGENTS.md, so process-pr-review's instruction-path precheck bars triage. The claim is true as a consistency point and not a live bug — `n` arrives via `-v` (a strnum) and `tok` comes from `sub()` (a string), so POSIX already resolves it as a string compare, and the 40-digit fixture passes either way on BSD awk 20200816. The point stands that it relied on type inference the rest of the file deliberately does not. Gate B triviality skip, documented per CLAUDE.md §5: one-token, behaviour- preserving change inside an existing awk expression, covered by the existing `4b: 40-digit claim rejected` and `4b: non-canonical 012 rejected` fixtures. Not hardened: no ledger class fits a style-consistency nit whose behaviour was already correct, and minting one would dilute the recurrence signal. (First attempt at this comment broke the script — an apostrophe in `awk's` terminated the single-quoted awk program. The suite caught it; the comment now says so.)
dsnger
added a commit
that referenced
this pull request
Jul 26, 2026
Three findings came out of a real project running this workflow. This is the second of two PRs; #13 took the mechanical checks. Finding A was cut after five Gate-A passes and finding B after two — both are stories now, carrying the findings that killed them as their opening evidence. **Companions (finding C).** The §5 protocol knew only about the findings file, so dispositions and interrupted-cycle state lived in chat history and died with the session. Both §5 copies now describe two advisory companions: a per-pass dispositions file, and a cycle-stable resume note — gate-a-spec-resume.md, gate-a-plan-resume.md, gate-b-resume.md. Cycle-stable rather than pass-named because a note keyed to the interrupted pass is exactly the file a resuming agent will not look for once the counter moves. Gate B gets one note even under reviewType: full: the per-branch findings files race only because Codex's two reviewers write them, while the resume note is written by the outer agent, sequentially. Both are optional and nothing enforces them — the ledger row says so rather than implying P std made context durable. **Template sync.** The ad-hoc-briefs paragraph, deferred since #12, is now in the scaffolded template — but not verbatim. The repo paragraph links a file /workflow-init never scaffolds and asserts this repo's own incident count, so downstream gets a neutral variant preserving both halves of the principle: briefs carry the checklist's habits, and nobody reviews a brief against all 12 items. The canvas provenance line stays repo-only, per prompt-standards item 8. **Bot completion signal.** #12 and #13 both merged heads that were never reviewed: the check passed while the comment read "Review rate limited", and on #13 the only CodeRabbit review record names eed589c while the merged head was 92de0d2. docs/pr-review-bots.md now separates "the check stopped pending" from "the head was reviewed" across all three sites, with a verification command that was found broken by running it — gh api --slurp is rejected with --jq — and is now measured in both directions. Gate A: 8 passes, clean. Gate B: 5 passes, final spec and quality both clean on an unchanged tree.
dsnger
added a commit
that referenced
this pull request
Jul 26, 2026
#12 and #13 merged unreviewed heads and the miss was found afterwards. On #14 the verification query caught it BEFORE merging: check green, comment rate-limited, zero qualifying reviews for the head. Re-trigger produced nothing; merged on an explicit human decision with the exception recorded. Gate B triviality skip, documented per CLAUDE.md §5: prose-only addition to a descriptive table's surrounding notes, no mechanism or routing changed.
dsnger
added a commit
that referenced
this pull request
Jul 26, 2026
* Ship the §5 companion files, the deferred template sync, and 0.6.0 Three findings came out of a real project running this workflow. This is the second of two PRs; #13 took the mechanical checks. Finding A was cut after five Gate-A passes and finding B after two — both are stories now, carrying the findings that killed them as their opening evidence. **Companions (finding C).** The §5 protocol knew only about the findings file, so dispositions and interrupted-cycle state lived in chat history and died with the session. Both §5 copies now describe two advisory companions: a per-pass dispositions file, and a cycle-stable resume note — gate-a-spec-resume.md, gate-a-plan-resume.md, gate-b-resume.md. Cycle-stable rather than pass-named because a note keyed to the interrupted pass is exactly the file a resuming agent will not look for once the counter moves. Gate B gets one note even under reviewType: full: the per-branch findings files race only because Codex's two reviewers write them, while the resume note is written by the outer agent, sequentially. Both are optional and nothing enforces them — the ledger row says so rather than implying P std made context durable. **Template sync.** The ad-hoc-briefs paragraph, deferred since #12, is now in the scaffolded template — but not verbatim. The repo paragraph links a file /workflow-init never scaffolds and asserts this repo's own incident count, so downstream gets a neutral variant preserving both halves of the principle: briefs carry the checklist's habits, and nobody reviews a brief against all 12 items. The canvas provenance line stays repo-only, per prompt-standards item 8. **Bot completion signal.** #12 and #13 both merged heads that were never reviewed: the check passed while the comment read "Review rate limited", and on #13 the only CodeRabbit review record names eed589c while the merged head was 92de0d2. docs/pr-review-bots.md now separates "the check stopped pending" from "the head was reviewed" across all three sites, with a verification command that was found broken by running it — gh api --slurp is rejected with --jq — and is now measured in both directions. Gate A: 8 passes, clean. Gate B: 5 passes, final spec and quality both clean on an unchanged tree. * Correct the process-pr-review step reference in the Finding A story CodeRabbit on #14: the parked Finding A entry said the mandated ledger check lives in `process-pr-review` step 4. It is step 5 — step 4 is the stop-and-ask-the-user step. Verified against the command itself, which this PR does not modify. Validated by hand rather than by subagent: #14 touches CLAUDE.md and plugins/, so process-pr-review's instruction-path precheck bars triage. Left alone deliberately: docs/superpowers/specs/2026-07-18-... also says step 4. That is a historical artifact recording what was true when written, this PR does not touch it, and docs/superpowers/ is excluded from the conformance checks for exactly that reason. Gate B triviality skip, documented per CLAUDE.md §5: a one-word correction to a backlog entry, no behaviour and no mechanism changed. * Record the fourth completion-signal observation (#14) #12 and #13 merged unreviewed heads and the miss was found afterwards. On #14 the verification query caught it BEFORE merging: check green, comment rate-limited, zero qualifying reviews for the head. Re-trigger produced nothing; merged on an explicit human decision with the exception recorded. Gate B triviality skip, documented per CLAUDE.md §5: prose-only addition to a descriptive table's surrounding notes, no mechanism or routing changed.
dsnger
added a commit
that referenced
this pull request
Jul 27, 2026
…ot a hazard Fourth occurrence (#12, #13, #15, #16 — the last observed while writing this row): the status check passes while the comment reads "Review rate limited" and the live head has no review record. The row now says never merge on the check alone; the review count is the arbiter, and the verification command runs on every merge rather than when something looks off. Docs-only (docs/**.md), so Gate B is N/A per CLAUDE.md §5's prose exemption.
dsnger
added a commit
that referenced
this pull request
Jul 27, 2026
* harden: three classes from the profiles cycle, at the rungs that fit Runs dev-workflow:harden-finding on the three classes that recurred through the risk/security/validation profiles work. All three land as text; none reaches a mechanical rung, and each ledger row says so rather than implying otherwise. unverified-enforcement-claim, 4th occurrence, rung P — prompt-standards item 11 gains two rules: where the reader can reach the authoritative source, cite it instead of restating it (with invariant 8's self-contained-template exception named), and delete a mechanism claim that has needed a fourth correction rather than refining it again. From a paragraph describing the hook's path matcher that took four consecutive Gate-B corrections, each a subtler version of the last, and closed only when the enumeration was deleted. rewrite-drops-prior-condition, NEW class, rung 1 — a new AGENTS.md Don't: never replace a decision procedure without listing its old conditions and marking each kept, moved, or deliberately dropped. Ten instances in one cycle, one of which briefly made an eligible profile sufficient for a Gate-B skip: a gate-off path invented by the change that exists to close one. docs-drift, 4th occurrence with a new mechanism, rung P — a standing lens on every Gate-B call in both §5 copies: "which existing statements does this diff falsify?" It found a shipped command that would have let a one-line fix skip Gate B, plus two docs teaching a rule the same change had narrowed. Two escalations deliberately refused, with the reasoning in the rows: both lineages' latest entries are rung-2 checks guarding one spelling each (Target-model lines; prose count claims), and both new defects fall outside those spellings — the over-escalation those rows warn about by name. Gate B: 7 findings at pass 1, then 1, 1, and clean on both branches at pass 4. Three of those findings were this change committing the classes it hardens — the item-11 fix reaching only the repo copy, the new rule contradicting invariant 8, and the lens claiming no check could reach the class. Verification: full battery green — shellcheck (6 files), hook tests, check-invariants + suite (123 assertions), check-version-bump + suite (36 assertions), claude plugin validate --strict; exit 0. * docs(bots): the CodeRabbit rate-limit pattern is settled behaviour, not a hazard Fourth occurrence (#12, #13, #15, #16 — the last observed while writing this row): the status check passes while the comment reads "Review rate limited" and the live head has no review record. The row now says never merge on the check alone; the review count is the arbiter, and the verification command runs on every merge rather than when something looks off. Docs-only (docs/**.md), so Gate B is N/A per CLAUDE.md §5's prose exemption. * docs(taxonomy): clarify the rewrite-drops-prior-condition definition PR #16 review finding: "one the old prose carried" was ambiguous; it now reads "one of the conditions the old prose carried". Docs-only (docs/**.md), so Gate B is N/A per CLAUDE.md §5's prose exemption. Validated directly rather than by a finding-triage subagent — the PR edits instruction-bearing paths, which the command's step-0 precheck routes to manual validation.
This was referenced Jul 27, 2026
dsnger
added a commit
that referenced
this pull request
Jul 27, 2026
The count rule carried two facts fused into one sentence. They separate: 1. The per-head count is the arbiter of whether a head was reviewed — unchanged, and the diagnostic for any bot. Routing a bot opportunistically does not make its review record less readable or the answer less true. 2. "Merge only on an explicit recorded human decision" binds only bots under Wait for. An absent review from an opportunistic bot blocks nothing and needs no exception: every head reaching a PR has already passed Gate B, the workflow's actual cross-model review, and the PR bots are a supplementary third instance. Requiring a recorded exception whenever a supplementary reviewer stayed quiet would reimpose as ceremony the blocking that opportunistic routing removed. The clause is dormant, not retired — Wait for is empty, so nothing triggers it; promote a bot back and it reactivates unchanged for that bot. The #12–#18 history and the #14 precedent stay as the record of when it was live: a real requirement under the routing of its time. Four sentences elsewhere attached a merge consequence to a `0` and are corrected to state the routing dependency: the scope paragraph above the completion-signal list, the "decides whether you may merge" bullet, "`0` means do not merge", and the "run it on every merge" lead-in to the query. Gate B: N/A — the only staged path is explanatory documentation (docs/pr-review-bots.md) per §5's prose rule. Battery green: shellcheck (6 files), hook tests, invariant tests + check (123 assertions), version-bump tests + check (36 assertions), `claude plugin validate . --strict`.
dsnger
added a commit
that referenced
this pull request
Jul 27, 2026
Docs-only, two files, no plugin path — no version bump. CodeRabbit leaves Wait for after five consecutive unreviewed heads (#12, #13, #15, #16, #17 — the last with zero review records on the PR) followed by a genuine review on #18. Real findings source, unpredictable delivery, and a completion signal that fires regardless of whether a review happened: the opportunistic category by this file's own definition. Wait for is now empty. Row additions: the status check goes green whether or not a review happened, and `@coderabbitai review` is a no-op while automatic reviews are active (CodeRabbit's own message on #17), which retroactively explains #14's "re-trigger produced nothing". Plan corrected to Free; "Pro Plus" was observed on PR #1 only. The count rule's two facts separated: the per-head count remains the arbiter of whether a head was reviewed, for any bot; the recorded-human-decision requirement binds only bots under Wait for, and is dormant while that list is empty. Every head reaching a PR has already passed Gate B, so a quiet supplementary reviewer needs no exception. MANIFEST.md: the bare `CLAUDE.md` row resolved to the repo root and produced a false Major on #18. Qualified to source-files/CLAUDE.md, with the three files distinguished, and root §6 (context canary) recorded as deliberately outside the §1–§5 template range and never to be synced into the scaffolded template. Pre-merge diagnostic: per-head count 1 on head 2634bf2 — reviewed. One Minor finding (drop or explicitly optionalize the re-trigger step) collected, not actioned, per §5's Minor/Nit rule. Gate B: N/A — every path is explanatory documentation per §5's prose rule. Battery green at each commit.
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
1 participant
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.
Adds docs/sparring-briefing.md (role, rules, prompt conventions for the sparring layer), extends prompt-standards scope to ad-hoc task briefs, and wires the briefing into the model-change revalidation row in todos.md. Docs-only — no plugin paths touched, no version bump required.
Summary by CodeRabbit