fix(download): score a multi-disc rip as one album, and count tracks

Three faults in how candidates are shaped and scored, one commit because
they meet in the same completeness number.

Multi-disc albums were split in two.  Soulseek shares them as
Album/CD1 and Album/CD2, and candidates were grouped by the immediate
parent, so each disc became its own candidate titled "CD1": about half
complete, with an album title that could not match.  Such a release
essentially never cleared auto-pick.  AlbumDir groups a disc folder
under its parent, ParsePath takes the disc number from the folder (a
disc in the filename still wins), and collect keeps the disc folders in
staging, where flattened, disc 2's "01 Intro.flac" overwrote disc 1's.

A single-track request could never be served from Soulseek.  A track
search matches one file per folder, and the two-file floor that screens
out noise for an album screened out every result.  A recording request
takes one.

Completeness counted files.  Ten files against a ten-track album scored
full marks whether or not they were its tracks, and title fit is the
mean over the files that did align, so a folder where three titles
matched read as near-perfect on both.  Coverage is now counted in
aligned tracks, with the file count still setting the penalty for
extras.

Closes #270

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017HJiuc3ZZhxsPXz3ozTirT

.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**
@@ -168,8 +186,12 @@ Merge when, and only when, **all** hold:
   `block_on_outdated_branch: true` refuses it anyway; never
   `force_manually_merged` around it.
 
-**Refresh before every merge.** In the loop worktree: fetch, then
-`git merge origin/main` on the PR branch, push. A textual conflict
+**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,
@@ -247,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
@@ -270,4 +295,12 @@ AVD), then `make android-emulator` per session.
   which leg it really is.
 - 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.
+  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.

.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/app.go

@@ -499,6 +499,10 @@ func (yj *YellowJacketApp) OnStartup(ctx context.Context) {
 		PostRemove: yj.explore.InvalidateLibrarySync,
 	})
 
+	// A deleted playlist must not leave the queue's "Playing from"
+	// label pointing at it.
+	yj.playlist.SetOnPlaylistDeleted(yj.queue.DropSourceForPlaylist)
+
 	// Register playback finished handler to drive queue auto-advance.
 	yj.player.SetPlaybackFinishedHandler(yj.queue.OnPlaybackFinished)
 
@@ -775,6 +779,8 @@ func (yj *YellowJacketApp) startJanitor() {
 	}
 
 	yj.janitor.Register(maintenance.ExpiredHTTPCacheJob(yj.database))
+	yj.janitor.Register(maintenance.StaleArtistMetadataJob(yj.database))
+	yj.janitor.Register(maintenance.StaleSearchClicksJob(yj.database))
 	yj.janitor.Register(maintenance.OrphanedCoverFilesJob(
 		yj.database, coversDir, library.CoverArtFileSet,
 	))

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/lyrics_search.go

@@ -151,16 +151,28 @@ func (d *DB) SetLyrics(audioFileID int64, lyrics, source, recordingMBID string)
 	return d.upsertLyricsIndex(audioFileID, lyrics)
 }
 
