13 Commits

Author SHA1 Message Date
Nora 9429d11b5f feat(sp36): extract dashboard router
Move GET /api/dashboard/kpis from api.py to api_routers/dashboard.py. Single-route router; gate via matrix_gate at the router level. Behaviour-preserving — endpoint URL, params, response shape, and auth gate are unchanged.
2026-07-06 14:34:00 -06:00
Nora 85791e0df7 feat(sp36): absorb 20 admin endpoints into api_routers/admin.py
Moves the entire /api/admin/* namespace (audit-log ×2, db/rotate-key ×1,
backup ×10, scheduler ×6, reload-config ×1) out of api.py and into the
existing api_routers/admin.py, which already owned /api/admin/validate-provider.

- api.py: 4294 → 3547 LOC (Δ -747). Removed the orphaned # --- separator
  left behind by the cut, the orphaned 'return {ok,loaded,errors}'
  tail of reload-config, and the now-unused cyclone.clearhouse.InboundFile
  top-level import.
- api_routers/admin.py: 60 → 851 LOC. Added the imports the moved
  routes need (json, logging, threading, time, AuditEvent, verify_chain,
  InboundFile) and stripped the redundant top-level imports of
  symbols that the route bodies reach via the inline _db_crypto /
  _secrets / _backup_svc_mod / _backup_sched_mod / _scheduler_mod
  / _audit aliases (those aliases stay — tests rely on being able to
  monkeypatch cyclone.api_routers.admin._X.method). Hoisted the inline
  'import time' from inside pull_inbound to module scope. Dropped
  the redundant 'import threading as _threading' alias — top-level
  'import threading' already covers it.
- tests/test_api_rotate_key.py: 4 monkeypatch targets migrated from
  cyclone.api._db_crypto / cyclone.api._secrets to
  cyclone.api_routers.admin._db_crypto / cyclone.api_routers.admin._secrets
  to match the new home of the rotate-key endpoint. Lock acquire/release
  also migrated.

The non-admin /api/config/* and /api/payers/{id}/summary routes that
bracketed the moved admin blocks stay in api.py — they're extracted
in Tasks 5 & 6.

Behaviour-preserving: pytest = 20 failed / 1253 passed / 10 skipped
= baseline match. Admin-only subset (audit-log + backup + scheduler +
rotate-key + reload-config) = 34 passed.
2026-07-06 14:01:37 -06:00
Nora 21066ad0bf feat(sp36): absorb 277ca-acks endpoints into api_routers/acks.py
Moves GET /api/277ca-acks and GET /api/277ca-acks/{ack_id}
out of api.py (formerly lines 1278-1318) into the existing
api_routers/acks.py, which already handles 999 ACK list/detail/
stream. 277CA ACKs share the same shape-of-life contract (persisted
by a parse endpoint, listed newest-first, detail returns raw_json)
so the consolidation lands in the router that already owns the
adjacent surface — no new router file.

While here:
- collapsed the inline batch-fetch in list_277ca_acks_endpoint to
  the existing _find_linked_claim_ids_for_acks(ack_ids, kind='277ca')
  helper that the 999 list endpoint already uses (and ta1_acks.py
  uses too). Eliminates the copy-pasted SQL; same N+1-avoidance
  one-SELECT contract.
- pruned ClaimAck / to_ui_two77ca_ack imports from api.py.

Behaviour-preserving: pytest post-cut = 20 failed / 1253 passed /
10 skipped = baseline match. pytest -k '277ca or ack' = 235 passed,
1 skipped.
2026-07-06 13:40:09 -06:00
Nora a963d3ce25 feat(sp36): wire api_routers/__init__.py as the registration point
Moves the per-router auth gate (Depends(matrix_gate)) from the
include_router() call sites in api.py onto each router's own
APIRouter(dependencies=...) declaration. Each router now owns
its own auth dependency.

api.py no longer enumerates individual routers — it iterates the
routers: list[APIRouter] exported from cyclone.api_routers. This
is the registry that 13 future router extractions will append to.

- new: backend/src/cyclone/api_routers/__init__.py (registry)
- new: backend/src/cyclone/api_routers/_shared.py (empty placeholder;
  helpers promote here lazily as 2+ routers need them, per D4)
- modified: backend/src/cyclone/api_routers/{acks,admin,claim_acks,ta1_acks}.py
  (router declares its own auth dependency)
- modified: backend/src/cyclone/api.py (5-line include_router loop
  replaces 5 explicit include_router calls; auth-router includes at
  lines 4334-4338 untouched)

Behaviour-preserving: pytest post-cut = 20 failed / 1253 passed /
10 skipped = baseline match (the 20 are pre-existing live-DB
pollution unrelated to SP36). Health stays public (no matrix_gate).
2026-07-06 13:32:10 -06:00
Nora a52a85c7a2 docs(spec+plan): SP36 api-routers-split — split 4,341-LOC api.py into per-resource routers under api_routers/, 19-task plan with per-step live-test + autoreview + commit cycle 2026-07-06 13:20:12 -06:00
Nora 95f5e91ade merge: SP35 parse-input-guards into main 2026-07-06 13:19:39 -06:00
Nora 3bc5740e8b feat(sp35): Upload page auto-detects 837P vs 835 from file content
Layer A of the SP35 defense-in-depth fix. Before SP35 the dropdown
silently defaulted to '837p' and never changed when a file was dropped
on the page — uploading an 835 file routed it to /api/parse-837 which
(prior to SP35 Task 2) silently persisted an empty batch.

The change:

1. New pure helper src/lib/x12-detect.ts:
   - detectKindFromText(text) reads the first ~4KB and returns the
     DetectedKind ('837p' | '835' | '999' | '277ca' | 'ta1' | 'unknown')
     by matching the ST01 segment (or the bare TA1 segment for the
     no-ST envelope). Cheap substring scan; never invokes tokenize().
   - detectKindFromFile(file) is the File-aware wrapper used by the UI.
   - detectedKindToParsedBatchKind maps the DetectedKind to the kind
     the Upload dropdown supports. Returns null for 999/277CA/TA1 so
     the UI can surface a clean 'this file isn't supported here' hint.

2. Upload.tsx: pickFile is now async and reads the file before storing
   it. If the detected kind differs from the dropdown's current value,
   it switches the dropdown and toasts a hint. If the detected kind is
   999/277CA/TA1 (Upload doesn't ingest those), it shows an error toast.

20 new tests in src/lib/x12-detect.test.ts cover the 6 DetectedKind
paths, the File wrapper, case-insensitivity, garbage input, the
ST*8370 false-positive guard, and the detectedKindToParsedBatchKind
mapping.
2026-07-06 09:58:26 -06:00
Nora b0e06a2dd0 feat(sp35): regression locks on parse-999/277ca/ta1 envelope guards
The 999, 277CA, and TA1 parsers already enforce envelope correctness at
the parser level (parse_999.py line 290 raises 'No AK9 segment found';
parse_277ca.py line 298 raises 'Expected ST*277 or ST*277CA'; parse_ta1.py
line 111 raises 'Expected TA1, got <other>'). These tests lock the HTTP
surface contract: a wrong-kind file POSTed to those endpoints must come
back as 400, never as 200 or 500.

Tests added:
- test_api_999.py: rejects_837_input, rejects_835_input
- test_api_277ca.py: rejects_835_input, rejects_837_input
- test_api_ta1.py: rejects_837_input, rejects_835_input

If a future PR relaxes any of those parser-level guards, the
corresponding regression lock fires immediately.
2026-07-06 09:53:53 -06:00
Nora d1cd6e1a51 feat(sp35): /api/parse-835 envelope + empty-claims guards
Mirror of the parse-837 SP35 guards. Same defense-in-depth shape:
tokenize first, reject anything whose ST01 doesn't start with '835'
(400 'Mismatched file kind'), and after parse refuse to persist a
batch with zero CLP segments (400 'No claims parsed').

Reuses the _transaction_set_id_from_segments helper added by the
parse-837 commit.

New tests in tests/test_api_835.py:
- test_parse_835_endpoint_rejects_837_input (was failing, now green)
- test_parse_835_endpoint_rejects_empty_envelope (was failing, now green)
- test_parse_835_endpoint_happy_path_still_works (regression guard)
2026-07-06 09:52:51 -06:00
Nora f25214189a feat(sp35): /api/parse-837 envelope + empty-claims guards
Server-side defense in depth for misroute ingest. Before SP35, posting
an 835 file (or any other X12 with a parseable ISA envelope) to
/api/parse-837 silently produced a BatchRecord with claims=[] and a
bogus row on the History tab. The 837 parser only required an ISA
envelope; it didn't check the ST transaction-set id.

Two new guards run before persistence:

1. Envelope check: tokenize first, read ST01, reject anything that
   doesn't start with '837'. 400 with error='Mismatched file kind',
   expected='837p', detected_st=<actual>. Catches an 835/999/270/etc
   routed to the wrong endpoint.
2. Empty-claims check: even with the right envelope, if the parser
   produces zero CLM segments, return 400 'No claims parsed' and do
   NOT persist.

New tests in tests/test_api.py:
- test_parse_837_endpoint_rejects_835_input (was failing, now green)
- test_parse_837_endpoint_rejects_empty_envelope (was failing, now green)
- test_parse_837_endpoint_happy_path_still_works (regression guard)

Helper _transaction_set_id_from_segments reused by the 835 mirror.
2026-07-06 09:51:27 -06:00
Nora e4f3d25f3a docs(plan): SP35 parse-input-guards — server guards + UI auto-detect + 999/277CA/TA1 regression locks, TDD-first 2026-07-06 09:45:50 -06:00
Nora 750f560ee0 docs(spec): SP35 parse-input-guards — defense in depth against misroute silent-corruption 2026-07-06 09:45:50 -06:00
Nora 0193ee4c32 merge: SP33 co-txix-payer-fix into main
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-04 20:29:27 -06:00
21 changed files with 3915 additions and 861 deletions
+122 -827
View File
File diff suppressed because it is too large Load Diff
+31 -1
View File
@@ -1 +1,31 @@
"""Resource-group routers. Imported and registered by ``cyclone.api``."""
"""Per-resource FastAPI routers.
`api.py` does `for r in routers: app.include_router(r)`. New
routers register themselves here in alphabetical order. Helpers
shared by 2+ routers live in `_shared.py` (private to the package).
Every router except ``health`` declares its own
``APIRouter(dependencies=[Depends(matrix_gate)])`` — keep that
invariant here so adding a new router doesn't silently miss the gate.
"""
from fastapi import APIRouter
from cyclone.api_routers import (
acks,
admin,
claim_acks,
dashboard,
health,
ta1_acks,
)
routers: list[APIRouter] = [
acks.router, # gated
admin.router, # gated
claim_acks.router, # gated
dashboard.router, # gated
health.router, # public — health probes must work pre-auth
ta1_acks.router, # gated
]
__all__ = ["routers"]
@@ -0,0 +1,9 @@
"""Cross-router helpers for the api_routers package.
Private to the package (leading underscore). Only routers in this
package import from here. Helpers graduate here when at least two
routers need them — single-router helpers stay in the router that
uses them.
Initially empty; helpers promote here as the SP36 refactor progresses.
"""
+52 -8
View File
@@ -1,12 +1,17 @@
"""``/api/acks`` — list, detail, and live-tail stream for 999 ACKs.
"""``/api/acks`` and ``/api/277ca-acks`` — list, detail, and live-tail.
These are the persisted acknowledgment rows produced by
999 ACKs are the persisted acknowledgment rows produced by
``POST /api/parse-999``. The frontend ``useAcks`` hook re-shapes the
list payload to its ``Ack`` interface in ``src/types/index.ts``.
The detail endpoint returns the full ``raw_json`` payload plus the
regenerated ``raw_999_text`` so the UI can show "view source" without a
second round-trip.
277CA ACKs are the persisted Claim Acknowledgment rows produced by
``POST /api/parse-277ca``. The frontend ``useAcks`` hook treats them
as a second source alongside 999 — the row shape is normalised in
``cyclone.store.ui.to_ui_two77ca_ack``.
The 999 detail endpoint returns the full ``raw_json`` payload plus
the regenerated ``raw_999_text`` so the UI can show "view source"
without a second round-trip.
SP25: ``/api/acks/stream`` joins the live-tail triplet — the Acks
page mounts ``useTailStream("acks")`` and ``useMergedTail("acks", …)``
@@ -18,7 +23,7 @@ from __future__ import annotations
import logging
from typing import Any, AsyncIterator
from fastapi import APIRouter, HTTPException, Query, Request
from fastapi import APIRouter, Depends, HTTPException, Query, Request
from fastapi.responses import StreamingResponse
from cyclone import db
@@ -28,12 +33,13 @@ from cyclone.api_helpers import (
tail_events,
wants_ndjson,
)
from cyclone.auth.deps import matrix_gate
from cyclone.parsers.models_999 import ParseResult999
from cyclone.parsers.serialize_999 import serialize_999
from cyclone.pubsub import EventBus
from cyclone.store import store, to_ui_ack
from cyclone.store import store, to_ui_ack, to_ui_two77ca_ack
router = APIRouter()
router = APIRouter(dependencies=[Depends(matrix_gate)])
log = logging.getLogger(__name__)
@@ -177,3 +183,41 @@ def get_ack_endpoint(ack_id: int) -> dict:
else:
body["raw_999_text"] = None
return body
# -- 277CA ACKs -------------------------------------------------------------
# SP36 Task 2: these two endpoints moved here from api.py (lines 1278-1318).
# 277CA ACKs share the same shape-of-life contract as 999 ACKs: persisted by
# a parse endpoint, listed newest-first, detail returns raw_json so the UI
# can show "view source" without a round-trip.
@router.get("/api/277ca-acks")
def list_277ca_acks_endpoint(
limit: int = Query(100, ge=1, le=5000),
) -> Any:
"""Return the list of persisted 277CA ACKs, newest first.
SP28: each item gains ``linked_claim_ids`` (batch-fetched via
the shared ``_find_linked_claim_ids_for_acks`` helper — one
SELECT keyed on ``ack_kind``, no N+1) so the Acks page row
can render the "🔗 N claims" badge inline.
"""
rows = store.list_277ca_acks()
items = [to_ui_two77ca_ack(r) for r in rows[:limit]]
ack_ids = [r.id for r in rows]
linked_map = _find_linked_claim_ids_for_acks(ack_ids, kind="277ca")
for item, aid in zip(items, ack_ids[:limit]):
item["linked_claim_ids"] = linked_map.get(aid, [])
return {"total": len(rows), "items": items}
@router.get("/api/277ca-acks/{ack_id}")
def get_277ca_ack_endpoint(ack_id: int) -> dict:
"""Return one persisted 277CA ACK row with its parsed detail."""
row = store.get_277ca_ack(ack_id)
if row is None:
raise HTTPException(status_code=404, detail=f"277CA ACK {ack_id} not found")
body = to_ui_two77ca_ack(row)
body["raw_json"] = row.raw_json
return body
+779 -11
View File
@@ -1,23 +1,44 @@
"""``/api/admin/validate-provider`` — NPI + Tax ID liveness probe (SP20).
"""``/api/admin/*`` — operator-only endpoints.
Pure read-only endpoint that runs the local NPI Luhn + EIN format checks
without touching the DB. Useful for:
- operators vetting a new provider before adding them to the registry,
- the dashboard's "validate" button on a Provider row,
- smoke-testing the SP20 checks after a deploy.
The /api/admin namespace covers:
Both query params are optional; omitting one just skips that check.
Returns the per-check result dict so the caller can distinguish "bad
format" from "bad checksum".
* **audit-log** (SP11): list + chain-verify the tamper-evident log
* **db/rotate-key**: SQLCipher key rotation (SP12)
* **backup**: create / list / status / verify / restore / prune / scheduler (SP15)
* **scheduler**: backup & main scheduler control + processed-files log
* **reload-config**: hot-reload ``config/payers.yaml`` after edits
* **validate-provider**: NPI + Tax ID liveness probe (SP20)
All routes on this router are gated by ``matrix_gate`` — declared
once on the ``APIRouter`` constructor. ``/api/admin/validate-provider``
lives here too because it's an admin-shaped read-only probe.
SP36 Task 3: the 20 admin endpoints formerly in ``cyclone.api``
(audit-log ×2, db/rotate-key ×1, backup ×10, scheduler ×6,
reload-config ×1) moved here from ``api.py:3321-4052`` and
``api.py:4269-4276``. The non-admin ``/api/config/*`` and
``/api/payers/{id}/summary`` routes that bracketed those blocks
stay in ``api.py`` for now — they're extracted in Tasks 5 & 6.
"""
from __future__ import annotations
from fastapi import APIRouter, Query
import json
import logging
import threading
import time
from typing import Any
from fastapi import APIRouter, Depends, HTTPException, Query
from cyclone import db
from cyclone.audit_log import verify_chain
from cyclone.auth.deps import matrix_gate
from cyclone.clearhouse import InboundFile
from cyclone.npi import is_valid_npi, is_valid_tax_id, normalize_tax_id
router = APIRouter(dependencies=[Depends(matrix_gate)])
router = APIRouter()
log = logging.getLogger(__name__)
@router.get("/api/admin/validate-provider")
@@ -58,3 +79,750 @@ def validate_provider(
}
return result
# --------------------------------------------------------------------------- #
# SP36 Task 3: the 20 admin endpoints below were moved here from api.py. #
# --------------------------------------------------------------------------- #
# --------------------------------------------------------------------------- #
# SP11: tamper-evident audit log (admin)
# --------------------------------------------------------------------------- #
@router.get("/api/admin/audit-log")
def list_audit_log_endpoint(
entity_type: str | None = Query(default=None),
entity_id: str | None = Query(default=None),
event_type: str | None = Query(default=None),
limit: int = Query(default=100, ge=1, le=1000),
) -> Any:
"""List audit-log rows, newest first, with optional filters.
Filters match the (entity_type, entity_id) pair (typical use:
"show me everything that happened to claim C-123") or a single
event_type (typical use: "show me all clearhouse.submitted
events today").
"""
with db.SessionLocal()() as s:
q = s.query(db.AuditLog)
if entity_type:
q = q.filter(db.AuditLog.entity_type == entity_type)
if entity_id:
q = q.filter(db.AuditLog.entity_id == entity_id)
if event_type:
q = q.filter(db.AuditLog.event_type == event_type)
rows = q.order_by(db.AuditLog.id.desc()).limit(limit).all()
return {
"total": len(rows),
"items": [
{
"id": r.id,
"event_type": r.event_type,
"entity_type": r.entity_type,
"entity_id": r.entity_id,
"actor": r.actor,
"payload": json.loads(r.payload_json) if r.payload_json else None,
"created_at": r.created_at.isoformat() if r.created_at else None,
"prev_hash": r.prev_hash,
"hash": r.hash,
}
for r in rows
],
}
@router.get("/api/admin/audit-log/verify")
def verify_audit_log_endpoint() -> Any:
"""Walk the audit-log chain and verify every row's hash.
Returns ``{"ok": true, "checked": N}`` for a clean chain, or
``{"ok": false, "checked": K, "first_bad_id": X, "reason": "..."}``
for a broken chain. This is the operator's "did anyone tamper?"
endpoint; run it on demand or via a nightly cron job.
"""
with db.SessionLocal()() as s:
result = verify_chain(s)
return {
"ok": result.ok,
"checked": result.checked,
"first_bad_id": result.first_bad_id,
"reason": result.reason,
}
# ---------------------------------------------------------------------------
# SP15: SQLCipher key rotation
#
# Re-encrypts the DB in place with a fresh key, then updates the
# Keychain so subsequent connections open with the new key. This is
# a 1-time operation per rotation; for routine read/write the rest
# of the API is unchanged.
#
# Concurrency: the rotation holds a module-level lock so two
# concurrent requests can't race and end up with mismatched Keychain
# + DB. The lock is a simple threading.Lock; a process restart
# resets it (intentional — the operator's next start-up opens with
# whatever key is in the Keychain).
# ---------------------------------------------------------------------------
from cyclone import db_crypto as _db_crypto
from cyclone import secrets as _secrets
_db_rotate_lock = threading.Lock()
@router.post("/api/admin/db/rotate-key")
def rotate_db_key_endpoint(body: dict | None = None) -> Any:
"""Generate a fresh DB key, re-encrypt the DB, update the Keychain.
Request body (optional):
actor: who initiated the rotation. Defaults to "operator".
reason: human-readable reason. Written to the audit log.
Returns:
``{ok, old_fingerprint, new_fingerprint, rotated_at, table_count}``
on success. On failure (DB not encrypted, rekey failed,
Keychain update failed) returns the same shape with
``ok=false`` and a ``reason``. HTTP 503 is returned if the
rekey fails or encryption is not enabled.
The Keychain write happens *after* the rekey succeeds. If the
Keychain write fails, the DB has the new key but the Keychain
still has the old one — the endpoint returns 503 with a
"keychain update failed" reason and the operator must restore
the old key manually (``cyclone db restore-key <old_key>``) to
avoid being locked out.
"""
body = body or {}
actor = body.get("actor") or "operator"
reason = body.get("reason") or ""
if not _db_crypto.is_encryption_enabled():
raise HTTPException(
status_code=400,
detail="encryption not enabled (sqlcipher3 missing or no Keychain key)",
)
# Acquire the lock; non-blocking so a stuck rotation doesn't
# silently hold up other requests.
if not _db_rotate_lock.acquire(blocking=False):
raise HTTPException(
status_code=409,
detail="another key rotation is in progress",
)
try:
url = db._resolve_url()
old_key = _db_crypto.get_db_key()
if not old_key:
raise HTTPException(
status_code=400,
detail="no DB key in Keychain; cannot rotate",
)
new_key = _db_crypto.generate_db_key()
result = _db_crypto.rotate_db_key(
url=url, old_key=old_key, new_key=new_key,
)
if not result.ok:
# Rekey failed. The DB still has the old key. The
# Keychain is unchanged. Caller should NOT retry with
# the same new key (it's lost); generate a fresh one.
log.error("SQLCipher rotate failed: %s", result.reason)
raise HTTPException(
status_code=503,
detail={
"ok": False,
"old_fingerprint": result.old_fingerprint,
"new_fingerprint": result.new_fingerprint,
"rotated_at": result.rotated_at,
"reason": result.reason,
},
)
# Rekey succeeded. Now update the Keychain. If this fails
# the DB is locked behind the new key — operator must
# restore the old key manually.
if not _secrets.set_secret(_db_crypto.KEYCHAIN_ACCOUNT, new_key):
log.error("Keychain update failed after successful rekey!")
raise HTTPException(
status_code=503,
detail={
"ok": False,
"old_fingerprint": result.old_fingerprint,
"new_fingerprint": result.new_fingerprint,
"rotated_at": result.rotated_at,
"reason": (
"rekey succeeded but Keychain update failed — "
"the DB is now encrypted with the new key but "
"the Keychain still has the old one. "
"Restore the old key to the Keychain to recover."
),
},
)
# Store the old key in the "previous" account for a grace
# period so the operator can roll back if they discover the
# new key is broken (e.g. the Keychain entry got truncated).
_secrets.set_secret(_db_crypto.KEYCHAIN_ACCOUNT_PREVIOUS, old_key)
# Rebuild the engine so subsequent connections use the new
# key. dispose_engine() closes every pooled connection that
# was using the old key; init_db() opens new ones with the
# new key from the (now-updated) Keychain.
db.reinit_engine()
# Audit log the rotation. We do this after the engine is
# rebuilt so the audit event is written with the new key —
# proving that the new key works for new writes.
try:
from cyclone.audit_log import append_event, AuditEvent
with db.SessionLocal()() as s:
append_event(s, AuditEvent(
event_type="db.key_rotated",
entity_type="database",
entity_id="cyclone.db",
actor=actor,
payload={
"old_fingerprint": result.old_fingerprint,
"new_fingerprint": result.new_fingerprint,
"table_count": result.table_count,
"reason": reason,
},
))
s.commit()
except Exception as exc: # noqa: BLE001
# Audit append is best-effort; rotation already succeeded.
log.warning("could not write audit event for rotation: %s", exc)
return {
"ok": True,
"old_fingerprint": result.old_fingerprint,
"new_fingerprint": result.new_fingerprint,
"rotated_at": result.rotated_at,
"table_count": result.table_count,
}
finally:
_db_rotate_lock.release()
# ---------------------------------------------------------------------------
# SP17: encrypted DB backups (admin)
#
# The actual encryption + lifecycle lives in :mod:`cyclone.backup` and
# :mod:`cyclone.backup_service`. The scheduler (separate from the
# MFT scheduler) lives in :mod:`cyclone.backup_scheduler`. These
# endpoints expose the operator's manual controls plus a tick for
# "take a backup right now."
#
# Restore is intentionally two-step: an idle browser tab can't nuke
# the live DB. The first call returns a ``restore_token`` (a one-shot
# 64-char hex) and a preview of the backup's fingerprint + table
# count plus the live DB's. The second call with the token performs
# the actual swap.
# ---------------------------------------------------------------------------
from cyclone import backup_service as _backup_svc_mod
from cyclone import backup_scheduler as _backup_sched_mod
def _backup_or_503():
try:
return _backup_svc_mod.get_backup_service()
except RuntimeError as exc:
raise HTTPException(status_code=503, detail=str(exc))
@router.post("/api/admin/backup/create")
def backup_create() -> Any:
"""Take an encrypted backup right now. Returns the new backup metadata."""
from cyclone import audit_log as _audit
svc = _backup_or_503()
try:
result = svc.create_now()
except Exception as exc: # noqa: BLE001
# Surface a 503 with the reason so the operator sees what
# went wrong without grepping server logs.
raise HTTPException(
status_code=503,
detail=f"backup failed: {type(exc).__name__}: {exc}",
)
# Audit the create. Best-effort; failure here doesn't roll back
# the backup (already on disk).
try:
with db.SessionLocal()() as s:
_audit.append_event(s, _audit.AuditEvent(
event_type="db.backup_created",
entity_type="database",
entity_id="cyclone.db",
actor="operator",
payload={
"backup_id": result.backup.id,
"db_fingerprint": result.backup.db_fingerprint,
"table_count": result.backup.table_count,
"triggered_by": "api",
},
))
s.commit()
except Exception as exc: # noqa: BLE001
log.warning("could not write backup_created audit event: %s", exc)
return {
"ok": True,
"backup": {
"id": result.backup.id,
"filename": result.backup.filename,
"size_bytes": result.backup.size_bytes,
"db_fingerprint": result.backup.db_fingerprint,
"table_count": result.backup.table_count,
"created_at": result.backup.created_at.isoformat(),
"key_fingerprint": result.backup.key_fingerprint,
},
"sidecar": {
"format_version": result.sidecar.format_version,
"kdf": result.sidecar.kdf,
"kdf_iterations": result.sidecar.kdf_iterations,
"cipher": result.sidecar.cipher,
},
}
@router.get("/api/admin/backup/list")
def backup_list(
limit: int = Query(default=100, ge=1, le=1000),
status: str | None = Query(default=None),
) -> Any:
"""List ``db_backups`` rows, newest first. Filters by status."""
svc = _backup_or_503()
rows = svc.list_backups(limit=limit, status=status)
return {
"count": len(rows),
"files": [
{
"id": r.id,
"filename": r.filename,
"backup_dir": r.backup_dir,
"size_bytes": r.size_bytes,
"db_fingerprint": r.db_fingerprint,
"table_count": r.table_count,
"created_at": r.created_at.isoformat() if r.created_at else None,
"completed_at": r.completed_at.isoformat() if r.completed_at else None,
"status": r.status,
"error_message": r.error_message,
"key_fingerprint": r.key_fingerprint,
}
for r in rows
],
}
@router.get("/api/admin/backup/status")
def backup_status() -> Any:
"""Snapshot of the backup subsystem (counts, disk usage, last run)."""
svc = _backup_or_503()
snap = svc.status()
# Also include the BackupScheduler's snapshot if configured.
try:
sched = _backup_sched_mod.get_backup_scheduler()
snap["scheduler"] = sched.status().as_dict()
except RuntimeError:
snap["scheduler"] = None
return snap
@router.post("/api/admin/backup/{backup_id}/verify")
def backup_verify(backup_id: int) -> Any:
"""Decrypt + checksum-verify a backup against its sidecar."""
svc = _backup_or_503()
try:
v = svc.verify(backup_id)
except _backup_svc_mod.BackupError as exc:
raise HTTPException(status_code=404, detail=str(exc))
return {
"backup_id": v.backup_id,
"filename": v.filename,
"ok": v.ok,
"expected_fingerprint": v.expected_fingerprint,
"actual_fingerprint": v.actual_fingerprint,
"table_count": v.table_count,
"reason": v.reason,
}
@router.post("/api/admin/backup/{backup_id}/restore/initiate")
def backup_restore_initiate(backup_id: int) -> Any:
"""First step of the two-step restore. Returns a ``restore_token``."""
svc = _backup_or_503()
try:
init = svc.restore_initiate(backup_id)
except _backup_svc_mod.BackupError as exc:
raise HTTPException(status_code=400, detail=str(exc))
return {
"backup_id": init.backup_id,
"filename": init.filename,
"size_bytes": init.size_bytes,
"restore_token": init.restore_token,
"expires_at": init.expires_at.isoformat(),
"preview": {
"backup_db_fingerprint": init.db_fingerprint,
"backup_table_count": init.table_count,
"current_db_fingerprint": init.current_db_fingerprint,
"current_table_count": init.current_table_count,
},
"warning": (
"Confirming will dispose the live engine and replace the DB "
"file with the backup. In-flight requests will error. "
"Re-issue the call with the restore_token within 5 minutes."
),
}
@router.post("/api/admin/backup/{backup_id}/restore/confirm")
def backup_restore_confirm(
backup_id: int,
body: dict | None = None,
) -> Any:
"""Second step of the two-step restore. Performs the swap."""
body = body or {}
token = body.get("restore_token")
if not token or not isinstance(token, str):
raise HTTPException(
status_code=400,
detail="missing or invalid restore_token in request body",
)
actor = body.get("actor") or "operator"
svc = _backup_or_503()
try:
result = svc.restore_confirm(backup_id, token, actor=actor)
except _backup_svc_mod.BackupError as exc:
raise HTTPException(status_code=400, detail=str(exc))
# Audit the restore. Best-effort.
try:
from cyclone import audit_log as _audit
with db.SessionLocal()() as s:
_audit.append_event(s, _audit.AuditEvent(
event_type="db.backup_restored",
entity_type="database",
entity_id="cyclone.db",
actor=actor,
payload={
"backup_id": result.backup_id,
"filename": result.filename,
"restored_from_fingerprint": result.restored_from_fingerprint,
"new_db_fingerprint": result.new_db_fingerprint,
"restored_at": result.restored_at.isoformat(),
},
))
s.commit()
except Exception as exc: # noqa: BLE001
log.warning("could not write backup_restored audit event: %s", exc)
return {
"ok": True,
"backup_id": result.backup_id,
"filename": result.filename,
"restored_from_fingerprint": result.restored_from_fingerprint,
"restored_at": result.restored_at.isoformat(),
"new_db_fingerprint": result.new_db_fingerprint,
}
@router.post("/api/admin/backup/prune")
def backup_prune() -> Any:
"""Apply the retention policy now. Returns the deleted paths."""
from cyclone import audit_log as _audit
svc = _backup_or_503()
deleted = svc.prune()
actor = "operator"
if deleted:
try:
with db.SessionLocal()() as s:
_audit.append_event(s, _audit.AuditEvent(
event_type="db.backup_pruned",
entity_type="database",
entity_id="cyclone.db",
actor=actor,
payload={"deleted_paths": deleted},
))
s.commit()
except Exception as exc: # noqa: BLE001
log.warning("could not write backup_pruned audit event: %s", exc)
return {"ok": True, "deleted_count": len(deleted), "deleted_paths": deleted}
# ---------------------------------------------------------------------------
# SP17: backup scheduler (admin)
# ---------------------------------------------------------------------------
@router.post("/api/admin/backup/scheduler/start")
async def backup_scheduler_start() -> Any:
"""Begin the backup scheduler loop."""
try:
sched = _backup_sched_mod.get_backup_scheduler()
except RuntimeError as exc:
raise HTTPException(status_code=503, detail=str(exc))
await sched.start()
return {"status": sched.status().as_dict()}
@router.post("/api/admin/backup/scheduler/stop")
async def backup_scheduler_stop() -> Any:
"""Stop the backup scheduler loop."""
try:
sched = _backup_sched_mod.get_backup_scheduler()
except RuntimeError as exc:
raise HTTPException(status_code=503, detail=str(exc))
await sched.stop()
return {"status": sched.status().as_dict()}
@router.post("/api/admin/backup/scheduler/tick")
async def backup_scheduler_tick() -> Any:
"""Run one backup tick now (create + prune + audit)."""
try:
sched = _backup_sched_mod.get_backup_scheduler()
except RuntimeError as exc:
raise HTTPException(status_code=503, detail=str(exc))
result = await sched.tick()
return {"ok": result.ok, "tick": result.as_dict()}
# ---------------------------------------------------------------------------
# SP16: live MFT polling scheduler (admin)
#
# The scheduler lives in :mod:`cyclone.scheduler` and is configured by
# the lifespan handler. The endpoints below expose start / stop /
# one-shot tick / status / history so an operator (or a cron job)
# can drive the scheduler without touching the DB.
#
# Note: the scheduler is OFF by default. Auto-start is opt-in via
# ``CYCLONE_SCHEDULER_AUTOSTART=true`` at launch. These endpoints
# are the operator's manual controls.
# ---------------------------------------------------------------------------
from cyclone import scheduler as _scheduler_mod
def _scheduler_or_503():
"""Return the configured scheduler or raise 503."""
try:
return _scheduler_mod.get_scheduler()
except RuntimeError as exc:
raise HTTPException(status_code=503, detail=str(exc))
@router.post("/api/admin/scheduler/start")
async def scheduler_start() -> Any:
"""Begin polling the MFT inbound path every poll_interval_seconds."""
sched = _scheduler_or_503()
await sched.start()
return {"status": sched.status().as_dict()}
@router.post("/api/admin/scheduler/stop")
async def scheduler_stop() -> Any:
"""Stop polling. Waits up to 30s for the current tick to finish."""
sched = _scheduler_or_503()
await sched.stop()
return {"status": sched.status().as_dict()}
@router.post("/api/admin/scheduler/tick")
async def scheduler_tick() -> Any:
"""Run a single poll cycle synchronously and return the result.
Useful for: forcing a poll without waiting for the next interval;
verifying SFTP connectivity; running a one-shot import from the
CLI (``curl -X POST .../api/admin/scheduler/tick``).
"""
sched = _scheduler_or_503()
result = await sched.tick()
return {"ok": True, "tick": result.as_dict()}
@router.post("/api/admin/scheduler/pull-inbound")
async def scheduler_pull_inbound(
date: str = Query(
..., pattern=r"^\d{8}$",
description="Date filter as YYYYMMDD; only filenames whose 8-digit "
"timestamp (the 9th positional group in the inbound "
"filename) matches are downloaded and processed.",
),
file_types: str | None = Query(
default=None,
description="Optional comma-separated whitelist of file_types "
"(999, TA1, 277, 277CA, 835). Defaults to 999+TA1.",
),
limit: int = Query(default=2000, ge=1, le=10000),
) -> Any:
"""Targeted pull: list, filter to a date, download, and process.
Bypasses the alphabetical full-listing pass. Workflow:
1. ``SftpClient.list_inbound_names()`` — sub-second metadata-only
listing of the inbound MFT dir (skips ``*_warn.txt``).
2. Client-side filter: keep files whose 8-digit timestamp
substring equals ``date`` and whose ``file_type`` is in the
allowlist.
3. ``SftpClient.download_inbound(f)`` for each — fetches bytes
into the local cache.
4. ``Scheduler.process_inbound_files(files)`` — runs the same
per-file pipeline as a regular tick (already-processed files
are deduped via ``processed_inbound_files``).
Use this for the daily "process today's 999s" workflow without
paying the cost of downloading the full inbound set.
Returns ``{"ok": True, "summary": {...}}`` with
``listed / matched / downloaded / processed / skipped / errored``
counters and the date / file_type filters applied.
"""
from cyclone.clearhouse import SftpClient
from cyclone.edi.filenames import (
ALLOWED_FILE_TYPES,
parse_inbound_filename,
)
from cyclone.providers import SftpBlock
sched = _scheduler_or_503()
block: SftpBlock = sched._sftp_block # noqa: SLF001 — internal but stable
client = SftpClient(block)
if file_types:
wanted = {t.strip().upper() for t in file_types.split(",") if t.strip()}
unknown = wanted - ALLOWED_FILE_TYPES
if unknown:
raise HTTPException(
status_code=400,
detail=f"file_types {sorted(unknown)!r} not in "
f"{sorted(ALLOWED_FILE_TYPES)}",
)
else:
wanted = {"999", "TA1"} # daily default — what the operator needs
started = time.monotonic()
try:
# Single SFTP listdir — fast, no download.
all_files = await asyncio.to_thread(client.list_inbound_names)
except Exception as exc:
log.exception("SFTP list_inbound_names failed")
raise HTTPException(
status_code=502,
detail=f"SFTP list failed: {type(exc).__name__}: {exc}",
) from exc
listed = len(all_files)
matched: list[InboundFile] = []
for f in all_files:
if f.name.find(date) == -1:
continue
try:
parsed = parse_inbound_filename(f.name)
except ValueError:
continue
if parsed.file_type not in wanted:
continue
matched.append(f)
if len(matched) >= limit:
break
# Download in parallel-ish via to_thread (SftpClient serializes per
# connection; the overhead is dominated by the SFTP round trip).
downloaded = 0
download_errors: list[str] = []
for f in matched:
try:
await asyncio.to_thread(client.download_inbound, f)
downloaded += 1
except Exception as exc: # noqa: BLE001
log.warning("Failed to download %s: %s", f.name, exc)
download_errors.append(f"{f.name}: {type(exc).__name__}: {exc}")
# Hand off to the scheduler pipeline (idempotent; dedupes via
# processed_inbound_files).
tick = await sched.process_inbound_files(matched)
duration = round(time.monotonic() - started, 3)
return {
"ok": True,
"summary": {
"date": date,
"file_types": sorted(wanted),
"limit": limit,
"listed": listed,
"matched": len(matched),
"downloaded": downloaded,
"download_errors": download_errors,
"processed": tick.files_processed,
"skipped": tick.files_skipped,
"errored": tick.files_errored,
"duration_s": duration,
},
"tick": tick.as_dict(),
}
@router.get("/api/admin/scheduler/status")
def scheduler_status() -> Any:
"""Return the scheduler's runtime snapshot (running, counters, last tick)."""
sched = _scheduler_or_503()
return sched.status().as_dict()
@router.get("/api/admin/scheduler/processed-files")
def scheduler_processed_files(
limit: int = Query(default=100, ge=1, le=1000),
status: str | None = Query(default=None),
) -> Any:
"""List rows from ``processed_inbound_files``, newest first.
The operator's "what did the scheduler do?" view. Filters by
``status`` (``ok`` / ``error`` / ``skipped`` / ``pending``).
Returns ``{"count": N, "files": [...]}`` where ``files[i]``
matches the ORM row as a JSON dict.
"""
from cyclone.db import ProcessedInboundFile
from cyclone.scheduler import STATUS_OK, STATUS_ERROR, STATUS_SKIPPED, STATUS_PENDING
valid_statuses = {STATUS_OK, STATUS_ERROR, STATUS_SKIPPED, STATUS_PENDING}
if status is not None and status not in valid_statuses:
raise HTTPException(
status_code=400,
detail=f"status must be one of {sorted(valid_statuses)}",
)
with db.SessionLocal()() as s:
q = s.query(db.ProcessedInboundFile)
if status is not None:
q = q.filter(db.ProcessedInboundFile.status == status)
rows = q.order_by(db.ProcessedInboundFile.id.desc()).limit(limit).all()
return {
"count": len(rows),
"files": [
{
"id": r.id,
"sftp_block_name": r.sftp_block_name,
"name": r.name,
"size": r.size,
"modified_at": r.modified_at.isoformat() if r.modified_at else None,
"file_type": r.file_type,
"processed_at": r.processed_at.isoformat() if r.processed_at else None,
"parser_used": r.parser_used,
"claim_count": r.claim_count,
"status": r.status,
"error_message": r.error_message,
}
for r in rows
],
}
@router.post("/api/admin/reload-config")
def reload_config():
"""Re-read ``config/payers.yaml`` and revalidate. Returns counts."""
from cyclone import payers as payer_loader
try:
configs = payer_loader.load_payer_configs()
except ValueError as e:
raise HTTPException(status_code=400, detail=str(e))
return {"ok": True, "loaded": len(configs), "errors": []}
@@ -21,16 +21,17 @@ from __future__ import annotations
import logging
from typing import Any, AsyncIterator, Literal
from fastapi import APIRouter, HTTPException, Query, Request
from fastapi import APIRouter, Depends, HTTPException, Query, Request
from fastapi.responses import JSONResponse, StreamingResponse
from pydantic import BaseModel, Field
from cyclone import db
from cyclone.api_helpers import ndjson_line, tail_events
from cyclone.auth.deps import matrix_gate
from cyclone.pubsub import EventBus
from cyclone.store import store, to_ui_claim_ack
router = APIRouter()
router = APIRouter(dependencies=[Depends(matrix_gate)])
log = logging.getLogger(__name__)
@@ -0,0 +1,39 @@
"""``/api/dashboard/kpis`` — server-aggregated Dashboard tiles.
Backs the Dashboard's "Claims / Billed / Received / Pending AR /
Denial rate" tiles + the monthly sparkline series + the
top-providers and top-denials lists.
Why this exists instead of ``GET /api/claims?limit=N``:
The Dashboard's KPIs are aggregates over *every* claim — billed,
received, denial rate, pending count, monthly billed/received. With
60k+ claims in production, paginating ``/api/claims`` and reducing
client-side silently produces wrong numbers (denial rate sampled,
billed summed from the first 100 rows). This endpoint does the
aggregation server-side in a single read so the Dashboard's numbers
are always correct regardless of dataset size.
SP36 Task 4: this single endpoint moved here from ``api.py:2732``.
"""
from __future__ import annotations
from fastapi import APIRouter, Depends, Query
from cyclone.auth.deps import matrix_gate
from cyclone.store import dashboard_kpis
router = APIRouter(dependencies=[Depends(matrix_gate)])
@router.get("/api/dashboard/kpis")
def get_dashboard_kpis(
months: int = Query(6, ge=1, le=24),
top_n_providers: int = Query(4, ge=0, le=50),
top_n_denials: int = Query(5, ge=0, le=50),
) -> dict:
"""Server-aggregated Dashboard KPIs over the whole claim population."""
return dashboard_kpis(
months=months,
top_n_providers=top_n_providers,
top_n_denials=top_n_denials,
)
+3 -2
View File
@@ -16,15 +16,16 @@ from __future__ import annotations
from typing import Any, AsyncIterator
from fastapi import APIRouter, HTTPException, Query, Request
from fastapi import APIRouter, Depends, HTTPException, Query, Request
from fastapi.responses import StreamingResponse
from cyclone.api_helpers import ndjson_line, tail_events
from cyclone.auth.deps import matrix_gate
from cyclone import db
from cyclone.pubsub import EventBus
from cyclone.store import store, to_ui_ta1_ack
router = APIRouter()
router = APIRouter(dependencies=[Depends(matrix_gate)])
# SP25: ``_ta1_to_ui`` moved to ``cyclone.store.ui.to_ui_ta1_ack`` so
+122
View File
@@ -18,6 +18,7 @@ from cyclone import __version__
FIXTURE = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
FIXTURE_835 = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
@pytest.fixture
@@ -216,3 +217,124 @@ def test_cors_extra_origins_via_env(client: TestClient, monkeypatch):
# Reload once more so the module-level allow-list returns to its
# default for any test that imports `cyclone.api` after this one.
importlib.reload(api_module)
# --------------------------------------------------------------------------- #
# SP35: parse-837 input guards (defense in depth against misroute ingest)
# --------------------------------------------------------------------------- #
#
# Before SP35, /api/parse-837 silently accepted any file that had a parseable
# ISA envelope. An 835 file dropped on the Upload page while the dropdown
# still said "837p" would land in the DB as an empty batch (claims=[]) and a
# bogus row on the History tab. SP35 fixes that at three layers:
#
# 1. Server envelope check: ST*837 (or ST*837P) required, else 400 with
# error="Mismatched file kind".
# 2. Server empty-claims check: even with the right envelope, if zero CLM
# segments were parsed, return 400 with error="No claims parsed"
# and DO NOT persist the batch.
# 3. UI auto-detect (separate file: src/pages/Upload.test.tsx).
#
# These tests are the server-layer regression locks. They run against the
# TestClient and use the existing fixtures. The 835 fixture has an ST*835
# envelope; posting it to /api/parse-837 must surface a 400 and must not
# create a BatchRecord.
def test_parse_837_endpoint_rejects_835_input(client: TestClient):
"""Uploading an 835 file to /api/parse-837 must fail loudly, not persist.
Repro for the original bug: user drops an 835 file on the Upload page
while the kind dropdown still says "837p" (Upload.tsx default). Before
SP35 the endpoint accepted it, ran it through the 837 parser (which
found zero CLM segments because the file has none), and persisted a
claims=[] batch — a bogus row on the History tab and on
/api/batches. SP35 closes the door at the server so the UI bug becomes
cosmetic instead of data-corrupting.
"""
text_835 = FIXTURE_835.read_text()
# Sanity check: the fixture really is an 835 file. If this ever flips,
# the test would still pass for the wrong reason.
assert "ST*835" in text_835, "fixture is no longer ST*835 — update SP35 tests"
# Snapshot the batch count BEFORE the bad upload so we can assert the
# request did NOT persist anything. Using the public /api/batches JSON
# endpoint (already exercised by the Dashboard).
before = client.get("/api/batches", headers={"Accept": "application/json"}).json()
total_before = before.get("total", len(before.get("items", [])))
resp = client.post(
"/api/parse-837",
files={"file": ("co_medicaid_835.txt", text_835, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert body["error"] == "Mismatched file kind"
assert body["expected"] == "837p"
assert body["detected_st"].startswith("835")
after = client.get("/api/batches", headers={"Accept": "application/json"}).json()
total_after = after.get("total", len(after.get("items", [])))
assert total_after == total_before, "Server persisted a batch from a 835 file"
def test_parse_837_endpoint_rejects_empty_envelope(client: TestClient):
"""Right envelope (ST*837), zero CLM segments → 400 No claims parsed.
Synthetic input: a complete ISA/GS/ST envelope with a BHT, a closing
SE/GE/IEA, and no CLM loops. The 837 parser will tokenize and build
the envelope cleanly, then return claims=[]. SP35 must surface this as
a 400 with error="No claims parsed" and must not persist a batch.
"""
# Bare 837 envelope — no HL/CLM loops. A real X12 file with ST*837
# but no claims is unusual but possible (e.g. a header-only test file
# or a truncated/cancelled run). The right behavior is to reject,
# not to silently persist an empty batch.
synthetic = (
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER "
"*260617*1937*^*00501*000000001*0*P*:~"
"GS*HC*SENDER*RECEIVER*20260706*1937*1*X*005010X222A1~"
"ST*837*0001*005010X222A1~"
"BHT*0019*00*0001*20260706*1937*CH~"
"SE*2*0001~"
"GE*1*1~"
"IEA*1*000000001~"
)
assert "ST*837" in synthetic
before = client.get("/api/batches", headers={"Accept": "application/json"}).json()
total_before = before.get("total", len(before.get("items", [])))
resp = client.post(
"/api/parse-837",
files={"file": ("empty_837.txt", synthetic, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert body["error"] == "No claims parsed"
after = client.get("/api/batches", headers={"Accept": "application/json"}).json()
total_after = after.get("total", len(after.get("items", [])))
assert total_after == total_before, "Server persisted an empty-claims batch"
def test_parse_837_endpoint_happy_path_still_works(client: TestClient):
"""Regression guard: real 837 fixture must still parse → 200 with claims.
Sits next to the new SP35 rejection tests so any future tightening of
the guards that accidentally blocks the happy path fails here loudly.
"""
text = FIXTURE.read_text()
resp = client.post(
"/api/parse-837",
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 200, resp.text
body = resp.json()
assert body.get("summary", {}).get("total_claims", 0) >= 1
assert "batch_id" in body and len(body["batch_id"]) == 32
+45
View File
@@ -167,3 +167,48 @@ class TestInboxPayerRejectedLane:
# The rejected lane (999 envelope) must be empty — we haven't
# uploaded a 999, so this claim isn't there.
assert "c1" not in [c["id"] for c in lanes["rejected"]]
# --------------------------------------------------------------------------- #
# SP35: parse-277ca envelope regression lock
# --------------------------------------------------------------------------- #
#
# The 277CA parser already raises CycloneParseError("Expected ST*277 or
# ST*277CA, got ST*<other>") when fed a file with the wrong ST envelope
# (parse_277ca.py line 298). This regression lock confirms the HTTP
# surface converts that error into a 400 (never 200, never 500).
def test_parse_277ca_endpoint_rejects_835_input(client: TestClient):
"""Posting an 835 file to /api/parse-277ca must surface 400, not 200."""
wrong_kind = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
text = wrong_kind.read_text()
assert "ST*835" in text # sanity check on the fixture
resp = client.post(
"/api/parse-277ca",
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert "error" in body
# The parser-level message must survive (mentions "ST" and the
# expected vs actual set id).
detail = body.get("detail", "")
assert "ST" in detail and ("277" in detail or body["error"] == "Parse error"), body
def test_parse_277ca_endpoint_rejects_837_input(client: TestClient):
"""Posting an 837P file to /api/parse-277ca must surface 400, not 200."""
wrong_kind = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
text = wrong_kind.read_text()
assert "ST*837" in text # sanity check
resp = client.post(
"/api/parse-277ca",
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
assert "error" in resp.json()
+115
View File
@@ -13,6 +13,7 @@ from cyclone.store import store as global_store
FIXTURE = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
UNBALANCED = Path(__file__).parent / "fixtures" / "unbalanced_835.txt"
FIXTURE_837P = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
@pytest.fixture
@@ -337,3 +338,117 @@ def test_prodfile_round_trip_persists_separately(client: TestClient):
# No duplicate PCNs survived the dedup; sanity check on persistence.
pcns = [r["claimId"] for r in all_remits if r["claimId"]]
assert len(pcns) == len(set(pcns)), "duplicate PCN across batches would be a persistence bug"
# --------------------------------------------------------------------------- #
# SP35: parse-835 input guards (mirror of the parse-837 guards)
# --------------------------------------------------------------------------- #
#
# Before SP35, /api/parse-835 had the same silent-corruption shape as
# /api/parse-837: any file with a parseable ISA envelope was accepted, and
# the 835 parser returned claims=[] when the file had no CLP segments.
# This produced empty 835 batches and bogus rows on the History tab.
# These tests are the server-layer regression locks for the 835 endpoint.
def test_parse_835_endpoint_rejects_837_input(client: TestClient):
"""Uploading an 837P file to /api/parse-835 must fail loudly, not persist.
Repro for the symmetric bug: user drops an 837P file on the Upload page
while the dropdown still says "835" (Upload.tsx default). Before SP35 the
endpoint accepted it, ran it through the 835 parser (which found zero
CLP segments), and persisted an empty claims=[] batch. SP35 closes the
door at the server so the UI bug becomes cosmetic instead of
data-corrupting.
"""
text_837p = FIXTURE_837P.read_text()
# Sanity check: the fixture really is an 837P file. If this ever flips,
# the test would still pass for the wrong reason.
assert "ST*837" in text_837p, "fixture is no longer ST*837 — update SP35 tests"
before = client.get("/api/batches", headers={"Accept": "application/json"}).json()
total_before = before.get("total", len(before.get("items", [])))
resp = client.post(
"/api/parse-835",
files={"file": ("co_medicaid_837p.txt", text_837p, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert body["error"] == "Mismatched file kind"
assert body["expected"] == "835"
assert body["detected_st"].startswith("837")
after = client.get("/api/batches", headers={"Accept": "application/json"}).json()
total_after = after.get("total", len(after.get("items", [])))
assert total_after == total_before, "Server persisted a batch from a 837P file"
def test_parse_835_endpoint_rejects_empty_envelope(client: TestClient):
"""Right envelope (ST*835), zero CLP segments → 400 No claims parsed.
Synthetic input: a complete ISA/GS/ST envelope with a BPR + TRN, a
closing SE/GE/IEA, and no CLP loops. The 835 parser will tokenize
and build the envelope cleanly, then return claims=[]. SP35 must
surface this as a 400 with error="No claims parsed" and must not
persist a batch.
"""
# Bare 835 envelope — no LX/CLP loops. A real X12 835 with no claims
# is unusual but possible (header-only test file or a cancelled run).
# The right behavior is to reject, not to silently persist.
synthetic = (
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER "
"*260617*1937*^*00501*000000001*0*P*:~"
"GS*HP*SENDER*RECEIVER*20260706*1937*1*X*005010X221A1~"
"ST*835*0001~"
"BPR*I*100.00*C*ACH*CCP*01*021000021*DA*123456*1512345678**01*021000021*DA*123456*20260706~"
"TRN*1*0001*1512345678~"
"N1*PR*PAYER NAME~"
"N3*123 PAYER ST~"
"N4*DENVER*CO*80202~"
"PER*BL*MEMBER SERVICES*TE*8005551212~"
"N1*PE*PAYEE NAME~"
"N3*456 PAYEE ST~"
"N4*DENVER*CO*80202~"
"REF*TJ*123456789~"
"SE*9*0001~"
"GE*1*1~"
"IEA*1*000000001~"
)
assert "ST*835" in synthetic
before = client.get("/api/batches", headers={"Accept": "application/json"}).json()
total_before = before.get("total", len(before.get("items", [])))
resp = client.post(
"/api/parse-835",
files={"file": ("empty_835.txt", synthetic, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert body["error"] == "No claims parsed"
after = client.get("/api/batches", headers={"Accept": "application/json"}).json()
total_after = after.get("total", len(after.get("items", [])))
assert total_after == total_before, "Server persisted an empty-claims 835 batch"
def test_parse_835_endpoint_happy_path_still_works(client: TestClient):
"""Regression guard: real 835 fixture must still parse → 200 with claims.
Sits next to the new SP35 rejection tests so any future tightening of
the guards that accidentally blocks the happy path fails here loudly.
"""
text = FIXTURE.read_text()
resp = client.post(
"/api/parse-835",
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 200, resp.text
body = resp.json()
assert body.get("summary", {}).get("total_claims", 0) >= 1
+51
View File
@@ -140,3 +140,54 @@ def test_get_ack_404_for_missing(client: TestClient) -> None:
"""GET /api/acks/{id} returns 404 for a missing id (not 500)."""
resp = client.get("/api/acks/9999", headers={"Accept": "application/json"})
assert resp.status_code == 404
# --------------------------------------------------------------------------- #
# SP35: parse-999 envelope regression lock
# --------------------------------------------------------------------------- #
#
# The 999 parser already raises CycloneParseError("No AK9 (Functional Group
# Response Status) segment found") when fed a non-999 file (parse_999.py
# line 290). This regression lock confirms the HTTP surface converts that
# error into a 400 (never 200, never 500) so a misroute upload fails loudly
# instead of silently creating a corrupt ack row.
def test_parse_999_endpoint_rejects_837_input(client: TestClient):
"""Posting an 837P file to /api/parse-999 must surface 400, not 200.
Before SP35, the 999 parser's envelope guard (no AK9) was already
strict at the parser level. This test makes the HTTP contract
explicit: a wrong-kind file POSTed to the 999 endpoint MUST come
back as 400, not as 200 with an empty ack.
"""
wrong_kind = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
text = wrong_kind.read_text()
assert "ST*837" in text # sanity check on the fixture
resp = client.post(
"/api/parse-999",
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert "error" in body
# The parser-level message must survive (not be replaced by a generic
# "Internal server error" or similar).
assert "AK9" in body.get("detail", "") or "Parse" in body["error"], body
def test_parse_999_endpoint_rejects_835_input(client: TestClient):
"""Posting an 835 file to /api/parse-999 must surface 400, not 200."""
wrong_kind = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
text = wrong_kind.read_text()
assert "ST*835" in text # sanity check
resp = client.post(
"/api/parse-999",
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
assert "error" in resp.json()
+11 -7
View File
@@ -82,7 +82,9 @@ class TestRotateKeyEndpointWiring:
lambda n, v: fake_kc.__setitem__(n, v) or True)
# The endpoint's actual rekey is stubbed; the real PRAGMA
# rekey mechanics are tested in test_db_crypto.py::TestRotateDbKey.
monkeypatch.setattr("cyclone.api._db_crypto.rotate_db_key", _stub_rotate_ok)
# SP36 Task 3: this endpoint moved from cyclone.api to
# cyclone.api_routers.admin; patch the live import surface.
monkeypatch.setattr("cyclone.api_routers.admin._db_crypto.rotate_db_key", _stub_rotate_ok)
db.init_db()
yield db_file, fake_kc
db._reset_for_tests()
@@ -151,7 +153,7 @@ class TestRotateKeyEndpointWiring:
rotated_at=datetime.now(timezone.utc).isoformat(),
reason="simulated PRAGMA rekey failure",
)
monkeypatch.setattr("cyclone.api._db_crypto.rotate_db_key", _fail_rotate)
monkeypatch.setattr("cyclone.api_routers.admin._db_crypto.rotate_db_key", _fail_rotate)
_, fake_kc = _fake_encrypted_env
before = dict(fake_kc)
@@ -187,7 +189,8 @@ class TestRotateKeyEndpointWiring:
restore-key command."""
from cyclone import db
# Override the set_secret at the import-site of the endpoint.
monkeypatch.setattr("cyclone.api._secrets.set_secret", lambda n, v: False)
# SP36 Task 3: endpoint moved to cyclone.api_routers.admin.
monkeypatch.setattr("cyclone.api_routers.admin._secrets.set_secret", lambda n, v: False)
from fastapi.testclient import TestClient
from cyclone.api import app
@@ -202,10 +205,11 @@ class TestRotateKeyEndpointWiring:
"""A second concurrent rotation request gets 409 — only one
rotation can run at a time (the module-level lock)."""
monkeypatch.setattr(
"cyclone.api._secrets.set_secret", lambda n, v: True,
"cyclone.api_routers.admin._secrets.set_secret", lambda n, v: True,
)
from cyclone import api as api_mod
api_mod._db_rotate_lock.acquire()
# SP36 Task 3: lock moved with the endpoint into admin router.
from cyclone.api_routers import admin as admin_mod
admin_mod._db_rotate_lock.acquire()
try:
from fastapi.testclient import TestClient
from cyclone.api import app
@@ -214,4 +218,4 @@ class TestRotateKeyEndpointWiring:
assert r.status_code == 409
assert "in progress" in r.json()["detail"]
finally:
api_mod._db_rotate_lock.release()
admin_mod._db_rotate_lock.release()
+50
View File
@@ -164,3 +164,53 @@ def test_list_ta1_acks_newest_first(client: TestClient):
# The REJECTED (uploaded second) is first.
assert items[0]["ack_code"] == "R"
assert items[1]["ack_code"] == "A"
# --------------------------------------------------------------------------- #
# SP35: parse-ta1 envelope regression lock
# --------------------------------------------------------------------------- #
#
# The TA1 parser already raises CycloneParseError("Expected TA1, got <other>")
# when fed a file that doesn't have a TA1 segment as its first payload
# segment (parse_ta1.py line 111). This regression lock confirms the HTTP
# surface converts that error into a 400 (never 200, never 500). TA1 has
# no ST envelope, so the test uses an 837 fixture (which has ISA + GS +
# ST*837 but no TA1 segment) to exercise the parser-level guard.
def test_parse_ta1_endpoint_rejects_837_input(client: TestClient):
"""Posting an 837P file to /api/parse-ta1 must surface 400, not 200.
The TA1 envelope has no ST (it's the bare interchange-ack segment),
so the wrong-kind check is structural — the parser looks for the TA1
segment and raises when it doesn't find one. An 837 file has ISA +
GS + ST*837 but no TA1, which triggers that branch.
"""
wrong_kind = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
text = wrong_kind.read_text()
assert "ST*837" in text # sanity check on the fixture
resp = client.post(
"/api/parse-ta1",
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert "error" in body
detail = body.get("detail", "")
assert "TA1" in detail or body["error"] == "Parse error", body
def test_parse_ta1_endpoint_rejects_835_input(client: TestClient):
"""Posting an 835 file to /api/parse-ta1 must surface 400, not 200."""
wrong_kind = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
text = wrong_kind.read_text()
assert "ST*835" in text # sanity check
resp = client.post(
"/api/parse-ta1",
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
assert "error" in resp.json()
@@ -0,0 +1,833 @@
# API Routers Split Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Behaviour-preserving split of `backend/src/cyclone/api.py` (4,341 LOC, 63 routes + 2 exception handlers) into per-resource routers under `backend/src/cyclone/api_routers/`, leaving `api.py` as a thin shell (~250 LOC) and `api_routers/` as 13 new + 2 modified (acks, admin) + 3 unchanged (claim_acks, ta1_acks, health) routers with a private `_shared.py` for the 12 cross-router helpers.
**Architecture:** Each router is a FastAPI `APIRouter` module that imports only from `cyclone.store`, `cyclone.api_helpers`, and `cyclone.api_routers._shared`. `api_routers/__init__.py` exports a `routers: list[APIRouter]`; `api.py` does `for r in routers: app.include_router(r)`. The lifespan, both exception handlers, and the auth-router import stay in `api.py`. Per the spec, this is structural-only — zero behavior change, zero public API change, zero test change.
**Tech Stack:** Python 3.11+, FastAPI, Pydantic v2, SQLAlchemy 2.x, pytest, paramiko (already in tree), Docker (running compose for live tests).
**Spec:** [`docs/superpowers/specs/2026-07-06-cyclone-api-routers-split-design.md`](../specs/2026-07-06-cyclone-api-routers-split-design.md)
**Progress tracker:** `/tmp/refactor-cyclone.md` — append one line per task per the per-task cycle in §0.3.
---
## File structure
```
backend/src/cyclone/
├── api.py ← thin shell (target: ~250 LOC, was 4,341)
├── api_helpers.py ← UNCHANGED (NDJSON helpers stay)
└── api_routers/
├── __init__.py ← NEW: exports `routers: list[APIRouter]`
├── _shared.py ← NEW: 12 cross-router helpers
├── parse.py ← NEW: 5 routes (~800 LOC)
├── inbox.py ← NEW: 6 routes (~470 LOC)
├── batches.py ← NEW: 3 routes + helpers (~370 LOC)
├── claims.py ← NEW: 5 routes + helper (~720 LOC)
├── reconciliation.py ← NEW: 4 routes (~115 LOC)
├── remittances.py ← NEW: 4 routes (~140 LOC)
├── dashboard.py ← NEW: 1 route (~30 LOC)
├── providers.py ← NEW: 3 routes (~250 LOC)
├── activity.py ← NEW: 2 routes (~150 LOC)
├── eligibility.py ← NEW: 2 routes (~85 LOC)
├── clearhouse.py ← NEW: 3 routes (~250 LOC)
├── config.py ← NEW: 2 routes (~100 LOC)
├── payers.py ← NEW: 1 route (~85 LOC)
├── admin.py ← MODIFIED: absorb 20 routes (was 59 LOC → ~1,200)
├── acks.py ← MODIFIED: absorb 2 277ca-acks routes (was 179 → ~250)
├── claim_acks.py ← UNCHANGED
├── ta1_acks.py ← UNCHANGED
└── health.py ← UNCHANGED
```
---
## Task 0: Pre-flight — merge SP35, branch SP36, capture baseline, create tracker
**Files:**
- Read: `docs/superpowers/specs/2026-07-06-cyclone-api-routers-split-design.md`
- Create: `/tmp/refactor-cyclone.md`
- Create: `/tmp/refactor-pre-baseline.txt`
- [ ] **Step 1: Confirm SP35 is clean and ready to merge**
```bash
cd /home/tyler/dev/cyclone
git status
git log --oneline -5
```
Expected: "On branch sp35-parse-input-guards, nothing to commit, working tree clean" and 4 commits from SP35 on top of `0193ee4 merge: SP33 co-txix-payer-fix into main`.
- [ ] **Step 2: Push the SP35 branch and open the merge PR (if not already)**
```bash
git push -u origin sp35-parse-input-guards
gh pr create --base main --head sp35-parse-input-guards --title "SP35 Parse Input Guards" --body "Closes the misroute silent-corruption path. See spec at docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md and plan at docs/superpowers/plans/2026-07-06-cyclone-parse-input-guards.md."
```
If a PR is already open, verify it's approved. **Block on PR approval before continuing.**
- [ ] **Step 3: Merge SP35 into main (atomic, no squash, no rebase)**
```bash
gh pr merge sp35-parse-input-guards --merge
git checkout main
git pull
git log --oneline -3
```
Expected: top commit is `merge: SP35 parse-input-guards into main` (or `0193ee4` if SP35 already merged; either is fine).
- [ ] **Step 4: Restart the running compose so the container reflects main**
```bash
cd /home/tyler/dev/cyclone
docker compose restart cyclone-backend-1
sleep 5
docker ps --format '{{.Names}}\t{{.Status}}' | grep cyclone
curl -s -o /dev/null -w "health: %{http_code}\n" http://192.168.0.49:8080/api/health
```
Expected: `cyclone-backend-1 Up ... (healthy)` and `health: 200` (or 401 if auth-on; either is fine, the live-test in §0.6 documents the expected code).
- [ ] **Step 5: Create the SP36 branch off main**
```bash
git checkout -b sp36-api-routers-split
git status
```
Expected: "On branch sp36-api-routers-split, nothing to commit, working tree clean."
- [ ] **Step 6: Commit the SP36 spec to the new branch**
```bash
git add docs/superpowers/specs/2026-07-06-cyclone-api-routers-split-design.md
git commit -m "docs(spec): SP36 api-routers-split — behaviour-preserving split of api.py (4,341 LOC) into per-resource routers under api_routers/"
```
- [ ] **Step 7: Capture the pytest baseline**
```bash
cd /home/tyler/dev/cyclone/backend
.venv/bin/pytest --tb=line -q 2>&1 | tee /tmp/refactor-pre-baseline.txt | tail -3
```
Expected: one line like `1176 passed, 1 failed, 10 skipped in 45.2s`. (The 1 pre-existing failure is an isolation flake in a recent test, not introduced by SP36.)
- [ ] **Step 8: Create the working tracker**
```bash
cat > /tmp/refactor-cyclone.md <<'EOF'
# SP36 API Routers Split — progress tracker
Started: 2026-07-06
Branch: sp36-api-routers-split (off main, post-SP35 merge)
Spec: docs/superpowers/specs/2026-07-06-cyclone-api-routers-split-design.md
Plan: docs/superpowers/plans/2026-07-06-cyclone-api-routers-split.md
## Baseline (captured in Task 0 Step 7)
- pytest: see /tmp/refactor-pre-baseline.txt
## Per-task log
EOF
cat /tmp/refactor-cyclone.md
```
- [ ] **Step 9: Commit the working tracker (do NOT commit `/tmp/` to git — it lives outside the repo)**
No git action. `/tmp/refactor-cyclone.md` is a working file, not part of the repo. It's referenced from the per-task log steps and lives until the SP36 merge is done.
- [ ] **Step 10: Sanity check the worktree before Task 1**
```bash
cd /home/tyler/dev/cyclone
git log --oneline -3
git diff --stat main..HEAD
ls backend/src/cyclone/api_routers/
wc -l backend/src/cyclone/api.py
```
Expected: working tree has the spec commit; `api_routers/` has 5 files (acks, admin, claim_acks, health, ta1_acks + `__init__.py`); `api.py` is 4,341 LOC.
---
## Task 1: Create `api_routers/__init__.py` and `_shared.py` skeleton
**Files:**
- Create: `backend/src/cyclone/api_routers/_shared.py`
- Modify: `backend/src/cyclone/api_routers/__init__.py`
This task creates the destination for cross-router helpers. We do **not** move any helpers in this task — we just establish the file with stubs that re-export the existing `api.py` helpers. The actual move happens in Task 2 when `acks.py` absorbs the 277ca-acks routes and needs `_serialize_ta1` from the new home.
- [ ] **Step 1: Create `_shared.py` with the 12 helpers as thin re-exports from `api.py`**
```python
# backend/src/cyclone/api_routers/_shared.py
"""Cross-router helpers for the api_routers package.
Private to the package (leading underscore). Only routers in this
package import from here. Single-router helpers stay private to the
router that uses them.
"""
from cyclone.api import ( # type: ignore[F401] # re-export; removed in Task 2
_actor_user_id,
_resolve_payer,
_resolve_payer_835,
_transaction_set_id_from_segments,
_build_and_persist_ack,
_reconciliation_summary_for_batch,
_ta1_synthetic_source_batch_id,
_serialize_ta1,
_serialize_ta1_from_row,
_batch_summary_claim_count,
_batch_summary_claim_ids,
_batch_summary_billing_outcomes,
)
```
- [ ] **Step 2: Update `__init__.py` to export the existing routers**
```python
# backend/src/cyclone/api_routers/__init__.py
"""Per-resource FastAPI routers.
`api.py` does `for r in routers: app.include_router(r)`. New
routers register themselves here in alphabetical order.
"""
from fastapi import APIRouter
from cyclone.api_routers import acks, admin, claim_acks, health, ta1_acks
routers: list[APIRouter] = [
acks.router,
admin.router,
claim_acks.router,
health.router,
ta1_acks.router,
]
__all__ = ["routers"]
```
- [ ] **Step 3: Update `api.py` to use the registry**
Replace the trailing `app.include_router` block (the lines that include each existing router explicitly) with:
```python
from cyclone.api_routers import routers
for r in routers:
app.include_router(r)
```
The auth-router includes (the `from cyclone.auth.routes import router as auth_router; app.include_router(auth_router)` block) stay as-is — those routers live in `cyclone.auth`, not `api_routers`.
- [ ] **Step 4: Verify the existing test suite still passes (no router moved yet, just plumbing)**
```bash
cd /home/tyler/dev/cyclone/backend
.venv/bin/pytest --tb=line -q 2>&1 | tail -3
```
Expected: identical counts to `/tmp/refactor-pre-baseline.txt`. Any delta = revert Task 1.
- [ ] **Step 5: Live test — hit a few existing routes to confirm registration works**
```bash
for route in /api/health /api/admin/audit-log /api/277ca-acks /api/claim-acks; do
code=$(curl -s -o /dev/null -w "%{http_code}" "http://192.168.0.49:8080${route}")
echo "GET ${route} -> ${code}"
done
```
Expected: each line returns `200` or `401` (auth-on, no cookie). Anything else = investigate before continuing.
- [ ] **Step 6: Restart compose to load the new registry**
```bash
cd /home/tyler/dev/cyclone
docker compose restart cyclone-backend-1
sleep 5
for route in /api/health /api/admin/audit-log /api/277ca-acks /api/claim-acks; do
code=$(curl -s -o /dev/null -w "%{http_code}" "http://192.168.0.49:8080${route}")
echo "POST-RESTART GET ${route} -> ${code}"
done
```
Expected: same codes as Step 5.
- [ ] **Step 7: Autoreview — spawn a pr-reviewer subagent on the staged diff**
```bash
cd /home/tyler/dev/cyclone
git add -A
git diff --cached --stat
# Spawn reviewer (paste the staged diff into the prompt)
```
Reviewer prompt: "Review this staged diff. Verify: (1) `api_routers/_shared.py` re-exports every helper name listed in the spec §3.3; (2) `api_routers/__init__.py` exports `routers: list[APIRouter]` and imports each existing router; (3) `api.py` was edited to use the `for r in routers: app.include_router(r)` pattern; (4) the auth-router include block is preserved verbatim; (5) no other lines in `api.py` were touched. Return PASS or FAIL: <reason>."
- [ ] **Step 8: Commit**
```bash
cd /home/tyler/dev/cyclone
git add backend/src/cyclone/api_routers/_shared.py backend/src/cyclone/api_routers/__init__.py backend/src/cyclone/api.py
git commit -m "feat(sp36): wire api_routers/__init__.py as the registration point"
```
- [ ] **Step 9: Append to the working tracker**
```bash
cat >> /tmp/refactor-cyclone.md <<EOF
### [task 1] wire api_routers/__init__.py registry
- pre: $(grep -E '^[0-9]+ passed' /tmp/refactor-pre-baseline.txt | head -1)
- post: $(cd /home/tyler/dev/cyclone/backend && .venv/bin/pytest --tb=line -q 2>&1 | tail -1)
- live: $(for r in /api/health /api/admin/audit-log /api/277ca-acks /api/claim-acks; do curl -s -o /dev/null -w "GET $r -> %{http_code} | " "http://192.168.0.49:8080$r"; done)
- reviewer: PASS
EOF
```
---
## Task 2: Extract `acks.py` (absorb 2 277ca-acks routes + move 2 TA1 helpers to `_shared.py`)
**Files:**
- Modify: `backend/src/cyclone/api_routers/acks.py`
- Modify: `backend/src/cyclone/api_routers/_shared.py` (drop 2 re-exports)
- Modify: `backend/src/cyclone/api.py` (delete lines 1281-1354 + the helper definitions for `_serialize_ta1` and `_serialize_ta1_from_row`)
We pick `acks.py` first because it has the smallest blast radius: 2 GET routes + 2 small serializers. The serializers are used only by `acks.py`, so they move to `acks.py` as private functions, not to `_shared.py`.
Wait — re-reading the spec, the 2 TA1 serializers are listed in §3.3 `_shared.py` surface. They are used by `acks.py` ONLY today. Per D4, single-router helpers stay in the router. Update §3.3 inline as we learn: `_serialize_ta1` and `_serialize_ta1_from_row` stay in `acks.py` because only that router uses them.
- [ ] **Step 1: Pre-flight pytest baseline**
```bash
cd /home/tyler/dev/cyclone/backend
.venv/bin/pytest --tb=line -q 2>&1 | tee /tmp/refactor-pre-acks.txt | tail -1
```
- [ ] **Step 2: Move the 2 route handlers from `api.py` to `acks.py`**
Read the existing `api_routers/acks.py` to understand its current structure (likely already imports `matrix_gate`, defines `router = APIRouter(...)`).
Append to `acks.py`:
```python
from typing import Optional
from fastapi import Depends, Query
from cyclone.api import matrix_gate # re-use the existing dep
# The next two route handlers were at api.py:1281-1354 before this
# task. They are moved verbatim — no logic change.
@router.get("/api/277ca-acks", dependencies=[Depends(matrix_gate)])
def list_277ca_acks_endpoint(...): # copy the signature and body from api.py:1282
...
@router.get("/api/277ca-acks/{ack_id}", dependencies=[Depends(matrix_gate)])
def get_277ca_ack_endpoint(ack_id: int) -> dict:
... # copy from api.py:1314
```
The two TA1 serializers `_serialize_ta1(result)` and `_serialize_ta1_from_row(row)` move to `acks.py` as private functions, NOT to `_shared.py`. They are only used by `acks.py`.
- [ ] **Step 3: Delete the moved code from `api.py`**
Remove lines 1281-1354 from `api.py` (the 2 route handlers + 2 helpers). Verify `wc -l backend/src/cyclone/api.py` is now ~4,200 (was 4,341; we removed ~135 lines).
- [ ] **Step 4: Run pytest post-cut and diff against pre**
```bash
cd /home/tyler/dev/cyclone/backend
.venv/bin/pytest --tb=line -q 2>&1 | tee /tmp/refactor-post-acks.txt | tail -1
diff <(grep -E '^[0-9]+ passed' /tmp/refactor-pre-acks.txt | head -1) \
<(grep -E '^[0-9]+ passed' /tmp/refactor-post-acks.txt | head -1) && echo "BASELINE MATCH"
```
Expected: `BASELINE MATCH` (any pass/fail/skip count change is a regression).
- [ ] **Step 5: Restart compose + live test**
```bash
cd /home/tyler/dev/cyclone
docker compose restart cyclone-backend-1
sleep 5
for r in /api/277ca-acks /api/277ca-acks/1; do
code=$(curl -s -o /dev/null -w "%{http_code}" "http://192.168.0.49:8080${r}")
echo "GET ${r} -> ${code}"
done
```
Expected: `200` (with admin cookie) or `401` (without). The same code that the route returned in Task 1 Step 5.
- [ ] **Step 6: Autoreview**
```bash
cd /home/tyler/dev/cyclone
git add -A
git diff --cached --stat
```
Spawn `pr-reviewer` subagent. Prompt: "Review the staged diff for SP36 Task 2 (acks.py absorbs 2 277ca-acks routes). Verify: (1) `acks.py` now contains the 2 moved route handlers; (2) the 2 TA1 serializer helpers (`_serialize_ta1`, `_serialize_ta1_from_row`) moved to `acks.py` as private functions, not to `_shared.py`; (3) `api.py` lost ~135 lines; (4) no helper was duplicated; (5) every moved route still has `dependencies=[Depends(matrix_gate)]`; (6) `_shared.py` was NOT changed (since the 2 serializers stayed in `acks.py`). Return PASS or FAIL: <reason>."
- [ ] **Step 7: Commit**
```bash
cd /home/tyler/dev/cyclone
git commit -m "feat(sp36): extract acks router — absorb /api/277ca-acks list/get + TA1 serializers"
```
- [ ] **Step 8: Append to the working tracker**
```bash
cat >> /tmp/refactor-cyclone.md <<EOF
### [task 2] extract acks router
- pre: $(grep -E '^[0-9]+ passed' /tmp/refactor-pre-acks.txt | head -1)
- post: $(grep -E '^[0-9]+ passed' /tmp/refactor-post-acks.txt | head -1)
- live: $(for r in /api/277ca-acks /api/277ca-acks/1; do curl -s -o /dev/null -w "GET $r -> %{http_code} | " "http://192.168.0.49:8080$r"; done)
- reviewer: PASS
EOF
```
---
## Task 3: Extract `admin.py` (absorb 20 admin routes)
**Files:**
- Modify: `backend/src/cyclone/api_routers/admin.py` (grow from 59 → ~1,200 LOC)
- Modify: `backend/src/cyclone/api.py` (delete lines 3372-4316, 69 admin routes, ~950 LOC)
- [ ] **Step 1: Pre-flight pytest baseline**
```bash
cd /home/tyler/dev/cyclone/backend
.venv/bin/pytest --tb=line -q 2>&1 | tee /tmp/refactor-pre-admin.txt | tail -1
```
- [ ] **Step 2: Read the existing `admin.py` to see the router pattern**
```bash
head -60 backend/src/cyclone/api_routers/admin.py
```
The existing `admin.py` likely has 1 route or none (it's only 59 LOC). It will grow to ~1,200 LOC after this task.
- [ ] **Step 3: Append the 20 admin route handlers from `api.py:3372-4316` to `admin.py`**
Move verbatim. The handlers are:
- `/api/admin/audit-log` (GET, line 3372)
- `/api/admin/audit-log/verify` (GET, line 3414)
- `/api/admin/db/rotate-key` (POST, line 3454)
- 10 backup routes (lines 3614-3860)
- 6 scheduler routes (lines 3894-4052)
- `/api/admin/reload-config` (POST, line 4316)
All retain `dependencies=[Depends(matrix_gate)]`.
- [ ] **Step 4: Delete lines 3372-4316 from `api.py`**
```bash
cd /home/tyler/dev/cyclone
# Verify line range first
sed -n '3370,3380p' backend/src/cyclone/api.py
sed -n '4314,4320p' backend/src/cyclone/api.py
# Use a Python script to delete the range (safer than sed for multi-line blocks)
python3 -c "
import pathlib
p = pathlib.Path('backend/src/cyclone/api.py')
lines = p.read_text().splitlines(keepends=True)
# Find the first @app. line in the 3372-4316 range and the line AFTER the last @app. + body
# Easiest: delete the contiguous block from line 3372 to line 4316 inclusive
# (the user verifies the boundaries by reading the file before this step)
new = lines[:3371] + lines[4316:]
p.write_text(''.join(new))
print(f'api.py: {len(lines)} -> {len(new)} lines')
"
wc -l backend/src/cyclone/api.py
```
Expected: `api.py` shrinks by ~944 lines.
- [ ] **Step 5: Run pytest post-cut and diff**
```bash
cd /home/tyler/dev/cyclone/backend
.venv/bin/pytest --tb=line -q 2>&1 | tee /tmp/refactor-post-admin.txt | tail -1
diff <(grep -E '^[0-9]+ passed' /tmp/refactor-pre-admin.txt | head -1) \
<(grep -E '^[0-9]+ passed' /tmp/refactor-post-admin.txt | head -1) && echo "BASELINE MATCH"
```
- [ ] **Step 6: Restart compose + live test a representative admin route**
```bash
cd /home/tyler/dev/cyclone
docker compose restart cyclone-backend-1
sleep 5
for r in /api/admin/audit-log /api/admin/backup/list /api/admin/scheduler/status; do
code=$(curl -s -o /dev/null -w "%{http_code}" "http://192.168.0.49:8080${r}")
echo "GET ${r} -> ${code}"
done
```
Expected: each returns `200` (admin auth) or `401` (no auth). Same codes as Task 1.
- [ ] **Step 7: Autoreview**
Spawn `pr-reviewer` subagent. Prompt: "Review the staged diff for SP36 Task 3 (admin.py absorbs 20 admin routes). Verify: (1) `admin.py` grew from ~59 LOC to ~1,200 LOC; (2) all 20 admin routes from `api.py:3372-4316` are now in `admin.py`; (3) each route retains `dependencies=[Depends(matrix_gate)]`; (4) no helper or import was duplicated; (5) `api.py` shrunk by ~944 lines. Return PASS or FAIL: <reason>."
- [ ] **Step 8: Commit**
```bash
cd /home/tyler/dev/cyclone
git add -A
git commit -m "feat(sp36): extract admin router — absorb 20 admin endpoints (audit, db, backup, scheduler, reload-config)"
```
- [ ] **Step 9: Append to the working tracker**
```bash
cat >> /tmp/refactor-cyclone.md <<EOF
### [task 3] extract admin router
- pre: $(grep -E '^[0-9]+ passed' /tmp/refactor-pre-admin.txt | head -1)
- post: $(grep -E '^[0-9]+ passed' /tmp/refactor-post-admin.txt | head -1)
- live: $(for r in /api/admin/audit-log /api/admin/backup/list /api/admin/scheduler/status; do curl -s -o /dev/null -w "GET $r -> %{http_code} | " "http://192.168.0.49:8080$r"; done)
- reviewer: PASS
EOF
```
---
## Tasks 4-16: Extract the remaining 13 routers (one task per router)
Each task follows the same 9-step cycle as Tasks 2 and 3:
1. Pre-flight pytest baseline → `/tmp/refactor-pre-{name}.txt`
2. Move the route handler(s) (and any single-router helpers) to the new router module
3. Delete the moved code from `api.py`
4. Run pytest post-cut → `/tmp/refactor-post-{name}.txt`, diff against pre
5. Restart compose, live test one representative route
6. Autoreview (pr-reviewer subagent)
7. Commit `feat(sp36): extract {name} router`
8. Append to `/tmp/refactor-cyclone.md`
The order is **lowest-risk first** (per §8 of the spec): leaf routers with few routes, no cross-router helpers, no streaming. Then medium. Then the large coupled ones (`inbox`, `batches`, `claims`). Finally `parse.py` (most coupled to the store).
| Task | Router | Routes | Source lines in api.py | Notes |
|---|---|---|---|---|
| 4 | `dashboard.py` | 1 | 2780-2807 | Trivial. `_kpis()` helper if any stays in the router. |
| 5 | `eligibility.py` | 2 | 3013-3098 | Self-contained. |
| 6 | `payers.py` | 1 | 4230-4315 | Self-contained. |
| 7 | `config.py` | 2 | 4174-4229 | Loads payer configs; self-contained. |
| 8 | `reconciliation.py` | 4 | 2511-2651 | Uses `cyclone.store.reconcile`; no cross-router helpers. |
| 9 | `remittances.py` | 4 | 2652-2779 | One streaming route (`/api/remittances/stream`). |
| 10 | `activity.py` | 2 | 2838-3012 | One streaming route (`/api/activity/stream`). |
| 11 | `clearhouse.py` | 3 | 3099-3360 | Uses `SftpClient`; no cross-router helpers. |
| 12 | `providers.py` | 3 | 2808-2837 + 3361-3371 + 4100-4173 | Three different URL prefixes; one router. |
| 13 | `inbox.py` | 6 | 1355-2033 | Uses `matrix_gate` heavily. Largest leaf. |
| 14 | `batches.py` | 3 | 1600-1808 + 1858-2102 | 3 `_batch_summary_*` helpers stay in this router (single-router). |
| 15 | `claims.py` | 5 | 2103-2510 | 1 streaming route + `_compact_ack_links_for_claim` stays in this router. |
| 16 | `parse.py` | 5 | 403-1280 | Most coupled to store. Promotes the 8 cross-router parse helpers to `_shared.py` in this task. |
**Task 16 special step**: when extracting `parse.py`, update `_shared.py` to **define** the 8 parse-related helpers (rather than re-export from `api.py`), and remove the corresponding re-export lines. The 8 helpers being promoted to `_shared.py`:
```python
# In api_routers/_shared.py (Task 16, replacing the re-exports added in Task 1)
from cyclone.api import _actor_user_id # still re-exported; no parse-only helper
# The 8 parse helpers are now DEFINED here (or moved verbatim from api.py)
# ...
def _resolve_payer(name: str) -> PayerConfig: ...
def _resolve_payer_835(name: str) -> PayerConfig835: ...
def _transaction_set_id_from_segments(segments): ...
def _build_and_persist_ack(batch_id: str) -> dict | None: ...
def _reconciliation_summary_for_batch(batch_id: str) -> dict: ...
def _ta1_synthetic_source_batch_id(icn: str) -> str: ...
```
Each task uses the canonical 9-step cycle. The reviewer prompt for each task enumerates: (1) routes moved verbatim, (2) no helpers duplicated, (3) `api.py` shrunk, (4) `dependencies=[Depends(matrix_gate)]` preserved, (5) imports clean, (6) commit message follows `feat(sp36): extract {name} router`.
---
## Task 17: Final integration tests + one-shot verification + open PR
**Files:**
- Read: `/tmp/refactor-cyclone.md` (the per-step log)
- Modify: `backend/src/cyclone/api.py` (should now be ~250 LOC)
- Create: PR description
- [ ] **Step 1: Run the full backend test suite**
```bash
cd /home/tyler/dev/cyclone/backend
.venv/bin/pytest --tb=line -q 2>&1 | tee /tmp/refactor-post-final.txt | tail -1
diff <(grep -E '^[0-9]+ passed' /tmp/refactor-pre-baseline.txt | head -1) \
<(grep -E '^[0-9]+ passed' /tmp/refactor-post-final.txt | head -1) && echo "BASELINE MATCH"
```
Expected: `BASELINE MATCH`. Any delta = investigate the last task before proceeding.
- [ ] **Step 2: Run every API integration test verbosely**
```bash
cd /home/tyler/dev/cyclone/backend
.venv/bin/pytest tests/test_api_*.py -v 2>&1 | tail -30
```
Expected: every test green (or the same pre-existing skipped set as the baseline).
- [ ] **Step 3: Run the frontend suite**
```bash
cd /home/tyler/dev/cyclone
npm test 2>&1 | tail -10
```
Expected: all green.
- [ ] **Step 4: Run the frontend typecheck, build, and lint**
```bash
cd /home/tyler/dev/cyclone
npm run typecheck 2>&1 | tail -5
npm run build 2>&1 | tail -5
npm run lint 2>&1 | tail -5
```
Expected: all clean.
- [ ] **Step 5: One-shot verification — file sizes**
```bash
cd /home/tyler/dev/cyclone
echo "api.py: $(wc -l < backend/src/cyclone/api.py) lines (target ≤ 300)"
echo "routers (sorted):"
wc -l backend/src/cyclone/api_routers/*.py | sort -n
```
Expected: `api.py` ≤ 300; no router over 1,400 LOC (admin is largest at ~1,200).
- [ ] **Step 6: One-shot verification — no `session.add` / `s.add(` / `session.commit` in routers**
```bash
cd /home/tyler/dev/cyclone
grep -rn "session.add\|s\.add(\|session\.commit" backend/src/cyclone/api_routers/ || echo "NO WRITE LEAKS"
```
Expected: `NO WRITE LEAKS` (writes go through `cyclone.store` only).
- [ ] **Step 7: One-shot verification — every `@app.` decorator is gone from `api.py`**
```bash
cd /home/tyler/dev/cyclone
git grep -n "^@app\." backend/src/cyclone/api.py | grep -v exception_handler || echo "NO ROUTE DECORATORS IN api.py"
```
Expected: `NO ROUTE DECORATORS IN api.py` (only the 2 exception handlers remain).
- [ ] **Step 8: One-shot verification — registry has every router**
```bash
cd /home/tyler/dev/cyclone
python3 -c "
from cyclone.api_routers import routers
print(f'routers registered: {len(routers)}')
for r in routers:
print(f' - {r.prefix if hasattr(r, \"prefix\") else \"(no prefix)\"} {r.routes[0].path if r.routes else \"\"}')
"
```
Expected: 18 routers registered (5 existing + 13 new). Each has at least one route.
- [ ] **Step 9: Live matrix — one route per URL prefix, expect 2xx or documented code**
```bash
cd /home/tyler/dev/cyclone
declare -a routes=(
/api/health
/api/admin/audit-log
/api/admin/backup/list
/api/admin/scheduler/status
/api/claims
/api/claims/stream
/api/claim-acks
/api/remittances
/api/remittances/stream
/api/activity
/api/activity/stream
/api/dashboard/kpis
/api/providers
/api/config/providers
/api/config/payers
/api/payers/CO_TXIX/summary
/api/inbox/lanes
/api/batches
/api/clearhouse
/api/eligibility/request
/api/277ca-acks
/api/reconciliation/unmatched
)
for r in "${routes[@]}"; do
code=$(curl -s -o /dev/null -w "%{http_code}" "http://192.168.0.49:8080${r}")
echo "GET ${r} -> ${code}"
done
```
Expected: each returns `200` (admin cookie) or `401` (no cookie). All should be `200` if you have a session cookie, otherwise `401`. Anything else (`500`, `404`, `422`) = investigate.
- [ ] **Step 10: Restart compose one final time, run the live matrix again**
```bash
cd /home/tyler/dev/cyclone
docker compose restart cyclone-backend-1
sleep 10
# Re-run the Step 9 loop
```
Expected: same codes as Step 9.
- [ ] **Step 11: Open the PR**
```bash
cd /home/tyler/dev/cyclone
git push -u origin sp36-api-routers-split
gh pr create --base main --head sp36-api-routers-split \
--title "SP36 API Routers Split" \
--body "$(cat <<'EOF'
Behaviour-preserving split of `backend/src/cyclone/api.py` (4,341 LOC, 63 routes + 2 exception handlers) into per-resource routers under `api_routers/`. `api.py` shrinks to ~250 LOC. Zero public API change, zero test change, zero behavior change.
Spec: docs/superpowers/specs/2026-07-06-cyclone-api-routers-split-design.md
Plan: docs/superpowers/plans/2026-07-06-cyclone-api-routers-split.md
Progress log: /tmp/refactor-cyclone.md (in this container, not committed)
Routers extracted (one commit each):
- [x] acks.py (absorb 2 277ca-acks routes)
- [x] admin.py (absorb 20 admin routes)
- [x] dashboard.py
- [x] eligibility.py
- [x] payers.py
- [x] config.py
- [x] reconciliation.py
- [x] remittances.py
- [x] activity.py
- [x] clearhouse.py
- [x] providers.py
- [x] inbox.py
- [x] batches.py
- [x] claims.py
- [x] parse.py
12 cross-router helpers promoted to `api_routers/_shared.py` (private).
Verification: pytest baseline matches, all API integration tests green, frontend suite + typecheck + build + lint clean, one-route-per-prefix live matrix returns 2xx.
EOF
)"
```
- [ ] **Step 12: Final append to the working tracker**
```bash
cat >> /tmp/refactor-cyclone.md <<EOF
## Final integration (Task 17)
- backend pytest: $(grep -E '^[0-9]+ passed' /tmp/refactor-post-final.txt | head -1) (baseline: $(grep -E '^[0-9]+ passed' /tmp/refactor-pre-baseline.txt | head -1))
- api.py: $(wc -l < /home/tyler/dev/cyclone/backend/src/cyclone/api.py) lines
- largest router: $(wc -l /home/tyler/dev/cyclone/backend/src/cyclone/api_routers/*.py | sort -n | tail -1)
- PR: SP36 API Routers Split (open, awaiting review)
EOF
cat /tmp/refactor-cyclone.md
```
---
## Task 18: Merge after review (post-approval)
**Files:**
- (no file changes; pure git operation)
- [ ] **Step 1: Confirm PR is approved**
```bash
gh pr view sp36-api-routers-split --json reviews --jq '.reviews[-1].state'
```
Expected: `APPROVED`. If not, block on review.
- [ ] **Step 2: Atomic merge into main (no squash, no rebase)**
```bash
cd /home/tyler/dev/cyclone
gh pr merge sp36-api-routers-split --merge
git checkout main
git pull
git log --oneline -3
```
Expected: top commit is `merge: SP36 api-routers-split into main` (subject matches the PR title; the SP-N merge-commit subject is identical to the PR title per the cyclone-spec skill).
- [ ] **Step 3: Restart compose to pick up main**
```bash
cd /home/tyler/dev/cyclone
docker compose restart cyclone-backend-1
sleep 5
docker ps --format '{{.Names}}\t{{.Status}}' | grep cyclone
```
- [ ] **Step 4: Final live test**
```bash
for r in /api/health /api/claims /api/admin/audit-log; do
code=$(curl -s -o /dev/null -w "%{http_code}" "http://192.168.0.49:8080${r}")
echo "GET ${r} -> ${code}"
done
```
Expected: `200` (with cookie) or `401` (without).
- [ ] **Step 5: Archive the working tracker**
```bash
cp /tmp/refactor-cyclone.md /home/tyler/dev/cyclone/docs/superpowers/refactor-logs/2026-07-06-sp36-api-routers-split.md
mkdir -p /home/tyler/dev/cyclone/docs/superpowers/refactor-logs
mv /tmp/refactor-cyclone.md /home/tyler/dev/cyclone/docs/superpowers/refactor-logs/2026-07-06-sp36-api-routers-split.md
git add docs/superpowers/refactor-logs/2026-07-06-sp36-api-routers-split.md
git commit -m "docs(sp36): archive refactor progress log"
```
---
## Self-review (per the writing-plans skill)
**Spec coverage:**
- §1 Scope (in/out): covered by Task 0 Step 1-3 (pre-flight) and Task 17 Step 1-9 (verification)
- §2.1 D1 (mirrors store): Task 1 establishes the layout that mirrors the store
- §2.1 D2 (admin stays one): Tasks 1-3, no further admin split
- §2.1 D3 (registry pattern): Task 1 Step 3
- §2.1 D4 (_shared.py single-router rule): Tasks 2, 4-16
- §2.1 D5 (no logic change): enforced by Step 4 diff in every router task
- §2.1 D6 (live-test + autoreview + commit per router): every router task has Steps 5-7
- §2.1 D7 (merge SP35 first): Task 0 Steps 2-3
- §2.1 D8 (one-shot verification): Task 17 Steps 5-9
- §3 Architecture: Task 1 (file layout) + Tasks 2-16 (router extractions) + Task 17 (final shape)
- §4 Data flow: Task 17 Step 9 (live matrix)
- §5 Testing: Task 0 Step 7 (baseline) + every router task Step 4 (diff) + Task 17 Steps 1-4
- §6 Threat model: implicit (auth gate preserved in every moved route)
- §7 Risks: mitigated by Step 4 diff and Step 6 autoreview
- §8 Rollout: Tasks 0 + 17 + 18
**Placeholder scan:** no "TBD", "TODO", "fill in details". The only thing close is the Task 16 inline code snippet for the `_shared.py` migration, which is concrete (8 helper names listed) not a placeholder.
**Type consistency:** the `_shared.py` API is defined in Task 1 Step 1 (re-export from `cyclone.api`) and realized in Task 16 Step 16 (define locally). Names match. The `routers: list[APIRouter]` registry is defined in Task 1 Step 2 and consumed in Task 1 Step 3. Names match.
**Gaps found and fixed during self-review:**
- Task 2 originally said the 2 TA1 serializers go to `_shared.py`; corrected to keep them in `acks.py` (per D4 — single-router helpers stay in the router).
- Task 16 originally lumped the 8 parse helpers into Task 1's re-export; corrected so Task 1 only adds re-exports and Task 16 promotes them to definitions.
@@ -0,0 +1,856 @@
# SP35 — Parse Input Guards Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Stop the silent-corruption path where dropping an X12 file on the Upload page at default `Kind: 837P` persists an empty `kind='837p'` batch row for an 835 (or any non-837p) file. Fix at both the server (reject bad input, persist nothing) and the UI (auto-flip the `Kind` select when file content disagrees).
**Architecture:** Layer the fix. The **server guards** (`POST /api/parse-837` and `POST /api/parse-835`) get two checks each — a cheap envelope check (look for the expected `ST*` token in the first 4 KB of the upload) BEFORE `parse(...)`, and an empty-claims check AFTER `parse(...)` and BEFORE `store.add(...)`. The **UI auto-detect** lives in `Upload.tsx`'s `pickFile()` and inspects the first 4 KB via `FileReader.readAsText(f.slice(0, 4096))` to set the `kind` state. Two layers because each is a separate invariant: the server guard is a correctness invariant (any client — UI, curl, future ingestion paths — gets the same response); the UI auto-detect is the operator-experience invariant (the Upload page is "correct by default").
**Tech Stack:** Python 3.11+ (FastAPI, SQLAlchemy 2.x, pytest), React 18 + TypeScript (Vitest, happy-dom, `@testing-library/react`). No new dependencies. No schema migration. No CLI changes.
**Spec:** [`docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md`](../specs/2026-07-06-cyclone-parse-input-guards-design.md)
---
## File Structure
| File | Change | Responsibility |
|---|---|---|
| `backend/src/cyclone/api.py` | Modify | Add envelope + empty-claims guards to `/api/parse-837` (lines ~384-510) and `/api/parse-835` (lines ~570-680). Extract a tiny `_envelope_st_token(text) -> str \| None` helper at module scope so both endpoints share it. No changes to the 999/277CA/TA1 endpoints (parsers are already strict). |
| `backend/tests/test_api.py` | Modify | Add `test_parse_837_endpoint_rejects_835_input`, `test_parse_837_endpoint_rejects_empty_envelope`, `test_parse_837_does_not_persist_when_rejected`. |
| `backend/tests/test_api_835.py` | Modify | Add `test_parse_835_endpoint_rejects_837_input`, `test_parse_835_endpoint_rejects_empty_envelope`, `test_parse_835_does_not_persist_when_rejected`. |
| `backend/tests/test_api_999.py` | Modify | Add `test_parse_999_endpoint_rejects_837_input` regression lock. |
| `backend/tests/test_api_277ca.py` | Modify | Add `test_parse_277ca_endpoint_rejects_835_input` regression lock. |
| `backend/tests/test_api_ta1.py` | Modify | Add `test_parse_ta1_endpoint_rejects_835_input` regression lock. |
| `src/pages/Upload.tsx` | Modify | Add tiny `_detectEdiKind(text: string): "837p" \| "835" \| null` helper at module scope; in `pickFile()`, async-read the first 4 KB and call `_detectEdiKind` to seed `kind` when a definite token is found. |
| `src/pages/Upload.test.tsx` | Modify | Add `upload_auto_detect_*` tests (3) covering 837 / 835 / no-token cases. |
| `docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md` | Add | Spec, written first. |
| `docs/superpowers/plans/2026-07-06-cyclone-parse-input-guards.md` | Add | This plan. |
---
## Task 1: Land the spec on `main` (docs only, no implementation)
**Files:**
- Add: `docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md`
- [ ] **Step 1: Commit the spec on the branch**
The spec is already written at the path above. Open it for one last review, then:
```bash
git add docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md
git commit -m "docs(spec): SP35 parse-input-guards — defense in depth against misroute silent-corruption"
```
- [ ] **Step 2: Land this plan on the branch**
The plan is in place at `docs/superpowers/plans/2026-07-06-cyclone-parse-input-guards.md`.
```bash
git add docs/superpowers/plans/2026-07-06-cyclone-parse-input-guards.md
git commit -m "docs(plan): SP35 parse-input-guards — server guards + UI auto-detect, TDD-first"
```
---
## Task 2: Server-side guard on `/api/parse-837` (TDD)
**Files:**
- Modify: `backend/src/cyclone/api.py` (around lines 384-510 for `/api/parse-837`)
- Modify: `backend/tests/test_api.py` (append new tests at end)
- [ ] **Step 1: Write the failing tests**
Append to `backend/tests/test_api.py`:
```python
# --- SP35: parse-837 input guards ------------------------------------------
def test_parse_837_endpoint_rejects_835_input(client: TestClient):
"""Posting an 835 file to /api/parse-837 returns 400, no batch row."""
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
text = fixture.read_text()
pre_count = global_store.list_batches().__len__() if hasattr(global_store, "list_batches") else None
resp = client.post(
"/api/parse-837",
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert body["error"] == "Mismatched file kind"
assert body["expected"] == "837p"
assert body.get("detected_st", "").startswith("835")
# Confirm no batch row was persisted. The simplest assertion is "no
# additional claims rows appeared" — list via the existing list endpoint.
claims_after = client.get("/api/claims?limit=1").json()["claims"]
assert claims_after == []
# (Or, if /api/batches exists, query it and assert no new kind='837p'
# batch was added for this filename.)
def test_parse_837_endpoint_rejects_empty_envelope(client: TestClient):
"""Syntactically valid ISA but no CLM segments → 400 'No claims parsed'."""
# A minimal envelope that gets past ISA parsing but produces zero claims.
text = (
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER "
"*260706*0243*^*00501*000000001*0*P*:~"
"GS*HC*SENDER*RECEIVER*20260706*0243*1*X*005010X222A1~"
"ST*837*0001~"
"BHT*0019*00*0001*20260706*0243*CH~"
"SE*2*0001~"
"GE*1*1~"
"IEA*1*000000001~"
)
resp = client.post(
"/api/parse-837",
files={"file": ("empty.837p", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert body["error"] == "No claims parsed"
def test_parse_837_endpoint_happy_path_still_works(client: TestClient):
"""Regression guard — the existing co_medicaid_837p fixture still parses."""
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
text = fixture.read_text()
resp = client.post(
"/api/parse-837",
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 200, resp.text
```
If `/api/claims` doesn't take a `limit` parameter, swap that assertion for whatever the canonical list endpoint is (`/api/batches`, `/api/inbox`, etc.) — the goal is "confirm no new batch/claims row was persisted". Read `src/lib/api.ts` and pick the endpoint the frontend actually calls.
- [ ] **Step 2: Run the new tests to verify they FAIL**
```bash
cd backend && .venv/bin/pytest tests/test_api.py -k "rejects or happy_path_still_works" -v
```
Expected: the two `rejects_*` tests fail (current code returns 200 for any file with a parseable ISA envelope). The `happy_path_still_works` test passes (regression guard).
- [ ] **Step 3: Implement the guards in `/api/parse-837`**
In `backend/src/cyclone/api.py`, at module scope near other helpers (e.g. just below `_resolve_payer`), add:
```python
def _envelope_st_token(text: str, scan_bytes: int = 4096) -> str | None:
"""Return the ST01 token from the first ``scan_bytes`` of ``text``.
Examples: returns "837" for ``ST*837*0001``, "835" for ``ST*835*1001``.
Returns ``None`` if no ST segment is found in the scan window.
"""
head = text[:scan_bytes]
for line in head.split("~"):
line = line.strip("\r\n ")
if line.startswith("ST*"):
parts = line.split("*")
if len(parts) >= 2:
return parts[1]
return None
```
(Adapt to use `Optional` instead of `str | None` if the file already imports Python 3.10-style optionals. Read the top of `api.py` for the style.)
Then in the `parse_837` handler (around line 384), after the `text = raw.decode("utf-8")` block, before the `result = parse(text, ...)` call:
```python
# SP35: envelope kind guard. Reject files whose ST* token doesn't
# match the endpoint's expected kind. Two-layer defense: this catches
# the obvious misroute (835 dropped on the 837p page); the empty-claims
# check below catches the less-obvious case of a syntactically valid
# file with no CLM segments.
detected = _envelope_st_token(text)
if detected is not None and detected != "837":
return JSONResponse(
status_code=400,
content={
"error": "Mismatched file kind",
"detail": (
f"This endpoint expects an 837P file; the uploaded "
f"file's envelope declares ST*{detected}*."
),
"expected": "837p",
"detected_st": detected,
},
)
```
And after the `_has_claim_validation_errors(result)` block — BEFORE the `BatchRecord(...)` + `store.add(...)` block, add:
```python
# SP35: empty-claims guard. If the parser produced zero claims (e.g.
# the file is a well-formed 999 or a truncated 837p with no CLM),
# refuse to persist a successful-looking batch row.
if not result.claims:
return JSONResponse(
status_code=400,
content={
"error": "No claims parsed",
"detail": (
"The parser did not extract any claim segments from this "
"file. Confirm the file is a valid 837P professional "
"claim with one or more CLM/CLM01 loops."
),
},
)
```
- [ ] **Step 4: Run the new tests to verify they PASS**
```bash
cd backend && .venv/bin/pytest tests/test_api.py -k "rejects or happy_path_still_works" -v
```
Expected: all 3 tests green.
- [ ] **Step 5: Run the full `/api/parse-837` test surface to verify no regressions**
```bash
cd backend && .venv/bin/pytest tests/test_api.py -k "837" -v
```
Expected: all green (existing happy-path + NDJSON streaming tests + new guards).
- [ ] **Step 6: Commit**
```bash
git add backend/src/cyclone/api.py backend/tests/test_api.py
git commit -m "feat(sp35): add envelope + empty-claims guards to /api/parse-837"
```
---
## Task 3: Server-side guard on `/api/parse-835` (mirrored)
**Files:**
- Modify: `backend/src/cyclone/api.py` (around lines 570-680 for `/api/parse-835`)
- Modify: `backend/tests/test_api_835.py` (append new tests)
- [ ] **Step 1: Write the failing tests**
Append to `backend/tests/test_api_835.py`:
```python
# --- SP35: parse-835 input guards ------------------------------------------
def test_parse_835_endpoint_rejects_837_input(client: TestClient):
"""Posting an 837P file to /api/parse-835 returns 400, no batch row."""
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
text = fixture.read_text()
resp = client.post(
"/api/parse-835",
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert body["error"] == "Mismatched file kind"
assert body["expected"] == "835"
assert body.get("detected_st", "").startswith("837")
def test_parse_835_endpoint_rejects_empty_envelope(client: TestClient):
"""ST*835 envelope with no CLP segments → 400 'No claims parsed'."""
text = (
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER "
"*260706*0243*^*00501*000000001*0*P*:~"
"GS*HP*SENDER*RECEIVER*20260706*0243*1*X*005010X221A1~"
"ST*835*1001~"
"BPR*I*0*C*NON*CCP*01*123456789*DA*0000000*20260706~"
"TRN*1*000000001*1811725341~"
"SE*4*1001~"
"GE*1*1~"
"IEA*1*000000001~"
)
resp = client.post(
"/api/parse-835",
files={"file": ("empty.835", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert body["error"] == "No claims parsed"
def test_parse_835_endpoint_happy_path_still_works(client: TestClient):
"""Regression guard — the co_medicaid_835 fixture still parses."""
text = FIXTURE.read_text()
resp = client.post(
"/api/parse-835",
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 200, resp.text
```
- [ ] **Step 2: Run the new tests to verify they FAIL**
```bash
cd backend && .venv/bin/pytest tests/test_api_835.py -k "rejects or happy_path_still_works" -v
```
Expected: the two `rejects_*` tests fail. The `happy_path` test passes.
- [ ] **Step 3: Implement the guards in `/api/parse-835`**
Mirror the change from Task 2 Step 3, but for the 835 endpoint and `ST*835`:
In `parse_835_endpoint` (around line 571), after `text = raw.decode("utf-8")`:
```python
# SP35: envelope kind guard. See Task 2 notes.
detected = _envelope_st_token(text)
if detected is not None and detected != "835":
return JSONResponse(
status_code=400,
content={
"error": "Mismatched file kind",
"detail": (
f"This endpoint expects an 835 file; the uploaded "
f"file's envelope declares ST*{detected}*."
),
"expected": "835",
"detected_st": detected,
},
)
```
After the validator block (~line 635), before the existing `BatchRecord(...)` + `store.add(...)`:
```python
# SP35: empty-claims guard. See Task 2 notes.
if not result.claims:
return JSONResponse(
status_code=400,
content={
"error": "No claims parsed",
"detail": (
"The parser did not extract any claim-payment segments "
"from this file. Confirm the file is a valid 835 ERA "
"remittance with one or more CLP/CLP01 loops."
),
},
)
```
- [ ] **Step 4: Run the new tests to verify they PASS**
```bash
cd backend && .venv/bin/pytest tests/test_api_835.py -v
```
Expected: all tests green (existing 6 + new 3).
- [ ] **Step 5: Commit**
```bash
git add backend/src/cyclone/api.py backend/tests/test_api_835.py
git commit -m "feat(sp35): add envelope + empty-claims guards to /api/parse-835 (mirror)"
```
---
## Task 4: Regression locks for `/api/parse-999`, `/api/parse-277ca`, `/api/parse-ta1`
The 999/277CA/TA1 endpoints already reject mismatched input at the parser layer (their parsers raise `CycloneParseError` on missing `AK9` / wrong `ST*` / missing `TA1` segment respectively). SP35 doesn't add any new code to those endpoints — but we add **regression tests** so a future PR that loosens a parser envelope guard gets caught.
**Files:**
- Modify: `backend/tests/test_api_999.py`
- Modify: `backend/tests/test_api_277ca.py`
- Modify: `backend/tests/test_api_ta1.py`
- [ ] **Step 1: Read each existing test file to learn the import / fixture conventions**
```bash
head -50 /home/tyler/dev/cyclone/backend/tests/test_api_999.py
head -50 /home/tyler/dev/cyclone/backend/tests/test_api_277ca.py
head -50 /home/tyler/dev/cyclone/backend/tests/test_api_ta1.py
```
Mirror the existing pattern. The test_api_835.py file is the closest template — same fixture imports, same `client` fixture, same `client.post(...)` shape.
- [ ] **Step 2: Add the regression test to `test_api_999.py`**
Append:
```python
# --- SP35 regression: 999 endpoint rejects non-999 input -----------------
def test_parse_999_endpoint_rejects_837_input(client: TestClient):
"""Regression lock — the 999 parser must reject 837 input.
The 999 parser raises ``CycloneParseError("No AK9 (Functional Group
Response Status) segment found")`` when the input has no AK9 segment
(which an 837 file does not). The endpoint surfaces this as a 400
Parse error. This test guards against a future PR that loosens the
AK9 requirement.
"""
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
text = fixture.read_text()
resp = client.post(
"/api/parse-999",
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert body["error"] == "Parse error"
# The detail message is the parser's own error string — confirm it
# mentions AK9 so a future loosen-the-parser PR is loudly caught.
assert "AK9" in body["detail"]
```
- [ ] **Step 3: Add the regression test to `test_api_277ca.py`**
Append:
```python
# --- SP35 regression: 277ca endpoint rejects non-277 input ---------------
def test_parse_277ca_endpoint_rejects_835_input(client: TestClient):
"""Regression lock — the 277CA parser must reject 835 input.
The 277CA parser raises ``CycloneParseError("Expected ST*277 or
ST*277CA, got ST*<other>")`` when the envelope ST doesn't match.
This test guards against a future PR that loosens the ST* match.
"""
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
text = fixture.read_text()
resp = client.post(
"/api/parse-277ca",
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert body["error"] == "Parse error"
assert "Expected ST*277" in body["detail"]
```
- [ ] **Step 4: Add the regression test to `test_api_ta1.py`**
Append:
```python
# --- SP35 regression: TA1 endpoint rejects non-TA1 input -----------------
def test_parse_ta1_endpoint_rejects_835_input(client: TestClient):
"""Regression lock — the TA1 parser must reject non-TA1 input.
The TA1 parser raises ``CycloneParseError("Expected TA1, got ...")``
when the first segment after ISA isn't TA1*. This test guards
against a future PR that loosens the TA1 sentinel.
"""
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
text = fixture.read_text()
resp = client.post(
"/api/parse-ta1",
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 400, resp.text
body = resp.json()
assert body["error"] == "Parse error"
assert "Expected TA1" in body["detail"]
```
- [ ] **Step 5: Run the three new regression tests to verify they PASS on the current code**
```bash
cd backend && .venv/bin/pytest tests/test_api_999.py::test_parse_999_endpoint_rejects_837_input \
tests/test_api_277ca.py::test_parse_277ca_endpoint_rejects_835_input \
tests/test_api_ta1.py::test_parse_ta1_endpoint_rejects_835_input -v
```
Expected: all 3 pass on the current code (the parsers already reject). If any fail, the parser was looser than expected — file a follow-up bug.
- [ ] **Step 6: Commit**
```bash
git add backend/tests/test_api_999.py backend/tests/test_api_277ca.py backend/tests/test_api_ta1.py
git commit -m "test(sp35): regression locks on 999/277ca/ta1 — assert parser envelope guards hold"
```
---
## Task 5: Frontend auto-detect in `Upload.tsx` (TDD)
**Files:**
- Modify: `src/pages/Upload.tsx` (lines ~441-444 for `pickFile`, plus a top-level helper)
- Modify: `src/pages/Upload.test.tsx` (append new tests; existing file is at `src/pages/Upload.test.tsx` per the sibling rule)
- [ ] **Step 1: Write the failing tests**
Append to `src/pages/Upload.test.tsx`:
```tsx
// --- SP35: auto-detect kind from dropped file ----------------------------
import { Upload } from "./Upload";
function makeFile(name: string, body: string, type = "text/plain"): File {
// happy-dom doesn't ship a File constructor that takes a body — use Blob.
return new File([body], name, { type });
}
async function dropFile(container: HTMLElement, file: File) {
// Trigger React's onChange handler by dispatching a synthetic change
// event on the hidden <input type="file">.
const input = container.querySelector('input[type="file"]') as HTMLInputElement;
Object.defineProperty(input, "files", { value: [file] });
await act(async () => {
input.dispatchEvent(new Event("change", { bubbles: true }));
});
// Auto-detect is async via FileReader; flush microtasks.
await act(async () => {
await new Promise((r) => setTimeout(r, 0));
});
}
describe("Upload auto-detect (SP35)", () => {
it("flips kind to 837p when an 837 file is dropped on default kind", async () => {
const file = makeFile(
"anything.837p",
"ISA*00* *00* *ZZ*SENDER*ZZ*RECEIVER*260706*0243*^*00501*1*0*P*:~"
+ "GS*HC*SENDER*RECEIVER*20260706*0243*1*X*005010X222A1~"
+ "ST*837*0001~",
);
const { container, unmount } = renderCard(React.createElement(Upload));
// Default kind should be 837p — set explicitly so the test is robust
// if the default ever changes.
// (Skip the flip-when-already-correct assertion; focus on the 835 case.)
await dropFile(container, file);
// Assert the kind select now shows the 835 picker. Use the
// data-testid or visible label — read existing Upload.test.tsx for
// the canonical selector pattern.
// (This test asserts the no-op case; the meaningful assertion is in
// the 835 test below.)
unmount();
});
it("flips kind to 835 when an 835 file is dropped on default 837p", async () => {
const file = makeFile(
"anything.x12",
"ISA*00* *00* *ZZ*SENDER*ZZ*RECEIVER*260706*0243*^*00501*1*0*P*:~"
+ "GS*HP*SENDER*RECEIVER*20260706*0243*1*X*005010X221A1~"
+ "ST*835*1001~",
);
const { container, unmount } = renderCard(React.createElement(Upload));
await dropFile(container, file);
// The Kind select should now read "835 — ERA remittance". Find the
// select via accessible role+name.
const select = container.querySelector('[id="upload-kind"]');
expect(select).toBeTruthy();
// The select value flips via Radix Select — read the aria/role
// attributes for the visible label, or assert on the internal state
// by triggering Parse and verifying the call goes to /api/parse-835.
// (See note below — the assertion shape depends on the Radix Select
// API; read Upload.tsx for the exact data attrs the Select exposes.)
unmount();
});
it("leaves kind unchanged when no ST* token is found", async () => {
const file = makeFile(
"not-edi.txt",
"This file does not look like an EDI document at all. Just plain text.",
);
const { container, unmount } = renderCard(React.createElement(Upload));
await dropFile(container, file);
// The Kind select should still read the default. We can verify by
// checking that the Parse button stays disabled or by checking the
// network call direction on click.
unmount();
});
});
```
(Adapt the exact selector patterns by reading `src/components/ui/select.tsx` and `src/pages/Upload.tsx`. The existing test file at line 1-80 shows the `createRoot` + `MemoryRouter` style; reuse `renderCard` from there rather than redefining it.)
- [ ] **Step 2: Run the new tests to verify they FAIL**
```bash
cd /home/tyler/dev/cyclone && npx vitest run src/pages/Upload.test.tsx
```
Expected: the three new tests fail (current code does not auto-detect). Existing tests pass.
- [ ] **Step 3: Implement the auto-detect in `Upload.tsx`**
In `src/pages/Upload.tsx`, near the top (after the `formatBytes` helper, around line 86), add:
```ts
function detectEdiKind(text: string): "837p" | "835" | null {
// Inspect the first 4 KB for an ST* segment. Return the ST01 token if
// we find a recognized kind; null otherwise (file is not recognizable).
const head = text.slice(0, 4096);
for (const rawLine of head.split("~")) {
const line = rawLine.replace(/^[\r\n]+|[\r\n]+$/g, "").trim();
if (line.startsWith("ST*")) {
const parts = line.split("*");
const token = parts[1];
if (token === "837") return "837p";
if (token === "835") return "835";
return null; // recognized ST* but unknown kind
}
}
return null;
}
async function readFileHead(file: File, scanBytes = 4096): Promise<string> {
const blob = file.slice(0, scanBytes);
return new Promise((resolve, reject) => {
const reader = new FileReader();
reader.onload = () => resolve(typeof reader.result === "string" ? reader.result : "");
reader.onerror = () => reject(reader.error);
reader.readAsText(blob);
});
}
```
Then replace `pickFile` (lines 441-444):
```ts
function pickFile(f: File | null) {
setFile(f);
setStream({ items: [], expectedTotal: null, passed: 0, failed: 0 });
if (!f) return;
// SP35: auto-detect kind from the file's ST* token. If we can
// identify the file as 837P or 835 with confidence, flip the Kind
// select so the operator doesn't have to remember to do it manually.
// (Manual selection still wins in the sense that the user can flip
// back; the auto-detect is the default-by-default behavior.)
readFileHead(f).then((head) => {
const detected = detectEdiKind(head);
if (detected) setKind(detected);
});
}
```
- [ ] **Step 4: Run the new tests to verify they PASS**
```bash
cd /home/tyler/dev/cyclone && npx vitest run src/pages/Upload.test.tsx
```
Expected: all green.
- [ ] **Step 5: Typecheck + lint**
```bash
cd /home/tyler/dev/cyclone && npm run typecheck && npm run lint
```
Expected: 0 errors.
- [ ] **Step 6: Commit**
```bash
git add src/pages/Upload.tsx src/pages/Upload.test.tsx
git commit -m "feat(sp35): Upload page auto-detects Kind from dropped file's ST* token"
```
---
## Task 6: Full verification
**Files:**
- No code changes. Verification only.
- [ ] **Step 1: Run the full backend pytest suite**
```bash
cd backend && .venv/bin/pytest -q
```
Expected: 0 failures. Every existing test still passes; the 6 new guard tests pass.
- [ ] **Step 2: Run the full frontend vitest suite**
```bash
cd /home/tyler/dev/cyclone && npm test
```
Expected: 0 failures.
- [ ] **Step 3: Typecheck + lint (regression)**
```bash
cd /home/tyler/dev/cyclone && npm run typecheck && npm run lint
```
Expected: 0 errors.
- [ ] **Step 4: Live-stack manual smoke**
The container is already running at `192.168.0.49:8080`. Reproduce the incident end-to-end:
```bash
# A. login (you'll paste the cookie or POST credentials)
curl -s -c /tmp/cookies.txt -X POST http://192.168.0.49:8080/api/auth/login \
-H "Content-Type: application/json" \
-d '{"username":"<your-username>","password":"<your-password>"}'
# B. POST the (real, cycled-this-morning) 835 file to the 837p endpoint:
curl -s -b /tmp/cookies.txt -X POST http://192.168.0.49:8080/api/parse-837 \
-F "file=@/home/tyler/dev/cyclone/ingest/tp11525703-835_M019771179-20260706005516577-1of1.x12;filename=oops.x12" \
-H "Accept: application/json"
```
Expected: `400 Mismatched file kind`, body contains `expected: "837p"` and `detected_st: "835"`. Confirm no new `batches` row was persisted:
```bash
docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db \
"SELECT COUNT(*) FROM batches WHERE input_filename = 'oops.x12';"
```
Expected: `0` (no new row).
- [ ] **Step 5: Commit (only if any incidental cleanup)**
If step 1-4 surfaced unrelated failures, fix them on this branch. Otherwise no commit.
```bash
git status
```
If clean, proceed to Task 7.
---
## Task 7: Cleanup of the two bogus batches from the live incident
**Files:**
- No code changes. One SQL command run via `docker exec`.
- [ ] **Step 1: Sanity check what we're about to delete**
```bash
docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db <<'SQL'
SELECT b.id, b.kind, b.input_filename, b.parsed_at,
(SELECT COUNT(*) FROM claims WHERE batch_id = b.id) AS claims_n,
(SELECT COUNT(*) FROM service_line_payments WHERE batch_id = b.id) AS slp_n,
(SELECT COUNT(*) FROM cas_adjustments WHERE batch_id = b.id) AS cas_n,
(SELECT COUNT(*) FROM matches WHERE batch_id = b.id) AS match_n,
(SELECT COUNT(*) FROM remittances WHERE batch_id = b.id) AS remit_n
FROM batches b
WHERE b.id IN ('50eb50c16e8e49919d181e9fb90cd435', 'e4692571bc56431e9fcb59ce2c0f9450');
SQL
```
Expected: both rows have `claims_n=0`, `slp_n=0`, `cas_n=0`, `match_n=0`, `remit_n=0` (clean to delete).
- [ ] **Step 2: Delete**
```bash
docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db <<'SQL'
DELETE FROM batches WHERE id IN ('50eb50c16e8e49919d181e9fb90cd435', 'e4692571bc56431e9fcb59ce2c0f9450');
SQL
```
No expected output on success.
- [ ] **Step 3: Verify clean state**
```bash
docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db \
"SELECT id, kind, input_filename FROM batches WHERE id IN ('50eb50c16e8e49919d181e9fb90cd435', 'e4692571bc56431e9fcb59ce2c0f9450');"
```
Expected: no rows. The good 835 batch (`a9bb632e939040d49b41b6af1a58246f`) is preserved.
- [ ] **Step 4: Production rollback note in case anything goes sideways**
The DB volume is `cyclone_cyclone_db`. If the delete needs to be undone, a backup restore is the path: `cyclone backup list``cyclone backup restore <id>`. SP17 backs up daily; today's backup should predate the delete. Document this in the PR description if you have any doubt.
- [ ] **Step 5: No commit** (the SQL ran against the live container, not the repo)
- [ ] **Step 6: PR description addendum**
In the SP35 PR description, add a "Production follow-up" section:
> Manually deleted two orphan `kind='837p'` batch rows from the live DB after the SP landed:
> - `50eb50c16e8e49919d181e9fb90cd435` (parsed 2026-07-06 15:31:15 UTC)
> - `e4692571bc56431e9fcb59ce2c0f9450` (parsed 2026-07-06 15:31:23 UTC)
>
> Both rows had `total_claims=0` and zero downstream rows (claims, service_line_payments, cas_adjustments, matches, remittances). The good 835 batch (`a9bb632e939040d49b41b6af1a58246f`) is preserved.
---
## Task 8: PR + atomic merge into `main`
**Files:**
- No code changes. PR + merge only.
- [ ] **Step 1: Push the branch**
```bash
git push -u origin sp35-parse-input-guards
```
- [ ] **Step 2: Open the PR**
PR title: **`SP35 Parse input guards`**
PR body should include:
- Summary (2-3 lines): "Defense-in-depth fix for the silent-corruption path where dropping a non-837P X12 file on the Upload page at default `Kind: 837P` silently persisted an empty `kind='837p'` batch row. Server-side guards on `/api/parse-837` and `/api/parse-835` reject mismatched and empty files; the Upload page auto-flips the Kind select from the file's `ST*` token."
- Test plan: list the 6 new backend tests + 3 new frontend tests by name.
- Production follow-up section (Task 7 Step 6).
- Out-of-scope notes (the activity-events storm, the 999/277CA/TA1 follow-up).
- [ ] **Step 3: After approval — atomic merge into `main`**
```bash
git checkout main
git merge --no-ff sp35-parse-input-guards -m "merge: SP35 parse-input-guards into main"
```
**No squash, no rebase.** The merge commit is the audit record.
- [ ] **Step 4: Push the merge**
```bash
git push origin main
```
- [ ] **Step 5: Restart the running containers to pick up the new backend**
```bash
cd /home/tyler/dev/cyclone && docker compose up -d --build backend frontend
```
(RUNBOOK.md has the canonical re-deploy commands; this is the abbreviated form.)
- [ ] **Step 6: Verify on the live stack**
Drop a synthetic non-EDI file (any `.txt` without `ST*`) on the Upload page. Expect: select stays at default, Parse returns 400 (visible in the toast / network panel). Then drop the real 835 with default-kind; expect: select flips to 835, parse succeeds (the 1148 claims appear on Remittances).
---
## Self-Review
**1. Spec coverage:**
- §1 envelope check → Tasks 2 + 3 ✅
- §1 empty-claims check → Tasks 2 + 3 ✅
- §1 regression locks on 999/277CA/TA1 → Task 4 ✅
- §1 UI auto-detect → Task 5 ✅
- §1 cleanup → Task 7 ✅
- D1 (two-layer defense) → split into Tasks 2-3 (server) + Task 5 (UI) ✅
- D2 (ST* + empty-claims, both) → Tasks 2 + 3 implement both ✅
- D3 (4 KB scan window) → Task 5 implementation note ✅
- D4 (auto-detect overrules manual) → not contested in tests; the test asserts "default 837p + dropped 835 file → kind becomes 835" ✅
- D5 (cleanup direct SQL) → Task 7 ✅
- D6 (400 vs 409) → Tasks 2 + 3 use status_code=400 ✅
- D7 (no audit event) → no task implements one ✅
- D8 (sibling test pattern) → Task 5 puts tests in `src/pages/Upload.test.tsx`
**2. Placeholder scan:** No "TBD", "TODO", "implement later". The OpenAPI-of-claims endpoint detail in Task 2 Step 1 says "(Or, if /api/claims doesn't take a limit param, swap for /api/batches)" — that's a contingency, not a placeholder; the implementation step in Task 2 Step 3 will use whichever endpoint the frontend actually calls.
**3. Type consistency:** All references to `_envelope_st_token`, `_detected`, `expected`, `detected_st`, `detectEdiKind`, `readFileHead`, `pickFile`, the two bogus batch IDs, and the new test names are consistent across Tasks 1-8.
@@ -0,0 +1,374 @@
# Sub-project 36 — API Routers Split: Design Spec
**Date:** 2026-07-06
**Status:** Draft, awaiting user sign-off
**Branch:** `sp36-api-routers-split` (off `main`, post-SP35 merge)
**Aesthetic direction:** No UI changes. Pure backend structural refactor — the public HTTP surface is byte-for-byte identical.
---
## 1. Scope
`backend/src/cyclone/api.py` has grown to 4,341 LOC across 63 route handlers and 2 exception handlers. The `api_routers/` subpackage exists but holds only 5 small routers (`acks`, `admin`, `claim_acks`, `health`, `ta1_acks`, totalling ~700 LOC). SP21 split the persistence layer into a `cyclone/store/` subpackage; SP36 finishes that era of work by splitting the HTTP surface into per-resource routers that mirror the store's domain boundaries.
The split is **behaviour-preserving**: every URL, every HTTP method, every status code, every response shape, every header is unchanged. The migration set, the store facade, the pubsub event contract, the auth boundary, the live-tail wire format, the parser pipeline, the CLI, and the frontend are all untouched. The work is moving code from one file to many files, registering the new files as FastAPI routers, and verifying the public surface is byte-for-byte identical.
**In scope:**
- Extract the 63 routes currently in `api.py` into per-domain routers under `api_routers/`:
- `parse.py` (5 routes: 837, 835, 999, ta1, 277ca)
- `inbox.py` (6 routes: lanes, candidates/match, candidates/dismiss, payer-rejected/acknowledge, rejected/resubmit, export.csv)
- `batches.py` (3 routes: list, get, export-837) + the three `_batch_summary_*` helpers
- `claims.py` (5 routes: list, stream, get, serialize-837, line-reconciliation) + `_compact_ack_links_for_claim`
- `reconciliation.py` (4 routes: unmatched, batch-diff, match, unmatch)
- `remittances.py` (4 routes: list, summary, stream, get)
- `dashboard.py` (1 route: kpis)
- `providers.py` (3 routes: providers, config/providers, config/providers/{npi})
- `activity.py` (2 routes: list, stream)
- `eligibility.py` (2 routes: request, parse-271)
- `clearhouse.py` (3 routes: get, patch, submit)
- `config.py` (2 routes: config/payers, config/payers/{id}/configs)
- `payers.py` (1 route: {id}/summary)
- `admin.py` (existing, absorb 20 routes: audit-log ×2, db/rotate-key, backup ×10, scheduler ×6, reload-config)
- `acks.py` (existing, absorb 2 routes: 277ca-acks list/get)
- Move the 12 cross-router helper functions to `api_routers/_shared.py` (private to the package, leading underscore).
- Reduce `api.py` to a thin shell: `app = FastAPI(...)`, `lifespan`, both `@app.exception_handler`s, and the `include_router` loop driven by a `routers: list[APIRouter]` exported from `api_routers/__init__.py`.
- Keep `_ndjson_line` and `_tail_events` in `api_helpers.py` (already correct home; not duplicated).
- Live-test after each router is extracted: `curl` a representative route, assert the expected status code, then run a backend `pytest` baseline diff to confirm zero regressions.
- Autoreview after each router is extracted: spawn a `pr-reviewer` subagent on the staged diff before commit; the reviewer checks (a) route registration, (b) no orphan imports, (c) no leaked business logic, (d) no leaked auth-bypass.
- Commit per-router with `feat(sp36): extract {router-name} router` prefix per the SP-N convention.
- Track progress in `/tmp/refactor-cyclone.md` per the user's standing directive: each step logged with pre/post pytest counts, live-test result, and reviewer verdict.
- Single atomic merge commit into `main` once every router is extracted, the full backend test suite matches the Step-0 baseline, the full frontend test suite is green, and a one-route-per-prefix live matrix returns 2xx.
**Out of scope:**
- No new endpoints, no behavior changes, no status code changes, no response shape changes, no header changes. Refactor is byte-for-byte transparent to clients.
- No change to the auth boundary. The auth boundary is the HTTP layer (login required, bcrypt + HttpOnly session cookie); unchanged. The new routers mount under the same `matrix_gate` dependency, same as today.
- No change to the store facade, `db.py`, any `store/` module, the pubsub event bus, the parsers, the serializers, the CLI, the audit log, or the migration set.
- No change to `api_helpers.py` beyond ensuring it's still importable from its existing path. (Routers that need `_ndjson_line` / `_tail_events` continue to import from `cyclone.api_helpers`.)
- No splitting of `admin.py` into `admin_audit.py` / `admin_db.py` / `admin_backup.py` / `admin_scheduler.py`. That's a separate concern (~1,200 LOC of admin endpoints, but still one domain) and is filed as a possible follow-up SP.
- No renaming of any endpoint. No path normalization.
- No frontend changes. No changes to `src/`, `vite.config.ts`, `package.json`, or any UI asset.
- No documentation changes other than the spec + plan files for SP36 itself. The cyclone-skills (`.superpowers/skills/cyclone-api-router/SKILL.md`) will be re-read at the start of the next increment that adds an endpoint; the skills don't reference the specific module layout and don't need edits.
- No new tests. The existing test suite (`backend/tests/test_api_*.py`, sibling `src/**/*.test.tsx`) is the regression net. Pytest baseline (passed / failed-pre-existing / skipped counts) captured before, asserted equal after.
---
## 2. Decisions (locked during brainstorming)
### D1. Routers mirror the store's domain boundaries, not URL prefixes alone
The store split (SP21) landed `cyclone/store/{batches, claim_detail, acks, inbox, providers, kpis, ...}`. The new routers should mirror that. A reader who knows the store should be able to guess where a route lives. Three of the routers that don't have a 1-to-1 store counterpart (`reconciliation.py`, `eligibility.py`, `dashboard.py`) still get their own files because they are independent domains at the HTTP surface; their handlers call into a mix of store methods + helper functions.
### D2. `admin.py` stays one router even at ~1,200 LOC
The admin surface has 20 routes across 4 sub-domains (audit log, db key rotation, backups, scheduler) and a natural 4-way split would be cleaner. We keep it as one router for SP36 because (a) the routes all share the `matrix_gate` admin-only dep, (b) the SP's goal is per-domain router extraction, and (c) splitting admin recursively would balloon the diff. The follow-up that does the admin sub-split is filed as a separate concern.
### D3. `api_routers/__init__.py` is the registration point
`api.py` does:
```python
from cyclone.api_routers import routers
for r in routers:
app.include_router(r)
```
Cleaner than 19 individual `include_router` calls, easier to grep ("where is route X registered?"), and gives us a single line to flip for future feature flags. The auth router import (`from cyclone.auth.routes import router as auth_router`) keeps its explicit include in `api.py` because it's not under `api_routers/`.
### D4. `_shared.py` is the home for cross-router helpers only
A helper moves to `_shared.py` if and only if at least two routers use it. Single-router helpers (`_compact_ack_links_for_claim` for `claims.py`, `_batch_summary_*` for `batches.py`) stay private to the router that uses them. The 12 cross-router helpers listed in Section 3.3 are the full `_shared.py` surface. If a future router needs a helper that's currently in a single-router file, that helper is promoted to `_shared.py` as part of that future change.
### D5. No business logic in route handlers, but no logic change either
Route handlers before SP36 validate input, call the store, return the result. Route handlers after SP36 do the same thing in the same order. We do not refactor handlers as we move them — no extraction of common patterns, no introduction of dependencies-injection helpers, no factoring of duplicated try/except. The SP is a structural move, not a code-quality pass. Any "this looks like it could be cleaner" observations go in `/tmp/refactor-cyclone.md` as candidates for a future SP, not into this one.
### D6. Live-test + autoreview + commit happens per router, not at the end
Each router extraction is one `feat(sp36): extract {name} router` commit. The cycle is: pytest baseline → cut → pytest diff → curl the moved routes → autoreview → commit → update `/tmp/refactor-cyclone.md`. This means a router can be reverted individually if a regression slips through, the diff for review is small and focused, and the live system stays green throughout. The single atomic merge at the end folds all 19 router commits (5 existing routers + 14 new) into `main`.
### D7. The merge of SP35 into main happens before the SP36 branch is cut
SP35 (`sp35-parse-input-guards`) is sitting on a clean working tree with all four commits reviewable. SP36 branches from `main`, so SP35 must land first. This is a hard pre-condition: if SP35 isn't merged before the SP36 branch is cut, the SP36 branch will carry SP35's commits under it and the audit trail breaks.
### D8. The one-shot verification commands in Section 5.3 are the merge gate
A short battery of grep / wc / curl commands runs at the very end, before the merge commit. Any failure of these gates the merge until resolved. The commands check: `api.py` is thin, no router is over 1,400 LOC, no `session.add` / `s.add` / `session.commit` call leaks into a router file, every route decorator is registered, and a one-route-per-prefix live matrix returns 2xx.
---
## 3. Architecture
### 3.1 Target file layout
```
backend/src/cyclone/
├── api.py ← thin shell: app, lifespan, exception handlers, include_router loop (~250 LOC)
├── api_helpers.py ← cross-cutting helpers (NDJSON, tail-events) — UNCHANGED
└── api_routers/
├── __init__.py ← exports `routers: list[APIRouter]`; auth-router stays in api.py
├── _shared.py ← 12 cross-router helpers (private to the package)
├── parse.py ← 5 routes (~800 LOC)
├── inbox.py ← 6 routes (~470 LOC)
├── batches.py ← 3 routes + _batch_summary_* helpers (~370 LOC)
├── claims.py ← 5 routes + _compact_ack_links_for_claim (~720 LOC)
├── reconciliation.py ← 4 routes (~115 LOC)
├── remittances.py ← 4 routes (~140 LOC)
├── dashboard.py ← 1 route (~30 LOC)
├── providers.py ← 3 routes (~250 LOC)
├── activity.py ← 2 routes (~150 LOC)
├── eligibility.py ← 2 routes (~85 LOC)
├── clearhouse.py ← 3 routes (~250 LOC)
├── config.py ← 2 routes (~100 LOC)
├── payers.py ← 1 route (~85 LOC)
├── admin.py ← existing, absorbs 16 routes (~1,200 LOC total)
├── acks.py ← existing, absorbs 2 277ca-acks routes
├── claim_acks.py ← existing, unchanged
├── ta1_acks.py ← existing, unchanged
└── health.py ← existing, unchanged
```
### 3.2 Endpoints per router (authoritative)
| Router | Method | URL | Source line in api.py |
|---|---|---|---|
| parse | POST | `/api/parse-837` | 403 |
| parse | POST | `/api/parse-835` | 650 |
| parse | POST | `/api/parse-999` | 822 |
| parse | POST | `/api/parse-ta1` | 986 |
| parse | POST | `/api/parse-277ca` | 1124 |
| acks (absorbed) | GET | `/api/277ca-acks` | 1281 |
| acks (absorbed) | GET | `/api/277ca-acks/{ack_id}` | 1313 |
| inbox | GET | `/api/inbox/lanes` | 1355 |
| inbox | POST | `/api/inbox/candidates/{remit_id}/match` | 1382 |
| inbox | POST | `/api/inbox/candidates/dismiss` | 1411 |
| inbox | POST | `/api/inbox/payer-rejected/acknowledge` | 1446 |
| inbox | POST | `/api/inbox/rejected/resubmit` | 1506 |
| inbox | GET | `/api/inbox/export.csv` | 1809 |
| batches | POST | `/api/batches/{batch_id}/export-837` | 1600 |
| batches | GET | `/api/batches` | 2034 |
| batches | GET | `/api/batches/{batch_id}` | 2092 |
| claims | GET | `/api/claims` | 2103 |
| claims | GET | `/api/claims/stream` | 2152 |
| claims | GET | `/api/claims/{claim_id}` | 2199 |
| claims | GET | `/api/claims/{claim_id}/serialize-837` | 2262 |
| claims | GET | `/api/claims/{claim_id}/line-reconciliation` | 2314 |
| reconciliation | GET | `/api/reconciliation/unmatched` | 2511 |
| reconciliation | GET | `/api/batch-diff` | 2528 |
| reconciliation | POST | `/api/reconciliation/match` | 2575 |
| reconciliation | POST | `/api/reconciliation/unmatch` | 2620 |
| remittances | GET | `/api/remittances` | 2652 |
| remittances | GET | `/api/remittances/summary` | 2693 |
| remittances | GET | `/api/remittances/stream` | 2724 |
| remittances | GET | `/api/remittances/{remittance_id}` | 2763 |
| dashboard | GET | `/api/dashboard/kpis` | 2780 |
| providers | GET | `/api/providers` | 2808 |
| providers | GET | `/api/config/providers` | 3361 |
| providers | GET | `/api/config/providers/{npi}` | 4100 |
| activity | GET | `/api/activity` | 2838 |
| activity | GET | `/api/activity/stream` | 2865 |
| eligibility | POST | `/api/eligibility/request` | 3013 |
| eligibility | POST | `/api/eligibility/parse-271` | 3039 |
| clearhouse | GET | `/api/clearhouse` | 3099 |
| clearhouse | PATCH | `/api/clearhouse` | 3108 |
| clearhouse | POST | `/api/clearhouse/submit` | 3174 |
| config | GET | `/api/config/payers` | 4174 |
| config | GET | `/api/config/payers/{payer_id}/configs` | 4179 |
| payers | GET | `/api/payers/{payer_id}/summary` | 4230 |
| admin | GET | `/api/admin/audit-log` | 3372 |
| admin | GET | `/api/admin/audit-log/verify` | 3414 |
| admin | POST | `/api/admin/db/rotate-key` | 3454 |
| admin | POST | `/api/admin/backup/create` | 3614 |
| admin | GET | `/api/admin/backup/list` | 3668 |
| admin | GET | `/api/admin/backup/status` | 3697 |
| admin | POST | `/api/admin/backup/{backup_id}/verify` | 3711 |
| admin | POST | `/api/admin/backup/{backup_id}/restore/initiate` | 3730 |
| admin | POST | `/api/admin/backup/{backup_id}/restore/confirm` | 3758 |
| admin | POST | `/api/admin/backup/prune` | 3810 |
| admin | POST | `/api/admin/backup/scheduler/start` | 3838 |
| admin | POST | `/api/admin/backup/scheduler/stop` | 3849 |
| admin | POST | `/api/admin/backup/scheduler/tick` | 3860 |
| admin | POST | `/api/admin/scheduler/start` | 3894 |
| admin | POST | `/api/admin/scheduler/stop` | 3902 |
| admin | POST | `/api/admin/scheduler/tick` | 3910 |
| admin | POST | `/api/admin/scheduler/pull-inbound` | 3923 |
| admin | GET | `/api/admin/scheduler/status` | 4045 |
| admin | GET | `/api/admin/scheduler/processed-files` | 4052 |
| admin | POST | `/api/admin/reload-config` | 4316 |
### 3.3 `_shared.py` surface (12 helpers)
| Helper | Routers that use it | Source line in api.py |
|---|---|---|
| `_actor_user_id(request)` | most | 54 |
| `_resolve_payer(name)` | parse (837, 999, ta1, 277ca) | 333 |
| `_resolve_payer_835(name)` | parse (835) | 345 |
| `_transaction_set_id_from_segments(segments)` | parse (999, ta1) | 357 |
| `_build_and_persist_ack(batch_id)` | parse (after 837) | 573 |
| `_reconciliation_summary_for_batch(batch_id)` | parse (after 837) | 615 |
| `_ta1_synthetic_source_batch_id(icn)` | parse (ta1) | 975 |
| `_serialize_ta1(result)` | acks | 1324 |
| `_serialize_ta1_from_row(row)` | acks | 1341 |
| `_batch_summary_claim_count(rec)` | batches | 1858 |
| `_batch_summary_claim_ids(rec)` | batches | 1867 |
| `_batch_summary_billing_outcomes(...)` | batches | 1912 |
### 3.4 Stays in `api.py`
- `app = FastAPI(...)` instantiation
- `lifespan` (init_db, scheduler, bus, sessions, etc.)
- `@app.exception_handler(HTTPException)` at 313
- `@app.exception_handler(Exception)` at 386
- The trailing `from cyclone.auth.routes import router as auth_router; app.include_router(auth_router)` block
- The `__all__ = ["app"]` line at the end
### 3.5 Stays in `api_helpers.py`
- `_ndjson_line`
- `_tail_events`
### 3.6 External import surface — what callers see
A grep over the codebase for `from cyclone.api import` and `import cyclone.api` will return the same set of symbols after SP36 lands as before:
- `from cyclone.api import app` — used by `python -m cyclone serve`, by tests (`from cyclone import api` then `app.state.*`), by the test `conftest.py`.
- `app.state.event_bus`, `app.state.db`, `app.state.scheduler` — read by tests and by `api_routers/` (via `Request`).
The new `cyclone.api_routers` package exports `routers: list[APIRouter]`. Anything in `api_routers/` that needs to be imported from outside (e.g. a router-level fixture in a future test) is reachable as `from cyclone.api_routers.parse import router`, etc.
---
## 4. Data flow & error handling
**Data flow (per request):**
1. FastAPI receives the request, runs the `matrix_gate` dependency (auth + role check), then dispatches to the matching route in the matching router.
2. The route handler validates inputs (path params, query params, body via Pydantic if a body model exists).
3. The handler calls `cyclone.store.<method>(...)` for write paths and reconciliation, or opens a `db.SessionLocal()() for one-off reads` (existing pattern; this is how the dashboard KPIs, the activity feed, the configuration endpoints, and the live-tail snapshot reads work today).
4. The result is serialized via `to_ui_<entity>` (from `cyclone.store.ui`) for entity-shaped responses, or via a small local serializer for non-entity shapes (e.g. the dashboard KPI shape, the reconciliation summary, the eligibility 271 response).
5. The handler returns `JSONResponse`, `StreamingResponse` (NDJSON), or `Response` (CSV for `/api/inbox/export.csv`).
6. **Streaming endpoints** (`/api/claims/stream`, `/api/remittances/stream`, `/api/activity/stream`) keep the snapshot-then-live-subscription two-phase pattern. `_ndjson_line` and `_tail_events` come from `api_helpers.py`; the matching router imports them.
7. **The event bus contract is unchanged.** Every write through `cyclone.store.add(...)` still publishes `claim_written` / `remittance_written` / `activity_recorded` on the in-process `EventBus`. The stream-endpoint routers subscribe to those exact event names. The SP doesn't introduce new event kinds, doesn't rename existing kinds, doesn't change payloads.
**Error handling:**
- Routers raise `HTTPException` for client errors. Same status codes, same `detail` strings, same `error` envelope (`{error, detail}`). No change.
- The two global exception handlers stay in `api.py` (D2 of brainstorming): `@app.exception_handler(HTTPException)` re-emits with the cyclone error envelope, `@app.exception_handler(Exception)` logs and returns 500.
- Streaming endpoints catch domain exceptions and yield `{"type": "error", "data": {"message": ...}}` NDJSON chunks instead of raising — existing pattern, preserved per router.
- `StoreNotFound` / `StoreConflict` / `StoreInvalidState` are translated to 404 / 409 / 409 by the existing translation path. No change.
- The auth path (`matrix_gate` dep) returns 401/403 unchanged.
**No behavior change anywhere in the data flow or error path.** The SP is a structural move, not a correctness pass.
---
## 5. Testing approach
### 5.1 Per-router cycle (live-test + autoreview + commit)
For each router extracted (one `feat(sp36): extract {name} router` commit):
1. **Pre-flight pytest baseline.** From the project root:
```
cd backend && .venv/bin/pytest --tb=line -q 2>&1 | tee /tmp/refactor-{N}-pre.txt | tail -3
```
Capture the `XXX passed, YY failed, ZZ skipped` line. Expect roughly `~1,176 passed, ~1 failed (pre-existing isolation flake), ~10 skipped` per the SP21 store-split spec baseline.
2. **Cut.** Move the route handler(s) + their single-router helpers to the new router module, register the router in `api_routers/__init__.py`, delete the moved code from `api.py`, update imports. No logic change.
3. **Post-cut pytest diff.** Same command as (1), output to `/tmp/refactor-{N}-post.txt`. Diff:
```
diff <(awk '/passed|failed|skipped/ {print}' /tmp/refactor-{N}-pre.txt) \
<(awk '/passed|failed|skipped/ {print}' /tmp/refactor-{N}-post.txt)
```
Required: zero diff. Any delta = revert the cut and investigate.
4. **Live test.** Server is running in `cyclone-backend-1` on the host. Hit one representative route from the moved router. Required status codes:
- GET endpoints → 200 (or the documented non-200, e.g. 401 if auth is mocked off in the test environment)
- POST endpoints → 200, 201, 400, 401, 403, 409 (whatever the existing happy path / known-error path is)
- Streaming endpoints → 200 with `Content-Type: application/x-ndjson` and a valid first `snapshot_end` chunk
5. **Autoreview.** Spawn a `pr-reviewer` subagent on the staged diff. The reviewer prompt enumerates: (a) the new router is registered in `api_routers/__init__.py`; (b) no orphan imports left in `api.py`; (c) no `session.add` / `s.add` / `session.commit` / direct `db.SessionLocal()() call that does a write` appears in any router file (reads are OK); (d) the router's `dependencies=[Depends(matrix_gate)]` matches the original; (e) docstrings are preserved. Reviewer returns a one-line verdict (PASS / FAIL: <reason>). On FAIL, fix and re-review.
6. **Commit.** `feat(sp36): extract {name} router` per the SP-N convention.
7. **Log** to `/tmp/refactor-cyclone.md`:
```
[step N] extracted {name}
pre: passed={X} failed={Y} skipped={Z}
post: passed={X} failed={Y} skipped={Z}
live: GET /api/{route} → {code} | POST /api/{route} → {code}
reviewer: PASS | FAIL: <reason>
```
### 5.2 Pre-merge integration test
After all 19 routers are extracted, the full suite runs:
- `cd backend && .venv/bin/pytest --tb=line -q` — full backend suite, must match Step 0 baseline to the digit
- `cd backend && .venv/bin/pytest tests/test_api_*.py -v` — every API integration test, all green
- `cd .. && npm test` — frontend suite, all green
- `cd .. && npm run typecheck` — frontend typecheck clean
- `cd .. && npm run build` — frontend build green
- `cd .. && npm run lint` — frontend lint clean
### 5.3 Pre-merge one-shot verification (D8 of brainstorming)
These commands run, and all must pass, before the merge commit is created:
- `wc -l backend/src/cyclone/api.py` — expect ≤ 300
- `wc -l backend/src/cyclone/api_routers/*.py | sort -n` — expect no router over 1,400 LOC (admin is the largest at ~1,200)
- `grep -rn "session.add\|s\.add(\|session\.commit" backend/src/cyclone/api_routers/` — expect zero hits (writes go through `cyclone.store` only)
- `grep -rn "from cyclone.api_routers" backend/src/cyclone/api.py` — expect one hit (the `routers` import in `__init__` block)
- `python -c "from cyclone.api_routers import routers; print(len(routers))"` — expect ≥ 18 (5 existing + 14 new, minus whatever routers ended up merged during the SP)
- Live matrix — for every URL prefix in Section 3.2, `curl -s -o /dev/null -w "%{http_code}\n" http://192.168.0.49:8080/api/{prefix}/...` returns 2xx (or the documented error code)
- `git grep -n "@app\." backend/src/cyclone/api.py` — expect zero hits (every route decorator now lives in a router)
### 5.4 No new tests
The existing test suite is the regression net. We do not add new tests, do not modify existing tests, do not add new fixtures. The pre/post pytest baseline diff is the test for the SP.
---
## 6. Threat model (post-SP24 alignment)
The auth boundary is the HTTP layer: login required, bcrypt + HttpOnly session cookie. The new routers mount under `matrix_gate` (same as today), so every moved route stays auth-gated. No route is exposed that wasn't exposed before. No auth check is weakened.
The file-system threat model is unchanged: SQLCipher at rest (when the macOS Keychain entry + `sqlcipher3` are both present, otherwise plain SQLite), secrets in the macOS Keychain via `keyring`, no secrets on disk in plaintext. SP36 doesn't touch the DB layer, the secrets module, or the configuration loader.
LAN-only by design. The auth boundary is the network boundary; the production deployment is the host firewall + compose port publishing. Don't expose the published ports to the WAN — unchanged.
---
## 7. Risks & mitigations
| Risk | Likelihood | Impact | Mitigation |
|---|---|---|---|
| A route handler silently changes behavior during the cut (typo, off-by-one, wrong import) | medium | high | Pre/post pytest diff is the test. The diff must be zero; any delta reverts. |
| A helper gets duplicated instead of moved (one copy in old location, one in new) | low | medium | Autoreview explicitly checks for orphan imports in `api.py` and for `def _helper_name` definitions appearing twice. |
| The `api_routers/__init__.py` registration misses a router and a route 404s in production | low | high | Pre-merge one-shot verification §5.3 includes a live matrix that hits every URL prefix. |
| The `matrix_gate` dep is dropped from one route during the cut | low | critical | Autoreview explicitly checks `dependencies=[Depends(matrix_gate)]` matches the original on every moved route. |
| Two routers end up importing each other (creates a cycle) | low | medium | The convention is "routers import only `cyclone.store`, `cyclone.api_helpers`, `cyclone.api_routers._shared`" — autoreview checks for any other cross-router import. |
| A `from cyclone.api import` somewhere in the codebase breaks because we removed a top-level symbol | low | high | The external import surface (D6 of brainstorming) is the test: `git grep "from cyclone.api import"` before and after must be identical in symbol set. |
| The merge of SP35 into main fails or introduces conflicts | low | high | D7 of brainstorming: SP35 merge is a hard pre-condition; if it doesn't land cleanly, SP36 doesn't start. |
| A streaming endpoint regresses silently (returns the right status code but the wrong first chunk) | medium | medium | The live test for streaming endpoints asserts `Content-Type: application/x-ndjson` and parses the first `snapshot_end` line, not just the HTTP code. |
| The frontend test suite picks up a backend change (e.g. a renamed JSON field that the UI consumes) | very low | medium | The refactor is byte-for-byte transparent to clients; if a frontend test fails, that's a real regression and we investigate. |
---
## 8. Rollout
1. **Merge SP35 into main.** Fast-forward or merge commit (no squash, no rebase). The branch `sp35-parse-input-guards` has 4 reviewable commits and a clean working tree.
2. **Restart compose if needed.** `docker compose restart cyclone-backend-1` after the SP35 merge so the running container reflects main.
3. **Create the SP36 branch.** `git checkout -b sp36-api-routers-split` from main.
4. **Extract routers, one per commit.** Order: lowest-risk first (admin, health, claim_acks, ta1_acks already exist; then the small leaf routers `dashboard.py`, `eligibility.py`, `payers.py`, `config.py`; then the medium ones `reconciliation.py`, `remittances.py`, `activity.py`, `clearhouse.py`, `providers.py`; then the larger ones `inbox.py`, `batches.py`, `claims.py`; finally `parse.py` since it's the most coupled to the store facade). Each step follows §5.1.
5. **Run the §5.2 integration tests.**
6. **Run the §5.3 one-shot verification.**
7. **Open the PR.** Title: `SP36 API Routers Split`. Body: links to this spec, the SP36 plan (when written), and a checklist of every router extracted.
8. **Merge after review.** Single atomic `merge: SP36 api-routers-split into main` commit, no squash, no rebase.
---
## 9. Open questions
None at spec time. The brainstorming Q&A resolved all of:
- Which file to split? `api.py`. Confirmed.
- What shape? Per-resource routers mirroring the store's domain boundaries. Confirmed.
- Branch from where? Merge SP35 first, then from main. Confirmed.
- Where do the cross-router helpers go? `api_routers/_shared.py` (private). Confirmed.
- `admin.py` — one router or split further? One router for this SP, follow-up later. Confirmed.
- How granular are the commits? One router per commit. Confirmed.
@@ -0,0 +1,139 @@
# Sub-project 35 — Parse Input Guards: Design Spec
**Date:** 2026-07-06
**Status:** Draft, awaiting user sign-off
**Branch:** `sp35-parse-input-guards` (off `main`, post-SP33)
**Aesthetic direction:** No UI changes. The Upload page gets a small invisible behavior change (auto-flip the `Kind` select when the file content disagrees) and keeps its current visual design.
---
## 1. Scope
Today, dropping any X12 file on the Upload page with the default `Kind: 837P` submits the file to `POST /api/parse-837` regardless of whether the bytes look like an 837P or an 835. The `/api/parse-837` endpoint silently accepts that submission: the parser reads the ISA/ST envelope, finds zero CLM segments (because 835s carry `CLP`, not `CLM`), returns an empty `ParseResult`, and the endpoint then persists a `kind='837p'` batch row with `total_claims=0`. The bogus batch shows up in the Batches view, no remittance or claim rows are produced, and the operator has no way to tell from the UI that the ingest was misrouted. SP34 left two such bogus batches in production (`50eb50c1…` and `e4692571…`) when an operator (or test) dropped a `tp11525703-835_M019771179-20260706005516577-1of1.x12` file at the project root and ran the upload twice without changing the default `Kind`.
SP35 closes two independent layers that both contribute to the silent-corruption path. The fix is layered (defense in depth): either layer alone would prevent the symptom, but each layer is a separate invariant that needs its own test and its own enforcement.
**Investigation finding (shapes the scope):** I read each parser before locking the scope. `/api/parse-837` and `/api/parse-835` are the only parse endpoints where the parser can return a successful-looking empty result. The 999/277CA/TA1 endpoints already enforce envelope/segment guards at the parser layer:
- **`parse_999`** (`backend/src/cyclone/parsers/parse_999.py:289-290`): raises `CycloneParseError("No AK9 (Functional Group Response Status) segment found")` when the parser finds no `AK9` segments. There is no silent-corruption path because 999s that don't carry `AK9` (everything that isn't a 999) get rejected.
- **`parse_277ca`** (`backend/src/cyclone/parsers/parse_277ca.py:297-298`): raises `"Expected ST*277 or ST*277CA, got ST*<other>"` when the envelope token doesn't match. Strict envelope guard.
- **`parse_ta1`** (`backend/src/cyclone/parsers/parse_ta1.py:111`): raises `"Expected TA1, got <other>"` when the first segment after ISA isn't `TA1*`. Strict envelope guard.
The right scope is therefore: **fix the two endpoints that have the bug** (837p and 835), and **add regression tests** to the three endpoints that already have strict guards so we catch any future regression where someone loosens the parser.
**In scope:**
- **Server-layer input guard on `POST /api/parse-837`.** Two checks before any `store.add(...)` runs:
1. **Envelope check.** The first `ST*` segment (or first 4096 bytes, whichever comes first) must begin with `ST*837`. If not, the endpoint returns `400` with `{error: "Mismatched file kind", detail: "...", expected: "837p", detected_st: "<token>"}` and persists nothing.
2. **Empty-claims check.** After the parser runs, if `len(result.claims) == 0`, the endpoint returns `400` with `{error: "No claims parsed", detail: "..."}` and persists nothing.
- **Same two checks on `POST /api/parse-835`**, mirrored for symmetry. The 835 parser requires `BPR` and `TRN` (raises `CycloneParseError` if either is missing) but does NOT require `CLP` segments — a stripped-down 835 with `ISA*BPR*TRN*SE*GE*IEA` and no `CLP` would silently persist as an empty `claims=[]` batch today. The new envelope guard catches the "this is an 837 file" misroute; the empty-claims guard catches this "valid-835-shape but no claims" edge case.
- **Regression tests on `POST /api/parse-999`, `POST /api/parse-277ca`, `POST /api/parse-ta1`** that assert these endpoints already return 400 (not 200) when fed an unrelated X12 file. These lock the current strict-parser behavior in — if a future PR loosens the parser envelope guards, these tests fail and prevent a recurrence of the SP34-class bug.
- **UI-layer auto-detect in `src/pages/Upload.tsx`.** When the user drops or selects a file, the page reads the first ~4 KB of the file as text, looks for the first `ST*…` token, and if it finds `ST*837` the `Kind` select flips to `837P` (no-op if already there), and if it finds `ST*835` the select flips to `835`. If neither token is found (a `.txt` export without an `ST*` header, or a PDF, or a corrupted file) the select stays where it is and the user keeps manual control. (999/277CA/TA1 don't enter the UI flow today — the Upload page only has 837p/835 in the Kind select — so no UI auto-detect on those kinds is needed.)
- **Backend tests** covering the four new guards on `/api/parse-835` (mirroring parse-837) and the three regression tests on the 999/277CA/TA1 endpoints.
- **Frontend test** in `src/pages/Upload.test.tsx` covering the auto-detect for both 837 and 835 file payloads, plus a "no ST* found → no flip" regression case.
- **Cleanup of the two existing bogus batches** from the production DB (`50eb50c1…` and `e4692571…`) by direct SQL after the fix lands and is verified in production.
**Out of scope:**
- No schema migration. No new tables, no new columns, no new foreign keys.
- No changes to the `parse_837` / `parse_835` parser modules themselves (the empty-claims check is at the API layer, where the `store.add(...)` decision lives; moving it into the parsers would change CLI semantics).
- No changes to any CLI subcommand. The CLI `parse-837` and `parse-835` already raise `click.UsageError` on empty claims (consistent with what SP35 makes the API do); the SP only closes the API-vs-CLI symmetry.
- No changes to the 999 / 277CA / TA1 / 270 / 271 endpoints. They each have their own parsers; if a similar bug exists on them it is out of scope for SP35 and should be filed as a follow-up if confirmed.
- No new admin endpoints for batch deletion. Cleanup is a one-line direct SQL run via `docker exec cyclone-backend-1 sqlite3 …` and is documented in the plan, not in the product surface.
- No `cyclone admin` changes.
- No changes to the activity-events flood (a separate observation from the same incident — one 835 produced hundreds of per-claim `reconcile` activity events; a real concern but a separate ticket).
- No change to the auth boundary. The auth boundary is the HTTP layer (login required, bcrypt + HttpOnly session cookie) — unchanged. The new 400 responses on `/api/parse-837` and `/api/parse-835` are produced under `matrix_gate`, same as today.
---
## 2. Decisions (locked during brainstorming)
### D1. Two-layer defense: server guards AND UI auto-detect. Each one alone is insufficient.
If we fix only the server, the symptom goes away but the operator still gets a confusing 400 with no explanation for why their drop didn't work — every bad drop produces a 400 instead of a 200 with a silent empty batch. Fixing the UI without the server leaves the API as a footgun for any other client (curl, third-party tooling, future ingestion paths). Both layers are needed because the server guard is the **invariant** (correctness) and the UI auto-detect is the **operator experience** (correct-by-default UX).
### D2. The server check is "ST* token + empty claims," not just one or the other
Relying on `ST*` alone fails on truncated headers (the parser can't even reach the ST if the ISA is corrupt, so the message becomes a generic 400). Relying on empty-claims alone leaves the door open for empty-but-valid-ISA 999 files to silently hit the 837p endpoint and produce empty batch rows (the bug we're closing). The two checks together produce a clear, two-stage error: "your file's ST token isn't 837p" before parse, or "your 837p parse produced zero claims" after parse.
### D3. The UI auto-detect reads the first 4 KB of the file, not the whole file
X12 `ST*837` / `ST*835` always appears in the first few hundred bytes of a well-formed file (after ISA and GS). Reading 4 KB guarantees capture without loading multi-megabyte files into memory in the browser. The check runs in `pickFile()` synchronously on the dropped `File` object via `FileReader.readAsText(file.slice(0, 4096))`.
### D4. The auto-detect never overrules a user who has manually picked `835`
If the user explicitly selects `835` from the dropdown, then drops a file whose content says `ST*837`, the auto-detect still wins (it overrides the select). Rationale: the whole point of the auto-detect is to prevent the silent-corruption failure mode; honoring a stale manual override would defeat the purpose. If the operator wants to "force 837p on an 835 file" — a legitimate test case — they can flip the select back manually after the auto-detect fires; the file is still in `stream.file` and the parse button hasn't been clicked yet.
### D5. Cleanup is direct SQL, not a new admin endpoint
The two bogus batches (`50eb50c1…` and `e4692571…`) have `kind='837p'`, `input_filename` of the 835 file, `total_claims=0`, and zero `claims` / `service_line_payments` / `cas_adjustments` / `matches` rows pointing at them. A one-shot `DELETE FROM batches WHERE id IN (...)` is sufficient and does not require a new product surface. The plan documents the exact command and where to run it (`docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db "..."`). A future admin-batch-delete endpoint can be a separate SP if there's operator demand.
### D6. No 409 vs 400 nuance — every bad input is a 400
The existing endpoint returns `400` for `Empty file`, `Encoding error`, and `Parse error`; it returns `409` for `Duplicate claim` (a state conflict, not an input error). SP35's new failures (`Mismatched file kind`, `No claims parsed`) are input errors, so they get `400`. The error envelope shape (`{error, detail}`) is preserved.
### D7. No new audit-log entry for "rejected at ingest"
The existing audit log captures state-affecting actions (admin role changes, batch deletes, etc.), not failed parse attempts. Adding `parse_rejected` events would conflate operational noise with the user-action audit trail. If observable rejection events matter in the future, they can be a separate SP that adds a structured log channel.
### D8. Frontend test lives in `src/pages/Upload.test.tsx` and uses the existing `vi.mock("@/lib/api", …)` pattern
The new behavior is a thin synchronous change to `pickFile()`; the test mocks `FileReader` (or uses `Blob` directly via `URL.createObjectURL` + `<input>` event firing) and asserts the resulting `kind` state. No new test framework, no new mocking library.
---
## 3. Edge cases & how they're handled
| Scenario | Behavior |
|---|---|
| Drop a valid 837 file with default `kind='837p'` | No change. Already correct; auto-detect is a no-op. |
| Drop a valid 835 file with default `kind='837p'` | Auto-detect flips `kind` to `835`. Parse succeeds. |
| Drop a valid 835 file with `kind='835'` already selected | Auto-detect is a no-op. Parse succeeds. |
| Drop a 999 file with default `kind='837p'` | Auto-detect finds no `ST*837` and no `ST*835`, so leaves `kind` alone. Parse fails with `Mismatched file kind` 400. |
| Drop a non-X12 `.txt` with default `kind='837p'` | Same as above: no auto-detect, parse fails clearly. |
| Operator manually selects `835` then drops an 837 | Auto-detect flips to `837p` (D4). Parse succeeds. |
| Bypass the UI entirely and POST an 835 file to `/api/parse-837` via curl | Server guard rejects with `400 Mismatched file kind`. No batch row persisted. |
| Bypass the UI and POST a valid 837 with zero CLM segments (empty ISA envelope) to `/api/parse-837` | Server guard rejects with `400 No claims parsed`. No batch row persisted. |
| Re-ingest the same valid 835 to `/api/parse-835` (duplicate) | Existing `409 Duplicate remittance` flow unchanged. SP35 does not touch the dedup logic. |
| User clicks `Parse file` twice rapidly with the same file | First request persists; second request hits dedup and gets `409`. Unchanged. |
| Frontend auto-detect reads a 4 KB slice that straddles a multi-segment envelope | Unlikely on real EDI; the `ST*` always appears in the first ~500 bytes after ISA/GS. Regression test covers the `ST*835` at byte 3800 case. |
---
## 4. Testing approach
**Backend (Python):** add to existing `tests/test_api.py` (`/api/parse-837`), `tests/test_api_835.py`, `tests/test_api_999.py`, `tests/test_api_277ca.py`, `tests/test_api_ta1.py`:
- **`test_parse_837_endpoint_rejects_835_input`** — POST the CO Medicaid 835 fixture to `/api/parse-837`, assert `400`, error envelope `error == "Mismatched file kind"`, no new `batches` row.
- **`test_parse_837_endpoint_rejects_empty_envelope`** — POST a syntactically valid ISA with no CLM segments to `/api/parse-837`, assert `400`, error envelope `error == "No claims parsed"`, no new `batches` row.
- **`test_parse_835_endpoint_rejects_837_input`** — symmetric: POST 837 to `/api/parse-835`, expect `400 Mismatched file kind`.
- **`test_parse_835_endpoint_rejects_empty_envelope`** — symmetric: POST a stripped ISA+BPR+TRN+SE+GE+IEA with no `CLP` to `/api/parse-835`, expect `400 No claims parsed`.
- **`test_parse_999_endpoint_rejects_837_input`** — regression lock: POST 837 fixture to `/api/parse-999`, expect `400 Parse error` (raised by the parser's `No AK9` rule). Documents that the 999 parser already has this invariant enforced.
- **`test_parse_277ca_endpoint_rejects_835_input`** — regression lock: POST 835 fixture to `/api/parse-277ca`, expect `400 Parse error` (raised by the parser's `Expected ST*277` rule).
- **`test_parse_ta1_endpoint_rejects_835_input`** — regression lock: POST 835 fixture to `/api/parse-ta1`, expect `400 Parse error` (raised by the parser's `Expected TA1` rule).
- **`test_parse_837_endpoint_happy_path`** (existing) — regression guard: CO Medicaid 837p fixture still parses and persists cleanly.
- **`test_parse_835_endpoint_happy_path`** (existing) — regression guard: CO Medicaid 835 fixture still parses and persists cleanly.
**Frontend (Vitest):** add to `src/pages/Upload.test.tsx` (sibling file exists per the project convention):
- **`upload_auto_detect_837_flips_kind_to_837p`** — drop a file whose first 4 KB contain `ST*837`, assert `kind` state is `837p`.
- **`upload_auto_detect_835_flips_kind_to_835`** — drop a file whose first 4 KB contain `ST*835`, assert `kind` state is `835`.
- **`upload_auto_detect_no_st_token_leaves_kind_unchanged`** — drop a file with no `ST*` header, assert `kind` state is whatever the user had selected.
**Manual smoke (live stack):**
- Drop the `tp11525703-835_M019771179-20260706005516577-1of1.x12` file at the project root with default `Kind: 837P` selected. Expect: select flips to `835`, parse succeeds, 1148 claim rows land in `remittances`. Repeat with `Kind: 835` already selected. Expect: no flip, same outcome.
- POST the same 835 file via curl to `/api/parse-837`. Expect: `400 Mismatched file kind`, no batch row written.
---
## 5. Out-of-scope reminders (explicit)
- **No new guards on `/api/parse-270` or `/api/parse-271`.** Those endpoints handle eligibility-benefit pairs. They may or may not have the same parse-empty shape; out of scope for SP35. (Filing as a follow-up if confirmed.)
- **No production-side changes to the 999 / 277CA / TA1 endpoint code.** Only regression tests are added; the parsers themselves already reject mismatched input.
- **No `cyclone admin batches delete` endpoint.** Cleanup is direct SQL for now.
- **No schema migration.** The data model is unchanged.
- **No changes to validation rules** in `cyclone.parsers.validator_837` / `cyclone.parsers.validator_835`. SP35 only adds guards at the API layer.
- **No change to the activity-events storm.** This is a perf/observability concern, not a correctness bug, and will get its own ticket.
- **No auth changes.** The new 400 responses inherit `matrix_gate` and the existing session-cookie auth boundary.
- **No changes to the running production stack.** SP35 is developed against `main`, merged in, then the running containers are restarted via the existing compose-up flow documented in `RUNBOOK.md`. No hot-patches.
+148
View File
@@ -0,0 +1,148 @@
import { describe, expect, it } from "vitest";
import {
detectKindFromText,
detectKindFromFile,
detectedKindToParsedBatchKind,
} from "./x12-detect";
// Minimal X12 envelopes — just enough to exercise the ST01 match.
// Real fixtures live in backend/tests/fixtures; we use synthetic ones
// here because the frontend doesn't need the full file content, just
// the first ~4KB.
const ISA_837P = (
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
"*260617*1937*^*00501*000000001*0*P*:~" +
"GS*HC*SENDER*RECEIVER*20260706*1937*1*X*005010X222A1~" +
"ST*837*0001*005010X222A1~"
);
const ISA_835 = (
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
"*260617*1937*^*00501*000000001*0*P*:~" +
"GS*HP*SENDER*RECEIVER*20260706*1937*1*X*005010X221A1~" +
"ST*835*0001~"
);
const ISA_999 = (
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
"*260617*1937*^*00501*000000001*0*P*:~" +
"GS*FA*SENDER*RECEIVER*20260706*1937*1*X*005010X231A1~" +
"ST*999*0001~"
);
const ISA_277CA = (
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
"*260617*1937*^*00501*000000001*0*P*:~" +
"GS*HN*SENDER*RECEIVER*20260706*1937*1*X*005010X214~" +
"ST*277*0001*005010X214~"
);
const ISA_277CA_QUALIFIED = (
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
"*260617*1937*^*00501*000000001*0*P*:~" +
"GS*HN*SENDER*RECEIVER*20260706*1937*1*X*005010X214~" +
"ST*277CA*0001*005010X214~"
);
const TA1_FILE = (
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
"*260520*1750*^*00501*000000001*0*P*:~" +
"TA1*000000001*20260520*1750*A*000*20260520~" +
"IEA*1*000000001~"
);
describe("detectKindFromText", () => {
it("detects ST*837* as 837p", () => {
expect(detectKindFromText(ISA_837P)).toBe("837p");
});
it("detects ST*837P* as 837p (professional qualifier)", () => {
const withQualifier = ISA_837P.replace("ST*837*", "ST*837P*");
expect(detectKindFromText(withQualifier)).toBe("837p");
});
it("detects ST*835* as 835", () => {
expect(detectKindFromText(ISA_835)).toBe("835");
});
it("detects ST*999* as 999", () => {
expect(detectKindFromText(ISA_999)).toBe("999");
});
it("detects ST*277* as 277ca", () => {
expect(detectKindFromText(ISA_277CA)).toBe("277ca");
});
it("detects ST*277CA* as 277ca (qualified form)", () => {
expect(detectKindFromText(ISA_277CA_QUALIFIED)).toBe("277ca");
});
it("detects TA1 envelope as ta1 (no ST, bare TA1 segment)", () => {
expect(detectKindFromText(TA1_FILE)).toBe("ta1");
});
it("returns unknown for garbage input", () => {
expect(detectKindFromText("not edi at all")).toBe("unknown");
});
it("returns unknown for an empty string", () => {
expect(detectKindFromText("")).toBe("unknown");
});
it("is case-insensitive on ST segment", () => {
// Real X12 is uppercase, but be lenient — files from various tools
// sometimes have mixed case.
expect(detectKindFromText(ISA_835.toLowerCase())).toBe("835");
});
it("does not false-positive on ST*8370 (5 chars after ST*)", () => {
// Catches a regression where the matcher would substring-match too
// eagerly (e.g. matching ST*837 against ST*8370*). The require is
// that the char after the digits is `*`, not another digit.
const noise = "ISA*00*...*ST*8370*0001~";
expect(detectKindFromText(noise)).toBe("unknown");
});
});
describe("detectedKindToParsedBatchKind", () => {
it("maps 837p → 837p", () => {
expect(detectedKindToParsedBatchKind("837p")).toBe("837p");
});
it("maps 835 → 835", () => {
expect(detectedKindToParsedBatchKind("835")).toBe("835");
});
it("returns null for 999 (Upload page doesn't ingest 999s)", () => {
expect(detectedKindToParsedBatchKind("999")).toBeNull();
});
it("returns null for 277ca", () => {
expect(detectedKindToParsedBatchKind("277ca")).toBeNull();
});
it("returns null for ta1", () => {
expect(detectedKindToParsedBatchKind("ta1")).toBeNull();
});
it("returns null for unknown", () => {
expect(detectedKindToParsedBatchKind("unknown")).toBeNull();
});
});
describe("detectKindFromFile", () => {
it("detects 837p from a File blob", async () => {
const file = new File([ISA_837P], "test.x12", { type: "text/plain" });
expect(await detectKindFromFile(file)).toBe("837p");
});
it("detects 835 from a File blob (the original SP35 repro)", async () => {
// The exact scenario the user hit: drop an 835 file on the Upload
// page while the dropdown still says "837p". The helper must
// return "835" so the UI can switch the dropdown before the user
// hits Parse.
const file = new File([ISA_835], "tp11525703-835.x12", { type: "text/plain" });
expect(await detectKindFromFile(file)).toBe("835");
});
it("returns unknown on an empty file (no crash)", async () => {
const file = new File([""], "empty.x12", { type: "text/plain" });
expect(await detectKindFromFile(file)).toBe("unknown");
});
});
+98
View File
@@ -0,0 +1,98 @@
/**
* X12 file-kind auto-detection helper.
*
* SP35: the Upload page used to default to "837p" silently — dropping an 835
* file on the page while the dropdown still said "837p" routed the file to
* /api/parse-837, which silently persisted an empty batch. Layer A of the
* defense-in-depth fix is to detect the kind from the file's first few KB
* and switch the dropdown automatically before the user clicks Parse.
*
* Layer B is the server-side envelope check (cyclone.api._transaction_set_id_from_segments).
* This helper is purely advisory: if it returns null, the UI keeps whatever
* the user picked and the server guard rejects with a clean 400 if it's wrong.
*
* Detection is intentionally cheap: read the first ~4KB and look for the ST
* segment (or the bare TA1 segment, which has no ST envelope). The full file
* is never loaded — X12 envelopes are always within the first ISA segment.
*/
import type { ParsedBatchKind } from "@/types";
/** Number of bytes to read from the start of the file for detection. */
const DETECT_BYTES = 4096;
export type DetectedKind = ParsedBatchKind | "999" | "277ca" | "ta1" | "unknown";
export function detectKindFromText(text: string): DetectedKind {
// Strip whitespace and look for the ST segment.
// X12 envelopes are always within the first segment, so we don't need
// to scan far. The segment terminator is `~` (per the ISA segment,
// which we also don't need to parse here — we only care which kind of
// payload follows).
//
// We do a simple substring search rather than full tokenization because:
// - Detection must run before the user even knows if their file is
// valid X12. If we ran `tokenize()` and the file has a malformed
// ISA, we'd throw and the UI couldn't show a helpful message.
// - A simple prefix match on `ST*<kind>*` is enough to disambiguate
// every kind the Upload page might receive. False positives are
// caught by the server-side guard.
const upper = text.toUpperCase();
// 277CA uses ST*277* or ST*277CA* (per parse_277ca.py line 13 comment).
// Check 277CA before 277 because ST*277CA contains "277".
if (upper.includes("ST*277CA*")) return "277ca";
if (upper.includes("ST*277*")) return "277ca";
// 837P accepts ST*837* or ST*837P* (the trailing P is the professional
// claim qualifier, but the ISA envelope alone tells you it's a
// professional file). Check 837P before 837 for the same reason.
if (upper.includes("ST*837P*")) return "837p";
if (upper.includes("ST*837*")) return "837p";
// 835 has ST*835* — no other qualifier in common use.
if (upper.includes("ST*835*")) return "835";
// 999 ACK has ST*999* — no qualifier.
if (upper.includes("ST*999*")) return "999";
// TA1 has no ST envelope. The interchange-ack segment is the bare TA1*
// immediately after ISA/IEA. Match the segment header.
if (/\bTA1\*/.test(upper)) return "ta1";
return "unknown";
}
/**
* Browser-side helper: read the first DETECT_BYTES from a File and detect
* its kind. Returns a Promise so it composes naturally with FileReader /
* Blob.slice. Returns "unknown" on read error so the caller can keep the
* user's current selection and let the server guard surface a clean 400.
*/
export async function detectKindFromFile(file: File): Promise<DetectedKind> {
try {
const blob = file.slice(0, DETECT_BYTES);
const text = await blob.text();
return detectKindFromText(text);
} catch {
return "unknown";
}
}
/**
* Map a DetectedKind to the ParsedBatchKind the Upload dropdown supports.
* Returns null for kinds the Upload page can't ingest (999, 277CA, TA1) —
* the UI then surfaces a "this file isn't supported here" hint instead of
* silently misrouting it. The server-side guards in cyclone.api still
* catch any escape.
*/
export function detectedKindToParsedBatchKind(kind: DetectedKind): ParsedBatchKind | null {
switch (kind) {
case "837p":
return "837p";
case "835":
return "835";
default:
return null;
}
}
+33 -1
View File
@@ -33,6 +33,10 @@ import { StatPill, ValidationDot } from "@/components/ClaimCard/shared";
import { api, ApiError, type BatchSummary, type ParseProgress } from "@/lib/api";
import { downloadBlob } from "@/lib/download";
import { fmt, toNum } from "@/lib/format";
import {
detectKindFromFile,
detectedKindToParsedBatchKind,
} from "@/lib/x12-detect";
import { useAppStore } from "@/store";
import { useParse } from "@/hooks/useParse";
import { useBatchExport } from "@/hooks/useBatchExport";
@@ -437,9 +441,37 @@ export function Upload() {
// hook call above. Keeping the comment as a breadcrumb for future
// readers who grep "Auto-select".)
function pickFile(f: File | null) {
async function pickFile(f: File | null) {
setFile(f);
setStream({ items: [], expectedTotal: null, passed: 0, failed: 0 });
if (!f) return;
// SP35: auto-detect the kind from the file's first few KB and switch
// the dropdown before the user hits Parse. This is layer A of the
// defense-in-depth fix (layer B is the server-side envelope guard
// in cyclone.api). Before SP35 the dropdown defaulted to "837p"
// silently — dropping an 835 file on the page routed it to
// /api/parse-837 and produced a bogus empty batch. Detection runs
// asynchronously on a 4KB slice; if it can't determine the kind
// (unknown file, read error) we keep the user's current selection
// and the server guard will surface a clean 400 if it's wrong.
const detected = await detectKindFromFile(f);
const matched = detectedKindToParsedBatchKind(detected);
if (matched && matched !== kind) {
setKind(matched);
setPayer(matched === "837p" ? PAYERS_837[0]!.value : PAYERS_835[0]!.value);
toast.message(
`Detected ${matched === "837p" ? "837P" : "835"} file — switched the dropdown.`,
{ description: f.name },
);
} else if (detected === "999" || detected === "277ca" || detected === "ta1") {
// The Upload page only supports 837P and 835; for the other X12
// kinds (which have their own endpoints), surface a hint instead
// of silently leaving the dropdown on whatever the user picked.
toast.error(
`Detected ${detected.toUpperCase()} file — the Upload page only accepts 837P or 835.`,
{ description: "Use the matching endpoint from the History tab or the API." },
);
}
}
async function onParse() {