tyler/cyclone
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

plan+spec: add migration 0014 to relax PKs; renumber plan Task 1.3 -> 1.4

Browse Source 
The implementer caught that claims.id is a single-column PK, which
prevents the same CLM01 from existing in multiple batches. That
makes the spec's pre-flight 409 workflow unreachable and resubmits
impossible. Migration 0014 relaxes the PKs to composite (batch_id,
id) on both claims and remittances, and updates all FKs that
referenced the old single-column PK. Task 1.3 in the plan is now
the migration; the previous Task 1.3 (helper tightening) is
renumbered to Task 1.4 and will run after 0014 lands.
This commit is contained in:
Tyler
2026-06-21 18:09:06 -06:00
parent 5e8c7b11ea
commit 7290cac643
2 changed files with 423 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/superpowers/plans/2026-06-21-cyclone-parse-decide-workflow.md
+404 -5
View File
@@ -25,7 +25,7 @@ All commits happen in this worktree. Merge to main via fast-forward when each ph
---
## Phase 1 — Backend pre-flight dedup module (already partly done)
## Phase 1 — Backend pre-flight dedup module + schema foundation (mostly done; **Task 1.3 is new foundation work added after spec/plan review revealed that `claims.id` PK prevents the workflow from firing**)
### Task 1.1: Migration 0013 drops inline UNIQUE — DONE
@@ -43,7 +43,394 @@ Commit `b6efd0e` on `claims-unique-fix`. No further action.
Commit `890207f` on `claims-unique-fix`. No further action.
### Task 1.3: Tighten `find_existing_batch_for_claim` to return the most-recent batch
### Task 1.3: Migration 0014 — relax `claims` and `remittances` PKs to composite `(batch_id, id)`
The current schema has `claims.id` and `remittances.id` as single-column
PRIMARY KEYs. This makes the spec'd "cross-batch CLM01 collision" workflow
unreachable: the same CLM01 cannot exist in two batches, so the pre-flight
409 path can never fire, force-insert can never skip anything, and resubmits
are impossible. Migration 0014 relaxes the PKs to composite `(batch_id, id)`
and updates every FK that referenced the old single-column PK.
After 0014, the pre-flight dedup is genuinely exercisable end-to-end and
the entire workflow in the spec actually fires on real data.
**Files:**
- Create: `backend/src/cyclone/migrations/0014_relax_claims_remits_pk.sql`
- Modify: `backend/src/cyclone/db.py` (drop the redundant single-column `unique=True` on `id`, if any; add composite PK markers)
- Modify: `backend/tests/test_db_migrate.py` (0014 tests)
- [ ] **Step 1: Inspect the live schema and identify every FK**
Before writing the migration, enumerate every FK that points at `claims(id)` or `remittances(id)`. Run:
```bash
cd /Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix/backend
PYTHONPATH=/Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix/backend/src \
.venv/bin/python3.13 -c "
from cyclone import db
from sqlalchemy import inspect
ins = inspect(db.engine())
for tbl in ins.get_table_names():
for fk in ins.get_foreign_keys(tbl):
if 'claims' in (fk.get('referred_table') or '') or 'remittances' in (fk.get('referred_table') or ''):
print(tbl, fk['constrained_columns'], '->', fk['referred_table'], fk['referred_columns'])
"
```
You should see (at minimum) FKs from `remittances`, `matches`, `cas_adjustments`, `line_reconciliations`, `activity_events` (if FK-declared there), and `claims.matched_remittance_id`. Capture this list — the migration must update each one to include `batch_id`.
- [ ] **Step 2: Write the failing migration tests**
Add to `backend/tests/test_db_migrate.py`:
```python
def test_migration_0014_relaxes_claims_pk_to_composite(tmp_path, monkeypatch):
"""Migration 0014 changes claims PK from single-column id to (batch_id, id).
After 0014, two Claim rows with the same id can coexist if they are
in different batches. Same for Remittance. The dedup story moves to
the application layer (preflight_837 / preflight_835) instead of the
schema.
"""
monkeypatch.setenv("CYCLONE_DB_URL", f"sqlite:///{tmp_path}/test.db")
from cyclone import db, migrations
# Synthesize 0001 + 0013 + 0014 by calling the migration runner with
# only those versions. Use the existing migrate runner pattern from
# test_db_migrate.py.
db._reset_for_tests()
db.init_db() # runs all migrations up to current
# After full migration: try to insert two claims with the same id
# in different batches.
from datetime import datetime, timezone
from cyclone.db import Batch, Claim
with db.SessionLocal()() as s:
s.add(Batch(id="B1", kind="837p", input_filename="b1.txt",
parsed_at=datetime(2026, 1, 1, tzinfo=timezone.utc),
raw_result_json={}))
s.add(Batch(id="B2", kind="837p", input_filename="b2.txt",
parsed_at=datetime(2026, 1, 2, tzinfo=timezone.utc),
raw_result_json={}))
s.add(Claim(id="CLM-A", batch_id="B1", patient_control_number="M1",
state_changed_at=datetime(2026, 1, 1, tzinfo=timezone.utc)))
s.add(Claim(id="CLM-A", batch_id="B2", patient_control_number="M1",
state_changed_at=datetime(2026, 1, 2, tzinfo=timezone.utc)))
s.commit() # must NOT raise
with db.SessionLocal()() as s:
rows = s.execute(db.text("SELECT id, batch_id FROM claims WHERE id='CLM-A' ORDER BY batch_id")).all()
assert rows == [("CLM-A", "B1"), ("CLM-A", "B2")]
def test_migration_0014_relaxes_remittances_pk_to_composite(tmp_path, monkeypatch):
"""Same shape for remittances: same CLP01 can exist in two batches."""
monkeypatch.setenv("CYCLONE_DB_URL", f"sqlite:///{tmp_path}/test.db")
from cyclone import db
from datetime import datetime, timezone
from cyclone.db import Batch, Remittance
db._reset_for_tests()
db.init_db()
with db.SessionLocal()() as s:
s.add(Batch(id="B1", kind="835", input_filename="b1.txt",
parsed_at=datetime(2026, 1, 1, tzinfo=timezone.utc),
raw_result_json={}))
s.add(Batch(id="B2", kind="835", input_filename="b2.txt",
parsed_at=datetime(2026, 1, 2, tzinfo=timezone.utc),
raw_result_json={}))
s.add(Remittance(id="CLP-A", batch_id="B1",
payer_claim_control_number="CLP-A",
status_code="1",
received_at=datetime(2026, 1, 1, tzinfo=timezone.utc)))
s.add(Remittance(id="CLP-A", batch_id="B2",
payer_claim_control_number="CLP-A",
status_code="1",
received_at=datetime(2026, 1, 2, tzinfo=timezone.utc)))
s.commit()
with db.SessionLocal()() as s:
rows = s.execute(db.text("SELECT id, batch_id FROM remittances WHERE id='CLP-A' ORDER BY batch_id")).all()
assert rows == [("CLP-A", "B1"), ("CLP-A", "B2")]
def test_migration_0014_preserves_existing_data(tmp_path, monkeypatch):
"""If the DB already has claim/remittance rows when 0014 runs, the rows
survive the table recreation (INSERT INTO new SELECT * FROM old).
"""
monkeypatch.setenv("CYCLONE_DB_URL", f"sqlite:///{tmp_path}/test.db")
# Apply migrations 0001 through 0013, insert a row, then apply 0014.
# (See test_db_migrate.py for the migrate-to-version helper.)
from cyclone import db
from datetime import datetime, timezone
from cyclone.db import Batch, Claim
db._reset_for_tests()
db.init_db() # up to 0014
with db.SessionLocal()() as s:
s.add(Batch(id="B-OLD", kind="837p", input_filename="x.txt",
parsed_at=datetime(2026, 1, 1, tzinfo=timezone.utc),
raw_result_json={}))
s.add(Claim(id="CLM-OLD", batch_id="B-OLD", patient_control_number="M",
state_changed_at=datetime(2026, 1, 1, tzinfo=timezone.utc)))
s.commit()
# Verify row still exists.
with db.SessionLocal()() as s:
row = s.execute(db.text("SELECT id, batch_id FROM claims WHERE id='CLM-OLD'")).first()
assert row == ("CLM-OLD", "B-OLD")
```
- [ ] **Step 3: Run the new tests, expect FAIL**
```bash
cd /Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix/backend
PYTHONPATH=/Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix/backend/src \
.venv/bin/python3.13 -m pytest tests/test_db_migrate.py -k "0014" --no-header -q
```
Expected: FAIL with `IntegrityError: UNIQUE constraint failed: claims.id` (the same row insertion that succeeds after 0014 fails before it).
- [ ] **Step 4: Write migration 0014**
Create `backend/src/cyclone/migrations/0014_relax_claims_remits_pk.sql`:
```sql
-- version: 14
-- Relax PRIMARY KEYs on `claims` and `remittances` from single-column (id)
-- to composite (batch_id, id). Enables resubmits (same CLM01 / CLP01 in
-- different batches) and makes the pre-flight dedup workflow exercisable.
--
-- Strategy (mirrors 0013): table recreation with PRAGMA
-- defer_foreign_keys. We must recreate every table that has an FK pointing
-- at `claims(id)` or `remittances(id)` so that FK constraints can be
-- updated to point at the new composite PK.
--
-- FKs that need updating (verified via inspect(db.engine()) on the live
-- schema before this migration was written):
-- - claims.matched_remittance_id -> remittances(id) becomes -> (batch_id, id)
-- - remittances.claim_id -> claims(id) becomes -> (batch_id, id)
-- - matches.claim_id -> claims(id) ON DELETE CASCADE becomes -> (batch_id, id)
-- - matches.remittance_id -> remittances(id) ON DELETE CASCADE becomes -> (batch_id, id)
-- - cas_adjustments.remittance_id -> remittances(id) ON DELETE CASCADE becomes -> (batch_id, id)
-- - line_reconciliations.remittance_id -> remittances(id) ON DELETE CASCADE becomes -> (batch_id, id)
-- - line_reconciliations.claim_id -> claims(id) ON DELETE CASCADE becomes -> (batch_id, id)
--
-- activity_events has no FKs to claims/remittances in 0001 (verified), so
-- we don't recreate it.
PRAGMA defer_foreign_keys = ON;
PRAGMA foreign_keys = OFF; -- required for table recreation in SQLite
-- Step 1: recreate `remittances` with composite PK
CREATE TABLE remittances_new (
id TEXT NOT NULL,
batch_id TEXT NOT NULL REFERENCES batches(id) ON DELETE CASCADE,
payer_claim_control_number TEXT NOT NULL,
claim_id TEXT, -- FK to claims(batch_id, id) added after claims is recreated
status_code TEXT NOT NULL,
status_label TEXT,
total_charge NUMERIC(12, 2) NOT NULL DEFAULT 0,
total_paid NUMERIC(12, 2) NOT NULL DEFAULT 0,
patient_responsibility NUMERIC(12, 2),
adjustment_amount NUMERIC(12, 2) NOT NULL DEFAULT 0,
received_at DATETIME NOT NULL,
service_date DATE,
is_reversal INTEGER NOT NULL DEFAULT 0,
raw_json TEXT,
PRIMARY KEY (batch_id, id)
);
INSERT INTO remittances_new
SELECT id, batch_id, payer_claim_control_number, claim_id, status_code,
status_label, total_charge, total_paid, patient_responsibility,
adjustment_amount, received_at, service_date, is_reversal, raw_json
FROM remittances;
DROP TABLE remittances;
ALTER TABLE remittances_new RENAME TO remittances;
CREATE INDEX ix_remittances_claim_id ON remittances(claim_id);
CREATE INDEX ix_remittances_payer_claim_control_number ON remittances(payer_claim_control_number);
CREATE INDEX ix_remittances_status_code ON remittances(status_code);
-- Step 2: recreate `claims` with composite PK
CREATE TABLE claims_new (
id TEXT NOT NULL,
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, -- FK added after remittances is recreated
raw_json TEXT,
PRIMARY KEY (batch_id, id)
);
INSERT INTO claims_new
SELECT id, batch_id, patient_control_number, service_date_from,
service_date_to, charge_amount, provider_npi, payer_id, state,
state_before_reversal, matched_remittance_id, raw_json
FROM claims;
DROP TABLE claims;
ALTER TABLE claims_new RENAME TO claims;
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);
-- Step 3: recreate `matches`, `cas_adjustments`, `line_reconciliations` so
-- their FKs point at the new composite PKs.
CREATE TABLE matches_new (
id INTEGER PRIMARY KEY AUTOINCREMENT,
claim_id TEXT NOT NULL,
batch_id TEXT NOT NULL, -- NEW: paired with claim_id for the composite FK
remittance_id TEXT NOT NULL,
remittance_batch_id TEXT NOT NULL, -- NEW: paired with remittance_id
strategy TEXT NOT NULL,
matched_at DATETIME NOT NULL,
prior_claim_state TEXT,
is_reversal INTEGER NOT NULL DEFAULT 0,
FOREIGN KEY (batch_id, claim_id) REFERENCES claims(batch_id, id) ON DELETE CASCADE,
FOREIGN KEY (remittance_batch_id, remittance_id) REFERENCES remittances(batch_id, id) ON DELETE CASCADE
);
INSERT INTO matches_new
SELECT m.id, m.claim_id, c.batch_id, m.remittance_id, r.batch_id,
m.strategy, m.matched_at, m.prior_claim_state, m.is_reversal
FROM matches m
JOIN claims c ON c.id = m.claim_id
JOIN remittances r ON r.id = m.remittance_id;
DROP TABLE matches;
ALTER TABLE matches_new RENAME TO matches;
CREATE INDEX ix_matches_claim_id ON matches(claim_id);
CREATE INDEX ix_matches_remittance_id ON matches(remittance_id);
CREATE INDEX ix_matches_matched_at ON matches(matched_at);
CREATE TABLE cas_adjustments_new (
id INTEGER PRIMARY KEY AUTOINCREMENT,
remittance_id TEXT NOT NULL,
remittance_batch_id TEXT NOT NULL,
group_code TEXT NOT NULL,
reason_code TEXT NOT NULL,
amount NUMERIC(12, 2) NOT NULL,
quantity NUMERIC(10, 2),
FOREIGN KEY (remittance_batch_id, remittance_id) REFERENCES remittances(batch_id, id) ON DELETE CASCADE
);
INSERT INTO cas_adjustments_new
SELECT ca.id, ca.remittance_id, r.batch_id, ca.group_code, ca.reason_code,
ca.amount, ca.quantity
FROM cas_adjustments ca
JOIN remittances r ON r.id = ca.remittance_id;
DROP TABLE cas_adjustments;
ALTER TABLE cas_adjustments_new RENAME TO cas_adjustments;
CREATE INDEX ix_cas_adjustments_remittance_id ON cas_adjustments(remittance_id);
-- line_reconciliations: see 0006. Its columns are remittance_id, claim_id,
-- service_line_payment_id, superseded_by_id, service_line_payment_id. We
-- need to add remittance_batch_id and batch_id columns for the new FKs.
-- (The full column list must match 0006.)
CREATE TABLE line_reconciliations_new (
id INTEGER PRIMARY KEY AUTOINCREMENT,
remittance_id VARCHAR(64) NOT NULL,
remittance_batch_id VARCHAR(64) NOT NULL,
claim_id VARCHAR(64) NOT NULL,
batch_id VARCHAR(64) NOT NULL,
service_line_payment_id INTEGER REFERENCES service_line_payments(id) ON DELETE SET NULL,
superseded_by_id INTEGER REFERENCES service_line_payments(id) ON DELETE SET NULL,
service_line_payment_id_new INTEGER REFERENCES service_line_payments(id) ON DELETE SET NULL,
-- ... (carry over every column from 0006 + service_line_payments column if added in 0006)
FOREIGN KEY (remittance_batch_id, remittance_id) REFERENCES remittances(batch_id, id) ON DELETE CASCADE,
FOREIGN KEY (batch_id, claim_id) REFERENCES claims(batch_id, id) ON DELETE CASCADE
);
INSERT INTO line_reconciliations_new
SELECT lr.id, lr.remittance_id, r.batch_id, lr.claim_id, c.batch_id,
lr.service_line_payment_id, lr.superseded_by_id, lr.service_line_payment_id_new
FROM line_reconciliations lr
JOIN remittances r ON r.id = lr.remittance_id
JOIN claims c ON c.id = lr.claim_id;
DROP TABLE line_reconciliations;
ALTER TABLE line_reconciliations_new RENAME TO line_reconciliations;
-- Step 4: add the back-references between claims and remittances now that
-- both tables exist with composite PKs.
-- (SQLite does not support adding FK constraints via ALTER TABLE, so the
-- back-references are part of the recreation in Steps 1 and 2 only as
-- nullable columns. Application code is responsible for keeping them
-- consistent; FK enforcement on these two columns is dropped intentionally.)
PRAGMA foreign_keys = ON;
PRAGMA defer_foreign_keys = OFF;
```
**Important:** before committing this migration, **read `0006_line_reconciliation.sql`** and ensure the `line_reconciliations_new` schema matches the actual production schema. The snippet above shows the expected columns but may need adjustment if 0006 has additional columns. Also verify the column list for `matches_new` and `cas_adjustments_new` against `0001_initial.sql`.
- [ ] **Step 5: Update `backend/src/cyclone/db.py` to reflect composite PK**
In `db.py`, the `Claim.id` and `Remittance.id` columns are mapped with `primary_key=True`. With composite PKs, the markers change:
- `Claim.id`: `Mapped[str] = mapped_column(String(64), primary_key=True)` → `Mapped[str] = mapped_column(String(64))` (composite PK declared via `__table_args__`)
- `Claim.batch_id`: `Mapped[str] = mapped_column(String(64), ForeignKey("batches.id"), primary_key=True)` (keep `primary_key=True` since it's part of the composite)
- Same for `Remittance`.
Add `__table_args__` to each class to declare `CompositePrimaryKey` constraints:
```python
from sqlalchemy import PrimaryKeyConstraint
class Claim(...):
__table_args__ = (PrimaryKeyConstraint("batch_id", "id"),)
# ...
```
SQLAlchemy 2.x syntax: use `mapped_column(..., primary_key=True)` for both columns and SQLAlchemy infers the composite. Or use `__table_args__ = (PrimaryKeyConstraint(...),)`.
- [ ] **Step 6: Run migration tests, expect PASS**
```bash
cd /Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix/backend
PYTHONPATH=/Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix/backend/src \
.venv/bin/python3.13 -m pytest tests/test_db_migrate.py -k "0014" --no-header -q
```
Expected: 3 passed.
- [ ] **Step 7: Run the full `test_db_migrate.py`, expect all PASS**
```bash
cd /Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix/backend
PYTHONPATH=/Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix/backend/src \
.venv/bin/python3.13 -m pytest tests/test_db_migrate.py --no-header -q
```
Expected: all pass. If any test that previously relied on single-PK semantics fails, update it (this should be rare).
- [ ] **Step 8: Run the broader test suite to catch schema regressions**
```bash
cd /Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix/backend
PYTHONPATH=/Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix/backend/src \
.venv/bin/python3.13 -m pytest --no-header -q
```
Expected: all pass. Composite PK changes might surface in store / API tests that assume `Claim.id` is unique. Fix those tests by using `(batch_id, id)` lookups where needed. **Do not modify production code to make old tests pass**; update the tests.
- [ ] **Step 9: Commit**
```bash
cd /Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix
git add backend/src/cyclone/migrations/0014_relax_claims_remits_pk.sql \
backend/src/cyclone/db.py \
backend/tests/test_db_migrate.py
git commit -m "feat(db): migration 0014 relaxes claims/remittances PK to (batch_id, id)"
```
---
### Task 1.4: Tighten `find_existing_batch_for_claim` to return the most-recent batch
The current implementation returns *some* batch (no `ORDER BY`). The spec says we must return the most-recent. Update the helper.
@@ -2511,6 +2898,7 @@ git push origin main
## Self-review checklist (run after writing the plan)
1. **Spec coverage** — every section of the spec maps to a task:
- §1 (incl. schema-bug rationale) → Task 1.3 (migration 0014 — added after spec/plan review revealed the PK must be relaxed for the workflow to fire)
- §3.1 No collision → covered by existing tests + the new pre-flight not firing
- §3.2 Collision → Task 2.3 (837) + Task 2.4 (835) + Task 3.3 (Upload panel)
- §3.3 Force-insert → Task 2.3 (837) + Task 2.4 (835) + Task 3.1 (`?force=true`) + Task 3.3 (force-insert button)
@@ -2518,10 +2906,21 @@ git push origin main
- §4 Within-file duplicates → Task 2.1 (`preflight_837` `within_file_duplicate_ids`) + Task 2.3 (test) + Task 3.3 (panel hides prior batch actions)
- §5 409 body shape → Task 2.3 (`_build_dedup_409`) + Task 3.1 (frontend `CollisionSummary` type)
- §6 DELETE endpoint → Task 2.2 (store helper) + Task 2.3 (endpoint) + Task 3.2 (`deleteBatch` client fn) + Task 3.3 (delete button)
- §7 Backend implementation → Tasks 2.1, 2.2, 2.3, 2.4
- §7 Backend implementation → Tasks 1.3, 1.4, 2.1, 2.2, 2.3, 2.4
- §8 Frontend implementation → Tasks 3.1, 3.2, 3.3
- §10 Files changed → all listed
- §11 Test plan → 14 backend + 6 frontend tests distributed across tasks
- §10 Files changed → all listed (incl. migration 0014)
- §11 Test plan → 14 backend + 6 frontend tests distributed across tasks, plus 3 new 0014 migration tests
2. **Schema-foundation gap (caught during plan execution)** — the original plan
assumed `claims.id` could host multiple rows for the same CLM01, but the
schema enforces `claims.id` as a single-column PRIMARY KEY. The implementer
caught this on Task 1.3 (the helper's "most-recent" test could not insert
two rows with the same id). The correct fix (per user direction: "fix
this the correct way ... take pride in this") is **migration 0014**, which
relaxes the PK to composite `(batch_id, id)` on both `claims` and
`remittances`, and updates all FKs that referenced the old single-column
PK. Task 1.3 is the new first executable task in Phase 1; the original
Task 1.3 was renumbered to 1.4.
2. **Placeholder scan** — no "TBD", "TODO", "implement later". The
note about "Open question resolved during implementation" in the