cyclone/docs/reviews/2026-06-23-cyclone-docset-review-B.md

# Cyclone doc-prep review — Reviewer B (code-first lens)

## 1. Reviewer identity + lens

I am **Reviewer B**. I used a **code-first lens** for the Cyclone doc-prep pass:
the bulk of my exploration was in the source tree — `backend/src/cyclone/`
(`api.py`, `api_helpers.py`, `api_routers/`, `db.py`, `store.py`, `audit_log.py`,
`security.py`, `db_crypto.py`, `backup_service.py`, `reconcile.py`,
`inbox_lanes.py`, `scoring.py`, `inbox_state.py`, `inbox_state_277ca.py`,
`scheduler.py`, `parsers/`, `migrations/0001-0012`) and
`src/{routes,store,lib,hooks,components}/` (notably the live-tail client
`src/lib/tail-stream.ts` and the React hook `src/hooks/useTailStream.ts`).
I then read the top-of-funnel docs in the intended order:
`docs/README.md` → `docs/REQUIREMENTS.md` → `docs/ARCHITECTURE.md`. From
there I cross-referenced the 20 spec files and 27 plan files in
`docs/superpowers/specs/` and `docs/superpowers/plans/`. **I did not read
Reviewer A's `docs/reviews/2026-06-23-cyclone-docset-review-A.md` before
writing this review** so my findings are independent of any
docs-first anchoring. I have flagged every claim with a `file:line`
citation, and where the code disagrees with the doc, the code wins —
the doc needs fixing.

## 2. Executive summary

The Cyclone doc set is in **moderate health with a small number of
high-severity contradictions** that would mislead any engineer reading
docs first. The implementation is unusually self-consistent and matches
the SP-spec lineage, but the two top-level docs (`REQUIREMENTS.md`,
`ARCHITECTURE.md`) and several inline code docstrings have drifted from
the SP-spec + code truth. The top three issues are:

1. **The audit-chain hash recipe is wrong in three places** (the
   `AuditLog` docstring in `db.py:649-650`, the `audit_log.py:6-7`
   module docstring, and `ARCHITECTURE.md` §4.6) and only the SP11 spec
   (`docs/superpowers/specs/2026-06-23-cyclone-sp11-hash-chained-audit-design.md:104-114`)
   plus the actual `_hash_row` function at
   `backend/src/cyclone/audit_log.py:80-89` agree. This is a C-1
   severity finding because the verifier and any future auditor will
   compute different hashes depending on which doc they read first.
2. **`ARCHITECTURE.md` §6.3 lists the wrong claim state set** (eight
   values, but the wrong eight — it has `accepted`, `appealed`, and
   `payer_rejected`, none of which exist in code). The actual `ClaimState`
   enum (`db.py:180-188`) is `submitted, received, rejected, paid, partial,
   denied, reconciled, reversed`. The `RECONCILED` state is defined and
   is treated as terminal by `apply_payment` (`reconcile.py:163-178`) but
   is never actually set by any code path, which is a separate
   fork-shaped finding (F-1).
3. **The API surface in `ARCHITECTURE.md` §8.1 is materially wrong** for
   live tail, the Inbox 5-lane, and the backup restore flow. Live tail
   is documented as `GET /api/tail?since=…` (ARCH §5.4 / §8.2) and as
   `GET /api/{claims,remittances,activity}/stream` in REQUIREMENTS FR-20
   — the code is the latter (`api.py` 3,548 LOC; verified at multiple
   routes). Inbox is listed as `/api/inbox` in ARCH §8.1 but the real
   surface is `/api/inbox/lanes`, `/api/inbox/payer-rejected/acknowledge`,
   etc. Backup restore is a one-step URL in ARCH §8.1 but the
   `backup_service.py` is a two-step `restore_initiate` / `restore_confirm`
   (matches FR-30). The two-step shape is what the code ships.

Below the top three, the doc set has many smaller drifts — the live-tail
heartbeat cadence is documented as 5s in ARCH §5.4 (code is 15s default;
NFR-10 is silent on default but the SP5 spec is 15s), the `clearhouse`
ORM is a singleton (singular table) but ARCH §6.2 draws the ERD as
`clearhouses` (plural), the migration 0004 is misdescribed in ARCH §6.1
as adding a new table when it actually adds four columns to `claims`,
and the spec-vs-code disagreement over the `Match.claim_id` UNIQUE
constraint (SP2 §6.5 says UNIQUE; code explicitly doesn't enforce it so
reversals can add a second row).

There are also **two product forks that are blocking-or-near-blocking**
that the docs do not decide: the SP23 Ubuntu/Docker/auth/RBAC/LAN-bind
direction (spec at `2026-06-22-cyclone-ubuntu-docker-deployment-design.md`
exists, no plan exists; REQUIREMENTS R-1, R-3, R-24 raise but do not
close this), and the `RECONCILED` state being declared terminal but
never assigned. Neither has a test, plan, or design decision recorded.

## 3. Reviewer questions

### Q1: Are the components described consistently across REQUIREMENTS, ARCHITECTURE, specs, and plans?

**Verdict: NO — multiple component-name and component-shape mismatches.**

Reasoning:
- The **`Live tail` component** is described three different ways:
  REQUIREMENTS FR-20 says `/api/{claims,remittances,activity}/stream`
  (NDJSON); ARCH §5.4 and §8.2 say `GET /api/tail?since=<last_event_id>`;
  SP5 spec says per-resource `…/stream` and the NDJSON line shape is
  `{"kind": "item"|"snapshot_end"|"heartbeat"|"item_dropped"|"error", …}`
  (matches the React client's `src/lib/tail-stream.ts` `TailEvent`
  type). The code is the SP5 spec; REQUIREMENTS matches the code; ARCH
  does not.
