Give two recurring prompt-conformance classes their mechanical rung - #13
Conversation
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.
|
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: 6 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 (1)
📝 WalkthroughWalkthroughAdds two Markdown-only prompt-conformance checks for model declarations and checklist-count claims, extensive regression fixtures, CI labeling, invariant documentation, hardening records, and completion tracking. ChangesPrompt Conformance
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant CI
participant Checker as check-invariants.sh
participant Prompts as Markdown prompt artifacts
participant Standards as docs/prompt-standards.md
participant Template as workflow-init template
CI->>Checker: Run invariant checks
Checker->>Prompts: Scan model declarations and count claims
Checker->>Standards: Parse checklist length
Checker->>Template: Parse checklist length
Checker-->>CI: Return validation status
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 two mechanical prompt-conformance checks and integrates them into the existing invariant gate.
Confidence Score: 5/5The pull request appears safe to merge, with no concrete correctness, security, or independently actionable quality issue identified. The new checks fail closed on scan and parsing errors, validate the intended constrained spellings, preserve the existing invariant behavior, and are backed by targeted regression fixtures for malformed input and exclusion boundaries. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Run invariant checker] --> B[Scan conforming Markdown files]
B --> C{Exactly one recognized target model?}
C -- No --> F[Fail invariant gate]
C -- Yes --> D[Parse both checklist definitions]
D --> E{Definitions valid and counts agree?}
E -- No --> F
E -- Yes --> G[Scan prose checklist-count claims]
G --> H{Every claim matches checklist count?}
H -- No --> F
H -- Yes --> I[Invariant gate passes]
Reviews (1): Last reviewed commit: "Give two recurring prompt-conformance cl..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/check-invariants.sh (1)
441-452: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDigit-claim comparison doesn't use the same string-forcing idiom as its sibling check.
prompt_checklist_count(line 408) deliberately writes$0 "" != n ""to force a string compare and avoid awk's numeric/strnum coercion on-v-assignedn— the header comment explicitly credits this to guarding against overflow on long digit runs. The claim validator's digit branch does the equivalent comparison as a baretok != n(line 448), without that same guard.Under strict POSIX semantics
tok(built viasub()) should already be a plain "string" type rather than a "numeric string", sotok != nlikely already resolves as a string compare today — but that safety currently depends on an implicit, easy-to-regress typing distinction rather than the explicit idiom this same file uses elsewhere for exactly this reason. Given the stated design goal ("never+0, so a 40-digit claim cannot overflow its way to a wrong verdict"), the same explicit idiom should be applied here for consistency and to remove reliance on awk-implementation-specific type-inference behavior.🛠️ Proposed fix
- if (tok != n) print $0 " <- checklist has " n + if (tok "" != n "") print $0 " <- checklist has " nWorth confirming across the awk implementations this script may run under (mawk, busybox awk, BSD/macOS awk) that
sub()-derived values are never treated as numeric strings, since this repo already treats BSD-vs-GNU regex differences as a real, previously-bitten class of bug.🤖 Prompt for 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. In `@scripts/check-invariants.sh` around lines 441 - 452, Update the digit comparison in the claim validator’s awk block to use the same explicit string-forcing comparison idiom as prompt_checklist_count, comparing tok and n as strings without numeric coercion or arithmetic conversion. Keep the existing canonical-number validation and mismatch reporting unchanged.
🤖 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.
Nitpick comments:
In `@scripts/check-invariants.sh`:
- Around line 441-452: Update the digit comparison in the claim validator’s awk
block to use the same explicit string-forcing comparison idiom as
prompt_checklist_count, comparing tok and n as strings without numeric coercion
or arithmetic conversion. Keep the existing canonical-number validation and
mismatch reporting unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9fb813ab-e333-41ae-81f8-e269a5b1343c
📒 Files selected for processing (9)
.github/workflows/ci.ymlAGENTS.mdREADME.mddocs/architecture.mddocs/hardening-log.mddocs/superpowers/plans/2026-07-26-prompt-conformance-checks.mdscripts/check-invariants.shscripts/check-invariants.test.shtodos.md
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.)
|
Processed — one tracked claim, accepted and fixed in 92de0d2. This PR touches CodeRabbit — digit-claim comparison lacks the string-forcing idiomAccept. Verified: Not a live bug, and the reply should be precise about that. The Gate B triviality skip, documented in the commit per CLAUDE.md §5 — one Not hardened. No ledger class fits a style-consistency nit whose behaviour One thing worth recording: my first attempt at that explanatory comment wrote Greptile5/5, no actionable issue — no defect claim, so nothing tracked. |
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.
#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.
* 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.
…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.
* 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.
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.
1 participant
PR 1 of a two-PR split from the canvas field-findings hardening round. This is
scripts/plus the doc sites describing those checks. Noplugins/**path istouched, so invariant 12 requires no version bump here — PR 2 carries the §5
cycle-close nudge, the optional companion files, the scaffolded-template sync and
the 0.6.0 bump.
What this resolves
Two ledger rows dated 2026-07-25 sat at rung
pending, both at a thirdoccurrence, both naming this work as their resolution vehicle. Both are resolved
here by appended rung-2 rows; the
pendingrows are byte-unchanged.unverified-enforcement-claim*.mdcarryingprompt artifact and follows: exactly one column-zeroTarget model:line, whose value begins with a recognized token and names exactly one distinct modeldocs-driftall <digits|one..twenty>( checklist)? items) equals the checklist it counts, in both definitions, which must agreeTwo design points that decided whether these are real:
Target model: any capable chat model— the exact PR docs: sparring briefing for the upstream advisor chat #12 defect. Tokenanywhere accepts it too, because the real defect line named Claude as
provenance. Measured: anywhere → 1 match, at-start → 0.
A digit-only check would have missed the very defect the row claims to harden.
The ledger is excluded from both checks. A ledger that quotes defects
self-rejects the checks that detect them —
docs/hardening-log.mdcarries thehistorical count claim as evidence, and scanning it would fail the repository
forever on rows that exist to record the fix.
Mutation evidence (documented manual run — nothing automates it)
Procedure and re-run trigger are in the checker header; the mapping is also
recorded in the test file so it travels with the code.
check 4ablock4a:rejects + 4exclusion: neighbouring …+4a value extraction failure firescheck 4bblock4b:rejects + 44b exclusion: neighbouring …+checklist parser failure fires+4b claim validator failure firesBaseline exit 0, mutant exit 1 in both. No accept case moved and nothing
unrelated moved.
scan error firesand4a/4b exclusion filter failure firesflip in neither, because they break a stage both checks share — expected, not an
omission.
Suite
61 → 123 assertions. A shared fixture initializer landed first: without it,
25 of the 61 pre-existing assertions failed on a missing checklist before
reaching their own assertion, so accept cases turned red and reject cases started
passing for the wrong reason.
Review
Gate A: 9 passes on the plan. Gate B: 8 passes, 13 findings, final spec and
quality passes both clean against an unchanged tree.
Every Gate-B finding was one of two shapes — a claim outrunning the code, or a
path where the gate could report success without doing its work. The second
family took four passes and was closed at six depths: the recursive scan, the
post-scan exclusion filters, the claim validator, the checklist parser, the
per-file commands inside 4a, and the value-extraction pipeline, where
grep | head | sedexposed onlysed's status. Each has a fixture.AGENTS.mdinvariant 11 is narrowed accordingly: "nothing mechanical checksthem" stopped being true, and it now says no comprehensive checker exists and
names these two as a floor.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation