feat: add SIWA feedback functionality to dashboard chat #7272
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
""" WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CustomChats
participant CustomChatContent
participant BackendAPI
User->>CustomChats: Clicks feedback (thumbs up/down) on assistant message
CustomChats->>CustomChatContent: Calls onFeedback callback with message, rating
CustomChatContent->>BackendAPI: POST /v1/chat/feedback with feedback data
BackendAPI-->>CustomChatContent: Returns success/failure
CustomChatContent->>CustomChats: Updates message feedback state
CustomChats-->>User: Disables feedback buttons, updates UI
Possibly related PRs
Suggested reviewers
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7272 +/- ##
=======================================
Coverage 55.58% 55.58%
=======================================
Files 909 909
Lines 58670 58670
Branches 4158 4158
=======================================
Hits 32609 32609
Misses 25954 25954
Partials 107 107
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChats.tsx (1)
257-260: Consider showing feedback submission state to users.Instead of hiding the buttons after feedback is submitted, consider showing a visual indicator that feedback was received (e.g., highlighting the selected option or showing a "Thank you" message).
apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChatContent.tsx (1)
212-212: Optimize useCallback dependencies.Including
messagesin the dependency array causes the callback to be recreated on every message update. Since the function only needs the message at a specific index during execution, consider removing this dependency.- [sessionId, props.authToken, props.teamId, trackEvent, messages], + [sessionId, props.authToken, props.teamId, trackEvent],Then access messages directly within the function using the functional setState pattern:
- const message = messages[messageIndex]; + // Get current message inside setState + setMessages((prev) => { + const message = prev[messageIndex]; + // ... validation logic + return prev.map((msg, index) => + index === messageIndex && msg.type === "assistant" + ? { ...msg, feedback } + : msg + ); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChatContent.tsx(5 hunks)apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChats.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChats.tsx (1)
321-326: Excellent TypeScript exhaustiveness checking!The use of
nevertype ensures that all message types are handled at compile time, preventing runtime errors from unhandled cases.
apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChats.tsx
Show resolved
Hide resolved
| } catch (_e) { | ||
| // Handle error silently | ||
| } finally { | ||
| setIsSubmitting(false); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling to provide user feedback.
Currently, errors during feedback submission are silently caught without notifying the user. Consider showing a toast notification or inline error message when feedback submission fails.
- } catch (_e) {
- // Handle error silently
+ } catch (error) {
+ console.error('Failed to submit feedback:', error);
+ // Consider showing user-facing error notification📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (_e) { | |
| // Handle error silently | |
| } finally { | |
| setIsSubmitting(false); | |
| } | |
| } catch (error) { | |
| console.error('Failed to submit feedback:', error); | |
| // Consider showing user-facing error notification | |
| } finally { | |
| setIsSubmitting(false); | |
| } |
🤖 Prompt for AI Agents
In apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChats.tsx
around lines 250 to 254, the catch block for feedback submission errors is
empty, causing silent failures. Modify the catch block to capture the error and
display a user-friendly notification, such as a toast message or inline error,
informing the user that the feedback submission failed. This will improve user
experience by providing clear feedback on errors.
| console.error("Failed to send feedback:", error); | ||
| // Optionally show user-facing error notification | ||
| // Consider implementing retry logic here | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Provide user-facing feedback for errors.
Errors are only logged to console. Users should be notified when feedback submission fails.
Consider implementing a toast notification system or showing an inline error message to inform users when feedback submission fails. This improves the user experience by providing transparency about the operation status.
🤖 Prompt for AI Agents
In
apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChatContent.tsx
around lines 207 to 210, the code only logs errors to the console when feedback
submission fails, without notifying the user. To fix this, add a user-facing
error notification such as a toast message or an inline error display to inform
users about the failure. Integrate this notification within the existing error
handling block to improve user experience by providing clear feedback on the
operation status.
apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChatContent.tsx
Show resolved
Hide resolved
apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChatContent.tsx
Show resolved
Hide resolved
size-limit report 📦
|
apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChats.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChats.tsx
Outdated
Show resolved
Hide resolved
Merge activity
|
<!--
## title your PR with this format: "[SDK/Dashboard/Portal] Feature/Fix: Concise title for the changes"
If you did not copy the branch name from Linear, paste the issue tag here (format is TEAM-0000):
## Notes for the reviewer
Anything important to call out? Be sure to also clarify these in your comments.
## How to test
Unit tests, playground, etc.
-->
<!-- start pr-codex -->
---
## PR-Codex overview
This PR focuses on refactoring the `CustomChatContent` and `CustomChats` components to improve type safety and functionality in handling user messages and feedback. It introduces new types and methods for managing chat messages and feedback submission.
### Detailed summary
- Replaced `NebulaUserMessage` with `UserMessage` and `CustomChatMessage`.
- Added `handleFeedback` function for submitting feedback on messages.
- Updated `CustomChats` component to handle new message types and feedback.
- Modified `sendMessage` logic in `ChatBar` to use the new message structure.
- Introduced `RenderMessage` and `StyledMarkdownRenderer` for rendering chat messages.
- Added feedback buttons for assistant messages in `RenderMessage`.
> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`
<!-- end pr-codex -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
- **New Features**
- Introduced a new chat interface supporting multiple message types with distinct visual styles.
- Enabled users to submit feedback (thumbs up/down) on assistant messages.
- Enhanced chat experience with auto-scrolling that respects user interactions.
- **Bug Fixes**
- Prevented duplicate feedback submissions on assistant messages.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
graphite-app
bot
force-pushed
the
yash/add-siwa-feedback
branch
from
8a2e633 to
d4aed43
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChats.tsx (2)
112-113: 🛠️ Refactor suggestionConsider using a stable unique identifier instead of array index as key.
Using array index as the key can lead to React reconciliation issues if messages are reordered or deleted. Consider adding a unique
idfield to each message or generating a stable key based on message content and timestamp.
241-245: 🛠️ Refactor suggestionImprove error handling to provide user feedback.
Currently, errors during feedback submission are silently caught without notifying the user. Consider showing a toast notification or inline error message when feedback submission fails.
- } catch (_e) { - // Handle error silently + } catch (error) { + console.error('Failed to submit feedback:', error); + // Consider showing user-facing error notification
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChatContent.tsx(11 hunks)apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChats.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChatContent.tsx
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChats.tsx (3)
14-36: Type definitions are well-structured and type-safe.The discriminated union for
CustomChatMessageprovides excellent type safety, and the feedback type using1 | -1is appropriate for thumbs up/down ratings.
312-318: Excellent use of exhaustive type checking.The default case with
nevertype ensures TypeScript will catch any unhandled message types at compile time. This is a great pattern for maintaining type safety as the codebase evolves.
327-328: Good security practice with skipHtml.Using
skipHtmlprevents potential XSS attacks through malicious HTML in markdown content, which is essential for user-generated content.
| // biome-ignore lint/suspicious/noArrayIndexKey: <explanation> | ||
| <div className="flex justify-end" key={index}> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace array index key with stable identifier.
Similar to the main messages loop, using array index as key for user message content can cause reconciliation issues.
- // biome-ignore lint/suspicious/noArrayIndexKey: <explanation>
- <div className="flex justify-end" key={index}>
+ <div className="flex justify-end" key={`${index}-${msg.text.slice(0, 50)}`}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // biome-ignore lint/suspicious/noArrayIndexKey: <explanation> | |
| <div className="flex justify-end" key={index}> | |
| <div className="flex justify-end" key={`${index}-${msg.text.slice(0, 50)}`}> |
🤖 Prompt for AI Agents
In apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChats.tsx
at lines 156-157, replace the use of the array index as the key in the user
message content div with a stable and unique identifier from the message data,
such as a message ID or timestamp, to avoid reconciliation issues during
rendering.
| useEffect(() => { | ||
| if (!enableAutoScroll) { | ||
| return; | ||
| } | ||
|
|
||
| const chatScrollContainer = | ||
| chatContainerRef.current?.querySelector("[data-scrollable]"); | ||
|
|
||
| if (!chatScrollContainer) { | ||
| return; | ||
| } | ||
|
|
||
| const disableScroll = () => { | ||
| setEnableAutoScroll(false); | ||
| chatScrollContainer.removeEventListener("mousedown", disableScroll); | ||
| chatScrollContainer.removeEventListener("wheel", disableScroll); | ||
| }; | ||
|
|
||
| chatScrollContainer.addEventListener("mousedown", disableScroll); | ||
| chatScrollContainer.addEventListener("wheel", disableScroll); | ||
| }, [setEnableAutoScroll, enableAutoScroll]); |
There was a problem hiding this comment.
Add cleanup for event listeners to prevent memory leaks.
The useEffect adds event listeners but doesn't clean them up when the component unmounts or when dependencies change, which can cause memory leaks.
// eslint-disable-next-line no-restricted-syntax
useEffect(() => {
if (!enableAutoScroll) {
return;
}
const chatScrollContainer =
chatContainerRef.current?.querySelector("[data-scrollable]");
if (!chatScrollContainer) {
return;
}
const disableScroll = () => {
setEnableAutoScroll(false);
- chatScrollContainer.removeEventListener("mousedown", disableScroll);
- chatScrollContainer.removeEventListener("wheel", disableScroll);
};
chatScrollContainer.addEventListener("mousedown", disableScroll);
chatScrollContainer.addEventListener("wheel", disableScroll);
+
+ return () => {
+ chatScrollContainer.removeEventListener("mousedown", disableScroll);
+ chatScrollContainer.removeEventListener("wheel", disableScroll);
+ };
}, [setEnableAutoScroll, enableAutoScroll]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (!enableAutoScroll) { | |
| return; | |
| } | |
| const chatScrollContainer = | |
| chatContainerRef.current?.querySelector("[data-scrollable]"); | |
| if (!chatScrollContainer) { | |
| return; | |
| } | |
| const disableScroll = () => { | |
| setEnableAutoScroll(false); | |
| chatScrollContainer.removeEventListener("mousedown", disableScroll); | |
| chatScrollContainer.removeEventListener("wheel", disableScroll); | |
| }; | |
| chatScrollContainer.addEventListener("mousedown", disableScroll); | |
| chatScrollContainer.addEventListener("wheel", disableScroll); | |
| }, [setEnableAutoScroll, enableAutoScroll]); | |
| useEffect(() => { | |
| if (!enableAutoScroll) { | |
| return; | |
| } | |
| const chatScrollContainer = | |
| chatContainerRef.current?.querySelector("[data-scrollable]"); | |
| if (!chatScrollContainer) { | |
| return; | |
| } | |
| const disableScroll = () => { | |
| setEnableAutoScroll(false); | |
| }; | |
| chatScrollContainer.addEventListener("mousedown", disableScroll); | |
| chatScrollContainer.addEventListener("wheel", disableScroll); | |
| return () => { | |
| chatScrollContainer.removeEventListener("mousedown", disableScroll); | |
| chatScrollContainer.removeEventListener("wheel", disableScroll); | |
| }; | |
| }, [setEnableAutoScroll, enableAutoScroll]); |
🤖 Prompt for AI Agents
In apps/dashboard/src/app/nebula-app/(app)/components/CustomChat/CustomChats.tsx
around lines 67 to 87, the useEffect adds event listeners to chatScrollContainer
but does not remove them on cleanup, risking memory leaks. Fix this by returning
a cleanup function from the useEffect that removes the "mousedown" and "wheel"
event listeners from chatScrollContainer when the component unmounts or
dependencies change.
PR-Codex overview
This PR focuses on enhancing the
CustomChatfunctionality by introducing new message types and feedback mechanisms, while refactoring existing message handling to improve clarity and maintainability.Detailed summary
NebulaUserMessageandChatMessagetypes.UserMessage,UserMessageContent, andCustomChatMessagetypes.ChatMessagetoCustomChatMessage.handleSendMessageto utilize new message types.handleFeedbackto submit user feedback on messages.Chatscomponent withCustomChatsfor improved message rendering.sendMessageprop inChatBarto constructUserMessage.RenderMessage,RenderResponse, andStyledMarkdownRenderercomponents for better message display.Summary by CodeRabbit
New Features
Bug Fixes