From e745acf88aff7dc73f0c17fee7e9aabe25e533a1 Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Wed, 9 Sep 2026 10:16:32 -0400 Subject: [PATCH 1/3] fix(maintenance): sweep artist_metadata rows nothing references artist_metadata was classified Cache/Swept but had no sweep and no DELETE anywhere, so long-lived entity data (no TTL by design) grew for the life of the install. Sweep rows whose MBID is neither a library artist nor holding cached artwork, and register the job with the janitor. Closes #248 --- backend/app.go | 1 + backend/maintenance/maintenance_test.go | 68 +++++++++++++++++++++++++ backend/maintenance/sweeps.go | 34 +++++++++++++ 3 files changed, 103 insertions(+) diff --git a/backend/app.go b/backend/app.go index 376f0ae..2aa531c 100644 --- a/backend/app.go +++ b/backend/app.go @@ -775,6 +775,7 @@ func (yj *YellowJacketApp) startJanitor() { } yj.janitor.Register(maintenance.ExpiredHTTPCacheJob(yj.database)) + yj.janitor.Register(maintenance.StaleArtistMetadataJob(yj.database)) yj.janitor.Register(maintenance.OrphanedCoverFilesJob( yj.database, coversDir, library.CoverArtFileSet, )) diff --git a/backend/maintenance/maintenance_test.go b/backend/maintenance/maintenance_test.go index f88d621..27bf0f7 100644 --- a/backend/maintenance/maintenance_test.go +++ b/backend/maintenance/maintenance_test.go @@ -666,3 +666,71 @@ 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) + } + } +} diff --git a/backend/maintenance/sweeps.go b/backend/maintenance/sweeps.go index 049eae4..b1b50af 100644 --- a/backend/maintenance/sweeps.go +++ b/backend/maintenance/sweeps.go @@ -628,3 +628,37 @@ 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 + }, + } +} -- 2.54.0 From 88f5524aa21d57261ff0bb8ed326104151cdb1d5 Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Wed, 9 Sep 2026 10:22:57 -0400 Subject: [PATCH 2/3] fix(maintenance): bound lyrics search and clicks, clear queue source Three unbounded or stale surfaces, each small on its own: - lyrics_index rows were never pruned on track removal, so the FTS index grew forever. Delete the entry where the library search FTS entry is already deleted, on the orphan and RemoveFromLibrary paths. - search_clicks had no ceiling; age out ranking rows after a retention window via a daily janitor job. - queue.source_* kept a "Playing from X" label after its playlist was deleted. Drop the source when the queue's own playlist goes, wired through a playlist-service hook like Library.SetRemovalHooks. Closes #249 --- backend/app.go | 5 +++ backend/database/lyrics_search.go | 20 ++++++++-- backend/library/library.go | 14 ++++++- backend/library/remove_tracks.go | 5 +++ backend/maintenance/maintenance_test.go | 49 +++++++++++++++++++++++++ backend/maintenance/sweeps.go | 32 ++++++++++++++++ backend/playlist/playlist.go | 28 ++++++++++++++ backend/queue/queue.go | 15 ++++++++ backend/queue/queue_test.go | 32 ++++++++++++++++ 9 files changed, 195 insertions(+), 5 deletions(-) diff --git a/backend/app.go b/backend/app.go index 376f0ae..830a245 100644 --- a/backend/app.go +++ b/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,7 @@ func (yj *YellowJacketApp) startJanitor() { } yj.janitor.Register(maintenance.ExpiredHTTPCacheJob(yj.database)) + yj.janitor.Register(maintenance.StaleSearchClicksJob(yj.database)) yj.janitor.Register(maintenance.OrphanedCoverFilesJob( yj.database, coversDir, library.CoverArtFileSet, )) diff --git a/backend/database/lyrics_search.go b/backend/database/lyrics_search.go index a1f9ead..185f533 100644 --- a/backend/database/lyrics_search.go +++ b/backend/database/lyrics_search.go @@ -151,16 +151,28 @@ func (d *DB) SetLyrics(audioFileID int64, lyrics, source, recordingMBID string) return d.upsertLyricsIndex(audioFileID, lyrics) } -// upsertLyricsIndex refreshes a single file's entry in the contentless -// lyrics_index. contentless_delete=1 makes the DELETE valid; an empty -// lyrics string leaves the row deleted. -func (d *DB) upsertLyricsIndex(audioFileID int64, lyrics string) error { +// DeleteLyricsIndex removes one file's entry from the contentless +// lyrics_index. It is called wherever a file row is deleted — the +// `lyrics` table cascades with its file, but the FTS entry does not and +// would otherwise accumulate for the life of the install (#249). +func (d *DB) DeleteLyricsIndex(audioFileID int64) error { if _, err := d.db.ExecContext(d.Ctx, "DELETE FROM lyrics_index WHERE rowid = ?", audioFileID, ); err != nil { return fmt.Errorf("could not delete lyrics_index row: %w", err) } + return nil +} + +// upsertLyricsIndex refreshes a single file's entry in the contentless +// lyrics_index. contentless_delete=1 makes the DELETE valid; an empty +// lyrics string leaves the row deleted. +func (d *DB) upsertLyricsIndex(audioFileID int64, lyrics string) error { + if err := d.DeleteLyricsIndex(audioFileID); err != nil { + return err + } + if strings.TrimSpace(lyrics) == "" { return nil } diff --git a/backend/library/library.go b/backend/library/library.go index e161c0f..e30d8fa 100644 --- a/backend/library/library.go +++ b/backend/library/library.go @@ -968,7 +968,7 @@ func (l *Library) scanInternal( } } - // Remove from FTS5 search index. + // Remove from FTS5 search index and the lyrics index. if err := l.db.DeleteSearchIndex( audioFile.ID, ); err != nil { @@ -981,6 +981,18 @@ func (l *Library) scanInternal( metrics.addWarning(path, "orphan", err) } + if err := l.db.DeleteLyricsIndex( + audioFile.ID, + ); err != nil { + l.logger.Warn( + "failed to delete lyrics index entry for orphan", + "id", audioFile.ID, + "err", err, + ) + + metrics.addWarning(path, "orphan", err) + } + removed.Add(1) return true diff --git a/backend/library/remove_tracks.go b/backend/library/remove_tracks.go index a6e3098..c554225 100644 --- a/backend/library/remove_tracks.go +++ b/backend/library/remove_tracks.go @@ -127,6 +127,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 diff --git a/backend/maintenance/maintenance_test.go b/backend/maintenance/maintenance_test.go index f88d621..09d46f4 100644 --- a/backend/maintenance/maintenance_test.go +++ b/backend/maintenance/maintenance_test.go @@ -666,3 +666,52 @@ func TestExpiredHTTPCacheJob_TrimsToBudget(t *testing.T) { t.Errorf("kept %q, want the longest-lived row", kept) } } + +// 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) + } +} diff --git a/backend/maintenance/sweeps.go b/backend/maintenance/sweeps.go index 049eae4..9a69b56 100644 --- a/backend/maintenance/sweeps.go +++ b/backend/maintenance/sweeps.go @@ -628,3 +628,35 @@ func dirSize(dir string) (bytes, files int64) { return bytes, files } + +// 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 + }, + } +} diff --git a/backend/playlist/playlist.go b/backend/playlist/playlist.go index fef27a6..126b531 100644 --- a/backend/playlist/playlist.go +++ b/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() diff --git a/backend/queue/queue.go b/backend/queue/queue.go index 8a29c28..03b96eb 100644 --- a/backend/queue/queue.go +++ b/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. diff --git a/backend/queue/queue_test.go b/backend/queue/queue_test.go index c062b6f..faaeae4 100644 --- a/backend/queue/queue_test.go +++ b/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) + } +} -- 2.54.0 From cf90030463b2bff55c2935924dce98651dfa26a9 Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Sun, 20 Sep 2026 20:22:16 -0400 Subject: [PATCH 3/3] chore(bindings): regenerate for the queue's DropSourceForPlaylist frontend/bindings is generated by wails3 and is not covered by the codegen pre-commit hook, so the new bound method went out without it and make bindings-check failed on the PR. --- frontend/bindings/yellowjacket/backend/queue/queue.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/frontend/bindings/yellowjacket/backend/queue/queue.ts b/frontend/bindings/yellowjacket/backend/queue/queue.ts index 5200a95..c6663aa 100644 --- a/frontend/bindings/yellowjacket/backend/queue/queue.ts +++ b/frontend/bindings/yellowjacket/backend/queue/queue.ts @@ -56,6 +56,16 @@ export function CycleRepeat(): $CancellablePromise { 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 { + return $Call.ByID(1435106374, playlistID); +} + /** * EmitCurrentState emits the current queue state to the frontend. * This is called after the frontend DOM is ready. -- 2.54.0