docs(skills): drop attestation vocabulary and unsourced figures from the implementer pipeline - #7571
Conversation
…the implementer pipeline Complements phase-rs#7570 (which strips the probe-floor receipt from the planner and reviewer skills). Closes the remaining post-merge review findings in the other two pipeline files: the executor report's 'attestation' vocabulary becomes plain checks the orchestrator re-verifies itself; the implementer's build-economy rules state their invariants instead of unsourced GB figures; and the surgical-mode note no longer records per-edit verification results — 'every exit is then auditable rather than asserted' was the receipt purpose in the receipt words. Assisted-by: ClaudeCode:claude-fable-5
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe changes update implementation and measurement instructions. They require explicit verification results, remove fixed build-measurement examples, and simplify surgical-fix record requirements. ChangesImplementation guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This documentation-only change removes misleading terminology and unsourced figures without changing runtime behavior; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
matthewevans
left a comment
There was a problem hiding this comment.
Changes requested — protected instruction-surface change.
This PR modifies .claude/agents/engine-implementation-executor.md and .claude/skills/engine-implementer/SKILL.md. Those files are protected agent/instruction surfaces under the repository review policy and require an explicit maintainer-owned review path; they are not eligible for normal contributor merge-queue handling.
This is a policy-routing block, not an implementation review of the proposed wording. Please obtain explicit maintainer direction for the protected-surface change or move the work to a maintainer-owned branch.
phase-rs#7570 replaced the build-withholding exception ritual with "because that isolation exists, a brief never needs to withhold builds". That is true of the thing an isolated `CARGO_TARGET_DIR` actually buys — the cargo target lock — and over-claimed for everything else. Isolation does not create disk or CPU. A fresh target dir costs capacity rather than saving it, and this repo has measured how much: `engine-implementer` records 17 of 28 GB of dead incremental state on one completion target and seven cold builds at 130 GB in a single phase. A planner reading the unconditional form would insist on probing a box that is genuinely out of room. Narrowed to the true claim and named the one case isolation cannot fix. No exception ritual returns: nothing here mandates fields, voids a clause, or makes a withheld build a reportable process defect — a full box is worth naming, and the probe happens once it clears. Companion to phase-rs#7570 and phase-rs#7571. Assisted-by: ClaudeCode:claude-opus-4.8
* docs(skills): scope the probe-isolation claim to lock contention #7570 replaced the build-withholding exception ritual with "because that isolation exists, a brief never needs to withhold builds". That is true of the thing an isolated `CARGO_TARGET_DIR` actually buys — the cargo target lock — and over-claimed for everything else. Isolation does not create disk or CPU. A fresh target dir costs capacity rather than saving it, and this repo has measured how much: `engine-implementer` records 17 of 28 GB of dead incremental state on one completion target and seven cold builds at 130 GB in a single phase. A planner reading the unconditional form would insist on probing a box that is genuinely out of room. Narrowed to the true claim and named the one case isolation cannot fix. No exception ritual returns: nothing here mandates fields, voids a clause, or makes a withheld build a reportable process defect — a full box is worth naming, and the probe happens once it clears. Companion to #7570 and #7571. Assisted-by: ClaudeCode:claude-opus-4.8 * docs(skills): scope the lock-contention exception to the target directory CodeRabbit review on #7572 (r3824636835): CARGO_TARGET_DIR isolation doesn't cover CARGO_HOME's registry/package-cache locks, a separate lock domain. Qualify "lock contention" to "target-directory lock contention" and note the shared-cache locks explicitly so the sentence can't be read as covering more than what target-dir isolation buys. Assisted-by: ClaudeCode:claude-sonnet-5
🤖 AI text below 🤖
Summary
Companion to #7570: closes the remaining post-merge review findings on #7502's pipeline prose. The executor report's "attestation" vocabulary becomes plain check results (the orchestrator re-verifies with
rev-parse/diffregardless), the implementer's build-economy rules state their invariants instead of unsourced GB figures, and the surgical-mode note stops recording per-edit verification results — "every exit is then auditable rather than asserted" was the receipt purpose in the receipt words, inside the file that prohibits receipts at line 30.Files changed
.claude/agents/engine-implementation-executor.md— "attestations"/"attestation" → "check results"/"check" in the report contract.claude/skills/engine-implementer/SKILL.md— invariants replace unsourced measured figures; surgical-mode record trimmed to what the loop reads back (switch, measurements, classification, exit reason)Track
Developer
LLM
Model: claude-fable-5
Tier: Frontier
Thinking: high
Implementation method (required)
Method: not-applicable — documentation-only change to agent skill/report prose; no
crates/engine/code touched.CR references
None.
Verification
Required checks ran clean, or the exact CI-owned alternative is stated below.
Gate A output below is for the current committed head.
Final review-impl below is clean for the current committed head.
Both anchors cite existing analogous code at the same seam.
scripts/check-parser-combinators.sh— Gate G PASS, Gate A PASS (head57873b788)pre-commit suite (Gates G/A/P) — all green at commit time
repo-wide grep for
attestationin.claude/,.agents/,AGENTS.md,CLAUDE.md— zero remaining occurrencesRust/TS builds — unaffected (docs-only diff), CI-owned checks on this PR
The final amend (
7757b31ce→57873b788) applies the review's own nit-1 prescription — removing the "measured" claims orphaned by the dropped figures; no other changeGate A
Gate A PASS head=57873b788f38db16672cc2a3d289441666a95049 base=b9f06c184d6cdfb1a95d935611bd0b9851ded0c3
Anchored on
HEAD == START_SHA/ clean-tree checks, which are why an executor-side "attestation" adds nothingFinal review-impl
Final review-impl PASS head=57873b788f38db16672cc2a3d289441666a95049
Claimed parse impact
None.
Scope Expansion
None.
Validation Failures
None.
CI Failures
None.
Summary by CodeRabbit