178898a8e1
Adds a parse → DB write → SFTP upload pipeline that closes the gap where 837P submissions leave no DB row, making every 999 ack an orphan. Locks the four brainstorming decisions (canonical submit flow, DB-first ordering, new Batch.transaction_set_control_number column, additive deprecation posture) and the architecture for one new CLI + one new HTTP endpoint sharing a cyclone.submission helper.
217 lines
17 KiB
Markdown
217 lines
17 KiB
Markdown
# Sub-project 37 — Canonical `submit-batch` flow: Design Spec
|
|
|
|
**Date:** 2026-07-07
|
|
**Status:** Draft, awaiting user sign-off
|
|
**Branch:** `sp37-submit-batch-canonical-flow` (off main, post-SP36 merge)
|
|
**Aesthetic direction:** No UI changes. Pure backend structural + data-model addition — the public HTTP surface gains one new endpoint; existing CLIs and endpoints are preserved unchanged.
|
|
|
|
---
|
|
|
|
## 1. Scope
|
|
|
|
Today, cyclone's `claims` table is essentially empty in production (`clm-1` is the only row as of 2026-07-07). The reason is structural: dzinesco generates the 837P files (in `ingest/corrected/batch-*-claims/*.x12`), cyclone's `resubmit-rejected-claims` CLI only validates and re-uploads them to Gainwell's MFT — it never writes the claims to the DB. When 999 acknowledgments come back referencing those batches, every AK2 set-response becomes an orphan because there is no `Batch` row to join against.
|
|
|
|
The 999→claim join (the D10 two-pass join in `claim_acks.lookup_claims_for_ack_set_response`) has two paths:
|
|
|
|
- **Pass 1 (primary):** `Batch.envelope.control_number == 999's set_control_number`. This currently fails because the 999's `set_control_number` (AK201) echoes the source 837's **ST02** (transaction set control number), not the **ISA13** (interchange control number). The parsed `Envelope` model stores ISA13 in `control_number`, so the keys never collide even when the DB row exists.
|
|
- **Pass 2 (fallback):** `Claim.patient_control_number == 999's set_control_number`. This also fails because the `_claim_837_row` ORM builder sets `patient_control_number = claim.claim_id` (CLM01), and CLM01 ≠ ST02 in production batches.
|
|
|
|
The fix is twofold: (a) make cyclone write a `Batch` + `Claim` rows at submission time so there is something to join against, and (b) make the join key actually match — by storing the ST02 explicitly on `Batch` and using it in Pass 1.
|
|
|
|
**In scope:**
|
|
|
|
- A new top-level CLI `cyclone submit-batch` that does parse → DB write → SFTP upload per file, in that order.
|
|
- A new HTTP endpoint `POST /api/submit-batch` with the same semantics.
|
|
- A shared helper module `cyclone.submission.submit_file` so the CLI and HTTP path share one source of truth.
|
|
- A new nullable column `Batch.transaction_set_control_number`, populated from the parsed 837's ST02 on every `add_record` write.
|
|
- An Alembic migration `0013_add_batch_txn_set_control_number.py` (additive, nullable, backfills from `raw_result_json.envelope.transaction_set_control_number` where present).
|
|
- An update to `claim_acks.batch_envelope_index` so Pass 1 also matches on `transaction_set_control_number` (Pass 2 unchanged).
|
|
- Per-claim `claim_submitted` activity events and per-file `clearhouse.submitted` audit events (same shape `resubmit-rejected-claims` already emits).
|
|
|
|
**Out of scope (explicit):**
|
|
|
|
- Deprecating `parse-837` CLI or `/api/parse-837` endpoint. They remain for one-off testing and dry-run validation.
|
|
- Deprecating `resubmit-rejected-claims`. It remains the "validate-then-upload without DB write" path for the dzinesco-generated files that aren't ready for canonical tracking.
|
|
- UI changes. The Claims page already renders the fields a new tracked claim needs; the existing `claim_written` live-tail event fires automatically from `add_record`.
|
|
- Bulk backfill of the existing `ingest/corrected/batch-*-claims/*.x12` backlog. A follow-up SP will walk that directory once `submit-batch` is proven.
|
|
- Any change to the `claims` row schema beyond the new `Batch` column.
|
|
- SFTP auth work. This box stays in manual mode (post-2026-07-07 incident); `submit-batch` works in stub mode for local testing and against a whitelisted IP for production.
|
|
|
|
## 2. Decisions (locked during brainstorming)
|
|
|
|
These were each a multiple-choice question with operator sign-off on 2026-07-07.
|
|
|
|
1. **Where does the DB write happen?** New canonical `submit` flow. `submit-batch` parses → writes to DB → uploads, replacing neither `parse-837` nor `resubmit-rejected-claims` but superseding them as the preferred path.
|
|
2. **DB vs upload ordering?** DB-first, upload-second. If DB write fails, the file never goes on the wire. If DB write succeeds but upload fails, re-running is safe (idempotent DB write, idempotent SFTP put).
|
|
3. **Join key for 999 acks?** Add `Batch.transaction_set_control_number` column. Update Pass 1 to match on it. Don't repurpose `envelope.control_number` (preserves backward compat with existing joins) and don't force CLM01 == ST02 (Gainwell's existing 999s use different values).
|
|
4. **Deprecation posture?** Add `submit-batch`; preserve `parse-837` and `resubmit-rejected-claims`. Document `submit-batch` as preferred in CLAUDE.md + RUNBOOK. Future SP can deprecate if usage shifts.
|
|
|
|
## 3. Architecture
|
|
|
|
The new flow is a single, small public surface — one CLI command, one HTTP endpoint, one shared helper. The helper owns the parse → write → upload sequence and is the only place that knows about the order.
|
|
|
|
### Module layout
|
|
|
|
A new package `cyclone.submission` (under `backend/src/cyclone/submission/`) holds the shared helper:
|
|
|
|
- `__init__.py` — re-exports `submit_file`, `SubmitResult`, `SubmitOutcome`
|
|
- `core.py` — `submit_file(path: Path, *, sftp_block, actor: str, event_bus=None, validate: bool = True) -> SubmitResult`. Owns its own DB session for the parse-write step, returns a result dataclass the caller inspects.
|
|
- `cli.py` — Click handler for `cyclone submit-batch`. Walks `--ingest-dir`, calls `submit_file` per file, aggregates counts, exits 0/1/2 per cyclone-cli convention.
|
|
- `api.py` — FastAPI handler for `POST /api/submit-batch`. Mounted in `api_routers/submission.py`. Calls `submit_file` per file. Auth-gated by the existing `matrix_gate`.
|
|
|
|
### `submit_file` algorithm
|
|
|
|
The walker globs `<ingest_dir>/batch-*-claims/*.x12` (mirrors `resubmit-rejected-claims` exactly — same pattern, same `_`-prefix skip for AppleDouble residue). For each matched file:
|
|
|
|
1. Read bytes; if `validate=True`, run `parse_837_text(bytes, payer_cfg)`. On parse failure, record `{path, "parse_failed", exc}` and return without writing or uploading.
|
|
2. Run a payer-id check (must be `CO_TXIX`); on mismatch, record and return.
|
|
3. Call `store.add_record(record, event_bus=...)`. On DB failure, record and return (no SFTP call).
|
|
4. Build the remote path `{outbound_root}/{filename}`. Open an `SftpClient` (the seeded singleton). On SFTP connect failure, record and return — the DB row is already written; the re-run will skip it via idempotency.
|
|
5. `sftp_client.stat(remote_path)` — if `st_size == local_size`, mark as `skipped` (already uploaded; do not re-emit audit). Otherwise `sftp_client.write_file(remote_path, bytes)`.
|
|
6. On successful upload, emit one `clearhouse.submitted` audit event with `source_file`, `batch_id`, `actor`.
|
|
|
|
### Idempotency
|
|
|
|
Two layers, both per-file:
|
|
|
|
- **DB layer:** `store.add_record` already does a `s.get(Claim, claim.claim_id)` check before insert. A re-parse of the same file logs "claim already exists; skipping" and continues. Re-running `submit-batch` on an already-tracked file is a no-op on the DB.
|
|
- **SFTP layer:** the `sftp.stat().st_size == local_size` check before `write_file` short-circuits already-uploaded files.
|
|
|
|
### Failure modes summary
|
|
|
|
- Parse fails → no DB write, no SFTP put, file counted as `failed`.
|
|
- Payer-id mismatch → same as parse fail.
|
|
- DB write fails → no SFTP put, file counted as `failed`. The DB session rolls back; no orphan rows.
|
|
- SFTP connect fails → DB row already committed; file counted as `failed`. Re-run is safe.
|
|
- SFTP stat succeeds + size match → counted as `skipped`; no audit event.
|
|
- SFTP write fails → file counted as `failed`. DB row already committed. Re-run is safe.
|
|
- Crash between DB write and SFTP put → DB row exists, file never landed. Re-run will see the size mismatch (remote doesn't have it) and upload.
|
|
|
|
## 4. Data flow & error handling
|
|
|
|
### `POST /api/submit-batch`
|
|
|
|
Request body (JSON):
|
|
- `ingest_dir: string` — root directory holding `batch-*-claims/*.x12` subfolders (mirrors the CLI flag).
|
|
- `validate: bool = true` — pass-through to `submit_file`.
|
|
- `actor: string = "api-submit-batch"` — audit actor tag.
|
|
- `limit: int | null = null` — stop after N files (smoke test).
|
|
|
|
Response body (200):
|
|
- `submitted: int` — files uploaded this run.
|
|
- `skipped: int` — files already on remote (size match).
|
|
- `failed: int` — files that hit parse / payer / DB / SFTP errors.
|
|
- `results: list<{file: string, outcome: "submitted"|"skipped"|"parse_failed"|"payer_mismatch"|"db_failed"|"sftp_failed", batch_id: string|null, error: string|null}>` — per-file detail.
|
|
|
|
Status codes:
|
|
- `200` — run completed (may have failures; details in the response body).
|
|
- `401` — no auth cookie (matrix_gate).
|
|
- `422` — body validation failure.
|
|
- `500` — unexpected exception (caught by the existing `@app.exception_handler(Exception)`).
|
|
|
|
### CLI exit codes (per `cyclone-cli` convention)
|
|
|
|
- `0` — run completed, even with some per-file failures (the per-file detail is in stdout).
|
|
- `1` — unexpected exception (uncaught error in the walker or the helper itself).
|
|
- `2` — config-level failure (no clearhouse seeded, SFTP block in stub mode for non-stubbed invocation, ingest-dir doesn't exist).
|
|
|
|
### Audit + live-tail events
|
|
|
|
- `claim_submitted` (one per Claim row) — emitted by `add_record` already; no new code path.
|
|
- `clearhouse.submitted` (one per successful upload) — emitted by `submit_file` after the SFTP put succeeds. Same actor tag and payload shape as the current `resubmit-rejected-claims` path uses.
|
|
|
|
### Migration `0013_add_batch_txn_set_control_number.py`
|
|
|
|
- `op.add_column('batches', sa.Column('transaction_set_control_number', sa.String(32), nullable=True))`.
|
|
- Backfill: `UPDATE batches SET transaction_set_control_number = json_extract(raw_result_json, '$.envelope.transaction_set_control_number') WHERE raw_result_json IS NOT NULL AND json_extract(raw_result_json, '$.envelope.transaction_set_control_number') IS NOT NULL`.
|
|
- Reversible: drop column.
|
|
- The down migration is `op.drop_column('batches', 'transaction_set_control_number')`.
|
|
|
|
### Join-key update in `claim_acks.batch_envelope_index`
|
|
|
|
The existing function reads every `Batch` row whose `raw_result_json` has an envelope and returns `dict[str, str]` keyed by `envelope.control_number`. The updated version returns the **same shape** — a single `dict[str, str]` — but **populated from two columns**:
|
|
|
|
- Each row contributes one entry keyed by `envelope.control_number` (preserved).
|
|
- Each row also contributes one entry keyed by `transaction_set_control_number` (new), when that column is non-NULL.
|
|
|
|
If both keys resolve to the same value (Gainwell batches where the ST02 happens to equal the ISA13 — unusual but legal), the second insert is a no-op. If they differ, both keys point to the same `batch.id` — a single `idx.get(set_control_number)` lookup will match either one. The dict may grow by up to `len(batches)` new entries; callers that previously assumed `len(index) == len(batches)` are unaffected because that invariant was already loose (multiple ST02s can share one ISA13).
|
|
|
|
`lookup_claims_for_ack_set_response` Pass 1 is unchanged at the call site — one `idx.get(set_control_number)` lookup that succeeds whether the source 837's ST02 matches a `transaction_set_control_number` row or an `envelope.control_number` row. Pass 2 (PCN) fires as today if both miss.
|
|
|
|
## 5. Testing approach
|
|
|
|
Per `cyclone-tests` skill: pytest under `backend/tests/`, two flavors — `test_api_*` (FastAPI integration) and `test_*` (pure unit). Frontend tests out of scope (no UI changes).
|
|
|
|
### Unit tests (`backend/tests/test_submission.py`)
|
|
|
|
- `submit_file happy path` — mock SFTP, real DB; assert one Batch + N Claim rows + one audit event.
|
|
- `submit_file idempotency on DB` — call `submit_file` twice with the same file; assert second call returns `skipped` (DB no-op).
|
|
- `submit_file idempotency on SFTP` — mock SFTP `stat` to return matching size; assert `skipped`, no second `write_file` call, no second audit event.
|
|
- `submit_file parse fail` — pass a bad 837; assert no DB write, no SFTP call, `parse_failed` outcome.
|
|
- `submit_file payer mismatch` — pass a valid 837 with wrong payer_id; assert no DB write, no SFTP call, `payer_mismatch` outcome.
|
|
- `submit_file DB fail` — monkey-patch `add_record` to raise; assert no SFTP call, `db_failed` outcome.
|
|
- `submit_file SFTP fail` — mock SFTP `write_file` to raise; assert `sftp_failed` outcome, DB row still present.
|
|
- `submit_file re-run after partial failure` — submit a file that fails SFTP, then submit again with SFTP working; assert second run uploads and emits audit.
|
|
|
|
### Integration tests (`backend/tests/test_api_submit_batch.py`)
|
|
|
|
- `POST /api/submit-batch` happy path — 3 valid 837 fixtures in `ingest_dir`; assert 200 + body shape.
|
|
- `POST /api/submit-batch` auth gate — no cookie → 401 (matrix_gate).
|
|
- `POST /api/submit-batch` body validation — missing `ingest_dir` → 422.
|
|
|
|
### Migration tests (`backend/tests/test_migration_0013.py`)
|
|
|
|
- Up + down on a fresh DB; column exists + is nullable; backfill populates the column from raw JSON.
|
|
|
|
### Join-key update tests (`backend/tests/test_claim_acks_index.py`)
|
|
|
|
- `batch_envelope_index` returns both maps; Pass 1 finds a claim by ST02 even when ISA13 differs; Pass 1 finds by ISA13 (backward compat); Pass 2 fallback still works.
|
|
|
|
### Live test (after merge)
|
|
|
|
Walk an actual `ingest/batch-*-claims/` directory via the CLI; verify:
|
|
- `claims` table has new rows with the expected `patient_control_number` (CLM01).
|
|
- `batches` table has new rows with `transaction_set_control_number` matching the parsed ST02.
|
|
- Live-tail `claim_written` events fire on the API.
|
|
|
|
Then drop a synthetic 999 (with the matching AK2 set_control_number) into `ingest/`, run `pull-inbound`, verify the `ClaimAck` link row is created (not an orphan).
|
|
|
|
## 6. Threat model (post-SP24 alignment)
|
|
|
|
No new attack surface. The new endpoint and CLI:
|
|
|
|
- Are gated by the existing `matrix_gate` (HTTP) and `CYCLONE_AUTH_DISABLED` env var.
|
|
- Use the same SFTP client and credential paths as `resubmit-rejected-claims` today.
|
|
- Write to the same store facade; no new SQL surface.
|
|
- Emit the same audit events with the same payload shape.
|
|
|
|
The `transaction_set_control_number` column is operator-visible (it's in the raw `batches` row), but no auth boundary depends on it being absent. PII exposure is unchanged (the column carries the same value the 999 ack would have echoed back — already public-to-Gainwell).
|
|
|
|
## 7. Risks & mitigations
|
|
|
|
- **Risk:** The new `submission` package is the third "command runner" in the codebase (alongside `cli.py`'s standalone commands and `scheduler.py`). **Mitigation:** Keep `submit_file` as a pure helper with no FastAPI/Click dependencies; the API and CLI handlers are thin wrappers. Document the layering in `submission/__init__.py`'s module docstring.
|
|
- **Risk:** `resubmit-rejected-claims` and `submit-batch` will diverge over time (one writes to DB, one doesn't). **Mitigation:** Both call the same `submit_file` helper if `submit-batch`'s `--write-to-db` flag is set to true (default). Document the divergence point in RUNBOOK.
|
|
- **Risk:** Migration backfill could be slow on a large DB. **Mitigation:** `UPDATE ... WHERE json_extract(...) IS NOT NULL` is one pass; on a million-row DB this is sub-second. Index on `transaction_set_control_number` not added (low cardinality per batch; existing `ix_claims_patient_control_number` covers the hot path).
|
|
- **Risk:** Live-tail event volume doubles when `submit-batch` runs (one `claim_submitted` per claim in addition to the historical `claim_written`). **Mitigation:** This is desired behavior — the operator wants the activity feed to show submissions. Document in the RUNBOOK that submitting a 1000-claim batch will publish ~1000 events.
|
|
|
|
## 8. Rollout
|
|
|
|
- **Branch:** `sp37-submit-batch-canonical-flow`, off main as of `710104f`.
|
|
- **Implementation order:**
|
|
1. Migration `0013` + backfill (additive, no behavior change yet).
|
|
2. Update `claim_acks.batch_envelope_index` + Pass 1 lookup (no behavior change yet — the new map is built but unused until `submit-batch` writes rows).
|
|
3. `cyclone/submission/core.py` helper + unit tests.
|
|
4. CLI handler `cyclone submit-batch` + smoke test.
|
|
5. HTTP handler `POST /api/submit-batch` + integration tests.
|
|
6. Live test with one of the existing `ingest/batch-*-claims/*.x12` directories (will be in stub mode on this host).
|
|
- **Live test cadence:** per the user's standing directive — live-test after each significant step, run autoreview, commit.
|
|
- **Merge shape:** single atomic merge commit per SP-N convention. No squash. No rebase.
|
|
- **Post-merge:** RUNBOOK update for the canonical flow; CLAUDE.md update to mark `submit-batch` as preferred.
|
|
|
|
## 9. Open questions
|
|
|
|
None blocking the spec. Tracked for follow-up SPs:
|
|
|
|
- **Backfill command** — once `submit-batch` is proven, a `cyclone backfill-claims` CLI that walks the existing `ingest/corrected/batch-*-claims/*.x12` backlog (without uploading — they're already on Gainwell's side) and writes the DB rows. This is what would un-orphan the 1491+ orphan 999 acks currently in the DB.
|
|
- **Deprecation timeline for `resubmit-rejected-claims`** — once `submit-batch` is the default, consider folding `resubmit-rejected-claims` into a `--no-write-to-db` flag on `submit-batch` to consolidate the surface.
|
|
- **Cross-link to 277CA acks** — same `transaction_set_control_number` join key will resolve 277CA orphans if/when those arrive. The migration and index update cover both 999 and 277CA since both use Pass 1. |