-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: use single onlook message format #2852
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughRefactors chat messaging to a single ChatMessage type across web, AI, DB, and tests. Moves getToolSetFromType to @onlook/ai. Updates chat streaming route to synchronous prompt/tool retrieval, richer telemetry/metadata, and stream conversion. Adjusts DB schema/mappers, prompt text, and adds a remote read option to sandbox with corresponding handler updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Route as API Route (/api/chat)
participant Tools as @onlook/ai Toolset
participant Stream as ai.streamText
participant DB as Usage/Storage
Client->>Route: POST { messages: ChatMessage[], chatType, ... }
Route->>Route: getSystemPromptFromType(chatType) [sync]
Route->>Tools: getToolSetFromType(chatType) [sync]
Route->>Route: convertToStreamMessages(messages)
Route->>DB: usage.increment()
Route->>Stream: streamText({ system, messages, tools, telemetry, generateMessageId })
Stream-->>Route: stream parts (+metadata: createdAt, checkpoints, finishReason)
Route->>Client: SSE stream response
alt error during stream
Route->>DB: usage.decrement(usageRecord)
Route-->>Client: error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (19)
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 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/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsx (1)
1-1: Missing "use client" in a hook-using component.This file uses React hooks and context; it must be a Client Component.
+ 'use client' + import { useChatContext } from '@/app/project/[id]/_hooks/use-chat';apps/web/client/src/components/tools/tools.ts (1)
64-68: Replace any with safer types per repo guidelines.Use
z.AnyZodObjectandunknownto avoidany.-interface ClientToolMap extends Record<string, { - name: string; - inputSchema: z.ZodObject<any>; - handler: (args: any, editorEngine: EditorEngine) => Promise<any>; -}> { } +interface ClientToolMap extends Record<string, { + name: string; + inputSchema: z.AnyZodObject; + handler: (args: unknown, editorEngine: EditorEngine) => Promise<unknown>; +}> { }apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (1)
134-139: Localize all user-facing strings in buttons, tooltips, and toasts.Replace literal text with next-intl keys per guidelines.
- <Button size="sm" variant={'ghost'} onClick={handleCancel}>Cancel</Button> + <Button size="sm" variant={'ghost'} onClick={handleCancel}> + {t(transKeys.common.actions.cancel)} + </Button> - <Button size="sm" variant={'outline'} onClick={handleSubmit}>Submit</Button> + <Button size="sm" variant={'outline'} onClick={handleSubmit}> + {t(transKeys.common.actions.submit)} + </Button> - <TooltipContent side="top" sideOffset={5}>Retry</TooltipContent> + <TooltipContent side="top" sideOffset={5}> + {t(transKeys.editor.chat.actions.retry)} + </TooltipContent> - <TooltipContent side="top" sideOffset={5}>Edit</TooltipContent> + <TooltipContent side="top" sideOffset={5}> + {t(transKeys.common.actions.edit)} + </TooltipContent> - <TooltipContent side="top" sideOffset={5}>Copy</TooltipContent> + <TooltipContent side="top" sideOffset={5}> + {t(transKeys.common.actions.copy)} + </TooltipContent> - toast.error('Failed to resubmit message. Please try again.', { + toast.error(t(transKeys.editor.chat.errors.resubmitFailedTitle), { - description: error instanceof Error ? error.message : 'Unknown error', + description: error instanceof Error ? error.message : t(transKeys.common.errors.unknown), }); - throw new Error('No commit oid found'); + throw new Error(t(transKeys.editor.chat.errors.noCommitOid)); - throw new Error('Failed to get commit'); + throw new Error(t(transKeys.editor.chat.errors.getCommitFailed)); - throw new Error('Failed to checkout commit'); + throw new Error(t(transKeys.editor.chat.errors.checkoutFailed)); - toast.error('Failed to restore checkpoint', { + toast.error(t(transKeys.editor.chat.errors.restoreFailedTitle), { - description: error instanceof Error ? error.message : 'Unknown error', + description: error instanceof Error ? error.message : t(transKeys.common.errors.unknown), }); - {isRestoring ? 'Restoring Checkpoint...' : 'Restore Checkpoint'} + {isRestoring ? t(transKeys.editor.chat.actions.restoringCheckpoint) : t(transKeys.editor.chat.actions.restoreCheckpoint)}Also applies to: 160-161, 176-177, 195-196, 246-247, 90-92, 100-101, 104-109, 112-114
🧹 Nitpick comments (24)
apps/web/client/src/server/api/routers/chat/suggestion.ts (2)
2-2: Avoid deep imports from package internals.
@onlook/ai/src/prompt/suggestis brittle. Prefer a stable, top-level export (e.g.,@onlook/ai), or re-export from the package index.Would you like me to open a follow-up to re-export
SUGGESTION_SYSTEM_PROMPTfrom@onlook/ai?
44-44: Overly large maxOutputTokens (10k).This can spike latency/cost. Clamp to a sane default (e.g., 2048) or make it configurable.
- maxOutputTokens: 10000, + maxOutputTokens: 2048,packages/models/src/chat/message/message.ts (2)
1-35: Remove commented-out legacy types.The dead code at the top obscures the new API. Delete to reduce noise.
-// import type { CodeDiff } from '../../code/index.ts'; -// import type { MessageCheckpoints } from './checkpoint.ts'; -// import type { MessageContext } from './context.ts'; -// import type { UIMessage } from 'ai'; -// -// export enum ChatMessageRole { -// USER = 'user', -// ASSISTANT = 'assistant', -// } -// -// 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; -// } -// -// export type ChatSnapshot = Record<string, CodeDiff>; -// -// export type ChatMessage = UserChatMessage | AssistantChatMessage;
49-52: Broaden ChatDataPart to future-proof tool payloads.
{}excludes primitives and can be awkward. Preferunknownor a record.-export type ChatDataPart = {}; +export type ChatDataPart = unknown; // or: Record<string, unknown>apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsx (2)
31-33: Localize user-facing text with next-intl.Avoid hardcoded strings under app/. Use translations.
-import { MessageContent } from './message-content'; +import { MessageContent } from './message-content'; +import { useTranslations } from 'next-intl'; @@ export const StreamMessage = () => { const { messages, isWaiting } = useChatContext(); + const t = useTranslations('chat'); // ensure 'chat.thinking' exists @@ - <div className="flex w-full h-full flex-row items-center gap-2 px-4 my-2 text-small content-start text-foreground-secondary"> + <div className="flex w-full h-full flex-row items-center gap-2 px-4 my-2 text-small content-start text-foreground-secondary" aria-live="polite"> <Icons.LoadingSpinner className="animate-spin" /> - <p>Thinking ...</p> + <p>{t('thinking')}</p> </div>
9-12: useMemo is unnecessary for a simple boolean.Compute directly to reduce cognitive overhead.
- const isAssistantStreamMessage = useMemo(() => - streamMessage?.role === 'assistant', - [streamMessage?.role] - ); + const isAssistantStreamMessage = streamMessage?.role === 'assistant';apps/web/client/src/components/tools/tools.ts (1)
186-191: Type output as unknown and improve error reporting.Avoid
anyand normalize error messages.-export function handleToolCall(toolCall: ToolCall<string, unknown>, editorEngine: EditorEngine, addToolResult: (toolResult: { tool: string, toolCallId: string, output: any }) => void) { +export function handleToolCall( + toolCall: ToolCall<string, unknown>, + editorEngine: EditorEngine, + addToolResult: (toolResult: { tool: string; toolCallId: string; output: unknown }) => void, +) { @@ - let output: any = null; + let output: unknown = null; @@ - } catch (error) { - output = 'error handling tool call ' + error; + } catch (error) { + const msg = error instanceof Error ? error.message : String(error); + output = `error handling tool call: ${msg}`; } finally { addToolResult({ tool: toolName, toolCallId: toolCall.toolCallId, output: output, });Also applies to: 210-217
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (3)
4-4: Unify model import path.Use the package root (
@onlook/models) for consistency and to avoid deep-entry coupling.- import { type ChatMessage } from '@onlook/models/chat'; + import type { ChatMessage } from '@onlook/models';
55-58: Localize user-facing loading text.Avoid hardcoded strings in app/*; use next-intl.
- <p className="text-regularPlus">Loading conversation...</p> + <p className="text-regularPlus">{t(transKeys.editor.panels.edit.tabs.chat.loading)}</p>
38-38: Use a stable key; drop the index.Index in keys can cause remounts on reorder.
- return <div key={`message-${message.id}-${index}`}>{messageNode}</div>; + return <div key={`message-${message.id}`}>{messageNode}</div>;apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (1)
65-70: Harden clipboard copy with error handling.
navigator.clipboard.writeTextcan throw (permissions/HTTP). Catch and notify.- async function handleCopyClick() { - const text = getUserMessageContent(message); - await navigator.clipboard.writeText(text); - setIsCopied(true); - setTimeout(() => setIsCopied(false), 2000); - } + async function handleCopyClick() { + const text = getUserMessageContent(message); + try { + await navigator.clipboard.writeText(text); + setIsCopied(true); + setTimeout(() => setIsCopied(false), 2000); + } catch (err) { + console.error('Clipboard copy failed', err); + toast.error(t(transKeys.common.errors.copyFailed)); + } + }packages/ai/src/tools/toolset.ts (2)
2-2: Avoid deep import to models.Use the package entrypoint to prevent brittle coupling.
-import { ChatType } from '../../../models/src/chat/type'; +import { ChatType } from '@onlook/models';
55-65: Optionally freeze tool sets to prevent accidental mutation.Defensive immutability for shared maps.
-export const ASK_TOOL_SET: ToolSet = { +export const ASK_TOOL_SET: ToolSet = Object.freeze({ ... -}; +}); -export const BUILD_TOOL_SET: ToolSet = { +export const BUILD_TOOL_SET: ToolSet = Object.freeze({ ... -}; +});packages/db/src/schema/chat/message.ts (2)
8-8: Typo in constant name.
CONVERSATION_MESSAGe_RELATION_NAMEhas a stray lowercasee.-export const CONVERSATION_MESSAGe_RELATION_NAME = 'conversation_messages'; +export const CONVERSATION_MESSAGE_RELATION_NAME = 'conversation_messages';
11-27: Consider index for query patterns.Common access is by
conversationIdordered bycreatedAt. Add a compound index.export const messages = pgTable("messages", { ... -}).enableRLS(); +}, (t) => ({ + convoCreatedIdx: index("messages_conversation_created_idx").on(t.conversationId, t.createdAt), +})).enableRLS();apps/web/client/src/app/api/chat/helpers/stream.ts (1)
63-86: Tool-call repair: OPEN_AI_GPT_5_NANO present — add optional runtime fallbackOPEN_AI_GPT_5_NANO is defined in packages/models/src/llm/index.ts (seen in your rg output). Still recommend adding a safe fallback around the initModel call in apps/web/client/src/app/api/chat/helpers/stream.ts to avoid repair failures if that model isn’t provisioned in a given environment.
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (4)
21-31: Instantiate useChat with ChatMessage; consider scoping the chat instance per conversation.To avoid cross‑conversation state bleed, key the chat by conversationId.
- const chat = useChat<ChatMessage>({ - id: 'user-chat', + const chat = useChat<ChatMessage>({ + id: `user-chat-${conversationId ?? 'none'}`,
42-47: Remove unused variable.currentConversationId is declared but never used.
- const currentConversationId = editorEngine.chat.conversation.current?.conversation.id;
48-54: Externalize user-facing error strings (i18n).Avoid hardcoded UI text in app/**/*.tsx.
- editorEngine.chat.error.handleChatError(new Error('Output length limit reached')); + editorEngine.chat.error.handleChatError(new Error(t('chat.errors.outputLengthLimit'))) - editorEngine.chat.error.handleChatError(new Error('Content filter error')); + editorEngine.chat.error.handleChatError(new Error(t('chat.errors.contentFilter')))Add:
+import { useTranslations } from 'next-intl'; ... - const posthog = usePostHog(); + const posthog = usePostHog(); + const t = useTranslations();
66-69: Externalize “No conversation id”.This is user-visible in thrown error.
- throw new Error('No conversation id'); + throw new Error(t('chat.errors.noConversationId'))apps/web/client/src/components/store/editor/chat/conversation.ts (1)
50-72: User-facing toast strings should be localized.- toast.error('Error starting new conversation.', { - description: error instanceof Error ? error.message : 'Unknown error', - }); + toast.error(t('chat.errors.startConversation.title'), { + description: error instanceof Error ? error.message : t('common.unknownError'), + });Add t via next-intl at the top of the class where you construct messages.
apps/web/client/src/components/store/editor/chat/index.ts (3)
2-2: Importing from models/chat is fine given package export; keep it consistent across files.
60-70: Hardcoded prompt should be localized.- const prompt = `How can I resolve these errors? If you propose a fix, please make it concise.`; + const prompt = t('chat.fixErrors.prompt');Also localize any other user-visible strings in this file.
102-108: Localize default commit message.- userPrompt ?? "Save before chat", + userPrompt ?? t('chat.commit.defaultMessage'),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/web/client/src/app/api/chat/helpers/stream.ts(2 hunks)apps/web/client/src/app/api/chat/route.ts(2 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(4 hunks)apps/web/client/src/components/store/editor/chat/conversation.ts(4 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/components/tools/tools.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(2 hunks)packages/db/src/mappers/chat/message.ts(1 hunks)packages/db/src/schema/chat/message.ts(2 hunks)packages/models/src/chat/message/message.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
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 insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/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/index.tsxapps/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/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat.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 insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/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/index.tsxapps/web/client/src/app/api/chat/helpers/stream.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsxapps/web/client/src/components/tools/tools.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/components/store/editor/chat/index.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.tsapps/web/client/src/server/api/routers/chat/suggestion.ts
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
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/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/index.tsxapps/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/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsxpackages/ai/src/tools/toolset.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/api/chat/helpers/stream.tspackages/ai/src/stream/index.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsxapps/web/client/src/components/tools/tools.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/components/store/editor/chat/index.tspackages/models/src/chat/message/message.tsapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxpackages/db/src/mappers/chat/message.tsapps/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.tsapps/web/client/src/server/api/routers/chat/suggestion.tspackages/db/src/schema/chat/message.ts
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/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/index.tsxapps/web/client/src/app/api/chat/helpers/stream.tsapps/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/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxapps/web/client/src/app/api/chat/route.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/stream-message.tsxpackages/ai/src/tools/toolset.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsxapps/web/client/src/app/api/chat/helpers/stream.tspackages/ai/src/stream/index.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsxapps/web/client/src/components/tools/tools.tsapps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsxapps/web/client/src/components/store/editor/chat/index.tspackages/models/src/chat/message/message.tsapps/web/client/src/app/project/[id]/_hooks/use-chat.tsxpackages/db/src/mappers/chat/message.tsapps/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.tsapps/web/client/src/server/api/routers/chat/suggestion.tspackages/db/src/schema/chat/message.ts
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/**
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization in tRPC procedures
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under src/server/api/routers/**
Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization
Files:
apps/web/client/src/server/api/routers/chat/message.tsapps/web/client/src/server/api/routers/chat/suggestion.ts
🧠 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.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
📚 Learning: 2025-09-16T19:22:52.423Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.423Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Avoid hardcoded user-facing text; use next-intl messages/hooks
Applied to files:
apps/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/user-message.tsxapps/web/client/src/app/project/[id]/_hooks/use-chat.tsx
🧬 Code graph analysis (13)
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)
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(52-52)
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(52-52)
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(52-52)
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(52-52)
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/project/[id]/_hooks/use-chat.tsx (4)
packages/models/src/chat/message/message.ts (1)
ChatMessage(52-52)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/db/src/mappers/chat/message.ts (1)
packages/models/src/chat/message/message.ts (1)
ChatMessage(52-52)
apps/web/client/src/server/api/routers/chat/message.ts (1)
packages/db/src/schema/chat/message.ts (1)
messages(11-27)
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(52-52)
apps/web/client/src/app/api/chat/route.ts (2)
apps/web/client/src/app/api/chat/helpers/stream.ts (1)
getSystemPromptFromType(28-44)packages/ai/src/tools/toolset.ts (1)
getToolSetFromType(69-71)
apps/web/client/src/components/store/editor/chat/conversation.ts (2)
packages/models/src/chat/message/message.ts (1)
ChatMessage(52-52)packages/db/src/mappers/chat/message.ts (1)
toDbMessage(17-34)
packages/db/src/schema/chat/message.ts (1)
packages/models/src/chat/message/message.ts (1)
ChatMessage(52-52)
🔇 Additional comments (25)
apps/web/client/src/components/store/editor/chat/message.ts (1)
9-19: LGTM — verify no remaining top-level message.createdAt usagesRole literal, parts, and metadata (createdAt/conversationId) look correct.
Automated ripgrep in the sandbox failed with PCRE/file-type errors; run locally in the repo root to confirm there are no reads of top-level message.createdAt:
# ripgrep (preferred) rg -n -S --glob '!**/node_modules/**' 'message\.createdAt' -g '**/*.ts' -g '**/*.tsx' # grep fallback grep -nR --include='*.ts' --include='*.tsx' -E 'message\.createdAt' .apps/web/client/src/app/api/chat/route.ts (2)
80-82: LGTM: sync prompt/tool selection.Removing unnecessary
awaitaligns with the new sync helpers.
2-2: No change needed —getToolSetFromTypeis re-exported from the AI package barrel.
packages/ai/src/tools/toolset.ts exports the function (line 69); packages/ai/src/index.ts re-exports it viaexport * from './toolset';(line 2).apps/web/client/src/app/api/chat/helpers/stream.ts (1)
28-44: LGTM:getSystemPromptFromTypenow synchronous.Matches updated call sites and simplifies flow.
packages/ai/src/stream/index.ts (1)
7-10: LGTM: switch to string roles.Aligns with DB/model role union and removes enum dependency.
Also applies to: 29-35
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/assistant-message.tsx (1)
1-4: Make this a Client Component (or ensure a client boundary above).Consumed by a client
observerparent; client components cannot import server ones. Add'use client'at the top of this file unless a parent client boundary already covers it and this file isn't imported by server code.+ 'use client';packages/db/src/schema/chat/message.ts (2)
16-16: New NOT NULLcontent: backfill or add default + update writersBreaking change — inserts will fail unless messages.content is provided. Search only showed seed writes at packages/db/src/seed/db.ts (tx.insert(messages).values([...message0...])). Either backfill + update all writer paths to set content, or add a default on the column:
- content: text("content").notNull(), + content: text("content").default('').notNull(),
9-10: Confirm 'system' role is handled across serializers/filters/streamsDB/migrations, suggestion router, API route and chat UI already include 'system'.
- Attention required: packages/ai/src/stream/index.ts — streaming/hydration logic only checks for 'user' and 'assistant' (findLastIndex / hydrate) and may be dropping/ignoring 'system' messages; decide whether system messages should be streamed or filtered and update logic.
- Tests/types to update: packages/ai/test/stream/convert.test.ts and packages/ai/test/tokens.test.ts (and any other places using the literal union 'user' | 'assistant') — add 'system' where appropriate.
- Do a final repo-wide search for remaining 'user'|'assistant' unions or z.enum([...]) and update any missed serializers/filters.
packages/ai/src/tools/toolset.ts (1)
69-71: Export verified — getToolSetFromType is re-exported at package root.
packages/ai/src/tools/index.ts exports './toolset'; packages/ai/src/index.ts exports './tools'.apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/index.tsx (1)
1-16: Client boundary present — no change required.Both apps/web/client/src/app/project/[id]/providers.tsx and apps/web/client/src/app/project/[id]/_components/right-panel/index.tsx declare 'use client', so ChatMessages is already rendered inside a client boundary; do not add 'use client' here.
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx (1)
209-213: Avoid using nanoid() as React list keys — use a stable key.Random keys force remounts on every render; prefer an intrinsic identifier.
- {message.metadata?.context?.map((context) => ( - <SentContextPill key={nanoid()} context={context} /> - ))} + {message.metadata?.context?.map((context, i) => ( + <SentContextPill + key={`${context.id ?? context.type ?? 'ctx'}:${i}`} + context={context} + /> + ))}If
contexthas a stableid, use that instead of the index.apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (4)
6-7: Type alignment to single ChatMessage looks good.
12-12: Extended helpers generic is correct.
17-17: Ref type updated correctly.
73-75: Passing raw ChatMessage[] is correct for the new model.apps/web/client/src/server/api/routers/chat/message.ts (3)
3-7: Import consolidation to fromDbMessage/messageUpdateSchema looks good.
64-67: Update path: explicit parts assignment is fine.
92-97: normalizeMessage: createdAt normalization and parts passthrough are good.apps/web/client/src/components/store/editor/chat/conversation.ts (5)
4-13: Types aligned to ChatMessage/MessageContext; good.
114-129: addUserMessage: return type change is consistent and storage sync happens via addOrReplaceMessage.
149-179: attachCommitToUserMessage guard + metadata rebuild look good; verify server expects Date not string.Confirm updateCheckpoints schema uses z.date (server) and that createdAt here is a Date (it is).
181-193: Replace-by-id/vercelId logic is solid; storage upsert ensures durability.
220-226: toDbMessage(conversationId) usage is correct post‑signature change.apps/web/client/src/components/store/editor/chat/index.ts (2)
40-45: addEditMessage flow matches new model; commit attach is fire‑and‑forget.
54-58: addAskMessage: OK.
.../client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/user-message.tsx
Show resolved
Hide resolved
| role: z.enum(['user', 'assistant', 'system']), | ||
| content: z.string(), | ||
| })), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restrict client-provided roles to user/assistant to avoid prompt injection via system.
Allowing system from clients can override/compete with our own system prompt.
- messages: z.array(z.object({
- role: z.enum(['user', 'assistant', 'system']),
+ messages: z.array(z.object({
+ role: z.enum(['user', 'assistant']),
content: z.string(),
})),Optionally, filter any incoming system roles server-side before calling the model.
📝 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(), | |
| })), |
🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/chat/suggestion.ts around lines 17 to
19, the schema currently allows clients to set role='system', which can enable
prompt injection; change the validation to only accept 'user' and 'assistant'
(remove 'system' from the enum) and additionally sanitize incoming messages
server-side by either rejecting requests containing any 'system' role or
stripping those messages before constructing the model prompt so only trusted
server-controlled system prompts are used.
| ...convertToModelMessages(input.messages.map((m) => ({ | ||
| role: m.role, | ||
| parts: [{ type: 'text', text: m.content }], | ||
| }))), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't pass provider messages to generateObject; use CoreMessage/UIMessage instead.
convertToModelMessages returns provider messages; generateObject expects Core/UI messages and will do provider conversion internally. Mixing shapes can break types and payloads.
Apply:
-import { ChatSuggestionsSchema } from '@onlook/models/chat';
-import { convertToModelMessages, generateObject } from 'ai';
+import { ChatSuggestionsSchema } from '@onlook/models/chat';
+import { generateObject } from 'ai';
@@
- ...convertToModelMessages(input.messages.map((m) => ({
- role: m.role,
- parts: [{ type: 'text', text: m.content }],
- }))),
+ ...input.messages.map((m) => ({
+ role: m.role,
+ content: m.content,
+ })),Also applies to: 6-7
🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/chat/suggestion.ts around lines 35-38
(and also update the similar usage at lines 6-7), you are passing provider-form
messages from convertToModelMessages into generateObject, but generateObject
expects CoreMessage/UIMessage and will perform provider conversion itself; this
mixes shapes and can break types/payloads. Fix by changing the flow so you pass
Core/UI message objects (e.g., the original m converted to the Core/UIMessage
shape with role and text parts) directly to generateObject, and remove/avoid
calling convertToModelMessages before generateObject; if you need provider
messages later, call convertToModelMessages only when invoking the provider.
Ensure types are updated so generateObject receives Core/UIMessage types.
| return { | ||
| ...message, | ||
| threadId: message.conversationId, | ||
| metadata: { | ||
| vercelId: message.id, | ||
| conversationId: message.conversationId, | ||
| createdAt: message.createdAt, | ||
| context: message.context ?? [], | ||
| checkpoints: message.checkpoints ?? [], | ||
| }, | ||
| parts: message.parts ?? [], | ||
|
|
||
| } | ||
| switch (message.role) { | ||
| case ChatMessageRole.ASSISTANT: | ||
| return { | ||
| ...baseMessage, | ||
| role: message.role as ChatMessageRole.ASSISTANT, | ||
| } satisfies AssistantChatMessage; | ||
| case ChatMessageRole.USER: | ||
| return { | ||
| ...baseMessage, | ||
| role: message.role as ChatMessageRole.USER, | ||
| } satisfies UserChatMessage; | ||
| default: | ||
| assertNever(message.role); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid leaking DB-only fields into ChatMessage (conversationId/content/createdAt at top level).
Construct ChatMessage explicitly to match the UI type.
-export const fromDbMessage = (message: DbMessage): ChatMessage => {
- return {
- ...message,
- metadata: {
- conversationId: message.conversationId,
- createdAt: message.createdAt,
- context: message.context ?? [],
- checkpoints: message.checkpoints ?? [],
- },
- parts: message.parts ?? [],
- }
-}
+export const fromDbMessage = (message: DbMessage): ChatMessage => {
+ return {
+ id: message.id,
+ role: message.role,
+ parts: message.parts ?? [],
+ metadata: {
+ conversationId: message.conversationId,
+ createdAt: message.createdAt,
+ context: message.context ?? [],
+ checkpoints: 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.
| return { | |
| ...message, | |
| threadId: message.conversationId, | |
| metadata: { | |
| vercelId: message.id, | |
| conversationId: message.conversationId, | |
| createdAt: message.createdAt, | |
| context: message.context ?? [], | |
| checkpoints: message.checkpoints ?? [], | |
| }, | |
| parts: message.parts ?? [], | |
| } | |
| switch (message.role) { | |
| case ChatMessageRole.ASSISTANT: | |
| return { | |
| ...baseMessage, | |
| role: message.role as ChatMessageRole.ASSISTANT, | |
| } satisfies AssistantChatMessage; | |
| case ChatMessageRole.USER: | |
| return { | |
| ...baseMessage, | |
| role: message.role as ChatMessageRole.USER, | |
| } satisfies UserChatMessage; | |
| default: | |
| assertNever(message.role); | |
| } | |
| } | |
| export const fromDbMessage = (message: DbMessage): ChatMessage => { | |
| return { | |
| id: message.id, | |
| role: message.role, | |
| parts: message.parts ?? [], | |
| metadata: { | |
| conversationId: message.conversationId, | |
| createdAt: message.createdAt, | |
| context: message.context ?? [], | |
| checkpoints: message.checkpoints ?? [], | |
| }, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In packages/db/src/mappers/chat/message.ts around lines 5 to 15, avoid leaking
DB-only fields (conversationId, content, createdAt, etc.) by not spreading the
raw DB message into the ChatMessage output; instead construct the ChatMessage
explicitly to match the UI type — pick only the public fields (e.g., id, role,
sender, other UI-visible props), set metadata with
conversationId/createdAt/context/checkpoints, and set parts to message.parts ??
[]; remove top-level conversationId/content/createdAt from the returned object
and ensure the returned shape strictly matches the ChatMessage interface.
| export const toDbMessage = (message: ChatMessage, conversationId: string): DbMessage => { | ||
| const createdAt = message.metadata?.createdAt; | ||
| return { | ||
| id: message.id, | ||
| createdAt: message.createdAt, | ||
| conversationId: message.threadId, | ||
| createdAt: createdAt instanceof Date ? createdAt : createdAt ? new Date(createdAt) : new Date(), | ||
| conversationId, | ||
| context: message?.metadata?.context ?? [], | ||
| parts: message.parts, | ||
| content: message.parts.map((part) => { | ||
| if (part.type === 'text') { | ||
| return part.text; | ||
| } | ||
| return ''; | ||
| }).join(''), | ||
| role: message.role as DbMessage['role'], | ||
| role: message.role, | ||
| checkpoints: message.metadata?.checkpoints ?? [], | ||
|
|
||
| // deprecated | ||
| applied: null, | ||
| commitOid: null, | ||
| snapshots: null, | ||
| } satisfies DbMessage; | ||
| } | ||
|
|
||
| export const toOnlookMessageFromVercel = (message: VercelMessage, conversationId: string): ChatMessage => { | ||
| const metadata = { | ||
| vercelId: message.id, | ||
| context: [], | ||
| checkpoints: [], | ||
| } | ||
| const baseMessage: ChatMessage = { | ||
| ...message, | ||
| id: uuidv4(), | ||
| createdAt: new Date(), | ||
| threadId: conversationId, | ||
| metadata, | ||
| parts: message.parts ?? [], | ||
| role: message.role as ChatMessageRole, | ||
| content: '', | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Verification inconclusive
Persist meaningful content instead of empty string.
Storing '' in content drops searchable text and complicates analytics.
export const toDbMessage = (message: ChatMessage, conversationId: string): DbMessage => {
const createdAt = message.metadata?.createdAt;
+ const content =
+ message.parts?.map(p => (p.type === 'text' ? p.text : '')).join(' ').trim() ?? '';
return {
id: message.id,
createdAt: createdAt instanceof Date ? createdAt : createdAt ? new Date(createdAt) : new Date(),
conversationId,
context: message?.metadata?.context ?? [],
parts: message.parts,
role: message.role,
checkpoints: message.metadata?.checkpoints ?? [],
// deprecated
applied: null,
commitOid: null,
snapshots: null,
- content: '',
+ content,
}
}If non-text parts should be represented differently in content, specify the policy (e.g., strip, annotate, or JSON-encode).
Persist meaningful content and fix runtime error in toDbMessage
- Don't store '' — derive content from text parts and guard against missing parts: e.g. const content = (message.parts ?? []).map(p => p.type === 'text' ? (p.text ?? '') : '').join(' ').trim(); set content instead of ''.
- Decide policy for non-text parts (strip, annotate like "[image]", or JSON-encode) and apply consistently.
File: packages/db/src/mappers/chat/message.ts (lines 17–33).
🤖 Prompt for AI Agents
packages/db/src/mappers/chat/message.ts around lines 17 to 33: toDbMessage
currently stores an empty string for content causing loss of message text and
potential runtime errors when parts are missing; replace the empty string by
deriving content from message.parts safely (const parts = message.parts ?? [];
map parts extracting text for parts with type 'text' using p.text ?? '' and join
with spaces, then .trim()), and decide/implement a consistent policy for
non-text parts (either ignore/strip them, annotate with a placeholder like
"[image]" or JSON-encode them) so content always contains meaningful data and
will not throw when parts is undefined. Ensure createdAt handling remains and
set content to the derived value.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Unifies chat message format, centralizes tool selection, and improves error handling and performance across the app.
UIMessagewithChatMessage.getToolSetFromType()intoolset.ts.streamResponse()inroute.tsanderrorHandler()instream.ts.stream.tsandroute.ts.stream.ts.conversation.ts.conversation.ts.suggestion.ts.index.tsxandmessage.tsx.prompt.test.tsandconvert.test.tsto reflect new message format.SYSTEM_PROMPTinsystem.tsto include new guidelines.message.tsandschema.tsto support new message format.This description was created by
for 63aedc1. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation
Tests
Chores