Merge pull request 'fix(database): preserve playlist phantoms across a stale audio_files retire' (#245) from fix/183-phantom-across-retire into main

Reviewed-on: #245

.pi/skills/yellowjacket-dev/references/fixtures.md

@@ -42,11 +42,16 @@ strings and identical specs produce different bytes on different builds.
   playback and then clicks pause races the track ending and fails
   against a correct UI. Use `LONG_TRACK` (90 s, `edge-lengths`) exported
   from `e2e/support/fixtures.ts`.
-- **WAV tracks scan in untitled.** `backend/tagwriter` writes WAV tags
+- **WAV tracks scan like every other format.** #104 added
-  into a RIFF `id3 ` chunk and `dhowden/tag` has no RIFF parser, so
+  `backend/riff`, so the scan reads the `id3 ` chunk `backend/tagwriter`
-  there is no "Field Recordings" artist in the Artists view. This is a
+  writes and both WAVs come in fully tagged: "Field Recordings" is an
-  known open bug pinned by `TestWAVTagsAreNotReadableYet`; do not
+  ordinary artist in the Artists view, with a "Test Tones" album and a
-  "fix" a spec by asserting the broken behaviour elsewhere.
+  cover. They are therefore not an example of an untitled or albumless
+  track — the only two tracks with no album are
+  `unsorted/no-tags-at-all.mp3` and `unsorted/title-only.mp3`. Prose
+  written before #104 says the opposite and names
+  `TestWAVTagsAreNotReadableYet`, a test that change deleted; that is
+  dated history rather than a description of the app.
 
 ## Seeds
 

.pi/skills/yj-loop/SKILL.md

@@ -81,6 +81,24 @@ rules. Never "go fix it" — the leg contract is in this file.
 | escalation | `yj-loop.escalate` | go/kimi-k3 (T3) | same leg re-run, seeded with failure summary |
 | prose (PR body, commit msgs, journal) | `yj-loop.scribe` | go/mimo-v2.5 (T0) | text only, from supplied facts |
 
+**Model fallback on quota exhaustion.** The pinned models are the
+intent, not a guarantee. The qwen token plan is a weekly pool and has
+run dry mid-tick (`429 … 1-week quota exhausted`). When a leg's launch
+fails with a 429, re-run it with a per-run `model` override one rung
+down and journal the substitution — never spend the T3 escalation
+model on a quota substitution. The qwen-pinned legs (`work`,
+`diffreview`) fall back `qwen/deepseek-v4-pro-0813` → `go/deepseek-v4-pro`
+→ `go/glm-5.3-flash`. Do **not** use the `deepseek/...` provider: it has
+no models, only catalog overrides, and fails silently (empty artifact,
+no session) — the model lives on the `go` gateway.
+
+**Launch legs in the foreground.** The async subagent runner has died
+without persisting a child session (nothing to resume) and emits
+spurious "needs attention" nudges on runs that are already complete.
+Foreground `subagent` calls are the reliable mode here. A worker that
+dies mid-leg leaves uncommitted work: inspect the tree, then relaunch
+to *complete* — never to re-implement.
+
 Orchestrator-only legs: **claim** (`issue.sh claim --branch` — atomic,
 refuses if held), **ship's PR/CI polling** (REST API below — `gitea_ci`
 job_logs 404s on this Gitea; the REST endpoints are the way), **merge**
@@ -163,7 +181,26 @@ Merge when, and only when, **all** hold:
 - the protection contexts `CI / check` and `CI / e2e` are green on the
   PR's head, read from the API, not from the PR page's badge;
 - the PR reports mergeable;
-- the critique leg ran and no open blocker stands.
+- the critique leg ran and no open blocker stands;
+- the branch is **not behind `origin/main`** — the protection's
+  `block_on_outdated_branch: true` refuses it anyway; never
+  `force_manually_merged` around it.
+
+**Refresh before every merge.** In the loop worktree: `git fetch origin`
+in the same breath, then `git merge origin/main` on the PR branch,
+push. The fetch must be immediate — a cached `origin/main` merges
+against the wrong base, CI goes green on it, and the merge comes back
+405 "behind base", one whole CI cycle wasted (measured on the adoption
+wave). A textual conflict
+stops the leg there — as diff text, not as a failed merge click: hunks
+the loop authored are resolved by the loop; anything else is left with
+`⟦loop⟧` comment for a human, never forced. After any refresh push,
+re-poll the PR's own required contexts on the **new head** before
+merging.
+
+**Merges happen one at a time**, each re-reading state — the previous
+merge moved `main`, and the next PR's mergeability is recomputed at
+its own turn.
 
 ```
 curl -sS -X POST -H "Authorization: token $GITEA_TOKEN" \
@@ -172,12 +209,20 @@ curl -sS -X POST -H "Authorization: token $GITEA_TOKEN" \
   -d '{"Do":"merge","merge_message_field":"default","force_manually_merged":false}'
 ```
 
-Afterwards: `scripts/issue.sh list --state open` and check the footer
+**Afterwards watch the `push` run on `main`** — the CI the merge
-took. Close stragglers with `issue.sh close`, naming the merge commit.
+started. A red main after a loop merge is a **halt**: comment what is
-`unclaim.yml` handles the label; it is not instant; reopening does not
+known on the offending PR, mark the state file, stop taking new issues.
-restore it. Merging fans out to nothing (releases are the manual
+That run is the only thing between a clean textual merge of
-`release.yml`, which the loop never runs) — the criticism stands before
+independently-written PRs and a self-contradicting main; no
-the merge because nothing stands after it.
+mergeability check sees it. Only a green main lets the tick proceed (to
+footer verification, below).
+
+Footer verification: `scripts/issue.sh list --state open` and check
+the footer took. Close stragglers with `issue.sh close`, naming the
+merge commit. `unclaim.yml` handles the label; it is not instant;
+reopening does not restore it. Merging fans out to nothing (releases
+are the manual `release.yml`, which the loop never runs) — the
+criticism stands before the merge because nothing stands after it.
 
 ## Rails — the loop's absolute rules
 
@@ -224,7 +269,10 @@ AVD), then `make android-emulator` per session.
 
 - **Worktree:** `git worktree add ~/.paseo/worktrees/loop/jumpy-hound
   origin/main` (from any clone; branch from origin/main in the loop
-  tree, never `git checkout main`).
+  tree, never `git checkout main`). **Provision it once before the
+  first push:** `make build-frontend` and `make testdata` — the pre-push
+  `go-test` hook needs `frontend/dist` (the `//go:embed` in `main.go`)
+  and the fixture library, and refuses the push without them.
 - **Session:** pi in that worktree, `/name loop`. Add the job via
   `/schedule-prompt` (name `yj-loop`, cron
   `0 0 10-18 * * 1-5`, prompt: "Read `.pi/skills/yj-loop/SKILL.md` and
@@ -248,3 +296,11 @@ AVD), then `make android-emulator` per session.
 - The job did not fire — the scheduler fires only while a session is
   open in its directory (documented); "the loop is off" is the correct
   reading, not a bug.
+- `error: object file … is empty` / `unpack-objects failed` / `bad
+  object refs/heads/…` during a fetch or checkout — the shared object
+  store was corrupted (a killed fetch leaves 0-byte object files, and a
+  local ref can end up pointing at the dead sha1). **Halt and report**;
+  do not retry, the churn only deepens it. Human repair: delete the
+  0-byte objects, `git fetch origin --prune`, delete any ref that
+  still dangles (`git update-ref -d refs/heads/<b>`), re-checkout the
+  worktree at `origin/main`, then `git fsck --full`.

.planning/plans/active/020-autonomous-backlog-loop.md

@@ -112,7 +112,9 @@ in the job's directory — that limitation is the switch:
 
 - **Worktree:** `git worktree add` a dedicated clone at
   `~/.paseo/worktrees/loop/jumpy-hound`. Loop edits happen only there; a
-  dirty tree there is the loop's business and nobody else's.
+  dirty tree there is the loop's business and nobody else's. **Provision
+  it once before its first push:** `make build-frontend` + `make testdata`
+  — the pre-push `go-test` hook needs both and refuses without them.
 - **Session:** pi in that worktree, `/name loop`. The job is bound to that
   session, so another pi elsewhere in the same directory does not
   double-fire it.
@@ -140,6 +142,20 @@ from a concurrent session is caught before the first edit.
 
 - **Only PRs the loop opened.** A collaborator's PR is never merged, never
   commented on for pressure, never touched.
+- **Every branch is refreshed against main before its merge**, in the
+  loop worktree — the refresh is where a textual conflict surfaces, as
+  diff text: hunks the loop authored are resolved there, anything else
+  is left to a human with a `⟦loop⟧` comment. The protection's
+  `block_on_outdated_branch` makes the refresh mandatory for adopted
+  (pre-loop) branches: behind `main`, a PR cannot merge at all.
+  Required contexts are re-polled on the refreshed head.
+- **Merges are one at a time**, each re-reading state — the previous
+  merge moved `main`, and the next PR's mergeability is recomputed at
+  its own turn.
+- **Post-merge, the `push` run on `main` is watched.** A red main after
+  a loop merge halts the loop. That run is the only guard against the
+  class no mergeability check sees: two PRs touching the same file,
+  merging cleanly, contradicting each other.
 - The gate is the protection rule itself, read from the API: contexts
   `CI / check*` and `CI / e2e*` green, PR mergeable. (Required approvals
   is 0 today; if a second person changes protection rules, the merge

.planning/plans/active/021-listening-accounting.md

@@ -0,0 +1,263 @@
+# 021 — Listening accounting: smart plays, skips, and a real history
+
+**Issue:** none yet — open one before the first edit (tracker is the
+source of truth; `./scripts/issue.sh search "skip play count"` comes
+back empty as of this writing).
+
+**Status:** plan — not started.
+
+**Relates:** play-count rendering (`frontend/src/components/track-list/columns.ts`),
+smart playlists (`backend/smartplaylist/`), the event contract
+(`TrackPlayCountChanged`), and any future Wrapped / "minutes listened"
+surface.
+
+---
+
+## What exists now
+
+Three facts, all load-bearing.
+
+**A "play" is recorded only on a natural finish.** `recordPlay`
+(`backend/queue/playhistory.go:9`) is called from exactly one place —
+`OnPlaybackFinished` (`backend/queue/handlers.go:14`), and only when
+`srcErr == nil`. A track the user skips past at 90% is *not* a play;
+neither is one they pause at 60% and abandon. `play_count` /
+`last_played` on `audio_files` reflect "finished to the end," nothing
+more.
+
+**There is no skip concept at all.** Skipping is indistinguishable
+from a natural finish, a pause, or a shutdown. Nothing records "the
+user rejected this track," so no downstream feature (smart playlists,
+shuffle, the revisit shelf, a future skip-rate heuristic) can ask
+about it.
+
+**`play_history` is a write-only log.** It holds
+`(audio_file_id, played_at)` and nothing reads it — no sqlc query
+touches it, no `PlayHistory` read path exists. Its only recorded
+purpose is the timestamps a future "minutes listened over time"
+feature would need. It is classified `Authored, Cascade` in
+`backend/datamap/datamap.go:272` ("Listening history").
+
+So the gaps are: (1) skips are invisible, and (2) "played" is
+under-counted — the opposite of the usual over-counting fear. The
+scrobble intuition (count a play once `min(50%, 4:00)` has been
+*heard*, independent of how it ends) is the fix for both.
+
+---
+
+## What we're building
+
+A single classification of every track *exit*, plus one row per exit in
+a listening log, plus the existing denormalized `play_count` /
+`last_played` updated to match the new meaning. Three exit kinds:
+
+| kind | condition |
+|---|---|
+| `complete` | reached natural end, **or** abandoned with `remaining <= tail` |
+| `play` | heard `>= playThreshold`, abandoned before the tail |
+| `skip` | user moved to a *different* track before `playThreshold` |
+
+Not counted, not any kind: decode failure, pause/stop/shutdown before
+the threshold, and tracks shorter than `minTrackLength`.
+
+### The thresholds — named judgements, one file
+
+Follow the `PreviousRestartThreshold` precedent (`backend/queue/queue.go:28`,
+a bare `const` with a comment). A new `backend/queue/listen.go` (or a
+tiny `backend/listencount` package) declares:
+
+```go
+const (
+    // A track this short is deliberated jingle / interstitial and is
+    // never counted, either way.
+    minTrackLength = 30 * time.Second
+    // The scrobble rule: half the track, or four minutes, whichever
+    // comes first (Last.fm / ListenBrainz).
+    playThresholdMax = 4 * time.Minute
+    // "Finished enough": within 15s of the end, or the last 10%,
+    // whichever is larger.  A 10:00 ambient track gets a 60s fade
+    // window; a 2:00 pop song gets 15s.
+    tailWindowFloor    = 15 * time.Second
+    tailWindowFraction = 0.10
+)
+
+func playThreshold(d time.Duration) time.Duration {
+    return min(d/2, playThresholdMax)
+}
+func tailWindow(d time.Duration) time.Duration {
+    return max(d/10, tailWindowFloor)
+}
+```
+
+Classification is a pure function of `(reason, position, duration)` and
+*therefore unit-testable without a player*:
+
+```go
+func classify(reason ExitReason, pos, dur time.Duration) Kind
+```
+
+`ExitReason` is `finished | skipped | failed | abandoned`. `skipped`
+means the queue moved to a different track by user action (Next,
+Previous past the restart threshold, PlayIndex, queue replacement,
+select-from-a-list). `failed` is the decode-error path. `abandoned` is
+pause/stop/unload/shutdown — and in v1 is a no-op (see open question 3).
+
+**"Heard" is approximated by the position at exit.** We read
+`player.CurrentPositionSeconds()` at the moment of the transition, not
+an accumulated listen-time ledger. A user who seeks to 80% and listens
+5 seconds reads as "heard 80%." That is deliberately accepted for v1:
+it is how most players actually behave, it is drastically simpler, and
+the failure mode ("counted a track you skimmed as played") is mild and
+exactly what the scrobble threshold already forgives. Written down
+because "position is not listen time" is the one assumption that will
+look like a bug if it is not.
+
+**Fires once per listen.** Leaving a track already leaves it; the
+`chainID` guard in `player.onPlaybackFinished` (`backend/player/player.go:633`)
+already swallows a stale finish callback, and a transition advances
+`currentIndex` past the finished track. The classifier needs the same
+guard so a Next-then-stale-finish cannot produce two rows. Key it on the
+`(audioFileID, chainID)` the transition was about.
+
+---
+
+## Schema — resolved: fresh design, no migration
+
+The A/B migration agonizing is moot.  This app has two users and both
+are devs, and play counts are explicitly not worth preserving yet — so
+the schema is written as if listening accounting had been designed in
+from the start, and the existing two databases rebuild what they need
+(see below).  There is no migration step and none is re-introduced.
+
+**`play_history` is renamed to `listening_events`** and grows the three
+kinds, plus the raw position/duration the classification was made from:
+
+```sql
+CREATE TABLE IF NOT EXISTS listening_events (
+    id               INTEGER PRIMARY KEY,
+    audio_file_id    INTEGER NOT NULL,
+    kind             TEXT NOT NULL DEFAULT 'complete'
+                     CHECK (kind IN ('complete','play','skip')),
+    position_seconds INTEGER NOT NULL DEFAULT 0,
+    duration_seconds INTEGER NOT NULL DEFAULT 0,
+    occurred_at      DATETIME NOT NULL DEFAULT (datetime('now')),
+    FOREIGN KEY(audio_file_id) REFERENCES audio_files(id) ON DELETE CASCADE
+);
+CREATE INDEX IF NOT EXISTS idx_listening_events_audio_file_id
+    ON listening_events(audio_file_id);
+CREATE INDEX IF NOT EXISTS idx_listening_events_occurred_at
+    ON listening_events(occurred_at);
+```
+
+`position_seconds`/`duration_seconds` are kept raw so a future re-tune
+of the threshold does not force the events to be re-recorded.  `kind`
+stays the write-time classification; the raw reading is evidence, not
+a second copy of the rule.
+
+**The counters are denormalized onto `audio_files`** — `skip_count` /
+`last_skipped` join the existing `play_count` / `last_played`, because
+that is where the hot read path already lives and a log join per track
+row is not acceptable.  This does grow the MIXED-KIND wart (see the
+survey below for the structural answer), but it is the *continuation* of
+the existing design, not a new leak: play counts sat on `audio_files`
+from before this feature existed.
+
+**What happens to the two real databases on next launch.**
+`listening_events` is a new table, created verbatim.  `audio_files`
+gains two columns, which `retireStaleTables` treats as a stale Owned
+table and rebuilds by rescan — dropping `play_count` / `last_played` /
+`tag_status` with it, which is the accepted cost stated in the issue.
+`play_history` is gone from the schema and the datamap, so
+`obsoleteTables` drops it; its (natural-finish-only) timestamp rows go
+with it.  Nothing here is wrong on a fresh install, and on the two dev
+machines the answer is the documented "delete and rescan."
+
+---
+
+## Wiring: where the classifier is called
+
+The risk is not the classifier — it is that **every track-replacement
+path must classify the outgoing track**, and there are many: `Next`,
+`Previous` (past the 3s restart threshold), `PlayIndex`, `playFromStart`,
+`SetQueue` / clear-and-play, remove-current, and select-from-a-list.
+Miss one and that path silently never records a skip.
+
+So the classification is centralized in one queue method —
+
+```go
+// leaveCurrent(reason) classifies the track at currentIndex as it is
+// about to be replaced, and records exactly one listening event.
+// Must be called without q.mu held (it writes to SQLite).
+func (q *Queue) leaveCurrent(reason ExitReason)
+```
+
+— which reads position/duration from the player, calls `classify`, and
+emits the play/skip row + `TrackPlayCountChanged` when `kind != skip`.
+`OnPlaybackFinished(nil)` routes through `leaveCurrent(finished)`, the
+navigation methods route through `leaveCurrent(skipped)` before they
+advance, and `recordPlay` becomes the "did a play happen" half of it.
+
+Because "one path forgot to call it" is the failure mode, a **source
+sweep** pins it, on the pattern of `TestNoDirectRuntimeEmits`
+(`backend/events/noemit_test.go`) and `TestCatalogCoversSchema`: a test
+walks `backend/queue` for assignments to `currentIndex` (and the
+`SetQueue` / remove paths) and fails if a mutation site does not sit
+adjacent to a `leaveCurrent` call. The sweep is the enforcement; the
+central method is the convenience.
+
+`recordPlay` keeps its existing contract *when a play happens* —
+`TrackPlayCountChanged` with `{audioFileId, filePath, playCount,
+lastPlayed}` — so the frontend patch path and
+`playhistory_test.go` keep passing. A skip emits no per-track event in
+v1 (open question 4).
+
+---
+
+## Phases
+
+1. **The classifier.** `listen.go`: the constants, `playThreshold`,
+   `tailWindow`, `classify`. Table-driven unit tests covering every
+   cell of the tristate, the <30s exemption, the tail window on both a
+   10:00 and a 2:00 track, and the clip at the 4:00 cap. No I/O.
+2. **Schema.** *Done in this session.* `listening_events` replaces
+   `play_history`; `skip_count` / `last_skipped` added to
+   `audio_files`; datamap entry and `TestAuthoredCascadesAreDeliberate`
+   allow-list renamed; `recordPlay` writes `listening_events
+   ('complete')`.  `make generate` run; database / datamap / queue
+   tests green.
+3. **Wiring.** `leaveCurrent`, the navigation/finish/error call sites,
+   the `fires once per listen` guard, and the source sweep. Extend
+   `playhistory_test.go` for skip/complete classification through the
+   queue rather than the pure function.
+4. **Smart-playlist field.** `skip_count` (and optionally
+   `days_since_skipped`) in `smartplaylist.go` field/numeric maps and
+   the editor's field list, via subquery. A frontend event for skip —
+   if a UI wants a skip column — follows separately.
+
+## Verification
+
+- **Go:** the classifier is pure and exhaustively unit-tested; the
+  queue wiring is tested in-process with `events.WithSink`
+  (`backend/queue/emit_test.go` is the model), asserting a Next at 90%
+  emits a *play*, a Next at 10% emits a *skip and no play*, a natural
+  finish emits a *complete*.
+- **Database:** schema + datamap tests fail-loud on any new or
+  reclassified table; `database_test.go`'s listening-events round-trip
+  asserts the new table and the four denormalized counter columns.
+- **e2e:** `e2e/specs/play-count.spec.ts` already awaits
+  `TrackPlayCountChanged`; add the skip case (advance early, assert no
+  `TrackPlayCountChanged` and a `skip` row via the `__/test/sql`
+  endpoint if convenient, or via the playlist effect).
+- No visual/component tier needed unless a skip column ships (phase 4).
+
+## Open questions / decisions needed
+
+1. **Migration mechanism.** Resolved — fresh design, no migration (see the schema section).  Play counts are not worth preserving, both users are devs, and `audio_files` / `play_history` rebuild-or-drop on next launch.
+2. **"Position is not listen time."** Accept the approximation for v1,
+   or track accumulated listen seconds (a real ledger on the player) now?
+3. **Abandon on shutdown.** A track paused at 70% and then app-killed:
+   count a `play` (scrobble says heard) or leave it unrecorded? v1
+   proposes *unrecorded* — same as today — to keep the write path off
+   the shutdown critical path.
+4. **Skip event to the frontend.** Emit now (parallel to
+   `TrackPlayCountChanged`) or only when a surface consumes it?

