Chat local first - #300
Conversation
|
🚅 Deployed to the conar-pr-300 environment in Conar
|
There was a problem hiding this comment.
Pull request overview
This pull request implements a local-first approach for the chat feature, shifting from a server-first model to a client-first architecture with background synchronization. The changes include refactoring the chat storage to use local collections that sync to the server, adding new API endpoints for CRUD operations on chats and messages, introducing a new streaming chat endpoint, and updating the database schema to enforce data integrity.
Changes:
- Implemented local-first chat architecture with client-side collections and background sync
- Added new API endpoints for chat/message CRUD operations (create, update, remove) with batch support
- Introduced new
ai.chatendpoint with improved streaming and replaced server-side resumable streams - Updated pnpm version to 10.28.0 across all workflows and configurations
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated pnpm version to 10.28.0 |
| apps/desktop/todesktop.json | Updated pnpm version to 10.28.0 |
| apps/desktop/src/routes/_protected/database/$id/sql/-components/chat/index.ts | Refactored to use local-first chat with await for persistence and new chat endpoint |
| apps/desktop/src/routes/_protected/database/$id/sql/-components/chat/chat-messages.tsx | Removed fallback model retry button |
| apps/desktop/src/routes/_protected/database/$id/sql/-components/chat/chat-header.tsx | Updated to use metadata for sync control in title generation |
| apps/desktop/src/routes/_protected/-components/databases-list.tsx | Improved code formatting for class names |
| apps/desktop/src/entities/chat/sync.ts | Added onInsert/onUpdate/onDelete hooks for local-first sync with metadata filtering |
| apps/desktop/src/drizzle/schema/chats.ts | Added NOT NULL constraint to databaseId column |
| apps/desktop/src/drizzle/migrations/* | Added migration for NOT NULL constraint |
| apps/api/src/orpc/routers/webhooks/index.ts | Refactored to use direct exports instead of object wrapper |
| apps/api/src/orpc/routers/queries/index.ts | Refactored to use direct exports instead of object wrapper |
| apps/api/src/orpc/routers/index.ts | Updated imports to use wildcard imports for refactored routers |
| apps/api/src/orpc/routers/databases/update.ts | Moved authorization check to WHERE clause, omitted more fields from update schema |
| apps/api/src/orpc/routers/databases/index.ts | Refactored to use direct exports instead of object wrapper |
| apps/api/src/orpc/routers/databases/create.ts | Removed return value, changed to requireSubscriptionMiddleware |
| apps/api/src/orpc/routers/chats/update.ts | New endpoint for updating chat metadata |
| apps/api/src/orpc/routers/chats/remove.ts | Updated to support batch deletion with array input |
| apps/api/src/orpc/routers/chats/index.ts | Refactored exports, removed get endpoint, added create and update |
| apps/api/src/orpc/routers/chats/get.ts | Removed file (endpoint deleted) |
| apps/api/src/orpc/routers/chats/create.ts | New endpoint for creating chats |
| apps/api/src/orpc/routers/chats-messages/update.ts | New endpoint for updating chat messages |
| apps/api/src/orpc/routers/chats-messages/remove.ts | New endpoint for batch deleting chat messages |
| apps/api/src/orpc/routers/chats-messages/index.ts | Added create, update, remove exports |
| apps/api/src/orpc/routers/chats-messages/create.ts | New endpoint for creating/upserting chat messages |
| apps/api/src/orpc/routers/ai/resume-stream.ts | Removed file (resumable streams removed) |
| apps/api/src/orpc/routers/ai/index.ts | Removed resumeStream, added chat endpoint, renamed ask to ask__legacy |
| apps/api/src/orpc/routers/ai/enhance-prompt.ts | Updated import to use ask__legacy |
| apps/api/src/orpc/routers/ai/chat.ts | New streaming chat endpoint with local-first approach |
| apps/api/src/orpc/routers/ai/ask__legacy.ts | Renamed from ask.ts, removed resumable stream logic |
| apps/api/src/orpc/routers/account/index.ts | Refactored to use direct exports instead of object wrapper |
| apps/api/src/orpc/index.ts | Split getSubscription into cached and uncached versions |
| apps/api/src/drizzle/schema/chats.ts | Added insert and update schemas |
| .github/workflows/release.yml | Updated pnpm version to 10.28.0 |
| .github/workflows/lint-check.yml | Updated pnpm version to 10.28.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1 @@ | |||
| ALTER TABLE "chats" ALTER COLUMN "database_id" SET NOT NULL; No newline at end of file | |||
There was a problem hiding this comment.
Adding a NOT NULL constraint to an existing column could fail if there are existing rows with NULL values in the database_id column. The migration should either include a data migration step to populate NULL values or use a safer approach like creating a new column and backfilling data.
| ALTER TABLE "chats" ALTER COLUMN "database_id" SET NOT NULL; | |
| DO $$ | |
| BEGIN | |
| -- Only set NOT NULL on "database_id" if there are no existing NULL values. | |
| IF EXISTS (SELECT 1 FROM "chats" WHERE "database_id" IS NULL) THEN | |
| RAISE NOTICE 'NOT NULL constraint on chats.database_id was not applied because NULL values exist. Please backfill and add the constraint in a follow-up migration.'; | |
| ELSE | |
| ALTER TABLE "chats" ALTER COLUMN "database_id" SET NOT NULL; | |
| END IF; | |
| END; | |
| $$; |
| `You are an SQL tool that generates valid SQL code for ${type} database.`, | ||
| 'You can use several tools to improve response.', | ||
| 'You can generate select queries using the tools to get data directly from the database.', | ||
| 'You can also search the web for information when the user asks about external resources, provides URLs, or needs current information beyond the database schema.', | ||
| '', | ||
| 'Requirements:', | ||
| `- Ensure the SQL is 100% valid and optimized for ${type} database`, |
There was a problem hiding this comment.
The template string references a variable 'type' but the input parameter is named 'type', not 'input.type'. This will cause a reference error. The variable should be 'input.type' instead of just 'type'.
| `You are an SQL tool that generates valid SQL code for ${type} database.`, | |
| 'You can use several tools to improve response.', | |
| 'You can generate select queries using the tools to get data directly from the database.', | |
| 'You can also search the web for information when the user asks about external resources, provides URLs, or needs current information beyond the database schema.', | |
| '', | |
| 'Requirements:', | |
| `- Ensure the SQL is 100% valid and optimized for ${type} database`, | |
| `You are an SQL tool that generates valid SQL code for ${input.type} database.`, | |
| 'You can use several tools to improve response.', | |
| 'You can generate select queries using the tools to get data directly from the database.', | |
| 'You can also search the web for information when the user asks about external resources, provides URLs, or needs current information beyond the database schema.', | |
| '', | |
| 'Requirements:', | |
| `- Ensure the SQL is 100% valid and optimized for ${input.type} database`, |
| await db.insert(chatsMessages).values({ | ||
| ...result.responseMessage, | ||
| updatedAt: result.responseMessage.metadata?.updatedAt, | ||
| chatId: input.id, | ||
| }).onConflictDoUpdate({ | ||
| target: chatsMessages.id, | ||
| set: { | ||
| ...result.responseMessage, |
There was a problem hiding this comment.
The onConflictDoUpdate is setting all fields from responseMessage including the chatId in the conflict set, but chatId should not be updated on conflict. The set object should omit chatId and only update the message content and metadata.
| await db.insert(chatsMessages).values({ | |
| ...result.responseMessage, | |
| updatedAt: result.responseMessage.metadata?.updatedAt, | |
| chatId: input.id, | |
| }).onConflictDoUpdate({ | |
| target: chatsMessages.id, | |
| set: { | |
| ...result.responseMessage, | |
| const { chatId: _ignoredChatId, ...responseMessageWithoutChatId } = result.responseMessage | |
| await db.insert(chatsMessages).values({ | |
| ...responseMessageWithoutChatId, | |
| updatedAt: result.responseMessage.metadata?.updatedAt, | |
| chatId: input.id, | |
| }).onConflictDoUpdate({ | |
| target: chatsMessages.id, | |
| set: { | |
| ...responseMessageWithoutChatId, |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…atabase operations
Description of Changes
Change chat to local-first approach