-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: reduce agent overhead #2953
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
This reverts commit 49d5dfa.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Caution Review failedThe pull request is closed. 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. WalkthroughConsolidates chat streaming to a single root-agent flow using createRootAgentStream, removes legacy agent/sub-agent architecture and related tools, updates tool handling APIs, drops Anthropic provider/models, adjusts enums and provider logic, refactors server conversation mutations, and bumps AI-related dependencies across packages. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Web Client (useChat)
participant API as /api/chat (route.ts)
participant Root as createRootAgentStream
participant Model as LLM (OpenRouter)
participant Tools as Toolset
Note over UI,API: UI sends messages, chatType, ids (no AgentType)
User->>UI: Send message
UI->>API: POST messages, chatType, conversationId, projectId
API->>Root: createRootAgentStream(payload, metadata)
Root->>Model: streamText(system, messages, tools, options)
Model-->>Root: Stream parts (text/toolCall/finish)
alt toolCall
Root->>Tools: Find tool by name
alt invalid tool input
Root->>Root: repairToolCall(tool, toolCall)
Root->>Model: Retry with repaired tool input
end
Tools-->>Root: Tool output
end
Root-->>API: UIMessage stream with metadata
API-->>UI: Stream response
UI-->>User: Render streaming output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 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 |
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: 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/server/api/routers/chat/conversation.ts (1)
39-44: Restore real upsert semanticsThis mutation now issues a raw insert with no conflict handling, so calling it for an existing conversation will trigger a duplicate-key error instead of updating the row as before. Please reinstate the conflict update (including refreshing
updatedAt) to keep the API contract intact.- const [conversation] = await ctx.db.insert(conversations).values(input).returning(); + const [conversation] = await ctx.db.insert(conversations) + .values(input) + .onConflictDoUpdate({ + target: conversations.id, + set: { + ...input, + updatedAt: new Date(), + }, + }) + .returning();
🧹 Nitpick comments (6)
packages/ai/src/agents/streamer.ts (3)
5-5: Remove unused import.
UIMessageStreamOnFinishCallbackis imported but not used anywhere in this file.Apply this diff to remove the unused import:
-import type { streamText, UIMessageStreamOnFinishCallback, UIMessageStreamOptions } from "ai"; +import type { streamText, UIMessageStreamOptions } from "ai";
28-29: Consider documenting empty context and checkpoints.The
contextandcheckpointsfields are hardcoded to empty arrays. If these are placeholders for future implementation or intentionally empty in this refactor, consider adding a brief comment to clarify intent.Example:
return { createdAt: new Date(), conversationId, + // TODO: Populate context and checkpoints when available context: [], checkpoints: [], finishReason: part.type === 'finish-step' ? part.finishReason : undefined, usage: part.type === 'finish-step' ? part.usage : undefined, } satisfies ChatMetadata;
16-16: Remove unnecessary async/await fromstreamText. SinceBaseAgent.streamTextis synchronous, dropasyncon line 16 and removeawaitbeforethis.agent.streamTexton line 19.packages/db/src/mappers/chat/conversation.ts (2)
4-10: Review spread operator usage in mapper.The spread operator copies all properties from the DB conversation, which may include fields that shouldn't be in the model (or vice versa). While this works if DB and model schemas are aligned, it creates coupling and could cause issues if extra fields exist.
Consider being more explicit about which fields to map:
export const fromDbConversation = (conversation: DbConversation): ChatConversation => { return { - ...conversation, + id: conversation.id, + projectId: conversation.projectId, + createdAt: conversation.createdAt, + updatedAt: conversation.updatedAt, title: conversation.displayName || null, suggestions: conversation.suggestions || [], } }
12-19: Review spread operator usage and redundant assignments.Similar to
fromDbConversation, the spread operator combined with explicit field assignments (lines 15-17) is redundant. The explicit assignments will override the spread values for those same keys.Consider removing the spread or removing the redundant explicit assignments:
export const toDbConversation = (conversation: ChatConversation): DbConversation => { return { - ...conversation, + id: conversation.id, projectId: conversation.projectId, + createdAt: conversation.createdAt, + updatedAt: conversation.updatedAt, displayName: conversation.title || null, suggestions: conversation.suggestions || [], } }apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx (1)
56-60: Reset the tool-call flag even on failuresLine 58: if
handleToolCallthrows, the.then(...)never runs andisExecutingToolCallstaystrue, leaving the UI in a perpetual "running" state. Swap to.finally(...)so the flag always resets.- void handleToolCall(toolCall.toolCall, editorEngine, addToolResult).then(() => { - setIsExecutingToolCall(false); - }); + void handleToolCall(toolCall.toolCall, editorEngine, addToolResult).finally(() => { + setIsExecutingToolCall(false); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
apps/web/client/src/app/api/chat/helpers/stream.ts(2 hunks)apps/web/client/src/app/api/chat/route.ts(4 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx(3 hunks)apps/web/client/src/components/store/editor/chat/conversation.ts(0 hunks)apps/web/client/src/components/tools/tools.ts(1 hunks)apps/web/client/src/server/api/routers/chat/conversation.ts(3 hunks)apps/web/client/src/server/api/routers/chat/suggestion.ts(1 hunks)apps/web/client/src/server/api/routers/project/project.ts(1 hunks)packages/ai/src/agents/classes/index.ts(1 hunks)packages/ai/src/agents/classes/root.ts(2 hunks)packages/ai/src/agents/classes/user.ts(0 hunks)packages/ai/src/agents/index.ts(0 hunks)packages/ai/src/agents/models/base.ts(2 hunks)packages/ai/src/agents/onlook-chat.ts(0 hunks)packages/ai/src/agents/streamer.ts(2 hunks)packages/ai/src/agents/tool-lookup.ts(0 hunks)packages/ai/src/agents/tools/base.ts(0 hunks)packages/ai/src/agents/tools/index.ts(0 hunks)packages/ai/src/agents/tools/user.ts(0 hunks)packages/ai/src/chat/providers.ts(2 hunks)packages/ai/src/tools/index.ts(1 hunks)packages/ai/src/tools/models/client.ts(1 hunks)packages/ai/src/tools/toolset-utils.ts(0 hunks)packages/ai/src/tools/toolset.ts(1 hunks)packages/db/src/defaults/conversation.ts(0 hunks)packages/db/src/mappers/chat/conversation.ts(1 hunks)packages/db/src/schema/chat/conversation.ts(2 hunks)packages/models/src/chat/conversation/index.ts(0 hunks)packages/models/src/chat/message/message.ts(0 hunks)
💤 Files with no reviewable changes (12)
- packages/models/src/chat/message/message.ts
- packages/ai/src/agents/index.ts
- packages/models/src/chat/conversation/index.ts
- packages/ai/src/agents/tools/user.ts
- apps/web/client/src/components/store/editor/chat/conversation.ts
- packages/ai/src/agents/tools/base.ts
- packages/ai/src/tools/toolset-utils.ts
- packages/ai/src/agents/classes/user.ts
- packages/ai/src/agents/tools/index.ts
- packages/ai/src/agents/onlook-chat.ts
- packages/ai/src/agents/tool-lookup.ts
- packages/db/src/defaults/conversation.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/ai/src/agents/classes/index.tspackages/db/src/mappers/chat/conversation.tsapps/web/client/src/server/api/routers/project/project.tspackages/ai/src/agents/classes/root.tsapps/web/client/src/server/api/routers/chat/conversation.tsapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/app/api/chat/helpers/stream.tspackages/ai/src/chat/providers.tspackages/ai/src/agents/streamer.tspackages/ai/src/agents/models/base.tspackages/db/src/schema/chat/conversation.tspackages/ai/src/tools/index.tsapps/web/client/src/components/tools/tools.tsapps/web/client/src/server/api/routers/chat/suggestion.tsapps/web/client/src/app/api/chat/route.tspackages/ai/src/tools/toolset.tspackages/ai/src/tools/models/client.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
packages/ai/src/agents/classes/index.tspackages/db/src/mappers/chat/conversation.tsapps/web/client/src/server/api/routers/project/project.tspackages/ai/src/agents/classes/root.tsapps/web/client/src/server/api/routers/chat/conversation.tsapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/app/api/chat/helpers/stream.tspackages/ai/src/chat/providers.tspackages/ai/src/agents/streamer.tspackages/ai/src/agents/models/base.tspackages/db/src/schema/chat/conversation.tspackages/ai/src/tools/index.tsapps/web/client/src/components/tools/tools.tsapps/web/client/src/server/api/routers/chat/suggestion.tsapps/web/client/src/app/api/chat/route.tspackages/ai/src/tools/toolset.tspackages/ai/src/tools/models/client.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/project/project.tsapps/web/client/src/server/api/routers/chat/conversation.tsapps/web/client/src/server/api/routers/chat/suggestion.ts
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/server/api/routers/project/project.tsapps/web/client/src/server/api/routers/chat/conversation.tsapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/app/api/chat/helpers/stream.tsapps/web/client/src/components/tools/tools.tsapps/web/client/src/server/api/routers/chat/suggestion.tsapps/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 insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/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
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]/_hooks/use-chat/index.tsx
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]/_hooks/use-chat/index.tsxapps/web/client/src/app/api/chat/helpers/stream.tsapps/web/client/src/app/api/chat/route.ts
🧬 Code graph analysis (13)
apps/web/client/src/server/api/routers/project/project.ts (1)
packages/ai/src/chat/providers.ts (1)
initModel(14-51)
packages/ai/src/agents/classes/root.ts (3)
packages/models/src/llm/index.ts (1)
ModelConfig(35-40)packages/ai/src/tools/toolset.ts (1)
getToolSetFromType(66-68)packages/ai/src/chat/providers.ts (1)
initModel(14-51)
apps/web/client/src/server/api/routers/chat/conversation.ts (2)
packages/db/src/schema/chat/conversation.ts (1)
conversations(11-20)packages/ai/src/chat/providers.ts (1)
initModel(14-51)
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx (1)
apps/web/client/src/components/tools/tools.ts (1)
handleToolCall(6-37)
apps/web/client/src/app/api/chat/helpers/stream.ts (4)
packages/ai/src/agents/classes/root.ts (2)
getModelFromType(38-54)getSystemPromptFromType(21-31)packages/models/src/llm/index.ts (1)
ModelConfig(35-40)packages/ai/src/chat/providers.ts (1)
initModel(14-51)packages/ai/src/prompt/provider.ts (3)
getCreatePageSystemPrompt(28-32)getAskModeSystemPrompt(40-44)getSystemPrompt(21-26)
packages/ai/src/chat/providers.ts (1)
packages/models/src/llm/index.ts (3)
InitialModelPayload(28-33)ModelConfig(35-40)MODEL_MAX_TOKENS(42-51)
packages/ai/src/agents/streamer.ts (3)
packages/models/src/chat/message/message.ts (1)
ChatMessage(18-18)packages/ai/src/agents/models/base.ts (1)
streamText(30-51)packages/ai/src/stream/index.ts (1)
convertToStreamMessages(5-23)
packages/ai/src/agents/models/base.ts (1)
packages/models/src/llm/index.ts (1)
ModelConfig(35-40)
apps/web/client/src/components/tools/tools.ts (2)
apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(34-140)packages/ai/src/tools/toolset.ts (1)
getToolClassesFromType(62-64)
apps/web/client/src/server/api/routers/chat/suggestion.ts (1)
packages/ai/src/chat/providers.ts (1)
initModel(14-51)
apps/web/client/src/app/api/chat/route.ts (3)
packages/models/src/chat/message/message.ts (1)
ChatMessage(18-18)apps/web/client/src/app/api/chat/helpers/usage.ts (1)
incrementUsage(43-65)packages/ai/src/agents/classes/root.ts (1)
RootAgent(5-55)
packages/ai/src/tools/toolset.ts (2)
packages/ai/src/tools/classes/list-files.ts (1)
ListFilesTool(8-144)packages/ai/src/tools/classes/index.ts (19)
ListFilesTool(8-8)ReadFileTool(10-10)BashReadTool(2-2)OnlookInstructionsTool(9-9)ReadStyleGuideTool(11-11)ListBranchesTool(7-7)ScrapeUrlTool(13-13)WebSearchTool(18-18)GlobTool(5-5)GrepTool(6-6)TypecheckTool(17-17)CheckErrorsTool(3-3)SearchReplaceEditTool(14-14)SearchReplaceMultiEditFileTool(15-15)FuzzyEditFileTool(4-4)WriteFileTool(19-19)BashEditTool(1-1)SandboxTool(12-12)TerminalCommandTool(16-16)
packages/ai/src/tools/models/client.ts (1)
apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(34-140)
🔇 Additional comments (8)
packages/db/src/schema/chat/conversation.ts (3)
1-1: LGTM: Type-only import is appropriate.The change to a type-only import for
ChatSuggestionis correct since it's only used as a type parameter in$type<ChatSuggestion[]>()on line 19.
22-25: Simplified insert schema and explicit UUID validation.The insert schema now correctly relies solely on the table definition without custom field handling. The update schema's explicit
id: z.string().uuid()ensures UUID validation at the API boundary, which is a good practice for update operations.
11-20: Verify migration for dropping agentType columnAutomated search didn’t locate any SQL or TS migration that drops
agent_type; please manually confirm a migration has been created and applied to remove this column in existing databases.packages/ai/src/agents/classes/root.ts (5)
1-2: LGTM!The import changes correctly support the refactored architecture:
ModelConfigandinitModelenable async model initialization, andgetToolSetFromTypereplaces the removed tool lookups.
6-6: LGTM!The
idfield provides a stable identifier for the root agent, which is useful for logging and debugging.
10-15: LGTM!The constructor refactor correctly supports the factory pattern by accepting a pre-initialized
modelConfig, avoiding async constructor issues and enabling cleaner initialization viaRootAgent.create.
33-36: LGTM!The static factory method correctly encapsulates async model initialization, following best practices for asynchronous construction patterns.
38-54: Validate availability ofOPENROUTER_MODELS.OPEN_AI_GPT_5
Ensure"openai/gpt-5"is listed by OpenRouter’s Models API or live model browser, and implement a fallback if it’s unavailable.
| case ChatType.EDIT: | ||
| default: | ||
| model = await initModel({ | ||
| provider: LLMProvider.OPENROUTER, | ||
| model: OPENROUTER_MODELS.CLAUDE_4_SONNET, | ||
| }); | ||
| break; |
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.
Align ASK/EDIT model selection with RootAgent
Line [19] currently returns OPENROUTER_MODELS.CLAUDE_4_SONNET, but RootAgent.getModelFromType already moved ASK/EDIT over to CLAUDE_4_5_SONNET (see packages/ai/src/agents/classes/root.ts, Lines 37-53). Leaving this helper on the older model makes the API choose a different LLM than the agents, undermining the overhead refactor and producing inconsistent chat behaviour. Please switch this branch to OPENROUTER_MODELS.CLAUDE_4_5_SONNET so both paths stay in sync.
🤖 Prompt for AI Agents
In apps/web/client/src/app/api/chat/helpers/stream.ts around lines 17 to 23, the
ASK/EDIT branch currently selects OPENROUTER_MODELS.CLAUDE_4_SONNET which is out
of sync with RootAgent.getModelFromType; update this branch to use
OPENROUTER_MODELS.CLAUDE_4_5_SONNET so the helper and RootAgent choose the same
LLM, ensuring consistent chat behavior across paths.
| const [conversation] = await ctx.db.update({ | ||
| ...conversations, | ||
| updatedAt: new Date(), | ||
| }).set(input) |
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.
Fix Drizzle update call
Spreading the table into a plain object strips the symbols Drizzle needs, so this will throw at runtime (table name becomes undefined) and updatedAt is no longer refreshed. Stick with the table instance and set updatedAt inside .set(...).
- const [conversation] = await ctx.db.update({
- ...conversations,
- updatedAt: new Date(),
- }).set(input)
+ const [conversation] = await ctx.db.update(conversations)
+ .set({
+ ...input,
+ updatedAt: new Date(),
+ })📝 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 [conversation] = await ctx.db.update({ | |
| ...conversations, | |
| updatedAt: new Date(), | |
| }).set(input) | |
| const [conversation] = await ctx.db.update(conversations) | |
| .set({ | |
| ...input, | |
| updatedAt: new Date(), | |
| }) |
🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/chat/conversation.ts around lines
48-51, the code spreads the `conversations` table into a plain object which
removes the Drizzle table symbols and prevents `updatedAt` from being updated;
change the update call to use the table instance (e.g.,
ctx.db.update(conversations)) and pass the update payload to .set(...) including
updatedAt (for example .set({ ...input, updatedAt: new Date() })) so the table
metadata is preserved and updatedAt is refreshed.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Refactor chat agent architecture to reduce overhead by unifying agents and simplifying tool handling.
stream.tsandroute.ts.root.tsandtoolset.ts.BaseAgentandUserAgentclasses.@ai-sdk/provider-utilsand@ai-sdk/reactversions inpackage.json.This description was created by
for 5b5ff0a. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit