add onboarding ai agent #86
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request includes various updates across multiple files in the Changes
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
|
@wtfdivyansh is attempting to deploy a commit to the BlueFinZ Team on Vercel. A member of the Team first needs to authorize it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Outside diff range and nitpick comments (8)
apps/app/app/(onboarding)/onboarding/page.tsx (1)
3-6: Consider adding error boundaries and loading states.The component could benefit from additional robustness:
- Add error boundaries to handle potential Chatbox failures
- Include loading states for better UX
- Add metadata for SEO
Consider updating the component like this:
+import { Suspense } from 'react'; +import { ErrorBoundary } from '@/components/error-boundary'; import Chatbox from "@/components/custom/onboarding/chatbox"; +import Loading from "@/components/loading"; + +export const metadata = { + title: 'Onboarding - Plura', + description: 'Complete your account setup with our AI assistant', +}; export default function Onboarding() { - return <Chatbox /> + return ( + <ErrorBoundary fallback={<div>Something went wrong</div>}> + <Suspense fallback={<Loading />}> + <Chatbox /> + </Suspense> + </ErrorBoundary> + ); }apps/app/app/(routes)/layout.tsx (1)
12-16: Consider adding error boundaries and loading statesThe layout should gracefully handle errors in child components and show appropriate loading states.
+import { ErrorBoundary } from '@/components/error-boundary'; +import { Suspense } from 'react'; <div className="p-2"> <ProgressBar /> <Infobar /> - {children} + <ErrorBoundary> + <Suspense fallback={<div>Loading...</div>}> + {children} + </Suspense> + </ErrorBoundary>apps/app/actions/action.tsx (2)
38-38: Remove console.log statementProduction code should not contain console.log statements.
- console.log("done", ...history.get());
46-46: Use a more reliable ID generation methodUsing
Date.now()for IDs could lead to collisions in high-frequency message scenarios.Consider using a UUID library:
- id: Date.now(), + id: crypto.randomUUID(),Note: This would require updating the ClientMessage type to use string IDs.
packages/auth/src/auth.ts (1)
23-29: Fix inconsistent spacing in field configuration.The spacing after colons is inconsistent in this block.
- isOnboarding: { - type: "boolean", - nullable:false, - required:true, - input:false, - defaultValue: false - } + isOnboarding: { + type: "boolean", + nullable: false, + required: true, + input: false, + defaultValue: false + }apps/app/components/custom/onboarding/message.tsx (1)
5-16: Consider refactoring to reduce code duplication.The message components share similar structure. Consider creating a base
MessageBubblecomponent to reduce duplication.type MessageBubbleProps = { children: React.ReactNode; icon: React.ReactNode; className?: string; hideIcon?: boolean; }; function MessageBubble({ children, icon, className, hideIcon = false }: MessageBubbleProps) { return ( <div className={cn("group relative flex items-start md:-ml-12", className)}> <div className={cn( "flex h-8 w-8 shrink-0 select-none items-center justify-center rounded-md border shadow-sm", !hideIcon && "invisible" )} > {icon} </div> <div className="ml-4 flex-1 space-y-2 overflow-hidden px-1">{children}</div> </div> ); } // Usage: export function UserMessage({ children }: { children: React.ReactNode }) { return <MessageBubble icon={<UserIcon />}>{children}</MessageBubble>; }Also applies to: 18-35, 37-57
apps/app/lib/ai.tsx (2)
7-13: Consider adding type validation for sendMessage responseThe
sendMessageaction's return type should be validated against theClientMessagetype to ensure type safety.export const AI = createAI< AIState, UIState, { - sendMessage: (message: string) => Promise<ClientMessage>; + sendMessage: (message: string) => Promise<ClientMessage & { + id: string; + content: string; + role: 'user' | 'assistant'; + }>; }
14-18: Consider enhancing the AI implementation for onboardingGiven this is an onboarding AI agent (as per PR title), consider adding:
- Documentation for the empty initial states
- Error handling for the sendMessage action
- Additional onboarding-specific actions (e.g., guiding users through specific steps)
Would you like me to help implement these enhancements? I can suggest:
- Additional onboarding-specific actions
- Error handling middleware
- State initialization with onboarding context
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
apps/api/package.json(1 hunks)apps/app/actions/action.tsx(1 hunks)apps/app/app/(onboarding)/onboarding/layout.tsx(1 hunks)apps/app/app/(onboarding)/onboarding/page.tsx(1 hunks)apps/app/app/(routes)/layout.tsx(1 hunks)apps/app/app/layout.tsx(1 hunks)apps/app/components/custom/onboarding/chatList.tsx(1 hunks)apps/app/components/custom/onboarding/chatbox.tsx(1 hunks)apps/app/components/custom/onboarding/message.tsx(1 hunks)apps/app/lib/ai.tsx(1 hunks)apps/app/middleware.ts(2 hunks)apps/app/package.json(2 hunks)packages/auth/src/auth.ts(1 hunks)packages/database/prisma/migrations/20241128154235_onboarding/migration.sql(1 hunks)packages/database/prisma/schema.prisma(1 hunks)
🔇 Additional comments (13)
packages/database/prisma/migrations/20241128154235_onboarding/migration.sql (1)
1-2: LGTM! Verify the migration's impact on existing users.
The migration looks correct with appropriate constraints and default value.
Let's verify if there are any existing users that might be affected:
✅ Verification successful
Migration is safe and properly integrated in the codebase
The migration is well integrated:
- The schema.prisma already includes the
isOnboardingfield with matching constraints - The field is properly used in middleware for onboarding flow control
- The auth configuration includes proper type definition for the field
- Default value of
falseensures existing users will be directed to onboarding
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any direct user table references in the codebase that might need updating
rg -l "user.*isOnboarding|isOnboarding.*user" --type ts --type tsx
# Check for any TypeScript interfaces or types that need to be updated
ast-grep --pattern 'interface User {
$$$
}'
Length of output: 144
Script:
#!/bin/bash
# Check for any direct user table references in the codebase that might need updating
rg -l "user.*isOnboarding|isOnboarding.*user" --type typescript
# Look for User interface/type definitions
rg -A 5 "interface User|type User" --type typescript
# Check Prisma schema for user model definition
fd "schema.prisma" --exec cat {}
Length of output: 1891
Script:
#!/bin/bash
# Try with different file extensions for TypeScript files
rg -l "user.*isOnboarding|isOnboarding.*user" -g "*.ts"
# Search for any files that might be using the User model or isOnboarding field
rg -A 3 "isOnboarding" -g "*.ts"
# Look for User type definitions with different extension pattern
rg -A 5 "interface User|type User" -g "*.ts"
Length of output: 801
apps/app/app/(onboarding)/onboarding/layout.tsx (1)
1-5: Verify AI component implementation and add error handling.
While the layout looks clean, we should verify the AI component implementation and consider adding error handling.
Let's verify the AI component implementation:
Consider enhancing the layout with error handling:
import { AI } from "@/lib/ai";
+import { ErrorBoundary } from '@/components/error-boundary';
export default function OnboardingLayout({children}: {children: React.ReactNode}) {
- return <AI>{children}</AI>;
+ return (
+ <ErrorBoundary fallback={<div>AI service unavailable</div>}>
+ <AI>{children}</AI>
+ </ErrorBoundary>
+ );
}apps/app/components/custom/onboarding/chatList.tsx (1)
1-1: Verify the UIState type import
Let's verify the type definition to ensure proper typing of the messages prop.
✅ Verification successful
UIState type import is correctly defined and used
The UIState type is properly defined in lib/ai.tsx as an alias for ClientMessage[], where ClientMessage is a well-defined type in actions/action.tsx containing the expected message properties (id, role, and display). The import and usage in chatList.tsx is correct and consistent with the type definition.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for UIState type definition
ast-grep --pattern 'type UIState = {
$$$
}'
Length of output: 47
Script:
#!/bin/bash
# Let's try a different approach to find UIState definition
# First, let's find all files that mention UIState
rg "UIState" -l
# Then let's get more context around UIState mentions
rg "UIState" -B 2 -A 5
Length of output: 2685
Script:
#!/bin/bash
# Let's find the ClientMessage type definition since UIState is defined as ClientMessage[]
rg "type ClientMessage" -B 2 -A 5
# Also search for any interface definition of ClientMessage
rg "interface ClientMessage" -B 2 -A 5
Length of output: 404
apps/app/app/layout.tsx (1)
18-33: LGTM! Verify impact on existing routes
The simplification of the root layout and moving layout-specific components to route layouts is a good architectural decision. However, let's verify the impact on existing routes.
✅ Verification successful
Root layout changes are properly handled by route layout
The route layout (apps/app/(routes)/layout.tsx) correctly implements all the necessary components that were removed from the root layout, including:
- Sidebar implementation with
AppSidebarandSidebarProvider - Progress bar and info bar components
- Proper layout structure for child components
Both existing routes ([slug] and settings) are wrapped by this layout and their page components are structured to work within this layout hierarchy.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for routes that might be affected by the layout change
fd --type f "page.tsx" "apps/app/app/(routes)"
Length of output: 124
Script:
#!/bin/bash
# Check the content of route pages to verify their layout dependencies
echo "=== Content of [slug]/page.tsx ==="
cat "apps/app/app/(routes)/[slug]/page.tsx"
echo -e "\n=== Content of settings/page.tsx ==="
cat "apps/app/app/(routes)/settings/page.tsx"
# Check if there are any layout files in the routes
echo -e "\n=== Layout files in routes ==="
fd --type f "layout.tsx" "apps/app/app/(routes)"
Length of output: 6205
Script:
#!/bin/bash
# Check the content of the route layout to verify it handles the sidebar and other layout components
cat "apps/app/app/(routes)/layout.tsx"
Length of output: 755
apps/app/actions/action.tsx (1)
30-30: 🛠️ Refactor suggestion
Review the system prompt restrictions
The system prompt restricts responses to 10 words, which might be too limiting for meaningful onboarding interactions.
Consider adjusting the system prompt to be more flexible and onboarding-specific:
- system: "You are a helpful assistant. give answer in only 10 words",
+ system: "You are an onboarding assistant helping new users understand our platform. Keep responses concise but informative.",✅ Verification successful
Consider increasing response length limit for better user experience
The 10-word response limit is indeed restrictive and unique in the codebase, as verified by searching all system prompts. This constraint could prevent the AI from providing adequate guidance during onboarding interactions.
apps/app/actions/action.tsx: Current 10-word limit may hinder:- Proper explanation of platform features
- Meaningful responses to user questions
- Natural conversation flow
The suggested modification to allow concise but informative responses would improve the user experience while maintaining response brevity.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any other system prompts in the codebase for consistency
rg -g '!*.{json,lock}' "system.*:.*\".*\""
Length of output: 143
apps/app/middleware.ts (1)
43-43: Review middleware matcher configuration
The current matcher configuration might be too broad. Consider adding specific paths that need authentication.
Consider making the matcher more specific to your needs:
- matcher: ['/((?!api|_next/static|_next/image|favicon.ico|sitemap.xml|robots.txt).*)']
+ matcher: [
+ '/onboarding/:path*',
+ '/home/:path*',
+ '/dashboard/:path*',
+ '/settings/:path*'
+ ]✅ Verification successful
Current middleware matcher configuration is appropriate
The current matcher configuration is actually well-suited for this application's needs because:
- The middleware implements a global authentication check that should run on all routes except the explicitly excluded ones
- The code shows proper handling of authentication states and redirects:
- Redirects unauthenticated users to the auth page
- Handles onboarding state appropriately
- Excludes necessary public assets and API routes
The excluded paths (api, _next/static, _next/image, favicon.ico, sitemap.xml, robots.txt) are the standard Next.js paths that should remain public, making the current configuration secure and maintainable.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for public routes that might need to be excluded
rg -l "export const dynamic = 'force-static'"
Length of output: 53
Script:
#!/bin/bash
# Let's check for public routes and authentication patterns in a different way
# First, let's look for route files
fd "page.tsx" "app"
# Then, let's look for authentication checks in middleware
rg -A 5 "auth" middleware.ts
# Also check for any route protection patterns
rg -l "useAuth|getSession|withAuth|isAuthenticated|requireAuth"
Length of output: 398
Script:
#!/bin/bash
# Let's correct the path and check the middleware content
cat apps/app/middleware.ts
# Let's also check the structure of protected routes
fd . apps/www/app/(auth)
fd . apps/app
# Check authentication implementation
rg -A 5 "useAuth|getSession|withAuth|isAuthenticated|requireAuth" apps/www/lib/server.ts apps/www/app/(auth)/dashboard/page.tsx
Length of output: 1646
packages/database/prisma/schema.prisma (1)
20-20: LGTM! Verify database migration impact.
The isOnboarding field addition follows best practices with appropriate type and default value.
Let's verify the migration impact:
✅ Verification successful
Migration and usage verified - implementation is sound
The isOnboarding field is properly implemented with:
- Clean migration that adds the column with appropriate default value
- Proper usage in middleware for onboarding flow control
- Correct type definition in auth package
- No foreign key constraints or complex dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any foreign key constraints or indexes referencing this field
rg -A 5 "isOnboarding" "packages/database/prisma/migrations"
# Check for any direct field usage in the codebase
rg "isOnboarding" --type typescript
Length of output: 286
Script:
#!/bin/bash
# Check for any direct field usage in the codebase with ripgrep
rg "isOnboarding"
# Check migration files to understand the full context
fd "migration.sql" packages/database/prisma/migrations --exec cat {}
Length of output: 4250
packages/auth/src/auth.ts (1)
21-31: LGTM! Security-conscious field configuration.
The isOnboarding field configuration is well-structured with appropriate security constraints:
- Prevents client-side modification (
input: false) - Ensures consistent state (
nullable: false,required: true) - Matches database schema definition
apps/app/package.json (1)
40-40: Verify compatibility with React RC version
The ai package needs to be verified for compatibility with React 19.0.0-rc that's being used in the project.
✅ Verification successful
ai package is compatible with React 19.0.0-rc
The ai package explicitly lists React 19.0.0-rc in its peer dependencies ("react": "^18 || ^19 || ^19.0.0-rc"), and there are no open React-related issues in the repository. The package is fully compatible with the React version being used in the project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check ai package's peer dependencies and React compatibility
echo "Checking ai package details:"
curl -s https://registry.npmjs.org/ai | jq '.["dist-tags"].latest, .versions[.["dist-tags"].latest].peerDependencies'
# Look for React-related issues
gh api graphql -f query='
{
repository(owner: "vercel", name: "ai") {
issues(last: 5, states: OPEN, labels: ["react"]) {
nodes {
title
createdAt
}
}
}
}'
Length of output: 499
apps/api/package.json (1)
7-8: Port change looks good, but verify integration points.
The port change from 5555 to 3001 is consistent across start and preview scripts. However, since this API will be serving the new AI agent functionality, we should ensure all integration points are updated.
Let's verify no hardcoded references to the old port exist:
✅ Verification successful
Port change is consistent across the codebase
The port change from 5555 to 3001 appears to be safe as:
- No references to the old port (5555) were found in the codebase
- Port 3001 is consistently used across multiple components:
- Authentication service (
packages/auth/src/auth.ts) - Web application (
apps/www/lib/server.ts) - Auth client (
apps/app/middleware.ts)
- Authentication service (
The integration points are properly configured to use the new port in both development and production environments.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to port 5555 in the codebase
rg -g '!package.json' '5555'
# Search for environment files that might need updating
fd -g '.env*'
Length of output: 252
Script:
#!/bin/bash
# Search for any remaining references to port 5555 in the codebase, including hidden files
rg -g '!package.json' --hidden '5555'
# Search for environment files and docker files that might contain port configurations
fd -H -e env -e env.example -e env.local -e env.development -e env.production -e dockerfile -e yml -e yaml
# Search for port 3001 references to understand the integration points
rg -g '!package.json' '3001'
Length of output: 838
apps/app/lib/ai.tsx (3)
4-5: LGTM! Clear and well-defined types
The type definitions are clear, properly exported, and follow TypeScript conventions.
1-2: Verify the imported types and function implementation
The imports look correct, but we should ensure that the imported types and function from @/actions/action are properly implemented and type-safe.
✅ Verification successful
Types and function implementation are properly defined and type-safe
The imported types and function are well-implemented in apps/app/actions/action.tsx:
ServerMessageandClientMessagetypes are properly defined with appropriate fields and type constraintssendMessagefunction is correctly typed, accepting a string message and returning a Promise with the expectedClientMessageshape- The implementation is being used correctly in the AI configuration and chatbox component
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of imported types and function
# Look for ClientMessage and ServerMessage type definitions
ast-grep --pattern 'type (ClientMessage|ServerMessage) = {
$$$
}'
# Look for sendMessage function implementation
ast-grep --pattern 'const sendMessage = async ($$$) => {
$$$
}'
Length of output: 137
Script:
#!/bin/bash
# Let's try a different approach to find the type definitions and function
# First, let's locate the action file
fd "action.ts" --type f
# Then, let's examine its contents
rg -A 5 "type (ClientMessage|ServerMessage)" --type ts
rg -A 5 "sendMessage" --type ts
Length of output: 2500
1-19: Verify integration with the onboarding flow
While the AI implementation looks good as a foundation, we should verify its integration with:
- The onboarding flow in the UI components
- User session management
- The database schema changes mentioned in the PR summary
✅ Verification successful
AI implementation is properly integrated with the onboarding flow
The verification shows that the AI implementation is well-integrated with:
- User session management through middleware that properly handles onboarding state
- Database schema with
isOnboardingfield in the User model - Complete onboarding UI components (Chatbox, Messages) using the AI context
- Proper message handling and display components for both user and bot interactions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify integration with onboarding components and user flow
# Look for onboarding-related components
fd -e tsx -e ts --exec grep -l "onboarding" {}
# Check for user session management integration
ast-grep --pattern 'useSession|getSession|withAuth'
# Look for database schema changes related to onboarding
fd "schema.prisma" --exec grep -A 5 -B 5 "onboarding" {}
Length of output: 262
Script:
#!/bin/bash
# Let's examine the onboarding-related files more closely
# Check the middleware implementation
cat apps/app/middleware.ts
# Examine the onboarding page
cat "apps/app/app/(onboarding)/onboarding/page.tsx"
# Look at the actions implementation
cat apps/app/actions/action.tsx
# Search for AI component usage
rg -l "AI" apps/app/app/
Length of output: 3055
Script:
#!/bin/bash
# Let's check the remaining components
# Check the chatbox component
cat "@/components/custom/onboarding/chatbox"
# Check the onboarding layout
cat "apps/app/app/(onboarding)/onboarding/layout.tsx"
# Look for BotMessage component
cat "@/components/custom/onboarding/message"
# Check for any database schema
fd "schema.prisma"
Length of output: 492
Script:
#!/bin/bash
# Let's check the components with correct paths and schema
# Check for chatbox component with correct path structure
fd -e tsx -e ts "chatbox" apps/app/components
# Look for message component
fd -e tsx -e ts "message" apps/app/components
# Examine the database schema
cat packages/database/prisma/schema.prisma
Length of output: 1902
Script:
#!/bin/bash
# Final check of the chatbox and message components
# Examine the chatbox implementation
cat apps/app/components/custom/onboarding/chatbox.tsx
# Check the message component
cat apps/app/components/custom/onboarding/message.tsx
Length of output: 2684
| {messages.map((message, index) => ( | ||
| <div key={index} className="pb-4"> | ||
| {message.display} | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
Avoid using array indices as React keys
Using array indices as keys can lead to rendering issues when messages are reordered, deleted, or inserted. Consider using unique message IDs instead.
- {messages.map((message, index) => (
- <div key={index} className="pb-4">
+ {messages.map((message) => (
+ <div key={message.id} className="pb-4">Committable suggestion skipped: line range outside the PR's diff.
| return ( | ||
| <div className="relative mx-auto max-w-2xl px-4"> | ||
| {messages.map((message, index) => ( | ||
| <div key={index} className="pb-4"> | ||
| {message.display} | ||
| </div> | ||
| ))} | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility and message differentiation
The chat messages lack semantic structure and visual differentiation between user and AI messages.
return (
- <div className="relative mx-auto max-w-2xl px-4">
+ <div
+ className="relative mx-auto max-w-2xl px-4"
+ role="log"
+ aria-label="Chat messages"
+ >
{messages.map((message) => (
- <div key={index} className="pb-4">
+ <article
+ key={message.id}
+ className={`pb-4 ${
+ message.role === 'assistant'
+ ? 'bg-muted/50 p-4 rounded-lg'
+ : ''
+ }`}
+ >
{message.display}
- </div>
+ </article>
))}
</div>
);Committable suggestion skipped: line range outside the PR's diff.
| const cookieStore = await cookies(); | ||
| const defaultOpen = cookieStore.get("plura-sidebar:state")?.value === "true"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for cookie access
The cookie access should handle potential errors and provide fallback behavior.
- const cookieStore = await cookies();
- const defaultOpen = cookieStore.get("plura-sidebar:state")?.value === "true";
+ const defaultOpen = await (async () => {
+ try {
+ const cookieStore = await cookies();
+ return cookieStore.get("plura-sidebar:state")?.value === "true";
+ } catch (error) {
+ console.error('Failed to access cookies:', error);
+ return false;
+ }
+ })();📝 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.
| const cookieStore = await cookies(); | |
| const defaultOpen = cookieStore.get("plura-sidebar:state")?.value === "true"; | |
| const defaultOpen = await (async () => { | |
| try { | |
| const cookieStore = await cookies(); | |
| return cookieStore.get("plura-sidebar:state")?.value === "true"; | |
| } catch (error) { | |
| console.error('Failed to access cookies:', error); | |
| return false; | |
| } | |
| })(); |
apps/app/actions/action.tsx
Outdated
| const history = getMutableAIState<typeof AI>(); | ||
| history.update([...history.get(), { role: "user", content: message }]); |
There was a problem hiding this comment.
Implement proper state management for message history
The current implementation might lead to memory issues as the history grows indefinitely.
Consider:
- Implementing a maximum history size
- Cleaning up old messages
- Using a more robust state management solution
+ const MAX_HISTORY_SIZE = 50;
const history = getMutableAIState<typeof AI>();
- history.update([...history.get(), { role: "user", content: message }]);
+ const currentHistory = history.get();
+ const newHistory = [...currentHistory.slice(-MAX_HISTORY_SIZE), { role: "user", content: message }];
+ history.update(newHistory);Also applies to: 34-39
apps/app/actions/action.tsx
Outdated
| export const sendMessage = async ( | ||
| message: string | ||
| ): Promise<{ | ||
| id: number; | ||
| role: "user" | "assistant"; | ||
| display: ReactNode; | ||
| }> => { | ||
| const history = getMutableAIState<typeof AI>(); | ||
| history.update([...history.get(), { role: "user", content: message }]); | ||
| const response = await streamUI({ | ||
| model: togetherai("deepseek-ai/deepseek-llm-67b-chat"), | ||
| system: "You are a helpful assistant. give answer in only 10 words", | ||
| messages: [{ role: "user", content: message }, ...history.get()], | ||
| text: ({ content, done }) => { | ||
| if (done) { | ||
| history.update([ | ||
| ...history.get(), | ||
| { role: "assistant", content: content }, | ||
| ]); | ||
| console.log("done", ...history.get()); | ||
| } | ||
| return <BotMessage>{content}</BotMessage>; | ||
| }, | ||
| }); | ||
|
|
||
|
|
||
| return { | ||
| id: Date.now(), | ||
| role: "assistant" as const, | ||
| display: response.value, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Add error handling and input validation
The function lacks error handling for API failures and input validation.
Consider implementing:
- Input validation for empty messages
- Error handling for API calls
- Timeout handling for streaming
export const sendMessage = async (
message: string
): Promise<ClientMessage> => {
+ if (!message?.trim()) {
+ throw new Error('Message cannot be empty');
+ }
+
const history = getMutableAIState<typeof AI>();
try {
// ... existing code ...
+ } catch (error) {
+ console.error('Error in AI response:', error);
+ throw new Error('Failed to get AI response');
+ }
};Committable suggestion skipped: line range outside the PR's diff.
| @@ -9,6 +9,8 @@ const redirectUrl = | |||
| process.env.NODE_ENV === "production" | |||
| ? "https://www.plura.pro/auth" | |||
| : "http://localhost:3003/auth"; | |||
| const appDomain = process.env.NODE_ENV === "production" ? "https://app.plura.pro" : "http://localhost:3002"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use environment variables for domain configuration
Hardcoding domains in the code makes it less maintainable and harder to configure.
-const appDomain = process.env.NODE_ENV === "production" ? "https://app.plura.pro" : "http://localhost:3002";
+const appDomain = process.env.NEXT_PUBLIC_APP_DOMAIN || (
+ process.env.NODE_ENV === "production" ? "https://app.plura.pro" : "http://localhost:3002"
+);📝 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.
| const appDomain = process.env.NODE_ENV === "production" ? "https://app.plura.pro" : "http://localhost:3002"; | |
| const appDomain = process.env.NEXT_PUBLIC_APP_DOMAIN || ( | |
| process.env.NODE_ENV === "production" ? "https://app.plura.pro" : "http://localhost:3002" | |
| ); |
| if (!currentPath.startsWith("/onboarding") && !session.user.isOnboarding) { | ||
| return NextResponse.redirect(`${appDomain}/onboarding`); | ||
| } | ||
| if (currentPath.startsWith("/onboarding") && session.user.isOnboarding) { | ||
| return NextResponse.redirect(`${appDomain}/home`); | ||
| } |
There was a problem hiding this comment.
Prevent potential redirect loops
The current implementation might cause redirect loops if the session state is inconsistent.
Add additional checks:
+ const MAX_REDIRECTS = 3;
+ const redirectCount = parseInt(request.headers.get('x-redirect-count') || '0');
+
+ if (redirectCount >= MAX_REDIRECTS) {
+ return new NextResponse('Too many redirects', { status: 500 });
+ }
+
if (!currentPath.startsWith("/onboarding") && !session.user.isOnboarding) {
- return NextResponse.redirect(`${appDomain}/onboarding`);
+ const response = NextResponse.redirect(`${appDomain}/onboarding`);
+ response.headers.set('x-redirect-count', (redirectCount + 1).toString());
+ return response;
}
if (currentPath.startsWith("/onboarding") && session.user.isOnboarding) {
- return NextResponse.redirect(`${appDomain}/home`);
+ const response = NextResponse.redirect(`${appDomain}/home`);
+ response.headers.set('x-redirect-count', (redirectCount + 1).toString());
+ return response;
}📝 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 (!currentPath.startsWith("/onboarding") && !session.user.isOnboarding) { | |
| return NextResponse.redirect(`${appDomain}/onboarding`); | |
| } | |
| if (currentPath.startsWith("/onboarding") && session.user.isOnboarding) { | |
| return NextResponse.redirect(`${appDomain}/home`); | |
| } | |
| const MAX_REDIRECTS = 3; | |
| const redirectCount = parseInt(request.headers.get('x-redirect-count') || '0'); | |
| if (redirectCount >= MAX_REDIRECTS) { | |
| return new NextResponse('Too many redirects', { status: 500 }); | |
| } | |
| if (!currentPath.startsWith("/onboarding") && !session.user.isOnboarding) { | |
| const response = NextResponse.redirect(`${appDomain}/onboarding`); | |
| response.headers.set('x-redirect-count', (redirectCount + 1).toString()); | |
| return response; | |
| } | |
| if (currentPath.startsWith("/onboarding") && session.user.isOnboarding) { | |
| const response = NextResponse.redirect(`${appDomain}/home`); | |
| response.headers.set('x-redirect-count', (redirectCount + 1).toString()); | |
| return response; | |
| } |
| export function UserMessage({ children }: { children: React.ReactNode }) { | ||
| return ( | ||
| <div className="group relative flex items-start md:-ml-12"> | ||
| <div className="flex h-8 w-8 shrink-0 select-none items-center justify-center rounded-md border shadow-sm bg-background"> | ||
| <UserIcon /> | ||
| </div> | ||
| <div className="ml-4 flex-1 space-y-2 overflow-hidden px-1"> | ||
| {children} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| export function BotMessage({ | ||
| children, | ||
| className, | ||
| }: { | ||
| children: React.ReactNode; | ||
| className?: string; | ||
| }) { | ||
| return ( | ||
| <div className={cn("group relative flex items-start md:-ml-12", className)}> | ||
| <div className="flex h-8 w-8 shrink-0 select-none items-center justify-center rounded-md border shadow-sm bg-primary text-primary-foreground"> | ||
| <Sparkle /> | ||
| </div> | ||
| <div className="ml-4 flex-1 space-y-2 overflow-hidden px-1"> | ||
| {children} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| export function BotCard({ | ||
| children, | ||
| showAvatar = true, | ||
| }: { | ||
| children: React.ReactNode; | ||
| showAvatar?: boolean; | ||
| }) { | ||
| return ( | ||
| <div className="group relative flex items-start md:-ml-12"> | ||
| <div | ||
| className={cn( | ||
| "flex h-8 w-8 shrink-0 select-none items-center justify-center rounded-md border shadow-sm bg-primary text-primary-foreground", | ||
| !showAvatar && "invisible" | ||
| )} | ||
| > | ||
| <Sparkle /> | ||
| </div> | ||
| <div className="ml-4 flex-1 px-1">{children}</div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes to message components.
Enhance accessibility by adding appropriate ARIA roles and labels.
export function UserMessage({ children }: { children: React.ReactNode }) {
return (
- <div className="group relative flex items-start md:-ml-12">
+ <div
+ role="log"
+ aria-label="User message"
+ className="group relative flex items-start md:-ml-12">
{/* ... */}
</div>
);
}
export function BotMessage({
children,
className,
}: {
children: React.ReactNode;
className?: string;
}) {
return (
- <div className={cn("group relative flex items-start md:-ml-12", className)}>
+ <div
+ role="log"
+ aria-label="Assistant message"
+ className={cn("group relative flex items-start md:-ml-12", className)}>
{/* ... */}
</div>
);
}
export function BotCard({
children,
showAvatar = true,
}: {
children: React.ReactNode;
showAvatar?: boolean;
}) {
return (
- <div className="group relative flex items-start md:-ml-12">
+ <div
+ role="log"
+ aria-label="Assistant card"
+ className="group relative flex items-start md:-ml-12">
{/* ... */}
</div>
);
}📝 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.
| export function UserMessage({ children }: { children: React.ReactNode }) { | |
| return ( | |
| <div className="group relative flex items-start md:-ml-12"> | |
| <div className="flex h-8 w-8 shrink-0 select-none items-center justify-center rounded-md border shadow-sm bg-background"> | |
| <UserIcon /> | |
| </div> | |
| <div className="ml-4 flex-1 space-y-2 overflow-hidden px-1"> | |
| {children} | |
| </div> | |
| </div> | |
| ); | |
| } | |
| export function BotMessage({ | |
| children, | |
| className, | |
| }: { | |
| children: React.ReactNode; | |
| className?: string; | |
| }) { | |
| return ( | |
| <div className={cn("group relative flex items-start md:-ml-12", className)}> | |
| <div className="flex h-8 w-8 shrink-0 select-none items-center justify-center rounded-md border shadow-sm bg-primary text-primary-foreground"> | |
| <Sparkle /> | |
| </div> | |
| <div className="ml-4 flex-1 space-y-2 overflow-hidden px-1"> | |
| {children} | |
| </div> | |
| </div> | |
| ); | |
| } | |
| export function BotCard({ | |
| children, | |
| showAvatar = true, | |
| }: { | |
| children: React.ReactNode; | |
| showAvatar?: boolean; | |
| }) { | |
| return ( | |
| <div className="group relative flex items-start md:-ml-12"> | |
| <div | |
| className={cn( | |
| "flex h-8 w-8 shrink-0 select-none items-center justify-center rounded-md border shadow-sm bg-primary text-primary-foreground", | |
| !showAvatar && "invisible" | |
| )} | |
| > | |
| <Sparkle /> | |
| </div> | |
| <div className="ml-4 flex-1 px-1">{children}</div> | |
| </div> | |
| ); | |
| } | |
| export function UserMessage({ children }: { children: React.ReactNode }) { | |
| return ( | |
| <div | |
| role="log" | |
| aria-label="User message" | |
| className="group relative flex items-start md:-ml-12"> | |
| <div className="flex h-8 w-8 shrink-0 select-none items-center justify-center rounded-md border shadow-sm bg-background"> | |
| <UserIcon /> | |
| </div> | |
| <div className="ml-4 flex-1 space-y-2 overflow-hidden px-1"> | |
| {children} | |
| </div> | |
| </div> | |
| ); | |
| } | |
| export function BotMessage({ | |
| children, | |
| className, | |
| }: { | |
| children: React.ReactNode; | |
| className?: string; | |
| }) { | |
| return ( | |
| <div | |
| role="log" | |
| aria-label="Assistant message" | |
| className={cn("group relative flex items-start md:-ml-12", className)}> | |
| <div className="flex h-8 w-8 shrink-0 select-none items-center justify-center rounded-md border shadow-sm bg-primary text-primary-foreground"> | |
| <Sparkle /> | |
| </div> | |
| <div className="ml-4 flex-1 space-y-2 overflow-hidden px-1"> | |
| {children} | |
| </div> | |
| </div> | |
| ); | |
| } | |
| export function BotCard({ | |
| children, | |
| showAvatar = true, | |
| }: { | |
| children: React.ReactNode; | |
| showAvatar?: boolean; | |
| }) { | |
| return ( | |
| <div | |
| role="log" | |
| aria-label="Assistant card" | |
| className="group relative flex items-start md:-ml-12"> | |
| <div | |
| className={cn( | |
| "flex h-8 w-8 shrink-0 select-none items-center justify-center rounded-md border shadow-sm bg-primary text-primary-foreground", | |
| !showAvatar && "invisible" | |
| )} | |
| > | |
| <Sparkle /> | |
| </div> | |
| <div className="ml-4 flex-1 px-1">{children}</div> | |
| </div> | |
| ); | |
| } |
| "@ai-sdk/google": "^1.0.4", | ||
| "@ai-sdk/togetherai": "^0.0.3", |
There was a problem hiding this comment.
💡 Codebase verification
Add error handling for AI SDK integration
The implementation in action.tsx lacks error handling for the AI SDK operations. While the packages are at stable versions (@ai-sdk/google at 1.0.4) and early but functional version (@ai-sdk/togetherai at 0.0.3), the following issues need attention:
- No try-catch blocks around the
streamUIandtogetheraioperations - Missing error handling for potential API failures or rate limits
- No fallback mechanism for failed AI responses
🔗 Analysis chain
Verify stability of AI SDK dependencies
The @ai-sdk/togetherai package is at a very early version (0.0.3), which might not be production-ready. Consider the following:
- Verify the package's stability and production readiness
- Check for known issues or limitations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest versions and weekly downloads for stability assessment
echo "Checking @ai-sdk/google package:"
curl -s https://registry.npmjs.org/@ai-sdk/google | jq '.["dist-tags"].latest, .versions[.["dist-tags"].latest].time'
echo -e "\nChecking @ai-sdk/togetherai package:"
curl -s https://registry.npmjs.org/@ai-sdk/togetherai | jq '.["dist-tags"].latest, .versions[.["dist-tags"].latest].time'
# Check for recent GitHub issues
gh api graphql -f query='
{
repository(owner: "togetherai", name: "sdk") {
issues(last: 5, states: OPEN) {
nodes {
title
createdAt
labels(first: 5) {
nodes {
name
}
}
}
}
}
}'
Length of output: 965
Script:
#!/bin/bash
# Let's check if these AI SDK packages are being used in the codebase
# and how they are implemented to assess the risk
# Search for imports/usage of @ai-sdk/togetherai
echo "Usage of @ai-sdk/togetherai:"
rg "@ai-sdk/togetherai" -A 3
# Search for imports/usage of @ai-sdk/google
echo -e "\nUsage of @ai-sdk/google:"
rg "@ai-sdk/google" -A 3
# Check if there are any error handling or try-catch blocks around these SDKs
echo -e "\nError handling around AI SDKs:"
ast-grep --pattern 'try {
$$$
@ai-sdk
$$$
} catch'
Length of output: 2360
Script:
#!/bin/bash
# Let's check the implementation in action.tsx to see how these SDKs are used
# and if there's proper error handling
echo "Checking AI SDK implementation in action.tsx:"
cat apps/app/actions/action.tsx
# Let's also check if there are any try-catch blocks in this file
# using a more general pattern
echo -e "\nChecking error handling in action.tsx:"
ast-grep --pattern 'try {
$$$
} catch' apps/app/actions/action.tsx
Length of output: 1625
| "start": "next start -p 3001", | ||
| "preview": "next build && next start -p 3001", |
There was a problem hiding this comment.
💡 Codebase verification
Documentation update needed for port 3001
The port change to 3001 in the API service requires documentation updates. Our verification found that:
- No port references exist in any README files
- No environment templates are present
- No port configurations in Docker files or config files
- No explicit port documentation in any of the existing documentation
Please update the following:
- Add port information to
apps/api/README.md - Create an
.env.examplefile in the API service directory - Update the main
README.mdwith the development ports for all services - Document port configurations in Docker setup if containerization is used
🔗 Analysis chain
Documentation update needed for port change.
Since this is a significant infrastructure change that affects local development and deployment, documentation should be updated to reflect the new port.
Please ensure the following are updated:
- README files
- Development setup guides
- Deployment documentation
- Environment variable templates
Let's check for documentation files that might need updating:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for documentation files
fd -e md -e mdx
# Look for environment templates
fd -g '.env.example' -g '.env.template'
Length of output: 294
Script:
#!/bin/bash
# Check README files for port mentions
echo "=== Checking main README.md ==="
rg -i "port|3001|3000" README.md || echo "No port mentions found"
echo -e "\n=== Checking API README ==="
rg -i "port|3001|3000" apps/api/README.md || echo "No port mentions found"
echo -e "\n=== Checking other documentation for port references ==="
rg -i "port|3001|3000" CONTRIBUTING.md || echo "No port mentions found"
echo -e "\n=== Checking for environment files ==="
fd ".env" --hidden --no-ignore
Length of output: 7840
Script:
#!/bin/bash
# Let's check for any docker or docker-compose files that might reference the port
echo "=== Checking Docker files for port references ==="
fd -e dockerfile -e yml -e yaml | xargs rg -i "port.*3001|port.*3000" || echo "No port mentions found in Docker files"
# Check for any configuration files that might contain port references
echo -e "\n=== Checking common config files ==="
fd -e json -e js -e ts -g "config*" | xargs rg -i "port.*3001|port.*3000" || echo "No port mentions found in config files"
# Check specifically for Next.js config files
echo -e "\n=== Checking Next.js config files ==="
fd "next.config" | xargs rg -i "port" || echo "No port mentions found in Next.js config"
Length of output: 1115
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (17)
apps/app/app/layout.tsx (1)
6-6: Remove unused importThe
AIimport is not being used in this file. If it's not needed for type definitions, consider removing it.-import { AI } from "@/lib/ai";🧰 Tools
🪛 eslint
[error] 6-6: 'AI' is defined but never used.
(@typescript-eslint/no-unused-vars)
apps/app/middleware.ts (2)
24-24: Remove debug console.log statementDebug statements should not be committed to production code.
- // console.log("seesion mid", session);
Line range hint
15-23: Add error handling for session fetchThe session fetch could fail silently if the API is unavailable.
Add proper error handling:
- const { data: session } = await betterFetch<Session>( - `${baseDomain}/api/auth/get-session`, - { - baseURL: request.nextUrl.origin, - headers: { - cookie: request.headers.get("cookie") || "", - }, - }, - ); + try { + const { data: session } = await betterFetch<Session>( + `${baseDomain}/api/auth/get-session`, + { + baseURL: request.nextUrl.origin, + headers: { + cookie: request.headers.get("cookie") || "", + }, + }, + ); + return session; + } catch (error) { + console.error('Failed to fetch session:', error); + return NextResponse.redirect(redirectUrl); + }apps/api/app/api/[[...route]]/user.ts (3)
Line range hint
6-26: Add input validation and sanitize user dataThe
/selfendpoint returns the complete user object without sanitization.Add data sanitization:
const user = await prisma.user.findUnique({ where: { id: currentUser.user.id, }, + select: { + id: true, + email: true, + name: true, + isOnboarding: true, + // Only include necessary fields - // Exclude sensitive fields + }, });
Line range hint
33-52: Improve pagination security and validationThe
/allendpoint needs better input validation and rate limiting.Add validation and limits:
+ const maxTake = 50; // Maximum items per page + const requestedTake = parseInt(take!) || 10; + const sanitizedTake = Math.min(requestedTake, maxTake); + const users = await prisma.user.findMany({ - take: parseInt(take!) || 10, + take: sanitizedTake, skip: 1, cursor: cursor ? { id: cursor, } : undefined, + select: { + id: true, + name: true, + // Only include necessary fields + }, });
Line range hint
53-73: Add rate limiting and improve error handlingThe
/:idendpoint needs rate limiting and better error handling.Consider implementing:
- Rate limiting middleware
- Better error messages
- Data sanitization
.get("/:id", async (c) => { + // Add rate limiting middleware const userId = c.req.param("id"); if (!userId) { return c.json({ - message: "user id is required", + message: "Invalid request: User ID is required", status: 400, }); } - const user = await prisma.user.findUnique({ - where: { - id: userId, - }, - }); + try { + const user = await prisma.user.findUnique({ + where: { id: userId }, + select: { + id: true, + name: true, + // Only include necessary fields + }, + }); + if (!user) { + return c.json({ message: "User not found", status: 404 }, 404); + } + return c.json({ user }, 200); + } catch (error) { + return c.json({ message: "Server error", status: 500 }, 500); + } - return c.json( - { - user, - }, - 200, - ); });apps/app/lib/utils.ts (1)
8-10: Add input validation for sleep durationWhile the implementation is correct, consider adding validation for the
msparameter to prevent negative values or extremely large timeouts.export const sleep = (ms: number) => { + if (ms < 0) throw new Error('Sleep duration must be non-negative'); + if (ms > 300000) throw new Error('Sleep duration exceeds maximum allowed (300s)'); return new Promise((resolve) => setTimeout(resolve, ms)); };apps/app/lib/ai.tsx (1)
1-2: Use consistent import path formatThe import paths use different formats (
ai/rscvs@/actions/action). Consider standardizing to use the@/alias for internal imports.-import { createAI } from "ai/rsc"; +import { createAI } from "@/node_modules/ai/rsc";apps/app/hooks/use-scroll-to-bottom.ts (2)
14-17: Consider adding error handling and scroll behavior options.The scroll behavior is hardcoded to "smooth", which might not be suitable for all use cases. Consider making it configurable.
- end.scrollIntoView({ behavior: "smooth" }); + end.scrollIntoView({ behavior: scrollBehavior }); export function useScrollToBottom<T extends HTMLElement>(): [ RefObject<T>, RefObject<T>, + { scrollBehavior?: ScrollBehavior } ] {
19-22: Consider performance optimization for MutationObserver.Observing both
childListandsubtreemight be excessive for most use cases. Consider making these options configurable or limiting the scope based on actual needs.apps/app/components/custom/onboarding/message.tsx (2)
24-28: Consider performance optimization for animations.The motion animation might cause performance issues on slower devices. Consider:
- Using
layoutIdfor better animation orchestration- Adding
whileFocusandwhileHoverstates- Using
transforminstead ofyfor better performance<motion.div className={`flex flex-row w-full md:px-0 `} - initial={{ y: 5, opacity: 0 }} - animate={{ y: 0, opacity: 1 }} + initial={{ transform: 'translateY(5px)', opacity: 0 }} + animate={{ transform: 'translateY(0)', opacity: 1 }} + transition={{ type: "spring", stiffness: 500, damping: 30 }}
18-22: Add missing className prop type.The BotMessage component should accept an optional className prop for consistency with BotCard.
export function BotMessage({ children, + className, }: { children: React.ReactNode; + className?: string; }) {apps/app/components/ui/textarea.tsx (1)
14-35: Consider memoizing the height calculation logicThe height calculation logic is well-implemented but could be optimized to prevent unnecessary recalculations.
+const calculateHeight = React.useCallback((textarea: HTMLTextAreaElement) => { + const style = window.getComputedStyle(textarea); + const borderHeight = parseInt(style.borderTopWidth) + parseInt(style.borderBottomWidth); + const paddingHeight = parseInt(style.paddingTop) + parseInt(style.paddingBottom); + const lineHeight = parseInt(style.lineHeight); + const maxHeight = maxRows ? lineHeight * maxRows + borderHeight + paddingHeight : Infinity; + return Math.min(textarea.scrollHeight + borderHeight, maxHeight); +}, [maxRows]); const handleInput = (e: ChangeEvent<HTMLTextAreaElement>) => { const textarea = e.target; textarea.style.height = "auto"; - const style = window.getComputedStyle(textarea); - const borderHeight = parseInt(style.borderTopWidth) + parseInt(style.borderBottomWidth); - // ... removed calculations - textarea.style.height = `${newHeight}px`; + textarea.style.height = `${calculateHeight(textarea)}px`; };apps/app/components/custom/onboarding/proceed.tsx (1)
29-37: Optimize button click handler and remove unused parameterThe event parameter is unused in the onClick handler.
-onClick={(e) => handleClick("yes")} +onClick={() => handleClick("yes")}🧰 Tools
🪛 eslint
[error] 30-30: 'e' is defined but never used.
(@typescript-eslint/no-unused-vars)
apps/app/components/custom/onboarding/workspace-form.tsx (2)
3-14: Clean up unused importsRemove the following unused imports to maintain code cleanliness:
CardFooter,CardHeaderfrom@/components/ui/cardCar,lucide-react- import { - Card, - CardContent, - CardDescription, - CardFooter, - CardHeader, - CardTitle, - } from "@/components/ui/card"; + import { + Card, + CardContent, + CardDescription, + CardTitle, + } from "@/components/ui/card"; - import { Car, LoaderCircle, Mail } from "lucide-react"; + import { LoaderCircle } from "lucide-react";🧰 Tools
🪛 eslint
[error] 7-7: 'CardFooter' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 8-8: 'CardHeader' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 14-14: 'Car' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 14-14: 'Mail' is defined but never used.
(@typescript-eslint/no-unused-vars)
51-85: Enhance accessibility featuresWhile the component is well-structured, consider these accessibility improvements:
- Add aria-label to the form
- Add error messages for screen readers
- Improve loading state announcement
- <Card className="bg-woodsmoke-950 rounded-lg shadow-md sm:w-[350px] shrink"> + <Card className="bg-woodsmoke-950 rounded-lg shadow-md sm:w-[350px] shrink" role="dialog" aria-labelledby="workspace-title"> <CardContent> <div className=" flex flex-col py-4 "> - <CardTitle className="sm:text-start text-bold text-2xl text-center text-neutral-200"> + <CardTitle id="workspace-title" className="sm:text-start text-bold text-2xl text-center text-neutral-200"> Create Workspace </CardTitle>apps/app/components/custom/onboarding/chatbox.tsx (1)
19-19: Clean up development artifacts
- Remove unused imports:
useanduseReffrom react- Remove unused variable:
count- Remove console.log statements
- import { use, useEffect, useRef } from "react"; + import { useEffect } from "react"; - console.log(data) - let count = 0 - console.log("greeting", response)Also applies to: 40-40, 61-61, 65-65
🧰 Tools
🪛 eslint
[error] 19-19: 'use' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 19-19: 'useRef' is defined but never used.
(@typescript-eslint/no-unused-vars)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (7)
apps/app/public/file.svgis excluded by!**/*.svgapps/app/public/globe.svgis excluded by!**/*.svgapps/app/public/images/plura-logo.pngis excluded by!**/*.pngapps/app/public/next.svgis excluded by!**/*.svgapps/app/public/vercel.svgis excluded by!**/*.svgapps/app/public/window.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
apps/api/app/api/[[...route]]/health.ts(1 hunks)apps/api/app/api/[[...route]]/route.ts(1 hunks)apps/api/app/api/[[...route]]/session.ts(1 hunks)apps/api/app/api/[[...route]]/user.ts(2 hunks)apps/api/package.json(1 hunks)apps/app/actions/action.tsx(1 hunks)apps/app/actions/session.ts(1 hunks)apps/app/app/layout.tsx(2 hunks)apps/app/components/custom/onboarding/BeatLoader.tsx(1 hunks)apps/app/components/custom/onboarding/ChatInput.tsx(1 hunks)apps/app/components/custom/onboarding/chatList.tsx(1 hunks)apps/app/components/custom/onboarding/chatbox.tsx(1 hunks)apps/app/components/custom/onboarding/message.tsx(1 hunks)apps/app/components/custom/onboarding/proceed.tsx(1 hunks)apps/app/components/custom/onboarding/workspace-form.tsx(1 hunks)apps/app/components/ui/textarea.tsx(1 hunks)apps/app/hooks/use-scroll-to-bottom.ts(1 hunks)apps/app/lib/ai.tsx(1 hunks)apps/app/lib/utils.ts(1 hunks)apps/app/middleware.ts(2 hunks)apps/app/package.json(3 hunks)apps/app/tailwind.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- apps/api/app/api/[[...route]]/session.ts
- apps/api/app/api/[[...route]]/health.ts
- apps/api/app/api/[[...route]]/route.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/app/components/custom/onboarding/chatList.tsx
- apps/api/package.json
- apps/app/package.json
🧰 Additional context used
🪛 eslint
apps/app/components/custom/onboarding/ChatInput.tsx
[error] 1-1: 'Button' is defined but never used.
(@typescript-eslint/no-unused-vars)
apps/app/components/custom/onboarding/workspace-form.tsx
[error] 7-7: 'CardFooter' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 8-8: 'CardHeader' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 14-14: 'Car' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 14-14: 'Mail' is defined but never used.
(@typescript-eslint/no-unused-vars)
apps/app/app/layout.tsx
[error] 6-6: 'AI' is defined but never used.
(@typescript-eslint/no-unused-vars)
apps/app/components/custom/onboarding/proceed.tsx
[error] 10-10: 'messages' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 30-30: 'e' is defined but never used.
(@typescript-eslint/no-unused-vars)
apps/app/components/ui/textarea.tsx
[error] 9-9: 'ref' is defined but never used.
(@typescript-eslint/no-unused-vars)
apps/app/components/custom/onboarding/chatbox.tsx
[error] 19-19: 'use' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 19-19: 'useRef' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 61-61: 'count' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 61-61: 'count' is never reassigned. Use 'const' instead.
(prefer-const)
apps/app/actions/action.tsx
[error] 3-3: 'createStreamableValue' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 10-10: 'Button' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 Biome (1.9.4)
apps/app/actions/action.tsx
[error] 61-61: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 79-79: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🔇 Additional comments (6)
apps/app/middleware.ts (2)
12-12: Use environment variables for domain configuration
Previous review comment about using environment variables for domain configuration is still valid.
33-38: Prevent potential redirect loops
Previous review comment about preventing redirect loops is still valid.
apps/app/components/custom/onboarding/message.tsx (1)
5-16: Add accessibility attributes to UserMessage component.
Enhance accessibility by adding appropriate ARIA roles and labels.
apps/app/tailwind.config.ts (1)
13-25: Verify color contrast ratios for accessibility
The woodsmoke color palette looks well-structured, but please verify that the contrast ratios between these colors meet WCAG accessibility guidelines, especially for text and interactive elements.
apps/app/components/custom/onboarding/workspace-form.tsx (1)
18-24:
Implement proper workspace creation logic
The current implementation is a placeholder that:
- Uses
Date.now()for ID generation which could lead to collisions - Lacks proper error handling and validation
- Doesn't persist the workspace data
Consider implementing proper workspace creation with:
- UUID for ID generation
- Database persistence
- Input validation
- Error handling
apps/app/actions/action.tsx (1)
32-35: Implement proper state management for message history
The current implementation might lead to memory issues as the history grows indefinitely.
| <Toaster /> | ||
| {children} |
There was a problem hiding this comment.
💡 Codebase verification
Warning: Inconsistent Toaster implementations across layouts
The Toaster components in the two layout files are using different implementations:
apps/app/app/layout.tsxusesimport { Toaster } from "sonner"apps/www/app/layout.tsxusesimport { Toaster } from "@/components/ui/toaster"
This inconsistency could lead to different toast notification behaviors across the applications.
🔗 Analysis chain
LGTM! Clean layout structure
The simplified layout with the Toaster component is well-structured. The removal of the sidebar-related complexity makes the code more maintainable.
Let's verify if the Toaster component is consistently used across other layouts:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent Toaster usage across layouts
# Expect: Other layout files should also use Toaster for consistency
# Search for other layout files and check Toaster usage
fd "layout.tsx" apps/ --exec rg -l "Toaster"
Length of output: 90
Script:
#!/bin/bash
# Let's check the actual implementation of Toaster in both layout files
rg -A 5 -B 5 "Toaster" apps/app/app/layout.tsx apps/www/app/layout.tsx
Length of output: 2372
| export const getSession = async () => { | ||
|
|
||
| const response = await betterFetch<Session>("http://localhost:3001/api/auth/get-session", | ||
| { | ||
| baseURL: "http://localhost:3002", | ||
| headers: { | ||
| cookie: (await headers()).get("cookie") || "", | ||
| }, | ||
| }); | ||
| return response.data | ||
|
|
||
| } |
There was a problem hiding this comment.
Add error handling and improve type safety
The function lacks error handling for failed requests and type validation.
Consider implementing error handling:
export const getSession = async () => {
+ try {
const response = await betterFetch<Session>(
// ... configuration ...
);
+ if (!response.data) {
+ throw new Error('Invalid session response');
+ }
return response.data;
+ } catch (error) {
+ console.error('Failed to fetch session:', error);
+ return null;
+ }
}📝 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.
| export const getSession = async () => { | |
| const response = await betterFetch<Session>("http://localhost:3001/api/auth/get-session", | |
| { | |
| baseURL: "http://localhost:3002", | |
| headers: { | |
| cookie: (await headers()).get("cookie") || "", | |
| }, | |
| }); | |
| return response.data | |
| } | |
| export const getSession = async () => { | |
| try { | |
| const response = await betterFetch<Session>("http://localhost:3001/api/auth/get-session", | |
| { | |
| baseURL: "http://localhost:3002", | |
| headers: { | |
| cookie: (await headers()).get("cookie") || "", | |
| }, | |
| }); | |
| if (!response.data) { | |
| throw new Error('Invalid session response'); | |
| } | |
| return response.data; | |
| } catch (error) { | |
| console.error('Failed to fetch session:', error); | |
| return null; | |
| } | |
| } |
| const response = await betterFetch<Session>("http://localhost:3001/api/auth/get-session", | ||
| { | ||
| baseURL: "http://localhost:3002", | ||
| headers: { | ||
| cookie: (await headers()).get("cookie") || "", | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Use environment variables for API endpoints
Hardcoded URLs make the code less maintainable and harder to configure across different environments.
Replace hardcoded URLs with environment variables:
- const response = await betterFetch<Session>("http://localhost:3001/api/auth/get-session",
- {
- baseURL: "http://localhost:3002",
+ const response = await betterFetch<Session>(
+ `${process.env.NEXT_PUBLIC_API_URL}/api/auth/get-session`,
+ {
+ baseURL: process.env.NEXT_PUBLIC_APP_URL,📝 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.
| const response = await betterFetch<Session>("http://localhost:3001/api/auth/get-session", | |
| { | |
| baseURL: "http://localhost:3002", | |
| headers: { | |
| cookie: (await headers()).get("cookie") || "", | |
| }, | |
| }); | |
| const response = await betterFetch<Session>( | |
| `${process.env.NEXT_PUBLIC_API_URL}/api/auth/get-session`, | |
| { | |
| baseURL: process.env.NEXT_PUBLIC_APP_URL, | |
| headers: { | |
| cookie: (await headers()).get("cookie") || "", | |
| }, | |
| }); |
| actions: { | ||
| sendMessage, | ||
| sendAiGreeting, | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Error handling improvements needed in AI message actions
The implementations of sendMessage and sendAiGreeting have several potential issues:
- No try-catch blocks to handle potential errors from
streamUI,getSession, or state updates - No validation of input parameters
- Unchecked state mutations with
history.updateandhistory.done - Console.log statements left in production code
- No error handling for cases where session user data is missing
Suggested improvements:
- Add proper error handling around async operations
- Validate input message and session data
- Remove debug console.log statements
- Add type guards for null checks
- Consider adding error boundaries for UI components
🔗 Analysis chain
Verify action implementations
The actions are imported but their implementations should be verified to ensure they handle errors appropriately and maintain consistent state updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for sendMessage and sendAiGreeting implementations
ast-grep --pattern 'export async function sendMessage($_) {
$$$
}'
ast-grep --pattern 'export async function sendAiGreeting($_) {
$$$
}'
Length of output: 144
Script:
#!/bin/bash
# Search for function implementations with more flexible patterns
rg "function sendMessage|function sendAiGreeting" -A 10
# Also search for const declarations in case they are defined as arrow functions
rg "const sendMessage|const sendAiGreeting" -A 10
Length of output: 1606
Script:
#!/bin/bash
# Let's check the complete implementation and error handling
rg "export const sendMessage" -A 30 apps/app/actions/action.tsx
# Also check for the streamUI implementation since it's used in sendAiGreeting
rg "streamUI" -A 10
Length of output: 3385
| @@ -0,0 +1,27 @@ | |||
| "use client"; | |||
|
|
|||
| import { motion } from "motion/react"; | |||
There was a problem hiding this comment.
Fix incorrect import path
The import from 'motion/react' is incorrect. It should be from 'framer-motion'.
-import { motion } from "motion/react";
+import { motion } from "framer-motion";📝 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 { motion } from "motion/react"; | |
| import { motion } from "framer-motion"; |
| @@ -1,19 +1,47 @@ | |||
| import * as React from "react"; | |||
|
|
|||
| import { ChangeEvent, useRef } from "react"; | |||
| import { cn } from "@/lib/utils"; | |||
|
|
|||
| export interface TextareaProps | |||
| extends React.TextareaHTMLAttributes<HTMLTextAreaElement> {} | |||
|
|
|||
| const Textarea = React.forwardRef<HTMLTextAreaElement, TextareaProps>( | |||
| ({ className, ...props }, ref) => { | |||
There was a problem hiding this comment.
Remove unused 'ref' parameter
The ref parameter is defined but never used, as it's been replaced with textareaRef.
-({ className, ...props }, ref) => {
+({ className, ...props }) => {📝 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.
| ({ className, ...props }, ref) => { | |
| ({ className, ...props }) => { |
🧰 Tools
🪛 eslint
[error] 9-9: 'ref' is defined but never used.
(@typescript-eslint/no-unused-vars)
| export default function Proceed() { | ||
| const [activeButton, setActiveButton] = useState<"yes" | "no" | null>(null); | ||
| const { sendMessage } = useActions<typeof AI>(); | ||
| const [messages, setMessages] = useUIState<typeof AI>(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused state and console.log statements
The messages state is never used, and console.log statements should be removed in production code.
-const [messages, setMessages] = useUIState<typeof AI>();
+const [, setMessages] = useUIState<typeof AI>();
-console.log("response", JSON.stringify(response.display));Also applies to: 16-16
🧰 Tools
🪛 eslint
[error] 10-10: 'messages' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
| const handleClick = async (reply: "yes"|"no") => { | ||
| setActiveButton(reply); | ||
| try { | ||
| const response = await sendMessage(reply); | ||
| setMessages((currentMessages) => [...currentMessages, response]); | ||
| console.log("response", JSON.stringify(response.display)); | ||
| } catch (error) { | ||
| setActiveButton(null); | ||
| console.log("error", error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and user feedback
The error handling could be enhanced to provide better user feedback.
const handleClick = async (reply: "yes"|"no") => {
setActiveButton(reply);
+ let timeoutId: NodeJS.Timeout;
+
+ // Set loading state after a brief delay
+ timeoutId = setTimeout(() => setActiveButton(reply), 500);
+
try {
const response = await sendMessage(reply);
setMessages((currentMessages) => [...currentMessages, response]);
- console.log("response", JSON.stringify(response.display));
} catch (error) {
setActiveButton(null);
- console.log("error", error);
+ // Show error toast or feedback to user
+ console.error('Failed to send message:', error);
+ } finally {
+ clearTimeout(timeoutId);
}
};Committable suggestion skipped: line range outside the PR's diff.
| const {name,email} = session!.user | ||
| const contentString = `Hi ${name}, welcome to Plura AI!.Your email is ${email}.I am going to help you with oboarding your acccount` |
There was a problem hiding this comment.
Security: PII exposure in AI prompt
The greeting message exposes user's email address to the AI model. This is unnecessary and could lead to PII leakage.
- const contentString = `Hi ${name}, welcome to Plura AI!.Your email is ${email}.I am going to help you with oboarding your acccount`
+ const contentString = `Hi ${name}, welcome to Plura AI! I am going to help you with onboarding your account.`📝 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.
| const {name,email} = session!.user | |
| const contentString = `Hi ${name}, welcome to Plura AI!.Your email is ${email}.I am going to help you with oboarding your acccount` | |
| const {name,email} = session!.user | |
| const contentString = `Hi ${name}, welcome to Plura AI! I am going to help you with onboarding your account.` |
| useEffect(() => { | ||
| const fetchProceed = async()=> { | ||
| await sleep(3000) | ||
| const response = await sendMessage("should we continue?") | ||
| setMessages((currentMessages) => [...currentMessages,response]); | ||
|
|
||
| } | ||
| if(messages.length === 1){ | ||
| fetchProceed() | ||
| } | ||
| }, [messages.length]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve effect hooks implementation
The current implementation has several issues:
- Hard-coded delay value
- Missing dependency on sendMessage
- Magic number (1) for message length check
+ const PROCEED_DELAY_MS = 3000;
+ const INITIAL_MESSAGE_COUNT = 1;
useEffect(() => {
const fetchProceed = async()=> {
- await sleep(3000)
+ await sleep(PROCEED_DELAY_MS)
const response = await sendMessage("should we continue?")
setMessages((currentMessages) => [...currentMessages,response]);
}
- if(messages.length === 1){
+ if(messages.length === INITIAL_MESSAGE_COUNT){
fetchProceed()
}
- }, [messages.length])
+ }, [messages.length, sendMessage])📝 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(() => { | |
| const fetchProceed = async()=> { | |
| await sleep(3000) | |
| const response = await sendMessage("should we continue?") | |
| setMessages((currentMessages) => [...currentMessages,response]); | |
| } | |
| if(messages.length === 1){ | |
| fetchProceed() | |
| } | |
| }, [messages.length]) | |
| const PROCEED_DELAY_MS = 3000; | |
| const INITIAL_MESSAGE_COUNT = 1; | |
| useEffect(() => { | |
| const fetchProceed = async()=> { | |
| await sleep(PROCEED_DELAY_MS) | |
| const response = await sendMessage("should we continue?") | |
| setMessages((currentMessages) => [...currentMessages,response]); | |
| } | |
| if(messages.length === INITIAL_MESSAGE_COUNT){ | |
| fetchProceed() | |
| } | |
| }, [messages.length, sendMessage]) |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
apps/app/components/custom/onboarding/workspace-form.tsx (1)
20-20: Add space after 'async' keyword for better readabilityAdding a space between the
asynckeyword and the function parameters improves code readability.Apply this diff:
- const createWorkspace =async(workspaceName:string)=>{ + const createWorkspace = async (workspaceName: string) => {- const handleSubmit = async(e: React.FormEvent<HTMLFormElement>) => { + const handleSubmit = async (e: React.FormEvent<HTMLFormElement>) => {Also applies to: 33-33
apps/app/components/custom/onboarding/proceed.tsx (1)
42-42: Remove unused parameter 'e' in the onClick handlerThe event parameter
ein theonClickhandler is not used and can be safely removed.Apply this diff to clean up the code:
- onClick={(e) => handleClick("yes")} + onClick={() => handleClick("yes")}🧰 Tools
🪛 eslint
[error] 42-42: 'e' is defined but never used.
(@typescript-eslint/no-unused-vars)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
apps/api/app/api/[[...route]]/route.ts(1 hunks)apps/app/actions/action.tsx(1 hunks)apps/app/components/custom/onboarding/ChatInput.tsx(1 hunks)apps/app/components/custom/onboarding/proceed.tsx(1 hunks)apps/app/components/custom/onboarding/workspace-form.tsx(1 hunks)packages/crypt/tsconfig.tsbuildinfo(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/crypt/tsconfig.tsbuildinfo
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/app/components/custom/onboarding/ChatInput.tsx
- apps/api/app/api/[[...route]]/route.ts
🧰 Additional context used
🪛 eslint
apps/app/components/custom/onboarding/proceed.tsx
[error] 10-10: 'messages' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 11-11: 'setAiMessages' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 42-42: 'e' is defined but never used.
(@typescript-eslint/no-unused-vars)
apps/app/components/custom/onboarding/workspace-form.tsx
[error] 7-7: 'CardFooter' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 8-8: 'CardHeader' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 31-31: 'messages' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
apps/app/actions/action.tsx
[error] 3-3: 'createStreamableValue' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 10-10: 'Button' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 14-14: 'CoreMessage' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 14-14: 'generateId' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 Biome (1.9.4)
apps/app/actions/action.tsx
[error] 74-74: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 98-98: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🔇 Additional comments (5)
apps/app/components/custom/onboarding/proceed.tsx (2)
10-11: Remove unused state variables 'messages' and 'setAiMessages'
The state variables messages and setAiMessages are assigned but never used in the component.
Apply this diff to remove the unused state variables:
- const [messages, setMessages] = useUIState<typeof AI>();
+ const [, setMessages] = useUIState<typeof AI>();
- const [aiMessages, setAiMessages] = useAIState<typeof AI>();
+ const [aiMessages] = useAIState<typeof AI>();🧰 Tools
🪛 eslint
[error] 10-10: 'messages' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 11-11: 'setAiMessages' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
19-19: Remove unnecessary console.log statement
Logging errors using console.log is not recommended in production code. Use console.error or handle the error appropriately.
Apply this diff to improve error handling:
- console.log("error", error);
+ console.error("Failed to send message:", error);apps/app/actions/action.tsx (3)
129-129: Avoid exposing user's email address in AI prompts
Including the user's email in the AI prompt may lead to PII leakage and potential security issues.
Apply this diff to remove the email from the prompt:
- const contentString = `Hi ${name}, welcome to Plura AI!.Your email is ${email}.I am going to help you with oboarding your acccount`
+ const contentString = `Hi ${name}, welcome to Plura AI! I am going to help you with onboarding your account.`36-37: Implement proper state management for message history
The message history can grow indefinitely, leading to potential memory issues. Implementing a maximum history size can alleviate this.
Consider applying this change to manage the history size:
+ const MAX_HISTORY_SIZE = 50;
const history = getMutableAIState<typeof AI>();
- history.update([...history.get(), { role: "user", content: message }]);
+ const currentHistory = history.get();
+ const newHistory = [...currentHistory.slice(-MAX_HISTORY_SIZE), { role: "user", content: message }];
+ history.update(newHistory);29-33:
Add error handling and input validation in 'sendMessage' function
The function lacks error handling for API failures and input validation for empty messages, which could lead to unhandled exceptions.
Apply this diff to enhance error handling and input validation:
export const sendMessage = async (
message: string
): Promise<ClientMessage> => {
+ if (!message?.trim()) {
+ throw new Error('Message cannot be empty');
+ }
const history = getMutableAIState<typeof AI>();
+ try {
/* ... existing code ... */
+ } catch (error) {
+ console.error('Error in AI response:', error);
+ throw new Error('Failed to get AI response');
+ }
};Likely invalid or redundant comment.
| CardFooter, | ||
| CardHeader, |
There was a problem hiding this comment.
Remove unused imports 'CardFooter' and 'CardHeader'
The imports CardFooter and CardHeader are imported but never used, which can be removed to clean up the code.
Apply this diff to remove the unused imports:
import {
Card,
CardContent,
CardDescription,
- CardFooter,
- CardHeader,
CardTitle,
} from "@/components/ui/card";📝 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.
| CardFooter, | |
| CardHeader, | |
| import { | |
| Card, | |
| CardContent, | |
| CardDescription, | |
| CardTitle, | |
| } from "@/components/ui/card"; |
🧰 Tools
🪛 eslint
[error] 7-7: 'CardFooter' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 8-8: 'CardHeader' is defined but never used.
(@typescript-eslint/no-unused-vars)
| const [isLoading, setIsLoading] = useState(false); | ||
| const [value, setValue] = useState(""); | ||
| const {sendMessage} = useActions<typeof AI>(); | ||
| const [messages, setMessages] = useUIState<typeof AI>(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused state variable 'messages'
The state variable messages is assigned but never used in the component.
Apply this diff to remove the unused state variable:
- const [messages, setMessages] = useUIState<typeof AI>();
+ const [, setMessages] = useUIState<typeof AI>();📝 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.
| const [messages, setMessages] = useUIState<typeof AI>(); | |
| const [, setMessages] = useUIState<typeof AI>(); |
🧰 Tools
🪛 eslint
[error] 31-31: 'messages' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
| description: | ||
| "when the user responds with yes then render the workspace form", | ||
| parameters: z.object({}), | ||
| generate: async function* ({}) { |
There was a problem hiding this comment.
Avoid using empty object patterns in Zod parameters
Using an empty object pattern {} in Zod's parameters is unnecessary and may cause linting errors.
If no parameters are needed, you can omit the parameters field or define it as z.unknown().
Apply this diff:
At line 74:
- parameters: z.object({}),
+ // You can omit parameters or define as z.unknown()At line 98:
- parameters: z.object({}),
+ // You can omit parameters or define as z.unknown()Also applies to: 98-98
🧰 Tools
🪛 Biome (1.9.4)
[error] 74-74: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
| @@ -0,0 +1,156 @@ | |||
| "use server"; | |||
| import { ReactNode } from "react"; | |||
| import { createStreamableValue, getMutableAIState, streamUI } from "ai/rsc"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused imports to clean up the code
The following imports are not used and can be removed:
createStreamableValueat line 3Buttonat line 10CoreMessage,generateIdat line 14
Apply this diff to remove unused imports:
import { ReactNode } from "react";
-import { createStreamableValue, getMutableAIState, streamUI } from "ai/rsc";
+ import { getMutableAIState, streamUI } from "ai/rsc";
/* ... */
-import { Button } from "@/components/ui/button";-import { CoreMessage, generateId, ToolInvocation } from "ai";
+import { ToolInvocation } from "ai";Also applies to: 10-10, 14-14
🧰 Tools
🪛 eslint
[error] 3-3: 'createStreamableValue' is defined but never used.
(@typescript-eslint/no-unused-vars)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
apps/api/app/v1/[[...route]]/route.ts (1)
Security: CORS configuration should be environment-specific and more restrictive
The review comment's concerns are valid. The code shows:
- Hardcoded CORS origins mixing both development (localhost) and production domains
- Basic header configuration that could be more specific
- No environment-based configuration currently exists
Additionally, the git merge markers (
<<<<<<< HEAD,=======,>>>>>>>) in the file indicate this is part of an unresolved merge conflict, which needs immediate attention.
apps/api/app/v1/[[...route]]/route.ts: Resolve merge conflict and implement environment-based CORS configuration- Consider moving CORS configuration to a separate configuration file for better maintainability
- Implement more specific Content-Type headers as suggested in the review
🔗 Analysis chain
Line range hint
20-38: Security: Consider environment-based CORS configurationWhile the CORS configuration is functional, it could be improved for better security and maintainability:
- Move allowed origins to environment configuration to avoid hardcoding
- Consider being more specific with allowed headers instead of allowing all Content-Type variations
Consider refactoring to use environment variables:
-const allowedOrigins = [ - "http://localhost:3002", - "http://localhost:3003", - "http://localhost:3004", - "https://www.plura.pro", - "https://app.plura.pro", -]; +const allowedOrigins = process.env.CORS_ALLOWED_ORIGINS?.split(',') ?? []; app.use( "*", cors({ origin: allowedOrigins, allowMethods: ["GET", "POST", "PATCH", "DELETE", "OPTIONS"], - allowHeaders: ["Content-Type", "Authorization"], + allowHeaders: [ + "Authorization", + "Content-Type: application/json", + "Content-Type: multipart/form-data" + ], exposeHeaders: ["Content-Length"], maxAge: 600, credentials: true, }), );Let's verify the current CORS configuration usage:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any potential CORS bypass or misconfigurations rg -l 'cors|CORS' --type tsLength of output: 90
Script:
#!/bin/bash # Let's check the content of both files to understand the CORS configuration better echo "=== Content of apps/api/middleware.ts ===" cat apps/api/middleware.ts echo -e "\n=== Content of apps/api/app/v1/[[...route]]/route.ts ===" cat apps/api/app/v1/[[...route]]/route.ts # Also check for any environment variable configurations echo -e "\n=== Checking for environment variable configurations ===" rg "process\.env\.CORS" --type tsLength of output: 2520
🧰 Tools
🪛 Biome (1.9.4)
[error] 17-19: Expected a statement but instead found '<<<<<<< HEAD:apps/api/app/api/[[...route]]/route.ts
======='.Expected a statement here.
(parse)
apps/app/middleware.ts (1)
24-24: Remove debug console.log statementRemove commented-out console.log statements before merging to production.
- // console.log("seesion mid", session);apps/api/app/v1/[[...route]]/user.ts (2)
Line range hint
6-26: Consider implementing field selection and proper error handlingThe current implementation could be improved in several areas:
- Implement field selection to avoid exposing sensitive user data
- Add error handling for database operations
- Use correct HTTP status codes (401 for unauthorized instead of 400)
Consider applying these improvements:
.get("/self", async (c) => { const currentUser = await auth.api.getSession({ headers: c.req.raw.headers, }); if (!currentUser) { return c.json( { message: "Not logged in", - status: 400, + status: 401, }, - 400, + 401, ); } - const user = await prisma.user.findUnique({ - where: { - id: currentUser.user.id, - }, - }); + try { + const user = await prisma.user.findUnique({ + where: { + id: currentUser.user.id, + }, + select: { + id: true, + name: true, + email: true, + image: true, + isOnboarding: true, + // Add other necessary fields + }, + }); + + if (!user) { + return c.json( + { + message: "User not found", + status: 404, + }, + 404, + ); + } return c.json( { user, }, 200, ); + } catch (error) { + return c.json( + { + message: "Internal server error", + status: 500, + }, + 500, + ); + } })
Line range hint
6-26: Consider implementing onboarding status checksSince this endpoint is part of the onboarding feature implementation, consider adding onboarding-specific response data to help guide the frontend flow.
Consider enhancing the response to include:
- Onboarding status flags
- Next onboarding steps
- Required actions
Example enhancement:
return c.json( { user, + onboarding: { + isComplete: !user.isOnboarding, + nextStep: user.isOnboarding ? 'profile_setup' : null, + requiredFields: user.isOnboarding ? ['name', 'preferences'] : [], + } }, 200, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
apps/api/app/v1/[[...route]]/health.ts(1 hunks)apps/api/app/v1/[[...route]]/route.ts(3 hunks)apps/api/app/v1/[[...route]]/session.ts(1 hunks)apps/api/app/v1/[[...route]]/user.ts(2 hunks)apps/app/middleware.ts(2 hunks)packages/auth/src/auth.ts(1 hunks)packages/database/prisma/schema.prisma(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/api/app/v1/[[...route]]/health.ts
- apps/api/app/v1/[[...route]]/session.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/database/prisma/schema.prisma
- packages/auth/src/auth.ts
🧰 Additional context used
🪛 Biome (1.9.4)
apps/api/app/v1/[[...route]]/route.ts
[error] 17-19: Expected a statement but instead found '<<<<<<< HEAD:apps/api/app/api/[[...route]]/route.ts
======='.
Expected a statement here.
(parse)
🔇 Additional comments (5)
apps/api/app/v1/[[...route]]/route.ts (1)
Line range hint 41-57: LGTM: Clean route structure and exports
The route handlers and exports are well-organized and follow Hono's best practices. The use of Edge Runtime is appropriate for this API setup.
apps/app/middleware.ts (3)
12-12: Use environment variables for domain configuration
The hardcoded domain configuration makes the application less maintainable and harder to configure across different environments.
44-44: LGTM! Proper middleware matcher configuration
The matcher configuration correctly excludes static assets, API routes, and other system files from middleware processing, which is a good practice for performance.
33-38:
Add safety checks and prevent redirect loops
The current implementation has two issues:
- Potential redirect loops if the session state is inconsistent
- Missing type safety checks for session.user properties
+ // Add type guard
+ if (!session.user || typeof session.user.isOnboarding !== 'boolean') {
+ console.error('Invalid session user data');
+ return NextResponse.redirect(redirectUrl);
+ }
+
+ // Add redirect loop prevention
+ const MAX_REDIRECTS = 3;
+ const redirectCount = parseInt(request.headers.get('x-redirect-count') || '0');
+
+ if (redirectCount >= MAX_REDIRECTS) {
+ return new NextResponse('Too many redirects', { status: 500 });
+ }
if (!currentPath.startsWith("/onboarding") && !session.user.isOnboarding) {
- return NextResponse.redirect(`${appDomain}/onboarding`);
+ const response = NextResponse.redirect(`${appDomain}/onboarding`);
+ response.headers.set('x-redirect-count', (redirectCount + 1).toString());
+ return response;
}
if (currentPath.startsWith("/onboarding") && session.user.isOnboarding) {
- return NextResponse.redirect(`${appDomain}/home`);
+ const response = NextResponse.redirect(`${appDomain}/home`);
+ response.headers.set('x-redirect-count', (redirectCount + 1).toString());
+ return response;
}Likely invalid or redundant comment.
apps/api/app/v1/[[...route]]/user.ts (1)
Line range hint 6-26: Verify session management implementation
The endpoint relies on the custom auth package for session management. Let's verify its implementation and security.
✅ Verification successful
Let me gather more information about the auth package implementation.
Let me check the better-auth package's usage across the codebase to ensure consistent implementation.
Session management implementation is secure and consistent
The implementation uses the well-established better-auth package with proper configuration:
- Secure session management with trusted origins and environment-specific domains
- Prisma adapter for database integration
- Multi-session support enabled
- Social provider authentication with proper client credentials
- Production-specific cross-subdomain cookie handling
- Consistent session verification across endpoints
- Protected routes with proper middleware implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the auth package implementation
rg -A 10 "getSession" packages/auth/
# Check for other session usage patterns
rg -A 5 "currentUser" apps/
Length of output: 961
Script:
#!/bin/bash
# Check auth package structure and implementation
fd -t f . packages/auth/ -x cat {}
Length of output: 2108
Script:
#!/bin/bash
# Check for auth usage patterns and middleware
rg -A 5 "betterAuth|auth\.api" apps/
# Check for any custom auth middleware
fd middleware apps/ -x cat {}
Length of output: 4152
| <<<<<<< HEAD:apps/api/app/api/[[...route]]/route.ts | ||
| ======= |
There was a problem hiding this comment.
Critical: Unresolved merge conflict markers detected
The file contains unresolved Git merge conflict markers. This will cause syntax errors and break the application. Please resolve the merge conflict before proceeding.
Remove the following lines:
-<<<<<<< HEAD:apps/api/app/api/[[...route]]/route.ts
-=======🧰 Tools
🪛 Biome (1.9.4)
[error] 17-19: Expected a statement but instead found '<<<<<<< HEAD:apps/api/app/api/[[...route]]/route.ts
======='.
Expected a statement here.
(parse)
| console.log("redirecting to sign in"); | ||
| return NextResponse.redirect(redirectUrl); | ||
| } | ||
| const currentPath = request.nextUrl.pathname; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for session fetch failures
The current implementation doesn't handle potential errors from the session fetch. Consider adding try-catch block to handle network failures or invalid responses gracefully.
- if (!session) {
- console.log("redirecting to sign in");
- return NextResponse.redirect(redirectUrl);
- }
+ try {
+ if (!session) {
+ return NextResponse.redirect(redirectUrl);
+ }
+ } catch (error) {
+ console.error('Session verification failed:', error);
+ return NextResponse.redirect(redirectUrl);
+ }Committable suggestion skipped: line range outside the PR's diff.
merger conflict markers
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style
Chores