Files
cyclone/docs/superpowers/specs/2026-06-21-cyclone-store-split-design.md
T
Tyler 5eb490851d docs(spec): design for CycloneStore split (Step 4)
Step 4 of the file-split refactor series. Targets backend/src/cyclone/store.py
(2,412 lines) which sits on the hot path of parse-999, parse-277ca,
reconciliation, and inbox match/unmatch.

Decisions locked:
- Public surface: cyclone/store/ subpackage + thin facade (__init__.py
  re-exports every currently-importable name).
- Class shape: module functions for bodies, CycloneStore keeps its current
  method signatures as 1-line delegations.
- Helper distribution: dedicated utility modules (records, orm_builders,
  ui, exceptions).

13 target modules total; largest is write.py at ~210 lines. Zero public API
changes, zero test changes. Single atomic commit migration; rollback is a
single git revert.
2026-06-21 10:32:52 -06:00

15 KiB
Raw Blame History

CycloneStore split (Step 4) — Design Spec

Date: 2026-06-21 Status: Draft, pending user review Branch: main (split will land as a single atomic commit) Scope: Behaviour-preserving structural split of backend/src/cyclone/store.py (2,412 lines) into a cyclone/store/ subpackage. Zero public API changes, zero test changes.

Context

store.py is the SQLAlchemy-backed facade over the parsed-X12 store. It sits on the hot path of parse-999, parse-277ca, reconciliation, and inbox match/unmatch. At 2,412 lines it has outgrown a single file: 41 methods on CycloneStore, ~17 module-level helpers, Pydantic models, ORM row builders, UI serializers, and exception types all live in one module.

This is "Step 4" of the ongoing refactor series. Steps 13 (the api.py splits — commits 931782b, fc73075, 3b5e2af, eb674f8, ff43f90, 6ce6385, e63be87, a63ba5e) established the pattern: turn a monolithic file into a subpackage with a thin facade, preserving every public import.

Decisions (locked during brainstorming)

  1. Public surface: Subpackage + thin facade. cyclone/store.py is deleted; replaced by cyclone/store/__init__.py re-exporting every currently-importable name.
  2. Class structure: Module functions for the bodies, CycloneStore keeps its current method signatures as 1-line delegations. (Mixin classes were considered and rejected for added MRO surface area; composition was rejected for breaking isinstance and adding 41 delegating boilerplate methods in a different shape.)
  3. Helper distribution: Dedicated utility modules for non-method helpers — records.py, orm_builders.py, ui.py, exceptions.py. Co-location was rejected for circular-import risk.

Target module layout

backend/src/cyclone/
├── store.py            ← DELETED
└── store/
    ├── __init__.py     ← thin facade: re-exports + CycloneStore + `store` singleton + utcnow
    ├── exceptions.py   ← AlreadyMatchedError, NotMatchedError, InvalidStateError
    ├── records.py      ← BatchKind, BatchRecord, BatchRecord837, BatchRecord835 (Pydantic)
    ├── orm_builders.py ← _service_dates_from_claim, _claim_837_row, _remittance_835_row,
    │                     _cas_adjustment_row, _persist_835_remit, _claim_status_from_validation
    ├── ui.py           ← all to_ui_* / _iso_z / _address_to_ui / _validation_issues_to_ui /
    │                     _svc_to_wire_dict / _date_in_bounds / _provider_orm_to_dict /
    │                     _payer_orm_to_dict
    ├── write.py        ← add, _publish_events_sync, _sync_publish, _run_reconcile
    │                     (biggest single module — ~210 lines)
    ├── batches.py      ← get_batch, get, list, all, load_two_for_diff, _BatchesShim, _row_to_record
    ├── claim_detail.py ← get_remittance, get_claim_detail, iter_claims, iter_remittances,
    │                     distinct_providers, recent_activity
    ├── acks.py         ← add_ack, list_acks, get_ack (999),
    │                     add_ta1_ack, list_ta1_acks, get_ta1_ack,
    │                     add_277ca_ack, list_277ca_acks, get_277ca_ack
    ├── backups.py      ← add_backup_pending
    ├── inbox.py        ← list_unmatched, manual_match, manual_unmatch
    └── providers.py    ← list_providers, get_provider, upsert_provider, list_payers,
                          get_payer_config, get_clearhouse, ensure_clearhouse_seeded

13 modules total. Largest is write.py at ~210 lines; all others are ≤150 lines. Facade __init__.py ≈ 80 lines (re-exports + class + singleton).

CycloneStore class shape

Three categories of methods

Category A — Pure read delegates. Open session, run query, return. No shared state, no private-method calls, no events.

Batches reads: get_batch, get, list, all, load_two_for_diff, get_remittance. Claim detail reads: get_claim_detail, iter_claims, iter_remittances, distinct_providers, recent_activity. ACK reads: list_acks, get_ack, list_ta1_acks, get_ta1_ack, list_277ca_acks, get_277ca_ack. Provider/payer reads: list_providers, get_provider, list_payers, get_payer_config, get_clearhouse, ensure_clearhouse_seeded.

