fix: the plain-English promise was kept in one skill of six (3.7.1) - #16
Conversation
Last item from the original handoff: seven reference files never reviewed. A tester said review-rubric.md "reads like undefined jargon trivia and contradicts its own skill's plain-English promise." The observation was right. The file it named was not the cause. The rubric does use ~15 undefined terms. It is also never shown to the owner. It decides what the review looks at, and the precision is doing work: "temporal coupling" names a failure that "order matters" does not. Softening a checklist to protect a reader who never sees it costs every review and buys nothing. The defect was one layer over. prod-readiness-coach has a glossary, six tone rules and a linter over its output. The two other skills the owner reads had one conditional half-sentence between them: "For beginner-facing reports, express evidence, impact, and recommendation as What / Why / Fix." Everyone this toolkit serves is beginner-facing, so the condition should never have existed. - Both owner-facing report formats carry the plain-English rules, and the What/Why/Fix instruction is unconditional. New rule: define a technical term inline, in one clause, on first use; if you cannot, you are using it to sound precise rather than to be understood. - validate-toolkit.sh fails if either format loses them. Proven by removing one and watching the build go red. - Both rubrics now say why they are engineer-voiced, so the next reviewer translates the report and not the checklist. - developer-handoff left alone on purpose: its reader is the developer receiving the project, and jargon is the correct register there. 130 tests. Decision 011.
|
Warning Review limit reached
Next review available in: 51 seconds Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
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 |
1 participant
The last open item from the original handoff: seven reference files that had never been reviewed. An independent tester had flagged one:
The observation was right. The file it named was not the cause.
The rubric does use about fifteen undefined terms: temporal coupling, characterization tests, dependency injection, helper fragmentation, evidence of variation or coupling. And the README row for that skill says "Review this code in plain English."
But that file is never shown to the project owner. It decides what the review looks at, and the precision is doing work. "Temporal coupling" names a specific failure that "order matters" does not. Softening a checklist to protect a reader who never sees it would cost the quality of every review and buy nothing.
So I did not do what the tester recommended. I found what they had actually noticed.
The defect was one layer over
prod-readiness-coachcheck_report.pylinting the outputclean-code-reviewproduct-readiness-reviewThat half-sentence:
Everyone this toolkit is built for is beginner-facing. There is no second, terser audience to switch to. The condition should never have existed.
A promise enforced in one skill of six and hoped for in the others is not a standard. That is what the tester felt, and it is why the rubric's vocabulary was leaking: a model mirrors the register of its instructions, and nothing was pulling the other way.
What changed
Both owner-facing report formats now carry the plain-English rules. Including one that is Tarik's own rule from
CLAUDE.md:Plus: never lead with jargon; use a real consequence not an abstract one; frame a gap as the next thing to learn; say a win plainly; short sentences.
The What / Why / Fix instruction is unconditional now.
validate-toolkit.shfails if either format loses the rules. Proven by removing one:Both rubrics now say why they are engineer-voiced, so the next reviewer translates the report and not the checklist:
What I deliberately did not touch
developer-handoff/references/handoff-template.md. Its reader is the developer receiving the project. Engineering vocabulary is the correct register there. Applying a plain-English rule to it would have been the same mistake pointing the other way.readiness-rubric.mdand both stack reference sets are good documents. The handoff template in particular is honest in ways worth keeping: "Unknowns for the owner: list questions that repository evidence cannot answer. Do not disguise them as facts."Being honest about the limit
I did not verify that these rules change the output. The five prompt-only skills still have no automated coverage. If a tester still calls the report jargon-heavy, the rules were not enough, and the next step is a linter over these two documents like the coach already has.
Full reasoning in
docs/decisions/011.Test plan
coverage_grid.py --fail-under 100exits 0--fail-on criticalon this repo, exit 0plugin.jsonandmarketplace.json(x2), CHANGELOG, decision 011