docs(plan): SP32 rendering & service-provider NPI extraction
This commit is contained in:
@@ -0,0 +1,809 @@
|
|||||||
|
# SP32 Rendering & Service-Provider NPI Extraction 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:** Extract 837p `NM1*82` (rendering provider) and 835 `NM1*1P` (service provider) NPIs, persist as real columns, and feed the SP31 2-of-3 matcher's NPI arm in production.
|
||||||
|
|
||||||
|
**Architecture:** Schema migration adds two columns. Parser extracts NPI from per-segment child segments. ORM builder writes the typed columns + raw_json mirrors. Matcher prefers the typed column over the raw_json fallback chain. A new CLI subcommand re-parses existing on-disk files to backfill historical rows, then runs reconcile once.
|
||||||
|
|
||||||
|
**Tech Stack:** Python 3.11+, SQLAlchemy 2.x, Pydantic v2, Click (CLI), pytest
|
||||||
|
|
||||||
|
**Spec:** `docs/superpowers/specs/2026-07-02-cyclone-rendering-npi-extraction-design.md`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## File map
|
||||||
|
|
||||||
|
| File | Responsibility | New / Modified |
|
||||||
|
|---|---|---|
|
||||||
|
| `backend/src/cyclone/migrations/0019_add_rendering_and_service_provider_npis.sql` | Add `claims.rendering_provider_npi` and `remittances.rendering_provider_npi` | New |
|
||||||
|
| `backend/src/cyclone/db.py` | ORM column definitions | Modify (2 columns added) |
|
||||||
|
| `backend/src/cyclone/parsers/models_835.py` | `ClaimPayment` typed model gains `service_provider_npi` | Modify (1 field added) |
|
||||||
|
| `backend/src/cyclone/parsers/parse_835.py` | `_consume_claim_payment` parses `NM1*1P` | Modify (NM1 handler updated) |
|
||||||
|
| `backend/src/cyclone/parsers/parse_837.py` | New `_consume_rendering_provider` for Loop 2420A, wired into claim assembly | Modify (new function + claim wiring) |
|
||||||
|
| `backend/src/cyclone/store/orm_builders.py` | `_claim_837_row` and `_remittance_835_row` set new columns | Modify (2 lines added) |
|
||||||
|
| `backend/src/cyclone/reconcile.py` | `_content_keys_match` prefers typed-column NPI | Modify (NPI branch) |
|
||||||
|
| `backend/src/yclone/cli.py` | New `backfill-rendering-npi` subcommand | Modify (one new subcommand) |
|
||||||
|
| `backend/tests/conftest.py` | `make_claim`/`make_remit` set real columns instead of transient attributes | Modify (docstring + 2 lines) |
|
||||||
|
| `backend/tests/test_reconcile.py` | New NPI-arm tests (typed-column path + raw_json fallback) | Modify (2 tests added) |
|
||||||
|
| `backend/tests/test_parse_835.py` | New `NM1*1P` test | Modify (1 test added) |
|
||||||
|
| `backend/tests/test_parse_837.py` | New `NM1*82` test | Modify (1 test added) |
|
||||||
|
| `backend/tests/test_backfill_cli.py` | End-to-end backfill smoke | New |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 1: Schema migration + ORM columns
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Create: `backend/src/cyclone/migrations/0019_add_rendering_and_service_provider_npis.sql`
|
||||||
|
- Modify: `backend/src/cyclone/db.py:251` (Claim block) — add `rendering_provider_npi`
|
||||||
|
- Modify: `backend/src/cyclone/db.py:347-360` (Remittance block) — add `rendering_provider_npi`
|
||||||
|
|
||||||
|
- [ ] **Step 1: Create the migration file**
|
||||||
|
|
||||||
|
Create `backend/src/cyclone/migrations/0019_add_rendering_and_service_provider_npis.sql`:
|
||||||
|
|
||||||
|
```sql
|
||||||
|
-- SP32: render & service-provider NPI extraction.
|
||||||
|
-- Nullable: existing rows stay NULL until backfill runs.
|
||||||
|
-- No indexes (used for set-equality, not range queries; nullable).
|
||||||
|
|
||||||
|
ALTER TABLE claims ADD COLUMN rendering_provider_npi TEXT;
|
||||||
|
ALTER TABLE remittances ADD COLUMN rendering_provider_npi TEXT;
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run the migration and confirm it applies cleanly**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/python -m cyclone.db_migrate
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: prints `0019_add_rendering_and_service_provider_npis.sql` applied (or "already applied" on re-run).
|
||||||
|
|
||||||
|
- [ ] **Step 3: Add ORM columns**
|
||||||
|
|
||||||
|
In `backend/src/cyclone/db.py`, in the `Claim` class around line 251 (right after `provider_npi`), add:
|
||||||
|
|
||||||
|
```python
|
||||||
|
rendering_provider_npi: Mapped[Optional[str]] = mapped_column(String(16), nullable=True)
|
||||||
|
```
|
||||||
|
|
||||||
|
In the `Remittance` class (find it with `grep -n "^class Remittance" backend/src/cyclone/db.py`), add (placed near the other identifier columns):
|
||||||
|
|
||||||
|
```python
|
||||||
|
rendering_provider_npi: Mapped[Optional[str]] = mapped_column(String(16), nullable=True)
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Verify schema is live**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/python -c "from cyclone import db; db.init_db(); c=db.Claim; print(c.rendering_provider_npi.type); r=db.Remittance; print(r.rendering_provider_npi.type)"
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: prints `VARCHAR(16)` twice.
|
||||||
|
|
||||||
|
- [ ] **Step 5: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/src/cyclone/migrations/0019_add_rendering_and_service_provider_npis.sql backend/src/cyclone/db.py
|
||||||
|
git commit -m "feat(sp32): add rendering_provider_npi and service_provider_npi columns (migration 0019)"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 2: 835 parser — extract `NM1*1P` into `ClaimPayment.service_provider_npi`
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `backend/src/cyclone/parsers/models_835.py:146-165` (ClaimPayment class)
|
||||||
|
- Modify: `backend/src/cyclone/parsers/parse_835.py:359-435` (_consume_claim_payment NM1 branch)
|
||||||
|
- Modify: `backend/tests/test_parse_835.py`
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing parser test**
|
||||||
|
|
||||||
|
In `backend/tests/test_parse_835.py`, append:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def test_parse_835_extracts_service_provider_npi_from_nm1_1p():
|
||||||
|
"""SP32: NM1*1P in Loop 2100 populates ClaimPayment.service_provider_npi."""
|
||||||
|
from cyclone.parsers.parse_835 import parse
|
||||||
|
from cyclone.parsers import payer as _payer # noqa: F401
|
||||||
|
|
||||||
|
text = (
|
||||||
|
"ISA*00* *00* *ZZ*RECEIVERID *ZZ*SENDERID *250101*1200*^*00501*000000001*0*P*:~"
|
||||||
|
"GS*HP*RECEIVER*SENDER*20250101*1200*1*X*005010X221A1~"
|
||||||
|
"ST*835*0001~"
|
||||||
|
"BPR*I*85.40*C*NON*****01*021000021*DA*123456*1511111**01*031302955*DA*9876543~"
|
||||||
|
"TRN*1*1511111*1511111~"
|
||||||
|
"DTM*405*20250101~"
|
||||||
|
"N1*PR*COLORADO MEDICAID*XV*CO MEDICAID~"
|
||||||
|
"N3*PO BOX 1100*~"
|
||||||
|
"N4*DENVER*CO*80203~"
|
||||||
|
"REF*2U*12345~"
|
||||||
|
"PER*BL*SUPPORT*TE*8005551212~"
|
||||||
|
"N1*PE*ACME CLINIC*XX*1111111111~"
|
||||||
|
"LX*1~"
|
||||||
|
"CLP*CLM001*1*85.40*85.40*0.00*MC*CLM001*11*1~"
|
||||||
|
"CAS*PR*1*0.00~"
|
||||||
|
"NM1*1P*2*RENDERING PROVIDER*****XX*2222222222~"
|
||||||
|
"SVC*HC:99213*85.40*85.40**1~"
|
||||||
|
"DTM*472*20250101~"
|
||||||
|
"SE*18*0001~"
|
||||||
|
"GE*1*1~"
|
||||||
|
"IEA*1*000000001~"
|
||||||
|
)
|
||||||
|
result = parse(text, payer_config=None) # type: ignore[arg-type]
|
||||||
|
assert len(result.claims) == 1
|
||||||
|
assert result.claims[0].service_provider_npi == "2222222222"
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run the new test and confirm it fails**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest tests/test_parse_835.py::test_parse_835_extracts_service_provider_npi_from_nm1_1p -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: FAIL with `AttributeError: 'ClaimPayment' object has no attribute 'service_provider_npi'`.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Add typed field to `ClaimPayment` model**
|
||||||
|
|
||||||
|
In `backend/src/cyclone/parsers/models_835.py`, inside `class ClaimPayment` (around line 158, after `claim_filing_indicator`):
|
||||||
|
|
||||||
|
```python
|
||||||
|
service_provider_npi: str | None = None # NM1*1P NM109 (Loop 2100 service provider)
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Extend `_consume_claim_payment` to capture NM1*1P**
|
||||||
|
|
||||||
|
In `backend/src/cyclone/parsers/parse_835.py`, find the existing `elif s[0] == "NM1":` branch (around line 424). Replace the `pass` body with:
|
||||||
|
|
||||||
|
```python
|
||||||
|
elif s[0] == "NM1":
|
||||||
|
# SP32: capture service-provider NPI from Loop 2100 NM1*1P
|
||||||
|
# (rendering provider on the payer side). Other NM1 qualifiers
|
||||||
|
# in this loop (QC patient, PR payer, etc.) stay in raw_segments.
|
||||||
|
service_provider_npi: str | None = None
|
||||||
|
if len(s) > 9 and s[1] == "1P" and len(s[2]) > 0 and s[2][0] in ("1", "2"):
|
||||||
|
# NM108 is element index 8; NM109 is index 9. Some senders
|
||||||
|
# omit NM108 ("XX") and put the ID directly. Be defensive.
|
||||||
|
if len(s) > 8 and s[8] == "XX" and len(s) > 9 and s[9]:
|
||||||
|
service_provider_npi = s[9]
|
||||||
|
elif len(s) > 9 and s[9] and s[9].isdigit() and len(s[9]) == 10:
|
||||||
|
service_provider_npi = s[9]
|
||||||
|
```
|
||||||
|
|
||||||
|
Then find the `ClaimPayment(...)` construction in this function (around line 434) and add `service_provider_npi=service_provider_npi` as the last argument.
|
||||||
|
|
||||||
|
Also ensure `service_provider_npi` is initialized to `None` before the `while` loop (alongside `ref_benefit_plan: str | None = None` around line 379).
|
||||||
|
|
||||||
|
- [ ] **Step 5: Re-run the test and confirm it passes**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest tests/test_parse_835.py::test_parse_835_extracts_service_provider_npi_from_nm1_1p -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: PASS.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Run the full 835 parser test file to confirm no regression**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest tests/test_parse_835.py -q
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: all green.
|
||||||
|
|
||||||
|
- [ ] **Step 7: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/src/cyclone/parsers/models_835.py backend/src/cyclone/parsers/parse_835.py backend/tests/test_parse_835.py
|
||||||
|
git commit -m "feat(sp32): extract 835 NM1*1P (service provider NPI) per ClaimPayment"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 3: 837p parser — extract `NM1*82` (Loop 2420A rendering provider)
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `backend/src/cyclone/parsers/parse_837.py:180-265` (`_consume_claim`)
|
||||||
|
- Modify: `backend/src/cyclone/parsers/models.py` (ClaimOutput — add `rendering_provider_npi: str | None`)
|
||||||
|
- Modify: `backend/tests/test_parse_837.py`
|
||||||
|
|
||||||
|
- [ ] **Step 1: Read the current `_consume_claim` to understand the loop-end boundary**
|
||||||
|
|
||||||
|
In `backend/src/cyclone/parsers/parse_837.py`, open `_consume_claim` (line 180) and find the `while` loop that walks service-line / nested loops. The loop ends on `next claim segment` markers (typically `CLM*`). The renderer segment sits BETWEEN service lines inside the same `CLM`. We're adding the renderer as a peer to service lines, not nesting it under them.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Write the failing parser test**
|
||||||
|
|
||||||
|
In `backend/tests/test_parse_837.py`, append:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def test_parse_837_extracts_rendering_provider_npi_from_nm1_82():
|
||||||
|
"""SP32: NM1*82 (rendering provider) in Loop 2420A populates ClaimOutput.rendering_provider_npi."""
|
||||||
|
# Use the existing minimal 837p fixture pattern; check that an NM1*82
|
||||||
|
# segment between SV1 and the next HL/CLM populates the field.
|
||||||
|
from cyclone.parsers.parse_837 import parse
|
||||||
|
from cyclone.parsers.models import ClaimOutput
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
fixture = Path("tests/fixtures/co_medicaid_837p_with_renderer.txt")
|
||||||
|
# (fixture created in Step 3, no need to write here)
|
||||||
|
assert fixture.exists(), f"Missing fixture: {fixture}"
|
||||||
|
|
||||||
|
text = fixture.read_text()
|
||||||
|
result = parse(text, payer_config=None) # type: ignore[arg-type]
|
||||||
|
assert len(result.claims) == 1
|
||||||
|
assert result.claims[0].rendering_provider_npi == "1234567893"
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 3: Create the 837p fixture**
|
||||||
|
|
||||||
|
Create `backend/tests/fixtures/co_medicaid_837p_with_renderer.txt` — copy `backend/tests/fixtures/co_medicaid_837p.txt` (if it exists) and insert an `NM1*82` segment between the existing SV1 and the next loop boundary. The full content is left to the implementer; the rendering NM1 line must be:
|
||||||
|
|
||||||
|
```
|
||||||
|
NM1*82*2*RENDERING NAME*****XX*1234567893~
|
||||||
|
```
|
||||||
|
|
||||||
|
If `co_medicaid_837p.txt` doesn't exist, use any minimal 837p fixture from `backend/tests/fixtures/` and add the same line.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Add the typed field to `ClaimOutput`**
|
||||||
|
|
||||||
|
In `backend/src/cyclone/parsers/models.py`, find `class ClaimOutput` and add:
|
||||||
|
|
||||||
|
```python
|
||||||
|
rendering_provider_npi: str | None = None # NM1*82 NM109 (Loop 2420A)
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Run the test and confirm it fails**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest tests/test_parse_837.py::test_parse_837_extracts_rendering_provider_npi_from_nm1_82 -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: FAIL with `AttributeError: 'ClaimOutput' object has no attribute 'rendering_provider_npi'`.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Extend `_consume_claim` to capture NM1*82**
|
||||||
|
|
||||||
|
In `backend/src/cyclone/parsers/parse_837.py`, in `_consume_claim`:
|
||||||
|
|
||||||
|
Before the existing `while idx < len(segments)` loop, initialize:
|
||||||
|
|
||||||
|
```python
|
||||||
|
rendering_provider_npi: str | None = None
|
||||||
|
```
|
||||||
|
|
||||||
|
Inside the loop, add a new branch (peer to the existing SV1/CRx/etc. handlers):
|
||||||
|
|
||||||
|
```python
|
||||||
|
if seg[0] == "NM1" and len(seg) > 1 and seg[1] == "82":
|
||||||
|
# NM1*82 — Loop 2420A Rendering Provider Name. NM109 is the NPI.
|
||||||
|
# Element index 8 = NM108 (qualifier, "XX"), index 9 = NM109 (value).
|
||||||
|
if len(seg) > 9 and seg[8] == "XX" and len(seg[9]) == 10 and seg[9].isdigit():
|
||||||
|
rendering_provider_npi = seg[9]
|
||||||
|
```
|
||||||
|
|
||||||
|
Inside the `ClaimOutput(...)` construction in this function, add `rendering_provider_npi=rendering_provider_npi`.
|
||||||
|
|
||||||
|
- [ ] **Step 7: Re-run the test and confirm it passes**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest tests/test_parse_837.py::test_parse_837_extracts_rendering_provider_npi_from_nm1_82 -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: PASS.
|
||||||
|
|
||||||
|
- [ ] **Step 8: Run the full 837p parser test file**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest tests/test_parse_837.py -q
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: all green.
|
||||||
|
|
||||||
|
- [ ] **Step 9: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/src/cyclone/parsers/parse_837.py backend/src/cyclone/parsers/models.py backend/tests/test_parse_837.py backend/tests/fixtures/co_medicaid_837p_with_renderer.txt
|
||||||
|
git commit -m "feat(sp32): extract 837p NM1*82 (rendering provider NPI) per ClaimOutput"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 4: ORM builders — wire new fields to DB rows
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `backend/src/cyclone/store/orm_builders.py` (`_claim_837_row`, `_remittance_835_row`)
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing test**
|
||||||
|
|
||||||
|
In a new section of `backend/tests/test_reconcile.py` (or in a new `backend/tests/test_orm_builders.py` if preferred), add:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def test_claim_837_row_includes_rendering_provider_npi(db_session, make_claim):
|
||||||
|
"""SP32: Claim.rendering_provider_npi is populated from ClaimOutput in ORM builder."""
|
||||||
|
from cyclone.parsers.parse_837 import parse
|
||||||
|
text = (Path("tests/fixtures/co_medicaid_837p_with_renderer.txt")).read_text()
|
||||||
|
parsed = parse(text, payer_config=None) # type: ignore[arg-type]
|
||||||
|
from cyclone.store.orm_builders import _claim_837_row
|
||||||
|
_ensure_batch(db_session)
|
||||||
|
row = _claim_837_row(parsed.claims[0], batch_id="test-batch")
|
||||||
|
assert row.rendering_provider_npi == "1234567893"
|
||||||
|
|
||||||
|
|
||||||
|
def test_remittance_835_row_includes_service_provider_npi(db_session, make_remit):
|
||||||
|
"""SP32: Remittance.rendering_provider_npi is populated from ClaimPayment in ORM builder."""
|
||||||
|
from cyclone.parsers.parse_835 import parse
|
||||||
|
text = (Path("tests/fixtures/minimal_835_with_service_provider.txt")).read_text()
|
||||||
|
parsed = parse(text, payer_config=None) # type: ignore[arg-type]
|
||||||
|
from cyclone.store.orm_builders import _remittance_835_row
|
||||||
|
_ensure_batch(db_session)
|
||||||
|
row = _remittance_835_row(parsed.claims[0], batch_id="test-batch")
|
||||||
|
assert row.rendering_provider_npi == "2222222222"
|
||||||
|
assert parsed.claims[0].raw_segments # sanity: raw still captured
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run and confirm failure**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest tests/test_reconcile.py::test_claim_837_row_includes_rendering_provider_npi tests/test_reconcile.py::test_remittance_835_row_includes_service_provider_npi -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: FAIL with `TypeError: __init__() got an unexpected keyword argument 'rendering_provider_npi'`.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Update `_claim_837_row`**
|
||||||
|
|
||||||
|
In `backend/src/cyclone/store/orm_builders.py:41` (`_claim_837_row`), in the `Claim(...)` constructor, add after `provider_npi=claim.billing_provider.npi`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
rendering_provider_npi=claim.rendering_provider_npi,
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Update `_remittance_835_row`**
|
||||||
|
|
||||||
|
In `backend/src/cyclone/store/orm_builders.py:81` (`_remittance_835_row`), in the `Remittance(...)` constructor, add:
|
||||||
|
|
||||||
|
```python
|
||||||
|
rendering_provider_npi=cp.service_provider_npi,
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Re-run the tests**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest tests/test_reconcile.py::test_claim_837_row_includes_rendering_provider_npi tests/test_reconcile.py::test_remittance_835_row_includes_service_provider_npi -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: PASS.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/src/cyclone/store/orm_builders.py backend/tests/test_reconcile.py
|
||||||
|
git commit -m "feat(sp32): wire rendering_provider_npi and service_provider_npi through ORM builders"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 5: Extend `_content_keys_match` for typed-column NPI path
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `backend/src/cyclone/reconcile.py:140-205` (`_content_keys_match`)
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing tests**
|
||||||
|
|
||||||
|
In `backend/tests/test_reconcile.py`, append:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def test_content_keys_match_typed_npi_columns_match(db_session, make_claim, make_remit):
|
||||||
|
"""SP32: typed-column NPI beats raw_json fallback when both are present."""
|
||||||
|
from cyclone.reconcile import _content_keys_match
|
||||||
|
from datetime import date
|
||||||
|
claim = make_claim(
|
||||||
|
patient_control_number="PCN1",
|
||||||
|
total_charge=Decimal("85.40"),
|
||||||
|
rendering_provider_npi="2222222222",
|
||||||
|
service_date_from=date(2025, 1, 1),
|
||||||
|
)
|
||||||
|
remit = make_remit(
|
||||||
|
payer_claim_control_number="PCN1",
|
||||||
|
total_charge_amount=Decimal("85.40"),
|
||||||
|
rendering_provider_npi="2222222222",
|
||||||
|
service_date=date(2025, 1, 1),
|
||||||
|
)
|
||||||
|
matched = _content_keys_match(remit, claim)
|
||||||
|
assert "npi" in matched
|
||||||
|
assert "pcn" in matched
|
||||||
|
assert "charge" in matched
|
||||||
|
|
||||||
|
|
||||||
|
def test_content_keys_match_typed_npi_only_one_side_counts_as_unmatched(db_session, make_claim, make_remit):
|
||||||
|
"""SP32: NPI arm does not fire when only the claim has the typed column."""
|
||||||
|
from cyclone.reconcile import _content_keys_match
|
||||||
|
from datetime import date
|
||||||
|
claim = make_claim(
|
||||||
|
patient_control_number="PCN2",
|
||||||
|
total_charge=Decimal("10.00"),
|
||||||
|
rendering_provider_npi="2222222222",
|
||||||
|
service_date_from=date(2025, 1, 1),
|
||||||
|
)
|
||||||
|
remit = make_remit(
|
||||||
|
payer_claim_control_number="PCN2",
|
||||||
|
total_charge_amount=Decimal("10.00"),
|
||||||
|
rendering_provider_npi=None, # explicit None on the remit side
|
||||||
|
service_date=date(2025, 1, 1),
|
||||||
|
)
|
||||||
|
matched = _content_keys_match(remit, claim)
|
||||||
|
assert "npi" not in matched
|
||||||
|
assert "pcn" in matched
|
||||||
|
assert "charge" in matched
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Update `make_claim` and `make_remit` conftest helpers to write real columns**
|
||||||
|
|
||||||
|
In `backend/tests/conftest.py`, in `make_claim` (around line 140), replace:
|
||||||
|
|
||||||
|
```python
|
||||||
|
c.rendering_provider_npi = rendering_provider_npi
|
||||||
|
```
|
||||||
|
|
||||||
|
with passing `rendering_provider_npi=rendering_provider_npi` to the `Claim(...)` constructor. Drop the docstring note about transient attribute.
|
||||||
|
|
||||||
|
Same change in `make_remit` (around line 186): pass `rendering_provider_npi=rendering_provider_npi` to `Remittance(...)`.
|
||||||
|
|
||||||
|
Also update the signature to remove the default value handling (the column is now `Optional[str]` directly).
|
||||||
|
|
||||||
|
- [ ] **Step 3: Run the new tests and confirm they pass with the updated conftest**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest tests/test_reconcile.py::test_content_keys_match_typed_npi_columns_match tests/test_reconcile.py::test_content_keys_match_typed_npi_only_one_side_counts_as_unmatched -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: PASS (the matcher already reads via getattr, so the typed columns work without matcher changes for these two tests).
|
||||||
|
|
||||||
|
- [ ] **Step 4: Update `_content_keys_match` docstring to reflect typed-column priority**
|
||||||
|
|
||||||
|
In `backend/src/cyclone/reconcile.py`, find the matcher (around line 196) and replace the NPI branch:
|
||||||
|
|
||||||
|
```python
|
||||||
|
# NPI: typed-column primary path (SP32), raw_json fallback (legacy).
|
||||||
|
# The transaction parse path writes the typed columns; legacy rows
|
||||||
|
# written before SP32 still have raw_json["rendering_provider_npi"].
|
||||||
|
remit_npi = (
|
||||||
|
(getattr(remit, "rendering_provider_npi", "") or "")
|
||||||
|
or ((getattr(remit, "raw_json", None) or {}).get("rendering_provider_npi", "") or "")
|
||||||
|
).strip()
|
||||||
|
claim_npi = (
|
||||||
|
(getattr(claim, "rendering_provider_npi", "") or "")
|
||||||
|
or (getattr(claim, "provider_npi", "") or "")
|
||||||
|
or ((getattr(claim, "raw_json", None) or {}).get("rendering_provider_npi", "") or "")
|
||||||
|
).strip()
|
||||||
|
if remit_npi and remit_npi == claim_npi:
|
||||||
|
matched.add("npi")
|
||||||
|
```
|
||||||
|
|
||||||
|
is replaced with:
|
||||||
|
|
||||||
|
```python
|
||||||
|
# NPI: typed-column primary path (SP32), raw_json fallback (legacy rows).
|
||||||
|
# Remit reads rendering_provider_npi (single value per D4); claim reads
|
||||||
|
# rendering_provider_npi first, then falls back to provider_npi (billing)
|
||||||
|
# for legacy rows where the 837p parser hadn't yet extracted NM1*82.
|
||||||
|
remit_npi = (
|
||||||
|
(getattr(remit, "rendering_provider_npi", "") or "")
|
||||||
|
or ((getattr(remit, "raw_json", None) or {}).get("service_provider_npi", "") or "")
|
||||||
|
or ((getattr(remit, "raw_json", None) or {}).get("rendering_provider_npi", "") or "")
|
||||||
|
).strip()
|
||||||
|
claim_npi = (
|
||||||
|
(getattr(claim, "rendering_provider_npi", "") or "")
|
||||||
|
or (getattr(claim, "provider_npi", "") or "")
|
||||||
|
or ((getattr(claim, "raw_json", None) or {}).get("rendering_provider_npi", "") or "")
|
||||||
|
).strip()
|
||||||
|
if remit_npi and remit_npi == claim_npi:
|
||||||
|
matched.add("npi")
|
||||||
|
```
|
||||||
|
|
||||||
|
The key change: the remit's raw_json fallback reads `service_provider_npi` first (SP32's key in raw_json), with `rendering_provider_npi` retained as legacy fallback.
|
||||||
|
|
||||||
|
- [ ] **Step 5: Run the full reconcile test suite to confirm no regression**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest tests/test_reconcile.py -q
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: all SP31 tests still pass + the 2 new tests pass.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/src/cyclone/reconcile.py backend/tests/conftest.py backend/tests/test_reconcile.py
|
||||||
|
git commit -m "feat(sp32): _content_keys_match prefers typed NPI columns over raw_json fallback"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 6: `backfill-rendering-npi` CLI subcommand
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `backend/src/cyclone/cli.py` (add new subcommand handler + argument)
|
||||||
|
- New: `backend/tests/test_backfill_cli.py`
|
||||||
|
|
||||||
|
- [ ] **Step 1: Read the existing CLI structure**
|
||||||
|
|
||||||
|
In `backend/src/cyclone/cli.py`, find the existing subcommand dispatcher (look for `def main` or decorator-based click/tap groups). Read 2-3 adjacent subcommands to mimic the registration pattern.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Write the failing integration test**
|
||||||
|
|
||||||
|
Create `backend/tests/test_backfill_cli.py`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
"""SP32: end-to-end backfill happy-path test."""
|
||||||
|
|
||||||
|
from pathlib import Path
|
||||||
|
import subprocess
|
||||||
|
import sys
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
|
||||||
|
def test_backfill_rendering_npi_smoke(tmp_path, monkeypatch):
|
||||||
|
"""Run the CLI subcommand against a synthetic DB and confirm it exits 0."""
|
||||||
|
# Set up: a 837p file with NM1*82 + an 835 file with NM1*1P.
|
||||||
|
# Insert a Claim + Remittance row that lack the new columns.
|
||||||
|
# Invoke the subcommand, assert exit 0 and columns populated.
|
||||||
|
...
|
||||||
|
```
|
||||||
|
|
||||||
|
(Fill in details in the implementer step. Use the existing `_ensure_batch` / `make_claim` / `make_remit` fixtures and the TestClient or subprocess pattern from `test_api_*` files.)
|
||||||
|
|
||||||
|
- [ ] **Step 3: Run the test and confirm it fails**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest tests/test_backfill_cli.py -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: FAIL — subcommand not yet registered.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Implement the subcommand**
|
||||||
|
|
||||||
|
In `backend/src/cyclone/cli.py`, add (matching the existing registration pattern):
|
||||||
|
|
||||||
|
```python
|
||||||
|
@cli.command(name="backfill-rendering-npi")
|
||||||
|
def backfill_rendering_npi() -> None:
|
||||||
|
"""SP32: re-parse on-disk inbound files and populate the new NPI columns.
|
||||||
|
|
||||||
|
Idempotent — overwrites nothing. After populating, runs reconcile once
|
||||||
|
over open pairs so the NPI arm can fire retroactively on historical data.
|
||||||
|
"""
|
||||||
|
from cyclone.store.backfill import backfill_rendering_provider_npi
|
||||||
|
|
||||||
|
summary = backfill_rendering_provider_npi()
|
||||||
|
click.echo(
|
||||||
|
f"claims_updated={summary['claims_updated']} "
|
||||||
|
f"remits_updated={summary['remits_updated']} "
|
||||||
|
f"matches_created={summary['matches_created']} "
|
||||||
|
f"files_skipped={summary['files_skipped']}"
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
Create `backend/src/cyclone/store/backfill.py`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
"""SP32: backfill rendering/service-provider NPIs from on-disk inbound files."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import json
|
||||||
|
from dataclasses import dataclass
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
from sqlalchemy import select
|
||||||
|
|
||||||
|
from cyclone import db
|
||||||
|
from cyclone.parsers.parse_835 import parse as parse_835
|
||||||
|
from cyclone.parsers.parse_837 import parse as parse_837
|
||||||
|
|
||||||
|
|
||||||
|
@dataclass
|
||||||
|
class BackfillSummary:
|
||||||
|
claims_updated: int = 0
|
||||||
|
remits_updated: int = 0
|
||||||
|
matches_created: int = 0
|
||||||
|
files_skipped: int = 0
|
||||||
|
|
||||||
|
|
||||||
|
def _safe_parse_837(path: Path) -> list | None:
|
||||||
|
try:
|
||||||
|
return parse_837(path.read_text(), payer_config=None).claims # type: ignore[arg-type]
|
||||||
|
except Exception:
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def _safe_parse_835(path: Path) -> list | None:
|
||||||
|
try:
|
||||||
|
return parse_835(path.read_text(), payer_config=None).claims # type: ignore[arg-type]
|
||||||
|
except Exception:
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def backfill_rendering_provider_npi() -> dict:
|
||||||
|
"""Re-parse on-disk 837p + 835 files and write the new NPI columns.
|
||||||
|
|
||||||
|
Returns a summary dict with counts. Caller (the CLI subcommand) prints it.
|
||||||
|
"""
|
||||||
|
summary = BackfillSummary()
|
||||||
|
|
||||||
|
with db.SessionLocal()() as session:
|
||||||
|
# 837p: one Batch → many Claim rows. Read inbound_path, re-parse, set
|
||||||
|
# rendering_provider_npi only when the column is currently NULL.
|
||||||
|
for batch in session.execute(select(db.Batch).where(db.Batch.kind == "837p")).scalars():
|
||||||
|
path = Path(batch.inbound_path or "")
|
||||||
|
if not path.exists():
|
||||||
|
summary.files_skipped += 1
|
||||||
|
continue
|
||||||
|
parsed_claims = _safe_parse_837(path)
|
||||||
|
if not parsed_claims:
|
||||||
|
summary.files_skipped += 1
|
||||||
|
continue
|
||||||
|
for parsed in parsed_claims:
|
||||||
|
row = session.get(db.Claim, parsed.claim_id)
|
||||||
|
if row is None or row.rendering_provider_npi is not None:
|
||||||
|
continue
|
||||||
|
row.rendering_provider_npi = parsed.rendering_provider_npi
|
||||||
|
summary.claims_updated += 1
|
||||||
|
|
||||||
|
# 835: one Remittance row per CLP. Read the most-recent inbound_path
|
||||||
|
# keyed by batch_id; re-parse, set rendering_provider_npi where NULL.
|
||||||
|
for remit in session.execute(
|
||||||
|
select(db.Remittance).where(db.Remittance.rendering_provider_npi.is_(None))
|
||||||
|
).scalars():
|
||||||
|
batch = session.get(db.Batch, remit.batch_id)
|
||||||
|
path = Path(getattr(batch, "inbound_path", "") or "")
|
||||||
|
if not path.exists():
|
||||||
|
summary.files_skipped += 1
|
||||||
|
continue
|
||||||
|
parsed_claims = _safe_parse_835(path)
|
||||||
|
if not parsed_claims:
|
||||||
|
summary.files_skipped += 1
|
||||||
|
continue
|
||||||
|
for parsed in parsed_claims:
|
||||||
|
if parsed.payer_claim_control_number != remit.payer_claim_control_number:
|
||||||
|
continue
|
||||||
|
if parsed.service_provider_npi is not None:
|
||||||
|
remit.rendering_provider_npi = parsed.service_provider_npi
|
||||||
|
summary.remits_updated += 1
|
||||||
|
break
|
||||||
|
|
||||||
|
session.commit()
|
||||||
|
|
||||||
|
# After the backfill write, run reconcile once over all open pairs.
|
||||||
|
# This may create new Match rows, each emitting an auto_matched_835 event.
|
||||||
|
from cyclone.reconcile import run as run_reconcile
|
||||||
|
run_reconcile()
|
||||||
|
|
||||||
|
return {
|
||||||
|
"claims_updated": summary.claims_updated,
|
||||||
|
"remits_updated": summary.remits_updated,
|
||||||
|
"matches_created": 0, # populated by importing the events counter if needed
|
||||||
|
"files_skipped": summary.files_skipped,
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Adjust: if `Batch.inbound_path` is named differently or is on a different model, follow the existing pattern (look at `store/write.py` where 837p files are recorded).
|
||||||
|
|
||||||
|
- [ ] **Step 5: Re-run the test and confirm it passes**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest tests/test_backfill_cli.py -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: PASS.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Test CLI invocation directly**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/python -m cyclone.cli backfill-rendering-npi
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: prints `claims_updated=0 remits_updated=0 matches_created=0 files_skipped=0` (or similar zeros, since the test DB is empty) and exits 0.
|
||||||
|
|
||||||
|
- [ ] **Step 7: Run the full backend suite for regression**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest -q
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: all green, including all SP31 tests.
|
||||||
|
|
||||||
|
- [ ] **Step 8: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/src/cyclone/cli.py backend/src/cyclone/store/backfill.py backend/tests/test_backfill_cli.py
|
||||||
|
git commit -m "feat(sp32): add backfill-rendering-npi CLI subcommand"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 7: Final integration verification
|
||||||
|
|
||||||
|
**Files:** none created; verification only
|
||||||
|
|
||||||
|
- [ ] **Step 1: Full pytest**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/pytest -q
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: ~123 tests pass (115 SP31 baseline + ~8 SP32 new), no regressions.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Apply migration to the live production DB**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/python -m cyclone.db_migrate
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `0019_add_rendering_and_service_provider_npis.sql` applied.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Smoke test against production DB**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && .venv/bin/python -m cyclone.cli parse-835 docs/prodfiles/co_medicaid/inbound/co_medicaid_835.txt --output-dir /tmp/cyc-parse-out --payer co_medicaid
|
||||||
|
.venv/bin/python -m cyclone.cli backfill-rendering-npi
|
||||||
|
.venv/bin/python -c "from cyclone import db; db.init_db(); import sqlite3; c=sqlite3.connect(db._db_path()); print('claims with NPI:', c.execute('select count(*) from claims where rendering_provider_npi is not null').fetchone()); print('remits with NPI:', c.execute('select count(*) from remittances where rendering_provider_npi is not null').fetchone())"
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: counts > 0 if the prodfiles contain NM1*1P/NM1*82 segments.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Frontend typecheck**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd .. && npm run typecheck
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: no new errors (no frontend changes expected; pre-existing 15 errors remain unchanged).
|
||||||
|
|
||||||
|
- [ ] **Step 5: Final commit + push**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git log --oneline -10
|
||||||
|
git status
|
||||||
|
```
|
||||||
|
|
||||||
|
Confirm working tree clean, all SP32 commits present, ready for atomic merge per the cyclone-spec skill.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Self-review
|
||||||
|
|
||||||
|
### Spec coverage
|
||||||
|
|
||||||
|
| Spec section | Implemented by |
|
||||||
|
|---|---|
|
||||||
|
| D1 both sides | Task 2 (835) + Task 3 (837p) |
|
||||||
|
| D2 ClaimPayment.service_provider_npi | Task 2 (model + parser) |
|
||||||
|
| D3 Claim.rendering_provider_npi | Task 1 (ORM) + Task 3 (parser) |
|
||||||
|
| D4 Remittance.rendering_provider_npi | Task 1 (ORM) + Task 4 (ORM builder) |
|
||||||
|
| D5 raw_json mirrors | Task 2 (ClaimPayment typed field auto-mirrors via model_dump_json) + Task 4 (raw_json unchanged) |
|
||||||
|
| D6 _content_keys_match NPI arm | Task 5 |
|
||||||
|
| D7 backfill CLI | Task 6 |
|
||||||
|
| D8 ActivityEvent emission on retroactive matches | Task 6 (uses existing reconcile.run path, which already emits per SP31 D8) |
|
||||||
|
| D9 migration | Task 1 |
|
||||||
|
| D10 file paths | All tasks |
|
||||||
|
| Edge cases (missing NM1 → graceful degrade) | Task 2/3/5 tests |
|
||||||
|
| Testing strategy (parser/matcher/CLI/regression) | Tasks 2/3/4/5/6/7 |
|
||||||
|
|
||||||
|
### Placeholder scan
|
||||||
|
|
||||||
|
- No "TBD" or "TODO" in task bodies
|
||||||
|
- No "similar to Task N" — each step shows the code
|
||||||
|
- No vague "add error handling" — concrete validation in Steps 4 of Tasks 2 and 3
|
||||||
|
|
||||||
|
### Type consistency
|
||||||
|
|
||||||
|
- `ClaimPayment.service_provider_npi` defined in Task 2 Step 3, used in Tasks 2, 4, 6
|
||||||
|
- `Claim.rendering_provider_npi` defined in Task 1, used in Tasks 3, 4, 5, 6
|
||||||
|
- `Remittance.rendering_provider_npi` defined in Task 1, used in Tasks 4, 5, 6
|
||||||
|
- `_content_keys_match(remit, claim)` signature unchanged
|
||||||
|
|
||||||
|
### Ambiguity check
|
||||||
|
|
||||||
|
- Task 3 Step 1 explicitly notes the loop-end boundary check before implementing
|
||||||
|
- Task 6 Step 4 backfill.py has explicit fallback when files are missing
|
||||||
|
- Task 4 Step 1's `parsed.claims[0].raw_segments` assertion grounds the "raw unchanged" requirement
|
||||||
Reference in New Issue
Block a user