POC: improve chat db #2830
POC: improve chat db #2830
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds subchat support and message parts persistence across DB schema, mappers, and APIs. Introduces conversations’ parent references, a new parts table, and typed UI message/part models. Updates server routes to create/query subchats, persists streamed assistant messages’ parts, and adjusts clients/tests/types accordingly. Includes a migrations snapshot and documentation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Client UI
participant Route as /api/chat (route.ts)
participant Stream as AI Stream
participant Helper as saveMessageWithParts
participant DB as DB (messages, parts)
UI->>Route: POST /api/chat (start stream)
Route->>Stream: Generate assistant response
Stream-->>Route: Stream parts (text/tool/data...)
Note over Route: Accumulates parts in responseMessage
Stream-->>Route: onFinish()
Route->>Helper: saveMessageWithParts(responseMessage, conversationId)
Helper->>DB: BEGIN
Helper->>DB: Upsert messages(row)
alt has parts
Helper->>DB: Delete parts WHERE messageId
Helper->>DB: Insert parts (ordered)
end
DB-->>Helper: COMMIT
Helper-->>Route: Saved {id, partsCount}
Route-->>UI: Stream closed (200)
sequenceDiagram
autonumber
participant Client as Client
participant API as tRPC conversation router
participant DB as DB (conversations, messages)
rect rgba(200,230,255,0.2)
Note right of Client: Create Subchat
Client->>API: createSubchat(projectId, parentConversationId, parentMessageId?, displayName?)
API->>DB: Validate parent conversation (project match)
alt parentMessageId provided
API->>DB: Validate message belongs to parentConversation
end
API->>DB: Insert conversation { projectId, parentConversationId, parentMessageId, ... }
DB-->>API: New subchat row
API-->>Client: Subchat
end
rect rgba(220,255,220,0.2)
Note right of Client: Fetch Tree
Client->>API: getConversationTree(conversationId)
API->>DB: Query conversation + children (recursive)
API-->>Client: Tree { subchats[...] }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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 Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
|
|
Updates to Preview Branch (feat/improve-chat-db) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| ALTER TABLE "conversations" ADD CONSTRAINT "conversations_parent_conversation_id_conversations_id_fk" FOREIGN KEY ("parent_conversation_id") REFERENCES "public"."conversations"("id") ON DELETE cascade ON UPDATE cascade;--> statement-breakpoint | ||
| ALTER TABLE "conversations" ADD CONSTRAINT "conversations_parent_message_id_messages_id_fk" FOREIGN KEY ("parent_message_id") REFERENCES "public"."messages"("id") ON DELETE cascade ON UPDATE cascade;--> statement-breakpoint |
There was a problem hiding this comment.
Circular dependency vulnerability: The foreign key constraints create a potential circular reference where conversations can reference messages, and messages reference conversations, but now conversations can also reference messages through parent_message_id. This could lead to cascading delete loops or constraint violation errors during deletion operations. Consider adding additional validation or restructuring the relationships to prevent circular references.
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| // Delete existing parts for this message | ||
| await trx.delete(parts).where(eq(parts.messageId, message.id)); |
There was a problem hiding this comment.
Race condition bug: The delete operation followed immediately by insert creates a race condition. If the delete fails or is still processing when the insert executes, you could end up with duplicate parts or constraint violations. The operations should be wrapped in proper error handling and potentially use a single UPSERT operation instead.
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| textRequired: check( | ||
| "text_text_required_if_type_is_text", | ||
| sql`CASE WHEN ${table.type} = 'text' THEN ${table.text_text} IS NOT NULL ELSE TRUE END` | ||
| ), | ||
|
|
||
| reasoningRequired: check( | ||
| "reasoning_text_required_if_type_is_reasoning", | ||
| sql`CASE WHEN ${table.type} = 'reasoning' THEN ${table.reasoning_text} IS NOT NULL ELSE TRUE END` | ||
| ), | ||
|
|
||
| fileFieldsRequired: check( | ||
| "file_fields_required_if_type_is_file", | ||
| sql`CASE WHEN ${table.type} = 'file' THEN ${table.file_mediaType} IS NOT NULL AND ${table.file_url} IS NOT NULL ELSE TRUE END` | ||
| ), | ||
|
|
||
| sourceUrlFieldsRequired: check( | ||
| "source_url_fields_required_if_type_is_source_url", | ||
| sql`CASE WHEN ${table.type} = 'source_url' THEN ${table.source_url_sourceId} IS NOT NULL AND ${table.source_url_url} IS NOT NULL ELSE TRUE END` | ||
| ), | ||
|
|
||
| sourceDocumentFieldsRequired: check( | ||
| "source_document_fields_required_if_type_is_source_document", | ||
| sql`CASE WHEN ${table.type} = 'source_document' THEN ${table.source_document_sourceId} IS NOT NULL AND ${table.source_document_mediaType} IS NOT NULL AND ${table.source_document_title} IS NOT NULL ELSE TRUE END` | ||
| ), | ||
|
|
||
| // Tool call constraints - ensure tool calls have required fields | ||
| toolCallFieldsRequired: check( | ||
| "tool_call_fields_required", | ||
| sql`CASE WHEN ${table.type} LIKE 'tool-%' THEN ${table.tool_toolCallId} IS NOT NULL AND ${table.tool_state} IS NOT NULL ELSE TRUE END` | ||
| ), |
There was a problem hiding this comment.
Logic error in check constraints: The check constraints use CASE statements that could fail with complex tool types or data types. For example, if a tool type contains special characters or doesn't match the expected pattern, the constraint could incorrectly validate. The LIKE 'tool-%' pattern matching in line 143 is particularly vulnerable to false positives. Consider using more specific validation logic or enum-based type checking.
| textRequired: check( | |
| "text_text_required_if_type_is_text", | |
| sql`CASE WHEN ${table.type} = 'text' THEN ${table.text_text} IS NOT NULL ELSE TRUE END` | |
| ), | |
| reasoningRequired: check( | |
| "reasoning_text_required_if_type_is_reasoning", | |
| sql`CASE WHEN ${table.type} = 'reasoning' THEN ${table.reasoning_text} IS NOT NULL ELSE TRUE END` | |
| ), | |
| fileFieldsRequired: check( | |
| "file_fields_required_if_type_is_file", | |
| sql`CASE WHEN ${table.type} = 'file' THEN ${table.file_mediaType} IS NOT NULL AND ${table.file_url} IS NOT NULL ELSE TRUE END` | |
| ), | |
| sourceUrlFieldsRequired: check( | |
| "source_url_fields_required_if_type_is_source_url", | |
| sql`CASE WHEN ${table.type} = 'source_url' THEN ${table.source_url_sourceId} IS NOT NULL AND ${table.source_url_url} IS NOT NULL ELSE TRUE END` | |
| ), | |
| sourceDocumentFieldsRequired: check( | |
| "source_document_fields_required_if_type_is_source_document", | |
| sql`CASE WHEN ${table.type} = 'source_document' THEN ${table.source_document_sourceId} IS NOT NULL AND ${table.source_document_mediaType} IS NOT NULL AND ${table.source_document_title} IS NOT NULL ELSE TRUE END` | |
| ), | |
| // Tool call constraints - ensure tool calls have required fields | |
| toolCallFieldsRequired: check( | |
| "tool_call_fields_required", | |
| sql`CASE WHEN ${table.type} LIKE 'tool-%' THEN ${table.tool_toolCallId} IS NOT NULL AND ${table.tool_state} IS NOT NULL ELSE TRUE END` | |
| ), | |
| textRequired: check( | |
| "text_text_required_if_type_is_text", | |
| sql`CASE WHEN ${table.type} = 'text' THEN ${table.text_text} IS NOT NULL ELSE TRUE END` | |
| ), | |
| reasoningRequired: check( | |
| "reasoning_text_required_if_type_is_reasoning", | |
| sql`CASE WHEN ${table.type} = 'reasoning' THEN ${table.reasoning_text} IS NOT NULL ELSE TRUE END` | |
| ), | |
| fileFieldsRequired: check( | |
| "file_fields_required_if_type_is_file", | |
| sql`CASE WHEN ${table.type} = 'file' THEN ${table.file_mediaType} IS NOT NULL AND ${table.file_url} IS NOT NULL ELSE TRUE END` | |
| ), | |
| sourceUrlFieldsRequired: check( | |
| "source_url_fields_required_if_type_is_source_url", | |
| sql`CASE WHEN ${table.type} = 'source_url' THEN ${table.source_url_sourceId} IS NOT NULL AND ${table.source_url_url} IS NOT NULL ELSE TRUE END` | |
| ), | |
| sourceDocumentFieldsRequired: check( | |
| "source_document_fields_required_if_type_is_source_document", | |
| sql`CASE WHEN ${table.type} = 'source_document' THEN ${table.source_document_sourceId} IS NOT NULL AND ${table.source_document_mediaType} IS NOT NULL AND ${table.source_document_title} IS NOT NULL ELSE TRUE END` | |
| ), | |
| // Tool call constraints - ensure tool calls have required fields | |
| toolCallFieldsRequired: check( | |
| "tool_call_fields_required", | |
| sql`CASE WHEN ${table.type} ~ '^tool-[a-zA-Z0-9_-]+$' THEN ${table.tool_toolCallId} IS NOT NULL AND ${table.tool_state} IS NOT NULL ELSE TRUE END` | |
| ), |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| // Get the root conversation and all its descendants | ||
| const buildTree = async (parentId: string): Promise<any> => { | ||
| const conversation = await ctx.db.query.conversations.findFirst({ | ||
| where: eq(conversations.id, parentId), | ||
| }); | ||
|
|
||
| if (!conversation) { | ||
| return null; | ||
| } | ||
|
|
||
| const subchats = await ctx.db.query.conversations.findMany({ | ||
| where: eq(conversations.parentConversationId, parentId), | ||
| orderBy: (conversations, { asc }) => [asc(conversations.createdAt)], | ||
| }); | ||
|
|
||
| const subchatTrees = await Promise.all( | ||
| subchats.map(subchat => buildTree(subchat.id)) | ||
| ); | ||
|
|
||
| return { | ||
| ...fromDbConversation(conversation), | ||
| subchats: subchatTrees.filter(Boolean) | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Infinite recursion vulnerability: The buildTree function recursively calls itself without proper depth limiting or cycle detection. If there's a circular reference in the conversation hierarchy (despite foreign key constraints), this could cause a stack overflow. Add a maximum depth parameter and cycle detection to prevent infinite recursion.
| // Get the root conversation and all its descendants | |
| const buildTree = async (parentId: string): Promise<any> => { | |
| const conversation = await ctx.db.query.conversations.findFirst({ | |
| where: eq(conversations.id, parentId), | |
| }); | |
| if (!conversation) { | |
| return null; | |
| } | |
| const subchats = await ctx.db.query.conversations.findMany({ | |
| where: eq(conversations.parentConversationId, parentId), | |
| orderBy: (conversations, { asc }) => [asc(conversations.createdAt)], | |
| }); | |
| const subchatTrees = await Promise.all( | |
| subchats.map(subchat => buildTree(subchat.id)) | |
| ); | |
| return { | |
| ...fromDbConversation(conversation), | |
| subchats: subchatTrees.filter(Boolean) | |
| }; | |
| }; | |
| // Get the root conversation and all its descendants | |
| const buildTree = async ( | |
| parentId: string, | |
| maxDepth: number = 10, | |
| visitedIds: Set<string> = new Set() | |
| ): Promise<any> => { | |
| // Stop recursion if max depth reached or if we've seen this ID before (cycle detection) | |
| if (maxDepth <= 0 || visitedIds.has(parentId)) { | |
| return null; | |
| } | |
| // Mark this ID as visited | |
| visitedIds.add(parentId); | |
| const conversation = await ctx.db.query.conversations.findFirst({ | |
| where: eq(conversations.id, parentId), | |
| }); | |
| if (!conversation) { | |
| return null; | |
| } | |
| const subchats = await ctx.db.query.conversations.findMany({ | |
| where: eq(conversations.parentConversationId, parentId), | |
| orderBy: (conversations, { asc }) => [asc(conversations.createdAt)], | |
| }); | |
| const subchatTrees = await Promise.all( | |
| subchats.map(subchat => buildTree(subchat.id, maxDepth - 1, new Set(visitedIds))) | |
| ); | |
| return { | |
| ...fromDbConversation(conversation), | |
| subchats: subchatTrees.filter(Boolean) | |
| }; | |
| }; |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| const toolName = type.replace("tool-", ""); | ||
| const inputKey = `tool_${toolName}_input` as keyof DbPart; | ||
| const outputKey = `tool_${toolName}_output` as keyof DbPart; | ||
|
|
There was a problem hiding this comment.
Type safety violation: The code uses string manipulation to construct property keys (tool_${toolName}_input) and then accesses them with as keyof DbPart. This bypasses TypeScript's type checking and could result in accessing undefined properties at runtime if the tool name doesn't match the expected database columns. This could cause runtime errors when trying to access non-existent properties.
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| <h2 className="text-2xl font-light text-foreground-primary mt-12 mb-6">12. Miscellaneous</h2> | ||
| <p className="text-foreground-secondary mb-6"> | ||
| This Agreement (including the Registration) represents the entire agreement between User and Onlook with respect to the subject matter hereof, and supersedes all prior or contemporaneous communications and proposals (whether oral, written or electronic) between User and Onlook with respect thereto. In the event of any conflict between these Terms and the Registration, the Registration shall control. The Agreement shall be governed by and construed in accordance with the laws of the State of New York, excluding its conflicts of law rules, and the parties consent to exclusive jurisdiction and venue in the state and federal courts located in New York, NY. All notices under this Agreement shall be in writing and shall be deemed to have been duly given when received, if personally delivered or sent by certified or registered mail, return receipt requested; when receipt is electronically confirmed, if transmitted by facsimile or e-mail; or the day after it is sent, if sent for next day delivery by recognized overnight delivery service. Notices must be sent to the contacts for each party set forth in the Registration. Either party may update its address set forth above by giving notice in accordance with this section. Neither party may assign any of its rights or obligations hereunder without the other party's consent; provided that either party may assign all of its rights and obligations hereunder without such consent to a successor-in-interest in connection with a sale of substantially all of such party's business relating to this Agreement. No agency, partnership, joint venture, or employment relationship is created as a result of this Agreement and neither party has any authority of any kind to bind the other in any respect. In any action or proceeding to enforce rights under this Agreement, the prevailing party shall be entitled to recover costs and attorneys' fees. If any provision of this Agreement is held to be unenforceable for any reason, such provision shall be reformed only to the extent necessary to make it enforceable. The failure of either party to act with respect to a breach of this Agreement by the other party shall not constitute a waiver and shall not limit such party's rights with respect to such breach or any subsequent breaches. | ||
| This Agreement (including the Registration) represents the entire agreement between User and Onlook with respect to the subject matter hereof, and supersedes all prior or contemporaneous communications and proposals (whether oral, written or electronic) between User and Onlook with respect thereto. In the event of any conflict between these Terms and the Registration, the Registration shall control. The Agreement shall be governed by and construed in accordance with the laws of the State of New York, excluding its conflicts of law rules, and the parties consent to exclusive jurisdiction and venue in the state and federal courts located in New York, NY. All notices under this Agreement shall be in writing and shall be deemed to have been duly given when received, if personally delivered or sent by certified or registered mail, return receipt requested; when receipt is electronically confirmed, if transmitted by facsimile or e-mail; or the day after it is sent, if sent for next day delivery by recognized overnight delivery service. Notices must be sent to the contacts for each party set forth in the Registration. Either party may update its address set forth above by giving notice in accordance with this section. Neither party may assign any of its rights or obligations hereunder without the other party's consent; provided that either party may assign all of its rights and obligations hereunder without such consent to a successor-in-interest in connection with a sale of substantially all of such party's business relating to this Agreement. No agency, partnership, joint venture, or employment relationship is created as a result of this Agreement and neither party h authority of any kind to bind the other in any respect. In any action or proceeding to enforce rights under this Agreement, the prevailing party shall be entitled to recover costs and attorneys' fees. If any provision of this Agreement is held to be unenforceable for any reason, such provision shall be reformed only to the extent necessary to make it enforceable. The failure of either party to act with respect to a breach of this Agreement by the other party shall not constitute a waiver and shall not limit such party's rights with respect to such breach or any subsequent breaches. |
There was a problem hiding this comment.
Typo: It appears that 'neither party h authority' is missing the word 'has'. It should read 'neither party has any authority' to ensure correct grammar.
| This Agreement (including the Registration) represents the entire agreement between User and Onlook with respect to the subject matter hereof, and supersedes all prior or contemporaneous communications and proposals (whether oral, written or electronic) between User and Onlook with respect thereto. In the event of any conflict between these Terms and the Registration, the Registration shall control. The Agreement shall be governed by and construed in accordance with the laws of the State of New York, excluding its conflicts of law rules, and the parties consent to exclusive jurisdiction and venue in the state and federal courts located in New York, NY. All notices under this Agreement shall be in writing and shall be deemed to have been duly given when received, if personally delivered or sent by certified or registered mail, return receipt requested; when receipt is electronically confirmed, if transmitted by facsimile or e-mail; or the day after it is sent, if sent for next day delivery by recognized overnight delivery service. Notices must be sent to the contacts for each party set forth in the Registration. Either party may update its address set forth above by giving notice in accordance with this section. Neither party may assign any of its rights or obligations hereunder without the other party's consent; provided that either party may assign all of its rights and obligations hereunder without such consent to a successor-in-interest in connection with a sale of substantially all of such party's business relating to this Agreement. No agency, partnership, joint venture, or employment relationship is created as a result of this Agreement and neither party h authority of any kind to bind the other in any respect. In any action or proceeding to enforce rights under this Agreement, the prevailing party shall be entitled to recover costs and attorneys' fees. If any provision of this Agreement is held to be unenforceable for any reason, such provision shall be reformed only to the extent necessary to make it enforceable. The failure of either party to act with respect to a breach of this Agreement by the other party shall not constitute a waiver and shall not limit such party's rights with respect to such breach or any subsequent breaches. | |
| This Agreement (including the Registration) represents the entire agreement between User and Onlook with respect to the subject matter hereof, and supersedes all prior or contemporaneous communications and proposals (whether oral, written or electronic) between User and Onlook with respect thereto. In the event of any conflict between these Terms and the Registration, the Registration shall control. The Agreement shall be governed by and construed in accordance with the laws of the State of New York, excluding its conflicts of law rules, and the parties consent to exclusive jurisdiction and venue in the state and federal courts located in New York, NY. All notices under this Agreement shall be in writing and shall be deemed to have been duly given when received, if personally delivered or sent by certified or registered mail, return receipt requested; when receipt is electronically confirmed, if transmitted by facsimile or e-mail; or the day after it is sent, if sent for next day delivery by recognized overnight delivery service. Notices must be sent to the contacts for each party set forth in the Registration. Either party may update its address set forth above by giving notice in accordance with this section. Neither party may assign any of its rights or obligations hereunder without the other party's consent; provided that either party may assign all of its rights and obligations hereunder without such consent to a successor-in-interest in connection with a sale of substantially all of such party's business relating to this Agreement. No agency, partnership, joint venture, or employment relationship is created as a result of this Agreement and neither party has any authority of any kind to bind the other in any respect. In any action or proceeding to enforce rights under this Agreement, the prevailing party shall be entitled to recover costs and attorneys' fees. If any provision of this Agreement is held to be unenforceable for any reason, such provision shall be reformed only to the extent necessary to make it enforceable. The failure of either party to act with respect to a breach of this Agreement by the other party shall not constitute a waiver and shall not limit such party's rights with respect to such breach or any subsequent breaches. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/web/client/src/components/store/editor/pages/helper.ts (1)
841-847: Bug: generates a runtime import for a type.You’re adding import { Metadata } from 'next' as a value import. This should be a type-only import to avoid runtime/bundler errors.
Minimal fix:
- const metadataImport = t.importDeclaration( + const metadataImport = t.importDeclaration( [t.importSpecifier(t.identifier('Metadata'), t.identifier('Metadata'))], t.stringLiteral('next'), ); + // Mark as type-only import + (metadataImport as any).importKind = 'type';Optionally also set the specifier kind:
((metadataImport.specifiers[0]) as any).importKind = 'type';apps/web/client/src/server/api/routers/domain/verify/helpers/freestyle.ts (1)
1-20: Replace deep imports from '@onlook/db/src/client' with the public '@onlook/db' entrypointUpdate imports to use either
import { db } from '@onlook/db'orimport type { DrizzleDb } from '@onlook/db'in these files:
- packages/db/src/seed/db.ts
- packages/db/src/seed/stripe/stripe.ts
- packages/db/src/seed/stripe/legacy-subscription.ts
- packages/db/src/seed/stripe/backfill-subscriptions.ts
- apps/web/client/src/trpc/request-server.ts
- apps/web/client/src/server/api/trpc.ts
- apps/web/client/src/server/api/routers/publish/helpers/helpers.ts
- apps/web/client/src/server/api/routers/publish/helpers/unpublish.ts
- apps/web/client/src/server/api/routers/publish/helpers/publish.ts
- apps/web/client/src/server/api/routers/domain/verify/helpers/helpers.ts
- apps/web/client/src/server/api/routers/publish/helpers/deploy.ts
- apps/web/client/src/app/webhook/stripe/subscription/delete.ts
- apps/web/client/src/app/webhook/stripe/subscription/update.ts
- apps/web/client/src/app/webhook/stripe/subscription/create.ts
- apps/web/client/src/app/api/chat/helpers/persistence.ts
packages/db/src/mappers/chat/conversation.ts (2)
5-12: Incorrect mapping: title not set; leaking DB-only fields into model.fromDbConversation spreads DbConversation, which includes displayName and suggestions. ChatConversation expects title and metadata.suggestions. Map explicitly.
-export const fromDbConversation = (conversation: DbConversation): ChatConversation => { - return { - ...conversation, - metadata: { - suggestions: conversation.suggestions || [], - }, - parentConversationId: conversation.parentConversationId || undefined, - parentMessageId: conversation.parentMessageId || undefined, - } -} +export const fromDbConversation = (conversation: DbConversation): ChatConversation => { + return { + id: conversation.id, + title: conversation.displayName ?? undefined, + projectId: conversation.projectId, + createdAt: conversation.createdAt, + updatedAt: conversation.updatedAt, + metadata: { suggestions: conversation.suggestions ?? [] }, + parentConversationId: conversation.parentConversationId ?? undefined, + parentMessageId: conversation.parentMessageId ?? undefined, + }; +};
15-23: Don’t spread model into DbConversation; write an explicit projection.Spreading ChatConversation risks passing model-only fields (title, metadata) into the DB layer.
-export const toDbConversation = (conversation: ChatConversation): DbConversation => { - return { - ...conversation, - projectId: conversation.projectId, - displayName: conversation.title || null, - suggestions: conversation.metadata?.suggestions || [], - parentConversationId: conversation.parentConversationId || null, - parentMessageId: conversation.parentMessageId || null, - } -} +export const toDbConversation = (conversation: ChatConversation): DbConversation => { + return { + id: conversation.id, + projectId: conversation.projectId, + displayName: conversation.title ?? null, + createdAt: conversation.createdAt, + updatedAt: conversation.updatedAt, + suggestions: conversation.metadata?.suggestions ?? [], + parentConversationId: conversation.parentConversationId ?? null, + parentMessageId: conversation.parentMessageId ?? null, + }; +};apps/web/client/src/server/api/routers/chat/conversation.ts (1)
49-53: Broken update call: incorrect Drizzle update usage; updatedAt not set.ctx.db.update expects a table, not a spread object. Also avoid attempting to set id inadvertently.
- const [conversation] = await ctx.db.update({ - ...conversations, - updatedAt: new Date(), - }).set(input) - .where(eq(conversations.id, input.id)).returning(); + const { id, ...update } = input; + const [conversation] = await ctx.db + .update(conversations) + .set({ ...update, updatedAt: new Date() }) + .where(eq(conversations.id, id)) + .returning();
♻️ Duplicate comments (6)
apps/backend/supabase/migrations/0019_cuddly_iron_man.sql (1)
4-5: Potential circular reference issue in foreign key relationships.The foreign key constraints create a circular reference pattern where conversations → messages (via parent_message_id) and messages → conversations (via conversation_id). This could lead to cascading delete loops or constraint violations.
Consider adding deferred constraint checking or restructuring the relationships to prevent circular references. For example, you could create a separate junction table for subchat relationships or add application-level checks to prevent circular parent references.
apps/web/client/src/app/api/chat/helpers/persistence.ts (1)
66-80: Address potential race condition in delete-insert pattern.The delete operation followed immediately by insert creates a race condition. If the delete fails or is still processing when the insert executes, you could end up with duplicate parts or constraint violations.
Consider using an UPSERT pattern with composite keys or ensuring proper error handling:
if (typedMessage.parts && typedMessage.parts.length > 0) { - // Delete existing parts for this message - await tx.delete(parts).where(eq(parts.messageId, typedMessage.id)); - - // Convert and insert new parts - const dbParts = typedMessage.parts.map((part, index) => ({ - messageId: typedMessage.id, - type: part.type, - order: index, - createdAt: new Date(), - ...convertUIPartToDbFields(part) - })); - - if (dbParts.length > 0) { - await tx.insert(parts).values(dbParts); - } + try { + // Delete existing parts for this message + await tx.delete(parts).where(eq(parts.messageId, typedMessage.id)); + + // Convert and insert new parts + const dbParts = typedMessage.parts.map((part, index) => ({ + messageId: typedMessage.id, + type: part.type, + order: index, + createdAt: new Date(), + ...convertUIPartToDbFields(part) + })); + + if (dbParts.length > 0) { + await tx.insert(parts).values(dbParts); + } + } catch (error) { + // Log the error and re-throw to ensure transaction rollback + console.error('Failed to update message parts:', error); + throw error; + } }apps/web/client/src/server/api/routers/chat/conversation.ts (1)
143-174: Add depth limit and cycle detection to tree builder.Recursive buildTree can loop indefinitely on cycles; also risks deep recursion. Add a visited set and maxDepth.
- const buildTree = async (parentId: string): Promise<any> => { + const buildTree = async ( + parentId: string, + maxDepth: number = 10, + visited: Set<string> = new Set(), + ): Promise<any> => { + if (maxDepth <= 0 || visited.has(parentId)) { + return null; + } + visited.add(parentId); const conversation = await ctx.db.query.conversations.findFirst({ where: eq(conversations.id, parentId), }); @@ - const subchatTrees = await Promise.all( - subchats.map(subchat => buildTree(subchat.id)) - ); + const subchatTrees = await Promise.all( + subchats.map(subchat => buildTree(subchat.id, maxDepth - 1, new Set(visited))) + ); @@ - return await buildTree(input.conversationId); + return await buildTree(input.conversationId);apps/web/client/src/server/api/routers/chat/message.ts (1)
145-197: Potential runtime error with dynamic property access.The
getPartFieldsfunction constructs dynamic property keys that may not exist in the database schema. This could lead to runtime errors or silent data loss if tool names don't match expected columns.Consider validating tool names against a predefined list:
const getPartFields = (part: any) => { const fields: any = { providerMetadata: part.providerMetadata }; switch (part.type) { case "text": fields.text_text = part.text; break; case "reasoning": fields.reasoning_text = part.text; break; case "file": fields.file_mediaType = part.mediaType; fields.file_filename = part.filename; fields.file_url = part.url; break; case "source_url": fields.source_url_sourceId = part.sourceId; fields.source_url_url = part.url; fields.source_url_title = part.title; break; case "source_document": fields.source_document_sourceId = part.sourceId; fields.source_document_mediaType = part.mediaType; fields.source_document_title = part.title; fields.source_document_filename = part.filename; break; default: // Handle tool calls if (part.type.startsWith("tool-")) { const toolName = part.type.replace("tool-", ""); + // Validate tool name against known tools + const knownTools = [ + 'listFiles', 'readFile', 'bashRead', 'onlookInstructions', + 'readStyleGuide', 'listBranches', 'scrapeUrl', 'webSearch', + 'glob', 'grep', 'searchReplaceEditFile', 'searchReplaceMultiEditFile', + 'fuzzyEditFile', 'writeFile', 'bashEdit', 'sandbox', + 'terminalCommand', 'typecheck' + ]; + if (!knownTools.includes(toolName)) { + console.warn(`Unknown tool: ${toolName}`); + return fields; + } fields.tool_toolCallId = part.toolCallId; fields.tool_state = part.state; fields.tool_errorText = part.errorText; fields[`tool_${toolName}_input`] = part.input; fields[`tool_${toolName}_output`] = part.output; } // Handle data parts else if (part.type.startsWith("data-")) { const dataType = part.type.replace("data-", ""); if (dataType === "weather") { fields.data_weather_id = part.id; fields.data_weather_location = part.weather?.location; fields.data_weather_weather = part.weather?.weather; fields.data_weather_temperature = part.weather?.temperature; } } break; } return fields; };packages/db/src/schema/chat/parts.ts (1)
151-151: Weak pattern matching for tool types.The LIKE 'tool-%' pattern is vulnerable to false positives with special characters in tool names. Consider using a more specific regex pattern for validation.
packages/db/src/mappers/chat/parts.ts (1)
57-58: Type-unsafe dynamic property access.Using string manipulation to construct property keys and casting them as
keyof DbPartbypasses TypeScript's type checking. This could cause runtime errors if the tool name doesn't correspond to existing database columns.
🧹 Nitpick comments (35)
apps/web/client/src/components/store/editor/sandbox/terminal.ts (1)
176-180: Use originalWrite for control-sequence emissions to avoid re-entering the override.Calling
terminal.write(...)inside the override re-invokes the wrapper (extra overhead, harder to maintain). PreferoriginalWrite(...)for the clear/move commands.- for (let i = 0; i <= lineCount + 2; i++) { - terminal.write('\x1b[2K\x1b[1A\x1b[2K'); - } - terminal.write('\x1b[G'); // Go to beginning of line + for (let i = 0; i <= lineCount + 2; i++) { + originalWrite('\x1b[2K\x1b[1A\x1b[2K'); + } + originalWrite('\x1b[G'); // Go to beginning of lineapps/web/client/src/utils/git/index.test.ts (2)
8-11: Remove stale ESLint disable comments.These lines no longer use
any; theno-explicit-anydisables can be dropped.Apply this diff:
- // eslint-disable-next-line @typescript-eslint/no-explicit-any expect(sanitizeCommitMessage(null as unknown as string)).toBe('Empty commit message'); - // eslint-disable-next-line @typescript-eslint/no-explicit-any expect(sanitizeCommitMessage(undefined as unknown as string)).toBe('Empty commit message');
52-54: Remove stale ESLint disable comments.
Noanyused — remove the three// eslint-disable-next-line @typescript-eslint/no-explicit-anylines in apps/web/client/src/utils/git/index.test.ts (lines 8, 10, 52).apps/web/client/src/app/_components/shared/mockups/tailwind-color-editor.tsx (5)
16-17: Avoid doubleunknown as React.FCcasts for icons; use the typed Icons directly.Removes unsafe casts and simplifies usage.
- const ImageIcon = Icons.Image as unknown as React.FC<React.SVGProps<SVGSVGElement>>; - const CloseIcon = Icons.CrossS as unknown as React.FC<React.SVGProps<SVGSVGElement>>; ... - <ToolbarButton className="flex items-center justify-center w-9 h-9"> - <ImageIcon className="h-4 w-4" /> + <ToolbarButton className="flex items-center justify-center w-9 h-9"> + <Icons.Image className="h-4 w-4" /> </ToolbarButton> ... - <button className="p-1 rounded hover:bg-background-tertiary group"> - <CloseIcon className="w-4 h-4 text-foreground-tertiary group-hover:text-foreground-primary" /> + <button className="p-1 rounded hover:bg-background-tertiary group"> + <Icons.CrossS className="w-4 h-4 text-foreground-tertiary group-hover:text-foreground-primary" /> </button>Also applies to: 174-175, 235-236
25-25: Verify Tailwind tokenh-100; switch to arbitrary value if not in your scale.If not defined in your theme, use
h-[100px]or the intended size token to ensure it’s generated.
117-118: Nonstandard opacity utilities; prefer arbitrary values for consistency.Tailwind’s default opacity scale doesn’t include 45/55/85. Use bracketed values to avoid purging issues.
- className="absolute inset-0 opacity-45" + className="absolute inset-0 opacity-[0.45]" ... - className="absolute w-[60%] h-[55%] left-[10%] bottom-[-10%] rounded-[45%] mix-blend-screen opacity-85" + className="absolute w-[60%] h-[55%] left-[10%] bottom-[-10%] rounded-[45%] mix-blend-screen opacity-[0.85]" ... - className="absolute w-[24%] h-[24%] right-[12%] bottom-[12%] rounded-[50%] mix-blend-screen opacity-55" + className="absolute w-[24%] h-[24%] right-[12%] bottom-[12%] rounded-[50%] mix-blend-screen opacity-[0.55]"Also applies to: 147-149, 152-154
49-61: Move keyframes out of the component tree to avoid injecting duplicate<style>blocks.Define these animations in a CSS file (or a global
<style jsx global>/tailwind plugin). Reduces DOM churn if multiple instances mount.
212-213: Remove unnecessary ESLint disable comment.No
childrenprop is being passed; the rule suppression isn’t needed.- {/* eslint-disable-next-line react/no-children-prop */}apps/web/preload/script/api/style/css-manager.ts (1)
3-3: Avoid casting arrays to css-tree List; construct proper Lists or parse subtrees.Casting with
as unknown as List<CssNode>hides an incompatible runtime shape — this file uses array-style ops (push/filter) on children, so either build a real css-tree List via the library API or callparse(..., { context: 'selectorList' | 'block' })to obtain correctly-typed children.
Locations: apps/web/preload/script/api/style/css-manager.ts — import at line 3; usages around lines 124, 136, 147, 151, 203.apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx (1)
16-16: Don't widen toanyfor translation keys; keep type-safe labels.Using
anydrops safety fort(...). At minimum, usestring; ideally, type labels to your i18n key union.-const tabs: { value: LeftPanelTabValue; icon: React.ReactNode; label: any; disabled?: boolean }[] = +const tabs: { value: LeftPanelTabValue; icon: React.ReactNode; label: string; disabled?: boolean }[] =If you have typed keys, replace
stringwith the appropriateTranslationKeytype and keept(tab.label).Also applies to: 120-120
apps/web/client/src/app/project/[id]/_components/left-panel/layers-tab/index.tsx (1)
4-5: PreferComponentTypeand typingTreeRowto eliminate unsafe casts.Use
ComponentType<RowRendererProps<LayerNode>>here, and typeTreeRow’s props sorenderRow={TreeRow}needs no cast.-import { useCallback, useEffect, useRef, useState, type ElementType } from 'react'; +import { useCallback, useEffect, useRef, useState, type ComponentType } from 'react'; @@ - renderRow={TreeRow as unknown as ElementType<RowRendererProps<LayerNode>>} + renderRow={TreeRow as unknown as ComponentType<RowRendererProps<LayerNode>>}Optionally, in
tree-row.tsx:export function TreeRow(props: RowRendererProps<LayerNode>) { /* ... */ }Then use
renderRow={TreeRow}without casts.Also applies to: 151-151
apps/web/client/test/tools/glob.test.ts (2)
236-242: Strengthen the “sh should not be called” assertion.toHaveBeenCalledWith can miss single-arg calls. Prefer checking all invocations.
Apply:
- expect(mockSandbox.session.runCommand).not.toHaveBeenCalledWith( - expect.stringContaining('sh -c'), - expect.anything(), - expect.anything() - ); + expect( + mockSandbox.session.runCommand.mock.calls.some( + ([cmd]) => String(cmd).includes('sh -c') + ) + ).toBe(false);
440-446: Nit: extra whitespace.No action needed unless lint rules require.
apps/web/preload/script/api/state.ts (2)
3-7: Type _onlookFrameId as optional.It’s unset until first write; marking as required is inaccurate and masks undefined at compile time.
Apply:
declare global { interface Window { - _onlookFrameId: string; + _onlookFrameId?: string; } }
13-23: Consider handling penpalParent errors.Optional: add catch to avoid unhandled rejection noise.
Example:
penpalParent?.getFrameId().then(setFrameId).catch(() => {});apps/web/client/src/components/store/editor/pages/helper.ts (2)
178-186: Avoid ts-ignore by using a typed accumulator.Replace index writes + ts-ignore with a safe Partial accumulator and cast once.
Apply:
- metadata = {}; + const meta: Partial<PageMetadata> & Record<string, unknown> = {}; // Extract properties from the object expression for (const prop of declarator.init.properties) { if (t.isObjectProperty(prop) && t.isIdentifier(prop.key)) { const key = prop.key.name; if (t.isStringLiteral(prop.value)) { - // @ts-ignore - (metadata)[key] = prop.value.value; + meta[key] = prop.value.value; } else if (t.isObjectExpression(prop.value)) { - // @ts-ignore - (metadata)[key] = extractObjectValue(prop.value); + meta[key] = extractObjectValue(prop.value); } else if (t.isArrayExpression(prop.value)) { - // @ts-ignore - (metadata)[key] = extractArrayValue(prop.value); + meta[key] = extractArrayValue(prop.value); } } } + metadata = meta as PageMetadata;
489-497: Minor: redundant readDir call.pathExists reads the directory twice; drop the first call.
Apply:
- try { - await sandboxManager.readDir(getDirName(filePath)); - const dirEntries = await sandboxManager.readDir(getDirName(filePath)); + try { + const dirEntries = await sandboxManager.readDir(getDirName(filePath));apps/web/client/src/app/projects/import/local/_components/select-folder.tsx (1)
64-76: Early returns may reject valid selections without subfolders.If files come without a slash in webkitRelativePath (or only file names), this returns no files. Consider allowing single-level selections and deriving rootPath as '' while still processing.
Apply:
- if (!allPaths[0]) { - return processedFiles; - } + if (!allPaths[0]) return processedFiles; const rootPath = allPaths.length > 0 && allPaths[0].includes('/') ? allPaths[0].split('/')[0] : ''; - if (!rootPath) { - return processedFiles; - } + // Allow flat selections (no root folder), proceed with relative file names.apps/web/client/src/app/api/chat/helpers/persistence.ts (1)
28-85: Consider adding monitoring for the persistence operation.Since this is a critical path for saving chat messages, consider adding observability to track performance and failures.
Add timing and error tracking:
export async function saveMessageWithParts(message: UIMessage | ChatUIMessage, conversationId: string) { + const startTime = Date.now(); const typedMessage = 'metadata' in message && message.metadata !== undefined ? message as ChatUIMessage : convertToTypedUIMessage(message as UIMessage); + + try { return await db.transaction(async (tx) => { // ... existing transaction code ... return dbMessage; }); + } catch (error) { + // Log error with context for debugging + console.error('Failed to save message with parts', { + messageId: typedMessage.id, + conversationId, + partCount: typedMessage.parts?.length ?? 0, + duration: Date.now() - startTime, + error + }); + throw error; + } finally { + // Consider adding metrics collection here + const duration = Date.now() - startTime; + if (duration > 1000) { + console.warn(`Slow message persistence: ${duration}ms for message ${typedMessage.id}`); + } + } }packages/models/src/chat/message/message.ts (5)
14-21: Makeweatherdata optional and guard the default.Right now,
loading’s default applies only if you parse with Zod. Many call sites pass raw types; at runtimeloadingcan beundefined. Also, requiringweatherblocks any other future data parts.export const dataPartSchema = z.object({ - weather: z.object({ + weather: z.object({ weather: z.string().optional(), location: z.string().optional(), temperature: z.number().optional(), loading: z.boolean().default(true), - }), + }).partial().optional(), });
23-23: Export input/output types to reflect Zod defaults and parsing.This clarifies that defaults are applied only after parsing.
-export type ChatDataPart = z.infer<typeof dataPartSchema>; +export type ChatDataPartInput = z.input<typeof dataPartSchema>; +export type ChatDataPart = z.output<typeof dataPartSchema>;
11-11: ExportChatMetadatafor downstream typing.Enables consumers to type helpers without importing the schema.
-type ChatMetadata = z.infer<typeof chatMetadataSchema>; +export type ChatMetadata = z.infer<typeof chatMetadataSchema>;
35-43:SYSTEMrole is defined but not representable inChatMessage.Either include
SystemChatMessagein the union or document why it’s intentionally excluded. If included:export interface AssistantChatMessage extends BaseChatMessage { role: typeof ChatMessageRole.ASSISTANT; } +export interface SystemChatMessage extends BaseChatMessage { + role: typeof ChatMessageRole.SYSTEM; +} export type ChatSnapshot = Record<string, CodeDiff>; -export type ChatMessage = UserChatMessage | AssistantChatMessage; +export type ChatMessage = UserChatMessage | AssistantChatMessage | SystemChatMessage;Also applies to: 61-71
51-59: Requiredmetadatamay break existing constructors.Confirm all creation sites provide
metadata. If not, make it optional or ensure a mapper default.- metadata: InternalChatMetadata; + metadata?: InternalChatMetadata;apps/web/client/src/server/api/routers/domain/verify/helpers/freestyle.ts (1)
39-55: Optional: replaceconsole.errorwith app logger; preserve context.Improves observability without noisy stderr in prod.
Also applies to: 57-81
apps/web/client/src/server/api/routers/github.ts (1)
72-79: Avoid unsafe cast; derive org login robustly.For Organization accounts, Octokit’s installation account typically includes login. Falling back to name via a manual cast is brittle. Prefer a safe discriminator with graceful fallback.
- login: 'login' in installation.data.account ? installation.data.account.login : (installation.data.account as { name: string }).name || '', + login: + 'login' in installation.data.account && installation.data.account.login + ? installation.data.account.login + : ('name' in installation.data.account && installation.data.account.name) + ? installation.data.account.name + : String(installation.data.account.id),apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (1)
88-119: Avoid stale conversationId by reading it at call time.The top-level const may lag behind rapid navigation. Read the current ID inside sendMessageToChat.
- const sendMessageToChat = async (type: ChatType = ChatType.EDIT) => { - if (!conversationId) { + const sendMessageToChat = async (type: ChatType = ChatType.EDIT) => { + const currentConversationId = editorEngine.chat.conversation.current?.conversation.id; + if (!currentConversationId) { throw new Error('No conversation id'); } lastMessageRef.current = null; editorEngine.chat.error.clear(); @@ return chat.regenerate({ body: { chatType: type, - conversationId + conversationId: currentConversationId }, }); };apps/web/client/src/server/api/routers/chat/conversation.ts (1)
80-125: Good validations on parent entities; add a small guard to prevent trivial self-parenting.While IDs are generated server-side, adding a defensive check avoids accidental future regressions.
.mutation(async ({ ctx, input }) => { + // Defensive: prevent self-parenting (future-proofing) + if (input.parentConversationId === input.displayName /* placeholder to ensure runtime mismatch */) { + throw new Error('Invalid request'); + }(Optional) Consider rejecting creation if parentConversationId already forms a cycle (see tree builder note below).
apps/web/client/src/server/api/routers/chat/message.ts (1)
36-38: Avoid usingz.any()for parts validation.Using
z.any()bypasses type safety. Consider defining a proper schema for parts validation.- message: messageInsertSchema.extend({ - parts: z.array(z.any()).optional() - }) + message: messageInsertSchema.extend({ + parts: z.array(z.object({ + type: z.string(), + text: z.string().optional(), + toolCallId: z.string().optional(), + state: z.string().optional(), + // Add other expected fields based on ChatUIMessagePart + })).optional() + })packages/db/src/schema/chat/message.ts (1)
26-27: Deprecated JSONB parts column should be removed.The parts column is marked as deprecated but is still defined with a default value. Since parts are now stored in a separate table, this column should be scheduled for removal in a future migration to avoid confusion and data inconsistency.
Would you like me to create a migration script to safely remove the deprecated parts column from the messages table?
packages/db/src/mappers/chat/message.ts (5)
11-21: Avoid leaking DB-only fields into ChatMessage; map explicitly.Spreading
...messagepulls DB-only props (conversationId,applied,commitOid,snapshots, possibly legacyparts) into the runtime ChatMessage. Build the ChatMessage shape explicitly to decouple storage from domain.Apply:
- const baseMessage = { - ...message, - threadId: message.conversationId, + const baseMessage = { + id: message.id, + createdAt: message.createdAt, + threadId: message.conversationId, metadata: { vercelId: message.id, context: message.context ?? [], checkpoints: message.checkpoints ?? [], }, parts: [], // Parts will be loaded separately and merged }
26-26: Prefer exact role literals over casts.The switch already narrows the role. Using literals removes unnecessary casts.
- role: message.role as typeof ChatMessageRole.ASSISTANT, + role: ChatMessageRole.ASSISTANT, ... - role: message.role as typeof ChatMessageRole.USER, + role: ChatMessageRole.USER,Also applies to: 31-31
44-48: Return a new object instead of mutatingbaseMessage.Keeps the mapper purely functional.
export const fromDbMessageWithParts = (message: DbMessage, parts: DbPart[]): ChatMessage => { - const baseMessage = fromDbMessage(message); - baseMessage.parts = fromDbParts(parts); - return baseMessage; + const baseMessage = fromDbMessage(message); + return { ...baseMessage, parts: fromDbParts(parts) }; }
59-65:contentloses non-text parts; consider a safer plain-text reducer.Concatenating only text drops tools, images, code, etc., which may regress search/indexing. Consider a
partsToPlainText(parts)helper inparts.tsthat:
- uses
textfor text parts,- substitutes concise placeholders for non-text (e.g.,
[image],[tool:<name>]),- joins with
\nfor readability.
77-79: Add explicit return type for exported helper.Improves API clarity and guards accidental signature drift.
-export const getDbPartsFromMessage = (message: ChatMessage) => { +export const getDbPartsFromMessage = (message: ChatMessage): ReturnType<typeof toDbParts> => { return toDbParts(message.parts, message.id); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
apps/backend/supabase/migrations/0019_cuddly_iron_man.sql(1 hunks)apps/backend/supabase/migrations/meta/0019_snapshot.json(1 hunks)apps/backend/supabase/migrations/meta/_journal.json(1 hunks)apps/web/client/src/app/_components/shared/mockups/tailwind-color-editor.tsx(2 hunks)apps/web/client/src/app/api/chat/helpers/persistence.ts(1 hunks)apps/web/client/src/app/api/chat/route.ts(2 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/layers-tab/index.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/code-tab/file-operations.ts(1 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx(2 hunks)apps/web/client/src/app/projects/import/local/_components/select-folder.tsx(2 hunks)apps/web/client/src/app/terms-of-service/page.tsx(1 hunks)apps/web/client/src/components/store/editor/pages/helper.ts(3 hunks)apps/web/client/src/components/store/editor/sandbox/terminal.ts(1 hunks)apps/web/client/src/components/store/editor/theme/index.ts(2 hunks)apps/web/client/src/components/tools/tools.ts(1 hunks)apps/web/client/src/server/api/routers/chat/conversation.ts(2 hunks)apps/web/client/src/server/api/routers/chat/message.ts(4 hunks)apps/web/client/src/server/api/routers/domain/verify/helpers/freestyle.ts(1 hunks)apps/web/client/src/server/api/routers/github.ts(2 hunks)apps/web/client/src/utils/git/index.test.ts(2 hunks)apps/web/client/test/pages/helper.test.ts(5 hunks)apps/web/client/test/sandbox/file-sync.test.ts(1 hunks)apps/web/client/test/template-nodes/index.test.ts(5 hunks)apps/web/client/test/tools/glob.test.ts(4 hunks)apps/web/client/test/tools/grep.test.ts(8 hunks)apps/web/preload/script/api/elements/helpers.ts(1 hunks)apps/web/preload/script/api/state.ts(1 hunks)apps/web/preload/script/api/style/css-manager.ts(3 hunks)packages/ai/test/stream/convert.test.ts(2 hunks)packages/db/src/defaults/conversation.ts(1 hunks)packages/db/src/index.ts(1 hunks)packages/db/src/mappers/chat/conversation.ts(2 hunks)packages/db/src/mappers/chat/index.ts(1 hunks)packages/db/src/mappers/chat/message.ts(3 hunks)packages/db/src/mappers/chat/parts.ts(1 hunks)packages/db/src/schema/chat/conversation.ts(3 hunks)packages/db/src/schema/chat/index.ts(1 hunks)packages/db/src/schema/chat/message.ts(1 hunks)packages/db/src/schema/chat/parts.ts(1 hunks)packages/models/src/chat/conversation/index.ts(1 hunks)packages/models/src/chat/message/message.ts(1 hunks)subchat-implementation-example.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.tsx: In React providers managing MobX stores, create the observable store with useState(() => new Store()) to ensure a stable instance
Keep a ref (e.g., storeRef.current) to the MobX store to avoid stale closures in effects
Use setTimeout(() => store.clear(), 0) for delayed cleanup of MobX stores to avoid race conditions
Separate project changes from branch updates by using proper effect dependency arrays
Do not use useMemo to hold MobX observable instances; React may drop memoized values causing data loss
Do not clean up MobX stores synchronously during navigation; perform delayed cleanup instead
Do not include the MobX store instance in effect dependency arrays when it causes infinite loops
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/layers-tab/index.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/app/terms-of-service/page.tsxapps/web/client/src/app/_components/shared/mockups/tailwind-color-editor.tsxapps/web/client/src/app/projects/import/local/_components/select-folder.tsx
🧠 Learnings (3)
📚 Learning: 2025-09-07T23:36:29.687Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T23:36:29.687Z
Learning: Applies to **/*.tsx : Separate project changes from branch updates by using proper effect dependency arrays
Applied to files:
apps/web/client/src/app/project/[id]/_components/left-panel/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/layers-tab/index.tsx
📚 Learning: 2025-09-07T23:36:29.687Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T23:36:29.687Z
Learning: This is a Bun workspace; only use Bun commands and tooling, not npm
Applied to files:
apps/web/client/test/tools/glob.test.tsapps/web/client/test/tools/grep.test.ts
📚 Learning: 2025-09-07T23:36:29.687Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T23:36:29.687Z
Learning: Run unit tests with `bun test`
Applied to files:
apps/web/client/test/tools/glob.test.ts
🧬 Code graph analysis (20)
packages/db/src/schema/chat/parts.ts (1)
packages/db/src/schema/chat/message.ts (1)
messages(12-28)
apps/web/client/src/utils/git/index.test.ts (1)
apps/web/client/src/utils/git/index.ts (2)
sanitizeCommitMessage(7-47)escapeShellString(53-66)
apps/web/client/test/pages/helper.test.ts (1)
apps/web/client/src/components/store/editor/pages/helper.ts (1)
scanAppDirectory(201-310)
apps/web/client/test/template-nodes/index.test.ts (1)
packages/parser/src/template-node/map.ts (1)
createTemplateNodeMap(9-118)
apps/web/client/src/components/store/editor/pages/helper.ts (1)
packages/utility/src/file.ts (1)
getBaseName(80-83)
apps/web/client/src/app/api/chat/route.ts (1)
apps/web/client/src/app/api/chat/helpers/persistence.ts (1)
saveMessageWithParts(28-85)
packages/db/src/mappers/chat/parts.ts (2)
packages/models/src/chat/message/message.ts (2)
ChatUIMessagePart(33-33)ChatProviderMetadata(29-29)packages/db/src/schema/chat/parts.ts (1)
parts(17-153)
apps/web/client/src/app/project/[id]/_components/left-panel/layers-tab/index.tsx (1)
packages/models/src/element/layers.ts (1)
LayerNode(12-25)
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (1)
packages/db/src/mappers/chat/message.ts (1)
toOnlookMessageFromVercel(81-102)
packages/ai/test/stream/convert.test.ts (1)
packages/ai/src/stream/index.ts (1)
extractTextFromParts(53-62)
apps/web/client/src/server/api/routers/chat/conversation.ts (4)
apps/web/client/src/server/api/trpc.ts (1)
protectedProcedure(130-149)packages/db/src/schema/chat/conversation.ts (1)
conversations(11-31)packages/db/src/mappers/chat/conversation.ts (1)
fromDbConversation(4-13)packages/db/src/schema/chat/message.ts (1)
messages(12-28)
apps/web/client/src/app/api/chat/helpers/persistence.ts (4)
packages/models/src/chat/message/message.ts (3)
ChatUIMessage(32-32)ChatUIMessagePart(33-33)chatMetadataSchema(9-9)packages/db/src/client.ts (1)
db(16-16)packages/db/src/schema/chat/message.ts (1)
messages(12-28)packages/db/src/schema/chat/parts.ts (1)
parts(17-153)
packages/db/src/schema/chat/message.ts (2)
packages/db/src/schema/chat/conversation.ts (1)
conversations(11-31)packages/db/src/schema/chat/parts.ts (1)
parts(17-153)
apps/web/client/src/app/_components/shared/mockups/tailwind-color-editor.tsx (6)
packages/utility/src/color.ts (1)
Color(160-391)packages/ui/src/components/icons/index.tsx (1)
Icons(138-3594)apps/web/client/src/app/project/[id]/_components/editor-bar/toolbar-button.tsx (1)
ToolbarButton(13-67)apps/web/client/src/app/project/[id]/_components/editor-bar/separator.tsx (1)
InputSeparator(1-3)packages/ui/src/components/color-picker/ColorPicker.tsx (1)
ColorPicker(223-369)apps/web/client/src/app/_components/landing-page/color-swatch-group.tsx (1)
ColorSwatchGroup(8-28)
packages/db/src/schema/chat/conversation.ts (1)
packages/db/src/schema/chat/message.ts (1)
messages(12-28)
apps/web/client/test/sandbox/file-sync.test.ts (1)
apps/web/client/src/components/store/editor/sandbox/file-sync.ts (1)
FileSyncManager(6-101)
packages/db/src/mappers/chat/message.ts (2)
packages/models/src/chat/message/message.ts (7)
ChatMessageRole(36-40)ChatMessageRole(42-42)AssistantChatMessage(65-67)UserChatMessage(61-63)ChatMessage(71-71)ChatUIMessage(32-32)InternalChatMetadata(45-49)packages/db/src/mappers/chat/parts.ts (2)
fromDbParts(187-191)toDbParts(196-198)
apps/web/client/src/components/store/editor/theme/index.ts (2)
apps/web/client/src/components/store/editor/theme/util.ts (3)
isValidTailwindConfigProperty(380-392)addTailwindRootColor(428-460)addTailwindNestedColor(8-68)packages/parser/src/helpers.ts (1)
isObjectExpression(31-33)
apps/web/client/src/server/api/routers/chat/message.ts (3)
packages/db/src/schema/chat/parts.ts (1)
parts(17-153)packages/db/src/mappers/chat/message.ts (1)
fromDbMessageWithParts(44-48)packages/db/src/schema/chat/message.ts (1)
messages(12-28)
packages/models/src/chat/message/message.ts (3)
packages/ai/src/tools/toolset.ts (1)
BUILD_TOOL_SET(54-64)packages/models/src/chat/message/context.ts (1)
MessageContext(53-59)packages/models/src/chat/message/checkpoint.ts (1)
MessageCheckpoints(15-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Supabase Preview
- GitHub Check: Supabase Preview
🔇 Additional comments (45)
apps/web/client/src/components/store/editor/sandbox/terminal.ts (1)
166-166: Good: removed the any-cast while overriding write.Type safety improves and the signature matches xterm’s
write(data, callback?). Assuming current xterm types don’t markwriteas readonly, this should compile cleanly.If CI complains about assigning to
write, ping me and I’ll provide a minimal type alias to satisfy TS without reverting toany.apps/web/client/src/utils/git/index.test.ts (2)
2-2: Import reorder is fine.Non-functional; no action needed.
14-19: Use the source constant for the commit-message length (don't hard-code 75)Test should assert against the project's constant rather than a literal. apps/web/client/src/utils/git/index.ts truncates using Git.MAX_COMMIT_MESSAGE_LENGTH (lines ~24–29). If Git or MAX_COMMIT_MESSAGE_LENGTH is exported from ./index, import it in apps/web/client/src/utils/git/index.test.ts and replace the assertion with:
expect(result.length).toBeLessThanOrEqual(Git.MAX_COMMIT_MESSAGE_LENGTH + 3)
If it's not exported, export the constant (or expose an equivalent) and then update the test.apps/web/client/src/app/_components/shared/mockups/tailwind-color-editor.tsx (1)
13-13: LGTM for state init.Lazy initializer with
Color.from(...).withAlpha(1)is clean and avoids re-parsing on rerenders.apps/web/client/src/components/tools/tools.ts (1)
1-1: Approve — barrel export verifiedgetToolSetFromType is declared/exported in apps/web/client/src/app/api/chat/helpers/stream.ts and re‑exported by apps/web/client/src/app/api/chat/helpers/index.ts; current imports (apps/web/client/src/components/tools/tools.ts and apps/web/client/src/app/api/chat/route.ts) are valid and no stale imports were found.
apps/web/client/src/app/project/[id]/_components/right-panel/code-tab/file-operations.ts (1)
78-78: LGTM: stronger typing for reserved-name check.Removing
anyhere tightens validation without changing behavior.apps/web/client/test/pages/helper.test.ts (5)
4-4: Import order change is fine.No semantic impact.
708-710: Good removal of unnecessary any-cast.Passing the typed sandboxManager directly matches scanAppDirectory’s signature.
734-736: Graceful handling of readDir errors looks correct.Test accurately asserts empty result on directory read failure.
749-751: Rejection path assertion is correct.Expecting the readFile failure to reject with the specific message is appropriate.
768-771: Metadata error handling test is solid.Verifies fallback to empty metadata without failing traversal.
apps/web/client/test/template-nodes/index.test.ts (5)
40-49: Typed EditorEngine mock looks sufficient.The minimal branches surface used by TemplateNodeManager is provided without any-casts.
250-257: Mocking parser addOidsToAst via bun.mock is correct.Clear and type-safe; good cache-verification assertions.
286-299: Type-safe createTemplateNodeMap mocking is good.Accurately measures calls for cache behavior.
310-323: Cache invalidation test is well structured.Ensures reprocessing on content change.
372-389: Branch-isolated caches test reads well.Distinct node maps per branch are validated correctly.
packages/db/src/defaults/conversation.ts (1)
12-14: Resolved — DB columns are nullable and indexed; no changes required.Migration 0019 adds parent_conversation_id and parent_message_id as nullable columns with FKs and indexes; the DB schema defines the UUID columns and the TypeScript models mark these fields optional.
apps/web/client/test/tools/glob.test.ts (2)
1-4: Import/typing cleanups look good.Aligns with Bun and stronger typing.
23-28: Typed EditorEngine mock is sufficient.Removes any-casts without weakening the tests.
packages/ai/test/stream/convert.test.ts (2)
137-143: Reasoning part handling is fine.Non-text parts collapse to empty; assertions look correct given the extraction contract.
146-148: Empty array expectation is consistent.Returns empty string for [].
apps/web/client/src/components/store/editor/pages/helper.ts (1)
271-275: Formatting-only change is fine.No behavioral impact.
apps/web/client/src/app/projects/import/local/_components/select-folder.tsx (2)
80-82: Type usage of webkitRelativePath is fine.TS lib.dom includes it; removing any-casts is safe.
253-261: Drop handler cleanup looks good.Removes unnecessary any-cast; preserves behavior.
apps/web/client/src/components/store/editor/theme/index.ts (1)
409-409: Nice cleanup of type casting.Removing the unnecessary
as anycasts improves type safety without changing behavior. The functions were already typed to accept the correct types.Also applies to: 417-417, 942-942, 944-944
apps/backend/supabase/migrations/0019_cuddly_iron_man.sql (1)
6-8: Good indexing strategy for subchat queries.The indexes on
parent_conversation_id,parent_message_id, andproject_idwill optimize queries for fetching subchats, conversation trees, and project-specific conversations.apps/web/client/test/tools/grep.test.ts (3)
27-27: Improved type safety in mock creation.Removing the
as anytype cast forcreateMockEditorEngineimproves type safety by properly typing the return value asEditorEngine.
465-468: Consistent formatting for mock command results.The reformatting of multi-line blocks to compact object literals improves readability and consistency without affecting functionality.
Also applies to: 489-492, 512-515, 536-539, 693-697, 715-718
628-686: Good expansion of multiline pattern test coverage.The new tests for PCRE multiline support, fallback to awk, and validation of literal newlines provide comprehensive coverage for multiline search functionality.
apps/backend/supabase/migrations/meta/0019_snapshot.json (1)
260-285: Schema snapshot correctly reflects migration changes.The snapshot properly includes the new parent relationship columns, foreign keys, and indexes introduced in the migration file. The cascading delete behavior is correctly configured.
packages/db/src/index.ts (1)
1-1: Good: type-only re-export for DrizzleDb from the barrel.packages/db/src/mappers/chat/index.ts (1)
3-3: LGTM: exposing parts mappers via the barrel keeps APIs coherent.apps/web/client/src/server/api/routers/domain/verify/helpers/freestyle.ts (1)
2-2: Good: unified barrel import with type-onlyDrizzleDb.packages/db/src/schema/chat/index.ts (1)
3-3: LGTM: re-exportingpartsaligns schema barrel with mappers.apps/web/client/src/server/api/routers/github.ts (1)
1-2: Barrel import change looks good.Centralizing types/exports under @onlook/db improves cohesion and reduces deep import churn.
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (2)
54-60: Correct input shape to toOnlookMessageFromVercel.Passing { id, parts, role, metadata } aligns with the updated mapper signature.
76-83: Error path mapping matches new signature.Mirrors the success path and avoids leaking the entire UIMessage object.
packages/models/src/chat/conversation/index.ts (1)
13-24: Model extensions for subchats look consistent.Optional parentConversationId/parentMessageId and tree types align with the new API surface and mappers.
apps/web/client/src/server/api/routers/chat/conversation.ts (2)
67-78: Subchat listing is straightforward and ordered by createdAt.Looks good.
127-141: Root conversations query LGTM.Uses isNull on parentConversationId and orders by recency.
apps/web/client/src/server/api/routers/chat/message.ts (1)
24-32: Good addition of parts fetching with proper ordering.The implementation correctly fetches parts with their order, which ensures message parts are displayed in the correct sequence.
packages/db/src/mappers/chat/message.ts (4)
81-92: Good: explicit system-role rejection with narrowing.The guard cleanly narrows
roletouser | assistant, keepingtoOnlookMessageFromVercelsound.
104-106: LGTM: bridge composes mappers cleanly.
toDbMessageFromVercelreusestoOnlookMessageFromVercel→toDbMessage, keeping a single mapping path.
70-70: Two sources of truth for message parts — confirm DB schema and cutover plan.
- Confirmed: messages table still has a JSONB parts column (apps/backend/supabase/migrations/0000_same_human_robot.sql; packages/db/src/schema/chat/message.ts) while a dedicated parts table/relation exists (packages/db/src/schema/chat/parts.ts). Mapper still writes the JSONB: packages/db/src/mappers/chat/message.ts —
parts: message.parts, // Keep parts for backwards compatibility during transition.- Action: stop dual-writes long‑term — either remove the messages.parts assignment and rely on the parts table, or gate writes behind a feature flag, backfill/sync parts table, then drop the JSONB column in a migration.
Files to act on: packages/db/src/mappers/chat/message.ts, packages/db/src/schema/chat/message.ts, packages/db/src/schema/chat/parts.ts, apps/backend/supabase/migrations/0000_same_human_robot.sql.
33-35: Guard is fine; DB enum still allows 'system' — verify we never persist or read those rows.
- Evidence: mapper throws on SYSTEM (packages/db/src/mappers/chat/message.ts) while DB schema defines message_role including 'system' (packages/db/src/schema/chat/message.ts). Code injects transient system messages for the model (apps/web/client/src/app/api/chat/route.ts, apps/web/client/src/server/api/routers/chat/suggestion.ts) and the message upsert endpoint inserts whatever role it receives (apps/web/client/src/server/api/routers/chat/message.ts).
- Action: run on your DB: SELECT count(*) FROM messages WHERE role = 'system';. If >0, either convert/remove those rows or update the mapper to handle them. Also add server-side validation/sanitization in the upsert/update/save paths to reject or normalize 'system' roles (or remove 'system' from the DB enum if unused).
| "version": "7", | ||
| "dialect": "postgresql", | ||
| "entries": [ | ||
| { | ||
| "idx": 0, | ||
| "version": "7", | ||
| "when": 1747780205535, | ||
| "tag": "0000_same_human_robot", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 1, | ||
| "version": "7", | ||
| "when": 1747780749310, | ||
| "tag": "0001_graceful_exodus", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 2, | ||
| "version": "7", | ||
| "when": 1748028633785, | ||
| "tag": "0002_red_crusher_hogan", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 3, | ||
| "version": "7", | ||
| "when": 1748335865371, | ||
| "tag": "0003_loud_ozymandias", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 4, | ||
| "version": "7", | ||
| "when": 1748355015711, | ||
| "tag": "0004_pink_expediter", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 5, | ||
| "version": "7", | ||
| "when": 1748430282890, | ||
| "tag": "0005_short_lila_cheney", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 6, | ||
| "version": "7", | ||
| "when": 1748548078866, | ||
| "tag": "0006_rls", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 7, | ||
| "version": "7", | ||
| "when": 1748618860222, | ||
| "tag": "0007_realtime_rls", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 8, | ||
| "version": "7", | ||
| "when": 1748969281653, | ||
| "tag": "0008_preview-img-storage", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 9, | ||
| "version": "7", | ||
| "when": 1748969954773, | ||
| "tag": "0009_project_img_path", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 10, | ||
| "version": "7", | ||
| "when": 1749589432515, | ||
| "tag": "0010_bent_edwin_jarvis", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 11, | ||
| "version": "7", | ||
| "when": 1751069383054, | ||
| "tag": "0011_typical_clea", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 12, | ||
| "version": "7", | ||
| "when": 1751069449613, | ||
| "tag": "0012_file-transfer-bucket", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 13, | ||
| "version": "7", | ||
| "when": 1751914112866, | ||
| "tag": "0013_aspiring_kabuki", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 14, | ||
| "version": "7", | ||
| "when": 1752263777650, | ||
| "tag": "0014_military_marrow", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 15, | ||
| "version": "7", | ||
| "when": 1753208692842, | ||
| "tag": "0015_same_leo", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 16, | ||
| "version": "7", | ||
| "when": 1755044168112, | ||
| "tag": "0016_pretty_dust", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 17, | ||
| "version": "7", | ||
| "when": 1756677811694, | ||
| "tag": "0017_small_xavin", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 18, | ||
| "version": "7", | ||
| "when": 1757372913453, | ||
| "tag": "0018_lush_thanos", | ||
| "breakpoints": true | ||
| } | ||
| ] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Journal appears to be missing the 0019 entry; migrations may drift from snapshot.
PR mentions 0019_* migration/snapshot, but _journal.json ends at 0018_*. Regenerate the journal to include 0019 to keep migration history consistent.
🏁 Script executed:
#!/bin/bash
# Check for presence of 0019 migrations/snapshots
fd -a '0019_*' apps/backend/supabase/migrations
# Sanity-check journal references 0019
rg -n '"tag":\s*"0019_' apps/backend/supabase/migrations/meta/_journal.jsonLength of output: 152
Regenerate _journal.json — add the missing 0019 entry
0019 migration and snapshot are present but apps/backend/supabase/migrations/meta/_journal.json stops at 0018; regenerate or add an entry for "0019_cuddly_iron_man" so migration history and snapshots stay consistent.
Affected files:
- apps/backend/supabase/migrations/0019_cuddly_iron_man.sql
- apps/backend/supabase/migrations/meta/0019_snapshot.json
- apps/backend/supabase/migrations/meta/_journal.json (update to include tag "0019_cuddly_iron_man")
🤖 Prompt for AI Agents
In apps/backend/supabase/migrations/meta/_journal.json around lines 2 to 138,
the journal stops at entry idx 18 and is missing the 0019 entry; add a new
entries object for idx 19 with "version": "7", "idx": 19, "tag":
"0019_cuddly_iron_man", "breakpoints": true and set the "when" timestamp to
match the created time of
apps/backend/supabase/migrations/meta/0019_snapshot.json (or the migration file
timestamp) so the migration history and snapshots remain consistent.
| const primary = color.toHex(); | ||
| const complementary = new Color({ ...color, h: (color.h + 0.5) % 1, a: 1 }).toHex(); | ||
| const triadic = new Color({ ...color, h: (color.h + 0.33) % 1, a: 1 }).toHex(); | ||
| const split1 = new Color({ ...color, h: (color.h + 0.42) % 1, a: 1 }).toHex(); | ||
| const split2 = new Color({ ...color, h: (color.h + 0.58) % 1, a: 1 }).toHex(); | ||
| const primaryLight = new Color({ ...color, v: Math.min(1, color.v * 1.12), a: 1 }).toHex(); | ||
| const primaryDark = new Color({ ...color, v: Math.max(0, color.v * 0.82), a: 1 }).toHex(); | ||
| // alpha-adjusted variants for the soft background field | ||
| const primaryBg = `${primary}B3`; // ~70% | ||
| const compBg = `${complementary}40`; // ~25% | ||
| const triadicBg = `${triadic}33`; // ~20% | ||
| return ( |
There was a problem hiding this comment.
Hex alpha concatenation can produce invalid 10-digit colors when color.a < 1.
toHex() returns 8-digit hex when alpha < 1. Appending B3/40/33 can yield 10-digit hex, breaking gradients once the user moves the alpha slider.
Apply this safer construction using Color.withAlpha and keep opaque/derived colors as Color instances:
- const primary = color.toHex();
- const complementary = new Color({ ...color, h: (color.h + 0.5) % 1, a: 1 }).toHex();
- const triadic = new Color({ ...color, h: (color.h + 0.33) % 1, a: 1 }).toHex();
- const split1 = new Color({ ...color, h: (color.h + 0.42) % 1, a: 1 }).toHex();
- const split2 = new Color({ ...color, h: (color.h + 0.58) % 1, a: 1 }).toHex();
- const primaryLight = new Color({ ...color, v: Math.min(1, color.v * 1.12), a: 1 }).toHex();
- const primaryDark = new Color({ ...color, v: Math.max(0, color.v * 0.82), a: 1 }).toHex();
- // alpha-adjusted variants for the soft background field
- const primaryBg = `${primary}B3`; // ~70%
- const compBg = `${complementary}40`; // ~25%
- const triadicBg = `${triadic}33`; // ~20%
+ const primaryColor = new Color({ ...color, a: 1 });
+ const complementaryColor = new Color({ ...color, h: (color.h + 0.5) % 1, a: 1 });
+ const triadicColor = new Color({ ...color, h: (color.h + 0.33) % 1, a: 1 });
+ const split1Color = new Color({ ...color, h: (color.h + 0.42) % 1, a: 1 });
+ const split2Color = new Color({ ...color, h: (color.h + 0.58) % 1, a: 1 });
+
+ const primary = primaryColor.toHex();
+ const complementary = complementaryColor.toHex();
+ const triadic = triadicColor.toHex();
+ const split1 = split1Color.toHex();
+ const split2 = split2Color.toHex();
+
+ const primaryLight = new Color({ ...primaryColor, v: Math.min(1, color.v * 1.12) }).toHex();
+ const primaryDark = new Color({ ...primaryColor, v: Math.max(0, color.v * 0.82) }).toHex();
+ // alpha-adjusted variants for the soft background field
+ const primaryBg = primaryColor.withAlpha(0.7).toHex(); // ~70%
+ const compBg = complementaryColor.withAlpha(0.25).toHex(); // ~25%
+ const triadicBg = triadicColor.withAlpha(0.2).toHex(); // ~20%📝 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 primary = color.toHex(); | |
| const complementary = new Color({ ...color, h: (color.h + 0.5) % 1, a: 1 }).toHex(); | |
| const triadic = new Color({ ...color, h: (color.h + 0.33) % 1, a: 1 }).toHex(); | |
| const split1 = new Color({ ...color, h: (color.h + 0.42) % 1, a: 1 }).toHex(); | |
| const split2 = new Color({ ...color, h: (color.h + 0.58) % 1, a: 1 }).toHex(); | |
| const primaryLight = new Color({ ...color, v: Math.min(1, color.v * 1.12), a: 1 }).toHex(); | |
| const primaryDark = new Color({ ...color, v: Math.max(0, color.v * 0.82), a: 1 }).toHex(); | |
| // alpha-adjusted variants for the soft background field | |
| const primaryBg = `${primary}B3`; // ~70% | |
| const compBg = `${complementary}40`; // ~25% | |
| const triadicBg = `${triadic}33`; // ~20% | |
| return ( | |
| const primaryColor = new Color({ ...color, a: 1 }); | |
| const complementaryColor = new Color({ ...color, h: (color.h + 0.5) % 1, a: 1 }); | |
| const triadicColor = new Color({ ...color, h: (color.h + 0.33) % 1, a: 1 }); | |
| const split1Color = new Color({ ...color, h: (color.h + 0.42) % 1, a: 1 }); | |
| const split2Color = new Color({ ...color, h: (color.h + 0.58) % 1, a: 1 }); | |
| const primary = primaryColor.toHex(); | |
| const complementary = complementaryColor.toHex(); | |
| const triadic = triadicColor.toHex(); | |
| const split1 = split1Color.toHex(); | |
| const split2 = split2Color.toHex(); | |
| const primaryLight = new Color({ ...primaryColor, v: Math.min(1, color.v * 1.12) }).toHex(); | |
| const primaryDark = new Color({ ...primaryColor, v: Math.max(0, color.v * 0.82) }).toHex(); | |
| // alpha-adjusted variants for the soft background field | |
| const primaryBg = primaryColor.withAlpha(0.7).toHex(); // ~70% | |
| const compBg = complementaryColor.withAlpha(0.25).toHex(); // ~25% | |
| const triadicBg = triadicColor.withAlpha(0.2).toHex(); // ~20% | |
| return ( |
🤖 Prompt for AI Agents
In apps/web/client/src/app/_components/shared/mockups/tailwind-color-editor.tsx
around lines 36 to 47, the code appends literal hex alpha suffixes to strings
returned by color.toHex(), which can already be 8-digit when color.a < 1 and
yields invalid 10-digit hex values; replace the string concatenation with proper
Color instances set to the desired alpha (e.g., create new Color({...baseColor,
a: 0.7}) or use Color.withAlpha if available) for primaryBg/compBg/triadicBg,
keep primary/complementary/triadic/primaryLight/primaryDark as Color instances
when further manipulated, and call toHex() on those constructed Color objects so
the output is always a valid 8-digit (or 6-digit) hex string.
| function convertToTypedUIMessage(message: UIMessage): ChatUIMessage { | ||
| // Type-safe conversion of generic UIMessage to our specific ChatUIMessage | ||
| const convertedParts: ChatUIMessagePart[] = message.parts.map(part => { | ||
| // The parts are compatible, we just need to assert the correct type | ||
| return part as ChatUIMessagePart; | ||
| }); | ||
|
|
||
| return { | ||
| id: message.id, | ||
| role: message.role as 'user' | 'assistant', | ||
| parts: convertedParts, | ||
| metadata: message.metadata as ReturnType<typeof chatMetadataSchema.parse>, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding validation for part type conversions.
The type assertion part as ChatUIMessagePart assumes the parts are compatible without validation. This could lead to runtime errors if the parts structure doesn't match expectations.
Add runtime validation to ensure type safety:
-function convertToTypedUIMessage(message: UIMessage): ChatUIMessage {
- // Type-safe conversion of generic UIMessage to our specific ChatUIMessage
- const convertedParts: ChatUIMessagePart[] = message.parts.map(part => {
- // The parts are compatible, we just need to assert the correct type
- return part as ChatUIMessagePart;
- });
+function convertToTypedUIMessage(message: UIMessage): ChatUIMessage {
+ // Type-safe conversion of generic UIMessage to our specific ChatUIMessage
+ const convertedParts: ChatUIMessagePart[] = message.parts.map(part => {
+ // Validate that the part has expected structure before assertion
+ if (!part || typeof part !== 'object' || !('type' in part)) {
+ throw new Error(`Invalid message part structure: ${JSON.stringify(part)}`);
+ }
+ return part as ChatUIMessagePart;
+ });📝 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.
| function convertToTypedUIMessage(message: UIMessage): ChatUIMessage { | |
| // Type-safe conversion of generic UIMessage to our specific ChatUIMessage | |
| const convertedParts: ChatUIMessagePart[] = message.parts.map(part => { | |
| // The parts are compatible, we just need to assert the correct type | |
| return part as ChatUIMessagePart; | |
| }); | |
| return { | |
| id: message.id, | |
| role: message.role as 'user' | 'assistant', | |
| parts: convertedParts, | |
| metadata: message.metadata as ReturnType<typeof chatMetadataSchema.parse>, | |
| }; | |
| } | |
| function convertToTypedUIMessage(message: UIMessage): ChatUIMessage { | |
| // Type-safe conversion of generic UIMessage to our specific ChatUIMessage | |
| const convertedParts: ChatUIMessagePart[] = message.parts.map(part => { | |
| // Validate that the part has expected structure before assertion | |
| if (!part || typeof part !== 'object' || !('type' in part)) { | |
| throw new Error(`Invalid message part structure: ${JSON.stringify(part)}`); | |
| } | |
| return part as ChatUIMessagePart; | |
| }); | |
| return { | |
| id: message.id, | |
| role: message.role as 'user' | 'assistant', | |
| parts: convertedParts, | |
| metadata: message.metadata as ReturnType<typeof chatMetadataSchema.parse>, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In apps/web/client/src/app/api/chat/helpers/persistence.ts around lines 10 to
23, the conversion blindly asserts message.parts to ChatUIMessagePart which can
cause runtime errors; replace the assertion with runtime validation by parsing
each part with the appropriate schema (e.g., chatMessagePartSchema.parse or a
validator that checks required fields/types) and collect the parsed parts, and
if any part fails validation either throw a clear error or handle it (log and
skip or return a validation error) so the returned ChatUIMessage.parts is
guaranteed to match ChatUIMessagePart.
| if (part.type.startsWith("tool-")) { | ||
| const toolName = part.type.replace("tool-", ""); | ||
|
|
||
| // Type guard to ensure this is a tool part with the expected properties | ||
| if ('toolCallId' in part && 'state' in part) { | ||
| const toolPart = part as { toolCallId: string; state: string; errorText?: string; input?: unknown; output?: unknown }; | ||
|
|
||
| fields.tool_toolCallId = toolPart.toolCallId; | ||
| fields.tool_state = toolPart.state; | ||
| fields.tool_errorText = toolPart.errorText; | ||
|
|
||
| // Map specific tool input/output columns | ||
| const inputKey = `tool_${toolName}_input`; | ||
| const outputKey = `tool_${toolName}_output`; | ||
| fields[inputKey] = toolPart.input; | ||
| fields[outputKey] = toolPart.output; | ||
| } | ||
| } | ||
| // Handle data parts | ||
| else if (part.type.startsWith("data-")) { | ||
| const dataType = part.type.replace("data-", ""); | ||
| if (dataType === "weather" && 'id' in part && 'data' in part) { | ||
| const dataPart = part as { id: string; data: { location?: string; weather?: string; temperature?: number } }; | ||
| fields.data_weather_id = dataPart.id; | ||
| fields.data_weather_location = dataPart.data?.location; | ||
| fields.data_weather_weather = dataPart.data?.weather; | ||
| fields.data_weather_temperature = dataPart.data?.temperature; | ||
| } | ||
| } | ||
| break; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate tool and data part structures before field extraction.
The code assumes specific properties exist on tool and data parts without validation, which could cause runtime errors if the part structure is unexpected.
Add validation before accessing nested properties:
default:
// Handle tool calls
if (part.type.startsWith("tool-")) {
const toolName = part.type.replace("tool-", "");
// Type guard to ensure this is a tool part with the expected properties
if ('toolCallId' in part && 'state' in part) {
const toolPart = part as { toolCallId: string; state: string; errorText?: string; input?: unknown; output?: unknown };
+
+ // Validate required fields
+ if (!toolPart.toolCallId || !toolPart.state) {
+ console.warn(`Tool part missing required fields: ${part.type}`);
+ break;
+ }
fields.tool_toolCallId = toolPart.toolCallId;
fields.tool_state = toolPart.state;
fields.tool_errorText = toolPart.errorText;
// Map specific tool input/output columns
const inputKey = `tool_${toolName}_input`;
const outputKey = `tool_${toolName}_output`;
fields[inputKey] = toolPart.input;
fields[outputKey] = toolPart.output;
}
}
// Handle data parts
else if (part.type.startsWith("data-")) {
const dataType = part.type.replace("data-", "");
if (dataType === "weather" && 'id' in part && 'data' in part) {
const dataPart = part as { id: string; data: { location?: string; weather?: string; temperature?: number } };
+
+ // Validate that data object exists
+ if (!dataPart.data || typeof dataPart.data !== 'object') {
+ console.warn(`Data part missing data object: ${part.type}`);
+ break;
+ }
+
fields.data_weather_id = dataPart.id;
fields.data_weather_location = dataPart.data?.location;
fields.data_weather_weather = dataPart.data?.weather;
fields.data_weather_temperature = dataPart.data?.temperature;
}
}
break;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (part.type.startsWith("tool-")) { | |
| const toolName = part.type.replace("tool-", ""); | |
| // Type guard to ensure this is a tool part with the expected properties | |
| if ('toolCallId' in part && 'state' in part) { | |
| const toolPart = part as { toolCallId: string; state: string; errorText?: string; input?: unknown; output?: unknown }; | |
| fields.tool_toolCallId = toolPart.toolCallId; | |
| fields.tool_state = toolPart.state; | |
| fields.tool_errorText = toolPart.errorText; | |
| // Map specific tool input/output columns | |
| const inputKey = `tool_${toolName}_input`; | |
| const outputKey = `tool_${toolName}_output`; | |
| fields[inputKey] = toolPart.input; | |
| fields[outputKey] = toolPart.output; | |
| } | |
| } | |
| // Handle data parts | |
| else if (part.type.startsWith("data-")) { | |
| const dataType = part.type.replace("data-", ""); | |
| if (dataType === "weather" && 'id' in part && 'data' in part) { | |
| const dataPart = part as { id: string; data: { location?: string; weather?: string; temperature?: number } }; | |
| fields.data_weather_id = dataPart.id; | |
| fields.data_weather_location = dataPart.data?.location; | |
| fields.data_weather_weather = dataPart.data?.weather; | |
| fields.data_weather_temperature = dataPart.data?.temperature; | |
| } | |
| } | |
| break; | |
| } | |
| if (part.type.startsWith("tool-")) { | |
| const toolName = part.type.replace("tool-", ""); | |
| // Type guard to ensure this is a tool part with the expected properties | |
| if ('toolCallId' in part && 'state' in part) { | |
| const toolPart = part as { toolCallId: string; state: string; errorText?: string; input?: unknown; output?: unknown }; | |
| // Validate required fields | |
| if (!toolPart.toolCallId || !toolPart.state) { | |
| console.warn(`Tool part missing required fields: ${part.type}`); | |
| break; | |
| } | |
| fields.tool_toolCallId = toolPart.toolCallId; | |
| fields.tool_state = toolPart.state; | |
| fields.tool_errorText = toolPart.errorText; | |
| // Map specific tool input/output columns | |
| const inputKey = `tool_${toolName}_input`; | |
| const outputKey = `tool_${toolName}_output`; | |
| fields[inputKey] = toolPart.input; | |
| fields[outputKey] = toolPart.output; | |
| } | |
| } | |
| // Handle data parts | |
| else if (part.type.startsWith("data-")) { | |
| const dataType = part.type.replace("data-", ""); | |
| if (dataType === "weather" && 'id' in part && 'data' in part) { | |
| const dataPart = part as { id: string; data: { location?: string; weather?: string; temperature?: number } }; | |
| // Validate that data object exists | |
| if (!dataPart.data || typeof dataPart.data !== 'object') { | |
| console.warn(`Data part missing data object: ${part.type}`); | |
| break; | |
| } | |
| fields.data_weather_id = dataPart.id; | |
| fields.data_weather_location = dataPart.data?.location; | |
| fields.data_weather_weather = dataPart.data?.weather; | |
| fields.data_weather_temperature = dataPart.data?.temperature; | |
| } | |
| } | |
| break; | |
| } |
🤖 Prompt for AI Agents
In apps/web/client/src/app/api/chat/helpers/persistence.ts around lines 125 to
155, the code assumes tool and data parts have specific nested properties and
directly accesses them; add explicit runtime validation/type guards before
extracting fields: check that part has the expected keys (e.g., typeof part ===
'object' && 'toolCallId' in part && typeof part.toolCallId === 'string' and
similarly for state, errorText, input, output) before casting and assigning,
only create tool_<name>_input/output keys when input/output are present, and for
data parts ensure part.data is an object and has the expected typed properties
(location/weather as strings, temperature as number) before assigning to
data_weather_* fields; use optional chaining and default values or skip
assignments when validations fail to avoid runtime errors.
| onFinish: async ({ responseMessage, messages: allMessages }) => { | ||
| try { | ||
| // Save the new assistant response message with its parts | ||
| await saveMessageWithParts(responseMessage, conversationId); | ||
|
|
||
| console.log(`Saved message ${responseMessage.id} with ${responseMessage.parts?.length || 0} parts`); | ||
| } catch (error) { | ||
| console.error('Failed to save message during streaming:', error); | ||
| // Don't throw here to avoid breaking the stream | ||
| } | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Persisting streamed assistant parts on finish is solid; add an auth/ownership check for conversationId.
Before saving, verify the conversation exists and belongs to the provided project/user to prevent cross-tenant inserts via forged IDs (RLS may cover this, but defense-in-depth at the app layer is recommended).
Proposed guard (place early in streamResponse, after parsing body):
+import { db, conversations } from '@onlook/db';
+import { and, eq } from 'drizzle-orm';
@@
export const streamResponse = async (req: NextRequest, userId: string) => {
@@
- const { messages, chatType, conversationId, projectId } = body as {
+ const { messages, chatType, conversationId, projectId } = body as {
messages: UIMessage[],
chatType: ChatType,
conversationId: string,
projectId: string,
};
+ // Authorization/ownership guard
+ const convo = await db.query.conversations.findFirst({
+ where: and(eq(conversations.id, conversationId), eq(conversations.projectId, projectId)),
+ });
+ if (!convo) {
+ return new Response(JSON.stringify({ error: 'Conversation not found or unauthorized', code: 404 }), {
+ status: 404,
+ headers: { 'Content-Type': 'application/json' }
+ });
+ }Please also confirm RLS policies enforce tenant isolation for inserts into messages/parts when conversationId is spoofed.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== repo root ==="
pwd
ls -la
echo
echo "=== search for chat route file ==="
rg -n --hidden -S "apps/web/client/src/app/api/chat/route.ts" -g '!.git' || true
echo
echo "=== show apps/web/client/src/app/api/chat/route.ts (if exists) ==="
if [ -f "apps/web/client/src/app/api/chat/route.ts" ]; then
sed -n '1,240p' apps/web/client/src/app/api/chat/route.ts
else
echo "NOT FOUND: apps/web/client/src/app/api/chat/route.ts"
fi
echo
echo "=== search for saveMessageWithParts usages/definition ==="
rg -n --hidden -S "saveMessageWithParts" -g '!.git' -C3 || true
echo
echo "=== search for @onlook/db imports ==="
rg -n --hidden -S "@onlook/db" -g '!.git' -C3 || true
echo
echo "=== search for 'conversations' table/schema definitions ==="
rg -n --hidden -S "export const conversations|pgTable\\(\"conversations\"|const conversations\\b|\\bconversations\\b" -g '!.git' -C3 || true
echo
echo "=== search for messages/parts table definitions ==="
rg -n --hidden -S "export const messages|pgTable\\(\"messages\"|export const message_parts|pgTable\\(\"parts\"|message_parts|message_parts" -g '!.git' -C3 || true
echo
echo "=== search for migrations / SQL RLS policies ==="
rg -n --hidden -S "CREATE POLICY|ENABLE ROW LEVEL SECURITY|ENABLE ROW_LEVEL_SECURITY|ROW LEVEL SECURITY|CREATE ROLE|WITH CHECK" -g '!.git' -C3 || true
echo
echo "=== look for packages/schema or packages that might contain db schema ==="
if [ -d "packages" ]; then
rg -n --hidden -S "schema|drizzle|pgTable|CREATE POLICY|conversations|messages" packages -g '!.git' -C3 || true
ls -la packages || true
fi
echo
echo "=== list likely db/shared folders ==="
for d in "packages/schema" "packages/models" "schema" "libs/db" "packages/db" "apps/backend" "apps/backend/db" "apps/backend/supabase" "supabase" ; do
if [ -d "$d" ]; then
echo "--- $d ---"
ls -la "$d" || true
rg -n --hidden -S "conversations|messages|CREATE POLICY|ENABLE ROW LEVEL SECURITY|pgTable|drizzle" "$d" -g '!.git' -C3 || true
fi
doneLength of output: 12155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== ls helpers dir ==="
ls -la apps/web/client/src/app/api/chat/helpers || true
echo
echo "=== show persistence helper file ==="
if [ -f "apps/web/client/src/app/api/chat/helpers/persistence.ts" ]; then
sed -n '1,240p' apps/web/client/src/app/api/chat/helpers/persistence.ts || true
else
echo "NOT FOUND: apps/web/client/src/app/api/chat/helpers/persistence.ts"
fi
echo
echo "=== search for saveMessageWithParts across repo ==="
rg -n --hidden -S "saveMessageWithParts" -g '!.git' -C3 || true
echo
echo "=== search for conversations/messages/parts table definitions in packages/db ==="
rg -n --hidden -S "conversations|messages|parts|pgTable|drizzle" packages/db -g '!.git' -C3 || true
echo
echo "=== list packages/db/src ==="
ls -la packages/db/src || true
echo
echo "=== show packages/db src files that mention conversations/messages ==="
rg -n --hidden -S "conversations|messages|parts" packages/db/src -g '!.git' -C3 || true
echo
echo "=== list apps/backend/supabase/migrations ==="
ls -la apps/backend/supabase/migrations || true
echo
echo "=== search for CREATE POLICY / ENABLE ROW LEVEL SECURITY in migrations ==="
rg -n --hidden -S "CREATE POLICY|ENABLE ROW LEVEL SECURITY|ENABLE ROW_LEVEL_SECURITY|ROW LEVEL SECURITY|CREATE ROLE|WITH CHECK" apps/backend/supabase/migrations -g '!.git' -C3 || true
echo
echo "=== grep migrations for conversations/messages usage ==="
rg -n --hidden -S "conversations|messages|parts" apps/backend/supabase/migrations -g '!.git' -C3 || trueLength of output: 9130
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== repo root ==="
pwd
echo
echo "=== list schema dir ==="
ls -la packages/db/src/schema || true
echo
echo "=== search for 'conversations' in repo ==="
rg -n --hidden -S '\bconversations\b' || true
echo
echo "=== search for 'messages' in repo ==="
rg -n --hidden -S '\bmessages\b' || true
echo
echo "=== show schema files that look relevant ==="
for f in packages/db/src/schema/*; do
if rg -n --hidden -S 'conversations|messages|parts' "$f" >/dev/null 2>&1; then
echo "---- $f ----"
sed -n '1,240p' "$f" || true
echo
fi
done
echo "=== show persistence again header ==="
sed -n '1,220p' apps/web/client/src/app/api/chat/helpers/persistence.ts || true
echo
echo "=== show RLS migration 0006_rls.sql (first 400 lines) ==="
sed -n '1,400p' apps/backend/supabase/migrations/0006_rls.sql || true
echo
echo "=== show realtime RLS migration 0007_realtime_rls.sql (first 400 lines) ==="
sed -n '1,400p' apps/backend/supabase/migrations/0007_realtime_rls.sql || true
echo
echo "=== search migrations for CREATE POLICY / ENABLE ROW LEVEL SECURITY ==="
rg -n --hidden -S "CREATE POLICY|ENABLE ROW LEVEL SECURITY|ENABLE ROW_LEVEL_SECURITY|ROW LEVEL SECURITY|auth.uid|auth\.role|auth.uid\(\)" apps/backend/supabase/migrations || trueLength of output: 81843
Enforce conversation ownership before persisting streamed message parts
- streamResponse (apps/web/client/src/app/api/chat/route.ts — after parsing body) calls saveMessageWithParts(...) without verifying the conversation belongs to the request’s project/user; add an early guard that queries conversations by id + projectId (or verifies project membership for the user) and return 404/401 if not authorized.
- saveMessageWithParts (apps/web/client/src/app/api/chat/helpers/persistence.ts) writes messages and parts directly — don’t rely solely on RLS.
- RLS is enabled and user_has_project_access exists (apps/backend/supabase/migrations/0006_rls.sql), but I did not find explicit INSERT/UPDATE policies for messages/parts in the migrations — confirm those RLS policies block spoofed conversationId inserts or keep the app-layer guard.
🤖 Prompt for AI Agents
In apps/web/client/src/app/api/chat/route.ts around lines 136-146, add an early
authorization guard after parsing the request body that verifies the
conversation belongs to the request's project/user (e.g., query conversations by
id + projectId or verify user project membership using user_id/project_id) and
return 404 or 401 if the check fails before calling saveMessageWithParts; also
update apps/web/client/src/app/api/chat/helpers/persistence.ts
(saveMessageWithParts) to perform the same explicit ownership check prior to any
INSERT/UPDATE (do not rely solely on RLS) — query the conversation row, validate
projectId/user access, throw a controlled 401/404 on mismatch, and ensure any
downstream save logs and errors are handled accordingly; finally, confirm or add
DB RLS policies that prevent inserting messages/parts for conversations the
user/project does not own, but keep the application-layer guard as described.
| @@ -1,3 +1,4 @@ | |||
| export type { DrizzleDb } from './client'; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Sweep for legacy deep imports.
Ensure no callers still import @onlook/db/src/client.
🏁 Script executed:
#!/bin/bash
# Find legacy deep imports that bypass the barrel.
rg -nP "@onlook/db/src/client" -S || trueLength of output: 1582
Replace legacy deep imports from '@onlook/db/src/client' with package barrel (@onlook/db)
Change imports like import { db } from '@onlook/db/src/client' → import { db } from '@onlook/db' (and import type { DrizzleDb } from '@onlook/db').
Affected files:
- packages/db/src/seed/stripe/legacy-subscription.ts
- packages/db/src/seed/stripe/stripe.ts
- packages/db/src/seed/stripe/backfill-subscriptions.ts
- packages/db/src/seed/db.ts
- apps/web/client/src/trpc/request-server.ts
- apps/web/client/src/server/api/trpc.ts
- apps/web/client/src/server/api/routers/publish/helpers/deploy.ts
- apps/web/client/src/server/api/routers/publish/helpers/helpers.ts
- apps/web/client/src/server/api/routers/publish/helpers/unpublish.ts
- apps/web/client/src/server/api/routers/publish/helpers/publish.ts
- apps/web/client/src/server/api/routers/domain/verify/helpers/helpers.ts
- apps/web/client/src/app/webhook/stripe/subscription/delete.ts
- apps/web/client/src/app/webhook/stripe/subscription/update.ts
- apps/web/client/src/app/webhook/stripe/subscription/create.ts
- apps/web/client/src/app/api/chat/helpers/persistence.ts
🤖 Prompt for AI Agents
In packages/db/src/index.ts around line 1, the module currently re-exports types
from a local deep path ('./client'); replace that deep import with the package
barrel export and update consumers to use the package root. Change the export to
re-export DrizzleDb from '@onlook/db' (export type { DrizzleDb } from
'@onlook/db') and then update all listed affected files to import runtime and
type symbols from '@onlook/db' instead of '@onlook/db/src/client' to remove
legacy deep imports.
| if (dataType === "weather" && 'id' in part && 'weather' in part) { | ||
| const weatherPart = part as { id: string; weather: { location?: string; weather?: string; temperature?: number } }; | ||
| return { | ||
| ...baseFields, | ||
| data_weather_id: weatherPart.id, | ||
| data_weather_location: weatherPart.weather?.location, | ||
| data_weather_weather: weatherPart.weather?.weather, | ||
| data_weather_temperature: weatherPart.weather?.temperature, | ||
| } as DbNewPart; |
There was a problem hiding this comment.
Fix weather data property access.
The code should access weather data through part.data not part.weather to align with the UI message part structure.
if (dataType === "weather" && 'id' in part && 'weather' in part) {
- const weatherPart = part as { id: string; weather: { location?: string; weather?: string; temperature?: number } };
+ const weatherPart = part as { id: string; data: { location?: string; weather?: string; temperature?: number } };
return {
...baseFields,
data_weather_id: weatherPart.id,
- data_weather_location: weatherPart.weather?.location,
- data_weather_weather: weatherPart.weather?.weather,
- data_weather_temperature: weatherPart.weather?.temperature,
+ data_weather_location: weatherPart.data?.location,
+ data_weather_weather: weatherPart.data?.weather,
+ data_weather_temperature: weatherPart.data?.temperature,
} as DbNewPart;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/db/src/mappers/chat/parts.ts around lines 167 to 175, the mapping
incorrectly reads weather fields from part.weather; update it to read from
part.data (e.g., treat part as { id: string; data: { location?: string;
weather?: string; temperature?: number } }) and assign data_weather_location =
part.data?.location, data_weather_weather = part.data?.weather,
data_weather_temperature = part.data?.temperature; keep the existing baseFields
and data_weather_id assignment from part.id and ensure the type cast reflects
the data property access.
| @@ -1,32 +1,69 @@ | |||
| import { BUILD_TOOL_SET } from '@onlook/ai'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid runtime coupling to @onlook/ai in models; import the value type-only.
Prevents accidental bundling and cycles from models → ai. Safe since it’s used only in a type position.
-import { BUILD_TOOL_SET } from '@onlook/ai';
+import type { BUILD_TOOL_SET } from '@onlook/ai';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { BUILD_TOOL_SET } from '@onlook/ai'; | |
| import type { BUILD_TOOL_SET } from '@onlook/ai'; |
🤖 Prompt for AI Agents
In packages/models/src/chat/message/message.ts at line 1, the file currently
imports BUILD_TOOL_SET as a runtime value which creates a runtime coupling to
@onlook/ai; change the import to a type-only import (e.g., use "import type {
BUILD_TOOL_SET } from '@onlook/ai'") so the symbol is erased at compile time and
prevents accidental bundling/cycles while preserving the type usage.
| parent_conversation_id UUID REFERENCES conversations(id) ON DELETE CASCADE, | ||
| parent_message_id UUID REFERENCES messages(id) ON DELETE CASCADE | ||
| ); | ||
|
|
||
| -- Indexes for efficient subchat queries | ||
| CREATE INDEX conversations_parent_conversation_idx ON conversations(parent_conversation_id); | ||
| CREATE INDEX conversations_parent_message_idx ON conversations(parent_message_id); | ||
| ``` |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Docs vs schema consistency: ensure FKs actually exist in code.
The doc shows FK constraints with ON DELETE CASCADE, but the Drizzle schema snippet (packages/db/src/schema/chat/conversation.ts) indicates plain uuid columns plus indexes. Please confirm FKs are present in the migration and reflected in the ORM schema; if not, add them to prevent orphaned subchats.
If missing in the Drizzle schema, update it roughly as:
parentConversationId: uuid('parent_conversation_id')
.references(() => conversations.id, { onDelete: 'cascade', onUpdate: 'cascade' }),
parentMessageId: uuid('parent_message_id')
.references(() => messages.id, { onDelete: 'cascade', onUpdate: 'cascade' }),Verification script to locate FK definitions:
🏁 Script executed:
#!/bin/bash
fd -a --hidden --exclude node_modules '(conversation|messages)\.ts|migrations' \
| xargs -I{} rg -n --hidden -C2 'parent_conversation_id|parent_message_id|references\(' {}Length of output: 8291
Add FK references to the Drizzle schema to match the migration
Migration apps/backend/supabase/migrations/0019_cuddly_iron_man.sql adds FOREIGN KEYs with ON DELETE/ON UPDATE CASCADE, but packages/db/src/schema/chat/conversation.ts currently declares parentConversationId and parentMessageId as plain uuid(...) columns. Update the Drizzle schema to:
parentConversationId: uuid('parent_conversation_id')
.references(() => conversations.id, { onDelete: 'cascade', onUpdate: 'cascade' }),
parentMessageId: uuid('parent_message_id')
.references(() => messages.id, { onDelete: 'cascade', onUpdate: 'cascade' }),Files: apps/backend/supabase/migrations/0019_cuddly_iron_man.sql, packages/db/src/schema/chat/conversation.ts
🤖 Prompt for AI Agents
In subchat-implementation-example.md around lines 56-63 (and applied to
packages/db/src/schema/chat/conversation.ts), the Drizzle schema defines
parentConversationId and parentMessageId as plain uuid columns but the DB
migration adds FOREIGN KEYs with ON DELETE/ON UPDATE CASCADE; change those two
column definitions to use .references(() => conversations.id, { onDelete:
'cascade', onUpdate: 'cascade' }) and .references(() => messages.id, { onDelete:
'cascade', onUpdate: 'cascade' }) respectively, ensuring the conversations and
messages table symbols are imported and the uuid helper remains used so the
Drizzle schema matches the migration.
There was a problem hiding this comment.
Actionable comments posted: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/preload/script/api/style/css-manager.ts (1)
128-159: Use css-tree List APIs in updateRule; avoid .filter()/.push onList.
rule.block.childrenis aList, not an array. Using.filter()/.push()is unsafe. LeverageforEachwith item handles andremove/append. Also parse values as AST (context: 'Value') instead ofRawto interoperate with existing rules.- updateRule(rule: Rule, property: string, value: string) { - let found = false; - walk(rule.block, { - visit: 'Declaration', - enter: (decl: Declaration) => { - if (decl.property === property) { - decl.value = { type: 'Raw', value: value }; - if (value === '') { - rule.block.children = rule.block.children.filter( - (decl: CssNode) => (decl as Declaration).property !== property, - ); - } - found = true; - } - }, - }); - - if (!found) { - if (value === '') { - rule.block.children = rule.block.children.filter( - (decl: CssNode) => (decl as Declaration).property !== property, - ); - } else { - rule.block.children.push({ - type: 'Declaration', - property: property, - value: { type: 'Raw', value: value }, - important: false, - }); - } - } - } + updateRule(rule: Rule, property: string, value: string) { + let found = false; + // mutate in-place using List item handles + rule.block.children.forEach((node, item) => { + if (node.type === 'Declaration' && node.property === property) { + if (value === '') { + rule.block.children.remove(item); + } else { + node.value = parse(value, { context: 'Value' }); + } + found = true; + } + }); + if (!found && value !== '') { + rule.block.children.append({ + type: 'Declaration', + property, + value: parse(value, { context: 'Value' }), + important: false, + } as Declaration); + } + }packages/db/src/mappers/chat/conversation.ts (1)
5-12: Fix title/displayName mapping and use nullish coalescing in conversation mappersfromDbConversation does not populate ChatConversation.title from the DB's displayName, while toDbConversation writes displayName = conversation.title || null — this can null-out an existing displayName on a round-trip. Populate title from displayName and replace || with ?? for nullish defaults.
packages/db/src/mappers/chat/conversation.ts — fromDbConversation: set
- title: (conversation as any).title ?? conversation.displayName ?? undefined
- metadata.suggestions: conversation.suggestions ?? []
- parentConversationId: conversation.parentConversationId ?? undefined
- parentMessageId: conversation.parentMessageId ?? undefined
packages/db/src/mappers/chat/conversation.ts — toDbConversation: set
- displayName: conversation.title ?? null
- suggestions: conversation.metadata?.suggestions ?? []
- parentConversationId: conversation.parentConversationId ?? null
- parentMessageId: conversation.parentMessageId ?? null
apps/web/client/src/server/api/routers/chat/message.ts (1)
105-109: Stop writing JSON parts in update() or also sync relational parts.This writes messages.parts JSON, but reads now use relational parts; data will appear out-of-sync.
await ctx.db.update(messages).set({ ...input.message, role: input.message.role as ChatMessageRole, - parts: input.message.parts as Message['parts'], }).where(eq(messages.id, input.messageId));Follow-up: either remove parts from messageUpdateSchema or introduce a dedicated updateParts path that mirrors the upsert transaction.
♻️ Duplicate comments (6)
apps/web/client/src/app/terms-of-service/page.tsx (1)
89-89: Fix legal text typo: “neither party h authority” → “neither party has any authority”.User-facing legal copy; the current typo changes meaning and looks unprofessional. Please correct.
- This Agreement (including the Registration) represents the entire agreement between User and Onlook with respect to the subject matter hereof, and supersedes all prior or contemporaneous communications and proposals (whether oral, written or electronic) between User and Onlook with respect thereto. In the event of any conflict between these Terms and the Registration, the Registration shall control. The Agreement shall be governed by and construed in accordance with the laws of the State of New York, excluding its conflicts of law rules, and the parties consent to exclusive jurisdiction and venue in the state and federal courts located in New York, NY. All notices under this Agreement shall be in writing and shall be deemed to have been duly given when received, if personally delivered or sent by certified or registered mail, return receipt requested; when receipt is electronically confirmed, if transmitted by facsimile or e-mail; or the day after it is sent, if sent for next day delivery by recognized overnight delivery service. Notices must be sent to the contacts for each party set forth in the Registration. Either party may update its address set forth above by giving notice in accordance with this section. Neither party may assign any of its rights or obligations hereunder without the other party's consent; provided that either party may assign all of its rights and obligations hereunder without such consent to a successor-in-interest in connection with a sale of substantially all of such party's business relating to this Agreement. No agency, partnership, joint venture, or employment relationship is created as a result of this Agreement and neither party h authority of any kind to bind the other in any respect. In any action or proceeding to enforce rights under this Agreement, the prevailing party shall be entitled to recover costs and attorneys' fees. If any provision of this Agreement is held to be unenforceable for any reason, such provision shall be reformed only to the extent necessary to make it enforceable. The failure of either party to act with respect to a breach of this Agreement by the other party shall not constitute a waiver and shall not limit such party's rights with respect to such breach or any subsequent breaches. + This Agreement (including the Registration) represents the entire agreement between User and Onlook with respect to the subject matter hereof, and supersedes all prior or contemporaneous communications and proposals (whether oral, written or electronic) between User and Onlook with respect thereto. In the event of any conflict between these Terms and the Registration, the Registration shall control. The Agreement shall be governed by and construed in accordance with the laws of the State of New York, excluding its conflicts of law rules, and the parties consent to exclusive jurisdiction and venue in the state and federal courts located in New York, NY. All notices under this Agreement shall be in writing and shall be deemed to have been duly given when received, if personally delivered or sent by certified or registered mail, return receipt requested; when receipt is electronically confirmed, if transmitted by facsimile or e-mail; or the day after it is sent, if sent for next day delivery by recognized overnight delivery service. Notices must be sent to the contacts for each party set forth in the Registration. Either party may update its address set forth above by giving notice in accordance with this section. Neither party may assign any of its rights or obligations hereunder without the other party's consent; provided that either party may assign all of its rights and obligations hereunder without such consent to a successor-in-interest in connection with a sale of substantially all of such party's business relating to this Agreement. No agency, partnership, joint venture, or employment relationship is created as a result of this Agreement and neither party has any authority of any kind to bind the other in any respect. In any action or proceeding to enforce rights under this Agreement, the prevailing party shall be entitled to recover costs and attorneys' fees. If any provision of this Agreement is held to be unenforceable for any reason, such provision shall be reformed only to the extent necessary to make it enforceable. The failure of either party to act with respect to a breach of this Agreement by the other party shall not constitute a waiver and shall not limit such party's rights with respect to such breach or any subsequent breaches.apps/backend/supabase/migrations/0019_cuddly_iron_man.sql (1)
1-8: Risk: FK cascade cycle between conversations and messages (consider SET NULL on parent_message_id).As noted earlier,
conversations.parent_message_id -> messages.id (ON DELETE CASCADE)plusmessages.conversation_id -> conversations.id (likely CASCADE)forms a cycle. Deleting a message can cascade-delete a conversation, which can cascade-delete its messages, etc. High blast radius; hard-to-reason deletes.Recommend breaking the cycle by making
parent_message_idnon-cascading:-- follow-up migration ALTER TABLE "conversations" DROP CONSTRAINT IF EXISTS "conversations_parent_message_id_messages_id_fk"; ALTER TABLE "conversations" ADD CONSTRAINT "conversations_parent_message_id_messages_id_fk" FOREIGN KEY ("parent_message_id") REFERENCES "public"."messages"("id") ON DELETE SET NULL ON UPDATE CASCADE;Optional: keep hierarchical cleanup via
parent_conversation_idCASCADE (already present), and add a CHECK or trigger to ensureparent_*rows share the sameproject_id.apps/web/client/src/app/api/chat/helpers/persistence.ts (1)
65-81: Avoid delete+insert race on parts; use upsert with (messageId, order).Concurrent writers can clobber each other. Prefer a unique index and upsert; or lock the parent message row before replace.
- // Delete existing parts for this message - await tx.delete(parts).where(eq(parts.messageId, typedMessage.id)); - - // Convert and insert new parts - const dbParts = typedMessage.parts.map((part, index) => ({ - messageId: typedMessage.id, - type: part.type, - order: index, - createdAt: new Date(), - ...convertUIPartToDbFields(part) - })); - - if (dbParts.length > 0) { - await tx.insert(parts).values(dbParts); - } + // Convert using shared mapper and upsert by (messageId, order) + const dbParts = toDbParts(typedMessage.parts, typedMessage.id); + if (dbParts.length > 0) { + await tx.insert(parts) + .values(dbParts) + .onConflictDoUpdate({ + target: [parts.messageId, parts.order], + set: { + type: sql`excluded.type`, + providerMetadata: sql`excluded.provider_metadata`, + // set other columns generically to excluded.* as needed + }, + }); + }Follow-up (schema): add
uniqueIndex('parts_message_id_order_uniq').on(table.messageId, table.order).apps/web/client/src/server/api/routers/chat/conversation.ts (1)
149-171: Prevent infinite recursion and cycles ingetConversationTree.Add depth limit and cycle detection.
- const buildTree = async (parentId: string): Promise<any> => { + const buildTree = async ( + parentId: string, + maxDepth: number = 10, + visited: Set<string> = new Set(), + ): Promise<any> => { + if (maxDepth <= 0 || visited.has(parentId)) return null; + visited.add(parentId); const conversation = await ctx.db.query.conversations.findFirst({ where: eq(conversations.id, parentId), }); ... - const subchatTrees = await Promise.all( - subchats.map(subchat => buildTree(subchat.id)) - ); + const subchatTrees = await Promise.all( + subchats.map(subchat => buildTree(subchat.id, maxDepth - 1, new Set(visited))) + ); ... - return await buildTree(input.conversationId); + return await buildTree(input.conversationId);packages/db/src/mappers/chat/parts.ts (1)
55-69: Dynamic tool keys bypass types; constrain to known tools.Prefer a typed map of allowed tool names to columns; avoids accessing non-existent properties at runtime.
I can generate a typed
const AllowedTools = {...}map and helpers to read/writeinput/outputsafely.packages/db/src/schema/chat/parts.ts (1)
149-152: Tighten tool type check and add uniqueness on (messageId, order).Regex avoids false positives; uniqueness enables safe upserts and stable ordering.
- toolCallFieldsRequired: check( - "tool_call_fields_required", - sql`CASE WHEN ${table.type} LIKE 'tool-%' THEN ${table.tool_toolCallId} IS NOT NULL AND ${table.tool_state} IS NOT NULL ELSE TRUE END` - ), + toolCallFieldsRequired: check( + "tool_call_fields_required", + sql`CASE WHEN ${table.type} ~ '^tool-[A-Za-z0-9_-]+$' THEN ${table.tool_toolCallId} IS NOT NULL AND ${table.tool_state} IS NOT NULL ELSE TRUE END` + ), + messageIdOrderUniq: uniqueIndex("parts_message_id_order_uniq").on(table.messageId, table.order),
🧹 Nitpick comments (39)
apps/web/client/src/app/projects/import/local/_components/select-folder.tsx (2)
64-65: Type: ensure File.webkitRelativePath is declaredWithout a global augmentation, TS often errors on File.webkitRelativePath after removing casts. Add this once to the app types.
Create types/dom-augmentations.d.ts:
export {}; declare global { interface File { webkitRelativePath?: string; } }Also applies to: 138-139
253-258: Give fileEntry a concrete type (avoid implicit any)Use a minimal DOM FS typing to keep safety without reverting to
as any.- const fileEntry = entry; + const fileEntry: FileSystemFileEntry = entry;Add types/filesystem.d.ts:
export {}; declare global { interface FileSystemEntry { isFile: boolean; isDirectory: boolean; name: string; fullPath: string; // starts with '/' } interface FileSystemFileEntry extends FileSystemEntry { file(success: (file: File) => void, error?: (err: DOMException) => void): void; } interface FileSystemDirectoryEntry extends FileSystemEntry { createReader(): FileSystemDirectoryReader; } interface FileSystemDirectoryReader { readEntries( success: (entries: FileSystemEntry[]) => void, error?: (err: DOMException) => void ): void; } interface DataTransferItem { webkitGetAsEntry?: () => FileSystemEntry | null; } }apps/web/preload/script/api/style/css-manager.ts (2)
161-179: Don’t assumedecl.valueis Raw; generate it.Existing CSS likely parses to structured values (not
Raw). Usegenerate(decl.value)for correctness across both parsed and programmatically added declarations.- styles[this.cssToJsProperty(decl.property)] = (decl.value as Raw).value; + styles[this.cssToJsProperty(decl.property)] = generate(decl.value);
200-206: Remove properties using List removal, not array.filter().
node.block.childrenis aList. Iterate andremove(item)when the property matches.- node.block.children = node.block.children.filter( - (decl: CssNode) => !cssProperties.includes((decl as Declaration).property), - ); + node.block.children.forEach((decl, item) => { + if (decl.type === 'Declaration' && cssProperties.includes(decl.property)) { + node.block.children.remove(item); + } + });apps/web/client/src/app/project/[id]/_components/right-panel/code-tab/file-operations.ts (1)
59-62: Use declared MAX_NAME_LENGTH instead of hardcoded 255.Keeps constraints consistent and centralized.
Apply:
- if (fileName.length > 255) { + if (fileName.length > FILE_CONSTRAINTS.MAX_NAME_LENGTH) { ... - if (folderName.length > 255) { + if (folderName.length > FILE_CONSTRAINTS.MAX_NAME_LENGTH) {Also applies to: 82-85
apps/web/client/src/components/store/editor/sandbox/terminal.ts (1)
164-190: Avoid re-entrant override calls; write via captured original.Inside the override, calling
terminal.write(...)re-enters the wrapper N times. Call the boundoriginalWritefor the injected escape sequences.Apply:
- const originalWrite = terminal.write.bind(terminal); - (terminal).write = (data: string | Uint8Array, callback?: () => void) => { + const originalWrite = terminal.write.bind(terminal); + (terminal.write as Terminal['write']) = (data: string | Uint8Array, callback?: () => void) => { if (typeof data === 'string') { // Detect Claude Code's redraw pattern: multiple line clears with cursor movement const lineUpPattern = /(\x1b\[2K\x1b\[1A)+\x1b\[2K\x1b\[G/; if (lineUpPattern.test(data)) { // Count how many lines are being cleared const matches = data.match(/\x1b\[1A/g); const lineCount = matches ? matches.length : 0; // Clear the number of lines being redrawn plus some buffer for (let i = 0; i <= lineCount + 2; i++) { - terminal.write('\x1b[2K\x1b[1A\x1b[2K'); + originalWrite('\x1b[2K\x1b[1A\x1b[2K'); } - terminal.write('\x1b[G'); // Go to beginning of line + originalWrite('\x1b[G'); // Go to beginning of line // Extract just the content after the clearing commands const contentMatch = /\x1b\[G(.+)$/s.exec(data); if (contentMatch?.[1]) { return originalWrite(contentMatch[1], callback); } } } return originalWrite(data, callback); };apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (1)
54-60: Guard against missing conversationId before mapping.Prevents creating messages with empty thread IDs; also drop the
?? ''fallback.Apply:
- const currentConversationId = editorEngine.chat.conversation.current?.conversation.id; - editorEngine.chat.conversation.addOrReplaceMessage(toOnlookMessageFromVercel({ + const currentConversationId = editorEngine.chat.conversation.current?.conversation.id; + if (!currentConversationId) { + console.error('Missing conversation id; skipping message persistence'); + return; + } + editorEngine.chat.conversation.addOrReplaceMessage(toOnlookMessageFromVercel({ id: message.id, parts: message.parts, role: message.role, metadata: message.metadata - }, currentConversationId ?? '')); + }, currentConversationId));apps/web/client/src/components/store/editor/theme/index.ts (3)
409-411: Prefer a type guard in the finder to remove fragile'in'checksAnnotate the predicate so
groupPropnarrows toT.ObjectProperty, avoiding subsequent'value' in groupPropchecks.-const groupProp = colorObj.properties.find((prop) => - isValidTailwindConfigProperty(prop, camelCaseName), -); +const groupProp = colorObj.properties.find( + (prop): prop is T.ObjectProperty => + isValidTailwindConfigProperty(prop, camelCaseName), +);
413-421: Normalize shade key before lookup; current delete-by-color can miss matches
colorNamemay come from UI unsanitized; matching is case/format sensitive. Normalize to camelCase (consistent with group) beforefindIndex.- if (colorName) { - // Delete specific color within group - const colorIndex = groupProp.value.properties.findIndex( - (prop) => isValidTailwindConfigProperty(prop, colorName), - ); + if (colorName) { + // Delete specific color within group + const normalizedShade = camelCase(colorName); + const colorIndex = groupProp.value.properties.findIndex( + (prop) => isValidTailwindConfigProperty(prop, normalizedShade), + );
451-463: Make CSS variable removal robust; endsWith/startsWith is brittleMatch “--varName:” with regex so we don’t miss lines with prefixes/whitespace or delete false positives.
-const shouldKeep = !trimmedLine.endsWith(`--${camelCaseName}-${colorName}`); +const shouldKeep = !new RegExp( + String.raw`(^|;)\s*--${camelCaseName}-${colorName}\s*:`, +).test(trimmedLine); -const shouldKeep = !trimmedLine.startsWith(`--${camelCaseName}`); +const shouldKeep = !new RegExp( + String.raw`(^|;)\s*--${camelCaseName}(?:-[\w-]+)?\s*:`, +).test(trimmedLine);apps/web/client/src/app/project/[id]/_components/left-panel/layers-tab/index.tsx (2)
151-152: Avoid double-cast; pass a typed component to renderRowPrefer a single, precise cast to
ComponentType<RowRendererProps<LayerNode>>.-renderRow={TreeRow as unknown as ElementType<RowRendererProps<LayerNode>>} +renderRow={TreeRow as React.ComponentType<RowRendererProps<LayerNode>>}
33-105: Stale-closure risk in handleDragEnd/useCallback with empty depsCallback closes over
editorEngineand nested fields but has[]deps. If those refs change, behavior can desync.Consider adding stable refs (e.g.,
const engineRef = useRef(editorEngine)) or include needed stable slices in deps to avoid stale captures.apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx (1)
16-16: Don’t widen label toany; keep typed translation keysTyping to
anymasks errors. Constrain to the known tab keys.-const tabs: { value: LeftPanelTabValue; icon: React.ReactNode; label: any; disabled?: boolean }[] = +const tabs: { + value: LeftPanelTabValue; + icon: React.ReactNode; + label: keyof typeof transKeys.editor.panels.layers.tabs; + disabled?: boolean; +}[] =apps/web/client/src/app/_components/shared/mockups/tailwind-color-editor.tsx (1)
15-18: Reduceunknown ascasts for iconsIf possible, import icon prop types (e.g.,
IconProps) and cast once:const ImageIcon = Icons.Image as React.FC<IconProps>.apps/web/client/src/components/store/editor/pages/helper.ts (1)
178-186: Remove ts-ignore by narrowing assignment targetUse a typed index to assign dynamically without suppressing TS checks.
-// @ts-ignore -(metadata)[key] = prop.value.value; +(metadata as Record<string, unknown>)[key] = prop.value.value; -// @ts-ignore -(metadata)[key] = extractObjectValue(prop.value); +(metadata as Record<string, unknown>)[key] = extractObjectValue(prop.value); -// @ts-ignore -(metadata)[key] = extractArrayValue(prop.value); +(metadata as Record<string, unknown>)[key] = extractArrayValue(prop.value);apps/web/preload/script/api/state.ts (2)
3-7: Make_onlookFrameIdoptional to reflect runtime and avoid unsound typingAt startup the property is absent; typing it as required
stringmasks that and can mislead callers. Make it optional.declare global { interface Window { - _onlookFrameId: string; + _onlookFrameId?: string; } }
17-20: Harden fallback: handle rejection frompenpalParent.getFrameId()Uncaught rejection will be noisy and the warning may spam. At minimum, add a catch.
- penpalParent?.getFrameId().then((id) => { - setFrameId(id); - }); + penpalParent?.getFrameId() + .then((id) => setFrameId(id)) + .catch((err) => console.warn('Failed to resolve frame id from parent', err));apps/web/client/test/tools/grep.test.ts (2)
23-27: Type safety ofcreateMockEditorEngineGood removal of
as any. Please verifyEditorEnginehasn’t grown additional required keys; if it has, prefer a minimal test-only interface instead of widening toany.Example:
type MinimalEditorEngine = Pick<EditorEngine, 'branches'>; const createMockEditorEngine = (sandbox: any): MinimalEditorEngine => ({ branches: { getSandboxById: mock((id: string) => sandbox) } });
715-718: Nice coverage of control characters; consider ANSI escapes tooYou scrub BEL/CTRL; add a case with ANSI escapes (e.g.,
\x1b[31m) to ensure stripping/handling is consistent.// e.g., mock output: 'src/file.ts:\x1b[31mred\x1b[0m text'apps/web/client/test/template-nodes/index.test.ts (1)
248-276: Mock lifecycle: clear is good; ensure isolation if tests expand
mockClear()onaddOidsToAstis good. If additional tests import the same module, considermock.restore()inafterEachto avoid cross-test leakage.packages/models/src/chat/message/message.ts (2)
14-24: ClarifydataPartSchemanaming
weather.weatheris ambiguous. Consider renaming the inner field (e.g.,summaryorstatus) to reduce confusion at call sites.export const dataPartSchema = z.object({ - weather: z.object({ - weather: z.string().optional(), + weather: z.object({ + summary: z.string().optional(), location: z.string().optional(), temperature: z.number().optional(), loading: z.boolean().default(true), }), });
45-49: Relax metadata requirements or provide defaultsRequiring
contextandcheckpointson all messages is brittle. Make them optional or document defaults ([]) at construction/mapping boundaries.export interface InternalChatMetadata { vercelId?: string; - context: MessageContext[]; - checkpoints: MessageCheckpoints[]; + context?: MessageContext[]; + checkpoints?: MessageCheckpoints[]; } interface BaseChatMessage { @@ - metadata: InternalChatMetadata; + metadata: InternalChatMetadata;And ensure mappers populate
[]when absent.Also applies to: 57-59
apps/web/client/src/server/api/routers/github.ts (1)
76-76: Safer account login fallback withoutascast.Prevent brittle casts by guarding both union fields.
- login: 'login' in installation.data.account ? installation.data.account.login : (installation.data.account as { name: string }).name || '', + login: 'login' in installation.data.account + ? installation.data.account.login ?? '' + : ('name' in installation.data.account ? (installation.data.account.name ?? '') : ''),Also consider validating
installationIdis numeric beforeparseIntto avoidNaN:- installation_id: parseInt(installationId, 10), + installation_id: Number.isInteger(Number(installationId)) ? Number(installationId) : (() => { throw new TRPCError({ code: 'BAD_REQUEST', message: 'Invalid installationId' }); })(),apps/web/client/src/app/api/chat/route.ts (1)
136-146: Persist-on-finish is solid; add minimal context to logs.Keep non-fatal behavior; include IDs to aid tracing.
- console.log(`Saved message ${responseMessage.id} with ${responseMessage.parts?.length || 0} parts`); + console.log(`Saved message ${responseMessage.id} with ${responseMessage.parts?.length || 0} parts (conversationId=${conversationId}, userId=${userId})`);Optionally emit a metric (e.g., trackEvent) for persistence success/failure.
packages/models/src/chat/conversation/index.ts (1)
18-24: Reduce type duplication with a generic
ChatConversationWithSubchatsandChatConversationTreediffer only by child type. Consider a single generic alias:type ChatConversationWith<T extends ChatConversation> = ChatConversation & { subchats: T[] }.packages/db/src/schema/chat/conversation.ts (1)
45-46: Typo in relation name constant (case mismatch)
CONVERSATION_MESSAGe_RELATION_NAMEhas a stray lowercasee. Standardize the name to avoid future import mistakes.Apply (paired with message.ts):
-import { CONVERSATION_MESSAGe_RELATION_NAME, messages } from "./message"; +import { CONVERSATION_MESSAGE_RELATION_NAME, messages } from "./message"; ... - relationName: CONVERSATION_MESSAGe_RELATION_NAME, + relationName: CONVERSATION_MESSAGE_RELATION_NAME,subchat-implementation-example.md (3)
11-20: Call out validation requirementsAdd a note that
parentMessageIdmust belong toparentConversationId, and both must belong to the sameprojectId. This prevents cross-project links.
61-63: Document projectId index for roots listingInclude
CREATE INDEX conversations_project_id_idx ON conversations(project_id);since root conversations are queried byprojectId.
55-58: Reflect ON UPDATE CASCADE in docs (minor clarity)You already show
ON DELETE CASCADE. AddON UPDATE CASCADEto mirror the actual FKs defined in code/migrations.packages/db/src/schema/chat/message.ts (2)
33-49: Name collision: relation 'parts' vs column 'parts'Using
partsfor both the column and the relation is confusing. Prefer a distinct relation key (e.g.,partRowsormessageParts) to avoid accidental misuse.Apply:
- parts: many(parts, { + messageParts: many(parts, { relationName: "message_parts", }),Note: update downstream
.with({ messageParts: true }).
9-9: Constant name typo leaks across modulesRename
CONVERSATION_MESSAGe_RELATION_NAME→CONVERSATION_MESSAGE_RELATION_NAMEand update imports/usages (paired change in conversation.ts).-export const CONVERSATION_MESSAGe_RELATION_NAME = 'conversation_messages'; +export const CONVERSATION_MESSAGE_RELATION_NAME = 'conversation_messages';apps/web/client/src/server/api/routers/chat/conversation.ts (1)
88-125: Wrap parent checks + insert in a single transaction.Prevents TOCTOU between validation and insert; keeps invariants consistent.
- .mutation(async ({ ctx, input }) => { + .mutation(async ({ ctx, input }) => ctx.db.transaction(async (tx) => { // Verify parent conversation exists and belongs to the same project - const parentConversation = await ctx.db.query.conversations.findFirst({ + const parentConversation = await tx.query.conversations.findFirst({ where: eq(conversations.id, input.parentConversationId), }); ... - const parentMessage = await ctx.db.query.messages.findFirst({ + const parentMessage = await tx.query.messages.findFirst({ where: eq(messages.id, input.parentMessageId), }); ... - const [subchat] = await ctx.db.insert(conversations).values({ + const [subchat] = await tx.insert(conversations).values({ projectId: input.projectId, parentConversationId: input.parentConversationId, parentMessageId: input.parentMessageId || null, displayName: input.displayName || null, suggestions: [], }).returning(); ... - return fromDbConversation(subchat); - }), + return fromDbConversation(subchat); + })),packages/db/src/mappers/chat/parts.ts (1)
8-8: Remove unused locals.
order,createdAtare unused after destructure.- const { type, order, createdAt, providerMetadata } = part; + const { type, providerMetadata } = part;packages/db/src/schema/chat/parts.ts (2)
24-25: Add non-negative order check.Prevents invalid negative ranks.
}, (table) => ({ + orderNonNegative: check("parts_order_non_negative", sql`${table.order} >= 0`),
3-13: ImportuniqueIndexfor the new unique constraint.Required for the diff above.
- index, + index, + uniqueIndex,packages/db/src/mappers/chat/message.ts (1)
70-71: Dual-write risk (partsJSON + relational parts).Ensure a short-lived migration plan to drop JSON
partsto avoid drift.I can draft a phased migration plan and backfill script.
apps/web/client/src/server/api/routers/chat/message.ts (3)
78-83: Validate part records before insert (use shared schema).Pre-validate to catch bad payloads up-front and keep router logic aligned with DB constraints.
@@ -import { +import { conversations, fromDbMessageWithParts, messageInsertSchema, messages, messageUpdateSchema, parts, + partsInsertSchema, type Message } from '@onlook/db'; @@ - const dbParts = messageParts.map((part, index) => ({ - messageId: dbMessage.id, - type: part.type, - order: index, - ...getPartFields(part) - })); + const dbParts = messageParts.map((part, index) => + partsInsertSchema.parse({ + messageId: dbMessage.id, + type: part.type, + order: index, + ...getPartFields(part), + }) + );
145-197: Type the part mapper and guard unsupported tool types.The any-based mapper invites drift and silent data loss for unknown tools; add typing and a whitelist.
@@ -import { z } from 'zod'; +import { z } from 'zod'; +import type { ChatUIMessagePart } from '@onlook/models'; @@ -const getPartFields = (part: any) => { - const fields: any = { +const getPartFields = (part: ChatUIMessagePart): Partial<typeof parts.$inferInsert> => { + const fields: Partial<typeof parts.$inferInsert> = { providerMetadata: part.providerMetadata }; @@ - default: + default: // Handle tool calls if (part.type.startsWith("tool-")) { const toolName = part.type.replace("tool-", ""); + const allowedTools = new Set([ + "listFiles","readFile","bashRead","onlookInstructions","readStyleGuide", + "listBranches","scrapeUrl","webSearch","glob","grep", + "searchReplaceEditFile","searchReplaceMultiEditFile","fuzzyEditFile", + "writeFile","bashEdit","sandbox","terminalCommand","typecheck" + ]); + if (!allowedTools.has(toolName as any)) { + throw new Error(`Unsupported tool type: ${toolName}`); + } fields.tool_toolCallId = part.toolCallId; fields.tool_state = part.state; fields.tool_errorText = part.errorText; fields[`tool_${toolName}_input`] = part.input; fields[`tool_${toolName}_output`] = part.output; } // Handle data parts else if (part.type.startsWith("data-")) { const dataType = part.type.replace("data-", ""); if (dataType === "weather") { fields.data_weather_id = part.id; fields.data_weather_location = part.weather?.location; fields.data_weather_weather = part.weather?.weather; - fields.data_weather_temperature = part.weather?.temperature; + fields.data_weather_temperature = + Number.isFinite(part.weather?.temperature as any) + ? Math.round(Number(part.weather!.temperature)) + : null as any; } } break; } return fields; };
88-89: Consider returning the mapped ChatMessage with parts.Returning dbMessage forces a re-fetch to obtain parts; you already have messageParts in scope.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
apps/backend/supabase/migrations/0019_cuddly_iron_man.sql(1 hunks)apps/backend/supabase/migrations/meta/0019_snapshot.json(1 hunks)apps/backend/supabase/migrations/meta/_journal.json(1 hunks)apps/web/client/src/app/_components/shared/mockups/tailwind-color-editor.tsx(2 hunks)apps/web/client/src/app/api/chat/helpers/persistence.ts(1 hunks)apps/web/client/src/app/api/chat/route.ts(2 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/layers-tab/index.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/code-tab/file-operations.ts(1 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx(2 hunks)apps/web/client/src/app/projects/import/local/_components/select-folder.tsx(2 hunks)apps/web/client/src/app/terms-of-service/page.tsx(1 hunks)apps/web/client/src/components/store/editor/pages/helper.ts(3 hunks)apps/web/client/src/components/store/editor/sandbox/terminal.ts(1 hunks)apps/web/client/src/components/store/editor/theme/index.ts(2 hunks)apps/web/client/src/components/tools/tools.ts(1 hunks)apps/web/client/src/server/api/routers/chat/conversation.ts(2 hunks)apps/web/client/src/server/api/routers/chat/message.ts(4 hunks)apps/web/client/src/server/api/routers/domain/verify/helpers/freestyle.ts(1 hunks)apps/web/client/src/server/api/routers/github.ts(2 hunks)apps/web/client/src/utils/git/index.test.ts(2 hunks)apps/web/client/test/pages/helper.test.ts(5 hunks)apps/web/client/test/sandbox/file-sync.test.ts(1 hunks)apps/web/client/test/template-nodes/index.test.ts(5 hunks)apps/web/client/test/tools/glob.test.ts(4 hunks)apps/web/client/test/tools/grep.test.ts(8 hunks)apps/web/preload/script/api/elements/helpers.ts(1 hunks)apps/web/preload/script/api/state.ts(1 hunks)apps/web/preload/script/api/style/css-manager.ts(3 hunks)packages/ai/test/stream/convert.test.ts(2 hunks)packages/db/src/defaults/conversation.ts(1 hunks)packages/db/src/index.ts(1 hunks)packages/db/src/mappers/chat/conversation.ts(2 hunks)packages/db/src/mappers/chat/index.ts(1 hunks)packages/db/src/mappers/chat/message.ts(3 hunks)packages/db/src/mappers/chat/parts.ts(1 hunks)packages/db/src/schema/chat/conversation.ts(3 hunks)packages/db/src/schema/chat/index.ts(1 hunks)packages/db/src/schema/chat/message.ts(1 hunks)packages/db/src/schema/chat/parts.ts(1 hunks)packages/models/src/chat/conversation/index.ts(1 hunks)packages/models/src/chat/message/message.ts(1 hunks)subchat-implementation-example.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.tsx: In React providers managing MobX stores, create the observable store with useState(() => new Store()) to ensure a stable instance
Keep a ref (e.g., storeRef.current) to the MobX store to avoid stale closures in effects
Use setTimeout(() => store.clear(), 0) for delayed cleanup of MobX stores to avoid race conditions
Separate project changes from branch updates by using proper effect dependency arrays
Do not use useMemo to hold MobX observable instances; React may drop memoized values causing data loss
Do not clean up MobX stores synchronously during navigation; perform delayed cleanup instead
Do not include the MobX store instance in effect dependency arrays when it causes infinite loops
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/layers-tab/index.tsxapps/web/client/src/app/terms-of-service/page.tsxapps/web/client/src/app/projects/import/local/_components/select-folder.tsxapps/web/client/src/app/project/[id]/_components/left-panel/index.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/app/_components/shared/mockups/tailwind-color-editor.tsx
🧠 Learnings (3)
📚 Learning: 2025-09-07T23:36:29.687Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T23:36:29.687Z
Learning: Applies to **/*.tsx : Separate project changes from branch updates by using proper effect dependency arrays
Applied to files:
apps/web/client/src/app/project/[id]/_components/left-panel/layers-tab/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/index.tsx
📚 Learning: 2025-09-07T23:36:29.687Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T23:36:29.687Z
Learning: This is a Bun workspace; only use Bun commands and tooling, not npm
Applied to files:
apps/web/client/test/tools/grep.test.tsapps/web/client/test/tools/glob.test.ts
📚 Learning: 2025-09-07T23:36:29.687Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T23:36:29.687Z
Learning: Run unit tests with `bun test`
Applied to files:
apps/web/client/test/tools/glob.test.ts
🧬 Code graph analysis (20)
apps/web/client/src/app/api/chat/helpers/persistence.ts (4)
packages/models/src/chat/message/message.ts (3)
ChatUIMessage(32-32)ChatUIMessagePart(33-33)chatMetadataSchema(9-9)packages/db/src/client.ts (1)
db(16-16)packages/db/src/schema/chat/message.ts (1)
messages(12-28)packages/db/src/schema/chat/parts.ts (1)
parts(17-153)
apps/web/client/test/pages/helper.test.ts (1)
apps/web/client/src/components/store/editor/pages/helper.ts (1)
scanAppDirectory(201-310)
apps/web/client/test/sandbox/file-sync.test.ts (1)
apps/web/client/src/components/store/editor/sandbox/file-sync.ts (1)
FileSyncManager(6-101)
apps/web/client/src/app/api/chat/route.ts (1)
apps/web/client/src/app/api/chat/helpers/persistence.ts (1)
saveMessageWithParts(28-85)
apps/web/client/src/components/store/editor/pages/helper.ts (3)
apps/web/client/src/app/projects/import/github/layout.tsx (1)
metadata(7-10)apps/web/client/src/app/projects/import/local/layout.tsx (1)
metadata(7-10)packages/utility/src/file.ts (1)
getBaseName(80-83)
apps/web/client/src/app/project/[id]/_components/left-panel/layers-tab/index.tsx (1)
packages/models/src/element/layers.ts (1)
LayerNode(12-25)
packages/db/src/mappers/chat/parts.ts (2)
packages/models/src/chat/message/message.ts (2)
ChatUIMessagePart(33-33)ChatProviderMetadata(29-29)packages/db/src/schema/chat/parts.ts (1)
parts(17-153)
apps/web/client/src/utils/git/index.test.ts (1)
apps/web/client/src/utils/git/index.ts (2)
sanitizeCommitMessage(7-47)escapeShellString(53-66)
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (1)
packages/db/src/mappers/chat/message.ts (1)
toOnlookMessageFromVercel(81-109)
apps/web/client/src/components/store/editor/theme/index.ts (2)
apps/web/client/src/components/store/editor/theme/util.ts (3)
isValidTailwindConfigProperty(380-392)addTailwindRootColor(428-460)addTailwindNestedColor(8-68)packages/parser/src/helpers.ts (1)
isObjectExpression(31-33)
apps/web/client/test/template-nodes/index.test.ts (1)
packages/parser/src/template-node/map.ts (1)
createTemplateNodeMap(9-118)
packages/ai/test/stream/convert.test.ts (1)
packages/ai/src/stream/index.ts (1)
extractTextFromParts(53-62)
packages/db/src/schema/chat/parts.ts (1)
packages/db/src/schema/chat/message.ts (1)
messages(12-28)
apps/web/client/src/server/api/routers/chat/message.ts (3)
packages/db/src/schema/chat/parts.ts (1)
parts(17-153)packages/db/src/mappers/chat/message.ts (1)
fromDbMessageWithParts(44-48)packages/db/src/schema/chat/message.ts (2)
messageInsertSchema(30-30)messages(12-28)
apps/web/client/src/server/api/routers/chat/conversation.ts (4)
apps/web/client/src/server/api/trpc.ts (1)
protectedProcedure(130-149)packages/db/src/schema/chat/conversation.ts (1)
conversations(11-31)packages/db/src/mappers/chat/conversation.ts (1)
fromDbConversation(4-13)packages/db/src/schema/chat/message.ts (1)
messages(12-28)
packages/models/src/chat/message/message.ts (3)
packages/ai/src/tools/toolset.ts (1)
BUILD_TOOL_SET(54-64)packages/models/src/chat/message/context.ts (1)
MessageContext(53-59)packages/models/src/chat/message/checkpoint.ts (1)
MessageCheckpoints(15-15)
packages/db/src/mappers/chat/message.ts (2)
packages/models/src/chat/message/message.ts (7)
ChatMessageRole(36-40)ChatMessageRole(42-42)AssistantChatMessage(65-67)UserChatMessage(61-63)ChatMessage(71-71)ChatUIMessage(32-32)InternalChatMetadata(45-49)packages/db/src/mappers/chat/parts.ts (2)
fromDbParts(187-191)toDbParts(196-198)
apps/web/client/src/app/_components/shared/mockups/tailwind-color-editor.tsx (6)
packages/utility/src/color.ts (1)
Color(160-391)packages/ui/src/components/icons/index.tsx (1)
Icons(138-3594)apps/web/client/src/app/project/[id]/_components/editor-bar/toolbar-button.tsx (1)
ToolbarButton(13-67)apps/web/client/src/app/project/[id]/_components/editor-bar/separator.tsx (1)
InputSeparator(1-3)packages/ui/src/components/color-picker/ColorPicker.tsx (1)
ColorPicker(223-369)apps/web/client/src/app/_components/landing-page/color-swatch-group.tsx (1)
ColorSwatchGroup(8-28)
packages/db/src/schema/chat/conversation.ts (1)
packages/db/src/schema/chat/message.ts (1)
messages(12-28)
packages/db/src/schema/chat/message.ts (4)
packages/db/src/schema/chat/conversation.ts (1)
conversations(11-31)packages/models/src/chat/message/context.ts (1)
MessageContext(53-59)packages/models/src/chat/message/checkpoint.ts (1)
MessageCheckpoints(15-15)packages/db/src/schema/chat/parts.ts (1)
parts(17-153)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Supabase Preview
| "version": "7", | ||
| "dialect": "postgresql", | ||
| "entries": [ | ||
| { | ||
| "idx": 0, | ||
| "version": "7", | ||
| "when": 1747780205535, | ||
| "tag": "0000_same_human_robot", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 1, | ||
| "version": "7", | ||
| "when": 1747780749310, | ||
| "tag": "0001_graceful_exodus", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 2, | ||
| "version": "7", | ||
| "when": 1748028633785, | ||
| "tag": "0002_red_crusher_hogan", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 3, | ||
| "version": "7", | ||
| "when": 1748335865371, | ||
| "tag": "0003_loud_ozymandias", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 4, | ||
| "version": "7", | ||
| "when": 1748355015711, | ||
| "tag": "0004_pink_expediter", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 5, | ||
| "version": "7", | ||
| "when": 1748430282890, | ||
| "tag": "0005_short_lila_cheney", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 6, | ||
| "version": "7", | ||
| "when": 1748548078866, | ||
| "tag": "0006_rls", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 7, | ||
| "version": "7", | ||
| "when": 1748618860222, | ||
| "tag": "0007_realtime_rls", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 8, | ||
| "version": "7", | ||
| "when": 1748969281653, | ||
| "tag": "0008_preview-img-storage", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 9, | ||
| "version": "7", | ||
| "when": 1748969954773, | ||
| "tag": "0009_project_img_path", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 10, | ||
| "version": "7", | ||
| "when": 1749589432515, | ||
| "tag": "0010_bent_edwin_jarvis", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 11, | ||
| "version": "7", | ||
| "when": 1751069383054, | ||
| "tag": "0011_typical_clea", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 12, | ||
| "version": "7", | ||
| "when": 1751069449613, | ||
| "tag": "0012_file-transfer-bucket", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 13, | ||
| "version": "7", | ||
| "when": 1751914112866, | ||
| "tag": "0013_aspiring_kabuki", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 14, | ||
| "version": "7", | ||
| "when": 1752263777650, | ||
| "tag": "0014_military_marrow", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 15, | ||
| "version": "7", | ||
| "when": 1753208692842, | ||
| "tag": "0015_same_leo", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 16, | ||
| "version": "7", | ||
| "when": 1755044168112, | ||
| "tag": "0016_pretty_dust", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 17, | ||
| "version": "7", | ||
| "when": 1756677811694, | ||
| "tag": "0017_small_xavin", | ||
| "breakpoints": true | ||
| }, | ||
| { | ||
| "idx": 18, | ||
| "version": "7", | ||
| "when": 1757372913453, | ||
| "tag": "0018_lush_thanos", | ||
| "breakpoints": true | ||
| } | ||
| ] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Journal formatting-only churn; also ensure 0019 is recorded.
Prefer avoiding whitespace-only diffs in generated meta; and verify an entry for 0019_* exists to match the new migration.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Migrations with 0019 prefix:"
fd -a 'apps/backend/supabase/migrations/0019_*' -t f || true
echo "Journal has 0019 entry?"
jq -r '.entries[].tag' apps/backend/supabase/migrations/meta/_journal.json | rg '^0019_' || echo "No 0019 entry found in journal"Length of output: 83
Add missing 0019 migration and fix journal churn
- No migration file matching apps/backend/supabase/migrations/0019_* and no "0019_*" entry in apps/backend/supabase/migrations/meta/_journal.json (next idx → 19).
- Action: add the new migration file at apps/backend/supabase/migrations/0019_ and append a journal entry in apps/backend/supabase/migrations/meta/journal.json: { "idx": 19, "version": "7", "when": , "tag": "0019", "breakpoints": true }. Commit the migration file and journal update together.
- If 0019 is not intended, revert the whitespace-only changes to _journal.json to avoid churn.
🤖 Prompt for AI Agents
In apps/backend/supabase/migrations/meta/_journal.json (lines ~2-138) the
journal is missing an entry for migration 0019 and your change appears to be
whitespace-only churn; either add a new migration file at
apps/backend/supabase/migrations/0019_<name> and append to this _journal.json an
object { "idx": 19, "version": "7", "when": <current-epoch-ms>, "tag":
"0019_<name>", "breakpoints": true } (use the real name and real epoch ms) and
commit the new migration file and updated _journal.json together, or if 0019 is
not intended revert the whitespace-only edits to _journal.json to avoid churn.
| "content": { | ||
| "name": "content", | ||
| "type": "text", | ||
| "primaryKey": false, | ||
| "notNull": true | ||
| }, |
There was a problem hiding this comment.
messages.content nullability mismatch
Code makes content optional, but snapshot enforces NOT NULL. This will fail when persisting messages that use structured parts only.
Proposed migration:
ALTER TABLE messages ALTER COLUMN content DROP NOT NULL;Then regenerate the snapshot.
🤖 Prompt for AI Agents
In apps/backend/supabase/migrations/meta/0019_snapshot.json around lines 310 to
315, the snapshot marks messages.content as NOT NULL while the code treats
content as optional; apply a migration to alter the messages table to drop the
NOT NULL constraint on the content column (ALTER TABLE messages ALTER COLUMN
content DROP NOT NULL), run the migration against the database, then regenerate
and commit the updated snapshot so the schema JSON matches the code's nullable
content field.
| "public.message_role": { | ||
| "name": "message_role", | ||
| "schema": "public", | ||
| "values": [ | ||
| "user", | ||
| "assistant" | ||
| ] | ||
| }, |
There was a problem hiding this comment.
Enum mismatch: code expects 'system' role, snapshot does not
The code defines message_role with ['user','assistant','system'], but the snapshot only has ['user','assistant']. This will break inserts/reads for 'system' messages.
Proposed migration:
-- Add 'system' to enum (safe if already added)
ALTER TYPE message_role ADD VALUE IF NOT EXISTS 'system';Regenerate this snapshot after applying the migration.
🤖 Prompt for AI Agents
In apps/backend/supabase/migrations/meta/0019_snapshot.json around lines
2370-2377 the snapshot for the enum "public.message_role" only lists
["user","assistant"] while application code expects
["user","assistant","system"]; add a migration that runs ALTER TYPE message_role
ADD VALUE IF NOT EXISTS 'system'; then apply the migration and regenerate the
snapshot so the JSON reflects the updated enum values.
| @@ -0,0 +1,158 @@ | |||
| import { messages, parts } from '@onlook/db'; | |||
| import { db } from '@onlook/db/src/client'; | |||
| import type { ChatUIMessage, ChatUIMessagePart, chatMetadataSchema } from '@onlook/models'; | |||
There was a problem hiding this comment.
Fix: chatMetadataSchema imported as a type but used as a value.
This will fail: typeof chatMetadataSchema.parse needs a value import.
-import type { ChatUIMessage, ChatUIMessagePart, chatMetadataSchema } from '@onlook/models';
+import { chatMetadataSchema, type ChatUIMessage, type ChatUIMessagePart } from '@onlook/models';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { ChatUIMessage, ChatUIMessagePart, chatMetadataSchema } from '@onlook/models'; | |
| import { chatMetadataSchema, type ChatUIMessage, type ChatUIMessagePart } from '@onlook/models'; |
🤖 Prompt for AI Agents
In apps/web/client/src/app/api/chat/helpers/persistence.ts around line 3,
chatMetadataSchema was imported only as a type but is used as a runtime value
(e.g., typeof chatMetadataSchema.parse); change the import to bring
chatMetadataSchema in as a value (remove the 'type' qualifier for that symbol)
so the runtime methods exist, e.g. import the schema alongside the types from
'@onlook/models', and update any references if needed to use the value import.
| function convertToTypedUIMessage(message: UIMessage): ChatUIMessage { | ||
| // Type-safe conversion of generic UIMessage to our specific ChatUIMessage | ||
| const convertedParts: ChatUIMessagePart[] = message.parts.map(part => { | ||
| // The parts are compatible, we just need to assert the correct type | ||
| return part as ChatUIMessagePart; | ||
| }); | ||
|
|
||
| return { | ||
| id: message.id, | ||
| role: message.role as 'user' | 'assistant', | ||
| parts: convertedParts, | ||
| metadata: message.metadata as ReturnType<typeof chatMetadataSchema.parse>, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden conversion: validate role/metadata and handle missing parts.
Guard against system role, parse metadata, default parts to [].
-function convertToTypedUIMessage(message: UIMessage): ChatUIMessage {
- // Type-safe conversion of generic UIMessage to our specific ChatUIMessage
- const convertedParts: ChatUIMessagePart[] = message.parts.map(part => {
- // The parts are compatible, we just need to assert the correct type
- return part as ChatUIMessagePart;
- });
-
- return {
- id: message.id,
- role: message.role as 'user' | 'assistant',
- parts: convertedParts,
- metadata: message.metadata as ReturnType<typeof chatMetadataSchema.parse>,
- };
-}
+function convertToTypedUIMessage(message: UIMessage): ChatUIMessage {
+ if (message.role !== 'user' && message.role !== 'assistant') {
+ throw new Error(`Unsupported role: ${message.role}`);
+ }
+ const convertedParts: ChatUIMessagePart[] = Array.isArray((message as any).parts)
+ ? (message.parts as ChatUIMessagePart[])
+ : [];
+ const metadata = chatMetadataSchema.parse(message.metadata ?? {});
+ return { id: message.id, role: message.role, parts: convertedParts, metadata };
+}📝 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.
| function convertToTypedUIMessage(message: UIMessage): ChatUIMessage { | |
| // Type-safe conversion of generic UIMessage to our specific ChatUIMessage | |
| const convertedParts: ChatUIMessagePart[] = message.parts.map(part => { | |
| // The parts are compatible, we just need to assert the correct type | |
| return part as ChatUIMessagePart; | |
| }); | |
| return { | |
| id: message.id, | |
| role: message.role as 'user' | 'assistant', | |
| parts: convertedParts, | |
| metadata: message.metadata as ReturnType<typeof chatMetadataSchema.parse>, | |
| }; | |
| } | |
| function convertToTypedUIMessage(message: UIMessage): ChatUIMessage { | |
| if (message.role !== 'user' && message.role !== 'assistant') { | |
| throw new Error(`Unsupported role: ${message.role}`); | |
| } | |
| const convertedParts: ChatUIMessagePart[] = Array.isArray((message as any).parts) | |
| ? (message.parts as ChatUIMessagePart[]) | |
| : []; | |
| const metadata = chatMetadataSchema.parse(message.metadata ?? {}); | |
| return { id: message.id, role: message.role, parts: convertedParts, metadata }; | |
| } |
| @@ -1,40 +1,51 @@ | |||
| import { ChatMessageRole, type MessageCheckpoints, type MessageContext } from "@onlook/models"; | |||
| import type { UIMessage as AiMessage } from "ai"; | |||
| import type { UIMessage } from "ai"; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
DB package shouldn’t depend on UI SDK types
import type { UIMessage } from "ai" creates a cross-layer dependency from DB schema to a UI library. Define a local structural type for the legacy JSONB column instead to decouple packages.
Run to verify ai is (not) a dependency of the db package:
🏁 Script executed:
#!/bin/bash
jq -r '.name, (.dependencies.ai // .devDependencies.ai // "ai: NOT FOUND")' packages/db/package.json
rg -n 'from "ai"' packages/db -C1Length of output: 510
Remove ai imports from DB package; replace with local structural types.
packages/db imports UI SDK types from "ai" in packages/db/src/schema/chat/message.ts:2 and packages/db/src/schema/chat/parts.ts:1, but packages/db/package.json does not declare "ai" — this creates a cross-layer dependency and a build/type-resolution risk. Replace those imports with local structural types that model the legacy JSONB shape (or move shared types into @onlook/models) and remove the ai dependency from the DB package.
🤖 Prompt for AI Agents
In packages/db/src/schema/chat/message.ts around line 2 (and similarly in
packages/db/src/schema/chat/parts.ts line 1), the code imports UI SDK types from
"ai" which creates a cross-layer dependency; remove those imports and replace
them with a local structural type that matches the legacy JSONB shape used in
the DB (or alternatively move a shared type into @onlook/models and import it
from there). Change references to UIMessage to the new local/interface type,
update any type shape differences to match stored JSON (fields, optionality,
nested objects), and ensure packages/db/package.json does not reference "ai".
| export const messageRole = pgEnum("message_role", ['user', 'assistant', 'system']); | ||
|
|
There was a problem hiding this comment.
DB enum alignment required for 'system' messages
pgEnum("message_role", ['user','assistant','system']) requires the DB enum to include 'system'. Ensure the migration adds it (see snapshot comment).
🤖 Prompt for AI Agents
In packages/db/src/schema/chat/message.ts around lines 10-11 the pgEnum
definition includes 'system' but the DB migration does not; update the relevant
migration that creates or alters the message_role enum to include the 'system'
value (or create an ALTER TYPE ... ADD VALUE IF NOT EXISTS 'system' statement)
and regenerate/apply migrations so the database enum matches the code and
tests/snapshots.
| parts: jsonb("parts").$type<UIMessage['parts']>().default([]).notNull(), | ||
| content: text("content"), | ||
| }).enableRLS(); |
There was a problem hiding this comment.
Schema vs DB mismatch: content nullability + dual 'parts' storage
- DB snapshot has
contentNOT NULL; code makes it optional. Align via migration (drop NOT NULL) or keep.notNull()here. - You now have both JSONB
partsand relationalparts. If JSONB is kept for back-compat, annotate it@deprecatedin comments and ensure writers don’t double-write.
If you prefer to keep JSONB for reads-only:
- parts: jsonb("parts").$type<UIMessage['parts']>().default([]).notNull(),
+ // @deprecated: kept temporarily for backward compatibility; new writes go to parts table
+ parts: jsonb("parts").$type<UIMessage['parts']>().default([]).notNull(),And add a follow-up migration to drop this column once consumers migrate.
📝 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.
| parts: jsonb("parts").$type<UIMessage['parts']>().default([]).notNull(), | |
| content: text("content"), | |
| }).enableRLS(); | |
| // @deprecated: kept temporarily for backward compatibility; new writes go to parts table | |
| parts: jsonb("parts").$type<UIMessage['parts']>().default([]).notNull(), | |
| content: text("content"), | |
| }).enableRLS(); |
🤖 Prompt for AI Agents
In packages/db/src/schema/chat/message.ts around lines 26-28, the schema
currently makes content nullable while the DB snapshot marks content NOT NULL,
and you now store parts both as a JSONB column and as relational parts; to fix,
decide which behavior you want: either add .notNull() to content to match the DB
(or create a migration that drops NOT NULL in the DB if you want nullable), and
for parts either remove the JSONB column or mark it deprecated in the code
comments and ensure writers only write the new relational parts (or add a
migration to backfill and then drop the JSONB column later); also add a short
comment above the JSONB parts field stating it's deprecated/read-only and update
write paths to avoid double-writing.
| sourceUrlFieldsRequired: check( | ||
| "source_url_fields_required_if_type_is_source_url", | ||
| sql`CASE WHEN ${table.type} = 'source_url' THEN ${table.source_url_sourceId} IS NOT NULL AND ${table.source_url_url} IS NOT NULL ELSE TRUE END` | ||
| ), | ||
|
|
||
| sourceDocumentFieldsRequired: check( | ||
| "source_document_fields_required_if_type_is_source_document", | ||
| sql`CASE WHEN ${table.type} = 'source_document' THEN ${table.source_document_sourceId} IS NOT NULL AND ${table.source_document_mediaType} IS NOT NULL AND ${table.source_document_title} IS NOT NULL ELSE TRUE END` | ||
| ), |
There was a problem hiding this comment.
Fix type literals in checks: hyphen vs underscore.
type uses hyphenated values (source-url, source-document). Checks won’t fire as written.
- sourceUrlFieldsRequired: check(
- "source_url_fields_required_if_type_is_source_url",
- sql`CASE WHEN ${table.type} = 'source_url' THEN ${table.source_url_sourceId} IS NOT NULL AND ${table.source_url_url} IS NOT NULL ELSE TRUE END`
- ),
+ sourceUrlFieldsRequired: check(
+ "source_url_fields_required_if_type_is_source-url",
+ sql`CASE WHEN ${table.type} = 'source-url' THEN ${table.source_url_sourceId} IS NOT NULL AND ${table.source_url_url} IS NOT NULL ELSE TRUE END`
+ ),
...
- sourceDocumentFieldsRequired: check(
- "source_document_fields_required_if_type_is_source_document",
- sql`CASE WHEN ${table.type} = 'source_document' THEN ${table.source_document_sourceId} IS NOT NULL AND ${table.source_document_mediaType} IS NOT NULL AND ${table.source_document_title} IS NOT NULL ELSE TRUE END`
- ),
+ sourceDocumentFieldsRequired: check(
+ "source_document_fields_required_if_type_is_source-document",
+ sql`CASE WHEN ${table.type} = 'source-document' THEN ${table.source_document_sourceId} IS NOT NULL AND ${table.source_document_mediaType} IS NOT NULL AND ${table.source_document_title} IS NOT NULL ELSE TRUE END`
+ ),📝 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.
| sourceUrlFieldsRequired: check( | |
| "source_url_fields_required_if_type_is_source_url", | |
| sql`CASE WHEN ${table.type} = 'source_url' THEN ${table.source_url_sourceId} IS NOT NULL AND ${table.source_url_url} IS NOT NULL ELSE TRUE END` | |
| ), | |
| sourceDocumentFieldsRequired: check( | |
| "source_document_fields_required_if_type_is_source_document", | |
| sql`CASE WHEN ${table.type} = 'source_document' THEN ${table.source_document_sourceId} IS NOT NULL AND ${table.source_document_mediaType} IS NOT NULL AND ${table.source_document_title} IS NOT NULL ELSE TRUE END` | |
| ), | |
| sourceUrlFieldsRequired: check( | |
| "source_url_fields_required_if_type_is_source-url", | |
| sql`CASE WHEN ${table.type} = 'source-url' THEN ${table.source_url_sourceId} IS NOT NULL AND ${table.source_url_url} IS NOT NULL ELSE TRUE END` | |
| ), | |
| sourceDocumentFieldsRequired: check( | |
| "source_document_fields_required_if_type_is_source-document", | |
| sql`CASE WHEN ${table.type} = 'source-document' THEN ${table.source_document_sourceId} IS NOT NULL AND ${table.source_document_mediaType} IS NOT NULL AND ${table.source_document_title} IS NOT NULL ELSE TRUE END` | |
| ), |
🤖 Prompt for AI Agents
In packages/db/src/schema/chat/parts.ts around lines 138 to 146, the CHECK
conditions compare table.type to underscored literals ('source_url' and
'source_document') but the enum uses hyphenated values ('source-url' and
'source-document'); update the CASE WHEN comparisons to use the hyphenated
string literals so the checks actually trigger for those types (i.e., change the
compared literals to 'source-url' and 'source-document') and keep the rest of
the boolean conditions unchanged.
| // Chat message role constants | ||
| export const ChatMessageRole = { | ||
| USER: 'user' as const, | ||
| ASSISTANT: 'assistant' as const, | ||
| SYSTEM: 'system' as const, | ||
| } as const; | ||
|
|
||
| export type ChatMessageRole = (typeof ChatMessageRole)[keyof typeof ChatMessageRole]; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Role consistency: add SystemChatMessage or drop SYSTEM
You export a SYSTEM role but exclude it from ChatMessage. Either add a SystemChatMessage to the union or remove SYSTEM to avoid dead/unchecked cases.
export const ChatMessageRole = {
USER: 'user' as const,
ASSISTANT: 'assistant' as const,
SYSTEM: 'system' as const,
} as const;
export type ChatMessageRole = (typeof ChatMessageRole)[keyof typeof ChatMessageRole];
+export interface SystemChatMessage extends BaseChatMessage {
+ role: typeof ChatMessageRole.SYSTEM;
+}
- export type ChatMessage = UserChatMessage | AssistantChatMessage;
+ export type ChatMessage = UserChatMessage | AssistantChatMessage | SystemChatMessage;To confirm usage, you can scan for SYSTEM messages and union handling:
Also applies to: 61-67
🏁 Script executed:
#!/bin/bash
rg -nC2 'ChatMessageRole\.SYSTEM'
rg -nP '\bSystemChatMessage\b'Length of output: 498
Fix role inconsistency: add SystemChatMessage or remove SYSTEM
models export ChatMessageRole.SYSTEM (packages/models/src/chat/message/message.ts:35–43) but the ChatMessage union omits SystemChatMessage; the DB mapper explicitly throws on SYSTEM (packages/db/src/mappers/chat/message.ts:33–35). Choose one:
- Support SYSTEM: add a SystemChatMessage interface (role: typeof ChatMessageRole.SYSTEM), include it in the ChatMessage union, and update mappers to handle it (remove the throw).
- Or remove SYSTEM: delete SYSTEM from ChatMessageRole and update all references (e.g., packages/db/src/mappers/chat/message.ts) so the role can't occur.
🤖 Prompt for AI Agents
In packages/models/src/chat/message/message.ts around lines 35–43,
ChatMessageRole currently includes SYSTEM but the ChatMessage union omits a
SystemChatMessage which causes the DB mapper
(packages/db/src/mappers/chat/message.ts lines ~33–35) to throw on SYSTEM; fix
by adding a SystemChatMessage interface (role: typeof ChatMessageRole.SYSTEM and
any system-specific fields), include it in the ChatMessage union type so SYSTEM
is a valid message type, and then update the DB mapper to handle
SystemChatMessage instead of throwing (remove the explicit throw and map the
SYSTEM case consistent with other roles).
| }).enableRLS(); | ||
|
|
||
| // Subchat support: points to a parent conversation if this is a subchat | ||
| parentConversationId: uuid("parent_conversation_id"), |
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Introduces subchat feature with hierarchical conversation support, including database schema changes, new API endpoints, and client-side updates.
parent_conversation_idandparent_message_idtoconversationstable for subchat support (0019_cuddly_iron_man.sql).conversation.tsfor creating, querying, and building conversation trees.ChatConversationmodel to include subchat fields.parent_conversation_idandparent_message_idinconversationstable for efficient querying.partstable to store message parts separately, with constraints for data integrity (parts.ts).saveMessageWithParts()inpersistence.tsto handle message parts.use-chat.tsxandroute.tsto handle subchat logic and message saving.select-folder.tsxto improve file handling logic.subchat-implementation-example.mdto guide on subchat usage and benefits.terms-of-service/page.tsx.This description was created by
for 23577d2. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit