code-review

Files (6)

• SKILL.md
• openai.yaml · agents/
• bug-heuristics.md · references/
• code-smell-baseline.md · references/
• evidence-and-launch-checklist.md · references/
• tree-concept-gradient.md · references/

SKILL.md (first lines)

Review the diff between `HEAD` and a fixed point along a **risk-calibrated gradient** across three complementary axes:

- **Standards** — does the code conform to repository standards, clean code, and the simplicity triad (KISS/YAGNI/DRY)?
- **Spec** — does the code faithfully implement the originating issue / PRD / spec without omissions or scope creep?
- **Machine Evidence & Blast Radius** — does the change supply verifiable, machine-executable proof (active test passes, mutation / Fast Sabotage results, typechecks, headless DOM/a11y trees, AST feature-flag checks) commensurate with its blast radius on the code tree?

All three axes run as **parallel adversarial sub-agents** with clean contexts, eliminating confirmation bias and correlated hallucinations from the original generation prompt.

```mermaid
flowchart TD
    FixedPoint["1. Pin Fixed Point & Calibrate Tree Gradient<br/>(Trunk vs Branch vs Leaf · Identify Agent Traps)"]
    SpecSource["2. Identify Spec & Enforce Machine Evidence<br/>(Active Test Runs, Mutation / Sabotage, Headless DOM/a11y)"]
    Standards["3. Check Standards, Tooling & Simplicity Triad<br/>(KISS, YAGNI, DRY Rule of Three, Smell Baseline)"]
    Adversarial["4. Independent Discovery in Parallel<br/>(Standards + Spec + Machine Evidence + Red-Team Probes)"]
    Aggregate["5. Aggregate, Cross-Examine & Adjudicate<br/>(Attack Findings · Rebuttal · Confirmed/Rejected/Partial)"]
    Final20["6. Evaluate The Final 20%<br/>(Agent Hardening + ESCALATE_TO_HUMAN Contract)"]
    AutoFix["7. Auto-Fix Confirmed Findings (No Stop)<br/>(Regression Test + Minimal Surgical Fix + Refresh Evidence)"]
    Loop["8. Closed-Loop Re-Review Until Clean<br/>(Full Diff + Fix Diff + Prior Review Audit)"]

    FixedPoint --> SpecSource --> Standards --> Adversarial --> Aggregate --> Final20 --> AutoFix --> Loop
    Loop -->|confirmed findings remain| Adversarial
    Loop -->|zero confirmed findings| Done["Done: Exit Loop"]
```

---

## Process

### 1. Pin the fixed point and calibrate the risk gradient (The Tree Concept)

1. **Resolve the fixed point and comparison refs**:
   - Capture the fixed point (`main`, commit SHA, branch name, tag, etc.). If omitted, prompt the user — unless the user named paths/area instead, in which case review the current tree filtered to those paths and skip the empty-diff gate.
   - Run `git rev-parse <fixed-point>` and confirm the diff is non-empty.
   - Capture comparison commands:
     ```sh
     git diff <fixed-point>...HEAD
     git log <fixed-point>..HEAD --oneline
     ```
   - Note the base ref as the initial `<review-point>`. When later running subsequent verify-fixes passes, the comparison ref is `<review-point>..HEAD` (or uncommitted changes), where `review-point` is the commit SHA evaluated during the prior review pass.

2. **Model the codebase as a tree & calibrate agent review posture**:
   Human reviewers skim Leaf code to save time and manually inspect Trunk code. Agent reviewers must take the opposite posture to guard against LLM failure modes:
   - **Trunk Code (High Blast Radius — The Agent Trap)**: Core infrastructure, networking, authentication/authorization engines, database schemas and migrations, global state managers, base framework lifecycles, and shared foundational utility packages.
     - *Agent Failure Mode*: An LLM reviewer is easily tricked by Trunk code that looks syntactically clean and idiomatic, missing subtle concurrency races, distributed deadlocks, schema lock contention, or caching edge cases.
     - *Agent Review Posture*: Do not trust syntax alone. Demand independent verification ([testing](../testing/SKILL.md)), require mutation falsification (`mutation_score >= 0.90` or documented pass of the Fast Sabotage Litmus Test), audit strict backwards compatibility, and **route to the Human Escalation Gate** for architectural sign-off.
   - **Branch Code (Moderate Blast Radius)**: Domain business logic, bounded feature services, API controllers, worker processors, and shared component modules.
     - *Agent Review Posture*: Dual-axis Standards + Spec review with active execution of automated unit/integration test suites.
   - **Leaf Code (Low / Zero Blast Radius — The Agent Strength)**: Isolated UI views, leaf endpoints, standalone scripts, or feature-gated components that can be safely disabled without collateral impact.
     - *Agent Review Posture*: High-velocity, exhaustive mechanical audit. Verify 100% of types, prop contracts, linter constraints, and headless DOM/a11y trees. Perform an **AST feature-flag check** to prove the feature is wrapped in a dynamic toggle with a working fallback.
   *(See [references/tree-concept-gradient.md](references/tree-concept-gradient.md) for detailed classification).*

3. **Calibrate operational exposure**:
   - Ask: **has any of this diff already run outside local development?** (deployed, migrated, sent real traffic/email/data). Pre-production code can be directly restructured; shipped code requires non-destructive, forward-compatible migrations.

