fix(database): preserve playlist phantoms across a stale audio_files retire

Retiring a stale audio_files dropped every playlist entry to an empty
row: ON DELETE SET NULL ran before the phantom_* columns were filled,
whereas the manual rescan path populates them first. Run the same
phantom population inside the retire transaction, before the drop, only
when audio_files is among the tables going, so
ResolvePhantomTracksAfterScan can re-link the entries.

Closes #183

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