diff --git a/.planning/NOTES.md b/.planning/NOTES.md index 8e4ba30..b429d49 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -5078,3 +5078,22 @@ last rendered card. ("ask the virtualizer for a larger overscan") is therefore not available without patching a private, which is why the request is issued ahead of the element instead. + +## Declined, and recorded nowhere else (2026-09-25, #256) + +When CLAUDE.md was cut down to rules, most of its "considered and +declined" paragraphs already had a home in a code or config comment +beside what they explain. These three did not: + +- **`touch-action: manipulation` was declined** (#54). The 300ms tap + delay it is offered for is already absent on a `width=device-width` + viewport; what it would actually change is the gesture stack #63 tuned + by measurement on the reference device. +- **There is no "Go to Genre"** in the phone row menu (#67). That menu + replaces name links a phone cannot use, and there has never been a + genre link to replace — it would be new navigation, which wants its + own issue. +- **The overlaid queue has no tap-outside gutter on a phone** (#171). + The drawer-style gutter would buy the affordance by taking width off a + full-screen surface on a 424px viewport; back and a 44px close button + answer it instead. diff --git a/CLAUDE.md b/CLAUDE.md index 605b8a8..3ee96cc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,153 +1,57 @@ # CLAUDE.md -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. +Guidance for agents working in this repository. `AGENTS.md` is a symlink +to this file (`make skill-check` asserts it). -## Project +YellowJacket is a music player for desktop Linux/macOS and Android: Go +backend, TypeScript/Lit frontend, bridged by Wails v3. Plays MP3, FLAC, +OGG Vorbis and WAV. -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. +**Where things are written down.** `README.md` is for users; +`CONTRIBUTING.md` is setup, build and the contributor workflow. This +file is the rules. The *reasons* for a specific shape live in a comment +beside the code that has it — the code here is heavily annotated, and +the comment usually cites the issue — so read the comments of what you +are changing before changing it. `.planning/NOTES.md` holds dated +measurements, gotchas and rejected ideas; the `yellowjacket-dev` skill +(`.pi/skills/yellowjacket-dev/`) holds the operational how-to for each +test tier. Packaging channels keep their own documents +(`packaging/*/README.md`, `docs/android-release.md`). -**The three prose documents are split by reader, not by topic** (#50). -`README.md` is the landing page and answers *a user's* questions only — -what it does, which channel installs it on which platform, where its -data lives — with three screenshots in `docs/images/`, captured from the -fixture library (`make sandbox-seed NAME=default` → `make dev-headless -SEED=default`) so they can be retaken by anyone. `CONTRIBUTING.md` holds -what used to be the second half of that README — prerequisites, the -system libraries, the build and codegen commands, which verification -tier a change demands, the tracker workflow and the commit grammar. This -file stays the deep reference both of them point at, and is the only one -of the three that explains *why* a shape is what it is. A fact that -belongs to a user goes in one place; the packaging channels keep their -own documents (`packaging/*/README.md`, `docs/android-release.md`) and -are linked rather than summarised, because a version-restart note copied -into the README is a second copy to keep true. +## Issues and workflow -## Issues +The Gitea tracker is the source of truth, shared with a collaborator +who cannot see this session. `scripts/issue.sh` is the whole interface +(`list`, `mine`, `search`, `show`, `new`, `claim`, `unclaim`, +`comment`, `close`, `label`, `depends`, `labels`; needs `GITEA_TOKEN` +with `write:issue`). -**The tracker is the source of truth for what is wanted and what is -already being worked on**, and it is shared with a collaborator who -cannot see this session. `scripts/issue.sh` is the whole interface to -it (`list`, `mine`, `search`, `show`, `new`, `claim`, `unclaim`, -`comment`, `close`, `label`, `depends`, `labels`); it needs a -`GITEA_TOKEN` with `write:issue`. +- **Search before starting** (`issue.sh search `, which covers + closed issues too). **If nothing covers the work, open an issue + first.** +- **Claim before the first edit.** `claim` sets the assignee, + `Status/In Progress` and a comment naming the branch and approach. If + someone else holds it, talk to them rather than working around it. +- **File findings.** A bug found along the way, or work deliberately not + done, is an issue with a reproduction — not a sentence in chat. +- **Labels are a taxonomy**: `Kind/*`, `Area/*`, `Priority/*`, + `Platform/*`, `Reviewed/Confirmed`, `Status/*` (`Status/*` and + `Reviewed/*` are exclusive). **#73 is the roadmap**; work in its + order. Hard blockers are Gitea dependencies plus `Status/Blocked`. +- **`Closes #n` goes in a commit body, one issue per line.** Gitea does + not parse PR bodies, and a comma list half-works. After a merge, run + `issue.sh list --state open` and close whatever did not take. Unclaim + happens automatically on close (`.gitea/workflows/unclaim.yml`). +- **`main` is protected**: feature branch and PR only. The issue number + goes in the branch name and PR body, never the commit subject. A PR + body carries a commit-to-issue table, the verification actually run, + and a `Closes` list (PR #83 is the shape). -**Search the tracker before starting any work, and claim what you -find.** Fifty-odd issues make that a real lookup rather than a -formality. `./scripts/issue.sh search ` covers open and closed — -closed matters, because "that was fixed three weeks ago" is the -cheapest possible answer. - -**Claiming happens before the first edit, not before the commit.** The -whole point is that the collaborator can see the work is taken *while -it is being done*, so `claim` sets the assignee, applies -`Status/In Progress` and posts a comment naming the branch and the -approach — all three, or none. It refuses outright if somebody else -already holds it, and that refusal is the feature: talk to them rather -than working around it. - -**If no issue covers the work, open one first.** The issue exists -before the branch does. That is what makes the tracker a description -of the project rather than a description of the past. - -**Findings get filed.** A bug tripped over while doing something else -is an issue with a reproduction, not a sentence in a chat message -nobody can search. So is a piece of work deliberately not done — the -issue is where "we decided not to, and here is why" survives. - -Four conventions are already established and are not up for -reinvention: - -- **The labels are a taxonomy**, not tags: `Kind/*`, `Area/*`, - `Priority/*`, `Platform/*`, plus `Reviewed/Confirmed` (the code was - read and the defect confirmed) and the `Status/*` family. `Status/*` - and `Reviewed/*` are **exclusive scopes** — one of each at most, so - applying a second replaces the first. -- **#73 is the roadmap.** It states the order the backlog should be - worked in and the soft relations that are not expressible as - blockers. Picking work off the open list by eye when a meta issue - states the sequence is how the sequence stops meaning anything. -- **Hard blockers are real Gitea dependencies**, which render on the - issue itself, and the blocked issue carries `Status/Blocked`. -- **A PR body carries a commit-to-issue table, the verification - actually run, and a `Closes` list** — PR #83 is the shape. That list - is for whoever reads the PR; what actually closes an issue is the - footer below. - -**The closing keyword goes in the commit body, one issue per line.** - -``` -docs: delete four documents that contradict the code - - - -Closes #98 -``` - -**Gitea parses commit messages that reach `main`; it does not parse the -PR body**, which only closes anything if the merge happens to copy it -into the merge commit. Both halves of that were measured. #83's merge -commit carried `Closes #9, #13, #14, …` and closed **five of ten** — a -comma list is partially matched. #93's merge commit body was one -`Reviewed-on:` trailer, so #92 stayed open behind a perfectly correct -`Closes` line in the PR description. - -A footer costs nothing elsewhere: Conventional Commits allows one, -`scripts/commit-check.sh` only regexes the subject, and -semantic-release reads the type from the subject — so this changes no -release decision. The rule that the issue number stays out of the -**subject** is unaffected, and was never about the body. - -**Check it anyway.** A squash, or a merge message edited by hand, -still drops the footer. `./scripts/issue.sh list --state open` after a -merge, looking for what you just shipped; `./scripts/issue.sh close -` for whatever did not take, with a comment naming the commit. - -**Unclaiming is automatic, and it is hooked to the close rather than -to the merge.** Gitea's auto-close changes state and nothing else, so a -footer left `Status/In Progress` on a closed issue — #100 was closed -and marked as being actively worked on at the same time. -`.gitea/workflows/unclaim.yml` runs on `issues: [closed]`, which covers -the footer, `issue.sh close` and a click in the web UI alike; stripping -the label in the PR instead would have been a per-PR habit, and habits -are what the footer removed. It is not instant — the runner has -capacity 1 — and reopening deliberately does not restore the label. -`./scripts/issue.sh list --state closed --label "Status/In Progress"` -is how you find out it has stopped firing. - -## Planning - -`.planning/` is **design documents and measured history**, not a queue -— the queue is the tracker, and a plan file that describes work nobody -has started is a second, staler answer to "what are we doing next". - -- `.planning/NOTES.md` — gotchas, measured facts, open architecture - questions, and the "we already considered and rejected" list. Dated, - because several are properties of someone else's server. **This is - where a decision reached on an issue gets written down** when it - outlives the issue. -- `.planning/plans/completed/` — one recap per shipped milestone, kept - for the arguments in it. Where a plan shipped incompletely, its - header says which issue carries the remainder. -- `.planning/audits/` — the read-only audits that produced the - reconciliation plans. Historical evidence; not a backlog. -- `.planning/plans/active/` — a multi-phase design document for work - **in flight**, linked from the issue that tracks it. Empty is the - normal state. There is no `pending/`: a plan nobody is executing is - an issue. - -Numbering is sequential and stable across status moves (a plan keeps -its `NNN-` prefix). Abandoned plans are deleted. - -**The autonomous loop** (plan 020, `.pi/skills/yj-loop/`) is the pi -configuration that works the tracker one issue at a time — a cron tick -in a dedicated worktree and session, with the tracker labels as its -state machine. It claims with `issue.sh` like anyone, merges only PRs -it opened once the protection contexts are green, and files what it -finds. Its switch is `.pi/schedule-prompts.json` (gitignored): it runs -only while that pi session is open, and that limitation is the whole -on/off design. Where a loop discovery contradicts this file, this file -is wrong and should be fixed by the diary leg — the loop never quietly -decides otherwise. +`.planning/` is design documents and measured history, not a queue: +`NOTES.md`, `plans/completed/` (one recap per milestone), `audits/`, +and `plans/active/` (in-flight work only; usually empty). A decision +reached on an issue that outlives it goes in `NOTES.md`. The autonomous +loop (`.pi/skills/yj-loop/`, plan 020) follows the same rules. ## Commands @@ -166,7 +70,8 @@ make perf-compare BEFORE= AFTER= # 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 bindings # Regenerate frontend/bindings (wails3) +make e2e # Playwright 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 @@ -174,3889 +79,298 @@ 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 a doc names a make target that doesn't exist make commit-check # Fail if a commit subject is not a Conventional Commit +make css-check # Fail on a nested CSS rule Chrome 113 would drop 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 release-dry # What a release from here would ship 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: +Go tests: `go test ./...`, or `go test -run TestName ./backend/player/`. +Two tagged passes are **not** covered by `./...` (`make test` runs all +three): ```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. - -**A test file does not get its own origin, so `setup.ts` clears -`localStorage` between tests.** `@vitest/browser-playwright` opens one -BrowserContext per session and runs several files in it one after -another, so everything a component persists — the track list's sort and -column widths, the cover size, `now-playing`'s scroll mode — is still -there when the next file mounts the same component. Which files share a -tab, and in what order, changes run to run, so the symptom is a spec -that fails about one test in three and passes every time it is run on -its own: #138 cost three scheduled runs, one of them a PR whose diff -held no frontend code at all. Measured on the build before the fix, a -single full run started **24** tests with storage already set. Two -things follow. The clear is safe precisely because the leak is -sequential — files in a session do not overlap, so it cannot wipe -storage a concurrently-running file is in the middle of using — and it -belongs in `setup.ts` rather than in the specs that write, because the -spec that *reads* is never the one that knows. And a spec whose -assertion depends on an order still **states that order** rather than -inheriting a default, or the next change to a default is the same -mystery again. - -**`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. - -**An activity is a view onto the process, and `main()` runs once per -process.** On Android the Wails entry point is `nativeInit`, which -`MainActivity.onCreate` calls — and it does two things: it re-points the -native library's global JNI reference at the calling `WailsBridge`, and -it runs `go mainFunc()`. Android destroys and recreates an activity -**without restarting the process** (a configuration change the manifest -does not declare, memory pressure, or every background under "Don't keep -activities"), so `main()` ran again on a live app. `application.New` -returns the *existing* app rather than building a second one, -`app.Run()` then refuses — `a.starting` is still true, because Android's -`platformRun` is `select{}` and never returns — and the `os.Exit(1)` -under that error took the **first**, healthy app down with it: its -database, its queue, and the audio a `mediaPlayback` foreground service -was holding the process alive to play. `mainStarted` latches it, first -statement in `main()`. - -Four things about it are load-bearing. - -**The answer to "restore the session or cold-start" is settled by -playback, not by preference.** The audio lives in the Go process, so a -cold start on every activity recreation would stop the music mid-song — -which is the exact thing the foreground service exists to prevent. The -activity is a view; the app is the process. The frontend already -cooperates, because a recreated WebView loads the page fresh and fetches -its state from a backend that never went away. - -**Returning early is not a degraded mode, and that is why the latch is -in Go rather than in Java.** The obvious fix — making -`WailsBridge.initialized` `static`, so the second `nativeInit` is -skipped — keeps the process alive and silently breaks the app, because -skipping `nativeInit` skips the reference re-point too: Go would keep -executing JavaScript against the *destroyed* activity's WebView, and the -app would open, render, and never receive another backend event. The -latch lets `nativeInit` do its first job and declines only its second. - -**`ServiceShutdown` has never run on Android**, and nothing should be -built on the assumption that it will. `App.Quit()` reaches an -`androidApp.destroy()` that is an empty method, and `Run()`'s deferred -`shutdownServices()` cannot fire behind `select{}`. Durability on this -platform is the persist writers, which submit on every mutation rather -than at exit — which is also why `MainActivity.onDestroy` no longer -calls `bridge.shutdown()`: the activity going away is not the app -shutting down, and there is no callback for the process going away -because Android simply kills it. - -**No tier here can see any of this**, so the guard is split. A source -sweep (`TestMainClaimsBeforeItDoesAnything`) asserts the latch is the -*first* statement of `main()` — the failure it exists for is not -deletion, which is loud, but a line creeping in above it, since a second -`NewYellowJacketApp` opens the SQLite database again on every -recreation. The rest is a documented device check in -`.pi/skills/yellowjacket-dev/references/android-tier.md`, with the -logcat signature and a one-line way to force a recreation. - -`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. - - **Its rows are shown where the work is started, not on a page of - their own.** #27 folded the Jobs destination away, and the shape it - folded into is `` embedded four times — scans in - Settings → Libraries, index and enrichment in Settings → Search - Index, downloads under the download clients, the autotag apply in - `autotag-view`. One "Background jobs" section in Settings was the - obvious reading of the report and is the tab again under another - name. - - Four things about it are load-bearing. **Four of the five kinds - already had a home** that showed their work — the tier list, the - download list, the apply ring — and what none of them had is the - *generic* affordances, so the panel carries pause, cancel, Details - and the log to each rather than replacing what is there. **The - controls are `applyJobControl`**, not a reimplementation, which is - what keeps the "you will discard hours of downloading" confirmation - alive: it is keyed on `KindIndexBuild` inside the shared handler, and - a host drawing its own buttons would drop it silently. **A panel with - nothing to say is `hidden`**, host margin included, because an idle - panel in four places is four pieces of furniture describing an - absence. And **there is no "Clear finished"** in it, because - `ClearFinishedJobs` is global — a Clear under Libraries would discard - the index build's history too; a finished row dismisses itself. - - The header `job-indicator` is still the one view of everything at - once, from every page — **on a desktop.** One consequence worth - knowing before writing a spec: a section holding a `job-panel` also - holds a `job-details-drawer`, whose own header carries `.header` — so - `config-section .header` is ambiguous the moment a job exists. - - **Below 600px that indicator stands down and `` takes - over** (#62), because a popover is a *disclosure* and background work - is the one thing a phone should not make you open something to see — - and because #57 deletes the bar it is anchored to and is blocked on - it having somewhere else to live. The band is the same `job-panel`, - so `applyJobControl` and its index-build confirmation come along - rather than being reimplemented; `kinds="*"` is how it says "every - kind", which is what the indicator was for. - - Three things about it are load-bearing. **It is in the layout, not - over it**, as its own grid row above the main panel: the first - version put it in `notification-host`'s fixed band, which reads fine - in a screenshot and is unusable — at 424×439 a compact panel is - ~200px of a 439px screen and it *covers* what is under it, which four - e2e specs caught by failing on taps it was intercepting. **It shows - active work only** (`active-only`), because in flow a finished row is - furniture that keeps the content pushed down after the work is done; - finished rows stay where the work was started, which is #27's rule. - And **it renders nothing above 600px**, from `matchMedia` rather than - a media query, because that decides whether the element *exists* — - Settings already holds four `job-panel`s and a fifth answering for - every kind is `bottom-nav`'s "resolved to 2 elements" trap again. - `index.css` keeps it `display: none` outside the phone for a second - reason: an in-flow grid child with no named area is auto-placed into - one of the shell's rows, which is what the skip link is absolutely - positioned to avoid. -- `config` — TOML-based settings. Settings page uses HTMX + templ for server-rendered HTML fragments. - - **A setter that can reject its argument puts the old value back**, and - that is a correctness rule rather than hygiene (#231). `Save()` - validates the *whole* config, so a value left behind by a failed write - does not merely fail its own call: it fails every later save, of every - unrelated setting — theme, launch page, shortcuts, libraries — for the - rest of the session. Nothing reaches disk, so a restart clears it, - which is exactly what makes the fault invisible and unreportable. One - rejected track-list column list was enough to stop the app saving - anything at all. - - Two shapes are safe and a third is the trap. A setter that assigns and - *then* validates snapshots the field first and restores it on the - error path — seven do. `SetLibraryDirectory` is the better shape where - the value can be built on its own: it validates a candidate *before* - assigning, so there is nothing to undo. And a setter whose argument no - validation inspects needs neither — the bools, the favourites playlist - id and the shortcut bindings, plus `SetViewVisible`, which refuses an - unknown, non-hideable or launch-page view up front so - `GeneralConfig.Validate` never sees one it would fail on. Which set a - new setter joins is decided by whether its own `Validate` can reject - it, not by preference. -- `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. On that platform "the user's - level" is a constant, since #64 pins it at maximum and refuses every - way to move it; the duck is the one thing that still may, and it - works unchanged because it was always an offset applied *to* that - level rather than a write of it. -- `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. -- **A comparison is typed on *both* sides, and a parameter is the half - that gets forgotten.** The paragraph above is about a literal; the - artifact merge positioned its batch walk with a Go `string` cursor - against the artifact's byte column, and SQLite answered rather than - complained: `mbid > ?` with a text key is true of every row, so the - bound the walk looked up was the same every time and the cursor - never advanced, while `mbid <= ?` is false of every row, so no batch - merged at all. The import looped indefinitely at 100% CPU behind a - progress bar reading "0 of 1,077,893 rows", merged nothing and - raised nothing (#258). Nothing caught it because the fixture that - guards the walk writes the old text form and the only compact one is - a single row — below `artifactMergeBatch`, so the bound query never - ran. `artifactKey` types the cursor to the artifact's own encoding - now, and the walk fails loudly when its bound does not strictly - advance, because the failure mode here is silence rather than a - wrong answer. - -**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. **There are two launch navigations**, which is what -defeated that rule for five phases: the eager `navigate → home` at the -foot of `index.ts` and the configured page `GetDefaultPage()` resolves -to later. Only the first replaced, so a fresh session was already one -entry deep, the first back press replayed home over home, and on Android -`canGoBack()` was true so the press that should have exited the app did -nothing (#142). The landing-page navigation carries `_replace`, honoured -only while still at index 0 — past that the user has navigated during -the backend call, and a slow answer must not overwrite an entry they -made. 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. - -**And there is one statement of which view is active**, for the same -reason: `popstate` calls `handleNavigate()` directly and dispatches no -`navigate`, so the two nav components — which learned the active view -from that event — kept highlighting the view the user had just *left*. -`store/active-view-store.ts` is the shell saying where the user is, and -both navs read it through `ActiveViewController` rather than holding an -`activeView` of their own. - -Four things about it are load-bearing. - -**"Please go to X" and "the active view is now X" are different -statements**, and only the first existed — dispatched from 28 call -sites across 18 files. A re-dispatch from inside `handleNavigate` is -not the fix and cannot be: that function is the `document` listener for -`navigate`, so it is an infinite loop. - -**It is a store rather than an event, because a component that mounts -after a navigation still has to know.** `bottom-nav`'s "More" sheet -creates its `` on open, and that copy had heard no -`navigate` at all — standing on Albums, it opened highlighting -Home. An event has no answer for a listener that was not there. - -**A detail view is not a view here**, so the destination it was opened -from stays lit. `app-sidebar` did that by accident (it guarded on -`navItems.some(...)`, so an unmatched name left its highlight alone) -and `bottom-nav` had no such guard and so lit *nothing* — which is why -one looked right and the other looked broken on the same screen. -Whether a view is primary is the shell's fact: `view in VIEW_TAGS` is -passed to `setView`, never re-derived, because a second copy of that -list is a second thing to forget. - -**Nothing is lit until the shell has navigated.** The store starts -empty rather than defaulting to `home`, which is what `app-sidebar`'s -field used to do to match the landing view — a default that is correct -only while `GetDefaultPage()` agrees with it. - -**Back and forward are chrome, and the depth is the shell's own -count.** `` in the top bar is #6: the stack was always -global — every navigation is an entry and `popstate` restores any of -them in either direction — so what was missing was an affordance, since -the only way back was a detail view's own button, which leaves the -screen with the view it belongs to. The buttons dispatch -`navigate-back` / `navigate-forward` and the shell owns both guards, -for the reason the old `navStack` was deleted: a second caller reaching -for `history` is how two stacks come to disagree. - -Three things about it are load-bearing. **Forward is not back -negated**, so the single `pushedEntries` counter could not express it — -`popstate` carries no direction and fires identically both ways, so a -counter decremented on every pop reads a forward as a second back. Each -entry carries its index (`yjIdx`) and the shell keeps the current one -and a high-water mark; that also survives a jump of more than one, -which `history.go(-n)` and a long-press on a browser's back button both -produce. **A control that cannot act is `disabled` here**, which is the -documented exception to `library-status-indicator`'s rule: the two are -a pair whose positions the user learns, and hiding one moves the other -under the cursor. And **it stands down below 900px** — the top bar is -what runs out of room first below that (it already overflows 600px by -11px, #143), and nothing becomes unreachable: `nav.back` / `nav.forward` -(`Alt+Left` / `Alt+Right`, the browser's own combination, and clear of -the bare arrows that seek) are global at every width, and the phone has -the platform's gesture. - -The assertion is `aria-current="page"`, in -`e2e/specs/back-navigation.spec.ts`. That file existed throughout the -bug, covered exactly these journeys, and asserted only -`data-active-view` — the shell's own bookkeeping, which was right the -whole way through — so it was green on the broken build. Same trap as -`layout-overflow.spec.ts` and `page-header`: a spec named for the -behaviour, measuring the plumbing. - -**Which destinations exist is configuration, and hiding one takes away -the nav item and nothing else.** Eleven sidebar entries is more than -most libraries need (#25), so each is toggleable from Settings → -Navigation, Autotag is off until asked for, and Downloads is absent -until there is a client to download with — a destination for a feature -that cannot work is worse than none. `navigate` still resolves a hidden -view, which is not a nicety: detail views navigate into these and the -launch page is one of them. Nothing needed a special case for the -highlight either, because the paragraph above moved that onto -`active-view-store`: the sidebar asks `isActive(id)` per *rendered* -item, so a hidden view lights nothing exactly as a detail view does. - -Five things about it are load-bearing. - -**The stored shape is a map keyed by view id, and an absent key means -that view's own default** (`backend/config.Views`). That is what makes -this need no migration in either direction, and it is the polarity rule -`AllowMeteredCatalogDownload` states: the zero value is the intended -answer. A `HiddenViews []string` cannot express "Autotag off by -default" at all — its zero value is *hide nothing* — and a struct with -a boolean per view turns a view that later stops existing into stored -garbage. Here an unknown key is dropped on load and a view added later -gets its own default rather than being invisible or forcibly visible. -It is also what makes #73's `#25 → #27` order safe rather than -backwards: when Jobs folds into Settings, `jobs = true` in somebody's -config is a key nothing asks about. - -**Two states the user could not get out of are refused, in the config -and not in the checkbox.** Settings is never hideable and the launch -page is not hideable while it is the launch page. `config.toml` is -hand-editable, so a disabled checkbox is the affordance and -`SetViewVisible` is the rule — an app that can be locked out of its own -Settings by a typo in TOML is a support problem nobody can debug -remotely. On *load* the launch page is instead un-hidden rather than -refused: there is nobody to tell, and the honest reading of "my launch -page is Autotag" is that this user wants Autotag, not that their launch -page should be silently reset to something they did not choose. - -**Downloads is gated at the nav and not in the config**, on -`downloadStore.available`, so switching it on in Settings still means -what it says once a client exists and the tab appears without a restart -(#37's rule). `available` is false until the providers have loaded, -which makes the item *appear* on a fresh launch rather than appearing -and then vanishing. - -**The tab bar honours the toggles too, and the reason is local rather -than a general rule about phones.** `PHONE_COLUMN_IDS` is the precedent -for "what a phone shows is a different question", and it would apply — -except that `bottom-nav`'s "More" opens the *same* ``, -which filters, so an unfiltered bar would contradict its own sheet one -tap away. Which four tabs is still plan 016's committed subset; this -only removes from it, and "More" is never filtered because it is how -everything else stays reachable. - -**A retired destination is the one shape this does not make free.** An -absent visibility key takes its default and an unknown one is dropped, -but `DefaultPage` is a *value*: a launch page naming a view that no -longer exists fails validation, and on the load path that means the app -refuses to start for whoever had it selected. `RetiredViews` is that -list, and `ApplyDefaults` treats a retired name as a zero value while -an unknown-but-not-retired one still errors — a typo is worth being -told about. #27 retiring `jobs` is its first entry. - -**The list of destinations is `services/view-meta.ts`**, on -`shortcut-meta.ts`'s pattern, because #25 gave it a second reader: -Settings renders a toggle per view and needs the same labels in the -same order. Which views exist and what an unconfigured install shows is -Go's (`backend/config.Views`, which `DefaultPage`'s validation reads -too, so the launchable set is not a second list); how they are *drawn* -is the frontend's, beside the rest of the icon vocabulary. The binding -returns the **resolved** map for every view, so the frontend holds no -copy of the defaults — which would be the copy that shipped in the -binary rather than the one being edited. - -**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 `` 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 `` 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 -`

` in the same shadow root as the native `` 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 `` 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 `
` 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 -`