From 5e7e9e2e686fcee4e4c538e0959d59e839c4a307 Mon Sep 17 00:00:00 2001 From: Tyler Date: Sun, 21 Jun 2026 12:31:28 -0600 Subject: [PATCH] feat(sp-skill-catalog): add cyclone-api-router skill (FastAPI conventions) --- .../skills/cyclone-api-router/SKILL.md | 189 ++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 .superpowers/skills/cyclone-api-router/SKILL.md diff --git a/.superpowers/skills/cyclone-api-router/SKILL.md b/.superpowers/skills/cyclone-api-router/SKILL.md new file mode 100644 index 0000000..6447533 --- /dev/null +++ b/.superpowers/skills/cyclone-api-router/SKILL.md @@ -0,0 +1,189 @@ +--- +name: cyclone-api-router +description: "Cyclone FastAPI router conventions (api_routers/, api_helpers.py, response shapes, error envelopes). Use when: adding or changing an HTTP endpoint, splitting a route out of api.py, or wiring a new helper into api_helpers.py." +--- + +# cyclone-api-router + +Cyclone splits its FastAPI surface two ways: small resource-group +routers live in `backend/src/cyclone/api_routers/.py` and are +mounted bare into `api.py`; the high-traffic streaming and parse +endpoints still live as top-level decorators in `api.py` itself. This +skill codifies the conventions so additions stay consistent with the +four routers already shipped (`acks`, `admin`, `health`, `ta1_acks`). + +As of this writing: **4 router modules** under +`backend/src/cyclone/api_routers/`, **one shared helpers module** at +`backend/src/cyclone/api_helpers.py` (248 lines, NDJSON primitives + +content negotiation + `tail_events`), and ~30 routes still inlined +in `backend/src/cyclone/api.py`. The next refactor target is the +parse endpoints. + +## When to use + +- **Adding an endpoint.** You're adding a new GET / POST handler — + you need to know whether it belongs in `api.py` (parse / streaming) + or in a new router under `api_routers/`, and what the response + shape and test file conventions look like. +- **Splitting a route.** You're moving a route out of `api.py` into a + dedicated `api_routers/.py` module and need the import / + mounting rules (`from cyclone.api_routers import ` then + `app.include_router(.router)`). +- **Adding a helper.** You're wiring a new function into + `api_helpers.py` (NDJSON primitive, content-negotiation probe, + tail-event helper) and need to keep it private to the API layer. +- **Defining an error response.** You're raising from a route handler + and need the conventional `HTTPException(status_code=..., detail=...)` + shape used everywhere else in the API surface. + +## Conventions + +1. **No new top-level routes in `api.py` for resource groups.** Any + endpoint grouped under a resource (`/api/` and its + `/{id}` detail) lives in `backend/src/cyclone/api_routers/.py` + as an `APIRouter`. The streaming list endpoints (`/api/claims/stream`, + `/api/remittances/stream`, `/api/activity/stream`) and the parse + endpoints (`/api/parse-*`) currently stay in `api.py` because they + span multiple store modules — don't move them unless you're also + restructuring the store split. +2. **Reuse `api_helpers.py`.** NDJSON primitives (`ndjson_line`, + `ndjson_stream_list`, `ndjson_stream_837`, `ndjson_stream_835`), + content negotiation (`client_wants_json`, `wants_ndjson`), the + strict / `raw_segments` rewrites (`strict_rewrite_837`, + `strict_rewrite_835`, `drop_raw_segments_837`, `drop_raw_segments_835`), + and the shared live-tail generator (`tail_events`, + `heartbeat_seconds`) all live there. Don't duplicate them in a + router. The module's docstring (`api_helpers.py:1-18`) declares it + private to the API layer — no business logic, no DB writes. +3. **Response shape.** Every successful response is a **plain dict** + produced by a per-router `_to_ui(row)` helper (see + `_ack_to_ui` at `api_routers/acks.py:29-48` and `_ta1_to_ui` at + `api_routers/ta1_acks.py:23-37`). This dict shape **must** match + the matching `_written` event payload so live-tail pages + don't drift (see `cyclone-store` for the serializer contract). + Errors use FastAPI's `HTTPException` with a `detail` dict of the + form `{"error": "", "detail": "<message>"}` — + `acks.py:85-88`, `ta1_acks.py:72`. There is **no** shared + `ErrorEnvelope` Pydantic model; the `detail` dict is the contract. +4. **Mounting.** Routers are mounted bare in `api.py:251-256` — + `app.include_router(<name>.router)` with **no** `prefix=` + argument. Each `@router.<verb>` decorator carries the **full** + `/api/<resource>` path itself (see `acks.py:51,75`, + `ta1_acks.py:49,67`, `admin.py:23`, `health.py:28`). The + `router = APIRouter()` declaration carries no `tags=` either — + keep it minimal. +5. **Streaming endpoints.** Use + `StreamingResponse(media_type="application/x-ndjson")` and feed it + either `ndjson_stream_list(items, total, returned, has_more)` (for + list pages) or `tail_events(request, bus, kinds)` (for live-tail + pages). See `acks.py:62-66` for the list-stream skeleton and + `api.py:1357-1401` for the full live-tail pattern (snapshot → + `snapshot_end` → subscription → heartbeats). See `cyclone-tail` + for the wire format. +6. **Tests.** Every new endpoint gets a `test_api_<topic>_<verb>.py` + under `backend/tests/` (see `cyclone-tests` for the naming + convention + autouse `conftest.py`). Existing examples: + `test_api_validate_provider.py` (admin), + `test_api_parse_persists_ack.py` (acks). + +## Patterns + +### A new `APIRouter` skeleton + +Pattern from `backend/src/cyclone/api_routers/acks.py:1-72`. Module +docstring names the resource, imports the shared helpers, declares +`router = APIRouter()` with no prefix, and defines a `_foo_to_ui(row)` +mapper at module scope. + +```python +"""``/api/foo`` — list & detail endpoints for <topic>.""" +from __future__ import annotations + +from fastapi import APIRouter, HTTPException, Query, Request +from fastapi.responses import StreamingResponse + +from cyclone.api_helpers import ndjson_stream_list, wants_ndjson +from cyclone.store import store + +router = APIRouter() + + +def _foo_to_ui(row) -> dict: + """Map a Foo ORM row to the UI shape used by ``/api/foo``.""" + return {"id": row.id, "name": row.name} + + +@router.get("/api/foo") +def list_foo( + request: Request, + limit: int = Query(100, ge=1, le=1000), +): + """Return the list of persisted Foo rows, newest first.""" + rows = store.list_foo() + items = [_foo_to_ui(r) for r in rows[:limit]] + total = len(rows) + returned = len(items) + has_more = total > returned + if wants_ndjson(request): + return StreamingResponse( + ndjson_stream_list(items, total, returned, has_more), + media_type="application/x-ndjson", + ) + return {"items": items, "total": total, "returned": returned, "has_more": has_more} +``` + +### A `get_<topic>` detail endpoint with 404 + +Pattern from `api_routers/acks.py:75-104` and `ta1_acks.py:67-76`. +Path param is `<entity>_id` (not `id`) so it doesn't shadow +FastAPI's internal `id` and the OpenAPI docs stay self-describing. + +```python +@router.get("/api/foo/{foo_id}") +def get_foo(foo_id: int) -> dict: + """Return one persisted Foo row with its parsed detail. + + Path param is ``foo_id`` (not ``id``) to avoid shadowing + FastAPI's internal ``id`` name and to keep OpenAPI docs + self-describing. Returns 404 when the row is missing — never 500. + """ + row = store.get_foo(foo_id) + if row is None: + raise HTTPException( + status_code=404, + detail={"error": "Not found", "detail": f"Foo {foo_id} not found"}, + ) + return _foo_to_ui(row) +``` + +### Mounting in `api.py` + +Pattern from `backend/src/cyclone/api.py:246-256`. The block lives +just after middleware registration and just before the first +`@app.<verb>` decorator. + +```python +# Resource-group routers. Each module owns its own APIRouter and is +# registered below. New resources go in `cyclone.api_routers.<name>` +# and are wired in here. +from cyclone.api_routers import acks, admin, health, ta1_acks # noqa: E402 + +app.include_router(health.router) +app.include_router(acks.router) +app.include_router(ta1_acks.router) +app.include_router(admin.router) +``` + +## Anti-patterns + +- **Don't import from `cyclone.api` into a router — the dependency runs the other way.** Routers are mounted *into* `api.py` (`api_routers/acks.py` etc. know nothing about `cyclone.api`). A circular import would silently break the `from cyclone.api_routers import ...` block at `api.py:251`. +- **Don't introduce Pydantic response models where the codebase returns dicts.** Every existing list / detail endpoint returns a plain dict produced by a `<entity>_to_ui(row)` helper (see `acks.py:29-48`, `ta1_acks.py:23-37`). That dict shape **is** the event payload that `<entity>_written` carries — see `cyclone-store`. Introducing a Pydantic model on one side drifts the event payload from the list shape and silently breaks live-tail dedup. +- **Don't bypass `CycloneStore` to query the ORM directly from a route.** Always call `store.<method>(...)` (`store.list_acks()`, `store.get_ta1_ack(ack_id)`) so the read path picks up the same session + snapshot serializer as the live-tail subscriber. A raw `with db.SessionLocal()() as s: s.get(Foo, foo_id)` in a handler bypasses the serializer contract and breaks the event-payload match. See `cyclone-store`. +- **Don't set `prefix=` on `APIRouter`.** Mount the router bare (`app.include_router(<name>.router)`) and put the full `/api/<resource>` path in the decorator. Mixing the two styles scatters the URL across two files and breaks `grep "/api/foo"` audits. + +## Related skills + +- **`cyclone-store`** — most routes call `store.<method>(...)` and the dict payload **is** the `<entity>_written` event payload; load when adding a route so the read path stays aligned with the pubsub contract. +- **`cyclone-tail`** — streaming endpoints (`/api/<resource>/stream`) and the NDJSON wire format; load when adding a live-tail route or changing the wire format. +- **`cyclone-edi`** — parse endpoints (`/api/parse-837`, `/api/parse-835`, `/api/parse-999`, `/api/parse-ta1`, `/api/parse-277ca`) currently live in `api.py`; load when adding or changing a parse endpoint. +- **`cyclone-tests`** — endpoint tests follow the `test_api_<topic>_<verb>.py` naming under `backend/tests/`; load when writing the test for a new endpoint. \ No newline at end of file