15 Commits

Author SHA1 Message Date
Tyler de55003484 chore: stash in-progress test edit
Captures the modified state of test_api_parse_persists.py so the
in-progress test work is preserved on the claims-unique-fix branch
as the sp-prefixed worktrees are cleaned up. The .venv and
.gitignore_local noise in the working tree are intentionally left
untracked.
2026-06-22 11:05:56 -06:00
Grok c055787bd4 refactor(db_migrate): extract _apply_migrations_until helper; add edge-case tests
Issue 1 (refactor): run() and migrate_to_version() duplicated the inner
loop body — every change to the migration loop had to be applied in both
places. Pull the loop into _apply_migrations_until(engine, target_version):
when target_version is None, apply all pending; otherwise stop when the
file's version exceeds target. run() and migrate_to_version() are now
thin wrappers around the helper.

Issue 2 (tests): migrate_to_version() had no direct tests for its
edge-case behavior. Add 4 tests:
- idempotent: calling migrate_to_version(engine, N) twice == once
- lower-than-current: migrate_to_version(engine, 3) after N=13 is a no-op
- zero on fresh DB: migrate_to_version(engine, 0) is a no-op
- beyond-max: migrate_to_version(engine, 999) applies every migration

Tests use a fresh sa.Engine (not db.engine()) so they exercise the
target-version behavior from a clean user_version=0 starting point —
the conftest autouse fixture has already brought the module engine to
the latest version, which would mask the no-op branches.
2026-06-21 19:43:23 -06:00
Tyler ced8d20aed fix(db+tests): data-preservation test for migration 0014; document before_insert events
Address the three concerns raised by the spec reviewer on commit 534130e:

1. The 0014 data-preservation test was degraded: it only verified
   post-0014 INSERTs round-trip, never proving that 0014's INSERT INTO
   claims_new SELECT * FROM claims actually carries pre-existing rows
   through the table recreation. Add migrate_to_version() helper to
   db_migrate.py and rewrite the test to seed at v=13, apply 0014, and
   assert the rows survive. Add a complementary test exercising the
   JOIN-FK tables (matches / cas_adjustments / service_line_payments /
   line_reconciliations) whose batch_id columns get populated by
   0014's JOIN against the parent tables.

2. The four SQLAlchemy ORM before_insert events on Match /
   CasAdjustment / ServiceLinePayment / LineReconciliation are
   undocumented in the codebase. Add a clear module-level docstring
   explaining what the events do, why they exist (composite-FK
   NOT NULL columns require the batch side which most call sites
   don't have readily available), when they fail (parent id not
   findable in session.new / identity_map / DB -> misleading
   'NOT NULL constraint failed' IntegrityError), and the workarounds.

3. The plan Task 1.3 Step 8 said 'do not modify production code to
   make old tests pass'. In practice production code DID need to
   change because composite-PK semantics make single-id lookups
   ambiguous. Amend the plan to acknowledge this and cite the
   s.get(Claim, X) -> s.query(Claim).filter().first() refactor as
   the example. Also document the before_insert events in Step 5.

Brings in docs/superpowers/plans/2026-06-21-cyclone-parse-decide-workflow.md
from main (this branch was created before the plan file was committed)
with Task 1.3 Step 5 and Step 8 amended per the reviewer feedback.
2026-06-21 19:24:13 -06:00
Tyler 534130ee2b feat(db): migration 0014 relaxes claims/remittances PK to (batch_id, id)
Migration 0014 changes the PRIMARY KEYs of claims and remittances from
single-column id to composite (batch_id, id). Enables the spec'd
cross-batch CLM01 / CLP01 collision workflow: same CLM01 in multiple
batches is now representable (resubmits), pre-flight dedup 409 path is
genuinely exercisable, force-insert can skip pre-existing duplicates.

Strategy: PRAGMA defer_foreign_keys = ON + table recreation for every
table whose FKs pointed at the old single-column PK. Tables recreated:
remittances, claims, matches, cas_adjustments, service_line_payments,
line_reconciliations. Each child table gains a batch_id (or
remittance_batch_id) column for the composite FK side; INSERT INTO new
SELECT FROM old JOINs populate it from the already-recreated parent.

Cross-table FKs (remittances.claim_id, claims.matched_remittance_id)
cannot be SQL-enforced with composite PKs (SQLite has no ALTER
CONSTRAINT). Dropped at SQL level; enforced via application-layer
invariants in store.manual_match / manual_unmatch / reconcile.run and
the dedup.preflight_* helpers.

ORM updates (db.py):
- Claim / Remittance: composite PK via explicit PrimaryKeyConstraint in
  __table_args__ (column order matches SQL: batch_id, id).
- New Claim.matched_remittance_batch_id column.
- Match / CasAdjustment / ServiceLinePayment / LineReconciliation:
  added the batch side of their composite FK to the parent table.
- SQLAlchemy before_insert events auto-populate the batch side of the
  composite FK from session.new, then identity_map, then SQL fallback.

Production code updates:
- store.manual_match / manual_unmatch: also write
  matched_remittance_batch_id on the claim (was missing).
- api.py manual-match endpoint: same fix.
- reconcile.run: same fix for auto-matched pairs.

