Improve feedback form.#3107
Improve feedback form.#3107openshift-merge-bot[bot] merged 1 commit intoopenshift-assisted:masterfrom
Conversation
WalkthroughAdds a streaming messages hook (useMessages), moves message sending/streaming into the hook, replaces inline feedback with an API-backed FeedbackForm, introduces an AIAlert component, refactors ChatBot/ChatBotWindow to use the hook and externalize send/error handling, adds modal/input focus tweaks, and removes two helper exports. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CBW as ChatBotWindow
participant Hook as useMessages.sentMessage
participant API as /v1/streaming_query (Backend)
participant HookState as useMessages (state)
User->>CBW: send message
CBW->>Hook: sentMessage(msg)
Hook->>API: POST /v1/streaming_query { message, conversationId }
API-->>Hook: 200 streaming body (SSE/JSON lines)
loop stream events
API-->>Hook: start/inference/tool/end events
Hook->>HookState: append/patch messages, actions, announcement
HookState-->>CBW: updated messages/state
end
Hook->>CBW: finalize (conversationId / error)
sequenceDiagram
autonumber
actor User
participant BM as BotMessage
participant API as /v1/feedback
participant FC as FeedbackForm
User->>BM: click thumbs-up
BM->>API: POST /v1/feedback { sentiment:1, llm_response, user_question, conversation_id }
API-->>BM: response
User->>BM: click thumbs-down
BM->>BM: open FeedbackForm
User->>FC: select category + write feedback + submit
FC->>API: POST /v1/feedback { sentiment:-1, category, userFeedback, llm_response, user_question, conversation_id }
API-->>FC: response
FC-->>BM: onClose()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-07-18T12:35:50.945ZApplied to files:
🧬 Code Graph Analysis (1)libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (10)
✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
openshift-ci
bot
added
approved
rawagner
force-pushed
the
chat_feedback
branch
3 times, most recently
from
81e3bca to
31865ba
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (5)
libs/chatbot/lib/components/ChatBot/FeedbackCard.tsx (3)
25-32: Consider making categories configurable for better reusability.The hardcoded categories object limits the component's flexibility. Consider making this configurable through props or a separate configuration file to support different feedback scenarios.
-const categories: { [key: string]: string } = { - incorrect: 'Incorrect', - not_relevant: 'Not relevant', - incomplete: 'Incomplete', - outdated_information: 'Outdated information', - unsafe: 'Unsafe', - other: 'Other', -}; +const defaultCategories: { [key: string]: string } = { + incorrect: 'Incorrect', + not_relevant: 'Not relevant', + incomplete: 'Incomplete', + outdated_information: 'Outdated information', + unsafe: 'Unsafe', + other: 'Other', +}; const FeedbackForm = ({ onFeedbackSubmit, onClose, + categories = defaultCategories, }: { onFeedbackSubmit: (req: FeedbackRequest) => Promise<void>; onClose: VoidFunction; + categories?: { [key: string]: string }; }) => {
94-97: Consider improving type safety for category selection.The category selection uses string conversion which could be error-prone. Consider using proper type guards or ensuring the value is always a valid category key.
- onSelect={(_, val) => { - setCategory(`${val || ''}`); - setCategoryOpen(false); - }} + onSelect={(_, val) => { + const categoryKey = val as string; + if (categoryKey && categories[categoryKey]) { + setCategory(categoryKey); + } + setCategoryOpen(false); + }}
146-148: Consider adding form validation before submission.The submit button doesn't validate if feedback text is provided. Consider adding basic validation to ensure meaningful feedback is collected.
+ const isFormValid = feedback.trim().length > 0; <Button onClick={() => { void submit(); }} - isDisabled={isSubmitting} + isDisabled={isSubmitting || !isFormValid} isLoading={isSubmitting} >libs/chatbot/lib/components/ChatBot/AIAlert.tsx (1)
29-34: Consider adding aria-label for better accessibility.The link button should have an aria-label to improve screen reader experience, especially since it opens in a new tab.
Apply this diff to enhance accessibility:
<Button variant="link" isInline icon={<ExternalLinkAltIcon />} component="a" href="https://www.redhat.com/en/about/privacy-policy" iconPosition="end" target="_blank" rel="noopener noreferrer" + aria-label="Red Hat Privacy Statement (opens in new tab)" >libs/chatbot/lib/hooks/use-message.ts (1)
92-154: Consider extracting SSE parsing logic for better maintainability.The SSE parsing logic is complex and could benefit from being extracted into a separate helper function for better testability and reusability.
Consider extracting the SSE parsing logic into a separate helper function:
// In a separate helpers file or at the top of this file const parseSSEMessage = (buffer: string): { events: StreamEvent[], remainder: string } => { const parts = buffer.split('\n\n'); const remainder = parts.pop() || ''; const events: StreamEvent[] = []; for (const part of parts) { const lines = part.split('\n'); let data = ''; for (const line of lines) { if (line.startsWith('data:')) { data += line.slice(5).trim() + '\n'; } } if (data) { try { events.push(JSON.parse(data) as StreamEvent); } catch (e) { console.error('Failed to parse SSE event:', e); } } } return { events, remainder }; };Then simplify the main loop to use this helper.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
libs/chatbot/lib/components/ChatBot/AIAlert.tsx(1 hunks)libs/chatbot/lib/components/ChatBot/BotMessage.tsx(3 hunks)libs/chatbot/lib/components/ChatBot/ChatBot.tsx(2 hunks)libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx(7 hunks)libs/chatbot/lib/components/ChatBot/ConfirmNewChatModal.tsx(2 hunks)libs/chatbot/lib/components/ChatBot/FeedbackCard.tsx(1 hunks)libs/chatbot/lib/components/ChatBot/helpers.ts(0 hunks)libs/chatbot/lib/hooks/use-message.ts(1 hunks)
💤 Files with no reviewable changes (1)
- libs/chatbot/lib/components/ChatBot/helpers.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/chatbot/lib/components/ChatBot/ConfirmNewChatModal.tsx
- libs/chatbot/lib/components/ChatBot/ChatBot.tsx
- libs/chatbot/lib/components/ChatBot/BotMessage.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 Learning: 2025-07-18T12:35:50.945Z
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:
libs/chatbot/lib/hooks/use-message.tslibs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx
🧬 Code Graph Analysis (3)
libs/chatbot/lib/components/ChatBot/AIAlert.tsx (1)
libs/ui-lib-tests/cypress/views/reusableComponents/Alert.ts (1)
Alert(1-20)
libs/chatbot/lib/hooks/use-message.ts (3)
libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx (1)
ChatBotWindowProps(26-38)libs/chatbot/lib/components/ChatBot/helpers.ts (4)
userRole(11-11)botRole(10-10)getToolAction(29-76)getErrorMessage(13-21)libs/chatbot/lib/components/ChatBot/types.ts (6)
StreamEvent(21-26)isEndStreamEvent(29-29)isStartStreamEvent(28-28)isInferenceStreamEvent(30-31)isToolArgStreamEvent(33-34)isToolResponseStreamEvent(36-37)
libs/chatbot/lib/components/ChatBot/FeedbackCard.tsx (2)
libs/chatbot/lib/components/ChatBot/BotMessage.tsx (1)
FeedbackRequest(14-18)libs/chatbot/lib/components/ChatBot/helpers.ts (1)
getErrorMessage(13-21)
🔇 Additional comments (11)
libs/chatbot/lib/components/ChatBot/FeedbackCard.tsx (10)
1-23: LGTM! Clean imports and dependencies.The imports are well-organized and all dependencies are properly imported from their respective modules.
34-45: LGTM! Well-structured component props and state management.The component props are properly typed and the state management using React hooks is appropriate for the feedback form functionality.
47-51: LGTM! Proper focus management for accessibility.The use of
useLayoutEffectwith a ref to focus the textarea on mount ensures good user experience and accessibility.
53-67: LGTM! Robust error handling and submission flow.The submit function properly handles the async operation with appropriate error handling and loading state management. The use of
getErrorMessagefor consistent error formatting is good practice.
69-84: LGTM! Well-structured card header with proper accessibility.The card header includes a proper title and close button with appropriate styling and accessibility considerations.
85-133: LGTM! Comprehensive form implementation with proper UX.The form structure is well-organized with:
- Proper PatternFly 6 components usage
- Controlled inputs for category selection and feedback text
- Appropriate disabled states during submission
- Good responsive design with full-width select toggle
135-141: LGTM! Appropriate error display with inline alerts.The error handling UI properly displays validation and submission errors using PatternFly's inline alert pattern.
144-156: LGTM! Proper submit button with loading states.The footer contains a well-implemented submit button with proper loading and disabled states during form submission.
159-159: LGTM! Proper default export.The component is properly exported as the default export, following React component conventions.
58-58: Negative feedback correctly uses sentiment = –1
The hardcodedsentiment: -1in FeedbackCard.tsx aligns with the “thumbs-down” flow (positive feedback usessentiment: 1in BotMessage.tsx), matching theFeedbackRequesttype and API expectations. No change required.libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx (1)
61-65: Good focus management implementation.The use of
useLayoutEffectto focus the message bar when the modal closes ensures a smooth user experience without visual flicker.
rawagner
force-pushed
the
chat_feedback
branch
from
31865ba to
a24518f
Compare
Extract remote calls to hook.
rawagner
force-pushed
the
chat_feedback
branch
from
a24518f to
3ed5f4f
Compare
|
/cherry-pick releases/v0.1-chatbot |
|
@rawagner: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[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 |
openshift-merge-bot
bot
merged commit 3c82686
into
openshift-assisted:master
|
@rawagner: new pull request created: #3111 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
3 participants
Extract remote calls to hook.
Summary by CodeRabbit
New Features
Improvements