Repository navigation
feat(autoplay): preferred-artist source + remove dead Spotify endpoints - #720
Closed
LucasSantana-Dev wants to merge 3 commits into
Closed
LucasSantana-Dev wants to merge 3 commits into
LucasSantana-Dev wants to merge 3 commits into
Conversation
… recs/audio-features This commit addresses two coupled issues: 1. **Remove dead Spotify endpoints**: Spotify's /v1/recommendations and /v1/audio-features endpoints have been silently returning 404/403 for months, rendering the Spotify-recs path useless. Deleted getSpotifyRecommendations, SpotifyRecommendationTrack, SpotifyAudioFeatureConstraints, getAudioFeatures, and getBatchAudioFeatures from spotifyApi.ts along with their audio-feature-only LRUCache. Cleaned up the dead code path from spotifyRecommender.ts and removed the currentFeatures parameter threading throughout the candidate collection pipeline. 2. **Add preferred-artist candidate source**: Artists saved on the Musical Taste page (via userArtistPreference table) now shape autoplay seeding via a new preferred-artist candidate source. New collectPreferredArtistCandidates seeder mirrors lastFmSeeder patterns: fetches artist top tracks via Last.fm, resolves to real Tracks via search, scores with a +0.2 boost so they reliably outrank generic sources. Wired into replenisher.ts to gather union of preferred artist names across all VC members and feed to the seeder immediately after recommendation candidates, before Last.fm sources. **Files changed**: - spotifyApi.ts: removed 5 dead functions + types - spotifyApi.spec.ts: dropped 3 describe blocks - spotifyRecommender.ts: removed collectSpotifyRecommendationCandidates - candidateCollector.ts: removed Spotify-recs call, dropped currentFeatures param - queueManipulation.ts: removed getTrackAudioFeatures, enrichWithAudioFeatures, audioFeatureCache - candidateScorer.ts: removed enrichWithAudioFeatures definition - replenisher.ts: removed audio-feature enrichment, wired preferred-artist seeder - lastFmApi.ts: added getArtistTopTracks (mirrors getSimilarTracks pattern) - lastfm/index.ts: re-exported getArtistTopTracks - feedbackService.ts: added getPreferredArtistNames - **NEW** preferredArtistSeeder.ts: collectPreferredArtistCandidates + searchLastFmQuery - Test updates: added coverage for new seeder and feedbackService method Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
LucasSantana-Dev
enabled auto-merge (squash)
April 19, 2026 15:30
📝 WalkthroughWalkthroughThe PR introduces Last.fm-based preferred artist candidate collection for autoplay, adding methods to fetch artist top tracks and user preferred artist names. Simultaneously, it removes Spotify audio-feature enrichment and recommendation logic throughout the recommendation pipeline. Changes
Sequence DiagramsequenceDiagram
participant Replenisher
participant FeedbackService
participant PreferredArtistSeeder
participant LastFmAPI
participant SearchEngine
participant Scorer
Replenisher->>FeedbackService: getPreferredArtistNames(guildId, userId)
FeedbackService->>FeedbackService: Query DB for preferred artists
FeedbackService-->>Replenisher: Set<artistNames>
Replenisher->>PreferredArtistSeeder: collectPreferredArtistCandidates(...preferredArtistNames)
loop For each preferred artist (rotated)
PreferredArtistSeeder->>LastFmAPI: getArtistTopTracks(artist, limit)
LastFmAPI-->>PreferredArtistSeeder: Track[] {artist, title}
PreferredArtistSeeder->>SearchEngine: searchLastFmQuery(queue, query)
SearchEngine-->>PreferredArtistSeeder: Track[] (filtered by duration)
loop For each search result
PreferredArtistSeeder->>Scorer: calculateRecommendationScore(...)
Scorer-->>PreferredArtistSeeder: score + reason
PreferredArtistSeeder->>PreferredArtistSeeder: upsertScoredCandidate(+BOOST)
end
end
PreferredArtistSeeder-->>Replenisher: (candidates map populated)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…fy-recs references
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/bot/src/utils/music/queueManipulation.spec.ts (2)
92-99:⚠️ Potential issue | 🟠 MajorRemove stale Spotify recommendation/audio-feature tests and mocks.
This PR removes
getAudioFeatures,getBatchAudioFeatures, andgetSpotifyRecommendationspaths, but this spec still mocks and asserts those calls. These tests will fail or keep validating dead behavior.🧹 Suggested cleanup direction
jest.mock('../../spotify/spotifyApi', () => ({ - getAudioFeatures: jest.fn().mockResolvedValue(null), searchSpotifyTrack: jest.fn().mockResolvedValue(null), - getBatchAudioFeatures: jest.fn().mockResolvedValue(new Map()), getArtistPopularity: jest.fn().mockResolvedValue(null), getArtistGenres: jest.fn().mockResolvedValue([]), - getSpotifyRecommendations: jest.fn().mockResolvedValue([]), }))Then delete the spec cases that assert audio-feature enrichment or Spotify recommendations are invoked.
Also applies to: 2792-3191, 3529-3597
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/utils/music/queueManipulation.spec.ts` around lines 92 - 99, The spec is still mocking and asserting removed Spotify paths (getAudioFeatures, getBatchAudioFeatures, getSpotifyRecommendations); remove those mocks from the jest.mock block and delete or disable the test cases that assert audio-feature enrichment or Spotify recommendation calls (any tests referencing getAudioFeatures, getBatchAudioFeatures, or getSpotifyRecommendations) so the spec no longer validates dead behavior; search for those symbol names in queueManipulation.spec.ts and remove the related mock entries and the test blocks that expect or verify those calls.
101-122:⚠️ Potential issue | 🔴 CriticalAdd
getPreferredArtistNamesto the feedback service mock throughout the spec.
replenisher.tsnow callsrecommendationFeedbackService.getPreferredArtistNames()in itsPromise.allblock (line 140). Without it in the mock, any test reaching that code path will fail withgetPreferredArtistNames is not a function.Fix locations
Add to the mock declaration (lines 101–122):
const getPreferredArtistKeysMock = jest.fn() +const getPreferredArtistNamesMock = jest.fn() const getBlockedArtistKeysMock = jest.fn()Add to the mock object definition (within the jest.mock call):
getPreferredArtistKeys: (...args: unknown[]) => getPreferredArtistKeysMock(...args), +getPreferredArtistNames: (...args: unknown[]) => + getPreferredArtistNamesMock(...args), getBlockedArtistKeys: (...args: unknown[]) =>Add to all
beforeEachblocks (lines 163–169, 2065–2071, 2158–2164, 2202–2208, 2264–2270, 2458–2465, 2551–2557, 3697–3703, 3789–3795, 4060–4066):getPreferredArtistKeysMock.mockResolvedValue(new Set()) +getPreferredArtistNamesMock.mockResolvedValue(new Set()) getBlockedArtistKeysMock.mockResolvedValue(new Set())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/utils/music/queueManipulation.spec.ts` around lines 101 - 122, The tests fail because recommendationFeedbackService.getPreferredArtistNames is now called by replenisher.ts but isn't mocked; add a new mock function (e.g. getPreferredArtistNamesMock = jest.fn()) alongside dislikedTrackWeightsMock/likedTrackWeightsMock/... and expose it in the jest.mock return under recommendationFeedbackService as getPreferredArtistNames: (...args: unknown[]) => getPreferredArtistNamesMock(...args); also update every beforeEach that resets/sets the other feedback mocks to initialize or mockImplementation for getPreferredArtistNamesMock so tests exercising replenisher.ts's Promise.all path won't throw "is not a function".packages/bot/src/services/musicRecommendation/feedbackService.spec.ts (1)
490-558:⚠️ Potential issue | 🔴 CriticalMove this
describeblock out of therecordImplicitFeedback trims...test and fix the mocking approach.The block is currently nested inside the test's
forloop (lines 490–558 within 492–560). The tests referenceservicebefore it's instantiated (line 563), and they incorrectly spy onmodule.getPrismaClientinstead of the imported function. The suite will fail with ReferenceError and invalid mock registration.Structural fix
Move the
describe('getPreferredArtistNames')block (lines 495–558) outside therecordImplicitFeedbacktest. Within each test, instantiate a fresh service and use(getPrismaClient as jest.Mock).mockReturnValue(mockDb)instead ofjest.spyOn(module, 'getPrismaClient').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/services/musicRecommendation/feedbackService.spec.ts` around lines 490 - 558, The getPreferredArtistNames test suite is accidentally nested inside the recordImplicitFeedback test and uses an improper spy; move the entire describe('getPreferredArtistNames') block out of the recordImplicitFeedback test (so it sits at the top-level in the file), and in each test instantiate a fresh service instance before calling service.getPreferredArtistNames; replace jest.spyOn(module, 'getPrismaClient') with (getPrismaClient as jest.Mock).mockReturnValue(mockDb) to mock the imported getPrismaClient function correctly.
🧹 Nitpick comments (1)
packages/bot/src/utils/music/autoplay/replenisher.ts (1)
151-154: Simplify the preferred-name union construction.
allPreferredArtistNamesalready contains sets, so the extranew Set(set)allocation is unnecessary.♻️ Proposed simplification
- const allPreferredArtistNameSets = allPreferredArtistNames.map((set) => new Set(set)) const preferredArtistNamesUnion = new Set<string>( - allPreferredArtistNameSets.flatMap((s) => [...s]), + allPreferredArtistNames.flatMap((s) => [...s]), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/utils/music/autoplay/replenisher.ts` around lines 151 - 154, The code creates intermediate allPreferredArtistNameSets by mapping each set to new Set(set) even though allPreferredArtistNames already contains Set instances; remove the unnecessary allPreferredArtistNameSets variable and build preferredArtistNamesUnion directly from allPreferredArtistNames by flattening each Set (e.g., flatMap(s => [...s])) into the new Set constructor so preferredArtistNamesUnion is created in one step.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bot/src/utils/music/autoplay/preferredArtistSeeder.spec.ts`:
- Around line 104-128: The blocked-key fixture is wrong: the test intends to
exercise the "skip blocked artists" branch in collectPreferredArtistCandidates
but uses 'Artist B' (normalizes to 'artistb') while blockedArtistKeys contains
'artistbkey', so lastfm.getArtistTopTracks is still called; update the
blockedArtistKeys set to contain the normalized key used by the function (e.g.,
'artistb') so collectPreferredArtistCandidates recognizes the artist as blocked
and the expectation that lastfm.getArtistTopTracks was not called will hold.
In `@packages/bot/src/utils/music/autoplay/preferredArtistSeeder.ts`:
- Around line 66-105: The candidate buffer size is only checked after each
artist loop so the inner search loop can overfill work; after calling
upsertScoredCandidate(...) inside the inner loop (the loop iterating over
searchTracks in preferredArtistSeeder.ts that processes searchedTrack),
immediately check if candidates.size >= AUTOPLAY_BUFFER_SIZE and break out of
the inner loop, and then ensure the outer topTracks loop also exits (use a
labeled loop, set a flag, or return early from the enclosing function) so no
further searchLastFmQuery or candidate processing occurs once the buffer is
full.
- Around line 56-61: The inline normalization in preferredArtistSeeder.ts
(calculating normalizedArtistKey from artistName) can miss variants cleaned by
RecommendationFeedbackService.cleanAuthor; change the code to use the same
cleaning function (import or access RecommendationFeedbackService.cleanAuthor)
when producing the artist key (e.g., compute cleaned = cleanAuthor(artistName)
and use that to check blockedArtistKeys.has(cleaned)), ensuring any
trimming/empty handling matches the feedback storage behavior.
---
Outside diff comments:
In `@packages/bot/src/services/musicRecommendation/feedbackService.spec.ts`:
- Around line 490-558: The getPreferredArtistNames test suite is accidentally
nested inside the recordImplicitFeedback test and uses an improper spy; move the
entire describe('getPreferredArtistNames') block out of the
recordImplicitFeedback test (so it sits at the top-level in the file), and in
each test instantiate a fresh service instance before calling
service.getPreferredArtistNames; replace jest.spyOn(module, 'getPrismaClient')
with (getPrismaClient as jest.Mock).mockReturnValue(mockDb) to mock the imported
getPrismaClient function correctly.
In `@packages/bot/src/utils/music/queueManipulation.spec.ts`:
- Around line 92-99: The spec is still mocking and asserting removed Spotify
paths (getAudioFeatures, getBatchAudioFeatures, getSpotifyRecommendations);
remove those mocks from the jest.mock block and delete or disable the test cases
that assert audio-feature enrichment or Spotify recommendation calls (any tests
referencing getAudioFeatures, getBatchAudioFeatures, or
getSpotifyRecommendations) so the spec no longer validates dead behavior; search
for those symbol names in queueManipulation.spec.ts and remove the related mock
entries and the test blocks that expect or verify those calls.
- Around line 101-122: The tests fail because
recommendationFeedbackService.getPreferredArtistNames is now called by
replenisher.ts but isn't mocked; add a new mock function (e.g.
getPreferredArtistNamesMock = jest.fn()) alongside
dislikedTrackWeightsMock/likedTrackWeightsMock/... and expose it in the
jest.mock return under recommendationFeedbackService as getPreferredArtistNames:
(...args: unknown[]) => getPreferredArtistNamesMock(...args); also update every
beforeEach that resets/sets the other feedback mocks to initialize or
mockImplementation for getPreferredArtistNamesMock so tests exercising
replenisher.ts's Promise.all path won't throw "is not a function".
---
Nitpick comments:
In `@packages/bot/src/utils/music/autoplay/replenisher.ts`:
- Around line 151-154: The code creates intermediate allPreferredArtistNameSets
by mapping each set to new Set(set) even though allPreferredArtistNames already
contains Set instances; remove the unnecessary allPreferredArtistNameSets
variable and build preferredArtistNamesUnion directly from
allPreferredArtistNames by flattening each Set (e.g., flatMap(s => [...s])) into
the new Set constructor so preferredArtistNamesUnion is created in one step.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6ab06d02-a22c-4556-9e74-b9016087f05f
📒 Files selected for processing (14)
packages/bot/src/lastfm/index.tspackages/bot/src/lastfm/lastFmApi.tspackages/bot/src/services/musicRecommendation/feedbackService.spec.tspackages/bot/src/services/musicRecommendation/feedbackService.tspackages/bot/src/spotify/spotifyApi.spec.tspackages/bot/src/spotify/spotifyApi.tspackages/bot/src/utils/music/autoplay/candidateCollector.tspackages/bot/src/utils/music/autoplay/candidateScorer.tspackages/bot/src/utils/music/autoplay/preferredArtistSeeder.spec.tspackages/bot/src/utils/music/autoplay/preferredArtistSeeder.tspackages/bot/src/utils/music/autoplay/replenisher.tspackages/bot/src/utils/music/autoplay/spotifyRecommender.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/queueManipulation.ts
💤 Files with no reviewable changes (4)
- packages/bot/src/utils/music/autoplay/candidateCollector.ts
- packages/bot/src/utils/music/autoplay/candidateScorer.ts
- packages/bot/src/spotify/spotifyApi.ts
- packages/bot/src/spotify/spotifyApi.spec.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Quality Gates
- GitHub Check: SonarCloud Scan
🔇 Additional comments (5)
packages/bot/src/lastfm/index.ts (1)
1-15: LGTM.The new
getArtistTopTracksexport is correctly wired through the Last.fm barrel module.packages/bot/src/lastfm/lastFmApi.ts (1)
324-350: LGTM.The helper follows the existing Last.fm client pattern and returns the simplified
{ artist, title }shape needed by the seeder.packages/bot/src/services/musicRecommendation/feedbackService.ts (1)
474-488: LGTM.The Prisma query shape matches the
UserArtistPreferenceschema, and trimming/filtering avoids blank artist names reaching the autoplay seeder.packages/bot/src/utils/music/autoplay/replenisher.ts (1)
97-154: LGTM.Preferred artist names are gathered across the requester/VC members and the new seeder is invoked before the Last.fm source, matching the intended priority.
Also applies to: 219-249
packages/bot/src/utils/music/autoplay/preferredArtistSeeder.ts (1)
7-11: No action needed. The import statement is correct.All three functions—
shouldIncludeCandidate,upsertScoredCandidate, andnormalizeTrackKey—are properly exported from../queueManipulation. WhileshouldIncludeCandidateandupsertScoredCandidateoriginate in./candidateCollector, they are explicitly re-exported fromqueueManipulation.tsfor backward compatibility. ThenormalizeTrackKeyfunction is defined directly inqueueManipulation.ts.> Likely an incorrect or invalid review comment.
Comment on lines
+104
to
+128
| it('skips blocked artists', async () => { | ||
| const blockedArtistKeys = new Set(['artistbkey']) | ||
| const candidates = new Map() | ||
|
|
||
| await collectPreferredArtistCandidates( | ||
| mockQueue as GuildQueue, | ||
| mockUser as User, | ||
| new Set(), | ||
| new Set(), | ||
| new Map(), | ||
| new Map(), | ||
| new Set(), | ||
| blockedArtistKeys, | ||
| mockCurrentTrack as Track, | ||
| new Set(), | ||
| candidates, | ||
| 'similar', | ||
| new Map(), | ||
| new Set(), | ||
| new Set(), | ||
| null, | ||
| ['Artist B'], | ||
| ) | ||
|
|
||
| expect(jest.mocked(lastfm.getArtistTopTracks)).not.toHaveBeenCalled() |
There was a problem hiding this comment.
Fix the blocked-key fixture so this test actually exercises the skip path.
'Artist B' normalizes to 'artistb', not 'artistbkey', so getArtistTopTracks will still be called and this assertion will fail.
✅ Proposed test fix
it('skips blocked artists', async () => {
- const blockedArtistKeys = new Set(['artistbkey'])
+ const blockedArtistKeys = new Set(['artistb'])
const candidates = new Map()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('skips blocked artists', async () => { | |
| const blockedArtistKeys = new Set(['artistbkey']) | |
| const candidates = new Map() | |
| await collectPreferredArtistCandidates( | |
| mockQueue as GuildQueue, | |
| mockUser as User, | |
| new Set(), | |
| new Set(), | |
| new Map(), | |
| new Map(), | |
| new Set(), | |
| blockedArtistKeys, | |
| mockCurrentTrack as Track, | |
| new Set(), | |
| candidates, | |
| 'similar', | |
| new Map(), | |
| new Set(), | |
| new Set(), | |
| null, | |
| ['Artist B'], | |
| ) | |
| expect(jest.mocked(lastfm.getArtistTopTracks)).not.toHaveBeenCalled() | |
| it('skips blocked artists', async () => { | |
| const blockedArtistKeys = new Set(['artistb']) | |
| const candidates = new Map() | |
| await collectPreferredArtistCandidates( | |
| mockQueue as GuildQueue, | |
| mockUser as User, | |
| new Set(), | |
| new Set(), | |
| new Map(), | |
| new Map(), | |
| new Set(), | |
| blockedArtistKeys, | |
| mockCurrentTrack as Track, | |
| new Set(), | |
| candidates, | |
| 'similar', | |
| new Map(), | |
| new Set(), | |
| new Set(), | |
| null, | |
| ['Artist B'], | |
| ) | |
| expect(jest.mocked(lastfm.getArtistTopTracks)).not.toHaveBeenCalled() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/utils/music/autoplay/preferredArtistSeeder.spec.ts` around
lines 104 - 128, The blocked-key fixture is wrong: the test intends to exercise
the "skip blocked artists" branch in collectPreferredArtistCandidates but uses
'Artist B' (normalizes to 'artistb') while blockedArtistKeys contains
'artistbkey', so lastfm.getArtistTopTracks is still called; update the
blockedArtistKeys set to contain the normalized key used by the function (e.g.,
'artistb') so collectPreferredArtistCandidates recognizes the artist as blocked
and the expectation that lastfm.getArtistTopTracks was not called will hold.
Comment on lines
+56
to
+61
| const normalizedArtistKey = artistName | ||
| .toLowerCase() | ||
| .replaceAll(/[^a-z0-9]+/g, '') | ||
| .trim() | ||
|
|
||
| if (blockedArtistKeys.has(normalizedArtistKey)) continue |
There was a problem hiding this comment.
Use the same artist-key normalization as feedback storage.
Blocked keys are produced via cleanAuthor(...) in RecommendationFeedbackService; this inline normalization can miss blocks for names with suffixes/noise that cleanAuthor strips.
🛡️ Proposed normalization alignment
-import { cleanSearchQuery } from '../searchQueryCleaner'
+import { cleanAuthor, cleanSearchQuery } from '../searchQueryCleaner'
@@
- const normalizedArtistKey = artistName
+ const normalizedArtistKey = cleanAuthor(artistName)
.toLowerCase()
.replaceAll(/[^a-z0-9]+/g, '')
.trim()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/utils/music/autoplay/preferredArtistSeeder.ts` around lines
56 - 61, The inline normalization in preferredArtistSeeder.ts (calculating
normalizedArtistKey from artistName) can miss variants cleaned by
RecommendationFeedbackService.cleanAuthor; change the code to use the same
cleaning function (import or access RecommendationFeedbackService.cleanAuthor)
when producing the artist key (e.g., compute cleaned = cleanAuthor(artistName)
and use that to check blockedArtistKeys.has(cleaned)), ensuring any
trimming/empty handling matches the feedback storage behavior.
Comment on lines
+66
to
+105
| for (const track of topTracks) { | ||
| const query = cleanSearchQuery(track.title, track.artist) | ||
| const searchTracks = await searchLastFmQuery(queue, query, requestedBy) | ||
| for (const searchedTrack of searchTracks) { | ||
| if (!shouldIncludeCandidate(searchedTrack, excludedUrls, excludedKeys)) | ||
| continue | ||
| const normalizedKey = normalizeTrackKey( | ||
| searchedTrack.title, | ||
| searchedTrack.author, | ||
| ) | ||
| const dislikedWeight = dislikedWeights.get(normalizedKey) | ||
| if (dislikedWeight !== undefined && dislikedWeight > 0.5) continue | ||
|
|
||
| const rec = calculateRecommendationScore( | ||
| searchedTrack, | ||
| currentTrack, | ||
| recentArtists, | ||
| likedWeights, | ||
| preferredArtistKeys, | ||
| blockedArtistKeys, | ||
| autoplayMode, | ||
| artistFrequency, | ||
| implicitDislikeKeys, | ||
| implicitLikeKeys, | ||
| dislikedWeights, | ||
| sessionMood, | ||
| true, | ||
| ) | ||
| if (rec.score === -Infinity) continue | ||
|
|
||
| upsertScoredCandidate(candidates, searchedTrack, { | ||
| score: rec.score + PREFERRED_ARTIST_SCORE_BOOST, | ||
| reason: rec.reason | ||
| ? `${rec.reason} • preferred artist` | ||
| : 'preferred artist', | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| if (candidates.size >= AUTOPLAY_BUFFER_SIZE) break |
There was a problem hiding this comment.
Stop as soon as the candidate buffer is full.
The buffer check only runs after an artist finishes, so one artist can trigger up to topTracks × searchResults candidate work after the buffer is already full. This can add avoidable Last.fm/search latency during autoplay replenishment.
⚡ Proposed early-exit guard
for (const track of topTracks) {
+ if (candidates.size >= AUTOPLAY_BUFFER_SIZE) break
+
const query = cleanSearchQuery(track.title, track.artist)
const searchTracks = await searchLastFmQuery(queue, query, requestedBy)
for (const searchedTrack of searchTracks) {
+ if (candidates.size >= AUTOPLAY_BUFFER_SIZE) break
+
if (!shouldIncludeCandidate(searchedTrack, excludedUrls, excludedKeys))
continue
@@
if (candidates.size >= AUTOPLAY_BUFFER_SIZE) break📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for (const track of topTracks) { | |
| const query = cleanSearchQuery(track.title, track.artist) | |
| const searchTracks = await searchLastFmQuery(queue, query, requestedBy) | |
| for (const searchedTrack of searchTracks) { | |
| if (!shouldIncludeCandidate(searchedTrack, excludedUrls, excludedKeys)) | |
| continue | |
| const normalizedKey = normalizeTrackKey( | |
| searchedTrack.title, | |
| searchedTrack.author, | |
| ) | |
| const dislikedWeight = dislikedWeights.get(normalizedKey) | |
| if (dislikedWeight !== undefined && dislikedWeight > 0.5) continue | |
| const rec = calculateRecommendationScore( | |
| searchedTrack, | |
| currentTrack, | |
| recentArtists, | |
| likedWeights, | |
| preferredArtistKeys, | |
| blockedArtistKeys, | |
| autoplayMode, | |
| artistFrequency, | |
| implicitDislikeKeys, | |
| implicitLikeKeys, | |
| dislikedWeights, | |
| sessionMood, | |
| true, | |
| ) | |
| if (rec.score === -Infinity) continue | |
| upsertScoredCandidate(candidates, searchedTrack, { | |
| score: rec.score + PREFERRED_ARTIST_SCORE_BOOST, | |
| reason: rec.reason | |
| ? `${rec.reason} • preferred artist` | |
| : 'preferred artist', | |
| }) | |
| } | |
| } | |
| if (candidates.size >= AUTOPLAY_BUFFER_SIZE) break | |
| for (const track of topTracks) { | |
| if (candidates.size >= AUTOPLAY_BUFFER_SIZE) break | |
| const query = cleanSearchQuery(track.title, track.artist) | |
| const searchTracks = await searchLastFmQuery(queue, query, requestedBy) | |
| for (const searchedTrack of searchTracks) { | |
| if (candidates.size >= AUTOPLAY_BUFFER_SIZE) break | |
| if (!shouldIncludeCandidate(searchedTrack, excludedUrls, excludedKeys)) | |
| continue | |
| const normalizedKey = normalizeTrackKey( | |
| searchedTrack.title, | |
| searchedTrack.author, | |
| ) | |
| const dislikedWeight = dislikedWeights.get(normalizedKey) | |
| if (dislikedWeight !== undefined && dislikedWeight > 0.5) continue | |
| const rec = calculateRecommendationScore( | |
| searchedTrack, | |
| currentTrack, | |
| recentArtists, | |
| likedWeights, | |
| preferredArtistKeys, | |
| blockedArtistKeys, | |
| autoplayMode, | |
| artistFrequency, | |
| implicitDislikeKeys, | |
| implicitLikeKeys, | |
| dislikedWeights, | |
| sessionMood, | |
| true, | |
| ) | |
| if (rec.score === -Infinity) continue | |
| upsertScoredCandidate(candidates, searchedTrack, { | |
| score: rec.score + PREFERRED_ARTIST_SCORE_BOOST, | |
| reason: rec.reason | |
| ? `${rec.reason} • preferred artist` | |
| : 'preferred artist', | |
| }) | |
| } | |
| } | |
| if (candidates.size >= AUTOPLAY_BUFFER_SIZE) break |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/utils/music/autoplay/preferredArtistSeeder.ts` around lines
66 - 105, The candidate buffer size is only checked after each artist loop so
the inner search loop can overfill work; after calling
upsertScoredCandidate(...) inside the inner loop (the loop iterating over
searchTracks in preferredArtistSeeder.ts that processes searchedTrack),
immediately check if candidates.size >= AUTOPLAY_BUFFER_SIZE and break out of
the inner loop, and then ensure the outer topTracks loop also exits (use a
labeled loop, set a flag, or return early from the enclosing function) so no
further searchLastFmQuery or candidate processing occurs once the buffer is
full.
|
Owner
Author
|
Closing — the refactor compiles (0 TS errors) but breaks replenisher tests (0 candidates produced where >=1 expected, ~12 failures across queueManipulation/candidateScorer/candidateCollector/replenisher/preferredArtistSeeder/feedbackService specs). The dead Spotify endpoints (/v1/recommendations, /v1/audio-features) were already failing silently — leaving them in place doesn't actively harm autoplay. Will re-attempt with proper test mock updates after the live UX bugs (no Discover pictures, broken genre autoplay) are addressed in the in-flight ultraplan. |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/v1/recommendationsand/v1/audio-featurescalls + audio-feature enrichment pipeline (getSpotifyRecommendations, SpotifyAudioFeatures, getAudioFeatures, getBatchAudioFeatures, enrichWithAudioFeatures, getTrackAudioFeatures). These endpoints were silently 404/403'ing in production for months and contributed nothing to autoplay.collectPreferredArtistCandidatessource that fetches top tracks from Last.fm for artists marked as "preferred" on the Musical Taste page, resolves them to real Tracks, and seeds the autoplay pool with a +0.2 score boost so UI preferences actually shape playback.Files Changed
Deleted:
spotifyApi.ts,spotifyRecommender.ts,queueManipulation.ts,candidateScorer.tsreplenisher.tscollectSpotifyRecommendationCandidatesfromspotifyRecommender.tsAdded:
preferredArtistSeeder.ts+preferredArtistSeeder.spec.tsgetArtistTopTracks()tolastFmApi.tsgetPreferredArtistNames()tofeedbackService.tsUpdated:
replenisher.ts: gather preferred artist names, call seedercandidateCollector.ts: remove Spotify-recs call, drop currentFeatures paramManual Test Plan
Summary by CodeRabbit
New Features
Removals
Tests