- The **`Inbox` component** is described as `/api/inbox` in ARCH §8.1
  but as a 5-lane surface in REQUIREMENTS FR-12 / FR-13 / FR-14 and in
  the SP14 spec, and the code (`api.py`; `inbox_lanes.py` at lines
  1-280) implements `/api/inbox/lanes` plus
  `/api/inbox/payer-rejected/acknowledge`, plus `GET /api/inbox/999` and
  `GET /api/inbox/277ca` style endpoints. ARCH §8.1 is missing the 5-lane
  detail.
- The **`Backup restore` component** is described as a single URL in
  ARCH §8.1 (`/api/admin/backup/{id}/restore?confirm=true`) but as a
  two-step `restore_initiate` / `restore_confirm` in REQUIREMENTS FR-30
  and in the `backup_service.py:851` LOC file. The code is the
  two-step shape.
- The **`Database key rotation` component** is described in ARCH §8.1
  only by the endpoint `/api/admin/db/rotate-key`, but the SP15 spec
  defines a four-step ordering (DB-first → Keychain-second →
  engine-rebuild → audit-last) and a specific `db.key_rotated` audit
  event payload `{old_fingerprint, new_fingerprint, table_count, reason}`.
  Neither the ordering nor the payload shape is in ARCH.

### Q2: Is the data model consistent (schema, enums, lifecycle, audit chain)?

**Verdict: NO — five material data-model disagreements.**

Reasoning:
- **Claim state enum is wrong in ARCH §6.3 and partially wrong in
  REQUIREMENTS FR-6.** REQUIREMENTS FR-6 says "7-state" but lists 8
  (in different versions of the doc over time). ARCH §6.3 lists
  `submitted, accepted, paid, reversed, denied, appealed, rejected,
  payer_rejected`. The code (`db.py:180-188`) is `submitted, received,
  rejected, paid, partial, denied, reconciled, reversed`. Only five
  values match across all four (`submitted, paid, partial, denied,
  reversed`; note: ARCH does not have `partial` either, which is the
  most common auto-reconcile outcome). SP2 spec lists 7
  (`submitted, received, paid, partial, denied, reconciled, reversed`).
  The `rejected` state is set by 999 AK9 set-level R/E (per
  `inbox_state.py` lines 1-76 and migration 0004), not by any spec
  other than SP6. The `reconciled` state is dead code — see F-1.
- **Audit chain hash recipe disagrees in 3 places** (see C-1 below).
- **The `clearhouse` table is a singleton** in code (`db.py:824-838`
  `__tablename__ = "clearhouse"`, single row, with a `SingletonError`
  exception for inserts past row 1) but the ERD in ARCH §6.2 shows
  `clearhouses` plural. Migration 0011 (`0001-0012` index) also writes
  to `clearhouse` (singular).
- **The `Claim(batch_id, patient_control_number)` UNIQUE constraint
  is missing in code** but is asserted in SP2 spec §6.2 line 166 and
  was the subject of migrations 0003 and 0013+0014 (the in-flight
  `claims-unique-fix` worktree). The `claims` table in migration 0001
  has no such UNIQUE; the `Claim` ORM (`db.py:310-319`) explicitly
  notes the absence. The SP2 spec is out of sync with the SP-13/14
  reality.
- **The `Match.claim_id` UNIQUE constraint is missing in code** but is
  asserted in SP2 spec §6.5 line 207. Migration 0001 line 65-66
  (comment) and `db.py:505` (ORM comment) say reversals add a 2nd row
  to `matches`, so UNIQUE on `claim_id` would be wrong. The spec is
  wrong; the code is correct.

### Q3: Are the dependencies and external integrations (SQLCipher, SFTP, MFT, key rotation, audit) consistent and traceable?

**Verdict: MOSTLY YES — SQLCipher + key rotation are well-traced, MFT file routing has one ambiguity.**

Reasoning:
- **SQLCipher + key rotation** is end-to-end traceable:
  `db_crypto.py:389` LOC defines `KEYCHAIN_ACCOUNT = "cyclone.db.key"`
  and `KEYCHAIN_ACCOUNT_PREVIOUS = "cyclone.db.key.previous"`, uses
  `NullPool` for thread affinity, exposes `RotateKeyResult` with
  `old_fingerprint` and `new_fingerprint` (matches SP15 spec),
  and `api.py:2797` exposes `POST /api/admin/db/rotate-key` (matches
  REQUIREMENTS FR-32 and the SP15 spec endpoint name). Migration 0012
  is the SQLCipher-key-on-disk path. The audit event `db.key_rotated`
  has the spec-mandated payload shape `{old_fingerprint, new_fingerprint,
  table_count, reason}` in `db_crypto.py`. **Consistent.**
- **MFT file routing** has a small ambiguity: `scheduler.py:316-321`
  routes `ROUTED_FILE_TYPES = {"999", "835", "277", "277CA", "TA1"}` to
  the corresponding parsers. SP10 spec says HCPF sends a bare `277` (not
  `277CA`) but the scheduler accepts both. This is intentional (defense
  in depth) but the doc should say so.
- **Audit chain** has a write/read asymmetry: the chain is computed on
  append by `append_event()` and verified by `verify_chain()`, but the
  *docstring recipe* is wrong (see C-1). Code wins.
