diff --git a/.claude/settings.local.json b/.claude/settings.local.json index 434dbc6..9e45ebb 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -20,7 +20,12 @@ "Bash(npm ci *)", "Bash(git commit -m ' *)", "Bash(git reset *)", - "Bash(git restore *)" + "Bash(git restore *)", + "Bash(gh issue *)", + "PowerShell(gh issue *)", + "WebFetch(domain:github.com)", + "Bash(az account show *)", + "Bash(az account *)" ] } } diff --git a/renderer/src/components/CardRow.tsx b/renderer/src/components/CardRow.tsx index 677cf9b..9438e9b 100644 --- a/renderer/src/components/CardRow.tsx +++ b/renderer/src/components/CardRow.tsx @@ -1,8 +1,34 @@ import { getName, browseSub, getItemArt, isAlbum, isArtist, isPlaylist, isContainer, isProgram } from '../lib/itemHelpers'; +import { useArtistImage } from '../hooks/useArtistBrowse'; import { MediaCard } from './common/MediaCard'; -import type { SonosItem } from '../types/sonos'; +import type { SonosItem, SonosItemId } from '../types/sonos'; import styles from '../styles/CardRow.module.css'; +// Artist cards used the item's imageUrl, which for leaderboard-derived artists +// is undefined (we no longer seed the artist map with track album-art). Fetch +// the real artist image from the Sonos artist endpoint instead. +function ArtistMediaCard({ + item, + onOpen, +}: { + item: SonosItem; + onOpen: (item: SonosItem) => void; +}) { + const rid = (item.resource?.id ?? (typeof item.id === 'object' ? item.id : undefined)) as + | SonosItemId + | undefined; + const { data: artistImg } = useArtistImage(rid?.objectId, rid?.serviceId, rid?.accountId); + return ( + onOpen(item)} + /> + ); +} + export function CardRow({ items, isLoading, @@ -29,17 +55,21 @@ export function CardRow({ if (!items.length) return null; return (
- {items.map((item, i) => ( - onAdd(item)} - onOpen={(isAlbum(item) || isPlaylist(item) || isContainer(item) || isProgram(item) || isArtist(item)) ? () => onOpen(item) : undefined} - /> - ))} + {items.map((item, i) => + isArtist(item) ? ( + + ) : ( + onAdd(item)} + onOpen={(isAlbum(item) || isPlaylist(item) || isContainer(item) || isProgram(item)) ? () => onOpen(item) : undefined} + /> + ), + )}
); } diff --git a/renderer/src/components/LeaderboardPanel.tsx b/renderer/src/components/LeaderboardPanel.tsx index 337a30e..9781367 100644 --- a/renderer/src/components/LeaderboardPanel.tsx +++ b/renderer/src/components/LeaderboardPanel.tsx @@ -1,10 +1,11 @@ -import { useState } from 'react'; +import { useState, Fragment } from 'react'; import { useNavigate } from 'react-router-dom'; import { Info, X } from 'lucide-react'; import type { GameRankTierKey } from '../hooks/useDailyGame'; import { useStats, StatsPeriod } from '../hooks/useStats'; import { useGameRankings } from '../hooks/useDailyGame'; import { useImage } from '../hooks/useImage'; +import { useArtistImage } from '../hooks/useArtistBrowse'; import { getGameRankIcon, getGameRankInfoImage } from '../lib/gameRankAssets'; import { useResolveAndOpen } from '../hooks/useResolveAndOpen'; import styles from '../styles/LeaderboardPanel.module.css'; @@ -15,6 +16,75 @@ function CachedArt({ url, className }: { url: string | undefined; className: str return ; } +// Tries to fetch the real artist image (so YT Music shows the artist's photo) +// and falls back to whatever the event sent us — typically the track's album +// art. The Sonos artist endpoint needs `defaults` we don't store in events, so +// the fetch quietly returns null on many services; the fallback keeps the row +// looking populated instead of empty. +function ArtistAvatar({ + artistId, + serviceId, + accountId, + fallbackUrl, + className, + placeholderClassName, +}: { + artistId: string | undefined; + serviceId: string | undefined; + accountId: string | undefined; + fallbackUrl?: string; + className: string; + placeholderClassName: string; +}) { + const { data: imageUrl } = useArtistImage(artistId, serviceId, accountId); + const cached = useImage(imageUrl ?? fallbackUrl ?? null); + if (!cached) return
; + return ; +} + +// Multi-artist subtitles arrive as a single joined string (e.g. "Sonny Stitt, Kenny Garrett"). +// Rendering them as one button means resolveAndOpen searches for the whole string and never +// matches an individual artist; for album rows the button also swallows the row click via +// stopPropagation. Splitting on commas gives each artist its own link. +function ArtistLinks({ + artist, + artistId, + serviceId, + accountId, + onNavigateArtist, + onResolveArtist, +}: { + artist: string; + artistId?: string; + serviceId?: string; + accountId?: string; + onNavigateArtist: (name: string, artistId: string, serviceId: string, accountId: string) => void; + onResolveArtist: (name: string) => void; +}) { + const parts = artist.split(/,\s*/).map((s) => s.trim()).filter(Boolean); + if (parts.length === 0) return null; + const canUseDirect = parts.length === 1 && !!artistId && !!serviceId && !!accountId; + return ( + <> + {parts.map((name, i) => ( + + {i > 0 && ', '} + + + ))} + + ); +} + const PERIODS: { value: StatsPeriod; label: string }[] = [ { value: 'today', label: 'Today' }, { value: 'week', label: 'This week' }, @@ -215,27 +285,28 @@ export function LeaderboardPanel() { {t.trackName} {t.artist ? ( - + }, + }) + } + onResolveArtist={(name) => resolveAndOpen(name, 'artist', { serviceId: t.serviceId, accountId: t.accountId })} + /> ) : null} {t.artist && t.album ? ' · ' : null} {t.album ? ( @@ -275,11 +346,14 @@ export function LeaderboardPanel() { data.topArtists.map((a, i) => (
{i + 1} - {a.imageUrl ? ( - - ) : ( -
- )} + + }, + }) + } + onResolveArtist={(name) => resolveAndOpen(name, 'artist', { serviceId: a.serviceId, accountId: a.accountId })} + /> ) : null}
diff --git a/renderer/src/components/__tests__/LeaderboardPanel.test.tsx b/renderer/src/components/__tests__/LeaderboardPanel.test.tsx index 9c73318..fadc18a 100644 --- a/renderer/src/components/__tests__/LeaderboardPanel.test.tsx +++ b/renderer/src/components/__tests__/LeaderboardPanel.test.tsx @@ -70,6 +70,14 @@ vi.mock('../../hooks/useImage', () => ({ useImage: (url: string | null | undefined) => url ?? null, })); +// LeaderboardPanel renders artist avatars via useArtistImage; the tests don't +// wrap with a QueryClientProvider, so stub it to return no image. +vi.mock('../../hooks/useArtistBrowse', () => ({ + useArtistImage: () => ({ data: null }), + useArtistBrowse: () => ({ data: undefined, isLoading: false, error: null }), + artistQueryOptions: vi.fn(() => ({ queryKey: [], queryFn: async () => null })), +})); + vi.mock('../../hooks/useDailyGame', () => ({ useGameLeaderboard: () => ({ data: { gameId: 'today', scores: [] }, isLoading: false }), useGameRankings: (userName: string | null | undefined, enabled: boolean) => mockUseGameRankings(userName, enabled), @@ -513,6 +521,52 @@ describe('LeaderboardPanel', () => { }); }); + it('renders each artist as a separate link for a multi-artist album', () => { + mockLoaded({ + ...mockData, + topAlbums: [ + { + album: 'Diz N Bird', + artist: 'Sonny Stitt, Kenny Garrett', + albumId: 'alb-multi', + serviceId: 'svc1', + accountId: 'acc1', + count: 2, + }, + ], + }); + render(); + // Each artist should render as its own clickable button, not as one joined string. + expect(screen.getByRole('button', { name: 'Sonny Stitt' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Kenny Garrett' })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Sonny Stitt, Kenny Garrett' })).toBeNull(); + }); + + it('clicking a single artist in a multi-artist album resolves that individual artist', async () => { + mockLoaded({ + ...mockData, + topAlbums: [ + { + album: 'Diz N Bird', + artist: 'Sonny Stitt, Kenny Garrett', + albumId: 'alb-multi', + serviceId: 'svc1', + accountId: 'acc1', + count: 2, + }, + ], + }); + // The search returns the individual artist matching the clicked name. + mockSearchResolves({ + artist: { name: 'Kenny Garrett', objectId: 'kg-id', serviceId: 'svc1', accountId: 'acc1' }, + }); + render(); + fireEvent.click(screen.getByRole('button', { name: 'Kenny Garrett' })); + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith(expect.stringMatching(/^\/artist\//), expect.anything()); + }); + }); + it('clicking album row without albumId picks the candidate that matches the displayed artist', async () => { mockLoaded({ ...mockData, diff --git a/renderer/src/components/album/AlbumPanel.tsx b/renderer/src/components/album/AlbumPanel.tsx index 692236f..edd0296 100644 --- a/renderer/src/components/album/AlbumPanel.tsx +++ b/renderer/src/components/album/AlbumPanel.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react'; +import { Fragment, useEffect, useRef, useState } from 'react'; import { useLocation, useNavigate } from 'react-router-dom'; import { useQueryClient } from '@tanstack/react-query'; import { useImage } from '../../hooks/useImage'; @@ -7,6 +7,7 @@ import { usePlaylistBrowse } from '../../hooks/usePlaylistBrowse'; import { useDominantColor } from '../../hooks/useDominantColor'; import { useGeniusAlbumYear } from '../../hooks/useGeniusAlbumYear'; import { artistQueryOptions } from '../../hooks/useArtistBrowse'; +import { useResolveAndOpen } from '../../hooks/useResolveAndOpen'; import { resolveAlbumParams, isPlaylist, isProgram, getItemArt } from '../../lib/itemHelpers'; import { createDragGhost } from '../../lib/dragHelpers'; import { AlbumTrackRow } from './AlbumTrackRow'; @@ -22,6 +23,7 @@ export function AlbumPanel({ onAddToQueue }: Props) { const item = (state as { item?: SonosItem } | null)?.item; const navigate = useNavigate(); const queryClient = useQueryClient(); + const { resolveAndOpen } = useResolveAndOpen(); const isPlaylistOrProgram = item ? (isPlaylist(item) || isProgram(item)) : false; const { albumId, serviceId, accountId, defaults } = item @@ -106,6 +108,11 @@ export function AlbumPanel({ onAddToQueue }: Props) { navigate(`/artist/${encodeURIComponent(rid?.objectId ?? '_')}`, { state: { item: data.artistItem } }); } + // Multi-artist subtitles ("Sonny Stitt, Kenny Garrett") arrive as a single string. + // Split on commas so each artist gets its own link; only the single-artist case + // can use the cached artistItem for direct navigation. + const artistParts = artist.split(/,\s*/).map((s) => s.trim()).filter(Boolean); + return (
@@ -115,12 +122,25 @@ export function AlbumPanel({ onAddToQueue }: Props) {
{title}
- {artist && ( -
- {artist} + {artistParts.length > 0 && ( +
+ {artistParts.length === 1 && data?.artistItem ? ( + + ) : ( + artistParts.map((name, i) => ( + + {i > 0 && ', '} + + + )) + )}
)} {data && ( diff --git a/renderer/src/components/album/__tests__/AlbumPanel.test.tsx b/renderer/src/components/album/__tests__/AlbumPanel.test.tsx index 1f33395..c76573e 100644 --- a/renderer/src/components/album/__tests__/AlbumPanel.test.tsx +++ b/renderer/src/components/album/__tests__/AlbumPanel.test.tsx @@ -270,6 +270,46 @@ describe('AlbumPanel', () => { expect(mockNavigate).toHaveBeenCalledWith('/artist/art-1', expect.anything()); }); + it('renders each artist as a separate link for a multi-artist album header', () => { + mockUseAlbumBrowse.mockReturnValue({ + data: makeAlbumData({ artist: 'Sonny Stitt, Kenny Garrett', artistItem: null }), + isLoading: false, + error: null, + }); + render(, { wrapper }); + expect(screen.getByRole('button', { name: 'Sonny Stitt' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Kenny Garrett' })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Sonny Stitt, Kenny Garrett' })).toBeNull(); + }); + + it('clicking a single artist on a multi-artist album header resolves that artist via search', async () => { + mockUseAlbumBrowse.mockReturnValue({ + data: makeAlbumData({ artist: 'Sonny Stitt, Kenny Garrett', artistItem: null }), + isLoading: false, + error: null, + }); + vi.mocked(window.sonos.fetch).mockResolvedValueOnce({ + data: { + resourceOrder: ['ARTISTS'], + ARTISTS: { + resources: [ + { + id: { objectId: 'kg-id', serviceId: 'svc-1', accountId: 'acc-1' }, + name: 'Kenny Garrett', + images: [], + }, + ], + }, + }, + } as never); + const user = userEvent.setup(); + render(, { wrapper }); + await user.click(screen.getByRole('button', { name: 'Kenny Garrett' })); + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith(expect.stringMatching(/^\/artist\//), expect.anything()); + }); + }); + // ── playlist mode ───────────────────────────────────────────────────────── it('renders playlist tracks when item is a playlist', () => { diff --git a/renderer/src/components/artist/ArtistPanel.tsx b/renderer/src/components/artist/ArtistPanel.tsx index 87a7109..e446a89 100644 --- a/renderer/src/components/artist/ArtistPanel.tsx +++ b/renderer/src/components/artist/ArtistPanel.tsx @@ -44,7 +44,13 @@ export function ArtistPanel({ onAddToQueue, currentTrackName, isPlaybackActive } ? resolveArtistParams(item) : { artistId: undefined, serviceId: undefined, accountId: undefined, defaults: undefined, name: undefined }; - const { data, isLoading } = useArtistBrowse(artistId, serviceId, accountId, defaults); + const { data, isLoading } = useArtistBrowse( + artistId, + serviceId, + accountId, + defaults, + fallbackName ?? (item?.title as string | undefined) ?? (item?.name as string | undefined), + ); const name = data?.name ?? fallbackName ?? item?.title ?? item?.name ?? ''; const imageUrl = diff --git a/renderer/src/hooks/useArtistBrowse.ts b/renderer/src/hooks/useArtistBrowse.ts index bca3ecb..ab4e765 100644 --- a/renderer/src/hooks/useArtistBrowse.ts +++ b/renderer/src/hooks/useArtistBrowse.ts @@ -1,7 +1,9 @@ import { useQuery } from '@tanstack/react-query'; import { api } from '../lib/sonosApi'; +import { isArtist, parseServiceSearch } from '../lib/itemHelpers'; import type { AlbumTrack } from './useAlbumBrowse'; import { parsePlaylistTracks } from './usePlaylistBrowse'; +import type { ServiceSearch } from '../types/ServiceSearch'; import type { SonosItem, SonosItemId } from '../types/sonos'; import { ArtistResponse } from '../types/ArtistResponse'; @@ -33,12 +35,43 @@ export function artistQueryOptions( artistId: string | undefined, serviceId: string | undefined, accountId: string | undefined, - defaults: string | undefined + defaults: string | undefined, + fallbackName?: string, ) { return { queryKey: ['artist', artistId] as const, queryFn: async (): Promise => { - const r = await api.browse.artist(artistId!, { serviceId, accountId, defaults, muse2: true }); + // First attempt: direct browse with whatever defaults we already have. + let r = await api.browse.artist(artistId!, { serviceId, accountId, defaults, muse2: true }); + + // YT Music (and some other services) need an opaque `defaults` blob to + // return a populated artist response. Navigation sources like the + // leaderboard don't capture defaults, so the direct browse comes back + // with no title/sections. Resolve via search to get a properly-defaulted + // item, then retry the browse. + const looksEmpty = !!r.error || !(r.data as ArtistResponse | undefined)?.title; + if (looksEmpty && fallbackName) { + const sResp = await api.search.serviceQuery(fallbackName, { count: 20 }); + if (!sResp.error && sResp.data) { + const items = parseServiceSearch(sResp.data as ServiceSearch); + const artists = items.filter(isArtist); + const matched = + artists.find((a) => { + const rid = a.resource?.id as SonosItemId | undefined; + return rid?.objectId === artistId; + }) ?? artists[0]; + if (matched) { + const mid = matched.resource?.id as SonosItemId | undefined; + const mDefaults = matched.resource?.defaults as string | undefined; + r = await api.browse.artist(mid?.objectId ?? artistId!, { + serviceId: mid?.serviceId ?? serviceId, + accountId: mid?.accountId ?? accountId, + defaults: mDefaults ?? defaults, + muse2: true, + }); + } + } + } if (r.error) throw new Error(r.error); const { parsed, topSongsItem } = parseArtist(r.data as ArtistResponse); @@ -72,10 +105,35 @@ export function useArtistBrowse( artistId: string | undefined, serviceId: string | undefined, accountId: string | undefined, - defaults?: string + defaults?: string, + fallbackName?: string, ) { return useQuery({ - ...artistQueryOptions(artistId, serviceId, accountId, defaults), + ...artistQueryOptions(artistId, serviceId, accountId, defaults, fallbackName), enabled: !!(artistId && serviceId && accountId), }); } + +/** + * Lightweight fetch for just the artist's image — used by avatar UI where the + * full artistQueryOptions (top songs + Genius bio) would be wasteful. Cached + * under a distinct key so it doesn't conflict with the full browse cache. + */ +export function useArtistImage( + artistId: string | undefined, + serviceId: string | undefined, + accountId: string | undefined, +) { + return useQuery({ + queryKey: ['artist-image', artistId] as const, + queryFn: async (): Promise => { + const r = await api.browse.artist(artistId!, { serviceId, accountId, muse2: true }); + if (r.error || !r.data) return null; + return (r.data as ArtistResponse).images?.tile1x1 ?? null; + }, + enabled: !!(artistId && serviceId && accountId), + staleTime: Infinity, + gcTime: 60 * 60 * 1000, + retry: false, + }); +} diff --git a/renderer/src/hooks/useResolveAndOpen.ts b/renderer/src/hooks/useResolveAndOpen.ts index 2edaf7a..dece4b0 100644 --- a/renderer/src/hooks/useResolveAndOpen.ts +++ b/renderer/src/hooks/useResolveAndOpen.ts @@ -10,6 +10,9 @@ type Target = 'artist' | 'album'; interface ResolveOpts { /** Artist hint used to disambiguate same-named albums by different artists. */ artist?: string; + /** Scope the Sonos search to this service+account; helps when the artist lives on YT Music etc. */ + serviceId?: string; + accountId?: string; } function itemArtistNames(item: SonosItem): string[] { @@ -28,7 +31,11 @@ export function useResolveAndOpen() { setPending(key); try { const searchQ = target === 'album' && opts?.artist ? `${query} ${opts.artist}` : query; - const r = await api.search.serviceQuery(searchQ, { count: 20 }); + const r = await api.search.serviceQuery(searchQ, { + count: 20, + serviceId: opts?.serviceId, + accountId: opts?.accountId, + }); if (r.error) return; const items = parseServiceSearch(r.data as ServiceSearch); const filter = target === 'artist' ? isArtist : isAlbum; diff --git a/server/src/shared/aggregate.ts b/server/src/shared/aggregate.ts index 8a011b9..1618eee 100644 --- a/server/src/shared/aggregate.ts +++ b/server/src/shared/aggregate.ts @@ -71,9 +71,38 @@ function albumKeyFor(e: RawEvent): string | null { return e.albumId ?? e.album; } -function artistKeyFor(e: RawEvent): string | null { - if (!e.artist) return null; - return e.artist; +/** + * Sonos serves multi-artist tracks/albums with a single joined subtitle + * ("Sonny Stitt, Kenny Garrett") which we publish verbatim. Splitting on + * commas lets each contributor count as their own artist. + * + * Some services (notably YT Music) instead pack release metadata into the + * subtitle — "Album, 2020", "Single, 4.2M views", "Album • Artist" — which + * isn't an artist at all. We drop those entries entirely instead of letting + * them become phantom "Album" / "2020" rows in topArtists. + */ +const RELEASE_TYPE_RE = /\b(Single|Album|EP|LP|Playlist|Various Artists|Compilation|Soundtrack)\b/i; +const METADATA_SEPARATOR_RE = /[•·|]/; +const VIEW_COUNT_RE = /views|plays|listeners|streams/i; + +function artistKeysFor(e: RawEvent): string[] { + if (!e.artist) return []; + const raw = e.artist.trim(); + if (!raw) return []; + + // Release-type subtitles and view/play counts aren't artists — drop them so + // they don't pollute topArtists at all. + if (RELEASE_TYPE_RE.test(raw)) return []; + if (METADATA_SEPARATOR_RE.test(raw)) return []; + if (VIEW_COUNT_RE.test(raw)) return []; + if (/^\d+$/.test(raw)) return []; + + // Contains digits but no separators (e.g. "blink-182", "Maroon 5") — keep as + // a single entry, don't try to split. + if (/\d/.test(raw)) return [raw]; + + const parts = raw.split(/,\s*/).map((s) => s.trim()).filter(Boolean); + return parts.length > 0 ? parts : [raw]; } function bumpQueuer( @@ -104,26 +133,31 @@ export function aggregateEvents(events: RawEvent[]): AggregateResult { if (countsUser && e.userId) userCounts[e.userId] = (userCounts[e.userId] ?? 0) + 1; if (isTrack) { - const aKey = artistKeyFor(e); - if (aKey) { + const aKeys = artistKeysFor(e); + aKeys.forEach((aKey, idx) => { + // Only the first name is the primary artist that e.artistId references. + // The track art (e.imageUrl) goes on every artist for the row — it's + // not their real photo but it's better than an empty placeholder until + // we can resolve the real artist image. + const isPrimary = idx === 0; if (!artistMap[aKey]) { artistMap[aKey] = { - artist: e.artist, + artist: aKey, serviceId: e.serviceId ?? undefined, accountId: e.accountId ?? undefined, - artistId: e.artistId ?? undefined, + artistId: isPrimary ? (e.artistId ?? undefined) : undefined, imageUrl: e.imageUrl ?? undefined, count: 0, }; } else { if (!artistMap[aKey].serviceId && e.serviceId) artistMap[aKey].serviceId = e.serviceId; if (!artistMap[aKey].accountId && e.accountId) artistMap[aKey].accountId = e.accountId; - if (!artistMap[aKey].artistId && e.artistId) artistMap[aKey].artistId = e.artistId; + if (isPrimary && !artistMap[aKey].artistId && e.artistId) artistMap[aKey].artistId = e.artistId; if (!artistMap[aKey].imageUrl && e.imageUrl) artistMap[aKey].imageUrl = e.imageUrl; } artistMap[aKey].count++; if (e.userId) bumpQueuer(queuersByItem, itemKey('artist', aKey), e.userId); - } + }); } if (isAlbumOnly) {