ci(index-artifact): import the exported artifact before publishing it

Nothing should be published until the code that imports it on a user's
machine has imported it here. The exporter and the importer are two
descriptions of one storage format, and every other tier tests the
importer against a *fixture* rather than against the file being shipped —
a second description free to be wrong in the same direction as the code
reading it.

That is how #258 reached everyone: the importer positioned its batch walk
with a Go `string` cursor against this file's 16-byte `mbid` column, and
SQLite neither coerces between TEXT and BLOB nor complains about the
comparison, so the walk merged no rows and never advanced. The fixture
guarding that walk writes the old text encoding, and the only compact
fixture is one row, below the batch size, so the bound query never ran.
Both were green throughout, and no install could finish a first index
build.

`TestImportPublishedArtifact` takes the published file and runs the
client's own path over it — checksum, decompress, merge — and asserts
that what the artifact holds is what the client ends up with: the row
count, the rows carrying a listen count, an FTS index in step with the
table, and one row read back through the app's own MBID conversions. It
skips without `YJ_CORE_INDEX_ARTIFACT`, so an ordinary run pays nothing.

It needs no Wails, which is why the step runs it under the indexbuild
tag: that container has no GTK. Measured on the current artifact, 64.8 MB
compressed: 39 seconds including the decompress.

Verified against the pre-fix comparison behaviour, the step goes red in
about 3 seconds — the strictly-advancing guard fails the import with a
named reason rather than the day-long spin it used to produce.

Refs #258

.gitea/workflows/ci.yml

@@ -107,15 +107,32 @@ jobs:
       # Conventional Commits.  `.releaserc.yml` has always derived the
       # version from the commit type; until now nothing checked that the
       # type was one it recognises, so a malformed subject silently meant
-      # "no release".  BEFORE is the push's previous tip and is absent or
+      # "no release".
-      # all-zeros for a new branch, in which case only the tip is linted.
+      #
+      # **On a `pull_request` there is no `before`.**  Gitea leaves
+      # `github.event.before` empty for one, so this step fell through to
+      # bare `make commit-check`, which lints `git log -1` — the tip
+      # alone.  Every other commit the branch would bring was first
+      # examined by *main's* post-merge run, which is a green PR that
+      # stops being true after the merge, and which happened twice (#254).
+      # The PR's base is the stand-in: the range below already excludes
+      # what the base shares with the branch, because base advances on
+      # main and those commits stay reachable from it.
+      #
+      # Both are handed to the shell rather than chosen in an expression:
+      # `github.event.issue.number` in unclaim.yml is this repo's proof
+      # that payload fields resolve, and the shell then falls back to
+      # today's behaviour for a dispatch run or a missing field instead of
+      # depending on how `&&`/`||` treat an absent context.
       - name: Commit messages
         working-directory: /src
         env:
-          BEFORE: ${{ github.event.before }}
+          PR_BASE: ${{ github.event.pull_request.base.sha }}
+          PUSH_BEFORE: ${{ github.event.before }}
         run: |
           set -eu
-          if [ -n "${BEFORE:-}" ] && [ "${BEFORE#0000000}" = "$BEFORE" ] \
+          BEFORE="${PR_BASE:-${PUSH_BEFORE:-}}"
+          if [ -n "$BEFORE" ] && [ "${BEFORE#0000000}" = "$BEFORE" ] \
              && git cat-file -e "$BEFORE^{commit}" 2>/dev/null; then
             make commit-check RANGE="$BEFORE..$SHA"
           else

.gitea/workflows/index-artifact.yml

@@ -68,7 +68,7 @@ jobs:
       # claim with a test behind it now (cmd/indexbuild/deps_test.go),
       # because the v3 migration quietly broke it and this job was where
       # that surfaced.
-      image: golang:1.25
+      image: golang:1.26
       # This host path must exist on the runner and be listed verbatim in
       # act_runner's container.valid_volumes. It holds explore-staging/
       # (counts.bin + state.json) and yj.db — the checkpoint that makes
@@ -148,6 +148,49 @@ jobs:
           sha256sum /tmp/core-index.db.zst | tee /tmp/core-index.db.zst.sha256
           ls -lh /tmp/core-index.db.zst
 
+      # Nothing is published until it has been imported by the code that
+      # imports it on a user's machine. The exporter and the importer are
+      # two descriptions of one storage format, and every other tier tests
+      # the importer against a *fixture* rather than against the file being
+      # shipped — a second description free to be wrong in the same
+      # direction as the code reading it.
+      #
+      # That is how #258 reached everyone: the importer positioned its batch
+      # walk with a Go `string` cursor against this file's 16-byte `mbid`
+      # column, and SQLite neither coerces between TEXT and BLOB nor
+      # complains about the comparison — so the walk merged no rows and
+      # never advanced, and no install could finish its first index build.
+      # The fixture guarding that walk writes the old text encoding, and the
+      # only compact fixture is one row, below the batch size, so the bound
+      # query never ran. Both were green throughout.
+      #
+      # Running it here is also what keeps the failure cheap: the previous
+      # artifact stays published while this runs, so a failure costs one
+      # stale catalog rather than an empty one for every install.
+      #
+      # `-tags indexbuild` because this container has no GTK and the default
+      # tag set links the app through Wails. The `--- PASS` grep is not
+      # decoration — the test skips without the path, and a skip is
+      # indistinguishable from a pass in a summary line.
+      - name: Import the exported artifact as a client does
+        if: steps.maintain.outputs.complete == 'true' && steps.maintain.outputs.changed == 'true'
+        working-directory: /src
+        env:
+          YJ_CORE_INDEX_ARTIFACT: /tmp/core-index.db.zst
+        run: |
+          set -eu
+          log=/tmp/import-check.log
+          if ! go test -tags indexbuild -count=1 -timeout 30m -v \
+            -run TestImportPublishedArtifact ./backend/explore/ > "$log" 2>&1;
+          then
+            tail -60 "$log"
+            echo "::error::The artifact does not import; not publishing it."
+            exit 1
+          fi
+          cat "$log"
+          grep -qF -- 'PASS: TestImportPublishedArtifact' "$log"
+          echo "::notice::The artifact imports as a client would merge it."
+
       - name: Publish to the Gitea package registry
         if: steps.maintain.outputs.complete == 'true' && steps.maintain.outputs.changed == 'true'
         run: |

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

@@ -57,6 +57,21 @@ behind `YJ_TESTCTL=1`, which `scripts/dev-headless.sh` sets and
   staging the work that would produce it — job progress, download
   progress, scan progress. It calls `events.Deliver`, which *errors*
   when the event reaches nobody, so a `200` means it really arrived.
