fix: continuous thread #386
Conversation
Reviewer's Guide by SourceryThis pull request addresses an issue with continuous threads, ensuring proper thread handling and state management. It also includes improvements to error handling, thread visibility updates, and UI enhancements. Sequence diagram for appending a new message in a continuous threadsequenceDiagram
participant User
participant MBChatProvider
participant createThread
participant append
participant saveNewMessage
participant setState
User->>MBChatProvider: Sends a new message
activate MBChatProvider
MBChatProvider->>createThread: Calls createThread if it's a new chat or continuing thread
activate createThread
createThread-->>MBChatProvider: Returns
deactivate createThread
MBChatProvider->>append: Calls append with the user message and previous messages
activate append
append-->>MBChatProvider: Returns appendResponse
deactivate append
MBChatProvider->>saveNewMessage: Saves user and assistant messages
activate saveNewMessage
saveNewMessage-->>MBChatProvider: Returns
deactivate saveNewMessage
MBChatProvider->>setState: Updates the state (isNewChat to false)
activate setState
setState-->>MBChatProvider: Returns
deactivate setState
MBChatProvider-->>User: Acknowledges message
deactivate MBChatProvider
Updated class diagram for ThreadclassDiagram
class Thread {
threadId: string
chatbot: Chatbot
chatbotId: string
createdAt: Date
isApproved: boolean
isPublic: boolean
messages: Message[]
userId: string
updatedAt: Date
isBlocked: boolean
model: string
user: User | null
thread: Thread | null
parentThreadId: string | null
threads: Thread[]
}
note for Thread "Added thread and parentThreadId attributes"
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 makes several changes across multiple files. It reorganizes and standardizes formatting in the layout, updates conditional rendering and styling in chat components, refines the URL and event handling logic in chat options, and introduces badge-based status display in thread components. Additionally, it improves continuous thread management with a new helper function, adjusts chat state management and asynchronous error handling in hooks, and renames plus modifies the mutation function in the service layer. 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/threads.tsOops! 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. ✨ Finishing Touches
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 the purpose of the
isContinuousThreadvariable and how it affects the chat behavior. - The
useEffecthook that updates the thread ID when the popup is closed could be simplified by moving the random thread ID generation directly into theuseEffect's callback.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 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: 2
🧹 Nitpick comments (12)
apps/masterbots.ai/lib/hooks/use-mb-chat.tsx (4)
28-28: Use of lodash'suniqByis acceptable.
This import helps ensure messages and prompts are de-duplicated. If bundle size or dependency reduction becomes a priority, consider using native JavaScript structures to handle duplicates.
75-75: Remove commented-out code if not needed.
Leaving commented code can lead to confusion and clutter. Consider removing the leftoverinitialIsNewChatline if no longer relevant.
165-169: Development-mode logging.
IncludingcustomSonnernotifications for dev debugging is helpful. Ensure no sensitive data is exposed in production.
311-320:updateNewThreadlogic.
Checks if there are no messages to mark the thread as new. Consider removing leftoverconsole.logcalls if not needed for future debugging.apps/masterbots.ai/app/layout.tsx (2)
53-54: Ensuring alt text is consistent.
alt: 'Masterbots'is fine for accessibility, though more descriptive alt text can sometimes be beneficial.
69-70: Twitter image alt text.
Same remark: consider more descriptive alt text if needed for better SEO and assistive technologies.apps/masterbots.ai/components/routes/thread/thread-component.tsx (2)
86-99: Consider enhancing accessibility and type safety.The badge implementation could benefit from some improvements:
- Add aria-label to the Badge for screen readers
- Consider using a type-safe enum for routeType instead of string literal
-const routeType = getRouteType(pathname) +type RouteType = 'chat' | 'browse' | 'profile' +const routeType = getRouteType(pathname) as RouteType -<Badge +<Badge + aria-label={`Thread visibility status: ${thread.isPublic ? 'Public' : 'Private'}`}
90-96: Consider extracting badge styles to a constant.The conditional styling logic could be more maintainable if extracted to a constant or utility function.
+const getBadgeStyles = (isApproved: boolean, isPublic: boolean) => cn({ + 'bg-[#BE17E8] text-white': isApproved && isPublic, + 'bg-[#09090B] text-white': isApproved && !isPublic, +}) -className={cn({ - 'bg-[#BE17E8] text-white': thread.isApproved && thread.isPublic, - 'bg-[#09090B] text-white': thread.isApproved && !thread.isPublic, -})} +className={getBadgeStyles(thread.isApproved, thread.isPublic)}apps/masterbots.ai/components/routes/chat/chat-options.tsx (1)
43-43: Consider using URL constructor for safer URL handling.Using string concatenation for URLs can be error-prone. Consider using URL constructor or path utilities.
-const url = `/b/${toSlug(thread.chatbot.categories[0].category.name)}/${thread.threadId}` +const url = new URL( + `/b/${toSlug(thread.chatbot.categories[0].category.name)}/${thread.threadId}`, + window.location.origin +).toString()apps/masterbots.ai/lib/hooks/use-thread-visibility.tsx (2)
70-82: Consider adding type for the update response.The updateThreadResponse structure should be typed for better type safety.
+interface ThreadVisibilityUpdateResponse { + success: boolean; + error?: string; +} const toggleVisibility = async (newIsPublic: boolean, threadId: string) => { try { - const updateThreadResponse = await updateThreadVisibility({ isPublic: newIsPublic, threadId, jwt }) + const updateThreadResponse: ThreadVisibilityUpdateResponse = await updateThreadVisibility({ + isPublic: newIsPublic, + threadId, + jwt + })
78-81: Consider more specific error handling.The error handling could be more specific to help with debugging and user feedback.
} catch (error) { - console.error('Failed to update thread visibility:', error) - customSonner({ type: 'error', text: 'Failed to update the thread visibility. Try again later.' }) + const errorMessage = error instanceof Error ? error.message : 'Unknown error occurred' + console.error('Failed to update thread visibility:', errorMessage) + customSonner({ + type: 'error', + text: `Failed to update thread visibility: ${errorMessage}. Please try again.` + }) }apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx (1)
153-164: Consider adding loading state for continuous thread fetching.The async operation for fetching continuous thread should indicate loading state to improve UX.
+const [isFetchingContinuousThread, setIsFetchingContinuousThread] = useState(false) useAsync(async () => { if (!session) return + setIsFetchingContinuousThread(true) if (!continuousThreadId && isContinuousThread) { setIsContinuousThread(false) + setIsFetchingContinuousThread(false) return } if (!continuousThreadId) return await getThreadByContinuousThreadId(continuousThreadId, session) + setIsFetchingContinuousThread(false) }, [session])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/masterbots.ai/app/layout.tsx(4 hunks)apps/masterbots.ai/components/routes/chat/chat-list.tsx(6 hunks)apps/masterbots.ai/components/routes/chat/chat-options.tsx(3 hunks)apps/masterbots.ai/components/routes/thread/thread-component.tsx(3 hunks)apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx(2 hunks)apps/masterbots.ai/components/ui/badge.tsx(2 hunks)apps/masterbots.ai/lib/hooks/use-mb-chat.tsx(15 hunks)apps/masterbots.ai/lib/hooks/use-thread-visibility.tsx(3 hunks)apps/masterbots.ai/services/hasura/hasura.service.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
apps/masterbots.ai/lib/hooks/use-thread-visibility.tsx (1)
Learnt from: AndlerRL
PR: bitcashorg/masterbots#287
File: apps/masterbots.ai/lib/hooks/use-thread-visibility.tsx:3-3
Timestamp: 2024-11-12T03:32:08.432Z
Learning: Always use explicit imports for React hooks to maintain consistency across the monorepo, and avoid using both named and default imports simultaneously.
🔇 Additional comments (34)
apps/masterbots.ai/components/routes/chat/chat-list.tsx (4)
171-189: Improved message pair rendering with better null checks and loading state.The changes enhance the robustness of the chat rendering by:
- Preventing incomplete message pairs from being rendered
- Showing loading state only for the last message when appropriate
219-224: Well-structured accordion state logic with clear documentation.The defaultState conditions are well-organized and clearly documented, making the code more maintainable. Each case is properly handled:
- First message in conversation
- Last message in current conversation
- First message in previous conversation
240-241: Enhanced visual distinction for previous thread messages.The styling updates improve the user experience by:
- Providing clear visual feedback with hover and active states
- Maintaining consistent opacity levels for better readability
Also applies to: 295-295
111-111:✅ Verification successful
Verify scroll behavior after removing overflow classes.
The removal of
overflow-auto scrollbar-thinclasses might affect content overflow handling. Please ensure that:
- The chat container still scrolls properly with long content
- The scrollbar is still visible and usable
🏁 Script executed:
#!/bin/bash # Search for overflow handling in parent components or global styles rg -g '*.{css,scss,tsx,jsx}' -A 3 'overflow-auto|scrollbar|scroll' .Length of output: 38529
Scroll Behavior Verification and Confirmation
The removal of the
overflow-auto scrollbar-thinclasses on line 111 ofapps/masterbots.ai/components/routes/chat/chat-list.tsxappears intentional. Our search shows that scroll behavior is managed globally (via CSS inapp/globals.css) and within parent containers (e.g., inchat-layout-section.tsx). Therefore, the scrolling functionality is expected to be inherited by the chat container.Please ensure that:
- The chat container still scrolls properly when content overflows.
- The scrollbar remains visible and user-friendly across different viewports.
apps/masterbots.ai/lib/hooks/use-mb-chat.tsx (16)
38-38: Parameters hook usage looks fine.
Ensure fallback or error handling is in place whenthreadIdand other parameters are absent in the URL.
67-69: Integration of new hooks is consistent.
CombiningisContinuousThread,customSonner, andisPowerUpfrom different hooks is clear. No immediate issues found.
72-73: Parameter and model usage.
AccessingthreadIdand destructuringselectedModel,clientTypeis a straightforward approach. Looks good.
85-85: Double-check forcedisNewChat: true.
Setting it unconditionally to true may override cases whereisNewChatshould remain false (e.g., partial conversation exist). Confirm that always starting fresh is intended.
116-116: Thread ID assignment logic.
UsingrandomThreadIdfor continuous threads can be valid. Confirm it won't accidentally overwrite existing thread references.
131-131: Use ofuseSearchParamsis fine.
No immediate concerns. Keep an eye on frequent param updates potentially triggering re-renders.
170-175:resolveThreadIdusage is logical.
Dynamically resolvingthreadIdbased onisContinuousThreadis consistent with the new approach.
177-186: Deleting new thread on error.
If creation fails, the thread is removed. Consider whether partial content or logs need preservation before deletion.
189-209: Simultaneous save for user and assistant messages.
UsingPromise.allto save messages is efficient. Confirm the backend handles rapid consecutive calls safely.
211-221: Transitioning from continuous to normal thread.
RemovingcontinuousThreadIdand settingisNewChatto false is correct. Verify that removing the param mid-session aligns with user expectations.
226-236: Error handling mirrors the success path.
Similar deletion logic on error. Confirm no partial data is left if concurrency issues occur (e.g., multiple errors at once).
295-295:useEffectdependency expanded tosearchParams.
This re-run on query param changes is valid but watch for potential infinite re-renders if the params are updated in quick succession.
299-300: Cleanup on unmount is straightforward.
Resetting state upon unmount helps avoid stale references or memory leaks.
327-327: Fetch with continuous thread condition.
Similar to earlier logic. Ensure the server can handle two references to the same thread if toggled quickly.
334-334: SettingisNewChatfrom empty messages.
Verifying both local and remote message arrays is sensible. Watch for edge cases if the arrays go out of sync.
495-497: Hydration edge cases.
UsingURLSearchParamsonwindow.location.searchpost-mount can differ from Next.js SSR. Test thoroughly to avoid mismatches.apps/masterbots.ai/components/ui/badge.tsx (2)
1-1: Refined import statement.
Switching to named imports (VariantProps, cva) tidies the code. No issues found.
16-16: Check color contrast for newoutlinevariant.
Applying'text-muted-foreground/50 bg-border'can reduce readability for certain themes or on certain screens. Verify accessibility.apps/masterbots.ai/app/layout.tsx (10)
4-4:<Toaster />import is valid.
No issues with thesonnertoaster integration here.
7-8: Font imports are well-structured.
Using thegeistfonts for Mono and Sans is neat. Ensure fallback fonts are also set if needed.
9-9: Type import for Metadata is proper.
This helps keep the import purely for type information, improving clarity.
15-15: Consolidated body class setup.
Usingcnutility withGeistSans.variableandGeistMono.variableis straightforward and readable.
16-16:NextTopLoaderposition.
Lowering theinitialPositionto0.2is fine, just verify it aligns with the intended design.
17-21: Global toaster configuration.
CustomizingtoastOptionsfor consistent theming is good. No concerns here.
39-39: Dynamic title template.
%s - Masterbotsis standard. No issues unless further customization is needed.
75-75: Apple icon usage.
No issue. Just confirm the file path is correct for all device sizes (e.g., iPad, iPhone).
78-78: Google site verification.
No concerns, as long as the key is correct. It won't affect runtime performance.
85-86: Dark theme color specification.
color: 'black'for dark scheme is a valid pairing. Confirm design contrast if device theme is toggled.apps/masterbots.ai/services/hasura/hasura.service.ts (2)
741-766: LGTM! Function name now follows TypeScript conventions.The changes look good:
- Function name changed from PascalCase to camelCase
- Mutation simplified to only handle visibility status
- Proper error handling with TypeScript types
741-766:✅ Verification successful
Verify the impact of removing
isApprovedfrom the mutation.The mutation now only updates
isPublicand no longer setsisApproved. Please verify that:
- Thread approval is handled separately
- No unintended side effects on thread visibility workflow
Let's check the usage of this function and the separation of concerns:
🏁 Script executed:
#!/bin/bash # Description: Check usage of updateThreadVisibility and thread approval logic # Search for function usage echo "Searching for updateThreadVisibility usage..." rg "updateThreadVisibility" # Search for thread approval logic echo "Searching for thread approval logic..." rg "isApproved.*thread"Length of output: 4586
Impact Verification on
updateThreadVisibilityMutationAfter reviewing the code and usage:
• TheupdateThreadVisibilityfunction is only used to toggleisPublic(as seen inapps/masterbots.ai/lib/hooks/use-thread-visibility.tsx).
• The thread approval logic is separately handled in the UI (e.g., the conditional rendering and theAdminModeApprovecomponent inapps/masterbots.ai/components/routes/thread/thread-component.tsx).
• The schema still supports theisApprovedfield, confirming that its update is expected to be managed elsewhere.It appears that removing the update to
isApprovedfrom this mutation is deliberate and aligns with a separation of concerns, ensuring that thread visibility and approval are handled independently.
apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx (1)
324-345: 🛠️ Refactor suggestionEnhance type safety in initialThread function.
The function uses unsafe type assertion. Consider using proper typing as suggested in previous reviews.
-export function initialThread(thread: Thread, session: Session): Thread { +export function initialThread(thread: Thread, session: Session): Omit<Thread, 'threadId'> & { threadId: string } { return { threadId: '', chatbot: thread.chatbot, chatbotId: thread.chatbotId, createdAt: new Date(), isApproved: false, isPublic: false, - messages: [thread.messages.filter(msg => msg.role === 'user')[0]], + messages: thread.messages.filter(msg => msg.role === 'user').slice(0, 1), userId: session.user?.id, updatedAt: new Date(), isBlocked: false, model: 'OPENAI', user: null, thread, parentThreadId: thread.threadId, threads: [], - } as unknown as Thread + } }
🧹 Nitpick comments (2)
apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx (2)
50-51: Consider breaking down the component into smaller, focused hooks.The TODO comment correctly identifies that this file handles too many responsibilities. Consider extracting the following functionalities into separate custom hooks:
- Thread fetching logic (
useThreadFetching)- Search functionality (
useThreadSearch)- Continuous thread handling (
useContinuousThread)
143-150: Remove commented-out code.The commented line
// setActiveThread(thread)should be removed as it's been replaced with the new implementation usinginitialThread.if (thread) { const defaultThread = initialThread(thread, session) - // ? here we can replace the active thread to appear as it is form a continuing thread with the thread parameter setActiveThread(defaultThread) - // setActiveThread(thread) setIsContinuousThread(true) setIsOpenPopup(true) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/masterbots.ai/components/routes/chat/chat-options.tsx(3 hunks)apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/masterbots.ai/components/routes/chat/chat-options.tsx
🔇 Additional comments (1)
apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx (1)
153-164: Add braces to if statements for consistency.Previous reviews have flagged the need for braces in if statements. Let's maintain consistent code style.
useAsync(async () => { - if (!session) return + if (!session) { + return + } - if (!continuousThreadId && isContinuousThread) { + if (!continuousThreadId && isContinuousThread) { setIsContinuousThread(false) return } - if (!continuousThreadId) return + if (!continuousThreadId) { + return + } await getThreadByContinuousThreadId(continuousThreadId, session) }, [session])
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/masterbots.ai/lib/threads.ts (2)
31-34: LGTM! Consider adding error handling for edge cases.The Set implementation effectively ensures message uniqueness. However, the String conversion might need additional error handling.
Consider adding null/undefined checks:
- const uniqueCleanMessages = Array.from(new Set(cleanMessages.map(String))) + const uniqueCleanMessages = Array.from(new Set(cleanMessages + .filter(msg => msg != null) + .map(String)))
28-29: Extract the marker text as a constant.The hard-coded marker text could be moved to a constant for better maintainability and reusability.
Consider extracting it to a constant:
+const THOUGHT_CHAIN_MARKER = 'Here are a list of questions that may be relevant for you to understand my chain of thoughts: [' export function getAllUserMessagesAsStringArray(allMessages: Message[] | AI.Message[]) { const userMessages = allMessages.filter((m) => m.role === 'user') const cleanMessages = userMessages.map((m) => extractBetweenMarkers( cleanPrompt(m.content), - 'Here are a list of questions that may be relevant for you to understand my chain of thoughts: [', + THOUGHT_CHAIN_MARKER, ), )
AndlerRL
added
bug
* impr: chat panel mobile a11y (#384) * impr: textarea a11y mobile * impr: scroll to bottom + regenerate cta a11y * fix: getRoutType in public * fix: improve normalizedPath chat condition * feat: improve clickable text with sentence content (#388) * feat: improve clickable text with sentence content * feat: clickable usage * chore: add coderabbit suggestion * [web] fix: continuous thread (#386) * fix: share url + missing devMode guard + restore public private badge in personal chat * fix: isContinuousChat w/chat-list impr * chore: ai feedbacks * fix: duplicated continue thread following question * chore: prompt order + formatting wip (#391) * chore: prompt order + formatting wip * chore: coderabbitai feedbacks * impr: coderabbitai feedback * [masterbots.ai] impr: output instructions prompt clarity * impr: output instruction prompt clarity * impr: output instructions * chore: init examples + init config upt (#392) * chore(wip): init examples + init config upt * chore: split example full seed, 8 parts * chore(wip): insert new masterbot prompts + wip relationship connection * chore(wip): new chatbots config + description + topic rel * chore(wip): upt current chatbots descriptions * chore(wip): chatbot metadata junction upt * chore: upt init config seeds, prompt rel + phase 2 chatbot upt * chore: impr init config seeds * [masterbots.ai] fix: instructions order and tags + tweaks * fix: prompt order and xml closing tags tweak * impr: system prompts order --------- Co-authored-by: bran18 <[email protected]> Co-authored-by: Brandon Fernández <[email protected]>
DEMO
2025-02-21.20-14-01.mp4
Summary by Sourcery
This pull request introduces several bug fixes and enhancements to the chat functionality. It fixes an issue with continuous threads, improves error handling, updates the UI to display thread visibility, and improves the thread update logic. It also adds support for continuing threads.
New Features:
Bug Fixes:
Enhancements:
CI:
Summary by CodeRabbit
New Features
Style
Refactor