Compare commits

...
Author SHA1 Message Date
yonlu a5c3990d12 Merge pull request 'Small-fix batch: artist_metadata sweep, and the three unbounded surfaces (#248, #249)' (#255) from batch/248-249 into main
CI / check (push) Successful in 3m2s
CI / e2e (push) Successful in 11m13s
Closes #248
Closes #249
2026-09-21 02:01:54 +00:00
yonlu 8dbdb7ad75 Merge branch 'fix/249-unbounded-growth' into batch/248-249
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m42s
CI / e2e (pull_request) Successful in 11m12s
Both branches add to the same two registries, so the conflicts are
between the two fixes rather than with main:

- backend/app.go: both register a janitor job.  Both are registered.
- backend/maintenance/sweeps.go: both append a job at the end of the
  file.  Both are kept, each with its own closing tail.
- backend/maintenance/maintenance_test.go: both append a test.  Both are
  kept as separate functions.
- backend/library/library.go: #249's orphan-path lyrics delete was
  written against the loop variable before #250 renamed it, so its
  `audioFile.ID` no longer exists in that function.  Adapted to `f.ID`.

Closes #248
Closes #249
2026-09-20 21:40:49 -04:00
yonlu a205224a26 Merge branch 'fix/248-artist-metadata-sweep' into batch/248-249 2026-09-20 21:34:50 -04:00
yonlu a96cc9be1f Merge remote-tracking branch 'origin/main' into fix/248-artist-metadata-sweep
CI / e2e (push) Skipped
CI / check (push) Skipped
CI / check (pull_request) Successful in 3m22s
CI / e2e (pull_request) Successful in 11m43s
2026-09-20 21:15:14 -04:00
yonlu 8db19622b2 Merge pull request 'fix(library): sweep orphaned cover art when an album empties' (#251) from fix/247-cover-art-orphans into main
CI / check (push) Successful in 3m20s
CI / e2e (push) Successful in 12m5s
default
2026-09-21 01:14:58 +00:00
yonlu 53f480980f Merge remote-tracking branch 'origin/main' into fix/247-cover-art-orphans
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m35s
CI / e2e (pull_request) Successful in 11m41s
2026-09-20 20:59:14 -04:00
yonlu 1335572f0a Merge pull request 'fix(library): preserve playlist phantoms on incremental scan and removal' (#250) from fix/246-incremental-scan-phantoms into main
CI / check (push) Successful in 3m28s
CI / e2e (push) Successful in 12m53s
default
2026-09-21 00:41:05 +00:00
yonlu cf90030463 chore(bindings): regenerate for the queue's DropSourceForPlaylist
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 6m50s
CI / e2e (pull_request) Successful in 11m9s
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.
2026-09-20 20:22:16 -04:00
yonlu 88f5524aa2 fix(maintenance): bound lyrics search and clicks, clear queue source
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Failing after 3m44s
CI / e2e (pull_request) Skipped
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
2026-09-11 17:14:41 -04:00
yonlu e745acf88a fix(maintenance): sweep artist_metadata rows nothing references
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m6s
CI / e2e (pull_request) Successful in 11m22s
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
2026-09-09 10:16:32 -04:00
yonlu 32bb64918c fix(library): sweep orphaned cover art when an album empties
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m19s
CI / e2e (pull_request) Successful in 10m53s
pruneEmptyEntities deleted empty albums but never the cover_art rows
they referenced, so removing the last track of an album leaked the row
and its files forever — the janitor's covers sweep computes its live set
from cover_art.file_path, which keeps the orphaned row's files exempt.

Extract sweepOrphanedCoverArt/removeCoverArtFiles as one implementation
and run it from pruneEmptyEntities (scan orphan path and
RemoveFromLibrary) and RemoveLibrary alike.

Closes #247
2026-09-09 10:14:07 -04:00
yonlu 1b9868ddd0 fix(library): preserve playlist phantoms on incremental scan and removal
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m33s
CI / e2e (pull_request) Successful in 11m12s
The incremental scan's orphan cleanup and RemoveFromLibrary deleted
audio_files rows without first filling the playlist phantom columns, so
a track removed from the library folder outside YellowJacket (or
removed from the library) became a permanently empty playlist row that
nothing could re-link — the same bug #183 fixed on the full-rescan and
retire paths, on the two paths it missed.

Add a scoped PreservePlaylistPhantomsForFiles and run it in the same
transaction as the deletes on both paths.

Closes #246
2026-09-09 10:09:17 -04:00
yonlu 6aeac42a46 Merge pull request 'fix(database): preserve playlist phantoms across a stale audio_files retire' (#245) from fix/183-phantom-across-retire into main
CI / check (push) Failing after 49s
CI / e2e (push) Skipped
Reviewed-on: #245
2026-09-09 13:54:36 +00:00
16 changed files with 864 additions and 94 deletions
+6
View File
@@ -499,6 +499,10 @@ func (yj *YellowJacketApp) OnStartup(ctx context.Context) {
PostRemove: yj.explore.InvalidateLibrarySync,
})
// A deleted playlist must not leave the queue's "Playing from"
// label pointing at it.
yj.playlist.SetOnPlaylistDeleted(yj.queue.DropSourceForPlaylist)
// Register playback finished handler to drive queue auto-advance.
yj.player.SetPlaybackFinishedHandler(yj.queue.OnPlaybackFinished)
@@ -775,6 +779,8 @@ func (yj *YellowJacketApp) startJanitor() {
}
yj.janitor.Register(maintenance.ExpiredHTTPCacheJob(yj.database))
yj.janitor.Register(maintenance.StaleArtistMetadataJob(yj.database))
yj.janitor.Register(maintenance.StaleSearchClicksJob(yj.database))
yj.janitor.Register(maintenance.OrphanedCoverFilesJob(
yj.database, coversDir, library.CoverArtFileSet,
))
+16 -4
View File
@@ -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
}
+60 -8
View File
@@ -5,6 +5,7 @@ import (
"database/sql"
"fmt"
"log/slog"
"strings"
)
// Preserving a playlist entry across the loss of its track is two
@@ -89,12 +90,14 @@ const (
// 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.
// Every path that empties `audio_files` must call this (or the scoped
// variant below) first, inside the same transaction as the delete.
// These paths have drifted before: the full rescan in backend/library
// did this and the stale-shape retire in this package did not, so the
// *documented* repair ("delete and rescan") preserved playlists while
// the automatic one that exists to spare the user that work silently
// emptied them (#183). The incremental scan's orphan cleanup and
// RemoveFromLibrary drifted the same way and are #246.
//
// The display half is skipped, with a warning, when `track_metadata`
// cannot answer -- see the note above. Skipping it costs a phantom
@@ -103,13 +106,39 @@ const (
func PreservePlaylistPhantoms(
ctx context.Context, tx *sql.Tx, logger *slog.Logger,
) error {
if _, err := tx.ExecContext(ctx, preservePhantomPathSQL); err != nil {
return preservePlaylistPhantoms(ctx, tx, nil, logger)
}
// PreservePlaylistPhantomsForFiles is PreservePlaylistPhantoms scoped to
// the given audio file ids, for the two removal paths that delete a
// known subset of the table rather than all of it: the incremental
// scan's orphan cleanup and RemoveFromLibrary. A bulk pass there would
// rewrite every linked playlist row on every scan for nothing.
func PreservePlaylistPhantomsForFiles(
ctx context.Context, tx *sql.Tx, ids []int64, logger *slog.Logger,
) error {
return preservePlaylistPhantoms(ctx, tx, ids, logger)
}
func preservePlaylistPhantoms(
ctx context.Context, tx *sql.Tx, ids []int64, logger *slog.Logger,
) error {
clause, args, skip := phantomIDFilter(ids)
if skip {
return nil
}
if _, err := tx.ExecContext(
ctx, preservePhantomPathSQL+clause, args...,
); err != nil {
return fmt.Errorf(
"could not preserve playlist track file paths: %w", err,
)
}
if _, err := tx.ExecContext(ctx, preservePhantomDisplaySQL); err != nil {
if _, err := tx.ExecContext(
ctx, preservePhantomDisplaySQL+clause, args...,
); err != nil {
// A failed statement does not roll back a SQLite transaction,
// so the path half above stands and the entries remain
// re-linkable.
@@ -123,3 +152,26 @@ func PreservePlaylistPhantoms(
return nil
}
// phantomIDFilter builds the extra WHERE terms and arguments that scope
// a preservation pass to a set of audio file ids. A nil ids returns the
// empty clause (a bulk run over every linked entry); an empty slice
// reports skip, since there is nothing to preserve.
func phantomIDFilter(ids []int64) (clause string, args []any, skip bool) {
switch {
case ids == nil:
return "", nil, false
case len(ids) == 0:
return "", nil, true
}
clause = " AND audio_file_id IN (" +
strings.Repeat("?,", len(ids)-1) + "?)"
args = make([]any, len(ids))
for i, id := range ids {
args[i] = id
}
return clause, args, false
}
+94
View File
@@ -0,0 +1,94 @@
package database
import (
"context"
"database/sql"
"testing"
)
// TestPreservePlaylistPhantomsForFilesScopesToTheRequestedIDs is the
// scoping half of the scoped variant: a run over one file's id must
// fill that file's playlist entries and leave every other entry alone,
// because the incremental scan calls this once per orphan batch and a
// pass that rewrote the whole table would touch every playlist row on
// every scan.
func TestPreservePlaylistPhantomsForFilesScopesToTheRequestedIDs(t *testing.T) {
ctx := context.Background()
db := openRaw(t, t.TempDir())
if _, err := db.ExecContext(ctx, "PRAGMA foreign_keys = ON"); err != nil {
t.Fatalf("pragma: %v", err)
}
if err := applySchema(ctx, db); err != nil {
t.Fatalf("applySchema: %v", err)
}
if _, err := db.ExecContext(ctx, `
INSERT INTO playlists (id, name) VALUES (1, 'keepme');
INSERT INTO libraries (id, name, path) VALUES (0, 'test', '/music');
INSERT INTO artists (id, name) VALUES (3, 'Aurora Fields');
INSERT INTO cover_art (id, file_path, mime_type)
VALUES (9, 'covers/7.jpg', 'image/jpeg');
INSERT INTO albums (id, name, artist_id, cover_art_id)
VALUES (4, 'Tideline', 3, 9);
INSERT INTO audio_files
(id, file_path, file_type_id, length_milliseconds,
title, artist_credit, artist_id, album_id)
VALUES
(7, '/music/a.flac', 1, 1000,
'Slack Water', 'Aurora Fields', 3, 4),
(8, '/music/b.flac', 1, 2000,
'Second Tide', 'Aurora Fields', 3, 4);
INSERT INTO playlist_tracks (playlist_id, audio_file_id, position)
VALUES (1, 7, 0), (1, 8, 1);
`); err != nil {
t.Fatalf("seed: %v", err)
}
tx, err := db.BeginTx(ctx, nil)
if err != nil {
t.Fatalf("begin: %v", err)
}
defer func() { _ = tx.Rollback() }()
if err := PreservePlaylistPhantomsForFiles(
ctx, tx, []int64{7}, testLogger(),
); err != nil {
t.Fatalf("preserve: %v", err)
}
if err := tx.Commit(); err != nil {
t.Fatalf("commit: %v", err)
}
var filled, untouched sql.NullString
if err := db.QueryRowContext(ctx,
"SELECT phantom_file_path FROM playlist_tracks WHERE audio_file_id = 7",
).Scan(&filled); err != nil {
t.Fatalf("read the requested entry: %v", err)
}
if filled.String != "/music/a.flac" {
t.Errorf(
"requested entry phantom_file_path = %q, want %q",
filled.String, "/music/a.flac",
)
}
if err := db.QueryRowContext(ctx,
"SELECT phantom_file_path FROM playlist_tracks WHERE audio_file_id = 8",
).Scan(&untouched); err != nil {
t.Fatalf("read the untouched entry: %v", err)
}
if untouched.Valid {
t.Errorf(
"untouched entry got phantom_file_path = %q, want NULL "+
"(a scoped run must not rewrite the whole table)",
untouched.String,
)
}
}
+72
View File
@@ -3,6 +3,7 @@ package library
import (
"bytes"
"crypto/sha256"
"database/sql"
"encoding/hex"
"fmt"
"image"
@@ -69,6 +70,77 @@ func CoverArtFileSet(coverPath string) []string {
return paths
}
// sweepOrphanedCoverArt deletes the cover_art rows no album references
// and returns their file paths, for the caller to remove from disk
// after the transaction commits. Cover art is referenced only by
// albums.cover_art_id, so an orphan is a cover whose album is gone —
// which is every album the caller just swept.
//
// One implementation because the scan path, RemoveFromLibrary and
// RemoveLibrary all reach this state, and the scan side used to skip it
// entirely while RemoveLibrary did it inline (#247).
func (l *Library) sweepOrphanedCoverArt(tx *sql.Tx) ([]string, error) {
const orphanSQL = `
SELECT file_path FROM cover_art WHERE id NOT IN (
SELECT DISTINCT cover_art_id FROM albums
WHERE cover_art_id IS NOT NULL
)`
rows, err := tx.QueryContext(l.ctx, orphanSQL)
if err != nil {
return nil, fmt.Errorf("could not query orphaned cover art: %w", err)
}
var paths []string
for rows.Next() {
var filePath string
if err := rows.Scan(&filePath); err != nil {
l.logger.Warn("could not scan cover art path", "err", err)
continue
}
paths = append(paths, filePath)
}
// Close before the DELETE: the two run on the one writer connection.
if err := rows.Close(); err != nil {
l.logger.Warn("could not close cover art rows", "err", err)
}
if len(paths) > 0 {
if _, err := tx.ExecContext(l.ctx, `
DELETE FROM cover_art WHERE id NOT IN (
SELECT DISTINCT cover_art_id FROM albums
WHERE cover_art_id IS NOT NULL
)`); err != nil {
return nil, fmt.Errorf("could not delete orphaned cover_art: %w", err)
}
}
return paths, nil
}
// removeCoverArtFiles removes a cover original and its derived size
// variants. Only the original is stored in cover_art.file_path; the
// _sm/_md/_lg tiers are derived filenames beside it, so they have to be
// removed by name or they accumulate forever.
func (l *Library) removeCoverArtFiles(coverPaths []string) {
for _, coverPath := range coverPaths {
for _, path := range CoverArtFileSet(coverPath) {
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
l.logger.Warn(
"could not remove orphaned cover art file",
"path", path,
"err", err,
)
}
}
}
}
// saveCoverArt saves embedded cover art to the cache directory.
// Returns the file path where the art was saved, or empty string
// if no picture data. Timing is recorded in the provided metrics.
+8 -50
View File
@@ -320,43 +320,12 @@ func (l *Library) RemoveLibrary(id int64) (*RemovalSummary, error) {
genresRemoved, _ := result.RowsAffected()
// 15. Collect orphaned cover_art file paths for post-commit cleanup.
// SAFETY: Hand-crafted SELECT for orphaned cover art identification.
// Parameterless.
rows, err := tx.QueryContext(l.ctx,
`SELECT file_path FROM cover_art WHERE id NOT IN (
SELECT DISTINCT cover_art_id FROM albums
WHERE cover_art_id IS NOT NULL
)`)
// Collect and delete orphaned cover_art rows before the commit. The
// shared helper is the one place this sweep lives, so the scan path,
// RemoveFromLibrary and this removal cannot drift (#247).
orphanedCoverArtPaths, err := l.sweepOrphanedCoverArt(tx)
if err != nil {
return nil, fmt.Errorf("could not query orphaned cover art: %w", err)
}
var orphanedCoverArtPaths []string
for rows.Next() {
var filePath string
if err := rows.Scan(&filePath); err != nil {
l.logger.Warn("could not scan cover art path", "err", err)
continue
}
orphanedCoverArtPaths = append(orphanedCoverArtPaths, filePath)
}
if err := rows.Close(); err != nil {
l.logger.Warn("could not close cover art rows", "err", err)
}
// 16. Delete orphaned cover_art rows.
// SAFETY: Hand-crafted orphan cleanup SQL. Parameterless.
if _, err := tx.ExecContext(l.ctx,
`DELETE FROM cover_art WHERE id NOT IN (
SELECT DISTINCT cover_art_id FROM albums
WHERE cover_art_id IS NOT NULL
)`); err != nil {
return nil, fmt.Errorf("could not delete orphaned cover_art: %w", err)
return nil, err
}
// 17. Delete the library's tagging queue. tagging_items holds a
@@ -392,20 +361,9 @@ func (l *Library) RemoveLibrary(id int64) (*RemovalSummary, error) {
// avoids a costly full re-index of all remaining tracks (~10s for
// 25K tracks).
// 21. Post-commit: Delete orphaned cover art files and their sized
// variants. Only the original is stored in cover_art.file_path; the
// _sm/_md/_lg thumbnails are derived filenames beside it, so they
// have to be removed by name or they accumulate forever.
for _, coverPath := range orphanedCoverArtPaths {
for _, path := range CoverArtFileSet(coverPath) {
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
l.logger.Warn("could not remove orphaned cover art file",
"path", path,
"err", err,
)
}
}
}
// Post-commit: remove the orphaned cover art files and their sized
// variants.
l.removeCoverArtFiles(orphanedCoverArtPaths)
// 22. Post-commit: Compact queue.
if l.removalHooks.CompactQueue != nil {
+119 -32
View File
@@ -911,30 +911,96 @@ func (l *Library) scanInternal(
orphanStart := time.Now()
// Snapshot the orphan set first. The playlist-phantom
// preservation and the deletes are one transaction (the
// preservation has to land before the ON DELETE SET NULL, and
// both have to succeed or neither does), and the ids are what
// scope that preservation to just these files instead of
// rewriting every playlist row on a routine scan.
var (
orphans []sqlcgen.AudioFile
orphanPaths []string
)
existingPaths.Range(func(key, value any) bool {
path := key.(string)
audioFile := value.(sqlcgen.AudioFile)
orphanPaths = append(orphanPaths, key.(string))
orphans = append(orphans, value.(sqlcgen.AudioFile))
l.logger.Debug(
"removing orphaned database entry",
"path", path, "id", audioFile.ID,
)
return true
})
if err := l.db.Queries.DeleteAudioFile(
l.ctx, audioFile.ID,
); err != nil {
l.logger.Warn(
"failed to delete orphaned audio file",
"path", path,
"id", audioFile.ID,
"err", err,
)
deleted := make([]bool, len(orphans))
metrics.addWarning(path, "orphan", err)
return true
if len(orphans) > 0 {
orphanIDs := make([]int64, len(orphans))
for i, f := range orphans {
orphanIDs[i] = f.ID
}
tx, beginErr := l.db.BeginTx()
if beginErr != nil {
metrics.addWarning("", "orphan", beginErr)
} else {
defer func() { _ = tx.Rollback() }() // no-op after commit
if err := database.PreservePlaylistPhantomsForFiles(
l.ctx, tx, orphanIDs, l.logger,
); err != nil {
// Deleting without the phantoms is exactly the
// playlist-emptying bug the preservation exists to
// prevent, so leave the rows for the next scan
// rather than empty the playlists now.
l.logger.Error(
"skipping orphan deletion: could not preserve "+
"playlist entries",
"err", err,
)
metrics.addWarning("", "orphan", err)
_ = tx.Rollback()
} else {
txq := l.db.Queries.WithTx(tx)
for i, f := range orphans {
if err := txq.DeleteAudioFile(
l.ctx, f.ID,
); err != nil {
l.logger.Warn(
"failed to delete orphaned audio file",
"path", orphanPaths[i],
"id", f.ID,
"err", err,
)
metrics.addWarning(orphanPaths[i], "orphan", err)
continue
}
deleted[i] = true
}
if err := tx.Commit(); err != nil {
l.logger.Error(
"could not commit orphan deletion",
"err", err,
)
metrics.addWarning("", "orphan", err)
}
}
}
}
// Post-commit bookkeeping for the files that actually went.
for i, f := range orphans {
if !deleted[i] {
continue
}
path := orphanPaths[i]
// Keep the file's tagging group in sync: drop the group's
// track count and clear it out once empty, mirroring the
// bookkeeping maybeRebindTaggingGroup does for a group_key
@@ -942,25 +1008,25 @@ func (l *Library) scanInternal(
// and replaced leaves a stale tagging_items row behind —
// its track_count still counts the deleted files, and it
// never clears from the autotag queue.
if audioFile.GroupKey != "" {
if f.GroupKey != "" {
if err := l.db.Queries.DecrementTaggingItemTrackCount(
l.ctx, audioFile.GroupKey,
l.ctx, f.GroupKey,
); err != nil {
l.logger.Warn(
"failed to decrement tagging group for orphan",
"path", path,
"group_key", audioFile.GroupKey,
"group_key", f.GroupKey,
"err", err,
)
metrics.addWarning(path, "orphan", err)
} else if err := l.db.Queries.DeleteTaggingItemIfEmpty(
l.ctx, audioFile.GroupKey,
l.ctx, f.GroupKey,
); err != nil {
l.logger.Warn(
"failed to clean up emptied tagging group for orphan",
"path", path,
"group_key", audioFile.GroupKey,
"group_key", f.GroupKey,
"err", err,
)
@@ -968,13 +1034,21 @@ func (l *Library) scanInternal(
}
}
// Remove from FTS5 search index.
if err := l.db.DeleteSearchIndex(
audioFile.ID,
); err != nil {
// Remove from FTS5 search index and the lyrics index.
if err := l.db.DeleteSearchIndex(f.ID); err != nil {
l.logger.Warn(
"failed to delete FTS entry for orphan",
"id", audioFile.ID,
"id", f.ID,
"err", err,
)
metrics.addWarning(path, "orphan", err)
}
if err := l.db.DeleteLyricsIndex(f.ID); err != nil {
l.logger.Warn(
"failed to delete lyrics index entry for orphan",
"id", f.ID,
"err", err,
)
@@ -982,9 +1056,7 @@ func (l *Library) scanInternal(
}
removed.Add(1)
return true
})
}
metrics.OrphanCleanup = time.Since(orphanStart)
@@ -1193,17 +1265,32 @@ func (l *Library) pruneEmptyEntities() {
}
}
// Cover art after albums: a cover whose album just went is
// unreferenced, and leaving the row behind keeps its files exempt
// from the janitor's covers sweep forever (#247).
orphanedCovers, err := l.sweepOrphanedCoverArt(tx)
if err != nil {
l.logger.Warn("could not sweep orphaned cover art", "err", err)
return
}
if err := tx.Commit(); err != nil {
l.logger.Warn("could not commit entity cleanup", "err", err)
return
}
if len(albumIDs) > 0 || len(artistIDs) > 0 || len(genreIDs) > 0 {
// Post-commit: the rows are gone, so their files can go too.
l.removeCoverArtFiles(orphanedCovers)
if len(albumIDs) > 0 || len(artistIDs) > 0 || len(genreIDs) > 0 ||
len(orphanedCovers) > 0 {
l.logger.Info("pruned empty library entities",
"albums", len(albumIDs),
"artists", len(artistIDs),
"genres", len(genreIDs),
"covers", len(orphanedCovers),
)
}
}
+101
View File
@@ -94,6 +94,57 @@ func countRows(
return n
}
// RemoveFromLibrary is the one path that empties a track *deliberately*:
// the file stays on disk but is excluded, so nothing re-imports it. The
// playlist entry must still survive as a re-linkable phantom rather than
// an empty row, because a later full rescan clears the exclusion and is
// what re-links the entry then (#246).
func TestRemoveFromLibrary_PreservesPlaylistPhantoms(t *testing.T) {
t.Parallel()
lib, db := setupTestLibrary(t)
seedRemovableLibrary(t, lib, "/nonexistent/cover.jpg")
if _, err := lib.db.ExecContext(
`INSERT INTO playlists (name) VALUES ('keepme')`,
); err != nil {
t.Fatalf("seed playlist: %v", err)
}
playlistID := queryInt(
t, db, `SELECT id FROM playlists WHERE name = 'keepme'`,
)
if _, err := lib.db.ExecContext(
`INSERT INTO playlist_tracks (playlist_id, audio_file_id, position)
SELECT ?, id, 0 FROM audio_files
WHERE file_path = '/music/song.mp3'`,
playlistID,
); err != nil {
t.Fatalf("seed playlist_tracks: %v", err)
}
if _, err := lib.RemoveFromLibrary([]string{"/music/song.mp3"}); err != nil {
t.Fatalf("RemoveFromLibrary: %v", err)
}
phantomPath := queryString(
t, db,
`SELECT phantom_file_path FROM playlist_tracks
WHERE playlist_id = ?`,
playlistID,
)
if phantomPath != "/music/song.mp3" {
t.Fatalf(
"phantom_file_path is %q, want %q -- the entry cannot be "+
"re-linked after a later full rescan",
phantomPath, "/music/song.mp3",
)
}
}
// A library with tagging_items must still be removable. tagging_items
// FK-references libraries with no ON DELETE clause, so leaving those
// rows behind fails the DELETE and rolls back the entire removal.
@@ -198,3 +249,53 @@ func TestCoverArtFileSet(t *testing.T) {
}
}
}
// Removing the last track of an album must take the album's cover art
// with it — both the row and every derived file — or the row keeps its
// files exempt from the janitor's covers sweep forever (#247).
func TestRemoveFromLibrary_DeletesOrphanedCoverArt(t *testing.T) {
t.Parallel()
lib, _ := setupTestLibrary(t)
dir := t.TempDir()
// The largest tier is what cover_art.file_path names; write every
// variant so the sweep has a real set to remove.
for _, tier := range thumbnailTiers {
p := filepath.Join(dir, coverart.SizedFilename("abc123.jpg", tier.Suffix))
if err := os.WriteFile(p, []byte("img"), 0o600); err != nil {
t.Fatalf("write %s: %v", p, err)
}
}
cover := filepath.Join(dir, coverart.SizedFilename("abc123.jpg", "_lg"))
seedRemovableLibrary(t, lib, cover)
// Link the album to the cover so it is not orphaned until the track
// (and with it the album) goes.
if _, err := lib.db.ExecContext(
`UPDATE albums SET cover_art_id =
(SELECT id FROM cover_art WHERE file_path = ?)
WHERE name = 'Test Album'`,
cover,
); err != nil {
t.Fatalf("link cover art: %v", err)
}
if _, err := lib.RemoveFromLibrary([]string{"/music/song.mp3"}); err != nil {
t.Fatalf("RemoveFromLibrary: %v", err)
}
if n := countRows(t, lib, "cover_art"); n != 0 {
t.Errorf("cover_art has %d rows after removal, want 0", n)
}
for _, tier := range thumbnailTiers {
p := filepath.Join(dir, coverart.SizedFilename("abc123.jpg", tier.Suffix))
if _, err := os.Stat(p); !os.IsNotExist(err) {
t.Errorf("cover art file still present: %s", filepath.Base(p))
}
}
}
+25
View File
@@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"yellowjacket/backend/database"
"yellowjacket/backend/events"
)
@@ -57,6 +58,25 @@ func (l *Library) RemoveFromLibrary(filePaths []string) (*RemovalResult, error)
var result RemovalResult
// Preserve the playlist entries before the rows go, so they survive
// as re-linkable phantoms rather than empty rows. The track is
// excluded and will not be re-imported on its own, but a later full
// rescan clears the exclusion and this is what lets the entry
// re-link then — the same preservation every other path that empties
// audio_files performs (#246).
rowIDs := make([]int64, len(rows))
for i, row := range rows {
rowIDs[i] = row.ID
}
if err := database.PreservePlaylistPhantomsForFiles(
l.ctx, tx, rowIDs, l.logger,
); err != nil {
return nil, fmt.Errorf(
"could not preserve playlist entries for removal: %w", err,
)
}
// Exclude every path the caller named, including one whose row has
// already gone: the user asked for that file to stay out, and a row
// that disappeared between the click and the commit is not a reason
@@ -127,6 +147,11 @@ func (l *Library) RemoveFromLibrary(filePaths []string) (*RemovalResult, error)
l.logger.Warn("could not delete FTS entry for removed track",
"path", row.FilePath, "id", row.ID, "err", err)
}
if err := l.db.DeleteLyricsIndex(row.ID); err != nil {
l.logger.Warn("could not delete lyrics index entry for removed track",
"path", row.FilePath, "id", row.ID, "err", err)
}
}
// Deleting an audio_files row cascades to queue_tracks, so the
+95
View File
@@ -231,3 +231,98 @@ func TestScan_MultipleDirectoriesDoNotCrossContaminate(t *testing.T) {
t.Errorf("Album A and Album B must not share a group_key: %+v", keys)
}
}
// TestScan_OrphanCleanupPreservesPlaylistPhantoms guards #246: a file
// deleted from the library folder *outside* YellowJacket is discovered
// as an orphan by the next scan, and its playlist entry must survive as
// a re-linkable phantom — the same preservation the full rescan and
// stale-retire paths already perform, scoped here to just the orphaned
// file. Before the fix the entry became an empty row (audio_file_id
// NULL and no phantom_file_path), which nothing can ever re-link.
func TestScan_OrphanCleanupPreservesPlaylistPhantoms(t *testing.T) {
t.Parallel()
lib, db := setupTestLibrary(t)
root := t.TempDir()
track := filepath.Join(root, "gone.mp3")
writeTestTrack(t, track, 0)
library, err := db.Queries.CreateLibrary(lib.ctx, sqlcgen.CreateLibraryParams{
Name: "orphans",
Path: root,
})
if err != nil {
t.Fatalf("create library: %v", err)
}
if metrics := lib.scanInternal(library.ID, library.Name, library.Path); metrics == nil {
t.Fatal("first scan returned nil metrics")
}
trackID := queryInt(
t, db, "SELECT id FROM audio_files WHERE file_path = ?", track,
)
if trackID == 0 {
t.Fatal("first scan did not import the track")
}
if _, err := db.ExecContext(
`INSERT INTO playlists (name) VALUES ('keepme')`,
); err != nil {
t.Fatalf("seed playlist: %v", err)
}
playlistID := queryInt(
t, db, "SELECT id FROM playlists WHERE name = 'keepme'",
)
if _, err := db.ExecContext(
`INSERT INTO playlist_tracks (playlist_id, audio_file_id, position)
VALUES (?, ?, 0)`,
playlistID, trackID,
); err != nil {
t.Fatalf("seed playlist_tracks: %v", err)
}
// The file goes away outside the app.
if err := os.Remove(track); err != nil {
t.Fatalf("remove track: %v", err)
}
if metrics := lib.scanInternal(library.ID, library.Name, library.Path); metrics == nil {
t.Fatal("second scan returned nil metrics")
}
if n := queryInt(
t, db, "SELECT COUNT(*) FROM audio_files WHERE file_path = ?", track,
); n != 0 {
t.Fatalf("audio_files still holds the removed path: %d rows", n)
}
if n := queryInt(
t, db,
"SELECT COUNT(*) FROM playlist_tracks WHERE playlist_id = ? "+
"AND audio_file_id IS NULL",
playlistID,
); n != 1 {
t.Fatalf(
"playlist entry did not become a phantom: %d null-id rows, want 1",
n,
)
}
phantomPath := queryString(
t, db,
"SELECT phantom_file_path FROM playlist_tracks WHERE playlist_id = ?",
playlistID,
)
if phantomPath != track {
t.Fatalf(
"phantom_file_path = %q, want %q -- the entry cannot be "+
"re-linked if the file comes back",
phantomPath, track,
)
}
}
+117
View File
@@ -666,3 +666,120 @@ func TestExpiredHTTPCacheJob_TrimsToBudget(t *testing.T) {
t.Errorf("kept %q, want the longest-lived row", kept)
}
}
// TestStaleArtistMetadataJob pins the sweep's two keep rules: an owned
// artist's metadata survives, a browsed artist's survives while it still
// holds cached artwork, and everything else goes (#248).
func TestStaleArtistMetadataJob(t *testing.T) {
t.Parallel()
db := database.NewTestDB(t)
const (
ownedMBID = "11111111-1111-1111-1111-111111111111"
browsedMBID = "22222222-2222-2222-2222-222222222222"
staleMBID = "33333333-3333-3333-3333-333333333333"
)
// The owned artist is in the library - which means a *file* says
// so. An artists row on its own is not ownership.
database.InsertTestTrack(t, db, database.TestTrack{
FilePath: "/music/owned.mp3",
Artist: "Owned",
ArtistMBID: ownedMBID,
})
for _, mbid := range []string{ownedMBID, browsedMBID, staleMBID} {
if _, err := db.ExecContext(
`INSERT INTO artist_metadata (mbid, source, data, fetched_at)
VALUES (?, 'wikidata-p18', x'00', CURRENT_TIMESTAMP)`,
mbid,
); err != nil {
t.Fatalf("seed artist_metadata for %s: %v", mbid, err)
}
}
// The browsed artist holds cached artwork, so its metadata is still
// referenced and must survive.
if _, err := db.ExecContext(
`INSERT INTO artist_images
(artist_mbid, source, source_url, file_path)
VALUES (?, 'test', 'http://x', '/art/primary.jpg')`,
browsedMBID,
); err != nil {
t.Fatalf("seed artist_images: %v", err)
}
if _, err := StaleArtistMetadataJob(db).Run(context.Background()); err != nil {
t.Fatalf("run job: %v", err)
}
for _, tc := range []struct {
mbid string
want int
}{
{ownedMBID, 1},
{browsedMBID, 1},
{staleMBID, 0},
} {
var n int
if err := db.QueryRowWriter(
"SELECT COUNT(*) FROM artist_metadata WHERE mbid = ?", tc.mbid,
).Scan(&n); err != nil {
t.Fatalf("count %s: %v", tc.mbid, err)
}
if n != tc.want {
t.Errorf("artist_metadata rows for %s = %d, want %d", tc.mbid, n, tc.want)
}
}
}
// TestStaleSearchClicksJob deletes only the clicks old enough to have
// left the retention window (#249).
func TestStaleSearchClicksJob(t *testing.T) {
t.Parallel()
db := database.NewTestDB(t)
count := func(mbid string) int {
t.Helper()
var n int
if err := db.QueryRowWriter(
"SELECT COUNT(*) FROM search_clicks WHERE entity_mbid = ?", mbid,
).Scan(&n); err != nil {
t.Fatalf("count %s: %v", mbid, err)
}
return n
}
seed := func(query, mbid, lastClicked string) {
t.Helper()
if _, err := db.ExecContext(
`INSERT INTO search_clicks
(query, entity_mbid, entity_type, click_count, last_clicked)
VALUES (?, ?, 'recording', 1, ?)`,
query, mbid, lastClicked,
); err != nil {
t.Fatalf("seed search_clicks: %v", err)
}
}
seed("tide", "aaaa", "2024-01-01 00:00:00") // stale
seed("tide", "bbbb", "2999-01-01 00:00:00") // recent
if _, err := StaleSearchClicksJob(db).Run(context.Background()); err != nil {
t.Fatalf("run job: %v", err)
}
if n := count("bbbb"); n != 1 {
t.Errorf("recent click was deleted: %d rows, want 1", n)
}
if n := count("aaaa"); n != 0 {
t.Errorf("stale click survived: %d rows, want 0", n)
}
}
+66
View File
@@ -628,3 +628,69 @@ func dirSize(dir string) (bytes, files int64) {
return bytes, files
}
// StaleArtistMetadataJob evicts long-lived artist metadata (bios, wiki
// leads, relationships) for artists the user no longer has any reason
// to keep around: not owned and holding no cached artwork.
//
// artist_metadata has no TTL by design — entity data changes rarely and
// re-fetching spends someone else's rate limit — so without a sweep it
// grows for the life of the install. This is the "swept when the
// artist is no longer referenced" contract the datamap always declared
// for it and nothing ever performed (#248).
func StaleArtistMetadataJob(db *database.DB) Job {
return Job{
Name: "artist-metadata-sweep",
MinInterval: dailyInterval,
Run: func(_ context.Context) (Result, error) {
res, err := db.ExecContext(
`DELETE FROM artist_metadata
WHERE mbid NOT IN (` + ownedArtistMBIDs + `)
AND mbid NOT IN (
SELECT artist_mbid FROM artist_images
)`,
)
if err != nil {
return Result{}, fmt.Errorf(
"delete stale artist_metadata rows: %w", err,
)
}
rows, _ := res.RowsAffected()
return Result{RowsDeleted: rows}, nil
},
}
}
// searchClicksRetention is how long a search-click ranking signal stays
// useful. search_clicks is authored behavioural data — nothing that
// owns a row ever drops it — so age is the ceiling that keeps the table
// from growing without bound for the life of the install (#249).
const searchClicksRetention = "-180 days"
// StaleSearchClicksJob deletes search-click ranking rows older than the
// retention window. Rows are small and the table grows slowly, so this
// runs daily and does almost nothing most runs.
func StaleSearchClicksJob(db *database.DB) Job {
return Job{
Name: "search-clicks-sweep",
MinInterval: dailyInterval,
Run: func(_ context.Context) (Result, error) {
res, err := db.ExecContext(
`DELETE FROM search_clicks
WHERE last_clicked < datetime('now', ?)`,
searchClicksRetention,
)
if err != nil {
return Result{}, fmt.Errorf(
"delete stale search_clicks rows: %w", err,
)
}
rows, _ := res.RowsAffected()
return Result{RowsDeleted: rows}, nil
},
}
}
+28
View File
@@ -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()
+15
View File
@@ -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.
+32
View File
@@ -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)
}
}
@@ -56,6 +56,16 @@ export function CycleRepeat(): $CancellablePromise<void> {
return $Call.ByID(3510519482);
}
/**
* DropSourceForPlaylist clears the queue's "Playing from" label when
* its source playlist is deleted. A link back to a playlist that no
* longer exists is worse than none, and the label otherwise survives
* the deletion until the next SetQueue (#249).
*/
export function DropSourceForPlaylist(playlistID: number): $CancellablePromise<void> {
return $Call.ByID(1435106374, playlistID);
}
/**
* EmitCurrentState emits the current queue state to the frontend.
* This is called after the frontend DOM is ready.