From a115bae7c6ae31b2ea0d1c72516bcf248d1b625f Mon Sep 17 00:00:00 2001 From: Colin Tinney Date: Tue, 28 Apr 2026 19:49:35 -0700 Subject: [PATCH] feat: add pluggable discovery sources for richer album mosaic Introduces a DiscoverySource interface so album discovery strategies are pluggable. Three sources: related artists (multi-artist, Last.fm + ListenBrainz), genre-based fallback, and 2-hop traversal. Also adds a song-level discovery cache and increases related artists cache TTL to 4h. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/plans/richer-album-discovery.md | 22 ++-- packages/server/src/cache/index.ts | 17 ++- .../__tests__/fetchAlbumsForArtists.test.ts | 94 +++++++++++++++ .../discovery/__tests__/runDiscovery.test.ts | 50 ++++++++ .../api/discovery/fetchAlbumsForArtists.ts | 58 ++++++++++ .../src/spotify/api/discovery/runDiscovery.ts | 27 +++++ .../sources/__tests__/genreSource.test.ts | 58 ++++++++++ .../discovery/sources/__tests__/helpers.ts | 18 +++ .../__tests__/relatedArtistsSource.test.ts | 72 ++++++++++++ .../sources/__tests__/twoHopSource.test.ts | 62 ++++++++++ .../api/discovery/sources/genreSource.ts | 58 ++++++++++ .../discovery/sources/relatedArtistsSource.ts | 31 +++++ .../api/discovery/sources/twoHopSource.ts | 41 +++++++ .../server/src/spotify/api/discovery/types.ts | 41 +++++++ .../getCurrentlyPlayingRelatedAlbums.test.ts | 109 ++++++------------ .../helpers/__tests__/resolveArtistId.test.ts | 40 +++++++ .../getCurrentlyPlayingRelatedAlbums.ts | 76 ++++-------- .../spotify/api/helpers/resolveArtistId.ts | 26 +++++ 18 files changed, 759 insertions(+), 141 deletions(-) create mode 100644 packages/server/src/spotify/api/discovery/__tests__/fetchAlbumsForArtists.test.ts create mode 100644 packages/server/src/spotify/api/discovery/__tests__/runDiscovery.test.ts create mode 100644 packages/server/src/spotify/api/discovery/fetchAlbumsForArtists.ts create mode 100644 packages/server/src/spotify/api/discovery/runDiscovery.ts create mode 100644 packages/server/src/spotify/api/discovery/sources/__tests__/genreSource.test.ts create mode 100644 packages/server/src/spotify/api/discovery/sources/__tests__/helpers.ts create mode 100644 packages/server/src/spotify/api/discovery/sources/__tests__/relatedArtistsSource.test.ts create mode 100644 packages/server/src/spotify/api/discovery/sources/__tests__/twoHopSource.test.ts create mode 100644 packages/server/src/spotify/api/discovery/sources/genreSource.ts create mode 100644 packages/server/src/spotify/api/discovery/sources/relatedArtistsSource.ts create mode 100644 packages/server/src/spotify/api/discovery/sources/twoHopSource.ts create mode 100644 packages/server/src/spotify/api/discovery/types.ts create mode 100644 packages/server/src/spotify/api/helpers/__tests__/resolveArtistId.test.ts create mode 100644 packages/server/src/spotify/api/helpers/resolveArtistId.ts diff --git a/docs/plans/richer-album-discovery.md b/docs/plans/richer-album-discovery.md index 156a58ff..174c34bc 100644 --- a/docs/plans/richer-album-discovery.md +++ b/docs/plans/richer-album-discovery.md @@ -18,31 +18,23 @@ Make the mosaic more interesting and varied by discovering more related albums f Currently `getRelatedArtists` is called with only `songArtists[0]`. For tracks with multiple artists (features, collaborations), query all song artists and merge the results. This immediately widens the pool for collaborative tracks. -### 2. Spotify recommendations API - -Add `GET /recommendations` as a third data source alongside Last.fm and ListenBrainz. Pass `seed_tracks` (current track) and/or `seed_artists` (track artists). This leverages Spotify's own collaborative filtering, which often surfaces different results than Last.fm's tag-based similarity. - -Extract the album IDs from the recommended tracks and fetch those albums. - -### 3. Deeper graph traversal (2-hop) +### 2. Deeper graph traversal (2-hop) For the top N most-similar artists (e.g., top 5 from Last.fm), fetch _their_ similar artists too. This creates a 2-hop graph that reaches more obscure artists. Apply diminishing priority: 1-hop results appear first in the mosaic, 2-hop results fill remaining space. Rate-limit carefully — each hop multiplies the number of external API calls. -### 4. Genre-based discovery +### 3. Genre-based discovery When the related artist pool is small (e.g., niche genres), fall back to genre-based search. Use the primary artist's genres from Spotify's artist object and search for other artists in the same genre via `GET /search?type=artist&genre=...`. -### 5. Caching improvements +### 4. Caching improvements - Increase `relatedArtistsCache` TTL from 1 hour to 4 hours — similar artists don't change frequently - Add a combined "discovery result" cache keyed by song ID that caches the final merged album list, avoiding re-running the full pipeline on repeated requests for the same song -## Suggested implementation order +## Rejected + +### ~~Spotify recommendations API~~ -1. Multi-artist discovery (low effort, immediate improvement for collaborative tracks) -2. Spotify recommendations API (moderate effort, best diversity improvement) -3. Caching improvements (low effort, reduces API load for all other changes) -4. Genre-based fallback (moderate effort, helps niche genres) -5. 2-hop traversal (higher effort, biggest pool expansion but needs rate-limit care) +`GET /recommendations` was deprecated by Spotify in November 2024 and is inaccessible to new applications. No replacement endpoint has been provided. diff --git a/packages/server/src/cache/index.ts b/packages/server/src/cache/index.ts index 57ac9c90..8320c7ab 100644 --- a/packages/server/src/cache/index.ts +++ b/packages/server/src/cache/index.ts @@ -1,6 +1,7 @@ import { LRUCache } from 'lru-cache'; -import type { ArtistAlbumsResult } from '../types'; +import type { ArtistAlbumsResult, SpotifyAlbum } from '../types'; +const FOUR_HOURS_MS = 4 * 60 * 60 * 1000; const ONE_HOUR_MS = 60 * 60 * 1000; const THIRTY_MIN_MS = 30 * 60 * 1000; @@ -9,10 +10,16 @@ export const NOT_FOUND = Symbol('NOT_FOUND'); export type ArtistIdCacheValue = string | typeof NOT_FOUND; +/** Final discovery result by Spotify track ID. */ +export const discoveryResultCache = new LRUCache({ + max: 100, + ttl: FOUR_HOURS_MS, +}); + /** Related artist names by primary artist name (case-insensitive key). */ export const relatedArtistsCache = new LRUCache({ max: 200, - ttl: ONE_HOUR_MS, + ttl: FOUR_HOURS_MS, }); /** Spotify artist ID by artist name (case-insensitive key). Uses NOT_FOUND for null results. */ @@ -27,6 +34,12 @@ export const artistAlbumsCache = new LRUCache({ ttl: THIRTY_MIN_MS, }); +/** Genre-based artist IDs by Spotify artist ID. */ +export const genreArtistsCache = new LRUCache({ + max: 200, + ttl: FOUR_HOURS_MS, +}); + export function normalizeKey(name: string): string { return String(name).toLowerCase().trim(); } diff --git a/packages/server/src/spotify/api/discovery/__tests__/fetchAlbumsForArtists.test.ts b/packages/server/src/spotify/api/discovery/__tests__/fetchAlbumsForArtists.test.ts new file mode 100644 index 00000000..c8de0e53 --- /dev/null +++ b/packages/server/src/spotify/api/discovery/__tests__/fetchAlbumsForArtists.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { fetchAlbumsForNames, fetchAlbumsForIds } from '../fetchAlbumsForArtists'; +import resolveArtistId from '../../helpers/resolveArtistId'; +import getArtistAlbums from '../../helpers/getArtistAlbums'; +import { artistIdCache, artistAlbumsCache } from '../../../../cache'; + +vi.mock('../../helpers/resolveArtistId'); +vi.mock('../../helpers/getArtistAlbums'); + +const mockedResolve = vi.mocked(resolveArtistId); +const mockedAlbums = vi.mocked(getArtistAlbums); + +const mockApi = {} as any; + +function albumResult(artistId: string, count: number) { + return { + artistId, + albums: Array.from({ length: count }, (_, i) => ({ name: `${artistId}-album-${i}` })), + } as any; +} + +describe('fetchAlbumsForNames()', () => { + beforeEach(() => { + vi.clearAllMocks(); + artistIdCache.clear(); + artistAlbumsCache.clear(); + }); + + it('resolves names to IDs and fetches their albums', async () => { + mockedResolve.mockResolvedValueOnce('id1').mockResolvedValueOnce('id2'); + mockedAlbums.mockImplementation((_, id) => Promise.resolve(albumResult(id as string, 2))); + + const albums = await fetchAlbumsForNames(mockApi, ['A', 'B'], new Set(), 100); + + expect(albums).toHaveLength(4); + expect(mockedResolve).toHaveBeenCalledTimes(2); + }); + + it('skips names that resolve to excluded IDs', async () => { + mockedResolve.mockResolvedValue('excluded-id'); + + const albums = await fetchAlbumsForNames(mockApi, ['Skip'], new Set(['excluded-id']), 100); + + expect(albums).toHaveLength(0); + expect(mockedAlbums).not.toHaveBeenCalled(); + }); + + it('skips names that resolve to null', async () => { + mockedResolve.mockResolvedValue(null); + + const albums = await fetchAlbumsForNames(mockApi, ['Unknown'], new Set(), 100); + + expect(albums).toHaveLength(0); + expect(mockedAlbums).not.toHaveBeenCalled(); + }); + + it('stops when maxAlbums is reached', async () => { + mockedResolve.mockImplementation((_, name) => Promise.resolve(`id-${name}`)); + mockedAlbums.mockImplementation((_, id) => Promise.resolve(albumResult(id as string, 10))); + + // 6 names but maxAlbums=15 — first batch of 5 yields 50 albums, should stop + const names = ['A', 'B', 'C', 'D', 'E', 'F']; + const albums = await fetchAlbumsForNames(mockApi, names, new Set(), 15); + + // First batch of 5 resolves and fetches (50 albums), second batch never runs + expect(albums).toHaveLength(50); + expect(mockedResolve).toHaveBeenCalledTimes(5); + }); +}); + +describe('fetchAlbumsForIds()', () => { + beforeEach(() => { + vi.clearAllMocks(); + artistAlbumsCache.clear(); + }); + + it('fetches albums for each ID', async () => { + mockedAlbums.mockImplementation((_, id) => Promise.resolve(albumResult(id as string, 3))); + + const albums = await fetchAlbumsForIds(mockApi, ['x', 'y'], 100); + + expect(albums).toHaveLength(6); + expect(mockedAlbums).toHaveBeenCalledTimes(2); + }); + + it('stops when maxAlbums is reached', async () => { + mockedAlbums.mockImplementation((_, id) => Promise.resolve(albumResult(id as string, 20))); + + await fetchAlbumsForIds(mockApi, ['a', 'b', 'c', 'd', 'e', 'f'], 5); + + // First batch of 5 yields 100 albums, second batch never runs + expect(mockedAlbums).toHaveBeenCalledTimes(5); + }); +}); diff --git a/packages/server/src/spotify/api/discovery/__tests__/runDiscovery.test.ts b/packages/server/src/spotify/api/discovery/__tests__/runDiscovery.test.ts new file mode 100644 index 00000000..d343aa20 --- /dev/null +++ b/packages/server/src/spotify/api/discovery/__tests__/runDiscovery.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect, vi } from 'vitest'; +import runDiscovery from '../runDiscovery'; +import type { DiscoveryContext, DiscoverySource } from '../types'; + +const mockContext = {} as DiscoveryContext; + +function makeSource(name: string, priority: number, albums: { name: string }[]): DiscoverySource { + return { + name, + priority, + discover: vi.fn().mockResolvedValue(albums), + }; +} + +describe('runDiscovery()', () => { + it('returns albums sorted by source priority', async () => { + const low = makeSource('low', 20, [{ name: 'C' }]); + const high = makeSource('high', 5, [{ name: 'A' }]); + const mid = makeSource('mid', 10, [{ name: 'B' }]); + + const albums = await runDiscovery([low, high, mid], mockContext); + expect(albums.map((a) => a.name)).toEqual(['A', 'B', 'C']); + }); + + it('runs all sources in parallel', async () => { + const s1 = makeSource('s1', 1, [{ name: 'X' }]); + const s2 = makeSource('s2', 2, [{ name: 'Y' }]); + + await runDiscovery([s1, s2], mockContext); + expect(s1.discover).toHaveBeenCalledWith(mockContext); + expect(s2.discover).toHaveBeenCalledWith(mockContext); + }); + + it('returns empty albums when a source throws', async () => { + const failing: DiscoverySource = { + name: 'failing', + priority: 1, + discover: vi.fn().mockRejectedValue(new Error('boom')), + }; + const working = makeSource('working', 2, [{ name: 'ok' }]); + + const albums = await runDiscovery([failing, working], mockContext); + expect(albums.map((a) => a.name)).toEqual(['ok']); + }); + + it('returns empty array when no sources provided', async () => { + const albums = await runDiscovery([], mockContext); + expect(albums).toEqual([]); + }); +}); diff --git a/packages/server/src/spotify/api/discovery/fetchAlbumsForArtists.ts b/packages/server/src/spotify/api/discovery/fetchAlbumsForArtists.ts new file mode 100644 index 00000000..0ccaa4ab --- /dev/null +++ b/packages/server/src/spotify/api/discovery/fetchAlbumsForArtists.ts @@ -0,0 +1,58 @@ +import type { SpotifyApi } from '@spotify/web-api-ts-sdk'; +import getArtistAlbums from '../helpers/getArtistAlbums'; +import resolveArtistId from '../helpers/resolveArtistId'; +import type { SpotifyAlbum } from '../../../types'; + +const BATCH_SIZE = 5; + +/** + * Resolve artist names to Spotify IDs and fetch their albums in batches. + * Skips names that resolve to null or to an ID in the exclude set. + * Stops early once `maxAlbums` is reached. + */ +export async function fetchAlbumsForNames( + spotifyApi: SpotifyApi, + names: string[], + excludeIds: Set, + maxAlbums: number, +): Promise { + const albums: SpotifyAlbum[] = []; + + for (let i = 0; i < names.length && albums.length < maxAlbums; i += BATCH_SIZE) { + const batch = names.slice(i, i + BATCH_SIZE); + const ids = await Promise.all(batch.map((name) => resolveArtistId(spotifyApi, name))); + + const validIds = ids.filter((id): id is string => id !== null && !excludeIds.has(id)); + + const results = await Promise.all(validIds.map((id) => getArtistAlbums(spotifyApi, id))); + + for (const result of results) { + albums.push(...result.albums); + } + } + + return albums; +} + +/** + * Fetch albums for a list of already-resolved artist IDs in batches. + * Stops early once `maxAlbums` is reached. + */ +export async function fetchAlbumsForIds( + spotifyApi: SpotifyApi, + ids: string[], + maxAlbums: number, +): Promise { + const albums: SpotifyAlbum[] = []; + + for (let i = 0; i < ids.length && albums.length < maxAlbums; i += BATCH_SIZE) { + const batch = ids.slice(i, i + BATCH_SIZE); + const results = await Promise.all(batch.map((id) => getArtistAlbums(spotifyApi, id))); + + for (const result of results) { + albums.push(...result.albums); + } + } + + return albums; +} diff --git a/packages/server/src/spotify/api/discovery/runDiscovery.ts b/packages/server/src/spotify/api/discovery/runDiscovery.ts new file mode 100644 index 00000000..fb5cac89 --- /dev/null +++ b/packages/server/src/spotify/api/discovery/runDiscovery.ts @@ -0,0 +1,27 @@ +import logger from '../../../logger'; +import type { SpotifyAlbum } from '../../../types'; +import type { DiscoveryContext, DiscoverySource } from './types'; + +/** + * Run all discovery sources in parallel, sort results by priority, + * and return a flat array of albums in priority order. + */ +export default async function runDiscovery( + sources: DiscoverySource[], + context: DiscoveryContext, +): Promise { + const results = await Promise.all( + sources.map(async (source) => { + try { + const albums = await source.discover(context); + logger.info(`Discovery source "${source.name}": ${albums.length} albums`); + return { priority: source.priority, albums }; + } catch { + logger.warn(`Discovery source "${source.name}" failed`); + return { priority: source.priority, albums: [] as SpotifyAlbum[] }; + } + }), + ); + + return results.sort((a, b) => a.priority - b.priority).flatMap((r) => r.albums); +} diff --git a/packages/server/src/spotify/api/discovery/sources/__tests__/genreSource.test.ts b/packages/server/src/spotify/api/discovery/sources/__tests__/genreSource.test.ts new file mode 100644 index 00000000..0b9dce78 --- /dev/null +++ b/packages/server/src/spotify/api/discovery/sources/__tests__/genreSource.test.ts @@ -0,0 +1,58 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { genreSource } from '../genreSource'; +import getArtistAlbums from '../../../helpers/getArtistAlbums'; +import { genreArtistsCache } from '../../../../../cache'; +import { makeContext, mockSpotifyApi } from './helpers'; + +vi.mock('../../../helpers/getArtistAlbums'); +const mockedGetArtistAlbums = vi.mocked(getArtistAlbums); + +describe('genreSource', () => { + beforeEach(() => { + vi.clearAllMocks(); + genreArtistsCache.clear(); + }); + + it('fetches genres for primary artist and searches for similar', async () => { + mockSpotifyApi.artists.get.mockResolvedValue({ genres: ['indie rock', 'shoegaze'] }); + mockSpotifyApi.search.mockResolvedValue({ + artists: { items: [{ id: 'g1' }, { id: 'g2' }, { id: 'a1' }] }, + }); + mockedGetArtistAlbums.mockResolvedValue({ + artistId: 'g1', + albums: [{ name: 'Genre Album' }], + } as any); + + await genreSource.discover(makeContext()); + + expect(mockSpotifyApi.artists.get).toHaveBeenCalledWith('a1'); + expect(mockSpotifyApi.search).toHaveBeenCalledWith( + 'genre:"indie rock" genre:"shoegaze"', + ['artist'], + undefined, + 20, + ); + expect(mockedGetArtistAlbums).toHaveBeenCalledTimes(2); + }); + + it('returns empty when artist has no genres', async () => { + mockSpotifyApi.artists.get.mockResolvedValue({ genres: [] }); + + const albums = await genreSource.discover(makeContext()); + + expect(albums).toHaveLength(0); + expect(mockSpotifyApi.search).not.toHaveBeenCalled(); + }); + + it('returns empty when no song artists', async () => { + const albums = await genreSource.discover(makeContext({ songArtists: [] })); + expect(albums).toHaveLength(0); + }); + + it('returns empty on API failure', async () => { + mockSpotifyApi.artists.get.mockRejectedValue(new Error('fail')); + + const albums = await genreSource.discover(makeContext()); + expect(albums).toHaveLength(0); + }); +}); diff --git a/packages/server/src/spotify/api/discovery/sources/__tests__/helpers.ts b/packages/server/src/spotify/api/discovery/sources/__tests__/helpers.ts new file mode 100644 index 00000000..49167dec --- /dev/null +++ b/packages/server/src/spotify/api/discovery/sources/__tests__/helpers.ts @@ -0,0 +1,18 @@ +import { vi } from 'vitest'; +import type { DiscoveryContext } from '../../types'; + +export const mockSpotifyApi = { + search: vi.fn(), + artists: { get: vi.fn() }, +}; + +export function makeContext(overrides: Partial = {}): DiscoveryContext { + return { + spotifyApi: mockSpotifyApi as any, + trackId: 'track1', + songArtists: [{ id: 'a1', name: 'Artist One' }], + albumArtists: [{ id: 'a1', name: 'Artist One' }], + trackArtistIdSet: new Set(['a1']), + ...overrides, + }; +} diff --git a/packages/server/src/spotify/api/discovery/sources/__tests__/relatedArtistsSource.test.ts b/packages/server/src/spotify/api/discovery/sources/__tests__/relatedArtistsSource.test.ts new file mode 100644 index 00000000..59610f8f --- /dev/null +++ b/packages/server/src/spotify/api/discovery/sources/__tests__/relatedArtistsSource.test.ts @@ -0,0 +1,72 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { relatedArtistsSource } from '../relatedArtistsSource'; +import getRelatedArtists from '../../../helpers/getRelatedArtists'; +import getArtistAlbums from '../../../helpers/getArtistAlbums'; +import { artistIdCache } from '../../../../../cache'; +import { makeContext, mockSpotifyApi } from './helpers'; + +vi.mock('../../../helpers/getRelatedArtists'); +vi.mock('../../../helpers/getArtistAlbums'); + +const mockedGetRelatedArtists = vi.mocked(getRelatedArtists); +const mockedGetArtistAlbums = vi.mocked(getArtistAlbums); + +describe('relatedArtistsSource', () => { + beforeEach(() => { + vi.clearAllMocks(); + artistIdCache.clear(); + }); + + it('fetches related artists for all song artists', async () => { + mockedGetRelatedArtists.mockResolvedValue([]); + + await relatedArtistsSource.discover( + makeContext({ + songArtists: [ + { id: 'a1', name: 'Artist One' }, + { id: 'a2', name: 'Artist Two' }, + ], + }), + ); + + expect(mockedGetRelatedArtists).toHaveBeenCalledWith('Artist One'); + expect(mockedGetRelatedArtists).toHaveBeenCalledWith('Artist Two'); + }); + + it('deduplicates names case-insensitively and resolves to albums', async () => { + mockedGetRelatedArtists + .mockResolvedValueOnce(['Related A', 'Related B']) + .mockResolvedValueOnce(['related a', 'Related C']); + + mockSpotifyApi.search.mockResolvedValue({ + artists: { items: [{ id: 'resolved-id' }] }, + }); + mockedGetArtistAlbums.mockResolvedValue({ + artistId: 'resolved-id', + albums: [{ name: 'Album' }], + } as any); + + const albums = await relatedArtistsSource.discover( + makeContext({ + songArtists: [ + { id: 'a1', name: 'Artist One' }, + { id: 'a2', name: 'Artist Two' }, + ], + }), + ); + + // 3 unique names (Related A, Related B, Related C) — 'related a' deduped + expect(mockSpotifyApi.search).toHaveBeenCalledTimes(3); + expect(albums.length).toBeGreaterThan(0); + }); + + it('skips artists already in trackArtistIdSet', async () => { + mockedGetRelatedArtists.mockResolvedValue(['Already There']); + mockSpotifyApi.search.mockResolvedValue({ artists: { items: [{ id: 'a1' }] } }); + + const albums = await relatedArtistsSource.discover(makeContext()); + + expect(mockedGetArtistAlbums).not.toHaveBeenCalled(); + expect(albums).toHaveLength(0); + }); +}); diff --git a/packages/server/src/spotify/api/discovery/sources/__tests__/twoHopSource.test.ts b/packages/server/src/spotify/api/discovery/sources/__tests__/twoHopSource.test.ts new file mode 100644 index 00000000..6a714e1d --- /dev/null +++ b/packages/server/src/spotify/api/discovery/sources/__tests__/twoHopSource.test.ts @@ -0,0 +1,62 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { twoHopSource } from '../twoHopSource'; +import getRelatedArtists from '../../../helpers/getRelatedArtists'; +import getArtistAlbums from '../../../helpers/getArtistAlbums'; +import { artistIdCache } from '../../../../../cache'; +import { makeContext, mockSpotifyApi } from './helpers'; + +vi.mock('../../../helpers/getRelatedArtists'); +vi.mock('../../../helpers/getArtistAlbums'); + +const mockedGetRelatedArtists = vi.mocked(getRelatedArtists); +const mockedGetArtistAlbums = vi.mocked(getArtistAlbums); + +describe('twoHopSource', () => { + beforeEach(() => { + vi.clearAllMocks(); + artistIdCache.clear(); + }); + + it('fetches related artists of top related artists', async () => { + mockedGetRelatedArtists + .mockResolvedValueOnce(['Related A', 'Related B']) + .mockResolvedValueOnce(['Deep C', 'Related A']) + .mockResolvedValueOnce(['Deep D']); + + mockSpotifyApi.search.mockResolvedValue({ + artists: { items: [{ id: 'deep-id' }] }, + }); + mockedGetArtistAlbums.mockResolvedValue({ + artistId: 'deep-id', + albums: [{ name: 'Deep Album' }], + } as any); + + const albums = await twoHopSource.discover(makeContext()); + + expect(mockedGetRelatedArtists).toHaveBeenCalledTimes(3); + // Deep C and Deep D (Related A filtered as 1st-hop duplicate) + expect(mockSpotifyApi.search).toHaveBeenCalledTimes(2); + expect(albums.length).toBeGreaterThan(0); + }); + + it('excludes first-hop names from results', async () => { + mockedGetRelatedArtists + .mockResolvedValueOnce(['Hop1']) + .mockResolvedValueOnce(['Hop1', 'Hop2New']); + + mockSpotifyApi.search.mockResolvedValue({ + artists: { items: [{ id: 'h2' }] }, + }); + mockedGetArtistAlbums.mockResolvedValue({ artistId: 'h2', albums: [] } as any); + + await twoHopSource.discover(makeContext()); + + expect(mockSpotifyApi.search).toHaveBeenCalledTimes(1); + expect(mockSpotifyApi.search).toHaveBeenCalledWith('Hop2New', ['artist'], undefined, 1); + }); + + it('returns empty when no primary artist', async () => { + const albums = await twoHopSource.discover(makeContext({ songArtists: [] })); + expect(albums).toHaveLength(0); + }); +}); diff --git a/packages/server/src/spotify/api/discovery/sources/genreSource.ts b/packages/server/src/spotify/api/discovery/sources/genreSource.ts new file mode 100644 index 00000000..07a342fe --- /dev/null +++ b/packages/server/src/spotify/api/discovery/sources/genreSource.ts @@ -0,0 +1,58 @@ +import type { SpotifyApi } from '@spotify/web-api-ts-sdk'; +import logger from '../../../../logger'; +import errorMessage from '../../../../utils/errorMessage'; +import { genreArtistsCache } from '../../../../cache'; +import { fetchAlbumsForIds } from '../fetchAlbumsForArtists'; +import type { DiscoverySource } from '../types'; + +const MAX_GENRE_ARTISTS = 20; +const MAX_ALBUMS = 200; + +async function getGenreArtistIds(spotifyApi: SpotifyApi, artistId: string): Promise { + const cached = genreArtistsCache.get(artistId); + if (cached) { + return cached; + } + + const artist = await spotifyApi.artists.get(artistId); + const genres = artist.genres; + + if (!genres || genres.length === 0) { + logger.info(`No genres found for artist ${artistId}`); + return []; + } + + const genreQuery = genres + .slice(0, 2) + .map((g) => `genre:"${g}"`) + .join(' '); + const results = await spotifyApi.search(genreQuery, ['artist'], undefined, MAX_GENRE_ARTISTS); + const ids = (results.artists?.items || []).map((a) => a.id).filter((id) => id !== artistId); + + logger.info(`Genre search: ${ids.length} artists for genres [${genres.slice(0, 2).join(', ')}]`); + genreArtistsCache.set(artistId, ids); + return ids; +} + +export const genreSource: DiscoverySource = { + name: 'genre', + priority: 20, + + async discover(context) { + const { spotifyApi, songArtists, trackArtistIdSet } = context; + const primaryArtistId = songArtists[0]?.id; + + if (!primaryArtistId) { + return []; + } + + try { + const genreArtistIds = await getGenreArtistIds(spotifyApi, primaryArtistId); + const newIds = genreArtistIds.filter((id) => !trackArtistIdSet.has(id)); + return fetchAlbumsForIds(spotifyApi, newIds, MAX_ALBUMS); + } catch (error) { + logger.warn(`Genre discovery failed: ${errorMessage(error)}`); + return []; + } + }, +}; diff --git a/packages/server/src/spotify/api/discovery/sources/relatedArtistsSource.ts b/packages/server/src/spotify/api/discovery/sources/relatedArtistsSource.ts new file mode 100644 index 00000000..f10026ef --- /dev/null +++ b/packages/server/src/spotify/api/discovery/sources/relatedArtistsSource.ts @@ -0,0 +1,31 @@ +import getRelatedArtists from '../../helpers/getRelatedArtists'; +import { fetchAlbumsForNames } from '../fetchAlbumsForArtists'; +import type { DiscoverySource } from '../types'; + +const MAX_ALBUMS = 200; + +export const relatedArtistsSource: DiscoverySource = { + name: 'related-artists', + priority: 10, + + async discover(context) { + const { spotifyApi, songArtists, trackArtistIdSet } = context; + + const artistNames = [...new Set(songArtists.map((a) => a.name))]; + const nameArrays = await Promise.all(artistNames.map((name) => getRelatedArtists(name))); + + const seen = new Set(); + const relatedNames: string[] = []; + for (const names of nameArrays) { + for (const name of names) { + const key = name.toLowerCase(); + if (!seen.has(key)) { + seen.add(key); + relatedNames.push(name); + } + } + } + + return fetchAlbumsForNames(spotifyApi, relatedNames, trackArtistIdSet, MAX_ALBUMS); + }, +}; diff --git a/packages/server/src/spotify/api/discovery/sources/twoHopSource.ts b/packages/server/src/spotify/api/discovery/sources/twoHopSource.ts new file mode 100644 index 00000000..7df7d29a --- /dev/null +++ b/packages/server/src/spotify/api/discovery/sources/twoHopSource.ts @@ -0,0 +1,41 @@ +import getRelatedArtists from '../../helpers/getRelatedArtists'; +import { fetchAlbumsForNames } from '../fetchAlbumsForArtists'; +import type { DiscoverySource } from '../types'; + +const TWO_HOP_COUNT = 5; +const MAX_ALBUMS = 100; + +export const twoHopSource: DiscoverySource = { + name: 'two-hop', + priority: 30, + + async discover(context) { + const { spotifyApi, songArtists, trackArtistIdSet } = context; + const primaryArtistName = songArtists[0]?.name; + + if (!primaryArtistName) { + return []; + } + + const firstHopNames = await getRelatedArtists(primaryArtistName); + const topRelated = firstHopNames.slice(0, TWO_HOP_COUNT); + const secondHopArrays = await Promise.all(topRelated.map((name) => getRelatedArtists(name))); + + // Only keep 2nd-hop names that weren't already in the 1st hop + const firstHopSet = new Set(firstHopNames.map((n) => n.toLowerCase())); + const seen = new Set(); + const twoHopNames: string[] = []; + + for (const names of secondHopArrays) { + for (const name of names) { + const key = name.toLowerCase(); + if (!firstHopSet.has(key) && !seen.has(key)) { + seen.add(key); + twoHopNames.push(name); + } + } + } + + return fetchAlbumsForNames(spotifyApi, twoHopNames, trackArtistIdSet, MAX_ALBUMS); + }, +}; diff --git a/packages/server/src/spotify/api/discovery/types.ts b/packages/server/src/spotify/api/discovery/types.ts new file mode 100644 index 00000000..a96460b4 --- /dev/null +++ b/packages/server/src/spotify/api/discovery/types.ts @@ -0,0 +1,41 @@ +import type { SpotifyApi } from '@spotify/web-api-ts-sdk'; +import type { SpotifyAlbum, SpotifyArtist } from '../../../types'; + +/** + * Snapshot of the currently playing track, passed to each discovery source. + * Sources use this to decide which external APIs to query and which artist + * IDs to exclude (to avoid duplicating the track artists' own albums, which + * are always fetched separately at higher priority). + */ +export interface DiscoveryContext { + spotifyApi: SpotifyApi; + trackId: string; + /** Artists credited on the song itself (may differ from album artists for features). */ + songArtists: SpotifyArtist[]; + /** Artists credited on the album (often the "main" artist on compilations). */ + albumArtists: SpotifyArtist[]; + /** Deduplicated union of song + album artist IDs, pre-computed as a Set for + * efficient exclusion checks across sources. */ + trackArtistIdSet: Set; +} + +/** + * A pluggable album discovery strategy. Each source independently queries + * external APIs (Last.fm, ListenBrainz, Spotify, etc.) and returns albums. + * + * Sources run in parallel via `runDiscovery()`. Their results are merged + * in priority order — lower priority number means albums appear earlier + * in the mosaic. Sources must handle their own errors and return an empty + * array rather than throwing. + * + * To add a new source: implement this interface, create a file in + * `discovery/sources/`, and add it to `DISCOVERY_SOURCES` in + * `getCurrentlyPlayingRelatedAlbums.ts`. + */ +export interface DiscoverySource { + name: string; + /** Controls album ordering in the final mosaic. Lower = closer to the front. + * Current scale: related-artists=10, genre=20, two-hop=30. */ + priority: number; + discover(context: DiscoveryContext): Promise; +} diff --git a/packages/server/src/spotify/api/helpers/__tests__/getCurrentlyPlayingRelatedAlbums.test.ts b/packages/server/src/spotify/api/helpers/__tests__/getCurrentlyPlayingRelatedAlbums.test.ts index e2175428..e7d8c334 100644 --- a/packages/server/src/spotify/api/helpers/__tests__/getCurrentlyPlayingRelatedAlbums.test.ts +++ b/packages/server/src/spotify/api/helpers/__tests__/getCurrentlyPlayingRelatedAlbums.test.ts @@ -1,95 +1,60 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import getCurrentlyPlayingRelatedAlbums from '../getCurrentlyPlayingRelatedAlbums'; - -import getRelatedArtists from '../getRelatedArtists'; import getArtistAlbums from '../getArtistAlbums'; -import { artistIdCache } from '../../../../cache'; +import runDiscovery from '../../discovery/runDiscovery'; +import { discoveryResultCache } from '../../../../cache'; -vi.mock('../getRelatedArtists'); vi.mock('../getArtistAlbums'); +vi.mock('../../discovery/runDiscovery'); -const mockedGetRelatedArtists = vi.mocked(getRelatedArtists); const mockedGetArtistAlbums = vi.mocked(getArtistAlbums); +const mockedRunDiscovery = vi.mocked(runDiscovery); const mockSpotifyApi = { - player: { - getCurrentlyPlayingTrack: vi.fn(), - }, - search: vi.fn(), + player: { getCurrentlyPlayingTrack: vi.fn() }, }; +function mockPlaying(id: string) { + mockSpotifyApi.player.getCurrentlyPlayingTrack.mockResolvedValue({ + item: { + id, + name: 'Test Song', + artists: [{ id: '1', name: 'Artist 1' }], + album: { artists: [{ id: '1', name: 'Artist 1' }] }, + }, + }); +} + describe('getCurrentlyPlayingRelatedAlbums()', () => { beforeEach(() => { vi.clearAllMocks(); - artistIdCache.clear(); + discoveryResultCache.clear(); }); - it('throws an error when the playing track is different than the request track', () => { - mockSpotifyApi.player.getCurrentlyPlayingTrack.mockImplementation(() => - Promise.resolve({ - item: { - id: 'bar', - artists: [], - album: { - artists: [], - }, - }, - }), - ); + it('rejects when playing track differs from requested songId', () => { + mockPlaying('bar'); expect(getCurrentlyPlayingRelatedAlbums(mockSpotifyApi, 'foo')).rejects.toBeInstanceOf(Error); }); - it('combines song artists, track artists, and related artists into a single object when successful', async () => { - mockSpotifyApi.player.getCurrentlyPlayingTrack.mockImplementation(() => - Promise.resolve({ - item: { - id: 'foo', - artists: [ - { - id: 1, - }, - { - id: 2, - }, - ], - album: { - artists: [ - { - id: 1, - }, - { - id: 2, - }, - ], - }, - }, - }), - ); + it('combines artist albums with discovery source albums', async () => { + mockPlaying('foo'); + mockedGetArtistAlbums.mockResolvedValue({ artistId: '1', albums: [{ name: 'A' }] } as any); + mockedRunDiscovery.mockResolvedValue([{ name: 'B' }] as any); + + const result = await getCurrentlyPlayingRelatedAlbums(mockSpotifyApi, 'foo'); + + expect(result.map((a) => a.name)).toEqual(['A', 'B']); + expect(mockedRunDiscovery).toHaveBeenCalledTimes(1); + }); + + it('returns cached result without calling Spotify API', async () => { + mockPlaying('foo'); + mockedGetArtistAlbums.mockResolvedValue({ artistId: '1', albums: [{ name: 'A' }] } as any); + mockedRunDiscovery.mockResolvedValue([]); - mockedGetRelatedArtists.mockImplementation(() => Promise.resolve([3, 4]) as any); - mockedGetArtistAlbums.mockImplementation( - (_spotifyApi: any, artistId: string) => - Promise.resolve({ - artistId, - albums: [ - { - name: 'cat', - }, - { - name: 'dog', - }, - ], - }) as any, - ); + await getCurrentlyPlayingRelatedAlbums(mockSpotifyApi, 'foo'); + await getCurrentlyPlayingRelatedAlbums(mockSpotifyApi, 'foo'); - const relatedAlbums = await getCurrentlyPlayingRelatedAlbums(mockSpotifyApi, 'foo'); - expect(relatedAlbums).toEqual([ - { - name: 'cat', - }, - { - name: 'dog', - }, - ]); + expect(mockSpotifyApi.player.getCurrentlyPlayingTrack).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/server/src/spotify/api/helpers/__tests__/resolveArtistId.test.ts b/packages/server/src/spotify/api/helpers/__tests__/resolveArtistId.test.ts new file mode 100644 index 00000000..4c96f6d5 --- /dev/null +++ b/packages/server/src/spotify/api/helpers/__tests__/resolveArtistId.test.ts @@ -0,0 +1,40 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import resolveArtistId from '../resolveArtistId'; +import { artistIdCache } from '../../../../cache'; + +const mockSpotifyApi = { + search: vi.fn(), +}; + +describe('resolveArtistId()', () => { + beforeEach(() => { + vi.clearAllMocks(); + artistIdCache.clear(); + }); + + it('returns the artist ID from Spotify search', async () => { + mockSpotifyApi.search.mockResolvedValue({ + artists: { items: [{ id: 'abc123' }] }, + }); + + const id = await resolveArtistId(mockSpotifyApi as any, 'Test Artist'); + + expect(id).toBe('abc123'); + expect(mockSpotifyApi.search).toHaveBeenCalledWith('Test Artist', ['artist'], undefined, 1); + }); + + it('caches null for unknown artists and avoids repeat API calls', async () => { + mockSpotifyApi.search.mockResolvedValue({ artists: { items: [] } }); + + expect(await resolveArtistId(mockSpotifyApi as any, 'Unknown')).toBeNull(); + expect(await resolveArtistId(mockSpotifyApi as any, 'Unknown')).toBeNull(); + expect(mockSpotifyApi.search).toHaveBeenCalledTimes(1); + }); + + it('returns null on API error without caching', async () => { + mockSpotifyApi.search.mockRejectedValue(new Error('API failure')); + + expect(await resolveArtistId(mockSpotifyApi as any, 'Error Artist')).toBeNull(); + expect(artistIdCache.get('error artist')).toBeUndefined(); + }); +}); diff --git a/packages/server/src/spotify/api/helpers/getCurrentlyPlayingRelatedAlbums.ts b/packages/server/src/spotify/api/helpers/getCurrentlyPlayingRelatedAlbums.ts index e7f37589..62c10e48 100644 --- a/packages/server/src/spotify/api/helpers/getCurrentlyPlayingRelatedAlbums.ts +++ b/packages/server/src/spotify/api/helpers/getCurrentlyPlayingRelatedAlbums.ts @@ -1,11 +1,15 @@ import type { SpotifyApi } from '@spotify/web-api-ts-sdk'; import getArtistAlbums from './getArtistAlbums'; -import getRelatedArtists from './getRelatedArtists'; import uniqueAlbums from '../../utils/uniqueAlbums'; import combineTrackArtists from '../../utils/combineTrackArtists'; +import runDiscovery from '../discovery/runDiscovery'; +import { relatedArtistsSource } from '../discovery/sources/relatedArtistsSource'; +import { genreSource } from '../discovery/sources/genreSource'; +import { twoHopSource } from '../discovery/sources/twoHopSource'; import logger from '../../../logger'; -import { artistIdCache, normalizeKey, NOT_FOUND } from '../../../cache'; +import { discoveryResultCache } from '../../../cache'; import type { SpotifyAlbum, SpotifyArtist } from '../../../types'; +import type { DiscoverySource } from '../discovery/types'; interface CurrentlyPlayingItem { id: string; @@ -20,31 +24,18 @@ interface CurrentlyPlayingResponse { item: CurrentlyPlayingItem; } -/** - * Search Spotify for an artist by name and return their ID. - * Results are cached by normalized artist name. - */ -async function resolveArtistId(spotifyApi: SpotifyApi, name: string): Promise { - const cacheKey = normalizeKey(name); - const cached = artistIdCache.get(cacheKey); - if (cached !== undefined) { - return cached === NOT_FOUND ? null : cached; - } - - try { - const results = await spotifyApi.search(name, ['artist'], undefined, 1); - const id = results.artists?.items?.[0]?.id || null; - artistIdCache.set(cacheKey, id ?? NOT_FOUND); - return id; - } catch { - return null; - } -} +const DISCOVERY_SOURCES: DiscoverySource[] = [relatedArtistsSource, genreSource, twoHopSource]; export default async function getCurrentlyPlayingRelatedAlbums( spotifyApi: SpotifyApi, songId: string, ): Promise { + const cachedResult = discoveryResultCache.get(songId); + if (cachedResult) { + logger.info(`Discovery cache hit for songId=${songId} (${cachedResult.length} albums)`); + return cachedResult; + } + const response = (await spotifyApi.player.getCurrentlyPlayingTrack()) as unknown as CurrentlyPlayingResponse; const { @@ -66,9 +57,8 @@ export default async function getCurrentlyPlayingRelatedAlbums( } const trackArtistIds = combineTrackArtists({ songArtists, albumArtists }); - const primaryArtistName = songArtists[0]?.name; + const trackArtistIdSet = new Set(trackArtistIds); - // 1. Fetch track artists' own albums (always included, first priority) const artistAlbumResults = await Promise.all( trackArtistIds.map((artistId) => getArtistAlbums(spotifyApi, artistId)), ); @@ -78,36 +68,18 @@ export default async function getCurrentlyPlayingRelatedAlbums( ); logger.info(`Artist albums: ${artistAlbums.length} from ${trackArtistIds.length} artist(s)`); - // 2. Get related artist names from Last.fm + ListenBrainz - const relatedNames = await getRelatedArtists(primaryArtistName); - - // 3. Resolve related artist names to Spotify IDs and fetch their albums - // Process in batches of 5 to avoid hammering the API - const relatedAlbums: SpotifyAlbum[] = []; - const trackArtistIdSet = new Set(trackArtistIds); - - for (let i = 0; i < relatedNames.length && relatedAlbums.length < 200; i += 5) { - const batch = relatedNames.slice(i, i + 5); - const ids = await Promise.all(batch.map((name) => resolveArtistId(spotifyApi, name))); - - const validIds = ids.filter( - (resolvedId): resolvedId is string => - resolvedId !== null && !trackArtistIdSet.has(resolvedId), - ); - const albumResults = await Promise.all( - validIds.map((resolvedId) => getArtistAlbums(spotifyApi, resolvedId)), - ); - - for (const result of albumResults) { - relatedAlbums.push(...result.albums); - } - } - - logger.info(`Related albums: ${relatedAlbums.length} from related artists`); + const sourceAlbums = await runDiscovery(DISCOVERY_SOURCES, { + spotifyApi, + trackId: id, + songArtists, + albumArtists, + trackArtistIdSet, + }); - // Artist albums first (priority), then related albums - const combined = uniqueAlbums([...artistAlbums, ...relatedAlbums]); + // Artist albums first (priority), then source albums (already priority-sorted) + const combined = uniqueAlbums([...artistAlbums, ...sourceAlbums]); logger.info(`Combined unique albums: ${combined.length}`); + discoveryResultCache.set(songId, combined); return combined; } diff --git a/packages/server/src/spotify/api/helpers/resolveArtistId.ts b/packages/server/src/spotify/api/helpers/resolveArtistId.ts new file mode 100644 index 00000000..67265454 --- /dev/null +++ b/packages/server/src/spotify/api/helpers/resolveArtistId.ts @@ -0,0 +1,26 @@ +import type { SpotifyApi } from '@spotify/web-api-ts-sdk'; +import { artistIdCache, normalizeKey, NOT_FOUND } from '../../../cache'; + +/** + * Search Spotify for an artist by name and return their ID. + * Results are cached by normalized artist name. + */ +export default async function resolveArtistId( + spotifyApi: SpotifyApi, + name: string, +): Promise { + const cacheKey = normalizeKey(name); + const cached = artistIdCache.get(cacheKey); + if (cached !== undefined) { + return cached === NOT_FOUND ? null : cached; + } + + try { + const results = await spotifyApi.search(name, ['artist'], undefined, 1); + const id = results.artists?.items?.[0]?.id || null; + artistIdCache.set(cacheKey, id ?? NOT_FOUND); + return id; + } catch { + return null; + } +}