Category B — Write-with-side-effects. Module functions take an explicit event_bus kwarg where applicable; the CycloneStore methods delegate. The 3 private methods (_publish_events_sync, _run_reconcile, _sync_publish) stay on the class as 1-line delegations so any third-party calling them by private API doesn't break.

Methods: add, add_ack, add_ta1_ack, add_277ca_ack, add_backup_pending. Only add has a non-trivial body (insert + idempotency + reconcile + publish); the ACK add methods and add_backup_pending are simple single-row inserts that move to acks.py and backups.py respectively.

Category C — Manual match/unmatch (3 methods). Use reconcile.apply_payment / apply_reversal; translate skipped=True into InvalidStateError. Module functions in inbox.py; class methods delegate.

Methods: list_unmatched, manual_match, manual_unmatch.

Iterator methods

iter_claims, iter_remittances are generators. Module functions in claim_detail.py use with db.SessionLocal()() as s: and yield inside; session lives for generator lifetime. Class methods delegate.

Session management

Each module function inlines with db.SessionLocal()() as s: at the top — matches the existing pattern verbatim. No extracted session_scope() helper (YAGNI; ~30 call sites would change with no behaviour gain).

Lock + shim stay on the class

CycloneStore.__init__ keeps self._lock = threading.RLock() and self._batches = _BatchesShim() because 17 test files use with store._lock: store._batches.clear() as the cleanup idiom. The _BatchesShim class itself moves to batches.py alongside _row_to_record.

Naming collision table

Class method Module function Reason
store.list batches.list_batches shadows builtin
store.all batches.all_batches shadows builtin
store.add write.add_record more descriptive
store.get batches.get_record matches existing test usage
store.utcnow top-level utcnow() in __init__.py was always a free function

Singleton

store = CycloneStore() declared exactly once, in __init__.py. All sibling modules reference cyclone.db directly for session access, never the singleton.

Public API preservation

Source callers — zero changes

Caller Import After split
cyclone/api.py from cyclone.store import (CycloneStore, store, BatchRecord, AlreadyMatchedError, ...) unchanged
cyclone/api.py (deferred) from cyclone.store import NotMatchedError unchanged
cyclone/batch_diff.py from cyclone.store import BatchRecord, BatchRecord835, BatchRecord837, _claim_status_from_validation facade re-exports _claim_status_from_validation
cyclone/backup_service.py (deferred) from cyclone.store import store as cycl_store unchanged
cyclone/api_routers/acks.py from cyclone.store import store unchanged
cyclone/api_routers/ta1_acks.py from cyclone.store import store unchanged
cyclone/scheduler.py from cyclone.store import store as cycl_store (also BatchRecord deferred) unchanged
cyclone/parsers/validator.py (3 deferred) from cyclone import store as store_mod unchanged
cyclone/api_helpers.py docstring references cyclone.store.utcnow unchanged (facade re-exports utcnow)

Open decision: _claim_status_from_validation is currently a module-level helper imported by batch_diff.py (line 32). Recommended: re-export it from __init__.py to preserve the import. This is a small facade pollution but matches the precedent of the api.py split (which re-exports routers).

Tests — zero changes

17 test files use with store._lock: store._batches.clear() — all continue to work because CycloneStore.__init__ still sets those attributes and _BatchesShim continues to function identically. The 3 dedicated store tests (test_store.py, test_store_claim_detail.py, test_store_reconcile.py) import from cyclone.store (still works) or cyclone (still works).

Migration plan

Single atomic commit. cyclone/store.py cannot coexist with cyclone/store/ (Python prefers packages), so deletion and creation must happen together:

rm backend/src/cyclone/store.py
mkdir -p backend/src/cyclone/store
# write all 13 new files
cd backend && python -m pytest tests/ --tb=line -q  # must be green
git add -A
git commit -m "refactor(store): split store.py into cyclone/store/ subpackage"

Follow-up commit (only if needed): Docs update — add "CycloneStore subpackage" section to README following the precedent of commits 81aebf5, 2718114, ea64e6e, 804e557.

Rollback: git revert <sha> — single-commit revert, no half-state.

Test strategy

No new tests, no test modifications

The split is structural only. Every test that passes today must pass after.

Verification tiers

Tier 1 — Full backend suite, baseline + after.

cd backend && python -m pytest tests/ --tb=line -q 2>&1 | tee /tmp/cyclone-baseline.txt | tail -5

Expected baseline: ~712 passed, ~31 failed (pre-existing), ~16 skipped. The 31 failures are documented cross-test pollution (11 test_serialize_837, 3 test_secrets, 2 test_sftp_paramiko, etc., verified in commit 931782b). Post-split count must match exactly.

Tier 2 — Hot-path focused (14 files). Exercises every method being split.