-// upsertLyricsIndex refreshes a single file's entry in the contentless
-// lyrics_index.  contentless_delete=1 makes the DELETE valid; an empty
-// lyrics string leaves the row deleted.
-func (d *DB) upsertLyricsIndex(audioFileID int64, lyrics string) error {
+// DeleteLyricsIndex removes one file's entry from the contentless
+// lyrics_index.  It is called wherever a file row is deleted — the
+// `lyrics` table cascades with its file, but the FTS entry does not and
+// would otherwise accumulate for the life of the install (#249).
+func (d *DB) DeleteLyricsIndex(audioFileID int64) error {
 	if _, err := d.db.ExecContext(d.Ctx,
 		"DELETE FROM lyrics_index WHERE rowid = ?", audioFileID,
 	); err != nil {
 		return fmt.Errorf("could not delete lyrics_index row: %w", err)
 	}
 
+	return nil
+}
+
+// upsertLyricsIndex refreshes a single file's entry in the contentless
+// lyrics_index.  contentless_delete=1 makes the DELETE valid; an empty
+// lyrics string leaves the row deleted.
+func (d *DB) upsertLyricsIndex(audioFileID int64, lyrics string) error {
+	if err := d.DeleteLyricsIndex(audioFileID); err != nil {
+		return err
+	}
+
 	if strings.TrimSpace(lyrics) == "" {
 		return nil
 	}

backend/database/playlistphantoms.go

@@ -0,0 +1,177 @@
+package database
+
+import (
+	"context"
+	"database/sql"
+	"fmt"
+	"log/slog"
+	"strings"
+)
+
+// 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 (or the scoped
+// variant below) first, inside the same transaction as the delete.
+// These paths have drifted before: 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 (#183).  The incremental scan's orphan cleanup and
+// RemoveFromLibrary drifted the same way and are #246.
+//
+// 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 {
+	return preservePlaylistPhantoms(ctx, tx, nil, logger)
+}
+
+// PreservePlaylistPhantomsForFiles is PreservePlaylistPhantoms scoped to
+// the given audio file ids, for the two removal paths that delete a
+// known subset of the table rather than all of it: the incremental
+// scan's orphan cleanup and RemoveFromLibrary.  A bulk pass there would
+// rewrite every linked playlist row on every scan for nothing.
+func PreservePlaylistPhantomsForFiles(
+	ctx context.Context, tx *sql.Tx, ids []int64, logger *slog.Logger,
+) error {
+	return preservePlaylistPhantoms(ctx, tx, ids, logger)
+}
+
+func preservePlaylistPhantoms(
+	ctx context.Context, tx *sql.Tx, ids []int64, logger *slog.Logger,
+) error {
+	clause, args, skip := phantomIDFilter(ids)
+	if skip {
+		return nil
+	}
+
+	if _, err := tx.ExecContext(
+		ctx, preservePhantomPathSQL+clause, args...,
+	); err != nil {
+		return fmt.Errorf(
+			"could not preserve playlist track file paths: %w", err,
+		)
+	}
+
+	if _, err := tx.ExecContext(
+		ctx, preservePhantomDisplaySQL+clause, args...,
+	); 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
+}
+
+// phantomIDFilter builds the extra WHERE terms and arguments that scope
+// a preservation pass to a set of audio file ids.  A nil ids returns the
+// empty clause (a bulk run over every linked entry); an empty slice
+// reports skip, since there is nothing to preserve.
+func phantomIDFilter(ids []int64) (clause string, args []any, skip bool) {
+	switch {
+	case ids == nil:
+		return "", nil, false
+	case len(ids) == 0:
+		return "", nil, true
+	}
+
+	clause = " AND audio_file_id IN (" +
+		strings.Repeat("?,", len(ids)-1) + "?)"
+
+	args = make([]any, len(ids))
+	for i, id := range ids {
+		args[i] = id
+	}
+
+	return clause, args, false
+}

backend/database/playlistphantoms_test.go

@@ -0,0 +1,94 @@
+package database
+
+import (
+	"context"
+	"database/sql"
+	"testing"
+)
+
+// TestPreservePlaylistPhantomsForFilesScopesToTheRequestedIDs is the
+// scoping half of the scoped variant: a run over one file's id must
+// fill that file's playlist entries and leave every other entry alone,
+// because the incremental scan calls this once per orphan batch and a
+// pass that rewrote the whole table would touch every playlist row on
+// every scan.
+func TestPreservePlaylistPhantomsForFilesScopesToTheRequestedIDs(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 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),
+				(8, '/music/b.flac', 1, 2000,
+				 'Second Tide', 'Aurora Fields', 3, 4);
+		INSERT INTO playlist_tracks (playlist_id, audio_file_id, position)
+			VALUES (1, 7, 0), (1, 8, 1);
+	`); err != nil {
+		t.Fatalf("seed: %v", err)
+	}
+
+	tx, err := db.BeginTx(ctx, nil)
+	if err != nil {
+		t.Fatalf("begin: %v", err)
+	}
+
+	defer func() { _ = tx.Rollback() }()
+
+	if err := PreservePlaylistPhantomsForFiles(
+		ctx, tx, []int64{7}, testLogger(),
+	); err != nil {
+		t.Fatalf("preserve: %v", err)
+	}
+
+	if err := tx.Commit(); err != nil {
+		t.Fatalf("commit: %v", err)
+	}
+
+	var filled, untouched sql.NullString
+
+	if err := db.QueryRowContext(ctx,
+		"SELECT phantom_file_path FROM playlist_tracks WHERE audio_file_id = 7",
+	).Scan(&filled); err != nil {
+		t.Fatalf("read the requested entry: %v", err)
+	}
+
+	if filled.String != "/music/a.flac" {
+		t.Errorf(
+			"requested entry phantom_file_path = %q, want %q",
+			filled.String, "/music/a.flac",
+		)
+	}
+
+	if err := db.QueryRowContext(ctx,
+		"SELECT phantom_file_path FROM playlist_tracks WHERE audio_file_id = 8",
+	).Scan(&untouched); err != nil {
+		t.Fatalf("read the untouched entry: %v", err)
+	}
+
+	if untouched.Valid {
+		t.Errorf(
+			"untouched entry got phantom_file_path = %q, want NULL "+
+				"(a scoped run must not rewrite the whole table)",
+			untouched.String,
+		)
+	}
+}

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
--- existing database this table already exists at schema-pass time with
--- its old column still named request_id, so an inline CREATE INDEX on
--- download_id would fail outright. See ensureDownloadIndexes in
--- backend/database/download_rename_migration.go.
+-- ListDownloadItemsForDownload filters on the parent download.
+CREATE INDEX IF NOT EXISTS idx_download_items_download
+    ON download_items(download_id);

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
--- declared here. This table name is reused from the old one-shot
--- attempt table (also called download_requests before the Want/Request
--- rename), so on an existing database this CREATE TABLE is a no-op
--- against a table that, at schema-pass time, is still shaped like the
--- OLD attempts table and lacks these columns entirely — an inline
--- CREATE INDEX here would fail outright rather than just no-op. See
--- migrateDownloadRename/ensureDownloadIndexes in
--- 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).
+CREATE INDEX IF NOT EXISTS idx_download_requests_due
+    ON download_requests(state, next_try_at);
+
+CREATE INDEX IF NOT EXISTS idx_download_requests_entity
+    ON download_requests(entity, state);
+
+CREATE INDEX IF NOT EXISTS idx_download_requests_parent
+    ON download_requests(parent_id) WHERE parent_id IS NOT NULL;

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
-    -- written it a nonzero value); source_type/source_id/source_label
-    -- below are its generalized replacement, covering albums, playlists,
-    -- smart playlists, genres and artists rather than playlists alone.
+    -- What the queue was built from ("Playing from: X"): an album,
+    -- playlist, smart playlist, genre or artist, identified by the id
+    -- that source_type's namespace gives it.
     source_type TEXT NOT NULL DEFAULT '',
     source_id INTEGER NOT NULL DEFAULT 0,
-    source_label TEXT NOT NULL DEFAULT '',
-    FOREIGN KEY(source_playlist_id) REFERENCES playlists(id) ON DELETE SET NULL
+    source_label TEXT NOT NULL DEFAULT ''
 );
 
 -- 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
-  -- though that reads oddly next to the rest of the table: sql/
-  -- migrations/0001 brings a pre-existing tagging_items up to date
-  -- with `ALTER TABLE ADD COLUMN`, which SQLite always appends at
-  -- 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.
+  -- These columns are appended after created_at rather than grouped
+  -- with the rest of the row: sqlc's `SELECT *` scans (GetTaggingItem)
+  -- bind column order positionally, so new columns always go at the
+  -- end.
   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
@@ -312,15 +317,14 @@ type PlaylistTrack struct {
 }
 
 type Queue struct {
-	ID               int64
-	SourcePlaylistID sql.NullInt64
-	CurrentPosition  int64
-	ShuffleMode      bool
-	RepeatMode       string
-	ShuffleOrder     sql.NullString
-	SourceType       string
-	SourceID         int64
-	SourceLabel      string
+	ID              int64
+	CurrentPosition int64
+	ShuffleMode     bool
+	RepeatMode      string
+	ShuffleOrder    sql.NullString
+	SourceType      string
+	SourceID        int64
+	SourceLabel     string
 }
 
 type QueueTrack struct {

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,
-		Note: "Listening history. Authored, but intentionally cascades " +
-			"with its track — history for a file no longer in the library " +
-			"has nothing to point at.",
+		Name: "listening_events", Kind: Authored, Lifetime: Cascade,
+		Note: "Listening history, one row per track exit (complete, play " +
+			"or skip). Authored, but intentionally cascades with its " +
+			"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,14 +212,14 @@ 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,
-		"queue_tracks": true,
+		"listening_events": true,
+		"queue_tracks":     true,
 
 		// Download history is scoped to the library it imported into.
 		// When that library is removed the files it acquired go with

backend/download/candidate_shape_test.go

@@ -0,0 +1,248 @@
+package download
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// Multi-disc rips, single-track results and coverage counted in tracks
+// rather than files (#270).
+
+func TestParsePathReadsTheDiscFromItsFolder(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		path   string
+		disc   int
+		track  int
+		folder string
+	}{
+		{`\share\Pink Floyd - The Wall (1979)\CD2\03 Hey You.flac`, 2, 3, "The Wall"},
+		{`\share\The Wall\Disc 1\01 In The Flesh.flac`, 1, 1, "The Wall"},
+		{`\share\The Wall\[Disk-2]\01 Hey You.flac`, 2, 1, "The Wall"},
+		{`\share\The Wall\CD1 - Live\04 Mother.flac`, 1, 4, "The Wall"},
+		// The filename's own disc number is more specific than the folder.
+		{`\share\The Wall\CD1\2-05 Comfortably Numb.flac`, 2, 5, "The Wall"},
+		// Not a disc folder: a number is required.
+		{`\share\CDs\The Wall\01 In The Flesh.flac`, 0, 1, "The Wall"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.path, func(t *testing.T) {
+			t.Parallel()
+
+			got := ParsePath(tc.path)
+			if got.Disc != tc.disc || got.Track != tc.track || got.Folder != tc.folder {
+				t.Errorf(
+					"ParsePath = disc %d track %d folder %q, want %d %d %q",
+					got.Disc, got.Track, got.Folder, tc.disc, tc.track, tc.folder,
+				)
+			}
+		})
+	}
+}
+
+func TestAlbumDir(t *testing.T) {
+	t.Parallel()
+
+	cases := map[string]string{
+		`\share\Album\CD1\01 A.flac`:  "/share/Album",
+		`\share\Album\01 A.flac`:      "/share/Album",
+		`CD1\01 A.flac`:               "CD1",
+		`\share\CD Collection\01.mp3`: "/share/CD Collection",
+	}
+
+	for in, want := range cases {
+		if got := AlbumDir(in); got != want {
+			t.Errorf("AlbumDir(%q) = %q, want %q", in, got, want)
+		}
+	}
+}
+
+// One album shared as CD1/CD2 is one candidate, named after the album.
+func TestSlskdGroupsDiscFoldersIntoOneCandidate(t *testing.T) {
+	t.Parallel()
+
+	stub := newSlskdStub(t)
+	stub.responses = []slskdResponse{{
+		Username: "peer",
+		Files: []slskdFile{
+			{Filename: `\m\The Wall\CD1\01 In The Flesh.flac`, Size: 1},
+			{Filename: `\m\The Wall\CD1\02 The Thin Ice.flac`, Size: 1},
+			{Filename: `\m\The Wall\CD2\01 Hey You.flac`, Size: 1},
+			{Filename: `\m\The Wall\CD2\02 Is There Anybody Out There.flac`, Size: 1},
+		},
+	}}
+
+	s, _ := newStubSlskd(t, stub)
+
+	got, err := s.Search(context.Background(), Download{Query: "the wall"})
+	if err != nil {
+		t.Fatalf("Search: %v", err)
+	}
+
+	if len(got) != 1 {
+		t.Fatalf("got %d candidates, want the two discs as one", len(got))
+	}
+
+	if got[0].Title != "The Wall" || len(got[0].Files) != 4 {
+		t.Errorf(
+			"candidate = %q with %d files, want \"The Wall\" with 4",
+			got[0].Title, len(got[0].Files),
+		)
+	}
+}
+
+// A track search matches one file per folder, so a single-track request
+// must accept a one-file folder that an album request rightly drops.
+func TestSlskdKeepsASingleFileForATrackRequest(t *testing.T) {
+	t.Parallel()
+
+	stub := newSlskdStub(t)
+	stub.responses = []slskdResponse{{
+		Username: "peer",
+		Files: []slskdFile{
+			{Filename: `\m\OK Computer\02 Paranoid Android.flac`, Size: 1},
+		},
+	}}
+
+	s, _ := newStubSlskd(t, stub)
+
+	track, err := s.Search(context.Background(), Download{
+		RecordingMBID: "rec-1", Artist: "Radiohead", Album: "Paranoid Android",
+	})
+	if err != nil {
+		t.Fatalf("Search: %v", err)
+	}
+
+	if len(track) != 1 {
+		t.Errorf("track request: got %d candidates, want 1", len(track))
+	}
+
+	album, err := s.Search(context.Background(), Download{
+		ReleaseMBID: "rel-1", Artist: "Radiohead", Album: "OK Computer",
+	})
+	if err != nil {
+		t.Fatalf("Search: %v", err)
+	}
+
+	if len(album) != 0 {
+		t.Errorf("album request: got %d candidates, want the one-file folder dropped", len(album))
+	}
+}
+
+// Two discs with a file of the same name both reach staging, each under
+// its disc folder, where the importer reads the disc number from.
+func TestSlskdCollectKeepsDiscFolders(t *testing.T) {
+	t.Parallel()
+
+	stub := newSlskdStub(t)
+	s, downloads := newStubSlskd(t, stub)
+
+	for _, disc := range []string{"CD1", "CD2"} {
+		dir := filepath.Join(downloads, disc)
+		if err := os.MkdirAll(dir, 0o750); err != nil {
+			t.Fatalf("mkdir: %v", err)
+		}
+
+		if err := os.WriteFile(
+			filepath.Join(dir, "01 Intro.flac"), []byte(disc), 0o600,
+		); err != nil {
+			t.Fatalf("write: %v", err)
+		}
+	}
+
+	dst := t.TempDir()
+
+	got, err := s.collect(Candidate{Files: []CandidateFile{
+		{Path: `\m\Album\CD1\01 Intro.flac`, IsAudio: true},
+		{Path: `\m\Album\CD2\01 Intro.flac`, IsAudio: true},
+	}}, dst)
+	if err != nil {
+		t.Fatalf("collect: %v", err)
+	}
+
+	if len(got.Files) != 2 {
+		t.Fatalf("collected %d files, want 2", len(got.Files))
+	}
+
+	for _, disc := range []string{"CD1", "CD2"} {
+		data, err := os.ReadFile(filepath.Join(dst, disc, "01 Intro.flac"))
+		if err != nil || string(data) != disc {
+			t.Errorf("%s's file missing or overwritten: %q, %v", disc, data, err)
+		}
+
+		if hint := ParsePath(filepath.Join(dst, disc, "01 Intro.flac")); hint.Disc == 0 {
+			t.Errorf("staged %s file lost its disc number", disc)
+		}
+	}
+}
+
+// A two-disc release whose discs both number from 01 aligns completely
+// once the disc comes from the folder; before, disc 2's 01 collided with
+// disc 1's.
+func TestMultiDiscCandidateAlignsEveryTrack(t *testing.T) {
+	t.Parallel()
+
+	dl := Download{
+		ReleaseMBID: "the-wall",
+		Artist:      "Pink Floyd",
+		Album:       "The Wall",
+		Expected: []ExpectedTrack{
+			{DiscNumber: 1, Position: 1, Title: "In the Flesh?"},
+			{DiscNumber: 1, Position: 2, Title: "The Thin Ice"},
+			{DiscNumber: 2, Position: 1, Title: "Hey You"},
+			{DiscNumber: 2, Position: 2, Title: "Is There Anybody Out There?"},
+		},
+	}
+
+	c := Candidate{
+		Title: "The Wall",
+		Files: []CandidateFile{
+			{Path: `\m\Pink Floyd - The Wall\CD1\01 In the Flesh.flac`, Size: 1},
+			{Path: `\m\Pink Floyd - The Wall\CD1\02 The Thin Ice.flac`, Size: 1},
+			{Path: `\m\Pink Floyd - The Wall\CD2\01 Hey You.flac`, Size: 1},
+			{Path: `\m\Pink Floyd - The Wall\CD2\02 Is There Anybody Out There.flac`, Size: 1},
+		},
+	}
+
+	got := Score(dl, c, 50, AutoDownloadPrefs{})
+
+	if got.Match.Completeness != 1 {
+		t.Errorf("completeness = %f, want 1", got.Match.Completeness)
+	}
+
+	if got.Match.AlbumFit < 0.99 {
+		t.Errorf("album fit = %f, want the album's own name to match", got.Match.AlbumFit)
+	}
+
+	if got.Match.Overall < minMatch {
+		t.Errorf("match = %f, want it to clear the auto-pick bar %f", got.Match.Overall, minMatch)
+	}
+}
+
+// Ten files against a ten-track album is not a complete album when only
+// three of them are its tracks.
+func TestCompletenessCountsTracksNotFiles(t *testing.T) {
+	t.Parallel()
+
+	dl := okComputer()
+
+	c := Candidate{Title: "OK Computer", Files: []CandidateFile{
+		{Path: `\m\Radiohead - OK Computer\Airbag.flac`, Size: 1},
+		{Path: `\m\Radiohead - OK Computer\Paranoid Android.flac`, Size: 1},
+		{Path: `\m\Radiohead - OK Computer\Exit Music (For a Film).flac`, Size: 1},
+		{Path: `\m\Radiohead - OK Computer\Creep.flac`, Size: 1},
+	}}
+
+	got := Score(dl, c, 50, AutoDownloadPrefs{})
+
+	if got.Match.Completeness > 0.76 {
+		t.Errorf(
+			"completeness = %f with 3 of 4 tracks present, want at most 0.75",
+			got.Match.Completeness,
+		)
+	}
+}

backend/download/concurrency_test.go

@@ -47,7 +47,7 @@ func grabAll(
 		go func() {
 			defer wg.Done()
 
-			f.manager.grab(ctx, dl, candidate, nil)
+			f.manager.grab(ctx, dl, candidate, nil, false)
 		}()
 	}
 

backend/download/fallback_test.go

@@ -0,0 +1,261 @@
+package download
+
+import (
+	"context"
+	"errors"
+	"os"
+	"testing"
+)
+
+// A transfer that fails on one copy of an album is not a failed
+// download while another acceptable copy exists.  On Soulseek the usual
+// failure is one peer being offline, with several others offering the
+// same folder.
+
+var errPeerOffline = errors.New("peer went offline")
+
+func TestManagerFallsBackToTheNextCandidate(t *testing.T) {
+	t.Parallel()
+
+	f := newManagerFixture(t)
+
+	// The failing source ranks first on priority, so the fallback is
+	// what reaches the one that works.
+	bad := fakeWithAlbum(1, "offline-peer", ".flac")
+	bad.GrabErr = errPeerOffline
+	good := fakeWithAlbum(2, "online-peer", ".flac")
+
+	f.manager.installProvider(Config{ID: 1, Priority: 90}, bad)
+	f.manager.installProvider(Config{ID: 2, Priority: 10}, good)
+
+	dl := fourTrackDownload()
+
+	if _, err := f.manager.Start(context.Background(), dl); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+
+	waitForDownloadState(t, f.store, dl.ID, StateComplete)
+
+	if bad.GrabCalls != 1 || good.GrabCalls != 1 {
+		t.Errorf(
+			"grabs: failing=%d working=%d, want 1 and 1",
+			bad.GrabCalls, good.GrabCalls,
+		)
+	}
+
+	// The abandoned attempt's staging goes with it; only a request that
+	// fails outright keeps its staging for inspection.
+	waitFor(t, func() bool {
+		entries, err := os.ReadDir(f.staging.Root())
+
+		return err == nil && len(entries) == 0
+	}, "the failed attempt's staging was never released")
+}
+
+// Falling back must not lower the bar.  A second choice outside the
+// user's guardrails is not a choice auto-pick may make, first or second.
+func TestManagerFallbackRespectsTheGuardrails(t *testing.T) {
+	t.Parallel()
+
+	f := newManagerFixture(t)
+	f.manager.SetPreferences(AutoDownloadPrefs{MaxSizeMB: 50})
+
+	bad := fakeWithAlbum(1, "offline-peer", ".flac")
+	bad.GrabErr = errPeerOffline
+	bad.Candidates[0].TotalSize = 40 << 20
+
+	huge := fakeWithAlbum(2, "oversized", ".flac")
+	huge.Candidates[0].TotalSize = 900 << 20
+
+	f.manager.installProvider(Config{ID: 1, Priority: 90}, bad)
+	f.manager.installProvider(Config{ID: 2, Priority: 10}, huge)
+
+	dl := fourTrackDownload()
+
+	if _, err := f.manager.Start(context.Background(), dl); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+
+	waitForDownloadState(t, f.store, dl.ID, StateFailed)
+
+	if huge.GrabCalls != 0 {
+		t.Errorf("fell back to a candidate over the size ceiling")
+	}
+}
+
+// A copy the user picked by hand is the copy they asked for.  Quietly
+// substituting another is a decision they did not make.
+func TestManagerPickDoesNotFallBack(t *testing.T) {
+	t.Parallel()
+
+	f := newManagerFixture(t)
+
+	bad := fakeWithAlbum(1, "offline-peer", ".flac")
+	bad.GrabErr = errPeerOffline
+	good := fakeWithAlbum(2, "online-peer", ".flac")
+
+	f.manager.installProvider(Config{ID: 1, Priority: 90}, bad)
+	f.manager.installProvider(Config{ID: 2, Priority: 10}, good)
+
+	// A ceiling below both copies parks the result set for the user.
+	f.manager.SetPreferences(AutoDownloadPrefs{MaxSizeMB: 1})
+
+	bad.Candidates[0].TotalSize = 30 << 20
+	good.Candidates[0].TotalSize = 30 << 20
+
+	dl := fourTrackDownload()
+
+	if _, err := f.manager.Start(context.Background(), dl); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+
+	if err := f.manager.Pick(
+		context.Background(), dl.ID, "offline-peer-cand",
+	); err != nil {
+		t.Fatalf("Pick: %v", err)
+	}
+
+	waitForDownloadState(t, f.store, dl.ID, StateFailed)
+
+	if good.GrabCalls != 0 {
+		t.Errorf("a hand-picked grab fell back to another candidate")
+	}
+}
+
+// Fallback is for surviving an offline peer or two, not for walking a
+// forty-peer list for six hours.
+func TestManagerFallbackIsBounded(t *testing.T) {
+	t.Parallel()
+
+	f := newManagerFixture(t)
+
+	var providers []*FakeProvider
+
+	for i := int64(1); i <= maxGrabAttempts+2; i++ {
+		p := fakeWithAlbum(i, "peer-"+itoa(int(i)), ".flac")
+		p.GrabErr = errPeerOffline
+
+		f.manager.installProvider(Config{ID: i, Priority: 50}, p)
+		providers = append(providers, p)
+	}
+
+	dl := fourTrackDownload()
+
+	if _, err := f.manager.Start(context.Background(), dl); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+
+	waitForDownloadState(t, f.store, dl.ID, StateFailed)
+
+	grabs := 0
+	for _, p := range providers {
+		grabs += p.GrabCalls
+	}
+
+	if grabs != maxGrabAttempts {
+		t.Errorf("grabs = %d, want %d", grabs, maxGrabAttempts)
+	}
+}
+
+// The veto judges the best candidate *inside* the guardrails, so the
+// grab has to take that one — not the overall best, which may be the
+// very copy the user said not to take unattended.
+func TestManagerAutoPickTakesTheBestEligibleCandidate(t *testing.T) {
+	t.Parallel()
+
+	f := newManagerFixture(t)
+	f.manager.SetPreferences(AutoDownloadPrefs{MaxSizeMB: 50})
+
+	huge := fakeWithAlbum(1, "oversized", ".flac")
+	huge.Candidates[0].TotalSize = 900 << 20
+
+	fits := fakeWithAlbum(2, "fits", ".flac")
+	fits.Candidates[0].TotalSize = 40 << 20
+
+	f.manager.installProvider(Config{ID: 1, Priority: 90}, huge)
+	f.manager.installProvider(Config{ID: 2, Priority: 10}, fits)
+
+	dl := fourTrackDownload()
+
+	ranked, err := f.manager.Start(context.Background(), dl)
+	if err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+
+	if ranked[0].ID != "oversized-cand" {
+		t.Fatalf("fixture: best overall is %s, want the oversized copy", ranked[0].ID)
+	}
+
+	waitForDownloadState(t, f.store, dl.ID, StateComplete)
+
+	if huge.GrabCalls != 0 || fits.GrabCalls != 1 {
+		t.Errorf(
+			"grabs: oversized=%d fits=%d, want 0 and 1",
+			huge.GrabCalls, fits.GrabCalls,
+		)
+	}
+}
+
+// On Soulseek a failure is the peer's, so every folder that peer offered
+// goes with it.  Elsewhere a failure is the release's, and one indexer's
+// other releases are still worth trying.
+func TestRuledOutBy(t *testing.T) {
+	t.Parallel()
+
+	failed := []Candidate{
+		{ID: "slskd:alice:Album", Kind: KindSlskd, ProviderID: 1, Origin: "alice"},
+		{ID: "tracker-1", Kind: KindProwlarr, ProviderID: 2, Origin: "indexer"},
+	}
+
+	cases := []struct {
+		name string
+		c    Candidate
+		want bool
+	}{
+		{
+			name: "the same candidate",
+			c:    Candidate{ID: "tracker-1", Kind: KindProwlarr, ProviderID: 2, Origin: "indexer"},
+			want: true,
+		},
+		{
+			name: "another folder from a failed peer",
+			c: Candidate{
+				ID:         "slskd:alice:Album (2)",
+				Kind:       KindSlskd,
+				ProviderID: 1,
+				Origin:     "alice",
+			},
+			want: true,
+		},
+		{
+			name: "another peer",
+			c:    Candidate{ID: "slskd:bob:Album", Kind: KindSlskd, ProviderID: 1, Origin: "bob"},
+			want: false,
+		},
+		{
+			name: "another release from the same indexer",
+			c:    Candidate{ID: "tracker-2", Kind: KindProwlarr, ProviderID: 2, Origin: "indexer"},
+			want: false,
+		},
+		{
+			name: "a peer of the same name on a different daemon",
+			c: Candidate{
+				ID:         "slskd:alice:Album",
+				Kind:       KindSlskd,
+				ProviderID: 3,
+				Origin:     "alice",
+			},
+			want: false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			if got := ruledOutBy(tc.c, failed); got != tc.want {
+				t.Errorf("ruledOutBy = %v, want %v", got, tc.want)
+			}
+		})
+	}
+}

backend/download/manager.go

@@ -577,12 +577,12 @@ func (m *Manager) Start(
 		))
 	}
 
-	if m.AutoPickable(dl, ranked) {
+	if pick, ok := autoPick(dl, ranked, m.preferences()); ok {
 		if job != nil {
 			job.Logf(jobs.LevelInfo, "Auto-selected best candidate")
 		}
 
-		go m.grab(context.WithoutCancel(ctx), dl, ranked[0], job)
+		go m.grab(context.WithoutCancel(ctx), dl, pick, job, true)
 
 		return ranked, nil
 	}
@@ -622,6 +622,13 @@ func (m *Manager) Attempt(
 		return false, veto, nil
 	}
 
+	pick, ok := autoPick(dl, ranked, m.preferences())
+	if !ok {
+		// Unreachable while autoPick and AutoPickVeto agree; kept so a
+		// future divergence refuses rather than grabbing blind.
+		return false, "no candidate clears the auto-download bar", nil
+	}
+
 	if err := m.store.CreateDownload(ctx, dl); err != nil {
 		return false, "", err
 	}
@@ -643,7 +650,7 @@ func (m *Manager) Attempt(
 		))
 	}
 
-	go m.grab(context.WithoutCancel(ctx), dl, ranked[0], job)
+	go m.grab(context.WithoutCancel(ctx), dl, pick, job, true)
 
 	return true, "", nil
 }
@@ -678,7 +685,7 @@ func (m *Manager) Pick(
 
 	job := m.startJob(dl)
 
-	go m.grab(context.WithoutCancel(ctx), dl, *chosen, job)
+	go m.grab(context.WithoutCancel(ctx), dl, *chosen, job, false)
 
 	return nil
 }
@@ -702,13 +709,20 @@ func (m *Manager) Cancel(ctx context.Context, downloadID string) error {
 	return nil
 }
 
-// grab drives one candidate all the way to the library.  It runs on its
+// grab drives one request all the way to the library.  It runs on its
 // own goroutine and owns the job from here on.
+//
+// When fallback is set and a candidate's transfer fails, the next
+// candidate that auto-pick would itself have accepted is tried in its
+// place (see nextCandidate).  It is set for the two unattended routes
+// and not for a candidate the user picked by hand: they chose that copy,
+// and quietly substituting another is a decision they did not make.
 func (m *Manager) grab(
 	ctx context.Context,
 	dl Download,
 	c Candidate,
 	job *jobs.Handle,
+	fallback bool,
 ) {
 	ctx, cancel := context.WithTimeout(ctx, grabTimeout)
 	defer cancel()
@@ -723,6 +737,82 @@ func (m *Manager) grab(
 		m.actMu.Unlock()
 	}()
 
+	var failed []Candidate
+
+	for {
+		out := m.attemptGrab(ctx, dl, c, job)
+		if out.err == nil {
+			m.finishGrab(ctx, dl, out.item, out.imported, job)
+
+			return
+		}
+
+		failed = append(failed, c)
+
+		next, ok := m.nextCandidate(ctx, dl, failed, out, fallback)
+		if !ok {
+			m.failDownload(ctx, job, dl.ID, out.err)
+
+			return
+		}
+
+		m.logger.Info(
+			"download candidate failed; trying the next",
+			"download", dl.ID,
+			"failed", c.ID,
+			"next", next.ID,
+			"error", out.err,
+		)
+
+		if job != nil {
+			job.Logf(jobs.LevelWarn, fmt.Sprintf(
+				"%s failed (%v); trying %s instead",
+				describeCandidate(c), out.err, describeCandidate(next),
+			))
+		}
+
+		// The failed attempt's staging holds at most a partial folder
+		// nobody is going to import, and the next attempt reserves its
+		// own.  Only the final failure keeps its staging for inspection.
+		if out.item.StagingDir != "" {
+			if err := m.staging.Release(out.item.StagingDir); err != nil {
+				m.logger.Warn("could not release staging dir", "error", err)
+			}
+		}
+
+		c = next
+	}
+}
+
+// maxGrabAttempts bounds how many candidates one request will try.  A
+// popular album can have dozens of peers; the point of falling back is
+// to survive the ordinary one or two that are offline, not to walk the
+// whole list for six hours.
+const maxGrabAttempts = 3
+
+// grabOutcome is how one candidate's attempt ended.
+type grabOutcome struct {
+	item     DownloadItem
+	imported ImportResult
+	err      error
+
+	// retryable reports whether another candidate might succeed where
+	// this one failed: the transfer failed, or delivered too little of
+	// the album.  Anything else — no staging space, no library root, a
+	// tag write failing — would fail the next candidate identically.
+	retryable bool
+}
+
+// attemptGrab takes one candidate through transfer and import.  It
+// records the item's own failure, but not the download's: whether the
+// download has failed is the caller's decision, since another candidate
+// may yet succeed.
+func (m *Manager) attemptGrab(
+	ctx context.Context,
+	dl Download,
+	c Candidate,
+	job *jobs.Handle,
+) grabOutcome {
 	// Who will move the bytes is decided before any slot is taken, so
 	// the transfer waits in its own provider's queue rather than in a
 	// global one.  A delegate takes no slot at all: the transfer is
@@ -731,9 +821,7 @@ func (m *Manager) grab(
 	// work against our budget.
 	plan, err := m.planTransfer(dl, c)
 	if err != nil {
-		m.failDownload(ctx, job, dl.ID, err)
-
-		return
+		return grabOutcome{err: err}
 	}
 
 	if !plan.delegated() {
@@ -743,9 +831,7 @@ func (m *Manager) grab(
 		case provSem <- struct{}{}:
 			defer func() { <-provSem }()
 		case <-ctx.Done():
-			m.failDownload(ctx, job, dl.ID, ctx.Err())
-
-			return
+			return grabOutcome{err: ctx.Err()}
 		}
 
 		globalSem := m.globalSem()
@@ -754,9 +840,7 @@ func (m *Manager) grab(
 		case globalSem <- struct{}{}:
 			defer func() { <-globalSem }()
 		case <-ctx.Done():
-			m.failDownload(ctx, job, dl.ID, ctx.Err())
-
-			return
+			return grabOutcome{err: ctx.Err()}
 		}
 	}
 
@@ -771,24 +855,30 @@ func (m *Manager) grab(
 
 	dir, err := m.staging.Reserve(item.ID)
 	if err != nil {
-		m.failDownload(ctx, job, dl.ID, err)
-
-		return
+		return grabOutcome{err: err}
 	}
 
 	item.StagingDir = dir
 
 	if err := m.store.CreateItem(ctx, item); err != nil {
-		m.failDownload(ctx, job, dl.ID, err)
+		return grabOutcome{item: item, err: err}
+	}
 
-		return
+	fail := func(err error, retryable bool) grabOutcome {
+		if serr := m.store.SetItemState(
+			ctx, item.ID, StateFailed, err.Error(),
+		); serr != nil {
+			m.logger.Warn("could not record item failure", "error", serr)
+		}
+
+		return grabOutcome{item: item, err: err, retryable: retryable}
 	}
 
 	result, err := m.transfer(ctx, dl, item, plan, job)
 	if err != nil {
-		m.failItem(ctx, job, item, dl.ID, err)
-
-		return
+		// A delegate's failure is the external manager's verdict on the
+		// whole request, not on one copy of it.
+		return fail(err, !plan.delegated())
 	}
 
 	m.setStates(ctx, dl.ID, item.ID, StateImporting)
@@ -798,42 +888,116 @@ func (m *Manager) grab(
 		job.SetStages(importStages(2))
 	}
 
-	var imported ImportResult
-
 	if result.Delegated {
 		// The external manager already placed and tagged these files in
 		// its own library.  Moving them out from under a system that is
 		// still managing them would be worse than useless, so the files
 		// are recorded where they are and the library scan picks them
 		// up in place.
-		imported = ImportResult{Paths: result.Files}
-
 		if job != nil {
 			job.Logf(jobs.LevelInfo, fmt.Sprintf(
 				"External manager imported %d files; recording them in place",
 				len(result.Files),
 			))
 		}
-	} else {
-		opts := m.importOptions()
-		opts.WriteTags = true
 
-		opts.LibraryRoot, err = m.library.LibraryPath(dl.LibraryID)
-		if err != nil {
-			m.failItem(ctx, job, item, dl.ID,
-				fmt.Errorf("resolve library root: %w", err))
-
-			return
-		}
-
-		imported, err = m.importer.Import(ctx, dl, result, opts)
-		if err != nil {
-			m.failItem(ctx, job, item, dl.ID, err)
-
-			return
+		return grabOutcome{
+			item:     item,
+			imported: ImportResult{Paths: result.Files},
 		}
 	}
 
+	opts := m.importOptions()
+	opts.WriteTags = true
+
+	opts.LibraryRoot, err = m.library.LibraryPath(dl.LibraryID)
+	if err != nil {
+		return fail(fmt.Errorf("resolve library root: %w", err), false)
+	}
+
+	imported, err := m.importer.Import(ctx, dl, result, opts)
+	if err != nil {
+		return fail(err, errors.Is(err, ErrTooIncomplete))
+	}
+
+	return grabOutcome{item: item, imported: imported}
+}
+
+// nextCandidate picks the candidate to try after the ones in failed.
+//
+// It only ever offers a candidate auto-pick would have taken on its own
+// (autoAcceptable), so falling back cannot lower the bar an unattended
+// download is held to: the second choice has to clear the same gates
+// the first did.
+//
+// On Soulseek a failure belongs to the *peer* — offline, refusing, or
+// holding us in a queue — so every folder that peer offered is skipped
+// with it.  Elsewhere a failure belongs to the release, and only that
+// candidate is.
+func (m *Manager) nextCandidate(
+	ctx context.Context,
+	dl Download,
+	failed []Candidate,
+	out grabOutcome,
+	fallback bool,
+) (Candidate, bool) {
+	if !fallback || !out.retryable || ctx.Err() != nil ||
+		len(failed) >= maxGrabAttempts {
+		return Candidate{}, false
+	}
+
+	m.resMu.RLock()
+	ranked := m.results[dl.ID]
+	m.resMu.RUnlock()
+
+	prefs := m.preferences()
+
+	for _, c := range ranked {
+		if ruledOutBy(c, failed) || !autoAcceptable(dl, c, prefs) {
+			continue
+		}
+
+		return c, true
+	}
+
+	return Candidate{}, false
+}
+
+// ruledOutBy reports whether a failure among failed also rules out c.
+func ruledOutBy(c Candidate, failed []Candidate) bool {
+	for _, f := range failed {
+		if c.ID == f.ID && c.ProviderID == f.ProviderID {
+			return true
+		}
+
+		if c.Kind == KindSlskd && f.Kind == KindSlskd &&
+			c.ProviderID == f.ProviderID && c.Origin != "" &&
+			c.Origin == f.Origin {
+			return true
+		}
+	}
+
+	return false
+}
+
+// describeCandidate names a candidate for the job log.
+func describeCandidate(c Candidate) string {
+	if c.Origin != "" {
+		return fmt.Sprintf("%q from %s", c.Title, c.Origin)
+	}
+
+	return fmt.Sprintf("%q", c.Title)
+}
+
+// finishGrab records a successful import and retires what the request
+// was holding.
+func (m *Manager) finishGrab(
+	ctx context.Context,
+	dl Download,
+	item DownloadItem,
+	imported ImportResult,
+	job *jobs.Handle,
+) {
 	if err := m.store.SetItemImported(
 		ctx, item.ID, imported.Paths,
 	); err != nil {
@@ -1177,23 +1341,6 @@ func (m *Manager) failDownload(
 	}
 }
 
-// failItem records an item-level failure and fails its download.
-func (m *Manager) failItem(
-	ctx context.Context,
-	job *jobs.Handle,
-	item DownloadItem,
-	downloadID string,
-	err error,
-) {
-	if serr := m.store.SetItemState(
-		ctx, item.ID, StateFailed, err.Error(),
-	); serr != nil {
-		m.logger.Warn("could not record item failure", "error", serr)
-	}
-
-	m.failDownload(ctx, job, downloadID, err)
-}
-
 // startJob registers the request in the background jobs panel.
 func (m *Manager) startJob(dl Download) *jobs.Handle {
 	if m.jobsReg == nil {

backend/download/pathmatch.go

@@ -66,6 +66,14 @@ var (
 
 	// separatorPattern splits "Artist - Album" style folder names.
 	separatorPattern = regexp.MustCompile(`\s+[-–—]\s+`)
+
+	// discFolderPattern matches a directory that holds one disc of an
+	// album rather than the album: "CD1", "CD 2", "Disc 3", "Disk-1",
+	// "[Disc 2]", "CD1 - The Early Years".  A number is required, so a
+	// folder merely called "CDs" is not one.
+	discFolderPattern = regexp.MustCompile(
+		`(?i)^\s*[\[(]?\s*(?:cd|disc|disk)\s*[-_.#]?\s*(\d{1,2})\b`,
+	)
 )
 
 // FormatForPath returns the audio format implied by a path's extension,
@@ -94,16 +102,63 @@ type TrackHint struct {
 	Folder string
 }
 
+// discFolder reports whether a directory name is one disc of an album,
+// and which.
+func discFolder(name string) (int, bool) {
+	m := discFolderPattern.FindStringSubmatch(name)
+	if m == nil {
+		return 0, false
+	}
+
+	n, err := strconv.Atoi(m[1])
+	if err != nil || n == 0 {
+		return 0, false
+	}
+
+	return n, true
+}
+
+// AlbumDir is the directory that holds a file's *album*: its parent,
+// or its grandparent when the parent is a disc folder.
+//
+// Multi-disc rips are shared as `Album/CD1/…` and `Album/CD2/…`, and
+// grouping candidates by the immediate parent split one album into two
+// half-albums, each titled "CD1".  Neither could clear the completeness
+// or album-title bars, so a multi-disc release could not be auto-picked
+// at all.  A disc folder at the root has no album above it and is
+// returned as it is.
+func AlbumDir(p string) string {
+	dir := path.Dir(strings.ReplaceAll(p, `\`, "/"))
+
+	if _, ok := discFolder(path.Base(dir)); !ok {
+		return dir
+	}
+
+	parent := path.Dir(dir)
+	if parent == "." || parent == "/" || parent == "" {
+		return dir
+	}
+
+	return parent
+}
+
 // ParsePath extracts what it can from one candidate file path.
 func ParsePath(p string) TrackHint {
 	// Soulseek paths are Windows-style; normalize before splitting.
 	norm := strings.ReplaceAll(p, `\`, "/")
 	base := path.Base(norm)
-	folder := path.Base(path.Dir(norm))
 
 	name := strings.TrimSuffix(base, path.Ext(base))
 
-	hint := TrackHint{Folder: cleanAlbumName(folder)}
+	// The album's name is the album directory's, not a disc folder's,
+	// and the disc folder is where a multi-disc rip says which disc a
+	// file is on.  A disc number in the filename ("2-01 …") is more
+	// specific and overrides it below.
+	hint := TrackHint{Folder: cleanAlbumName(path.Base(AlbumDir(norm)))}
+
+	if disc, ok := discFolder(path.Base(path.Dir(norm))); ok {
+		hint.Disc = disc
+	}
 
 	if m := trackNumPattern.FindStringSubmatch(name); m != nil {
 		if m[1] != "" {

backend/download/provider_slskd.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"net/url"
 	"os"
 	"path"
 	"path/filepath"
@@ -69,12 +70,31 @@ const (
 	slskdTransferPoll = 3 * time.Second
 
 	// slskdMinFiles is the fewest audio files a folder needs before it
-	// is offered as a candidate.  Soulseek returns a lot of one-file
-	// noise for common queries.
+	// is offered as a candidate for an album.  Soulseek returns a lot of
+	// one-file noise for common queries.  A single-track request takes
+	// one (see minFilesFor).
 	slskdMinFiles = 2
 
 	// slskdHTTPTimeout bounds one API call.
 	slskdHTTPTimeout = 20 * time.Second
+
+	// slskdStallAfter is how long a grab may go without a byte arriving
+	// before the peer is given up on.  It is measured from enqueue, so
+	// it covers a peer that queues us and never starts as well as one
+	// that starts and stops.  Ten minutes is long enough for a short
+	// queue ahead of us to clear and short enough that one unresponsive
+	// peer does not hold slskd's single transfer slot for an evening.
+	slskdStallAfter = 10 * time.Minute
+
+	// slskdAbsentGrace is how long a requested file may be missing from
+	// slskd's transfer list before it is counted as failed.  slskd lists
+	// a transfer as soon as it accepts it, so a file still absent after
+	// a few polls was refused.
+	slskdAbsentGrace = 30 * time.Second
+
+	// slskdCancelTimeout bounds the cleanup that cancels abandoned
+	// transfers.
+	slskdCancelTimeout = 15 * time.Second
 )
 
 func init() {
@@ -134,6 +154,8 @@ type slskd struct {
 	searchPoll   time.Duration
 	searchWait   time.Duration
 	transferPoll time.Duration
+	stallAfter   time.Duration
+	absentGrace  time.Duration
 }
 
 // newSlskd builds the provider from config.
@@ -188,6 +210,8 @@ func newSlskd(
 		searchPoll:    slskdSearchPoll,
 		searchWait:    slskdSearchWait,
 		transferPoll:  slskdTransferPoll,
+		stallAfter:    slskdStallAfter,
+		absentGrace:   slskdAbsentGrace,
 	}, nil
 }
 
@@ -307,7 +331,24 @@ func (s *slskd) Search(ctx context.Context, dl Download) ([]Candidate, error) {
 		)
 	}()
 
-	return s.candidatesFrom(search), nil
+	return s.candidatesFrom(search, minFilesFor(dl)), nil
+}
+
+// minFilesFor is the fewest audio files a folder must offer to be a
+// candidate for this request.
+//
+// Soulseek answers a search with the files that match it, not with the
+// folders they sit in.  An album query matches every file in the album's
+// folder, because the folder name carries the terms; a *track* query
+// usually matches one file per folder.  The two-file floor that filters
+// out one-file noise for an album therefore filtered out every result
+// for a track, and a single-track request could never be served here.
+func minFilesFor(dl Download) int {
+	if dl.RecordingMBID != "" {
+		return 1
+	}
+
+	return slskdMinFiles
 }
 
 // awaitSearch polls until the search completes or the budget runs out.
@@ -348,8 +389,9 @@ func (s *slskd) awaitSearch(
 	return last, nil
 }
 
-// candidatesFrom groups a search's responses into candidates.
-func (s *slskd) candidatesFrom(search slskdSearch) []Candidate {
+// candidatesFrom groups a search's responses into candidates, dropping
+// folders with fewer than minFiles audio files.
+func (s *slskd) candidatesFrom(search slskdSearch, minFiles int) []Candidate {
 	out := make([]Candidate, 0, len(search.Responses))
 
 	for _, resp := range search.Responses {
@@ -377,7 +419,7 @@ func (s *slskd) candidatesFrom(search slskdSearch) []Candidate {
 				total += f.Size
 			}
 
-			if audio < slskdMinFiles {
+			if audio < minFiles {
 				continue
 			}
 
@@ -398,13 +440,15 @@ func (s *slskd) candidatesFrom(search slskdSearch) []Candidate {
 	return out
 }
 
-// groupByFolder buckets a peer's files by their containing directory.
+// groupByFolder buckets a peer's files by the album directory they sit
+// in — the containing directory, or the one above it for a disc folder
+// (see AlbumDir), so a multi-disc rip is one candidate and not two.
 func groupByFolder(files []slskdFile) map[string][]slskdFile {
 	out := map[string][]slskdFile{}
 
 	for _, f := range files {
-		norm := strings.ReplaceAll(f.Filename, `\`, "/")
-		out[path.Dir(norm)] = append(out[path.Dir(norm)], f)
+		dir := AlbumDir(f.Filename)
+		out[dir] = append(out[dir], f)
 	}
 
 	return out
@@ -467,6 +511,15 @@ func (s *slskd) Grab(
 		)
 	}
 
+	// slskd keeps finished transfers listed until someone removes them,
+	// and a transfer is matched to the request by filename.  A record
+	// left by an earlier attempt at the same file from the same peer
+	// would otherwise be read as this attempt's answer the moment the
+	// first poll came back — an old failure failing a transfer that has
+	// not started.  So what is already terminal is noted before enqueueing
+	// and ignored after.
+	stale := s.terminalTransferIDs(ctx, username)
+
 	wanted := make([]map[string]any, 0, len(c.Files))
 	for _, f := range c.Files {
 		wanted = append(wanted, map[string]any{
@@ -476,24 +529,69 @@ func (s *slskd) Grab(
 	}
 
 	if err := s.client.post(
-		ctx, "/api/v0/transfers/downloads/"+username, wanted, nil,
+		ctx, slskdDownloadsPath(username), wanted, nil,
 	); err != nil {
 		return Result{}, err
 	}
 
-	if err := s.awaitTransfers(ctx, username, c, onProgress); err != nil {
+	if err := s.awaitTransfers(
+		ctx, username, stale, c, onProgress,
+	); err != nil {
 		return Result{}, err
 	}
 
 	return s.collect(c, dst)
 }
 
+// slskdDownloadsPath is the transfers endpoint for one peer.  Soulseek
+// usernames may contain spaces and punctuation, so the name is escaped
+// rather than spliced into the path.
+func slskdDownloadsPath(username string) string {
+	return "/api/v0/transfers/downloads/" + url.PathEscape(username)
+}
+
+// terminalTransferIDs returns the ids of this peer's transfers that are
+// already finished.  Best effort: slskd answers 404 for a peer it has no
+// transfers with, and any failure here means only that there is nothing
+// to ignore.
+func (s *slskd) terminalTransferIDs(
+	ctx context.Context,
+	username string,
+) map[string]bool {
+	transfers, err := s.transfersFor(ctx, username)
+	if err != nil {
+		return nil
+	}
+
+	out := make(map[string]bool, len(transfers))
+
+	for _, t := range transfers {
+		if finished, _ := t.done(); finished && t.ID != "" {
+			out[t.ID] = true
+		}
+	}
+
+	return out
+}
+
 // awaitTransfers polls until every requested file reaches a terminal
-// state.  Soulseek queues are measured in hours, so the only deadline
-// is the caller's context.
+// state, the transfer stalls, or the caller gives up.
+//
+// Soulseek queues are measured in hours, so there is no deadline on the
+// transfer as a whole — but there is one on *progress*.  slskd's
+// transfer limit is one, so a peer that holds us in its queue without
+// sending a byte is not only failing this download, it is holding every
+// other Soulseek download behind it.  After stallAfter with nothing
+// moving the peer is given up on, and the manager tries another.
+//
+// Whatever way this ends short of every file finishing, the transfers
+// still live in slskd are cancelled there.  Returning without doing so
+// leaves the daemon downloading into its own folder for a request
+// nobody is waiting on any more.
 func (s *slskd) awaitTransfers(
 	ctx context.Context,
 	username string,
+	stale map[string]bool,
 	c Candidate,
 	onProgress ProgressFunc,
 ) error {
@@ -502,9 +600,18 @@ func (s *slskd) awaitTransfers(
 		wanted[f.Path] = true
 	}
 
+	var (
+		started      = time.Now()
+		lastProgress = started
+		lastBytes    int64
+		live         []slskdTransfer
+	)
+
 	for {
 		select {
 		case <-ctx.Done():
+			s.cancelTransfers(username, live)
+
 			return fmt.Errorf("%w: transfer cancelled", ErrSlskdTimeout)
 		case <-time.After(s.transferPoll):
 		}
@@ -512,60 +619,177 @@ func (s *slskd) awaitTransfers(
 		transfers, err := s.transfersFor(ctx, username)
 		if err != nil {
 			// A blip talking to the daemon should not abandon a
-			// transfer that may be hours in.
+			// transfer that may be hours in — but a daemon that stays
+			// away is a stall like any other.
 			s.logger.Debug("slskd transfer poll failed", "error", err)
 
+			if time.Since(lastProgress) >= s.stallAfter {
+				s.cancelTransfers(username, live)
+
+				return fmt.Errorf(
+					"%w: slskd has not answered for %s: %w",
+					ErrSlskdTimeout, s.stallAfter, err,
+				)
+			}
+
 			continue
 		}
 
-		var (
-			done, failed int
-			current      int64
+		tally := tallyTransfers(
+			transfers, wanted, stale,
+			time.Since(started) >= s.absentGrace,
 		)
+		live = tally.live
 
-		for _, t := range transfers {
-			if !wanted[t.Filename] {
-				continue
-			}
-
-			current += t.BytesTransferred
-
-			finished, ok := t.done()
-			if !finished {
-				continue
-			}
-
-			if ok {
-				done++
-			} else {
-				failed++
-			}
+		if tally.bytes > lastBytes {
+			lastBytes = tally.bytes
+			lastProgress = time.Now()
 		}
 
 		if onProgress != nil {
 			onProgress(Progress{
-				Current: current,
+				Current: tally.bytes,
 				Total:   c.TotalSize,
 				Phase: fmt.Sprintf(
-					"Transferring from %s (%d/%d)", username, done, len(wanted),
+					"Transferring from %s (%d/%d)",
+					username, tally.done, len(wanted),
 				),
 			})
 		}
 
-		if done+failed < len(wanted) {
+		if tally.done+tally.failed >= len(wanted) {
+			// Some files failing is normal — a peer goes offline
+			// mid-folder.  Let the importer's completeness check decide
+			// whether what arrived is enough, rather than discarding it
+			// here.
+			if tally.done == 0 {
+				return fmt.Errorf(
+					"%w: all %d files failed",
+					ErrSlskdTransferFailed, tally.failed,
+				)
+			}
+
+			return nil
+		}
+
+		if time.Since(lastProgress) < s.stallAfter {
 			continue
 		}
 
-		// Some files failing is normal — a peer goes offline mid-folder.
-		// Let the importer's completeness check decide whether what
-		// arrived is enough, rather than discarding it here.
-		if done == 0 {
-			return fmt.Errorf(
-				"%w: all %d files failed", ErrSlskdTransferFailed, failed,
+		s.cancelTransfers(username, live)
+
+		// A folder that stalls on its last track is the same shape as
+		// one whose last track failed, and goes forward the same way.
+		if tally.done > 0 {
+			s.logger.Info(
+				"slskd transfer stalled; keeping what arrived",
+				"peer", username,
+				"done", tally.done,
+				"wanted", len(wanted),
 			)
+
+			return nil
 		}
 
-		return nil
+		return fmt.Errorf(
+			"%w: %s sent nothing in %s",
+			ErrSlskdTimeout, username, s.stallAfter,
+		)
+	}
+}
+
+// transferTally is one poll's reading of the files a grab asked for.
+type transferTally struct {
+	done, failed int
+	bytes        int64
+
+	// live are the requested transfers slskd is still working on,
+	// which are what has to be cancelled if the grab is abandoned.
+	live []slskdTransfer
+}
+
+// tallyTransfers reads a peer's transfer list against the files a grab
+// asked for.
+//
+// A requested file slskd does not list at all is one it never accepted
+// — refused at enqueue, or dropped — and it will never reach a terminal
+// state to be counted by.  Once absentExpired, such a file counts as
+// failed, or the grab would wait on it until the six-hour ceiling.
+func tallyTransfers(
+	transfers []slskdTransfer,
+	wanted map[string]bool,
+	stale map[string]bool,
+	absentExpired bool,
+) transferTally {
+	seen := make(map[string]slskdTransfer, len(wanted))
+
+	for _, t := range transfers {
+		if !wanted[t.Filename] || stale[t.ID] {
+			continue
+		}
+
+		seen[t.Filename] = t
+	}
+
+	var out transferTally
+
+	for name := range wanted {
+		t, ok := seen[name]
+		if !ok {
+			if absentExpired {
+				out.failed++
+			}
+
+			continue
+		}
+
+		out.bytes += t.BytesTransferred
+
+		finished, succeeded := t.done()
+
+		switch {
+		case !finished:
+			out.live = append(out.live, t)
+		case succeeded:
+			out.done++
+		default:
+			out.failed++
+		}
+	}
+
+	return out
+}
+
+// cancelTransfers asks slskd to cancel and forget transfers this grab
+// is abandoning.  It runs on a context of its own: the usual reason to
+// be here is that the caller's context has just been cancelled, and a
+// cleanup that inherited it would never be sent.
+func (s *slskd) cancelTransfers(username string, live []slskdTransfer) {
+	if len(live) == 0 {
+		return
+	}
+
+	ctx, cancel := context.WithTimeout(
+		context.Background(), slskdCancelTimeout,
+	)
+	defer cancel()
+
+	for _, t := range live {
+		if t.ID == "" {
+			continue
+		}
+
+		endpoint := slskdDownloadsPath(username) + "/" +
+			url.PathEscape(t.ID) + "?remove=true"
+
+		if err := s.client.delete(ctx, endpoint); err != nil {
+			s.logger.Warn(
+				"could not cancel slskd transfer",
+				"peer", username,
+				"file", t.Filename,
+				"error", err,
+			)
+		}
 	}
 }
 
@@ -581,9 +805,7 @@ func (s *slskd) transfersFor(
 		} `json:"directories"`
 	}
 
-	if err := s.client.get(
-		ctx, "/api/v0/transfers/downloads/"+username, &raw,
-	); err != nil {
+	if err := s.client.get(ctx, slskdDownloadsPath(username), &raw); err != nil {
 		return nil, err
 	}
 
@@ -616,7 +838,13 @@ func (s *slskd) collect(c Candidate, dst string) (Result, error) {
 			continue
 		}
 
+		// A multi-disc candidate keeps its disc folders in staging.
+		// Flattened, disc 2's "01 Intro.flac" overwrites disc 1's, and
+		// the importer loses the folder it reads the disc number from.
 		target := filepath.Join(dst, base)
+		if _, ok := discFolder(folder); ok {
+			target = filepath.Join(dst, folder, base)
+		}
 
 		if err := movePath(src, target); err != nil {
 			return Result{}, fmt.Errorf("collect %s: %w", base, err)

backend/download/provider_slskd_test.go

@@ -31,8 +31,18 @@ type slskdStub struct {
 	transfers [][]slskdTransfer
 	pollCount int
 
+	// before is what the downloads endpoint reports until something is
+	// enqueued: records slskd already held from earlier attempts.
+	before []slskdTransfer
+
 	// enqueued records what was requested for download.
 	enqueued []map[string]any
+	posted   bool
+
+	// paths records the escaped path of every transfers call, and
+	// cancelled the escaped request URI of every DELETE.
+	paths     []string
+	cancelled []string
 
 	// unauthorized makes every call return 401.
 	unauthorized bool
@@ -87,7 +97,12 @@ func newSlskdStub(t *testing.T) *slskdStub {
 			return
 		}
 
-		if r.Method == http.MethodPost {
+		s.mu.Lock()
+		s.paths = append(s.paths, r.URL.EscapedPath())
+		s.mu.Unlock()
+
+		switch r.Method {
+		case http.MethodPost:
 			var body []map[string]any
 
 			if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
@@ -96,15 +111,35 @@ func newSlskdStub(t *testing.T) *slskdStub {
 
 			s.mu.Lock()
 			s.enqueued = body
+			s.posted = true
 			s.mu.Unlock()
 
 			w.WriteHeader(http.StatusCreated)
 
+			return
+		case http.MethodDelete:
+			s.mu.Lock()
+			s.cancelled = append(s.cancelled, r.URL.RequestURI())
+			s.mu.Unlock()
+
+			w.WriteHeader(http.StatusNoContent)
+
 			return
 		}
 
 		s.mu.Lock()
 
+		if !s.posted {
+			before := s.before
+			s.mu.Unlock()
+
+			writeJSON(t, w, map[string]any{
+				"directories": []map[string]any{{"files": before}},
+			})
+
+			return
+		}
+
 		idx := s.pollCount
 		if idx >= len(s.transfers) {
 			idx = len(s.transfers) - 1
@@ -191,6 +226,11 @@ func newStubSlskd(t *testing.T, stub *slskdStub) (*slskd, string) {
 	s.searchWait = 200 * time.Millisecond
 	s.transferPoll = time.Millisecond
 
+	// Long enough that no existing test trips them by accident; the
+	// tests about stalls and absences set their own.
+	s.stallAfter = time.Minute
+	s.absentGrace = time.Minute
+
 	return s, downloads
 }
 
@@ -565,3 +605,293 @@ func TestSlskdRequiresConfiguration(t *testing.T) {
 		})
 	}
 }
+
+// slskdAlbum is a two-file candidate from peer, with the files slskd
+// would have written already in place under downloads.
+func slskdAlbum(t *testing.T, downloads, peer string, arrived ...string) Candidate {
+	t.Helper()
+
+	folder := filepath.Join(downloads, "Album")
+	if err := os.MkdirAll(folder, 0o750); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+
+	for _, name := range arrived {
+		if err := os.WriteFile(
+			filepath.Join(folder, name), []byte("audio"), 0o600,
+		); err != nil {
+			t.Fatalf("write: %v", err)
+		}
+	}
+
+	return Candidate{
+		Files: []CandidateFile{
+			{Path: `\s\Album\01 A.flac`, Size: 500, IsAudio: true},
+			{Path: `\s\Album\02 B.flac`, Size: 500, IsAudio: true},
+		},
+		TotalSize: 1000,
+		Payload:   map[string]string{"username": peer},
+	}
+}
+
+// cancelledURIs returns what the stub was asked to cancel.
+func (s *slskdStub) cancelledURIs() []string {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	return append([]string(nil), s.cancelled...)
+}
+
+// A peer that queues us and never sends a byte is given up on, and the
+// queued transfers are cancelled in slskd rather than left to start
+// hours later for a request nobody is waiting on.
+func TestSlskdGrabGivesUpOnAStalledPeer(t *testing.T) {
+	t.Parallel()
+
+	stub := newSlskdStub(t)
+	stub.transfers = [][]slskdTransfer{{
+		{ID: "t1", Filename: `\s\Album\01 A.flac`, State: "Queued, Remotely"},
+		{ID: "t2", Filename: `\s\Album\02 B.flac`, State: "Queued, Remotely"},
+	}}
+
+	s, downloads := newStubSlskd(t, stub)
+	s.stallAfter = 30 * time.Millisecond
+
+	_, err := s.Grab(
+		context.Background(), slskdAlbum(t, downloads, "peer"), t.TempDir(), nil,
+	)
+	if !errors.Is(err, ErrSlskdTimeout) {
+		t.Fatalf("error = %v, want ErrSlskdTimeout", err)
+	}
+
+	got := stub.cancelledURIs()
+	if len(got) != 2 {
+		t.Fatalf("cancelled %v, want both queued transfers", got)
+	}
+
+	for _, uri := range got {
+		if !strings.HasSuffix(uri, "?remove=true") {
+			t.Errorf("cancel %s does not remove the record", uri)
+		}
+	}
+}
+
+// A folder that stalls on its last track goes forward with what
+// arrived, the same as one whose last track failed; the importer's
+// completeness check decides whether that is enough.
+func TestSlskdGrabKeepsWhatArrivedBeforeAStall(t *testing.T) {
+	t.Parallel()
+
+	stub := newSlskdStub(t)
+	stub.transfers = [][]slskdTransfer{{
+		{
+			ID: "t1", Filename: `\s\Album\01 A.flac`,
+			State: "Completed, Succeeded", BytesTransferred: 500,
+		},
+		{ID: "t2", Filename: `\s\Album\02 B.flac`, State: "Queued, Remotely"},
+	}}
+
+	s, downloads := newStubSlskd(t, stub)
+	s.stallAfter = 30 * time.Millisecond
+
+	got, err := s.Grab(
+		context.Background(),
+		slskdAlbum(t, downloads, "peer", "01 A.flac"),
+		t.TempDir(), nil,
+	)
+	if err != nil {
+		t.Fatalf("Grab: %v", err)
+	}
+
+	if len(got.Files) != 1 {
+		t.Errorf("collected %d files, want the 1 that arrived", len(got.Files))
+	}
+
+	if cancelled := stub.cancelledURIs(); len(cancelled) != 1 ||
+		!strings.Contains(cancelled[0], "/t2") {
+		t.Errorf("cancelled %v, want only the stalled t2", cancelled)
+	}
+}
+
+// Progress is what holds the stall timer off.  A transfer that keeps
+// moving bytes is never abandoned, however long it takes.
+func TestSlskdGrabWaitsOnATransferThatIsMoving(t *testing.T) {
+	t.Parallel()
+
+	stub := newSlskdStub(t)
+
+	for b := int64(1); b <= 100; b++ {
+		stub.transfers = append(stub.transfers, []slskdTransfer{
+			{ID: "t1", Filename: `\s\Album\01 A.flac`, State: "InProgress", BytesTransferred: b},
+			{ID: "t2", Filename: `\s\Album\02 B.flac`, State: "Queued, Remotely"},
+		})
+	}
+
+	stub.transfers = append(stub.transfers, []slskdTransfer{
+		{
+			ID:               "t1",
+			Filename:         `\s\Album\01 A.flac`,
+			State:            "Completed, Succeeded",
+			BytesTransferred: 500,
+		},
+		{
+			ID:               "t2",
+			Filename:         `\s\Album\02 B.flac`,
+			State:            "Completed, Succeeded",
+			BytesTransferred: 500,
+		},
+	})
+
+	s, downloads := newStubSlskd(t, stub)
+	// A hundred polls take several times the stall window; each one
+	// moves a byte.  The window is kept well above one poll so a
+	// descheduled test runner does not read as a stall.
+	s.transferPoll = 5 * time.Millisecond
+	s.stallAfter = 150 * time.Millisecond
+
+	got, err := s.Grab(
+		context.Background(),
+		slskdAlbum(t, downloads, "peer", "01 A.flac", "02 B.flac"),
+		t.TempDir(), nil,
+	)
+	if err != nil {
+		t.Fatalf("Grab: %v", err)
+	}
+
+	if len(got.Files) != 2 {
+		t.Errorf("collected %d files, want 2", len(got.Files))
+	}
+}
+
+// A file slskd never lists was refused at enqueue and will never reach
+// a terminal state.  It counts as failed once the grace period is up,
+// rather than being waited on until the six-hour ceiling.
+func TestSlskdGrabCountsAnUnlistedFileAsFailed(t *testing.T) {
+	t.Parallel()
+
+	stub := newSlskdStub(t)
+	stub.transfers = [][]slskdTransfer{{
+		{
+			ID: "t1", Filename: `\s\Album\01 A.flac`,
+			State: "Completed, Succeeded", BytesTransferred: 500,
+		},
+	}}
+
+	s, downloads := newStubSlskd(t, stub)
+	s.absentGrace = 20 * time.Millisecond
+
+	got, err := s.Grab(
+		context.Background(),
+		slskdAlbum(t, downloads, "peer", "01 A.flac"),
+		t.TempDir(), nil,
+	)
+	if err != nil {
+		t.Fatalf("Grab: %v", err)
+	}
+
+	if len(got.Files) != 1 {
+		t.Errorf("collected %d files, want 1", len(got.Files))
+	}
+}
+
+// A finished record left by an earlier attempt at the same file is not
+// this attempt's answer.  Without the snapshot it would fail the grab on
+// the first poll, before the new transfer had started.
+func TestSlskdGrabIgnoresAnEarlierAttemptsRecord(t *testing.T) {
+	t.Parallel()
+
+	stale := slskdTransfer{
+		ID: "old", Filename: `\s\Album\01 A.flac`, State: "Completed, Errored",
+	}
+
+	stub := newSlskdStub(t)
+	stub.before = []slskdTransfer{stale}
+	stub.transfers = [][]slskdTransfer{
+		{stale},
+		{
+			stale,
+			{
+				ID: "new", Filename: `\s\Album\01 A.flac`,
+				State: "Completed, Succeeded", BytesTransferred: 500,
+			},
+		},
+	}
+
+	s, downloads := newStubSlskd(t, stub)
+
+	c := slskdAlbum(t, downloads, "peer", "01 A.flac")
+	c.Files = c.Files[:1]
+
+	got, err := s.Grab(context.Background(), c, t.TempDir(), nil)
+	if err != nil {
+		t.Fatalf("Grab: %v", err)
+	}
+
+	if len(got.Files) != 1 {
+		t.Errorf("collected %d files, want 1", len(got.Files))
+	}
+}
+
+// Cancelling the download cancels the transfer in slskd too.  The
+// cleanup must not inherit the cancelled context, or it is never sent.
+func TestSlskdGrabCancelsTransfersWhenTheCallerGivesUp(t *testing.T) {
+	t.Parallel()
+
+	stub := newSlskdStub(t)
+	stub.transfers = [][]slskdTransfer{{
+		{ID: "t1", Filename: `\s\Album\01 A.flac`, State: "InProgress", BytesTransferred: 10},
+		{ID: "t2", Filename: `\s\Album\02 B.flac`, State: "Queued, Remotely"},
+	}}
+
+	s, downloads := newStubSlskd(t, stub)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
+	defer cancel()
+
+	_, err := s.Grab(ctx, slskdAlbum(t, downloads, "peer"), t.TempDir(), nil)
+	if !errors.Is(err, ErrSlskdTimeout) {
+		t.Fatalf("error = %v, want ErrSlskdTimeout", err)
+	}
+
+	if got := stub.cancelledURIs(); len(got) != 2 {
+		t.Errorf("cancelled %v, want both live transfers", got)
+	}
+}
+
+// Soulseek usernames carry spaces and punctuation; spliced raw into the
+// path, a name with a slash addresses a different endpoint entirely.
+func TestSlskdEscapesTheUsername(t *testing.T) {
+	t.Parallel()
+
+	stub := newSlskdStub(t)
+	stub.transfers = [][]slskdTransfer{{
+		{
+			ID: "t1", Filename: `\s\Album\01 A.flac`,
+			State: "Completed, Succeeded", BytesTransferred: 500,
+		},
+		{
+			ID: "t2", Filename: `\s\Album\02 B.flac`,
+			State: "Completed, Succeeded", BytesTransferred: 500,
+		},
+	}}
+
+	s, downloads := newStubSlskd(t, stub)
+
+	if _, err := s.Grab(
+		context.Background(),
+		slskdAlbum(t, downloads, "dj a/b", "01 A.flac", "02 B.flac"),
+		t.TempDir(), nil,
+	); err != nil {
+		t.Fatalf("Grab: %v", err)
+	}
+
+	stub.mu.Lock()
+	paths := append([]string(nil), stub.paths...)
+	stub.mu.Unlock()
+
+	for _, p := range paths {
+		if p != "/api/v0/transfers/downloads/dj%20a%2Fb" {
+			t.Errorf("transfers call went to %s", p)
+		}
+	}
+}

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(
-		path, []byte("#!/bin/sh\n"+script), 0o700,
-	); err != nil {
+	syscall.ForkLock.Lock()
+	err := os.WriteFile(path, []byte("#!/bin/sh\n"+script), 0o700)
+	syscall.ForkLock.Unlock()
+
+	if err != nil {
 		t.Fatalf("write stub: %v", err)
 	}
 

backend/download/rank.go

@@ -347,7 +347,9 @@ func scoreMatch(
 		TitleFit: titleFit,
 	}
 
-	m.Completeness = completeness(len(audio), len(dl.Expected))
+	m.Completeness = completeness(
+		alignedCount(c.Files), len(audio), len(dl.Expected),
+	)
 
 	// The candidate's own title, and the folder its files sit in, are
 	// two independent guesses at the album name.  Take the better one:
@@ -415,30 +417,54 @@ func artistFit(want string, c Candidate) float64 {
 	return best
 }
 
-// completeness scores audio file count against the expected track
-// count.  Extra files are penalized far more gently than missing ones:
+// completeness scores how much of the expected tracklist a candidate
+// covers.  Extra files are penalized far more gently than missing ones:
 // a folder with bonus tracks or a stray intro is still the album, while
 // a folder missing half the tracks is not.
-func completeness(got, want int) float64 {
+//
+// **Coverage is counted in aligned tracks, not in files.**  It used to
+// be the audio file count, so any ten files scored full marks against
+// a ten-track album whether or not they were its tracks — and since
+// title fit is the mean over the files that *did* align, a folder where
+// three titles matched read as a near-perfect candidate on both counts.
+// `aligned` is how many files matchFiles assigned to an expected track;
+// `audio` still sets the penalty for extras, because a folder of thirty
+// files holding the ten wanted is a worse copy than one holding ten.
+func completeness(aligned, audio, want int) float64 {
 	if want == 0 {
-		if got > 0 {
+		if audio > 0 {
 			return 0.5
 		}
 
 		return 0
 	}
 
-	if got == 0 {
+	if aligned == 0 {
 		return 0
 	}
 
-	if got >= want {
-		extra := float64(got-want) / float64(want)
+	cover := float64(min(aligned, want)) / float64(want)
 
-		return math.Max(0.75, 1.0-0.25*extra)
+	if audio > want {
+		extra := float64(audio-want) / float64(want)
+		cover *= math.Max(0.75, 1.0-0.25*extra)
 	}
 
-	return float64(got) / float64(want)
+	return cover
+}
+
+// alignedCount is how many audio files were assigned to an expected
+// track.
+func alignedCount(files []CandidateFile) int {
+	n := 0
+
+	for _, f := range files {
+		if f.IsAudio && f.MatchedTo != 0 {
+			n++
+		}
+	}
+
+	return n
 }
 
 // scoreQuality answers whether this is a good copy.
@@ -735,6 +761,45 @@ func AutoPickVeto(
 	return ""
 }
 
+// autoAcceptable reports whether auto-pick may take this one candidate
+// without asking: the request is anchored to a tracklist, and the
+// candidate is inside the user's guardrails and clears the match and
+// quality bars.  It is AutoPickVeto's test applied to a single
+// candidate, which is what falling back to a second choice needs.
+func autoAcceptable(dl Download, c Candidate, prefs AutoDownloadPrefs) bool {
+	return dl.Anchored() &&
+		len(dl.Expected) > 0 &&
+		prefs.eligible(c, dl.runtimeMillis()) &&
+		c.Match.Overall >= minMatch &&
+		c.Quality.Overall >= minQuality
+}
+
+// autoPick returns the candidate auto-pick takes: the best-ranked one
+// it may take at all.
+//
+// That is not `ranked[0]`.  AutoPickVeto judges the best candidate
+// *inside* the guardrails, so when the overall best is outside them —
+// over the size ceiling, say — the veto passes on the strength of the
+// second, and grabbing the first would download exactly the copy the
+// user said not to take unattended.
+func autoPick(
+	dl Download,
+	ranked []Candidate,
+	prefs AutoDownloadPrefs,
+) (Candidate, bool) {
+	if AutoPickVeto(dl, ranked, prefs) != "" {
+		return Candidate{}, false
+	}
+
+	for _, c := range ranked {
+		if autoAcceptable(dl, c, prefs) {
+			return c, true
+		}
+	}
+
+	return Candidate{}, false
+}
+
 // mergeMatched copies MatchedTo assignments from the audio-only slice
 // back onto the full file list.
 func mergeMatched(all, matched []CandidateFile) []CandidateFile {

backend/download/rank_test.go

@@ -282,28 +282,32 @@ func TestCompleteness(t *testing.T) {
 
 	tests := []struct {
 		name     string
-		got      int
+		aligned  int
+		audio    int
 		want     int
 		minScore float64
 		maxScore float64
 	}{
-		{"exact", 10, 10, 1.0, 1.0},
-		{"half missing", 5, 10, 0.49, 0.51},
-		{"one bonus track", 11, 10, 0.95, 1.0},
-		{"double", 20, 10, 0.74, 0.76},
-		{"nothing", 0, 10, 0, 0},
-		{"no expectation", 5, 0, 0.5, 0.5},
+		{"exact", 10, 10, 10, 1.0, 1.0},
+		{"half missing", 5, 5, 10, 0.49, 0.51},
+		{"one bonus track", 10, 11, 10, 0.95, 1.0},
+		{"double", 10, 20, 10, 0.74, 0.76},
+		{"nothing", 0, 0, 10, 0, 0},
+		{"no expectation", 0, 5, 0, 0.5, 0.5},
+		// Ten files are not ten tracks: three that align are three.
+		{"right count, wrong tracks", 3, 10, 10, 0.29, 0.31},
+		{"files that align to nothing", 0, 10, 10, 0, 0},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			got := completeness(tt.got, tt.want)
+			got := completeness(tt.aligned, tt.audio, tt.want)
 			if got < tt.minScore || got > tt.maxScore {
 				t.Errorf(
-					"completeness(%d, %d) = %f, want in [%f, %f]",
-					tt.got, tt.want, got, tt.minScore, tt.maxScore,
+					"completeness(%d, %d, %d) = %f, want in [%f, %f]",
+					tt.aligned, tt.audio, tt.want, got, tt.minScore, tt.maxScore,
 				)
 			}
 		})

backend/library/coverart.go

@@ -3,6 +3,7 @@ package library
 import (
 	"bytes"
 	"crypto/sha256"
+	"database/sql"
 	"encoding/hex"
 	"fmt"
 	"image"
@@ -69,6 +70,77 @@ func CoverArtFileSet(coverPath string) []string {
 	return paths
 }
 
+// sweepOrphanedCoverArt deletes the cover_art rows no album references
+// and returns their file paths, for the caller to remove from disk
+// after the transaction commits.  Cover art is referenced only by
+// albums.cover_art_id, so an orphan is a cover whose album is gone —
+// which is every album the caller just swept.
+//
+// One implementation because the scan path, RemoveFromLibrary and
+// RemoveLibrary all reach this state, and the scan side used to skip it
+// entirely while RemoveLibrary did it inline (#247).
+func (l *Library) sweepOrphanedCoverArt(tx *sql.Tx) ([]string, error) {
+	const orphanSQL = `
+		SELECT file_path FROM cover_art WHERE id NOT IN (
+			SELECT DISTINCT cover_art_id FROM albums
+			WHERE cover_art_id IS NOT NULL
+		)`
+
+	rows, err := tx.QueryContext(l.ctx, orphanSQL)
+	if err != nil {
+		return nil, fmt.Errorf("could not query orphaned cover art: %w", err)
+	}
+
+	var paths []string
+
+	for rows.Next() {
+		var filePath string
+
+		if err := rows.Scan(&filePath); err != nil {
+			l.logger.Warn("could not scan cover art path", "err", err)
+
+			continue
+		}
+
+		paths = append(paths, filePath)
+	}
+
+	// Close before the DELETE: the two run on the one writer connection.
+	if err := rows.Close(); err != nil {
+		l.logger.Warn("could not close cover art rows", "err", err)
+	}
+
+	if len(paths) > 0 {
+		if _, err := tx.ExecContext(l.ctx, `
+			DELETE FROM cover_art WHERE id NOT IN (
+				SELECT DISTINCT cover_art_id FROM albums
+				WHERE cover_art_id IS NOT NULL
+			)`); err != nil {
+			return nil, fmt.Errorf("could not delete orphaned cover_art: %w", err)
+		}
+	}
+
+	return paths, nil
+}
+
+// removeCoverArtFiles removes a cover original and its derived size
+// variants.  Only the original is stored in cover_art.file_path; the
+// _sm/_md/_lg tiers are derived filenames beside it, so they have to be
+// removed by name or they accumulate forever.
+func (l *Library) removeCoverArtFiles(coverPaths []string) {
+	for _, coverPath := range coverPaths {
+		for _, path := range CoverArtFileSet(coverPath) {
+			if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
+				l.logger.Warn(
+					"could not remove orphaned cover art file",
+					"path", path,
+					"err", err,
+				)
+			}
+		}
+	}
+}
+
 // saveCoverArt saves embedded cover art to the cache directory.
 // Returns the file path where the art was saved, or empty string
 // if no picture data.  Timing is recorded in the provided metrics.

backend/library/coverart_storage_test.go

@@ -8,6 +8,7 @@ import (
 
 	"yellowjacket/backend/coverart"
 	"yellowjacket/backend/database/sql/sqlcgen"
+	"yellowjacket/internal/testfixtures"
 )
 
 // TestScan_StoresOnlyCoverTiers pins the size decision: a scan writes
@@ -26,10 +27,9 @@ func TestScan_StoresOnlyCoverTiers(t *testing.T) {
 
 	lib, db := setupTestLibrary(t)
 
-	root, err := filepath.Abs("../../test_data/music_library_test")
-	if err != nil {
-		t.Fatalf("resolve fixture path: %v", err)
-	}
+	// Load skips when the fixture library has not been generated, as
+	// every other fixture test does.
+	root := testfixtures.Load(t).Root()
 
 	library, err := db.Queries.CreateLibrary(lib.ctx, sqlcgen.CreateLibraryParams{
 		Name: "Fixtures",

backend/library/crud.go

@@ -320,43 +320,12 @@ func (l *Library) RemoveLibrary(id int64) (*RemovalSummary, error) {
 
 	genresRemoved, _ := result.RowsAffected()
 
-	// 15. Collect orphaned cover_art file paths for post-commit cleanup.
-	// SAFETY: Hand-crafted SELECT for orphaned cover art identification.
-	// Parameterless.
-	rows, err := tx.QueryContext(l.ctx,
-		`SELECT file_path FROM cover_art WHERE id NOT IN (
-			SELECT DISTINCT cover_art_id FROM albums
-			WHERE cover_art_id IS NOT NULL
-		)`)
+	// Collect and delete orphaned cover_art rows before the commit.  The
+	// shared helper is the one place this sweep lives, so the scan path,
+	// RemoveFromLibrary and this removal cannot drift (#247).
+	orphanedCoverArtPaths, err := l.sweepOrphanedCoverArt(tx)
 	if err != nil {
-		return nil, fmt.Errorf("could not query orphaned cover art: %w", err)
-	}
-
-	var orphanedCoverArtPaths []string
-
-	for rows.Next() {
-		var filePath string
-		if err := rows.Scan(&filePath); err != nil {
-			l.logger.Warn("could not scan cover art path", "err", err)
-
-			continue
-		}
-
-		orphanedCoverArtPaths = append(orphanedCoverArtPaths, filePath)
-	}
-
-	if err := rows.Close(); err != nil {
-		l.logger.Warn("could not close cover art rows", "err", err)
-	}
-
-	// 16. Delete orphaned cover_art rows.
-	// SAFETY: Hand-crafted orphan cleanup SQL. Parameterless.
-	if _, err := tx.ExecContext(l.ctx,
-		`DELETE FROM cover_art WHERE id NOT IN (
-			SELECT DISTINCT cover_art_id FROM albums
-			WHERE cover_art_id IS NOT NULL
-		)`); err != nil {
-		return nil, fmt.Errorf("could not delete orphaned cover_art: %w", err)
+		return nil, err
 	}
 
 	// 17. Delete the library's tagging queue.  tagging_items holds a
@@ -392,20 +361,9 @@ func (l *Library) RemoveLibrary(id int64) (*RemovalSummary, error) {
 	// avoids a costly full re-index of all remaining tracks (~10s for
 	// 25K tracks).
 
-	// 21. Post-commit: Delete orphaned cover art files and their sized
-	// variants.  Only the original is stored in cover_art.file_path; the
-	// _sm/_md/_lg thumbnails are derived filenames beside it, so they
-	// have to be removed by name or they accumulate forever.
-	for _, coverPath := range orphanedCoverArtPaths {
-		for _, path := range CoverArtFileSet(coverPath) {
-			if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
-				l.logger.Warn("could not remove orphaned cover art file",
-					"path", path,
-					"err", err,
-				)
-			}
-		}
-	}
+	// Post-commit: remove the orphaned cover art files and their sized
+	// variants.
+	l.removeCoverArtFiles(orphanedCoverArtPaths)
 
 	// 22. Post-commit: Compact queue.
 	if l.removalHooks.CompactQueue != nil {

backend/library/library.go

@@ -911,30 +911,96 @@ func (l *Library) scanInternal(
 
 		orphanStart := time.Now()
 
+		// Snapshot the orphan set first.  The playlist-phantom
+		// preservation and the deletes are one transaction (the
+		// preservation has to land before the ON DELETE SET NULL, and
+		// both have to succeed or neither does), and the ids are what
+		// scope that preservation to just these files instead of
+		// rewriting every playlist row on a routine scan.
+		var (
+			orphans     []sqlcgen.AudioFile
+			orphanPaths []string
+		)
+
 		existingPaths.Range(func(key, value any) bool {
-			path := key.(string)
-			audioFile := value.(sqlcgen.AudioFile)
+			orphanPaths = append(orphanPaths, key.(string))
+			orphans = append(orphans, value.(sqlcgen.AudioFile))
 
-			l.logger.Debug(
-				"removing orphaned database entry",
-				"path", path, "id", audioFile.ID,
-			)
+			return true
+		})
 
-			if err := l.db.Queries.DeleteAudioFile(
-				l.ctx, audioFile.ID,
-			); err != nil {
-				l.logger.Warn(
-					"failed to delete orphaned audio file",
-					"path", path,
-					"id", audioFile.ID,
-					"err", err,
-				)
+		deleted := make([]bool, len(orphans))
 
-				metrics.addWarning(path, "orphan", err)
-
-				return true
+		if len(orphans) > 0 {
+			orphanIDs := make([]int64, len(orphans))
+			for i, f := range orphans {
+				orphanIDs[i] = f.ID
 			}
 
+			tx, beginErr := l.db.BeginTx()
+			if beginErr != nil {
+				metrics.addWarning("", "orphan", beginErr)
+			} else {
+				defer func() { _ = tx.Rollback() }() // no-op after commit
+
+				if err := database.PreservePlaylistPhantomsForFiles(
+					l.ctx, tx, orphanIDs, l.logger,
+				); err != nil {
+					// Deleting without the phantoms is exactly the
+					// playlist-emptying bug the preservation exists to
+					// prevent, so leave the rows for the next scan
+					// rather than empty the playlists now.
+					l.logger.Error(
+						"skipping orphan deletion: could not preserve "+
+							"playlist entries",
+						"err", err,
+					)
+
+					metrics.addWarning("", "orphan", err)
+
+					_ = tx.Rollback()
+				} else {
+					txq := l.db.Queries.WithTx(tx)
+
+					for i, f := range orphans {
+						if err := txq.DeleteAudioFile(
+							l.ctx, f.ID,
+						); err != nil {
+							l.logger.Warn(
+								"failed to delete orphaned audio file",
+								"path", orphanPaths[i],
+								"id", f.ID,
+								"err", err,
+							)
+
+							metrics.addWarning(orphanPaths[i], "orphan", err)
+
+							continue
+						}
+
+						deleted[i] = true
+					}
+
+					if err := tx.Commit(); err != nil {
+						l.logger.Error(
+							"could not commit orphan deletion",
+							"err", err,
+						)
+
+						metrics.addWarning("", "orphan", err)
+					}
+				}
+			}
+		}
+
+		// Post-commit bookkeeping for the files that actually went.
+		for i, f := range orphans {
+			if !deleted[i] {
+				continue
+			}
+
+			path := orphanPaths[i]
+
 			// Keep the file's tagging group in sync: drop the group's
 			// track count and clear it out once empty, mirroring the
 			// bookkeeping maybeRebindTaggingGroup does for a group_key
@@ -942,25 +1008,25 @@ func (l *Library) scanInternal(
 			// and replaced leaves a stale tagging_items row behind —
 			// its track_count still counts the deleted files, and it
 			// never clears from the autotag queue.
-			if audioFile.GroupKey != "" {
+			if f.GroupKey != "" {
 				if err := l.db.Queries.DecrementTaggingItemTrackCount(
-					l.ctx, audioFile.GroupKey,
+					l.ctx, f.GroupKey,
 				); err != nil {
 					l.logger.Warn(
 						"failed to decrement tagging group for orphan",
 						"path", path,
-						"group_key", audioFile.GroupKey,
+						"group_key", f.GroupKey,
 						"err", err,
 					)
 
 					metrics.addWarning(path, "orphan", err)
 				} else if err := l.db.Queries.DeleteTaggingItemIfEmpty(
-					l.ctx, audioFile.GroupKey,
+					l.ctx, f.GroupKey,
 				); err != nil {
 					l.logger.Warn(
 						"failed to clean up emptied tagging group for orphan",
 						"path", path,
-						"group_key", audioFile.GroupKey,
+						"group_key", f.GroupKey,
 						"err", err,
 					)
 
@@ -968,13 +1034,21 @@ func (l *Library) scanInternal(
 				}
 			}
 
-			// Remove from FTS5 search index.
-			if err := l.db.DeleteSearchIndex(
-				audioFile.ID,
-			); err != nil {
+			// Remove from FTS5 search index and the lyrics index.
+			if err := l.db.DeleteSearchIndex(f.ID); err != nil {
 				l.logger.Warn(
 					"failed to delete FTS entry for orphan",
-					"id", audioFile.ID,
+					"id", f.ID,
+					"err", err,
+				)
+
+				metrics.addWarning(path, "orphan", err)
+			}
+
+			if err := l.db.DeleteLyricsIndex(f.ID); err != nil {
+				l.logger.Warn(
+					"failed to delete lyrics index entry for orphan",
+					"id", f.ID,
 					"err", err,
 				)
 
@@ -982,9 +1056,7 @@ func (l *Library) scanInternal(
 			}
 
 			removed.Add(1)
-
-			return true
-		})
+		}
 
 		metrics.OrphanCleanup = time.Since(orphanStart)
 
@@ -1193,17 +1265,32 @@ func (l *Library) pruneEmptyEntities() {
 		}
 	}
 
+	// Cover art after albums: a cover whose album just went is
+	// unreferenced, and leaving the row behind keeps its files exempt
+	// from the janitor's covers sweep forever (#247).
+	orphanedCovers, err := l.sweepOrphanedCoverArt(tx)
+	if err != nil {
+		l.logger.Warn("could not sweep orphaned cover art", "err", err)
+
+		return
+	}
+
 	if err := tx.Commit(); err != nil {
 		l.logger.Warn("could not commit entity cleanup", "err", err)
 
 		return
 	}
 
-	if len(albumIDs) > 0 || len(artistIDs) > 0 || len(genreIDs) > 0 {
+	// Post-commit: the rows are gone, so their files can go too.
+	l.removeCoverArtFiles(orphanedCovers)
+
+	if len(albumIDs) > 0 || len(artistIDs) > 0 || len(genreIDs) > 0 ||
+		len(orphanedCovers) > 0 {
 		l.logger.Info("pruned empty library entities",
 			"albums", len(albumIDs),
 			"artists", len(artistIDs),
 			"genres", len(genreIDs),
+			"covers", len(orphanedCovers),
 		)
 	}
 }

backend/library/removal_test.go

@@ -94,6 +94,57 @@ func countRows(
 	return n
 }
 
+// RemoveFromLibrary is the one path that empties a track *deliberately*:
+// the file stays on disk but is excluded, so nothing re-imports it.  The
+// playlist entry must still survive as a re-linkable phantom rather than
+// an empty row, because a later full rescan clears the exclusion and is
+// what re-links the entry then (#246).
+func TestRemoveFromLibrary_PreservesPlaylistPhantoms(t *testing.T) {
+	t.Parallel()
+
+	lib, db := setupTestLibrary(t)
+
+	seedRemovableLibrary(t, lib, "/nonexistent/cover.jpg")
+
+	if _, err := lib.db.ExecContext(
+		`INSERT INTO playlists (name) VALUES ('keepme')`,
+	); err != nil {
+		t.Fatalf("seed playlist: %v", err)
+	}
+
+	playlistID := queryInt(
+		t, db, `SELECT id FROM playlists WHERE name = 'keepme'`,
+	)
+
+	if _, err := lib.db.ExecContext(
+		`INSERT INTO playlist_tracks (playlist_id, audio_file_id, position)
+		 SELECT ?, id, 0 FROM audio_files
+		 WHERE file_path = '/music/song.mp3'`,
+		playlistID,
+	); err != nil {
+		t.Fatalf("seed playlist_tracks: %v", err)
+	}
+
+	if _, err := lib.RemoveFromLibrary([]string{"/music/song.mp3"}); err != nil {
+		t.Fatalf("RemoveFromLibrary: %v", err)
+	}
+
+	phantomPath := queryString(
+		t, db,
+		`SELECT phantom_file_path FROM playlist_tracks
+		 WHERE playlist_id = ?`,
+		playlistID,
+	)
+
+	if phantomPath != "/music/song.mp3" {
+		t.Fatalf(
+			"phantom_file_path is %q, want %q -- the entry cannot be "+
+				"re-linked after a later full rescan",
+			phantomPath, "/music/song.mp3",
+		)
+	}
+}
+
 // A library with tagging_items must still be removable.  tagging_items
 // FK-references libraries with no ON DELETE clause, so leaving those
 // rows behind fails the DELETE and rolls back the entire removal.
@@ -198,3 +249,53 @@ func TestCoverArtFileSet(t *testing.T) {
 		}
 	}
 }
+
+// Removing the last track of an album must take the album's cover art
+// with it — both the row and every derived file — or the row keeps its
+// files exempt from the janitor's covers sweep forever (#247).
+func TestRemoveFromLibrary_DeletesOrphanedCoverArt(t *testing.T) {
+	t.Parallel()
+
+	lib, _ := setupTestLibrary(t)
+
+	dir := t.TempDir()
+
+	// The largest tier is what cover_art.file_path names; write every
+	// variant so the sweep has a real set to remove.
+	for _, tier := range thumbnailTiers {
+		p := filepath.Join(dir, coverart.SizedFilename("abc123.jpg", tier.Suffix))
+		if err := os.WriteFile(p, []byte("img"), 0o600); err != nil {
+			t.Fatalf("write %s: %v", p, err)
+		}
+	}
+
+	cover := filepath.Join(dir, coverart.SizedFilename("abc123.jpg", "_lg"))
+
+	seedRemovableLibrary(t, lib, cover)
+
+	// Link the album to the cover so it is not orphaned until the track
+	// (and with it the album) goes.
+	if _, err := lib.db.ExecContext(
+		`UPDATE albums SET cover_art_id =
+			(SELECT id FROM cover_art WHERE file_path = ?)
+		 WHERE name = 'Test Album'`,
+		cover,
+	); err != nil {
+		t.Fatalf("link cover art: %v", err)
+	}
+
+	if _, err := lib.RemoveFromLibrary([]string{"/music/song.mp3"}); err != nil {
+		t.Fatalf("RemoveFromLibrary: %v", err)
+	}
+
+	if n := countRows(t, lib, "cover_art"); n != 0 {
+		t.Errorf("cover_art has %d rows after removal, want 0", n)
+	}
+
+	for _, tier := range thumbnailTiers {
+		p := filepath.Join(dir, coverart.SizedFilename("abc123.jpg", tier.Suffix))
+		if _, err := os.Stat(p); !os.IsNotExist(err) {
+			t.Errorf("cover art file still present: %s", filepath.Base(p))
+		}
+	}
+}

backend/library/remove_tracks.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 
+	"yellowjacket/backend/database"
 	"yellowjacket/backend/events"
 )
 
@@ -57,6 +58,25 @@ func (l *Library) RemoveFromLibrary(filePaths []string) (*RemovalResult, error)
 
 	var result RemovalResult
 
+	// Preserve the playlist entries before the rows go, so they survive
+	// as re-linkable phantoms rather than empty rows.  The track is
+	// excluded and will not be re-imported on its own, but a later full
+	// rescan clears the exclusion and this is what lets the entry
+	// re-link then — the same preservation every other path that empties
+	// audio_files performs (#246).
+	rowIDs := make([]int64, len(rows))
+	for i, row := range rows {
+		rowIDs[i] = row.ID
+	}
+
+	if err := database.PreservePlaylistPhantomsForFiles(
+		l.ctx, tx, rowIDs, l.logger,
+	); err != nil {
+		return nil, fmt.Errorf(
+			"could not preserve playlist entries for removal: %w", err,
+		)
+	}
+
 	// Exclude every path the caller named, including one whose row has
 	// already gone: the user asked for that file to stay out, and a row
 	// that disappeared between the click and the commit is not a reason
@@ -127,6 +147,11 @@ func (l *Library) RemoveFromLibrary(filePaths []string) (*RemovalResult, error)
 			l.logger.Warn("could not delete FTS entry for removed track",
 				"path", row.FilePath, "id", row.ID, "err", err)
 		}
+
+		if err := l.db.DeleteLyricsIndex(row.ID); err != nil {
+			l.logger.Warn("could not delete lyrics index entry for removed track",
+				"path", row.FilePath, "id", row.ID, "err", err)
+		}
 	}
 
 	// Deleting an audio_files row cascades to queue_tracks, so the

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
-		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_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,
-		)
+	//
+	// Shared with the stale-shape retire in backend/database, which is
+	// the other path that empties this table and which did not do this
+	// (#183): the statement lives there so the two cannot drift again.
+	if err := database.PreservePlaylistPhantoms(l.ctx, tx, l.logger); err != nil {
+		return err
 	}
 
 	// Phase 2: the files.  file_genres cascades with them.

backend/library/scan_dirdisc_test.go

@@ -231,3 +231,98 @@ func TestScan_MultipleDirectoriesDoNotCrossContaminate(t *testing.T) {
 		t.Errorf("Album A and Album B must not share a group_key: %+v", keys)
 	}
 }
+
+// TestScan_OrphanCleanupPreservesPlaylistPhantoms guards #246: a file
+// deleted from the library folder *outside* YellowJacket is discovered
+// as an orphan by the next scan, and its playlist entry must survive as
+// a re-linkable phantom — the same preservation the full rescan and
+// stale-retire paths already perform, scoped here to just the orphaned
+// file.  Before the fix the entry became an empty row (audio_file_id
+// NULL and no phantom_file_path), which nothing can ever re-link.
+func TestScan_OrphanCleanupPreservesPlaylistPhantoms(t *testing.T) {
+	t.Parallel()
+
+	lib, db := setupTestLibrary(t)
+
+	root := t.TempDir()
+	track := filepath.Join(root, "gone.mp3")
+
+	writeTestTrack(t, track, 0)
+
+	library, err := db.Queries.CreateLibrary(lib.ctx, sqlcgen.CreateLibraryParams{
+		Name: "orphans",
+		Path: root,
+	})
+	if err != nil {
+		t.Fatalf("create library: %v", err)
+	}
+
+	if metrics := lib.scanInternal(library.ID, library.Name, library.Path); metrics == nil {
+		t.Fatal("first scan returned nil metrics")
+	}
+
+	trackID := queryInt(
+		t, db, "SELECT id FROM audio_files WHERE file_path = ?", track,
+	)
+	if trackID == 0 {
+		t.Fatal("first scan did not import the track")
+	}
+
+	if _, err := db.ExecContext(
+		`INSERT INTO playlists (name) VALUES ('keepme')`,
+	); err != nil {
+		t.Fatalf("seed playlist: %v", err)
+	}
+
+	playlistID := queryInt(
+		t, db, "SELECT id FROM playlists WHERE name = 'keepme'",
+	)
+
+	if _, err := db.ExecContext(
+		`INSERT INTO playlist_tracks (playlist_id, audio_file_id, position)
+		 VALUES (?, ?, 0)`,
+		playlistID, trackID,
+	); err != nil {
+		t.Fatalf("seed playlist_tracks: %v", err)
+	}
+
+	// The file goes away outside the app.
+	if err := os.Remove(track); err != nil {
+		t.Fatalf("remove track: %v", err)
+	}
+
+	if metrics := lib.scanInternal(library.ID, library.Name, library.Path); metrics == nil {
+		t.Fatal("second scan returned nil metrics")
+	}
+
+	if n := queryInt(
+		t, db, "SELECT COUNT(*) FROM audio_files WHERE file_path = ?", track,
+	); n != 0 {
+		t.Fatalf("audio_files still holds the removed path: %d rows", n)
+	}
+
+	if n := queryInt(
+		t, db,
+		"SELECT COUNT(*) FROM playlist_tracks WHERE playlist_id = ? "+
+			"AND audio_file_id IS NULL",
+		playlistID,
+	); n != 1 {
+		t.Fatalf(
+			"playlist entry did not become a phantom: %d null-id rows, want 1",
+			n,
+		)
+	}
+
+	phantomPath := queryString(
+		t, db,
+		"SELECT phantom_file_path FROM playlist_tracks WHERE playlist_id = ?",
+		playlistID,
+	)
+	if phantomPath != track {
+		t.Fatalf(
+			"phantom_file_path = %q, want %q -- the entry cannot be "+
+				"re-linked if the file comes back",
+			phantomPath, track,
+		)
+	}
+}

backend/maintenance/maintenance_test.go

@@ -666,3 +666,120 @@ func TestExpiredHTTPCacheJob_TrimsToBudget(t *testing.T) {
 		t.Errorf("kept %q, want the longest-lived row", kept)
 	}
 }
+
+// TestStaleArtistMetadataJob pins the sweep's two keep rules: an owned
+// artist's metadata survives, a browsed artist's survives while it still
+// holds cached artwork, and everything else goes (#248).
+func TestStaleArtistMetadataJob(t *testing.T) {
+	t.Parallel()
+
+	db := database.NewTestDB(t)
+
+	const (
+		ownedMBID   = "11111111-1111-1111-1111-111111111111"
+		browsedMBID = "22222222-2222-2222-2222-222222222222"
+		staleMBID   = "33333333-3333-3333-3333-333333333333"
+	)
+
+	// The owned artist is in the library - which means a *file* says
+	// so.  An artists row on its own is not ownership.
+	database.InsertTestTrack(t, db, database.TestTrack{
+		FilePath:   "/music/owned.mp3",
+		Artist:     "Owned",
+		ArtistMBID: ownedMBID,
+	})
+
+	for _, mbid := range []string{ownedMBID, browsedMBID, staleMBID} {
+		if _, err := db.ExecContext(
+			`INSERT INTO artist_metadata (mbid, source, data, fetched_at)
+			 VALUES (?, 'wikidata-p18', x'00', CURRENT_TIMESTAMP)`,
+			mbid,
+		); err != nil {
+			t.Fatalf("seed artist_metadata for %s: %v", mbid, err)
+		}
+	}
+
+	// The browsed artist holds cached artwork, so its metadata is still
+	// referenced and must survive.
+	if _, err := db.ExecContext(
+		`INSERT INTO artist_images
+		   (artist_mbid, source, source_url, file_path)
+		 VALUES (?, 'test', 'http://x', '/art/primary.jpg')`,
+		browsedMBID,
+	); err != nil {
+		t.Fatalf("seed artist_images: %v", err)
+	}
+
+	if _, err := StaleArtistMetadataJob(db).Run(context.Background()); err != nil {
+		t.Fatalf("run job: %v", err)
+	}
+
+	for _, tc := range []struct {
+		mbid string
+		want int
+	}{
+		{ownedMBID, 1},
+		{browsedMBID, 1},
+		{staleMBID, 0},
+	} {
+		var n int
+		if err := db.QueryRowWriter(
+			"SELECT COUNT(*) FROM artist_metadata WHERE mbid = ?", tc.mbid,
+		).Scan(&n); err != nil {
+			t.Fatalf("count %s: %v", tc.mbid, err)
+		}
+
+		if n != tc.want {
+			t.Errorf("artist_metadata rows for %s = %d, want %d", tc.mbid, n, tc.want)
+		}
+	}
+}
+
+// TestStaleSearchClicksJob deletes only the clicks old enough to have
+// left the retention window (#249).
+func TestStaleSearchClicksJob(t *testing.T) {
+	t.Parallel()
+
+	db := database.NewTestDB(t)
+
+	count := func(mbid string) int {
+		t.Helper()
+
+		var n int
+		if err := db.QueryRowWriter(
+			"SELECT COUNT(*) FROM search_clicks WHERE entity_mbid = ?", mbid,
+		).Scan(&n); err != nil {
+			t.Fatalf("count %s: %v", mbid, err)
+		}
+
+		return n
+	}
+
+	seed := func(query, mbid, lastClicked string) {
+		t.Helper()
+
+		if _, err := db.ExecContext(
+			`INSERT INTO search_clicks
+			   (query, entity_mbid, entity_type, click_count, last_clicked)
+			 VALUES (?, ?, 'recording', 1, ?)`,
+			query, mbid, lastClicked,
+		); err != nil {
+			t.Fatalf("seed search_clicks: %v", err)
+		}
+	}
+
+	seed("tide", "aaaa", "2024-01-01 00:00:00") // stale
+	seed("tide", "bbbb", "2999-01-01 00:00:00") // recent
+
+	if _, err := StaleSearchClicksJob(db).Run(context.Background()); err != nil {
+		t.Fatalf("run job: %v", err)
+	}
+
+	if n := count("bbbb"); n != 1 {
+		t.Errorf("recent click was deleted: %d rows, want 1", n)
+	}
+
+	if n := count("aaaa"); n != 0 {
+		t.Errorf("stale click survived: %d rows, want 0", n)
+	}
+}

backend/maintenance/sweeps.go

@@ -628,3 +628,69 @@ func dirSize(dir string) (bytes, files int64) {
 
 	return bytes, files
 }
+
+// StaleArtistMetadataJob evicts long-lived artist metadata (bios, wiki
+// leads, relationships) for artists the user no longer has any reason
+// to keep around: not owned and holding no cached artwork.
+//
+// artist_metadata has no TTL by design — entity data changes rarely and
+// re-fetching spends someone else's rate limit — so without a sweep it
+// grows for the life of the install.  This is the "swept when the
+// artist is no longer referenced" contract the datamap always declared
+// for it and nothing ever performed (#248).
+func StaleArtistMetadataJob(db *database.DB) Job {
+	return Job{
+		Name:        "artist-metadata-sweep",
+		MinInterval: dailyInterval,
+		Run: func(_ context.Context) (Result, error) {
+			res, err := db.ExecContext(
+				`DELETE FROM artist_metadata
+				 WHERE mbid NOT IN (` + ownedArtistMBIDs + `)
+				   AND mbid NOT IN (
+				       SELECT artist_mbid FROM artist_images
+				   )`,
+			)
+			if err != nil {
+				return Result{}, fmt.Errorf(
+					"delete stale artist_metadata rows: %w", err,
+				)
+			}
+
+			rows, _ := res.RowsAffected()
+
+			return Result{RowsDeleted: rows}, nil
+		},
+	}
+}
+
+// searchClicksRetention is how long a search-click ranking signal stays
+// useful.  search_clicks is authored behavioural data — nothing that
+// owns a row ever drops it — so age is the ceiling that keeps the table
+// from growing without bound for the life of the install (#249).
+const searchClicksRetention = "-180 days"
+
+// StaleSearchClicksJob deletes search-click ranking rows older than the
+// retention window.  Rows are small and the table grows slowly, so this
+// runs daily and does almost nothing most runs.
+func StaleSearchClicksJob(db *database.DB) Job {
+	return Job{
+		Name:        "search-clicks-sweep",
+		MinInterval: dailyInterval,
+		Run: func(_ context.Context) (Result, error) {
+			res, err := db.ExecContext(
+				`DELETE FROM search_clicks
+				 WHERE last_clicked < datetime('now', ?)`,
+				searchClicksRetention,
+			)
+			if err != nil {
+				return Result{}, fmt.Errorf(
+					"delete stale search_clicks rows: %w", err,
+				)
+			}
+
+			rows, _ := res.RowsAffected()
+
+			return Result{RowsDeleted: rows}, nil
+		},
+	}
+}

backend/playlist/playlist.go

@@ -134,6 +134,12 @@ type Service struct {
 	libraryDir    LibraryDirProvider
 	favoritesConf FavoritesConfigProvider
 
+	// onDeleted, when set, is called after a playlist is deleted so
+	// cross-cutting state that points at it (the queue's "Playing
+	// from" label) can stop pointing at a playlist that no longer
+	// exists.  Wired from app.go, like Library.SetRemovalHooks.
+	onDeleted func(playlistID int64)
+
 	// dataDirOverride, when non-empty, replaces the OS user data
 	// directory as the base for the playlists folder. Set by tests to
 	// keep M3U writes out of the real user data directory.
@@ -166,6 +172,17 @@ func (s *Service) SetFavoritesConfig(
 	s.favoritesConf = provider
 }
 
+// SetOnPlaylistDeleted registers a callback invoked after a playlist is
+// deleted, for cross-cutting invalidation.
+//
+//wails:ignore // internal wiring, not part of the app's IPC surface.
+func (s *Service) SetOnPlaylistDeleted(onDeleted func(playlistID int64)) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s.onDeleted = onDeleted
+}
+
 // ServiceStartup is v3's service lifecycle hook: it runs once the
 // runtime exists, and ctx is cancelled when the app shuts down.  It
 // replaces v2's SetContext, which had to be called by hand from
@@ -766,6 +783,17 @@ func (s *Service) DeletePlaylist(playlistID int64) error {
 
 	s.emitEvent(events.PlaylistDeleted, playlistID)
 
+	// Cross-cutting invalidation: the queue's "Playing from" label may
+	// point at this playlist, and a link to a playlist that no longer
+	// exists is worse than none.
+	s.mu.Lock()
+	onDeleted := s.onDeleted
+	s.mu.Unlock()
+
+	if onDeleted != nil {
+		onDeleted(playlistID)
+	}
+
 	// Recreate the default playlist if we just deleted it.
 	if s.defaultPlaylistID() == playlistID {
 		s.EnsureDefaultPlaylist()

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)
-		 VALUES (?, ?)`,
+		`INSERT INTO listening_events (audio_file_id, kind, occurred_at)
+		 VALUES (?, 'complete', ?)`,
 		audioFileID, now,
 	)
 	if err != nil {

backend/queue/queue.go

@@ -1581,6 +1581,21 @@ func (q *Queue) dropSource() {
 	q.source = Source{}
 }
 
+// DropSourceForPlaylist clears the queue's "Playing from" label when
+// its source playlist is deleted.  A link back to a playlist that no
+// longer exists is worse than none, and the label otherwise survives
+// the deletion until the next SetQueue (#249).
+func (q *Queue) DropSourceForPlaylist(playlistID int64) {
+	q.mu.Lock()
+	defer q.mu.Unlock()
+
+	if (q.source.Type == "playlist" || q.source.Type == "smartPlaylist") &&
+		q.source.ID == playlistID {
+		q.dropSource()
+		q.persistState()
+	}
+}
+
 // commitMutation persists the current queue state after a mutation.
 // When reindex is true, track positions are renumbered first.
 // The caller must hold q.mu.

backend/queue/queue_test.go

@@ -535,3 +535,35 @@ func TestCycleRepeat_CyclesThroughModes(t *testing.T) {
 		t.Errorf("after third cycle: got %q, want %q", state.RepeatMode, RepeatOff)
 	}
 }
+
+// TestDropSourceForPlaylist clears the "Playing from" label when the
+// queue's source playlist is deleted, and leaves it alone otherwise
+// (#249).
+func TestDropSourceForPlaylist(t *testing.T) {
+	t.Parallel()
+
+	q, db := setupTestQueue(t)
+	paths := seedAudioFiles(t, db, 2)
+
+	q.SetQueue(paths, 0, false, Source{Type: "playlist", ID: 42, Label: "Road Trip"})
+	q.DropSourceForPlaylist(42)
+
+	if got := q.GetState().Source; got != (Source{}) {
+		t.Errorf("source = %+v, want empty after playlist 42 deleted", got)
+	}
+}
+
+func TestDropSourceForPlaylistIgnoresOtherPlaylists(t *testing.T) {
+	t.Parallel()
+
+	q, db := setupTestQueue(t)
+	paths := seedAudioFiles(t, db, 2)
+
+	source := Source{Type: "smartPlaylist", ID: 42, Label: "Road Trip"}
+	q.SetQueue(paths, 0, false, source)
+	q.DropSourceForPlaylist(7)
+
+	if got := q.GetState().Source; got != source {
+		t.Errorf("source = %+v, want %+v unchanged for a different playlist", got, source)
+	}
+}

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) =>
-  page.evaluate(() => {
+const headerFit = (page: import('@playwright/test').Page, view = 'playlist-view') =>
+  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/bindings/yellowjacket/backend/queue/queue.ts

@@ -56,6 +56,16 @@ export function CycleRepeat(): $CancellablePromise<void> {
     return $Call.ByID(3510519482);
 }
 
+/**
+ * DropSourceForPlaylist clears the queue's "Playing from" label when
+ * its source playlist is deleted.  A link back to a playlist that no
+ * longer exists is worse than none, and the label otherwise survives
+ * the deletion until the next SetQueue (#249).
+ */
+export function DropSourceForPlaylist(playlistID: number): $CancellablePromise<void> {
+    return $Call.ByID(1435106374, playlistID);
+}
+
 /**
  * EmitCurrentState emits the current queue state to the frontend.
  * This is called after the frontend DOM is ready.

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.
-        if (paths.length === 0) return;
-
-        // `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.
-        if (shuffle && !queueStore.getState().shuffleMode) {
-            queueStore.toggleShuffle();
-        }
-
-        queueStore.setQueue(paths, 0, shuffle, this.queueSource());
+        // so an empty set here is not a state the user can reach.  The
+        // shuffle-mode semantics live in `playAll`, shared with the
+        // play-all/shuffle-all pair on every track list.
+        playAll(this.ownedFilePaths(), this.queueSource(), shuffle);
     }
 
     /** 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;
-
-            if (this.dialog) this.dialog.open = false;
-
-            this.active = false;
+            this.dismiss();
         } 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() {
-        const filePaths = this.tracks
+    private playableFilePaths(): string[] {
+        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() {
-        const filePaths = this.tracks
+    private playableFilePaths(): string[] {
+        return this.tracks
             .filter((t) => !t.Phantom)
             .map((t) => t.FilePath);
+    }
 
-        if (filePaths.length === 0) return;
-
-        queueStore.setQueue(filePaths, 0, false, { type: 'smartPlaylist', id: this.playlistId, label: this.playlistName });
+    private handlePlay() {
+        playAll(
+            this.playableFilePaths(),
+            { type: 'smartPlaylist', id: this.playlistId, label: this.playlistName },
+            false,
+        );
     }
 
     private handleShuffle() {
-        const filePaths = this.tracks
-            .filter((t) => !t.Phantom)
-            .map((t) => t.FilePath);
-
-        if (filePaths.length === 0) return;
-
-        queueStore.setQueue(filePaths, 0, true, { type: 'smartPlaylist', id: this.playlistId, label: this.playlistName });
+        // This used to be a no-op when shuffle mode was off: it passed
+        // `shuffleStart` without turning the mode on, so the queue
+        // started at track 1 in order.  `playAll` sets the mode first.
+        playAll(
+            this.playableFilePaths(),
+            { 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
-        // label is user-visible: the phone has no column headers, and
-        // the page header's sort list is built from the *configured*
-        // columns rather than the drawn ones.
+        // Named for what it sorts by. That label is drawn nowhere
+        // today: the phone has no column headers, and the page header's
+        // sort list is built from the *configured* columns, which this
+        // 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.
- * Used by the settings UI to list available columns.
+ * The column IDs Settings may offer, in default display order.
+ *
+ * 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
-    # cliff with total confidence.  Instant.
+    # The docs document make targets; a stale one sends an agent — or a
+    # 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