feat: incremental saving chat #2833
Conversation
|
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. WalkthroughReworks chat domain to use a metadata-driven ChatMessage and ChatConversation.displayName, updates DB schema/mappers, migrates streaming to ChatMessage-first flow (upsert before/after stream, load history), refactors hooks/UI to expose streamingMessage and sendMessageToChat(message, type), renames AI tool exports, and replaces Markdown renderer. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Web UI
participant Store as Editor Store
participant Hook as ChatProvider/useChat
participant API as /api/chat (route)
participant DB as Database
User->>UI: Submit text
UI->>Store: addAsk/addEdit/addFix... -> returns ChatMessage
Store-->>UI: ChatMessage (with metadata)
UI->>Hook: sendMessageToChat(message, chatType)
Hook->>API: POST { message, chatType, conversationId, projectId, traceId }
API->>DB: upsertMessage(id, conversationId, message)
API->>DB: loadChat(conversationId)
API->>API: stream using history -> chunks
API-->>Hook: stream chunks (assistant partial)
Hook-->>UI: streamingMessage updates
API->>DB: upsertMessage(final assistant message)
Hook->>Store: add assistant message to conversation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| parts: jsonb("parts").$type<AiMessage['parts']>().default([]), | ||
| content: text("content"), |
There was a problem hiding this comment.
Data integrity bug: The 'content' field has been moved after 'parts' and is no longer marked as .notNull(), but the original schema had content as required. This breaking change will cause runtime errors when existing code expects content to always be present. Either keep content as .notNull() or update all consuming code to handle null content values.
| parts: jsonb("parts").$type<AiMessage['parts']>().default([]), | |
| content: text("content"), | |
| parts: jsonb("parts").$type<AiMessage['parts']>().default([]), | |
| content: text("content").notNull(), |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| export const conversationInsertSchema = createInsertSchema(conversations); | ||
| export const conversationUpdateSchema = createUpdateSchema(conversations, { | ||
| id: z.string().uuid(), | ||
| id: z.uuid(), |
There was a problem hiding this comment.
Schema validation bug: Changed z.string().uuid() to z.uuid() in conversationUpdateSchema. The z.uuid() method doesn't exist in standard Zod - it should be z.string().uuid(). This will cause runtime errors when the schema is used for validation.
| id: z.uuid(), | |
| id: z.string().uuid(), |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/history.tsx (1)
1-1: Missing"use client"directive.
This component uses React state and event handlers underapp/; mark it as a Client Component.+'use client'; + import { useEditorEngine } from '@/components/store/editor';
🧹 Nitpick comments (5)
packages/db/src/schema/chat/message.ts (2)
24-24: Avoidanytype for snapshots.
Switch tounknownor a specific shape to meet the repo guideline of avoidingany.- snapshots: jsonb("snapshots").$type<any>(), + snapshots: jsonb("snapshots").$type<Record<string, unknown> | unknown[]>(),
8-8: Nit: constant name casing.
CONVERSATION_MESSAGe_RELATION_NAMEhas a stray lowercase “e”. Consider renaming toCONVERSATION_MESSAGE_RELATION_NAMEfor consistency, and update imports.-export const CONVERSATION_MESSAGe_RELATION_NAME = 'conversation_messages'; +export const CONVERSATION_MESSAGE_RELATION_NAME = 'conversation_messages';And in files importing it, update the symbol name.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/history.tsx (1)
91-91: Fallback should handle empty strings too.
Nullish coalescing won’t catch''. Use logical OR so blank titles render the fallback.- {conversation.displayName ?? 'New Conversation'} + {conversation.displayName || 'New Conversation'}packages/db/src/schema/chat/conversation.ts (2)
21-25: Add indexes for new FK columns.
Subchat lookups and cascades will benefit from indexes.-import { jsonb, pgTable, timestamp, uuid, varchar, type AnyPgColumn } from "drizzle-orm/pg-core"; +import { jsonb, pgTable, timestamp, uuid, varchar, index, type AnyPgColumn } from "drizzle-orm/pg-core"; @@ -export const conversations = pgTable("conversations", { +export const conversations = pgTable("conversations", { /* columns */ -}).enableRLS(); +}, (t) => ({ + parentConversationIdx: index("conversations_parent_conversation_id_idx").on(t.parentConversationId), + parentMessageIdx: index("conversations_parent_message_id_idx").on(t.parentMessageId), +})).enableRLS();
7-8: Nit: constant name casing (imported).
If you rename the constant toCONVERSATION_MESSAGE_RELATION_NAMEin messages, update imports and usages here too.-import { CONVERSATION_MESSAGe_RELATION_NAME, messages } from "./message"; +import { CONVERSATION_MESSAGE_RELATION_NAME, messages } from "./message"; @@ - messages: many(messages, { - relationName: CONVERSATION_MESSAGe_RELATION_NAME, + messages: many(messages, { + relationName: CONVERSATION_MESSAGE_RELATION_NAME,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/history.tsx(1 hunks)apps/web/client/src/components/store/editor/chat/conversation.ts(3 hunks)packages/db/src/defaults/conversation.ts(1 hunks)packages/db/src/mappers/chat/conversation.ts(1 hunks)packages/db/src/schema/chat/conversation.ts(2 hunks)packages/db/src/schema/chat/message.ts(2 hunks)packages/models/src/chat/conversation/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the 'any' type unless necessary
Files:
packages/db/src/mappers/chat/conversation.tsapps/web/client/src/components/store/editor/chat/conversation.tspackages/db/src/defaults/conversation.tspackages/models/src/chat/conversation/index.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/history.tsxpackages/db/src/schema/chat/message.tspackages/db/src/schema/chat/conversation.ts
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer imports using @/* or ~/* aliases instead of deep relative paths within the client app
Files:
apps/web/client/src/components/store/editor/chat/conversation.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/history.tsx
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libraries
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/history.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use process.env in client code; prefer importing env from @/env
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/history.tsx
🧬 Code graph analysis (4)
packages/db/src/mappers/chat/conversation.ts (1)
packages/models/src/chat/conversation/index.ts (1)
ChatConversation(3-12)
packages/models/src/chat/conversation/index.ts (1)
packages/models/src/chat/suggestion.ts (1)
ChatSuggestion(9-12)
packages/db/src/schema/chat/message.ts (3)
packages/db/src/schema/chat/conversation.ts (1)
conversations(11-26)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/conversation.ts (1)
packages/db/src/schema/chat/message.ts (1)
messages(11-27)
🔇 Additional comments (12)
packages/db/src/schema/chat/message.ts (3)
4-4: Typed FK import looks correct.
This fixes Drizzle typing for cross-table references.
15-15: Good use of typed references to break TS cyclic typing.
This is the recommended pattern with Drizzle when cross-referencing tables.
30-30: ExportingmessageUpdateSchemais helpful.
Unblocks partial updates without hand-writing Zod.packages/db/src/defaults/conversation.ts (1)
12-13: Defaults for subchat IDs look good.
Null-initialized optional parents align with schema and mappers.packages/db/src/mappers/chat/conversation.ts (2)
5-5: Pass-throughfromDbConversationassumes shapes are identical. Verify.
If DB types ever diverge (e.g., optional vs. required), this will leak DB-only fields or nullability mismatches.Optionally make mapping explicit to lock the contract.
-export const fromDbConversation = (conversation: DbConversation): ChatConversation => { - return conversation; -} +export const fromDbConversation = (c: DbConversation): ChatConversation => ({ + id: c.id, + displayName: c.displayName ?? null, + projectId: c.projectId, + createdAt: c.createdAt, + updatedAt: c.updatedAt, + suggestions: c.suggestions ?? null, + parentConversationId: c.parentConversationId ?? null, + parentMessageId: c.parentMessageId ?? null, +});
12-15: Mapping to DB with null coalescing looks right.
Ensures we don’t store empty strings/undefined.apps/web/client/src/components/store/editor/chat/conversation.ts (2)
53-53: Guard depends ondisplayNamesemantics.
With default displayName currently set (see defaults), this won’t block creating redundant empty conversations. Prefer defaultnullin defaults, or adapt the guard accordingly.- if (this.current?.messages.length === 0 && !this.current?.conversation.displayName) { + if (this.current?.messages.length === 0 && this.current?.conversation.displayName == null) {
146-146: Persist renamed title to storage (unless mutate already does).
IfgenerateTitledoesn’t update the DB, call the update API to keep list and DB in sync.listConversation.displayName = title; + await this.updateConversationInStorage({ id: conversationId, displayName: title });packages/models/src/chat/conversation/index.ts (1)
5-11: API shape LGTM.
displayName and top-level suggestions align with UI and DB; optional parent IDs support subchats.packages/db/src/schema/chat/conversation.ts (3)
3-3: Import ofAnyPgColumnis appropriate.
Required for typed self/cross references.
30-31: z.uuid() on update schema is a nice tightening.
Removes reliance on string parsing.
21-25: Cyclic table refs use callbacks — verify bundler/transpiler preserves lazy evaluation.conversations and messages are both exported (packages/db/src/schema/chat/conversation.ts and packages/db/src/schema/chat/message.ts) and .references(...) uses callback form; confirm your build (ESM→CJS transpile, tree-shaking/module inlining) does not evaluate or remove those callbacks before module initialization.
| if (!this.current.conversation.displayName) { | ||
| this.addConversationTitle(this.current.conversation.id, content); | ||
| } |
There was a problem hiding this comment.
Title generation won’t run if default displayName is a non-null placeholder.
Switch defaults to null (preferred), or treat the placeholder as empty.
Preferred (after changing defaults):
- if (!this.current.conversation.displayName) {
+ if (this.current.conversation.displayName == null) {Fallback (if keeping placeholder string):
- if (!this.current.conversation.displayName) {
+ if (
+ !this.current.conversation.displayName ||
+ this.current.conversation.displayName === 'New Conversation'
+ ) {📝 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 (!this.current.conversation.displayName) { | |
| this.addConversationTitle(this.current.conversation.id, content); | |
| } | |
| if (this.current.conversation.displayName == null) { | |
| this.addConversationTitle(this.current.conversation.id, content); | |
| } |
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/editor/chat/conversation.ts around lines
125-127, the title-generation guard checks only for falsy displayName but a
non-null placeholder string prevents generation; update the default for
conversation.displayName to null (preferred) so the existing check works, or
alter this block to treat the placeholder as empty by changing the condition to
detect the placeholder string (e.g., compare against the placeholder constant or
trim and check length) and then call addConversationTitle when appropriate.
| projectId, | ||
| createdAt: new Date(), | ||
| updatedAt: new Date(), | ||
| displayName: 'New Conversation', |
There was a problem hiding this comment.
Defaulting displayName breaks title generation flow.
UI/store generate a title only when displayName is falsy; setting a literal 'New Conversation' prevents generation.
- displayName: 'New Conversation',
+ displayName: null,UI can still show 'New Conversation' via fallback.
📝 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.
| displayName: 'New Conversation', | |
| displayName: null, |
🤖 Prompt for AI Agents
In packages/db/src/defaults/conversation.ts around line 10, the default sets
displayName to the literal 'New Conversation', which prevents the UI/store from
generating a title (they only generate when displayName is falsy); change the
default so displayName is falsy (e.g., remove the property or set it to
undefined or an empty string) so the title generation flow runs, and let the UI
handle showing 'New Conversation' as a fallback.
| parts: jsonb("parts").$type<AiMessage['parts']>().default([]), | ||
| content: text("content"), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Nullability change can produce empty/invalid messages; harden constraints.
- parts is now nullable; callers may not expect null. Keep default([]) but add notNull() to avoid NULLs.
- With content now nullable, ensure at least one of content or parts is present in every row.
Apply:
- parts: jsonb("parts").$type<AiMessage['parts']>().default([]),
+ parts: jsonb("parts").$type<AiMessage['parts']>().default([]).notNull(),Optionally add a CHECK so rows always have content or parts (requires table builder block):
// import { sql } from "drizzle-orm";
export const messages = pgTable("messages", { /* cols */ }, (t) => ({
contentOrParts: /* check name */ {
// Pseudocode: enforce content IS NOT NULL OR jsonb_array_length(parts) > 0
// Implement using your preferred check helper and sql template literal.
}
}));Run a quick audit to find consumers assuming non-null content:
🏁 Script executed:
#!/bin/bash
rg -n --type=ts --type=tsx -C2 '\bmessage\.content\b|\bcontent:\s*message\.content\b'Length of output: 27
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Show schema file (provided path in review): packages/db/src/schema/chat/message.ts"
file="packages/db/src/schema/chat/message.ts"
if [ -f "$file" ]; then
echo "=== $file (lines 1-240) ==="
sed -n '1,240p' "$file"
else
echo "File not found at $file"
fi
echo
echo "2) Grep for jsonb('parts') occurrences across the repo"
rg -n --hidden -S 'jsonb\("parts"\)' || true
echo
echo "3) Search for direct consumers referencing message.content or message.parts (repo-wide)"
rg -n --hidden -S '\bmessage\.content\b' || true
rg -n --hidden -S '\bmessage\.parts\b' || true
echo
echo "4) Broader search for `.content` / `.parts` on variables named `message` or destructured patterns (limited to likely code dirs)"
rg -n --hidden -S '\bcontent:\s*message\.content\b' || true
rg -n --hidden -S '\bmessage\s*\.\s*content\b' || true
rg -n --hidden -S '\bmessage\s*\.\s*parts\b' || trueLength of output: 5694
Make messages.parts NOT NULL and enforce content-or-parts invariant
- parts is nullable but multiple callers use message.parts without null checks (will throw at runtime). Make parts NOT NULL and add a DB CHECK requiring content IS NOT NULL OR jsonb_array_length(parts) > 0.
- Apply:
- parts: jsonb("parts").$type<AiMessage['parts']>().default([]),
+ parts: jsonb("parts").$type<AiMessage['parts']>().default([]).notNull(),- Add DB CHECK (Postgres): CHECK ((content IS NOT NULL) OR (jsonb_array_length(parts) > 0))
- Update callers/types (examples found): packages/db/src/mappers/chat/message.ts, packages/ai/src/stream/index.ts, packages/ai/src/prompt/provider.ts, packages/ai/src/apply/client.ts, apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx
📝 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<AiMessage['parts']>().default([]), | |
| content: text("content"), | |
| parts: jsonb("parts").$type<AiMessage['parts']>().default([]).notNull(), | |
| content: text("content"), |
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (1)
209-213: Use stable React keys; avoid random keys.Using
nanoid()re-mounts pills every render, losing local state and hurting perf.- {message.metadata?.context?.map((context) => ( - <SentContextPill key={nanoid()} context={context} /> - ))} + {message.metadata?.context?.map((context, idx) => ( + <SentContextPill key={idx} context={context} /> + ))}If a stable identifier exists on
context, prefer it over the index.
♻️ Duplicate comments (2)
packages/db/src/schema/chat/message.ts (1)
20-20: Make parts NOT NULL and keep default [].Multiple callers assume message.parts is always an array; allowing NULL increases runtime risk.
- parts: jsonb("parts").$type<ChatMessage['parts']>().default([]), + parts: jsonb("parts").$type<ChatMessage['parts']>().default([]).notNull(),apps/web/client/src/components/store/editor/chat/conversation.ts (1)
125-127: Title generation guard: handle placeholder defaults.If displayName defaults to a non-empty placeholder (e.g., "New Conversation"), this condition won’t trigger.
- if (!this.current.conversation.displayName) { + if ( + this.current.conversation.displayName == null || + this.current.conversation.displayName === 'New Conversation' + ) {Prefer making the default displayName null so a simple null-check suffices.
🧹 Nitpick comments (32)
apps/web/client/src/server/api/routers/chat/suggestion.ts (2)
43-43: Prefer explicit typing oversatisfieshere.
satisfieswon’t setsuggestions’s variable type. Make it explicit for clearer intent and IDE help.- const suggestions = object.suggestions satisfies ChatSuggestion[]; + const suggestions: ChatSuggestion[] = object.suggestions;
2-2: Avoid deep package internals import.Import the prompt from the package’s public entry (re-export it if needed) to prevent brittle coupling to internal paths.
-import { SUGGESTION_SYSTEM_PROMPT } from '@onlook/ai/src/prompt/suggest'; +import { SUGGESTION_SYSTEM_PROMPT } from '@onlook/ai';apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsx (2)
9-12: DropuseMemohere; simple comparison is cheaper and clearer.- const isAssistantStreamMessage = useMemo(() => - streamMessage?.role === 'assistant', - [streamMessage?.role] - ); + const isAssistantStreamMessage = streamMessage?.role === 'assistant';
4-4: Consider alias imports for consistency.Prefer '@/...' to avoid relative paths in client code.
-import { MessageContent } from './message-content'; +import { MessageContent } from '@/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content';packages/ai/test/tools/web.test.ts (1)
3-4: Rename test description to match new export names.The body uses buildTools/askTools; update the spec name for clarity.
- it('should be included in both buildToolSet and askToolSet', () => { + it('should be included in both buildTools and askTools', () => {Also applies to: 18-21
packages/ai/test/tools/web-search.test.ts (1)
3-4: Align spec title with renamed exports.- it('should be included in both buildToolSet and askToolSet', () => { + it('should be included in both buildTools and askTools', () => {Also applies to: 18-21
packages/ai/test/tools/read.test.ts (2)
3-4: Rename spec title to reflect buildTools/askTools.- test('should be included in both toolsets', () => { + test('should be included in both tool sets (buildTools, askTools)', () => {Also applies to: 21-24
87-90: Rename spec title to reflect buildTools/askTools.- test('should be included in both toolsets', () => { + test('should be included in both tool sets (buildTools, askTools)', () => {apps/web/client/src/app/api/chat/helpers/stream.ts (1)
28-30: Remove unnecessaryasync.Function returns synchronously.
-export async function getToolSetFromType(chatType: ChatType) { - return chatType === ChatType.ASK ? askTools : buildTools; -} +export function getToolSetFromType(chatType: ChatType) { + return chatType === ChatType.ASK ? askTools : buildTools; +}apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (1)
41-41: TODO mismatch: code still saves on finish.Either remove the TODO or gate the save behind a flag to avoid accidental persistence.
packages/ai/src/tools/toolset.ts (1)
41-52: Optional: tighten typing with satisfies to prevent accidental key/value drift.Apply:
-export const askTools: ToolSet = { +export const askTools = { [LIST_FILES_TOOL_NAME]: listFilesTool, [READ_FILE_TOOL_NAME]: readFileTool, [BASH_READ_TOOL_NAME]: bashReadTool, [ONLOOK_INSTRUCTIONS_TOOL_NAME]: onlookInstructionsTool, [READ_STYLE_GUIDE_TOOL_NAME]: readStyleGuideTool, [LIST_BRANCHES_TOOL_NAME]: listBranchesTool, [SCRAPE_URL_TOOL_NAME]: scrapeUrlTool, [WEB_SEARCH_TOOL_NAME]: webSearchTool, [GLOB_TOOL_NAME]: globTool, [GREP_TOOL_NAME]: grepTool, -}; +} as const satisfies ToolSet;And similarly for buildTools:
-export const buildTools: ToolSet = { - ...askTools, +export const buildTools = { + ...askTools, [SEARCH_REPLACE_EDIT_FILE_TOOL_NAME]: searchReplaceEditFileTool, [SEARCH_REPLACE_MULTI_EDIT_FILE_TOOL_NAME]: searchReplaceMultiEditFileTool, [FUZZY_EDIT_FILE_TOOL_NAME]: fuzzyEditFileTool, [WRITE_FILE_TOOL_NAME]: writeFileTool, [BASH_EDIT_TOOL_NAME]: bashEditTool, [SANDBOX_TOOL_NAME]: sandboxTool, [TERMINAL_COMMAND_TOOL_NAME]: terminalCommandTool, [TYPECHECK_TOOL_NAME]: typecheckTool, -}; +} as const satisfies ToolSet;packages/ai/src/stream/index.ts (2)
29-44: Handle 'system' explicitly for clarity (minor).Currently falls through to default return; being explicit improves readability and future-proofing.
export const toVercelMessageFromOnlook = ( message: ChatMessage, opt: HydrateMessageOptions, ): VercelMessage => { if (message.role === 'assistant') { return { ...message, parts: message.parts, } satisfies VercelMessage; - } else if (message.role === 'user') { + } else if (message.role === 'user') { const hydratedMessage = getHydratedUserMessage( message.id, message.parts, message.metadata?.context ?? [], opt, ); return hydratedMessage; + } else if (message.role === 'system') { + return message; } return message; };
46-55: Guard against undefined parts in extractTextFromParts.Avoids a potential runtime error if parts is ever undefined.
-export const extractTextFromParts = (parts: ChatMessage['parts']): string => { - return parts +export const extractTextFromParts = (parts: ChatMessage['parts']): string => { + return (parts ?? []) ?.map((part) => { if (part.type === 'text') { return part.text; } return ''; }) .join(''); };apps/web/client/src/server/api/routers/chat/message.ts (2)
54-55: upsertMany is an insert; add conflict handling (or rename).Without onConflict, duplicates will error or create multiple rows. Either add conflict handling or rename to insertMany.
-await ctx.db.insert(messages).values(input.messages); +await ctx.db + .insert(messages) + .values(input.messages) + .onConflictDoNothing(); // simplest idempotencyIf you truly want upsert semantics, update on conflict to EXCLUDED values (requires sql import):
+import { asc, eq, inArray, sql } from 'drizzle-orm'; @@ await ctx.db .insert(messages) .values(input.messages) .onConflictDoUpdate({ target: [messages.id], set: { role: sql`excluded.role`, parts: sql`excluded.parts`, context: sql`excluded.context`, checkpoints: sql`excluded.checkpoints`, }, });
62-63: Optional: restrict update to mutable fields.Similar to upsert, consider whitelisting fields to avoid accidental updates to immutable columns via messageUpdateSchema.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (2)
26-37: Skip rendering wrappers for system messages (minor).Returning a wrapper div with null child adds empty nodes. Either filter system messages earlier or return null directly from map.
- return <div key={`message-${message.id}-${index}`}>{messageNode}</div>; + if (messageNode == null) return null; + return <div key={`message-${message.id}-${index}`}>{messageNode}</div>;Or pre-filter:
- {messagesToRender.map((message, index) => renderMessage(message, index))} + {messagesToRender + .filter((m) => m.role !== 'system') + .map((message, index) => renderMessage(message, index))}
38-39: Key stability: drop index from key.Using id alone avoids unnecessary remounts on reordering.
- return <div key={`message-${message.id}-${index}`}>{messageNode}</div>; + return <div key={message.id}>{messageNode}</div>;apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (2)
65-70: Handle clipboard write failures.
navigator.clipboard.writeTextcan reject (permissions/insecure context). Show a toast on failure.- const text = getUserMessageContent(message); - await navigator.clipboard.writeText(text); - setIsCopied(true); - setTimeout(() => setIsCopied(false), 2000); + try { + const text = getUserMessageContent(message); + await navigator.clipboard.writeText(text); + setIsCopied(true); + setTimeout(() => setIsCopied(false), 2000); + } catch { + toast.error('Failed to copy.'); + }
12-13: Optional: prefer alias imports for local components.To follow client import guidelines, consider aliasing
../context-pills/sent-context-pilland./message-contentvia@/*if configured.Also applies to: 147-200
apps/web/client/src/components/store/editor/chat/message.ts (1)
1-1: Import path consistency.Elsewhere
ChatMessagecomes from@onlook/models. Consider standardizing import source to reduce churn.apps/web/client/src/app/api/chat/helpers/message.ts (3)
2-2: Avoid deep package import for db client.Importing from
@onlook/db/src/clientreaches into package internals and can break builds. Use the package export.-import { db } from "@onlook/db/src/client"; +import { db } from "@onlook/db";
27-33: Name consistency: use conversationId.Parameter is
chatIdbut filters onconversationId. Rename for clarity.-export const loadChat = async (chatId: string): Promise<ChatMessage[]> => { +export const loadChat = async (conversationId: string): Promise<ChatMessage[]> => { const result = await db.query.messages.findMany({ - where: eq(messages.conversationId, chatId), + where: eq(messages.conversationId, conversationId), orderBy: (messages, { asc }) => [asc(messages.createdAt)], }); return result.map((message) => fromDbMessage(message)); };
27-31: Index for query pattern (conversationId, createdAt).Since you frequently query by
conversationIdordered bycreatedAt, ensure a composite index exists to avoid full scans.packages/models/src/chat/message/message.ts (2)
6-26: Remove commented-out legacy types.Dead code adds noise and confuses type surface.
-// interface BaseChatMessage { -// id: string; -// createdAt: Date; -// role: ChatMessageRole; -// threadId: string; -// parts: UIMessage['parts']; -// metadata: { -// vercelId?: string; -// context: MessageContext[]; -// checkpoints: MessageCheckpoints[]; -// }; -// } - -// export interface UserChatMessage extends BaseChatMessage { -// role: ChatMessageRole.USER; -// } - -// export interface AssistantChatMessage extends BaseChatMessage { -// role: ChatMessageRole.ASSISTANT; -// }
36-36: BroadenChatDataParttype.
{}is too restrictive/ambiguous. Prefer a JSON-safe map for tool data.-export type ChatDataPart = {}; +export type ChatDataPart = Record<string, JSONValue>;apps/web/client/src/components/store/editor/chat/index.ts (2)
87-89: Guard undefined when removing messages.Ensure you pass an array.
- const messagesToRemove = this.conversation.current?.messages.filter((m) => m.metadata?.createdAt && m.metadata.createdAt > oldMessage.metadata?.createdAt); - await this.conversation.removeMessages(messagesToRemove); + const messagesToRemove = this.conversation.current?.messages.filter( + (m) => m.metadata?.createdAt && oldMessage.metadata?.createdAt && m.metadata.createdAt > oldMessage.metadata.createdAt + ) ?? []; + await this.conversation.removeMessages(messagesToRemove);
2-2: Import path consistency.Consider aligning with other files (
@onlook/modelsvs@onlook/models/chat) to reduce friction.packages/db/src/mappers/chat/message.ts (1)
5-15: Optional: retire metadata.vercelId or keep semantics consistent.New UI messages initialize metadata.vercelId with a fresh UUID, but fromDbMessage sets it to the DB id. If you still need vercelId for dedup, ensure it’s stable end-to-end; otherwise, drop it.
packages/db/src/schema/chat/message.ts (2)
8-8: Typo in relation name constant.Use a consistent name.
-export const CONVERSATION_MESSAGe_RELATION_NAME = 'conversation_messages'; +export const CONVERSATION_MESSAGES_RELATION_NAME = 'conversation_messages'; @@ - relationName: CONVERSATION_MESSAGe_RELATION_NAME, + relationName: CONVERSATION_MESSAGES_RELATION_NAME,
11-27: Add index for common read path (by conversation, ordered by createdAt).Improves loadChat performance under growth.
// After pgTable definition // drizzle does this via `pgIndex` in newer versions; example: export const messagesByConversationCreatedAtIdx = pgIndex('messages_conversation_created_at_idx') .on(messages.conversationId, messages.createdAt);apps/web/client/src/components/store/editor/chat/conversation.ts (2)
212-214: Avoid mapping to DB shape on the client; push conversion to the server.Keep the client on public models; let the API accept ChatMessage + conversationId and call toDbMessage server-side.
- async upsertMessageInStorage(message: ChatMessage, conversationId: string) { - await api.chat.message.upsert.mutate({ message: toDbMessage(message, conversationId) }); - } + async upsertMessageInStorage(message: ChatMessage, conversationId: string) { + await api.chat.message.upsert.mutate({ message, conversationId }); + }
53-55: Guard condition readability.Minor: invert early-return or extract a helper (isCurrentEmptyUntitled) for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
apps/web/client/src/app/api/chat/helpers/index.ts(1 hunks)apps/web/client/src/app/api/chat/helpers/message.ts(1 hunks)apps/web/client/src/app/api/chat/helpers/stream.ts(2 hunks)apps/web/client/src/app/api/chat/route.ts(3 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx(3 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx(3 hunks)apps/web/client/src/components/store/editor/chat/conversation.ts(6 hunks)apps/web/client/src/components/store/editor/chat/index.ts(4 hunks)apps/web/client/src/components/store/editor/chat/message.ts(1 hunks)apps/web/client/src/server/api/routers/chat/message.ts(3 hunks)apps/web/client/src/server/api/routers/chat/suggestion.ts(3 hunks)packages/ai/src/stream/index.ts(2 hunks)packages/ai/src/tools/toolset.ts(4 hunks)packages/ai/test/tools/read.test.ts(3 hunks)packages/ai/test/tools/web-search.test.ts(2 hunks)packages/ai/test/tools/web.test.ts(2 hunks)packages/db/src/mappers/chat/index.ts(1 hunks)packages/db/src/mappers/chat/message.ts(1 hunks)packages/db/src/schema/chat/index.ts(1 hunks)packages/db/src/schema/chat/message.ts(1 hunks)packages/models/src/chat/message/message.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/db/src/mappers/chat/index.ts
- packages/db/src/schema/chat/index.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use process.env in client code; prefer importing env from @/env
Files:
apps/web/client/src/app/api/chat/helpers/index.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsxapps/web/client/src/app/api/chat/helpers/message.tsapps/web/client/src/app/api/chat/helpers/stream.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/app/api/chat/route.ts
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer imports using @/* or ~/* aliases instead of deep relative paths within the client app
Files:
apps/web/client/src/app/api/chat/helpers/index.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsxapps/web/client/src/app/api/chat/helpers/message.tsapps/web/client/src/components/store/editor/chat/index.tsapps/web/client/src/app/api/chat/helpers/stream.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/server/api/routers/chat/suggestion.tsapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/server/api/routers/chat/message.tsapps/web/client/src/components/store/editor/chat/message.tsapps/web/client/src/app/api/chat/route.tsapps/web/client/src/components/store/editor/chat/conversation.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the 'any' type unless necessary
Files:
apps/web/client/src/app/api/chat/helpers/index.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsxpackages/ai/test/tools/web.test.tspackages/ai/test/tools/read.test.tsapps/web/client/src/app/api/chat/helpers/message.tspackages/ai/test/tools/web-search.test.tspackages/ai/src/stream/index.tsapps/web/client/src/components/store/editor/chat/index.tspackages/models/src/chat/message/message.tsapps/web/client/src/app/api/chat/helpers/stream.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxpackages/ai/src/tools/toolset.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/server/api/routers/chat/suggestion.tspackages/db/src/mappers/chat/message.tsapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/server/api/routers/chat/message.tsapps/web/client/src/components/store/editor/chat/message.tsapps/web/client/src/app/api/chat/route.tsapps/web/client/src/components/store/editor/chat/conversation.tspackages/db/src/schema/chat/message.ts
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libraries
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat.tsx
apps/web/client/src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under apps/web/client/src/server/api/routers and export them from the API root
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return SuperJSON-serializable data (plain objects/arrays) from tRPC procedures
Files:
apps/web/client/src/server/api/routers/chat/suggestion.tsapps/web/client/src/server/api/routers/chat/message.ts
apps/web/client/src/app/**/{page,layout,route}.ts?(x)
📄 CodeRabbit inference engine (AGENTS.md)
Follow Next.js App Router structure: page.tsx, layout.tsx, and route.ts under apps/web/client/src/app
Files:
apps/web/client/src/app/api/chat/route.ts
🧠 Learnings (2)
📚 Learning: 2025-09-12T18:42:26.836Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T18:42:26.836Z
Learning: Applies to apps/web/client/messages/**/*.{json,ts} : Add or modify i18n message keys under apps/web/client/messages; avoid breaking renames
Applied to files:
apps/web/client/src/app/api/chat/helpers/index.tsapps/web/client/src/app/api/chat/helpers/message.tsapps/web/client/src/components/store/editor/chat/index.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/server/api/routers/chat/message.ts
📚 Learning: 2025-09-12T18:42:26.836Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T18:42:26.836Z
Learning: Run unit tests with 'bun test' and type checking with 'bun run typecheck'
Applied to files:
packages/ai/test/tools/read.test.ts
🧬 Code graph analysis (19)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsx (1)
packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)
packages/ai/test/tools/web.test.ts (1)
packages/ai/src/tools/toolset.ts (2)
buildTools(54-64)askTools(41-52)
packages/ai/test/tools/read.test.ts (1)
packages/ai/src/tools/toolset.ts (2)
buildTools(54-64)askTools(41-52)
apps/web/client/src/app/api/chat/helpers/message.ts (4)
packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)packages/db/src/mappers/chat/message.ts (2)
toDbMessage(18-34)fromDbMessage(4-16)packages/db/src/client.ts (1)
db(16-16)packages/db/src/schema/chat/message.ts (1)
messages(11-27)
packages/ai/test/tools/web-search.test.ts (1)
packages/ai/src/tools/toolset.ts (2)
buildTools(54-64)askTools(41-52)
packages/ai/src/stream/index.ts (2)
packages/db/src/schema/chat/message.ts (1)
messages(11-27)packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)
apps/web/client/src/components/store/editor/chat/index.ts (2)
packages/models/src/chat/message/context.ts (1)
MessageContext(53-59)packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)
packages/models/src/chat/message/message.ts (3)
packages/models/src/chat/message/context.ts (1)
MessageContext(53-59)packages/models/src/chat/message/checkpoint.ts (1)
MessageCheckpoints(15-15)packages/ai/src/tools/toolset.ts (1)
ChatTools(66-66)
apps/web/client/src/app/api/chat/helpers/stream.ts (1)
packages/ai/src/tools/toolset.ts (2)
askTools(41-52)buildTools(54-64)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (2)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsx (1)
AssistantMessage(4-15)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (1)
UserMessage(24-253)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsx (2)
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (1)
useChatContext(113-119)packages/db/src/schema/chat/message.ts (1)
messages(11-27)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (1)
packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)
packages/db/src/mappers/chat/message.ts (1)
packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (1)
packages/db/src/schema/chat/message.ts (1)
messages(11-27)
apps/web/client/src/server/api/routers/chat/message.ts (1)
packages/db/src/schema/chat/message.ts (2)
messages(11-27)messageUpdateSchema(30-30)
apps/web/client/src/components/store/editor/chat/message.ts (2)
packages/models/src/chat/message/context.ts (1)
MessageContext(53-59)packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)
apps/web/client/src/app/api/chat/route.ts (3)
packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)apps/web/client/src/app/api/chat/helpers/message.ts (2)
upsertMessage(6-25)loadChat(27-33)packages/db/src/schema/chat/message.ts (1)
messages(11-27)
apps/web/client/src/components/store/editor/chat/conversation.ts (4)
packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)apps/web/client/src/components/store/editor/chat/message.ts (1)
getUserChatMessageFromString(4-21)packages/git/src/git.ts (1)
GitCommit(19-25)packages/db/src/mappers/chat/message.ts (1)
toDbMessage(18-34)
packages/db/src/schema/chat/message.ts (4)
packages/db/src/schema/chat/conversation.ts (1)
conversations(11-26)packages/models/src/chat/message/context.ts (1)
MessageContext(53-59)packages/models/src/chat/message/checkpoint.ts (1)
MessageCheckpoints(15-15)packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)
🪛 Biome (2.1.2)
apps/web/client/src/components/store/editor/chat/index.ts
[error] 83-83: Invalid assignment to oldMessage.metadata?.context
This expression cannot be assigned to
(parse)
apps/web/client/src/components/store/editor/chat/conversation.ts
[error] 165-165: Invalid assignment to userMessage.metadata?.checkpoints
This expression cannot be assigned to
(parse)
🔇 Additional comments (15)
apps/web/client/src/server/api/routers/chat/suggestion.ts (1)
35-39: Keep internal system prompt last and immutable.Even with validation, ensure no user message with system semantics precedes or follows your system prompt. Current spread is fine if 'system' is disallowed; otherwise, move user messages after the final user prompt or filter as shown above.
apps/web/client/src/app/api/chat/helpers/index.ts (1)
1-1: Barrel re-export looks good.apps/web/client/src/app/api/chat/helpers/stream.ts (1)
2-2: Import rename LGTM.apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (2)
89-91:findLastIndexbrowser support.If you target older browsers, consider a fallback for
findLastIndex.Would you like a tiny helper util to compute last index compatible with older environments?
52-56: Ensure useChat picks up updated conversationId on re-renders.addOrReplaceMessage(toOnlookMessageFromVercel(...)) is invoked with currentConversationId in apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (lines 52–56 and 69–72 — rg confirms). Sandbox typecheck failed (
bun: command not found), so run typecheck/CI locally. Ensure the hook re-reads conversation.current?.conversation.id (or subscribes to conversation changes / avoids stale closures) so messages land in the correct conversation.apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsx (1)
1-1: LGTM: type migration to ChatMessage is consistent.No client-only hooks here; safe to keep as a Server Component boundary.
Also applies to: 4-4
packages/ai/src/tools/toolset.ts (1)
41-66: Approve rename — verify no leftover referencesaskTools/buildTools + ChatTools rename looks good and preserves behavior via the spread. Repository search found no matches for ASK_TOOL_SET or BUILD_TOOL_SET; run locally: rg -nP 'ASK_TOOL_SET|BUILD_TOOL_SET' (or confirm CI) to ensure no stragglers remain.
packages/ai/src/stream/index.ts (1)
7-10: String-literal role migration is aligned with the new model.apps/web/client/src/server/api/routers/chat/message.ts (2)
23-24: LGTM: mapping DB rows via fromDbMessage.Keeps router output aligned with the unified ChatMessage shape.
31-38: Ownership/authorization check relies on RLS—please verify policies.You validate existence but not ownership. If RLS enforces tenant scoping on conversations/messages, fine; otherwise add a workspace/user check.
Provide evidence (policy file or tests) that messages/conversations have RLS policies restricting access to the current user/workspace.
apps/web/client/src/components/store/editor/chat/message.ts (1)
9-20: VerifyvercelIdsemantics; mapping currently overwrites it.
fromDbMessagesetsmetadata.vercelId = message.idwhen reading from DB, which discards the UUID assigned here. IfvercelIdis intended to track provider IDs, align creation and mappers, or set it equal toidat creation for consistency.Possible local alignment:
- return { - id: uuidv4(), + const id = uuidv4(); + return { + id, role: 'user', parts: [{ type: 'text', text: content }], metadata: { createdAt: new Date(), conversationId, - vercelId: uuidv4(), + vercelId: id, context, checkpoints: [], }, }apps/web/client/src/app/api/chat/helpers/message.ts (1)
18-23: Confirm upsert update set.On conflict you only update
conversationId. If messages can change (e.g., parts/role/context edits), you may want to update those fields or enforce immutability explicitly.packages/models/src/chat/message/message.ts (1)
3-5: Verify TS/ESM resolver for .ts import specifierspackages/models/src/chat/message/message.ts imports './checkpoint.ts' and './context.ts'. packages/models/tsconfig.json extends '@onlook/typescript/base.json' (no local flags) — confirm the extended/base config or consuming build sets either:
- moduleResolution: "bundler"
OR- module: "NodeNext" / moduleResolution: "nodeNext"
and also has allowImportingTsExtensions: true and verbatimModuleSyntax: true.
If those aren't present, remove the '.ts' extensions or update the tsconfig/build to avoid runtime/build breakage. Check packages/models/tsconfig.json and the referenced @onlook/typescript/base.json (or root/bundler config).apps/web/client/src/app/api/chat/route.ts (1)
79-81: Trace id choice looks good.Stable across retries and useful for telemetry correlation.
packages/db/src/schema/chat/message.ts (1)
9-9: Confirm whether 'tool' message role is needed; add 'tool' to the enum if used.Repo search returned no matches for role:'tool'; confirm whether the AI pipeline emits tool messages — if yes, add 'tool' to packages/db/src/schema/chat/message.ts (message_role enum).
| const { message, chatType, conversationId, projectId } = body as { | ||
| message: ChatMessage, | ||
| chatType: ChatType, | ||
| conversationId: string, | ||
| projectId: string, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate request body.
Add schema validation (zod) to ensure message.id/role/parts, chatType, conversationId, and projectId are present and well-formed before proceeding.
import { v4 as uuidv4 } from 'uuid';
+import { z } from 'zod';
@@
- const { message, chatType, conversationId, projectId } = body as {
- message: ChatMessage,
- chatType: ChatType,
- conversationId: string,
- projectId: string,
- };
+ const Body = z.object({
+ message: z.any(), // ideally: refine to your ChatMessage runtime shape
+ chatType: z.nativeEnum(ChatType),
+ conversationId: z.string().uuid(),
+ projectId: z.string().uuid(),
+ });
+ const { message, chatType, conversationId, projectId } = Body.parse(body);📝 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 { message, chatType, conversationId, projectId } = body as { | |
| message: ChatMessage, | |
| chatType: ChatType, | |
| conversationId: string, | |
| projectId: string, | |
| }; | |
| import { v4 as uuidv4 } from 'uuid'; | |
| import { z } from 'zod'; | |
| const Body = z.object({ | |
| message: z.any(), // ideally: refine to your ChatMessage runtime shape | |
| chatType: z.nativeEnum(ChatType), | |
| conversationId: z.string().uuid(), | |
| projectId: z.string().uuid(), | |
| }); | |
| const { message, chatType, conversationId, projectId } = Body.parse(body); |
🤖 Prompt for AI Agents
In apps/web/client/src/app/api/chat/route.ts around lines 56-61, the request
body is destructured without validation; add a zod schema to validate that
message has id: string, role: enum/string matching allowed roles, and parts:
non-empty array of strings (or objects matching ChatMessagePart), and that
chatType, conversationId, and projectId are present non-empty strings (chatType
should match the ChatType union). Replace the current unchecked destructuring
with schema.parse(body) (or safeParse and return a 400 response on failure), and
use the parsed values for subsequent logic so malformed requests are rejected
early with a clear 400 error.
| // create or update last message in database | ||
| // https://github.com/vercel-labs/ai-sdk-persistence-db/blob/main/lib/db/actions.ts#L50 | ||
| await upsertMessage({ conversationId, message }); | ||
|
|
||
| // load the previous messages from the server: | ||
| // https://github.com/vercel-labs/ai-sdk-persistence-db/blob/main/lib/db/actions.ts#L50 | ||
| const messages = await loadChat(conversationId); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Authorize conversation access and enforce server-side conversationId.
Ensure the provided conversationId belongs to userId/projectId before upserting. Ignore any client-sent metadata.conversationId and trust only the route param.
- await upsertMessage({ conversationId, message });
+ // Optionally: await assertUserOwnsConversation({ userId, projectId, conversationId });
+ await upsertMessage({ conversationId, message: { ...message, metadata: { ...(message.metadata ?? {}), conversationId } } });Also, the helper’s onConflictDoUpdate currently updates only conversationId. Consider updating role, parts, context, checkpoints, and createdAt for idempotence:
- .onConflictDoUpdate({
- target: messages.id,
- set: { conversationId },
- })
+ .onConflictDoUpdate({
+ target: messages.id,
+ set: {
+ conversationId,
+ role: dbMessage.role,
+ parts: dbMessage.parts,
+ context: dbMessage.context,
+ checkpoints: dbMessage.checkpoints,
+ createdAt: dbMessage.createdAt,
+ },
+ })Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/client/src/app/api/chat/route.ts around lines 63-70, you must
enforce server-side conversationId and authorize access before persisting or
loading messages: ignore any client-sent metadata.conversationId and
derive/validate conversationId from the authenticated route param and
user/project context (check that conversationId belongs to the current
userId/projectId and return 403 if not); only then call upsertMessage and
loadChat with the server-validated conversationId. Also update the
upsert/onConflictDoUpdate logic so it updates role, parts, context, checkpoints,
and createdAt (not just conversationId) to make the operation idempotent and
keep message state consistent. Ensure errors are handled and reject requests
when authorization fails.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
Show resolved
Hide resolved
| import { useChatContext } from '@/app/project/[id]/_hooks/use-chat'; | ||
| import { useEditorEngine } from '@/components/store/editor'; | ||
| import { ChatType, MessageCheckpointType, type UserChatMessage } from '@onlook/models'; | ||
| import { ChatType, MessageCheckpointType, type ChatMessage } from '@onlook/models'; |
There was a problem hiding this comment.
Add 'use client' — hooks used in this component will crash without it.
This file uses React state/effects and browser APIs; mark it as a Client Component.
+ 'use client';
+
import { useChatContext } from '@/app/project/[id]/_hooks/use-chat';📝 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 { ChatType, MessageCheckpointType, type ChatMessage } from '@onlook/models'; | |
| 'use client'; | |
| import { useChatContext } from '@/app/project/[id]/_hooks/use-chat'; | |
| import { ChatType, MessageCheckpointType, type ChatMessage } from '@onlook/models'; |
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx
around line 3, this component uses React hooks/browser APIs but lacks the "use
client" directive; add a "use client" literal as the very first line of the file
(before any imports) so React treats it as a Client Component and hooks/effects
will run correctly.
| .values(input.message) | ||
| .onConflictDoUpdate({ | ||
| target: [messages.id], | ||
| set: { | ||
| ...normalizedMessage, | ||
| ...input.message, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid updating immutable fields during upsert.
Spreading input into set may update id/conversationId/createdAt unintentionally. Restrict to mutable columns.
- .onConflictDoUpdate({
- target: [messages.id],
- set: {
- ...input.message,
- },
- });
+ .onConflictDoUpdate({
+ target: [messages.id],
+ set: {
+ // explicitly allow only mutable fields
+ role: input.message.role,
+ parts: input.message.parts,
+ context: input.message.context,
+ checkpoints: input.message.checkpoints,
+ },
+ });📝 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.
| .values(input.message) | |
| .onConflictDoUpdate({ | |
| target: [messages.id], | |
| set: { | |
| ...normalizedMessage, | |
| ...input.message, | |
| }, | |
| }); | |
| .values(input.message) | |
| .onConflictDoUpdate({ | |
| target: [messages.id], | |
| set: { | |
| // explicitly allow only mutable fields | |
| role: input.message.role, | |
| parts: input.message.parts, | |
| context: input.message.context, | |
| checkpoints: input.message.checkpoints, | |
| }, | |
| }); |
| role: z.enum(['user', 'assistant', 'system']), | ||
| content: z.string(), | ||
| })), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Disallow client-supplied 'system' role to prevent prompt injection.
Allowing 'system' in input lets users override internal instructions. Restrict to 'user' | 'assistant' or sanitize before passing to the model.
- messages: z.array(z.object({
- role: z.enum(['user', 'assistant', 'system']),
- content: z.string(),
- })),
+ messages: z.array(z.object({
+ role: z.enum(['user', 'assistant']),
+ content: z.string().min(1),
+ })),And defensively filter just in case:
- ...input.messages,
+ ...input.messages.filter(m => m.role !== 'system'),📝 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.
| role: z.enum(['user', 'assistant', 'system']), | |
| content: z.string(), | |
| })), | |
| role: z.enum(['user', 'assistant']), | |
| content: z.string().min(1), | |
| })), |
🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/chat/suggestion.ts around lines 17 to
19, the Zod schema currently allows a client-supplied 'system' role which can
enable prompt injection; change the schema to only accept 'user' and 'assistant'
(remove 'system') and/or add defensive filtering after parsing to drop any
messages with role === 'system' before forwarding to the model, ensuring the
server enforces internal system instructions and never trusts client-provided
system roles.
| applied: boolean("applied"), | ||
| commitOid: text("commit_oid"), | ||
| snapshots: jsonb("snapshots").$type<any>(), | ||
| content: text("content"), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enforce “content OR parts” invariant at DB-level.
Prevent empty messages by adding a CHECK constraint.
// import { sql } from "drizzle-orm";
export const messages = pgTable("messages", {
// ...columns...
}, (t) => ({
contentOrPartsChk: {
// CHECK ((content IS NOT NULL) OR (jsonb_array_length(parts) > 0))
// If using drizzle-orm/sql:
constraint: sql`(content IS NOT NULL) OR (jsonb_array_length(${t.parts}) > 0)`,
},
}));🤖 Prompt for AI Agents
In packages/db/src/schema/chat/message.ts around line 26, the messages table
allows empty rows because there is no DB-level CHECK enforcing that either
content is present or parts (jsonb array) has length > 0; add a CHECK constraint
to the pgTable options so the table definition includes a constraint like:
constraint: sql`(content IS NOT NULL) OR (jsonb_array_length(${t.parts}) > 0)`,
and import sql from "drizzle-orm" (or drizzle-orm/sql) at the top; update the
table factory call to include this constraint entry (e.g., in the third argument
callback returning contentOrPartsChk) so the database prevents empty messages.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
packages/db/src/mappers/chat/message.ts (2)
5-15: Stop spreading DbMessage into ChatMessage (leaks DB-only fields).Spreading
...messagepulls in DB-only/deprecated fields (e.g.,content,applied,commitOid,snapshots, possiblyconversationId) into the UI object and risks excess-property issues. Map explicitly.Apply:
export const fromDbMessage = (message: DbMessage): ChatMessage => { - return { - ...message, - metadata: { - conversationId: message.conversationId, - createdAt: message.createdAt, - vercelId: message.id, - context: message.context ?? [], - checkpoints: message.checkpoints ?? [], - }, - parts: message.parts ?? [], - } + return { + id: message.id, + role: message.role, + parts: message.parts ?? [], + metadata: { + conversationId: message.conversationId, + createdAt: message.createdAt, + vercelId: message.id, + context: message.context ?? [], + checkpoints: message.checkpoints ?? [], + // include parent links if present (see toDbMessage comment) + parentConversationId: (message as any).parentConversationId ?? null, + parentMessageId: (message as any).parentMessageId ?? null, + }, + }; }
24-27: Guaranteepartsis an array when persisting.
parts: message.partscan writeundefined|null, violating the schema expectation (array).- parts: message.parts, + parts: Array.isArray(message.parts) ? message.parts : (message.parts ?? []),apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (1)
1-11: Missing 'use client' directive.This component uses React hooks and browser APIs but lacks the required directive for client components.
+'use client'; + import { useChatContext } from '@/app/project/[id]/_hooks/use-chat';apps/web/client/src/app/api/chat/route.ts (1)
64-69: Add request-body validation (use zod)Validate req.json() in streamResponse before destructuring; zod is present in apps/web/client/package.json (zod ^4.1.3). File: apps/web/client/src/app/api/chat/route.ts — replace the current req.json() destructure.
+import { z } from 'zod'; + +const requestBodySchema = z.object({ + message: z.object({ + id: z.string(), + role: z.string(), + parts: z.array(z.any()).min(1), + metadata: z.any().optional(), + }), + chatType: z.nativeEnum(ChatType), + conversationId: z.string().uuid(), + projectId: z.string().uuid(), +}); export const streamResponse = async (req: NextRequest, userId: string) => { // ... existing code ... try { - const { message, chatType, conversationId, projectId }: { - message: ChatMessage, - chatType: ChatType, - conversationId: string, - projectId: string, - } = await req.json() + const body = await req.json(); + const validationResult = requestBodySchema.safeParse(body); + if (!validationResult.success) { + return new Response(JSON.stringify({ + error: 'Invalid request body', + code: 400, + details: validationResult.error.flatten() + }), { + status: 400, + headers: { 'Content-Type': 'application/json' } + }); + } + const { message, chatType, conversationId, projectId } = validationResult.data;apps/web/client/src/components/store/editor/chat/conversation.ts (1)
123-125: Title generation blocked by non-null placeholder displayName.Same issue as line 51 - if
displayNamedefaults to a placeholder string, automatic title generation won't occur.- if (!this.current.conversation.displayName) { + if (!this.current.conversation.displayName || this.current.conversation.displayName === 'New Conversation') {
🧹 Nitpick comments (2)
apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx (1)
101-106: Confirm intent: using addEditMessage for CREATE flow.If CREATE needs different metadata (e.g., tags, parent links), a dedicated
addCreateMessagehelper may avoid misclassification.Proposed tweak:
- const message = await editorEngine.chat.addEditMessage( + const message = await editorEngine.chat.addCreateMessage?.( prompt, context, ); - sendMessageToChat(message, ChatType.CREATE); + sendMessageToChat(message, ChatType.CREATE);If
addCreateMessagedoesn't exist, verifyaddEditMessagesets appropriate metadata for CREATE and carries parent IDs when resuming requests.packages/ai/src/tools/toolset.ts (1)
69-71: Drop unnecessary async from getToolSetFromType.It’s synchronous; returning a Promise adds overhead without benefit.
-export async function getToolSetFromType(chatType: ChatType) { - return chatType === ChatType.ASK ? askTools : buildTools; -} +export function getToolSetFromType(chatType: ChatType): ToolSet { + return chatType === ChatType.ASK ? askTools : buildTools; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/client/src/app/api/chat/helpers/message.ts(1 hunks)apps/web/client/src/app/api/chat/helpers/stream.ts(1 hunks)apps/web/client/src/app/api/chat/route.ts(3 hunks)apps/web/client/src/app/project/[id]/_components/canvas/overlay/elements/buttons/chat.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx(4 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/error.tsx(1 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx(4 hunks)apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx(1 hunks)apps/web/client/src/components/store/editor/chat/conversation.ts(5 hunks)apps/web/client/src/components/store/editor/chat/index.ts(4 hunks)apps/web/client/src/components/tools/tools.ts(2 hunks)packages/ai/src/tools/toolset.ts(4 hunks)packages/db/src/mappers/chat/message.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/client/src/app/api/chat/helpers/message.ts
🧰 Additional context used
📓 Path-based instructions (5)
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer imports using @/* or ~/* aliases instead of deep relative paths within the client app
Files:
apps/web/client/src/components/tools/tools.tsapps/web/client/src/app/project/[id]/_components/canvas/overlay/elements/buttons/chat.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/error.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/api/chat/helpers/stream.tsapps/web/client/src/app/project/[id]/_hooks/use-start-project.tsxapps/web/client/src/components/store/editor/chat/conversation.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/components/store/editor/chat/index.tsapps/web/client/src/app/api/chat/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the 'any' type unless necessary
Files:
apps/web/client/src/components/tools/tools.tsapps/web/client/src/app/project/[id]/_components/canvas/overlay/elements/buttons/chat.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/error.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/api/chat/helpers/stream.tsapps/web/client/src/app/project/[id]/_hooks/use-start-project.tsxpackages/db/src/mappers/chat/message.tsapps/web/client/src/components/store/editor/chat/conversation.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/components/store/editor/chat/index.tspackages/ai/src/tools/toolset.tsapps/web/client/src/app/api/chat/route.ts
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libraries
Files:
apps/web/client/src/app/project/[id]/_components/canvas/overlay/elements/buttons/chat.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/error.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/project/[id]/_hooks/use-start-project.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use process.env in client code; prefer importing env from @/env
Files:
apps/web/client/src/app/project/[id]/_components/canvas/overlay/elements/buttons/chat.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/error.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsxapps/web/client/src/app/api/chat/helpers/stream.tsapps/web/client/src/app/project/[id]/_hooks/use-start-project.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/app/api/chat/route.ts
apps/web/client/src/app/**/{page,layout,route}.ts?(x)
📄 CodeRabbit inference engine (AGENTS.md)
Follow Next.js App Router structure: page.tsx, layout.tsx, and route.ts under apps/web/client/src/app
Files:
apps/web/client/src/app/api/chat/route.ts
🧠 Learnings (2)
📚 Learning: 2025-09-12T18:42:26.836Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T18:42:26.836Z
Learning: Applies to apps/web/client/messages/**/*.{json,ts} : Add or modify i18n message keys under apps/web/client/messages; avoid breaking renames
Applied to files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx
📚 Learning: 2025-09-12T18:42:26.836Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T18:42:26.836Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libraries
Applied to files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx
🧬 Code graph analysis (6)
packages/db/src/mappers/chat/message.ts (1)
packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)
apps/web/client/src/components/store/editor/chat/conversation.ts (3)
packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)apps/web/client/src/components/store/editor/chat/message.ts (1)
getUserChatMessageFromString(4-21)packages/db/src/mappers/chat/message.ts (1)
toDbMessage(18-35)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (1)
packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (4)
packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)apps/web/client/src/components/store/editor/chat/context.ts (1)
ChatContext(16-291)apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)packages/db/src/schema/chat/message.ts (1)
messages(11-27)
apps/web/client/src/components/store/editor/chat/index.ts (4)
packages/models/src/chat/message/context.ts (1)
MessageContext(53-59)packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)packages/git/src/git.ts (1)
GitCommit(19-25)apps/web/client/src/trpc/react.tsx (1)
api(23-23)
apps/web/client/src/app/api/chat/route.ts (4)
packages/models/src/chat/message/message.ts (1)
ChatMessage(38-38)apps/web/client/src/app/api/chat/helpers/message.ts (2)
upsertMessage(6-26)loadChat(28-34)packages/db/src/schema/chat/message.ts (1)
messages(11-27)apps/web/client/src/app/api/chat/helpers/usage.ts (1)
incrementUsage(43-65)
🔇 Additional comments (17)
packages/db/src/mappers/chat/message.ts (1)
18-24: Persist subchat relationships (parentConversationId / parentMessageId)
- packages/models/src/chat/message/message.ts — ChatMetadata does NOT include parentConversationId/parentMessageId.
- packages/db/src/schema/chat/message.ts — messages table has no parentConversationId/parentMessageId columns.
- If subchat links should live on Message: add optional parentConversationId?: string | null and parentMessageId?: string | null to ChatMetadata; add corresponding columns + migration to messages table; update packages/db/src/mappers/chat/message.ts:
- toDbMessage: parentConversationId: message.metadata?.parentConversationId ?? null, parentMessageId: message.metadata?.parentMessageId ?? null
- fromDbMessage: mirror message.parentConversationId/message.parentMessageId into metadata.
- If subchat links belong on Conversation (conversation.parentConversationId/parentMessageId already implemented), keep message mapper as-is and confirm intended design.
apps/web/client/src/components/tools/tools.ts (1)
12-13: Good move: consume getToolSetFromType from the AI package.Centralizing toolset resolution in the library reduces drift between UI and backend logic.
apps/web/client/src/app/api/chat/helpers/stream.ts (1)
2-2: LGTM: toolset mapping removed from here and sourced from the AI lib.This keeps stream helpers focused on model/system-prompt concerns.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (1)
139-139: API update wired correctly: sending the constructed message to chat.Passing
(message, chatMode)matches the new signature and preserves the built payload.apps/web/client/src/app/project/[id]/_components/canvas/overlay/elements/buttons/chat.tsx (1)
31-33: Mini-chat path updated to two-arg send — looks good.Capturing
messagebefore sending aligns with the new flow.apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/error.tsx (1)
23-25: Fix flow is consistent with new API; errors surfaced to users.Good user feedback on failure.
packages/ai/src/tools/toolset.ts (2)
42-53: askTools composition looks correct and minimal for ASK.Clean separation of read-only capabilities.
55-66: buildTools correctly supersets askTools.Good reuse; avoids duplication.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (1)
87-87: LGTM! Correct update to sendMessageToChat signature.The function call now correctly passes both the message and chat type, aligning with the updated signature.
apps/web/client/src/components/store/editor/chat/index.ts (3)
49-54: LGTM! Proper separation of concerns for commit attachment.The new
attachCommitToUserMessagemethod properly handles the commit attachment logic that was previously in the conversation manager.
56-87: Good error handling and metadata management.The function properly checks for missing conversation and message, builds metadata safely without optional chaining on the left-hand side, and correctly updates checkpoints.
119-134: Proper metadata reconstruction without invalid optional chaining.The metadata object is correctly rebuilt using spread operator, avoiding the previous invalid optional chaining assignment issue.
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (3)
1-2: Add 'use client' directive at the top of the file.This file uses hooks, state, and browser APIs. According to the coding guidelines, client components need the 'use client' directive as the first line.
The file already has 'use client', which is correct.
28-41: Clean implementation of single message sending.The
prepareSendMessagesRequestcorrectly sends only the last message with necessary context, avoiding redundant message history transmission.
86-99: Good use of deep cloning for message immutability.Using
jsonCloneensures the original message object isn't mutated, which is important for React state management.apps/web/client/src/app/api/chat/route.ts (1)
71-72: Add access control check before upserting messageConfirm whether a conversation/project access-check exists; if absent, add one before calling upsertMessage and return 403 on unauthorized.
+ // Verify user has access to this conversation + const hasAccess = await verifyUserConversationAccess(userId, conversationId, projectId); + if (!hasAccess) { + return new Response(JSON.stringify({ + error: 'Unauthorized access to conversation', + code: 403 + }), { + status: 403, + headers: { 'Content-Type': 'application/json' } + }); + } + // create or update last message in database await upsertMessage({ conversationId, message });apps/web/client/src/components/store/editor/chat/conversation.ts (1)
185-187: LGTM! Proper update to upsertMessageInStorage signature.The method correctly passes both the message and conversationId to maintain proper data relationships.
| try { | ||
| this.creatingConversation = true; | ||
| if (this.current?.messages.length === 0 && !this.current?.conversation.title) { | ||
| if (this.current?.messages.length === 0 && !this.current?.conversation.displayName) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Title generation check needs adjustment for non-null default displayName.
The condition checks for falsy displayName, but if it defaults to a non-null placeholder string like "New Conversation", title generation won't trigger.
- if (this.current?.messages.length === 0 && !this.current?.conversation.displayName) {
+ if (this.current?.messages.length === 0 && (!this.current?.conversation.displayName || this.current?.conversation.displayName === 'New Conversation')) {Let me verify what the default displayName value is:
#!/bin/bash
# Check for default displayName values in conversation creation
rg -n "displayName.*=.*['\"]New Conversation['\"]|displayName:.*['\"]New Conversation['\"]" --type ts🤖 Prompt for AI Agents
In apps/web/client/src/components/store/editor/chat/conversation.ts around line
51, the current condition only checks for a falsy displayName so it won't
trigger title generation when displayName defaults to a non-null placeholder
like "New Conversation"; update the condition to also treat the default
placeholder as "no name" (e.g., check if !this.current?.conversation.displayName
|| this.current.conversation.displayName === 'New Conversation' or compare
against the shared/default constant if one exists) so that when messages length
is 0 and the displayName is the default placeholder the title generation runs.
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| return updatedMessage; | ||
| }; | ||
|
|
||
| export const debouncedUpsertMessage = debounce(upsertMessage, 500); |
There was a problem hiding this comment.
Using lodash's debounce with an async function may not behave as expected. Debounced functions don’t reliably propagate promise results, which could lead to race conditions or missed DB updates. Consider using an async‐aware debounce or ensuring proper flush (e.g. with { leading: false, trailing: true } and manual flush) if DB persistence is critical.
| }); | ||
| }, | ||
| onFinish: ({ message }) => { | ||
| onFinish: (options: { message: ChatMessage, }) => { |
There was a problem hiding this comment.
The onFinish callback accesses 'options.finishReason', but the parameter type only specifies { message: ChatMessage }. If finishReason is expected, update the type definition to include it to avoid potential undefined behavior.
| onFinish: (options: { message: ChatMessage, }) => { | |
| onFinish: (options: { message: ChatMessage, finishReason: string }) => { |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/client/src/server/api/routers/usage/index.ts (2)
75-83: Idempotency bug: credit is decremented even when insert conflicts (duplicate traceId).You decrement rate limits before inserting the usage record; with
onConflictDoNothing()a duplicate insert returns no row but the credit remains deducted. Add back the credit when no row was inserted, and target the specific unique constraint to avoid masking unrelated conflicts.- const usageRecord = await tx.insert(usageRecords).values({ + const usageRecord = await tx.insert(usageRecords).values({ userId: user.id, type: input.type, timestamp: new Date(), traceId: input.traceId, - }).onConflictDoNothing().returning({ id: usageRecords.id }); + }) + .onConflictDoNothing({ target: [usageRecords.userId, usageRecords.traceId] }) + .returning({ id: usageRecords.id }); + + const usageRecordId = usageRecord?.[0]?.id; + if (!usageRecordId && rateLimitId) { + // roll back the deducted credit on duplicate traceId + await tx.update(rateLimits).set({ + left: sql`${rateLimits.left} + 1`, + }).where(eq(rateLimits.id, rateLimitId)); + rateLimitId = undefined; + } - - return { rateLimitId, usageRecordId: usageRecord?.[0]?.id }; + return { rateLimitId, usageRecordId };
4-4: Incorrect date-fns import – likely runtime error.
date-fns/subuses a default export; named import will be undefined at runtime. Switch to default or top-level named import.- import { sub } from 'date-fns/sub'; + import sub from 'date-fns/sub'; + // or: import { sub } from 'date-fns';apps/web/client/src/app/api/chat/route.ts (1)
79-87: Authorize conversation access and enforce server-side conversationId in message metadata.Ensure
conversationIdbelongs touserId/projectIdand override any client-sentmetadata.conversationId.- const modelConfig = await getModelFromType(chatType); + // TODO: assert user owns conversation before proceeding (return 403 on failure) + // await assertUserOwnsConversation({ userId, projectId, conversationId }); + const modelConfig = await getModelFromType(chatType);And keep overriding metadata when upserting (as shown in the previous diff).
♻️ Duplicate comments (6)
packages/db/src/mappers/chat/message.ts (2)
5-15: Don’t spread DB rows into UI messages (leaks deprecated/DB-only fields).Reintroduces the previously fixed issue; map explicitly to the UI shape.
export const fromDbMessage = (message: DbMessage): ChatMessage => { - return { - ...message, - metadata: { - conversationId: message.conversationId, - createdAt: message.createdAt, - context: message.context ?? [], - checkpoints: message.checkpoints ?? [], - }, - parts: message.parts ?? [], - } + return { + id: message.id, + role: message.role, + parts: message.parts ?? [], + metadata: { + conversationId: message.conversationId, + createdAt: message.createdAt, + context: message.context ?? [], + checkpoints: message.checkpoints ?? [], + }, + }; }
23-26: Guaranteepartsis always an array when persisting.Avoid writing null/undefined; aligns with schema default([]).
- parts: message.parts, + parts: message.parts ?? [],apps/web/client/src/components/store/editor/chat/index.ts (1)
55-85: Good fix: no optional-chaining on LHS when updating metadata.You correctly materialize and replace
message.metadatainstead of assigning to?.checkpoints/?.context.Minor: consider using the commit’s timestamp for the checkpoint.
- { + { type: MessageCheckpointType.GIT, oid: commit.oid, - createdAt: new Date(), + createdAt: new Date(commit.timestamp), },apps/web/client/src/components/store/editor/chat/conversation.ts (2)
112-127: Title generation condition has same placeholder issue.Same as above: if a non-null placeholder is used, this won’t trigger title generation.
- if (!this.current.conversation.displayName) { + if (!this.current.conversation.displayName || + this.current.conversation.displayName === 'New Conversation') {
51-53: Guard misses “placeholder” displayName; can create duplicate empty conversations.If
displayNamedefaults to a non-empty placeholder (e.g., "New Conversation"), this check won’t block duplicates. Treat the placeholder as empty or usenulldefault.- if (this.current?.messages.length === 0 && !this.current?.conversation.displayName) { + if ( + this.current?.messages.length === 0 && + (!this.current?.conversation.displayName || + this.current?.conversation.displayName === 'New Conversation') + ) {apps/web/client/src/app/api/chat/route.ts (1)
66-72: Validate request body (schema) before use.Add Zod (or similar) to enforce shape; reject bad requests with 400.
+import { z } from 'zod'; @@ - const { message, chatType, conversationId, projectId }: { - message: ChatMessage, - chatType: ChatType, - conversationId: string, - projectId: string, - } = await req.json() + const Body = z.object({ + message: z.any(), // TODO: refine to ChatMessage runtime schema + chatType: z.nativeEnum(ChatType), + conversationId: z.string().uuid(), + projectId: z.string().uuid(), + }); + const body = await req.json(); + const { message, chatType, conversationId, projectId } = Body.parse(body);
🧹 Nitpick comments (8)
apps/web/client/src/server/api/routers/usage/index.ts (1)
29-33: Validate traceId length to match DB schema.DB column is
varchar(255); cap the input to prevent insert errors.- traceId: z.string().optional(), + traceId: z.string().max(255, 'traceId must be <= 255 chars').optional(),apps/web/client/src/components/store/editor/chat/message.ts (1)
4-20: Guard against empty input and normalize text.Trim the content and avoid emitting empty text parts; default context to [].
export const getUserChatMessageFromString = ( content: string, - context: MessageContext[], + context: MessageContext[] = [], conversationId: string, ): ChatMessage => { + const text = content.trim(); return { id: uuidv4(), role: 'user', - parts: [{ type: 'text', text: content }], + parts: text ? [{ type: 'text', text }] : [], metadata: { createdAt: new Date(), conversationId, context, checkpoints: [], }, } }packages/models/src/chat/message/message.ts (1)
14-16: Tighten the “empty data” type.
{}accepts any non-nullish object. Use a no-extra-keys shape to prevent accidental properties.-export type ChatDataPart = {}; +export type ChatDataPart = Record<string, never>;apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (1)
54-54: Drop noisy console log.Avoid spamming logs in production.
- console.log('onFinish', options); + // onFinish: handled belowapps/web/client/src/components/store/editor/chat/index.ts (1)
41-46: Fire-and-forget commit attach: confirm intent or await it.
this.createAndAttachCommitToUserMessage(...)is not awaited; if the commit/checkpoint must exist before streaming or UI actions, await it or explicitly mark as fire-and-forget withvoidto signal intent.Apply:
- this.createAndAttachCommitToUserMessage(userMessage.id, content); + void this.createAndAttachCommitToUserMessage(userMessage.id, content);apps/web/client/src/components/store/editor/chat/conversation.ts (2)
139-145: Keep current and list entries in sync after title generation.Also update
this.current.conversation.displayNamewhen the selected conversation matches.const listConversation = this.conversations.find((c) => c.id === conversationId); if (!listConversation) { console.error('No conversation found'); return; } - listConversation.displayName = title; + listConversation.displayName = title; + if (this.current?.conversation.id === conversationId) { + this.current.conversation.displayName = title; + }
147-158: Consider persisting on add/replace or remove unused helper.
addOrReplaceMessageno longer upserts; if persistence is now centralized in the API route, removeupsertMessageInStorageto avoid dead code. If local persistence is still desired in some flows, call it here.apps/web/client/src/app/api/chat/route.ts (1)
131-137: Assistant message metadata: consider propagating subchat linkage if applicable.If metadata now supports
parentConversationId/parentMessageId, set them here when continuing a subchat.messageMetadata: (_) => ({ createdAt: new Date(), conversationId, context: [], checkpoints: [], + // parentConversationId, + // parentMessageId, }),Would you like me to wire this based on the last user message ID in
messages?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/client/src/app/api/chat/helpers/message.ts(1 hunks)apps/web/client/src/app/api/chat/route.ts(4 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx(3 hunks)apps/web/client/src/components/store/editor/chat/conversation.ts(5 hunks)apps/web/client/src/components/store/editor/chat/index.ts(4 hunks)apps/web/client/src/components/store/editor/chat/message.ts(1 hunks)apps/web/client/src/server/api/routers/usage/index.ts(1 hunks)packages/db/src/mappers/chat/message.ts(1 hunks)packages/models/src/chat/message/message.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/client/src/app/api/chat/helpers/message.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under apps/web/client/src/server/api/routers and export them from the API root
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return SuperJSON-serializable data (plain objects/arrays) from tRPC procedures
Files:
apps/web/client/src/server/api/routers/usage/index.ts
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer imports using @/* or ~/* aliases instead of deep relative paths within the client app
Files:
apps/web/client/src/server/api/routers/usage/index.tsapps/web/client/src/components/store/editor/chat/message.tsapps/web/client/src/components/store/editor/chat/conversation.tsapps/web/client/src/components/store/editor/chat/index.tsapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/app/api/chat/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the 'any' type unless necessary
Files:
apps/web/client/src/server/api/routers/usage/index.tsapps/web/client/src/components/store/editor/chat/message.tspackages/db/src/mappers/chat/message.tsapps/web/client/src/components/store/editor/chat/conversation.tsapps/web/client/src/components/store/editor/chat/index.tsapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxpackages/models/src/chat/message/message.tsapps/web/client/src/app/api/chat/route.ts
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libraries
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use process.env in client code; prefer importing env from @/env
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/app/api/chat/route.ts
apps/web/client/src/app/**/{page,layout,route}.ts?(x)
📄 CodeRabbit inference engine (AGENTS.md)
Follow Next.js App Router structure: page.tsx, layout.tsx, and route.ts under apps/web/client/src/app
Files:
apps/web/client/src/app/api/chat/route.ts
🧬 Code graph analysis (8)
apps/web/client/src/server/api/routers/usage/index.ts (1)
packages/db/src/schema/subscription/usage.ts (1)
usageRecords(8-21)
apps/web/client/src/components/store/editor/chat/message.ts (2)
packages/models/src/chat/message/context.ts (1)
MessageContext(53-59)packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)
packages/db/src/mappers/chat/message.ts (1)
packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)
apps/web/client/src/components/store/editor/chat/conversation.ts (3)
packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)apps/web/client/src/components/store/editor/chat/message.ts (1)
getUserChatMessageFromString(4-20)packages/db/src/mappers/chat/message.ts (1)
toDbMessage(17-34)
apps/web/client/src/components/store/editor/chat/index.ts (4)
packages/models/src/chat/message/context.ts (1)
MessageContext(53-59)packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)packages/git/src/git.ts (1)
GitCommit(19-25)apps/web/client/src/trpc/react.tsx (1)
api(23-23)
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (4)
packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)apps/web/client/src/components/store/editor/chat/context.ts (1)
ChatContext(16-291)apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)packages/db/src/schema/chat/message.ts (1)
messages(11-27)
packages/models/src/chat/message/message.ts (3)
packages/models/src/chat/message/context.ts (1)
MessageContext(53-59)packages/models/src/chat/message/checkpoint.ts (1)
MessageCheckpoints(15-15)packages/ai/src/tools/toolset.ts (1)
ChatTools(67-67)
apps/web/client/src/app/api/chat/route.ts (5)
packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)apps/web/client/src/app/api/chat/helpers/message.ts (2)
debouncedUpsertMessage(34-34)loadChat(36-42)packages/db/src/schema/chat/message.ts (1)
messages(11-27)apps/web/client/src/app/api/chat/helpers/usage.ts (1)
incrementUsage(43-65)apps/web/client/src/app/api/chat/helpers/stream.ts (1)
errorHandler(89-108)
🔇 Additional comments (2)
apps/web/client/src/components/store/editor/chat/index.ts (1)
93-102: No change needed — getProjectContext is synchronous. getProjectContext(): ProjectMessageContext[] in apps/web/client/src/components/store/editor/chat/context.ts (line 169). Spreading its return is safe; no await required.apps/web/client/src/app/api/chat/route.ts (1)
66-78: Ensure debouncedUpsertMessage is awaitable (returns a Promise). If the debounce implementation (e.g. lodash/debounce) does not forward/return the wrapped async function’s Promise, awaiting debouncedUpsertMessage(...) is ineffective — use a promise-aware debounce (p-debounce or similar) or call upsertMessage directly.
File: apps/web/client/src/app/api/chat/helpers/message.ts (debouncedUpsertMessage = debounce(upsertMessage, 500)).
| // create or update last message in database | ||
| await debouncedUpsertMessage({ id: messageId, conversationId, message }); | ||
| const messages = await loadChat(conversationId); | ||
|
|
There was a problem hiding this comment.
Race: debounced upsert before load can omit the latest user message from context.
Debounced functions often don’t await the inner call; loadChat may run before persistence, so the streamed context can miss the last message. Upsert immediately and/or append the in-memory message to the streamed messages.
-import { debouncedUpsertMessage } from './helpers/message';
+import { upsertMessage, debouncedUpsertMessage } from './helpers/message';
@@
- // create or update last message in database
- await debouncedUpsertMessage({ id: messageId, conversationId, message });
- const messages = await loadChat(conversationId);
+ // upsert immediately to avoid missing context, and include the in-memory message defensively
+ await upsertMessage({ id: messageId, conversationId, message: { ...message, metadata: { ...(message.metadata ?? {}), conversationId } } });
+ const messages = [...await loadChat(conversationId), message];📝 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.
| // create or update last message in database | |
| await debouncedUpsertMessage({ id: messageId, conversationId, message }); | |
| const messages = await loadChat(conversationId); | |
| import { upsertMessage, debouncedUpsertMessage } from './helpers/message'; | |
| // upsert immediately to avoid missing context, and include the in-memory message defensively | |
| await upsertMessage({ id: messageId, conversationId, message: { ...message, metadata: { ...(message.metadata ?? {}), conversationId } } }); | |
| const messages = [...await loadChat(conversationId), message]; |
🤖 Prompt for AI Agents
In apps/web/client/src/app/api/chat/route.ts around lines 75-78, the code uses a
debounced upsert before loading messages which can allow loadChat to run before
persistence and omit the latest user message; replace the debounced call with an
immediate awaited upsert (or call the underlying non-debounced upsert function
and await it) so persistence completes before loadChat, and as a secondary
safety, after loading messages check whether the in-memory message is present
and, if not, append it to the messages array returned to the stream.
| import { DefaultChatTransport, lastAssistantMessageIsCompleteWithToolCalls, type UIMessage } from 'ai'; | ||
| import { ChatType, type ChatMessage } from '@onlook/models'; | ||
| import { jsonClone } from '@onlook/utility'; | ||
| import { DefaultChatTransport, lastAssistantMessageIsCompleteWithToolCalls } from 'ai'; |
There was a problem hiding this comment.
Fix types for onFinish; import FinishReason.
You read finishReason but the callback param type omits it.
-import { DefaultChatTransport, lastAssistantMessageIsCompleteWithToolCalls } from 'ai';
+import { DefaultChatTransport, lastAssistantMessageIsCompleteWithToolCalls, type FinishReason } from '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 { DefaultChatTransport, lastAssistantMessageIsCompleteWithToolCalls } from 'ai'; | |
| import { DefaultChatTransport, lastAssistantMessageIsCompleteWithToolCalls, type FinishReason } from 'ai'; |
🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx around line 8, the
onFinish callback type omits the finishReason while the code reads finishReason;
import FinishReason from 'ai' and update the onFinish parameter type to include
finishReason: FinishReason (or FinishReason | undefined if it can be missing).
Ensure the Import list adds FinishReason alongside DefaultChatTransport and
lastAssistantMessageIsCompleteWithToolCalls, and adjust any call sites or type
annotations to accept the new finishReason property.
| const chat = useChat({ | ||
| id: 'user-chat', | ||
| const chat = useChat<ChatMessage>({ | ||
| id: conversationId, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify useChat id handling when conversationId is undefined.
Passing id: conversationId may be undefined on first render; confirm AI SDK behavior and server expectations, or provide a stable fallback id.
🏁 Script executed:
#!/bin/bash
# Inspect server handlers to confirm conversationId is optional and handled.
rg -nC2 -g 'apps/**' -P "api/chat|conversationId|z\.object\(\s*\{[^}]*conversationId" || trueLength of output: 19347
Guard against undefined conversationId before passing it to useChat/send.
editorEngine.chat.conversation.current?.conversation.id can be undefined on first render while server APIs expect a conversationId string (app/api/chat/route.ts; server/api/routers/chat/message.ts, suggestion.ts, conversation.ts). Do not pass id: conversationId directly — either (A) delay creating the chat transport / sending messages until conversationId is defined (apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx:24), or (B) generate a stable temporary id and update server handlers to accept/translate it.
🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx around line 24, the
code passes id: conversationId directly even though
editorEngine.chat.conversation.current?.conversation.id can be undefined on
first render; change the hook to guard against undefined by delaying creation of
the chat transport and any send calls until conversationId is a non-empty string
(e.g., return early, set up transport inside an effect that runs when
conversationId becomes defined, and expose a loading/ready flag), or
alternatively implement a stable temporary id flow and update server handlers to
accept/translate temporary ids — pick option A (delay until defined) unless you
also update server code.
| onFinish: (options: { message: ChatMessage, }) => { | ||
| const { message } = options; | ||
| console.log('onFinish', options); | ||
| if (!message.metadata) { | ||
| return; | ||
| } | ||
| const finishReason = (message.metadata as { finishReason?: string }).finishReason; | ||
| const finishReason = options.finishReason; | ||
| lastMessageRef.current = message; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Type the onFinish options to include finishReason.
Prevents TS error and matches AI SDK.
- onFinish: (options: { message: ChatMessage, }) => {
+ onFinish: (options: { message: ChatMessage, finishReason: FinishReason }) => {
const { message } = options;
- console.log('onFinish', options);
if (!message.metadata) {
return;
}
const finishReason = options.finishReason;📝 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.
| onFinish: (options: { message: ChatMessage, }) => { | |
| const { message } = options; | |
| console.log('onFinish', options); | |
| if (!message.metadata) { | |
| return; | |
| } | |
| const finishReason = (message.metadata as { finishReason?: string }).finishReason; | |
| const finishReason = options.finishReason; | |
| lastMessageRef.current = message; | |
| onFinish: (options: { message: ChatMessage, finishReason: FinishReason }) => { | |
| const { message } = options; | |
| if (!message.metadata) { | |
| return; | |
| } | |
| const finishReason = options.finishReason; | |
| lastMessageRef.current = message; |
🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx around lines 52 to
59, the onFinish callback options are currently typed as { message: ChatMessage
} but the code reads options.finishReason; update the options type to include
finishReason (e.g. { message: ChatMessage; finishReason?: string } or the
appropriate SDK enum/type) and adjust the function signature and destructuring
accordingly so TypeScript recognizes finishReason and the code compiles without
errors.
| async resubmitMessage(id: string, newMessageContent: string): Promise<ChatMessage | null> { | ||
| if (!this.conversation.current?.conversation.id) { | ||
| console.error('No conversation found'); | ||
| return null; | ||
| } | ||
| const oldMessageIndex = this.conversation.current?.messages.findIndex((m) => m.id === id && m.role === 'user'); | ||
| if (oldMessageIndex === undefined || oldMessageIndex === -1 || !this.conversation.current?.messages[oldMessageIndex]) { | ||
| console.error('No message found with id', id); | ||
| return null; | ||
| } | ||
|
|
||
| const oldMessage = this.conversation.current?.messages[oldMessageIndex] as UserChatMessage; | ||
| const oldMessage = this.conversation.current?.messages[oldMessageIndex]; | ||
|
|
||
| // Update the old message with the new content | ||
| const newContext = await this.context.getRefreshedContext(oldMessage.metadata.context); | ||
| oldMessage.metadata.context = newContext; | ||
| const newContext = await this.context.getRefreshedContext(oldMessage.metadata?.context ?? []); | ||
| oldMessage.metadata = { | ||
| ...oldMessage.metadata, | ||
| context: newContext, | ||
| createdAt: oldMessage.metadata?.createdAt || new Date(), | ||
| conversationId: oldMessage.metadata?.conversationId || this.conversation.current?.conversation.id, | ||
| checkpoints: oldMessage.metadata?.checkpoints ?? [], | ||
| }; | ||
| oldMessage.parts = [{ type: 'text', text: newMessageContent }]; | ||
|
|
||
| // Remove all messages after the old message | ||
| const messagesToRemove = this.conversation.current?.messages.filter((m) => m.createdAt > oldMessage.createdAt); | ||
| const messagesToRemove = this.conversation.current?.messages.filter((m) => m.metadata?.createdAt && m.metadata.createdAt > (oldMessage.metadata?.createdAt ?? new Date())); | ||
| await this.conversation.removeMessages(messagesToRemove); | ||
| return oldMessage; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use index-based removal to avoid Date serialization pitfalls.
Relying on createdAt comparisons can fail if some entries are strings. Slice by index instead.
- const messagesToRemove = this.conversation.current?.messages.filter((m) => m.metadata?.createdAt && m.metadata.createdAt > (oldMessage.metadata?.createdAt ?? new Date()));
- await this.conversation.removeMessages(messagesToRemove);
+ const messagesToRemove = this.conversation.current.messages.slice(oldMessageIndex + 1);
+ await this.conversation.removeMessages(messagesToRemove);📝 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.
| async resubmitMessage(id: string, newMessageContent: string): Promise<ChatMessage | null> { | |
| if (!this.conversation.current?.conversation.id) { | |
| console.error('No conversation found'); | |
| return null; | |
| } | |
| const oldMessageIndex = this.conversation.current?.messages.findIndex((m) => m.id === id && m.role === 'user'); | |
| if (oldMessageIndex === undefined || oldMessageIndex === -1 || !this.conversation.current?.messages[oldMessageIndex]) { | |
| console.error('No message found with id', id); | |
| return null; | |
| } | |
| const oldMessage = this.conversation.current?.messages[oldMessageIndex] as UserChatMessage; | |
| const oldMessage = this.conversation.current?.messages[oldMessageIndex]; | |
| // Update the old message with the new content | |
| const newContext = await this.context.getRefreshedContext(oldMessage.metadata.context); | |
| oldMessage.metadata.context = newContext; | |
| const newContext = await this.context.getRefreshedContext(oldMessage.metadata?.context ?? []); | |
| oldMessage.metadata = { | |
| ...oldMessage.metadata, | |
| context: newContext, | |
| createdAt: oldMessage.metadata?.createdAt || new Date(), | |
| conversationId: oldMessage.metadata?.conversationId || this.conversation.current?.conversation.id, | |
| checkpoints: oldMessage.metadata?.checkpoints ?? [], | |
| }; | |
| oldMessage.parts = [{ type: 'text', text: newMessageContent }]; | |
| // Remove all messages after the old message | |
| const messagesToRemove = this.conversation.current?.messages.filter((m) => m.createdAt > oldMessage.createdAt); | |
| const messagesToRemove = this.conversation.current?.messages.filter((m) => m.metadata?.createdAt && m.metadata.createdAt > (oldMessage.metadata?.createdAt ?? new Date())); | |
| await this.conversation.removeMessages(messagesToRemove); | |
| return oldMessage; | |
| } | |
| async resubmitMessage(id: string, newMessageContent: string): Promise<ChatMessage | null> { | |
| if (!this.conversation.current?.conversation.id) { | |
| console.error('No conversation found'); | |
| return null; | |
| } | |
| const oldMessageIndex = this.conversation.current?.messages.findIndex((m) => m.id === id && m.role === 'user'); | |
| if (oldMessageIndex === undefined || oldMessageIndex === -1 || !this.conversation.current?.messages[oldMessageIndex]) { | |
| console.error('No message found with id', id); | |
| return null; | |
| } | |
| const oldMessage = this.conversation.current?.messages[oldMessageIndex]; | |
| // Update the old message with the new content | |
| const newContext = await this.context.getRefreshedContext(oldMessage.metadata?.context ?? []); | |
| oldMessage.metadata = { | |
| ...oldMessage.metadata, | |
| context: newContext, | |
| createdAt: oldMessage.metadata?.createdAt || new Date(), | |
| conversationId: oldMessage.metadata?.conversationId || this.conversation.current?.conversation.id, | |
| checkpoints: oldMessage.metadata?.checkpoints ?? [], | |
| }; | |
| oldMessage.parts = [{ type: 'text', text: newMessageContent }]; | |
| // Remove all messages after the old message | |
| const messagesToRemove = this.conversation.current.messages.slice(oldMessageIndex + 1); | |
| await this.conversation.removeMessages(messagesToRemove); | |
| return oldMessage; | |
| } |
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/editor/chat/index.ts around lines
105-133, the removal logic compares createdAt dates (which may be strings)
causing brittle behavior; instead compute the old message index once, validate
it, and remove messages by index: get const oldIndex =
this.conversation.current!.messages.findIndex(...); if oldIndex < 0 return null;
then build messagesToRemove = this.conversation.current!.messages.slice(oldIndex
+ 1) and call await this.conversation.removeMessages(messagesToRemove). Ensure
you stop using createdAt comparisons for slicing and use the existing oldIndex
for deterministic removal.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (8)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (1)
1-1: This component uses client-only hooks; add "use client".The component imports MobX
observerand next-intluseTranslationswhich require client-side execution.+'use client'; + import { useChatContext } from '@/app/project/[id]/_hooks/use-chat';apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (2)
61-66: Type the onFinish options parameter correctly.The
onFinishcallback receives additional properties beyond justmessage. Import and use the proper type from the AI SDK.+import { type FinishReason } from 'ai'; - onFinish: (options: { message: ChatMessage; isAbort?: boolean; isDisconnect?: boolean; isError?: boolean }) => { + onFinish: (options: { message: ChatMessage; finishReason?: FinishReason; isAbort?: boolean; isDisconnect?: boolean; isError?: boolean }) => {
26-29: Handle undefined conversationId gracefully.The
conversationIdcan be undefined on first render, which may cause issues with the chat transport.Consider delaying initialization or providing a fallback:
const conversationId = editorEngine.chat.conversation.current?.conversation.id; + + // Skip chat initialization if no conversation + if (!conversationId) { + return <ChatContext.Provider value={null}>{children}</ChatContext.Provider>; + }apps/web/client/src/app/api/chat/helpers/message.ts (1)
40-40: Debounced async function may cause race conditions.Using lodash's debounce with an async function doesn't properly handle promise results, which could lead to race conditions or missed DB updates. The debounced function returns immediately without waiting for the promise to resolve.
Consider these alternatives:
- Use an async-aware debounce library like
p-debounce- Implement manual flush with proper promise handling
- Use a queue-based approach for message persistence
-import debounce from "lodash/debounce"; +import pDebounce from "p-debounce"; -export const debouncedUpsertMessage = debounce(upsertMessage, 500); +export const debouncedUpsertMessage = pDebounce(upsertMessage, 500);apps/web/client/src/app/api/chat/route.ts (4)
66-72: Validate request body schema.The request body is destructured without validation, which could cause runtime errors if the structure doesn't match expectations.
+import { z } from 'zod'; + +const requestBodySchema = z.object({ + message: z.object({ + id: z.string(), + role: z.enum(['user', 'assistant', 'system']), + parts: z.array(z.any()), + metadata: z.any().optional(), + }), + chatType: z.nativeEnum(ChatType), + conversationId: z.string().uuid(), + projectId: z.string().uuid(), + traceId: z.string(), +}); try { - const { message, chatType, conversationId, projectId, traceId }: { - message: ChatMessage, - chatType: ChatType, - conversationId: string, - projectId: string, - traceId: string, - } = await req.json() + const body = await req.json(); + const { message, chatType, conversationId, projectId, traceId } = requestBodySchema.parse(body);
75-76: Race condition: debounced upsert before loadChat.The debounced upsert may not complete before
loadChatruns, causing the loaded messages to miss the latest user message. This could lead to incomplete context in the AI stream.+import { upsertMessage } from './helpers/message'; - // create or update last message in database - await debouncedUpsertMessage({ id: message.id, conversationId, message }); + // Use immediate upsert to ensure persistence before loading + await upsertMessage({ id: message.id, conversationId, message }); const messages = await loadChat(conversationId);
139-144: Avoid debounce for final assistant message.Using debounced persistence for the final message could result in data loss if the process terminates or if multiple requests override each other.
+import { upsertMessage } from './helpers/message'; onFinish: async (message) => { - await debouncedUpsertMessage({ + await upsertMessage({ id: message.responseMessage.id, conversationId, message: message.responseMessage, }); },
75-76: Authorize conversation access.Add authorization checks to ensure the user has access to the specified conversation before performing operations.
#!/bin/bash # Check if there are existing authorization patterns in the codebase rg -n "conversationId.*userId|authorize.*conversation" --type ts apps/webAdd authorization before message operations:
// Verify user owns the conversation const hasAccess = await verifyConversationAccess(userId, conversationId, projectId); if (!hasAccess) { return new Response(JSON.stringify({ error: 'Unauthorized access to conversation' }), { status: 403, headers: { 'Content-Type': 'application/json' } }); }
🧹 Nitpick comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (1)
22-22: Potential undefined dereference in filter operation.When
engineMessagesis undefined, the filter will fail. Add a nullish coalescing operator to handle this case safely.- const messagesToRender = useMemo(() => isWaiting ? engineMessages?.filter((m) => m.id !== streamingMessage?.id) : engineMessages, [engineMessages, isWaiting]); + const messagesToRender = useMemo(() => isWaiting ? (engineMessages ?? []).filter((m) => m.id !== streamingMessage?.id) : engineMessages, [engineMessages, isWaiting, streamingMessage]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/client/src/app/api/chat/helpers/message.ts(1 hunks)apps/web/client/src/app/api/chat/route.ts(4 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx(3 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/reasoning.tsx(1 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/reasoning.tsx
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Files:
apps/web/client/src/app/api/chat/helpers/message.tsapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/api/chat/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/api/chat/helpers/message.tsapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/api/chat/route.ts
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env instead
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-14T01:44:21.199Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.199Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Applied to files:
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
📚 Learning: 2025-09-14T01:44:21.199Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.199Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Applied to files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
🧬 Code graph analysis (4)
apps/web/client/src/app/api/chat/helpers/message.ts (3)
packages/db/src/mappers/chat/message.ts (2)
toDbMessage(17-34)fromDbMessage(4-15)packages/db/src/client.ts (1)
db(16-16)packages/db/src/schema/chat/message.ts (1)
messages(11-27)
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (4)
packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)apps/web/client/src/components/store/editor/chat/context.ts (1)
ChatContext(16-291)apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)apps/web/client/src/components/tools/tools.ts (1)
handleToolCall(179-206)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (4)
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (1)
useChatContext(100-105)packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsx (1)
AssistantMessage(4-15)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (1)
UserMessage(24-253)
apps/web/client/src/app/api/chat/route.ts (4)
packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)apps/web/client/src/app/api/chat/helpers/message.ts (2)
debouncedUpsertMessage(40-40)loadChat(42-48)packages/db/src/schema/chat/message.ts (1)
messages(11-27)apps/web/client/src/app/api/chat/helpers/stream.ts (1)
errorHandler(89-108)
🔇 Additional comments (4)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (1)
33-35: System messages filtered correctly.The implementation properly handles system messages by setting
messageNode = null, which prevents rendering as intended per the PR objectives.apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (2)
25-26: Eliminate potential race condition with refs.Using refs instead of state for
chatTypeandtraceIdis a good approach to avoid stale closures.Also applies to: 37-37, 71-72
85-95: Efficient streaming message computation.The
useMemohook correctly computes the streaming message and updateslastMessageReffor error recovery. The implementation properly handles the edge cases.apps/web/client/src/app/api/chat/helpers/message.ts (1)
18-22: Verify cascade deletion behavior (time-based).Deleting messages with createdAt > dbMessage?.createdAt can remove legitimate messages when timestamps are very close or system clocks drift — createdAt is set on create (apps/web/client/src/app/api/chat/route.ts:132) and used for ordering (apps/web/client/src/app/api/chat/helpers/message.ts:45).
- Prefer a monotonic sequence or message-order field (or use message ID as a deterministic tiebreaker) instead of relying solely on timestamps.
- If timestamps must be used, add a small buffer/tiebreaker (e.g., include id or use >= with explicit tie-break) and add tests for near-simultaneous inserts and clock skew.
packages/ai/src/stream/index.ts
Outdated
| return parts.filter((part) => part.type !== 'reasoning'); | ||
| }; | ||
|
|
||
| export const ensureToolResultParts = (parts: ChatMessage['parts']): ChatMessage['parts'] => {}; |
There was a problem hiding this comment.
The function ensureToolResultParts is currently an empty stub. Either implement its functionality or remove it if not needed.
| export const ensureToolResultParts = (parts: ChatMessage['parts']): ChatMessage['parts'] => {}; |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (5)
apps/web/client/src/app/api/chat/route.ts (4)
84-87: Good fix: upsert before loading historyDebounce removed; persistence happens before load preventing context gaps.
129-147: Good fix: persist final assistant message immediatelyDropping debounce here prevents lost final writes.
76-83: Validate request body (schema)Body is parsed without validation; enforce shape (message, chatType, conversationId, projectId, traceId).
+import { z } from 'zod'; @@ - const { message, chatType, conversationId, projectId, traceId }: { - message: ChatMessage, - chatType: ChatType, - conversationId: string, - projectId: string, - traceId: string, - } = await req.json() + const Body = z.object({ + message: z.any(), // TODO: refine to runtime ChatMessage + chatType: z.nativeEnum(ChatType), + conversationId: z.string().uuid(), + projectId: z.string().uuid(), + traceId: z.string().min(1), + }); + const { message, chatType, conversationId, projectId, traceId } = Body.parse(await req.json());
84-87: Authorize conversation and restrict message.roleVerify the conversation belongs to userId/projectId and only accept user‑role inputs on this route.
+ // Enforce ownership/visibility before persistence + await assertUserOwnsConversation({ userId, projectId, conversationId }); + + // Only users can initiate a streamed turn + if (message.role !== 'user') { + return new Response(JSON.stringify({ error: 'Invalid message role' }), { status: 400 }); + }If helper doesn’t exist, I can provide an implementation against your DB schema.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (1)
1-1: Add "use client"This file uses client‑only hooks (observer, useTranslations, context). It must be a Client Component.
+'use client';
🧹 Nitpick comments (7)
apps/web/client/src/app/api/chat/helpers/message.ts (1)
2-2: Prefer package barrel import over deep importImport db from @onlook/db to keep package boundaries stable.
-import { db } from "@onlook/db/src/client"; +import { db } from "@onlook/db";apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsx (2)
35-36: lastToolInvocationIdx calculation is wrongUsing parts.length - 1 marks the last part, not the last tool part. This breaks the “loading” state for tool calls.
- lastToolInvocationIdx={parts.length - 1} + lastToolInvocationIdx={parts.reduce((acc, p, i) => p.type.startsWith('tool-') ? i : acc, -1)}
23-23: Use a stable, collision‑free keypart.text can duplicate and cause key collisions; prefer messageId+index.
- key={part.text} + key={`${messageId}-text-${idx}`}apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (3)
20-23: Localize user‑facing text“Loading conversation...” should use next‑intl per guidelines.
- <p className="text-regularPlus">Loading conversation...</p> + <p className="text-regularPlus">{useTranslations()(transKeys.editor.panels.edit.tabs.chat.loading)}</p>
27-33: contentKey should key the list to the conversationAn empty key disables list resets. Use conversation.id.
- <ChatMessageList contentKey={``}> + <ChatMessageList contentKey={conversation.id}>
59-76: Avoid rendering empty wrappers for system messagesReturning a div with null content creates gaps. Skip rendering instead.
- return <div key={`message-${message.id}-${index}`}>{messageNode}</div>; + return messageNode ? <div key={`message-${message.id}-${index}`}>{messageNode}</div> : null;apps/web/client/src/app/api/chat/route.ts (1)
107-108: Optional: normalize tool parts before converting to model messagesIf you plan to use ensureToolResultParts, apply it before convertToStreamMessages.
- ...convertToStreamMessages(messages), + ...convertToStreamMessages(messages /* optionally map( m => ({ ...m, parts: ensureToolResultParts(m.parts) }) */),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/client/src/app/api/chat/helpers/message.ts(1 hunks)apps/web/client/src/app/api/chat/route.ts(5 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx(3 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/markdown/index.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsx(1 hunks)packages/ai/src/prompt/provider.ts(0 hunks)packages/ai/src/stream/index.ts(3 hunks)
💤 Files with no reviewable changes (1)
- packages/ai/src/prompt/provider.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/markdown/index.tsx
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Files:
apps/web/client/src/app/api/chat/helpers/message.tsapps/web/client/src/app/api/chat/route.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/api/chat/helpers/message.tspackages/ai/src/stream/index.tsapps/web/client/src/app/api/chat/route.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env instead
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsxapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-14T01:44:21.199Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.199Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Applied to files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
📚 Learning: 2025-09-14T01:44:21.199Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.199Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Applied to files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
🧬 Code graph analysis (5)
apps/web/client/src/app/api/chat/helpers/message.ts (4)
packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)packages/db/src/mappers/chat/message.ts (2)
toDbMessage(17-34)fromDbMessage(4-15)packages/db/src/client.ts (1)
db(16-16)packages/db/src/schema/chat/message.ts (1)
messages(11-27)
packages/ai/src/stream/index.ts (3)
packages/db/src/schema/chat/message.ts (1)
messages(11-27)packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)packages/ai/src/prompt/provider.ts (1)
getHydratedUserMessage(51-116)
apps/web/client/src/app/api/chat/route.ts (5)
packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)apps/web/client/src/app/api/chat/helpers/message.ts (2)
upsertMessage(6-37)loadChat(39-45)packages/db/src/schema/chat/message.ts (1)
messages(11-27)packages/ai/src/stream/index.ts (1)
convertToStreamMessages(5-23)apps/web/client/src/app/api/chat/helpers/stream.ts (1)
errorHandler(89-108)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsx (3)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/markdown/index.tsx (1)
MarkdownRenderer(6-45)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsx (1)
ToolCallDisplay(26-238)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/reasoning.tsx (1)
ReasoningBlock(4-20)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (7)
packages/ui/src/components/chat/chat-message-list.tsx (1)
ChatMessageList(53-53)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsx (1)
StreamMessage(5-27)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/error-message.tsx (1)
ErrorMessage(8-50)apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (1)
useChatContext(100-105)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsx (1)
AssistantMessage(4-15)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (1)
UserMessage(24-253)
🔇 Additional comments (1)
packages/ai/src/stream/index.ts (1)
7-10: Runtime support for findLastIndexArray.prototype.findLastIndex requires modern runtimes (Node 18+/modern browsers). Confirm your targets or polyfill.
Do we guarantee Node >= 18 and modern browsers across all bundles?
| const dbMessage = toDbMessage(message, conversationId); | ||
| return await db.transaction(async (tx) => { | ||
| // Remove messages newer than the updated message | ||
| await tx.delete(messages).where(and( | ||
| eq(messages.conversationId, conversationId), | ||
| gt(messages.createdAt, dbMessage?.createdAt ?? new Date()), | ||
| )); | ||
| const [updatedMessage] = await tx | ||
| .insert(messages) | ||
| .values({ | ||
| ...dbMessage, | ||
| id, | ||
| }) | ||
| .onConflictDoUpdate({ | ||
| target: [messages.id], | ||
| set: { | ||
| ...dbMessage, | ||
| id, | ||
| }, | ||
| }).returning(); | ||
| return updatedMessage; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return domain type for consistency
upsertMessage returns a raw DB row; loadChat returns ChatMessage. Standardize on ChatMessage.
-export const upsertMessage = async ({
+export const upsertMessage = async ({
@@
-}) => {
+}): Promise<ChatMessage> => {
@@
- const [updatedMessage] = await tx
+ const [updatedMessage] = await tx
.insert(messages)
@@
- }).returning();
- return updatedMessage;
+ }).returning();
+ return fromDbMessage(updatedMessage);📝 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 dbMessage = toDbMessage(message, conversationId); | |
| return await db.transaction(async (tx) => { | |
| // Remove messages newer than the updated message | |
| await tx.delete(messages).where(and( | |
| eq(messages.conversationId, conversationId), | |
| gt(messages.createdAt, dbMessage?.createdAt ?? new Date()), | |
| )); | |
| const [updatedMessage] = await tx | |
| .insert(messages) | |
| .values({ | |
| ...dbMessage, | |
| id, | |
| }) | |
| .onConflictDoUpdate({ | |
| target: [messages.id], | |
| set: { | |
| ...dbMessage, | |
| id, | |
| }, | |
| }).returning(); | |
| return updatedMessage; | |
| }); | |
| export const upsertMessage = async ({ | |
| message, | |
| conversationId, | |
| id, | |
| }): Promise<ChatMessage> => { | |
| const dbMessage = toDbMessage(message, conversationId); | |
| return await db.transaction(async (tx) => { | |
| // Remove messages newer than the updated message | |
| await tx.delete(messages).where(and( | |
| eq(messages.conversationId, conversationId), | |
| gt(messages.createdAt, dbMessage?.createdAt ?? new Date()), | |
| )); | |
| const [updatedMessage] = await tx | |
| .insert(messages) | |
| .values({ | |
| ...dbMessage, | |
| id, | |
| }) | |
| .onConflictDoUpdate({ | |
| target: [messages.id], | |
| set: { | |
| ...dbMessage, | |
| id, | |
| }, | |
| }).returning(); | |
| return fromDbMessage(updatedMessage); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In apps/web/client/src/app/api/chat/helpers/message.ts around lines 15 to 36,
the function currently returns a raw DB row; change it to return the domain
ChatMessage: after receiving updatedMessage from the transaction, map it to the
domain type (use the existing mapper function such as
fromDbMessage/toChatMessage—or add one if missing) and return that mapped
ChatMessage; update the function signature to Promise<ChatMessage>, add the
required import for the mapper and types, and ensure any callers are updated to
expect the domain type.
| .onConflictDoUpdate({ | ||
| target: [messages.id], | ||
| set: { | ||
| ...dbMessage, | ||
| id, | ||
| }, | ||
| }).returning(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid updating primary key on conflict
Setting id in the UPDATE clause is redundant and can cause unnecessary write churn.
.onConflictDoUpdate({
target: [messages.id],
set: {
- ...dbMessage,
- id,
+ ...dbMessage,
},
})📝 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.
| .onConflictDoUpdate({ | |
| target: [messages.id], | |
| set: { | |
| ...dbMessage, | |
| id, | |
| }, | |
| }).returning(); | |
| .onConflictDoUpdate({ | |
| target: [messages.id], | |
| set: { | |
| ...dbMessage, | |
| }, | |
| }).returning(); |
🤖 Prompt for AI Agents
In apps/web/client/src/app/api/chat/helpers/message.ts around lines 28 to 34,
the ON CONFLICT DO UPDATE currently includes id in the SET clause which updates
the primary key (redundant and causes write churn); remove id from the update
payload by excluding it from the object passed to set (e.g., spread dbMessage
but omit id or build a new object of fields to update), keep the conflict target
as messages.id and return the updated row as before.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/ai/src/stream/index.ts (1)
29-39: Normalize tool parts after filtering (apply in both branches).Wire ensureToolResultParts after filterReasoningParts so the final parts shape is consistent for both assistant and user messages.
- const parts = filterReasoningParts(message.parts); + const filtered = filterReasoningParts(message.parts); + const parts = ensureToolResultParts(filtered);Also applies to: 47-49
apps/web/client/src/app/api/chat/route.ts (2)
76-83: Validate request body (schema).Parse and reject malformed input early; reduces 500s and abuse surface.
-import { +import { checkMessageLimit, decrementUsage, errorHandler, getModelFromType, getSupabaseUser, getSystemPromptFromType, incrementUsage, loadChat, repairToolCall, upsertMessage } from './helpers'; +import { z } from 'zod'; @@ - const { message, chatType, conversationId, projectId, traceId }: { - message: ChatMessage, - chatType: ChatType, - conversationId: string, - projectId: string, - traceId: string, - } = await req.json() + const Body = z.object({ + message: z.object({ + id: z.string().uuid(), + role: z.enum(['user','assistant','system']), + parts: z.array(z.any()), + metadata: z.any().optional(), + }), + chatType: z.nativeEnum(ChatType), + conversationId: z.string().uuid(), + projectId: z.string().uuid(), + traceId: z.string().min(1), + }); + const { message, chatType, conversationId, projectId, traceId } = Body.parse(await req.json());
84-86: Authorize conversation and enforce server-side conversationId.Verify ownership before upsert; do not trust client-sent IDs.
- // create or update last message in database - await upsertMessage({ id: message.id, conversationId, message }); + // TODO: assert the user owns this conversationId/projectId pair (403 on failure). + // await assertUserOwnsConversation({ userId, projectId, conversationId }); + // create or update last message in database (force server conversationId) + await upsertMessage({ + id: message.id, + conversationId, + message: { ...message, metadata: { ...(message.metadata ?? {}), conversationId } as any }, + });apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (1)
1-1: Add "use client".This component uses client-only hooks (MobX, next-intl). Must be a Client Component.
+'use client';
🧹 Nitpick comments (6)
packages/ai/src/tools/tools/read.ts (3)
11-12: Clarify absolute vs. relative path semanticsExplicitly state whether relative paths are supported and what they’re relative to for consistency with
list_files’ param docs.Apply this diff if accurate:
- 'The path to the file to read. Supports fuzzy path matching if exact path is not found.', + 'The path to the file to read. Can be absolute or relative (relative to the working directory). Supports fuzzy path matching if the exact path is not found.',
58-61: Restore key capabilities in tool description for parityMention absolute/relative and fuzzy matching in the top-level description so UIs that only surface this text remain clear.
- 'List files and directories in a specified path. Can filter by type and exclude patterns. Returns file paths with type information (file/directory). Only lists immediate children (non-recursive).', + 'List files and directories in a specified path. Supports absolute and relative paths with fuzzy matching when exact paths are not found. Can filter by type and exclude patterns. Returns file paths with type information (file/directory). Only lists immediate children (non-recursive).',
13-24: Validate offset/limit and remove ambiguity (1‑based vs 0‑based)Docs say “cat -n” (1‑based). Enforce integers and bounds to match behavior and prevent large reads.
- offset: z - .number() + offset: z + .number().int().min(1) .optional() .describe( - 'The line number to start reading from. Only provide if the file is too large to read at once', + 'The 1-based line number to start reading from. Only provide if the file is too large to read at once', ), - limit: z - .number() + limit: z + .number().int().min(1).max(2000) .optional() .describe( - 'The number of lines to read. Only provide if the file is too large to read at once.', + 'The number of lines to read (max 2000). Only provide if the file is too large to read at once.', ),packages/ai/src/stream/index.ts (1)
51-73: Tighten guard; prefer map for immutability.Use a map pass with a precise predicate; avoid index mutation and broadened startsWith checks.
-export const ensureToolResultParts = (parts: ChatMessage['parts']): ChatMessage['parts'] => { - const processedParts = [...parts]; - - for (let i = 0; i < processedParts.length; i++) { - const part = processedParts[i]; - if (!part) continue; - - // Check if this is a tool part that needs completion - if (part.type.startsWith('tool-') && 'toolCallId' in part && 'state' in part) { - // If tool call is in streaming state, mark it as having input available - if (part.state === 'input-streaming') { - const updatedPart = { - ...part, - state: 'input-available' as const, - }; - processedParts[i] = updatedPart; - } - } - } - - return processedParts; -}; +export const ensureToolResultParts = (parts: ChatMessage['parts']): ChatMessage['parts'] => { + return parts.map((part) => { + if ( + part && + 'type' in part && + (part.type === 'tool-call' || part.type === 'tool-result' || part.type === 'tool') && + 'state' in part && + (part as any).state === 'input-streaming' + ) { + return { ...(part as any), state: 'input-available' as const }; + } + return part; + }); +};apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (2)
21-24: Externalize user-facing text to i18n.Replace hardcoded “Loading conversation...”.
- <p className="text-regularPlus">Loading conversation...</p> + {/* + TODO: add a key like transKeys.editor.panels.edit.tabs.chat.loading + */} + <p className="text-regularPlus"> + {useTranslations()(transKeys.editor.panels.edit.tabs.chat.loading)} + </p>
75-76: Avoid index in React key.Use stable id-only key to prevent reorder bugs.
- return <div key={`message-${message.id}-${index}`}>{messageNode}</div>; + return <div key={message.id}>{messageNode}</div>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/client/src/app/api/chat/route.ts(5 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx(3 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/reasoning.tsx(1 hunks)packages/ai/src/stream/index.ts(3 hunks)packages/ai/src/tools/tools/edit.ts(4 hunks)packages/ai/src/tools/tools/read.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/ai/src/tools/tools/edit.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/reasoning.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/ai/src/tools/tools/read.tspackages/ai/src/stream/index.tsapps/web/client/src/app/api/chat/route.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Files:
apps/web/client/src/app/api/chat/route.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env instead
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-14T01:44:21.199Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.199Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Applied to files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
📚 Learning: 2025-09-14T01:44:21.199Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.199Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Applied to files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
🧬 Code graph analysis (3)
packages/ai/src/stream/index.ts (3)
packages/db/src/schema/chat/message.ts (1)
messages(11-27)packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)packages/ai/src/prompt/provider.ts (1)
getHydratedUserMessage(51-116)
apps/web/client/src/app/api/chat/route.ts (5)
packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)apps/web/client/src/app/api/chat/helpers/message.ts (2)
upsertMessage(6-37)loadChat(39-45)packages/db/src/schema/chat/message.ts (1)
messages(11-27)packages/ai/src/stream/index.ts (1)
convertToStreamMessages(5-23)apps/web/client/src/app/api/chat/helpers/stream.ts (1)
errorHandler(89-108)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (6)
apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (1)
useChatContext(100-105)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsx (1)
StreamMessage(5-27)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/error-message.tsx (1)
ErrorMessage(8-50)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsx (1)
AssistantMessage(4-15)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (1)
UserMessage(24-253)
🔇 Additional comments (1)
apps/web/client/src/app/api/chat/route.ts (1)
141-147: LGTM: final response persisted immediately.Immediate awaited upsert removes debounce drop risk.
| <ChatMessageList contentKey={streamingMessage?.parts?.join('') ?? ''}> | ||
| <RenderedChatMessages /> | ||
| <StreamMessage /> | ||
| <ErrorMessage /> | ||
| </ChatMessageList> |
There was a problem hiding this comment.
Fix contentKey: joining objects yields “[object Object]”.
Derive from text parts (or a hash) to ensure meaningful re-renders.
- <ChatMessageList contentKey={streamingMessage?.parts?.join('') ?? ''}>
+ <ChatMessageList
+ contentKey={
+ streamingMessage
+ ? streamingMessage.parts
+ .filter((p) => p.type === 'text')
+ .map((p: any) => p.text)
+ .join('')
+ : ''
+ }
+ >📝 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.
| <ChatMessageList contentKey={streamingMessage?.parts?.join('') ?? ''}> | |
| <RenderedChatMessages /> | |
| <StreamMessage /> | |
| <ErrorMessage /> | |
| </ChatMessageList> | |
| <ChatMessageList | |
| contentKey={ | |
| (streamingMessage?.parts ?? []) | |
| .filter((p) => p?.type === 'text') | |
| .map((p) => p.text ?? '') | |
| .join('') | |
| } | |
| > | |
| <RenderedChatMessages /> | |
| <StreamMessage /> | |
| <ErrorMessage /> | |
| </ChatMessageList> |
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx
around lines 29 to 33, the contentKey is built by joining streamingMessage.parts
directly which yields “[object Object]”; instead derive a stable, meaningful key
from the actual text content (e.g., map parts to their text property and join
those) or compute a short hash of JSON.stringify(parts) if parts may contain
nested objects — replace the current join with parts.map(p => p.text ??
'').join('') or a hash of the serialized parts so React re-renders correctly
when textual content changes.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/db/src/mappers/chat/message.ts (2)
24-24: Guaranteepartsis persisted as an array.Avoid null/undefined writes; align with schema default.
- parts: message.parts, + parts: message.parts ?? [],
5-14: Don’t spread DbMessage into ChatMessage; map explicitly.Spreading pulls DB‑only/deprecated columns onto the UI object and risks TS excess‑property errors. Return a minimal, typed
ChatMessage.export const fromDbMessage = (message: DbMessage): ChatMessage => { - return { - ...message, - metadata: { - conversationId: message.conversationId, - createdAt: message.createdAt, - context: message.context ?? [], - checkpoints: message.checkpoints ?? [], - }, - parts: message.parts ?? [], - } + return { + id: message.id, + role: message.role, + parts: message.parts ?? [], + metadata: { + conversationId: message.conversationId, + createdAt: message.createdAt, + context: message.context ?? [], + checkpoints: message.checkpoints ?? [], + }, + }; }
🧹 Nitpick comments (3)
packages/db/src/mappers/chat/message.ts (3)
18-22: createdAt normalization: confirm type contract.If
ChatMetadata.createdAtisDate | string, the current coercion is fine. If it’s always a string (ISO), consider simplifying and asserting early to avoid invalidDatewrites.
17-23: Defensive check for conversationId mismatch (optional).Guard against accidental cross‑conversation writes when callers pass an inconsistent
conversationId.export const toDbMessage = (message: ChatMessage, conversationId: string): DbMessage => { - const createdAt = message.metadata?.createdAt; + if (message.metadata?.conversationId && message.metadata.conversationId !== conversationId) { + throw new Error("conversationId mismatch between param and message.metadata"); + } + const createdAt = message.metadata?.createdAt;
28-33: Deprecated columns: verify DB constraints; consider omitting writes.If these columns are nullable or have defaults, prefer not writing them to reduce noise. If non‑null, keep as is.
- // deprecated - applied: null, - commitOid: null, - snapshots: null, - content: '', + // deprecated: omit if schema allows (nullable/defaulted)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/db/src/mappers/chat/message.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/db/src/mappers/chat/message.ts
🧬 Code graph analysis (1)
packages/db/src/mappers/chat/message.ts (1)
packages/models/src/chat/message/message.ts (1)
ChatMessage(16-16)
🔇 Additional comments (1)
packages/db/src/mappers/chat/message.ts (1)
2-2: Type‑only import LGTM.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Enhances chat functionality with incremental message saving, refactors toolset and message handling, and improves Markdown rendering.
upsertMessageandloadChatinmessage.tsfor incremental saving and loading of chat messages.streamResponseinroute.tsto use new message handling logic.ChatMessagetype from@onlook/models.ASK_TOOL_SETtoaskToolsandBUILD_TOOL_SETtobuildToolsintoolset.ts.askToolsandbuildToolsbased onChatType.markdown-renderer.tsxwithmarkdown/index.tsxandmarkdown/block.tsxfor improved Markdown handling.message.tsschema to includeChatMessageparts and metadata.conversation.tsandmessage.tsmappers to align with new chat message structure.read.test.ts,web-search.test.ts, andweb.test.tsto reflect toolset changes and validate new logic.This description was created by
for 20742b6. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Enhancements
UX Improvements
Chores