Tyler
7290cac643
The implementer caught that claims.id is a single-column PK, which prevents the same CLM01 from existing in multiple batches. That makes the spec's pre-flight 409 workflow unreachable and resubmits impossible. Migration 0014 relaxes the PKs to composite (batch_id, id) on both claims and remittances, and updates all FKs that referenced the old single-column PK. Task 1.3 in the plan is now the migration; the previous Task 1.3 (helper tightening) is renumbered to Task 1.4 and will run after 0014 lands.
799 lines
32 KiB
Markdown
799 lines
32 KiB
Markdown
# Parse → Detect → Decide: 837P/835 Upload Workflow
|
|
|
|
**Date:** 2026-06-21
|
|
**Branch:** `claims-unique-fix` (worktree)
|
|
**Status:** Draft (brainstorming approved, awaiting writing-plans)
|
|
**Supersedes:** `2026-06-21-cyclone-claims-unique-constraint-and-409-ux-design.md` (kept for the migration 0013 + store helper sections, which still apply).
|
|
|
|
---
|
|
|
|
## 1. Why this exists
|
|
|
|
Today's upload flow is "parse → validate → persist" in a single call.
|
|
When a claim's CLM01 collides with a prior batch, the persist raises
|
|
`IntegrityError`, the transaction rolls back, and the API returns 409
|
|
with **no parse result, no list of colliding claims, and no way to act**.
|
|
The user sees only an error message and a `batch_id` that doesn't exist.
|
|
|
|
This is wrong: the parse already happened. The user should see what was
|
|
parsed, see which claims collide with which prior batches, and decide
|
|
what to do (force-insert, delete the prior batch, or pick a different
|
|
file). The 409 response body today is too thin to make that decision.
|
|
|
|
The root cause of the 409s is a schema bug — see
|
|
`2026-06-21-cyclone-claims-unique-constraint-and-409-ux-design.md` §1.
|
|
Migration 0013 drops the `UNIQUE(batch_id, patient_control_number)` inline
|
|
constraint that 0003 was supposed to drop. After 0013 lands, **multi-claim
|
|
837P files where many CLM segments share a subscriber's `member_id` will
|
|
ingest cleanly for the first time**.
|
|
|
|
But 0013 alone is not enough. The current schema has `claims.id` and
|
|
`remittances.id` as single-column PRIMARY KEYs, which means the same
|
|
CLM01 cannot exist in two different batches. That makes "cross-batch
|
|
CLM01 collisions" impossible to express in the data — but it also makes
|
|
resubmits impossible, and it makes the 409-with-collision-summary workflow
|
|
this SP describes unreachable. **Migration 0014 (added as Task 1.3 to the
|
|
plan) relaxes the PKs to composite `(batch_id, id)`.** After 0014 lands,
|
|
real resubmits are representable, the pre-flight 409 path actually fires,
|
|
and the workflow defined below is exercisable end-to-end against real data.
|
|
|
|
This SP defines the workflow for both classes of collision:
|
|
1. Multi-claim files with shared `member_id` (no longer a 409 after 0013).
|
|
2. Files where one or more CLM01s exist in a prior batch (a 409 after 0014; this SP defines the UX for it).
|
|
|
|
---
|
|
|
|
## 2. Operator surface
|
|
|
|
| Surface | Change |
|
|
|---|---|
|
|
| Backend | Pre-flight dedup check in `parse_837` and `parse_835`. New `?force=true` query param. New `DELETE /api/batches/{id}` endpoint. 409 body shape changes. |
|
|
| Frontend | `Upload.tsx` panel renders the full parse result + collision summary, with actions: "Force insert (skip dups)", "Open prior batch", "Delete prior batch and retry", "Pick a different file". |
|
|
| Tests | Migration + store helpers (already done in `claims-unique-fix`). New tests for: pre-flight dedup, force-insert, within-file dup, race 409, DELETE endpoint, frontend panel. |
|
|
|
|
---
|
|
|
|
## 3. The workflow
|
|
|
|
### 3.1 No collision (the happy path)
|
|
|
|
```
|
|
User → POST /api/parse-837 (file)
|
|
← 200 + ParseResult + batch_id
|
|
Batch persisted. UI shows parsed claims and links to the new batch.
|
|
```
|
|
|
|
### 3.2 Collision (the new path)
|
|
|
|
```
|
|
User → POST /api/parse-837 (file)
|
|
← 409 + {
|
|
error: "Duplicate claim",
|
|
detail: "...",
|
|
existing_batch_id: "B123", # most-recent prior batch with a colliding CLM01
|
|
collisions: {
|
|
colliding_claim_ids: ["A", "B"],
|
|
total_collisions: 2,
|
|
total_claims: 141, # claims in the file
|
|
new_claims_after_skip: 139, # claims that WOULD be inserted on force
|
|
},
|
|
parse_result: { ... full ParseResult ... },
|
|
}
|
|
User sees the parse result in the panel.
|
|
User can:
|
|
- Click "Force insert (skip 2 dups)" → POST /api/parse-837?force=true (same file)
|
|
← 200 + ParseResult + { skipped_claim_ids: ["A", "B"], inserted: 139 }
|
|
- Click "Open prior batch" → navigate to /batches/B123
|
|
- Click "Delete prior batch" → DELETE /api/batches/B123, then click "Re-upload"
|
|
- Click "Pick a different file" → clear the upload state
|
|
```
|
|
|
|
### 3.3 Force-insert after collision
|
|
|
|
`force=true` skips the pre-flight check. The store's existing per-row
|
|
`s.get(Claim, claim_id)` dedup still skips colliding rows silently, so
|
|
the new batch persists with only the non-colliding claims. The response
|
|
body includes `skipped_claim_ids` so the UI can show what was skipped.
|
|
|
|
`force=true` does NOT bypass the parser. If the file fails validation
|
|
(missing diagnosis, malformed segment), the response is still 422.
|
|
|
|
### 3.4 Race condition (pre-flight clean, persist fails)
|
|
|
|
If a pre-flight dedup check finds no collisions, but a concurrent process
|
|
ingests a colliding CLM01 between the check and the persist, the persist
|
|
will still raise `IntegrityError`. The handler catches it and returns
|
|
**the same 409 shape as the pre-flight collision** with
|
|
`existing_batch_id` set to the racing batch and `detail` mentioning
|
|
"another process ingested this between the check and the persist —
|
|
re-upload to retry". The user re-runs the same flow.
|
|
|
|
---
|
|
|
|
## 4. Within-file duplicates
|
|
|
|
If the file itself has the same CLM01 twice (a malformed file, not a
|
|
cross-batch collision), the pre-flight check catches it the same way:
|
|
it returns 409 with `existing_batch_id: null` and `detail: "CLM01 A
|
|
appears twice in this file"`. The user can only force-insert (which
|
|
skips the second instance). They can't "delete the prior batch" because
|
|
there isn't one — it's a bad file.
|
|
|
|
---
|
|
|
|
## 5. The 409 body shape
|
|
|
|
```json
|
|
{
|
|
"error": "Duplicate claim",
|
|
"detail": "This file (or one previously ingested with the same claim control number) collides with an existing record. 2 of 141 claims collide with batch B123.",
|
|
"batch_id": null,
|
|
"existing_batch_id": "B123",
|
|
"collisions": {
|
|
"colliding_claim_ids": ["A", "B"],
|
|
"total_collisions": 2,
|
|
"total_claims": 141,
|
|
"new_claims_after_skip": 139
|
|
},
|
|
"parse_result": { ... full ParseResult ... }
|
|
}
|
|
```
|
|
|
|
Field semantics:
|
|
- `error`: short tag for the UI ("Duplicate claim", "Duplicate remittance", "Within-file duplicate CLM01").
|
|
- `detail`: human-readable, mentions the count and the existing batch when known.
|
|
- `batch_id`: always `null` on 409 (the insert rolled back).
|
|
- `existing_batch_id`: the most-recent prior batch that contains a colliding CLM01, or `null` if (a) the collision is within-file, or (b) the colliding claim has since been deleted (race).
|
|
- `collisions.colliding_claim_ids`: subset of `parse_result.claims[].claim_id` that collides.
|
|
- `collisions.total_claims`: count from `parse_result.summary.total_claims`.
|
|
- `collisions.new_claims_after_skip`: `total_claims - total_collisions`.
|
|
- `parse_result`: the full `ParseResult` (same shape as a 200 response body). The UI uses this to render the parsed claims list.
|
|
|
|
The 200 body on `force=true` adds `skipped_claim_ids: ["A", "B"]` at the top level so the UI can show a "skipped" badge per claim.
|
|
|
|
---
|
|
|
|
## 6. `DELETE /api/batches/{id}`
|
|
|
|
New endpoint. Cascades through `ON DELETE CASCADE` FKs:
|
|
|
|
```
|
|
batches ─┬─ claims ─┬─ matches
|
|
│ ├─ activity_events (claim_id)
|
|
│ └─ line_reconciliations
|
|
├─ remittances ─┬─ cas_adjustments
|
|
│ ├─ service_line_payments
|
|
│ └─ activity_events (remittance_id)
|
|
└─ activity_events (batch_id only)
|
|
```
|
|
|
|
FKs already declare `ON DELETE CASCADE` in the migrations, so the
|
|
SQLite engine handles the cascade. The endpoint just needs to
|
|
`session.delete(batch_row)` and commit.
|
|
|
|
The endpoint:
|
|
- `204 No Content` on success.
|
|
- `404 Not Found` if the batch doesn't exist.
|
|
- `409 Conflict` if the batch has any claims in a non-`submitted` state
|
|
(e.g., `paid`, `reversed`, `denied`). Forces the user to first
|
|
unreconcile — same as the existing 409 pattern for `manual_match` /
|
|
`manual_unmatch` (see `store.py:AlreadyMatchedError`).
|
|
|
|
A `batch_deleted` activity event is recorded before the delete so the
|
|
audit log has a tombstone. The event's `batch_id` will be `null` after
|
|
the cascade (the FK is to `batches.id` with no `ON DELETE` clause
|
|
specified in any migration; verify in `migrations/0001_initial.sql` —
|
|
the spec says we preserve audit history). If the FK is `ON DELETE
|
|
CASCADE`, we record the event AFTER the cascade with `batch_id` set to
|
|
the deleted id and rely on the cascade to remove it (acceptable, or we
|
|
use a no-cascade FK and keep the tombstone). **Open question resolved
|
|
during implementation by reading the actual FK clauses.**
|
|
|
|
---
|
|
|
|
## 7. Backend implementation
|
|
|
|
### 7.1 New dedup helper
|
|
|
|
`backend/src/cyclone/store.py` (already added in `claims-unique-fix`):
|
|
|
|
```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.
|
|
|
|
Returns the most-recent batch (ORDER BY parsed_at DESC LIMIT 1) so the
|
|
UI links to the most likely "where did the dup come from" answer.
|
|
"""
|
|
from sqlalchemy import select
|
|
from cyclone.db import Claim
|
|
with db.SessionLocal()() as s:
|
|
row = s.execute(
|
|
select(Claim.batch_id)
|
|
.where(Claim.id == claim_id)
|
|
.order_by(Claim.state_changed_at.desc()) # most-recent touch
|
|
.limit(1)
|
|
).first()
|
|
return row[0] if row else None
|
|
|
|
|
|
def find_existing_batch_for_remit(remit_id: str) -> str | None:
|
|
"""Same shape as find_existing_batch_for_claim but for remittances."""
|
|
from sqlalchemy import select
|
|
from cyclone.db import Remittance
|
|
with db.SessionLocal()() as s:
|
|
row = s.execute(
|
|
select(Remittance.batch_id)
|
|
.where(Remittance.id == remit_id)
|
|
.order_by(Remittance.received_at.desc())
|
|
.limit(1)
|
|
).first()
|
|
return row[0] if row else None
|
|
```
|
|
|
|
The current `claims-unique-fix` implementation uses
|
|
`select(Claim.batch_id).where(Claim.id == claim_id).limit(1)` without
|
|
`ORDER BY`. We replace it with the ordered version to satisfy
|
|
"return the most-recent colliding batch".
|
|
|
|
### 7.2 New pre-flight dedup check
|
|
|
|
`backend/src/cyclone/dedup.py` (new file, single responsibility):
|
|
|
|
```python
|
|
"""Pre-flight dedup for parsed 837P/835 batches.
|
|
|
|
Splits the parsed result into "would-insert" and "would-skip" sets by
|
|
querying the DB for any claim_id / payer_claim_control_number already
|
|
present. Also detects within-file duplicates by counting claim_id
|
|
frequencies.
|
|
|
|
Used by the parse-837 and parse-835 endpoints between validation and
|
|
persist, so the user can see the parse result + collision summary
|
|
before any DB write.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
from collections import Counter
|
|
from dataclasses import dataclass
|
|
|
|
from sqlalchemy import select
|
|
from sqlalchemy.orm import Session
|
|
|
|
from cyclone import db
|
|
from cyclone.db import Claim, Remittance
|
|
|
|
|
|
@dataclass(frozen=True)
|
|
class CollisionReport:
|
|
"""What the parse endpoint needs to render a 409 response."""
|
|
colliding_claim_ids: list[str] # CLM01s (837) or CLP01s (835)
|
|
existing_batch_id: str | None # most-recent prior batch with a collision, or None
|
|
within_file_duplicate_ids: list[str] # CLM01s appearing twice in this file (subset of colliding_claim_ids)
|
|
total_claims: int
|
|
|
|
|
|
def preflight_837(result, session: Session | None = None) -> CollisionReport:
|
|
"""Detect 837 collisions: within-file dupes + cross-batch CLM01 dupes."""
|
|
claim_ids = [c.claim_id for c in result.claims]
|
|
counts = Counter(claim_ids)
|
|
within_file_duplicate_ids = sorted(
|
|
cid for cid, n in counts.items() if n > 1
|
|
)
|
|
seen: set[str] = set(claim_ids)
|
|
if not seen:
|
|
return CollisionReport(
|
|
colliding_claim_ids=[],
|
|
existing_batch_id=None,
|
|
within_file_duplicate_ids=[],
|
|
total_claims=0,
|
|
)
|
|
|
|
own_session = session is None
|
|
if own_session:
|
|
session = db.SessionLocal()()
|
|
try:
|
|
rows = session.execute(
|
|
select(Claim.id, Claim.batch_id)
|
|
.where(Claim.id.in_(seen))
|
|
.order_by(Claim.state_changed_at.desc())
|
|
).all()
|
|
finally:
|
|
if own_session:
|
|
session.close()
|
|
|
|
db_collisions = {cid: bid for cid, bid in rows}
|
|
colliding = sorted(cid for cid in seen if cid in db_collisions)
|
|
existing_batch_id = next(iter(db_collisions.values()), None) if db_collisions else None
|
|
return CollisionReport(
|
|
colliding_claim_ids=colliding,
|
|
existing_batch_id=existing_batch_id,
|
|
within_file_duplicate_ids=within_file_duplicate_ids,
|
|
total_claims=len(claim_ids),
|
|
)
|
|
|
|
|
|
def preflight_835(result, session: Session | None = None) -> CollisionReport:
|
|
"""Same shape for 835 remittances. Payer claim control number = CLP01 = remittance.id."""
|
|
pcns = [c.payer_claim_control_number for c in result.claims]
|
|
counts = Counter(pcns)
|
|
within_file_duplicate_ids = sorted(p for p, n in counts.items() if n > 1)
|
|
seen = set(pcns)
|
|
if not seen:
|
|
return CollisionReport(
|
|
colliding_claim_ids=[],
|
|
existing_batch_id=None,
|
|
within_file_duplicate_ids=[],
|
|
total_claims=0,
|
|
)
|
|
|
|
own_session = session is None
|
|
if own_session:
|
|
session = db.SessionLocal()()
|
|
try:
|
|
rows = session.execute(
|
|
select(Remittance.id, Remittance.batch_id)
|
|
.where(Remittance.id.in_(seen))
|
|
.order_by(Remittance.received_at.desc())
|
|
).all()
|
|
finally:
|
|
if own_session:
|
|
session.close()
|
|
|
|
db_collisions = {pcn: bid for pcn, bid in rows}
|
|
colliding = sorted(pcn for pcn in seen if pcn in db_collisions)
|
|
existing_batch_id = next(iter(db_collisions.values()), None) if db_collisions else None
|
|
return CollisionReport(
|
|
colliding_claim_ids=colliding,
|
|
existing_batch_id=existing_batch_id,
|
|
within_file_duplicate_ids=within_file_duplicate_ids,
|
|
total_claims=len(pcns),
|
|
)
|
|
```
|
|
|
|
### 7.3 Modified `parse_837` endpoint
|
|
|
|
```python
|
|
@app.post("/api/parse-837")
|
|
async def parse_837(
|
|
request: Request,
|
|
file: UploadFile = File(...),
|
|
payer: str = Query("co_medicaid"),
|
|
include_raw_segments: bool = Query(True),
|
|
strict: bool = Query(False),
|
|
ack: bool = Query(False),
|
|
force: bool = Query(False), # NEW
|
|
) -> Any:
|
|
# ... existing parse + validate ...
|
|
|
|
if _has_claim_validation_errors(result):
|
|
return JSONResponse(status_code=422, content=json.loads(result.model_dump_json()))
|
|
|
|
# NEW: pre-flight dedup check
|
|
if not force and result.claims:
|
|
report = dedup.preflight_837(result)
|
|
if report.colliding_claim_ids or report.within_file_duplicate_ids:
|
|
return _build_409_response(
|
|
result=result,
|
|
report=report,
|
|
error="Duplicate claim",
|
|
kind="cross_batch" if report.existing_batch_id else "within_file",
|
|
)
|
|
|
|
# Persist (existing path). On IntegrityError (race), same 409 shape.
|
|
rec = BatchRecord(
|
|
id=uuid.uuid4().hex,
|
|
kind="837p",
|
|
input_filename=file.filename or "upload.txt",
|
|
parsed_at=utcnow(),
|
|
result=result,
|
|
)
|
|
try:
|
|
store.add(rec, event_bus=request.app.state.event_bus)
|
|
except IntegrityError as exc:
|
|
# Race: pre-flight said clean, but persist hit a PK. Re-run pre-flight
|
|
# so the 409 body has the same shape.
|
|
report = dedup.preflight_837(result)
|
|
return _build_409_response(
|
|
result=result,
|
|
report=report,
|
|
error="Duplicate claim (race condition)",
|
|
kind="race",
|
|
)
|
|
|
|
# ... existing response ...
|
|
if _client_wants_json(request):
|
|
body = json.loads(result.model_dump_json())
|
|
if ack:
|
|
ack_body = _build_and_persist_ack(rec.id)
|
|
if ack_body is not None:
|
|
body["ack"] = ack_body
|
|
# If force=true, the store.add silently skipped some claims.
|
|
# Surface what was skipped so the UI can show a "skipped" badge.
|
|
if force:
|
|
body["skipped_claim_ids"] = sorted({
|
|
c.claim_id for c in result.claims
|
|
if _claim_skipped(c.claim_id, rec.id)
|
|
})
|
|
return JSONResponse(content=body)
|
|
# ... streaming response ...
|
|
```
|
|
|
|
Where:
|
|
|
|
```python
|
|
def _build_409_response(
|
|
result, report, error: str, kind: str
|
|
) -> JSONResponse:
|
|
"""Build the standard 409 body for any dedup failure."""
|
|
if kind == "cross_batch":
|
|
detail = (
|
|
f"{len(report.colliding_claim_ids)} of {report.total_claims} "
|
|
f"claims collide with prior batch {report.existing_batch_id}. "
|
|
f"Force-insert to skip the duplicates, or delete the prior batch."
|
|
)
|
|
elif kind == "within_file":
|
|
detail = (
|
|
f"CLM01(s) {', '.join(report.within_file_duplicate_ids)} appear "
|
|
f"twice in this file. Force-insert will keep the first occurrence "
|
|
f"and skip the rest."
|
|
)
|
|
else: # race
|
|
detail = (
|
|
f"Another process ingested a colliding batch between the check "
|
|
f"and the persist. Re-upload to retry with the latest state."
|
|
)
|
|
body = {
|
|
"error": error,
|
|
"detail": detail,
|
|
"batch_id": None,
|
|
"existing_batch_id": report.existing_batch_id,
|
|
"collisions": {
|
|
"colliding_claim_ids": report.colliding_claim_ids,
|
|
"total_collisions": len(report.colliding_claim_ids),
|
|
"total_claims": report.total_claims,
|
|
"new_claims_after_skip": report.total_claims - len(report.colliding_claim_ids),
|
|
},
|
|
"parse_result": json.loads(result.model_dump_json()),
|
|
}
|
|
return JSONResponse(status_code=409, content=body)
|
|
```
|
|
|
|
`force=true` does NOT bypass validation (still 422 for bad data). It
|
|
only bypasses the pre-flight dedup. The `store.add` dedup still skips
|
|
colliding claims silently, but the response surfaces the skip list.
|
|
|
|
### 7.4 Modified `parse_835` endpoint
|
|
|
|
Same pattern, with `dedup.preflight_835` and the 835 parse result. Not
|
|
shown in detail; the structure mirrors 837.
|
|
|
|
### 7.5 New `DELETE /api/batches/{id}`
|
|
|
|
```python
|
|
@app.delete("/api/batches/{batch_id}")
|
|
def delete_batch(batch_id: str) -> dict:
|
|
"""Hard-delete a batch and all its child rows.
|
|
|
|
Returns 204 on success, 404 if missing, 409 if the batch has any
|
|
claims/remits in a non-`submitted` state (must unreconcile first).
|
|
"""
|
|
from cyclone import db
|
|
with db.SessionLocal()() as s:
|
|
batch = s.get(db.Batch, batch_id)
|
|
if batch is None:
|
|
raise HTTPException(404, f"Batch {batch_id} not found")
|
|
# Refuse if any claim/remittance is past 'submitted' state
|
|
non_submitted = s.execute(
|
|
select(db.Claim.id)
|
|
.where(db.Claim.batch_id == batch_id)
|
|
.where(db.Claim.state != "submitted")
|
|
.limit(1)
|
|
).first()
|
|
if non_submitted is not None:
|
|
raise HTTPException(
|
|
409,
|
|
f"Batch {batch_id} has claims in non-submitted state; "
|
|
f"unreconcile first before deleting.",
|
|
)
|
|
# Record tombstone activity event before the cascade
|
|
s.add(db.ActivityEvent(
|
|
ts=utcnow(),
|
|
kind="batch_deleted",
|
|
batch_id=batch_id,
|
|
payload_json={"message": f"Batch {batch_id} deleted"},
|
|
))
|
|
s.flush()
|
|
s.delete(batch)
|
|
s.commit()
|
|
return {"ok": True, "batch_id": batch_id}
|
|
```
|
|
|
|
The FKs in the schema (`migrations/0001_initial.sql` and later) declare
|
|
`ON DELETE CASCADE` on `claims.batch_id`, `remittances.batch_id`, etc.
|
|
SQLite handles the cascade at the engine level. We verify this assumption
|
|
in the implementation test by deleting a batch with child rows and
|
|
asserting the child rows are gone.
|
|
|
|
---
|
|
|
|
## 8. Frontend
|
|
|
|
### 8.1 `src/lib/api.ts`
|
|
|
|
`ApiError` carries more collision data:
|
|
|
|
```typescript
|
|
export class ApiError extends Error {
|
|
constructor(
|
|
public status: number,
|
|
message: string,
|
|
public existingBatchId: string | null = null,
|
|
public collisions: CollisionSummary | null = null,
|
|
public parseResult: unknown = null,
|
|
) {
|
|
super(message);
|
|
}
|
|
}
|
|
|
|
export type CollisionSummary = {
|
|
colliding_claim_ids: string[];
|
|
total_collisions: number;
|
|
total_claims: number;
|
|
new_claims_after_skip: number;
|
|
};
|
|
```
|
|
|
|
`parse837` adds `?force=true` to the URL when called for the
|
|
"force-insert" action:
|
|
|
|
```typescript
|
|
export async function parse837(
|
|
file: File,
|
|
options: { onProgress?: (p: number) => void; force?: boolean } = {},
|
|
): Promise<ParseResult> {
|
|
const url = `${base}/api/parse-837${options.force ? "?force=true" : ""}`;
|
|
// ... existing fetch + body parse ...
|
|
if (!res.ok) {
|
|
const { message, existingBatchId, collisions, parseResult } = await readErrorBody(res);
|
|
throw new ApiError(res.status, message, existingBatchId, collisions, parseResult);
|
|
}
|
|
return res.json();
|
|
}
|
|
```
|
|
|
|
### 8.2 `src/pages/Upload.tsx`
|
|
|
|
New state:
|
|
|
|
```typescript
|
|
type UploadError = {
|
|
kind: "duplicate";
|
|
existingBatchId: string | null;
|
|
collisions: CollisionSummary;
|
|
parseResult: ParseResult;
|
|
filename: string;
|
|
};
|
|
const [uploadError, setUploadError] = useState<UploadError | null>(null);
|
|
const [forceInserting, setForceInserting] = useState(false);
|
|
```
|
|
|
|
Panel JSX (above the streaming results):
|
|
|
|
```tsx
|
|
{uploadError ? (
|
|
<div
|
|
role="alert"
|
|
className="rounded-md border border-destructive/40 bg-destructive/5 p-4 mx-auto max-w-3xl"
|
|
>
|
|
<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">
|
|
{uploadError.collisions.total_collisions} of {uploadError.collisions.total_claims} claims
|
|
collide
|
|
{uploadError.existingBatchId
|
|
? ` with batch ${uploadError.existingBatchId}`
|
|
: " within this file"}
|
|
</span>
|
|
</div>
|
|
|
|
<p className="mt-2 text-sm text-muted-foreground">
|
|
File <span className="font-mono">{uploadError.filename}</span> would persist
|
|
{" "}{uploadError.collisions.new_claims_after_skip} of {uploadError.collisions.total_claims} claims.
|
|
Colliding CLM01s: {uploadError.collisions.colliding_claim_ids.join(", ")}.
|
|
</p>
|
|
|
|
<div className="mt-3 flex flex-wrap gap-2">
|
|
<Button
|
|
disabled={forceInserting}
|
|
onClick={async () => {
|
|
setForceInserting(true);
|
|
try {
|
|
// re-call with force=true; the response will be 200 + skipped_claim_ids
|
|
const result = await parse837(file, { onProgress: () => {}, force: true });
|
|
setParseResult(result);
|
|
setUploadError(null);
|
|
toast.success(
|
|
`Force-inserted: ${result.summary.total_claims - (result.skipped_claim_ids?.length ?? 0)} of ${result.summary.total_claims} claims (skipped ${result.skipped_claim_ids?.length ?? 0} dups)`,
|
|
);
|
|
} catch (err) {
|
|
toast.error(err instanceof Error ? err.message : "Force-insert failed");
|
|
} finally {
|
|
setForceInserting(false);
|
|
}
|
|
}}
|
|
>
|
|
Force insert (skip {uploadError.collisions.total_collisions} dups)
|
|
</Button>
|
|
|
|
{uploadError.existingBatchId ? (
|
|
<>
|
|
<Button variant="outline" onClick={() => navigate(`/batches/${uploadError.existingBatchId}`)}>
|
|
Open prior batch
|
|
</Button>
|
|
<Button
|
|
variant="outline"
|
|
onClick={async () => {
|
|
if (!confirm(`Delete batch ${uploadError.existingBatchId}? This cannot be undone.`)) return;
|
|
await deleteBatch(uploadError.existingBatchId);
|
|
toast.success(`Deleted ${uploadError.existingBatchId}`);
|
|
setUploadError(null);
|
|
pickFile(null);
|
|
}}
|
|
>
|
|
Delete prior batch
|
|
</Button>
|
|
</>
|
|
) : null}
|
|
|
|
<Button variant="ghost" onClick={() => { setUploadError(null); pickFile(null); }}>
|
|
Pick a different file
|
|
</Button>
|
|
</div>
|
|
|
|
{/* The full parse result is rendered below so the user can see what was parsed. */}
|
|
<details className="mt-3 text-sm">
|
|
<summary>Show parsed claims ({uploadError.parseResult.claims.length})</summary>
|
|
<pre className="mt-2 max-h-64 overflow-auto rounded bg-muted p-2 text-xs">
|
|
{JSON.stringify(uploadError.parseResult.summary, null, 2)}
|
|
</pre>
|
|
</details>
|
|
</div>
|
|
) : null}
|
|
```
|
|
|
|
---
|
|
|
|
## 9. Database
|
|
|
|
Migration 0013 already exists on the `claims-unique-fix` worktree. It
|
|
drops the `UNIQUE(batch_id, patient_control_number)` inline constraint.
|
|
After it runs:
|
|
|
|
- The 409 fires only on actual CLM01 collisions (not the `member_id`
|
|
dedup that was over-constraining before).
|
|
- Multi-claim 837P files with shared `member_id` ingest cleanly for
|
|
the first time.
|
|
|
|
Migration 0014 (added as Task 1.3 in the plan) further relaxes the schema:
|
|
it changes the PKs on `claims` and `remittances` from single-column
|
|
(`id`) to composite (`batch_id`, `id`). This is what allows resubmits and
|
|
makes the workflow in §3 reachable.
|
|
|
|
No new tables. No new columns. The DELETE endpoint relies on existing
|
|
`ON DELETE CASCADE` FKs.
|
|
|
|
---
|
|
|
|
## 10. Files changed
|
|
|
|
| File | Change |
|
|
|---|---|
|
|
| `backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql` | new (DONE on `claims-unique-fix`) |
|
|
| `backend/src/cyclone/migrations/0014_relax_claims_remits_pk.sql` | new: composite PK `(batch_id, id)` on `claims` and `remittances`; updates FKs |
|
|
| `backend/src/cyclone/store.py` | new `find_existing_batch_for_claim` / `find_existing_batch_for_remit` (DONE) + new `delete_batch` method |
|
|
| `backend/src/cyclone/dedup.py` | new file: pre-flight `preflight_837` / `preflight_835` + `CollisionReport` dataclass |
|
|
| `backend/src/cyclone/api.py` | 837/835 endpoints: pre-flight check, force param, new 409 body, race handler, new DELETE endpoint |
|
|
| `src/lib/api.ts` | `ApiError` adds `collisions` + `parseResult`; `parse837`/`parse835` accept `force`; new `deleteBatch` |
|
|
| `src/pages/Upload.tsx` | new `UploadError` state, error panel JSX, force-insert handler, delete-prior handler |
|
|
| `src/pages/Upload.test.tsx` | new tests (4 cases from §11) |
|
|
| `backend/tests/test_db_migrate.py` | 0013 idempotency + UNIQUE-dropped tests (DONE); 0014 composite-PK + FK-cascade tests |
|
|
| `backend/tests/test_store.py` | `find_existing_batch_for_claim`/`remit` tests (DONE) |
|
|
| `backend/tests/test_dedup.py` | new tests for `preflight_837` / `preflight_835` (§11) |
|
|
| `backend/tests/test_api_parse_persists.py` | new tests: pre-flight 409, force-insert, within-file 409, race 409, DELETE endpoint (§11) |
|
|
|
|
No new dependencies. No config changes.
|
|
|
|
---
|
|
|
|
## 11. Test plan
|
|
|
|
### Backend (pytest)
|
|
|
|
| Test | File | Asserts |
|
|
|---|---|---|
|
|
| `test_preflight_837_finds_no_collisions_on_empty_db` | `test_dedup.py` | empty DB → empty `colliding_claim_ids`, no `existing_batch_id` |
|
|
| `test_preflight_837_finds_cross_batch_collision` | `test_dedup.py` | pre-seed a claim; pre-flight returns that claim_id in `colliding_claim_ids` and the seeded batch in `existing_batch_id` |
|
|
| `test_preflight_837_finds_within_file_duplicate` | `test_dedup.py` | parsed result has the same CLM01 twice; pre-flight returns it in both `colliding_claim_ids` and `within_file_duplicate_ids` |
|
|
| `test_preflight_837_returns_most_recent_batch_id` | `test_dedup.py` | pre-seed 3 batches with the same CLM01 at different times; pre-flight returns the most-recent batch_id |
|
|
| `test_preflight_835_mirrors_837` | `test_dedup.py` | same shape for remittances |
|
|
| `test_parse_837_409_includes_parse_result_and_collisions` | `test_api_parse_persists.py` | pre-seed a claim; upload a file with a colliding CLM01; assert 409 with `parse_result`, `collisions.colliding_claim_ids`, `existing_batch_id` |
|
|
| `test_parse_837_409_within_file_duplicate_has_null_batch_id` | `test_api_parse_persists.py` | upload a file with the same CLM01 twice; assert 409 with `existing_batch_id: null` and `within_file_duplicate_ids` populated |
|
|
| `test_parse_837_force_true_persists_non_colliding_claims` | `test_api_parse_persists.py` | pre-seed a claim; upload a file with 3 claims, 1 colliding; assert 200 with `skipped_claim_ids: [colliding_id]`, the 2 non-colliding claims persist |
|
|
| `test_parse_837_force_true_does_not_bypass_validation` | `test_api_parse_persists.py` | a file that fails validation still returns 422 with `force=true` |
|
|
| `test_parse_837_race_409_uses_same_body_shape` | `test_api_parse_persists.py` | mock `store.add` to raise IntegrityError; assert 409 body has the same shape as the pre-flight 409 |
|
|
| `test_delete_batch_cascades_to_claims` | `test_api_parse_persists.py` | persist a batch with 2 claims; DELETE; assert batch and both claims are gone |
|
|
| `test_delete_batch_404_on_unknown` | `test_api_parse_persists.py` | DELETE /api/batches/does-not-exist → 404 |
|
|
| `test_delete_batch_409_on_reconciled_claims` | `test_api_parse_persists.py` | persist a batch, mark a claim state='paid'; DELETE → 409 |
|
|
| `test_parse_837_after_delete_succeeds` | `test_api_parse_persists.py` | pre-seed a colliding claim; DELETE that batch; re-upload the same file; assert 200 |
|
|
|
|
### Frontend (vitest)
|
|
|
|
| Test | File | Asserts |
|
|
|---|---|---|
|
|
| `test_error_panel_renders_on_409_with_collisions` | `Upload.test.tsx` | mock `parse837` to throw `ApiError(409, ..., PRIOR, collisions, parseResult)`; assert panel visible with all collision data |
|
|
| `test_force_insert_button_re_calls_with_force_true` | `Upload.test.tsx` | user clicks "Force insert"; assert `parse837` is called with `{ force: true }` |
|
|
| `test_delete_prior_button_calls_deleteBatch` | `Upload.test.tsx` | user clicks "Delete prior batch"; assert `deleteBatch(existingBatchId)` is called |
|
|
| `test_pick_different_clears_error` | `Upload.test.tsx` | user clicks "Pick a different file"; assert `uploadError` is cleared and file picker is reset |
|
|
| `test_no_panel_on_non_409` | `Upload.test.tsx` | 400 error; assert panel absent |
|
|
| `test_within_file_duplicate_omits_prior_batch_actions` | `Upload.test.tsx` | 409 with `existingBatchId: null`; assert "Open prior batch" and "Delete prior batch" buttons are absent |
|
|
|
|
---
|
|
|
|
## 12. Out of scope
|
|
|
|
* Batch editing (update claim state, edit claim fields). Future SP.
|
|
* Cross-batch dedup REPORT (a "find all CLM01s in batches B1+B2+B3"
|
|
query). Future SP.
|
|
* Migration reversibility for 0013 — the recreation preserves data but
|
|
not schema history. Acceptable since 0013 just drops an inline
|
|
constraint; recreating the constraint would be a separate migration.
|
|
* Audit event for force-insert skips. The user explicitly chose to
|
|
skip silently; we honor that.
|
|
|
|
---
|
|
|
|
## 13. Risk
|
|
|
|
* **Pre-flight check race**: between the check and the persist, a
|
|
concurrent process could ingest a colliding claim. The persist would
|
|
then raise `IntegrityError`; the handler returns the same 409 shape
|
|
with `detail` mentioning the race. The user re-runs. Acceptable.
|
|
* **DELETE on a large batch**: cascade through `claims`, `remittances`,
|
|
`matches`, `line_reconciliations`, `activity_events`. SQLite handles
|
|
the cascade in a single transaction; a 140-claim batch deletes in
|
|
<100ms. The endpoint refuses if any claim is past `submitted` state.
|
|
* **`force=true` silent skip**: the user clicks "Force insert" and
|
|
the response says "X of Y claims persisted, Z skipped". They
|
|
acknowledged this in the panel before clicking. No undo.
|
|
* **Within-file duplicates and force-insert**: the user can force-insert
|
|
a file with the same CLM01 twice. The first instance persists, the
|
|
second is silently skipped. This is intentional — within-file dupes
|
|
are usually a typo, and the user has explicitly asked to proceed.
|
|
* **`existing_batch_id` may be stale**: the helper returns the
|
|
most-recent batch by `state_changed_at` (or `received_at` for 835).
|
|
The user clicks "Open prior batch" and the batch may have been
|
|
deleted in the meantime. The BatchesList page already handles 404
|
|
gracefully.
|
|
|
|
---
|
|
|
|
## 14. Rollout
|
|
|
|
1. **Schema**: migration 0013 applies on next `cyclone` startup.
|
|
Idempotent and reversible only by rebuilding the `claims` table
|
|
(acceptable; production data preserved by the INSERT...SELECT).
|
|
2. **Backend API**: new `force` param + new 409 body shape + new
|
|
DELETE endpoint. Existing clients that don't pass `force` see the
|
|
same behavior as before for collision-free files. Collision cases
|
|
now get a richer 409 body that includes `parse_result`; clients
|
|
that ignore the new fields keep working.
|
|
3. **Frontend**: `Upload.tsx` panel replaces the toast on 409. Users
|
|
who don't read the panel still see the toast and the 409 message
|
|
in the streaming view.
|
|
4. **No data migration**: nothing to migrate. 0013 is structural only.
|