--- name: code-review description: "Review changed code against repository coding standards and original specification intent. Use when reviewing a branch, diff, PR, pull request, merge-base changes, or verifying code against documented standards — even if the user just says \"review this\". Do NOT use for authoring new code or diagnosing failing runtime bugs." --- Read [Studio policy](../../references/studio-policy.md) before this adapted workflow. User and host instructions override inherited defaults. All workflow dependencies must resolve locally using [the registry](../../references/integration-registry.json); never load companion plugins. # Code Review Two-axis review of the diff between `HEAD` and a fixed point: 1. **Standards**: Does the code conform to documented repository standards and clean architecture smells? 2. **Spec**: Does the code faithfully and completely implement the originating issue/spec without scope creep? Both axes execute as isolated parallel sub-agents to prevent context contamination, with findings reported side by side. --- ## Core Invariants 1. **Strict Two-Axis Separation**: Keep Standards and Spec findings isolated; never merge or cross-rank them into a blended score. 2. **Pinned Merge-Base Diff**: Always resolve refs with `git rev-parse` and review `git diff ...HEAD`. 3. **Evidence-Based Citations**: Every finding must quote the exact file, line range, and standard/spec clause violated. 4. **Tooling Non-Duplication**: Skip formatting, syntax, or lint errors that automated pre-commit tooling already catches. 5. **No Blind Approvals**: If the spec is missing, report "No spec provided - verified against standards only" explicitly. --- ## Architecture & Map of Content (MOC) ``` [ Pin Fixed Point ] ──► [ Identify Spec & Standards ] ──► [ Parallel Review Subagents ] ──► [ Side-by-Side Synthesis ] │ ┌────────────────────────────────┴────────────────────────────────┐ ▼ ▼ [ Standards Subagent ] [ Spec Subagent ] - Documented repo rules - Missing requirements - Fowler code smells - Unasked scope creep - Architectural boundaries - Flawed implementations ``` | Component | Responsibility | Evaluation Source | |---|---|---| | **Standards Axis** | Architecture smells, naming, cohesion | `CODING_STANDARDS.md`, `CONTRIBUTING.md`, smell baseline | | **Spec Axis** | Functional completeness, scope boundaries | Issue description, `specs/*.md`, user requirements | | **Aggregation** | Side-by-side balanced reporting | Verbatim findings categorized by axis | --- ## Step-by-Step Procedure (TWI) ### Step 1: Pin the Fixed Point & Diff - **Action**: Resolve the base ref and confirm a non-empty diff (`git diff ...HEAD`). - **Key Point**: Fail fast if the ref is invalid or the working tree is empty. - **Why**: Reviewing against an incorrect base compares irrelevant changes. ### Step 2: Extract Standards and Spec Sources - **Action**: Locate repository guidelines (`CODING_STANDARDS.md`) and originating requirements/issues. - **Key Point**: Equip the standards agent with Fowler smell baselines (Mysterious Name, Duplicated Code, Feature Envy, Primitive Obsession, Speculative Generality). - **Inline Checklist**: - [ ] Diff base confirmed - [ ] Standards docs identified - [ ] Spec/issue requirements extracted ### Step 3: Dispatch Parallel Sub-Agents - **Action**: Spawn Standards subagent and Spec subagent concurrently with dedicated prompts. - **Key Point**: Restrict each subagent to its designated domain (< 400 words per report). - **Why**: Combining standards and spec evaluation into a single pass leads to halo bias where clean code masks missing features. ### Step 4: Aggregate and Synthesize Report - **Action**: Present findings under `## Standards` and `## Spec` headers with actionable remediation recommendations. - **Key Point**: State the single most severe issue within each axis clearly. - **Why**: Clear prioritization helps authors address critical design issues first. --- ## Anti-Rationalization Guardrails | Tempting Rationalization | Binding Rule | Engineering Rationale | |---|---|---| | *"The code looks beautifully formatted, so it must be correct."* | **Standards pass $\neq$ Spec pass.** | Elegant code can completely fail to implement required business logic. | | *"It does what the ticket asked, so ignore messy architecture."* | **Spec pass $\neq$ Standards pass.** | Quick hacks that bypass standards generate severe technical debt. | | *"Merge both reviews into one combined score."* | **Strict two-axis separation.** | Blending scores obscures which dimension requires remediation. | | *"Point out minor indentation issues in the review."* | **Skip issues handled by automated linters.** | Manual review should focus on semantics, architecture, and intent. |