Files
bike-app/docs/DECISIONS.md
BBergleandClaude Sonnet 5 a114a7d3d8
CI / Repo hygiene (pull_request) Successful in 2s
CI / Web (lint, typecheck, build) (pull_request) Successful in 16s
CI / Migrations reversible (pull_request) Successful in 6s
CI / API (lint, types, tests) (pull_request) Successful in 54s
fix(deploy): push through a TLS-terminating proxy, not raw Gitea HTTP
release.yml's first real run failed: docker/login-action against
192.168.0.3:3000 hit "server gave HTTP response to HTTPS client" — Docker
refuses any non-localhost registry over plain HTTP by default, so this was
never actually a workflow bug.

Rejected insecure-registries in daemon.json after reading this Unraid host's
own rc.docker script: applying it needs a full dockerd restart, and with
Live Restore disabled here, that stops every one of the ~40 other containers
on the box first. Also rejected a real Let's Encrypt cert on a public
bbergle.com subdomain — this host's other subdomains are Cloudflare-proxied,
which would terminate TLS at Cloudflare's edge and never reach our own cert
at all.

Chosen instead, scoped to touch nothing already working: a self-signed cert
for registry.bbergle.com behind a new NPMplus proxy host (found its real
HTTPS port, 9537, by reading `docker port NPMplus` rather than assuming 443,
which is a different nginx process on this box entirely); an /etc/hosts
entry on the Unraid host so only that host needs to resolve the name (no
DNS record, no router/NAT dependency); and its CA dropped into
/etc/docker/certs.d, which Docker's own docs confirm is read per-connection
with no daemon restart required. Also pins buildx to driver: docker instead
of setup-buildx-action's default docker-container driver, which runs an
isolated builder that doesn't see /etc/docker/certs.d and would have quietly
defeated all of the above.

Full record, including what was rejected and why, in docs/DECISIONS.md D17.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R2ZKeWkZV7ehf7fivrAkkG
2026-09-21 20:52:56 -04:00

304 lines
22 KiB
Markdown

