Add group agent chat mode#361
Open
ModerRAS wants to merge 2 commits into
Open
Conversation
|
Warning Review limit reached
More reviews will be available in 41 minutes and 14 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 (9)
📝 WalkthroughWalkthroughThis PR adds administrator-controlled Agent chat mode for group chats. Admins can enable guided (batched) or sequential message processing through LLM agents. The system persists settings in the database, routes incoming messages through a new controller, buffers messages to Redis with configurable batch windows, and dispatches batches via background worker. ChangesAgent Chat Mode System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
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: 3
🧹 Nitpick comments (6)
README.md (1)
183-185: ⚡ Quick winKeep README command variants aligned with parser aliases.
Consider listing
开启Agent引导聊天and查看Agent聊天as accepted aliases to match runtime behavior and reduce support friction.🤖 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 `@README.md` around lines 183 - 185, The README command list currently shows '开启Agent聊天', '开启Agent队列聊天', and '关闭Agent聊天' / 'Agent聊天状态' but does not include aliases used by the parser; update the README to include the runtime aliases '开启Agent引导聊天' and '查看Agent聊天' alongside the existing entries so the docs match the parser behavior (update the lines listing '开启Agent聊天', '开启Agent队列聊天', and '关闭Agent聊天' / 'Agent聊天状态' to include the additional accepted variants).TelegramSearchBot.LLM/Service/AI/LLM/GroupLlmSettingsService.cs (1)
134-140: ⚡ Quick winConsider extracting magic numbers to named constants.
The batch window clamping uses hardcoded values
1and60without explanation or constants.♻️ Proposed refactor to improve maintainability
+ private const int MinBatchWindowSeconds = 1; + private const int MaxBatchWindowSeconds = 60; + private static int NormalizeBatchWindow(int? batchWindowSeconds) { if (!batchWindowSeconds.HasValue || batchWindowSeconds.Value <= 0) { return GroupAgentChatSettings.DefaultBatchWindowSeconds; } - return Math.Clamp(batchWindowSeconds.Value, 1, 60); + return Math.Clamp(batchWindowSeconds.Value, MinBatchWindowSeconds, MaxBatchWindowSeconds); }🤖 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/GroupLlmSettingsService.cs` around lines 134 - 140, The NormalizeBatchWindow method uses magic numbers 1 and 60 for clamping; define private const int MinBatchWindowSeconds = 1 and private const int MaxBatchWindowSeconds = 60 (near the top of the class) and replace the literals in Math.Clamp and any other comparisons with these constants while keeping GroupAgentChatSettings.DefaultBatchWindowSeconds unchanged; update references in NormalizeBatchWindow to use MinBatchWindowSeconds and MaxBatchWindowSeconds to improve readability and maintainability.TelegramSearchBot/Service/AI/LLM/AgentChatBatchDispatchService.cs (1)
53-61: Dispatch processes due chats serially.
DispatchDueBatchesAsyncawaitsTryDispatchChatBatchAsyncper chat, and that call awaits the full LLM run. With many active groups, a single slow agent response stalls dispatch for all other due chats (up to 20 per cycle), and slow runs causePeriodicTimerticks to be skipped. Consider bounded-concurrency parallel dispatch (e.g.,Task.WhenAllover a degree-limited set) since each chat is independently locked.🤖 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/Service/AI/LLM/AgentChatBatchDispatchService.cs` around lines 53 - 61, DispatchDueBatchesAsync currently iterates dueChatIds and awaits TryDispatchChatBatchAsync serially, causing slow LLM runs to block other dispatches; change this to bounded-concurrency parallel dispatch (e.g., create tasks for each parsed chatId from dueChatIds and run them with a SemaphoreSlim or use Parallel.ForEachAsync with a max degree) so multiple TryDispatchChatBatchAsync(db, chatId, cancellationToken) calls run concurrently but limited (configurable max concurrency), still removing invalid IDs from LlmAgentRedisKeys.AgentChatBatchDueSet as before and honoring cancellationToken.TelegramSearchBot/Service/AI/LLM/AgentChatBatchQueueService.cs (1)
12-13: ConfirmTransientlifetime is intentional.This
IServiceis registered asTransient, but its only dependency (IConnectionMultiplexer) is singleton-safe. The repo convention registersIServiceimplementations asSingleton. Confirm the deviation is intentional.As per coding guidelines: "Use Scrutor DI scanning for IOnUpdate, IService, and IView implementations with [Injectable(ServiceLifetime.Singleton)] attribute".
🤖 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/Service/AI/LLM/AgentChatBatchQueueService.cs` around lines 12 - 13, AgentChatBatchQueueService is marked [Injectable(ServiceLifetime.Transient)] but project convention requires IService implementations to be singletons; either change the attribute to [Injectable(ServiceLifetime.Singleton)] on the AgentChatBatchQueueService class or, if transient behavior is deliberate, add a brief comment above the class explaining why Transient is required (and confirm no singleton-safety issues with any future dependencies), ensuring the class name AgentChatBatchQueueService and the IService registration follow the Scrutor DI scanning convention.TelegramSearchBot/Controller/AI/LLM/AgentChatModeController.cs (2)
282-297: ⚡ Quick winUse shared Redis key builders for pending-state checks (avoid future drift).
In
AgentChatModeController.HasPendingConfigurationStateAsync, the current literals match the key formats used elsewhere (modelselect:{chatId}:state, andimage_generation:model_select:{chatId}:{userId}/music_generation:model_select:{chatId}:{userId}), but this method still duplicates the string formats. Extract these into a shared canonical key builder/helper and reuse it here to prevent silent false-negatives if formats change 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/Controller/AI/LLM/AgentChatModeController.cs` around lines 282 - 297, HasPendingConfigurationStateAsync currently inlines Redis key strings which duplicates formats and risks future drift; replace the literal keys with calls to the shared Redis key builder helpers (e.g., use the existing canonical key methods such as RedisKeys.GetModelSelectStateKey(chatId), RedisKeys.GetImageModelSelectKey(chatId, userId) and RedisKeys.GetMusicModelSelectKey(chatId, userId) or the project's equivalent key-builder methods) inside HasPendingConfigurationStateAsync so it reads keys via those helpers, preserving chatId/userId parameters and keeping the same existence checks against db.StringGetAsync.
27-27: ⚡ Quick winFix DI registration concern for
AgentChatModeController(IOnUpdate).
AgentChatModeControllerdoesn’t need[Injectable]for DI wiring becauseTelegramSearchBot/Extension/ServiceCollectionExtension.csregisters allIOnUpdateimplementations via Scrutor (AddClasses(...AssignableTo<IOnUpdate>()).AsImplementedInterfaces()), so it will be picked up by that scan.The pending-state Redis keys in
HasPendingConfigurationStateAsyncare hardcoded, but the current strings match existing key formats used elsewhere (modelselect:{chatId}:stateinAdminService, andimage_generation:model_select:{chatId}:{userId}/music_generation:model_select:{chatId}:{userId}inGeneralLLMController); consider centralizing these key builders to avoid future drift.🤖 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/AgentChatModeController.cs` at line 27, Remove the unnecessary [Injectable] attribute from the AgentChatModeController class because the DI registration already scans and registers implementations of IOnUpdate; update the class declaration for AgentChatModeController to rely on the Scrutor-based registration instead of explicit attribute wiring. In HasPendingConfigurationStateAsync, replace the hard-coded Redis key strings ("modelselect:{chatId}:state", "image_generation:model_select:{chatId}:{userId}", "music_generation:model_select:{chatId}:{userId}") with a call to a centralized key-building helper (create a shared KeyBuilder or reuse an existing service) and use that helper inside HasPendingConfigurationStateAsync so all key formats are defined in one place and cannot drift. Ensure references to AgentChatModeController and HasPendingConfigurationStateAsync are updated to use the centralized key helper.
🤖 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/Bot_Commands_User_Guide.md`:
- Around line 66-75: Add the accepted command aliases to the Agent chat section:
update the listed command variants for Agent 聊天模式 to include `开启Agent引导聊天`
alongside `开启Agent聊天`, and add `查看Agent聊天` alongside `Agent聊天状态`; ensure the
same alias additions appear in the other occurrence referenced (the other Agent
chat entry). Keep the permission/requirement text unchanged and only append the
extra accepted command strings for discoverability.
In `@TelegramSearchBot/Service/AI/LLM/AgentChatBatchDispatchService.cs`:
- Around line 125-128: The deserialized AgentChatBufferedMessage might have a
null Message causing a NPE when accessing buffered.Message.Content; update the
conditional in AgentChatBatchDispatchService (where buffered is created from
JsonConvert.DeserializeObject<AgentChatBufferedMessage>) to also guard that
buffered.Message != null before checking Content (e.g., require buffered != null
&& buffered.Message != null &&
!string.IsNullOrWhiteSpace(buffered.Message.Content)) and only then call
result.Add(buffered).
- Around line 64-113: TryDispatchChatBatchAsync currently sets a Redis lock
(lockKey) with a lockValue but releases it unconditionally via
db.KeyDeleteAsync, which can delete another worker's lock if the TTL (LockTtl)
expired and ownership changed; replace the unconditional delete with an
ownership-checked compare-and-delete using a small Lua script that only removes
lockKey when its value equals lockValue, and ensure lock ownership is preserved
for the full dispatch by either increasing/parametrizing LockTtl to safely cover
ExecuteAsync or implementing periodic lock renewal (extend the key TTL while
awaiting execution) so the original owner never loses the lock before releasing
it with the compare-and-delete script.
---
Nitpick comments:
In `@README.md`:
- Around line 183-185: The README command list currently shows '开启Agent聊天',
'开启Agent队列聊天', and '关闭Agent聊天' / 'Agent聊天状态' but does not include aliases used
by the parser; update the README to include the runtime aliases '开启Agent引导聊天'
and '查看Agent聊天' alongside the existing entries so the docs match the parser
behavior (update the lines listing '开启Agent聊天', '开启Agent队列聊天', and '关闭Agent聊天' /
'Agent聊天状态' to include the additional accepted variants).
In `@TelegramSearchBot.LLM/Service/AI/LLM/GroupLlmSettingsService.cs`:
- Around line 134-140: The NormalizeBatchWindow method uses magic numbers 1 and
60 for clamping; define private const int MinBatchWindowSeconds = 1 and private
const int MaxBatchWindowSeconds = 60 (near the top of the class) and replace the
literals in Math.Clamp and any other comparisons with these constants while
keeping GroupAgentChatSettings.DefaultBatchWindowSeconds unchanged; update
references in NormalizeBatchWindow to use MinBatchWindowSeconds and
MaxBatchWindowSeconds to improve readability and maintainability.
In `@TelegramSearchBot/Controller/AI/LLM/AgentChatModeController.cs`:
- Around line 282-297: HasPendingConfigurationStateAsync currently inlines Redis
key strings which duplicates formats and risks future drift; replace the literal
keys with calls to the shared Redis key builder helpers (e.g., use the existing
canonical key methods such as RedisKeys.GetModelSelectStateKey(chatId),
RedisKeys.GetImageModelSelectKey(chatId, userId) and
RedisKeys.GetMusicModelSelectKey(chatId, userId) or the project's equivalent
key-builder methods) inside HasPendingConfigurationStateAsync so it reads keys
via those helpers, preserving chatId/userId parameters and keeping the same
existence checks against db.StringGetAsync.
- Line 27: Remove the unnecessary [Injectable] attribute from the
AgentChatModeController class because the DI registration already scans and
registers implementations of IOnUpdate; update the class declaration for
AgentChatModeController to rely on the Scrutor-based registration instead of
explicit attribute wiring. In HasPendingConfigurationStateAsync, replace the
hard-coded Redis key strings ("modelselect:{chatId}:state",
"image_generation:model_select:{chatId}:{userId}",
"music_generation:model_select:{chatId}:{userId}") with a call to a centralized
key-building helper (create a shared KeyBuilder or reuse an existing service)
and use that helper inside HasPendingConfigurationStateAsync so all key formats
are defined in one place and cannot drift. Ensure references to
AgentChatModeController and HasPendingConfigurationStateAsync are updated to use
the centralized key helper.
In `@TelegramSearchBot/Service/AI/LLM/AgentChatBatchDispatchService.cs`:
- Around line 53-61: DispatchDueBatchesAsync currently iterates dueChatIds and
awaits TryDispatchChatBatchAsync serially, causing slow LLM runs to block other
dispatches; change this to bounded-concurrency parallel dispatch (e.g., create
tasks for each parsed chatId from dueChatIds and run them with a SemaphoreSlim
or use Parallel.ForEachAsync with a max degree) so multiple
TryDispatchChatBatchAsync(db, chatId, cancellationToken) calls run concurrently
but limited (configurable max concurrency), still removing invalid IDs from
LlmAgentRedisKeys.AgentChatBatchDueSet as before and honoring cancellationToken.
In `@TelegramSearchBot/Service/AI/LLM/AgentChatBatchQueueService.cs`:
- Around line 12-13: AgentChatBatchQueueService is marked
[Injectable(ServiceLifetime.Transient)] but project convention requires IService
implementations to be singletons; either change the attribute to
[Injectable(ServiceLifetime.Singleton)] on the AgentChatBatchQueueService class
or, if transient behavior is deliberate, add a brief comment above the class
explaining why Transient is required (and confirm no singleton-safety issues
with any future dependencies), ensuring the class name
AgentChatBatchQueueService and the IService registration follow the Scrutor DI
scanning convention.
🪄 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: c37b22a0-d454-4bf0-b19e-ff19d9af6134
📒 Files selected for processing (19)
Docs/Bot_Commands_User_Guide.mdREADME.mdTelegramSearchBot.Common/Model/AI/LlmAgentContracts.csTelegramSearchBot.Database/Migrations/20260531153140_AddGroupAgentChatMode.Designer.csTelegramSearchBot.Database/Migrations/20260531153140_AddGroupAgentChatMode.csTelegramSearchBot.Database/Migrations/DataDbContextModelSnapshot.csTelegramSearchBot.Database/Model/Data/GroupSettings.csTelegramSearchBot.LLM.Test/Service/AI/LLM/GroupLlmSettingsServiceTests.csTelegramSearchBot.LLM/Interface/AI/LLM/IGroupLlmSettingsService.csTelegramSearchBot.LLM/Service/AI/LLM/GroupLlmSettingsService.csTelegramSearchBot.Test/Service/AI/LLM/LLMTaskQueueServiceTests.csTelegramSearchBot/Controller/AI/ASR/AutoASRController.csTelegramSearchBot/Controller/AI/LLM/AgentChatModeController.csTelegramSearchBot/Extension/ServiceCollectionExtension.csTelegramSearchBot/Model/AI/AgentChatModels.csTelegramSearchBot/Service/AI/LLM/AgentChatBatchDispatchService.csTelegramSearchBot/Service/AI/LLM/AgentChatBatchQueueService.csTelegramSearchBot/Service/AI/LLM/AgentChatExecutionService.csTelegramSearchBot/Service/AI/LLM/LLMTaskQueueService.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
Commands
开启Agent聊天/开启Agent引导聊天: enable guided short-window batching.开启Agent队列聊天: enable sequential per-message queueing.关闭Agent聊天/Agent聊天状态: disable or inspect current group mode.Validation
dotnet test TelegramSearchBot.LLM.Test\TelegramSearchBot.LLM.Test.csproj --filter "GroupLlmSettingsServiceTests"dotnet test TelegramSearchBot.Test\TelegramSearchBot.Test.csproj --filter "LLMTaskQueueServiceTests"dotnet build TelegramSearchBot.sln --configuration Debug --no-restoreNotes
Summary by CodeRabbit
New Features
Documentation