Corteksa

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 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

  1. Names reveal intent. Never data, data2, result, resultFinal, item, temp, value, obj, info, helper, manager, utils, or bare handle* / process* / do*. A name answers why it exists and what it does.
  2. 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.
  3. 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).
  4. 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

  1. 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.
  2. 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

  1. One actor per module. Answerable to one stakeholder group. Two unrelated subsystems reaching into the same class → split it. (SRP)
  2. 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)
  3. 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)
  4. 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

  1. 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.
  2. 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)
  3. Complexity ceiling: cyclomatic ≤10, nesting depth ≤5. Refactor before exceeding.
  4. 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

  1. 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.
  2. 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.
  3. 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.)
  4. 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.
  5. 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.
  6. Enumerate boundary cases before writing them. Range, off-by-one, null/empty/one/many, timezone, unicode. List them, then cover each.
  7. 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."
  8. 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

  1. 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.

#RuleWhat it cost us
R1Reference 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.
R2Never 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
R3Every 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.
R4Cache 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.
R5No 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.
R6No 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.
R7Never re-implement the authz fold. Levels resolve only through access-level.util.ts.A second implementation drifts and silently grants access. AUTHORIZATION.md
R8Endpoint gate + row filter, both. @RouteName alone is not access control.Coarse-only checks returned other admins' records past the gate.
R9No hardcoded config fallback. Read env, fail loud when missing — never process.env.X || 'https://default'.A baked-in domain shipped to the wrong environment.
R10Never 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.

On this page