diff --git a/backend/src/cyclone/reconcile.py b/backend/src/cyclone/reconcile.py index fd07817..d90f6f0 100644 --- a/backend/src/cyclone/reconcile.py +++ b/backend/src/cyclone/reconcile.py @@ -133,9 +133,15 @@ def _content_keys_match(remit, claim) -> bool: - shim / dataclass test objects (planned names: total_charge_amount, total_charge, rendering_provider_npi), and - real SQLAlchemy ORM instances (Claim.charge_amount, Remittance.total_charge, - Claim.provider_npi). The Remittance ORM has no NPI column — callers - that need NPI on a Remit set the transient ``rendering_provider_npi`` - attribute (the 835 parser does this on raw_json; tests do it directly). + Claim.provider_npi). + + Production note: the 835 parser stores ``rendering_provider_npi`` and + ``total_charge_amount`` on ``Remittance.raw_json`` (the ORM has no + dedicated column), so the read order below is: + - NPI: transient attribute → raw_json + - Charge: planned attribute name → real ORM column → raw_json + The ``raw_json`` fallback is what makes this helper safe against a + raw ``Remittance`` ORM instance from ``reconcile.run()``. """ keys_matched = 0 @@ -144,19 +150,40 @@ def _content_keys_match(remit, claim) -> bool: (getattr(claim, "patient_control_number", "") or "").strip(): keys_matched += 1 - # Charge: total_charge_amount/total_charge ↔ total_charge/charge_amount (within $0.01). - remit_charge = getattr(remit, "total_charge_amount", None) or getattr(remit, "total_charge", None) - claim_charge = getattr(claim, "total_charge", None) or getattr(claim, "charge_amount", None) - if remit_charge is not None and claim_charge is not None and \ - abs(remit_charge - claim_charge) < CHARGE_TOLERANCE: + # Charge: prefer the planned attribute names, fall back to real ORM columns, + # then to raw_json (where the 835 parser stores CLP03). Explicit ``is None`` + # checks avoid the ``Decimal("0")`` truthiness footgun — ``Decimal("0")`` + # is truthy in a boolean context but ``or`` would still let it through; + # the more important property is that we don't accidentally replace a real + # value with a default. + _remit_charge = getattr(remit, "total_charge_amount", None) + if _remit_charge is None: + _remit_charge = getattr(remit, "total_charge", None) + if _remit_charge is None: + _remit_charge = (getattr(remit, "raw_json", None) or {}).get("total_charge_amount") + + _claim_charge = getattr(claim, "total_charge", None) + if _claim_charge is None: + _claim_charge = getattr(claim, "charge_amount", None) + if _claim_charge is None: + _claim_charge = (getattr(claim, "raw_json", None) or {}).get("total_charge_amount") + + if _remit_charge is not None and _claim_charge is not None and \ + abs(_remit_charge - _claim_charge) < CHARGE_TOLERANCE: keys_matched += 1 - # NPI: rendering_provider_npi (or provider_npi fallback on Claim) ↔ - # rendering_provider_npi (must be non-empty on the remit side). - remit_npi = (getattr(remit, "rendering_provider_npi", "") or "").strip() + # NPI: prefer attribute, fall back to raw_json (where the 835 parser + # stores rendering_provider_npi). The Remittance ORM has no NPI column, + # so the raw_json path is the production read for raw ORM instances — + # it must NOT be removed. + remit_npi = ( + (getattr(remit, "rendering_provider_npi", "") or "") + or ((getattr(remit, "raw_json", None) or {}).get("rendering_provider_npi", "") or "") + ).strip() claim_npi = ( (getattr(claim, "rendering_provider_npi", "") or "") or (getattr(claim, "provider_npi", "") or "") + or ((getattr(claim, "raw_json", None) or {}).get("rendering_provider_npi", "") or "") ).strip() if remit_npi and remit_npi == claim_npi: keys_matched += 1 diff --git a/backend/tests/test_reconcile.py b/backend/tests/test_reconcile.py index 0deaa74..890d03a 100644 --- a/backend/tests/test_reconcile.py +++ b/backend/tests/test_reconcile.py @@ -609,3 +609,47 @@ def test_score_fallback_outside_window_excluded(db_session, make_claim, make_rem rendering_provider_npi="1111111111", service_date=remit_date) assert _score_fallback_candidates(db_session, r) is None + + +# --- SP31 Task 3 fix: Remittance NPI lives in raw_json ---------------------- + + +def test_content_keys_npi_from_remit_raw_json(db_session, make_claim): + """NPI on the Remit is read from ``raw_json`` when the attribute is absent. + + The 835 parser stores rendering_provider_npi on ``Remittance.raw_json`` + (the ORM has no dedicated column) — reconcile.run() hands a raw + ``Remittance`` instance to ``_content_keys_match``, so the helper must + fall through ``getattr(...rendering_provider_npi...)`` to the raw_json + read. This test pins that path with a real ORM row (no transient + ``setattr`` shadowing the production layout). + + Configuration: PCN mismatched, charge matches, NPI matches via raw_json + → 2 of 3 keys agree → True. + """ + from decimal import Decimal + from datetime import date, datetime, timezone + from cyclone.db import Remittance + from cyclone.reconcile import _content_keys_match + claim = make_claim( + patient_control_number="ABC", + total_charge=Decimal("100.00"), + rendering_provider_npi="1111111111", + service_date_from=date(2026, 6, 1), + ) + # Real Remittance ORM instance — NPI lives ONLY in raw_json, not as a + # transient attribute. (The 835 parser populates raw_json that way; + # the make_remit factory's transient ``rendering_provider_npi`` setattr + # would shadow the production read.) + remit = Remittance( + id="real-remit-1", + batch_id="test-batch", + payer_claim_control_number="OTHER-PCN", # PCN: won't match "ABC" + status_code="1", + total_charge=Decimal("100.00"), # charge: matches + received_at=datetime(2026, 6, 19, tzinfo=timezone.utc), + raw_json={"rendering_provider_npi": "1111111111"}, # NPI: matches + ) + db_session.add(remit) + db_session.flush() + assert _content_keys_match(remit, claim) is True # charge + NPI = 2 of 3