Code Review

---
name: code-review
description: Structured code review protocol for inspecting code quality against the full rule set. Use when auditing code written by yourself or another agent, during the /audit workflow, or when the user asks for a code review.
---

# Code Review Skill

## Purpose
Systematically review code against the full antigravity rule set. Catches issues that linters miss: architectural violations, missing observability, business logic errors, pattern inconsistencies.

## When to Invoke
- During the `/audit` workflow (as part of parallel subagent dispatch)
- When user asks for a code review outside any workflow
- **Best practice:** Invoke in a fresh conversation (not the same one that authored the code) to avoid confirmation bias

## Review Process

### 1. Scope the Review
Identify the files/features to review. Determine the review scope:
- **Feature review** — all files in a feature directory
- **PR review** — only changed files
- **Full codebase audit** — all features

### 2. Load the Rule Set
Read all applicable rules from `.agents/rules/`. Use `rule-priority.md` for severity classification.

### 3. Review Categories (Priority Order)

Review each file/feature against these categories, in order from `rule-priority.md`:

#### Critical (Must Fix)
- **Security** — injection, hardcoded secrets, broken auth
- **Data loss** — missing error handling on writes, no transaction boundaries
- **Resource leaks** — unclosed connections, missing cleanup

#### Major (Should Fix)
- **Testability** — I/O not behind interfaces, untested error paths
- **Observability** — missing logging on operations, no correlation IDs
- **Error handling** — empty catch blocks, swallowed errors
- **Architecture** — circular dependencies, wrong layer access

#### Minor (Nice to Fix)
- **Pattern consistency** — deviation from established codebase patterns
- **Naming** — unclear variable/function names
- **Code organization** — functions too long, mixed responsibilities

#### Enhancement (Backlog)
- **Style** — formatting issues or non-critical refactorings
- **Documentation** — missing comments on complex logic

### 4. Produce Findings

Output a structured findings document:

```markdown
# Code Review: {Feature/Module Name}
Date: {date}
Reviewer: AI Agent (fresh context)

## Summary
- **Files reviewed:** N
- **Issues found:** N (X critical, Y major, Z minor, W enhancement)

## Critical Issues
- [ ] **[SEC]** {description} — [{file}:{line}](file:///path)
- [ ] **[DATA]** {description} — [{file}:{line}](file:///path)

## Major Issues
- [ ] **[TEST]** {description} — [{file}:{line}](file:///path)
- [ ] **[OBS]** {description} — [{file}:{line}](file:///path)

## Minor Issues
- [ ] **[PAT]** {description} — [{file}:{line}](file:///path)

## Enhancement Issues
- [ ] {description} — [{file}:{line}](file:///path)

## Rules Applied
List of rules referenced during this review.
```

### 5. Save the Report

When invoked via the `/audit` workflow, you **MUST** persist the findings to the repo:

**Path:** `docs/audits/review-findings-{feature}-{YYYY-MM-DD}-{HHmm}.md`

1. Create `docs/audits/` if it doesn't exist
2. Write the findings document to that path
3. This makes the report accessible from other conversations and agents

When invoked as a standalone review (not via `/audit`), saving to `docs/audits/` is recommended but optional.

### 6. Severity Tags

| Tag      | Category             | Rule Source                                        |
| -------- | -------------------- | -------------------------------------------------- |
| `[SEC]`  | Security             | `security-principles.md`                           |
| `[DATA]` | Data integrity       | `error-handling-principles.md`                     |
| `[RES]`  | Resource leak        | `resources-and-memory-management-principles.md`    |
| `[TEST]` | Testability          | `architectural-pattern.md`, `testing-strategy.md`  |
| `[OBS]`  | Observability        | `logging-and-observability-mandate.md`             |
| `[ERR]`  | Error handling       | `error-handling-principles.md`                     |
| `[ARCH]` | Architecture         | `architectural-pattern.md`, `project-structure.md` |
| `[PAT]`  | Pattern consistency  | `code-organization-principles.md`                  |
| `[INT]`  | Integration contract | `api-design-principles.md`                         |
| `[DB]`   | Database design      | `database-design-principles.md`                    |
| `[CFG]`  | Configuration        | `configuration-management-principles.md`           |
| `[SPEC]` | Spec compliance      | `structured-spec` profiles                         |

### 7. Language-Specific Anti-Patterns

Load the anti-pattern checklist for the language(s) under review:

| Language | Anti-Patterns |
|---|---|
| **Go** | `languages/go.md` |
| **TypeScript** | `languages/typescript.md` |
| **Python** | `languages/python.md` |
| **Rust** | `languages/rust.md` |
| **Java** | `languages/java.md` |
| **C#** | `languages/csharp.md` |
| **Swift** | `languages/swift.md` |
| **Flutter/Dart** | `languages/flutter.md` |
| **C++** | `languages/cpp.md` |
| **Kotlin** | `languages/kotlin.md` |
| **PHP** | `languages/php.md` |
| **Ruby** | `languages/ruby.md` |

> Anti-patterns listed in language files are **auto-fail** — they require no judgment call. If the pattern exists in the code, it is a finding.

### 8. Cross-Boundary Checks

For full audits, cross-boundary concerns (integration contracts, database schema, configuration hygiene, dependency health, test coverage gaps) are checked via the dedicated MECE dimension scope cards in the `/audit` workflow (Dimension F & G) — defined in `.agents/skills/code-audit/references/audit-dimensions.md`.

When invoking this skill standalone (outside `/audit`), apply the applicable dimensions from that checklist manually and tag findings with `[INT]`, `[DB]`, or `[CFG]` as appropriate.

When structured specs exist in the repository (files with structured-spec YAML frontmatter), verify that:
- Contracts declared in specs (`CT-*`) are implemented in code
- Tests declared in specs (`TC-*`) exist and pass
- No requirement (`REQ-*`) is unimplemented without documented rationale
Tag findings with `[SPEC]`.

**Zero-Findings Guard:** If this review produces fewer than 3 findings, you MUST produce a "Dimensions Covered" attestation section in the findings document, listing each cross-boundary dimension and the specific files or queries you examined. Only then may you declare a clean result.

---

## Rule Compliance
This skill enforces all rules in `.agents/rules/`. Key references:
- Rule Priority @rule-priority.md (severity classification)
- Security Principles @security-principles.md
- Architectural Patterns @architectural-pattern.md
- Testing Strategy @testing-strategy.md
- Logging and Observability Mandate @logging-and-observability-mandate.md
- Error Handling Principles @error-handling-principles.md

## Quick Reference Audit Checklist

Consolidated from rule-based checklists. Use as a rapid scan after detailed review.

### Architecture *(from architectural-pattern.md)*
- [ ] I/O behind interfaces; pure business logic; dependencies point inward

### Database *(from database-design-principles.md)*
- [ ] Parameterized queries; migrations reversible; indexes documented

### Dependencies *(from dependency-management-principles.md)*
- [ ] Versions pinned; lock file committed; unused deps removed

### Git *(from git-workflow-principles.md)*
- [ ] Conventional commits; no large binaries; .gitignore complete

### Monitoring *(from monitoring-and-alerting-principles.md)*
- [ ] Health endpoint; key metrics instrumented; alerting rules defined

### Performance *(from performance-optimization-principles.md)*
- [ ] No premature optimization; profiling before tuning; budgets defined