fix: [CRITICAL FIX] mixed message bug #10950
Open
Conversation
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
|
@joshuap233 is attempting to deploy a commit to the LobeHub OSS Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Reviewer's GuideAdjusts AI chat message selection logic to properly isolate messages between main topics and their threads when generating responses. Sequence diagram for AI chat message selection with thread isolationsequenceDiagram
actor User
participant UI as ChatUI
participant Store as AIChatStore
participant Action as generateAIChatV2
participant ThreadSelectors as ThreadSelectors
participant ChatSelectors as ChatSelectors
User->>UI: SendMessage(topicId, optional threadId)
UI->>Store: dispatch generateAIChatV2(data)
Store->>Action: invoke(data, get, set)
Action->>Store: get() to read state
Action->>ThreadSelectors: getThreadsByTopic(activeTopicId)
ThreadSelectors-->>Action: threads
Action->>Action: find active ThreadItem by id
Action->>Action: set sourceMessageId from ThreadItem
Action->>ChatSelectors: activeBaseChats(get())
ChatSelectors-->>Action: baseMessages
Action->>Action: filter out assistantMessageId
Action->>Action: apply thread isolation filter
Note over Action: If no activeThreadId
Action->>Action: keep messages with null threadId
Note over Action: If activeThreadId is set
Action->>Action: track isParent flag
Action->>Action: include messages while isParent is true
Action->>Action: when message.id == sourceMessageId
Action->>Action: set isParent to false
Action->>Action: from then on, include only messages with threadId == activeThreadId
Action-->>Store: proceed with AI response using filtered messages
Store-->>UI: update chat with new assistant message
UI-->>User: display isolated thread conversation
Class diagram for updated AI chat threading and message selectionclassDiagram
class AIChatStore {
+generateAIChatV2(data, get, set)
}
class ThreadItem {
+string id
+string topicId
+string sourceMessageId
}
class UIChatMessage {
+string id
+string topicId
+string threadId
+string role
+string content
}
class ThreadSelectors {
+getThreadsByTopic(topicId)
}
class ChatSelectors {
+activeBaseChats(storeState)
}
class GenerateAIChatContext {
+string activeTopicId
+string activeThreadId
+string sourceMessageId
+boolean isParent
+UIChatMessage[] baseMessages
+UIChatMessage[] filteredMessages
+void applyAssistantFilter(assistantMessageId)
+void applyThreadIsolationFilter()
}
AIChatStore --> GenerateAIChatContext : uses
AIChatStore --> ThreadSelectors : calls
AIChatStore --> ChatSelectors : calls
ThreadSelectors --> ThreadItem : returns
ChatSelectors --> UIChatMessage : returns
GenerateAIChatContext --> UIChatMessage : filters
GenerateAIChatContext --> ThreadItem : reads sourceMessageId
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
dosubot
bot
added
the
size:S
Contributor
TestGru AssignmentSummary
Files
Tip You can |
![sourcery-ai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
isParentflag being mutated inside thebaseMessagesfilter callback makes the inclusion logic order-dependent and hard to reason about; consider deriving the parent/child sets in a separate pass (e.g., first slice messages up tosourceMessageId, then filter bythreadId) instead of relying on mutable outer state. - When resolving
sourceMessageId, usingthreads.find(t => t.id === activeThreadId)instead offorEachwould be clearer and allow you to avoid mutating a local variable inside the loop, which would simplify the control flow.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `isParent` flag being mutated inside the `baseMessages` filter callback makes the inclusion logic order-dependent and hard to reason about; consider deriving the parent/child sets in a separate pass (e.g., first slice messages up to `sourceMessageId`, then filter by `threadId`) instead of relying on mutable outer state.
- When resolving `sourceMessageId`, using `threads.find(t => t.id === activeThreadId)` instead of `forEach` would be clearer and allow you to avoid mutating a local variable inside the loop, which would simplify the control flow.
## Individual Comments
### Comment 1
<location> `src/store/chat/slices/aiChat/actions/generateAIChatV2.ts:244-253` </location>
<code_context>
+ }
+ });
+
+ let isParent = true;
const baseMessages = chatSelectors
.activeBaseChats(get())
- .filter((item) => item.id !== data.assistantMessageId);
+ .filter((item) => item.id !== data.assistantMessageId)
+ .filter((item) => {
+ // eslint-disable-next-line eqeqeq
+ if (activeThreadId == null) {
+ // eslint-disable-next-line eqeqeq
+ return item.threadId == null;
+ }
+ const _isParent = isParent;
+ if (item.id === sourceMessageId) {
+ isParent = false;
+ }
+ return item.threadId === activeThreadId || _isParent;
+ });
if (data.topicId) get().internal_updateTopicLoading(data.topicId, true);
</code_context>
<issue_to_address>
**issue (bug_risk):** The `isParent` side effect inside `filter` can lead to subtle ordering-dependent behavior, especially when `sourceMessageId` is missing.
This relies on mutating `isParent` during `filter` to include all messages up to `sourceMessageId`, then only thread-specific ones. Two issues:
1) If `sourceMessageId` is not in `baseMessages`, `isParent` never becomes `false`, so with a non-null `activeThreadId` you include all messages instead of limiting to that thread.
2) Mutating `isParent` inside `filter` makes control flow order-dependent and harder to reason about.
Consider first finding the index of `sourceMessageId` and slicing/splitting the array, or using an explicit `for` loop / `reduce` to separate parent vs thread messages without side effects.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dosubot
bot
added
size:M
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.
💻 Change Type
🔗 Related Issue
Fixes #8327
🔀 Description of Change
Fixes two issues related to message isolation within threaded conversations:
🧪 How to Test
📸 Screenshots / Videos
N/A
📝 Additional Information
Impact of the Bug:
Without this fix, users with existing conversation threads would incur significantly higher operational costs due to unnecessary token expenditure. Each request was effectively billed for double the actual required context.
Summary by Sourcery
Fix message selection for AI responses so that threaded conversations remain isolated from each other and from the main topic.
Bug Fixes: