chore: february 3rd release Reorder imports, update JSX/styling, threads, hooks, and utilities #390
Conversation
* impr: textarea a11y mobile * impr: scroll to bottom + regenerate cta a11y * fix: getRoutType in public
* feat: improve clickable text with sentence content * feat: clickable usage * chore: add coderabbit suggestion
* 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
Reviewer's Guide by SourceryThis pull request introduces several enhancements and refactorings to the Masterbots.ai application, focusing on improving chat functionality, thread management, and UI components. Key changes include the introduction of continuous threads, improved error handling, and UI adjustments for better user experience. Sequence diagram for appending a new message to a threadsequenceDiagram
participant User
participant ChatPanel
participant MBChatProvider
participant createThread
participant append
participant saveNewMessage
participant setState
User->>ChatPanel: Enters and sends a message
ChatPanel->>MBChatProvider: appendNewMessage(userMessage)
alt (!allMessages.length && isNewChat) || isContinuingThread
MBChatProvider->>createThread: createThread(threadId, chatbotId, parentThreadId, jwt, isPublic)
createThread-->>MBChatProvider: Thread created
end
MBChatProvider->>append: append(userMessage, chatAttachments)
append-->>MBChatProvider: Response from AI
MBChatProvider->>saveNewMessage: saveNewMessage(newUserMessage)
saveNewMessage-->>MBChatProvider: User message saved
MBChatProvider->>saveNewMessage: saveNewMessage(newAssistantMessage)
saveNewMessage-->>MBChatProvider: Assistant message saved
MBChatProvider->>setState: setState({ isNewChat: false })
setState-->>MBChatProvider: State updated
MBChatProvider-->>ChatPanel: Update UI with new messages
Sequence diagram for toggling thread visibilitysequenceDiagram
participant User
participant ChatOptions
participant ThreadVisibilityProvider
participant updateThreadVisibility
User->>ChatOptions: Clicks on toggle visibility
ChatOptions->>ThreadVisibilityProvider: toggleVisibility(newIsPublic, threadId)
ThreadVisibilityProvider->>updateThreadVisibility: updateThreadVisibility(isPublic, threadId, jwt)
updateThreadVisibility-->>ThreadVisibilityProvider: Success or Error
alt Success
ThreadVisibilityProvider->>ThreadVisibilityProvider: setIsPublic(newIsPublic)
ThreadVisibilityProvider-->>ChatOptions: Update UI
else Error
ThreadVisibilityProvider-->>ChatOptions: Display error message
end
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
thread: Thread
parentThreadId: string
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 introduces numerous formatting and functional adjustments across several files. The changes include reordering of import statements, modifications to JSX structure and metadata formatting, and updates to function signatures and component props. Enhancements to clickable text processing add a new 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/components/routes/chat/chat-clickable-text.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/lib/actions.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. apps/masterbots.ai/components/routes/chat/chat-message.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.
✨ 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 (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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 changeset to document the changes in this PR.
- The diff includes a mix of formatting changes and logic changes; consider separating these into different PRs for better reviewability.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 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
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
apps/masterbots.ai/lib/hooks/use-thread-visibility.tsx (1)
70-82: Enhance error handling and input validation.While the error handling has improved, consider these enhancements:
- Add input validation for
threadIdandjwt- Make error messages more specific by including the error details
- Consider adding retry logic for transient failures
const toggleVisibility = async (newIsPublic: boolean, threadId: string) => { + if (!threadId?.trim()) { + customSonner({ type: 'error', text: 'Invalid thread ID provided.' }) + return + } + if (!jwt) { + customSonner({ type: 'error', text: 'Authentication required. Please log in.' }) + return + } try { const updateThreadResponse = await updateThreadVisibility({ isPublic: newIsPublic, threadId, jwt }) if (updateThreadResponse.success) { setIsPublic(newIsPublic) - customSonner({ type: 'success', text: `Thread is now ${newIsPublic ? 'public' : 'private'}!` }) + customSonner({ + type: 'success', + text: `Thread visibility updated successfully. The thread is now ${newIsPublic ? 'visible to everyone' : 'private and only visible to you'}.` + }) } } 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' + customSonner({ + type: 'error', + text: `Failed to update thread visibility: ${errorMessage}. Please try again.` + }) } }apps/masterbots.ai/components/routes/thread/thread-component.tsx (1)
86-101: Enhance accessibility for the visibility badge.While the visibility indicator is a good addition, consider these accessibility improvements:
- Add ARIA attributes to the Badge
- Verify color contrast ratios for accessibility
#!/usr/bin/env python3 from colorsys import rgb_to_hls import re def hex_to_rgb(hex_color): hex_color = hex_color.lstrip('#') return tuple(int(hex_color[i:i+2], 16) for i in (0, 2, 4)) def calculate_contrast_ratio(color1, color2): def get_luminance(rgb): rgb = [v/255 for v in rgb] rgb = [v/12.92 if v <= 0.03928 else ((v+0.055)/1.055)**2.4 for v in rgb] return 0.2126 * rgb[0] + 0.7152 * rgb[1] + 0.0722 * rgb[2] l1 = get_luminance(color1) l2 = get_luminance(color2) lighter = max(l1, l2) darker = min(l1, l2) return (lighter + 0.05) / (darker + 0.05) # Colors from the code colors = { 'public_bg': '#BE17E8', # Purple background for public threads 'private_bg': '#09090B', # Dark background for private threads 'text': '#FFFFFF' # White text } # Check contrast ratios public_contrast = calculate_contrast_ratio(hex_to_rgb(colors['public_bg']), hex_to_rgb(colors['text'])) private_contrast = calculate_contrast_ratio(hex_to_rgb(colors['private_bg']), hex_to_rgb(colors['text'])) print(f"Public badge contrast ratio: {public_contrast:.2f}") print(f"Private badge contrast ratio: {private_contrast:.2f}") print(f"WCAG AA requires: 4.5:1 for normal text, 3:1 for large text")<div className="flex gap-3 items-center justify-center pl-2 pr-4 sm:pl-4 sm:pr-8"> {routeType === 'chat' && ( <Badge variant="outline" + role="status" + aria-label={`Thread visibility status: ${thread.isPublic ? 'Public' : 'Private'}`} className={cn({ 'bg-[#BE17E8] text-white': thread.isApproved && thread.isPublic, 'bg-[#09090B] text-white': thread.isApproved && !thread.isPublic, })} > {thread.isPublic ? 'Public' : 'Private'} </Badge> )} <ChatOptions threadId={thread.threadId} thread={thread} isBrowse /> </div>apps/masterbots.ai/components/shared/button-scroll-to-bottom.tsx (1)
7-31: Add TypeScript types and documentation.Consider adding TypeScript types and JSDoc documentation for better maintainability.
+/** + * A button component that scrolls the content to the bottom. + * @param {Object} props - The component props + * @param {string} [props.className] - Additional CSS classes for the button + * @param {string} [props.textClassName] - CSS classes for the button text + * @param {boolean} [props.isAtBottom] - Whether the content is already at the bottom + * @param {() => void} props.scrollToBottom - Function to scroll content to bottom + */ export function ButtonScrollToBottom({ className, isAtBottom, scrollToBottom, ...props -}: ButtonProps & { textClassName?: string; isAtBottom?: boolean; scrollToBottom: () => void }) { +}: ButtonProps & { + /** CSS classes for the button text */ + textClassName?: string; + /** Whether the content is already at the bottom */ + isAtBottom?: boolean; + /** Function to scroll content to bottom */ + scrollToBottom: () => void; +}) {apps/masterbots.ai/components/routes/chat/chat-options.tsx (1)
125-133: Enhance error handling with user feedback.While the error handling is a good addition, consider providing user feedback when the visibility toggle fails.
try { await toggleVisibility(!thread?.isPublic, threadId) thread.isPublic = !thread?.isPublic + customSonner({ type: 'success', text: `Thread is now ${thread.isPublic ? 'public' : 'private'}` }) } catch (error) { console.error('Failed to update thread visibility:', error) + customSonner({ type: 'error', text: 'Failed to update thread visibility' }) }apps/masterbots.ai/components/routes/chat/chat-list.tsx (1)
240-241: Consider accessibility improvements for previous message styling.Based on the learnings from PR #377, ensure the background color contrast for previous messages meets WCAG guidelines.
#!/usr/bin/python3 import colorsys import sys def check_contrast(bg_color, text_color): # Convert hex to RGB bg_rgb = tuple(int(bg_color.lstrip('#')[i:i+2], 16)/255 for i in (0, 2, 4)) text_rgb = tuple(int(text_color.lstrip('#')[i:i+2], 16)/255 for i in (0, 2, 4)) # Calculate relative luminance def get_luminance(rgb): rgb = [c/12.92 if c <= 0.03928 else ((c+0.055)/1.055)**2.4 for c in rgb] return 0.2126 * rgb[0] + 0.7152 * rgb[1] + 0.0722 * rgb[2] l1 = get_luminance(bg_rgb) l2 = get_luminance(text_rgb) # Calculate contrast ratio ratio = (max(l1, l2) + 0.05) / (min(l1, l2) + 0.05) return ratio >= 4.5 # WCAG AA standard for normal text # Check contrast for accent background accent_bg = "#1d283a9a" # From the code text_color = "#000000" # Default text color print(f"Contrast ratio meets WCAG AA: {check_contrast(accent_bg, text_color)}")apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx (1)
324-345: Consider adding validation for thread initialization.The
initialThreadfunction should validate the input parameters and handle edge cases.export function initialThread(thread: Thread, session: Session): Thread { + if (!thread || !session?.user?.id) { + throw new Error('Invalid thread or session data'); + } + return { threadId: '', chatbot: thread.chatbot, chatbotId: thread.chatbotId, createdAt: new Date(), isApproved: false, isPublic: false, messages: [thread.messages.filter(msg => msg.role === 'user')[0]], userId: session.user?.id, updatedAt: new Date(), isBlocked: false, model: 'OPENAI', user: null, thread, parentThreadId: thread.threadId, threads: [], } as unknown as Thread }apps/masterbots.ai/lib/clickable-results.ts (1)
178-188: Consider adding sentence validation.The first sentence extraction could be improved with validation to ensure meaningful context.
// * Extract the first sentence of the content (up to the first period) const firstSentenceMatch = content.match(/^(.+?\.)(?:\s|$)/); +const isValidSentence = (sentence: string) => { + return sentence.length >= 10 && sentence.split(' ').length >= 3; +}; const firstSentence = firstSentenceMatch - ? title + ': ' + firstSentenceMatch[1] + ? title + ': ' + (isValidSentence(firstSentenceMatch[1]) ? firstSentenceMatch[1] : content) : title + ': ' + content;apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx (2)
89-107: Consider memoizing the renderClickableContent function.The function is recreated on every render but its logic remains constant. Consider using useCallback to optimize performance.
- const renderClickableContent = ( + const renderClickableContent = useCallback(( clickableText: string, restText: string, fullContext: string - ) => ( + ) => ( <span className="inline"> <button className={cn( 'inline-block cursor-pointer hover:underline bg-transparent border-none p-0 m-0 text-left', isListItem ? 'text-blue-500' : 'text-link' )} onClick={createClickHandler(clickableText, fullContext)} type="button" > {clickableText} </button> {restText.startsWith(':') ? restText.replace(/^:/, ': ') : restText} </span> - ) + ), [isListItem, createClickHandler])
124-142: Consider extracting strong content handling logic.The strong content handling logic is becoming complex. Consider extracting it into a separate function for better maintainability.
+ const handleStrongContent = useCallback((content: React.ReactNode, index: number) => { + const strongContent = extractTextFromReactNodeNormal( + (content.props as { children: React.ReactNode }).children + ) + const { clickableText, restText, fullContext } = parseClickableText( + strongContent + ':' + ) + + if (!clickableText.trim()) return content + + return ( + <button + key={`clickable-${index}`} + className={cn( + 'cursor-pointer hover:underline', + isListItem ? 'text-blue-500' : 'text-link' + )} + onClick={createClickHandler( + clickableText, + fullContext || strongContent + )} + type="button" + tabIndex={0} + > + {strongContent} + </button> + ) + }, [isListItem, createClickHandler])apps/masterbots.ai/lib/hooks/use-mb-chat.tsx (2)
492-582: Consider breaking down appendNewMessage function.The appendNewMessage function is quite long and handles multiple responsibilities. Consider breaking it down into smaller, focused functions.
+ const handleThreadCreation = async (threadId: string, chatbot: Chatbot, isContinuousThread: boolean) => { + await createThread({ + threadId: threadId as string, + chatbotId: chatbot.chatbotId, + parentThreadId: isContinuousThread ? threadId : undefined, + jwt: session?.user?.hasuraJwt, + isPublic: activeChatbot?.name !== 'BlankBot', + }) + } + const prepareMessagesToAppend = (chatbotMetadata: ChatbotMetadataExamples) => { + return uniqBy( + [ + ...systemPrompts, + { + id: 'examples-' + nanoid(10), + role: 'system' as 'data' | 'system' | 'user' | 'assistant', + content: examplesPrompt(chatbotMetadata), + }, + ], + 'content', + ) + } const appendNewMessage = async (userMessage: AiMessage | CreateMessage) => { try { const chatbotMetadata = await getMetadataLabels() const recentSearchParams = new URLSearchParams(window.location.search) const isContinuingThread = recentSearchParams.has('continuousThreadId') if (((!allMessages.length && isNewChat) || isContinuingThread) && chatbot) { - await createThread(...) + await handleThreadCreation(threadId, chatbot, isContinuousThread) } - const chatMessagesToAppend = uniqBy(...) + const chatMessagesToAppend = prepareMessagesToAppend(chatbotMetadata) setMessages(chatMessagesToAppend) // ... rest of the function } catch (error) { // ... error handling } }
495-496: Document hydration issue workaround.The comment about hydration issues should be expanded to better explain the problem and solution for future maintainers.
- // ? Hydration issues is causing the continuing thread to update until the 2nd attempt... + /** + * FIXME: Hydration Issue + * The continuing thread updates require multiple attempts due to React hydration. + * This occurs because: + * 1. Client components try to update state before server response + * 2. React state updates don't sync with latest searchParams immediately + * + * Current workaround: Manually check window.location.search + * TODO: Consider using Next.js middleware or proper state management + */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/masterbots.ai/app/layout.tsx(4 hunks)apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx(8 hunks)apps/masterbots.ai/components/routes/chat/chat-combobox.tsx(1 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/chat/chat-panel.tsx(3 hunks)apps/masterbots.ai/components/routes/chat/prompt-form.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/shared/button-scroll-to-bottom.tsx(2 hunks)apps/masterbots.ai/components/ui/badge.tsx(2 hunks)apps/masterbots.ai/lib/clickable-results.ts(3 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/lib/threads.ts(1 hunks)apps/masterbots.ai/lib/utils.ts(1 hunks)apps/masterbots.ai/services/hasura/hasura.service.ts(2 hunks)apps/masterbots.ai/types/types.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
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.
apps/masterbots.ai/lib/clickable-results.ts (1)
Learnt from: AndlerRL
PR: bitcashorg/masterbots#388
File: apps/masterbots.ai/lib/clickable-results.ts:148-149
Timestamp: 2025-02-24T20:46:17.229Z
Learning: Issue #389 is tracking potential improvements and refactoring needs for the ClickableText component, including the evaluation of using context objects instead of multiple parameters.
apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx (1)
Learnt from: AndlerRL
PR: bitcashorg/masterbots#388
File: apps/masterbots.ai/lib/clickable-results.ts:148-149
Timestamp: 2025-02-24T20:46:17.229Z
Learning: Issue #389 is tracking potential improvements and refactoring needs for the ClickableText component, including the evaluation of using context objects instead of multiple parameters.
apps/masterbots.ai/components/routes/chat/chat-list.tsx (1)
Learnt from: AndlerRL
PR: bitcashorg/masterbots#377
File: apps/masterbots.ai/components/routes/chat/chat-list.tsx:141-207
Timestamp: 2025-02-14T01:43:05.202Z
Learning: In the chat interface, messages from previous threads should be visually distinct from current messages through styling (e.g., different background colors, borders, badges) to help users understand the context and origin of the messages.
🔇 Additional comments (22)
apps/masterbots.ai/components/routes/chat/chat-combobox.tsx (1)
63-63:❓ Verification inconclusive
LGTM! Verify the button positioning across different screen sizes.
The change from Tailwind's default spacing scale to precise pixel values provides more control over the button's position. The addition of the
sm:left-[14px]breakpoint ensures proper responsive behavior.Please verify that the button is correctly positioned and aligned across different screen sizes and breakpoints:
🏁 Script executed:
#!/bin/bash # Description: Find all files that might be affected by this positioning change rg -l "left-\[(?:8|14)px\]|top-\[14px\]" --type tsxLength of output: 81
Button Positioning Responsive Check – Manual Verification Required
The code snippet in
apps/masterbots.ai/components/routes/chat/chat-combobox.tsx(line 63) correctly switches from Tailwind's default spacing scale to precise pixel values usingleft-[8px],top-[14px], and adds a responsive breakpoint withsm:left-[14px]. This should offer more control over the button’s positioning on different screen sizes.However, the automated search using
rg --type tsxresulted in an error due to an unrecognized file type. To proceed further, please re-run or manually verify the positioning across various breakpoints. For instance, you can execute a revised shell script:#!/bin/bash # Revised script to search for potential positioning patterns in .tsx files rg -l --type-add 'tsx:*.tsx' -t tsx "left-\[(?:8|14)px\]|top-\[14px\]"If you confirm that the button displays correctly in all scenarios, then this change can be approved.
apps/masterbots.ai/types/types.ts (1)
359-363: LGTM! Well-documented interface addition.The
ParsedTextinterface is clearly documented with descriptive comments for each property, making its purpose and usage clear to other developers.apps/masterbots.ai/services/hasura/hasura.service.ts (1)
741-766: LGTM! Function renamed to follow camelCase convention.The changes improve consistency by:
- Renaming the function to follow camelCase convention
- Updating the mutation to correctly toggle
isPublicinstead ofisApproved- Maintaining proper error handling
apps/masterbots.ai/lib/threads.ts (1)
31-34: LGTM! Optimized to prevent duplicate messages.The changes improve the function by:
- Using Set to ensure message uniqueness
- Adding clear comments explaining the purpose
- Maintaining the same output format
apps/masterbots.ai/components/routes/chat/prompt-form.tsx (3)
110-110: LGTM! Nice branding touch with the purple border.The border color (#be16e8) adds a distinctive brand identity to the form.
121-123: LGTM! Improved textarea responsiveness.The changes enhance the textarea's usability:
- Dynamic height control with minRows={2} and maxRows={6}
- Better mobile-first design with responsive min-height
Also applies to: 132-133
138-138: LGTM! Better button alignment.The button positioning has been refined to maintain proper alignment with the new textarea dimensions.
apps/masterbots.ai/components/ui/badge.tsx (2)
1-1: LGTM! Cleaner import statement.Direct imports of
VariantPropsandcvaimprove code clarity.
16-16: LGTM! Enhanced outline variant styling.The new styling provides better visual hierarchy with muted text and border background.
apps/masterbots.ai/lib/utils.ts (1)
299-307: LGTM! Improved route detection logic.The enhanced chat route detection now correctly handles edge cases:
- Properly excludes
/careerand/career/paths- Better documented with clear comments
apps/masterbots.ai/components/routes/chat/chat-panel.tsx (2)
59-60: LGTM! Well-structured animation constants.Good practice extracting animation classes into named constants for reusability and maintainability.
132-138: LGTM! Enhanced button interactions.The buttons now have smooth hover animations that reveal additional context:
- Text smoothly appears on hover
- Consistent animation behavior across buttons
- Improved visual feedback for user interactions
Also applies to: 161-166
apps/masterbots.ai/lib/hooks/use-thread-visibility.tsx (1)
6-7: LGTM! Clean import organization.The imports are well-organized and follow the explicit import pattern as per the learnings from PR #287.
apps/masterbots.ai/components/shared/button-scroll-to-bottom.tsx (1)
3-5: LGTM! Clean import organization.The imports are well-organized and follow a logical grouping pattern.
apps/masterbots.ai/app/layout.tsx (2)
83-86: LGTM! Proper theme color configuration.The theme color configuration correctly handles both light and dark color schemes.
78-79:❓ Verification inconclusive
Verify the google-site-verification token.
Ensure that the verification token is valid and matches the one in Google Search Console.
🏁 Script executed:
#!/bin/bash # Description: Verify that the google-site-verification token exists in Google Search Console # Note: This is a basic check to ensure the token format is valid TOKEN="By9aM0DbPDDO9qa7Y3zNwDFyYuSPslVzje76EVOCcY0" # Check token format (should be base64) if echo "$TOKEN" | base64 -d > /dev/null 2>&1; then echo "Token format appears valid (base64 encoded)" else echo "Warning: Token format may not be valid base64" fi # Verify token is accessible curl -s -I "https://www.google.com/webmasters/verification/verification-file?domain=masterbots.ai&verification-token=$TOKEN"Length of output: 733
Action: Confirm the Google Site Verification Token
The token configured in
apps/masterbots.ai/app/layout.tsxappears as follows:'google-site-verification': 'By9aM0DbPDDO9qa7Y3zNwDFyYuSPslVzje76EVOCcY0',Our automated check raised a warning that the token did not decode as valid Base64, and the curl request returned a 301 redirect to the Search Console URL. Please manually verify that this token is correct and matches the one provided in your Google Search Console configuration. If the token is intended to be in a different format (i.e., not Base64), it would be helpful to document that for clarity.
apps/masterbots.ai/components/routes/chat/chat-options.tsx (2)
105-117: LGTM! Improved dropdown menu styling.The dropdown menu trigger styling has been enhanced with better size and radius properties, making it more consistent with the design system.
43-43:❓ Verification inconclusive
Verify the impact of URL path structure change.
The URL path format has changed from
/${category}/${threadId}to/b/${category}/${threadId}. Ensure all route handlers and navigation links are updated accordingly.
🏁 Script executed:
#!/bin/bash # Search for old URL path patterns that might need updating rg -A 2 $'/${toSlug\(.*?category\.name\)}/${.*?threadId}'Length of output: 190
Action: Verify URL Path Updates in Navigation and Route Handlers
The code now builds URLs using the new
/b/${toSlug(thread.chatbot.categories[0].category.name)}/${thread.threadId}format. Please ensure that all route handlers, navigation links, and any related code references have been updated accordingly. To support your review, you can re-run a refined search for any occurrences of the old URL pattern that lacks the/b/prefix. For example, try executing:#!/bin/bash # Search for template strings starting with a slash not followed by "b/" rg -P -A 2 '`/(?!b/)\$\{toSlug\(' .If this search shows no results, it confirms that no old URL patterns remain. Otherwise, update any found instances appropriately.
apps/masterbots.ai/components/routes/chat/chat-list.tsx (1)
171-189: Improved message rendering with null checks.The conditional rendering ensures both user message and chatbot response exist before rendering, preventing potential null reference issues.
apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx (1)
153-164: Improved continuous thread handling with proper cleanup.The refactored code properly handles cleanup of continuous thread state when the ID is not present.
apps/masterbots.ai/lib/clickable-results.ts (1)
147-159: Enhanced unique phrase handling with better context extraction.The improved logic for extracting the first sentence provides better context for clickable text.
apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx (1)
26-34: LGTM! Enhanced context handling in click handler.The addition of
fullContextparameter improves the message generation by using more comprehensive context when available, falling back to cleaned text when not provided.
* chore: prompt order + formatting wip * chore: coderabbitai feedbacks * impr: coderabbitai feedback
* impr: output instruction prompt clarity * impr: output instructions
* 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
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/masterbots.ai/lib/hooks/use-mb-chat.tsx (1)
523-526: Remove debug console statements before production.There are several console.log statements that should be removed or wrapped in a debug flag check to keep production logs clean.
- console.info( - 'Before appending a new message, we check the following to know if is new chat or not: ', - ) + if (appConfig.features.devMode) { + console.info( + 'Before appending a new message, we check the following to know if is new chat or not: ', + ) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
apps/hasura/seeds/masterbots/1724269974513_init_config_seeds.sql(1 hunks)apps/hasura/seeds/masterbots/1738179677920_init_domain_enum.sql(0 hunks)apps/hasura/seeds/masterbots/1738179680569_init_category_enum.sql(0 hunks)apps/hasura/seeds/masterbots/1738179682864_init_subcategory_enum.sql(0 hunks)apps/hasura/seeds/masterbots/1738179687443_init_tag_enum.sql(0 hunks)apps/hasura/seeds/masterbots/1738179716486_init_example_pt1.sql(0 hunks)apps/hasura/seeds/masterbots/1738179716487_init_example_pt1_5.sql(0 hunks)apps/hasura/seeds/masterbots/1738179716488_init_example_pt2.sql(0 hunks)apps/hasura/seeds/masterbots/1738186795715_init_chatbot_metadata_seeds.sql(0 hunks)apps/hasura/seeds/masterbots/1740386118928_init_domain_enum.sql(1 hunks)apps/hasura/seeds/masterbots/1740386119321_init_chatbot_metadata_seeds.sql(1 hunks)apps/hasura/seeds/masterbots/1740386128297_init_category_enum.sql(1 hunks)apps/hasura/seeds/masterbots/1740386133107_init_subcategory_enum.sql(1 hunks)apps/hasura/seeds/masterbots/1740386141151_init_tag_enum.sql(1 hunks)apps/hasura/seeds/masterbots/1740386174715_init_example_pt_1.sql(1 hunks)apps/hasura/seeds/masterbots/1740386174815_init_example_pt_2.sql(1 hunks)apps/hasura/seeds/masterbots/1740386174915_init_example_pt_3.sql(1 hunks)apps/hasura/seeds/masterbots/1740386175015_init_example_pt_4.sql(1 hunks)apps/hasura/seeds/masterbots/1740386175115_init_example_pt_5.sql(1 hunks)apps/hasura/seeds/masterbots/1740386175215_init_example_pt_6.sql(1 hunks)apps/hasura/seeds/masterbots/1740386175315_init_example_pt_7.sql(1 hunks)apps/hasura/seeds/masterbots/1740386175415_init_example_pt_8.sql(1 hunks)apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx(8 hunks)apps/masterbots.ai/components/routes/chat/chat-message.tsx(1 hunks)apps/masterbots.ai/lib/actions.ts(1 hunks)apps/masterbots.ai/lib/constants/prompts.ts(2 hunks)apps/masterbots.ai/lib/hooks/use-mb-chat.tsx(17 hunks)apps/masterbots.ai/services/hasura/hasura.service.ts(5 hunks)apps/masterbots.ai/types/types.ts(4 hunks)
💤 Files with no reviewable changes (8)
- apps/hasura/seeds/masterbots/1738179682864_init_subcategory_enum.sql
- apps/hasura/seeds/masterbots/1738179716488_init_example_pt2.sql
- apps/hasura/seeds/masterbots/1738179677920_init_domain_enum.sql
- apps/hasura/seeds/masterbots/1738179680569_init_category_enum.sql
- apps/hasura/seeds/masterbots/1738179687443_init_tag_enum.sql
- apps/hasura/seeds/masterbots/1738186795715_init_chatbot_metadata_seeds.sql
- apps/hasura/seeds/masterbots/1738179716486_init_example_pt1.sql
- apps/hasura/seeds/masterbots/1738179716487_init_example_pt1_5.sql
✅ Files skipped from review due to trivial changes (14)
- apps/hasura/seeds/masterbots/1740386141151_init_tag_enum.sql
- apps/hasura/seeds/masterbots/1740386128297_init_category_enum.sql
- apps/hasura/seeds/masterbots/1740386175415_init_example_pt_8.sql
- apps/hasura/seeds/masterbots/1740386118928_init_domain_enum.sql
- apps/hasura/seeds/masterbots/1740386133107_init_subcategory_enum.sql
- apps/hasura/seeds/masterbots/1740386119321_init_chatbot_metadata_seeds.sql
- apps/hasura/seeds/masterbots/1740386174915_init_example_pt_3.sql
- apps/hasura/seeds/masterbots/1740386175015_init_example_pt_4.sql
- apps/hasura/seeds/masterbots/1740386175315_init_example_pt_7.sql
- apps/hasura/seeds/masterbots/1740386175215_init_example_pt_6.sql
- apps/hasura/seeds/masterbots/1740386174815_init_example_pt_2.sql
- apps/hasura/seeds/masterbots/1724269974513_init_config_seeds.sql
- apps/hasura/seeds/masterbots/1740386175115_init_example_pt_5.sql
- apps/hasura/seeds/masterbots/1740386174715_init_example_pt_1.sql
🧰 Additional context used
🧠 Learnings (1)
apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx (1)
Learnt from: AndlerRL
PR: bitcashorg/masterbots#388
File: apps/masterbots.ai/lib/clickable-results.ts:148-149
Timestamp: 2025-02-24T20:46:17.229Z
Learning: Issue #389 is tracking potential improvements and refactoring needs for the ClickableText component, including the evaluation of using context objects instead of multiple parameters.
🔇 Additional comments (19)
apps/masterbots.ai/lib/constants/prompts.ts (2)
127-134: Improved example format with XML-style tagging.The formatting change from markdown headers to XML-style tags makes the structure more consistent and parser-friendly. This aligns well with the structured format used in other parts of the system.
208-219: Well-structured new output instructions prompt.The new
setOutputInstructionPromptfunction adds useful formatting guidelines for responses. This should help produce more consistently structured responses with improved readability.apps/masterbots.ai/services/hasura/hasura.service.ts (2)
432-434: Good sorting addition for consistent prompt order.Adding ordering by
promptIdin descending order ensures consistent retrieval of the most recent prompts first, improving predictability in the UI.
744-769:❓ Verification inconclusive
Renamed function and updated visibility logic.
The function has been renamed from
UpdateThreadVisibilitytoupdateThreadVisibility(changing from PascalCase to camelCase) and now updates theisPublicfield instead ofisApproved. This change improves naming consistency with JavaScript conventions.Make sure all invocations of this function have been updated to use the new name, and that the semantic change from
isApprovedtoisPublicis intended and consistent with the application's requirements.
🏁 Script executed:
#!/bin/bash # Find all references to UpdateThreadVisibility to ensure they've been updated echo "Searching for references to the old function name:" rg "UpdateThreadVisibility" --type ts --type tsx # Find references to the new function name to verify changes have been made echo "Searching for references to the new function name:" rg "updateThreadVisibility" --type ts --type tsx # Look for any remaining references to isApproved in thread visibility context echo "Searching for isApproved fields that might need updating:" rg "isApproved.*visibility" --type ts --type tsxLength of output: 579
Renaming and Field Update Verification Needed
The function has been correctly renamed from
UpdateThreadVisibilitytoupdateThreadVisibilityand updated to modify theisPublicfield instead ofisApproved. However, the automated check did not produce expected output due to file type recognition issues. Please manually verify that:
- All invocations of the old function name (
UpdateThreadVisibility) have been updated.- The new function name (
updateThreadVisibility) is used consistently across the codebase.- The change from
isApprovedtoisPublicaligns with the intended business logic.apps/masterbots.ai/lib/actions.ts (1)
6-26: Enhanced system prompt formatting with better structure and prioritization.The changes to the
formatSystemPromptsfunction offer several improvements:
- Sorting prompts prioritizes those of type 'prompt', ensuring consistent order
- The new XML tag approach with proper opening and closing tags improves readability
- The refactored structure is more maintainable and aligns with the XML tag pattern used elsewhere
apps/masterbots.ai/types/types.ts (3)
358-362: Good addition of ParsedText interface to centralize type definitions.The new ParsedText interface provides a clear structure for handling clickable text, with separate properties for the clickable portion, the visual rendering text, and the full context. This improves type safety and makes the intent clearer.
70-70: Nice refactoring using Pick utility type.The change to use Pick<Prompt, 'promptId' | 'content' | 'type'> is a good practice as it ensures type safety while keeping the code DRY by reusing the Prompt type.
292-295: Interface enhancement with optional node property.The addition of the optional node property will help determine the nesting structure of content, supporting better contextual handling of clickable text elements.
apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx (4)
21-21: Helpful TODO comment for future enhancement.This comment provides good insight into potential future improvements for using the node property to determine nested content.
Since this relates to a tracked improvement, is this TODO associated with issue #389 mentioned in the learning notes about refactoring the ClickableText component?
28-36: Improved context handling in click handler.The updated createClickHandler now accepts a fullContext parameter, providing more complete context for follow-up questions. This should result in more relevant responses when users click on text elements.
91-109: Enhanced renderClickableContent with fullContext parameter.Adding the fullContext parameter to renderClickableContent ensures consistent context propagation throughout the component, improving the user experience when interacting with clickable elements.
141-144: Good fallback for missing fullContext.Using
fullContext || strongContentprovides a sensible fallback when fullContext is not available, ensuring the handler always has context to work with.apps/masterbots.ai/lib/hooks/use-mb-chat.tsx (7)
52-53: Initialize outputInstructionPrompt outside component.Good practice to initialize this constant outside the component to prevent recreation on each render.
120-125: Enhanced system prompts with output instructions.The updated systemPrompts array now includes outputInstructionPrompt, which improves the clarity of prompt formatting and should result in better responses.
212-237: Improve error handling structure.The error handling in onFinish still has nested try-catch blocks and duplicated thread deletion logic. Consider extracting common error handling to a separate function.
Create a reusable error handling function to eliminate duplication:
async onFinish(message: any, options: any) { + const handleError = async (error?: any) => { + console.error('Error saving new message: ', error) + customSonner({ type: 'error', text: 'Failed to save the Masterbot message. Please try again.' }) + + if (isNewChat) { + await deleteThread({ + threadId: params?.threadId ?? activeThread?.threadId, + jwt: session?.user?.hasuraJwt, + userId: session?.user.id, + }) + } + } try { if (appConfig.features.devMode) { customSonner({ type: 'info', text: `Ai generation finished, reason: ${options.finishReason}` }) } // ... existing code ... if (options.finishReason === 'error') { - customSonner({ type: 'error', text: 'Failed to finish communication with the Masterbot. Please try again.' }) - if (isNewChat) { - await deleteThread({ - threadId: params?.threadId ?? activeThread?.threadId, - jwt: session?.user?.hasuraJwt, - userId: session?.user.id, - }) - } + return handleError(new Error('Failed to finish communication with the Masterbot')) } // ... rest of success logic ... } catch (error) { - console.error('Error saving new message: ', error) - customSonner({ type: 'error', text: 'Failed to save the Masterbot message. Please try again.' }) - if (isNewChat) { - await deleteThread({ - threadId: params?.threadId ?? activeThread?.threadId, - jwt: session?.user?.hasuraJwt, - userId: session?.user.id, - }) - } + return handleError(error) } }
238-245: Good cleanup of continuousThreadId search parameter.Removing the continuousThreadId parameter from the URL after a thread is created prevents confusion and maintains clean navigation state.
518-520: Consider addressing hydration issues.The comment about hydration issues suggests there might be problems with state synchronization between client and server. This could lead to inconsistent UI behavior.
Consider using a useEffect with proper dependencies to handle the search params update:
useEffect(() => { const recentSearchParams = new URLSearchParams(window.location.search) const isContinuingThread = recentSearchParams.has('continuousThreadId') // Update state based on this value }, [searchParams])
79-89: Good simplification of isNewChat state initialization.Simplifying the isNewChat initialization to always start as true and then updating based on conditions improves code readability and reduces potential edge cases.
139-139: Clear logic for threadId resolution.The conditional assignment of threadId based on isContinuousThread makes the thread resolution logic more explicit and easier to follow.
| li({ children, ordered, node, ...props }) { | ||
| const allowedTags = ['li', 'ul', 'ol'] | ||
| const Tag = allowedTags.includes(node.tagName) ? node.tagName : 'li' | ||
| const hasNestedList = React.Children.toArray(children).some( | ||
| child => | ||
| React.isValidElement(child) && | ||
| (child.type === 'ul' || child.type === 'ol') | ||
| ) | ||
|
|
||
| return ( | ||
| <li | ||
| className={cn( | ||
| "ml-4", | ||
| ordered ? "list-decimal" : "list-disc", | ||
| hasNestedList && "mt-2" | ||
| )} | ||
| // @ts-ignore | ||
| <Tag | ||
| as={node.tagName as keyof JSX.IntrinsicElements} | ||
| className="gap-2" | ||
| > | ||
| {/* TODO: This modifies the lists, removes the formatting. maybe only to grab what we received form the node object, would be enough */} | ||
| <ClickableText | ||
| isListItem | ||
| sendMessageFromResponse={sendMessageFromResponse} | ||
| node={node} | ||
| > | ||
| {children} | ||
| </ClickableText> | ||
| </li> | ||
| </Tag> | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improved list element handling, but with TypeScript and formatting concerns.
The changes here provide more flexible rendering by using the appropriate tag from the node, but there are a few issues to address:
- The
@ts-ignorecomment indicates a TypeScript type issue that should be resolved properly. - The TODO comment suggests there's a known problem with list formatting.
- There may be JSX validation issues with the dynamic
asprop usage.
Here's a safer implementation approach:
- li({ children, ordered, node, ...props }) {
- const allowedTags = ['li', 'ul', 'ol']
- const Tag = allowedTags.includes(node.tagName) ? node.tagName : 'li'
+ li({ children, ordered, node, ...props }) {
+ // Use React.createElement instead of JSX for dynamic elements
+ const allowedTags = ['li', 'ul', 'ol']
+ const tagName = allowedTags.includes(node.tagName) ? node.tagName : 'li'
const hasNestedList = React.Children.toArray(children).some(
child =>
React.isValidElement(child) &&
(child.type === 'ul' || child.type === 'ol')
)
return (
- // @ts-ignore
- <Tag
- as={node.tagName as keyof JSX.IntrinsicElements}
- className="gap-2"
- >
+ React.createElement(tagName, { className: "gap-2" },
+ <React.Fragment>
{/* TODO: This modifies the lists, removes the formatting. maybe only to grab what we received form the node object, would be enough */}
<ClickableText
isListItem
sendMessageFromResponse={sendMessageFromResponse}
node={node}
>
{children}
</ClickableText>
+ </React.Fragment>
- </Tag>
+ )
)
}📝 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.
| li({ children, ordered, node, ...props }) { | |
| const allowedTags = ['li', 'ul', 'ol'] | |
| const Tag = allowedTags.includes(node.tagName) ? node.tagName : 'li' | |
| const hasNestedList = React.Children.toArray(children).some( | |
| child => | |
| React.isValidElement(child) && | |
| (child.type === 'ul' || child.type === 'ol') | |
| ) | |
| return ( | |
| <li | |
| className={cn( | |
| "ml-4", | |
| ordered ? "list-decimal" : "list-disc", | |
| hasNestedList && "mt-2" | |
| )} | |
| // @ts-ignore | |
| <Tag | |
| as={node.tagName as keyof JSX.IntrinsicElements} | |
| className="gap-2" | |
| > | |
| {/* TODO: This modifies the lists, removes the formatting. maybe only to grab what we received form the node object, would be enough */} | |
| <ClickableText | |
| isListItem | |
| sendMessageFromResponse={sendMessageFromResponse} | |
| node={node} | |
| > | |
| {children} | |
| </ClickableText> | |
| </li> | |
| </Tag> | |
| ) | |
| li({ children, ordered, node, ...props }) { | |
| // Use React.createElement instead of JSX for dynamic elements | |
| const allowedTags = ['li', 'ul', 'ol'] | |
| const tagName = allowedTags.includes(node.tagName) ? node.tagName : 'li' | |
| const hasNestedList = React.Children.toArray(children).some( | |
| child => | |
| React.isValidElement(child) && | |
| (child.type === 'ul' || child.type === 'ol') | |
| ) | |
| return ( | |
| React.createElement(tagName, { className: "gap-2" }, | |
| <React.Fragment> | |
| {/* TODO: This modifies the lists, removes the formatting. maybe only to grab what we received form the node object, would be enough */} | |
| <ClickableText | |
| isListItem | |
| sendMessageFromResponse={sendMessageFromResponse} | |
| node={node} | |
| > | |
| {children} | |
| </ClickableText> | |
| </React.Fragment> | |
| ) | |
| ) | |
| } |
Summary by Sourcery
This pull request includes several bug fixes and enhancements to the chat interface, thread handling, and overall user experience. It also updates the google-site-verification metadata.
Bug Fixes:
Enhancements:
CI:
Summary by CodeRabbit
New Features
Style
Bug Fixes