peter
182ec31120
The register test detected narrowings by modelling config shapes, and each review round found one more shape it did not model (a basePath global ignore, a rule src never declares, a widened glob filed under a narrow row, a substring check on a CI line, knip.json beside knip.jsonc). It now compares instead: gates.snapshot.json holds ESLint's effective config for every code file and the lines that run each lane, and any difference fails until it is regenerated. The register is read off the snapshot's configs, which surfaced the ui/ and lib/ test scopes the sampling never reached. The complexity ratchet also names a wrapped function by its binding (const onSave = useCallback(() => ...)), not as "useCallback callback".
135 lines
21 KiB
Markdown
135 lines
21 KiB
Markdown
# CLAUDE.md — working agreement for AI agents on Siftlode
|
|
|
|
This repository is **fully AI-authored** — every line was written by an AI agent, and it continues to
|
|
be developed that way. This file is the rules of the road: read it before you touch anything. It is
|
|
loaded into every agent session, so keep it **short and true**. A rule here that has gone stale is
|
|
worse than no rule — if you change how the code works, change this file in the same commit.
|
|
|
|
Siftlode is a self-hosted YouTube reader (+ a local Plex browser/player and a download center). It is a
|
|
**modular monolith**: one Postgres, one FastAPI process (uvicorn + APScheduler), one worker process
|
|
(yt-dlp/ffmpeg), one PO-token sidecar. React 19 (with the React Compiler) + react-query frontend, with a
|
|
router-free navigation provider.
|
|
|
|
## The gate — never merge or ship red
|
|
|
|
The local gate is **ruff (`check` + `format --check`) + basedpyright (backend) + the lock check + typecheck (3 tsc projects) +
|
|
eslint + prettier --check + knip (both modes) + vitest + vite build + pytest**, then a coverage report (no threshold). It runs
|
|
before every `release` merges and on every `publish`, which **refuses to build on failure — there is deliberately no `--skip`
|
|
flag**; a release also requires the Playwright suite, and the image must pass a Trivy scan before it is pushed. Run it and get it
|
|
green **before** any merge, release, or handing work to review.
|
|
|
|
- Full gate (agents with the private ops dispatcher): `siftlode check`.
|
|
- Directly, without the dispatcher: the commands in [`docs/development.md`](docs/development.md) § The gate (frontend lanes, `ruff` over `backend scripts`, basedpyright in the test image, and `pytest` — the DB-lane needs Postgres).
|
|
- **A gate with any real hit is never switched off or narrowed** — not a lint rule, a compiler flag, a lane or a release step. Fix the sites, or suppress each one inline naming its invariant (`-- why`, lint-enforced). What stays narrowed is in [`docs/gates.md`](docs/gates.md), and a contract test fails when a narrowing is missing from it. The effective ESLint config of every file and the CI/lane lines are pinned in `frontend/src/lib/gates.snapshot.json`: a new code file or any gate change fails until you regenerate it (`GATE_SNAPSHOT=update`, see gates.md) — then read that diff as a gate change. A comment saying "X is enforced by Y" is checked by running Y (E32: one such comment hid 95 functions the React Compiler skipped).
|
|
- **The backend type lane runs in the test image, not on the host** — `requirements.lock` cannot be installed on Windows (uvloop), and a type checker without the real packages reports nonsense. It fails on NEW errors only; the pre-existing ones sit in `backend/.basedpyright/baseline.json` and burn down epic by epic. Fixed some? Regenerate with `--writebaseline`.
|
|
- New DB-backed test? It must actually run, not silently skip — assume `SIFTLODE_TEST_REQUIRE_DB=1`.
|
|
|
|
## Workflow
|
|
|
|
- **Trunk model.** One long-lived branch, `main`. Branch off it (`feature/`, `bug/`, `improvement/`),
|
|
keep commits logical and Conventional-Commit-styled.
|
|
- **No AI attribution in commits.** No `Co-Authored-By` trailer, no "generated by" lines. (This file
|
|
being AI-written is the intended, visible exception; commits are not.)
|
|
- **Release: user-owned in timing, agent-run in execution.** The user decides _when_ to ship; on their
|
|
go-ahead _you_ run `siftlode release` from the feature branch (merge to `main` → publish → deploy →
|
|
verify `/api/version`). Never self-ship — bring work to a green, committed state and stop for the
|
|
go-ahead first, then run it yourself.
|
|
- **Public repo.** Keep ops/topology/secrets out — those live in a separate private repo. Nothing here
|
|
should assume or leak them.
|
|
|
|
## Collaboration — how to work with the human
|
|
|
|
The human is the product owner: they bring ideas and requirements. You are the technical lead — you own
|
|
how it gets built. Concretely:
|
|
|
|
- **Plan before you build.** For anything non-trivial, present a short plan (sprint-level for larger
|
|
work) and get a go-ahead before writing code.
|
|
- **Offer choices, mark the optimal — and explain the trade-off.** When several solutions are viable,
|
|
list them and mark the recommended one. This is the _one_ place to spend extra words: give the _why_
|
|
behind each option and the weight of the decision. These decision points are where the product owner
|
|
engages with the build and learns from it, so make them understandable — a reasoned menu, not a bare one.
|
|
- **Mockups for design/UX decisions.** For any visual or interaction choice, show mockup options to
|
|
choose from before building. How the UI looks and behaves is decided in
|
|
[`docs/DESIGN.md`](docs/DESIGN.md) (its variants render in the dev-only `/design-lab`): build to it,
|
|
and change it only through its decision log.
|
|
- **Challenge unsound requests.** If a request is technically ill-advised and you can back it with real
|
|
arguments, push back or propose an alternative — don't comply blindly.
|
|
- **Verify UI in the browser before calling it done.** tsc/unit/DOM tests miss visual and interaction
|
|
bugs; do a real visual check of any UI change, and run DESIGN.md's four-step Conformance check.
|
|
- **Be terse everywhere else.** Outside those decision points, lead with the answer or decision. During
|
|
implementation and debugging especially: no restating the same point reworded, no filler.
|
|
|
|
## Architecture shape (don't fight it)
|
|
|
|
`routes/*` (HTTP only: validation, auth-dependency, serialization) → domain packages `sync/`, `plex/`,
|
|
`downloads/`, `youtube/`, `issues/` (shared by routes + scheduler + worker) → flat core (`auth`, `quota`,
|
|
`sysconfig`, `state`, `audit`, `notifications`, `userscope`, `realtime`, `ratelimit`, `bodylimit`, `progress`, `collation`,
|
|
`security`, `logs`, `logsink`, `diagnostics`, `debugsession`) → `models/db/config`. **Dependency direction is one-way: core never imports routes.**
|
|
Frontend: `main.tsx` provider tree → `App.tsx` shell → `moduleRegistry` route-chunks → module
|
|
components → the **platform layer `components/ui/`** (domain-free primitives: the popover engine and
|
|
its menus, `Modal`/`Overlay`, `Button`/`form`, `DataTable`/`VirtualGrid`, `QueryState`, …). That
|
|
boundary is one-way and **lint-enforced**, not just described: a platform component may not import a
|
|
feature component or a domain API slice, and one layer down `lib/` may not import from `components/` at
|
|
all, not even a type (`no-restricted-imports` in `eslint.config.js`, with its own test). A new generic
|
|
component goes in `ui/`; see [`frontend/src/components/ui/README.md`](frontend/src/components/ui/README.md).
|
|
Data via react-query (`qk` factory), per-account `localStorage`, and server prefs; API through
|
|
`lib/api/*` slices over `core.req<T>`.
|
|
|
|
**Not wanted** (reviewed and rejected as over-engineering for this size): microservices, event bus,
|
|
CQRS, a repository/DI framework, a router library, a generic CRUD-router factory or base serializer.
|
|
|
|
## Canonical patterns — when you have a choice, pick THIS
|
|
|
|
| Area | Use | Not |
|
|
| ------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
|
| ORM | SQLAlchemy 2.0 `select()` | `db.query(...)` |
|
|
| Request body | a Pydantic model | `payload: dict` |
|
|
| Request SIZE | nothing — `bodylimit` caps every route (SEC-20); need more? add a ceiling to `ROUTE_MAX_BODY_BYTES` | `capped_body` on a route that does not read raw bytes itself |
|
|
| Ownership check | `owned_or_404` / `get_or_404` | ad-hoc `_own_playlist`/`_owned`/`_own_job` |
|
|
| Admin gate import | `app.auth` | the `app.routes.admin` re-export |
|
|
| Upstream failure status | `422` with JSON detail | `502` (reserve 502 for raw byte-proxies) |
|
|
| User-facing error | structured `detail` (`{code, reason}`) | a bare English string under a bilingual UI |
|
|
| Mutation response | `{"ok": true, …}` (`{"deleted": id}`), test-held | a bare `{"unsubscribed"}`/`{"cleared": n}`, no `ok` |
|
|
| Transaction commit | the handler commits; helpers `flush` (test-held list) | helpers that commit (only `quota` owns its session) |
|
|
| Admin knob storage | `sysconfig` (values) | env / `app_state` / `scheduler_settings` scatter |
|
|
| Job status word | the `downloads/status.py` StrEnum | loose `"canceled"`/`"cancelled"` literals |
|
|
| Current time | inline `datetime.now(timezone.utc)` | new `now_utc`/`_now` helpers |
|
|
| FE per-account state | `usePref` | raw `readAccount`/`writeAccount` or bespoke hooks |
|
|
| FE button | `ui/Button` (a `SIZES` rung) · `IconButton` · `Pressable` (a clickable card/row) · `Chip` · `DragHandle` | a raw `<button>` or `FocusSafeButton` outside the control primitives (lint-enforced) |
|
|
| FE keyboard shortcut | a row in `lib/keymap.ts` + `dispatch()` | a private `keydown` listener with its own key literals |
|
|
| FE list/grid keys | `NavGrid`/`keyboardNav` + `navItemAttrs` + `data-nav-open` | a private arrow-key / roving-tabindex handler |
|
|
| FE view push (history) | `rememberNavPosition()` right before `history.pushState` | a push that forgets: Back strands focus on `<body>` |
|
|
| FE text field / checkbox / slider | `ui/Input` · `Textarea` · `InnerInput` in a `fieldClass` frame · `Checkbox`/`CheckRow` · `Slider` | a raw `<input>`/`<textarea>` anywhere but those primitives (lint-enforced) |
|
|
| FE glass surface | a role: `SURFACE.panel` / `card` / `menu` / `popover` / `tooltip` (`ui/surfaces.ts`) | a bare `glass`/`glass-card`/`glass-menu` beside a hand-picked radius (lint-enforced) |
|
|
| FE static label / button-like link | `Badge` (flat, no edge) · `LinkButton` / `IconLink` | a glass pill that reads as a chip; an `<a>` hand-painted like a button |
|
|
| FE control | `SegmentedToggle`; a long or user's list: `FilterSelect` | a native `<select>` (lint-enforced) |
|
|
| FE loading/empty/error | `QueryState` / `StateMessage` | ad-hoc conditionals |
|
|
| FE failed write | `meta.failTitle`, else `notifyActionError(err, title)` | a bare `catch {}` or `notify()` with no error attached |
|
|
| FE network call | `req`/`uploadReq`/`beacon`, or `lib/http.ts` `send` (public pages) | a bare `fetch`/`XMLHttpRequest`/`WebSocket` (lint-enforced) |
|
|
| FE diagnostics | `lib/log.ts` (`log.info("event.code", msg, attrs)`); a filter change → `lib/filterLog.ts`, a player → `lib/mediaLog.ts` | `console.*` (lint-enforced); a value only a higher level may carry, logged unguarded |
|
|
| FE state that MIRRORS a prop/query | derive it, or hold a draft (`draft ?? server`) | a copy an effect re-syncs — that paints the stale value first |
|
|
| FE state reset on a key change | adjust it DURING render against a remembered key | `useEffect(() => setX(init), [key])` |
|
|
| FE effect that must not re-subscribe | `useEffectEvent` (React 19.2+); a handler or timer: `useLatest` / `useStableCallback` | a render-phase `ref.current = x` — the React Compiler skips the component |
|
|
| FE hook suppression | an inline `eslint-disable-next-line` NAMING the invariant | `eslint-suppressions.json` (deliberately absent — E27 S2) |
|
|
| FE popover / scroll-fade ref | destructure it: `const { setReference } = pop.refs`, then `ref={setReference}` | `ref={pop.refs.setReference}` / `ref={fade.ref}` — the compiler then treats the whole object as a ref and skips |
|
|
| FE async handler with cleanup | `try { … } catch { … }`, then the cleanup after it (steps that `return` early go in their own function); a conditional, loop or `?.` the `try` needs goes before it or into a module-level helper | `try/finally`, or a `?:` / `??` / loop / `?.` inside `try` — the React Compiler skips the whole component (and `react-hooks/todo` misses most of these) |
|
|
|
|
## House rules that bite (learned from real bugs)
|
|
|
|
- **A rule stated in a comment must also be enforced in code.** The worst bugs here came from a comment
|
|
and the code/migration drifting apart (a `cancelled`≠`canceled` literal killed a cancel path). Put the
|
|
invariant in an enum/constant/shared function, not in prose.
|
|
- **Fix a principle everywhere, in one sweep.** If you correct one owner/commit/serialization site,
|
|
`grep` for the siblings and fix them too — asymmetric fixes are a recurring smell here.
|
|
- **Honest states.** An error must not render as an empty state; a swallowed failure must not look like
|
|
"no results". Reads degrade honestly; writes are conditional (`UPDATE … WHERE status='running'`).
|
|
- **i18n parity.** HU/EN, zero key drift. Every user-facing string is translated.
|
|
- **Comment the "why", keep it current.** This codebase uses comments as its architecture doc; a stale
|
|
"why" is a liability, so update it when the behavior changes.
|
|
|
|
---
|
|
|
|
_Run-it-locally and the full gate commands live in [`docs/development.md`](docs/development.md); backend
|
|
code conventions in [`backend/app/README.md`](backend/app/README.md). This file stays the source of
|
|
truth for the canonical-patterns table and the gate/workflow/collaboration rules above._
|