Appearance
Code Review Checklist Standard
AI Copy Block (
AGENTS.md)
markdown
<!-- START AGENT-STANDARD: CODE-REVIEW -->
## Code Review Checklist Rules
- [ ] **Architecture & Modular Design**: Verify Clean/Hexagonal inward dependency flow. Enforce separation between core domain logic, infrastructure adapters, and UI rendering. Ban framework/ORM leakage into core logic, enforce server-only import guards (`import 'server-only'`), mandate explicit monorepo package export maps with `"types"` condition-first ordering, restrict adapter wiring exclusively to the application Composition Root, and flag over-engineering or premature abstractions (favor YAGNI/KISS).
- [ ] **Security & Zero-Trust Input Validation**: Ensure zero-trust boundary input validation using strict schemas (e.g. Zod/Pydantic) that reject unknown fields (400 Bad Request). Prevent mass assignment (BOPLA) via explicit write allowlists. Enforce BOLA/IDOR scoping by user/tenant ID on all server queries and Server Actions / RPC endpoints. Ban credentials/tokens/PII in `localStorage` (require `httpOnly`, `Secure`, and `SameSite` cookies). Verify rate limiting / IP throttling on public endpoints & Server Actions, strict CORS origin whitelisting for decoupled origins, parameterized SQL, DOMPurify HTML sanitization, Anti-CSRF headers, dynamic import guards, client-generated idempotency keys (`X-Idempotency-Key`) on non-idempotent stateful mutations, and framework client env prefixes (`NEXT_PUBLIC_`, `VITE_`).
- [ ] **Data Safety & Database Persistence Invariants**: Enforce Expand-Migrate-Contract zero-downtime database migrations (additive schema changes first, contract in follow-up deployments). Enforce defensive repository schema mapping. Ban N+1 query patterns, enforce query statement timeouts, mandate zero network I/O inside DB transactions, mandate transactional outbox persistence with explicit status tracking (`PENDING`, `PROCESSING`, `PUBLISHED`, `FAILED`), lock timeout recovery, `SKIP LOCKED` worker concurrency with aggregate-level stream locking/partitioning, W3C Trace Context (`traceparent`, `tracestate`, `baggage`) event payload propagation, automated retention pruning, and consumer idempotency/DLQ routing for cross-boundary domain events, and verify mandatory 4-key audit metadata (`created_at`, `updated_at`, `created_by`, `updated_by`).
- [ ] **Automated Testing & QA Verification**: Verify unit (60%), component (30%), and E2E smoke (10%) coverage. Test behavior over implementation (prefer `getByRole` over `getByTestId`). Mandate MSW for network mocking, fake timers for async control, Playwright `storageState` for auth context reuse, visual regression snapshots (Chromatic/Percy/Playwright) on shared UI component/design token PRs, and explicit E2E paths for CRUD, auth, and error boundaries. Ensure strict typed test data stubs without unsafe `as` type assertions.
- [ ] **Performance & Resource Utilization Baseline**: Enforce Core Web Vitals SLA (LCP <= 2.5s, INP <= 200ms, CLS <= 0.1). Require dynamic imports for heavy client components, explicit image dimensions/aspect ratios, path-level tree-shakeable imports, and `font-display: swap`. On the backend, mandate explicit socket/network timeouts on HTTP/RPC calls, collection endpoint pagination, and bounded concurrency. Keep total PR changeset size strictly under 200-400 LOC and maintain review speed between 200-400 LOC/hour to optimize defect detection efficiency.
- [ ] **Error Handling & Resilient Fault Isolation**: Require the standard 5-key error JSON envelope (`code`, `message`, `details`, `timestamp`, `request_id`). Require transport handlers to map domain exceptions into HTTP/RPC status codes (400, 401, 403, 404, 409, 429, 500) and mask raw unhandled internal 500 errors. Require guard clauses (early returns) over deeply nested logic. Ensure infrastructure/driver exceptions are translated into typed domain errors preserving internal cause (`Error.cause`). Enforce error boundary telemetry forwarding to APM services (Sentry/Datadog) with trace context.
- [ ] **Accessibility (a11y) & UX Baseline**: Enforce WCAG 2.2 AA compliance (minimum 4.5:1 color contrast). Verify keyboard navigation via WAI-ARIA authoring patterns and forbid `outline: none` without accessible focus replacements. Prefer semantic HTML tags over `div`+ARIA. Mandate `aria-live` announcements for async feedback. Require shareable UI state to live in URL search parameters rather than duplicate local state.
- [ ] **Operational Readiness & Observability**: Require dual health probes (`/healthz/liveness` and `/healthz/readiness`). Enforce structured JSON logging with W3C `traceparent` correlation ID context, vendor-agnostic OpenTelemetry (OTel) SDK instrumentation, and automatic PII masking. Verify graceful SIGTERM/SIGINT shutdown with 30-second in-flight request drain. Ensure container resource limits, non-root user execution, and pre-flight CI smoke test gates.
<!-- END AGENT-STANDARD: CODE-REVIEW -->Detailed Human Guide & Rationale
1. Architecture & Modular Design
- Clean Architecture & Inward Dependency Flow: All pull requests MUST respect clean/hexagonal architecture principles. Core business domain logic must remain framework-agnostic and free of direct dependencies on databases, ORMs, HTTP routers, or external third-party SDKs.
- Adapter & Boundary Isolation: Storage mechanisms, protocol handlers (REST, gRPC, GraphQL), and UI controllers must act as peripheral adapters communicating with domain models exclusively through typed ports (interfaces). Server-only code must be guarded with build-time assertions (e.g.,
import 'server-only') and restricted via explicit packageexportsmaps using"types"condition-first ordering. Adapter instantiation and dependency injection MUST take place exclusively at the application Composition Root (e.g.,main.ts/ container bootstrap), injecting concrete interface implementations into domain/application services via constructors. - YAGNI, KISS & Review Velocity: Code additions must address concrete requirements without introducing speculative abstractions, generic wrappers for single implementations, or unneeded configuration options. Per empirical research, PR changesets should ideally stay under 200 lines of code (LOC) and not exceed 400 LOC to maintain reviewer defect detection efficiency.
- Primary Sources & Rationale: Grounded in Robert C. Martin's Clean Architecture (2017), Alistair Cockburn's Ports and Adapters Architecture (2005), Google Engineering Practices Code Review Developer Guide (favor simplicity and reject over-engineering), and SmartBear/Cisco Code Review Study (lightweight reviews under 200-400 LOC optimize defect detection and review velocity).
2. Security & Zero-Trust Input Validation
- Zero-Trust Boundary Schema Validation: Every API endpoint and input boundary MUST validate untrusted payloads against strict runtime schemas (e.g., Zod, Valibot, Pydantic). Requests containing unexpected or extraneous parameters MUST be rejected immediately with a
400 Bad Requesterror to prevent mass assignment (BOPLA) vulnerabilities. - BOLA / IDOR Scoping: Server-side query handlers MUST scope data access directly by the authenticated user or tenant ID extracted from verified session tokens. Client-provided IDs in paths or query parameters must never be trusted without explicit authorization checks.
- Credential & Secret Hygiene: Authentication tokens (JWTs, session IDs) and sensitive user PII MUST NEVER be stored in
localStorageorsessionStorage; they must be transmitted strictly viahttpOnly,Secure, andSameSite=Lax/Strictcookies. Environment variables embedded into frontend client builds must be strictly restricted to framework public prefixes (NEXT_PUBLIC_,VITE_). - Injection & Cross-Site Attack Protections: Database queries must exclusively use parameterized SQL or ORM query builders. Rendered HTML content containing dynamic user input must be sanitized via DOMPurify. Stateful non-GET API routes and Server Actions / RPC endpoints must validate Anti-CSRF headers, double-submit cookies, strict CORS origin whitelisting, and rate limiting / IP throttling rules before executing domain logic.
- Dynamic Import Security Guards: Dynamic module imports (
import(...)) accepting runtime parameters MUST resolve strictly through static module maps or hardcoded allowlists to prevent arbitrary path injection and unsafe dynamic code execution. - Cross-Boundary Mutation Idempotency: Stateful, non-idempotent API mutations (such as payment processing or order submission) MUST require client-generated idempotency keys (e.g.,
X-Idempotency-Key) and server-side deduplication to handle network retries safely without duplicate side effects. - Primary Sources & Rationale: Grounded in OWASP Top 10 (A01:2021 Broken Access Control, A03:2021 Injection, A08:2021 Software and Data Integrity Failures), OWASP Code Review Guide (v2.0), and NIST SP 800-63B Digital Identity Guidelines.
3. Data Safety & Database Persistence Invariants
- Expand-Migrate-Contract Migrations: Database schema modifications MUST follow the zero-downtime Expand-Migrate-Contract pattern. All schema changes in a PR must be strictly additive (adding nullable columns or new tables). Modifying existing columns or dropping legacy fields requires a phased release process across separate deployments.
- Defensive Repository Mappers: Persistence mappers inside repositories MUST map database records defensively to domain models, handling missing or transitioning schema fields gracefully during rolling deployments.
- Transaction Isolation & I/O Hygiene: Database transactions must be kept as short as possible. Performing network I/O, external HTTP requests, or third-party service calls inside an active database transaction is strictly prohibited. State-changing operations that emit domain events MUST use a transactional outbox pattern (persisting event records inside the DB transaction). Outbox relay workers MUST combine
SELECT ... FOR UPDATE SKIP LOCKEDwith explicit lifecycle status tracking (PENDING,PROCESSING,PUBLISHED,FAILED), lock timeout recovery for crashed workers, aggregate-level locking/partitioning for strict sequence ordering, and automated retention/pruning. Outbox event payloads MUST propagate full W3C Trace Context (traceparent,tracestate,baggage) in event metadata so consumers can bind correlation identifiers into execution contexts. Asynchronous event consumers MUST implement idempotency checks and Dead-Letter Queue (DLQ) routing for poison-pill messages. - Query Performance & Audit Metadata: Reviewers must verify that queries do not introduce N+1 access patterns and that non-trivial queries execute within defined statement timeouts. Every persistent entity table MUST maintain mandatory 4-key audit metadata:
created_at,updated_at,created_by, andupdated_by. - Primary Sources & Rationale: Grounded in Fowler's Refactoring Databases: Evolutionary Database Design, PostgreSQL Reliability Best Practices, and AWS Well-Architected Framework (Relational Database Design).
4. Automated Testing & QA Verification
- Test Pyramid & Coverage Distribution: Code submissions must include comprehensive test coverage adhering to a balanced test pyramid: ~60% isolated unit tests (focusing on business domain rules and custom hooks), ~30% component integration tests, and ~10% end-to-end (E2E) smoke tests.
- Behavior-Driven Component Testing: Component tests MUST test user-observable behavior rather than internal state or implementation details. DOM queries should prefer accessible roles (e.g., React Testing Library
getByRole) over implementation-dependent selectors likegetByTestId. - Network Mocking & Async Control: Network requests in integration tests MUST be intercepted using Mock Service Worker (MSW) or deterministic fixture servers rather than monkey-patching
fetchoraxios. Asynchronous state transitions must use fake timers to eliminate flaky test execution. - Mandatory E2E Paths & Strict Typing: E2E test suites must validate complete CRUD workflows, authentication flows, and error boundary fallbacks, leveraging Playwright
storageStatefor efficient auth context reuse. Test files MUST NOT use unsafeastype assertions (such asas anyoras unknown as Tcasts) or un-typed stubs; test data generators or strict schema builders must be used instead. - Visual Regression Testing: Pull requests modifying shared component libraries, design tokens, or CSS baselines SHOULD execute automated visual snapshot comparisons (e.g., via Chromatic, Percy, or Playwright visual testing) to prevent layout regressions across downstream views.
- Primary Sources & Rationale: Grounded in Google Engineering Practices (Testing and Reviewing Guidelines), Kent C. Dodds' Testing Trophy, and Martin Fowler's Practical Test Pyramid.
5. Performance & Resource Utilization Baseline
- Core Web Vitals SLA Compliance: Frontend PRs must maintain explicit SLA targets: Largest Contentful Paint (LCP) <= 2.5 seconds, Interaction to Next Paint (INP) <= 200 milliseconds, and Cumulative Layout Shift (CLS) <= 0.1.
- Bundle Optimization & Render Protection: Heavy client components or third-party visual libraries must be lazy-loaded using dynamic code-splitting imports. Images and media assets MUST specify explicit
width,height, oraspect-ratioattributes to prevent layout shifts. Library imports must use tree-shakeable path references, and custom fonts must enforcefont-display: swap. - Backend Timeout & Concurrency Limits: Outbound HTTP/RPC client calls to third-party APIs MUST configure explicit socket, connect, and read timeouts. Collection endpoints returning lists MUST enforce pagination bounds (e.g., maximum page size limits) to prevent unbounded memory allocation. Concurrent backend tasks MUST enforce bounded execution limits (e.g., worker pool caps, semaphores, or rate limiters) to prevent resource exhaustion under peak traffic.
- Review Velocity & Defect Rates: Code reviews must be performed efficiently. Empirical data indicates that defect detection rates drop significantly when reviewing code at speeds higher than 500 LOC per hour; ideal review velocity is between 200 and 400 LOC per hour (distinct from keeping overall PR changeset size under 400 LOC).
- Primary Sources & Rationale: Grounded in Google Web Vitals SLA Specifications, SmartBear/Cisco Code Review Research (review rate vs. defect density curves), and High Performance Browser Networking (Ilya Grigorik).
6. Error Handling & Resilient Fault Isolation
- Standardized 5-Key Error Envelope: All API error responses across services MUST follow a unified 5-key JSON error structure containing
code(application error code string),message(human-readable message),details(array of contextual validation/error objects),timestamp(ISO-8601 string), andrequest_id(correlation/trace identifier string). - Guard Clauses & Structural Clarity: Code logic MUST favor guard clauses (early returns) over deeply nested
if/elsecontrol structures, keeping the main happy-path execution aligned at the left margin. - Protocol Status Mapping & 500 Sanitization: Transport handlers MUST map typed domain exceptions into appropriate protocol status codes (e.g., 400 Bad Request, 401 Unauthorized, 403 Forbidden, 404 Not Found, 409 Conflict, 429 Too Many Requests) and strictly mask unhandled internal system errors as sanitized
500 Internal Server Errorenvelopes without leaking raw database queries or runtime stack traces. - Domain Exception Translation: Infrastructure drivers, ORM exceptions, and network errors must be caught at repository or gateway boundaries and mapped into typed domain exceptions (e.g.,
EntityNotFoundError,DomainConflictError), preserving the lower-level exception as an internalcauseproperty (e.g.,Error.cause) for APM logging. Low-level stack traces or raw database error strings must never leak to API callers. - Error Telemetry Integration: Frontend UI error boundaries and backend uncaught exception handlers MUST forward error payloads along with correlation trace IDs to centralized observability platforms (e.g., Sentry, Datadog) rather than silently suppressing exceptions.
- Downstream Resilience & Fault Isolation: External HTTP/RPC client calls to downstream dependencies MUST incorporate circuit breakers, bulkheads, and exponential backoff retry policies with jitter to isolate downstream failures and prevent cascading system collapse.
- Primary Sources & Rationale: Grounded in Michael Nygard's Release It! Design and Deploy Production-Ready Software (Circuit Breaker & Bulkhead patterns), OWASP Error Handling Guidelines, and REST API Error Response Standards.
7. Accessibility (a11y) & UX Baseline
- WCAG 2.2 AA Compliance: All visual interfaces MUST meet WCAG 2.2 Level AA criteria, including maintaining a minimum text-to-background visual contrast ratio of 4.5:1 (3:1 for large text).
- Keyboard Navigation & ARIA Patterns: All interactive elements MUST be operable via keyboard (Tab, Enter, Space, Arrow keys) following WAI-ARIA Authoring Practices. Default browser focus indicators MUST NOT be removed with
outline: noneunless replaced with a visible, accessible custom focus indicator. - Semantic HTML First: Developers must use semantic HTML tags (
<main>,<nav>,<article>,<button>,<header>) rather than generic<div>or<span>elements augmented with ARIA roles. - Dynamic Feedback & URL State Sync: Async operation states (loading spinners, form submit updates, error banners) MUST announce changes to assistive technologies via
aria-liveregions ("polite" or "assertive"). Shareable UI state (filters, search queries, pagination indices) SHOULD reside in URL search parameters rather than ephemeral component state. - Primary Sources & Rationale: Grounded in W3C Web Content Accessibility Guidelines (WCAG 2.2 Level AA Standard), WAI-ARIA Authoring Practices Guide (APG), and WebAIM Contrast Standards.
8. Operational Readiness & Observability
- Dual Health Probes: Backend application deployments MUST expose distinct liveness (
/healthz/liveness) and readiness (/healthz/readiness) endpoints. Liveness probes confirm process responsiveness, while readiness probes verify downstream dependency health (database, cache, message broker connections). - Structured JSON Logging & Trace Propagation: Logs MUST be written to
stdout/stderrin structured JSON format. HTTP/RPC request handlers must extract or inject W3Ctraceparentheaders, propagatingtrace_idandspan_idacross log contexts for distributed tracing. Distributed tracing and metrics MUST be instrumented using vendor-agnostic OpenTelemetry (OTel) SDKs or OTel-compatible libraries across server and BFF layers, avoiding vendor-proprietary agent lock-in. - Automatic PII Masking: Log formatters and telemetry pipelines MUST apply automatic redaction rules to prevent sensitive data (passwords, credit card numbers, auth tokens, personal identities) from being recorded in persistent log aggregators.
- Graceful Shutdown & CI Deployment Safety: Applications must listen for
SIGTERMandSIGINTsignals, immediately turning readiness probes unhealthy and draining in-flight HTTP requests over a 30-second grace period before closing database connection pools. CI pipelines MUST run automated contract diff checks and container pre-flight smoke tests prior to traffic cutover. - Container Hardening & Resource Allocation: Containerized applications MUST execute as unprivileged non-root users (
USER nonrootor explicit UID) and declare explicit CPU and memory resource requests and limits in deployment manifests to prevent host resource starvation and unauthorized container breakout. - Primary Sources & Rationale: Grounded in The Twelve-Factor App methodology (Factor XI: Logs, Factor IX: Disposability), W3C Trace Context Specification, and Google Site Reliability Engineering (SRE) Book.