cyclone
375 lines
28 KiB
Markdown
375 lines
28 KiB
Markdown
# Sub-project 36 — API Routers Split: Design Spec
|
||
|
||
**Date:** 2026-07-06
|
||
**Status:** Merged into main (`f005494 merge: SP36 api-routers-split into main` on 2026-07-07)
|
||
**Branch:** `sp36-api-routers-split` (merged + deleted)
|
||
**Aesthetic direction:** No UI changes. Pure backend structural refactor — the public HTTP surface is byte-for-byte identical.
|
||
|
||
---
|
||
|
||
## 1. Scope
|
||
|
||
`backend/src/cyclone/api.py` has grown to 4,341 LOC across 63 route handlers and 2 exception handlers. The `api_routers/` subpackage exists but holds only 5 small routers (`acks`, `admin`, `claim_acks`, `health`, `ta1_acks`, totalling ~700 LOC). SP21 split the persistence layer into a `cyclone/store/` subpackage; SP36 finishes that era of work by splitting the HTTP surface into per-resource routers that mirror the store's domain boundaries.
|
||
|
||
The split is **behaviour-preserving**: every URL, every HTTP method, every status code, every response shape, every header is unchanged. The migration set, the store facade, the pubsub event contract, the auth boundary, the live-tail wire format, the parser pipeline, the CLI, and the frontend are all untouched. The work is moving code from one file to many files, registering the new files as FastAPI routers, and verifying the public surface is byte-for-byte identical.
|
||
|
||
**In scope:**
|
||
|
||
- Extract the 63 routes currently in `api.py` into per-domain routers under `api_routers/`:
|
||
- `parse.py` (5 routes: 837, 835, 999, ta1, 277ca)
|
||
- `inbox.py` (6 routes: lanes, candidates/match, candidates/dismiss, payer-rejected/acknowledge, rejected/resubmit, export.csv)
|
||
- `batches.py` (3 routes: list, get, export-837) + the three `_batch_summary_*` helpers
|
||
- `claims.py` (5 routes: list, stream, get, serialize-837, line-reconciliation) + `_compact_ack_links_for_claim`
|
||
- `reconciliation.py` (4 routes: unmatched, batch-diff, match, unmatch)
|
||
- `remittances.py` (4 routes: list, summary, stream, get)
|
||
- `dashboard.py` (1 route: kpis)
|
||
- `providers.py` (3 routes: providers, config/providers, config/providers/{npi})
|
||
- `activity.py` (2 routes: list, stream)
|
||
- `eligibility.py` (2 routes: request, parse-271)
|
||
- `clearhouse.py` (3 routes: get, patch, submit)
|
||
- `config.py` (2 routes: config/payers, config/payers/{id}/configs)
|
||
- `payers.py` (1 route: {id}/summary)
|
||
- `admin.py` (existing, absorb 20 routes: audit-log ×2, db/rotate-key, backup ×10, scheduler ×6, reload-config)
|
||
- `acks.py` (existing, absorb 2 routes: 277ca-acks list/get)
|
||
- Move the 12 cross-router helper functions to `api_routers/_shared.py` (private to the package, leading underscore).
|
||
- Reduce `api.py` to a thin shell: `app = FastAPI(...)`, `lifespan`, both `@app.exception_handler`s, and the `include_router` loop driven by a `routers: list[APIRouter]` exported from `api_routers/__init__.py`.
|
||
- Keep `_ndjson_line` and `_tail_events` in `api_helpers.py` (already correct home; not duplicated).
|
||
- Live-test after each router is extracted: `curl` a representative route, assert the expected status code, then run a backend `pytest` baseline diff to confirm zero regressions.
|
||
- Autoreview after each router is extracted: spawn a `pr-reviewer` subagent on the staged diff before commit; the reviewer checks (a) route registration, (b) no orphan imports, (c) no leaked business logic, (d) no leaked auth-bypass.
|
||
- Commit per-router with `feat(sp36): extract {router-name} router` prefix per the SP-N convention.
|
||
- Track progress in `/tmp/refactor-cyclone.md` per the user's standing directive: each step logged with pre/post pytest counts, live-test result, and reviewer verdict.
|
||
- Single atomic merge commit into `main` once every router is extracted, the full backend test suite matches the Step-0 baseline, the full frontend test suite is green, and a one-route-per-prefix live matrix returns 2xx.
|
||
|
||
**Out of scope:**
|
||
|
||
- No new endpoints, no behavior changes, no status code changes, no response shape changes, no header changes. Refactor is byte-for-byte transparent to clients.
|
||
- No change to the auth boundary. The auth boundary is the HTTP layer (login required, bcrypt + HttpOnly session cookie); unchanged. The new routers mount under the same `matrix_gate` dependency, same as today.
|
||
- No change to the store facade, `db.py`, any `store/` module, the pubsub event bus, the parsers, the serializers, the CLI, the audit log, or the migration set.
|
||
- No change to `api_helpers.py` beyond ensuring it's still importable from its existing path. (Routers that need `_ndjson_line` / `_tail_events` continue to import from `cyclone.api_helpers`.)
|
||
- No splitting of `admin.py` into `admin_audit.py` / `admin_db.py` / `admin_backup.py` / `admin_scheduler.py`. That's a separate concern (~1,200 LOC of admin endpoints, but still one domain) and is filed as a possible follow-up SP.
|
||
- No renaming of any endpoint. No path normalization.
|
||
- No frontend changes. No changes to `src/`, `vite.config.ts`, `package.json`, or any UI asset.
|
||
- No documentation changes other than the spec + plan files for SP36 itself. The cyclone-skills (`.superpowers/skills/cyclone-api-router/SKILL.md`) will be re-read at the start of the next increment that adds an endpoint; the skills don't reference the specific module layout and don't need edits.
|
||
- No new tests. The existing test suite (`backend/tests/test_api_*.py`, sibling `src/**/*.test.tsx`) is the regression net. Pytest baseline (passed / failed-pre-existing / skipped counts) captured before, asserted equal after.
|
||
|
||
---
|
||
|
||
## 2. Decisions (locked during brainstorming)
|
||
|
||
### D1. Routers mirror the store's domain boundaries, not URL prefixes alone
|
||
|
||
The store split (SP21) landed `cyclone/store/{batches, claim_detail, acks, inbox, providers, kpis, ...}`. The new routers should mirror that. A reader who knows the store should be able to guess where a route lives. Three of the routers that don't have a 1-to-1 store counterpart (`reconciliation.py`, `eligibility.py`, `dashboard.py`) still get their own files because they are independent domains at the HTTP surface; their handlers call into a mix of store methods + helper functions.
|
||
|
||
### D2. `admin.py` stays one router even at ~1,200 LOC
|
||
|
||
The admin surface has 20 routes across 4 sub-domains (audit log, db key rotation, backups, scheduler) and a natural 4-way split would be cleaner. We keep it as one router for SP36 because (a) the routes all share the `matrix_gate` admin-only dep, (b) the SP's goal is per-domain router extraction, and (c) splitting admin recursively would balloon the diff. The follow-up that does the admin sub-split is filed as a separate concern.
|
||
|
||
### D3. `api_routers/__init__.py` is the registration point
|
||
|
||
`api.py` does:
|
||
```python
|
||
from cyclone.api_routers import routers
|
||
for r in routers:
|
||
app.include_router(r)
|
||
```
|
||
Cleaner than 19 individual `include_router` calls, easier to grep ("where is route X registered?"), and gives us a single line to flip for future feature flags. The auth router import (`from cyclone.auth.routes import router as auth_router`) keeps its explicit include in `api.py` because it's not under `api_routers/`.
|
||
|
||
### D4. `_shared.py` is the home for cross-router helpers only
|
||
|
||
A helper moves to `_shared.py` if and only if at least two routers use it. Single-router helpers (`_compact_ack_links_for_claim` for `claims.py`, `_batch_summary_*` for `batches.py`) stay private to the router that uses them. The 12 cross-router helpers listed in Section 3.3 are the full `_shared.py` surface. If a future router needs a helper that's currently in a single-router file, that helper is promoted to `_shared.py` as part of that future change.
|
||
|
||
### D5. No business logic in route handlers, but no logic change either
|
||
|
||
Route handlers before SP36 validate input, call the store, return the result. Route handlers after SP36 do the same thing in the same order. We do not refactor handlers as we move them — no extraction of common patterns, no introduction of dependencies-injection helpers, no factoring of duplicated try/except. The SP is a structural move, not a code-quality pass. Any "this looks like it could be cleaner" observations go in `/tmp/refactor-cyclone.md` as candidates for a future SP, not into this one.
|
||
|
||
### D6. Live-test + autoreview + commit happens per router, not at the end
|
||
|
||
Each router extraction is one `feat(sp36): extract {name} router` commit. The cycle is: pytest baseline → cut → pytest diff → curl the moved routes → autoreview → commit → update `/tmp/refactor-cyclone.md`. This means a router can be reverted individually if a regression slips through, the diff for review is small and focused, and the live system stays green throughout. The single atomic merge at the end folds all 19 router commits (5 existing routers + 14 new) into `main`.
|
||
|
||
### D7. The merge of SP35 into main happens before the SP36 branch is cut
|
||
|
||
SP35 (`sp35-parse-input-guards`) is sitting on a clean working tree with all four commits reviewable. SP36 branches from `main`, so SP35 must land first. This is a hard pre-condition: if SP35 isn't merged before the SP36 branch is cut, the SP36 branch will carry SP35's commits under it and the audit trail breaks.
|
||
|
||
### D8. The one-shot verification commands in Section 5.3 are the merge gate
|
||
|
||
A short battery of grep / wc / curl commands runs at the very end, before the merge commit. Any failure of these gates the merge until resolved. The commands check: `api.py` is thin, no router is over 1,400 LOC, no `session.add` / `s.add` / `session.commit` call leaks into a router file, every route decorator is registered, and a one-route-per-prefix live matrix returns 2xx.
|
||
|
||
---
|
||
|
||
## 3. Architecture
|
||
|
||
### 3.1 Target file layout
|
||
|
||
```
|
||
backend/src/cyclone/
|
||
├── api.py ← thin shell: app, lifespan, exception handlers, include_router loop (~250 LOC)
|
||
├── api_helpers.py ← cross-cutting helpers (NDJSON, tail-events) — UNCHANGED
|
||
└── api_routers/
|
||
├── __init__.py ← exports `routers: list[APIRouter]`; auth-router stays in api.py
|
||
├── _shared.py ← 12 cross-router helpers (private to the package)
|
||
├── parse.py ← 5 routes (~800 LOC)
|
||
├── inbox.py ← 6 routes (~470 LOC)
|
||
├── batches.py ← 3 routes + _batch_summary_* helpers (~370 LOC)
|
||
├── claims.py ← 5 routes + _compact_ack_links_for_claim (~720 LOC)
|
||
├── reconciliation.py ← 4 routes (~115 LOC)
|
||
├── remittances.py ← 4 routes (~140 LOC)
|
||
├── dashboard.py ← 1 route (~30 LOC)
|
||
├── providers.py ← 3 routes (~250 LOC)
|
||
├── activity.py ← 2 routes (~150 LOC)
|
||
├── eligibility.py ← 2 routes (~85 LOC)
|
||
├── clearhouse.py ← 3 routes (~250 LOC)
|
||
├── config.py ← 2 routes (~100 LOC)
|
||
├── payers.py ← 1 route (~85 LOC)
|
||
├── admin.py ← existing, absorbs 16 routes (~1,200 LOC total)
|
||
├── acks.py ← existing, absorbs 2 277ca-acks routes
|
||
├── claim_acks.py ← existing, unchanged
|
||
├── ta1_acks.py ← existing, unchanged
|
||
└── health.py ← existing, unchanged
|
||
```
|
||
|
||
### 3.2 Endpoints per router (authoritative)
|
||
|
||
| Router | Method | URL | Source line in api.py |
|
||
|---|---|---|---|
|
||
| parse | POST | `/api/parse-837` | 403 |
|
||
| parse | POST | `/api/parse-835` | 650 |
|
||
| parse | POST | `/api/parse-999` | 822 |
|
||
| parse | POST | `/api/parse-ta1` | 986 |
|
||
| parse | POST | `/api/parse-277ca` | 1124 |
|
||
| acks (absorbed) | GET | `/api/277ca-acks` | 1281 |
|
||
| acks (absorbed) | GET | `/api/277ca-acks/{ack_id}` | 1313 |
|
||
| inbox | GET | `/api/inbox/lanes` | 1355 |
|
||
| inbox | POST | `/api/inbox/candidates/{remit_id}/match` | 1382 |
|
||
| inbox | POST | `/api/inbox/candidates/dismiss` | 1411 |
|
||
| inbox | POST | `/api/inbox/payer-rejected/acknowledge` | 1446 |
|
||
| inbox | POST | `/api/inbox/rejected/resubmit` | 1506 |
|
||
| inbox | GET | `/api/inbox/export.csv` | 1809 |
|
||
| batches | POST | `/api/batches/{batch_id}/export-837` | 1600 |
|
||
| batches | GET | `/api/batches` | 2034 |
|
||
| batches | GET | `/api/batches/{batch_id}` | 2092 |
|
||
| claims | GET | `/api/claims` | 2103 |
|
||
| claims | GET | `/api/claims/stream` | 2152 |
|
||
| claims | GET | `/api/claims/{claim_id}` | 2199 |
|
||
| claims | GET | `/api/claims/{claim_id}/serialize-837` | 2262 |
|
||
| claims | GET | `/api/claims/{claim_id}/line-reconciliation` | 2314 |
|
||
| reconciliation | GET | `/api/reconciliation/unmatched` | 2511 |
|
||
| reconciliation | GET | `/api/batch-diff` | 2528 |
|
||
| reconciliation | POST | `/api/reconciliation/match` | 2575 |
|
||
| reconciliation | POST | `/api/reconciliation/unmatch` | 2620 |
|
||
| remittances | GET | `/api/remittances` | 2652 |
|
||
| remittances | GET | `/api/remittances/summary` | 2693 |
|
||
| remittances | GET | `/api/remittances/stream` | 2724 |
|
||
| remittances | GET | `/api/remittances/{remittance_id}` | 2763 |
|
||
| dashboard | GET | `/api/dashboard/kpis` | 2780 |
|
||
| providers | GET | `/api/providers` | 2808 |
|
||
| providers | GET | `/api/config/providers` | 3361 |
|
||
| providers | GET | `/api/config/providers/{npi}` | 4100 |
|
||
| activity | GET | `/api/activity` | 2838 |
|
||
| activity | GET | `/api/activity/stream` | 2865 |
|
||
| eligibility | POST | `/api/eligibility/request` | 3013 |
|
||
| eligibility | POST | `/api/eligibility/parse-271` | 3039 |
|
||
| clearhouse | GET | `/api/clearhouse` | 3099 |
|
||
| clearhouse | PATCH | `/api/clearhouse` | 3108 |
|
||
| clearhouse | POST | `/api/clearhouse/submit` | 3174 |
|
||
| config | GET | `/api/config/payers` | 4174 |
|
||
| config | GET | `/api/config/payers/{payer_id}/configs` | 4179 |
|
||
| payers | GET | `/api/payers/{payer_id}/summary` | 4230 |
|
||
| admin | GET | `/api/admin/audit-log` | 3372 |
|
||
| admin | GET | `/api/admin/audit-log/verify` | 3414 |
|
||
| admin | POST | `/api/admin/db/rotate-key` | 3454 |
|
||
| admin | POST | `/api/admin/backup/create` | 3614 |
|
||
| admin | GET | `/api/admin/backup/list` | 3668 |
|
||
| admin | GET | `/api/admin/backup/status` | 3697 |
|
||
| admin | POST | `/api/admin/backup/{backup_id}/verify` | 3711 |
|
||
| admin | POST | `/api/admin/backup/{backup_id}/restore/initiate` | 3730 |
|
||
| admin | POST | `/api/admin/backup/{backup_id}/restore/confirm` | 3758 |
|
||
| admin | POST | `/api/admin/backup/prune` | 3810 |
|
||
| admin | POST | `/api/admin/backup/scheduler/start` | 3838 |
|
||
| admin | POST | `/api/admin/backup/scheduler/stop` | 3849 |
|
||
| admin | POST | `/api/admin/backup/scheduler/tick` | 3860 |
|
||
| admin | POST | `/api/admin/scheduler/start` | 3894 |
|
||
| admin | POST | `/api/admin/scheduler/stop` | 3902 |
|
||
| admin | POST | `/api/admin/scheduler/tick` | 3910 |
|
||
| admin | POST | `/api/admin/scheduler/pull-inbound` | 3923 |
|
||
| admin | GET | `/api/admin/scheduler/status` | 4045 |
|
||
| admin | GET | `/api/admin/scheduler/processed-files` | 4052 |
|
||
| admin | POST | `/api/admin/reload-config` | 4316 |
|
||
|
||
### 3.3 `_shared.py` surface (12 helpers)
|
||
|
||
| Helper | Routers that use it | Source line in api.py |
|
||
|---|---|---|
|
||
| `_actor_user_id(request)` | most | 54 |
|
||
| `_resolve_payer(name)` | parse (837, 999, ta1, 277ca) | 333 |
|
||
| `_resolve_payer_835(name)` | parse (835) | 345 |
|
||
| `_transaction_set_id_from_segments(segments)` | parse (999, ta1) | 357 |
|
||
| `_build_and_persist_ack(batch_id)` | parse (after 837) | 573 |
|
||
| `_reconciliation_summary_for_batch(batch_id)` | parse (after 837) | 615 |
|
||
| `_ta1_synthetic_source_batch_id(icn)` | parse (ta1) | 975 |
|
||
| `_serialize_ta1(result)` | acks | 1324 |
|
||
| `_serialize_ta1_from_row(row)` | acks | 1341 |
|
||
| `_batch_summary_claim_count(rec)` | batches | 1858 |
|
||
| `_batch_summary_claim_ids(rec)` | batches | 1867 |
|
||
| `_batch_summary_billing_outcomes(...)` | batches | 1912 |
|
||
|
||
### 3.4 Stays in `api.py`
|
||
|
||
- `app = FastAPI(...)` instantiation
|
||
- `lifespan` (init_db, scheduler, bus, sessions, etc.)
|
||
- `@app.exception_handler(HTTPException)` at 313
|
||
- `@app.exception_handler(Exception)` at 386
|
||
- The trailing `from cyclone.auth.routes import router as auth_router; app.include_router(auth_router)` block
|
||
- The `__all__ = ["app"]` line at the end
|
||
|
||
### 3.5 Stays in `api_helpers.py`
|
||
|
||
- `_ndjson_line`
|
||
- `_tail_events`
|
||
|
||
### 3.6 External import surface — what callers see
|
||
|
||
A grep over the codebase for `from cyclone.api import` and `import cyclone.api` will return the same set of symbols after SP36 lands as before:
|
||
|
||
- `from cyclone.api import app` — used by `python -m cyclone serve`, by tests (`from cyclone import api` then `app.state.*`), by the test `conftest.py`.
|
||
- `app.state.event_bus`, `app.state.db`, `app.state.scheduler` — read by tests and by `api_routers/` (via `Request`).
|
||
|
||
The new `cyclone.api_routers` package exports `routers: list[APIRouter]`. Anything in `api_routers/` that needs to be imported from outside (e.g. a router-level fixture in a future test) is reachable as `from cyclone.api_routers.parse import router`, etc.
|
||
|
||
---
|
||
|
||
## 4. Data flow & error handling
|
||
|
||
**Data flow (per request):**
|
||
|
||
1. FastAPI receives the request, runs the `matrix_gate` dependency (auth + role check), then dispatches to the matching route in the matching router.
|
||
2. The route handler validates inputs (path params, query params, body via Pydantic if a body model exists).
|
||
3. The handler calls `cyclone.store.<method>(...)` for write paths and reconciliation, or opens a `db.SessionLocal()() for one-off reads` (existing pattern; this is how the dashboard KPIs, the activity feed, the configuration endpoints, and the live-tail snapshot reads work today).
|
||
4. The result is serialized via `to_ui_<entity>` (from `cyclone.store.ui`) for entity-shaped responses, or via a small local serializer for non-entity shapes (e.g. the dashboard KPI shape, the reconciliation summary, the eligibility 271 response).
|
||
5. The handler returns `JSONResponse`, `StreamingResponse` (NDJSON), or `Response` (CSV for `/api/inbox/export.csv`).
|
||
6. **Streaming endpoints** (`/api/claims/stream`, `/api/remittances/stream`, `/api/activity/stream`) keep the snapshot-then-live-subscription two-phase pattern. `_ndjson_line` and `_tail_events` come from `api_helpers.py`; the matching router imports them.
|
||
7. **The event bus contract is unchanged.** Every write through `cyclone.store.add(...)` still publishes `claim_written` / `remittance_written` / `activity_recorded` on the in-process `EventBus`. The stream-endpoint routers subscribe to those exact event names. The SP doesn't introduce new event kinds, doesn't rename existing kinds, doesn't change payloads.
|
||
|
||
**Error handling:**
|
||
|
||
- Routers raise `HTTPException` for client errors. Same status codes, same `detail` strings, same `error` envelope (`{error, detail}`). No change.
|
||
- The two global exception handlers stay in `api.py` (D2 of brainstorming): `@app.exception_handler(HTTPException)` re-emits with the cyclone error envelope, `@app.exception_handler(Exception)` logs and returns 500.
|
||
- Streaming endpoints catch domain exceptions and yield `{"type": "error", "data": {"message": ...}}` NDJSON chunks instead of raising — existing pattern, preserved per router.
|
||
- `StoreNotFound` / `StoreConflict` / `StoreInvalidState` are translated to 404 / 409 / 409 by the existing translation path. No change.
|
||
- The auth path (`matrix_gate` dep) returns 401/403 unchanged.
|
||
|
||
**No behavior change anywhere in the data flow or error path.** The SP is a structural move, not a correctness pass.
|
||
|
||
---
|
||
|
||
## 5. Testing approach
|
||
|
||
### 5.1 Per-router cycle (live-test + autoreview + commit)
|
||
|
||
For each router extracted (one `feat(sp36): extract {name} router` commit):
|
||
|
||
1. **Pre-flight pytest baseline.** From the project root:
|
||
```
|
||
cd backend && .venv/bin/pytest --tb=line -q 2>&1 | tee /tmp/refactor-{N}-pre.txt | tail -3
|
||
```
|
||
Capture the `XXX passed, YY failed, ZZ skipped` line. Expect roughly `~1,176 passed, ~1 failed (pre-existing isolation flake), ~10 skipped` per the SP21 store-split spec baseline.
|
||
2. **Cut.** Move the route handler(s) + their single-router helpers to the new router module, register the router in `api_routers/__init__.py`, delete the moved code from `api.py`, update imports. No logic change.
|
||
3. **Post-cut pytest diff.** Same command as (1), output to `/tmp/refactor-{N}-post.txt`. Diff:
|
||
```
|
||
diff <(awk '/passed|failed|skipped/ {print}' /tmp/refactor-{N}-pre.txt) \
|
||
<(awk '/passed|failed|skipped/ {print}' /tmp/refactor-{N}-post.txt)
|
||
```
|
||
Required: zero diff. Any delta = revert the cut and investigate.
|
||
4. **Live test.** Server is running in `cyclone-backend-1` on the host. Hit one representative route from the moved router. Required status codes:
|
||
- GET endpoints → 200 (or the documented non-200, e.g. 401 if auth is mocked off in the test environment)
|
||
- POST endpoints → 200, 201, 400, 401, 403, 409 (whatever the existing happy path / known-error path is)
|
||
- Streaming endpoints → 200 with `Content-Type: application/x-ndjson` and a valid first `snapshot_end` chunk
|
||
5. **Autoreview.** Spawn a `pr-reviewer` subagent on the staged diff. The reviewer prompt enumerates: (a) the new router is registered in `api_routers/__init__.py`; (b) no orphan imports left in `api.py`; (c) no `session.add` / `s.add` / `session.commit` / direct `db.SessionLocal()() call that does a write` appears in any router file (reads are OK); (d) the router's `dependencies=[Depends(matrix_gate)]` matches the original; (e) docstrings are preserved. Reviewer returns a one-line verdict (PASS / FAIL: <reason>). On FAIL, fix and re-review.
|
||
6. **Commit.** `feat(sp36): extract {name} router` per the SP-N convention.
|
||
7. **Log** to `/tmp/refactor-cyclone.md`:
|
||
```
|
||
[step N] extracted {name}
|
||
pre: passed={X} failed={Y} skipped={Z}
|
||
post: passed={X} failed={Y} skipped={Z}
|
||
live: GET /api/{route} → {code} | POST /api/{route} → {code}
|
||
reviewer: PASS | FAIL: <reason>
|
||
```
|
||
|
||
### 5.2 Pre-merge integration test
|
||
|
||
After all 19 routers are extracted, the full suite runs:
|
||
|
||
- `cd backend && .venv/bin/pytest --tb=line -q` — full backend suite, must match Step 0 baseline to the digit
|
||
- `cd backend && .venv/bin/pytest tests/test_api_*.py -v` — every API integration test, all green
|
||
- `cd .. && npm test` — frontend suite, all green
|
||
- `cd .. && npm run typecheck` — frontend typecheck clean
|
||
- `cd .. && npm run build` — frontend build green
|
||
- `cd .. && npm run lint` — frontend lint clean
|
||
|
||
### 5.3 Pre-merge one-shot verification (D8 of brainstorming)
|
||
|
||
These commands run, and all must pass, before the merge commit is created:
|
||
|
||
- `wc -l backend/src/cyclone/api.py` — expect ≤ 300
|
||
- `wc -l backend/src/cyclone/api_routers/*.py | sort -n` — expect no router over 1,400 LOC (admin is the largest at ~1,200)
|
||
- `grep -rn "session.add\|s\.add(\|session\.commit" backend/src/cyclone/api_routers/` — expect zero hits (writes go through `cyclone.store` only)
|
||
- `grep -rn "from cyclone.api_routers" backend/src/cyclone/api.py` — expect one hit (the `routers` import in `__init__` block)
|
||
- `python -c "from cyclone.api_routers import routers; print(len(routers))"` — expect ≥ 18 (5 existing + 14 new, minus whatever routers ended up merged during the SP)
|
||
- Live matrix — for every URL prefix in Section 3.2, `curl -s -o /dev/null -w "%{http_code}\n" http://192.168.0.49:8080/api/{prefix}/...` returns 2xx (or the documented error code)
|
||
- `git grep -n "@app\." backend/src/cyclone/api.py` — expect zero hits (every route decorator now lives in a router)
|
||
|
||
### 5.4 No new tests
|
||
|
||
The existing test suite is the regression net. We do not add new tests, do not modify existing tests, do not add new fixtures. The pre/post pytest baseline diff is the test for the SP.
|
||
|
||
---
|
||
|
||
## 6. Threat model (post-SP24 alignment)
|
||
|
||
The auth boundary is the HTTP layer: login required, bcrypt + HttpOnly session cookie. The new routers mount under `matrix_gate` (same as today), so every moved route stays auth-gated. No route is exposed that wasn't exposed before. No auth check is weakened.
|
||
|
||
The file-system threat model is unchanged: SQLCipher at rest (when the macOS Keychain entry + `sqlcipher3` are both present, otherwise plain SQLite), secrets in the macOS Keychain via `keyring`, no secrets on disk in plaintext. SP36 doesn't touch the DB layer, the secrets module, or the configuration loader.
|
||
|
||
LAN-only by design. The auth boundary is the network boundary; the production deployment is the host firewall + compose port publishing. Don't expose the published ports to the WAN — unchanged.
|
||
|
||
---
|
||
|
||
## 7. Risks & mitigations
|
||
|
||
| Risk | Likelihood | Impact | Mitigation |
|
||
|---|---|---|---|
|
||
| A route handler silently changes behavior during the cut (typo, off-by-one, wrong import) | medium | high | Pre/post pytest diff is the test. The diff must be zero; any delta reverts. |
|
||
| A helper gets duplicated instead of moved (one copy in old location, one in new) | low | medium | Autoreview explicitly checks for orphan imports in `api.py` and for `def _helper_name` definitions appearing twice. |
|
||
| The `api_routers/__init__.py` registration misses a router and a route 404s in production | low | high | Pre-merge one-shot verification §5.3 includes a live matrix that hits every URL prefix. |
|
||
| The `matrix_gate` dep is dropped from one route during the cut | low | critical | Autoreview explicitly checks `dependencies=[Depends(matrix_gate)]` matches the original on every moved route. |
|
||
| Two routers end up importing each other (creates a cycle) | low | medium | The convention is "routers import only `cyclone.store`, `cyclone.api_helpers`, `cyclone.api_routers._shared`" — autoreview checks for any other cross-router import. |
|
||
| A `from cyclone.api import` somewhere in the codebase breaks because we removed a top-level symbol | low | high | The external import surface (D6 of brainstorming) is the test: `git grep "from cyclone.api import"` before and after must be identical in symbol set. |
|
||
| The merge of SP35 into main fails or introduces conflicts | low | high | D7 of brainstorming: SP35 merge is a hard pre-condition; if it doesn't land cleanly, SP36 doesn't start. |
|
||
| A streaming endpoint regresses silently (returns the right status code but the wrong first chunk) | medium | medium | The live test for streaming endpoints asserts `Content-Type: application/x-ndjson` and parses the first `snapshot_end` line, not just the HTTP code. |
|
||
| The frontend test suite picks up a backend change (e.g. a renamed JSON field that the UI consumes) | very low | medium | The refactor is byte-for-byte transparent to clients; if a frontend test fails, that's a real regression and we investigate. |
|
||
|
||
---
|
||
|
||
## 8. Rollout
|
||
|
||
1. **Merge SP35 into main.** Fast-forward or merge commit (no squash, no rebase). The branch `sp35-parse-input-guards` has 4 reviewable commits and a clean working tree.
|
||
2. **Restart compose if needed.** `docker compose restart cyclone-backend-1` after the SP35 merge so the running container reflects main.
|
||
3. **Create the SP36 branch.** `git checkout -b sp36-api-routers-split` from main.
|
||
4. **Extract routers, one per commit.** Order: lowest-risk first (admin, health, claim_acks, ta1_acks already exist; then the small leaf routers `dashboard.py`, `eligibility.py`, `payers.py`, `config.py`; then the medium ones `reconciliation.py`, `remittances.py`, `activity.py`, `clearhouse.py`, `providers.py`; then the larger ones `inbox.py`, `batches.py`, `claims.py`; finally `parse.py` since it's the most coupled to the store facade). Each step follows §5.1.
|
||
5. **Run the §5.2 integration tests.**
|
||
6. **Run the §5.3 one-shot verification.**
|
||
7. **Open the PR.** Title: `SP36 API Routers Split`. Body: links to this spec, the SP36 plan (when written), and a checklist of every router extracted.
|
||
8. **Merge after review.** Single atomic `merge: SP36 api-routers-split into main` commit, no squash, no rebase.
|
||
|
||
---
|
||
|
||
## 9. Open questions
|
||
|
||
None at spec time. The brainstorming Q&A resolved all of:
|
||
|
||
- Which file to split? `api.py`. Confirmed.
|
||
- What shape? Per-resource routers mirroring the store's domain boundaries. Confirmed.
|
||
- Branch from where? Merge SP35 first, then from main. Confirmed.
|
||
- Where do the cross-router helpers go? `api_routers/_shared.py` (private). Confirmed.
|
||
- `admin.py` — one router or split further? One router for this SP, follow-up later. Confirmed.
|
||
- How granular are the commits? One router per commit. Confirmed.
|