Files
cyclone/docs/reviews/2026-06-23-cyclone-docset-review-B.md
T
cyclone 616d467c65 docs(reviews): capture 2026-06-23 docset review + CSS reduction experiment
Commit the day's review artifacts so they aren't lost in the working tree:

* 2026-06-23-cyclone-docset-review-A.md — full docset review (Reviewer A:
  requirements + architecture lens; flags the 8-state vs 7-state claim
  lifecycle divergence, FR-20 vs ARCHITECTURE §5.4 tail-endpoint conflict,
  api_routers/ package split status, etc.)
* 2026-06-23-cyclone-docset-review-B.md — code-first counterpart (Reviewer B)
* 2026-06-23-cyclone-groundtruth-audit.md — live-data readiness audit
* 2026-06-23-css-reduction/ — iterative CSS reduction experiment
  (baseline 60,695 B raw / 11,786 B gzip / 9,875 B brotli; 18 prioritized
  candidates; per-iter reports and progress log)

Binary artifacts (PNGs, dist build output) are intentionally not committed —
they're working files for the experiment, not review records.
2026-06-23 16:28:15 -06:00

40 KiB
Raw Blame History

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.mddocs/REQUIREMENTS.mddocs/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
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 (SP1SP22) and a future SP23 (Ubuntu/Docker). docs/superpowers/specs/ directory contains specs for SP1SP22 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 SUBMITTEDREJECTED. 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.