Tyler
386 lines
14 KiB
Markdown
386 lines
14 KiB
Markdown
# Drop `UNIQUE(batch_id, patient_control_number)` on Claims + Robust 409 UX
|
|
|
|
**Date:** 2026-06-21
|
|
**Branch:** `main`
|
|
**Status:** Draft (brainstorming approved, awaiting writing-plans)
|
|
**Scope:** Backend schema + minimal frontend error handling for collision 409.
|
|
|
|
---
|
|
|
|
## 1. Why this exists
|
|
|
|
Today, every multi-claim 837P file in `docs/prodfiles/837p-from-axiscare/` and
|
|
`docs/prodfiles/FromHPE/` returns **HTTP 409** on upload. The cause:
|
|
|
|
* The `claims` table has `UNIQUE(batch_id, patient_control_number)` declared
|
|
inline in `CREATE TABLE` (auto-index `sqlite_autoindex_claims_2`).
|
|
* `store._claim_837_row` populates `patient_control_number` from
|
|
`claim.subscriber.member_id` (the subscriber's insurance member id),
|
|
not from the CLM01 segment value.
|
|
* A real 837P submission routinely contains many `CLM*` segments per
|
|
subscriber (a member seeing multiple providers on the same day). All
|
|
those rows share the same `member_id` → same `patient_control_number`.
|
|
Within one batch, the constraint fires on claim #2.
|
|
|
|
Migration `0003_drop_claims_remits_unique_constraints.sql` already exists
|
|
with the right intent but is **buggy**: it does
|
|
`DROP INDEX IF EXISTS uq_claims_batch_pcn`, but the constraint is inline,
|
|
not a named index — so the `DROP INDEX` is a no-op and the constraint
|
|
survives.
|
|
|
|
The 409 error message ("duplicate CLM01 control numbers") is also
|
|
misleading because the column stores `member_id`, not CLM01.
|
|
|
|
This SP fixes both: drops the constraint properly via SQLite table
|
|
recreation, and gives the upload page a structured error panel for the
|
|
collision case so the operator can find the existing batch in one click.
|
|
|
|
---
|
|
|
|
## 2. Operator surface
|
|
|
|
| Surface | Change |
|
|
|---|---|
|
|
| DB | New migration `0013_drop_claims_unique_constraint.sql`; idempotent. |
|
|
| Python | `store.find_existing_batch_for_claim(claim_id)` new helper. |
|
|
| API | `POST /api/parse-837` and `/api/parse-835` 409 responses gain `existing_batch_id`. |
|
|
| UI | `/upload` page renders an inline error panel for 409 with a link to the existing batch. |
|
|
| Tests | New migration test, store helper test, API test, component test. |
|
|
|
|
No CLI / settings changes.
|
|
|
|
---
|
|
|
|
## 3. Schema migration
|
|
|
|
`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 file in an
|
|
-- implicit transaction via engine.begin(), so we MUST NOT use BEGIN/COMMIT
|
|
-- inside the file (nested transactions fail in SQLite). We defer FK
|
|
-- enforcement with PRAGMA defer_foreign_keys instead of turning FKs off
|
|
-- (which is a no-op inside a transaction in SQLite). The deferred
|
|
-- checks fire at commit and validate against the renamed claims table.
|
|
|
|
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;
|
|
```
|
|
|
|
**Note on the `db.py` ORM model:** `backend/src/cyclone/db.py` declares
|
|
the `Claim` ORM model with `UNIQUE(batch_id, patient_control_number)` in
|
|
`__table_args__`. After this migration the DB no longer enforces that
|
|
constraint, so the ORM declaration becomes aspirational. We leave it in
|
|
place as documentation (and as a guard if the DB ever gets rebuilt from
|
|
ORM on a fresh schema). No code reads through it; the dedup happens via
|
|
PK on `claims.id` in `store.add`.
|
|
|
|
---
|
|
|
|
## 4. Store helper
|
|
|
|
New function in `backend/src/cyclone/store.py`:
|
|
|
|
```python
|
|
def find_existing_batch_for_claim(claim_id: str) -> str | None:
|
|
"""Return the batch_id of the first batch containing this CLM01, or None."""
|
|
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(payer_claim_control_number: str) -> str | None:
|
|
"""Return the batch_id of the first batch containing this CLP01, or None."""
|
|
from cyclone import db
|
|
from cyclone.db import Remittance
|
|
with db.SessionLocal()() as s:
|
|
row = s.execute(
|
|
select(Remittance.batch_id)
|
|
.where(Remittance.id == payer_claim_control_number)
|
|
.limit(1)
|
|
).first()
|
|
return row[0] if row else None
|
|
```
|
|
|
|
Both pure reads; no transaction management needed.
|
|
|
|
---
|
|
|
|
## 5. API change
|
|
|
|
`backend/src/cyclone/api.py`:
|
|
|
|
### 837 path (line 394-415)
|
|
|
|
After the UNIQUE constraint is dropped in migration 0013, an `IntegrityError`
|
|
in `store.add` can only originate from the PK on `claims.id` (CLM01) —
|
|
either two claims in the same file share CLM01 (rare) or the same CLM01
|
|
exists in a prior batch. The handler picks the first claim from
|
|
`result.claims` and asks the helper whether a prior batch already holds
|
|
that CLM01:
|
|
|
|
```python
|
|
except IntegrityError as exc:
|
|
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)
|
|
```
|
|
|
|
The `existing_batch_id != rec.id` guard avoids surfacing the just-failed
|
|
batch as a "previous" batch (it never persisted).
|
|
|
|
### 835 path (line 588-602)
|
|
|
|
Same pattern with `find_existing_batch_for_remit` and the first remit's
|
|
`payer_claim_control_number` (`result.claims[0].payer_claim_control_number`):
|
|
|
|
```python
|
|
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)
|
|
```
|
|
|
|
---
|
|
|
|
## 6. Frontend change
|
|
|
|
### `src/lib/api.ts`
|
|
|
|
`ApiError` carries an optional `existingBatchId`:
|
|
|
|
```typescript
|
|
export class ApiError extends Error {
|
|
constructor(
|
|
public status: number,
|
|
message: string,
|
|
public existingBatchId: string | null = null,
|
|
) {
|
|
super(message);
|
|
}
|
|
}
|
|
```
|
|
|
|
`readErrorBody()` parses JSON and, when status ≥ 400, attempts to extract
|
|
`existing_batch_id` and pass it through. `parse837`/`parse835` throw
|
|
`new ApiError(res.status, detail, existingBatchId)`.
|
|
|
|
### `src/pages/Upload.tsx`
|
|
|
|
New local state:
|
|
|
|
```typescript
|
|
type UploadError = {
|
|
kind: "duplicate";
|
|
existingBatchId: string | null;
|
|
filename: string;
|
|
};
|
|
const [uploadError, setUploadError] = useState<UploadError | null>(null);
|
|
```
|
|
|
|
On mutation error:
|
|
|
|
```typescript
|
|
} 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");
|
|
}
|
|
}
|
|
```
|
|
|
|
Inline error panel JSX (renders above streaming results when
|
|
`uploadError` is set):
|
|
|
|
```tsx
|
|
{uploadError ? (
|
|
<div className="error-panel">
|
|
<Badge variant="destructive">409</Badge>
|
|
<div className="error-title">Duplicate claim — file not ingested</div>
|
|
<div className="error-detail">
|
|
{filename} collides with an existing record.
|
|
{existingBatchId
|
|
? " Open the existing batch to compare, or pick a different file."
|
|
: " Pick a different file."}
|
|
</div>
|
|
<div className="error-actions">
|
|
{existingBatchId ? (
|
|
<Button onClick={() => navigate(`/batches/${existingBatchId}`)}>
|
|
Open existing batch →
|
|
</Button>
|
|
) : null}
|
|
<Button variant="ghost" onClick={() => { pickFile(null); }}>
|
|
Pick a different file
|
|
</Button>
|
|
</div>
|
|
</div>
|
|
) : null}
|
|
```
|
|
|
|
Styling matches the existing paper-toned surface (slate/parchment) per
|
|
the hybrid dark/paper treatment used elsewhere in the app.
|
|
|
|
---
|
|
|
|
## 7. Files changed
|
|
|
|
* `backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql` — new
|
|
* `backend/src/cyclone/store.py` — `find_existing_batch_for_claim`,
|
|
`find_existing_batch_for_remit`
|
|
* `backend/src/cyclone/api.py` — both 409 handlers add
|
|
`existing_batch_id` lookup
|
|
* `src/lib/api.ts` — `ApiError.existingBatchId`
|
|
* `src/pages/Upload.tsx` — error state + inline panel
|
|
* `src/pages/Upload.test.tsx` — new test file
|
|
* `backend/tests/test_api_parse_persists.py` — new test cases
|
|
* `backend/tests/test_db_migrate.py` — new migration test
|
|
* `backend/tests/test_store.py` — new helper tests
|
|
|
|
No new dependencies. No config changes.
|
|
|
|
---
|
|
|
|
## 8. Out of scope
|
|
|
|
* A "Replace existing batch" destructive action — requires
|
|
`DELETE /api/batches/{id}` and reconciliation cascade handling; deferred.
|
|
* General 4xx error UI for other statuses (empty file, parse error,
|
|
validation errors). Those already surface in toasts and the streaming
|
|
view; a future SP can generalize the inline panel pattern.
|
|
* Renaming `patient_control_number` to `subscriber_member_id` to match
|
|
what it actually stores. Out of scope for this fix; tracked separately
|
|
if it becomes a source of confusion.
|
|
|
|
---
|
|
|
|
## 9. Risk
|
|
|
|
* **Migration reversibility**: SQLite has no `DROP CONSTRAINT`; the
|
|
recreation is destructive to schema (but not to data — `INSERT INTO
|
|
claims_new SELECT * FROM claims` preserves every row). If the
|
|
migration fails mid-way, the implicit transaction (`engine.begin()`
|
|
in `db_migrate.py`) rolls back. `PRAGMA defer_foreign_keys = ON` is
|
|
required because SQLite otherwise can't drop a table that other
|
|
tables reference (`remittances.claim_id`, `matches.claim_id`,
|
|
`line_reconciliations.claim_id`, `activity_events.claim_id`). The
|
|
deferred checks fire at commit and validate against the renamed
|
|
`claims` table.
|
|
* **Loss of uniqueness**: after the migration, two claims in one batch
|
|
*can* share a `patient_control_number`. This is the intended behavior.
|
|
Claim identity is still unique via `claims.id` (CLM01, PK) and the
|
|
existing dedup check in `store.add` (`s.get(Claim, claim.claim_id)`).
|
|
* **`existing_batch_id` race**: the helper runs after the failed
|
|
`store.add` transaction has rolled back. Between rollback and helper
|
|
call, another writer could delete the colliding claim. Result: 409
|
|
fires with no `existing_batch_id`; UI shows panel without link.
|
|
Acceptable — the panel still explains the collision and offers
|
|
"Pick a different file."
|
|
|
|
---
|
|
|
|
## 10. Test plan
|
|
|
|
| Test | Asserts |
|
|
|---|---|
|
|
| `test_db_migrate.py::test_drop_claims_unique_constraint` | After running migrations from v12, `user_version=13`, no `*claims*unique*` index exists, two rows with same `(batch_id, patient_control_number)` insert cleanly. |
|
|
| `test_db_migrate.py::test_migration_idempotent` | Running migrations on a v13 DB is a no-op. |
|
|
| `test_store.py::test_find_existing_batch_for_claim` | Helper returns None for unknown, returns batch_id for known, deterministic on duplicates. |
|
|
| `test_api_parse_persists.py::test_409_response_includes_existing_batch_id` | Upload duplicate; assert body has `existing_batch_id` pointing to the right batch. |
|
|
| `test_api_parse_persists.py::test_multi_claim_batch_with_duplicate_member_id_succeeds` | Upload a file with duplicate `member_id` claims; assert 200, all claims persisted. |
|
|
| `Upload.test.tsx::test_error_panel_renders_on_409_with_link` | Mock `useParse` to throw `ApiError(409, ..., existingBatchId)`; assert panel visible, link present. |
|
|
| `Upload.test.tsx::test_error_panel_renders_on_409_without_link` | Mock 409 with `existingBatchId: null`; assert panel visible, no link. |
|
|
| `Upload.test.tsx::test_no_panel_on_non_409` | Mock 400; assert panel absent, only toast. |
|
|
| `Upload.test.tsx::test_pick_different_clears_error` | Click button; assert `pickFile(null)` called and `errorState` cleared. | |