BBergle/bike-app
Public Access
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
9fa50cb2ead5a4d418894de4575078390a202aad
bike-app/CLAUDE.md
T
BBergleandClaude Opus 5 e7392a5723
CI / Repo hygiene (pull_request) Successful in 2s
Details
CI / Web (lint, typecheck, build) (pull_request) Successful in 13s
Details
CI / Migrations reversible (pull_request) Successful in 6s
Details
CI / API (lint, types, tests) (pull_request) Successful in 52s
Details
refactor(api): move from Postgres+RLS to single-engine SQLite 
Reverses a shipped, tested, merged decision (D4/PR #2) rather than building
on it — see docs/DECISIONS.md D15 for the full record: what was rejected
(Postgres as a second container; Postgres+PostGIS bundled inside the single
container via a supervisor), what this costs (no database-level RLS, no
PostGIS, procrastinate needs replacing — all stated as a concern before this
was decided, and reaffirmed anyway, which is the user's call to make about
their own instance).

The one invariant-critical consequence: isolation between users now rests
entirely on the repository-layer scope (db.py's `Scope.select()`), not two
layers. CLAUDE.md's invariant #4 is revised accordingly. This is not a
downgrade-and-hope — `Scope` is built so an unfiltered query against a
user-owned table is structurally harder to write than a scoped one (there is
no method on `Scope` that returns one), and
tests/test_auth.py::test_scoped_session_blocks_cross_user_reads replaces the
old RLS proof with the same empirical standard: it doesn't trust the query
builder filters correctly because the code reads correctly, it registers two
real users and checks.
test_unscoped_session_can_see_every_user_when_misused is the deliberately
alarming companion — it demonstrates exactly what a reviewer must now catch,
since nothing else will.

Six real, non-obvious SQLite behaviours found and fixed by actually running
this against a real file, not assumed from docs:

- Foreign keys, ON DELETE CASCADE included, are OFF by default per
  connection — deleting a user silently left orphaned sessions/api_tokens,
  no error either way. Fixed with PRAGMA foreign_keys=ON on every connect.
- Transactions default to DEFERRED, which only takes a write lock on the
  first actual write — a real check-then-act race for invite redemption
  (two concurrent redemptions could both read used_count < max_uses as true
  before either commits). Fixed by disabling the driver's implicit BEGIN and
  issuing BEGIN IMMEDIATE ourselves — SQLAlchemy's own documented recipe for
  this, not improvised.
- DateTime(timezone=True) does NOT round-trip tzinfo on SQLite — a tz-aware
  datetime goes in, a naive one comes back out, and every
  `expires_at < datetime.now(UTC)` comparison in auth/service.py then raises
  TypeError. Fixed once at the Base level with a UTCDateTime TypeDecorator
  rather than per-column.
- Uuid(as_uuid=True) stores as 32-char hex with NO hyphens on SQLite, not
  str(uuid)'s hyphenated form. A test fixture that raw-inserted the
  hyphenated form left rows the ORM's own later UPDATE (via
  invite.used_count += 1's autoflush) could never match by primary key,
  updating zero rows and raising StaleDataError. Fixed by using .hex to
  match exactly what the ORM itself writes.
- BEGIN IMMEDIATE applies to every transaction, reads included — a
  long-lived test fixture that autobegins a transaction via a bare read and
  never explicitly closes it holds SQLite's exclusive write lock for the
  rest of the test, and a later scoped_session() call fails with "database
  is locked". Not an app-code bug (every real session block closes cleanly
  on exit), but real enough to document since the next person writing a
  test against the db_auth fixture will hit it too.
- Python's sqlite3 module deprecates its own implicit datetime adapter as of
  3.12 — silent today, warns on every raw-SQL datetime bind. Only ever hit
  test fixture code (the ORM path never uses it, confirmed by running the
  ORM-only health test with warnings promoted to errors and it stayed
  clean); fixed there with an explicit .isoformat() rather than left for a
  future Python version to turn into a real failure.

Also, since with_for_update() silently no-ops on SQLite (confirmed —
SQLAlchemy emits no SQL for it, no error either) rather than actually
locking anything: removed it from register()'s invite-redemption query and
corrected the comment to attribute the concurrency guarantee to BEGIN
IMMEDIATE, where it now actually lives.

One PR, not several, for the same reason PR #2 was: the migration, the
models, db.py, and the docs recording why are five views of one decision —
splitting them wouldn't make review easier, just disconnected. 552
insertions / 548 deletions across 17 files, most of it necessarily touching
what PR #2 shipped rather than net-new code.

Deliberately deferred, not solved here: PostGIS's replacement for spatial
storage, procrastinate's replacement for background jobs, and the
EXCLUDE USING gist constraint's replacement for component_installs — none
of those tables exist yet (Phase 1-2), so none of it is broken, and
docs/DECISIONS.md D15 records exactly what each future phase needs to
decide before it can be built. .gitea/workflows/deploy pipeline (PR #4,
built for the old 3-container Postgres compose stack) was closed as
superseded rather than merged; the single-container image build is
follow-up work, not part of this change.

Verified: ruff check, ruff format --check, and mypy --strict all clean.
13/13 pytest passing against a real SQLite file, including with
DeprecationWarning promoted to an error (confirms the sqlite3 adapter
deprecation fix actually holds, not just that it's quiet by default). Full
alembic upgrade -> downgrade -1 -> upgrade cycle run clean. alembic check
clean with no include_object filter needed at all now (SQLite starts with
nothing but what our own migrations create — no PostGIS/TIGER noise to
filter out in the first place). CI's exact migration command sequence
reproduced locally end to end before touching the workflow file.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-21 15:42:44 -04:00

185 lines
8.8 KiB
Markdown
Raw Blame History

# CLAUDE.md
Conventions for this repo. Agents and humans both follow these. Read before making changes.
## What this is
A self-hosted cycling app: ride sync from a Bryton Rider 650, mileage tracking, spare-parts
inventory, maintenance records with mileage-milestone reminders. See `docs/PLAN.md` for the full
design, `docs/DECISIONS.md` for settled decisions, `docs/RESEARCH.md` for source material.
**Read `docs/DECISIONS.md` before proposing an architectural change.** If a decision there is wrong,
say so and argue it — but don't silently contradict it.
## Layout
```
apps/api/ Python 3.12 / FastAPI / SQLAlchemy async / Alembic
apps/web/ SvelteKit static SPA (installable PWA)
packages/openapi/ openapi.json — COMMITTED contract artefact, CI enforces it matches the code
deploy/ single-container Dockerfile, Caddyfile, systemd units, backup scripts —
see docs/DECISIONS.md D15 for why this isn't docker-compose
docs/ plan, decisions, research
scripts/ repo tooling (PR helpers, etc.)
.gitea/workflows/ CI
```
## Non-negotiable invariants
These are load-bearing. Breaking one is a correctness bug, not a style choice.
1. **Raw bytes are the only truth.** Every ingested file is written to the content-addressed blob
store *before* parsing, and is never mutated or deleted. Every table is a rebuildable projection:
deleting everything derived from a `raw_file_id` and re-parsing must be semantically a no-op.
2. **No odometer columns.** Component wear is always derived by replaying activities against
time-ranged `component_installs`. Never add a stored running total to a component.
3. **All physical quantities are SI integers** in storage — metres, seconds, mm/s, centimetres,
grams, minor currency units. Imperial is display-only. Never store a float mile.
4. **Every user-owned table has `user_id`, and every query against it goes through the
repository-layer scope helper — never a raw query filtered by hand.** This used to be backed
by Postgres RLS as a second, database-enforced layer (see `docs/DECISIONS.md` D4/D15); SQLite
has no equivalent, so the repository-layer scope is now the *only* enforcement, which makes it
non-negotiable rather than defense-in-depth. A new domain table without a passing isolation
test (see `tests/test_auth.py`'s pattern) is not done.
5. **Secrets never leave the server.** The Bryton credential is password-equivalent. It must not
appear in any API response model, any log line, or any error message.
6. **One ingestion path.** All sources funnel through `ingest_bytes()`. Never add a second parse
path for a new source.
## Style
- **Python:** `ruff` (lint + format), `mypy --strict`. Type everything. Async throughout; no sync DB
calls in request handlers.
- **SQL:** migrations via Alembic only, never manual DDL. Every migration must survive
`upgrade -> downgrade -1 -> upgrade`.
- **Tests:** pytest against a real SQLite file, never mocks for DB behaviour. Parser changes need
a golden fixture in `apps/api/tests/fixtures/fit/`.
- **Commits:** imperative mood, explain *why* in the body. Conventional-commit prefixes
(`feat:`, `fix:`, `refactor:`, `test:`, `docs:`, `chore:`, `ci:`).
- Match surrounding code. Don't introduce a new pattern when one exists.
## Branching
**Never commit directly to `main`.** `main` is protected and only moves via merged PRs.
```
feat/<scope>-<short-desc> new capability feat/ingest-fit-parser
fix/<scope>-<short-desc> bug fix fix/wear-wet-multiplier
refactor/<scope>-<desc> no behaviour change
test/<scope>-<desc> tests only
docs/<desc> documentation
chore/<desc> tooling, deps, CI
```
Scope is usually the area: `ingest`, `wear`, `auth`, `notify`, `web`, `deploy`, `ci`.
One branch = one reviewable change. If a branch grows past roughly 400 changed lines, it should
probably have been two.
## PR workflow
1. Branch from an up-to-date `main`.
2. Commit in logical steps. Push the branch.
3. Open a PR with `scripts/pr.sh` (see below) or the web UI. Fill in the template honestly —
especially "How this was verified."
4. CI must be green. A red PR is not ready for review, and shouldn't be requested.
5. Review happens on the PR. Address feedback with new commits, don't force-push over review history
unless asked.
6. Squash-merge into `main`. Delete the branch.
**Agents:** you open PRs, you do not merge them. Merging is a human decision.
## Model allocation
The orchestrator runs **Opus 5**. Worker agents do not default to it.
The rule is **match the model to the cost of being wrong, not the size of the task.** A big,
well-specified job with obvious failure modes is cheap work. A small change to dedupe logic that
silently corrupts data for six months is expensive work.
### Opus 5 — orchestration, and anything subtly wrong-able
- Task decomposition, planning, and **all code review** (see below — this is where it pays for itself)
- Anything touching the six invariants above
- `ingest/` — FIT parsing, the three dedupe layers, activity-vs-course discrimination. Errors here
are silent and corrupt the archive.
- `wear/` — the wear SQL. Wrong numbers that still look plausible are the worst failure mode in the
product, because nobody notices.
- `auth/`, any repository-layer user-scoping code — security, and a mistake exposes another user's
data. This carries more weight than it used to: there is no database-enforced RLS backstop
anymore (see invariant #4 and `docs/DECISIONS.md` D15), so this code *is* the isolation
boundary, not one layer of it.
- `sources/bryton/` — a reverse-engineered protocol with no spec to check against.
- Schema migrations that alter or drop existing columns.
- Debugging anything that two Sonnet attempts have already failed to fix.
### Sonnet — the bulk of implementation
Default for normal feature work. `docs/PLAN.md` is detailed enough that most implementation is
careful transcription plus ordinary judgement, and CI catches the rest:
- CRUD endpoints, Pydantic models, repository methods
- SvelteKit components, routes, styling, the service worker
- Tests against an already-decided behaviour
- Additive migrations, the deploy Dockerfile/Caddy config, CI workflows
- Documentation
### Haiku — mechanical work
- Dependency bumps, formatting, renames, changelog entries
- Log triage, "find every call site of X"
- Anything where a script would also work
### The economics
**Sonnet implements, Opus reviews** is the default pairing, and it is much cheaper than Opus
implementing while catching most of the same problems. Review reads a focused diff; implementation
reads the whole repo and writes for hours. If budget is tight, cut Opus from implementation before
you cut it from review.
Escalate a task to a stronger model when it has actually failed, not preemptively. Two failed Sonnet
attempts is a real signal; "this feels hard" is not.
Never run more agents in parallel than there are genuinely independent branches of work. Parallel
agents that touch the same files cost more than one agent working in sequence, because the merge
conflicts and re-review are paid twice.
## Working as a parallel agent
Each agent works in its own git worktree so parallel branches don't collide:
```bash
git worktree add ../bike-app-<branch> -b feat/<scope>-<desc>
# work there, push, open PR
git worktree remove ../bike-app-<branch>
```
Rules:
- **Stay in your lane.** Touch only files your task needs. If you need a change in someone else's
area, note it in the PR rather than making it.
- **Never rebase or force-push another agent's branch.**
- **Rebase on `main` before opening the PR**, so the reviewer sees a clean diff.
- **Don't invent scope.** If the task is ambiguous, ask rather than guess — a wrong guess costs a
full review cycle.
- **Report honestly.** If tests fail, say so with the output. If you skipped something, say that.
"Done" means done and verified.
## What needs a human decision
Don't do these autonomously:
- Merging a PR
- Anything touching `docs/DECISIONS.md` (propose it in a PR, argue the case)
- Schema changes that drop or rewrite existing columns
- Anything that sends data off the server, or adds a third-party runtime dependency on one
- Rotating or changing secrets
- Force-pushing anything, ever, to `main`
## Environment
- Gitea at `http://192.168.0.3:3000`, repo `BBergle/bike-app`, act_runner on the same host.
- SSH remote `git@192.168.0.3:BBergle/bike-app.git`, key pinned in `~/.ssh/config`.
- **`secrets.GITEA_TOKEN` cannot push to the Gitea container registry** — use the `REGISTRY_TOKEN`
PAT secret (`package:write`). This is a documented Gitea limitation, not a misconfiguration.
- `jobs.*.environment` is silently ignored by Gitea Actions. Don't build a gate on it.
- Always pair `schedule:` with `workflow_dispatch:` — Gitea's cron has shipped flaky.
Reference in New Issue View Git Blame Copy Permalink