Repository navigation
refactor: split autoplay.ts into queueHandlers, settingsHandlers, artistHandlers - #814
Merged
Merged
Conversation
…istHandlers - Extract queue operations (skip, clear, status) into queueHandlers.ts - Extract settings operations (mode, genre, analytics) into settingsHandlers.ts - Extract artist preference operations (prefer, block, list, remove) into artistHandlers.ts - Refactor main autoplay.ts to thin router pattern with dispatcher - Maintain all existing functionality and type safety - Reduce main file from 1074 to ~250 lines
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (46)
📝 WalkthroughWalkthroughRefactors the autoplay command into three new handler modules (queueHandlers, settingsHandlers, artistHandlers), migrates many command replies to a centralized interactionReply helper and MessageFlags usage, adds tests and mocks, updates backend Jest test config and session-file-store mock, and updates README branding and a package.json override. ChangesAutoplay Modularization
Interaction Reply Migration
Backend Tests & Jest Config
Documentation & Packaging
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as "Discord User"
participant Interaction as "ChatInputCommandInteraction"
participant Command as "Autoplay Command"
participant Queue as "GuildQueue"
participant Service as "replenishQueue / lastFmLinkService"
Interaction->>Command: invoke /autoplay status /skip /clear /artist ...
Command->>Queue: resolve guild queue & metadata
alt artist subcommand
Command->>Service: recommendationFeedbackService (set/remove/get)
Service-->>Command: confirmation / summary
else queue operation
Command->>Queue: inspect tracks (isAutoplayTrack)
Command->>Queue: removeTrack(index)*
Command->>Service: replenishQueue(queue)
Service-->>Command: replenishment result
end
Command->>Interaction: interactionReply(content: embed/status)
(Note: rectangles use default styles; interactions emphasize sequential flow among User, Command, Queue, and Services.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
LucasSantana-Dev
enabled auto-merge (squash)
May 6, 2026 01:10
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
packages/bot/src/functions/music/commands/autoplay.ts (2)
222-224: 💤 Low valueDead code:
case 'artist'is unreachable.
artistis declared as aaddSubcommandGroup(Line 112), not a top-level subcommand, sointeraction.options.getSubcommand(false)will return one ofprefer|block|list|removehere — never'artist'. The artist group is already routed at Lines 186-189. Drop this case to avoid misleading future readers.🔧 Proposed fix
case 'mode': await handleAutoplayMode(interaction) break - case 'artist': - await handleAutoplayArtist(interaction) - break default:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bot/src/functions/music/commands/autoplay.ts` around lines 222 - 224, The switch contains an unreachable branch: remove the "case 'artist'" branch (which calls handleAutoplayArtist) because 'artist' is defined as an addSubcommandGroup and is already handled in the artist-group routing (lines calling handleAutoplayArtist at the earlier artist-group branch); rely on interaction.options.getSubcommand(false) returning the actual subcommands (prefer|block|list|remove) and delete the dead case to avoid confusion.
158-174: 💤 Low valueUse
flags: MessageFlags.Ephemeralinstead of deprecatedephemeral: truefordeferReply.The
ephemeralboolean option fordeferReply()is deprecated in discord.js v14.26.4. Replaceinteraction.deferReply({ ephemeral: true })withinteraction.deferReply({ flags: MessageFlags.Ephemeral }). The same pattern is visible in the internalinteractionReplywrapper utility (lines 87–88, 120–121), which correctly usesflagsinstead of the deprecated option. This also affects other command files likedjrole.ts,version.ts,settings.ts, andguildconfig.tswhere directdeferReply()calls use the deprecated syntax.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bot/src/functions/music/commands/autoplay.ts` around lines 158 - 174, Replace the deprecated boolean option in the execute function's deferReply call: in the execute handler (execute: async ({ client, interaction }: CommandExecuteParams) => { ... }) change interaction.deferReply({ ephemeral: true }) to use the MessageFlags API (interaction.deferReply({ flags: MessageFlags.Ephemeral })) so it matches the interactionReply wrapper pattern; update the import if needed to bring in MessageFlags from discord.js and apply the same replacement to other command files that call deferReply directly (e.g., djrole.ts, version.ts, settings.ts, guildconfig.ts) to remove the deprecated usage.packages/bot/src/functions/music/commands/autoplay/queueHandlers.ts (1)
15-17: ⚡ Quick winReplace
anywith proper types from discord-player.The
isAutoplayTrackfunction usesanytwice for the parameter and metadata cast. ImportTrackfromdiscord-playerand type the metadata properly to get compile-time guarantees.🔧 Proposed fix
-import type { GuildQueue } from 'discord-player' +import type { GuildQueue, Track } from 'discord-player' import type { QueueMetadata } from '../../../../types/QueueMetadata' -function isAutoplayTrack(track: any): boolean { - return (track?.metadata as any)?.isAutoplay === true +function isAutoplayTrack(track: Track | null | undefined): boolean { + return (track?.metadata as { isAutoplay?: boolean } | undefined)?.isAutoplay === true }Alternatively, if you prefer using the existing
TrackMetadatatype from@lucky/shared/types:+import type { TrackMetadata } from '@lucky/shared/types' -import type { GuildQueue } from 'discord-player' +import type { GuildQueue, Track } from 'discord-player' import type { QueueMetadata } from '../../../../types/QueueMetadata' -function isAutoplayTrack(track: any): boolean { - return (track?.metadata as any)?.isAutoplay === true +function isAutoplayTrack(track: Track | null | undefined): boolean { + return (track?.metadata as TrackMetadata | undefined)?.isAutoplay === true }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bot/src/functions/music/commands/autoplay/queueHandlers.ts` around lines 15 - 17, isAutoplayTrack currently uses `any` for the parameter and metadata; import `Track` from `discord-player` (or use the existing `TrackMetadata` from `@lucky/shared/types`) and change the function signature to accept `track: Track` (or `Track | null | undefined` as needed) and type the metadata access so `(track.metadata as TrackMetadata).isAutoplay` is type-checked; update the import statements and adjust the function body to use the proper types (preserve optional chaining) to remove `any` usages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bot/src/functions/music/commands/autoplay/artistHandlers.ts`:
- Around line 159-195: The list output is inconsistent because
getArtistFeedbackSummary(userId) reads global per-user feedback while
setArtistFeedback(...) and removeArtistFeedback(...) accept but ignore guildId
(they store to music:artist_feedback:${userId}); fix by making behavior
consistent: either remove guildId from setArtistFeedback/removeArtistFeedback
signatures and docs to keep global per-user storage, or change persistence and
getArtistFeedbackSummary to include a guildId parameter and store/read under a
per-guild key (e.g., include guildId in the Redis key) so setArtistFeedback,
removeArtistFeedback, and getArtistFeedbackSummary all use the same (user,guild)
scope; update the functions setArtistFeedback, removeArtistFeedback, and
getArtistFeedbackSummary and any callers (including the list subcommand) to the
chosen contract.
- Around line 1-6: The import statements at the top of artistHandlers.ts are
using incorrect module paths; update them to match the project structure the
same way queueHandlers.ts does: replace recommendationFeedbackService import
with "@/services/musicRecommendation/feedbackService", replace createEmbed and
createErrorEmbed import with "@/utils/general/embeds", replace EMBED_COLORS and
EMOJIS import with "@/config/constants", replace errorLog import with
"@lucky/shared/utils" (the shared export), and change the local interactionReply
import to the relative path "../../../../utils/general/interactionReply"; keep
the same named imports (recommendationFeedbackService, createEmbed,
createErrorEmbed, EMBED_COLORS, EMOJIS, errorLog, interactionReply) so all
usages in this file continue to work.
In `@packages/bot/src/functions/music/commands/autoplay/queueHandlers.ts`:
- Around line 23-37: The check on queue.tracks.size currently uses
createErrorEmbed('Queue Empty', 'No autoplay tracks to skip.') which mislabels
an actually-empty queue; change the message in the branch that checks
queue.tracks.size === 0 to clearly indicate the queue is empty (e.g.,
createErrorEmbed('Queue Empty', 'The queue is empty.') or similar) so it differs
from the later !skipped branch that reports "no autoplay tracks among existing
tracks"; update the interactionReply payload in that branch only (the code paths
involving interactionReply, createErrorEmbed, and the queue.tracks.size === 0
condition).
In `@packages/bot/src/functions/music/commands/autoplay/settingsHandlers.ts`:
- Around line 110-117: handleAutoplayGenreAdd (and the corresponding
handleAutoplayGenreRemove) silently return when guildId or tag is missing,
leaving the deferred interaction unresolved; change these guards to send the
same error reply used by handleAutoplayMode/handleAutoplayAnalytics (e.g., call
the shared interaction reply helper with a "Guild Not Found" or similar error)
before returning so the user isn't left on a spinner — update the early-return
paths in handleAutoplayGenreAdd and handleAutoplayGenreRemove to call the
standard interaction reply and then return.
- Around line 418-432: handleAutoplayGenre currently returns without responding
when subcommand is null or unknown (leaving the prior deferReply unresolved);
update handleAutoplayGenre to include a default branch that sends a user-facing
error response (e.g., an embed) and resolves the deferred interaction—use
interaction.editReply (or interaction.followUp if preferred) to send a clear
error message when subcommand is not one of 'add'|'remove'|'list'|'clear';
ensure the default path handles null safely and returns a Promise<void> like the
other branches.
- Around line 1-13: Update the broken import paths in settingsHandlers.ts:
replace the six-level relative imports for utilities and services with the
correct four-level relative imports for local helpers (so imports like
createEmbed, createErrorEmbed, EMBED_COLORS, EMOJIS, errorLog, interactionReply
should come from ../../../../utils/...) and import shared services from the
shared package (replace guildSettingsService and trackHistoryService relative
imports with imports from `@lucky/shared/services`) and any shared utils from
`@lucky/shared/utils`; ensure the imported symbols (createEmbed, createErrorEmbed,
EMBED_COLORS, EMOJIS, guildSettingsService, trackHistoryService, errorLog,
interactionReply) match their original names so existing references in this file
continue to work.
---
Nitpick comments:
In `@packages/bot/src/functions/music/commands/autoplay.ts`:
- Around line 222-224: The switch contains an unreachable branch: remove the
"case 'artist'" branch (which calls handleAutoplayArtist) because 'artist' is
defined as an addSubcommandGroup and is already handled in the artist-group
routing (lines calling handleAutoplayArtist at the earlier artist-group branch);
rely on interaction.options.getSubcommand(false) returning the actual
subcommands (prefer|block|list|remove) and delete the dead case to avoid
confusion.
- Around line 158-174: Replace the deprecated boolean option in the execute
function's deferReply call: in the execute handler (execute: async ({ client,
interaction }: CommandExecuteParams) => { ... }) change interaction.deferReply({
ephemeral: true }) to use the MessageFlags API (interaction.deferReply({ flags:
MessageFlags.Ephemeral })) so it matches the interactionReply wrapper pattern;
update the import if needed to bring in MessageFlags from discord.js and apply
the same replacement to other command files that call deferReply directly (e.g.,
djrole.ts, version.ts, settings.ts, guildconfig.ts) to remove the deprecated
usage.
In `@packages/bot/src/functions/music/commands/autoplay/queueHandlers.ts`:
- Around line 15-17: isAutoplayTrack currently uses `any` for the parameter and
metadata; import `Track` from `discord-player` (or use the existing
`TrackMetadata` from `@lucky/shared/types`) and change the function signature to
accept `track: Track` (or `Track | null | undefined` as needed) and type the
metadata access so `(track.metadata as TrackMetadata).isAutoplay` is
type-checked; update the import statements and adjust the function body to use
the proper types (preserve optional chaining) to remove `any` usages.
🪄 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: 55e6424c-2b72-439e-9b19-1c911e4b2094
📒 Files selected for processing (5)
README.mdpackages/bot/src/functions/music/commands/autoplay.tspackages/bot/src/functions/music/commands/autoplay/artistHandlers.tspackages/bot/src/functions/music/commands/autoplay/queueHandlers.tspackages/bot/src/functions/music/commands/autoplay/settingsHandlers.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). (1)
- GitHub Check: SonarCloud Scan
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use theisPrisma*Error()helper functions to check for specific Prisma error types (e.g.,isPrismaForeignKeyError,isPrismaUniqueConstraintError) instead of manually checking error codes
Use Prisma's$transaction()method to ensure database operations are atomic and avoid partial updates when multiple related tables are modified
Always useselectorincludein Prisma queries to explicitly specify which fields to return, avoiding unnecessary data transfer
For Redis operations, use connection pooling and implement exponential backoff retry logic for transient failures
Always uselogAndRethrow()orlogAndSwallow()utilities when handling errors to ensure errors are logged with context before propagating or suppressing
Files:
packages/bot/src/functions/music/commands/autoplay/settingsHandlers.tspackages/bot/src/functions/music/commands/autoplay/queueHandlers.tspackages/bot/src/functions/music/commands/autoplay.tspackages/bot/src/functions/music/commands/autoplay/artistHandlers.ts
packages/{bot,backend,shared}/src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/{bot,backend,shared}/src/**/*.ts: For feature toggles, check both global and guild-specific toggles usingFeatureToggleService.isEnabledForGuild()rather than checking them separately
Use branded types (e.g.,GuildId,UserId,ChannelId) for Discord IDs throughout the codebase to prevent type-level ID confusion
Files:
packages/bot/src/functions/music/commands/autoplay/settingsHandlers.tspackages/bot/src/functions/music/commands/autoplay/queueHandlers.tspackages/bot/src/functions/music/commands/autoplay.tspackages/bot/src/functions/music/commands/autoplay/artistHandlers.ts
packages/bot/src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
When building Discord embeds, use
EmbedBuilderService.createTemplate()orEmbedBuilderService.getTemplate()instead of constructing embeds directly
Files:
packages/bot/src/functions/music/commands/autoplay/settingsHandlers.tspackages/bot/src/functions/music/commands/autoplay/queueHandlers.tspackages/bot/src/functions/music/commands/autoplay.tspackages/bot/src/functions/music/commands/autoplay/artistHandlers.ts
🪛 GitHub Actions: CI/CD Pipeline / 1_Quality Gates.txt
packages/bot/src/functions/music/commands/autoplay/artistHandlers.ts
[error] 2-2: Cannot find module '@/services/recommendationFeedbackService' or its corresponding type declarations.
🪛 GitHub Actions: CI/CD Pipeline / Quality Gates
packages/bot/src/functions/music/commands/autoplay/artistHandlers.ts
[error] 2-2: TypeScript error TS2307: Cannot find module '@/services/recommendationFeedbackService' or its corresponding type declarations.
🔇 Additional comments (2)
README.md (1)
23-23: LGTM — Invite badge and "Why Lucky?" section are clear and consistent with the rest of the README.Also applies to: 52-62
packages/bot/src/functions/music/commands/autoplay/queueHandlers.ts (1)
96-121: LGTM — index-collect-then-reverse-remove is correct.Iterating forward to collect autoplay indices and then removing from the end avoids index shifting during removal.
replenishQueueis called once after the bulk removal, which is efficient.
Comment on lines
+159
to
+195
| case 'list': { | ||
| const summary = | ||
| await recommendationFeedbackService.getArtistFeedbackSummary( | ||
| userId, | ||
| ) | ||
|
|
||
| const preferredText = | ||
| summary.preferred.length > 0 | ||
| ? summary.preferred | ||
| .map((a: string) => `⭐ ${a}`) | ||
| .join('\n') | ||
| : 'No preferred artists.' | ||
|
|
||
| const blockedText = | ||
| summary.blocked.length > 0 | ||
| ? summary.blocked | ||
| .map((a: string) => `🚫 ${a}`) | ||
| .join('\n') | ||
| : 'No blocked artists.' | ||
|
|
||
| const listEmbed = createEmbed({ | ||
| title: '🎯 Your Artist Preferences', | ||
| description: `**Preferred:** (${summary.preferred.length})\n${preferredText}\n\n**Blocked:** (${summary.blocked.length})\n${blockedText}`, | ||
| color: EMBED_COLORS.AUTOPLAY as ColorResolvable, | ||
| emoji: EMOJIS.AUTOPLAY, | ||
| timestamp: true, | ||
| }) | ||
|
|
||
| await interactionReply({ | ||
| interaction, | ||
| content: { | ||
| embeds: [listEmbed], | ||
| ephemeral: true, | ||
| }, | ||
| }) | ||
| break | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the recommendationFeedbackService API to confirm expected scoping
ast-grep --pattern $'class $_ {
$$$
setArtistFeedback($$$) {
$$$
}
$$$
}'
rg -nP "(setArtistFeedback|removeArtistFeedback|getArtistFeedbackSummary)\s*[(:]" -C3 --type=tsRepository: LucasSantana-Dev/Lucky
Length of output: 9766
🏁 Script executed:
sed -n '372,410p' packages/bot/src/services/musicRecommendation/feedbackService.tsRepository: LucasSantana-Dev/Lucky
Length of output: 1370
🏁 Script executed:
sed -n '340,370p' packages/bot/src/services/musicRecommendation/feedbackService.tsRepository: LucasSantana-Dev/Lucky
Length of output: 1059
🏁 Script executed:
sed -n '320,365p' packages/bot/src/services/musicRecommendation/feedbackService.tsRepository: LucasSantana-Dev/Lucky
Length of output: 1526
🏁 Script executed:
sed -n '200,265p' packages/bot/src/services/musicRecommendation/feedbackService.tsRepository: LucasSantana-Dev/Lucky
Length of output: 2159
🏁 Script executed:
sed -n '264,310p' packages/bot/src/services/musicRecommendation/feedbackService.tsRepository: LucasSantana-Dev/Lucky
Length of output: 1467
🏁 Script executed:
cat -n packages/bot/src/functions/music/commands/autoplay/artistHandlers.ts | sed -n '40,170p'Repository: LucasSantana-Dev/Lucky
Length of output: 5542
The getArtistFeedbackSummary() API has a guild-scoping inconsistency with the write-side methods.
The setArtistFeedback() and removeArtistFeedback() methods accept a guildId parameter (lines 51–56, 93–98, 135–139), but both methods ignore it entirely—storing feedback in Redis using only the userId key: music:artist_feedback:${userId}. Conversely, getArtistFeedbackSummary(userId) retrieves the same per-user global feedback without any guild context. This means the list subcommand displays preferences across all guilds, while prefer/block/remove appear guild-scoped by accepting guildId. The service contract is misleading: either the write methods should not accept guildId, or the read method should accept it and filter accordingly (or store feedback per-(guild,user) in Redis).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bot/src/functions/music/commands/autoplay/artistHandlers.ts` around
lines 159 - 195, The list output is inconsistent because
getArtistFeedbackSummary(userId) reads global per-user feedback while
setArtistFeedback(...) and removeArtistFeedback(...) accept but ignore guildId
(they store to music:artist_feedback:${userId}); fix by making behavior
consistent: either remove guildId from setArtistFeedback/removeArtistFeedback
signatures and docs to keep global per-user storage, or change persistence and
getArtistFeedbackSummary to include a guildId parameter and store/read under a
per-guild key (e.g., include guildId in the Redis key) so setArtistFeedback,
removeArtistFeedback, and getArtistFeedbackSummary all use the same (user,guild)
scope; update the functions setArtistFeedback, removeArtistFeedback, and
getArtistFeedbackSummary and any callers (including the list subcommand) to the
chosen contract.
Comment on lines
+23
to
+37
| if (queue.tracks.size === 0) { | ||
| await interactionReply({ | ||
| interaction, | ||
| content: { | ||
| embeds: [ | ||
| createErrorEmbed( | ||
| 'Queue Empty', | ||
| 'No autoplay tracks to skip.', | ||
| ), | ||
| ], | ||
| ephemeral: true, | ||
| }, | ||
| }) | ||
| return | ||
| } |
There was a problem hiding this comment.
Wrong error message when queue is non-empty but contains no autoplay tracks vs. queue is entirely empty.
The queue.tracks.size === 0 branch shows "No autoplay tracks to skip", but the more accurate "Queue Empty" / "queue is empty" message would help the user. The downstream if (!skipped) branch already handles the "no autoplay tracks among existing tracks" case correctly — these two messages should be distinguishable.
🔧 Proposed tweak
if (queue.tracks.size === 0) {
await interactionReply({
interaction,
content: {
embeds: [
createErrorEmbed(
'Queue Empty',
- 'No autoplay tracks to skip.',
+ 'The queue is empty.',
),
],
ephemeral: true,
},
})
return
}📝 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
| if (queue.tracks.size === 0) { | |
| await interactionReply({ | |
| interaction, | |
| content: { | |
| embeds: [ | |
| createErrorEmbed( | |
| 'Queue Empty', | |
| 'No autoplay tracks to skip.', | |
| ), | |
| ], | |
| ephemeral: true, | |
| }, | |
| }) | |
| return | |
| } | |
| if (queue.tracks.size === 0) { | |
| await interactionReply({ | |
| interaction, | |
| content: { | |
| embeds: [ | |
| createErrorEmbed( | |
| 'Queue Empty', | |
| 'The queue is empty.', | |
| ), | |
| ], | |
| ephemeral: true, | |
| }, | |
| }) | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bot/src/functions/music/commands/autoplay/queueHandlers.ts` around
lines 23 - 37, The check on queue.tracks.size currently uses
createErrorEmbed('Queue Empty', 'No autoplay tracks to skip.') which mislabels
an actually-empty queue; change the message in the branch that checks
queue.tracks.size === 0 to clearly indicate the queue is empty (e.g.,
createErrorEmbed('Queue Empty', 'The queue is empty.') or similar) so it differs
from the later !skipped branch that reports "no autoplay tracks among existing
tracks"; update the interactionReply payload in that branch only (the code paths
involving interactionReply, createErrorEmbed, and the queue.tracks.size === 0
condition).
All three handler files (artistHandlers, settingsHandlers, queueHandlers) were using non-existent @/ aliases and wrong relative depths. Fixed to use correct 4-level relative paths and @lucky/shared/* workspace imports. Rewrote handleAutoplayStatus in queueHandlers to remove references to non-existent QueueMetadata fields (preferences, nowPlayingLastFm) and non-existent lastFmLinkService.getTrackUrl(). Replaced Array.from(queue.tracks) with queue.tracks.at(i) for-loop pattern to fix TS2769 overload error. Also: fix session-file-store mock in backend tests, switch backend jest to tsconfig.test.json, add ip-address security override.
github-actions
Bot
added
dependencies
Covers handleAutoplayStatus (status flags, track counting via for-loop, Blend field with/without Last.fm links, null metadata), handleSkipAutoplayTrack (empty queue, no autoplay tracks, correct remove+replenish), and handleClearAutoplayTracks (reverse-order removal, correct count in embed). These tests would have caught: broken QueueMetadata field references (metadata.preferences, metadata.nowPlayingLastFm), non-existent lastFmLinkService.getTrackUrl(), and Array.from(queue.tracks) overload error.
|
Size Change: 0 B Total Size: 367 kB ℹ️ View Unchanged
|
… assertions case 'artist' in the top-level subcommand switch was unreachable — artist is a subcommand group and is handled by the early-return group routing above the switch. Removing it eliminates the dead branch CodeRabbit flagged. Also fixed two autoplay.spec.ts assertions that checked for embed.description containing 'Blending taste' — the implementation uses fields (not description) and the blend copy is 'Mixing taste for N users'. Tests now assert the correct field structure produced by handleAutoplayStatus.
… adapter (Phase 1) Replace direct interaction.reply/editReply/deferReply calls with the interactionReply wrapper and MessageFlags.Ephemeral across all command handlers. Update all 13 affected spec files to match the new call signatures, fixing ESM chain failures from uuid v14 by mocking the interactionReply module.
…ling - Replace `any` types in isAutoplayTrack with Track and typed metadata cast - Add error replies in handleAutoplayGenreAdd/Remove when guildId or tag is missing (was silently returning, leaving users on a spinner) - Add default branch to handleAutoplayGenre switch to handle unknown subcommands Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Extract yt-dlp stream functions to streamBridge.ts and SoundCloud search/matching logic to soundcloudMatcher.ts. playerFactory.ts retains only extractor registration and re-exports moved symbols for backward compatibility. No behavioral changes.
Cover deferReply/editReply/followUp branching, ephemeral flag mapping, text-to-embed conversion, and error swallowing in the interactionReply wrapper introduced in Phase 1. 11 tests.
Wrap replenishQueue in handleSkipAutoplayTrack with a try/catch so that a replenish failure surfaces an ephemeral error embed instead of silently leaving the queue empty.
… test coverage - Wrap replenishQueue() calls in try-catch in handleSkipAutoplayTrack and handleClearAutoplayTracks; silent queue replenishment failures now surface an error reply - Add index re-verification before removal in handleClearAutoplayTracks to prevent removing wrong tracks if queue shifts between collection and removal - Wrap lastFmLinkService.getByDiscordId() in try-catch in handleAutoplayStatus; Blend field is omitted gracefully on lookup failure - Add error replies to bare !guildId guards in handleAutoplayGenreList and handleAutoplayGenreClear (was silently returning) - Add settingsHandlers.spec.ts (40 tests covering all mode/genre/analytics paths and service failure cases) - Add artistHandlers.spec.ts (22 tests covering all 5 artist subcommands, error paths, and service error handling) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Add soundcloudMatcher.ts and streamBridge.ts to sonar.cpd.exclusions (extracted from playerFactory.ts, not new logic). Add NOSONAR comment for S5852 on bounded regex in normalizeForMatch.
… S4036 Add playHandler.ts and autoplayPreference.ts to sonar.cpd.exclusions (extracted from play/index.ts). Add NOSONAR S4036 on spawn() in streamBridge.ts — command is hardcoded, URL validated before call.
Suppress CPD on queueHandlers.ts, settingsHandlers.ts and artistHandlers.ts — all three were extracted from autoplay.ts in this branch, so duplication against the original file is expected and not a real concern.
|
3 tasks
LucasSantana-Dev
added a commit
that referenced
this pull request
May 13, 2026
…istHandlers (#814) * docs: add invite badge, Why Lucky section, and expanded feature description * refactor: split autoplay.ts into queueHandlers, settingsHandlers, artistHandlers - Extract queue operations (skip, clear, status) into queueHandlers.ts - Extract settings operations (mode, genre, analytics) into settingsHandlers.ts - Extract artist preference operations (prefer, block, list, remove) into artistHandlers.ts - Refactor main autoplay.ts to thin router pattern with dispatcher - Maintain all existing functionality and type safety - Reduce main file from 1074 to ~250 lines * fix: correct broken import paths in split autoplay handlers All three handler files (artistHandlers, settingsHandlers, queueHandlers) were using non-existent @/ aliases and wrong relative depths. Fixed to use correct 4-level relative paths and @lucky/shared/* workspace imports. Rewrote handleAutoplayStatus in queueHandlers to remove references to non-existent QueueMetadata fields (preferences, nowPlayingLastFm) and non-existent lastFmLinkService.getTrackUrl(). Replaced Array.from(queue.tracks) with queue.tracks.at(i) for-loop pattern to fix TS2769 overload error. Also: fix session-file-store mock in backend tests, switch backend jest to tsconfig.test.json, add ip-address security override. * test(bot): add unit tests for autoplay queueHandlers Covers handleAutoplayStatus (status flags, track counting via for-loop, Blend field with/without Last.fm links, null metadata), handleSkipAutoplayTrack (empty queue, no autoplay tracks, correct remove+replenish), and handleClearAutoplayTracks (reverse-order removal, correct count in embed). These tests would have caught: broken QueueMetadata field references (metadata.preferences, metadata.nowPlayingLastFm), non-existent lastFmLinkService.getTrackUrl(), and Array.from(queue.tracks) overload error. * refactor(bot): remove dead artist case from autoplay switch, fix spec assertions case 'artist' in the top-level subcommand switch was unreachable — artist is a subcommand group and is handled by the early-return group routing above the switch. Removing it eliminates the dead branch CodeRabbit flagged. Also fixed two autoplay.spec.ts assertions that checked for embed.description containing 'Blending taste' — the implementation uses fields (not description) and the blend copy is 'Mixing taste for N users'. Tests now assert the correct field structure produced by handleAutoplayStatus. * refactor(bot): normalize all Discord replies through interactionReply adapter (Phase 1) Replace direct interaction.reply/editReply/deferReply calls with the interactionReply wrapper and MessageFlags.Ephemeral across all command handlers. Update all 13 affected spec files to match the new call signatures, fixing ESM chain failures from uuid v14 by mocking the interactionReply module. * fix(autoplay): address CodeRabbit review — type safety and error handling - Replace `any` types in isAutoplayTrack with Track and typed metadata cast - Add error replies in handleAutoplayGenreAdd/Remove when guildId or tag is missing (was silently returning, leaving users on a spinner) - Add default branch to handleAutoplayGenre switch to handle unknown subcommands * refactor(bot): extract play command execute logic into handlers/ (Phase 2) * refactor(bot): split playerFactory.ts into focused modules Extract yt-dlp stream functions to streamBridge.ts and SoundCloud search/matching logic to soundcloudMatcher.ts. playerFactory.ts retains only extractor registration and re-exports moved symbols for backward compatibility. No behavioral changes. * test(shared): add interactionReply unit tests Cover deferReply/editReply/followUp branching, ephemeral flag mapping, text-to-embed conversion, and error swallowing in the interactionReply wrapper introduced in Phase 1. 11 tests. * fix(bot): notify user when autoplay replenish fails on skip Wrap replenishQueue in handleSkipAutoplayTrack with a try/catch so that a replenish failure surfaces an ephemeral error embed instead of silently leaving the queue empty. * fix(autoplay): address PR review — error handling, guard replies, and test coverage - Wrap replenishQueue() calls in try-catch in handleSkipAutoplayTrack and handleClearAutoplayTracks; silent queue replenishment failures now surface an error reply - Add index re-verification before removal in handleClearAutoplayTracks to prevent removing wrong tracks if queue shifts between collection and removal - Wrap lastFmLinkService.getByDiscordId() in try-catch in handleAutoplayStatus; Blend field is omitted gracefully on lookup failure - Add error replies to bare !guildId guards in handleAutoplayGenreList and handleAutoplayGenreClear (was silently returning) - Add settingsHandlers.spec.ts (40 tests covering all mode/genre/analytics paths and service failure cases) - Add artistHandlers.spec.ts (22 tests covering all 5 artist subcommands, error paths, and service error handling) * chore(sonar): suppress CPD and S5852 hotspot on extracted player modules Add soundcloudMatcher.ts and streamBridge.ts to sonar.cpd.exclusions (extracted from playerFactory.ts, not new logic). Add NOSONAR comment for S5852 on bounded regex in normalizeForMatch. * chore(sonar): add extracted play handlers to CPD exclusions, suppress S4036 Add playHandler.ts and autoplayPreference.ts to sonar.cpd.exclusions (extracted from play/index.ts). Add NOSONAR S4036 on spawn() in streamBridge.ts — command is hardcoded, URL validated before call. * chore: add autoplay split files to sonar cpd.exclusions Suppress CPD on queueHandlers.ts, settingsHandlers.ts and artistHandlers.ts — all three were extracted from autoplay.ts in this branch, so duplication against the original file is expected and not a real concern. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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
Modularizes the 1074-line
autoplay.tscommand file into three focused handler modules, reducing complexity and improving maintainability.Changes
Type Safety
ChatInputCommandInteractionandGuildQueueTesting
--passWithNoTests)Summary by CodeRabbit
New Features
Documentation
UX Improvements
Stability