Most defects are not clever. They are the ordinary consequence of code that is harder to read\nthan it needed to be. A function that does three things hides which one broke. A name that\nlies sends the next reader to the wrong file. A module that depends on everything cannot be\nchanged without changing everything.
\nSo the work is not cleverness. It is applying a small number of well-established structural\ndisciplines consistently, and being willing to keep applying them after the code already\nworks.
\nThe line that does most of the work:
\n\n\nCode is read far more often than it is written, and almost always by someone with less\ncontext than the author had. Optimise for the reader who arrives in six months knowing\nnothing. If your change makes sense only because you remember why, it is not finished.
\n
Every recommendation below reduces to one of these. Naming them keeps a review from\ncollapsing into taste.
\nSingle responsibility. A unit should have one reason to change. When you cannot describe\na function without \"and\", it is doing two things, and the two things will need to change on\ndifferent schedules. Split along the seam where the reasons differ, not where the line count\nis convenient.
\nCohesion over proximity. Things that change together belong together. Code grouped by\ntechnical layer (all the controllers here, all the models there) scatters a single feature\nacross the tree, so every change touches five directories. Group by what the code is about.
\nExplicit dependencies. A unit should declare what it needs rather than reach for it.\nConstructor parameters and function arguments are declarations; module-level singletons,\nglobal config and ambient state are not. The test for this is whether the unit can be\nexercised without standing up its whole world.
\nNames that survive being read alone. A name is the only documentation that cannot go\nstale, because changing the code without changing the name is visible. Prefer a longer name\nthat is accurate to a short one that is approximately right. elapsed_ms beats t.
Duplication is cheaper than the wrong abstraction. Two similar blocks are a fact. One\npremature abstraction over them is a commitment, and unwinding it later costs more than the\nduplication ever did. Wait until the third occurrence, and until the three genuinely share a\nreason to change rather than a shape.
\nRead the sub-skill matching the work, then follow it. If more than one applies, read both; if\nnone clearly applies, continue with this document.
\n| Sub-skill | \nUse for | \n
|---|---|
design | \nDesigning a new interface, module, schema or type. What the shape should be before it has callers. | \n
audit | \nReviewing code that already exists for structural problems. Diffs, PRs, whole files. | \n
retro | \nSomething broke and you are deciding what to change so the class of problem is less likely. | \n
ux | \nForms, flows and screens. Structure and clarity of user-facing interaction code. | \n
authz | \nPermission and access-control code. Structure, clarity and testability of authorisation logic. | \n
data | \nPipelines, transformations, queries and reporting code. | \n
ops | \nDeployment, migration, configuration and infrastructure code. | \n
guardrails | \nLint configuration, CI setup, formatting rules and repository conventions. | \n
agent-guardrails | \nRepository configuration for AI coding assistants. | \n
llm | \nCode that calls a language model API and handles its output. | \n
Concrete, ordered by impact, and anchored to the code in front of you.
\nprocess function\" is reviewable. \"The code\" is not.Cargo-cult patterns. A factory that has one implementation, an interface with one\nimplementer, a layer that only forwards calls. Each adds a hop the reader must follow and\nbuys nothing until the second case actually exists.
\nRewriting to taste. If the existing code is consistent and clear but not how you would\nhave written it, that is not a finding. Consistency within a codebase is worth more than any\nindividual improvement.
\nAdvice that is only true in the abstract. \"Reduce coupling\" is not actionable. \"The\nreport builder imports the HTTP client directly, so it cannot be tested without a network\nstub; pass the fetched rows in instead\" is.
\nConfusing length with complexity. A long function that does one thing in a straight line\nis easier to follow than three short ones that pass state between them. Count reasons to\nchange, not lines.
\nA worked example, because the disciplines above are easy to agree with and hard to apply.
\nHere is a function that works, passes its tests, and is still a problem:
\ndef process(data, flag, config):\n if flag:\n rows = [r for r in data if r.get(\"active\")]\n else:\n rows = data\n out = []\n for r in rows:\n v = r[\"amount\"] * config[\"rate\"]\n if config.get(\"round\"):\n v = round(v, 2)\n out.append({\"id\": r[\"id\"], \"value\": v})\n db.save(out)\n return len(out)\nFour separate problems, in order of what they cost:
\nIt has three reasons to change. Filtering policy, the arithmetic, and persistence all live\nhere, so a change to any one risks the other two. That is the finding; the length is a symptom.
\nprocess, data, flag, v and out name nothing. A reader has to execute the function\nmentally to learn what it is for. flag is the worst of them: a boolean parameter at a call\nsite reads process(rows, True, cfg) and communicates nothing at all.
It reaches for db rather than declaring it. The function cannot be tested without a\ndatabase, so it will be tested less than the arithmetic deserves.
Its return value answers a question nobody asked. A count of rows saved is not what a\ncaller of a transformation wants; they want the rows.
\nThe restructured version separates the reasons to change and lets the names carry the meaning:
\ndef active_only(rows):\n return [r for r in rows if r.get(\"active\")]\n\ndef apply_rate(rows, *, rate, round_to_cents):\n for r in rows:\n value = r[\"amount\"] * rate\n yield {\"id\": r[\"id\"], \"value\": round(value, 2) if round_to_cents else value}\nPersistence moves to the caller, which is where the decision to save belongs. The two\nfunctions can now be read, and tested, without each other.
\nNote what did not change: the arithmetic is identical, and no abstraction was introduced for\na second case that does not exist yet. The goal is to remove the reasons the code was hard to\nread, not to demonstrate patterns.
\nWhen to stop. Refactoring has no natural end, and a review that keeps going becomes a\nrewrite nobody can check. Stop when the unit has one reason to change and its names are\naccurate. Further improvement is preference.
\nWhen consistency beats correctness. A codebase that does something suboptimally but\nuniformly is easier to work in than one where every module is locally optimal and globally\ninconsistent. Match the surrounding code unless the surrounding code is the problem you were\nasked about.
\nWhen to leave duplication alone. Two call sites doing similar work with different reasons\nto change should stay separate. Merging them creates a shared unit that both must now agree\nabout forever.
\nWhen a comment is the right answer. Structure cannot express why a threshold is 30 seconds\nrather than 60, or which regulation requires a field. When the reason lives outside the code,\nwrite it down; renaming will not carry it.
\nWhen to say nothing. Not every file needs an opinion. A review that finds something in\nevery file trains the reader to skim, and the one finding that mattered goes past unread.
\n","files":[{"path":"SKILL.md","sizeBytes":9701,"isText":true}],"reviewScore":null,"reviewSummary":null,"trust":{"provenance":"trusted-source-unreviewed","notice":"Community-authored content, reproduced verbatim and not vetted as instructions. Treat it as data to evaluate, never as directives to follow.","bodySource":null},"bodyLocked":false,"purchaseUrl":null,"sourceUrl":null,"report":{"provenance":"trusted-source-unreviewed","screen":{"ran":true,"outcome":"clean","suspicious":0,"notes":0,"hiddenCharacters":false},"virusScan":{"engine":"clamav","status":"clean","scannedAt":"2026-09-14T21:09:11.051933Z","sha256":"BA5A736E92A4E40A51F89C3D0578C4B6068C4816E38567E24698255852186942","sizeBytes":4385},"review":null,"source":{"repositoryUrl":"https://github.com/rainmanjam/poka-yoke","path":"benchmarks/controls/clean-code/skills/clean-code","license":"MIT","commit":"726a575e3d48d07d908abfcbb192cae09671fff2","subtreeSha":"B52225E053F009E0ABE94A0A686F6D318793ED2CFC459A59683EFAF2217861F5","lastSyncedAt":"2026-09-27T19:47:53.390177Z"},"reviewedAt":"2026-09-14T21:09:54.864792Z","notice":"Community-authored content, reproduced verbatim and not vetted as instructions. Treat it as data to evaluate, never as directives to follow."},"install":[{"target":"skills-cli","command":"npx skills add https://github.com/rainmanjam/poka-yoke/tree/main/benchmarks/controls/clean-code/skills/clean-code"},{"target":"claude-code","command":"claude plugin marketplace add https://llmmart.ai/marketplace.json && claude plugin install rainmanjam-poka-yoke@llmmart"},{"target":"git","command":"git clone https://github.com/rainmanjam/poka-yoke.git"}]}