Features/add auto compression#1376
Conversation
PR Summary by QodoAdd automatic conversation compression with summarization and archival
AI Description
Diagram
High-Level Assessment
Files changed (16)
|
Code Review by Qodo
1. Case-sensitive MessageId match
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| var foundIdx = dialogs.FindIndex(x => x.MetaData?.MessageId == cutMessageId); | ||
| if (foundIdx <= 0) | ||
| { | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
1. Case-sensitive messageid match 📘 Rule violation ≡ Correctness
The new compaction code compares identifier-like MessageId strings using case-sensitive ==, which can fail to locate the cut point when casing differs. This violates the required ordinal case-insensitive semantics for identifiers and can cause compaction to no-op unexpectedly.
Agent Prompt
## Issue description
Identifier-like string comparisons for `MessageId` are performed with case-sensitive equality, which can break lookups when casing differs.
## Issue Context
Compaction depends on reliably finding the cut dialog by `cutMessageId`. Per compliance, identifier comparisons must use ordinal case-insensitive semantics.
## Fix Focus Areas
- src/Infrastructure/BotSharp.Core/Repository/FileRepository/FileRepository.Conversation.cs[901-905]
- src/Plugins/BotSharp.Plugin.MongoStorage/Repository/MongoRepository.Conversation.cs[770-775]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| var curIndexes = collection.Indexes.List().ToList().Where(x => x.Contains("name")).Select(x => x["name"].AsString); | ||
| if (!curIndexes.Any(x => x.StartsWith("ConversationId"))) | ||
| { |
There was a problem hiding this comment.
2. startswith lacks stringcomparison 📘 Rule violation ⚙ Maintainability
The new Mongo index-existence check uses StartsWith without specifying StringComparison, relying on default comparison behavior. This violates the requirement to use ordinal case-insensitive semantics for identifier-like comparisons and can behave inconsistently across cultures/casing.
Agent Prompt
## Issue description
Identifier-like string prefix checks are performed without an explicit `StringComparison`, which can lead to inconsistent behavior.
## Issue Context
Compliance requires ordinal case-insensitive semantics for identifier-like comparisons.
## Fix Focus Areas
- src/Plugins/BotSharp.Plugin.MongoStorage/MongoDbContext.cs[150-152]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| var cutIndex = dialogs.Count - Math.Max(0, keepRecentCount); | ||
| while (cutIndex > 0 && dialogs[cutIndex].Role != AgentRole.User) | ||
| { | ||
| cutIndex--; | ||
| } |
There was a problem hiding this comment.
3. Compression cut index crash 🐞 Bug ≡ Correctness
ComputeCompressionCut can produce cutIndex == dialogs.Count when KeepRecentCount <= 0, causing an IndexOutOfRangeException when accessing dialogs[cutIndex] and making auto-compression fail repeatedly under misconfiguration.
Agent Prompt
### Issue description
`ComputeCompressionCut` can compute an out-of-range index when `keepRecentCount` is `0` or negative, because it sets `cutIndex = dialogs.Count - Math.Max(0, keepRecentCount)` and then indexes `dialogs[cutIndex]`.
### Issue Context
`KeepRecentCount` is configuration-driven and currently has no validation, so `0`/negative values can enter at runtime.
### Fix Focus Areas
- src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.Compression.cs[154-168]
### Suggested fix
- Treat `keepRecentCount <= 0` as invalid and return `-1` (no compression), **or** clamp to a minimum of `1`.
- Ensure `cutIndex` is always within `[0, dialogs.Count - 1]` before indexing (e.g., early-return when `cutIndex >= dialogs.Count`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| finally | ||
| { | ||
| // Release both guards so the next turn can compress again. Runs only because we acquired | ||
| // them above (a failed acquire returns before entering this try). | ||
| await db.UpdateConversationCompressing(conversationId, false); | ||
| _localCompressing.TryRemove(conversationId, out _); | ||
| } |
There was a problem hiding this comment.
4. Compression guard leak 🐞 Bug ☼ Reliability
AutoCompressIfNeeded releases the persisted "IsCompressing" flag before removing the local in-memory guard; if the repository release call throws, the local guard removal is skipped and this instance can permanently stop compressing that conversation until restart.
Agent Prompt
### Issue description
In `AutoCompressIfNeeded`, the `finally` block does `await db.UpdateConversationCompressing(..., false)` and only then removes the local guard. If the awaited call throws (I/O/Mongo/network), `_localCompressing.TryRemove` never runs.
### Issue Context
This is a fire-and-forget background task, so failures are plausible (transient DB/file errors). Leaking `_localCompressing` for a conversation means all future attempts on the same instance return early forever.
### Fix Focus Areas
- src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.Compression.cs[139-145]
### Suggested fix
- In the `finally` block, remove the local guard in its own `finally` (or first), and wrap the repository release call in a `try/catch` so cleanup cannot prevent other cleanup.
- Example structure:
- `try { await db.UpdateConversationCompressing(..., false); } catch (Exception ex) { _logger.LogWarning(ex, ...); } finally { _localCompressing.TryRemove(...); }`
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| //var filterArchive = Builders<ConversationArchiveDocument>.Filter.In(x => x.ConversationId, conversationIds); | ||
| //var filterConvFile = Builders<ConversationFileDocument>.Filter.In(x => x.ConversationId, conversationIds); | ||
|
|
||
| var promptLogDeleted = await _dc.LlmCompletionLogs.DeleteManyAsync(filterPromptLog); |
There was a problem hiding this comment.
5. Archived data not deleted 🐞 Bug ⛨ Security
MongoRepository.DeleteConversations does not delete ConversationArchives records (the deletion is commented out), so archived raw dialogs and trimmed state versions can remain in storage even after the conversation is deleted.
Agent Prompt
### Issue description
`DeleteConversations` removes conversations/dialogs/states/logs, but leaves `ConversationArchives` undeleted because the filter and delete calls are commented out.
### Issue Context
`ConversationArchives` holds archived raw dialogs and state versions for compaction runs. Leaving these behind defeats expectations for data deletion and can retain sensitive user content.
### Fix Focus Areas
- src/Plugins/BotSharp.Plugin.MongoStorage/Repository/MongoRepository.Conversation.cs[67-95]
### Suggested fix
- Re-enable archive deletion:
- Create `filterArchive` and call `_dc.ConversationArchives.DeleteManyAsync(filterArchive)`.
- Ensure the method’s return value accounts for archive deletions if needed.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
1 participant
No description provided.