Repository navigation
feat(bot): last.fm duration fix, normalizers, and top tracks seeds - #382
Conversation
📝 WalkthroughWalkthroughAdds Last.fm-based autoplay seeds (fetches/caches a user’s 3-month top tracks, samples them into candidate selection with a "last.fm taste" tag), adds Last.fm artist/title normalization, fixes duration computation for Last.fm scrobbling, and updates tests and re-exports. Changes
Sequence DiagramsequenceDiagram
participant User
participant QueueMgr as QueueManipulation
participant Cache as LastFmSeedsCache
participant LastFmAPI as Last.fmAPI
participant Search as PlayerSearch
participant CandPool as CandidatesPool
User->>QueueMgr: replenishQueue(requestedBy)
QueueMgr->>Cache: getLastFmSeedTracks(userId)
alt cached
Cache->>QueueMgr: return cached tracks
else not cached
Cache->>LastFmAPI: getTopTracks(username, '3month', 20)
LastFmAPI-->>Cache: top tracks [{artist,title,playCount}...]
Cache->>Cache: store with 1h TTL
Cache->>QueueMgr: return mapped [{artist,title}...]
end
QueueMgr->>QueueMgr: collectLastFmCandidates()
QueueMgr->>QueueMgr: sample ≤3 seeds
loop for each sampled seed
QueueMgr->>Search: search("title artist", AUTO)
Search-->>CandPool: results
CandPool->>CandPool: compute score + LASTFM_SCORE_BOOST
CandPool->>CandPool: tag reason "last.fm taste"
end
QueueMgr->>CandPool: selectDiverseCandidates()
QueueMgr->>User: return replenished queue
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
🚥 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
packages/bot/src/utils/music/autoplay/lastFmSeeds.spec.ts (1)
30-36: Minor: redundantclearAllMocksin bothbeforeEachandafterEach.Both hooks call
jest.clearAllMocks(). One is sufficient—typicallybeforeEachis preferred to ensure clean state before each test.🧹 Remove redundant afterEach
beforeEach(() => { jest.clearAllMocks() }) - - afterEach(() => { - jest.clearAllMocks() - })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/utils/music/autoplay/lastFmSeeds.spec.ts` around lines 30 - 36, The test setup redundantly calls jest.clearAllMocks() in both beforeEach and afterEach; remove the afterEach hook and keep a single jest.clearAllMocks() in beforeEach to ensure a clean state before each test; locate the beforeEach and afterEach functions around lastFmSeeds.spec.ts and delete the afterEach block (which only calls jest.clearAllMocks()), leaving beforeEach intact.packages/bot/src/utils/music/queueManipulation.ts (1)
432-475: Refactor:collectLastFmCandidatesexceeds parameter and complexity limits.SonarCloud flags:
- 9 parameters (maximum 7)
- Cognitive complexity 17 (maximum 15)
Consider extracting a context object to reduce the parameter count and simplify the function signature.
♻️ Proposed refactor: Extract candidate collection context
+type CandidateCollectionContext = { + queue: GuildQueue + excludedUrls: Set<string> + excludedKeys: Set<string> + dislikedTrackKeys: Set<string> + likedTrackKeys: Set<string> + currentTrack: Track + recentArtists: Set<string> + candidates: Map<string, ScoredTrack> +} + async function collectLastFmCandidates( - queue: GuildQueue, requestedBy: User, - excludedUrls: Set<string>, - excludedKeys: Set<string>, - dislikedTrackKeys: Set<string>, - likedTrackKeys: Set<string>, - currentTrack: Track, - recentArtists: Set<string>, - candidates: Map<string, ScoredTrack>, + ctx: CandidateCollectionContext, ): Promise<void> { + const { + queue, + excludedUrls, + excludedKeys, + dislikedTrackKeys, + likedTrackKeys, + currentTrack, + recentArtists, + candidates, + } = ctx // ... rest of function }This pattern could also be applied to
collectRecommendationCandidatesfor consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/utils/music/queueManipulation.ts` around lines 432 - 475, collectLastFmCandidates currently has too many parameters and high complexity; create a CandidateCollectionContext (or similar) to bundle queue, requestedBy, excludedUrls, excludedKeys, dislikedTrackKeys, likedTrackKeys, currentTrack, recentArtists, and candidates into one object, then change collectLastFmCandidates signature to accept that context plus any small primitives (e.g., LASTFM_SEED_COUNT if needed); also extract the inner logic that builds seeds and the nested loops into helper functions like buildLastFmSeeds(context, lastFmTracks) and processLastFmTrack(context, track, currentTrack) to reduce cognitive complexity and keep upsertScoredCandidate/shouldIncludeCandidate/normalizeTrackKey/calculateRecommendationScore usage intact; update all callers to pass the new context object.packages/bot/src/utils/music/autoplay/lastFmSeeds.ts (1)
13-13: Consider bounding the in-memory cache to prevent unbounded growth.The
cacheMap grows indefinitely as new Discord users request autoplay. While entries have a 1-hour TTL, stale entries are only evicted on access—they're never proactively cleaned. For a bot serving many guilds, this could lead to memory pressure over time.A simple improvement would be to cap the cache size (e.g., LRU with 500 entries, consistent with other bounded maps in the codebase like
lastPlayedTracks).💡 Optional: Add cache size limit
const cache = new Map<string, CacheEntry>() +const MAX_CACHE_SIZE = 500 + +function pruneCache(): void { + if (cache.size <= MAX_CACHE_SIZE) return + const now = Date.now() + for (const [key, entry] of cache) { + if (entry.expiresAt <= now || cache.size > MAX_CACHE_SIZE) { + cache.delete(key) + } + } +}Call
pruneCache()before setting a new entry.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/utils/music/autoplay/lastFmSeeds.ts` at line 13, The cache Map named `cache` (type Map<string, CacheEntry>) is unbounded; implement a size cap (e.g., 500) and LRU-style eviction to prevent memory growth: add a small helper (e.g., `pruneCache()` or `ensureCacheLimit()`) that is called before inserting into `cache` from functions that call `cache.set(...)`, which removes the oldest entries (or expired entries first) until size <= 500 while preserving the existing TTL logic in `CacheEntry`; alternatively replace `cache` with a simple LRU wrapper that exposes the same .get/.set/.delete semantics but evicts least-recently-used keys when capacity is exceeded — reference `cache`, `CacheEntry`, and `pruneCache()` when making the change.packages/bot/src/lastfm/lastFmApi.ts (4)
135-135: UseNumber.parseIntinstead of globalparseInt.Static analysis flags that
Number.parseIntis preferred over the globalparseIntfunction for clarity and to avoid potential issues with the global scope.♻️ Proposed change
- playCount: parseInt(t.playcount, 10) || 0, + playCount: Number.parseInt(t.playcount, 10) || 0,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/lastfm/lastFmApi.ts` at line 135, Replace the global parseInt usage with the static Number.parseInt in the mapping that sets playCount (currently using parseInt(t.playcount, 10) || 0) inside lastFmApi.ts; update the expression to use Number.parseInt(t.playcount, 10) || 0 so you avoid relying on the global and satisfy static analysis.
86-88: Consider simplifying regex complexity and fixing escape character.The
TITLE_NOISE_PARENSregex has high complexity (35 vs recommended 20) which impacts maintainability. Also, line 88 has an unnecessary escape\[inside the character class.Consider splitting
TITLE_NOISE_PARENSinto separate patterns for clearer intent:♻️ Proposed simplification
-const TITLE_NOISE_PARENS = - /\s*[([](official\s*(music\s*)?video|official\s*audio|audio|lyric\s*video|lyrics?|live|hd|4k|ft\.?[^)\]]*|feat\.?[^)\]]*)[)\]]/gi -const FEAT_CLAUSE = /\s*[\[(]?feat\.?\s+[^\])[]+[\])]?/gi +const OFFICIAL_NOISE = /\s*[([](official\s*(music\s*)?video|official\s*audio)[)\]]/gi +const MEDIA_TAGS = /\s*[([](audio|lyric\s*video|lyrics?|live|hd|4k)[)\]]/gi +const FEAT_PARENS = /\s*[([](ft\.?|feat\.?)[^)\]]*[)\]]/gi +const FEAT_CLAUSE = /\s*[[(]?feat\.?\s+[^\])[(]+[\])]?/giThen chain them in
normalizeLastFmTitle:export function normalizeLastFmTitle(raw: string): string { - return raw.replace(TITLE_NOISE_PARENS, '').replace(FEAT_CLAUSE, '').trim() + return raw + .replace(OFFICIAL_NOISE, '') + .replace(MEDIA_TAGS, '') + .replace(FEAT_PARENS, '') + .replace(FEAT_CLAUSE, '') + .trim() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/lastfm/lastFmApi.ts` around lines 86 - 88, Split the overly complex TITLE_NOISE_PARENS into several focused, simpler regexes (e.g., TITLE_NOISE_VIDEO, TITLE_NOISE_AUDIO, TITLE_NOISE_VERSION/FEAT) and apply them sequentially in normalizeLastFmTitle to strip noise, replacing the single complex pattern; also fix the FEAT_CLAUSE character-class bug by escaping the literal '[' inside the class (change [^\])[]+ to [^\])\[]+ or equivalent) so the pattern is correct and less error-prone.
94-96: UsereplaceAll()for clarity with global replacements.Static analysis flags that
replaceAll()is preferred overreplace()when using global regex patterns. While functionally equivalent,replaceAll()more clearly expresses intent.♻️ Proposed change
export function normalizeLastFmTitle(raw: string): string { - return raw.replace(TITLE_NOISE_PARENS, '').replace(FEAT_CLAUSE, '').trim() + return raw.replaceAll(TITLE_NOISE_PARENS, '').replaceAll(FEAT_CLAUSE, '').trim() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/lastfm/lastFmApi.ts` around lines 94 - 96, The normalizeLastFmTitle function currently uses replace(...) twice; update it to use replaceAll(...) for the two global regex replacements (TITLE_NOISE_PARENS and FEAT_CLAUSE) to make the intent explicit and satisfy static analysis. Locate the normalizeLastFmTitle function and swap the two .replace(...) calls to .replaceAll(...), preserving the same regex constants and the final .trim() call on the result.
98-103: Type naming convention.Per coding guidelines, type aliases should use
T{Name}naming convention, and interfaces for public object shapes should useI{Name}.♻️ Proposed naming convention fix
-export type LastFmTopTrack = { +export interface ILastFmTopTrack { artist: string title: string playCount: number } -export type LastFmPeriod = '7day' | '1month' | '3month' | '6month' | '12month' +export type TLastFmPeriod = '7day' | '1month' | '3month' | '6month' | '12month'As per coding guidelines: "Use
I{Name}naming convention for interfaces in TypeScript" and "UseT{Name}naming convention for type aliases and utility types in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/lastfm/lastFmApi.ts` around lines 98 - 103, Rename the public object shape type LastFmTopTrack to an interface ILastFmTopTrack and keep the period union as a type but rename LastFmPeriod to TLastFmPeriod; update the declaration names (export interface ILastFmTopTrack { artist: string; title: string; playCount: number }) and (export type TLastFmPeriod = '7day' | '1month' | '3month' | '6month' | '12month'), then update every usage/import/annotation referencing LastFmTopTrack and LastFmPeriod (e.g., function signatures, return types, tests) to use ILastFmTopTrack and TLastFmPeriod respectively to satisfy the naming conventions.
🤖 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/lastfm/lastFmApi.ts`:
- Around line 120-139: The fetch in the Last.fm call (the block using API_BASE
and params in lastFmApi.ts that returns mapped toptracks) needs an
AbortController timeout to avoid dangling requests: create an AbortController,
start a timer (e.g., setTimeout) to call controller.abort() after a configurable
timeout, pass controller.signal to fetch(`${API_BASE}?${params.toString()}`),
and clear the timer after fetch completes; ensure the catch handles aborts
(returning [] as now) and any timer is cleaned up to avoid leaks.
---
Nitpick comments:
In `@packages/bot/src/lastfm/lastFmApi.ts`:
- Line 135: Replace the global parseInt usage with the static Number.parseInt in
the mapping that sets playCount (currently using parseInt(t.playcount, 10) || 0)
inside lastFmApi.ts; update the expression to use Number.parseInt(t.playcount,
10) || 0 so you avoid relying on the global and satisfy static analysis.
- Around line 86-88: Split the overly complex TITLE_NOISE_PARENS into several
focused, simpler regexes (e.g., TITLE_NOISE_VIDEO, TITLE_NOISE_AUDIO,
TITLE_NOISE_VERSION/FEAT) and apply them sequentially in normalizeLastFmTitle to
strip noise, replacing the single complex pattern; also fix the FEAT_CLAUSE
character-class bug by escaping the literal '[' inside the class (change
[^\])[]+ to [^\])\[]+ or equivalent) so the pattern is correct and less
error-prone.
- Around line 94-96: The normalizeLastFmTitle function currently uses
replace(...) twice; update it to use replaceAll(...) for the two global regex
replacements (TITLE_NOISE_PARENS and FEAT_CLAUSE) to make the intent explicit
and satisfy static analysis. Locate the normalizeLastFmTitle function and swap
the two .replace(...) calls to .replaceAll(...), preserving the same regex
constants and the final .trim() call on the result.
- Around line 98-103: Rename the public object shape type LastFmTopTrack to an
interface ILastFmTopTrack and keep the period union as a type but rename
LastFmPeriod to TLastFmPeriod; update the declaration names (export interface
ILastFmTopTrack { artist: string; title: string; playCount: number }) and
(export type TLastFmPeriod = '7day' | '1month' | '3month' | '6month' |
'12month'), then update every usage/import/annotation referencing LastFmTopTrack
and LastFmPeriod (e.g., function signatures, return types, tests) to use
ILastFmTopTrack and TLastFmPeriod respectively to satisfy the naming
conventions.
In `@packages/bot/src/utils/music/autoplay/lastFmSeeds.spec.ts`:
- Around line 30-36: The test setup redundantly calls jest.clearAllMocks() in
both beforeEach and afterEach; remove the afterEach hook and keep a single
jest.clearAllMocks() in beforeEach to ensure a clean state before each test;
locate the beforeEach and afterEach functions around lastFmSeeds.spec.ts and
delete the afterEach block (which only calls jest.clearAllMocks()), leaving
beforeEach intact.
In `@packages/bot/src/utils/music/autoplay/lastFmSeeds.ts`:
- Line 13: The cache Map named `cache` (type Map<string, CacheEntry>) is
unbounded; implement a size cap (e.g., 500) and LRU-style eviction to prevent
memory growth: add a small helper (e.g., `pruneCache()` or `ensureCacheLimit()`)
that is called before inserting into `cache` from functions that call
`cache.set(...)`, which removes the oldest entries (or expired entries first)
until size <= 500 while preserving the existing TTL logic in `CacheEntry`;
alternatively replace `cache` with a simple LRU wrapper that exposes the same
.get/.set/.delete semantics but evicts least-recently-used keys when capacity is
exceeded — reference `cache`, `CacheEntry`, and `pruneCache()` when making the
change.
In `@packages/bot/src/utils/music/queueManipulation.ts`:
- Around line 432-475: collectLastFmCandidates currently has too many parameters
and high complexity; create a CandidateCollectionContext (or similar) to bundle
queue, requestedBy, excludedUrls, excludedKeys, dislikedTrackKeys,
likedTrackKeys, currentTrack, recentArtists, and candidates into one object,
then change collectLastFmCandidates signature to accept that context plus any
small primitives (e.g., LASTFM_SEED_COUNT if needed); also extract the inner
logic that builds seeds and the nested loops into helper functions like
buildLastFmSeeds(context, lastFmTracks) and processLastFmTrack(context, track,
currentTrack) to reduce cognitive complexity and keep
upsertScoredCandidate/shouldIncludeCandidate/normalizeTrackKey/calculateRecommendationScore
usage intact; update all callers to pass the new context object.
🪄 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: d7096f6b-9210-45ef-a74f-59fa463bfba4
📒 Files selected for processing (9)
CHANGELOG.mdpackages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/lastfm/index.tspackages/bot/src/lastfm/lastFmApi.spec.tspackages/bot/src/lastfm/lastFmApi.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/queueManipulation.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (19)
**/*.{js,jsx,ts,tsx,vue,html}
📄 CodeRabbit inference engine (.cursor/rules/accessibility-openness.mdc)
Provide accessible UI components using semantic HTML and ARIA attributes where necessary
Files:
packages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/lastfm/index.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.tspackages/bot/src/lastfm/lastFmApi.spec.tspackages/bot/src/lastfm/lastFmApi.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dependency-injection.mdc)
**/*.{ts,tsx,js,jsx}: Prefer constructor injection for classes that require dependencies
Avoid global mutable singletons unless necessary
Use explicit interfaces for external dependencies to make testing easier
**/*.{ts,tsx,js,jsx}: Include required references in PRs/code for non-trivial logic: TypeScript (official docs), MDN (JavaScript reference), and official docs for any runtime/framework/libraries used (e.g., Node.js, React) as applicable.
Before assuming behavior of an API, include the doc link and a ≤25-word quote when the change relies on it.
**/*.{ts,tsx,js,jsx}: Prefer named exports for clear usage and easier refactors in TypeScript/JavaScript
Keep import order consistent: external first, then internal modules
Remove dead code and unused imports
**/*.{ts,tsx,js,jsx}: Use PascalCase naming convention for React/UI components
Use camelCase naming convention for variables and functions
Use UPPER_SNAKE_CASE naming convention for constants
Maintain consistent import grouping and ordering within the project, keeping third-party imports separate from local imports
For external data sources (HTTP, database), always validate and sanitize input using type guards or schema validators
**/*.{ts,tsx,js,jsx}: Use Prettier with no semicolons, single quotes, 4-space indent, 80 character width
Files must not exceed 250 lines and this is enforcedImplement TypeScript typecheck and linter in CI quality checks
**/*.{ts,tsx,js,jsx}: Use TypeScript for enhanced type safety
Implement error handling and error logging
Avoid commenting code unless extremely necessary - code should explain itself with descriptive names
Leave NO todos, placeholders or missing pieces in the code
Variables and functions must use camelCase
Constants must use UPPER_SNAKE_CASE
Use arrow functions for methods and computed properties
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Maintain consistent import grouping/order: external imports first, then...
Files:
packages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/lastfm/index.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.tspackages/bot/src/lastfm/lastFmApi.spec.tspackages/bot/src/lastfm/lastFmApi.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{js,jsx,ts,tsx}: Never throw strings. ThrowError(or typed subclasses) with descriptive messages
Include causal error ascausewhen available for better debugging
Define clear, stable error codes (e.g.,ERR_AUTH_EXPIRED,ERR_NETWORK_TIMEOUT)
Provide optional metadata (e.g.,details,retryable,status,correlationId) in error objects
Use domain error classes per area (e.g.,AuthenticationError,ValidationError,NetworkError)
Log errors with structure (message, code, stack, cause, correlationId, user context where appropriate)
MarkretryablevsnonRetryableerrors where helpful for operations
Set timeouts and handle aborts/cancellations; avoid dangling requests in API/network code
Implement backoff for transient failures; avoid infinite retries
Map HTTP status → domain errors; 4xx vs 5xx behave differently (e.g., retry for 5xx/network)
**/*.{js,jsx,ts,tsx}: Use functional components with hooks in React/React Native. Avoid class components.
Keep components focused on a single responsibility; extract complex logic into custom hooks.
Keep state local when possible. Use Context/Zustand/Redux only when necessary for state management.
If props or state traverse more than 3 levels, consider using context or a feature-scoped store instead of prop drilling.
Use performance optimization techniques:React.memo,useMemo,useCallback,Suspense(web), and virtualization for long lists; avoid unnecessary re-renders.
Web accessibility: use semantic HTML, labels, focus management, keyboard navigation, andaria-*attributes as needed.
React Native accessibility: use accessibility props (accessible,accessibilityLabel), proper roles and labels.
Identify and extract repetitive UI components proactively tocomponents/with clear props and minimal coupling.
Web styles: prefer co-located styles or design system tokens; avoid global style leakage.
React Native styles: preferStyleSheet.create, design tokens, and theme providers; avoid in...
Files:
packages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/lastfm/index.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.tspackages/bot/src/lastfm/lastFmApi.spec.tspackages/bot/src/lastfm/lastFmApi.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
Introduce interfaces at module boundaries to enable testing and substitutions
**/*.{ts,tsx}: Avoid usinganytype in TypeScript. If unavoidable, useunknownwith type guards and justify with a code comment
Preferinterfacefor defining public object shapes in TypeScript, usetypefor unions and utility types
Use TypeScript utility types such asPartial,Pick,Omit,Readonly, andRecordwhen appropriate
UseI{Name}naming convention for interfaces in TypeScript
UseT{Name}naming convention for type aliases and utility types in TypeScript
**/*.{ts,tsx}: Functions must be less than 50 lines with cyclomatic complexity less than 10
Do not useanytypes - ESLint enforces this at error level
**/*.{ts,tsx}: Prefer types over interfaces for most cases
Don't ever useany- type safety always
Avoid enums; use const objects instead
For complex types, create a separate file to declare them and import them
Avoid usinganytype; if unavoidable, useunknownwith type guards and justify with code comment
Preferinterfacefor public API shapes; usetypefor unions and utility types
Use TypeScript utility types (Partial, Pick, Omit, Readonly, Record)
Files:
packages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/lastfm/index.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.tspackages/bot/src/lastfm/lastFmApi.spec.tspackages/bot/src/lastfm/lastFmApi.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/documentation.mdc)
**/*.{js,ts,tsx,jsx}: Minimize comments in code; explain the 'why' when non-obvious, let code express the 'what' through clear naming
Document trade-offs briefly when deviating from ideal patterns
**/*.{js,ts,tsx,jsx}: Store secrets, ports, and hosts in environment variables (.env,.env.example) and never hardcode them
Avoid redundant or decorative AI comments; code should be self-explanatory and only commented when logic is non-obvious; prefer refactoring over lengthy commentsNever hardcode secrets, IPs, or ports; use
.envanddocs/for required configuration variables
Files:
packages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/lastfm/index.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.tspackages/bot/src/lastfm/lastFmApi.spec.tspackages/bot/src/lastfm/lastFmApi.ts
packages/bot/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)
packages/bot/**/*.{ts,tsx}: UseuseMainPlayer()fromdiscord-playerto access the player instance; do not instantiate player directly
Do not duplicate queue or player state outside Discord Player; use shared services from@lucky/sharedfor persistent data like track history and session information
UseerrorLoganddebugLogfrom@lucky/shared/utilsfor logging throughout the bot package
Use embed and reply utilities from@lucky/sharedfor consistent message formatting and error sanitization across the bot
Use services from@lucky/shared(DatabaseService, Redis client) for database and cache access; do not instantiate Prisma or Redis directly in the bot package
Files:
packages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/lastfm/index.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.tspackages/bot/src/lastfm/lastFmApi.spec.tspackages/bot/src/lastfm/lastFmApi.ts
packages/bot/src/handlers/player/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)
Track handling, errors, and lifecycle must be managed through dedicated handlers in
packages/bot/src/handlers/player/(trackHandlers, errorHandlers, lifecycleHandlers)
Files:
packages/bot/src/handlers/player/trackNowPlaying.ts
packages/bot/**
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
The
botpackage depends onsharedand contains Discord bot commands and player handlers using Discord.js and Discord Player
Files:
packages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/lastfm/index.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.tspackages/bot/src/lastfm/lastFmApi.spec.tspackages/bot/src/lastfm/lastFmApi.ts
**/*.{js,mjs,ts,mts}
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
Use Node.js version ≥22 with ESM (ECMAScript modules) only; no CommonJS
Files:
packages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/lastfm/index.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.tspackages/bot/src/lastfm/lastFmApi.spec.tspackages/bot/src/lastfm/lastFmApi.ts
packages/bot/src/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)
Use
@lucky/sharedfor database, Redis, logging, and embed utilities instead of implementing them locally
Files:
packages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/lastfm/index.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.tspackages/bot/src/lastfm/lastFmApi.spec.tspackages/bot/src/lastfm/lastFmApi.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.{test,spec}.{js,jsx,ts,tsx}: Test behavior, not implementation. Prefer Testing Library utilities for testing React/React Native components.
For React Native tests: mock native modules and test component interactions and accessibility labels.
Files:
packages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/lastfm/lastFmApi.spec.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Test behavior, not implementation details
Prefer unit tests for core logic; add integration tests at meaningful boundaries
Files:
packages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/lastfm/lastFmApi.spec.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/testing-quality.mdc)
**/*.{test,spec}.{js,ts,jsx,tsx}: Use Jest + a React testing library for unit and component tests as applicable
Test behavior, not implementation details
Files:
packages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/lastfm/lastFmApi.spec.ts
**/*.{spec,test}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/quality.mdc)
**/*.{spec,test}.{ts,tsx,js,jsx}: Use Jest for unit and integration tests
Test behavior, not implementation details
Run unit, integration tests, and coverage report in CI quality checks
Files:
packages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/lastfm/lastFmApi.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/quality.mdc)
Unit tests must use naming convention
*.spec.ts
Files:
packages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/lastfm/lastFmApi.spec.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
Use
index.tsonly to re-export a small, intentional surface per module
Files:
packages/bot/src/lastfm/index.ts
{CHANGELOG.md,README.md}
📄 CodeRabbit inference engine (.cursor/rules/agent-rules.mdc)
ALWAYS update CHANGELOG.md and README.md as changes are made.
Files:
CHANGELOG.md
CHANGELOG.md
📄 CodeRabbit inference engine (.cursor/rules/templates-examples.mdc)
CHANGELOG.md must be updated with all changes in pull requests
Always update CHANGELOG.md with all code changes
Update CHANGELOG.md with all changes, include breaking changes documentation, and reference issues and PRs
Files:
CHANGELOG.md
{CHANGELOG.md,docs/**}
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
Update
CHANGELOG.mdand relevantdocs/files when behavior or setup changes
Files:
CHANGELOG.md
🧠 Learnings (13)
📚 Learning: 2026-03-09T20:21:52.065Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/music/commands/**/*.ts : Use `.cursor/skills/music-queue-player/SKILL.md` for play, queue, skip, volume commands and player lifecycle management
Applied to files:
packages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/lastfm/index.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.ts
📚 Learning: 2026-03-09T20:21:52.065Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/music/**/*.ts : Use existing voice/queue/guild validators before manipulating player or queue state
Applied to files:
packages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/src/handlers/player/**/*.{ts,tsx} : Track handling, errors, and lifecycle must be managed through dedicated handlers in `packages/bot/src/handlers/player/` (trackHandlers, errorHandlers, lifecycleHandlers)
Applied to files:
packages/bot/src/handlers/player/trackNowPlaying.ts
📚 Learning: 2026-03-09T20:21:52.065Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/{general,music,download}/commands/**/*.ts : Apply `.cursor/rules/lucky-discord-bot.mdc` rules for Discord bot commands and player implementation
Applied to files:
packages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : Do not duplicate queue or player state outside Discord Player; use shared services from `lucky/shared` for persistent data like track history and session information
Applied to files:
packages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.ts
📚 Learning: 2026-03-09T20:21:08.612Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-project.mdc:0-0
Timestamp: 2026-03-09T20:21:08.612Z
Learning: Applies to {packages/*/tests/**/*.test.{js,ts},tests/**/*.test.{js,ts}} : Add or adjust unit and integration tests when changing behavior; follow existing patterns in `packages/*/tests` and root `tests/` directories
Applied to files:
packages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/lastfm/lastFmApi.spec.ts
📚 Learning: 2026-03-09T20:21:38.098Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-backend.mdc:0-0
Timestamp: 2026-03-09T20:21:38.098Z
Learning: Applies to packages/backend/tests/**/*.ts : Follow existing patterns for unit and integration tests in `packages/backend/tests/`
Applied to files:
packages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/lastfm/lastFmApi.spec.ts
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to **/*.{spec,test}.{ts,tsx,js,jsx} : Test behavior, not implementation details
Applied to files:
packages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/lastfm/lastFmApi.spec.ts
📚 Learning: 2026-03-15T21:57:49.951Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-15T21:57:49.951Z
Learning: When working on music queue/player features (play/queue/skip/volume, player lifecycle), use the `music-queue-player` skill
Applied to files:
packages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/queueManipulation.ts
📚 Learning: 2026-03-15T21:57:49.951Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-15T21:57:49.951Z
Learning: When working on unit tests, Jest ESM mocks, or fixing disabled tests, use the `testing-lucky` skill
Applied to files:
packages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/lastfm/lastFmApi.spec.ts
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to tests/**/*.test.{ts,tsx,js,jsx} : Add integration tests where appropriate
Applied to files:
packages/bot/src/utils/music/autoplay/lastFmSeeds.spec.ts
📚 Learning: 2026-03-09T20:20:56.356Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-frontend.mdc:0-0
Timestamp: 2026-03-09T20:20:56.356Z
Learning: Applies to packages/frontend/tests/**/*.{ts,tsx,js} : Write tests in `packages/frontend/tests/` using existing test patterns (e.g., Playwright for e2e if configured)
Applied to files:
packages/bot/src/utils/music/autoplay/lastFmSeeds.spec.ts
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to **/*.{spec,test}.{ts,tsx,js,jsx} : Use Jest for unit and integration tests
Applied to files:
packages/bot/src/lastfm/lastFmApi.spec.ts
🪛 GitHub Check: SonarCloud Code Analysis
packages/bot/src/utils/music/queueManipulation.ts
[failure] 432-432: Refactor this function to reduce its Cognitive Complexity from 17 to the 15 allowed.
[warning] 432-432: Async function 'collectLastFmCandidates' has too many parameters (9). Maximum allowed is 7.
packages/bot/src/lastfm/lastFmApi.ts
[warning] 95-95: Prefer String#replaceAll() over String#replace().
[warning] 88-88: Unnecessary escape character: [.
[warning] 87-87: Simplify this regular expression to reduce its complexity from 35 to the 20 allowed.
[warning] 95-95: Prefer String#replaceAll() over String#replace().
[warning] 135-135: Prefer Number.parseInt over parseInt.
🔇 Additional comments (11)
packages/bot/src/handlers/player/trackNowPlaying.ts (1)
155-156: LGTM! Correct fix for the Last.fm duration NaN issue.The change correctly derives
durationSecfromtrack.durationMS / 1000instead of the formatted stringtrack.duration. The guarddurationMS > 0appropriately handles edge cases, passingundefinedwhen duration is unavailable. This aligns with theRawTrackinterface inmappers.tswheredurationMSis typed asnumber.Also applies to: 182-185
packages/bot/src/utils/music/queueManipulation.spec.ts (1)
45-51: LGTM! Mock setup follows existing patterns.The
getLastFmSeedTracksmock is correctly added and reset to[]inbeforeEach, ensuring existingreplenishQueuetests remain deterministic. The indirect mock pattern matches other mocks in the file.Also applies to: 103-103
packages/bot/src/lastfm/index.ts (1)
1-10: LGTM! Clean re-export of new Last.fm API surface.The barrel file correctly re-exports the new runtime functions and type-only exports for
LastFmTopTrackandLastFmPeriod.CHANGELOG.md (1)
14-15: LGTM! Changelog entries accurately document the PR changes.The entries correctly categorize:
- Added: Last.fm top tracks seeding and normalizers
- Fixed: Duration NaN issue with proper root cause explanation
Also applies to: 20-20
packages/bot/src/utils/music/autoplay/lastFmSeeds.ts (1)
15-51: LGTM! Well-structured seed tracks provider with appropriate error handling.The function correctly:
- Returns cached results when valid (TTL check)
- Gracefully handles missing Last.fm links (returns
[])- Logs errors via
errorLogand returns[]on failure- Maps the response to the minimal
{ artist, title }shape needed downstreampackages/bot/src/lastfm/lastFmApi.spec.ts (2)
77-127: LGTM! Comprehensive normalization tests.The tests cover the key normalization scenarios:
- Artist:
- Topicsuffix stripping, comma/slash separator handling- Title:
(Official Video),[Official Music Video],feat./ft.clause removal
129-179: LGTM! Thorough getTopTracks error handling coverage.Tests validate all failure modes: network errors, missing API key, and non-OK responses all correctly return
[]. The success case verifies proper mapping includingplaycountstring →playCountnumber conversion.packages/bot/src/utils/music/autoplay/lastFmSeeds.spec.ts (1)
38-91: LGTM! Solid test coverage for seed tracks behavior.The tests verify:
- Correct mapping when user has a linked Last.fm account
- Returns
[]when no link exists or username is null- Cache reuse within TTL (single
getTopTrackscall)- Graceful error handling
packages/bot/src/utils/music/queueManipulation.ts (2)
238-250: LGTM! Clean integration of Last.fm seeding into replenishQueue.The conditional
if (requestedBy?.id)correctly gates the Last.fm candidate collection to users who have an identity, and the candidates are merged before diversity selection.
477-491: LGTM! Simple helper for Last.fm query search.
searchLastFmQuerycleanly wraps the player search with appropriate error handling (silent catch returning[]).packages/bot/src/lastfm/lastFmApi.ts (1)
149-151: LGTM!Good integration of the normalization functions into both
updateNowPlayingandscrobble. The consistent application ensures cleaner track metadata is sent to Last.fm.Also applies to: 167-170
| try { | ||
| const res = await fetch(`${API_BASE}?${params.toString()}`) | ||
| if (!res.ok) return [] | ||
| const data = (await res.json()) as { | ||
| toptracks?: { | ||
| track?: Array<{ | ||
| name: string | ||
| artist: { name: string } | ||
| playcount: string | ||
| }> | ||
| } | ||
| } | ||
| return (data.toptracks?.track ?? []).map((t) => ({ | ||
| artist: t.artist.name, | ||
| title: t.name, | ||
| playCount: parseInt(t.playcount, 10) || 0, | ||
| })) | ||
| } catch { | ||
| return [] | ||
| } |
There was a problem hiding this comment.
Add timeout to prevent dangling requests.
The fetch call lacks a timeout, which could leave requests hanging indefinitely if the Last.fm API is unresponsive. Per coding guidelines, API calls should "set timeouts and handle aborts/cancellations."
🛡️ Proposed fix with AbortSignal timeout
try {
- const res = await fetch(`${API_BASE}?${params.toString()}`)
+ const res = await fetch(`${API_BASE}?${params.toString()}`, {
+ signal: AbortSignal.timeout(10000),
+ })
if (!res.ok) return []As per coding guidelines: "Set timeouts and handle aborts/cancellations; avoid dangling requests in API/network code."
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 135-135: Prefer Number.parseInt over parseInt.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/lastfm/lastFmApi.ts` around lines 120 - 139, The fetch in
the Last.fm call (the block using API_BASE and params in lastFmApi.ts that
returns mapped toptracks) needs an AbortController timeout to avoid dangling
requests: create an AbortController, start a timer (e.g., setTimeout) to call
controller.abort() after a configurable timeout, pass controller.signal to
fetch(`${API_BASE}?${params.toString()}`), and clear the timer after fetch
completes; ensure the catch handles aborts (returning [] as now) and any timer
is cleaned up to avoid leaks.
- Fix scrobble/nowPlaying duration: use track.durationMS (number in ms) instead of track.duration (formatted string) in discord-player 7 - Add artist/title normalizers to strip YouTube noise from scrobbles - Add getTopTracks via user.getTopTracks API (3month, 20 tracks) - Add getLastFmSeedTracks with 1h in-memory cache per Discord user - Integrate Last.fm seeds into replenishQueue with +0.1 score boost - 27 new tests (591 total passing)
a61b50c to
3898e48
Compare
|
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/lastfm/lastFmApi.ts`:
- Around line 120-138: The code currently turns any fetch failure or non-OK
response into an empty array; change it so transport/response failures throw
while only a genuinely empty payload returns []. Specifically, in the fetch
block around API_BASE and params.toString(), replace the early `if (!res.ok)
return []` with code that throws an Error (include status/text), and in the
catch block rethrow the caught error instead of returning []; only after
successfully parsing JSON inspect data.toptracks?.track and return [] if that
array is absent/empty; keep the mapping that uses parseInt(t.playcount, 10) || 0
and the fields artist/name/title unchanged so callers of this Last.fm helper
(the fetch to API_BASE) receive errors for transient failures but still get []
for a real empty result.
- Around line 84-95: Current normalizers rewrite canonical metadata: stop
collapsing valid artist names and altering titles by removing meaningful
parentheticals and splitting on commas/slashes. In normalizeLastFmArtist and
constants (ARTIST_SEPARATORS) revert the split-on-separators behavior so the
function only strips the TOPIC_SUFFIX and trims the raw string (do not split on
commas or slashes). In normalizeLastFmTitle and constants (TITLE_NOISE_PARENS,
FEAT_CLAUSE) stop stripping generic parentheticals like "(Live)" or removing
feat clauses from display titles; only remove the explicit "- topic" suffix and
optionally remove very specific noise tokens such as "official video"/"official
audio"/"lyrics"/"hd"/"4k" when they appear as standalone trailing tags,
preserving other parenthetical content and "feat." text.
In `@packages/bot/src/utils/music/queueManipulation.ts`:
- Around line 443-448: The code incorrectly limits the Last.fm sampling pool to
10 by slicing lastFmTracks into pool = lastFmTracks.slice(0, 10), which
contradicts the cached/fetched 20-track contract; change the pool construction
to use the full cached list (e.g., pool = lastFmTracks) so sampling covers all
available tracks, keep the rest of the seeding logic (seeds, LASTFM_SEED_COUNT)
unchanged or, if the 10-item cap is intentional, update docs/tests to reflect
it.
🪄 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: ca22d442-cdb4-4dd9-bfb3-fd9e8a16dd5b
📒 Files selected for processing (9)
CHANGELOG.mdpackages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/lastfm/index.tspackages/bot/src/lastfm/lastFmApi.spec.tspackages/bot/src/lastfm/lastFmApi.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.spec.tspackages/bot/src/utils/music/autoplay/lastFmSeeds.tspackages/bot/src/utils/music/queueManipulation.spec.tspackages/bot/src/utils/music/queueManipulation.ts
✅ Files skipped from review due to trivial changes (4)
- packages/bot/src/lastfm/index.ts
- packages/bot/src/utils/music/queueManipulation.spec.ts
- packages/bot/src/utils/music/autoplay/lastFmSeeds.spec.ts
- packages/bot/src/handlers/player/trackNowPlaying.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/bot/src/lastfm/lastFmApi.spec.ts
- packages/bot/src/utils/music/autoplay/lastFmSeeds.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: SonarCloud Scan
- GitHub Check: Quality Gates
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,jsx,ts,tsx,vue,html}
📄 CodeRabbit inference engine (.cursor/rules/accessibility-openness.mdc)
Provide accessible UI components using semantic HTML and ARIA attributes where necessary
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/lastfm/lastFmApi.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dependency-injection.mdc)
**/*.{ts,tsx,js,jsx}: Prefer constructor injection for classes that require dependencies
Avoid global mutable singletons unless necessary
Use explicit interfaces for external dependencies to make testing easier
**/*.{ts,tsx,js,jsx}: Include required references in PRs/code for non-trivial logic: TypeScript (official docs), MDN (JavaScript reference), and official docs for any runtime/framework/libraries used (e.g., Node.js, React) as applicable.
Before assuming behavior of an API, include the doc link and a ≤25-word quote when the change relies on it.
**/*.{ts,tsx,js,jsx}: Prefer named exports for clear usage and easier refactors in TypeScript/JavaScript
Keep import order consistent: external first, then internal modules
Remove dead code and unused imports
**/*.{ts,tsx,js,jsx}: Use PascalCase naming convention for React/UI components
Use camelCase naming convention for variables and functions
Use UPPER_SNAKE_CASE naming convention for constants
Maintain consistent import grouping and ordering within the project, keeping third-party imports separate from local imports
For external data sources (HTTP, database), always validate and sanitize input using type guards or schema validators
**/*.{ts,tsx,js,jsx}: Use Prettier with no semicolons, single quotes, 4-space indent, 80 character width
Files must not exceed 250 lines and this is enforcedImplement TypeScript typecheck and linter in CI quality checks
**/*.{ts,tsx,js,jsx}: Use TypeScript for enhanced type safety
Implement error handling and error logging
Avoid commenting code unless extremely necessary - code should explain itself with descriptive names
Leave NO todos, placeholders or missing pieces in the code
Variables and functions must use camelCase
Constants must use UPPER_SNAKE_CASE
Use arrow functions for methods and computed properties
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Maintain consistent import grouping/order: external imports first, then...
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/lastfm/lastFmApi.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{js,jsx,ts,tsx}: Never throw strings. ThrowError(or typed subclasses) with descriptive messages
Include causal error ascausewhen available for better debugging
Define clear, stable error codes (e.g.,ERR_AUTH_EXPIRED,ERR_NETWORK_TIMEOUT)
Provide optional metadata (e.g.,details,retryable,status,correlationId) in error objects
Use domain error classes per area (e.g.,AuthenticationError,ValidationError,NetworkError)
Log errors with structure (message, code, stack, cause, correlationId, user context where appropriate)
MarkretryablevsnonRetryableerrors where helpful for operations
Set timeouts and handle aborts/cancellations; avoid dangling requests in API/network code
Implement backoff for transient failures; avoid infinite retries
Map HTTP status → domain errors; 4xx vs 5xx behave differently (e.g., retry for 5xx/network)
**/*.{js,jsx,ts,tsx}: Use functional components with hooks in React/React Native. Avoid class components.
Keep components focused on a single responsibility; extract complex logic into custom hooks.
Keep state local when possible. Use Context/Zustand/Redux only when necessary for state management.
If props or state traverse more than 3 levels, consider using context or a feature-scoped store instead of prop drilling.
Use performance optimization techniques:React.memo,useMemo,useCallback,Suspense(web), and virtualization for long lists; avoid unnecessary re-renders.
Web accessibility: use semantic HTML, labels, focus management, keyboard navigation, andaria-*attributes as needed.
React Native accessibility: use accessibility props (accessible,accessibilityLabel), proper roles and labels.
Identify and extract repetitive UI components proactively tocomponents/with clear props and minimal coupling.
Web styles: prefer co-located styles or design system tokens; avoid global style leakage.
React Native styles: preferStyleSheet.create, design tokens, and theme providers; avoid in...
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/lastfm/lastFmApi.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
Introduce interfaces at module boundaries to enable testing and substitutions
**/*.{ts,tsx}: Avoid usinganytype in TypeScript. If unavoidable, useunknownwith type guards and justify with a code comment
Preferinterfacefor defining public object shapes in TypeScript, usetypefor unions and utility types
Use TypeScript utility types such asPartial,Pick,Omit,Readonly, andRecordwhen appropriate
UseI{Name}naming convention for interfaces in TypeScript
UseT{Name}naming convention for type aliases and utility types in TypeScript
**/*.{ts,tsx}: Functions must be less than 50 lines with cyclomatic complexity less than 10
Do not useanytypes - ESLint enforces this at error level
**/*.{ts,tsx}: Prefer types over interfaces for most cases
Don't ever useany- type safety always
Avoid enums; use const objects instead
For complex types, create a separate file to declare them and import them
Avoid usinganytype; if unavoidable, useunknownwith type guards and justify with code comment
Preferinterfacefor public API shapes; usetypefor unions and utility types
Use TypeScript utility types (Partial, Pick, Omit, Readonly, Record)
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/lastfm/lastFmApi.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/documentation.mdc)
**/*.{js,ts,tsx,jsx}: Minimize comments in code; explain the 'why' when non-obvious, let code express the 'what' through clear naming
Document trade-offs briefly when deviating from ideal patterns
**/*.{js,ts,tsx,jsx}: Store secrets, ports, and hosts in environment variables (.env,.env.example) and never hardcode them
Avoid redundant or decorative AI comments; code should be self-explanatory and only commented when logic is non-obvious; prefer refactoring over lengthy commentsNever hardcode secrets, IPs, or ports; use
.envanddocs/for required configuration variables
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/lastfm/lastFmApi.ts
packages/bot/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)
packages/bot/**/*.{ts,tsx}: UseuseMainPlayer()fromdiscord-playerto access the player instance; do not instantiate player directly
Do not duplicate queue or player state outside Discord Player; use shared services from@lucky/sharedfor persistent data like track history and session information
UseerrorLoganddebugLogfrom@lucky/shared/utilsfor logging throughout the bot package
Use embed and reply utilities from@lucky/sharedfor consistent message formatting and error sanitization across the bot
Use services from@lucky/shared(DatabaseService, Redis client) for database and cache access; do not instantiate Prisma or Redis directly in the bot package
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/lastfm/lastFmApi.ts
packages/bot/**
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
The
botpackage depends onsharedand contains Discord bot commands and player handlers using Discord.js and Discord Player
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/lastfm/lastFmApi.ts
**/*.{js,mjs,ts,mts}
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
Use Node.js version ≥22 with ESM (ECMAScript modules) only; no CommonJS
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/lastfm/lastFmApi.ts
packages/bot/src/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)
Use
@lucky/sharedfor database, Redis, logging, and embed utilities instead of implementing them locally
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/lastfm/lastFmApi.ts
{CHANGELOG.md,README.md}
📄 CodeRabbit inference engine (.cursor/rules/agent-rules.mdc)
ALWAYS update CHANGELOG.md and README.md as changes are made.
Files:
CHANGELOG.md
CHANGELOG.md
📄 CodeRabbit inference engine (.cursor/rules/templates-examples.mdc)
CHANGELOG.md must be updated with all changes in pull requests
Always update CHANGELOG.md with all code changes
Update CHANGELOG.md with all changes, include breaking changes documentation, and reference issues and PRs
Files:
CHANGELOG.md
{CHANGELOG.md,docs/**}
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
Update
CHANGELOG.mdand relevantdocs/files when behavior or setup changes
Files:
CHANGELOG.md
🧠 Learnings (6)
📚 Learning: 2026-03-09T20:21:52.065Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/music/commands/**/*.ts : Use `.cursor/skills/music-queue-player/SKILL.md` for play, queue, skip, volume commands and player lifecycle management
Applied to files:
packages/bot/src/utils/music/queueManipulation.ts
📚 Learning: 2026-03-09T20:21:52.065Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/music/**/*.ts : Use existing voice/queue/guild validators before manipulating player or queue state
Applied to files:
packages/bot/src/utils/music/queueManipulation.ts
📚 Learning: 2026-03-15T21:57:49.951Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-15T21:57:49.951Z
Learning: When working on music queue/player features (play/queue/skip/volume, player lifecycle), use the `music-queue-player` skill
Applied to files:
packages/bot/src/utils/music/queueManipulation.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : Do not duplicate queue or player state outside Discord Player; use shared services from `lucky/shared` for persistent data like track history and session information
Applied to files:
packages/bot/src/utils/music/queueManipulation.ts
📚 Learning: 2026-03-09T20:21:52.065Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/{general,music,download}/commands/**/*.ts : Apply `.cursor/rules/lucky-discord-bot.mdc` rules for Discord bot commands and player implementation
Applied to files:
packages/bot/src/utils/music/queueManipulation.ts
📚 Learning: 2026-03-09T20:20:32.245Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/documentation.mdc:0-0
Timestamp: 2026-03-09T20:20:32.245Z
Learning: Applies to README.md : Update README.md if behavior changed
Applied to files:
CHANGELOG.md
🪛 GitHub Check: SonarCloud Code Analysis
packages/bot/src/utils/music/queueManipulation.ts
[failure] 432-432: Refactor this function to reduce its Cognitive Complexity from 17 to the 15 allowed.
[warning] 432-432: Async function 'collectLastFmCandidates' has too many parameters (9). Maximum allowed is 7.
packages/bot/src/lastfm/lastFmApi.ts
[warning] 95-95: Prefer String#replaceAll() over String#replace().
[warning] 87-87: Simplify this regular expression to reduce its complexity from 35 to the 20 allowed.
[warning] 88-88: Unnecessary escape character: [.
[warning] 95-95: Prefer String#replaceAll() over String#replace().
[warning] 135-135: Prefer Number.parseInt over parseInt.
| const TOPIC_SUFFIX = / - topic$/i | ||
| const ARTIST_SEPARATORS = /\s*[,/]\s*/ | ||
| const TITLE_NOISE_PARENS = | ||
| /\s*[([](official\s*(music\s*)?video|official\s*audio|audio|lyric\s*video|lyrics?|live|hd|4k|ft\.?[^)\]]*|feat\.?[^)\]]*)[)\]]/gi | ||
| const FEAT_CLAUSE = /\s*[\[(]?feat\.?\s+[^\])[]+[\])]?/gi | ||
|
|
||
| export function normalizeLastFmArtist(raw: string): string { | ||
| return raw.replace(TOPIC_SUFFIX, '').split(ARTIST_SEPARATORS)[0].trim() | ||
| } | ||
|
|
||
| export function normalizeLastFmTitle(raw: string): string { | ||
| return raw.replace(TITLE_NOISE_PARENS, '').replace(FEAT_CLAUSE, '').trim() |
There was a problem hiding this comment.
The new normalizers rewrite canonical artist/title metadata.
Splitting on raw , / / turns valid artists like Tyler, The Creator and AC/DC into bad scrobbles, and stripping (Live) collapses a distinct release into the studio track. These helpers sit on every now-playing/scrobble path, so the bad metadata also feeds back into the user’s Last.fm top tracks and autoplay seeds.
Proposed fix
-const ARTIST_SEPARATORS = /\s*[,/]\s*/
const TITLE_NOISE_PARENS =
- /\s*[([](official\s*(music\s*)?video|official\s*audio|audio|lyric\s*video|lyrics?|live|hd|4k|ft\.?[^)\]]*|feat\.?[^)\]]*)[)\]]/gi
+ /\s*[([](official\s*(music\s*)?video|official\s*audio|audio|lyric\s*video|lyrics?|hd|4k|ft\.?[^)\]]*|feat\.?[^)\]]*)[)\]]/gi
export function normalizeLastFmArtist(raw: string): string {
- return raw.replace(TOPIC_SUFFIX, '').split(ARTIST_SEPARATORS)[0].trim()
+ return raw.replace(TOPIC_SUFFIX, '').trim()
}If you still want collaborator collapsing, it needs to happen from structured provider metadata upstream rather than by splitting raw display strings.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 95-95: Prefer String#replaceAll() over String#replace().
[warning] 87-87: Simplify this regular expression to reduce its complexity from 35 to the 20 allowed.
[warning] 88-88: Unnecessary escape character: [.
[warning] 95-95: Prefer String#replaceAll() over String#replace().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/lastfm/lastFmApi.ts` around lines 84 - 95, Current
normalizers rewrite canonical metadata: stop collapsing valid artist names and
altering titles by removing meaningful parentheticals and splitting on
commas/slashes. In normalizeLastFmArtist and constants (ARTIST_SEPARATORS)
revert the split-on-separators behavior so the function only strips the
TOPIC_SUFFIX and trims the raw string (do not split on commas or slashes). In
normalizeLastFmTitle and constants (TITLE_NOISE_PARENS, FEAT_CLAUSE) stop
stripping generic parentheticals like "(Live)" or removing feat clauses from
display titles; only remove the explicit "- topic" suffix and optionally remove
very specific noise tokens such as "official video"/"official
audio"/"lyrics"/"hd"/"4k" when they appear as standalone trailing tags,
preserving other parenthetical content and "feat." text.
| try { | ||
| const res = await fetch(`${API_BASE}?${params.toString()}`) | ||
| if (!res.ok) return [] | ||
| const data = (await res.json()) as { | ||
| toptracks?: { | ||
| track?: Array<{ | ||
| name: string | ||
| artist: { name: string } | ||
| playcount: string | ||
| }> | ||
| } | ||
| } | ||
| return (data.toptracks?.track ?? []).map((t) => ({ | ||
| artist: t.artist.name, | ||
| title: t.name, | ||
| playCount: parseInt(t.playcount, 10) || 0, | ||
| })) | ||
| } catch { | ||
| return [] |
There was a problem hiding this comment.
Don’t cache transient Last.fm failures as “no top tracks.”
Right now every non-OK/JSON/network failure becomes []. packages/bot/src/utils/music/autoplay/lastFmSeeds.ts then caches that array for 1 hour, so one Last.fm blip disables taste seeding until the TTL expires. Throw on transport/response failures and only return [] for a real empty payload.
Proposed fix
try {
const res = await fetch(`${API_BASE}?${params.toString()}`)
- if (!res.ok) return []
+ if (!res.ok) {
+ throw new Error(
+ `Last.fm user.getTopTracks failed with ${res.status}`,
+ )
+ }
const data = (await res.json()) as {
toptracks?: {
track?: Array<{
@@
return (data.toptracks?.track ?? []).map((t) => ({
artist: t.artist.name,
title: t.name,
playCount: parseInt(t.playcount, 10) || 0,
}))
- } catch {
- return []
+ } catch (error) {
+ throw new Error('Last.fm user.getTopTracks failed', { cause: error })
}
}🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 135-135: Prefer Number.parseInt over parseInt.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/lastfm/lastFmApi.ts` around lines 120 - 138, The code
currently turns any fetch failure or non-OK response into an empty array; change
it so transport/response failures throw while only a genuinely empty payload
returns []. Specifically, in the fetch block around API_BASE and
params.toString(), replace the early `if (!res.ok) return []` with code that
throws an Error (include status/text), and in the catch block rethrow the caught
error instead of returning []; only after successfully parsing JSON inspect
data.toptracks?.track and return [] if that array is absent/empty; keep the
mapping that uses parseInt(t.playcount, 10) || 0 and the fields
artist/name/title unchanged so callers of this Last.fm helper (the fetch to
API_BASE) receive errors for transient failures but still get [] for a real
empty result.
| const lastFmTracks = await getLastFmSeedTracks(requestedBy.id) | ||
| if (lastFmTracks.length === 0) return | ||
|
|
||
| const pool = lastFmTracks.slice(0, 10) | ||
| const seeds: typeof lastFmTracks = [] | ||
| while (seeds.length < LASTFM_SEED_COUNT && pool.length > 0) { |
There was a problem hiding this comment.
This caps the Last.fm sampling pool at 10 tracks.
Line 446 limits sampling to 10 even though the feature fetches/caches 20, which narrows taste coverage and diverges from the changelog/PR contract. If the top-10 cap is intentional, the docs/tests should say so; otherwise sample from the full cached list.
Proposed fix
- const pool = lastFmTracks.slice(0, 10)
+ const pool = [...lastFmTracks]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/utils/music/queueManipulation.ts` around lines 443 - 448,
The code incorrectly limits the Last.fm sampling pool to 10 by slicing
lastFmTracks into pool = lastFmTracks.slice(0, 10), which contradicts the
cached/fetched 20-track contract; change the pool construction to use the full
cached list (e.g., pool = lastFmTracks) so sampling covers all available tracks,
keep the rest of the seeding logic (seeds, LASTFM_SEED_COUNT) unchanged or, if
the 10-item cap is intentional, update docs/tests to reflect it.
* feat(bot): last.fm duration fix, normalizers, and top tracks seeds - Fix scrobble/nowPlaying duration: use track.durationMS (number in ms) instead of track.duration (formatted string) in discord-player 7 - Add artist/title normalizers to strip YouTube noise from scrobbles - Add getTopTracks via user.getTopTracks API (3month, 20 tracks) - Add getLastFmSeedTracks with 1h in-memory cache per Discord user - Integrate Last.fm seeds into replenishQueue with +0.1 score boost - 27 new tests (591 total passing) * docs: add changelog entries for PR #382 last.fm improvements * test(bot): add coverage for collectLastFmCandidates path
This branch was successfully deployed
1 participant
Summary
track.durationin discord-player 7 is a formatted string like"3:45", not a number. Now usestrack.durationMS / 1000correctly.- Topicsuffix, split multi-artist (Artist A, Artist B→Artist A), remove noise from titles ((Official Video),(feat. X),[Official Music Video], etc.)getTopTracksfetches user's most-played tracks from Last.fm (user.getTopTracks, 3-month window, 20 tracks)getLastFmSeedTracks(1h TTL cache per Discord user) feeds intoreplenishQueueviacollectLastFmCandidates— adds Last.fm taste-aware candidates with+0.1score boost andlast.fm tastereason taglastFmApi.specand newlastFmSeeds.spec(591 total passing)Test plan
- Topicno longer appear in Last.fm historylast.fm tastereason on eligible tracks when user has a linked Last.fm account[]early)Summary by CodeRabbit
New Features
Bug Fixes
Improvements