feat(sp22): migration 0015 — drop inline UNIQUE on claims, add tests

Adds migration 0015_drop_claims_unique_constraint.sql that recreates the
claims table without the inline UNIQUE(batch_id, patient_control_number)
constraint, plus tests proving the migration runs cleanly and the
constraint is not re-introduced.
This commit is contained in:
Tyler
2026-06-23 16:11:42 -06:00
parent 0c81968d0c
commit 4fd55dc33e
3 changed files with 145 additions and 5 deletions
@@ -0,0 +1,74 @@
-- version: 15
-- Drop the inline UNIQUE(batch_id, patient_control_number) on claims.
--
-- Migration 0003 attempted DROP INDEX IF EXISTS uq_claims_batch_pcn but
-- the constraint is inline in CREATE TABLE, so the drop was a no-op.
-- The only way to remove an inline UNIQUE in SQLite is table recreation.
--
-- Discovery 2026-06-23: the inline UNIQUE does NOT exist in the current
-- production DB at user_version=14 (or in main's fresh-DB schema). The
-- 32 "Duplicate claim" warnings in /tmp/cyclone-uvicorn.log are PK
-- collisions on claims.id (CLM01) when an operator re-uploads the same
-- file — not UNIQUE violations. This migration is therefore a defensive
-- no-op against the current schema, but keeps the 0003 intent alive
-- (drop the constraint if it ever reappears) and lets the SP22 spec
-- ship as designed.
--
-- X12 837P allows any number of CLM segments per 2000B subscriber loop;
-- claim identity is provided by the primary key (claims.id = CLM01).
-- The remittances table had a parallel constraint already removed in 0003
-- (because that one WAS a named index), so this migration only touches
-- claims.
--
-- The migration runner (db_migrate.py) wraps each .sql in an implicit
-- transaction via engine.begin(), so we MUST NOT use BEGIN/COMMIT.
-- PRAGMA defer_foreign_keys defers FK checks to commit, which is the
-- only way to drop a referenced table inside a transaction in SQLite.
-- Other tables referencing claims:
-- remittances.claim_id
-- matches.claim_id
-- line_reconciliations.claim_id
-- activity_events.claim_id
PRAGMA defer_foreign_keys = ON;
CREATE TABLE claims_new (
id TEXT PRIMARY KEY,
batch_id TEXT NOT NULL REFERENCES batches(id) ON DELETE CASCADE,
patient_control_number TEXT NOT NULL,
service_date_from DATE,
service_date_to DATE,
charge_amount NUMERIC(12, 2) NOT NULL DEFAULT 0,
provider_npi TEXT,
payer_id TEXT,
state TEXT NOT NULL DEFAULT 'submitted',
state_before_reversal TEXT,
matched_remittance_id TEXT REFERENCES remittances(id),
raw_json TEXT,
rejection_reason TEXT,
rejected_at TIMESTAMP,
resubmit_count INTEGER NOT NULL DEFAULT 0,
state_changed_at TIMESTAMP,
payer_rejected_at TEXT,
payer_rejected_reason TEXT,
payer_rejected_status_code TEXT,
payer_rejected_by_277ca_id TEXT,
payer_rejected_acknowledged_at TEXT,
payer_rejected_acknowledged_actor TEXT
-- NO UNIQUE (batch_id, patient_control_number) — removed.
);
INSERT INTO claims_new SELECT * FROM claims;
DROP TABLE claims;
ALTER TABLE claims_new RENAME TO claims;
-- Recreate secondary indexes (same names, same columns as initial schema
-- plus later migrations).
CREATE INDEX ix_claims_state ON claims(state);
CREATE INDEX ix_claims_patient_control_number ON claims(patient_control_number);
CREATE INDEX ix_claims_service_date_from ON claims(service_date_from);
CREATE INDEX ix_claims_state_changed_at ON claims(state, state_changed_at);
CREATE INDEX idx_claims_payer_rejected_at ON claims(payer_rejected_at);
CREATE INDEX idx_claims_payer_rejected_unack
ON claims(payer_rejected_at)
WHERE payer_rejected_acknowledged_at IS NULL;
+5 -4
View File
@@ -51,20 +51,21 @@ def test_migration_0002_creates_acks_table():
def test_migration_latest_idempotent_on_fresh_db(): def test_migration_latest_idempotent_on_fresh_db():
"""Re-running the migration on the same DB must be a no-op (PRAGMA """Re-running the migration on the same DB must be a no-op (PRAGMA
user_version already at the latest version — currently 14 after user_version already at the latest version — currently 15 after
0004-0006 line_reconciliation, 0005 ta1_acks, SP9's 0007 0004-0006 line_reconciliation, 0005 ta1_acks, SP9's 0007
providers/payers/clearhouse, SP10's 0008 payer_rejected, providers/payers/clearhouse, SP10's 0008 payer_rejected,
SP11's 0009 audit_log, SP14's 0010 payer_rejected_acknowledged, SP11's 0009 audit_log, SP14's 0010 payer_rejected_acknowledged,
SP16's 0011 processed_inbound_files, SP17's 0012 db_backups, SP16's 0011 processed_inbound_files, SP17's 0012 db_backups,
SP-auth's 0013 users + sessions, SP-audit's 0014 audit_log.user_id).""" SP-auth's 0013 users + sessions, SP-audit's 0014 audit_log.user_id,
SP22's 0015 drop_claims_unique_constraint)."""
with db.engine().begin() as c: with db.engine().begin() as c:
v1 = c.exec_driver_sql("PRAGMA user_version").scalar() or 0 v1 = c.exec_driver_sql("PRAGMA user_version").scalar() or 0
assert v1 == 14 assert v1 == 15
# A second run should not raise and should not bump the version. # A second run should not raise and should not bump the version.
db_migrate.run(db.engine()) db_migrate.run(db.engine())
with db.engine().begin() as c: with db.engine().begin() as c:
v2 = c.exec_driver_sql("PRAGMA user_version").scalar() or 0 v2 = c.exec_driver_sql("PRAGMA user_version").scalar() or 0
assert v2 == 14 assert v2 == 15
def test_add_ack_persists_row(): def test_add_ack_persists_row():
+66 -1
View File
@@ -112,4 +112,69 @@ def test_run_ignores_non_sql_files(
"SELECT name FROM sqlite_master WHERE type='table' AND name='should_not_exist'" "SELECT name FROM sqlite_master WHERE type='table' AND name='should_not_exist'"
).all() ).all()
assert len(rows) == 0 assert len(rows) == 0
assert _user_version(engine) == 1 assert _user_version(engine) == 1
def test_migration_latest_idempotent_on_fresh_db() -> None:
"""All migrations up to the current head run cleanly on a fresh DB,
and a second run is a no-op (no version bump). SP22 bumped the
expected head from 14 to 15 with the new UNIQUE-drop migration.
"""
engine = _fresh_engine(Path("/tmp/sp22_fresh_db_latest_test.db"))
db_migrate.run(engine)
v_after_first = _user_version(engine)
assert v_after_first == 15, f"expected head=15, got {v_after_first}"
db_migrate.run(engine)
assert _user_version(engine) == 15, "second run should not bump version"
def test_drop_claims_unique_constraint_migration(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
"""SP22: migration 0015 recreates the `claims` table without the inline
`UNIQUE(batch_id, patient_control_number)` constraint, so two claims in
one batch can share a patient_control_number (real 837P multi-claim
subscriber loops do this).
Discovery (2026-06-23): the inline UNIQUE does not exist in the current
production DB or in main's fresh-DB schema, so this migration is a
defensive no-op against the current state. The test still proves
migration correctness: (a) all migrations up to v15 run cleanly,
(b) two rows with the same (batch_id, patient_control_number) can
be inserted after the migration (proving no UNIQUE was re-introduced
by the table recreation).
"""
# Real migrations dir so the test exercises the actual 0015 file.
monkeypatch.setattr(
db_migrate,
"MIGRATIONS_DIR",
Path(__file__).parent.parent / "src" / "cyclone" / "migrations",
)
engine = _fresh_engine(tmp_path)
db_migrate.run(engine)
assert _user_version(engine) == 15, f"expected head=15, got {_user_version(engine)}"
# Two claims in one batch with the same patient_control_number
# must be insertable. If 0015's table recreation re-introduced a
# UNIQUE(batch_id, patient_control_number), this would raise
# IntegrityError. (The test also implicitly asserts the FK from
# claims to batches still works after the recreation.)
with engine.begin() as conn:
conn.exec_driver_sql(
"INSERT INTO batches (id) VALUES ('B1')"
)
conn.exec_driver_sql(
"INSERT INTO claims (id, batch_id, patient_control_number, charge_amount) "
"VALUES ('CLM-1', 'B1', 'SAME-PCN', 100)"
)
conn.exec_driver_sql(
"INSERT INTO claims (id, batch_id, patient_control_number, charge_amount) "
"VALUES ('CLM-2', 'B1', 'SAME-PCN', 200)"
)
rows = conn.exec_driver_sql(
"SELECT id, charge_amount FROM claims "
"WHERE patient_control_number='SAME-PCN' ORDER BY id"
).all()
assert [r[0] for r in rows] == ["CLM-1", "CLM-2"]
assert [float(r[1]) for r in rows] == [100.0, 200.0]