Add LLM invisibility for group users#363
Merged
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 11 minutes and 21 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 (12)
📝 WalkthroughWalkthroughThis PR introduces a per-user/group "LLM invisibility" feature allowing users to suppress LLM processing and filtering. A new ChangesLLM Invisibility System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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: 6
🧹 Nitpick comments (3)
TelegramSearchBot.LLM/Service/AI/LLM/LlmVisibilityService.cs (2)
64-77: 💤 Low valueRemove redundant
Update()call on tracked entity.Line 76 calls
Update()onuserWithGroupretrieved viaFirstOrDefaultAsync, which returns a tracked entity. EF Core automatically detects changes to tracked entities, so theUpdate()call is redundant (though harmless). Simply modifying the property on Line 75 is sufficient.♻️ Proposed refactor
} else { userWithGroup.IsLlmInvisible = isInvisible; - _dbContext.UsersWithGroup.Update(userWithGroup); }🤖 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/LlmVisibilityService.cs` around lines 64 - 77, The code is calling _dbContext.UsersWithGroup.Update(userWithGroup) after retrieving userWithGroup via FirstOrDefaultAsync (a tracked entity); remove the redundant Update() call and rely on EF Core change tracking by simply setting userWithGroup.IsLlmInvisible = isInvisible (keep the AddAsync branch as-is for new entities) and ensure SaveChangesAsync is called later to persist the change.
33-42: ⚖️ Poor tradeoffConsider adding a composite index for invisibility queries.
GetInvisibleUserIdsAsyncfilters byGroupIdandIsLlmInvisible. The current unique index on(UserId, GroupId)(line 741-742 in the snapshot) doesn't cover queries filtering by(GroupId, IsLlmInvisible). If invisibility checks become frequent or groups grow large, a composite index on(GroupId, IsLlmInvisible)would improve query performance.🤖 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/LlmVisibilityService.cs` around lines 33 - 42, GetInvisibleUserIdsAsync performs queries on UsersWithGroup filtering by GroupId and IsLlmInvisible; add a composite index on (GroupId, IsLlmInvisible) to speed these lookups and avoid full scans for large groups. Update the EF Core model/DbContext configuration (where the UsersWithGroup entity is configured) to include HasIndex(e => new { e.GroupId, e.IsLlmInvisible }) (make it non-unique) so the database will create the composite index; then add/emit a migration and apply it to ensure the index exists in the database. Ensure the index name is descriptive and include the same property names (GroupId, IsLlmInvisible) so it directly supports GetInvisibleUserIdsAsync.TelegramSearchBot/Controller/AI/LLM/GeneralLLMController.cs (1)
248-253: ⚡ Quick winNotice is sent on every mention/reply from an invisible user.
Each time an invisible user mentions or replies to the bot, a public message is posted to the group. This is noisy and repeatedly advertises the user's invisibility state to the whole chat. Consider rate-limiting this notice (the codebase already has a
SendRateLimitedWarningAsyncRedis-TTL pattern inAgentChatModeController) or suppressing it after the first hit.🤖 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 248 - 253, The current invisibility notice is sent every time an invisible user triggers LLM checks in GeneralLLMController (the block using _llmVisibilityService.IsUserInvisibleAsync and SendMessageService.SendMessage), which is noisy; change it to rate-limit or suppress repeated notices by reusing the existing rate-limit pattern (e.g., AgentChatModeController.SendRateLimitedWarningAsync or a Redis-TTL key per chat+user). Specifically, before calling SendMessageService.SendMessage, check/create a TTL key (unique by telegramMessage.Chat.Id + fromUserId + "llm_invisible") and only send the notice if the key is missing, then set the TTL (e.g., minutes) so subsequent hits skip the send; if your codebase exposes SendRateLimitedWarningAsync, call that helper instead of direct SendMessageService.SendMessage to enforce the same behavior.
🤖 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 `@TelegramSearchBot.LLM/Service/AI/LLM/GeminiService.cs`:
- Around line 126-133: The cached ChatSession in _chatSessions[chatId] can
retain messages for users who later become invisible; modify ExecAsync (the code
that retrieves/reuses _chatSessions[ChatId]) to detect visibility changes and
evict/rebuild the session: before reusing a cached ChatSession, call
_llmVisibilityService.IsUserInvisibleAsync for the participants or run
FilterVisibleMessagesAsync against the session's stored messages and if any
messages would be filtered out, remove _chatSessions[chatId] (or create a fresh
ChatSession) so the filtered history is not reused; ensure you reference
_chatSessions, ExecAsync, ChatId, _llmVisibilityService.IsUserInvisibleAsync and
FilterVisibleMessagesAsync when implementing the eviction logic.
In `@TelegramSearchBot.LLM/Service/AI/LLM/LlmVisibilityService.cs`:
- Line 13: The LlmVisibilityService is registered as
[Injectable(ServiceLifetime.Transient)] but depends on DataDbContext (a scoped
DbContext), so change the registration to use ServiceLifetime.Scoped to match
the DbContext lifetime; update the attribute on the LlmVisibilityService class
from ServiceLifetime.Transient to ServiceLifetime.Scoped so the service and its
DataDbContext dependency share the same request scope.
In `@TelegramSearchBot/Controller/Manage/LlmVisibilityController.cs`:
- Around line 31-34: The code extracts message text into the local variable
command and then compares it to exact strings; update the normalization where
command is set so slash commands with a bot suffix match: take the first token
of (message.Text ?? message.Caption ?? string.Empty).Trim() (i.e., up to the
first space) and, if that token contains '@', truncate everything from the '@'
onward, then use that normalized command for the existing comparisons (e.g.,
against "/llm_invisible_on", "/llm_invisible_off", "/llm_invisible_status") so
commands like "/llm_invisible_on@YourBot" will match.
In `@TelegramSearchBot/Service/Manage/RefreshService.cs`:
- Around line 330-333: The loop in RefreshService.cs skips invisible messages
via _llmVisibilityService.IsUserInvisibleAsync(chatId, message.FromUserId) and
uses continue, but it doesn't update the processed counter, causing stalled
progress; update the code so the processed-count variable (e.g.,
processed/processedCount/processedFiles used in this method) is incremented for
invisible items before the continue, or better, move the increment into a common
tail/finally block that runs for every iteration; apply the same fix to the
other early-continue path referenced around lines 347-350 so all skipped items
are counted as processed.
In `@TelegramSearchBot/Service/Tools/SearchToolService.cs`:
- Around line 29-37: Make LlmVisibilityService mandatory by removing the default
null from the SearchToolService constructor parameter (llmVisibilityService =
null → LlmVisibilityService llmVisibilityService), add an ArgumentNullException
check in the constructor to enforce non-null, and remove the empty-set fallback
logic that currently silently disables filtering (the code block around lines
325-328 that returns an empty set when _llmVisibilityService is null). Ensure
all usages of _llmVisibilityService assume it is present and rely on it to
perform LLM visibility filtering rather than skipping or returning an empty
visibility set.
- Around line 337-350: The code currently returns visibleDtos.Count as the
total, which underreports when you only fetched a window; update the return to
compute a correct totalVisibleFound: call _luceneManager.Search and build
visibleDtos as now, then when deciding the return value use this rule — if you
fetched the entire result set (searchResult.Item2.Count >= searchResult.Item1)
set totalVisibleFound = visibleDtos.Count; otherwise set totalVisibleFound =
searchResult.Item1 - invisibleHitsInFetchedSet (where invisibleHitsInFetchedSet
= searchResult.Item2.Count(m => invisibleUserIds.Contains(m.FromUserId))) so the
total reflects Lucene's total hits minus known invisible hits instead of the
current window size; keep the rest of the paging logic and return
(totalVisibleFound, visibleDtos.Skip(skip).Take(take).ToList()).
---
Nitpick comments:
In `@TelegramSearchBot.LLM/Service/AI/LLM/LlmVisibilityService.cs`:
- Around line 64-77: The code is calling
_dbContext.UsersWithGroup.Update(userWithGroup) after retrieving userWithGroup
via FirstOrDefaultAsync (a tracked entity); remove the redundant Update() call
and rely on EF Core change tracking by simply setting
userWithGroup.IsLlmInvisible = isInvisible (keep the AddAsync branch as-is for
new entities) and ensure SaveChangesAsync is called later to persist the change.
- Around line 33-42: GetInvisibleUserIdsAsync performs queries on UsersWithGroup
filtering by GroupId and IsLlmInvisible; add a composite index on (GroupId,
IsLlmInvisible) to speed these lookups and avoid full scans for large groups.
Update the EF Core model/DbContext configuration (where the UsersWithGroup
entity is configured) to include HasIndex(e => new { e.GroupId, e.IsLlmInvisible
}) (make it non-unique) so the database will create the composite index; then
add/emit a migration and apply it to ensure the index exists in the database.
Ensure the index name is descriptive and include the same property names
(GroupId, IsLlmInvisible) so it directly supports GetInvisibleUserIdsAsync.
In `@TelegramSearchBot/Controller/AI/LLM/GeneralLLMController.cs`:
- Around line 248-253: The current invisibility notice is sent every time an
invisible user triggers LLM checks in GeneralLLMController (the block using
_llmVisibilityService.IsUserInvisibleAsync and SendMessageService.SendMessage),
which is noisy; change it to rate-limit or suppress repeated notices by reusing
the existing rate-limit pattern (e.g.,
AgentChatModeController.SendRateLimitedWarningAsync or a Redis-TTL key per
chat+user). Specifically, before calling SendMessageService.SendMessage,
check/create a TTL key (unique by telegramMessage.Chat.Id + fromUserId +
"llm_invisible") and only send the notice if the key is missing, then set the
TTL (e.g., minutes) so subsequent hits skip the send; if your codebase exposes
SendRateLimitedWarningAsync, call that helper instead of direct
SendMessageService.SendMessage to enforce the same behavior.
🪄 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: 409d4d76-c8a2-418c-9697-4a029691e25f
📒 Files selected for processing (19)
TelegramSearchBot.Database/Migrations/20260601111353_AddLlmInvisibilityToUserWithGroup.Designer.csTelegramSearchBot.Database/Migrations/20260601111353_AddLlmInvisibilityToUserWithGroup.csTelegramSearchBot.Database/Migrations/DataDbContextModelSnapshot.csTelegramSearchBot.Database/Model/Data/UserWithGroup.csTelegramSearchBot.LLM.Test/Service/AI/LLM/LlmVisibilityServiceTests.csTelegramSearchBot.LLM/Service/AI/LLM/AnthropicService.csTelegramSearchBot.LLM/Service/AI/LLM/GeminiService.csTelegramSearchBot.LLM/Service/AI/LLM/LlmVisibilityService.csTelegramSearchBot.LLM/Service/AI/LLM/OpenAIResponsesService.csTelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.csTelegramSearchBot.Test/Service/AI/LLM/LLMTaskQueueServiceTests.csTelegramSearchBot/Controller/AI/LLM/AgentChatModeController.csTelegramSearchBot/Controller/AI/LLM/AltPhotoController.csTelegramSearchBot/Controller/AI/LLM/GeneralLLMController.csTelegramSearchBot/Controller/Manage/LlmVisibilityController.csTelegramSearchBot/Service/AI/LLM/AgentChatBatchDispatchService.csTelegramSearchBot/Service/AI/LLM/LLMTaskQueueService.csTelegramSearchBot/Service/Manage/RefreshService.csTelegramSearchBot/Service/Tools/SearchToolService.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
IsLlmInvisibleflag and self-service commands for users to enable/disable/check LLM invisibilityTests
Summary by CodeRabbit
New Features
Tests