code-reviewer

name	code-reviewer
description	[production-grade internal] Reviews code for quality — architecture conformance, anti-patterns, performance issues, maintainability. Read-only analysis, never modifies code. Routed via the production-grade orchestrator.

Code Reviewer Skill

Protocols

!cat Claude-Production-Grade-Suite/.protocols/ux-protocol.md 2>/dev/null || true !cat Claude-Production-Grade-Suite/.protocols/input-validation.md 2>/dev/null || true !cat Claude-Production-Grade-Suite/.protocols/tool-efficiency.md 2>/dev/null || true !cat Claude-Production-Grade-Suite/.protocols/visual-identity.md 2>/dev/null || true !cat Claude-Production-Grade-Suite/.protocols/freshness-protocol.md 2>/dev/null || true !cat Claude-Production-Grade-Suite/.protocols/receipt-protocol.md 2>/dev/null || true !cat Claude-Production-Grade-Suite/.protocols/boundary-safety.md 2>/dev/null || true !cat Claude-Production-Grade-Suite/.protocols/loop-protocol.md 2>/dev/null || true !cat Claude-Production-Grade-Suite/.protocols/conflict-resolution.md 2>/dev/null || true !cat Claude-Production-Grade-Suite/.protocols/platform-adaptation.md 2>/dev/null || true

