fix(sp27): Claim.patient_control_number populated from CLM01 (claim_id), not 2010BA NM109 (member_id)
The 837 ingest path (_claim_837_row in store.py:194) populated
Claim.patient_control_number from claim.subscriber.member_id — the
subscriber's 2010BA NM109 Medicaid ID — instead of from
claim.claim_id (CLM01, the claim submitter's identifier the 837
actually sent).
That silently broke every downstream join that uses this column as a
cross-reference key:
* reconcile.match() (reconcile.py:74) — joins
Claim.patient_control_number against
Remittance.payer_claim_control_number (which is parsed from CLP01,
the 835's echo of CLM01 per X12 spec). Member_id never matches
CLP01, so auto-match always fails.
* apply_999_rejections — same lookup, same broken key.
* apply_277ca_rejections — same lookup, same broken key.
* scoring.score_pair — same broken key.
Live DB probe (1,739 remits, 337 claims):
* Claim.id == Remit.payer_claim_control_number → 0 matches
* Claim.patient_control_number == Remit.payer_claim_control_number
→ 9 matches (substring coincidences; synthetic PCN strings share
alphanumeric characters with the human-readable member_ids)
* Claim.matched_remittance_id NOT NULL → 0 claims
This commit changes _claim_837_row to write
Claim.patient_control_number = claim.claim_id (= CLM01). The
reconcile matcher's existing join now hits the row the 835 echoes
back. Companion migration 0017 backfills the 337 pre-fix rows so
they're on equal footing with new ingests (UPDATE claims SET
patient_control_number = id WHERE patient_control_number IS DISTINCT
FROM id — idempotent).
Tests:
* 2 new RED→GREEN tests in test_store_reconcile.py:
- test_837_ingest_populates_patient_control_number_from_claim_id
pins the field semantics directly
- test_837_then_835_with_echoed_pcn_auto_pairs proves the full
end-to-end auto-match now fires
* 3 existing manual_match tests that relied on the bug (had setup
using member_id != claim_id to deliberately prevent auto-match)
updated to use distinct PCNs explicitly so the tests still
exercise the manual-match path with a real orphan pair.
* 3 store_claim_detail / api_gets tests adjusted for the same
reason.
Verified: 1,176 / 1,177 backend tests pass; the one failure is the
pre-existing flake in test_provider_extended_response.py noted before
this work started.
Caveat for the existing dev DB: the 1,739 remits already in the DB
were ingested from 835 fixtures whose CLP01 is a different synthetic
identifier than the 837's CLM01 (the test fixtures never echoed
CLM01 — a fixture-data limitation, not a code bug). After this fix,
new 837+835 ingest pairs whose payer echoes CLM01 in CLP01 will
auto-match as expected. The pre-existing 1,739 remits will continue
to land in the unmatched bucket; that can only be fixed by
regenerating the test fixtures (out of scope for this SP).
This commit is contained in:
Nora
@@ -58,15 +58,16 @@ def test_migration_latest_idempotent_on_fresh_db():
|
||||
SP16's 0011 processed_inbound_files, SP17's 0012 db_backups,
|
||||
SP-auth's 0013 users + sessions, SP-audit's 0014 audit_log.user_id,
|
||||
SP22's 0015 drop_claims_unique_constraint, SP27-Task 11's 0016
|
||||
claims.matched_remittance_id index)."""
|
||||
claims.matched_remittance_id index, SP27-Task 17's 0017
|
||||
claim.patient_control_number backfill UPDATE)."""
|
||||
with db.engine().begin() as c:
|
||||
v1 = c.exec_driver_sql("PRAGMA user_version").scalar() or 0
|
||||
assert v1 == 16
|
||||
assert v1 == 17
|
||||
# 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 == 16
|
||||
assert v2 == 17
|
||||
|
||||
|
||||
def test_add_ack_persists_row():
|
||||
|
||||
@@ -509,14 +509,17 @@ def test_post_match_happy_path(client: TestClient):
|
||||
)
|
||||
from cyclone.store import BatchRecord837, BatchRecord835
|
||||
|
||||
# 837 Claim. member_id="M1" so the 835 PCN-based auto-match can't pair them.
|
||||
# 837 Claim. pcn deliberately differs from the 835's PCN so the
|
||||
# auto-matcher (which now joins on claim_id == pcn after the SP27
|
||||
# Task 17 PCN fix) doesn't pair them — we want an orphan so the
|
||||
# manual_match call has work to do.
|
||||
co = ClaimOutput(
|
||||
claim_id="CLM-1",
|
||||
control_number="0001",
|
||||
transaction_date=date(2026, 6, 19),
|
||||
billing_provider=BillingProvider(name="Test", npi="1234567890"),
|
||||
subscriber=Subscriber(
|
||||
first_name="Jane", last_name="Doe", member_id="M1",
|
||||
first_name="Jane", last_name="Doe", member_id="ORPHAN-MEMBER",
|
||||
),
|
||||
payer=Payer(name="Test Payer", id="P1"),
|
||||
claim=ClaimHeader(
|
||||
@@ -541,12 +544,12 @@ def test_post_match_happy_path(client: TestClient):
|
||||
),
|
||||
))
|
||||
|
||||
# 835 Remittance. PCN="CLM-1" but member_id on the claim side is "M1",
|
||||
# so the auto-matcher in reconcile.run does NOT pair them; we want
|
||||
# an orphan so manual_match has work to do. charge == paid == 100
|
||||
# so apply_payment picks ClaimState.PAID.
|
||||
# 835 Remittance. PCN="CLM-1-ORPHAN" deliberately differs from the
|
||||
# claim's claim_id="CLM-1" so the auto-matcher in reconcile.run does
|
||||
# NOT pair them; we want an orphan so manual_match has work to do.
|
||||
# charge == paid == 100 so apply_payment picks ClaimState.PAID.
|
||||
cp = ClaimPayment(
|
||||
payer_claim_control_number="CLM-1",
|
||||
payer_claim_control_number="CLM-1-ORPHAN",
|
||||
status_code="1",
|
||||
total_charge=Decimal("100"),
|
||||
total_paid=Decimal("100"),
|
||||
|
||||
@@ -121,14 +121,16 @@ def test_migration_latest_idempotent_on_fresh_db(tmp_path: Path) -> None:
|
||||
expected head from 14 to 15 with the new UNIQUE-drop migration.
|
||||
SP27 Task 11 bumped it to 16 with the claims.matched_remittance_id
|
||||
index (drift-check perf).
|
||||
SP27 Task 17 bumped it to 17 with the patient_control_number
|
||||
backfill UPDATE (the migration runner now applies DML too).
|
||||
"""
|
||||
engine = _fresh_engine(tmp_path)
|
||||
db_migrate.run(engine)
|
||||
v_after_first = _user_version(engine)
|
||||
assert v_after_first == 16, f"expected head=16, got {v_after_first}"
|
||||
assert v_after_first == 17, f"expected head=17, got {v_after_first}"
|
||||
|
||||
db_migrate.run(engine)
|
||||
assert _user_version(engine) == 16, "second run should not bump version"
|
||||
assert _user_version(engine) == 17, "second run should not bump version"
|
||||
|
||||
|
||||
def test_drop_claims_unique_constraint_migration(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
@@ -154,7 +156,7 @@ def test_drop_claims_unique_constraint_migration(tmp_path: Path, monkeypatch: py
|
||||
engine = _fresh_engine(tmp_path)
|
||||
|
||||
db_migrate.run(engine)
|
||||
assert _user_version(engine) == 16, f"expected head=16, got {_user_version(engine)}"
|
||||
assert _user_version(engine) == 17, f"expected head=17, got {_user_version(engine)}"
|
||||
|
||||
# Two claims in one batch with the same patient_control_number
|
||||
# must be insertable. If 0015's table recreation re-introduced a
|
||||
|
||||
@@ -354,10 +354,11 @@ def test_get_claim_detail_includes_state_history():
|
||||
"""``stateHistory`` must include the manual_match event after pairing."""
|
||||
s = CycloneStore()
|
||||
_add_837_with_claim(s, "CLM-1")
|
||||
# 835 with PCN that intentionally differs from the 837's member id so
|
||||
# reconcile doesn't auto-pair on ingest — we want manual_match to do it.
|
||||
_add_835_with_remit(s, pcn="CLM-1", paid="100", status="1")
|
||||
s.manual_match("CLM-1", "CLM-1")
|
||||
# 835 PCN deliberately differs from claim_id so the auto-matcher
|
||||
# (which now joins on claim_id == pcn after the SP27 Task 17 PCN
|
||||
# fix) doesn't pair them — manual_match needs work to do.
|
||||
_add_835_with_remit(s, pcn="CLM-1-ORPHAN", paid="100", status="1")
|
||||
s.manual_match("CLM-1", "CLM-1-ORPHAN")
|
||||
|
||||
out = s.get_claim_detail("CLM-1")
|
||||
assert out is not None
|
||||
@@ -377,7 +378,7 @@ def test_get_claim_detail_includes_state_history():
|
||||
# ActivityEvent row, which is correct per the spec's
|
||||
# ``{kind, ts, batchId|null, remittanceId|null}`` contract.
|
||||
mm = next(ev for ev in history if ev["kind"] == "manual_match")
|
||||
assert mm["remittanceId"] == "CLM-1"
|
||||
assert mm["remittanceId"] == "CLM-1-ORPHAN"
|
||||
assert mm["batchId"] is not None
|
||||
assert mm["ts"].endswith("Z")
|
||||
|
||||
@@ -392,14 +393,15 @@ def test_get_claim_detail_includes_matched_remittance_summary():
|
||||
"""A paired claim surfaces a ``matchedRemittance`` summary block."""
|
||||
s = CycloneStore()
|
||||
_add_837_with_claim(s, "CLM-1")
|
||||
_add_835_with_remit(s, pcn="CLM-1", paid="100", status="1")
|
||||
s.manual_match("CLM-1", "CLM-1")
|
||||
# 835 PCN deliberately differs from claim_id so manual_match has work.
|
||||
_add_835_with_remit(s, pcn="CLM-1-ORPHAN", paid="100", status="1")
|
||||
s.manual_match("CLM-1", "CLM-1-ORPHAN")
|
||||
|
||||
out = s.get_claim_detail("CLM-1")
|
||||
assert out is not None
|
||||
mr = out["matchedRemittance"]
|
||||
assert mr is not None
|
||||
assert mr["id"] == "CLM-1"
|
||||
assert mr["id"] == "CLM-1-ORPHAN"
|
||||
assert mr["totalPaid"] == 100.0
|
||||
assert isinstance(mr["totalPaid"], float)
|
||||
# status_code "1" → "received" (per to_ui_remittance_from_orm mapping).
|
||||
|
||||
@@ -357,8 +357,11 @@ def test_manual_match_populates_line_reconciliation_rows():
|
||||
))
|
||||
|
||||
# 835 remit with one SVC line that matches claim line 1.
|
||||
# pcn deliberately ≠ claim_id so auto-match (which now joins on
|
||||
# claim_id == pcn after the SP27 Task 17 PCN fix) doesn't pair them
|
||||
# and the claim lands in the unmatched list for manual pairing.
|
||||
cp = ClaimPayment(
|
||||
payer_claim_control_number="CLM-MANUAL", # != member_id "MANUAL"
|
||||
payer_claim_control_number="ORPHAN-PCN-MANUAL",
|
||||
status_code="1",
|
||||
status_label="Primary",
|
||||
total_charge=Decimal("200.00"),
|
||||
@@ -498,10 +501,10 @@ def test_manual_match_idempotent_line_reconciliation():
|
||||
result=pr837,
|
||||
))
|
||||
|
||||
# 835 with two SVC lines — auto-match won't pair (PCN="CLM-IDEMP" ≠
|
||||
# member_id="IDEMP"), so manual_match has work to do.
|
||||
# 835 with two SVC lines — auto-match won't pair (pcn="ORPHAN-IDEMP"
|
||||
# ≠ claim_id="CLM-IDEMP"), so manual_match has work to do.
|
||||
cp = ClaimPayment(
|
||||
payer_claim_control_number="CLM-IDEMP",
|
||||
payer_claim_control_number="ORPHAN-IDEMP",
|
||||
status_code="1",
|
||||
status_label="Primary",
|
||||
total_charge=Decimal("200.00"),
|
||||
@@ -586,4 +589,170 @@ def test_manual_match_idempotent_line_reconciliation():
|
||||
r = session.get(Remittance, remit_id)
|
||||
assert r is not None
|
||||
assert r.adjustment_amount == Decimal("20.00")
|
||||
assert r.claim_level_adjustment_amount == Decimal("20.00")
|
||||
assert r.claim_level_adjustment_amount == Decimal("20.00")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# SP27 Task 17: Claim.patient_control_number must be the CLM01 (claim_id)
|
||||
# the 837 sent, NOT the 2010BA subscriber member_id. The reconcile matcher
|
||||
# joins Claim.patient_control_number == Remittance.payer_claim_control_number,
|
||||
# and the 835 echoes CLM01 in CLP01 per X12 spec. Storing member_id here
|
||||
# silently breaks every auto-match in production (the 9/1,739 "matches"
|
||||
# observed in prod data are substring coincidences between synthetic CLM01
|
||||
# and the human-readable PCN).
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_837_result(claims):
|
||||
"""Build a minimal ParseResult with the given ClaimOutputs."""
|
||||
from cyclone.parsers.models import (
|
||||
BatchSummary as BatchSummary837,
|
||||
BillingProvider, ClaimHeader, ClaimOutput, Envelope,
|
||||
Payer, ParseResult, Subscriber, ValidationReport,
|
||||
)
|
||||
return ParseResult(
|
||||
envelope=Envelope(
|
||||
sender_id="S", receiver_id="R", control_number="0001",
|
||||
transaction_date=date(2026, 6, 19),
|
||||
),
|
||||
claims=claims,
|
||||
summary=BatchSummary837(
|
||||
input_file="c.txt", control_number="0001",
|
||||
transaction_date=date(2026, 6, 19),
|
||||
total_claims=len(claims), passed=len(claims), failed=0,
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
def _make_claim_output(claim_id="CLM-A", member_id="MEMBER-X",
|
||||
charge="100.00",
|
||||
service_date=date(2026, 6, 19)):
|
||||
"""Single ClaimOutput with distinct claim_id and subscriber.member_id.
|
||||
|
||||
``service_date`` populates the SV1 service-line date so the
|
||||
reconcile matcher's ``_pick_claim`` window check can find this
|
||||
claim when the matching 835 has the same date in SVC.
|
||||
"""
|
||||
from cyclone.parsers.models import (
|
||||
BillingProvider, ClaimHeader, ClaimOutput,
|
||||
Subscriber, Payer, Procedure, ServiceLine,
|
||||
ValidationReport,
|
||||
)
|
||||
return ClaimOutput(
|
||||
claim_id=claim_id,
|
||||
control_number="0001",
|
||||
transaction_date=date(2026, 6, 19),
|
||||
billing_provider=BillingProvider(name="Test", npi="1234567890"),
|
||||
subscriber=Subscriber(first_name="Jane", last_name="Doe",
|
||||
member_id=member_id),
|
||||
payer=Payer(name="Test Payer", id="P1"),
|
||||
claim=ClaimHeader(claim_id=claim_id, total_charge=Decimal(charge),
|
||||
frequency_code="1", place_of_service="11"),
|
||||
diagnoses=[],
|
||||
service_lines=[
|
||||
ServiceLine(
|
||||
line_number=1,
|
||||
procedure=Procedure(qualifier="HC", code="99213",
|
||||
modifiers=[]),
|
||||
charge=Decimal(charge),
|
||||
unit_type="UN",
|
||||
units=Decimal("1"),
|
||||
service_date=service_date,
|
||||
),
|
||||
],
|
||||
validation=ValidationReport(passed=True, errors=[], warnings=[]),
|
||||
raw_segments=[],
|
||||
)
|
||||
|
||||
|
||||
def test_837_ingest_populates_patient_control_number_from_claim_id():
|
||||
"""Claim.patient_control_number == claim_id (CLM01), not member_id.
|
||||
|
||||
RED: today the 837 ingest writes ``Claim.patient_control_number =
|
||||
claim.subscriber.member_id`` (store.py:194). That breaks the
|
||||
reconcile join against the 835's CLP01 (which echoes CLM01).
|
||||
"""
|
||||
from cyclone.db import Claim
|
||||
from cyclone.store import BatchRecord837
|
||||
|
||||
s = CycloneStore()
|
||||
s.add(BatchRecord837(
|
||||
id="b-837-pcn", kind="837p", input_filename="c.txt",
|
||||
parsed_at=datetime(2026, 6, 19, tzinfo=timezone.utc),
|
||||
result=_make_837_result([
|
||||
_make_claim_output(claim_id="CLM-A", member_id="MEMBER-X"),
|
||||
]),
|
||||
))
|
||||
|
||||
with db.SessionLocal()() as session:
|
||||
claim = session.get(Claim, "CLM-A")
|
||||
assert claim is not None
|
||||
assert claim.patient_control_number == "CLM-A", (
|
||||
f"expected CLM01 (claim_id) 'CLM-A', "
|
||||
f"got Claim.patient_control_number={claim.patient_control_number!r} "
|
||||
f"(should be the value the 837 sent in CLM01, not the "
|
||||
f"subscriber's 2010BA NM109 member_id)."
|
||||
)
|
||||
|
||||
|
||||
def test_837_then_835_with_echoed_pcn_auto_pairs():
|
||||
"""End-to-end: 837 CLM01='CLM-A' + 835 CLP01='CLM-A' must auto-match.
|
||||
|
||||
RED: today ``Claim.patient_control_number == subscriber.member_id``
|
||||
so the join on ``Claim.patient_control_number ==
|
||||
Remittance.payer_claim_control_number`` fails even when the payer
|
||||
echoes CLM01 correctly.
|
||||
"""
|
||||
from cyclone.db import Claim, Remittance
|
||||
from cyclone.store import BatchRecord837, BatchRecord835
|
||||
|
||||
s = CycloneStore()
|
||||
|
||||
# 837: CLM01 = 'CLM-A', subscriber.member_id = 'MEMBER-X' (different)
|
||||
s.add(BatchRecord837(
|
||||
id="b-837-echo", kind="837p", input_filename="c.txt",
|
||||
parsed_at=datetime(2026, 6, 19, tzinfo=timezone.utc),
|
||||
result=_make_837_result([
|
||||
_make_claim_output(claim_id="CLM-A", member_id="MEMBER-X",
|
||||
charge="100.00"),
|
||||
]),
|
||||
))
|
||||
|
||||
# 835: CLP01 = 'CLM-A' (the payer echoes CLM01 back), paid == charge
|
||||
cp = ClaimPayment(
|
||||
payer_claim_control_number="CLM-A",
|
||||
status_code="1", status_label="Primary",
|
||||
total_charge=Decimal("100.00"), total_paid=Decimal("100.00"),
|
||||
service_payments=[
|
||||
ServicePayment(
|
||||
line_number=1, procedure_qualifier="HC",
|
||||
procedure_code="99213",
|
||||
charge=Decimal("100.00"), payment=Decimal("100.00"),
|
||||
units=Decimal("1"),
|
||||
service_date=date(2026, 6, 19),
|
||||
adjustments=[],
|
||||
),
|
||||
],
|
||||
)
|
||||
s.add(BatchRecord835(
|
||||
id="b-835-echo", kind="835", input_filename="era.txt",
|
||||
parsed_at=datetime(2026, 6, 19, tzinfo=timezone.utc),
|
||||
result=_make_835_result([cp]),
|
||||
))
|
||||
|
||||
with db.SessionLocal()() as session:
|
||||
claim = session.get(Claim, "CLM-A")
|
||||
r = session.query(Remittance).filter(
|
||||
Remittance.payer_claim_control_number == "CLM-A"
|
||||
).first()
|
||||
# Both sides must have the FK set: Claim.matched_remittance_id
|
||||
# AND Remittance.claim_id, written in the same transaction.
|
||||
assert claim.matched_remittance_id is not None, (
|
||||
"Claim should be auto-matched to the remit (CLM01 echoed "
|
||||
"in CLP01), but matched_remittance_id is still NULL. The "
|
||||
"837 ingest is storing patient_control_number from "
|
||||
"subscriber.member_id instead of claim.claim_id."
|
||||
)
|
||||
assert r.claim_id == "CLM-A"
|
||||
assert r.claim_id == claim.id
|
||||
assert claim.state == "paid"
|
||||
Reference in New Issue
Block a user