fix(webui-v2): render chat errors inline - #5822
Merged
Merged
Conversation
italic-jinxin
added
size: L
Contributor
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 8, 2026 11:00
Destroyed
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChat error handling now uses centralized roles and typed failure messages. Request and stream failures are appended inline with deterministic IDs, retries remove linked failure bubbles, and error-role bubbles use left-aligned chat-stream layout. Unit, rendering, runtime, and E2E coverage were updated. ChangesInline error message rendering
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Chat
participant useChat
participant useChatEvents
participant ThreadStore
participant MessageBubble
Chat->>useChat: send message
useChat->>ThreadStore: add optimistic user message
useChatEvents-->>useChat: report stream error
useChat->>ThreadStore: append inline error message
ThreadStore->>MessageBubble: render error-role message
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
github-actions
Bot
added
risk: low
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 8, 2026 11:01
Destroyed
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces inline error message rendering for request and stream failures in the chat UI, replacing toast notifications. It adds error handling to thread creation and message sending, formats stream and request errors, and updates unit and E2E tests to cover these new failure scenarios. Feedback is provided regarding the stream error deduplication logic, where using a static ID derived solely from error properties can cause subsequent stream errors of the same type to be silently ignored.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Contributor
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.07% — 285099 / 335144 lines Per-crate breakdown (63 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
|
🚅 Deployed to the ironclaw-pr-5822 environment in ironclaw-ci-preview
|
…t-errors # Conflicts: # crates/ironclaw_webui_v2/frontend/src/pages/chat/lib/failureMessages.test.mts
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 8, 2026 11:55
Destroyed
…t-errors # Conflicts: # crates/ironclaw_webui_v2/frontend/src/pages/chat/lib/useChat-send.test.mts
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 8, 2026 13:00
Destroyed
github-actions
Bot
added
size: XL
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 8, 2026 13:07
Destroyed
…nto issue-5708-inline-chat-errors
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 8, 2026 13:53
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 8, 2026 14:03
Destroyed
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 9, 2026 12:46
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 9, 2026 12:49
Destroyed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts (1)
1004-1042: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRetry-failure path drops the inline error bubble it just claimed to restore.
removeFailedstrips bothmessage.idandfailedRequestErrorId. On a successful retry that's correct —sendwill render its own bubbles if it fails again. But on the early-exit paths (response === nullfrom admission block, orsendthrowing before its own try/catch, e.g.approvalGatePendingSendError()),restoreFailedIfNoReplacementpushes back onlymessage:return hasReplacement || prev.some((item) => item.id === message.id) ? prev : [...prev, message];The comment at the catch site says this should "restore the original retryable error bubble," but no bubble is ever re-added — only the failed user message. The user is left with a retryable bubble and no explanation, which is exactly the floating/disappearing-error UX this PR sets out to fix.
useChat-send.test.ts's new retry test only exercises the success path, so this gap is untested.🐛 Proposed fix: restore the removed error bubble alongside the message
const restoreFailedIfNoReplacement = (prev) => { const hasReplacement = prev.some( (item) => item.id !== message.id && item.role === CHAT_MESSAGE_ROLES.USER && item.status === "error" && item.retryContent === content, ); - return hasReplacement || prev.some((item) => item.id === message.id) - ? prev - : [...prev, message]; + if (hasReplacement || prev.some((item) => item.id === message.id)) { + return prev; + } + const restored = [...prev, message]; + return message.error + ? [...restored, requestFailureMessageForContent(message.id, message.error)] + : restored; };🤖 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 `@crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts` around lines 1004 - 1042, The retry rollback in useChat’s retry flow restores only the original message but not the inline failed-request bubble that removeFailed deletes. Update restoreFailedIfNoReplacement in useChat to re-add both message and failedRequestErrorId on early-exit paths (response === null or caught send failures) unless a replacement already exists, so the original retryable error bubble is truly restored. Verify the retry handling around send, removeFailed, and restoreFailedIfNoReplacement keeps the inline error visible for admission-failure cases.
🤖 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 `@crates/ironclaw_webui_v2/src/static_assets/assets.rs`:
- Around line 282-283: The assertions in the asset test are checking outdated
string literals, so they no longer match the current `message-bubble.tsx`
source. Update the `message_bubble` `contains` checks to look for the
`CHAT_MESSAGE_ROLES.SYSTEM` and `CHAT_MESSAGE_ROLES.ERROR` references used in
the source text. Keep the fix localized to the test in `assets.rs`, using the
existing `source_text`/`message_bubble` assertions.
In `@tests/e2e/scenarios/test_reborn_webui_v2_legacy_pending_messages.py`:
- Line 278: The private helper handle_failed_send is missing an explicit return
type annotation, which triggers ANN202. Update the function signature in the
test helper to declare that it returns nothing by adding a None return
annotation, keeping the existing route/_payload/_fulfill_json parameters
unchanged.
---
Outside diff comments:
In `@crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts`:
- Around line 1004-1042: The retry rollback in useChat’s retry flow restores
only the original message but not the inline failed-request bubble that
removeFailed deletes. Update restoreFailedIfNoReplacement in useChat to re-add
both message and failedRequestErrorId on early-exit paths (response === null or
caught send failures) unless a replacement already exists, so the original
retryable error bubble is truly restored. Verify the retry handling around send,
removeFailed, and restoreFailedIfNoReplacement keeps the inline error visible
for admission-failure cases.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f63d39aa-2835-4ca1-ba41-a446f8004466
📒 Files selected for processing (12)
crates/ironclaw_webui_v2/frontend/src/pages/chat/components/message-bubble.test.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/components/message-bubble.tsxcrates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/failureMessages.test.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/failureMessages.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/message-types.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/useChat-send.test.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/useChatEvents.test.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/useChatEvents.tscrates/ironclaw_webui_v2/src/static_assets/assets.rstests/e2e/reborn_coverage_tests.txttests/e2e/scenarios/test_reborn_webui_v2_legacy_pending_messages.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts (1)
1117-1122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the dead helper —
requestFailureMessageForErrorhas no callers incrates/ironclaw_webui_v2/frontend/srcand just wraps the same content path already used byappendRequestFailureMessage. Delete it, or wire the intended caller if one exists.🤖 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 `@crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts` around lines 1117 - 1122, `requestFailureMessageForError` is a dead helper in `useChat.ts` with no callers under the frontend src tree and it duplicates the failure-message path already handled by `appendRequestFailureMessage`. Remove the unused `requestFailureMessageForError` function, or if it is meant to be used, update the relevant caller to reference it explicitly so there is a single owned path for request failure message creation.
🤖 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.
Outside diff comments:
In `@crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts`:
- Around line 1117-1122: `requestFailureMessageForError` is a dead helper in
`useChat.ts` with no callers under the frontend src tree and it duplicates the
failure-message path already handled by `appendRequestFailureMessage`. Remove
the unused `requestFailureMessageForError` function, or if it is meant to be
used, update the relevant caller to reference it explicitly so there is a single
owned path for request failure message creation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4e7751d7-8768-45ad-bdb4-d84f710719a1
📒 Files selected for processing (4)
crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/useChat-send.test.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/useChatEvents.test.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/useChatEvents.ts
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 9, 2026 13:29
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 9, 2026 13:51
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 9, 2026 13:56
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 9, 2026 14:25
Destroyed
github-actions
Bot
added
size: XL
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts (1)
1033-1050: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRetry restore path drops the inline error explanation.
removeFailednow strips the request-failure bubble (isRequestFailureForMessage) along with the user message, butrestoreFailedIfNoReplacementonly re-appendsmessage— never the stripped bubble. Whensend()throwsapprovalGatePendingSendError()(thrown beforesend's own try/catch, so it never appends a replacement) or returnsnullfrom the busy/admission guards,retryMessage's catch/nullbranches callrestoreFailedIfNoReplacement, so the user bubble reappears but its inline error explanation is gone for good — the exact regression this PR (#5708) is meant to fix. No existing test exercises this combination (an error bubble present + a blocked retry).🐛 Proposed fix: capture and restore the stripped bubble
+ let removedRequestFailure; const removeFailed = (prev) => - prev.filter( - (item) => - item.id !== message.id && - !isRequestFailureForMessage(item, message.id), - ); + prev.filter((item) => { + if (item.id === message.id) return false; + if (isRequestFailureForMessage(item, message.id)) { + removedRequestFailure = removedRequestFailure || item; + return false; + } + return true; + }); const restoreFailedIfNoReplacement = (prev) => { const hasReplacement = prev.some( (item) => item.id !== message.id && item.role === CHAT_MESSAGE_ROLES.USER && item.status === "error" && item.retryContent === content, ); - return hasReplacement || prev.some((item) => item.id === message.id) - ? prev - : [...prev, message]; + if (hasReplacement || prev.some((item) => item.id === message.id)) { + return prev; + } + return removedRequestFailure + ? [...prev, message, removedRequestFailure] + : [...prev, message]; };🤖 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 `@crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts` around lines 1033 - 1050, The retry restore path in useChat loses the inline request-failure bubble because removeFailed filters out both the user message and isRequestFailureForMessage, while restoreFailedIfNoReplacement only re-adds message. Update retryMessage’s restore logic to preserve and restore the stripped failure bubble when send() is blocked or throws approvalGatePendingSendError(), using the existing helpers removeFailed, restoreFailedIfNoReplacement, and isRequestFailureForMessage so the original inline explanation returns with the user bubble.
🤖 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.
Outside diff comments:
In `@crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts`:
- Around line 1033-1050: The retry restore path in useChat loses the inline
request-failure bubble because removeFailed filters out both the user message
and isRequestFailureForMessage, while restoreFailedIfNoReplacement only re-adds
message. Update retryMessage’s restore logic to preserve and restore the
stripped failure bubble when send() is blocked or throws
approvalGatePendingSendError(), using the existing helpers removeFailed,
restoreFailedIfNoReplacement, and isRequestFailureForMessage so the original
inline explanation returns with the user bubble.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4a8d41dc-81f2-490b-95ab-d86eedeb3a1c
📒 Files selected for processing (4)
crates/ironclaw_webui_v2/frontend/src/pages/chat/components/message-bubble.tsxcrates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/message-types.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/useChat-send.test.ts
…t-errors # Conflicts: # crates/ironclaw_webui_v2/frontend/src/pages/chat/components/message-bubble.tsx
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5822
July 10, 2026 04:57
Destroyed
think-in-universe
approved these changes
Jul 10, 2026
This was referenced Jul 10, 2026
8 tasks done
This branch was successfully deployed
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
2 participants
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
errorframes in the chat event reducer and dedupe reconnect repeats.Linked Issue
Closes #5708
Validation
node --test crates/ironclaw_webui_v2/static/js/pages/chat/lib/failureMessages.test.mjsnode --test crates/ironclaw_webui_v2/static/js/pages/chat/lib/useChatEvents.test.mjsnode --test crates/ironclaw_webui_v2/static/js/pages/chat/lib/useChat-send.test.mjsNO_PROXY=127.0.0.1,localhost no_proxy=127.0.0.1,localhost tests/e2e/.venv/bin/pytest tests/e2e/scenarios/test_reborn_webui_v2_legacy_pending_messages.py::test_reborn_legacy_send_failure_renders_inline_error_not_toast -qgit diff --checkSecurity Impact
No auth, secret, sandbox, listener, or outbound-network behavior changes.
Database Impact
No database or migration changes.
Blast Radius
Limited to WebChat v2 frontend chat error rendering and one E2E coverage entry.
Rollback Plan
Revert this PR; send/stream failures return to the previous failed-bubble/stream-error behavior.
Review track: C