Skip to content

SOP-054 — Code review (Engineering Director)

Guide

<!-- last-reviewed: 2026-06-01 · owner: Engineering (Engineering Director) -->

SOP-054 — Code review (Engineering Director)

SOP · owner: Engineering (Engineering Director). Layer: SOP/Runbook (docs/canon.md). The Director’s review authority over any product code before it ships. Conventions are in CLAUDE.md (linked, not restated — one home per fact).

Interface Contract

  • Inputs: product-code diff / review request ← from <Engineering member or upstream unit> · trigger: review requested
  • Inputs: requirements / constraints (Legal, spec, schema) ← from <issuing unit> · trigger: requirements issued
  • Inputs: architecture decision (ADR / approve-revise) ← from <CPTO or reviewer> · trigger: review passed
  • Inputs: accepted sprint story set ← from <Product (Product Manager)> · trigger: story set accepted
  • Inputs: approved UI design spec / design-token reference ← from <Creative (Visual Designer / Creative Director)> · trigger: ui design spec approved
  • Outputs: reviewed code change → to Staff Engineer / Platform Engineer · trigger: review passed
  • Outputs: implementation risk against a story → to Product (Product Manager) · trigger: implementation risk raised
  • Outputs: approved ADR / technical spec → to Archivist · trigger: artifact approved
  • Handoff trigger (in): a member’s product-code change is build-green and ready for review; or requirements / an architecture decision / an accepted story set arrives for the Engineering unit to act on
  • Handoff trigger (out): the change passes review against the conventions, or routes back with rationale; implementation risks are raised back to Product
  • Definition of Done before handoff:
    • [ ] cd intranet && npm run build reported green by the author (~50 pages, [sync] …)
    • [ ] Diff read in full against CLAUDE.md Coding Conventions + CONTRIBUTING.md
    • [ ] No new pattern where an existing one fits; no scope creep into Operations/DA/AI territory
    • [ ] Architecture decisions (>1 file / new boundary) were sketched + confirmed, not unilateral
    • [ ] Any durable decision captured as an ADR handed to the Archivist (artifact approved)
    • [ ] Learnings captured to docs/knowledge/ if the review surfaced one

review requested / review passed are drawn from docs/handoff-triggers.md. The review requested in-edge is an intra-unit handoff from the Staff/Platform Engineer; the Q&R fan-in sink also absorbs review requested for independent assurance (interface-contract-spec.md §3.1 R1) — this SOP is the Director’s first-line review, not the Q&R second-line assurance.

Purpose

Ensure every product-code change is correct, convention-compliant, and free of silent architecture drift before it merges — so the build stays green and the codebase stays extensible.

When to use

Whenever a Staff or Platform Engineer reports a change build-green and ready for review. If the build is not green, stop — the author fixes it first (review is not your trigger yet).

Flow

flowchart LR
  A["review requested<br/>(build green)"] --> B["Read full diff<br/>vs CLAUDE.md"]
  B --> C{"Conventions met?<br/>no scope creep?"}
  C -->|no| D["Return with<br/>rationale"]
  C -->|yes| E["review passed"]
  D --> A
  classDef s fill:#1d4ed8,color:#fff,stroke:#1e3a8a
  classDef d fill:#0f766e,color:#fff,stroke:#134e4a
  class A,B,D,E s
  class C d

Text fallback: a build-green change arrives (review requested) → read the full diff against CLAUDE.md conventions → if conventions are met and there is no scope creep, emit review passed; otherwise return the diff with specific rationale and re-review.

Steps

  1. Confirm the build is green. The author must report npm run build output (green, ~50 pages). No build evidence → return.
  2. Read the diff in full — not just changed lines. Apply CLAUDE.md Coding Conventions (PascalCase components, kebab-case scripts, camelCase vars, two-space indent, single quotes, ESM imports) and CONTRIBUTING.md.
  3. Check architecture discipline. Any change touching >1 file or a new module boundary must have been sketched + confirmed (charter rule 2). A unilateral architecture change is a defect — return it.
  4. Check scope. Reject creep into Operations (pipeline), Data & Analytics (schema/ scoring), or AI Systems (prompts/models) — flag those as edges, not edits.
  5. Capture durable decisions as an ADR; hand to the Archivist (artifact approved).
  6. Emit review passed — the change is clear to merge per the release procedure (SOP-055).

Roles / approvers

Run by the Engineering Director (review authority). Author is the Staff or Platform Engineer. ADR approval follows docs/sop/change-management.md (SOP-002 RACI).

Guardrails

  • Director approval is first-line, not independent (README.md R11) — it is recorded but does not substitute for the Q&R sample.
  • Do not restate CLAUDE.md here — link to it.
  • Security-dimension changes → flag to the Principal (Security unstaffed); do not approve security policy.
  • Personal information is PA-only — reject any code that reads/publishes it.

Job aid: (N/A — review is judgment work; the convention checklist lives in CLAUDE.md; no separate cheat-sheet adds value, GR-4). See also: docs/units/engineering/manuals/engineering-director.md · SOP-055 · docs/sop/change-management.md · CLAUDE.md · docs/handoff-triggers.md.