fix(explore): clear in_library on a row that has no local id

`in_library = 1 AND local_*_id IS NULL` was a fixed point.
upsertBatch's conflict clause is `MAX(in_library, excluded.in_library)`,
so it can only ever raise the flag, and pruneStaleLocalCrossReferences —
which its own comment calls the only place a removal from the library is
reflected back into the index — was gated on the id being present. So
nothing in the app could clear such a row, ever: a permanent claim of
ownership with no local row to check it against.

The gate is now the flag *or* the id, for all three entity types. A NULL
id fails the existence test on its own, so this needs no second clause to
say what "not owned" means.

Nothing in the tree writes that shape today — collectLibraryEntities sets
both together — which is why this is worth closing rather than leaving:
the exposure is a database written by a version whose local-id columns
were populated differently, and the next writer that sets the flag
without an id, which nothing structurally prevents and which this shape
made permanent rather than merely wrong until the next scan.

The test seeds the row with raw SQL on purpose. upsertBatch writes a zero
LocalArtistID as literal 0, and 0 satisfies `IS NOT NULL`, so the old
gate already caught that shape — a fixture built through the upsert
cannot reproduce this at all. NULL is what the artifact importer and any
older writer leave behind, the columns being nullable with no default.
Reverted against the old gate, it fails on all three types.

Closes #118

backend/explore/prune_test.go

@@ -82,6 +82,100 @@ func TestPruneStaleLocalCrossReferences(t *testing.T) {
 	}
 }
 
+// TestPruneClearsInLibraryWithNoLocalID covers the fixed point: a row
+// carrying in_library with a NULL local_*_id.  The upsert's conflict
+// clause is `in_library = MAX(in_library, excluded.in_library)`, so it
+// can only ever raise the flag, and this pass used to be gated on the id
+// being present — which meant nothing in the app could clear such a row,
+// ever.  It is asserted for all three entity types because the gate was
+// written once and used three times, so a fix applied to one is a fix
+// that looks complete.
+//
+// The rows are seeded with raw SQL rather than through seedIndexResult
+// deliberately: upsertBatch writes a zero LocalArtistID as literal 0,
+// not NULL, and 0 satisfies `IS NOT NULL` — so the old gate already
+// caught that shape and a fixture built through the upsert cannot
+// reproduce this at all.  NULL is what the artifact importer and any
+// older writer leave behind, the column being nullable with no default.
+func TestPruneClearsInLibraryWithNoLocalID(t *testing.T) {
+	t.Parallel()
+
+	db := database.NewTestDB(t)
+	si := NewSearchIndex(db, nil, nil, slog.Default())
+
+	// A genuinely owned artist, to prove the wider gate does not simply
+	// clear everything it now looks at.
+	database.InsertTestTrack(t, db, database.TestTrack{
+		FilePath: "/music/owned.mp3",
+		Artist:   "Owned",
+	})
+
+	artist, err := db.Queries.GetArtistByName(t.Context(), "Owned")
+	if err != nil {
+		t.Fatalf("read seeded artist: %v", err)
+	}
+
+	seedIndexResult(t, db, SearchIndexResult{
+		EntityType:    EntityArtist,
+		MBID:          testMBID("owned"),
+		Title:         "Owned",
+		ArtistName:    "Owned",
+		ArtistMBID:    testMBID("owned"),
+		InLibrary:     true,
+		LocalArtistID: artist.ID,
+	})
+
+	orphans := []struct {
+		name       string
+		entityType string
+		mbid       string
+	}{
+		{"artist", EntityArtist, "orphan-artist"},
+		{"release group", EntityReleaseGroup, "orphan-release-group"},
+		{"recording", EntityRecording, "orphan-recording"},
+	}
+
+	for _, o := range orphans {
+		if _, err := db.ExecContext(
+			`INSERT INTO explore_index
+			     (entity_type, mbid, title, artist_name, artist_mbid,
+			      in_library,
+			      local_artist_id, local_release_group_id, local_recording_id)
+			 VALUES (?, ?, ?, ?, ?, 1, ?, ?, ?)`,
+			dbEntityType(o.entityType), dbMBID(testMBID(o.mbid)), o.name, o.name,
+			dbMBID(testMBID(o.mbid)),
+			nil, nil, nil,
+		); err != nil {
+			t.Fatalf("seed %s orphan: %v", o.name, err)
+		}
+	}
+
+	si.pruneStaleLocalCrossReferences()
+
+	inLibrary := func(t *testing.T, mbid string) int {
+		t.Helper()
+
+		var flag int
+		if err := db.QueryRowWriter(
+			"SELECT in_library FROM explore_index WHERE mbid = ?", dbMBID(mbid),
+		).Scan(&flag); err != nil {
+			t.Fatalf("read in_library for %q: %v", mbid, err)
+		}
+
+		return flag
+	}
+
+	for _, o := range orphans {
+		if got := inLibrary(t, testMBID(o.mbid)); got != 0 {
+			t.Errorf("%s with a NULL local id: in_library = %d, want 0", o.name, got)
+		}
+	}
+
+	if got := inLibrary(t, testMBID("owned")); got != 1 {
+		t.Errorf("owned artist: in_library = %d, want 1 (it still has a file)", got)
+	}
+}
+
 // TestUnenrichedLibraryArtistMBIDs_OrdersByOwnedTrackCount verifies the
 // backfill queue prioritizes artists by how many tracks the user actually
 // owns, not by how many duplicate-mbid artist rows happen to exist (the