Test updates: replaced s.get(Claim, X) with the composite key
(batch_id, id) where batch_id is known, or s.query().filter().first()
where the test only knows the id. Tests that previously inserted a Match
row pointing at a non-existent Remittance now seed the parent Remittance
so the new NOT NULL composite FK is satisfied.
2026-06-21 18:56:18 -06:00
Tyler 890207f40d feat(store): add find_existing_batch_for_claim and find_existing_batch_for_remit 2026-06-21 17:40:10 -06:00
Tyler b6efd0eaee feat(db): drop inline UNIQUE(batch_id, patient_control_number) via migration 0013 2026-06-21 17:39:28 -06:00
Tyler 9a313d2c1b feat(activity): remit_received events drill to RemitDrawer 2026-06-21 17:19:31 -06:00
Tyler 2eb61f16ff feat(reconciliation): candidate rows drill to RemitDrawer or ClaimDrawer 2026-06-21 17:19:31 -06:00
Tyler 12c2913ba1 feat(batchdiff): remit rows drill to RemitDrawer 2026-06-21 17:19:31 -06:00
Tyler 54440da2cd feat(inbox): candidates + unmatched rows drill to RemitDrawer or ClaimDrawer 2026-06-21 17:19:31 -06:00
Tyler 7427838292 feat(remits): row click opens RemitDrawer (replaces inline CAS expand) 2026-06-21 17:19:31 -06:00
Tyler b606e8c9a2 plan: drop claims UNIQUE + 409 UX implementation plan 2026-06-21 17:02:10 -06:00
Tyler ac709c07c3 spec: self-review fixes (explicit claim_id arg, defer_foreign_keys) 2026-06-21 16:58:50 -06:00
Tyler 08e83da91d feat(drill): ProviderDrawer — Claims + Activity tabs from extended /providers/{npi} 2026-06-21 16:56:36 -06:00
Tyler 3f672d5db3 spec: drop claims UNIQUE constraint + 409 UX 2026-06-21 16:54:07 -06:00
43 changed files with 6751 additions and 265 deletions
+1
View File
@@ -7,3 +7,4 @@ __pycache__/
venv/
build/
dist/
backend/.venv
+21 -7
View File
@@ -1093,8 +1093,12 @@ def inbox_match_candidate(remit_id: str, body: dict):
if not claim_id:
raise HTTPException(400, "claim_id required")
with db.SessionLocal()() as s:
claim = s.get(Claim, claim_id)
remit = s.get(Remittance, remit_id)
# Migration 0014: composite PKs. The URL takes only id (not
# batch_id); look up by id alone, first match wins. Cross-batch
# duplicates are handled at ingest time by parse-decide-workflow;
# this manual-match endpoint inherits the legacy "id alone" API.
claim = s.query(Claim).filter(Claim.id == claim_id).first()
remit = s.query(Remittance).filter(Remittance.id == remit_id).first()
if claim is None or remit is None:
raise HTTPException(404, "claim or remit not found")
if claim.matched_remittance_id and claim.matched_remittance_id != remit_id:
@@ -1110,6 +1114,11 @@ def inbox_match_candidate(remit_id: str, body: dict):
},
)
claim.matched_remittance_id = remit_id
# Migration 0014: composite back-reference to remittances.
# matched_remittance_batch_id is the batch side; the SQL-level FK
# to remittances(batch_id, id) can't be declared in SQLite (no
# ALTER CONSTRAINT) so the application keeps both columns in sync.
claim.matched_remittance_batch_id = remit.batch_id
remit.claim_id = claim_id
s.commit()
return {"ok": True, "claim_id": claim_id, "remit_id": remit_id}
@@ -1159,7 +1168,8 @@ def inbox_acknowledge_payer_rejected(body: dict):
not_found = 0
not_rejected = 0
for cid in claim_ids:
claim = session.get(Claim, cid)
# Migration 0014: composite PK; URL takes only claim_id.
claim = session.query(Claim).filter(Claim.id == cid).first()
if claim is None:
not_found += 1
continue
@@ -1229,7 +1239,8 @@ def inbox_resubmit_rejected(
accepted_with_rows: list[tuple[str, "Claim"]] = []
with db.SessionLocal()() as s:
for cid in ids:
c = s.get(Claim, cid)
# Migration 0014: composite PK; URL takes only claim_id.
c = s.query(Claim).filter(Claim.id == cid).first()
if c is None:
continue
if c.state != ClaimState.REJECTED:
@@ -1521,7 +1532,8 @@ def serialize_claim_as_837(claim_id: str):
issue, not a transient failure).
"""
with db.SessionLocal()() as s:
row = s.get(Claim, claim_id)
# Migration 0014: composite PK; URL takes only claim_id.
row = s.query(Claim).filter(Claim.id == claim_id).first()
if row is None:
return JSONResponse(
{"error": "Not found", "detail": f"Claim {claim_id} not found"},
@@ -1584,7 +1596,8 @@ def get_claim_line_reconciliation(claim_id: str) -> dict:
from decimal import Decimal
with db.SessionLocal()() as s:
claim = s.get(db.Claim, claim_id)
# Migration 0014: composite PK; URL takes only claim_id.
claim = s.query(db.Claim).filter(db.Claim.id == claim_id).first()
if claim is None:
raise HTTPException(
status_code=404,
@@ -2363,7 +2376,8 @@ def _serialize_claim_for_submit(claim_id: str) -> str:
from cyclone.parsers.serialize_837 import serialize_837
from cyclone import db
with db.SessionLocal()() as s:
row = s.get(db.Claim, claim_id)
# Migration 0014: composite PK; URL takes only claim_id.
row = s.query(db.Claim).filter(db.Claim.id == claim_id).first()
if row is None:
raise ValueError(f"claim {claim_id!r} not found")
# Re-parse the stored raw_json to get a ClaimOutput
+215 -22
View File
@@ -34,6 +34,7 @@ from sqlalchemy import (
)
from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column, relationship, sessionmaker
from sqlalchemy.types import TypeDecorator
from sqlalchemy.schema import PrimaryKeyConstraint
DEFAULT_DB_PATH = Path.home() / ".local" / "share" / "cyclone" / "cyclone.db"
@@ -239,9 +240,15 @@ class Batch(Base):
class Claim(Base):
__tablename__ = "claims"
id: Mapped[str] = mapped_column(String(64), primary_key=True)
# Migration 0014: composite PRIMARY KEY (batch_id, id). Enables
# resubmits (same CLM01 in different batches) and makes the
# pre-flight dedup workflow exercisable. The composite PK is declared
# explicitly in __table_args__ below so the column order matches the
# migration (`PRIMARY KEY (batch_id, id)`).
id: Mapped[str] = mapped_column(String(64))
batch_id: Mapped[str] = mapped_column(
String(32), ForeignKey("batches.id", ondelete="CASCADE"), nullable=False
String(32),
ForeignKey("batches.id", ondelete="CASCADE"),
)
patient_control_number: Mapped[str] = mapped_column(String(64), nullable=False)
service_date_from: Mapped[Optional[date]] = mapped_column(Date, nullable=True)
@@ -290,29 +297,38 @@ class Claim(Base):
resubmit_count: Mapped[int] = mapped_column(
Integer, nullable=False, default=0, server_default=text("0")
)
# Back-reference to remittances. The composite pointer is split into
# matched_remittance_id + matched_remittance_batch_id. Migration 0014
# drops the SQL-level FK (composite FK to remittances(batch_id, id)
# cannot be declared as inline REFERENCES in SQLite; no ALTER CONSTRAINT).
# App-layer invariants in store.manual_match / manual_unmatch / dedup
# keep these consistent. matched_remittance_id points at
# remittances.id (a non-PK column under composite PK) — SQLAlchemy
# accepts this because FKs reference any column, not just PKs.
matched_remittance_id: Mapped[Optional[str]] = mapped_column(
# ORM policy: SET NULL on remittance delete. The claim may outlive its
# remittance during reversal/reimport flows; without this, deleting a
# remittance with matched claims raises IntegrityError.
# Note: 0001_initial.sql predates this policy (no ON DELETE clause).
# SQLite ignores FK direction by default unless PRAGMA foreign_keys=ON,
# so the inconsistency is benign there; the ORM is the source of truth
# for PostgreSQL/test environments with FK enforcement. Amendment to the
# migration is deferred to post-SQLite rollout.
String(64),
ForeignKey("remittances.id", ondelete="SET NULL"),
nullable=True,
)
matched_remittance_batch_id: Mapped[Optional[str]] = mapped_column(
String(32),
nullable=True,
)
raw_json: Mapped[Optional[dict]] = mapped_column(JSONText, nullable=True)
batch: Mapped["Batch"] = relationship(back_populates="claims")
__table_args__ = (
# Migration 0014: explicit composite PK column order (batch_id, id).
# SQLAlchemy's default composite-PK assembly is by declaration order,
# but `id` is declared first for readability; we override here so
# ``s.get(Claim, (batch_id, id))`` looks up by the right key order.
PrimaryKeyConstraint("batch_id", "id", name="pk_claims"),
# NOTE: no (batch_id, patient_control_number) unique constraint.
# X12 837P allows any number of CLM segments inside one 2000B
# subscriber loop, so a single member routinely has multiple
# claims in a single submission batch. Claim identity is provided
# by ``id`` (= CLM01 claim_id), which is the primary key.
# by the composite primary key (batch_id, id).
Index("ix_claims_state", "state"),
Index("ix_claims_patient_control_number", "patient_control_number"),
Index("ix_claims_service_date_from", "service_date_from"),
@@ -322,17 +338,19 @@ class Claim(Base):
class Remittance(Base):
__tablename__ = "remittances"
id: Mapped[str] = mapped_column(String(64), primary_key=True)
# Migration 0014: composite PRIMARY KEY (batch_id, id). Same rationale
# as Claim: enables resubmits (same CLP01 in different batches). The
# composite PK is declared explicitly in __table_args__ below so the
# column order matches the migration.
id: Mapped[str] = mapped_column(String(64))
batch_id: Mapped[str] = mapped_column(
String(32), ForeignKey("batches.id", ondelete="CASCADE"), nullable=False
String(32),
ForeignKey("batches.id", ondelete="CASCADE"),
)
payer_claim_control_number: Mapped[str] = mapped_column(String(64), nullable=False)
# ORM policy: no ON DELETE (forward FK from Remittance → Claim). A claim
# is unlikely to be deleted in normal flow (audit trail); if it ever is,
# the application layer (T7 reconciliation) must clear this FK. The
# migration predates this rationale; amendment deferred to post-SQLite
# rollout. SQLite without `PRAGMA foreign_keys=ON` does not enforce,
# so the divergence is benign in this engine.
# Forward FK from Remittance → Claim. As with Claim.matched_remittance_id,
# migration 0014 drops the SQL-level FK (cannot declare composite FK
# inline in SQLite). App-layer invariants keep these consistent.
claim_id: Mapped[Optional[str]] = mapped_column(
String(64), ForeignKey("claims.id"), nullable=True
)
@@ -352,14 +370,24 @@ class Remittance(Base):
batch: Mapped["Batch"] = relationship(back_populates="remittances")
cas_adjustments: Mapped[list["CasAdjustment"]] = relationship(
back_populates="remittance", cascade="all, delete-orphan"
back_populates="remittance",
cascade="all, delete-orphan",
# Migration 0014: CasAdjustment has TWO FKs to remittances
# (remittance_id and remittance_batch_id). Tell SQLAlchemy to use
# remittance_id for the relationship (the batch_id side is just a
# denormalized copy used for the composite FK constraint).
foreign_keys="CasAdjustment.remittance_id",
)
__table_args__ = (
# Migration 0014: explicit composite PK column order (batch_id, id).
# See Claim.__table_args__ for the rationale — same as for Claim.
PrimaryKeyConstraint("batch_id", "id", name="pk_remittances"),
# NOTE: no (batch_id, payer_claim_control_number) unique constraint.
# An 835 ERA can contain multiple CLP segments that share a
# payer_claim_control_number (e.g. reversals re-referencing the
# original PCN). Remittance identity is provided by ``id``.
# original PCN). Remittance identity is provided by the composite
# PK (batch_id, id).
Index("ix_remittances_claim_id", "claim_id"),
Index("ix_remittances_payer_claim_control_number", "payer_claim_control_number"),
Index("ix_remittances_status_code", "status_code"),
@@ -373,6 +401,15 @@ class CasAdjustment(Base):
remittance_id: Mapped[str] = mapped_column(
String(64), ForeignKey("remittances.id", ondelete="CASCADE"), nullable=False
)
# Migration 0014: remittances.id is no longer a single-column PK; we add
# the batch side of the composite FK to make the constraint declarative
# again. The SQL migration declares it as a real composite FK; here we
# use a plain column reference because the application always queries
# via remittance_id alone (the batch_id is denormalized for FK
# enforcement only — same value for every row with a given remittance_id).
remittance_batch_id: Mapped[str] = mapped_column(
String(32), ForeignKey("remittances.batch_id"), nullable=False
)
group_code: Mapped[str] = mapped_column(String(4), nullable=False)
reason_code: Mapped[str] = mapped_column(String(8), nullable=False)
amount: Mapped[Decimal] = mapped_column(Numeric(12, 2), nullable=False)
@@ -383,7 +420,12 @@ class CasAdjustment(Base):
nullable=True,
)
remittance: Mapped["Remittance"] = relationship(back_populates="cas_adjustments")
remittance: Mapped["Remittance"] = relationship(
back_populates="cas_adjustments",
# See Remittance.cas_adjustments for the rationale: disambiguate
# between the two FKs from CasAdjustment to remittances.
foreign_keys="CasAdjustment.remittance_id",
)
__table_args__ = (
Index("ix_cas_adjustments_remittance_id", "remittance_id"),
@@ -405,6 +447,10 @@ class ServiceLinePayment(Base):
remittance_id: Mapped[str] = mapped_column(
String(64), ForeignKey("remittances.id", ondelete="CASCADE"), nullable=False
)
# Migration 0014: see CasAdjustment.remittance_batch_id for the same rationale.
remittance_batch_id: Mapped[str] = mapped_column(
String(32), ForeignKey("remittances.batch_id"), nullable=False
)
line_number: Mapped[int] = mapped_column(Integer, nullable=False)
procedure_qualifier: Mapped[str] = mapped_column(String(4), nullable=False)
procedure_code: Mapped[str] = mapped_column(String(16), nullable=False)
@@ -453,6 +499,10 @@ class LineReconciliation(Base):
claim_id: Mapped[str] = mapped_column(
String(64), ForeignKey("claims.id", ondelete="CASCADE"), nullable=False
)
# Migration 0014: see CasAdjustment.remittance_batch_id for the same rationale.
batch_id: Mapped[str] = mapped_column(
String(32), ForeignKey("claims.batch_id"), nullable=False
)
claim_service_line_number: Mapped[Optional[int]] = mapped_column(
Integer, nullable=True
)
@@ -491,9 +541,17 @@ class Match(Base):
String(64), ForeignKey("claims.id", ondelete="CASCADE"),
nullable=False,
)
# Migration 0014: batch side of the composite FK to claims.
batch_id: Mapped[str] = mapped_column(
String(32), ForeignKey("claims.batch_id"), nullable=False
)
remittance_id: Mapped[str] = mapped_column(
String(64), ForeignKey("remittances.id", ondelete="CASCADE"), nullable=False
)
# Migration 0014: batch side of the composite FK to remittances.
remittance_batch_id: Mapped[str] = mapped_column(
String(32), ForeignKey("remittances.batch_id"), nullable=False
)
strategy: Mapped[str] = mapped_column(String(16), nullable=False)
matched_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False)
prior_claim_state: Mapped[Optional[ClaimState]] = mapped_column(
@@ -836,3 +894,138 @@ class ClearhouseORM(Base):
filename_block_json: Mapped[dict] = mapped_column(JSONText, nullable=False)
sftp_block_json: Mapped[dict] = mapped_column(JSONText, nullable=False)
updated_at: Mapped[str] = mapped_column(String(32), nullable=False)
# =============================================================================
# Migration 0014: ORM `before_insert` events auto-populate the batch side
# of composite FKs.
# =============================================================================
#
# **What.** The four listeners below (``_match_before_insert``,
# ``_cas_before_insert``, ``_slp_before_insert``, ``_lr_before_insert``)
# run on every INSERT of a ``Match`` / ``CasAdjustment`` /
# ``ServiceLinePayment`` / ``LineReconciliation`` row. They look up the
# parent ``Claim`` / ``Remittance`` and copy its ``batch_id`` (and
# ``remittance_batch_id``) into the new row's composite-FK column.
#
# **Why.** After migration 0014, every child table has a composite FK to
# its parent: ``(batch_id, claim_id)`` for Match / LineReconciliation and
# ``(remittance_batch_id, remittance_id)`` for CasAdjustment /
# ServiceLinePayment. Both sides are ``NOT NULL`` at the SQL level, so
# every insert must supply ``batch_id``. Application code that creates
# these rows usually has the parent ``claim_id`` / ``remittance_id`` in
# scope (often as a string) but does not always have ``batch_id``
# readily available — the batch is created elsewhere in the same ingest
# transaction, or the parent is fetched by id alone. Threading
# ``batch_id`` through every call site would be error-prone. The events
# solve this transparently: caller passes the parent id as before, and
# the event fills in the batch side.
#
# **Lookup order** (preferred-to-fallback, to minimize round-trips):
# 1. ``session.new`` — pending parents added in this transaction
# (covers the common ingest/match flow where parent + child are
# added in the same session).
# 2. ``session.identity_map`` — already-persisted parents loaded
# earlier in this session.
# 3. ``session.execute(select(...))`` — DB query as a last resort.
# ``autoflush`` is OFF on our ``SessionLocal``, so this won't
# trigger an unintended flush of ``session.new``.
#
# **When it fails.** If the parent is in none of the above (e.g. the
# caller passed a parent id that was never added to the session and is
# not in the DB), the column stays NULL and the INSERT fails with
# ``IntegrityError: NOT NULL constraint failed: <table>.batch_id`` (or
# ``remittance_batch_id``). The error message is misleading — it looks
# like the column was forgotten, not that the parent id was wrong.
# Workarounds:
# * If the parent exists in another session, ``session.add(parent)``
# first (or use ``session.merge(parent)``) so the lookup finds it
# in ``session.new`` / ``session.identity_map``.
# * If you have the parent object in scope and know its ``batch_id``,
# set ``child.batch_id = ...`` explicitly — the event only fills
# in when the column is None, so explicit values win.
# * If the parent is on a different connection entirely, populate the
# column by hand before ``session.add(child)``.
#
# **Idempotency.** The events only act when ``target.batch_id is None``
# (or ``remittance_batch_id``). Application code may explicitly set
# either column — e.g., a bulk-loader that already has batch_id in hand
# skips the lookup.
#
# **Scope.** ``propagate=True`` so subclasses of these mappers (in
# tests, mainly) inherit the listeners. Production code does not use
# mapper inheritance for these tables.
from sqlalchemy import event as _sa_event # noqa: E402
from sqlalchemy.orm import Session as _Session # noqa: E402
def _resolve_claim_batch_id(session: "_Session", claim_id: "str | None") -> "str | None":
"""Look up a Claim's batch_id, preferring session-cached state to SQL."""
if claim_id is None:
return None
for obj in session.new:
if isinstance(obj, Claim) and obj.id == claim_id:
return obj.batch_id
for obj in session.identity_map.values():
if isinstance(obj, Claim) and obj.id == claim_id:
return obj.batch_id
from sqlalchemy import select as _select
return session.execute(
_select(Claim.batch_id).where(Claim.id == claim_id)
).scalar_one_or_none()
def _resolve_remit_batch_id(session: "_Session", remittance_id: "str | None") -> "str | None":
"""Look up a Remittance's batch_id, preferring session-cached state to SQL."""
if remittance_id is None:
return None
for obj in session.new:
if isinstance(obj, Remittance) and obj.id == remittance_id:
return obj.batch_id
for obj in session.identity_map.values():
if isinstance(obj, Remittance) and obj.id == remittance_id:
return obj.batch_id
from sqlalchemy import select as _select
return session.execute(
_select(Remittance.batch_id).where(Remittance.id == remittance_id)
).scalar_one_or_none()
@_sa_event.listens_for(Match, "before_insert", propagate=True)
def _match_before_insert(mapper, connection, target):
if target.batch_id is None or target.remittance_batch_id is None:
session = _Session.object_session(target)
if session is None:
return
if target.batch_id is None:
target.batch_id = _resolve_claim_batch_id(session, target.claim_id)
if target.remittance_batch_id is None:
target.remittance_batch_id = _resolve_remit_batch_id(session, target.remittance_id)
@_sa_event.listens_for(CasAdjustment, "before_insert", propagate=True)
def _cas_before_insert(mapper, connection, target):
if target.remittance_batch_id is None:
session = _Session.object_session(target)
if session is None:
return
target.remittance_batch_id = _resolve_remit_batch_id(session, target.remittance_id)
@_sa_event.listens_for(ServiceLinePayment, "before_insert", propagate=True)
def _slp_before_insert(mapper, connection, target):
if target.remittance_batch_id is None:
session = _Session.object_session(target)
if session is None:
return
target.remittance_batch_id = _resolve_remit_batch_id(session, target.remittance_id)
@_sa_event.listens_for(LineReconciliation, "before_insert", propagate=True)
def _lr_before_insert(mapper, connection, target):
if target.batch_id is None:
session = _Session.object_session(target)
if session is None:
return
target.batch_id = _resolve_claim_batch_id(session, target.claim_id)
+39 -12
View File
@@ -47,31 +47,58 @@ def _strip_comments(sql: str) -> str:
return "\n".join(lines)
def run(engine: sa.Engine) -> None:
"""Apply any pending migrations. Idempotent."""
migrations_dir = MIGRATIONS_DIR
if not migrations_dir.exists():
return # no migrations directory yet — fresh checkout, no-op
migration_files = sorted(migrations_dir.glob("*.sql"))
if not migration_files:
return
def _apply_migrations_until(engine: sa.Engine, target_version: int | None) -> None:
"""Apply migrations up to ``target_version`` (inclusive).
If ``target_version`` is ``None``, apply every pending migration. If a
migration's version is greater than ``target_version``, stop iterating.
Idempotent: migrations whose version is <= the current ``PRAGMA user_version``
are skipped.
"""
with engine.begin() as conn:
current = conn.exec_driver_sql("PRAGMA user_version").scalar() or 0
for path in migration_files:
for path in sorted(MIGRATIONS_DIR.glob("*.sql")):
sql = path.read_text()
version = _migration_version(sql, path.name)
if version <= current:
continue
if target_version is not None and version > target_version:
break
statements_sql = _strip_comments(sql)
statements = [s.strip() for s in statements_sql.split(";") if s.strip()]
with engine.begin() as conn:
for stmt in statements:
conn.exec_driver_sql(stmt)
conn.exec_driver_sql(f"PRAGMA user_version = {version}")
current = version
current = version
def run(engine: sa.Engine) -> None:
"""Apply any pending migrations. Idempotent."""
if not MIGRATIONS_DIR.exists():
return # no migrations directory yet — fresh checkout, no-op
if not list(MIGRATIONS_DIR.glob("*.sql")):
return
_apply_migrations_until(engine, None)
def migrate_to_version(engine: sa.Engine, target_version: int) -> None:
"""Apply migrations up to and including ``target_version``. Idempotent.
Reads the current ``PRAGMA user_version`` and applies any migrations
whose version is in the range ``(current, target_version]``. Stops
before applying migrations with version > ``target_version`` so the
caller can seed data between N and N+1.
Useful for tests that need to seed data after migrations ``0001..N``
have applied but before ``N+1`` runs — exercise a migration against
real, pre-existing rows rather than only verifying that post-migration
inserts round-trip.
Calling with ``target_version`` <= the current version is a no-op.
"""
if not MIGRATIONS_DIR.exists():
return
_apply_migrations_until(engine, target_version)
@@ -0,0 +1,59 @@
-- 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;
@@ -0,0 +1,253 @@
-- version: 14
-- Relax PRIMARY KEYs on `claims` and `remittances` from single-column (id)
-- to composite (batch_id, id). Enables resubmits (same CLM01 / CLP01 in
-- different batches) and makes the pre-flight dedup workflow exercisable.
--
-- Strategy (mirrors 0013): table recreation with PRAGMA
-- defer_foreign_keys. We must recreate every table that has an FK pointing
-- at `claims(id)` or `remittances(id)` so that FK constraints can be
-- updated to point at the new composite PK.
--
-- FKs that need updating (verified via pragma_foreign_key_list on the
-- pre-migration schema):
-- - remittances.claim_id -> claims(id) becomes -> (batch_id, id)
-- - claims.matched_remittance_id -> remittances(id) becomes -> (batch_id, id)
-- - matches.claim_id -> claims(id) ON DELETE CASCADE becomes -> (batch_id, id)
-- - matches.remittance_id -> remittances(id) ON DELETE CASCADE becomes -> (batch_id, id)
-- - cas_adjustments.remittance_id -> remittances(id) ON DELETE CASCADE becomes -> (batch_id, id)
-- - service_line_payments.remittance_id -> remittances(id) ON DELETE CASCADE becomes -> (batch_id, id)
-- - line_reconciliations.claim_id -> claims(id) ON DELETE CASCADE becomes -> (batch_id, id)
--
-- The cross-table FKs (remittances.claim_id, claims.matched_remittance_id)
-- can NOT be SQL-enforced with composite PKs because SQLite has no
-- ALTER TABLE ADD CONSTRAINT and DROP/ADD CONSTRAINT for an existing FK.
-- We keep them as plain TEXT columns (no REFERENCES clause) and rely on
-- application-layer invariants (store.manual_match / manual_unmatch,
-- dedup.preflight_*) to keep them consistent. The application code already
-- owns those paths; this is a deliberate trade-off documented in 0001's
-- comments ("amendment deferred to post-SQLite rollout").
--
-- The single-column FKs from claims/remittances DOWN to other tables
-- (matches, cas_adjustments, service_line_payments, line_reconciliations)
-- MUST be updated to composite FKs because those tables are recreated with
-- composite FK columns. We add a `batch_id` (or `remittance_batch_id`)
-- column to each and JOIN against the parent table to populate it during
-- the migration.
--
-- Why PRAGMA defer_foreign_keys (and not foreign_keys=OFF): the migration
-- runner wraps each .sql file in `engine.begin()` (a transaction). Inside a
-- transaction, `PRAGMA defer_foreign_keys = ON` defers FK checks to COMMIT,
-- which is the only safe way to drop a referenced table. `PRAGMA
-- foreign_keys` cannot be changed inside a transaction in SQLite, so the
-- defer_foreign_keys approach is what works inside the runner's transaction.
PRAGMA defer_foreign_keys = ON;
-- ============================================================================
-- Step 1: recreate `remittances` with composite PK (batch_id, id).
-- ============================================================================
-- Column shape: every column from 0001_initial.sql + claim_level_adjustment_amount
-- (added by 0006_line_reconciliation.sql). The `claim_id` column is kept
-- without an SQL-level FK — composite-FK to claims is enforced at the
-- application layer (see plan).
CREATE TABLE remittances_new (
id TEXT NOT NULL,
batch_id TEXT NOT NULL REFERENCES batches(id) ON DELETE CASCADE,
payer_claim_control_number TEXT NOT NULL,
claim_id TEXT,
status_code TEXT NOT NULL,
status_label TEXT,
total_charge NUMERIC(12, 2) NOT NULL DEFAULT 0,
total_paid NUMERIC(12, 2) NOT NULL DEFAULT 0,
patient_responsibility NUMERIC(12, 2),
adjustment_amount NUMERIC(12, 2) NOT NULL DEFAULT 0,
claim_level_adjustment_amount NUMERIC(12, 2) NOT NULL DEFAULT 0,
received_at DATETIME NOT NULL,
service_date DATE,
is_reversal INTEGER NOT NULL DEFAULT 0,
raw_json TEXT,
PRIMARY KEY (batch_id, id)
);
INSERT INTO remittances_new
SELECT id, batch_id, payer_claim_control_number, claim_id, status_code,
status_label, total_charge, total_paid, patient_responsibility,
adjustment_amount, claim_level_adjustment_amount, received_at,
service_date, is_reversal, raw_json
FROM remittances;
DROP TABLE remittances;
ALTER TABLE remittances_new RENAME TO remittances;
CREATE INDEX ix_remittances_claim_id ON remittances(claim_id);
CREATE INDEX ix_remittances_payer_claim_control_number ON remittances(payer_claim_control_number);
CREATE INDEX ix_remittances_status_code ON remittances(status_code);
-- ============================================================================
-- Step 2: recreate `claims` with composite PK (batch_id, id).
-- ============================================================================
-- Column shape: every column from 0013_drop_claims_unique_constraint.sql
-- (which itself consolidated 0001 + 0004 + 0008 + 0010), plus a new
-- `matched_remittance_batch_id` column. The back-reference to remittances
-- becomes a 2-column pointer (matched_remittance_id + matched_remittance_batch_id)
-- but the SQL-level FK is dropped (no ALTER CONSTRAINT in SQLite); see plan.
--
-- Column order MUST match the 0013 column order exactly so that
-- `INSERT INTO claims_new SELECT ... FROM claims` populates the right
-- columns. The trailing NULL fills the new matched_remittance_batch_id.
CREATE TABLE claims_new (
id TEXT NOT NULL,
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: kept without an SQL-level FK (composite
-- back-reference requires the application layer; see plan header).
matched_remittance_id TEXT,
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,
-- matched_remittance_batch_id: NEW. Always NULL on migration because
-- the pre-0014 schema only stored matched_remittance_id (single column).
-- App code populates it on manual_match / match_auto going forward.
matched_remittance_batch_id TEXT,
PRIMARY KEY (batch_id, id)
);
INSERT INTO claims_new
SELECT id, batch_id, patient_control_number, service_date_from,
service_date_to, charge_amount, provider_npi, payer_id, state,
state_before_reversal, matched_remittance_id, raw_json,
rejection_reason, rejected_at, resubmit_count, state_changed_at,
payer_rejected_at, payer_rejected_reason,
payer_rejected_status_code, payer_rejected_by_277ca_id,
payer_rejected_acknowledged_at, payer_rejected_acknowledged_actor,
NULL
FROM claims;
DROP TABLE claims;
ALTER TABLE claims_new RENAME TO claims;
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 3: recreate `matches`, `cas_adjustments`, `service_line_payments`,
-- `line_reconciliations` so their FKs point at the new composite PKs.
--
-- At this point the OLD `claims` and `remittances` tables are gone (dropped
-- in Steps 1 and 2) — only the recreated (composite-PK) versions exist.
-- The JOIN below reads the NEW parents, which have identical data
-- (same row count, same id values, same batch_id values); we only need
-- `batch_id` from each parent to populate the composite-FK column.
-- ============================================================================
CREATE TABLE matches_new (
id INTEGER PRIMARY KEY AUTOINCREMENT,
claim_id TEXT NOT NULL,
batch_id TEXT NOT NULL,
remittance_id TEXT NOT NULL,
remittance_batch_id TEXT NOT NULL,
strategy TEXT NOT NULL,
matched_at DATETIME NOT NULL,
prior_claim_state TEXT,
is_reversal INTEGER NOT NULL DEFAULT 0,
FOREIGN KEY (batch_id, claim_id) REFERENCES claims(batch_id, id) ON DELETE CASCADE,
FOREIGN KEY (remittance_batch_id, remittance_id) REFERENCES remittances(batch_id, id) ON DELETE CASCADE
);
INSERT INTO matches_new
SELECT m.id, m.claim_id, c.batch_id, m.remittance_id, r.batch_id,
m.strategy, m.matched_at, m.prior_claim_state, m.is_reversal
FROM matches m
JOIN claims c ON c.id = m.claim_id
JOIN remittances r ON r.id = m.remittance_id;
DROP TABLE matches;
ALTER TABLE matches_new RENAME TO matches;
CREATE INDEX ix_matches_claim_id ON matches(claim_id);
CREATE INDEX ix_matches_remittance_id ON matches(remittance_id);
CREATE INDEX ix_matches_matched_at ON matches(matched_at);
CREATE TABLE cas_adjustments_new (
id INTEGER PRIMARY KEY AUTOINCREMENT,
remittance_id TEXT NOT NULL,
remittance_batch_id TEXT NOT NULL,
group_code TEXT NOT NULL,
reason_code TEXT NOT NULL,
amount NUMERIC(12, 2) NOT NULL,
quantity NUMERIC(10, 2),
service_line_payment_id INTEGER REFERENCES service_line_payments(id) ON DELETE SET NULL,
FOREIGN KEY (remittance_batch_id, remittance_id) REFERENCES remittances(batch_id, id) ON DELETE CASCADE
);
INSERT INTO cas_adjustments_new
SELECT ca.id, ca.remittance_id, r.batch_id, ca.group_code, ca.reason_code,
ca.amount, ca.quantity, ca.service_line_payment_id
FROM cas_adjustments ca
JOIN remittances r ON r.id = ca.remittance_id;
DROP TABLE cas_adjustments;
ALTER TABLE cas_adjustments_new RENAME TO cas_adjustments;
CREATE INDEX ix_cas_adjustments_remittance_id ON cas_adjustments(remittance_id);
CREATE INDEX ix_cas_adjustments_service_line_payment_id ON cas_adjustments(service_line_payment_id);
CREATE TABLE service_line_payments_new (
id INTEGER PRIMARY KEY AUTOINCREMENT,
remittance_id TEXT NOT NULL,
remittance_batch_id TEXT NOT NULL,
line_number INTEGER NOT NULL,
procedure_qualifier VARCHAR(4) NOT NULL,
procedure_code VARCHAR(16) NOT NULL,
modifiers_json TEXT NOT NULL DEFAULT '[]',
charge NUMERIC(12, 2) NOT NULL,
payment NUMERIC(12, 2) NOT NULL,
units NUMERIC(10, 2),
unit_type VARCHAR(8),
service_date DATE,
ref_benefit_plan VARCHAR(64),
superseded_by_id INTEGER REFERENCES service_line_payments(id) ON DELETE SET NULL,
FOREIGN KEY (remittance_batch_id, remittance_id) REFERENCES remittances(batch_id, id) ON DELETE CASCADE
);
INSERT INTO service_line_payments_new
SELECT slp.id, slp.remittance_id, r.batch_id, slp.line_number,
slp.procedure_qualifier, slp.procedure_code, slp.modifiers_json,
slp.charge, slp.payment, slp.units, slp.unit_type, slp.service_date,
slp.ref_benefit_plan, slp.superseded_by_id
FROM service_line_payments slp
JOIN remittances r ON r.id = slp.remittance_id;
DROP TABLE service_line_payments;
ALTER TABLE service_line_payments_new RENAME TO service_line_payments;
CREATE INDEX ix_service_line_payments_remittance_id ON service_line_payments(remittance_id);
CREATE INDEX ix_service_line_payments_procedure_code_date ON service_line_payments(procedure_code, service_date);
CREATE TABLE line_reconciliations_new (
id INTEGER PRIMARY KEY AUTOINCREMENT,
claim_id VARCHAR(64) NOT NULL,
batch_id VARCHAR(64) NOT NULL,
claim_service_line_number INTEGER,
service_line_payment_id INTEGER REFERENCES service_line_payments(id) ON DELETE SET NULL,
status VARCHAR(16) NOT NULL,
match_score INTEGER,
reconciled_at TIMESTAMP NOT NULL,
FOREIGN KEY (batch_id, claim_id) REFERENCES claims(batch_id, id) ON DELETE CASCADE
);
INSERT INTO line_reconciliations_new
SELECT lr.id, lr.claim_id, c.batch_id, lr.claim_service_line_number,
lr.service_line_payment_id, lr.status, lr.match_score, lr.reconciled_at
FROM line_reconciliations lr
JOIN claims c ON c.id = lr.claim_id;
DROP TABLE line_reconciliations;
ALTER TABLE line_reconciliations_new RENAME TO line_reconciliations;
CREATE INDEX ix_line_reconciliations_claim_id ON line_reconciliations(claim_id);
CREATE INDEX ix_line_reconciliations_claim_service_line_number ON line_reconciliations(claim_service_line_number);
CREATE INDEX ix_line_reconciliations_service_line_payment_id ON line_reconciliations(service_line_payment_id);
+3
View File
@@ -287,6 +287,9 @@ def run(session, batch_id: str) -> ReconcileResult:
else:
m.claim.state = ClaimState.REVERSED
m.claim.matched_remittance_id = m.remittance.id
# Migration 0014: composite back-reference to remittances — both
# sides needed for get_claim_detail to JOIN back to remittances.
m.claim.matched_remittance_batch_id = m.remittance.batch_id
# Symmetric FK: also set Remittance.claim_id so list_unmatched (which
# filters on Remittance.claim_id IS NULL) correctly drops auto-matched
# remits from the orphan bucket. Manual matches do this too.
+87 -11
View File
@@ -64,6 +64,40 @@ class AlreadyMatchedError(Exception):
"""
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.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.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
class NotMatchedError(Exception):
"""Raised by ``CycloneStore.manual_unmatch`` when the claim has no match.
@@ -952,7 +986,10 @@ class CycloneStore:
if isinstance(record, BatchRecord837):
result: ParseResult = record.result
for claim in result.claims:
if s.get(Claim, claim.claim_id) is not None:
# Migration 0014: claims PK is composite (batch_id, id).
# Duplicate-check is per-batch now — same CLM01 in a
# DIFFERENT batch is allowed (resubmits / retry).
if s.get(Claim, (record.id, claim.claim_id)) is not None:
log.warning(
"add: claim %s already exists; skipping (batch=%s)",
claim.claim_id, record.id,
@@ -978,7 +1015,9 @@ class CycloneStore:
result835: ParseResult835 = record.result
payer_name = result835.payer.name
for cp in result835.claims:
if s.get(Remittance, cp.payer_claim_control_number) is not None:
# Migration 0014: remittances PK is composite. Per-batch
# duplicate-check; cross-batch CLP01 is allowed.
if s.get(Remittance, (record.id, cp.payer_claim_control_number)) is not None:
log.warning(
"add: remittance %s already exists; skipping (batch=%s)",
cp.payer_claim_control_number, record.id,
@@ -1062,7 +1101,11 @@ class CycloneStore:
try:
with db.SessionLocal()() as s:
for cid in claim_ids:
row = s.get(Claim, cid)
# Migration 0014: composite PK. The event log
# (ActivityEvent.claim_id) only stores the id, not
# batch_id, so look up by id alone within this batch
# context (record.id).
row = s.get(Claim, (record.id, cid))
if row is None:
continue
ui = to_ui_claim_from_orm(
@@ -1071,7 +1114,7 @@ class CycloneStore:
)
self._sync_publish(event_bus, "claim_written", ui)
for rid in remit_ids:
row = s.get(Remittance, rid)
row = s.get(Remittance, (record.id, rid))
if row is None:
continue
ui = to_ui_remittance_from_orm(
@@ -1218,7 +1261,12 @@ class CycloneStore:
per-line payments + adjustments without a second fetch.
"""
with db.SessionLocal()() as s:
row = s.get(Remittance, remittance_id)
# Migration 0014: composite PK. The public store.get_remittance
# API takes only remittance_id (callers may not know batch_id),
# so we look up by id alone. If duplicates exist across
# batches (resubmits), we return the first match — the same
# semantics the old single-PK s.get() provided.
row = s.query(Remittance).filter(Remittance.id == remittance_id).first()
if row is None:
return None
cas_rows = (
@@ -1290,7 +1338,9 @@ class CycloneStore:
from cyclone import db as _db
with _db.SessionLocal()() as s:
row = s.get(Claim, claim_id)
# Migration 0014: see get_remittance above — public API takes
# only claim_id; look up by id alone, return first match.
row = s.query(Claim).filter(Claim.id == claim_id).first()
if row is None:
return None
@@ -1330,7 +1380,12 @@ class CycloneStore:
]
if row.matched_remittance_id is not None:
remit = s.get(Remittance, row.matched_remittance_id)
# Migration 0014: composite PK; matched_remittance_batch_id
# is the batch side of the back-reference.
remit = s.get(
Remittance,
(row.matched_remittance_batch_id, row.matched_remittance_id),
)
if remit is not None:
status = (
"reconciled"
@@ -1983,7 +2038,12 @@ class CycloneStore:
from cyclone import reconcile as _reconcile
with db.SessionLocal()() as s:
claim = s.get(Claim, claim_id)
# Migration 0014: public API takes only claim_id / remit_id.
# Look up by id alone — if duplicates exist across batches,
# the first match wins (the manual_match workflow is per-batch
# so the user knows which batch they mean; cross-batch
# collisions are handled at ingest time by parse-decide-workflow).
claim = s.query(Claim).filter(Claim.id == claim_id).first()
if claim is None:
raise LookupError(f"claim {claim_id} not found")
if claim.matched_remittance_id is not None:
@@ -1992,7 +2052,7 @@ class CycloneStore:
f"{claim.matched_remittance_id}"
)
remit = s.get(Remittance, remit_id)
remit = s.query(Remittance).filter(Remittance.id == remit_id).first()
if remit is None:
raise LookupError(f"remittance {remit_id} not found")
@@ -2031,6 +2091,12 @@ class CycloneStore:
))
claim.state = new_state
claim.matched_remittance_id = remit_id
# Migration 0014: composite back-reference to remittances. The
# claim side needs both id and batch_id (we drop the SQL-level
# cross-table FK because SQLite has no ALTER CONSTRAINT;
# matched_remittance_id alone is not enough to look up the
# remittance under composite PK).
claim.matched_remittance_batch_id = remit.batch_id
# Symmetric FK update — see list_unmatched docstring for why
# we don't rely on T10 to do this.
remit.claim_id = claim_id
@@ -2100,7 +2166,9 @@ class CycloneStore:
6. Return ``{"claim": <ui>, "deletedMatches": <count>}``.
"""
with db.SessionLocal()() as s:
claim = s.get(Claim, claim_id)
# Migration 0014: see manual_match — public API takes only
# claim_id; look up by id alone, first match wins.
claim = s.query(Claim).filter(Claim.id == claim_id).first()
if claim is None:
raise LookupError(f"claim {claim_id} not found")
if claim.matched_remittance_id is None:
@@ -2134,12 +2202,20 @@ class CycloneStore:
claim.state = restored_state
claim.matched_remittance_id = None
# Migration 0014: clear the batch side of the composite
# back-reference too. Both columns must move together.
claim.matched_remittance_batch_id = None
# Clear the symmetric FK on the remittance so list_unmatched
# surfaces the pair again. The remittance may have been
# deleted between the match and this call — guard with a
# get() so we don't blow up on a stale FK.
if latest is not None:
paired_remit = s.get(Remittance, latest.remittance_id)
# Migration 0014: composite PK — latest is a Match row
# which has remittance_batch_id (the batch side of the FK).
paired_remit = s.get(
Remittance,
(latest.remittance_batch_id, latest.remittance_id),
)
if paired_remit is not None:
paired_remit.claim_id = None
+2 -1
View File
@@ -68,7 +68,8 @@ def test_999_set_rejection_moves_claim_to_rejected_state(client: TestClient):
assert r.status_code == 200, r.text
with db.SessionLocal()() as s:
c = s.get(Claim, "CLP-1")
# Migration 0014: composite PK (batch_id, id).
c = s.get(Claim, ("B-1", "CLP-1"))
assert c.state == ClaimState.REJECTED
assert c.rejected_at is not None
assert "999" in (c.rejection_reason or "")
+6 -4
View File
@@ -51,19 +51,21 @@ def test_migration_0002_creates_acks_table():
def test_migration_latest_idempotent_on_fresh_db():
"""Re-running the migration on the same DB must be a no-op (PRAGMA
user_version already at the latest version — currently 12 after
user_version already at the latest version — currently 14 after
0004-0006 line_reconciliation, 0005 ta1_acks, SP9's 0007
providers/payers/clearhouse, SP10's 0008 payer_rejected,
SP11's 0009 audit_log, SP14's 0010 payer_rejected_acknowledged,
SP16's 0011 processed_inbound_files, SP17's 0012 db_backups)."""
SP16's 0011 processed_inbound_files, SP17's 0012 db_backups,
SP19's 0013 drop_claims_unique_constraint, SP20's 0014
relax_claims_remits_pk)."""
with db.engine().begin() as c:
v1 = c.exec_driver_sql("PRAGMA user_version").scalar() or 0
assert v1 == 12
assert v1 == 14
# A second run should not raise and should not bump the version.
db_migrate.run(db.engine())
with db.engine().begin() as c:
v2 = c.exec_driver_sql("PRAGMA user_version").scalar() or 0
assert v2 == 12
assert v2 == 14
def test_add_ack_persists_row():
+4 -2
View File
@@ -98,8 +98,10 @@ class TestParse277CAEndpointHappyPath:
files={"file": ("minimal_277ca.txt", text, "text/plain")},
)
with db.SessionLocal()() as s:
c1 = s.get(Claim, "c1")
c2 = s.get(Claim, "c2")
# Migration 0014: composite PK (batch_id, id). The _seed_claim
# helper uses batch_id="BATCH-1".
c1 = s.get(Claim, ("BATCH-1", "c1"))
c2 = s.get(Claim, ("BATCH-1", "c2"))
assert c1.payer_rejected_at is None
assert c2.payer_rejected_at is not None
assert c2.payer_rejected_status_code == "A6"
+57
View File
@@ -357,3 +357,60 @@ def test_prodfile_cross_pipeline_reconciles(client: TestClient):
assert claim_rows >= len(unique_837_pcns), (
f"claim_rows {claim_rows} < unique_837_pcns {len(unique_837_pcns)}"
)
def test_409_response_includes_existing_batch_id_for_837(client: TestClient) -> None:
"""When a CLM01 already exists in a prior batch, the 409 body has existing_batch_id."""
from datetime import datetime, timezone
from cyclone import db as _db
from cyclone.db import Batch, Claim
# Seed a prior batch with claim CLM-X.
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"
+16 -8
View File
@@ -70,7 +70,9 @@ class TestApply277CARejectionsHappyPath:
assert outcome.matched == ["c1"]
assert outcome.orphans == []
with db.SessionLocal()() as s:
c = s.get(Claim, "c1")
# Migration 0014: composite PK; only one claim with id "c1"
# exists in this test, so filter-by-id returns it.
c = s.query(Claim).filter(Claim.id == "c1").first()
assert c.payer_rejected_at is not None
assert c.payer_rejected_status_code == "A6"
assert "A6" in (c.payer_rejected_reason or "")
@@ -116,14 +118,16 @@ class TestApply277CARejectionsIdempotent:
return s.query(Claim).filter_by(patient_control_number="CLAIM001").first()
outcome1 = apply_277ca_rejections(s, result, claim_lookup=_lookup, two77ca_id="ACK-1")
assert outcome1.matched == ["c1"]
original_reason = s.get(Claim, "c1").payer_rejected_reason
original_at = s.get(Claim, "c1").payer_rejected_at
# Migration 0014: composite PK — see test_rejected_status_stamps_matching_claim.
original_reason = s.query(Claim).filter(Claim.id == "c1").first().payer_rejected_reason
original_at = s.query(Claim).filter(Claim.id == "c1").first().payer_rejected_at
# Run again with same code.
outcome2 = apply_277ca_rejections(s, result, claim_lookup=_lookup, two77ca_id="ACK-1")
assert outcome2.matched == []
assert outcome2.already_rejected == ["c1"]
with db.SessionLocal()() as s:
c = s.get(Claim, "c1")
# Migration 0014: see above — composite PK.
c = s.query(Claim).filter(Claim.id == "c1").first()
# Reason and timestamp unchanged.
assert c.payer_rejected_reason == original_reason
assert c.payer_rejected_at == original_at
@@ -143,7 +147,8 @@ class TestApply277CAOnlyRejectsRejected:
outcome = apply_277ca_rejections(s, result, claim_lookup=_lookup, two77ca_id="ACK-1")
assert outcome.matched == []
with db.SessionLocal()() as s:
c = s.get(Claim, "c1")
# Migration 0014: composite PK.
c = s.query(Claim).filter(Claim.id == "c1").first()
assert c.payer_rejected_at is None
@@ -183,9 +188,12 @@ class TestApply277CAMultipleStatuses:
outcome = apply_277ca_rejections(s, result, claim_lookup=_lookup, two77ca_id="ACK-1")
assert outcome.matched == ["c2"]
with db.SessionLocal()() as s:
assert s.get(Claim, "c1").payer_rejected_at is None
assert s.get(Claim, "c2").payer_rejected_at is not None
assert s.get(Claim, "c3").payer_rejected_at is None
# Migration 0014: composite PK — filter-by-id returns the
# single claim with that id (no cross-batch duplicates in
# this test fixture).
assert s.query(Claim).filter(Claim.id == "c1").first().payer_rejected_at is None
assert s.query(Claim).filter(Claim.id == "c2").first().payer_rejected_at is not None
assert s.query(Claim).filter(Claim.id == "c3").first().payer_rejected_at is None
# --------------------------------------------------------------------------- #
+542 -1
View File
@@ -2,6 +2,7 @@
from __future__ import annotations
from datetime import datetime, timezone
from pathlib import Path
import pytest
@@ -112,4 +113,544 @@ def test_run_ignores_non_sql_files(
"SELECT name FROM sqlite_master WHERE type='table' AND name='should_not_exist'"
).all()
assert len(rows) == 0
assert _user_version(engine) == 1
assert _user_version(engine) == 1
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.
We copy the real 0013 file (so this test exercises the real migration, not
a synthetic one) and stub 0001-0012 with a synthetic 0001 that has the
historical inline UNIQUE. If 0013 doesn't exist as a real file, the runner
will leave user_version at 1 and the test will fail.
"""
monkeypatch.setattr(db_migrate, "MIGRATIONS_DIR", tmp_path)
# Synthetic 0001: claims with INLINE UNIQUE(batch_id, patient_control_number).
# The schema mirrors the real 0001 column shape so 0013's
# ``INSERT INTO claims_new SELECT * FROM claims`` works against this fixture.
(tmp_path / "0001_initial.sql").write_text(
"-- version: 1\n"
"CREATE TABLE batches (id TEXT PRIMARY KEY, kind TEXT NOT NULL, "
"input_filename TEXT NOT NULL, parsed_at DATETIME NOT NULL);\n"
"CREATE TABLE claims ("
"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,"
"UNIQUE (batch_id, patient_control_number));\n"
)
# Copy the REAL 0013 from the source tree. If it's missing, the test fails
# because user_version will stay at 1 and we'll never get to assert it == 13.
real_migrations = Path(db_migrate.__file__).parent / "migrations"
real_0013 = real_migrations / "0013_drop_claims_unique_constraint.sql"
assert real_0013.exists(), (
f"Real migration 0013 missing at {real_0013} — this test is meant to "
f"exercise the real migration, not a synthetic one."
)
(tmp_path / "0013_drop_claims_unique_constraint.sql").write_text(real_0013.read_text())
engine = _fresh_engine(tmp_path)
db_migrate.run(engine)
assert _user_version(engine) == 13, (
"0013 did not apply; user_version did not reach 13. Either the migration "
"file is missing, or it has a syntax error."
)
# Before-0013 assertion: with the inline UNIQUE, two claims sharing
# (batch_id, patient_control_number) on the same batch must fail. The
# recreation must remove that constraint, so this insert must now succeed.
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')"
)
c.exec_driver_sql(
"INSERT INTO claims(id, batch_id, patient_control_number) VALUES ('c2', 'b1', 'M')"
)
ids = [r[0] for r in c.exec_driver_sql("SELECT id FROM claims ORDER BY id").all()]
assert ids == ["c1", "c2"], (
"Both claims with same (batch_id, patient_control_number) must persist "
"after 0013 — the UNIQUE(batch_id, patient_control_number) is gone."
)
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)
# Use synthetic 0001 + copy the real 0013 so we exercise the real file.
(tmp_path / "0001_initial.sql").write_text(
"-- version: 1\n"
"CREATE TABLE batches (id TEXT PRIMARY KEY, kind TEXT NOT NULL, "
"input_filename TEXT NOT NULL, parsed_at DATETIME NOT NULL);\n"
"CREATE TABLE claims ("
"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,"
"UNIQUE (batch_id, patient_control_number));\n"
)
real_migrations = Path(db_migrate.__file__).parent / "migrations"
real_0013 = real_migrations / "0013_drop_claims_unique_constraint.sql"
assert real_0013.exists(), "Real 0013 missing — test cannot exercise it."
(tmp_path / "0013_drop_claims_unique_constraint.sql").write_text(real_0013.read_text())
engine = _fresh_engine(tmp_path)
db_migrate.run(engine) # applies both
version_after_first = _user_version(engine)
assert version_after_first == 13
db_migrate.run(engine) # second call: no-op
assert _user_version(engine) == version_after_first
# =============================================================================
# Migration 0014: relax claims/remittances PK to composite (batch_id, id)
# =============================================================================
# Synthetic 0001 with the FULL post-0013 column shape, so 0014's
# INSERT...SELECT statements find every column they expect on the source
# tables. Mirrors the shape after 0001 + 0004 + 0006 + 0008 + 0010 + 0013.
# The tables and columns here are exactly what 0014 reads from; we do NOT
# add migrations 0002..0012 because they're orthogonal to the FK chain
# 0014 walks (no claims/remittances FKs to acks, audit_log, etc).
_0014_SYNTHETIC_0001 = (
"-- version: 1\n"
"CREATE TABLE batches ("
"id TEXT PRIMARY KEY, kind TEXT NOT NULL, input_filename TEXT NOT NULL,"
"parsed_at DATETIME NOT NULL, totals_json TEXT, validation_json TEXT,"
"raw_result_json TEXT);\n"
"CREATE TABLE claims ("
"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"
"CREATE TABLE remittances ("
"id TEXT PRIMARY KEY, batch_id TEXT NOT NULL REFERENCES batches(id) ON DELETE CASCADE,"
"payer_claim_control_number TEXT NOT NULL, claim_id TEXT REFERENCES claims(id),"
"status_code TEXT NOT NULL, status_label TEXT,"
"total_charge NUMERIC(12, 2) NOT NULL DEFAULT 0,"
"total_paid NUMERIC(12, 2) NOT NULL DEFAULT 0,"
"patient_responsibility NUMERIC(12, 2),"
"adjustment_amount NUMERIC(12, 2) NOT NULL DEFAULT 0,"
"claim_level_adjustment_amount NUMERIC(12, 2) NOT NULL DEFAULT 0,"
"received_at DATETIME NOT NULL, service_date DATE,"
"is_reversal INTEGER NOT NULL DEFAULT 0, raw_json TEXT);\n"
"CREATE TABLE matches ("
"id INTEGER PRIMARY KEY AUTOINCREMENT,"
"claim_id TEXT NOT NULL REFERENCES claims(id) ON DELETE CASCADE,"
"remittance_id TEXT NOT NULL REFERENCES remittances(id) ON DELETE CASCADE,"
"strategy TEXT NOT NULL, matched_at DATETIME NOT NULL,"
"prior_claim_state TEXT, is_reversal INTEGER NOT NULL DEFAULT 0);\n"
"CREATE TABLE cas_adjustments ("
"id INTEGER PRIMARY KEY AUTOINCREMENT,"
"remittance_id TEXT NOT NULL REFERENCES remittances(id) ON DELETE CASCADE,"
"group_code TEXT NOT NULL, reason_code TEXT NOT NULL,"
"amount NUMERIC(12, 2) NOT NULL, quantity NUMERIC(10, 2),"
"service_line_payment_id INTEGER REFERENCES service_line_payments(id) ON DELETE SET NULL);\n"
"CREATE TABLE service_line_payments ("
"id INTEGER PRIMARY KEY AUTOINCREMENT,"
"remittance_id VARCHAR(64) NOT NULL REFERENCES remittances(id) ON DELETE CASCADE,"
"line_number INTEGER NOT NULL, procedure_qualifier VARCHAR(4) NOT NULL,"
"procedure_code VARCHAR(16) NOT NULL,"
"modifiers_json TEXT NOT NULL DEFAULT '[]',"
"charge NUMERIC(12, 2) NOT NULL, payment NUMERIC(12, 2) NOT NULL,"
"units NUMERIC(10, 2), unit_type VARCHAR(8), service_date DATE,"
"ref_benefit_plan VARCHAR(64),"
"superseded_by_id INTEGER REFERENCES service_line_payments(id) ON DELETE SET NULL);\n"
"CREATE TABLE line_reconciliations ("
"id INTEGER PRIMARY KEY AUTOINCREMENT,"
"claim_id VARCHAR(64) NOT NULL REFERENCES claims(id) ON DELETE CASCADE,"
"claim_service_line_number INTEGER,"
"service_line_payment_id INTEGER REFERENCES service_line_payments(id) ON DELETE SET NULL,"
"status VARCHAR(16) NOT NULL, match_score INTEGER, reconciled_at TIMESTAMP NOT NULL);\n"
)
def _apply_0014_only(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> sa.Engine:
"""Apply just 0014 (with a synthetic 0001 that has the post-0013 column
shape) on a fresh engine. Returns the engine with user_version=14.
"""
monkeypatch.setattr(db_migrate, "MIGRATIONS_DIR", tmp_path)
(tmp_path / "0001_initial.sql").write_text(_0014_SYNTHETIC_0001)
real_migrations = Path(db_migrate.__file__).parent / "migrations"
real_0014 = real_migrations / "0014_relax_claims_remits_pk.sql"
assert real_0014.exists(), (
f"Real migration 0014 missing at {real_0014} — these tests are "
f"meant to exercise the real migration, not a synthetic one."
)
(tmp_path / "0014_relax_claims_remits_pk.sql").write_text(real_0014.read_text())
engine = _fresh_engine(tmp_path)
db_migrate.run(engine)
assert _user_version(engine) == 14, (
"0014 did not apply; user_version did not reach 14. Either the "
"migration file is missing, or it has a syntax error."
)
return engine
def test_migration_0014_relaxes_claims_pk_to_composite(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Migration 0014 changes claims PK from single-column id to (batch_id, id).
After 0014, two Claim rows with the same id can coexist if they are in
different batches. The dedup story moves to the application layer
(preflight_837 / preflight_835) instead of the schema.
"""
engine = _apply_0014_only(tmp_path, monkeypatch)
# After 0014: same id in two different batches must coexist.
with engine.begin() as c:
c.exec_driver_sql(
"INSERT INTO batches(id, kind, input_filename, parsed_at) "
"VALUES ('B1', '837p', 'b1.txt', '2026-01-01')"
)
c.exec_driver_sql(
"INSERT INTO batches(id, kind, input_filename, parsed_at) "
"VALUES ('B2', '837p', 'b2.txt', '2026-01-02')"
)
c.exec_driver_sql(
"INSERT INTO claims(id, batch_id, patient_control_number, "
"state) VALUES ('CLM-A', 'B1', 'M1', 'submitted')"
)
c.exec_driver_sql(
"INSERT INTO claims(id, batch_id, patient_control_number, "
"state) VALUES ('CLM-A', 'B2', 'M1', 'submitted')"
)
rows = c.exec_driver_sql(
"SELECT id, batch_id FROM claims WHERE id='CLM-A' ORDER BY batch_id"
).all()
assert rows == [("CLM-A", "B1"), ("CLM-A", "B2")], (
"After 0014, the same claims.id in two different batches must "
"coexist. Composite PK is (batch_id, id)."
)
def test_migration_0014_relaxes_remittances_pk_to_composite(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Same shape for remittances: same CLP01 can exist in two batches."""
engine = _apply_0014_only(tmp_path, monkeypatch)
with engine.begin() as c:
c.exec_driver_sql(
"INSERT INTO batches(id, kind, input_filename, parsed_at) "
"VALUES ('B1', '835', 'b1.txt', '2026-01-01')"
)
c.exec_driver_sql(
"INSERT INTO batches(id, kind, input_filename, parsed_at) "
"VALUES ('B2', '835', 'b2.txt', '2026-01-02')"
)
c.exec_driver_sql(
"INSERT INTO remittances(id, batch_id, payer_claim_control_number, "
"status_code, received_at) "
"VALUES ('CLP-A', 'B1', 'CLP-A', '1', '2026-01-01')"
)
c.exec_driver_sql(
"INSERT INTO remittances(id, batch_id, payer_claim_control_number, "
"status_code, received_at) "
"VALUES ('CLP-A', 'B2', 'CLP-A', '1', '2026-01-02')"
)
rows = c.exec_driver_sql(
"SELECT id, batch_id FROM remittances WHERE id='CLP-A' ORDER BY batch_id"
).all()
assert rows == [("CLP-A", "B1"), ("CLP-A", "B2")], (
"After 0014, the same remittances.id in two different batches must "
"coexist. Composite PK is (batch_id, id)."
)
def test_migration_0014_preserves_existing_data(tmp_path, monkeypatch):
"""Data inserted at v=13 must survive the v=14 table recreation.
0014 uses ``INSERT INTO claims_new SELECT * FROM claims`` (and the
remittance-side equivalent) to populate the recreated tables. Any rows
present before 0014 must still be there after 0014 finishes — that is
the contract that lets the schema change ship without data loss.
This test exercises the real 0014 against real, pre-existing rows:
1. Apply 0001-0013 via ``migrate_to_version``.
2. Seed a claim and a remittance in v=13 (raw SQL — the ORM model
declares ``matched_remittance_batch_id`` which doesn't exist at v=13).
3. Apply 0014 via ``migrate_to_version``.
4. Verify the seeded rows survive, with the same id and batch_id.
5. Verify the SAME id can be inserted in a DIFFERENT batch after 0014
(proving the composite PK took effect — a single-column PK would
have rejected the second insert).
Uses a fresh ``sa.Engine`` (not the module-global ``db.engine()``)
because the conftest autouse fixture has already initialized the
module engine at v=14 — we need a clean engine that starts at v=0
so ``migrate_to_version(13)`` actually applies 0001..0013.
"""
from sqlalchemy.orm import sessionmaker
engine = _fresh_engine(tmp_path)
SessionLocal = sessionmaker(bind=engine, autoflush=False, autocommit=False, expire_on_commit=False)
from cyclone.db_migrate import migrate_to_version
from cyclone.db import Batch, Claim
# Apply 0001-0013 only.
migrate_to_version(engine, 13)
assert _user_version(engine) == 13
# Seed a claim and a remittance in v=13. We use raw SQL here because
# the ORM Claim/Remittance models declare post-0014 columns
# (``matched_remittance_batch_id``) that don't exist at v=13. The
# migration's data-preservation contract is independent of how the
# data got there — raw SQL inserts count as "data that needs to
# survive the migration" just like ORM inserts do.
with engine.begin() as c:
c.exec_driver_sql(
"INSERT INTO batches(id, kind, input_filename, parsed_at) "
"VALUES ('B-V13', '837p', 'x.txt', '2026-01-01')"
)
c.exec_driver_sql(
"INSERT INTO claims(id, batch_id, patient_control_number, "
"state, state_changed_at) VALUES "
"('CLM-PRESERVED', 'B-V13', 'M', 'submitted', '2026-01-01')"
)
c.exec_driver_sql(
"INSERT INTO batches(id, kind, input_filename, parsed_at) "
"VALUES ('B-V13R', '835', 'x.txt', '2026-01-02')"
)
c.exec_driver_sql(
"INSERT INTO remittances(id, batch_id, payer_claim_control_number, "
"status_code, received_at) VALUES "
"('CLP-PRESERVED', 'B-V13R', 'CLP-PRESERVED', '1', '2026-01-02')"
)
# Apply 0014. Data must survive.
migrate_to_version(engine, 14)
assert _user_version(engine) == 14
with engine.connect() as c:
claim = c.exec_driver_sql(
"SELECT id, batch_id, patient_control_number "
"FROM claims WHERE id='CLM-PRESERVED'"
).first()
remit = c.exec_driver_sql(
"SELECT id, batch_id, payer_claim_control_number "
"FROM remittances WHERE id='CLP-PRESERVED'"
).first()
assert claim == ("CLM-PRESERVED", "B-V13", "M")
assert remit == ("CLP-PRESERVED", "B-V13R", "CLP-PRESERVED")
# Confirm that AFTER v=14, the same id can be inserted in a different
# batch via the ORM (now that the composite PK exists, this is the
# canonical insert path).
with SessionLocal() as s:
s.add(Batch(id="B-V14", kind="837p", input_filename="y.txt",
parsed_at=datetime(2026, 1, 3, tzinfo=timezone.utc),
raw_result_json={}))
s.add(Claim(id="CLM-PRESERVED", batch_id="B-V14",
patient_control_number="M2",
state_changed_at=datetime(2026, 1, 3, tzinfo=timezone.utc)))
s.commit()
with engine.connect() as c:
rows = c.exec_driver_sql(
"SELECT batch_id FROM claims WHERE id='CLM-PRESERVED' ORDER BY batch_id"
).all()
assert [r[0] for r in rows] == ["B-V13", "B-V14"]
def test_migration_0014_preserves_joint_fk_rows(tmp_path, monkeypatch):
"""Migration 0014 also recreates the JOIN-FK tables (matches,
cas_adjustments, service_line_payments, line_reconciliations). Each of
those tables has a composite FK to claims/remittances whose ``batch_id``
side gets populated by JOIN in the migration. Insert representative
rows at v=13 and verify they survive 0014 with the new batch_id column
populated.
This is the complement to ``test_migration_0014_preserves_existing_data``
(which covers the claims/remittances tables) and exercises the
``INSERT INTO ..._new SELECT ... JOIN parent`` statements that the
simpler test cannot.
Like the simpler test, we seed at v=13 via raw SQL — the ORM model
declares ``batch_id`` on the JOIN-FK tables but that column is only
added by 0014 itself, so an ORM insert at v=13 would fail.
"""
from sqlalchemy.orm import sessionmaker
engine = _fresh_engine(tmp_path)
SessionLocal = sessionmaker(bind=engine, autoflush=False, autocommit=False, expire_on_commit=False)
from cyclone.db_migrate import migrate_to_version
migrate_to_version(engine, 13)
# Seed parent rows + one row per JOIN-FK child table (raw SQL at v=13).
with engine.begin() as c:
c.exec_driver_sql(
"INSERT INTO batches(id, kind, input_filename, parsed_at) "
"VALUES ('B-JOIN', '837p', 'j.txt', '2026-01-01')"
)
c.exec_driver_sql(
"INSERT INTO claims(id, batch_id, patient_control_number, "
"state, state_changed_at) VALUES "
"('CLM-J', 'B-JOIN', 'M', 'submitted', '2026-01-01')"
)
c.exec_driver_sql(
"INSERT INTO batches(id, kind, input_filename, parsed_at) "
"VALUES ('B-JOIN-R', '835', 'jr.txt', '2026-01-02')"
)
c.exec_driver_sql(
"INSERT INTO remittances(id, batch_id, payer_claim_control_number, "
"status_code, received_at) VALUES "
"('CLP-J', 'B-JOIN-R', 'CLP-J', '1', '2026-01-02')"
)
# Seed the four JOIN-FK child rows. At v=13 these tables only have
# the parent-id columns (no batch_id side yet) — 0014 adds the
# batch_id columns and JOINs against the parents to populate them.
c.exec_driver_sql(
"INSERT INTO service_line_payments("
"remittance_id, line_number, procedure_qualifier, procedure_code, "
"modifiers_json, charge, payment) VALUES "
"('CLP-J', 1, 'HC', '99213', '[]', 100.00, 80.00)"
)
c.exec_driver_sql(
"INSERT INTO cas_adjustments("
"remittance_id, group_code, reason_code, amount) VALUES "
"('CLP-J', 'CO', '45', 20.00)"
)
c.exec_driver_sql(
"INSERT INTO line_reconciliations("
"claim_id, status, match_score, reconciled_at) VALUES "
"('CLM-J', 'matched', 95, '2026-01-02')"
)
c.exec_driver_sql(
"INSERT INTO matches("
"claim_id, remittance_id, strategy, matched_at) VALUES "
"('CLM-J', 'CLP-J', 'exact', '2026-01-02')"
)
# Apply 0014 — the JOIN-FK tables get the batch_id column populated by
# the migration's INSERT...SELECT...JOIN statements.
migrate_to_version(engine, 14)
with engine.connect() as c:
rows = c.exec_driver_sql(
"SELECT remittance_id, remittance_batch_id "
"FROM service_line_payments WHERE remittance_id='CLP-J'"
).all()
assert rows == [("CLP-J", "B-JOIN-R")], (
"service_line_payments.remittance_batch_id must be populated "
"by 0014's JOIN against remittances; got " + repr(rows)
)
rows = c.exec_driver_sql(
"SELECT remittance_id, remittance_batch_id "
"FROM cas_adjustments WHERE remittance_id='CLP-J'"
).all()
assert rows == [("CLP-J", "B-JOIN-R")], (
"cas_adjustments.remittance_batch_id must be populated by 0014; got "
+ repr(rows)
)
rows = c.exec_driver_sql(
"SELECT claim_id, batch_id FROM line_reconciliations WHERE claim_id='CLM-J'"
).all()
assert rows == [("CLM-J", "B-JOIN")], (
"line_reconciliations.batch_id must be populated by 0014; got "
+ repr(rows)
)
rows = c.exec_driver_sql(
"SELECT claim_id, batch_id, remittance_id, remittance_batch_id "
"FROM matches WHERE claim_id='CLM-J'"
).all()
assert rows == [("CLM-J", "B-JOIN", "CLP-J", "B-JOIN-R")], (
"matches batch_id + remittance_batch_id must be populated by 0014; "
"got " + repr(rows)
)
# =============================================================================
# migrate_to_version: edge-case behavior
# =============================================================================
# These tests use a fresh ``sa.Engine`` (not ``db.engine()``) because the
# autouse conftest fixture has already initialized the module engine at the
# latest version. We need a clean engine that starts at ``user_version=0``
# so that ``migrate_to_version(N)`` actually exercises the apply / no-op /
# beyond-max branches.
def test_migrate_to_version_idempotent(tmp_path, monkeypatch):
"""Calling migrate_to_version(engine, N) twice is equivalent to calling once."""
from cyclone.db_migrate import migrate_to_version
engine = _fresh_engine(tmp_path)
migrate_to_version(engine, 5)
with engine.begin() as conn:
v1 = conn.exec_driver_sql("PRAGMA user_version").scalar()
migrate_to_version(engine, 5)
with engine.begin() as conn:
v2 = conn.exec_driver_sql("PRAGMA user_version").scalar()
assert v1 == v2 == 5
def test_migrate_to_version_lower_than_current_is_noop(tmp_path, monkeypatch):
"""migrate_to_version(engine, 3) after migrate_to_version(engine, 13) is a no-op."""
from cyclone.db_migrate import migrate_to_version
engine = _fresh_engine(tmp_path)
migrate_to_version(engine, 13)
with engine.begin() as conn:
v_after_13 = conn.exec_driver_sql("PRAGMA user_version").scalar()
migrate_to_version(engine, 3) # lower, no-op
with engine.begin() as conn:
v_after_3 = conn.exec_driver_sql("PRAGMA user_version").scalar()
assert v_after_13 == v_after_3 == 13
def test_migrate_to_version_zero_on_fresh_db(tmp_path, monkeypatch):
"""migrate_to_version(engine, 0) on a fresh DB is a no-op."""
from cyclone.db_migrate import migrate_to_version
engine = _fresh_engine(tmp_path)
migrate_to_version(engine, 0)
with engine.begin() as conn:
v = conn.exec_driver_sql("PRAGMA user_version").scalar()
assert v == 0
def test_migrate_to_version_beyond_max_applies_all(tmp_path, monkeypatch):
"""migrate_to_version(engine, 999) applies every migration (equivalent to run)."""
import re
from cyclone.db_migrate import MIGRATIONS_DIR, migrate_to_version
engine = _fresh_engine(tmp_path)
migrate_to_version(engine, 999)
with engine.begin() as conn:
v = conn.exec_driver_sql("PRAGMA user_version").scalar()
# Should equal the highest version present in the migrations directory.
max_version = max(
int(re.match(r"^(\d+)", p.name).group(1))
for p in MIGRATIONS_DIR.glob("*.sql")
)
assert v == max_version
+31 -17
View File
@@ -72,8 +72,9 @@ def test_create_and_query_claim_with_default_state():
s.add(c)
s.commit()
# Migration 0014: composite PK (batch_id, id). PK lookups use tuples.
with db.SessionLocal()() as s:
loaded = s.get(Claim, "CLM-1")
loaded = s.get(Claim, ("b1", "CLM-1"))
assert loaded is not None
assert loaded.state == ClaimState.SUBMITTED
assert loaded.charge_amount == Decimal("124.00")
@@ -99,7 +100,7 @@ def test_claim_uses_db_default_when_state_omitted():
s.commit()
with db.SessionLocal()() as s:
loaded = s.get(Claim, "CLM-1")
loaded = s.get(Claim, ("b1", "CLM-1"))
assert loaded is not None
assert loaded.state == ClaimState.SUBMITTED
@@ -123,7 +124,7 @@ def test_state_before_reversal_round_trips():
s.commit()
with db.SessionLocal()() as s:
loaded = s.get(Claim, "CLM-1")
loaded = s.get(Claim, ("b1", "CLM-1"))
assert loaded is not None
assert loaded.state_before_reversal == ClaimState.PAID
@@ -154,7 +155,8 @@ def test_batch_cascade_delete_drops_claims():
with db.SessionLocal()() as s:
assert s.get(Batch, "b1") is None
assert s.get(Claim, "CLM-1") is None
# Composite PK lookup — both batch_id AND id are required.
assert s.get(Claim, ("b1", "CLM-1")) is None
def test_create_and_query_remittance():
@@ -178,7 +180,7 @@ def test_create_and_query_remittance():
s.commit()
with db.SessionLocal()() as s:
loaded = s.get(Remittance, "CLP-1")
loaded = s.get(Remittance, ("b1", "CLP-1"))
assert loaded is not None
assert loaded.status_code == "1"
assert loaded.total_paid == Decimal("124.00")
@@ -201,10 +203,14 @@ def test_cas_adjustment_aggregation():
)
s.add(r)
s.flush()
# Migration 0014: remittance_batch_id is required (NOT NULL) on
# CasAdjustment; it mirrors remittance_id's batch.
s.add_all([
CasAdjustment(remittance_id="CLP-1", group_code="CO", reason_code="45",
CasAdjustment(remittance_id="CLP-1", remittance_batch_id="b1",
group_code="CO", reason_code="45",
amount=Decimal("30.00")),
CasAdjustment(remittance_id="CLP-1", group_code="PR", reason_code="1",
CasAdjustment(remittance_id="CLP-1", remittance_batch_id="b1",
group_code="PR", reason_code="1",
amount=Decimal("32.00")),
])
s.commit()
@@ -235,15 +241,16 @@ def test_remittance_raw_json_round_trips_dict():
s.commit()
with db.SessionLocal()() as s:
loaded = s.get(Remittance, "CLP-1")
loaded = s.get(Remittance, ("b1", "CLP-1"))
assert loaded.raw_json == {"x12_segments": ["CLP", "NM1", "SVC"], "version": "005010X221A1"}
def test_remittance_allows_duplicate_pcn_in_same_batch():
"""An 835 ERA can carry multiple CLP segments with the same
payer_claim_control_number (e.g. reversals re-referencing the
original PCN). Remittance identity is by ``id`` (CLP01), not by
(batch_id, PCN). This test documents the absence of the constraint
original PCN). Remittance identity is by the composite PK
(batch_id, id) — id (CLP01) distinguishes rows in the same batch.
This test documents the absence of the (batch_id, PCN) constraint
removed in migration 0003.
"""
with db.SessionLocal()() as s:
@@ -286,12 +293,13 @@ def test_remittance_cascade_delete_drops_cas_adjustments():
)
s.add(r)
s.flush()
s.add(CasAdjustment(remittance_id="CLP-1", group_code="CO",
reason_code="45", amount=Decimal("30.00")))
s.add(CasAdjustment(remittance_id="CLP-1", remittance_batch_id="b1",
group_code="CO", reason_code="45",
amount=Decimal("30.00")))
s.commit()
with db.SessionLocal()() as s:
r = s.get(Remittance, "CLP-1")
r = s.get(Remittance, ("b1", "CLP-1"))
s.delete(r)
s.commit()
@@ -318,8 +326,10 @@ def test_create_match_and_activity_event():
status_code="1", total_charge=Decimal("124.00"), total_paid=Decimal("124.00"),
received_at=datetime(2026, 6, 19, tzinfo=timezone.utc),
))
# Migration 0014: Match needs batch_id + remittance_batch_id (NOT NULL).
m = Match(
claim_id="CLM-1", remittance_id="CLP-1",
claim_id="CLM-1", batch_id="b1",
remittance_id="CLP-1", remittance_batch_id="b1",
strategy="auto", is_reversal=False,
matched_at=datetime(2026, 6, 19, 13, 0, tzinfo=timezone.utc),
)
@@ -361,9 +371,13 @@ def test_match_multiple_rows_per_claim():
status_code="1", received_at=datetime.now(timezone.utc)))
s.add(Remittance(id="CLP-2", batch_id="b1", payer_claim_control_number="y",
status_code="1", received_at=datetime.now(timezone.utc)))
s.add(Match(claim_id="CLM-1", remittance_id="CLP-1", strategy="auto",
matched_at=datetime.now(timezone.utc)))
s.add(Match(claim_id="CLM-1", remittance_id="CLP-2", strategy="manual",
# Migration 0014: batch_id + remittance_batch_id required on Match.
s.add(Match(claim_id="CLM-1", batch_id="b1",
remittance_id="CLP-1", remittance_batch_id="b1",
strategy="auto", matched_at=datetime.now(timezone.utc)))
s.add(Match(claim_id="CLM-1", batch_id="b1",
remittance_id="CLP-2", remittance_batch_id="b1",
strategy="manual",
matched_at=datetime.now(timezone.utc),
is_reversal=True, prior_claim_state=ClaimState.PAID))
# No IntegrityError — two Match rows per claim are now allowed.
+10 -4
View File
@@ -75,7 +75,9 @@ def test_match_endpoint_links_remit_to_claim(client: TestClient):
assert r.status_code == 200, r.text
with db.SessionLocal()() as s:
c = s.get(Claim, "C1")
# Migration 0014: composite PK — filter-by-id returns the single
# claim with this id (no cross-batch duplicates in this fixture).
c = s.query(Claim).filter(Claim.id == "C1").first()
assert c.matched_remittance_id == "R1"
@@ -119,7 +121,8 @@ def test_resubmit_moves_rejected_to_submitted_and_increments_count(client: TestC
assert r.status_code == 200, r.text
with db.SessionLocal()() as s:
c = s.get(Claim, "C1")
# Migration 0014: composite PK (batch_id, id).
c = s.get(Claim, ("B-1", "C1"))
assert c.state == ClaimState.SUBMITTED
assert c.rejection_reason is None
assert c.resubmit_count == 3
@@ -175,7 +178,9 @@ def _seed_rejected_claim_with_raw_837(client: TestClient) -> str:
real_id = r.json()["claims"][0]["claim_id"]
_seed_batch()
with db.SessionLocal()() as s:
c = s.get(Claim, real_id)
# Migration 0014: filter-by-id since the parse-837 batch id is
# a fresh UUID we don't know here.
c = s.query(Claim).filter(Claim.id == real_id).first()
assert c is not None
c.state = ClaimState.REJECTED
c.rejection_reason = "test fixture"
@@ -216,7 +221,8 @@ def test_resubmit_with_download_returns_zip_with_one_x12_per_accepted_claim(clie
# Side effect: claim actually moved to SUBMITTED.
with db.SessionLocal()() as s:
c = s.get(Claim, real_id)
# Migration 0014: filter-by-id (batch_id is a fresh UUID).
c = s.query(Claim).filter(Claim.id == real_id).first()
assert c.state == ClaimState.SUBMITTED
+6 -3
View File
@@ -116,17 +116,20 @@ def _seed_pair_with_two_lines():
reconcile_run(s, bid_835)
s.commit()
return claim_id
return claim_id, bid_837
def test_done_today_lane_includes_matched_remittance_line_counts(client):
claim_id = _seed_pair_with_two_lines()
claim_id, bid_837 = _seed_pair_with_two_lines()
# Backfill state_changed_at so the claim lands in the done_today lane
# (reconcile.run() doesn't write it; the API surface is exercised by
# the existing manual-match flow which sets it).
from sqlalchemy.orm import Session
with db.SessionLocal()() as s:
cl = s.get(db.Claim, claim_id)
# Migration 0014: composite PK (batch_id, id). Use filter-by-id
# since we don't have a single canonical batch_id (the test seeds
# both 837 and 835 batches under fresh UUIDs).
cl = s.query(db.Claim).filter(db.Claim.id == claim_id).first()
cl.state_changed_at = datetime.now(timezone.utc)
s.commit()
+4 -2
View File
@@ -131,9 +131,11 @@ def test_done_today_includes_recent_terminal_states():
# Force C1's state_changed_at to be recent, C2 to be old.
now = datetime.now(timezone.utc)
with db.SessionLocal()() as s:
c1 = s.get(Claim, "C1")
# Migration 0014: composite PK — filter-by-id returns the single
# claim with that id (no cross-batch duplicates in this test).
c1 = s.query(Claim).filter(Claim.id == "C1").first()
c1.state_changed_at = now - timedelta(hours=2)
c2 = s.get(Claim, "C2")
c2 = s.query(Claim).filter(Claim.id == "C2").first()
c2.state_changed_at = now - timedelta(hours=30)
s.commit()
+7 -3
View File
@@ -111,7 +111,9 @@ def test_rejection_moves_submitted_claim_to_rejected_state():
assert result.matched == ["CLP-1"]
assert result.orphans == []
with db.SessionLocal()() as s:
c = s.get(Claim, "CLP-1")
# Migration 0014: composite PK — claim is in batch "B-1"; use
# filter-by-id since the test has only one claim with this id.
c = s.query(Claim).filter(Claim.id == "CLP-1").first()
assert c.state == ClaimState.REJECTED
assert c.rejected_at is not None
assert "999" in (c.rejection_reason or "")
@@ -130,7 +132,8 @@ def test_accepted_set_leaves_claim_alone():
assert result.matched == []
with db.SessionLocal()() as s:
c = s.get(Claim, "CLP-1")
# Migration 0014: composite PK — see test_rejection_moves...
c = s.query(Claim).filter(Claim.id == "CLP-1").first()
assert c.state == ClaimState.SUBMITTED
assert c.rejected_at is None
assert c.rejection_reason is None
@@ -162,5 +165,6 @@ def test_already_rejected_claim_is_idempotent():
assert result.matched == [] # no-op
with db.SessionLocal()() as s:
c = s.get(Claim, "CLP-1")
# Migration 0014: composite PK — see test_rejection_moves...
c = s.query(Claim).filter(Claim.id == "CLP-1").first()
assert c.state == ClaimState.REJECTED
@@ -67,7 +67,9 @@ def test_acknowledge_marks_unacknowledged_claim():
assert body["not_rejected"] == 0
with db.SessionLocal()() as session:
c = session.get(Claim, "C1")
# Migration 0014: composite PK — filter-by-id returns the single
# claim with this id (no cross-batch duplicates in this fixture).
c = session.query(Claim).filter(Claim.id == "C1").first()
assert c.payer_rejected_acknowledged_at is not None
assert c.payer_rejected_acknowledged_actor == "operator"
# Original fields stay intact for audit.
+27 -4
View File
@@ -271,7 +271,9 @@ def test_run_matches_and_updates_state(fixture_835):
assert result.unmatched_claims == 0
with db.SessionLocal()() as s:
claim = s.get(Claim, "CLM-1")
# Migration 0014: claims PK is composite (batch_id, id). The test
# only has one claim with id "CLM-1" so filter-by-id returns it.
claim = s.query(Claim).filter(Claim.id == "CLM-1").first()
assert claim.state == ClaimState.PAID
assert claim.matched_remittance_id == "CLP-1:b1xxxxxx"
@@ -303,13 +305,32 @@ def test_run_reversal_flips_paid_to_reversed(fixture_835):
# (9 days, outside window) which produced no match.
_make_claim(s, "CLM-1", "PCN-A", "100.00", date(2026, 6, 5),
state=ClaimState.PAID)
# Match row already exists from prior reconcile.
# Prior Match exists from an earlier reconcile — represented by a
# Match row pointing at a Remittance that already happened. Migration
# 0014: composite FK requires the parent Remittance row to exist, so
# we add a prior batch to host it (separate batch so reconcile.run
# below only sees CLP-REV in batch "b1"). The prior Match row is just
# a hint that the claim was paid earlier — we don't care about the
# prior Match's batch membership here.
_make_batch(s, batch_id="b-PRIOR", kind="835")
s.add(Remittance(
id="CLP-OLD:bPRIORxxxx", batch_id="b-PRIOR",
payer_claim_control_number="PCN-A",
status_code="1", total_charge=Decimal("100.00"),
total_paid=Decimal("100.00"),
received_at=datetime(2026, 6, 1, tzinfo=timezone.utc),
service_date=date(2026, 6, 1),
))
s.flush()
s.add(Match(
claim_id="CLM-1", remittance_id="CLP-OLD:b1xxxxxx",
claim_id="CLM-1", batch_id="b1",
remittance_id="CLP-OLD:bPRIORxxxx",
remittance_batch_id="b-PRIOR",
strategy="auto", matched_at=datetime.now(timezone.utc),
is_reversal=False,
))
s.flush()
# The reversal hits batch "b1" — the one reconcile.run() targets.
_make_remit(s, "CLP-REV", "PCN-A", "22", "100.00", "-100.00",
date(2026, 6, 10), is_reversal=True)
s.commit()
@@ -321,7 +342,9 @@ def test_run_reversal_flips_paid_to_reversed(fixture_835):
assert result.matched == 1
with db.SessionLocal()() as s:
claim = s.get(Claim, "CLM-1")
# Migration 0014: see test_run_matches_and_updates_state — use
# filter-by-id since the test has one claim with id "CLM-1".
claim = s.query(Claim).filter(Claim.id == "CLM-1").first()
assert claim.state == ClaimState.REVERSED
# Match rows: original + reversal.
matches = s.query(Match).filter(Match.claim_id == "CLM-1").all()
+56 -1
View File
@@ -200,4 +200,59 @@ def test_persistence_across_session():
s2 = CycloneStore() # fresh instance, same engine
loaded = s2.get_batch("b-x")
assert loaded is not None
assert loaded["kind"] == "837p"
assert loaded["kind"] == "837p"
def test_find_existing_batch_for_claim_returns_none_for_unknown():
"""Unknown claim_id -> None."""
from cyclone import store as store_mod # noqa: F401
assert store_mod.find_existing_batch_for_claim("nope") is None
def test_find_existing_batch_for_claim_returns_batch_id():
"""Known claim_id -> batch_id of the holding batch."""
from cyclone import db as _db
from cyclone.db import Batch, Claim
from datetime import datetime, timezone
with _db.SessionLocal()() as s:
s.add(Batch(
id="B1", kind="837p", input_filename="x.txt",
parsed_at=datetime(2026, 1, 1, tzinfo=timezone.utc),
raw_result_json={},
))
s.add(Claim(id="CLM-A", batch_id="B1", patient_control_number="M1"))
s.commit()
from cyclone import store as store_mod # noqa: F401
assert store_mod.find_existing_batch_for_claim("CLM-A") == "B1"
def test_find_existing_batch_for_remit_returns_none_for_unknown():
"""Unknown remit id -> None."""
from cyclone import store as store_mod # noqa: F401
assert store_mod.find_existing_batch_for_remit("nope") is None
def test_find_existing_batch_for_remit_returns_batch_id():
"""Known remit id -> batch_id of the holding batch."""
from cyclone import db as _db
from cyclone.db import Batch, Remittance
from datetime import datetime, timezone
with _db.SessionLocal()() as s:
s.add(Batch(
id="B2", kind="835", input_filename="y.txt",
parsed_at=datetime(2026, 1, 1, tzinfo=timezone.utc),
raw_result_json={},
))
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()
from cyclone import store as store_mod # noqa: F401
assert store_mod.find_existing_batch_for_remit("CLP-A") == "B2"
+6 -3
View File
@@ -221,7 +221,8 @@ def test_manual_match_pairs_claim_with_orphan_remit():
# Match row was persisted with strategy="manual".
with db.SessionLocal()() as session:
from cyclone.db import Claim, Match
claim_row = session.get(Claim, claim_id)
# Migration 0014: composite PK — claim ingested under batch "b-837".
claim_row = session.get(Claim, ("b-837", claim_id))
assert claim_row is not None
assert claim_row.matched_remittance_id == remit_id
match_rows = (
@@ -417,7 +418,8 @@ def test_manual_match_populates_line_reconciliation_rows():
assert unmatched.service_line_payment_id is None
# Remittance aggregates were recomputed (0 CAS in this test).
r = session.get(Remittance, remit_id)
# Migration 0014: composite PK — remit ingested under batch "b-835-sp7".
r = session.get(Remittance, ("b-835-sp7", remit_id))
assert r is not None
assert r.adjustment_amount == Decimal("0")
assert r.claim_level_adjustment_amount == Decimal("0")
@@ -583,7 +585,8 @@ def test_manual_match_idempotent_line_reconciliation():
# CLP-level CAS aggregate now reflects the inserted row.
from cyclone.db import Remittance
r = session.get(Remittance, remit_id)
# Migration 0014: composite PK — remit ingested under batch "b-835-idemp".
r = session.get(Remittance, ("b-835-idemp", remit_id))
assert r is not None
assert r.adjustment_amount == Decimal("20.00")
assert r.claim_level_adjustment_amount == Decimal("20.00")
@@ -0,0 +1,926 @@
# Drop `UNIQUE(batch_id, patient_control_number)` + 409 UX Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Allow multi-claim 837P files (where many `CLM*` segments share a `member_id`) to ingest without 409, and surface a structured error panel on the upload page when a true duplicate claim still trips a 409.
**Architecture:** Backend migration drops the inline `UNIQUE(batch_id, patient_control_number)` on `claims` via SQLite table recreation. Two new store helpers (`find_existing_batch_for_claim`, `find_existing_batch_for_remit`) enable both 837 and 835 409 handlers to surface the id of the prior batch. Frontend `ApiError` carries `existingBatchId`; `Upload.tsx` renders an inline error panel above the streaming results with a link to the existing batch and a "Pick a different file" escape hatch.
**Tech Stack:** Python 3.11+, SQLAlchemy 2.x, FastAPI, SQLite (sqlcipher3 optional), React 18 + TanStack Query + Radix UI + sonner, Vitest + React Testing Library, pytest.
**Spec:** `docs/superpowers/specs/2026-06-21-cyclone-claims-unique-constraint-and-409-ux-design.md` — read fully before starting.
**Worktree setup (one-time):**
```bash
cd /Users/openclaw/dev/cyclone
git worktree add .worktrees/claims-unique-fix -b claims-unique-fix main
cd .worktrees/claims-unique-fix
# Install backend deps if needed (venv assumed active)
pip install -e backend
# Install frontend deps if needed
npm install
```
All commits happen in this worktree. Merge to main via fast-forward when each phase ends.
---
## Phase 1 — Backend migration + helpers + API
### Task 1.1: Migration 0013 drops inline UNIQUE via table recreation
**Files:**
- Create: `backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql`
- Modify: `backend/tests/test_db_migrate.py` (append test)
The runner (`backend/src/cyclone/db_migrate.py`) wraps each `.sql` in an implicit transaction via `engine.begin()`, so the migration MUST NOT use `BEGIN`/`COMMIT` or `PRAGMA foreign_keys=OFF` (no-op inside a transaction). Use `PRAGMA defer_foreign_keys = ON` instead — checks fire at commit against the renamed table.
- [ ] **Step 1: Write the failing test**
Add to `backend/tests/test_db_migrate.py`:
```python
def test_drop_claims_unique_constraint_migration(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
"""Migration 0013 must drop the inline UNIQUE(batch_id, patient_control_number)
on claims by recreating the table. Idempotent and preserves data."""
monkeypatch.setattr(db_migrate, "MIGRATIONS_DIR", tmp_path)
# Copy the real migrations so the test starts from v1.
real_dir = Path(db_migrate.__file__).parent / "migrations"
for src in sorted(real_dir.glob("00*.sql")):
(tmp_path / src.name).write_text(src.read_text())
engine = _fresh_engine(tmp_path)
db_migrate.run(engine)
# Before 0013: insert two claims with same (batch_id, patient_control_number) raises.
with engine.begin() as c:
c.exec_driver_sql("INSERT INTO batches(id, kind, input_filename, parsed_at) VALUES ('b1', '837p', 'x.txt', '2026-01-01')")
c.exec_driver_sql("INSERT INTO claims(id, batch_id, patient_control_number) VALUES ('c1', 'b1', 'M')")
with pytest.raises(sqlalchemy.exc.IntegrityError):
c.exec_driver_sql("INSERT INTO claims(id, batch_id, patient_control_number) VALUES ('c2', 'b1', 'M')")
# Now drop the migration in by name only:
(tmp_path / "0013_drop_claims_unique_constraint.sql").write_text(
"-- version: 13\n"
"PRAGMA defer_foreign_keys = ON;\n"
"CREATE TABLE claims_new ("
"id TEXT PRIMARY KEY, batch_id TEXT NOT NULL REFERENCES batches(id) ON DELETE CASCADE,"
"patient_control_number TEXT NOT NULL, service_date_from DATE, service_date_to DATE,"
"charge_amount NUMERIC(12, 2) NOT NULL DEFAULT 0, provider_npi TEXT, payer_id TEXT,"
"state TEXT NOT NULL DEFAULT 'submitted', state_before_reversal TEXT,"
"matched_remittance_id TEXT REFERENCES remittances(id), raw_json TEXT,"
"rejection_reason TEXT, rejected_at TIMESTAMP, resubmit_count INTEGER NOT NULL DEFAULT 0,"
"state_changed_at TIMESTAMP, payer_rejected_at TEXT, payer_rejected_reason TEXT,"
"payer_rejected_status_code TEXT, payer_rejected_by_277ca_id TEXT,"
"payer_rejected_acknowledged_at TEXT, payer_rejected_acknowledged_actor TEXT);\n"
"INSERT INTO claims_new SELECT * FROM claims;"
"DROP TABLE claims;"
"ALTER TABLE claims_new RENAME TO claims;"
)
db_migrate.run(engine)
# After 0013: same insert succeeds.
with engine.begin() as c:
c.exec_driver_sql("INSERT INTO claims(id, batch_id, patient_control_number) VALUES ('c2', 'b1', 'M')")
rows = c.exec_driver_sql("SELECT COUNT(*) FROM claims").scalar()
assert rows == 2
assert _user_version(engine) == 13
def test_migration_0013_is_idempotent(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
"""Re-running db_migrate.run on a v13 DB is a no-op."""
monkeypatch.setattr(db_migrate, "MIGRATIONS_DIR", tmp_path)
real_dir = Path(db_migrate.__file__).parent / "migrations"
for src in sorted(real_dir.glob("00*.sql")):
(tmp_path / src.name).write_text(src.read_text())
engine = _fresh_engine(tmp_path)
db_migrate.run(engine) # applies all
version_after_first = _user_version(engine)
db_migrate.run(engine) # second call: no-op
assert _user_version(engine) == version_after_first
```
- [ ] **Step 2: Run test to verify it fails**
Run: `cd backend && python -m pytest tests/test_db_migrate.py::test_drop_claims_unique_constraint_migration tests/test_db_migrate.py::test_migration_0013_is_idempotent -v`
Expected: `test_drop_claims_unique_constraint_migration` FAILS because 0013 doesn't exist yet; `test_migration_0013_is_idempotent` PASSES (idempotency already works for any v).
- [ ] **Step 3: Write migration 0013**
Create `backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql`:
```sql
-- version: 13
-- Drop the inline UNIQUE(batch_id, patient_control_number) on claims.
-- Migration 0003 attempted DROP INDEX IF EXISTS uq_claims_batch_pcn but
-- the constraint is inline in CREATE TABLE, so the drop was a no-op.
-- The only way to remove an inline UNIQUE in SQLite is table recreation.
--
-- X12 837P allows any number of CLM segments per 2000B subscriber loop;
-- claim identity is provided by the primary key (claims.id = CLM01).
-- The remittances table had a parallel constraint already removed in 0003
-- (because that one WAS a named index), so this migration only touches
-- claims.
--
-- The migration runner (db_migrate.py) wraps each .sql in an implicit
-- transaction via engine.begin(), so we MUST NOT use BEGIN/COMMIT.
-- PRAGMA defer_foreign_keys defers FK checks to commit, which is the
-- only way to drop a referenced table inside a transaction in SQLite.
PRAGMA defer_foreign_keys = ON;
CREATE TABLE claims_new (
id TEXT PRIMARY KEY,
batch_id TEXT NOT NULL REFERENCES batches(id) ON DELETE CASCADE,
patient_control_number TEXT NOT NULL,
service_date_from DATE,
service_date_to DATE,
charge_amount NUMERIC(12, 2) NOT NULL DEFAULT 0,
provider_npi TEXT,
payer_id TEXT,
state TEXT NOT NULL DEFAULT 'submitted',
state_before_reversal TEXT,
matched_remittance_id TEXT REFERENCES remittances(id),
raw_json TEXT,
rejection_reason TEXT,
rejected_at TIMESTAMP,
resubmit_count INTEGER NOT NULL DEFAULT 0,
state_changed_at TIMESTAMP,
payer_rejected_at TEXT,
payer_rejected_reason TEXT,
payer_rejected_status_code TEXT,
payer_rejected_by_277ca_id TEXT,
payer_rejected_acknowledged_at TEXT,
payer_rejected_acknowledged_actor TEXT
-- NO UNIQUE (batch_id, patient_control_number) — removed.
);
INSERT INTO claims_new SELECT * FROM claims;
DROP TABLE claims;
ALTER TABLE claims_new RENAME TO claims;
-- Recreate secondary indexes (same names, same columns as initial schema
-- plus later migrations).
CREATE INDEX ix_claims_state ON claims(state);
CREATE INDEX ix_claims_patient_control_number ON claims(patient_control_number);
CREATE INDEX ix_claims_service_date_from ON claims(service_date_from);
CREATE INDEX ix_claims_state_changed_at ON claims(state, state_changed_at);
CREATE INDEX idx_claims_payer_rejected_at ON claims(payer_rejected_at);
CREATE INDEX idx_claims_payer_rejected_unack
ON claims(payer_rejected_at)
WHERE payer_rejected_acknowledged_at IS NULL;
```
- [ ] **Step 4: Run tests to verify they pass**
Run: `cd backend && python -m pytest tests/test_db_migrate.py::test_drop_claims_unique_constraint_migration tests/test_db_migrate.py::test_migration_0013_is_idempotent -v`
Expected: PASS for both. The first test asserts the inline UNIQUE is gone (insert succeeds) and `user_version==13`. The second asserts idempotency.
- [ ] **Step 5: Commit**
```bash
git add backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql backend/tests/test_db_migrate.py
git commit -m "feat(db): drop inline UNIQUE(batch_id, patient_control_number) via migration 0013"
```
---
### Task 1.2: Store helpers `find_existing_batch_for_claim` and `find_existing_batch_for_remit`
**Files:**
- Modify: `backend/src/cyclone/store.py:1-50` (imports) and add new functions near the top
- Modify: `backend/tests/test_store.py` (append tests)
The helpers are pure reads. They return the batch_id of the first batch containing the given claim_id / remit_id, or None.
- [ ] **Step 1: Write the failing test**
Add to `backend/tests/test_store.py`:
```python
def test_find_existing_batch_for_claim_returns_none_for_unknown(global_store):
"""Unknown claim_id -> None."""
assert global_store.find_existing_batch_for_claim("nope") is None # type: ignore[attr-defined]
def test_find_existing_batch_for_claim_returns_batch_id(global_store):
"""Known claim_id -> batch_id of the holding batch."""
from cyclone.store import BatchRecord, _claim_837_row # noqa: F401
from cyclone.db import Batch, Claim, db as _db_mod
from datetime import datetime, timezone
from cyclone.parser.parsers_837p import ClaimOutput # noqa: F401
# Simpler: insert a batch + claim directly via the DB.
with _db_mod.SessionLocal()() as s:
s.add(Batch(id="B1", kind="837p", input_filename="x.txt",
parsed_at=datetime(2026,1,1,tzinfo=timezone.utc)))
s.add(Claim(id="CLM-A", batch_id="B1", patient_control_number="M1"))
s.commit()
assert global_store.find_existing_batch_for_claim("CLM-A") == "B1" # type: ignore[attr-defined]
def test_find_existing_batch_for_remit_returns_none_for_unknown(global_store):
"""Unknown remit id -> None."""
assert global_store.find_existing_batch_for_remit("nope") is None # type: ignore[attr-defined]
def test_find_existing_batch_for_remit_returns_batch_id(global_store):
"""Known remit id -> batch_id of the holding batch."""
from cyclone.db import Batch, Remittance, db as _db_mod
from datetime import datetime, timezone
with _db_mod.SessionLocal()() as s:
s.add(Batch(id="B2", kind="835", input_filename="y.txt",
parsed_at=datetime(2026,1,1,tzinfo=timezone.utc)))
s.add(Remittance(id="CLP-A", batch_id="B2",
payer_claim_control_number="CLP-A",
status_code="1", received_at=datetime(2026,1,1,tzinfo=timezone.utc)))
s.commit()
assert global_store.find_existing_batch_for_remit("CLP-A") == "B2" # type: ignore[attr-defined]
```
- [ ] **Step 2: Run test to verify it fails**
Run: `cd backend && python -m pytest tests/test_store.py::test_find_existing_batch_for_claim_returns_none_for_unknown -v`
Expected: FAIL with `AttributeError: 'CycloneStore' object has no attribute 'find_existing_batch_for_claim'`.
- [ ] **Step 3: Implement helpers**
Add to `backend/src/cyclone/store.py` near the top, after imports:
```python
def find_existing_batch_for_claim(claim_id: str) -> str | None:
"""Return the batch_id of the first batch containing this claim id, or None.
Pure read; opens a short-lived session. Used by the 837 409 handler to
surface which prior batch already holds the same CLM01.
"""
from sqlalchemy import select
from cyclone import db
from cyclone.db import Claim
with db.SessionLocal()() as s:
row = s.execute(
select(Claim.batch_id).where(Claim.id == claim_id).limit(1)
).first()
return row[0] if row else None
def find_existing_batch_for_remit(remit_id: str) -> str | None:
"""Return the batch_id of the first batch containing this remit id, or None.
Pure read; opens a short-lived session. Used by the 835 409 handler.
`remit_id` is the PK on `remittances.id` (= payer_claim_control_number = CLP01).
"""
from sqlalchemy import select
from cyclone import db
from cyclone.db import Remittance
with db.SessionLocal()() as s:
row = s.execute(
select(Remittance.batch_id).where(Remittance.id == remit_id).limit(1)
).first()
return row[0] if row else None
```
- [ ] **Step 4: Run tests to verify they pass**
Run: `cd backend && python -m pytest tests/test_store.py -k "find_existing_batch" -v`
Expected: PASS for all 4 tests.
- [ ] **Step 5: Commit**
```bash
git add backend/src/cyclone/store.py backend/tests/test_store.py
git commit -m "feat(store): add find_existing_batch_for_claim and find_existing_batch_for_remit"
```
---
### Task 1.3: 409 handlers in api.py surface `existing_batch_id`
**Files:**
- Modify: `backend/src/cyclone/api.py:394-415` (837 409 handler)
- Modify: `backend/src/cyclone/api.py:588-602` (835 409 handler)
- Modify: `backend/tests/test_api_parse_persists.py` (append tests)
After migration 0013, IntegrityError on the 837 path can only fire when two claims in the same file share the same CLM01 (rare). The helper may still return a batch_id if the colliding CLM01 was previously ingested and not deleted (the dedup `s.get(Claim, claim_id)` only queries DB state, not pending session state, so cross-batch duplicates still get inserted and trip the PK).
- [ ] **Step 1: Write the failing tests**
Add to `backend/tests/test_api_parse_persists.py`:
```python
def test_409_response_includes_existing_batch_id_for_837(client: TestClient):
"""When a CLM01 already exists in a prior batch, the 409 body has existing_batch_id."""
from cyclone.db import Batch, Claim
from datetime import datetime, timezone
# Seed a prior batch with claim CLM-X.
with global_store._lock:
pass
from cyclone import db as _db
with _db.SessionLocal()() as s:
s.add(Batch(id="PRIOR", kind="837p", input_filename="prior.txt",
parsed_at=datetime(2026,1,1,tzinfo=timezone.utc),
raw_result_json={}))
s.add(Claim(id="CLM-X", batch_id="PRIOR", patient_control_number="M"))
s.commit()
# Build a file with two CLM* segments both using CLM-X (forces PK collision).
text = (
"ISA*00* *00* *ZZ*SUBMITTERID *ZZ*RECEIVERID "
"*240101*1200*^*00501*000000001*0*P*:~\n"
"GS*HC*SUBMITTERID*RECEIVERID*20240101*1200*1*X*005010X222A1~\n"
"ST*837*0001*005010X222A1~\n"
"BHT*0019*00*1*20240101*1200*CH~\n"
"NM1*41*2*SUBMITTER*****46*SUBMITTERID~\n"
"PER*IC*CONTACT*TE*5555555555~\n"
"NM1*40*2*RECEIVER*****46*RECEIVERID~\n"
"HL*1**20*1~\n"
"NM1*85*2*BILLING*****XX*1881068062~\n"
"N3*123 MAIN*\nN4*DENVER*CO*80202~\n"
"REF*EI*123456789~\n"
"HL*2*1*22*0~\n"
"SBR*P*18*******CI~\n"
"NM1*IL*1*DOE*JOHN****MI*M~\n"
"N3*456 ELM*\nN4*DENVER*CO*80202~\n"
"DMG*D8*19700101*M~\n"
"NM1*PR*2*MEDICAID*****PI*MCD~\n"
"CLM*CLM-X*100***11:B:1*Y*A*Y*Y~\n"
"LX*1~\nSV1*HC:99213*100*UN*1***1~\n"
"DTP*472*D8*20240101~\n"
"CLM*CLM-X*100***11:B:1*Y*A*Y*Y~\n"
"LX*2~\nSV1*HC:99213*100*UN*1***1~\n"
"DTP*472*D8*20240101~\n"
"SE*30*0001~\n"
"GE*1*1~\n"
"IEA*1*000000001~\n"
)
resp = client.post(
"/api/parse-837",
files={"file": ("dup.txt", text, "text/plain")},
headers={"Accept": "application/json"},
)
assert resp.status_code == 409, resp.text
body = resp.json()
assert body.get("existing_batch_id") == "PRIOR"
def test_409_response_includes_existing_batch_id_for_835(client: TestClient):
"""When a CLP01 already exists in a prior batch, the 835 409 body has existing_batch_id."""
from cyclone.db import Batch, Remittance
from datetime import datetime, timezone
from cyclone import db as _db
with _db.SessionLocal()() as s:
s.add(Batch(id="PRIOR835", kind="835", input_filename="prior.txt",
parsed_at=datetime(2026,1,1,tzinfo=timezone.utc),
raw_result_json={}))
s.add(Remittance(id="CLP-X", batch_id="PRIOR835",
payer_claim_control_number="CLP-X",
status_code="1",
received_at=datetime(2026,1,1,tzinfo=timezone.utc)))
s.commit()
# Build a minimal 835 with two CLP segments using CLP-X. This is contrived;
# we just need to trigger IntegrityError. The exact parser robustness to this
# fixture is not the focus — the test asserts the 409 body shape.
# ... or instead: directly invoke the handler via a unit-style test that
# pre-seeds and then makes a minimal 835 that includes CLP-X twice.
# For brevity, drop this test if constructing a fixture is too brittle;
# the 837 test above already exercises the same handler pattern.
pytest.skip("835 fixture with duplicate CLP01 is fragile; 837 case covers the pattern")
```
Note: the 835 test is intentionally skipped — constructing a minimal valid 835 with duplicate CLP01 is brittle. The 837 case exercises the handler pattern (call `find_existing_batch_for_remit` from a similarly-shaped except block in the 835 handler). If you need 835 coverage, manually inspect by running the API on a real fixture after the implementation lands.
- [ ] **Step 2: Run test to verify it fails**
Run: `cd backend && python -m pytest tests/test_api_parse_persists.py::test_409_response_includes_existing_batch_id_for_837 -v`
Expected: FAIL because the 409 handler does not yet add `existing_batch_id`.
- [ ] **Step 3: Modify 837 409 handler**
Edit `backend/src/cyclone/api.py` lines 394-415:
Replace the `except IntegrityError` block in `parse_837_endpoint` with:
```python
try:
store.add(rec, event_bus=request.app.state.event_bus)
except IntegrityError as exc:
# After migration 0013, the only way an IntegrityError fires here
# is a PK collision on claims.id (CLM01) — either within-file or
# against a prior batch. Look up the first claim's id and ask the
# helper which batch already holds it.
first_claim_id = (
result.claims[0].claim_id if result.claims else None
)
existing_batch_id = (
store.find_existing_batch_for_claim(first_claim_id)
if first_claim_id else None
)
body = {
"error": "Duplicate claim",
"detail": (
"This file (or one previously ingested with the same "
"claim control number) collides with an existing record. "
"Inspect the file for duplicate CLM01 control numbers, or "
"remove the existing batch before retrying."
),
"batch_id": rec.id,
}
if existing_batch_id and existing_batch_id != rec.id:
body["existing_batch_id"] = existing_batch_id
log.warning("Duplicate claim while persisting batch %s: %s", rec.id, exc)
return JSONResponse(status_code=409, content=body)
```
- [ ] **Step 4: Modify 835 409 handler**
Edit `backend/src/cyclone/api.py` lines 588-602:
Replace the `except IntegrityError` block in `parse_835_endpoint` with:
```python
try:
store.add(rec, event_bus=request.app.state.event_bus)
except IntegrityError as exc:
first_pcn = (
result.claims[0].payer_claim_control_number
if result.claims else None
)
existing_batch_id = (
store.find_existing_batch_for_remit(first_pcn)
if first_pcn else None
)
body = {
"error": "Duplicate remittance",
"detail": (
"This 835 file (or one previously ingested with the same "
"payer claim control number) collides with an existing record. "
"Remove the existing remittance before retrying."
),
"batch_id": rec.id,
}
if existing_batch_id and existing_batch_id != rec.id:
body["existing_batch_id"] = existing_batch_id
log.warning("Duplicate remittance while persisting batch %s: %s", rec.id, exc)
return JSONResponse(status_code=409, content=body)
```
- [ ] **Step 5: Run tests to verify they pass**
Run: `cd backend && python -m pytest tests/test_api_parse_persists.py -v`
Expected: All PASS, including the new 409 test.
- [ ] **Step 6: Commit**
```bash
git add backend/src/cyclone/api.py backend/tests/test_api_parse_persists.py
git commit -m "feat(api): 409 responses for 837/435 include existing_batch_id"
```
---
## Phase 2 — Frontend
### Task 2.1: `ApiError.existingBatchId` + parse837/835 surface it
**Files:**
- Modify: `src/lib/api.ts:164-167` (ApiError class)
- Modify: `src/lib/api.ts:174-191` (readErrorBody)
- Modify: `src/lib/api.ts:280-339` (parse837, parse835)
- Create: `src/lib/api.test.ts`
- [ ] **Step 1: Write the failing test**
Create `src/lib/api.test.ts`:
```typescript
import { describe, it, expect, vi, afterEach } from "vitest";
import { ApiError, parse837 } from "@/lib/api";
afterEach(() => vi.restoreAllMocks());
describe("ApiError", () => {
it("carries existingBatchId when constructed", () => {
const e = new ApiError(409, "dup", "BATCH-1");
expect(e.status).toBe(409);
expect(e.existingBatchId).toBe("BATCH-1");
});
it("defaults existingBatchId to null", () => {
const e = new ApiError(500, "boom");
expect(e.existingBatchId).toBeNull();
});
});
describe("parse837 throws ApiError with existingBatchId", () => {
it("extracts existing_batch_id from 409 body", async () => {
vi.stubGlobal("import.meta.env", { VITE_API_BASE_URL: "http://x" });
const res = new Response(
JSON.stringify({
error: "Duplicate claim",
detail: "collision",
batch_id: "NEW",
existing_batch_id: "PRIOR",
}),
{ status: 409, headers: { "Content-Type": "application/json" } },
);
vi.stubGlobal(
"fetch",
vi.fn(async () => res),
);
const file = new File(["x"], "f.txt", { type: "text/plain" });
await expect(parse837(file, { onProgress: () => {} })).rejects.toMatchObject({
status: 409,
existingBatchId: "PRIOR",
});
});
it("sets existingBatchId null when body omits it", async () => {
vi.stubGlobal("import.meta.env", { VITE_API_BASE_URL: "http://x" });
const res = new Response(
JSON.stringify({ error: "X", detail: "y", batch_id: "N" }),
{ status: 409, headers: { "Content-Type": "application/json" } },
);
vi.stubGlobal(
"fetch",
vi.fn(async () => res),
);
const file = new File(["x"], "f.txt", { type: "text/plain" });
await expect(parse837(file)).rejects.toMatchObject({
status: 409,
existingBatchId: null,
});
});
});
```
- [ ] **Step 2: Run test to verify it fails**
Run: `cd /Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix && npm test -- src/lib/api.test.ts`
Expected: FAIL — `existingBatchId` not a constructor argument yet; `parse837` throws `Error` not `ApiError`.
- [ ] **Step 3: Update `ApiError`**
Edit `src/lib/api.ts` line 164-167:
```typescript
export class ApiError extends Error {
constructor(
public status: number,
message: string,
public existingBatchId: string | null = null,
) {
super(message);
}
}
```
- [ ] **Step 4: Update `readErrorBody` to return parsed body**
Edit `src/lib/api.ts` line 174-191. Change return type and add JSON parse branch:
```typescript
type ErrorBody = {
detail?: unknown;
error?: unknown;
existing_batch_id?: unknown;
};
async function readErrorBody(
res: Response,
): Promise<{ message: string; existingBatchId: string | null }> {
try {
const t = await res.text();
if (!t) return { message: "", existingBatchId: null };
try {
const obj = JSON.parse(t) as ErrorBody;
let message = "";
if (typeof obj.detail === "string") message = obj.detail;
else if (typeof obj.error === "string") message = obj.error;
else message = t;
const existing =
typeof obj.existing_batch_id === "string"
? obj.existing_batch_id
: null;
return { message, existingBatchId: existing };
} catch {
return { message: t, existingBatchId: null };
}
} catch {
return { message: "", existingBatchId: null };
}
}
```
- [ ] **Step 5: Update parse837 to throw ApiError**
Edit `src/lib/api.ts` lines 293-298:
```typescript
if (!res.ok) {
const { message, existingBatchId } = await readErrorBody(res);
throw new ApiError(
res.status,
`${res.status} ${res.statusText}${message ? `${message}` : ""}`,
existingBatchId,
);
}
```
- [ ] **Step 6: Update parse835 to throw ApiError**
Edit `src/lib/api.ts` lines 329-334:
```typescript
if (!res.ok) {
const { message, existingBatchId } = await readErrorBody(res);
throw new ApiError(
res.status,
`${res.status} ${res.statusText}${message ? `${message}` : ""}`,
existingBatchId,
);
}
```
- [ ] **Step 7: Run tests to verify they pass**
Run: `cd .worktrees/claims-unique-fix && npm test -- src/lib/api.test.ts`
Expected: PASS for all 4 tests.
- [ ] **Step 8: Commit**
```bash
git add src/lib/api.ts src/lib/api.test.ts
git commit -m "feat(api): ApiError carries existingBatchId; parse837/parse835 surface it"
```
---
### Task 2.2: `Upload.tsx` inline error panel for 409
**Files:**
- Modify: `src/pages/Upload.tsx` (add error state + panel JSX + 409 catch branch)
- Create: `src/pages/Upload.test.tsx`
The panel renders above the streaming results when `uploadError.kind === "duplicate"`. It shows:
- A 409 badge
- "Duplicate claim — file not ingested" title
- Detail mentioning the file and (if `existingBatchId` is set) "Open the existing batch to compare, or pick a different file."
- A button linking to `/batches/{existingBatchId}` if present
- A "Pick a different file" ghost button that calls `pickFile(null)` and clears the error state
- [ ] **Step 1: Write the failing tests**
Create `src/pages/Upload.test.tsx`:
```tsx
import { describe, it, expect, vi, beforeEach } from "vitest";
import { render, screen, fireEvent } from "@testing-library/react";
import { MemoryRouter, Route, Routes } from "react-router-dom";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import * as apiModule from "@/lib/api";
import { ApiError } from "@/lib/api";
import { Upload } from "@/pages/Upload";
// We mock the api module so the component doesn't actually call fetch.
vi.mock("@/lib/api", async () => {
const actual = await vi.importActual<typeof apiModule>("@/lib/api");
return { ...actual, parse837: vi.fn(), parse835: vi.fn() };
});
beforeEach(() => {
vi.clearAllMocks();
});
function renderUpload() {
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
return render(
<QueryClientProvider client={qc}>
<MemoryRouter initialEntries={["/upload"]}>
<Routes>
<Route path="/upload" element={<Upload />} />
<Route path="/batches/:id" element={<div data-testid="batch-page" />} />
</Routes>
</MemoryRouter>
</QueryClientProvider>,
);
}
describe("Upload error panel", () => {
it("renders panel with link when 409 carries existingBatchId", async () => {
vi.mocked(apiModule.parse837).mockRejectedValueOnce(
new ApiError(409, "409 Conflict — dup", "PRIOR-BATCH"),
);
renderUpload();
// Find the file input and upload a file.
const input = document.querySelector('input[type="file"]') as HTMLInputElement;
expect(input).toBeTruthy();
const file = new File(["x"], "test.txt", { type: "text/plain" });
fireEvent.change(input, { target: { files: [file] } });
// Wait for the panel to render.
const link = await screen.findByRole("button", { name: /open existing batch/i });
expect(link).toBeInTheDocument();
expect(screen.getByText(/duplicate claim/i)).toBeInTheDocument();
});
it("renders panel without link when 409 omits existingBatchId", async () => {
vi.mocked(apiModule.parse837).mockRejectedValueOnce(
new ApiError(409, "409 Conflict — dup", null),
);
renderUpload();
const input = document.querySelector('input[type="file"]') as HTMLInputElement;
const file = new File(["x"], "test.txt", { type: "text/plain" });
fireEvent.change(input, { target: { files: [file] } });
await screen.findByText(/duplicate claim/i);
expect(
screen.queryByRole("button", { name: /open existing batch/i }),
).toBeNull();
});
it("does NOT render panel for non-409 errors", async () => {
vi.mocked(apiModule.parse837).mockRejectedValueOnce(
new ApiError(400, "bad file"),
);
renderUpload();
const input = document.querySelector('input[type="file"]') as HTMLInputElement;
const file = new File(["x"], "test.txt", { type: "text/plain" });
fireEvent.change(input, { target: { files: [file] } });
// Give the async error handler a tick.
await new Promise((r) => setTimeout(r, 50));
expect(screen.queryByText(/duplicate claim/i)).toBeNull();
});
it("'Pick a different file' button clears error", async () => {
vi.mocked(apiModule.parse837).mockRejectedValueOnce(
new ApiError(409, "409 Conflict — dup", "PRIOR"),
);
renderUpload();
const input = document.querySelector('input[type="file"]') as HTMLInputElement;
const file = new File(["x"], "test.txt", { type: "text/plain" });
fireEvent.change(input, { target: { files: [file] } });
const clearBtn = await screen.findByRole("button", { name: /pick a different file/i });
fireEvent.click(clearBtn);
expect(screen.queryByText(/duplicate claim/i)).toBeNull();
});
});
```
- [ ] **Step 2: Run test to verify it fails**
Run: `cd .worktrees/claims-unique-fix && npm test -- src/pages/Upload.test.tsx`
Expected: FAIL — no error panel exists yet, all 4 tests fail.
- [ ] **Step 3: Add error state and 409 catch branch in Upload.tsx**
Edit `src/pages/Upload.tsx`:
1. At the top imports, add:
```tsx
import { ApiError } from "@/lib/api";
import { useNavigate } from "react-router-dom";
```
2. Find the existing error-handling catch block (around line 683-687) and replace with:
```tsx
} catch (err) {
if (err instanceof ApiError && err.status === 409) {
setUploadError({
kind: "duplicate",
existingBatchId: err.existingBatchId,
filename: file.name,
});
toast.error("Duplicate claim — file not ingested");
} else {
toast.error(
err instanceof Error ? err.message : "Failed to parse file"
);
}
}
```
3. Add state declaration near the other useState calls:
```tsx
type UploadError =
| { kind: "duplicate"; existingBatchId: string | null; filename: string };
const [uploadError, setUploadError] = useState<UploadError | null>(null);
const navigate = useNavigate();
```
4. In the JSX, add the inline panel above the streaming-results section. Find the place where streaming results render (search for "streamDelay" or the section that shows the parsed batch) and insert just before it:
```tsx
{uploadError && uploadError.kind === "duplicate" ? (
<div
role="alert"
data-testid="duplicate-error-panel"
className="error-panel mx-auto max-w-3xl rounded-md border border-destructive/40 bg-destructive/5 p-4"
>
<div className="flex items-center gap-2">
<span className="inline-flex items-center rounded-md bg-destructive px-2 py-0.5 text-xs font-semibold text-destructive-foreground">
409
</span>
<span className="font-semibold">Duplicate claim file not ingested</span>
</div>
<p className="mt-2 text-sm text-muted-foreground">
<span className="font-mono">{uploadError.filename}</span> collides with an
existing record.
{uploadError.existingBatchId
? " Open the existing batch to compare, or pick a different file."
: " Pick a different file."}
</p>
<div className="mt-3 flex gap-2">
{uploadError.existingBatchId ? (
<Button
onClick={() => navigate(`/batches/${uploadError.existingBatchId}`)}
>
Open existing batch
</Button>
) : null}
<Button
variant="ghost"
onClick={() => {
setUploadError(null);
pickFile(null);
}}
>
Pick a different file
</Button>
</div>
</div>
) : null}
```
- [ ] **Step 4: Run tests to verify they pass**
Run: `cd .worktrees/claims-unique-fix && npm test -- src/pages/Upload.test.tsx`
Expected: PASS for all 4 tests.
- [ ] **Step 5: Commit**
```bash
git add src/pages/Upload.tsx src/pages/Upload.test.tsx
git commit -m "feat(upload): inline 409 error panel with existing-batch link"
```
---
## Phase 3 — End-to-end verification
### Task 3.1: Repro the original 409 with the actual multi-claim 837P file
**Files:** none (manual verification)
- [ ] **Step 1: Start backend + frontend**
In one terminal: `cd backend && uvicorn cyclone.api:app --reload --port 8000`
In another: `cd .worktrees/claims-unique-fix && npm run dev`
- [ ] **Step 2: Upload the reproducer file**
Use `docs/prodfiles/837p-from-axiscare/tp11525703-837P-20260618153339862-1of1.txt` (93696 bytes, 28 NM1*IL segments, multi-claim member-id collisions).
Expected: 200 OK + batch persisted with multiple claims having identical `member_id` (this was previously 409).
- [ ] **Step 3: Re-upload the same file to trigger 409**
Expected: 409 with `existing_batch_id` pointing at the first batch.
- [ ] **Step 4: Verify inline panel in the browser**
Open the upload page, drag the file in, verify the panel appears with the "Open existing batch →" link.
- [ ] **Step 5: Commit any tweaks**
If the panel needed any styling tweaks (e.g. spacing, colors), commit them:
```bash
git add src/pages/Upload.tsx
git commit -m "polish(upload): 409 error panel visual tweaks"
```
---
## Self-review checklist (run after writing the plan)
1. **Spec coverage** — Each section of the spec maps to a task:
- §3 Schema migration → Task 1.1
- §4 Store helpers → Task 1.2
- §5 API change → Task 1.3
- §6 Frontend `ApiError.existingBatchId` → Task 2.1
- §6 Frontend `Upload.tsx` panel → Task 2.2
- §10 Test plan (9 tests) → All 9 covered (5 backend + 4 frontend)
2. **Placeholder scan** — No "TBD", "TODO", "implement later" markers. The only intentional skip is the 835 duplicate-CLP01 test, called out explicitly.
3. **Type consistency**
- `find_existing_batch_for_claim` / `find_existing_batch_for_remit` return `str | None` everywhere.
- `ApiError.existingBatchId` is `string | null` in TS and `str | None` everywhere it's referenced.
- `setUploadError` payload shape `{ kind: "duplicate"; existingBatchId: string | null; filename: string }` is consistent in JSX and the catch branch.
4. **Risk acknowledged** — Migration reversibility, loss of uniqueness, race window are all documented in spec §9 and reflected in test scope (we don't test the race condition).
File diff suppressed because it is too large Load Diff
@@ -0,0 +1,386 @@
# Drop `UNIQUE(batch_id, patient_control_number)` on Claims + Robust 409 UX
**Date:** 2026-06-21
**Branch:** `main`
**Status:** Draft (brainstorming approved, awaiting writing-plans)
**Scope:** Backend schema + minimal frontend error handling for collision 409.
---
## 1. Why this exists
Today, every multi-claim 837P file in `docs/prodfiles/837p-from-axiscare/` and
`docs/prodfiles/FromHPE/` returns **HTTP 409** on upload. The cause:
* The `claims` table has `UNIQUE(batch_id, patient_control_number)` declared
inline in `CREATE TABLE` (auto-index `sqlite_autoindex_claims_2`).
* `store._claim_837_row` populates `patient_control_number` from
`claim.subscriber.member_id` (the subscriber's insurance member id),
not from the CLM01 segment value.
* A real 837P submission routinely contains many `CLM*` segments per
subscriber (a member seeing multiple providers on the same day). All
those rows share the same `member_id` → same `patient_control_number`.
Within one batch, the constraint fires on claim #2.
Migration `0003_drop_claims_remits_unique_constraints.sql` already exists
with the right intent but is **buggy**: it does
`DROP INDEX IF EXISTS uq_claims_batch_pcn`, but the constraint is inline,
not a named index — so the `DROP INDEX` is a no-op and the constraint
survives.
The 409 error message ("duplicate CLM01 control numbers") is also
misleading because the column stores `member_id`, not CLM01.
This SP fixes both: drops the constraint properly via SQLite table
recreation, and gives the upload page a structured error panel for the
collision case so the operator can find the existing batch in one click.
---
## 2. Operator surface
| Surface | Change |
|---|---|
| DB | New migration `0013_drop_claims_unique_constraint.sql`; idempotent. |
| Python | `store.find_existing_batch_for_claim(claim_id)` new helper. |
| API | `POST /api/parse-837` and `/api/parse-835` 409 responses gain `existing_batch_id`. |
| UI | `/upload` page renders an inline error panel for 409 with a link to the existing batch. |
| Tests | New migration test, store helper test, API test, component test. |
No CLI / settings changes.
---
## 3. Schema migration
`backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql`:
```sql
-- version: 13
-- Drop the inline UNIQUE(batch_id, patient_control_number) on claims.
-- Migration 0003 attempted DROP INDEX IF EXISTS uq_claims_batch_pcn but
-- the constraint is inline in CREATE TABLE, so the drop was a no-op.
-- The only way to remove an inline UNIQUE in SQLite is table recreation.
--
-- X12 837P allows any number of CLM segments per 2000B subscriber loop;
-- claim identity is provided by the primary key (claims.id = CLM01).
-- The remittances table had a parallel constraint already removed in 0003
-- (because that one WAS a named index), so this migration only touches
-- claims.
--
-- The migration runner (db_migrate.py) wraps each .sql file in an
-- implicit transaction via engine.begin(), so we MUST NOT use BEGIN/COMMIT
-- inside the file (nested transactions fail in SQLite). We defer FK
-- enforcement with PRAGMA defer_foreign_keys instead of turning FKs off
-- (which is a no-op inside a transaction in SQLite). The deferred
-- checks fire at commit and validate against the renamed claims table.
PRAGMA defer_foreign_keys = ON;
CREATE TABLE claims_new (
id TEXT PRIMARY KEY,
batch_id TEXT NOT NULL REFERENCES batches(id) ON DELETE CASCADE,
patient_control_number TEXT NOT NULL,
service_date_from DATE,
service_date_to DATE,
charge_amount NUMERIC(12, 2) NOT NULL DEFAULT 0,
provider_npi TEXT,
payer_id TEXT,
state TEXT NOT NULL DEFAULT 'submitted',
state_before_reversal TEXT,
matched_remittance_id TEXT REFERENCES remittances(id),
raw_json TEXT,
rejection_reason TEXT,
rejected_at TIMESTAMP,
resubmit_count INTEGER NOT NULL DEFAULT 0,
state_changed_at TIMESTAMP,
payer_rejected_at TEXT,
payer_rejected_reason TEXT,
payer_rejected_status_code TEXT,
payer_rejected_by_277ca_id TEXT,
payer_rejected_acknowledged_at TEXT,
payer_rejected_acknowledged_actor TEXT
-- NO UNIQUE (batch_id, patient_control_number) — removed.
);
INSERT INTO claims_new SELECT * FROM claims;
DROP TABLE claims;
ALTER TABLE claims_new RENAME TO claims;
-- Recreate secondary indexes (same names, same columns as initial schema
-- plus later migrations).
CREATE INDEX ix_claims_state ON claims(state);
CREATE INDEX ix_claims_patient_control_number ON claims(patient_control_number);
CREATE INDEX ix_claims_service_date_from ON claims(service_date_from);
CREATE INDEX ix_claims_state_changed_at ON claims(state, state_changed_at);
CREATE INDEX idx_claims_payer_rejected_at ON claims(payer_rejected_at);
CREATE INDEX idx_claims_payer_rejected_unack
ON claims(payer_rejected_at)
WHERE payer_rejected_acknowledged_at IS NULL;
```
**Note on the `db.py` ORM model:** `backend/src/cyclone/db.py` declares
the `Claim` ORM model with `UNIQUE(batch_id, patient_control_number)` in
`__table_args__`. After this migration the DB no longer enforces that
constraint, so the ORM declaration becomes aspirational. We leave it in
place as documentation (and as a guard if the DB ever gets rebuilt from
ORM on a fresh schema). No code reads through it; the dedup happens via
PK on `claims.id` in `store.add`.
---
## 4. Store helper
New function in `backend/src/cyclone/store.py`:
```python
def find_existing_batch_for_claim(claim_id: str) -> str | None:
"""Return the batch_id of the first batch containing this CLM01, or None."""
from cyclone import db
from cyclone.db import Claim
with db.SessionLocal()() as s:
row = s.execute(
select(Claim.batch_id).where(Claim.id == claim_id).limit(1)
).first()
return row[0] if row else None
def find_existing_batch_for_remit(payer_claim_control_number: str) -> str | None:
"""Return the batch_id of the first batch containing this CLP01, or None."""
from cyclone import db
from cyclone.db import Remittance
with db.SessionLocal()() as s:
row = s.execute(
select(Remittance.batch_id)
.where(Remittance.id == payer_claim_control_number)
.limit(1)
).first()
return row[0] if row else None
```
Both pure reads; no transaction management needed.
---
## 5. API change
`backend/src/cyclone/api.py`:
### 837 path (line 394-415)
After the UNIQUE constraint is dropped in migration 0013, an `IntegrityError`
in `store.add` can only originate from the PK on `claims.id` (CLM01) —
either two claims in the same file share CLM01 (rare) or the same CLM01
exists in a prior batch. The handler picks the first claim from
`result.claims` and asks the helper whether a prior batch already holds
that CLM01:
```python
except IntegrityError as exc:
first_claim_id = result.claims[0].claim_id if result.claims else None
existing_batch_id = (
store.find_existing_batch_for_claim(first_claim_id)
if first_claim_id else None
)
body = {
"error": "Duplicate claim",
"detail": (
"This file (or one previously ingested with the same "
"claim control number) collides with an existing record. "
"Inspect the file for duplicate CLM01 control numbers, or "
"remove the existing batch before retrying."
),
"batch_id": rec.id,
}
if existing_batch_id and existing_batch_id != rec.id:
body["existing_batch_id"] = existing_batch_id
log.warning("Duplicate claim while persisting batch %s: %s", rec.id, exc)
return JSONResponse(status_code=409, content=body)
```
The `existing_batch_id != rec.id` guard avoids surfacing the just-failed
batch as a "previous" batch (it never persisted).
### 835 path (line 588-602)
Same pattern with `find_existing_batch_for_remit` and the first remit's
`payer_claim_control_number` (`result.claims[0].payer_claim_control_number`):
```python
except IntegrityError as exc:
first_pcn = result.claims[0].payer_claim_control_number if result.claims else None
existing_batch_id = (
store.find_existing_batch_for_remit(first_pcn)
if first_pcn else None
)
body = {
"error": "Duplicate remittance",
"detail": (
"This 835 file (or one previously ingested with the same "
"payer claim control number) collides with an existing record. "
"Remove the existing remittance before retrying."
),
"batch_id": rec.id,
}
if existing_batch_id and existing_batch_id != rec.id:
body["existing_batch_id"] = existing_batch_id
log.warning("Duplicate remittance while persisting batch %s: %s", rec.id, exc)
return JSONResponse(status_code=409, content=body)
```
---
## 6. Frontend change
### `src/lib/api.ts`
`ApiError` carries an optional `existingBatchId`:
```typescript
export class ApiError extends Error {
constructor(
public status: number,
message: string,
public existingBatchId: string | null = null,
) {
super(message);
}
}
```
`readErrorBody()` parses JSON and, when status ≥ 400, attempts to extract
`existing_batch_id` and pass it through. `parse837`/`parse835` throw
`new ApiError(res.status, detail, existingBatchId)`.
### `src/pages/Upload.tsx`
New local state:
```typescript
type UploadError = {
kind: "duplicate";
existingBatchId: string | null;
filename: string;
};
const [uploadError, setUploadError] = useState<UploadError | null>(null);
```
On mutation error:
```typescript
} catch (err) {
if (err instanceof ApiError && err.status === 409) {
setUploadError({
kind: "duplicate",
existingBatchId: err.existingBatchId,
filename: file.name,
});
toast.error("Duplicate claim — file not ingested");
} else {
toast.error(err instanceof Error ? err.message : "Failed to parse file");
}
}
```
Inline error panel JSX (renders above streaming results when
`uploadError` is set):
```tsx
{uploadError ? (
<div className="error-panel">
<Badge variant="destructive">409</Badge>
<div className="error-title">Duplicate claim — file not ingested</div>
<div className="error-detail">
{filename} collides with an existing record.
{existingBatchId
? " Open the existing batch to compare, or pick a different file."
: " Pick a different file."}
</div>
<div className="error-actions">
{existingBatchId ? (
<Button onClick={() => navigate(`/batches/${existingBatchId}`)}>
Open existing batch →
</Button>
) : null}
<Button variant="ghost" onClick={() => { pickFile(null); }}>
Pick a different file
</Button>
</div>
</div>
) : null}
```
Styling matches the existing paper-toned surface (slate/parchment) per
the hybrid dark/paper treatment used elsewhere in the app.
---
## 7. Files changed
* `backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql` — new
* `backend/src/cyclone/store.py` — `find_existing_batch_for_claim`,
`find_existing_batch_for_remit`
* `backend/src/cyclone/api.py` — both 409 handlers add
`existing_batch_id` lookup
* `src/lib/api.ts` — `ApiError.existingBatchId`
* `src/pages/Upload.tsx` — error state + inline panel
* `src/pages/Upload.test.tsx` — new test file
* `backend/tests/test_api_parse_persists.py` — new test cases
* `backend/tests/test_db_migrate.py` — new migration test
* `backend/tests/test_store.py` — new helper tests
No new dependencies. No config changes.
---
## 8. Out of scope
* A "Replace existing batch" destructive action — requires
`DELETE /api/batches/{id}` and reconciliation cascade handling; deferred.
* General 4xx error UI for other statuses (empty file, parse error,
validation errors). Those already surface in toasts and the streaming
view; a future SP can generalize the inline panel pattern.
* Renaming `patient_control_number` to `subscriber_member_id` to match
what it actually stores. Out of scope for this fix; tracked separately
if it becomes a source of confusion.
---
## 9. Risk
* **Migration reversibility**: SQLite has no `DROP CONSTRAINT`; the
recreation is destructive to schema (but not to data — `INSERT INTO
claims_new SELECT * FROM claims` preserves every row). If the
migration fails mid-way, the implicit transaction (`engine.begin()`
in `db_migrate.py`) rolls back. `PRAGMA defer_foreign_keys = ON` is
required because SQLite otherwise can't drop a table that other
tables reference (`remittances.claim_id`, `matches.claim_id`,
`line_reconciliations.claim_id`, `activity_events.claim_id`). The
deferred checks fire at commit and validate against the renamed
`claims` table.
* **Loss of uniqueness**: after the migration, two claims in one batch
*can* share a `patient_control_number`. This is the intended behavior.
Claim identity is still unique via `claims.id` (CLM01, PK) and the
existing dedup check in `store.add` (`s.get(Claim, claim.claim_id)`).
* **`existing_batch_id` race**: the helper runs after the failed
`store.add` transaction has rolled back. Between rollback and helper
call, another writer could delete the colliding claim. Result: 409
fires with no `existing_batch_id`; UI shows panel without link.
Acceptable — the panel still explains the collision and offers
"Pick a different file."
---
## 10. Test plan
| Test | Asserts |
|---|---|
| `test_db_migrate.py::test_drop_claims_unique_constraint` | After running migrations from v12, `user_version=13`, no `*claims*unique*` index exists, two rows with same `(batch_id, patient_control_number)` insert cleanly. |
| `test_db_migrate.py::test_migration_idempotent` | Running migrations on a v13 DB is a no-op. |
| `test_store.py::test_find_existing_batch_for_claim` | Helper returns None for unknown, returns batch_id for known, deterministic on duplicates. |
| `test_api_parse_persists.py::test_409_response_includes_existing_batch_id` | Upload duplicate; assert body has `existing_batch_id` pointing to the right batch. |
| `test_api_parse_persists.py::test_multi_claim_batch_with_duplicate_member_id_succeeds` | Upload a file with duplicate `member_id` claims; assert 200, all claims persisted. |
| `Upload.test.tsx::test_error_panel_renders_on_409_with_link` | Mock `useParse` to throw `ApiError(409, ..., existingBatchId)`; assert panel visible, link present. |
| `Upload.test.tsx::test_error_panel_renders_on_409_without_link` | Mock 409 with `existingBatchId: null`; assert panel visible, no link. |
| `Upload.test.tsx::test_no_panel_on_non_409` | Mock 400; assert panel absent, only toast. |
| `Upload.test.tsx::test_pick_different_clears_error` | Click button; assert `pickFile(null)` called and `errorState` cleared. |
+62
View File
@@ -12,6 +12,7 @@
"@radix-ui/react-label": "^2.1.0",
"@radix-ui/react-select": "^2.1.2",
"@radix-ui/react-slot": "^1.1.0",
"@radix-ui/react-tabs": "^1.1.15",
"@tanstack/react-query": "^5.101.0",
"ansi-styles": "^6.2.3",
"class-variance-authority": "^0.7.0",
@@ -1331,6 +1332,37 @@
}
}
},
"node_modules/@radix-ui/react-roving-focus": {
"version": "1.1.13",
"resolved": "https://registry.npmjs.org/@radix-ui/react-roving-focus/-/react-roving-focus-1.1.13.tgz",
"integrity": "sha512-9gkwneI0guf8JDmrFxPjJF6Ozzgioyw+/lonYNCwefS9ZHA05er0BVHiXr+LbWGHxUfczvMY6G1oiZZi1VzjRw==",
"license": "MIT",
"dependencies": {
"@radix-ui/primitive": "1.1.4",
"@radix-ui/react-collection": "1.1.10",
"@radix-ui/react-compose-refs": "1.1.3",
"@radix-ui/react-context": "1.1.4",
"@radix-ui/react-direction": "1.1.2",
"@radix-ui/react-id": "1.1.2",
"@radix-ui/react-primitive": "2.1.6",
"@radix-ui/react-use-callback-ref": "1.1.2",
"@radix-ui/react-use-controllable-state": "1.2.3"
},
"peerDependencies": {
"@types/react": "*",
"@types/react-dom": "*",
"react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc",
"react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc"
},
"peerDependenciesMeta": {
"@types/react": {
"optional": true
},
"@types/react-dom": {
"optional": true
}
}
},
"node_modules/@radix-ui/react-select": {
"version": "2.3.1",
"resolved": "https://registry.npmjs.org/@radix-ui/react-select/-/react-select-2.3.1.tgz",
@@ -1393,6 +1425,36 @@
}
}
},
"node_modules/@radix-ui/react-tabs": {
"version": "1.1.15",
"resolved": "https://registry.npmjs.org/@radix-ui/react-tabs/-/react-tabs-1.1.15.tgz",
"integrity": "sha512-kxc9gI6/HfcU4nfMMVS3AmQK414kbU1IE6UCJmMmxjhO3cRPXOyYnmvyKD+ODt7q56nRq9l7Wovi6uaGwKgMlg==",
"license": "MIT",
"dependencies": {
"@radix-ui/primitive": "1.1.4",
"@radix-ui/react-context": "1.1.4",
"@radix-ui/react-direction": "1.1.2",
"@radix-ui/react-id": "1.1.2",
"@radix-ui/react-presence": "1.1.6",
"@radix-ui/react-primitive": "2.1.6",
"@radix-ui/react-roving-focus": "1.1.13",
"@radix-ui/react-use-controllable-state": "1.2.3"
},
"peerDependencies": {
"@types/react": "*",
"@types/react-dom": "*",
"react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc",
"react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc"
},
"peerDependenciesMeta": {
"@types/react": {
"optional": true
},
"@types/react-dom": {
"optional": true
}
}
},
"node_modules/@radix-ui/react-use-callback-ref": {
"version": "1.1.2",
"resolved": "https://registry.npmjs.org/@radix-ui/react-use-callback-ref/-/react-use-callback-ref-1.1.2.tgz",
+1
View File
@@ -17,6 +17,7 @@
"@radix-ui/react-label": "^2.1.0",
"@radix-ui/react-select": "^2.1.2",
"@radix-ui/react-slot": "^1.1.0",
"@radix-ui/react-tabs": "^1.1.15",
"@tanstack/react-query": "^5.101.0",
"ansi-styles": "^6.2.3",
"class-variance-authority": "^0.7.0",
@@ -44,6 +44,51 @@ const SAMPLE_PROVIDER: Provider = {
outstandingAr: 12450,
};
/**
* Extended provider fixture (SP21 Task 3.1) — adds the
* `recent_claims` and `recent_activity` slices the Claims/Activity
* tabs read from. Used by the tabs tests below. Field shapes match
* `ClaimSummary` and `ActivityEvent` from `@/types`.
*/
const SAMPLE_PROVIDER_WITH_DETAILS: Provider = {
...SAMPLE_PROVIDER,
recent_claims: [
{
id: "CLM-0001",
state: "submitted",
billedAmount: 250,
patientName: "Jane Q Patient",
providerNpi: "1881068062",
payerName: "Aetna",
cptCode: "99213",
submissionDate: "2026-06-20",
parsedAt: "2026-06-20",
status: "submitted",
batchId: "batch-001",
},
],
recent_activity: [
{
id: 1,
ts: "2026-06-20T15:30:00Z",
kind: "claim_submitted",
batchId: "batch-001",
claimId: "CLM-0001",
remittanceId: null,
payload: {},
},
{
id: 2,
ts: "2026-06-20T15:35:00Z",
kind: "claim_paid",
batchId: "batch-001",
claimId: "CLM-0001",
remittanceId: null,
payload: {},
},
],
};
/**
* Configure the mocked hook's return value for a single test. The
* `refetch` default is a fresh `vi.fn()` — tests that need to assert
@@ -193,4 +238,66 @@ describe("ProviderDrawer", () => {
fireEvent.click(closeBtn!);
expect(onClose).toHaveBeenCalledTimes(1);
});
// -- Tabs (SP21 Task 3.1) -------------------------------------------------
//
// Radix Tabs only mounts the active `Tabs.Content` into the DOM (no
// `forceMount`), so clicking a tab trigger causes the previous panel to
// unmount and the new one to mount. The tests below rely on that —
// "panel X renders" is asserted by checking that unique text from that
// panel's component is present in `document.body` after the click.
it("test_renders_three_tabs_overview_claims_activity", () => {
mockDetail({ data: SAMPLE_PROVIDER_WITH_DETAILS });
render(<ProviderDrawer npi="1881068062" onClose={() => {}} />);
// All three tab triggers are present, with the spec-mandated labels
// in the spec-mandated order.
const triggers = Array.from(document.querySelectorAll('[role="tab"]'));
expect(triggers.length).toBe(3);
expect(triggers.map((t) => t.textContent)).toEqual([
"Overview",
"Claims",
"Activity",
]);
// Overview is the default; its trigger must be selected on mount.
expect(triggers[0]?.getAttribute("aria-selected")).toBe("true");
expect(triggers[0]?.getAttribute("data-state")).toBe("active");
});
it("test_switches_to_claims_tab_and_shows_recent_claims", () => {
mockDetail({ data: SAMPLE_PROVIDER_WITH_DETAILS });
render(<ProviderDrawer npi="1881068062" onClose={() => {}} />);
const claimsTrigger = Array.from(document.querySelectorAll('[role="tab"]'))
.find((t) => t.textContent === "Claims");
expect(claimsTrigger).not.toBeNull();
// Radix Tabs wires `onMouseDown` (not `onClick`) to its
// `onValueChange` handler — fire the matching event so the tab
// actually activates under happy-dom.
fireEvent.mouseDown(claimsTrigger!);
// Claims tab is now selected — and ProviderRecentClaims content is in
// the DOM (claim id + patient name + the "View all claims" link).
expect(claimsTrigger?.getAttribute("aria-selected")).toBe("true");
expect(document.body.textContent).toContain("CLM-0001");
expect(document.body.textContent).toContain("Jane Q Patient");
expect(document.body.textContent).toContain("View all claims");
});
it("test_switches_to_activity_tab_and_shows_recent_activity", () => {
mockDetail({ data: SAMPLE_PROVIDER_WITH_DETAILS });
render(<ProviderDrawer npi="1881068062" onClose={() => {}} />);
const activityTrigger = Array.from(document.querySelectorAll('[role="tab"]'))
.find((t) => t.textContent === "Activity");
expect(activityTrigger).not.toBeNull();
fireEvent.mouseDown(activityTrigger!);
// Activity tab is now selected — and ProviderRecentActivity content
// is in the DOM (both `kind` strings from the fixture).
expect(activityTrigger?.getAttribute("aria-selected")).toBe("true");
expect(document.body.textContent).toContain("claim_submitted");
expect(document.body.textContent).toContain("claim_paid");
});
});
@@ -1,9 +1,12 @@
import { Dialog, DialogContent } from "@/components/ui/dialog";
import { Tabs } from "@/components/ui/tabs";
import { ApiError } from "@/lib/api";
import { DrillDrawerHeader } from "@/components/drill/DrillDrawerHeader";
import { Skeleton } from "@/components/ui/skeleton";
import { useProviderDetail } from "@/hooks/useProviderDetail";
import { ProviderOverview } from "./ProviderOverview";
import { ProviderRecentClaims } from "./ProviderRecentClaims";
import { ProviderRecentActivity } from "./ProviderRecentActivity";
import { ProviderDrawerError } from "./ProviderDrawerError";
interface Props {
@@ -12,12 +15,17 @@ interface Props {
}
/**
* Provider drill-down drawer (SP21 Task 2.2).
* Provider drill-down drawer (SP21 Task 2.2 + 3.1).
*
* Side-panel shell that consumes ``useProviderDetail(npi)`` and renders
* the Overview tab content (Phase 2 ships only this tab; Phase 3 adds
* Claims/Activity tabs once ``recent_claims`` and ``recent_activity``
* rendering lands).
* a three-tab body:
*
* - Overview — base provider fields (identity + activity shape)
* - Claims — top-10 claims joined to this provider
* (extended `/api/config/providers/{npi}.recent_claims`,
* populated by Task 1.6)
* - Activity — top-10 events joined to this provider's claims
* (extended `/api/config/providers/{npi}.recent_activity`)
*
* Layout mirrors the ClaimDrawer / RemitDrawer pattern: Radix Dialog
* repositioned to the right edge as a fixed-height side panel, with
@@ -76,9 +84,22 @@ export function ProviderDrawer({ npi, onClose }: Props) {
title={data.name}
onClose={onClose}
/>
<div className="p-6">
<ProviderOverview provider={data} />
</div>
<Tabs.Root defaultValue="overview" className="px-6 py-4">
<Tabs.List>
<Tabs.Trigger value="overview">Overview</Tabs.Trigger>
<Tabs.Trigger value="claims">Claims</Tabs.Trigger>
<Tabs.Trigger value="activity">Activity</Tabs.Trigger>
</Tabs.List>
<Tabs.Content value="overview">
<ProviderOverview provider={data} />
</Tabs.Content>
<Tabs.Content value="claims">
<ProviderRecentClaims provider={data} />
</Tabs.Content>
<Tabs.Content value="activity">
<ProviderRecentActivity provider={data} />
</Tabs.Content>
</Tabs.Root>
</div>
)}
</DialogContent>
@@ -0,0 +1,34 @@
import type { Provider } from "@/types";
/**
* Activity tab content for the ProviderDrawer (SP21 Task 3.1).
*
* STUB — renders the top-10 `recent_activity` events as a flat list
* (`ts` + `kind`). Real activity-event routing (linking each event to
* its source claim/remit/provider) arrives in Phase 4 once
* `provider_added` and `remit_received` events get their drawer
* surfaces; for now the events aren't drillable from this view —
* clicking them does nothing. The ProviderDrawer's Overview already
* surfaces the provider's `provider_added` event via the Dashboard
* activity feed (Task 2.5).
*/
export function ProviderRecentActivity({ provider }: { provider: Provider }) {
const items = provider.recent_activity ?? [];
if (items.length === 0) {
return (
<div className="text-muted-foreground text-[13px]">No recent activity.</div>
);
}
return (
<ul className="space-y-1.5 text-[12.5px]">
{items.map((a) => (
<li key={a.id} className="flex items-center gap-2">
<span className="mono text-[10.5px] text-muted-foreground">
{new Date(a.ts).toLocaleString()}
</span>
<span>{a.kind}</span>
</li>
))}
</ul>
);
}
@@ -0,0 +1,43 @@
import type { Provider } from "@/types";
import { fmt } from "@/lib/format";
/**
* Claims tab content for the ProviderDrawer (SP21 Task 3.1).
*
* Reads `provider.recent_claims` — the top-10 claims joined to this
* provider from the extended `GET /api/config/providers/{npi}` endpoint
* (Task 1.6). Renders one row per claim (`id` + `patientName` +
* `billedAmount`) with a "View all claims →" link to the global claims
* page at the bottom. Falls back to an empty-state line when the
* provider has no recent claims (e.g. legacy callers that hit the
* detail endpoint without the recent-claims slice).
*/
export function ProviderRecentClaims({ provider }: { provider: Provider }) {
const claims = provider.recent_claims ?? [];
if (claims.length === 0) {
return (
<div className="text-muted-foreground text-[13px]">No recent claims.</div>
);
}
return (
<div className="space-y-2">
{claims.map((c) => (
<div
key={c.id}
className="flex items-center gap-3 py-2 border-b border-border/30 last:border-0"
>
<div className="display mono text-[12.5px] w-32 shrink-0">{c.id}</div>
<div className="flex-1 min-w-0 text-[12.5px] text-muted-foreground truncate">
{c.patientName ?? "—"}
</div>
<div className="display mono text-[13px]">
{fmt.usd(c.billedAmount)}
</div>
</div>
))}
<a href="/claims" className="text-[12.5px] text-accent hover:underline">
View all claims
</a>
</div>
);
}
+84
View File
@@ -0,0 +1,84 @@
import * as React from "react";
import * as TabsPrimitive from "@radix-ui/react-tabs";
import { cn } from "@/lib/utils";
/**
* Tabs primitive — SP21 Universal Drill-Down (Task 3.1).
*
* Thin wrapper over `@radix-ui/react-tabs`. Mirrors the same re-export
* pattern used by `dialog.tsx`: the individual primitives are forwardRef'd
* styled components, and the `Tabs` namespace object lets callers use
* the spec's `<Tabs.Root>`, `<Tabs.List>`, `<Tabs.Trigger>`,
* `<Tabs.Content>` dot-notation directly:
*
* import { Tabs } from "@/components/ui/tabs";
* <Tabs.Root defaultValue="overview">
* <Tabs.List>
* <Tabs.Trigger value="overview">Overview</Tabs.Trigger>
* ...
* </Tabs.List>
* <Tabs.Content value="overview">...</Tabs.Content>
* </Tabs.Root>
*/
const TabsList = React.forwardRef<
React.ElementRef<typeof TabsPrimitive.List>,
React.ComponentPropsWithoutRef<typeof TabsPrimitive.List>
>(({ className, ...props }, ref) => (
<TabsPrimitive.List
ref={ref}
className={cn(
"flex gap-2 border-b border-border/30 mb-4",
className
)}
{...props}
/>
));
TabsList.displayName = TabsPrimitive.List.displayName;
const TabsTrigger = React.forwardRef<
React.ElementRef<typeof TabsPrimitive.Trigger>,
React.ComponentPropsWithoutRef<typeof TabsPrimitive.Trigger>
>(({ className, ...props }, ref) => (
<TabsPrimitive.Trigger
ref={ref}
className={cn(
"px-3 py-2 text-[12.5px] text-muted-foreground",
"data-[state=active]:text-foreground",
"data-[state=active]:border-b-2 data-[state=active]:border-accent",
"focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2",
className
)}
{...props}
/>
));
TabsTrigger.displayName = TabsPrimitive.Trigger.displayName;
const TabsContent = React.forwardRef<
React.ElementRef<typeof TabsPrimitive.Content>,
React.ComponentPropsWithoutRef<typeof TabsPrimitive.Content>
>(({ className, ...props }, ref) => (
<TabsPrimitive.Content
ref={ref}
className={cn(
"focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2",
className
)}
{...props}
/>
));
TabsContent.displayName = TabsPrimitive.Content.displayName;
/**
* Namespace export matching the spec's `import * as Tabs from ...`
* dot-notation. `Root` is the unstyled Radix primitive; the rest are the
* styled wrappers above.
*/
export const Tabs = {
Root: TabsPrimitive.Root,
List: TabsList,
Trigger: TabsTrigger,
Content: TabsContent,
};
export { TabsList, TabsTrigger, TabsContent };
+110
View File
@@ -21,6 +21,7 @@ vi.mock("@/lib/api", () => ({
api: {
isConfigured: true,
listActivity: vi.fn(),
getRemittance: vi.fn(),
},
}));
@@ -158,6 +159,14 @@ describe("ActivityLog page filters", () => {
(api.listActivity as unknown as ReturnType<typeof vi.fn>).mockResolvedValue(
EMPTY,
);
// Default for the per-remit detail fetch — the drawer fetches
// this whenever `?remit=` is in the URL or `remit_received`
// events drill in. Return a never-resolving promise so the
// drawer stays in the loading state; the smoke tests only
// assert the drawer mounts.
(
api.getRemittance as unknown as ReturnType<typeof vi.fn>
).mockReturnValue(new Promise(() => {}));
});
it("test_renders_filter_controls_when_mounted", async () => {
@@ -446,4 +455,105 @@ describe("ActivityLog page filters", () => {
unmount();
});
});
describe("SP21 Task 4.7: ActivityLog → RemitDrawer wiring", () => {
beforeEach(() => {
vi.clearAllMocks();
(api.listActivity as unknown as ReturnType<typeof vi.fn>).mockResolvedValue(
EMPTY,
);
(
api.getRemittance as unknown as ReturnType<typeof vi.fn>
).mockReturnValue(new Promise(() => {}));
});
it("clicking a remit_received event opens the RemitDrawer", async () => {
(api.listActivity as unknown as ReturnType<typeof vi.fn>).mockResolvedValue({
items: [
{
id: "A-1",
kind: "remit_received",
message: "Remit PCN-1 received",
timestamp: "2026-06-20T10:00:00Z",
remittanceId: "REM-1",
},
],
total: 1,
returned: 1,
has_more: false,
});
const { unmount } = renderActivity();
// Wait for the row to render so we can click it.
await settle(() =>
document.body.textContent?.includes("Remit PCN-1 received") ?? false,
);
// Drawer should not be mounted before the click.
expect(document.body.querySelector('[data-testid="remit-drawer"]')).toBeNull();
// Find the <li role="button"> row and click it. ActivityFeed renders
// each row as a button-role <li> when `onItemClick` is provided, with
// an aria-label like "View remit received: <message>".
const row = document.body.querySelector('[aria-label^="View remit received"]') as
| HTMLLIElement
| null;
expect(row).not.toBeNull();
await act(async () => {
row!.click();
});
// After click, the RemitDrawer should be mounted (with the skeleton,
// since the never-resolving getRemittance keeps it in loading state).
await settle(
() => document.body.querySelector('[data-testid="remit-drawer"]') !== null,
);
expect(
document.body.querySelector('[data-testid="remit-drawer"]'),
).not.toBeNull();
unmount();
});
it("deep-link ?remit=ID opens the RemitDrawer on mount", async () => {
(api.listActivity as unknown as ReturnType<typeof vi.fn>).mockResolvedValue({
items: [
{
id: "A-1",
kind: "remit_received",
message: "Remit PCN-1 received",
timestamp: "2026-06-20T10:00:00Z",
remittanceId: "REM-7",
},
],
total: 1,
returned: 1,
has_more: false,
});
// The hook reads `?remit=` from `window.location.search`, so set
// BOTH the MemoryRouter initial entry (for the page's URL display)
// AND `window.happyDOM.setURL` (for the hook).
(window as unknown as { happyDOM: { setURL: (u: string) => void } })
.happyDOM.setURL("http://localhost/activity?remit=REM-7");
const { unmount } = renderActivity({
initialEntries: ["/activity?remit=REM-7"],
});
await settle(
() => document.body.querySelector('[data-testid="remit-drawer"]') !== null,
);
expect(
document.body.querySelector('[data-testid="remit-drawer"]'),
).not.toBeNull();
unmount();
// Reset URL so any subsequent tests see a clean /activity URL.
(window as unknown as { happyDOM: { setURL: (u: string) => void } })
.happyDOM.setURL("http://localhost/activity");
});
});
+52 -2
View File
@@ -1,12 +1,16 @@
import { useCallback, useMemo } from "react";
import { useSearchParams } from "react-router-dom";
import { useSearchParams, useNavigate } from "react-router-dom";
import { toast } from "sonner";
import { useActivity } from "@/hooks/useActivity";
import { useTailStream } from "@/hooks/useTailStream";
import { useMergedTail } from "@/hooks/useMergedTail";
import { useRemitDrawerUrlState } from "@/hooks/useRemitDrawerUrlState";
import { eventKindToUrl } from "@/lib/event-routing";
import { TailStatusPill } from "@/components/TailStatusPill";
import { PageHeader } from "@/components/PageHeader";
import { ActivityFeed } from "@/components/ActivityFeed";
import { ActivityFilters, type SinceValue } from "@/components/ActivityFilters";
import { RemitDrawer } from "@/components/RemitDrawer";
import { Skeleton } from "@/components/ui/skeleton";
import { EmptyState } from "@/components/ui/empty-state";
import { ErrorState } from "@/components/ui/error-state";
@@ -35,6 +39,13 @@ function useSinceIso(since: SinceValue): string | undefined {
export function ActivityLog() {
const [searchParams, setSearchParams] = useSearchParams();
const navigate = useNavigate();
// SP21 Phase 4 Task 4.7: `remit_received` events with a
// remittanceId drill into the RemitDrawer. Calling `open(id)`
// pushes `?remit=ID` onto the current URL (no navigation away
// from `/activity`), so the activity feed stays visible behind
// the drawer and the drawer portals in over it.
const { remitId, open, close } = useRemitDrawerUrlState();
const selectedKinds = useMemo<ActivityKind[]>(
() =>
@@ -163,9 +174,48 @@ export function ActivityLog() {
}
/>
) : (
<ActivityFeed items={items} emptyMessage="No activity recorded yet." />
<ActivityFeed
items={items}
emptyMessage="No activity recorded yet."
onItemClick={(evt) => {
// SP21 Phase 4 Task 4.7: drill into the right surface
// based on event kind. `claim_*` and `provider_added`
// navigate away via `eventKindToUrl` (the Dashboard uses
// the same helper). `remit_received` events stay on
// `/activity` and open the RemitDrawer via `open(id)`
// — `eventKindToUrl` still returns `null` for that kind
// because cross-page navigation isn't the right UX here
// (we want to keep the activity feed as context behind
// the drawer). Anything else falls back to the
// "coming soon" toast so the click still gives feedback.
const url = eventKindToUrl(evt);
if (url) navigate(url);
else if (evt.kind === "remit_received" && evt.remittanceId) {
open(evt.remittanceId);
} else {
toast.info(
`Drill for ${evt.kind.replace(/_/g, " ")} coming in a later phase.`,
);
}
}}
/>
)}
</div>
{/* SP21 Phase 4 Task 4.7: RemitDrawer mount. The activity
feed's `remit_received` rows drill into the drawer via
`open()`. `remits` is empty (the activity feed doesn't
keep a flat list of remits around), so j/k is a no-op
here — closing reverts the URL via `close()`. */}
<RemitDrawer
remitId={remitId}
remits={[]}
onClose={close}
onNavigate={open}
onToggleHelp={() => {
// ActivityLog has no cheatsheet; `?` is a no-op here.
}}
/>
</div>
);
}
+44
View File
@@ -29,6 +29,7 @@ vi.mock("@/lib/api", () => ({
isConfigured: true,
listBatches: vi.fn(),
getBatchDiff: vi.fn(),
getRemittance: vi.fn(),
},
ApiError: class ApiError extends Error {
constructor(public status: number, message: string) {
@@ -180,6 +181,13 @@ function makeDiffPayload(): BatchDiffResponse {
describe("BatchDiff page", () => {
beforeEach(() => {
vi.clearAllMocks();
// Default for the per-remit detail fetch — the drawer fetches
// this whenever `?remit=` is in the URL. Return a never-resolving
// promise so the drawer stays in the loading state; the smoke
// test only asserts the drawer mounts, not the loaded data.
(
api.getRemittance as unknown as ReturnType<typeof vi.fn>
).mockReturnValue(new Promise(() => {}));
});
afterEach(() => {
@@ -189,6 +197,42 @@ describe("BatchDiff page", () => {
.happyDOM.setURL("http://localhost/batch-diff");
});
it("SP21 Task 4.5: deep-link ?remit=ID opens the RemitDrawer on mount", async () => {
// Pre-set the URL with `?remit=`. The page doesn't surface any
// per-remit rows (the diff payload is claim-level), but the
// drawer must still mount so the URL contract is honored.
//
// `useRemitDrawerUrlState` reads `window.location.search`
// (NOT React Router's search params) so we have to set the
// global window URL via happyDOM AND give MemoryRouter an
// initial entry — both stay in lockstep.
(window as unknown as { happyDOM: { setURL: (u: string) => void } })
.happyDOM.setURL("http://localhost/batch-diff?remit=REM-7");
(
api.listBatches as unknown as ReturnType<typeof vi.fn>
).mockResolvedValue([BATCH_A, BATCH_B]);
const { unmount } = renderIntoContainer(
<BatchDiff />,
["/batch-diff?remit=REM-7"],
);
// Page header must be present + drawer must be open.
await waitFor(
() => !!document.querySelector('[data-testid="batch-diff-page"]'),
"page header mounted",
);
await waitFor(
() => !!document.querySelector('[data-testid="remit-drawer"]'),
"remit drawer mounted via deep link",
);
expect(
document.querySelector('[data-testid="remit-drawer"]'),
).not.toBeNull();
unmount();
});
it("renders the awaiting-picks empty state when no batches are selected", async () => {
(api.listBatches as unknown as ReturnType<typeof vi.fn>).mockResolvedValue([
BATCH_A, BATCH_B,
+28
View File
@@ -21,8 +21,10 @@ import {
BatchDiffView,
BatchDiffViewSkeleton,
} from "@/components/BatchDiffView";
import { RemitDrawer } from "@/components/RemitDrawer";
import { useBatches } from "@/hooks/useBatches";
import { useBatchDiff } from "@/hooks/useBatchDiff";
import { useRemitDrawerUrlState } from "@/hooks/useRemitDrawerUrlState";
import type { BatchSummary as ApiBatchSummary } from "@/lib/api";
import { cn } from "@/lib/utils";
@@ -269,6 +271,14 @@ export function BatchDiff() {
// BrowserRouter (production) symmetric: both update the
// useSearchParams hook on every navigation.
const [searchParams, setSearchParams] = useSearchParams();
// SP21 Phase 4 Task 4.5: mount the RemitDrawer so a deep link to
// /batch-diff?remit=REM-1 opens the drawer in-place. The BatchDiff
// data model (BatchClaimDiffSummary) is a claim-level projection —
// there are no per-remit IDs in the diff payload to wrap, so no
// click-to-drill is wired here. The drawer still mounts so the
// `?remit=` URL contract is honored when the user lands on this
// page with the param set (e.g. from an external link).
const { remitId, open, close } = useRemitDrawerUrlState();
const { a, b } = useMemo(
() => readIdsFromParams(searchParams),
[searchParams],
@@ -835,6 +845,24 @@ export function BatchDiff() {
<span>{ready ? "A vs B" : "awaiting picks"}</span>
</div>
</div>
{/* SP21 Phase 4 Task 4.5: RemitDrawer mount. There are no
per-remit rows in the diff payload (the page is a
claim-level projection), so this drawer is for deep-link
support only — clicking a claim row does not open it.
When the user lands on /batch-diff?remit=REM-1, the drawer
opens in-place. The `remits` list is empty so j/k is a
no-op while the drawer is open. */}
<RemitDrawer
remitId={remitId}
remits={[]}
onClose={close}
onNavigate={open}
onToggleHelp={() => {
// BatchDiff has no cheatsheet surface; `?` is a no-op
// here, but the prop is required by the drawer's contract.
}}
/>
</div>
);
}
+114 -4
View File
@@ -1,10 +1,30 @@
// @vitest-environment happy-dom
import { afterEach, describe, expect, it, vi } from "vitest";
import { act, cleanup, fireEvent, render, waitFor } from "@testing-library/react";
import { MemoryRouter } from "react-router-dom";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import Inbox from "./Inbox";
import * as inboxApi from "@/lib/inbox-api";
import * as downloadModule from "@/lib/download";
// SP21 Phase 4 Task 4.4: Inbox now uses `useNavigate` (for unmatched
// claim drilldown to /claims?claim=ID), so the render harness needs
// a Router context. We use a fresh QueryClient per test so the
// drawer's per-remit query doesn't leak cache between cases, and a
// MemoryRouter so `useNavigate` has a router to push into.
function renderInbox() {
const qc = new QueryClient({
defaultOptions: { queries: { retry: false, retryDelay: 0 } },
});
return render(
<MemoryRouter initialEntries={["/inbox"]}>
<QueryClientProvider client={qc}>
<Inbox />
</QueryClientProvider>
</MemoryRouter>,
);
}
afterEach(() => {
cleanup();
vi.unstubAllGlobals();
@@ -29,7 +49,7 @@ describe("Inbox page", () => {
}),
}),
);
const { container } = render(<Inbox />);
const { container } = renderInbox();
await waitFor(() => {
expect(container.textContent).toContain("REJECTED");
expect(container.textContent).toContain("PAYER REJECTED");
@@ -65,7 +85,7 @@ describe("Inbox page", () => {
}),
}),
);
const { container } = render(<Inbox />);
const { container } = renderInbox();
await waitFor(() => {
expect(container.textContent).toContain("1");
expect(container.textContent).toContain("items need eyes");
@@ -112,7 +132,7 @@ describe("Inbox page", () => {
});
vi.stubGlobal("fetch", fetchMock);
const { container, getByTestId } = render(<Inbox />);
const { container, getByTestId } = renderInbox();
await waitFor(() => {
expect(container.textContent).toContain("PR1");
});
@@ -202,7 +222,7 @@ describe("Inbox page", () => {
.spyOn(downloadModule, "downloadBlob")
.mockImplementation(() => {});
const { container, getByText } = render(<Inbox />);
const { container, getByText } = renderInbox();
await waitFor(() => {
expect(container.textContent).toContain("C1");
expect(container.textContent).toContain("C2");
@@ -245,4 +265,94 @@ describe("Inbox page", () => {
expect(filename).toBe("resubmit-2-claims.zip");
expect(blob).toBeInstanceOf(Blob);
});
it("SP21 Task 4.4: clicking a candidate row opens the RemitDrawer", async () => {
// Candidates are remits (payer_claim_control_number-keyed) — the
// row's `id` is the remit id, so clicking drills into the
// RemitDrawer via `?remit=ID`.
vi.stubGlobal(
"fetch",
vi.fn().mockResolvedValue({
ok: true,
json: async () => ({
rejected: [],
payer_rejected: [],
candidates: [
{
id: "REM-7",
kind: "remit",
payer_claim_control_number: "REM-7",
charge_amount: 200,
payer_id: "P1",
rendering_provider_npi: "1234567890",
service_date: "2026-06-19",
candidates: [],
},
],
unmatched: [],
done_today: [],
}),
}),
);
const { container } = renderInbox();
await waitFor(() => {
expect(container.textContent).toContain("REM-7");
});
// No drawer yet — the URL is `/inbox` with no `?remit=`.
expect(
container.querySelector('[data-testid="remit-drawer"]')
).toBeNull();
// Click the candidate row. The InboxRow renders as a <tr> with
// the remit id in its first cell; clicking that row bubbles to
// the Lane's onRowClick handler we wired in Task 4.4.
const cell = Array.from(container.querySelectorAll("td")).find(
(td) => td.textContent === "REM-7",
);
const row = cell?.closest("tr");
expect(row).toBeTruthy();
await act(async () => {
fireEvent.click(row as HTMLElement);
});
// Drawer portals into document.body — check there, not container.
await waitFor(() => {
expect(
document.body.querySelector('[data-testid="remit-drawer"]'),
).not.toBeNull();
});
});
it("SP21 Task 4.4: deep-link ?remit=ID opens the drawer on mount", async () => {
// Pre-set the URL so the hook reads REM-7 off `window.location.search`
// during its `useState` initializer — no click needed. The inbox
// already mounts the drawer, so a deep link to /inbox?remit=REM-7
// should land with the drawer open.
vi.stubGlobal(
"fetch",
vi.fn().mockResolvedValue({
ok: true,
json: async () => ({
rejected: [],
payer_rejected: [],
candidates: [],
unmatched: [],
done_today: [],
}),
}),
);
(window as unknown as { happyDOM: { setURL: (u: string) => void } }).happyDOM.setURL(
"http://localhost/inbox?remit=REM-7",
);
renderInbox();
await waitFor(() => {
expect(
document.body.querySelector('[data-testid="remit-drawer"]'),
).not.toBeNull();
});
});
});
+42 -2
View File
@@ -13,10 +13,13 @@
// ---------------------------------------------------------------------------
import { useState } from "react";
import { useNavigate } from "react-router-dom";
import { cn } from "@/lib/utils";
import { Lane, type LaneRow } from "@/components/inbox/Lane";
import { InboxHeader } from "@/components/inbox/InboxHeader";
import { BulkBar } from "@/components/inbox/BulkBar";
import { RemitDrawer } from "@/components/RemitDrawer";
import { useRemitDrawerUrlState } from "@/hooks/useRemitDrawerUrlState";
import { useInboxLanes } from "@/hooks/useInboxLanes";
import {
exportInboxCsvUrl,
@@ -40,6 +43,12 @@ function rowKey(row: LaneRow): string {
export default function Inbox() {
const { lanes, loading, error, refetch } = useInboxLanes();
// SP21 Phase 4 Task 4.4: drill-down from inbox rows. The hook reads
// `?remit=` off `window.location.search` so opening a row from the
// /inbox URL pushes `?remit=ID` onto /inbox itself (it doesn't
// navigate to /remittances). The drawer just opens in-place.
const navigate = useNavigate();
const { remitId, open, close } = useRemitDrawerUrlState();
const [selected, setSelected] = useState<Record<LaneKey, string[]>>({
rejected: [],
payer_rejected: [],
@@ -218,6 +227,22 @@ export default function Inbox() {
className="min-h-screen"
style={{ background: "var(--tt-bg)", color: "var(--tt-ink)" }}
>
{/* SP21 Phase 4 Task 4.4: RemitDrawer mounts here so a row click
in the candidates / unmatched lanes drills into the parent
remit. The drawer portals into document.body (Radix Dialog),
so the surrounding dark surface is decorative — the drawer
overlays it when open. The hook reads `?remit=` off the URL,
so deep links restore the open remit on reload. */}
<RemitDrawer
remitId={remitId}
remits={[]}
onClose={close}
onNavigate={open}
onToggleHelp={() => {
// No cheatsheet on the inbox surface — `?` is a no-op
// here, but the prop is required by the drawer's contract.
}}
/>
<InboxHeader needEyesCount={needEyes} doneTodayCount={lanes.done_today.length} />
{/* Fold — a thin amber rule + italic serif annotation that
@@ -290,14 +315,29 @@ export default function Inbox() {
name="CANDIDATES"
accent="amber"
rows={lanes.candidates}
onRowClick={() => {}}
// Candidates are remits waiting for a claim match — drill
// straight into the RemitDrawer for the source remit.
onRowClick={(row) => open(row.id)}
onSelectionChange={(ids) => setLaneSelected("candidates", ids)}
/>
<Lane
name="UNMATCHED"
accent="ink-blue"
rows={lanes.unmatched}
onRowClick={() => {}}
// Unmatched is a mixed bucket (kind === "claim" | "remit" per
// the InboxClaimRow union). Defensive branch — today the
// backend only emits "claim" rows here, but the type allows
// both, and the next data-source change shouldn't require a
// page edit.
onRowClick={(row) => {
if (row.kind === "remit") {
open(row.id);
} else if (row.kind === "claim") {
navigate(
`/claims?claim=${encodeURIComponent(row.id)}`,
);
}
}}
onSelectionChange={(ids) => setLaneSelected("unmatched", ids)}
/>
<Lane
+68
View File
@@ -19,6 +19,7 @@ vi.mock("@/lib/api", () => ({
listUnmatched: vi.fn(),
matchRemit: vi.fn(),
unmatchClaim: vi.fn(),
getRemittance: vi.fn(),
},
ApiError: class ApiError extends Error {
constructor(public status: number, message: string) {
@@ -86,6 +87,73 @@ async function waitForText(
describe("ReconciliationPage", () => {
beforeEach(() => {
vi.clearAllMocks();
// Default for the per-remit detail fetch — the drawer fetches
// this whenever `?remit=` is in the URL. Return a never-resolving
// promise so the drawer stays in the loading state; the smoke
// test only asserts the drawer mounts.
(
api.getRemittance as unknown as ReturnType<typeof vi.fn>
).mockReturnValue(new Promise(() => {}));
});
it("SP21 Task 4.6: deep-link ?remit=ID opens the RemitDrawer on mount", async () => {
// Non-empty unmatched payload so the page renders the two-column
// matching surface (the "Pair them." branch). The pre-existing
// empty-state branch has a flaky happy-dom/race that's unrelated
// to Phase 4 — using non-empty data sidesteps that bug and still
// proves the deep-link → drawer mount works. We pre-set
// `window.location` (which `useRemitDrawerUrlState` reads on
// mount) so `?remit=REM-7` resolves to a truthy `remitId`.
(
api.listUnmatched as unknown as ReturnType<typeof vi.fn>
).mockResolvedValue({
claims: [
{
id: "CLM-1",
patientName: "Patient A",
billedAmount: 100,
providerNpi: "1234567890",
serviceDate: "2026-06-01",
payerId: "P1",
state: "submitted",
},
],
remittances: [
{
id: "REM-7",
payerClaimControlNumber: "PCN-A",
status: "received",
paidAmount: 100,
adjustmentAmount: 0,
receivedDate: "2026-06-01",
isReversal: false,
totalCharge: 100,
serviceDate: "2026-06-01",
batchId: "b1",
},
],
});
(window as unknown as { happyDOM: { setURL: (u: string) => void } })
.happyDOM.setURL("http://localhost/reconciliation?remit=REM-7");
const { unmount } = renderIntoContainer(
React.createElement(ReconciliationPage),
);
// Wait for the loaded two-column view (the "Pair them." headline
// is the clearest signal that listUnmatched has resolved and the
// page is past the loading + empty branches), then assert the
// drawer is mounted.
await waitForText("Pair them.");
expect(
document.body.querySelector('[data-testid="remit-drawer"]'),
).not.toBeNull();
unmount();
// Reset URL so the next test sees a clean /reconciliation URL.
(window as unknown as { happyDOM: { setURL: (u: string) => void } })
.happyDOM.setURL("http://localhost/reconciliation");
});
it("renders both columns with unmatched rows when api returns data", async () => {
+50 -5
View File
@@ -11,6 +11,9 @@ import { ApiError } from "@/lib/api";
import { Skeleton } from "@/components/ui/skeleton";
import { ErrorState } from "@/components/ui/error-state";
import { Button } from "@/components/ui/button";
import { RemitDrawer } from "@/components/RemitDrawer";
import { DrillableCell } from "@/components/drill/DrillableCell";
import { useRemitDrawerUrlState } from "@/hooks/useRemitDrawerUrlState";
import { cn } from "@/lib/utils";
/**
@@ -26,6 +29,11 @@ import { cn } from "@/lib/utils";
*/
export function ReconciliationPage() {
const { unmatched, match } = useReconciliation();
// SP21 Phase 4 Task 4.6: drill into the RemitDrawer from the
// remits column. The hook reads `?remit=` from the URL so deep
// links land with the drawer open. Selection state stays local
// — the drill is a separate gesture from the match selection.
const { remitId, open, close } = useRemitDrawerUrlState();
const [selectedClaim, setSelectedClaim] = useState<string | null>(null);
const [selectedRemit, setSelectedRemit] = useState<string | null>(null);
@@ -735,11 +743,18 @@ export function ReconciliationPage() {
{remittances.map((r) => {
const active = selectedRemit === r.id;
return (
<button
<div
key={r.id}
type="button"
onClick={() => setSelectedRemit(r.id)}
role="button"
tabIndex={0}
aria-pressed={active}
onClick={() => setSelectedRemit(r.id)}
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
setSelectedRemit(r.id);
}
}}
className={cn(
"w-full text-left p-3.5 min-h-[44px] rounded-md border transition-colors",
active
@@ -751,7 +766,21 @@ export function ReconciliationPage() {
className="mono text-[13.5px] flex items-center gap-2"
style={{ color: "hsl(var(--surface-ink))" }}
>
<span className="display">{r.payerClaimControlNumber}</span>
{/* DrillableCell wraps the PCN text clicking
the text drills into the RemitDrawer; the
surrounding div onClick (which selects the
row for the match action) is suppressed
because DrillableCell calls
e.stopPropagation() in its onClick. The
DrillableCell is its own <button>, so the
outer row had to move off <button> to
avoid invalid nested-button HTML. */}
<DrillableCell
onClick={() => open(r.id)}
ariaLabel={`View remittance ${r.payerClaimControlNumber}`}
>
<span className="display">{r.payerClaimControlNumber}</span>
</DrillableCell>
{r.isReversal ? (
<span
className="text-[10px] uppercase tracking-[0.18em] mono font-semibold"
@@ -768,7 +797,7 @@ export function ReconciliationPage() {
Status {r.status} · ${r.paidAmount.toFixed(2)} paid · $
{r.adjustmentAmount.toFixed(2)} adj
</div>
</button>
</div>
);
})}
</PairColumn>
@@ -881,6 +910,22 @@ export function ReconciliationPage() {
</span>
</div>
</div>
{/* SP21 Phase 4 Task 4.6: RemitDrawer mount. The remits column
drills into the parent remit via the PCN text (DrillableCell
+ open). The drawer portals into document.body, so the
surrounding paper plane stays put while the drawer is open.
`remits` is empty (we don't keep a list of all remits on
this page), so j/k is a no-op while the drawer is open. */}
<RemitDrawer
remitId={remitId}
remits={[]}
onClose={close}
onNavigate={open}
onToggleHelp={() => {
// Reconciliation has no cheatsheet; `?` is a no-op here.
}}
/>
</div>
);
}
+63 -14
View File
@@ -22,6 +22,11 @@ vi.mock("@/lib/api", () => ({
listRemittances: vi.fn(),
getRemittance: vi.fn(),
},
ApiError: class ApiError extends Error {
constructor(public status: number, message: string) {
super(message);
}
},
}));
// Mock the live-tail hook so the page renders the pill in the settled
@@ -128,6 +133,17 @@ function rowAt(idx: number): HTMLTableRowElement | null {
) as HTMLTableRowElement | null;
}
/**
* Point happy-dom's URL at a known value. happy-dom v20 doesn't expose a
* writable `window.location.search`, but `window.happyDOM.setURL` updates
* the URL the window reports without triggering a navigation exactly
* what we want for mounting the page at `/remittances?remit=PCN-1` etc.
* Same helper used by Claims.test.tsx and useRemitDrawerUrlState.test.ts.
*/
function setLocation(url: string): void {
(window as unknown as { happyDOM: { setURL: (u: string) => void } }).happyDOM.setURL(url);
}
/** True iff exactly one row carries `data-state="selected"`. */
function hasExactlyOneSelectedRow(): boolean {
const selected = document.querySelectorAll(
@@ -191,6 +207,11 @@ const SAMPLE_REMITS = [
describe("Remittances", () => {
beforeEach(() => {
vi.clearAllMocks();
// Reset URL to the bare remittances page between tests so a
// `?remit=` leaked from a prior test (via pushState) doesn't
// bleed into the next. happy-dom's URL survives across tests in
// the same file unless explicitly reset.
setLocation("http://localhost/remittances");
// Singleton tail-store: clear the remittances slice between tests
// so a tail-arrival case (if added later) doesn't see rows from a
// previous test.
@@ -203,18 +224,26 @@ describe("Remittances", () => {
returned: SAMPLE_REMITS.length,
has_more: false,
});
// Default for the per-remit detail fetch — the drawer fetches
// this whenever `?remit=` is in the URL. Return a never-resolving
// promise so the drawer stays in the loading state; the smoke
// tests only assert the drawer mounts, not the loaded data.
(
api.getRemittance as unknown as ReturnType<typeof vi.fn>
).mockReturnValue(new Promise(() => {}));
});
it("renders a CAS adjustment label inside the expanded detail row", async () => {
it("clicking a row opens the RemitDrawer (no more inline expand)", async () => {
// SP21 Phase 4 Task 4.3: the inline CAS expansion is gone — the
// whole row now drills into the RemitDrawer via `?remit=ID`. The
// CAS panel is now inside the drawer, not in a second <tr>.
const { unmount } = renderIntoContainer(React.createElement(Remittances));
await waitForText("PCN-1");
// The chevron + "Adjustments" header should not yet be visible because
// the row hasn't been expanded yet.
expect(document.body.textContent).not.toContain("Adjustments (2)");
// No drawer in the DOM yet — the URL has no `?remit=`.
expect(document.body.querySelector('[data-testid="remit-drawer"]')).toBeNull();
// Expand the row by clicking on the remit ID cell. We click the parent
// row by selecting the cell containing "PCN-1" and bubbling up.
// Click the row containing PCN-1.
const cell = Array.from(document.querySelectorAll("td")).find(
(td) => td.textContent === "PCN-1"
);
@@ -223,16 +252,36 @@ describe("Remittances", () => {
await act(async () => {
(row as HTMLTableRowElement).click();
});
await waitForText("Adjustments (2)");
// Both CAS labels must surface (not the raw codes alone).
expect(document.body.textContent).toContain(
"Charge exceeds fee schedule/maximum allowable"
// The drawer must mount into document.body via Radix's portal.
await settle(
() => document.body.querySelector('[data-testid="remit-drawer"]') !== null
);
expect(document.body.textContent).toContain("Deductible amount");
// Group/reason pills show the CARC code alongside.
expect(document.body.textContent).toContain("CO-45");
expect(document.body.textContent).toContain("PR-1");
expect(
document.body.querySelector('[data-testid="remit-drawer"]')
).not.toBeNull();
// URL must reflect the open remit.
expect(window.location.search).toContain("remit=PCN-1");
unmount();
});
it("deep-link ?remit=ID opens the drawer on mount", async () => {
// Pre-set the URL so the hook reads PCN-1 off `window.location.search`
// during its `useState` initializer — no click needed.
setLocation("http://localhost/remittances?remit=PCN-1");
const { unmount } = renderIntoContainer(React.createElement(Remittances));
await settle(
() => document.body.querySelector('[data-testid="remit-drawer"]') !== null
);
// Drawer should appear immediately, without user interaction.
expect(
document.body.querySelector('[data-testid="remit-drawer"]')
).not.toBeNull();
unmount();
});
+67 -125
View File
@@ -1,5 +1,4 @@
import { Fragment, useCallback, useMemo, useState } from "react";
import { ChevronDown, ChevronRight, Receipt } from "lucide-react";
import { useCallback, useMemo, useState } from "react";
import {
Table,
TableBody,
@@ -17,13 +16,15 @@ import { Pagination } from "@/components/ui/pagination";
import { KeyboardCheatsheet } from "@/components/KeyboardCheatsheet";
import { PageHeader } from "@/components/PageHeader";
import { TailStatusPill } from "@/components/TailStatusPill";
import { RemitDrawer } from "@/components/RemitDrawer";
import { useRemittances } from "@/hooks/useRemittances";
import { useRemitDrawerUrlState } from "@/hooks/useRemitDrawerUrlState";
import { useRowKeyboard } from "@/hooks/useRowKeyboard";
import { useTailStream } from "@/hooks/useTailStream";
import { useMergedTail } from "@/hooks/useMergedTail";
import { fmt } from "@/lib/format";
import { cn } from "@/lib/utils";
import type { CasAdjustment, Remittance, RemittanceStatus } from "@/types";
import type { Remittance, RemittanceStatus } from "@/types";
const PAGE_SIZE = 25;
@@ -33,39 +34,20 @@ const STATUS_OPTIONS: FilterChipOption[] = [
{ value: "reconciled", label: "Reconciled" },
];
/**
* One persisted CAS row, rendered as a "code — label" pair plus the
* dollar amount. Lives inside the expanded detail row of a remit so
* the operator can see exactly why the payer adjusted the claim.
*/
function AdjustmentRow({ adj }: { adj: CasAdjustment }) {
return (
<div className="flex items-start justify-between gap-4 py-2 border-b border-border/30 last:border-0">
<div className="min-w-0 flex-1">
<div className="mono text-[10.5px] text-muted-foreground">
{adj.group}-{adj.reason}
{adj.quantity !== null ? (
<span className="ml-2 text-muted-foreground/60">
qty {adj.quantity}
</span>
) : null}
</div>
<div className="text-[12.5px] text-foreground/90 truncate">{adj.label}</div>
</div>
<div className="display mono text-[12.5px] tabular-nums whitespace-nowrap text-muted-foreground">
{fmt.usdPrecise(adj.amount)}
</div>
</div>
);
}
export function Remittances() {
const [page, setPage] = useState(1);
const [status, setStatus] = useState<RemittanceStatus | null>(null);
const [expanded, setExpanded] = useState<Set<string>>(() => new Set());
const [selectedIndex, setSelectedIndex] = useState<number | null>(null);
const [helpOpen, setHelpOpen] = useState(false);
// SP21 Phase 4 Task 4.3: row click → RemitDrawer. The drawer is
// URL-driven (`?remit=ID`) so deep links restore the open remit
// on reload — same pattern as the ClaimDrawer on /claims.
// `remits` is the j/k navigation list (the current page of rows).
// `setRemitId` (REPLACE history, not push) is what j/k uses so a
// single keypress doesn't add a history entry.
const { remitId, open, close, setRemitId } = useRemitDrawerUrlState();
const { data, isLoading, isError, error, refetch, dataUpdatedAt } = useRemittances({
sort: "receivedDate",
order: "desc",
@@ -93,15 +75,6 @@ export function Remittances() {
{ paid: 0, adjustments: 0 }
);
const toggleExpand = (id: string) => {
setExpanded((prev) => {
const next = new Set(prev);
if (next.has(id)) next.delete(id);
else next.add(id);
return next;
});
};
const moveNext = useCallback(() => {
setSelectedIndex((i) => {
if (items.length === 0) return null;
@@ -119,19 +92,42 @@ export function Remittances() {
}, [items.length]);
useRowKeyboard({
enabled: !helpOpen && items.length > 0,
// Page-level j/k only fires when the drawer is closed — once
// `?remit=` is set, the drawer's own `useDrawerKeyboard` listener
// owns the j/k keys (with its own wrap-around semantics over
// `remits`). Letting the page-level listener stay active here
// would mean a single `j` keypress both advances the drawer's
// remittance AND bumps the page-level selectedIndex — exactly
// the "double navigation" surprise we want to avoid.
enabled: !helpOpen && items.length > 0 && remitId === null,
onNext: moveNext,
onPrev: movePrev,
onClose: () => setHelpOpen(false),
onToggleHelp: () => setHelpOpen((v) => !v),
});
// j/k navigation through the remits list. The drawer's own keyboard
// handler (useDrawerKeyboard, only attached while the drawer is
// open) uses the same keys with its own wrap-around semantics, so
// page-level nav only fires when the drawer is closed.
const drawerRemits = useMemo(
() => items.map((r) => ({ id: r.id })),
[items],
);
return (
<>
<KeyboardCheatsheet
open={helpOpen}
onClose={() => setHelpOpen(false)}
/>
<RemitDrawer
remitId={remitId}
remits={drawerRemits}
onClose={close}
onNavigate={setRemitId}
onToggleHelp={() => setHelpOpen((v) => !v)}
/>
<div className="space-y-6 lg:space-y-8 animate-fade-in">
<PageHeader
eyebrow="Remittances"
@@ -204,7 +200,6 @@ export function Remittances() {
<Table>
<TableHeader>
<TableRow>
<TableHead className="w-8" aria-label="Expand" />
<TableHead>Remit</TableHead>
<TableHead>Claim</TableHead>
<TableHead>Payer</TableHead>
@@ -216,92 +211,39 @@ export function Remittances() {
</TableHeader>
<TableBody>
{items.map((r, idx) => {
const isOpen = expanded.has(r.id);
const hasAdjustments =
!!r.adjustments && r.adjustments.length > 0;
const isSelected = selectedIndex === idx;
return (
<Fragment key={`${r.id}-${dataUpdatedAt}`}>
<TableRow
data-row-index={idx}
data-state={isSelected ? "selected" : undefined}
aria-selected={isSelected}
className={cn(
"animate-row-flash",
isSelected && [
"bg-accent/10 ring-1 ring-inset ring-accent/40 shadow-[inset_2px_0_0_0_hsl(var(--accent))]",
],
)}
onClick={() =>
hasAdjustments ? toggleExpand(r.id) : undefined
}
aria-expanded={hasAdjustments ? isOpen : undefined}
style={{ cursor: hasAdjustments ? "pointer" : undefined }}
>
<TableCell className="text-muted-foreground">
{hasAdjustments ? (
isOpen ? (
<ChevronDown
className="h-3.5 w-3.5"
strokeWidth={1.75}
aria-hidden
/>
) : (
<ChevronRight
className="h-3.5 w-3.5"
strokeWidth={1.75}
aria-hidden
/>
)
) : null}
</TableCell>
<TableCell className="display mono text-[12.5px]">{r.id}</TableCell>
<TableCell className="display mono text-[12.5px] text-muted-foreground">
{r.claimId}
</TableCell>
<TableCell>{r.payerName}</TableCell>
<TableCell className="text-right display mono">
{fmt.usdPrecise(r.paidAmount)}
</TableCell>
<TableCell className="text-right display mono text-muted-foreground">
{r.adjustmentAmount > 0 ? fmt.usdPrecise(r.adjustmentAmount) : "—"}
</TableCell>
<TableCell>
<RemitStatusBadge status={r.status} />
</TableCell>
<TableCell className="text-muted-foreground mono text-[12px]">
{fmt.dateShort(r.receivedDate)}
</TableCell>
</TableRow>
{isOpen && hasAdjustments ? (
<TableRow
key={`${r.id}-${dataUpdatedAt}-detail`}
className="bg-muted/20 hover:bg-muted/20"
>
<TableCell />
<TableCell colSpan={7} className="py-3">
<div className="flex items-center gap-2 mb-2">
<Receipt
className="h-3.5 w-3.5 text-muted-foreground"
strokeWidth={1.5}
aria-hidden
/>
<div className="eyebrow">
Adjustments ({r.adjustments!.length})
</div>
</div>
<div className="pl-5">
{r.adjustments!.map((adj, i) => (
<AdjustmentRow
key={`${adj.group}-${adj.reason}-${i}`}
adj={adj}
/>
))}
</div>
</TableCell>
</TableRow>
) : null}
</Fragment>
<TableRow
key={`${r.id}-${dataUpdatedAt}`}
data-row-index={idx}
data-state={isSelected ? "selected" : undefined}
aria-selected={isSelected}
className={cn(
"animate-row-flash cursor-pointer drillable",
isSelected && [
"bg-accent/10 ring-1 ring-inset ring-accent/40 shadow-[inset_2px_0_0_0_hsl(var(--accent))]",
],
)}
onClick={() => open(r.id)}
>
<TableCell className="display mono text-[12.5px]">{r.id}</TableCell>
<TableCell className="display mono text-[12.5px] text-muted-foreground">
{r.claimId}
</TableCell>
<TableCell>{r.payerName}</TableCell>
<TableCell className="text-right display mono">
{fmt.usdPrecise(r.paidAmount)}
</TableCell>
<TableCell className="text-right display mono text-muted-foreground">
{r.adjustmentAmount > 0 ? fmt.usdPrecise(r.adjustmentAmount) : "—"}
</TableCell>
<TableCell>
<RemitStatusBadge status={r.status} />
</TableCell>
<TableCell className="text-muted-foreground mono text-[12px]">
{fmt.dateShort(r.receivedDate)}
</TableCell>
</TableRow>
);
})}
</TableBody>