# Code Review

> ---

- Skill: `tranhieutt/code-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add tranhieutt/code-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/tranhieutt/code-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: tranhieutt (https://skillmd.com/u/tranhieutt)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/tranhieutt/code-review

---

---
name: code-review
type: workflow
description: "Performs architectural and quality code review checking coding standards, SOLID principles, architectural compliance, and common software issues. Use when reviewing a file or directory before merge, or when the user mentions code review, PR review, or quality check."
argument-hint: "[path-to-file-or-directory]"
user-invocable: true
allowed-tools: Read, Glob, Grep, Bash
context: fork
agent: lead-programmer
effort: 2
when_to_use: "When a full architectural and quality review is needed before merging a PR"
---

When this skill is invoked:

1. **Read the target file(s)** in full.

2. **Read the CLAUDE.md** for project coding standards.

3. **Identify the system category** (api, service, repository, component, utility, infrastructure)
   and apply category-specific standards.

4. **Evaluate against coding standards**:
   - [ ] Public methods and classes have doc comments
   - [ ] Cyclomatic complexity under 10 per method
   - [ ] No method exceeds 40 lines (excluding data declarations)
   - [ ] Dependencies are injected (no singletons for business state)
   - [ ] Configuration values loaded from external config, not hardcoded
   - [ ] Systems expose interfaces (not concrete class dependencies)

5. **Check architectural compliance**:
   - [ ] Correct dependency direction (infrastructure ← domain ← application)
   - [ ] No circular dependencies between modules
   - [ ] Proper layer separation (UI does not own business logic)
   - [ ] Events/messages used for cross-service communication
   - [ ] Consistent with established patterns in the codebase
   - [ ] Interfaces are earning their complexity: prefer deep modules over thin pass-through wrappers
   - [ ] Suspected abstractions survive the deletion test: deleting them would reintroduce complexity across callers, not simplify the system

6. **Check SOLID compliance**:
   - [ ] Single Responsibility: Each class has one reason to change
   - [ ] Open/Closed: Extendable without modification
   - [ ] Liskov Substitution: Subtypes substitutable for base types
   - [ ] Interface Segregation: No fat interfaces
   - [ ] Dependency Inversion: Depends on abstractions, not concretions

7. **Check for common web/software issues**:
   - [ ] No N+1 query patterns (use eager loading or joins)
   - [ ] Proper async/await usage (no unhandled promises, no floating async)
   - [ ] Input validation at system boundaries (user input, external APIs)
   - [ ] Proper error handling with meaningful, safe messages (no stack traces exposed)
   - [ ] No secrets or sensitive data hardcoded (API keys, passwords, tokens)
   - [ ] Resource cleanup (DB connections, streams, subscriptions, event listeners)
   - [ ] Thread/concurrency safety where required

8. **Output the review** in this format:

```
## Code Review: [File/System Name]

### Standards Compliance: [X/6 passing]
[List failures with line references]

### Architecture: [CLEAN / MINOR ISSUES / VIOLATIONS FOUND]
[List specific architectural concerns]

### SOLID: [COMPLIANT / ISSUES FOUND]
[List specific violations]

### Web/Software Concerns
[List web and software-specific issues found]

### Positive Observations
[What is done well — always include this section]

### Required Changes
[Must-fix items before approval]

### Suggestions
[Nice-to-have improvements]

### Verdict: [APPROVED / APPROVED WITH SUGGESTIONS / CHANGES REQUIRED]
```

## Protocol

Architecture vocabulary to use when it helps clarify a finding:

- **Module** — unit with an interface and implementation
- **Deep module** — small interface, high leverage behind it
- **Shallow module** — interface nearly as complex as implementation
- **Deletion test** — if deleting a wrapper mostly removes indirection instead of
  reintroducing complexity in callers, the abstraction may be shallow

Use this vocabulary to sharpen architectural findings, not to force jargon into
every review.

- **Question**: Auto-starts from argument (file or directory path); no clarification needed
- **Options**: Skip — single review path
- **Decision**: Skip — verdict is advisory
- **Draft**: Full review shown in conversation only
- **Approval**: Skip — read-only; no files written

## Output

Deliver exactly:

- **Risk Tier** (Low / Medium / High) with one-sentence justification
- **Standards & Architecture compliance** score (X/6, X/5)
- **Blocking issues** — must fix before merge (or "None")
- **Suggestions** — optional improvements, max 3
- **Verdict**: `APPROVED` / `APPROVED WITH SUGGESTIONS` / `CHANGES REQUIRED`