diff --git a/docs/superpowers/plans/2026-06-21-cyclone-claims-unique-constraint-and-409-ux.md b/docs/superpowers/plans/2026-06-21-cyclone-claims-unique-constraint-and-409-ux.md new file mode 100644 index 0000000..dbe7b6e --- /dev/null +++ b/docs/superpowers/plans/2026-06-21-cyclone-claims-unique-constraint-and-409-ux.md @@ -0,0 +1,926 @@ +# Drop `UNIQUE(batch_id, patient_control_number)` + 409 UX 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:** Allow multi-claim 837P files (where many `CLM*` segments share a `member_id`) to ingest without 409, and surface a structured error panel on the upload page when a true duplicate claim still trips a 409. + +**Architecture:** Backend migration drops the inline `UNIQUE(batch_id, patient_control_number)` on `claims` via SQLite table recreation. Two new store helpers (`find_existing_batch_for_claim`, `find_existing_batch_for_remit`) enable both 837 and 835 409 handlers to surface the id of the prior batch. Frontend `ApiError` carries `existingBatchId`; `Upload.tsx` renders an inline error panel above the streaming results with a link to the existing batch and a "Pick a different file" escape hatch. + +**Tech Stack:** Python 3.11+, SQLAlchemy 2.x, FastAPI, SQLite (sqlcipher3 optional), React 18 + TanStack Query + Radix UI + sonner, Vitest + React Testing Library, pytest. + +**Spec:** `docs/superpowers/specs/2026-06-21-cyclone-claims-unique-constraint-and-409-ux-design.md` — read fully before starting. + +**Worktree setup (one-time):** + +```bash +cd /Users/openclaw/dev/cyclone +git worktree add .worktrees/claims-unique-fix -b claims-unique-fix main +cd .worktrees/claims-unique-fix +# Install backend deps if needed (venv assumed active) +pip install -e backend +# Install frontend deps if needed +npm install +``` + +All commits happen in this worktree. Merge to main via fast-forward when each phase ends. + +--- + +## Phase 1 — Backend migration + helpers + API + +### Task 1.1: Migration 0013 drops inline UNIQUE via table recreation + +**Files:** +- Create: `backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql` +- Modify: `backend/tests/test_db_migrate.py` (append test) + +The runner (`backend/src/cyclone/db_migrate.py`) wraps each `.sql` in an implicit transaction via `engine.begin()`, so the migration MUST NOT use `BEGIN`/`COMMIT` or `PRAGMA foreign_keys=OFF` (no-op inside a transaction). Use `PRAGMA defer_foreign_keys = ON` instead — checks fire at commit against the renamed table. + +- [ ] **Step 1: Write the failing test** + +Add to `backend/tests/test_db_migrate.py`: + +```python +def test_drop_claims_unique_constraint_migration(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Migration 0013 must drop the inline UNIQUE(batch_id, patient_control_number) + on claims by recreating the table. Idempotent and preserves data.""" + monkeypatch.setattr(db_migrate, "MIGRATIONS_DIR", tmp_path) + # Copy the real migrations so the test starts from v1. + real_dir = Path(db_migrate.__file__).parent / "migrations" + for src in sorted(real_dir.glob("00*.sql")): + (tmp_path / src.name).write_text(src.read_text()) + + engine = _fresh_engine(tmp_path) + db_migrate.run(engine) + # Before 0013: insert two claims with same (batch_id, patient_control_number) raises. + with engine.begin() as c: + c.exec_driver_sql("INSERT INTO batches(id, kind, input_filename, parsed_at) VALUES ('b1', '837p', 'x.txt', '2026-01-01')") + c.exec_driver_sql("INSERT INTO claims(id, batch_id, patient_control_number) VALUES ('c1', 'b1', 'M')") + with pytest.raises(sqlalchemy.exc.IntegrityError): + c.exec_driver_sql("INSERT INTO claims(id, batch_id, patient_control_number) VALUES ('c2', 'b1', 'M')") + # Now drop the migration in by name only: + (tmp_path / "0013_drop_claims_unique_constraint.sql").write_text( + "-- version: 13\n" + "PRAGMA defer_foreign_keys = ON;\n" + "CREATE TABLE claims_new (" + "id TEXT PRIMARY KEY, batch_id TEXT NOT NULL REFERENCES batches(id) ON DELETE CASCADE," + "patient_control_number TEXT NOT NULL, service_date_from DATE, service_date_to DATE," + "charge_amount NUMERIC(12, 2) NOT NULL DEFAULT 0, provider_npi TEXT, payer_id TEXT," + "state TEXT NOT NULL DEFAULT 'submitted', state_before_reversal TEXT," + "matched_remittance_id TEXT REFERENCES remittances(id), raw_json TEXT," + "rejection_reason TEXT, rejected_at TIMESTAMP, resubmit_count INTEGER NOT NULL DEFAULT 0," + "state_changed_at TIMESTAMP, payer_rejected_at TEXT, payer_rejected_reason TEXT," + "payer_rejected_status_code TEXT, payer_rejected_by_277ca_id TEXT," + "payer_rejected_acknowledged_at TEXT, payer_rejected_acknowledged_actor TEXT);\n" + "INSERT INTO claims_new SELECT * FROM claims;" + "DROP TABLE claims;" + "ALTER TABLE claims_new RENAME TO claims;" + ) + db_migrate.run(engine) + # After 0013: same insert succeeds. + with engine.begin() as c: + c.exec_driver_sql("INSERT INTO claims(id, batch_id, patient_control_number) VALUES ('c2', 'b1', 'M')") + rows = c.exec_driver_sql("SELECT COUNT(*) FROM claims").scalar() + assert rows == 2 + assert _user_version(engine) == 13 + + +def test_migration_0013_is_idempotent(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Re-running db_migrate.run on a v13 DB is a no-op.""" + monkeypatch.setattr(db_migrate, "MIGRATIONS_DIR", tmp_path) + real_dir = Path(db_migrate.__file__).parent / "migrations" + for src in sorted(real_dir.glob("00*.sql")): + (tmp_path / src.name).write_text(src.read_text()) + + engine = _fresh_engine(tmp_path) + db_migrate.run(engine) # applies all + version_after_first = _user_version(engine) + db_migrate.run(engine) # second call: no-op + assert _user_version(engine) == version_after_first +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd backend && python -m pytest tests/test_db_migrate.py::test_drop_claims_unique_constraint_migration tests/test_db_migrate.py::test_migration_0013_is_idempotent -v` + +Expected: `test_drop_claims_unique_constraint_migration` FAILS because 0013 doesn't exist yet; `test_migration_0013_is_idempotent` PASSES (idempotency already works for any v). + +- [ ] **Step 3: Write migration 0013** + +Create `backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql`: + +```sql +-- version: 13 +-- Drop the inline UNIQUE(batch_id, patient_control_number) on claims. +-- Migration 0003 attempted DROP INDEX IF EXISTS uq_claims_batch_pcn but +-- the constraint is inline in CREATE TABLE, so the drop was a no-op. +-- The only way to remove an inline UNIQUE in SQLite is table recreation. +-- +-- X12 837P allows any number of CLM segments per 2000B subscriber loop; +-- claim identity is provided by the primary key (claims.id = CLM01). +-- The remittances table had a parallel constraint already removed in 0003 +-- (because that one WAS a named index), so this migration only touches +-- claims. +-- +-- The migration runner (db_migrate.py) wraps each .sql in an implicit +-- transaction via engine.begin(), so we MUST NOT use BEGIN/COMMIT. +-- PRAGMA defer_foreign_keys defers FK checks to commit, which is the +-- only way to drop a referenced table inside a transaction in SQLite. + +PRAGMA defer_foreign_keys = ON; + +CREATE TABLE claims_new ( + id TEXT PRIMARY KEY, + batch_id TEXT NOT NULL REFERENCES batches(id) ON DELETE CASCADE, + patient_control_number TEXT NOT NULL, + service_date_from DATE, + service_date_to DATE, + charge_amount NUMERIC(12, 2) NOT NULL DEFAULT 0, + provider_npi TEXT, + payer_id TEXT, + state TEXT NOT NULL DEFAULT 'submitted', + state_before_reversal TEXT, + matched_remittance_id TEXT REFERENCES remittances(id), + raw_json TEXT, + rejection_reason TEXT, + rejected_at TIMESTAMP, + resubmit_count INTEGER NOT NULL DEFAULT 0, + state_changed_at TIMESTAMP, + payer_rejected_at TEXT, + payer_rejected_reason TEXT, + payer_rejected_status_code TEXT, + payer_rejected_by_277ca_id TEXT, + payer_rejected_acknowledged_at TEXT, + payer_rejected_acknowledged_actor TEXT + -- NO UNIQUE (batch_id, patient_control_number) — removed. +); + +INSERT INTO claims_new SELECT * FROM claims; +DROP TABLE claims; +ALTER TABLE claims_new RENAME TO claims; + +-- Recreate secondary indexes (same names, same columns as initial schema +-- plus later migrations). +CREATE INDEX ix_claims_state ON claims(state); +CREATE INDEX ix_claims_patient_control_number ON claims(patient_control_number); +CREATE INDEX ix_claims_service_date_from ON claims(service_date_from); +CREATE INDEX ix_claims_state_changed_at ON claims(state, state_changed_at); +CREATE INDEX idx_claims_payer_rejected_at ON claims(payer_rejected_at); +CREATE INDEX idx_claims_payer_rejected_unack + ON claims(payer_rejected_at) + WHERE payer_rejected_acknowledged_at IS NULL; +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd backend && python -m pytest tests/test_db_migrate.py::test_drop_claims_unique_constraint_migration tests/test_db_migrate.py::test_migration_0013_is_idempotent -v` + +Expected: PASS for both. The first test asserts the inline UNIQUE is gone (insert succeeds) and `user_version==13`. The second asserts idempotency. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql backend/tests/test_db_migrate.py +git commit -m "feat(db): drop inline UNIQUE(batch_id, patient_control_number) via migration 0013" +``` + +--- + +### Task 1.2: Store helpers `find_existing_batch_for_claim` and `find_existing_batch_for_remit` + +**Files:** +- Modify: `backend/src/cyclone/store.py:1-50` (imports) and add new functions near the top +- Modify: `backend/tests/test_store.py` (append tests) + +The helpers are pure reads. They return the batch_id of the first batch containing the given claim_id / remit_id, or None. + +- [ ] **Step 1: Write the failing test** + +Add to `backend/tests/test_store.py`: + +```python +def test_find_existing_batch_for_claim_returns_none_for_unknown(global_store): + """Unknown claim_id -> None.""" + assert global_store.find_existing_batch_for_claim("nope") is None # type: ignore[attr-defined] + + +def test_find_existing_batch_for_claim_returns_batch_id(global_store): + """Known claim_id -> batch_id of the holding batch.""" + from cyclone.store import BatchRecord, _claim_837_row # noqa: F401 + from cyclone.db import Batch, Claim, db as _db_mod + from datetime import datetime, timezone + from cyclone.parser.parsers_837p import ClaimOutput # noqa: F401 + # Simpler: insert a batch + claim directly via the DB. + with _db_mod.SessionLocal()() as s: + s.add(Batch(id="B1", kind="837p", input_filename="x.txt", + parsed_at=datetime(2026,1,1,tzinfo=timezone.utc))) + s.add(Claim(id="CLM-A", batch_id="B1", patient_control_number="M1")) + s.commit() + assert global_store.find_existing_batch_for_claim("CLM-A") == "B1" # type: ignore[attr-defined] + + +def test_find_existing_batch_for_remit_returns_none_for_unknown(global_store): + """Unknown remit id -> None.""" + assert global_store.find_existing_batch_for_remit("nope") is None # type: ignore[attr-defined] + + +def test_find_existing_batch_for_remit_returns_batch_id(global_store): + """Known remit id -> batch_id of the holding batch.""" + from cyclone.db import Batch, Remittance, db as _db_mod + from datetime import datetime, timezone + with _db_mod.SessionLocal()() as s: + s.add(Batch(id="B2", kind="835", input_filename="y.txt", + parsed_at=datetime(2026,1,1,tzinfo=timezone.utc))) + s.add(Remittance(id="CLP-A", batch_id="B2", + payer_claim_control_number="CLP-A", + status_code="1", received_at=datetime(2026,1,1,tzinfo=timezone.utc))) + s.commit() + assert global_store.find_existing_batch_for_remit("CLP-A") == "B2" # type: ignore[attr-defined] +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd backend && python -m pytest tests/test_store.py::test_find_existing_batch_for_claim_returns_none_for_unknown -v` + +Expected: FAIL with `AttributeError: 'CycloneStore' object has no attribute 'find_existing_batch_for_claim'`. + +- [ ] **Step 3: Implement helpers** + +Add to `backend/src/cyclone/store.py` near the top, after imports: + +```python +def find_existing_batch_for_claim(claim_id: str) -> str | None: + """Return the batch_id of the first batch containing this claim id, or None. + + Pure read; opens a short-lived session. Used by the 837 409 handler to + surface which prior batch already holds the same CLM01. + """ + from sqlalchemy import select + from cyclone import db + from cyclone.db import Claim + with db.SessionLocal()() as s: + row = s.execute( + select(Claim.batch_id).where(Claim.id == claim_id).limit(1) + ).first() + return row[0] if row else None + + +def find_existing_batch_for_remit(remit_id: str) -> str | None: + """Return the batch_id of the first batch containing this remit id, or None. + + Pure read; opens a short-lived session. Used by the 835 409 handler. + `remit_id` is the PK on `remittances.id` (= payer_claim_control_number = CLP01). + """ + from sqlalchemy import select + from cyclone import db + from cyclone.db import Remittance + with db.SessionLocal()() as s: + row = s.execute( + select(Remittance.batch_id).where(Remittance.id == remit_id).limit(1) + ).first() + return row[0] if row else None +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd backend && python -m pytest tests/test_store.py -k "find_existing_batch" -v` + +Expected: PASS for all 4 tests. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/cyclone/store.py backend/tests/test_store.py +git commit -m "feat(store): add find_existing_batch_for_claim and find_existing_batch_for_remit" +``` + +--- + +### Task 1.3: 409 handlers in api.py surface `existing_batch_id` + +**Files:** +- Modify: `backend/src/cyclone/api.py:394-415` (837 409 handler) +- Modify: `backend/src/cyclone/api.py:588-602` (835 409 handler) +- Modify: `backend/tests/test_api_parse_persists.py` (append tests) + +After migration 0013, IntegrityError on the 837 path can only fire when two claims in the same file share the same CLM01 (rare). The helper may still return a batch_id if the colliding CLM01 was previously ingested and not deleted (the dedup `s.get(Claim, claim_id)` only queries DB state, not pending session state, so cross-batch duplicates still get inserted and trip the PK). + +- [ ] **Step 1: Write the failing tests** + +Add to `backend/tests/test_api_parse_persists.py`: + +```python +def test_409_response_includes_existing_batch_id_for_837(client: TestClient): + """When a CLM01 already exists in a prior batch, the 409 body has existing_batch_id.""" + from cyclone.db import Batch, Claim + from datetime import datetime, timezone + # Seed a prior batch with claim CLM-X. + with global_store._lock: + pass + from cyclone import db as _db + with _db.SessionLocal()() as s: + s.add(Batch(id="PRIOR", kind="837p", input_filename="prior.txt", + parsed_at=datetime(2026,1,1,tzinfo=timezone.utc), + raw_result_json={})) + s.add(Claim(id="CLM-X", batch_id="PRIOR", patient_control_number="M")) + s.commit() + # Build a file with two CLM* segments both using CLM-X (forces PK collision). + text = ( + "ISA*00* *00* *ZZ*SUBMITTERID *ZZ*RECEIVERID " + "*240101*1200*^*00501*000000001*0*P*:~\n" + "GS*HC*SUBMITTERID*RECEIVERID*20240101*1200*1*X*005010X222A1~\n" + "ST*837*0001*005010X222A1~\n" + "BHT*0019*00*1*20240101*1200*CH~\n" + "NM1*41*2*SUBMITTER*****46*SUBMITTERID~\n" + "PER*IC*CONTACT*TE*5555555555~\n" + "NM1*40*2*RECEIVER*****46*RECEIVERID~\n" + "HL*1**20*1~\n" + "NM1*85*2*BILLING*****XX*1881068062~\n" + "N3*123 MAIN*\nN4*DENVER*CO*80202~\n" + "REF*EI*123456789~\n" + "HL*2*1*22*0~\n" + "SBR*P*18*******CI~\n" + "NM1*IL*1*DOE*JOHN****MI*M~\n" + "N3*456 ELM*\nN4*DENVER*CO*80202~\n" + "DMG*D8*19700101*M~\n" + "NM1*PR*2*MEDICAID*****PI*MCD~\n" + "CLM*CLM-X*100***11:B:1*Y*A*Y*Y~\n" + "LX*1~\nSV1*HC:99213*100*UN*1***1~\n" + "DTP*472*D8*20240101~\n" + "CLM*CLM-X*100***11:B:1*Y*A*Y*Y~\n" + "LX*2~\nSV1*HC:99213*100*UN*1***1~\n" + "DTP*472*D8*20240101~\n" + "SE*30*0001~\n" + "GE*1*1~\n" + "IEA*1*000000001~\n" + ) + resp = client.post( + "/api/parse-837", + files={"file": ("dup.txt", text, "text/plain")}, + headers={"Accept": "application/json"}, + ) + assert resp.status_code == 409, resp.text + body = resp.json() + assert body.get("existing_batch_id") == "PRIOR" + + +def test_409_response_includes_existing_batch_id_for_835(client: TestClient): + """When a CLP01 already exists in a prior batch, the 835 409 body has existing_batch_id.""" + from cyclone.db import Batch, Remittance + from datetime import datetime, timezone + from cyclone import db as _db + with _db.SessionLocal()() as s: + s.add(Batch(id="PRIOR835", kind="835", input_filename="prior.txt", + parsed_at=datetime(2026,1,1,tzinfo=timezone.utc), + raw_result_json={})) + s.add(Remittance(id="CLP-X", batch_id="PRIOR835", + payer_claim_control_number="CLP-X", + status_code="1", + received_at=datetime(2026,1,1,tzinfo=timezone.utc))) + s.commit() + # Build a minimal 835 with two CLP segments using CLP-X. This is contrived; + # we just need to trigger IntegrityError. The exact parser robustness to this + # fixture is not the focus — the test asserts the 409 body shape. + # ... or instead: directly invoke the handler via a unit-style test that + # pre-seeds and then makes a minimal 835 that includes CLP-X twice. + # For brevity, drop this test if constructing a fixture is too brittle; + # the 837 test above already exercises the same handler pattern. + pytest.skip("835 fixture with duplicate CLP01 is fragile; 837 case covers the pattern") +``` + +Note: the 835 test is intentionally skipped — constructing a minimal valid 835 with duplicate CLP01 is brittle. The 837 case exercises the handler pattern (call `find_existing_batch_for_remit` from a similarly-shaped except block in the 835 handler). If you need 835 coverage, manually inspect by running the API on a real fixture after the implementation lands. + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd backend && python -m pytest tests/test_api_parse_persists.py::test_409_response_includes_existing_batch_id_for_837 -v` + +Expected: FAIL because the 409 handler does not yet add `existing_batch_id`. + +- [ ] **Step 3: Modify 837 409 handler** + +Edit `backend/src/cyclone/api.py` lines 394-415: + +Replace the `except IntegrityError` block in `parse_837_endpoint` with: + +```python + try: + store.add(rec, event_bus=request.app.state.event_bus) + except IntegrityError as exc: + # After migration 0013, the only way an IntegrityError fires here + # is a PK collision on claims.id (CLM01) — either within-file or + # against a prior batch. Look up the first claim's id and ask the + # helper which batch already holds it. + first_claim_id = ( + result.claims[0].claim_id if result.claims else None + ) + existing_batch_id = ( + store.find_existing_batch_for_claim(first_claim_id) + if first_claim_id else None + ) + body = { + "error": "Duplicate claim", + "detail": ( + "This file (or one previously ingested with the same " + "claim control number) collides with an existing record. " + "Inspect the file for duplicate CLM01 control numbers, or " + "remove the existing batch before retrying." + ), + "batch_id": rec.id, + } + if existing_batch_id and existing_batch_id != rec.id: + body["existing_batch_id"] = existing_batch_id + log.warning("Duplicate claim while persisting batch %s: %s", rec.id, exc) + return JSONResponse(status_code=409, content=body) +``` + +- [ ] **Step 4: Modify 835 409 handler** + +Edit `backend/src/cyclone/api.py` lines 588-602: + +Replace the `except IntegrityError` block in `parse_835_endpoint` with: + +```python + try: + store.add(rec, event_bus=request.app.state.event_bus) + except IntegrityError as exc: + first_pcn = ( + result.claims[0].payer_claim_control_number + if result.claims else None + ) + existing_batch_id = ( + store.find_existing_batch_for_remit(first_pcn) + if first_pcn else None + ) + body = { + "error": "Duplicate remittance", + "detail": ( + "This 835 file (or one previously ingested with the same " + "payer claim control number) collides with an existing record. " + "Remove the existing remittance before retrying." + ), + "batch_id": rec.id, + } + if existing_batch_id and existing_batch_id != rec.id: + body["existing_batch_id"] = existing_batch_id + log.warning("Duplicate remittance while persisting batch %s: %s", rec.id, exc) + return JSONResponse(status_code=409, content=body) +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `cd backend && python -m pytest tests/test_api_parse_persists.py -v` + +Expected: All PASS, including the new 409 test. + +- [ ] **Step 6: Commit** + +```bash +git add backend/src/cyclone/api.py backend/tests/test_api_parse_persists.py +git commit -m "feat(api): 409 responses for 837/435 include existing_batch_id" +``` + +--- + +## Phase 2 — Frontend + +### Task 2.1: `ApiError.existingBatchId` + parse837/835 surface it + +**Files:** +- Modify: `src/lib/api.ts:164-167` (ApiError class) +- Modify: `src/lib/api.ts:174-191` (readErrorBody) +- Modify: `src/lib/api.ts:280-339` (parse837, parse835) +- Create: `src/lib/api.test.ts` + +- [ ] **Step 1: Write the failing test** + +Create `src/lib/api.test.ts`: + +```typescript +import { describe, it, expect, vi, afterEach } from "vitest"; +import { ApiError, parse837 } from "@/lib/api"; + +afterEach(() => vi.restoreAllMocks()); + +describe("ApiError", () => { + it("carries existingBatchId when constructed", () => { + const e = new ApiError(409, "dup", "BATCH-1"); + expect(e.status).toBe(409); + expect(e.existingBatchId).toBe("BATCH-1"); + }); + it("defaults existingBatchId to null", () => { + const e = new ApiError(500, "boom"); + expect(e.existingBatchId).toBeNull(); + }); +}); + +describe("parse837 throws ApiError with existingBatchId", () => { + it("extracts existing_batch_id from 409 body", async () => { + vi.stubGlobal("import.meta.env", { VITE_API_BASE_URL: "http://x" }); + const res = new Response( + JSON.stringify({ + error: "Duplicate claim", + detail: "collision", + batch_id: "NEW", + existing_batch_id: "PRIOR", + }), + { status: 409, headers: { "Content-Type": "application/json" } }, + ); + vi.stubGlobal( + "fetch", + vi.fn(async () => res), + ); + const file = new File(["x"], "f.txt", { type: "text/plain" }); + await expect(parse837(file, { onProgress: () => {} })).rejects.toMatchObject({ + status: 409, + existingBatchId: "PRIOR", + }); + }); + + it("sets existingBatchId null when body omits it", async () => { + vi.stubGlobal("import.meta.env", { VITE_API_BASE_URL: "http://x" }); + const res = new Response( + JSON.stringify({ error: "X", detail: "y", batch_id: "N" }), + { status: 409, headers: { "Content-Type": "application/json" } }, + ); + vi.stubGlobal( + "fetch", + vi.fn(async () => res), + ); + const file = new File(["x"], "f.txt", { type: "text/plain" }); + await expect(parse837(file)).rejects.toMatchObject({ + status: 409, + existingBatchId: null, + }); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd /Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix && npm test -- src/lib/api.test.ts` + +Expected: FAIL — `existingBatchId` not a constructor argument yet; `parse837` throws `Error` not `ApiError`. + +- [ ] **Step 3: Update `ApiError`** + +Edit `src/lib/api.ts` line 164-167: + +```typescript +export class ApiError extends Error { + constructor( + public status: number, + message: string, + public existingBatchId: string | null = null, + ) { + super(message); + } +} +``` + +- [ ] **Step 4: Update `readErrorBody` to return parsed body** + +Edit `src/lib/api.ts` line 174-191. Change return type and add JSON parse branch: + +```typescript +type ErrorBody = { + detail?: unknown; + error?: unknown; + existing_batch_id?: unknown; +}; + +async function readErrorBody( + res: Response, +): Promise<{ message: string; existingBatchId: string | null }> { + try { + const t = await res.text(); + if (!t) return { message: "", existingBatchId: null }; + try { + const obj = JSON.parse(t) as ErrorBody; + let message = ""; + if (typeof obj.detail === "string") message = obj.detail; + else if (typeof obj.error === "string") message = obj.error; + else message = t; + const existing = + typeof obj.existing_batch_id === "string" + ? obj.existing_batch_id + : null; + return { message, existingBatchId: existing }; + } catch { + return { message: t, existingBatchId: null }; + } + } catch { + return { message: "", existingBatchId: null }; + } +} +``` + +- [ ] **Step 5: Update parse837 to throw ApiError** + +Edit `src/lib/api.ts` lines 293-298: + +```typescript + if (!res.ok) { + const { message, existingBatchId } = await readErrorBody(res); + throw new ApiError( + res.status, + `${res.status} ${res.statusText}${message ? ` — ${message}` : ""}`, + existingBatchId, + ); + } +``` + +- [ ] **Step 6: Update parse835 to throw ApiError** + +Edit `src/lib/api.ts` lines 329-334: + +```typescript + if (!res.ok) { + const { message, existingBatchId } = await readErrorBody(res); + throw new ApiError( + res.status, + `${res.status} ${res.statusText}${message ? ` — ${message}` : ""}`, + existingBatchId, + ); + } +``` + +- [ ] **Step 7: Run tests to verify they pass** + +Run: `cd .worktrees/claims-unique-fix && npm test -- src/lib/api.test.ts` + +Expected: PASS for all 4 tests. + +- [ ] **Step 8: Commit** + +```bash +git add src/lib/api.ts src/lib/api.test.ts +git commit -m "feat(api): ApiError carries existingBatchId; parse837/parse835 surface it" +``` + +--- + +### Task 2.2: `Upload.tsx` inline error panel for 409 + +**Files:** +- Modify: `src/pages/Upload.tsx` (add error state + panel JSX + 409 catch branch) +- Create: `src/pages/Upload.test.tsx` + +The panel renders above the streaming results when `uploadError.kind === "duplicate"`. It shows: +- A 409 badge +- "Duplicate claim — file not ingested" title +- Detail mentioning the file and (if `existingBatchId` is set) "Open the existing batch to compare, or pick a different file." +- A button linking to `/batches/{existingBatchId}` if present +- A "Pick a different file" ghost button that calls `pickFile(null)` and clears the error state + +- [ ] **Step 1: Write the failing tests** + +Create `src/pages/Upload.test.tsx`: + +```tsx +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { MemoryRouter, Route, Routes } from "react-router-dom"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import * as apiModule from "@/lib/api"; +import { ApiError } from "@/lib/api"; +import { Upload } from "@/pages/Upload"; + +// We mock the api module so the component doesn't actually call fetch. +vi.mock("@/lib/api", async () => { + const actual = await vi.importActual("@/lib/api"); + return { ...actual, parse837: vi.fn(), parse835: vi.fn() }; +}); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +function renderUpload() { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + + + } /> + } /> + + + , + ); +} + +describe("Upload error panel", () => { + it("renders panel with link when 409 carries existingBatchId", async () => { + vi.mocked(apiModule.parse837).mockRejectedValueOnce( + new ApiError(409, "409 Conflict — dup", "PRIOR-BATCH"), + ); + renderUpload(); + // Find the file input and upload a file. + const input = document.querySelector('input[type="file"]') as HTMLInputElement; + expect(input).toBeTruthy(); + const file = new File(["x"], "test.txt", { type: "text/plain" }); + fireEvent.change(input, { target: { files: [file] } }); + // Wait for the panel to render. + const link = await screen.findByRole("button", { name: /open existing batch/i }); + expect(link).toBeInTheDocument(); + expect(screen.getByText(/duplicate claim/i)).toBeInTheDocument(); + }); + + it("renders panel without link when 409 omits existingBatchId", async () => { + vi.mocked(apiModule.parse837).mockRejectedValueOnce( + new ApiError(409, "409 Conflict — dup", null), + ); + renderUpload(); + const input = document.querySelector('input[type="file"]') as HTMLInputElement; + const file = new File(["x"], "test.txt", { type: "text/plain" }); + fireEvent.change(input, { target: { files: [file] } }); + await screen.findByText(/duplicate claim/i); + expect( + screen.queryByRole("button", { name: /open existing batch/i }), + ).toBeNull(); + }); + + it("does NOT render panel for non-409 errors", async () => { + vi.mocked(apiModule.parse837).mockRejectedValueOnce( + new ApiError(400, "bad file"), + ); + renderUpload(); + const input = document.querySelector('input[type="file"]') as HTMLInputElement; + const file = new File(["x"], "test.txt", { type: "text/plain" }); + fireEvent.change(input, { target: { files: [file] } }); + // Give the async error handler a tick. + await new Promise((r) => setTimeout(r, 50)); + expect(screen.queryByText(/duplicate claim/i)).toBeNull(); + }); + + it("'Pick a different file' button clears error", async () => { + vi.mocked(apiModule.parse837).mockRejectedValueOnce( + new ApiError(409, "409 Conflict — dup", "PRIOR"), + ); + renderUpload(); + const input = document.querySelector('input[type="file"]') as HTMLInputElement; + const file = new File(["x"], "test.txt", { type: "text/plain" }); + fireEvent.change(input, { target: { files: [file] } }); + const clearBtn = await screen.findByRole("button", { name: /pick a different file/i }); + fireEvent.click(clearBtn); + expect(screen.queryByText(/duplicate claim/i)).toBeNull(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd .worktrees/claims-unique-fix && npm test -- src/pages/Upload.test.tsx` + +Expected: FAIL — no error panel exists yet, all 4 tests fail. + +- [ ] **Step 3: Add error state and 409 catch branch in Upload.tsx** + +Edit `src/pages/Upload.tsx`: + +1. At the top imports, add: +```tsx +import { ApiError } from "@/lib/api"; +import { useNavigate } from "react-router-dom"; +``` + +2. Find the existing error-handling catch block (around line 683-687) and replace with: + +```tsx + } catch (err) { + if (err instanceof ApiError && err.status === 409) { + setUploadError({ + kind: "duplicate", + existingBatchId: err.existingBatchId, + filename: file.name, + }); + toast.error("Duplicate claim — file not ingested"); + } else { + toast.error( + err instanceof Error ? err.message : "Failed to parse file" + ); + } + } +``` + +3. Add state declaration near the other useState calls: +```tsx + type UploadError = + | { kind: "duplicate"; existingBatchId: string | null; filename: string }; + const [uploadError, setUploadError] = useState(null); + const navigate = useNavigate(); +``` + +4. In the JSX, add the inline panel above the streaming-results section. Find the place where streaming results render (search for "streamDelay" or the section that shows the parsed batch) and insert just before it: + +```tsx +{uploadError && uploadError.kind === "duplicate" ? ( +
+
+ + 409 + + Duplicate claim — file not ingested +
+

