-
-
Notifications
You must be signed in to change notification settings - Fork 534
Feat: Introduce Biome.js, pnpm workspaces, and new commands #828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Integrates Biome.js for consistent code formatting and linting. Sets up pnpm workspaces for improved dependency management. Adds new commands: - `movenode`: Allows moving players to a different Lavalink node. - `fairplay`: Sets the bot to play music fairly. - `playlocal`: Enables playing local audio files. Updates localization files to include descriptions for the new commands.
Refactor `Play` command to use `ctx.sendDeferMessage` for deferring replies, centralizing the logic. Improve `MessageCreate` event performance by fetching multiple database values concurrently using `Promise.all`, reducing sequential database lookups.
- Perform extensive code refactoring across commands, events, and core modules. - Update project dependencies and apply consistent code formatting rules. - Improve autoplay logic for Spotify and YouTube sources. - Update localization strings across all supported languages.
Disables the `noNonNullAssertion` rule in Biome's linter configuration to allow the use of explicit non-null assertions where intended.
WalkthroughThis update introduces new configuration files for Biome and pnpm, shifts linting and formatting from ESLint/Prettier to Biome, and changes environment variable management to dotenvx. It also downgrades the package version, removes Fastify-related dependencies, and reformats a large number of TypeScript and localization files for consistent indentation and style. Additionally, new localization entries for several commands are added across multiple languages. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Bot
participant Lavalink
User->>Bot: Run lyrics command (optional song query)
Bot->>Lavalink: Search track if query provided
Lavalink-->>Bot: Return track info or error
Bot->>Lavalink: Fetch lyrics for track or current player track
Lavalink-->>Bot: Return lyrics data
Bot->>User: Send paginated lyrics embed with navigation and subscribe buttons
alt User subscribes to live lyrics
Bot->>Lavalink: Subscribe to live lyrics updates
loop on lyric line update
Lavalink-->>Bot: Current lyric line
Bot->>User: Update lyrics embed highlighting current line
end
end
User->>Bot: Interact with buttons (next, prev, subscribe, unsubscribe)
Bot->>User: Update embed or subscription status accordingly
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ 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)
🔇 Additional comments (5)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Updates the project's version number from `5.0.0-beta` to the stable `4.7.0`.
Replaced the `dotenv` package with `@dotenvx/dotenvx` for loading environment variables. Updated `dev` and `start` scripts to use `dotenvx run --` for automatic environment loading. Removed manual `dotenv` configuration from `src/env.ts`, simplifying the setup. This change streamlines environment variable handling and improves consistency.
Add a logger info message when stats are successfully posted to Top.gg. This provides confirmation and better visibility for the stat submission process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 22
♻️ Duplicate comments (3)
src/commands/music/PlayNext.ts (1)
140-154
: See comment in Play.ts – same autocomplete duplication hereReuse a shared helper to build autocomplete choices instead of repeating the loop.
locales/Vietnamese.json (2)
503-518
: English text present in Vietnamese locale.Same untranslated/typo issues as other locales – please provide Vietnamese equivalents and correct “Plays an your…”.
649-651
: String not localised.Translate
"The loop mode you want to set"
for Vietnamese users.
🧹 Nitpick comments (64)
src/commands/filters/Tremolo.ts (2)
42-42
: Return a more precise type thanPromise<any>
run
always resolves toPromise<void>
(it doesn’t return any value apart fromawait
-ed calls). Tightening the return type makes the contract clearer and lets TypeScript catch accidental future changes.- public async run(client: Lavamusic, ctx: Context): Promise<any> { + public async run(client: Lavamusic, ctx: Context): Promise<void> {
48-69
: Remove duplicated logic by toggling once, then branch on the new stateThe current implementation calls
toggleTremolo()
in each branch and duplicates the embed construction. You can make the code shorter and less error-prone by toggling first, reading the new state, and sending one embed accordingly.- const tremoloEnabled = player.filterManager.filters.tremolo; - - if (tremoloEnabled) { - player.filterManager.toggleTremolo(); - await ctx.sendMessage({ - embeds: [ - { - description: ctx.locale("cmd.tremolo.messages.disabled"), - color: this.client.color.main, - }, - ], - }); - } else { - player.filterManager.toggleTremolo(); - await ctx.sendMessage({ - embeds: [ - { - description: ctx.locale("cmd.tremolo.messages.enabled"), - color: this.client.color.main, - }, - ], - }); - } + // Toggle and inspect the *new* state + const newState = player.filterManager.toggleTremolo(); + + await ctx.sendMessage({ + embeds: [ + { + description: newState + ? ctx.locale("cmd.tremolo.messages.enabled") + : ctx.locale("cmd.tremolo.messages.disabled"), + color: this.client.color.main, + }, + ], + });src/commands/filters/Vibrato.ts (2)
42-71
: Collapse duplicate branches to DRY up therun
method
The sametoggleVibrato()
call and almost-identical embed construction appear in both branches. A single toggle followed by one embed keeps intent clear and reduces maintenance overhead.- const vibratoEnabled = player.filterManager.filters.vibrato; - - if (vibratoEnabled) { - player.filterManager.toggleVibrato(); - await ctx.sendMessage({ - embeds: [ - { - description: ctx.locale("cmd.vibrato.messages.disabled"), - color: this.client.color.main, - }, - ], - }); - } else { - player.filterManager.toggleVibrato(); - await ctx.sendMessage({ - embeds: [ - { - description: ctx.locale("cmd.vibrato.messages.enabled"), - color: this.client.color.main, - }, - ], - }); - } + const wasEnabled = player.filterManager.filters.vibrato; + player.filterManager.toggleVibrato(); + + await ctx.sendMessage({ + embeds: [ + { + description: ctx.locale( + wasEnabled + ? "cmd.vibrato.messages.disabled" + : "cmd.vibrato.messages.enabled", + ), + color: this.client.color.main, + }, + ], + });
42-42
: Narrow the return type: usePromise<void>
instead ofPromise<any>
The method only sends a message and exits; specifyingPromise<void>
prevents theany
escape hatch and improves type-safety across the command layer.src/commands/filters/Pitch.ts (2)
40-44
: Prefer the “NUMBER” slash-option type instead of parsing a string manually
type: 3
registers the option as a STRING, which forces you to re-implement number validation/parsing below.
Discord’s API already provides a native numeric option (type: 10
, “NUMBER”). Switching to it lets Discord handle basic validation, removes the need for the regex, and simplifies the command logic.{ name: "pitch", description: "cmd.pitch.options.pitch", - type: 3, // STRING + type: 10, // NUMBER required: true, },
59-63
: Validation can be simplified & made more robust
- The regex still rejects inputs such as
5.
(trailing dot) thatparseFloat
accepts.- Duplicating validation across commands is error-prone.
Leverage
Number.isFinite
afterparseFloat
(and, if you adopt the NUMBER option above, most of this block disappears entirely):-const pitchString = args[0].replace(",", "."); -const isValidNumber = /^[0-9]*\.?[0-9]+$/.test(pitchString); -const pitch = Number.parseFloat(pitchString); - -if (!isValidNumber || Number.isNaN(pitch) || pitch < 0.5 || pitch > 5) { +const pitch = Number.parseFloat(args[0].replace(",", ".")); +if (!Number.isFinite(pitch) || pitch < 0.5 || pitch > 5) {Consider extracting common numeric-range validation into a shared utility; the other filter commands appear to duplicate similar checks.
src/commands/filters/Rate.ts (1)
59-64
: Simplify and harden rate parsingYou already normalise the comma, but the regex still allows multiple dots or leading dots (e.g.
.5
,1..2
). Consider:- const isValidNumber = /^[0-9]*\.?[0-9]+$/.test(rateString); - const rate = Number.parseFloat(rateString); + const rate = Number(rateString); + const isValidNumber = Number.isFinite(rate);
Number()
plusNumber.isFinite
rejects malformed inputs and removes the need for a custom regex.src/commands/filters/LowPass.ts (2)
37-40
: Slash-command definition missing an option for frequency
slashCommand: true
is enabled butoptions
is an empty array, so/lowpass
cannot accept the<number>
parameter hinted at in the description.
Add a numeric option (min/max as appropriate) to keep UX consistent.- options: [], + options: [ + { + name: "frequency", + description: "Cut-off frequency (Hz)", + type: "NUMBER", + required: false, + minValue: 10, + maxValue: 22050, + }, + ],
50-61
: Hard-coded cutoff value reduces usefulnessWhen the filter is enabled the cutoff is always
20 Hz
:await player.filterManager.toggleLowPass(20);That is barely audible and probably not the intent.
Either expose the parameter to users (see previous comment) or document why exactly20 Hz
is chosen.- await player.filterManager.toggleLowPass(20); + // Use provided argument or a sensible default (e.g. 1000 Hz) + const freq = Number(ctx.options?.getNumber?.("frequency") ?? 1000); + await player.filterManager.toggleLowPass(freq);scripts/restart.js (1)
4-12
: Optional: logstdout
for easier debuggingCurrently only
stderr
is emitted. Capturingstdout
can help when diagnosing start-up issues.if (stderr) { console.error(`Error starting Lavamusic: ${stderr}`); + } else if (stdout) { + console.log(stdout); }src/commands/playlist/List.ts (1)
117-123
: Possible embed overflow when listing many playlists
description: playlists.map(...).join("\n")
can exceed Discord’s 4 096-character embed description limit if a user owns a large number of playlists or uses very long playlist names. Consider chunking into multiple fields/pages or truncating with a “and X more…” suffix.- description: playlists - .map((playlist: any) => playlist.name) - .join("\n"), + description: client.utils.truncateList( + playlists.map((p: any) => p.name), + 4000 /* safe margin */ + ),src/commands/music/PlayLocal.ts (1)
65-76
: Add basic size check to guard against huge uploadsAn attacker can upload multi-hundred-MB audio files, blocking the bot while the file streams. A quick pre-check on
attachment.size
(orcontentLength
via HEAD request) prevents abuse.+// Reject files larger than 25 MB +if (attachment.size > 25 * 1024 * 1024) { + return ctx.sendMessage({ + embeds: [ + this.client + .embed() + .setColor(this.client.color.red) + .setDescription(ctx.locale("cmd.playlocal.errors.file_too_large")), + ], + }); +}src/commands/music/Loop.ts (1)
13-14
: Remove self-alias to keep help output cleanThe command already registers under the name
loop
; having"aliases": ["loop"]
is redundant and clutters auto-generated help / slash metadata.
Recommend dropping the duplicate entry.src/commands/music/Stop.ts (1)
39-44
: Guard againstctx.guild
beingundefined
and drop redundantawait
.
ctx.guild.id
assumes the command can never be invoked outside a guild.
Ifctx.guild
is typed as possiblyundefined
insideContext
, a runtime
crash will occur. Either:
- Re-introduce the non-null assertion (
ctx.guild!.id
), or- Perform an early
if (!ctx.guild) return …
guard.
return await ctx.sendMessage(…)
adds an unnecessary micro-await.
Unless you are inside atry/ catch
, you can just return the promise.- const player = client.manager.getPlayer(ctx.guild.id); + if (!ctx.guild) { + return ctx.sendMessage(ctx.locale("event.errors.guild_only")); + } + const player = client.manager.getPlayer(ctx.guild.id); … - return await ctx.sendMessage( + return ctx.sendMessage( ctx.locale("event.message.no_music_playing"), ); … - return await ctx.sendMessage({ + return ctx.sendMessage({ embeds: [ embed .setColor(this.client.color.main) .setDescription(ctx.locale("cmd.stop.messages.stopped")), ], });Also applies to: 47-54
src/commands/music/Volume.ts (1)
51-71
: Reject non-integer or floating-point volume values.
Number(args[0])
permits"10.5"
⇒10.5
.
Discord volume setters (and your localisation strings) imply integers.
Add an integer check to prevent silent flooring.- const number = Number(args[0]); + const number = Number(args[0]); + if (!Number.isInteger(number)) { + return ctx.sendMessage({ + embeds: [ + embed + .setColor(this.client.color.red) + .setDescription(ctx.locale("cmd.volume.messages.invalid_number")), + ], + }); + }src/commands/config/Prefix.ts (2)
90-94
: Externalise magic-number & trim user input
prefix.length > 3
is a hard-coded rule. Exposing the limit in a config/env constant (and trimming whitespace before measuring) makes the constraint discoverable and avoids silent divergence if the requirement changes.- if (prefix.length > 3) { + const MAX_PREFIX_LEN = this.client.constants.MAX_PREFIX_LEN ?? 3; + prefix = prefix.trim(); + if (prefix.length > MAX_PREFIX_LEN) {
70-75
: Type-safety nit: guard.value
access
ctx.options.get("prefix")
can beundefined
; double-chained access is safe, but assigning the result directly tostring | undefined
then calling.length
(Line 90) relies on the earlier!prefix
branch. An explicit non-null assertion or early return would make intent clearer and keeps the compiler happy ifstrictNullChecks
is enabled.src/commands/music/Pause.ts (1)
38-42
: Guard against undefined player for extra safety
player
should always exist due to theplayer.active: true
gate in the baseCommand
, but an explicit null-check improves self-containment and prevents future regressions if that gate changes.- const player = client.manager.getPlayer(ctx.guild.id); - const embed = this.client.embed(); + const player = client.manager.getPlayer(ctx.guild.id); + if (!player) return; // unreachable today, safe tomorrow + const embed = this.client.embed();src/commands/filters/Speed.ts (1)
59-64
: Streamline number validation & reject edge-case inputsThe regex allows
.5
but rejects1.
– an unintuitive discrepancy.
A simpler approach is to rely onNumber.isFinite()
afterparseFloat
, which also handles scientific notation and trims whitespace.- const isValidNumber = /^[0-9]*\.?[0-9]+$/.test(speedString); - const speed = Number.parseFloat(speedString); + const speed = Number.parseFloat(speedString); + const isValidNumber = Number.isFinite(speed);The subsequent range check remains unchanged.
This reduces maintenance overhead while covering more cases.src/commands/dev/DeleteInvites.ts (2)
3-7
: Align class name with file & command for clarityThe file is
DeleteInvites.ts
, the command is registered as"destroyinvites"
, but the class isDestroyInvites
.
For grep-ability and DX, keep the three in sync.-export default class DestroyInvites extends Command { +export default class DeleteInvites extends Command {Optionally add
deleteinvites
as an alias to stay backwards-compatible.
48-57
: Consider per-invite error isolation
Promise.all()
stops on the first rejection. A single failed deletion aborts the rest.-for (const invite of botInvites) await invite.delete(); +for (const invite of botInvites) { + try { await invite.delete(); } catch (_) { /* ignore single failures */ } +}Ensures maximum cleanup even when some invites are already revoked or missing permissions.
src/events/client/ChannelDelete.ts (1)
17-30
: Replace magic numbers & strengthen typing for channel checksUsing raw numeric literals (
2
,0
) to identify channel kinds is brittle and obscures intent. Discord.js exportsChannelType
(e.g.ChannelType.GuildVoice
,ChannelType.GuildText
) which is safer and self-documenting.
While you’re touching this, thechannel
parameter can be narrowed fromany
toGuildChannel
(orVoiceBasedChannel & GuildTextBasedChannel
union) to catch type errors at compile time.-import { Event, type Lavamusic } from "../../structures/index"; +import { Event, type Lavamusic } from "../../structures/index"; +import { ChannelType, type GuildChannel } from "discord.js"; // … -public async run(channel: any): Promise<void> { +public async run(channel: GuildChannel): Promise<void> { - if (channel.type === 2 && s.voiceId === channel.id) { + if (channel.type === ChannelType.GuildVoice && s.voiceId === channel.id) { - if (channel.type === 2 && stay.voiceId === channel.id) { + if (channel.type === ChannelType.GuildVoice && stay.voiceId === channel.id) { - if (setup && channel.type === 0 && setup.textId === channel.id) { + if (setup && channel.type === ChannelType.GuildText && setup.textId === channel.id) {This small refactor improves readability and guards you if Discord.js ever changes its underlying enum values.
src/events/client/GuildDelete.ts (1)
60-77
: Minor: verify log-channel type before cast
channels.fetch()
can return a non-text-based channel. A quickif (!channel?.isTextBased())
guard before the cast would prevent runtime failures if the ID is accidentally mis-configured.No blocking issues; just a safety net.
src/commands/music/Remove.ts (1)
65-71
: Validate that the provided song number is an integer before mutating the queue
Number(args[0])
will happily coerce"1.5"
or"2e1"
into numeric values that satisfy the current range checks, leading to unexpected removals or runtime errors insideplayer.queue.remove
.
Guard against non-integer input:-const songNumber = Number(args[0]); +const songNumber = Number(args[0]); +if (!Number.isInteger(songNumber)) { + return await ctx.sendMessage({ + embeds: [ + embed + .setColor(this.client.color.red) + .setDescription(ctx.locale("cmd.remove.errors.invalid_number")), + ], + }); +}src/commands/music/Play.ts (1)
146-160
: Autocomplete logic duplicated across multiple commands – extract to a shared helperThe block that builds
songs
from a search result is identical inPlay.ts
,PlayNext.ts
, and several other commands. Consider centralising this logic (e.g.buildAutocompleteChoices(query, user)
) to:
- keep behaviour consistent,
- avoid copy-paste bugs,
- make it easier to unit-test.
No functional change required – purely a refactor opportunity.
src/commands/music/Leave.ts (1)
42-50
: Use channel mention instead of raw ID for better UX
channelId
is rendered as a plain string, forcing users to manually map the ID to a voice channel. Wrapping it as<#${channelId}>
produces a clickable mention.- .setDescription(ctx.locale("cmd.leave.left", { channelId })) + .setDescription( + ctx.locale("cmd.leave.left", { channel: `<#${channelId}>` }), + )src/commands/music/FairPlay.ts (1)
55-66
: Defer the reply while re-ordering large queues
applyFairPlayToQueue
can be O(n²) on large queues (map → while loop → splice → add). Users receive no feedback during this potentially long-running operation.
Consider deferring the interaction or sending a “Re-ordering queue…” message before mutation, then editing it once the embed is ready.src/commands/music/Grab.ts (1)
70-76
: Avoid markdown in embed titles & guard against missing artwork
- Bold markdown is stripped from embed titles by Discord; it’s unnecessary noise.
song.info.artworkUrl!
will throw if the provider omitted artwork (e.g., some radio streams).- .setTitle(`**${song.info.title}**`) - .setThumbnail(song.info.artworkUrl!) + .setTitle(song.info.title) + if (song.info.artworkUrl) + embed.setThumbnail(song.info.artworkUrl)src/commands/dev/Shutdown.ts (1)
45-50
: Fix unmatched bold markdownThere’s an opening
**
before “bot” but no matching closing delimiter.- `**Are you sure you want to shutdown the bot **\`${client.user?.username}\`?`, + `**Are you sure you want to shutdown the bot \`${client.user?.username}\`?**`,locales/Norwegian.json (2)
503-518
: English strings inside Norwegian locale – translate or mark as pendingThe newly-added command descriptions (
movenode
,fairplay
,playlocal
) and option labels are still in English. End-users with the Norwegian language selected will suddenly see English UI fragments, breaking localisation consistency.Either provide proper Norwegian translations or add a TODO marker so translators can spot the gap.
649-650
: Root-level key added in English
"The loop mode you want to set"
is appended in English. All prior root-level strings are translated; keep the same convention for UX consistency.locales/ChineseCN.json (2)
503-518
: Untranslated English fragments in Simplified-Chinese fileThe three new commands (
movenode
,fairplay
,playlocal
) were inserted with English descriptions/options. Provide Chinese translations or flag them for later to avoid mixed-language UI.
649-650
: English root string slipped through
"The loop mode you want to set"
remains English – translate or tag for translators.locales/Turkish.json (2)
513-528
: New commands are still in English
movenode
,fairplay
, andplaylocal
carry English text in the Turkish locale. Translate before shipping.
661-662
: Untranslated root-level message
"The loop mode you want to set"
is English – needs Turkish version for consistency.src/commands/music/Search.ts (1)
238-240
: Avoid// @ts-ignore
– update typings insteadTwo
// @ts-ignore
directives suppress type errors aroundctx.editMessage(...)
. Relying on ignore comments hides real issues and defeats Biome’s stricter checks introduced in this PR.If the discord.js typings are missing an overload, extend the types locally or cast explicitly instead of silencing the compiler.
Also applies to: 317-335
src/commands/dev/CreateInvite.ts (2)
1-1
: Global lint suppression is broad – scope it tighter
/** biome-ignore-all lint/style/noNonNullAssertion */
disables the rule for the whole file. Limit the ignore to the single line(s) that actually require!
to keep the rest of the file protected by Biome.
81-85
: Security / moderation consideration for unlimited invites
maxUses: 0
creates an invite with unlimited uses for any member who receives the link. Consider setting a reasonablemaxUses
or restricting permissions if the invite is meant only for troubleshooting.locales/Korean.json (1)
669-671
: Root-level key is not localised
"The loop mode you want to set"
is copied verbatim from English. Add a Korean version, otherwise users will get an English tooltip in a fully Korean interface.locales/Russian.json (1)
649-651
: Строка верхнего уровня не переведена
The loop mode you want to set
требуется русский перевод.locales/SpanishES.json (1)
649-651
: Cadena raíz sin traducir
The loop mode you want to set
continúa en inglés; proporcione la traducción al español.locales/Hindi.json (1)
649-651
: शीर्ष-स्तरीय स्ट्रिंग का अनुवाद नहीं हुआ
The loop mode you want to set
अंग्रेज़ी में ही है; हिन्दी अनुवाद जोड़ें।locales/Japanese.json (1)
649-651
: ルート文字列が未翻訳
The loop mode you want to set
は英語のままです。日本語訳を追記してください。locales/Indonesian.json (2)
505-518
: Indonesian entries are still in EnglishThe newly-added keys under
cmd.movenode
,cmd.fairplay
, andcmd.playlocal
are written in English ("Move the players…"
,"Set the bot…"
, etc.).
Because this file will be shown to Indonesian users, the values should be translated rather than copied verbatim from English.
The same applies to the option"file": "The audio file you want to play"
.
649-650
: Missing localisation for generic stringsThe two root-level strings
"Restart the bot": "Restart bot", "The loop mode you want to set": "The loop mode you want to set"
are still English or half-translated.
Please provide full Indonesian wording (e.g."Mulai ulang bot"
and""Mode loop yang ingin Anda atur"
), otherwise users will see mixed languages.src/commands/dev/MoveNode.ts (2)
54-58
:currentNodeId
only reflects the first player
currentNodeId
is derived fromallPlayers.values().next().value
, so if players are already distributed across multiple nodes you will hide only the first node from the available list.
Consider collecting the full set of node ids currently in use to avoid suggesting nodes that already host some players.-const currentNodeId = - allPlayers.size > 0 ? allPlayers.values().next().value?.node.options.id : null; +const usedNodeIds = new Set( + Array.from(allPlayers.values()).map((p) => p.node.options.id), +);…and later filter with
!usedNodeIds.has(node.options.id)
.
165-183
: Move operations run sequentially – can be parallelisedPlayers are moved one-by-one with
await
inside afor…of
loop.
With dozens of players this can take seconds; there is no data dependency between moves, so they can be executed concurrently:-for (const player of allPlayers.values()) { - if (player.node.options.id === nodeId) continue; - /* … */ - await player.moveNode(nodeId); - /* … */ -} +await Promise.all( + Array.from(allPlayers.values()).map(async (player) => { + if (player.node.options.id === nodeId) return; + const from = player.node.options.id ?? "unknown"; + try { + await player.moveNode(nodeId); + results.push({ guildId: player.guildId, from, to: nodeId }); + } catch (err) { + results.push({ + guildId: player.guildId, + from, + to: nodeId, + error: err instanceof Error ? err.message : String(err), + }); + } + }), +);This keeps the summary logic intact while shortening total execution time.
src/commands/music/Queue.ts (1)
38-108
: Minor pre-existing edge-case worth notingUnrelated to the formatting, but while we are here:
player.queue.utils.totalDuration() - player.queue.current?.info.duration!
will throw ifcurrent
isundefined
(e.g. race when the track finishes while the command is executed). Consider guarding the subtraction.No action required for this PR, just flagging it for a later hardening pass.
src/commands/playlist/AddSong.ts (1)
56-139
: Potential null-value pitfall in new code pathExisting logic sets
trackStrings
only for three explicitloadType
s. If Lavalink ever returns a different value (e.g."search" | "track" | "playlist"
typo or future type),trackStrings
will be leftundefined
, triggering a runtime error indb.addTracksToPlaylist
.+ else { + return await ctx.sendMessage({ embeds: [ this.client.embed() + .setDescription("Unsupported load type returned by Lavalink.") + .setColor(this.client.color.red) ]}); + }Not blocking this PR, but worth tightening.
src/commands/playlist/Delete.ts (1)
55-86
: Input validation suggestionIf
playlistName === ""
(user passes only spaces) the DB is queried with an empty string. A quick guard would avoid the extra query:- const playlistName = args.join(" ").trim(); + const playlistName = args.join(" ").trim(); + if (!playlistName) { + return ctx.sendMessage({ embeds: [embed.setDescription("Provide a playlist name").setColor(this.client.color.red)] }); + }src/commands/playlist/RemoveSong.ts (1)
99-126
: Catch block silences DB-layer errorsThe generic
catch
logs to console but still returns a generic embed. Consider re-throwing or at least including the error ID so ops can correlate logs to users’ reports.Not critical for this style-only PR, flagged for future reliability work.
locales/ChineseTW.json (2)
504-518
: Untranslated / grammatically incorrect additionsNew command strings (
movenode
,fairplay
,playlocal
) and option descriptions remain in English.
Provide Traditional Chinese equivalents for a consistent UX.
649-651
: Root-level key not localised
"The loop mode you want to set"
is still English; translate or remove if unused.locales/Polish.json (2)
528-542
: English text in Polish localeThe new command descriptions/options are English.
Please translate ("Przenieś odtwarzacze na inny węzeł Lavalink"
, etc.) for consistency.
674-676
: Missing translationRoot-level
"The loop mode you want to set"
is not translated to Polish.src/commands/playlist/Steal.ts (2)
53-70
: Prefer separate variables instead of re-typingtargetUser
.
targetUser
flips betweenstring
,User
, andnull
, which defeats type-narrowing and forces a lot ofany
/casting.
Maintain two distinct variables – e.g.rawUserInput: string
andresolvedUser: User
– to keep the types sound and readable.
This also avoids accidental misuse of string methods on aUser
instance later on.
151-156
: Bulk-insert could exceed DB parameter limits.
createPlaylistWithTracks
is called with the entiretargetSongs
array. For large playlists this can blow past parameter/packet limits (e.g. 65 535 for MySQL).
Consider batching or relying on a singleINSERT … SELECT
query inside the DAO instead.locales/German.json (2)
503-518
: Untranslated strings & typo inside German locale.The newly added command descriptions/options remain in English and the phrase
“Plays an your local audio files” contains a grammatical error.Translate these keys and fix the typo before shipping to avoid mixed-language UI.
649-651
: Root-level string still English.
"The loop mode you want to set"
should be provided in German for consistency.locales/French.json (2)
503-518
: English strings slipped into French locale.
movenode
,fairplay
,playlocal
entries are still English and include the same “Plays an your local audio files” typo.
Localise or they will display in English for FR users.
649-651
: Missing French translation for new prompt string.
"The loop mode you want to set"
should be translated (e.g."Le mode de boucle à définir"
).package.json (2)
12-13
: Provide a fix-mode lint script for CI convenienceYou now run Biome for both lint and format, but there is no “auto-fix” target. A common pattern is:
"lint": "biome check", + "lint:fix": "biome check --apply",
This allows contributors (and CI) to distinguish between checking and fixing.
35-44
: Trim unused or redundant devDependencies
@swc/core
– only needed iftsup
is configured with the--loader swc
option; remove otherwise.@types/[email protected]
– upstreami18n
jumped to 0.15; the 0.13 typings are for an older API and will generate[email protected]
errors. Either bump to@types/[email protected].*
or drop if not imported directly.Cleaning these out reduces cold-install size and avoids stale typings.
src/commands/dev/Deploy.ts (1)
88-94
: Collector attached to the channel is broader than necessary.
(ctx.channel as TextChannel).createMessageComponentCollector(...)
listens for any button in that channel. A safer pattern is to scope the collector to the specific message you just sent:const collector = msg!.createMessageComponentCollector({ filter, componentType: ComponentType.Button, time: 30_000, });This prevents unrelated button presses in the same channel from triggering the filter and simplifies permissions logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (92)
biome.json
(1 hunks)locales/ChineseCN.json
(2 hunks)locales/ChineseTW.json
(2 hunks)locales/Dutch.json
(1 hunks)locales/EnglishUS.json
(1 hunks)locales/French.json
(2 hunks)locales/German.json
(2 hunks)locales/Hindi.json
(2 hunks)locales/Indonesian.json
(2 hunks)locales/Italian.json
(1 hunks)locales/Japanese.json
(2 hunks)locales/Korean.json
(2 hunks)locales/Norwegian.json
(2 hunks)locales/Polish.json
(2 hunks)locales/Russian.json
(2 hunks)locales/SpanishES.json
(2 hunks)locales/Thai.json
(1 hunks)locales/Turkish.json
(2 hunks)locales/Vietnamese.json
(2 hunks)package.json
(1 hunks)pnpm-workspace.yaml
(1 hunks)scripts/restart.js
(1 hunks)src/LavaClient.ts
(1 hunks)src/commands/config/247.ts
(1 hunks)src/commands/config/Dj.ts
(1 hunks)src/commands/config/Language.ts
(1 hunks)src/commands/config/Prefix.ts
(1 hunks)src/commands/config/Setup.ts
(1 hunks)src/commands/dev/CreateInvite.ts
(1 hunks)src/commands/dev/DeleteInvites.ts
(1 hunks)src/commands/dev/Deploy.ts
(1 hunks)src/commands/dev/Eval.ts
(1 hunks)src/commands/dev/GuildLeave.ts
(1 hunks)src/commands/dev/GuildList.ts
(1 hunks)src/commands/dev/MoveNode.ts
(1 hunks)src/commands/dev/Restart.ts
(1 hunks)src/commands/dev/Shutdown.ts
(1 hunks)src/commands/filters/8d.ts
(1 hunks)src/commands/filters/BassBoost.ts
(1 hunks)src/commands/filters/Karaoke.ts
(1 hunks)src/commands/filters/LowPass.ts
(1 hunks)src/commands/filters/NightCore.ts
(1 hunks)src/commands/filters/Pitch.ts
(1 hunks)src/commands/filters/Rate.ts
(1 hunks)src/commands/filters/Reset.ts
(1 hunks)src/commands/filters/Rotation.ts
(1 hunks)src/commands/filters/Speed.ts
(1 hunks)src/commands/filters/Tremolo.ts
(1 hunks)src/commands/filters/Vibrato.ts
(1 hunks)src/commands/info/About.ts
(1 hunks)src/commands/info/Botinfo.ts
(1 hunks)src/commands/info/Help.ts
(1 hunks)src/commands/info/Invite.ts
(1 hunks)src/commands/info/LavaLink.ts
(1 hunks)src/commands/info/Ping.ts
(1 hunks)src/commands/music/Autoplay.ts
(1 hunks)src/commands/music/ClearQueue.ts
(1 hunks)src/commands/music/FairPlay.ts
(1 hunks)src/commands/music/Grab.ts
(1 hunks)src/commands/music/Join.ts
(1 hunks)src/commands/music/Leave.ts
(1 hunks)src/commands/music/Loop.ts
(1 hunks)src/commands/music/Lyrics.ts
(1 hunks)src/commands/music/Nowplaying.ts
(1 hunks)src/commands/music/Pause.ts
(1 hunks)src/commands/music/Play.ts
(1 hunks)src/commands/music/PlayLocal.ts
(1 hunks)src/commands/music/PlayNext.ts
(1 hunks)src/commands/music/Queue.ts
(1 hunks)src/commands/music/Remove.ts
(1 hunks)src/commands/music/Replay.ts
(1 hunks)src/commands/music/Resume.ts
(1 hunks)src/commands/music/Search.ts
(1 hunks)src/commands/music/Seek.ts
(1 hunks)src/commands/music/Shuffle.ts
(1 hunks)src/commands/music/Skip.ts
(1 hunks)src/commands/music/Skipto.ts
(1 hunks)src/commands/music/Stop.ts
(1 hunks)src/commands/music/Volume.ts
(1 hunks)src/commands/playlist/AddSong.ts
(1 hunks)src/commands/playlist/Create.ts
(1 hunks)src/commands/playlist/Delete.ts
(1 hunks)src/commands/playlist/List.ts
(1 hunks)src/commands/playlist/Load.ts
(1 hunks)src/commands/playlist/RemoveSong.ts
(1 hunks)src/commands/playlist/Steal.ts
(1 hunks)src/config.ts
(1 hunks)src/database/server.ts
(1 hunks)src/env.ts
(1 hunks)src/events/client/ChannelDelete.ts
(1 hunks)src/events/client/GuildCreate.ts
(1 hunks)src/events/client/GuildDelete.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (12)
src/commands/info/About.ts (2)
src/structures/Lavamusic.ts (2)
Lavamusic
(28-246)embed
(43-45)src/structures/Context.ts (1)
Context
(22-171)
src/commands/config/Prefix.ts (2)
src/structures/Lavamusic.ts (2)
Lavamusic
(28-246)embed
(43-45)src/structures/Context.ts (2)
Context
(22-171)isInteraction
(68-70)
src/commands/music/PlayNext.ts (3)
src/structures/Command.ts (1)
Command
(45-107)src/structures/Lavamusic.ts (2)
Lavamusic
(28-246)embed
(43-45)src/structures/Context.ts (1)
Context
(22-171)
src/events/client/GuildCreate.ts (2)
src/structures/Event.ts (1)
Event
(20-38)src/structures/Lavamusic.ts (2)
Lavamusic
(28-246)embed
(43-45)
src/commands/dev/Eval.ts (2)
src/structures/Lavamusic.ts (1)
Lavamusic
(28-246)src/structures/Context.ts (1)
Context
(22-171)
src/commands/dev/MoveNode.ts (1)
src/structures/Context.ts (1)
Context
(22-171)
src/commands/music/Pause.ts (2)
src/structures/Lavamusic.ts (2)
Lavamusic
(28-246)embed
(43-45)src/structures/Context.ts (1)
Context
(22-171)
src/commands/info/Ping.ts (2)
src/structures/Lavamusic.ts (2)
Lavamusic
(28-246)embed
(43-45)src/structures/Context.ts (1)
Context
(22-171)
src/commands/music/Play.ts (4)
src/structures/Command.ts (1)
Command
(45-107)src/structures/Lavamusic.ts (2)
Lavamusic
(28-246)embed
(43-45)src/structures/Context.ts (1)
Context
(22-171)src/utils/functions/player.ts (1)
applyFairPlayToQueue
(147-184)
src/commands/music/Leave.ts (2)
src/structures/Lavamusic.ts (2)
Lavamusic
(28-246)embed
(43-45)src/structures/Context.ts (1)
Context
(22-171)
src/commands/music/Stop.ts (2)
src/structures/Lavamusic.ts (2)
Lavamusic
(28-246)embed
(43-45)src/structures/Context.ts (1)
Context
(22-171)
src/commands/music/FairPlay.ts (2)
src/structures/Lavamusic.ts (2)
Lavamusic
(28-246)embed
(43-45)src/utils/functions/player.ts (1)
applyFairPlayToQueue
(147-184)
🔇 Additional comments (43)
src/commands/filters/NightCore.ts (1)
1-84
: Formatting-only diff ‒ logic unchanged, looks good
All modifications are indentation / spacing normalisation introduced by Biome; functional flow and metadata remain identical. No issues spotted.src/commands/filters/Vibrato.ts (1)
8-40
: Formatting looks good – no functional concerns
Indentation now follows the Biome-mandated tab style and the constructor metadata remains unchanged.src/commands/filters/Rate.ts (1)
1-46
: Formatting-only diff looks goodAll changes in this chunk are indentation / whitespace. Nothing appears functionally altered. The new tab-based style aligns with the Biome rules introduced in this PR.
locales/EnglishUS.json (1)
721-722
: Nothing to flag – newline addition is harmlessNo functional or structural change; keeping the file POSIX-compliant with a trailing newline is fine.
src/commands/info/Invite.ts (1)
5-61
: Formatting-only diff looks goodIndentation was normalised; behaviour, permissions and URLs remain untouched.
src/commands/music/Replay.ts (1)
4-63
: Logic unchanged and still correctThe early‐return guard after the seekability check is intact; formatting only.
src/commands/music/Resume.ts (1)
4-63
: Formatting-only diff looks goodBehaviour is unchanged; the paused check and feedback flow are still correct.
src/commands/music/Skip.ts (1)
38-70
: Looks good – purely stylistic changes.No behavioural differences introduced; constructor metadata and
run
flow remain intact.
Nothing further to address here.src/commands/music/Nowplaying.ts (1)
57-78
: LGTM – logic unchanged and safeThe only edits are indentation; existing flow for building the components remains sound.
src/commands/music/ClearQueue.ts (1)
38-70
: LGTM – functional behaviour unchangedOnly indentation was modified; logic for player checks, empty queue handling, and queue clearing remains sound.
src/commands/dev/GuildList.ts (1)
38-47
: Guard against empty guild list in single-process deployments
broadcastEval
returnsundefined
when the bot isn’t sharded.
client.utils.chunk(guildList!, 10)
will throw ifguildList
isundefined
.Consider a fallback such as:
const allGuilds = guilds?.reduce((acc, val) => acc.concat(val), []) ?? client.guilds.cache.map((g) => ({ name: g.name, id: g.id }));This keeps the command usable in dev / test environments.
src/commands/info/About.ts (1)
39-92
: LGTM – cosmetic re-indent onlyNo behavioural changes; button URLs, embed fields and translations remain intact.
src/commands/music/Autoplay.ts (1)
38-66
: LGTM – purely stylistic diff.No functional or behavioural changes introduced. Toggle logic, localisation keys, and embed construction remain intact.
src/commands/music/Skipto.ts (1)
58-72
: Confirmplayer.skip(num)
expects a 1-based index – possible off-by-one.The validation allows
num === queue.length
, implying a 1-based index.
If the underlying implementation is zero-based, users will skip past
queue bounds and drop the currently playing track.Please verify the contract of
player.skip()
.
If it is zero-based, subtract1
before calling or tighten validation.src/commands/info/Ping.ts (1)
40-75
: LGTM – no semantic change.Deferred reply, latency calculation, embed fields and localisation remain unchanged.
src/LavaClient.ts (1)
14-24
: Intents / allowed mentions block looks goodNo behavioural changes—just re-indentation. Values and order remain intact.
src/config.ts (1)
1-44
: Config re-indentation only – looks goodNo semantic changes; consistent tab style aligns with the new Biome rules.
src/commands/config/Dj.ts (1)
4-219
: LGTM! Consistent formatting appliedThe formatting changes successfully convert indentation from spaces to tabs throughout the file, aligning with the Biome.js configuration introduced in this PR. No functional changes were made to the DJ command logic.
src/commands/music/Lyrics.ts (1)
2-308
: LGTM! Formatting standardization appliedThe indentation has been consistently converted from spaces to tabs throughout the entire file, including the constructor,
cleanLyrics
,run
, andpaginateLyrics
methods. All lyrics functionality and pagination logic remain unchanged.src/commands/filters/8d.ts (1)
2-71
: LGTM! Consistent tab formatting appliedThe file has been successfully reformatted to use tabs instead of spaces, maintaining all 8D filter functionality while aligning with the new Biome.js code style standards.
src/commands/info/LavaLink.ts (1)
4-90
: LGTM! Formatting consistency maintainedThe indentation has been converted from spaces to tabs throughout the file, including the constructor and run method. All Lavalink node status display and pagination functionality remains intact.
src/events/client/GuildCreate.ts (1)
2-82
: LGTM! Event handler formatting standardizedThe file has been reformatted to use consistent tab indentation while preserving all guild creation event handling logic, including owner fetching, embed creation, and error handling for log channel operations.
src/commands/config/247.ts (1)
64-78
: Guard against possible undefined channel and regionI wasn’t able to locate the
Context
orVoiceChannel
type definitions to confirm whetherctx.channel
ormember.voice.channel.rtcRegion
can benull
/undefined
. Please verify their types. If they are indeed optional, updatesrc/commands/config/247.ts
(around lines 64–78) as follows:• Add a runtime check for
ctx.channel
before using its.id
• Replace the forced non-null assertion onrtcRegion
with a nullish coalescing fallback- await client.db.set_247(ctx.guild.id, ctx.channel!.id, member.voice.channel.id); + if (!ctx.channel) { + return await ctx.sendMessage({ + embeds: [ + embed + .setDescription(ctx.locale("errors.channel_not_found")) + .setColor(client.color.red), + ], + }); + } + await client.db.set_247( + ctx.guild.id, + ctx.channel.id, + member.voice.channel.id, + ); … - vcRegion: member.voice.channel.rtcRegion!, + vcRegion: member.voice.channel.rtcRegion ?? undefined,This will prevent runtime crashes if either value is missing.
src/commands/filters/Reset.ts (1)
42-58
: No functional changes – looks goodOnly indentation changed; logic for clearing filters/EQ is intact and robust.
src/commands/music/Shuffle.ts (1)
38-74
: LGTM – shuffle logic untouched and well-guardedAll functional checks (player exists, queue length, FairPlay lock) remain intact after re-formatting.
src/commands/info/Botinfo.ts (1)
7-100
: Only whitespace/indentation touched – implementation unchanged.No behavioural changes detected; logic, types, and error handling remain intact.
LGTM.src/commands/music/Join.ts (1)
5-87
: Whitespace-only diff – nothing to review.File was re-indented; execution flow and side-effects stay the same.
Good to go.src/commands/filters/Karaoke.ts (1)
2-71
: Pure re-formatting; code behaviour identical.No action required.
locales/Dutch.json (1)
2-648
: JSON re-formatted for readability – content intact.No localisation keys or values changed.
Looks fine.locales/Thai.json (1)
2-662
: Indentation update only.Semantic content untouched; approve.
src/commands/filters/Rotation.ts (1)
42-69
: LGTM – purely stylistic changesNo functional changes were introduced; logic remains sound and the indentation now matches the new Biome configuration.
src/commands/info/Help.ts (1)
50-57
: Null-safety forguild
record fetched from DB
const guild = await client.db.get(ctx.guild.id);
If the record is missing, subsequentguild.prefix
dereferences will throw. Consider a fallback to the default prefix to keep the help command functional for first-time guilds.-const guild = await client.db.get(ctx.guild.id); +const guild = (await client.db.get(ctx.guild.id)) ?? { prefix: this.client.config.prefix };Please verify that
this.client.config.prefix
(or equivalent) exists in your config object.src/commands/dev/Eval.ts (1)
53-62
: Verify Hastebin endpoint –/post
is no longer standardMost hastebin instances expect
POST /documents
. The current call tohttps://hasteb.in/post
may 404, resulting in an unhandled fetch error.- const response = await fetch("https://hasteb.in/post", { + const response = await fetch("https://hasteb.in/documents", {pnpm-workspace.yaml (1)
1-2
: Verify that@prisma/client
really must be ignored
ignoredBuiltDependencies
preventspnpm
from re-building@prisma/client
for every workspace.
If any package in the monorepo relies on runtime-generated Prisma types (created viaprisma generate
), skipping the build may cause missing or stale type definitions at runtime.Double-check that:
- No package imports code from
node_modules/.prisma
- CI still executes
prisma generate
at least once before packagingsrc/commands/playlist/Create.ts (1)
4-43
: Formatting-only change – looks goodIndentation switch to tabs complies with the new Biome configuration. No functional impact.
src/commands/config/Language.ts (1)
7-58
: Formatting-only change – looks goodStylistic changes only; logic untouched.
src/commands/music/Queue.ts (1)
4-36
: Formatting-only changes – no issues foundThis hunk is strictly indentation/whitespace. Behaviour, metadata and typings are untouched.
Looks good.src/commands/playlist/AddSong.ts (1)
5-53
: Formatting-only changes – no issues foundIndentation converted to tabs, no semantic impact.
src/commands/playlist/Delete.ts (1)
5-45
: Formatting-only changes – no issues foundTabs introduced, logic untouched.
src/commands/playlist/RemoveSong.ts (1)
5-51
: Formatting-only changes – no issues foundNo semantic differences observed.
locales/Italian.json (1)
1-649
: JSON re-indentation looks goodOnly tab indentation changed; keys/values preserved.
Nothing else to note.package.json (2)
3-3
: Downgrade may violate sem-ver expectations on the public npm registryPublishing
4.7.0
after a previously-published5.0.0-beta
is technically allowed, but many consumers will already be on a higher major via the pre-release tag. This unexpected “rollback” can lead to:• Install/lock-file churn in CI pipelines
• Confusingnpm outdated
output
• Potentially breaking^5.0.0-beta
ranges if they get replaced bylatest
pointing to 4.xDouble-check what was published to npm and consider either continuing the
5.0.0-beta → 5.0.0
line or yanking the beta before releasing 4.7.0.
46-58
:undici
is bundled with Node 18+; pinning a newer minor may shadow the builtinIf the codebase only relies on the global
fetch()
/undici
shipped with Node 18/20, an explicit dependency is unnecessary and can bring subtle version mismatches. Consider removing it unless you really need^7.13.0
features.
"files": { | ||
"ignoreUnknown": false, | ||
"includes": [ | ||
"**/src/**", | ||
"!/node_modules/", | ||
"!/dist", | ||
"!/package.json", | ||
"!/tsconfig.json", | ||
"!/.vscode" | ||
] | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Path patterns risk matching node_modules
& build output.
Because useIgnoreFile
is false
, the ignore list is your only guard.
The leading slash in "/node_modules/"
, "/dist"
etc. is treated literally by Biome globs and will not match nested paths (e.g. packages/foo/node_modules
).
Use **/node_modules/**
, **/dist/**
or re-enable .gitignore
handling.
- "!/node_modules/",
- "!/dist",
+ "!**/node_modules/**",
+ "!**/dist/**",
🤖 Prompt for AI Agents
In biome.json around lines 8 to 18, the ignore patterns for node_modules and
dist use leading slashes which do not match nested directories. Replace
"/node_modules/" and "/dist" with "**/node_modules/**" and "**/dist/**"
respectively to correctly ignore all nested instances. Alternatively, re-enable
useIgnoreFile to leverage .gitignore handling for these paths.
}, | ||
"movenode": { | ||
"description": "Move the players to a different Lavalink node", | ||
"options": { | ||
"node": "The node ID to move the players" | ||
} | ||
}, | ||
"fairplay": { | ||
"description": "Set the bot to play music fairly" | ||
}, | ||
"playlocal": { | ||
"description": "Plays an your local audio files", | ||
"options": { | ||
"file": "The audio file you want to play" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
नई कमांड के लिए अनुवाद ग़ायब हैं
movenode
, fairplay
, playlocal
की descriptions/option अंग्रेज़ी में हैं। कृपया इन्हें हिन्दी में अनूदित करें।
-"description": "Move the players to a different Lavalink node"
+"description": "प्लेयरों को किसी अन्य Lavalink नोड पर स्थानांतरित करें"
📝 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.
}, | |
"movenode": { | |
"description": "Move the players to a different Lavalink node", | |
"options": { | |
"node": "The node ID to move the players" | |
} | |
}, | |
"fairplay": { | |
"description": "Set the bot to play music fairly" | |
}, | |
"playlocal": { | |
"description": "Plays an your local audio files", | |
"options": { | |
"file": "The audio file you want to play" | |
} | |
} | |
}, | |
"movenode": { | |
- "description": "Move the players to a different Lavalink node", | |
+ "description": "प्लेयरों को किसी अन्य Lavalink नोड पर स्थानांतरित करें", | |
"options": { | |
"node": "The node ID to move the players" | |
} | |
}, | |
"fairplay": { | |
"description": "Set the bot to play music fairly" | |
}, | |
"playlocal": { | |
"description": "Plays an your local audio files", | |
"options": { | |
"file": "The audio file you want to play" | |
} | |
} |
🤖 Prompt for AI Agents
In locales/Hindi.json around lines 503 to 518, the descriptions and options for
the new commands 'movenode', 'fairplay', and 'playlocal' are still in English.
Translate these strings into Hindi to maintain consistency and localization
quality.
}, | ||
"movenode": { | ||
"description": "Move the players to a different Lavalink node", | ||
"options": { | ||
"node": "The node ID to move the players" | ||
} | ||
}, | ||
"fairplay": { | ||
"description": "Set the bot to play music fairly" | ||
}, | ||
"playlocal": { | ||
"description": "Plays an your local audio files", | ||
"options": { | ||
"file": "The audio file you want to play" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
未翻訳の追加コマンド
movenode
/ fairplay
/ playlocal
の説明とオプションが英語のままです。日本語訳を追加してください。
例:
-"description": "Move the players to a different Lavalink node"
+"description": "プレイヤーを別の Lavalink ノードへ移動します"
📝 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.
}, | |
"movenode": { | |
"description": "Move the players to a different Lavalink node", | |
"options": { | |
"node": "The node ID to move the players" | |
} | |
}, | |
"fairplay": { | |
"description": "Set the bot to play music fairly" | |
}, | |
"playlocal": { | |
"description": "Plays an your local audio files", | |
"options": { | |
"file": "The audio file you want to play" | |
} | |
} | |
}, | |
"movenode": { | |
"description": "プレイヤーを別の Lavalink ノードへ移動します", | |
"options": { | |
"node": "The node ID to move the players" | |
} | |
}, | |
"fairplay": { | |
"description": "Set the bot to play music fairly" | |
}, | |
"playlocal": { | |
"description": "Plays an your local audio files", | |
"options": { | |
"file": "The audio file you want to play" | |
} | |
} |
🤖 Prompt for AI Agents
In locales/Japanese.json around lines 503 to 518, the descriptions and options
for the commands "movenode", "fairplay", and "playlocal" are still in English.
Translate these strings into Japanese, ensuring the descriptions and option
labels are accurately localized to maintain consistency with the rest of the
file.
}, | ||
"movenode": { | ||
"description": "Move the players to a different Lavalink node", | ||
"options": { | ||
"node": "The node ID to move the players" | ||
} | ||
}, | ||
"fairplay": { | ||
"description": "Set the bot to play music fairly" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Untranslated strings – new command still in English
movenode
/ fairplay
entries are added but the description and option label are left in English.
This breaks the locale’s UX consistency and will surface mixed-language UI.
-"description": "Move the players to a different Lavalink node",
-"node": "The node ID to move the players"
+"description": "플레이어를 다른 Lavalink 노드로 이동해요",
+"node": "플레이어를 이동할 노드 ID"
Please provide proper Korean translations (same applies to fairplay
).
📝 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.
}, | |
"movenode": { | |
"description": "Move the players to a different Lavalink node", | |
"options": { | |
"node": "The node ID to move the players" | |
} | |
}, | |
"fairplay": { | |
"description": "Set the bot to play music fairly" | |
} | |
}, | |
"movenode": { | |
"description": "플레이어를 다른 Lavalink 노드로 이동해요", | |
"options": { | |
"node": "플레이어를 이동할 노드 ID" | |
} | |
}, | |
"fairplay": { | |
"description": "Set the bot to play music fairly" | |
} |
🤖 Prompt for AI Agents
In locales/Korean.json around lines 527 to 536, the new command entries
"movenode" and "fairplay" have descriptions and option labels still in English,
which breaks the Korean locale consistency. Replace the English text with
accurate Korean translations for both the "description" and the "options.node"
fields to maintain a consistent user experience in the Korean locale.
}, | ||
"movenode": { | ||
"description": "Move the players to a different Lavalink node", | ||
"options": { | ||
"node": "The node ID to move the players" | ||
} | ||
}, | ||
"fairplay": { | ||
"description": "Set the bot to play music fairly" | ||
}, | ||
"playlocal": { | ||
"description": "Plays an your local audio files", | ||
"options": { | ||
"file": "The audio file you want to play" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Новые команды не переведены
Для movenode
, fairplay
, playlocal
строки остались на английском; это нарушает целостность локализации.
-"description": "Move the players to a different Lavalink node"
+"description": "Переместить плееры на другой Lavalink-узел"
Аналогично переведите option-label "node"
и остальные новые строки.
📝 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.
}, | |
"movenode": { | |
"description": "Move the players to a different Lavalink node", | |
"options": { | |
"node": "The node ID to move the players" | |
} | |
}, | |
"fairplay": { | |
"description": "Set the bot to play music fairly" | |
}, | |
"playlocal": { | |
"description": "Plays an your local audio files", | |
"options": { | |
"file": "The audio file you want to play" | |
} | |
} | |
"movenode": { | |
"description": "Переместить плееры на другой Lavalink-узел", | |
"options": { | |
"node": "The node ID to move the players" | |
} | |
}, |
🤖 Prompt for AI Agents
In locales/Russian.json between lines 503 and 518, the new command descriptions
and option labels for "movenode", "fairplay", and "playlocal" are still in
English, breaking localization consistency. Translate all these strings into
Russian, including the "description" fields and the "node" and "file" option
labels, to maintain full Russian localization.
public async run( | ||
client: Lavamusic, | ||
ctx: Context, | ||
args: string[], | ||
): Promise<any> { | ||
const player = client.manager.getPlayer(ctx.guild.id); | ||
const current = player?.queue.current?.info; | ||
const embed = this.client.embed(); | ||
const duration = client.utils.parseTime(args.join(" ")); | ||
if (!duration) { | ||
return await ctx.sendMessage({ | ||
embeds: [ | ||
embed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slash-command path ignores the duration
option – /seek
currently breaks
args
is populated for prefix commands, but for slash commands ctx.options
is the source of truth.
At the moment /seek 1m
results in duration === 0
because args.join(" ")
is empty.
- const duration = client.utils.parseTime(args.join(" "));
+ const durationInput =
+ (args.length ? args.join(" ") : ctx.options?.getString("duration")) ??
+ "";
+ const duration = client.utils.parseTime(durationInput);
Without this fix the new Biome formatting masks a functional bug.
📝 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.
public async run( | |
client: Lavamusic, | |
ctx: Context, | |
args: string[], | |
): Promise<any> { | |
const player = client.manager.getPlayer(ctx.guild.id); | |
const current = player?.queue.current?.info; | |
const embed = this.client.embed(); | |
const duration = client.utils.parseTime(args.join(" ")); | |
if (!duration) { | |
return await ctx.sendMessage({ | |
embeds: [ | |
embed | |
public async run( | |
client: Lavamusic, | |
ctx: Context, | |
args: string[], | |
): Promise<any> { | |
const player = client.manager.getPlayer(ctx.guild.id); | |
const current = player?.queue.current?.info; | |
const embed = this.client.embed(); | |
const durationInput = | |
(args.length ? args.join(" ") : ctx.options?.getString("duration")) ?? | |
""; | |
const duration = client.utils.parseTime(durationInput); | |
if (!duration) { | |
return await ctx.sendMessage({ | |
embeds: [ | |
embed |
🤖 Prompt for AI Agents
In src/commands/music/Seek.ts around lines 45 to 57, the code uses args.join("
") to get the duration, which works for prefix commands but fails for slash
commands where ctx.options holds the input. To fix this, modify the code to
check if ctx.options.duration exists and use that value for duration parsing
instead of args.join(" "). This ensures the slash-command correctly receives and
processes the duration input.
const nodes = client.manager.nodeManager.leastUsedNodes(); | ||
const node = nodes[Math.floor(Math.random() * nodes.length)]; | ||
const tracks = await node.decode.multipleTracks(songs as any, ctx.author); | ||
if (tracks.length === 0) { | ||
return await ctx.sendMessage({ | ||
embeds: [ | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible crash when no Lavalink nodes are available
const nodes = client.manager.nodeManager.leastUsedNodes();
const node = nodes[Math.floor(Math.random() * nodes.length)];
If nodes.length === 0
(all nodes down, mis-configured, etc.), nodes.length
is 0
and Math.random()*0
returns NaN
, so nodes[NaN]
⇒ undefined
⇒ throws on the next line.
Guard this path or surface a graceful error to the user.
🤖 Prompt for AI Agents
In src/commands/playlist/Load.ts around lines 97 to 103, the code selects a
random Lavalink node from the least used nodes without checking if any nodes are
available. If nodes.length is 0, this causes an undefined node and a crash. Add
a guard to check if nodes array is empty before selecting a node, and if so,
send a clear error message to the user indicating no Lavalink nodes are
available.
private prisma: PrismaClient; | ||
|
||
constructor() { | ||
this.prisma = new PrismaClient(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Multiple PrismaClient instances – move to a shared singleton
Every new ServerData()
creates a fresh PrismaClient
, which quickly exhausts database connection limits in long-running bots. Hoist a single PrismaClient
(or use globalThis
) and reuse it.
- private prisma: PrismaClient;
-
- constructor() {
- this.prisma = new PrismaClient();
- }
+ private static prisma = new PrismaClient();
+
+ constructor() {
+ // reuse the singleton
+ this.prisma = ServerData.prisma;
+ }
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/database/server.ts around lines 14 to 18, the constructor creates a new
PrismaClient instance each time, which can exhaust database connections.
Refactor to use a shared singleton PrismaClient instance by hoisting the
PrismaClient outside the class or attaching it to globalThis, then reuse this
single instance in the ServerData class instead of creating a new one every
time.
Introduces live lyrics subscription and unsubscription buttons to the lyrics command, allowing users to receive real-time lyric updates. Updates localization strings for new button labels and error messages, refactors lyrics fetching logic to support both current and searched tracks, and improves interaction handling for lyrics navigation and live updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/commands/music/Lyrics.ts (3)
66-68
: Avoid empty catch blocksEmpty catch blocks can hide unexpected errors. Consider logging the error or adding a more specific check instead of catching all exceptions.
- } catch (e) { - // No options - ignore - } + } catch (e) { + // Expected when not using slash commands + if (client.logger?.debug) { + client.logger.debug("No slash command options available"); + } + }
348-348
: Use proper async delay utilityCreating promises with setTimeout can be improved for better readability and potential cleanup.
- await new Promise((res) => setTimeout(res, 1000)); + await new Promise<void>((resolve) => setTimeout(resolve, 1000));
414-416
: Empty catch block in interaction loopThe error from the interaction timeout is silently ignored. Consider logging timeout events for debugging.
} catch (e) { collectorActive = false; + if (client.logger?.debug) { + client.logger.debug("Lyrics interaction timed out"); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
locales/EnglishUS.json
(2 hunks)src/commands/music/Lyrics.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- locales/EnglishUS.json
⏰ 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: docker_publish
🔇 Additional comments (4)
src/commands/music/Lyrics.ts (4)
1-13
: Imports are properly structured for the new lyrics functionalityThe new imports for Discord.js v2 components and Lavalink client types are appropriate for the refactored implementation.
14-56
: Constructor properly configured with new song search optionThe addition of the optional "song" slash command parameter is well-implemented, allowing users to search for lyrics of any song while maintaining backward compatibility.
475-518
: Helper methods are well-implementedBoth
paginateLyrics
andcleanLyrics
methods are properly implemented with good edge case handling and appropriate string manipulation.
314-320
:⚠️ Verify LyricsLine timestamp property namesI wasn’t able to locate the
LyricsLine
definition in the repository or in thelavalink-client
types. Please double-check which property(ies) the API actually exposes for timestamps—whether it’sstartTime
,time
,timestamp
, or something else—to avoid accessing an undefined field at runtime.• Search your type definitions (or the
lavalink-client
package) for theLyricsLine
interface/alias.
• Confirm the correct timestamp property and update this lookup accordingly.
• If multiple properties remain valid, add a clear type guard or fallback to handle missing values.
Updated the Lyrics command to allow fetching lyrics without an active player by querying the node directly when no player exists. Adjusted the control flow to handle both cases (with and without player) and fixed the logic for handling song queries and error responses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/commands/music/Lyrics.ts (5)
88-88
: Remove outdated comment that contradicts the implementationThe comment states "according to the new logic" but this contradicts the actual implementation which correctly allows lyrics fetching without a player when a song query is provided.
The logic has been properly implemented to allow lyrics search without requiring an active player when a song query is provided, which addresses the previous concern about unnecessarily restrictive functionality.
96-100
: Remove unnecessary undefined parameterThe search method call includes an explicit
undefined
third parameter which is redundant since it's optional and defaults correctly.- const searchRes = await client.manager.search( - songQuery, - ctx.author, - undefined, - ); + const searchRes = await client.manager.search( + songQuery, + ctx.author, + );
129-133
: Remove unnecessary undefined parameter (duplicate occurrence)Same issue as the previous occurrence - the explicit
undefined
parameter should be removed for consistency.- const searchRes = await client.manager.search( - songQuery, - ctx.author, - undefined, - ); + const searchRes = await client.manager.search( + songQuery, + ctx.author, + );
330-330
: Fix unsafe type castingThe code uses unsafe
as any
casting which bypasses TypeScript's type safety. Since the type check was already performed earlier, use proper typing.- const lyricsLines = (lyricsResult as any).lines as LyricsLine[]; + const lyricsLines = (lyricsResult as LyricsResult).lines!;
377-377
: Fix unsafe type casting (second occurrence)Another instance of unsafe
as any
casting that should be replaced with proper typing.- const lyricsLines = (lyricsResult as any).lines as LyricsLine[]; + const lyricsLines = (lyricsResult as LyricsResult).lines!;
🧹 Nitpick comments (2)
src/commands/music/Lyrics.ts (2)
315-438
: Consider extracting live lyrics subscription logic into a separate methodThe while loop containing the live lyrics subscription logic is quite complex (120+ lines). This could be extracted into a separate private method to improve readability and maintainability.
Consider creating a method like
private async handleLyricsInteractions()
to encapsulate this logic and reduce the complexity of the mainrun
method.
369-369
: Consider making the update interval configurableThe hardcoded 1-second interval for lyrics updates might be too frequent or too slow depending on the use case.
Consider making this configurable or adjusting based on the lyrics timing precision:
- await new Promise((res) => setTimeout(res, 1000)); + await new Promise((res) => setTimeout(res, 500)); // More responsive updates
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/commands/music/Lyrics.ts
(1 hunks)
⏰ 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: docker_publish
🔇 Additional comments (3)
src/commands/music/Lyrics.ts (3)
15-55
: Constructor implementation looks good with proper slash command integrationThe constructor properly defines the new "song" option for slash commands and correctly sets the player requirements to allow lyrics fetching without an active player.
496-527
: Pagination logic is well implementedThe pagination method properly handles character limits and edge cases, providing a good user experience for long lyrics.
529-539
: Lyrics cleaning method is comprehensiveThe cleaning method properly removes metadata and normalizes whitespace, which will improve the display quality of lyrics.
Integrates Biome.js for consistent code formatting and linting.
Summary by CodeRabbit
New Features
Chores
Style