Clean code rules
Clean code rules
What this is. The 23 imperatives the /clean-code-guard gate (CLAUDE.md §1.3) checks on
every diff, plus the repo-specific rules we learned the hard way. They live here, in the repo, so the
standard is the same for Claude, for a teammate without the skill installed, and for CI.
Layer: is this line of code well written? Sits beside CODE_PRINCIPLES.md (the 10 rules with our own war stories) — that doc is the why, this one is the checklist. Where they overlap they agree; where this is stricter, this wins.
Sources: Clean Code (Martin), Refactoring (Fowler), Pragmatic Programmer (Hunt & Thomas), McCabe 1976, Sandi Metz, plus 2024–2026 research on LLM code generation (GitClear, USENIX Security '25, arXiv).
Why an AI-specific layer exists
Generic "follow clean code" does not catch how LLMs actually fail. Measured:
- Code duplication grew 8× in tracked codebases 2021→2024 (GitClear 2025).
- Package hallucination rate averages 19.6% across 16 models (USENIX Security '25).
- Function size grew 142 → 267 LoC, cyclomatic complexity 4.2 → 8.1 in AI-assisted commits (GitClear).
- Agents declare success despite failing tests by returning hardcoded fixtures (Fowler).
- Models wrap risky operations in catch-all handlers that swallow errors.
§4 below is the highest-leverage section. Read it first.
1. Functions and names
- Names reveal intent. Never
data,data2,result,resultFinal,item,temp,value,obj,info,helper,manager,utils, or barehandle*/process*/do*. A name answers why it exists and what it does. - Functions stay small. Target ≤20 lines, one level of abstraction, one thing. If you can extract a function whose name doesn't just restate the body, the parent was doing more than one thing.
- Four arguments is the hard ceiling. At five, introduce a DTO/options object. Never boolean flag arguments — split into two functions (see CODE_PRINCIPLES.md Rule 6).
- No output arguments. A function returns a value (query) or has a side effect (command), never both. Commands are verbs; queries are nouns/getters.
2. Comments and structure
- Comments explain why, never what. This is the one that bites AI output hardest.
- Delete any comment that paraphrases the line below it (
// increment counter). - Delete step-number scaffolding (
// 1. fetch user,// 2. validate). - Delete commented-out code — git has it.
- Delete section banners restating the method name.
- Keep the non-obvious: why this order, why this workaround, which invariant holds, what breaks if you "simplify" it, a link to the incident or spec.
- A comment that would surprise a maintainer if removed is worth keeping. Everything else is noise.
- Never narrate an edit in a comment (
// changed to fix bug,// added per review) — that belongs in the commit message.
- Delete any comment that paraphrases the line below it (
- Match the file's existing style. Read the file you're editing and one neighbor before writing. Mirror casing, import order, error handling, logging, DI style. Never introduce a second pattern.
3. SOLID
- One actor per module. Answerable to one stakeholder group. Two unrelated subsystems reaching into the same class → split it. (SRP)
- Extension via new code, not edits. If a new variant needs another
switch/type-tag branch in an existing function, refactor to a registry/strategy first. This repo already does it — field types, workflow triggers/actions/resolvers, messaging providers, AI tools all use registries. Follow that. (OCP) - No subclass refuses its parent's contract. Never override to throw "not implemented". Never strengthen preconditions or weaken postconditions. If you need to, the inheritance is wrong. (LSP)
- Abstractions live with the client. Put the interface/token in the package that consumes it, not
next to the concrete class — this is exactly the
adapters/pattern in ARCHITECTURE.md §7. (DIP)
4. DRY, KISS, YAGNI
- Delete duplicated knowledge, not duplicated text. Two functions that look alike but encode different business rules are not a DRY violation. One rule expressed in code + docs + schema is.
- The wrong abstraction is worse than duplication. If an abstraction has grown a branch per caller, re-inline it into the callers, delete the dead branches, then re-abstract. (Metz)
- Complexity ceiling: cyclomatic ≤10, nesting depth ≤5. Refactor before exceeding.
- No speculative anything. No optional param, config flag, env var, feature toggle, interface,
factory, or base class without a caller today. If you're adding
enable*,use*V2, or*Mode, delete it and ship the concrete behavior. (Fowler, YAGNI — and the Rule of Three in CODE_PRINCIPLES.md Rule 9.)
5. AI-specific guardrails — highest leverage
- Never swallow errors with a catch-all. Catch only the specific error you can recover from. If you
can't recover, let it propagate. Returning
null/[]/success from a catch block is forbidden unless the contract documents it. In this repo: throw the right NestJS exception (NESTJS_BEST_PRACTICES.md §5) and let the global filter shape it. - No defensive guards for impossible cases. Don't null-check a value whose type or caller contract already excludes null. Trust the contract; fix the type if it lies.
- Verify every import and external call. Before calling a library method, confirm it exists in the installed version — read the package or the lockfile. Do not generate code from what the API "should" look like. (19.6% hallucination rate.)
- No hardcoded success or fixture data in production code. Never return canned
{ status: 'ok' }from a function whose spec says it does real work. If you can't implement it, fail explicitly and say so. Never disable, skip, or weaken a test to make it pass. - Re-derive, don't copy-from-similar. When tempted to copy a function and tweak it, stop and re-derive from the spec. Off-by-one and wrong-null-semantics bugs enter here.
- Enumerate boundary cases before writing them. Range, off-by-one, null/empty/one/many, timezone, unicode. List them, then cover each.
- Strip dead code before delivery. Unused imports, unused exports, unreachable branches, "just in case" helpers. A function nothing calls today doesn't get to live for "someday."
- Read before write. In unfamiliar code, read the target file, one neighbor, and the project rules (CLAUDE.md §0) first. Use the existing helpers, error types, and logging — don't invent parallel ones.
6. Refactoring discipline
- Preserve observable behavior when refactoring. Same inputs → same outputs, same exceptions, same side effects, same ordering. Spot a bug mid-refactor? Flag it separately and ask — refactor and bug fix are two changes, never one commit.
7. Repo-specific hard rules (earned in production)
These are not from a book. Each one cost us an incident.
| # | Rule | What it cost us |
|---|---|---|
| R1 | Reference by stable id, never mutable text. A select-option link stores option.id, never option.label. | Renaming an option orphaned every record referencing it. Full detail: CODE_PRINCIPLES.md Rule 10. |
| R2 | Never reach past the pinned connection. No getDataSource(), no repo.manager.connection, no unpinned createQueryRunner(). | On hyper-tenant the RLS GUC is empty → INSERT throws, UPDATE/DELETE/SELECT silently match zero rows. ISOLATION.md |
| R3 | Every Bull processor is @TenantScoped() with a TenantJobData payload. | Background jobs read/write the wrong workspace, or nothing at all. CI-enforced: npm run lint:processor-tenant-scope. |
| R4 | Cache keys carry tenant + workspace. v1:<entity>:<op>:tenant=<db>:ws=<id>:… | Redis has no RLS. A missing ws= leaks one workspace's data into another's cache. |
| R5 | No logging on a hot path. Use the lazy debugLog helper; never build a log string that's thrown away at the current level. | Log-string construction pinned production CPU. |
| R6 | No unbounded retry on a provider error. Classify the error; retry only what is transient, cap it, and drop what is permanently unrecoverable. | A WAHA "revoked" response drove a retry storm. |
| R7 | Never re-implement the authz fold. Levels resolve only through access-level.util.ts. | A second implementation drifts and silently grants access. AUTHORIZATION.md |
| R8 | Endpoint gate + row filter, both. @RouteName alone is not access control. | Coarse-only checks returned other admins' records past the gate. |
| R9 | No hardcoded config fallback. Read env, fail loud when missing — never process.env.X || 'https://default'. | A baked-in domain shipped to the wrong environment. |
| R10 | Never any; unknown only where the type is genuinely unknowable. Default to a real interface/DTO/union. unknown belongs at the entry point (webhook payload, JSON.parse, catch (e: unknown), untyped SDK) — narrow it there with one guard, then pass a typed shape inward. unknown in a service signature or return type is a bug, and as any / as unknown as T / @ts-ignore / object are the same violation renamed. CI-enforced on changed files: as unknown as is a hard error (no-restricted-syntax, tests exempt); any is a warning. | Every any that reached production became a runtime shape bug — and banning any alone bred 115 as unknown as casts in prod that just moved the bug behind a cast. |
8. Self-check before you deliver
Walk this against your diff. A "no" is a finding — fix it or state why it's accepted.
- Imperatives 1–23 clean on every changed hunk?
- New functions: ≤20 lines, ≤4 params, complexity ≤10, names reveal intent?
- Every new comment explains why? Deleted the ones explaining what? (Rule 5)
- Caught error types specific, handlers doing something other than silently returning? (Rule 15)
- New abstraction has a second real caller today? If not, inlined. (Rules 12, 14)
- Read the edited file and a neighbor; style matches? (Rules 6, 22)
- Zero hardcoded "ok" returns, fixtures, skipped or weakened tests? (Rule 18)
- If a refactor: observable behavior identical, bug fixes split out? (Rule 23)
- Repo rules R1–R10 all hold?
- Docs updated for anything new (CLAUDE.md §7)?
9. Overriding a rule
The rules are defensible — primary sources plus published research. If you have a context-specific reason to break one (a config DTO genuinely needs 8 fields), document the exception in a code comment naming the principle and the reason. An undocumented exception is just a violation.