# 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.py` that
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 by `user_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 no `activities` table), 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 a `TEXT`/`BLOB` column with spatial math done in application code.
- **`procrastinate` is 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 gist` constraint design for `component_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.
### D16 — Single-container packaging: entrypoint migrations, tini + a two-line supervisor, Caddy binary copy, Unraid template
**Chosen:** one Docker image (root `Dockerfile`), built by copying the SvelteKit static build and
the API's venv into a runtime stage alongside a copied-out `caddy` binary. `deploy/entrypoint.sh`
runs `alembic upgrade head`, then starts uvicorn (loopback-only) and Caddy as two background
processes under `tini` as PID 1, and kills+exits if either one dies. Config surfaces as env vars
read by the existing `VELODROME_`-prefixed Pydantic settings; `deploy/unraid-template.xml` exposes
the required ones as Unraid Community Applications web UI fields instead of a `.env` file.
**Why not a real process manager (s6-overlay, supervisord):** two long-running processes with no
dependency graph between them (Caddy doesn't need to wait on uvicorn — it just proxies) doesn't
need a supervisor with restart policies, readiness ordering, or log multiplexing. A ~20-line bash
script under `tini` (for correct signal forwarding and zombie reaping, which a bare shell script as
PID 1 doesn't do) gets the one property that matters — if either process dies, the whole container
exits non-zero so Docker/Unraid restarts it — without a new dependency or a config format to learn.
Revisit if a third long-running process gets added later; two is the reasonable ceiling for "just
write the script."
**Why migrations run from the entrypoint, contradicting what apps/api/Dockerfile's own comment
used to say** ("Migrations run as an explicit step before this in deploy.yml... never from the
entrypoint, so a failed migration fails the deploy visibly instead of crash-looping here"): that
comment described the 3-container Postgres plan, where a separate `run --rm api alembic upgrade
head` step existed *before* `compose up -d`. A single container has nowhere else to put that step.
The property it was protecting — a failed migration must be visible, not silently served — still
holds: `set -e` means the script exits non-zero on migration failure, so the container never starts
serving traffic and shows as exited/restarting in `docker ps`/Unraid, which is the same visibility
by a different mechanism. What's genuinely lost is the *old* mechanism's failure mode of "the
previous version keeps running while the bad migration is investigated" — a single container that
fails to start migrations has no previous version still up. Acceptable for a single-instance
home-lab deployment; would need reconsidering (e.g. a blue/green swap) if this ever needed
zero-downtime deploys.
**Why the Caddy binary is copied from `caddy:2` rather than using a Caddy base image:** the runtime
needs both Python (for uvicorn) and Caddy; picking either official base image as the starting
point means installing the other stack into it by hand. Caddy's official images are a single
statically-linked Go binary with no CGO, so `COPY --from=caddy:2 /usr/bin/caddy /usr/bin/caddy`
into a `python:3.12-slim` base is the documented, standard way to get both without a second
package manager or a source build.
**Why an Unraid template file, not just documentation:** the earlier decision (in-session) was to
move configuration out of a `.env` file and into fields the Unraid web UI can fill in — a plain env
var table in a README doesn't do that by itself, since Unraid still needs a `Config`-tagged XML
entry per field to render one. `deploy/unraid-template.xml` is that; every field stays hand-editable
in the UI afterward regardless of what the template pre-fills, so getting a default slightly wrong
here isn't load-bearing.
**What this doesn't do:** `.gitea/workflows/release.yml` builds and pushes the image to the Gitea
registry; it does not SSH into the Unraid host and recreate the running container. Rolling a new
image out is a manual/Unraid-side action (pull + Apply, or Unraid's own update check), not
something CI does unattended — consistent with treating "affects a shared, already-running system"
as something a human triggers, not automation.
### D17 — Registry TLS: self-signed cert behind NPMplus, not `insecure-registries`, not a real domain
**Problem:** `release.yml`'s first real run failed — `docker/login-action` against
`192.168.0.3:3000` (Gitea's plain-HTTP address) hit `server gave HTTP response to HTTPS client`.
Docker refuses TLS-less registries by default; this was never a workflow misconfiguration, it's
expected Docker behaviour for any non-localhost registry.
**Rejected: `insecure-registries` in `daemon.json`.** The obvious fix. Rejected after actually
reading `/etc/rc.d/rc.docker` on the Unraid host rather than assuming: applying a `daemon.json`
change requires a full `dockerd` restart, and (with `Live Restore` disabled on this host) both
Unraid's own restart path *and* a raw `kill` of `dockerd` stop every one of the ~40 other
containers running on the box first, as part of the restart/shutdown sequence — Plex, Home
Assistant, Vaultwarden, everything. Correct fix for the narrow problem, unacceptable blast radius
for this specific host.
**Rejected: a real Let's Encrypt cert on a new `bbergle.com` subdomain routed publicly.** The
user's other NPMplus-fronted subdomains resolve through Cloudflare's proxy (orange-cloud), not
directly to the home IP. A Cloudflare-proxied hostname would have terminated TLS at Cloudflare's
edge with Cloudflare's own cert, never reaching our self-signed cert or NPMplus's own TLS
config at all — the entire trust chain would depend on Cloudflare's origin SSL mode, and likely on
firewall rules restricting port 443 to Cloudflare's IP ranges, neither of which this problem
needed to involve.
**Chosen:** a small, fully self-contained fix, scoped to touch nothing already working:
- A 10-year self-signed cert for `registry.bbergle.com` (SAN-only, no real domain dependency).
- An NPMplus proxy host (`registry.bbergle.com` -> `192.168.0.3:3000` over plain HTTP internally)
terminating TLS with that cert, on NPMplus's existing HTTPS port (`9537` on this host — found by
reading `docker port NPMplus` rather than assuming 443, which is a *different* nginx process on
this box entirely).
- `/etc/hosts` on the Unraid host mapping `registry.bbergle.com` -> `192.168.0.103` (itself) —
chosen over a real DNS record specifically because the only client that ever needs to resolve
this hostname is the Unraid host's own `dockerd` (Gitea Actions runs in DooD mode against that
same host's Docker socket). This sidesteps Cloudflare, the router's NAT/hairpin behaviour, and
any port-forwarding question entirely — verified separately that hairpin NAT works by default on
this user's UniFi gateway, but it turned out to be unnecessary for this fix regardless.
- `/etc/docker/certs.d/registry.bbergle.com:9537/ca.crt` on the Unraid host, trusting that cert for
that host:port specifically. Confirmed (Docker's own docs) that `certs.d` is read per-connection,
not baked in at daemon start — no `dockerd` restart, no impact on any other container.
- `docker/setup-buildx-action@v3` pinned to `driver: docker` in `release.yml` instead of its
default `docker-container` driver — the default runs BuildKit in an isolated builder container
that does not see the host's `/etc/docker/certs.d`, which would have silently defeated the whole
point of the trust setup above. We don't build multi-platform images, so nothing the
`docker-container` driver offers is actually needed here.
**Not persisted across a reboot, deliberately, for now:** neither the `/etc/hosts` line nor the
`certs.d` file are wired into `/boot/config/go` — both live under `/`, which Unraid rebuilds fresh
from `/boot` on every boot. Raised explicitly rather than assumed: the user was (rightly) wary of
hand-editing anything under `/boot` after an earlier, unrelated discussion of what a broken `go`
script could do to boot. Persisting this is a five-minute follow-up (append two lines to `go`) once
they're ready to make that call deliberately, not bundled into this fix.
**What's unaffected:** Gitea's own web UI, git remote, and API — all still plain
`http://192.168.0.3:3000`, exactly as CLAUDE.md documents. NPMplus's existing public proxy hosts
and certs (`vaultwarden.bbergle.com` etc.) — untouched, new proxy host only. No other container on
the Unraid host was restarted, reconfigured, or otherwise touched to make this work.
---
## 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.