fix(server/playlists): dedupe covers in playlist cover collage

The collage builder pulled exactly 4 tracks (in playlist position order) and rendered each track's album cover into a 2x2 cell. A playlist where the first 4 tracks share an album rendered as four identical cells of the same cover; a "Songs like X" mix where two similar artists dominated produced very similar collages. Now pulls 50 rows from the same query and drops adjacent / duplicate covers in Go before passing to the renderer. NULL and empty cover_art_paths are NOT deduped (they each render the fallback glyph in their own cell — the right behavior for a partially-covered playlist). Preserves playlist position order: the first occurrence of each unique cover wins. Five unit tests cover the behavior: drops duplicates, keeps NULLs, handles empty-string paths, preserves order, mixed null/dup case. Tangential to the For-You CTA slice but lands here as a small quality-of-life fix that benefits every playlist's collage.
2026-05-07 10:29:05 -04:00
parent b20738f925
commit ac774f09fc
2 changed files with 146 additions and 2 deletions
@@ -33,13 +33,20 @@ const (
 // "" when the playlist is empty (cover_path also cleared).
 func GenerateCollage(ctx context.Context, pool *pgxpool.Pool, playlistID pgtype.UUID, dataDir string) (string, error) {
 	q := dbq.New(pool)
-	rows, err := q.ListAllPlaylistTracksForCollage(ctx, dbq.ListAllPlaylistTracksForCollageParams{
+	// Pull more rows than cells so we can dedupe identical covers and
+	// still fill every cell. A 25-track "Songs like X" playlist that
+	// has 5 tracks each from 5 albums shouldn't render as four
+	// identical cells of the first album's cover; pulling 50 and
+	// deduping by cover_art_path gives us four distinct covers in
+	// realistic libraries.
+	allRows, err := q.ListAllPlaylistTracksForCollage(ctx, dbq.ListAllPlaylistTracksForCollageParams{
 		PlaylistID: playlistID,
-		Limit:      4,
+		Limit:      50,
 	})
 	if err != nil {
 		return "", fmt.Errorf("list collage tracks: %w", err)
 	}
+	rows := dedupeCollageRows(allRows, 4)

 	if len(rows) == 0 {
 		// Empty playlist: clear cover_path and remove any stale file.
@@ -101,6 +108,34 @@ func GenerateCollage(ctx context.Context, pool *pgxpool.Pool, playlistID pgtype.
 	return relPath, nil
 }

+// dedupeCollageRows trims the input to at most max rows such that no
+// non-empty cover_art_path appears twice. Rows with NULL or empty
+// cover_art_path are kept as-is — they each render the fallback glyph
+// in their own cell, which is the right behavior (a partially-covered
+// playlist still gets a sensible visual rather than collapsing every
+// missing cover into a single cell).
+//
+// Preserves input order (which is playlist position order from the SQL).
+func dedupeCollageRows(rows []dbq.ListAllPlaylistTracksForCollageRow, max int) []dbq.ListAllPlaylistTracksForCollageRow {
+	seen := make(map[string]bool, max)
+	out := make([]dbq.ListAllPlaylistTracksForCollageRow, 0, max)
+	for _, r := range rows {
+		if len(out) >= max {
+			break
+		}
+		if r.AlbumCoverPath == nil || *r.AlbumCoverPath == "" {
+			out = append(out, r)
+			continue
+		}
+		if seen[*r.AlbumCoverPath] {
+			continue
+		}
+		seen[*r.AlbumCoverPath] = true
+		out = append(out, r)
+	}
+	return out
+}
+
 // loadCellImage tries to load the album cover at the given path. On any
 // failure (empty path, missing file, decode error), returns the rendered
 // fallback glyph as a 300x300 image.
@@ -0,0 +1,109 @@
+package playlists
+
+import (
+	"testing"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
+)
+
+func strPtr(s string) *string { return &s }
+
+func TestDedupeCollageRows_DropsDuplicates(t *testing.T) {
+	rows := []dbq.ListAllPlaylistTracksForCollageRow{
+		{Position: 0, AlbumCoverPath: strPtr("a.jpg")},
+		{Position: 1, AlbumCoverPath: strPtr("a.jpg")}, // dup → drop
+		{Position: 2, AlbumCoverPath: strPtr("b.jpg")},
+		{Position: 3, AlbumCoverPath: strPtr("a.jpg")}, // dup → drop
+		{Position: 4, AlbumCoverPath: strPtr("c.jpg")},
+		{Position: 5, AlbumCoverPath: strPtr("d.jpg")},
+		{Position: 6, AlbumCoverPath: strPtr("e.jpg")}, // beyond max=4 → drop
+	}
+	got := dedupeCollageRows(rows, 4)
+	if len(got) != 4 {
+		t.Fatalf("len = %d, want 4", len(got))
+	}
+	wantPaths := []string{"a.jpg", "b.jpg", "c.jpg", "d.jpg"}
+	for i, w := range wantPaths {
+		if got[i].AlbumCoverPath == nil || *got[i].AlbumCoverPath != w {
+			t.Errorf("got[%d].AlbumCoverPath = %v, want %q", i, got[i].AlbumCoverPath, w)
+		}
+	}
+}
+
+func TestDedupeCollageRows_NullCoversNotDeduped(t *testing.T) {
+	// NULL covers each render the fallback glyph in their own cell.
+	// They shouldn't dedupe against each other; the playlist with all
+	// missing covers should still produce 4 cells.
+	rows := []dbq.ListAllPlaylistTracksForCollageRow{
+		{Position: 0, AlbumCoverPath: nil},
+		{Position: 1, AlbumCoverPath: nil},
+		{Position: 2, AlbumCoverPath: nil},
+		{Position: 3, AlbumCoverPath: nil},
+	}
+	got := dedupeCollageRows(rows, 4)
+	if len(got) != 4 {
+		t.Errorf("len = %d, want 4 (nil covers shouldn't dedupe)", len(got))
+	}
+}
+
+func TestDedupeCollageRows_EmptyStringCoverNotDeduped(t *testing.T) {
+	// An empty-string cover_art_path is treated the same as NULL —
+	// renders the glyph fallback. Should not dedupe.
+	empty := ""
+	rows := []dbq.ListAllPlaylistTracksForCollageRow{
+		{Position: 0, AlbumCoverPath: &empty},
+		{Position: 1, AlbumCoverPath: &empty},
+	}
+	got := dedupeCollageRows(rows, 4)
+	if len(got) != 2 {
+		t.Errorf("len = %d, want 2", len(got))
+	}
+}
+
+func TestDedupeCollageRows_PreservesPositionOrder(t *testing.T) {
+	rows := []dbq.ListAllPlaylistTracksForCollageRow{
+		{Position: 0, AlbumCoverPath: strPtr("z.jpg")},
+		{Position: 1, AlbumCoverPath: strPtr("a.jpg")},
+		{Position: 2, AlbumCoverPath: strPtr("m.jpg")},
+	}
+	got := dedupeCollageRows(rows, 4)
+	if len(got) != 3 {
+		t.Fatalf("len = %d, want 3", len(got))
+	}
+	// First-seen wins; original input order is preserved (z, a, m).
+	wantPaths := []string{"z.jpg", "a.jpg", "m.jpg"}
+	for i, w := range wantPaths {
+		if got[i].AlbumCoverPath == nil || *got[i].AlbumCoverPath != w {
+			t.Errorf("got[%d] = %v, want %q", i, got[i].AlbumCoverPath, w)
+		}
+	}
+}
+
+func TestDedupeCollageRows_MixedNullAndDups(t *testing.T) {
+	// Mixed: a few NULL covers + dup non-NULL. NULLs always pass;
+	// non-NULL dedups.
+	rows := []dbq.ListAllPlaylistTracksForCollageRow{
+		{Position: 0, AlbumCoverPath: strPtr("a.jpg")},
+		{Position: 1, AlbumCoverPath: nil},
+		{Position: 2, AlbumCoverPath: strPtr("a.jpg")}, // dup of [0] → drop
+		{Position: 3, AlbumCoverPath: nil},
+		{Position: 4, AlbumCoverPath: strPtr("b.jpg")},
+	}
+	got := dedupeCollageRows(rows, 4)
+	if len(got) != 4 {
+		t.Fatalf("len = %d, want 4", len(got))
+	}
+	// Expected: a.jpg, nil, nil, b.jpg
+	if got[0].AlbumCoverPath == nil || *got[0].AlbumCoverPath != "a.jpg" {
+		t.Errorf("got[0] = %v, want a.jpg", got[0].AlbumCoverPath)
+	}
+	if got[1].AlbumCoverPath != nil {
+		t.Errorf("got[1] = %v, want nil", got[1].AlbumCoverPath)
+	}
+	if got[2].AlbumCoverPath != nil {
+		t.Errorf("got[2] = %v, want nil", got[2].AlbumCoverPath)
+	}
+	if got[3].AlbumCoverPath == nil || *got[3].AlbumCoverPath != "b.jpg" {
+		t.Errorf("got[3] = %v, want b.jpg", got[3].AlbumCoverPath)
+	}
+}