From 7ad190c26784fb6be9fa95446bea064dc38d418f Mon Sep 17 00:00:00 2001 From: Nora Date: Tue, 7 Jul 2026 10:30:23 -0600 Subject: [PATCH] feat(sp37): batch_envelope_index now resolves by ST02 or ISA13 Each 837p batch row contributes up to two entries to the join-key index (envelope.control_number + transaction_set_control_number). One idx.get(set_control_number) call resolves either, so 999 acks whose AK201 echoes the source 837's ST02 now hit Pass 1 where they previously fell through to Pass 2 (and to the orphan log). Backward compat preserved: rows with transaction_set_control_number NULL (pre-SP37 batches) still resolve by ISA13. --- backend/src/cyclone/store/claim_acks.py | 41 ++++-- .../tests/test_batch_envelope_index_sp37.py | 126 ++++++++++++++++++ 2 files changed, 155 insertions(+), 12 deletions(-) create mode 100644 backend/tests/test_batch_envelope_index_sp37.py diff --git a/backend/src/cyclone/store/claim_acks.py b/backend/src/cyclone/store/claim_acks.py index dbbdc4e..94cfaad 100644 --- a/backend/src/cyclone/store/claim_acks.py +++ b/backend/src/cyclone/store/claim_acks.py @@ -67,26 +67,43 @@ def _safe_publish(event_bus: "EventBus | None", kind: str, payload: dict) -> Non def batch_envelope_index() -> dict[str, str]: - """Build a ``{envelope.control_number: batch.id}`` map. + """Build a ``{key: batch.id}`` map populated from two columns. - D10 primary join key (spec §D10). Built once per ingest from - every Batch row whose ``raw_result_json`` carries an envelope — - cost is O(N batches), currently ~16, so trivial. Kept as a - plain dict so callers can ``index.get(scn)`` to resolve. + D10 primary join key (spec §D10). Each 837p batch row contributes + up to two entries: + + * ``Batch.raw_result_json.envelope.control_number`` (ISA13) + * ``Batch.transaction_set_control_number`` (ST02, new in SP37) + + Either key resolves to the same ``batch.id``; callers do a single + ``idx.get(set_control_number)`` lookup. Pass 2 (PCN) is unchanged. + + Built once per ingest from every Batch row whose kind is "837p"; + cost is O(N batches), currently small, so trivial. 835 batches + are excluded — the column is an 837P-specific join key (see + ``Batch.transaction_set_control_number`` docstring). """ - out: dict[str, str] = {} + idx: dict[str, str] = {} with db.SessionLocal()() as s: rows = ( - s.query(Batch.id, Batch.raw_result_json) + s.query( + Batch.id, + Batch.raw_result_json, + Batch.transaction_set_control_number, + ) .filter(Batch.kind == "837p") .all() ) - for bid, raw in rows: + for bid, raw, stcn in rows: env = (raw or {}).get("envelope") or {} - ctrl = env.get("control_number") - if isinstance(ctrl, str) and ctrl: - out[ctrl] = bid - return out + isa_cn = env.get("control_number") + if isinstance(isa_cn, str) and isa_cn: + # First write wins (setdefault) — if ISA13 and ST02 + # ever collide they map to the same batch anyway. + idx.setdefault(isa_cn, bid) + if isinstance(stcn, str) and stcn: + idx.setdefault(stcn, bid) + return idx # --------------------------------------------------------------------------- diff --git a/backend/tests/test_batch_envelope_index_sp37.py b/backend/tests/test_batch_envelope_index_sp37.py new file mode 100644 index 0000000..d3ee54c --- /dev/null +++ b/backend/tests/test_batch_envelope_index_sp37.py @@ -0,0 +1,126 @@ +"""SP37 Task 3: batch_envelope_index populates from BOTH columns. + +The dict returned by ``batch_envelope_index`` should resolve lookups +by EITHER ``Batch.raw_result_json.envelope.control_number`` (ISA13, +preserved) OR ``Batch.transaction_set_control_number`` (ST02, new in +SP37 Task 2). One dict, both keys — a single ``.get(set_control_number)`` +call hits whichever matches, so the D10 Pass 1 join in +:func:`cyclone.claim_acks.lookup_claims_for_ack_set_response` resolves +999 AK201 (which echoes the source 837's ST02) back to the right batch +even when ST02 != ISA13. +""" +from __future__ import annotations + +from datetime import datetime, timezone + +import pytest + +from cyclone import db as db_mod +from cyclone.db import Batch +from cyclone.store.claim_acks import batch_envelope_index + + +@pytest.fixture(autouse=True) +def _db(tmp_path, monkeypatch): + """Per-test DB; conftest's autouse already wires one too, but we + pin the URL again so this module is self-contained if anyone ever + lifts it out of the conftest tree.""" + monkeypatch.setenv("CYCLONE_DB_URL", f"sqlite:///{tmp_path}/test.db") + db_mod._reset_for_tests() + db_mod.init_db() + yield + + +def _make_batch( + s, + *, + id: str, + icn: str, + stcn: str | None = None, + raw_json: dict | None = None, +) -> Batch: + """Insert one Batch row whose envelope + ST02 mirror what + ``store.write.add_record`` writes for an 837P batch.""" + row = Batch( + id=id, + kind="837p", + input_filename=id + ".x12", + parsed_at=datetime.now(timezone.utc), + raw_result_json=raw_json or {"envelope": {"control_number": icn}}, + transaction_set_control_number=stcn, + ) + s.add(row) + s.commit() + s.refresh(row) + return row + + +def test_index_resolves_by_envelope_control_number(): + """Pre-SP37 path: ISA13 (envelope.control_number) still resolves.""" + with db_mod.SessionLocal()() as s: + _make_batch(s, id="b1", icn="ISA000001") + idx = batch_envelope_index() + assert idx.get("ISA000001") == "b1" + + +def test_index_resolves_by_transaction_set_control_number(): + """SP37 path: ST02 (transaction_set_control_number) resolves too. + + The whole point of Task 3 — Gainwell batches have ST02 != ISA13, + so 999 AK201 echoes ST02 and must hit this branch. + """ + with db_mod.SessionLocal()() as s: + _make_batch( + s, id="b2", icn="ISA000002", stcn="ST000002", + raw_json={"envelope": {"control_number": "ISA000002"}}, + ) + idx = batch_envelope_index() + assert idx.get("ST000002") == "b2" + # Backward compat: ISA still resolves. + assert idx.get("ISA000002") == "b2" + + +def test_index_handles_row_with_no_transaction_set_control_number(): + """Pre-migration rows (stcn NULL) should still resolve by ISA. + + Spec says: 'Pre-migration rows (stcn NULL) should still resolve by ISA.' + Production row ``b1`` (the only one with claims) has ST02 NULL because + the migration's backfill only ran over rows whose raw_result_json + envelope had the ST02 key — and that row was written before the + parser populated it. + """ + with db_mod.SessionLocal()() as s: + _make_batch(s, id="b3", icn="ISA000003") # no stcn + idx = batch_envelope_index() + assert idx.get("ISA000003") == "b3" + assert idx.get("ST000003") is None + + +def test_index_ignores_non_837_batches(): + """835 batches don't contribute to the 999 join index. + + The D10 two-pass join is specifically for 837 → 999 linkage; 835 + remittances live on the response side of the loop and have no ST02 + join key. Filtering by ``kind == "837p"`` keeps the index scoped + correctly and avoids an 835 row shadowing an 837 ST02 by accident. + """ + with db_mod.SessionLocal()() as s: + # Add an 837 row first so we can prove the 835 doesn't leak in. + _make_batch(s, id="b-837", icn="ISA837", stcn="ST837") + # And an 835 row whose ISA13 / ST02 would otherwise pollute. + s.add(Batch( + id="b-835", + kind="835", + input_filename="b-835.x12", + parsed_at=datetime.now(timezone.utc), + raw_result_json={"envelope": {"control_number": "ISA999"}}, + transaction_set_control_number="ST999", + )) + s.commit() + idx = batch_envelope_index() + # 837 row resolves both ways. + assert idx.get("ISA837") == "b-837" + assert idx.get("ST837") == "b-837" + # 835 row contributes nothing. + assert "ISA999" not in idx + assert "ST999" not in idx