# Code Review

> Review branch, PR, commit, or WIP diffs against repository standards and intended behavior.

- Skill: `sgaabdu4/code-review` (Agent Skill, multi-file: 3 files)
- Install (CLI): `npx skillmds@latest add sgaabdu4/code-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/sgaabdu4/code-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: sgaabdu4 (https://skillmd.com/u/sgaabdu4)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/sgaabdu4/code-review

---


# Code Review

## Target

- Base = user ref → upstream merge-base → `origin/main` → `main`.
- Prove base resolves + combined committed/WIP evidence is non-empty; else `FAIL`.

| Scope | Evidence |
|---|---|
| Committed | `git diff <base>...HEAD` + `git log <base>..HEAD --oneline`; commit range → each patch + cumulative diff |
| WIP | cached diff + unstaged diff + every untracked in-scope file |

- Review full hunks + nearby owners; stats/names/subjects = insufficient.

## Axes

| Axis | Question |
|---|---|
| Standards | Correct + safe + maintainable + tested + repo-compliant + gate-clean? |
| Spec | Originating behavior complete + faithful + no scope creep? |

- Behavioral source exists/likely → [spec.md](references/spec.md); confirmed absent/standards-only → skip Spec + why.
- One issue affecting both axes → distinct evidence per axis; never merge/rerank.

## Review

- Standards = applicable repo rules/docs; repo rules override heuristics.
- Finding = exact file:line/hunk + code fact + risk + simpler fix + confidence.
- Coverage/blast radius = apply global `AGENTS.md` Evidence contract.

| Lens | Challenge |
|---|---|
| Architecture | `codebase-design`; report structural contract/ownership gaps |
| Tests | `test-quality`; report unproven behavior/strength gaps |
| Ripple | null/empty + concurrency/timezone + permission/network + downstream |
| Security | `security-review`; report confirmed/unknown security evidence |
| UI | `atomic-ui`; report token/component/visual-SSOT gaps |
| Specialist | touched stack + performance + DevOps only |

- Smells = mystery name + duplication + data clump/primitive obsession + repeated switch + shotgun/divergent change + message chain/middle man + speculative layer.
- Also challenge = wrong owner + scattered flags + casts/nullable modes + hidden fallback + non-atomic orchestration + oversized owner + implementation-detail test.
- Required change ≠ taste; prefer deletion + owner reuse + simpler invariant.
- Gates/hooks/pre-push = `deterministic-checks`; report commands/exits/reports + suppression/wiring gaps.
- Required real UI proof = `e2e`; absent evidence = unknown, never reviewer-inferred `PASS`.
- Final audit = reject uncited, preference-only, overstated, duplicate, or non-actionable candidates; retain unknowns.

## Complete

- Every hunk/file + applicable lens + blast-radius surface + Spec requirement + gate = reviewed, `N/A`, or unknown.
- Verdict-changing unknown/incomplete coverage → `CONCERNS | FAIL`; never approve.

## Severity

| Level | Meaning |
|---|---|
| Critical | Correctness/security/privacy/data/migration/schema/API/invariant/gate/root-owner/file-size/cross-package blocker |
| Medium | Local maintainability/duplication + missing public-boundary test/docs + brittle special case |
| Low | Cheap local clarity/naming/test/docs fix |
| Info | Optional alternative/education/praise/future cleanup; never required |

## Report

| Section | Content |
|---|---|
| Standards | Findings by severity |
| Spec | Findings by severity or explicit skip |
| Coverage | Hunks/files + lenses + blast radius + requirements + delegation |
| Unknowns | Unverified evidence + impact |
| Gates | Command + result + evidence |
| Verdict | Block / approve with reservations / approve |
| Final line | Count + worst issue per axis |

- Critical → Block; Medium → fix before merge unless explicitly deferred.