clean-code-review

name	clean-code-review
description	Comprehensive code review across 10 dimensions (Naming, Functions, Classes, Error Handling, Comments, Formatting, Tests, Architecture, Patterns, Professional Standards). Coordinates specialized skills for deep analysis. Final quality gate: code must pass with no critical issues.
allowed-tools	Read, Grep, Glob
argument-hint	["file or directory path"]
model	opus
delegates-to	["naming","functions","solid","tdd","architecture","patterns","components","functional-programming","acceptance-testing","legacy-code","professional"]

Clean Code Review (Operational)

Comprehensive code review against Clean Code principles. Final quality gate — code is NOT complete until review passes with no critical issues.

This skill is a COORDINATOR. It detects change scope, delegates to specialized skills for deep analysis, and produces a structured review report. It enforces the quality gate (no CRITICAL issues = pass).

---

When to Activate

• After ANY code modification task is completed
• Before marking ANY implementation work as "done"
• Before committing code
• After TDD cycles complete
• After refactoring
• User mentions: review, quality check, code quality, verify standards

When NOT to Apply

• Trivial changes (1-2 line fixes, typo corrections)
• Generated code (scaffolding, schema generators, transpiled output)
• Third-party vendor code or dependencies
• Configuration files with no business logic
• Single-file, single-function changes under 10 lines (use /naming or /functions inline instead)

---

Step 0: Detect Context

Before starting the review, gather context about the change:

Detect Language & Framework

• Primary language(s) in changed files
• Runtime (PHP, Node.js, Python, Java, Go, Rust, etc.)
• Language-specific idioms or testing approaches

Detect Scope of Change

• Count changed files
• Count lines changed (estimate: small <50, medium 50-500, large >500)
• Is this a new feature, refactor, bug fix, or configuration change?

Detect Test Existence

• Do test files exist for changed modules?
• Is coverage reasonable? (check test/src ratio, coverage reports if available)
• Are new tests present for new logic?

Classify Change Size

• Small: 1 file, <50 lines changed, single function or class
• Medium: 2-3 files, 50-500 lines, multiple functions or one new class
• Large: 4+ files, >500 lines, new module/feature, or architecture change

Output of Step 0:

Context: [Language] [Framework]
Scope: [small|medium|large] ([# files] files, [~# lines] lines)
Tests: [exist|missing] ([coverage info if available])
Change Type: [feature|refactor|bugfix|config]

---

Step 1: Scope the Review

Apply review depth proportionally to change size.

WHEN Small change:

• Run inline checks only (naming, formatting, basic function checks)
• Skip delegation
• Complete in 5 minutes
• Checklist: items 1, 5, 6, 10 only

WHEN Medium change:

• Run inline checks on all 10 dimensions
• Delegate only to skills relevant to changed code (e.g., if functions changed → /functions, if classes changed → /solid)
• Complete in 15-30 minutes
• Checklist: all 10 items, selective delegation

WHEN Large change (feature, architecture, new module):

• Run full review across all 10 dimensions
• Delegate to ALL relevant skills
• Multiple review passes if needed (architecture first, then details)
• Complete in 30+ minutes
• Checklist: all 10 items, full delegation

Output of Step 1:

Review Depth: [inline-only|selective-delegation|full-delegation]
Dimensions to check: [list 1-10]
Skills to delegate to: [list if applicable]
Estimated Time: [5|15|30]+ minutes

---

Step 2: Execute Review Dimensions

Work through each of the 10 dimensions (use checklist below as quick reference). For each, apply the inline check, then decide whether to delegate for deep analysis.

Dimension 1: NAMING

Severity: 🔴 CRITICAL | Inline Check: Names clear and intent-revealing? No cryptic/misleading names (a1, data, temp)? Consistent vocabulary? | Delegate /naming if: 5+ names changed or touches public API | Pass: Clear intent-revealing names, no misleading vocabulary

Dimension 2: FUNCTIONS

Severity: 🔴 CRITICAL | Inline Check: Functions <40 lines? 0-3 arguments? Each does ONE thing? No flag arguments or unwanted side effects? | Delegate /functions if: >40 lines, >3 args, or refactoring call sites | Pass: Small functions, 0-3 args, single purpose, no side effects

Dimension 3: CLASSES & OBJECTS

Severity: 🔴 CRITICAL | Inline Check: Single responsibility? High cohesion? Low coupling? Clear encapsulation? | Delegate /solid if: New class or major refactor | Pass: Single responsibility, high cohesion, SOLID respected

Dimension 4: ERROR HANDLING

Severity: 🔴 CRITICAL | Inline Check: No bare except/catch-all? No null returns? Exceptions carry context? | Delegate: Rarely needed | Pass: Exceptions not codes, no bare except, no null returns, input validation

Dimension 5: COMMENTS

Severity: 🟡 WARNING | Inline Check: No commented-out code? Comments explain WHY not WHAT? | Delegate: Not needed | Pass: Self-documenting code, WHY comments only, no redundant comments

Dimension 6: FORMATTING

Severity: 🟡 WARNING | Inline Check: Consistent indentation, line length <120 chars, vertical openness? | Delegate: Use formatter instead | Pass: Consistent style, reasonable line length

Dimension 7: TESTS

Severity: 🔴 CRITICAL | Inline Check: Tests for new logic? Happy path AND error cases? Readable (AAA pattern)? Independent? | Delegate /tdd if: No tests for new code or <70% coverage | Pass: Tests for ALL new logic, happy path + errors, readable

Dimension 8: ARCHITECTURE

Severity: 🟡 WARNING | Inline Check: File structure reveals intent? Dependencies inward? Clear boundaries? | Delegate /architecture if: New module or dependency rule violations | Pass: Clear separation of concerns, dependencies inward, boundaries defined

Dimension 9: DESIGN PATTERNS

Severity: 🟡 WARNING | Inline Check: Patterns necessary (solving real problem)? Implemented correctly? | Delegate /patterns if: New pattern or uncertain correctness | Pass: Patterns solve real problems, correct implementation

Dimension 10: PROFESSIONAL STANDARDS

Severity: 🟡 WARNING (🔴 if worse) | Inline Check: Best effort? Code no worse than before? Knowledge shared? | Delegate /professional if: Ethical concerns | Pass: Best work, no worse than before, maintainable

---

Step 3: Produce Review Report

Generate a structured review report following this template:

## Code Review Report

**Date:** [date] | **Files:** [count] | **Type:** [feature|refactor|bugfix] | **Scope:** [small|medium|large]

### VERDICT: [PASS | PASS WITH WARNINGS | FAIL]

Critical Issues: [#] | Warnings: [#] | Suggestions: [#]

### CRITICAL ISSUES
[List: Dimension (X), File, Line, Issue, Action]

### WARNINGS
[List: Dimension (X), File, Issue, Suggestion]

### SUGGESTIONS
[List: Dimension (X), Brief note]

### QUALITY GATE DECISION

✅ PASS — No critical issues. Ready to commit.
⚠️ PASS WITH WARNINGS — No critical issues. Address warnings in follow-up PR.
❌ FAIL — Critical issues found. Must fix before merge.

---

Step 4: Quality Gate

PASS: Zero CRITICAL issues

• Code is done
• Ready to commit

PASS WITH WARNINGS: Zero CRITICAL issues, 1+ WARNINGS

• Code is done
• Schedule follow-up PR to address warnings
• Document why warnings weren't critical

FAIL: 1+ CRITICAL issues

• Code is NOT done
• Fix critical issues
• Re-run review (may run Steps 2-3 only)

---

Review Checklist (10 Dimensions)

Use this checklist to ensure all dimensions are covered:

#	Dimension	Severity	Inline Check	Delegate To	Pass Criteria
1	Naming	🔴 CRITICAL	Scan for misleading/cryptic names	/naming if 5+ names changed	Clear, intent-revealing names; no misleading vocabulary
2	Functions	🔴 CRITICAL	Check line count, arity, single responsibility	/functions if >40 lines or >3 args	Small (<40 lines), 0-3 args, ONE thing, no side effects
3	Classes	🔴 CRITICAL	Check cohesion, coupling, SRP	/solid if new class or major refactor	Single responsibility, high cohesion, low coupling
4	Error Handling	🔴 CRITICAL	Check for null returns, bare except, swallowed exceptions	(inline)	Exceptions (not codes), no bare except, no null returns
5	Comments	🟡 WARNING	Check for commented-out code, redundancy	(inline)	No commented-out code, comments explain WHY
6	Formatting	🟡 WARNING	Check indentation, line length, blank lines	(inline or use formatter)	Consistent style, reasonable line length, vertical openness
7	Tests	🔴 CRITICAL	Check for test existence, coverage, readability	/tdd if no tests for new logic	Tests for ALL new logic, cover happy path + errors
8	Architecture	🟡 WARNING	Check dependencies, separation of concerns, boundaries	/architecture if new module or major change	Dependencies point inward, clear boundaries
9	Patterns	🟡 WARNING	Check for unnecessary or incorrect patterns	/patterns if new pattern introduced	Patterns solve real problems, correct implementation
10	Professional Standards	🟡 WARNING (🔴 if worse)	Check Boy Scout Rule, knowledge sharing	/professional if ethical concerns	Code is best effort, no worse than before, maintainable

---

Refactoring Patterns for This Skill

This skill does NOT refactor code — it produces review reports and makes quality gate decisions. Key operational patterns:

Triage Review: Prioritize critical issues, defer suggestions. Fix CRITICALs, schedule WARNINGs in follow-up, mention SUGGESTIONs informally.

Scoped Delegation: Only delegate dimensions relevant to the change. Small function rename → /naming only. New class → /solid + /components. New feature → full delegation.

Incremental Review: For large PRs (500+ lines), review commit-by-commit or file-by-file to avoid missing issues. Apply Steps 0-3 for each logical group.

Boy Scout Check: Before finalizing review, ask: "Is this code cleaner than we found it?" If not, escalate to FAIL or WARNING.

Delegation Report: When delegating, include context in the report explaining why delegation occurred and what sub-skill should verify.

---

K-Line History

What Worked:

• Detecting scope first (Step 0) prevents over-reviewing small changes
• Selective delegation based on change size prevents unnecessary context switching
• CRITICAL/WARNING/SUGGESTION triage focuses effort on real issues
• Structured report template ensures consistency and clarity

What Failed:

• Reviewing all 10 dimensions equally for 1-line changes wastes time
• Reviewing without detecting language/framework context leads to inapplicable advice
• Skipping tests during review misses major issues
• Nitpicking formatting while ignoring design wastes energy

---

Communication Style

Direct, operational, outcome-focused.

• State findings concisely: "Dimension 2: validateAndTransform() is 65 lines doing 3 things. Refactor into 3 functions."
• Avoid vague criticism: ❌ "This doesn't feel clean" → ✅ "Function has 4 arguments; reduce to 2-3 by extracting object parameter."
• Use severity labels: 🔴 CRITICAL, 🟡 WARNING, 🟢 SUGGESTION
• Explain reasoning briefly: "Tests are CRITICAL because this is public API — callers depend on error behavior."
• Propose next steps: "Fix 2 CRITICAL issues, then re-run review on those files only."

---

Related Skills

• /naming — Deep naming analysis and refactoring
• /functions — Function decomposition, arity, abstraction levels
• /solid — SOLID principles, class design, dependency injection
• /architecture — Clean Architecture, dependency rules, layer boundaries
• /tdd — Test quality, test-driven design, testing pyramid
• /patterns — Design pattern correctness and necessity
• /components — Component cohesion, coupling, dependency management
• /functional-programming — Purity, immutability, side effect isolation
• /acceptance-testing — Acceptance test quality, ATDD
• /legacy-code — Boy Scout Rule, characterization tests, strangulation
• /professional — Professional responsibility, estimation, ethical standards