# 0014 — A review is a third kind

- **Status**: Proposed
- **Date**: 2026-11-04
- **Supersedes**: the "a review is an ordinary `kind=task` card" rule in
  `docs/lifecycle.md` (D3 and the rejected "A `kind=review`" option),
  `docs/scheduling.md`, `GLOSSARY.md` ("Review card"), and
  `skills/meerkat/references/review-protocol.md`; it also replaces the
  "`kind=review` is only a marker filter" statement in
  `docs/maintenance/contract.md`.

## Context

A review card is stored as `kind='task'` plus the immutable marker
`review_of_id` / `review_round`; only the wire derives `role='review'`. Every
surface therefore needs two facts — the stored kind and a derived projection —
to answer "is this a review?", and the board, the list filter, the drawer and
external consumers each spell the answer differently. The product wants one
stored discriminant for card type.

## Decision

`kind` is `task | epic | review`. A review card is `kind='review'`.

- **Identity stays the marker.** `review_of_id` and `review_round` still say
  which card is audited and in which delivery round; `kind='review'` only says
  what type of card this is. `kind` never becomes a second identity.
- **One database invariant**: `CHECK ((kind = 'review') = (review_of_id IS NOT NULL))`.
  A row with the marker is a review; a row without it is not. The migration
  backfills `kind='review'` for every existing row whose marker is set.
- **Creation.** `review_of` resolves to the marker and forces `kind='review'`.
  `CREATABLE_KINDS` is `task | epic`: without a marker, only those two may be
  stored; `kind='review'` without `review_of` is `VALIDATION_FAILED`.
- **No flip.** A row may not move between review and non-review. The existing
  marker-immutability trigger is widened to `BEFORE UPDATE OF kind,
  review_of_id, review_round` and raises `REVIEW_OF_IMMUTABLE` when the review
  bit changes.
- **Review safety invariants are unchanged**: `acceptance_mode='off'`, no
  reopen, no parent/child edge, no review-of-review, and no `off` direct
  `in_progress → done` edge. Every existing guard is marker-based; the new
  CHECK keeps kind and marker in lockstep, so they continue to hold.
- **Wire.** `role` remains the externally visible discriminant: a review card
  still reports `role='review'`. New rows have `role = kind`; a legacy row that
  carries only `meta.review_of` keeps the marker fallback.

### Read semantics

The read is literal and clean: `kind='task'` returns only task cards,
`kind='epic'` only epics, and `kind='review'` only review cards. The previous
`kind=review` translation — a filter over `review_of_id IS NOT NULL` — is
removed. `kind=task` no longer returns review cards.

The one exception is a legacy row whose `meta.review_of` names a card that
cannot be resolved in its own project. Such a row keeps `kind='task'` and its
meta mirror (the marker fallback in `foundation/review-identity.mjs` still
lets the behavioural layers read it as a review), but it is not returned by
`kind='review'`; it remains reachable as `kind='task'`. This is recorded, not
silent: the migration resolves every marker it can, and the residue is a
read-only compatibility case.

## Why

One stored discriminant removes the double reading: board columns, list
filters, the drawer's review rounds, external consumers that aggregate by kind,
and the API contract all say the same word. Keeping identity in the marker
prevents a second source of truth: `kind` cannot disagree with "which card is
audited" because the database CHECK ties them together.

## Considered options

1. **Keep the derived `role`** (zero migration, the previous round's
   recommendation). Rejected: every consumer still projects the type, and the
   board/list/drawer keep three spellings of one question.
2. **Add a wire-only `display_kind`.** Rejected: it adds a second vocabulary
   without a storage truth; the API would still expose `kind=task` for a review.
3. **Merge the review back into its original card.** Rejected earlier: it
   loses the independent verdict report and the delivery-round audit trail.
4. **Third kind** (this ADR). Chosen.

## Consequences

- **Schema.** `0004_review_kind.sql` rebuilds `tasks` (SQLite cannot widen a
  CHECK in place), widens the kind CHECK, adds the table-level review CHECK,
  backfills the marker and kind, and recreates **8 indexes** (5 `ix_*` and 3
  `ux_*`) and **31 triggers** (22 on `tasks`, 9 on neighbouring tables). The
  rebuild is the documented create-new / copy / drop-old / rename-new
  procedure with `-- @foreign-keys: off`.
- **Contract.** The API and CLI `kind` vocabulary becomes `task | epic |
  review`; the snapshots are regenerated by `scripts/verify/contract.mjs
  --update` and `scripts/verify/cli-contract.mjs --update`, never hand-edited.
  `kind=task` is now a breaking narrowing for callers that relied on it
  returning review cards.
- **Behaviour.** The review path is still selected by the marker
  (`reviewIdentityOf`); `kind='review'` is added as a second reading so a
  caller holding only a kind cannot miss the review semantics. Reviews remain
  claimable, are never containers, never enter rollup and never auto-accept.
- **Web.** The board and list hide `kind='review'` by default; a filter-bar
  switch shows them. The drawer's Acceptance panel lists the card's review
  rounds. `board.tasks` still loads review cards because the waiting queue,
  roster and drawer read them.
- **Documents replaced.** The "review is an ordinary `kind=task`" rule is
  replaced in these five records: `docs/lifecycle.md` (D3 and the rejected
  "A `kind=review`" option), `docs/scheduling.md` ("There is no `kind=review`"),
  `GLOSSARY.md` "Review card", `skills/meerkat/references/review-protocol.md`,
  and `docs/maintenance/contract.md`. The create/list vocabulary in
  `src/core/README.md`, `skills/meerkat/references/cli.md` and
  `skills/meerkat/references/capabilities.md` moves with them;
  `docs/architecture.md` needed no edit — it records review *identity*, not the
  kind enum.
- **Legacy.** The migration backfills every resolvable `meta.review_of` into
  the marker and flips the kind. An unresolvable reference stays `kind='task'`
  with its meta mirror, as the read-only exception above.

## Migration and rollback

Forward: `0004_review_kind.sql`, applied automatically at startup. Back up the
board database (`.db`, `-wal`, `-shm`) and rehearse on a copy before release.

Rollback is a **future forward migration**, not a shipped file: `0005` would
rebuild `tasks` with the two-value CHECK, set `kind='task' WHERE
kind='review'`, keep the marker columns, and drop the review-kind CHECK. A
`0005` file in the migrations directory would apply immediately and undo this
decision, so it is authored only if a rollback is actually taken.
