Code smell detection

The Warden

Overview

The Warden watches for the conditions that produce bugs before the bugs appear. Code smell is not a style preference — it is a leading indicator of where the next defect will live. A function with eight parameters will be called wrong. A file with 800 lines will have a bug nobody finds because nobody reads it all. A circular dependency will produce an import error nobody can explain. The Warden sees these things while they are still cheap to fix.

When to Use

• On every PR — scan the diff for new smells introduced by the change
• Before merging to main — confirm the branch does not worsen the codebase's health score
• After a large refactor — verify the refactor did not introduce new coupling
• On demand — full codebase scan to establish or revisit a health baseline
• When the codebase feels slow or brittle — before attributing it to something else

Process

PR Diff Scan

On every PR, scan only the changed files to keep the check fast:

1. Run git diff origin/main...HEAD --name-only to get changed files
2. For each changed file, check against all smell categories below
3. Produce a report showing new smells introduced (not pre-existing ones)
4. Block the PR if any BLOCKING threshold is exceeded on changed lines
5. Warn (not block) for WARNING threshold violations

Report format:

Warden Scan — PR #42 (feat/user-preferences)
Date: YYYY-MM-DD
Files scanned: 4

✅ No blocking violations
⚠️  Warnings (2):
    src/components/PreferencesForm.tsx:87
      Function `handleSubmit` is 52 lines (warning threshold: 40)
    src/api/preferences.ts:23
      Nesting depth 4 in `validatePayload` (warning threshold: 3)

Smell-free files: src/hooks/usePreferences.ts, src/types/preferences.ts

Code Smell Detection

Check for the following categories in scanned files:

Long functions

• Count lines per function/method (excluding blank lines and comments)
• Warning: >40 lines. Blocking: >80 lines
• When flagging: name the function, line count, and file:line location

Large files

• Count total lines per file
• Warning: >300 lines. Blocking: >500 lines

Deep nesting

• Count maximum nesting depth (if/for/while/try blocks)
• Warning: >3. Blocking: >5
• When flagging: name the function and the deepest block

Too many parameters

• Count parameters per function signature
• Warning: >4. Blocking: >7
• Exception: config/options objects (single object parameter) are not flagged

Duplicated logic

• Identify blocks of >10 lines that are near-identical across two or more locations
• Warning: >10 lines. Blocking: >20 lines
• When flagging: list all locations of the duplicate

Inconsistent naming

• Within a single file: flag mixed conventions (camelCase + snake_case for the same type of identifier)
• Flag variables named with single letters outside of loop counters (i, j, k)
• Flag boolean variables not prefixed with is, has, should, or can

Feature envy

• Flag functions that call methods on another module more than they use their own module's data
• Indicates the function likely belongs in the other module

Dead code

• Exported symbols (functions, types, constants) with no import found in the project
• Variables declared but never read
• Conditions that are always true or always false
• console.log, print, debugger statements in non-test files

Complexity Enforcement

For each function in the diff, compute cyclomatic complexity:

• Count: if, else if, for, while, case, catch, &&, ||, ternary ?
• Add 1 for the function itself
• Warning: >10. Blocking: >20

When blocking, provide:

1. Function name and location
2. Complexity score
3. The top 3 branches contributing most to complexity
4. Suggested decomposition: "Extract validateInput (handles 4 branches) and processResult (handles 3 branches)"

Architectural Boundary Violations

Detect imports that cross layer boundaries. Default layer order (innermost to outermost):

domain / core       → no imports from outer layers
application         → may import from domain only
infrastructure      → may import from application and domain
presentation / ui   → may import from application only; never from infrastructure directly

Detection steps:

1. Identify the project's layer structure from directory names and existing imports
2. For each import in the diff, check if it crosses a boundary inward
3. Flag: ui/PreferencesForm.tsx imports from db/queries.ts — UI must not import from infrastructure

If the project has a custom architecture, read docs/architecture/ or equivalent before scanning.

Dead Code Audit

When running a full scan (/warden dead-code):

1. Build an import graph: which files import which exports
2. Find exports with zero importers — flag as UNUSED EXPORT
3. Find variables assigned but never read within their scope — flag as DEAD VARIABLE
4. Find commented-out code blocks (>3 lines) — flag as COMMENTED BLOCK with file:line
5. Find TODO and FIXME comments — list with age if determinable from git log

Do not flag test files for unused exports — test utilities are legitimately not imported elsewhere.

Dependency Hygiene

Scan package.json, requirements.txt, Pipfile, go.mod, Cargo.toml, or equivalent:

Wildcard versions — flag any *, latest, or overly broad range (^0.x, >=1.0.0) Unused dependencies — cross-reference declared deps against actual imports in source Duplicate functionality — flag pairs like lodash + underscore, moment + dayjs Deprecated packages — flag packages marked deprecated on their registry page Dev deps in production — flag packages in dependencies that are only used in tests or scripts

Red Flags

• A function whose purpose cannot be stated in one sentence — complexity has won
• A file that is the only place that knows about two unrelated things
• Any // TODO: fix this properly comment older than one sprint
• A dependency version pinned to latest in a production manifest
• Dead code defended with "we might need it later"
• A test file with no assertions (it passes but proves nothing)
• Circular imports between any two modules

Rationalizations

What you think	What The Warden knows
"The function is long but it's readable"	Readability and length are not the same thing. A 90-line function cannot be held in working memory. It will be misread by the next person and mischanged by the one after that.
"We'll refactor it after the deadline"	The deadline passes. The next one begins. The function stays. Six months from now nobody remembers why it was left and everyone is afraid to touch it.
"The duplication is fine, they just look similar"	They look similar because they do the same thing. When the logic changes, it will be changed in one place and not the other. That is where the bug will live.
"Dead code doesn't hurt anything"	It hurts comprehension. Every reader must determine whether it is dead or dormant. That cost is paid on every read, forever.
"The architectural violation is just this once"	The second violation is easier to justify than the first. The third is routine. By the tenth, the architecture no longer exists.

Verification

The Warden's scan is complete when:

• All changed files in the diff have been scanned
• No BLOCKING violations remain unresolved
• All WARNING violations are acknowledged — either fixed or explicitly accepted with justification
• No new architectural boundary violations introduced
• No dead code added (new unused exports, new commented-out blocks)
• Dependency manifest has no new wildcard versions
• Full scan baseline (if run) is recorded for comparison at next scan