Files
cyclone/docs/superpowers/specs/2026-06-29-cyclone-remittances-architecture-refactor-design.md
Nora 454c3598b1 docs(spec): design for SP27 remittances architecture refactor
Tier 1: split scheduler.py into handlers/ subpackage, dedup helpers,
loosen INBOUND_RE, add SFTP operation timeouts, surface SFTP errors
in Scheduler.status().

Tier 2: unify 835 ingest + reconciliation into one critical section,
add GET /api/claims/{id}/chain, guard matched_remittance_id ↔
Remittance.claim_id invariant, emit claim.rejected_after_remit audit
when a 277CA rejection hits a matched claim.

Status: Draft, awaiting user sign-off.
2026-06-29 09:54:33 -06:00

392 lines
18 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Sub-project 27 — Remittances Architecture Refactor: Design Spec
**Date:** 2026-06-29
**Status:** Draft, awaiting user sign-off
**Branch:** `sp27-remittances-architecture-refactor`
**Aesthetic direction:** No new UI (chain view rendered into existing ClaimDrawer; no new page)
## 1. Scope
Cyclone today ships the full claim/ack/remit cycle end-to-end: outbound
837 submitted via SFTP, inbound 999 / 277CA / 835 / TA1 polled and parsed,
remits matched against claims. Live data is now flowing on the production
system (~60K remits, ~542K 999 acks since 06/19). The architecture serving
that cycle has accumulated rough edges that an afternoon of operational
pressure surfaced:
- **Scheduler is fragile.** A 06/25 silent hang (no error logged) took
the MFT poll down for ~3 days; recovered only because the operator
restarted the backend. Root cause: `sftp.listdir_attr()` runs without
a socket timeout, so when the MFT TCP-acks but stops responding the
thread blocks indefinitely. The scheduler's status surface is too
shallow to detect this in real time.
- **Filename classification is brittle.** `edi/filenames.py:INBOUND_RE`
requires a `_file_type.x12` suffix on inbound files. Gainwell's
actual 835s ship without that suffix (see the 5×835 batch from
6/156/19), so the only thing the scheduler could do was mark them
`skipped`. Operators had to ingest those by hand.
- **Helpers duplicate API logic.** `cyclone.scheduler._ack_count_summary`
and `_ack_synthetic_source_batch_id` copy logic from
`cyclone.api` with a comment saying so. Two places to keep in sync.
- **Reconciliation is decoupled from ingest.** 835 ingest writes a
`Remittance` with `adjustment_amount=0`, and a separate
`reconcile.run()` pass later sums `CasAdjustment.amount` rows and
overwrites `adjustment_amount`. The UI sees stale values until the
pass runs, and the two phases can drift if reconcile is skipped or
crashes mid-way.
- **No unified claim-chain view.** A claim's 837 submission, 999 ack,
277CA status, and 835 remit are reconstructed by joining across four
pages and three queries. The "did this remit's claim ever get
matched?" question requires manual SQL.
### In scope
Tier 1 (scheduler/ingest split, SFTP hardening, filename classifier):
1. Move the four per-file-type handlers (`999`, `835`, `277CA`, `TA1`)
out of `backend/src/cyclone/scheduler.py` into
`backend/src/cyclone/handlers/<type>.py`. Each handler exposes one
`handle(text, source_file) -> (parser_used, claim_count)` function.
2. Pull `_ack_count_summary`, `_ack_synthetic_source_batch_id`, and
`_277ca_synthetic_source_batch_id` out of `scheduler.py` and out of
`api.py` into a new `backend/src/cyclone/handlers/_ack_id.py`. Both
callers import from one module.
3. Loosen `edi/filenames.py:parse_inbound_filename` to accept filenames
lacking `_file_type.x12`; fall back to `orig_tx` as the
provisional `file_type`, and accept the file as either `835` (if
`orig_tx` ends in `835`) or `999`/`277CA` (if `orig_tx` does).
4. Wrap `clearhouse.SftpClient._list_inbound_paramiko` /
`_list_inbound_names_paramiko` / `_download_inbound_paramiko` /
`_read_file_paramiko` calls in `asyncio.wait_for(... timeout=N)`
via `asyncio.to_thread`, with `N` configurable via
`CYCLONE_SFTP_OP_TIMEOUT_SECONDS` (default 30s).
5. Surface SFTP errors in `Scheduler.status()``last_error_at`,
`last_error`, `consecutive_failures`, `last_sftp_attempt_at`.
When `consecutive_failures >= 3`, the operator pill flips to a
destructive state until a successful tick clears it.
6. Stop swallowing `IOError` from `sftp.listdir_attr()` — return the
error in `result.errors` and let the scheduler record `last_error`.
Tier 2 (reconciliation atomicity, chain view, invariant guards):
7. Unify 835 ingest + `reconcile.run()` into a single critical
section inside `cyclone.handlers.handle_835`. The whole flow
(parse → validate → persist batch + remits + CasAdjustments → match
claims → write back `adjustment_amount` and `matched_remittance_id`)
happens in one `db.SessionLocal()()`. The `Remittance.adjustment_amount`
is computed in the same transaction as the `CasAdjustment` rows.
8. Add `GET /api/claims/{id}/chain` returning the claim's full chain
in one response: `submission` (837 → `Claim` row),
`ack_999` (matching `Ack` rows), `ack_277ca` (matching
`Two77caAck` rows), `remittance` (`Remittance` row including
adjustments and matched claim FK). Empty slots when a piece is
missing — never 404 the whole endpoint just because one piece is
absent.
9. The 999 ack handler and 277CA ack handler — when they reject a
claim, and the claim is already matched to a remit — emit an
`ActivityEvent` `claim.rejected_after_remit` so the operator
surfaces the conflict in the Activity page rather than silently
breaking the manual-match workflow.
10. Verify the denormalized pair `Claim.matched_remittance_id`
`Remittance.claim_id` is written in the same transaction on every
write-path (`store.manual_match`, `store.manual_unmatch`, the new
atomic 835 handler). Add a startup invariant check: if any Claim
has `matched_remittance_id` but the matching Remittance's
`claim_id` is unset (or vice versa), log a structured error so an
operator sees the drift in the logs.
### Out of scope (deferred to future increments)
- **Frontend scheduler operator UI** (start/stop/tick/status page,
reconnect button, live log tail). The `/api/admin/scheduler/*`
JSON endpoints remain sufficient; the operator uses curl.
- **Multi-tenant SFTP blocks** (per-payer or per-tenant). Single
dzinesco block remains.
- **Docker-secret `_FILE` conventions** beyond what SP26 already added.
- **Outbound file pickup verification** (polling HPE's MFT to
confirm the operator-dropped 837 was received).
- **Frontend chain drawer component.** The `GET /api/claims/{id}/chain`
endpoint is exposed; rendering it inside the existing
`ClaimDrawer` is a separate UI increment.
## 2. Decisions (locked during brainstorming)
1. **Branch base is `Version-1.0.0`, not `main`.** `main` is at
`74aa64f` (SP26 only) and is missing the SP25+26 SFTP work, the
`[sftp]` docker extra, and the permission-matrix fixes. The
production system runs `Version-1.0.0`, so the refactor lands
there. A subsequent forward-merge from `Version-1.0.0``main`
can pull SP27 in if the operator wants it on main.
2. **One module per handler, not a class.** Each handler exposes one
pure function `handle(text: str, source_file: str) -> HandleResult`,
where `HandleResult` is a small dataclass with `parser_used`,
`claim_count`, `batch_id?`, and per-handler extras (e.g. 835
carries `matched_count`). No class hierarchy, no plugin registry —
the scheduler keeps its `HANDLERS` dict as the registry.
3. **The `_FILE` env-var convention from SP26 stays.** This spec
doesn't add a new secret tier; the existing `secrets.get_secret()`
three-tier lookup (env var → file → Keychain) is reused.
4. **Time-bounded SFTP operations use `asyncio.to_thread` +
`asyncio.wait_for`.** `paramiko` is synchronous; the scheduler
already wraps calls in `asyncio.to_thread` for the stub path.
Real-mode calls get the same treatment so a hanging `listdir_attr`
on the worker thread can be cancelled by the event loop and
surfaced as a `Scheduler._last_error`.
5. **Default SFTP operation timeout is 30s.** `CYCLONE_SFTP_OP_TIMEOUT_SECONDS`
lets operators tune it down (e.g. 10s for a flaky VPN uplink) or
up for slow connections. The poll-interval itself remains
`CYCLONE_SCHEDULER_POLL_SECONDS` (default 60s).
6. **Reconciliation runs inside the 835 ingest session.** No
background reconciler process. If a future need emerges (catch-up
reconcile on old batches) it lands in a future SP with a
separate `cyclone reconcile` CLI subcommand.
7. **Chain endpoint renders even when slots are empty.** The contract
is "give me this claim's chain; if any piece is missing, return
`null` for that field with `missing: ["277ca_ack"]` so the UI can
show a placeholder." This matches the operator mental model:
"I submitted 837 week-1, the 999 came back accepted, the 277CA
came back accepted, but I haven't seen the 835 yet — show me that
intermediate state."
8. **`Scheduler.status()` gains `consecutive_failures`,
`last_error_at`, `last_error`, `last_sftp_attempt_at`.**
The existing fields (`running`, `poll_interval_seconds`,
`sftp_block_name`, `last_poll_at`, `poll_count`, totals, last_tick)
are unchanged for backward-compat.
9. **The `claim.rejected_after_remit` audit event is informational,
not blocking.** The 999/277CA handlers still write the rejection
even when a matched remit exists; the audit event is the operator's
signal to manually unmatch the pair. We do NOT auto-unmatch (would
violate the manual-match semantics from T15).
10. **The startup invariant check is log-only.** It does not block
boot. A drift between `Claim.matched_remittance_id` and
`Remittance.claim_id` indicates a historical bug; we surface it
in the logs and continue. Operators who want to reconcile the
drift have a one-shot CLI subcommand (`cyclone reconcile
reindex-matches`) deferred to a follow-up SP.
## 3. Architecture (after SP27)
```
backend/src/cyclone/
scheduler.py — slim: Scheduler class + HANDLERS registry + lifecycle
handlers/ NEW subpackage
__init__.py
_ack_id.py — ack_count_summary, ack_synthetic_source_batch_id, *_277ca_*
ack.py — top-level ack dispatch (re-exported helpers)
handle_999.py — text → parsed 999 → ack row → claim rejections
handle_835.py — text → parsed 835 → batch + remits + CasAdjustments
+ match claims (atomic one-section)
handle_277ca.py — text → parsed 277CA → ack row + claim rejections
+ emit claim.rejected_after_remit
handle_ta1.py — text → parsed TA1 → ack row
clearhouse/__init__.py — _paramiko ops wrapped in asyncio.wait_for via to_thread
edi/filenames.py — parse_inbound_filename accepts suffix-less names
store.py — manual_match / manual_unmatch keep matched_remittance_id
and Remittance.claim_id in sync (same transaction);
startup invariant check
api.py — /api/parse-835 deduped vs scheduler handlers via
handlers.handle_835; new /api/claims/{id}/chain endpoint
reconcile.py — run() folded into handle_835; top-level `match()`
kept for the CLI subcommand (deferred)
api_routers/chain.py NEW — GET /api/claims/{id}/chain (auth, matrix_gate)
src/
hooks/useClaimChain.ts NEW — TanStack Query wrapper around /api/claims/{id}/chain
pages/ClaimDrawer.tsx — one new section: "Chain" with submission / 999 / 277CA / remit
placeholders; renders adjustments inline
```
The scheduler shrinks from 860 LOC to ~250 LOC (the Scheduler class +
singleton plumbing). Each `handle_*.py` is ~80150 LOC and can be
tested in isolation against a real prodfiles fixture.
## 4. Files
**New:**
- `backend/src/cyclone/handlers/__init__.py`
- `backend/src/cyclone/handlers/_ack_id.py`
- `backend/src/cyclone/handlers/handle_999.py`
- `backend/src/cyclone/handlers/handle_835.py`
- `backend/src/cyclone/handlers/handle_277ca.py`
- `backend/src/cyclone/handlers/handle_ta1.py`
- `backend/src/cyclone/api_routers/chain.py`
- `src/hooks/useClaimChain.ts`
- `backend/tests/test_handlers_999.py`
- `backend/tests/test_handlers_835.py`
- `backend/tests/test_handlers_277ca.py`
- `backend/tests/test_handlers_ta1.py`
- `backend/tests/test_inbound_filename_loose.py`
- `backend/tests/test_sftp_op_timeout.py`
- `backend/tests/test_scheduler_status_errors.py`
- `backend/tests/test_handler_835_atomic_reconcile.py`
- `backend/tests/test_api_claim_chain.py`
- `backend/tests/test_handler_277ca_rejected_after_remit.py`
- `backend/tests/test_store_match_invariant.py`
- `src/hooks/useClaimChain.test.ts`
- `src/pages/ClaimDrawer.test.tsx` (extension — the chain drawer section)
**Modified:**
- `backend/src/cyclone/scheduler.py` — slim to Scheduler class + lifecycle
- `backend/src/cyclone/edi/filenames.py``parse_inbound_filename`
accepts suffix-less inbound filenames; `is_inbound_filename` likewise
- `backend/src/cyclone/clearhouse/__init__.py` — wrap paramiko
operations in `asyncio.wait_for(asyncio.to_thread(...), timeout=N)`
- `backend/src/cyclone/store.py``manual_match` /
`manual_unmatch` write the pair in one transaction; startup
invariant log; `add` (835 branch) calls `handlers.handle_835`
instead of inlining
- `backend/src/cyclone/reconcile.py``run` exposed for the
follow-up CLI subcommand; `match` kept; the scheduler / 835
handler stop calling `run` directly
- `backend/src/cyclone/api.py``/api/parse-835` reuses
`handlers.handle_835`; `/api/parse-999`, `/api/parse-277ca`,
`/api/parse-ta1` likewise; add `/api/claims/{id}/chain` route
- `backend/src/cyclone/api_helpers.py` (or new file) — wire
`/api/claims/{id}/chain` if the route doesn't fit the existing
pattern
- `src/pages/ClaimDrawer.tsx` — one new "Chain" section
- `src/lib/api.ts` — expose `api.fetchClaimChain(id)`
- `src/types/index.ts` — add `ClaimChain` type
**New migrations:** none. The DB schema is unchanged.
## 5. API surface
| Method | Path | Returns | Auth |
|---|---|---|---|
| GET | `/api/claims/{id}/chain` | `ClaimChain` JSON | `matrix_gate` (any logged-in user) |
No new endpoints beyond this one. No new env-var-driven behavior
beyond `CYCLONE_SFTP_OP_TIMEOUT_SECONDS` (default 30s).
```jsonc
// GET /api/claims/{id}/chain
{
"claim_id": "...",
"submission": {
"batch_id": "...",
"patient_control_number": "...",
"service_date_from": "2026-06-15",
"service_date_to": "2026-06-15",
"charge_amount": "125.00",
"state": "submitted",
"submitted_at": "2026-06-16T..."
},
"ack_999": {
"source_batch_id": "999-PCN-12345678",
"ack_code": "A",
"received_count": 1,
"accepted_count": 1,
"rejected_count": 0,
"received_at": "2026-06-17T..."
} | null,
"ack_277ca": {
"source_batch_id": "277CA-000000001",
"classification": "accepted",
"received_at": "2026-06-18T..."
} | null,
"remittance": {
"id": "...",
"payer_claim_control_number": "...",
"total_paid": "85.00",
"patient_responsibility": "15.00",
"adjustment_amount": "25.00",
"status_label": "Paid",
"received_at": "2026-06-19T...",
"matched": true,
"adjustments": [
{ "group_code": "CO", "reason_code": "97", "amount": "25.00", "label": "..." }
]
} | null,
"missing": ["277ca_ack"] // list of any of the above that came back null
}
```
## 6. Env vars (operator-facing)
| Variable | Default | Purpose |
|---|---|---|
| `CYCLONE_SFTP_OP_TIMEOUT_SECONDS` | `30` | Per-operation SFTP timeout in `clearhouse`. Lower for flaky links, higher for slow ones. The existing `CYCLONE_SCHEDULER_POLL_SECONDS` and `CYCLONE_SFTP_PASSWORD[_FILE]` from SP25+26 are unchanged. |
No new env vars beyond this one.
## 7. Validation rules
No new R-code rules. The 835 validator (`parser_835.validator_835`)
is called unchanged by `handlers.handle_835` inside the same
transaction; the new chain endpoint doesn't introduce new parser
or validation logic.
## 8. Testing plan
Target backend test count after SP27: current + 17 new tests.
1. **Per-handler unit tests** (4 files × ~3 tests each):
`test_handlers_999.py`, `test_handlers_835.py`,
`test_handlers_277ca.py`, `test_handlers_ta1.py` — call each
handler against a prodfiles fixture and assert the persisted
rows.
2. **`test_inbound_filename_loose.py`** (4 cases):
- filename with `_835.x12` suffix → `file_type="835"` (existing behavior).
- filename without `_file_type.x12` and `orig_tx=835``file_type="835"` (new).
- filename without suffix and `orig_tx=999``file_type="999"` (new).
- filename without suffix and `orig_tx=837P``file_type="999"` rejected by `ALLOWED_FILE_TYPES` — no false positive for 837s.
3. **`test_sftp_op_timeout.py`** (3 cases): stub SFTP returns a slow
`sftp.listdir_attr` (raises after 60s); assert the scheduler
times out within `CYCLONE_SFTP_OP_TIMEOUT_SECONDS` + slack;
stub returns fast → assert happy path still works.
4. **`test_scheduler_status_errors.py`** (4 cases): `consecutive_failures`
increments on tick error; `last_error` populated;
`status_sftp_health` flips to `failing` after 3 fails; flips back
on next success.
5. **`test_handler_835_atomic_reconcile.py`** (3 cases): 835 ingest
persists `Remittance` with `adjustment_amount` correct from the
start; reconcile crash mid-transaction leaves no orphans; an
existing claim gets `matched_remittance_id` set in the same
transaction.
6. **`test_api_claim_chain.py`** (5 cases): happy path with all four
slots populated; missing ack_999 → `null + missing=["ack_999"]`;
missing remittance → `null + missing=["remittance"]`; missing
claim → 404; auth → 401.
7. **`test_handler_277ca_rejected_after_remit.py`** (2 cases):
277CA rejects a claim that is already matched → emits
`claim.rejected_after_remit` audit event with both IDs;
277CA rejects an unmatched claim → no audit event.
8. **`test_store_match_invariant.py`** (3 cases):
`manual_match` writes `Claim.matched_remittance_id` and
`Remittance.claim_id` in one transaction;
`manual_unmatch` clears both; pre-existing drift is logged at
startup without blocking boot.
9. **Frontend useClaimChain.test.ts** (3 cases): hook fetches the
chain and returns typed data; loading/error states; caches
for 30s.
10. **Frontend ClaimDrawer.test.tsx extension** (2 cases): renders
the chain section; renders placeholders for missing pieces.
The full backend suite (`cd backend && .venv/bin/pytest`) and
frontend suite (`npm test`) remain the merge gate.
## 9. Out of scope (future SPs)
- Frontend scheduler operator UI (start/stop/tick/status page).
- Multi-tenant / per-payer SFTP blocks.
- `<NAME>_FILE` Docker-secret convention beyond what SP26 added.
- Outbound file pickup verification.
- Auto-unmatch on rejection (would violate T15 manual-match contract).
- `cyclone reconcile reindex-matches` one-shot CLI for historical drift.
- Per-payer chain-fanout (different payer-specific parsing of the same
277CA segment).
- Persisting the chain response server-side (it is a join, not a stored
view; keeping it computed avoids drift).