Nora
857 lines
33 KiB
Markdown
857 lines
33 KiB
Markdown
# SP35 — Parse Input Guards Implementation Plan
|
|
|
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
|
|
|
**Goal:** Stop the silent-corruption path where dropping an X12 file on the Upload page at default `Kind: 837P` persists an empty `kind='837p'` batch row for an 835 (or any non-837p) file. Fix at both the server (reject bad input, persist nothing) and the UI (auto-flip the `Kind` select when file content disagrees).
|
|
|
|
**Architecture:** Layer the fix. The **server guards** (`POST /api/parse-837` and `POST /api/parse-835`) get two checks each — a cheap envelope check (look for the expected `ST*` token in the first 4 KB of the upload) BEFORE `parse(...)`, and an empty-claims check AFTER `parse(...)` and BEFORE `store.add(...)`. The **UI auto-detect** lives in `Upload.tsx`'s `pickFile()` and inspects the first 4 KB via `FileReader.readAsText(f.slice(0, 4096))` to set the `kind` state. Two layers because each is a separate invariant: the server guard is a correctness invariant (any client — UI, curl, future ingestion paths — gets the same response); the UI auto-detect is the operator-experience invariant (the Upload page is "correct by default").
|
|
|
|
**Tech Stack:** Python 3.11+ (FastAPI, SQLAlchemy 2.x, pytest), React 18 + TypeScript (Vitest, happy-dom, `@testing-library/react`). No new dependencies. No schema migration. No CLI changes.
|
|
|
|
**Spec:** [`docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md`](../specs/2026-07-06-cyclone-parse-input-guards-design.md)
|
|
|
|
---
|
|
|
|
## File Structure
|
|
|
|
| File | Change | Responsibility |
|
|
|---|---|---|
|
|
| `backend/src/cyclone/api.py` | Modify | Add envelope + empty-claims guards to `/api/parse-837` (lines ~384-510) and `/api/parse-835` (lines ~570-680). Extract a tiny `_envelope_st_token(text) -> str \| None` helper at module scope so both endpoints share it. No changes to the 999/277CA/TA1 endpoints (parsers are already strict). |
|
|
| `backend/tests/test_api.py` | Modify | Add `test_parse_837_endpoint_rejects_835_input`, `test_parse_837_endpoint_rejects_empty_envelope`, `test_parse_837_does_not_persist_when_rejected`. |
|
|
| `backend/tests/test_api_835.py` | Modify | Add `test_parse_835_endpoint_rejects_837_input`, `test_parse_835_endpoint_rejects_empty_envelope`, `test_parse_835_does_not_persist_when_rejected`. |
|
|
| `backend/tests/test_api_999.py` | Modify | Add `test_parse_999_endpoint_rejects_837_input` regression lock. |
|
|
| `backend/tests/test_api_277ca.py` | Modify | Add `test_parse_277ca_endpoint_rejects_835_input` regression lock. |
|
|
| `backend/tests/test_api_ta1.py` | Modify | Add `test_parse_ta1_endpoint_rejects_835_input` regression lock. |
|
|
| `src/pages/Upload.tsx` | Modify | Add tiny `_detectEdiKind(text: string): "837p" \| "835" \| null` helper at module scope; in `pickFile()`, async-read the first 4 KB and call `_detectEdiKind` to seed `kind` when a definite token is found. |
|
|
| `src/pages/Upload.test.tsx` | Modify | Add `upload_auto_detect_*` tests (3) covering 837 / 835 / no-token cases. |
|
|
| `docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md` | Add | Spec, written first. |
|
|
| `docs/superpowers/plans/2026-07-06-cyclone-parse-input-guards.md` | Add | This plan. |
|
|
|
|
---
|
|
|
|
## Task 1: Land the spec on `main` (docs only, no implementation)
|
|
|
|
**Files:**
|
|
- Add: `docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md`
|
|
|
|
- [ ] **Step 1: Commit the spec on the branch**
|
|
|
|
The spec is already written at the path above. Open it for one last review, then:
|
|
|
|
```bash
|
|
git add docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md
|
|
git commit -m "docs(spec): SP35 parse-input-guards — defense in depth against misroute silent-corruption"
|
|
```
|
|
|
|
- [ ] **Step 2: Land this plan on the branch**
|
|
|
|
The plan is in place at `docs/superpowers/plans/2026-07-06-cyclone-parse-input-guards.md`.
|
|
|
|
```bash
|
|
git add docs/superpowers/plans/2026-07-06-cyclone-parse-input-guards.md
|
|
git commit -m "docs(plan): SP35 parse-input-guards — server guards + UI auto-detect, TDD-first"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 2: Server-side guard on `/api/parse-837` (TDD)
|
|
|
|
**Files:**
|
|
- Modify: `backend/src/cyclone/api.py` (around lines 384-510 for `/api/parse-837`)
|
|
- Modify: `backend/tests/test_api.py` (append new tests at end)
|
|
|
|
- [ ] **Step 1: Write the failing tests**
|
|
|
|
Append to `backend/tests/test_api.py`:
|
|
|
|
```python
|
|
# --- SP35: parse-837 input guards ------------------------------------------
|
|
|
|
def test_parse_837_endpoint_rejects_835_input(client: TestClient):
|
|
"""Posting an 835 file to /api/parse-837 returns 400, no batch row."""
|
|
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
|
|
text = fixture.read_text()
|
|
|
|
pre_count = global_store.list_batches().__len__() if hasattr(global_store, "list_batches") else None
|
|
resp = client.post(
|
|
"/api/parse-837",
|
|
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
|
|
headers={"Accept": "application/json"},
|
|
)
|
|
assert resp.status_code == 400, resp.text
|
|
body = resp.json()
|
|
assert body["error"] == "Mismatched file kind"
|
|
assert body["expected"] == "837p"
|
|
assert body.get("detected_st", "").startswith("835")
|
|
# Confirm no batch row was persisted. The simplest assertion is "no
|
|
# additional claims rows appeared" — list via the existing list endpoint.
|
|
claims_after = client.get("/api/claims?limit=1").json()["claims"]
|
|
assert claims_after == []
|
|
# (Or, if /api/batches exists, query it and assert no new kind='837p'
|
|
# batch was added for this filename.)
|
|
|
|
|
|
def test_parse_837_endpoint_rejects_empty_envelope(client: TestClient):
|
|
"""Syntactically valid ISA but no CLM segments → 400 'No claims parsed'."""
|
|
# A minimal envelope that gets past ISA parsing but produces zero claims.
|
|
text = (
|
|
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER "
|
|
"*260706*0243*^*00501*000000001*0*P*:~"
|
|
"GS*HC*SENDER*RECEIVER*20260706*0243*1*X*005010X222A1~"
|
|
"ST*837*0001~"
|
|
"BHT*0019*00*0001*20260706*0243*CH~"
|
|
"SE*2*0001~"
|
|
"GE*1*1~"
|
|
"IEA*1*000000001~"
|
|
)
|
|
resp = client.post(
|
|
"/api/parse-837",
|
|
files={"file": ("empty.837p", text, "text/plain")},
|
|
headers={"Accept": "application/json"},
|
|
)
|
|
assert resp.status_code == 400, resp.text
|
|
body = resp.json()
|
|
assert body["error"] == "No claims parsed"
|
|
|
|
|
|
def test_parse_837_endpoint_happy_path_still_works(client: TestClient):
|
|
"""Regression guard — the existing co_medicaid_837p fixture still parses."""
|
|
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
|
|
text = fixture.read_text()
|
|
resp = client.post(
|
|
"/api/parse-837",
|
|
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
|
|
headers={"Accept": "application/json"},
|
|
)
|
|
assert resp.status_code == 200, resp.text
|
|
```
|
|
|
|
If `/api/claims` doesn't take a `limit` parameter, swap that assertion for whatever the canonical list endpoint is (`/api/batches`, `/api/inbox`, etc.) — the goal is "confirm no new batch/claims row was persisted". Read `src/lib/api.ts` and pick the endpoint the frontend actually calls.
|
|
|
|
- [ ] **Step 2: Run the new tests to verify they FAIL**
|
|
|
|
```bash
|
|
cd backend && .venv/bin/pytest tests/test_api.py -k "rejects or happy_path_still_works" -v
|
|
```
|
|
|
|
Expected: the two `rejects_*` tests fail (current code returns 200 for any file with a parseable ISA envelope). The `happy_path_still_works` test passes (regression guard).
|
|
|
|
- [ ] **Step 3: Implement the guards in `/api/parse-837`**
|
|
|
|
In `backend/src/cyclone/api.py`, at module scope near other helpers (e.g. just below `_resolve_payer`), add:
|
|
|
|
```python
|
|
def _envelope_st_token(text: str, scan_bytes: int = 4096) -> str | None:
|
|
"""Return the ST01 token from the first ``scan_bytes`` of ``text``.
|
|
|
|
Examples: returns "837" for ``ST*837*0001``, "835" for ``ST*835*1001``.
|
|
Returns ``None`` if no ST segment is found in the scan window.
|
|
"""
|
|
head = text[:scan_bytes]
|
|
for line in head.split("~"):
|
|
line = line.strip("\r\n ")
|
|
if line.startswith("ST*"):
|
|
parts = line.split("*")
|
|
if len(parts) >= 2:
|
|
return parts[1]
|
|
return None
|
|
```
|
|
|
|
(Adapt to use `Optional` instead of `str | None` if the file already imports Python 3.10-style optionals. Read the top of `api.py` for the style.)
|
|
|
|
Then in the `parse_837` handler (around line 384), after the `text = raw.decode("utf-8")` block, before the `result = parse(text, ...)` call:
|
|
|
|
```python
|
|
# SP35: envelope kind guard. Reject files whose ST* token doesn't
|
|
# match the endpoint's expected kind. Two-layer defense: this catches
|
|
# the obvious misroute (835 dropped on the 837p page); the empty-claims
|
|
# check below catches the less-obvious case of a syntactically valid
|
|
# file with no CLM segments.
|
|
detected = _envelope_st_token(text)
|
|
if detected is not None and detected != "837":
|
|
return JSONResponse(
|
|
status_code=400,
|
|
content={
|
|
"error": "Mismatched file kind",
|
|
"detail": (
|
|
f"This endpoint expects an 837P file; the uploaded "
|
|
f"file's envelope declares ST*{detected}*."
|
|
),
|
|
"expected": "837p",
|
|
"detected_st": detected,
|
|
},
|
|
)
|
|
```
|
|
|
|
And after the `_has_claim_validation_errors(result)` block — BEFORE the `BatchRecord(...)` + `store.add(...)` block, add:
|
|
|
|
```python
|
|
# SP35: empty-claims guard. If the parser produced zero claims (e.g.
|
|
# the file is a well-formed 999 or a truncated 837p with no CLM),
|
|
# refuse to persist a successful-looking batch row.
|
|
if not result.claims:
|
|
return JSONResponse(
|
|
status_code=400,
|
|
content={
|
|
"error": "No claims parsed",
|
|
"detail": (
|
|
"The parser did not extract any claim segments from this "
|
|
"file. Confirm the file is a valid 837P professional "
|
|
"claim with one or more CLM/CLM01 loops."
|
|
),
|
|
},
|
|
)
|
|
```
|
|
|
|
- [ ] **Step 4: Run the new tests to verify they PASS**
|
|
|
|
```bash
|
|
cd backend && .venv/bin/pytest tests/test_api.py -k "rejects or happy_path_still_works" -v
|
|
```
|
|
|
|
Expected: all 3 tests green.
|
|
|
|
- [ ] **Step 5: Run the full `/api/parse-837` test surface to verify no regressions**
|
|
|
|
```bash
|
|
cd backend && .venv/bin/pytest tests/test_api.py -k "837" -v
|
|
```
|
|
|
|
Expected: all green (existing happy-path + NDJSON streaming tests + new guards).
|
|
|
|
- [ ] **Step 6: Commit**
|
|
|
|
```bash
|
|
git add backend/src/cyclone/api.py backend/tests/test_api.py
|
|
git commit -m "feat(sp35): add envelope + empty-claims guards to /api/parse-837"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 3: Server-side guard on `/api/parse-835` (mirrored)
|
|
|
|
**Files:**
|
|
- Modify: `backend/src/cyclone/api.py` (around lines 570-680 for `/api/parse-835`)
|
|
- Modify: `backend/tests/test_api_835.py` (append new tests)
|
|
|
|
- [ ] **Step 1: Write the failing tests**
|
|
|
|
Append to `backend/tests/test_api_835.py`:
|
|
|
|
```python
|
|
# --- SP35: parse-835 input guards ------------------------------------------
|
|
|
|
def test_parse_835_endpoint_rejects_837_input(client: TestClient):
|
|
"""Posting an 837P file to /api/parse-835 returns 400, no batch row."""
|
|
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
|
|
text = fixture.read_text()
|
|
resp = client.post(
|
|
"/api/parse-835",
|
|
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
|
|
headers={"Accept": "application/json"},
|
|
)
|
|
assert resp.status_code == 400, resp.text
|
|
body = resp.json()
|
|
assert body["error"] == "Mismatched file kind"
|
|
assert body["expected"] == "835"
|
|
assert body.get("detected_st", "").startswith("837")
|
|
|
|
|
|
def test_parse_835_endpoint_rejects_empty_envelope(client: TestClient):
|
|
"""ST*835 envelope with no CLP segments → 400 'No claims parsed'."""
|
|
text = (
|
|
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER "
|
|
"*260706*0243*^*00501*000000001*0*P*:~"
|
|
"GS*HP*SENDER*RECEIVER*20260706*0243*1*X*005010X221A1~"
|
|
"ST*835*1001~"
|
|
"BPR*I*0*C*NON*CCP*01*123456789*DA*0000000*20260706~"
|
|
"TRN*1*000000001*1811725341~"
|
|
"SE*4*1001~"
|
|
"GE*1*1~"
|
|
"IEA*1*000000001~"
|
|
)
|
|
resp = client.post(
|
|
"/api/parse-835",
|
|
files={"file": ("empty.835", text, "text/plain")},
|
|
headers={"Accept": "application/json"},
|
|
)
|
|
assert resp.status_code == 400, resp.text
|
|
body = resp.json()
|
|
assert body["error"] == "No claims parsed"
|
|
|
|
|
|
def test_parse_835_endpoint_happy_path_still_works(client: TestClient):
|
|
"""Regression guard — the co_medicaid_835 fixture still parses."""
|
|
text = FIXTURE.read_text()
|
|
resp = client.post(
|
|
"/api/parse-835",
|
|
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
|
|
headers={"Accept": "application/json"},
|
|
)
|
|
assert resp.status_code == 200, resp.text
|
|
```
|
|
|
|
- [ ] **Step 2: Run the new tests to verify they FAIL**
|
|
|
|
```bash
|
|
cd backend && .venv/bin/pytest tests/test_api_835.py -k "rejects or happy_path_still_works" -v
|
|
```
|
|
|
|
Expected: the two `rejects_*` tests fail. The `happy_path` test passes.
|
|
|
|
- [ ] **Step 3: Implement the guards in `/api/parse-835`**
|
|
|
|
Mirror the change from Task 2 Step 3, but for the 835 endpoint and `ST*835`:
|
|
|
|
In `parse_835_endpoint` (around line 571), after `text = raw.decode("utf-8")`:
|
|
|
|
```python
|
|
# SP35: envelope kind guard. See Task 2 notes.
|
|
detected = _envelope_st_token(text)
|
|
if detected is not None and detected != "835":
|
|
return JSONResponse(
|
|
status_code=400,
|
|
content={
|
|
"error": "Mismatched file kind",
|
|
"detail": (
|
|
f"This endpoint expects an 835 file; the uploaded "
|
|
f"file's envelope declares ST*{detected}*."
|
|
),
|
|
"expected": "835",
|
|
"detected_st": detected,
|
|
},
|
|
)
|
|
```
|
|
|
|
After the validator block (~line 635), before the existing `BatchRecord(...)` + `store.add(...)`:
|
|
|
|
```python
|
|
# SP35: empty-claims guard. See Task 2 notes.
|
|
if not result.claims:
|
|
return JSONResponse(
|
|
status_code=400,
|
|
content={
|
|
"error": "No claims parsed",
|
|
"detail": (
|
|
"The parser did not extract any claim-payment segments "
|
|
"from this file. Confirm the file is a valid 835 ERA "
|
|
"remittance with one or more CLP/CLP01 loops."
|
|
),
|
|
},
|
|
)
|
|
```
|
|
|
|
- [ ] **Step 4: Run the new tests to verify they PASS**
|
|
|
|
```bash
|
|
cd backend && .venv/bin/pytest tests/test_api_835.py -v
|
|
```
|
|
|
|
Expected: all tests green (existing 6 + new 3).
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add backend/src/cyclone/api.py backend/tests/test_api_835.py
|
|
git commit -m "feat(sp35): add envelope + empty-claims guards to /api/parse-835 (mirror)"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 4: Regression locks for `/api/parse-999`, `/api/parse-277ca`, `/api/parse-ta1`
|
|
|
|
The 999/277CA/TA1 endpoints already reject mismatched input at the parser layer (their parsers raise `CycloneParseError` on missing `AK9` / wrong `ST*` / missing `TA1` segment respectively). SP35 doesn't add any new code to those endpoints — but we add **regression tests** so a future PR that loosens a parser envelope guard gets caught.
|
|
|
|
**Files:**
|
|
- Modify: `backend/tests/test_api_999.py`
|
|
- Modify: `backend/tests/test_api_277ca.py`
|
|
- Modify: `backend/tests/test_api_ta1.py`
|
|
|
|
- [ ] **Step 1: Read each existing test file to learn the import / fixture conventions**
|
|
|
|
```bash
|
|
head -50 /home/tyler/dev/cyclone/backend/tests/test_api_999.py
|
|
head -50 /home/tyler/dev/cyclone/backend/tests/test_api_277ca.py
|
|
head -50 /home/tyler/dev/cyclone/backend/tests/test_api_ta1.py
|
|
```
|
|
|
|
Mirror the existing pattern. The test_api_835.py file is the closest template — same fixture imports, same `client` fixture, same `client.post(...)` shape.
|
|
|
|
- [ ] **Step 2: Add the regression test to `test_api_999.py`**
|
|
|
|
Append:
|
|
|
|
```python
|
|
# --- SP35 regression: 999 endpoint rejects non-999 input -----------------
|
|
|
|
def test_parse_999_endpoint_rejects_837_input(client: TestClient):
|
|
"""Regression lock — the 999 parser must reject 837 input.
|
|
|
|
The 999 parser raises ``CycloneParseError("No AK9 (Functional Group
|
|
Response Status) segment found")`` when the input has no AK9 segment
|
|
(which an 837 file does not). The endpoint surfaces this as a 400
|
|
Parse error. This test guards against a future PR that loosens the
|
|
AK9 requirement.
|
|
"""
|
|
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
|
|
text = fixture.read_text()
|
|
resp = client.post(
|
|
"/api/parse-999",
|
|
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
|
|
headers={"Accept": "application/json"},
|
|
)
|
|
assert resp.status_code == 400, resp.text
|
|
body = resp.json()
|
|
assert body["error"] == "Parse error"
|
|
# The detail message is the parser's own error string — confirm it
|
|
# mentions AK9 so a future loosen-the-parser PR is loudly caught.
|
|
assert "AK9" in body["detail"]
|
|
```
|
|
|
|
- [ ] **Step 3: Add the regression test to `test_api_277ca.py`**
|
|
|
|
Append:
|
|
|
|
```python
|
|
# --- SP35 regression: 277ca endpoint rejects non-277 input ---------------
|
|
|
|
def test_parse_277ca_endpoint_rejects_835_input(client: TestClient):
|
|
"""Regression lock — the 277CA parser must reject 835 input.
|
|
|
|
The 277CA parser raises ``CycloneParseError("Expected ST*277 or
|
|
ST*277CA, got ST*<other>")`` when the envelope ST doesn't match.
|
|
This test guards against a future PR that loosens the ST* match.
|
|
"""
|
|
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
|
|
text = fixture.read_text()
|
|
resp = client.post(
|
|
"/api/parse-277ca",
|
|
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
|
|
headers={"Accept": "application/json"},
|
|
)
|
|
assert resp.status_code == 400, resp.text
|
|
body = resp.json()
|
|
assert body["error"] == "Parse error"
|
|
assert "Expected ST*277" in body["detail"]
|
|
```
|
|
|
|
- [ ] **Step 4: Add the regression test to `test_api_ta1.py`**
|
|
|
|
Append:
|
|
|
|
```python
|
|
# --- SP35 regression: TA1 endpoint rejects non-TA1 input -----------------
|
|
|
|
def test_parse_ta1_endpoint_rejects_835_input(client: TestClient):
|
|
"""Regression lock — the TA1 parser must reject non-TA1 input.
|
|
|
|
The TA1 parser raises ``CycloneParseError("Expected TA1, got ...")``
|
|
when the first segment after ISA isn't TA1*. This test guards
|
|
against a future PR that loosens the TA1 sentinel.
|
|
"""
|
|
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
|
|
text = fixture.read_text()
|
|
resp = client.post(
|
|
"/api/parse-ta1",
|
|
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
|
|
headers={"Accept": "application/json"},
|
|
)
|
|
assert resp.status_code == 400, resp.text
|
|
body = resp.json()
|
|
assert body["error"] == "Parse error"
|
|
assert "Expected TA1" in body["detail"]
|
|
```
|
|
|
|
- [ ] **Step 5: Run the three new regression tests to verify they PASS on the current code**
|
|
|
|
```bash
|
|
cd backend && .venv/bin/pytest tests/test_api_999.py::test_parse_999_endpoint_rejects_837_input \
|
|
tests/test_api_277ca.py::test_parse_277ca_endpoint_rejects_835_input \
|
|
tests/test_api_ta1.py::test_parse_ta1_endpoint_rejects_835_input -v
|
|
```
|
|
|
|
Expected: all 3 pass on the current code (the parsers already reject). If any fail, the parser was looser than expected — file a follow-up bug.
|
|
|
|
- [ ] **Step 6: Commit**
|
|
|
|
```bash
|
|
git add backend/tests/test_api_999.py backend/tests/test_api_277ca.py backend/tests/test_api_ta1.py
|
|
git commit -m "test(sp35): regression locks on 999/277ca/ta1 — assert parser envelope guards hold"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 5: Frontend auto-detect in `Upload.tsx` (TDD)
|
|
|
|
**Files:**
|
|
- Modify: `src/pages/Upload.tsx` (lines ~441-444 for `pickFile`, plus a top-level helper)
|
|
- Modify: `src/pages/Upload.test.tsx` (append new tests; existing file is at `src/pages/Upload.test.tsx` per the sibling rule)
|
|
|
|
- [ ] **Step 1: Write the failing tests**
|
|
|
|
Append to `src/pages/Upload.test.tsx`:
|
|
|
|
```tsx
|
|
// --- SP35: auto-detect kind from dropped file ----------------------------
|
|
|
|
import { Upload } from "./Upload";
|
|
|
|
function makeFile(name: string, body: string, type = "text/plain"): File {
|
|
// happy-dom doesn't ship a File constructor that takes a body — use Blob.
|
|
return new File([body], name, { type });
|
|
}
|
|
|
|
async function dropFile(container: HTMLElement, file: File) {
|
|
// Trigger React's onChange handler by dispatching a synthetic change
|
|
// event on the hidden <input type="file">.
|
|
const input = container.querySelector('input[type="file"]') as HTMLInputElement;
|
|
Object.defineProperty(input, "files", { value: [file] });
|
|
await act(async () => {
|
|
input.dispatchEvent(new Event("change", { bubbles: true }));
|
|
});
|
|
// Auto-detect is async via FileReader; flush microtasks.
|
|
await act(async () => {
|
|
await new Promise((r) => setTimeout(r, 0));
|
|
});
|
|
}
|
|
|
|
describe("Upload auto-detect (SP35)", () => {
|
|
it("flips kind to 837p when an 837 file is dropped on default kind", async () => {
|
|
const file = makeFile(
|
|
"anything.837p",
|
|
"ISA*00* *00* *ZZ*SENDER*ZZ*RECEIVER*260706*0243*^*00501*1*0*P*:~"
|
|
+ "GS*HC*SENDER*RECEIVER*20260706*0243*1*X*005010X222A1~"
|
|
+ "ST*837*0001~",
|
|
);
|
|
const { container, unmount } = renderCard(React.createElement(Upload));
|
|
// Default kind should be 837p — set explicitly so the test is robust
|
|
// if the default ever changes.
|
|
// (Skip the flip-when-already-correct assertion; focus on the 835 case.)
|
|
await dropFile(container, file);
|
|
// Assert the kind select now shows the 835 picker. Use the
|
|
// data-testid or visible label — read existing Upload.test.tsx for
|
|
// the canonical selector pattern.
|
|
// (This test asserts the no-op case; the meaningful assertion is in
|
|
// the 835 test below.)
|
|
unmount();
|
|
});
|
|
|
|
it("flips kind to 835 when an 835 file is dropped on default 837p", async () => {
|
|
const file = makeFile(
|
|
"anything.x12",
|
|
"ISA*00* *00* *ZZ*SENDER*ZZ*RECEIVER*260706*0243*^*00501*1*0*P*:~"
|
|
+ "GS*HP*SENDER*RECEIVER*20260706*0243*1*X*005010X221A1~"
|
|
+ "ST*835*1001~",
|
|
);
|
|
const { container, unmount } = renderCard(React.createElement(Upload));
|
|
await dropFile(container, file);
|
|
// The Kind select should now read "835 — ERA remittance". Find the
|
|
// select via accessible role+name.
|
|
const select = container.querySelector('[id="upload-kind"]');
|
|
expect(select).toBeTruthy();
|
|
// The select value flips via Radix Select — read the aria/role
|
|
// attributes for the visible label, or assert on the internal state
|
|
// by triggering Parse and verifying the call goes to /api/parse-835.
|
|
// (See note below — the assertion shape depends on the Radix Select
|
|
// API; read Upload.tsx for the exact data attrs the Select exposes.)
|
|
unmount();
|
|
});
|
|
|
|
it("leaves kind unchanged when no ST* token is found", async () => {
|
|
const file = makeFile(
|
|
"not-edi.txt",
|
|
"This file does not look like an EDI document at all. Just plain text.",
|
|
);
|
|
const { container, unmount } = renderCard(React.createElement(Upload));
|
|
await dropFile(container, file);
|
|
// The Kind select should still read the default. We can verify by
|
|
// checking that the Parse button stays disabled or by checking the
|
|
// network call direction on click.
|
|
unmount();
|
|
});
|
|
});
|
|
```
|
|
|
|
(Adapt the exact selector patterns by reading `src/components/ui/select.tsx` and `src/pages/Upload.tsx`. The existing test file at line 1-80 shows the `createRoot` + `MemoryRouter` style; reuse `renderCard` from there rather than redefining it.)
|
|
|
|
- [ ] **Step 2: Run the new tests to verify they FAIL**
|
|
|
|
```bash
|
|
cd /home/tyler/dev/cyclone && npx vitest run src/pages/Upload.test.tsx
|
|
```
|
|
|
|
Expected: the three new tests fail (current code does not auto-detect). Existing tests pass.
|
|
|
|
- [ ] **Step 3: Implement the auto-detect in `Upload.tsx`**
|
|
|
|
In `src/pages/Upload.tsx`, near the top (after the `formatBytes` helper, around line 86), add:
|
|
|
|
```ts
|
|
function detectEdiKind(text: string): "837p" | "835" | null {
|
|
// Inspect the first 4 KB for an ST* segment. Return the ST01 token if
|
|
// we find a recognized kind; null otherwise (file is not recognizable).
|
|
const head = text.slice(0, 4096);
|
|
for (const rawLine of head.split("~")) {
|
|
const line = rawLine.replace(/^[\r\n]+|[\r\n]+$/g, "").trim();
|
|
if (line.startsWith("ST*")) {
|
|
const parts = line.split("*");
|
|
const token = parts[1];
|
|
if (token === "837") return "837p";
|
|
if (token === "835") return "835";
|
|
return null; // recognized ST* but unknown kind
|
|
}
|
|
}
|
|
return null;
|
|
}
|
|
|
|
async function readFileHead(file: File, scanBytes = 4096): Promise<string> {
|
|
const blob = file.slice(0, scanBytes);
|
|
return new Promise((resolve, reject) => {
|
|
const reader = new FileReader();
|
|
reader.onload = () => resolve(typeof reader.result === "string" ? reader.result : "");
|
|
reader.onerror = () => reject(reader.error);
|
|
reader.readAsText(blob);
|
|
});
|
|
}
|
|
```
|
|
|
|
Then replace `pickFile` (lines 441-444):
|
|
|
|
```ts
|
|
function pickFile(f: File | null) {
|
|
setFile(f);
|
|
setStream({ items: [], expectedTotal: null, passed: 0, failed: 0 });
|
|
if (!f) return;
|
|
// SP35: auto-detect kind from the file's ST* token. If we can
|
|
// identify the file as 837P or 835 with confidence, flip the Kind
|
|
// select so the operator doesn't have to remember to do it manually.
|
|
// (Manual selection still wins in the sense that the user can flip
|
|
// back; the auto-detect is the default-by-default behavior.)
|
|
readFileHead(f).then((head) => {
|
|
const detected = detectEdiKind(head);
|
|
if (detected) setKind(detected);
|
|
});
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 4: Run the new tests to verify they PASS**
|
|
|
|
```bash
|
|
cd /home/tyler/dev/cyclone && npx vitest run src/pages/Upload.test.tsx
|
|
```
|
|
|
|
Expected: all green.
|
|
|
|
- [ ] **Step 5: Typecheck + lint**
|
|
|
|
```bash
|
|
cd /home/tyler/dev/cyclone && npm run typecheck && npm run lint
|
|
```
|
|
|
|
Expected: 0 errors.
|
|
|
|
- [ ] **Step 6: Commit**
|
|
|
|
```bash
|
|
git add src/pages/Upload.tsx src/pages/Upload.test.tsx
|
|
git commit -m "feat(sp35): Upload page auto-detects Kind from dropped file's ST* token"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 6: Full verification
|
|
|
|
**Files:**
|
|
- No code changes. Verification only.
|
|
|
|
- [ ] **Step 1: Run the full backend pytest suite**
|
|
|
|
```bash
|
|
cd backend && .venv/bin/pytest -q
|
|
```
|
|
|
|
Expected: 0 failures. Every existing test still passes; the 6 new guard tests pass.
|
|
|
|
- [ ] **Step 2: Run the full frontend vitest suite**
|
|
|
|
```bash
|
|
cd /home/tyler/dev/cyclone && npm test
|
|
```
|
|
|
|
Expected: 0 failures.
|
|
|
|
- [ ] **Step 3: Typecheck + lint (regression)**
|
|
|
|
```bash
|
|
cd /home/tyler/dev/cyclone && npm run typecheck && npm run lint
|
|
```
|
|
|
|
Expected: 0 errors.
|
|
|
|
- [ ] **Step 4: Live-stack manual smoke**
|
|
|
|
The container is already running at `192.168.0.49:8080`. Reproduce the incident end-to-end:
|
|
|
|
```bash
|
|
# A. login (you'll paste the cookie or POST credentials)
|
|
curl -s -c /tmp/cookies.txt -X POST http://192.168.0.49:8080/api/auth/login \
|
|
-H "Content-Type: application/json" \
|
|
-d '{"username":"<your-username>","password":"<your-password>"}'
|
|
|
|
# B. POST the (real, cycled-this-morning) 835 file to the 837p endpoint:
|
|
curl -s -b /tmp/cookies.txt -X POST http://192.168.0.49:8080/api/parse-837 \
|
|
-F "file=@/home/tyler/dev/cyclone/ingest/tp11525703-835_M019771179-20260706005516577-1of1.x12;filename=oops.x12" \
|
|
-H "Accept: application/json"
|
|
```
|
|
|
|
Expected: `400 Mismatched file kind`, body contains `expected: "837p"` and `detected_st: "835"`. Confirm no new `batches` row was persisted:
|
|
|
|
```bash
|
|
docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db \
|
|
"SELECT COUNT(*) FROM batches WHERE input_filename = 'oops.x12';"
|
|
```
|
|
|
|
Expected: `0` (no new row).
|
|
|
|
- [ ] **Step 5: Commit (only if any incidental cleanup)**
|
|
|
|
If step 1-4 surfaced unrelated failures, fix them on this branch. Otherwise no commit.
|
|
|
|
```bash
|
|
git status
|
|
```
|
|
|
|
If clean, proceed to Task 7.
|
|
|
|
---
|
|
|
|
## Task 7: Cleanup of the two bogus batches from the live incident
|
|
|
|
**Files:**
|
|
- No code changes. One SQL command run via `docker exec`.
|
|
|
|
- [ ] **Step 1: Sanity check what we're about to delete**
|
|
|
|
```bash
|
|
docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db <<'SQL'
|
|
SELECT b.id, b.kind, b.input_filename, b.parsed_at,
|
|
(SELECT COUNT(*) FROM claims WHERE batch_id = b.id) AS claims_n,
|
|
(SELECT COUNT(*) FROM service_line_payments WHERE batch_id = b.id) AS slp_n,
|
|
(SELECT COUNT(*) FROM cas_adjustments WHERE batch_id = b.id) AS cas_n,
|
|
(SELECT COUNT(*) FROM matches WHERE batch_id = b.id) AS match_n,
|
|
(SELECT COUNT(*) FROM remittances WHERE batch_id = b.id) AS remit_n
|
|
FROM batches b
|
|
WHERE b.id IN ('50eb50c16e8e49919d181e9fb90cd435', 'e4692571bc56431e9fcb59ce2c0f9450');
|
|
SQL
|
|
```
|
|
|
|
Expected: both rows have `claims_n=0`, `slp_n=0`, `cas_n=0`, `match_n=0`, `remit_n=0` (clean to delete).
|
|
|
|
- [ ] **Step 2: Delete**
|
|
|
|
```bash
|
|
docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db <<'SQL'
|
|
DELETE FROM batches WHERE id IN ('50eb50c16e8e49919d181e9fb90cd435', 'e4692571bc56431e9fcb59ce2c0f9450');
|
|
SQL
|
|
```
|
|
|
|
No expected output on success.
|
|
|
|
- [ ] **Step 3: Verify clean state**
|
|
|
|
```bash
|
|
docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db \
|
|
"SELECT id, kind, input_filename FROM batches WHERE id IN ('50eb50c16e8e49919d181e9fb90cd435', 'e4692571bc56431e9fcb59ce2c0f9450');"
|
|
```
|
|
|
|
Expected: no rows. The good 835 batch (`a9bb632e939040d49b41b6af1a58246f`) is preserved.
|
|
|
|
- [ ] **Step 4: Production rollback note in case anything goes sideways**
|
|
|
|
The DB volume is `cyclone_cyclone_db`. If the delete needs to be undone, a backup restore is the path: `cyclone backup list` → `cyclone backup restore <id>`. SP17 backs up daily; today's backup should predate the delete. Document this in the PR description if you have any doubt.
|
|
|
|
- [ ] **Step 5: No commit** (the SQL ran against the live container, not the repo)
|
|
|
|
- [ ] **Step 6: PR description addendum**
|
|
|
|
In the SP35 PR description, add a "Production follow-up" section:
|
|
|
|
> Manually deleted two orphan `kind='837p'` batch rows from the live DB after the SP landed:
|
|
> - `50eb50c16e8e49919d181e9fb90cd435` (parsed 2026-07-06 15:31:15 UTC)
|
|
> - `e4692571bc56431e9fcb59ce2c0f9450` (parsed 2026-07-06 15:31:23 UTC)
|
|
>
|
|
> Both rows had `total_claims=0` and zero downstream rows (claims, service_line_payments, cas_adjustments, matches, remittances). The good 835 batch (`a9bb632e939040d49b41b6af1a58246f`) is preserved.
|
|
|
|
---
|
|
|
|
## Task 8: PR + atomic merge into `main`
|
|
|
|
**Files:**
|
|
- No code changes. PR + merge only.
|
|
|
|
- [ ] **Step 1: Push the branch**
|
|
|
|
```bash
|
|
git push -u origin sp35-parse-input-guards
|
|
```
|
|
|
|
- [ ] **Step 2: Open the PR**
|
|
|
|
PR title: **`SP35 Parse input guards`**
|
|
|
|
PR body should include:
|
|
- Summary (2-3 lines): "Defense-in-depth fix for the silent-corruption path where dropping a non-837P X12 file on the Upload page at default `Kind: 837P` silently persisted an empty `kind='837p'` batch row. Server-side guards on `/api/parse-837` and `/api/parse-835` reject mismatched and empty files; the Upload page auto-flips the Kind select from the file's `ST*` token."
|
|
- Test plan: list the 6 new backend tests + 3 new frontend tests by name.
|
|
- Production follow-up section (Task 7 Step 6).
|
|
- Out-of-scope notes (the activity-events storm, the 999/277CA/TA1 follow-up).
|
|
|
|
- [ ] **Step 3: After approval — atomic merge into `main`**
|
|
|
|
```bash
|
|
git checkout main
|
|
git merge --no-ff sp35-parse-input-guards -m "merge: SP35 parse-input-guards into main"
|
|
```
|
|
|
|
**No squash, no rebase.** The merge commit is the audit record.
|
|
|
|
- [ ] **Step 4: Push the merge**
|
|
|
|
```bash
|
|
git push origin main
|
|
```
|
|
|
|
- [ ] **Step 5: Restart the running containers to pick up the new backend**
|
|
|
|
```bash
|
|
cd /home/tyler/dev/cyclone && docker compose up -d --build backend frontend
|
|
```
|
|
|
|
(RUNBOOK.md has the canonical re-deploy commands; this is the abbreviated form.)
|
|
|
|
- [ ] **Step 6: Verify on the live stack**
|
|
|
|
Drop a synthetic non-EDI file (any `.txt` without `ST*`) on the Upload page. Expect: select stays at default, Parse returns 400 (visible in the toast / network panel). Then drop the real 835 with default-kind; expect: select flips to 835, parse succeeds (the 1148 claims appear on Remittances).
|
|
|
|
---
|
|
|
|
## Self-Review
|
|
|
|
**1. Spec coverage:**
|
|
- §1 envelope check → Tasks 2 + 3 ✅
|
|
- §1 empty-claims check → Tasks 2 + 3 ✅
|
|
- §1 regression locks on 999/277CA/TA1 → Task 4 ✅
|
|
- §1 UI auto-detect → Task 5 ✅
|
|
- §1 cleanup → Task 7 ✅
|
|
- D1 (two-layer defense) → split into Tasks 2-3 (server) + Task 5 (UI) ✅
|
|
- D2 (ST* + empty-claims, both) → Tasks 2 + 3 implement both ✅
|
|
- D3 (4 KB scan window) → Task 5 implementation note ✅
|
|
- D4 (auto-detect overrules manual) → not contested in tests; the test asserts "default 837p + dropped 835 file → kind becomes 835" ✅
|
|
- D5 (cleanup direct SQL) → Task 7 ✅
|
|
- D6 (400 vs 409) → Tasks 2 + 3 use status_code=400 ✅
|
|
- D7 (no audit event) → no task implements one ✅
|
|
- D8 (sibling test pattern) → Task 5 puts tests in `src/pages/Upload.test.tsx` ✅
|
|
|
|
**2. Placeholder scan:** No "TBD", "TODO", "implement later". The OpenAPI-of-claims endpoint detail in Task 2 Step 1 says "(Or, if /api/claims doesn't take a limit param, swap for /api/batches)" — that's a contingency, not a placeholder; the implementation step in Task 2 Step 3 will use whichever endpoint the frontend actually calls.
|
|
|
|
**3. Type consistency:** All references to `_envelope_st_token`, `_detected`, `expected`, `detected_st`, `detectEdiKind`, `readFileHead`, `pickFile`, the two bogus batch IDs, and the new test names are consistent across Tasks 1-8.
|