BBergleandClaude Opus 5
e7392a5723
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>
13 KiB
Decisions
Settled during the planning session on 2026-09-20. Each entry records what was chosen, what was rejected, and why — so we don't relitigate them.
D1 — Build fresh, don't fork
Chosen: greenfield, borrowing data models only. Rejected: forking Endurain (AGPL, FastAPI+Vue, already has gear/component tracking, but in a feature freeze with a thin maps story) or FitTrackee (AGPL, mature heatmaps, but its "equipment" is one flat tag per workout with no wear intervals — the entire parts/maintenance system would be bolted on anyway). Why: nothing existing covers rides + real parts inventory + maintenance + the self-host features. Take Endurain's gear/component table structure and strava-gear's retroactive wear computation as references; own every line; avoid AGPL entanglement.
D2 — PWA, not a native iOS app
Chosen: SvelteKit static SPA, installed to the iPhone home screen. Rejected: a native SwiftUI app. Why: a home-screen PWA gets the icon, standalone display, offline caching, and — since iOS 16.4 — real push notifications, which was the only thing that used to force native. Native would cost $99/yr for an Apple Developer account, TestFlight/sideloading to reach family phones, and a second codebase forever. The two real PWA gaps on iOS (Web Bluetooth, Background Sync) are irrelevant here: Bryton BLE is a dead end regardless, and the server does all syncing. Kept as insurance: the backend stays strictly API-first with a CI-enforced OpenAPI contract, so if Apple ever makes the PWA route untenable, a native client is a code-generation exercise.
D3 — Bryton cloud poller is the primary ingestion path
Chosen: server-side poller against the reverse-engineered Bryton Active API, every 15-20 min.
Rejected: Strava as a source (no export_original — decoded smoothed streams only; plus the
June 2026 tier restructure caps new apps at 10 users and requires a paid dev subscription);
BLE/ANT-FS direct (nobody has reverse-engineered Bryton's BLE — weeks of work, breaks on firmware
updates); depending on the Bryton Active phone app (the original complaint).
Why: the Rider 650 has on-device Wi-Fi (Main Menu -> Data Sync) and uploads to Bryton's cloud
with no phone involved, and the cloud API returns the original unmodified FIT bytes. That's both
zero-touch and higher fidelity than the current Strava route.
Fallbacks, both built: USB watch folder (also the historical-backfill mechanism, so it stays
exercised rather than bit-rotting) and manual upload.
D4 — Python / FastAPI / Postgres+PostGIS — database choice superseded by D15
Chosen: Python 3.12, FastAPI, Pydantic v2, SQLAlchemy 2.0 async, Alembic, PostgreSQL 16 + PostGIS 3.4.
Rejected: TypeScript full-stack, Go.
Why: fitdecode is Python-only, and the Bryton poller reference implementation is Python
(~200 lines of Meteor DDP to vendor rather than reimplement — DDP over SockJS is fiddlier than it
looks in JS). FastAPI emits OpenAPI 3.1 for free. PostGIS is needed for heatmaps, bbox queries, and
self-segment matching. Go has the best raw performance and a fine FIT library but the weakest
data-analysis ecosystem for the later analytics work.
The Python/FastAPI half of this still stands. The database half — Postgres+PostGIS — was replaced with SQLite in D15, after Phase 0 was already built and merged against Postgres. Kept here, marked superseded rather than deleted, so the PostGIS-specific reasoning (heatmaps, bbox queries, segment matching) is still visible as context for whatever Phase 3 ends up doing about spatial storage without it.
D5 — procrastinate for background jobs — needs a replacement, see D15
Chosen: procrastinate (Postgres-backed queue).
Rejected: Celery (needs Redis/RabbitMQ, heavyweight, weak async, second-class Postgres broker),
arq (Redis-only — a whole container for ~50 tasks/day), APScheduler (a scheduler, not a durable
queue: no retries, no dead-lettering, no multi-worker coordination).
Why: the decisive property is transactional enqueue — the raw_files INSERT and the parse-job
enqueue commit atomically on one connection, so there are no orphaned blobs and no jobs pointing at
rolled-back rows. That's impossible with a Redis broker without inventing an outbox. It also has
built-in cron, which removes the scheduler container, and keeps job state inside the same pg_dump.
procrastinate is Postgres-only — no SQLite backend exists, so D15's move to SQLite invalidates
this choice. Nothing consumes a job queue yet (no ingestion pipeline exists), so this is a
deferred decision, not an urgent one: whatever replaces it (APScheduler for a scheduler, or a
hand-rolled SELECT ... WHERE claimed_at IS NULL LIMIT 1 polling table for real job durability —
SQLite's single-writer model makes even a crude polling table viable at this app's scale) needs
picking before Phase 1's ingestion pipeline, not before.
D6 — Opaque bearer tokens, no JWT
Chosen: Argon2id passwords + opaque tokens in a sessions table, HttpOnly cookie for the PWA.
Rejected: JWT.
Why: at 5-15 users, verification is one indexed PK lookup (~0.1ms), and you get instant
revocation, a real device list, and no key-rotation or clock-skew bug class. JWT's only advantage is
stateless horizontal scale, which will never arrive — choosing it would be a permanent complexity
tax against a benefit that never materialises.
D7 — Raw bytes are the only truth
Chosen: every ingested file is written to a content-addressed blob store before parsing, and is
never mutated or deleted. All tables are rebuildable projections.
Why: this converts "a parser bug wrote wrong elevation to 4,000 rides" and "reprocess a decade of
history against a better DEM" from incidents/migrations into routine batch jobs (parser_version
bump + requeue). It is the single most load-bearing rule in the design, and it's also the primary
data-loss control.
D8 — No odometer column anywhere; wear is derived
Chosen: component_installs as a time-ranged association (strava-gear's model, made relational
with a GIST EXCLUDE constraint), with wear computed by replaying the activity stream.
Rejected: a stored running odometer per component.
Why: correcting "I actually swapped that chain a week earlier" becomes one UPDATE and every
downstream number self-corrects. Parts moving between bikes is two rows. A stored counter can do
neither without a reconciliation nightmare — which is exactly where FitTrackee's flat equipment tag
falls over in year two.
The time-ranged-association model doesn't depend on Postgres. The EXCLUDE USING gist constraint
enforcing "a component is in exactly one place at a time" does — SQLite has no range types and no
exclusion constraints. component_installs doesn't exist yet (Phase 2), so this is another
deferred casualty of D15, not an active one: the same invariant will need enforcing at the
application layer (check-then-insert inside a transaction) instead of the database refusing an
overlapping row outright.
D9 — Streams as columnar int32 arrays
Chosen: one row per channel per activity, values_i32[] with a scale factor.
Rejected: a normalized per-sample table (~5x larger with index, and every real query wants the
whole stream anyway), JSONB (untyped, 3-5x larger, slow to deserialise), TimescaleDB (solves
cross-entity firehose scans; we do per-entity blob reads — and it would mean abandoning the
postgis/postgis base image and taking on extension-version coupling at every Postgres upgrade).
Bonus: FIT stores position as int32 semicircles, so lat/lon are lossless and free in this encoding.
Aggregates are precomputed at ingest into activity_stats, never scanned from streams.
D10 — Imperial display units
Chosen: users.unit_system defaults to imperial. Storage stays SI integers throughout
(metres, seconds, mm/s, minor currency units); units are strictly a presentation concern.
D11 — Notification dedupe by cycle sequence
Chosen: notification_log with UNIQUE (user_id, dedupe_key) where the key is
service_due:<rule_id>:<component_id>:<cycle_seq>:<threshold> and cycle_seq counts service
events logged against that (component, rule).
Why: a naive nightly evaluator nags you about the same chain every night until you fix it, and
you learn to ignore it. This fires exactly once at 80%, once at 100%, then goes quiet; logging the
service increments the cycle and re-arms the next 200 miles. It's how recurrence works without a
cron-style recurrence engine.
D12 — ntfy first, Web Push second
Chosen: apprise -> ntfy as the primary notification channel; Web Push (VAPID/pywebpush) as
the nicer layer on top; every notification is also an in-app inbox row.
Why: apprise is already in the stack for poller alerts and works on iOS with no PWA-install
requirement, so notifications can ship early. Apple's web.push.apple.com does speak standard RFC
8291 (no Apple Developer account needed), but only for home-screen-installed PWAs, and iOS silently
drops subscriptions after OS updates. Push must never be the only path to the information.
D13 — Monorepo
Chosen: one repo for API + web + deploy + workflows. Why: one maintainer, and API and client change together constantly. D2 removed the only real argument for splitting (a native app would have needed macOS runners that a Linux act_runner can't provide) — now every artefact builds on the same runner.
D14 — Research agent models
Chosen: Opus for the Bryton protocol research and the architecture design (ambiguous, reverse-engineering, synthesis-heavy); Sonnet for the two breadth surveys (existing self-hosted apps, Gitea CI patterns) where material is well-documented and the work is volume.
D15 — SQLite, single container, no database-level RLS
Chosen: SQLite as the database, and the whole app (Caddy + API, static web build baked in) as
a single container. Made explicitly after Phase 0 was already built, tested, and merged against
Postgres+PostGIS with a two-role RLS architecture (D4, and the velodrome_app/velodrome_auth
split in apps/api/velodrome/db.py) — this is a deliberate reversal of a shipped decision, not a
greenfield choice, and it was made with the costs stated plainly first.
Rejected, with reasons on the record: keeping Postgres as a second container (rejected — explicitly wanted exactly one container total); bundling Postgres+PostGIS inside the single container via a process supervisor (offered as the way to get "one container" without losing RLS or PostGIS — rejected in favour of SQLite specifically).
What this costs, stated once here rather than re-litigated every time it's felt:
- Row-level security is gone. SQLite has no roles, no session variables, no policy engine —
there is no database-enforced layer left, only the repository-layer scope. CLAUDE.md's
invariant #4 is revised accordingly (see the file) to describe app-layer scoping as the sole
mechanism rather than one of two layers. The user isolation test in
tests/test_auth.pythat used to prove RLS itself now proves the repository-layer scope does the same job in its absence — read it before touching any query that filters byuser_id. - PostGIS is gone. No native geometry columns, no GIST spatial indexes, no
ST_Envelope. Nothing in the schema uses it yet (Phase 0 has noactivitiestable), so this is a live decision for Phase 1/3 to make, not a retrofit — options include SpatiaLite, or storing tracks as GeoJSON/WKB in aTEXT/BLOBcolumn with spatial math done in application code. procrastinateis gone (D5) — Postgres-only, no SQLite backend. Also nothing consumes it yet; a replacement gets picked before Phase 1's ingestion pipeline needs one, not now.- The
EXCLUDE USING gistconstraint design forcomponent_installs(D8) — doesn't exist yet either (Phase 2); the "one place at a time" invariant will need application-layer enforcement instead of the database refusing an overlapping row outright.
Why proceed anyway: raised as a concern in-session, with each cost above stated before this decision was made; the user heard the full list and confirmed SQLite regardless. That's their call to make about their own single-user/family-scale instance, not an oversight to correct for them.
What's unchanged: invariants #1 (raw bytes immutable), #3 (SI integers in storage), #5 (secret containment), #6 (single ingestion path) — none of those were ever Postgres-specific. Auth design (D6, opaque bearer tokens) is unaffected. FastAPI/SQLAlchemy/Alembic stay exactly as chosen in D4; only the database engine underneath them changed.
Deliberately deferred
- Routing (Valhalla/Photon/Overpass) — Phase 5, optional. Several GB of RAM for something Komoot already does well.
- Local LLM ride summaries (Ollama) — Phase 4, behind a compose profile.
- Friends/family cross-visibility — Phase 4, as an additive widening of the repository-layer scope (an RLS policy pre-D15; see D15 for why that's no longer the mechanism), never as removal of the default per-user scope.
- Legacy Bryton format support — out of scope entirely. The Rider 650 writes
.fit. - Reverse-engineering Bryton's BLE — explicitly rejected. See RESEARCH.md §1.