4. **Unreleased code & legacy retention policy**:
   - When deciding whether to keep legacy methods, deprecated routes, fallback mechanisms, or dual-writes on development branches, check if `main` has already merged the legacy code. If `main` has never merged or released it, delete the legacy code and keep only the canonical implementation (YAGNI/KISS). Reviewers must not flag the removal of unreleased legacy paths as a breaking change.

---

### 2. Identify the spec source and demand machine-verifiable evidence

1. **Locate the originating spec** in order:
   - Issue tracker references in commits (`#123`, `Closes #45`, GitLab `!67`) — fetch via `docs/agents/issue-tracker.md` (run `/setup-matt-pocock-skills` if `docs/agents/issue-tracker.md` is missing).
   - A path passed by the user.
   - PRD or spec file under `docs/`, `specs/`, or `.scratch/` matching the branch or feature.
   - If missing, ask the user for the spec.
   - If no spec exists, run in **degraded mode**: treat commit messages as a weak spec and verify self-consistency (flagging commit claims not honored or unmentioned diff behavior). Mark as `Spec (degraded — self-consistency vs commit messages)`.

2. **Machine-Verifiable Evidence Gate**:
   An agent reviewer requires active, reproducible proof rather than passive text claims. Summary scorecards and detailed protocol live in [references/evidence-and-launch-checklist.md](references/evidence-and-launch-checklist.md):
   - **Active Execution**: The reviewer executes the test suite, confirming exit code `0` and clean stderr without unhandled rejections or memory warnings.
   - **Logic Falsification**: Require mutation testing (`mutation_score >= 0.90`) **or** a documented pass of the Fast Sabotage Litmus Test.
   - **Static & Structural Validation**: Confirm clean typecheck passes (`tsc --noEmit`), headless DOM/a11y tree structures for UI components, and AST proof of feature-flag wrapping.

---

### 3. Identify standards sources and the simplicity triad

1. **Survey standards**:
   - Check `CONTRIBUTING.md`, `CODING_STANDARDS.md`, agent instructions (`AGENTS.md`, `CLAUDE.md`, `.agents/**`), architecture docs, and README files.
   - **Check what tooling enforces first**: Lint configs, formatters, type-checkers, CI gates. Anything tooling already enforces is **out of scope** for agent review; focus strictly on logic, design, security, and maintainability.

2. **The methodology test**:
   Every finding must demonstrate harm to one of three outcomes: **changeable**, **readable**, or **testable**. State the concrete maintenance cost, not just stylistic preferences.

3. **KISS, YAGNI, and DRY Simplicity Triad**:
   - **KISS (Keep It Simple, Stupid)**: Favor the most straightforward, readable solution for every line. Extreme readability over cleverness.
   - **YAGNI (You Aren't Gonna Need It)**: Code only for current, proven requirements. Reject speculative helpers, single-use classes, extra parameters, or premature generalizations.
   - **DRY vs. YAGNI (The Tension)**: Enforce the **Rule of Three** — abstract only on the third occurrence. Duplication predates the diff; grep codebase-wide to confirm the true count before suggesting extraction.
   - **DRY vs. KISS (The Balance)**: A little duplication is far better than a bad, confusing abstraction (**KISS wins**).

4. **Code Smell Baseline**:
   Apply the smell baseline (Naming & Structure, Duplication & Coupling, Overengineering, Bigness, Error Flow, Tests as Review Surface) as defined in [references/code-smell-baseline.md](references/code-smell-baseline.md).

---

### 4. Run the reviews (Adversarial Agentic Topology)

1. **Topology & Adversarial Isolation**:
   - Launch **three parallel sub-agents** with clean, isolated contexts:
     1. **Standards Sub-agent** (correctness / requirements / maintainability)
     2. **Spec Sub-agent** (spec fidelity; attacks the spec too — implementation must satisfy actual requirements, not mere internal coherence)
     3. **Machine Evidence Sub-agent** (security / edge cases / robustness + active execution)
   - **Independent discovery first**: Reviewers do not see each other's findings during discovery. Sub-agents must run in fresh contexts without access to the author agent's prompt history or sibling findings. This breaks **confirmation bias** and prevents **correlated LLM hallucinations**.
   - **Assume-broken mindset**: Each reviewer assumes the implementation is wrong, fragile, or exploitable and tries to prove it with a concrete counterexample.

2. **Red-Team Probe Mandate (tiered)**:
   - **Trunk**: concurrency/race, duplicate/non-idempotent requests, lying/malicious client, partial dependency outage, auth bypass, unbounded loop/allocation, cross-user data leak, 0/negative/empty/huge/malformed inputs.
   - **Branch**: duplicate/idempotency, null/empty/boundary values, malformed dependency responses, cross-tenant leak, missing authorization on new paths.
   - **Leaf**: prop/state extremes (empty/loading/error/populated), flag-off fallback, keyboard/a11y path only. No retry queues or speculative hardening.
   - Every probe needs a runnable `input/state → actual vs expected` sketch. Hypotheticals without a realistic caller/path are dropped or capped at Low.

3. **Partitioning Large Diffs**:
   - For large diffs (>2,000 lines or >40 files), partition the **Standards** axis by architectural area (e.g., `api/`, `web/`, `infra/`, `tests/`) into additional parallel sub-agents. Give every partition identical methodology text; partition coverage, not judgment.
   - The Spec agent and Evidence agent always evaluate the whole diff.

3. **Interface & Boundary Checks**:
   - When diffs cross boundaries (API ↔ client, schema ↔ model, template ↔ CSS, docs ↔ code), assign an explicit cross-surface pass to verify contract alignment.

From werlang/user-skills, MIT.

More from werlang/user-skills