backend/explore/searchindex.go

@@ -2562,6 +2562,19 @@ func (si *SearchIndex) PopulateLocalCrossReferences() {
 // The row itself is left in place (it may still be part of the shipped
 // catalog, just no longer owned) — only the "this is mine" bookkeeping
 // is cleared.
+//
+// It is gated on the flag *or* the id, not on the id alone.  Gated on
+// the id, `in_library = 1 AND local_*_id IS NULL` is a fixed point: the
+// upsert can only ever raise the flag and this pass skipped such a row
+// by construction, so nothing in the app could clear it — a row claiming
+// to be owned, permanently, with no local row to check the claim
+// against.  Nothing in the tree writes that shape today
+// (collectLibraryEntities sets both together), which is exactly why it
+// is worth closing now: the exposure is a database written by an older
+// version, and the next writer that sets the flag without an id, which
+// nothing structurally prevents.  A NULL id fails the existence test on
+// its own, so the wider gate needs no second clause to say what "not
+// owned" means.
 func (si *SearchIndex) pruneStaleLocalCrossReferences() {
 	type prune struct {
 		entityType string
@@ -2594,7 +2607,8 @@ func (si *SearchIndex) pruneStaleLocalCrossReferences() {
 		result, err := si.db.ExecContext(
 			`UPDATE explore_index
 			 SET in_library = 0, `+p.column+` = NULL
-			 WHERE entity_type = ? AND `+p.column+` IS NOT NULL
+			 WHERE entity_type = ?
+			   AND (`+p.column+` IS NOT NULL OR in_library = 1)
 			   AND NOT EXISTS (`+p.exists+`)`,
 			dbEntityType(p.entityType),
 		)

frontend/src/components/now-playing/now-playing.ts

@@ -13,7 +13,6 @@ import {
     isQueueSourceNavigable,
     navigateToQueueSource,
 } from '@utils/queue-source-link';
-import { PHONE_QUERY } from '@utils/breakpoints';
 import { PlayerController } from '@store/controllers/player-controller';
 import { creditStore } from '@store/credit-store';
 import { QueueController } from '@store/controllers/queue-controller';
@@ -81,19 +80,6 @@ export class NowPlaying extends LitElement {
 
     private reduceMotionQuery?: MediaQueryList;
 
-    /**
-     * Phone width, from the shell's own breakpoint.
-     *
-     * This is in JS rather than in the stylesheet because what changes
-     * is the *content*, not its appearance: the title, artist and
-     * source render as plain text instead of as links, and no CSS rule
-     * can take a click handler off an element.
-     */
-    @state()
-    private phone = false;
-
-    private phoneQuery?: MediaQueryList;
-
     /** Whether each field is actively mid-scroll (class toggle). */
     @state()
     private titleScrolling = false;
@@ -355,12 +341,6 @@ export class NowPlaying extends LitElement {
         this.reduceMotion = this.reduceMotionQuery?.matches ?? false;
         this.reduceMotionQuery?.addEventListener('change', this.handleReduceMotionChange);
 
-        // Same reasoning as above: looked up here, not at module load,
-        // so a test can install its own matchMedia first.
-        this.phoneQuery = window.matchMedia?.(PHONE_QUERY);
-        this.phone = this.phoneQuery?.matches ?? false;
-        this.phoneQuery?.addEventListener('change', this.handlePhoneChange);
-
         this.resizeObserver = new ResizeObserver(() => {
             this.geometryDirty = true;
             this.requestUpdate();
@@ -384,7 +364,6 @@ export class NowPlaying extends LitElement {
         this.attachDragListeners(false);
         window.removeEventListener(SCROLL_CHANGE_EVENT, this.handleScrollModeEvent);
         this.reduceMotionQuery?.removeEventListener('change', this.handleReduceMotionChange);
-        this.phoneQuery?.removeEventListener('change', this.handlePhoneChange);
         this.resizeObserver?.disconnect();
         this.stopScrollCycle('title');
         this.stopScrollCycle('artist');
@@ -509,7 +488,7 @@ export class NowPlaying extends LitElement {
               @mouseleave=${this.handleTitleMouseLeave}
               @transitionend=${() => this.onScrollCycleEnd('title')}
             >
-              <span class="scroll-content">${this.phone ? track.title : trackLink(track.title, track.album, track.releaseGroupMbid, track.recordingMbid) || track.title}</span>
+              <span class="scroll-content">${trackLink(track.title, track.album, track.releaseGroupMbid, track.recordingMbid) || track.title}</span>
             </span>
             <span
               class="track-artist ${artistScrolling ? 'will-scroll' : ''} ${this.artistScrolling ? 'scrolling' : ''}"
@@ -519,15 +498,14 @@ export class NowPlaying extends LitElement {
               @mouseleave=${this.handleArtistMouseLeave}
               @transitionend=${() => this.onScrollCycleEnd('artist')}
             >
-              <span class="scroll-content">${this.phone ? track.artist || 'Unknown Artist' : creditLink(creditStore.credits(track.recordingMbid), track.artist, track.artistMbid) || 'Unknown Artist'}</span>
+              <span class="scroll-content">${creditLink(creditStore.credits(track.recordingMbid), track.artist, track.artistMbid) || 'Unknown Artist'}</span>
             </span>
             ${describeQueueSource(this.queue.source)
                 ? html`
                   <span
-                    class="track-source ${!this.phone && isQueueSourceNavigable(this.queue.source) ? 'navigable' : ''}"
+                    class="track-source ${isQueueSourceNavigable(this.queue.source) ? 'navigable' : ''}"
                     data-testid="now-playing-source"
                     @click=${(e: MouseEvent) => {
-                        if (this.phone) return;
                         if (!isQueueSourceNavigable(this.queue.source)) return;
                         navigateToQueueSource(
                             e.currentTarget as EventTarget,
@@ -593,10 +571,6 @@ export class NowPlaying extends LitElement {
         this.reduceMotion = e.matches;
     };
 
-    private handlePhoneChange = (e: MediaQueryListEvent): void => {
-        this.phone = e.matches;
-    };
-
     private shouldScroll(field: 'title' | 'artist'): boolean {
         const overflows = field === 'title' ? this.titleOverflows : this.artistOverflows;
 
@@ -632,12 +606,6 @@ export class NowPlaying extends LitElement {
             track?.artist ?? '',
             this.shouldScroll('title') ? '1' : '0',
             this.shouldScroll('artist') ? '1' : '0',
-            // Crossing the breakpoint swaps a link for a bare string,
-            // and a link is not guaranteed to measure the same as the
-            // text inside it. The marquee travels a distance read from
-            // that measurement, so this belongs in the key even though
-            // the words are identical either side.
-            this.phone ? '1' : '0',
         ].join('\u0000');
     }
 

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

@@ -11,7 +11,6 @@ import {
 import { SelectionController } from '@utils/selection-controller';
 import type { SelectionHost } from '@utils/selection-controller';
 import { ViewLifecycleMixin } from '@utils/view-lifecycle';
-import { PHONE_QUERY } from '@utils/breakpoints';
 import {
     ContextMenuController,
     contextMenuStyles,
@@ -106,6 +105,9 @@ const ROW_CHROME_WIDTH =
 const ROW_HEIGHT = 33;
 const PHONE_ROW_HEIGHT = 52;
 
+/** The shell's phone breakpoint, as `index.css` and every component
+ *  stylesheet spells it. */
+const PHONE_QUERY = '(max-width: 599px)';
 
 // Inline SVG paths for favorite icons — eliminates wa-icon shadow DOM
 // overhead (30-50 shadow roots during scroll).  Font Awesome 6 paths.

frontend/src/utils/breakpoints.ts

@@ -1,23 +0,0 @@
-/**
- * The shell's breakpoints, where JavaScript has to agree with CSS.
- *
- * A media query inside a shadow root is answered by the viewport, so a
- * component normally states what it drops at phone width in its own
- * stylesheet and needs nothing from here. This exists for the cases
- * where the decision is not a style: `track-list` computes its grid in
- * JS from the host width, and `now-playing` renders *different content*
- * on a phone — a plain string instead of a link — which no stylesheet
- * can express.
- *
- * One breakpoint, several expressions of it. It was a private const in
- * track-list.ts when there was one; a second reader is where a copy
- * would start drifting from index.css.
- */
-
-/**
- * Phone width. 600px rather than the sidebar's 900px because 900 is a
- * laptop: the answer there is a narrower sidebar, which is still a
- * sidebar. Below this the shell drops the sidebar column entirely and
- * bottom-nav takes over.
- */
-export const PHONE_QUERY = '(max-width: 599px)';

frontend/test/components/now-playing-phone.test.ts

@@ -1,166 +0,0 @@
-/**
- * The mini player's links are a desktop affordance.
- *
- * `utils/explore-link.ts` makes every track and artist name navigate,
- * and `utils/queue-source-link.ts` makes "Playing from X" navigate — in
- * the bottom bar those are a few characters of text at a font size
- * chosen for a bar, which is not a touch target. Worse, explore-link
- * holds the navigation for one double-click interval and drops it if a
- * second click arrives: a gesture that exists so double-clicking a row
- * can play it, and which means nothing at all on touch.
- *
- * So below the shell's phone breakpoint the three render as plain text
- * and the whole bar's cover art opens the full-screen Now Playing view,
- * which is where the links live.
- *
- * The breakpoint is stubbed rather than emulated for the reason
- * track-list-phone.test.ts states: this tier's viewport is fixed at
- * 1280x800 by the runner, and the component reads matchMedia in
- * connectedCallback precisely so a test can answer it first.
- */
-import { describe, expect, it, beforeEach } from 'vitest';
-
-import '@components/now-playing/now-playing';
-import { Events } from '../../src/events';
-import { emit, flush } from '@test/support/harness';
-import { fixture, shadow, shadowAll, text } from '@test/support/render';
-import type { TrackInfo } from '@store/player-store';
-import type { QueueTrack } from '@store/queue-store';
-
-const TRACK: TrackInfo = {
-  fileName: 'ashes.mp3',
-  filePath: '/music/ashes.mp3',
-  trackLength: 215,
-  seekPosition: 0,
-  state: 'playing',
-  title: 'Ashes to Ashes',
-  artist: 'David Bowie',
-  album: 'Scary Monsters',
-  coverArt: '',
-  coverArtSmall: '',
-  coverArtMedium: '',
-  coverArtLarge: '',
-  trackChangeId: 1,
-  artistMbid: '',
-  releaseGroupMbid: '',
-  recordingMbid: '',
-};
-
-function queueTrack(n: number, title: string): QueueTrack {
-  return {
-    id: n,
-    audioFileId: n,
-    filePath: `/music/${n}.mp3`,
-    position: n,
-    title,
-    artist: 'David Bowie',
-    album: 'Scary Monsters',
-    coverArtPath: '',
-    artistMbid: '',
-    releaseGroupMbid: '',
-    recordingMbid: '',
-  };
-}
-
-/** Mount the bar with the phone breakpoint answering `matches`. */
-async function mountAt(phone: boolean) {
-  const real = window.matchMedia.bind(window);
-
-  window.matchMedia = ((q: string) =>
-    q.includes('max-width: 599px')
-      ? {
-          matches: phone,
-          media: q,
-          addEventListener() {},
-          removeEventListener() {},
-        }
-      : real(q)) as typeof window.matchMedia;
-
-  try {
-    const el = await fixture('now-playing');
-
-    emit(Events.TrackChanged, { ...TRACK, trackChangeId: 20 });
-    emit(Events.QueueChanged, {
-      tracks: [queueTrack(1, 'Ashes to Ashes')],
-      currentIndex: 0,
-      source: { type: 'album', id: 7, label: 'Scary Monsters' },
-    });
-    await flush();
-    await el.updateComplete;
-
-    return el;
-  } finally {
-    window.matchMedia = real;
-  }
-}
-
-describe('the mini player on a phone', () => {
-  beforeEach(() => {
-    emit(Events.TrackChanged, { ...TRACK, trackChangeId: 1 });
-  });
-
-  it('renders the title and artist as plain text', async () => {
-    const el = await mountAt(true);
-
-    expect(shadowAll(el, '.explore-link').length).toBe(0);
-
-    // The words are unchanged — this is about what they are, not about
-    // hiding them. A fix that dropped the text would pass an assertion
-    // about links alone.
-    expect(text(el, '[data-testid="now-playing-title"]')).toContain(
-      'Ashes to Ashes',
-    );
-    expect(text(el, '[data-testid="now-playing-artist"]')).toContain(
-      'David Bowie',
-    );
-  });
-
-  it('does not navigate from the source line', async () => {
-    const el = await mountAt(true);
-    const source = shadow<HTMLElement>(el, '[data-testid="now-playing-source"]');
-
-    expect(source?.classList.contains('navigable')).toBe(false);
-
-    let navigated = false;
-    el.addEventListener('navigate', () => {
-      navigated = true;
-    });
-
-    source?.click();
-
-    expect(navigated).toBe(false);
-  });
-
-  it('still says where the queue came from', async () => {
-    const el = await mountAt(true);
-
-    // Dropping the *link* is the change; dropping the information would
-    // be a different and worse one.
-    expect(text(el, '[data-testid="now-playing-source"]')).toBe(
-      'Playing from Scary Monsters',
-    );
-  });
-
-  it('leaves the desktop bar exactly as it was', async () => {
-    const el = await mountAt(false);
-
-    expect(shadowAll(el, '.explore-link').length).toBeGreaterThan(0);
-
-    const source = shadow<HTMLElement>(el, '[data-testid="now-playing-source"]');
-
-    expect(source?.classList.contains('navigable')).toBe(true);
-
-    let detail: unknown;
-    el.addEventListener('navigate', (e) => {
-      detail = (e as CustomEvent).detail;
-    });
-
-    source?.click();
-
-    expect(detail).toEqual({
-      view: 'explore-album-details',
-      localAlbumId: 7,
-      albumName: 'Scary Monsters',
-    });
-  });
-});