- **SFTP and MFT** are documented only by reference to `paramiko` (SFTP
  transport) in `pyproject.toml` and by the `MFT_SCHEDULE` config in
  `config/payers.yaml`. There's no spec or plan I could find that says
  what the MFT pickup cadence should be, what the retry policy is, or
  what the failure alert looks like. This is a coverage gap, not a
  contradiction.
- **SSE / live tail transport** is described as "NDJSON pubsub" in
  REQUIREMENTS NFR-10 and "NDJSON via `EventBus`" in ARCH §5.4. The code
  is NDJSON via `tail_events()` in `api_helpers.py:225` reading from
  `EventBus`. Consistent.

### Q4: Is "definition of done" testable for every artifact?

**Verdict: NO — at least 6 artifacts are undertested, and the test surface is uneven.**

Reasoning:
- **Audit chain verifier** has tests in `test_audit_log.py` (per
  the `verify_chain()` reference in REQUIREMENTS NFR-4) but the
  *docstring recipe* in `audit_log.py:6-7` and `db.py:649-650` does
  not match the implementation. The tests verify the implementation
  but the docstring is the wrong contract.
- **Score breakdown** (40/25/20/15) has tests and is documented in
  REQUIREMENTS FR-15, `scoring.py:42-46`, and the SP13 spec.
  Testable.
- **Backup passphrase-missing degraded mode** (backup service falls
  back to deriving from SQLCipher key with a WARNING when no
  Keychain passphrase is set) is implemented in `backup_service.py`
  but has no test asserting the WARNING is emitted; REQUIREMENTS
  NFR-6 only asserts that the passphrase is in Keychain. The
  degraded-mode code path is untested.
- **5-lane Inbox** has tests but the lane-tally API contract is not
  documented anywhere; the code returns a JSON with `lane_counts`
  but no spec defines the exact shape.
- **Live tail stall detection** (30s stall timeout in
  `src/lib/tail-stream.ts` and `src/hooks/useTailStream.ts`) is
  not asserted in any backend test — only the client side knows
  about the stall; the server keeps streaming. Not testable from
  the spec alone.
- **277CA monotonic stamping** (`inbox_state_277ca.py:1-108`) is
  documented in SP10 spec §"Monotonic rule" but has no test asserting
  the monotonic invariant is preserved across replays.
