Bump PF chatbot, update loading msgs and improve error handling#3100
Conversation
WalkthroughThis update corrects the token expiration logic in the chatbot component, refines error handling and streaming response management in the chatbot window, and introduces a loading state to bot messages. It also updates the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatBotWindow
participant BotMessage
participant TokenService
User->>ChatBotWindow: Sends message
ChatBotWindow->>TokenService: Check token expiration
TokenService-->>ChatBotWindow: Return valid/refresh token
ChatBotWindow->>BotMessage: Render with isLoading=true
ChatBotWindow->>API: Send message, start streaming
API-->>ChatBotWindow: Stream 'token' events
ChatBotWindow->>BotMessage: Update with new tokens
API-->>ChatBotWindow: Stream 'end' event
ChatBotWindow->>BotMessage: Render with isLoading=false
ChatBotWindow-->>User: Display final message or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ammont82, rawagner The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
apps/assisted-ui/package.json(1 hunks)apps/assisted-ui/src/components/Chatbot.tsx(1 hunks)libs/chatbot/lib/components/ChatBot/BotMessage.tsx(3 hunks)libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx(6 hunks)libs/chatbot/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in the assisted-installer-ui chatbot feedback implementation, the onfeedbacksubmit callback requires...
Learnt from: celdrake
PR: openshift-assisted/assisted-installer-ui#3051
File: libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx:196-222
Timestamp: 2025-07-18T12:35:50.945Z
Learning: In the assisted-installer-ui chatbot feedback implementation, the onFeedbackSubmit callback requires access to the messages array to retrieve both the bot response content and the associated user question for the API call, making it necessary to include messages in the useCallback dependency array rather than passing message content as props to avoid duplicating potentially long message data.
Applied to files:
apps/assisted-ui/package.jsonlibs/chatbot/lib/components/ChatBot/BotMessage.tsxlibs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx
🧬 Code Graph Analysis (1)
libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx (1)
libs/types/assisted-installer-service.d.ts (1)
Error(997-1018)
🔇 Additional comments (12)
apps/assisted-ui/package.json (1)
4-4: LGTM! Dependency reordering and version alignment.The dependency reordering and
@patternfly-6/chatbotversion update to 6.3.2 aligns with the coordinated changes across the codebase to support enhanced chatbot functionality.Also applies to: 6-6
apps/assisted-ui/src/components/Chatbot.tsx (1)
14-14: Excellent fix! Token expiration logic corrected.The condition has been properly corrected to refresh tokens when they expire within the next 5 seconds (
Date.now() > expiration - 5000). The previous logic (Date.now() - 5000 > expiration) was incorrect and would have caused authentication failures.libs/chatbot/lib/components/ChatBot/BotMessage.tsx (3)
3-3: LGTM! Clean import for loading component.
59-59: LGTM! Well-designed loading prop addition.The new
isLoadingboolean prop provides a clear interface for managing the loading state of bot messages.Also applies to: 67-67
129-140: Excellent loading state implementation.The conditional rendering properly handles the loading state by:
- Disabling user actions when loading (line 133)
- Showing the loading indicator with clear messaging (line 136)
- Maintaining clean JSX structure with proper conditional logic
libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx (7)
111-111: Good addition for tracking stream completion.The
eventEndedflag provides proper tracking of whether the streaming response completed successfully.
139-147: Improved error handling with graceful fallback.The enhanced error handling correctly attempts to parse detailed error messages from the response JSON while providing a safe fallback to generic messaging if parsing fails.
175-177: Proper stream completion detection.Explicitly detecting the 'end' event and setting the
eventEndedflag ensures reliable tracking of stream completion.
179-179: Good validation to prevent empty token processing.Adding the check
&& !!ev.data.tokenprevents empty or falsy tokens from being processed, which could cause issues in the UI.
216-218: Essential error handling for incomplete streams.This check ensures that if the stream doesn't properly end with an 'end' event, users receive appropriate feedback rather than being left in an uncertain state.
351-351: Perfect integration with BotMessage loading state.The
isLoadingprop is correctly passed only to the last message during streaming, providing appropriate visual feedback to users.
363-363: More appropriate generic error message.The change from "Failed to send the message" to "An error occurred" is more accurate since errors can happen during various phases of the interaction, not just message sending.
|
/lgtm |
8500c47
into
openshift-assisted:master
Summary by CodeRabbit
New Features
Bug Fixes
Chores