review-spec

<!-- codex: tool-name mapping applied. See .codex/scripts/session-start.sh -->

# Implementation Review

## Conventions

- **Read capy knowledge base conventions** at [shared-capy-knowledge-protocol.md](shared-capy-knowledge-protocol.md).
- **Read profile detection** at [shared-profile-detection.md](shared-profile-detection.md) for the shared detection procedure. When an active profile populates a `review-spec/` phase slot, its `index.md` content is loaded before per-task verification begins.

## Overview

Systematically compare implemented code against a feature's `design.md`, `implementation.md`, and `tasks.md` in `/docs/wip/[feature]/`. Works both mid-implementation (reviewing completed tasks only) and post-implementation (full feature review).

Findings go in **both directions** — code that deviates from spec AND spec that is wrong or outdated given the code.

## Required Outputs

Before declaring the review complete, verify all outputs are delivered:

- [ ] Review report presented to user
- [ ] User-confirmed intentional `SPEC_DEV`/`EXTRA_IMPL` findings indexed as `kk:arch-decisions` (skip if none confirmed)
- [ ] Next steps confirmation from user

Indexing is owned by this skill — callers (e.g., `$kk:implement`) do NOT duplicate it.

## Review Modes

### Standard Mode (`$kk:review-spec`)

Reviews spec conformance in the main conversation context. Single-pass review using the workflow below.

### Isolated Mode (`$kk:review-spec:isolated`)

Delegates detection to an independent `spec-reviewer` sub-agent that did not write the code, then annotates its findings with type-specific author context. Low-relevance types (MISSING_IMPL, DOC_INCON, OUTDATED_DOC, AMBIGUOUS) get brief annotations; high-relevance types (SPEC_DEV, EXTRA_IMPL) get detailed annotations with spec update suggestions.

- **Cost**: Higher (sub-agent + annotation)
- **Isolation**: True — reviewer has zero authorship bias or session context
- **Degradation**: If sub-agent fails, suggests standard mode fallback
- **Best for**: When extra rigor is worth the cost (post-implementation, pre-merge)

See [review-isolated.md](./review-isolated.md) for the isolated workflow.

## Finding Types

Each finding is classified by type (what kind of mismatch) and severity (how urgent).

| Type                   | Code           | Description                                                      | Example                                                                      |
| ---------------------- | -------------- | ---------------------------------------------------------------- | ---------------------------------------------------------------------------- |
| Missing Implementation | `MISSING_IMPL` | Spec describes something that was not implemented                | Design says "rate limiting on /api/auth" but no rate limiter exists          |
| Extra Implementation   | `EXTRA_IMPL`   | Code implements something not in the spec                        | A caching layer was added that design docs don't mention                     |
| Spec Deviation         | `SPEC_DEV`     | Code implements the feature but differently than specified       | Design says "bcrypt cost 12" but code uses cost 10                           |
| Doc Inconsistency      | `DOC_INCON`    | Documentation contradicts itself or is internally inconsistent   | design.md says JWT tokens, implementation.md says session cookies            |
| Outdated Doc           | `OUTDATED_DOC` | Code is correct but docs haven't been updated to reflect reality | Endpoint was renamed during implementation but docs still reference old name |
| Ambiguous Spec         | `AMBIGUOUS`    | Spec is unclear enough that multiple interpretations are valid   | "Support pagination" without specifying cursor vs offset                     |

### IaC Profile Semantics

When profile detection identifies an Infrastructure-as-Code profile (e.g., Kubernetes, Terraform), the declarative artifacts ARE the implementation — there is no separate runtime code to trace. Apply these adjusted type-mappings:

- A design-specified resource whose manifest is absent → `MISSING_IMPL` (absence in declarative systems is a gap, not a pending item or inconsistency)
- A field value in a manifest that disagrees with the design → `SPEC_DEV`
- A manifest resource the design does not mention → `EXTRA_IMPL`

`DOC_INCON` and `OUTDATED_DOC` apply unchanged — their semantics are doc-vs-doc or code-vs-doc, which declarative IaC does not alter.

For each active IaC profile that populates a `review-spec/` slot, load `../../profiles/<name>/review-spec/index.md` for domain-specific verification patterns.

## Severity Levels

Same P0–P3 scale as `$kk:review-code`, adapted for spec conformance:

| Level  | Name     | Description                                                                           | Action                  |
| ------ | -------- | ------------------------------------------------------------------------------------- | ----------------------- |
| **P0** | Critical | Missing core functionality, security spec violated, data model mismatch               | Must fix before merge   |
| **P1** | High     | Significant behavioral deviation from spec, missing error handling that spec requires | Should fix before merge |
| **P2** | Medium   | Minor deviation, doc inconsistency, partial implementation of a spec requirement      | Fix or create follow-up |
| **P3** | Low      | Naming mismatch, doc typo, cosmetic deviation from spec                               | Optional                |

## Confidence Levels

Each finding gets a confidence score (1–10) with **mandatory reasoning** explaining what was checked, what evidence supports the finding, and what uncertainty remains.

| Score | Meaning                                                                                      |
| ----- | -------------------------------------------------------------------------------------------- |
| 9–10  | Certain — direct, unambiguous contradicti