-
-
Notifications
You must be signed in to change notification settings - Fork 6
Supabase #327
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
base: main
Are you sure you want to change the base?
Supabase #327
Conversation
This commit refactors the user invitation and chat sharing functionality to integrate with the existing settings UI, providing a more coherent and streamlined user experience. Key changes include: - The `inviteUserToChat` server action is now called from the `UserManagementForm` in the settings section. - The `chatId` is now passed down to the settings components to enable chat-specific invitations. - The redundant `ChatShareDialog` component has been removed from the `ChatPanel`. - The unused `lib/actions/users.ts` file has been deleted. - The `settingsFormSchema` has been updated to include 'collaborator' and 'owner' as valid roles.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
WalkthroughMigrates chat persistence from Drizzle to Supabase (server/browser clients, RPCs, migrations, RLS), rewires API routes and UI to new chat types/flows, adds realtime subscriptions, sharing/invitations via chat participants, and RAG-based prompt augmentation with optional geospatial rendering. Removes legacy DB modules/config. Adds Playwright verification scripts. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client (Chat UI)
participant API as POST /api/chat
participant S as Supabase (RPC)
participant Auth as Auth
UI->>API: POST initialMessageContent
API->>Auth: get current user
Auth-->>API: userId or 401
API->>S: save_chat_with_messages(chat+first message)
S-->>API: chatId or error
API-->>UI: 201 { chatId } or error
sequenceDiagram
participant UI as Chat Component
participant SB as Supabase Realtime
participant Srv as Server Actions
UI->>Srv: getChat(id), getChatMessages(id)
Srv-->>UI: chatData, messages
UI->>SB: subscribe channel (messages, presence)
SB-->>UI: INSERT message event
UI->>UI: append to messages
SB-->>UI: presence sync
UI->>UI: update onlineUsers
sequenceDiagram
participant UI as Client
participant A as app/actions.tsx
participant RAG as retrieveContext
participant LLM as Researcher/Writer
UI->>A: submit user input (+optional location, chatId)
A->>RAG: retrieveContext(query, chatId, location)
RAG-->>A: [context snippets]
A->>A: augment system prompt
A->>LLM: call with augmentedSystemPrompt
LLM-->>A: response (incl. geoJson?)
A-->>UI: UI state (messages + GeoJsonLayer if present)
sequenceDiagram
participant UI as Settings/UserMgmt
participant Srv as inviteUserToChat
participant SB as Supabase
participant Auth as Auth
UI->>Srv: inviteUserToChat(chatId, email)
Srv->>Auth: get current user
Auth-->>Srv: userId
Srv->>SB: verify owner role in chat_participants
Srv->>SB: find user by email
Srv->>SB: insert chat_participants (collaborator)
SB-->>Srv: ok or unique violation
Srv-->>UI: success or error
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||||
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.
- Realtime subscription is mutating UI state with DB messages, corrupting the UI state shape; switch to updating AI state and map fields consistently.
- The collaboration invite flow queries a non-existent
public.users.email; use an RPC againstauth.usersor a profiles table. clearChatsredirects from within an API-used function, breaking JSON responses.- Migrations reference
public.usersand usegen_random_uuid()without enablingpgcrypto, which will fail in typical Supabase setups.
Additional notes (1)
- Maintainability |
components/chat-panel.tsx:14-26
ChatPanelreceived new props (chatId,shareableLink) but they’re not used anywhere in the component. Leaving unused props around increases component surface area and confusion.
Summary of changes
- Replaced Drizzle-based persistence with Supabase across API routes and actions.
- Integrated RAG context retrieval into the system prompt flow and augmented researcher prompt.
- Added realtime Supabase subscription and presence tracking in Chat component; passed
chatIdto settings and user management. - Removed
ChatShareDialogand Drizzle-related files; added Supabase clients, persistence layer, and migrations for collaboration (chat_participants, RLS policies) and RAG helpers. - Updated settings schema to use roles
ownerandcollaborator; connectedUserManagementFormtoinviteUserToChatserver action. - Adjusted history components and routes to use new chat fetching/deletion APIs.
| <GeoJsonLayer | ||
| id={`${id}-geojson`} | ||
| data={location.geojson} | ||
| /> | ||
| )} | ||
| </> |
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.
GeoJsonLayer is used in JSX but not imported anywhere in this file. This will throw at runtime (and likely at compile time). Please import the component or remove the render until it’s properly wired up.
Suggestion
Add a proper import for the layer component (adjust the path to your actual component), and guard against missing location.geojson:
import { GeoJsonLayer } from '@/components/map/geojson-layer'
...
{location?.geojson && (
<GeoJsonLayer id={`${id}-geojson`} data={location.geojson} />
)}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion
| revalidatePath('/') | ||
| redirect('/') |
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.
This handler calls clearChats, which unconditionally performs a redirect('/'). Redirecting from within a function used by an API route will short-circuit your JSON response and likely break the client-side fetch call. API handlers should return a response, not trigger navigation.
Suggestion
Refactor clearChats to not redirect by default. For example:
export async function clearChats(userId?: string | null, options: { redirectOnSuccess?: boolean } = {}) {
...
if (options.redirectOnSuccess) {
revalidatePath('/')
redirect('/')
}
}Then update callers in API routes to omit redirectOnSuccess and keep JSON responses consistent; use redirectOnSuccess: true only from server actions that can redirect. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this refactor
| .select('*, chat_participants!inner(*)') | ||
| .eq('id', id) | ||
| .single() | ||
|
|
||
| if (error) { | ||
| console.error(`Error fetching chat ${id} from Supabase:`, error) | ||
| return null | ||
| } | ||
|
|
||
| return data as Chat | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves all messages for a specific chat. | ||
| * @param chatId The ID of the chat. | ||
| * @returns A promise that resolves to an array of DrizzleMessage objects. | ||
| */ | ||
| export async function getChatMessages(chatId: string): Promise<DrizzleMessage[]> { | ||
| export async function getChatMessages(chatId: string): Promise<any[]> { |
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.
getChat still returns early if userId is missing, even though the Supabase server client is session-aware and RLS can enforce access. This prevents server callers that don’t have the user id at hand from fetching chats, and it already breaks the caller in components/chat.tsx.
Suggestion
Remove the if (!userId) return null guard and rely on the Supabase server client’s session with RLS to authorize. The function signature can keep userId for explicit filtering, but should not require it:
export async function getChat(id: string, userId?: string): Promise<Chat | null> {
const supabase = getSupabaseServerClient()
const { data, error } = await supabase
.from('chats')
.select('*, chat_participants!inner(*)')
.eq('id', id)
.single()
if (error) { ... }
return data as Chat
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change
| const supabase = getSupabaseServerClient() | ||
| const { data, error } = await supabase | ||
| .from('chats') | ||
| .select('*') | ||
|
|
||
| if (error) { | ||
| console.error('Error fetching chats from Supabase:', error) | ||
| return [] | ||
| } | ||
|
|
||
| return (data as Chat[]) || [] |
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.
getChats ignores the userId parameter and selects * from chats. While RLS will restrict rows, it’s clearer and more efficient to filter at the query (e.g., by membership in chat_participants) to minimize scanned rows and make intent explicit.
Suggestion
Join/filter by chat_participants to return only chats the user belongs to:
const supabase = getSupabaseServerClient();
const { data, error } = await supabase
.from('chats')
.select('id, title, created_at, visibility')
.in('id', (
await supabase.from('chat_participants').select('chat_id').eq('user_id', userId)
).data?.map((r) => r.chat_id) || []);Or use a single RPC tailored for this list. Reply with "@CharlieHelps yes please" if you want me to implement the filtered query.
| if (!messages.some((m: AIMessage) => m.id === newMessage.id)) { | ||
| setMessages((prevMessages: AIMessage[]) => [...prevMessages, newMessage]); | ||
| } | ||
| }) | ||
| .on('presence', { event: 'sync' }, () => { | ||
| const newState = channel.presenceState(); | ||
| const users = Object.keys(newState).map(key => (newState[key][0] as any).user_id); | ||
| setOnlineUsers(users); |
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.
The Supabase realtime insert handler is pushing raw DB rows into messages via setMessages, but messages here is the UI state from useUIState (an array of { id, component }), not the AI message list. Appending DB rows to UI state will corrupt the UI state shape and cause rendering issues.
Suggestion
Use useAIState to append to the AI messages instead of useUIState, and map the DB row to your AIMessage shape:
const [aiState, setAIState] = useAIState()
...
.on('postgres_changes', { event: 'INSERT', schema: 'public', table: 'messages', filter: `chat_id=eq.${id}` }, (payload) => {
const m: any = payload.new
const next = { id: m.id, role: m.role, content: m.content, createdAt: new Date(m.created_at) }
setAIState(prev => ({ ...prev, messages: prev.messages.some(mm => mm.id === next.id) ? prev.messages : [...prev.messages, next] }))
})Also remove the use of setMessages for this flow. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change
app/search/[id]/page.tsx
Outdated
| const initialMessages = await getChatMessages(chat.id); | ||
|
|
||
| return ( | ||
| <AI | ||
| initialAIState={{ | ||
| chatId: chat.id, | ||
| messages: initialMessages, // Use the transformed messages from the database | ||
| messages: initialMessages as AIMessage[], // Use the transformed messages from the database | ||
| // isSharePage: true, // This was in PR#533, but share functionality is removed. |
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.
getChatMessages now returns any[] and downstream code casts DB rows to AIMessage without mapping. The DB uses snake_case (created_at) and joins (locations(*)), which won’t match AIMessage fields. This will create subtle runtime issues.
Suggestion
Map DB rows to the AIMessage shape explicitly:
const raw = await getChatMessages(chat.id)
const initialMessages: AIMessage[] = raw.map((m: any) => ({
id: m.id,
role: m.role,
content: m.content,
createdAt: m.created_at ? new Date(m.created_at) : undefined,
locations: m.locations,
}))Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change
| const result = await inviteUserToChat(chatId, newUserEmail); | ||
|
|
||
| if (result.error) { | ||
| toast({ title: 'Error adding user', description: result.error, variant: 'destructive' }); | ||
| form.setError("newUserEmail", { type: "manual", message: result.error }); // Show server error on field | ||
| } else if (result.user) { | ||
| toast({ title: 'User Added', description: `${result.user.email} was successfully added.` }); | ||
| append(result.user); // Add user with ID from server | ||
| form.setError("newUserEmail", { type: "manual", message: result.error }); | ||
| } else { | ||
| toast({ title: 'User Invited', description: `${newUserEmail} was successfully invited.` }); | ||
| // We don't append here because the user needs to accept the invite. | ||
| // We can add a "pending invitations" section in the future. | ||
| form.resetField("newUserEmail"); |
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.
The role selector in the User Management form is no longer used—the inviteUserToChat action ignores the selected role and always inserts collaborator. This is confusing UX and dead state in the form.
Suggestion
Either remove the role selector from the form, or plumb the selected role through to the action:
const role = form.getValues('newUserRole') || 'collaborator'
const result = await inviteUserToChat(chatId, newUserEmail, role)and update inviteUserToChat signature to accept role: 'owner' | 'collaborator' and insert it. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change
| const userId = 'anonymous'; | ||
|
|
||
| const form = useForm<SettingsFormValues>({ |
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.
Settings still uses a hard-coded userId = 'anonymous' when saving the system prompt, so everyone is writing to the same record. This will overwrite prompts across users.
Suggestion
Refactor the saveSystemPrompt action to obtain the current user server-side (via getCurrentUserIdOnServer) so the client doesn’t need to pass an id. For example:
// server action
export async function saveSystemPromptForCurrentUser(prompt: string) {
const userId = await getCurrentUserIdOnServer(); if (!userId) return { error: 'Not authenticated' }
return supabaseSaveSystemPrompt(userId, prompt)
}Then call that from the form submit. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change
| const result = await clearChats(userId); | ||
| if (result && 'error' in result) { | ||
| return NextResponse.json({ error: result.error }, { status: 500 }); | ||
| } | ||
|
|
||
| return NextResponse.json({ message: 'History cleared successfully' }, { status: 200 }); | ||
|
|
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.
This API route will still succeed even if clearChats triggers a redirect (see related comment). Ensure the delete call returns a 200 JSON response to the client instead of a redirect payload.
Suggestion
After refactoring clearChats to not redirect by default, call it with await clearChats(userId, { redirectTo: null }) and keep returning the success JSON. Reply with "@CharlieHelps yes please" and I’ll update both the action and this call site.
| # Wait for the Share button to be visible and click it | ||
| share_button = page.locator('button:has-text("Share")') | ||
| share_button.wait_for(state='visible', timeout=30000) | ||
| share_button.click() | ||
|
|
||
| # Wait for the dialog to appear before taking a screenshot | ||
| page.wait_for_selector('div[role="dialog"]', state='visible', timeout=10000) | ||
|
|
||
| # Take a screenshot of the share dialog | ||
| page.screenshot(path="jules-scratch/verification/verification.png") |
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.
The Playwright script targets a "Share" button and dialog, but this PR removes the redundant ChatShareDialog. This script is likely stale and will fail CI or local verification.
Suggestion
Update or remove this verification. For example, verify collaboration UI in Settings instead (e.g., opening the settings panel and checking for the User Management section). Reply with "@CharlieHelps yes please" and I’ll replace this script with one that validates the new collaboration flow.
This commit fixes a critical client-side exception that occurred in the production deployment. The error "getChat called without userId" was caused by the `getChat` server action being called from a client component (`Chat.tsx`) without a valid user session being passed, which returned `null` and crashed the application. The fix involves two main changes: 1. **Robust `getChat` Action:** The `getChat` function in `lib/actions/chat.ts` has been refactored to no longer accept a `userId` as a parameter. Instead, it now reliably fetches the authenticated user's ID directly from the server-side session context using `getCurrentUserIdOnServer`. This makes the action more secure and independent of the client's state. 2. **Updated Component Calls:** All components that previously called `getChat(id, userId)`, such as `Chat.tsx` and the search page, have been updated to call the function with only the chat `id`, i.e., `getChat(id)`. This change ensures that data fetching is secure and functional in both local development (with a mock user) and the production environment, resolving the deployment failure.
|
@coderabbitai review pr |
|
@ngoiyaeric: I'll review the changes in this pull request right away. ✅ Actions performedReview triggered.
|
|
@ngoiyaeric: I'll review this PR right away. This appears to be a significant refactoring that migrates from Drizzle ORM to Supabase and integrates collaboration features with the settings UI. I'll analyze all the changes across the 27 files. ✅ Actions performedReview triggered.
|
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: 11
♻️ Duplicate comments (9)
components/chat.tsx (3)
91-102: Fix real-time handler to append UI messages correctly.
useUIStatestores entries shaped like{ id, component }, but here we push the raw Supabase row (payload.new) straight intomessages. That yields elements withoutcomponent, so the first realtime insert corrupts the UI state and nothing renders for remote messages. Map the row into your AI state (viauseAIState) and derive the UI entry the same waysubmit()does instead of splicing the raw payload intomessages.
84-112: Stop resubscribing the channel on every UI update.Including
messages/setMessagesin this effect’s dependency list tears the channel down and recreates it on every render, which drops realtime events and thrashes presence. Keep the dependency toidand rely on functional state updates inside the handlers to avoid stale closures.
98-106: Track the real user in presence updates.Calling
channel.trackwith the hard-coded'user-placeholder'causes every session to share the same presence key, so your online-user list is wrong and tabs stomp each other. Fetch the authenticated user id (e.g.const session = await supabase.auth.getSession()) and pass that instead.lib/actions/collaboration.ts (1)
27-35: Use an auth-aware lookup for invitee email.Querying
from('users')will fail in production—Supabase doesn’t expose apublic.userstable with emails, so this returns a 42P01/404 and invites always error out. Resolve the user id via a SECURITY DEFINER RPC againstauth.users(or another RLS-safe view) before inserting intochat_participants.components/settings/components/settings.tsx (1)
67-90: Hard-codeduserId = 'anonymous'causes cross-user data leaks.Persisting the system prompt under a fixed
"anonymous"key means every user reads/writes the same record—whoever saves last overwrites everyone else, and prompts bleed between accounts. Please resolve by fetching the real user id server-side in the action (or at least threading the actual id into this component).app/search/[id]/page.tsx (1)
37-44: Map Supabase rows toAIMessagebefore casting.
getChatMessagesstill returns raw Supabase rows (snake_case fields likecreated_at, joined payloads, etc.). Casting them toAIMessage[]without normalization leavescreatedAt/locationsundefined at runtime, breaking chronological ordering and geo rendering. Please restore an explicit mapper (id, role, content, createdAt, locations, …) before handing the data to<AI>.jules-scratch/verification/verify_share_button.py (1)
17-27: Verification script targets removed Share dialog.This flow now lives in Settings—the Share button/dialog got removed with
ChatShareDialog, so the locator never resolves and the run hangs. Please update the verification to cover the new collaboration UI instead of the old dialog.components/settings/components/user-management-form.tsx (1)
45-57: Role selection is ignored byinviteUserToChat.We still surface an Owner/Collaborator selector, but
inviteUserToChat(chatId, email)drops that value—every invite is stored as the default role. That’s misleading UX and wrong data when owners try to elevate someone. Please either remove the selector or plumb the chosen role through to the action + persistence.Also applies to: 91-107
lib/actions/chat.ts (1)
95-96: Redirect from a shared action still breaks API callers
clearChatsstill unconditionallyredirect('/'). When API routes (e.g.app/api/chats) call this helper, the redirect short-circuits the JSON response. Please make the redirect optional so API contexts can return normally.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (28)
app/actions.tsx(4 hunks)app/api/chat/route.ts(2 hunks)app/api/chats/all/route.ts(2 hunks)app/api/chats/route.ts(2 hunks)app/search/[id]/page.tsx(1 hunks)components/chat-panel.tsx(2 hunks)components/chat.tsx(5 hunks)components/history-item.tsx(1 hunks)components/history-list.tsx(2 hunks)components/settings/components/settings.tsx(4 hunks)components/settings/components/user-management-form.tsx(6 hunks)components/settings/settings-view.tsx(2 hunks)components/sidebar/chat-history-client.tsx(5 hunks)drizzle.config.ts(0 hunks)jules-scratch/verification/verify_chat_creation.py(1 hunks)jules-scratch/verification/verify_share_button.py(1 hunks)lib/actions/chat-db.ts(0 hunks)lib/actions/chat.ts(3 hunks)lib/actions/collaboration.ts(1 hunks)lib/actions/rag.ts(1 hunks)lib/actions/users.ts(0 hunks)lib/db/index.ts(0 hunks)lib/db/schema.ts(0 hunks)lib/supabase/browser-client.ts(1 hunks)lib/supabase/client.ts(1 hunks)lib/supabase/persistence.ts(1 hunks)supabase/migrations/0000_init.sql(1 hunks)supabase/migrations/0001_realtime_collaboration.sql(1 hunks)
💤 Files with no reviewable changes (5)
- lib/actions/chat-db.ts
- drizzle.config.ts
- lib/actions/users.ts
- lib/db/schema.ts
- lib/db/index.ts
🧰 Additional context used
🪛 Biome (2.1.2)
components/sidebar/chat-history-client.tsx
[error] 21-23: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 25-25: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
app/actions.tsx
[error] 574-574: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (8)
app/actions.tsx (2)
288-291: Context augmentation logic is sound.RAG context is properly retrieved and prepended to the system prompt when available, with a sensible fallback.
584-589: Verifymessage.locationsdata structure. Automated search didn’t locate its definition or assignment—confirm upstream code populates eachlocationwith ageojsonproperty before rendering.components/sidebar/chat-history-client.tsx (1)
38-44: LGTM! Clean migration to simplified API.The component correctly fetches chats from the new non-paginated
/api/chatsendpoint and handles the response structure properly.components/history-item.tsx (1)
6-10: LGTM! Clean type migration.The component correctly migrates from the internal
DrizzleChattype to the publicChattype, aligning with the broader architectural shift.components/history-list.tsx (2)
5-5: LGTM! Well-executed type migration.The component properly migrates to the public
Chattype and correctly constructs the chat path as/search/${chat.id}, which aligns with the routing pattern used throughout the application.Also applies to: 11-12, 37-43
53-62: Good: Added error handling.The try/catch block properly handles errors during chat loading and displays a user-friendly error message, improving the component's resilience.
components/settings/settings-view.tsx (1)
8-8: Handle empty-string fallback for chatId in SettingsView
In components/chat.tsx (lines 121 & 165), SettingsView is rendered withchatId={id || ''}, which will pass''whenidis falsy. Ensure an empty string is acceptable as a chatId or validate/guardidbefore rendering to avoid unintended behavior.app/api/chats/all/route.ts (1)
12-17: Dismiss error-handling warning.clearChatsis typed to return either{ error?: string }on failure orvoidon success, soif (result && 'error' in result)safely covers all cases.Likely an incorrect or invalid review comment.
| ) { | ||
| const { fullResponse, hasError, toolResponses } = await researcher( | ||
| currentSystemPrompt, | ||
| augmentedSystemPrompt, |
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.
Critical: Inconsistent system prompt usage breaks RAG integration.
Line 329 correctly uses augmentedSystemPrompt in the researcher call, but line 373 uses currentSystemPrompt in the writer call. This means the writer agent won't receive the retrieved context, breaking the RAG workflow for the useSpecificAPI path.
Apply this diff to fix the inconsistency:
const latestMessages = modifiedMessages.slice(maxMessages * -1)
answer = await writer(
- currentSystemPrompt,
+ augmentedSystemPrompt,
uiStream,
streamText,
latestMessages
)Also applies to: 372-377
🤖 Prompt for AI Agents
In app/actions.tsx around lines 329 and 372-377, the researcher call correctly
passes augmentedSystemPrompt but the writer call uses currentSystemPrompt, which
prevents the retrieved RAG context from being passed to the writer; update the
writer agent invocation(s) in that useSpecificAPI code path to pass
augmentedSystemPrompt (same variable used for the researcher) instead of
currentSystemPrompt so both researcher and writer receive the
retrieved/context-augmented system prompt. Ensure any other writer calls in that
block are similarly updated to use augmentedSystemPrompt.
| const location = (message as any).locations | ||
| return { | ||
| id, | ||
| component: ( | ||
| <UserMessage | ||
| content={messageContent} | ||
| chatId={chatId} | ||
| showShare={index === 0 && !isSharePage} | ||
| /> | ||
| <> | ||
| <UserMessage | ||
| content={messageContent} | ||
| chatId={chatId} | ||
| showShare={index === 0 && !isSharePage} | ||
| /> | ||
| {location && ( | ||
| <GeoJsonLayer | ||
| id={`${id}-geojson`} | ||
| data={location.geojson} | ||
| /> | ||
| )} | ||
| </> |
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.
Fix: Scope the location variable to prevent switch case leakage.
The location variable declared in line 574 can be accessed by other switch cases, which is error-prone. The static analysis tool correctly flags this.
Apply this diff to wrap the declaration in a block:
case 'user':
switch (type) {
case 'input':
- case 'input_related':
+ case 'input_related': {
let messageContent: string | any[]
try {
// For backward compatibility with old messages that stored a JSON string
const json = JSON.parse(content as string)
messageContent =
type === 'input' ? json.input : json.related_query
} catch (e) {
// New messages will store the content array or string directly
messageContent = content
}
const location = (message as any).locations
return {
id,
component: (
<>
<UserMessage
content={messageContent}
chatId={chatId}
showShare={index === 0 && !isSharePage}
/>
{location && (
<GeoJsonLayer
id={`${id}-geojson`}
data={location.geojson}
/>
)}
</>
)
}
+ }
case 'inquiry':📝 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 location = (message as any).locations | |
| return { | |
| id, | |
| component: ( | |
| <UserMessage | |
| content={messageContent} | |
| chatId={chatId} | |
| showShare={index === 0 && !isSharePage} | |
| /> | |
| <> | |
| <UserMessage | |
| content={messageContent} | |
| chatId={chatId} | |
| showShare={index === 0 && !isSharePage} | |
| /> | |
| {location && ( | |
| <GeoJsonLayer | |
| id={`${id}-geojson`} | |
| data={location.geojson} | |
| /> | |
| )} | |
| </> | |
| case 'user': | |
| switch (type) { | |
| case 'input': | |
| case 'input_related': { | |
| let messageContent: string | any[] | |
| try { | |
| // For backward compatibility with old messages that stored a JSON string | |
| const json = JSON.parse(content as string) | |
| messageContent = | |
| type === 'input' ? json.input : json.related_query | |
| } catch (e) { | |
| // New messages will store the content array or string directly | |
| messageContent = content | |
| } | |
| const location = (message as any).locations | |
| return { | |
| id, | |
| component: ( | |
| <> | |
| <UserMessage | |
| content={messageContent} | |
| chatId={chatId} | |
| showShare={index === 0 && !isSharePage} | |
| /> | |
| {location && ( | |
| <GeoJsonLayer | |
| id={`${id}-geojson`} | |
| data={location.geojson} | |
| /> | |
| )} | |
| </> | |
| ) | |
| } | |
| } | |
| case 'inquiry': | |
| // … | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 574-574: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In app/actions.tsx around lines 574 to 590, the variable "location" is declared
at function/switch scope causing leakage into other switch cases; wrap the JSX
branch that uses it in its own block (e.g., surround the code that declares and
uses location with { /* block */ { const location = (message as any).locations;
return { ... } } } ) so the "location" const is lexically scoped to that branch
only, preventing accidental access from other cases.
| const chats = await getChats(userId); | ||
| return NextResponse.json({ chats }); |
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.
🧩 Analysis chain
Potential scalability issue: Removed pagination.
The endpoint now returns all chats at once instead of supporting pagination. For users with many chats (hundreds or thousands), this could cause:
- Increased memory consumption on the server
- Slow response times
- Large payload sizes affecting client performance
Verify that this is intentional and that there are practical limits on the number of chats per user, or consider re-introducing pagination or implementing a reasonable limit.
Check if there are database-level or application-level limits on chat count:
🏁 Script executed:
#!/bin/bash
# Description: Check for chat limits in persistence layer and schema
# Search for limits in persistence functions
rg -nP 'limit|LIMIT' lib/supabase/persistence.ts lib/actions/chat.ts -C 2
# Search for RLS policies or constraints in migrations
fd -e sql . -x cat {} \; | grep -i "limit\|check\|constraint" -C 2Length of output: 2718
🏁 Script executed:
#!/bin/bash
# Description: Locate and inspect the getChats implementation to see if it includes any implicit limits or pagination
rg -nP 'getChats' -n -C10 lib/actions/chat.ts || rg -nP 'getChats' -n -C10 lib/supabase/persistence.tsLength of output: 905
Missing user filter and pagination in getChats
- In lib/actions/chat.ts getChats():
.from('chats').select('*')ignores theuserIdparameter, returning all chats for all users. Add.eq('user_id', userId)(and fix the shadoweduserIdbinding). - No pagination or limit is applied—consider using
.range()or.limit()to prevent excessive data loads.
🤖 Prompt for AI Agents
In app/api/chats/route.ts around lines 12-13, the handler calls getChats(userId)
but the underlying getChats implementation currently queries
`.from('chats').select('*')` which ignores the userId, may shadow the parameter,
and returns unbounded results; update getChats to use the provided userId
(remove any shadowing) and add a filter `.eq('user_id', userId)` plus a
pagination/limit call such as `.range(start, end)` or `.limit(n)` (and
validate/normalize start/end or limit inputs) to prevent returning all chats for
all users.
| interface ChatHistoryClientProps { | ||
| // userId is no longer passed as prop; API route will use authenticated user | ||
| } | ||
|
|
||
| export function ChatHistoryClient({}: ChatHistoryClientProps) { |
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.
🧹 Nitpick | 🔵 Trivial
Optional: Simplify empty props declaration.
The empty ChatHistoryClientProps interface and empty object destructuring are unnecessary.
Apply this diff to simplify:
-interface ChatHistoryClientProps {
-}
-
-export function ChatHistoryClient({}: ChatHistoryClientProps) {
+export function ChatHistoryClient() {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| interface ChatHistoryClientProps { | |
| // userId is no longer passed as prop; API route will use authenticated user | |
| } | |
| export function ChatHistoryClient({}: ChatHistoryClientProps) { | |
| export function ChatHistoryClient() { |
🧰 Tools
🪛 Biome (2.1.2)
[error] 25-25: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🤖 Prompt for AI Agents
In components/sidebar/chat-history-client.tsx around lines 22 to 25, the empty
ChatHistoryClientProps interface and the empty destructured parameter are
unnecessary; remove the interface and change the component to accept no props
(i.e., use function ChatHistoryClient() { ... }) or, if you prefer explicit
typing, type the function as React.FC without a props interface; update
imports/types if needed and delete the unused interface declaration and the
empty object destructuring from the function signature.
| return [] | ||
| } | ||
|
|
||
| return (data as Chat[]) || [] |
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.
Normalize Supabase rows before returning Chat objects
getChats/getChat return the raw Supabase payload cast to Chat, but the payload is still snake_case (user_id, created_at, shareable_link_id, nested chat_participants, etc.). Anywhere that expects the camelCase Chat shape (e.g., this module’s own saveChat check on chat.userId) will now see undefined and break downstream logic. Please convert the rows to the Chat contract before returning them.
Also applies to: 58-60
🤖 Prompt for AI Agents
In lib/actions/chat.ts around line 36 (and similarly at 58-60), the function
currently returns raw Supabase rows cast to Chat but those rows are snake_case
(user_id, created_at, shareable_link_id, chat_participants, etc.); map each
returned row into the Chat contract by converting snake_case keys to camelCase
(user_id -> userId, created_at -> createdAt, shareable_link_id ->
shareableLinkId), transform nested chat_participants into chatParticipants with
their camelCase fields, and preserve/convert types (e.g., createdAt to
string/Date if your Chat type expects that); perform null/undefined checks and
return the mapped array (or single mapped object) instead of the raw payload so
downstream code like saveChat sees chat.userId and other camelCase properties.
| export function getSupabaseBrowserClient() { | ||
| return createBrowserClient( | ||
| process.env.NEXT_PUBLIC_SUPABASE_URL!, | ||
| process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY! | ||
| ) |
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.
Critical: Non-null assertions on environment variables.
Lines 7-8 use non-null assertions (!) on process.env.NEXT_PUBLIC_SUPABASE_URL and process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY. If these environment variables are missing or undefined, the code will throw at runtime with no graceful error handling.
Apply this diff to add validation with a helpful error message:
export function getSupabaseBrowserClient() {
+ const url = process.env.NEXT_PUBLIC_SUPABASE_URL
+ const key = process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY
+
+ if (!url || !key) {
+ throw new Error(
+ 'Missing Supabase environment variables. Please ensure NEXT_PUBLIC_SUPABASE_URL and NEXT_PUBLIC_SUPABASE_ANON_KEY are set.'
+ )
+ }
+
return createBrowserClient(
- process.env.NEXT_PUBLIC_SUPABASE_URL!,
- process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!
+ url,
+ key
)
}📝 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 getSupabaseBrowserClient() { | |
| return createBrowserClient( | |
| process.env.NEXT_PUBLIC_SUPABASE_URL!, | |
| process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY! | |
| ) | |
| export function getSupabaseBrowserClient() { | |
| const url = process.env.NEXT_PUBLIC_SUPABASE_URL | |
| const key = process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY | |
| if (!url || !key) { | |
| throw new Error( | |
| 'Missing Supabase environment variables. Please ensure NEXT_PUBLIC_SUPABASE_URL and NEXT_PUBLIC_SUPABASE_ANON_KEY are set.' | |
| ) | |
| } | |
| return createBrowserClient( | |
| url, | |
| key | |
| ) | |
| } |
🤖 Prompt for AI Agents
In lib/supabase/browser-client.ts around lines 5 to 9, replace the non-null
assertions on process.env.NEXT_PUBLIC_SUPABASE_URL and
process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY with explicit validation: read each
env var into a const, check if it's falsy, and if so throw a clear Error that
names the missing variable and gives a helpful hint (e.g., "Ensure
NEXT_PUBLIC_SUPABASE_URL is set in your environment or .env.local"), then pass
the validated strings into createBrowserClient so runtime failures are replaced
by an informative startup error.
| return createServerClient( | ||
| process.env.NEXT_PUBLIC_SUPABASE_URL!, | ||
| process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!, | ||
| { | ||
| cookies: { | ||
| async get(name: string) { | ||
| const store = await cookieStore | ||
| return store.get(name)?.value | ||
| }, | ||
| async set(name: string, value: string, options: CookieOptions) { | ||
| const store = await cookieStore | ||
| store.set({ name, value, ...options }) | ||
| }, | ||
| async remove(name: string, options: CookieOptions) { | ||
| const store = await cookieStore | ||
| store.set({ name, value: '', ...options }) | ||
| }, | ||
| }, | ||
| } | ||
| ) |
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.
Cookie handlers must be synchronous (do not return Promises).
createServerClient expects cookies.get/set/remove to return synchronously; giving it async handlers hands Supabase a Promise instead of the cookie string, so auth/session parsing breaks. Also, removal should actually delete (e.g., cookieStore.delete). Please drop the async/await, use cookieStore directly, and call delete/set synchronously.
- async get(name: string) {
- const store = await cookieStore
- return store.get(name)?.value
- },
+ get(name: string) {
+ return cookieStore.get(name)?.value
+ },
- async set(name: string, value: string, options: CookieOptions) {
- const store = await cookieStore
- store.set({ name, value, ...options })
- },
+ set(name: string, value: string, options: CookieOptions) {
+ cookieStore.set({ name, value, ...options })
+ },
- async remove(name: string, options: CookieOptions) {
- const store = await cookieStore
- store.set({ name, value: '', ...options })
- },
+ remove(name: string, options: CookieOptions) {
+ cookieStore.delete?.(name, options) ?? cookieStore.set({ name, value: '', maxAge: 0, ...options })
+ },📝 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.
| return createServerClient( | |
| process.env.NEXT_PUBLIC_SUPABASE_URL!, | |
| process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!, | |
| { | |
| cookies: { | |
| async get(name: string) { | |
| const store = await cookieStore | |
| return store.get(name)?.value | |
| }, | |
| async set(name: string, value: string, options: CookieOptions) { | |
| const store = await cookieStore | |
| store.set({ name, value, ...options }) | |
| }, | |
| async remove(name: string, options: CookieOptions) { | |
| const store = await cookieStore | |
| store.set({ name, value: '', ...options }) | |
| }, | |
| }, | |
| } | |
| ) | |
| return createServerClient( | |
| process.env.NEXT_PUBLIC_SUPABASE_URL!, | |
| process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!, | |
| { | |
| cookies: { | |
| get(name: string) { | |
| return cookieStore.get(name)?.value | |
| }, | |
| set(name: string, value: string, options: CookieOptions) { | |
| cookieStore.set({ name, value, ...options }) | |
| }, | |
| remove(name: string, options: CookieOptions) { | |
| cookieStore.delete?.(name, options) | |
| ?? cookieStore.set({ name, value: '', maxAge: 0, ...options }) | |
| }, | |
| }, | |
| } | |
| ) |
🤖 Prompt for AI Agents
In lib/supabase/client.ts around lines 7 to 26, the cookie handlers are declared
async and await cookieStore, returning Promises and breaking Supabase's
synchronous API; also remove currently sets an empty value instead of deleting.
Fix by removing async/await from get/set/remove, access cookieStore directly
(synchronously) inside each handler, have get return
cookieStore.get(name)?.value, set call cookieStore.set({ name, value, ...options
}), and remove call cookieStore.delete(name, options) (or equivalent synchronous
delete API) so all handlers are synchronous and removal actually deletes the
cookie.
| export async function getMessagesByChatId(chatId: string): Promise<{ data: any[] | null; error: PostgrestError | null }> { | ||
| const supabase = getSupabaseServerClient() | ||
| const { data, error } = await supabase | ||
| .from('messages') | ||
| .select('*, locations(*)') | ||
| .eq('chat_id', chatId) | ||
| .order('created_at', { ascending: true }) | ||
|
|
||
| if (error) { | ||
| console.error('Error fetching messages:', error) | ||
| return { data: null, error } | ||
| } | ||
|
|
||
| return { data: data, error: null } | ||
| } |
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.
Adapt Supabase message rows to AIMessage before returning
Both getMessagesByChatId and createMessage hand back the raw Supabase rows but advertise AIMessage. Those rows contain created_at/location_id etc., so the caller will never see createdAt/locationId. Normalize (or at least document) before returning so the contract stays truthful.
Also applies to: 102-115
🤖 Prompt for AI Agents
In lib/supabase/persistence.ts around lines 31 to 45 (and also apply the same
change to lines 102-115), the function returns raw Supabase rows but advertises
AIMessage; transform each row before returning by mapping snake_case DB fields
to the AIMessage shape (e.g., created_at -> createdAt, location_id ->
locationId) and normalize nested relations (locations -> location) as needed so
the returned data matches the declared AIMessage interface; adjust the function
return type if necessary and ensure both getMessagesByChatId and createMessage
perform this mapping and return the normalized objects instead of raw rows.
| INSERT INTO public.chat_participants (chat_id, user_id, role) | ||
| VALUES (new_chat_id, user_id, 'owner'); | ||
|
|
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.
Create chat_participants before referencing it in the RPC
save_chat_with_messages inserts into public.chat_participants, but this table is only introduced in migration 0001. Because PostgreSQL validates relations when compiling the function, 0000 will fail with “relation public.chat_participants does not exist.” Move the table creation (and its policies) into 0000 or defer this function to the later migration.
🤖 Prompt for AI Agents
In supabase/migrations/0000_init.sql around lines 139 to 141, the RPC function
save_chat_with_messages inserts into public.chat_participants but that table is
created in migration 0001, causing compilation failure; either move the CREATE
TABLE (and its RLS policies/indexes) for public.chat_participants into this 0000
migration before defining the RPC, or remove/replace the INSERT in the RPC and
defer creating the RPC to 0001. Ensure the table definition and any required
policies exist earlier in the same file so the function compiles successfully.
| CREATE POLICY "Users can select chats they are a part of" ON public.chats | ||
| FOR SELECT USING ( | ||
| EXISTS ( | ||
| SELECT 1 FROM public.chat_participants | ||
| WHERE chat_participants.chat_id = chats.id AND chat_participants.user_id = auth.uid() | ||
| ) | ||
| ); | ||
|
|
||
| CREATE POLICY "Chat owners can update and delete their chats" ON public.chats | ||
| FOR UPDATE, DELETE USING ( | ||
| (SELECT role FROM public.chat_participants WHERE chat_id = chats.id AND user_id = auth.uid()) = 'owner' | ||
| ); |
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.
Reintroduce an INSERT policy for public.chats
After dropping the old “manage their own chats” policy you only recreated SELECT and UPDATE/DELETE rules. With RLS enabled, lack of an INSERT policy blocks any user session from creating chats, so save_chat_with_messages will always fail. Add an explicit FOR INSERT ... WITH CHECK (auth.uid() = user_id) (or equivalent) so owners can still create chats.
🤖 Prompt for AI Agents
In supabase/migrations/0001_realtime_collaboration.sql around lines 38 to 49,
RLS currently defines SELECT and UPDATE/DELETE policies but no INSERT policy,
which prevents authenticated users from creating chats; add an explicit FOR
INSERT ... WITH CHECK policy that permits creation only when auth.uid() equals
the chat owner (for example WITH CHECK (auth.uid() = user_id)) or an equivalent
condition that ensures the inserting user becomes/ is the owner, so
save_chat_with_messages can insert new chats successfully.
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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
components/chat-panel.tsx (1)
81-86: Align accepted file types with handling; avoid object URL leaks.
- You accept
text/plainbut only render images; either handle text files or removetext/plainfrom accept.URL.createObjectURL(selectedFile)is never revoked; consider storing the URL and revoking it on clear/submit.- accept="text/plain,image/png,image/jpeg,image/webp" + accept="image/png,image/jpeg,image/webp"Also applies to: 176-182
app/actions.tsx (2)
329-335: Use the augmented system prompt in the writer fallback too.RAG context is applied to
researcherviaaugmentedSystemPromptbut thewriterpath still usescurrentSystemPrompt, dropping context.- const { fullResponse, hasError, toolResponses } = await researcher( - augmentedSystemPrompt, + const { fullResponse, hasError, toolResponses } = await researcher( + augmentedSystemPrompt, uiStream, streamText, messages, useSpecificAPI ) ... - answer = await writer( - currentSystemPrompt, + answer = await writer( + augmentedSystemPrompt, uiStream, streamText, latestMessages )Also applies to: 372-377
559-653: Wrapswitchcase declarations in blocks to satisfy lints and prevent leakage across cases.Static analysis flagged declarations inside cases. Wrap each case body with
{}.- case 'assistant': - const answer = createStreamableValue() + case 'assistant': { + const answer = createStreamableValue() answer.done(content) switch (type) { - case 'response': + case 'response': { return { id, component: ( <Section title="response"> <BotMessage content={answer.value} /> </Section> ) } - case 'related': + } + case 'related': { const relatedQueries = createStreamableValue<RelatedQueries>() relatedQueries.done(JSON.parse(content as string)) return { id, component: ( <Section title="Related" separator={true}> <SearchRelated relatedQueries={relatedQueries.value} /> </Section> ) } - case 'followup': + } + case 'followup': { return { id, component: ( <Section title="Follow-up" className="pb-8"> <FollowupPanel /> </Section> ) } - case 'resolution_search_result': { + } + case 'resolution_search_result': { const analysisResult = JSON.parse(content as string); const summaryValue = createStreamableValue(); summaryValue.done(analysisResult.summary); const geoJson = analysisResult.geoJson as FeatureCollection; return { id, component: ( <> <Section title="Map Analysis"> <BotMessage content={summaryValue.value} /> </Section> {geoJson && ( <GeoJsonLayer id={id} data={geoJson} /> )} </> ) } } } - break + break + }This addresses the linter hint: “Other switch clauses can erroneously access this declaration.” As per coding guidelines
♻️ Duplicate comments (8)
components/settings/components/settings.tsx (1)
67-69: Security issue: Hard-coded user ID overwrites settings across all users.As noted in the previous review, using
userId = 'anonymous'means all users write to the same record, overwriting each other's system prompts.jules-scratch/verification/verify_share_button.py (1)
17-26: The script targets removed UI elements.As noted in the previous review, this script verifies the Share button and dialog, but the PR removes
ChatShareDialog. The script will fail because the targeted UI no longer exists.app/api/chats/all/route.ts (1)
12-18: EnsureclearChatsdoes not trigger a redirect in this API context.If
clearChatsperforms a redirect, this route may return a redirect payload instead of JSON. Ensure it returns control without redirect (e.g., an option like{ redirectTo: null }) and keep sending 200 JSON here.app/search/[id]/page.tsx (1)
31-44: Map DB rows to AIMessage; don’t castany[]toAIMessage[].Current code passes
any[]asAIMessage[], leading to runtime shape mismatches (snake_case to camelCase). Map explicitly.- const initialMessages = await getChatMessages(chat.id); + const raw = await getChatMessages(chat.id); + const initialMessages: AIMessage[] = raw.map((m: any) => ({ + id: m.id, + role: m.role, + content: m.content, + createdAt: m.created_at ? new Date(m.created_at) : undefined, + locations: m.locations, + })); ... - messages: initialMessages as AIMessage[], + messages: initialMessages,components/settings/components/user-management-form.tsx (1)
91-111: Role selector is dead state; action ignores it. Align UI with behavior.The form collects
newUserRolebutinviteUserToChat(chatId, email)always insertscollaborator. This confuses users.Options:
- Prefer: Plumb the role through to the action (and update action signature).
- Or: Remove the role selector until roles are supported.
Quick removal (if not plumbing now):
- <FormField - control={form.control} - name="newUserRole" - defaultValue="collaborator" - render={({ field }) => ( - <FormItem> - <FormLabel>Role</FormLabel> - <Select onValueChange={field.onChange} defaultValue={field.value}> - <FormControl> - <SelectTrigger> - <SelectValue placeholder="Select a role" /> - </SelectTrigger> - </FormControl> - <SelectContent> - <SelectItem value="owner">Owner</SelectItem> - <SelectItem value="collaborator">Collaborator</SelectItem> - </SelectContent> - </Select> - <FormMessage /> - </FormItem> - )} - /> + {/* Role selection removed until server action supports roles */}Also guard against empty
chatIdbefore calling the action, to avoid misleading errors.Also applies to: 45-57
components/chat.tsx (2)
41-49: VerifygetChatdoesn’t early-return whenuserIdis missing.You’re calling
getChat(id)without a user id. IfgetChatstill guards on!userId, you’ll always getnull.Run this to confirm and locate any guard you need to remove:
#!/bin/bash # Find getChat and check for early-return on missing userId rg -nP -C3 'export\s+async\s+function\s+getChat\s*\(' rg -nP -C2 'getChat\s*\(.*userId' || true rg -nP -C3 "(?s)export\\s+async\\s+function\\s+getChat\\b.*?\\{.*?if\\s*\\(\\s*!userId\\s*\\)"
84-112: Fix realtime handler: don’t push DB rows into UI state; update AI state and avoid resubscribe churn.
messagesfromuseUIStateis UI nodes, not AI messages; appending DB rows will corrupt UI state.- Effect depends on
messages/setMessages, tearing down/recreating the channel on every UI change.Apply these changes:
- Use
useAIStatesetter to append to AI messages.- Remove
messages/setMessagesfrom deps.- Use functional updates to avoid stale closures.
- const [messages, setMessages] = useUIState() - const [aiState] = useAIState() + const [messages, setMessages] = useUIState() + const [aiState, setAIState] = useAIState() ... - useEffect(() => { + useEffect(() => { if (!id) return; const supabase = getSupabaseBrowserClient(); const channel = supabase.channel(`chat-${id}`); - const subscription = channel + const subscription = channel .on('postgres_changes', { event: 'INSERT', schema: 'public', table: 'messages', filter: `chat_id=eq.${id}` }, (payload) => { - const newMessage = payload.new as AIMessage; - if (!messages.some((m: AIMessage) => m.id === newMessage.id)) { - setMessages((prevMessages: AIMessage[]) => [...prevMessages, newMessage]); - } + const m: any = payload.new + const next: AIMessage = { id: m.id, role: m.role, content: m.content, createdAt: new Date(m.created_at) } + setAIState(prev => ({ + ...prev, + messages: prev.messages.some(mm => mm.id === next.id) + ? prev.messages + : [...prev.messages, next] + })) }) .on('presence', { event: 'sync' }, () => { const newState = channel.presenceState(); const users = Object.keys(newState).map(key => (newState[key][0] as any).user_id); setOnlineUsers(users); }) .subscribe(async (status) => { if (status === 'SUBSCRIBED') { - await channel.track({ user_id: 'user-placeholder', online_at: new Date().toISOString() }); + const { data } = await supabase.auth.getSession() + await channel.track({ + user_id: data.session?.user?.id ?? 'anonymous', + online_at: new Date().toISOString() + }) } }); return () => { supabase.removeChannel(channel); }; - }, [id, messages, setMessages]); + }, [id]); // only depends on chat idlib/actions/collaboration.ts (1)
26-35: Queryingpublic.usersby email will fail; useauth.usersvia SECURITY DEFINER RPC.Supabase user emails live in
auth.users. Queryingpublic.usersbyApply this change to the query section:
- // Get the user ID of the person being invited - const { data: userData, error: userError } = await supabase - .from('users') - .select('id') - .eq('email', email) - .single() + // Lookup user id via SECURITY DEFINER RPC that reads from auth.users + const { data: userId, error: userError } = await supabase + .rpc('get_user_id_by_email', { p_email: email.toLowerCase().trim() })And update the insert to use
userId:- .insert({ chat_id: chatId, user_id: userData.id, role: 'collaborator' }) + .insert({ chat_id: chatId, user_id: userId, role: 'collaborator' })Optionally accept role from caller:
-export async function inviteUserToChat(chatId: string, email: string): Promise<{ error?: string }> { +export async function inviteUserToChat( + chatId: string, + email: string, + role: 'owner' | 'collaborator' = 'collaborator' +): Promise<{ error?: string }> { ... - .insert({ chat_id: chatId, user_id: userId, role: 'collaborator' }) + .insert({ chat_id: chatId, user_id: userId, role })Ensure the RPC exists with SECURITY DEFINER and proper grants (as suggested earlier). Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (28)
app/actions.tsx(4 hunks)app/api/chat/route.ts(2 hunks)app/api/chats/all/route.ts(2 hunks)app/api/chats/route.ts(2 hunks)app/search/[id]/page.tsx(1 hunks)components/chat-panel.tsx(2 hunks)components/chat.tsx(5 hunks)components/history-item.tsx(1 hunks)components/history-list.tsx(2 hunks)components/settings/components/settings.tsx(4 hunks)components/settings/components/user-management-form.tsx(6 hunks)components/settings/settings-view.tsx(2 hunks)components/sidebar/chat-history-client.tsx(5 hunks)drizzle.config.ts(0 hunks)jules-scratch/verification/verify_chat_creation.py(1 hunks)jules-scratch/verification/verify_share_button.py(1 hunks)lib/actions/chat-db.ts(0 hunks)lib/actions/chat.ts(3 hunks)lib/actions/collaboration.ts(1 hunks)lib/actions/rag.ts(1 hunks)lib/actions/users.ts(0 hunks)lib/db/index.ts(0 hunks)lib/db/schema.ts(0 hunks)lib/supabase/browser-client.ts(1 hunks)lib/supabase/client.ts(1 hunks)lib/supabase/persistence.ts(1 hunks)supabase/migrations/0000_init.sql(1 hunks)supabase/migrations/0001_realtime_collaboration.sql(1 hunks)
💤 Files with no reviewable changes (5)
- lib/actions/chat-db.ts
- lib/db/index.ts
- lib/actions/users.ts
- drizzle.config.ts
- lib/db/schema.ts
🧰 Additional context used
🪛 Biome (2.1.2)
app/actions.tsx
[error] 574-574: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
components/sidebar/chat-history-client.tsx
[error] 21-23: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 25-25: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🔇 Additional comments (5)
components/history-item.tsx (1)
6-6: LGTM: Clean type migration.The import change from
DrizzleChattoChatand the corresponding type update align with the broader migration from Drizzle to Supabase. The change is straightforward and maintains type safety.Also applies to: 10-10
components/history-list.tsx (1)
5-5: LGTM: Improved type safety and error handling.The changes include several improvements:
- Migration to the new
Chattype for better consistency across the codebase- Explicit path construction (
/search/${chat.id}) is clearer and more maintainable- Added try/catch error handling provides better user experience when chat loading fails
These changes align well with the broader Supabase migration.
Also applies to: 11-13, 42-42, 53-61
components/settings/components/settings.tsx (1)
36-36: Ensure no legacy role values remain in the database
Run a real query against your Supabase instance (e.g. viasupabase db queryor direct psql) to find any records inchat_participants(or other user tables) whererole NOT IN ('owner','collaborator'). If any rows are returned, add a migration to update old values before deploying.components/settings/settings-view.tsx (1)
8-8: All callers pass thechatIdprop – both usages incomponents/chat.tsxsupplychatId={id || ''}, satisfying the required string prop.components/chat-panel.tsx (1)
108-112: VerifyclearChatAPI usage.If
clearChatoperates per chat, it may requirechatId. Confirm the action signature and pass the id if needed.
| const retrievedContext = await retrieveContext(userInput, aiState.get().chatId) | ||
| const augmentedSystemPrompt = retrievedContext.length > 0 | ||
| ? `Context: ${retrievedContext.join('\n')}\n${currentSystemPrompt}` | ||
| : currentSystemPrompt | ||
|
|
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.
Guard retrieveContext when userInput is empty/undefined.
userInput can be falsy (e.g., file-only input or skip). Calling retrieveContext with undefined may throw or degrade results.
- const retrievedContext = await retrieveContext(userInput, aiState.get().chatId)
+ const retrievedContext = userInput
+ ? await retrieveContext(userInput, aiState.get().chatId)
+ : []📝 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 retrievedContext = await retrieveContext(userInput, aiState.get().chatId) | |
| const augmentedSystemPrompt = retrievedContext.length > 0 | |
| ? `Context: ${retrievedContext.join('\n')}\n${currentSystemPrompt}` | |
| : currentSystemPrompt | |
| const retrievedContext = userInput | |
| ? await retrieveContext(userInput, aiState.get().chatId) | |
| : [] | |
| const augmentedSystemPrompt = retrievedContext.length > 0 | |
| ? `Context: ${retrievedContext.join('\n')}\n${currentSystemPrompt}` | |
| : currentSystemPrompt |
🤖 Prompt for AI Agents
In app/actions.tsx around lines 288 to 292, guard the call to retrieveContext so
it is only invoked when userInput is non-empty: if userInput is falsy, set
retrievedContext to an empty array (or directly set augmentedSystemPrompt =
currentSystemPrompt) instead of calling retrieveContext; otherwise call
retrieveContext(userInput, aiState.get().chatId) and build augmentedSystemPrompt
as before. Ensure types reflect that retrievedContext may be an empty array and
avoid passing undefined into retrieveContext.
| const newChat: Chat = { | ||
| id: uuidv4(), | ||
| userId: userId, | ||
| title: title || 'New Chat', // Default title if not provided | ||
| // createdAt: new Date(), // Handled by defaultNow() in schema | ||
| visibility: 'private', // Default visibility | ||
| }; | ||
|
|
||
| // Use a transaction if creating chat and first message together | ||
| // For simplicity here, let's assume saveChat handles chat creation and returns ID, then we create a message. | ||
| // A more robust `saveChat` might create the chat and first message in one go. | ||
| // The `saveChat` in chat-db.ts is designed to handle this. | ||
|
|
||
| const firstMessage: Omit<NewMessage, 'chatId'> = { | ||
| // id: generateUUID(), // Drizzle schema now has defaultRandom for UUIDs | ||
| // chatId is omitted as it will be set by saveChat | ||
| userId: userId, | ||
| role: role as NewMessage['role'], // Ensure role type matches schema expectation | ||
| content: initialMessageContent, | ||
| // createdAt: new Date(), // Handled by defaultNow() in schema, not strictly needed here | ||
| title: title || 'New Chat', | ||
| createdAt: new Date(), | ||
| path: '', | ||
| messages: [ | ||
| { | ||
| id: uuidv4(), | ||
| role: role, | ||
| content: initialMessageContent, | ||
| createdAt: new Date(), | ||
| } | ||
| ] | ||
| }; |
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.
🧹 Nitpick | 🔵 Trivial
Initialize path to the search URL to avoid empty links.
path: '' can break shareable links downstream. Set it up-front.
- const newChat: Chat = {
+ const newChat: Chat = {
id: uuidv4(),
userId: userId,
title: title || 'New Chat',
createdAt: new Date(),
- path: '',
+ path: `/search/${/* chat id */ ''}`, // set after id is known
messages: [Or compute after the id is set:
- const newChat: Chat = {
- id: uuidv4(),
+ const chatId = uuidv4()
+ const newChat: Chat = {
+ id: chatId,
userId: userId,
title: title || 'New Chat',
createdAt: new Date(),
- path: '',
+ path: `/search/${chatId}`,
messages: [📝 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 newChat: Chat = { | |
| id: uuidv4(), | |
| userId: userId, | |
| title: title || 'New Chat', // Default title if not provided | |
| // createdAt: new Date(), // Handled by defaultNow() in schema | |
| visibility: 'private', // Default visibility | |
| }; | |
| // Use a transaction if creating chat and first message together | |
| // For simplicity here, let's assume saveChat handles chat creation and returns ID, then we create a message. | |
| // A more robust `saveChat` might create the chat and first message in one go. | |
| // The `saveChat` in chat-db.ts is designed to handle this. | |
| const firstMessage: Omit<NewMessage, 'chatId'> = { | |
| // id: generateUUID(), // Drizzle schema now has defaultRandom for UUIDs | |
| // chatId is omitted as it will be set by saveChat | |
| userId: userId, | |
| role: role as NewMessage['role'], // Ensure role type matches schema expectation | |
| content: initialMessageContent, | |
| // createdAt: new Date(), // Handled by defaultNow() in schema, not strictly needed here | |
| title: title || 'New Chat', | |
| createdAt: new Date(), | |
| path: '', | |
| messages: [ | |
| { | |
| id: uuidv4(), | |
| role: role, | |
| content: initialMessageContent, | |
| createdAt: new Date(), | |
| } | |
| ] | |
| }; | |
| const chatId = uuidv4(); | |
| const newChat: Chat = { | |
| id: chatId, | |
| userId: userId, | |
| title: title || 'New Chat', | |
| createdAt: new Date(), | |
| path: `/search/${chatId}`, | |
| messages: [ | |
| { | |
| id: uuidv4(), | |
| role: role, | |
| content: initialMessageContent, | |
| createdAt: new Date(), | |
| } | |
| ] | |
| }; |
🤖 Prompt for AI Agents
In app/api/chat/route.ts around lines 22 to 36, the new Chat object sets path:
'' which can create broken shareable links; after generating the chat id,
populate path with the proper search URL (for example using the generated id,
e.g. `/search/${id}`) or assign path after constructing the id (compute path =
`/search/${newChat.id}`) so the created chat always contains a valid shareable
URL.
| const chats = await getChats(userId); | ||
| return NextResponse.json({ chats }); |
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.
Consider reintroducing pagination for scalability.
Removing pagination means all chats for a user are loaded in a single request. For users with hundreds or thousands of chats, this could cause:
- Slow API response times
- High memory consumption on both server and client
- Poor user experience with large datasets
- Potential timeout errors
If the previous pagination was removed intentionally (e.g., because most users have few chats), consider adding it back with sensible defaults:
export async function GET(request: NextRequest) {
try {
const userId = await getCurrentUserIdOnServer();
if (!userId) {
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 });
}
+ const { searchParams } = new URL(request.url);
+ const limit = Math.min(parseInt(searchParams.get('limit') || '50'), 100);
+ const offset = Math.max(parseInt(searchParams.get('offset') || '0'), 0);
+
- const chats = await getChats(userId);
+ const chats = await getChats(userId, { limit, offset });
return NextResponse.json({ chats });
} catch (error) {
console.error('Error fetching chats:', error);
return NextResponse.json({ error: 'Internal Server Error fetching chats' }, { status: 500 });
}
}Alternatively, if pagination is truly unnecessary, add a comment explaining the rationale.
📝 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 chats = await getChats(userId); | |
| return NextResponse.json({ chats }); | |
| export async function GET(request: NextRequest) { | |
| try { | |
| const userId = await getCurrentUserIdOnServer(); | |
| if (!userId) { | |
| return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); | |
| } | |
| // Parse pagination parameters with sensible defaults and limits | |
| const { searchParams } = new URL(request.url); | |
| const limit = Math.min(parseInt(searchParams.get('limit') || '50', 10), 100); | |
| const offset = Math.max(parseInt(searchParams.get('offset') || '0', 10), 0); | |
| const chats = await getChats(userId, { limit, offset }); | |
| return NextResponse.json({ chats }); | |
| } catch (error) { | |
| console.error('Error fetching chats:', error); | |
| return NextResponse.json({ error: 'Internal Server Error fetching chats' }, { status: 500 }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/api/chats/route.ts around lines 12-13, the handler currently returns all
chats for a user which is not scalable; change the endpoint to accept pagination
params (e.g., page and pageSize or cursor) with sensible defaults (page=1,
pageSize=20), update getChats to accept limit/offset or cursor and return
paginated results plus metadata (total or hasMore, page, pageSize), and return
NextResponse.json with { chats, page, pageSize, total/hasMore }; if you
intentionally want no pagination, add a concise comment explaining why (e.g.,
bounded small dataset) instead of leaving it implicit.
| export interface SearchPageProps { | ||
| params: Promise<{ id: string }>; // Keep as is for now | ||
| params: Promise<{ id: string }>; | ||
| } | ||
|
|
||
| export async function generateMetadata({ params }: SearchPageProps) { | ||
| const { id } = await params; // Keep as is for now | ||
| // TODO: Metadata generation might need authenticated user if chats are private | ||
| // For now, assuming getChat can be called or it handles anon access for metadata appropriately | ||
| const userId = await getCurrentUserIdOnServer(); // Attempt to get user for metadata | ||
| const chat = await getChat(id, userId || 'anonymous'); // Pass userId or 'anonymous' if none | ||
| const { id } = await params; | ||
| const chat = await getChat(id); | ||
| return { |
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.
Fix Next.js params typing; remove unnecessary awaits.
params is not a Promise in Next.js route/page props.
-export interface SearchPageProps {
- params: Promise<{ id: string }>;
-}
+export interface SearchPageProps {
+ params: { id: string };
+}
export async function generateMetadata({ params }: SearchPageProps) {
- const { id } = await params;
+ const { id } = params;-export default async function SearchPage({ params }: SearchPageProps) {
- const { id } = await params;
+export default async function SearchPage({ params }: SearchPageProps) {
+ const { id } = params;Also applies to: 23-25
🤖 Prompt for AI Agents
In app/search/[id]/page.tsx around lines 11 to 18 (and similarly lines 23–25),
change the props typing to reflect Next.js passes params as a plain object (not
a Promise) by replacing "params: Promise<{ id: string }>" with "params: { id:
string }"; remove the unnecessary "await" when extracting id (use "const { id }
= params;" instead of "const { id } = await params;"); apply the same typing and
removal of awaits to the other function(s) on lines 23–25 that currently treat
params as a Promise.
| chatId: string | ||
| shareableLink: string | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
Remove or use newly added props.
chatId and shareableLink are not used. Either wire them where needed or drop to avoid dead props.
Also applies to: 25-25
🤖 Prompt for AI Agents
In components/chat-panel.tsx around lines 17-19 (and also line 25), the newly
added props chatId and shareableLink are declared but never used; either remove
them from the component props type/signature and from any call sites, or wire
them into the component where needed (e.g., pass chatId to data-fetching hooks
or event handlers and render or generate the shareableLink where required).
Update the component signature, any children or hooks that need these values,
and all callers to match the new prop shape to eliminate dead props and
TypeScript/prop warnings.
| return createServerClient( | ||
| process.env.NEXT_PUBLIC_SUPABASE_URL!, | ||
| process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!, | ||
| { |
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 | 🟠 Major
Add explicit env validation instead of non-null assertions.
Fail fast with a clear error if URL/KEY are missing.
- return createServerClient(
- process.env.NEXT_PUBLIC_SUPABASE_URL!,
- process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!,
+ const url = process.env.NEXT_PUBLIC_SUPABASE_URL
+ const anon = process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY
+ if (!url || !anon) {
+ throw new Error('Missing NEXT_PUBLIC_SUPABASE_URL or NEXT_PUBLIC_SUPABASE_ANON_KEY')
+ }
+ return createServerClient(
+ url,
+ anon,📝 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.
| return createServerClient( | |
| process.env.NEXT_PUBLIC_SUPABASE_URL!, | |
| process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!, | |
| { | |
| const url = process.env.NEXT_PUBLIC_SUPABASE_URL | |
| const anon = process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY | |
| if (!url || !anon) { | |
| throw new Error('Missing NEXT_PUBLIC_SUPABASE_URL or NEXT_PUBLIC_SUPABASE_ANON_KEY') | |
| } | |
| return createServerClient( | |
| url, | |
| anon, | |
| { |
🤖 Prompt for AI Agents
In lib/supabase/client.ts around lines 7 to 10, replace the non-null assertions
on process.env.NEXT_PUBLIC_SUPABASE_URL and
process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY with explicit validation: read both
env vars into local constants, check if either is falsy and if so throw a clear
Error describing which variable(s) are missing, then pass the validated
constants to createServerClient; do not use the "!" operator.
| cookies: { | ||
| async get(name: string) { | ||
| const store = await cookieStore | ||
| return store.get(name)?.value | ||
| }, | ||
| async set(name: string, value: string, options: CookieOptions) { | ||
| const store = await cookieStore | ||
| store.set({ name, value, ...options }) | ||
| }, | ||
| async remove(name: string, options: CookieOptions) { | ||
| const store = await cookieStore | ||
| store.set({ name, value: '', ...options }) | ||
| }, | ||
| }, |
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.
Cookie adapter must be synchronous; fix async usage and ensure proper removal.
@supabase/ssr expects sync cookie handlers. These async functions will not match types and may behave unexpectedly. Also, remove should explicitly expire the cookie.
Apply:
- cookies: {
- async get(name: string) {
- const store = await cookieStore
- return store.get(name)?.value
- },
- async set(name: string, value: string, options: CookieOptions) {
- const store = await cookieStore
- store.set({ name, value, ...options })
- },
- async remove(name: string, options: CookieOptions) {
- const store = await cookieStore
- store.set({ name, value: '', ...options })
- },
- },
+ cookies: {
+ get(name: string) {
+ return cookieStore.get(name)?.value
+ },
+ set(name: string, value: string, options: CookieOptions) {
+ cookieStore.set({ name, value, ...options })
+ },
+ remove(name: string, options: CookieOptions) {
+ cookieStore.set({ name, value: '', maxAge: 0, ...options })
+ },
+ },📝 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.
| cookies: { | |
| async get(name: string) { | |
| const store = await cookieStore | |
| return store.get(name)?.value | |
| }, | |
| async set(name: string, value: string, options: CookieOptions) { | |
| const store = await cookieStore | |
| store.set({ name, value, ...options }) | |
| }, | |
| async remove(name: string, options: CookieOptions) { | |
| const store = await cookieStore | |
| store.set({ name, value: '', ...options }) | |
| }, | |
| }, | |
| cookies: { | |
| get(name: string) { | |
| return cookieStore.get(name)?.value | |
| }, | |
| set(name: string, value: string, options: CookieOptions) { | |
| cookieStore.set({ name, value, ...options }) | |
| }, | |
| remove(name: string, options: CookieOptions) { | |
| cookieStore.set({ name, value: '', maxAge: 0, ...options }) | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In lib/supabase/client.ts around lines 11–24, the cookie adapter currently uses
async functions which conflicts with @supabase/ssr's synchronous cookie handler
requirement and the remove implementation doesn't explicitly expire the cookie;
resolve by ensuring the cookie store is resolved before creating the adapter
(e.g., await cookieStore once and capture the store in a local variable),
replace the async get/set/remove with plain synchronous functions that use that
resolved store, and implement remove to explicitly expire the cookie (set value:
'' and options.expires = new Date(0) or options.maxAge = 0) while preserving
other passed CookieOptions.
| CREATE EXTENSION IF NOT EXISTS "uuid-ossp"; | ||
| CREATE EXTENSION IF NOT EXISTS "postgis"; | ||
| CREATE EXTENSION IF NOT EXISTS "vector"; |
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.
Missing pgcrypto extension for gen_random_uuid()
Every table here uses DEFAULT gen_random_uuid(), but this migration only enables uuid-ossp. Without also enabling pgcrypto, gen_random_uuid() is undefined and the migration will fail immediately. Add CREATE EXTENSION IF NOT EXISTS "pgcrypto"; (or switch your defaults to uuid_generate_v4() from uuid-ossp) before relying on gen_random_uuid().
🤖 Prompt for AI Agents
In supabase/migrations/0000_init.sql around lines 2 to 4, the migration enables
uuid-ossp, postgis, and vector but does not enable pgcrypto, yet the schema uses
gen_random_uuid(); add CREATE EXTENSION IF NOT EXISTS "pgcrypto"; before any
tables that use gen_random_uuid() (or switch the default UUID expressions to
uuid_generate_v4() if you prefer uuid-ossp) so the function exists and the
migration doesn't fail.
| user_id UUID NOT NULL REFERENCES public.users(id) ON DELETE CASCADE, | ||
| title TEXT NOT NULL DEFAULT 'Untitled Chat', | ||
| visibility TEXT DEFAULT 'private' CHECK (visibility IN ('private', 'public')), | ||
| created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() | ||
| ); | ||
| ALTER TABLE public.chats ENABLE ROW LEVEL SECURITY; | ||
| CREATE POLICY "Users can manage their own chats" ON public.chats FOR ALL USING (auth.uid() = user_id); | ||
|
|
||
| -- Messages Table | ||
| CREATE TABLE public.messages ( | ||
| id UUID PRIMARY KEY DEFAULT gen_random_uuid(), | ||
| chat_id UUID NOT NULL REFERENCES public.chats(id) ON DELETE CASCADE, | ||
| user_id UUID NOT NULL REFERENCES public.users(id) ON DELETE CASCADE, | ||
| role TEXT NOT NULL CHECK (role IN ('user', 'assistant', 'system', 'tool')), | ||
| content TEXT NOT NULL, | ||
| embedding VECTOR(1536), | ||
| location_id UUID, -- Defer FK constraint until locations table is created | ||
| created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() | ||
| ); | ||
| ALTER TABLE public.messages ENABLE ROW LEVEL SECURITY; | ||
| CREATE POLICY "Users can manage messages in their own chats" ON public.messages FOR ALL USING ( | ||
| (SELECT user_id FROM public.chats WHERE id = chat_id) = auth.uid() | ||
| ); | ||
|
|
||
| -- System Prompts Table | ||
| CREATE TABLE public.system_prompts ( | ||
| id UUID PRIMARY KEY DEFAULT gen_random_uuid(), | ||
| user_id UUID NOT NULL REFERENCES public.users(id) ON DELETE CASCADE, | ||
| prompt TEXT NOT NULL, | ||
| created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), | ||
| updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() | ||
| ); | ||
| ALTER TABLE public.system_prompts ENABLE ROW LEVEL SECURITY; | ||
| CREATE POLICY "Users can manage their own system prompts" ON public.system_prompts FOR ALL USING (auth.uid() = user_id); | ||
|
|
||
| -- Locations Table (for drawings/map states) | ||
| CREATE TABLE public.locations ( | ||
| id UUID PRIMARY KEY DEFAULT gen_random_uuid(), | ||
| user_id UUID NOT NULL REFERENCES public.users(id) ON DELETE CASCADE, | ||
| chat_id UUID REFERENCES public.chats(id) ON DELETE CASCADE, |
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.
FKs point at public.users, but Supabase auth lives in auth.users
Supabase projects provide auth.users; there is no managed public.users table to alter or reference. These foreign keys and RLS policies will fail unless you manage a shadow public.users table yourself. Update every FK and policy here to target auth.users(id) (and remove the ALTER TABLE public.users block) so the migration runs on a standard Supabase stack.
🤖 Prompt for AI Agents
In supabase/migrations/0000_init.sql around lines 16–55, the migration creates
FKs and RLS policies that reference public.users (which doesn't exist in a
standard Supabase project); update every REFERENCES public.users(id) to
REFERENCES auth.users(id), ensure any user_id columns keep the same UUID type,
update the RLS policy expressions to continue comparing auth.uid() to the
user_id column (no change to the auth.uid() usage required), and remove any
ALTER TABLE public.users ... blocks or references to a shadow public.users table
so the migration will run against the built-in auth.users table instead.
| INSERT INTO public.chat_participants (chat_id, user_id, role) | ||
| VALUES (new_chat_id, user_id, 'owner'); | ||
|
|
||
| FOR message IN SELECT * FROM jsonb_array_elements(messages) | ||
| LOOP | ||
| INSERT INTO public.messages (id, chat_id, user_id, role, content, created_at) | ||
| VALUES ( | ||
| (message->>'id')::UUID, | ||
| new_chat_id, | ||
| user_id, | ||
| message->>'role', | ||
| message->>'content', | ||
| (message->>'createdAt')::TIMESTAMPTZ | ||
| ); |
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.
save_chat_with_messages references chat_participants before it exists
This function inserts into public.chat_participants, but that table is created in migration 0001. Because SQL functions are validated at creation time, this migration will fail as soon as the function is defined. Either create chat_participants in this migration (before the function) or defer the function definition until after the table exists.
🤖 Prompt for AI Agents
In supabase/migrations/0000_init.sql around lines 139 to 152, the function
save_chat_with_messages inserts into public.chat_participants before that table
exists (it’s created in migration 0001), causing validation to fail; fix by
either adding the CREATE TABLE public.chat_participants statement to this
migration before the function definition or remove/relocate the function
definition to a later migration (0001 or later) after chat_participants is
created, and ensure any dependent indexes/constraints are present before
creating the function so SQL validation succeeds.
|
@jules can you look at these comments for me |
User description
This submission refactors the collaboration UI to integrate with the existing settings section. It removes the redundant
ChatShareDialogand connects theUserManagementFormto theinviteUserTo-Chataction.PR created automatically by Jules for task 5223135038854488445
PR Type
Enhancement, Bug fix
Description
Migrated database layer from Drizzle ORM to Supabase for persistence and real-time features
Integrated collaboration UI with settings, enabling chat-specific user invitations
Implemented RAG (Retrieval-Augmented Generation) with hybrid search for context retrieval
Added real-time collaboration with presence tracking and live message updates
Diagram Walkthrough
File Walkthrough
22 files
Refactored to use Supabase persistence layerRemoved Drizzle-based database actionsAdded user invitation to chat functionalityImplemented hybrid search for context retrievalRemoved placeholder user management actionsRefactored to use Supabase SSR clientAdded browser-side Supabase clientImplemented Supabase persistence functions for chats and messagesRemoved Drizzle database clientRemoved Drizzle schema definitionsAdded chatId prop for collaboration contextIntegrated with inviteUserToChat actionPassed chatId to Settings componentAdded real-time collaboration and presence trackingAdded chatId and shareableLink propsIntegrated RAG context retrieval into AI workflowUpdated to fetch messages from SupabaseRefactored to use Supabase saveChat functionSimplified to use Supabase getChats functionUpdated to use Supabase clearChats functionCreated initial database schema with RLS policiesAdded chat_participants table and collaboration features1 files
Removed Drizzle configuration file3 files
Updated type imports to use Chat from typesUpdated to use Chat type and generate pathsUpdated to use Chat type from types1 files
Added verification script for share buttonSummary by CodeRabbit
New Features
Improvements
Tests