CLAUDE.md

@@ -172,7 +172,7 @@ 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 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 lint             # golangci-lint v2 (strict), all three build configurations
 make test             # All tests with race detector, all three build configurations
@@ -673,6 +673,28 @@ rather than renaming them.
   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
@@ -3286,6 +3308,27 @@ 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.
 
+**And the *valid* columns are declared twice too, which is the pair
+that drifted.** `tracklist.AllColumnIDs` is what the backend accepts;
+`COLUMN_DEFS` is what the frontend knows how to draw, and they are not
+the same set — `titleArtist` is a definition and not a choice, since it
+is the phone's stacked column and is picked by width in
+`PHONE_COLUMN_IDS`. Settings built its list from `Object.keys(
+COLUMN_DEFS)` and so offered it: **two rows both called "Track Name"**
+(#197), the second unselectable, because ticking it sends a column set
+Go rejects with `unknown track-list column ID` and `config-page`
+swallows that into a `console.error`. `CONFIGURABLE_COLUMN_IDS` is what
+the configurator reads now, derived from a `configurable` flag on the
+definition, and `settings-column-list.test.ts` reads Go's own list out
+of the source rather than writing it down a third time — the rule being
+about every column, so checking one checks nothing.
+
+One thing it does **not** fix, because it is reachable from any invalid
+input rather than from that row: `SetTrackListColumns` assigns before it
+validates, so a rejected list stays in memory and `Save()` validates the
+whole config — one tick and **no setting saves for the rest of the
+session**, silently. That is #231.
+
 **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`

Makefile

@@ -192,7 +192,7 @@ css-check: ## Fail on a css`` literal ended early by a backtick, or a nested rul
 # Every command in them is a make target on purpose, so this is
 # checkable.  It also asserts AGENTS.md is a symlink to CLAUDE.md, so the
 # two harnesses cannot drift onto two descriptions of one project.
-skill-check: ## Fail if the agent docs name a missing make target, or AGENTS.md is not a symlink
+skill-check: ## Fail if the docs name a missing make target, or AGENTS.md is not a symlink
 	@./scripts/skill-check.sh
 
 # Conventional Commits, which CLAUDE.md claimed CI enforced for a long

backend/config/config.go

@@ -303,6 +303,24 @@ func (c *Config) GetLibraryDirectory() string {
 	return string(c.Library.DirectoryPath)
 }
 
+// A rejected setter puts the old value back, and that is not tidiness
+// (#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, silently and for the rest of
+// the session.  Nothing reaches disk, so a restart clears it -- which
+// is exactly what makes the fault hard to see and impossible to report.
+//
+// The setters below that assign and then validate therefore snapshot
+// the field first and restore it on the error path.  SetLibraryDirectory
+// is the other safe shape and the better one where the value can be
+// built on its own: it validates a candidate *before* assigning
+// anything, so there is nothing to undo.
+//
+// Not every setter needs either.  A bool, an int64 and the shortcut
+// bindings pass through no validation that can reject them, and
+// SetViewVisible refuses an unknown, non-hideable or launch-page view
+// up front, so GeneralConfig.Validate never sees one it would fail on.
+
 // SetLibraryDirectory validates and saves a new library directory,
 // then emits the LibraryConfigChanged event so listeners (e.g. the
 // Library scanner) can react.
@@ -360,11 +378,14 @@ func (c *Config) SetScanConcurrency(mode string) error {
 		c.Library.ApplyDefaults()
 	}
 
+	previous := c.Library.ScanConcurrency
 	c.Library.ScanConcurrency = library.ScanConcurrency(
 		mode,
 	)
 
 	if err := c.Library.Validate(); err != nil {
+		c.Library.ScanConcurrency = previous
+
 		return fmt.Errorf(
 			"invalid scan concurrency mode: %w", err,
 		)
@@ -455,9 +476,12 @@ func (c *Config) SetThemeAccentColor(
 		c.Theme.ApplyDefaults()
 	}
 
+	previous := c.Theme.AccentColor
 	c.Theme.AccentColor = color
 
 	if err := c.Theme.Validate(); err != nil {
+		c.Theme.AccentColor = previous
+
 		return fmt.Errorf(
 			"invalid theme accent color: %w", err,
 		)
@@ -488,9 +512,12 @@ func (c *Config) SetThemeBackgroundShade(
 		c.Theme.ApplyDefaults()
 	}
 
+	previous := c.Theme.BackgroundShade
 	c.Theme.BackgroundShade = theme.BackgroundShade(shade)
 
 	if err := c.Theme.Validate(); err != nil {
+		c.Theme.BackgroundShade = previous
+
 		return fmt.Errorf(
 			"invalid theme background shade: %w", err,
 		)
@@ -544,9 +571,12 @@ func (c *Config) SetDefaultPage(page string) error {
 		c.General.ApplyDefaults()
 	}
 
+	previous := c.General.DefaultPage
 	c.General.DefaultPage = View(page)
 
 	if err := c.General.Validate(); err != nil {
+		c.General.DefaultPage = previous
+
 		return fmt.Errorf(
 			"invalid default page: %w", err,
 		)
@@ -591,9 +621,12 @@ func (c *Config) SetQueueFallback(mode string) error {
 		c.General.ApplyDefaults()
 	}
 
+	previous := c.General.QueueFallback
 	c.General.QueueFallback = QueueFallback(mode)
 
 	if err := c.General.Validate(); err != nil {
+		c.General.QueueFallback = previous
+
 		return fmt.Errorf(
 			"invalid queue fallback: %w", err,
 		)
@@ -801,9 +834,12 @@ func (c *Config) SetTrackListColumns(
 		c.TrackList = &tracklist.Config{}
 	}
 
+	previous := c.TrackList.Columns
 	c.TrackList.Columns = columns
 
 	if err := c.TrackList.Validate(); err != nil {
+		c.TrackList.Columns = previous
+
 		return fmt.Errorf(
 			"invalid track-list columns: %w", err,
 		)
@@ -901,9 +937,12 @@ func (c *Config) SetFavoritesIconStyle(
 		c.Favorites.ApplyDefaults()
 	}
 
+	previous := c.Favorites.IconStyle
 	c.Favorites.IconStyle = favorites.IconStyle(style)
 
 	if err := c.Favorites.Validate(); err != nil {
+		c.Favorites.IconStyle = previous
+
 		return fmt.Errorf(
 			"invalid favorites icon style: %w", err,
 		)

backend/config/setter_rollback_test.go

@@ -0,0 +1,262 @@
+package config
+
+import (
+	"log/slog"
+	"path/filepath"
+	"testing"
+
+	"yellowjacket/backend/library"
+	"yellowjacket/backend/tracklist"
+)
+
+// newSavableConfig builds a loaded, valid config in a temp directory,
+// so Save() writes rather than refusing with errSaveBeforeLoad.
+//
+// The library directory is real and set, because Config.Validate only
+// validates the Library section when DirectoryPath is non-empty -- an
+// empty one would hide a poisoned ScanConcurrency from the whole-config
+// save that is the symptom under test.
+func newSavableConfig(t *testing.T) *Config {
+	t.Helper()
+
+	c := &Config{
+		logger:   slog.Default(),
+		filePath: filepath.Join(t.TempDir(), "config.toml"),
+		Library: &library.Config{
+			DirectoryPath: library.Directory(t.TempDir()),
+		},
+	}
+
+	c.applyDefaults()
+
+	if err := c.Load(); err != nil {
+		t.Fatalf("Load() error: %v", err)
+	}
+
+	if err := c.Save(); err != nil {
+		t.Fatalf("Save() on a fresh config error: %v", err)
+	}
+
+	return c
+}
+
+// TestSetterRejectionDoesNotPoisonTheConfig is the whole of #231.
+//
+// Every setter here assigns to the in-memory config and then validates.
+// When the validation rejects the argument, the rejected value has to go
+// back -- not because the caller sees it (it gets an error either way),
+// but because Config.Save() validates the *whole* config.  A value left
+// behind by a failed setter therefore fails every later save, of every
+// unrelated setting, silently and for the rest of the session.
+//
+// So each case asserts three things in order: the setter reports the
+// error, the getter still reports the old value, and an unrelated save
+// still works.  The third is the one the user feels.
+func TestSetterRejectionDoesNotPoisonTheConfig(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name string
+		// reject calls the setter with an argument its own Validate
+		// refuses.
+		reject func(*Config) error
+		// read reports the value the setter writes, so the rollback is
+		// asserted on the config rather than only on the save.
+		read func(*Config) string
+	}{
+		{
+			name: "scan concurrency",
+			reject: func(c *Config) error {
+				return c.SetScanConcurrency("telepathy")
+			},
+			read: (*Config).GetScanConcurrency,
+		},
+		{
+			name: "theme accent colour",
+			reject: func(c *Config) error {
+				return c.SetThemeAccentColor("not-a-hex")
+			},
+			read: (*Config).GetThemeAccentColor,
+		},
+		{
+			name: "theme background shade",
+			reject: func(c *Config) error {
+				return c.SetThemeBackgroundShade("chartreuse")
+			},
+			read: (*Config).GetThemeBackgroundShade,
+		},
+		{
+			name: "default page",
+			reject: func(c *Config) error {
+				return c.SetDefaultPage("nowhere")
+			},
+			read: (*Config).GetDefaultPage,
+		},
+		{
+			name: "queue fallback",
+			reject: func(c *Config) error {
+				return c.SetQueueFallback("improvise")
+			},
+			read: (*Config).GetQueueFallback,
+		},
+		{
+			name: "favorites icon style",
+			reject: func(c *Config) error {
+				return c.SetFavoritesIconStyle("asterisk")
+			},
+			read: (*Config).GetFavoritesIconStyle,
+		},
+		{
+			name: "track-list columns",
+			reject: func(c *Config) error {
+				// titleArtist is a drawing definition, not a
+				// configurable column (#197), so it is exactly what
+				// the frontend used to be able to send.
+				return c.SetTrackListColumns([]tracklist.Column{
+					{ID: "titleArtist"},
+				})
+			},
+			read: func(c *Config) string {
+				return columnIDs(c.GetTrackListColumns())
+			},
+		},
+		{
+			name: "track-list columns, duplicated",
+			reject: func(c *Config) error {
+				// The route #197 closed was one invalid id; a
+				// duplicate is the one still reachable from a client
+				// that assembles the list itself.
+				return c.SetTrackListColumns([]tracklist.Column{
+					{ID: tracklist.ColTrackName},
+					{ID: tracklist.ColTrackName},
+				})
+			},
+			read: func(c *Config) string {
+				return columnIDs(c.GetTrackListColumns())
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			c := newSavableConfig(t)
+			before := tc.read(c)
+
+			if err := tc.reject(c); err == nil {
+				t.Fatal("setter accepted an invalid value, want an error")
+			}
+
+			if after := tc.read(c); after != before {
+				t.Errorf(
+					"value after a rejected write = %q, want the previous %q",
+					after, before,
+				)
+			}
+
+			// The symptom: an unrelated setting can no longer be saved.
+			if err := c.SetPopupVolume(true); err != nil {
+				t.Errorf("an unrelated setter failed after a rejected write: %v", err)
+			}
+
+			if err := c.Save(); err != nil {
+				t.Errorf("Save() failed after a rejected write: %v", err)
+			}
+		})
+	}
+}
+
+// TestRejectedSetterLeavesNothingOnDisk pairs with the sweep above: the
+// rollback must not be undone by what the file already holds, so a
+// config reloaded from disk after a rejected write agrees with memory.
+func TestRejectedSetterLeavesNothingOnDisk(t *testing.T) {
+	t.Parallel()
+
+	c := newSavableConfig(t)
+
+	if err := c.SetThemeAccentColor("#123456"); err != nil {
+		t.Fatalf("SetThemeAccentColor() error: %v", err)
+	}
+
+	if err := c.SetThemeAccentColor("not-a-hex"); err == nil {
+		t.Fatal("SetThemeAccentColor accepted a non-colour, want an error")
+	}
+
+	reloaded := &Config{logger: slog.Default(), filePath: c.filePath}
+	reloaded.applyDefaults()
+
+	if err := reloaded.Load(); err != nil {
+		t.Fatalf("Load() error: %v", err)
+	}
+
+	if got := reloaded.GetThemeAccentColor(); got != "#123456" {
+		t.Errorf("accent colour on disk = %q, want %q", got, "#123456")
+	}
+
+	if c.GetThemeAccentColor() != reloaded.GetThemeAccentColor() {
+		t.Errorf(
+			"in-memory accent %q disagrees with disk %q after a rejected write",
+			c.GetThemeAccentColor(), reloaded.GetThemeAccentColor(),
+		)
+	}
+}
+
+// TestSetLibraryDirectoryValidatesBeforeAssigning pins the precedent the
+// seven rolled-back setters follow: this one has always built and
+// validated a candidate before assigning, so a bad path never reaches
+// the config at all.
+func TestSetLibraryDirectoryValidatesBeforeAssigning(t *testing.T) {
+	t.Parallel()
+
+	c := newSavableConfig(t)
+	before := c.GetLibraryDirectory()
+
+	if err := c.SetLibraryDirectory(filepath.Join(t.TempDir(), "no-such-dir")); err == nil {
+		t.Fatal("SetLibraryDirectory accepted a missing directory, want an error")
+	}
+
+	if after := c.GetLibraryDirectory(); after != before {
+		t.Errorf("library directory = %q, want the previous %q", after, before)
+	}
+
+	if err := c.Save(); err != nil {
+		t.Errorf("Save() failed after a rejected library directory: %v", err)
+	}
+}
+
+// TestSetViewVisibleRefusesBeforeAssigning covers the other setter left
+// out of the rollback pass: it guards its own argument up front, so
+// GeneralConfig.Validate never sees a view it would reject.
+func TestSetViewVisibleRefusesBeforeAssigning(t *testing.T) {
+	t.Parallel()
+
+	c := newSavableConfig(t)
+
+	if err := c.SetViewVisible("no-such-view", false); err == nil {
+		t.Fatal("SetViewVisible accepted an unknown view, want an error")
+	}
+
+	if err := c.SetViewVisible(c.GetDefaultPage(), false); err == nil {
+		t.Fatal("SetViewVisible hid the launch page, want an error")
+	}
+
+	if err := c.Save(); err != nil {
+		t.Errorf("Save() failed after a refused view visibility change: %v", err)
+	}
+}
+
+// columnIDs renders a column list for comparison in the table above.
+func columnIDs(cols []tracklist.Column) string {
+	ids := make([]byte, 0, len(cols)*8)
+
+	for i, col := range cols {
+		if i > 0 {
+			ids = append(ids, ',')
+		}
+
+		ids = append(ids, col.ID...)
+	}
+
+	return string(ids)
+}

backend/database/database_test.go

@@ -662,19 +662,19 @@ func TestSmartPlaylistColumns(t *testing.T) {
 }
 
 // ---------------------------------------------------------------------------
-// Migration 10 — play history tracking
+// Listening events tracking
 // ---------------------------------------------------------------------------
 
-func TestPlayHistoryTable(t *testing.T) {
+func TestListeningEventsTable(t *testing.T) {
 	t.Parallel()
 
 	db := NewTestDB(t)
 
-	// Verify play_history table exists.
+	// Verify listening_events table exists.
 	var tableCount int64
 
 	tblRows, err := db.QueryContext(
-		"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='play_history'",
+		"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='listening_events'",
 	)
 	if err != nil {
 		t.Fatalf("query sqlite_master: %v", err)
@@ -695,12 +695,14 @@ func TestPlayHistoryTable(t *testing.T) {
 	_ = tblRows.Close()
 
 	if tableCount != 1 {
-		t.Errorf("play_history table count = %d, want 1", tableCount)
+		t.Errorf("listening_events table count = %d, want 1", tableCount)
 	}
 
-	// Verify audio_files has play_count and last_played columns.
+	// Verify audio_files has the denormalized listening counters.
 	hasPlayCount := false
 	hasLastPlayed := false
+	hasSkipCount := false
+	hasLastSkipped := false
 
 	colRows, err := db.QueryContext("PRAGMA table_info(audio_files)")
 	if err != nil {
@@ -732,6 +734,14 @@ func TestPlayHistoryTable(t *testing.T) {
 		if name == "last_played" {
 			hasLastPlayed = true
 		}
+
+		if name == "skip_count" {
+			hasSkipCount = true
+		}
+
+		if name == "last_skipped" {
+			hasLastSkipped = true
+		}
 	}
 
 	_ = colRows.Close()
@@ -744,6 +754,14 @@ func TestPlayHistoryTable(t *testing.T) {
 		t.Error("audio_files missing last_played column")
 	}
 
+	if !hasSkipCount {
+		t.Error("audio_files missing skip_count column")
+	}
+
+	if !hasLastSkipped {
+		t.Error("audio_files missing last_skipped column")
+	}
+
 	// Verify track_metadata VIEW includes play_count and last_played.
 	viewCols := map[string]bool{}
 
@@ -783,7 +801,7 @@ func TestPlayHistoryTable(t *testing.T) {
 		t.Error("track_metadata VIEW missing last_played column")
 	}
 
-	// Round-trip: insert a play_history row and verify play_count update.
+	// Round-trip: insert a listening_events row and verify play_count update.
 	// First, set up test data. The test DB already has library id=0.
 	InsertTestTrack(t, db, TestTrack{
 		FilePath:    "/test/play_history.mp3",
@@ -821,12 +839,12 @@ func TestPlayHistoryTable(t *testing.T) {
 		t.Errorf("initial play_count = %d, want 0", playCount)
 	}
 
-	// Insert a play_history row and update play_count.
+	// Insert a listening_events row (kind defaults to 'complete').
 	_, err = db.ExecContext(
-		"INSERT INTO play_history (audio_file_id) VALUES (1)",
+		"INSERT INTO listening_events (audio_file_id, kind) VALUES (1, 'complete')",
 	)
 	if err != nil {
-		t.Fatalf("insert play_history: %v", err)
+		t.Fatalf("insert listening_events: %v", err)
 	}
 
 	_, err = db.ExecContext(

backend/database/playlistphantoms.go

@@ -0,0 +1,125 @@
+package database
+
+import (
+	"context"
+	"database/sql"
+	"fmt"
+	"log/slog"
+)
+
+// Preserving a playlist entry across the loss of its track is two
+// statements, not one, and the split is not tidiness -- it is what
+// makes the important half work in the situation that needs it most.
+//
+// `playlist_tracks.audio_file_id` is ON DELETE SET NULL, so an entry
+// outlives its file as an id-less row that says nothing about what the
+// user put in the playlist.  The phantom_* columns carry the answer
+// across and ResolvePhantomTracksAfterScan re-links them afterwards --
+// but only if something fills them *before* the rows go.
+//
+// The two halves are not equally important and are not equally
+// available:
+//
+//   - **phantom_file_path is the one that matters.**
+//     ResolvePhantomTracksAfterScan matches it against
+//     `audio_files.file_path`, so without it an entry can never be
+//     re-linked and the playlist is empty for good.  It comes straight
+//     off `audio_files`, whose `file_path` is the table's natural key
+//     and has been present in every shape it has ever had -- including
+//     the pre-013 stub of `(id, file_path, recording_id)`.
+//   - The rest is *display* for a phantom entry before a rescan
+//     re-links it, and it comes from the `track_metadata` view, which
+//     is the one definition of a track row and not worth restating.
+//
+// Reading the view is what cannot be relied on here, and that is the
+// whole reason for the split.  This runs *before* applySchema, which is
+// precisely the moment the schema is inconsistent: the view is whatever
+// the last launch's schema declared, while `audio_files` is whatever
+// the launch before that left behind.  A view over columns the table no
+// longer has is not merely empty -- `pragma_table_info` on it *errors*,
+// and so does selecting from it.  `cmd/indexbuild`'s fixture is exactly
+// that shape and is what caught this.
+//
+// COALESCE keeps an existing phantom value in both halves: an entry
+// already phantom is one whose file went missing in an earlier pass,
+// and its recorded metadata is the only copy left.  Overwriting that
+// from a NULL join erases the rows this exists to protect.
+const (
+	preservePhantomPathSQL = `
+		UPDATE playlist_tracks
+		SET phantom_file_path = COALESCE(phantom_file_path, (
+			SELECT af.file_path FROM audio_files af
+			WHERE af.id = playlist_tracks.audio_file_id
+		))
+		WHERE audio_file_id IS NOT NULL
+	`
+
+	preservePhantomDisplaySQL = `
+		UPDATE playlist_tracks
+		SET
+			phantom_title = COALESCE(phantom_title, (
+				SELECT tm.title FROM track_metadata tm
+				WHERE tm.id = playlist_tracks.audio_file_id
+			)),
+			phantom_artist = COALESCE(phantom_artist, (
+				SELECT tm.artist_name FROM track_metadata tm
+				WHERE tm.id = playlist_tracks.audio_file_id
+			)),
+			phantom_album = COALESCE(phantom_album, (
+				SELECT tm.album FROM track_metadata tm
+				WHERE tm.id = playlist_tracks.audio_file_id
+			)),
+			phantom_duration_ms = COALESCE(phantom_duration_ms, (
+				SELECT af.length_milliseconds FROM audio_files af
+				WHERE af.id = playlist_tracks.audio_file_id
+			)),
+			phantom_genre = COALESCE(phantom_genre, (
+				SELECT tm.genre FROM track_metadata tm
+				WHERE tm.id = playlist_tracks.audio_file_id
+			)),
+			phantom_cover_art_path = COALESCE(phantom_cover_art_path, (
+				SELECT tm.cover_art_path FROM track_metadata tm
+				WHERE tm.id = playlist_tracks.audio_file_id
+			))
+		WHERE audio_file_id IS NOT NULL
+	`
+)
+
+// PreservePlaylistPhantoms records every linked playlist entry's track
+// metadata on the entry itself, so the entry survives the rows being
+// deleted underneath it.
+//
+// Every path that empties `audio_files` must call this first, inside
+// the same transaction as the delete.  There are two such paths and
+// they had drifted: the full rescan in backend/library did this and the
+// stale-shape retire in this package did not, so the *documented*
+// repair ("delete and rescan") preserved playlists while the automatic
+// one that exists to spare the user that work silently emptied them.
+//
+// The display half is skipped, with a warning, when `track_metadata`
+// cannot answer -- see the note above.  Skipping it costs a phantom
+// entry its title until a rescan re-links it; skipping the path half
+// would cost the entry outright, so that one is an error.
+func PreservePlaylistPhantoms(
+	ctx context.Context, tx *sql.Tx, logger *slog.Logger,
+) error {
+	if _, err := tx.ExecContext(ctx, preservePhantomPathSQL); err != nil {
+		return fmt.Errorf(
+			"could not preserve playlist track file paths: %w", err,
+		)
+	}
+
+	if _, err := tx.ExecContext(ctx, preservePhantomDisplaySQL); err != nil {
+		// A failed statement does not roll back a SQLite transaction,
+		// so the path half above stands and the entries remain
+		// re-linkable.
+		logger.Warn(
+			"could not record display metadata for playlist entries; "+
+				"they will be re-linked by the next scan but read as "+
+				"unknown until then",
+			"err", err,
+		)
+	}
+
+	return nil
+}

backend/database/sql/schemas/audio_files.sql

@@ -68,8 +68,14 @@ CREATE TABLE IF NOT EXISTS audio_files (
   -- compared against the on-disk mtime during a scan to detect files
   -- another application retagged in place.
   modified_at        INTEGER NOT NULL DEFAULT 0,
+  -- Listening counts, denormalized from listening_events so the hot
+  -- read path (track list sort, shelves, smart playlists) never joins
+  -- a log table.  Authored: a rescan cannot rebuild them.  This is the
+  -- "MIXED KIND" half of audio_files the datamap notes.
   play_count         INTEGER NOT NULL DEFAULT 0,
   last_played        DATETIME,
+  skip_count         INTEGER NOT NULL DEFAULT 0,
+  last_skipped       DATETIME,
   tag_status         TEXT NOT NULL DEFAULT 'untagged'
     CHECK(tag_status IN (
       'untagged', 'auto_matched', 'user_confirmed', 'user_skipped_permanent'

backend/database/sql/schemas/download_items.sql

@@ -45,8 +45,6 @@ CREATE INDEX IF NOT EXISTS idx_download_items_live
 CREATE INDEX IF NOT EXISTS idx_download_items_state
     ON download_items(state);
 
--- idx_download_items_download is deliberately NOT declared here: on an
+-- ListDownloadItemsForDownload filters on the parent download.
--- existing database this table already exists at schema-pass time with
+CREATE INDEX IF NOT EXISTS idx_download_items_download
--- its old column still named request_id, so an inline CREATE INDEX on
+    ON download_items(download_id);
--- download_id would fail outright. See ensureDownloadIndexes in
--- backend/database/download_rename_migration.go.

backend/database/sql/schemas/download_requests.sql

@@ -66,14 +66,11 @@ CREATE TABLE IF NOT EXISTS download_requests (
     FOREIGN KEY(parent_id) REFERENCES download_requests(id) ON DELETE CASCADE
 );
 
--- idx_download_requests_{due,entity,parent} are deliberately NOT
+CREATE INDEX IF NOT EXISTS idx_download_requests_due
--- declared here. This table name is reused from the old one-shot
+    ON download_requests(state, next_try_at);
--- attempt table (also called download_requests before the Want/Request
+
--- rename), so on an existing database this CREATE TABLE is a no-op
+CREATE INDEX IF NOT EXISTS idx_download_requests_entity
--- against a table that, at schema-pass time, is still shaped like the
+    ON download_requests(entity, state);
--- OLD attempts table and lacks these columns entirely — an inline
+
--- CREATE INDEX here would fail outright rather than just no-op. See
+CREATE INDEX IF NOT EXISTS idx_download_requests_parent
--- migrateDownloadRename/ensureDownloadIndexes in
+    ON download_requests(parent_id) WHERE parent_id IS NOT NULL;
--- backend/database/download_rename_migration.go, which create these
--- once the rename has actually happened (or immediately, on a fresh
--- database where the columns exist from the start).

backend/database/sql/schemas/listening_events.sql

@@ -0,0 +1,42 @@
+-- One row per track *exit*, three ways a listen can end: it reached
+-- the end, it was heard enough to count and then skipped past, or it
+-- was abandoned for another track before anyone had really listened.
+--
+-- This is the source of truth for listening behaviour.  The
+-- denormalized `play_count` / `last_played` / `skip_count` /
+-- `last_skipped` on audio_files are materialized from it, because the
+-- hot read path (track-list sort, the shelves, smart playlists) must
+-- not join a log that grows by one row per song forever.
+--
+-- `kind` is the classification, applied at write time:
+--
+--   complete   the track reached its natural end, or was skipped in
+--              its tail window (the last few seconds of a long fade).
+--   play       the scrobble threshold was heard — half the track or
+--              four minutes, whichever is less — and the user moved on
+--              before the end.
+--   skip       the user moved to a different track before that.
+--
+-- `position_seconds` / `duration_seconds` are the raw reading the
+-- classification was made from, kept so a future re-tune of the
+-- threshold does not need the events re-recorded.  0/0 on a row means
+-- "not captured for this event" (e.g. a natural finish recorded before
+-- these columns existed), not "a zero-second track".
+CREATE TABLE IF NOT EXISTS listening_events (
+    id               INTEGER PRIMARY KEY,
+    audio_file_id    INTEGER NOT NULL,
+    kind             TEXT NOT NULL DEFAULT 'complete'
+                     CHECK (kind IN ('complete', 'play', 'skip')),
+    position_seconds INTEGER NOT NULL DEFAULT 0,
+    duration_seconds INTEGER NOT NULL DEFAULT 0,
+    occurred_at      DATETIME NOT NULL DEFAULT (datetime('now')),
+    FOREIGN KEY(audio_file_id) REFERENCES audio_files(id) ON DELETE CASCADE
+);
+
+CREATE INDEX IF NOT EXISTS idx_listening_events_audio_file_id
+    ON listening_events(audio_file_id);
+
+-- "What did I listen to this month" walks this, rather than the
+-- per-track index above.
+CREATE INDEX IF NOT EXISTS idx_listening_events_occurred_at
+    ON listening_events(occurred_at);

backend/database/sql/schemas/play_history.sql

@@ -1,9 +0,0 @@
-CREATE TABLE IF NOT EXISTS play_history (
-    id INTEGER PRIMARY KEY,
-    audio_file_id INTEGER NOT NULL,
-    played_at DATETIME NOT NULL DEFAULT (datetime('now')),
-    FOREIGN KEY(audio_file_id) REFERENCES audio_files(id) ON DELETE CASCADE
-);
-
-CREATE INDEX IF NOT EXISTS idx_play_history_audio_file_id
-    ON play_history(audio_file_id);

backend/database/sql/schemas/queue.sql

@@ -1,18 +1,15 @@
 CREATE TABLE IF NOT EXISTS queue (
     id INTEGER PRIMARY KEY CHECK(id = 1),
-    source_playlist_id INTEGER,
     current_position INTEGER NOT NULL DEFAULT 0,
     shuffle_mode BOOLEAN NOT NULL DEFAULT false,
     repeat_mode TEXT NOT NULL DEFAULT 'off',
     shuffle_order TEXT,
-    -- source_playlist_id above is unused dead weight (nothing has ever
+    -- What the queue was built from ("Playing from: X"): an album,
-    -- written it a nonzero value); source_type/source_id/source_label
+    -- playlist, smart playlist, genre or artist, identified by the id
-    -- below are its generalized replacement, covering albums, playlists,
+    -- that source_type's namespace gives it.
-    -- smart playlists, genres and artists rather than playlists alone.
     source_type TEXT NOT NULL DEFAULT '',
     source_id INTEGER NOT NULL DEFAULT 0,
-    source_label TEXT NOT NULL DEFAULT '',
+    source_label TEXT NOT NULL DEFAULT ''
-    FOREIGN KEY(source_playlist_id) REFERENCES playlists(id) ON DELETE SET NULL
 );
 
 -- Singleton row: there is exactly one playback queue.

backend/database/sql/schemas/tagging_items.sql

@@ -26,17 +26,10 @@ CREATE TABLE IF NOT EXISTS tagging_items (
   -- complete rip of their own directory.  parent_group_key is the
   -- original folder group they were split from.
   --
-  -- These two columns are declared LAST, after created_at, even
+  -- These columns are appended after created_at rather than grouped
-  -- though that reads oddly next to the rest of the table: sql/
+  -- with the rest of the row: sqlc's `SELECT *` scans (GetTaggingItem)
-  -- migrations/0001 brings a pre-existing tagging_items up to date
+  -- bind column order positionally, so new columns always go at the
-  -- with `ALTER TABLE ADD COLUMN`, which SQLite always appends at
+  -- end.
-  -- the end of the column list.  A fresh install (this file) and an
-  -- upgraded database (this file + the migration) must end up with
-  -- IDENTICAL column order, because sqlc-generated `SELECT *` scans
-  -- (e.g. GetTaggingItem) bind columns positionally — see the
-  -- schema/migration column-order test in database_test.go.  Put
-  -- new columns wherever reads best when adding a table for the
-  -- first time; append-only from the second migration on.
   synthetic               INTEGER NOT NULL DEFAULT 0,
   parent_group_key        TEXT NOT NULL DEFAULT '',
   -- album_artist_conflict latches to 1 the first time two tracks
@@ -58,10 +51,3 @@ CREATE INDEX IF NOT EXISTS idx_tagging_items_library_status
 
 CREATE INDEX IF NOT EXISTS idx_tagging_items_status_pending
     ON tagging_items(library_id) WHERE status = 'pending';
-
--- idx_tagging_items_parent_group_key is NOT declared here on
--- purpose: this file runs unconditionally, before migrations, even
--- against a database that hasn't run 0001 yet — an index predicate
--- referencing parent_group_key would fail on that table.  It lives
--- solely in sql/migrations/0001_tagging_items_synthetic.sql, which
--- runs after the column exists either way (see database.go).

backend/database/sql/sqlcgen/audio_files.sql.go

@@ -39,7 +39,7 @@ INSERT INTO audio_files (
   ?, ?, ?, ?, ?, ?,
   ?, ?, ?, ?, ?
 )
-RETURNING id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, tag_status
+RETURNING id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, skip_count, last_skipped, tag_status
 `
 
 type CreateAudioFileParams struct {
@@ -135,6 +135,8 @@ func (q *Queries) CreateAudioFile(ctx context.Context, arg CreateAudioFileParams
 		&i.ModifiedAt,
 		&i.PlayCount,
 		&i.LastPlayed,
+		&i.SkipCount,
+		&i.LastSkipped,
 		&i.TagStatus,
 	)
 	return i, err
@@ -192,7 +194,7 @@ func (q *Queries) GetAllAudioFilePaths(ctx context.Context) ([]GetAllAudioFilePa
 
 const getAudioFile = `-- name: GetAudioFile :one
 
-SELECT id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, tag_status FROM audio_files WHERE id = ? LIMIT 1
+SELECT id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, skip_count, last_skipped, tag_status FROM audio_files WHERE id = ? LIMIT 1
 `
 
 // ---------------------------------------------------------------------
@@ -228,13 +230,15 @@ func (q *Queries) GetAudioFile(ctx context.Context, id int64) (AudioFile, error)
 		&i.ModifiedAt,
 		&i.PlayCount,
 		&i.LastPlayed,
+		&i.SkipCount,
+		&i.LastSkipped,
 		&i.TagStatus,
 	)
 	return i, err
 }
 
 const getAudioFileByPath = `-- name: GetAudioFileByPath :one
-SELECT id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, tag_status FROM audio_files WHERE file_path = ? LIMIT 1
+SELECT id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, skip_count, last_skipped, tag_status FROM audio_files WHERE file_path = ? LIMIT 1
 `
 
 func (q *Queries) GetAudioFileByPath(ctx context.Context, filePath string) (AudioFile, error) {
@@ -267,6 +271,8 @@ func (q *Queries) GetAudioFileByPath(ctx context.Context, filePath string) (Audi
 		&i.ModifiedAt,
 		&i.PlayCount,
 		&i.LastPlayed,
+		&i.SkipCount,
+		&i.LastSkipped,
 		&i.TagStatus,
 	)
 	return i, err
@@ -334,7 +340,7 @@ func (q *Queries) GetAudioFilesByPaths(ctx context.Context, paths []string) ([]G
 }
 
 const getAudioFilesInLibrary = `-- name: GetAudioFilesInLibrary :many
-SELECT id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, tag_status FROM audio_files WHERE library_id = ?
+SELECT id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, skip_count, last_skipped, tag_status FROM audio_files WHERE library_id = ?
 `
 
 func (q *Queries) GetAudioFilesInLibrary(ctx context.Context, libraryID int64) ([]AudioFile, error) {
@@ -373,6 +379,8 @@ func (q *Queries) GetAudioFilesInLibrary(ctx context.Context, libraryID int64) (
 			&i.ModifiedAt,
 			&i.PlayCount,
 			&i.LastPlayed,
+			&i.SkipCount,
+			&i.LastSkipped,
 			&i.TagStatus,
 		); err != nil {
 			return nil, err

backend/database/sql/sqlcgen/models.go

@@ -94,6 +94,8 @@ type AudioFile struct {
 	ModifiedAt         int64
 	PlayCount          int64
 	LastPlayed         sql.NullTime
+	SkipCount          int64
+	LastSkipped        sql.NullTime
 	TagStatus          string
 }
 
@@ -261,6 +263,15 @@ type Library struct {
 	AutotagWarningAcked int64
 }
 
+type ListeningEvent struct {
+	ID              int64
+	AudioFileID     int64
+	Kind            string
+	PositionSeconds int64
+	DurationSeconds int64
+	OccurredAt      time.Time
+}
+
 type Lyric struct {
 	AudioFileID   int64
 	Text          string
@@ -273,12 +284,6 @@ type LyricsIndex struct {
 	Lyrics string
 }
 
-type PlayHistory struct {
-	ID          int64
-	AudioFileID int64
-	PlayedAt    time.Time
-}
-
 type PlayerState struct {
 	ID                  int64
 	Volume              int64
@@ -313,7 +318,6 @@ type PlaylistTrack struct {
 
 type Queue struct {
 	ID              int64
-	SourcePlaylistID sql.NullInt64
 	CurrentPosition int64
 	ShuffleMode     bool
 	RepeatMode      string

backend/database/staleshape.go

@@ -245,6 +245,13 @@ func dropDeferred(
 	ctx context.Context, db *sql.DB, logger *slog.Logger,
 	drop map[string]string,
 ) error {
+	// Asked before the transaction opens, because the answer is about
+	// which tables are live and that cannot change underneath us here.
+	preserve, err := shouldPreservePhantoms(ctx, db, drop)
+	if err != nil {
+		return err
+	}
+
 	tx, err := db.BeginTx(ctx, nil)
 	if err != nil {
 		return fmt.Errorf("could not begin the retire transaction: %w", err)
@@ -256,6 +263,22 @@ func dropDeferred(
 		return fmt.Errorf("could not defer foreign keys: %w", err)
 	}
 
+	// Before any drop, so every entry still has a track to read.  It is
+	// in this transaction rather than beside it because the preservation
+	// and the delete have to succeed or fail together: a commit that
+	// dropped the files without the phantoms is the bug, and a commit
+	// that wrote phantoms without dropping anything is a lie about rows
+	// that are still there.
+	if preserve {
+		logger.Info(
+			"preserving playlist entries across the retire of audio_files",
+		)
+
+		if err := PreservePlaylistPhantoms(ctx, tx, logger); err != nil {
+			return err
+		}
+	}
+
 	// Sorted, so a failure is reproducible.  Map order is random, and a
 	// bug that depends on which table happens to go first reproduces on
 	// one run in three and passes review on the other two -- which is
@@ -283,6 +306,38 @@ func dropDeferred(
 	return nil
 }
 
+// shouldPreservePhantoms reports whether this retire is about to take
+// `audio_files` out from under the playlists.
+//
+// The `playlist_tracks` check is not defensive padding.  This runs
+// *before* applySchema, which is the moment the schema is by definition
+// mid-repair, and the preservation reads a table it does not drop.  A
+// database old enough not to have it would otherwise fail here, and
+// failing here means the app does not open at all -- while nothing is
+// lost by skipping, since an absent `playlist_tracks` holds no
+// playlists to save.
+//
+// It deliberately does *not* ask after `track_metadata`.  Whether that
+// view can answer is PreservePlaylistPhantoms's own business, because a
+// view broken against an older `audio_files` is a state this function
+// cannot detect without hitting the same error it is trying to avoid:
+// pragma_table_info on such a view errors rather than reporting no
+// columns.
+func shouldPreservePhantoms(
+	ctx context.Context, db *sql.DB, drop map[string]string,
+) (bool, error) {
+	if _, going := drop["audio_files"]; !going {
+		return false, nil
+	}
+
+	cols, err := liveColumns(ctx, db, "playlist_tracks")
+	if err != nil {
+		return false, err
+	}
+
+	return len(cols) > 0, nil
+}
+
 // staleReason reports why a live table disagrees with its declaration,
 // or "" when it agrees.  A column the live table does not have is the
 // additive case; a column whose declared type changed is the one an

backend/database/staleshape_test.go

@@ -497,3 +497,180 @@ func TestParseCreateTablesReadsTheRealSchema(t *testing.T) {
 		}
 	}
 }
+
+// TestRetiringAudioFilesKeepsPlaylistContents is the symptom this
+// repair exists for: a playlist survived the retire as a row count and
+// nothing else.
+//
+// TestRetiringOwnedTablesDoesNotDangle already asserts the entry does
+// not keep a stale id, which is the *dangerous* half.  It is satisfied
+// just as well by an entry that says nothing at all, which is the
+// half that quietly emptied every playlist -- so this asserts what the
+// entry still knows, and specifically phantom_file_path, because that
+// is the column ResolvePhantomTracksAfterScan matches back against
+// audio_files.file_path.
+//
+// Note the seed drops `comment`, not `artist_credit`: the mutation has
+// to leave `track_metadata` standing, since a real launch reaches the
+// retire with the view the previous launch created.  A test that drops
+// the view first is testing the skip path, not this one.
+func TestRetiringAudioFilesKeepsPlaylistContents(t *testing.T) {
+	ctx := context.Background()
+	db := openRaw(t, t.TempDir())
+
+	if _, err := db.ExecContext(ctx, "PRAGMA foreign_keys = ON"); err != nil {
+		t.Fatalf("pragma: %v", err)
+	}
+
+	if err := applySchema(ctx, db); err != nil {
+		t.Fatalf("applySchema: %v", err)
+	}
+
+	if _, err := db.ExecContext(ctx, `
+		INSERT INTO playlists (id, name) VALUES (1, 'keepme');
+		INSERT INTO libraries (id, name, path) VALUES (0, 'test', '/music');
+		INSERT INTO artists (id, name) VALUES (3, 'Aurora Fields');
+		INSERT INTO cover_art (id, file_path, mime_type)
+			VALUES (9, 'covers/7.jpg', 'image/jpeg');
+		INSERT INTO genres (id, name) VALUES (5, 'Ambient');
+		INSERT INTO albums (id, name, artist_id, cover_art_id)
+			VALUES (4, 'Tideline', 3, 9);
+		INSERT INTO audio_files
+			(id, file_path, file_type_id, length_milliseconds,
+			 title, artist_credit, artist_id, album_id)
+			VALUES (7, '/music/a.flac', 1, 1000,
+			        'Slack Water', 'Aurora Fields', 3, 4);
+		INSERT INTO file_genres (audio_file_id, genre_id) VALUES (7, 5);
+		INSERT INTO playlist_tracks (playlist_id, audio_file_id, position)
+			VALUES (1, 7, 0);
+		ALTER TABLE audio_files DROP COLUMN comment;
+	`); err != nil {
+		t.Fatalf("seed: %v", err)
+	}
+
+	if err := retireStaleTables(ctx, db, testLogger()); err != nil {
+		t.Fatalf("retire: %v", err)
+	}
+
+	if err := applySchema(ctx, db); err != nil {
+		t.Fatalf("applySchema: %v", err)
+	}
+
+	var (
+		path, title, artist, album, genre, cover sql.NullString
+		duration                                 sql.NullInt64
+	)
+
+	if err := db.QueryRowContext(ctx, `
+		SELECT phantom_file_path, phantom_title, phantom_artist,
+		       phantom_album, phantom_duration_ms, phantom_genre,
+		       phantom_cover_art_path
+		FROM playlist_tracks WHERE playlist_id = 1
+	`).Scan(&path, &title, &artist, &album, &duration, &genre, &cover); err != nil {
+		t.Fatalf("read the surviving entry: %v", err)
+	}
+
+	// The one that matters: without it the entry can never be re-linked
+	// by the rescan the retire itself provokes.
+	if path.String != "/music/a.flac" {
+		t.Fatalf(
+			"phantom_file_path is %q, want %q -- the playlist entry "+
+				"cannot be re-linked and the playlist is empty for good",
+			path.String, "/music/a.flac",
+		)
+	}
+
+	if title.String != "Slack Water" {
+		t.Errorf("phantom_title is %q, want %q", title.String, "Slack Water")
+	}
+
+	if artist.String != "Aurora Fields" {
+		t.Errorf("phantom_artist is %q, want %q", artist.String, "Aurora Fields")
+	}
+
+	if album.String != "Tideline" {
+		t.Errorf("phantom_album is %q, want %q", album.String, "Tideline")
+	}
+
+	if duration.Int64 != 1000 {
+		t.Errorf("phantom_duration_ms is %d, want 1000", duration.Int64)
+	}
+
+	if genre.String != "Ambient" {
+		t.Errorf("phantom_genre is %q, want %q", genre.String, "Ambient")
+	}
+
+	if cover.String != "covers/7.jpg" {
+		t.Errorf("phantom_cover_art_path is %q, want %q", cover.String, "covers/7.jpg")
+	}
+}
+
+// TestRetiringAudioFilesKeepsPathsWhenTheViewCannotAnswer is the case
+// that broke cmd/indexbuild: this repair runs *before* applySchema, so
+// `track_metadata` is whatever the last launch declared while
+// `audio_files` is whatever the launch before that left behind, and a
+// view over columns the table no longer has does not read as empty --
+// it errors.
+//
+// The pre-013 stub shape below is the real one that fixture carries.
+// What must survive is phantom_file_path, because `file_path` is the
+// table's natural key and has been in every shape it ever had; the
+// display columns are allowed to be absent, and the open must not fail.
+func TestRetiringAudioFilesKeepsPathsWhenTheViewCannotAnswer(t *testing.T) {
+	ctx := context.Background()
+	db := openRaw(t, t.TempDir())
+
+	if _, err := db.ExecContext(ctx, "PRAGMA foreign_keys = ON"); err != nil {
+		t.Fatalf("pragma: %v", err)
+	}
+
+	if err := applySchema(ctx, db); err != nil {
+		t.Fatalf("applySchema: %v", err)
+	}
+
+	// The rows go in *after* the reshape: dropping audio_files with
+	// foreign keys on would fire the ON DELETE SET NULL and null the
+	// entry this test is about, which would pass for the wrong reason.
+	if _, err := db.ExecContext(ctx, `
+		DROP TABLE audio_files;
+		CREATE TABLE audio_files (
+			id           INTEGER PRIMARY KEY,
+			file_path    TEXT NOT NULL UNIQUE,
+			recording_id INTEGER
+		);
+		INSERT INTO playlists (id, name) VALUES (1, 'keepme');
+		INSERT INTO audio_files (id, file_path) VALUES (7, '/music/a.flac');
+		INSERT INTO playlist_tracks (playlist_id, audio_file_id, position)
+			VALUES (1, 7, 0);
+	`); err != nil {
+		t.Fatalf("seed: %v", err)
+	}
+
+	// The symptom this guards: the repair must not turn a recoverable
+	// database into one the app refuses to open.
+	if err := retireStaleTables(ctx, db, testLogger()); err != nil {
+		t.Fatalf(
+			"the retire failed on a view it could not read, so the app "+
+				"would not open at all: %v", err,
+		)
+	}
+
+	if err := applySchema(ctx, db); err != nil {
+		t.Fatalf("applySchema: %v", err)
+	}
+
+	var path sql.NullString
+	if err := db.QueryRowContext(ctx,
+		"SELECT phantom_file_path FROM playlist_tracks WHERE playlist_id = 1",
+	).Scan(&path); err != nil {
+		t.Fatalf("read the surviving entry: %v", err)
+	}
+
+	if path.String != "/music/a.flac" {
+		t.Fatalf(
+			"phantom_file_path is %q, want %q -- the display half being "+
+				"unavailable must not cost the entry its one re-link key",
+			path.String, "/music/a.flac",
+		)
+	}
+}

backend/datamap/datamap.go

@@ -269,10 +269,11 @@ var tables = []Table{
 			"from owned files plus the LRCLIB backfill.",
 	},
 	{
-		Name: "play_history", Kind: Authored, Lifetime: Cascade,
+		Name: "listening_events", Kind: Authored, Lifetime: Cascade,
-		Note: "Listening history. Authored, but intentionally cascades " +
+		Note: "Listening history, one row per track exit (complete, play " +
-			"with its track — history for a file no longer in the library " +
+			"or skip). Authored, but intentionally cascades with its " +
-			"has nothing to point at.",
+			"track — history for a file no longer in the library has " +
+			"nothing to point at.",
 	},
 	{
 		Name: "player_state", Kind: Authored, Lifetime: Retained,

backend/datamap/datamap_test.go

@@ -212,13 +212,13 @@ func TestLifetimesMatchSchema(t *testing.T) {
 
 // Authored data is unrecoverable, so it must never be removed as a side
 // effect of deleting owned data.  Cascade is allowed only where the
-// catalog explains why (play_history, queue_tracks); this test pins the
+// catalog explains why (listening_events, queue_tracks); this test pins the
 // set so a new cascade onto authored data is a deliberate decision.
 func TestAuthoredCascadesAreDeliberate(t *testing.T) {
 	t.Parallel()
 
 	allowed := map[string]bool{
-		"play_history": true,
+		"listening_events": true,
 		"queue_tracks":     true,
 
 		// Download history is scoped to the library it imported into.

backend/download/provider_ytdlp_test.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
+	"syscall"
 	"testing"
 )
 
@@ -17,6 +18,21 @@ import (
 
 // stubYtDlp writes an executable script that echoes the given stdout
 // and returns it as a provider config binary path.
+//
+// The write is held under syscall.ForkLock, and that is not tidiness:
+// the kernel refuses to exec a file that is open for writing anywhere
+// in the process, and these tests are parallel, so a *sibling* test's
+// fork can duplicate this descriptor in the moment it is open and
+// carry it past our close — the exec a moment later then fails with
+// ETXTBSY, "text file busy".  That is #146, seen once in CI and once
+// locally, on trees containing no Go at all.  Closing sooner is not
+// available (os.WriteFile has already closed the file before anything
+// execs it) and O_CLOEXEC does not help, because the window is between
+// another goroutine's fork and its own exec.  ForkLock is the lock
+// syscall.forkExec takes across that fork, so holding it here means no
+// child can exist while the descriptor does.  Measured on this helper
+// under 12 concurrent writers: 176-189 of 2400 execs refused without
+// it, 0 of 2400 with it.
 func stubYtDlp(t *testing.T, script string) string {
 	t.Helper()
 
@@ -26,9 +42,11 @@ func stubYtDlp(t *testing.T, script string) string {
 
 	path := filepath.Join(t.TempDir(), "yt-dlp")
 
-	if err := os.WriteFile(
+	syscall.ForkLock.Lock()
-		path, []byte("#!/bin/sh\n"+script), 0o700,
+	err := os.WriteFile(path, []byte("#!/bin/sh\n"+script), 0o700)
-	); err != nil {
+	syscall.ForkLock.Unlock()
+
+	if err != nil {
 		t.Fatalf("write stub: %v", err)
 	}
 

backend/library/rescan.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"yellowjacket/backend/coverart"
+	"yellowjacket/backend/database"
 )
 
 var errNoLibrariesConfigured = errors.New(
@@ -137,34 +138,12 @@ func (l *Library) clearLibraryTables() error {
 	// metadata for all linked tracks before audio_files are deleted.
 	// ON DELETE SET NULL will null out audio_file_id, converting them
 	// to phantoms that ResolvePhantomTracksAfterScan can re-link.
-	if _, err := tx.ExecContext(l.ctx, `
+	//
-		UPDATE playlist_tracks
+	// Shared with the stale-shape retire in backend/database, which is
-		SET
+	// the other path that empties this table and which did not do this
-			phantom_title = COALESCE(phantom_title, (
+	// (#183): the statement lives there so the two cannot drift again.
-				SELECT tm.title FROM track_metadata tm
+	if err := database.PreservePlaylistPhantoms(l.ctx, tx, l.logger); err != nil {
-				WHERE tm.id = playlist_tracks.audio_file_id
+		return err
-			)),
-			phantom_artist = COALESCE(phantom_artist, (
-				SELECT tm.artist_name FROM track_metadata tm
-				WHERE tm.id = playlist_tracks.audio_file_id
-			)),
-			phantom_album = COALESCE(phantom_album, (
-				SELECT tm.album FROM track_metadata tm
-				WHERE tm.id = playlist_tracks.audio_file_id
-			)),
-			phantom_duration_ms = COALESCE(phantom_duration_ms, (
-				SELECT af.length_milliseconds FROM audio_files af
-				WHERE af.id = playlist_tracks.audio_file_id
-			)),
-			phantom_file_path = COALESCE(phantom_file_path, (
-				SELECT af.file_path FROM audio_files af
-				WHERE af.id = playlist_tracks.audio_file_id
-			))
-		WHERE audio_file_id IS NOT NULL
-	`); err != nil {
-		return fmt.Errorf(
-			"could not preserve playlist track metadata: %w", err,
-		)
 	}
 
 	// Phase 2: the files.  file_genres cascades with them.

backend/queue/playhistory.go

@@ -6,7 +6,7 @@ import (
 	"yellowjacket/backend/events"
 )
 
-// recordPlay inserts a play_history row and updates the denormalized
+// recordPlay inserts a listening_events row and updates the denormalized
 // play_count / last_played columns on audio_files. Called from
 // OnPlaybackFinished for the track that just finished.
 //
@@ -20,10 +20,12 @@ func (q *Queue) recordPlay(audioFileID int64) {
 
 	now := time.Now().UTC().Format(time.DateTime)
 
-	// Insert play_history row.
+	// Insert the listening event.  A natural finish is a 'complete' by
+	// construction; position/duration are the classifier's to fill once
+	// skips are recorded (see .planning/plans/active/021).
 	_, err := q.db.ExecContext(
-		`INSERT INTO play_history (audio_file_id, played_at)
+		`INSERT INTO listening_events (audio_file_id, kind, occurred_at)
-		 VALUES (?, ?)`,
+		 VALUES (?, 'complete', ?)`,
 		audioFileID, now,
 	)
 	if err != nil {

e2e/specs/header-action-overflow.spec.ts

@@ -42,10 +42,10 @@ const ACTIONS = ['Import', 'New Playlist', 'New Smart Playlist'];
  * because the number this issue is about (a button 48px wider than the
  * box holding it) is not in the accessibility tree at all.
  */
-const headerFit = (page: import('@playwright/test').Page) =>
+const headerFit = (page: import('@playwright/test').Page, view = 'playlist-view') =>
-  page.evaluate(() => {
+  page.evaluate((tag) => {
     const root = document
-      .querySelector('[data-testid="main-content"] playlist-view')
+      .querySelector(`[data-testid="main-content"] ${tag}`)
       ?.shadowRoot?.querySelector('page-header')?.shadowRoot;
 
     if (!root) return null;
@@ -76,7 +76,7 @@ const headerFit = (page: import('@playwright/test').Page) =>
         ...root.querySelectorAll('#page-header-overflow wa-dropdown-item'),
       ].map((i) => i.textContent?.trim() ?? ''),
     };
-  });
+  }, view);
 
 test.describe('the page header never clips an action', () => {
   test.beforeEach(async ({ app }) => {
@@ -316,3 +316,58 @@ test.describe('the page header never clips an action', () => {
     await expect.poll(async () => (await headerFit(app))?.menu).toEqual([]);
   });
 });
+
+/**
+ * The Tracks header carries the play-all/shuffle-all pair (#31), so
+ * the promise above has to hold for it too — the same per-button
+ * measurement, one view over. Its two actions are the whole of the
+ * header's declared set, and the pair is what plays the list the row
+ * is in, so a button rendered 20px of its 90px is a queue of nothing.
+ */
+const TRACK_ACTIONS = ['Play all', 'Shuffle all'];
+
+test.describe('the Tracks header never clips an action', () => {
+  test.beforeEach(async ({ app }) => {
+    await app.getByTestId('nav-tracks').click();
+    await expect(app.getByTestId('main-content')).toHaveAttribute(
+      'data-active-view',
+      'tracks',
+    );
+  });
+
+  test.afterEach(async ({ app }) => {
+    await app.setViewportSize({ width: 1280, height: 800 });
+  });
+
+  for (const vp of VIEWPORTS) {
+    test(`every action is reachable at ${vp.name}`, async ({ app }) => {
+      await app.setViewportSize({ width: vp.width, height: vp.height });
+
+      await expect
+        .poll(async () => (await headerFit(app, 'track-list'))?.clipped)
+        .toEqual([]);
+
+      const fit = (await headerFit(app, 'track-list'))!;
+
+      expect(fit.overflow).toBeLessThanOrEqual(0);
+
+      // Between them, buttons and menu account for both actions —
+      // not "it fits" but "nothing was dropped to make it fit".
+      expect([...fit.buttons, ...fit.menu].sort()).toEqual(
+        [...TRACK_ACTIONS].sort(),
+      );
+    });
+  }
+
+  /**
+   * The pair's names, through the accessibility tree — a shadow query
+   * measures, but it cannot say what a screen reader is offered.
+   */
+  test('both actions are named controls', async ({ app }) => {
+    for (const label of TRACK_ACTIONS) {
+      await expect(
+        app.getByRole('button', { name: label, exact: true }),
+      ).toBeVisible();
+    }
+  });
+});

e2e/specs/play-all.spec.ts

@@ -0,0 +1,220 @@
+import { test, expect, callBinding, resetEvents, waitForEvent } from '../support/fixtures.js';
+
+type Page = import('@playwright/test').Page;
+
+/**
+ * Play-all/Shuffle-all, asserted on what the backend queued rather than
+ * on playback pixels.
+ *
+ * `SetQueue` reports the queue through `QueueChanged`, and `GetState`
+ * says exactly what it holds: the tracks in order, whether shuffle is
+ * on, and the `Source` the "Playing from" link is built from.  That is
+ * the honest contract here — the buttons are only as good as the queue
+ * they build, and the queue is only as good as the source it names.
+ */
+
+interface QueueState {
+  tracks: { filePath: string; title: string }[];
+  currentIndex: number;
+  shuffleMode: boolean;
+  source: { type: string; id: number; label: string };
+}
+
+const TRACKS_SOURCE = { type: 'tracks', id: 0, label: 'All Tracks' };
+
+const getQueue = (app: Page) =>
+  callBinding<QueueState>(app, 'queue.Queue.GetState');
+
+/** The track paths a rendered track list shows, in row order. */
+function displayedPaths(app: Page, scope: string): Promise<string[]> {
+  return app
+    .locator(`${scope} [data-testid="track-row"]`)
+    .evaluateAll((els) =>
+      els.map((el) => el.getAttribute('data-file-path') ?? ''),
+    );
+}
+
+/** Leave shuffle in a known state. The mode persists across specs in
+ *  one backend process, so a test that asserts on it has to set it. */
+async function setShuffleMode(app: Page, on: boolean): Promise<void> {
+  const state = await getQueue(app);
+
+  if (state.shuffleMode !== on) {
+    await resetEvents(app);
+    await callBinding(app, 'queue.Queue.ToggleShuffle');
+    await waitForEvent(app, 'QueueModeChanged');
+  }
+}
+
+test.describe('play-all/shuffle-all on the track list', () => {
+  test.beforeEach(async ({ app }) => {
+    await callBinding(app, 'queue.Queue.Clear').catch(() => {
+      /* the queue is clearable on every build these specs run against */
+    });
+    await setShuffleMode(app, false);
+  });
+
+  test('Tracks Play all queues the displayed list with an honest source', async ({
+    app,
+  }) => {
+    await app.getByTestId('nav-tracks').click();
+    await expect(app.getByTestId('main-content')).toHaveAttribute(
+      'data-active-view',
+      'tracks',
+    );
+    await expect(
+      app.locator('track-list [data-testid="track-row"]').first(),
+    ).toBeVisible();
+
+    const paths = await displayedPaths(app, 'track-list');
+
+    await resetEvents(app);
+    await app.getByTestId('page-action-play-all').click();
+    await waitForEvent(app, 'QueueChanged');
+
+    const state = await getQueue(app);
+
+    expect(state.tracks.map((t) => t.filePath)).toEqual(paths);
+    expect(state.currentIndex).toBe(0);
+    expect(state.shuffleMode).toBe(false);
+    expect(state.source).toEqual(TRACKS_SOURCE);
+  });
+
+  test('Tracks Shuffle all turns shuffle on and keeps the source', async ({
+    app,
+  }) => {
+    await app.getByTestId('nav-tracks').click();
+    await expect(
+      app.locator('track-list [data-testid="track-row"]').first(),
+    ).toBeVisible();
+
+    const paths = await displayedPaths(app, 'track-list');
+
+    await resetEvents(app);
+    await app.getByTestId('page-action-shuffle-all').click();
+    await waitForEvent(app, 'QueueChanged');
+
+    const state = await getQueue(app);
+
+    expect(state.tracks.map((t) => t.filePath)).toEqual(paths);
+    expect(state.shuffleMode).toBe(true);
+    expect(state.source).toEqual(TRACKS_SOURCE);
+  });
+});
+
+test.describe('play-all on an embedded track list', () => {
+  test.beforeEach(async ({ app }) => {
+    await callBinding(app, 'queue.Queue.Clear').catch(() => {});
+    await setShuffleMode(app, false);
+  });
+
+  test('a genre page queues the genre with its name as the source', async ({
+    app,
+  }) => {
+    await app.getByTestId('nav-genres').click();
+    await expect(app.getByTestId('main-content')).toHaveAttribute(
+      'data-active-view',
+      'genres',
+    );
+
+    const first = app.locator('genres-view .genre-card').first();
+
+    await expect(first).toBeVisible();
+    await first.click();
+
+    await expect(app.getByTestId('main-content')).toHaveAttribute(
+      'data-active-view',
+      'genre-details',
+    );
+    await expect(
+      app.locator('genre-details [data-testid="track-row"]').first(),
+    ).toBeVisible();
+
+    const genreName = (await app
+      .locator('genre-details .genre-title')
+      .textContent())?.trim();
+    const paths = await displayedPaths(app, 'genre-details');
+
+    await resetEvents(app);
+    await app
+      .locator('genre-details [data-testid="page-action-play-all"]')
+      .click();
+    await waitForEvent(app, 'QueueChanged');
+
+    const state = await getQueue(app);
+
+    expect(state.tracks.map((t) => t.filePath)).toEqual(paths);
+    expect(state.currentIndex).toBe(0);
+    expect(state.source).toEqual({ type: 'genre', id: 0, label: genreName });
+  });
+});
+
+test.describe('play-all on the library artist page', () => {
+  test.beforeEach(async ({ app }) => {
+    await callBinding(app, 'queue.Queue.Clear').catch(() => {});
+    await setShuffleMode(app, false);
+  });
+
+  test('an artist page queues album paths in album order with the artist source', async ({
+    app,
+  }) => {
+    const artists = await callBinding<{ ID: number; Name: string }[]>(
+      app,
+      'library.Library.GetArtists',
+      [0],
+    );
+    const first = artists[0]!;
+
+    await app.evaluate(
+      ([id, name]) => {
+        document.dispatchEvent(
+          new CustomEvent('navigate', {
+            detail: {
+              view: 'artist-details',
+              artistId: id,
+              artistName: name,
+            },
+            bubbles: true,
+            composed: true,
+          }),
+        );
+      },
+      [first.ID, first.Name] as const,
+    );
+
+    await expect(app.getByTestId('main-content')).toHaveAttribute(
+      'data-active-view',
+      'artist-details',
+    );
+    await expect(app.getByTestId('artist-play-all')).toBeEnabled();
+
+    const albums = await callBinding<{ ID: number }[]>(
+      app,
+      'library.Library.GetAlbumsByArtist',
+      [first.Name, 0],
+    );
+    const byAlbum = await callBinding<Record<string, string[]>>(
+      app,
+      'library.Library.GetFilePathsByAlbums',
+      [albums.map((a) => a.ID), 0],
+    );
+    const expected: string[] = [];
+
+    for (const album of albums) {
+      expected.push(...(byAlbum[String(album.ID)] ?? []));
+    }
+
+    await resetEvents(app);
+    await app.getByTestId('artist-play-all').click();
+    await waitForEvent(app, 'QueueChanged');
+
+    const state = await getQueue(app);
+
+    expect(state.tracks.map((t) => t.filePath)).toEqual(expected);
+    expect(state.source).toEqual({
+      type: 'artist',
+      id: first.ID,
+      label: first.Name,
+    });
+  });
+});

e2e/specs/playback.spec.ts

@@ -137,7 +137,7 @@ test.describe('queue', () => {
   });
 
   test('shuffle and repeat toggles report their state', async ({ app }) => {
-    const shuffle = app.getByRole('button', { name: 'Shuffle' });
+    const shuffle = app.getByRole('button', { name: 'Shuffle', exact: true });
 
     await resetEvents(app);
     await shuffle.click();

frontend/src/components/artist-details/artist-details.ts

@@ -11,11 +11,22 @@ import {
     GetArtistImageCachedPath,
     GetArtistMBID,
 } from '@go/explore/service.js';
+import { GetFilePathsByAlbums } from '@go/library/library.js';
+import { libraryStore } from '@store/library-store';
+import { notificationStore } from '@store/notification-store';
+import { dict } from '@utils/binding';
+import { playAll } from '@utils/play-all';
+import { describeError } from '@utils/describe-error';
+import { ICON_PLAY, ICON_SHUFFLE } from '@utils/icon-language';
 import '@awesome.me/webawesome/dist/components/icon/icon.js';
 import '@components/cover-grid/cover-grid.js';
+import '../notifications/inline-notice';
 import { designTokens } from '../../styles/tokens.css';
 import { backButton } from '../../styles/back-button.css';
 
+/** The region the artist header's own failures are rendered in. */
+const ArtistRegion = 'library-artist';
+
 @customElement('artist-details')
 export class ArtistDetails extends LitElement {
     @property({ type: Number, attribute: 'artist-id' })
@@ -131,6 +142,39 @@ export class ArtistDetails extends LitElement {
             );
         }
 
+        .header-actions {
+            margin-left: auto;
+            display: flex;
+            align-items: center;
+            gap: 8px;
+            flex-shrink: 0;
+        }
+
+        .header-action {
+            background: none;
+            border: 1px solid var(--yj-border-subtle, #555);
+            border-radius: 4px;
+            color: var(--yj-text-primary, #fff);
+            padding: 6px 12px;
+            font-size: var(--yj-text-md, 13px);
+            font-family: inherit;
+            cursor: pointer;
+            display: flex;
+            align-items: center;
+            gap: 6px;
+            white-space: nowrap;
+        }
+
+        .header-action:hover {
+            border-color: var(--yj-accent, #ffd43b);
+            color: var(--yj-accent-text, #ffd43b);
+        }
+
+        .header-action:disabled {
+            opacity: 0.5;
+            cursor: default;
+        }
+
         /* ====================================
          * Content
          * ==================================== */
@@ -145,6 +189,23 @@ export class ArtistDetails extends LitElement {
             height: 100%;
         }
 
+        /* Phone widths: the header's flex row squeezed .artist-info to
+         * nothing, so the title ellipsised away entirely and the
+         * actions clipped against the host's own overflow — the album
+         * page's fault one detail view over (#66). The pair takes its
+         * own row instead. Written last, because a media query adds no
+         * specificity and a rule placed above the plain ones it
+         * overrides is silently dead. */
+        @media (max-width: 599px) {
+            .artist-header {
+                flex-wrap: wrap;
+            }
+
+            .header-actions {
+                flex-basis: 100%;
+                margin-left: 0;
+            }
+        }
     `];
 
     override connectedCallback() {
@@ -302,6 +363,45 @@ export class ArtistDetails extends LitElement {
         return name.charAt(0).toUpperCase();
     }
 
+    /**
+     * Play every track on this artist's albums, in album order.
+     *
+     * One `GetFilePathsByAlbums` call returns the paths grouped by
+     * album id; the caller owns the ordering, so they are flattened in
+     * `this.albums` order rather than by id.
+     */
+    private async playAllTracks(shuffle: boolean): Promise<void> {
+        if (this.albums.length === 0) return;
+
+        try {
+            const libId = libraryStore.getSelectedLibraryId() ?? 0;
+            const ids = this.albums.map((a) => a.ID);
+            const byAlbum = await dict(
+                GetFilePathsByAlbums(ids, libId),
+            );
+            const paths: string[] = [];
+
+            for (const id of ids) {
+                paths.push(...(byAlbum[id] ?? []));
+            }
+
+            playAll(
+                paths,
+                {
+                    type: 'artist',
+                    id: this.artistId,
+                    label: this.artistName,
+                },
+                shuffle,
+            );
+        } catch (error) {
+            console.error('Could not play artist:', error);
+            notificationStore.inline(ArtistRegion, {
+                text: describeError(error, 'Could not play this artist’s tracks.'),
+            });
+        }
+    }
+
     /* ================================================================
      * Rendering
      * ================================================================ */
@@ -351,12 +451,38 @@ export class ArtistDetails extends LitElement {
                           `
                         : ''}
                 </div>
+                <div class="header-actions">
+                    <button
+                        class="header-action"
+                        data-testid="artist-play-all"
+                        ?disabled=${this.albums.length === 0}
+                        @click=${() =>
+                            void this.playAllTracks(false)}
+                    >
+                        <wa-icon name=${ICON_PLAY}></wa-icon>
+                        Play all
+                    </button>
+                    <button
+                        class="header-action"
+                        data-testid="artist-shuffle-all"
+                        ?disabled=${this.albums.length === 0}
+                        @click=${() =>
+                            void this.playAllTracks(true)}
+                    >
+                        <wa-icon name=${ICON_SHUFFLE}></wa-icon>
+                        Shuffle all
+                    </button>
+                </div>
             </div>
             <div class="content">
                 <cover-grid
                     .externalAlbums=${this.albums}
                 ></cover-grid>
             </div>
+            <inline-notice
+                region=${ArtistRegion}
+                testid="artist-play-message"
+            ></inline-notice>
         `;
     }
 }

frontend/src/components/config-page/config-page.ts

@@ -51,7 +51,7 @@ import type { BackgroundShade } from '@store/theme-store';
 import type { IconStyle } from '@store/favorites-store';
 import {
     COLUMN_DEFS,
-    ALL_COLUMN_IDS,
+    CONFIGURABLE_COLUMN_IDS,
 } from '@components/track-list/columns';
 
 import './config-field';
@@ -1640,7 +1640,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
             ...this.trackListCtrl.columnIds,
         ];
 
-        const disabledIds = ALL_COLUMN_IDS.filter(
+        const disabledIds = CONFIGURABLE_COLUMN_IDS.filter(
             (id) => !enabledIds.includes(id),
         );
 

frontend/src/components/explore-album-details/explore-album-details.ts

@@ -43,6 +43,7 @@ import type * as autotagservice from '@go/autotagservice/models.js';
 import { confirmAction } from '../confirm-dialog/confirm-dialog';
 import { queueStore } from '../../store/queue-store';
 import type { QueueSource } from '../../store/queue-store';
+import { playAll } from '@utils/play-all';
 import { notificationStore } from '../../store/notification-store';
 import '../notifications/inline-notice';
 import {
@@ -2771,20 +2772,11 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
 
     /** Play what the user owns of this release, optionally shuffled. */
     private playOwned(shuffle: boolean): void {
-        const paths = this.ownedFilePaths();
-
         // The button is only rendered when there is something to play,
-        // so an empty set here is not a state the user can reach.
+        // so an empty set here is not a state the user can reach.  The
-        if (paths.length === 0) return;
+        // shuffle-mode semantics live in `playAll`, shared with the
-
+        // play-all/shuffle-all pair on every track list.
-        // `shuffleStart` only picks a random first track when shuffle
+        playAll(this.ownedFilePaths(), this.queueSource(), shuffle);
-        // mode is *already* on — it does not turn it on — so the mode
-        // has to be set before the queue, not after.
-        if (shuffle && !queueStore.getState().shuffleMode) {
-            queueStore.toggleShuffle();
-        }
-
-        queueStore.setQueue(paths, 0, shuffle, this.queueSource());
     }
 
     /** Append what the user owns of this release to the queue. */

frontend/src/components/first-run-wizard/first-run-wizard.ts

@@ -2,12 +2,14 @@ import { LitElement, html, css, nothing } from 'lit';
 import { customElement, state, query } from 'lit/decorators.js';
 import '@awesome.me/webawesome/dist/components/dialog/dialog.js';
 import '@awesome.me/webawesome/dist/components/icon/icon.js';
+import { EventsOn } from '@runtime/runtime';
 import {
     AddLibrary,
     GetAllLibrariesWithTrackCounts,
 } from '@go/library/library.js';
 import { describeError, explainError } from '@utils/describe-error';
 import { nameDialogsIn } from '@utils/name-dialog';
+import { Events } from '../../events';
 import { pickDirectory } from '../../utils/pick-directory';
 
 /**
@@ -19,6 +21,13 @@ import { pickDirectory } from '../../utils/pick-directory';
  * prompting the user to pick their music folder, registers it through the
  * library CRUD API, and dismisses itself.  AddLibrary emits LibraryAdded
  * and kicks off the initial scan automatically.
+ *
+ * **The dismissal follows the library existing, not the button being
+ * pressed.** `AddLibrary` emits `LibraryAdded` whoever calls it, so the
+ * wizard waits on the state it exists to wait for rather than on a step
+ * in its own flow — a library arriving by any other route (Settings, a
+ * direct call) leaves a full-screen modal up otherwise, intercepting
+ * every pointer event.
  */
 @customElement('first-run-wizard')
 export class FirstRunWizard extends LitElement {
@@ -37,9 +46,18 @@ export class FirstRunWizard extends LitElement {
     /** Error message from a failed pick/save, if any. */
     @state() private errorMessage = '';
 
+    /** Unsubscribe from LibraryAdded, while this element is connected. */
+    private cancelLibraryAdded?: () => void;
+
     override async connectedCallback(): Promise<void> {
         super.connectedCallback();
 
+        // Subscribed before the read below, so a library arriving while
+        // that call is in flight is not answered with a stale empty list.
+        this.cancelLibraryAdded = EventsOn(Events.LibraryAdded, () => {
+            this.dismiss();
+        });
+
         try {
             const existing = await GetAllLibrariesWithTrackCounts();
 
@@ -54,6 +72,8 @@ export class FirstRunWizard extends LitElement {
             return;
         }
 
+        if (this.finished) return;
+
         this.active = true;
 
         await this.updateComplete;
@@ -61,6 +81,13 @@ export class FirstRunWizard extends LitElement {
         if (this.dialog) this.dialog.open = true;
     }
 
+    override disconnectedCallback(): void {
+        this.cancelLibraryAdded?.();
+        this.cancelLibraryAdded = undefined;
+
+        super.disconnectedCallback();
+    }
+
     static override styles = css`
         wa-dialog {
             --width: 480px;
@@ -239,6 +266,20 @@ export class FirstRunWizard extends LitElement {
         if (!this.finished) e.preventDefault();
     };
 
+    /**
+     * Close, and stay closed: a library exists, so setup is over.
+     *
+     * `finished` is set first, or `preventClose` cancels the hide this
+     * asks for.
+     */
+    private dismiss(): void {
+        this.finished = true;
+
+        if (this.dialog) this.dialog.open = false;
+
+        this.active = false;
+    }
+
     private handleChoose = async (): Promise<void> => {
         this.errorMessage = '';
 
@@ -264,11 +305,7 @@ export class FirstRunWizard extends LitElement {
         try {
             await AddLibrary(this.selectedDirectory);
 
-            this.finished = true;
+            this.dismiss();
-
-            if (this.dialog) this.dialog.open = false;
-
-            this.active = false;
         } catch (err) {
             this.errorMessage = explainError(
                 err,

frontend/src/components/playlist-details/playlist-details.ts

@@ -85,7 +85,9 @@ import {
     ICON_PLAYLIST,
     ICON_QUEUE,
     ICON_REMOVE,
+    ICON_SHUFFLE,
 } from '@utils/icon-language';
+import { playAll } from '@utils/play-all';
 
 /** One playlist row: the track and its position in the *playlist*,
  *  which is not its position in the filtered view. */
@@ -355,14 +357,32 @@ export class PlaylistDetails
     // Track interactions
     // =================================================================
 
-    private handlePlayAll() {
+    private playableFilePaths(): string[] {
-        const filePaths = this.tracks
+        return this.tracks
             .filter((t) => !t.Phantom)
             .map((t) => t.FilePath);
+    }
 
-        if (filePaths.length === 0) return;
+    private handlePlayAll() {
+        // Start at the first row, not at a random one: the old `true`
+        // was `shuffleStart`, which only picks a random first track
+        // when shuffle mode is already on — so "Play All" quietly did
+        // "play from the top" while leaving the mode as it was.  The
+        // mode semantics now live in `playAll`, shared with the other
+        // track lists.
+        playAll(
+            this.playableFilePaths(),
+            { type: 'playlist', id: this.playlistId, label: this.playlistName },
+            false,
+        );
+    }
 
-        queueStore.setQueue(filePaths, 0, true, { type: 'playlist', id: this.playlistId, label: this.playlistName });
+    private handleShuffleAll() {
+        playAll(
+            this.playableFilePaths(),
+            { type: 'playlist', id: this.playlistId, label: this.playlistName },
+            true,
+        );
     }
 
     private handleTrackClick(
@@ -1599,9 +1619,16 @@ export class PlaylistDetails
                     class="play-all-button"
                     @click=${() => this.handlePlayAll()}
                 >
-                    <wa-icon name="play"></wa-icon>
+                    <wa-icon name=${ICON_PLAY}></wa-icon>
                     Play All
                 </button>
+                <button
+                    class="play-all-button"
+                    @click=${() => this.handleShuffleAll()}
+                >
+                    <wa-icon name=${ICON_SHUFFLE}></wa-icon>
+                    Shuffle All
+                </button>
             </div>
             <div class="track-header">
                 <div class="header-cell col-number">#</div>

frontend/src/components/smart-playlist-details/smart-playlist-details.ts

@@ -15,6 +15,7 @@ import {
 import { EventsOn } from '@runtime/runtime';
 import { Events } from '../../events';
 import { queueStore } from '@store/queue-store';
+import { playAll } from '@utils/play-all';
 import { creditStore } from '@store/credit-store';
 import { PlayerController } from '@store/controllers/player-controller';
 import { SearchController } from '@store/controllers/search-controller';
@@ -771,24 +772,29 @@ export class SmartPlaylistDetails
     // Actions
     // =================================================================
 
-    private handlePlay() {
+    private playableFilePaths(): string[] {
-        const filePaths = this.tracks
+        return this.tracks
             .filter((t) => !t.Phantom)
             .map((t) => t.FilePath);
+    }
 
-        if (filePaths.length === 0) return;
+    private handlePlay() {
-
+        playAll(
-        queueStore.setQueue(filePaths, 0, false, { type: 'smartPlaylist', id: this.playlistId, label: this.playlistName });
+            this.playableFilePaths(),
+            { type: 'smartPlaylist', id: this.playlistId, label: this.playlistName },
+            false,
+        );
     }
 
     private handleShuffle() {
-        const filePaths = this.tracks
+        // This used to be a no-op when shuffle mode was off: it passed
-            .filter((t) => !t.Phantom)
+        // `shuffleStart` without turning the mode on, so the queue
-            .map((t) => t.FilePath);
+        // started at track 1 in order.  `playAll` sets the mode first.
-
+        playAll(
-        if (filePaths.length === 0) return;
+            this.playableFilePaths(),
-
+            { type: 'smartPlaylist', id: this.playlistId, label: this.playlistName },
-        queueStore.setQueue(filePaths, 0, true, { type: 'smartPlaylist', id: this.playlistId, label: this.playlistName });
+            true,
+        );
     }
 
     private async handleRefresh() {

frontend/src/components/track-list/columns.ts

@@ -32,6 +32,18 @@ export interface ColumnDef {
     id: string;
     /** Human-readable header label. */
     label: string;
+    /**
+     * Whether Settings may offer this column. Defaults to true.
+     *
+     * A definition is not the same thing as a *choice*. `titleArtist`
+     * is the phone's stacked column, picked by width in
+     * `PHONE_COLUMN_IDS`, and `tracklist.AllColumnIDs` in Go does not
+     * list it — so a tick in the configurator sends a column set the
+     * backend rejects with `unknown track-list column ID`, the tick
+     * reverts on the next render, and the only trace is a
+     * `console.error` (#197).
+     */
+    configurable?: boolean;
     /** Extracts the display value from a track. */
     accessor: (track: library.Track) => string;
     /** Default CSS width (used when no saved width exists). */
@@ -98,11 +110,16 @@ export const COLUMN_DEFS: Record<string, ColumnDef> = {
     },
     titleArtist: {
         id: 'titleArtist',
-        // Named for what it sorts by, since that is the only place the
+        // Named for what it sorts by. That label is drawn nowhere
-        // label is user-visible: the phone has no column headers, and
+        // today: the phone has no column headers, and the page header's
-        // the page header's sort list is built from the *configured*
+        // sort list is built from the *configured* columns, which this
-        // columns rather than the drawn ones.
+        // one can never be — see `configurable` below.
         label: 'Track Name',
+        // Chosen by width, never by the user, and rejected by the
+        // backend if it ever were. #197: Settings listed it anyway, so
+        // there were two rows called "Track Name" and the second one
+        // could not be selected.
+        configurable: false,
         accessor: (t) => t.TrackName,
         defaultWidth: '1fr',
         comparator: (a, b) => compareStr(a.TrackName, b.TrackName),
@@ -266,10 +283,15 @@ export const COLUMN_DEFS: Record<string, ColumnDef> = {
 };
 
 /**
- * All column IDs in default display order.
+ * The column IDs Settings may offer, in default display order.
- * Used by the settings UI to list available columns.
+ *
+ * Not every definition is one: a column the user cannot choose has no
+ * row in the configurator, because a checkbox that cannot change
+ * anything is worse than an absent one — see `ColumnDef.configurable`.
  */
-export const ALL_COLUMN_IDS: string[] = Object.keys(COLUMN_DEFS);
+export const CONFIGURABLE_COLUMN_IDS: string[] = Object.keys(
+    COLUMN_DEFS,
+).filter((id) => COLUMN_DEFS[id]?.configurable !== false);
 
 /**
  * Column IDs that are always searched regardless of visibility.

frontend/src/components/track-list/track-list.ts

@@ -26,7 +26,10 @@ import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller
 import { PlayerController } from '@store/controllers/player-controller';
 import { SearchController } from '@store/controllers/search-controller';
 import '@components/page-header/page-header';
-import type { SortOption } from '@components/page-header/page-header';
+import type {
+    SortOption,
+    PageAction,
+} from '@components/page-header/page-header';
 import { TrackListController } from '@store/controllers/tracklist-controller';
 import { FavoritesController } from '@store/controllers/favorites-controller';
 import { queueStore } from '@store/queue-store';
@@ -87,7 +90,9 @@ import {
     ICON_PLAYLIST,
     ICON_PLAY_NEXT,
     ICON_QUEUE,
+    ICON_SHUFFLE,
 } from '@utils/icon-language';
+import { playAll } from '@utils/play-all';
 
 const COLUMN_STORAGE_KEY = 'track-list-column-widths';
 const SORT_FIELD_KEY = 'track-list-sort-field';
@@ -2393,6 +2398,27 @@ export class TrackList
                 .map((c) => ({ id: c.id, label: c.label })),
         ];
 
+        const hasTracks = this.cachedSortedTracks.length > 0;
+
+        const actions: PageAction[] = [
+            {
+                id: 'play-all',
+                label: 'Play all',
+                icon: ICON_PLAY,
+                priority: 1,
+                disabled: !hasTracks,
+                onSelect: this.handlePlayAll,
+            },
+            {
+                id: 'shuffle-all',
+                label: 'Shuffle all',
+                icon: ICON_SHUFFLE,
+                priority: 0,
+                disabled: !hasTracks,
+                onSelect: this.handleShuffleAll,
+            },
+        ];
+
         return html`
             <page-header
                 heading=${this.externalTracks === undefined ? 'Tracks' : ''}
@@ -2404,11 +2430,29 @@ export class TrackList
                 sort-field=${this.sortField ?? ''}
                 sort-direction=${this.sortDirection}
                 search-term=${this.searchCtrl.term}
+                .actions=${actions}
                 @sort-change=${this.onPageHeaderSort}
             ></page-header>
         `;
     }
 
+    /** The queue is the list as displayed, in the order the user sees. */
+    private handlePlayAll = (): void => {
+        playAll(
+            this.cachedSortedTracks.map((t) => t.FilePath),
+            this.effectiveQueueSource,
+            false,
+        );
+    };
+
+    private handleShuffleAll = (): void => {
+        playAll(
+            this.cachedSortedTracks.map((t) => t.FilePath),
+            this.effectiveQueueSource,
+            true,
+        );
+    };
+
     private onPageHeaderSort = (
         e: CustomEvent<{ field: string; direction: 'asc' | 'desc' }>,
     ) => {

frontend/src/utils/play-all.ts

@@ -0,0 +1,26 @@
+import { queueStore } from '@store/queue-store';
+import type { QueueSource } from '@store/queue-store';
+
+/**
+ * Queue a list and start it, optionally shuffled.
+ *
+ * This is the one place that owns what "shuffle this collection" means.
+ * `SetQueue`'s `shuffleStart` only picks a random first track when
+ * shuffle mode is *already* on — it does not turn it on — so the mode
+ * has to be set before the queue, not after.  The album page used to
+ * carry that rule privately; the play-all/shuffle-all pair on every
+ * track list now shares it.
+ */
+export function playAll(
+    paths: string[],
+    source: QueueSource | undefined,
+    shuffle: boolean,
+): void {
+    if (paths.length === 0) return;
+
+    if (shuffle && !queueStore.getState().shuffleMode) {
+        queueStore.toggleShuffle();
+    }
+
+    queueStore.setQueue(paths, 0, shuffle, source);
+}

frontend/test/components/first-run-wizard.test.ts

@@ -0,0 +1,123 @@
+/**
+ * #175: the first-run wizard's dismissal follows the library existing,
+ * not its own button being pressed.
+ *
+ * The wizard is a modal that blocks every pointer event, so a library
+ * arriving by another route — Settings, a direct call — used to leave
+ * it up over an app that was already set up. `LibraryAdded` is emitted
+ * by `AddLibrary` whoever calls it, which is what makes one
+ * subscription the whole fix.
+ */
+import { beforeEach, describe, expect, it } from 'vitest';
+
+import { Events } from '../../src/events';
+import { emit, stub } from '../support/harness';
+import { fixture, shadow, shadowAll } from '../support/render';
+import { wails } from '../support/wails-fake';
+
+import '@components/first-run-wizard/first-run-wizard';
+
+import type { FirstRunWizard } from '@components/first-run-wizard/first-run-wizard';
+
+/** A library row, as `GetAllLibrariesWithTrackCounts` returns one. */
+const aLibrary = {
+    id: 1,
+    name: 'Music',
+    path: '/home/logan/Music',
+    trackCount: 9,
+};
+
+/** Mount the wizard on a fresh install: no libraries yet. */
+async function wizardOnAFreshInstall(): Promise<FirstRunWizard> {
+    stub('library.Library.GetAllLibrariesWithTrackCounts', []);
+
+    return fixture<FirstRunWizard>('first-run-wizard');
+}
+
+/** Whether the wizard is rendering its modal at all. */
+function isShowing(el: FirstRunWizard): boolean {
+    return shadow(el, 'wa-dialog') !== null;
+}
+
+beforeEach(() => {
+    stub('library.Library.AddLibrary', aLibrary);
+});
+
+describe('first-run-wizard', () => {
+    it('shows on a fresh install and stays up until a library exists', async () => {
+        const el = await wizardOnAFreshInstall();
+
+        expect(isShowing(el)).toBe(true);
+    });
+
+    it('stays hidden when a library is already configured', async () => {
+        stub('library.Library.GetAllLibrariesWithTrackCounts', [aLibrary]);
+
+        const el = await fixture<FirstRunWizard>('first-run-wizard');
+
+        expect(isShowing(el)).toBe(false);
+    });
+
+    it('dismisses when a library appears by another route', async () => {
+        const el = await wizardOnAFreshInstall();
+
+        expect(isShowing(el)).toBe(true);
+
+        emit(Events.LibraryAdded, aLibrary);
+        await el.updateComplete;
+
+        expect(isShowing(el)).toBe(false);
+    });
+
+    it('does not raise itself when a library arrives while it is asking', async () => {
+        // The read is still in flight when the event lands, so its
+        // answer — an empty list — is stale by the time it returns.
+        let answer: (libraries: unknown[]) => void = () => {};
+
+        stub(
+            'library.Library.GetAllLibrariesWithTrackCounts',
+            () =>
+                new Promise((resolve) => {
+                    answer = resolve;
+                }),
+        );
+
+        const el = await fixture<FirstRunWizard>('first-run-wizard');
+
+        emit(Events.LibraryAdded, aLibrary);
+        answer([]);
+
+        await el.updateComplete;
+        await new Promise((r) => setTimeout(r, 0));
+        await el.updateComplete;
+
+        expect(isShowing(el)).toBe(false);
+    });
+
+    it('still dismisses through its own Get Started button', async () => {
+        stub('frontendutil.FrontendUtil.HasNativeDirectoryPicker', true);
+        stub('frontendutil.FrontendUtil.DirectoryPicker', '/home/logan/Music');
+
+        const { resetDirectoryPickerCache } = await import(
+            '@utils/pick-directory'
+        );
+
+        resetDirectoryPickerCache();
+
+        const el = await wizardOnAFreshInstall();
+        const [choose, finish] = shadowAll<HTMLButtonElement>(el, '.btn');
+
+        choose?.click();
+        await new Promise((r) => setTimeout(r, 0));
+        await el.updateComplete;
+
+        finish?.click();
+        await new Promise((r) => setTimeout(r, 0));
+        await el.updateComplete;
+
+        expect(
+            wails.calls.filter((c) => c.path === 'library.Library.AddLibrary'),
+        ).toHaveLength(1);
+        expect(isShowing(el)).toBe(false);
+    });
+});

frontend/test/components/play-all.test.ts

@@ -0,0 +1,373 @@
+/**
+ * The play-all/shuffle-all pair on every page that lists tracks.
+ *
+ * The pair is driven by one helper (`utils/play-all`) that owns the
+ * one rule the album page already carried: `shuffleStart` does not turn
+ * shuffle on, it only picks a random first track once the mode is on —
+ * so the mode has to be set *before* the queue, not after.  The hosts
+ * differ only in where their paths come from and what `Source` they
+ * hand over.
+ */
+import { describe, expect, it, beforeEach } from 'vitest';
+import type { LitElement } from 'lit';
+
+import '@components/track-list/track-list';
+import '@components/artist-details/artist-details';
+import '@components/playlist-details/playlist-details';
+import '@components/smart-playlist-details/smart-playlist-details';
+import {
+  stub,
+  flush,
+  resetHarness,
+  calls,
+  lastArgs,
+  emit,
+} from '@test/support/harness';
+import {
+  fixture,
+  shadowAll,
+  deepShadow,
+} from '@test/support/render';
+
+/** The action button rendered by `<page-header>`, through the nested
+ *  shadow roots (track-list → page-header). */
+function pageAction(
+  host: LitElement,
+  id: string,
+): HTMLElement | null {
+  return deepShadow<HTMLElement>(host, `[data-testid="page-action-${id}"]`);
+}
+
+function setShuffleMode(on: boolean): void {
+  emit('QueueModeChanged', { shuffleMode: on, repeatMode: 'off' });
+}
+
+/** The queue's `SetQueue` args, with the shuffle flag and source. */
+function queued(): {
+  paths: string[];
+  startIndex: number;
+  shuffleStart: boolean;
+  source: unknown;
+} {
+  const args = lastArgs('queue.Queue.SetQueue');
+
+  if (!args) throw new Error('nothing was queued');
+
+  return {
+    paths: args[0] as string[],
+    startIndex: args[1] as number,
+    shuffleStart: args[2] as boolean,
+    source: args[3],
+  };
+}
+
+// =====================================================================
+// The track list (Tracks, and every embedding detail view)
+// =====================================================================
+
+const PATHS = Array.from({ length: 12 }, (_, i) => `/music/track-${i}.mp3`);
+
+const LIST = PATHS.map((FilePath, i) => ({
+  FilePath,
+  TrackName: `Track ${i}`,
+  ArtistName: 'An Artist',
+  Album: 'An Album',
+  Duration: 180,
+}));
+
+const GENRE_SOURCE = { type: 'genre', id: 0, label: 'Dream Pop' };
+
+async function embeddedTrackList(): Promise<LitElement> {
+  resetHarness();
+  localStorage.removeItem('track-list-column-widths');
+
+  const el = await fixture<LitElement>('track-list', {
+    externalTracks: LIST,
+    queueSource: GENRE_SOURCE,
+  });
+
+  // Say which order is being asserted rather than inheriting a
+  // persisted sort. See play-in-context.test.ts for the same trap.
+  (el as unknown as { sortField: string | null }).sortField = null;
+
+  el.style.display = 'block';
+  el.style.height = '600px';
+  await flush();
+  await el.updateComplete;
+  await new Promise((r) => setTimeout(r, 60));
+
+  return el;
+}
+
+describe('the track-list play-all/shuffle-all pair', () => {
+  beforeEach(() => {
+    resetHarness();
+    localStorage.removeItem('track-list-column-widths');
+  });
+
+  it('renders in the primary Tracks header and is disabled while empty', async () => {
+    const el = await fixture<LitElement>('track-list', {});
+
+    const play = pageAction(el, 'play-all');
+    const shuffle = pageAction(el, 'shuffle-all');
+
+    expect(play).not.toBeNull();
+    expect(shuffle).not.toBeNull();
+    expect(play?.hasAttribute('disabled')).toBe(true);
+    expect(shuffle?.hasAttribute('disabled')).toBe(true);
+  });
+
+  it('renders in an embedded track-list header too', async () => {
+    const el = await embeddedTrackList();
+
+    expect(pageAction(el, 'play-all')).not.toBeNull();
+    expect(pageAction(el, 'shuffle-all')).not.toBeNull();
+    expect(pageAction(el, 'play-all')?.hasAttribute('disabled')).toBe(false);
+    expect(pageAction(el, 'shuffle-all')?.hasAttribute('disabled')).toBe(false);
+  });
+
+  it('Play all queues the displayed list in order, unshuffled', async () => {
+    const el = await embeddedTrackList();
+
+    pageAction(el, 'play-all')!.click();
+    await flush();
+
+    expect(queued()).toEqual({
+      paths: PATHS,
+      startIndex: 0,
+      shuffleStart: false,
+      source: GENRE_SOURCE,
+    });
+  });
+
+  it('Shuffle all turns shuffle on before queueing when the mode is off', async () => {
+    const el = await embeddedTrackList();
+
+    setShuffleMode(false);
+    await flush();
+
+    pageAction(el, 'shuffle-all')!.click();
+    await flush();
+
+    const all = calls();
+    const toggle = all.findLastIndex(
+      (c) => c.path === 'queue.Queue.ToggleShuffle',
+    );
+    const setQueue = all.findLastIndex(
+      (c) => c.path === 'queue.Queue.SetQueue',
+    );
+
+    expect(toggle).toBeGreaterThanOrEqual(0);
+    expect(toggle).toBeLessThan(setQueue);
+
+    expect(queued()).toEqual({
+      paths: PATHS,
+      startIndex: 0,
+      shuffleStart: true,
+      source: GENRE_SOURCE,
+    });
+  });
+
+  it('Shuffle all does not toggle when the mode is already on', async () => {
+    const el = await embeddedTrackList();
+
+    setShuffleMode(true);
+    await flush();
+
+    pageAction(el, 'shuffle-all')!.click();
+    await flush();
+
+    expect(calls('queue.Queue.ToggleShuffle')).toHaveLength(0);
+
+    expect(queued()).toEqual({
+      paths: PATHS,
+      startIndex: 0,
+      shuffleStart: true,
+      source: GENRE_SOURCE,
+    });
+  });
+});
+
+// =====================================================================
+// The library artist page
+// =====================================================================
+
+const ALBUMS = [
+  { ID: 3, Name: 'Third', ArtistName: 'Aurora Fields', ArtistMBID: '', MBID: '', CoverArtPath: '', CoverArtSmall: '', CoverArtMedium: '', CoverArtLarge: '', Year: 0, ReleaseYear: 0 },
+  { ID: 1, Name: 'First', ArtistName: 'Aurora Fields', ArtistMBID: '', MBID: '', CoverArtPath: '', CoverArtSmall: '', CoverArtMedium: '', CoverArtLarge: '', Year: 0, ReleaseYear: 0 },
+  { ID: 2, Name: 'Second', ArtistName: 'Aurora Fields', ArtistMBID: '', MBID: '', CoverArtPath: '', CoverArtSmall: '', CoverArtMedium: '', CoverArtLarge: '', Year: 0, ReleaseYear: 0 },
+];
+
+describe('the artist page play-all pair', () => {
+  beforeEach(() => {
+    resetHarness();
+    stub('explore.Service.GetArtistMBID', '');
+    stub('library.Library.GetAlbumsByArtist', ALBUMS);
+    stub('library.Library.GetFilePathsByAlbums', {
+      '3': ['/a3-1', '/a3-2'],
+      '1': ['/a1'],
+      '2': ['/a2-1', '/a2-2', '/a2-3'],
+    });
+  });
+
+  it('flattens album paths in the album list order', async () => {
+    const el = await fixture<LitElement>('artist-details', {
+      artistId: 7,
+      artistName: 'Aurora Fields',
+      artistMBID: '',
+    });
+
+    await flush();
+    await el.updateComplete;
+
+    const play = shadowAll<HTMLElement>(el, '[data-testid="artist-play-all"]')[0];
+
+    play!.click();
+    await flush();
+
+    expect(queued()).toEqual({
+      paths: ['/a3-1', '/a3-2', '/a1', '/a2-1', '/a2-2', '/a2-3'],
+      startIndex: 0,
+      shuffleStart: false,
+      source: { type: 'artist', id: 7, label: 'Aurora Fields' },
+    });
+  });
+});
+
+// =====================================================================
+// A smart playlist
+// =====================================================================
+
+function smartPlaylistTracks(n: number) {
+  return Array.from({ length: n }, (_, i) => ({
+    ID: i + 1,
+    FilePath: `/music/track-${i}.mp3`,
+    Title: `Track ${i}`,
+    Artist: 'An Artist',
+    Album: 'An Album',
+    Duration: 180000,
+    CoverArtSmall: `/covers/${i}_sm.jpg`,
+    CoverArtMedium: `/covers/${i}_md.jpg`,
+    CoverArtPath: `/covers/${i}.jpg`,
+    Phantom: false,
+  }));
+}
+
+describe('the smart-playlist play-all pair', () => {
+  beforeEach(() => {
+    resetHarness();
+    stub('playlist.Service.GetSmartPlaylistTracks', smartPlaylistTracks(8));
+    stub('playlist.Service.GetSmartPlaylistRules', '{"rules":[]}');
+    stub('playlist.Service.GetAllPlaylists', []);
+  });
+
+  /** The details header's own action row, not a page-header action. */
+  function actionButton(el: LitElement, label: string): HTMLElement {
+    const button = shadowAll<HTMLElement>(el, '.action-button').find(
+      (b) => b.textContent?.trim() === label,
+    );
+
+    if (!button) throw new Error(`no "${label}" action button rendered`);
+
+    return button;
+  }
+
+  it('Shuffle turns the mode on before queueing, so the queue starts shuffled', async () => {
+    const el = await fixture<LitElement>('smart-playlist-details', {
+      playlistId: 1,
+      playlistName: 'A smart playlist',
+    });
+
+    el.style.display = 'block';
+    el.style.height = '600px';
+    await flush();
+    await el.updateComplete;
+    await new Promise((r) => setTimeout(r, 60));
+
+    setShuffleMode(false);
+    await flush();
+
+    actionButton(el, 'Shuffle').click();
+    await flush();
+
+    // The issue's headline fix. `SetQueue`'s `shuffleStart` only picks
+    // a random first track when shuffle mode is already on — it does
+    // not turn it on — so reverting the mode toggle puts the queue back
+    // to track 1 in order while every assertion about the queue's
+    // contents still passes. Order of the calls is the assertion.
+    const all = calls();
+    const toggle = all.findLastIndex(
+      (c) => c.path === 'queue.Queue.ToggleShuffle',
+    );
+    const setQueue = all.findLastIndex(
+      (c) => c.path === 'queue.Queue.SetQueue',
+    );
+
+    expect(toggle).toBeGreaterThanOrEqual(0);
+    expect(toggle).toBeLessThan(setQueue);
+
+    expect(queued()).toEqual({
+      paths: Array.from({ length: 8 }, (_, i) => `/music/track-${i}.mp3`),
+      startIndex: 0,
+      shuffleStart: true,
+      source: { type: 'smartPlaylist', id: 1, label: 'A smart playlist' },
+    });
+  });
+});
+
+// =====================================================================
+// A regular playlist
+// =====================================================================
+
+function playlistTracks(n: number) {
+  return Array.from({ length: n }, (_, i) => ({
+    ID: i + 1,
+    FilePath: `/music/track-${i}.mp3`,
+    Title: `Track ${i}`,
+    Artist: 'An Artist',
+    Album: 'An Album',
+    Duration: 180000,
+    Phantom: false,
+  }));
+}
+
+describe('the playlist play-all pair', () => {
+  beforeEach(() => {
+    resetHarness();
+    stub('playlist.Service.GetPlaylistTracks', playlistTracks(8));
+    stub('playlist.Service.GetAllPlaylists', []);
+  });
+
+  it('offers Shuffle All beside Play All, both through the helper', async () => {
+    const el = await fixture<LitElement>('playlist-details', {
+      playlistId: 1,
+      playlistName: 'A playlist',
+    });
+
+    el.style.display = 'block';
+    el.style.height = '600px';
+    await flush();
+    await el.updateComplete;
+    await new Promise((r) => setTimeout(r, 60));
+
+    const buttons = shadowAll<HTMLElement>(el, '.play-all-button');
+
+    expect(buttons.map((b) => b.textContent?.trim())).toEqual([
+      'Play All',
+      'Shuffle All',
+    ]);
+
+    setShuffleMode(false);
+    await flush();
+
+    buttons[1]!.click();
+    await flush();
+
+    expect(queued()).toEqual({
+      paths: Array.from({ length: 8 }, (_, i) => `/music/track-${i}.mp3`),
+      startIndex: 0,
+      shuffleStart: true,
+      source: { type: 'playlist', id: 1, label: 'A playlist' },
+    });
+  });
+});

frontend/test/components/settings-column-list.test.ts

@@ -0,0 +1,190 @@
+/**
+ * Settings offers the columns the backend will accept, and no others.
+ *
+ * The list is built from `COLUMN_DEFS`, which is the *drawing* table:
+ * every definition the track list knows how to render, including
+ * `titleArtist` — the phone's stacked column, chosen by width in
+ * `PHONE_COLUMN_IDS` and never by a person. `tracklist.AllColumnIDs` in
+ * Go does not list that id, so the configurator offered a nineteenth
+ * row that could not be ticked:
+ *
+ * ```
+ * validate = unknown track-list column ID: "titleArtist"
+ * titleArtist valid = false
+ * ```
+ *
+ * What a user saw was **two rows both called "Track Name"** (#197), one
+ * of which did nothing — and a screen reader heard "Show the Track Name
+ * column" twice with nothing to tell them apart, which is `a11y.32`'s
+ * complaint inside the list that was fixed for exactly that.
+ *
+ * It is worse than an inert control, which is why the duplicate name
+ * was not the thing to fix. `SetTrackListColumns` assigns before it
+ * validates, so a rejected list stays in memory and `Save()` validates
+ * the whole config:
+ *
+ * ```
+ * later, unrelated SetThemeAccentColor = could not save config: invalid
+ *   config: ... unknown track-list column ID: "titleArtist"
+ * ```
+ *
+ * — one tick and no setting saves for the rest of the session. That
+ * half is filed separately; this file keeps the row from being offered.
+ *
+ * The last test is the one that would have caught it when the column
+ * was added: the two lists are in different languages, so nothing but a
+ * sweep can hold them together.
+ */
+import { beforeEach, describe, expect, it } from 'vitest';
+
+import '@components/config-page/config-page';
+
+import {
+  COLUMN_DEFS,
+  CONFIGURABLE_COLUMN_IDS,
+} from '@components/track-list/columns';
+import { flush, stub } from '@test/support/harness';
+import { fixture, shadowAll } from '@test/support/render';
+
+/** Go's own list of column ids, as text. */
+const GO_CONFIG = Object.values(
+  import.meta.glob<string>('../../../backend/tracklist/config.go', {
+    eager: true,
+    query: '?raw',
+    import: 'default',
+  }),
+)[0];
+
+/**
+ * The ids `tracklist.AllColumnIDs` actually contains.
+ *
+ * Read out of the source rather than written down here, because a
+ * third copy of this list is a third thing to forget — which is the
+ * defect, one copy earlier.
+ */
+function goColumnIDs(source: string): string[] {
+  const constants = new Map<string, string>();
+  const constBlock = /const \(([\s\S]*?)\n\)/.exec(source)?.[1] ?? '';
+
+  for (const [, name, id] of constBlock.matchAll(
+    /(\w+)\s+ColumnID\s*=\s*"([^"]+)"/g,
+  )) {
+    constants.set(name!, id!);
+  }
+
+  const listBlock =
+    /var AllColumnIDs = \[\]ColumnID\{([\s\S]*?)\n\}/.exec(source)?.[1] ?? '';
+
+  return [...listBlock.matchAll(/(\w+),/g)]
+    .map(([, name]) => constants.get(name!))
+    .filter((id): id is string => id !== undefined);
+}
+
+/**
+ * The column rows, and only those.
+ *
+ * Settings’ view-visibility list (#25) is drawn with the same two
+ * classes, so a bare `.column-label` sweeps 29 rows across two
+ * sections — and "Albums" the destination sitting beside "Album" the
+ * column is not the fault this file is about. The `for`/`id` prefix is
+ * what tells them apart.
+ */
+const COLUMN_ROW_LABEL = 'label.column-label[for^="column-"]';
+const COLUMN_ROW_BOX = 'input.column-toggle[id^="column-"]';
+
+/** The rows the configurator draws, by their visible name. */
+async function columnRowNames(): Promise<string[]> {
+  const page = await fixture('config-page');
+
+  await flush();
+  await page.updateComplete;
+
+  // Every section renders collapsed, and a collapsed body is `hidden`.
+  for (const section of shadowAll<HTMLElement>(page, 'config-section')) {
+    section.shadowRoot
+      ?.querySelector<HTMLButtonElement>('button[aria-expanded="false"]')
+      ?.click();
+  }
+
+  await flush();
+  await page.updateComplete;
+
+  return shadowAll<HTMLElement>(page, COLUMN_ROW_LABEL).map(
+    (label) => label.textContent?.trim() ?? '',
+  );
+}
+
+describe('the Settings column list', () => {
+  beforeEach(() => {
+    for (const path of [
+      'library.Library.GetAllLibrariesWithTrackCounts',
+      'jobs.Service.GetJobs',
+      'download.Service.ListProviders',
+      'download.Service.ProviderKinds',
+    ]) {
+      stub(path, []);
+    }
+
+    stub('config.Config.GetShortcuts', {});
+    stub('config.Config.GetDownloadPreferences', {});
+    stub('config.Config.GetThemeAccentColor', '#ffd43b');
+    stub('config.Config.GetThemeBackgroundShade', 'dark');
+  });
+
+  it('names each row once', async () => {
+    const names = await columnRowNames();
+
+    // A sweep over nothing passes.
+    expect(names.length, 'the page draws column rows').toBeGreaterThan(5);
+
+    const seen = new Set<string>();
+    const duplicated = names.filter((name) => !seen.add(name));
+
+    expect(duplicated).toEqual([]);
+    expect(names.filter((n) => n === 'Track Name')).toHaveLength(1);
+  });
+
+  it('gives each checkbox a name that identifies it', async () => {
+    // The visible half above is what was reported; this is the half a
+    // screen reader gets, and it is the one `config-page` computes
+    // from the same string.
+    const page = await fixture('config-page');
+
+    await flush();
+    await page.updateComplete;
+
+    const labels = shadowAll<HTMLInputElement>(page, COLUMN_ROW_BOX).map(
+      (box) => box.getAttribute('aria-label') ?? '',
+    );
+
+    expect(labels.length, 'the page draws column checkboxes').toBeGreaterThan(5);
+    expect(new Set(labels).size).toBe(labels.length);
+  });
+});
+
+describe('the column table', () => {
+  it('offers no column the backend would reject', async () => {
+    const accepted = goColumnIDs(GO_CONFIG ?? '');
+
+    // Two non-vacuity guards: a glob that stopped matching, and a
+    // parse that stopped finding the list it names.
+    expect(GO_CONFIG, 'backend/tracklist/config.go is readable').toBeTruthy();
+    expect(accepted.length, 'AllColumnIDs was parsed').toBeGreaterThan(10);
+
+    expect(
+      CONFIGURABLE_COLUMN_IDS.filter((id) => !accepted.includes(id)),
+    ).toEqual([]);
+  });
+
+  it('still knows how to draw every column it offers', async () => {
+    // The filter must not have taken a column *out* of the drawing
+    // table: `configurable` says what Settings may list, not what the
+    // list may render.
+    expect(
+      CONFIGURABLE_COLUMN_IDS.filter((id) => COLUMN_DEFS[id] === undefined),
+    ).toEqual([]);
+    expect(CONFIGURABLE_COLUMN_IDS).not.toContain('titleArtist');
+    expect(COLUMN_DEFS['titleArtist'], 'the phone still has its column')
+      .toBeTruthy();
+  });
+});

lefthook.yml

@@ -38,10 +38,14 @@ pre-commit:
       glob: "*.go"
       run: ./scripts/bindings-check.sh
 
-    # .pi/ documents make targets; a stale one sends an agent off a
+    # The docs document make targets; a stale one sends an agent — or a
-    # cliff with total confidence.  Instant.
+    # contributor reading CONTRIBUTING.md — off a cliff with total
+    # confidence.  The glob is the script's own scanned set, because a
+    # hook that does not fire on a file the check reads is the drift the
+    # check exists to prevent: it was `{Makefile,.pi/**/*.md}` while the
+    # script already read CLAUDE.md.  Instant.
     skill-check:
-      glob: "{Makefile,.pi/**/*.md}"
+      glob: "{Makefile,.pi/**/*.md,AGENTS.md,CLAUDE.md,README.md,CONTRIBUTING.md}"
       run: ./scripts/skill-check.sh
 
     frontend-typecheck:

scripts/skill-check.sh

@@ -14,6 +14,11 @@
 # missing: CLAUDE.md names 27 targets and nothing verified one of them,
 # so the file the agents trust most was the file least checked.
 #
+# README.md and CONTRIBUTING.md are in it too, and the header sentence
+# above is why: a person who has *not* read the Makefile goes looking in
+# the contributor-facing doc, so a renamed target sends them off the
+# same cliff it sends an agent off.  CONTRIBUTING.md names 21 targets.
+#
 # **AGENTS.md is a symlink to CLAUDE.md.**  This repo is worked on by
 # two agent harnesses that read different files by convention — Claude
 # Code reads CLAUDE.md, others read AGENTS.md — and two harnesses
@@ -43,7 +48,19 @@ if [ -e AGENTS.md ] || [ -L AGENTS.md ]; then
 	fi
 fi
 
-[ -d .pi ] || exit 0
+# The scan is over the docs that are actually there: a checkout without
+# .pi/ still has README.md and CONTRIBUTING.md to check, and gating the
+# whole run on .pi/ would have made the human-facing half conditional on
+# the agent-facing one.  This list is used twice — once to read the
+# mentions out and once to say which file a missing target came from —
+# because a second list is a second thing to forget.
+# `ls` exits non-zero when *any* of its arguments is missing while still
+# printing the ones that are there, and under `set -e` that would sink
+# the assignment rather than scanning what exists, so swallow it.
+docs="$({ find .pi -name '*.md' 2>/dev/null
+	ls CLAUDE.md README.md CONTRIBUTING.md 2>/dev/null || true; })"
+
+[ -n "$docs" ] || exit 0
 
 # `make -pq` prints the database including every rule, without running
 # anything.  It exits non-zero when a target is out of date, and under
@@ -68,7 +85,7 @@ targets="$({ make -pqRr 2>/dev/null || true; } |
 # AGENTS.md is deliberately not in this list: it is a symlink to
 # CLAUDE.md, asserted above, so scanning it would report every failure
 # twice under two names.
-mentioned="$({ find .pi -name '*.md' 2>/dev/null; echo CLAUDE.md; } |
+mentioned="$(printf '%s\n' "$docs" |
 	xargs awk '
 		FNR == 1 { fence = 0 }
 		/^```/   { fence = !fence; next }
@@ -93,10 +110,10 @@ for t in $mentioned; do
 done
 
 if [ -n "$missing" ]; then
-	echo "skill-check: the agent docs name make targets that do not exist:" >&2
+	echo "skill-check: the docs name make targets that do not exist:" >&2
 	for t in $missing; do
 		echo "  make $t" >&2
-		grep -rln "make $t" .pi CLAUDE.md --include='*.md' | sed 's/^/      /' >&2
+		printf '%s\n' "$docs" | xargs grep -ln "make $t" | sed 's/^/      /' >&2
 	done
 	echo "Fix the docs, or restore the target." >&2
 	exit 1