cyclone/docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md

# Sub-project 35 — Parse Input Guards: Design Spec

**Date:** 2026-07-06
**Status:** Draft, awaiting user sign-off
**Branch:** `sp35-parse-input-guards` (off `main`, post-SP33)
**Aesthetic direction:** No UI changes. The Upload page gets a small invisible behavior change (auto-flip the `Kind` select when the file content disagrees) and keeps its current visual design.

---

## 1. Scope

Today, dropping any X12 file on the Upload page with the default `Kind: 837P` submits the file to `POST /api/parse-837` regardless of whether the bytes look like an 837P or an 835. The `/api/parse-837` endpoint silently accepts that submission: the parser reads the ISA/ST envelope, finds zero CLM segments (because 835s carry `CLP`, not `CLM`), returns an empty `ParseResult`, and the endpoint then persists a `kind='837p'` batch row with `total_claims=0`. The bogus batch shows up in the Batches view, no remittance or claim rows are produced, and the operator has no way to tell from the UI that the ingest was misrouted. SP34 left two such bogus batches in production (`50eb50c1…` and `e4692571…`) when an operator (or test) dropped a `tp11525703-835_M019771179-20260706005516577-1of1.x12` file at the project root and ran the upload twice without changing the default `Kind`.

SP35 closes two independent layers that both contribute to the silent-corruption path. The fix is layered (defense in depth): either layer alone would prevent the symptom, but each layer is a separate invariant that needs its own test and its own enforcement.

**Investigation finding (shapes the scope):** I read each parser before locking the scope. `/api/parse-837` and `/api/parse-835` are the only parse endpoints where the parser can return a successful-looking empty result. The 999/277CA/TA1 endpoints already enforce envelope/segment guards at the parser layer:

- **`parse_999`** (`backend/src/cyclone/parsers/parse_999.py:289-290`): raises `CycloneParseError("No AK9 (Functional Group Response Status) segment found")` when the parser finds no `AK9` segments. There is no silent-corruption path because 999s that don't carry `AK9` (everything that isn't a 999) get rejected.
- **`parse_277ca`** (`backend/src/cyclone/parsers/parse_277ca.py:297-298`): raises `"Expected ST*277 or ST*277CA, got ST*<other>"` when the envelope token doesn't match. Strict envelope guard.
- **`parse_ta1`** (`backend/src/cyclone/parsers/parse_ta1.py:111`): raises `"Expected TA1, got <other>"` when the first segment after ISA isn't `TA1*`. Strict envelope guard.

The right scope is therefore: **fix the two endpoints that have the bug** (837p and 835), and **add regression tests** to the three endpoints that already have strict guards so we catch any future regression where someone loosens the parser.

**In scope:**

- **Server-layer input guard on `POST /api/parse-837`.** Two checks before any `store.add(...)` runs:
  1. **Envelope check.** The first `ST*` segment (or first 4096 bytes, whichever comes first) must begin with `ST*837`. If not, the endpoint returns `400` with `{error: "Mismatched file kind", detail: "...", expected: "837p", detected_st: "<token>"}` and persists nothing.
  2. **Empty-claims check.** After the parser runs, if `len(result.claims) == 0`, the endpoint returns `400` with `{error: "No claims parsed", detail: "..."}` and persists nothing.
