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.
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.
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.
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).
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.
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)
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.
Finishes the in-tree was_skipped draft: size-matched remote files no
longer emit clearhouse.submitted events or increment uploaded, and the
periodic reconnect now triggers only after real uploads.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- payer-mismatch files are now skipped instead of uploaded
- SFTP session persists across files (was reopened and leaked per file)
- --reconnect-every now counts successful uploads, after increment
- drop unused SftpClient instantiation; read each file once
- apply_999_rejections docstring: R/E/X, not R/E
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Adds `cyclone resubmit-rejected-claims` to push corrected single-claim
837 files to the Gainwell ToHPE SFTP dir. Idempotent (stat-then-skip by
byte size). One persistent paramiko session per batch with
reconnect-every=50 to dodge MOVEit's silent per-session file cap
(~200 puts/session, no exception).
- Validates each file via `parse_837` before upload and rejects any
whose payer_id is not `CO_TXIX` (catches a bad byte-fix early).
- Refreshes test fixtures (`minimal_837p.txt`, `co_medicaid_837p.txt`,
`co_medicaid_837p_with_renderer.txt`) and the corresponding test
assertions (`test_payer.py`, `test_payer_summary.py`,
`test_co_medicaid_fixture.py`, `test_parse_837.py`) from the old
`SKCO0`/`COHCPF` payer IDs to `CO_TXIX`, matching
PayerConfig.co_medicaid() and the HCPF 837P Companion Guide.
- Adds `ingest/` to .gitignore — local scratch / production-data
staging only.
Add the 'cyclone backfill-999-rejections' subcommand to replay the
cascade fix in apply_999_rejections for any 999 acks already in the
DB. Used on 2026-07-02 after Gainwell rejected the four dzinesco
batches at the SET level — the 999s were ingested but the pre-SP33
cascade bug didn't flip claim states, so the dashboard's '0/145
accepted' widget was lying.
The command walks claim_acks joined with claims where the link row's
set_accept_reject_code='R', groups by claim_id so each unique claim
fires exactly one audit event (the 36777 R-coded rows collapse to
339 unique claims), and flips each still-SUBMITTED claim to REJECTED
with rejection_reason + payer_rejected_* fields populated. Claims
already in REJECTED are skipped.
Also moves 'if __name__ == "__main__": main()' to the bottom of the
file. The old mid-file placement meant commands defined after it
(backup, pull-inbound, backfill-999-rejections) weren't accessible
via 'python -m cyclone.cli <sub>' — only the 'cyclone' console script
worked. Latent bug since SP17; surfaced when SP33 added another
post-block command.
The 999 handler's rejection pass was looking up claims by patient
control number, but Gainwell rejects at the SET level (ST envelope)
when the whole batch fails the NM109=CO_TXIX rule. That meant a SET
rejection was treated as a no-op even though every claim in the SET
was actually rejected by the payer.
Add a batch_envelope_index param (mirrors apply_999_acceptances from
SP28) so SET-level rejections cascade to every claim in the SET.
Falls back to the legacy PCN lookup when the index has no entry.
Also tightens test_payer.py: PayerConfig.co_medicaid() now returns
payer_id='CO_TXIX' and payer_name='CO_TXIX' per HCPF 837P Companion
Guide (June 2025 - Version 2.5).
- backend: GET /api/batches now returns acceptedCount/rejectedCount/
pendingCount/billedTotal/topRejectionReason/hasProblem per item
(one GROUP BY query + one ordered scan, no N+1)
- backend: 2 tests pin the 837p full-bucket case + the 835 zero case
- frontend: BatchSummary extended with 6 optional fields
(backwards compat preserved)
- frontend: new RecentBatchesWidget renders one row per batch with
status icon + billed total + accepted count + top rejection reason
- frontend: 837p rows show $ total + 'N/M accepted'; 835 rows show
payment count (Remittance has no batch-level total_charge)
- frontend: full-width row between KPI tiles and Activity on the
Dashboard; click navigates to /batches?batch=ID
- frontend: 3 component tests cover empty/clean/problem/835 branches
- backend: _ack_summary_for_claims helper attaches claim_acks payload
to inbox rejected-lane rows (3 tests)
- frontend: InboxRow renders newest 3 AK2 chips + '+N more' overflow
under the rejection reason, with '999 not linked' marker when an
ack couldn't be linked to a claim
- frontend: per-row Resubmit button downloads a single corrected 837
via api.serializeClaim837 (no bulk modal, no zip — one click, one
.x12 file)
- frontend: 2 new tests for the chip rendering and the per-row
download flow
The SP28 helpers (batch_envelope_index + lookup_claims_for_ack_set_response
+ two _batch_lookup closures in handle_ta1 + api.py) all filter
Batch.kind == "837", but prod Batch rows are persisted with
kind="837p" (lowercase p — see api.py:443 / store/records.py:58).
Result: the D10 batch-envelope index was empty on prod and zero 999 /
277CA / TA1 acks auto-linked to their claims via Pass 1 (ST02 via
batch.envelope.control_number). Pass 2 (PCN) was the only path firing,
which on this codebase matches 0 acks (Gainwell's 999 echoes the 837's
ST02, not its CLM01 — see spec §D10).
Fix: 4 production sites swapped to Batch.kind == "837p". 3 test
files updated to use the prod value (test_apply_claim_ack_links.py +
test_api_claim_acks.py + test_e2e_999_to_claim_drawer.py). All 25 SP28
tests + 20 handler tests pass. Expected prod effect: claim
t991102989o1c120d's 143 incoming 999 AK2 acks now auto-link via the
batch with envelope.control_number=991102989 (Pass 1), plus 727/1,398
total acks across all batches.
Phase 5 of SP28 (Ack↔Claim Auto-Link). Adds the public HTTP
surface for the claim_acks join table and wires it into the
existing claims + acks list endpoints.
New router (registered with matrix_gate auth, any-logged-in-user):
* GET /api/claims/{id}/acks — per-claim links
* GET /api/claims/{id}/acks/stream — NDJSON live-tail
* GET /api/acks/{kind}/{id}/claims — per-ack links
* GET /api/acks/{kind}/{id}/claims/stream — NDJSON live-tail
* POST /api/acks/{kind}/{id}/match-claim — manual link (D5)
* DELETE /api/acks/{kind}/{id}/match-claim/{claim_id} — unlink
* GET /api/inbox/ack-orphans — orphan reconciliation
Manual match is any-logged-in-user (D5/D9), idempotent on the
(claim_id, ack_kind, ack_id) dedup key, rejects with 409 when the
claim is in a terminal state (REVERSED), and publishes
claim_ack_written / claim_ack_dropped on the bus so live-tail
subscribers refresh.
Existing list endpoints extended:
* /api/acks — each item gains linked_claim_ids
* /api/ta1-acks — each item gains linked_claim_ids
* /api/277ca-acks — each item gains linked_claim_ids
* /api/claims/{id} — body gains ack_links (compact form)
All three list extensions use a single batched SELECT against
claim_acks to avoid N+1.
Tests:
* tests/test_api_claim_acks.py — 8 tests covering all 7
endpoints + the spec §6 named tests for the extended
surfaces (claim_detail_includes_ack_links,
acks_list_includes_linked_claim_ids,
claim_acks_stream_emits_claim_ack_written). The stream test
uses the established direct-endpoint-invocation pattern
from test_api_stream_live.py so it gets true byte-streaming
+ clean async cancellation.
Phase 4 of SP28 (Ack↔Claim Auto-Link). Refactors the per-AK2
helpers in cyclone.claim_acks to return ClaimAckLinkRow dataclasses
instead of mutating the session directly — the orchestrator (the
999 / 277CA / TA1 handlers + the matching parse-* API endpoints)
now persists each row via cycl_store.add_claim_ack so the
publish-from-store contract owns the live-tail event emission.
* handle_999 / handle_277ca / handle_ta1 — build batch envelope
index outside the work session (SQLite + concurrent sessions
causes 'database is locked'), call apply_X to get
ClaimAckLinkRow dataclasses, snapshot the rows before committing
the work session, then call cycl_store.add_claim_ack per row
in fresh sessions.
* /api/parse-999 / /api/parse-277ca / /api/parse-ta1 — mirror the
handler chain with event_bus passed through so live-tail
subscribers on the claim and ack sides see the new rows the
moment they land. Adds a 'claim_ack_links_count' field to each
ack response (spec §4).
* lookup_claims_for_ack_set_response — now accepts either a
callable OR a plain dict as batch_envelope_index (the store
returns a dict; tests pass callables).
* test_apply_claim_ack_links.py — 15 tests updated to assert on
the dataclass shape and exercise the full helper→add_claim_ack
cycle (so idempotency is verified at the store layer).
* test_e2e_999_to_claim_drawer.py — 2 new tests covering the
D10 two-pass join end-to-end via FastAPI TestClient (Pass 1
via ST02 + Pass 2 via PCN fallback).
The persistence layer for the SP28 join table lands in
cyclone/store/claim_acks.py:
- add_claim_ack — insert + publish 'claim_ack_written' on the bus
(mirrors the publish-from-store pattern used by the ACKs paths)
- list_acks_for_claim / list_claims_for_ack — read helpers for the
two list endpoints
- find_ack_orphans — Inbox ack-orphans lane source: every ack row
whose ack_id has no ClaimAck row tied to it
- remove_claim_ack — unlink + publish 'claim_ack_dropped'
- batch_envelope_index — D10 in-memory map of
{envelope.control_number: batch.id}, called once per ingest (cost
is ~16 lookups today; trivially cheap)
Plus the to_ui_claim_ack serializer in store/ui.py mirroring
to_ui_ack shape — single source of truth for the wire format so the
live-tail event payload matches the list endpoint byte-for-byte.
Includes a single SELECT against claims for the claim_state field
(TA1 batch-level rows return 'n/a').
The CycloneStore facade in store/__init__.py re-exports all six
methods (add_claim_ack, list_acks_for_claim, list_claims_for_ack,
find_ack_orphans, remove_claim_ack, batch_envelope_index) plus
to_ui_claim_ack from the ui module.
Migration-version assertions in test_acks.py and test_db_migrate.py
bumped 17 → 18 to match the new migration head.
Steps 3.1/3.2/3.3/3.4 of the SP28 implementation plan.
The auto-linker closes the operator gap where every inbound 999 /
277CA / TA1 ack was persisted but never linked back to the claim it
acknowledges. Five pure helpers land in cyclone/claim_acks.py:
- lookup_claims_for_ack_set_response — D10 two-pass join. Primary is
Batch.envelope.control_number (== source 837 ST02 for Gainwell
batches); fallback is Claim.patient_control_number. Pass 1 wins,
the two paths cannot both fire.
- apply_999_acceptances — walks parsed_999.set_responses, emits one
ClaimAck per AK2 per matched claim (one-ack-to-many supported).
Both accepted AND rejected AK2s link; per-AK2 granularity.
- apply_277ca_acks — same shape for parsed_277ca.claim_statuses.
STC category code carried on the link row's
set_accept_reject_code so the UI can render the lane inline
without re-parsing raw_json.
- apply_ta1_envelope_link — envelope-level link. The link row has
claim_id NULL + batch_id populated (the spec's batch-level TA1
trace). Sender/receiver matching is delegated to a closure the
caller supplies.
- link_manual — manually link an ack to a claim. Used by the new
/api/acks/{kind}/{id}/match-claim endpoint. Idempotent.
All five helpers are pure (callers own the session); idempotent via
the partial unique index ux_claim_acks_dedup (helpers pre-check to
avoid IntegrityError log noise on re-ingest); flush-only (callers
commit).
14 of 15 named tests pass (the facade surface check belongs in
Phase 3 once store/claim_acks.py lands).
Steps 2.1/2.2/2.3/2.4/2.5 of the SP28 implementation plan.
The claim_acks join table is the durable record of which inbound
999 / 277CA / TA1 ack acknowledged which claim (or, for TA1, which
originating 837 Batch). One row per AK2 set-response / ClaimStatus
/ envelope. The match granularity is per-AK2 so the operator can
answer 'which claims does this ack acknowledge?' with a single
SELECT and so the ClaimDrawer panel can show per-segment accept /
reject status without re-parsing raw_json.
Schema mirrors spec §3.1:
- claim_id NULLable + batch_id NULLable (TA1 envelope links land
on batch_id; CHECK enforces at least one populated)
- unique partial index ux_claim_acks_dedup enforces idempotent
re-ingest of the same 999 file
- set_control_number stores the value the upstream ack ACTUALLY
CARRIED (== source 837 ST02 for Gainwell batches) for orphan
traceability — the link survives even when the join had to fall
back from ST02 to PCN matching
Mirrored on the ORM via Index(..., sqlite_where=text(...)) so
Base.metadata.create_all (the test-time safety net) emits the same
partial-unique constraint as the migration.
Step 1.1/1.2/1.3 of the SP28 implementation plan.
Adds two streaming endpoints that match the live-tail wire format
established by /api/claims/stream, /api/remittances/stream, and
/api/activity/stream:
* /api/acks/stream — subscribes to ack_received
* /api/ta1-acks/stream — subscribes to ta1_ack_received
Both yield a snapshot of existing rows (newest first, capped by the
`limit` query param), then a `snapshot_end` line, then forward
live events from the bus. They are registered before the
/{ack_id} path-param endpoints so the literal `stream` segment
isn't matched as an id.
Tests use the same direct-coroutine pattern as test_api_stream_live.py
because httpx.ASGITransport buffers the response body and never
delivers a disconnect message — iterating body_iterator directly
with body_iterator.aclose() simulates a client disconnect.
- write.py: drop orphan run_reconcile() — no callers, and the
pre-split store.py never had it. (The CycloneStore class already
has _publish_events_sync + _sync_publish delegations; the spec's
mention of _run_reconcile was a doc drift from the 2026-06-21
plan, not a real requirement.)
- ui.py: drop duplicate _provider_orm_to_dict + _payer_orm_to_dict
(canonical copies live in providers.py and are the only ones used;
the ui.py copies were accidental duplicates from the split).
- orm_builders.py: drop orphan _cas_adjustment_row() — never called;
_persist_835_remit() builds CAS rows inline.
- __init__.py: prune 30+ unused top-level imports and 9 unused
re-exports. The facade only ever needs the type-hint-bearing
BatchRecord family, the 4 read-path ORM-row serializers actually
used inside the package, the 3 documented private helpers, and
the 7 function-level re-exports the spec promises. Everything
else was carryover from the original store.py header.
Tests at exact baseline: 1 failed, 1176 passed, 10 skipped
(the 1 failure is the pre-existing test_provider_detail isolation
flake documented in the spec).
Behaviour-preserving structural split of the 2,995-LOC store.py into
a 14-module subpackage with a thin facade. CycloneStore class keeps
its full method surface as 1-line delegations to module functions.
14 modules:
exceptions, records, orm_builders, ui, write,
batches, claim_detail, kpis, acks,
backups, inbox, providers
(+ __init__.py facade)
Facade re-exports 12 public symbols (CycloneStore, store, BatchRecord*,
BatchKind, AlreadyMatchedError/NotMatchedError/InvalidStateError, utcnow,
dashboard_kpis, check_matched_pair_drift) + 3 private helpers
(_claim_status_from_validation, _persist_835_remit, _remittance_835_row)
to preserve the 4 test files that import them.
Zero public API changes, zero test changes, zero importer changes.
CycloneStore._lock and CycloneStore._batches.clear() remain intact for
the 7 test files still using the cleanup idiom.
Verified: 1,176 / 1 (pre-existing isolation flake) / 10 tests pass —
identical to baseline.
The Remittances page's three KPI tiles — REMITS / TOTAL PAID /
ADJUSTMENTS — were computed page-locally via items.reduce(...) over
the merged tail of the current page + live delta. With a 100-row
default limit, a 1,739-row population showed count=100, paid=$16,934,
adjustments=$147 — silently understating reality because the page
hadn't loaded the remaining rows yet.
This change mirrors the silent-incompleteness fix that
/api/dashboard/kpis (commit 59c3275) and /api/remittances (commit
d81b6ed) made for their tiles:
* CycloneStore.summarize_remittances() iterates the full filtered
remittance population (no limit) and returns
{count, total_paid, total_adjustments}. Mirrors iter_remittances
with limit=_ITER_UNBOUNDED.
* GET /api/remittances/summary — server endpoint with the same
filter parameters as /api/remittances. Registered BEFORE the
/api/remittances/stream handler so FastAPI doesn't treat
'summary' as a stream sub-path.
* api.listRemittanceSummary + useRemittanceSummary hook.
* Remittances.tsx swaps off items.reduce, consumes the server
summary. Tiles render the server totals so the values reflect
the entire DB population, not the page-local sample.
Verified live: /api/remittances/summary returns
{count: 1739, total_paid: 227181.58, total_adjustments: 13792.65},
which matches DB ground truth exactly.
Tests: 8 new backend tests in test_api_remittances_summary.py; 1 new
frontend test in Remittances.test.tsx (kpi_tiles_use_server_summary_not
_page_local_reduce) plus the page-level test for the zero-valued mock
default.
The 837 ingest path (_claim_837_row in store.py:194) populated
Claim.patient_control_number from claim.subscriber.member_id — the
subscriber's 2010BA NM109 Medicaid ID — instead of from
claim.claim_id (CLM01, the claim submitter's identifier the 837
actually sent).
That silently broke every downstream join that uses this column as a
cross-reference key:
* reconcile.match() (reconcile.py:74) — joins
Claim.patient_control_number against
Remittance.payer_claim_control_number (which is parsed from CLP01,
the 835's echo of CLM01 per X12 spec). Member_id never matches
CLP01, so auto-match always fails.
* apply_999_rejections — same lookup, same broken key.
* apply_277ca_rejections — same lookup, same broken key.
* scoring.score_pair — same broken key.
Live DB probe (1,739 remits, 337 claims):
* Claim.id == Remit.payer_claim_control_number → 0 matches
* Claim.patient_control_number == Remit.payer_claim_control_number
→ 9 matches (substring coincidences; synthetic PCN strings share
alphanumeric characters with the human-readable member_ids)
* Claim.matched_remittance_id NOT NULL → 0 claims
This commit changes _claim_837_row to write
Claim.patient_control_number = claim.claim_id (= CLM01). The
reconcile matcher's existing join now hits the row the 835 echoes
back. Companion migration 0017 backfills the 337 pre-fix rows so
they're on equal footing with new ingests (UPDATE claims SET
patient_control_number = id WHERE patient_control_number IS DISTINCT
FROM id — idempotent).
Tests:
* 2 new RED→GREEN tests in test_store_reconcile.py:
- test_837_ingest_populates_patient_control_number_from_claim_id
pins the field semantics directly
- test_837_then_835_with_echoed_pcn_auto_pairs proves the full
end-to-end auto-match now fires
* 3 existing manual_match tests that relied on the bug (had setup
using member_id != claim_id to deliberately prevent auto-match)
updated to use distinct PCNs explicitly so the tests still
exercise the manual-match path with a real orphan pair.
* 3 store_claim_detail / api_gets tests adjusted for the same
reason.
Verified: 1,176 / 1,177 backend tests pass; the one failure is the
pre-existing flake in test_provider_extended_response.py noted before
this work started.
Caveat for the existing dev DB: the 1,739 remits already in the DB
were ingested from 835 fixtures whose CLP01 is a different synthetic
identifier than the 837's CLM01 (the test fixtures never echoed
CLM01 — a fixture-data limitation, not a code bug). After this fix,
new 837+835 ingest pairs whose payer echoes CLM01 in CLP01 will
auto-match as expected. The pre-existing 1,739 remits will continue
to land in the unmatched bucket; that can only be fixed by
regenerating the test fixtures (out of scope for this SP).
Before this fix, the list endpoints computed `total` via
`len(list(store.iter_*(**common)))`. Both `iter_claims` and
`iter_remittances` default to `limit=100`, so the reported total
silently capped at 100 even when the DB held 60k claims or 835
remits. The frontend rendered a `data.total` of 100 in the KPI
tile and a 100-row table, the page looked complete, and the bug
stayed hidden — exactly the Dashboard silent-failure pattern that
59c3275 (server-aggregated KPIs) was meant to retire.
The Remittances page symptom reported on Jun 29 ('REMITS 100',
empty CLAIM column on 100 rows) was this bug. The empty CLAIM is
a separate matching concern (those remits have `claim_id = NULL`
in the DB); the count fix addresses the population-size lie.
Fix:
* `CycloneStore.count_claims` + `count_remittances` reuse the
iter's filter pipeline (DB filters + in-memory payer/date
filters) with an effectively-unbounded limit so the count
reflects the true DB population.
* `list_claims` + `list_remittances` call the new count
helpers instead of slicing `iter_*` twice.
* 13 new tests in `test_list_endpoint_counts.py` cover the
empty/filter/per-dimension/cardinality cases plus the HTTP
regression (seed 150/120, assert total matches).
Bonus fix:
* `conftest._reset_rate_limit_buckets` walks the middleware
stack and clears `RateLimitMiddleware._buckets` between
tests. Without this, the full suite tripped the 300 req/60s
rate limiter mid-run and 9 later tests (4 of mine, 5
pre-existing in test_inbox_endpoints / test_payer_summary)
got 429s. All 1167 tests now pass.
Follow-ups noted but out of scope:
* `/api/admin/audit-log`, `/api/admin/backup/list`,
`/api/admin/scheduler/processed-files` use the same
`len(rows)`-after-`.limit(limit)` pattern. Admin-only,
no `has_more` consumption, so impact is bounded.
* `count_*` could short-circuit to `func.count()` to skip
the UI-dict build, but iter already calls `q.all()` so
the win is modest.
The Dashboard was hardcoded to useClaims({ limit: 100 }) and reduce
KPIs client-side. With 60k+ claims in production, every tile
(Billed $940K, Received $59K, Denial rate, Pending AR, monthly
sparkline, top providers, recent denials) was computed from a
0.16% sample of the dataset — silently wrong numbers on the
operator's primary view.
Fix: add GET /api/dashboard/kpis that aggregates server-side in
one read over the entire claim population. The new useDashboardKpis
hook consumes it and polls every 60s. Dashboard.tsx drops
useClaims({limit:100}) + useProviders() + the client-side
buildMonthly reduce.
Backend:
- store.py: dashboard_kpis() — one Claim query (selectinload on
batch to avoid N+1) + one bulk Remittance lookup, Python reduce
over the full population. Zero-filled response for empty DB.
- api.py: GET /api/dashboard/kpis behind matrix_gate, query-param
clamps (1..24 months, 0..50 top_n_*).
Frontend:
- api.ts: DashboardKpis types + getDashboardKpis() wrapper.
- useDashboardKpis.ts: TanStack Query hook, 60s refetchInterval,
bypass to data:undefined when not configured.
- Dashboard.tsx: switched to useDashboardKpis, extracted
ZERO_TOTALS constant, dropped the buildMonthly helper.
Tests:
- backend/tests/test_dashboard_kpis.py: 12 tests covering empty DB,
matched-remit math, pending-state semantics, monthly binning,
top-providers/top-denials sort + cap, orphan-claim defensive
guard, HTTP wiring + param validation.
- src/hooks/useDashboardKpis.test.ts: 3 tests for the hook
contract (configured path, unconfigured fallback, param
passthrough).
- src/pages/Dashboard.test.tsx: wrapped renders in
QueryClientProvider + stubbed useAuth + isConfigured=false. This
fixes 3 pre-existing Dashboard test failures (the page never had
a QueryClient set up because useClaims/useProviders were the
first useQuery hooks in the page).
Reviewer fixes (same commit):
1. topDenials sort placed empty submissionDate claims first under
reverse-lex. Drop them at append time.
2. r.batch lazy-load → N+1 on 60k rows. selectinload(Claim.batch).
3. pending_states rebuilt per call as a mutable set — moved to
module-level _DASHBOARD_PENDING_STATES frozenset.
4. Module-level ProviderORM import inconsistent with the
"local import inside the function" pattern — moved inline.
Add check_matched_pair_drift() at module level in store.py — read-only
audit of the Claim.matched_remittance_id ↔ Remittance.claim_id FK pair.
Logs WARNING on drift with up to 5 examples of each of two cases
(claim-side and remit-side), returns the count of drifted rows. Wired
into api.py::lifespan right after db.init_db() and ensure_clearhouse_seeded(),
wrapped in try/except so a query failure logs but doesn't crash boot.
The symmetric-write + symmetric-clear invariants on
manual_match / manual_unmatch were already correct; this commit pins
them with focused regression tests in test_store_match_invariant.py
(8 tests covering both directions, both happy-path, rollback pin, and
drift-check unit behavior). PCN asymmetry in the fixture prevents the
auto-match inside CycloneStore.add (Task 10) from pre-pairing, so
manual_match is the only writer.
Migration 0016 adds the missing ix_claims_matched_remittance_id index —
the drift check scans WHERE matched_remittance_id IS NOT NULL on every
boot and would become a full-table scan past ~10k claims. Symmetric
with ix_remittances_claim_id (added in 0007). Migration tests bumped
from head=15 to head=16.
Move 'reconcile.run(s, record.id)' inside CycloneStore.add()'s ingest
session, before s.commit(). The placeholder adjustment_amount set by
_remittance_835_row is overwritten by reconcile's CAS-aggregate pass
in the same transaction — readers never see a half-reconciled
Remittance row. If reconcile raises, the entire 835 ingest rolls back
via the session's __exit__.
The previous flow committed batch + remittance rows in session-1,
then opened session-2 to run reconcile.fail-soft. Two visible
problems closed: a race window where readers fetched the placeholder
adjustment_amount, and a half-reconciled state left visible if
reconcile crashed.
Deviation from the plan (N4 in autoreview): the reconcile call lives
in cycl_store.add() rather than handle_835 calling a new
reconcile.run_now(batch_id) helper after add(). Same end state, one
fewer module surface, handler stays a thin wrapper over the store.
The 06/25 silent hang was a hung ``sftp.listdir_attr()`` on the
worker thread — paramiko TCP-acked then went silent, the scheduler's
``asyncio.to_thread`` waited forever, and the operator had no
signal that polling had stalled. Every poll cycle since was
suspected of the same failure mode.
``backend/src/cyclone/clearhouse/__init__.``
- New ``_op_timeout_seconds()`` helper reads
``CYCLONE_SFTP_OP_TIMEOUT_SECONDS`` (default 30s). Rejects
unparseable, zero, and negative values — ``asyncio.wait_for(timeout=0)``
raises immediately and ``timeout<0`` is undefined per the asyncio
docs, so a typo would silently turn every SFTP call into an
instant failure. Logs a WARNING and falls back to the default.
- New ``async_list_inbound()`` async wrapper applies
``asyncio.wait_for(asyncio.to_thread(self.list_inbound),
timeout=N)``. ``wait_for`` cancels the awaiter after N
seconds but the worker thread keeps running until paramiko returns
on its own (paramiko is not asyncio-aware, can't be cancelled
cleanly). ``asyncio.TimeoutError`` propagates so the scheduler
can surface it as a transient SFTP error.
``backend/src/cyclone/scheduler.py``
- ``_tick_impl`` now calls ``client.async_list_inbound()``
instead of ``asyncio.to_thread(self._list_inbound)``. Catches
``asyncio.TimeoutError`` explicitly with a clear error message
("list_inbound: timeout") — separate from the generic
``Exception`` catch-all so the operator's tick-result error
reads as the actual cause.
- Deleted the now-dead ``Scheduler._list_inbound()`` shim.
``backend/tests/test_sftp_op_timeout.py`` (new, 6 tests):
1. Default timeout is 30s when the env var is unset.
2. Operator can override via env var without restart-rebuild.
3. Unparseable value (e.g. "30s") falls back to default.
4. Zero and negative values fall back to default.
5. A hanging list_inbound raises ``asyncio.TimeoutError`` within
the configured bound (the 06/25 hang pin).
6. A non-hanging call returns the stub's empty list normally
(sanity check that the wrapper doesn't break the happy path).
Scope note: only ``async_list_inbound`` is wired up. The other
SFTP methods (``list_inbound_names``, ``download_inbound``,
``read_file``, ``write_file``) are called from
operator-triggered paths (admin endpoints, CLI, claim submission)
where the operator can Ctrl-C the request, so a hang is at least
visible. Wrapping them would require making the FastAPI handlers
async, which is out of scope for Task 8. Tracked as a follow-up —
worth folding into the same shape when those endpoints are next
touched.
Gainwell's production filer has shipped at least two inbound filename
shapes — the spec form with a trailing `_{file_type}.x12` suffix and
a shorter suffix-less form where the disambiguator token (between
`-` and `_M`) doubles as both orig_tx and file_type. The 6/15–6/19
835 batch arrived in the suffix-less form, and the strict
`INBOUND_RE` rejected them outright — the scheduler silently
dropped 5 days of production data without logging an error.
`backend/src/cyclone/edi/filenames.py`
- New `INBOUND_RE_LOOSE` regex: same prefix/tpid/tracking/ts/seq
rules as `INBOUND_RE`, but the disambiguator token is required
to be 3–5 uppercase alnum (covers 999, TA1, 835, 277CA, ENCR; the
5-char cap stops the engine from over-eating the next `_M` token
in a degenerate input).
- `parse_inbound_filename` now tries `INBOUND_RE` first
(preserves historical behavior for every existing caller) and
falls back to `INBOUND_RE_LOOSE`. In the loose form, orig_tx is
set to the disambiguator token so the parsed shape matches what
the strict form produces when orig_tx == file_type. The
`ALLOWED_FILE_TYPES` check is enforced in both branches.
- `is_inbound_filename` is loosened in the same shape so the two
never disagree — a refactor that pre-filters a directory listing
with `is_inbound_filename` then re-parses with
`parse_inbound_filename` would otherwise see the suffix-less
files rejected twice.
`backend/tests/test_inbound_filename_loose.py` (new, 10 tests):
1. Spec form with explicit `_835.x12` suffix still wins (strict
path unchanged).
2-5. Suffix-less 835 / 999 / 277CA / ENCR all parse correctly.
6. Suffix-less `.txt` is still rejected (the `.x12` ext check
applies to both forms).
7. Suffix-less unknown type (4-char `ABCD`) is rejected by
ALLOWED_FILE_TYPES — the loose regex's `{3,5}` shape would
otherwise let it through.
8. Suffix-less 6-char token (`999XX6`) is rejected by the
5-char cap, preventing the engine from swallowing the next
`_M` token.
9. Strict form takes precedence over the loose form when both
match — pinning the parser's branch order so a refactor can't
silently change the parsed shape.
10. `is_inbound_filename` accepts the loose form, the spec form,
and rejects garbage.
Live smoke: 5/5 suffix-less Gainwell patterns now parse correctly
(were ValueError before); spec form unchanged; 4 invalid forms
still rejected. Full backend suite: 1085/1121 — the 36 pre-existing
failures (test_serialize_837, test_api_stream_live,
test_inbox_endpoints) are unrelated and confirmed pre-existing by
running them on stashed pre-change code.
Nora
Tyler Martinez