cd backend && python -m pytest tests/test_parse_999.py tests/test_parse_277ca.py \
    tests/test_api_999.py tests/test_api_277ca.py \
    tests/test_reconcile.py tests/test_reconcile_line_level.py \
    tests/test_store_reconcile.py tests/test_api_line_reconciliation.py \
    tests/test_inbox_state.py tests/test_inbox_lanes.py \
    tests/test_inbox_endpoints.py tests/test_inbox_endpoints_sp7.py \
    tests/test_payer_rejected_acknowledge.py tests/test_store.py \
    --tb=short -q

Tier 3 — Store-specific (3 files). Covers the class surface directly.

cd backend && python -m pytest tests/test_store.py tests/test_store_claim_detail.py \
    tests/test_store_reconcile.py --tb=short -q

Tier 4 — Live smoke on 8 endpoints (matches commit 931782b precedent):

  • POST /api/parse-999 (valid → ack_code=A; rejected → ack_code=R)
  • POST /api/parse-277ca (valid → 200 with 3 claim statuses)
  • GET /api/inbox?lane=unmatched (list_unmatched shape)
  • POST /api/inbox/match (manual_match happy path + AlreadyMatchedError)
  • POST /api/inbox/unmatch (manual_unmatch happy path + NotMatchedError)
  • GET /api/batches + GET /api/batches/{id} (get_batch shape)
  • GET /api/claims/{id} (get_claim_detail shape)
  • GET /api/999-acks/{id} and GET /api/277ca-acks/{id} (ack list/get shape)

Smoke output is documented in the commit message.

Risk register

# Risk Likelihood Impact Mitigation
R1 Circular import between __init__.py and a sibling Low High Siblings never import from __init__.py. Module docstrings state this rule.
R2 Module function loses self. context Low Medium CycloneStore.__init__ keeps _lock and _batches. Module functions don't touch them.
R3 _BatchesShim.clear() semantics change Low High _BatchesShim moves verbatim; line-by-line verification during impl.
R4 add_record() body diverges from current add() Medium High Mechanical copy of lines 898-1107, strip self. prefix, change 3 private-method calls to module-function calls. No logic refactoring.
R5 Singleton duplicated Low High Declared exactly once, in __init__.py.
R6 TYPE_CHECKING imports break runtime Low Low EventBus already used in current add() signature; same pattern.
R7 Test imports of private helpers break Low Medium grep -rn "from cyclone.store import _" backend/tests/ before impl; re-export anything found.
R8 Package discovery misses new subpackage Low Medium Verify with pip install -e . before commit; if needed, add explicit packages = [...].
R9 Generator methods leak sessions Low Medium Preserve with inside generator body.
R10 Docstring drift Low Low Preserve existing docstrings verbatim on re-exports.

Implementation discipline

  1. Don't refactor logic. The split is structural. Code smells stay for separate tasks.
  2. Don't add type hints. Match existing annotations exactly.
  3. Don't add docstrings. Preserve existing; only add a short module-level docstring per new file.
  4. Preserve order of operations. Identical to current add() body.
  5. Don't optimize imports in passing. Move them as-is.

Pre-flight checklist

# 1. Capture baseline
cd backend && python -m pytest tests/ --tb=line -q 2>&1 | tee /tmp/cyclone-baseline.txt | tail -5

# 2. Find every private helper imported from cyclone.store in tests
grep -rn "from cyclone.store import _" backend/tests/

# 3. Find every private helper imported from cyclone.store in source
grep -rn "from cyclone.store import _" backend/src/

# 4. Verify package discovery
cd backend && pip install -e . --quiet
python -c "import cyclone.store; print(cyclone.store.__file__)"

# 5. Verify no other cyclone/store* conflicts
find backend/src/cyclone -maxdepth 1 -name "store*"

Post-flight checklist

# 1. Compare to baseline
cd backend && python -m pytest tests/ --tb=line -q 2>&1 | tail -5
# Must match baseline exactly

# 2. Hot-path focused (14 files) — must be 100% green

# 3. Import sanity check
python -c "
from cyclone.store import (
    CycloneStore, store, BatchRecord, BatchRecord837, BatchRecord835, BatchKind,
    AlreadyMatchedError, NotMatchedError, InvalidStateError, utcnow,
    _claim_status_from_validation,
)
print('all imports OK')
"

# 4. Live smoke on 8 endpoints

Out of scope (explicit)

  • Renaming any public symbol
  • Changing any method signature
  • Refactoring manual_match / manual_unmatch business logic
  • Refactoring add() body or its idempotency strategy
  • Adding new tests
  • Modifying DB models in cyclone.db
  • Updating SQLCipher key rotation flow
  • Touching any of the 17 test files that use _lock / _batches.clear()
  • Touching any importer in cyclone.api, cyclone.api_routers.*, cyclone.batch_diff, cyclone.backup_service, cyclone.scheduler, cyclone.parsers.validator, cyclone.api_helpers

If any of these surface during implementation, they get deferred to follow-up tasks — never silently included.

Open items

  1. _claim_status_from_validation re-export — recommended re-export from __init__.py. User to confirm during spec review.