Add MiniMax music generation tool#359
Merged
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 44 minutes and 12 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds a complete music generation feature to a Telegram bot using MiniMax API. It includes model capability detection, database schema for per-chat overrides, a comprehensive tool service handling MiniMax integration and Telegram delivery, admin commands for configuration, startup initialization, and documentation. ChangesMusic Generation Tool Feature
Sequence DiagramsequenceDiagram
participant Admin as Admin User
participant Controller as GeneralLLMController
participant Settings as MusicGenerationToolSettingsService
participant Service as MusicGenerationToolService
participant MiniMax as MiniMax API
participant Telegram as Telegram Bot
Admin->>Controller: /选择音乐模型 (choose model)
Controller->>Service: LoadMusicGenerationModelOptionsAsync()
Service->>Service: GetMusicGenerationModels()
Service-->>Controller: List of available models
Controller->>Admin: Show model selection menu
Admin->>Controller: Select model by number
Controller->>Settings: SetGroupModelNameAsync(chatId, model)
Settings->>Settings: Update GroupSettings
Settings-->>Controller: Success
Controller->>Admin: Model set confirmation
Note over Admin,Telegram: Later, when model is invoked...
Admin->>Controller: Generate music via /generate_music
Controller->>Service: GenerateMusic(prompt, ...)
Service->>Settings: GetModelNameAsync(chatId)
Settings-->>Service: Group model or default
Service->>MiniMax: POST music_generation request
MiniMax-->>Service: Audio (URL or hex bytes)
Service->>Service: Save audio file
Service->>Telegram: Send audio to chat
Telegram-->>Service: MessageId
Service-->>Controller: MusicGenerationResult
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 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)
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 |
Contributor
PR Check ReportSummary
Test Results
Code Quality
Test Artifacts
LinksThis report is auto-generated by GitHub Actions |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
TelegramSearchBot/Controller/Help/HelpController.cs (1)
27-52: ⚡ Quick winAvoid duplicating command docs in hardcoded strings.
helpTextnow carries another growing command list in code, which will keep drifting fromDocs/Bot_Commands_User_Guide.md. Consider loading/templating help content from one source to reduce future inconsistencies.🤖 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 `@TelegramSearchBot/Controller/Help/HelpController.cs` around lines 27 - 52, The hardcoded helpText string in HelpController (variable helpText) duplicates documentation and will diverge from Docs/Bot_Commands_User_Guide.md; replace the inline literal with code that reads/loads the canonical help content (e.g., read the markdown file Docs/Bot_Commands_User_Guide.md or use a shared template service) at startup or on-demand and assign it to helpText, falling back to a small default message if the file is missing, and ensure any formatting/encoding is preserved when returning the help text.TelegramSearchBot.Test/Manage/EditLLMConfTest.cs (1)
119-126: ⚖️ Poor tradeoffConsider adding test coverage for music tool configuration flows.
The test setup correctly wires
MusicGenerationToolSettingsService, but no new test methods were added to verify the music-specific functionality introduced inEditLLMConfService:
- Music tool enable/disable commands
- Music tool status query
- Setting default music model via state machine
- Music model state handler (
HandleSettingMusicGenerationModelAsync)Adding test coverage similar to the existing tests (e.g.,
ExecuteAsync_SetMaxRetryCount) would help catch regressions in the music configuration flows.🤖 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 `@TelegramSearchBot.Test/Manage/EditLLMConfTest.cs` around lines 119 - 126, Add unit tests covering the music tool configuration flows introduced in EditLLMConfService: write tests that use the existing test harness (with MusicGenerationToolSettingsService already wired) to verify enable/disable commands for the music tool, the music tool status query, setting the default music model via the state machine, and the state handler HandleSettingMusicGenerationModelAsync; mirror the style of existing tests such as ExecuteAsync_SetMaxRetryCount to assert command handling, state transitions, and persisted settings changes in the MusicGenerationToolSettingsService and EditLLMConfService interactions.TelegramSearchBot/Controller/AI/LLM/GeneralLLMController.cs (1)
401-436: 💤 Low valueConsider refactoring duplicated model selection logic.
The music model selection methods (lines 401-436, 458-476, 485-490, 508-522, 528-533) are nearly identical to the image model selection methods (lines 364-399, 438-456, 478-483, 492-506, 524-526). The only differences are the service reference, Redis key prefix, and user-facing strings.
While the current implementation is correct and follows the established pattern, consolidating this logic into a generic helper (e.g.,
ModelSelectionHelper<TService, TOption>) would reduce maintenance burden and prevent divergence.Also applies to: 458-476, 485-490, 508-522, 528-533
🤖 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 `@TelegramSearchBot/Controller/AI/LLM/GeneralLLMController.cs` around lines 401 - 436, The music vs image model selection handlers duplicate the same flow (checking Redis key, parsing index/取消, validating range, calling SetGroupModelNameAsync, clearing key, logging and sending messages); factor this into a single reusable helper (e.g., ModelSelectionHelper.HandlePendingSelectionAsync) that accepts the Redis key generator (GetMusicGenerationModelSelectionKey / image equivalent), the service caller delegate (Func<long, string, Task<(string previous, string current)>> or an interface method wrapper to call SetGroupModelNameAsync on IMusicGenerationToolSettingsService/ IImageGenerationToolSettingsService), the localized prompt/result strings, and required dependencies (IDatabase from _connectionMultiplexer, SendMessageService, logger). Replace TryHandlePendingMusicGenerationModelSelectionAsync and the image methods with thin wrappers that construct the key, choose messages and service delegate, then call the generic handler; ensure the helper preserves existing behaviors (trim/parse/取消, range checks, key deletion, log message using telegramMessage.Chat.Id and MessageId).TelegramSearchBot.LLM/Service/AI/LLM/ModelCapabilityService.cs (1)
189-199: 💤 Low valueRedundant MiniMax branch — names already covered.
IsKnownMusicGenerationModelName(Line 193) already matchesmusic-2.6,music-2.6-free,music-cover, andmusic-cover-freecase-insensitively for any provider, so the MiniMax-specific check on Lines 194-198 is dead code. It mirrors the existing image record helper, so this is a non-blocking cleanup; safe to drop the branch (or keep it intentionally if you expect the known-name list and MiniMax list to diverge later).🤖 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 `@TelegramSearchBot.LLM/Service/AI/LLM/ModelCapabilityService.cs` around lines 189 - 199, The IsMusicGenerationModelRecord method contains a redundant MiniMax-specific branch: remove the trailing condition that checks model.LLMChannel?.Provider == LLMProvider.MiniMax and the explicit model.ModelName comparisons ("music-2.6", "music-2.6-free", "music-cover", "music-cover-free") because ModelWithCapabilities.IsKnownMusicGenerationModelName(model.ModelName) already covers those names case-insensitively for any provider; update IsMusicGenerationModelRecord to rely on the capability checks and IsKnownMusicGenerationModelName only, leaving the provider-specific branch out (or delete that clause) to eliminate dead code.
🤖 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 `@Docs/README_MCP.md`:
- Around line 76-90: The docs mix camelCase and snake_case for the
generate_music tool parameters which breaks real calls; pick the canonical
naming used by the tool schema and make all parameter names consistent (e.g., if
the tool expects snake_case, change lyricsOptimizer->lyrics_optimizer,
audioUrl->audio_url, audioBase64->audio_base64, isInstrumental->is_instrumental,
outputFormat->output_format, sampleRate->sample_rate, bitrate->bitrate,
format->format, aigcWatermark->aigc_watermark, coverFeatureId->cover_feature_id,
sendToChat->send_to_chat, replyToMessageId->reply_to_message_id,
timeoutSeconds->timeout_seconds, etc.), update the example/parameter table and
any descriptive text referencing these fields, and ensure the README_MCP.md
generate_music parameter list exactly matches the actual tool schema used by the
implementation.
In `@TelegramSearchBot/Service/Tools/MusicGenerationToolService.cs`:
- Around line 414-426: TryAcquireChannelAsync currently uses a non-atomic
check-then-increment on Redis (reading key via redisDb.StringGetAsync and then
StringIncrementAsync) which allows a race and can exceed channel.Parallel;
replace this with an atomic Lua script executed via ScriptEvaluateAsync that
takes GetSemaphoreKey(channel.Id) as KEYS[1] and channel.Parallel (use
Math.Max(1, channel.Parallel)) as ARGV[1], checks GET or 0, INCRs only when
current < limit and returns success/failure; update TryAcquireChannelAsync to
call ScriptEvaluateAsync, interpret the numeric result as bool, and keep
references to _connectionMultiplexer and GetSemaphoreKey to locate the code to
change.
---
Nitpick comments:
In `@TelegramSearchBot.LLM/Service/AI/LLM/ModelCapabilityService.cs`:
- Around line 189-199: The IsMusicGenerationModelRecord method contains a
redundant MiniMax-specific branch: remove the trailing condition that checks
model.LLMChannel?.Provider == LLMProvider.MiniMax and the explicit
model.ModelName comparisons ("music-2.6", "music-2.6-free", "music-cover",
"music-cover-free") because
ModelWithCapabilities.IsKnownMusicGenerationModelName(model.ModelName) already
covers those names case-insensitively for any provider; update
IsMusicGenerationModelRecord to rely on the capability checks and
IsKnownMusicGenerationModelName only, leaving the provider-specific branch out
(or delete that clause) to eliminate dead code.
In `@TelegramSearchBot.Test/Manage/EditLLMConfTest.cs`:
- Around line 119-126: Add unit tests covering the music tool configuration
flows introduced in EditLLMConfService: write tests that use the existing test
harness (with MusicGenerationToolSettingsService already wired) to verify
enable/disable commands for the music tool, the music tool status query, setting
the default music model via the state machine, and the state handler
HandleSettingMusicGenerationModelAsync; mirror the style of existing tests such
as ExecuteAsync_SetMaxRetryCount to assert command handling, state transitions,
and persisted settings changes in the MusicGenerationToolSettingsService and
EditLLMConfService interactions.
In `@TelegramSearchBot/Controller/AI/LLM/GeneralLLMController.cs`:
- Around line 401-436: The music vs image model selection handlers duplicate the
same flow (checking Redis key, parsing index/取消, validating range, calling
SetGroupModelNameAsync, clearing key, logging and sending messages); factor this
into a single reusable helper (e.g.,
ModelSelectionHelper.HandlePendingSelectionAsync) that accepts the Redis key
generator (GetMusicGenerationModelSelectionKey / image equivalent), the service
caller delegate (Func<long, string, Task<(string previous, string current)>> or
an interface method wrapper to call SetGroupModelNameAsync on
IMusicGenerationToolSettingsService/ IImageGenerationToolSettingsService), the
localized prompt/result strings, and required dependencies (IDatabase from
_connectionMultiplexer, SendMessageService, logger). Replace
TryHandlePendingMusicGenerationModelSelectionAsync and the image methods with
thin wrappers that construct the key, choose messages and service delegate, then
call the generic handler; ensure the helper preserves existing behaviors
(trim/parse/取消, range checks, key deletion, log message using
telegramMessage.Chat.Id and MessageId).
In `@TelegramSearchBot/Controller/Help/HelpController.cs`:
- Around line 27-52: The hardcoded helpText string in HelpController (variable
helpText) duplicates documentation and will diverge from
Docs/Bot_Commands_User_Guide.md; replace the inline literal with code that
reads/loads the canonical help content (e.g., read the markdown file
Docs/Bot_Commands_User_Guide.md or use a shared template service) at startup or
on-demand and assign it to helpText, falling back to a small default message if
the file is missing, and ensure any formatting/encoding is preserved when
returning the help text.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0179e40b-5366-4348-b26d-482d16b87f99
📒 Files selected for processing (23)
Docs/Bot_Commands_User_Guide.mdDocs/Existing_Codebase_Overview.mdDocs/README_MCP.mdDocs/README_ModelCapabilities.mdTelegramSearchBot.Common/Model/AI/ModelWithCapabilities.csTelegramSearchBot.Common/Model/Tools/MusicGenerationResult.csTelegramSearchBot.Database/Migrations/20260531090000_AddGroupMusicGenerationModel.Designer.csTelegramSearchBot.Database/Migrations/20260531090000_AddGroupMusicGenerationModel.csTelegramSearchBot.Database/Migrations/DataDbContextModelSnapshot.csTelegramSearchBot.Database/Model/Data/GroupSettings.csTelegramSearchBot.LLM.Test/Service/AI/LLM/ModelCapabilityServiceTests.csTelegramSearchBot.LLM.Test/Service/AI/LLM/ModelWithCapabilitiesTests.csTelegramSearchBot.LLM/Interface/IModelCapabilityService.csTelegramSearchBot.LLM/Service/AI/LLM/ModelCapabilityService.csTelegramSearchBot.Test/Manage/EditLLMConfTest.csTelegramSearchBot.Test/Service/Tools/MusicGenerationToolServiceTests.csTelegramSearchBot/AppBootstrap/GeneralBootstrap.csTelegramSearchBot/Controller/AI/LLM/GeneralLLMController.csTelegramSearchBot/Controller/Help/HelpController.csTelegramSearchBot/Model/AI/LLMConfState.csTelegramSearchBot/Service/Manage/EditLLMConfService.csTelegramSearchBot/Service/Tools/ImageGenerationToolService.csTelegramSearchBot/Service/Tools/MusicGenerationToolService.cs
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
generate_musictool backed by MiniMax/v1/music_generationValidation
MusicGenerationToolServiceTests|FullyQualifiedNameImageGenerationToolServiceTests|FullyQualifiedName~EditLLMConfTest"ModelCapabilityServiceTests|FullyQualifiedNameModelWithCapabilitiesTests"Summary by CodeRabbit