docs: write the backend conventions + a development guide (E3 S3, B-core-20)
The layering rule ("request-only logic in routes/; scheduler+worker-shared logic
in the plex|downloads|sync packages; cross-cutting primitives as flat core
modules; extract at the SECOND consumer") was real but unwritten — it lived only
in three module docstrings and a one-line README mention. Collect it, plus the
transaction-ownership rule (a mid-operation helper stages, the caller commits),
the atomic-counter / JobStatus-StrEnum conventions, and the one-home-per-config
rule, into backend/app/README.md. Add docs/development.md (run locally + the gate
commands + branching), so a new session/contributor has one place to look instead
of copying whichever pattern they happen to find first.
This commit is contained in:
@@ -0,0 +1,73 @@
|
|||||||
|
# Backend conventions
|
||||||
|
|
||||||
|
The rules below are already followed across `backend/app` — they lived only in a few module
|
||||||
|
docstrings (`demo.py`, `userscope.py`, `routes/_common.py`) until this file collected them (E3 S3,
|
||||||
|
B-core-20). When you add code, match these so the next reader doesn't have to guess "which pattern do
|
||||||
|
I copy".
|
||||||
|
|
||||||
|
## Module layering
|
||||||
|
|
||||||
|
Three layers, by *who* runs the code — not by feature:
|
||||||
|
|
||||||
|
1. **`routes/…` — request-only.** A route module holds HTTP concerns: auth dependencies, request/
|
||||||
|
response models, status-code mapping, serialization. It owns logic that runs *only* on a request.
|
||||||
|
Business logic that a request happens to trigger does **not** live here if anything else runs it too.
|
||||||
|
|
||||||
|
2. **`plex/`, `downloads/`, `sync/` — domain logic shared by more than one caller.** Anything the
|
||||||
|
**scheduler or the worker** runs (not just a request) lives in one of these domain packages, so the
|
||||||
|
route, the worker, and the scheduler share one definition. Examples from this codebase:
|
||||||
|
`downloads/service.py` owns enqueue + the job state transitions (routes call it); `plex/sync.py` is
|
||||||
|
run by both the admin route and the scheduler; `downloads/pipeline.py` is the worker's child pipeline.
|
||||||
|
|
||||||
|
3. **Flat `app/*.py` core modules — cross-cutting primitives.** `auth`, `sysconfig`, `quota`, `audit`,
|
||||||
|
`notifications`, `userscope`, `realtime`, `ratelimit`, `progress`, … are single-file primitives many
|
||||||
|
layers import. They depend on nothing above them (a core module must never import a `routes/…`
|
||||||
|
module).
|
||||||
|
|
||||||
|
**Extraction trigger: the SECOND consumer.** Keep new logic inline in its first caller. The moment a
|
||||||
|
*second* caller needs it (a worker as well as a route; a scheduler as well as a route), extract it down
|
||||||
|
a layer — into the relevant domain package, or a flat core module if it's cross-cutting. Don't
|
||||||
|
pre-abstract for a single caller; don't leave shared logic duplicated once there are two.
|
||||||
|
|
||||||
|
## Transaction ownership
|
||||||
|
|
||||||
|
**A helper that mutates rows as part of a larger operation only `db.add()`s / stages its change; the
|
||||||
|
CALLER commits, so the whole operation is one transaction.** `audit.record()` is the reference: it adds
|
||||||
|
the audit row and the caller commits it together with the mutation it records, so an audit entry can
|
||||||
|
never exist without its change (or vice-versa).
|
||||||
|
|
||||||
|
- A self-contained transition that owns its whole unit of work MAY commit (e.g. `service.cancel/…`
|
||||||
|
each commit their one transition; the worker's short-lived `jobdb` helpers each commit their own
|
||||||
|
write). But a helper called *in the middle* of a caller's transaction must not commit — that splits
|
||||||
|
the caller's atomic operation in two.
|
||||||
|
- `SessionLocal` is `expire_on_commit=False`: after a raw `UPDATE` + commit, an ORM object you already
|
||||||
|
loaded is **stale**. Read the fresh value from the row (RETURNING, or a re-`SELECT`), not the ORM
|
||||||
|
attribute — see `service.decr_ref_count`.
|
||||||
|
|
||||||
|
## Shared counters and status: do it atomically at the DB
|
||||||
|
|
||||||
|
- **Never read-modify-write a shared counter in Python** (`x.count = x.count + 1`). Concurrent sessions
|
||||||
|
lose an update. Use a DB-atomic statement — `col = col + 1`, or `greatest(col - 1, 0)` with
|
||||||
|
`RETURNING` so the caller branches on the post-value under the row lock (`service.incr_ref_count` /
|
||||||
|
`decr_ref_count`).
|
||||||
|
- **Download job status is a `JobStatus` StrEnum** (`downloads/status.py`), not bare strings. The
|
||||||
|
`HOLDING` set defines which states hold a `MediaAsset` (so release accounting stays correct);
|
||||||
|
`STOPPED_BY_ROUTE` is the route-initiated stop set. A worker child's terminal write goes through
|
||||||
|
`service.finalize_if_running` (an atomic `UPDATE … WHERE status='running'`) so a finishing child can't
|
||||||
|
resurrect a job a route just stopped.
|
||||||
|
|
||||||
|
## Config: one home per kind
|
||||||
|
|
||||||
|
- **`app.config.settings`** — boot/env config (DB URL, download root, worker concurrency): read once at
|
||||||
|
startup, not user-tunable at runtime.
|
||||||
|
- **`sysconfig.SPECS`** — the runtime admin knobs (DB-override-or-default). A new admin-tunable value
|
||||||
|
goes in `SPECS`; don't invent a fourth store. (`state.py`'s `maintenance_batch` predates this and is
|
||||||
|
slated to fold into `sysconfig` — do not add more like it.)
|
||||||
|
|
||||||
|
## The gate
|
||||||
|
|
||||||
|
Everything below must be green before a change is done (run via `siftlode check`):
|
||||||
|
`ruff` (backend) · `tsc` ×3 + `eslint` + `prettier` + `knip` (frontend) · `vitest` · `pytest`. The
|
||||||
|
pytest lane runs in `backend/docker-compose.test.yml` — an ephemeral Postgres + the backend tree
|
||||||
|
bind-mounted — so the DB-backed integration tests (the `db` fixture) actually run; `SIFTLODE_TEST_
|
||||||
|
REQUIRE_DB=1` there turns a would-be silent skip into a hard failure. See `docs/development.md`.
|
||||||
@@ -0,0 +1,61 @@
|
|||||||
|
# Development
|
||||||
|
|
||||||
|
How to run Siftlode locally and pass the gate. Conventions for the backend code itself live in
|
||||||
|
[`backend/app/README.md`](../backend/app/README.md).
|
||||||
|
|
||||||
|
## Run it locally
|
||||||
|
|
||||||
|
The whole stack (Postgres + API + worker) comes up from the localdev compose file:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose -f docker-compose.localdev.yml up --build
|
||||||
|
```
|
||||||
|
|
||||||
|
Browse at **http://localhost:8080** (Google login works — the OAuth redirect is
|
||||||
|
`http://localhost:8080/auth/callback`). The API runs `alembic upgrade head` on startup, so the schema
|
||||||
|
is always current.
|
||||||
|
|
||||||
|
For a fast frontend edit loop, run the Vite dev server with HMR against that API:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd frontend && npm install && npm run dev # http://localhost:5173, proxying the API on :8080
|
||||||
|
```
|
||||||
|
|
||||||
|
## The gate
|
||||||
|
|
||||||
|
A change is done when all of this is green. CI runs the same checks server-side
|
||||||
|
(`.gitea/workflows/check.yml`), so "main is always green".
|
||||||
|
|
||||||
|
**Backend**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
# lint
|
||||||
|
python -m ruff check --config backend/ruff.toml backend scripts
|
||||||
|
|
||||||
|
# tests — pure-logic + the DB-backed integration lane, against an ephemeral Postgres.
|
||||||
|
# The backend tree is bind-mounted, so it always tests your working files.
|
||||||
|
docker compose -f backend/docker-compose.test.yml up --build --abort-on-container-exit --exit-code-from test
|
||||||
|
docker compose -f backend/docker-compose.test.yml down -v
|
||||||
|
```
|
||||||
|
|
||||||
|
The test compose sets `SIFTLODE_TEST_REQUIRE_DB=1`, which turns a would-be silent skip (no DB, or no
|
||||||
|
ffmpeg) into a hard failure — so the gate can't print green while the DB-backed tests were quietly not
|
||||||
|
run. Tests that need a database take the `db` fixture (`backend/tests/conftest.py`); the rest are
|
||||||
|
pure-logic and run without one.
|
||||||
|
|
||||||
|
**Frontend** (from `frontend/`)
|
||||||
|
|
||||||
|
```bash
|
||||||
|
npm run typecheck # tsc x3 (app / node / e2e)
|
||||||
|
npm run lint # eslint
|
||||||
|
npm run format:check # prettier
|
||||||
|
npm run knip # unused exports/deps
|
||||||
|
npm run test # vitest
|
||||||
|
```
|
||||||
|
|
||||||
|
## Branching
|
||||||
|
|
||||||
|
Trunk-based: branch off `main`, keep the branch gate-green, and land the whole change back to `main`
|
||||||
|
in one reviewed step. Migrations are Alembic revisions under `backend/alembic/versions`; the API
|
||||||
|
applies them on startup, and the DB-lane rebuilds the schema via `alembic upgrade head`, so a broken
|
||||||
|
migration chain fails the tests.
|
||||||
Reference in New Issue
Block a user