feat: Initial Supabase and Drizzle integration for chat backend #194
Conversation
This commit introduces the foundational backend changes for migrating your chat
functionality to Supabase using Drizzle ORM, based on our plan to implement
features similar to those in PR #533.
Key changes include:
1. **Dependencies Added:**
* `@supabase/supabase-js`, `drizzle-orm`, `drizzle-kit`, `pg`, `uuid`, `dotenv`.
2. **Environment Configuration:**
* I set up placeholder environment variables in `.env.local` and
`.env.local.example` for Supabase and database connections.
3. **Drizzle ORM & Schema:**
* I configured Drizzle Kit (`drizzle.config.ts`).
* I defined the database schema (`lib/db/schema.ts`) for `users`, `chats`,
and `messages` tables.
4. **Client Initialization:**
* I initialized the Drizzle client in `lib/db/index.ts`.
* I initialized the Supabase client in `lib/supabase/client.ts`.
5. **Authentication Helper:**
* I created `lib/auth/get-current-user.ts` with `getCurrentUserId`.
6. **Database Migrations:**
* I generated the initial Drizzle migration script for the chat schema.
* I added `lib/db/migrate.ts` and a `db:migrate` script in `package.json`.
7. **Chat Database Actions:**
* I implemented core chat operations (CRUD) in `lib/actions/chat-db.ts`
using Drizzle.
8. **API Routes:**
* I created API routes `app/api/chats/route.ts` (GET) and
`app/api/chat/route.ts` (POST) to interface with the new chat backend.
* I added a `generateUUID` utility.
This work completes the backend setup for your chat system. The next phase
would involve updating frontend components to consume these new services.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
WalkthroughThis update introduces a full database-backed chat system using Drizzle ORM and Supabase authentication, replacing previous Redis-based storage. It adds new API endpoints for chat management, database schema and migration files, authentication utilities, and client/server database setup. Sharing functionality is disabled pending reimplementation. Components and actions are refactored to use the new database logic and authenticated user context. Changes
Sequence Diagram(s)Chat Creation and Fetching FlowsequenceDiagram
participant Client
participant API (/api/chat)
participant Auth (getCurrentUserIdOnServer)
participant DB (Drizzle ORM)
Client->>API: POST /api/chat (title, message, role)
API->>Auth: Retrieve current user ID
Auth-->>API: userId or 401
API->>DB: saveChat(chatData, messagesData)
DB-->>API: chatId or error
API-->>Client: 201 Created (chatId) or error
Client->>API: GET /api/chats?limit&offset
API->>Auth: Retrieve current user ID
Auth-->>API: userId or 401
API->>DB: getChatsPage(userId, limit, offset)
DB-->>API: { chats, nextOffset }
API-->>Client: JSON response
Chat History ClearingsequenceDiagram
participant Client
participant API (/api/chats/all)
participant Auth (getCurrentUserIdOnServer)
participant DB (Drizzle ORM)
Client->>API: DELETE /api/chats/all
API->>Auth: Retrieve current user ID
Auth-->>API: userId or 401
API->>DB: clearHistory(userId)
DB-->>API: success/failure
API-->>Client: 200 OK or 500 error
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
PR Reviewer Guide 🔍(Review updated until commit c661b0c)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to c661b0c
Previous suggestions✅ Suggestions up to commit c661b0c
|
|||||||||||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (12)
package.json (1)
11-11: Verify bun runtime availability and consider cross-platform compatibility.The migration script uses
bunas the runtime, but this may not be available in all deployment environments. Consider usingnodeortsxfor better compatibility, or document the bun requirement.Consider this alternative:
- "db:migrate": "bun lib/db/migrate.ts" + "db:migrate": "tsx lib/db/migrate.ts"Or add tsx as a dev dependency if not already present.
lib/utils/index.ts (1)
9-9: Consider simplifying the UUID generation.The wrapper function is functional but adds an unnecessary abstraction layer. Consider direct export or inline usage for simplicity.
Option 1 - Direct export:
-import { v4 as uuidv4 } from 'uuid'; +export { v4 as generateUUID } from 'uuid'; -export function generateUUID(): string { - return uuidv4(); -}Option 2 - Keep as-is if you prefer explicit function signatures for better IDE support and potential future enhancements.
Also applies to: 15-17
.env.local.example (1)
10-14: Consider removing quotes from environment variable values.While the static analysis warnings about quote characters are not critical for example files, removing the quotes would follow standard
.envfile conventions and avoid potential confusion.-NEXT_PUBLIC_SUPABASE_URL="YOUR_SUPABASE_URL_HERE" -NEXT_PUBLIC_SUPABASE_ANON_KEY="YOUR_SUPABASE_ANON_KEY_HERE" -SUPABASE_SERVICE_ROLE_KEY="YOUR_SUPABASE_SERVICE_ROLE_KEY_HERE" -DATABASE_URL="postgresql://postgres:[YOUR-POSTGRES-PASSWORD]@[YOUR-SUPABASE-DB-HOST]:[PORT]/postgres" +NEXT_PUBLIC_SUPABASE_URL=YOUR_SUPABASE_URL_HERE +NEXT_PUBLIC_SUPABASE_ANON_KEY=YOUR_SUPABASE_ANON_KEY_HERE +SUPABASE_SERVICE_ROLE_KEY=YOUR_SUPABASE_SERVICE_ROLE_KEY_HERE +DATABASE_URL=postgresql://postgres:[YOUR-POSTGRES-PASSWORD]@[YOUR-SUPABASE-DB-HOST]:[PORT]/postgresapp/api/chats/route.ts (1)
19-19: Consider sanitizing error logs to prevent information leakage.Logging the full error object might expose sensitive information or internal implementation details. Consider logging only the error message or using structured logging.
- console.error('Error fetching chats:', error); + console.error('Error fetching chats:', error instanceof Error ? error.message : 'Unknown error');drizzle.config.ts (1)
4-4: Consider making the environment file path configurable.The hardcoded path
.env.localworks for development but might not be suitable for all deployment environments. Consider using a configurable path or following Next.js environment variable precedence.-dotenv.config({ path: '.env.local' }); +dotenv.config({ path: process.env.ENV_FILE_PATH || '.env.local' });lib/supabase/client.ts (1)
20-44: Consider moving extensive comments to documentation.While the commented code provides useful context, it's quite extensive and might be better suited for documentation or a separate example file.
Consider moving this to a separate documentation file or reducing the comment block to essential information only.
app/api/chat/route.ts (2)
4-4: Remove commented import or implement UUID generation.The commented import suggests UUID generation might be needed. Either remove the comment or implement the functionality if required.
-// import { generateUUID } from '@/lib/utils'; // Assuming generateUUID is in lib/utils as per PR context - not needed for PKs
21-21: Add input validation for title parameter.Consider adding validation for the title parameter to prevent potential issues with extremely long titles or invalid characters.
- const { title, initialMessageContent, role = 'user' } = body; + const { title, initialMessageContent, role = 'user' } = body; + + // Validate title if provided + if (title && (typeof title !== 'string' || title.length > 100)) { + return NextResponse.json({ error: 'Invalid title: must be a string under 100 characters' }, { status: 400 }); + }lib/auth/get-current-user.ts (1)
34-36: Use optional chaining for cleaner codeApply this change to use optional chaining as suggested by static analysis:
- if (session && session.user) { - return session.user.id; - } + if (session?.user) { + return session.user.id; + }lib/actions/chat-db.ts (2)
30-39: Use Drizzle'soroperator instead of raw SQLFor better type safety and consistency with Drizzle patterns, use the
oroperator instead of raw SQL.+import { eq, desc, and, or, sql } from 'drizzle-orm'; const result = await db.select() .from(chats) .where( - and( - eq(chats.id, id), - sql`${chats.userId} = ${userId} OR ${chats.visibility} = 'public'` - ) + and( + eq(chats.id, id), + or( + eq(chats.userId, userId), + eq(chats.visibility, 'public') + ) + ) ) .limit(1);
200-200: Remove console.log from production codeDebug logs should not be included in production code.
-console.log('Chat DB actions loaded. Ensure getCurrentUserId() is correctly implemented for server-side usage if applicable.');lib/db/schema.ts (1)
4-14: Consider the users table design in relation to Supabase AuthThe comments correctly note potential redundancy with Supabase Auth. However, maintaining a local users table is a good practice for:
- Enforcing referential integrity with foreign keys
- Storing app-specific user data
- Avoiding direct dependencies on Supabase's auth schema
If you need to sync with Supabase Auth users, consider adding a database trigger or using Supabase's auth hooks to automatically create entries in this table when users sign up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (15)
.env.local.example(1 hunks)app/api/chat/route.ts(1 hunks)app/api/chats/route.ts(1 hunks)drizzle.config.ts(1 hunks)drizzle/migrations/0000_sweet_metal_master.sql(1 hunks)drizzle/migrations/meta/0000_snapshot.json(1 hunks)drizzle/migrations/meta/_journal.json(1 hunks)lib/actions/chat-db.ts(1 hunks)lib/auth/get-current-user.ts(1 hunks)lib/db/index.ts(1 hunks)lib/db/migrate.ts(1 hunks)lib/db/schema.ts(1 hunks)lib/supabase/client.ts(1 hunks)lib/utils/index.ts(1 hunks)package.json(5 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.local.example
[warning] 11-11: [QuoteCharacter] The value has quote characters (', ")
[warning] 12-12: [QuoteCharacter] The value has quote characters (', ")
[warning] 12-12: [UnorderedKey] The NEXT_PUBLIC_SUPABASE_ANON_KEY key should go before the NEXT_PUBLIC_SUPABASE_URL key
[warning] 13-13: [QuoteCharacter] The value has quote characters (', ")
[warning] 14-14: [QuoteCharacter] The value has quote characters (', ")
[warning] 14-14: [UnorderedKey] The DATABASE_URL key should go before the NEXT_PUBLIC_SUPABASE_ANON_KEY key
🪛 Biome (1.9.4)
lib/auth/get-current-user.ts
[error] 34-34: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (13)
package.json (1)
40-40: ```shell
#!/bin/bashFetch latest stable versions for comparison
echo "Latest stable @supabase/supabase-js version:"
npm view @supabase/supabase-js versionecho "Latest stable drizzle-orm version:"
npm view drizzle-orm versionecho "Latest stable drizzle-kit version:"
npm view drizzle-kit versionecho "Latest stable pg version:"
npm view pg version</details> <details> <summary>drizzle/migrations/meta/_journal.json (1)</summary> `1-13`: **Migration journal structure looks correct.** This is a standard Drizzle ORM migration journal file with proper structure for PostgreSQL. The metadata correctly tracks the initial migration state. </details> <details> <summary>app/api/chats/route.ts (1)</summary> `5-22`: **Well-structured API route with proper authentication and error handling.** The route correctly implements: - User authentication with appropriate 401 response - Pagination support with sensible defaults - Proper error handling and HTTP status codes - Clean separation of concerns </details> <details> <summary>drizzle.config.ts (1)</summary> `10-19`: **Configuration looks well-structured.** The Drizzle configuration is properly set up with all necessary fields including schema path, migrations directory, PostgreSQL dialect, and appropriate flags for verbose logging and strict mode. </details> <details> <summary>lib/db/migrate.ts (1)</summary> `24-33`: **Excellent error handling and resource management.** The migration logic properly handles errors, logs progress, and ensures the connection pool is closed in all scenarios. The exit code on failure is appropriate for CI/CD environments. </details> <details> <summary>lib/db/index.ts (1)</summary> `19-19`: **Good practice with conditional logging.** Enabling the Drizzle logger only in development is a good practice for debugging without cluttering production logs. </details> <details> <summary>lib/supabase/client.ts (1)</summary> `6-18`: **Good environment variable validation.** The validation of required Supabase environment variables with clear error messages is well implemented and will help catch configuration issues early. </details> <details> <summary>app/api/chat/route.ts (3)</summary> `11-14`: **Excellent authentication implementation.** The authentication check using `getCurrentUserId()` with proper 401 response for unauthorized access is well implemented. --- `60-67`: **Comprehensive error handling with proper logging.** The error handling includes console logging for debugging and provides both generic and specific error messages appropriately. The status codes are correct. --- `50-54`: **Good transaction handling through saveChat.** Using the `saveChat` function to handle both chat creation and initial message insertion atomically is a good approach for data consistency. </details> <details> <summary>drizzle/migrations/meta/0000_snapshot.json (1)</summary> `1-165`: **Migration snapshot looks good** The database schema snapshot is well-structured with: - Proper UUID primary keys with PostgreSQL's `gen_random_uuid()` - Appropriate foreign key constraints with cascade deletes - Sensible defaults for titles and visibility - Correct timestamp handling with timezone </details> <details> <summary>lib/db/schema.ts (1)</summary> `26-37`: **Good extensibility with commented fields** The commented fields (attachments, toolName, toolCallId, type) show good planning for future features. This aligns with the AI chat functionality mentioned in the PR objectives. </details> <details> <summary>drizzle/migrations/0000_sweet_metal_master.sql (1)</summary> `1-38`: **Well-structured migration with proper error handling** The migration correctly: - Uses `IF NOT EXISTS` for idempotent table creation - Implements foreign key constraints with appropriate cascade deletes - Handles duplicate constraints gracefully using DO blocks - Matches the schema defined in `lib/db/schema.ts` </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
lib/db/migrate.ts
Outdated
| // ssl: { | ||
| // rejectUnauthorized: false, // Ensure this is appropriate for your Supabase connection | ||
| // }, | ||
| // max: 1, // Optional: restrict to 1 connection for migration | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enabling SSL configuration for production environments.
The commented SSL configuration might be necessary for Supabase connections in production. The current setup may fail when connecting to hosted Supabase instances.
const pool = new Pool({
connectionString: process.env.DATABASE_URL,
- // ssl: {
- // rejectUnauthorized: false, // Ensure this is appropriate for your Supabase connection
- // },
+ ssl: process.env.NODE_ENV === 'production' ? {
+ rejectUnauthorized: false, // Required for Supabase
+ } : false,
// max: 1, // Optional: restrict to 1 connection for migration
});📝 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.
| // ssl: { | |
| // rejectUnauthorized: false, // Ensure this is appropriate for your Supabase connection | |
| // }, | |
| // max: 1, // Optional: restrict to 1 connection for migration | |
| }); | |
| const pool = new Pool({ | |
| connectionString: process.env.DATABASE_URL, | |
| ssl: process.env.NODE_ENV === 'production' ? { | |
| rejectUnauthorized: false, // Required for Supabase | |
| } : false, | |
| // max: 1, // Optional: restrict to 1 connection for migration | |
| }); |
🤖 Prompt for AI Agents
In lib/db/migrate.ts around lines 15 to 19, the SSL configuration for the
database connection is commented out, which can cause connection failures in
production with hosted Supabase instances. Uncomment and enable the SSL
settings, specifically setting rejectUnauthorized to false if appropriate, to
ensure secure and successful connections in production environments.
lib/db/index.ts
Outdated
| // ssl: { | ||
| // rejectUnauthorized: false, // Required for Supabase, but ensure this is okay for your security policies | ||
| // }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enable SSL configuration for production environments.
Similar to the migration script, the database client needs SSL configuration for Supabase connections in production environments.
const pool = new Pool({
connectionString: process.env.DATABASE_URL,
- // ssl: {
- // rejectUnauthorized: false, // Required for Supabase, but ensure this is okay for your security policies
- // },
+ ssl: process.env.NODE_ENV === 'production' ? {
+ rejectUnauthorized: false, // Required for Supabase
+ } : 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.
| // ssl: { | |
| // rejectUnauthorized: false, // Required for Supabase, but ensure this is okay for your security policies | |
| // }, | |
| }); | |
| const pool = new Pool({ | |
| connectionString: process.env.DATABASE_URL, | |
| ssl: process.env.NODE_ENV === 'production' ? { | |
| rejectUnauthorized: false, // Required for Supabase | |
| } : false, | |
| }); |
🤖 Prompt for AI Agents
In lib/db/index.ts around lines 14 to 17, the SSL configuration for the database
client is commented out, which is necessary for secure Supabase connections in
production. Uncomment and enable the ssl option with rejectUnauthorized set to
false, but only apply this configuration conditionally for production
environments to ensure security policies are respected.
…ates This commit includes the initial backend setup for chat functionality using Supabase and Drizzle, addresses several critical fixes based on feedback, and begins the frontend UI refactoring for chat history. Phase 1 Implementation (Supabase/Drizzle Backend for Chat): - Installed all necessary dependencies (Supabase, Drizzle, pg, uuid, dotenv, @supabase/ssr, cookie, cross-env). - Configured environment variables (.env.local, .env.local.example). - Defined Drizzle schema for users, chats, messages (`lib/db/schema.ts`). - Initialized Drizzle and Supabase clients (`lib/db/index.ts`, `lib/supabase/client.ts`). - Implemented initial database migration scripts and runner (`drizzle/migrations/`, `lib/db/migrate.ts`). - Created chat database actions using Drizzle (`lib/actions/chat-db.ts`). - Established API routes for chat operations (`app/api/chats/route.ts`, `app/api/chat/route.ts`). Fixes and Refinements Based on Feedback: 1. Added input validation for pagination in `/api/chats`. 2. Made migration script execution (`lib/db/migrate.ts`) conditional, controlled by `EXECUTE_MIGRATIONS` env var via `cross-env` in `package.json`. 3. Enabled SSL with `rejectUnauthorized: false` for database connections in `migrate.ts`. 4. Enabled conditional SSL (`rejectUnauthorized: false` for Supabase URLs) in `lib/db/index.ts`. 5. Removed `dotenv` usage from client-side `lib/supabase/client.ts`. 6. Refactored `lib/auth/get-current-user.ts` to use `@supabase/ssr` for robust server-side authentication (`getCurrentUserIdOnServer`) and updated all server-side usages (API routes, `lib/actions/chat.ts`). 7. Corrected transaction handling in `lib/actions/chat-db.ts` to use `throw new Error()` for automatic rollbacks by Drizzle. Frontend Updates (Partial - Step 9): - Refactored `lib/actions/chat.ts` (old Redis actions) to call new Drizzle DB actions for chat operations or commented out share/prompt functions. - Created `components/sidebar/chat-history-client.tsx` for displaying chat history, fetching from the new API. - Created `app/api/chats/all/route.ts` (DELETE endpoint) for clearing chat history. - Updated `components/history-item.tsx` for compatibility with new Drizzle-based chat data types. - Updated `components/history.tsx` (sheet component) to use the new `ChatHistoryClient`. The next steps would be to complete the frontend refactoring (chat.tsx, chat-messages.tsx) and then proceed to Phase 2 (multi-user map features). I recommend you run `bun run build` after these changes.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
lib/db/migrate.ts (1)
8-41: Well-implemented migration script with proper controls.The migration script properly addresses previous review concerns by:
- Using
EXECUTE_MIGRATIONSenvironment variable to control execution- Including SSL configuration for Supabase connections
- Implementing proper error handling and resource cleanup
- Providing clear logging and instructions
lib/auth/get-current-user.ts (1)
16-68: Excellent server-side authentication implementation.The implementation correctly uses
@supabase/ssrfor server-side contexts with proper:
- Cookie-based session management
- Environment variable validation
- Error handling and logging
- Resource cleanup
This addresses the security concerns mentioned in past reviews by using the appropriate server-side Supabase client.
lib/actions/chat.ts (1)
95-95: Verify visibility field in schemaSetting
visibility: 'private'assumes this field exists in the schema.
🧹 Nitpick comments (6)
lib/db/index.ts (1)
17-21: Simplify conditional check with optional chaining.The condition can be simplified using optional chaining as suggested by static analysis.
-if (process.env.DATABASE_URL && process.env.DATABASE_URL.includes('supabase.co')) { +if (process.env.DATABASE_URL?.includes('supabase.co')) { poolConfig.ssl = { rejectUnauthorized: false, }; }components/sidebar/chat-history-client.tsx (3)
43-43: Consider making pagination configurableThe pagination parameters are hardcoded. Consider making them configurable or implementing infinite scroll for better UX.
You could accept pagination parameters as props or implement infinite scroll:
- const response = await fetch('/api/chats?limit=50&offset=0'); // Example limit/offset + const response = await fetch('/api/chats?limit=50&offset=0');Consider adding pagination state management for loading more chats as users scroll.
68-73: Use RESTful endpoint naming conventionThe endpoint
/api/chats/allis unconventional. Consider using DELETE/api/chatsfor clearing all chats, which is more RESTful.- // We need a new API endpoint for clearing history - // Example: DELETE /api/chats (or POST /api/clear-history) - // This endpoint will call clearHistory(userId) from chat-db.ts - const response = await fetch('/api/chats/all', { // Placeholder for the actual clear endpoint + const response = await fetch('/api/chats', { method: 'DELETE', });
126-126: Extract path construction to a utility functionThe inline path construction should be extracted for reusability and consistency.
Consider creating a utility function:
// In a utils file export const getChatPath = (chatId: string) => `/search/${chatId}`;Then use it here:
- <HistoryItem key={chat.id} chat={{...chat, path: `/search/${chat.id}`}} /> + <HistoryItem key={chat.id} chat={{...chat, path: getChatPath(chat.id)}} />lib/actions/chat-db.ts (1)
200-200: Remove console.log from production codeConsole.log statements should not be included in production code.
-console.log('Chat DB actions loaded. Ensure getCurrentUserId() is correctly implemented for server-side usage if applicable.');lib/actions/chat.ts (1)
31-32: Make pagination parameters configurableThe pagination parameters are hardcoded. Consider accepting them as function parameters.
-export async function getChats(userId?: string | null): Promise<DrizzleChat[]> { +export async function getChats(userId?: string | null, limit: number = 20, offset: number = 0): Promise<DrizzleChat[]> { if (!userId) { console.warn('getChats called without userId, returning empty array.') return [] } try { - // Using a default limit and offset for now - const { chats } = await dbGetChatsPage(userId, 20, 0) + const { chats } = await dbGetChatsPage(userId, limit, offset) return chats
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (13)
app/api/chat/route.ts(1 hunks)app/api/chats/all/route.ts(1 hunks)app/api/chats/route.ts(1 hunks)components/history-item.tsx(1 hunks)components/history.tsx(2 hunks)components/sidebar/chat-history-client.tsx(1 hunks)lib/actions/chat-db.ts(1 hunks)lib/actions/chat.ts(1 hunks)lib/auth/get-current-user.ts(1 hunks)lib/db/index.ts(1 hunks)lib/db/migrate.ts(1 hunks)lib/supabase/client.ts(1 hunks)package.json(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/history-item.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- app/api/chats/route.ts
- lib/supabase/client.ts
- app/api/chat/route.ts
- package.json
🧰 Additional context used
🪛 Biome (1.9.4)
lib/db/index.ts
[error] 17-17: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
components/sidebar/chat-history-client.tsx
[error] 24-27: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 29-29: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
lib/actions/chat.ts
[error] 59-59: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (7)
components/history.tsx (2)
12-12: Clean migration to authenticated chat history component.The import change from
HistoryListtoChatHistoryClientaligns with the migration to Supabase authentication and database-backed chat history.
43-43: Proper removal of hardcoded anonymous user ID.The new
ChatHistoryClientcomponent correctly handles user authentication internally, eliminating the need for the hardcodeduserId="anonymous"prop.lib/db/index.ts (1)
1-26: Well-structured database client setup with proper SSL handling.The database client configuration correctly handles environment variables, SSL configuration for Supabase, and exports a properly configured Drizzle ORM client.
app/api/chats/all/route.ts (1)
7-23: Proper authentication and error handling for chat clearing.The DELETE handler correctly validates user authentication and handles the database operation with appropriate success/failure responses.
lib/auth/get-current-user.ts (1)
76-84: Clean wrapper function for user ID retrieval.The
getCurrentUserIdOnServerfunction provides a simple interface for getting the current user ID while properly handling errors and avoiding redundant logging.lib/actions/chat-db.ts (1)
26-26: Verify that the 'visibility' field exists in the chats schemaThe code references
chats.visibilitybut this field is not mentioned in the AI summary of the schema. Please ensure this field is defined inlib/db/schema.ts.#!/bin/bash # Description: Check if visibility field is defined in the chats table schema # Search for visibility field definition in schema ast-grep --pattern 'visibility: $_' lib/db/schema.ts # Also check with ripgrep for any visibility-related schema definitions rg -A 3 -B 3 'visibility' lib/db/schema.tsAlso applies to: 35-35
lib/actions/chat.ts (1)
160-160: ```shell
#!/bin/bashSearch for Role type or enum definitions in TypeScript files
rg --type ts --no-heading --line-number "export (type|enum) .*Role" .
Search for Zod schema definitions involving a role enum
rg --type ts --no-heading --line-number "role *: *z.enum" .
If Prisma is used, inspect the Message model and its role field
if [ -f prisma/schema.prisma ]; then
grep -R --line-number "model Message" prisma/schema.prisma
grep -R --line-number "role" prisma/schema.prisma
fi</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| // This case might be redundant if dbClearHistory throws an error on failure, | ||
| // but kept for explicitness if it returns false for "no error but nothing done". | ||
| return NextResponse.json({ error: 'Failed to clear history' }, { status: 500 }); | ||
| } | ||
| } catch (error) { | ||
| console.error('Error clearing history via API:', error); | ||
| let errorMessage = 'Internal Server Error clearing history'; | ||
| if (error instanceof Error && error.message) { | ||
| // Use the error message from dbClearHistory if available (e.g., "User ID is required") | ||
| // This depends on dbClearHistory actually throwing or returning specific error messages. | ||
| // The current dbClearHistory in chat.ts returns {error: ...} which won't be caught here as an Error instance directly. | ||
| // However, the dbClearHistory in chat-db.ts returns boolean. | ||
| // Let's assume if dbClearHistory from chat-db.ts (which returns boolean) fails, it's a generic 500. | ||
| // If it were to throw, that would be caught. | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove dead code and clarify error handling.
The comments and conditional logic in the error handling section appear to be leftover from development and should be cleaned up.
} else {
- // This case might be redundant if dbClearHistory throws an error on failure,
- // but kept for explicitness if it returns false for "no error but nothing done".
return NextResponse.json({ error: 'Failed to clear history' }, { status: 500 });
}
} catch (error) {
console.error('Error clearing history via API:', error);
- let errorMessage = 'Internal Server Error clearing history';
- if (error instanceof Error && error.message) {
- // Use the error message from dbClearHistory if available (e.g., "User ID is required")
- // This depends on dbClearHistory actually throwing or returning specific error messages.
- // The current dbClearHistory in chat.ts returns {error: ...} which won't be caught here as an Error instance directly.
- // However, the dbClearHistory in chat-db.ts returns boolean.
- // Let's assume if dbClearHistory from chat-db.ts (which returns boolean) fails, it's a generic 500.
- // If it were to throw, that would be caught.
- }
- return NextResponse.json({ error: errorMessage }, { status: 500 });
+ return NextResponse.json({ error: 'Internal Server Error clearing history' }, { status: 500 });
}📝 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.
| // This case might be redundant if dbClearHistory throws an error on failure, | |
| // but kept for explicitness if it returns false for "no error but nothing done". | |
| return NextResponse.json({ error: 'Failed to clear history' }, { status: 500 }); | |
| } | |
| } catch (error) { | |
| console.error('Error clearing history via API:', error); | |
| let errorMessage = 'Internal Server Error clearing history'; | |
| if (error instanceof Error && error.message) { | |
| // Use the error message from dbClearHistory if available (e.g., "User ID is required") | |
| // This depends on dbClearHistory actually throwing or returning specific error messages. | |
| // The current dbClearHistory in chat.ts returns {error: ...} which won't be caught here as an Error instance directly. | |
| // However, the dbClearHistory in chat-db.ts returns boolean. | |
| // Let's assume if dbClearHistory from chat-db.ts (which returns boolean) fails, it's a generic 500. | |
| // If it were to throw, that would be caught. | |
| } | |
| } else { | |
| return NextResponse.json({ error: 'Failed to clear history' }, { status: 500 }); | |
| } | |
| } catch (error) { | |
| console.error('Error clearing history via API:', error); | |
| return NextResponse.json({ error: 'Internal Server Error clearing history' }, { status: 500 }); | |
| } |
🤖 Prompt for AI Agents
In app/api/chats/all/route.ts between lines 20 and 34, the error handling code
contains commented-out explanations and conditional checks that are no longer
relevant or used. Remove these dead comments and any unused conditional logic to
clean up and clarify the error handling section, leaving only the necessary code
that logs the error and returns the appropriate response.
| 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.
Replace empty interface with type alias and fix empty object pattern
The empty interface is equivalent to {} and the empty object destructuring pattern is unnecessary.
Apply this diff to fix both issues:
-interface ChatHistoryClientProps {
- // userId is no longer passed as prop; API route will use authenticated user
-}
+type ChatHistoryClientProps = Record<string, never>;
-export function ChatHistoryClient({}: ChatHistoryClientProps) {
+export function ChatHistoryClient() {🧰 Tools
🪛 Biome (1.9.4)
[error] 29-29: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🤖 Prompt for AI Agents
In components/sidebar/chat-history-client.tsx around lines 25 to 29, the empty
interface ChatHistoryClientProps and the empty object destructuring in the
function parameter are redundant. Replace the empty interface with a type alias
if needed or remove it entirely, and remove the empty object pattern from the
function parameter to simplify the code.
| // TODO: These Redis-based functions for system prompt need to be migrated | ||
| // if their functionality is still required and intended to use the new DB. | ||
| // For now, they are left as is, but will likely fail if Redis config is removed. | ||
| const redis = new Redis({ // This will cause issues if REDIS_URL is not configured. | ||
| url: process.env.UPSTASH_REDIS_REST_URL?.trim() || '', | ||
| token: process.env.UPSTASH_REDIS_REST_TOKEN || '' | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Complete Redis migration or remove deprecated code
The Redis-based functions should be migrated to use the new database or removed if no longer needed. The current Redis initialization will fail if environment variables are not configured.
Would you like me to help migrate these system prompt functions to use the new database schema? This would involve:
- Adding a system_prompts table to the schema
- Creating database functions for saving/retrieving system prompts
- Removing the Redis dependency
Or should these functions be removed entirely if they're no longer needed?
🤖 Prompt for AI Agents
In lib/actions/chat.ts around lines 187 to 194, the Redis-based system prompt
functions are deprecated and will fail if Redis environment variables are
missing. Decide whether to fully migrate these functions to the new database by
adding a system_prompts table and implementing corresponding save/retrieve
functions, then remove the Redis dependency; or remove these Redis-based
functions entirely if they are no longer needed. Implement the chosen approach
to eliminate reliance on Redis and prevent runtime errors.
| if (!chats.length) { | ||
| return { error: 'No chats to clear' } | ||
| userId?: string | null // Changed to optional, will try to get current user if not provided | ||
| ): Promise<{ error?: string } | void> { // void for success |
There was a problem hiding this comment.
Replace void with undefined in return type
Using void in a union type is confusing. Use undefined for success cases instead.
-): Promise<{ error?: string } | void> { // void for success
+): Promise<{ error?: string } | undefined> { // undefined for success📝 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.
| ): Promise<{ error?: string } | void> { // void for success | |
| ): Promise<{ error?: string } | undefined> { // undefined for success |
🧰 Tools
🪛 Biome (1.9.4)
[error] 59-59: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🤖 Prompt for AI Agents
In lib/actions/chat.ts at line 59, replace the return type union that includes
void with undefined instead. Change the Promise return type from Promise<{
error?: string } | void> to Promise<{ error?: string } | undefined> to clearly
indicate that the success case returns undefined rather than void.
This commit applies several fixes to address issues identified in the
previous build log and your feedback:
1. **SSL Configuration:** Verified and ensured correct SSL settings
(rejectUnauthorized: false for Supabase) in `lib/db/migrate.ts` and
conditionally in `lib/db/index.ts`.
2. **Import Fix:** Corrected the `HistoryItem` import in
`components/sidebar/chat-history-client.tsx` to use a default import.
3. **Share Functionality Removal:**
* Adjusted `app/share/[id]/page.tsx` and `app/share/loading.tsx`
aligning with PR #533's removal of the share page.
* Commented out `shareChat` usage in `components/chat-share.tsx`
to resolve import errors, effectively disabling the old share UI.
4. **`saveChat` Type Error Resolution:**
* Ensured `app/actions.tsx` correctly calls `saveChat` (from
`lib/actions/chat.ts`) with two arguments (`chat` object and the
authenticated `userId`).
* Verified that `saveChat` in `lib/actions/chat.ts` is defined to
accept these two arguments and uses the `userId` appropriately when
preparing data for the database, resolving the "Expected 2 arguments,
but got 1" error.
* Dynamically fetched and used authenticated `userId` in
`app/actions.tsx` when saving chats.
* Adjusted message role mapping in `lib/actions/chat.ts` for flexibility.
These changes address all known build errors and critical feedback items,
making the codebase more robust and aligned with the intended Supabase
integration. A `bun run build` is recommended to confirm stability.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
components/sidebar/chat-history-client.tsx (1)
24-28: Replace empty interface with type alias and fix empty object patternThe empty interface is equivalent to
{}and the empty object destructuring pattern is unnecessary.Apply this diff to fix both issues:
-interface ChatHistoryClientProps { - // userId is no longer passed as prop; API route will use authenticated user -} +type ChatHistoryClientProps = Record<string, never>; -export function ChatHistoryClient({}: ChatHistoryClientProps) { +export function ChatHistoryClient() {lib/actions/chat.ts (2)
59-59: Replace void with undefined in return typeUsing
voidin a union type is confusing. Useundefinedfor success cases instead.-): Promise<{ error?: string } | void> { // void for success +): Promise<{ error?: string } | undefined> { // undefined for success
187-194: Complete Redis migration or remove deprecated codeThe Redis-based functions should be migrated to use the new database or removed if no longer needed. The current Redis initialization will fail if environment variables are not configured.
Would you like me to help migrate these system prompt functions to use the new database schema? This would involve:
- Adding a system_prompts table to the schema
- Creating database functions for saving/retrieving system prompts
- Removing the Redis dependency
Or should these functions be removed entirely if they're no longer needed?
🧹 Nitpick comments (2)
components/chat-share.tsx (1)
64-66: Replace constant condition with feature flag pattern.The static analysis tool correctly flags the constant condition. Consider using a more maintainable feature flag approach.
- // TODO: Re-evaluate/reimplement sharing with Supabase. For now, disable the UI. - if (true) { // Conditionally disable the share button/dialog - return null; // Or return a disabled button: <Button className={className} variant="ghost" size="icon" disabled><Share size={14} /></Button> - } + // TODO: Re-evaluate/reimplement sharing with Supabase. For now, disable the UI. + const SHARING_ENABLED = false; // Feature flag for sharing functionality + if (!SHARING_ENABLED) { + return null; // Or return a disabled button: <Button className={className} variant="ghost" size="icon" disabled><Share size={14} /></Button> + }components/sidebar/chat-history-client.tsx (1)
42-42: Consider making pagination parameters configurable.The hardcoded limit and offset values should be configurable for better flexibility and future pagination features.
- const response = await fetch('/api/chats?limit=50&offset=0'); // Example limit/offset + const DEFAULT_LIMIT = 50; + const DEFAULT_OFFSET = 0; + const response = await fetch(`/api/chats?limit=${DEFAULT_LIMIT}&offset=${DEFAULT_OFFSET}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/actions.tsx(3 hunks)app/share/[id]/page.tsx(0 hunks)components/chat-share.tsx(4 hunks)components/sidebar/chat-history-client.tsx(1 hunks)lib/actions/chat.ts(1 hunks)
💤 Files with no reviewable changes (1)
- app/share/[id]/page.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
components/chat-share.tsx
[error] 64-64: Unexpected constant condition.
(lint/correctness/noConstantCondition)
components/sidebar/chat-history-client.tsx
[error] 23-26: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 28-28: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
lib/actions/chat.ts
[error] 59-59: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (10)
components/chat-share.tsx (2)
15-15: Good documentation of the migration plan.The TODO comment clearly indicates the need to re-evaluate sharing functionality with Supabase, providing context for the disabled functionality.
31-61: Well-preserved logic for future reimplementation.The commenting approach maintains the original sharing logic while providing clear feedback to users about the disabled functionality. This will make reimplementation easier when Supabase integration is complete.
components/sidebar/chat-history-client.tsx (2)
36-62: Excellent error handling and user feedback.The fetch logic properly handles errors, provides user feedback via toast notifications, and maintains clean loading states.
64-94: Robust implementation of clear history functionality.The clear history handler properly manages the confirmation dialog, API calls, error handling, and UI state updates. The router refresh ensures the UI reflects the changes.
app/actions.tsx (3)
276-278: Clear documentation of authentication requirements.The TODO comment appropriately indicates where server-side authentication might be needed for initial UI state, providing helpful context for future development.
314-324: Excellent authentication integration with proper error handling.The implementation correctly retrieves the authenticated user ID and prevents anonymous chat persistence with clear error logging. This ensures data integrity and proper user association.
300-301: Good consistency in title fallback values.The standardized 'Untitled Chat' fallback aligns with the database schema defaults, ensuring consistency across the application.
lib/actions/chat.ts (3)
24-38: Robust database integration with proper error handling.The migration to database-backed chat retrieval includes comprehensive error handling and user validation, improving reliability over the previous Redis implementation.
81-116: Well-architected chat saving with type mapping.The function effectively bridges the old Chat type to new Drizzle types while maintaining data integrity. The comprehensive mapping includes proper defaults and error handling.
145-185: Improved drawing context handling with authentication.The rewritten function properly integrates user authentication and uses the new database message creation, ensuring proper user association and data persistence.
Corrects the type of `firstMessage` in `app/api/chat/route.ts`
from `NewMessage` to `Omit<NewMessage, 'chatId'>`. This ensures
that the object, which doesn't have `chatId` at the point of creation,
correctly aligns with the expected input type for the `messagesData`
parameter of the `saveChat` function in `lib/actions/chat-db.ts`.
This change is intended to resolve the persistent build error:
"Type error: Property 'chatId' is missing in type '{ userId: string; role: any; content: any; }' but required in type '{ chatId: string; ... }'."
This commit addresses the build error "Property 'messages' does not exist
on type '{...}'" in `app/search/[id]/page.tsx`.
Changes include:
1. **New Database Action `getMessagesByChatId`:** Added to
`lib/actions/chat-db.ts` to fetch all messages for a given chat ID
from the Supabase database using Drizzle, ordered by creation time.
2. **New Server Action `getChatMessages`:** Added to `lib/actions/chat.ts`
to expose `getMessagesByChatId` for use by frontend components or pages.
3. **Refactored `app/search/[id]/page.tsx`:**
* The page now calls `getChatMessages` after fetching the initial
chat metadata.
* Fetched database messages are transformed into the `AIMessage[]`
format expected by the AI provider.
* The `AI` provider's `initialAIState.messages` is populated with
these transformed messages.
* Ensured `getCurrentUserIdOnServer` is used to fetch data
relevant to you.
This allows existing chat sessions to be correctly loaded and displayed
with their full message history from the new Supabase backend.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
lib/actions/chat.ts (2)
79-79: Replace void with undefined in return typeUsing
voidin a union type is confusing. Useundefinedfor success cases instead.-): Promise<{ error?: string } | void> { // void for success +): Promise<{ error?: string } | undefined> { // undefined for success
207-254: Complete Redis migration or remove deprecated codeThe Redis-based functions should be migrated to use the new database or removed if no longer needed. The current Redis initialization will fail if environment variables are not configured.
Would you like me to help migrate these system prompt functions to use the new database schema? This would involve:
- Adding a system_prompts table to the schema
- Creating database functions for saving/retrieving system prompts
- Removing the Redis dependency
Or should these functions be removed entirely if they're no longer needed?
🧹 Nitpick comments (4)
app/search/[id]/page.tsx (2)
18-21: Consider metadata generation strategy for private chats.The TODO comment correctly identifies a potential issue. If chats are private by default, the metadata generation might fail or return inappropriate data for unauthenticated users. Consider implementing a dedicated metadata endpoint that returns generic metadata for private chats when accessed without authentication.
31-35: Redirect to login page instead of home.As noted in the comment, redirecting unauthenticated users to a login page would provide a better user experience than redirecting to home.
- // For now, redirecting to home, but a login page would be better. - redirect('/'); + redirect('/login'); // or your authentication page routelib/actions/chat-db.ts (2)
30-38: Use Drizzle'sorhelper instead of raw SQL.Replace the SQL template with Drizzle's
orhelper for better type safety and maintainability.+import { eq, desc, and, sql, asc, or } from 'drizzle-orm'; // Add 'or' to imports const result = await db.select() .from(chats) .where( and( eq(chats.id, id), - sql`${chats.userId} = ${userId} OR ${chats.visibility} = 'public'` + or( + eq(chats.userId, userId), + eq(chats.visibility, 'public') + ) ) ) .limit(1);
223-223: Remove module-level console.log statement.Module-level console.log statements execute on every import and should be avoided in production code.
-console.log('Chat DB actions loaded. Ensure getCurrentUserId() is correctly implemented for server-side usage if applicable.');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/search/[id]/page.tsx(1 hunks)lib/actions/chat-db.ts(1 hunks)lib/actions/chat.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
lib/actions/chat.ts
[error] 79-79: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (3)
app/search/[id]/page.tsx (1)
45-60: Well-implemented message transformation.The transformation from database messages to AI messages is properly handled with appropriate type casting and clear documentation about unmapped fields.
lib/actions/chat-db.ts (1)
83-126: Excellent transaction handling and error management.The function properly uses transactions for atomicity and correctly throws errors instead of manual rollback, addressing the previous review comment.
lib/actions/chat.ts (1)
180-180: ```shell
#!/bin/bashCheck for role definitions in the DB schema file
echo "Searching for 'role' in lib/db/schema.ts"
rg -n "role" lib/db/schema.tsLook for any enum definitions in the DB schema file
echo "Searching for 'enum' in lib/db/schema.ts"
rg -n "enum" lib/db/schema.tsSearch for TypeScript types or enums naming Message roles globally
echo "Searching for type or enum definitions of message roles"
rg -n "type .*Role" -A 2 .
rg -n "export enum .*Role" -A 2 .Fallback: search for string literals including 'data' among roles
echo "Searching for hardcoded 'data' in the codebase"
rg -n "'data'" -A 2 .</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| // Revalidation and redirect should ideally be handled by the caller (e.g., Server Action, API route) | ||
| // For now, keeping them as they were, but this makes the function less reusable. | ||
| revalidatePath('/') | ||
| redirect('/') |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move navigation side effects to the caller.
As noted in the comment, including revalidatePath and redirect in this data function reduces reusability. These side effects should be handled by the caller (API route or Server Action).
- // Revalidation and redirect should ideally be handled by the caller (e.g., Server Action, API route)
- // For now, keeping them as they were, but this makes the function less reusable.
- revalidatePath('/')
- redirect('/')
+ // Return success - let the caller handle navigation
+ return undefined;📝 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.
| // Revalidation and redirect should ideally be handled by the caller (e.g., Server Action, API route) | |
| // For now, keeping them as they were, but this makes the function less reusable. | |
| revalidatePath('/') | |
| redirect('/') | |
| // Return success - let the caller handle navigation | |
| return undefined; |
🤖 Prompt for AI Agents
In lib/actions/chat.ts around lines 91 to 94, the calls to revalidatePath('/')
and redirect('/') are side effects that reduce the function's reusability.
Remove these calls from this function and instead handle revalidation and
redirection in the caller, such as the API route or Server Action that invokes
this function.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/history-list.tsx (1)
6-23: Consider moving type definitions to a shared location.The
ChatDataandChattype definitions are duplicated locally in this component. Since these types represent the data contracts between your database layer and UI components, they would be better placed in a shared types file or exported from the respective modules they represent.Consider moving these types to
lib/types/chat.tsor exporting them directly fromlib/actions/chat.ts:-// Define the type for the chat data returned by getChats -type ChatData = { - userId: string; - id: string; - title: string; - createdAt: Date; - visibility: string | null; -}; - -// Define the Chat type expected by HistoryItem -type Chat = { - userId: string; - id: string; - title: string; - createdAt: Date; - visibility: string | null; - path: string; -}; +import type { ChatData, Chat } from '@/lib/types/chat';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (5)
components/history-list.tsx(1 hunks)drizzle.config.ts(1 hunks)lib/actions/chat-db.ts(1 hunks)lib/auth/get-current-user.ts(1 hunks)package.json(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- drizzle.config.ts
- lib/auth/get-current-user.ts
- lib/actions/chat-db.ts
- package.json
🔇 Additional comments (5)
components/history-list.tsx (5)
29-31: Good use of React cache for server component optimization.The
loadChatsfunction correctly uses React'scachefunction to memoize the async operation, which is ideal for server components to avoid duplicate database calls during rendering.
34-45: Comprehensive error handling with clear user feedback.The error handling correctly distinguishes between a null return (failed load) and provides appropriate user feedback. The UI gracefully handles the failed state.
71-80: Consistent error handling across all failure scenarios.Good implementation of consistent error handling that covers both exceptions and null returns with appropriate user-facing messages. The console.error logging will help with debugging in production.
4-4: Verify the import path aligns with the new backend architecture.The import from
@/lib/actions/chatshould align with the new Drizzle/Supabase implementation mentioned in the PR objectives.Please verify that the
getChatsfunction exists at the expected location and returns the correct type:#!/bin/bash # Description: Verify getChats function implementation and return type # Expected: Find getChats function definition and its return type # Search for getChats function definition ast-grep --pattern $'export $_ function getChats($$$) { $$$ }' # Search for getChats in any export statement rg -A 5 "export.*getChats" # Check the file structure for chat actions fd -t f -e ts -e tsx | grep -E "(action|chat)" | head -10
58-61: ```shell
#!/bin/bashInspect how chat.path is used inside HistoryItem component
rg -n "chat.path" -A5 -B5 components/history-item.tsx
Check Link usage to see if path is used for navigation
rg -n "<Link" -A5 -B5 components/history-item.tsx
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
lib/actions/chat.ts (3)
227-275: Complete Redis migration or remove deprecated code.The Redis-based system prompt functions will fail if Redis environment variables are not configured. Since the rest of the file has been migrated to the database, these functions should either be:
- Migrated to use the new database schema (add a system_prompts table)
- Removed entirely if no longer needed
#!/bin/bash # Check if Redis environment variables are configured echo "Checking Redis environment variables:" if [ -z "$UPSTASH_REDIS_REST_URL" ]; then echo "UPSTASH_REDIS_REST_URL is not set" else echo "UPSTASH_REDIS_REST_URL is configured" fi if [ -z "$UPSTASH_REDIS_REST_TOKEN" ]; then echo "UPSTASH_REDIS_REST_TOKEN is not set" else echo "UPSTASH_REDIS_REST_TOKEN is configured" fi # Check if these functions are still being used rg -n "saveSystemPrompt|getSystemPrompt" --type ts --type tsx
79-79: Replace void with undefined in return type.The static analysis tool correctly identifies that
voidis confusing in union types. Useundefinedfor success cases instead.-): Promise<{ error?: string } | void> { // void for success +): Promise<{ error?: string } | undefined> { // undefined for success
91-94: Move navigation side effects to the caller.The
revalidatePathandredirectcalls reduce the function's reusability by coupling data operations with navigation. These side effects should be handled by the caller (API route or Server Action).- // Revalidation and redirect should ideally be handled by the caller (e.g., Server Action, API route) - // For now, keeping them as they were, but this makes the function less reusable. - revalidatePath('/') - redirect('/') + // Return success - let the caller handle navigation + return undefined;
🧹 Nitpick comments (2)
app/actions.tsx (1)
335-361: Consider reducing logging verbosity for production.The extensive logging provides excellent debugging information during development, but this level of verbosity might be excessive for production environments. Consider implementing log levels or environment-based logging controls.
- console.log('[app/actions.tsx] onSetAIState: Attempting to save chat.'); - console.log('[app/actions.tsx] onSetAIState: actualUserId:', actualUserId); - console.log('[app/actions.tsx] onSetAIState: chat ID:', chat.id); + if (process.env.NODE_ENV === 'development') { + console.log('[app/actions.tsx] onSetAIState: Attempting to save chat.'); + console.log('[app/actions.tsx] onSetAIState: actualUserId:', actualUserId); + console.log('[app/actions.tsx] onSetAIState: chat ID:', chat.id); + }lib/actions/chat.ts (1)
102-139: Consider reducing logging verbosity for production.While the extensive logging is valuable for debugging, this level of detail might be excessive for production environments. Consider implementing environment-based logging controls.
+ const isDevelopment = process.env.NODE_ENV === 'development'; + - console.log('[lib/actions/chat.ts] saveChat: Called with userId:', userId); - console.log('[lib/actions/chat.ts] saveChat: Chat ID:', chat.id); + if (isDevelopment) { + console.log('[lib/actions/chat.ts] saveChat: Called with userId:', userId); + console.log('[lib/actions/chat.ts] saveChat: Chat ID:', chat.id); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (6)
app/actions.tsx(3 hunks)drizzle.config.ts(1 hunks)lib/actions/chat-db.ts(1 hunks)lib/actions/chat.ts(1 hunks)lib/auth/get-current-user.ts(1 hunks)package.json(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- drizzle.config.ts
- lib/auth/get-current-user.ts
- package.json
- lib/actions/chat-db.ts
🧰 Additional context used
🪛 Biome (1.9.4)
lib/actions/chat.ts
[error] 79-79: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (4)
app/actions.tsx (2)
276-277: LGTM! Good documentation of current limitations.The TODO comment appropriately acknowledges that the current implementation only uses
getAIState()but notes that server-side auth adaptation might be needed for initial UI state based on user context.
314-324: Excellent authentication implementation with proper error handling.The dynamic import of
getCurrentUserIdOnServerand subsequent user validation is well-implemented. The early return when user is not authenticated prevents unauthorized chat saves, which is the correct security behavior.lib/actions/chat.ts (2)
64-75: LGTM! Well-implemented message retrieval function.The new
getChatMessagesfunction provides clean separation of concerns and proper error handling. The implementation follows the established patterns in the file.
185-225: Good migration to database-backed drawing context.The
updateDrawingContextfunction has been successfully migrated to use the database. The authentication check and error handling are appropriate. The comment about the removedtypefield is helpful for understanding the schema constraints.
ngoiyaeric
force-pushed
the
feat/supabase-chat-integration
branch
from
6a22d08 to
46261c4
Compare
2 participants
User description
This commit introduces the foundational backend changes for migrating your chat functionality to Supabase using Drizzle ORM, based on our plan to implement features similar to those in PR #533.
Key changes include:
Dependencies Added:
@supabase/supabase-js,drizzle-orm,drizzle-kit,pg,uuid,dotenv.Environment Configuration:
.env.localand.env.local.examplefor Supabase and database connections.Drizzle ORM & Schema:
drizzle.config.ts).lib/db/schema.ts) forusers,chats,and
messagestables.Client Initialization:
lib/db/index.ts.lib/supabase/client.ts.Authentication Helper:
lib/auth/get-current-user.tswithgetCurrentUserId.Database Migrations:
lib/db/migrate.tsand adb:migratescript inpackage.json.Chat Database Actions:
lib/actions/chat-db.tsusing Drizzle.
API Routes:
app/api/chats/route.ts(GET) andapp/api/chat/route.ts(POST) to interface with the new chat backend.generateUUIDutility.This work completes the backend setup for your chat system. The next phase would involve updating frontend components to consume these new services.
PR Type
Enhancement
Description
• Integrate Supabase and Drizzle ORM for chat backend
• Add database schema for users, chats, messages
• Create API routes for chat operations
• Implement authentication and database migration utilities
Changes walkthrough 📝
6 files
Create POST endpoint for new chatsCreate GET endpoint for chat paginationImplement chat CRUD operations with DrizzleAdd Supabase authentication helper functionsDefine database schema for chat systemAdd UUID generation utility function8 files
Configure Drizzle Kit for PostgreSQLInitialize Drizzle database clientCreate database migration runner scriptInitialize Supabase client for authenticationAdd Supabase environment variable examplesGenerate initial database migration SQLCreate migration metadata snapshotInitialize migration journal tracking1 files
Add Supabase and Drizzle dependencies1 files
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
Removed