Files

T

Tyler b606e8c9a2 plan: drop claims UNIQUE + 409 UX implementation plan

2026-06-21 17:02:10 -06:00

36 KiB

Raw 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):

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:

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:

-- 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

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:

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:

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

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:

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:

    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:

    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

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:

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:

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:

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:

  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:

  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

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:

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:

At the top imports, add:

import { ApiError } from "@/lib/api";
import { useNavigate } from "react-router-dom";

Find the existing error-handling catch block (around line 683-687) and replace with:

    } 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"
        );
      }
    }

Add state declaration near the other useState calls:

  type UploadError =
    | { kind: "duplicate"; existingBatchId: string | null; filename: string };
  const [uploadError, setUploadError] = useState<UploadError | null>(null);
  const navigate = useNavigate();

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:

{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

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:

git add src/pages/Upload.tsx
git commit -m "polish(upload): 409 error panel visual tweaks"

Self-review checklist (run after writing the plan)

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)
Placeholder scan — No "TBD", "TODO", "implement later" markers. The only intentional skip is the 835 duplicate-CLP01 test, called out explicitly.
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.
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).

36 KiB Raw Blame History

Drop UNIQUE(batch_id, patient_control_number) + 409 UX Implementation Plan

Phase 1 — Backend migration + helpers + API

Task 1.1: Migration 0013 drops inline UNIQUE via table recreation

Task 1.2: Store helpers find_existing_batch_for_claim and find_existing_batch_for_remit

Task 1.3: 409 handlers in api.py surface existing_batch_id

Phase 2 — Frontend

Task 2.1: ApiError.existingBatchId + parse837/835 surface it

Task 2.2: Upload.tsx inline error panel for 409

Phase 3 — End-to-end verification

Task 3.1: Repro the original 409 with the actual multi-claim 837P file

Self-review checklist (run after writing the plan)

36 KiB

Raw Blame History

Drop `UNIQUE(batch_id, patient_control_number)` + 409 UX Implementation Plan

Task 1.2: Store helpers `find_existing_batch_for_claim` and `find_existing_batch_for_remit`

Task 1.3: 409 handlers in api.py surface `existing_batch_id`

Task 2.1: `ApiError.existingBatchId` + parse837/835 surface it

Task 2.2: `Upload.tsx` inline error panel for 409