diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index 9c577762..9bf00598 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -178,7 +178,7 @@ func run() error { srv := server.New(logger, pool, scanner, subsonic.Config{ AllowPlaintextPassword: cfg.Subsonic.AllowPlaintextPassword, }, cfg.Events, cfg.Recommendation, cfg.Storage.DataDir, cfg.Branding, coverEnricher, cfg.Library.CoverArtBackfillCap, coverSettings, scanner, scanCfg, scheduler) - playlists.StartSystemPlaylistCron(ctx, pool, logger.With("component", "system_playlist_cron")) + playlists.StartSystemPlaylistCron(ctx, pool, logger.With("component", "system_playlist_cron"), cfg.Storage.DataDir) httpServer := &http.Server{ Addr: cfg.Server.Address, Handler: srv.Router(), diff --git a/internal/api/playlists.go b/internal/api/playlists.go index f7017453..8673f2c3 100644 --- a/internal/api/playlists.go +++ b/internal/api/playlists.go @@ -158,7 +158,7 @@ func (h *handlers) maybeKickBackgroundBuild(ctx context.Context, userID pgtype.U } go func() { bgCtx := context.Background() - if err := playlists.BuildSystemPlaylists(bgCtx, h.pool, h.logger, userID, time.Now()); err != nil { + if err := playlists.BuildSystemPlaylists(bgCtx, h.pool, h.logger, userID, time.Now(), h.dataDir); err != nil { h.logger.Warn("lazy system playlist build failed", "user_id", "redacted", "err", err) } diff --git a/internal/playlists/system.go b/internal/playlists/system.go index 5b5aedb1..5d131586 100644 --- a/internal/playlists/system.go +++ b/internal/playlists/system.go @@ -103,7 +103,7 @@ func noopRNG() float64 { return 0 } // Callable from both the cron loop (system_cron.go) and the lazy fallback. // Returns nil when the user has no library activity to build from. Errors // are captured in system_playlist_runs.last_error before returning. -func BuildSystemPlaylists(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, userID pgtype.UUID, now time.Time) error { +func BuildSystemPlaylists(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, userID pgtype.UUID, now time.Time, dataDir string) error { q := dbq.New(pool) dateStr := now.UTC().Format("2006-01-02") dateOnly := pgtype.Date{Time: now.UTC(), Valid: true} @@ -184,7 +184,6 @@ func BuildSystemPlaylists(ctx context.Context, pool *pgxpool.Pool, logger *slog. type seedMix struct { ArtistID pgtype.UUID ArtistName string - CoverPath *string Tracks []rankedCandidate } seedMixes := make([]seedMix, 0, len(seeds)) @@ -220,13 +219,9 @@ func BuildSystemPlaylists(ctx context.Context, pool *pgxpool.Pool, logger *slog. } } tracks := pickTopN(filtered, dateStr, now, systemMixLength) - coverPath, _ := q.PickTopAlbumCoverForArtistByUser(ctx, dbq.PickTopAlbumCoverForArtistByUserParams{ - UserID: userID, ArtistID: artistID, - }) seedMixes = append(seedMixes, seedMix{ ArtistID: artistID, ArtistName: artistRow.Name, - CoverPath: coverPath, Tracks: tracks, }) } @@ -245,12 +240,20 @@ func BuildSystemPlaylists(ctx context.Context, pool *pgxpool.Pool, logger *slog. return buildErr } + // Track the playlist IDs we create so we can generate collage covers + // for each one after the transaction commits. Collage generation + // reads the playlist's tracks via a separate query that won't see + // uncommitted rows, so the work happens post-commit. + createdIDs := make([]pgtype.UUID, 0, 1+len(seedMixes)) + if len(forYouTracks) > 0 { - if err := insertSystemPlaylist(ctx, qtx, userID, "For You", "for_you", - pgtype.UUID{}, nil, forYouTracks); err != nil { + id, err := insertSystemPlaylist(ctx, qtx, userID, "For You", "for_you", + pgtype.UUID{}, forYouTracks) + if err != nil { buildErr = fmt.Errorf("insert for-you: %w", err) return buildErr } + createdIDs = append(createdIDs, id) } for _, m := range seedMixes { @@ -258,17 +261,35 @@ func BuildSystemPlaylists(ctx context.Context, pool *pgxpool.Pool, logger *slog. continue } name := fmt.Sprintf("Songs like %s", m.ArtistName) - if err := insertSystemPlaylist(ctx, qtx, userID, name, "songs_like_artist", - m.ArtistID, m.CoverPath, m.Tracks); err != nil { + id, err := insertSystemPlaylist(ctx, qtx, userID, name, "songs_like_artist", + m.ArtistID, m.Tracks) + if err != nil { buildErr = fmt.Errorf("insert songs-like %s: %w", uuidStringPL(m.ArtistID), err) return buildErr } + createdIDs = append(createdIDs, id) } if err := tx.Commit(ctx); err != nil { buildErr = fmt.Errorf("commit tx: %w", err) return buildErr } + + // Post-commit: generate a 4-cell collage cover for each system + // playlist from the contributing tracks' album covers. The collage + // gracefully falls back to the FabledSword glyph in cells whose + // album lacks cover art, so a partially-covered library still + // produces a sensible visual rather than an empty placeholder. + // Errors are non-fatal — the playlist is already committed; a + // missing collage just leaves cover_path NULL until next build. + if dataDir != "" { + for _, plID := range createdIDs { + if _, cerr := GenerateCollage(ctx, pool, plID, dataDir); cerr != nil { + logger.Warn("system playlist: collage generation failed", + "playlist_id", uuidStringPL(plID), "err", cerr) + } + } + } return nil } @@ -289,23 +310,29 @@ func pickTopN(cands []recommendation.Candidate, dateStr string, now time.Time, n // insertSystemPlaylist inserts a kind='system' playlists row plus its // playlist_tracks via AppendPlaylistTrack (slice 1's existing query, which -// does the snapshot lookup from tracks/albums/artists). Then recomputes rollups. +// does the snapshot lookup from tracks/albums/artists). Then recomputes +// rollups. Returns the new playlist's ID so the caller can drive +// post-commit collage generation. // // Pass seed_artist_id with Valid=false (zero pgtype.UUID) for the For-You // variant; the CreateSystemPlaylist query persists NULL. +// +// cover_path is always inserted as NULL — the post-commit collage step +// in BuildSystemPlaylists overrides it with a 4-cell composite of the +// contributing tracks' album art. func insertSystemPlaylist(ctx context.Context, qtx *dbq.Queries, userID pgtype.UUID, - name, variant string, seedArtistID pgtype.UUID, coverPath *string, - tracks []rankedCandidate) error { + name, variant string, seedArtistID pgtype.UUID, + tracks []rankedCandidate) (pgtype.UUID, error) { variantStr := variant p, err := qtx.CreateSystemPlaylist(ctx, dbq.CreateSystemPlaylistParams{ UserID: userID, Name: name, SystemVariant: &variantStr, SeedArtistID: seedArtistID, - CoverPath: coverPath, + CoverPath: nil, }) if err != nil { - return fmt.Errorf("insert playlist row: %w", err) + return pgtype.UUID{}, fmt.Errorf("insert playlist row: %w", err) } for _, t := range tracks { @@ -318,14 +345,14 @@ func insertSystemPlaylist(ctx context.Context, qtx *dbq.Queries, userID pgtype.U if errors.Is(err, pgx.ErrNoRows) { continue } - return fmt.Errorf("append track %s: %w", uuidStringPL(t.TrackID), err) + return pgtype.UUID{}, fmt.Errorf("append track %s: %w", uuidStringPL(t.TrackID), err) } } if err := qtx.UpdatePlaylistRollups(ctx, p.ID); err != nil { - return fmt.Errorf("update rollups: %w", err) + return pgtype.UUID{}, fmt.Errorf("update rollups: %w", err) } - return nil + return p.ID, nil } // uuidStringPL renders a pgtype.UUID as the canonical 8-4-4-4-12 form. diff --git a/internal/playlists/system_cron.go b/internal/playlists/system_cron.go index df832130..8ac11426 100644 --- a/internal/playlists/system_cron.go +++ b/internal/playlists/system_cron.go @@ -23,14 +23,14 @@ import ( // // Per-user errors are logged but don't halt the loop. Per-cycle errors (e.g., // "list active users" failed) are logged but the next tick still fires. -func StartSystemPlaylistCron(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) { +func StartSystemPlaylistCron(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string) { go func() { q := dbq.New(pool) if err := q.ClearStaleSystemPlaylistInFlight(ctx); err != nil { logger.Warn("system playlist cron: startup recovery failed", "err", err) } - runOnce(ctx, pool, logger) + runOnce(ctx, pool, logger, dataDir) ticker := time.NewTicker(24 * time.Hour) defer ticker.Stop() @@ -39,13 +39,13 @@ func StartSystemPlaylistCron(ctx context.Context, pool *pgxpool.Pool, logger *sl case <-ctx.Done(): return case <-ticker.C: - runOnce(ctx, pool, logger) + runOnce(ctx, pool, logger, dataDir) } } }() } -func runOnce(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) { +func runOnce(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string) { q := dbq.New(pool) users, err := q.ListActiveUsersForSystemPlaylists(ctx) if err != nil { @@ -53,7 +53,7 @@ func runOnce(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) { return } for _, userID := range users { - if err := BuildSystemPlaylists(ctx, pool, logger, userID, time.Now()); err != nil { + if err := BuildSystemPlaylists(ctx, pool, logger, userID, time.Now(), dataDir); err != nil { logger.Warn("system playlist build failed in cron", "user_id", uuidStringPL(userID), "err", err) } diff --git a/internal/playlists/system_test.go b/internal/playlists/system_test.go index 0f58ba52..ee67c746 100644 --- a/internal/playlists/system_test.go +++ b/internal/playlists/system_test.go @@ -71,7 +71,7 @@ func TestBuildSystemPlaylists_SufficientActivity(t *testing.T) { pool := newPool(t) logger := discardLogger() u, _ := seedActiveLibrary(t, pool, "act1", 4, 3) - if err := playlists.BuildSystemPlaylists(context.Background(), pool, logger, u.ID, time.Now().UTC()); err != nil { + if err := playlists.BuildSystemPlaylists(context.Background(), pool, logger, u.ID, time.Now().UTC(), t.TempDir()); err != nil { t.Fatalf("build: %v", err) } @@ -131,7 +131,7 @@ func TestBuildSystemPlaylists_QuarantineExcluded(t *testing.T) { quarantined := tracks[0] seedQuarantine(t, pool, u.ID, quarantined.ID) - if err := playlists.BuildSystemPlaylists(context.Background(), pool, logger, u.ID, time.Now().UTC()); err != nil { + if err := playlists.BuildSystemPlaylists(context.Background(), pool, logger, u.ID, time.Now().UTC(), t.TempDir()); err != nil { t.Fatalf("build: %v", err) } @@ -165,7 +165,7 @@ func TestBuildSystemPlaylists_AtomicReplace(t *testing.T) { ctx := context.Background() // First build. - if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, time.Now().UTC()); err != nil { + if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, time.Now().UTC(), t.TempDir()); err != nil { t.Fatalf("first build: %v", err) } var ids1 []pgtype.UUID @@ -180,7 +180,7 @@ func TestBuildSystemPlaylists_AtomicReplace(t *testing.T) { } // Second build — old rows must be gone, replaced with fresh ones. - if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, time.Now().UTC()); err != nil { + if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, time.Now().UTC(), t.TempDir()); err != nil { t.Fatalf("second build: %v", err) } rows2, _ := dbq.New(pool).ListPlaylistsByUserAndKind(ctx, dbq.ListPlaylistsByUserAndKindParams{ @@ -204,8 +204,9 @@ func TestBuildSystemPlaylists_Concurrency(t *testing.T) { var wg sync.WaitGroup wg.Add(2) - go func() { defer wg.Done(); _ = playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, now) }() - go func() { defer wg.Done(); _ = playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, now) }() + dataDir := t.TempDir() + go func() { defer wg.Done(); _ = playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, now, dataDir) }() + go func() { defer wg.Done(); _ = playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, now, dataDir) }() wg.Wait() // Exactly one run must have completed; in_flight must be false either way. @@ -228,12 +229,13 @@ func TestBuildSystemPlaylists_DailyNonceDeterminism(t *testing.T) { ctx := context.Background() day1 := time.Date(2026, 5, 4, 12, 0, 0, 0, time.UTC) - if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, day1); err != nil { + dataDir := t.TempDir() + if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, day1, dataDir); err != nil { t.Fatalf("build day1 first: %v", err) } snap1 := snapshotSystemTracks(t, pool, u.ID) - if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, day1); err != nil { + if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, day1, dataDir); err != nil { t.Fatalf("build day1 second: %v", err) } snap2 := snapshotSystemTracks(t, pool, u.ID)