From b6efd0eaee59137330bbe0b686e6da4a2d4b6763 Mon Sep 17 00:00:00 2001 From: Tyler Date: Sun, 21 Jun 2026 17:39:28 -0600 Subject: [PATCH] feat(db): drop inline UNIQUE(batch_id, patient_control_number) via migration 0013 --- backend/.gitignore | 1 + .../0013_drop_claims_unique_constraint.sql | 59 ++++++++++ backend/tests/test_db_migrate.py | 105 +++++++++++++++++- 3 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql diff --git a/backend/.gitignore b/backend/.gitignore index cd7bf33..e9e93af 100644 --- a/backend/.gitignore +++ b/backend/.gitignore @@ -7,3 +7,4 @@ __pycache__/ venv/ build/ dist/ +backend/.venv diff --git a/backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql b/backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql new file mode 100644 index 0000000..4297cac --- /dev/null +++ b/backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql @@ -0,0 +1,59 @@ +-- version: 13 +-- 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. +-- +-- 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. + +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; diff --git a/backend/tests/test_db_migrate.py b/backend/tests/test_db_migrate.py index d99b995..983896f 100644 --- a/backend/tests/test_db_migrate.py +++ b/backend/tests/test_db_migrate.py @@ -112,4 +112,107 @@ def test_run_ignores_non_sql_files( "SELECT name FROM sqlite_master WHERE type='table' AND name='should_not_exist'" ).all() assert len(rows) == 0 - assert _user_version(engine) == 1 \ No newline at end of file + assert _user_version(engine) == 1 + + +def test_drop_claims_unique_constraint_migration(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Migration 0013 must drop the inline UNIQUE(batch_id, patient_control_number) + on claims by recreating the table. Idempotent and preserves data. + + We copy the real 0013 file (so this test exercises the real migration, not + a synthetic one) and stub 0001-0012 with a synthetic 0001 that has the + historical inline UNIQUE. If 0013 doesn't exist as a real file, the runner + will leave user_version at 1 and the test will fail. + """ + monkeypatch.setattr(db_migrate, "MIGRATIONS_DIR", tmp_path) + + # Synthetic 0001: claims with INLINE UNIQUE(batch_id, patient_control_number). + # The schema mirrors the real 0001 column shape so 0013's + # ``INSERT INTO claims_new SELECT * FROM claims`` works against this fixture. + (tmp_path / "0001_initial.sql").write_text( + "-- version: 1\n" + "CREATE TABLE batches (id TEXT PRIMARY KEY, kind TEXT NOT NULL, " + "input_filename TEXT NOT NULL, parsed_at DATETIME NOT NULL);\n" + "CREATE TABLE claims (" + "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," + "UNIQUE (batch_id, patient_control_number));\n" + ) + + # Copy the REAL 0013 from the source tree. If it's missing, the test fails + # because user_version will stay at 1 and we'll never get to assert it == 13. + real_migrations = Path(db_migrate.__file__).parent / "migrations" + real_0013 = real_migrations / "0013_drop_claims_unique_constraint.sql" + assert real_0013.exists(), ( + f"Real migration 0013 missing at {real_0013} — this test is meant to " + f"exercise the real migration, not a synthetic one." + ) + (tmp_path / "0013_drop_claims_unique_constraint.sql").write_text(real_0013.read_text()) + + engine = _fresh_engine(tmp_path) + db_migrate.run(engine) + assert _user_version(engine) == 13, ( + "0013 did not apply; user_version did not reach 13. Either the migration " + "file is missing, or it has a syntax error." + ) + + # Before-0013 assertion: with the inline UNIQUE, two claims sharing + # (batch_id, patient_control_number) on the same batch must fail. The + # recreation must remove that constraint, so this insert must now succeed. + with engine.begin() as c: + c.exec_driver_sql( + "INSERT INTO batches(id, kind, input_filename, parsed_at) " + "VALUES ('b1', '837p', 'x.txt', '2026-01-01')" + ) + c.exec_driver_sql( + "INSERT INTO claims(id, batch_id, patient_control_number) VALUES ('c1', 'b1', 'M')" + ) + c.exec_driver_sql( + "INSERT INTO claims(id, batch_id, patient_control_number) VALUES ('c2', 'b1', 'M')" + ) + ids = [r[0] for r in c.exec_driver_sql("SELECT id FROM claims ORDER BY id").all()] + assert ids == ["c1", "c2"], ( + "Both claims with same (batch_id, patient_control_number) must persist " + "after 0013 — the UNIQUE(batch_id, patient_control_number) is gone." + ) + + +def test_migration_0013_is_idempotent(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Re-running db_migrate.run on a v13 DB is a no-op.""" + monkeypatch.setattr(db_migrate, "MIGRATIONS_DIR", tmp_path) + + # Use synthetic 0001 + copy the real 0013 so we exercise the real file. + (tmp_path / "0001_initial.sql").write_text( + "-- version: 1\n" + "CREATE TABLE batches (id TEXT PRIMARY KEY, kind TEXT NOT NULL, " + "input_filename TEXT NOT NULL, parsed_at DATETIME NOT NULL);\n" + "CREATE TABLE claims (" + "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," + "UNIQUE (batch_id, patient_control_number));\n" + ) + real_migrations = Path(db_migrate.__file__).parent / "migrations" + real_0013 = real_migrations / "0013_drop_claims_unique_constraint.sql" + assert real_0013.exists(), "Real 0013 missing — test cannot exercise it." + (tmp_path / "0013_drop_claims_unique_constraint.sql").write_text(real_0013.read_text()) + + engine = _fresh_engine(tmp_path) + db_migrate.run(engine) # applies both + version_after_first = _user_version(engine) + assert version_after_first == 13 + db_migrate.run(engine) # second call: no-op + assert _user_version(engine) == version_after_first \ No newline at end of file