-
-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add onboarding ai agent #86
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes in this pull request encompass modifications to several files in the Changes
Possibly related PRs
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
MessageBubble
component 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
sendMessage
action's return type should be validated against theClientMessage
type 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.yaml
is 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
isOnboarding
field 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
false
ensures 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
AppSidebar
andSidebarProvider
- 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
:
ServerMessage
andClientMessage
types are properly defined with appropriate fields and type constraintssendMessage
function is correctly typed, accepting a string message and returning a Promise with the expectedClientMessage
shape- 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
isOnboarding
field 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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; | |
} | |
})(); |
const history = getMutableAIState<typeof AI>(); | ||
history.update([...history.get(), { role: "user", content: message }]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 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
streamUI
andtogetherai
operations - 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 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.example
file in the API service directory - Update the main
README.md
with 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
Summary by CodeRabbit
Release Notes
New Features
Enhancements
isOnboarding
field to user profiles to track onboarding status.Bug Fixes
Dependencies