diff --git a/backend/src/cyclone/reconcile.py b/backend/src/cyclone/reconcile.py index 1fed5d7..b30bca5 100644 --- a/backend/src/cyclone/reconcile.py +++ b/backend/src/cyclone/reconcile.py @@ -17,7 +17,7 @@ Match algorithm: from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, field from datetime import date, datetime, timezone from decimal import Decimal from typing import Iterable, Optional, Protocol @@ -50,6 +50,18 @@ class Match: remittance: _RemitLike strategy: str is_reversal: bool + # SP31: which content-keys the 2-of-3 (or 3-of-3) rule agreed on. + # Populated for ``score-auto`` matches; empty for ``pcn-exact`` / + # ``manual`` (PCN-exact only uses the PCN key by definition; manual + # is operator-driven). Floats into the ``auto_matched_835`` + # ActivityEvent payload so the audit trail records *why* the link + # fired. Transient — the ORM Match row does not persist these + # (spec keeps the audit trail in ActivityEvent.payload_json only). + keys_matched: set[str] = field(default_factory=set) + # SP31: how many candidates were in the ±30-day window pool when + # the score-auto match fired. Useful when an operator is reviewing + # a borderline auto-link — 1-of-1 is unambiguous; 1-of-5 was lucky. + candidate_count: int = 0 def match( @@ -123,8 +135,14 @@ CHARGE_TOLERANCE = Decimal("0.01") KEYS_REQUIRED = 2 -def _content_keys_match(remit, claim) -> bool: - """Return True when at least KEYS_REQUIRED of {PCN, charge, NPI} agree. +def _content_keys_match(remit, claim) -> set[str]: + """Return the set of content-keys that agree between ``remit`` and ``claim``. + + Compares {``pcn``, ``charge``, ``npi``} between the two sides and returns + the subset whose values match. Caller checks ``len(returned) >= KEYS_REQUIRED`` + to decide whether to auto-link. Returning the matched keys (rather than a + bool) lets the ActivityEvent payload record exactly which 2-of-3 (or 3-of-3) + produced the auto-link — the audit trail for the SP31 content-keys rule. Charge compared with ``CHARGE_TOLERANCE`` tolerance (rounding drift). NPI counts as "not matched" when the remit's NPI is empty. @@ -143,12 +161,12 @@ def _content_keys_match(remit, claim) -> bool: The ``raw_json`` fallback is what makes this helper safe against a raw ``Remittance`` ORM instance from ``reconcile.run()``. """ - keys_matched = 0 + matched: set[str] = set() # PCN: payer_claim_control_number ↔ patient_control_number (stripped). if (getattr(remit, "payer_claim_control_number", "") or "").strip() == \ (getattr(claim, "patient_control_number", "") or "").strip(): - keys_matched += 1 + matched.add("pcn") # 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`` @@ -170,7 +188,7 @@ def _content_keys_match(remit, claim) -> bool: if _remit_charge is not None and _claim_charge is not None and \ abs(_remit_charge - _claim_charge) < CHARGE_TOLERANCE: - keys_matched += 1 + matched.add("charge") # NPI: prefer attribute, fall back to raw_json (where the 835 parser # stores rendering_provider_npi). The Remittance ORM has no NPI column, @@ -186,9 +204,9 @@ def _content_keys_match(remit, claim) -> bool: or ((getattr(claim, "raw_json", None) or {}).get("rendering_provider_npi", "") or "") ).strip() if remit_npi and remit_npi == claim_npi: - keys_matched += 1 + matched.add("npi") - return keys_matched >= KEYS_REQUIRED + return matched # --- SP31: DB-side fallback candidate matcher ------------------------------- @@ -196,8 +214,8 @@ def _content_keys_match(remit, claim) -> bool: SCORE_FALLBACK_WINDOW_DAYS = 30 -def _score_fallback_candidates(session, remit) -> Optional[str]: - """Return a unique matching claim_id via the 2-of-3 content-keys rule. +def _score_fallback_candidates(session, remit) -> Optional[tuple[str, set[str], int]]: + """Return (claim_id, keys_matched, candidate_count) for a unique 2-of-3 match. Queries a ±SCORE_FALLBACK_WINDOW_DAYS candidate pool filtered by: - claim.matched_remittance_id IS NULL (not yet linked) @@ -205,8 +223,15 @@ def _score_fallback_candidates(session, remit) -> Optional[str]: REVERSED, RECONCILED) - claim.service_date_from within window of remit.service_date - Returns the unique claim.id when exactly one candidate passes the - 2-of-3 rule (via :func:`_content_keys_match`), otherwise ``None``. + Returns ``(claim_id, keys_matched, candidate_count)`` when exactly one + candidate passes the 2-of-3 rule (via :func:`_content_keys_match`), + otherwise ``None``. ``keys_matched`` is the set returned by the helper + (``{"pcn", "charge"}`` or any 2-of-3 / 3-of-3 combination) — the + ActivityEvent payload records it as the audit trail for *why* the + auto-link fired. ``candidate_count`` is the size of the window pool + (independent of how many matched) so the operator can see whether + the match was 1-of-N or 1-of-1. + Ambiguous matches (2+ candidates) also return ``None`` — they land in the Inbox Unlinked lane for manual review. @@ -244,9 +269,16 @@ def _score_fallback_candidates(session, remit) -> Optional[str]: ) ).scalars().all()) - matched_ids = [c.id for c in candidates if _content_keys_match(remit, c)] - if len(matched_ids) == 1: - return matched_ids[0] + matched_pairs = [ + (c.id, _content_keys_match(remit, c)) + for c in candidates + ] + matched_pairs = [ + (cid, ks) for cid, ks in matched_pairs if len(ks) >= KEYS_REQUIRED + ] + if len(matched_pairs) == 1: + cid, keys_matched = matched_pairs[0] + return (cid, keys_matched, len(candidates)) return None # 0 matches OR 2+ (ambiguous) @@ -399,9 +431,10 @@ def run(session, batch_id: str) -> ReconcileResult: continue if getattr(remit, "claim_id", None) is not None: continue # already linked - matched_claim_id = _score_fallback_candidates(session, remit) - if matched_claim_id is None: + fallback_result = _score_fallback_candidates(session, remit) + if fallback_result is None: continue + matched_claim_id, keys_matched, candidate_count = fallback_result # Find the claim object in the unmatched_claims list (it was loaded). target_claim = next( (c for c in unmatched_claims if c.id == matched_claim_id), @@ -412,6 +445,8 @@ def run(session, batch_id: str) -> ReconcileResult: matches.append(Match( claim=target_claim, remittance=remit, strategy="score-auto", is_reversal=remit.is_reversal, + keys_matched=keys_matched, + candidate_count=candidate_count, )) used_claim_ids.add(target_claim.id) @@ -461,7 +496,17 @@ def run(session, batch_id: str) -> ReconcileResult: batch_id=batch_id, claim_id=m.claim.id, remittance_id=m.remittance.id, payload_json=( - {"new_state": m.claim.state.value, "strategy": m.strategy} + { + "new_state": m.claim.state.value, "strategy": m.strategy, + # SP31 spec D8: the auto_matched_835 payload records + # which content-keys the 2-of-3 (or 3-of-3) rule + # agreed on, plus how many candidates were in the + # window pool. ``sorted()`` converts the set to a + # JSON-serializable list and pins a deterministic + # order for tests + audit reads. + "keys_matched": sorted(m.keys_matched), + "candidate_count": m.candidate_count, + } if m.strategy == "score-auto" else {"new_state": m.claim.state.value} ), diff --git a/backend/tests/test_provider_extended_response.py b/backend/tests/test_provider_extended_response.py index 400381d..d92df27 100644 --- a/backend/tests/test_provider_extended_response.py +++ b/backend/tests/test_provider_extended_response.py @@ -153,19 +153,18 @@ def test_provider_detail_includes_orphan_remit_received(seeded_db: TestClient): the moment an 835 lands — the most common activity, invisible. Setup: ingest 837+835 for ``TEST_837_NPI`` (claim CLM001 + remit - CLM001), then manually match them so ``Remittance.claim_id`` is - populated. The bug presents as: only ``claim_submitted`` and - ``manual_match`` appear (no ``remit_received``). The fix surfaces - all three. + CLM001). With SP31, the minimal fixtures now auto-link via the + content-keys fallback (PCN + charge agree → 2-of-3) so + ``Remittance.claim_id`` is populated as part of the 835 ingest + itself — no separate manual-match call needed. The bug presents + as: only ``claim_submitted`` appears (no ``remit_received``). The + fix surfaces both. """ - # Force the match — simulates the post-reconciliation state that - # populate Remittance.claim_id without depending on auto-reconcile - # heuristics (which don't match this minimal fixture). - match_resp = seeded_db.post( - "/api/reconciliation/match", - json={"claim_id": "CLM001", "remit_id": "CLM001"}, - ) - assert match_resp.status_code == 200, match_resp.text + # The minimal fixture's PCN ("CLM001") + charge ("$100.00") match, + # so SP31's content-keys fallback pairs the remit during ingest. + # No manual-match call is required (and would now 409 "already_matched"). + # The intent of this test — surfacing the remit_received event via + # the Remittance.claim_id join — is fully exercised either way. resp = seeded_db.get(f"/api/config/providers/{TEST_837_NPI}") assert resp.status_code == 200, resp.text diff --git a/backend/tests/test_reconcile.py b/backend/tests/test_reconcile.py index a2dc8ab..63b2424 100644 --- a/backend/tests/test_reconcile.py +++ b/backend/tests/test_reconcile.py @@ -396,86 +396,86 @@ def _make_claim_for_keys(claim_id: str, pcn: str, charge, npi: str): def test_content_keys_pcn_plus_charge_matches(): - """PCN + charge match (NPI mismatched) → True.""" + """PCN + charge match (NPI mismatched) → 2-of-3, returns ``{"pcn","charge"}``.""" from cyclone.reconcile import _content_keys_match from decimal import Decimal r = _make_remit_for_keys("r1", "ABC", Decimal("100.00"), "") c = _make_claim_for_keys("c1", "ABC", Decimal("100.00"), "1111111111") - assert _content_keys_match(r, c) is True + assert _content_keys_match(r, c) == {"pcn", "charge"} def test_content_keys_pcn_plus_npi_matches(): - """PCN + NPI match (charge mismatched) → True.""" + """PCN + NPI match (charge mismatched) → 2-of-3, returns ``{"pcn","npi"}``.""" from cyclone.reconcile import _content_keys_match from decimal import Decimal r = _make_remit_for_keys("r1", "ABC", Decimal("100.00"), "1111111111") c = _make_claim_for_keys("c1", "ABC", Decimal("200.00"), "1111111111") - assert _content_keys_match(r, c) is True + assert _content_keys_match(r, c) == {"pcn", "npi"} def test_content_keys_charge_plus_npi_matches(): - """Charge + NPI match (PCN mismatched) → True.""" + """Charge + NPI match (PCN mismatched) → 2-of-3, returns ``{"charge","npi"}``.""" from cyclone.reconcile import _content_keys_match from decimal import Decimal r = _make_remit_for_keys("r1", "ABC", Decimal("100.00"), "1111111111") c = _make_claim_for_keys("c1", "XYZ", Decimal("100.00"), "1111111111") - assert _content_keys_match(r, c) is True + assert _content_keys_match(r, c) == {"charge", "npi"} def test_content_keys_all_three_match(): - """All 3 match → True.""" + """All 3 match → 3-of-3, returns ``{"pcn","charge","npi"}``.""" from cyclone.reconcile import _content_keys_match from decimal import Decimal r = _make_remit_for_keys("r1", "ABC", Decimal("100.00"), "1111111111") c = _make_claim_for_keys("c1", "ABC", Decimal("100.00"), "1111111111") - assert _content_keys_match(r, c) is True + assert _content_keys_match(r, c) == {"pcn", "charge", "npi"} def test_content_keys_only_pcn_does_not_match(): - """Only PCN matches (charge + NPI both wrong) → False.""" + """Only PCN matches (charge + NPI both wrong) → set is just ``{"pcn"}``.""" from cyclone.reconcile import _content_keys_match from decimal import Decimal r = _make_remit_for_keys("r1", "ABC", Decimal("100.00"), "1111111111") c = _make_claim_for_keys("c1", "ABC", Decimal("200.00"), "2222222222") - assert _content_keys_match(r, c) is False + assert _content_keys_match(r, c) == {"pcn"} def test_content_keys_only_charge_does_not_match(): - """Only charge matches (PCN + NPI both wrong) → False.""" + """Only charge matches (PCN + NPI both wrong) → set is just ``{"charge"}``.""" from cyclone.reconcile import _content_keys_match from decimal import Decimal r = _make_remit_for_keys("r1", "ABC", Decimal("100.00"), "1111111111") c = _make_claim_for_keys("c1", "XYZ", Decimal("100.00"), "2222222222") - assert _content_keys_match(r, c) is False + assert _content_keys_match(r, c) == {"charge"} def test_content_keys_none_match(): - """All 3 fields disagree → False.""" + """All 3 fields disagree → empty set.""" from cyclone.reconcile import _content_keys_match from decimal import Decimal r = _make_remit_for_keys("r1", "ABC", Decimal("100.00"), "1111111111") c = _make_claim_for_keys("c1", "XYZ", Decimal("200.00"), "2222222222") - assert _content_keys_match(r, c) is False + assert _content_keys_match(r, c) == set() def test_content_keys_charge_tolerance_is_one_cent(): - """Charge differs by exactly $0.005 (rounding) → True ($0.01 tolerance).""" + """Charge differs by exactly $0.005 (rounding) → charge counts as matched.""" from cyclone.reconcile import _content_keys_match from decimal import Decimal r = _make_remit_for_keys("r1", "ABC", Decimal("100.005"), "1111111111") c = _make_claim_for_keys("c1", "ABC", Decimal("100.00"), "1111111111") - assert _content_keys_match(r, c) is True + assert _content_keys_match(r, c) == {"pcn", "charge", "npi"} def test_content_keys_charge_differs_by_two_cents_no_match(): - """Charge differs by $0.02 → False (outside tolerance).""" + """Charge differs by $0.02 → charge does NOT match (outside tolerance).""" from cyclone.reconcile import _content_keys_match from decimal import Decimal # PCN and NPI deliberately differ so the only potential match is charge. - # Charge is outside the $0.01 tolerance → 0 of 3 match → False. + # Charge is outside the $0.01 tolerance → 0 of 3 match → empty set. r = _make_remit_for_keys("r1", "ABC", Decimal("100.02"), "1111111111") c = _make_claim_for_keys("c1", "XYZ", Decimal("100.00"), "2222222222") - assert _content_keys_match(r, c) is False + assert _content_keys_match(r, c) == set() def test_content_keys_empty_npi_does_not_count(): @@ -484,16 +484,16 @@ def test_content_keys_empty_npi_does_not_count(): from decimal import Decimal r = _make_remit_for_keys("r1", "ABC", Decimal("100.00"), "") c = _make_claim_for_keys("c1", "ABC", Decimal("100.00"), "1111111111") - assert _content_keys_match(r, c) is True # PCN + charge still match + assert _content_keys_match(r, c) == {"pcn", "charge"} # NPI skipped on remit c2 = _make_claim_for_keys("c2", "XYZ", Decimal("100.00"), "1111111111") - assert _content_keys_match(r, c2) is False # only charge, no PCN + assert _content_keys_match(r, c2) == {"charge"} # only charge, no PCN # --- SP31: content-keys fallback matcher (DB helper) ------------------------ def test_score_fallback_unique_match_returns_claim_id(db_session, make_claim, make_remit): - """One claim in pool matches 2-of-3 → returns that claim.""" + """One claim in pool matches 2-of-3 → returns (claim_id, keys, candidate_count).""" from decimal import Decimal from datetime import date, timedelta from cyclone.reconcile import _score_fallback_candidates @@ -507,7 +507,12 @@ def test_score_fallback_unique_match_returns_claim_id(db_session, make_claim, ma total_charge_amount=Decimal("100.00"), rendering_provider_npi="1111111111", service_date=date(2026, 6, 5)) # ±30-day window - assert _score_fallback_candidates(db_session, r) == c1.id + result = _score_fallback_candidates(db_session, r) + assert result is not None + claim_id, keys_matched, candidate_count = result + assert claim_id == c1.id + assert keys_matched == {"pcn", "charge", "npi"} # 3-of-3 + assert candidate_count == 2 # pool had 2 candidates def test_score_fallback_zero_matches_returns_none(db_session, make_claim, make_remit): @@ -591,7 +596,11 @@ def test_score_fallback_widened_window_catches_late_remit(db_session, make_claim total_charge_amount=Decimal("100.00"), rendering_provider_npi="1111111111", service_date=remit_date) - assert _score_fallback_candidates(db_session, r) == c1.id + result = _score_fallback_candidates(db_session, r) + assert result is not None + claim_id, keys_matched, candidate_count = result + assert claim_id == c1.id + assert candidate_count == 1 def test_score_fallback_outside_window_excluded(db_session, make_claim, make_remit): @@ -652,7 +661,7 @@ def test_content_keys_npi_from_remit_raw_json(db_session, make_claim): ) db_session.add(remit) db_session.flush() - assert _content_keys_match(remit, claim) is True # charge + NPI = 2 of 3 + assert _content_keys_match(remit, claim) == {"charge", "npi"} # PCN mismatched # --- SP31: end-to-end integration via reconcile.run() ----------------------- @@ -692,6 +701,12 @@ def test_reconcile_run_emits_score_auto_match_when_pcn_misses(db_session, make_c assert len(matches) == 1 assert matches[0].strategy == "score-auto" assert matches[0].claim_id == claim.id + # Symmetric FK: the auto-match path sets Remittance.claim_id so the + # list_unmatched filter (``Remittance.claim_id IS NULL``) correctly + # drops this pair from the orphan bucket. Mirrors the manual_match + # contract — both paths must populate both sides of the FK pair. + db_session.refresh(remit) + assert remit.claim_id == claim.id # Verify ActivityEvent was emitted with kind="auto_matched_835" from cyclone.db import ActivityEvent events = list(db_session.execute( @@ -702,6 +717,13 @@ def test_reconcile_run_emits_score_auto_match_when_pcn_misses(db_session, make_c ).scalars().all()) assert len(events) == 1 assert events[0].claim_id == claim.id + # Spec D8: payload records keys_matched + candidate_count so the + # operator can see *why* the auto-link fired. + payload = events[0].payload_json or {} + assert "keys_matched" in payload + assert "candidate_count" in payload + assert payload["candidate_count"] == 1 + assert set(payload["keys_matched"]) >= {"charge", "npi"} # PCN differs in this test # Verify claim state was flipped db_session.refresh(claim) assert claim.state in {ClaimState.PAID, ClaimState.PARTIAL, ClaimState.RECEIVED}