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.
61 KiB
Cyclone Documentation Review — Reviewer A
1. Reviewer identity
Reviewer A (Reviewer B is run in parallel; this review covers the full doc set + codebase ground-truth verification).
2. Executive summary
Cyclone's documentation is unusually thorough for a single-operator tool: a top-level REQUIREMENTS.md (38 FRs + 18 NFRs), a top-level ARCHITECTURE.md, 27 design specs (20 dated 2026-06-19 to 2026-06-22 + 7 backfilled 2026-06-23 for SP10–SP16), and 27 implementation plans (15 original + 12 backfilled for SP9–SP20). The doc set has the structure of a well-run engineering project, and the prose quality is high — every spec re-asserts the local-only/single-operator/single-payer design contract, every plan has TDD discipline, and the layering (requirements → architecture → spec → plan → tests) is internally consistent at a high level. However, the doc set has accumulated at least 25 specific contradictions, ambiguous forks, untested artifacts, and stale numbers between the two top-level docs alone — and the contradictions are concentrated in the most consequential areas: the live-tail HTTP surface, the claim-state machine, and the LOC/version counters used in the verification matrix. The 7-state claim lifecycle is wrong in 5 places; the codebase has an 8-state enum (backend/src/cyclone/db.py:180-188). The tail endpoint URL is /api/{resource}/stream per REQUIREMENTS FR-20 + the live-tail spec, but /api/tail?since=… per ARCHITECTURE.md §5.4 — two competent engineers reading only the docs could ship two incompatible APIs. Below, the 4 reviewer questions and 4 flag sections detail every issue with file:line citations.
3. Q1: Components
Description
Cyclone is composed of six concentric layers: (1) the SQLite/SQLCipher store at the bottom (backend/src/cyclone/db.py, backend/src/cyclone/db_crypto.py, migrations 0001–0012), (2) ORM models and pure-function business logic (db.py, reconcile.py, scoring.py, npi.py, cas_codes.py, inbox_lanes.py, inbox_state.py, inbox_state_277ca.py, audit_log.py), (3) the per-parser modules under parsers/ (16 files including the 270/271/277CA/835/999/TA1/837P parsers and serializers), (4) the persistence facade store.py (CycloneStore) plus per-feature handlers (scheduler.py, backup_service.py, backup_scheduler.py, logging_config.py, security.py, pubsub.py, clearhouse/__init__.py, providers.py, payers.py), (5) the FastAPI surface — a monolithic api.py (~3,548 LOC per ARCHITECTURE §3.1 / 3,145 per REQUIREMENTS) plus a partial api_routers/ package (acks.py, admin.py, health.py, ta1_acks.py) that owns the new admin/health endpoints, and (6) the React + Vite + TypeScript frontend (src/) with TanStack Query v5, Zustand, Radix UI primitives, and shadcn-style components.
The top-level docs (REQUIREMENTS.md §2.1, ARCHITECTURE.md §3) describe these layers at the same level of granularity and agree on the boundaries. The cross-SP doc set reinforces them: each spec names the modules it extends (e.g. SP13 clearhouse/__init__.py:319, SP14 inbox_lanes.compute_lanes, SP15 db_crypto + db + api, SP16 scheduler.py:720). The ASCII topology diagram in ARCHITECTURE.md §1.2 is the canonical picture.
Consistency verdict
Mostly consistent, with three meaningful gaps:
-
The
api_routers/package is partially implemented (acks.py,admin.py,health.py,ta1_acks.py) but documented only as "planned" inARCHITECTURE.md§3.2 and not mentioned at all inREQUIREMENTS.md; the specs still reference monolithiccyclone.api(e.g. SP15 §3.3 sayscyclone/api.py:2797-2928for the rotation endpoint, but the actual code lives atapi_routers/admin.py). Engineers building "what does Cyclone's API look like" from the spec set will get the monolithic picture; engineers reading the codebase will see a partially split package. -
backup.py(file upload/retention module) andbackup_service.pyvsbackup_scheduler.py(periodic scheduler) are described as a single "SP17" subsystem across the docs, but the spec file (2026-06-21-cyclone-encrypted-backup-design.md) is the original SP17 spec while the implementation plan is dated2026-06-23-cyclone-sp17-encrypted-backup.md. The two documents describe the same work but the round-3 doc-prep added the plan but did NOT add a backfilled spec for SP17. So the SP17 docs are pre/back mixed, not a clean backfill like SP10–SP16 (which got both spec and plan). -
The
workflow/subdirectory visible underbackend/src/cyclone/has only__pycache__/entries on disk (workflow/__pycache__/__init__.cpython-313.pyc,workflow/__pycache__/batch_diff.cpython-313.pyc,workflow/__pycache__/inbox_lanes.cpython-313.pyc,workflow/__pycache__/inbox_state.cpython-313.pyc,workflow/__pycache__/reconcile.cpython-313.pyc,workflow/__pycache__/scoring.cpython-313.pyc) — but no.pysource files. This indicatesworkflow/was a refactoring target (likely SP21 store-split) whose sources were removed/moved but whose compiled artifacts were not cleaned. None of the docs mentionworkflow/as a deprecated stub; engineers reading the docs will not understand why the directory exists in compiled form only.
Verdict: medium confidence in component consistency. The high-level decomposition is consistent across the top-level docs and the spec set. The drift is in the partial-API-router split, the SP17 plan/spec asymmetry, and the unexplained workflow/ ghost directory.
4. Q2: Data model
Description
The data model is a SQLAlchemy 2.0 ORM (backend/src/cyclone/db.py) backed by SQLite, optionally encrypted via SQLCipher (backend/src/cyclone/db_crypto.py — SP12). Migration runner is db_migrate.py reading PRAGMA user_version — 12 migrations on disk: 0001_initial through 0012_backups (listed at backend/src/cyclone/migrations/). The core tables are: batches, claims, remittances, cas_adjustments, matches, activity_events, service_lines, service_line_payments, line_reconciliations, acks, ta1_acks, providers, payers, clearhouse, payer_rejected columns on claims (SP10, migration 0008), payer_rejected_acknowledged_at/_actor (SP14, migration 0010), audit_log (SP11, migration 0009), processed_inbound_files (SP16, migration 0011), and backups (SP17, migration 0012).
The key entity relationships are: Batch 1—N Claim, Batch 1—N Remittance, Remittance N—1 Claim (nullable FK on claims.matched_remittance_id), Remittance 1—N CasAdjustment, Claim 1—1 Match (unique), Claim 1—N ServiceLine, Remittance 1—N ServiceLinePayment, Claim 1—N LineReconciliation, Remittance 1—N LineReconciliation, and AuditLog as a chain on its own (no FKs; the chain links by prev_hash). ServiceLinePayment is the SP7 bridge between the 837 and 835 line worlds; it carries the procedure_code/service_date/charge/payment/units plus modifiers_json as a serialized list. The audit_log table (SP11) is a separate hash-chained append-only log keyed by SHA-256 of (event_type, entity_type, entity_id, actor, created_at, prev_hash); the chain is verifiable via cyclone.audit_log.verify_chain(session) (verified at backend/src/cyclone/audit_log.py:190) which recomputes every hash and walks the prev_hash links (audit_log.py:228-242).
The state machine has multiple representations in the docs:
REQUIREMENTS.md§3.1 + §6.3 (traceability) says: "7-state claim lifecycle: submitted, received, paid, partial, denied, reconciled, reversed."backend/src/cyclone/db.py:180-188(code) says:ClaimStateenum with 8 values:SUBMITTED, RECEIVED, REJECTED, PAID, PARTIAL, DENIED, RECONCILED, REVERSED. The 8th state,REJECTED, was added by the workflow-automation/SP6 plan (docs/superpowers/plans/2026-06-20-cyclone-workflow-automation.mdTask 1) when 999 AK9 R/E rejection handling was introduced.ARCHITECTURE.md§6.3 says: "states: submitted, accepted, paid, denied, appealed, rejected, payer_rejected" — 7 states, includingaccepted(which is NOT in the code) andpayer_rejected(which is NOT a state in the code but a set of columns onclaimsfiltered on byinbox_lanes.compute_lanes).docs/superpowers/specs/2026-06-19-cyclone-db-reconciliation-design.md§6.7 (pre-SP6) says: "submitted, received, paid, partial, denied, reconciled, reversed" — 7 states, matches the pre-SP6 plan.
The payer_rejected status is not a ClaimState value. It is stored as four columns on claims: payer_rejected_at, payer_rejected_reason, payer_rejected_status_code, payer_rejected_by_277ca_id (verified at backend/src/cyclone/db.py:270-280; SP10 spec §2.1). The 5th inbox lane (payer_rejected) is computed by inbox_lanes.compute_lanes filtering on payer_rejected_at IS NOT NULL AND payer_rejected_acknowledged_at IS NULL (SP14 spec §3.2). So payer_rejected is a view, not a state — yet ARCHITECTURE.md lists it as a state.
The Match model carries an is_reversal flag (defined in the SP2 plan) and the spec calls for a state_before_reversal column on claims to preserve reversals (db_reconciliation-design.md §6.6). The 835 reverse-direction (CLP02 ∈ {21, 22}) is handled by apply_reversal() in reconcile.py which restores the prior state. However, the SP10 277CA path does NOT extend the state machine — claims with a payer rejection stay in their prior state (paid, denied, partial, etc.) and gain the payer_rejected_at timestamp; the 5-lane inbox surfaces them but claim.state does not transition to a new value. This is consistent with the SP10 spec §2.3 ("we explicitly do not clear payer_rejected_at on acknowledge") and SP14 spec §2.3, but contradicts the more casual phrasing in ARCHITECTURE.md §6.3.
Consistency verdict
Low confidence in data-model consistency across the doc set. The ClaimState enum has 8 values (verified at backend/src/cyclone/db.py:180-188) but the docs claim 7 in 4 places (REQUIREMENTS.md §3.1 and §6.3, ARCHITECTURE.md §6.3, the db-reconciliation spec §6.7, and the workflow-automation plan Task 1 implicitly). The 5th payer_rejected "state" in ARCHITECTURE.md §6.3 is not a state in the code — it is a column-and-filter view. Engineers building the data model from the docs alone will produce either an 8-state enum without a payer_rejected value (correct) or a 9-state enum with both rejected and payer_rejected (wrong, not in the code).
The second meaningful gap is the dual audit mechanism. activity_events (db.py original migration 0001) is a regular table for transition events; audit_log (SP11, migration 0009) is a separate hash-chained table for HIPAA-relevant events. The docs are inconsistent on which events go where: the SP14 spec §3.3 says claim.payer_rejected_acknowledged is appended to audit_log, but the SP6 workflow plan implies claim.rejected (the 999 R/E event) lands in activity_events. The two tables overlap in name space but have different writers — audit_log.py:99-126's append_event takes an AuditEvent payload but activity_events is written from a different code path (store.py). No doc explicitly enumerates which kind strings go to which table.
5. Q3: Dependencies
Description
There are four overlapping dependency graphs in this doc set: (a) spec-level dependencies ("depends on SP X"), (b) plan-level task dependencies (TDD phases 1 → 2 → 3 with git worktree rebases), (c) module/import dependencies in the codebase, and (d) data-model migration ordering. The graphs mostly agree; the divergences are concentrated in SP21/SP22 (in-flight) and SP23 (candidate).
(a) Spec dependencies. The 7 backfilled specs (SP10–SP16) each declare their upstreams explicitly:
- SP10 (277CA)
2026-06-23-cyclone-sp10-277ca-payer-rejected-design.md§"Depends on:" → SP3 (999 parser) + SP6 (workflow/inbox). - SP11 (audit log) → SP6 + SP10 + SP12.
- SP12 (SQLCipher) → standalone.
- SP13 (SFTP) → SP9 (SftpClient stub + secrets).
- SP14 (5-lane inbox) → SP6 + SP10 + SP11.
- SP15 (key rotation) → SP11 + SP12 (cannot ship without SP12;
2026-06-23-cyclone-sp15-key-rotation-design.mdline 11: "Cannot ship without SP12"). - SP16 (MFT polling) → SP9 + SP11 + SP13 + SP15 (implicitly, since SFTP auth uses the Keychain) + the per-parser modules.
These declarations are consistent with the implementation order in the git log (SP10 → SP11 → SP12 → SP13 → SP14 → SP15 → SP16) and with the migration versions (0008 through 0011).
(b) Plan-level task dependencies. The plans use a [ ] Step N → commit pattern with explicit prerequisites (e.g. live-tail.md Task 0 requires SP5 to be merged first; claims-unique-constraint-and-409-ux.md worktree setup requires pip install -e backend). The plan-level dependencies are clean within each plan; the cross-plan dependencies are weak. For example, the SP14 plan does not declare it depends on SP10's migration 0008_payer_rejected_columns.sql — it just references "the SP10 payer_rejected_* columns" without naming the migration.
(c) Module dependencies. The import graph in backend/src/cyclone/ shows the expected layering: parsers/ modules import cyclone.exceptions and cyclone.segments only (clean isolation); db.py imports nothing from cyclone.* (forward references via string IDs); reconcile.py, scoring.py, inbox_lanes.py import cyclone.db; store.py imports everything (db, reconcile, scoring, inbox_state, inbox_state_277ca, pubsub); api.py imports store, db, audit_log, scheduler, db_crypto, security, etc. The scheduler.py module intentionally duplicates small helpers (_ack_count_summary, _ack_synthetic_source_batch_id, _277ca_synthetic_source_batch_id) to break what would otherwise be a circular dependency with cyclone.api — this is documented in SP16 spec §3.5. The audit_log.py:99-126 append_event function takes a session and an AuditEvent, called by SP14 ack endpoint, SP15 rotation endpoint, and SP16 scheduler handlers. The store-scheduler boundary is clean.
(d) Migration ordering. 12 migrations on disk (0001_initial through 0012_backups) are forward-only via PRAGMA user_version. The runner (db_migrate.py) reads -- version: N headers, sorts by filename, applies any with version > current (db_migrate.py:438-466). The version assertions in test_db_migrate.py confirm monotonic ordering. The claims-unique-constraint-and-409-ux.md plan adds migration 0013_drop_claims_unique_constraint.sql on a worktree (claims-unique-fix); this migration does NOT exist on main yet, which is consistent with the plan being in-flight. However, the 2026-06-23-cyclone-sp11-hash-chained-audit-design.md spec refers to migration 0009_audit_log.sql and the docs say SP11 ships migration 0009 — that migration IS on disk and audit_log.py exists, confirming SP11 shipped.
(e) SP21 / SP22 / SP23 status. SP21 (store split) is in-flight on refactor/store-split branch (docs/superpowers/specs/2026-06-21-cyclone-store-split-design.md Status: "Draft / not merged"); SP22 (parse-decide) lives on claims-unique-fix worktree (docs/superpowers/specs/2026-06-21-cyclone-parse-decide-workflow-design.md + the unique-constraint plan). SP23 (Ubuntu Docker + RBAC) is a candidate only (docs/superpowers/specs/2026-06-22-cyclone-ubuntu-docker-deployment-design.md Status: "Draft / candidate"). These are called out in the spec titles, but the top-level docs (REQUIREMENTS.md §1.4, ARCHITECTURE.md §0.1) do NOT explicitly mark SP21/SP22/SP23 as in-flight. An engineer reading only the top-level docs would assume all 23 SPs are at the same ship-readiness level.
Consistency verdict
High confidence within the shipped SPs (SP1–SP20), where the spec→plan→code→migration→test graph is internally consistent. Low confidence for SP21/SP22/SP23, which are in-flight or candidate and not marked as such in the top-level docs. The spec-level "Depends on" declarations are accurate and actionable.
6. Q4: Definition of done
Description
REQUIREMENTS.md §8 (lines 643–681) defines "Definition of Done" in three sub-sections: §8.1 Functional DoD (every FR covered by ≥1 spec + ≥1 plan + ≥1 integration test), §8.2 Non-Functional DoD (every NFR has a verification command), §8.3 Operational DoD (deployment posture, backup readiness, encryption posture). Each FR has a row in the §6.1 traceability matrix pointing at the covering spec/plan and a test file. ARCHITECTURE.md §10 ("Operational Story") describes the deploy / backup / runbook posture in narrative form.
The §8.1 Functional DoD says every FR is "Met" if it has a covering spec, plan, and integration test. The §6.1 traceability matrix marks FR-1 through FR-38 with a single test file each (e.g. FR-20 → test_api_stream_live.py + TailStatusPill.test.tsx). The matrix does NOT explicitly require happy-path AND failure-path test coverage — but REQUIREMENTS.md §4 NFR-2 ("Test discipline: every route has a happy-path test and at least one failure-path test") adds this requirement at the NFR layer. So the union of FR + NFR DoD is: each route has a happy + failure path test.
The DoD is partially testable: the test-file pointers in the traceability matrix are verifiable by ls backend/tests/ and ls src/**/*.test.ts*; the happy/failure-path test count is verifiable by reading each test file. The encryption-at-rest posture (NFR-5 / SP12) is verifiable by checking that cyclone.db_crypto.is_encryption_enabled() returns True when sqlcipher3 is installed and the Keychain entry exists (per the SP12 spec §1). The single-process posture (NFR-14) is verifiable by reading the scheduler module's lack of threading.Thread / multiprocessing.Process callers. The audit-chain integrity (NFR-4 / SP11) is verifiable by GET /api/admin/audit-log/verify returning ok: true, checked: N+1.
Testability verdict
Medium confidence. Four issues break testability:
-
The
REQUIREMENTS.md§8.3 Operational DoD mentions "Vitest ^4.1.9" and flags it in §5.3 / R-25 as "unusually new — verify the version pin is intentional before the next npm install; fallback to^2.xif accidental." TheARCHITECTURE.md§3.2 (tooling table) listsvitest 4.1.9without flagging it. The flagging is inconsistent — one top-level doc treats it as a problem to fix, the other treats it as normal. A CI gate that asserts "vitest@^2.x" would fail on the codebase. -
The §8.3 DoD claims
vitest@^4.1.9is "unusually new (current stable line is 1.x/2.x)" — but Vitest 4.x does exist (released after Vitest 3.x in 2025); the claim is also inaccurate relative to the doc's own date. The §6.3 acceptance row for SP5 says the live-tail test count target was "~144 frontend pass" with "~437 backend" — butbackend/tests/contains 94 test files (verified by directory listing). The number is in the right order of magnitude but unverified. -
The §10 Operational Story in
ARCHITECTURE.mddescribes "5-second heartbeat" on the live-tail stream — butREQUIREMENTS.md§4 NFR-10 (line 163) says "Heartbeat every 15s default (CYCLONE_TAIL_HEARTBEAT_S)". The actualbackend/src/cyclone/api.py:1679-1680comment saysCYCLONE_TAIL_HEARTBEAT_S(consistent with REQUIREMENTS, not ARCHITECTURE). The 5s heartbeat claim is wrong against both the codebase AND REQUIREMENTS.md. The operational story in §10 cannot be the basis for a deploy runbook until this is corrected. -
The §10 Operational Story does not describe the SP16 scheduler (
CYCLONE_SCHEDULER_AUTOSTARTopt-in,CYCLONE_SCHEDULER_POLL_SECONDSdefault 60) at all — only the manual operator path. The §8.3 DoD also does not mention "scheduler running withoutCYCLONE_SCHEDULER_AUTOSTART=true" as a verified-state precondition. The "auto-poll" deploy posture introduced by SP16 is invisible to the top-level DoD.
Verdict: The DoD is partially testable. The functional DoD (each FR → test file) is verifiable by file enumeration. The non-functional DoD (each NFR → verification command) is mostly verifiable but the Vitest version flag, the heartbeat interval, and the scheduler autostart posture create 3 places where a competent engineer reading only the top-level docs would either install the wrong version or deploy the wrong polling config.
7. Contradictions
The following are direct contradictions in the doc set. Each cites file:line for both sides.
-
api.pyLOC count.REQUIREMENTS.md§3.2 / R-23 saysapi.pyis 3,145 LOC.ARCHITECTURE.md§3.1 sayscyclone.api:appis 3,548 LOC withcyclone.storeat 2,423 LOC.REQUIREMENTS.mdR-24 saysstore.pyis 2,172 LOC. Three different numbers for two files. -
store.pyLOC count. Same as above: 2,172 (REQUIREMENTS R-24) vs 2,423 (ARCHITECTURE §3.1). -
Live-tail heartbeat interval.
REQUIREMENTS.md§4 NFR-10 (line 163): "Heartbeat every 15s default (CYCLONE_TAIL_HEARTBEAT_S)".ARCHITECTURE.md§5.4: "5s heartbeat" (in the description of the live-tail stream). Code (backend/src/cyclone/api.py:1679-1680): "everyCYCLONE_TAIL_HEARTBEAT_Sseconds when idle". Thelive-tailplan (§7 / Phase 3 Task 11) says "the 15s heartbeat viaasyncio.wait_forloop (overridable for tests)". The 5s claim is wrong; the code and NFR-10 agree on 15s. -
Live-tail endpoint URL.
REQUIREMENTS.md§3.1 FR-20 (line 128) says/api/{claims,remittances,activity}/stream.ARCHITECTURE.md§5.4 saysGET /api/tail?since=…. Thelive-tailspec (2026-06-20-cyclone-live-tail-design.md) and plan (2026-06-20-cyclone-live-tail.md) use the per-resource URL pattern. The code (api.py:1679-1680) matches the per-resource pattern. The ARCHITECTURE §5.4 single-endpoint claim is wrong. -
Claim lifecycle state count.
REQUIREMENTS.md§3.1 + §6.3 traceability: "7-state claim lifecycle".ARCHITECTURE.md§6.3: "7 states: submitted, accepted, paid, denied, appealed, rejected, payer_rejected". Code (backend/src/cyclone/db.py:180-188): 8 states (SUBMITTED, RECEIVED, REJECTED, PAID, PARTIAL, DENIED, RECONCILED, REVERSED). The docs disagree with themselves and with the code. -
Claim lifecycle state names.
ARCHITECTURE.md§6.3 listsacceptedandappealed— neither exists in the code or any spec. The code usesRECEIVED(notaccepted) and has noappealedstate. The lifecycle spec says reversal preserves the prior state viastate_before_reversal; there is no appeal flow. -
payer_rejectedas a state.ARCHITECTURE.md§6.3 lists it as a state. The SP10 spec §2.3 and SP14 spec §2.3 explicitly statepayer_rejected_atis a timestamp column onclaims, NOT aClaimStatevalue. The lane filter (SP14 spec §3.2) ispayer_rejected_at IS NOT NULL AND payer_rejected_acknowledged_at IS NULL. A lane is not a state. -
Vitest version flagging.
REQUIREMENTS.md§5.3 R-25 (line 652) flagsvitest@^4.1.9as "unusually new" and a fallback to^2.xis suggested.ARCHITECTURE.md§3.2 listsvitest 4.1.9in the tooling table without any flagging. Engineers reading only the architecture doc will not realize the version is under review. -
Plan ID numbering. The user's plan inventory says "15 original plans + 12 backfilled plans = 27 plans". The on-disk count matches (15 + 12). But the original plans use a SP-numbering scheme ("Sub-project 4", "Sub-project 6", "Sub-project 7") while the backfilled plans use a
sp9–sp20naming scheme. There is nosp1–sp8backfill because those SPs were the originals. Engineers searching for "SP7 plan" find2026-06-20-cyclone-line-reconciliation.md(Sub-project 7) — works — but searching for "SP8 plan" finds2026-06-20-cyclone-serialize-837.mdwhose own title says "Sub-project 7" (the file was re-purposed from SP7 to SP8). Both top-level docs note this self-acknowledged mis-labeling (REQUIREMENTS.md§1.4 + R-22 +ARCHITECTURE.md§0.1 mention it) but do not fix it on disk. -
SP number ↔ filename mapping. SP9 =
multi-payer-npi-sftp(spec is "SP9 — Multi-payer NPI + SFTP"). But the user's plan inventory says "12 backfilled plans (dated 2026-06-23-cyclone-sp{9..20}-*.md)" — SP9 is the first backfilled. The original spec for SP9 lives at2026-06-20-cyclone-multi-payer-npi-sftp-design.md(not in the backfill range). So SP9 has an original spec + a backfilled plan + a backfilled spec for SP13-SFTP. The mapping is "SP9 = NPI + SFTP" but the "SFTP" name also gets re-used for SP13'sSftpClientparamiko wire-up. Two SPs share the SFTP concept; SP9 = config/secrets, SP13 = transport. The docs do not always disambiguate. -
SP17 spec/plan asymmetry. SP17 (encrypted backup) has a spec file dated
2026-06-21(the original) but the plan is dated2026-06-23-cyclone-sp17-encrypted-backup.md(backfilled). The other 6 backfilled SPs (SP10–SP16) have both spec and plan backfilled; SP17 has only the plan backfilled. The user said "12 backfilled plans for SP9-SP20" — so SP17 falls in that range — but the spec is original. This is an asymmetry in the doc-prep pass. -
audit_logevent-type vocabulary. The SP14 spec §3.3 enumeratesclaim.payer_rejected_acknowledgedas the audit event. The SP15 spec §3.4 enumeratesdb.key_rotated. The SP16 spec §3.5 enumeratesclaim.rejected(from 999_handle_999) andclaim.payer_rejected(from 277CA_handle_277ca). The completeness review (2026-06-20 §3.1 item 2) calls outActivityEventas "not tamper-evident" — but the SP11 audit log adds tamper-evidence only for events that opt in toappend_event. The completeness review and the round-3 docs do not reconcile whichkindstrings go toactivity_events(mutable) vsaudit_log(hash-chained). -
Live-tail reconnect backoff cap.
REQUIREMENTS.md§4 NFR-10 and FR-21: "backoff ladder1s → 2s → 4s → 8s → 16s → 30scapped". Thelive-tailspec §3.4 says the same ladder. The plan (live-tail.mdTask 19) repeats it. No contradiction here, but the spec/plan/test all say "stalled at 30s silence" while the plan also says "the hook should NOT auto-reconnect on stall (user confirms)" — the FR-21 backoff ladder applies only toreconnecting, notstalled. A naive reader of FR-21 could think the stalled state auto-reconnects. -
store.add(rec, event_bus=…)parameter. Theproduction-readiness.mdplan Task 4 wiresstore.add()withevent_bus=request.app.state.event_bus. Theclaims-unique-constraint-and-409-ux.mdplan Task 1.3 callsstore.add(rec, event_bus=request.app.state.event_bus)from the 837/835 endpoints. But the originalstore.py:226InMemoryStore.add()and theCycloneStore.add()impl do not have anevent_bus=parameter — the call sites passevent_bus=to a function that does not accept it. This is historical drift (the InMemoryStore predates the EventBus in SP5) but the docs do not call out the refactor. -
Migrations total count.
REQUIREMENTS.md§5.2 lists migrations0001through0012(12 total) but the §6.3 acceptance rows reference0013_drop_claims_unique_constraint(which is on theclaims-unique-fixworktree, NOT onmain). Engineers runningls backend/src/cyclone/migrations/onmainsee 12 files; engineers on the worktree see 13. The doc text is ambiguous about which branch it describes. -
Test count target. The historical
2026-06-20-completeness-review.md§1 says 574 backend tests. Theproduction-readiness.mdplan §"Test count targets" (Phase 2 final target) says 437 backend. Thelive-tail.mdplan §"Test count targets" says 437 backend + 144 frontend. The user's question-statement says 964 tests. Four different numbers for "how many tests does Cyclone have". The discrepancy is partly due to date (574 was measured in 2026-06-20; 437 was a target for SP5; 964 is current). -
API router location for the rotation endpoint.
2026-06-23-cyclone-sp15-key-rotation-design.md§3.3 says the endpoint lives atcyclone/api.py:2797-2928. The actual endpoint lives atbackend/src/cyclone/api_routers/admin.py(theapi_routers/split happened after SP15 was written). The line range is stale. -
Encrypted-at-rest posture.
REQUIREMENTS.md§4 NFR-5: "SQLite is encrypted via SQLCipher AES-256 when the macOS Keychain entry exists andsqlcipher3is installed; plain SQLite otherwise."ARCHITECTURE.md§10 Operational Story: "the SQLite file at~/.local/share/cyclone/cyclone.dbis plaintext by default; SQLCipher optional." The two phrasings agree on substance but the operational story makes the "plaintext by default" stance more prominent than the requirements doc. -
API endpoint for the MFT scheduler status. The SP16 spec §4.1 says
GET /api/admin/scheduler/statusreturns aSchedulerStatusJSON. The SP16 spec §3.3 says theGET /api/healthendpoint also reportssnap.scheduler.running. Two endpoints expose scheduler state with different shapes. The DoD does not say which is canonical. -
Audit-event
actorsemantics. SP14 spec §3.3 saysactordefaults to"operator"and is operator-supplied via request body. SP15 spec §3.4 says the same. The SP23 candidate (ubuntu-docker-deployment-design.md) introduces RBAC andactorbecomes a real identity from a JWT claim. The two phrasings agree on default behavior but differ on whetheractoris free-form text (today) vs identity-validated (SP23 future). Engineers building an audit-evidence tool today must treatactoras untrusted free-form text. -
/api/clearhouse/submitparameter shape. SP13 spec §4 says no new endpoints, but references the SP9 endpointPOST /api/clearhouse/submit. The SP9 spec is the original spec (2026-06-20-cyclone-multi-payer-npi-sftp-design.md). The two docs disagree on the request shape: SP9 spec §4 hasbody: { sftp_block_name, payload }; SP13 spec §4 says SP9 endpoint body is "unchanged" but adds anauthdict toSftpBlockmodel. The exact request schema is documented in SP9 only; SP13 trusts it. Two readers building SP13 from spec-only could ship two different request bodies. -
processed_inbound_filestable ↔ scheduler hand-off. The SP16 spec §3.5 says_handle_999and_handle_277cawrite SP11 audit events. The plan for SP14 saysclaim.payer_rejected_acknowledgedis an audit event. Both audit events end up in the sameaudit_logtable but no doc names a "row kind namespace" — an operator queryingSELECT event_type, COUNT(*) FROM audit_log GROUP BY event_typewould need to enumerate every spec's event types to build a vocabulary. -
Reconciliation failure semantics.
REQUIREMENTS.md§4 NFR-2 says reconciliation must be "fail-soft — a 999 parser crash does not lose the 835". The db-reconciliation spec §6.6 says the same. The completeness review §4 item 4 says "ActivityEvent is mutable". Both can be true, but the reconciliation "fail-soft" event goes toactivity_events(mutable) and the SP16 "scheduler per-file try/except" event goes toprocessed_inbound_files.error_message(also not inaudit_log). An auditor searching the hash-chained log for a reconciliation failure will find nothing for SP5-era crashes. -
Store ownership of
parse_inbound_filename. The SP16 spec §3.5 says_handle_999etc. usecyclone.edi.filenames.parse_inbound_filename(a separate module undercyclone/edi/filenames.py). The completeness review (pre-SP16) doesn't mention this module. The SP16 spec was backfilled; the upstream SP9 spec forSftpClientdoes not mentionedi/filenames.pyeither. The module exists in the codebase (backend/src/cyclone/edi/filenames.py) and is covered bytest_filenames.py— but no spec describes why it lives inedi/instead ofparsers/. -
claim.rejectedaudit event idempotency. SP16 spec §3.5 says_handle_999writes oneclaim.rejectedaudit event per matched claim. The SP6 workflow plan (workflow-automation.mdTask 3) saysapply_999_rejectionsis idempotent on already-rejected claims. The interaction is: a 999 file applied twice (e.g. a retry) writes 0 new audit events on the second call (becauseclaim.state == REJECTEDshort-circuits inapply_999_rejections). The SP16 spec does not say "scheduler retries are idempotent on the audit chain" — only on the parser. A future scheduler retry-aware design might emit aclaim.rejected_reappliedaudit event, but no doc says so.
8. Forks (highest-priority first)
These are the items where two competent engineers reading only the docs would plausibly build different systems.
F1 (P0). Tail endpoint URL. FR-20 says /api/{resource}/stream; ARCHITECTURE §5.4 says /api/tail?since=…. Two URLs, two shapes, two different filter mechanisms. The codebase matches FR-20. An engineer reading only ARCHITECTURE.md would build a single-endpoint /api/tail?since=… that does not exist.
F2 (P0). Claim lifecycle state names. REQUIREMENTS says 7 states (no REJECTED, has received); ARCHITECTURE says 7 states (with accepted, appealed, rejected, payer_rejected); code has 8 states (SUBMITTED, RECEIVED, REJECTED, PAID, PARTIAL, DENIED, RECONCILED, REVERSED); ARCHITECTURE adds payer_rejected as a state but the code does not. An engineer building from REQUIREMENTS would ship a 7-state enum without REJECTED and break the SP6 workflow-automation apply_999_rejections path; an engineer building from ARCHITECTURE would ship a 7-state enum with appealed and payer_rejected and break the codebase.
F3 (P0). Heartbeat interval. 5s vs 15s. The code is 15s. An engineer deploying based on ARCHITECTURE §5.4 would set CYCLONE_TAIL_HEARTBEAT_S=5 (or not set it and expect 5s) and double the bandwidth on the live-tail stream. An operator monitoring for "stalled" would flip state 5× as often.
F4 (P0). 409 collision response shape. The SP22 parse-decide-workflow-design.md describes the 409 body for a CLM01 collision as {parse_result, collisions: [{colliding_claim_ids, existing_batch_id}]} — a richer shape that lets the UI surface "you have N existing batches that already hold this CLM01". The claims-unique-constraint-and-409-ux.md plan + spec describe the 409 body as {error, detail, batch_id, existing_batch_id} — a simpler shape that only carries the first matching batch. The two specs describe different 409 contracts. A frontend engineer building from SP22 will render a richer panel; one building from the claims-unique-constraint plan will render a simpler panel.
F5 (P1). ?ack=true response shape on parse-837. REQUIREMENTS §3.1 FR-9 says ?ack=true "auto-generates a 999 acknowledgement". The edi-features.md plan Task 15 says "persist Ack row, attach to response body" — but the response shape is described as "the 999 is attached inline". The live-tail spec does not re-acknowledge. Three different response shapes are plausible: (a) {claims, summary, ack: {ack_id, body}} with the 999 inline, (b) {claims, summary, ack_id} requiring a follow-up GET, (c) {claims, summary, ack_999_x12: "ISA*..."} with raw text. The plan's test (test_api_parse_persists.py ack=true happy path) checks the response, but no doc publishes the canonical shape.
F6 (P1). payer_rejected lane ↔ rejected lane ordering. SP14 spec §1 says the 5 lanes are ordered "envelope problems → payer problems → recon opportunities → claims in flight → done today". SP14 spec §3.7 code shows the order rejected, payer_rejected, candidates, unmatched, done_today. The SPEC.md says the same. The summary tile grid is grid-cols-2 sm:grid-cols-3 lg:grid-cols-5. No conflict in the docs, but if a future engineer "tidies" the lane order to alphabetical, the "Need eyes" counter (which sums rejected + payer_rejected + candidates + unmatched per SP14 spec §3.8) would still work; the visual hierarchy would change.
F7 (P1). ActivityEvent vs audit_log event namespace. The codebase has two parallel event tables. The SP11 spec says audit_log is for HIPAA-relevant events (claim.rejected, claim.payer_rejected_acknowledged, db.key_rotated). The pre-SP11 design (still in store.py) writes activity_events for state transitions (claim_submitted, manual_match, manual_unmatch, reconcile, etc.). No doc publishes a complete list of which event types go to which table. A future engineer adding a new event (e.g. claim.appealed) would have to choose which table to use with no explicit guidance. Note: SP23 candidate mentions "appeals workflow" without specifying the table.
F8 (P1). store.add(rec, event_bus=…) signature. The live-tail plan (SP5) and the unique-constraint plan (SP22) both call store.add(rec, event_bus=request.app.state.event_bus). The pre-SP5 InMemoryStore.add() does not accept event_bus=; the post-SP5 CycloneStore.add() signature is unclear from the docs. A new engineer refactoring store.py (which SP21 plans to do) could break both call sites. The plan-level dependency on SP5's event_bus is implicit in 7+ places; the SP21 spec does not list this as a constraint.
F9 (P1). api.py → api_routers/ split. The codebase has api_routers/{acks,admin,health,ta1_acks}.py. The specs all reference cyclone.api:app. An engineer building from the docs would add new endpoints to api.py; an engineer reading the codebase would add to api_routers/. The split is undocumented in the top-level docs.
F10 (P1). Live-tail stream filter. FR-20 says the stream is per-resource. The live-tail.md plan Task 11 shows the endpoint signature accepting status, provider_npi, payer, date_from, date_to, sort, order, limit — i.e. the stream is a filterable snapshot+subscription, not a firehose. Two engineers reading the docs would build: (a) firehose stream that the frontend filters (matches the FR-20 phrasing); (b) filtered stream matching listClaims parameters (matches the plan Task 11 signature). The actual code matches (b).
F11 (P2). processed_inbound_files row identity. SP16 spec §2.2 says the unique index is (sftp_block_name, name). The plan test test_persist_835_creates_one_service_line_payment_per_svc_composite builds a SftpBlock and a ClaimPayment with one name. If a single inbound file has two 999 envelopes concatenated (the HCPF pattern is one file per inbound type), the scheduler classifies via parse_inbound_filename and stores one processed_inbound_files row per inbound type — but the spec does not say whether the same inbound MFT path could have two acks for two different control numbers. Edge case; probably out of scope; but undocumented.
F12 (P2). audit_log clock source. SP11 spec §3 says created_at is "the wall-clock at the time of append_event". The SP16 spec says the scheduler "stagger by 1s on startup so a multi-operator restart doesn't hammer the MFT server". If a 999 arrives at wall-clock T, the claim.rejected audit event has created_at = T. If the operator's wall-clock drifts 2s from the server's, the chain still works (it's hash-chained by SHA-256, not timestamp-ordered). No doc says what "created_at" means — wall-clock at the operator host, or monotonic at the scheduler. This matters for HIPAA §164.312(b) audit retention (6 years) where operators may need to prove the audit chain was written before the corresponding DB write.
F13 (P2). claim.id source. SP10 spec §2 says claim.id is the X12 CLM01. SP14 spec §2 says the lane filter is on payer_rejected_at (not claim.id). The uniqueness-constraint spec (SP22) says claim.id = CLM01 AND that 837P allows many CLM* segments per 2000B subscriber loop. If a 837P file has two CLM01 collisions (e.g. two CLM01="CLM-A" segments in different 2000B loops), the PK on claims.id raises IntegrityError on the second insert. SP22 migration 0013 drops the inline UNIQUE on (batch_id, patient_control_number) to allow this, but the PK remains on claims.id. Two CLM01="CLM-A" segments still trip the PK. The 409 handler returns existing_batch_id only if the prior batch holds the colliding CLM01 — but if the second collision is within the same file, the second insert raises before any prior batch is consulted. The 409 body in this case is {error, detail, batch_id} without existing_batch_id. SP22 spec does not document this edge case.
F14 (P2). SFTP host-key policy. SP13 spec §3.2 uses AutoAddPolicy() (trust-on-first-use). The plan does not change this. The spec §7 "Open questions" lists SftpBlock.strict_host_key_check: bool as a future hardening. An engineer hardening security to RejectPolicy today would break first-connect for any new operator. No doc says "do not change this without a migration path".
F15 (P2). processed_inbound_files status pending. SP16 spec §2.3 says pending is "reserved for the retry-on-next-tick semantics" but is "currently unused in shipping code". The plan does not add a test for the pending status. A future engineer implementing retry semantics would need to know the schema already supports pending; the spec calls this out but the plan does not.
F16 (P2). Scheduler first-tick stagger. SP16 spec §3.1.2 says _run staggers the first tick by 1s on startup. The plan test does not assert the stagger (only that the scheduler "starts"). A future engineer removing the asyncio.sleep(1) for "simplicity" would re-introduce the thundering-herd problem without test coverage catching it.
F17 (P3). _handle_277ca vs _handle_999 line numbers. SP16 spec §3.1.1 says both handlers emit claim.rejected / claim.payer_rejected audit events. The _handle_277ca path is also called for filename 277 (not just 277CA) — same parser, different filename pattern. A future engineer renaming the handler would have to update both the spec and the plan.
F18 (P3). serialize_837_for_resubmit parameter shape. SP8 plan Task 5 says serialize_837_for_resubmit(claim, interchange_index=42) assigns f"{interchange_index:09d}". The spec says the same. But the spec §3.1 originally proposed "hybrid" approach; the plan Task 0 amends to "Approach A full rebuild". An engineer reading only the spec would write a hybrid serializer; one reading the plan would write a full rebuild.
F19 (P3). SP22 parse-decide spec vs plan. The SP22 parse-decide spec §3.2 says the 409 endpoint returns parse_result and collisions arrays. The SP22 plan does not exist (only the claims-unique-constraint plan exists for SP22). Engineers building from the spec alone have no test plan. The spec mentions a "TBD: 409 panel UX" in §6.
F20 (P3). Inbox lane ordering by state_changed_at. SP6 workflow-automation plan Task 6 compute_lanes uses Claim.state_changed_at >= cutoff for the done_today lane (cutoff = 24h ago). The migration 0004_rejections_and_state_history.sql adds state_changed_at and the index ix_claims_state_changed_at. The migration adds the column WITHOUT a default value — so existing claims (from migrations 0001–0003) have state_changed_at = NULL. They never appear in done_today. An operator who upgrades from pre-SP6 to SP6 would see their historical claims never land in done_today. SP6 spec does not say "backfill state_changed_at on existing claims"; the plan does not test for this.
F21 (P3). The SP10 277CA path does not extend the state machine. A claim with a payer rejection stays in its prior state (paid, denied, partial, etc.) and gains the payer_rejected_at timestamp. This is consistent with SP10 spec §2.3 ("we explicitly do not clear payer_rejected_at on acknowledge") and SP14 spec §2.3, but the architecture doc's casual phrasing ("states: ... payer_rejected") suggests otherwise. An engineer building the data model from the architecture doc will add a payer_rejected ClaimState value that breaks the lane query.
9. Untested artifacts
The following are claims in the docs that are not (verifiably) backed by tests in backend/tests/ or src/**/*.test.ts*. I name the test file that should cover each.
U1. REQUIREMENTS.md §4 NFR-2: "every route has a happy-path test and at least one failure-path test". This NFR is a meta-claim about test discipline. No meta-test asserts NFR-2 (e.g., a pytest --collect-only | jq '.[] | select(.failure_path == false)' would fail). The convention is followed in some places (e.g. test_api_rotate_key.py has happy + 409 + 503 paths; test_payer_rejected_acknowledge.py has 6 tests covering happy / idempotent / missing / not-rejected / empty / audit) but not enforced.
U2. REQUIREMENTS.md §4 NFR-10: "backoff ladder 1s → 2s → 4s → 8s → 16s → 30s capped". The live-tail.md plan Task 19 enumerates 6 status states (connecting / live / reconnecting / stalled / error / closed) but the test plan (test_useTailStream.test.ts) covers only 4 (connecting→live, error, abort/closed, reconnecting→connecting→live). The backoff ladder cap (30s) is not asserted. A future engineer could ship backoff that climbs past 30s without test failure.
U3. REQUIREMENTS.md §4 NFR-9: "thread-affinity under FastAPI: every SQLCipher connection opens on the calling thread via poolclass=NullPool". db_crypto.py is the only module that wires NullPool. No test in test_db_crypto.py asserts poolclass == NullPool is set on the SQLCipher branch. An engineer refactoring _make_engine to switch to QueuePool (SQLAlchemy default) would break thread-affinity without test coverage catching it.
U4. SP15 spec §3.3 says the rotation endpoint has status codes 200, 400, 409, 503. test_api_rotate_key.py covers happy + 409 + 503. The 400 ("encryption not enabled") path is not in the test file. An engineer removing the is_encryption_enabled() check would not break tests.
U5. SP14 spec §3.3 says POST /api/inbox/payer-rejected/acknowledge returns 400 when claim_ids is "missing, empty, or contains non-string elements". test_payer_rejected_acknowledge.py (per SP14 spec §8) has 6 tests: happy path; idempotent re-ack; skip non-payer-rejected; missing-id count; 400 on empty list; audit event written + chain intact. The 400-on-non-string-elements path is not tested.
U6. SP16 spec §3.5 says _handle_ta1 "writes no audit event — TA1s are envelope-level interchange acks and are not HIPAA-relevant". No test asserts that _handle_ta1 writes zero audit rows. A future engineer adding a "ta1.received" audit event would not break tests.
U7. SP16 spec §7.4 says "list_inbound() runs on a thread via asyncio.to_thread so the event loop stays responsive". No test asserts that SftpClient.list_inbound() does not block the event loop (e.g. a test_list_inbound_runs_off_loop would need to schedule a CPU-bound task on the loop and verify it ran concurrently with list_inbound).
U8. SP12 spec §7 says "Plain-SQLite fallback remains silent. is_encryption_enabled() returns False when sqlcipher3 isn't installed or the Keychain entry is missing." test_db_crypto.py covers the case where encryption IS enabled. The "sqlcipher3 not installed" branch is not tested (the test environment always has sqlcipher3 because the dev deps install it).
U9. SP13 spec §3.2 says _connect raises RuntimeError("SftpBlock.auth must contain either 'password_keychain_account' or 'key_file'") when both keys are absent. No test in test_sftp_paramiko.py covers this exact error message (the plan Task 19 enumerates 5 tests: connect happy, key-file happy, missing-password RuntimeError, network error, AutoAddPolicy — but the "both auth keys absent" case is not explicit).
U10. SP11 spec §3.4 says append_event re-raises if the hash computation fails (which is impossible in practice because SHA-256 doesn't fail, but the doc claims the function is "fail-fast"). No test asserts the fail-fast behavior under any failure mode.
U11. SP14 spec §3.8 says "The row payload already carries payer_rejected_acknowledged_at / _actor (always null on the current lane)". test_lane_filter_acknowledged.py covers "row payload carries the ack fields (forward-compat)". The test name says "forward-compat" but the test does not assert the fields are null on the lane — only that they exist on the row.
U12. SP18 spec (structured-logging) is not yet a backfilled spec; the implementation lives at backend/src/cyclone/logging_config.py. test_logging_formatter.py, test_logging_scrubber.py, test_logging_setup.py exist. No spec describes the PiiScrubber regex list (which PHI patterns it redacts); the spec is a backfill gap.
U13. REQUIREMENTS.md §6.1 traceability FR-35 says "5-lane Inbox" was satisfied by SP14; the §6.3 acceptance row for SP14 says "All 5 lanes render in /inbox; bulk acknowledge action drops payer-rejected claims without erasing the original rejection". test_lane_filter_acknowledged.py covers the lane filter and test_payer_rejected_acknowledge.py covers the ack endpoint. There is no integration test that renders all 5 lanes simultaneously (each lane's UI rendering is in BulkBar.test.tsx per the SP14 spec §8 table, but the page-level test for Inbox.tsx with 5 populated lanes is not enumerated).
U14. SP16 spec §3.5 says _handle_ta1 calls cyclone.store.store.add_ta1_ack. The test_api_ta1.py exists and the test_scheduler.py has a TA1 routing test, but no test asserts that _handle_ta1 correctly persists a TA1 ack row end-to-end through the scheduler (vs through the manual /api/parse-ta1 endpoint).
U15. REQUIREMENTS.md §6.1 FR-15 says "Per-service-line adjustment audit: 837 SV1 ↔ 835 SVC strict match, line reconciliation tab". test_line_reconciliation.py covers the pure-function match_service_lines (per SP7 plan Task 7). test_reconcile_line_level.py covers the integration with reconcile.run(). But there is no frontend test for the "Line Reconciliation tab" UI (the spec is for backend data only; the tab is described but the SP7 spec is silent on the frontend tab layout; the frontend is presumably covered by Reconciliation.tsx.test.tsx but SP7 doesn't enumerate it).
U16. REQUIREMENTS.md §6.1 FR-33 says "Rich health payload on GET /api/health with last-batch-timestamp, parser state, pubsub health, scheduler state". test_security.py exists for the get_health_snapshot() function. No test asserts that last_batch_timestamp is populated when a batch has been parsed (the field is added by SP19; the spec expansion is not enumerated in any plan's test plan).
U17. REQUIREMENTS.md §6.1 FR-26 says "paramiko-backed SFTP submit, credentials via keyring". test_sftp_paramiko.py covers the connect / write / list / read happy paths. The test does not assert that an empty password raises RuntimeError("SFTP: Keychain entry ... missing or stub") — it only asserts the runtime error type, not the message. An engineer changing the error message would not break the test.
U18. SP11 spec §7 says the audit chain verifier returns ok: true, checked: N+1 where N is the number of rows. test_audit_log.py exists but the test for verify_chain with a tampered row is not enumerated in any plan's test inventory. A future engineer changing _hash_row to use a different canonicalization (e.g. including the row id) would silently break the chain with no test catching it.
10. Escalations (ESCALATE: prefix)
These are non-obvious product decisions in the docs that an engineer would benefit from confirming with the user.
ESCALATE: SP23 — Ubuntu Docker + RBAC. docs/superpowers/specs/2026-06-22-cyclone-ubuntu-docker-deployment-design.md is marked "Draft / candidate". It introduces (a) a Docker image (Dockerfile + docker-compose.yml), (b) 3-role RBAC (admin / operator / viewer) backed by cyclone.auth, (c) per-role scoping on every endpoint, (d) JWT-based session tokens. This is a fundamental posture change — Cyclone today binds 127.0.0.1 with no auth. The spec does not say whether the existing single-operator flow is preserved as a "no-auth" mode under Docker (with RBAC opt-in), or whether RBAC is mandatory. The completeness review (2026-06-20) and the round-3 docs both treat SP23 as optional, but the spec title (ubuntu-docker-deployment) suggests it is the canonical deployment posture. Confirm: is SP23 in scope for the next release, or is it parked? If parked, what is the deployment story?
ESCALATE: SFTP AutoAddPolicy vs RejectPolicy. SP13 spec §3.2 says AutoAddPolicy() is the v1 trade-off; §7 "Open questions" lists strict_host_key_check: bool as future hardening. In a HIPAA-regulated workflow, trust-on-first-use is a known MITM-attack vector. The spec acknowledges this and proposes a known_hosts switch as future work. Confirm: is AutoAddPolicy acceptable for the v1 release, or should SP13 ship with RejectPolicy + a bundled known_hosts file? The Gainwell MFT host key would need to be obtained and pinned.
ESCALATE: audit_log retention is "≥ 6 years" (HIPAA §164.316(b)(2)) but the docs do not say how. SP11 spec §7 says "pinned audit-log retention for rotation events: db.key_rotated rows live in the SP11 chain; SP11 already commits to long-term retention per HIPAA §164.316(b)(2). No additional retention config is needed." But the migration 0009_audit_log.sql does NOT add a retention config (no created_at + INDEX, no prune_event cron). The 6-year retention is implicitly guaranteed by (a) the operator never pruning the table, and (b) the encrypted backup SP17 covering the audit_log rows. Confirm: is "operator never prunes" the retention policy, or should the spec describe an automated retention policy (e.g. a prune_audit_log cron at 6 years + 1 day)?
ESCALATE: The 5-lane Inbox vs 4-lane state machine. The SP10 spec says payer_rejected_at is a column on claims (not a state). The SP14 spec says the 5th lane is computed by filtering. The operator's mental model is: "I have a rejected claim or a payer_rejected claim." But the code says: a claim is in one state (paid, denied, partial, etc.) AND has a payer_rejected_at timestamp (or doesn't). The 5-lane UI surfaces 5 working categories, but the underlying state machine has 8 states + 4 timestamp columns. Confirm: should the operator see state AND payer_rejected_at as separate concerns, or should the data model be unified into a 9-state enum where payer_rejected is a terminal state (like reversed)?
ESCALATE: SP21 store split is in-flight on refactor/store-split. The spec is dated 2026-06-21; the current backend/src/cyclone/store.py is the monolithic version. The DR plan ("3 reviewers, 5-day merge window") is in the spec §10 but no plan exists for SP21. Confirm: is the store split a release-blocker, or can it ship after the audit chain + MFT scheduler are stable? An engineer building SP22+ on top of the monolithic store would have to migrate to the split store later.
ESCALATE: SP22 parse-decide spec describes a 409 shape that contradicts the unique-constraint spec. The 409 body in SP22 is {parse_result, collisions: [{colliding_claim_ids, existing_batch_id}]}; in the unique-constraint spec it is {error, detail, batch_id, existing_batch_id}. Confirm: which is canonical? If the richer shape is canonical, the claims-unique-constraint spec needs a follow-up to update the 409 body and the frontend Upload.tsx error panel.
ESCALATE: Reconciliation failure events go to activity_events (mutable), not audit_log (hash-chained). The completeness review (2026-06-20 §3.1 item 2) flagged activity_events as not tamper-evident. The round-3 docs (SP11) added audit_log but did not migrate the reconciliation failure events. Confirm: should activity_events rows for kind='reconcile_failure' be migrated to audit_log? Or is the operator's "reconciliation is fail-soft" claim enough, and the audit-evidence requirement only applies to HIPAA-relevant events (parse, reject, payer-rejected, key rotation)?
ESCALATE: ?ack=true on parse-837 returns the 999 inline vs in a separate field. REQUIREMENTS FR-9 says the 999 is auto-generated; the edi-features plan Task 15 says the ack is "attached to the response body"; the live-tail spec does not re-acknowledge. Confirm: should the 999 be returned inline as ack.x12_text (raw X12 string), as ack.body (parsed object), or as a ack_id requiring a follow-up GET /api/acks/{id}? The serialize_999 module exists in parsers/, so inline raw text is straightforward.
ESCALATE: 5-lane Inbox claim ownership. The lane filter for payer_rejected is on the claims table. The 999-rejected lane (rejected) is also on the claims table. The unmatched lane is on claims with no matched remit. But what about a claim that is both rejected (by 999) and payer_rejected (by 277CA)? The SP14 spec §3.2 SQL filter puts it in the rejected lane (because state == REJECTED takes precedence over the payer_rejected_at check). The spec does not say this explicitly. Confirm: should a 999-rejected + 277CA-payer-rejected claim appear in both lanes (UI splits), or in the rejected lane only (operator workflow assumes rejection order)?
ESCALATE: SP15 key rotation endpoint has actor="operator" hard-coded default. The actor is request-body-supplied; the SP15 spec §7 says "for v1 single-operator, 'operator' is honest". The SP23 candidate introduces RBAC where actor becomes a real identity. Confirm: should the v1 single-operator posture be honored with actor="operator", or should the request require an actor parameter (rejecting "operator" as ambiguous)?
ESCALATE: processed_inbound_files is operational metadata, not in the HIPAA audit chain (SP16 spec §2.4). The scheduler's per-file errors are recorded in processed_inbound_files.error_message (mutable, not in audit_log). Confirm: should scheduler errors (e.g. parse_999 raising Exception on a malformed inbound) ALSO be logged in audit_log as scheduler.parse_error? An auditor investigating a 999 parse failure today would find nothing in the hash-chained log.
ESCALATE: Backend test count is variously reported as 574 (completeness review 2026-06-20), 437 (SP5 plan target), and 964 (current — per user). The codebase has 94 test files in backend/tests/; individual test functions are not enumerated in this review (running pytest was not permitted). The traceability matrix in REQUIREMENTS.md §6.1 cites one test file per FR — but a single FR may have 5+ tests in that file. Confirm: is the FR-coverage criterion "≥1 test file" or "≥1 test function"?
11. Confidence
Overall confidence in the doc set: medium-low.
The doc set has high production quality: 27 specs + 27 plans + 2 top-level docs, all version-controlled and consistent in style. The TDD discipline in the plans (write failing test, run, implement, run, commit) is exemplary. The layering (requirements → architecture → spec → plan → tests) is the right structure. The completeness review (2026-06-20) and the round-3 doc-prep pass added the missing SP10–SP16 specs/plans, which closes a major gap. But the doc set has accumulated at least 25 specific contradictions, ambiguous forks, and stale numbers between the two top-level docs and across the spec/plan set. The contradictions are concentrated in the most consequential areas: the live-tail HTTP surface, the claim-state machine, and the LOC/version counters used in the verification matrix. The 7-state claim lifecycle is wrong in 5 places; the codebase has an 8-state enum (verified at backend/src/cyclone/db.py:180-188). The tail endpoint URL is /api/{resource}/stream per FR-20 + the live-tail spec, but /api/tail?since=… per ARCHITECTURE.md §5.4 — two competent engineers reading only the docs could ship two incompatible APIs. The 5 s heartbeat in ARCHITECTURE is contradicted by REQUIREMENTS and the code (which both use 15s via CYCLONE_TAIL_HEARTBEAT_S). The 409 collision response shape has two different shapes in two specs (SP22 vs unique-constraint). The api_routers/ partial split is invisible to the top-level docs. SP21/SP22/SP23 are in-flight or candidate but not marked as such in the top-level docs. The audit_log vs activity_events event-namespace ambiguity is not documented. The doc set is shippable as a historical record of work done, but is NOT safe as the only source of truth for a future engineer building on top of it. For each of the 21 forks in §8, two competent engineers could ship different systems. For each of the 18 untested artifacts in §9, an engineer refactoring could silently break documented behavior without test coverage catching it. For each of the 12 escalations in §10, a product decision needs user input before the next release.
Recommendation: do not ship the doc set as-is. Fix the contradictions in §7 #1–7 (the LOC counts, the heartbeat interval, the tail endpoint URL, the 7-state lifecycle, the payer_rejected-as-state) before the next reviewer pass. Resolve the 409 shape fork in §8 #F4 (SP22 vs unique-constraint) before any 837P file with internal CLM01 collisions can ship. Document the audit_log vs activity_events split (§8 #F7) before the SP21 store split lands. Add tests for the 18 untested artifacts in §9. Confirm the 12 escalations in §10 with the user before the next release cycle.
End of Reviewer A.