Fix thinking blocks rendering below streaming text due to SSE race condition#456
Conversation
…ndition Sort assistant group items so reasoning items always appear before message items, regardless of the order SSE events arrive. This prevents the race condition where the assistant message 'added' event arrives before the reasoning 'added' event, causing thinking blocks to render below the '...' loading dots or streamed text. Co-Authored-By: tony@opensecret.cloud <TonyGiorgio@protonmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Deploying maple with
|
| Latest commit: |
e7e31ee
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://871e1912.maple-ca8.pages.dev |
| Branch Preview URL: | https://devin-1772688876-fix-thinkin.maple-ca8.pages.dev |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThe change adds deterministic sorting for assistant-related items in the UnifiedChat component, establishing a fixed order where reasoning items render first, followed by tool calls and outputs, then messages. This sort is applied when processing assistant item groups during streaming, improving resilience to out-of-order events. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/UnifiedChat.tsx`:
- Around line 525-530: The group ID is taken from the unsorted
currentAssistantItems[0].id which can differ from the first item after sorting;
update both places where you build the group object (the block using items:
[...currentAssistantItems].sort((a,b)=>assistantItemSortOrder(a)-assistantItemSortOrder(b))
and the later, identical block) to compute the sorted array into a local
variable (e.g., const sorted = [...currentAssistantItems].sort(...)) and use
sorted[0].id for the group id (`assistant-${sorted[0].id}`) and sorted for
items, then clear currentAssistantItems as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 10830f14-d847-4e6c-bc41-9debd908742c
📒 Files selected for processing (1)
frontend/src/components/UnifiedChat.tsx
| items: [...currentAssistantItems].sort( | ||
| (a, b) => assistantItemSortOrder(a) - assistantItemSortOrder(b) | ||
| ), | ||
| id: `assistant-${currentAssistantItems[0].id}` | ||
| }); | ||
| currentAssistantItems = []; |
There was a problem hiding this comment.
Group ID derived from unsorted array may cause key instability.
The sort result is used for items:, but currentAssistantItems[0].id (lines 528 and 550) references the original unsorted array. If SSE events arrive out of order (the exact scenario this PR fixes), the first item before sorting could be a message item, while after sorting it becomes a reasoning item.
This means the group's React key (assistant-${id}) may reference different item types across renders, potentially causing unnecessary re-renders or subtle reconciliation issues.
Consider using the sorted array's first item for the ID:
🔧 Suggested fix
if (currentAssistantItems.length > 0) {
+ const sortedItems = [...currentAssistantItems].sort(
+ (a, b) => assistantItemSortOrder(a) - assistantItemSortOrder(b)
+ );
groups.push({
type: "assistant",
- items: [...currentAssistantItems].sort(
- (a, b) => assistantItemSortOrder(a) - assistantItemSortOrder(b)
- ),
- id: `assistant-${currentAssistantItems[0].id}`
+ items: sortedItems,
+ id: `assistant-${sortedItems[0].id}`
});
currentAssistantItems = [];
}Apply the same pattern for lines 547-551.
Also applies to: 547-551
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/UnifiedChat.tsx` around lines 525 - 530, The group ID
is taken from the unsorted currentAssistantItems[0].id which can differ from the
first item after sorting; update both places where you build the group object
(the block using items:
[...currentAssistantItems].sort((a,b)=>assistantItemSortOrder(a)-assistantItemSortOrder(b))
and the later, identical block) to compute the sorted array into a local
variable (e.g., const sorted = [...currentAssistantItems].sort(...)) and use
sorted[0].id for the group id (`assistant-${sorted[0].id}`) and sorted for
items, then clear currentAssistantItems as before.
Summary
Fixes a race condition where thinking/reasoning blocks would occasionally render below the "..." loading dots or streamed text in desktop and mobile Tauri builds. This happened because SSE events can arrive out of order — the assistant message
response.output_item.addedevent sometimes arrived before the reasoningresponse.output_item.addedevent, andmergeMessagesByIdpreserves insertion order.The fix sorts items within each assistant group at render time so that reasoning items always appear first, followed by tool-related items (web search, function calls), with assistant message content last. This is done in the
groupedMessagesuseMemo computation insideMessageList.Review & Testing Checklist for Human
assistantItemSortOrderfunction being defined outside theuseMemodoesn't cause unnecessary re-computation (it's a pure function with no captured state, so it should be fine)Notes
Summary by CodeRabbit
Bug Fixes