Files
Levyraati26_go/docs/decisions.md
T
Esa Kataja 400b5d3833 Release 2026.08.01-1
Lyrics, versioning, and the song page's fact line.

- Lyrics are suggested at submission and never imposed: the worker makes one
  LRCLIB lookup, and Hae sanoitukset re-queries with whatever title and artist
  are typed. Neither overwrites what the submitter wrote. They live on the
  submission, travel to the song at publish, and stay editable after the song
  locks — the lock freezes what a song claims to be, and nobody reviewed the
  lyrics
- The review strip reads and writes side by side: lyrics left, review right.
  Synced LRC highlights the playing line and seeks on click; plain text scrolls
  continuously with a nudge knob. Following can be turned off
- CalVer YYYY.MM.DD-N, injected from the git tag with -ldflags, shown in the
  footer, the startup log and /healthz
- The song page's metadata became four labelled cells instead of one flat run
  of five different kinds of fact

Fixes: publishing wiped lyrics the worker had just fetched (a request that
omitted a field cleared it), lyric auto-scroll landed in the wrong place, and
the fader shifted the deck sideways at score 100.
2026-08-01 00:45:55 +03:00

212 lines
17 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Decisions
Append-only. Entries are never rewritten — if one is reversed, the reversal is a new entry that says
so. What the app does today is in [spec.md](./spec.md); this is why.
---
## 2026-07-31 — the rewrite
Levyraati existed as a Nuxt 4 app that reached v1.0-rc.3 and never launched. It is being rebuilt in
Go and the Nuxt repo archived. The reason was proportion: no SSR or SEO requirement for a
login-walled app for ten friends, no client state worth a reactive framework, and a dependency
surface larger than the feature set. There is no data to migrate — everything in the old `pgdata`
and `storage` was test data, so **the schema has no legacy to respect.**
1. **Go, `net/http`, `html/template`, `pgx`, plain SQL.** Dependency budget is `pgx` and
`x/crypto`; anything else needs a reason. Go 1.22 routing patterns cover every route, so no
router. Seven tables, so no ORM.
2. **Migrations run at startup**, before serving. One less deploy step. Carried over from the old app,
which got this right.
3. **Docker Compose shape carried over**: app + postgres, `./storage` and `./pgdata` bind mounts,
healthcheck-gated `depends_on`.
4. **No Tailwind, no bundler, no npm.** One hand-written stylesheet with CSS custom properties. The
dark rock/metal theme — Oswald headings, orange/red accents — survives dropping Tailwind
unchanged, as theme tokens rather than utility soup.
5. **Finnish only.** No locale JSON, no `T()`, no language cookie, no switcher. Strings go directly
in templates. The audience is ten Finnish speakers.
6. **`bigserial` ids, not UUID.** No id-generation code, no `google/uuid` in a two-dependency budget,
smaller indexes, and `/audio/{id}` validates by parsing an integer. Enumerable ids are not a
threat model here. Session tokens stay random — those are secrets.
7. **stdout logging only.** The rejected alternative was mirroring every entry into a `logs` table for
an in-app viewer: a custom handler, a buffered channel, a drain goroutine, drop accounting, a
retention `DELETE`, a table and a filtered page — ~150 lines to avoid `docker compose logs`. If
in-app visibility is ever wanted, build an *audit* view of domain events instead; those are
queries over tables that already exist.
8. **The admin is not a user.** Env credentials, Basic Auth, its own loopback listener. This deletes
the `role` column, first-launch seeding, admin sessions, the "cannot ban the last admin" rules,
and every "exclude the admin" clause that would otherwise appear in user and stats queries.
9. **Same process, two listeners** — not a second binary. A management binary would need its own
deploy and would race the startup migrations. Two listeners give the network isolation, which was
the only real benefit.
10. **The admin recovery endpoint is dropped.** The old app had a key-gated credential reset with a
`qwerty123` default in `docker-compose.yml`. The password is an env var now, so recovery is
editing it and restarting. No route, no key, no default.
11. **Conversion runs in the background; nothing enters `songs` until it succeeds and the submitter
confirms.** Costs a `submissions` table, buys a `songs` table where every row is a real song and
no query filters on readiness.
12. **Metadata is read synchronously at submit, conversion asynchronously.** Otherwise prefill races
the submitter's typing. This also revived the YouTube prefill cut earlier — the objection was the
extra round trip, and the redesign removed it.
13. **Publish is an explicit click**, never automatic on conversion success.
14. **Native `<audio controls>` for now.** Swapping in a custom player later is client-side and edits
one template partial; the element and `http.ServeContent` keep doing the real work.
15. **The API is JSON-only, and its contract is fixed now.** Pages return HTML, `/api/…` returns
JSON, nothing returns HTML fragments as data. Both surfaces are thin adapters over one data
function, so the domain rules cannot diverge. Explicit routes rather than `Accept` negotiation: a
client should not depend on a header being honored, and negotiation drags `Vary: Accept` along.
16. **HTMX for server round trips, Alpine for local state.** Reversed twice, so the reasoning matters
more than the outcome: the first pass rejected a JS framework outright; the API decision made
JSON-to-DOM plumbing necessary, which argued for Alpine; HTMX then removed that plumbing entirely
by swapping rendered partials. Fragments are a page-surface concern and never come from `/api`,
so this does not compromise 15.
17. **The contract is fixed now, the endpoints are built on demand.** Nothing consumes `/api` while
the browser talks HTML, so building all of it up front would be handlers with no callers. The
contract prevents breakage; each handler is one line whenever a consumer appears.
18. **The issue reporter ships in v1.** One table and two handlers, and the month it is most needed
is the first one. Members never touch the Gitea tracker; the admin transcribes anything worth
tracking.
19. **yt-dlp is updated by rebuilding the image, monthly.** Pinning a version only schedules the
breakage for a moment you did not choose. Originally `pip install -U yt-dlp`; changed on
2026-07-31 to `apk add yt-dlp` once Alpine 3.24 turned out to carry the current release
(2026.07.04, four weeks old) — which drops python3 and pip from the image entirely. The apk
route inherits Alpine's packaging lag, so pip is the fallback if it ever goes stale at a bad
moment. Note that this only holds on the *active* branch: 3.21 was 16 months behind.
20. **Parity plus YouTube, then iterate.** Nothing from the old `docs/IDEAS.md` and nothing from the
unbuilt-stats list ships in v1.
---
## 2026-07-31 — design session
Decisions 2137 came out of grilling the handoff. Several reverse entries above; where they do, it
says so.
21. **English identifiers, Finnish rendered text.** Schema, structs, and routes are English; every
Finnish word is pinned per term in `CONTEXT.md` so the UI cannot drift between *kappale* and
*biisi*. Finnish identifiers would mean ASCII-folding (`lahettaja`) and hand-diffing the spec
against the schema forever. The admin panel is Finnish too — one vocabulary, not two.
22. **The reveal rule covers the average score.** The earlier design hid the review text but printed
`average_score: 71.4` in the feed — the number is the part that actually anchors a reviewer, so
hiding the sentences alone was decorative. `/stats` stays fully public: it is a page you walk to
deliberately, and gating it would make Top 10 per-viewer, which stops rankings being something
the club can talk about. Costs a nullable `average_score` and a `reviews_revealed` flag on
`song_summary`.
23. **A reviewer may edit or delete their own review within 30 minutes.** One rule, two verbs: for
half an hour your review is yours to change or withdraw, after that it is on the record. Delete
is bounded by the same window because an unbounded one is an unlimited edit window with extra
steps — review, unlock the reveal, read everyone, delete, repost recalibrated. Nobody else ever
deletes a review; the admin's remedy is deleting the song or talking to the person.
24. **Locked is a live state.** A song freezes when it has any review and unfreezes if it drops back
to zero. Since delete is confined to 30 minutes (23), the unfreeze can only happen shortly after
a song's only review.
25. **A ban drops live sessions, keeps all content, and is reversible.** Checking `banned` only at
login would leave a banned member browsing until their session expired. Purging their content
would cascade *other people's* reviews and retroactively rewrite the leaderboards; a ban ends
participation, it does not rewrite history.
26. **The feed is a queue: unreviewed only, oldest first.** It drains, and the oldest unreviewed song
is the one most likely waiting on someone.
27. **Your own songs are excluded from the queue** — reverses the earlier decision to include them
marked "yours, can't review". Under a queue they can never be actioned, so they would sit
permanently at the front of your own list, oldest-first putting your least actionable item first
every visit. The original complaint — submitters thinking their song vanished — is answered by
listing your songs on your profile and in the browse list.
28. **A browse list at `/songs`.** The queue is a worklist, so a reviewed song leaves it forever;
without a browse list a song ranked 11th is unreachable even by its own submitter, including for
re-listening, which is the app's core loop. Nav is four items: Jono, Kappaleet, Tilastot, Oma
profiili.
29. **The queue is `GET /api/songs?unreviewed=1&order=oldest`**, not `/api/queue`. The queue is a
*view* of songs, and a future filter is another parameter rather than another route with another
shape.
30. **Profiles show counts and history-wide averages, never per-song opinions.** Listing a member's
reviews would walk straight through 22. The resulting principle covers every future page:
*per-song opinion is gated; whole-history aggregate is public.*
31. **Email stays as the login identifier**, even though v1 never sends mail. If the display name
were the login, renaming yourself would break your own credentials. Password reset and
announcements over SMTP are wanted and deferred (see [later.md](./later.md)) — v1 cannot verify
addresses, so that feature will need the admin as a fallback for typos.
32. **Sessions are idle timeouts: 24 hours, or 30 days with "remember me".** Absolute expiry was
rejected in favour of "N with no requests", so an everyday member is never logged out and a
borrowed machine forgets quickly. The TTL lives on the session row, so extending is one
assignment with no branch.
33. **Max 5 submissions per member per rolling 24 hours.** A queue turns one member's enthusiasm into
everyone else's workload — forty songs in a weekend is forty items at the front of nine other
queues. A rate limit was chosen over round-robin queue ordering (more code, harder to explain)
and over a cap on in-flight submissions (punishes the most engaged member, no non-arbitrary
value). Rolling rather than calendar-day: no midnight hovering and no timezone question.
34. **Failed and discarded submissions never count against the quota.** No audio, no submission.
yt-dlp rot is expected and periodic (19), and burning someone's day on your own infrastructure's
failure is the wrong lesson.
35. **All nine leaderboards stay, with deterministic tie-breaks.** The count-based ones (Most
Reviewed, Most Active) go stale once everyone drains the queue and everything ties, but they are
a query each and genuinely differ in the early months when people actually look. Prune them when
they are proven useless. The tie-break is not optional: ties plus `LIMIT 10` gives unstable
ordering, and a leaderboard that reshuffles between reloads reads as a bug.
36. **Title, artist and genre are required to publish; genre joins the editable-while-unlocked set.**
Prefill can legitimately produce nothing (a real yt-dlp dump of an ordinary upload has no
`track`, `artist` or `album`), so publishing has to validate or a nameless song reaches the feed.
Validation happens at publish, not at submit, because the form is meant to be filled during the
conversion wait. Genre is editable because it is the one field with no prefill at all — picked in
a hurry, most likely wrong.
37. **No avatar renders as initials in a circle, in CSS.** `avatar_url` is `null` and
`/avatars/{id}` 404s. No default image on disk, no identicon generator, and ten initials
distinguish ten people better than ten grey silhouettes.
38. **Build order is admin → pipeline → product loop → YouTube → surroundings.** Registration needs
an invite and invites come from the admin panel, so the admin surface exists before any member
can. The upload path is built and proven before yt-dlp is added, so the worker's failure modes
are not debugged alongside the network's. The app is usable by ten people at step 4.
39. **`HANDOFF.md` is deleted.** It was a relay document from the Nuxt project. Its lasting content
lives in `CONTEXT.md` (language), `docs/spec.md` (behaviour), this file (why), and
`docs/later.md` (deferred). Its case for the rewrite and its notes on the old app were
archaeology once the decision was made.
40. **`main` is release code, `dev` is development.** Work lands on `dev` and reaches `main` by merge
at release, so `main` is always a list of things that shipped. Nightly builds, if any, come off
`dev`.
42. **The theme handoff is implemented as CSS custom properties, not a Tailwind config.** Its
palette, spacing, shadows, motion and component shapes are followed as written; the parts that
assumed Tailwind, Pico or cover artwork are adapted rather than dropped, and each adaptation is
listed in [theme.md](./theme.md). Oswald's phantom weight 900 resolved to 700 — loading a weight
you do not have is what made the brand render differently per platform. The custom audio player
stays deferred: `color-scheme: dark` makes the native control fit the palette, which was the
actual complaint.
41. **No password minimum; rate limit logins instead.** A length policy protects against guessing,
and guessing is better answered directly: 10 failures per email in 15 minutes, then a 15-minute
lockout, cleared by a correct password. The floor was rejected because typing an 8-character
password on every dev account is friction with nothing behind it — there is no public
registration to spray, and the admin is the reset path. It was also deliberately *not* made
configurable: settings like this belong in code, not in an env file that grows a line per
preference. The limiter is keyed by email rather than IP (a proxy would mean trusting
`X-Forwarded-For`) and locks the *attempt rate*, not the account, so nobody can lock someone
else out by trying.
43. **The JSON API is deferred entirely, not built on demand.** Decision 17 kept the contract fixed
and expected handlers to appear one at a time; in practice nothing consumes `/api` at all, so
even that trickle would be handlers with no callers, plus golden tests guarding shapes nothing
reads. The contract in [spec.md](./spec.md) stays as the design — it is what stops the shape
changing under a future client — and the first endpoint gets built the day something actually
calls it. Both surfaces being thin adapters over one data function is already true of the page
handlers, so adding the JSON side later stays a one-line-per-route job.
44. **CalVer, `YYYY.MM.DD-N`, injected at build time.** The date is the useful part: this app ships
when there is something to ship and gets rebuilt monthly for yt-dlp anyway, so a semantic
version would communicate nothing a date does not. `N` is the build number within that day,
starting at 1, for the second attempt at a release. The string lives in a git tag and reaches
the binary through `-ldflags`, so no file in the repo has to be bumped and a local build
honestly reports `dev`. It surfaces in the footer, the startup log and `/healthz`.
45. **Lyrics are suggested at submission, and stay editable forever.** Four decisions in one, taken
2026-07-31 while scoping the feature in [later.md](./later.md):
- **Suggested, never imposed.** The worker attempts one LRCLIB lookup after conversion, and the
waiting page carries a *Hae sanoitukset* button that re-queries with whatever title and artist
are currently typed. The button exists because our metadata comes from ID3 tags and YouTube
uploaders, so the automatic attempt misses exactly the songs with messy names — and would look
broken rather than absent. Neither path overwrites text the submitter has typed.
- **Lyrics live on the submission, not just the song**, and are copied across at publish, because
they are part of preparing a song rather than something bolted on afterwards.
- **No migration 002.** Nothing has launched, so the column goes into `001_init.sql` and the
database is recreated. The schema has no legacy to respect until there is data worth keeping.
- **The lock does not cover lyrics.** It exists so the thing people reviewed stops changing under
them, and nobody reviewed the lyrics — the rule is now *the lock freezes what the song claims
to be; lyrics are an attachment to it.* This also allows pasting lyrics for an old song, which
is when the feature is worth most.
The coverage assumption that shaped the earlier sketch was wrong and is corrected in `later.md`:
LRCLIB has synced lyrics for a good share of Finnish rock, not almost none.