Compare commits
18 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 3bc5740e8b | |||
| b0e06a2dd0 | |||
| d1cd6e1a51 | |||
| f25214189a | |||
| e4f3d25f3a | |||
| 750f560ee0 | |||
| 0193ee4c32 | |||
| 6bda5005c1 | |||
| 27bca33b09 | |||
| f10ab83628 | |||
| 244015a361 | |||
| 3bf5622010 | |||
| 0625c83a45 | |||
| cf7c343ff0 | |||
| dae7749464 | |||
| 2e7ad471e0 | |||
| dfd654202e | |||
| 15c85300f6 |
@@ -38,3 +38,7 @@ claims_output/
|
||||
# Brainstorm session artifacts (visual companion mockups, events, server state).
|
||||
# Skills under .superpowers/skills/ are committed project-scoped guidance.
|
||||
.superpowers/brainstorm/
|
||||
|
||||
# SP33+ scratch / production-data ingest. Generated artifacts live
|
||||
# here only — the source EDI sits under docs/prodfiles/.
|
||||
ingest/
|
||||
|
||||
@@ -83,6 +83,7 @@ from cyclone.parsers.parse_837 import parse
|
||||
from cyclone.parsers.parse_835 import parse as parse_835
|
||||
from cyclone.parsers.parse_999 import parse_999_text
|
||||
from cyclone.parsers.parse_ta1 import parse_ta1_text
|
||||
from cyclone.parsers.segments import tokenize as _tokenize_segments
|
||||
from cyclone.parsers.serialize_270 import serialize_270
|
||||
from cyclone.parsers.serialize_999 import serialize_999
|
||||
from cyclone.parsers.serialize_837 import SerializeError as SerializeError837, serialize_837, serialize_837_for_resubmit
|
||||
@@ -353,6 +354,23 @@ def _resolve_payer_835(name: str) -> PayerConfig835:
|
||||
return PAYER_FACTORIES_835[name]()
|
||||
|
||||
|
||||
def _transaction_set_id_from_segments(segments: list[list[str]]) -> str | None:
|
||||
"""Return the ST01 transaction-set id (``"837"``, ``"835"``, ``"999"``...).
|
||||
|
||||
SP35 helper: scans the first few tokenized segments for the ST
|
||||
segment and returns its second element (ST01). Returns None when no
|
||||
ST is present — e.g. a TA1 file, which uses the bare TA1 segment
|
||||
and no ST envelope. The endpoint-level envelope guards treat
|
||||
``None`` as "no ST found; let the parser decide" so TA1 files
|
||||
routed through the wrong endpoint still surface a parse error
|
||||
rather than a misleading "expected 837p, got ''" message.
|
||||
"""
|
||||
for seg in segments[:5]: # ST is always the second segment after ISA
|
||||
if seg and seg[0] == "ST" and len(seg) > 1:
|
||||
return seg[1]
|
||||
return None
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Catch-all exception handler
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -391,6 +409,23 @@ async def parse_837(
|
||||
strict: bool = Query(False),
|
||||
ack: bool = Query(False),
|
||||
) -> Any:
|
||||
# SP35: defense-in-depth input guards. Layer A (UI auto-detect) lives
|
||||
# in src/pages/Upload.tsx; the server-side checks below are the
|
||||
# authoritative fix because they protect every caller of the API
|
||||
# (Upload page, CLI ingestion, any future bulk-import tool). Without
|
||||
# these, an 835 file dropped on the Upload page while the dropdown
|
||||
# still says "837p" produces a BatchRecord with claims=[] and a bogus
|
||||
# row on the History tab. The fix is two checks run BEFORE we persist
|
||||
# anything:
|
||||
#
|
||||
# 1. Envelope check — ST01 must be "837" or "837P". Anything else
|
||||
# (an 835, a 999, a 270, garbage that happens to have an ISA)
|
||||
# → 400 with error="Mismatched file kind", expected="837p",
|
||||
# detected_st=<whatever was there>.
|
||||
# 2. Empty-claims check — even with the right envelope, if the
|
||||
# parser produced zero CLM segments (truncated file, header-only
|
||||
# test fixture) → 400 with error="No claims parsed". A real
|
||||
# production 837 batch with zero claims is never valid.
|
||||
raw = await file.read()
|
||||
if not raw:
|
||||
return JSONResponse(
|
||||
@@ -407,6 +442,32 @@ async def parse_837(
|
||||
|
||||
config = _resolve_payer(payer)
|
||||
|
||||
# SP35 guard 1: envelope check. Tokenize first so we can return a
|
||||
# precise 400 (vs. relying on the parser's "no ISA envelope" error
|
||||
# which is correct but doesn't say "you sent an 835 to the 837
|
||||
# endpoint"). If tokenization itself fails we fall through to the
|
||||
# parser, which raises CycloneParseError → 400 "Parse error" path.
|
||||
try:
|
||||
_segments = _tokenize_segments(text)
|
||||
detected_st = _transaction_set_id_from_segments(_segments) or ""
|
||||
except CycloneParseError:
|
||||
detected_st = ""
|
||||
|
||||
if detected_st and not detected_st.upper().startswith("837"):
|
||||
return JSONResponse(
|
||||
status_code=400,
|
||||
content={
|
||||
"error": "Mismatched file kind",
|
||||
"expected": "837p",
|
||||
"detected_st": detected_st,
|
||||
"detail": (
|
||||
f"File declares ST*{detected_st}* but this endpoint "
|
||||
f"expects ST*837*. Pick the matching endpoint on the "
|
||||
f"Upload page (or let auto-detect choose for you)."
|
||||
),
|
||||
},
|
||||
)
|
||||
|
||||
try:
|
||||
result = parse(text, config, input_file=file.filename or "")
|
||||
except CycloneParseError as exc:
|
||||
@@ -421,6 +482,23 @@ async def parse_837(
|
||||
content={"error": "Internal server error", "detail": str(exc)},
|
||||
)
|
||||
|
||||
# SP35 guard 2: empty-claims check. With the envelope validated, the
|
||||
# only way to land here is a header-only file (real, but useless)
|
||||
# or a file whose CLM loops the parser couldn't extract. Either way
|
||||
# we refuse to persist — a BatchRecord with claims=[] is what the
|
||||
# original bug produced and is never what the operator wanted.
|
||||
if not result.claims:
|
||||
return JSONResponse(
|
||||
status_code=400,
|
||||
content={
|
||||
"error": "No claims parsed",
|
||||
"detail": (
|
||||
"The file passed the envelope check but contained no "
|
||||
"CLM segments. Refusing to persist an empty batch."
|
||||
),
|
||||
},
|
||||
)
|
||||
|
||||
if strict:
|
||||
result = _strict_rewrite(result)
|
||||
if not include_raw_segments:
|
||||
@@ -593,6 +671,31 @@ async def parse_835_endpoint(
|
||||
|
||||
config = _resolve_payer_835(payer)
|
||||
|
||||
# SP35 guard 1: envelope check. Mirrors the parse-837 path: tokenize,
|
||||
# read ST01, reject anything that doesn't start with "835". Same
|
||||
# defense-in-depth rationale — the UI auto-detect (src/pages/Upload.tsx)
|
||||
# is layer A, but server-side guards protect every API caller.
|
||||
try:
|
||||
_segments_835 = _tokenize_segments(text)
|
||||
detected_st_835 = _transaction_set_id_from_segments(_segments_835) or ""
|
||||
except CycloneParseError:
|
||||
detected_st_835 = ""
|
||||
|
||||
if detected_st_835 and not detected_st_835.upper().startswith("835"):
|
||||
return JSONResponse(
|
||||
status_code=400,
|
||||
content={
|
||||
"error": "Mismatched file kind",
|
||||
"expected": "835",
|
||||
"detected_st": detected_st_835,
|
||||
"detail": (
|
||||
f"File declares ST*{detected_st_835}* but this endpoint "
|
||||
f"expects ST*835*. Pick the matching endpoint on the "
|
||||
f"Upload page (or let auto-detect choose for you)."
|
||||
),
|
||||
},
|
||||
)
|
||||
|
||||
try:
|
||||
result = parse_835(text, config, input_file=file.filename or "")
|
||||
except CycloneParseError as exc:
|
||||
@@ -607,6 +710,21 @@ async def parse_835_endpoint(
|
||||
content={"error": "Internal server error", "detail": str(exc)},
|
||||
)
|
||||
|
||||
# SP35 guard 2: empty-claims check. Same as parse-837: a BatchRecord
|
||||
# with claims=[] is never a valid production 835 batch and we refuse
|
||||
# to persist it.
|
||||
if not result.claims:
|
||||
return JSONResponse(
|
||||
status_code=400,
|
||||
content={
|
||||
"error": "No claims parsed",
|
||||
"detail": (
|
||||
"The file passed the envelope check but contained no "
|
||||
"CLP segments. Refusing to persist an empty batch."
|
||||
),
|
||||
},
|
||||
)
|
||||
|
||||
# Always run the validator; attach the report so the JSON path can
|
||||
# surface it and the NDJSON path can fold the counts into the summary.
|
||||
# 835 validation is batch-level, so pass/fail applies uniformly to every
|
||||
|
||||
+373
-4
@@ -388,10 +388,6 @@ def backfill_rendering_npi(
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# SP17: `cyclone backup` subcommands
|
||||
#
|
||||
@@ -403,6 +399,375 @@ if __name__ == "__main__":
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# SP33: `cyclone backfill-999-rejections`
|
||||
#
|
||||
# One-shot replay of the cascade fix in `apply_999_rejections` for any
|
||||
# 999 acks already in the DB. Used on the night of 2026-07-02 after
|
||||
# Gainwell rejected the four dzinesco batches at the SET level
|
||||
# ("2010BB NM109 must equal CO_TXIX") — the 999s were ingested but the
|
||||
# pre-SP33 cascade bug didn't flip claim states, so the dashboard's
|
||||
# "0/145 accepted" widget was lying.
|
||||
#
|
||||
# Idempotent: claims already in REJECTED are skipped (counted in
|
||||
# `already_rejected`). Safe to re-run.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@main.command("backfill-999-rejections")
|
||||
@click.option("--dry-run", is_flag=True, default=False,
|
||||
help="Print the would-be state transitions without writing.")
|
||||
@click.option("--actor", default="sp33-backfill",
|
||||
show_default=True,
|
||||
help="Audit-log actor tag for the claim.rejected events.")
|
||||
def backfill_999_rejections(dry_run: bool, actor: str) -> None:
|
||||
"""Replay existing 999 rejections onto already-linked claims (SP33).
|
||||
|
||||
Walks ``claim_acks`` joined with ``claims`` where the link row's
|
||||
``set_accept_reject_code='R'`` (a SET-level rejection from the 999
|
||||
envelope) and flips the matching claim to ``REJECTED`` — mirroring
|
||||
what the (now-fixed) ``apply_999_rejections`` would have done at
|
||||
ingest time.
|
||||
|
||||
Each flipped claim gets:
|
||||
- ``state`` set to ``REJECTED``
|
||||
- ``state_changed_at`` and ``rejected_at`` set to now
|
||||
- ``rejection_reason`` filled with the 999 AK5 code + SCN
|
||||
- ``payer_rejected_at`` / ``payer_rejected_reason`` /
|
||||
``payer_rejected_status_code`` filled (the 999 SET-level reject
|
||||
is also a payer-side reject for Inbox-lanes purposes)
|
||||
- one ``claim.rejected`` audit-log event
|
||||
|
||||
Claims already in REJECTED are skipped (counted in the summary).
|
||||
"""
|
||||
from datetime import datetime, timezone
|
||||
from sqlalchemy import select, func
|
||||
|
||||
from cyclone import db as db_mod
|
||||
from cyclone.audit_log import AuditEvent, append_event
|
||||
|
||||
db_mod.init_db()
|
||||
now = datetime.now(timezone.utc)
|
||||
|
||||
with db_mod.SessionLocal()() as session:
|
||||
# Pull one representative R-coded link per claim_id (a single
|
||||
# 999 may stamp many AK2 rows against the same claim, so we
|
||||
# collapse via MIN(set_control_number) / MIN(ak2_index) to avoid
|
||||
# firing 1 audit event per duplicate ack row).
|
||||
#
|
||||
# 36777 R-coded rows resolve to 339 unique claims (338 still in
|
||||
# SUBMITTED + 1 already in REJECTED). Grouping in SQL keeps the
|
||||
# in-Python loop small AND emits exactly 1 audit event per claim.
|
||||
rows = session.execute(
|
||||
select(
|
||||
db_mod.Claim.id,
|
||||
db_mod.Claim.state,
|
||||
func.min(db_mod.ClaimAck.set_control_number).label("scn"),
|
||||
func.min(db_mod.ClaimAck.ak2_index).label("ak2"),
|
||||
func.min(db_mod.Ack.ack_code).label("ack_code"),
|
||||
)
|
||||
.join(db_mod.ClaimAck, db_mod.ClaimAck.claim_id == db_mod.Claim.id)
|
||||
.join(db_mod.Ack, db_mod.Ack.id == db_mod.ClaimAck.ack_id)
|
||||
.where(db_mod.ClaimAck.set_accept_reject_code == "R")
|
||||
.where(db_mod.ClaimAck.claim_id.is_not(None))
|
||||
.group_by(db_mod.Claim.id, db_mod.Claim.state)
|
||||
).all()
|
||||
|
||||
matched = 0
|
||||
already = 0
|
||||
errors = 0
|
||||
for (claim_id, current_state, scn, ak2_idx, ack_code) in rows:
|
||||
if current_state == db_mod.ClaimState.REJECTED:
|
||||
already += 1
|
||||
continue
|
||||
|
||||
if dry_run:
|
||||
matched += 1
|
||||
continue
|
||||
|
||||
claim = session.get(db_mod.Claim, claim_id)
|
||||
if claim is None:
|
||||
errors += 1
|
||||
continue
|
||||
|
||||
claim.state = db_mod.ClaimState.REJECTED
|
||||
claim.state_changed_at = now
|
||||
claim.rejected_at = now
|
||||
claim.rejection_reason = (
|
||||
f"999 AK5={ack_code or 'R'} SCN={scn} ak2={ak2_idx}"
|
||||
)
|
||||
# Mirror the 999 SET-level reject into the payer-rejected
|
||||
# lane so the Inbox sees it as a Payer-Rejected claim too.
|
||||
claim.payer_rejected_at = now
|
||||
claim.payer_rejected_reason = f"999 SET-level reject at SCN={scn}"
|
||||
claim.payer_rejected_status_code = "R"
|
||||
|
||||
append_event(session, AuditEvent(
|
||||
event_type="claim.rejected",
|
||||
entity_type="claim",
|
||||
entity_id=claim_id,
|
||||
payload={"source": "backfill-999-rejections",
|
||||
"scn": scn, "ak2_index": ak2_idx,
|
||||
"ack_code": ack_code},
|
||||
actor=actor,
|
||||
))
|
||||
matched += 1
|
||||
|
||||
if not dry_run:
|
||||
session.commit()
|
||||
|
||||
click.echo(
|
||||
f"matched={matched} already_rejected={already} errors={errors} "
|
||||
f"dry_run={dry_run}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# SP33: `cyclone resubmit-rejected-claims`
|
||||
#
|
||||
# Push the corrected single-claim 837 files to Gainwell's SFTP ToHPE
|
||||
# dir so dzinesco can resubmit the batch. The byte-level
|
||||
# SKCO0 -> CO_TXIX fix is assumed to have already been applied (see
|
||||
# the SP33 plan §4.4 / `docs/ingest/corrected/`). This CLI just walks
|
||||
# the corrected directory, validates each file via the parser, and
|
||||
# uploads via the real SftpClient.
|
||||
#
|
||||
# Idempotent: a file already present on the remote with the same byte
|
||||
# size is skipped (counted in `skipped`). Re-runnable after a partial
|
||||
# failure without re-uploading files that landed.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@main.command("resubmit-rejected-claims")
|
||||
@click.option("--ingest-dir", default="ingest/corrected",
|
||||
show_default=True,
|
||||
help="Root dir holding batch-*-claims/ subfolders of fixed .x12 files.")
|
||||
@click.option("--actor", default="sp33-resubmit",
|
||||
show_default=True,
|
||||
help="Audit-log actor tag for the clearhouse.submitted events.")
|
||||
@click.option("--validate/--no-validate", default=True,
|
||||
help="Parse each file via parse_837 before upload (catches a bad fix).")
|
||||
@click.option("--limit", type=int, default=None,
|
||||
help="Stop after checking this many files (smoke-tests). Counts "
|
||||
"all attempts, not just successful uploads.")
|
||||
@click.option("--reconnect-every", type=int, default=50, show_default=True,
|
||||
help="Reconnect SFTP every N uploads to avoid MOVEit's per-session cap.")
|
||||
def resubmit_rejected_claims(
|
||||
ingest_dir: str,
|
||||
actor: str,
|
||||
validate: bool,
|
||||
limit: int | None,
|
||||
reconnect_every: int,
|
||||
) -> None:
|
||||
"""Upload corrected 837 files to the Gainwell SFTP ToHPE dir (SP33).
|
||||
|
||||
Walks every ``batch-*-claims/*.x12`` under ``--ingest-dir`` (default
|
||||
``./ingest/corrected``), validates each one through ``parse_837``,
|
||||
and uploads via the seeded real-SFTP ``SftpClient``.
|
||||
|
||||
Idempotent: a file already present on the remote with the same
|
||||
byte size is skipped (counted in ``skipped``). Reconnects every
|
||||
``--reconnect-every`` uploads to avoid MOVEit's silent per-session
|
||||
file cap (observed: ~200 puts/session before silent drops with no
|
||||
exception — see SP33 root-cause notes).
|
||||
|
||||
Doesn't mutate claim state — claims stay in REJECTED until a 999
|
||||
ACK confirms Gainwell accepted the resubmit. Emits one
|
||||
``clearhouse.submitted`` audit event per successful upload.
|
||||
"""
|
||||
import time
|
||||
|
||||
from cyclone import db as db_mod
|
||||
from cyclone.audit_log import AuditEvent, append_event
|
||||
from cyclone.parsers.parse_837 import parse as parse_837_text
|
||||
from cyclone.parsers.payer import PayerConfig
|
||||
from cyclone.store import store as cycl_store
|
||||
|
||||
db_mod.init_db()
|
||||
cycl_store.ensure_clearhouse_seeded()
|
||||
clearhouse = cycl_store.get_clearhouse()
|
||||
if clearhouse is None:
|
||||
click.echo("No clearhouse seeded; cannot resolve SFTP block.", err=True)
|
||||
sys.exit(2)
|
||||
sftp_block = clearhouse.sftp_block
|
||||
if sftp_block.stub:
|
||||
click.echo("Clearhouse SFTP block is in stub mode; refusing to upload.", err=True)
|
||||
sys.exit(2)
|
||||
remote_root = sftp_block.paths["outbound"]
|
||||
|
||||
root = Path(ingest_dir).resolve()
|
||||
if not root.exists():
|
||||
click.echo(f"--ingest-dir does not exist: {root}", err=True)
|
||||
sys.exit(2)
|
||||
|
||||
files: list[Path] = []
|
||||
for batch_dir in sorted(root.glob("batch-*-claims")):
|
||||
files.extend(sorted(p for p in batch_dir.glob("*.x12")
|
||||
if not p.name.startswith("._")))
|
||||
if not files:
|
||||
click.echo(f"No .x12 files under {root}", err=True)
|
||||
sys.exit(2)
|
||||
|
||||
click.echo(f"found {len(files)} files under {root}")
|
||||
|
||||
uploaded = 0
|
||||
skipped = 0
|
||||
failed = 0
|
||||
validated = 0
|
||||
payer_cfg = PayerConfig.co_medicaid()
|
||||
start = time.monotonic()
|
||||
last_progress = start
|
||||
|
||||
# One persistent paramiko session per batch, with periodic
|
||||
# reconnect to dodge MOVEit's silent per-session file cap
|
||||
# (~200 puts/session, no exception — see SP33 root-cause notes).
|
||||
import paramiko
|
||||
from cyclone.secrets import get_secret
|
||||
|
||||
def _open_session() -> tuple[paramiko.SSHClient, paramiko.SFTPClient]:
|
||||
pw = get_secret(sftp_block.auth.get("password_keychain_account", ""))
|
||||
ssh = paramiko.SSHClient()
|
||||
ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
|
||||
ssh.connect(
|
||||
sftp_block.host, port=sftp_block.port,
|
||||
username=sftp_block.username, password=pw,
|
||||
timeout=15, banner_timeout=15, auth_timeout=15,
|
||||
)
|
||||
return ssh, ssh.open_sftp()
|
||||
|
||||
def _close_session(ssh: paramiko.SSHClient | None) -> None:
|
||||
if ssh is None:
|
||||
return
|
||||
try: ssh.close()
|
||||
except Exception: pass
|
||||
|
||||
ssh: paramiko.SSHClient | None = None
|
||||
sftp: paramiko.SFTPClient | None = None
|
||||
|
||||
for i, src in enumerate(files, 1):
|
||||
if limit is not None and i > limit:
|
||||
break
|
||||
|
||||
content = src.read_bytes()
|
||||
|
||||
# Validate: parse must succeed AND payer_id must match the
|
||||
# companion-guide CO_TXIX (catches a bad byte-fix early).
|
||||
if validate:
|
||||
try:
|
||||
parsed = parse_837_text(content.decode(), payer_cfg)
|
||||
except Exception as exc: # noqa: BLE001
|
||||
failed += 1
|
||||
click.echo(f"PARSE FAIL {src.name}: {exc.__class__.__name__}: {exc}", err=True)
|
||||
continue
|
||||
mismatch = next(
|
||||
(c for c in parsed.claims if c.payer.id != "CO_TXIX"), None
|
||||
)
|
||||
if mismatch is not None:
|
||||
failed += 1
|
||||
click.echo(
|
||||
f"PAYER MISMATCH {src.name}: payer.id={mismatch.payer.id!r} "
|
||||
f"(expected 'CO_TXIX')", err=True,
|
||||
)
|
||||
continue
|
||||
validated += 1
|
||||
|
||||
local_size = len(content)
|
||||
remote_path = f"{remote_root}/{src.name}"
|
||||
|
||||
attempts = 0
|
||||
ok = False
|
||||
was_skipped = False
|
||||
while attempts < 3:
|
||||
attempts += 1
|
||||
if ssh is None:
|
||||
try:
|
||||
ssh, sftp = _open_session()
|
||||
except Exception as exc: # noqa: BLE001
|
||||
click.echo(
|
||||
f"SFTP CONNECT FAIL attempt {attempts}: "
|
||||
f"{exc.__class__.__name__}: {exc}", err=True,
|
||||
)
|
||||
continue
|
||||
try:
|
||||
# Idempotency check.
|
||||
try:
|
||||
rs = sftp.stat(remote_path) # type: ignore[union-attr]
|
||||
if rs.st_size == local_size:
|
||||
skipped += 1
|
||||
was_skipped = True
|
||||
break
|
||||
except IOError:
|
||||
pass # not on remote yet
|
||||
sftp.put(str(src), remote_path) # type: ignore[union-attr]
|
||||
ok = True
|
||||
break
|
||||
except Exception as exc: # noqa: BLE001
|
||||
# Connection died — drop the session and let the next
|
||||
# attempt reopen.
|
||||
_close_session(ssh)
|
||||
ssh, sftp = None, None
|
||||
if attempts >= 3:
|
||||
click.echo(
|
||||
f"UPLOAD FAIL {src.name}: "
|
||||
f"{exc.__class__.__name__}: {exc}", err=True,
|
||||
)
|
||||
continue
|
||||
|
||||
if not ok and not was_skipped:
|
||||
failed += 1
|
||||
continue
|
||||
|
||||
# Audit + reconnect cadence apply only to real uploads.
|
||||
if not was_skipped:
|
||||
# Audit (best-effort; if the DB is unavailable we still
|
||||
# keep the file on the wire).
|
||||
try:
|
||||
with db_mod.SessionLocal()() as session:
|
||||
append_event(session, AuditEvent(
|
||||
event_type="clearhouse.submitted",
|
||||
entity_type="claim_file",
|
||||
entity_id=src.name,
|
||||
payload={"remote_path": remote_path,
|
||||
"source": "resubmit-rejected-claims",
|
||||
"size": local_size},
|
||||
actor=actor,
|
||||
))
|
||||
session.commit()
|
||||
except Exception as exc: # noqa: BLE001
|
||||
click.echo(
|
||||
f"audit-log write failed for {src.name}: "
|
||||
f"{exc.__class__.__name__}: {exc}", err=True,
|
||||
)
|
||||
|
||||
uploaded += 1
|
||||
|
||||
# Reconnect periodically to dodge MOVEit's per-session cap.
|
||||
if uploaded % reconnect_every == 0:
|
||||
click.echo(f"reconnecting (after {uploaded} uploads)", err=True)
|
||||
_close_session(ssh)
|
||||
ssh, sftp = None, None
|
||||
|
||||
# Progress every 10s of wall-clock (or at end).
|
||||
now = time.monotonic()
|
||||
if now - last_progress >= 10 or i == len(files):
|
||||
elapsed = now - start
|
||||
rate = uploaded / elapsed if elapsed else 0
|
||||
click.echo(
|
||||
f"progress {i}/{len(files)} uploaded={uploaded} "
|
||||
f"skipped={skipped} failed={failed} rate={rate:.2f}/s "
|
||||
f"elapsed={elapsed:.1f}s", err=True,
|
||||
)
|
||||
last_progress = now
|
||||
|
||||
# Tear down the long-lived session if one is still open.
|
||||
if ssh is not None:
|
||||
_close_session(ssh)
|
||||
|
||||
elapsed = time.monotonic() - start
|
||||
click.echo(
|
||||
f"DONE uploaded={uploaded} skipped={skipped} failed={failed} "
|
||||
f"validated={validated} files_total={len(files)} elapsed={elapsed:.1f}s"
|
||||
)
|
||||
|
||||
|
||||
@main.group()
|
||||
def backup() -> None:
|
||||
"""Encrypted DB backup management (SP17)."""
|
||||
@@ -824,3 +1189,7 @@ def pull_inbound(
|
||||
click.echo("download errors:", err=True)
|
||||
for e in summary["download_errors"]:
|
||||
click.echo(f" {e}", err=True)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
|
||||
@@ -90,7 +90,9 @@ def handle(
|
||||
.first()
|
||||
)
|
||||
rejection_result = apply_999_rejections(
|
||||
session, result, claim_lookup=_lookup,
|
||||
session, result,
|
||||
claim_lookup=_lookup,
|
||||
batch_envelope_index=batch_index,
|
||||
)
|
||||
if rejection_result.matched:
|
||||
for cid in rejection_result.matched:
|
||||
|
||||
@@ -33,42 +33,63 @@ def apply_999_rejections(
|
||||
parsed_999,
|
||||
*,
|
||||
claim_lookup: Callable[[str], Claim | None],
|
||||
batch_envelope_index: dict[str, list[str]] | None = None,
|
||||
) -> Apply999Result:
|
||||
"""For each set response with code R or E, look up the matching claim and
|
||||
"""For each set response with code R, E, or X, look up the matching claim and
|
||||
move it to REJECTED. Idempotent on already-rejected claims.
|
||||
|
||||
Args:
|
||||
session: SQLAlchemy session.
|
||||
parsed_999: a ParseResult999 (or any object with .set_responses).
|
||||
claim_lookup: callable from patient_control_number → Claim or None.
|
||||
Legacy fallback; rarely hits when batch_envelope_index is present.
|
||||
batch_envelope_index: SP33 — mapping from SET control_number (the 837
|
||||
envelope's ST02) to list of Claim.id for the claims in that SET.
|
||||
Mirrors the SP28 fix in apply_999_acceptances so SET-level
|
||||
rejections correctly cascade across every claim under the SET.
|
||||
|
||||
Returns:
|
||||
Apply999Result with lists of matched claim ids and orphan PCNs.
|
||||
"""
|
||||
result = Apply999Result()
|
||||
now = datetime.now(timezone.utc)
|
||||
index = batch_envelope_index or {}
|
||||
|
||||
for sr in parsed_999.set_responses:
|
||||
code = sr.set_accept_reject.code
|
||||
if code not in ("R", "E", "X"):
|
||||
continue
|
||||
|
||||
claim = claim_lookup(sr.set_control_number)
|
||||
if claim is None:
|
||||
result.orphans.append(sr.set_control_number)
|
||||
continue
|
||||
# SP33: prefer batch_envelope_index (SCN -> [claim_id]) so a SET-level
|
||||
# rejection correctly flips every claim in the SET. Fall back to
|
||||
# the legacy claim_lookup when the index is empty for this SCN.
|
||||
candidate_ids = index.get(sr.set_control_number, []) or []
|
||||
claims_to_reject: list[Claim] = []
|
||||
if candidate_ids:
|
||||
claims_to_reject = (
|
||||
session.query(Claim)
|
||||
.filter(Claim.id.in_(candidate_ids))
|
||||
.all()
|
||||
)
|
||||
else:
|
||||
legacy = claim_lookup(sr.set_control_number)
|
||||
if legacy is not None:
|
||||
claims_to_reject = [legacy]
|
||||
else:
|
||||
result.orphans.append(sr.set_control_number)
|
||||
continue
|
||||
|
||||
if claim.state == ClaimState.REJECTED:
|
||||
# Idempotent: don't double-mutate.
|
||||
continue
|
||||
|
||||
claim.state = ClaimState.REJECTED
|
||||
claim.state_changed_at = now
|
||||
claim.rejected_at = now
|
||||
claim.rejection_reason = _build_reason(
|
||||
code, len(sr.segment_errors or [])
|
||||
)
|
||||
result.matched.append(claim.id)
|
||||
for claim in claims_to_reject:
|
||||
if claim.state == ClaimState.REJECTED:
|
||||
# Idempotent: don't double-mutate.
|
||||
continue
|
||||
claim.state = ClaimState.REJECTED
|
||||
claim.state_changed_at = now
|
||||
claim.rejected_at = now
|
||||
claim.rejection_reason = _build_reason(
|
||||
code, len(sr.segment_errors or [])
|
||||
)
|
||||
result.matched.append(claim.id)
|
||||
|
||||
if result.matched or result.orphans:
|
||||
session.commit()
|
||||
|
||||
@@ -66,8 +66,8 @@ class PayerConfig(BaseModel):
|
||||
# Lenient in v1 — see spec §9 R031.
|
||||
require_ref_g1_for_adjustments=False,
|
||||
allowed_bht06={"CH"},
|
||||
payer_id="SKCO0",
|
||||
payer_name="COHCPF",
|
||||
payer_id="CO_TXIX",
|
||||
payer_name="CO_TXIX",
|
||||
no_patient_loop=True,
|
||||
encounter_claim_in_same_batch=False,
|
||||
allowed_facility_qualifiers={"B"},
|
||||
|
||||
+2
-2
@@ -17,7 +17,7 @@ NM1*IL*1*Balliache*Marianela****MI*P060946~
|
||||
N3*1811 PAVILION DR APT 303~
|
||||
N4*Montrose*CO*814016072~
|
||||
DMG*D8*19590223*F~
|
||||
NM1*PR*2*COHCPF*****PI*SKCO0~
|
||||
NM1*PR*2*CO_TXIX*****PI*CO_TXIX~
|
||||
CLM*t991102984o1c1d*85.40***12:B:1*Y*A*Y*Y~
|
||||
REF*G1*3173~
|
||||
HI*ABK:R69~
|
||||
@@ -35,7 +35,7 @@ NM1*IL*1*Barella*Victoria****MI*H582447~
|
||||
N3*1900 Kellie DR~
|
||||
N4*Montrose*CO*814019524~
|
||||
DMG*D8*19570727*F~
|
||||
NM1*PR*2*COHCPF*****PI*SKCO0~
|
||||
NM1*PR*2*CO_TXIX*****PI*CO_TXIX~
|
||||
CLM*t991102984o1c2d*155.76***12:B:1*Y*A*Y*Y~
|
||||
REF*G1*3173~
|
||||
HI*ABK:R69~
|
||||
|
||||
@@ -17,7 +17,7 @@ NM1*IL*1*Balliache*Marianela****MI*P060946~
|
||||
N3*1811 PAVILION DR APT 303~
|
||||
N4*Montrose*CO*814016072~
|
||||
DMG*D8*19590223*F~
|
||||
NM1*PR*2*COHCPF*****PI*SKCO0~
|
||||
NM1*PR*2*CO_TXIX*****PI*CO_TXIX~
|
||||
CLM*t991102984o1c1d*85.40***12:B:1*Y*A*Y*Y~
|
||||
REF*G1*3173~
|
||||
HI*ABK:R69~
|
||||
@@ -36,7 +36,7 @@ NM1*IL*1*Barella*Victoria****MI*H582447~
|
||||
N3*1900 Kellie DR~
|
||||
N4*Montrose*CO*814019524~
|
||||
DMG*D8*19570727*F~
|
||||
NM1*PR*2*COHCPF*****PI*SKCO0~
|
||||
NM1*PR*2*CO_TXIX*****PI*CO_TXIX~
|
||||
CLM*t991102984o1c2d*155.76***12:B:1*Y*A*Y*Y~
|
||||
REF*G1*3173~
|
||||
HI*ABK:R69~
|
||||
|
||||
+1
-1
@@ -17,7 +17,7 @@ NM1*IL*1*Doe*John****MI*ABC123~
|
||||
N3*456 Member St~
|
||||
N4*Denver*CO*80203~
|
||||
DMG*D8*19800101*M~
|
||||
NM1*PR*2*COHCPF*****PI*SKCO0~
|
||||
NM1*PR*2*CO_TXIX*****PI*CO_TXIX~
|
||||
CLM*CLM001*100.00***12:B:1*Y*A*Y*Y~
|
||||
REF*G1*PA123~
|
||||
HI*ABK:Z00~
|
||||
|
||||
@@ -18,6 +18,7 @@ from cyclone import __version__
|
||||
|
||||
|
||||
FIXTURE = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
|
||||
FIXTURE_835 = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
@@ -216,3 +217,124 @@ def test_cors_extra_origins_via_env(client: TestClient, monkeypatch):
|
||||
# Reload once more so the module-level allow-list returns to its
|
||||
# default for any test that imports `cyclone.api` after this one.
|
||||
importlib.reload(api_module)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# SP35: parse-837 input guards (defense in depth against misroute ingest)
|
||||
# --------------------------------------------------------------------------- #
|
||||
#
|
||||
# Before SP35, /api/parse-837 silently accepted any file that had a parseable
|
||||
# ISA envelope. An 835 file dropped on the Upload page while the dropdown
|
||||
# still said "837p" would land in the DB as an empty batch (claims=[]) and a
|
||||
# bogus row on the History tab. SP35 fixes that at three layers:
|
||||
#
|
||||
# 1. Server envelope check: ST*837 (or ST*837P) required, else 400 with
|
||||
# error="Mismatched file kind".
|
||||
# 2. Server empty-claims check: even with the right envelope, if zero CLM
|
||||
# segments were parsed, return 400 with error="No claims parsed"
|
||||
# and DO NOT persist the batch.
|
||||
# 3. UI auto-detect (separate file: src/pages/Upload.test.tsx).
|
||||
#
|
||||
# These tests are the server-layer regression locks. They run against the
|
||||
# TestClient and use the existing fixtures. The 835 fixture has an ST*835
|
||||
# envelope; posting it to /api/parse-837 must surface a 400 and must not
|
||||
# create a BatchRecord.
|
||||
|
||||
|
||||
def test_parse_837_endpoint_rejects_835_input(client: TestClient):
|
||||
"""Uploading an 835 file to /api/parse-837 must fail loudly, not persist.
|
||||
|
||||
Repro for the original bug: user drops an 835 file on the Upload page
|
||||
while the kind dropdown still says "837p" (Upload.tsx default). Before
|
||||
SP35 the endpoint accepted it, ran it through the 837 parser (which
|
||||
found zero CLM segments because the file has none), and persisted a
|
||||
claims=[] batch — a bogus row on the History tab and on
|
||||
/api/batches. SP35 closes the door at the server so the UI bug becomes
|
||||
cosmetic instead of data-corrupting.
|
||||
"""
|
||||
text_835 = FIXTURE_835.read_text()
|
||||
# Sanity check: the fixture really is an 835 file. If this ever flips,
|
||||
# the test would still pass for the wrong reason.
|
||||
assert "ST*835" in text_835, "fixture is no longer ST*835 — update SP35 tests"
|
||||
|
||||
# Snapshot the batch count BEFORE the bad upload so we can assert the
|
||||
# request did NOT persist anything. Using the public /api/batches JSON
|
||||
# endpoint (already exercised by the Dashboard).
|
||||
before = client.get("/api/batches", headers={"Accept": "application/json"}).json()
|
||||
total_before = before.get("total", len(before.get("items", [])))
|
||||
|
||||
resp = client.post(
|
||||
"/api/parse-837",
|
||||
files={"file": ("co_medicaid_835.txt", text_835, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert body["error"] == "Mismatched file kind"
|
||||
assert body["expected"] == "837p"
|
||||
assert body["detected_st"].startswith("835")
|
||||
|
||||
after = client.get("/api/batches", headers={"Accept": "application/json"}).json()
|
||||
total_after = after.get("total", len(after.get("items", [])))
|
||||
assert total_after == total_before, "Server persisted a batch from a 835 file"
|
||||
|
||||
|
||||
def test_parse_837_endpoint_rejects_empty_envelope(client: TestClient):
|
||||
"""Right envelope (ST*837), zero CLM segments → 400 No claims parsed.
|
||||
|
||||
Synthetic input: a complete ISA/GS/ST envelope with a BHT, a closing
|
||||
SE/GE/IEA, and no CLM loops. The 837 parser will tokenize and build
|
||||
the envelope cleanly, then return claims=[]. SP35 must surface this as
|
||||
a 400 with error="No claims parsed" and must not persist a batch.
|
||||
"""
|
||||
# Bare 837 envelope — no HL/CLM loops. A real X12 file with ST*837
|
||||
# but no claims is unusual but possible (e.g. a header-only test file
|
||||
# or a truncated/cancelled run). The right behavior is to reject,
|
||||
# not to silently persist an empty batch.
|
||||
synthetic = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER "
|
||||
"*260617*1937*^*00501*000000001*0*P*:~"
|
||||
"GS*HC*SENDER*RECEIVER*20260706*1937*1*X*005010X222A1~"
|
||||
"ST*837*0001*005010X222A1~"
|
||||
"BHT*0019*00*0001*20260706*1937*CH~"
|
||||
"SE*2*0001~"
|
||||
"GE*1*1~"
|
||||
"IEA*1*000000001~"
|
||||
)
|
||||
assert "ST*837" in synthetic
|
||||
|
||||
before = client.get("/api/batches", headers={"Accept": "application/json"}).json()
|
||||
total_before = before.get("total", len(before.get("items", [])))
|
||||
|
||||
resp = client.post(
|
||||
"/api/parse-837",
|
||||
files={"file": ("empty_837.txt", synthetic, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert body["error"] == "No claims parsed"
|
||||
|
||||
after = client.get("/api/batches", headers={"Accept": "application/json"}).json()
|
||||
total_after = after.get("total", len(after.get("items", [])))
|
||||
assert total_after == total_before, "Server persisted an empty-claims batch"
|
||||
|
||||
|
||||
def test_parse_837_endpoint_happy_path_still_works(client: TestClient):
|
||||
"""Regression guard: real 837 fixture must still parse → 200 with claims.
|
||||
|
||||
Sits next to the new SP35 rejection tests so any future tightening of
|
||||
the guards that accidentally blocks the happy path fails here loudly.
|
||||
"""
|
||||
text = FIXTURE.read_text()
|
||||
resp = client.post(
|
||||
"/api/parse-837",
|
||||
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200, resp.text
|
||||
body = resp.json()
|
||||
assert body.get("summary", {}).get("total_claims", 0) >= 1
|
||||
assert "batch_id" in body and len(body["batch_id"]) == 32
|
||||
|
||||
@@ -167,3 +167,48 @@ class TestInboxPayerRejectedLane:
|
||||
# The rejected lane (999 envelope) must be empty — we haven't
|
||||
# uploaded a 999, so this claim isn't there.
|
||||
assert "c1" not in [c["id"] for c in lanes["rejected"]]
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# SP35: parse-277ca envelope regression lock
|
||||
# --------------------------------------------------------------------------- #
|
||||
#
|
||||
# The 277CA parser already raises CycloneParseError("Expected ST*277 or
|
||||
# ST*277CA, got ST*<other>") when fed a file with the wrong ST envelope
|
||||
# (parse_277ca.py line 298). This regression lock confirms the HTTP
|
||||
# surface converts that error into a 400 (never 200, never 500).
|
||||
|
||||
|
||||
def test_parse_277ca_endpoint_rejects_835_input(client: TestClient):
|
||||
"""Posting an 835 file to /api/parse-277ca must surface 400, not 200."""
|
||||
wrong_kind = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
|
||||
text = wrong_kind.read_text()
|
||||
assert "ST*835" in text # sanity check on the fixture
|
||||
|
||||
resp = client.post(
|
||||
"/api/parse-277ca",
|
||||
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert "error" in body
|
||||
# The parser-level message must survive (mentions "ST" and the
|
||||
# expected vs actual set id).
|
||||
detail = body.get("detail", "")
|
||||
assert "ST" in detail and ("277" in detail or body["error"] == "Parse error"), body
|
||||
|
||||
|
||||
def test_parse_277ca_endpoint_rejects_837_input(client: TestClient):
|
||||
"""Posting an 837P file to /api/parse-277ca must surface 400, not 200."""
|
||||
wrong_kind = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
|
||||
text = wrong_kind.read_text()
|
||||
assert "ST*837" in text # sanity check
|
||||
|
||||
resp = client.post(
|
||||
"/api/parse-277ca",
|
||||
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
assert "error" in resp.json()
|
||||
|
||||
@@ -13,6 +13,7 @@ from cyclone.store import store as global_store
|
||||
|
||||
FIXTURE = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
|
||||
UNBALANCED = Path(__file__).parent / "fixtures" / "unbalanced_835.txt"
|
||||
FIXTURE_837P = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
@@ -337,3 +338,117 @@ def test_prodfile_round_trip_persists_separately(client: TestClient):
|
||||
# No duplicate PCNs survived the dedup; sanity check on persistence.
|
||||
pcns = [r["claimId"] for r in all_remits if r["claimId"]]
|
||||
assert len(pcns) == len(set(pcns)), "duplicate PCN across batches would be a persistence bug"
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# SP35: parse-835 input guards (mirror of the parse-837 guards)
|
||||
# --------------------------------------------------------------------------- #
|
||||
#
|
||||
# Before SP35, /api/parse-835 had the same silent-corruption shape as
|
||||
# /api/parse-837: any file with a parseable ISA envelope was accepted, and
|
||||
# the 835 parser returned claims=[] when the file had no CLP segments.
|
||||
# This produced empty 835 batches and bogus rows on the History tab.
|
||||
# These tests are the server-layer regression locks for the 835 endpoint.
|
||||
|
||||
|
||||
def test_parse_835_endpoint_rejects_837_input(client: TestClient):
|
||||
"""Uploading an 837P file to /api/parse-835 must fail loudly, not persist.
|
||||
|
||||
Repro for the symmetric bug: user drops an 837P file on the Upload page
|
||||
while the dropdown still says "835" (Upload.tsx default). Before SP35 the
|
||||
endpoint accepted it, ran it through the 835 parser (which found zero
|
||||
CLP segments), and persisted an empty claims=[] batch. SP35 closes the
|
||||
door at the server so the UI bug becomes cosmetic instead of
|
||||
data-corrupting.
|
||||
"""
|
||||
text_837p = FIXTURE_837P.read_text()
|
||||
# Sanity check: the fixture really is an 837P file. If this ever flips,
|
||||
# the test would still pass for the wrong reason.
|
||||
assert "ST*837" in text_837p, "fixture is no longer ST*837 — update SP35 tests"
|
||||
|
||||
before = client.get("/api/batches", headers={"Accept": "application/json"}).json()
|
||||
total_before = before.get("total", len(before.get("items", [])))
|
||||
|
||||
resp = client.post(
|
||||
"/api/parse-835",
|
||||
files={"file": ("co_medicaid_837p.txt", text_837p, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert body["error"] == "Mismatched file kind"
|
||||
assert body["expected"] == "835"
|
||||
assert body["detected_st"].startswith("837")
|
||||
|
||||
after = client.get("/api/batches", headers={"Accept": "application/json"}).json()
|
||||
total_after = after.get("total", len(after.get("items", [])))
|
||||
assert total_after == total_before, "Server persisted a batch from a 837P file"
|
||||
|
||||
|
||||
def test_parse_835_endpoint_rejects_empty_envelope(client: TestClient):
|
||||
"""Right envelope (ST*835), zero CLP segments → 400 No claims parsed.
|
||||
|
||||
Synthetic input: a complete ISA/GS/ST envelope with a BPR + TRN, a
|
||||
closing SE/GE/IEA, and no CLP loops. The 835 parser will tokenize
|
||||
and build the envelope cleanly, then return claims=[]. SP35 must
|
||||
surface this as a 400 with error="No claims parsed" and must not
|
||||
persist a batch.
|
||||
"""
|
||||
# Bare 835 envelope — no LX/CLP loops. A real X12 835 with no claims
|
||||
# is unusual but possible (header-only test file or a cancelled run).
|
||||
# The right behavior is to reject, not to silently persist.
|
||||
synthetic = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER "
|
||||
"*260617*1937*^*00501*000000001*0*P*:~"
|
||||
"GS*HP*SENDER*RECEIVER*20260706*1937*1*X*005010X221A1~"
|
||||
"ST*835*0001~"
|
||||
"BPR*I*100.00*C*ACH*CCP*01*021000021*DA*123456*1512345678**01*021000021*DA*123456*20260706~"
|
||||
"TRN*1*0001*1512345678~"
|
||||
"N1*PR*PAYER NAME~"
|
||||
"N3*123 PAYER ST~"
|
||||
"N4*DENVER*CO*80202~"
|
||||
"PER*BL*MEMBER SERVICES*TE*8005551212~"
|
||||
"N1*PE*PAYEE NAME~"
|
||||
"N3*456 PAYEE ST~"
|
||||
"N4*DENVER*CO*80202~"
|
||||
"REF*TJ*123456789~"
|
||||
"SE*9*0001~"
|
||||
"GE*1*1~"
|
||||
"IEA*1*000000001~"
|
||||
)
|
||||
assert "ST*835" in synthetic
|
||||
|
||||
before = client.get("/api/batches", headers={"Accept": "application/json"}).json()
|
||||
total_before = before.get("total", len(before.get("items", [])))
|
||||
|
||||
resp = client.post(
|
||||
"/api/parse-835",
|
||||
files={"file": ("empty_835.txt", synthetic, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert body["error"] == "No claims parsed"
|
||||
|
||||
after = client.get("/api/batches", headers={"Accept": "application/json"}).json()
|
||||
total_after = after.get("total", len(after.get("items", [])))
|
||||
assert total_after == total_before, "Server persisted an empty-claims 835 batch"
|
||||
|
||||
|
||||
def test_parse_835_endpoint_happy_path_still_works(client: TestClient):
|
||||
"""Regression guard: real 835 fixture must still parse → 200 with claims.
|
||||
|
||||
Sits next to the new SP35 rejection tests so any future tightening of
|
||||
the guards that accidentally blocks the happy path fails here loudly.
|
||||
"""
|
||||
text = FIXTURE.read_text()
|
||||
resp = client.post(
|
||||
"/api/parse-835",
|
||||
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200, resp.text
|
||||
body = resp.json()
|
||||
assert body.get("summary", {}).get("total_claims", 0) >= 1
|
||||
|
||||
@@ -140,3 +140,54 @@ def test_get_ack_404_for_missing(client: TestClient) -> None:
|
||||
"""GET /api/acks/{id} returns 404 for a missing id (not 500)."""
|
||||
resp = client.get("/api/acks/9999", headers={"Accept": "application/json"})
|
||||
assert resp.status_code == 404
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# SP35: parse-999 envelope regression lock
|
||||
# --------------------------------------------------------------------------- #
|
||||
#
|
||||
# The 999 parser already raises CycloneParseError("No AK9 (Functional Group
|
||||
# Response Status) segment found") when fed a non-999 file (parse_999.py
|
||||
# line 290). This regression lock confirms the HTTP surface converts that
|
||||
# error into a 400 (never 200, never 500) so a misroute upload fails loudly
|
||||
# instead of silently creating a corrupt ack row.
|
||||
|
||||
|
||||
def test_parse_999_endpoint_rejects_837_input(client: TestClient):
|
||||
"""Posting an 837P file to /api/parse-999 must surface 400, not 200.
|
||||
|
||||
Before SP35, the 999 parser's envelope guard (no AK9) was already
|
||||
strict at the parser level. This test makes the HTTP contract
|
||||
explicit: a wrong-kind file POSTed to the 999 endpoint MUST come
|
||||
back as 400, not as 200 with an empty ack.
|
||||
"""
|
||||
wrong_kind = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
|
||||
text = wrong_kind.read_text()
|
||||
assert "ST*837" in text # sanity check on the fixture
|
||||
|
||||
resp = client.post(
|
||||
"/api/parse-999",
|
||||
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert "error" in body
|
||||
# The parser-level message must survive (not be replaced by a generic
|
||||
# "Internal server error" or similar).
|
||||
assert "AK9" in body.get("detail", "") or "Parse" in body["error"], body
|
||||
|
||||
|
||||
def test_parse_999_endpoint_rejects_835_input(client: TestClient):
|
||||
"""Posting an 835 file to /api/parse-999 must surface 400, not 200."""
|
||||
wrong_kind = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
|
||||
text = wrong_kind.read_text()
|
||||
assert "ST*835" in text # sanity check
|
||||
|
||||
resp = client.post(
|
||||
"/api/parse-999",
|
||||
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
assert "error" in resp.json()
|
||||
|
||||
@@ -163,4 +163,54 @@ def test_list_ta1_acks_newest_first(client: TestClient):
|
||||
assert len(items) == 2
|
||||
# The REJECTED (uploaded second) is first.
|
||||
assert items[0]["ack_code"] == "R"
|
||||
assert items[1]["ack_code"] == "A"
|
||||
assert items[1]["ack_code"] == "A"
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# SP35: parse-ta1 envelope regression lock
|
||||
# --------------------------------------------------------------------------- #
|
||||
#
|
||||
# The TA1 parser already raises CycloneParseError("Expected TA1, got <other>")
|
||||
# when fed a file that doesn't have a TA1 segment as its first payload
|
||||
# segment (parse_ta1.py line 111). This regression lock confirms the HTTP
|
||||
# surface converts that error into a 400 (never 200, never 500). TA1 has
|
||||
# no ST envelope, so the test uses an 837 fixture (which has ISA + GS +
|
||||
# ST*837 but no TA1 segment) to exercise the parser-level guard.
|
||||
|
||||
|
||||
def test_parse_ta1_endpoint_rejects_837_input(client: TestClient):
|
||||
"""Posting an 837P file to /api/parse-ta1 must surface 400, not 200.
|
||||
|
||||
The TA1 envelope has no ST (it's the bare interchange-ack segment),
|
||||
so the wrong-kind check is structural — the parser looks for the TA1
|
||||
segment and raises when it doesn't find one. An 837 file has ISA +
|
||||
GS + ST*837 but no TA1, which triggers that branch.
|
||||
"""
|
||||
wrong_kind = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
|
||||
text = wrong_kind.read_text()
|
||||
assert "ST*837" in text # sanity check on the fixture
|
||||
|
||||
resp = client.post(
|
||||
"/api/parse-ta1",
|
||||
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert "error" in body
|
||||
detail = body.get("detail", "")
|
||||
assert "TA1" in detail or body["error"] == "Parse error", body
|
||||
|
||||
|
||||
def test_parse_ta1_endpoint_rejects_835_input(client: TestClient):
|
||||
"""Posting an 835 file to /api/parse-ta1 must surface 400, not 200."""
|
||||
wrong_kind = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
|
||||
text = wrong_kind.read_text()
|
||||
assert "ST*835" in text # sanity check
|
||||
|
||||
resp = client.post(
|
||||
"/api/parse-ta1",
|
||||
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
assert "error" in resp.json()
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
"""SP33: apply_999_rejections must use batch_envelope_index (mirrors
|
||||
SP28's apply_999_acceptances fix) so SET-level 999 rejections cascade
|
||||
to claim state transitions.
|
||||
|
||||
The pre-SP33 bug: ``claim_lookup(sr.set_control_number)`` passed the
|
||||
SET control_number to a PCN-based lookup, so no claim was ever matched
|
||||
and rejections silently died. SP33 threads the same
|
||||
``batch_envelope_index`` that ``apply_999_acceptances`` already has.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from cyclone.db import ClaimState
|
||||
from cyclone.inbox_state import apply_999_rejections
|
||||
|
||||
|
||||
def _mk_set_response(code: str = "R", scn: str = "991102994"):
|
||||
"""Build a stand-in for ``ParseResult999.set_responses[0]``."""
|
||||
return SimpleNamespace(
|
||||
set_accept_reject=SimpleNamespace(code=code),
|
||||
set_control_number=scn,
|
||||
segment_errors=[],
|
||||
)
|
||||
|
||||
|
||||
def _mk_claim(cid: str = "claim-x", state=ClaimState.SUBMITTED):
|
||||
c = MagicMock()
|
||||
c.id = cid
|
||||
c.state = state
|
||||
return c
|
||||
|
||||
|
||||
def test_set_level_rejection_cascades_via_envelope_index():
|
||||
"""When the SET-level 999 has set_control_number=991102994 and the
|
||||
batch_envelope_index maps it to a list of 3 claim_ids, all 3 should
|
||||
transition to REJECTED.
|
||||
"""
|
||||
session = MagicMock()
|
||||
claims = [_mk_claim(f"c-{i}") for i in range(3)]
|
||||
session.query.return_value.filter.return_value.all.return_value = claims
|
||||
|
||||
parsed = SimpleNamespace(set_responses=[_mk_set_response(code="R", scn="991102994")])
|
||||
index = {"991102994": [c.id for c in claims]}
|
||||
|
||||
result = apply_999_rejections(
|
||||
session, parsed,
|
||||
claim_lookup=lambda _pcn: None,
|
||||
batch_envelope_index=index,
|
||||
)
|
||||
|
||||
assert sorted(result.matched) == sorted(c.id for c in claims)
|
||||
assert result.orphans == []
|
||||
session.commit.assert_called_once()
|
||||
for c in claims:
|
||||
# ClaimState.REJECTED is the lowercase 'rejected' string (the enum
|
||||
# value stored in the DB column via SQLAlchemy Enum type).
|
||||
assert c.state == ClaimState.REJECTED
|
||||
|
||||
|
||||
def test_set_level_rejection_without_index_still_uses_legacy_lookup():
|
||||
"""Backwards compat: when batch_envelope_index is None, the function
|
||||
must fall back to claim_lookup (existing behavior preserved).
|
||||
"""
|
||||
session = MagicMock()
|
||||
parsed = SimpleNamespace(set_responses=[_mk_set_response(code="R", scn="991102994")])
|
||||
legacy_claim = _mk_claim("legacy-claim")
|
||||
|
||||
result = apply_999_rejections(
|
||||
session, parsed,
|
||||
claim_lookup=lambda scn: legacy_claim if scn == "991102994" else None,
|
||||
)
|
||||
|
||||
assert result.matched == ["legacy-claim"]
|
||||
|
||||
|
||||
def test_no_envelope_index_no_match_becomes_orphan():
|
||||
"""Empty index + empty legacy lookup => the SET becomes an orphan."""
|
||||
session = MagicMock()
|
||||
parsed = SimpleNamespace(set_responses=[_mk_set_response(code="R", scn="991102999")])
|
||||
|
||||
result = apply_999_rejections(
|
||||
session, parsed,
|
||||
claim_lookup=lambda _pcn: None,
|
||||
batch_envelope_index={}, # empty index
|
||||
)
|
||||
assert result.matched == []
|
||||
assert "991102999" in result.orphans
|
||||
|
||||
|
||||
def test_accepted_999_does_not_reject():
|
||||
"""An 'A' code must not transition any claim to REJECTED."""
|
||||
session = MagicMock()
|
||||
parsed = SimpleNamespace(set_responses=[_mk_set_response(code="A", scn="991102994")])
|
||||
|
||||
result = apply_999_rejections(
|
||||
session, parsed,
|
||||
claim_lookup=lambda _pcn: None,
|
||||
batch_envelope_index={"991102994": ["claim-x"]},
|
||||
)
|
||||
assert result.matched == []
|
||||
session.query.assert_not_called()
|
||||
session.commit.assert_not_called()
|
||||
|
||||
|
||||
def test_already_rejected_is_idempotent():
|
||||
"""Pre-REJECTED claims in the index are left alone (no second commit)."""
|
||||
session = MagicMock()
|
||||
already = _mk_claim("already-rejected", state=ClaimState.REJECTED)
|
||||
session.query.return_value.filter.return_value.all.return_value = [already]
|
||||
|
||||
parsed = SimpleNamespace(set_responses=[_mk_set_response(code="R", scn="991102994")])
|
||||
result = apply_999_rejections(
|
||||
session, parsed,
|
||||
claim_lookup=lambda _pcn: None,
|
||||
batch_envelope_index={"991102994": ["already-rejected"]},
|
||||
)
|
||||
assert result.matched == []
|
||||
assert already.state == ClaimState.REJECTED # untouched
|
||||
@@ -36,5 +36,5 @@ def test_subscriber_payer_reused_across_claims():
|
||||
result = parse(FIXTURE.read_text(), PayerConfig.co_medicaid())
|
||||
assert result.claims[0].billing_provider.npi == "1881068062"
|
||||
assert result.claims[1].billing_provider.npi == "1881068062"
|
||||
assert result.claims[0].payer.id == "SKCO0"
|
||||
assert result.claims[1].payer.id == "SKCO0"
|
||||
assert result.claims[0].payer.id == "CO_TXIX"
|
||||
assert result.claims[1].payer.id == "CO_TXIX"
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
"""SP33 regression test: ``PayerConfig.co_medicaid()`` emits CO_TXIX.
|
||||
|
||||
The HCPF 837P Companion Guide (June 2025 - Version 2.5) requires
|
||||
``NM1*PR NM108=PI, NM109=CO_TXIX`` for the Payer Name loop (2010BB).
|
||||
The in-code factory historically emitted ``SKCO0`` which Gainwell
|
||||
rejects with ``IK3*NM1*17*2010*8``. This test pins the corrected value.
|
||||
See ``docs/superpowers/specs/2026-07-02-cyclone-co-txix-payer-fix-design.md``.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
|
||||
def test_co_medicaid_factory_emits_CO_TXIX():
|
||||
"""Per HCPF 837P Companion Guide, NM1*PR NM109 must equal CO_TXIX."""
|
||||
from cyclone.parsers.payer import PayerConfig
|
||||
cfg = PayerConfig.co_medicaid()
|
||||
assert cfg.payer_id == "CO_TXIX", (
|
||||
f"co_medicaid().payer_id must be 'CO_TXIX' (HCPF guide), got {cfg.payer_id!r}"
|
||||
)
|
||||
assert cfg.payer_name == "CO_TXIX", (
|
||||
f"co_medicaid().payer_name must be 'CO_TXIX' (matches docs/goodclaim.x12), got {cfg.payer_name!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_skco0_not_present_in_factory():
|
||||
"""Hard guard: SKCO0 must never be emitted by the factory again."""
|
||||
from cyclone.parsers.payer import PayerConfig
|
||||
cfg = PayerConfig.co_medicaid()
|
||||
assert "SKCO0" not in cfg.payer_id, (
|
||||
f"SKCO0 must not appear in payer_id (HCPF guide says CO_TXIX), got {cfg.payer_id!r}"
|
||||
)
|
||||
assert "SKCO0" not in cfg.payer_name, (
|
||||
f"SKCO0 must not appear in payer_name, got {cfg.payer_name!r}"
|
||||
)
|
||||
@@ -18,7 +18,7 @@ def test_parse_minimal_fixture_returns_one_claim():
|
||||
assert claim.subscriber.last_name == "Doe"
|
||||
assert claim.subscriber.first_name == "John"
|
||||
assert claim.subscriber.member_id == "ABC123"
|
||||
assert claim.payer.id == "SKCO0"
|
||||
assert claim.payer.id == "CO_TXIX"
|
||||
assert claim.claim.frequency_code == "1"
|
||||
assert claim.claim.place_of_service == "12"
|
||||
assert claim.claim.prior_auth == "PA123"
|
||||
|
||||
@@ -8,8 +8,8 @@ def test_co_medicaid_defaults():
|
||||
assert cfg.allowed_claim_frequencies == {1, 7, 8}
|
||||
assert cfg.require_ref_g1_for_adjustments is False # lenient in v1
|
||||
assert cfg.allowed_bht06 == {"CH"}
|
||||
assert cfg.payer_id == "SKCO0"
|
||||
assert cfg.payer_name == "COHCPF"
|
||||
assert cfg.payer_id == "CO_TXIX"
|
||||
assert cfg.payer_name == "CO_TXIX"
|
||||
assert cfg.no_patient_loop is True
|
||||
assert cfg.encounter_claim_in_same_batch is False
|
||||
|
||||
|
||||
@@ -5,13 +5,13 @@ The endpoint is the payer-level aggregate that the drill-down UI's
|
||||
denial rate, and the top 5 NPIs by claim volume for one payer_id,
|
||||
cached in-process for 60s.
|
||||
|
||||
The minimal 837P fixture ships one CLM with ``payer_id="SKCO0"``,
|
||||
The minimal 837P fixture ships one CLM with ``payer_id="CO_TXIX"``,
|
||||
charge_amount=100.00; the minimal 835 carries one CLP for the same
|
||||
claim with total_paid=85.00. So ``/api/payers/SKCO0/summary`` returns
|
||||
claim with total_paid=85.00. So ``/api/payers/CO_TXIX/summary`` returns
|
||||
``claim_count >= 1`` after both files are ingested.
|
||||
|
||||
Note: the spec calls this ``payer_id`` (the X12 NM1*PR*PI qualifier,
|
||||
e.g. ``SKCO0``). It is NOT the configured payer name from
|
||||
e.g. ``CO_TXIX``). It is NOT the configured payer name from
|
||||
``config/payers.yaml``. The filter key in the store layer is
|
||||
``Claim.payer_id`` — not the ``payer=`` substring filter used by
|
||||
``/api/claims?payer=...``.
|
||||
@@ -54,7 +54,7 @@ def client() -> TestClient:
|
||||
def seeded_db(client: TestClient):
|
||||
"""Ingest one minimal 837P + one minimal 835.
|
||||
|
||||
Both fixtures carry ``payer_id="SKCO0"`` so the summary endpoint
|
||||
Both fixtures carry ``payer_id="CO_TXIX"`` so the summary endpoint
|
||||
has something to aggregate. ``client`` is yielded back so the
|
||||
test can hit the API on the same TestClient that ingested the
|
||||
fixtures (parses share the per-test SQLite from conftest).
|
||||
@@ -77,11 +77,11 @@ def seeded_db(client: TestClient):
|
||||
|
||||
|
||||
def test_payer_summary_happy_path(seeded_db: TestClient):
|
||||
"""Seeded db has at least one claim for SKCO0 → 200 with the spec shape."""
|
||||
resp = seeded_db.get("/api/payers/SKCO0/summary")
|
||||
"""Seeded db has at least one claim for CO_TXIX → 200 with the spec shape."""
|
||||
resp = seeded_db.get("/api/payers/CO_TXIX/summary")
|
||||
assert resp.status_code == 200, resp.text
|
||||
data = resp.json()
|
||||
assert data["payer_id"] == "SKCO0"
|
||||
assert data["payer_id"] == "CO_TXIX"
|
||||
assert "claim_count" in data
|
||||
assert "billed_total" in data
|
||||
assert "received_total" in data
|
||||
@@ -99,8 +99,8 @@ def test_payer_summary_unknown_payer_returns_404(client: TestClient):
|
||||
|
||||
def test_payer_summary_caches_then_invalidates(seeded_db: TestClient):
|
||||
"""Two back-to-back calls return identical payloads (in-process cache)."""
|
||||
resp1 = seeded_db.get("/api/payers/SKCO0/summary")
|
||||
resp2 = seeded_db.get("/api/payers/SKCO0/summary")
|
||||
resp1 = seeded_db.get("/api/payers/CO_TXIX/summary")
|
||||
resp2 = seeded_db.get("/api/payers/CO_TXIX/summary")
|
||||
assert resp1.status_code == 200
|
||||
assert resp2.status_code == 200
|
||||
assert resp1.json() == resp2.json()
|
||||
@@ -45,7 +45,7 @@ full walker.
|
||||
|
||||
## CO Medicaid specifics
|
||||
|
||||
- Trading partner: `SKCO0` (sender) ↔ `COHCPF` (receiver) on `NM1*PR` / `NM1*40`
|
||||
- Trading partners: `COMEDASSISTPROG` (NM1*40 NM109, 1000B receiver) and `CO_TXIX` (NM1*PR NM109, 2010BB payer). See `docs/goodclaim.x12` for a canonical example and the HCPF 837P Companion Guide for the full segment table.
|
||||
- `CLM05` is a composite of three components: place of service, facility code
|
||||
qualifier, and frequency code (in that order)
|
||||
- `CLM05-1` = place of service (any valid CMS POS code)
|
||||
|
||||
@@ -9,11 +9,14 @@ Financing (HCPF) requires.
|
||||
|
||||
| Role | 837P | 835 |
|
||||
|---|---|---|
|
||||
| Sender | `SKCO0` | (varies; usually the clearinghouse) |
|
||||
| Receiver | `COHCPF` | (varies) |
|
||||
| Sender (submitter, `NM1*41`) | `Dzinesco` (TPID `11525703`) | (varies; usually the clearinghouse) |
|
||||
| Receiver (`NM1*40`) | `COMEDASSISTPROG` | (varies) |
|
||||
| Payer (`NM1*PR`) | `CO_TXIX` (per HCPF 837P Companion Guide) | (varies) |
|
||||
|
||||
These appear in the `NM1*PR` (payer) and `NM1*40` (receiver) segments of
|
||||
the 837P file.
|
||||
the 837P file. The legacy codes `SKCO0` / `COHCPF` are no longer
|
||||
accepted by Gainwell (HCPF error: "2010BB NM109 must equal CO_TXIX or
|
||||
CO_BHA"); see [SP33](../superpowers/specs/2026-07-02-cyclone-co-txix-payer-fix-design.md).
|
||||
|
||||
## dzinesco's TPID (clearinghouse identity)
|
||||
|
||||
@@ -115,7 +118,7 @@ ls -la "./var/sftp/staging/CO XIX/PROD/coxix_prod_11525703/ToHPE/"
|
||||
|
||||
### 837P (claims)
|
||||
|
||||
- `NM1*PR N104 = "SKCO0"` (COHCPF)
|
||||
- `NM1*PR NM108 = "PI"`, `NM1*PR NM109 = "CO_TXIX"` (per HCPF 837P Companion Guide, June 2025 — Version 2.5). dzinesco submits against this code under dzinesco TPID `11525703`. The legacy trading-partner ID `SKCO0` is no longer accepted as the payer identifier.
|
||||
|
||||
### 835 (remittance)
|
||||
|
||||
|
||||
@@ -0,0 +1,768 @@
|
||||
# SP33 — Co TXIX Payer Fix Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use `superpowers:executing-plans` (Tyler picked inline execution for SP33, single-operator night-shift). Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Make dzinesco's CO Medicaid 837P submissions emit `NM1*PR*...*PI*CO_TXIX` (per HCPF companion guide), cascade existing 999-rejections into `claims.state=REJECTED`, backfill the 338 rejected claims, and resubmit corrected 837s to Gainwell SFTP — all in one tonight.
|
||||
|
||||
**Architecture:** A 2-line payer-config edit + a 3-line `apply_999_rejections` signature fix unlock the cascade. Two new `cyclone.cli` subcommands do the backfill and the resubmit. The doc updates align the in-repo reference with the authoritative HCPF companion guide Tyler pasted. Atomic single merge into `main`.
|
||||
|
||||
**Tech Stack:** Python 3.11, SQLAlchemy + SQLite/SQLCipher, paramiko (SFTP), pytest. Frontend untouched.
|
||||
|
||||
**Spec:** [`docs/superpowers/specs/2026-07-02-cyclone-co-txix-payer-fix-design.md`](../specs/2026-07-02-cyclone-co-txix-payer-fix-design.md)
|
||||
|
||||
**Authoritative source:** CO HCPF "Health Care Claim Professional (837) Transaction Standard Companion Guide" (June 2025 – Version 2.5). Loop 2010BB NM1 Payer Name says `NM108=PI`, `NM109=CO_TXIX` — confirmed by Tyler via chat 2026-07-02. In-repo reference for shape: [`docs/goodclaim.x12`](../../goodclaim.x12).
|
||||
|
||||
---
|
||||
|
||||
## File structure
|
||||
|
||||
| Path | Action | Responsibility |
|
||||
|---|---|---|
|
||||
| `backend/src/cyclone/parsers/payer.py` | modify (lines 69-70) | `co_medicaid()` factory emits CO_TXIX |
|
||||
| `backend/src/cyclone/inbox_state.py` | modify (`apply_999_rejections` signature) | Cascade fix w/ batch_envelope_index |
|
||||
| `backend/src/cyclone/handlers/handle_999.py` | modify (caller) | Pass batch_envelope_index through |
|
||||
| `backend/src/cyclone/cli.py` | extend (2 new subcommands) | backfill-999-rejections, resubmit-rejected-claims |
|
||||
| `backend/tests/test_apply_999_rejections.py` | create | Cascade-fix coverage |
|
||||
| `backend/tests/test_serialize_837.py` | extend | Round-trip with CO_TXIX |
|
||||
| `docs/reference/837p.md` | modify (line 48) | Doc update |
|
||||
| `docs/reference/co-medicaid.md` | modify (line 118) | Doc update |
|
||||
|
||||
## Task 0: Pre-flight + branch
|
||||
|
||||
**Files:** none (git only)
|
||||
|
||||
- [ ] **Step 1: Verify clean tree**
|
||||
|
||||
Run: `git status --short`
|
||||
Expected: only the spec commit `2e7ad47 docs(spec): SP33 CO TXIX payer fix design`.
|
||||
|
||||
- [ ] **Step 2: Create branch off main**
|
||||
|
||||
Run: `git checkout -b sp33-co-txix-payer-fix main`
|
||||
Expected: `Switched to a new branch 'sp33-co-txix-payer-fix'` (already on `main` at `2e7ad47` or `dfd6542+`).
|
||||
|
||||
- [ ] **Step 3: Confirm Docker backend is up**
|
||||
|
||||
Run: `docker ps --filter name=cyclone-backend-1 --format '{{.Names}} {{.Status}}'`
|
||||
Expected: `cyclone-backend-1 Up X minutes (healthy)`.
|
||||
|
||||
## Task 1: Fix `co_medicaid()` factory
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/src/cyclone/parsers/payer.py:69-70`
|
||||
|
||||
- [ ] **Step 1: Read the current block (sanity)**
|
||||
|
||||
Run: `sed -n '67,72p' backend/src/cyclone/parsers/payer.py`
|
||||
Expected output:
|
||||
|
||||
```
|
||||
allowed_bht06={"CH"},
|
||||
payer_id="SKCO0",
|
||||
payer_name="COHCPF",
|
||||
no_patient_loop=True,
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Apply the fix (2 lines)**
|
||||
|
||||
`search_replace` exact:
|
||||
|
||||
OLD:
|
||||
```
|
||||
payer_id="SKCO0",
|
||||
payer_name="COHCPF",
|
||||
```
|
||||
|
||||
NEW:
|
||||
```
|
||||
payer_id="CO_TXIX",
|
||||
payer_name="CO_TXIX",
|
||||
```
|
||||
|
||||
(Drop the bad stale-comment line above it too — the `co_medicaid()` docstring still says "Source: `docs/companionguides/837p.md`" which is wrong; leave the docstring alone in this increment, ship only the data-fix tonight, docstring cleanup is a separate clean-up.)
|
||||
|
||||
- [ ] **Step 3: Verify the diff**
|
||||
|
||||
Run: `git diff backend/src/cyclone/parsers/payer.py`
|
||||
Expected: a 2-line change, exactly the two `payer_id`/`payer_name` lines.
|
||||
|
||||
- [ ] **Step 4: Add test**
|
||||
|
||||
Create: `backend/tests/test_co_medicaid_payer_id_emits_CO_TXIX.py`:
|
||||
|
||||
```python
|
||||
"""SP33 regression test: ``PayerConfig.co_medicaid()`` emits CO_TXIX."""
|
||||
from __future__ import annotations
|
||||
|
||||
|
||||
def test_co_medicaid_factory_emits_CO_TXIX():
|
||||
"""Per HCPF 837P Companion Guide, NM1*PR NM109 must equal CO_TXIX."""
|
||||
from cyclone.parsers.payer import PayerConfig
|
||||
cfg = PayerConfig.co_medicaid()
|
||||
assert cfg.payer_id == "CO_TXIX", (
|
||||
f"co_medicaid().payer_id must be 'CO_TXIX' (HCPF guide), got {cfg.payer_id!r}"
|
||||
)
|
||||
assert cfg.payer_name == "CO_TXIX", (
|
||||
f"co_medicaid().payer_name must be 'CO_TXIX' (matches docs/goodclaim.x12), got {cfg.payer_name!r}"
|
||||
)
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run the test in the container (since the venv on host isn't trustworthy)**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
docker exec cyclone-backend-1 bash -c "cd /var/lib/cyclone && .venv/bin/pytest tests/test_co_medicaid_payer_id_emits_CO_TXIX.py -v"
|
||||
```
|
||||
|
||||
Expected: 1 passed.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add backend/src/cyclone/parsers/payer.py backend/tests/test_co_medicaid_payer_id_emits_CO_TXIX.py
|
||||
git commit -m "feat(sp33): PayerConfig.co_medicaid() emits CO_TXIX"
|
||||
```
|
||||
|
||||
## Task 2: Fix `apply_999_rejections` cascade
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/src/cyclone/inbox_state.py` (`apply_999_rejections` signature)
|
||||
- Modify: `backend/src/cyclone/handlers/handle_999.py:88-99` (caller)
|
||||
|
||||
- [ ] **Step 1: Read current function**
|
||||
|
||||
Run: `sed -n '44,90p' backend/src/cyclone/inbox_state.py`
|
||||
Expected: see `claim = claim_lookup(sr.set_control_number)` at ~line 56.
|
||||
|
||||
- [ ] **Step 2: Apply signature + body change**
|
||||
|
||||
OLD (the full function body, lines 44-69):
|
||||
|
||||
```python
|
||||
def apply_999_rejections(
|
||||
session: Session,
|
||||
parsed_999,
|
||||
*,
|
||||
claim_lookup: Callable[[str], Claim | None],
|
||||
) -> Apply999Result:
|
||||
"""For each set response with code R or E, look up the matching claim and
|
||||
move it to REJECTED. Idempotent on already-rejected claims.
|
||||
|
||||
Args:
|
||||
session: SQLAlchemy session.
|
||||
parsed_999: a ParseResult999 (or any object with .set_responses).
|
||||
claim_lookup: callable from patient_control_number → Claim or None.
|
||||
|
||||
Returns:
|
||||
Apply999Result with lists of matched claim ids and orphan PCNs.
|
||||
"""
|
||||
result = Apply999Result()
|
||||
now = datetime.now(timezone.utc)
|
||||
|
||||
for sr in parsed_999.set_responses:
|
||||
code = sr.set_accept_reject.code
|
||||
if code not in ("R", "E", "X"):
|
||||
continue
|
||||
|
||||
claim = claim_lookup(sr.set_control_number)
|
||||
if claim is None:
|
||||
result.orphans.append(sr.set_control_number)
|
||||
continue
|
||||
|
||||
if claim.state == ClaimState.REJECTED:
|
||||
# Idempotent: don't double-mutate.
|
||||
continue
|
||||
|
||||
claim.state = ClaimState.REJECTED
|
||||
claim.state_changed_at = now
|
||||
claim.rejected_at = now
|
||||
claim.rejection_reason = _build_reason(
|
||||
code, len(sr.segment_errors or [])
|
||||
)
|
||||
result.matched.append(claim.id)
|
||||
|
||||
if result.matched or result.orphans:
|
||||
session.commit()
|
||||
|
||||
return result
|
||||
```
|
||||
|
||||
NEW:
|
||||
|
||||
```python
|
||||
def apply_999_rejections(
|
||||
session: Session,
|
||||
parsed_999,
|
||||
*,
|
||||
claim_lookup: Callable[[str], Claim | None],
|
||||
batch_envelope_index: dict[str, list[str]] | None = None,
|
||||
) -> Apply999Result:
|
||||
"""For each set response with code R or E, look up the matching claim and
|
||||
move it to REJECTED. Idempotent on already-rejected claims.
|
||||
|
||||
Args:
|
||||
session: SQLAlchemy session.
|
||||
parsed_999: a ParseResult999 (or any object with .set_responses).
|
||||
claim_lookup: callable from patient_control_number → Claim or None
|
||||
(legacy fallback; rarely hits with the batch_envelope_index path).
|
||||
batch_envelope_index: SP33 — mapping from SET control_number
|
||||
(the 837 envelope's ST02) to claim.id list for claims in that
|
||||
SET. Mirrors apply_999_acceptances so SET-level rejections
|
||||
correctly cascade.
|
||||
|
||||
Returns:
|
||||
Apply999Result with lists of matched claim ids and orphan PCNs.
|
||||
"""
|
||||
result = Apply999Result()
|
||||
now = datetime.now(timezone.utc)
|
||||
index = batch_envelope_index or {}
|
||||
|
||||
for sr in parsed_999.set_responses:
|
||||
code = sr.set_accept_reject.code
|
||||
if code not in ("R", "E", "X"):
|
||||
continue
|
||||
|
||||
# SP33: prefer batch_envelope_index (SCN -> [claim_id]) so a SET-level
|
||||
# rejection correctly flips every claim in the SET. Fall back to
|
||||
# the legacy claim_lookup when the index is empty for this SCN.
|
||||
candidate_ids = index.get(sr.set_control_number, []) or []
|
||||
claims_to_reject = []
|
||||
if candidate_ids:
|
||||
claims_to_reject = (
|
||||
session.query(Claim)
|
||||
.filter(Claim.id.in_(candidate_ids))
|
||||
.all()
|
||||
)
|
||||
else:
|
||||
legacy = claim_lookup(sr.set_control_number)
|
||||
if legacy is not None:
|
||||
claims_to_reject = [legacy]
|
||||
else:
|
||||
result.orphans.append(sr.set_control_number)
|
||||
continue
|
||||
|
||||
for claim in claims_to_reject:
|
||||
if claim.state == ClaimState.REJECTED:
|
||||
# Idempotent: don't double-mutate.
|
||||
continue
|
||||
claim.state = ClaimState.REJECTED
|
||||
claim.state_changed_at = now
|
||||
claim.rejected_at = now
|
||||
claim.rejection_reason = _build_reason(
|
||||
code, len(sr.segment_errors or [])
|
||||
)
|
||||
result.matched.append(claim.id)
|
||||
|
||||
if result.matched or result.orphans:
|
||||
session.commit()
|
||||
|
||||
return result
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Update the caller in handle_999.py**
|
||||
|
||||
OLD (line 88):
|
||||
|
||||
```python
|
||||
rejection_result = apply_999_rejections(
|
||||
session, result, claim_lookup=_lookup,
|
||||
)
|
||||
```
|
||||
|
||||
NEW:
|
||||
|
||||
```python
|
||||
rejection_result = apply_999_rejections(
|
||||
session, result,
|
||||
claim_lookup=_lookup,
|
||||
batch_envelope_index=batch_index,
|
||||
)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Create the cascade test**
|
||||
|
||||
Create: `backend/tests/test_apply_999_rejections.py`:
|
||||
|
||||
```python
|
||||
"""SP33: apply_999_rejections must use batch_envelope_index (mirrors
|
||||
SP28's apply_999_acceptances fix) so SET-level 999 rejections cascade
|
||||
to claim state transitions.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from cyclone.db import Claim, ClaimState
|
||||
from cyclone.inbox_state import apply_999_rejections, Apply999Result
|
||||
|
||||
|
||||
def _mk_set_response(code: str = "R", scn: str = "991102994"):
|
||||
"""Build a stand-in for ParseResult999.set_responses[0]."""
|
||||
return SimpleNamespace(
|
||||
set_accept_reject=SimpleNamespace(code=code),
|
||||
set_control_number=scn,
|
||||
segment_errors=[],
|
||||
)
|
||||
|
||||
|
||||
def _mk_claim(state: str = "SUBMITTED"):
|
||||
c = MagicMock(spec=Claim)
|
||||
c.id = "claim-" + state.lower()
|
||||
c.state = state
|
||||
return c
|
||||
|
||||
|
||||
def test_set_level_rejection_cascades_via_envelope_index():
|
||||
session = MagicMock()
|
||||
session.query.return_value.filter.return_value.all.return_value = [_mk_claim()]
|
||||
|
||||
parsed = SimpleNamespace(
|
||||
set_responses=[_mk_set_response(code="R", scn="991102994")]
|
||||
)
|
||||
index = {"991102994": ["claim-submitted"]}
|
||||
|
||||
result = apply_999_rejections(
|
||||
session, parsed,
|
||||
claim_lookup=lambda _pcn: None,
|
||||
batch_envelope_index=index,
|
||||
)
|
||||
|
||||
assert len(result.matched) == 1
|
||||
assert result.matched[0] == "claim-submitted"
|
||||
assert result.orphans == []
|
||||
session.commit.assert_called_once()
|
||||
|
||||
|
||||
def test_set_level_rejection_without_index_still_uses_legacy_lookup():
|
||||
"""Backwards compat: when batch_envelope_index is None, the function
|
||||
must fall back to claim_lookup (existing behavior preserved)."""
|
||||
session = MagicMock()
|
||||
parsed = SimpleNamespace(
|
||||
set_responses=[_mk_set_response(code="R", scn="991102994")]
|
||||
)
|
||||
legacy_claim = _mk_claim()
|
||||
result = apply_999_rejections(
|
||||
session, parsed,
|
||||
claim_lookup=lambda scn: legacy_claim if scn == "991102994" else None,
|
||||
)
|
||||
|
||||
assert result.matched == ["claim-submitted"]
|
||||
|
||||
|
||||
def test_no_envelope_index_no_match_becomes_orphan():
|
||||
session = MagicMock()
|
||||
parsed = SimpleNamespace(
|
||||
set_responses=[_mk_set_response(code="R", scn="991102999")]
|
||||
)
|
||||
result = apply_999_rejections(
|
||||
session, parsed,
|
||||
claim_lookup=lambda _pcn: None,
|
||||
batch_envelope_index={}, # empty index
|
||||
)
|
||||
assert result.matched == []
|
||||
assert "991102999" in result.orphans
|
||||
|
||||
|
||||
def test_accepted_999_does_not_reject():
|
||||
session = MagicMock()
|
||||
parsed = SimpleNamespace(
|
||||
set_responses=[_mk_set_response(code="A", scn="991102994")]
|
||||
)
|
||||
result = apply_999_rejections(
|
||||
session, parsed,
|
||||
claim_lookup=lambda _pcn: None,
|
||||
batch_envelope_index={"991102994": ["claim-x"]},
|
||||
)
|
||||
assert result.matched == []
|
||||
session.commit.assert_not_called()
|
||||
|
||||
|
||||
def test_already_rejected_is_idempotent():
|
||||
session = MagicMock()
|
||||
already_rejected = _mk_claim(state="REJECTED")
|
||||
session.query.return_value.filter.return_value.all.return_value = [already_rejected]
|
||||
|
||||
parsed = SimpleNamespace(
|
||||
set_responses=[_mk_set_response(code="R", scn="991102994")]
|
||||
)
|
||||
result = apply_999_rejections(
|
||||
session, parsed,
|
||||
claim_lookup=lambda _pcn: None,
|
||||
batch_envelope_index={"991102994": ["claim-rejected"]},
|
||||
)
|
||||
assert result.matched == []
|
||||
# nothing was mutated on the already-rejected claim
|
||||
assert already_rejected.state == "REJECTED"
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run both test files**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
docker exec cyclone-backend-1 bash -c "cd /var/lib/cyclone && .venv/bin/pytest tests/test_apply_999_rejections.py tests/test_co_medicaid_payer_id_emits_CO_TXIX.py -v"
|
||||
```
|
||||
|
||||
Expected: 6 passed (1 from Task 1 + 5 from Task 2).
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add backend/src/cyclone/inbox_state.py backend/src/cyclone/handlers/handle_999.py backend/tests/test_apply_999_rejections.py
|
||||
git commit -m "feat(sp33): apply_999_rejections uses batch_envelope_index"
|
||||
```
|
||||
|
||||
## Task 3: CLI subcommand `backfill-999-rejections`
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/src/cyclone/cli.py` (add subcommand)
|
||||
|
||||
- [ ] **Step 1: Locate the cli.py entry point**
|
||||
|
||||
Run: `grep -n '@click.command\|def cli\|^@' backend/src/cyclone/cli.py | head -20`
|
||||
|
||||
Find an existing subcommand (e.g. `parse-837`) and copy its structure.
|
||||
|
||||
- [ ] **Step 2: Add `backfill-999-rejections` subcommand**
|
||||
|
||||
Insert after an existing subcommand. The exact insertion point depends on the layout — pick a sensible spot. Use this implementation:
|
||||
|
||||
```python
|
||||
@click.command(name="backfill-999-rejections")
|
||||
@click.option("--dry-run", is_flag=True, default=False,
|
||||
help="Print the would-be changes without writing.")
|
||||
@click.option("--actor", default="sp33-backfill",
|
||||
help="Audit log actor for the claim.rejected events.")
|
||||
def backfill_999_rejections(dry_run: bool, actor: str):
|
||||
"""Replay existing 999 rejections onto already-linked claims.
|
||||
|
||||
SP33. Idempotent — claims already in REJECTED state are skipped.
|
||||
|
||||
Walks claim_acks JOIN acks WHERE set_accept_reject_code='R',
|
||||
then sets claims.state='REJECTED' and fills payer_rejected_* fields.
|
||||
"""
|
||||
from sqlalchemy import select
|
||||
from cyclone import db as cycl_db
|
||||
from cyclone.audit_log import AuditEvent, append_event
|
||||
from cyclone.models import Claim as ClaimModel, ClaimAck, Ack
|
||||
|
||||
cycl_db.init_db()
|
||||
with cycl_db.SessionLocal()() as session:
|
||||
rows = session.execute(
|
||||
select(ClaimAck.claim_id, Claim.ack_code, ClaimAck.set_control_number,
|
||||
ClaimAck.set_accept_reject_code, ClaimAck.ak2_index)
|
||||
.join(Ack, Ack.id == ClaimAck.ack_id)
|
||||
.join(ClaimModel, ClaimModel.id == ClaimAck.claim_id)
|
||||
.where(ClaimAck.set_accept_reject_code == "R")
|
||||
).all()
|
||||
matched = already = errors = 0
|
||||
from datetime import datetime, timezone
|
||||
now = datetime.now(timezone.utc)
|
||||
for (claim_id, ack_code, scn, arc, ak2_idx) in rows:
|
||||
claim = session.get(ClaimModel, claim_id)
|
||||
if claim is None:
|
||||
errors += 1
|
||||
continue
|
||||
if claim.state == "REJECTED":
|
||||
already += 1
|
||||
continue
|
||||
if not dry_run:
|
||||
claim.state = "REJECTED"
|
||||
claim.state_changed_at = now
|
||||
claim.rejected_at = now
|
||||
claim.rejection_reason = f"999 AK5={ack_code or 'R'} SCN={scn} ak2={ak2_idx}"
|
||||
claim.payer_rejected_at = now
|
||||
claim.payer_rejected_reason = f"999 SET-level reject at SCN={scn}"
|
||||
claim.payer_rejected_status_code = "R"
|
||||
append_event(session, AuditEvent(
|
||||
event_type="claim.rejected",
|
||||
entity_type="claim",
|
||||
entity_id=claim_id,
|
||||
payload={"source": "backfill-999-rejections", "scn": scn},
|
||||
actor=actor,
|
||||
))
|
||||
matched += 1
|
||||
if not dry_run:
|
||||
session.commit()
|
||||
click.echo(
|
||||
f"matched={matched} already_rejected={already} errors={errors} dry_run={dry_run}"
|
||||
)
|
||||
```
|
||||
|
||||
The exact model class names (`Claim`, `ClaimAck`, `Ack`) and module names (`cyclone.models`, `cyclone.db`, `cyclone.audit_log`) may differ slightly — verify against the existing imports at the top of `cli.py` and adjust.
|
||||
|
||||
- [ ] **Step 3: Sanity check (dry-run)**
|
||||
|
||||
Run: `docker exec cyclone-backend-1 bash -c "cd /var/lib/cyclone && .venv/bin/python -m cyclone.cli backfill-999-rejections --dry-run"`
|
||||
Expected: `matched=338 already_rejected=0 errors=0 dry_run=True` (or similar; the exact number depends on how many unique claim_ids have R-coded links).
|
||||
|
||||
- [ ] **Step 4: Run for real**
|
||||
|
||||
Run: `docker exec cyclone-backend-1 bash -c "cd /var/lib/cyclone && .venv/bin/python -m cyclone.cli backfill-999-rejections"`
|
||||
Expected: `matched=338 already_rejected=0 errors=0 dry_run=False`.
|
||||
|
||||
- [ ] **Step 5: Verify claim states flipped in the DB**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
docker exec cyclone-backend-1 python3 -c "
|
||||
import sqlite3
|
||||
conn = sqlite3.connect('/var/lib/cyclone/db/cyclone.db')
|
||||
cur = conn.cursor()
|
||||
cur.execute('SELECT state, count(*) FROM claims GROUP BY state ORDER BY 2 DESC')
|
||||
for row in cur.fetchall(): print(row)
|
||||
"
|
||||
```
|
||||
|
||||
Expected: `('REJECTED', 341)` and `('SUBMITTED', 334)` (338 newly-rejected + the 3 pre-existing rejected = 341; original 672 minus 338 = 334).
|
||||
|
||||
- [ ] **Step 6: Commit (CLI only — backfill itself is a live op, no commit for that)**
|
||||
|
||||
```bash
|
||||
git add backend/src/cyclone/cli.py
|
||||
git commit -m "feat(sp33): cli backfill-999-rejections"
|
||||
```
|
||||
|
||||
## Task 4: CLI subcommand `resubmit-rejected-claims`
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/src/cyclone/cli.py` (add subcommand)
|
||||
|
||||
- [ ] **Step 1: Add `resubmit-rejected-claims` subcommand**
|
||||
|
||||
```python
|
||||
@click.command(name="resubmit-rejected-claims")
|
||||
@click.option("--payer", default="co_medicaid",
|
||||
help="Payer factory key (default: co_medicaid).")
|
||||
@click.option("--ingest-dir", default="/home/tyler/dev/cyclone/ingest",
|
||||
help="Directory holding batch-*-claims/ subfolders (per Tyler's split).")
|
||||
@click.option("--actor", default="sp33-resubmit",
|
||||
help="Audit log actor for the clearhouse.submitted events.")
|
||||
@click.option("--dry-run", is_flag=True, default=False,
|
||||
help="Replace + validate but do not upload.")
|
||||
def resubmit_rejected_claims(payer: str, ingest_dir: str, actor: str, dry_run: bool):
|
||||
"""Byte-level SKCO0->CO_TXIX fix + SFTP resubmit of rejected claims.
|
||||
|
||||
SP33. For each claims row in REJECTED state, locate the matching single-
|
||||
claim 837 file in ingest_dir/batch-<batch_id>-<N>-claims/*.x12, replace
|
||||
PI*SKCO0 with PI*CO_TXIX in the bytes, validate via parse_837_text, then
|
||||
upload via SftpClient. Idempotent per claim (resubmit_count gates re-run).
|
||||
"""
|
||||
from pathlib import Path
|
||||
import glob
|
||||
from cyclone import db as cycl_db
|
||||
from cyclone.audit_log import AuditEvent, append_event
|
||||
from cyclone.clearhouse import SftpClient
|
||||
from cyclone.parsers.parse_837 import parse_837_text
|
||||
|
||||
cycl_db.init_db()
|
||||
upload_root = "/CO XIX/PROD/coxix_prod_11525703/ToHPE"
|
||||
|
||||
# 1. collect rejected claims
|
||||
with cycl_db.SessionLocal()() as session:
|
||||
from sqlalchemy import select
|
||||
from cyclone.models import Claim as ClaimModel
|
||||
rows = session.execute(
|
||||
select(ClaimModel.id, ClaimModel.batch_id, ClaimModel.patient_control_number,
|
||||
ClaimModel.resubmit_count, ClaimModel.charge_amount)
|
||||
.where(ClaimModel.state == "REJECTED")
|
||||
.order_by(ClaimModel.batch_id, ClaimModel.id)
|
||||
).all()
|
||||
|
||||
# 2. discover candidate files
|
||||
files_by_batch = {}
|
||||
for batch_dir in glob.glob(os.path.join(ingest_dir, "batch-*-*-claims")):
|
||||
batch_id = os.path.basename(batch_dir).split("-")[1] # batch-<id>-<N>-claims
|
||||
files_by_batch[batch_id] = sorted(glob.glob(os.path.join(batch_dir, "*.x12")))
|
||||
|
||||
ok = err = skip = 0
|
||||
for (cid, bid, pcn, prev_resub, charge) in rows:
|
||||
if prev_resub and prev_resub > 0:
|
||||
skip += 1; continue
|
||||
|
||||
candidate_files = files_by_batch.get(bid, [])
|
||||
# Find the file whose CLM01 == this claim's pcn
|
||||
target_path = None
|
||||
for fp in candidate_files:
|
||||
with open(fp, "rb") as fh:
|
||||
txt = fh.read().decode("ascii", errors="replace")
|
||||
if pcn in txt:
|
||||
target_path = fp
|
||||
break
|
||||
if target_path is None:
|
||||
click.echo(f" no-file claim={cid} pcn={pcn}", err=True)
|
||||
err += 1
|
||||
continue
|
||||
|
||||
# Byte-level fix
|
||||
with open(target_path, "rb") as fh:
|
||||
raw = fh.read()
|
||||
if b"PI*SKCO0" not in raw:
|
||||
click.echo(f" no-replace claim={cid} file={os.path.basename(target_path)}", err=True)
|
||||
err += 1; continue
|
||||
fixed = raw.replace(b"PI*SKCO0", b"PI*CO_TXIX")
|
||||
|
||||
# Validate
|
||||
try:
|
||||
parsed = parse_837_text(fixed.decode("ascii"))
|
||||
except Exception as exc:
|
||||
click.echo(f" parse-fail claim={cid} {exc.__class__.__name__}: {exc}", err=True)
|
||||
err += 1; continue
|
||||
|
||||
# Upload
|
||||
remote_name = os.path.basename(target_path)
|
||||
if not dry_run:
|
||||
# Write the fixed bytes to a sibling tmp file for the upload
|
||||
tmp_path = target_path + ".fixed"
|
||||
with open(tmp_path, "wb") as fh:
|
||||
fh.write(fixed)
|
||||
try:
|
||||
SftpClient().put(tmp_path, f"{upload_root}/{remote_name}")
|
||||
finally:
|
||||
os.unlink(tmp_path)
|
||||
claim = session.get(ClaimModel, cid)
|
||||
claim.resubmit_count = (claim.resubmit_count or 0) + 1
|
||||
append_event(session, AuditEvent(
|
||||
event_type="clearhouse.submitted",
|
||||
entity_type="claim",
|
||||
entity_id=cid,
|
||||
payload={"file": remote_name, "scn": parsed.envelope.control_number},
|
||||
actor=actor,
|
||||
))
|
||||
session.commit()
|
||||
click.echo(f" ok claim={cid} file={remote_name}")
|
||||
ok += 1
|
||||
|
||||
click.echo(f"\nDONE ok={ok} err={err} skip={skip} dry_run={dry_run}")
|
||||
```
|
||||
|
||||
Imports needed: `import os` (glob, path). Confirm the `SftpClient` constructor — it may take an SFTP block or a config; check existing usage in `backend/src/cyclone/api.py` (the `/api/clearhouse/submit` endpoint) and mirror it.
|
||||
|
||||
- [ ] **Step 2: Run with --dry-run first**
|
||||
|
||||
Run: `docker exec cyclone-backend-1 bash -c "cd /var/lib/cyclone && CYCLONE_INGEST_DIR=/tmp/ingest_passthrough .venv/bin/python -m cyclone.cli resubmit-rejected-claims --dry-run"`
|
||||
Expected: a line per claim plus a `DONE ok=338 err=0 skip=0 dry_run=True` summary.
|
||||
|
||||
- [ ] **Step 3: Run for real (upload)**
|
||||
|
||||
Mount /home/tyler/dev/cyclone/ingest into the container so the `--ingest-dir` resolves correctly. Easiest: copy the batch-* subdirs into the container:
|
||||
|
||||
```bash
|
||||
docker exec cyclone-backend-1 mkdir -p /tmp/ingest_split
|
||||
tar -C /home/tyler/dev/cyclone/ingest -cf - 'batch-*-*-claims' | docker exec -i cyclone-backend-1 tar -xf - -C /tmp/ingest_split
|
||||
```
|
||||
|
||||
Then: `docker exec cyclone-backend-1 bash -c "cd /var/lib/cyclone && .venv/bin/python -m cyclone.cli resubmit-rejected-claims --ingest-dir /tmp/ingest_split"`
|
||||
|
||||
Expected: `DONE ok=338 err=0 skip=0 dry_run=False`.
|
||||
|
||||
- [ ] **Step 4: Verify the upload landed**
|
||||
|
||||
Connect to SFTP and check the latest 338 files in `/ToHPE/`:
|
||||
|
||||
```bash
|
||||
docker exec cyclone-backend-1 bash -c "
|
||||
import paramiko, time
|
||||
ssh = paramiko.SSHClient(); ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
|
||||
ssh.connect('mft.gainwelltechnologies.com', username='colorado-fts\\\\coxix_prod_11525703',
|
||||
password=\$(cat /run/secrets/cyclone_sftp_password))
|
||||
sftp = ssh.open_sftp()
|
||||
files = sorted(sftp.listdir_attr('/CO XIX/PROD/coxix_prod_11525703/ToHPE'),
|
||||
key=lambda a: a.st_mtime)[-10:]
|
||||
for f in files: print(f.filename, time.ctime(f.st_mtime))
|
||||
"
|
||||
```
|
||||
|
||||
Expected: ~10 837P files dated 2026-07-02 (or now).
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add backend/src/cyclone/cli.py
|
||||
git commit -m "feat(sp33): cli resubmit-rejected-claims"
|
||||
```
|
||||
|
||||
## Task 5: Doc updates
|
||||
|
||||
**Files:**
|
||||
- Modify: `docs/reference/837p.md:48`
|
||||
- Modify: `docs/reference/co-medicaid.md:118`
|
||||
|
||||
- [ ] **Step 1: Update `docs/reference/837p.md` line 48**
|
||||
|
||||
OLD line:
|
||||
```
|
||||
- Trading partner: `SKCO0` (sender) ↔ `COHCPF` (receiver) on `NM1*PR` / `NM1*40`
|
||||
```
|
||||
|
||||
NEW line:
|
||||
```
|
||||
- Trading partners: `COMEDASSISTPROG` (NM1*40 NM109, 1000B receiver) and `CO_TXIX` (NM1*PR NM109, 2010BB payer). See `docs/goodclaim.x12` for a canonical example and the HCPF 837P Companion Guide for the full segment table.
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Update `docs/reference/co-medicaid.md` line 118**
|
||||
|
||||
OLD:
|
||||
```
|
||||
- `NM1*PR N104 = "SKCO0"` (COHCPF)
|
||||
```
|
||||
|
||||
NEW:
|
||||
```
|
||||
- `NM1*PR NM108 = "PI"`, `NM1*PR NM109 = "CO_TXIX"` (per HCPF 837P Companion Guide, June 2025 — Version 2.5). dzinesco submits against this code under dzinesco TPID `11525703`. The legacy trading-partner ID `SKCO0` is no longer accepted as the payer identifier.
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add docs/reference/837p.md docs/reference/co-medicaid.md
|
||||
git commit -m "docs(sp33): update reference docs to CO_TXIX (per HCPF 837P Companion Guide)"
|
||||
```
|
||||
|
||||
## Task 6: Verify + final test run
|
||||
|
||||
**Files:** none (verification only)
|
||||
|
||||
- [ ] **Step 1: Full test suite**
|
||||
|
||||
Run: `docker exec cyclone-backend-1 bash -c "cd /var/lib/cyclone && .venv/bin/pytest -x -q"`
|
||||
Expected: all tests pass. The 4 affected test files (this plan + Spec §6 list) all green.
|
||||
|
||||
- [ ] **Step 2: Smoke-test the live DB dashboard widget**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
curl -fsS http://localhost:8000/api/batches | python3 -m json.tool | head -30
|
||||
```
|
||||
|
||||
Expected: the dashboard reflects the freshly-REJECTED batches (acceptedCount=0 for the 4 affected batches — this is now correct! They were rejected by Gainwell, and the cascade now shows that).
|
||||
|
||||
- [ ] **Step 3: Atomic merge**
|
||||
|
||||
```bash
|
||||
git checkout main
|
||||
git merge --no-ff sp33-co-txix-payer-fix -m "merge: SP33 Co TXIX payer fix + 999 cascade repair into main"
|
||||
git log --oneline -3
|
||||
```
|
||||
|
||||
Expected: a single merge commit `merge: SP33 ...` on top of `dfd6542` (current main tip pre-SP33), with the SP33 branch's commits visible as parents.
|
||||
|
||||
## Acceptance checklist (must all be true before claiming "done")
|
||||
|
||||
- [ ] `cyclone -m cyclone.cli backfill-999-rejections` reports `matched=338 already_rejected=0 errors=0`
|
||||
- [ ] `claims.state` count: REJECTED ≥ 341, SUBMITTED ≤ 334
|
||||
- [ ] 338 new 837 files visible on SFTP `/CO XIX/PROD/cox17_prod_11525703/ToHPE/` with `CO_TXIX` in their bytes
|
||||
- [ ] `claims.resubmit_count > 0` for the 338 claims
|
||||
- [ ] `audit_log` shows 338 `claim.rejected` + 338 `clearhouse.submitted` events with actor=`sp33-*`
|
||||
- [ ] Dashboard "Recent batches" widget for the 4 affected batches shows the same `acceptedCount` it was showing — but now it's accurate (0 rejected by Gainwell, not 0 accepted by Gainwell)
|
||||
|
||||
## Rollback
|
||||
|
||||
If anything breaks and SP33 needs to back out:
|
||||
|
||||
1. `git revert -m 1 <merge-sha>` on `main` — produces a fresh revert commit, preserves history.
|
||||
2. The 2 cli subcommands revert cleanly (pure additions).
|
||||
3. To un-reject the 338 claims after the revert, run a sister SQL: `UPDATE claims SET state='SUBMITTED', rejection_reason=NULL, rejected_at=NULL, payer_rejected_at=NULL, payer_rejected_reason=NULL, payer_rejected_status_code=NULL WHERE resubmit_count > 0;`
|
||||
4. The 338 837 files already on SFTP /ToHPE/ cannot be unsent — Gainwell will pick them up regardless. Discretion required.
|
||||
@@ -0,0 +1,856 @@
|
||||
# SP35 — Parse Input Guards Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Stop the silent-corruption path where dropping an X12 file on the Upload page at default `Kind: 837P` persists an empty `kind='837p'` batch row for an 835 (or any non-837p) file. Fix at both the server (reject bad input, persist nothing) and the UI (auto-flip the `Kind` select when file content disagrees).
|
||||
|
||||
**Architecture:** Layer the fix. The **server guards** (`POST /api/parse-837` and `POST /api/parse-835`) get two checks each — a cheap envelope check (look for the expected `ST*` token in the first 4 KB of the upload) BEFORE `parse(...)`, and an empty-claims check AFTER `parse(...)` and BEFORE `store.add(...)`. The **UI auto-detect** lives in `Upload.tsx`'s `pickFile()` and inspects the first 4 KB via `FileReader.readAsText(f.slice(0, 4096))` to set the `kind` state. Two layers because each is a separate invariant: the server guard is a correctness invariant (any client — UI, curl, future ingestion paths — gets the same response); the UI auto-detect is the operator-experience invariant (the Upload page is "correct by default").
|
||||
|
||||
**Tech Stack:** Python 3.11+ (FastAPI, SQLAlchemy 2.x, pytest), React 18 + TypeScript (Vitest, happy-dom, `@testing-library/react`). No new dependencies. No schema migration. No CLI changes.
|
||||
|
||||
**Spec:** [`docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md`](../specs/2026-07-06-cyclone-parse-input-guards-design.md)
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
| File | Change | Responsibility |
|
||||
|---|---|---|
|
||||
| `backend/src/cyclone/api.py` | Modify | Add envelope + empty-claims guards to `/api/parse-837` (lines ~384-510) and `/api/parse-835` (lines ~570-680). Extract a tiny `_envelope_st_token(text) -> str \| None` helper at module scope so both endpoints share it. No changes to the 999/277CA/TA1 endpoints (parsers are already strict). |
|
||||
| `backend/tests/test_api.py` | Modify | Add `test_parse_837_endpoint_rejects_835_input`, `test_parse_837_endpoint_rejects_empty_envelope`, `test_parse_837_does_not_persist_when_rejected`. |
|
||||
| `backend/tests/test_api_835.py` | Modify | Add `test_parse_835_endpoint_rejects_837_input`, `test_parse_835_endpoint_rejects_empty_envelope`, `test_parse_835_does_not_persist_when_rejected`. |
|
||||
| `backend/tests/test_api_999.py` | Modify | Add `test_parse_999_endpoint_rejects_837_input` regression lock. |
|
||||
| `backend/tests/test_api_277ca.py` | Modify | Add `test_parse_277ca_endpoint_rejects_835_input` regression lock. |
|
||||
| `backend/tests/test_api_ta1.py` | Modify | Add `test_parse_ta1_endpoint_rejects_835_input` regression lock. |
|
||||
| `src/pages/Upload.tsx` | Modify | Add tiny `_detectEdiKind(text: string): "837p" \| "835" \| null` helper at module scope; in `pickFile()`, async-read the first 4 KB and call `_detectEdiKind` to seed `kind` when a definite token is found. |
|
||||
| `src/pages/Upload.test.tsx` | Modify | Add `upload_auto_detect_*` tests (3) covering 837 / 835 / no-token cases. |
|
||||
| `docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md` | Add | Spec, written first. |
|
||||
| `docs/superpowers/plans/2026-07-06-cyclone-parse-input-guards.md` | Add | This plan. |
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Land the spec on `main` (docs only, no implementation)
|
||||
|
||||
**Files:**
|
||||
- Add: `docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md`
|
||||
|
||||
- [ ] **Step 1: Commit the spec on the branch**
|
||||
|
||||
The spec is already written at the path above. Open it for one last review, then:
|
||||
|
||||
```bash
|
||||
git add docs/superpowers/specs/2026-07-06-cyclone-parse-input-guards-design.md
|
||||
git commit -m "docs(spec): SP35 parse-input-guards — defense in depth against misroute silent-corruption"
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Land this plan on the branch**
|
||||
|
||||
The plan is in place at `docs/superpowers/plans/2026-07-06-cyclone-parse-input-guards.md`.
|
||||
|
||||
```bash
|
||||
git add docs/superpowers/plans/2026-07-06-cyclone-parse-input-guards.md
|
||||
git commit -m "docs(plan): SP35 parse-input-guards — server guards + UI auto-detect, TDD-first"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Server-side guard on `/api/parse-837` (TDD)
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/src/cyclone/api.py` (around lines 384-510 for `/api/parse-837`)
|
||||
- Modify: `backend/tests/test_api.py` (append new tests at end)
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Append to `backend/tests/test_api.py`:
|
||||
|
||||
```python
|
||||
# --- SP35: parse-837 input guards ------------------------------------------
|
||||
|
||||
def test_parse_837_endpoint_rejects_835_input(client: TestClient):
|
||||
"""Posting an 835 file to /api/parse-837 returns 400, no batch row."""
|
||||
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
|
||||
text = fixture.read_text()
|
||||
|
||||
pre_count = global_store.list_batches().__len__() if hasattr(global_store, "list_batches") else None
|
||||
resp = client.post(
|
||||
"/api/parse-837",
|
||||
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert body["error"] == "Mismatched file kind"
|
||||
assert body["expected"] == "837p"
|
||||
assert body.get("detected_st", "").startswith("835")
|
||||
# Confirm no batch row was persisted. The simplest assertion is "no
|
||||
# additional claims rows appeared" — list via the existing list endpoint.
|
||||
claims_after = client.get("/api/claims?limit=1").json()["claims"]
|
||||
assert claims_after == []
|
||||
# (Or, if /api/batches exists, query it and assert no new kind='837p'
|
||||
# batch was added for this filename.)
|
||||
|
||||
|
||||
def test_parse_837_endpoint_rejects_empty_envelope(client: TestClient):
|
||||
"""Syntactically valid ISA but no CLM segments → 400 'No claims parsed'."""
|
||||
# A minimal envelope that gets past ISA parsing but produces zero claims.
|
||||
text = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER "
|
||||
"*260706*0243*^*00501*000000001*0*P*:~"
|
||||
"GS*HC*SENDER*RECEIVER*20260706*0243*1*X*005010X222A1~"
|
||||
"ST*837*0001~"
|
||||
"BHT*0019*00*0001*20260706*0243*CH~"
|
||||
"SE*2*0001~"
|
||||
"GE*1*1~"
|
||||
"IEA*1*000000001~"
|
||||
)
|
||||
resp = client.post(
|
||||
"/api/parse-837",
|
||||
files={"file": ("empty.837p", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert body["error"] == "No claims parsed"
|
||||
|
||||
|
||||
def test_parse_837_endpoint_happy_path_still_works(client: TestClient):
|
||||
"""Regression guard — the existing co_medicaid_837p fixture still parses."""
|
||||
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
|
||||
text = fixture.read_text()
|
||||
resp = client.post(
|
||||
"/api/parse-837",
|
||||
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200, resp.text
|
||||
```
|
||||
|
||||
If `/api/claims` doesn't take a `limit` parameter, swap that assertion for whatever the canonical list endpoint is (`/api/batches`, `/api/inbox`, etc.) — the goal is "confirm no new batch/claims row was persisted". Read `src/lib/api.ts` and pick the endpoint the frontend actually calls.
|
||||
|
||||
- [ ] **Step 2: Run the new tests to verify they FAIL**
|
||||
|
||||
```bash
|
||||
cd backend && .venv/bin/pytest tests/test_api.py -k "rejects or happy_path_still_works" -v
|
||||
```
|
||||
|
||||
Expected: the two `rejects_*` tests fail (current code returns 200 for any file with a parseable ISA envelope). The `happy_path_still_works` test passes (regression guard).
|
||||
|
||||
- [ ] **Step 3: Implement the guards in `/api/parse-837`**
|
||||
|
||||
In `backend/src/cyclone/api.py`, at module scope near other helpers (e.g. just below `_resolve_payer`), add:
|
||||
|
||||
```python
|
||||
def _envelope_st_token(text: str, scan_bytes: int = 4096) -> str | None:
|
||||
"""Return the ST01 token from the first ``scan_bytes`` of ``text``.
|
||||
|
||||
Examples: returns "837" for ``ST*837*0001``, "835" for ``ST*835*1001``.
|
||||
Returns ``None`` if no ST segment is found in the scan window.
|
||||
"""
|
||||
head = text[:scan_bytes]
|
||||
for line in head.split("~"):
|
||||
line = line.strip("\r\n ")
|
||||
if line.startswith("ST*"):
|
||||
parts = line.split("*")
|
||||
if len(parts) >= 2:
|
||||
return parts[1]
|
||||
return None
|
||||
```
|
||||
|
||||
(Adapt to use `Optional` instead of `str | None` if the file already imports Python 3.10-style optionals. Read the top of `api.py` for the style.)
|
||||
|
||||
Then in the `parse_837` handler (around line 384), after the `text = raw.decode("utf-8")` block, before the `result = parse(text, ...)` call:
|
||||
|
||||
```python
|
||||
# SP35: envelope kind guard. Reject files whose ST* token doesn't
|
||||
# match the endpoint's expected kind. Two-layer defense: this catches
|
||||
# the obvious misroute (835 dropped on the 837p page); the empty-claims
|
||||
# check below catches the less-obvious case of a syntactically valid
|
||||
# file with no CLM segments.
|
||||
detected = _envelope_st_token(text)
|
||||
if detected is not None and detected != "837":
|
||||
return JSONResponse(
|
||||
status_code=400,
|
||||
content={
|
||||
"error": "Mismatched file kind",
|
||||
"detail": (
|
||||
f"This endpoint expects an 837P file; the uploaded "
|
||||
f"file's envelope declares ST*{detected}*."
|
||||
),
|
||||
"expected": "837p",
|
||||
"detected_st": detected,
|
||||
},
|
||||
)
|
||||
```
|
||||
|
||||
And after the `_has_claim_validation_errors(result)` block — BEFORE the `BatchRecord(...)` + `store.add(...)` block, add:
|
||||
|
||||
```python
|
||||
# SP35: empty-claims guard. If the parser produced zero claims (e.g.
|
||||
# the file is a well-formed 999 or a truncated 837p with no CLM),
|
||||
# refuse to persist a successful-looking batch row.
|
||||
if not result.claims:
|
||||
return JSONResponse(
|
||||
status_code=400,
|
||||
content={
|
||||
"error": "No claims parsed",
|
||||
"detail": (
|
||||
"The parser did not extract any claim segments from this "
|
||||
"file. Confirm the file is a valid 837P professional "
|
||||
"claim with one or more CLM/CLM01 loops."
|
||||
),
|
||||
},
|
||||
)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the new tests to verify they PASS**
|
||||
|
||||
```bash
|
||||
cd backend && .venv/bin/pytest tests/test_api.py -k "rejects or happy_path_still_works" -v
|
||||
```
|
||||
|
||||
Expected: all 3 tests green.
|
||||
|
||||
- [ ] **Step 5: Run the full `/api/parse-837` test surface to verify no regressions**
|
||||
|
||||
```bash
|
||||
cd backend && .venv/bin/pytest tests/test_api.py -k "837" -v
|
||||
```
|
||||
|
||||
Expected: all green (existing happy-path + NDJSON streaming tests + new guards).
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add backend/src/cyclone/api.py backend/tests/test_api.py
|
||||
git commit -m "feat(sp35): add envelope + empty-claims guards to /api/parse-837"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Server-side guard on `/api/parse-835` (mirrored)
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/src/cyclone/api.py` (around lines 570-680 for `/api/parse-835`)
|
||||
- Modify: `backend/tests/test_api_835.py` (append new tests)
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Append to `backend/tests/test_api_835.py`:
|
||||
|
||||
```python
|
||||
# --- SP35: parse-835 input guards ------------------------------------------
|
||||
|
||||
def test_parse_835_endpoint_rejects_837_input(client: TestClient):
|
||||
"""Posting an 837P file to /api/parse-835 returns 400, no batch row."""
|
||||
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
|
||||
text = fixture.read_text()
|
||||
resp = client.post(
|
||||
"/api/parse-835",
|
||||
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert body["error"] == "Mismatched file kind"
|
||||
assert body["expected"] == "835"
|
||||
assert body.get("detected_st", "").startswith("837")
|
||||
|
||||
|
||||
def test_parse_835_endpoint_rejects_empty_envelope(client: TestClient):
|
||||
"""ST*835 envelope with no CLP segments → 400 'No claims parsed'."""
|
||||
text = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER "
|
||||
"*260706*0243*^*00501*000000001*0*P*:~"
|
||||
"GS*HP*SENDER*RECEIVER*20260706*0243*1*X*005010X221A1~"
|
||||
"ST*835*1001~"
|
||||
"BPR*I*0*C*NON*CCP*01*123456789*DA*0000000*20260706~"
|
||||
"TRN*1*000000001*1811725341~"
|
||||
"SE*4*1001~"
|
||||
"GE*1*1~"
|
||||
"IEA*1*000000001~"
|
||||
)
|
||||
resp = client.post(
|
||||
"/api/parse-835",
|
||||
files={"file": ("empty.835", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert body["error"] == "No claims parsed"
|
||||
|
||||
|
||||
def test_parse_835_endpoint_happy_path_still_works(client: TestClient):
|
||||
"""Regression guard — the co_medicaid_835 fixture still parses."""
|
||||
text = FIXTURE.read_text()
|
||||
resp = client.post(
|
||||
"/api/parse-835",
|
||||
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200, resp.text
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the new tests to verify they FAIL**
|
||||
|
||||
```bash
|
||||
cd backend && .venv/bin/pytest tests/test_api_835.py -k "rejects or happy_path_still_works" -v
|
||||
```
|
||||
|
||||
Expected: the two `rejects_*` tests fail. The `happy_path` test passes.
|
||||
|
||||
- [ ] **Step 3: Implement the guards in `/api/parse-835`**
|
||||
|
||||
Mirror the change from Task 2 Step 3, but for the 835 endpoint and `ST*835`:
|
||||
|
||||
In `parse_835_endpoint` (around line 571), after `text = raw.decode("utf-8")`:
|
||||
|
||||
```python
|
||||
# SP35: envelope kind guard. See Task 2 notes.
|
||||
detected = _envelope_st_token(text)
|
||||
if detected is not None and detected != "835":
|
||||
return JSONResponse(
|
||||
status_code=400,
|
||||
content={
|
||||
"error": "Mismatched file kind",
|
||||
"detail": (
|
||||
f"This endpoint expects an 835 file; the uploaded "
|
||||
f"file's envelope declares ST*{detected}*."
|
||||
),
|
||||
"expected": "835",
|
||||
"detected_st": detected,
|
||||
},
|
||||
)
|
||||
```
|
||||
|
||||
After the validator block (~line 635), before the existing `BatchRecord(...)` + `store.add(...)`:
|
||||
|
||||
```python
|
||||
# SP35: empty-claims guard. See Task 2 notes.
|
||||
if not result.claims:
|
||||
return JSONResponse(
|
||||
status_code=400,
|
||||
content={
|
||||
"error": "No claims parsed",
|
||||
"detail": (
|
||||
"The parser did not extract any claim-payment segments "
|
||||
"from this file. Confirm the file is a valid 835 ERA "
|
||||
"remittance with one or more CLP/CLP01 loops."
|
||||
),
|
||||
},
|
||||
)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the new tests to verify they PASS**
|
||||
|
||||
```bash
|
||||
cd backend && .venv/bin/pytest tests/test_api_835.py -v
|
||||
```
|
||||
|
||||
Expected: all tests green (existing 6 + new 3).
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add backend/src/cyclone/api.py backend/tests/test_api_835.py
|
||||
git commit -m "feat(sp35): add envelope + empty-claims guards to /api/parse-835 (mirror)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Regression locks for `/api/parse-999`, `/api/parse-277ca`, `/api/parse-ta1`
|
||||
|
||||
The 999/277CA/TA1 endpoints already reject mismatched input at the parser layer (their parsers raise `CycloneParseError` on missing `AK9` / wrong `ST*` / missing `TA1` segment respectively). SP35 doesn't add any new code to those endpoints — but we add **regression tests** so a future PR that loosens a parser envelope guard gets caught.
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/tests/test_api_999.py`
|
||||
- Modify: `backend/tests/test_api_277ca.py`
|
||||
- Modify: `backend/tests/test_api_ta1.py`
|
||||
|
||||
- [ ] **Step 1: Read each existing test file to learn the import / fixture conventions**
|
||||
|
||||
```bash
|
||||
head -50 /home/tyler/dev/cyclone/backend/tests/test_api_999.py
|
||||
head -50 /home/tyler/dev/cyclone/backend/tests/test_api_277ca.py
|
||||
head -50 /home/tyler/dev/cyclone/backend/tests/test_api_ta1.py
|
||||
```
|
||||
|
||||
Mirror the existing pattern. The test_api_835.py file is the closest template — same fixture imports, same `client` fixture, same `client.post(...)` shape.
|
||||
|
||||
- [ ] **Step 2: Add the regression test to `test_api_999.py`**
|
||||
|
||||
Append:
|
||||
|
||||
```python
|
||||
# --- SP35 regression: 999 endpoint rejects non-999 input -----------------
|
||||
|
||||
def test_parse_999_endpoint_rejects_837_input(client: TestClient):
|
||||
"""Regression lock — the 999 parser must reject 837 input.
|
||||
|
||||
The 999 parser raises ``CycloneParseError("No AK9 (Functional Group
|
||||
Response Status) segment found")`` when the input has no AK9 segment
|
||||
(which an 837 file does not). The endpoint surfaces this as a 400
|
||||
Parse error. This test guards against a future PR that loosens the
|
||||
AK9 requirement.
|
||||
"""
|
||||
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_837p.txt"
|
||||
text = fixture.read_text()
|
||||
resp = client.post(
|
||||
"/api/parse-999",
|
||||
files={"file": ("co_medicaid_837p.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert body["error"] == "Parse error"
|
||||
# The detail message is the parser's own error string — confirm it
|
||||
# mentions AK9 so a future loosen-the-parser PR is loudly caught.
|
||||
assert "AK9" in body["detail"]
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Add the regression test to `test_api_277ca.py`**
|
||||
|
||||
Append:
|
||||
|
||||
```python
|
||||
# --- SP35 regression: 277ca endpoint rejects non-277 input ---------------
|
||||
|
||||
def test_parse_277ca_endpoint_rejects_835_input(client: TestClient):
|
||||
"""Regression lock — the 277CA parser must reject 835 input.
|
||||
|
||||
The 277CA parser raises ``CycloneParseError("Expected ST*277 or
|
||||
ST*277CA, got ST*<other>")`` when the envelope ST doesn't match.
|
||||
This test guards against a future PR that loosens the ST* match.
|
||||
"""
|
||||
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
|
||||
text = fixture.read_text()
|
||||
resp = client.post(
|
||||
"/api/parse-277ca",
|
||||
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert body["error"] == "Parse error"
|
||||
assert "Expected ST*277" in body["detail"]
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Add the regression test to `test_api_ta1.py`**
|
||||
|
||||
Append:
|
||||
|
||||
```python
|
||||
# --- SP35 regression: TA1 endpoint rejects non-TA1 input -----------------
|
||||
|
||||
def test_parse_ta1_endpoint_rejects_835_input(client: TestClient):
|
||||
"""Regression lock — the TA1 parser must reject non-TA1 input.
|
||||
|
||||
The TA1 parser raises ``CycloneParseError("Expected TA1, got ...")``
|
||||
when the first segment after ISA isn't TA1*. This test guards
|
||||
against a future PR that loosens the TA1 sentinel.
|
||||
"""
|
||||
fixture = Path(__file__).parent / "fixtures" / "co_medicaid_835.txt"
|
||||
text = fixture.read_text()
|
||||
resp = client.post(
|
||||
"/api/parse-ta1",
|
||||
files={"file": ("co_medicaid_835.txt", text, "text/plain")},
|
||||
headers={"Accept": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
body = resp.json()
|
||||
assert body["error"] == "Parse error"
|
||||
assert "Expected TA1" in body["detail"]
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run the three new regression tests to verify they PASS on the current code**
|
||||
|
||||
```bash
|
||||
cd backend && .venv/bin/pytest tests/test_api_999.py::test_parse_999_endpoint_rejects_837_input \
|
||||
tests/test_api_277ca.py::test_parse_277ca_endpoint_rejects_835_input \
|
||||
tests/test_api_ta1.py::test_parse_ta1_endpoint_rejects_835_input -v
|
||||
```
|
||||
|
||||
Expected: all 3 pass on the current code (the parsers already reject). If any fail, the parser was looser than expected — file a follow-up bug.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add backend/tests/test_api_999.py backend/tests/test_api_277ca.py backend/tests/test_api_ta1.py
|
||||
git commit -m "test(sp35): regression locks on 999/277ca/ta1 — assert parser envelope guards hold"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 5: Frontend auto-detect in `Upload.tsx` (TDD)
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/pages/Upload.tsx` (lines ~441-444 for `pickFile`, plus a top-level helper)
|
||||
- Modify: `src/pages/Upload.test.tsx` (append new tests; existing file is at `src/pages/Upload.test.tsx` per the sibling rule)
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Append to `src/pages/Upload.test.tsx`:
|
||||
|
||||
```tsx
|
||||
// --- SP35: auto-detect kind from dropped file ----------------------------
|
||||
|
||||
import { Upload } from "./Upload";
|
||||
|
||||
function makeFile(name: string, body: string, type = "text/plain"): File {
|
||||
// happy-dom doesn't ship a File constructor that takes a body — use Blob.
|
||||
return new File([body], name, { type });
|
||||
}
|
||||
|
||||
async function dropFile(container: HTMLElement, file: File) {
|
||||
// Trigger React's onChange handler by dispatching a synthetic change
|
||||
// event on the hidden <input type="file">.
|
||||
const input = container.querySelector('input[type="file"]') as HTMLInputElement;
|
||||
Object.defineProperty(input, "files", { value: [file] });
|
||||
await act(async () => {
|
||||
input.dispatchEvent(new Event("change", { bubbles: true }));
|
||||
});
|
||||
// Auto-detect is async via FileReader; flush microtasks.
|
||||
await act(async () => {
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
});
|
||||
}
|
||||
|
||||
describe("Upload auto-detect (SP35)", () => {
|
||||
it("flips kind to 837p when an 837 file is dropped on default kind", async () => {
|
||||
const file = makeFile(
|
||||
"anything.837p",
|
||||
"ISA*00* *00* *ZZ*SENDER*ZZ*RECEIVER*260706*0243*^*00501*1*0*P*:~"
|
||||
+ "GS*HC*SENDER*RECEIVER*20260706*0243*1*X*005010X222A1~"
|
||||
+ "ST*837*0001~",
|
||||
);
|
||||
const { container, unmount } = renderCard(React.createElement(Upload));
|
||||
// Default kind should be 837p — set explicitly so the test is robust
|
||||
// if the default ever changes.
|
||||
// (Skip the flip-when-already-correct assertion; focus on the 835 case.)
|
||||
await dropFile(container, file);
|
||||
// Assert the kind select now shows the 835 picker. Use the
|
||||
// data-testid or visible label — read existing Upload.test.tsx for
|
||||
// the canonical selector pattern.
|
||||
// (This test asserts the no-op case; the meaningful assertion is in
|
||||
// the 835 test below.)
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("flips kind to 835 when an 835 file is dropped on default 837p", async () => {
|
||||
const file = makeFile(
|
||||
"anything.x12",
|
||||
"ISA*00* *00* *ZZ*SENDER*ZZ*RECEIVER*260706*0243*^*00501*1*0*P*:~"
|
||||
+ "GS*HP*SENDER*RECEIVER*20260706*0243*1*X*005010X221A1~"
|
||||
+ "ST*835*1001~",
|
||||
);
|
||||
const { container, unmount } = renderCard(React.createElement(Upload));
|
||||
await dropFile(container, file);
|
||||
// The Kind select should now read "835 — ERA remittance". Find the
|
||||
// select via accessible role+name.
|
||||
const select = container.querySelector('[id="upload-kind"]');
|
||||
expect(select).toBeTruthy();
|
||||
// The select value flips via Radix Select — read the aria/role
|
||||
// attributes for the visible label, or assert on the internal state
|
||||
// by triggering Parse and verifying the call goes to /api/parse-835.
|
||||
// (See note below — the assertion shape depends on the Radix Select
|
||||
// API; read Upload.tsx for the exact data attrs the Select exposes.)
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("leaves kind unchanged when no ST* token is found", async () => {
|
||||
const file = makeFile(
|
||||
"not-edi.txt",
|
||||
"This file does not look like an EDI document at all. Just plain text.",
|
||||
);
|
||||
const { container, unmount } = renderCard(React.createElement(Upload));
|
||||
await dropFile(container, file);
|
||||
// The Kind select should still read the default. We can verify by
|
||||
// checking that the Parse button stays disabled or by checking the
|
||||
// network call direction on click.
|
||||
unmount();
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
(Adapt the exact selector patterns by reading `src/components/ui/select.tsx` and `src/pages/Upload.tsx`. The existing test file at line 1-80 shows the `createRoot` + `MemoryRouter` style; reuse `renderCard` from there rather than redefining it.)
|
||||
|
||||
- [ ] **Step 2: Run the new tests to verify they FAIL**
|
||||
|
||||
```bash
|
||||
cd /home/tyler/dev/cyclone && npx vitest run src/pages/Upload.test.tsx
|
||||
```
|
||||
|
||||
Expected: the three new tests fail (current code does not auto-detect). Existing tests pass.
|
||||
|
||||
- [ ] **Step 3: Implement the auto-detect in `Upload.tsx`**
|
||||
|
||||
In `src/pages/Upload.tsx`, near the top (after the `formatBytes` helper, around line 86), add:
|
||||
|
||||
```ts
|
||||
function detectEdiKind(text: string): "837p" | "835" | null {
|
||||
// Inspect the first 4 KB for an ST* segment. Return the ST01 token if
|
||||
// we find a recognized kind; null otherwise (file is not recognizable).
|
||||
const head = text.slice(0, 4096);
|
||||
for (const rawLine of head.split("~")) {
|
||||
const line = rawLine.replace(/^[\r\n]+|[\r\n]+$/g, "").trim();
|
||||
if (line.startsWith("ST*")) {
|
||||
const parts = line.split("*");
|
||||
const token = parts[1];
|
||||
if (token === "837") return "837p";
|
||||
if (token === "835") return "835";
|
||||
return null; // recognized ST* but unknown kind
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
async function readFileHead(file: File, scanBytes = 4096): Promise<string> {
|
||||
const blob = file.slice(0, scanBytes);
|
||||
return new Promise((resolve, reject) => {
|
||||
const reader = new FileReader();
|
||||
reader.onload = () => resolve(typeof reader.result === "string" ? reader.result : "");
|
||||
reader.onerror = () => reject(reader.error);
|
||||
reader.readAsText(blob);
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
Then replace `pickFile` (lines 441-444):
|
||||
|
||||
```ts
|
||||
function pickFile(f: File | null) {
|
||||
setFile(f);
|
||||
setStream({ items: [], expectedTotal: null, passed: 0, failed: 0 });
|
||||
if (!f) return;
|
||||
// SP35: auto-detect kind from the file's ST* token. If we can
|
||||
// identify the file as 837P or 835 with confidence, flip the Kind
|
||||
// select so the operator doesn't have to remember to do it manually.
|
||||
// (Manual selection still wins in the sense that the user can flip
|
||||
// back; the auto-detect is the default-by-default behavior.)
|
||||
readFileHead(f).then((head) => {
|
||||
const detected = detectEdiKind(head);
|
||||
if (detected) setKind(detected);
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the new tests to verify they PASS**
|
||||
|
||||
```bash
|
||||
cd /home/tyler/dev/cyclone && npx vitest run src/pages/Upload.test.tsx
|
||||
```
|
||||
|
||||
Expected: all green.
|
||||
|
||||
- [ ] **Step 5: Typecheck + lint**
|
||||
|
||||
```bash
|
||||
cd /home/tyler/dev/cyclone && npm run typecheck && npm run lint
|
||||
```
|
||||
|
||||
Expected: 0 errors.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/pages/Upload.tsx src/pages/Upload.test.tsx
|
||||
git commit -m "feat(sp35): Upload page auto-detects Kind from dropped file's ST* token"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 6: Full verification
|
||||
|
||||
**Files:**
|
||||
- No code changes. Verification only.
|
||||
|
||||
- [ ] **Step 1: Run the full backend pytest suite**
|
||||
|
||||
```bash
|
||||
cd backend && .venv/bin/pytest -q
|
||||
```
|
||||
|
||||
Expected: 0 failures. Every existing test still passes; the 6 new guard tests pass.
|
||||
|
||||
- [ ] **Step 2: Run the full frontend vitest suite**
|
||||
|
||||
```bash
|
||||
cd /home/tyler/dev/cyclone && npm test
|
||||
```
|
||||
|
||||
Expected: 0 failures.
|
||||
|
||||
- [ ] **Step 3: Typecheck + lint (regression)**
|
||||
|
||||
```bash
|
||||
cd /home/tyler/dev/cyclone && npm run typecheck && npm run lint
|
||||
```
|
||||
|
||||
Expected: 0 errors.
|
||||
|
||||
- [ ] **Step 4: Live-stack manual smoke**
|
||||
|
||||
The container is already running at `192.168.0.49:8080`. Reproduce the incident end-to-end:
|
||||
|
||||
```bash
|
||||
# A. login (you'll paste the cookie or POST credentials)
|
||||
curl -s -c /tmp/cookies.txt -X POST http://192.168.0.49:8080/api/auth/login \
|
||||
-H "Content-Type: application/json" \
|
||||
-d '{"username":"<your-username>","password":"<your-password>"}'
|
||||
|
||||
# B. POST the (real, cycled-this-morning) 835 file to the 837p endpoint:
|
||||
curl -s -b /tmp/cookies.txt -X POST http://192.168.0.49:8080/api/parse-837 \
|
||||
-F "file=@/home/tyler/dev/cyclone/ingest/tp11525703-835_M019771179-20260706005516577-1of1.x12;filename=oops.x12" \
|
||||
-H "Accept: application/json"
|
||||
```
|
||||
|
||||
Expected: `400 Mismatched file kind`, body contains `expected: "837p"` and `detected_st: "835"`. Confirm no new `batches` row was persisted:
|
||||
|
||||
```bash
|
||||
docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db \
|
||||
"SELECT COUNT(*) FROM batches WHERE input_filename = 'oops.x12';"
|
||||
```
|
||||
|
||||
Expected: `0` (no new row).
|
||||
|
||||
- [ ] **Step 5: Commit (only if any incidental cleanup)**
|
||||
|
||||
If step 1-4 surfaced unrelated failures, fix them on this branch. Otherwise no commit.
|
||||
|
||||
```bash
|
||||
git status
|
||||
```
|
||||
|
||||
If clean, proceed to Task 7.
|
||||
|
||||
---
|
||||
|
||||
## Task 7: Cleanup of the two bogus batches from the live incident
|
||||
|
||||
**Files:**
|
||||
- No code changes. One SQL command run via `docker exec`.
|
||||
|
||||
- [ ] **Step 1: Sanity check what we're about to delete**
|
||||
|
||||
```bash
|
||||
docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db <<'SQL'
|
||||
SELECT b.id, b.kind, b.input_filename, b.parsed_at,
|
||||
(SELECT COUNT(*) FROM claims WHERE batch_id = b.id) AS claims_n,
|
||||
(SELECT COUNT(*) FROM service_line_payments WHERE batch_id = b.id) AS slp_n,
|
||||
(SELECT COUNT(*) FROM cas_adjustments WHERE batch_id = b.id) AS cas_n,
|
||||
(SELECT COUNT(*) FROM matches WHERE batch_id = b.id) AS match_n,
|
||||
(SELECT COUNT(*) FROM remittances WHERE batch_id = b.id) AS remit_n
|
||||
FROM batches b
|
||||
WHERE b.id IN ('50eb50c16e8e49919d181e9fb90cd435', 'e4692571bc56431e9fcb59ce2c0f9450');
|
||||
SQL
|
||||
```
|
||||
|
||||
Expected: both rows have `claims_n=0`, `slp_n=0`, `cas_n=0`, `match_n=0`, `remit_n=0` (clean to delete).
|
||||
|
||||
- [ ] **Step 2: Delete**
|
||||
|
||||
```bash
|
||||
docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db <<'SQL'
|
||||
DELETE FROM batches WHERE id IN ('50eb50c16e8e49919d181e9fb90cd435', 'e4692571bc56431e9fcb59ce2c0f9450');
|
||||
SQL
|
||||
```
|
||||
|
||||
No expected output on success.
|
||||
|
||||
- [ ] **Step 3: Verify clean state**
|
||||
|
||||
```bash
|
||||
docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db \
|
||||
"SELECT id, kind, input_filename FROM batches WHERE id IN ('50eb50c16e8e49919d181e9fb90cd435', 'e4692571bc56431e9fcb59ce2c0f9450');"
|
||||
```
|
||||
|
||||
Expected: no rows. The good 835 batch (`a9bb632e939040d49b41b6af1a58246f`) is preserved.
|
||||
|
||||
- [ ] **Step 4: Production rollback note in case anything goes sideways**
|
||||
|
||||
The DB volume is `cyclone_cyclone_db`. If the delete needs to be undone, a backup restore is the path: `cyclone backup list` → `cyclone backup restore <id>`. SP17 backs up daily; today's backup should predate the delete. Document this in the PR description if you have any doubt.
|
||||
|
||||
- [ ] **Step 5: No commit** (the SQL ran against the live container, not the repo)
|
||||
|
||||
- [ ] **Step 6: PR description addendum**
|
||||
|
||||
In the SP35 PR description, add a "Production follow-up" section:
|
||||
|
||||
> Manually deleted two orphan `kind='837p'` batch rows from the live DB after the SP landed:
|
||||
> - `50eb50c16e8e49919d181e9fb90cd435` (parsed 2026-07-06 15:31:15 UTC)
|
||||
> - `e4692571bc56431e9fcb59ce2c0f9450` (parsed 2026-07-06 15:31:23 UTC)
|
||||
>
|
||||
> Both rows had `total_claims=0` and zero downstream rows (claims, service_line_payments, cas_adjustments, matches, remittances). The good 835 batch (`a9bb632e939040d49b41b6af1a58246f`) is preserved.
|
||||
|
||||
---
|
||||
|
||||
## Task 8: PR + atomic merge into `main`
|
||||
|
||||
**Files:**
|
||||
- No code changes. PR + merge only.
|
||||
|
||||
- [ ] **Step 1: Push the branch**
|
||||
|
||||
```bash
|
||||
git push -u origin sp35-parse-input-guards
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Open the PR**
|
||||
|
||||
PR title: **`SP35 Parse input guards`**
|
||||
|
||||
PR body should include:
|
||||
- Summary (2-3 lines): "Defense-in-depth fix for the silent-corruption path where dropping a non-837P X12 file on the Upload page at default `Kind: 837P` silently persisted an empty `kind='837p'` batch row. Server-side guards on `/api/parse-837` and `/api/parse-835` reject mismatched and empty files; the Upload page auto-flips the Kind select from the file's `ST*` token."
|
||||
- Test plan: list the 6 new backend tests + 3 new frontend tests by name.
|
||||
- Production follow-up section (Task 7 Step 6).
|
||||
- Out-of-scope notes (the activity-events storm, the 999/277CA/TA1 follow-up).
|
||||
|
||||
- [ ] **Step 3: After approval — atomic merge into `main`**
|
||||
|
||||
```bash
|
||||
git checkout main
|
||||
git merge --no-ff sp35-parse-input-guards -m "merge: SP35 parse-input-guards into main"
|
||||
```
|
||||
|
||||
**No squash, no rebase.** The merge commit is the audit record.
|
||||
|
||||
- [ ] **Step 4: Push the merge**
|
||||
|
||||
```bash
|
||||
git push origin main
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Restart the running containers to pick up the new backend**
|
||||
|
||||
```bash
|
||||
cd /home/tyler/dev/cyclone && docker compose up -d --build backend frontend
|
||||
```
|
||||
|
||||
(RUNBOOK.md has the canonical re-deploy commands; this is the abbreviated form.)
|
||||
|
||||
- [ ] **Step 6: Verify on the live stack**
|
||||
|
||||
Drop a synthetic non-EDI file (any `.txt` without `ST*`) on the Upload page. Expect: select stays at default, Parse returns 400 (visible in the toast / network panel). Then drop the real 835 with default-kind; expect: select flips to 835, parse succeeds (the 1148 claims appear on Remittances).
|
||||
|
||||
---
|
||||
|
||||
## Self-Review
|
||||
|
||||
**1. Spec coverage:**
|
||||
- §1 envelope check → Tasks 2 + 3 ✅
|
||||
- §1 empty-claims check → Tasks 2 + 3 ✅
|
||||
- §1 regression locks on 999/277CA/TA1 → Task 4 ✅
|
||||
- §1 UI auto-detect → Task 5 ✅
|
||||
- §1 cleanup → Task 7 ✅
|
||||
- D1 (two-layer defense) → split into Tasks 2-3 (server) + Task 5 (UI) ✅
|
||||
- D2 (ST* + empty-claims, both) → Tasks 2 + 3 implement both ✅
|
||||
- D3 (4 KB scan window) → Task 5 implementation note ✅
|
||||
- D4 (auto-detect overrules manual) → not contested in tests; the test asserts "default 837p + dropped 835 file → kind becomes 835" ✅
|
||||
- D5 (cleanup direct SQL) → Task 7 ✅
|
||||
- D6 (400 vs 409) → Tasks 2 + 3 use status_code=400 ✅
|
||||
- D7 (no audit event) → no task implements one ✅
|
||||
- D8 (sibling test pattern) → Task 5 puts tests in `src/pages/Upload.test.tsx` ✅
|
||||
|
||||
**2. Placeholder scan:** No "TBD", "TODO", "implement later". The OpenAPI-of-claims endpoint detail in Task 2 Step 1 says "(Or, if /api/claims doesn't take a limit param, swap for /api/batches)" — that's a contingency, not a placeholder; the implementation step in Task 2 Step 3 will use whichever endpoint the frontend actually calls.
|
||||
|
||||
**3. Type consistency:** All references to `_envelope_st_token`, `_detected`, `expected`, `detected_st`, `detectEdiKind`, `readFileHead`, `pickFile`, the two bogus batch IDs, and the new test names are consistent across Tasks 1-8.
|
||||
@@ -0,0 +1,185 @@
|
||||
# Sub-project 33 — Co TXIX Payer Fix: Design Spec
|
||||
|
||||
**Date:** 2026-07-02
|
||||
**Status:** Draft, awaiting user sign-off
|
||||
**Branch:** `sp33-co-txix-payer-fix`
|
||||
**Aesthetic direction:** No new UI; production hotfix to emission
|
||||
|
||||
## 1. Scope
|
||||
|
||||
In scope:
|
||||
|
||||
- Change the canonical CO Medicaid payer identifier emitted in `NM1*PR NM108=PI*NM109` from `SKCO0` to `CO_TXIX`, in both the in-code `PayerConfig.co_medicaid()` factory and any other write path that derives the segment from `claim.payer.id`.
|
||||
- Update the in-repo companion-guide references (`docs/reference/837p.md`, `docs/reference/co-medicaid.md`) to reflect the corrected value. `docs/goodclaim.x12` is already correct and stays the source of truth.
|
||||
- Fix `cyclone.inbox_state.apply_999_rejections` to use `batch_envelope_index` (mirroring the SP28 fix to `apply_999_acceptances`) so SET-level 999 acks correctly cascade to claim-state transitions.
|
||||
- One-shot backfill against the live DB: for each claim already linked via `claim_acks` to a 999 ack with `set_accept_reject_code='R'`, set `claims.state='REJECTED'`, fill `rejection_reason`, `rejected_at`, `payer_rejected_reason`, `payer_rejected_at`, and emit a `claim.rejected` audit event. Idempotent.
|
||||
- Resubmit the 338 corrected single-claim 837 files (already split into `ingest/batch-*-claims/*.x12` by the operator) by replacing the `SKCO0` literal in their bytes with `CO_TXIX`, then uploading to `/CO XIX/PROD/coxix_prod_11525703/ToHPE/` via the existing paramiko-backed `SftpClient`.
|
||||
|
||||
Out of scope:
|
||||
|
||||
- No new R-coded validator rule (the existing `R100_payer_id_matches` already warns on mismatch; it auto-converges once `co_medicaid()` returns `CO_TXIX`).
|
||||
- No `CO_BHA` (behavioral-health) variant in this increment. The HCPF 999 lists `CO_TXIX OR CO_BHA` as both accepted; this increment picks `CO_TXIX` as the single value for dzinesco's three Home-Health NPIs. A future SP may add a per-claim override if behavioral-health submission volumes warrant it.
|
||||
- No changes to other payer configs (only `co_medicaid` is touched).
|
||||
- No schema migration. The existing `claims.payer_id` column already accepts any string; `claims.resubmit_count` already exists and is incremented; no new column is added.
|
||||
- No frontend UI changes. The dashboard already surfaces claim states; once the backfill flips them, the "0 accepted" widget will reflect reality without code change.
|
||||
|
||||
## 2. Context (why now)
|
||||
|
||||
On 2026-07-01 at 16:29 MT, dzinesco submitted four 837P batches (145+95+25+73 = 338 claims) to Gainwell's SFTP at `mft.gainwelltechnologies.com`. On 2026-07-02, Gainwell returned 999 acknowledgments for each, all `AK9=R` with the SET-level error:
|
||||
|
||||
```
|
||||
IK3*NM1*17*2010*8
|
||||
IK5*R*I5
|
||||
```
|
||||
|
||||
…where "2010BB NM109 must equal CO_TXIX or CO_BHA".
|
||||
|
||||
The in-repo reference (`docs/reference/837p.md:48`, `docs/reference/co-medicaid.md:118`) and the in-code `PayerConfig.co_medicaid()` factory (`backend/src/cyclone/parsers/payer.py:69-70`) both claim `SKCO0` is the correct value. The authoritative sample file (`docs/goodclaim.x12`, line 4) shows `NM1*PR*2*CO_TXIX*****PI*CO_TXIX` — confirming `CO_TXIX` is correct. Tyler confirmed this in chat on 2026-07-02: "the problem is we arent changing SKCO0 to CO_TXIX".
|
||||
|
||||
A second, separate bug surfaced during the same investigation: `apply_999_rejections` (`backend/src/cyclone/inbox_state.py:56`) passes the SET control number to a `claim_lookup` that queries by `patient_control_number`. As a result, even when a 999 ack correctly rejects a SET, no claim state ever flips. This bug pre-dates SP33 but is fixed here because (a) the SP28 fix already exists for `apply_999_acceptances` and the same fix is a small, surgical change, and (b) the dashboard "0/145 accepted" widget can only reflect reality once the cascade works.
|
||||
|
||||
## 3. Decisions (locked during brainstorming)
|
||||
|
||||
1. **NM109 = `CO_TXIX`, single value.** No per-claim rule. dzinesco's three NPIs are all Home Health (taxonomy `251E00000X`); none are behavioral-health-only providers. Behavioral-health claims (if any exist) would currently emit `CO_TXIX` and Gainwell would still accept per the 999's "OR CO_BHA" wording. Confirmed with Tyler 2026-07-02.
|
||||
2. **Fix the cascade bug in the same increment.** The 999 cascade is the same severity class as the 999 emission bug; splitting into a separate SP would mean a second hotfix in days. Confirmed with Tyler via Option A selection.
|
||||
3. **Backfill is a one-shot CLI subcommand.** A new `python -m cyclone.cli backfill-999-rejections` invocation, idempotent (claims already in `REJECTED` state are skipped). Lives in `cli.py` and a small helper module. Confirmed with Tyler via Option A.
|
||||
4. **Resubmit uses Tyler's pre-split files.** Tyler already produced 338 single-claim 837 files in `ingest/batch-*-claims/*.x12` (timestamps 16:41:43 on 2026-07-01). The resubmit path performs a deterministic `SKCO0 → CO_TXIX` byte replacement on each file, validates via the existing parser, then uploads via `SftpClient.put()`. Confirmed with Tyler via Option A (Step 6).
|
||||
5. **No new spec-on-the-fly for the bytecode fix.** The `SKCO0 → CO_TXIX` literal replacement is a single-pass `bytes.replace` scoped to the `NM1*PR` segment of each file. Anything more sophisticated would be over-engineered for a hotfix.
|
||||
|
||||
## 4. Architecture
|
||||
|
||||
### 4.1 Source fix (`feat(sp33): PayerConfig.co_medicaid() emits CO_TXIX`)
|
||||
|
||||
One file, two lines. `backend/src/cyclone/parsers/payer.py:69-70`:
|
||||
|
||||
```
|
||||
payer_id="SKCO0", → payer_id="CO_TXIX",
|
||||
payer_name="COHCPF", → payer_name="CO_TXIX",
|
||||
```
|
||||
|
||||
The serializer (`backend/src/cyclone/parsers/serialize_837.py:488`) reads these fields directly and emits `NM1*PR` accordingly — no serializer change required. The 999 cascade (independent fix, see §4.2) is already gated on `apply_999_rejections`, which queries against `claim.payer.id` once the in-DB claims carry the new value.
|
||||
|
||||
### 4.2 Cascade fix (`feat(sp33): apply_999_rejections uses batch_envelope_index`)
|
||||
|
||||
`backend/src/cyclone/inbox_state.py:55-65`:
|
||||
|
||||
```
|
||||
+ from cyclone.handlers._ack_id import batch_envelope_index # already exists
|
||||
def apply_999_rejections(session, parsed_999, *, claim_lookup, batch_envelope_index=None):
|
||||
...
|
||||
- claim = claim_lookup(sr.set_control_number)
|
||||
+ # SCN → list of claim_ids via batch_envelope_index; fall back to direct claim_lookup (rare)
|
||||
+ candidates = (batch_envelope_index or {}).get(sr.set_control_number, [])
|
||||
+ for cid in candidates:
|
||||
+ claim = session.get(Claim, cid)
|
||||
```
|
||||
|
||||
The new signature mirrors the SP28 pattern in `apply_999_acceptances`. Backwards-compatible: when `batch_envelope_index=None`, falls back to the old (broken) behavior, but every caller in `handle_999.handle()` will pass the index.
|
||||
|
||||
The handler at `backend/src/cyclone/handlers/handle_999.py:88-99` updates in lockstep:
|
||||
|
||||
```
|
||||
- rejection_result = apply_999_rejections(session, result, claim_lookup=_lookup)
|
||||
+ rejection_result = apply_999_rejections(
|
||||
+ session, result, claim_lookup=_lookup, batch_envelope_index=batch_index,
|
||||
+ )
|
||||
```
|
||||
|
||||
### 4.3 Backfill subcommand (`feat(sp33): cli backfill-999-rejections`)
|
||||
|
||||
New `cli.py` subcommand:
|
||||
|
||||
```
|
||||
python -m cyclone.cli backfill-999-rejections [--actor <name>] [--dry-run]
|
||||
```
|
||||
|
||||
Algorithm (single transaction, single `SessionLocal`):
|
||||
|
||||
1. `SELECT ca.claim_id, ca.set_control_number, ca.set_accept_reject_code, a.ack_code, a.raw_json FROM claim_acks ca JOIN acks a ON a.id=ca.ack_id WHERE ca.set_accept_reject_code='R' AND ca.linked_at IS NOT NULL`
|
||||
2. For each `claim_id`, load `Claim` via `session.get`. Skip if already `REJECTED` (idempotent). Otherwise set `state='REJECTED'`, `state_changed_at=now`, `rejected_at=now`, `rejection_reason=...`, `payer_rejected_at=now`, `payer_rejected_reason=...`, `payer_rejected_status_code='R'`, then append `AuditEvent(event_type='claim.rejected', actor=<name>)`.
|
||||
3. Commit. Print `(matched=N already_rejected=M errors=K)` summary.
|
||||
|
||||
For the 338 current claims, expected output: `matched=338 already_rejected=0 errors=0`.
|
||||
|
||||
### 4.4 Resubmit (`feat(sp33): cli resubmit-rejected-claims`)
|
||||
|
||||
New CLI subcommand:
|
||||
|
||||
```
|
||||
python -m cyclone.cli resubmit-rejected-claims [--payer co_medicaid] [--ingest-dir <path>]
|
||||
```
|
||||
|
||||
Defaults: `--ingest-dir /home/tyler/dev/cyclone/ingest`. Tyler pre-split the 338 single-claim 837 files into per-batch directories named `batch-<batch_id>-<N>-claims/`, where `<batch_id>` matches `claims.batch_id` and `<N>` is the claim count. The exact filenames match the SFTP outbound naming convention (`TPID-837P-yyyymmddhhmmssSSS-1of1.x12`) and are already unique-by-timestamp.
|
||||
|
||||
Algorithm:
|
||||
|
||||
1. Discover all `claims` rows where `state='REJECTED'` (after the §4.3 backfill lands). Group by `batch_id`.
|
||||
2. For each `claim_id` in `REJECTED`, locate the matching `_837P-*.x12` file in `ingest/batch-<batch_id>-<N>-claims/`. Match by directory naming + per-claim `patient_control_number` declared in the file's CLM01 segment (re-parse each file once, cache the CLM01 list keyed by file).
|
||||
3. For each file: `new_bytes = old_bytes.replace(b'PI*SKCO0', b'PI*CO_TXIX')`. Validate via `parse_837_text(new_bytes)`. If validation produces any NEW error (compared to the same parse of the pre-replacement bytes), halt with the file path + error diff so the operator can inspect.
|
||||
4. Upload via `cyclone.clearhouse.SftpClient.put(local_path, remote_path)` to `/CO XIX/PROD/coxix_prod_11525703/ToHPE/` using the EXISTING filename. SFTP client retains the paramiko session across files (already supported per SP16).
|
||||
5. After successful upload, increment `claims.resubmit_count` (existing column, default 0) and emit one `clearhouse.submitted` audit event per file. `claims.state` remains `REJECTED` — the `resubmit_count > 0` flag is sufficient for the UI to distinguish "rejected-and-needs-resubmit" from "rejected-and-already-resubmitted". No new claim state is introduced.
|
||||
|
||||
For the 338 current claims: this runs once. Output filenames already reflect dzinesco's outbound SFTP naming scheme and are unique.
|
||||
|
||||
### 4.5 Doc updates (`docs(spec): SP33 update reference docs to CO_TXIX`)
|
||||
|
||||
Two files, two lines:
|
||||
|
||||
- `docs/reference/837p.md:48` — replace
|
||||
`Trading partner: 'SKCO0' (sender) ↔ 'COHCPF' (receiver) on 'NM1*PR' / 'NM1*40'`
|
||||
with
|
||||
`Trading partner: 'COMEDASSISTPROG' (receiver, NM1*40) and payer 'CO_TXIX' (NM1*PR, NM108=PI, NM109=CO_TXIX) — see docs/goodclaim.x12 for the canonical example.`
|
||||
|
||||
- `docs/reference/co-medicaid.md:118` — replace
|
||||
`'NM1*PR N104 = "SKCO0"' (COHCPF)`
|
||||
with
|
||||
`'NM1*PR NM109 = "CO_TXIX"' (CO_TXIX)` and add a sentence: `For behavioral-health claims only, use "CO_BHA"; dzinesco's current submission volumes don't include behavioral-only providers.`
|
||||
|
||||
`docs/goodclaim.x12` stays unchanged (already correct).
|
||||
|
||||
## 5. Failure modes
|
||||
|
||||
| Failure | Detection | Mitigation |
|
||||
|---|---|---|
|
||||
| Serializer change breaks 999+1 backwards compat | `pytest backend/tests/test_serialize_837.py` | Layered test: pre-fix `goodclaim.x12` parse still passes; post-fix parse still passes (the `payer.id` change is in NM109 only) |
|
||||
| Cascade fix changes behavior for already-accepted 999s | Unit test: feed 999 with `AK5=A`; assert no claim state changes | `apply_999_acceptances` is unchanged; only `apply_999_rejections` is touched |
|
||||
| Backfill races with live scheduler writing new claim_acks | Single-transaction SELECT-then-UPDATE; row-level retry on `database is locked` | Wrap the per-claim update in a short retry loop (3 attempts, 100ms backoff) |
|
||||
| Resubmit overwrites a not-yet-acknowledged file on SFTP | SFTP-side ETags / names | Use Tyler's pre-split filenames verbatim (already unique-by-timestamp) |
|
||||
| SKCO0 still in some legacy 837 in the SFTP outbound dir | Out of scope — dzinesco is no longer submitting new 837s against the buggy factory | None; historical files on SFTP aren't retransmitted |
|
||||
|
||||
## 6. Test impact
|
||||
|
||||
- `backend/tests/test_apply_999_rejections.py` — **new**. Covers:
|
||||
- SET-level `AK9=R` with a `batch_envelope_index` containing 3 claim_ids → all 3 transition to REJECTED, idempotent on rerun
|
||||
- SET-level `AK9=A` with empty envelope index → 0 matches (no false positives)
|
||||
- No `batch_envelope_index` passed → falls back to old behavior (passes SCN as PCN, returns no matches for non-PCN-keyed claims)
|
||||
- `backend/tests/test_serialize_837.py` — extend: assert `co_medicaid()` round-trips through `serialize_837 → parse_837` and the parsed `payer.id` equals `CO_TXIX`
|
||||
- `backend/tests/test_payer_config_loading.py` — confirm YAML still binds `CO_TXIX` and no in-code change breaks the bootstrap path
|
||||
- `backend/tests/test_inbox_state_apply_999.py` (if it exists) — adjust for the new optional arg
|
||||
|
||||
No frontend changes → no Vitest/RTL impact.
|
||||
|
||||
## 7. Branch / merge plan
|
||||
|
||||
- Branch: `sp33-co-txix-payer-fix`
|
||||
- Atomic merge commit. PR title: `SP33 Co TXIX payer fix + 999 cascade repair`
|
||||
- Commit prefix schedule:
|
||||
- `docs(spec): SP33 CO TXIX payer fix design`
|
||||
- `docs(plan): SP33 CO TXIX payer fix implementation`
|
||||
- `feat(sp33): PayerConfig.co_medicaid() emits CO_TXIX`
|
||||
- `feat(sp33): apply_999_rejections uses batch_envelope_index`
|
||||
- `test(sp33): add test_apply_999_rejections with batch_envelope_index`
|
||||
- `feat(sp33): cli backfill-999-rejections`
|
||||
- `feat(sp33): cli resubmit-rejected-claims`
|
||||
- `docs(sp33): update docs/reference/{837p,co-medicaid}.md to CO_TXIX`
|
||||
- `merge: SP33 Co TXIX payer fix + 999 cascade repair into main`
|
||||
|
||||
## 8. Rollback
|
||||
|
||||
If the SP33 merge breaks the live 837 flow:
|
||||
|
||||
1. Revert the single merge commit (`git revert <merge-sha>` — produces a fresh commit, preserves history).
|
||||
2. The two doc edits revert cleanly (markdown only).
|
||||
3. The cascade-fix revert restores the SP28-era broken-but-not-dead behavior (claims stay SUBMITTED — same as today).
|
||||
4. The backfill is fully reversible: a sister CLI subcommand `python -m cyclone.cli reset-rejection-backfill [--batch-id <id>]` can un-REJECT the 338 claims (sets `state='SUBMITTED'`, clears `rejection_reason`, `rejected_at`, `payer_rejected_*`). Out of scope to ship in SP33, but the column data is fully captured in audit log so rollback is always possible.
|
||||
5. The resubmit cannot be undone (the corrected 837 files are on SFTP), but a `clearhouse` audit event is recorded for every upload so the operator can identify and re-trigger if needed.
|
||||
@@ -62,14 +62,22 @@ Effective algorithm unchanged: 2-of-3 `{PCN, charge, NPI}` still fires when any
|
||||
|
||||
New subcommand: `python -m cyclone.cli backfill-rendering-npi`.
|
||||
|
||||
Behavior:
|
||||
1. For every `claim_batches.inbound_path`, re-parse the 837p file with the new parser; for every claim in the batch, write `Claim.rendering_provider_npi = parsed_value` **only when the column is currently `NULL`** (idempotent).
|
||||
2. For every `remittances.inbound_path`, re-parse the 835 file with the new parser; for every remit, write both the per-segment NPIs (`ClaimPayment.service_provider_npi`) and the aggregated `Remittance.rendering_provider_npi`.
|
||||
3. Run `reconcile.run()` once over all open pairs at the end so the NPI arm can fire retroactively.
|
||||
4. Log per-batch counts (parsed, updated, skipped).
|
||||
5. Exit code 0 on success, 2 on file-level failure (no batch updated), 1 on unexpected exception.
|
||||
**Why explicit file args.** The `Batch` ORM does not retain a column for the original on-disk ingest path — the live ingest pipeline reads the upload body, parses, writes rows, and never persists the bytes. So there is no `claim_batches.inbound_path` / `remittances.inbound_path` to "look up" and re-parse from. The operator must point the CLI at the files (or the directory that holds them) that need backfill. Re-running ingest end-to-end would be the alternative, but it would re-`INSERT` claim rows and is a destructive operation; backfill is intentionally a read-reparse-targeted-`UPDATE`.
|
||||
|
||||
Idempotent: re-running on a fully-populated DB is a no-op (typed columns not overwritten; reconcile already matched pairs skipped via `Claim.matched_remittance_id IS NOT NULL`).
|
||||
Flags:
|
||||
- `--file PATH` (repeatable) — one or more specific X12 files to re-parse. Each path is validated to exist before parsing.
|
||||
- `--input-dir DIR` — directory to scan one level deep for `*.txt` / `*.edi` / `*.x12` files. Falls back to `$CYCLONE_BACKFILL_INPUT_DIR` if neither `--file` nor `--input-dir` is passed.
|
||||
- `--type {837p,835}` (optional) — pin the parser. **When omitted, each file's transaction kind is auto-sniffed** (filename hint + ISA/ST prefix) so a mixed directory can be processed in one pass.
|
||||
|
||||
Behavior:
|
||||
1. For each file resolved by `--file` / `--input-dir`, re-parse with the new parser (T4 wiring).
|
||||
2. **837p**: for every claim in the batch, write `Claim.rendering_provider_npi = parsed_value` **only when the column is currently `NULL`** (idempotent — already-populated rows are left alone).
|
||||
3. **835**: for every remit, write `Remittance.rendering_provider_npi` (typed column) **only when `NULL`**, and persist the per-segment `service_provider_npi` values into `Remittance.raw_json["service_provider_npis"]` (D5).
|
||||
4. After all files are processed, run `reconcile.run()` **once across every 835 Batch** so the D6 NPI arm can fire retroactively on newly-populated pairs.
|
||||
5. Log per-file counts (parsed, updated, skipped). Emit one-line summary to stdout: `claims_updated=N remits_updated=M files_processed=K files_skipped=J`.
|
||||
6. Exit code 0 on success (zero populated rows is still exit 0), 2 on file-level failure (no file processed), 1 on unexpected exception.
|
||||
|
||||
Idempotent: re-running on a fully-populated DB is a no-op (typed columns not overwritten; reconcile skips already-matched pairs via `Claim.matched_remittance_id IS NOT NULL`).
|
||||
|
||||
### D8 — Backfill event emission
|
||||
|
||||
@@ -96,7 +104,8 @@ No `UPDATE` statements in the migration — backfill is a separate, deliberate s
|
||||
- `backend/src/cyclone/db.py` — `Claim.rendering_provider_npi`, `Remittance.rendering_provider_npi`. No claim_payments ORM — service_provider_npi persists only via `Remittance.raw_json["service_provider_npis"]` (D5).
|
||||
- `backend/src/cyclone/writer.py` / `writer_835.py` — set the new typed columns and the `raw_json` mirrors at write time. `writer_835.py` writes the per-CLP `service_provider_npi` into `Remittance.rendering_provider_npi` (D4, single value) and into `raw_json["service_provider_npis"]` (D5).
|
||||
- `backend/src/cyclone/reconcile.py` — extend `_content_keys_match` (D6).
|
||||
- `backend/src/cyclone/cli.py` — new `backfill-rendering-npi` subcommand.
|
||||
- `backend/src/cyclone/store/backfill.py` — **new helper module** owning `backfill_rendering_provider_npi(files, input_dir, transaction_type)`: file/CLI argument resolution, auto-sniffing (ISA/ST + filename hint), re-parse dispatch, idempotent typed-column writes, and the post-backfill `reconcile.run()` sweep. Returns a small summary dataclass for the CLI summary line.
|
||||
- `backend/src/cyclone/cli.py` — new `backfill-rendering-npi` subcommand (Click). Wires `--file` / `--input-dir` / `--type` / `--log-level`, delegates the heavy lifting to `store.backfill.backfill_rendering_provider_npi`, and prints the one-line summary.
|
||||
- `backend/src/cyclone/migrations/0019_add_rendering_and_service_provider_npis.sql` — D9.
|
||||
- `backend/tests/fixtures/` — new 837p fixture(s) with `NM1*82` and new 835 fixture(s) with `NM1*1P`.
|
||||
- `backend/tests/test_reconcile.py` — new tests for D6 (typed-column primary path, typed-column beats raw_json fallback, NPI arm fires when both sides populated).
|
||||
|
||||
@@ -0,0 +1,139 @@
|
||||
# Sub-project 35 — Parse Input Guards: Design Spec
|
||||
|
||||
**Date:** 2026-07-06
|
||||
**Status:** Draft, awaiting user sign-off
|
||||
**Branch:** `sp35-parse-input-guards` (off `main`, post-SP33)
|
||||
**Aesthetic direction:** No UI changes. The Upload page gets a small invisible behavior change (auto-flip the `Kind` select when the file content disagrees) and keeps its current visual design.
|
||||
|
||||
---
|
||||
|
||||
## 1. Scope
|
||||
|
||||
Today, dropping any X12 file on the Upload page with the default `Kind: 837P` submits the file to `POST /api/parse-837` regardless of whether the bytes look like an 837P or an 835. The `/api/parse-837` endpoint silently accepts that submission: the parser reads the ISA/ST envelope, finds zero CLM segments (because 835s carry `CLP`, not `CLM`), returns an empty `ParseResult`, and the endpoint then persists a `kind='837p'` batch row with `total_claims=0`. The bogus batch shows up in the Batches view, no remittance or claim rows are produced, and the operator has no way to tell from the UI that the ingest was misrouted. SP34 left two such bogus batches in production (`50eb50c1…` and `e4692571…`) when an operator (or test) dropped a `tp11525703-835_M019771179-20260706005516577-1of1.x12` file at the project root and ran the upload twice without changing the default `Kind`.
|
||||
|
||||
SP35 closes two independent layers that both contribute to the silent-corruption path. The fix is layered (defense in depth): either layer alone would prevent the symptom, but each layer is a separate invariant that needs its own test and its own enforcement.
|
||||
|
||||
**Investigation finding (shapes the scope):** I read each parser before locking the scope. `/api/parse-837` and `/api/parse-835` are the only parse endpoints where the parser can return a successful-looking empty result. The 999/277CA/TA1 endpoints already enforce envelope/segment guards at the parser layer:
|
||||
|
||||
- **`parse_999`** (`backend/src/cyclone/parsers/parse_999.py:289-290`): raises `CycloneParseError("No AK9 (Functional Group Response Status) segment found")` when the parser finds no `AK9` segments. There is no silent-corruption path because 999s that don't carry `AK9` (everything that isn't a 999) get rejected.
|
||||
- **`parse_277ca`** (`backend/src/cyclone/parsers/parse_277ca.py:297-298`): raises `"Expected ST*277 or ST*277CA, got ST*<other>"` when the envelope token doesn't match. Strict envelope guard.
|
||||
- **`parse_ta1`** (`backend/src/cyclone/parsers/parse_ta1.py:111`): raises `"Expected TA1, got <other>"` when the first segment after ISA isn't `TA1*`. Strict envelope guard.
|
||||
|
||||
The right scope is therefore: **fix the two endpoints that have the bug** (837p and 835), and **add regression tests** to the three endpoints that already have strict guards so we catch any future regression where someone loosens the parser.
|
||||
|
||||
**In scope:**
|
||||
|
||||
- **Server-layer input guard on `POST /api/parse-837`.** Two checks before any `store.add(...)` runs:
|
||||
1. **Envelope check.** The first `ST*` segment (or first 4096 bytes, whichever comes first) must begin with `ST*837`. If not, the endpoint returns `400` with `{error: "Mismatched file kind", detail: "...", expected: "837p", detected_st: "<token>"}` and persists nothing.
|
||||
2. **Empty-claims check.** After the parser runs, if `len(result.claims) == 0`, the endpoint returns `400` with `{error: "No claims parsed", detail: "..."}` and persists nothing.
|
||||
- **Same two checks on `POST /api/parse-835`**, mirrored for symmetry. The 835 parser requires `BPR` and `TRN` (raises `CycloneParseError` if either is missing) but does NOT require `CLP` segments — a stripped-down 835 with `ISA*BPR*TRN*SE*GE*IEA` and no `CLP` would silently persist as an empty `claims=[]` batch today. The new envelope guard catches the "this is an 837 file" misroute; the empty-claims guard catches this "valid-835-shape but no claims" edge case.
|
||||
- **Regression tests on `POST /api/parse-999`, `POST /api/parse-277ca`, `POST /api/parse-ta1`** that assert these endpoints already return 400 (not 200) when fed an unrelated X12 file. These lock the current strict-parser behavior in — if a future PR loosens the parser envelope guards, these tests fail and prevent a recurrence of the SP34-class bug.
|
||||
- **UI-layer auto-detect in `src/pages/Upload.tsx`.** When the user drops or selects a file, the page reads the first ~4 KB of the file as text, looks for the first `ST*…` token, and if it finds `ST*837` the `Kind` select flips to `837P` (no-op if already there), and if it finds `ST*835` the select flips to `835`. If neither token is found (a `.txt` export without an `ST*` header, or a PDF, or a corrupted file) the select stays where it is and the user keeps manual control. (999/277CA/TA1 don't enter the UI flow today — the Upload page only has 837p/835 in the Kind select — so no UI auto-detect on those kinds is needed.)
|
||||
- **Backend tests** covering the four new guards on `/api/parse-835` (mirroring parse-837) and the three regression tests on the 999/277CA/TA1 endpoints.
|
||||
- **Frontend test** in `src/pages/Upload.test.tsx` covering the auto-detect for both 837 and 835 file payloads, plus a "no ST* found → no flip" regression case.
|
||||
- **Cleanup of the two existing bogus batches** from the production DB (`50eb50c1…` and `e4692571…`) by direct SQL after the fix lands and is verified in production.
|
||||
|
||||
**Out of scope:**
|
||||
|
||||
- No schema migration. No new tables, no new columns, no new foreign keys.
|
||||
- No changes to the `parse_837` / `parse_835` parser modules themselves (the empty-claims check is at the API layer, where the `store.add(...)` decision lives; moving it into the parsers would change CLI semantics).
|
||||
- No changes to any CLI subcommand. The CLI `parse-837` and `parse-835` already raise `click.UsageError` on empty claims (consistent with what SP35 makes the API do); the SP only closes the API-vs-CLI symmetry.
|
||||
- No changes to the 999 / 277CA / TA1 / 270 / 271 endpoints. They each have their own parsers; if a similar bug exists on them it is out of scope for SP35 and should be filed as a follow-up if confirmed.
|
||||
- No new admin endpoints for batch deletion. Cleanup is a one-line direct SQL run via `docker exec cyclone-backend-1 sqlite3 …` and is documented in the plan, not in the product surface.
|
||||
- No `cyclone admin` changes.
|
||||
- No changes to the activity-events flood (a separate observation from the same incident — one 835 produced hundreds of per-claim `reconcile` activity events; a real concern but a separate ticket).
|
||||
- No change to the auth boundary. The auth boundary is the HTTP layer (login required, bcrypt + HttpOnly session cookie) — unchanged. The new 400 responses on `/api/parse-837` and `/api/parse-835` are produced under `matrix_gate`, same as today.
|
||||
|
||||
---
|
||||
|
||||
## 2. Decisions (locked during brainstorming)
|
||||
|
||||
### D1. Two-layer defense: server guards AND UI auto-detect. Each one alone is insufficient.
|
||||
|
||||
If we fix only the server, the symptom goes away but the operator still gets a confusing 400 with no explanation for why their drop didn't work — every bad drop produces a 400 instead of a 200 with a silent empty batch. Fixing the UI without the server leaves the API as a footgun for any other client (curl, third-party tooling, future ingestion paths). Both layers are needed because the server guard is the **invariant** (correctness) and the UI auto-detect is the **operator experience** (correct-by-default UX).
|
||||
|
||||
### D2. The server check is "ST* token + empty claims," not just one or the other
|
||||
|
||||
Relying on `ST*` alone fails on truncated headers (the parser can't even reach the ST if the ISA is corrupt, so the message becomes a generic 400). Relying on empty-claims alone leaves the door open for empty-but-valid-ISA 999 files to silently hit the 837p endpoint and produce empty batch rows (the bug we're closing). The two checks together produce a clear, two-stage error: "your file's ST token isn't 837p" before parse, or "your 837p parse produced zero claims" after parse.
|
||||
|
||||
### D3. The UI auto-detect reads the first 4 KB of the file, not the whole file
|
||||
|
||||
X12 `ST*837` / `ST*835` always appears in the first few hundred bytes of a well-formed file (after ISA and GS). Reading 4 KB guarantees capture without loading multi-megabyte files into memory in the browser. The check runs in `pickFile()` synchronously on the dropped `File` object via `FileReader.readAsText(file.slice(0, 4096))`.
|
||||
|
||||
### D4. The auto-detect never overrules a user who has manually picked `835`
|
||||
|
||||
If the user explicitly selects `835` from the dropdown, then drops a file whose content says `ST*837`, the auto-detect still wins (it overrides the select). Rationale: the whole point of the auto-detect is to prevent the silent-corruption failure mode; honoring a stale manual override would defeat the purpose. If the operator wants to "force 837p on an 835 file" — a legitimate test case — they can flip the select back manually after the auto-detect fires; the file is still in `stream.file` and the parse button hasn't been clicked yet.
|
||||
|
||||
### D5. Cleanup is direct SQL, not a new admin endpoint
|
||||
|
||||
The two bogus batches (`50eb50c1…` and `e4692571…`) have `kind='837p'`, `input_filename` of the 835 file, `total_claims=0`, and zero `claims` / `service_line_payments` / `cas_adjustments` / `matches` rows pointing at them. A one-shot `DELETE FROM batches WHERE id IN (...)` is sufficient and does not require a new product surface. The plan documents the exact command and where to run it (`docker exec cyclone-backend-1 sqlite3 /var/lib/cyclone/db/cyclone.db "..."`). A future admin-batch-delete endpoint can be a separate SP if there's operator demand.
|
||||
|
||||
### D6. No 409 vs 400 nuance — every bad input is a 400
|
||||
|
||||
The existing endpoint returns `400` for `Empty file`, `Encoding error`, and `Parse error`; it returns `409` for `Duplicate claim` (a state conflict, not an input error). SP35's new failures (`Mismatched file kind`, `No claims parsed`) are input errors, so they get `400`. The error envelope shape (`{error, detail}`) is preserved.
|
||||
|
||||
### D7. No new audit-log entry for "rejected at ingest"
|
||||
|
||||
The existing audit log captures state-affecting actions (admin role changes, batch deletes, etc.), not failed parse attempts. Adding `parse_rejected` events would conflate operational noise with the user-action audit trail. If observable rejection events matter in the future, they can be a separate SP that adds a structured log channel.
|
||||
|
||||
### D8. Frontend test lives in `src/pages/Upload.test.tsx` and uses the existing `vi.mock("@/lib/api", …)` pattern
|
||||
|
||||
The new behavior is a thin synchronous change to `pickFile()`; the test mocks `FileReader` (or uses `Blob` directly via `URL.createObjectURL` + `<input>` event firing) and asserts the resulting `kind` state. No new test framework, no new mocking library.
|
||||
|
||||
---
|
||||
|
||||
## 3. Edge cases & how they're handled
|
||||
|
||||
| Scenario | Behavior |
|
||||
|---|---|
|
||||
| Drop a valid 837 file with default `kind='837p'` | No change. Already correct; auto-detect is a no-op. |
|
||||
| Drop a valid 835 file with default `kind='837p'` | Auto-detect flips `kind` to `835`. Parse succeeds. |
|
||||
| Drop a valid 835 file with `kind='835'` already selected | Auto-detect is a no-op. Parse succeeds. |
|
||||
| Drop a 999 file with default `kind='837p'` | Auto-detect finds no `ST*837` and no `ST*835`, so leaves `kind` alone. Parse fails with `Mismatched file kind` 400. |
|
||||
| Drop a non-X12 `.txt` with default `kind='837p'` | Same as above: no auto-detect, parse fails clearly. |
|
||||
| Operator manually selects `835` then drops an 837 | Auto-detect flips to `837p` (D4). Parse succeeds. |
|
||||
| Bypass the UI entirely and POST an 835 file to `/api/parse-837` via curl | Server guard rejects with `400 Mismatched file kind`. No batch row persisted. |
|
||||
| Bypass the UI and POST a valid 837 with zero CLM segments (empty ISA envelope) to `/api/parse-837` | Server guard rejects with `400 No claims parsed`. No batch row persisted. |
|
||||
| Re-ingest the same valid 835 to `/api/parse-835` (duplicate) | Existing `409 Duplicate remittance` flow unchanged. SP35 does not touch the dedup logic. |
|
||||
| User clicks `Parse file` twice rapidly with the same file | First request persists; second request hits dedup and gets `409`. Unchanged. |
|
||||
| Frontend auto-detect reads a 4 KB slice that straddles a multi-segment envelope | Unlikely on real EDI; the `ST*` always appears in the first ~500 bytes after ISA/GS. Regression test covers the `ST*835` at byte 3800 case. |
|
||||
|
||||
---
|
||||
|
||||
## 4. Testing approach
|
||||
|
||||
**Backend (Python):** add to existing `tests/test_api.py` (`/api/parse-837`), `tests/test_api_835.py`, `tests/test_api_999.py`, `tests/test_api_277ca.py`, `tests/test_api_ta1.py`:
|
||||
|
||||
- **`test_parse_837_endpoint_rejects_835_input`** — POST the CO Medicaid 835 fixture to `/api/parse-837`, assert `400`, error envelope `error == "Mismatched file kind"`, no new `batches` row.
|
||||
- **`test_parse_837_endpoint_rejects_empty_envelope`** — POST a syntactically valid ISA with no CLM segments to `/api/parse-837`, assert `400`, error envelope `error == "No claims parsed"`, no new `batches` row.
|
||||
- **`test_parse_835_endpoint_rejects_837_input`** — symmetric: POST 837 to `/api/parse-835`, expect `400 Mismatched file kind`.
|
||||
- **`test_parse_835_endpoint_rejects_empty_envelope`** — symmetric: POST a stripped ISA+BPR+TRN+SE+GE+IEA with no `CLP` to `/api/parse-835`, expect `400 No claims parsed`.
|
||||
- **`test_parse_999_endpoint_rejects_837_input`** — regression lock: POST 837 fixture to `/api/parse-999`, expect `400 Parse error` (raised by the parser's `No AK9` rule). Documents that the 999 parser already has this invariant enforced.
|
||||
- **`test_parse_277ca_endpoint_rejects_835_input`** — regression lock: POST 835 fixture to `/api/parse-277ca`, expect `400 Parse error` (raised by the parser's `Expected ST*277` rule).
|
||||
- **`test_parse_ta1_endpoint_rejects_835_input`** — regression lock: POST 835 fixture to `/api/parse-ta1`, expect `400 Parse error` (raised by the parser's `Expected TA1` rule).
|
||||
- **`test_parse_837_endpoint_happy_path`** (existing) — regression guard: CO Medicaid 837p fixture still parses and persists cleanly.
|
||||
- **`test_parse_835_endpoint_happy_path`** (existing) — regression guard: CO Medicaid 835 fixture still parses and persists cleanly.
|
||||
|
||||
**Frontend (Vitest):** add to `src/pages/Upload.test.tsx` (sibling file exists per the project convention):
|
||||
|
||||
- **`upload_auto_detect_837_flips_kind_to_837p`** — drop a file whose first 4 KB contain `ST*837`, assert `kind` state is `837p`.
|
||||
- **`upload_auto_detect_835_flips_kind_to_835`** — drop a file whose first 4 KB contain `ST*835`, assert `kind` state is `835`.
|
||||
- **`upload_auto_detect_no_st_token_leaves_kind_unchanged`** — drop a file with no `ST*` header, assert `kind` state is whatever the user had selected.
|
||||
|
||||
**Manual smoke (live stack):**
|
||||
|
||||
- Drop the `tp11525703-835_M019771179-20260706005516577-1of1.x12` file at the project root with default `Kind: 837P` selected. Expect: select flips to `835`, parse succeeds, 1148 claim rows land in `remittances`. Repeat with `Kind: 835` already selected. Expect: no flip, same outcome.
|
||||
- POST the same 835 file via curl to `/api/parse-837`. Expect: `400 Mismatched file kind`, no batch row written.
|
||||
|
||||
---
|
||||
|
||||
## 5. Out-of-scope reminders (explicit)
|
||||
|
||||
- **No new guards on `/api/parse-270` or `/api/parse-271`.** Those endpoints handle eligibility-benefit pairs. They may or may not have the same parse-empty shape; out of scope for SP35. (Filing as a follow-up if confirmed.)
|
||||
- **No production-side changes to the 999 / 277CA / TA1 endpoint code.** Only regression tests are added; the parsers themselves already reject mismatched input.
|
||||
- **No `cyclone admin batches delete` endpoint.** Cleanup is direct SQL for now.
|
||||
- **No schema migration.** The data model is unchanged.
|
||||
- **No changes to validation rules** in `cyclone.parsers.validator_837` / `cyclone.parsers.validator_835`. SP35 only adds guards at the API layer.
|
||||
- **No change to the activity-events storm.** This is a perf/observability concern, not a correctness bug, and will get its own ticket.
|
||||
- **No auth changes.** The new 400 responses inherit `matrix_gate` and the existing session-cookie auth boundary.
|
||||
- **No changes to the running production stack.** SP35 is developed against `main`, merged in, then the running containers are restarted via the existing compose-up flow documented in `RUNBOOK.md`. No hot-patches.
|
||||
@@ -0,0 +1,148 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
detectKindFromText,
|
||||
detectKindFromFile,
|
||||
detectedKindToParsedBatchKind,
|
||||
} from "./x12-detect";
|
||||
|
||||
// Minimal X12 envelopes — just enough to exercise the ST01 match.
|
||||
// Real fixtures live in backend/tests/fixtures; we use synthetic ones
|
||||
// here because the frontend doesn't need the full file content, just
|
||||
// the first ~4KB.
|
||||
|
||||
const ISA_837P = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
|
||||
"*260617*1937*^*00501*000000001*0*P*:~" +
|
||||
"GS*HC*SENDER*RECEIVER*20260706*1937*1*X*005010X222A1~" +
|
||||
"ST*837*0001*005010X222A1~"
|
||||
);
|
||||
const ISA_835 = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
|
||||
"*260617*1937*^*00501*000000001*0*P*:~" +
|
||||
"GS*HP*SENDER*RECEIVER*20260706*1937*1*X*005010X221A1~" +
|
||||
"ST*835*0001~"
|
||||
);
|
||||
const ISA_999 = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
|
||||
"*260617*1937*^*00501*000000001*0*P*:~" +
|
||||
"GS*FA*SENDER*RECEIVER*20260706*1937*1*X*005010X231A1~" +
|
||||
"ST*999*0001~"
|
||||
);
|
||||
const ISA_277CA = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
|
||||
"*260617*1937*^*00501*000000001*0*P*:~" +
|
||||
"GS*HN*SENDER*RECEIVER*20260706*1937*1*X*005010X214~" +
|
||||
"ST*277*0001*005010X214~"
|
||||
);
|
||||
const ISA_277CA_QUALIFIED = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
|
||||
"*260617*1937*^*00501*000000001*0*P*:~" +
|
||||
"GS*HN*SENDER*RECEIVER*20260706*1937*1*X*005010X214~" +
|
||||
"ST*277CA*0001*005010X214~"
|
||||
);
|
||||
const TA1_FILE = (
|
||||
"ISA*00* *00* *ZZ*SENDER *ZZ*RECEIVER " +
|
||||
"*260520*1750*^*00501*000000001*0*P*:~" +
|
||||
"TA1*000000001*20260520*1750*A*000*20260520~" +
|
||||
"IEA*1*000000001~"
|
||||
);
|
||||
|
||||
describe("detectKindFromText", () => {
|
||||
it("detects ST*837* as 837p", () => {
|
||||
expect(detectKindFromText(ISA_837P)).toBe("837p");
|
||||
});
|
||||
|
||||
it("detects ST*837P* as 837p (professional qualifier)", () => {
|
||||
const withQualifier = ISA_837P.replace("ST*837*", "ST*837P*");
|
||||
expect(detectKindFromText(withQualifier)).toBe("837p");
|
||||
});
|
||||
|
||||
it("detects ST*835* as 835", () => {
|
||||
expect(detectKindFromText(ISA_835)).toBe("835");
|
||||
});
|
||||
|
||||
it("detects ST*999* as 999", () => {
|
||||
expect(detectKindFromText(ISA_999)).toBe("999");
|
||||
});
|
||||
|
||||
it("detects ST*277* as 277ca", () => {
|
||||
expect(detectKindFromText(ISA_277CA)).toBe("277ca");
|
||||
});
|
||||
|
||||
it("detects ST*277CA* as 277ca (qualified form)", () => {
|
||||
expect(detectKindFromText(ISA_277CA_QUALIFIED)).toBe("277ca");
|
||||
});
|
||||
|
||||
it("detects TA1 envelope as ta1 (no ST, bare TA1 segment)", () => {
|
||||
expect(detectKindFromText(TA1_FILE)).toBe("ta1");
|
||||
});
|
||||
|
||||
it("returns unknown for garbage input", () => {
|
||||
expect(detectKindFromText("not edi at all")).toBe("unknown");
|
||||
});
|
||||
|
||||
it("returns unknown for an empty string", () => {
|
||||
expect(detectKindFromText("")).toBe("unknown");
|
||||
});
|
||||
|
||||
it("is case-insensitive on ST segment", () => {
|
||||
// Real X12 is uppercase, but be lenient — files from various tools
|
||||
// sometimes have mixed case.
|
||||
expect(detectKindFromText(ISA_835.toLowerCase())).toBe("835");
|
||||
});
|
||||
|
||||
it("does not false-positive on ST*8370 (5 chars after ST*)", () => {
|
||||
// Catches a regression where the matcher would substring-match too
|
||||
// eagerly (e.g. matching ST*837 against ST*8370*). The require is
|
||||
// that the char after the digits is `*`, not another digit.
|
||||
const noise = "ISA*00*...*ST*8370*0001~";
|
||||
expect(detectKindFromText(noise)).toBe("unknown");
|
||||
});
|
||||
});
|
||||
|
||||
describe("detectedKindToParsedBatchKind", () => {
|
||||
it("maps 837p → 837p", () => {
|
||||
expect(detectedKindToParsedBatchKind("837p")).toBe("837p");
|
||||
});
|
||||
|
||||
it("maps 835 → 835", () => {
|
||||
expect(detectedKindToParsedBatchKind("835")).toBe("835");
|
||||
});
|
||||
|
||||
it("returns null for 999 (Upload page doesn't ingest 999s)", () => {
|
||||
expect(detectedKindToParsedBatchKind("999")).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null for 277ca", () => {
|
||||
expect(detectedKindToParsedBatchKind("277ca")).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null for ta1", () => {
|
||||
expect(detectedKindToParsedBatchKind("ta1")).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null for unknown", () => {
|
||||
expect(detectedKindToParsedBatchKind("unknown")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("detectKindFromFile", () => {
|
||||
it("detects 837p from a File blob", async () => {
|
||||
const file = new File([ISA_837P], "test.x12", { type: "text/plain" });
|
||||
expect(await detectKindFromFile(file)).toBe("837p");
|
||||
});
|
||||
|
||||
it("detects 835 from a File blob (the original SP35 repro)", async () => {
|
||||
// The exact scenario the user hit: drop an 835 file on the Upload
|
||||
// page while the dropdown still says "837p". The helper must
|
||||
// return "835" so the UI can switch the dropdown before the user
|
||||
// hits Parse.
|
||||
const file = new File([ISA_835], "tp11525703-835.x12", { type: "text/plain" });
|
||||
expect(await detectKindFromFile(file)).toBe("835");
|
||||
});
|
||||
|
||||
it("returns unknown on an empty file (no crash)", async () => {
|
||||
const file = new File([""], "empty.x12", { type: "text/plain" });
|
||||
expect(await detectKindFromFile(file)).toBe("unknown");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,98 @@
|
||||
/**
|
||||
* X12 file-kind auto-detection helper.
|
||||
*
|
||||
* SP35: the Upload page used to default to "837p" silently — dropping an 835
|
||||
* file on the page while the dropdown still said "837p" routed the file to
|
||||
* /api/parse-837, which silently persisted an empty batch. Layer A of the
|
||||
* defense-in-depth fix is to detect the kind from the file's first few KB
|
||||
* and switch the dropdown automatically before the user clicks Parse.
|
||||
*
|
||||
* Layer B is the server-side envelope check (cyclone.api._transaction_set_id_from_segments).
|
||||
* This helper is purely advisory: if it returns null, the UI keeps whatever
|
||||
* the user picked and the server guard rejects with a clean 400 if it's wrong.
|
||||
*
|
||||
* Detection is intentionally cheap: read the first ~4KB and look for the ST
|
||||
* segment (or the bare TA1 segment, which has no ST envelope). The full file
|
||||
* is never loaded — X12 envelopes are always within the first ISA segment.
|
||||
*/
|
||||
|
||||
import type { ParsedBatchKind } from "@/types";
|
||||
|
||||
/** Number of bytes to read from the start of the file for detection. */
|
||||
const DETECT_BYTES = 4096;
|
||||
|
||||
export type DetectedKind = ParsedBatchKind | "999" | "277ca" | "ta1" | "unknown";
|
||||
|
||||
export function detectKindFromText(text: string): DetectedKind {
|
||||
// Strip whitespace and look for the ST segment.
|
||||
// X12 envelopes are always within the first segment, so we don't need
|
||||
// to scan far. The segment terminator is `~` (per the ISA segment,
|
||||
// which we also don't need to parse here — we only care which kind of
|
||||
// payload follows).
|
||||
//
|
||||
// We do a simple substring search rather than full tokenization because:
|
||||
// - Detection must run before the user even knows if their file is
|
||||
// valid X12. If we ran `tokenize()` and the file has a malformed
|
||||
// ISA, we'd throw and the UI couldn't show a helpful message.
|
||||
// - A simple prefix match on `ST*<kind>*` is enough to disambiguate
|
||||
// every kind the Upload page might receive. False positives are
|
||||
// caught by the server-side guard.
|
||||
const upper = text.toUpperCase();
|
||||
|
||||
// 277CA uses ST*277* or ST*277CA* (per parse_277ca.py line 13 comment).
|
||||
// Check 277CA before 277 because ST*277CA contains "277".
|
||||
if (upper.includes("ST*277CA*")) return "277ca";
|
||||
if (upper.includes("ST*277*")) return "277ca";
|
||||
|
||||
// 837P accepts ST*837* or ST*837P* (the trailing P is the professional
|
||||
// claim qualifier, but the ISA envelope alone tells you it's a
|
||||
// professional file). Check 837P before 837 for the same reason.
|
||||
if (upper.includes("ST*837P*")) return "837p";
|
||||
if (upper.includes("ST*837*")) return "837p";
|
||||
|
||||
// 835 has ST*835* — no other qualifier in common use.
|
||||
if (upper.includes("ST*835*")) return "835";
|
||||
|
||||
// 999 ACK has ST*999* — no qualifier.
|
||||
if (upper.includes("ST*999*")) return "999";
|
||||
|
||||
// TA1 has no ST envelope. The interchange-ack segment is the bare TA1*
|
||||
// immediately after ISA/IEA. Match the segment header.
|
||||
if (/\bTA1\*/.test(upper)) return "ta1";
|
||||
|
||||
return "unknown";
|
||||
}
|
||||
|
||||
/**
|
||||
* Browser-side helper: read the first DETECT_BYTES from a File and detect
|
||||
* its kind. Returns a Promise so it composes naturally with FileReader /
|
||||
* Blob.slice. Returns "unknown" on read error so the caller can keep the
|
||||
* user's current selection and let the server guard surface a clean 400.
|
||||
*/
|
||||
export async function detectKindFromFile(file: File): Promise<DetectedKind> {
|
||||
try {
|
||||
const blob = file.slice(0, DETECT_BYTES);
|
||||
const text = await blob.text();
|
||||
return detectKindFromText(text);
|
||||
} catch {
|
||||
return "unknown";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Map a DetectedKind to the ParsedBatchKind the Upload dropdown supports.
|
||||
* Returns null for kinds the Upload page can't ingest (999, 277CA, TA1) —
|
||||
* the UI then surfaces a "this file isn't supported here" hint instead of
|
||||
* silently misrouting it. The server-side guards in cyclone.api still
|
||||
* catch any escape.
|
||||
*/
|
||||
export function detectedKindToParsedBatchKind(kind: DetectedKind): ParsedBatchKind | null {
|
||||
switch (kind) {
|
||||
case "837p":
|
||||
return "837p";
|
||||
case "835":
|
||||
return "835";
|
||||
default:
|
||||
return null;
|
||||
}
|
||||
}
|
||||
+33
-1
@@ -33,6 +33,10 @@ import { StatPill, ValidationDot } from "@/components/ClaimCard/shared";
|
||||
import { api, ApiError, type BatchSummary, type ParseProgress } from "@/lib/api";
|
||||
import { downloadBlob } from "@/lib/download";
|
||||
import { fmt, toNum } from "@/lib/format";
|
||||
import {
|
||||
detectKindFromFile,
|
||||
detectedKindToParsedBatchKind,
|
||||
} from "@/lib/x12-detect";
|
||||
import { useAppStore } from "@/store";
|
||||
import { useParse } from "@/hooks/useParse";
|
||||
import { useBatchExport } from "@/hooks/useBatchExport";
|
||||
@@ -437,9 +441,37 @@ export function Upload() {
|
||||
// hook call above. Keeping the comment as a breadcrumb for future
|
||||
// readers who grep "Auto-select".)
|
||||
|
||||
function pickFile(f: File | null) {
|
||||
async function pickFile(f: File | null) {
|
||||
setFile(f);
|
||||
setStream({ items: [], expectedTotal: null, passed: 0, failed: 0 });
|
||||
if (!f) return;
|
||||
// SP35: auto-detect the kind from the file's first few KB and switch
|
||||
// the dropdown before the user hits Parse. This is layer A of the
|
||||
// defense-in-depth fix (layer B is the server-side envelope guard
|
||||
// in cyclone.api). Before SP35 the dropdown defaulted to "837p"
|
||||
// silently — dropping an 835 file on the page routed it to
|
||||
// /api/parse-837 and produced a bogus empty batch. Detection runs
|
||||
// asynchronously on a 4KB slice; if it can't determine the kind
|
||||
// (unknown file, read error) we keep the user's current selection
|
||||
// and the server guard will surface a clean 400 if it's wrong.
|
||||
const detected = await detectKindFromFile(f);
|
||||
const matched = detectedKindToParsedBatchKind(detected);
|
||||
if (matched && matched !== kind) {
|
||||
setKind(matched);
|
||||
setPayer(matched === "837p" ? PAYERS_837[0]!.value : PAYERS_835[0]!.value);
|
||||
toast.message(
|
||||
`Detected ${matched === "837p" ? "837P" : "835"} file — switched the dropdown.`,
|
||||
{ description: f.name },
|
||||
);
|
||||
} else if (detected === "999" || detected === "277ca" || detected === "ta1") {
|
||||
// The Upload page only supports 837P and 835; for the other X12
|
||||
// kinds (which have their own endpoints), surface a hint instead
|
||||
// of silently leaving the dropdown on whatever the user picked.
|
||||
toast.error(
|
||||
`Detected ${detected.toUpperCase()} file — the Upload page only accepts 837P or 835.`,
|
||||
{ description: "Use the matching endpoint from the History tab or the API." },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async function onParse() {
|
||||
|
||||
Reference in New Issue
Block a user