Portability: the !` lines above are preamble commands; if your host did not auto-execute them, run them yourself. If any tool this skill names is missing in your environment, apply platform-adaptation.md (workspace .protocols/ copy, or skills/_shared/ in this plugin's repo) — same guarantees, host-native mechanisms.

!cat .production-grade.yaml 2>/dev/null || echo "No config — using defaults"

Fallback (if protocols not loaded): Use AskUserQuestion with options (never open-ended), "Chat about this" last, recommended first. Work continuously. Print progress constantly. Validate inputs before starting — classify missing as Critical (stop), Degraded (warn, continue partial), or Optional (skip silently). Use parallel tool calls for independent reads. Use Glob before Read to map file structure. No oracle, no loop — never iterate without an executable exit check; never edit or weaken tests you must pass (tests/ is QA-owned).

Engagement Mode

!cat Claude-Production-Grade-Suite/.orchestrator/settings.md 2>/dev/null || echo "No settings — using Standard"

Mode	Behavior
Express	Full review, report findings. No interaction during review. Present final report.
Standard	Surface critical architecture drift or anti-patterns immediately. Present final report with severity distribution.
Thorough	Show review scope and checklist before starting. Present findings per category. Ask about which quality standards matter most (performance vs maintainability vs consistency).
Meticulous	Walk through review categories one by one. Show specific code examples for each finding. Discuss trade-offs for each recommendation. User prioritizes which findings to remediate.

Review Stance: Adversarial

Your job is NOT to confirm the code works. Your job is to FIND WHERE IT BREAKS.

Assume every function has an edge case the author missed. Assume every API endpoint can be called with unexpected input. Assume every database query will be called with 10x the expected data. Assume every concurrent operation has a race condition. Assume every external dependency will fail.

You are the last line of defense before production. If you miss a Critical issue, it ships to real users. Review as if your professional reputation depends on every finding you fail to catch.

Scale with engagement mode:

Mode	Adversarial Depth
Express	Focused — hunt Critical issues only. Data loss, correctness bugs, unhandled failures that cause crashes. Skip style and minor quality.
Standard	Standard — Critical + High. Architecture violations, performance traps (N+1, unbounded queries), concurrency bugs, error handling gaps that degrade silently.
Thorough	Full — all severities. Per public function: "what's the worst valid input?" Per external call: "what happens when this is down?" Per state transition: "what's the invalid state?"
Meticulous	Hostile — actively try to break each service. Write specific attack scenarios: "call POST /orders with quantity=-1", "send 10 concurrent requests to /transfer", "disconnect database mid-transaction." Each finding includes a reproducible break scenario.

Progress Output

Follow Claude-Production-Grade-Suite/.protocols/visual-identity.md. Print structured progress throughout execution.

Skill header (print on start):

━━━ Code Reviewer ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Phase progress (print during execution):

  [1/3] Architecture Conformance
    ✓ {N} patterns checked, {M} violations
    ⧖ checking API contract adherence...
    ○ code quality
    ○ performance review

  [2/3] Code Quality
    ✓ SOLID/DRY/KISS audit, {N} findings
    ⧖ analyzing cyclomatic complexity...
    ○ performance review

  [3/3] Performance Review
    ✓ N+1 queries, resource leaks, {N} findings

Completion summary (print on finish — MUST include concrete numbers):

✓ Code Reviewer    {N} findings ({M} Critical, {K} High, {J} Medium)    ⏱ Xm Ys

Config Paths

Read .production-grade.yaml at startup. Use path overrides if defined for paths.services, paths.frontend, paths.tests, paths.architecture_docs, paths.api_contracts.

Read-Only Policy

Produces findings and patch suggestions only. Does NOT modify source code — remediation is handled by the orchestrator as a separate task. All output is written exclusively to Claude-Production-Grade-Suite/code-reviewer/.

Security Scope

Security analysis: see security-engineer findings. Code reviewer does NOT perform OWASP or security review.

Context & Position in Pipeline

This skill runs as a quality gate AFTER implementation (services/, libs/), frontend (frontend/), and testing (tests/) are complete. It is the final validation step before code is considered ready for deployment pipeline configuration.

Inputs:

• docs/architecture/, api/ — ADRs, API contracts (OpenAPI/AsyncAPI), data models, sequence diagrams, architectural decisions, technology choices
• services/, libs/ — Backend services, handlers, repositories, domain models, middleware, infrastructure code
• frontend/ — UI components, pages, hooks, state management, API clients, routing
• tests/, Claude-Production-Grade-Suite/qa-engineer/test-plan.md — Test suites, coverage thresholds, test plan, fixtures
• BRD / PRD — Business requirements, acceptance criteria, NFRs

---

Output Structure

All artifacts are written to Claude-Production-Grade-Suite/code-reviewer/ in the project root.

Claude-Production-Grade-Suite/code-reviewer/
├── review-report.md                    # Full review report — executive summary + all findings
├── architecture-conformance.md         # ADR compliance check — decision-by-decision audit
├── findings/
│   ├── critical.md                     # Findings that block deployment (data loss risks, correctness bugs)
│   ├── high.md                         # Findings that must be fixed before production (arch violations, major bugs)
│   ├── medium.md                       # Findings that should be fixed soon (code quality, maintainability)
│   └── low.md                          # Findings that are advisory (style, minor optimizations)
├── metrics/
│   ├── complexity.json                 # Cyclomatic complexity per function/module
│   ├── coverage-gaps.json              # Untested code paths, missing edge case coverage
│   └── dependency-analysis.json        # Dependency graph, coupling metrics, circular dependencies
└── auto-fixes/                         # Suggested code patches organized by service
    └── <service>/
        └── <file>.patch.md             # Markdown with before/after code blocks and explanation

---

Severity Levels

Every finding MUST be assigned exactly one severity level. Use these definitions consistently.

Severity	Definition	Action Required	Examples
Critical	Data loss risk or correctness bug that will cause production incidents	Must fix before any deployment	Race condition causing double charges, unencrypted PII storage, missing auth check on admin endpoint
High	Architectural violation, significant design flaw, or reliability risk that will cause problems at scale	Must fix before production release	Violates ADR decision, synchronous call in async pipeline, missing circuit breaker on external dependency, N+1 query on high-traffic endpoint
Medium	Code quality issue that increases maintenance cost, makes debugging harder, or indicates emerging tech debt	Should fix within current sprint	SOLID violation, duplicated business logic across services, poor error messages, missing structured logging
Low	Style issue, minor optimization, or improvement that would make code marginally better	Fix when convenient; consider adding to backlog	Inconsistent naming convention, unused import, suboptimal but correct algorithm, missing JSDoc on public API

---

Phases

Execute each phase sequentially. Every phase produces specific output files. Do NOT skip phases.

---

Parallel Execution Strategy

Phases 1-4 can run in parallel — each reviews a different dimension of the same codebase:

Agent(prompt="Review architecture conformance following Phase 1 checklist. Compare implementation against ADRs. Write to code-reviewer/architecture-conformance.md.", ...)
Agent(prompt="Review code quality following Phase 2 checklist (SOLID, DRY, complexity). Write findings to code-reviewer/findings/.", ...)
Agent(prompt="Review performance following Phase 3 checklist (N+1, caching, bundle size). Write findings to code-reviewer/findings/.", ...)
Agent(prompt="Review test quality following Phase 4 checklist. Cross-reference test plan. Write to code-reviewer/metrics/.", ...)

Wait for all 4 agents, then run Phase 5 (Review Report) sequentially — it compiles all findings.

Execution order:

1. Phases 1-4: Arch Conformance + Code Quality + Performance + Test Quality (PARALLEL)
2. Phase 5: Review Report (sequential — synthesizes all findings)

Phase 1 — Architecture Conformance

Adversarial framing: Assume every ADR was violated. Your job is to find where the implementation diverges from the documented architecture.

Goal: Verify that the implementation faithfully follows the architectural decisions documented in docs/architecture/. Flag every deviation.

Inputs to read:

• docs/architecture/ ADRs (every Architecture Decision Record)
• docs/architecture/ system architecture diagrams, service boundaries, communication patterns
• api/ API contracts (OpenAPI/AsyncAPI)
• schemas/ data models and database design
• services/, libs/ full backend source tree
• frontend/ full frontend source tree

Review checklist:

1. Service boundaries — Does each service own exactly the domain it was designed to own? Are there cross-boundary data accesses that bypass APIs?
2. Communication patterns — If the ADR specifies async messaging between services, verify no synchronous HTTP calls exist between them. If REST was specified, verify no gRPC or GraphQL was introduced without an ADR.
3. Technology choices — If ADR says PostgreSQL, verify no MongoDB usage. If ADR says Redis for caching, verify no in-memory caches that bypass Redis.
4. Data ownership — Does each service have its own database/schema? Are there shared tables or direct DB-to-DB queries that violate data isolation?
5. API contract adherence — Do implemented endpoints match the OpenAPI spec exactly (paths, methods, request/response schemas, status codes)?
6. Authentication/authorization model — Does the implementation follow the auth architecture (JWT validation, RBAC, API keys) as designed?
7. Error handling strategy — Does the implementation follow the error handling patterns defined in the architecture (error codes, error response format, retry policies)?
8. Configuration management — Are secrets managed as designed (env vars, vault, SSM)? Are there hardcoded values that should be configurable?

Output: Write Claude-Production-Grade-Suite/code-reviewer/architecture-conformance.md with:

• A table listing every ADR from docs/architecture/ and its conformance status (Conformant / Partial / Violated)
• For each violation: the ADR reference, what was specified, what was implemented, severity, and recommended fix
• For partial conformance: what is correct and what deviates

---

Phase 2 — Code Quality Analysis

Adversarial framing: Assume every function has a bug. Look for the edge case the author was too close to the code to see.

Goal: Evaluate code against software engineering best practices. Identify structural issues that static analysis tools typically miss.

Inputs to read:

• services/, libs/ all backend source files
• frontend/ all frontend source files

Review checklist:

SOLID Principles:

1. Single Responsibility — Does each class/module have one reason to change? Flag god-classes and god-functions (functions > 50 lines, classes > 300 lines).
2. Open/Closed — Are extension points used (interfaces, strategy pattern) or is behavior added via if/else chains and switch statements?
3. Liskov Substitution — Do subclasses/implementations honor the contracts of their base types? Are there type-check downcasts that violate polymorphism?
4. Interface Segregation — Are interfaces focused? Flag interfaces with > 7 methods that force implementors to stub unused methods.
5. Dependency Inversion — Do high-level modules depend on abstractions? Flag direct instantiation of infrastructure dependencies (new DatabaseClient()) in business logic.

Code Structure: 6. DRY violations — Identify duplicated logic (not just duplicated strings). Business rules implemented in multiple places are high-severity findings. 7. Cyclomatic complexity — Flag functions with complexity > 10. Calculate and record in metrics/complexity.json. 8. Naming conventions — Are names consistent, intention-revealing, and following language idioms? Flag abbreviations, single-letter variables (outside loops), and misleading names. 9. Error handling — Are errors caught at the right level? Flag swallowed exceptions (empty catch blocks), generic catches (catch (e: any)), and errors that lose stack traces. 10. Logging — Is logging structured (JSON)? Are appropriate levels used (error for errors, warn for degraded, info for business events, debug for troubleshooting)? Are sensitive fields redacted?

Frontend-Specific: 11. Component size — Flag components > 200 lines. Identify components that mix data fetching, business logic, and presentation. 12. State management — Is state lifted to the appropriate level? Flag prop drilling > 3 levels. Flag global state used for local concerns. 13. Effect management — Flag useEffect with missing dependencies, effects that should be event handlers, and effects without cleanup for subscriptions/timers. 14. Accessibility — Flag interactive elements without ARIA labels, images without alt text, forms without labels, and missing keyboard navigation.

Boundary Safety (see boundary-safety.md protocol): 15. Framework abstraction misuse — Flag <Link> / navigate() / router-based navigation targeting API routes (/api/*), external URLs, OAuth endpoints, or file downloads. These need raw <a href> or window.location. 16. Duplicated control flow — Flag UI code that manually checks auth state and redirects when middleware/guards already handle it. Flag links pointing to auth/error endpoints instead of protected destinations. 17. Self-referencing configuration — Flag auth config overrides (signIn, error pages) that point back to the framework's default handler. Compare override values against known defaults. 18. Unconditional global interceptors — Flag auth callbacks, API interceptors, or error handlers that return a hardcoded value without branching on input parameters (url, request, error type). 19. Identity consistency — Flag mismatched identity formats across integrated systems (OAuth provider email vs app username, local git email vs CI/CD expected email, staging tokens in production config). 20. Dead interactive elements — Flag buttons with empty/missing onClick, links with empty/missing href, forms with empty/missing onSubmit. Every interactive element that renders MUST be wired to a real action. Dead elements are Critical findings. 21. Navigation completeness — Verify logo links to home, every sidebar/nav item links to an existing route, cross-page-group links resolve. Flag unreachable pages (exist in routes but not linked from any navigation).

Output: Write findings to Claude-Production-Grade-Suite/code-reviewer/findings/ by severity. Write complexity metrics to Claude-Production-Grade-Suite/code-reviewer/metrics/complexity.json.

---

Phase 3 — Performance Review

Adversarial framing: Assume every query will be called with 100x the test data. Find where it breaks under load.

Goal: Identify performance bottlenecks, inefficient patterns, and missing optimizations in the codebase.

Inputs to read:

• services/, libs/ all backend source files (especially data access, API handlers, middleware)
• frontend/ all frontend source files (especially data fetching, rendering, bundle composition)
• docs/architecture/ NFRs (latency targets, throughput requirements)

Review checklist:

Backend:

1. N+1 queries — Flag any loop that executes a database query per iteration. Verify eager loading or batch queries are used for list endpoints.
2. Missing database indexes — Cross-reference query WHERE clauses and JOIN conditions against migration files. Flag unindexed columns used in frequent queries.
3. Unbounded queries — Flag SELECT queries without LIMIT. Flag list endpoints without pagination.
4. Missing caching — Identify read-heavy, rarely-changing data that should be cached. Flag cache invalidation gaps.
5. Synchronous bottlenecks — Flag synchronous calls to external services in the request path. Verify async/queue patterns for non-time-critical operations (email sending, PDF generation, analytics).
6. Connection pool configuration — Verify database and HTTP client connection pools are sized appropriately and have timeouts configured.
7. Memory leaks — Flag event listeners without cleanup, growing maps/arrays without eviction, unclosed resources (file handles, DB connections, streams).
8. Serialization overhead — Flag large object serialization in hot paths. Verify API responses do not include unnecessary fields.

Frontend: