Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 7 additions & 15 deletions docs/plans/richer-album-discovery.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
17 changes: 15 additions & 2 deletions packages/server/src/cache/index.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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<string, SpotifyAlbum[]>({
max: 100,
ttl: FOUR_HOURS_MS,
});

/** Related artist names by primary artist name (case-insensitive key). */
export const relatedArtistsCache = new LRUCache<string, string[]>({
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. */
Expand All @@ -27,6 +34,12 @@ export const artistAlbumsCache = new LRUCache<string, ArtistAlbumsResult>({
ttl: THIRTY_MIN_MS,
});

/** Genre-based artist IDs by Spotify artist ID. */
export const genreArtistsCache = new LRUCache<string, string[]>({
max: 200,
ttl: FOUR_HOURS_MS,
});

export function normalizeKey(name: string): string {
return String(name).toLowerCase().trim();
}
Original file line number Diff line number Diff line change
@@ -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;

Check warning on line 13 in packages/server/src/spotify/api/discovery/__tests__/fetchAlbumsForArtists.test.ts

View workflow job for this annotation

GitHub Actions / server

Unexpected any. Specify a different type

function albumResult(artistId: string, count: number) {
return {
artistId,
albums: Array.from({ length: count }, (_, i) => ({ name: `${artistId}-album-${i}` })),
} as any;

Check warning on line 19 in packages/server/src/spotify/api/discovery/__tests__/fetchAlbumsForArtists.test.ts

View workflow job for this annotation

GitHub Actions / server

Unexpected any. Specify a different type
}

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);
});
});
Original file line number Diff line number Diff line change
@@ -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([]);
});
});
58 changes: 58 additions & 0 deletions packages/server/src/spotify/api/discovery/fetchAlbumsForArtists.ts
Original file line number Diff line number Diff line change
@@ -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<string>,
maxAlbums: number,
): Promise<SpotifyAlbum[]> {
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<SpotifyAlbum[]> {
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;
}
27 changes: 27 additions & 0 deletions packages/server/src/spotify/api/discovery/runDiscovery.ts
Original file line number Diff line number Diff line change
@@ -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<SpotifyAlbum[]> {
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);
}
Original file line number Diff line number Diff line change
@@ -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);

Check warning on line 24 in packages/server/src/spotify/api/discovery/sources/__tests__/genreSource.test.ts

View workflow job for this annotation

GitHub Actions / server

Unexpected any. Specify a different type

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);
});
});
Original file line number Diff line number Diff line change
@@ -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> = {}): DiscoveryContext {
return {
spotifyApi: mockSpotifyApi as any,

Check warning on line 11 in packages/server/src/spotify/api/discovery/sources/__tests__/helpers.ts

View workflow job for this annotation

GitHub Actions / server

Unexpected any. Specify a different type
trackId: 'track1',
songArtists: [{ id: 'a1', name: 'Artist One' }],
albumArtists: [{ id: 'a1', name: 'Artist One' }],
trackArtistIdSet: new Set(['a1']),
...overrides,
};
}
Loading
Loading