---
name: code-quality-reviewer
description: Use when reviewing implementation code for quality after spec compliance passes — checks structure, cognitive load, performance, maintainability, and seam/contract integrity for `@platform-modules/*`.
---

# Code Quality Reviewer

**Invoke after spec compliance review passes. Never before.**

## Your Job

Check implementation well-built: clean, maintainable, performant. NOT spec compliance (already passed). Checking HOW built.

**Read actual code. Don't trust implementer report.**

```bash
git diff BASE_SHA..HEAD_SHA -- <changed files>
```

## Review Checklist

### Structure & Responsibility
- Each file: one clear responsibility, well-defined interface?
- Units decomposed for independent understanding + testing?
- Follows plan file structure?
- New files already large? (Ignore pre-existing sizes.)

### Cognitive Load
- Flag abstraction where interface doesn't simplify what's behind it
- Flag paths requiring 3+ layer jumps for no hiding benefit
- Redundant indirection = defect, not style

### Performance Floor
- N+1 queries → flag for justification (fixed small set OK; dynamic loops over external calls not OK)
- Repeated external calls in loop that could batch → flag
- Retry without cap/backoff → **reject**
- Connection-per-request where pool/client exists → **reject**

### Standard Quality
- Dead code, unused imports, unreferenced vars
- Error paths handled (not silently swallowed)
- No magic numbers/strings without named constants where meaning non-obvious
- No commented-out code

### `@platform-modules/*` Seam Integrity (apply when diff touches `@platform-modules/*` packages)

**Canonical source:** `docs/standards/coding-standard.md` §§3–4.

- Donor default baked into a public param — locale/currency/tenant/region default at a seam **(R1) → reject**
- Identity factory — exported factory returns its input without validating/normalizing/freezing/deriving **(R2) → flag**
- Expensive native rebuilt per call — `new Intl.*Format`/`PluralRules`/`RegExp`/`TextEncoder` on every invocation, not memoized **(R3) → flag**
- Inferred `any` or leaked internal type in a public export signature → flag
- Provider SDK (`stripe`, AWS, CF) imported into core (`peerDependency` behind an adapter is the contract) → **reject**
- Core imports a provider at runtime — L0 must be zero-dep → **reject**
- Swappable provider wired directly into core with no adapter seam → flag
- Public export throws bare string/status instead of a named, contextful error → flag
- Trust boundary assumed from caller rather than enforced at the seam entry → flag
- New public export missing a co-located `*.test.ts` (vitest) covering at least one edge/fallback path → flag

### Cross-module / layer-DAG integrity (apply when diff adds/changes cross-`@platform-modules/*` imports or `package.json` deps)

Per-export checks above are blind to the *graph*. Cross-module defects are invisible per-file — verify against the **workspace dependency graph**, never infer by reading one diff.

- Dependency points **up or sideways** across layers — a foundation/L0 package (`util`/`db`/`events`) importing a higher layer, or any import violating the documented layer order → **reject**. Deps point **down-only**.
- **Cycle** between `@platform-modules/*` packages (A→B→A, direct or transitive) → **reject**. The graph is acyclic by contract.
- **Foundation gains a runtime dependency** — L0 must stay zero-runtime-dep; a provider SDK or sibling pulled into core instead of a `peerDependency` behind an adapter → **reject**.
- **Framework peer leaking into a core** — a `@platform-modules/<core>` gaining a `react`/framework peer, or a `-react` adapter depending on a second core → **reject** (poisons the zero-dep core; the framework axis is a flat sibling package).
- **Phantom dependency** — a cross-package import not declared in that package's `package.json` deps/peerDeps → flag.

**Enforce deterministically, not by eye:** the source of truth for cycles + direction is a dependency-graph check (`dependency-cruiser` with layer rules, or `madge --circular`), not a reviewer reading diffs — a transitive cycle is unseeable per-file.

## Return Format

```
## Code Quality Review

### Strengths
[What was done well]

### Issues
**Critical** (must fix before merge):
- file:line — description

**Important** (should fix):
- file:line — description

**Minor** (optional polish):
- file:line — description

### Assessment
✅ Approved / ❌ Needs fixes — [one sentence summary]
```

Omit category if no issues.