fix: isNewChat regression #382
Conversation
Reviewer's Guide by SourceryThis pull request fixes a regression that prevented new chats from being properly initiated. It also updates the text for creating a new thread and adds an error notification for when sending messages fails. The primary change involves adjusting the conditions under which a new chat thread is created within the Sequence diagram for creating a new chat threadsequenceDiagram
participant User
participant MBChatProvider
participant getMetadataLabels
participant createThread
User->>MBChatProvider: Sends a message to the chatbot
activate MBChatProvider
MBChatProvider->>getMetadataLabels: Fetches chatbot metadata
activate getMetadataLabels
getMetadataLabels-->>MBChatProvider: Returns chatbot metadata
deactivate getMetadataLabels
alt isNewChat && chatbot
MBChatProvider->>createThread: Creates a new thread
activate createThread
createThread-->>MBChatProvider: Returns the new thread ID
deactivate createThread
end
MBChatProvider-->>User: Displays the chatbot's response
deactivate MBChatProvider
Updated class diagram for MBChatProviderclassDiagram
class MBChatProvider {
-messagesFromDB: Message[]
-isNewChat: boolean
+sendMessage(message: string): Promise<void>
+createThread(threadId: string, chatbotId: string, title: string): Promise<void>
}
note for MBChatProvider "The isInitLoaded and loadingState properties were removed."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces modifications to the chat functionality by refining messaging logic and streamlining state management. In the Changes
Possibly related PRs
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/masterbots.ai/lib/hooks/use-mb-chat.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/masterbots.ai/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. apps/masterbots.ai/components/routes/thread/thread-popup.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/masterbots.ai/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @AndlerRL - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a comment explaining why
activeThread?.messages.lengthwas removed from the conditional.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/masterbots.ai/components/routes/chat/chat.tsx (1)
123-124: Consider keeping the thread title in the message.The previous message provided more context by showing the thread title. The new static message "Continue this thread here" is less informative.
Consider reverting to the previous message or combining both:
-// return `Create new thread from "${threadTitle}" by making a new question.` -return 'Continue this thread here' +return `Continue thread "${threadTitle}" here`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/masterbots.ai/components/routes/chat/chat.tsx(2 hunks)apps/masterbots.ai/lib/hooks/use-mb-chat.tsx(4 hunks)
🔇 Additional comments (4)
apps/masterbots.ai/lib/hooks/use-mb-chat.tsx (4)
67-77: LGTM! State cleanup.The removal of
isInitLoadedfrom state is a good cleanup as it's not used elsewhere in the code.
277-278: LGTM! Clear comment.The added comment clearly explains the purpose of the useEffect hook.
551-551: LGTM! Improved error message.The error message is now more specific about the failure being related to sending a message to the Masterbot.
486-494:✅ Verification successful
Verify thread creation logic.
The condition for creating a new thread has been simplified to only check
isNewChat. Please verify that this doesn't miss any edge cases that were previously caught by checkingactiveThread?.messages.length.Run this script to check thread creation patterns:
🏁 Script executed:
#!/bin/bash # Description: Search for thread creation patterns to verify the change is safe # Search for thread creation conditions rg -A 5 "createThread\(" # Search for isNewChat usage rg -A 5 "isNewChat"Length of output: 12697
Thread Creation Logic Verified: No Edge Cases Missed
After reviewing the code and verifying the output of the search scripts, it appears that the
isNewChatstate is correctly derived from the absence of messages inactiveThread(i.e., usingBoolean(!activeThread?.messages.length)). This means that the previous edge case—where an empty message list was explicitly checked—is now implicitly handled byisNewChat. The thread creation inapps/masterbots.ai/lib/hooks/use-mb-chat.tsxnow safely relies on this state.No further modifications seem necessary. Please ensure that integration tests continue to cover these scenarios for future changes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/masterbots.ai/components/routes/thread/thread-popup.tsx (1)
45-57: Verify the impact of removing auto-scroll functionality.The commented-out useEffect hook was responsible for automatically scrolling to the bottom when new messages load. While manual scrolling is still available through the Chat component, verify that this change doesn't negatively impact the user experience during active chats.
Consider keeping the auto-scroll functionality but with these improvements:
- // Uses smoothScrollToBottom from custom hook useMBScroll - // biome-ignore lint/correctness/useExhaustiveDependencies: smoothScrollToBottom might be necessary however, it has his own memoization (useCallback). That should be enough, else, can be added as a dependency - useEffect(() => { - let timeout: NodeJS.Timeout - if (isLoading && isOpenPopup) { - timeout = setTimeout(() => { - smoothScrollToBottom() - }, 150) - } - return () => { - clearTimeout(timeout) - } - }, [isLoading, isOpenPopup]) + useEffect(() => { + if (isLoading && isOpenPopup && !isNearBottom) { + const timeout = setTimeout(smoothScrollToBottom, 150) + return () => clearTimeout(timeout) + } + }, [isLoading, isOpenPopup, isNearBottom, smoothScrollToBottom])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/masterbots.ai/components/routes/thread/thread-popup.tsx(1 hunks)apps/masterbots.ai/lib/hooks/use-mb-chat.tsx(5 hunks)
🔇 Additional comments (4)
apps/masterbots.ai/lib/hooks/use-mb-chat.tsx (4)
67-78: LGTM! State management improvements.The state structure has been simplified by:
- Removing unnecessary
isInitLoadedstate- Deriving
isNewChatdirectly fromactiveThread.messages.length- Using a more explicit boolean conversion
552-552: LGTM! Improved error message clarity.The error message has been made more specific and user-friendly, clearly indicating that the message failed to send to the Masterbot.
278-286: Verify chat reset behavior.The useEffect hook now includes a clarifying comment and resets the chat state when there's no active thread and the popup is closed. Verify that this doesn't cause any unintended state resets during normal chat interactions.
487-495:✅ Verification successful
Verify the simplified thread creation condition.
The thread creation condition has been simplified to only check
isNewChat. Verify that this doesn't cause any edge cases where threads might be created unnecessarily or not created when needed.
🏁 Script executed:
#!/bin/bash # Search for other thread creation conditions in the codebase ast-grep --pattern 'createThread({ $$$ })'Length of output: 1314
Thread creation condition verified
The simplified condition in
use-mb-chat.tsx—checking onlyisNewChat && chatbot—appears consistent with the thread creation logic used elsewhere (e.g., innew-chat.tsx). The conditional construction still accounts for continuous threads through theisContinuousThread ? activeThread?.threadId : undefinedhandling and the public check viaactiveChatbot?.name !== 'BlankBot'. Based on the codebase search, no edge cases were identified that would result in unnecessary thread creation or omission, so the change seems both intentional and maintainable.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/masterbots.ai/lib/hooks/use-mb-chat.tsx (3)
541-541: Enhance error message clarity.The error message has been improved to be more specific about the failure point (Masterbot). However, consider adding error details for debugging in development mode.
- customSonner({ type: 'error', text: 'Failed to send the message to the Masterbot. Please try again.' }) + customSonner({ + type: 'error', + text: 'Failed to send the message to the Masterbot. Please try again.', + ...(appConfig.features.devMode && { description: error.message }) + })
488-499: Potential memory optimization for chat messages.The
uniqByoperation on system prompts could be memoized since these messages don't change frequently.+ const memoizedSystemPrompts = useMemo(() => uniqBy( + [ + ...systemPrompts, + { + id: 'examples-' + nanoid(10), + role: 'system' as 'data' | 'system' | 'user' | 'assistant', + content: examplesPrompt(chatbotMetadata), + }, + ], + 'content', + ), [systemPrompts, chatbotMetadata]) + - const chatMessagesToAppend = uniqBy( - [ - ...systemPrompts, - { - id: 'examples-' + nanoid(10), - role: 'system' as 'data' | 'system' | 'user' | 'assistant', - content: examplesPrompt(chatbotMetadata), - }, - ], - 'content', - ) + const chatMessagesToAppend = memoizedSystemPrompts
519-533: Consider adding retry mechanism for append operations.The
appendoperation could benefit from a retry mechanism to handle transient failures, especially since it's a critical operation for chat functionality.+ const MAX_RETRIES = 3; + const RETRY_DELAY = 1000; + let retries = 0; + let appendResponse = null; + + while (retries < MAX_RETRIES) { + try { const appendResponse = await append( { ...userMessage, content: isNewChat ? userContentRef.current : followingQuestionsPrompt( userContentRef.current, previousAiUserMessages.concat(userMessages), ), }, ) + break; + } catch (error) { + retries++; + if (retries === MAX_RETRIES) throw error; + await new Promise(resolve => setTimeout(resolve, RETRY_DELAY)); + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/masterbots.ai/components/routes/thread/thread-popup.tsx(2 hunks)apps/masterbots.ai/lib/hooks/use-mb-chat.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/masterbots.ai/components/routes/thread/thread-popup.tsx
🔇 Additional comments (3)
apps/masterbots.ai/lib/hooks/use-mb-chat.tsx (3)
67-78: Improved state initialization logic.The initialization of
isNewChatnow correctly derives from the thread's message length, making it more reliable and consistent with the actual state of the conversation.
267-275: Clear documentation of chat reset behavior.The added comment clearly explains the purpose of resetting the chat when the popup is closed, improving code maintainability.
476-484:❓ Verification inconclusive
Verify thread creation logic.
The condition for creating a new thread has been simplified to only check
isNewChat. However, we should verify that this doesn't create unnecessary threads.
🏁 Script executed:
#!/bin/bash # Search for other thread creation conditions in the codebase rg "createThread\(" -A 5Length of output: 1548
Thread creation condition appears consistent but please double-check parameter differences.
After searching for all calls to
createThread, we confirmed that in the hook (apps/masterbots.ai/lib/hooks/use-mb-chat.tsx, lines 476–484) the thread is only created when bothisNewChatand a validchatbotare present. This simplified condition is in line with the similar call inapps/masterbots.ai/components/routes/chat/new-chat.tsx—except that the latter also provides auserId. No evidence was found that this simplified logic is causing duplicate or unnecessary thread creations. Please verify that omittinguserIdin this hook is intentional based on your requirements.
… regression + followingQuestionPrompt (#383) * [masterbots.ai] fix: isNewChat regression (#382) * impr: continue thread placeholder * fix: isNewChat regression * fix: isNewChat state * impr: error toaster at appendNewMessage * style: console.log cleanup * impr: initIsNewChat guard * impr: thread-popup scrolldown * impr: scrollToBottomPopup + onFinish cb * chore: enable onFinished and appendingNewMessage logs + env var openai config * fix: typo * fix: typo * fix: createThread condition * impr: isnewChat guard * impr: isnewChat guard * fix: ts typo * fix: loaders + getAllUserMessagesAsStringArray cleanup * impr: chat panel a11y
Summary by Sourcery
Addresses an issue where new chats were not starting correctly and enhances error handling and chat state management. The changes ensure that new threads are properly created and that users receive informative error messages when message sending fails.
Bug Fixes:
Enhancements:
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Chatcomponent for clarity.