fix: pre launch merge conflict #347
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Reviewer's Guide by SourceryThis pull request addresses a merge conflict that arose before the launch. The primary change involves updating the Hasura service to resolve inconsistencies in how data is fetched and structured, particularly around categories, chatbots, and threads. Additionally, some minor formatting and query adjustments were made to improve data retrieval. Sequence diagram for thread visibility flowsequenceDiagram
participant U as User
participant C as Client
participant H as Hasura Service
participant DB as Database
U->>C: Toggle thread visibility
C->>H: UpdateThreadVisibility(threadId, isPublic, jwt)
H->>DB: Update thread visibility
DB-->>H: Confirm update
H-->>C: Return success/failure
C-->>U: Show visibility status
U->>C: Request thread
C->>H: getThread(threadId, jwt)
H->>DB: Fetch thread with visibility check
DB-->>H: Return thread data
H-->>C: Return thread with messages
C-->>U: Display thread
Class diagram showing updated data structuresclassDiagram
class Thread {
+threadId: string
+chatbotId: number
+userId: string
+isPublic: boolean
+isApproved: boolean
+messages: Message[]
+chatbot: Chatbot
}
class Message {
+messageId: string
+threadId: string
+content: string
+role: string
+createdAt: string
}
class Chatbot {
+chatbotId: number
+name: string
+categories: Category[]
+threads: Thread[]
+prompts: Prompt[]
}
class Category {
+categoryId: number
+name: string
+chatbots: Chatbot[]
}
Thread "*" -- "1" Chatbot
Thread "1" -- "*" Message
Chatbot "*" -- "*" Category
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
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/app/actions/ai-executers.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. WalkthroughThis pull request introduces a comprehensive set of changes across the MasterBots.ai application, focusing on refactoring, code formatting, and replacing the notification system. The primary modifications include transitioning from a custom notification mechanism to Changes
Suggested Labels
Poem
✨ 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:
- Error handling is inconsistent across files - consider implementing a standardized error handling utility that can be reused. Current implementation mixes different patterns which could lead to bugs.
- Several instances of reduced type safety (e.g. removal of type checks and explicit type assertions). Please maintain strict type checking to prevent runtime errors.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 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.
| export async function getThread({ threadId, jwt }: GetThreadParams) { | ||
| let client = getHasuraClient({}) |
There was a problem hiding this comment.
suggestion: Consider adding null checks for threadId parameter
The removal of Partial type means we should explicitly handle cases where threadId might be undefined. Consider adding validation at the start of the function.
| export async function getThread({ threadId, jwt }: GetThreadParams) { | |
| let client = getHasuraClient({}) | |
| export async function getThread({ threadId, jwt }: GetThreadParams) { | |
| if (!threadId) { | |
| throw new Error('threadId is required'); | |
| } | |
| let client = getHasuraClient({}) |
| sendMessageFromResponse?: (message: string) => void // Callback to send a new message | ||
| chatbot?: Chatbot // Chatbot configuration | ||
| actionRequired?: boolean // Whether to show message actions | ||
| } | ||
|
|
||
| export function ChatMessage({ |
There was a problem hiding this comment.
suggestion: Add error handling for markdown parsing failures
The markdown rendering could fail with malformed content. Consider wrapping the MemoizedReactMarkdown component in an error boundary.
Suggested implementation:
// Component to handle markdown rendering errors
class MarkdownErrorBoundary extends React.Component<
{ children: React.ReactNode },
{ hasError: boolean }
> {
constructor(props: { children: React.ReactNode }) {
super(props)
this.state = { hasError: false }
}
static getDerivedStateFromError(_: Error) {
return { hasError: true }
}
render() {
if (this.state.hasError) {
return <div className="text-red-500">Error rendering markdown content</div>
}
return this.props.children
}
}
export interface ChatMessageProps {
message: Message // The chat message to display
sendMessageFromResponse?: (message: string) => void // Callback to send a new message
chatbot?: Chatbot // Chatbot configuration
actionRequired?: boolean // Whether to show message actions
}You'll also need to:
- Wrap the MemoizedReactMarkdown component (which I don't see in the provided code) with the new MarkdownErrorBoundary component like this:
<MarkdownErrorBoundary>
<MemoizedReactMarkdown>
{/* existing markdown content */}
</MemoizedReactMarkdown>
</MarkdownErrorBoundary>- Import React at the top of the file if not already imported:
import React from 'react'![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (44)
apps/masterbots.ai/components/layout/sidebar/sidebar-actions.tsx (1)
22-22: Useimport typefor type-only imports.The
ServerActionResultandChatimports are only used as types.-import { ServerActionResult, type Chat } from '@/types/types' +import type { ServerActionResult, Chat } from '@/types/types'🧰 Tools
🪛 Biome (1.9.4)
[error] 22-22: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
apps/masterbots.ai/components/layout/sidebar/sidebar-link.tsx (2)
Line range hint
90-111: Add missing dependencies to useCallback.The dependency array is missing
setSelectedCategoriesandsetSelectedChatbots. While React guarantees the stability of setState functions, it's best practice to include all dependencies for consistency and to prevent potential issues with future refactoring.const handleCheckboxChange = useCallback( (checked: boolean) => { setSelectedCategories(prev => checked ? [...prev, category.categoryId] : prev.filter(id => id !== category.categoryId) ) setSelectedChatbots(prev => checked ? [ ...prev, ...category.chatbots.map(chatbot => chatbot.chatbot.chatbotId) ] : prev.filter( id => !category.chatbots.some( chatbot => chatbot.chatbot.chatbotId === id ) ) ) }, - [category.categoryId, category.chatbots] + [category.categoryId, category.chatbots, setSelectedCategories, setSelectedChatbots] )
47-47: Document reasons for ignoring linting rules.The
biome-ignorecomments contain placeholder<explanation>text. Please provide clear explanations for why these linting rules need to be ignored. This helps other developers understand the rationale behind these exceptions and whether they're still necessary in future updates.Example of a well-documented lint ignore:
// biome-ignore lint/correctness/useExhaustiveDependencies: Router instance is stable and doesn't need to be in depsAlso applies to: 90-90
apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx (3)
13-23: Enhance component documentation.While the current JSDoc is sufficient, it would be helpful to document:
- The expected format of
children- The purpose of
isListItem- Example usage
/** * ClickableText component * Renders phrases as clickable links, triggering a message when clicked. + * + * @param children - React nodes containing the text to be processed + * @param isListItem - Indicates if the text is part of a list item + * @param sendMessageFromResponse - Optional callback to handle click events + * + * @example + * <ClickableText isListItem={false} sendMessageFromResponse={handleMessage}> + * Click here to learn about React + * </ClickableText> */
25-30: Add keyboard event handler for better accessibility.While the click handler works well, consider adding keyboard event handling for better accessibility.
const handleClick = () => { if (sendMessageFromResponse && clickableText.trim()) { const cleanedText = cleanClickableText(clickableText) sendMessageFromResponse(`Tell me more about ${cleanedText}`) } } +const handleKeyDown = (event: React.KeyboardEvent) => { + if (event.key === 'Enter' || event.key === ' ') { + handleClick() + } +}
38-44: Add explanation for the semantic elements lint ignore.The Biome ignore comment for semantic elements is missing an explanation.
- // biome-ignore lint/a11y/useSemanticElements: <explanation> + // biome-ignore lint/a11y/useSemanticElements: Using span with role="button" for styling consistencyapps/masterbots.ai/lib/hooks/use-profile.tsx (3)
10-14: Replaceanytype with a more specific return type.The
getuserInfomethod usesanywhich reduces type safety. Consider defining a proper return type interface.interface profileContextProps { - getuserInfo: (username: string) => Promise<any> + getuserInfo: (username: string) => Promise<{ user: User }> isSameUser: (userId: string) => boolean updateUserInfo: (bio: string | null, topic: string | null, profilePicture: string | null) => void currentUser: User | null, setCurrentUser: React.Dispatch<React.SetStateAction<User | null>> }🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
36-37: Remove unnecessary empty lines.These empty lines don't contribute to code readability.
82-83: Improve error message specificity.The generic error message could be more helpful to users by including specific failure reasons.
- console.error('Failed to update user info', error); - toast.error('Failed to update user info'); + const errorMessage = error instanceof Error ? error.message : 'Failed to update user info'; + console.error('Failed to update user info:', error); + toast.error(errorMessage);apps/masterbots.ai/components/routes/chat/chat-message.tsx (3)
16-22: Great job on interface documentation and co-location!Moving the interface closer to its usage point follows the principle of colocation and improves maintainability. The props are well-documented with clear descriptions.
Consider adopting this pattern of co-locating interfaces with their components across the codebase for better maintainability, unless the types are shared across multiple components.
Line range hint
85-116: Well-structured code block implementation with room for improvement.The code block implementation handles all cases well (streaming cursor, syntax highlighting, inline vs block). Consider adding comments explaining the magic character '▍' for better maintainability.
code({ node, inline, className, children, ...props }) { //* Handle cursor animation for streaming responses if (children.length) { - if (children[0] === '▍') + // '▍' represents the cursor character for streaming responses + if (children[0] === '▍')🧰 Tools
🪛 Biome (1.9.4)
[error] 88-88: Use === instead of ==. == is only allowed when comparing against
null== is only allowed when comparing against null
Using == may be unsafe if you are relying on type coercion
Unsafe fix: Use ===(lint/suspicious/noDoubleEquals)
121-121: Improve conditional rendering pattern.Instead of using an empty string, use null for the false case in conditional rendering. This follows React best practices.
- {actionRequired ? <ChatMessageActions className="md:!right-0" message={message} /> : ''} + {actionRequired ? <ChatMessageActions className="md:!right-0" message={message} /> : null}apps/masterbots.ai/components/routes/profile/user-card.tsx (7)
8-10: Optimize imports for better bundle size.Convert type-only imports to use the
typekeyword to ensure they're removed during compilation.-import { type Message, useChat } from "ai/react" -import { BookUser, BotIcon, ImagePlus, Loader, MessageSquareHeart, Wand2 } from 'lucide-react' -import { User } from 'mb-genql' +import { useChat } from "ai/react" +import type { Message } from "ai/react" +import { BookUser, BotIcon, ImagePlus, Loader, MessageSquareHeart, Wand2 } from 'lucide-react' +import type { User } from 'mb-genql'🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
153-157: Simplify cleanup effect.The cleanup effect can be simplified by removing the unnecessary state updates.
useEffect(() => { return () => { - setLastMessage(null); - setIsLoading(false); }; }, []);The state will be automatically cleaned up when the component unmounts.
160-164: Fix effect formatting.The effect's dependency array is incorrectly formatted.
useEffect(() => { // update bio and topic when user changes setBio(user?.bio) setFavouriteTopic(user?.favouriteTopic) - } - , [user]) + }, [user])
82-83: Remove console.error in production code.Replace console.error with proper error logging mechanism.
- console.error('Failed to upload image xx:', cloudinaryError); toast.error('Failed to upload image: \n' + cloudinaryError?.message);
128-149: Improve generateBio function structure.The function could be simplified with early returns and better error handling.
const generateBio = (type: string) => { + if (!userQuestions?.length) { + toast.error('No thread history available to generate content'); + return; + } + + setIsLoading(true); + setGenerateType(type); + try { - setIsLoading(true) - setGenerateType(type) - if (!userQuestions?.length) { - toast.error('No thread history available to generate content'); - setIsLoading(false); - return; - } const promptContent = UserPersonalityPrompt(type, userQuestions) return append({ id: nanoid(), content: promptContent, role: 'system', createdAt: new Date(), }) } catch (error) { - setIsLoading(false); toast.error('Failed to generate content: \n' + (error as Error).message); - console.error('Bio generation failed:', error); + return; + } finally { + setIsLoading(false); } }
293-299: Extract fallback avatar URL to a constant.The fallback URL is duplicated and should be extracted to a constant.
+ const FALLBACK_AVATAR_URL = 'https://api.dicebear.com/9.x/identicon/svg?seed=default_masterbots_ai_user_avatar'; <Image className="transition-opacity duration-300 rounded-full select-none size-full ring-1 ring-zinc-100/10 hover:opacity-80 object-cover" - src={userProfilePicture ? userProfilePicture : 'https://api.dicebear.com/9.x/identicon/svg?seed=default_masterbots_ai_user_avatar'} + src={userProfilePicture ?? FALLBACK_AVATAR_URL} alt={`${user.username}'s profile picture`} height={136} width={136} onError={(e) => { - e.currentTarget.src = 'https://api.dicebear.com/9.x/identicon/svg?seed=default_masterbots_ai_user_avatar' + e.currentTarget.src = FALLBACK_AVATAR_URL }} />
259-282: Remove commented-out code.The social features section is commented out and should be removed if not needed. If these features are planned for future implementation, consider tracking them in a separate issue.
Would you like me to help create an issue to track the implementation of these social features?
apps/masterbots.ai/tailwind.config.js (1)
4-8: Consider documenting content paths configuration.To help future maintainers understand the project structure, consider adding a comment explaining which directories should be included in the content paths configuration.
module.exports = { darkMode: ['class'], content: [ + // Scan all TypeScript/React components for Tailwind classes './components/**/*.{ts,tsx}', './app/**/*.{ts,tsx}', './src/**/*.{ts,tsx}' ],apps/masterbots.ai/lib/hooks/use-mb-chat.ts (3)
3-6: Consider grouping related imports together.The imports could be better organized by grouping them into:
- External dependencies
- Internal utilities and hooks
- Types
- Constants
+ // External dependencies import { useChat } from 'ai/react' import { uniqBy } from 'lodash' import { useSession } from 'next-auth/react' import { useParams } from 'next/navigation' import { useEffect, useRef } from 'react' import toast from 'react-hot-toast' import { useAsync, useSetState } from 'react-use' + // Internal utilities and hooks import { improveMessage } from '@/app/actions' import { formatSystemPrompts } from '@/lib/actions' import { useModel } from '@/lib/hooks/use-model' import { useSidebar } from '@/lib/hooks/use-sidebar' import { useThread } from '@/lib/hooks/use-thread' import { delayFetch } from '@/lib/utils' + // Services import { createThread, deleteThread, getMessages, getThread, saveNewMessage } from '@/services/hasura' + // Constants import { followingQuestionsPrompt, setDefaultUserPreferencesPrompt } from '@/lib/constants/prompts' + // Types import type { AiClientType, AiToolCall } from '@/types/types' import type { Message as AiMessage, ChatRequestOptions, CreateMessage } from 'ai' import type { Chatbot, Message, Thread } from 'mb-genql'Also applies to: 19-23
271-273: Remove commented-out code.Consider removing the commented-out code block about chat attachments. If this is a planned feature, track it in your issue tracker instead.
326-329: Enhance error messages with more context.The error messages could be more helpful by including specific details about what went wrong and how to resolve it.
- console.error('Error processing user message. Using og message. Error: ', error) + console.error( + `Failed to process message: ${error instanceof Error ? error.message : 'Unknown error'}. ` + + 'Falling back to original message.', + { error } + ) - toast.error('Failed to load messages. Please try again.') + toast.error( + 'Failed to load chat history. Please check your connection and try again.', + { id: 'load-messages-error' } + )Also applies to: 347-347
apps/masterbots.ai/app/api/og/route.tsx (1)
45-45: Enhance error logging for better debuggingCurrently, only the error message is logged. Consider logging the full error stack to aid in debugging.
Apply this diff to log the error stack:
console.log(`${e.message}`) +console.error(e)apps/masterbots.ai/components/layout/sidebar/sidebar.tsx (2)
19-19: Provide a valid explanation for the lint ignore commentThe comment
// biome-ignore lint/correctness/useExhaustiveDependencies: <explanation>contains a placeholder<explanation>. Please replace it with a meaningful explanation for why the lint rule is being ignored.Example:
-// biome-ignore lint/correctness/useExhaustiveDependencies: <explanation> +// biome-ignore lint/correctness/useExhaustiveDependencies: Dependencies are managed manually to avoid unnecessary re-renders
37-39: Consider using theme variables instead of hardcoded color valuesHardcoding hex color values can make future maintenance and theming more difficult. Consider using CSS variables or a theme configuration to manage colors for better scalability and consistency.
Example:
isChatRoute - ? 'bg-[#fae8ff] dark:bg-[#17021D]' // For /c routes only - : 'bg-[#eeffea] dark:bg-[#0B1608]' // For other routes + ? 'bg-theme-chat-light dark:bg-theme-chat-dark' // For /c routes only + : 'bg-theme-other-light dark:bg-theme-other-dark' // For other routesThen, define the theme classes in your CSS or Tailwind configuration.
apps/masterbots.ai/components/auth/forgot-password-form.tsx (1)
37-37: Standardize error messages across notifications.While the implementation is correct, consider standardizing error messages for consistency:
- Line 40: "An error occurred" vs Line 44: "An unexpected error occurred"
- Consider using more specific error messages when possible
- toast.error(data.error || 'An error occurred') + toast.error(data.error || 'An unexpected error occurred')Also applies to: 40-40, 44-44
apps/masterbots.ai/lib/url.ts (2)
1-1: Optimize type imports.Use
import typefor type-only imports to ensure proper tree-shaking.-import { z, ZodSchema } from 'zod' +import { z } from 'zod' +import type { ZodSchema } from 'zod'🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
Line range hint
12-14: Enhance URL encoding robustness.The
encodeQueryfunction might not handle all special characters correctly. Consider using a more comprehensive approach.export const encodeQuery = (input: string): string => { - return encodeURIComponent(input).replace(/%20/g, '+').replace(/ /g, '+') + return encodeURIComponent(input) + .replace(/%20/g, '+') + .replace(/ /g, '+') + .replace(/[!'()]/g, escape) + .replace(/\*/g, '%2A')🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
apps/masterbots.ai/components/routes/browse/browse-chatbot-details.tsx (1)
32-32: Enhance error messages with more context.The current error messages are generic. Consider providing more specific information:
- Line 32: Include the actual error message from response
- Line 34: Specify the type of request that failed
- Line 39: Include the error details
- Line 72: Add context about what content failed to generate
- toast.error(response.statusText) + toast.error(`Authentication failed: ${response.statusText}`) - toast.error('Failed to process request') + toast.error(`Failed to process chatbot request: ${response.statusText}`) - toast.error('An error occurred') + toast.error(`Failed to communicate with chatbot: ${error.message}`) - toast.error('Failed to generate content') + toast.error('Failed to generate chatbot bio')Also applies to: 34-34, 39-39, 72-72
apps/masterbots.ai/app/layout.tsx (1)
Line range hint
24-26: Consider adding position and duration options to Toaster.The current Toaster configuration only sets the className. Consider adding position and duration options for better UX.
<Toaster toastOptions={{ className: 'bg-background text-background-foreground', + position: 'top-right', + duration: 4000, }} />🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
apps/masterbots.ai/components/routes/chat/chat-share-dialog.tsx (1)
72-74: Improve messages count display.The messages count is displayed without context, making it unclear what the number represents.
Apply this diff to improve clarity:
- <div className="text-muted-foreground"> - {chat.messages.length} - </div> + <div className="text-muted-foreground"> + {chat.messages.length} message{chat.messages.length === 1 ? '' : 's'} + </div>apps/masterbots.ai/components/layout/header/user-menu.tsx (1)
18-18: Remove unnecessary empty lines.These empty lines don't serve any purpose and affect code readability.
Remove the empty lines at 18 and 34.
Also applies to: 34-34
apps/masterbots.ai/components/routes/chat/chat-chatbot-details.tsx (1)
76-79: Implement proper follow functionality placeholder.The current implementation only logs to console, which might be insufficient for tracking feature usage or debugging.
Consider adding a toast notification to inform users that the feature is coming soon:
const handleFollow = () => { - // follow logic once ready goes here - console.log('Following:', botName) + toast.info(`Following feature for ${botName} coming soon!`) }apps/masterbots.ai/components/auth/reset-password-form.tsx (1)
72-79: Consider enhancing error handling with more specific error messages.The current error handling could be more informative for users. Consider extracting specific error messages from the API response when available.
if (response.ok) { toast.success(data.message) router.push('/auth/signin') } else { - toast.error(data.error || 'An unexpected error occurred') + const errorMessage = data.error?.message || data.error || 'An unexpected error occurred' + toast.error(errorMessage) } } catch (error) { console.error('Error:', error) -toast.error('An unexpected error occurred') +toast.error(error instanceof Error ? error.message : 'An unexpected error occurred')apps/masterbots.ai/app/actions/ai-executers.ts (2)
134-154: Consider making the example format more generic or moving it to a constant.The hard-coded example format with specific baseball-related content might be misleading as it suggests a specific domain. Consider making it more generic or moving it to a configuration file.
+const SEARCH_RESULT_TEMPLATE = ` +**Resume:** +[Content] + +**Summary:** +[Content] + +**Key Points:** +[Content] + +**Sources:** +1. [URL1] +2. [URL2] +` + return `${response.outputs['web search'].output} -## EXAMPLE: -... +${SEARCH_RESULT_TEMPLATE}`
156-157: Enhance error logging with error details.The current error logging could be more informative for debugging purposes.
-console.error('Error fetching app data: ', error) +console.error('Error in getWebSearchTool: ', { + error, + query, + status: error?.response?.status +})apps/masterbots.ai/components/layout/sidebar/profile-sidebar.tsx (1)
46-57: Consider adding URL validation for the user slug.While the URL construction is simplified, consider adding validation to ensure the user slug meets the expected format before navigation.
const goToProfile = useCallback( (e: React.MouseEvent) => { e.preventDefault() e.stopPropagation() const userSlug = toSlugWithUnderScore(user.name || '') - if (userSlug) { + if (userSlug && /^[a-z0-9-_]+$/i.test(userSlug)) { setIsOpen(false) router.push(`/u/${userSlug}/t`) + } else { + console.warn('Invalid user slug format:', userSlug) } }, [router, user.name] )apps/masterbots.ai/lib/hooks/use-thread-visibility.tsx (2)
91-127: Enhance error handling in initiateDeleteThread.The error handling can be improved by:
- Using early returns for better readability
- Adding type safety for the error object
const initiateDeleteThread = async (threadId: string): Promise<DeleteThreadResponse> => { try { if (!session?.data?.user?.id || !jwt) { - return { - success: false, - message: 'User session not found', - error: 'User session not found. Please log in again.' - }; + throw new Error('User session not found. Please log in again.'); }; const deleteT = await deleteThread({ threadId, jwt, userId: session?.data?.user.id }); await getThreadForUser(); - if(deleteT.success){ + if (deleteT.success) { return { success: true, message: 'Thread deleted successfully', error: null }; } - return { - success: false, - message: deleteT.error || 'Failed to delete thread', - error: deleteT.error || 'An unknown error occurred' - }; + throw new Error(deleteT.error || 'Failed to delete thread'); } catch (error) { console.error('Error deleting thread:', error); return { success: false, message: 'Failed to delete thread', - error: error instanceof Error ? error.message : 'An unknown error occurred' + error: error instanceof Error ? error.message : String(error) }; } }
143-150: Add loading state for admin mode toggle.The admin mode toggle involves asynchronous operations but lacks loading state management, which could lead to a poor user experience during the transition.
+const [isAdminLoading, setIsAdminLoading] = useState(false); const handleToggleAdminMode = async () => { + setIsAdminLoading(true); try { if (!isAdminMode) { await loadUnapprovedThreads(); } else { await getThreadForUser(); } setIsAdminMode(!isAdminMode); + } finally { + setIsAdminLoading(false); + } }apps/masterbots.ai/components/routes/chat/chat-options.tsx (1)
53-55: Enhance error messages for better user feedback.Consider providing more specific error messages to help users understand and resolve issues.
- toast.success(result.message) + toast.success('Thread deleted successfully') } else { - toast.error(result?.message) + toast.error(result?.message || 'Failed to delete thread. Please try again later.') }apps/masterbots.ai/lib/utils.ts (2)
29-30: LGTM! Consider simplifying the error handling.The error handling logic is correct, but can be simplified by removing the unnecessary
elseclause since theifblock always throws.- } else { - throw new Error('An unexpected error occurred') - } + } + throw new Error('An unexpected error occurred')🧰 Tools
🪛 Biome (1.9.4)
[error] 29-31: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
93-95: Simplify the control flow.The control flow can be simplified by:
- Removing the unnecessary
elseclause since theifblock breaks- Removing the redundant
continueas it's the last statement in the loopif (!chatGptMessage) { break - } else { - chatGptMessages.push(chatGptMessage) - continue - } + } + chatGptMessages.push(chatGptMessage)🧰 Tools
🪛 Biome (1.9.4)
[error] 93-96: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 95-95: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
apps/masterbots.ai/app/globals.css (1)
53-54: Consider DRYing up the gradient definitions.The gradient definitions are duplicated between light and dark modes. Consider moving them to a shared location since they're identical.
@layer base { :root { /* ... other variables ... */ /* Shared gradients */ + --public-gradient: linear-gradient(180deg, rgba(115, 201, 97, 0.2) 0%, rgba(122, 214, 104, 0.4) 100%); + --public-hover-gradient: linear-gradient(-180deg, rgba(113, 199, 96, 0.1) 0%, rgba(117, 205, 99, 0.3) 100%); + --chat-gradient: linear-gradient(180deg, rgba(155, 22, 232, 0.2) 0%, rgba(190, 22, 232, 0.53) 100%); + --chat-hover-gradient: linear-gradient(-180deg, rgba(166, 22, 232, 0.1) 0%, rgba(190, 22, 232, 0.3) 100%); - --background-public-gradient: linear-gradient(180deg, rgba(115, 201, 97, 0.2) 0%, rgba(122, 214, 104, 0.4) 100%); - --background-public-hover-gradient: linear-gradient(-180deg, rgba(113, 199, 96, 0.1) 0%, rgba(117, 205, 99, 0.3) 100%); + --background-public-gradient: var(--public-gradient); + --background-public-hover-gradient: var(--public-hover-gradient); - --background-chat-gradient: linear-gradient(180deg, rgba(155, 22, 232, 0.2) 0%, rgba(190, 22, 232, 0.53) 100%); - --background-chat-hover-gradient: linear-gradient(-180deg, rgba(166, 22, 232, 0.1) 0%, rgba(190, 22, 232, 0.3) 100%); + --background-chat-gradient: var(--chat-gradient); + --background-chat-hover-gradient: var(--chat-hover-gradient); } .dark { /* ... other variables ... */ - --background-public-gradient: linear-gradient(180deg, rgba(115, 201, 97, 0.2) 0%, rgba(122, 214, 104, 0.4) 100%); - --background-public-hover-gradient: linear-gradient(-180deg, rgba(113, 199, 96, 0.1) 0%, rgba(117, 205, 99, 0.3) 100%); + --background-public-gradient: var(--public-gradient); + --background-public-hover-gradient: var(--public-hover-gradient); - --background-chat-gradient: linear-gradient(180deg, rgba(155, 22, 232, 0.2) 0%, rgba(190, 22, 232, 0.53) 100%); - --background-chat-hover-gradient: linear-gradient(-180deg, rgba(166, 22, 232, 0.1) 0%, rgba(190, 22, 232, 0.3) 100%); + --background-chat-gradient: var(--chat-gradient); + --background-chat-hover-gradient: var(--chat-hover-gradient); } }Also applies to: 57-58, 105-106, 109-110
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (45)
apps/masterbots.ai/app/(browse)/page.tsx(0 hunks)apps/masterbots.ai/app/actions/ai-executers.ts(1 hunks)apps/masterbots.ai/app/actions/ai-main-call.ts(3 hunks)apps/masterbots.ai/app/api/og/route.tsx(2 hunks)apps/masterbots.ai/app/b/[id]/page.tsx(1 hunks)apps/masterbots.ai/app/c/page.tsx(1 hunks)apps/masterbots.ai/app/globals.css(3 hunks)apps/masterbots.ai/app/layout.tsx(3 hunks)apps/masterbots.ai/components/auth/forgot-password-form.tsx(2 hunks)apps/masterbots.ai/components/auth/reset-password-form.tsx(3 hunks)apps/masterbots.ai/components/auth/signup-form.tsx(9 hunks)apps/masterbots.ai/components/layout/header/user-menu.tsx(5 hunks)apps/masterbots.ai/components/layout/profile/profile-page-sidebar.tsx(1 hunks)apps/masterbots.ai/components/layout/providers.tsx(1 hunks)apps/masterbots.ai/components/layout/sidebar/profile-sidebar.tsx(1 hunks)apps/masterbots.ai/components/layout/sidebar/sidebar-actions.tsx(5 hunks)apps/masterbots.ai/components/layout/sidebar/sidebar-category-general.tsx(1 hunks)apps/masterbots.ai/components/layout/sidebar/sidebar-item.tsx(2 hunks)apps/masterbots.ai/components/layout/sidebar/sidebar-link.tsx(3 hunks)apps/masterbots.ai/components/layout/sidebar/sidebar.tsx(3 hunks)apps/masterbots.ai/components/routes/browse/browse-chatbot-details.tsx(3 hunks)apps/masterbots.ai/components/routes/browse/browse-list.tsx(2 hunks)apps/masterbots.ai/components/routes/chat/chat-chatbot-details.tsx(2 hunks)apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx(1 hunks)apps/masterbots.ai/components/routes/chat/chat-message.tsx(4 hunks)apps/masterbots.ai/components/routes/chat/chat-options.tsx(2 hunks)apps/masterbots.ai/components/routes/chat/chat-share-dialog.tsx(5 hunks)apps/masterbots.ai/components/routes/chat/new-chat.tsx(2 hunks)apps/masterbots.ai/components/routes/chat/onboarding-chatbot-details.tsx(2 hunks)apps/masterbots.ai/components/routes/chat/onboarding-chatbot-mobile-details.tsx(2 hunks)apps/masterbots.ai/components/routes/profile/user-card.tsx(4 hunks)apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx(1 hunks)apps/masterbots.ai/lib/constants/prompts.ts(1 hunks)apps/masterbots.ai/lib/hooks/use-mb-chat.ts(17 hunks)apps/masterbots.ai/lib/hooks/use-profile.tsx(4 hunks)apps/masterbots.ai/lib/hooks/use-thread-visibility.tsx(1 hunks)apps/masterbots.ai/lib/metadata.ts(0 hunks)apps/masterbots.ai/lib/threads.ts(0 hunks)apps/masterbots.ai/lib/url.ts(2 hunks)apps/masterbots.ai/lib/utils.ts(3 hunks)apps/masterbots.ai/package.json(3 hunks)apps/masterbots.ai/services/hasura/hasura.service.ts(35 hunks)apps/masterbots.ai/services/hasura/hasura.service.type.ts(0 hunks)apps/masterbots.ai/tailwind.config.js(1 hunks)apps/masterbots.ai/types/types.ts(0 hunks)
💤 Files with no reviewable changes (5)
- apps/masterbots.ai/lib/metadata.ts
- apps/masterbots.ai/app/(browse)/page.tsx
- apps/masterbots.ai/lib/threads.ts
- apps/masterbots.ai/services/hasura/hasura.service.type.ts
- apps/masterbots.ai/types/types.ts
✅ Files skipped from review due to trivial changes (3)
- apps/masterbots.ai/components/layout/sidebar/sidebar-category-general.tsx
- apps/masterbots.ai/components/routes/chat/onboarding-chatbot-mobile-details.tsx
- apps/masterbots.ai/services/hasura/hasura.service.ts
🧰 Additional context used
🪛 Biome (1.9.4)
apps/masterbots.ai/components/layout/sidebar/sidebar-actions.tsx
[error] 22-22: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
apps/masterbots.ai/components/routes/chat/chat-message.tsx
[error] 88-88: Use === instead of ==. == is only allowed when comparing against null
== is only allowed when comparing against null
Using == may be unsafe if you are relying on type coercion
Unsafe fix: Use ===
(lint/suspicious/noDoubleEquals)
apps/masterbots.ai/components/routes/browse/browse-list.tsx
[error] 138-138: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
apps/masterbots.ai/components/routes/profile/user-card.tsx
[error] 10-10: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 12-12: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
apps/masterbots.ai/app/api/og/route.tsx
[error] 4-5: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
apps/masterbots.ai/lib/utils.ts
[error] 29-31: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 93-96: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 95-95: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
apps/masterbots.ai/lib/hooks/use-mb-chat.ts
[error] 126-126: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 127-127: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 139-139: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 148-148: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 173-173: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 174-174: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 249-249: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 250-250: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 257-257: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 294-294: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 295-295: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 309-309: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
apps/masterbots.ai/lib/hooks/use-profile.tsx
[error] 10-10: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 4-4: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 40-40: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
apps/masterbots.ai/lib/url.ts
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
apps/masterbots.ai/lib/hooks/use-thread-visibility.tsx
[error] 52-52: This hook specifies more dependencies than necessary: jwt
This dependency can be removed from the list.
(lint/correctness/useExhaustiveDependencies)
🔇 Additional comments (32)
apps/masterbots.ai/components/layout/sidebar/sidebar-actions.tsx (4)
15-15: LGTM! Icon imports consolidated and toast library added.The changes align with the PR objective of replacing the notification system.
Also applies to: 25-25
42-47: LGTM! Clean hook usage and visibility implementation.The destructuring is well-formatted and the visibility check is implemented effectively.
Also applies to: 50-50
54-56: LGTM! Clean notification system migration.The transition from sonner to react-hot-toast is well-implemented with proper error and success handling.
Also applies to: 68-68, 75-75
102-102: LGTM! Improved accessibility and tooltip consistency.The screen reader text and tooltip content updates enhance user experience.
Also applies to: 105-105
apps/masterbots.ai/components/layout/sidebar/sidebar-link.tsx (1)
38-38: LGTM: State management addition is consistent.The addition of
selectedChatbotsto the destructured values fromuseSidebarhook aligns with the component's functionality for managing chatbot selections.apps/masterbots.ai/components/layout/sidebar/sidebar-item.tsx (1)
15-15: LGTM! Good type-only import.The change to a type-only import for the Chat type is appropriate since it's only used for type annotations. This change follows TypeScript best practices and helps with tree-shaking.
apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx (1)
1-11: LGTM! Clean imports and well-defined interface.The imports are focused and the interface is properly typed with clear prop definitions.
apps/masterbots.ai/app/c/page.tsx (1)
44-44: Verify if removing the Open Graph image URL is intentional.Setting
ogImageUrlto an empty string will affect how this page appears when shared on social media platforms. If this was changed to resolve merge conflicts, please verify if removing the dynamic URL generation was intentional.Let's check if other pages still use dynamic Open Graph images:
✅ Verification successful
Empty
ogImageUrlis consistent with the codebase patternThe empty
ogImageUrlin this file follows an established pattern where:
- List/category pages use the default image (
/masterbots_og.png)- Only specific pages (user profiles, chatbot pages) use dynamic images
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other ogImageUrl patterns in the codebase echo "Searching for other ogImageUrl patterns..." rg "ogImageUrl.*BASE_URL" -A 2 -B 2Length of output: 132
Script:
#!/bin/bash # Broader search for ogImageUrl patterns and metadata configurations echo "Searching for ogImageUrl patterns..." rg "ogImageUrl" -A 2 -B 2 echo -e "\nSearching for metadata configurations..." rg "metadata.*=.*{" -A 5 -B 1 echo -e "\nSearching for image-related metadata..." rg "openGraph.*image" -A 2 -B 2Length of output: 5311
apps/masterbots.ai/lib/hooks/use-profile.tsx (4)
17-19: LGTM!The context creation formatting changes improve readability while maintaining functionality.
45-48: LGTM!The consolidated sessionSlug logic and addition of isSameUser parameter improve code clarity.
70-72: LGTM!The addition of JWT validation improves security by ensuring proper authentication.
89-89: LGTM!The consolidated provider value improves code conciseness while maintaining readability.
apps/masterbots.ai/app/b/[id]/page.tsx (2)
67-67: Verify avatar URL formatting for Open Graph compatibility.Ensure that
chatbot.avatarprovides an absolute URL that is properly formatted for Open Graph images. Open Graph requires absolute URLs.Run this script to check the avatar URL format:
✅ Verification successful
Avatar URL handling is properly configured for Open Graph.
The application has correct
metadataBaseconfiguration inapp/layout.tsxwhich ensures all metadata URLs, including the avatar URL, are automatically converted to absolute URLs by Next.js. No additional URL formatting is required.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check avatar URL patterns in the codebase # Look for avatar URL generation or assignment patterns # Search for avatar-related code rg -A 5 'avatar.*=' # Search for URL construction patterns rg -A 5 'new URL|\.origin|absolute.*url'Length of output: 5343
67-67: Verify the impact of removing thread-based Open Graph images.The change from thread-based to avatar-based Open Graph images might affect social media sharing features. Please confirm if this is intentional and that no other components rely on thread-based image URLs.
Run this script to check for potential dependencies:
✅ Verification successful
No dependencies found on thread-based Open Graph images
The switch from thread-based to avatar-based Open Graph images is safe to implement. The codebase shows no direct dependencies on thread-based image URLs, and the sharing functionality operates independently of the OG image source. Using chatbot avatars will provide consistent branding across social media shares.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Look for code that might depend on thread-based Open Graph images # Search for thread-related metadata or URL patterns rg 'thread.*og(Image|Url)|og(Image|Url).*thread' # Search for social sharing related code rg -A 5 'share|social|meta.*image'Length of output: 130984
apps/masterbots.ai/components/routes/chat/chat-message.tsx (2)
1-15: LGTM! Well-organized imports with proper attribution.The imports are cleanly organized and the source attribution is properly maintained.
31-33: LGTM! Clean implementation of message sanitization.Good practice using immutable operations by creating a new object instead of mutating the original message.
apps/masterbots.ai/package.json (1)
54-54: Verify the impact of major version downgrades.Several dependencies have been downgraded to older versions:
- lucide-react: ^0.473.0 → ^0.468.0
- next-themes: ^0.4.4 → ^0.2.1
These downgrades might introduce compatibility issues or remove needed features.
Run the following script to check for breaking changes in the downgraded versions:
Also applies to: 62-62
✅ Verification successful
Both dependency downgrades are safe to proceed with
The codebase only uses basic icons from lucide-react and standard theming features from next-themes. The breaking changes in next-themes v0.2.1 actually improve CSP support and don't affect current implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check changelogs for breaking changes in downgraded packages # Check lucide-react changelog gh api repos/lucide-icons/lucide/releases --paginate | jq -r '.[] | select(.tag_name | contains("[email protected]")) | .body' # Check next-themes changelog gh api repos/pacocoursey/next-themes/releases --paginate | jq -r '.[] | select(.tag_name | contains("v0.2")) | .body'Length of output: 833
Script:
#!/bin/bash # Check lucide-react icon usage and next-themes implementation # Search for lucide-react imports and icon usage echo "=== Lucide React Usage ===" rg "from ['|\"]lucide-react['|\"]" -A 1 # Search for next-themes script tags and configuration echo -e "\n=== Next-themes Usage ===" rg "next/script|ThemeProvider|useTheme" -A 2Length of output: 65731
apps/masterbots.ai/lib/hooks/use-mb-chat.ts (1)
Line range hint
322-325: Document the performance improvement TODO.The TODO comment indicates a potential performance issue but lacks context about the specific problem and potential solutions.
Consider adding more detailed documentation:
/** * TODO: Improve response velocity * Current issue: UI feedback is delayed until active thread update * Potential solutions: * 1. Split into separate callback functions * 2. Implement optimistic updates * 3. Use React Suspense for loading states */✅ Verification successful
Document the performance bottleneck and solutions.
/** * TODO: Improve response velocity * Current flow: * 1. Update active thread (optimistic update) * 2. Set loading state * 3. Process user content (async) * 4. Show popup feedback * * Issue: UI feedback (popup) is delayed until after content processing, * even though thread update is immediate * * Potential solutions: * 1. Move popup before content processing: * + Immediate feedback * - May need to handle error states * * 2. Split into separate effects: * + Parallel execution * + Better separation of concerns * - More complex state management * * 3. Use React Suspense: * + Built-in loading states * + Consistent loading UX * - Requires component restructuring */🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find the complete function implementation and surrounding context ast-grep --pattern 'setIsOpenPopup(true)' -A 10 -B 30 # Search for similar performance-related TODO comments rg -i "todo.*performance|todo.*velocity|todo.*improve.*response" -g '*.ts' -g '*.tsx' # Look for React Suspense usage patterns rg "Suspense|useTransition|useDeferredValue" -g '*.ts' -g '*.tsx' # Find other instances of setIsOpenPopup rg "setIsOpenPopup" -g '*.ts' -g '*.tsx'Length of output: 66196
apps/masterbots.ai/components/layout/providers.tsx (1)
17-19: SimplifyProviderscomponent by usingThemeProviderPropsRefactoring the
Providerscomponent to useThemeProviderPropsand spreading...propsintoNextThemesProvidersimplifies the code and enhances reusability. Good job on this improvement.apps/masterbots.ai/app/layout.tsx (1)
60-60: Verify image availability at hardcoded URLs.The static URLs for OpenGraph and Twitter card images might cause issues if:
- The images are not deployed to production
- The URLs need to be environment-specific
Also applies to: 75-75
✅ Verification successful
Image URL verification successful
The image is properly deployed and accessible in production with CDN caching enabled.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify image availability curl -I https://masterbots.ai/images/masterbots.pngLength of output: 985
apps/masterbots.ai/components/routes/chat/new-chat.tsx (1)
59-61: Address potential race condition in chat completion.The console log message "NEW CHAT FINISHED FIRST, NOT GOOD" suggests an unresolved race condition. This could lead to messages being processed in the wrong order or lost.
Please verify the message ordering and consider implementing a proper message queue or synchronization mechanism. Run this script to check for similar race condition handling patterns in the codebase:
apps/masterbots.ai/components/routes/chat/chat-chatbot-details.tsx (1)
88-88:⚠️ Potential issueReplace hardcoded follower count.
The follower count is hardcoded to 3200 with a comment indicating it needs to be changed. This could mislead users.
Consider one of these approaches:
- Remove the follower count display until the following feature is ready
- Set it to 0 with a "Coming soon" indicator
- Implement the actual follower count tracking
Apply this diff for approach #2:
- followersCount: 3200, // This nees to be changed once following feat is ready + followersCount: 0, // Feature coming soon⛔ Skipped due to learnings
Learnt from: Bran18 PR: bitcashorg/masterbots#314 File: apps/masterbots.ai/components/routes/chat/chat-chatbot-details.tsx:130-132 Timestamp: 2024-11-25T19:41:33.853Z Learning: In the file `apps/masterbots.ai/components/routes/chat/chat-chatbot-details.tsx`, the follower count displayed as "3.2k" is a temporary hardcoded value that will be replaced with real data from the database eventually.apps/masterbots.ai/components/routes/chat/onboarding-chatbot-details.tsx (1)
Line range hint
1-82: LGTM! Clean removal of follower-related functionality.The changes effectively streamline the component by removing unnecessary complexity around follower counts and thread counts, while maintaining the core functionality of displaying chatbot details.
apps/masterbots.ai/components/auth/reset-password-form.tsx (1)
11-11: LGTM! Clean migration to react-hot-toast.The change from useSonner to react-hot-toast aligns with the standardization of notification handling across the application.
apps/masterbots.ai/components/layout/profile/profile-page-sidebar.tsx (1)
73-73: LGTM! URL construction is consistent.The URL construction using template literals is consistent with the pattern used across the codebase.
apps/masterbots.ai/lib/constants/prompts.ts (1)
70-71: LGTM! Improved clarity of the bot configuration prompt.The changes make the formatting instruction more concise and clearer, while preserving the essential functionality.
apps/masterbots.ai/components/auth/signup-form.tsx (2)
Line range hint
54-86: LGTM! Successfully migrated to react-hot-toast.The notification system has been consistently updated to use react-hot-toast for error and success messages.
109-110: LGTM! Added verification link expiry notice.Good addition of the 15-day expiry notice, providing important information to users about the verification link validity period.
apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx (1)
Line range hint
217-223: LGTM! Well-integrated search functionality.The ChatSearchInput component is:
- Conditionally rendered only when threads exist
- Properly positioned with responsive padding
- Well-integrated with existing states
apps/masterbots.ai/app/actions/ai-main-call.ts (1)
226-226: Verify the impact of reduced tool roundtrips.The maxToolRoundtrips parameter has been reduced from 2 to 1 for all AI clients. This could affect the quality of responses that require multiple tool uses, particularly for web searches.
Run the following script to analyze the impact:
Also applies to: 245-245, 265-265
✅ Verification successful
Reduced tool roundtrips appear to be an intentional optimization
The change from 2 to 1 roundtrip is consistently applied across all AI clients in the core implementation. No error logs or issues related to tool usage were found, and web search functionality remains properly feature-flagged and controlled.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for web search usage patterns in threads # Look for web search tool usage patterns rg -A 5 'webSearch.*true' . # Check for any error logs related to tool usage rg -i 'tool.*error|failed.*tool' .Length of output: 80009
Script:
#!/bin/bash # Search for tool usage patterns and error handling # Look for tool usage implementation ast-grep --pattern 'function $FUNC($$$) { $$$ maxToolRoundtrips $$$ }' # Search for tool-related error handling rg -A 5 'catch.*tool|error.*tool|fail.*tool' . # Find configuration usage rg -A 3 'maxToolRoundtrips' .Length of output: 103096
Script:
#!/bin/bash # Search for tool configuration and web search implementation # Look for tool configuration and web search implementation rg -A 5 "webSearch.*config|tool.*config" . # Search for AI client initialization and configuration ast-grep --pattern 'new $CLIENT({ $$$ maxToolRoundtrips $$$ })' # Find web search related functions rg -A 10 "async.*webSearch|function.*webSearch" .Length of output: 13236
Script:
#!/bin/bash # Final search for tool configuration and usage # Look for direct maxToolRoundtrips references rg -l "maxToolRoundtrips" . # Search for tool configuration files fd -e ts -e js -e json . | xargs rg -l "tool.*config|maxTool|webSearch" # Find files containing tool implementation rg -l "class.*Tool|interface.*Tool|type.*Tool" .Length of output: 779
apps/masterbots.ai/lib/utils.ts (1)
364-364: LGTM! Clean implementation of route type detection.The function provides a clean way to determine the route type, which is used for applying route-specific gradient styles.
apps/masterbots.ai/app/globals.css (1)
63-63: LGTM! Improved dark mode contrast.The change from 98% to 100% white for the foreground color in dark mode improves text contrast and readability.
| @@ -1,6 +1,6 @@ | |||
| 'use client' | |||
|
|
|||
| import * as React from 'react' | |||
| import type * as React from 'react' | |||
There was a problem hiding this comment.
Revert React import to include runtime dependency.
The change to a type-only import (import type * as React) in a Client Component is incorrect. Since this is a 'use client' component that uses React's runtime features, it requires the actual React import, not just its types.
Apply this diff to fix the import:
-import type * as React from 'react'
+import * as React from 'react'📝 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.
| import type * as React from 'react' | |
| import * as React from 'react' |
| @@ -1,20 +1,22 @@ | |||
| 'use client' | |||
|
|
|||
| import { getUserBySlug, updateUserPersonality } from '@/services/hasura' | |||
| import type { User } from 'mb-genql' | |||
| import { User } from 'mb-genql' | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use import type for type-only imports.
The User type should be imported using import type to ensure proper tree-shaking and avoid loading unnecessary modules at runtime.
-import { User } from 'mb-genql'
+import type { User } from 'mb-genql'📝 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.
| import { User } from 'mb-genql' | |
| import type { User } from 'mb-genql' |
🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
| code({ node, inline, className, children, ...props }) { | ||
| //* Handle cursor animation for streaming responses | ||
| if (children.length) { | ||
| if (children[0] == '▍') { | ||
| if (children[0] == '▍') | ||
| return ( | ||
| <span className="mt-1 cursor-default animate-pulse">▍</span> | ||
| ) | ||
| } | ||
| children[0] = (children[0] as string).replace('▍', '▍') | ||
|
|
||
| children[0] = (children[0] as string).replace('`▍`', '▍') |
There was a problem hiding this comment.
Fix unsafe equality comparison.
Replace the double equals comparison with strict equality to prevent type coercion issues.
- if (children[0] == '▍')
+ if (children[0] === '▍')📝 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.
| code({ node, inline, className, children, ...props }) { | |
| //* Handle cursor animation for streaming responses | |
| if (children.length) { | |
| if (children[0] == '▍') { | |
| if (children[0] == '▍') | |
| return ( | |
| <span className="mt-1 cursor-default animate-pulse">▍</span> | |
| ) | |
| } | |
| children[0] = (children[0] as string).replace('▍', '▍') | |
| children[0] = (children[0] as string).replace('`▍`', '▍') | |
| code({ node, inline, className, children, ...props }) { | |
| //* Handle cursor animation for streaming responses | |
| if (children.length) { | |
| if (children[0] === '▍') | |
| return ( | |
| <span className="mt-1 cursor-default animate-pulse">▍</span> | |
| ) | |
| children[0] = (children[0] as string).replace('`▍`', '▍') |
🧰 Tools
🪛 Biome (1.9.4)
[error] 88-88: Use === instead of ==. == is only allowed when comparing against null
== is only allowed when comparing against null
Using == may be unsafe if you are relying on type coercion
Unsafe fix: Use ===
(lint/suspicious/noDoubleEquals)
| @@ -4,8 +4,7 @@ module.exports = { | |||
| content: [ | |||
| './components/**/*.{ts,tsx}', | |||
| './app/**/*.{ts,tsx}', | |||
| './src/**/*.{ts,tsx}', | |||
| './lib/**/*.{ts,tsx}' | |||
| './src/**/*.{ts,tsx}' | |||
There was a problem hiding this comment.
💡 Codebase verification
Revert removal of lib directory from Tailwind content paths
The lib directory exists and contains React components that require Tailwind styling. Removing './lib/**/*.{ts,tsx}' from the content paths will cause styles to be missing for components in the lib directory.
lib/hooks/useSonner.tsxand other components in lib directory use Tailwind classes- Many components across the application import and use these lib components
- The lib directory is at the root level, not inside src, so it needs its own content path
🔗 Analysis chain
Verify removal of lib directory from content paths.
The removal of './lib/**/*.{ts,tsx}' from content paths could lead to missing styles if any components still exist in the lib directory.
Let's verify if there are any remaining TypeScript/React components in the lib directory that might need styling:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if lib directory exists and contains any TypeScript/React files
fd -e ts -e tsx . lib/
# If lib directory doesn't exist, this might be intentional as part of cleanup
if [ $? -ne 0 ]; then
echo "lib directory not found - this might be intentional"
fi
Length of output: 247
Script:
#!/bin/bash
# Search for any references to the lib directory in TypeScript/React files
rg -i "from ['\"]\.\./lib/" src/
rg -i "from ['\"]\.\/lib/" src/
rg -i "import.*from ['\"]\.\./lib/" src/
rg -i "import.*from ['\"]\.\/lib/" src/
Length of output: 457
Script:
#!/bin/bash
# Search for any lib directory references in the masterbots.ai app
cd apps/masterbots.ai 2>/dev/null
echo "Checking for lib imports..."
rg -i "from ['\"].*lib/" .
echo -e "\nChecking for any .ts/.tsx files outside src directory..."
fd -e ts -e tsx . . | grep -v "^src/"
Length of output: 29417
| "mb-env": "workspace:*", | ||
| "mb-genql": "workspace:*", | ||
| "mb-lib": "workspace:*", | ||
| "mb-types": "workspace:*", | ||
| "nanoid": "^3.3.6", | ||
| "nanoid": "latest", |
There was a problem hiding this comment.
Avoid using "latest" tag for dependencies.
Using the "latest" tag for nanoid and postcss can lead to unexpected breaking changes and makes builds non-reproducible. Please specify exact versions or version ranges instead.
Apply this diff to fix the versioning:
- "nanoid": "latest",
+ "nanoid": "^3.3.6",
- "postcss": "latest",
+ "postcss": "^8.4.31",Also applies to: 97-97
| startShareTransition(async () => { | ||
| }) |
There was a problem hiding this comment.
Fix incomplete implementation of share button click handler.
The startShareTransition function is empty and using @ts-ignore. This appears to be a merge conflict artifact that needs to be resolved.
Apply this diff to fix the implementation:
- // @ts-ignore
- startShareTransition(async () => {
- })
+ startShareTransition(() => {
+ copyShareLink(chat)
+ })Committable suggestion skipped: line range outside the PR's diff.
| if (!username) return ''; | ||
| return username.length > maxLength ? `${username.slice(0, maxLength - 4)}` : username; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix incomplete username truncation implementation.
The truncation function cuts off the username but doesn't add ellipsis, which might confuse users about whether the name is complete.
Apply this diff to fix the truncation:
- if (!username) return '';
- return username.length > maxLength ? `${username.slice(0, maxLength - 4)}` : username;
+ if (!username) return '';
+ return username.length > maxLength ? `${username.slice(0, maxLength - 3)}...` : username;📝 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.
| if (!username) return ''; | |
| return username.length > maxLength ? `${username.slice(0, maxLength - 4)}` : username; | |
| if (!username) return ''; | |
| return username.length > maxLength ? `${username.slice(0, maxLength - 3)}...` : username; |
| @@ -140,7 +135,7 @@ export default function BrowseList() { | |||
| {filteredThreads.map((thread: Thread, key) => ( | |||
| <BrowseListItem | |||
| thread={thread} | |||
| key={thread.threadId} | |||
| key={key} | |||
There was a problem hiding this comment.
Replace array index key with a unique identifier.
Using array indices as React keys can lead to rendering issues when items are reordered or modified. The thread object likely has a unique identifier that would be more suitable.
-key={key}
+key={thread.threadId}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 138-138: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
|
|
||
| const session = useSession() | ||
| const jwt = session?.data?.user?.hasuraJwt | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary dependency from useEffect.
The useEffect hook has jwt as a dependency, but it's not directly used within the effect. The getThreadForUser function already has access to the latest jwt through closure.
useEffect(() => {
getThreadForUser();
- }, [jwt])
+ }, [])📝 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(() => { | |
| useEffect(() => { | |
| getThreadForUser(); | |
| }, []) |
🧰 Tools
🪛 Biome (1.9.4)
[error] 52-52: This hook specifies more dependencies than necessary: jwt
This dependency can be removed from the list.
(lint/correctness/useExhaustiveDependencies)
| interface SignupState { | ||
| email: string | ||
| password: string | ||
| username: string | ||
| passwordVerify: string | ||
| isLoading: boolean | ||
| showVerificationNotice: boolean | ||
| showPassword: boolean | ||
| showPasswordVerify: boolean | ||
| email: string; | ||
| password: string; | ||
| username: string; | ||
| passwordVerify: string; | ||
| isLoading: boolean; | ||
| showVerificationNotice: boolean; | ||
| showPassword: boolean; | ||
| showPasswordVerify: boolean; | ||
| } | ||
|
|
||
| interface SignupState { | ||
| email: string | ||
| password: string | ||
| username: string | ||
| passwordVerify: string | ||
| isLoading: boolean | ||
| showVerificationNotice: boolean | ||
| email: string; | ||
| password: string; | ||
| username: string; | ||
| passwordVerify: string; | ||
| isLoading: boolean; | ||
| showVerificationNotice: boolean; |
There was a problem hiding this comment.
Remove duplicate SignupState interface.
The SignupState interface is defined twice. The second definition is missing some properties from the first one.
Apply this diff to fix the duplication:
-interface SignupState {
- email: string;
- password: string;
- username: string;
- passwordVerify: string;
- isLoading: boolean;
- showVerificationNotice: boolean;
-}📝 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.
| interface SignupState { | |
| email: string | |
| password: string | |
| username: string | |
| passwordVerify: string | |
| isLoading: boolean | |
| showVerificationNotice: boolean | |
| showPassword: boolean | |
| showPasswordVerify: boolean | |
| email: string; | |
| password: string; | |
| username: string; | |
| passwordVerify: string; | |
| isLoading: boolean; | |
| showVerificationNotice: boolean; | |
| showPassword: boolean; | |
| showPasswordVerify: boolean; | |
| } | |
| interface SignupState { | |
| email: string | |
| password: string | |
| username: string | |
| passwordVerify: string | |
| isLoading: boolean | |
| showVerificationNotice: boolean | |
| email: string; | |
| password: string; | |
| username: string; | |
| passwordVerify: string; | |
| isLoading: boolean; | |
| showVerificationNotice: boolean; | |
| interface SignupState { | |
| email: string; | |
| password: string; | |
| username: string; | |
| passwordVerify: string; | |
| isLoading: boolean; | |
| showVerificationNotice: boolean; | |
| showPassword: boolean; | |
| showPasswordVerify: boolean; | |
| } |
|
Closing PR. Reason: There is an updated PR version related to solve the same issue. Related |
Summary by Sourcery
Remove unnecessary properties from the query and fix trailing commas.
Bug Fixes:
Chores:
Summary by CodeRabbit
Release Notes
Notification System
react-hot-toastacross multiple componentsURL and Routing
urlBuildersutilityDependency Updates
react-hot-toastnext-themesCode Refactoring
Styling