CI / check (push) Skipped
CI / e2e (push) Skipped
main is protected (enable_push: false, empty whitelist), so @semantic-release/git's commit-back is rejected by the pre-receive hook -- and it would be rejected *after* the tag was pushed, leaving a tagged release the run then reports as failed. Found by trying to push this branch to main. Whitelisting the CI user was the alternative and is declined: it weakens a protection someone set deliberately and lets a bot push to main without the checks every human PR has to pass. So the release page is the changelog. The changelog plugin now writes a gitignored .release-notes.md, which exists only to carry the notes into gitea-release.sh without interpolating them into a shell command, and CHANGELOG.md is a signpost -- a file claiming to be a changelog while silently never updating is worse than no file. Tags are not protected, so the tag push is unaffected.
2279 lines
134 KiB
Markdown
2279 lines
134 KiB
Markdown
# CLAUDE.md
|
||
|
||
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
|
||
|
||
## Project
|
||
|
||
YellowJacket is a cross-platform desktop music player built with Go (backend) and TypeScript/Lit (frontend), using the Wails framework to bridge them. It supports MP3, FLAC, OGG Vorbis, and WAV playback.
|
||
|
||
## Planning
|
||
|
||
Active and historical plans live in `.planning/`:
|
||
|
||
- `.planning/NOTES.md` — gotchas, deferred items, open architecture questions, the "we already considered and rejected" list.
|
||
- `.planning/plans/active/` — work currently in progress (read first).
|
||
- `.planning/plans/pending/` — sequenced future work.
|
||
- `.planning/plans/completed/` — one concise recap per shipped milestone.
|
||
|
||
Numbering is sequential and stable across status moves (a plan keeps its `NNN-` prefix as it migrates between `pending → active → completed`). Abandoned plans are deleted; paused work stays in `pending/`.
|
||
|
||
## Commands
|
||
|
||
```bash
|
||
make dev # Hot-reload development (installs deps, generates code, cleans frontend)
|
||
make dev-debug # Same as dev but with YJ_LOG_LEVEL=debug
|
||
make dev-headless # Start headless in the background and return (SEED=<name> to seed)
|
||
make dev-stop # Stop it (SIGTERM, so shutdown hooks run)
|
||
make dev-logs # Tail .dev/app.log
|
||
make testdata # Generate the deterministic fixture music library
|
||
make bulkdata # Generate the ~50k-track measurement library (BULK_TRACKS=)
|
||
make sandbox-seed NAME=<n> # Build a seeded YJ_HOME by *running* the app
|
||
make sandbox-seed-bulk # Same, from the bulk library (minutes; it is a real scan)
|
||
make perf LABEL=<n> # Measure a running app; writes .dev/perf/<n>.json
|
||
make perf-compare BEFORE=<a> AFTER=<b> # Print the before/after table
|
||
make build-dev # Debug build with symbols
|
||
make build-prod # Production build (stripped and trimmed; no UPX)
|
||
make generate # Run code generators (sqlc + templ via go generate)
|
||
make e2e # Playwright smoke suite against a running dev-headless app
|
||
make e2e-setup # Install the e2e runner + its browser (once)
|
||
make ui-test # Vitest component/store suite in a real browser (no app)
|
||
make ui-visual # Same, including toMatchScreenshot comparisons
|
||
make ui-setup # Install the Vitest provider's own Chromium (once)
|
||
make bindings-check # Fail if frontend/bindings is stale vs the Go bindings
|
||
make skill-check # Fail if .pi/ documents a make target that doesn't exist
|
||
make commit-check # Fail if a commit subject is not a Conventional Commit
|
||
make lint # golangci-lint v2 (strict), all three build configurations
|
||
make test # All tests with race detector, all three build configurations
|
||
make vulncheck # govulncheck for CVEs
|
||
make setup # Install go tools, frontend deps, git hooks (lefthook)
|
||
```
|
||
|
||
### Running tests
|
||
|
||
Go test commands need no build tag for the app configuration. The
|
||
`webkit2_41` tag every command here used to carry is gone with wails
|
||
v2: v3 builds against GTK4 + WebKitGTK 6.0 by default, which both Arch
|
||
and ubuntu:24.04 ship.
|
||
|
||
```bash
|
||
go test ./... # All tests
|
||
go test ./backend/player/ # Single package
|
||
go test -run TestName ./backend/player/ # Single test
|
||
```
|
||
|
||
The central index builder is behind a second tag and is **not** covered
|
||
by the command above — `make test` runs both passes, but a manual run
|
||
needs it spelled out:
|
||
|
||
```bash
|
||
go test -tags indexbuild ./backend/explore/... ./cmd/...
|
||
```
|
||
|
||
`backend/testctl` is behind a third tag and needs its own pass too
|
||
(`make test` runs all three):
|
||
|
||
```bash
|
||
go test -tags dev ./backend/testctl/...
|
||
```
|
||
|
||
Audio playback integration tests require `YELLOWJACKET_INTEGRATION=1`.
|
||
|
||
### Fixtures and the headless harness
|
||
|
||
`test_data/music_library_test/` is **generated, not committed**: run
|
||
`make testdata` (~1 s) before anything that needs audio. Tests reach it
|
||
through `internal/testfixtures`, selecting files by *case*
|
||
(`CaseCoverDedup`, `CaseUnicode`, `CaseDuplicates`, …) rather than by
|
||
path, and skip themselves when it has not been generated.
|
||
|
||
The app itself can be run without a blocking window — `make
|
||
dev-headless` — and driven with `playwright-cli` against it on
|
||
`:34115`. That is wails v3's first-class `-tags server` mode: the real
|
||
app, the real bindings, the same Go backend a desktop window would use,
|
||
served over HTTP with **no display at all**. The Xvfb this used to
|
||
require is gone, from the script and from CI; `dbus-run-session` stays,
|
||
for MPRIS.
|
||
|
||
**The operational half of all this lives in the
|
||
`yellowjacket-dev` skill** (`.pi/skills/yellowjacket-dev/`): which tier
|
||
to reach for, the exact command sequences, seed lifecycle, and the
|
||
failure modes worth knowing before you meet them. It is deliberately
|
||
not repeated here — this section describes what exists, the skill says
|
||
what to run.
|
||
|
||
Two things ride on top of the headless launch, both from plan 005
|
||
phase 3:
|
||
|
||
- **The event bridge.** `.playwright/cli.config.json` loads
|
||
`.playwright/init-events.js` as an `initScript`, which records every
|
||
backend event on `window.__yjEvents`. Half this app is push-driven,
|
||
so assertions **await an event, not a timeout**:
|
||
`await window.__yjEvents.wait('LibraryScanComplete', {timeoutMs: 60000})`.
|
||
It also provides `ready()` and `call('queue.Queue.GetState', [])`.
|
||
Both hook v3's own seams rather than an internal: inbound is
|
||
`window._wails.dispatchWailsEvent`, the entry point the backend's push
|
||
uses, and outbound is **`fetch`** — v3 routes every runtime call
|
||
through one POST to `/wails/runtime`, so one hook sees calls from any
|
||
module and cannot miss one made before the harness looked. `call()`
|
||
posts by method name, so it depends on nothing in the app's bundle and
|
||
works on a page with no init script — which is why `seed-sandbox.sh`
|
||
is `curl` now and needs no browser. It no longer races a timeout
|
||
either: v3 rejects bad arguments and unknown methods cleanly.
|
||
- **The dev-only control surface**, `backend/testctl`, mounted at
|
||
`/__test/` on the same port: `health`, `db/snapshot`, `db/restore`,
|
||
`emit` (force any backend event, which renders push-driven views
|
||
without staging the work that would produce them) and `sql`. It is
|
||
compiled out of non-dev builds and additionally requires
|
||
`YJ_TESTCTL=1`, which `dev-headless.sh` sets and `make dev` does not.
|
||
|
||
Frozen regression specs live in `e2e/` (its own npm package, so the
|
||
Vitest browser mode does not share a package with the Playwright
|
||
runner): `make e2e` against an already-running app.
|
||
|
||
**A fifth tier exists for questions whose answer is a number**, not a
|
||
pass: `make bulkdata` generates a ~50 000-track library (11 s, 466 MB,
|
||
gitignored), `make sandbox-seed-bulk` seeds from it by running the app
|
||
like any other seed, and `make perf LABEL=x` measures startup, the
|
||
bundle's shape and each view's first open, keystroke cost, what a
|
||
finished track provokes, what one favourite toggle costs, what sitting
|
||
idle on Settings costs, and heap after a scripted browse. A measurement
|
||
that needs state the seed does not have stages it itself, idempotently,
|
||
so a before and an after see the same shape — the favourite number is
|
||
meaningless against the seed's one empty playlist, so it builds ten
|
||
500-track ones first.
|
||
It wraps every bound Go method, so "did that refetch the library" is a
|
||
fact rather than an inference. It is not a spec and does not run in CI.
|
||
|
||
**The cheapest tier needs none of that.** `make ui-test` runs 776
|
||
Vitest tests in a real Chromium with no Wails, no backend, no seeded
|
||
library and no virtual display, because **v3 routes every runtime call
|
||
— bindings, event emits, window, dialogs, clipboard — through one IPC
|
||
transport**, and `frontend/test/support/wails-fake.ts` replaces it via
|
||
`setTransport()`, which is public documented API. So the fake covers
|
||
strictly more than v2's two globals did while being shorter, and the
|
||
tests exercise the real generated bindings, the real runtime and the
|
||
real store code. A binding carries an **ID**, not a name
|
||
(`$Call.ByID(2822423495)` is FNV-1a over
|
||
`yellowjacket/backend/home.Service.GetShelves`), so the fake derives
|
||
that map from the generated tree rather than writing it down.
|
||
|
||
**`frontend/bindings/` is generated by `wails3`, not `go generate`**, so
|
||
the pre-commit codegen check does not cover it. `make bindings-check`
|
||
(~3.5 s warm, ~20 s on a cold build cache, also a pre-commit hook)
|
||
regenerates it and fails on a dirty tree; `make bindings` regenerates it
|
||
for real. It is slower than v2's because v3's generator is a **static
|
||
analyser** over the whole package graph rather than runtime reflection
|
||
— which is also why the tag set it runs under matters, and why it is
|
||
the *default* one: that is the configuration users run, and neither
|
||
`indexbuild` nor `dev` adds a bound service. See
|
||
`scripts/bindings-check.sh`.
|
||
|
||
**Seeds are produced by running the app**, never by hand-writing a
|
||
`config.toml` and DB rows — the same discipline `sql/schemas/` gets,
|
||
for the same reason.
|
||
|
||
See `.planning/plans/completed/005-agent-development-harness.md`.
|
||
|
||
## Architecture
|
||
|
||
**Wails app lifecycle** (`main.go` → `backend/app.go`): `main.go` is
|
||
`application.New(opts)` → `app.Window.NewWithOptions(…)` → `app.Run()`.
|
||
Each bound service takes its context from `ServiceStartup(ctx,
|
||
application.ServiceOptions{})` — v3 calls it on every service, in
|
||
registration order, on the main goroutine — and gives it back in
|
||
`ServiceShutdown()`. That context is **cancelled on app shutdown**,
|
||
which `SetContext` never was.
|
||
|
||
Three things about it are load-bearing.
|
||
|
||
**`ServiceShutdown()` takes no context.** A method with a
|
||
`context.Context` parameter does not satisfy the interface and is
|
||
**silently never called** — no error, no warning.
|
||
|
||
**The cross-service wiring is a service, not an event.** v3 has no
|
||
`OnStartup`/`OnDomReady` option, and the obvious replacement —
|
||
`app.Event.OnApplicationEvent(events.Common.ApplicationStarted, …)` —
|
||
is the right *moment* and the wrong *mechanism*: **server mode emits no
|
||
application events at all** (`setupCommonEvents` is an explicit no-op
|
||
under `-tags server`), so the desktop build wired itself and the
|
||
headless harness did not. `backend/startup.go` is registered last
|
||
instead, which takes the ordering from the mechanism rather than from
|
||
an event and therefore holds in every mode. Anything else keyed on
|
||
`Common.*` is suspect for the same reason.
|
||
|
||
**The quit veto is asynchronous now.** v2's `MessageDialog` blocked and
|
||
returned the button; v3's `Show()` returns immediately and the answer
|
||
arrives on a `Button.OnClick` callback, so `ShouldQuit` cannot ask and
|
||
answer in one call — it vetoes, shows the dialog, and calls
|
||
`app.Quit()` from the callback. `quitConfirmed` is what stops that
|
||
second `Quit()` asking again; `quitAsking` stops a second close attempt
|
||
stacking dialogs. Window state moved off that path entirely, onto a
|
||
`Common.WindowClosing` hook, because the size has to be read while the
|
||
window still exists and `OnShutdown` has neither a context nor a
|
||
window.
|
||
|
||
`internalServiceMethods` auto-excludes `ServiceStartup`,
|
||
`ServiceShutdown`, `ServiceName` and `ServeHTTP` from bindings, so this
|
||
shape **removed** 12 spurious bindings and the bogus `context` model
|
||
rather than renaming them.
|
||
|
||
**Backend packages** (under `backend/`):
|
||
- `player` — Audio playback via beep. `BufferedStreamer` provides a ring buffer for smooth seeking.
|
||
- `queue` — Track queue with shuffle (Fisher-Yates), repeat modes, auto-advance, and session persistence.
|
||
- `library` — Concurrent library scanning, metadata extraction, cover art deduplication, incremental rescan. Also **removal**, below.
|
||
- `database` — SQLite via pure-Go driver. Schema in `database/sql/schemas/`, queries in `database/sql/queries/`. **sqlc** generates Go code into `database/sql/sqlcgen/` — never edit that directory by hand.
|
||
|
||
**The schema is one description, and there is no migration chain.**
|
||
`sql/schemas/*.sql` is `CREATE ... IF NOT EXISTS`, declares the
|
||
current shape of every table, and is what sqlc reads and what an
|
||
install gets verbatim. That is the whole mechanism: `sql/migrations/`,
|
||
`applyMigrations` and `schema_migrations` were squashed away with the
|
||
file-shaped rewrite (plan 013). A schema change is one edit to one
|
||
file plus `make generate`. Reintroducing a chain means reintroducing
|
||
the drift it caused before — `sql/schemas/` and the migrations
|
||
disagreed, and sqlc generated against the stale one.
|
||
|
||
**What that costs an existing database is repaired once, at open.**
|
||
`CREATE ... IF NOT EXISTS` reaches an existing table only if its shape
|
||
already matches and otherwise silently no-ops, so a *changed* table
|
||
never migrates. Plan 014 added `total_tracks` to `explore_index` and
|
||
to `indexRowFields` — the projection every explore read uses — and no
|
||
database that already existed grew the column: **every** Explore
|
||
search, browse, artist and album page on such an install failed with
|
||
`no such column: total_tracks`, while a fresh install was perfectly
|
||
healthy, which is exactly why no test saw it. Plan 013 was worse on
|
||
the same install: `applySchema` could not be applied at all over a
|
||
pre-013 `audio_files`, so the app did not open.
|
||
|
||
`backend/database/staleshape.go` runs before `applySchema` and
|
||
retires what is stale, so the create is a create. Five things about
|
||
it are load-bearing:
|
||
- **It parses `sql/schemas/` for the expectation** rather than
|
||
writing the column list down a second time, because a second list
|
||
is a second thing to forget — the fault it exists to repair.
|
||
- **It notices a changed *type*, not just a missing column.** 013
|
||
moved `mbid` from TEXT to BLOB, and SQLite does not coerce between
|
||
them: a comparison against 16 raw bytes returns no rows rather than
|
||
an error. `ALTER TABLE ADD COLUMN` would have handled
|
||
`total_tracks` alone and cannot express this at all, which is why
|
||
the repair drops rather than migrates.
|
||
- **`Authored` is never retired**, and that boundary is a test
|
||
(`TestAuthoredTablesAreNeverRetired`), not a comment. Everything
|
||
else is rebuildable: `Cache` by definition, `Owned` by a rescan —
|
||
plan 013's stated "delete and rescan" — and `Derived` from Owned.
|
||
A table the schema no longer describes at all goes too; 013 left
|
||
seven behind plus `schema_migrations`.
|
||
- **Whether a stale `Cache` table may be rebuilt is a build tag**, and
|
||
it is the most expensive thing in this file to get wrong. In the app
|
||
the catalog is *downloaded*, so a wrong shape costs a minute of
|
||
re-fetching the artifact and keeping it costs every Explore read. In
|
||
`cmd/indexbuild` the catalog is *derived*, and the only way back is
|
||
the ~205 GB dump stream the `/cache` volume exists to avoid — so
|
||
`retireStaleCache` is false there (`staleshape_policy_indexbuild.go`)
|
||
and `TestTheCatalogSurvivesAStaleShape` fails the moment it is not.
|
||
`TestNoCacheTableIsRetiredHere` is the same assertion made of *every*
|
||
`datamap` Cache table rather than one, because the risk is not that
|
||
shape recurring — it is the next destructive repair added to
|
||
`database.NewDB`, the chokepoint every binary here shares, without
|
||
asking which binary it is in.
|
||
This is written down because it already happened: the repair shipped
|
||
without the distinction and dropped the real CI catalog on its first
|
||
run, with `reason="column entity_type is TEXT, schema declares
|
||
INTEGER"`. The mismatch was genuine — that database is deliberately
|
||
kept in the older encoding, which `fix(indexexport): read an index
|
||
older than the binary` exists to tolerate — so it would have been
|
||
dropped on *every* run. The consequence is that a future
|
||
`explore_index` column fails the index job loudly on `applySchema`
|
||
rather than silently costing it a rebuild, which is the trade a
|
||
human should get to make.
|
||
- **The drops are one transaction with `defer_foreign_keys`.** Those
|
||
legacy tables reference each other, so dropping them in any order
|
||
fails on whichever goes first, and turning foreign keys *off*
|
||
instead would silently take `playlist_tracks.audio_file_id`'s
|
||
ON DELETE SET NULL with it — leaving playlist entries pointing at
|
||
ids a rescan reissues to *different songs*. Nulled entries are
|
||
empty; stale ones are wrong, and wrong quietly.
|
||
- **The order is sorted, so a failure reproduces.** Map order is
|
||
random, and the foreign-key bug above passed its own regression
|
||
test on two runs in three until the order was fixed.
|
||
|
||
Retiring `explore_index` takes its FTS and its meta with it, because
|
||
the `dump_import_done` marker is what would otherwise stop the
|
||
artifact ever being fetched again.
|
||
|
||
**What that costs an existing database is that it does not open**, and
|
||
"delete and rescan" is the answer (plan 013, open question 1) — free
|
||
for everyone except one machine. The index job's `/cache` volume is a
|
||
real `YJ_HOME` that survives between runs, and half of it is the
|
||
catalog: deleting it means re-downloading ~205 GB. So `cmd/indexbuild`
|
||
repairs it instead (`staleschema.go`), dropping every table `datamap`
|
||
does not classify as `Cache` **before** the schema is applied. Nothing
|
||
scans, plays or authors in that database, so its non-catalog half is
|
||
empty by construction and a shape left over from an older schema is
|
||
pure liability. 013's reshaped `audio_files` failed every run of that
|
||
job on `CREATE INDEX ... album_id` against the old table until this;
|
||
`TestRetireLibraryTables` reproduces exactly that, symptom first.
|
||
|
||
**The local library is shaped like files, not like MusicBrainz.**
|
||
`audio_files` carries its own tags — title, artist credit, track and
|
||
disc numbers, year, composer, the recording MBID — and points at two
|
||
shared rows: `albums` (many files to one) and `artists` (many albums
|
||
to one). `file_genres` is the one genuine many-to-many. That is the
|
||
entire local model.
|
||
|
||
It used to be MusicBrainz's: `recordings`, `release_group_recordings`,
|
||
`artist_credit` and `artist_credit_artist` sat between a file and its
|
||
own tags. Measured on a real 25,966-file library, **every**
|
||
many-to-many that model expressed was 1:1 in the data — no recording
|
||
had two files, none belonged to two release groups, and 3 credits of
|
||
2,823 listed more than one artist. What it cost was a six-way join in
|
||
every read, a `MIN(release_group_id)` subquery in eleven queries and a
|
||
first-credited-artist subquery in nine to undo fan-outs that never
|
||
happened, and a class of bugs where a metadata row **outlived the file
|
||
that created it**: retagging a file created a new recording and
|
||
abandoned the old one, so that library carried 812 orphaned
|
||
recordings, 216 release groups and 260 artists — and 129 catalog rows
|
||
that confidently claimed to be owned by files that no longer existed.
|
||
|
||
A few things that follow, and bite if forgotten:
|
||
- **Ownership is a file.** "Do I own this" is asked of `audio_files`
|
||
and nothing else — `GetFilePathsByRecordingMBIDs`,
|
||
`LibraryMBIDIndex.CheckMBIDs`, `collectLibraryEntities` and
|
||
`pruneStaleLocalCrossReferences` all join it. A metadata row is not
|
||
ownership; that was the bug, and it is now structurally impossible
|
||
for a row to exist without its file.
|
||
- **The projection is defined once, in the `track_metadata` view.**
|
||
Every query that returns a track selects from it, which is why
|
||
there is one row type (`sqlcgen.TrackMetadatum`) and one mapper
|
||
(`trackFromRow`). It existed before and only the raw-SQL search
|
||
paths used it, so nine hand-rolled copies had already drifted: the
|
||
view preferred the album's original year and one copy used the
|
||
track's, and the same library reported different years on different
|
||
screens. The FTS searches cannot be sqlc queries (MATCH is not in
|
||
its grammar) and spell the column list out in `search.go` — that is
|
||
the one exception and it is one constant.
|
||
- **`library_id = 0` means every library.** Each list query used to
|
||
exist twice, scoped and unscoped, with a branch at every call site
|
||
and a separate binding for each. One query answers both, and the
|
||
scoped form costs nothing measurable (23 ms against 21 ms over 26k
|
||
rows).
|
||
- **A slice and a named parameter do not compose in sqlc.**
|
||
`sqlc.slice` expands to N placeholders but a named argument is
|
||
numbered independently, so `GetFilePathsByAlbums([1,2], 0)` read
|
||
album id 2 as the library id. Where a query needs both, return
|
||
`library_id` and filter in Go (`inLibrary`).
|
||
- **A cache without a ceiling is a leak with a schedule.** Every
|
||
store that grows with use declares a budget beside its retention
|
||
(`browsedArtBudget`, `httpCacheBudget`), because an age bound does
|
||
not bound anything a user can outrun in an afternoon.
|
||
- **A query file must be ASCII.** sqlc's parameter rewriter works on
|
||
byte offsets, so one non-ASCII character in a *query* comment
|
||
corrupts the generated Go into garbage like `SELECid`. Schema files
|
||
are not rewritten and may contain anything.
|
||
- **A view is dropped and recreated, not migrated.**
|
||
`CREATE VIEW IF NOT EXISTS` no-ops against a database holding the
|
||
old definition, so `track_metadata.sql` opens with
|
||
`DROP VIEW IF EXISTS`; a view holds no data, so rebuilding it on
|
||
every open costs nothing.
|
||
- **A write wearing a query's shape still needs the writer.**
|
||
`DB.QueryContext`/`QueryContextWith`/`QueryRow` route to a
|
||
*query-only* read pool (a second `sql.DB` over the same file), so
|
||
an `INSERT ... RETURNING` issued through one fails at runtime with
|
||
"attempt to write a readonly database (8)" — which is exactly what
|
||
`CreateSmartPlaylist` did, meaning no smart playlist could be
|
||
created at all. Use `ExecContext`, or `QueryRowWriter` when the
|
||
statement really does return a row. Nothing caught this because
|
||
`NewTestDB` shares one in-memory connection and leaves `readDB`
|
||
nil, so `reader()` returns the *writer* under test.
|
||
`TestNoWritesOnTheReadPool` walks the tree for it, in the same
|
||
spirit as `TestNoDirectRuntimeEmits` and for the same reason — a
|
||
lint pass only sees one build configuration.
|
||
- **A new table has to say what kind of data it holds.**
|
||
`backend/datamap` is a catalogue of every table's Kind and
|
||
Lifetime, and `TestCatalogCoversSchema` fails on a table missing
|
||
from it. `TestAuthoredCascadesAreDeliberate` then makes an
|
||
*authored* table that cascades an explicit, argued exemption —
|
||
authored data is what a user cannot get back. Two entries say
|
||
**MIXED KIND** and mean it: `audio_files` is an owned projection
|
||
except for `play_count`, `last_played` and `tag_status`, which are
|
||
authored; `lyrics` carries a `source` column because a lyric read
|
||
from a tag is free to rebuild and one fetched from LRCLIB is not.
|
||
- **Test data has one seeder.** `database.InsertTestTrack` inserts a
|
||
file with its artist, album and genres. Twenty test files used to
|
||
carry their own, each assembling the old FK chain in a slightly
|
||
different order.
|
||
- `metadata` — Tag extraction (ID3v2, Vorbis Comments, FLAC).
|
||
- `jobs` — The registry every long-running operation reports through:
|
||
progress, pause/cancel, a global indicator and (for scans) a pause
|
||
that survives a restart. Library scans, index builds, downloads and
|
||
the autotag apply are registered; anything that is not registered has
|
||
none of that, which is exactly how the three gaps the audit found
|
||
came about.
|
||
- `config` — TOML-based settings. Settings page uses HTMX + templ for server-rendered HTML fragments.
|
||
- `playlist` / `smartplaylist` — Playlist CRUD and rule-based smart playlists.
|
||
- `mediacontrols` — OS media controls behind one `Handler`: MPRIS over
|
||
D-Bus on desktop Linux, a MediaSession on Android, a no-op stub
|
||
elsewhere. The split is by build tag and `android` implies `linux`,
|
||
so the three files read `linux && !android`, `android` and `!linux`.
|
||
Its Android half needs no JNI beyond what Wails exports — a JSON
|
||
payload out through `application.Android.StartForegroundService`, a
|
||
command event back through `WailsBridge.emitEvent` — and the Java it
|
||
talks to is `build/android/.../WailsForegroundService.java`. That
|
||
contract (payload keys, state words, command names) is in
|
||
`androidpayload.go` **without** the build tag, because a tagged file
|
||
is compiled by nothing `make lint` or `make test` runs and is
|
||
untestable off a phone.
|
||
|
||
`OnDuck` is the one callback MPRIS does not use: Android asks for
|
||
attenuation rather than a pause when something short needs the
|
||
output. `Player.SetDuck` keeps it as an offset on top of the user's
|
||
level rather than writing through to the volume, so it cannot
|
||
accumulate and nothing persists or emits a level the user did not
|
||
choose — and it only ever fires below API 26, where the framework
|
||
does not already duck the app itself.
|
||
- `system` — OS-specific paths (XDG on Linux, `%LOCALAPPDATA%` on Windows).
|
||
- `explore` — Catalog search and browse over `explore_index`. See below.
|
||
Its **shelves** (`shelves.go`) are the page Explore shows before
|
||
anyone types, on `home`'s terms — a shelf is a reason, it carries the
|
||
sentence that says so, and an empty one is omitted. Queries return
|
||
`explore_index` row ids and are joined back by `rowsByIDs`, so a card
|
||
has one definition (`artistFromIndex` / `releaseGroupFromIndex` /
|
||
`recordingFromIndex`, shared with the search path, which is where
|
||
they were inlined).
|
||
|
||
Three things about it are load-bearing. **"No shelves" is three
|
||
different statements here and the page says which** — Home can omit
|
||
an empty shelf honestly, because a library with no history really has
|
||
less to say, but Explore's data is a *downloaded artifact* that can
|
||
be absent or still arriving, so `ShelfPage.State` is `ready`,
|
||
`building` or `no-index` and the empty page names the missing catalog
|
||
and points at Settings. **Whether there is a catalog is asked of the
|
||
database, not of a flag**: `GetIndexStatus().TotalRows` is refreshed
|
||
only between build tiers (0 beside a full catalog on an ordinary
|
||
launch) and `IsReady()` is set once at startup (so rows staged by a
|
||
spec afterwards are invisible) — both are the shape `emitStatus`
|
||
warns about, and one `SELECT 1 … LIMIT 1` cannot be stale. And **two
|
||
shelves with disjoint ids still repeat each other**: ordered by raw
|
||
listen count the top albums are one act and its members and the
|
||
artists row underneath was the same people, which `home`'s
|
||
duplicate guard cannot see because the rows hold different entity
|
||
types. Shelves are one album per artist and skip whoever a row above
|
||
already showed. Found by reading a screenshot.
|
||
|
||
Two of the four shelves the plan named **cannot be built**, and the
|
||
schema decides that rather than the design: `explore_index` has no
|
||
genre column to join a genre shelf to, and `similar_artist_map` is
|
||
not in the shipped artifact and is filled lazily from the network, so
|
||
a "similar artists" shelf is empty exactly when the page most needs
|
||
content. The library-joining shelf reads `in_library`, which is set
|
||
by MBID, so it is correctly absent on an untagged library — the
|
||
fixture one included.
|
||
- `home` — The home page's "start listening" shelves. Each shelf is a
|
||
*reason* (what you played last, what you never played, a genre you
|
||
have depth in) rather than a filter, and carries the sentence that
|
||
says so. Its queries (`sql/queries/home.sql`) return album ids only
|
||
and are joined back to `GetAllAlbumsWithDetails` in Go, so the album
|
||
projection has one definition. A shelf with nothing behind it is
|
||
omitted, never rendered empty — **and so is a shelf that repeats the
|
||
one above it**, which is the same rule one step further: "On repeat"
|
||
was "Pick up where you left off" reordered, because a small library
|
||
has one signal and answers several questions with the same albums.
|
||
Two guards make that safe, and both were arrived at by breaking the
|
||
existing tests: only shelves of three or more albums are judged (two
|
||
rows of one overlap by 100% whenever they agree at all), and only
|
||
when the shelf is **not showing the whole library** — a repeat is a
|
||
fault only if a different row was possible. Measured against a fixed
|
||
shelf size instead, an 11-album library kept three identical shelves
|
||
while a 13-album one lost them.
|
||
|
||
**The app lands here**, from `index.ts` after the stores are wired.
|
||
`index.html` still renders the track list eagerly and it is still what
|
||
paints first — it is the cached `tracks` view, so the navigation is a
|
||
class toggle plus one chunk rather than a second render of the shell.
|
||
`app-sidebar`'s default `activeView` is `home` to match, because the
|
||
sidebar does not hear a `navigate` it did not send.
|
||
- `profiling` — pprof server on `:6060`, compiled out in non-dev builds via build tags (`internal/dev/`).
|
||
|
||
**Explore catalog** (`backend/explore/`): the searchable MusicBrainz/
|
||
ListenBrainz catalog in `explore_index`. Deriving it from the MetaBrainz
|
||
dumps means streaming ~89 GB from a server that caps a client near
|
||
2 MB/s — half a day, for a catalog identical for every user. So that
|
||
work happens **once, centrally**, and users download the result:
|
||
|
||
- `cmd/indexbuild` builds the catalog from the dumps; `cmd/indexexport`
|
||
cuts it down to a shippable core and stamps its provenance.
|
||
`.gitea/workflows/index-artifact.yml` runs both and publishes the
|
||
compressed artifact under a fixed `latest` version.
|
||
- The app fetches and merges that artifact (`artifactfetch.go`,
|
||
`artifactimport.go`) — about a minute, versus a day.
|
||
- Everything the app does **not** need is behind the `indexbuild` build
|
||
tag (`dumpimport.go`, `dumpcounts.go`, `dumpcatalog.go`,
|
||
`dumpproject.go`, `dumpparallel.go`, `indexpatch.go`) so it is not
|
||
linked into the binary. `dumpbuild_stub.go` is the app-side entry
|
||
point; `dumpshared.go` holds what both sides use.
|
||
- The app keeps popularity current with the daily incremental dumps
|
||
(`dumpincremental.go`), and resolves artists outside the artifact's
|
||
coverage lazily on first view.
|
||
|
||
**The catalog stores ids as bytes, and that is a size decision.**
|
||
`explore_index` is 2,052,200 rows, and its MBIDs and entity types were
|
||
half of it: three 36-character text columns and one storing the words
|
||
"artist", "release_group", "recording" two million times. They are 16
|
||
raw bytes and a small integer now. Measured on a real catalog, the
|
||
table and its six indexes went **780 MB to 405 MB** — the largest
|
||
single saving available in this app, and the reason a fresh install is
|
||
~0.6 GB rather than ~1.0 GB.
|
||
|
||
`backend/explore/mbid.go` is the only place that encoding is known.
|
||
Everything above it speaks dashed strings and entity names —
|
||
`SearchIndexResult`, the bindings, the frontend — and `dbMBID` /
|
||
`dbEntityType` convert at the SQL boundary. That confinement is the
|
||
point: the alternative is blobs reaching code that has no use for them.
|
||
|
||
Four things about it are load-bearing, and they exist because of *how*
|
||
this fails when it fails: **SQLite does not coerce between TEXT and
|
||
BLOB**, so a query comparing the column against a 36-character string
|
||
returns no rows rather than an error, and a scan into a plain string
|
||
yields sixteen bytes of mojibake. Neither is visible except as a result
|
||
that is quietly empty.
|
||
|
||
- **The column checks itself.** `CHECK(length(mbid) = 16)` means a
|
||
stringly *write* fails at the insert that made it. It also caught
|
||
every fixture that had been using `"rh"` as an MBID; `testMBID()`
|
||
hashes a label into a real one so they stay readable.
|
||
- **The projection is one constant and one scanner.**
|
||
`indexRowColumns` / `scanIndexRow` replaced four copies of a 22-column
|
||
list and four matching `Scan` calls — four chances to decode wrongly.
|
||
`indexRowColumnsFor("i")` is the same list qualified, for the FTS join
|
||
where both sides have a `title`.
|
||
- **A query that names an entity type inline writes the code with the
|
||
name beside it** — `entity_type = 1 /* artist */`. Splicing a Go
|
||
constant in would keep them in step automatically but makes every such
|
||
query a concatenation; `TestEntityCodesAreStable` pins the mapping
|
||
instead, because it is a storage format and changing one is not a
|
||
refactor.
|
||
- **`TestStoredEncodingRoundTrips` sweeps every read path** — lookup,
|
||
top-N, exact match, FTS search, popularity batch, the CAA map — and
|
||
asserts each returns something with a dashed id. A missed conversion
|
||
site shows up there and essentially nowhere else.
|
||
|
||
**The artifact is read in either encoding.** A published artifact
|
||
carries whichever form the exporter that built it used, and there is one
|
||
already out there in the old text form. `artifactStoresText` asks the
|
||
artifact (`typeof(mbid)`) rather than trusting a version number, and
|
||
`artifactSelectColumns` converts on the way in — one `unhex` per row on
|
||
a once-a-month import, against requiring a rebuilt artifact before a new
|
||
build can read anything. That probe **must** run on the writer:
|
||
`core` is attached to that one connection, so `QueryContext` asks a
|
||
pool where the artifact does not exist, and the error would silently
|
||
select the conversion path for an artifact that needs none.
|
||
|
||
**Its shape is the pattern for every column added after the fact.**
|
||
`artifactHasTotals` is the same question about `total_tracks`, on the
|
||
same handle: an artifact built before a column existed is still a
|
||
perfectly good catalog, so it is *asked* and the missing column is
|
||
selected as a literal `0`. Adding the column to the importer's SELECT
|
||
list without that is how a published artifact — which nobody can re-cut
|
||
retroactively — starts failing with `no such column`.
|
||
|
||
**A credit is ordered parts, and the string is derived from them.** A
|
||
track credited to several artists had exactly one navigable artist and
|
||
the rest were punctuation: `primaryArtist()` string-parses the credit,
|
||
strips a " feat. " clause and discards the guest, and deliberately does
|
||
not split on `&`, `with` or `,` because those live inside real artist
|
||
names ("Simon & Garfunkel"). Measured on a real 26,069-file library,
|
||
**13%** of recordings are multi-artist upstream while only **0.86%** of
|
||
files carry a structured multi-artist tag — mp3 carries *zero* files
|
||
with multiple `MUSICBRAINZ_ARTISTID` across 19,840 — so this cannot be
|
||
a tag-parsing feature. (The "3 credits of 2,823" figure that justified
|
||
plan 013's removal of the credit tables measured our own *writer*:
|
||
`cachedLinkArtist` ran once per credit, so a collaboration could never
|
||
have been recorded. Dropping the join table was still right on cost.)
|
||
|
||
`artist_credit_part` / `artist_credit_ref` carry the decomposition for
|
||
multi-artist credits only — a single-artist credit is already
|
||
`explore_index`'s own `artist_name`, and storing those would triple the
|
||
table to say nothing. Five things about it are load-bearing:
|
||
|
||
- **Join phrases are assembly instructions, not disassembly ones.**
|
||
`creditLink` concatenates parts, so link boundaries are known by
|
||
construction. Locating a `credited_name` *inside* the stored credit
|
||
string would reintroduce the fault this exists to fix: that string may
|
||
come from the file's tags while the parts come from the catalog, and
|
||
the two disagree for ~1 in 3 multi-artist credits (`'Skrillex feat.
|
||
Swae Lee'` tagged against `'Skrillex & Swae Lee'` upstream).
|
||
- **`credited_name` is stored per row**, never joined from `artists`:
|
||
MusicBrainz credits "Snoop Dogg" on a track by the artist called
|
||
"Snoop Doggy Dogg". Display follows the credit, navigation the MBID.
|
||
- **The lookup is keyed on the recording MBID**, which the catalog and a
|
||
local file both carry (`library.Track.RecordingMBID`), so one binding
|
||
serves Explore and the library's own lists — which is why this needed
|
||
no local table. `file_artists` remains the offline-resilience step and
|
||
is deliberately *not* declared until something writes it.
|
||
- **Absence is cached as an answer.** `credit-store.ts` stores `[]` for
|
||
a single-artist credit — *asked*, not *answered* — or the ~87% that
|
||
have nothing to decompose are re-requested on every render forever.
|
||
`request()` is per-row and coalesces into one call per frame, because
|
||
a virtualized list cannot hand over "the whole list": 50,000 rows is
|
||
100 queries for the ~30 on screen.
|
||
- **The dump is a third source, and it had to be.** The canonical dump
|
||
CI already streams has no join phrases and no as-credited names, and
|
||
the JSON dumps cover 153,691 recordings of ~35M with *zero* overlap
|
||
against a real library. So `mbdump.tar.bz2` — 7.1 GB, ~13.7 min in
|
||
pure-Go bzip2, whose members are alphabetical, which is what lets one
|
||
pass resolve an entity's credit without buffering 35M recordings. The
|
||
pass runs on **every** mode, because a complete import means
|
||
`refresh`, which never enters the importer at all, and it reports
|
||
whether it populated anything so `changed` republishes the artifact.
|
||
|
||
**A 0.6 GB download asks about the connection first.** `explore`'s
|
||
catalog artifact had no network awareness at all, which on a phone is a
|
||
month's data allowance spent without being asked (plan 016 B4).
|
||
`netpolicy.go` is the gate, and its shape is dictated by one constraint:
|
||
`explore` is imported by `cmd/indexbuild`, which is built with
|
||
`CGO_ENABLED=0` and must not link Wails — so the *policy* and the
|
||
*parsing* live here and are tested on every platform, while the platform
|
||
call is a closure injected from `app.go`. It is
|
||
`application.Mobile.NetworkJSON()`, not `application.Android`'s: the
|
||
latter exists only under the `android` build tag, and `Mobile`'s desktop
|
||
implementation is a stub returning `""`.
|
||
|
||
Three rules in it are load-bearing. **An unknown answer is not a metered
|
||
one** — only mobile answers at all, so treating silence as metered would
|
||
refuse the download on every desktop. **Cellular is the only signal
|
||
available**: the runtime reports `wifi|cellular|ethernet|none` and no
|
||
metered flag, so a metered *Wi-Fi* (a hotspot, a hotel) cannot be
|
||
detected and is not refused, which is a documented gap rather than an
|
||
oversight. And **the gate runs before anything is staged**, so declining
|
||
is a no-op rather than a job in the indicator and a status the user has
|
||
to dismiss. The permission (`AllowMeteredCatalogDownload`, default
|
||
false, so an existing config is careful without a migration) is read at
|
||
the moment a download would start, so turning it on takes effect on the
|
||
next attempt rather than the next launch.
|
||
|
||
**Background work yields, and says so in the context.** The post-scan
|
||
backfills share MusicBrainz's rate limiters with every page the user
|
||
can open, and both were FIFO — so a thousand-artist enrichment put an
|
||
album page behind an hour of queued work.
|
||
`RateLimiter.WithBackgroundLane(perSecond)` adds a second, slower lane
|
||
and `WithBackgroundPriority(ctx)` marks a caller as belonging to it: a
|
||
marked wait takes no token at all while any interactive wait is
|
||
outstanding, and is then paced at MB's own 1/s rather than the
|
||
interactive burst rate. It is a **context marker rather than a
|
||
parameter** because a backfill calls the same `MusicBrainzClient`
|
||
methods a detail page does — `GetArtistImage` takes a `ctx` for no
|
||
other reason than to carry it. One request of slippage is accepted and
|
||
documented at `waitBackground`: cancelling an already-granted
|
||
reservation is not something a token bucket can express, and the cost
|
||
is one request-time.
|
||
|
||
The other half is that a long backfill has to be **visible and
|
||
stoppable**: `jobs.KindCatalogEnrich` and `startBackfillJob`
|
||
(`backfilljob.go`) register both backfills with progress and cancel.
|
||
Two rules in it are load-bearing. The job is registered *after* the
|
||
work is counted, because these passes are a no-op on every launch once
|
||
the library is covered and an empty job in the indicator is noise. And
|
||
the kind is distinct from `index-build` rather than reused, because
|
||
`job-controls.ts` keys its "you will discard hours of downloading"
|
||
confirmation on that kind — wrong prompt for a pass that is resumable
|
||
per artist and free to stop.
|
||
|
||
**An owned artist's discography is fetched, not sampled.**
|
||
`BackfillLibraryDiscographies` is two fetches per owned artist, each
|
||
skipped by its own persistent mark, because they fail independently and
|
||
one boolean covering both either over-claims or forces repeats:
|
||
the ListenBrainz top release groups and recordings
|
||
(`explore_index.discog_fetched`) and the **full** MusicBrainz browse
|
||
(`artist_enrichment.browsed_at`).
|
||
|
||
**What it does not fetch is the point.** It ran for hours against a
|
||
900-artist library and marked nothing, because three of the four things
|
||
it did per artist were work nobody had asked for. Similar artists were
|
||
fetched for every owned artist, when the artist page already resolves
|
||
them on view through `SimilarArtists` → `ensureSimilarArtistsAsync` —
|
||
which is what stamps `similar_at` now. And `indexOneArtist` reached the
|
||
MB artist lookup it wants (`GetArtistDetails` reads that cache) by
|
||
calling `GetArtistImage`, which additionally queried fanart.tv,
|
||
TheAudioDB, Wikidata and Wikipedia and downloaded up to ten full-size
|
||
portraits; `EnsureArtistRels` is the lookup on its own. The corollary
|
||
is written into the query: **`similar_at` must not be one of the
|
||
conditions** in `unenrichedLibraryArtistMBIDs`, because testing a mark
|
||
this pass no longer sets makes every owned artist a candidate on every
|
||
run, forever.
|
||
|
||
The rest is that the pass was **serial across artists** while every
|
||
limiter that keeps us polite is per-host and idle — so one artist's
|
||
slowest upstream set the pace for the whole run. It runs
|
||
`discogBackfillWorkers` artists at once (concurrency here raises no
|
||
origin's request rate), each under `discogBackfillArtistTimeout`,
|
||
because the MB client retries a 503 five times honouring Retry-After
|
||
and one throttled artist could otherwise outlast a hundred healthy
|
||
ones. A timed-out artist goes unmarked and is retried next run, which
|
||
is what every other failure here already does. SQLite's writer pool is
|
||
`MaxOpenConns(1)`, so the workers queue at the Go level rather than
|
||
racing for the file.
|
||
|
||
Five things about the marks are load-bearing. **A mark records that the
|
||
upstream was asked, not that it answered with something.** ListenBrainz
|
||
returns 200 and `[]` for an artist it has no popularity data for — which
|
||
is most of a long-tail library, and the same is true of an artist whose
|
||
every row falls under `indexMinPopularity` — and keying
|
||
`discog_fetched` on "did rows come back" made those artists permanent
|
||
candidates: "Filling in artist details" re-ran for up to
|
||
`discogBackfillMaxPerRun` of them on **every launch**, forever, doing
|
||
the same two fetches to the same empty answer. So `indexOneArtist` sets
|
||
the mark when both fetches *succeeded* (`fetchTopReleaseGroups` and
|
||
`fetchTopRecordings` return an error for that reason), and only a real
|
||
failure — transport, non-2xx, unreadable body — leaves the artist for
|
||
the next run. `browseFullDiscography` already had this right: an artist
|
||
with genuinely no release groups is still marked browsed. The marks are **a table, not
|
||
more `explore_index` columns**, because `artifactimport.go` merges the
|
||
downloaded catalog by column list — a flag added there is a second
|
||
place to remember, and forgetting it silently wipes every mark on the
|
||
next catalog update. `discog_fetched` stays in `explore_index` for the
|
||
opposite reason: the artifact legitimately answers it for artists it
|
||
covers. **The artifact answering it is not "we have their
|
||
discography"** — its per-artist coverage is graded, so an artist can
|
||
arrive `discog_fetched = 1` and never have been browsed, which is why
|
||
the unenriched query ORs its conditions instead of testing the
|
||
first. **`BrowseReleaseGroupsAll` pages to exhaustion** where
|
||
`BrowseReleaseGroups` asks for `MaxLimit` once and takes what comes
|
||
back — a prolific artist was silently cut at 100 release groups, and a
|
||
hundred albums looks like a complete answer unless you count. And the
|
||
per-artist mark **replaced a heuristic that could never be satisfied**:
|
||
`BrowseReleaseGroups` used to re-browse whenever no indexed row carried
|
||
a secondary type, which is permanently true for an artist whose
|
||
releases are all plain albums.
|
||
|
||
**One artist portrait is downloaded; the rest are remembered as URLs.**
|
||
`resolveAllSources` asked five upstreams what images they had for an
|
||
artist and then downloaded **every** candidate, up to ten, full size,
|
||
serially — while nothing in the app has ever read anything but
|
||
`primary.jpg` and its three tiers. Measured on a real cache: 5.3 GB,
|
||
of which 4.1 GB was candidates no code path can reach, ~940 kB per
|
||
artist against the ~217 kB that is actually used.
|
||
|
||
It is split now. `resolveCandidates` does the metadata lookups and
|
||
returns an ordered list; `fetchPrimary` walks that list downloading
|
||
until one **succeeds**, makes that the primary, and records the rest
|
||
with an empty `file_path` — known, not fetched — so replacing a
|
||
portrait later is one download rather than five lookups again. Taking
|
||
the first that succeeds rather than the first outright is also a fix:
|
||
the old loop keyed `is_primary` on the index, so a failed candidate 0
|
||
left the artist with a stored image, no `primary.jpg`, and a `.miss`
|
||
marker claiming there was no artwork at all. The winning candidate is
|
||
no longer also written under its own name, since `setPrimary` writes
|
||
the same bytes to `primary.jpg`.
|
||
|
||
Two janitor jobs go with it, and the first is why the waste survived.
|
||
`OrphanedArtistImagesJob` joined the bare MBID onto the images
|
||
directory — but artist directories are **sharded** under a two-character
|
||
prefix, so it named a path that has never existed, `RemoveAll`
|
||
succeeded on it, and the job deleted the rows that were the only record
|
||
of the files it left behind. `explore.ArtistImageDir` is that layout's
|
||
one definition, passed in the way `OrphanedCoverFilesJob` takes
|
||
`expandVariants`, and the test lays its fixtures out with it — a test
|
||
that invents its own flat layout agrees with the bug.
|
||
`StrayArtistImageFilesJob` reclaims what earlier versions downloaded,
|
||
keeping only `explore.ArtistImageKeepNames()` and refusing an empty
|
||
keep set for the reason the covers sweep refuses an empty live set.
|
||
|
||
**An age is not a ceiling, and a cache needs one.** Art for an artist
|
||
the user owns is kept indefinitely; everything else aged out after 90
|
||
days and nothing counted it, so the same install held portraits for
|
||
**5,770 artists in a 1,301-artist library** — every artist page opened
|
||
in Explore fetches one, and a browsing afternoon is entirely inside the
|
||
retention window. `browsedArtBudget` (256 MB) is the second pass:
|
||
oldest browsed artist first, until what is left fits, with owned
|
||
artists outside the budget entirely. `httpCacheBudget` is the same rule
|
||
one cache over, and it is what makes the year-long entity TTL below
|
||
safe — once answers stop expiring, expiry stops being a bound.
|
||
|
||
**"Owned" is a file here too.** The sweep's live set used to be "there
|
||
is an `artists` row", which the file-shaped schema made meaningless;
|
||
it joins `audio_files` now, like every other ownership question. Its
|
||
test had seeded an artists row with no file and called it owned — the
|
||
exact phantom, in the fixture of the test that guards it.
|
||
|
||
**Only the tiers of a cover are stored.** `saveCoverArt` writes
|
||
`_sm`/`_md`/`_lg` and records the largest as `cover_art.file_path`;
|
||
`coverart.ResolveURLs` reports that one as `Original` too, because it
|
||
is the largest kept. The full-resolution image used to be written
|
||
beside them and was **1,134 MB of a 1.4 GB covers directory** against
|
||
110 MB for all three tiers — with nothing rendering it, since the grid
|
||
caps at 350 px and the largest tier is 400. The bytes are still in the
|
||
audio file, which is where they came from, so the repair pass that
|
||
regenerated tiers *from the stored original* went with it.
|
||
|
||
**Frontend** (`frontend/`): Lit 3.2 web components + Web Awesome UI library + HTMX. State management via singleton reactive stores in `src/store/`. Wails bindings auto-generated as TypeScript in `frontend/bindings/`, nested by Go import path — don't edit by hand. The `@go` alias absorbs the constant prefix, so a call site imports `@go/library/library.js`.
|
||
|
||
**One seam states what the generated types get wrong, rather than 78 patches.** v3's generator is honest where v2's lied: a Go `nil` slice marshals to JSON `null` and always has (v2 typed it `T[]`), and a Go named string type is a closed set (v2 typed it `string`). There is no flag to turn either off, correctly. So `utils/binding.ts` states the app's actual contract at the only place it is true — `list` yields `[]` for a nil slice, `dict`/`dictByName` yield `{}` for a nil map and drop null-valued keys (which loses nothing: `noUncheckedIndexedAccess` already makes every read `V | undefined`), and `compact` is the same for a map arriving as a *field*. Where a nullable slice is a model field there is no boundary to put it at, and those are `?? []` at the point of use.
|
||
|
||
All three also return a **plain `Promise`**: v3 bindings return a `CancellablePromise` and nothing in this app cancels one, so letting it inward would put a Wails type in every store signature for a capability none of them use.
|
||
|
||
**A view is a chunk, and three components are not.** `index.ts` holds a
|
||
loader table (`VIEW_LOADERS`, `DETAIL_LOADERS`) and `await`s a view's
|
||
module before creating its element — `document.createElement` on an
|
||
undefined tag yields an inert `HTMLElement` rather than throwing, so a
|
||
missing entry is a blank page, not an error. Navigations are numbered
|
||
and anything after the `await` re-checks it is still the newest, or a
|
||
slow chunk lands on top of a faster navigation. Every chunk is then
|
||
warmed on idle, so the split is paid once at startup rather than on
|
||
every first visit. **`notification-host`, `inline-notice` and
|
||
`confirm-dialog` stay eager on purpose**: a failure surface that has to
|
||
fetch a chunk before it can speak is not a failure surface, and the
|
||
moment it is most needed is the likeliest moment loading one fails.
|
||
`first-run-wizard` and the startup chrome are eager for the ordinary
|
||
reason — they are the first paint.
|
||
|
||
**A navigation is a history entry, and that is the whole back stack.**
|
||
`index.ts` records each navigation with `pushState` (same URL — the app
|
||
has no routes, and a path a reload cannot resolve is worse than none)
|
||
and replays `popstate` with `_isBack`. It exists for Android, whose back
|
||
button is not a key the page can bind: the scaffold's
|
||
`MainActivity.onBackPressed` asks `webView.canGoBack()` and finishes the
|
||
activity otherwise, so an app that never touched `history` quit from any
|
||
depth — which is what a device reported. Hooking the platform's own
|
||
mechanism rather than adding a JNI callback is also what makes it
|
||
testable in a browser (`page.goBack()`), and the Java half needed no
|
||
change at all.
|
||
|
||
Two rules hold it up. The **first** navigation *replaces* the launch
|
||
entry rather than pushing one, or every launch costs a back press before
|
||
the app will close. And the in-app back buttons (`navigate-back`, fired
|
||
by the detail views and `now-playing-view`) go through `history.back()`
|
||
rather than a stack of their own: the old `navStack` is **deleted**, not
|
||
kept beside it, because two stacks is precisely how a view's own back
|
||
button and the phone's gesture come to disagree about what one press
|
||
means.
|
||
|
||
**A primary view is cached, not unmounted.** `index.ts` keeps every
|
||
primary view in the DOM and toggles a `.view-hidden` class, because that
|
||
is what preserves `scrollTop` across navigation — so
|
||
`disconnectedCallback` never fires for one, and anything registered
|
||
there runs for the life of the session from pages it is not on. The
|
||
missing half is `utils/view-lifecycle.ts`: navigation calls
|
||
`viewDeactivated()` on the outgoing view and `viewActivated()` on the
|
||
incoming one, and a view registers its document listeners, timers and
|
||
backend subscriptions through `listenWhileActive` /
|
||
`intervalWhileActive` / `whileActive`, which are torn down on the way
|
||
out. An off-screen view also does not render. A shared reactive
|
||
controller gets the same treatment via `registerViewAware`.
|
||
|
||
**The player's position comes from the player.** `seek-bar` renders
|
||
`PlaybackPositionChanged` (payload `player.PositionInfo`), emitted at
|
||
1 Hz while playing and immediately on load, play, pause, seek and
|
||
natural finish. Its local `setInterval` is interpolation *between*
|
||
reports only, stopped and restarted by every one of them — it used to
|
||
be the clock, and counted itself 30 s away from the backend across four
|
||
keyboard seeks. A report carries `trackChangeId` (the store is a
|
||
singleton, so a bar mounting later must not adopt a report about the
|
||
previous track) and a `seq` (the same second reported twice still has
|
||
to reset the interpolation).
|
||
|
||
What the player cannot do, it says: `PlaybackFailed` is emitted from
|
||
both the load and the play path, auto-advance **skips** the failed
|
||
track (bounded by the queue length, so a disconnected drive stops after
|
||
one pass), and the bottom bar shows one coalescing line — "Skipped 12
|
||
tracks that could not be played." That line is the Inline level of the
|
||
app's one notification surface, below.
|
||
|
||
**Failure has one voice, and the caller picks how loud.**
|
||
`store/notification-store.ts` is the only notification surface; before
|
||
it, 84 `catch` blocks ended at `console.error` and two components had
|
||
grown private toasts. Four levels, chosen by the call site from one
|
||
rule — *a failure is only worth interrupting for if the user can do
|
||
something about it that they are not already doing*:
|
||
|
||
- **Blocking** (`wa-dialog`, must be acknowledged) for data at risk: a
|
||
folder left holding a mix of old and new tags. Two callers are
|
||
anticipated; a third should be argued for.
|
||
- **Persistent** (stays, with an action) for something the user asked
|
||
for that did not happen and retrying is meaningful.
|
||
- **Transient** (a toast) for a small action whose state visibly
|
||
reverted anyway — a favourite that came back.
|
||
- **Inline**, rendered by `<inline-notice region="…">` in the panel
|
||
that failed, never as a toast.
|
||
|
||
Three things about it are load-bearing. **Coalescing lives in the
|
||
store**, keyed by `(level, region, key)` within a window, so 200
|
||
unplayable files are one message with a count and no future caller has
|
||
to remember that. **An inline notification carries a region**, because
|
||
"inline" says *not global*, not *where*. And **the bottom band belongs
|
||
to the player** — the app-level stack sits under the header, since the
|
||
player's own floating notice grows upward by however many lines it
|
||
needs and a bottom-anchored stack collides with it on a small window.
|
||
|
||
What reaches a person is a sentence: `utils/describe-error.ts` maps the
|
||
causes a user can act on (offline, timeout, not found, permission,
|
||
database busy) to copy, `explainError` repeats a backend message when it
|
||
is one of *our* sentinels rather than a Go wrapping chain, and the raw
|
||
text stays in `console.error`. The one documented exception is a
|
||
download client's connection test, whose verbatim error is the user's
|
||
debugging tool.
|
||
|
||
Destructive actions ask once, through `confirmAction()`
|
||
(`components/confirm-dialog/`), which is a `wa-dialog` and so brings the
|
||
focus trap and Escape the hand-rolled overlays do not have.
|
||
|
||
**Every dialog in the app is a `wa-dialog`, and there is no sixth
|
||
pattern.** The four hand-rolled autotag overlays and the remove-library
|
||
confirmation had no `role`, no `aria-modal`, no focus trap and no focus
|
||
restore — including the two gating an irreversible on-disk metadata
|
||
rewrite. The split is by *shape*, not by owner: a dialog that only asks
|
||
a question is a `confirmAction()` call (title, message, impact,
|
||
confirm/cancel), and a dialog carrying **input** is a `<wa-dialog>` in
|
||
the host's own template. Both remaining autotag dialogs render
|
||
unconditionally with `?open` deciding which is up — mounting one on
|
||
demand puts the element and its `showModal()` in the same update.
|
||
`autotag-view`'s last document keydown listener died with them; it
|
||
existed only because its dialogs could not close themselves.
|
||
|
||
**None of them had an accessible name, and one helper gives all of them
|
||
one.** Every call site passes `label`; Web Awesome renders it into an
|
||
`<h2 id="title">` in the same shadow root as the native `<dialog>` and
|
||
never points `aria-labelledby` at it — so for eleven dialogs
|
||
`getByRole('dialog', {name})` matched nothing and a screen reader
|
||
announced an unnamed dialog. `utils/name-dialog.ts` sets that IDREF
|
||
(and falls back to `aria-label` under `without-header`, which renders
|
||
no heading to point at), called from each host's `updated()`.
|
||
|
||
Three things about it are load-bearing. It **reaches into another
|
||
library's shadow root**, which is open but is not API — acceptable
|
||
here only because the failure is bounded: if Web Awesome moves the
|
||
structure the query misses, nothing is written, and the dialog is as
|
||
unnamed as it was. It uses **`aria-labelledby`, not `aria-label`**,
|
||
because three call sites compute their label at render time and an
|
||
IDREF to the heading Web Awesome re-renders stays correct with nothing
|
||
resyncing it. And it **waits for the dialog's own first update**, not
|
||
its host's: `wa-dialog` is a Lit element whose shadow root is populated
|
||
in *its* update, so a query at the host's `firstUpdated` finds an empty
|
||
root and names nothing — the same lifecycle trap that hid
|
||
`wa-dropdown-item`'s role from the menu keyboard model.
|
||
|
||
Two awkwardnesses remain, and they are about *locating* one rather than
|
||
naming it. The host is `display: contents`, so the element carrying the
|
||
testid always reports hidden — what is visible is the `<dialog>` inside
|
||
it — and what holds the slotted content is the *host's* shadow root,
|
||
not the dialog's subtree. A third is worth knowing before checking any
|
||
of this: the Playwright **a11y snapshot never prints a dialog's name**,
|
||
named or not, so it cannot tell you whether this works. `getByRole`
|
||
can, and CDP's `Accessibility.getFullAXTree` gives the browser's own
|
||
answer.
|
||
|
||
**A disclosure is a button, and it says what it controls.**
|
||
`config-section`'s header was a bare `<div @click>` with no `tabindex`,
|
||
no `role` and no `aria-expanded`, and every section defaults to
|
||
collapsed — so every setting in the app sat behind a control that could
|
||
not be tabbed to (the audit's last Critical). It is a
|
||
`<button aria-expanded aria-controls>` now, on the pattern
|
||
`explore-artist-details` has had five of all along, and **the body
|
||
renders unconditionally and is toggled with `hidden`**: `aria-controls`
|
||
has to name an element that is in the DOM, and a conditional `<slot>`
|
||
only stops projecting light-DOM children that exist either way.
|
||
Downloads' two tabs are the same fix one page over — `role=tablist` over
|
||
`role=tab`, one roving tab stop, Left/Right/Home/End, and a
|
||
`role=tabpanel` whose id and `aria-labelledby` swap with the tab.
|
||
|
||
**A menu has a keyboard model, and it is one model.**
|
||
`utils/context-menu-controller.ts` exports **`MenuKeyboard`** — focus
|
||
the first item on open, Arrow/Home/End to move (wrapping, as a menu
|
||
does and a listbox does not), Enter/Space to activate, Escape or Tab to
|
||
close, and focus back to the element it opened from. It is standalone
|
||
rather than part of `ContextMenuController` because `playlist-view`
|
||
renders a menu without that controller, and two menus with two keyboard
|
||
models is exactly what this is for. `isContextMenuKey()` is the
|
||
Shift+F10 / ContextMenu-key test, and `openFrom(el)` is the keyboard
|
||
open: anchored to the element, restoring focus to it.
|
||
|
||
Four things in it are load-bearing, and two of them are only visible
|
||
against the real components:
|
||
|
||
- **The items are not items yet when the host finishes updating.**
|
||
`wa-dropdown-item` sets its `role` in its *own* first update, so a
|
||
`[role^="menuitem"]` query at `updateComplete` finds nothing — which
|
||
reads exactly like a menu that opened and refused to take focus.
|
||
- **`focus()` on a popup that has not positioned itself is a silent
|
||
no-op**, so the first focus is retried across a few frames.
|
||
- **Focus is only taken back if the menu had it.** A click elsewhere
|
||
closes the menu too, and pulling focus to the row the user
|
||
right-clicked a moment ago is worse than leaving it.
|
||
- **Web Awesome keys an item's tabindex and highlight off `active`**, so
|
||
moving focus without setting it leaves the highlight on whichever
|
||
item the mouse last touched.
|
||
|
||
**And a menu opens from a finger, through the event it already has.**
|
||
`utils/long-press.ts` is one document-capture listener installed once
|
||
from `index.ts`: a touch that holds still for 500 ms dispatches a
|
||
synthetic `contextmenu` at the touch point, so all six components that
|
||
bind one — delegated on a virtualizer, per row, per card — gained the
|
||
gesture without changing. The target is `composedPath()[0]` rather than
|
||
`elementFromPoint`, which stops at the outermost shadow host and so
|
||
reaches a delegated listener and no per-row one; a browser that fires
|
||
its own long-press `contextmenu` (Chromium does, WebKit and the WebView
|
||
vary) wins, ours being told from theirs by **identity** rather than
|
||
`isTrusted`, since no test can dispatch a trusted event; and the click
|
||
that ends the gesture is swallowed, keyed on the gesture rather than on
|
||
a time window so the first tap on the menu it opened is not eaten too.
|
||
|
||
Three lists had no focused row to open a menu *from* — the queue panel
|
||
and both playlist detail views — and gained a roving tab stop through
|
||
`utils/roving-rows.ts`. **`track-list` deliberately does not use it**:
|
||
its equivalent predates this, carries selection semantics (shift-extend,
|
||
ctrl-toggle) the other three do not have, and is pinned by its own
|
||
tests.
|
||
|
||
**A shared panel computes its own name, and its items ask what the
|
||
target can do.** `explore-artist-details` had a menu for its top
|
||
*tracks* and none on the release cards, which are most of the page. It
|
||
has one now on both release shapes — the top section's
|
||
`LBTopReleaseGroup` and the discography's `MBReleaseGroup` — normalised
|
||
to a `ReleaseMenuTarget` at the moment the menu opens, so the union
|
||
does not reach five action handlers. `ctxMenuTarget` is a discriminated
|
||
union rather than one nullable field per kind **because the panel is
|
||
shared**: that is what keeps `aria-label` moving with the target, which
|
||
is the fault `cover-grid` shipped (every menu announced as "Album
|
||
actions").
|
||
|
||
Which items appear is decided by what the release can actually do, and
|
||
those are three different questions. Playback is gated on a **local
|
||
album id**, not on the badge's "owned": a release matched by MBID with
|
||
no local album behind it has nothing to queue, and `GetFilePathsBy‐
|
||
Albums` is keyed on the id for the reason the album page is — an owned
|
||
but untagged release has no recording MBIDs, so an MBID-keyed lookup
|
||
returns nothing while looking entirely correct. The request item needs
|
||
the opposite, a catalog MBID, so it is absent for a library-only
|
||
release (`local:<n>`, unwrapped the same way `navigateToAlbum` does) —
|
||
which is also the one case where wanting it makes no sense.
|
||
|
||
**Async surfaces say what they are doing.** `styles/sr-only.css.ts`
|
||
carries the visually-hidden class and the rule that comes with it: a
|
||
live region must be **in the DOM before the text it announces is**,
|
||
because most screen readers announce a change to a region they are
|
||
already watching and ignore one that appears with its content already
|
||
in it. So these regions render unconditionally and empty, and only
|
||
their text changes. Four surfaces have one — the track list (loading,
|
||
failed, and how many rows a search matched), Explore's search,
|
||
`now-playing` (in **both** render branches, so it exists before the
|
||
first track arrives) and `job-indicator`, whose label swings between
|
||
"Scanning Music", "3 background jobs" and "Finished". The notification
|
||
surface already had one from Phase 3.
|
||
|
||
**A colour's role decides whether it can be fixed across themes.** A
|
||
*fill* — `--yj-error`, `--yj-success` — is "what colour is a danger
|
||
button", which is red in every theme and stays fixed. A *text* colour
|
||
— `--yj-error-text` — is "what colour is the word *failed* on this
|
||
background", which cannot be: one value cannot clear 4.5:1 against both
|
||
a near-black and a near-white surface, and the old fixed set measured
|
||
2.31–4.28:1 on nearly all of them. And every fill carries a **computed**
|
||
foreground (`--yj-accent-fg`, `--yj-*-fg`) rather than a written-down
|
||
one, because the accent is a colour picker: white on the default
|
||
`#ffd43b` is 1.43:1. `readableOn()` keeps white where white clears and
|
||
flips to black where it does not, and `accentTextOn()` mixes the accent
|
||
along its own hue until it clears the ramp's surface — returning it
|
||
unchanged on both dark ramps. Two accent buttons used to take their
|
||
foreground from `--yj-bg-base`, which *inverts with the ramp*; that is a
|
||
token used for the wrong meaning, and it only shows in the theme nobody
|
||
looks at.
|
||
|
||
**Contrast is a property of the ramp, and the ramps are data.**
|
||
`theme-store`'s `SHADE_PALETTES` — not `tokens.css.ts`, which holds only
|
||
the type scale and icon sizes — is where the colours live, applied to
|
||
`:root` at runtime, which is why the `var(--yj-…, #fallback)` at every
|
||
call site is dead in practice. Every text colour clears 4.5:1 against
|
||
every surface it can sit on, and `theme-contrast.test.ts` computes that
|
||
from the table rather than trusting it. Three rules hold it up.
|
||
**`bgOverlay` is not a text surface on the dark ramp** — sizing tertiary
|
||
to clear it needs a grey lighter than *secondary*, and an inverted ramp
|
||
is a worse answer than the problem, so the one component that put text
|
||
there uses primary. **A generated colour is a family, not a colour**:
|
||
`utils/avatar-color.ts` exists because `hsl(hue, 45%, 35%)` behind white
|
||
initials failed for 35 of the 360 hues, so the failure came and went
|
||
with how an artist's name hashed — the test walks all 360. And
|
||
**`make ui-visual` cannot see any of this**: the component tier renders
|
||
the fallbacks, because the theme only reaches `:root` in the real app.
|
||
|
||
**A stated motion preference outranks an app setting, and the state a
|
||
fix lands in is a state nobody has looked at.** `now-playing`'s marquee
|
||
ran for as long as a track played with no way to pause it (WCAG 2.2.2),
|
||
and the guard is in `shouldScroll()` rather than in CSS: the cycle is a
|
||
transition out, a `transitionend` and a transition back, so suppressing
|
||
the animation strands the text off its own box with nothing to bring it
|
||
back. It covers `hover` as well as `always` — `reduce` is a request
|
||
about motion, not about autoplay. The two bugs behind it were both in
|
||
the *fallback*: `text-overflow` sat on the outer span while the box that
|
||
overflows is the inline-block child, so the non-scrolling state had
|
||
never produced an ellipsis **in any mode**, including the default; and
|
||
moving the ellipsis to the child stops the parent overflowing, which
|
||
silently disabled overflow *detection* and would have stopped anything
|
||
scrolling ever again. Both measurements come from the child now. The
|
||
first was found by reading a screenshot, the second by the new test's
|
||
positive case.
|
||
|
||
**Roles have to be wired to each other.** `combobox`, `listbox` and
|
||
`option` were all present on `<yj-combobox>` and nothing connected them,
|
||
so arrowing through nineteen options moved a highlight and announced
|
||
nothing. Ids on the listbox and every option, `aria-controls`,
|
||
`aria-activedescendant` — and `aria-selected` meaning *chosen*, which is
|
||
the distinction the pattern rests on: the highlight is what
|
||
`activedescendant` points at. Unlike `config-section`'s disclosure this
|
||
IDREF may dangle while closed, because the popup genuinely does not
|
||
exist then and `aria-expanded` says so. Checked against
|
||
`Accessibility.getFullAXTree`, not against a snapshot — and read the
|
||
whole property, since `activedescendant` reports `value.type: "idref"`
|
||
and an extraction expecting a string reports `(none)` on a working
|
||
build.
|
||
|
||
**The queue's order is reachable from the keyboard.** Alt+ArrowUp/Down
|
||
moves the focused row, with a live region saying where it went. It is
|
||
in `queue-panel`'s own *delegated* keydown beside Enter and the roving
|
||
arrows, not a backend panel binding: it cannot collide with the global
|
||
Up/Down volume bindings, and a reordering key does not belong in a
|
||
user-editable table where it could be rebound onto something
|
||
unmodified. Two things in it are load-bearing. **The index arithmetic
|
||
is not symmetric** — `MoveQueueTracks` takes an index into the array
|
||
*before* the move, so down-by-one asks for `i + 2`, because `i + 1` is
|
||
where the row already is once its own removal is accounted for and the
|
||
backend's contiguous-block guard correctly treats it as a no-op. And
|
||
**the index comes off the row the event came from**: `focusedIndex` was
|
||
only ever moved by an arrow key, so a row reached by a click or by Tab
|
||
left it at 0 and `Enter` played the first track in the queue from any
|
||
focused row.
|
||
|
||
**A name is computed where the role is, and that is rarely where you
|
||
wrote it.** Four surfaces wrote a name somewhere the accessibility tree
|
||
never looked. `wa-slider` puts `role="slider"` on a div in its own
|
||
shadow root pointing `aria-labelledby` at an empty internal `<label>`,
|
||
which outranks the host's `aria-label` — so both sliders computed a
|
||
name of `""`, and `a11y.md` lists both under *what is already correct*.
|
||
The name comes from `label` now, the library's own API, and
|
||
`styles/wa-slider-label.css.ts` hides it: preferred over reaching into
|
||
the shadow root the way `name-dialog.ts` must, because if Web Awesome
|
||
renames the part the label becomes *visible and correctly named*
|
||
rather than silently nameless. Its second rule is load-bearing —
|
||
`#slider` takes an 8px margin the moment a label exists, which grows
|
||
the bar 6px → 14px and moves the transport with it.
|
||
`wa-progress-bar`'s `label` *is* an `aria-label` and is invisible, so
|
||
there it is just the right attribute.
|
||
|
||
The same thing in the light DOM: `config-field` rendered a `<label>`
|
||
as a **sibling** with no `for`, so **24 of 93 controls on Settings**
|
||
computed an empty name. They use `for`/`id` (a fixed id, safe only
|
||
because each field is its own shadow root) rather than `aria-label`,
|
||
for what it buys beyond the name — the label text becomes a click
|
||
target. And three surfaces are named but identify nothing, which is
|
||
the same fault one step milder: three shortcut buttons announced
|
||
themselves as "S", thirty-six column arrows as "Move up", and every
|
||
queue row's remove button as "Remove from queue".
|
||
|
||
**Checking any of this needs the browser's own answer, and "0 unnamed"
|
||
is not it.** A `placeholder` is an accname fallback, so an
|
||
`Accessibility.getFullAXTree` sweep of all eleven views reported
|
||
Explore's search box — the audit's own `a11y.26` — as clean. A sweep
|
||
for *empty* names cannot see a *weak* one.
|
||
|
||
**The shell scrolls sideways and not down.** `body` is
|
||
`overflow-x: auto; overflow-y: hidden`, and both halves are measured.
|
||
Vertically there is nothing to fix: the middle grid row is `1fr` and
|
||
absorbs the 4em bars exactly — at 200% text on an 800×600 window the
|
||
bars go 64 → 128px, the main panel 472 → 344px, and the footer still
|
||
lands on 600. Horizontally the shell is 784px inside a 320px viewport
|
||
(400% page zoom, the width WCAG 1.4.10 names) and 464px of it,
|
||
including the job indicator and the queue button, used to sit behind
|
||
`overflow: hidden`. Keeping the vertical axis fixed is what keeps the
|
||
transport where a desktop player's transport belongs. At every size
|
||
this app promises, no scrollbar appears. Note that `overflow: hidden`
|
||
still permits *programmatic* scrolling, so a probe that sets
|
||
`scrollLeft` passes on the broken build; the spec uses a wheel gesture.
|
||
|
||
**Below 600px it reflows instead, and that is the phone.** The sideways
|
||
scroll above was the concession available while the shell had one
|
||
layout; plan 016 B2 gives it a second. Under 600px the grid drops its
|
||
sidebar column, `<bottom-nav>` takes over as the primary navigation,
|
||
the header's controls shrink or stand down, and the shell measures
|
||
exactly 320px in a 320px viewport — so `layout-overflow.spec.ts` now
|
||
asserts *nothing needs scrolling to*, which is what WCAG 1.4.10 wanted
|
||
all along. 600 rather than the sidebar's 900 because 900 is a laptop:
|
||
the answer there is a narrower sidebar, which is still a sidebar.
|
||
|
||
Three rules in it are load-bearing, and the second cost 30 specs.
|
||
|
||
**A grid item's implicit minimum is its content**, so one child that
|
||
insists on 580px makes the *body* 580px wide inside a 360px viewport
|
||
and `overflow-x: hidden` then hides a third of the app rather than
|
||
fitting it. Every box between the viewport and the content that must
|
||
shrink carries `min-width: 0`, and the things that cannot shrink say so
|
||
in their own stylesheet — `search-bar`'s 200px floor, `job-indicator`'s
|
||
label, `audio-player`'s seek bar and volume. A media query inside a
|
||
shadow root is answered by the viewport, so a component states what it
|
||
drops at phone width itself rather than the shell reaching in.
|
||
|
||
**A duplicated component duplicates its handles.** `bottom-nav`'s
|
||
"More" opens the *same* `<app-sidebar>` in a `wa-drawer` rather than
|
||
listing the destinations again — but rendering it unconditionally put a
|
||
second copy of every `data-testid="nav-*"` in the DOM, and 30 existing
|
||
specs failed with "strict mode violation: resolved to 2 elements" on a
|
||
desktop viewport where the element is not even visible. It renders only
|
||
while the drawer is open, and `bottom-nav.test.ts` asserts its absence
|
||
before that.
|
||
|
||
**The tab bar is four destinations and a way to the rest.** Three to
|
||
five is where touch targets stop being thumb-sized; eleven over 360px
|
||
is 32px each. Which four is plan 016's committed subset, and everything
|
||
else — Settings included, because a phone still needs it — is behind
|
||
"More".
|
||
|
||
**The phone section of `index.css` is last on purpose.** A media query
|
||
adds no specificity, so a `@media (max-width: 599px)` block placed
|
||
above the plain rules it overrides loses to them — which is how phase 1
|
||
shipped a header that kept its 2em gutters and 24px title on a 390px
|
||
phone with every declaration dead and nothing failing. The shell fitted
|
||
anyway, because the fitting is done by `min-width: 0` and by each
|
||
component's own media query, which live in their own stylesheets and
|
||
have no later rule to lose to. Cosmetic declarations are exactly what
|
||
no assertion sees; a screenshot found it.
|
||
|
||
**`<now-playing-view>` is where the seek bar and volume went.** It is a
|
||
*detail* view (`DETAIL_LOADERS`, so the nav stack carries the way out —
|
||
a tab you cannot leave by pressing again is not a tab), reached from a
|
||
phone-only button over the mini player's art, and it **composes the
|
||
real `<seek-bar>`, `<player-controls>` and `<volume-control>`** rather
|
||
than reimplementing them. While it is up, `index.css` hides the bottom
|
||
bar through `body:has(#main-content[data-active-view="now-playing"])` —
|
||
the active view is already published as an attribute, and a class
|
||
toggled from `index.ts` would be a second expression of the same fact.
|
||
The view therefore carries its own queue button, because that button
|
||
lives in the bar it hides.
|
||
|
||
**The playing row is a shape, not a hue.** `track-list` and
|
||
`queue-panel` draw a `::before` triangle in each row's own left
|
||
padding, plus `aria-current` — before, both rows were a background tint
|
||
and a text colour and nothing else (WCAG 1.4.1). It is in the padding
|
||
because the track list's grid columns are computed from the host width,
|
||
so a marker in the flow moves every cell on the playing row and nothing
|
||
else. Both tiers assert it is **absent** on the other rows: a marker
|
||
that renders everywhere satisfies "the playing row has one" for free.
|
||
One thing to know before checking it — a track started from the *track
|
||
list* leaves the queue's `currentIndex` at −1, so the panel has no
|
||
current row at all in that flow, which reads exactly like the marker
|
||
not working.
|
||
|
||
**The first thing Tab reaches is a skip link.** Two details are
|
||
load-bearing and neither is the link's text. It is `position: absolute`
|
||
in **both** states, because `body` is a grid with named areas and an
|
||
in-flow extra child is auto-placed into one of them. And `<main>`
|
||
carries `tabindex="-1"`, or the fragment moves the scroll, leaves the
|
||
tab sequence exactly where it was, and looks like it worked. The
|
||
subtitle beside it is a `<p>`, which is also what an `hgroup` is
|
||
supposed to contain — and dropping the `<h3>`'s bottom margin shortened
|
||
the flex-centred title block enough to move it down into the 4em bar's
|
||
clip, so `.title` zeroes both margins.
|
||
|
||
**A selectable grid is a listbox.** The four grids that ctrl/shift-select
|
||
(`artists-view`, `genres-view`, `cover-grid`, and the queue) are
|
||
`role="listbox" aria-multiselectable` over `role="option"` cards, not
|
||
rows of `role="button"`: `aria-selected` on a button is *invalid* and is
|
||
dropped outright, so the state the whole ctrl/shift interaction exists
|
||
to produce was invisible to anything but a sighted user. `track-list`'s
|
||
column headers carry `aria-sort` (Phase 1 added `role="columnheader"`
|
||
without it) and are activated by Enter/Space as well as by a click.
|
||
|
||
**One keyboard authority.** No component owns a document keydown
|
||
listener for its own shortcuts; it registers *panel-scoped* bindings
|
||
(`autotag.*`, `tracklist.*` in `backend/shortcuts/config.go`) and
|
||
claims a scope by setting `shortcutScope` on the mixin, which publishes
|
||
`data-shortcut-scope` and claims it as the ambient scope
|
||
(`services/shortcut-scope.ts`) while it is on screen. The shortcut
|
||
service resolves focus → panel → global, and yields a key to a focused
|
||
control that owns it (button/select/slider/checkbox, or anything inside
|
||
an open dialog) so the unmodified single-key global bindings do not
|
||
steal Space and the arrows.
|
||
|
||
**A list owns the arrows it moves on, which is the vertical ones.**
|
||
Granting a `row`/`option`/`grid` all six took `←`/`→` away from seeking
|
||
and gave them to nobody: `track-list`'s own handler and
|
||
`utils/roving-rows.ts` both take Up/Down/Home/End and ignore
|
||
Left/Right, so a focused track row produced zero `Player.Seek` calls
|
||
against one per press from the body. Grant them back in
|
||
`keysOwnedBy` if a list ever moves horizontally.
|
||
|
||
**The keys are written down in one place and shown in two.**
|
||
`services/shortcut-meta.ts` is the label, category, scope and default
|
||
for every action; `?` opens `<shortcuts-overlay>` and Settings edits
|
||
the same table. It used to be a private static in `config-page`, which
|
||
listed three of the four categories by hand — so the autotag bindings
|
||
were written down nowhere. **The overlay is not a toggle**: a dialog
|
||
owns every unmodified key while it is up, so a second `?` never reaches
|
||
the service; Escape closes it. And a shifted character does not report
|
||
Shift (`?`, not `Shift+?`) — the character already carries it.
|
||
|
||
Two cross-cutting pieces of that UI are worth knowing before touching
|
||
a list or a detail view:
|
||
|
||
- **`utils/explore-link.ts`** renders every track/album/artist name in
|
||
the app. A name always navigates: to the MusicBrainz page when the
|
||
entity is tagged, and to the *library* page for the same thing when
|
||
it is not (`explore-album-details` and `explore-artist-details` both
|
||
accept a local id instead of an MBID). It fires on a genuine single
|
||
click only — the navigation is held for one double-click interval
|
||
and dropped if a second click arrives, because the title is the
|
||
widest thing in a row and double-clicking a row plays it. Rows do
|
||
not need to know links exist.
|
||
- **`<catalog-scope-notice>`** is how a detail page admits what it is
|
||
showing: catalog data (silent), a library stand-in while a fetch is
|
||
in flight, library-only because the entity has no MBID, or a failed/
|
||
empty catalog answer with a retry. Both detail views track
|
||
`catalogPending`/`catalogLoaded` separately from their loading flags,
|
||
since "something is renderable" and "this is the catalog's answer"
|
||
are different questions.
|
||
|
||
**Icons are bundled, and the app works offline.** `src/icons/`
|
||
overrides Web Awesome's `default` icon library, whose resolver fetches
|
||
every `<wa-icon>` from `ka-f.fontawesome.com` at runtime — so the app
|
||
had no icons at all offline, and `setBasePath()` does not affect it
|
||
(only the component autoloader reads that). Overriding the library
|
||
fixes all 165 call sites without changing one of them. Three things
|
||
about it are load-bearing. The set is **Font Awesome Free** (CC BY 4.0,
|
||
vendored with its licence by `frontend/scripts/fetch-icons.mjs`)
|
||
because the kit CDN serves **Pro**, which cannot be redistributed. The
|
||
names are a committed list (`src/icons/names.txt`) rather than anything
|
||
derived, because twenty call sites compute their icon name from state
|
||
and no static pass can enumerate them. And a name that is not bundled
|
||
is therefore **reported at runtime** to `window.__yjIconMisses` and
|
||
drawn as a fallback — an e2e sweep asserts there are none — since a
|
||
missing icon used to be impossible, the CDN having had everything.
|
||
|
||
**An album page says how much of the album is yours.**
|
||
`explore-album-details` is a *catalog* page and there is no
|
||
library-side album detail page at all, so the album on it may be
|
||
wholly the user's, partly theirs, or not theirs — and its primary
|
||
action has to mean the same thing in each case. It says which: **Play**
|
||
when the whole release is owned, **Play 7 of 12** when some of it is,
|
||
and **no play button at all** when none is, because a Play button that
|
||
plays nothing (or seven tracks of forty) is worse than none.
|
||
|
||
**The page asks one question, once, and it is "is there a file".**
|
||
Ownership used to be several claims of decreasing confidence OR'd
|
||
together — a local album id, the backend's cross-reference, a cached
|
||
MBID match, and finally *any single track* flagged `inLibrary` — none
|
||
of which is "there is a file to play", which is why the tick could be
|
||
green on an album whose every action did nothing, and why the
|
||
tracklist's context menu asked the backend on **hover** whether the row
|
||
it was drawing was owned. `filePaths` is the one answer: a map from a
|
||
displayed track to its path, filled once from `updated()` by a single
|
||
batched `GetFilePathsByRecordingMBIDs`, and read by the badge, the Play
|
||
button's count, the dimmed rows and every menu item. `askedFor` is a
|
||
separate set from `filePaths` because the guard has to be *asked*, not
|
||
*answered* — an unowned MBID never lands in the map, so guarding on the
|
||
map re-requests it on every render, forever.
|
||
|
||
The other half is that the *displayed* tracklist is also one thing.
|
||
`buildVersionEntries` synthesises the "Your Library" entry from
|
||
`localTracks`, so an album the catalog cannot answer for is exactly the
|
||
case that needs the version list rebuilt — `loadLocalTracks` guarded
|
||
that rebuild on `releases.length > 0` and so rendered "No release data
|
||
available" over a tracklist it was holding in memory.
|
||
|
||
**And the key it plays by is not the key it looks owned by.** The local
|
||
album id is used wherever there is one, because a library-only album
|
||
has *no* recording MBIDs — its tracks are synthesised from
|
||
`GetAlbumTracks` with `mbid: RecordingMBID || ''` — so an MBID-keyed
|
||
lookup on an untagged library resolves to nothing and Play queues
|
||
nothing while looking entirely correct. Those synthesised tracks carry
|
||
their `FilePath` from the same rows, so they populate `filePaths`
|
||
directly and cost no lookup at all; the batched
|
||
`GetFilePathsByRecordingMBIDs` is what answers for a *catalog*
|
||
tracklist. It is the third member of the `GetFilePathsBy…` family: one
|
||
query, paths only, grouped so the caller keeps the tracklist's order.
|
||
It exists rather than a lookup by track id because **`MBTrack.LocalID`
|
||
is declared and nothing in the backend ever writes it**.
|
||
|
||
**How much of an album is here is a question the files can answer.**
|
||
`ownership()` above counts the *displayed* tracklist, which for a
|
||
library copy is a tautology — every local track has a file, so owned
|
||
always equals total and "do I have all of this" had no local answer.
|
||
The album page therefore asked MusicBrainz, and
|
||
`BrowseReleases` is the most expensive call the app makes: releases
|
||
plus every version's full tracklist, on a 1 req/s limiter shared with
|
||
`PrefetchReleases`, which fires up to eight when an artist page
|
||
renders.
|
||
|
||
The denominator was already on disk. `metadata` has read the "5/12"
|
||
totals off every file since forever (`m.Track()`, `m.Disc()`) and
|
||
discarded them; they persist to `audio_files.total_tracks` now, and
|
||
`GetAlbumCompleteness` sums them. **A complete, MBID-matched album
|
||
makes no catalog call at all** — identity from the MBID, tracklist from
|
||
the tags, which between them are what the browse was being spent on.
|
||
|
||
Three things about it are load-bearing. **Totals are declared per
|
||
disc**, so the expectation is a sum over discs and not one number, and
|
||
a disc whose files declared nothing leaves the whole album unknowable
|
||
rather than being covered by the discs that did. **Unknown is a third
|
||
state and must render as neither** — a great deal of any untagged
|
||
library has no total, and a ring drawn from its absence would mark most
|
||
of a library incomplete on no evidence; `Known` is what guards that,
|
||
and the badge falls back to the plain tick. And **complete is `>=`,
|
||
not `==`**, because bonus and hidden tracks routinely put a folder over
|
||
its declared total and that is a complete album, not a broken one.
|
||
Owned counts *distinct track numbers* for the same reason in reverse:
|
||
this app detects duplicates, and counting two files of track 3 twice
|
||
would report a short album as complete.
|
||
|
||
**Where the tags have no total, the catalog does.** `explore_index`
|
||
carries a per-release-group `total_tracks` — ~2 bytes across 400,677
|
||
rows, about 800 kB of artifact — and `completenessAnswer()` merges the
|
||
two: the numerator stays local (distinct track numbers on disk) and
|
||
only the denominator is borrowed, because a catalog total describes the
|
||
canonical release while the files' own total, where they declare one,
|
||
describes the release the user actually has. Zero still means "the
|
||
catalog does not say", so an album neither side can total keeps the
|
||
plain tick.
|
||
|
||
Two rules hold up the column itself. **It is counted before the
|
||
popularity filter**: `cmd/indexbuild` counts the canonical dump's rows
|
||
per kept release, and counting only the *kept recordings* would say "9"
|
||
about a twelve-track album whose other three nobody has played — the
|
||
same confident lie that kept whole tracklists out of the artifact.
|
||
And **adding a column to the importer's SELECT is how you break every
|
||
artifact already published**, so `artifactHasTotals()` asks the
|
||
attached artifact whether the column is there (on the writer, where
|
||
`core` is attached) and selects a literal `0` when it is not — the same
|
||
shape as the encoding probe beside it.
|
||
|
||
What neither side can give is *which* tracks are missing, only how many
|
||
— so an incomplete album still browses, and that is now the exception
|
||
rather than every album load. Two smaller consequences: existing databases
|
||
read "unknown" until a rescan repopulates the column (which degrades to
|
||
exactly the old behaviour, so nothing breaks), and our own `tagwriter`
|
||
writes track and disc *numbers* but not totals, so autotagging a folder
|
||
currently degrades the field this rests on.
|
||
|
||
**The absence is what gets marked, not the presence.** The tracklist
|
||
put a green tick against every owned track and a legend underneath
|
||
explaining the tick — a positive mark on the *common* case, so an album
|
||
you own outright wore a column of circles and a key for them. It is the
|
||
streaming-service treatment now: rows not in the library are **dimmed
|
||
in place**, and nothing marks the ones that are. Two things follow.
|
||
Dimming is a colour, so it cannot be the only signal — the row carries
|
||
`aria-disabled`, which is what reaches anyone not seeing it. And the
|
||
dimmed rows are why the `loading` banner could go: tracks arriving
|
||
dimmed reads as the album filling in, so a line of text about the
|
||
page's own plumbing earns nothing. `unavailable` survives because it is
|
||
not about plumbing — it says rows may be missing from the page
|
||
altogether, which nothing on screen can show. (`explore-artist-details`
|
||
still uses `loading`; it has no equivalent per-row signal.)
|
||
|
||
**A partly-owned album draws the release, not the part.** Once the tags
|
||
say nine of twelve, `buildLibraryEntry` shows the *catalog's* twelve
|
||
with three dimmed, rather than the nine on disk — the missing tracks
|
||
are the useful information and a tracklist trimmed to what is owned
|
||
cannot show them. It is guarded on `completeness.known` rather than on
|
||
"fewer tracks than the cluster", which would swap a catalog tracklist
|
||
in for every album whose tags simply never declared a total. A
|
||
side-effect worth knowing: this is what finally makes `ownership()`
|
||
say something true here, since counting the displayed tracklist of a
|
||
library-only entry could only ever produce "9 of 9".
|
||
|
||
**A dropdown is only a choice if the choices differ.** The version
|
||
selector tested `versionEntries.length`, but a release group routinely
|
||
has several releases — reissues, regional pressings, a remaster — whose
|
||
tracklists are identical, and the synthetic "Your Library" entry is
|
||
often a third name for the same one, so the control appeared with every
|
||
option showing the same rows. `distinctTracklistCount()` is the real
|
||
test. It deliberately does **not** use `fingerprint()`, which keys on
|
||
recording MBIDs alone: a library entry built from untagged files has
|
||
none, so every such tracklist fingerprints to the same run of empty
|
||
strings and compares equal to every other. It falls back to the title,
|
||
which is what lets a local copy be recognised as the same tracklist the
|
||
catalog is describing.
|
||
|
||
**Say which version you own, not that you own one.** A synthetic "Your
|
||
Library" entry used to stand in for the matching release, which hid the
|
||
thing worth knowing: you could see that you owned *a* version but not
|
||
*which*, while the real release — its date, country and release count —
|
||
sat underneath under a different name. The matching release carries
|
||
`inLibrary` and is marked (★ **and** the words "in your library", since
|
||
a `<select>` cannot be styled per option and a bare glyph is the
|
||
unexplained symbol the tracklist's green ticks were). The synthetic
|
||
survives only where there is nothing to mark: local files matching no
|
||
release, or the no-local-album overlap guess.
|
||
|
||
**A merged cluster shows the order the most releases agree on.**
|
||
`mergeNearDuplicateClusters` folds by track *set*, so a resequenced
|
||
pressing — same songs, different running order — merges correctly. But
|
||
the survivor was whichever release came first in the browse response,
|
||
which is meaningless ordering. On a real album one 2021 pressing
|
||
arrived ahead of eleven 2013 ones, so the cluster wore the 2021 running
|
||
order; the user's files then matched no cluster **fingerprint**, and
|
||
the page both called their copy unlinked to MusicBrainz and offered a
|
||
second "version" whose only difference was an ordering almost nothing
|
||
was pressed in. `withConsensusRepresentative` re-picks by how many
|
||
releases share each exact ordering, earliest date breaking the tie —
|
||
**and moves the cluster's fingerprint with it**, since that is what the
|
||
library match is tested against. Note the consequence for the version
|
||
list: a resequence is not a separate version, because the merge folds
|
||
it before any of this runs.
|
||
|
||
**A slow catalog fetch is not a failed one.** The same page used to
|
||
reach `unavailable` — "No catalog details for this album right now" —
|
||
from a **12-second timer**, which is the only signal it had, because
|
||
`ensureReleasesAsync` emitted `AlbumReleasesReady` on success and
|
||
nothing at all on failure. Against a browse queued behind eight
|
||
prefetches at 1 req/s, that reported healthy fetches as catalog
|
||
failures on correctly-matched albums. `AlbumReleasesFailed` is the
|
||
missing half; `catalogFailed` is the only route to `unavailable` now,
|
||
and the timer is a 60 s backstop for a genuine hang rather than the
|
||
verdict.
|
||
|
||
**Activating a row plays the list the row is in, from that row.** A
|
||
double-click — and Play on a single row's context menu — queues the
|
||
list as *displayed* with `startIndex` on that row, not a queue of one
|
||
track that stops when the song ends. The two playlist views and
|
||
`cover-grid`'s album dropdown always did this; the album page and
|
||
`track-list` did not, so playing anything from the two largest
|
||
tracklists in the app discarded the album around it. Three rules come
|
||
with it. **A menu asks how much is selected**: one row is a position
|
||
and means "from here", several rows are an explicit choice of *those*
|
||
tracks and become the queue on their own (which is also the only case
|
||
where `shuffleStart` still applies, since no one row was named as the
|
||
place to start). **The index is into the paths, not into the rows** —
|
||
`explore-album-details` queues `ownedFilePaths()` and its dimmed rows
|
||
are not in it, so an index taken from the tracklist starts an album
|
||
somewhere else entirely, or past its end. And **the index is looked up
|
||
when it is used, not remembered**: selection keys are file paths
|
||
because those survive a re-sort, a re-filter and a refetch, and an
|
||
index survives none of the three — `displayIndexOf` is that lookup,
|
||
against `cachedSortedTracks`, which is the only order the user can see
|
||
and therefore the only one they can mean.
|
||
|
||
**Ask for what the caller uses, once.** "Play this artist" resolved
|
||
file paths with one `GetAlbumTracks` per album, sequentially, and every
|
||
one of the four sites doing that asked for whole track rows to read
|
||
`FilePath` off them — 5 genres cost 6 MB over the IPC (`perf.m2`).
|
||
`GetFilePathsByAlbums(ids, libraryID)` and `GetFilePathsByGenres(names,
|
||
libraryID)` answer in one query and carry only the paths. They return
|
||
the paths **grouped by album id / genre name**, because the caller owns
|
||
the order — an album list is sorted by name, not by id, and a flattened
|
||
result would silently reorder a queue — and because `cover-grid`'s drag
|
||
cache stores them per album. A `libraryID` of 0 means "every library",
|
||
matching an unset library filter.
|
||
|
||
**A badge is a button only where it can act, and it says which.**
|
||
`library-status-indicator` — the tick/hourglass/plus on every Explore
|
||
card and track row — was a `<button>` whose click handler was a
|
||
`stopPropagation()` and a comment saying to wire up the download client
|
||
later: 20 of the 66 tab stops on a results page announced themselves as
|
||
buttons and did nothing. 007 made it `role="img"` on the rule that a
|
||
control which cannot act is worse than none, and named the condition
|
||
that would change the answer: a `<button>` again *with* a handler,
|
||
never a handler bolted onto something already shaped like one.
|
||
|
||
It is that now, and three rules hold it up. **A call site opts in** by
|
||
passing `request-mbid`, so a redundancy is visible in the template
|
||
rather than hidden in the component — `explore-album-details`'s header
|
||
has "Want this" in words directly below it and does not opt in. **An
|
||
owned entity is never a button**, because there is nothing left to ask
|
||
for, which is what stops the returned tab stops being spent on nothing.
|
||
And **the name is the action and the action is a request**: "Want album
|
||
X" / "Cancel the request for album X". Clicking adds a row to the
|
||
request list and nothing to the library, which is exactly what made the
|
||
original "Add … to library" a promise the control could not keep.
|
||
|
||
Two entities and not the third. A track is requestable because
|
||
`EntityRecording` is real work in the backend — `Reconciler.tracklistFor`
|
||
has a branch for it, since one expected title is what lets filename
|
||
matching score a single-track download at all. An artist is not: there
|
||
is no artist badge anywhere (`top-results-row` renders `nothing` for
|
||
one), and a discography subscription — never satisfied, expanding into
|
||
child requests — belongs on `explore-artist-details`'s Follow button,
|
||
which can say what it commits to.
|
||
|
||
Two smaller things, both still true: a `<span>` does not get
|
||
`box-sizing: border-box` from the UA stylesheet the way a `<button>`
|
||
does (the badge grew 36→38px, caught by a stored screenshot, and both
|
||
branches now set it), and the click is swallowed again — for the
|
||
opposite reason to before. With no action of its own the badge was part
|
||
of its card and a click on it meant what the card means; with one, it
|
||
does not. Enter and Space are stopped for the same reason, since every
|
||
card holding one is a `role="button"` or `role="option"` with its own
|
||
handler.
|
||
|
||
**Its third state was declared for a year and produced by nothing.**
|
||
`queued` was styled amber, given an hourglass and given the sentence
|
||
"… is queued for download", and all eight call sites were a two-way
|
||
ternary — so an album already on the request list showed a plus and
|
||
said it was not in the library, on the same page as a filled button
|
||
reading "Wanted". `utils/library-status.ts` is that rule written once:
|
||
`libraryStatusFor()` (owning outranks wanting; a *satisfied* request is
|
||
not queued, because nothing is coming; a request is by MBID, so a track
|
||
inside a requested album is not itself requested) and `toggleRequest()`
|
||
beside it, because the "Want this" button asks the same question and
|
||
two definitions of *what wanting means* is the fault this replaced.
|
||
|
||
**A grid moves by a row, and `offsetTop` cannot tell you how wide a row
|
||
is.** `utils/roving-grid.ts` measured columns by counting cards sharing
|
||
an `offsetTop`, and every card in these grids is positioned by
|
||
`lit-virtualizer` with a **transform**, which `offsetTop` does not see —
|
||
so all of them reported 0, every rendered card counted as one row, and
|
||
ArrowDown was `min(i + everything, last)` while ArrowUp was
|
||
`max(i - everything, 0)`. The vertical arrows were End and Home in the
|
||
albums, artists and genres grids alike, from the day it was written.
|
||
`getBoundingClientRect().top`, rounded, is the measurement. Two things
|
||
behind it are only visible once `cover-grid` splits: `scrollToIndex`
|
||
must pick the half that holds the index and rebase it (it was
|
||
`querySelector('lit-virtualizer')`, always the first), and the focus is
|
||
retried on a **time** budget rather than taken once at `updateComplete`
|
||
— a scroll of 5 000 rows produces the card a few hundred ms later, so
|
||
the tab stop moved and nothing took focus, which is indistinguishable
|
||
from the key not being handled.
|
||
|
||
**A view says what it is, in one component.** `<page-header>`
|
||
(`components/page-header/`) is title, count, sort and actions, and the
|
||
nine primary views use it rather than each writing its own — which is
|
||
how four came to have a heading and four not, and how the same sort
|
||
toolbar came to exist three times with different bugs. Three rules in
|
||
it are load-bearing. **An empty `heading` is a mode, not a missing
|
||
value**: `cover-grid` and `track-list` are also embedded in the artist
|
||
and genre pages, which have a heading already, so there they keep the
|
||
count and the sort and drop the title rather than growing a second
|
||
arrangement. **The header renders while the view loads** — a heading
|
||
that arrives with the data is the shifting layout it exists to stop —
|
||
and the *count* is `null` until there is an answer, because "0 albums"
|
||
that corrects itself a moment later is worse than saying nothing. And
|
||
**the header asks for a sort, it does not perform one**: the host owns
|
||
the field, the direction and their persistence, so the control cannot
|
||
disagree with the list.
|
||
|
||
**The header search box is view-scoped, and now says so.** It sits in
|
||
the app header and reads as global; typing `tide` on Playlists answered
|
||
"No playlists match your search" with three *Tideline* tracks in the
|
||
library. The scope was always real — the fix is that the placeholder
|
||
names it ("Search albums"), the page header repeats it ("Showing
|
||
artists matching ‘tide’"), and `search-store` holds the one map of
|
||
what each view searches. It also **keeps its slot everywhere and is
|
||
disabled** where it cannot serve, rather than being hidden: its
|
||
appearing and disappearing is what moved the library filter and the job
|
||
indicator on every navigation. On Explore, which has its own catalog
|
||
search, the disabled box points at it. **A view that filters on the
|
||
term belongs in that map**, detail views included —
|
||
`smart-playlist-details` narrowed its list as you typed under a
|
||
placeholder saying there was nothing to search here, because its
|
||
sibling was in the map and it was not.
|
||
|
||
**The window's minimum is measured, not aspirational.** `MinWidth`/
|
||
`MinHeight` are 800×600 because that is where the shell was checked to
|
||
still work: below ~780 the header subtitle wraps and pushes the title
|
||
out of the 4em bar, and below ~600 tall the eleven sidebar items stop
|
||
fitting at once. The old 512×384 promised a size at which Settings and
|
||
Jobs were unreachable — the sidebar clipped them with `overflow:
|
||
hidden` and nothing scrolled. The sidebar scrolls now, and collapses to
|
||
its (long-existing, previously drag-only) icon mode below 900px, which
|
||
is the same breakpoint that hides the subtitle.
|
||
|
||
**A row's columns must fit the row.** `track-list`'s
|
||
`computeDefaultWidths` shared out the host's whole `clientWidth` while
|
||
every row spends 24px on the favourite column and 2×8px on its own
|
||
padding first, so the grid was always exactly 40px too wide and the
|
||
last column was clipped at every size. Both numbers are constants read
|
||
by the three places that need them (the default widths, the
|
||
normaliser, and the resize handles' positions), because they were
|
||
written out separately and that is how they came to disagree.
|
||
|
||
**A phone draws one column of two lines, and that is a column set
|
||
rather than a second row template.** Measured on the device: at 424 px
|
||
the four configured columns fit the row *exactly* (`--grid-cols` came
|
||
out `24px 102px 101px 101px 80px`) and not one of them fit its content
|
||
— "Duration" did not fit its own header. The columns were never too
|
||
wide; there were too many of them. `PHONE_COLUMN_IDS` is `titleArtist`
|
||
(title over artist, sharing the row's whole width) plus the duration, so
|
||
the row, the delegated events, the selection semantics, the playing
|
||
marker and the virtualizer are all untouched: from their side only the
|
||
number of columns changed. Three rules come with it. **The row height
|
||
lives in two places and they must agree** — `PHONE_ROW_HEIGHT` and the
|
||
CSS rule — because the virtualizer positions rows from that number, so a
|
||
taller row overlaps its neighbour. **What is drawn and what can be
|
||
sorted are different questions**: the page header's sort list is built
|
||
from `configuredColumns`, or a phone (which has no column headers
|
||
either) could sort by nothing but title and duration. And **a phone's
|
||
widths are neither loaded nor saved**: `loadColumnWidths` is keyed by
|
||
column *id* and fills a gap with the minimum, so the stacked column —
|
||
which nothing can ever have saved a width for — came out at 148 px
|
||
beside a duration column of 236, and saving would have replaced the
|
||
width the user dragged on a desktop for the same id.
|
||
|
||
**The default columns are declared twice and must agree.**
|
||
`tracklist.DefaultColumns` is what a fresh install persists;
|
||
`DEFAULT_COLUMN_IDS` in `track-list/columns.ts` is what the list draws
|
||
until the config arrives. Album is in both (a library manager with
|
||
duplicate detection whose rows are track/artist/duration cannot tell
|
||
its own duplicates apart) — and changing either is invisible against an
|
||
existing `YJ_HOME`, whose `config.toml` already holds the old list, so
|
||
`make sandbox-seed NAME=default` before believing the app.
|
||
|
||
**Event-driven communication**: Backend emits events via Wails runtime; frontend stores subscribe to them. Event names are constants in `backend/events/`.
|
||
|
||
`frontend/src/events.ts` is **generated** from `backend/events/events.go`
|
||
by `backend/events/cmd/genevents` — never edit it. It renders a const
|
||
block's doc comment as TypeScript line comments, every line of it: it
|
||
used to prefix only the first, so a comment that ran to a second
|
||
paragraph emitted bare prose into the object literal and `make generate`
|
||
— a pre-commit hook — produced a file that does not parse.
|
||
|
||
**An event's cost is part of its meaning.** `TrackMetadataChanged`
|
||
means *the tags on disk were rewritten*, which can change an album, an
|
||
artist or a genre — so `library-store` answers it by discarding every
|
||
cached collection and refetching. That makes it the most expensive
|
||
event in the app, and it must not be reused for something cheap:
|
||
finishing a track used to emit it, which cost ~37 MB across the IPC and
|
||
~0.8 s of blocked main thread *per song* at 50 000 tracks, and cleared
|
||
the user's track selection while it did. `TrackPlayCountChanged`
|
||
carries `{audioFileId, filePath, playCount, lastPlayed}` — deliberately
|
||
everything needed to patch one track in place, so no consumer has any
|
||
reason to invalidate anything. The store replaces the tracks array
|
||
(consumers key memoized filter/sort caches on its identity) while
|
||
sharing every unchanged Track, and `track-list` **retains** its
|
||
selection across a refetch rather than clearing it, since the keys are
|
||
file paths and those survive one.
|
||
|
||
The same rule reaches the other way: **an event carries what a consumer
|
||
needs so it never has to invalidate.** `PlaylistTracksChanged` carries
|
||
the playlist id, and `playlist-store` refetches *that* playlist
|
||
(`GetPlaylistTracks`, plus `GetAllPlaylists` for the summaries, since
|
||
`UpdatedAt` is a sort key) rather than `GetAllPlaylistsWithTracks`,
|
||
which is every row of every playlist — 2.6 MB and 172 ms for one heart
|
||
before the fix. It falls back to a full invalidate only where a patch
|
||
cannot be shown to be equivalent: no id (the bulk paths emit one), a
|
||
cold cache, an unknown id, or a fetch already in flight. And **a store
|
||
with no subscriber fetches nothing**: `playlist-view` is the only
|
||
reader and is created lazily, so neither the invalidation nor — more
|
||
expensively — the singleton's own construction warms a cache for a page
|
||
that may never open.
|
||
|
||
**Playing a track does not wait for the database to hear about it.**
|
||
Every write in this app goes through one connection —
|
||
`database.DB` is `MaxOpenConns(1)`, because SQLite has one writer — and
|
||
a background pass can hold it for a long time. The player and the queue
|
||
used to write inline from paths that hold their own mutexes, so a
|
||
contended writer did not merely slow persistence down: `SetQueue`
|
||
blocked in `LoadFile`'s `saveState` and then in `persistState`, **while
|
||
holding `q.mu` and `p.mu`**. The user's report is the exact shape of
|
||
that — the track changed and the transport sat at paused (`LoadFile`
|
||
had emitted `TrackChanged` and `PlaybackStateChanged(paused)`, and
|
||
`p.Play()` is *after* the writes), nothing appeared in the queue
|
||
(`emitQueueChanged` is after them too), and the play button did nothing
|
||
because `Queue.Play` was waiting on the same held `q.mu`. It was
|
||
diagnosed by profiling the running app: 91% of its CPU was
|
||
`explore.BackfillLibraryDiscographies` → `upsertBatch`, with four more
|
||
of its six workers parked in `sql.(*DB).conn`.
|
||
|
||
So a write is **submitted, not performed**
|
||
(`queue/persistwriter.go`, `player/persistwriter.go`): jobs run in
|
||
submission order on one goroutine per component, each carrying its own
|
||
snapshot — which is what keeps "clear and rewrite the queue" and
|
||
"insert three tracks at 4" meaning what they meant when they were
|
||
called. Two rules come with it. A job **must not touch the component's
|
||
fields**: it holds no lock and the state has moved on, which is why
|
||
`persistTracks` clones. And `SaveState` — shutdown, and the tests —
|
||
still flushes and waits, because that is the one caller for which the
|
||
row has to exist on return.
|
||
|
||
The corollary for anything new: **a durability write is not a step in a
|
||
user action**. If a mutation path needs the database to have finished
|
||
before it returns, that is a claim worth arguing for, not a default.
|
||
|
||
**"Remove from library" removes the row and excludes the path, and
|
||
never touches the file.** `RemoveFromLibrary(filePaths)` deletes the
|
||
`audio_files` rows the way the scan's own orphan cleanup does (tagging
|
||
group bookkeeping, FTS entry, `pruneOrphanedMetadata` for an album
|
||
whose last track just went) and records each path in `excluded_paths`.
|
||
The exclusion is not an enhancement — without it the next scan finds
|
||
the file, sees no row and imports it again, so the button undoes itself
|
||
and is worse than no button. It is reached from the track list's
|
||
context menu behind `confirmAction()`, whose **impact line says the
|
||
files are not deleted**, and from `tracklist.delete` (Delete), which is
|
||
bound to *opening that dialog* and nothing else: one keystroke from a
|
||
focused row, a key that asks is defensible and a key that acts is not.
|
||
|
||
Four things about it are load-bearing, and two are invisible from the
|
||
track list. **The soft scan compares files on disk against rows in the
|
||
database**, so an excluded path — on disk, deliberately not a row —
|
||
makes the two disagree forever and queues a full scan of the whole
|
||
library on *every* launch; `surveyAudioFiles` and `countAudioFiles`
|
||
therefore both take the exclusion set, because they answer "how many
|
||
files would a scan import", not "how many files are there". **Deleting
|
||
an `audio_files` row cascades to `queue_tracks`**, so the removal calls
|
||
the same `CompactQueue` hook `RemoveLibrary` does, which reloads the
|
||
queue and unloads the player if the removed track was the one playing.
|
||
**A full rescan clears the exclusions**, which is the only way back for
|
||
a path removed by mistake until there is a UI for the list. And
|
||
`TracksRemovedFromLibrary` carries `{filePaths, count}` so
|
||
`library-store` splices the tracks array in place and refetches only
|
||
the album/artist/genre summaries — falling back to a full invalidate
|
||
only when a tracks fetch is already in flight.
|
||
|
||
**Deleting the file from disk is deliberately not this**, and is not
|
||
foreclosed; it needs its own argument.
|
||
|
||
**An unchanged payload is not an event.** `explore`'s index status used
|
||
to be pushed on a 3 s ticker for the life of the process, byte-identical
|
||
once the index was ready, and `config-page` assigns it to a `@state`
|
||
field — so a user who had once opened Settings paid a full re-render of
|
||
a 2 000-line template every 3 s, forever, for no news. `emitStatus` now
|
||
drops a status equal to the last one it sent, which is the rule stated
|
||
once instead of at twenty call sites. The corollary is load-bearing:
|
||
**every mutation of something the status derives must call `emitStatus`
|
||
itself**, because there is no longer a poll to notice it. `si.ready`
|
||
and `si.cancel` are both derived, and both were relying on the ticker.
|
||
|
||
**A cache on a cached view needs a ceiling, and so does everything
|
||
else holding what it holds.** `explore-view` never unmounts, and its
|
||
two art caches were plain `Map`s: twenty-four searches retained
|
||
20.58 MB and were still accelerating, because a cover thumbnail is a
|
||
~27 kB base64 data URL and an artist photo is a ~128 kB one. They are
|
||
`LRUMap`s now (`utils/lru-map.ts` — a `Map` re-inserted on read and
|
||
trimmed from the front), capped from the measured size of an entry and
|
||
kept several times larger than a screenful, since a cap below the
|
||
visible count evicts art that is still rendered and the re-render
|
||
fetches it straight back.
|
||
|
||
Two things about it are load-bearing. **A cap is only a bound if it
|
||
covers every reference**: the artist photo's data URL is held by both
|
||
`artistImageCache` and `exploreCache.artists`, so capping either alone
|
||
frees nothing at all and reads as a fix that did not work —
|
||
`ARTIST_IMAGE_CACHE_LIMIT` is exported and shared for that reason. And
|
||
**a bound has to stay checkable**: caches register with
|
||
`utils/cache-stats.ts`, so `window.__yjCacheStats()` reports entries,
|
||
retained chars and cap in one eval, rather than the next session having
|
||
to rebuild the twenty-four-search reproduction before it can tell
|
||
whether the ceiling still holds.
|
||
|
||
**Expanding an album shows its tracks, and the code to do it was
|
||
written and never called.** `cover-grid`'s dropdown — the album's
|
||
tracks drawn between the two halves of a split grid — was reachable
|
||
only from Enter/Space on a focused card (a plain *click* navigates to
|
||
`explore-album-details`), and that path fetched the tracks over the
|
||
IPC, ran the whole split state machine and then rendered the single
|
||
grid, because `render()` never consulted `splitMode`.
|
||
`connectedCallback` referenced `renderSplitGrid` purely to satisfy
|
||
`noUnusedLocals`. `perf.p2` files this as dead code in the bundle; it
|
||
is the only route from the albums grid to `track-details`.
|
||
|
||
Two things it needed that are not in the audit. **The grid could not
|
||
scroll at all**: `.grid-scroll-container` is the same markup
|
||
`artists-view` and `genres-view` use, and `cover-grid` had the class
|
||
with *no rule for it*, so the container grew to its full content height
|
||
inside an `overflow: hidden` host — 186 984 px of albums in a 772 px
|
||
box at 5 000 albums, unreachable by wheel, keyboard or scrollbar, and
|
||
invisible on the eight-album fixture. That is also the element
|
||
`scroll-manager.ts` saves and restores, so its `scrollTop` was
|
||
permanently 0; with a real scroller the manager works as designed
|
||
(2891 preserved exactly across an expand). And the shared context-menu
|
||
panel was **labelled "Album actions" unconditionally**, which nothing
|
||
could observe while the only menu that could open on a track was
|
||
unreachable.
|
||
|
||
The manager **moves the scroll to reveal the dropdown** rather than
|
||
preserving it — on a small library that is most of the way back to the
|
||
top (80 → 4, with the content *taller* after, so it is not clamping).
|
||
"The position is preserved" is the wrong assertion; "the dropdown is on
|
||
screen" is the contract.
|
||
|
||
**A list pays per row, and only while scrolling.** The track list's Art
|
||
column rendered `CoverArtPath` — the original artwork — into a 24 px
|
||
box while `CoverArtSmall` sat unused on the same model, and
|
||
`artists-view` linear-scanned every cached album per card per frame to
|
||
find an avatar fallback. Both are invisible to every test tier: nothing
|
||
renders differently and nothing fails, the app is just slower to
|
||
scroll. Pick the tier for the box you are drawing (`cover-grid`'s
|
||
`getCoverUrl()` is the model), always `loading="lazy" decoding="async"`
|
||
on a row image, and build a lookup keyed on the store array's identity
|
||
rather than searching it — the store replaces that array when its
|
||
contents change and shares the unchanged members, which is the same
|
||
signal `track-list`'s memoized caches key on.
|
||
|
||
**The same rule, on the selection path, was the worst stall in the
|
||
app.** Five components turned selected file paths back into tracks with
|
||
`filePaths.map(fp => tracks.find(…))`, so "Select all → Edit tags" at
|
||
50 000 tracks blocked the main thread for **three to six seconds**
|
||
(`perf.m6`). `utils/track-index.ts` is that lookup written once: a
|
||
`WeakMap` from the array's identity to a `Map<FilePath, Track>`, safe
|
||
for exactly the reason above and collected for free when the store
|
||
drops the array. **68 ms after.**
|
||
|
||
Its neighbour is a deliberate non-fix. `getSelectedKeysOrdered()` walks
|
||
the *list* rather than the selection, which the same finding calls out —
|
||
measured at **3 ms** for 50 000 items, so it stays a walk, with an early
|
||
exit once everything is found (which helps a selection near the top of
|
||
the list and, honestly, almost nothing at the bottom). The obvious fix —
|
||
keep each key's index beside it — is the one thing that cannot be done
|
||
here: an index goes stale on any re-sort, re-filter or refetch while a
|
||
file path survives all three, which is precisely why `retain()` drops
|
||
`lastSelectedIndex` and keeps the keys. Three milliseconds does not buy
|
||
a silently mis-ordered queue insert.
|
||
|
||
**Work in `updated()` runs on every pass, so it has to say what it
|
||
depends on.** `now-playing` measured and rewrote its text geometry
|
||
every update — six `querySelector`s and a read/write interleave — and
|
||
`player-store` notifies while playing, so it did that several times a
|
||
second about a component whose DOM had not changed (`perf.m5`). It now
|
||
runs only when its geometry key changes: the rendered title, the
|
||
rendered artist, **the two scroll flags**, or the ResizeObserver
|
||
reporting the panel resized. The scroll flags are in that list because
|
||
`.will-scroll .scroll-content` carries `padding-right: 2em`, so
|
||
applying the class changes the distance the marquee travels (−128 px
|
||
before it, −158 px after) — a guard on the text alone leaves every
|
||
first hover scrolling short, and nothing would fail. Reads before
|
||
writes in one place, so the interleave cannot come back.
|
||
|
||
Two corollaries. **Cost is measured where the work runs, not where it
|
||
is written**: those same reads cost 3 µs when the layout is clean and
|
||
0.103 ms when it is dirty, so the finding's magnitude only appears if
|
||
the DOM actually changed. And **a drag's document listeners belong to
|
||
the drag** — `track-list` and `now-playing` attach `mousemove`/`mouseup`
|
||
on `mousedown` and drop them on `mouseup` (and on disconnect, for a
|
||
drag interrupted by the component going away).
|
||
|
||
**A virtualized list repaints only when you tell it to, and the
|
||
accidental way you were telling it may be the thing you are about to
|
||
delete.** `<lit-virtualizer>`'s rows are produced by the `virtualize`
|
||
directive, which runs when one of the *virtualizer's own* properties
|
||
changes — not when its parent re-renders. So a list with memoized
|
||
`items` and a stable `renderItem` never repaints on host state, and
|
||
selection silently stops highlighting while the controller holds
|
||
exactly the right keys. Both playlist detail views virtualize now
|
||
(`perf.M5`: 22 090 elements and 2 000 eager cover requests for a
|
||
2 000-track playlist, against 487 and 0), and both therefore push
|
||
`virtualizer.requestUpdate()` on a selection change and on a
|
||
playing-track change, which is what `track-list` has always done.
|
||
|
||
The corollary is that **`artists-view` and `genres-view`'s per-render
|
||
arrow functions are load-bearing.** `perf.m1` asks for them to be
|
||
hoisted to stable fields the way `cover-grid` does; hoisting them is
|
||
what *stops* the virtualizer seeing a changed property, so the cards
|
||
keep whatever classes they had — measured at 1 highlighted card before
|
||
and 0 after, with no compensating win to pay for it.
|
||
`frontend/test/components/card-grid-repaint.test.ts` fails on that
|
||
change and exists for no other reason.
|
||
|
||
Two smaller rules from the same pass. A row inside a virtualizer needs
|
||
`width: 100%`, because the virtualizer positions its children
|
||
absolutely and a grid row otherwise shrinks to fit its content and
|
||
stops lining up with the header above it. And a panel that is closed
|
||
renders no list at all (`perf.m7`): `width: 0` and `contain` bound the
|
||
damage but do not stop a virtualizer inside from measuring its window
|
||
on every change, or `scrollToIndex` from calling `scrollIntoView()` on
|
||
something invisible.
|
||
|
||
Emit through **`events.Emit(ctx, name, data...)`**, never
|
||
`app.Event.Emit`. `TestNoDirectRuntimeEmits` fails the build on a
|
||
direct call anywhere outside `backend/events`.
|
||
|
||
**Its justification changed with v3 and is now the weaker one.** Under
|
||
v2 this was a safety rule: `runtime.EventsEmit` `log.Fatalf`'d —
|
||
unrecoverably, taking the process down — on any context not carrying
|
||
the runtime, which includes every `context.Background()`, so a
|
||
background worker could kill the app by emitting. That is gone. v3's
|
||
emit takes **no context at all**, and `application.Get()` with no app
|
||
running returns `nil` rather than dying, so `Deliver` is
|
||
`if app == nil { return ErrNoRuntime }` where it used to probe the
|
||
**v2-private context key** `ctx.Value("events")`. What remains worth
|
||
pinning is narrower and still real: one emit path is what lets
|
||
`emitStatus` drop an unchanged payload for every caller at once.
|
||
|
||
**`events.Emit` keeps its `ctx` anyway, and it is now purely a test
|
||
seam.** Delivery does not go through it; `events.WithSink(ctx, rec)`
|
||
does, which is how a service is asserted on in-process
|
||
(`backend/queue/emit_test.go` is the model). Dropping the parameter
|
||
would have churned 45 call sites and every test for no gain.
|
||
|
||
That wrapper is what makes services testable in-process: install a
|
||
recorder with `events.WithSink(ctx, rec)` and assert on the payload the
|
||
frontend would receive (`backend/queue/emit_test.go` is the model).
|
||
`events.Deliver` is the same call returning an error instead of
|
||
dropping, and has one legitimate caller — `/__test/emit`.
|
||
|
||
**Naming the Wails application costs cgo, so exactly two files may.**
|
||
v3's `application` package is GTK/WebKit bindings on Linux, and
|
||
`cmd/indexbuild` / `cmd/indexexport` are built in a plain `golang`
|
||
container with `CGO_ENABLED=0` — the index workflow says so and it is
|
||
the one job that must not fail, since it owns the ~205 GB checkpoint.
|
||
So the single `app.Event.Emit` lives in `backend/events/runtime_wails.go`
|
||
under `//go:build !indexbuild` (with `runtime_indexbuild.go` returning
|
||
`ErrNoRuntime`, which is what the app itself returns before Run), and
|
||
`explore`'s `ServiceStartup` — the only other thing in that dependency
|
||
tree naming `application` — sits in `backend/explore/servicestartup.go`
|
||
under the same tag. `TestIndexToolsDoNotImportWails` walks the dependency
|
||
graph with `go list -deps -tags indexbuild` and is what keeps it that
|
||
way; a `ServiceStartup` hook added to a package the index tools import
|
||
is the way this comes back.
|
||
|
||
## Code Generation
|
||
|
||
Two generators run via `go generate ./...` (or `make generate`):
|
||
1. **sqlc** — SQL → Go. Config at `backend/database/sqlc.yaml`. Add queries in `backend/database/sql/queries/`, get generated Go in `database/sql/sqlcgen/`.
|
||
2. **templ** — `.templ` files → `*_templ.go` files (same directory).
|
||
|
||
Pre-commit hooks verify generated code is fresh — always run `make generate` after changing `.sql` or `.templ` files.
|
||
|
||
## Code Style
|
||
|
||
- **Go**: golangci-lint v2 with strict linters including `err113` (static errors), `nlreturn`, `wsl_v5` (whitespace), `godot` (comment periods), `sloglint`, `perfsprint`. Imports grouped: stdlib → third-party → `yellowjacket/...` (enforced by gci).
|
||
- **TypeScript**: Strict mode, no implicit any, no unused locals/parameters.
|
||
- **Commits**: Conventional Commits, enforced by `scripts/commit-check.sh` —
|
||
a lefthook `commit-msg` hook locally, and a CI step over every commit in
|
||
a push. It is twenty lines of shell rather than commitlint, because the
|
||
grammar is one regex and commitlint would mean a Node dependency tree at
|
||
the root of a Go repo. **Its type list is `.releaserc.yml`'s**; keep the
|
||
two in step or semantic-release will decline to release something the
|
||
check accepted.
|
||
|
||
`.releaserc.yml` **is** what runs now, from `release.yml`, and it is why
|
||
the commit grammar is load-bearing rather than decorative: a merge to
|
||
`main` whose commits are all `chore`/`ci`/`docs` releases nothing, and a
|
||
mistyped `feat` ships a minor version. `make release-dry` answers "what
|
||
would this merge release" without pushing.
|
||
|
||
**`@semantic-release/github` is not in that config and must not be.**
|
||
Gitea's API is `/api/v1` and is not GitHub's surface, so
|
||
`@semantic-release/exec` calls `scripts/gitea-release.sh` instead — one
|
||
`POST`, which is the whole of the Gitea-shaped work. The community
|
||
plugin (`@saithodev/semantic-release-gitea`) was considered and
|
||
rejected: last published 2022, on `got@10`, declaring no peer
|
||
dependency on semantic-release at all.
|
||
|
||
Two things in it fail *silently* and are therefore pinned with their
|
||
reasons. **The notes come from `CHANGELOG.md`, not from an argument**:
|
||
release notes are rendered commit messages — arbitrary text carrying
|
||
backticks, quotes and `$` — so templating `${nextRelease.notes}` into
|
||
`publishCmd` would be a shell injection whose input is the commit log.
|
||
And **`conventional-changelog-conventionalcommits` is held at 9**,
|
||
because at 10 it is quietly incompatible with the writer
|
||
`release-notes-generator@14` pulls in: every release note renders as a
|
||
bare `## 0.0.1 (date)` heading with no sections and no commits beneath
|
||
it, no step fails, and the release ships with an empty body. Check the
|
||
rendered notes, never the exit code.
|
||
|
||
## Testing
|
||
|
||
Tests use `database.NewTestDB(t)` for in-memory SQLite, built by the same
|
||
`applySchema` production uses so the two cannot diverge. Test audio fixtures live in `test_data/music_library_test/`. Table-driven tests are the norm.
|
||
|
||
## Git Workflow
|
||
|
||
Feature branches and PRs are the only way in: **`main` is a protected
|
||
branch** (`enable_push: false`, an empty push whitelist, and `CI / check*`
|
||
+ `CI / e2e*` as required status checks), so a direct push is rejected by
|
||
the pre-receive hook. This file said otherwise for a long time. Tags are
|
||
*not* protected, which is what lets `release.yml` push one.
|
||
|
||
Pre-commit runs vet, lint, codegen check, and frontend typecheck in parallel. Pre-push runs the full test suite.
|
||
|
||
## CI
|
||
|
||
Seven workflows in `.gitea/workflows/`. Five of them package and
|
||
publish (`arch-package`, `homebrew-formula`, `index-artifact`,
|
||
`android-apk`, `desktop-assets`); `release.yml` decides *whether* four of
|
||
those run at all; only `ci.yml` gates, and it is the one to look at when
|
||
deciding whether a push was healthy.
|
||
|
||
**`release.yml` is the entry point for all of it.** On every push to
|
||
`main` it reads the Conventional Commits since the last tag and, if any
|
||
is releasable, writes the changelog, pushes the tag and creates the Gitea
|
||
release whose body is that changelog section. `arch-package`,
|
||
`homebrew-formula`, `android-apk` and `desktop-assets` are all keyed on
|
||
`v*`, so **the tag push is what starts them** — nothing is released by
|
||
hand any more.
|
||
|
||
Four things about it are load-bearing:
|
||
|
||
- **The tag is pushed with a user PAT, not the Actions token.** Gitea,
|
||
like GitHub, does not start a workflow from a ref pushed by a
|
||
workflow's own token (go-gitea#33123). The token is what decides this,
|
||
so `PACKAGE_TOKEN` is handed to semantic-release as the
|
||
`repositoryUrl` credential and the push is attributed to a person.
|
||
- **That same limitation is used deliberately, once.** semantic-release
|
||
calls the first release of a tagless repo `1.0.0` and offers no way to
|
||
say otherwise, so a `v0.0.0` floor tag is what makes the first release
|
||
`0.0.1` — and it is pushed with the *Actions* token precisely so it
|
||
triggers nothing. All four publishers additionally skip `v0.0.0`
|
||
explicitly, cleanly rather than by failing, because a floor is not a
|
||
shipment.
|
||
- **The release page is the changelog, and that follows from the branch
|
||
protection.** `@semantic-release/git` would push a `chore(release):`
|
||
commit back to `main`, which the pre-receive hook rejects — *after* the
|
||
tag had been pushed, leaving a tagged release the run then reports as
|
||
failed. Whitelisting the CI user was the alternative and was declined:
|
||
it weakens a protection someone set on purpose and lets a bot push to
|
||
`main` without the checks every human PR passes. So the plugin is
|
||
absent, `@semantic-release/changelog` writes to a gitignored
|
||
`.release-notes.md` purely to carry the notes into
|
||
`scripts/gitea-release.sh`, and `CHANGELOG.md` is a signpost to the
|
||
releases page rather than a file that would silently stop updating.
|
||
The workflow keeps its `chore(release):` guard anyway, for the day
|
||
someone adds the plugin back.
|
||
- **An asset upload waits for the release to exist.** semantic-release
|
||
pushes the tag in `prepare` and creates the release in `publish`, so
|
||
the tag push that starts these workflows happens *before* there is a
|
||
release id to attach to. `scripts/release-asset.sh` polls for it. The
|
||
capacity-1 runner serialises things enough that this would usually work
|
||
by accident, which is the worst kind of bug.
|
||
|
||
**Releases restarted at `0.0.1`, which is a downgrade on every channel.**
|
||
pacman and Homebrew both silently offer no upgrade from the old `1.x`,
|
||
and Android refuses the install outright — its remedy is an uninstall
|
||
that takes the user's library. This was chosen over pacman's `epoch` and
|
||
over offsetting `versionCode`, on the grounds that both are permanent and
|
||
a reinstall is once. `packaging/homebrew/README.md` and
|
||
`docs/android-release.md` say so where a user would look.
|
||
|
||
**`desktop-assets.yml` publishes Linux and nothing else, and macOS is not
|
||
an oversight.** `GOOS=darwin CGO_ENABLED=0` fails at
|
||
`wails/v3/pkg/mac: build constraints exclude all Go files` — the darwin
|
||
backend is Objective-C behind cgo, so a `.app` needs a macOS host and the
|
||
runner is a Linux container. That is exactly why the Homebrew formula
|
||
builds from source on the user's own Mac. Windows *does* cross-compile
|
||
cleanly (`GOOS=windows CGO_ENABLED=0`, a couple of seconds — oto uses
|
||
WinMM through `x/sys`, sqlite is modernc's pure-Go driver, WebView2 is
|
||
COM syscalls, MPRIS is `linux && !android`-tagged) and is deliberately
|
||
not published: no Windows build of this app has ever been *run*, and no
|
||
tier here can exercise one.
|
||
|
||
**`android-apk.yml` is the one that can lose something irrecoverable.** It builds the signed
|
||
`arm64-v8a` APK (the only ABI Android can run this app on — see
|
||
`app/build.gradle`) on every `v*` tag and publishes it to the *generic* registry, which is
|
||
readable without credentials — the reason Obtainium can poll a plain
|
||
URL. Android refuses to update an app whose signing certificate
|
||
changed, and the only remedy is an uninstall that takes the user's
|
||
library with it, so the job **refuses to build** without the keystore
|
||
secret rather than falling through to Gradle's debug-key default, and
|
||
**refuses to publish** an artifact whose certificate says `CN=Android
|
||
Debug`. It is deliberately not a job in `ci.yml`: that workflow runs on
|
||
every branch push, this one takes tens of minutes on a cold cache, and
|
||
the runner has capacity 1. `docs/android-release.md` is the operating
|
||
document.
|
||
|
||
Two jobs, both in an `ubuntu:24.04` container:
|
||
|
||
- **`check`** — no display: `make commit-check` over the push's commits,
|
||
`make lint` and `make test` (three build configurations each),
|
||
`tsc --noEmit`, `make ui-test`, `make bindings-check`,
|
||
`make skill-check`.
|
||
- **`e2e`** — under a private D-Bus and **no display at all**, since
|
||
v3's `-tags server` is a real headless mode: fixtures, a seed built by
|
||
running the app (`curl` against the runtime endpoint — no browser),
|
||
`make dev-headless`, then the Playwright suite
|
||
against **both** Chromium and WebKit. Playwright's Linux WebKit links
|
||
Ubuntu 24.04 libraries that Arch does not provide, so CI is the only
|
||
place it can run, and it is the closest available approximation of
|
||
the WebKit2GTK renderer that ships. The WebKit step carries
|
||
`if: ${{ !cancelled() }}`, without which a chromium failure skips it
|
||
— which is how the one source of WebKit signal came to produce none
|
||
for two sessions while the plan recorded it as "unverified".
|
||
|
||
**A failing job's log is readable, and `gitea_ci job_logs` is not the
|
||
only way.** That endpoint 404s on this Gitea build; the REST API does
|
||
not. `GET /api/v1/repos/{owner}/{repo}/actions/runs/{run}/jobs` gives
|
||
per-step status (which is how "WebKit was skipped" was found) and
|
||
`GET /api/v1/repos/{owner}/{repo}/actions/jobs/{job_id}/logs` returns
|
||
the whole log, with `Authorization: token $GITEA_TOKEN`.
|
||
|
||
Two things the container needs that a developer machine does not.
|
||
**It needs an audio device that keeps time**, because the player's
|
||
position is derived from what has been consumed — so a device that
|
||
accepts audio instantly makes every track finish instantly and the
|
||
clock never move. That is what ALSA's `null` plugin does, contrary to
|
||
a year of this file and `ci.yml` saying it paces on a timer: measured
|
||
through beep and oto with `player.InitSpeaker`'s own arguments,
|
||
**3000 ms of audio consumed in 2.96 ms**. It is a PulseAudio null sink
|
||
now (the same 3000 ms takes 3762 ms), started in system mode because
|
||
the job runs as root, with a step that plays three seconds and fails
|
||
if they take under two — a dependency with a rate, checked like one.
|
||
And
|
||
`YJ_CORE_INDEX_URL` points at a dead address so no run fetches the real
|
||
explore artifact, matching what `scripts/seed-sandbox.sh` already does.
|
||
|
||
**`make lint`'s tag sets must stay identical to `make test`'s**, or
|
||
lint is checking configurations nothing builds. There are three, and
|
||
the app's is now the *default* tag set: v3 resolves GTK4 +
|
||
WebKitGTK 6.0, which Arch and ubuntu:24.04 both ship, so the
|
||
`webkit2_41` tag that used to be mandatory everywhere is gone. A
|
||
machine without `webkitgtk-6.0` can still build with `-tags gtk3`, but
|
||
that is an escape hatch, not what CI or a release builds.
|
||
|
||
## Packaging
|
||
|
||
**The Makefile is the front door and `Taskfile.yml` is an
|
||
implementation detail behind it.** `make dev`, `make build-dev`,
|
||
`make build-prod`, `make bindings` and `make e2e` all keep their names;
|
||
what changed underneath is that a build is now a Taskfile tree
|
||
(`Taskfile.yml` → `build/<platform>/Taskfile.yml`) rather than one
|
||
`wails build` invocation, and `wails3` is still a **vendored Go tool**
|
||
(`go tool wails3`), never a global install.
|
||
|
||
Four things about that tree bite anything outside the Makefile, and all
|
||
four bit the packaging recipes:
|
||
|
||
- **The tasks invoke `wails3` by bare name**, in 54 places across the
|
||
scaffold files. `scripts/toolbin/wails3` puts that name on PATH
|
||
pointing back at the vendored tool; without it a build dies at its
|
||
first sub-task with `wails3: command not found`. The Makefile
|
||
prepends it, and so must `packaging/arch/PKGBUILD` and the Homebrew
|
||
formula.
|
||
- **`wails3 build` has no `-ldflags`, `-trimpath` or `-clean`** — those
|
||
were v2's. `-trimpath` and `-w -s` are already in the production
|
||
task's own flags; the version stamp goes through **`LDFLAGS_EXTRA`**,
|
||
this repo's one edit to the scaffold Taskfiles (linux and darwin
|
||
alike), passed as `wails3 task build LDFLAGS_EXTRA="-X 'main.version=…'"`.
|
||
- **The output is `bin/`, not v2's `build/bin/`.** `build/` is *tracked
|
||
build assets* now.
|
||
- **Bundling is a separate step from building.** `task build` produces
|
||
a bare binary on every platform; the macOS `.app` is `task package`.
|
||
|
||
**`build/`'s platform metadata is generated from `build/config.yml`.**
|
||
`wails3 task common:update:build-assets` rewrites `Info.plist`, the
|
||
`.desktop` template, `nfpm.yaml` and the Windows manifest from that
|
||
one file — so a hand edit to any of them is lost on the next refresh.
|
||
nfpm's `homepage` and `license` say in place that the refresh does not
|
||
own them, and **that comment is wrong**: a refresh reset them to
|
||
`https://wails.io` and `MIT`. Re-check those two after any refresh.
|
||
|
||
**That refresh does not touch the mobile trees**, contrary to what this
|
||
file said for five phases. `update build-assets` extracts only
|
||
`updatable_build_assets` (darwin/ios/linux/windows); `build/android/`
|
||
and `build/ios/` come from `generate build-assets`, which rewrites the
|
||
whole of `build/`. So `build/android/` is **committed and hand-edited
|
||
like source** — it was generated once into a scratch directory and
|
||
copied across (plan 015), it carries one deliberate edit to its
|
||
`Taskfile.yml`, and only its output is gitignored. `build/ios/` is
|
||
still not carried and its `includes:` entry is still dropped.
|
||
**Its `MainActivity` owns the safe area, because `targetSdk 35` does
|
||
not leave that to the theme.** Android 15 lays every app out
|
||
edge-to-edge and ignores the `statusBarColor`/`navigationBarColor` the
|
||
scaffold's theme sets, and the WebView is `match_parent`, so the page's
|
||
bottom band — the transport and, on a phone, the tab bar — would be
|
||
drawn under the gesture bar. `applyWindowInsets()` pads the container by
|
||
`systemBars | displayCutout | ime` and returns the insets rather than
|
||
consuming them; the window background is black to match the app's own
|
||
ramp, since that padding is what shows through. It is **pre-emptive**:
|
||
the phone this was checked against is Android 14, where the system still
|
||
insets the window, and the enforcement applies to an app *running on*
|
||
15. No browser tier can see this class of fault either way — a viewport
|
||
has no system bars.
|
||
|
||
**And a device is an engine, not just a screen.** The phone this app was
|
||
first run on renders in **Chrome 113** — two years behind every browser
|
||
any other tier uses — at a 424x439 CSS px viewport. It has `:has()`,
|
||
`color-mix()` and `dialog.showModal()`; it does **not** have relaxed CSS
|
||
nesting (Chrome 120, so a nested rule beginning with a bare element
|
||
selector is silently dropped), the Popover API (114, which Web Awesome's
|
||
popups set `popover="manual"` for), `light-dark()` or relative colour
|
||
syntax. So "it renders at that size in Chromium" is not evidence about
|
||
the phone, and resizing a spec cannot recover the missing signal. `make
|
||
android-inspect` forwards the WebView's devtools socket and `make
|
||
android-eval` asks the real page — raw CDP, because `connectOverCDP`
|
||
calls `Browser.setDownloadBehavior` and a WebView refuses it.
|
||
|
||
`build/config.yml`'s `version` is the
|
||
*metadata* version and is not what the app reports — `main.version` is
|
||
stamped at link time from the packaging recipe's git-derived version.
|