+ {uploadError.filename} collides with an + existing record. + {uploadError.existingBatchId + ? " Open the existing batch to compare, or pick a different file." + : " Pick a different file."} +

+
+ {uploadError.existingBatchId ? ( + + ) : null} + +
+
+) : null} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd .worktrees/claims-unique-fix && npm test -- src/pages/Upload.test.tsx` + +Expected: PASS for all 4 tests. + +- [ ] **Step 5: Commit** + +```bash +git add src/pages/Upload.tsx src/pages/Upload.test.tsx +git commit -m "feat(upload): inline 409 error panel with existing-batch link" +``` + +--- + +## Phase 3 — End-to-end verification + +### Task 3.1: Repro the original 409 with the actual multi-claim 837P file + +**Files:** none (manual verification) + +- [ ] **Step 1: Start backend + frontend** + +In one terminal: `cd backend && uvicorn cyclone.api:app --reload --port 8000` +In another: `cd .worktrees/claims-unique-fix && npm run dev` + +- [ ] **Step 2: Upload the reproducer file** + +Use `docs/prodfiles/837p-from-axiscare/tp11525703-837P-20260618153339862-1of1.txt` (93696 bytes, 28 NM1*IL segments, multi-claim member-id collisions). + +Expected: 200 OK + batch persisted with multiple claims having identical `member_id` (this was previously 409). + +- [ ] **Step 3: Re-upload the same file to trigger 409** + +Expected: 409 with `existing_batch_id` pointing at the first batch. + +- [ ] **Step 4: Verify inline panel in the browser** + +Open the upload page, drag the file in, verify the panel appears with the "Open existing batch →" link. + +- [ ] **Step 5: Commit any tweaks** + +If the panel needed any styling tweaks (e.g. spacing, colors), commit them: + +```bash +git add src/pages/Upload.tsx +git commit -m "polish(upload): 409 error panel visual tweaks" +``` + +--- + +## Self-review checklist (run after writing the plan) + +1. **Spec coverage** — Each section of the spec maps to a task: + - §3 Schema migration → Task 1.1 + - §4 Store helpers → Task 1.2 + - §5 API change → Task 1.3 + - §6 Frontend `ApiError.existingBatchId` → Task 2.1 + - §6 Frontend `Upload.tsx` panel → Task 2.2 + - §10 Test plan (9 tests) → All 9 covered (5 backend + 4 frontend) + +2. **Placeholder scan** — No "TBD", "TODO", "implement later" markers. The only intentional skip is the 835 duplicate-CLP01 test, called out explicitly. + +3. **Type consistency** — + - `find_existing_batch_for_claim` / `find_existing_batch_for_remit` return `str | None` everywhere. + - `ApiError.existingBatchId` is `string | null` in TS and `str | None` everywhere it's referenced. + - `setUploadError` payload shape `{ kind: "duplicate"; existingBatchId: string | null; filename: string }` is consistent in JSX and the catch branch. + +4. **Risk acknowledged** — Migration reversibility, loss of uniqueness, race window are all documented in spec §9 and reflected in test scope (we don't test the race condition). \ No newline at end of file