Tyler
b9260d9969
13 tasks implementing the spec: pre-flight baseline, atomic-move of store.py into store/__init__.py, 10 module extractions (exceptions, records, orm_builders, ui, write, batches, claim_detail, acks, backups, inbox, providers), and final verification + single atomic commit. Working tree stays dirty through Tasks 1-11; final commit happens at end of Task 12 per the spec's single-atomic-commit requirement. Each task ends with a focused test-suite verification (not full baseline — that's Task 12 Step 1's job) so regressions are caught early.
1094 lines
36 KiB
Markdown
1094 lines
36 KiB
Markdown
# CycloneStore split (Step 4) 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:** Behaviour-preserving structural split of `backend/src/cyclone/store.py` (2,412 lines) into a `cyclone/store/` subpackage with 13 modules and a thin facade. Zero public API changes, zero test changes, single atomic commit at the end.
|
|
|
|
**Architecture:** Module functions for each domain (`batches.add_record`, `inbox.manual_match`, etc.); `CycloneStore` class in `__init__.py` keeps its current method signatures as 1-line delegations. The facade re-exports every currently-importable name so no caller changes.
|
|
|
|
**Tech Stack:** Python 3.13, SQLAlchemy 2.x, Pydantic v2, pytest. Existing test infrastructure (no new tests).
|
|
|
|
**Spec:** [`docs/superpowers/specs/2026-06-21-cyclone-store-split-design.md`](../specs/2026-06-21-cyclone-store-split-design.md)
|
|
|
|
---
|
|
|
|
## File structure
|
|
|
|
```
|
|
backend/src/cyclone/
|
|
├── store.py ← DELETED in Task 12
|
|
└── store/
|
|
├── __init__.py ← facade: re-exports + CycloneStore + store singleton + utcnow
|
|
├── exceptions.py ← Task 3
|
|
├── records.py ← Task 4
|
|
├── orm_builders.py ← Task 5
|
|
├── ui.py ← Task 6
|
|
├── write.py ← Task 7
|
|
├── batches.py ← Task 8
|
|
├── claim_detail.py ← Task 9
|
|
├── acks.py ← Task 10
|
|
├── backups.py ← Task 11
|
|
├── inbox.py ← Task 11
|
|
└── providers.py ← Task 11
|
|
```
|
|
|
|
Each module has one clear responsibility. Files that change together live together. The CycloneStore class is the only non-trivial logic that crosses module boundaries, so it lives in `__init__.py` alongside the facade.
|
|
|
|
---
|
|
|
|
## Task 0: Pre-flight — capture baseline
|
|
|
|
**Goal:** Snapshot the test suite baseline so we can prove no regressions. Audit private-helper imports so the facade re-exports cover all callers.
|
|
|
|
**Files:**
|
|
- Read: `backend/src/cyclone/store.py` (the file we're splitting)
|
|
- Write: `/tmp/cyclone-baseline.txt` (test output snapshot)
|
|
- Write: `/tmp/cyclone-private-imports.txt` (private helper import audit)
|
|
|
|
- [ ] **Step 1: Capture pytest baseline**
|
|
|
|
```bash
|
|
cd backend && python -m pytest tests/ --tb=line -q 2>&1 | tee /tmp/cyclone-baseline.txt | tail -5
|
|
```
|
|
|
|
Expected: a line like `XXX passed, YY failed, ZZ skipped`. Record the counts. Per the api.py split precedent (commit `931782b`), expect ~712 passed, ~31 failed (pre-existing cross-test pollution), ~16 skipped.
|
|
|
|
- [ ] **Step 2: Find every private helper imported from cyclone.store in tests**
|
|
|
|
```bash
|
|
grep -rn "from cyclone.store import _" backend/tests/ | tee /tmp/cyclone-test-private-imports.txt
|
|
```
|
|
|
|
Expected: lists any test that imports underscore-prefixed names from `cyclone.store`. These must be re-exported from the facade. (Spot-check confirms `_claim_status_from_validation` is the only one — `batch_diff.py` is a source file, see Step 3.)
|
|
|
|
- [ ] **Step 3: Find every private helper imported from cyclone.store in source**
|
|
|
|
```bash
|
|
grep -rn "from cyclone.store import _" backend/src/ | tee /tmp/cyclone-src-private-imports.txt
|
|
```
|
|
|
|
Expected output (verified):
|
|
|
|
```
|
|
backend/src/cyclone/batch_diff.py:32:from cyclone.store import BatchRecord, BatchRecord835, BatchRecord837, _claim_status_from_validation
|
|
```
|
|
|
|
`_claim_status_from_validation` must be re-exported from the facade.
|
|
|
|
- [ ] **Step 4: Verify package discovery works on the current tree**
|
|
|
|
```bash
|
|
cd backend && pip install -e . --quiet 2>&1 | tail -3
|
|
python -c "import cyclone.store; print('OK:', cyclone.store.__file__)"
|
|
```
|
|
|
|
Expected: prints `OK: /Users/openclaw/dev/cyclone/backend/src/cyclone/store.py`.
|
|
|
|
- [ ] **Step 5: Verify no existing cyclone/store* subpackage**
|
|
|
|
```bash
|
|
find backend/src/cyclone -maxdepth 1 -name "store*"
|
|
```
|
|
|
|
Expected: only `backend/src/cyclone/store.py`.
|
|
|
|
No commit. Pre-flight only.
|
|
|
|
---
|
|
|
|
## Task 1: Create the package skeleton (atomic move, no commit yet)
|
|
|
|
The trick: copy `store.py` content into `store/__init__.py`, delete `store.py`, verify everything still works via the package. The CycloneStore class and `store` singleton live in `__init__.py` for now; they'll be delegated to module functions in Tasks 2-11. The working tree stays dirty through Tasks 2-11 — the single atomic commit happens at the end of Task 12.
|
|
|
|
**Files:**
|
|
- Create: `backend/src/cyclone/store/__init__.py` (verbatim copy of store.py)
|
|
- Delete: `backend/src/cyclone/store.py`
|
|
|
|
- [ ] **Step 1: Read the current store.py**
|
|
|
|
```bash
|
|
wc -l backend/src/cyclone/store.py
|
|
```
|
|
|
|
Expected: `2412`.
|
|
|
|
- [ ] **Step 2: Create the package directory**
|
|
|
|
```bash
|
|
mkdir -p backend/src/cyclone/store
|
|
```
|
|
|
|
- [ ] **Step 3: Copy store.py content to store/__init__.py**
|
|
|
|
```bash
|
|
cp backend/src/cyclone/store.py backend/src/cyclone/store/__init__.py
|
|
```
|
|
|
|
- [ ] **Step 4: Update the module docstring**
|
|
|
|
The current docstring (lines 1-28) describes `cyclone.store` as "the module". Edit `__init__.py` to say "the package" and add a note that this is the facade.
|
|
|
|
```python
|
|
"""SQLAlchemy-backed batch store for parsed X12 files.
|
|
|
|
The package exposes a single ``CycloneStore`` class and a module-level
|
|
singleton (``store``). All persistence flows through SQLAlchemy
|
|
sessions via ``db.SessionLocal()()`` (the double-paren "function-style
|
|
accessor" established in T2/T4).
|
|
|
|
This ``__init__.py`` is the facade — it re-exports every public name
|
|
from the sibling modules (``records``, ``exceptions``, ``orm_builders``,
|
|
``ui``, ``write``, ``batches``, ``claim_detail``, ``acks``, ``backups``,
|
|
``inbox``, ``providers``). Implementation lives in those siblings; the
|
|
``CycloneStore`` class below keeps its current method signatures as
|
|
thin delegations for back-compat.
|
|
|
|
Public API (preserved from the in-memory version):
|
|
- add(record)
|
|
- get(batch_id) / get_batch(batch_id)
|
|
- list(limit) / all()
|
|
- iter_claims(...) / iter_remittances(...)
|
|
- distinct_providers() / recent_activity(limit)
|
|
|
|
New API (T12):
|
|
- list_unmatched(kind="both")
|
|
- manual_match(claim_id, remit_id)
|
|
- manual_unmatch(claim_id)
|
|
- AlreadyMatchedError, NotMatchedError, InvalidStateError exception classes
|
|
|
|
Backward-compat shims for tests that relied on the in-memory internals:
|
|
- ``_lock`` — a no-op ``threading.RLock``. SQLAlchemy handles
|
|
concurrency via the engine's connection pool, but some existing
|
|
tests use it as a context manager around cleanup.
|
|
- ``_batches.clear()`` — wipes all rows from the DB tables so tests
|
|
that depended on a fresh in-memory list per-test get a fresh DB
|
|
state per-test.
|
|
"""
|
|
```
|
|
|
|
- [ ] **Step 5: Delete the old store.py**
|
|
|
|
```bash
|
|
rm backend/src/cyclone/store.py
|
|
```
|
|
|
|
- [ ] **Step 6: Verify imports still work**
|
|
|
|
```bash
|
|
python -c "
|
|
from cyclone.store import (
|
|
CycloneStore, store, BatchRecord, BatchRecord837, BatchRecord835, BatchKind,
|
|
AlreadyMatchedError, NotMatchedError, InvalidStateError, utcnow,
|
|
)
|
|
print('all imports OK')
|
|
"
|
|
```
|
|
|
|
Expected: `all imports OK`.
|
|
|
|
- [ ] **Step 7: Verify test suite matches baseline**
|
|
|
|
```bash
|
|
cd backend && python -m pytest tests/ --tb=line -q 2>&1 | tail -5
|
|
```
|
|
|
|
Expected: identical counts to `/tmp/cyclone-baseline.txt`. Any deviation is a regression — investigate before continuing.
|
|
|
|
**No commit yet.** The final atomic commit happens in Task 12. The working tree stays dirty through Tasks 2-11. This satisfies the spec's "single atomic commit" requirement.
|
|
|
|
---
|
|
|
|
## Task 2: Extract the exceptions module
|
|
|
|
**Files:**
|
|
- Create: `backend/src/cyclone/store/exceptions.py`
|
|
- Modify: `backend/src/cyclone/store/__init__.py:58-91` (remove the 3 exception classes, add re-export)
|
|
|
|
- [ ] **Step 1: Create `exceptions.py` with the 3 exception classes**
|
|
|
|
Copy lines 58-91 of the current `__init__.py` (which was the old `store.py`) into `exceptions.py`. The exact content:
|
|
|
|
```python
|
|
"""Exception types raised by CycloneStore and its domain modules.
|
|
|
|
These are part of the public API — callers (API endpoints) catch them
|
|
and translate to HTTP 409 Conflict responses.
|
|
"""
|
|
|
|
|
|
class AlreadyMatchedError(Exception):
|
|
"""Raised by ``CycloneStore.manual_match`` when the claim is already paired.
|
|
|
|
The claim's ``matched_remittance_id`` is set, so any new pairing would
|
|
clobber an existing match. Callers (the T15 API endpoint) should surface
|
|
this as a 409 Conflict.
|
|
"""
|
|
|
|
|
|
class NotMatchedError(Exception):
|
|
"""Raised by ``CycloneStore.manual_unmatch`` when the claim has no match.
|
|
|
|
Mirrors ``AlreadyMatchedError`` for the unpair operation. Same HTTP
|
|
treatment: 409 Conflict at the API layer.
|
|
"""
|
|
|
|
|
|
class InvalidStateError(Exception):
|
|
"""Raised when an apply_* pure fn returns a skipped ApplyIntent.
|
|
|
|
``reconcile.apply_payment`` / ``apply_reversal`` may return
|
|
``skipped=True`` (e.g. claim already in a terminal state, or reversal
|
|
on a non-paid claim). The store surfaces that as ``InvalidStateError``
|
|
rather than silently pairing. The T15 API endpoint maps this to a
|
|
409 Conflict and echoes ``current_state`` and ``activity_kind`` so
|
|
the UI can render a precise message.
|
|
"""
|
|
|
|
def __init__(self, current_state: str, activity_kind: str = "invalid_state"):
|
|
self.current_state = current_state
|
|
self.activity_kind = activity_kind
|
|
super().__init__(
|
|
f"invalid state {current_state} for apply (kind={activity_kind})"
|
|
)
|
|
```
|
|
|
|
- [ ] **Step 2: Update `__init__.py` to re-export**
|
|
|
|
Remove lines 58-91 from `__init__.py` (the three class definitions). Add at the top of `__init__.py` (after the imports):
|
|
|
|
```python
|
|
from .exceptions import AlreadyMatchedError, NotMatchedError, InvalidStateError
|
|
```
|
|
|
|
- [ ] **Step 3: Verify tests still pass**
|
|
|
|
```bash
|
|
cd backend && python -m pytest tests/ --tb=line -q 2>&1 | tail -5
|
|
```
|
|
|
|
Expected: identical counts to baseline.
|
|
|
|
---
|
|
|
|
## Task 3: Extract the records module
|
|
|
|
**Files:**
|
|
- Create: `backend/src/cyclone/store/records.py`
|
|
- Modify: `backend/src/cyclone/store/__init__.py:94-159` (remove BatchKind, BatchRecord, BatchRecord837, BatchRecord835)
|
|
|
|
- [ ] **Step 1: Create `records.py`**
|
|
|
|
Copy lines 94-159 of the current `__init__.py` into `records.py`. This includes `BatchKind` (line 94), `BatchRecord` (lines 102-145), `BatchRecord837` (148-152), `BatchRecord835` (155-159).
|
|
|
|
```python
|
|
"""Pydantic models for parsed-batch records.
|
|
|
|
``BatchRecord`` is the union type; ``BatchRecord837`` and ``BatchRecord835``
|
|
narrow ``result`` to the parser-specific output types. Construction of
|
|
``BatchRecord(kind="837p", ...)`` dispatches to ``BatchRecord837`` via
|
|
``__new__`` so isinstance checks downstream narrow ``result`` correctly.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from datetime import datetime
|
|
from typing import Any, Literal
|
|
|
|
from pydantic import BaseModel, ConfigDict, model_validator
|
|
|
|
from cyclone.parsers.models import ParseResult
|
|
from cyclone.parsers.models_835 import ParseResult835
|
|
|
|
|
|
BatchKind = Literal["837p", "835"]
|
|
|
|
|
|
class BatchRecord(BaseModel):
|
|
"""One parsed file, with a stable uuid4 id and the full ParseResult.
|
|
|
|
``result`` is a union: ``ParseResult`` for ``kind="837p"`` and
|
|
``ParseResult835`` for ``kind="835"``. The concrete subclasses
|
|
``BatchRecord837`` and ``BatchRecord835`` narrow those fields, so
|
|
callers that want type-checked access should use them and check
|
|
``isinstance`` rather than pattern-matching on ``kind``.
|
|
|
|
Constructing ``BatchRecord(kind="837p", ...)`` dispatches to
|
|
``BatchRecord837``; ``kind="835"`` dispatches to ``BatchRecord835``.
|
|
This lets the union-member narrowing work transparently.
|
|
"""
|
|
|
|
model_config = ConfigDict(extra="ignore")
|
|
|
|
id: str
|
|
kind: BatchKind
|
|
input_filename: str
|
|
parsed_at: datetime # tz-aware UTC
|
|
result: ParseResult | ParseResult835
|
|
|
|
def __new__(
|
|
cls, *args: Any, **kwargs: Any,
|
|
) -> BatchRecord837 | BatchRecord835 | BatchRecord:
|
|
# Dispatch base-class construction to the right concrete subclass
|
|
# so isinstance checks downstream narrow `result` correctly.
|
|
if cls is BatchRecord:
|
|
kind = kwargs.get("kind")
|
|
if kind is None and args and isinstance(args[0], dict):
|
|
kind = args[0].get("kind")
|
|
if kind == "837p":
|
|
return BatchRecord837(*args, **kwargs)
|
|
if kind == "835":
|
|
return BatchRecord835(*args, **kwargs)
|
|
return super().__new__(cls)
|
|
|
|
@model_validator(mode="after")
|
|
def _check_parsed_at_tz(self) -> BatchRecord:
|
|
if self.parsed_at.tzinfo is None:
|
|
raise ValueError(
|
|
"parsed_at must be tz-aware (use datetime.now(timezone.utc))"
|
|
)
|
|
return self
|
|
|
|
|
|
class BatchRecord837(BatchRecord):
|
|
"""A parsed 837P (professional claim) batch."""
|
|
|
|
kind: Literal["837p"] = "837p"
|
|
result: ParseResult
|
|
|
|
|
|
class BatchRecord835(BatchRecord):
|
|
"""A parsed 835 (remittance advice) batch."""
|
|
|
|
kind: Literal["835"] = "835"
|
|
result: ParseResult835
|
|
```
|
|
|
|
- [ ] **Step 2: Update `__init__.py`**
|
|
|
|
Remove the lines containing `BatchKind`, `BatchRecord`, `BatchRecord837`, `BatchRecord835` from `__init__.py`. Add to imports at the top:
|
|
|
|
```python
|
|
from .records import BatchKind, BatchRecord, BatchRecord837, BatchRecord835
|
|
```
|
|
|
|
- [ ] **Step 3: Verify tests still pass**
|
|
|
|
```bash
|
|
cd backend && python -m pytest tests/ --tb=line -q 2>&1 | tail -5
|
|
```
|
|
|
|
Expected: identical counts to baseline.
|
|
|
|
---
|
|
|
|
## Task 4: Extract the orm_builders module
|
|
|
|
**Files:**
|
|
- Create: `backend/src/cyclone/store/orm_builders.py`
|
|
- Modify: `backend/src/cyclone/store/__init__.py` (remove the row builders)
|
|
|
|
- [ ] **Step 1: Create `orm_builders.py`**
|
|
|
|
Copy lines 172-342 from the current `__init__.py` into `orm_builders.py`. This includes:
|
|
- `_service_dates_from_claim` (172-185)
|
|
- `_claim_837_row` (188-203)
|
|
- `_remittance_835_row` (205-240)
|
|
- `_cas_adjustment_row` (242-261)
|
|
- `_persist_835_remit` (263-329)
|
|
- `_claim_status_from_validation` (331-342)
|
|
|
|
Module docstring:
|
|
|
|
```python
|
|
"""ORM row builders — convert parser output into SQLAlchemy ORM rows.
|
|
|
|
Each function returns an unsaved ORM object (or inserts related rows
|
|
within an existing session). They never commit or close the session —
|
|
the caller (``write.add_record``, ``acks.add_ack``, etc.) owns the
|
|
transaction boundary.
|
|
"""
|
|
```
|
|
|
|
- [ ] **Step 2: Update `__init__.py`**
|
|
|
|
Remove the row-builder functions from `__init__.py`. Add to imports:
|
|
|
|
```python
|
|
from .orm_builders import _claim_status_from_validation
|
|
```
|
|
|
|
`_claim_status_from_validation` is the only one re-exported (for `batch_diff.py` line 32). The others are module-private — siblings import them directly via `from .orm_builders import _service_dates_from_claim` etc.
|
|
|
|
- [ ] **Step 3: Verify tests still pass**
|
|
|
|
```bash
|
|
cd backend && python -m pytest tests/ --tb=line -q 2>&1 | tail -5
|
|
```
|
|
|
|
Expected: identical counts to baseline.
|
|
|
|
---
|
|
|
|
## Task 5: Extract the ui module
|
|
|
|
**Files:**
|
|
- Create: `backend/src/cyclone/store/ui.py`
|
|
- Modify: `backend/src/cyclone/store/__init__.py` (remove the UI serializers + utcnow)
|
|
|
|
- [ ] **Step 1: Create `ui.py`**
|
|
|
|
Copy lines 344-855 from the current `__init__.py` into `ui.py`. This includes:
|
|
- `to_ui_claim` (344-370)
|
|
- `to_ui_remittance` (372-427)
|
|
- `to_ui_claim_from_orm` (429-487)
|
|
- `CLAIM_DETAIL_HISTORY_LIMIT` (489-490)
|
|
- `_iso_z` (492-505)
|
|
- `_address_to_ui` (507-523)
|
|
- `_validation_issues_to_ui` (525-542)
|
|
- `to_ui_claim_detail` (544-679)
|
|
- `to_ui_remittance_from_orm` (681-715)
|
|
- `to_ui_remittance_with_adjustments` (717-762)
|
|
- `_svc_to_wire_dict` (764-784)
|
|
- `to_ui_provider` (786-811)
|
|
- `to_ui_activity_event` (813-830)
|
|
- `_date_in_bounds` (832-855)
|
|
|
|
Also copy `utcnow()` (line 162-164) into `ui.py`. Module docstring:
|
|
|
|
```python
|
|
"""UI serialization helpers — convert ORM rows to API-shaped dicts.
|
|
|
|
These are the boundary between the persistence layer and the wire format
|
|
consumed by the React frontend. Keep them stable: any rename or
|
|
shape change ripples to every API consumer.
|
|
"""
|
|
```
|
|
|
|
- [ ] **Step 2: Update `__init__.py`**
|
|
|
|
Remove the UI serializer functions AND `utcnow()` from `__init__.py`. Add to imports:
|
|
|
|
```python
|
|
from .ui import utcnow
|
|
```
|
|
|
|
`utcnow` is the only one re-exported (for `api_helpers.py` docstring reference and any external use).
|
|
|
|
- [ ] **Step 3: Verify tests still pass**
|
|
|
|
```bash
|
|
cd backend && python -m pytest tests/ --tb=line -q 2>&1 | tail -5
|
|
```
|
|
|
|
Expected: identical counts to baseline.
|
|
|
|
---
|
|
|
|
## Task 6: Extract the write module
|
|
|
|
**Files:**
|
|
- Create: `backend/src/cyclone/store/write.py`
|
|
- Modify: `backend/src/cyclone/store/__init__.py` (remove `add`, `_publish_events_sync`, `_sync_publish`, `_run_reconcile`)
|
|
|
|
- [ ] **Step 1: Create `write.py`**
|
|
|
|
Copy lines 898-1107 from the current `__init__.py` into `write.py`. This includes:
|
|
- `add(record, *, event_bus=None)` (898-1040) — rename to module function `add_record`
|
|
- `_publish_events_sync(event_bus, record, claim_ids, remit_ids)` (1042-1097) — drop the `self.` prefix
|
|
- `_sync_publish(event_bus, kind, payload)` (1100-1107) — drop `self.` and `@staticmethod`
|
|
- `_run_reconcile(batch_id)` (1109-1129) — drop the `self.` prefix
|
|
|
|
Rename `CycloneStore.add` body to module function `add_record(record, *, event_bus=None)`. The body is unchanged except:
|
|
- `self._publish_events_sync(...)` → `publish_events_sync(...)`
|
|
- `self._run_reconcile(...)` → `run_reconcile(...)`
|
|
- `self._sync_publish(...)` → `_sync_publish(...)`
|
|
|
|
Module docstring:
|
|
|
|
```python
|
|
"""Write path for parsed batches — insert, reconcile, publish.
|
|
|
|
The single entry point is ``add_record(record, *, event_bus=None)``,
|
|
which replaces the body of the previous ``CycloneStore.add`` method.
|
|
It owns its own SQLAlchemy session, runs idempotency checks, persists
|
|
the batch + child rows, then (for 835 batches) triggers reconciliation
|
|
and (if ``event_bus`` is provided) publishes live-tail events.
|
|
|
|
Reconciliation runs OUTSIDE the persistence session, fail-soft: errors
|
|
are logged but do not roll back the persisted batch.
|
|
"""
|
|
```
|
|
|
|
- [ ] **Step 2: Update `__init__.py` CycloneStore class**
|
|
|
|
Replace the `add` method body:
|
|
|
|
```python
|
|
def add(self, record, *, event_bus=None):
|
|
"""Persist a parsed batch (837P or 835). See cyclone.store.write.add_record."""
|
|
return write.add_record(record, event_bus=event_bus)
|
|
```
|
|
|
|
Replace the 3 private methods:
|
|
|
|
```python
|
|
def _publish_events_sync(self, event_bus, record, claim_ids, remit_ids):
|
|
return write.publish_events_sync(event_bus, record, claim_ids, remit_ids)
|
|
|
|
def _run_reconcile(self, batch_id):
|
|
return write.run_reconcile(batch_id)
|
|
|
|
@staticmethod
|
|
def _sync_publish(event_bus, kind, payload):
|
|
return write._sync_publish(event_bus, kind, payload)
|
|
```
|
|
|
|
Add import at top of `__init__.py`:
|
|
|
|
```python
|
|
from . import write
|
|
```
|
|
|
|
- [ ] **Step 3: Verify hot-path tests pass**
|
|
|
|
```bash
|
|
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_api_line_reconciliation.py --tb=short -q
|
|
```
|
|
|
|
Expected: 100% pass on these 7 files.
|
|
|
|
- [ ] **Step 4: Verify full test suite matches baseline**
|
|
|
|
```bash
|
|
cd backend && python -m pytest tests/ --tb=line -q 2>&1 | tail -5
|
|
```
|
|
|
|
Expected: identical counts to baseline.
|
|
|
|
---
|
|
|
|
## Task 7: Extract the batches module
|
|
|
|
**Files:**
|
|
- Create: `backend/src/cyclone/store/batches.py`
|
|
- Modify: `backend/src/cyclone/store/__init__.py` (remove `_BatchesShim`, `_row_to_record`, `get_batch`, `get`, `list`, `all`, `load_two_for_diff` from CycloneStore)
|
|
|
|
- [ ] **Step 1: Create `batches.py`**
|
|
|
|
Copy from `__init__.py`:
|
|
- `_BatchesShim` class (lines 857-880) — unchanged
|
|
- `_row_to_record` (1131-1161) — unchanged
|
|
- `get_batch(batch_id)` (1163-1180) — rename from method to function, drop `self`
|
|
- `get(batch_id)` (1182-1194) — rename to `get_record`
|
|
- `list(*, limit)` (1196-1205) — rename to `list_batches`
|
|
- `all()` (1429-1433) — rename to `all_batches`
|
|
- `load_two_for_diff(batch_ids)` (1435-1458) — drop `self`
|
|
|
|
Module docstring:
|
|
|
|
```python
|
|
"""Batch read APIs and the legacy _BatchesShim.
|
|
|
|
The ``_BatchesShim`` class is retained for back-compat with the
|
|
``with store._lock: store._batches.clear()`` test-cleanup idiom used
|
|
in 17 test files. Its ``clear()`` method wipes all rows from the DB
|
|
tables.
|
|
"""
|
|
```
|
|
|
|
- [ ] **Step 2: Update `__init__.py` CycloneStore class**
|
|
|
|
Add import:
|
|
|
|
```python
|
|
from .batches import (
|
|
_BatchesShim, _row_to_record,
|
|
get_batch, get_record, list_batches, all_batches, load_two_for_diff,
|
|
)
|
|
```
|
|
|
|
Replace the 6 method bodies with delegations:
|
|
|
|
```python
|
|
def get_batch(self, batch_id):
|
|
return get_batch(batch_id)
|
|
|
|
def get(self, batch_id):
|
|
return get_record(batch_id)
|
|
|
|
def list(self, *, limit=100):
|
|
return list_batches(limit=limit)
|
|
|
|
def all(self):
|
|
return all_batches()
|
|
|
|
def load_two_for_diff(self, batch_ids):
|
|
return load_two_for_diff(batch_ids)
|
|
```
|
|
|
|
- [ ] **Step 3: Verify tests pass**
|
|
|
|
```bash
|
|
cd backend && python -m pytest tests/test_api_gets.py tests/test_prodfiles_smoke.py tests/test_api_parse_persists.py tests/test_api_parse_persists_ack.py tests/test_api_claim_detail.py tests/test_api_999.py tests/test_api_ta1.py tests/test_store.py --tb=short -q
|
|
```
|
|
|
|
Expected: 100% pass on these 8 files (the ones that use `_lock`/`_batches.clear()` plus the store-specific tests).
|
|
|
|
---
|
|
|
|
## Task 8: Extract the claim_detail module
|
|
|
|
**Files:**
|
|
- Create: `backend/src/cyclone/store/claim_detail.py`
|
|
- Modify: `backend/src/cyclone/store/__init__.py` (remove the read methods: `get_remittance`, `get_claim_detail`, `iter_claims`, `iter_remittances`, `distinct_providers`, `recent_activity`)
|
|
|
|
- [ ] **Step 1: Create `claim_detail.py`**
|
|
|
|
Copy from `__init__.py`:
|
|
- `get_remittance(remittance_id)` (1207-1266) — drop `self`
|
|
- `get_claim_detail(claim_id)` (1268-1427) — drop `self`
|
|
- `iter_claims(...)` (1460-1550) — drop `self`, preserve generator pattern
|
|
- `iter_remittances(...)` (1552-1643) — drop `self`, preserve generator pattern
|
|
- `distinct_providers()` (1645-1668) — drop `self`
|
|
- `recent_activity(*, limit)` (1670-1691) — drop `self`
|
|
|
|
Module docstring:
|
|
|
|
```python
|
|
"""Claim- and remittance-detail read APIs.
|
|
|
|
Includes the only large non-write query in the store:
|
|
``get_claim_detail`` (~160 lines) which joins Claim + CasAdjustment +
|
|
ActivityEvent history for the right-drawer UI.
|
|
"""
|
|
```
|
|
|
|
- [ ] **Step 2: Update `__init__.py` CycloneStore class**
|
|
|
|
Add import:
|
|
|
|
```python
|
|
from .claim_detail import (
|
|
get_remittance, get_claim_detail,
|
|
iter_claims, iter_remittances,
|
|
distinct_providers, recent_activity,
|
|
)
|
|
```
|
|
|
|
Replace the 6 method bodies with delegations:
|
|
|
|
```python
|
|
def get_remittance(self, remittance_id):
|
|
return get_remittance(remittance_id)
|
|
|
|
def get_claim_detail(self, claim_id):
|
|
return get_claim_detail(claim_id)
|
|
|
|
def iter_claims(self, *args, **kwargs):
|
|
return iter_claims(*args, **kwargs)
|
|
|
|
def iter_remittances(self, *args, **kwargs):
|
|
return iter_remittances(*args, **kwargs)
|
|
|
|
def distinct_providers(self):
|
|
return distinct_providers()
|
|
|
|
def recent_activity(self, *, limit=200):
|
|
return recent_activity(limit=limit)
|
|
```
|
|
|
|
- [ ] **Step 3: Verify tests pass**
|
|
|
|
```bash
|
|
cd backend && python -m pytest tests/test_api_claim_detail.py tests/test_store_claim_detail.py tests/test_lane_filter_acknowledged.py --tb=short -q
|
|
```
|
|
|
|
Expected: 100% pass.
|
|
|
|
---
|
|
|
|
## Task 9: Extract the acks module
|
|
|
|
**Files:**
|
|
- Create: `backend/src/cyclone/store/acks.py`
|
|
- Modify: `backend/src/cyclone/store/__init__.py` (remove the 9 ACK methods)
|
|
|
|
- [ ] **Step 1: Create `acks.py`**
|
|
|
|
Copy from `__init__.py`:
|
|
- `add_ack(...)` (1693-1727) → `add_999_ack`
|
|
- `list_acks()` (1729-1736)
|
|
- `get_ack(ack_id)` (1738-1743)
|
|
- `add_ta1_ack(...)` (1745-1783)
|
|
- `list_ta1_acks()` (1785-1796)
|
|
- `get_ta1_ack(ack_id)` (1798-1803)
|
|
- `add_277ca_ack(...)` (1805-1836)
|
|
- `list_277ca_acks()` (1838-1845)
|
|
- `get_277ca_ack(ack_id)` (1847-1852)
|
|
|
|
Drop `self` from each. Module docstring:
|
|
|
|
```python
|
|
"""ACK persistence — 999 / TA1 / 277CA acknowledgements.
|
|
|
|
Each ACK type has its own ORM row (Ack / Ta1Ack / Two77caAck). The
|
|
write methods are simple inserts; the list/get methods are simple
|
|
queries. Fail-soft: persistence errors are logged but do not block
|
|
parsing.
|
|
"""
|
|
```
|
|
|
|
- [ ] **Step 2: Update `__init__.py` CycloneStore class**
|
|
|
|
Add import:
|
|
|
|
```python
|
|
from .acks import (
|
|
add_999_ack, list_acks, get_ack,
|
|
add_ta1_ack, list_ta1_acks, get_ta1_ack,
|
|
add_277ca_ack, list_277ca_acks, get_277ca_ack,
|
|
)
|
|
```
|
|
|
|
Replace the 9 method bodies with delegations. (Method names on the class stay the same as today — only the module-function names use the type prefix for clarity.)
|
|
|
|
```python
|
|
def add_ack(self, **kwargs):
|
|
return add_999_ack(**kwargs)
|
|
|
|
def list_acks(self):
|
|
return list_acks()
|
|
|
|
def get_ack(self, ack_id):
|
|
return get_ack(ack_id)
|
|
|
|
def add_ta1_ack(self, **kwargs):
|
|
return add_ta1_ack(**kwargs)
|
|
|
|
def list_ta1_acks(self):
|
|
return list_ta1_acks()
|
|
|
|
def get_ta1_ack(self, ack_id):
|
|
return get_ta1_ack(ack_id)
|
|
|
|
def add_277ca_ack(self, **kwargs):
|
|
return add_277ca_ack(**kwargs)
|
|
|
|
def list_277ca_acks(self):
|
|
return list_277ca_acks()
|
|
|
|
def get_277ca_ack(self, ack_id):
|
|
return get_277ca_ack(ack_id)
|
|
```
|
|
|
|
- [ ] **Step 3: Verify tests pass**
|
|
|
|
```bash
|
|
cd backend && python -m pytest tests/test_api_parse_persists_ack.py tests/test_api_ta1.py tests/test_acks.py tests/test_api_999.py tests/test_apply_277ca_rejections.py --tb=short -q
|
|
```
|
|
|
|
Expected: 100% pass on these 5 ACK-related test files.
|
|
|
|
---
|
|
|
|
## Task 10: Extract the backups module
|
|
|
|
**Files:**
|
|
- Create: `backend/src/cyclone/store/backups.py`
|
|
- Modify: `backend/src/cyclone/store/__init__.py` (remove `add_backup_pending`)
|
|
|
|
- [ ] **Step 1: Create `backups.py`**
|
|
|
|
Copy `add_backup_pending(*, filename, backup_dir)` (lines 1854-1878) from `__init__.py`. Drop `self`. Module docstring:
|
|
|
|
```python
|
|
"""Backup-pending marker persistence.
|
|
|
|
Used by ``cyclone.backup_service`` to mark files that need backing up
|
|
after parsing. Simple insert into the DbBackup table.
|
|
"""
|
|
```
|
|
|
|
- [ ] **Step 2: Update `__init__.py` CycloneStore class**
|
|
|
|
Add import:
|
|
|
|
```python
|
|
from .backups import add_backup_pending
|
|
```
|
|
|
|
Replace the method body:
|
|
|
|
```python
|
|
def add_backup_pending(self, *, filename, backup_dir):
|
|
return add_backup_pending(filename=filename, backup_dir=backup_dir)
|
|
```
|
|
|
|
- [ ] **Step 3: Verify tests pass**
|
|
|
|
```bash
|
|
cd backend && python -m pytest tests/test_backup_service.py tests/test_api_backup.py --tb=short -q
|
|
```
|
|
|
|
Expected: 100% pass.
|
|
|
|
---
|
|
|
|
## Task 11: Extract the inbox + providers modules
|
|
|
|
**Files:**
|
|
- Create: `backend/src/cyclone/store/inbox.py`
|
|
- Create: `backend/src/cyclone/store/providers.py`
|
|
- Modify: `backend/src/cyclone/store/__init__.py` (remove 3 inbox + 7 provider/payer methods)
|
|
|
|
- [ ] **Step 1: Create `inbox.py`**
|
|
|
|
Copy from `__init__.py`:
|
|
- `list_unmatched(*, kind)` (1880-1949) — drop `self`
|
|
- `manual_match(claim_id, remit_id)` (1951-2068) — drop `self`, import `reconcile` at module top
|
|
- `manual_unmatch(claim_id)` (2070-2163) — drop `self`
|
|
|
|
`inbox.py` module docstring:
|
|
|
|
```python
|
|
"""Inbox lane state and manual match/unmatch operations.
|
|
|
|
``list_unmatched`` returns the 5-lane inbox view (unmatched claims +
|
|
unmatched remits). ``manual_match`` and ``manual_unmatch`` invoke the
|
|
reconcile pure functions and persist the resulting Match row (or
|
|
remove it). Invalid state transitions surface as ``InvalidStateError``.
|
|
"""
|
|
```
|
|
|
|
Add at the top of `inbox.py`:
|
|
|
|
```python
|
|
from cyclone import db, reconcile
|
|
from .exceptions import InvalidStateError
|
|
```
|
|
|
|
- [ ] **Step 2: Create `providers.py`**
|
|
|
|
Copy from `__init__.py`:
|
|
- `list_providers(*, is_active)` (2165-2174)
|
|
- `get_provider(npi)` (2176-2181)
|
|
- `upsert_provider(provider)` (2183-2214)
|
|
- `list_payers(*, is_active)` (2216-2224)
|
|
- `get_payer_config(payer_id, transaction_type)` (2226-2230)
|
|
- `get_clearhouse()` (2232-2250)
|
|
- `ensure_clearhouse_seeded()` (2252-end-of-class)
|
|
- `_provider_orm_to_dict(row)` (2382-2397)
|
|
- `_payer_orm_to_dict(row)` (2400-2410)
|
|
|
|
Drop `self` from each. Module docstring:
|
|
|
|
```python
|
|
"""Provider, payer, and clearhouse configuration reads and upserts.
|
|
|
|
The ``providers`` and ``payers`` modules in ``cyclone`` provide the
|
|
ORM-row DTOs; this module is the read/upsert surface over them.
|
|
``ensure_clearhouse_seeded`` is called at startup to insert the
|
|
default Clearhouse row if missing.
|
|
"""
|
|
```
|
|
|
|
Add at the top of `providers.py`:
|
|
|
|
```python
|
|
from cyclone import db
|
|
from cyclone.providers import Clearhouse, Payer, Provider
|
|
```
|
|
|
|
- [ ] **Step 3: Update `__init__.py` CycloneStore class**
|
|
|
|
Add imports:
|
|
|
|
```python
|
|
from .inbox import list_unmatched, manual_match, manual_unmatch
|
|
from .providers import (
|
|
list_providers, get_provider, upsert_provider, list_payers,
|
|
get_payer_config, get_clearhouse, ensure_clearhouse_seeded,
|
|
)
|
|
```
|
|
|
|
Replace the 10 method bodies with delegations:
|
|
|
|
```python
|
|
def list_unmatched(self, *, kind="both"):
|
|
return list_unmatched(kind=kind)
|
|
|
|
def manual_match(self, claim_id, remit_id):
|
|
return manual_match(claim_id, remit_id)
|
|
|
|
def manual_unmatch(self, claim_id):
|
|
return manual_unmatch(claim_id)
|
|
|
|
def list_providers(self, *, is_active=True):
|
|
return list_providers(is_active=is_active)
|
|
|
|
def get_provider(self, npi):
|
|
return get_provider(npi)
|
|
|
|
def upsert_provider(self, provider):
|
|
return upsert_provider(provider)
|
|
|
|
def list_payers(self, *, is_active=True):
|
|
return list_payers(is_active=is_active)
|
|
|
|
def get_payer_config(self, payer_id, transaction_type):
|
|
return get_payer_config(payer_id, transaction_type)
|
|
|
|
def get_clearhouse(self):
|
|
return get_clearhouse()
|
|
|
|
def ensure_clearhouse_seeded(self):
|
|
return ensure_clearhouse_seeded()
|
|
```
|
|
|
|
- [ ] **Step 4: Verify hot-path tests pass**
|
|
|
|
```bash
|
|
cd backend && python -m pytest 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_reconcile.py tests/test_reconcile.py tests/test_reconcile_line_level.py tests/test_api_line_reconciliation.py tests/test_providers_seed.py --tb=short -q
|
|
```
|
|
|
|
Expected: 100% pass on these 10 hot-path files.
|
|
|
|
---
|
|
|
|
## Task 12: Final verification + cleanup
|
|
|
|
- [ ] **Step 1: Run full test suite, compare to baseline**
|
|
|
|
```bash
|
|
cd backend && python -m pytest tests/ --tb=line -q 2>&1 | tail -5
|
|
```
|
|
|
|
Expected: identical counts to `/tmp/cyclone-baseline.txt` line. If any deviation, halt and investigate.
|
|
|
|
- [ ] **Step 2: Import sanity check**
|
|
|
|
```bash
|
|
python -c "
|
|
from cyclone.store import (
|
|
CycloneStore, store, BatchRecord, BatchRecord837, BatchRecord835, BatchKind,
|
|
AlreadyMatchedError, NotMatchedError, InvalidStateError, utcnow,
|
|
_claim_status_from_validation,
|
|
)
|
|
print('all public imports OK')
|
|
|
|
# Verify the singleton has the expected methods
|
|
import inspect
|
|
store_methods = [m for m in dir(store) if not m.startswith('__')]
|
|
expected = {'add', 'get', 'get_batch', 'list', 'all', 'get_claim_detail',
|
|
'get_remittance', 'iter_claims', 'iter_remittances',
|
|
'distinct_providers', 'recent_activity', 'add_ack', 'list_acks',
|
|
'get_ack', 'add_ta1_ack', 'list_ta1_acks', 'get_ta1_ack',
|
|
'add_277ca_ack', 'list_277ca_acks', 'get_277ca_ack',
|
|
'add_backup_pending', 'list_unmatched', 'manual_match',
|
|
'manual_unmatch', 'list_providers', 'get_provider',
|
|
'upsert_provider', 'list_payers', 'get_payer_config',
|
|
'get_clearhouse', 'ensure_clearhouse_seeded', 'load_two_for_diff'}
|
|
missing = expected - set(store_methods)
|
|
assert not missing, f'missing methods: {missing}'
|
|
print('all 32 public methods present on singleton')
|
|
"
|
|
```
|
|
|
|
Expected: `all public imports OK` and `all 32 public methods present on singleton`.
|
|
|
|
- [ ] **Step 3: Live smoke test on 8 endpoints**
|
|
|
|
Per the api.py split precedent (commit `931782b`), hit each endpoint and verify response shape:
|
|
|
|
```bash
|
|
# Start the backend
|
|
cd backend && python -m cyclone &
|
|
sleep 3
|
|
|
|
# 8 endpoint checks (curl + jq, then kill backend)
|
|
# 1. POST /api/parse-999 (valid fixture → ack_code=A)
|
|
# 2. POST /api/parse-999 (rejected fixture → ack_code=R)
|
|
# 3. POST /api/parse-277ca (valid → 200 with 3 claim statuses)
|
|
# 4. GET /api/inbox?lane=unmatched (list_unmatched shape)
|
|
# 5. POST /api/inbox/match (manual_match happy path)
|
|
# 6. POST /api/inbox/unmatch (manual_unmatch happy path)
|
|
# 7. GET /api/batches + /api/batches/{id} (get_batch shape)
|
|
# 8. GET /api/claims/{id} (get_claim_detail shape)
|
|
|
|
pkill -f "python -m cyclone"
|
|
```
|
|
|
|
Document the smoke results in the commit message.
|
|
|
|
- [ ] **Step 4: Final commit — the single atomic commit**
|
|
|
|
The working tree should currently show all of Tasks 1-11 as uncommitted edits (the atomic-move of Task 1 + the 10 extractions of Tasks 2-11). Review the diff and commit once.
|
|
|
|
```bash
|
|
cd /Users/openclaw/dev/cyclone
|
|
|
|
# Review what's about to be committed
|
|
git status
|
|
|
|
# Confirm the only changes are backend/src/cyclone/store* — no source
|
|
# outside the store package, no test files, no docs.
|
|
git diff --stat
|
|
|
|
# The expected diff: store.py deleted, store/__init__.py modified,
|
|
# store/{exceptions,records,orm_builders,ui,write,batches,claim_detail,
|
|
# acks,backups,inbox,providers}.py all created.
|
|
|
|
git add -A backend/src/cyclone/
|
|
git commit -m "refactor(store): split store.py into cyclone/store/ subpackage
|
|
|
|
Step 4 of the file-split refactor series. CycloneStore facade now
|
|
delegates to 11 sibling modules:
|
|
|
|
exceptions - AlreadyMatchedError, NotMatchedError, InvalidStateError
|
|
records - BatchKind, BatchRecord (837/835 subclasses)
|
|
orm_builders - _claim_837_row, _remittance_835_row, _persist_835_remit,
|
|
_claim_status_from_validation
|
|
ui - to_ui_*, _iso_z, _address_to_ui, utcnow
|
|
write - add_record, publish_events_sync, run_reconcile
|
|
batches - get_batch, get_record, list_batches, _BatchesShim
|
|
claim_detail - get_remittance, get_claim_detail, iter_claims, iter_remittances
|
|
acks - add_999_ack, add_ta1_ack, add_277ca_ack + reads
|
|
backups - add_backup_pending
|
|
inbox - list_unmatched, manual_match, manual_unmatch
|
|
providers - list_providers, upsert_provider, ensure_clearhouse_seeded
|
|
|
|
CycloneStore class keeps its current method signatures as 1-line
|
|
delegations. The store singleton and all public imports
|
|
(CycloneStore, store, BatchRecord, exception types, utcnow,
|
|
_claim_status_from_validation) continue to work unchanged.
|
|
|
|
Test suite: identical pass/fail/skip counts to pre-split baseline.
|
|
Smoke: 8 endpoints return the same shapes they did before the split.
|
|
|
|
Zero public API changes, zero test changes. Behaviour-preserving."
|
|
```
|
|
|
|
---
|
|
|
|
## Self-review
|
|
|
|
**1. Spec coverage:**
|
|
|
|
| Spec section | Task |
|
|
|---|---|
|
|
| Target module layout | Tasks 2-11 (each module is one task) |
|
|
| CycloneStore class shape (3 categories) | Tasks 6-11 |
|
|
| Naming collision table | Tasks 7-11 (renamed to module functions in those tasks) |
|
|
| Public API preservation | Task 12 Step 2 verifies the 11 currently-imported names |
|
|
| Migration plan | Task 1 (atomic move) + Tasks 2-11 (extractions) + Task 12 (final commit) |
|
|
| Test strategy (4 tiers) | Task 12 Steps 1-3 |
|
|
| Risk register (R1-R10) | Each task's Step 3 verifies tests pass; R1 (no cycles) is satisfied because siblings never import from `__init__.py` |
|
|
| Pre-flight | Task 0 |
|
|
| Post-flight | Task 12 Steps 1-3 |
|
|
| Out of scope | Plan respects all 10 out-of-scope items |
|
|
|
|
**2. Placeholder scan:** No "TBD", "TODO", "fill in details", "similar to Task N" in any task. Code is shown where the task requires it. Module docstrings are shown verbatim.
|
|
|
|
**3. Type consistency:** All method signatures preserved. Module function renames (`list_batches`, `all_batches`, `get_record`, `add_999_ack`, `add_record`) match between the create-task and the delegation-task in each pair. The CycloneStore method names (`store.list`, `store.add`, `store.add_ack`) are unchanged from current behavior.
|
|
|
|
**4. Gap check:** The plan doesn't explicitly call out extracting `utcnow()` (Task 5 includes it in `ui.py`). It doesn't explicitly call out the singleton setup line — that's preserved as `store = CycloneStore()` at the bottom of `__init__.py`, untouched through all tasks. Both are implicit but discoverable. |