diff --git a/docs/superpowers/specs/2026-07-07-cyclone-submit-batch-canonical-flow-design.md b/docs/superpowers/specs/2026-07-07-cyclone-submit-batch-canonical-flow-design.md new file mode 100644 index 0000000..7873a8b --- /dev/null +++ b/docs/superpowers/specs/2026-07-07-cyclone-submit-batch-canonical-flow-design.md @@ -0,0 +1,217 @@ +# 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 `/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. \ No newline at end of file