+- **State you stage, you own** (#168). Nothing resets those stores, so
+  clear yours in `test.afterEach` with the same event that staged it
+  (`emit('JobsChanged', [])`) — the store replaces its list from every
+  snapshot, so `testctl` needs no special case. **Measured: this does
+  not currently cross a spec boundary**, because every test gets a fresh
+  page and `JobStore.init()` refetches `GetJobs()` from a backend
+  registry that `/__test/emit` never writes to. Stated anyway, because
+  it costs one line and the leak needs only one spec that keeps a page
+  alive — but do not cite #168 for a symptom you have not reproduced.
+- **Measure against the thing next to you, not an absolute
+  coordinate.** An absolute number in a shell measurement is also a
+  claim about everything above it — `contentTop === 0` quietly asserts
+  "and no background job is running", which is not what that spec was
+  about or could arrange, while `contentTop === jobBandBottom` is true
+  either way. This is the half of #168 that stands on its own.
 - **`restore` is slow** (~40 s in the suite) because it copies every
   table. Prefer snapshotting once and restoring only when a spec
   genuinely mutates state.

.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

@@ -845,6 +845,22 @@ that is quietly empty.
   top-N, exact match, FTS search, popularity batch, the CAA map — and
   asserts each returns something with a dashed id. A missed conversion
   site shows up there and essentially nowhere else.
+- **A comparison is typed on *both* sides, and a parameter is the half
+  that gets forgotten.** The paragraph above is about a literal; the
+  artifact merge positioned its batch walk with a Go `string` cursor
+  against the artifact's byte column, and SQLite answered rather than
+  complained: `mbid > ?` with a text key is true of every row, so the
+  bound the walk looked up was the same every time and the cursor
+  never advanced, while `mbid <= ?` is false of every row, so no batch
+  merged at all. The import looped indefinitely at 100% CPU behind a
+  progress bar reading "0 of 1,077,893 rows", merged nothing and
+  raised nothing (#258). Nothing caught it because the fixture that
+  guards the walk writes the old text form and the only compact one is
+  a single row — below `artifactMergeBatch`, so the bound query never
+  ran. `artifactKey` types the cursor to the artifact's own encoding
+  now, and the walk fails loudly when its bound does not strictly
+  advance, because the failure mode here is silence rather than a
+  wrong answer.
 
 **The artifact is read in either encoding.** A published artifact
 carries whichever form the exporter that built it used, and there is one

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/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
+// DeleteLyricsIndex removes one file's entry from the contentless
-// lyrics_index.  contentless_delete=1 makes the DELETE valid; an empty
+// lyrics_index.  It is called wherever a file row is deleted — the
-// lyrics string leaves the row deleted.
+// `lyrics` table cascades with its file, but the FTS entry does not and
-func (d *DB) upsertLyricsIndex(audioFileID int64, lyrics string) error {
+// 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
+-- ListDownloadItemsForDownload filters on the parent download.
--- existing database this table already exists at schema-pass time with
+CREATE INDEX IF NOT EXISTS idx_download_items_download
--- its old column still named request_id, so an inline CREATE INDEX on
+    ON download_items(download_id);
--- download_id would fail outright. See ensureDownloadIndexes in
--- backend/database/download_rename_migration.go.

backend/database/sql/schemas/download_requests.sql

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

backend/database/sql/schemas/listening_events.sql

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

backend/database/sql/schemas/play_history.sql

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

backend/database/sql/schemas/queue.sql

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

backend/database/sql/schemas/tagging_items.sql

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

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

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

backend/database/sql/sqlcgen/models.go

@@ -94,6 +94,8 @@ type AudioFile struct {
 	ModifiedAt         int64
 	PlayCount          int64
 	LastPlayed         sql.NullTime
+	SkipCount          int64
+	LastSkipped        sql.NullTime
 	TagStatus          string
 }
 
@@ -261,6 +263,15 @@ type Library struct {
 	AutotagWarningAcked int64
 }
 
+type ListeningEvent struct {
+	ID              int64
+	AudioFileID     int64
+	Kind            string
+	PositionSeconds int64
+	DurationSeconds int64
+	OccurredAt      time.Time
+}
+
 type Lyric struct {
 	AudioFileID   int64
 	Text          string
@@ -273,12 +284,6 @@ type LyricsIndex struct {
 	Lyrics string
 }
 
-type PlayHistory struct {
-	ID          int64
-	AudioFileID int64
-	PlayedAt    time.Time
-}
-
 type PlayerState struct {
 	ID                  int64
 	Volume              int64
@@ -312,15 +317,14 @@ type PlaylistTrack struct {
 }
 
 type Queue struct {
-	ID               int64
+	ID              int64
-	SourcePlaylistID sql.NullInt64
+	CurrentPosition int64
-	CurrentPosition  int64
+	ShuffleMode     bool
-	ShuffleMode      bool
+	RepeatMode      string
-	RepeatMode       string
+	ShuffleOrder    sql.NullString
-	ShuffleOrder     sql.NullString
+	SourceType      string
-	SourceType       string
+	SourceID        int64
-	SourceID         int64
+	SourceLabel     string
-	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,
+		Name: "listening_events", Kind: Authored, Lifetime: Cascade,
-		Note: "Listening history. Authored, but intentionally cascades " +
+		Note: "Listening history, one row per track exit (complete, play " +
-			"with its track — history for a file no longer in the library " +
+			"or skip). Authored, but intentionally cascades with its " +
-			"has nothing to point at.",
+			"track — history for a file no longer in the library has " +
+			"nothing to point at.",
 	},
 	{
 		Name: "player_state", Kind: Authored, Lifetime: Retained,

backend/datamap/datamap_test.go

@@ -212,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,
+		"listening_events": true,
-		"queue_tracks": 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/explore/artifactimport.go

@@ -1,8 +1,10 @@
 package explore
 
 import (
+	"bytes"
 	"context"
 	"database/sql"
+	"database/sql/driver"
 	"errors"
 	"fmt"
 	"os"
@@ -283,6 +285,25 @@ func (si *SearchIndex) importCoreArtifact(ctx context.Context, path string) erro
 	}
 
 	merged, mergeErr := si.mergeArtifactRows(ctx, info.rows)
+
+	// Every row the artifact declares has to land.  The walk partitions
+	// the artifact's key space, so a total short of info.rows does not
+	// mean the artifact was smaller than it said -- it means a predicate
+	// filtered rows out, and the catalog is quietly partial.  Equality
+	// rather than a lower bound because RowsAffected counts an upsert
+	// that changes nothing, and a row already merged locally is counted
+	// again here.
+	//
+	// One reachable case, so this is not merely a tripwire: a row whose
+	// mbid is empty is excluded by `mbid > ?` in both encodings, and an
+	// artifact carrying one would otherwise import as complete.
+	if mergeErr == nil && merged != info.rows {
+		mergeErr = fmt.Errorf(
+			"%w: merged %d of %d rows — a row the artifact holds was not selected",
+			ErrArtifactUnusable, merged, info.rows,
+		)
+	}
+
 	if mergeErr == nil {
 		si.mergeArtifactCredits(ctx)
 	}
@@ -348,8 +369,12 @@ func (si *SearchIndex) analyzeIndex() {
 // is an index range scan and a cancelled import leaves committed work
 // behind rather than rolling it all back.
 func (si *SearchIndex) mergeArtifactRows(ctx context.Context, total int) (int, error) {
+	// Asked once, because it is a property of the file and it decides
+	// how the walk's own comparisons are typed.  See artifactKey.
+	storesText := si.artifactStoresText()
+
 	selectColumns := artifactSelectColumns(
-		si.artifactStoresText(), si.artifactHasTotals(),
+		storesText, si.artifactHasTotals(),
 	)
 
 	insertSQL := `
@@ -367,7 +392,7 @@ func (si *SearchIndex) mergeArtifactRows(ctx context.Context, total int) (int, e
 		WHERE mbid > ? AND mbid <= ?` + upsertIndexConflictSQL
 
 	var (
-		cursor string
+		cursor artifactKey
 		merged int
 	)
 
@@ -376,17 +401,30 @@ func (si *SearchIndex) mergeArtifactRows(ctx context.Context, total int) (int, e
 			return merged, err
 		}
 
-		upper, hasUpper, err := si.artifactBatchBound(cursor)
+		upper, hasUpper, err := si.artifactBatchBound(storesText, cursor)
 		if err != nil {
 			return merged, err
 		}
 
+		if hasUpper && bytes.Compare(upper, cursor) <= 0 {
+			// The predicate matched the cursor itself, so the walk can
+			// never advance.  SQLite says nothing when a comparison is
+			// made between types it will not coerce - the query simply
+			// answers wrongly - so a mismatch here would otherwise spin
+			// forever behind an unmoving progress bar.  Fail instead.
+			return merged, fmt.Errorf(
+				"%w: artifact walk did not advance past %x",
+				ErrArtifactUnusable, []byte(cursor),
+			)
+		}
+
 		var res sql.Result
 
 		if hasUpper {
-			res, err = si.db.ExecContext(insertRangeSQL, cursor, upper)
+			res, err = si.db.ExecContext(insertRangeSQL,
+				cursor.bind(storesText), upper.bind(storesText))
 		} else {
-			res, err = si.db.ExecContext(insertSQL, cursor)
+			res, err = si.db.ExecContext(insertSQL, cursor.bind(storesText))
 		}
 
 		if err != nil {
@@ -413,26 +451,65 @@ func (si *SearchIndex) mergeArtifactRows(ctx context.Context, total int) (int, e
 	}
 }
 
+// artifactKey is one MBID as the attached artifact stores it: 16 raw
+// bytes in a compact artifact, the dashed 36-character form in one
+// published before that storage change.
+//
+// It is a type with a bind method rather than a string because the
+// comparison it feeds is typed, and the wrong type is silent.  SQLite
+// does not coerce between TEXT and BLOB and orders every blob after
+// every text value, so a cursor bound as text against a byte column
+// makes `mbid > ?` true of the whole table - the walk rediscovers the
+// same batch bound forever, and `mbid <= ?` false of the whole table,
+// so no batch merges at all.  Nothing errors; the import simply never
+// finishes.  bind is the one place that knows which form the column is
+// in, decided by artifactStoresText, which asks the artifact rather than
+// trusting a version number.
+type artifactKey []byte
+
+// bind renders the key as a statement argument in the artifact's own
+// encoding.
+func (k artifactKey) bind(storesText bool) driver.Value {
+	if storesText {
+		return string(k)
+	}
+
+	// Never nil.  database/sql converts a nil []byte to SQL NULL, and
+	// `mbid > NULL` is NULL for every row - so an unset cursor would
+	// agree with nothing and import nothing, which is the same silently
+	// empty merge this type exists to prevent, one type over.
+	if k == nil {
+		return []byte{}
+	}
+
+	return []byte(k)
+}
+
 // artifactBatchBound returns the MBID that ends the next batch, and
 // whether one exists — no bound means the remainder is the last batch.
-func (si *SearchIndex) artifactBatchBound(cursor string) (string, bool, error) {
+//
-	var bound string
+// The bound is read out of the artifact and handed back as an
+// artifactKey, because it becomes the next comparison the walk makes.
+func (si *SearchIndex) artifactBatchBound(
+	storesText bool, cursor artifactKey,
+) (artifactKey, bool, error) {
+	var bound []byte
 
 	err := si.db.QueryRowWriter(
 		`SELECT mbid FROM core.explore_index
 		 WHERE mbid > ? ORDER BY mbid LIMIT 1 OFFSET ?`,
-		cursor, artifactMergeBatch-1,
+		cursor.bind(storesText), artifactMergeBatch-1,
 	).Scan(&bound)
 
 	if errors.Is(err, sql.ErrNoRows) {
-		return "", false, nil
+		return nil, false, nil
 	}
 
 	if err != nil {
-		return "", false, fmt.Errorf("%w: batch bound: %w", ErrArtifactUnusable, err)
+		return nil, false, fmt.Errorf("%w: batch bound: %w", ErrArtifactUnusable, err)
 	}
 
-	return bound, true, nil
+	return artifactKey(bound), true, nil
 }
 
 // stampArtifactMeta records what the merge established: the catalog half

backend/explore/artifactimport_test.go

@@ -1,6 +1,7 @@
 package explore
 
 import (
+	"bytes"
 	"context"
 	"database/sql"
 	"encoding/hex"
@@ -71,13 +72,7 @@ func writeTestArtifact(
 		}
 	}
 
-	for k, v := range meta {
+	stampArtifactMeta(t, db, meta)
-		if _, err := db.Exec(
-			`INSERT INTO artifact_meta (key, value) VALUES (?, ?)`, k, v,
-		); err != nil {
-			t.Fatalf("stamp artifact meta: %v", err)
-		}
-	}
 
 	for _, r := range rows {
 		if _, err := db.Exec(`
@@ -93,6 +88,101 @@ func writeTestArtifact(
 	return path
 }
 
+// compactArtifactSchema is the artifact cmd/indexexport publishes: the
+// catalog's ids as 16 raw bytes, its entity types as codes, and the
+// per-release-group total_tracks the exporter added after the first
+// artifact was shipped.
+//
+// It matters that a fixture carries this encoding and not the older
+// text one, because SQLite does not coerce between TEXT and BLOB and
+// every comparison the importer makes against an mbid is therefore
+// encoding-sensitive.  writeTestArtifact above is the *other* fixture:
+// it still writes the text form, which is what the first published
+// artifact carries and what the importer must keep reading.
+var compactArtifactSchema = []string{
+	`CREATE TABLE explore_index (
+		entity_type      INTEGER NOT NULL,
+		mbid             BLOB NOT NULL,
+		title            TEXT NOT NULL,
+		artist_name      TEXT NOT NULL,
+		artist_mbid      BLOB NOT NULL,
+		aliases          TEXT NOT NULL DEFAULT '',
+		popularity       INTEGER NOT NULL DEFAULT 0,
+		listener_count   INTEGER NOT NULL DEFAULT 0,
+		duration         INTEGER NOT NULL DEFAULT 0,
+		caa_release_mbid BLOB NOT NULL DEFAULT x'',
+		release_name     TEXT NOT NULL DEFAULT '',
+		primary_type     TEXT NOT NULL DEFAULT '',
+		secondary_types  TEXT NOT NULL DEFAULT '',
+		release_date     TEXT NOT NULL DEFAULT '',
+		total_tracks     INTEGER NOT NULL DEFAULT 0,
+		artist_type      TEXT NOT NULL DEFAULT '',
+		country          TEXT NOT NULL DEFAULT '',
+		disambiguation   TEXT NOT NULL DEFAULT '',
+		sort_name        TEXT NOT NULL DEFAULT '',
+		discog_fetched   INTEGER NOT NULL DEFAULT 0,
+		PRIMARY KEY (mbid)
+	) WITHOUT ROWID`,
+	`CREATE TABLE artifact_meta (
+		key   TEXT PRIMARY KEY,
+		value TEXT NOT NULL
+	)`,
+}
+
+// writeCompactTestArtifact builds the artifact the exporter publishes
+// today, in its own encoding, so the importer is exercised against what
+// a client actually downloads rather than against what it was written
+// for.
+func writeCompactTestArtifact(
+	t *testing.T, meta map[string]string, rows []artifactRow,
+) string {
+	t.Helper()
+
+	path := filepath.Join(t.TempDir(), "core-index.db")
+
+	db, err := sql.Open("sqlite", "file:"+path)
+	if err != nil {
+		t.Fatalf("open artifact: %v", err)
+	}
+
+	defer func() { _ = db.Close() }()
+
+	for _, stmt := range compactArtifactSchema {
+		if _, err := db.Exec(stmt); err != nil {
+			t.Fatalf("create artifact schema: %v", err)
+		}
+	}
+
+	stampArtifactMeta(t, db, meta)
+
+	for _, r := range rows {
+		if _, err := db.Exec(`
+			INSERT INTO explore_index
+				(entity_type, mbid, title, artist_name, artist_mbid, popularity)
+			VALUES (?, ?, ?, ?, ?, ?)`,
+			entityCode(r.entityType), mbidBytes(r.mbid), r.title,
+			r.artistName, mbidBytes(r.artistMBID), r.popularity,
+		); err != nil {
+			t.Fatalf("insert artifact row: %v", err)
+		}
+	}
+
+	return path
+}
+
+// stampArtifactMeta writes the artifact_meta rows a fixture declares.
+func stampArtifactMeta(t *testing.T, db *sql.DB, meta map[string]string) {
+	t.Helper()
+
+	for k, v := range meta {
+		if _, err := db.Exec(
+			`INSERT INTO artifact_meta (key, value) VALUES (?, ?)`, k, v,
+		); err != nil {
+			t.Fatalf("stamp artifact meta: %v", err)
+		}
+	}
+}
+
 // validMeta is the artifact_meta a well-formed artifact carries.
 func validMeta() map[string]string {
 	return map[string]string{
@@ -270,6 +360,71 @@ func TestImportCoreArtifactBatchWalkCoversAllRows(t *testing.T) {
 	}
 }
 
+// TestImportCoreArtifactBatchWalkCoversAllRowsCompact is the batch walk
+// on the encoding the exporter actually publishes.
+//
+// The walk positions itself by comparing the artifact's own mbid column
+// against the last id it reached, and that column holds 16 raw bytes.
+// SQLite does not coerce between TEXT and BLOB, and a blob sorts after
+// every text value, so a cursor bound as text is a predicate that either
+// matches every row or none: `mbid > ?` with an empty text key is true
+// of the whole table, so
+// the 100th row is always the 100th row and the bound never advances,
+// while `mbid <= <text>` is false of the whole table, so no batch ever
+// merges.  The result is not a wrong import but an unbounded loop that
+// merges nothing and never fails.
+//
+// Both encodings are covered on purpose.  The walk was only ever tested
+// against the text fixture above, which is why it shipped broken on the
+// one the clients download.
+func TestImportCoreArtifactBatchWalkCoversAllRowsCompact(t *testing.T) {
+	db := database.NewTestDB(t)
+	si := NewSearchIndex(db, nil, nil, testLogger())
+
+	original := artifactMergeBatch
+	artifactMergeBatch = 100
+
+	t.Cleanup(func() { artifactMergeBatch = original })
+
+	const total = 337
+
+	rows := make([]artifactRow, 0, total)
+	for i := range total {
+		rows = append(rows, artifactRow{
+			entityType: EntityRecording,
+			mbid:       syntheticMBID(i),
+			title:      "Song",
+			artistName: "Artist",
+			artistMBID: artA,
+			popularity: i,
+		})
+	}
+
+	path := writeCompactTestArtifact(t, validMeta(), rows)
+
+	if err := si.importCoreArtifact(context.Background(), path); err != nil {
+		t.Fatalf("importCoreArtifact: %v", err)
+	}
+
+	var got, top int
+
+	if err := db.QueryRowWriter(
+		"SELECT COUNT(*), MAX(popularity) FROM explore_index",
+	).Scan(&got, &top); err != nil {
+		t.Fatalf("count rows: %v", err)
+	}
+
+	if got != total {
+		t.Errorf("merged %d rows, want %d", got, total)
+	}
+
+	// A count alone would pass if the walk re-merged the same first
+	// batch forever, so the far end of the artifact is checked too.
+	if top != total-1 {
+		t.Errorf("highest popularity = %d, want %d", top, total-1)
+	}
+}
+
 func TestImportCoreArtifactRejectsBadArtifacts(t *testing.T) {
 	tests := []struct {
 		name string
@@ -454,61 +609,9 @@ func TestArtifactColumnsMatchExporter(t *testing.T) {
 // the importer decides by asking the artifact, not by trusting a
 // version number, and both must land identically.
 func TestImportCoreArtifactAcceptsBothEncodings(t *testing.T) {
-	compact := filepath.Join(t.TempDir(), "core-index.db")
+	compact := writeCompactTestArtifact(t, validMeta(), []artifactRow{
-
+		{EntityArtist, artA, "Artist A", "Artist A", artA, 5000},
-	db, err := sql.Open("sqlite", "file:"+compact)
+	})
-	if err != nil {
-		t.Fatalf("open artifact: %v", err)
-	}
-
-	if _, err := db.Exec(`CREATE TABLE explore_index (
-		entity_type      INTEGER NOT NULL,
-		mbid             BLOB NOT NULL,
-		title            TEXT NOT NULL,
-		artist_name      TEXT NOT NULL,
-		artist_mbid      BLOB NOT NULL,
-		aliases          TEXT NOT NULL DEFAULT '',
-		popularity       INTEGER NOT NULL DEFAULT 0,
-		listener_count   INTEGER NOT NULL DEFAULT 0,
-		duration         INTEGER NOT NULL DEFAULT 0,
-		caa_release_mbid BLOB NOT NULL DEFAULT x'',
-		release_name     TEXT NOT NULL DEFAULT '',
-		primary_type     TEXT NOT NULL DEFAULT '',
-		secondary_types  TEXT NOT NULL DEFAULT '',
-		release_date     TEXT NOT NULL DEFAULT '',
-		artist_type      TEXT NOT NULL DEFAULT '',
-		country          TEXT NOT NULL DEFAULT '',
-		disambiguation   TEXT NOT NULL DEFAULT '',
-		sort_name        TEXT NOT NULL DEFAULT '',
-		discog_fetched   INTEGER NOT NULL DEFAULT 0,
-		PRIMARY KEY (mbid)
-	)`); err != nil {
-		t.Fatalf("create artifact table: %v", err)
-	}
-
-	if _, err := db.Exec(
-		`CREATE TABLE artifact_meta (key TEXT PRIMARY KEY, value TEXT NOT NULL)`,
-	); err != nil {
-		t.Fatalf("create artifact meta: %v", err)
-	}
-
-	for k, v := range validMeta() {
-		if _, err := db.Exec(
-			"INSERT INTO artifact_meta (key, value) VALUES (?, ?)", k, v,
-		); err != nil {
-			t.Fatalf("write artifact meta: %v", err)
-		}
-	}
-
-	if _, err := db.Exec(`
-		INSERT INTO explore_index (entity_type, mbid, title, artist_name, artist_mbid, popularity)
-		VALUES (1, ?, 'Artist A', 'Artist A', ?, 5000)`,
-		mbidBytes(artA), mbidBytes(artA),
-	); err != nil {
-		t.Fatalf("write artifact row: %v", err)
-	}
-
-	_ = db.Close()
 
 	live := database.NewTestDB(t)
 	si := NewSearchIndex(live, nil, nil, testLogger())
@@ -748,3 +851,88 @@ func TestImportCoreArtifactWithoutCredits(t *testing.T) {
 		t.Errorf("credit refs = %d, want 0", refs)
 	}
 }
+
+// TestImportCoreArtifactRefusesAMergeThatLosesRows is the count guard's
+// positive case.
+//
+// The walk's predicates partition the artifact's key space, so a merge
+// that lands fewer rows than the artifact declares means a predicate
+// dropped some — and the failure is a catalog that looks populated and
+// is missing things nobody can name.  An empty mbid is the reachable
+// way to get there: `mbid > ?` is false of it in both encodings, so it
+// is never selected, and nothing else in the import would notice.
+func TestImportCoreArtifactRefusesAMergeThatLosesRows(t *testing.T) {
+	db := database.NewTestDB(t)
+	si := NewSearchIndex(db, nil, nil, testLogger())
+
+	path := writeCompactTestArtifact(t, validMeta(), []artifactRow{
+		{EntityArtist, artA, "Artist A", "Artist A", artA, 5000},
+		{EntityArtist, "", "Nameless", "Artist A", artA, 4000},
+	})
+
+	err := si.importCoreArtifact(context.Background(), path)
+	if err == nil {
+		t.Fatal("a merge that lost a row was reported as a complete import")
+	}
+
+	if !strings.Contains(err.Error(), "merged 1 of 2 rows") {
+		t.Errorf("error = %v, want it to name the shortfall", err)
+	}
+
+	// And the same rule as every other rejection: a failed merge must not
+	// leave the index claiming it has a catalog, or the real build would
+	// never run again.
+	if si.hasMeta(dumpImportDoneKey) {
+		t.Error("a failed import still stamped dump_import_done")
+	}
+}
+
+// TestArtifactKeyBindsInTheArtifactsOwnEncoding pins the one place the
+// batch walk's comparison type is decided.
+//
+// Every wrong answer is silent, which is why it is worth pinning all
+// four.  SQLite does not coerce TEXT to BLOB and orders every blob after
+// every text value, so a text key against a byte column makes
+// `mbid > ?` true of the whole artifact - the cursor never advances and
+// the walk spins forever without merging a row - while a byte key
+// against a text column makes it false of the whole artifact, so every
+// batch merges nothing and the import "succeeds" empty.  An unset cursor
+// is the same fault once more: database/sql converts a nil []byte to
+// SQL NULL, and `mbid > NULL` matches no row at all.
+func TestArtifactKeyBindsInTheArtifactsOwnEncoding(t *testing.T) {
+	raw := mbidBytes(artA)
+
+	for _, tt := range []struct {
+		name string
+		key  artifactKey
+		want []byte
+	}{
+		{"unset", nil, []byte{}},
+		{"set", artifactKey(raw), raw},
+	} {
+		t.Run("bytes/"+tt.name, func(t *testing.T) {
+			got, ok := tt.key.bind(false).([]byte)
+			if !ok {
+				t.Fatalf("bind(false) = %T, want []byte", tt.key.bind(false))
+			}
+
+			if got == nil {
+				t.Fatal("bound to SQL NULL, which matches no row")
+			}
+
+			if !bytes.Equal(got, tt.want) {
+				t.Errorf("bind(false) = %x, want %x", got, tt.want)
+			}
+		})
+	}
+
+	// The dashed form is what an artifact published before the storage
+	// change carries, and it has to compare as text against text.
+	if got := artifactKey(nil).bind(true); got != "" {
+		t.Errorf("bind(true) on an unset cursor = %#v, want an empty string", got)
+	}
+
+	if got := artifactKey(artA).bind(true); got != artA {
+		t.Errorf("bind(true) = %#v, want %q", got, artA)
+	}
+}

backend/explore/publishedartifact_test.go

@@ -0,0 +1,266 @@
+package explore
+
+import (
+	"context"
+	"database/sql"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"yellowjacket/backend/database"
+)
+
+// Import of the artifact we actually publish, as a client imports it.
+//
+// Every other test here builds a fixture, and a fixture is a second
+// description of the storage format that can be wrong in the same
+// direction as the code reading it.  That is how #258 shipped: the
+// importer positioned its batch walk with a Go `string` cursor against
+// the artifact's 16-byte `mbid` column, and SQLite neither coerces
+// between TEXT and BLOB nor complains about the comparison — so the walk
+// merged nothing and never advanced, and no install could finish its
+// first index build.  The fixture that guards the walk writes the old
+// text encoding; the only compact fixture is one row, below the batch
+// size, so the bound query never ran.  Both passed throughout.
+//
+// So this one takes the published file and runs the client's own path
+// over it — checksum, decompress, merge — and asserts that what the
+// artifact holds is what the client ends up with.
+//
+// It skips without the path, so an ordinary test run pays nothing for
+// it, and the publish job is where it is meant to run:
+//
+//	YJ_CORE_INDEX_ARTIFACT=/tmp/core-index.db.zst \
+//	  go test -tags indexbuild -run TestImportPublishedArtifact \
+//	  ./backend/explore/
+//
+// The indexbuild tag is not incidental: that job's container has no GTK,
+// and the default tag set links the app through Wails.
+
+// publishedArtifactEnv points at the published artifact: the compressed
+// core-index.db.zst, or the unpacked core-index.db.
+const publishedArtifactEnv = "YJ_CORE_INDEX_ARTIFACT"
+
+// artifactTotals is the pair this test compares across the boundary.
+//
+// Rows is the whole point — a merge that lands fewer of them than the
+// artifact declares is a catalog that looks populated and is missing
+// things nobody can name — and popularity is the half whose absence was
+// reported when it happened, because it arrives only through the merge.
+type artifactTotals struct {
+	rows       int
+	withListen int
+}
+
+func TestImportPublishedArtifact(t *testing.T) {
+	published := strings.TrimSpace(os.Getenv(publishedArtifactEnv))
+	if published == "" {
+		t.Skipf("set %s=<core-index.db.zst> to import the published artifact",
+			publishedArtifactEnv)
+	}
+
+	if _, err := os.Stat(published); err != nil {
+		t.Fatalf("%s: %v", publishedArtifactEnv, err)
+	}
+
+	// A file-backed database rather than NewTestDB's in-memory one: the
+	// artifact is ~135MB and a million rows, which is not a thing to hold
+	// in RAM inside a test.  YJ_HOME is how NewDB is pointed somewhere
+	// disposable, and going through NewDB means this is the constructor,
+	// the schema and the read pool the app itself opens.
+	//
+	// Nothing closes it, because nothing can: `DB` has no Close and the
+	// app's handles are process-lifetime by design.  The directory is
+	// unlinked at cleanup and the file goes with it.
+	t.Setenv("YJ_HOME", t.TempDir())
+
+	db, err := database.NewDB(testLogger())
+	if err != nil {
+		t.Fatalf("open database: %v", err)
+	}
+
+	si := NewSearchIndex(db, nil, nil, testLogger())
+
+	// The checksum the publisher shipped, if it shipped one.  Every
+	// client verifies it and refuses the artifact when it does not
+	// match, so a wrong one breaks Explore for everyone who has not
+	// already imported — and nothing else would see it, because the
+	// comparison is between two files only the publisher has.
+	if want, ok := publishedChecksum(published); ok {
+		got, err := fileSHA256(published)
+		if err != nil {
+			t.Fatalf("checksum the artifact: %v", err)
+		}
+
+		if got != want {
+			t.Errorf("published artifact hashes to %s, but its .sha256 says %s",
+				got, want)
+		}
+	}
+
+	unpacked := unpackPublishedArtifact(t, si, published)
+
+	want, err := artifactTotalsOf(unpacked)
+	if err != nil {
+		t.Fatalf("count the artifact's rows: %v", err)
+	}
+
+	if err := si.importCoreArtifact(context.Background(), unpacked); err != nil {
+		t.Fatalf("importCoreArtifact: %v", err)
+	}
+
+	got, err := indexTotalsOf(db)
+	if err != nil {
+		t.Fatalf("count the index's rows: %v", err)
+	}
+
+	if got.rows != want.rows {
+		t.Errorf("merged %d rows, but the artifact holds %d",
+			got.rows, want.rows)
+	}
+
+	if got.withListen != want.withListen {
+		t.Errorf("%d rows carry a listen count, but the artifact holds %d of them",
+			got.withListen, want.withListen)
+	}
+
+	// The FTS index is rebuilt from the table once the merge is done, and
+	// it is what search actually reads: a merge that lands without it
+	// leaves Explore silently matching nothing, which is the state #258
+	// produced by a different route.
+	var indexed int
+	if err := db.QueryRowWriter(
+		"SELECT COUNT(*) FROM explore_index_fts",
+	).Scan(&indexed); err != nil {
+		t.Fatalf("count the FTS index: %v", err)
+	}
+
+	if indexed != got.rows {
+		t.Errorf("FTS index holds %d rows against the table's %d",
+			indexed, got.rows)
+	}
+
+	// And one row read back through the app's own path, which is the
+	// other direction of every conversion the merge makes: a byte MBID
+	// out of the table, the app's dashed form, and back in as a lookup.
+	var raw []byte
+	if err := db.QueryRowWriter(`
+		SELECT mbid FROM explore_index
+		WHERE entity_type = 1 /* artist */ AND popularity > 0
+		ORDER BY popularity DESC LIMIT 1`).Scan(&raw); err != nil {
+		t.Fatalf("read a stored mbid: %v", err)
+	}
+
+	dashed, err := mbidFromBytes(raw)
+	if err != nil {
+		t.Fatalf("the stored mbid is not one: %v", err)
+	}
+
+	artist := si.LookupArtistByMBID(dashed)
+	if artist == nil {
+		t.Fatalf("the artifact's most popular artist %s does not look up", dashed)
+	}
+
+	if artist.Popularity == 0 {
+		t.Errorf("artist %s came back with no popularity", dashed)
+	}
+}
+
+// publishedChecksum reads the sha256 the publisher wrote beside the
+// artifact, in `sha256sum` output form.  A missing file is not a
+// failure: it is only there when the artifact came from the publish job.
+func publishedChecksum(path string) (string, bool) {
+	body, err := os.ReadFile(path + ".sha256")
+	if err != nil {
+		return "", false
+	}
+
+	sum := strings.TrimSpace(string(body))
+	if i := strings.IndexAny(sum, " \t"); i > 0 {
+		sum = sum[:i]
+	}
+
+	if len(sum) != 64 {
+		return "", false
+	}
+
+	return strings.ToLower(sum), true
+}
+
+// unpackPublishedArtifact returns a path to the unpacked database,
+// going through the client's own decompression when it is handed the
+// compressed file that is actually published.
+func unpackPublishedArtifact(t *testing.T, si *SearchIndex, path string) string {
+	t.Helper()
+
+	if strings.HasSuffix(path, ".db") {
+		return path
+	}
+
+	// Copied into the test's own directory first: decompress writes
+	// beside the compressed file, and the publisher's directory is not
+	// this test's to write in.
+	staging := t.TempDir()
+	dst := filepath.Join(staging, coreArtifactFile)
+
+	src, err := os.Open(path)
+	if err != nil {
+		t.Fatalf("open the published artifact: %v", err)
+	}
+
+	defer func() { _ = src.Close() }()
+
+	out, err := os.Create(dst)
+	if err != nil {
+		t.Fatalf("create a staging copy: %v", err)
+	}
+
+	if _, err := io.Copy(out, src); err != nil {
+		t.Fatalf("copy the published artifact: %v", err)
+	}
+
+	if err := out.Close(); err != nil {
+		t.Fatalf("close the staging copy: %v", err)
+	}
+
+	fetcher := &artifactFetcher{si: si, stagingDir: staging}
+
+	if err := fetcher.decompress(context.Background()); err != nil {
+		t.Fatalf("decompress the published artifact: %v", err)
+	}
+
+	return fetcher.unpackedPath()
+}
+
+// artifactTotalsOf counts what an artifact file holds, read directly so
+// the numbers do not depend on anything the client does.
+func artifactTotalsOf(path string) (artifactTotals, error) {
+	db, err := sql.Open("sqlite", "file:"+path+"?mode=ro")
+	if err != nil {
+		return artifactTotals{}, err
+	}
+
+	defer func() { _ = db.Close() }()
+
+	var totals artifactTotals
+
+	err = db.QueryRow(`SELECT COUNT(*), COALESCE(SUM(popularity > 0), 0)
+		FROM explore_index`).Scan(&totals.rows, &totals.withListen)
+	if err != nil {
+		return artifactTotals{}, err
+	}
+
+	return totals, nil
+}
+
+// indexTotalsOf counts what the client ended up with.
+func indexTotalsOf(db *database.DB) (artifactTotals, error) {
+	var totals artifactTotals
+
+	err := db.QueryRowWriter(`SELECT COUNT(*), COALESCE(SUM(popularity > 0), 0)
+		FROM explore_index`).Scan(&totals.rows, &totals.withListen)
+
+	return totals, err
+}

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/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.
+	// Collect and delete orphaned cover_art rows before the commit.  The
-	// SAFETY: Hand-crafted SELECT for orphaned cover art identification.
+	// shared helper is the one place this sweep lives, so the scan path,
-	// Parameterless.
+	// RemoveFromLibrary and this removal cannot drift (#247).
-	rows, err := tx.QueryContext(l.ctx,
+	orphanedCoverArtPaths, err := l.sweepOrphanedCoverArt(tx)
-		`SELECT file_path FROM cover_art WHERE id NOT IN (
-			SELECT DISTINCT cover_art_id FROM albums
-			WHERE cover_art_id IS NOT NULL
-		)`)
 	if err != nil {
-		return nil, fmt.Errorf("could not query orphaned cover art: %w", err)
+		return nil, 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)
 	}
 
 	// 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
+	// Post-commit: remove the orphaned cover art files and their sized
-	// variants.  Only the original is stored in cover_art.file_path; the
+	// variants.
-	// _sm/_md/_lg thumbnails are derived filenames beside it, so they
+	l.removeCoverArtFiles(orphanedCoverArtPaths)
-	// 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,
-				)
-			}
-		}
-	}
 
 	// 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)
+			orphanPaths = append(orphanPaths, key.(string))
-			audioFile := value.(sqlcgen.AudioFile)
+			orphans = append(orphans, value.(sqlcgen.AudioFile))
 
-			l.logger.Debug(
+			return true
-				"removing orphaned database entry",
+		})
-				"path", path, "id", audioFile.ID,
-			)
 
-			if err := l.db.Queries.DeleteAudioFile(
+		deleted := make([]bool, len(orphans))
-				l.ctx, audioFile.ID,
-			); err != nil {
-				l.logger.Warn(
-					"failed to delete orphaned audio file",
-					"path", path,
-					"id", audioFile.ID,
-					"err", err,
-				)
 
-				metrics.addWarning(path, "orphan", err)
+		if len(orphans) > 0 {
-
+			orphanIDs := make([]int64, len(orphans))
-				return true
+			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.
+			// Remove from FTS5 search index and the lyrics index.
-			if err := l.db.DeleteSearchIndex(
+			if err := l.db.DeleteSearchIndex(f.ID); err != nil {
-				audioFile.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
+	// Shared with the stale-shape retire in backend/database, which is
-		SET
+	// the other path that empties this table and which did not do this
-			phantom_title = COALESCE(phantom_title, (
+	// (#183): the statement lives there so the two cannot drift again.
-				SELECT tm.title FROM track_metadata tm
+	if err := database.PreservePlaylistPhantoms(l.ctx, tx, l.logger); err != nil {
-				WHERE tm.id = playlist_tracks.audio_file_id
+		return err
-			)),
-			phantom_artist = COALESCE(phantom_artist, (
-				SELECT tm.artist_name FROM track_metadata tm
-				WHERE tm.id = playlist_tracks.audio_file_id
-			)),
-			phantom_album = COALESCE(phantom_album, (
-				SELECT tm.album FROM track_metadata tm
-				WHERE tm.id = playlist_tracks.audio_file_id
-			)),
-			phantom_duration_ms = COALESCE(phantom_duration_ms, (
-				SELECT af.length_milliseconds FROM audio_files af
-				WHERE af.id = playlist_tracks.audio_file_id
-			)),
-			phantom_file_path = COALESCE(phantom_file_path, (
-				SELECT af.file_path FROM audio_files af
-				WHERE af.id = playlist_tracks.audio_file_id
-			))
-		WHERE audio_file_id IS NOT NULL
-	`); err != nil {
-		return fmt.Errorf(
-			"could not preserve playlist track metadata: %w", err,
-		)
 	}
 
 	// Phase 2: the files.  file_genres cascades with them.

backend/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)
+		`INSERT INTO listening_events (audio_file_id, kind, occurred_at)
-		 VALUES (?, ?)`,
+		 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/jobs-on-a-phone.spec.ts

@@ -60,6 +60,23 @@ const PHONE = { width: 424, height: 439 };
 const DESKTOP = { width: 1100, height: 800 };
 
 test.describe('background jobs on a phone', () => {
+  /**
+   * **State a spec stages is the spec's to clear.** `/__test/emit` writes
+   * to a store nothing resets, so the event that staged a job is the
+   * event that clears it — `JobStore` replaces its whole list from every
+   * snapshot, so `testctl` needs no special case.
+   *
+   * **Measured on #168: this does not currently outlive the page.** Every
+   * test gets a fresh page, and `JobStore.init()` refetches `GetJobs()`
+   * from a backend registry that `/__test/emit` never writes to, so the
+   * staged job is gone before the next spec starts. Ownership is stated
+   * rather than a live leak repaired — the leak needs a page that
+   * survives its own spec, and there is none today.
+   */
+  test.afterEach(async ({ testctl }) => {
+    await testctl.emit('JobsChanged', []);
+  });
+
   test('are shown in the band, without opening anything', async ({
     app,
     testctl,

e2e/specs/top-bar-fit.spec.ts

@@ -102,6 +102,22 @@ const collapsed = (page: Page) =>
   }));
 
 test.describe('the top bar fits the window', () => {
+  /**
+   * **State a spec stages is the spec's to clear** (#168). `/__test/emit`
+   * writes to a store nothing resets, and this file stages the widest job
+   * in the app, so it puts it back — with the same event, since the store
+   * replaces its whole list from every snapshot.
+   *
+   * **Measured: it does not currently outlive the page.** Every test gets
+   * a fresh page and `JobStore.init()` refetches `GetJobs()` from a
+   * backend registry `/__test/emit` never writes to, so nothing is being
+   * repaired here; the rule is stated because it costs one line and the
+   * leak would need only one spec that keeps a page alive.
+   */
+  test.afterEach(async ({ testctl }) => {
+    await testctl.emit('JobsChanged', []);
+  });
+
   /**
    * The phone's answer, which is not "it fits" (#57).
    *

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.

scripts/skill-check.sh

@@ -82,23 +82,69 @@ targets="$({ make -pqRr 2>/dev/null || true; } |
 # happened to break there, and a check that fails on reflow gets
 # disabled rather than fixed.
 #
+# **An inline span may be hard-wrapped, and then the mention is split
+# across two lines.**  `make` at the end of one line and its target at
+# the start of the next is one code span to Markdown and two strings to
+# a per-line regex, so the target was invisible — and these docs are
+# mostly hard-wrapped prose, so the wrap is what the author does not
+# think about.  Lines are therefore joined while the span is still open,
+# which is what an odd number of backticks means.
+#
+# Joining re-opens the reflow trap above unless it is bounded, so it is
+# bounded three ways: a fence flushes first (a fenced command is already
+# whole, and joining inside one would break the line-start rule), a
+# blank line flushes (CommonMark does not allow a blank line inside a
+# code span, so nothing legitimate is split by one), and so does a file
+# boundary.  A stray odd backtick in prose therefore costs one paragraph
+# of over-matching rather than the rest of the file.
+#
 # 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="$(printf '%s\n' "$docs" |
 	xargs awk '
-		FNR == 1 { fence = 0 }
+		function scan(text,    rest) {
-		/^```/   { fence = !fence; next }
+			rest = text
-		{
-			rest = $0
 			while (match(rest, /`make [a-z][a-z0-9-]*/)) {
 				print substr(rest, RSTART + 6, RLENGTH - 6)
 				rest = substr(rest, RSTART + RLENGTH)
 			}
-			if (fence && match($0, /^make [a-z][a-z0-9-]*/)) {
+		}
-				print substr($0, 6, RLENGTH - 5)
+
+		function lineStart(text) {
+			if (match(text, /^make [a-z][a-z0-9-]*/)) {
+				print substr(text, 6, RLENGTH - 5)
 			}
 		}
+
+		function ticks(s,    n, i) {
+			n = 0
+			for (i = 1; i <= length(s); i++) {
+				if (substr(s, i, 1) == "`") n++
+			}
+			return n
+		}
+
+		function flush() {
+			if (buf == "") return
+			scan(buf)
+			if (fence) lineStart(buf)
+			buf = ""
+		}
+
+		FNR == 1 { flush(); fence = 0 }
+
+		/^```/ { flush(); fence = !fence; next }
+
+		/^[[:space:]]*$/ { flush(); next }
+
+		{
+			if (fence) { scan($0); lineStart($0); next }
+			buf = (buf == "" ? $0 : buf " " $0)
+			if (ticks(buf) % 2 == 0) flush()
+		}
+
+		END { flush() }
 	' | sort -u)"
 
 missing=""
@@ -113,7 +159,16 @@ if [ -n "$missing" ]; then
 	echo "skill-check: the docs name make targets that do not exist:" >&2
 	for t in $missing; do
 		echo "  make $t" >&2
-		printf '%s\n' "$docs" | xargs grep -ln "make $t" | sed 's/^/      /' >&2
+		# `make <t>` on one line first, because that is where a target is
+		# normally named and it is the precise answer.  The bare name is the
+		# fallback, and it exists because the parser above can now find a
+		# mention that *this* grep cannot: a wrapped span has `make` and its
+		# target on different lines.  Without it a missing target reported no
+		# file at all, and `set -o pipefail` turned the empty grep into exit
+		# 123, before the line telling the author what to do.
+		hits="$(printf '%s\n' "$docs" | xargs grep -ln "make $t" 2>/dev/null || true)"
+		[ -n "$hits" ] || hits="$(printf '%s\n' "$docs" | xargs grep -ln -- "$t" 2>/dev/null || true)"
+		[ -n "$hits" ] && printf '%s\n' "$hits" | sed 's/^/      /' >&2
 	done
 	echo "Fix the docs, or restore the target." >&2
 	exit 1