feat(sp31): expose keys_matched/candidate_count in Match dataclass and ActivityEvent payload, fix broken pre-existing test
This commit is contained in:
@@ -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}
|
||||
),
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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}
|
||||
|
||||
Reference in New Issue
Block a user