/nr declares the audit lens's scope as an argument, not a paragraph the skill overrides - #156
Conversation
`nr.md` step 5 described the lens's scope correctly and at length — the changed lines plus the callees, callers, siblings sharing the changed invariant and every current-behaviour claim, with "would understanding it change the ruling on THIS diff?" as the inclusion test, and explicit prohibitions on both the diff alone and a whole-repo audit. None of it survived the invocation. `Skill audit` loads a document whose own first rule is "whole-repo snapshot, never a diff", and once it is loaded `nr.md` is not in the room: on rain.deploy#21 the lens ran whole-repo, twelve findings, five bearing on the PR and seven in code the diff never touches, with the scope hand-typed as free text nothing could validate. - `/nr` now DECLARES the scope: `scope=pr:<number>` beside the `dir` and the changed-file list, from the three values the audit skill accepts (`whole-repo`, `pr:<number>`, `paths:<globs>` — the vocabulary its own run stamp records). - Every part of that argument comes from a typed result: the number out of the row's own `pr` field, the file list out of `pr_context.files` with its `filesTruncated` caveat, the tree out of `pr_checkout.dir`. A hand-assembled scope is the defect #132 removed one level up. - Step 5's inclusion test is kept as the REASON the PR scope admits the surrounding files it does, rather than as the only thing carrying the scope. - The presented ruling names the scope it was formed at, so a PR-scoped review is distinguishable from a whole-repo sweep without counting how many findings miss the diff. No lens means no scope, never a scope that would have been declared. - `whole-repo` stays reachable as a separate deliberate invocation on a repo somebody named, and `/nr` has no mode, flag or argument that declares it: a scope that defaults to the widest reading is indistinguishable from a chosen one, which is how this read as working. The scope vocabulary is one set of constants rather than a literal per caller, and a test pins that nr.md still declares a value from it and declares only the PR one. Blocked on rainlanguage/claude-audit-skills#66, which is what makes the skill parse the declaration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 36 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: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
QA run — checks and outputAll run from the pinned rainix shell CI itself uses 1.
|
The first version of the test asserted `scope=pr:<number>` appeared somewhere in `nr.md`. Moving the literal out of the invocation bullet into any other paragraph survived it — and a document that mentions the scope while the CALL does not pass it is precisely the #154 state. The assertion is now scoped to the bullet that invokes the skill, and a missing bullet panics with what it was looking for. Mutants killed, each at a distinct site: the declaration removed; the declaration reworded out of the bullet but left elsewhere; the invoke bullet renamed; the declared value changed to whole-repo or to a spelling outside the vocabulary; and a SECOND spelling introduced elsewhere while the bullet stays correct (which is what the per-value loop exists for). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🤖 ai:producer |
QA run — updated for
|
| mutant | change | killed at | message |
|---|---|---|---|
| M1 | scope=pr:<number> removed from the invocation bullet |
main.rs:29617 |
"the bullet that invokes the skill must DECLARE the PR scope as an argument…" |
| M2 | declared value → scope=whole-repo |
main.rs:29617 |
same assertion — the bullet stops declaring the PR scope |
| M3 | declared value → scope=diff-only |
main.rs:29617 |
same assertion |
| M4 | literal reworded out of the bullet, left in 3 other paragraphs | main.rs:29617 |
survived the previous test; killed now |
| M5 | the invocation bullet's anchor renamed | panic! |
"step 5 has no INVOKE bullet — the scope is declared in the invocation, so there is nowhere for it to be declared" |
| M6 | last occurrence → scope=whole-repo, bullet left correct |
main.rs:29651 |
"/nr passes "whole-repo", but the human PR gate declares only pr:<number>…" |
| M7 | last occurrence → scope=diff-only, bullet left correct |
main.rs:29643 |
"scope="diff-only" is not one of ["whole-repo", "pr:", "paths:"]…" |
M6 and M7 are the ones that keep the invocation correct and introduce a second
spelling elsewhere — the case the per-value loop exists for, and the reason it is
not folded into the bullet assertion. Four distinct assertion sites fire across the
seven, so nothing in the test is decoration. Verbatim panic output:
--- M1 declaration removed from the invocation bullet (prose-only) ---
the bullet that invokes the skill must DECLARE the PR scope as an argument; a scope
only described in prose is the #154 defect, and the skill's own whole-repo rule wins
over a description. The bullet reads: " with the `Skill` tool** — `audit` — and
DECLARE THE SCOPE\n AS AN ARGUMENT, not as something the reader is asked to
remember:\n the `dir` the checkout returned, and the changed-file\n list. …"
test result: FAILED. 0 passed; 1 failed
--- M5 the INVOKE bullet itself renamed ---
step 5 has no INVOKE bullet — the scope is declared in the invocation, so there is
nowhere for it to be declared
test result: FAILED. 0 passed; 1 failed
--- M6 second spelling elsewhere: last occurrence -> scope=whole-repo ---
panicked at pr-review-report-rs/src/main.rs:29651:13:
/nr passes "whole-repo", but the human PR gate declares only pr:<number>; whole-repo
and paths: belong to an invocation somebody asked for
test result: FAILED. 0 passed; 1 failed
--- M7 second spelling elsewhere: last occurrence -> scope=diff-only ---
panicked at pr-review-report-rs/src/main.rs:29643:13:
scope="diff-only" is not one of ["whole-repo", "pr:", "paths:"] — a spelling the
skill does not accept is free text again, which is what #154 removed
test result: FAILED. 0 passed; 1 failed
--- restored baseline ---
test result: ok. 1 passed; 0 failed
CI-equivalent checks, at 545648e
Everything from the pinned rainix shell CI uses
(nix develop github:rainlanguage/rainix/53e96a7d0a97d7c7c75c3b2412521324776fdac6#rust-shell),
with main.rs touched first so a warm target/ could not mask a clippy lint.
rainix-rs-static (rustfmt + clippy) STATIC_RC=0
pre-commit run --all-files
deadnix ....... Passed no-consumer-prettier ... Passed statix ... Passed
denofmt ....... Passed prettier-rainix ........ Passed taplo .... Passed
nil ........... Passed rustfmt ................ Passed yamlfmt .. Passed
nixfmt ........ Passed shellcheck ............. Passed
cargo test --release -p pr-review-report
test result: ok. 616 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
test result: ok. 47 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
plugin-version-lockstep ok human-fsm 0.6.0
The test is not passing by never running
repo_root_text returns None in the flake build sandbox, and a test that bails
there passes silently. Verified it executes:
cargo test -- --exact marketplace_tests::nr_declares_the_lens_scope_as_a_value_from_the_skills_own_vocabulary
running 1 test
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 615 filtered out
The grant set is unchanged, and Bash is still refused
No tool grant is added, so command_contract / command_check are untouched. The
tests pinning that all pass unchanged in the run above:
nr_grants_the_two_reads_the_source_it_audits_and_the_lens,
a_shell_grant_is_still_refused_beside_the_audit_lens (Bash last, Bash first,
bare Bash, Task, Write, Edit, Grep, Glob each refused beside a typed
read), the_native_admission_is_by_exact_name_not_by_prefix, and
the_audit_lens_alone_is_refused_because_the_subject_arrives_typed.
Not verified here
The audit skill does not yet parse scope=
(rainlanguage/claude-audit-skills#66 is open with no PR),
so there is no end-to-end run showing the skill honouring the declaration. What is
verified is this side of the contract: the value is declared in the invocation,
built from typed results, one of the three literals, and the PR one.
"The skill accepts exactly three scopes" is a claim about a parse that claude-audit-skills#66 has not landed yet — a stated current behaviour the source contradicts, which is the false-premise finding this very command teaches its reader to look for. What IS true today is the vocabulary: the three values the skill's run stamp records, under the field name `scope`. Both `nr.md` and the README now say that, and both stay true after #66 lands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
claude-audit-skills#67 (the PR for #66, opened while this was in flight) settles the caller-side spelling: "Scope is an input the caller declares … It is exactly one of three literals, and there are no others" — `whole-repo`, `pr:<number>`, `paths:<comma-separated globs>`. The string `scope=` appears nowhere in it. The key was derived from the run stamp's FIELD name and passed off as the argument form, which is inventing a fourth spelling in the middle of a change whose whole point is that the vocabulary is closed — and #155's gate compares a recorded scope against those literals, so a keyed value is one the gate refuses. So `/nr` declares the literal `pr:<number>`. The closed vocabulary moves into its own bullet, which lets the test assert the invocation bullet declares the PR scope and NAMES NO OTHER — the quoted whole-repo RULE is prose, a backticked literal is a declaration, and the two are now distinguishable. A third assertion requires the file to state all three literals, so "declare a literal" has the list behind it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Re-mutating found a survivor: deleting `whole-repo` from the vocabulary bullet still passed, because the file names `whole-repo` elsewhere — in the section about the scope this command deliberately does NOT pass. A mention is not a statement of the closed set, so the assertion moves onto the bullet that states it, and a file with no vocabulary bullet at all panics saying what is missing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
QA run — final, at
|
| mutant | change | killed by |
|---|---|---|
| M1 | declared literal removed from the invocation bullet | "the bullet that invokes the skill must DECLARE pr:<number> as the scope…" |
| M2 | declared literal → whole-repo |
same assertion |
| M3 | declared literal → diff-only |
same assertion |
| M4 | a SECOND scope (whole-repo) added beside the right one |
"the invocation bullet also names whole-repo. /nr rules on ONE PR…" |
| M5 | the invocation bullet's anchor renamed | "step 5 has no INVOKE bullet — the scope is declared in the invocation, so there is nowhere for it to be declared" |
| M6 | paths: dropped from the vocabulary bullet |
"the vocabulary bullet omits "paths:"…" |
| M7 | whole-repo dropped from the vocabulary bullet |
"the vocabulary bullet omits "whole-repo"…" (survived the previous revision) |
| M8 | the vocabulary bullet's anchor renamed | "nr.md states no scope VOCABULARY — a command told to declare a literal, and not told which literals exist, is one invented spelling away from free text again" |
Four distinct assertion sites fire across the eight, so no assertion is decoration.
M4 is the one that needs the prose/literal distinction to work at all: the
invocation bullet quotes the skill's whole-repo rule in italics, and the
assertion matches the backticked literal, so a quoted rule is not read as a declared
value.
Verbatim run:
--- M0 baseline --- test result: ok. 1 passed; 0 failed
--- M1 --- test result: FAILED. 0 passed; 1 failed
--- M2 --- test result: FAILED. 0 passed; 1 failed
--- M3 --- test result: FAILED. 0 passed; 1 failed
--- M4 --- the invocation bullet also names `whole-repo`. /nr rules on ONE PR, so it
declares `pr:<number>` and nothing else — a whole-repo sweep is an
invocation somebody asked for, never one this gate can reach
test result: FAILED. 0 passed; 1 failed
--- M5 --- step 5 has no INVOKE bullet — the scope is declared in the invocation, so
there is nowhere for it to be declared
test result: FAILED. 0 passed; 1 failed
--- M6 --- test result: FAILED. 0 passed; 1 failed
--- M7 --- test result: FAILED. 0 passed; 1 failed
--- M8 --- test result: FAILED. 0 passed; 1 failed
--- restored baseline --- test result: ok. 1 passed; 0 failed
git status --short (clean)
CI-equivalent checks
From the pinned rainix shell CI uses
(nix develop github:rainlanguage/rainix/53e96a7d0a97d7c7c75c3b2412521324776fdac6#rust-shell),
with main.rs touched first so a warm target/ could not mask a clippy lint.
rainix-rs-static (rustfmt + clippy) STATIC_RC=0
pre-commit run --all-files
deadnix ....... Passed no-consumer-prettier ... Passed statix ... Passed
denofmt ....... Passed prettier-rainix ........ Passed taplo .... Passed
nil ........... Passed rustfmt ................ Passed yamlfmt .. Passed
nixfmt ........ Passed shellcheck ............. Passed
cargo test --release -p pr-review-report
test result: ok. 616 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
test result: ok. 47 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
plugin-version-lockstep ok human-fsm 0.6.0
denofmt reflowed both markdown files on each pass and the reflow is in the
commits, so the hook is clean on a re-run rather than clean-by-not-looking.
The QA block was validated by the gate's own predicate
pr-review-report repair-qa-block rainlanguage/issue-pr-cron 156 \
--block-file <the body's ## QA section> --replace --dry-run
rainlanguage/issue-pr-cron#156: body already carries this exact QA block — nothing to write
The test is not passing by never running
repo_root_text returns None in the flake build sandbox, and a test that bails
there passes silently. Verified it executes:
cargo test -- --exact marketplace_tests::nr_declares_the_lens_scope_as_a_value_from_the_skills_own_vocabulary
running 1 test
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 615 filtered out
The grant set is unchanged, and Bash is still refused
No tool grant is added, so command_contract / command_check are untouched. The
tests pinning that all pass unchanged in the run above:
nr_grants_the_two_reads_the_source_it_audits_and_the_lens,
a_shell_grant_is_still_refused_beside_the_audit_lens (Bash last, Bash first,
bare Bash, Task, Write, Edit, Grep, Glob each refused beside a typed
read), the_native_admission_is_by_exact_name_not_by_prefix, and
the_audit_lens_alone_is_refused_because_the_subject_arrives_typed.
Two things a reviewer should not read as verified
- CodeRabbit's green is vacuous on this PR. Its check reports
pass, and its
comment says "Review limit reached … you've reached your PR review limit, so we
couldn't start this review." That is therate-limitedcoverage value, not a
review, so "0 unresolved threads" here means nothing was looked at. Worth
re-triggering before merge. - No end-to-end run shows the skill honouring the declaration, because
audit: scope is a declared input with three literals; the bendless rule is never diff-only claude-audit-skills#67 is unmerged. What is
verified is this side of the contract: the literal is declared in the invocation,
built from typed results, one of the three, and the PR one.
|
Reviewed 888724c: ready — /nr declares the bare pr: literal built from typed results, form verified compatible with #157's LensScope parser, whole-repo kept as a separate explicit invocation, mutation re-run from a committed baseline killed both real survivors. Blocked-by claude-audit-skills#67 is resolving on the human's own merge word for both. |
2 participants
Closes #154
nr.mdstep 5 described the audit lens's scope correctly and at length — thechanged lines plus the callees, callers, siblings sharing the changed invariant
and every current-behaviour claim, with "would understanding it change the ruling
on THIS diff?" as the inclusion test, and explicit prohibitions on both the diff
alone and a whole-repo audit. None of it survived the invocation.
Skill auditloads a document whose own first rule is "Whole-repo snapshot, never a diff. …Do
not scope by recent changes / PR diff.", and once that is loaded
nr.mdis not inthe room. Measured on rainlanguage/rain.deploy#21 the lens
ran whole-repo: twelve findings, five bearing on the PR and seven in code the diff
never touches, with the scope hand-typed as free text in an args string that
nothing could validate.
What changed
/nrinvokes the skill with the literalpr:<number>as its declared scope, beside thedirthe checkout returned andthe changed-file list — a value, not a paragraph a reader is asked to remember.
literals —
whole-repo,pr:<number>,paths:<comma-separated globs>— thesame strings its run stamp records verbatim, and they live in one bullet of their
own so "declare a literal" has the closed list behind it. There is no
scope=key: an earlier revision of this PR invented one from the run stamp's field
name, which is a fourth spelling in the middle of a change whose whole point is
that the vocabulary is closed. Corrected in
f5b5964onceaudit: scope is a declared input with three literals; the bendless rule is never diff-only claude-audit-skills#67 settled the
caller-side form.
<number>is the one inside the row's ownprfield (owner/repo#n, the same string step 2 was addressed with), thechanged-file list is
pr_context.fileswhole with itsfilesTruncatedcaveatstated, and the tree is
pr_checkout.dir. A scope the reader assembled is thesame defect class
#132removed one level up: it looks identical to a derivedone and nothing downstream can tell them apart.
says
pr:<number>; the prose explains which surrounding files that admits andwhy — callees, callers, siblings sharing the invariant, and every
current-behaviour claim the PR or issue makes.
a reader can tell a PR-scoped review from a whole-repo sweep without inferring it
from the proportion of findings that miss the diff. A failed checkout means no
lens and no scope — never a scope that would have been declared.
AUDIT_SCOPE_WHOLE_REPO/AUDIT_SCOPE_PR/AUDIT_SCOPE_PATHSand theAUDIT_SCOPESset they compose, besideMCP_COMMAND_NATIVE_TOOLS. The value/nrdeclares, the value a lens ledgerrecords and the value a gate refuses on are definitionally the same three strings
— The lens ledger records that the skill ran but not at what scope, so a whole-repo or diff-only review passes NoLens #155 gates a verdict on
them, and a second copy is a second spelling waiting to drift.
0.5.0 → 0.6.0in both the manifest and the marketplace listing (acontent change is a release), and the README's
/nrsection records why thescope is an argument.
No tool grant changes, so
command_contract/command_checkare untouched andthe tests pinning that
Bashis refused beside a typed read pass unchanged.The open question the issue raises: is a whole-repo audit worth keeping?
Yes, and not in
/nr. It stays available as a separate, explicit invocation ofthe same skill with
whole-repodeclared, on a repo somebody named — the case theissue describes, a repo you are about to take a dependency on.
/nrgets no mode,flag or argument that reaches it. Three reasons:
/nrrules on one PR. Its lens exists to decide THAT merge. A whole-reposweep answers a question nobody asked at this gate, and it answers it by burying
the findings that bear on the diff under the ones that do not — five among
twelve, measured. The human gate is exactly where that dilution is most
expensive, because the next action is a merge.
findings as
audit-labelled issues and writes awhole-reporun stamp thatrain-org-healthreads as audit recency;/nrwrites neither and rules on a PRinstead. Folding the two into one command would mean one invocation whose
obligations depend on an argument, which is the shape this issue is about.
that defaults to the widest reading is indistinguishable from one that was
chosen. So there is no default to get wrong: the value is passed on every
invocation, and the only scope
/nrcan pass is the PR one — pinned by a test,not by this paragraph.
whole-repois therefore reachable, never reachable by omission. It is stated innr.mdunder its own heading so the next reader does not have to re-derive it.claude-audit-skills#67reaches the same answer from the other side, which isworth recording: a request whose subject is a PR never resolves to
whole-repo,while a caller may still explicitly declare it — "a repo someone is about to
depend on". The two changes agree without having been written together.
Blocked by: the skill must accept the declaration
rainlanguage/claude-audit-skills#66 is open, and its
PR rainlanguage/claude-audit-skills#67 is open and
unmerged. Until it lands, the shipped skill still carries the bendless
"whole-repo snapshot, never a diff" rule and reads a declared scope as ordinary
args text, so
/nrdeclares a scope nothing yet parses.This PR is built against that contract rather than around it — the three literals,
declared as a value. Restating step 5 more firmly is the thing that already failed,
and it is not what is here. The blocked half is one repo away and nothing in this
diff needs to change when it lands.
Not in this PR
review-prompt.txt's AUDIT LENS step carries the identical prose-only scope forthe vetter, and #155 — the ledger row that records the declared scope and the
record_gatethat refuses a wrong one — is the change that gives that sidesomething to record. Kept out of this diff deliberately: #155 is in flight against
this same repo and the vetter's lens paragraph is one line both changes would have
to edit. If #155 lands without touching it, that is a follow-up issue, not a silent
gap.
QA
marketplace_tests::nr_declares_the_lens_scope_as_a_value_from_the_skills_own_vocabulary— fails on base (the pre-change
nr.mddeclares no scope literal in itsinvocation at all; verified by reverting that bullet to prose-only, mutation M1
below).
marketplace_tests::nr_grants_the_two_reads_the_source_it_audits_and_the_lensstill pins the grant set unchanged —
SkillandReadbeside four typed reads —and
a_shell_grant_is_still_refused_beside_the_audit_lensstill refusesBash,both passing untouched, because this change adds no grant.
nr.md; each restored and therestore verified with
git diff --quietbefore the next, baseline re-run green):bullet that invokes the skill must DECLARE
pr:<number>as the scope"whole-repo→ killed by the same assertiondiff-only→ killed by the same assertionwhole-repo) added beside the right one in the invocationbullet → killed, "the invocation bullet also names
whole-repo" — the assertionthat exists because a quoted whole-repo RULE is prose while a backticked literal
is a declaration
— the scope is declared in the invocation, so there is nowhere for it to be
declared"
paths:dropped from the vocabulary bullet → killed, "the vocabulary bulletomits "paths:""
whole-repodropped from the vocabulary bullet → killed. This mutantSURVIVED an earlier revision whose assertion was over the whole file: the file
names
whole-repoelsewhere, in the section about the scope this commanddeliberately does not pass, and a mention is not a statement of the closed set.
Fixed in
888724c.VOCABULARY"
the test is decoration.
SKILL.mdatrainlanguage/claude-audit-skillsHEAD for the run stamp's three scope values, and
audit: scope is a declared input with three literals; the bendless rule is never diff-only claude-audit-skills#67 for the caller-side
form ("Scope is an input the caller declares … It is exactly one of three
literals, and there are no others"). Read out of those files, not from what this
repo would find convenient to pass — which is how the invented
scope=key wascaught rather than shipped.
prose, (b) step 5's inclusion test kept as the reason for the scope, (c) the scope
built from
pr_context/pr_checkoutresults and never hand-assembled, (d) thepresented ruling stating the scope the lens ran at, (e) a decision on whether a
whole-repo audit is worth keeping and, if so, only as an explicit invocation. All
five covered — (a)
pr:<number>declared in the step-5 invocation bullet, (b) the"what
pr:<number>admits, and why" bullet, (c) the "every part of that argumentcomes from a typed result" bullet, (d) the new paragraph in "Present the result"
plus the carry-through bullet in step 5, (e) the
whole-reposection innr.mdand the decision section of this body. The skill-side parse is human-queue: emit per-state
owner(single source of truth) + split the two dual-owner states (human:reject, ai:blocked-on) #66/prompts: require a screenshot (or a stated why-not) for GUI claims #67 and isnamed as a blocked-by rather than worked around.