feat(sp-skill-catalog): add cyclone-api-router skill (FastAPI conventions)
This commit is contained in:
@@ -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/<topic>.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/<topic>.py` module and need the import /
|
||||
mounting rules (`from cyclone.api_routers import <topic>` then
|
||||
`app.include_router(<topic>.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/<resource>` and its
|
||||
`/{id}` detail) lives in `backend/src/cyclone/api_routers/<topic>.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 `<entity>_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 `<entity>_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": "<Title>", "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.
|
||||
Reference in New Issue
Block a user