Fix UnifiedChat streamed item rendering#479
Merged
AnthonyRonning merged 3 commits intomasterfrom Apr 23, 2026
Merged
Conversation
📝 WalkthroughWalkthroughRefactors UnifiedChat.tsx to introduce Changes
Sequence DiagramsequenceDiagram
participant Client as Client/WebSocket
participant StreamHandler as StreamHandler<br/>(processStreamingResponse)
participant Buffer as DeltaBuffer<br/>(item_id,content_index)
participant Normalizer as normalizeConversationItem
participant MessageStore as MessageStore / upsert
participant MessageList as MessageList
participant ToolRenderer as ToolCallRenderer
Client->>StreamHandler: response.output_item.added (tool_call)
StreamHandler->>Normalizer: normalizeConversationItem(item)
Normalizer-->>StreamHandler: ToolCallItem
StreamHandler->>MessageStore: upsert ToolCallItem (status: in_progress)
Client->>StreamHandler: response.output_item.added (tool_output)
StreamHandler->>Normalizer: normalizeConversationItem(item)
Normalizer-->>StreamHandler: ToolOutputItem
StreamHandler->>MessageStore: upsert ToolOutputItem
Client->>StreamHandler: response.tool_output.done / tool_call.done
StreamHandler->>MessageStore: update related ToolCallItem -> completed
Client->>StreamHandler: response.output_text.delta / reasoning_text.delta
StreamHandler->>Buffer: append delta to (item_id,content_index)
Buffer-->>StreamHandler: assembled chunk
StreamHandler->>MessageStore: mergeStreamingConversationItem -> update content/status
MessageList->>MessageList: build toolCallsByCallId, pair outputs with preceding call
MessageList->>ToolRenderer: render grouped toolOutputs + status
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying maple with
|
| Latest commit: |
17f1e4b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://38a4c841.maple-ca8.pages.dev |
| Branch Preview URL: | https://fix-unified-chat-streaming-u.maple-ca8.pages.dev |
AnthonyRonning
force-pushed
the
fix/unified-chat-streaming-ui
branch
2 times, most recently
from
26ee483 to
fbccf72
Compare
Preserve backend-driven streamed item state so reasoning, tool progress, and loading UI stay stable as SSE lifecycle events arrive. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
The tool_call and tool_output branches of mergeStreamingConversationItem were hard-coding status to the existing value (or 'in_progress' as fallback), discarding item.status. This could leave a failing tool call stuck on a spinner if response.output_item.done carried 'incomplete' or 'error' without a matching tool_output.created follow-up. Bring both branches in line with the message/reasoning branches by preferring item.status when provided. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
AnthonyRonning
force-pushed
the
fix/unified-chat-streaming-ui
branch
from
fbccf72 to
c941c89
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/components/UnifiedChat.tsx (1)
2356-2358: Consider removing debug logging in production.The SSE event logging on every event could be noisy in production. Consider either removing it or wrapping it in a debug flag.
♻️ Suggested change
- // Log all SSE events for debugging - console.log("🔵 SSE Event:", eventType, event); + // Uncomment for debugging SSE events: + // console.log("🔵 SSE Event:", eventType, event);🤖 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 2356 - 2358, Remove or guard the noisy SSE console logging by eliminating the unconditional console.log("🔵 SSE Event:", eventType, event) in UnifiedChat or wrapping it behind a debug flag; update the SSE handler where that line appears (search for the string "🔵 SSE Event:" or the SSE event handler function in UnifiedChat.tsx) to only log when a debug mode is enabled (e.g., a local DEBUG/REACT_APP_DEBUG flag or NODE_ENV !== 'production') so production builds do not emit per-event logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/components/UnifiedChat.tsx`:
- Around line 2356-2358: Remove or guard the noisy SSE console logging by
eliminating the unconditional console.log("🔵 SSE Event:", eventType, event) in
UnifiedChat or wrapping it behind a debug flag; update the SSE handler where
that line appears (search for the string "🔵 SSE Event:" or the SSE event
handler function in UnifiedChat.tsx) to only log when a debug mode is enabled
(e.g., a local DEBUG/REACT_APP_DEBUG flag or NODE_ENV !== 'production') so
production builds do not emit per-event logs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c051f037-4f99-4133-bb83-0f70e0b1bb46
📒 Files selected for processing (1)
frontend/src/components/UnifiedChat.tsx
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/components/UnifiedChat.tsx (2)
1138-1169: Consider adding a comment to clarify the loop index modification.The pattern of modifying
indexon line 1165 to skip consumed tool outputs works correctly, but this imperative index manipulation within a for loop can be confusing. A brief comment would help future readers understand the intent:if (matchedOutputs.length > 0) { + // Advance index past consumed tool_output items index = nextIndex - 1; }🤖 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 1138 - 1169, The loop mutates the for-loop index to skip consumed tool output items after rendering a ToolCall, which is correct but unclear; add a concise inline comment above the index reassignment explaining that matchedOutputs were consumed and index is advanced to nextIndex - 1 to avoid re-processing those items (referencing the variables itemType/"tool_call", ToolCallItem, completedToolCallIds, renderedToolCall, matchedOutputs, nextIndex, isToolOutputItem, and ToolCallRenderer) so future readers understand the imperative index modification.
582-595: Consider adding"searching"toMessageStatustype for consistency.Line 589 checks for
"searching"status, but this value isn't included in theMessageStatustype definition on line 113. While this works at runtime due to the cast on line 585, it creates a type inconsistency.If
"searching"is a valid status forweb_search_callitems (as used inToolCallRendereron lines 858-865), consider adding it toMessageStatusfor type safety:-type MessageStatus = "completed" | "in_progress" | "incomplete" | "streaming" | "error"; +type MessageStatus = "completed" | "in_progress" | "incomplete" | "streaming" | "error" | "searching";🤖 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 582 - 595, The code filters for a "searching" status in updateActiveItemStatuses but the MessageStatus type (MessageStatus union) doesn't include "searching", causing a type mismatch; update the MessageStatus union/type to include the "searching" literal so TypeScript recognizes it as valid, and ensure any related enums/types used by Message, ToolCallRenderer, and web_search_call handling also accept "searching" to keep consistency across updateActiveItemStatuses, ToolCallRenderer, and Message definitions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/components/UnifiedChat.tsx`:
- Around line 1138-1169: The loop mutates the for-loop index to skip consumed
tool output items after rendering a ToolCall, which is correct but unclear; add
a concise inline comment above the index reassignment explaining that
matchedOutputs were consumed and index is advanced to nextIndex - 1 to avoid
re-processing those items (referencing the variables itemType/"tool_call",
ToolCallItem, completedToolCallIds, renderedToolCall, matchedOutputs, nextIndex,
isToolOutputItem, and ToolCallRenderer) so future readers understand the
imperative index modification.
- Around line 582-595: The code filters for a "searching" status in
updateActiveItemStatuses but the MessageStatus type (MessageStatus union)
doesn't include "searching", causing a type mismatch; update the MessageStatus
union/type to include the "searching" literal so TypeScript recognizes it as
valid, and ensure any related enums/types used by Message, ToolCallRenderer, and
web_search_call handling also accept "searching" to keep consistency across
updateActiveItemStatuses, ToolCallRenderer, and Message definitions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35d58dc3-c232-478f-a872-c5be80535fb4
📒 Files selected for processing (1)
frontend/src/components/UnifiedChat.tsx
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
Validation
Summary by CodeRabbit
New Features
Improvements