fix(sp31): read NPI and charge from raw_json when ORM attribute missing

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
+        Claim.provider_npi).
-        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).
+    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).
+    # Charge: prefer the planned attribute names, fall back to real ORM columns,
-    remit_charge = getattr(remit, "total_charge_amount", None) or getattr(remit, "total_charge", None)
+    # then to raw_json (where the 835 parser stores CLP03). Explicit ``is None``
-    claim_charge = getattr(claim, "total_charge", None) or getattr(claim, "charge_amount", None)
+    # checks avoid the ``Decimal("0")`` truthiness footgun — ``Decimal("0")``
-    if remit_charge is not None and claim_charge is not None and \
+    # is truthy in a boolean context but ``or`` would still let it through;
-       abs(remit_charge - claim_charge) < CHARGE_TOLERANCE:
+    # 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) ↔
+    # NPI: prefer attribute, fall back to raw_json (where the 835 parser
-    #      rendering_provider_npi (must be non-empty on the remit side).
+    # stores rendering_provider_npi). The Remittance ORM has no NPI column,
-    remit_npi = (getattr(remit, "rendering_provider_npi", "") or "").strip()
+    # 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

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