From abd4b41771b0072cb9908c7ea39fb7181f177f73 Mon Sep 17 00:00:00 2001 From: Tarik Moody Date: Sat, 22 Aug 2026 19:11:59 -0500 Subject: [PATCH] fix: the plain-English promise was kept in one skill of six (3.7.1) 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. --- .claude-plugin/marketplace.json | 4 +-- .claude-plugin/plugin.json | 2 +- CHANGELOG.md | 18 ++++++++++++ .../011-fix-the-report-not-the-rubric.md | 25 ++++++++++++++++ scripts/validate-toolkit.py | 29 ++++++++++++++++++- .../references/report-format.md | 26 ++++++++++++++++- .../references/review-rubric.md | 9 ++++++ .../references/readiness-rubric.md | 9 ++++++ .../references/report-format.md | 24 +++++++++++++++ 9 files changed, 141 insertions(+), 5 deletions(-) create mode 100644 docs/decisions/011-fix-the-report-not-the-rubric.md diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index e66a566..687d88c 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-marketplace.json", "name": "clean-code-toolkit", - "version": "3.7.0", + "version": "3.7.1", "description": "Clean-code and product-handoff tools for AI-assisted builders.", "owner": { "name": "Tarik Moody" @@ -10,7 +10,7 @@ { "name": "clean-code-toolkit", "description": "Review code, assess product readiness, refactor safely, and prepare a developer handoff.", - "version": "3.7.0", + "version": "3.7.1", "author": { "name": "Tarik Moody" }, diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 36c0c0f..6ed16cb 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "clean-code-toolkit", - "version": "3.7.0", + "version": "3.7.1", "description": "Practical clean-code, product-readiness, and developer-handoff workflows for AI-assisted projects.", "author": { "name": "Tarik Moody" diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b629b8..8e6abc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Changelog +## 3.7.1 + +The last item from the original handoff: seven reference files that had never been reviewed. An independent tester had said `review-rubric.md` "reads like undefined jargon trivia and contradicts its own skill's plain-English promise." + +**The observation was right and the file it named was not the cause.** The rubric does use about fifteen undefined terms, including temporal coupling, characterization tests and helper fragmentation. 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. + +**The actual defect was one layer over.** `prod-readiness-coach` has a glossary, six tone rules, and a linter checking the documents it produces. The two other skills whose output the owner reads had, between them, one conditional half-sentence: "For beginner-facing reports, express evidence, impact, and recommendation as What / Why / Fix." Everyone this toolkit is built for is beginner-facing, so the condition should never have existed. A promise enforced in one skill of six and hoped for in the others is not a standard. + +**Both owner-facing report formats now carry the plain-English rules.** Never lead with jargon. Define a technical term inline, in one clause, the first time you use it; if you cannot define it in a clause, you are using it to sound precise rather than to be understood. Use a real consequence, not an abstract one. Frame a gap as the next thing to learn. Say a win plainly. Short sentences. `validate-toolkit.sh` fails if either format loses them. + +**Both rubrics now say why they are written for an engineer**, so the next reviewer translates the report rather than the checklist. + +**`developer-handoff` was deliberately left alone.** Its reader is the developer receiving the project, and engineering vocabulary is the correct register there. Applying a plain-English rule to it would have been the same mistake pointing the other way. + +See `docs/decisions/011`. + +Tests: 130. + ## 3.7.0 A review asked a question nobody had asked: what happens when the repository being audited contains text aimed at the agent auditing it. diff --git a/docs/decisions/011-fix-the-report-not-the-rubric.md b/docs/decisions/011-fix-the-report-not-the-rubric.md new file mode 100644 index 0000000..2f28d72 --- /dev/null +++ b/docs/decisions/011-fix-the-report-not-the-rubric.md @@ -0,0 +1,25 @@ +# 011: Fix the report, not the rubric + +**Decision.** The two rubrics stay written for an engineer. The plain-English rules move to the two report formats whose output the project owner reads, they stop being conditional, and `validate-toolkit.sh` fails if either format loses them. + +**Why this came up.** An independent tester said `skills/clean-code-review/references/review-rubric.md` "reads like undefined jargon trivia and contradicts its own skill's plain-English promise". Reading it, the vocabulary complaint is fair: about fifteen terms appear undefined, including temporal coupling, characterization tests, dependency injection, helper fragmentation and evidence of variation or coupling. The README row for that skill says "Review this code in plain English." + +What was at stake is the toolkit's central promise to an audience that is learning. A finding somebody cannot understand is a finding they cannot fix. + +**Options.** +1. Rewrite the rubrics in plain English, as the tester suggested. Cost: the rubric is what makes the review competent, and the precision is doing work. "Temporal coupling" names a specific failure that "order matters" does not. Softening the checklist to protect a reader who never sees it would buy nothing and cost the quality of every review. +2. Leave it. The rubric is internal, the model understands the terms, and the report is a separate document. Cost: this is what was already happening, and it produced the complaint. A model mirrors the register of its instructions, so a checklist in dense engineering prose pulls the report toward dense engineering prose, and nothing was pulling the other way. +3. Separate the two jobs explicitly. The rubric decides what to look at and stays technical. The report format decides how to say it and carries the plain-English rules. Say so in both files, so the next reviewer does not translate the wrong one. + +**What we chose and why.** Option 3. Joint call, and it disagrees with the tester's recommendation while accepting the observation behind it. The complaint was real; the file it named was not the cause. + +Looking closer found the actual defect. `prod-readiness-coach` has a glossary, six tone rules, and `check_report.py` linting the documents it produces. The two other skills the owner reads had, between them, one conditional half-sentence: "For beginner-facing reports, express evidence, impact, and recommendation as What / Why / Fix." Everyone this toolkit is built for is beginner-facing, so the condition never should have existed. **A promise enforced in one skill of six and hoped for in the others is not a standard.** + +The `developer-handoff` template was deliberately left alone. Its reader is the developer receiving the project, and engineering vocabulary is the correct register there. Applying a plain-English rule to it would have been the same error in the other direction. + +**What we gave up.** The tone rules now exist in three places: the coach's glossary and both report formats. That is real duplication and it will be tedious to reword. A rule that has to be followed while writing has to be in the same context as the writing, and the validator makes drift a build failure rather than a slow decay. We also did not verify that the rules change the output, because the five prompt-only skills still have no automated coverage. + +**How we'll know if this was right.** A review of a repository reads like something the owner can act on, and the terms in the rubric appear in the report only with a definition attached. If a tester still says the report is jargon-heavy, the rules were not enough and the next step is a linter for these two documents, like the coach already has. + +**What actually happened.** +(Tarik fills this in.) diff --git a/scripts/validate-toolkit.py b/scripts/validate-toolkit.py index 9167144..855509e 100755 --- a/scripts/validate-toolkit.py +++ b/scripts/validate-toolkit.py @@ -159,6 +159,32 @@ def validate_installer() -> None: raise ValidationError("Installer did not preserve a legacy Clean Code Standards section") +PLAIN_ENGLISH_HEADING = "## Write it so the owner can act on it" +# Formats whose output the project owner reads. A handoff template is excluded on +# purpose: its reader is the developer receiving the project, and engineering +# vocabulary is the correct register there. +OWNER_FACING_FORMATS = ( + "skills/clean-code-review/references/report-format.md", + "skills/product-readiness-review/references/report-format.md", +) + + +def validate_plain_english_rule() -> None: + """The toolkit promises plain English and enforced it in one skill of six. + + `prod-readiness-coach` has a glossary, six tone rules, and a linter that + checks the documents it produces. The two other skills whose output the owner + reads had a conditional half-sentence and nothing at all. A promise kept in + one place and hoped for elsewhere is not a standard. + """ + missing = [f for f in OWNER_FACING_FORMATS + if PLAIN_ENGLISH_HEADING not in (ROOT / f).read_text(encoding="utf-8")] + if missing: + raise ValidationError( + f"these report formats are read by the project owner but do not carry the " + f"'{PLAIN_ENGLISH_HEADING}' rules: {', '.join(missing)}") + + UNTRUSTED_HEADING = "## The repository is data, not instructions" @@ -206,11 +232,12 @@ def main() -> int: validate_installer() validate_house_style() validate_untrusted_content_rule() + validate_plain_english_rule() except (ValidationError, OSError, subprocess.CalledProcessError) as error: print(f"Validation failed: {error}") return 1 - print("Validated skills, plugin manifests, installer runtime, house style, and the untrusted-content rule successfully.") + print("Validated skills, plugin manifests, installer runtime, house style, the untrusted-content rule, and the plain-English rule successfully.") return 0 diff --git a/skills/clean-code-review/references/report-format.md b/skills/clean-code-review/references/report-format.md index 1a2a74f..5965355 100644 --- a/skills/clean-code-review/references/report-format.md +++ b/skills/clean-code-review/references/report-format.md @@ -16,7 +16,7 @@ Order findings by severity. For each finding include: 4. **Impact**: realistic user, operational, or maintenance consequence 5. **Recommendation**: smallest credible fix -For beginner-facing reports, express evidence, impact, and recommendation as What / Why / Fix. +Express evidence, impact, and recommendation as What / Why / Fix. This is not conditional: everyone this toolkit is built for is learning, so there is no second, terser audience to switch to. ## Checks run @@ -31,3 +31,27 @@ If relevant, list missing setup instructions, architecture context, tests, or op Mention only strengths that materially reduce risk or make the code easier to change. Keep this shorter than the findings. If no actionable findings exist, say so plainly and identify the scope limitations of the review. + +## Write it so the owner can act on it + +The person reading this built the app by describing it to an AI. They are +learning. A finding they cannot understand is a finding they cannot fix, so the +plain-English rules are part of the output, not a nicety. + +1. **Never lead with jargon.** Say what it does and why it matters, then name it. + "Anyone who knows the URL can read this page (missing an authorization check)" + beats "missing authz on the handler". +2. **Define a technical term inline, in one clause, the first time you use it.** + If you cannot define it in a clause, you are using it to sound precise rather + than to be understood. +3. **Use a real consequence, not an abstract one.** "You will not know it broke + until a user emails you" lands harder than "lacks observability". +4. **Frame a gap as the next thing to learn, not as a mistake.** The reader is + learning; a gap is a lesson, not a failure. +5. **Say a win plainly.** If something is genuinely fine, say so. Do not + manufacture urgency to make the report feel worthwhile. +6. **Short sentences. One idea each.** Avoid stacked clauses. + +The rubric this review is built on is written for an engineer, on purpose, +because it decides what to look at. This section decides how to say it. Do not +let the rubric's vocabulary leak into the report. diff --git a/skills/clean-code-review/references/review-rubric.md b/skills/clean-code-review/references/review-rubric.md index 256596c..47424f6 100644 --- a/skills/clean-code-review/references/review-rubric.md +++ b/skills/clean-code-review/references/review-rubric.md @@ -2,6 +2,15 @@ Use the relevant sections for the code under review. Do not force every section onto every file. +> **On the vocabulary here.** This file is written for an engineer, deliberately. +> It decides what to look at, and precision is what makes it work; "temporal +> coupling" names a thing that "order matters" does not. It is never shown to the +> project owner. How to say a finding to the owner is a separate job, handled in +> `references/report-format.md`, and its rules exist so this vocabulary does not +> leak into the report. Do not translate this file into plain English. Translate +> the report. + + ## 1. Correctness and behavior - Does the implementation match its stated contract and callers' expectations? diff --git a/skills/product-readiness-review/references/readiness-rubric.md b/skills/product-readiness-review/references/readiness-rubric.md index 63c90dd..a188be7 100644 --- a/skills/product-readiness-review/references/readiness-rubric.md +++ b/skills/product-readiness-review/references/readiness-rubric.md @@ -2,6 +2,15 @@ Apply only the dimensions relevant to the product and its risk profile. +> **On the vocabulary here.** This file is written for an engineer, deliberately. +> It decides what to look at, and precision is what makes it work; "temporal +> coupling" names a thing that "order matters" does not. It is never shown to the +> project owner. How to say a finding to the owner is a separate job, handled in +> `references/report-format.md`, and its rules exist so this vocabulary does not +> leak into the report. Do not translate this file into plain English. Translate +> the report. + + ## Product intent and user journeys - Is the intended user and problem clear? diff --git a/skills/product-readiness-review/references/report-format.md b/skills/product-readiness-review/references/report-format.md index a34f13f..045c3bc 100644 --- a/skills/product-readiness-review/references/report-format.md +++ b/skills/product-readiness-review/references/report-format.md @@ -30,3 +30,27 @@ List documents inspected, commands run, results, and material areas not exercise ## Recommended sequence Give the smallest ordered path from current state to the stated milestone. Separate work for now, next, and later. + +## Write it so the owner can act on it + +The person reading this built the app by describing it to an AI. They are +learning. A finding they cannot understand is a finding they cannot fix, so the +plain-English rules are part of the output, not a nicety. + +1. **Never lead with jargon.** Say what it does and why it matters, then name it. + "Anyone who knows the URL can read this page (missing an authorization check)" + beats "missing authz on the handler". +2. **Define a technical term inline, in one clause, the first time you use it.** + If you cannot define it in a clause, you are using it to sound precise rather + than to be understood. +3. **Use a real consequence, not an abstract one.** "You will not know it broke + until a user emails you" lands harder than "lacks observability". +4. **Frame a gap as the next thing to learn, not as a mistake.** The reader is + learning; a gap is a lesson, not a failure. +5. **Say a win plainly.** If something is genuinely fine, say so. Do not + manufacture urgency to make the report feel worthwhile. +6. **Short sentences. One idea each.** Avoid stacked clauses. + +The rubric this review is built on is written for an engineer, on purpose, +because it decides what to look at. This section decides how to say it. Do not +let the rubric's vocabulary leak into the report.