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

Reviewed-on: #245

.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?

backend/database/database_test.go

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

backend/database/playlistphantoms.go

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

backend/database/sql/schemas/audio_files.sql

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

backend/database/sql/schemas/download_items.sql

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

backend/database/sql/schemas/download_requests.sql

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

backend/database/sql/schemas/listening_events.sql

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

backend/database/sql/schemas/play_history.sql

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

backend/database/sql/schemas/queue.sql

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

backend/database/sql/schemas/tagging_items.sql

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

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

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

backend/database/sql/sqlcgen/models.go

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

backend/database/staleshape.go

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

backend/database/staleshape_test.go

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

backend/datamap/datamap.go

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

backend/datamap/datamap_test.go

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

backend/library/rescan.go

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

backend/queue/playhistory.go

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