- **Same two checks on `POST /api/parse-835`**, mirrored for symmetry. The 835 parser requires `BPR` and `TRN` (raises `CycloneParseError` if either is missing) but does NOT require `CLP` segments — a stripped-down 835 with `ISA*BPR*TRN*SE*GE*IEA` and no `CLP` would silently persist as an empty `claims=[]` batch today. The new envelope guard catches the "this is an 837 file" misroute; the empty-claims guard catches this "valid-835-shape but no claims" edge case.
- **Regression tests on `POST /api/parse-999`, `POST /api/parse-277ca`, `POST /api/parse-ta1`** that assert these endpoints already return 400 (not 200) when fed an unrelated X12 file. These lock the current strict-parser behavior in — if a future PR loosens the parser envelope guards, these tests fail and prevent a recurrence of the SP34-class bug.
- **UI-layer auto-detect in `src/pages/Upload.tsx`.** When the user drops or selects a file, the page reads the first ~4 KB of the file as text, looks for the first `ST*…` token, and if it finds `ST*837` the `Kind` select flips to `837P` (no-op if already there), and if it finds `ST*835` the select flips to `835`. If neither token is found (a `.txt` export without an `ST*` header, or a PDF, or a corrupted file) the select stays where it is and the user keeps manual control. (999/277CA/TA1 don't enter the UI flow today — the Upload page only has 837p/835 in the Kind select — so no UI auto-detect on those kinds is needed.)
- **Backend tests** covering the four new guards on `/api/parse-835` (mirroring parse-837) and the three regression tests on the 999/277CA/TA1 endpoints.
- **Frontend test** in `src/pages/Upload.test.tsx` covering the auto-detect for both 837 and 835 file payloads, plus a "no ST* found → no flip" regression case.
- **Cleanup of the two existing bogus batches** from the production DB (`50eb50c1…` and `e4692571…`) by direct SQL after the fix lands and is verified in production.

**Out of scope:**

- No schema migration. No new tables, no new columns, no new foreign keys.
- No changes to the `parse_837` / `parse_835` parser modules themselves (the empty-claims check is at the API layer, where the `store.add(...)` decision lives; moving it into the parsers would change CLI semantics).
- No changes to any CLI subcommand. The CLI `parse-837` and `parse-835` already raise `click.UsageError` on empty claims (consistent with what SP35 makes the API do); the SP only closes the API-vs-CLI symmetry.
- No changes to the 999 / 277CA / TA1 / 270 / 271 endpoints. They each have their own parsers; if a similar bug exists on them it is out of scope for SP35 and should be filed as a follow-up if confirmed.
- No new admin endpoints for batch deletion. Cleanup is a one-line direct SQL run via `docker exec cyclone-backend-1 sqlite3 …` and is documented in the plan, not in the product surface.
- No `cyclone admin` changes.
- No changes to the activity-events flood (a separate observation from the same incident — one 835 produced hundreds of per-claim `reconcile` activity events; a real concern but a separate ticket).
- No change to the auth boundary. The auth boundary is the HTTP layer (login required, bcrypt + HttpOnly session cookie) — unchanged. The new 400 responses on `/api/parse-837` and `/api/parse-835` are produced under `matrix_gate`, same as today.

---

## 2. Decisions (locked during brainstorming)

### D1. Two-layer defense: server guards AND UI auto-detect. Each one alone is insufficient.

If we fix only the server, the symptom goes away but the operator still gets a confusing 400 with no explanation for why their drop didn't work — every bad drop produces a 400 instead of a 200 with a silent empty batch. Fixing the UI without the server leaves the API as a footgun for any other client (curl, third-party tooling, future ingestion paths). Both layers are needed because the server guard is the **invariant** (correctness) and the UI auto-detect is the **operator experience** (correct-by-default UX).

### D2. The server check is "ST* token + empty claims," not just one or the other

Relying on `ST*` alone fails on truncated headers (the parser can't even reach the ST if the ISA is corrupt, so the message becomes a generic 400). Relying on empty-claims alone leaves the door open for empty-but-valid-ISA 999 files to silently hit the 837p endpoint and produce empty batch rows (the bug we're closing). The two checks together produce a clear, two-stage error: "your file's ST token isn't 837p" before parse, or "your 837p parse produced zero claims" after parse.

### D3. The UI auto-detect reads the first 4 KB of the file, not the whole file

X12 `ST*837` / `ST*835` always appears in the first few hundred bytes of a well-formed file (after ISA and GS). Reading 4 KB guarantees capture without loading multi-megabyte files into memory in the browser. The check runs in `pickFile()` synchronously on the dropped `File` object via `FileReader.readAsText(file.slice(0, 4096))`.

### D4. The auto-detect never overrules a user who has manually picked `835`

If the user explicitly selects `835` from the dropdown, then drops a file whose content says `ST*837`, the auto-detect still wins (it overrides the select). Rationale: the whole point of the auto-detect is to prevent the silent-corruption failure mode; honoring a stale manual override would defeat the purpose. If the operator wants to "force 837p on an 835 file" — a legitimate test case — they can flip the select back manually after the auto-detect fires; the file is still in `stream.file` and the parse button hasn't been clicked yet.

### D5. Cleanup is direct SQL, not a new admin endpoint

The two bogus batches (`50eb50c1…` and `e4692571…`) have `kind='837p'`, `input_filename` of the 835 file, `total_claims=0`, and zero `claims` / `service_line_payments` / `cas_adjustments` / `matches` rows pointing at them. A one-shot `DELETE FROM batches WHERE id IN (...)` is sufficient and does not require a new product surface. The plan documents the exact command and where to run it (`docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db "..."`). A future admin-batch-delete endpoint can be a separate SP if there's operator demand.

### D6. No 409 vs 400 nuance — every bad input is a 400

The existing endpoint returns `400` for `Empty file`, `Encoding error`, and `Parse error`; it returns `409` for `Duplicate claim` (a state conflict, not an input error). SP35's new failures (`Mismatched file kind`, `No claims parsed`) are input errors, so they get `400`. The error envelope shape (`{error, detail}`) is preserved.

### D7. No new audit-log entry for "rejected at ingest"

The existing audit log captures state-affecting actions (admin role changes, batch deletes, etc.), not failed parse attempts. Adding `parse_rejected` events would conflate operational noise with the user-action audit trail. If observable rejection events matter in the future, they can be a separate SP that adds a structured log channel.

### D8. Frontend test lives in `src/pages/Upload.test.tsx` and uses the existing `vi.mock("@/lib/api", …)` pattern

The new behavior is a thin synchronous change to `pickFile()`; the test mocks `FileReader` (or uses `Blob` directly via `URL.createObjectURL` + `<input>` event firing) and asserts the resulting `kind` state. No new test framework, no new mocking library.

---

## 3. Edge cases & how they're handled

| Scenario | Behavior |
|---|---|
| Drop a valid 837 file with default `kind='837p'` | No change. Already correct; auto-detect is a no-op. |
| Drop a valid 835 file with default `kind='837p'` | Auto-detect flips `kind` to `835`. Parse succeeds. |
| Drop a valid 835 file with `kind='835'` already selected | Auto-detect is a no-op. Parse succeeds. |
| Drop a 999 file with default `kind='837p'` | Auto-detect finds no `ST*837` and no `ST*835`, so leaves `kind` alone. Parse fails with `Mismatched file kind` 400. |
| Drop a non-X12 `.txt` with default `kind='837p'` | Same as above: no auto-detect, parse fails clearly. |
| Operator manually selects `835` then drops an 837 | Auto-detect flips to `837p` (D4). Parse succeeds. |
| Bypass the UI entirely and POST an 835 file to `/api/parse-837` via curl | Server guard rejects with `400 Mismatched file kind`. No batch row persisted. |
| Bypass the UI and POST a valid 837 with zero CLM segments (empty ISA envelope) to `/api/parse-837` | Server guard rejects with `400 No claims parsed`. No batch row persisted. |
| Re-ingest the same valid 835 to `/api/parse-835` (duplicate) | Existing `409 Duplicate remittance` flow unchanged. SP35 does not touch the dedup logic. |
| User clicks `Parse file` twice rapidly with the same file | First request persists; second request hits dedup and gets `409`. Unchanged. |
| Frontend auto-detect reads a 4 KB slice that straddles a multi-segment envelope | Unlikely on real EDI; the `ST*` always appears in the first ~500 bytes after ISA/GS. Regression test covers the `ST*835` at byte 3800 case. |

---

## 4. Testing approach

**Backend (Python):** add to existing `tests/test_api.py` (`/api/parse-837`), `tests/test_api_835.py`, `tests/test_api_999.py`, `tests/test_api_277ca.py`, `tests/test_api_ta1.py`:

- **`test_parse_837_endpoint_rejects_835_input`** — POST the CO Medicaid 835 fixture to `/api/parse-837`, assert `400`, error envelope `error == "Mismatched file kind"`, no new `batches` row.
- **`test_parse_837_endpoint_rejects_empty_envelope`** — POST a syntactically valid ISA with no CLM segments to `/api/parse-837`, assert `400`, error envelope `error == "No claims parsed"`, no new `batches` row.
- **`test_parse_835_endpoint_rejects_837_input`** — symmetric: POST 837 to `/api/parse-835`, expect `400 Mismatched file kind`.
- **`test_parse_835_endpoint_rejects_empty_envelope`** — symmetric: POST a stripped ISA+BPR+TRN+SE+GE+IEA with no `CLP` to `/api/parse-835`, expect `400 No claims parsed`.
- **`test_parse_999_endpoint_rejects_837_input`** — regression lock: POST 837 fixture to `/api/parse-999`, expect `400 Parse error` (raised by the parser's `No AK9` rule). Documents that the 999 parser already has this invariant enforced.
- **`test_parse_277ca_endpoint_rejects_835_input`** — regression lock: POST 835 fixture to `/api/parse-277ca`, expect `400 Parse error` (raised by the parser's `Expected ST*277` rule).
- **`test_parse_ta1_endpoint_rejects_835_input`** — regression lock: POST 835 fixture to `/api/parse-ta1`, expect `400 Parse error` (raised by the parser's `Expected TA1` rule).
- **`test_parse_837_endpoint_happy_path`** (existing) — regression guard: CO Medicaid 837p fixture still parses and persists cleanly.
- **`test_parse_835_endpoint_happy_path`** (existing) — regression guard: CO Medicaid 835 fixture still parses and persists cleanly.

**Frontend (Vitest):** add to `src/pages/Upload.test.tsx` (sibling file exists per the project convention):

- **`upload_auto_detect_837_flips_kind_to_837p`** — drop a file whose first 4 KB contain `ST*837`, assert `kind` state is `837p`.
- **`upload_auto_detect_835_flips_kind_to_835`** — drop a file whose first 4 KB contain `ST*835`, assert `kind` state is `835`.
- **`upload_auto_detect_no_st_token_leaves_kind_unchanged`** — drop a file with no `ST*` header, assert `kind` state is whatever the user had selected.

**Manual smoke (live stack):**

- Drop the `tp11525703-835_M019771179-20260706005516577-1of1.x12` file at the project root with default `Kind: 837P` selected. Expect: select flips to `835`, parse succeeds, 1148 claim rows land in `remittances`. Repeat with `Kind: 835` already selected. Expect: no flip, same outcome.
- POST the same 835 file via curl to `/api/parse-837`. Expect: `400 Mismatched file kind`, no batch row written.

---

## 5. Out-of-scope reminders (explicit)

- **No new guards on `/api/parse-270` or `/api/parse-271`.** Those endpoints handle eligibility-benefit pairs. They may or may not have the same parse-empty shape; out of scope for SP35. (Filing as a follow-up if confirmed.)
- **No production-side changes to the 999 / 277CA / TA1 endpoint code.** Only regression tests are added; the parsers themselves already reject mismatched input.
- **No `cyclone admin batches delete` endpoint.** Cleanup is direct SQL for now.
- **No schema migration.** The data model is unchanged.
- **No changes to validation rules** in `cyclone.parsers.validator_837` / `cyclone.parsers.validator_835`. SP35 only adds guards at the API layer.
- **No change to the activity-events storm.** This is a perf/observability concern, not a correctness bug, and will get its own ticket.
- **No auth changes.** The new 400 responses inherit `matrix_gate` and the existing session-cookie auth boundary.
- **No changes to the running production stack.** SP35 is developed against `main`, merged in, then the running containers are restarted via the existing compose-up flow documented in `RUNBOOK.md`. No hot-patches.