tyler/cyclone
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
main
cyclone/docs/superpowers/plans/2026-06-21-cyclone-claims-unique-constraint-and-409-ux.md
T

926 lines
36 KiB
Markdown
Raw Permalink Blame History

# 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<typeof apiModule>("@/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(
<QueryClientProvider client={qc}>
<MemoryRouter initialEntries={["/upload"]}>
<Routes>
<Route path="/upload" element={<Upload />} />
<Route path="/batches/:id" element={<div data-testid="batch-page" />} />
</Routes>
</MemoryRouter>
</QueryClientProvider>,
);
}
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<UploadError | null>(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" ? (
<div
role="alert"
data-testid="duplicate-error-panel"
className="error-panel mx-auto max-w-3xl rounded-md border border-destructive/40 bg-destructive/5 p-4"
>
<div className="flex items-center gap-2">
<span className="inline-flex items-center rounded-md bg-destructive px-2 py-0.5 text-xs font-semibold text-destructive-foreground">
409
</span>
<span className="font-semibold">Duplicate claim — file not ingested</span>
</div>
<p className="mt-2 text-sm text-muted-foreground">
<span className="font-mono">{uploadError.filename}</span> collides with an
existing record.
{uploadError.existingBatchId
? " Open the existing batch to compare, or pick a different file."
: " Pick a different file."}
</p>
<div className="mt-3 flex gap-2">
{uploadError.existingBatchId ? (
<Button
onClick={() => navigate(`/batches/${uploadError.existingBatchId}`)}
>
Open existing batch →
</Button>
) : null}
<Button
variant="ghost"
onClick={() => {
setUploadError(null);
pickFile(null);
}}
>
Pick a different file
</Button>
</div>
</div>
) : 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).
Reference in New Issue View Git Blame Copy Permalink