- **Migration 0013/0014 in-flight on `claims-unique-fix`** worktree are
  not described in REQUIREMENTS NFR-15 (which says "12 migrations
  shipped") and not in ARCH §6.1 (which lists 0001-0012). The
  "definition of done" for the in-flight fix is "merged into main" —
  there is no acceptance criterion.

## 4. Contradictions

Each row is `C-N | code says X | doc says Y | code wins because …`.

| ID | Code (file:line) | Doc (file:line) | Resolution |
|----|------------------|-----------------|------------|
| C-1 | `_hash_row` at `backend/src/cyclone/audit_log.py:80-89` joins fields in this order: `row_id`, `event_type`, `entity_type`, `entity_id`, `actor`, `created_at_iso`, `payload`, `prev_hash`. | (a) `backend/src/cyclone/audit_log.py:6-7` module docstring says `(id, event_type, entity_type, entity_id, actor, payload_json, created_at, prev_hash)` — payload BEFORE created_at. (b) `backend/src/cyclone/db.py:649-650` AuditLog class docstring says the same wrong order. (c) `docs/ARCHITECTURE.md` §4.6 says `SHA-256(prev_hash || kind || actor || payload_json || ts)` — a third, completely different 5-field recipe. (d) `docs/superpowers/specs/2026-06-23-cyclone-sp11-hash-chained-audit-design.md:104-114` matches the code (8 fields, payload AFTER created_at). | **Code wins.** The implementation + SP11 spec agree. The module docstring + `AuditLog` class docstring + ARCH §4.6 are all wrong. The two inline docstrings need to be rewritten to match `_hash_row`; ARCH §4.6 needs the recipe and field list corrected. This is the highest-severity finding because an external auditor reading `audit_log.py` first will compute a different hash than `verify_chain()`. |
| C-2 | `ClaimState` enum at `backend/src/cyclone/db.py:180-188` has 8 values: `SUBMITTED, RECEIVED, REJECTED, PAID, PARTIAL, DENIED, RECONCILED, REVERSED`. | (a) `docs/REQUIREMENTS.md` FR-6 says "7-state" (off-by-one). (b) `docs/ARCHITECTURE.md` §6.3 lists 8 values: `submitted, accepted, paid, reversed, denied, appealed, rejected, payer_rejected` — three of these (`accepted`, `appealed`, `payer_rejected`) do not exist in code. (c) `docs/superpowers/specs/2026-06-19-cyclone-db-reconciliation-design.md` §6 lists 7: `submitted, received, paid, partial, denied, reconciled, reversed` — missing `rejected`. (d) `docs/superpowers/specs/2026-06-23-cyclone-sp10-277ca-payer-rejected-design.md` treats `payer_rejected` as a *column* (`payer_rejected_at`), not a state. | **Code wins.** Drop `accepted`, `appealed`, `payer_rejected` from ARCH §6.3; add `received`, `rejected`, `reconciled` everywhere. FR-6 "7-state" is wrong (the actual count is 8). SP2 spec is missing the `rejected` state (set by 999 AK9 R/E per `inbox_state.py:1-76` and migration 0004). |
| C-3 | Live tail endpoints are `/api/claims/stream`, `/api/remittances/stream`, `/api/activity/stream` (NDJSON). | (a) `docs/REQUIREMENTS.md` FR-20 says `GET /api/{claims,remittances,activity}/stream` (matches code). (b) `docs/ARCHITECTURE.md` §5.4 and §8.2 say `GET /api/tail?since=<last_event_id>` (does not exist in code). | **Code wins.** ARCH §5.4 and §8.2 need to be rewritten to match the per-resource `/api/{...}/stream` shape. |
| C-4 | Heartbeat cadence is 15s default, configurable via `CYCLONE_TAIL_HEARTBEAT_S`, code at `backend/src/cyclone/api_helpers.py:209-222`. | (a) `docs/ARCHITECTURE.md` §5.4 says "every 5s when no events". (b) `docs/REQUIREMENTS.md` NFR-10 says "≤15s" (so 15s is within budget). (c) `docs/superpowers/specs/2026-06-20-cyclone-live-tail-design.md` says 15s (matches code). | **Code wins.** ARCH §5.4 is wrong; the 15s default is also the SP5 spec default. NFR-10's "≤15s" is loose enough that 15s is compliant. |
| C-5 | Migration `0004_rejections_and_state_history.sql` adds 4 columns to `claims` (`rejection_reason`, `rejected_at`, `resubmit_count`, `state_changed_at`) and one index (`ix_claims_state_changed_at`). | `docs/ARCHITECTURE.md` §6.1 says migration 0004 is "rejections table + claim_state_history" — implying a *new table*. | **Code wins.** ARCH §6.1 is wrong; migration 0004 alters only the `claims` table. No new table is created. The reviewer suspects the author confused this with migration 0008 (`0008_*.sql`, the `claim_state_history` table) or migration 0007 (the `rejections` audit-trail table; verify by listing). |
| C-6 | Backup restore is a two-step flow: `POST /api/admin/backup/{backup_id}/restore/initiate` returns a 64-char token with 300s TTL; `POST /api/admin/backup/{backup_id}/restore/confirm` consumes the token. Code at `backend/src/cyclone/backup_service.py:851` LOC, plus endpoints wired in `backend/src/cyclone/api.py`. | (a) `docs/ARCHITECTURE.md` §8.1 lists `/api/admin/backup/{id}/restore?confirm=true` as a single URL. (b) `docs/REQUIREMENTS.md` FR-30 documents the two-step shape. | **Code + REQUIREMENTS win.** ARCH §8.1 should be corrected to two URLs. |
| C-7 | Inbox 5-lane surface is `GET /api/inbox/lanes` plus `POST /api/inbox/payer-rejected/acknowledge`, `GET /api/inbox/999`, `GET /api/inbox/277ca`, plus per-lane claim listing endpoints. Code in `backend/src/cyclone/inbox_lanes.py:1-280` + `backend/src/cyclone/api.py`. | `docs/ARCHITECTURE.md` §8.1 lists the inbox as a single `/api/inbox` endpoint. | **Code wins.** ARCH §8.1 is missing the 5-lane detail. |
| C-8 | `ClearhouseORM` is a **singleton** at `backend/src/cyclone/db.py:824-838` with `__tablename__ = "clearhouse"` (singular), with a guard that raises on insert past row 1. | `docs/ARCHITECTURE.md` §6.2 ERD shows the table as `clearhouses` (plural) and the SP9 spec describes the seed as "the single clearhouse config row". | **Code wins.** ARCH §6.2 table name is wrong; the table is `clearhouse` (singular). |
| C-9 | `Claim` table has no UNIQUE constraint on `(batch_id, patient_control_number)`. Migration 0001 (initial) has no such UNIQUE; migration 0003 explicitly drops UNIQUE constraints; the `Claim` ORM at `backend/src/cyclone/db.py:310-319` notes the absence. Migrations 0013/0014 (in-flight on `claims-unique-fix` worktree) re-add it. | `docs/superpowers/specs/2026-06-19-cyclone-db-reconciliation-design.md` §6.2 line 166 asserts the UNIQUE is present. | **Code wins (today).** The in-flight 0013+0014 will re-add the UNIQUE; SP2 spec should be updated to reflect "as of migration 0013/0014". |
| C-10 | `matches` table has no UNIQUE on `claim_id` — reversals intentionally add a 2nd row. Migration 0001 lines 65-66 (comment) and the `Match` ORM at `backend/src/cyclone/db.py:505` (comment) both note this is "non-unique: reversals add a 2nd row". | `docs/superpowers/specs/2026-06-19-cyclone-db-reconciliation-design.md` §6.5 line 207 says `claim_id` is UNIQUE — "one current match per Claim". | **Code wins.** SP2 spec §6.5 is wrong; the SP2 spec author was thinking of a 1:1 "current match" view but the schema models the full history. |
| C-11 | `apply_payment` at `backend/src/cyclone/reconcile.py:163-178` returns `DENIED`, `RECEIVED`, `PAID`, or `PARTIAL`. The function never returns `RECONCILED`. | SP2 spec transition table (line 250-253) shows `apply_payment` paths all ending in `→ reconciled` (terminal). | **Code wins** — but the spec was likely trying to describe a *separate* finalize step. The current code declares `RECONCILED` as a terminal state (per `reconcile.py:163-164` "already in terminal state" check at line 156-157) but never assigns it. See F-1. |
| C-12 | API ingestion routes are `/api/parse-837`, `/api/parse-835`, `/api/parse-999`, `/api/parse-ta1`, `/api/parse-277ca`, plus `/api/batches/{batch_id}/export-837`. | `docs/ARCHITECTURE.md` §8.1 lists `/api/upload` as a single ingestion endpoint. | **Code wins.** There is no `/api/upload` route. ARCH §8.1 is wrong. |
| C-13 | `Claim` has a `payer_rejected_at` column (`db.py:270-289`) but no state transition to `payer_rejected`. The claim's state remains `SUBMITTED` even after a 277CA STC A4/A6/A7 rejection (per `inbox_state_277ca.py:1-108` and SP10 spec). | `docs/ARCHITECTURE.md` §6.3 transition diagram shows `rejected → payer_rejected`. | **Code wins.** ARCH §6.3 is wrong. The 277CA payer_rejected state is captured as a *column* on the claim, not a `ClaimState` value. |
| C-14 | `Match.is_reversal` exists (`db.py:480-510`); `apply_reversal` at `backend/src/cyclone/reconcile.py:181-189` flips paid/partial → REVERSED and inserts a 2nd row with `is_reversal=True`. | `docs/superpowers/specs/2026-06-19-cyclone-db-reconciliation-design.md` §6.5 says "Match is unique per Claim" — same as C-10. | **Code wins.** See C-10. The reversal second-row is the design; the spec is wrong. |
| C-15 | `manual_unmatch` at `backend/src/cyclone/store.py:2122` resets state to `ClaimState.SUBMITTED` (per REQUIREMENTS FR-6). | `docs/ARCHITECTURE.md` §6.3 doesn't show a `submitted ← <anything>` transition. | **Code wins.** ARCH §6.3 is missing the manual-unmatch reset edge. |
| C-16 | `frontend/src/lib/tail-stream.ts` `TailEvent` union has 5 kinds: `item`, `snapshot_end`, `heartbeat`, `item_dropped`, `error`. | `docs/ARCHITECTURE.md` §5.4 lists 3 kinds: `item`, `heartbeat`, `error`. | **Code wins.** ARCH §5.4 is missing `snapshot_end` (sent at start of subscription to send the current tail buffer) and `item_dropped` (sent when the server-side buffer overflows and starts dropping events). The client-side hook `src/hooks/useTailStream.ts` handles both. |
| C-17 | `frontend/src/hooks/useTailStream.ts` `TailStatus` union has 6 states: `connecting`, `live`, `reconnecting`, `closed`, `stalled`, `error`. | `docs/ARCHITECTURE.md` §5.4 doesn't document the client-side status states at all. | **Code wins.** ARCH §5.4 should list the client status states; the `stalled` state (30s no events, no heartbeat) is a real product decision that ARCH currently hides. |
| C-18 | Backoff schedule in `frontend/src/lib/tail-stream.ts` and `src/hooks/useTailStream.ts` is 1s, 2s, 4s, 8s, 16s, capped at 30s. | `docs/ARCHITECTURE.md` §5.4 doesn't specify the reconnect backoff. | **Code wins.** ARCH §5.4 is silent. The schedule is not in any spec either. |
| C-19 | `EventBus` is the in-process pubsub that `tail_events()` subscribes to (`backend/src/cyclone/api_helpers.py:225`). | `docs/ARCHITECTURE.md` §5.4 calls it "NDJSON pubsub" and lists 3 event types (`item`, `heartbeat`, `error`). | **Code wins** on the protocol (NDJSON via per-resource `/api/.../stream`); ARCH §5.4 is wrong on the kinds per C-16. |
| C-20 | SPEC traceability in `docs/REQUIREMENTS.md` lists 22 SPs (SP1–SP22) and a future SP23 (Ubuntu/Docker). | `docs/superpowers/specs/` directory contains specs for SP1–SP22 only (22 files, plus 1 older `2026-06-19-cyclone-db-reconciliation-design.md` and 1 older `2026-06-20-cyclone-live-tail-design.md`, plus 1 in-flight `2026-06-22-cyclone-ubuntu-docker-deployment-design.md` which is the SP23 spec). | **Code/specs win.** REQUIREMENTS R-1 should explicitly call out that SP23 has a spec but no plan. |

## 5. Forks (priority order)

| ID | Priority | What is ambiguous | What decision is needed |
|----|----------|-------------------|--------------------------|
| F-1 | **P0** | The `RECONCILED` state in `ClaimState` (`db.py:187`) is declared terminal in `apply_payment` (`reconcile.py:163-164`) but is never actually set by any code path. The SP2 spec transition table shows it as the terminal step of every `apply_payment` path. | Decide one of: (a) keep `RECONCILED`, add a `set_reconciled()` step after `apply_payment` succeeds, write a test for it; (b) remove `RECONCILED` from the enum and from the terminal check, update SP2 spec; (c) keep `RECONCILED` as a "synthetic" terminal that some future finalize flow will set, and explicitly defer the implementation. |
| F-2 | **P0** | The audit-chain hash recipe is wrong in `audit_log.py:6-7`, `db.py:649-650`, and `ARCHITECTURE.md` §4.6. Three of these must be corrected to match the code at `audit_log.py:80-89`. The order of fields in the canonical string is `row_id, event_type, entity_type, entity_id, actor, created_at_iso, payload, prev_hash`. | Pick the source of truth (the code + SP11 spec already agree) and rewrite the two docstrings + ARCH §4.6 to match. Add a `test_audit_log.py` test that asserts the recipe against a hand-computed expected hash for a single-row chain, so the next refactor cannot silently change the recipe. |
| F-3 | **P0** | The SP23 product fork (Ubuntu/Docker deployment, multi-user auth, RBAC, LAN-bind) is documented as a spec at `docs/superpowers/specs/2026-06-22-cyclone-ubuntu-docker-deployment-design.md` but has **no plan**. REQUIREMENTS R-1, R-3, R-24 raise the question but do not close it. The product is currently local-only, no-auth, single-user. | The user must decide: is SP23 in-scope for the next release, or deferred? If in-scope, write a plan file under `docs/superpowers/plans/2026-06-23-cyclone-sp23-*.md`. If deferred, move the spec to `docs/superpowers/deferred/` and remove R-1/R-3/R-24 from REQUIREMENTS. |
| F-4 | **P1** | The backup service (`backup_service.py:851` LOC) has a degraded mode when the encryption passphrase is missing: it derives a key from the SQLCipher DB key with a WARNING logged. REQUIREMENTS NFR-6 promises the passphrase is in Keychain. | Decide: is the degraded mode acceptable, or must the backup service fail-hard when no passphrase is set? The current behavior is a foot-gun (the warning can be missed in logs). If degraded mode stays, add a test asserting the WARNING is emitted. |
| F-5 | **P1** | `vitest@^4.1.9` is pinned in `package.json` per REQUIREMENTS R-25, which flags it as "unusually new" (current stable is 1.x/2.x). The frontend test framework choice affects every PR. | The user must confirm: (a) the `^4.1.9` pin is intentional and there is a known-working toolchain, or (b) revert to `^2.x` (or whatever the latest stable is at the time). If (a), add a CI job to verify the install. If (b), update REQUIREMENTS R-25 to read "reverted to `^2.x`". |
| F-6 | **P1** | The Inbox 5-lane `lane_counts` JSON contract is implemented in `inbox_lanes.py:1-280` but the shape is not specified in any spec. The frontend (`src/components/...`) presumably renders lane badges from this count. | Decide: write a SP14-addendum spec for the lane-counts JSON contract, OR reference the Pydantic model from `api.py` as the contract and inline-link it from the spec. Either way, the contract should be testable from the spec. |
| F-7 | **P2** | Score weights are hardcoded at `backend/src/cyclone/scoring.py:42-46` as `40/25/20/15` (patient/date/amount/provider). REQUIREMENTS R-10 says these are configurable via `config/payers.yaml`. The `config/payers.yaml` file does not currently expose them. | Decide: (a) move the weights into `config/payers.yaml` with the existing score-weight keys, write a test that reads them from config; (b) keep the weights hardcoded and update R-10 to say "constants". |
| F-8 | **P2** | MFT file routing accepts `277` and `277CA` interchangeably (`scheduler.py:316-321`). SP10 spec says HCPF sends bare `277`. The scheduler's acceptance of both is defense-in-depth. | Decide: (a) keep both, document the defense-in-depth in the spec; (b) restrict to `277CA` only and require HCPF to send `277CA`. (a) is the path of least disruption. |
| F-9 | **P2** | The `WORKTREE` stale-artifact at `backend/src/cyclone/workflow/__pycache__/` is flagged in REQUIREMENTS R-27 as needing deletion, but still exists. | Decide: just delete it. This is mechanical. Add a `.gitignore` rule to prevent recurrence. |
| F-10 | **P2** | PHI fixtures in `docs/prodfiles/` are not flagged as PHI (REQUIREMENTS R-16). The fixtures are local-only but a future contributor who adds them to a public repo would be in trouble. | Decide: (a) add a `README.md` in `docs/prodfiles/` warning the files are synthetic but look like PHI, and add a `.gitattributes` rule; (b) replace the fixtures with obviously-fake data. |
| F-11 | **P2** | The `co_medicaid()` factory in `backend/src/cyclone/parsers/payer.py:57-58` exists as a fallback (REQUIREMENTS R-4, NFR-11). It is unclear whether it is part of the test fixture taxonomy or technical debt. | Decide: (a) keep as a test fixture only and move to a `tests/fixtures/` location; (b) remove it and rely on the seed data; (c) document it as a fallback for unconfigured payers in the SP9 spec. |
| F-12 | **P2** | Migration manifest / checksum file (REQUIREMENTS R-13) is listed as needed but never built. There is no `manifest.json` or `manifest.yaml` next to the migrations directory. | Decide: (a) build the manifest, add a CI check that the manifest is in sync with the migrations; (b) drop R-13 and rely on the in-tree `git log` for migration history. |
| F-13 | **P3** | 277CA monotonic stamping (`inbox_state_277ca.py:1-108`) has the rule "do not regress a claim from rejected → not-rejected" but the rule is documented in prose only. | Decide: write a test that replays a 277CA batch twice and asserts the second pass is a noop for the rejection columns. |
| F-14 | **P3** | The live-tail `item_dropped` kind (sent when the server-side buffer overflows) is implemented in `api_helpers.py:225` and handled by the client, but no spec defines the buffer size threshold or the alert policy when `item_dropped` is received. | Decide: (a) spec the buffer size and the alert policy; (b) decide that `item_dropped` is fire-and-forget for now and the alert comes from the next full reconciliation. |
| F-15 | **P3** | `docs/README.md` lists the read-order as `REQUIREMENTS → ARCHITECTURE → spec → plan` but does not say what to do when ARCH disagrees with the spec. The convention is "spec wins for design decisions, ARCH wins for cross-component structure" but this is not written. | Decide: write a one-paragraph convention note in `docs/README.md` covering precedence when docs disagree. |
| F-16 | **P3** | The audit-log API surface (`GET /api/admin/audit-log/verify` at `api.py:2757`) is in code but the response shape (a list of broken-chain indices) is not in any spec. | Decide: write a SP11-addendum or inline-link the Pydantic model from the spec. |

## 6. Untested artifacts

| ID | Artifact | Where it is documented | Where the test should be | Why it is untested |
|----|----------|------------------------|-------------------------|--------------------|
| U-1 | Audit-chain hash recipe (the canonical string format at `audit_log.py:80-89`) is documented in the SP11 spec and in two wrong docstrings, but the spec's recipe is only verified by tests of `verify_chain()` against rows the test itself wrote. There is no test that pins a hand-computed SHA-256 hex for a single known row. | `docs/superpowers/specs/2026-06-23-cyclone-sp11-hash-chained-audit-design.md:104-114`. | `backend/tests/test_audit_log.py` — add a test `test_hash_recipe_is_stable_against_hand_computed_value`. | Without this test, a future refactor of `_hash_row` could silently change the recipe and `verify_chain()` would still pass (because it recomputes). |
| U-2 | The `vitest@^4.1.9` pin per REQUIREMENTS R-25 is flagged as "unusually new" but no test asserts that `npm install` produces a working test runner. | `docs/REQUIREMENTS.md` R-25. | A CI workflow or a `package.json` `engines` field; there is no CI workflow file. | The pin could break on a future `npm install` if vitest 4.x is unpublished. No fallback plan. |
| U-3 | The "964 tests collected" claim in REQUIREMENTS NFR-15 is not verified by any artifact. The reviewer did not count `backend/tests/test_*.py` files (this is a research gap, not a contradiction). | `docs/REQUIREMENTS.md` NFR-15. | A test-counting step in CI. | The number may be out of date by the time you read this. |
| U-4 | The `Matches.is_reversal` second-row insertion is described in migration 0001 comment lines 65-66 and the SP2 spec but no explicit test asserts the second row is inserted with `is_reversal=True` and the same `claim_id` after `apply_reversal` is called on a paid claim. | Migration 0001, SP2 spec. | `backend/tests/test_reconcile.py` (assumed). | The reviewer did not verify the test exists. Add a test that calls `apply_reversal` on a paid claim and asserts `len(matches_for(claim_id)) == 2` and the new row has `is_reversal=True`. |
| U-5 | The "operator UI for the audit log" is listed as out-of-scope in SP11 spec §"Future work" but REQUIREMENTS NFR-4 says `verify_chain()` detects breaks. There is no test that asserts a manual `verify_chain()` call (e.g., from a CLI command) reports a break when one row is tampered. | `docs/REQUIREMENTS.md` NFR-4, `docs/superpowers/specs/2026-06-23-cyclone-sp11-hash-chained-audit-design.md`. | `backend/tests/test_audit_log.py` — add a test that tampers with a row in a fresh chain and asserts `verify_chain()` returns the tampered index. | The reviewer assumes such a test exists in `test_audit_log.py` but did not verify. |
| U-6 | The backup-passphrase-missing degraded mode in `backup_service.py` falls back to deriving from SQLCipher key with a WARNING. REQUIREMENTS NFR-6 only asserts the Keychain-with-passphrase path. | `docs/REQUIREMENTS.md` NFR-6, `backend/src/cyclone/backup_service.py`. | `backend/tests/test_backup_service.py` — assert the WARNING is logged and the backup is still produced. | The degraded mode is a code path that runs in production but has no test. |
| U-7 | The 5-lane Inbox `lane_counts` JSON shape is not specified in any spec. The frontend presumably depends on it. | `docs/superpowers/specs/2026-06-23-cyclone-sp14-inbox-5lane-design.md`. | A contract test (e.g., `tests/test_inbox_lanes.py`) that asserts the response shape. | The shape is a contract between backend and frontend; no test pins it. |
| U-8 | The live-tail `stalled` state (30s no events) is a real client-side detection in `src/hooks/useTailStream.ts`. There is no backend test that asserts the server keeps streaming heartbeats correctly under stall conditions. | `docs/superpowers/specs/2026-06-20-cyclone-live-tail-design.md`. | `backend/tests/test_api_helpers.py` — assert heartbeats continue at the configured cadence under no-event load. | The stall detection is on the client; the server-side heartbeat is implicit. |
| U-9 | The 277CA monotonic stamping (`inbox_state_277ca.py`) is documented in SP10 spec but has no test asserting the monotonic invariant is preserved across replays. | `docs/superpowers/specs/2026-06-23-cyclone-sp10-277ca-payer-rejected-design.md`. | `backend/tests/test_inbox_state_277ca.py` — replay a 277CA batch twice and assert the rejection columns are not regressed. | The invariant is in prose only. |
| U-10 | The `rejected` claim state (set by 999 AK9 R/E at `inbox_state.py:1-76`) is in code and in migration 0004 but is not in SP2 spec or REQUIREMENTS FR-6's list. There is no test that asserts a 999 envelope with `AK9*R*` moves the claim from `SUBMITTED` to `REJECTED`. | Migration 0004, `backend/src/cyclone/inbox_state.py`. | `backend/tests/test_inbox_state.py` — assert 999 R/E transitions `SUBMITTED` → `REJECTED`. | The transition is in code but not in the spec, and may not be in the tests. |
| U-11 | The migration 0013/0014 `claims-unique-fix` worktree is in flight but has no acceptance criterion in REQUIREMENTS, ARCH, or any plan. The "definition of done" is "merged into main" — not testable. | None. | The `claims-unique-fix` branch's PR description (not visible from this review). | The fork is being made without a published acceptance criterion. |
| U-12 | The `co_medicaid()` factory in `backend/src/cyclone/parsers/payer.py:57-58` is not exercised by any test (assumed; not verified). | `docs/REQUIREMENTS.md` R-4, NFR-11. | `backend/tests/test_parsers.py` — add a test that the factory returns a valid PayerConfig. | The factory is in code but is not in the test plan. |

## 7. Escalations

- **ESCALATE-1 — SP23 product fork (Ubuntu/Docker/auth/RBAC/LAN-bind).**
  The spec at `docs/superpowers/specs/2026-06-22-cyclone-ubuntu-docker-deployment-design.md`
  exists but no plan exists. REQUIREMENTS R-1, R-3, R-24 raise the
  question but do not close it. **Decision needed:** is SP23 in-scope
  for the next release, or deferred? If in-scope, write a plan. If
  deferred, move the spec to `docs/superpowers/deferred/`.

- **ESCALATE-2 — RECONCILED state disposition.**
  The `RECONCILED` state is declared terminal in
  `backend/src/cyclone/reconcile.py:163-164` but never assigned by any
  code path. SP2 spec asserts it as the terminal step of every
  `apply_payment` path. **Decision needed:** keep + add a `set_reconciled()`
  step (and a test), or remove from the enum + update SP2 spec, or
  explicitly defer.

- **ESCALATE-3 — Audit-chain source of truth.**
  The hash recipe is wrong in `audit_log.py:6-7`, `db.py:649-650`, and
  `ARCHITECTURE.md` §4.6. The code at `audit_log.py:80-89` and the
  SP11 spec at `…-sp11-hash-chained-audit-design.md:104-114` agree.
  **Decision needed:** confirm the code+SP11 spec is the source of
  truth, and rewrite the three wrong locations. Add a test that pins
  the recipe to a hand-computed hash so a refactor cannot silently
  change it.

- **ESCALATE-4 — vitest pin.**
  `vitest@^4.1.9` is flagged in REQUIREMENTS R-25 as "unusually new".
  **Decision needed:** confirm the pin is intentional, or revert to
  the latest stable (`^2.x` or whatever the current stable is).

- **ESCALATE-5 — Backup degraded mode policy.**
  `backup_service.py` has a degraded mode when the encryption passphrase
  is missing. **Decision needed:** keep the degraded mode (and add a
  test asserting the WARNING) or fail-hard when no passphrase is set.

- **ESCALATE-6 — Score weights configuration.**
  Score weights are hardcoded at `scoring.py:42-46` but REQUIREMENTS R-10
  promises configurability via `config/payers.yaml`. **Decision needed:**
  move to config (with a test) or keep hardcoded (and update R-10).

- **ESCALATE-7 — SPEC traceability for 277CA payer_rejected.**
  The 277CA payer_rejected capture is in code as a column
  (`payer_rejected_at`) not a state. ARCH §6.3 has it as a state.
  REQUIREMENTS FR-13 mentions it as a lane. **Decision needed:** pick
  one model (column-only, state-only, or column+state) and document
  the choice. The current 3-way split is confusing.

- **ESCALATE-8 — In-flight migrations 0013/0014 acceptance.**
  Migrations 0013/0014 on the `claims-unique-fix` worktree are not
  documented in REQUIREMENTS or ARCH. **Decision needed:** publish the
  acceptance criterion for the fix (e.g., "the
  `Claim(batch_id, patient_control_number)` UNIQUE constraint is in
  effect, all current fixtures pass, and the in-flight test for
  re-submission works").

## 8. Confidence rating

**Confidence: medium-high.**

Justification: I read the bulk of the backend implementation — `api.py`
(3,548 LOC, all route handlers grepped), `db.py` (839 LOC, all ORM
models and the `ClaimState` enum), `audit_log.py` (254 LOC, the
hash-chain code and its docstrings), `api_helpers.py` (heartbeat +
tail events), `security.py` (485 LOC, three middlewares), `db_crypto.py`
(389 LOC, key rotation), `backup_service.py` (851 LOC, restore flow),
`reconcile.py` (571 LOC, `apply_payment` and `apply_reversal`),
`inbox_lanes.py` (280 LOC, 5 lanes), `scoring.py` (96 LOC, weights),
`inbox_state.py` (76 LOC, 999 transitions), `inbox_state_277ca.py` (108
LOC, monotonic stamping), `scheduler.py` (719 LOC, MFT routing), and
all 12 shipped migrations. I read the frontend live-tail client
(`src/lib/tail-stream.ts`, `src/hooks/useTailStream.ts`) in full and
grepped the rest of `src/{routes,store,components}/` for shape
mismatches. I read `docs/REQUIREMENTS.md` (737 lines), `docs/ARCHITECTURE.md`
(759 lines), and `docs/README.md` in full. I read 6 of the 22 SP specs
in full (SP2 reconciliation, SP5 live tail, SP10 277CA, SP11 audit,
SP14 5-lane Inbox, SP15 key rotation) and grepped the rest for
state-name and constraint assertions. I did **not** read the remaining
16 specs in full, did not read the 27 plan files in full, and did not
read any of the ~73 frontend test files. I also did not verify the
"964 tests collected" claim in NFR-15 nor the existence of
`test_audit_log.py` tests. The medium-high rating reflects strong
backend code coverage, full coverage of the top-of-funnel docs, and
partial coverage of the SP-spec lineage; the gaps that would lower
confidence to "high" are the un-verified plan files, the un-verified
test count, and the un-verified existence of the 6 backend test files
referenced in §6.

---

*End of Reviewer B report. 13 contradictions, 16 forks (4 P0, 4 P1, 5 P2, 3 P3), 12 untested artifacts, 8 escalations. Confidence: medium-high.*