peter
6d8b7be6d8
The demo account — one shared user that whitelisted emails entered from the login page, reset on a schedule — goes end to end (owner decision, E34): demo.py and the reset job, users.is_demo and demo_whitelist (migration 0098 deletes the demo user, its audit rows, the reset job's override and notices, then drops the column and table; the downgrade restores the empty schema), the /auth/demo entry, the admin whitelist and reset, the Demo tab, the banner, the login page's "Try the demo", and every is_demo branch in routes, userscope, scheduler and the UI. require_human only ever refused the demo account, so its 87 uses become current_user (owner-approved); require_write_scope keeps its own check. The login page's "or" divider now shows only beside the Google button: without the demo entry under it, an instance without Google would have said "or" to nothing. A contract test (removedFeatures) fails on the word anywhere in the backend, frontend, e2e or scripts — seen red on a planted hit. Size ceilings tightened for the modules that shrank; admin.py is back under the limit.
81 lines
5.3 KiB
Markdown
81 lines
5.3 KiB
Markdown
# Backend conventions
|
||
|
||
The rules below are already followed across `backend/app` — they lived only in a few module
|
||
docstrings (`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/`, `issues/` — 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;
|
||
`issues/bundle.py` is the one definition of a bug report's files, for the zip export and the tracker.
|
||
|
||
3. **Flat `app/*.py` core modules — cross-cutting primitives.** `auth`, `sysconfig`, `quota`, `audit`,
|
||
`notifications`, `userscope`, `ratelimit`, `progress`, `logs` (the one logging
|
||
configuration), `logsink` (its `log_events` table), `diagnostics` (a user's consented log
|
||
level), `debugsession` (a recording that tags its user's lines, E16) and `collation` (A→Z in the
|
||
reader's alphabet, E30), … 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
|
||
|
||
This follows CLAUDE.md's canonical rule verbatim — **"the handler/job-wrapper commits; helpers flush
|
||
(only `quota` owns its session)"** — so read it that way, not as an exception to it. The commit belongs
|
||
to the **operation owner**; anything called *inside* another transaction only stages its change
|
||
(`db.add` / `flush`) and lets the owner commit, 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).
|
||
|
||
- The **operation owner** is usually the route handler. A `service.*` operation the handler delegates a
|
||
whole transition to (e.g. `service.cancel`) *is* that owner and commits its one transition — it isn't
|
||
a mid-operation helper. A helper that owns its OWN short-lived session (the worker's `jobdb` writes,
|
||
the same carve-out CLAUDE.md grants `quota`) commits that session. Everything else flushes.
|
||
- `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` + `basedpyright` (backend) · the lock check + `tsc` ×3 + `eslint` + `prettier` + `knip` +
|
||
`vite build` (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; a missing database is a hard failure, not a silent skip, unless a hand run asks
|
||
for pure-logic only with `SIFTLODE_TEST_REQUIRE_DB=0`. See `docs/development.md`, and `docs/gates.md`
|
||
for what each gate deliberately leaves out.
|