spec: parse-then-decide workflow for 837/835 upload dedup
This commit is contained in:
@@ -0,0 +1,784 @@
|
||||
# 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 true cross-batch CLM01
|
||||
collisions can still fire, and the user needs the new workflow to handle
|
||||
them.
|
||||
|
||||
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 (still a 409).
|
||||
|
||||
---
|
||||
|
||||
## 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.
|
||||
|
||||
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/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) |
|
||||
| `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.
|
||||
Reference in New Issue
Block a user