Sweetmantech/myc 3473 add a routing agent without slowing down the initial chat#1374
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Rate limit exceeded@sweetmantech has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 1 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis PR introduces a routing agent system and refactors message rendering. Changes extend Changes
Sequence DiagramsequenceDiagram
participant Client as Client/Request
participant Execute as getExecute
participant Setup as setupChatRequest
participant Routing as getRoutingDecision
participant Agent as routingAgent
participant Stream as Writer (Stream)
Client->>Execute: Execute chat request with writer
Execute->>Setup: setupChatRequest(body, writer)
activate Setup
Setup->>Routing: getRoutingDecision(body, writer)
activate Routing
Routing->>Stream: writeRoutingStatus("analyzing")
Routing->>Agent: Query routing decision
activate Agent
Agent-->>Routing: { agent, reason }
deactivate Agent
Routing->>Stream: writeRoutingStatus("complete", agent, reason)
Routing-->>Setup: RoutingDecision
deactivate Routing
Setup->>Setup: Build chatConfig with routing info
Setup-->>Execute: chatConfig
deactivate Setup
Execute->>Execute: Continue streaming & usage handling
Execute-->>Client: Response with streaming updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (1 passed)
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 |
Braintrust eval reportNo experiments to report |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Inline comments
⚠️ Logic —lib/chat/getRoutingDecision.ts(image-edit routing)
return {
model: \"openai/gpt-5\",
excludeTools: [\"generate_image\"],
reason: \"image-editing-detected\",
};Hard-coded model may be unsupported; use a configured constant (e.g., vision-capable) or fall back to DEFAULT_MODEL.
⚠️ Logic —hooks/useVercelChat.ts(onDatatimeout)
if (routingData.status === \"complete\") {
setTimeout(() => setRoutingStatus(null), 2000);
}Store the timeout id in a ref and clear it on unmount to avoid setting state after unmount.
⚠️ Logic —lib/chat/getExecute.ts(applying routed model)
model: routingDecision.model || body.model,Validate against an allowed models list before applying to prevent invalid-model failures downstream.
- ✨ Improvement —
providers/VercelChatProvider.tsx(context defaults)
Ensure the context default value includesroutingStatusto match the updated type and avoidundefinedaccess in consumers outside the provider.
Summary
Most changes look good; the transient routing status UX is clean. Flagged model/config validation and timeout cleanup to prevent runtime issues.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
lib/chat/getExecute.ts (1)
15-44: Consider extracting routing UI logic for better maintainability.The routing status updates (lines 15-44) add significant complexity to
getExecute, pushing it beyond the 50-line guideline for utility functions. The message construction logic is also inline, making it harder to test and reuse.Extract the routing flow to a separate function:
// lib/chat/routingHelpers.ts import { UIMessageStreamWriter } from "ai"; import { ChatRequest } from "./types"; import { getRoutingDecision } from "./getRoutingDecision"; async function performRouting( writer: UIMessageStreamWriter, body: ChatRequest ): Promise<ChatRequest> { // Send analyzing status writer.write({ type: "data-routing-status", data: { status: "analyzing", message: "Determining optimal model...", }, transient: true, }); // Get routing decision const routingDecision = await getRoutingDecision(body); console.log("🚀 Routing decision:", routingDecision); // Send complete status writer.write({ type: "data-routing-status", data: { status: "complete", message: routingDecision.reason === "user-selected" ? "Using selected model" : routingDecision.model ? `Routing to ${routingDecision.model}` : "Using default model", model: routingDecision.model, reason: routingDecision.reason, }, transient: true, }); // Apply routing decision return { ...body, model: routingDecision.model || body.model, excludeTools: routingDecision.excludeTools ? [...(body.excludeTools || []), ...routingDecision.excludeTools] : body.excludeTools, }; }Then simplify
getExecute:const getExecute = async (options: ExecuteOptions, body: ChatRequest) => { const { writer } = options; console.log("🚀 getExecute START - Model:", body.model); - // Send routing status to UI immediately (transient - won't be in message history) - writer.write({ - type: "data-routing-status", - data: { - status: "analyzing", - message: "Determining optimal model...", - }, - transient: true, - }); - - // Fast routing decision (non-blocking, uses lightweight model) - const routingDecision = await getRoutingDecision(body); - console.log("🚀 getExecute - Routing decision:", routingDecision); - - // Update UI with routing result (transient) - writer.write({ - type: "data-routing-status", - data: { - status: "complete", - message: - routingDecision.reason === "user-selected" - ? "Using selected model" - : routingDecision.model - ? `Routing to ${routingDecision.model}` - : "Using default model", - model: routingDecision.model, - reason: routingDecision.reason, - }, - transient: true, - }); - - // Apply routing decision to request - const routedBody: ChatRequest = { - ...body, - model: routingDecision.model || body.model, - excludeTools: routingDecision.excludeTools - ? [...(body.excludeTools || []), ...routingDecision.excludeTools] - : body.excludeTools, - }; + // Perform routing with UI feedback + const routedBody = await performRouting(writer, body); const chatConfig = await setupChatRequest(routedBody);This improves testability, reduces
getExecutecomplexity, and follows the single responsibility principle.As per coding guidelines (DRY, KISS, functions under 50 lines).
components/VercelChat/ChatInput.tsx (1)
56-57: Consider keeping the multi-line format for readability.The condition now spans multiple checks on a single line. While functional, the previous multi-line format may have been more readable for a condition with 4 clauses.
- if (input === "" || isDisabled || hasPendingUploads || isLoadingSignedUrls) - return; + if ( + input === "" || + isDisabled || + hasPendingUploads || + isLoadingSignedUrls + ) { + return; + }Or extract to a well-named boolean:
+ const cannotSend = input === "" || isDisabled || hasPendingUploads || isLoadingSignedUrls; + if (cannotSend) return;components/VercelChat/RoutingStatus.tsx (1)
5-9: Export the RoutingStatusProps interface.The interface is not exported, which limits its reusability and makes it harder for consumers to reference the component's prop types. Export it for better TypeScript ergonomics.
-interface RoutingStatusProps { +export interface RoutingStatusProps { status: "analyzing" | "complete"; message: string; model?: string; }Note: This interface should eventually use the shared
RoutingStatustype mentioned in the provider review to maintain consistency.lib/chat/getRoutingDecision.ts (3)
43-60: Extract hardcoded constants and consider pattern extensibility.The image edit patterns and model selection are hardcoded, making them harder to maintain and extend. Consider extracting them to constants and making the pattern matching more robust.
Create a routing constants file:
// lib/chat/routingConstants.ts export const ROUTING_PATTERNS = { IMAGE_EDITING: [ "edit image", "modify image", "change image", "adjust image", "update image", "alter image", ], // Future patterns can be added here } as const; export const ROUTING_MODELS = { VISION: "openai/gpt-5", DEFAULT: "openai/gpt-5-mini", LIGHTWEIGHT: "openai/gpt-4o-mini", } as const; export const TOOL_EXCLUSIONS = { IMAGE_EDITING: ["generate_image"], } as const;Then use them in your function:
+ import { ROUTING_PATTERNS, ROUTING_MODELS, TOOL_EXCLUSIONS } from "./routingConstants"; // Fast pattern-based routing for common cases (avoids LLM call) - const imageEditPatterns = [ - "edit image", - "modify image", - "change image", - "adjust image", - ]; - const hasImageEdit = imageEditPatterns.some((pattern) => + const hasImageEdit = ROUTING_PATTERNS.IMAGE_EDITING.some((pattern) => messageText.includes(pattern) ); if (hasImageEdit) { return { - model: "openai/gpt-5", // Vision-capable model - excludeTools: ["generate_image"], + model: ROUTING_MODELS.VISION, + excludeTools: TOOL_EXCLUSIONS.IMAGE_EDITING, reason: "image-editing-detected", }; }This makes the patterns easier to maintain and extend as routing rules grow.
As per coding guidelines (DRY, avoid magic strings).
64-81: Consider extracting the prompt and adding runtime validation.The routing prompt is inline and spans multiple lines, making it harder to test and maintain. Additionally, the type cast on line 81 bypasses runtime validation.
1. Extract the prompt to a separate function:
function buildRoutingPrompt(messageText: string): string { const truncatedMessage = messageText.substring(0, 200); return `You are a fast routing agent. Determine the optimal model for this message. Message: "${truncatedMessage}" Rules: - Image editing → "openai/gpt-5" - Complex reasoning → "openai/gpt-5" - Simple queries → "${DEFAULT_MODEL}" - Default → "${DEFAULT_MODEL}" Return routing decision. Only route if needed, otherwise leave model empty.`; }2. Use schema validation instead of type casting:
try { const { object } = await generateObject({ model: gateway(LIGHTWEIGHT_MODEL), schema: RoutingDecisionSchema, - prompt: `You are a fast routing agent. Determine the optimal model for this message. - -Message: "${messageText.substring(0, 200)}" - -Rules: -- Image editing → "openai/gpt-5" -- Complex reasoning → "openai/gpt-5" -- Simple queries → "${DEFAULT_MODEL}" -- Default → "${DEFAULT_MODEL}" - -Return routing decision. Only route if needed, otherwise leave model empty.`, + prompt: buildRoutingPrompt(messageText), temperature: 0, // Deterministic for speed }); - return object as RoutingDecision; + // Validate the returned object matches our schema + return RoutingDecisionSchema.parse(object); } catch (error) {This improves testability and adds runtime type safety.
20-91: Consider splitting this function for better maintainability.The
getRoutingDecisionfunction is 71 lines, exceeding the 50-line guideline for utility functions. Breaking it into smaller, focused functions would improve testability and maintainability.Split into focused helper functions:
// Extract message text function extractLastMessageText(messages: Array<UIMessage>): string { const lastMessage = messages[messages.length - 1]; return ( lastMessage?.parts .filter((part) => part.type === "text") .map((part) => part.text) .join(" ") || "" ).toLowerCase(); } // Pattern-based routing function tryPatternBasedRouting(messageText: string): RoutingDecision | null { const hasImageEdit = ROUTING_PATTERNS.IMAGE_EDITING.some((pattern) => messageText.includes(pattern) ); if (hasImageEdit) { return { model: ROUTING_MODELS.VISION, excludeTools: TOOL_EXCLUSIONS.IMAGE_EDITING, reason: "image-editing-detected", }; } return null; } // LLM-based routing async function performLLMRouting(messageText: string): Promise<RoutingDecision> { try { const { object } = await generateObject({ model: gateway(LIGHTWEIGHT_MODEL), schema: RoutingDecisionSchema, prompt: buildRoutingPrompt(messageText), temperature: 0, }); return RoutingDecisionSchema.parse(object); } catch (error) { console.error("Routing agent error:", error); return { model: DEFAULT_MODEL, excludeTools: undefined, reason: "routing-error-fallback", }; } } // Main function export async function getRoutingDecision( body: ChatRequest ): Promise<RoutingDecision> { const { messages, model: requestedModel } = body; // Respect user selection if (requestedModel) { return { model: requestedModel, excludeTools: undefined, reason: "user-selected", }; } // Extract message text const messageText = extractLastMessageText(messages); // Try pattern-based routing first (fast) const patternResult = tryPatternBasedRouting(messageText); if (patternResult) return patternResult; // Fall back to LLM routing return performLLMRouting(messageText); }This approach:
- Keeps each function under 30 lines
- Improves testability (each helper can be tested independently)
- Follows single responsibility principle
- Makes the main function read like documentation
As per coding guidelines (functions under 50 lines, single responsibility).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/VercelChat/ChatInput.tsx(4 hunks)components/VercelChat/RoutingStatus.tsx(1 hunks)hooks/useVercelChat.ts(3 hunks)lib/chat/getExecute.ts(2 hunks)lib/chat/getRoutingDecision.ts(1 hunks)providers/VercelChatProvider.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
lib/**/*.ts
⚙️ CodeRabbit configuration file
lib/**/*.ts: For utility functions, ensure:
- Pure functions when possible
- Single responsibility per function
- Proper error handling
- Use TypeScript for type safety
- Avoid side effects
- Keep functions under 50 lines
- DRY: Consolidate similar logic into shared utilities
- KISS: Prefer simple, readable implementations over clever optimizations
Files:
lib/chat/getRoutingDecision.tslib/chat/getExecute.ts
components/**/*.tsx
⚙️ CodeRabbit configuration file
components/**/*.tsx: For React components, ensure:
- Single responsibility per component
- Proper prop interfaces (ISP)
- Use composition over inheritance
- Avoid prop drilling (use context or state management)
- Keep components under 200 lines
- Extract custom hooks for complex logic
- Use TypeScript interfaces for props
- DRY: Extract common UI patterns into reusable components
- KISS: Prefer simple component structure over complex abstractions
Files:
components/VercelChat/RoutingStatus.tsxcomponents/VercelChat/ChatInput.tsx
hooks/**/*.ts
⚙️ CodeRabbit configuration file
hooks/**/*.ts: For custom hooks, ensure:
- Single responsibility per hook
- Return consistent interface
- Handle edge cases and errors
- Use proper dependency arrays
- Keep hooks focused and reusable
- Follow naming convention (use prefix)
- DRY: Extract common hook logic into shared utilities
- KISS: Avoid complex hook compositions, prefer simple state management
Files:
hooks/useVercelChat.ts
providers/**/*.tsx
⚙️ CodeRabbit configuration file
providers/**/*.tsx: For React providers, ensure:
- Single responsibility per provider
- Proper context interface design
- Avoid prop drilling
- Use composition for complex state
- Handle loading and error states
- Keep providers focused and testable
- DRY: Extract common provider patterns into reusable hooks
- KISS: Avoid over-abstracting state management, keep it simple
Files:
providers/VercelChatProvider.tsx
🧬 Code graph analysis (3)
lib/chat/getRoutingDecision.ts (2)
lib/chat/types.ts (1)
ChatRequest(12-23)lib/consts.ts (2)
LIGHTWEIGHT_MODEL(43-43)DEFAULT_MODEL(41-41)
lib/chat/getExecute.ts (3)
lib/chat/getRoutingDecision.ts (1)
getRoutingDecision(20-91)lib/chat/types.ts (1)
ChatRequest(12-23)lib/chat/setupChatRequest.ts (1)
setupChatRequest(14-105)
components/VercelChat/ChatInput.tsx (2)
providers/VercelChatProvider.tsx (1)
useVercelChatContext(163-173)components/VercelChat/RoutingStatus.tsx (1)
RoutingStatus(11-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Vercel Agent Review
- GitHub Check: code-review
🔇 Additional comments (8)
providers/VercelChatProvider.tsx (1)
88-88: LGTM! Clean integration of routingStatus.The pass-through of
routingStatusfrom the hook to the context value follows the established provider pattern correctly. The implementation maintains consistency with other context fields.Also applies to: 130-130
hooks/useVercelChat.ts (1)
234-235: LGTM! Proper state clearing and exposure.The
routingStatusis correctly cleared inonFinishand properly exposed in the hook's return value, maintaining a consistent interface for consumers.Also applies to: 366-366
lib/chat/getExecute.ts (1)
55-55: LGTM! Correct application of routing decision.The use of
routedBodyinstead ofbodycorrectly applies the routing decision to the chat configuration. This ensures the routed model and tool exclusions are properly propagated.components/VercelChat/ChatInput.tsx (2)
36-43: LGTM! Clean context integration.The addition of
routingStatusto the destructured context values follows the established pattern and avoids prop drilling, as recommended in the coding guidelines.
71-79: LGTM! Proper conditional rendering of routing status.The routing status UI is correctly rendered only when present, with appropriate props and styling. The centered positioning provides good UX feedback during routing.
components/VercelChat/RoutingStatus.tsx (1)
27-33: LGTM! Clean conditional rendering logic.The conditional rendering of the loader (during "analyzing") and model (when "complete" and provided) is well-structured and provides appropriate visual feedback.
lib/chat/getRoutingDecision.ts (2)
25-32: LGTM! Efficient early return for user-selected models.Respecting the user's explicit model choice and bypassing routing is the correct behavior. This optimization avoids unnecessary processing when routing isn't needed.
82-90: LGTM! Robust error handling with safe fallback.The try-catch block properly handles routing errors and provides a safe fallback to the default model, ensuring the system remains functional even when routing fails. The error logging helps with debugging.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
providers/VercelChatProvider.tsx (1)
25-30: Extract the routingStatus type to eliminate duplication.The inline type definition for
routingStatusduplicates the type fromhooks/useVercelChat.ts, violating the DRY principle emphasized in the coding guidelines for providers.As per coding guidelines.
A past review already suggested extracting this to a shared types file. Please address that comment to consolidate the type definition.
🧹 Nitpick comments (3)
lib/agents/routingAgent.ts (2)
5-10: Consider making theagentfield required.The
agentfield is currently optional, but examininglib/chat/getRoutingDecision.tsshows all code paths return a decision with anagentvalue. Making it optional adds unnecessary null-checking burden on consumers without providing clear benefits.Apply this diff to make
agentrequired:const RoutingDecisionSchema = z.object({ - agent: z.enum(["googleSheetsAgent", "generalAgent"]).optional(), + agent: z.enum(["googleSheetsAgent", "generalAgent"]), reason: z.string().optional(), });
16-23: Enhance routing instructions with specific agent descriptions.The instructions are generic and don't specify what each agent handles. More specific instructions would improve routing accuracy and make the agent's behavior more predictable and maintainable.
Apply this diff to add specific agent descriptions:
export const routingAgent = new ToolLoopAgent({ model: LIGHTWEIGHT_MODEL, - instructions: `You are a fast routing agent that determines which specialized agent should handle user messages.`, + instructions: `You are a fast routing agent that determines which specialized agent should handle user messages. + +Available agents: +- googleSheetsAgent: Handles Google Sheets, spreadsheets, CSV, Excel, and tabular data operations +- generalAgent: Handles all other general conversational queries + +Route to the most appropriate agent based on the message content.`, output: Output.object({ schema: RoutingDecisionSchema, }), stopWhen: stepCountIs(1), // Single step for speed });lib/chat/getRoutingDecision.ts (1)
14-21: Add validation for empty messages array.The function accesses
messages[messages.length - 1]without checking if the array is empty. While optional chaining handles the undefined case, explicit validation improves code clarity and prevents unexpected routing decisions on empty input.Apply this diff to add validation:
export async function getRoutingDecision( body: ChatRequest ): Promise<RoutingDecision> { const { messages } = body; + // Validate messages array + if (!messages.length) { + return { + agent: "generalAgent", + reason: "no-messages", + }; + } + // Extract last user message for routing const lastMessage = messages[messages.length - 1];As per coding guidelines (proper error handling).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
components/VercelChat/ChatInput.tsx(4 hunks)components/VercelChat/RoutingStatus.tsx(1 hunks)hooks/useVercelChat.ts(3 hunks)lib/agents/routingAgent.ts(1 hunks)lib/chat/getExecute.ts(2 hunks)lib/chat/getRoutingDecision.ts(1 hunks)providers/VercelChatProvider.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- lib/chat/getExecute.ts
- hooks/useVercelChat.ts
- components/VercelChat/ChatInput.tsx
- components/VercelChat/RoutingStatus.tsx
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.ts
⚙️ CodeRabbit configuration file
lib/**/*.ts: For utility functions, ensure:
- Pure functions when possible
- Single responsibility per function
- Proper error handling
- Use TypeScript for type safety
- Avoid side effects
- Keep functions under 50 lines
- DRY: Consolidate similar logic into shared utilities
- KISS: Prefer simple, readable implementations over clever optimizations
Files:
lib/agents/routingAgent.tslib/chat/getRoutingDecision.ts
providers/**/*.tsx
⚙️ CodeRabbit configuration file
providers/**/*.tsx: For React providers, ensure:
- Single responsibility per provider
- Proper context interface design
- Avoid prop drilling
- Use composition for complex state
- Handle loading and error states
- Keep providers focused and testable
- DRY: Extract common provider patterns into reusable hooks
- KISS: Avoid over-abstracting state management, keep it simple
Files:
providers/VercelChatProvider.tsx
🧬 Code graph analysis (2)
lib/agents/routingAgent.ts (1)
lib/consts.ts (1)
LIGHTWEIGHT_MODEL(43-43)
lib/chat/getRoutingDecision.ts (2)
lib/chat/types.ts (1)
ChatRequest(12-23)lib/agents/routingAgent.ts (2)
RoutingDecision(10-10)routingAgent(16-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Vercel Agent Review
- GitHub Check: code-review
🔇 Additional comments (2)
providers/VercelChatProvider.tsx (1)
88-88: LGTM on routingStatus integration.The routing status is correctly destructured from
useVercelChatand propagated through the context value, following proper provider patterns and single responsibility principles.Also applies to: 130-130
lib/chat/getRoutingDecision.ts (1)
62-69: LGTM on error handling strategy.The error handling correctly catches routing failures, logs for debugging, and falls back to the general agent. This ensures the routing decision never blocks the chat flow, aligning with the PR's objective of fast, non-blocking routing.
There was a problem hiding this comment.
🚨 Critical — lib/agents/routingAgent.ts:1
import { ToolLoopAgent, Output, stepCountIs } from "ai";
These imports may not exist in ai@6.0.0-beta.99; if unavailable this will crash at runtime. Please verify and use the documented agents API (or generateText + ToolSet) or upgrade the SDK accordingly.
const routedBody: ChatRequest = {
...body,
// Agent routing is handled separately - body remains unchanged for now
};
The routing decision isn’t applied; routedBody is identical to body. Propagate the chosen agent/model into chatConfig or remove this for now (YAGNI).
onData: (dataPart) => {
if (dataPart.type === "data-routing-status" && dataPart.data) {
Confirm useChat supports onData with custom type parts; otherwise this callback won’t fire and routingStatus will never update. If unsupported, use onResponse/onTextDelta or a custom SSE handler.
✨ Improvement — hooks/useVercelChat.ts (error cleanup)
onError: (e) => {
console.error("An error occurred, please try again!", e);
toast.error("An error occurred, please try again!");
Clear routingStatus on error to avoid a stale “analyzing/complete” badge persisting after failures.
Summary
- Adds routing UI/status; overall direction looks good.
- Address the import/API risk and ensure the routing decision actually affects execution; also wire robust client-side updates and cleanup.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
lib/agents/routingAgent.ts (3)
5-10: Consider exporting the schema for validation reuse.The
RoutingDecisionSchemais kept internal while the type is exported. If other modules need to validate routing decisions at runtime (e.g., ingetRoutingDecision.tsline 30 where unsafe type assertion occurs), exporting the schema would enable proper validation viaRoutingDecisionSchema.safeParse().Apply this diff to export the schema:
-const RoutingDecisionSchema = z.object({ +export const RoutingDecisionSchema = z.object({ agent: z.enum(["googleSheetsAgent", "generalAgent"]).optional(), reason: z.string().optional(), });This would allow dependent modules to perform runtime validation instead of unsafe type assertions.
6-6: Consider extracting agent identifiers to shared constants.The agent enum values
"googleSheetsAgent"and"generalAgent"are hardcoded and likely referenced in multiple places. Extracting these to a shared constant (e.g.,AGENT_TYPESinlib/consts.ts) would improve maintainability and reduce the risk of typos.Example:
// In lib/consts.ts export const AGENT_TYPES = { GOOGLE_SHEETS: "googleSheetsAgent", GENERAL: "generalAgent", } as const;Then use:
z.enum([AGENT_TYPES.GOOGLE_SHEETS, AGENT_TYPES.GENERAL])
16-23: Agent instructions could be more specific about routing criteria.The instructions briefly describe the agent's role but don't explain what distinguishes
googleSheetsAgentfromgeneralAgent. More detailed instructions could improve routing accuracy, though this trades off with the "fast routing" objective.Consider enhancing the instructions:
export const routingAgent = new ToolLoopAgent({ model: LIGHTWEIGHT_MODEL, - instructions: `You are a fast routing agent that determines which specialized agent should handle customer messages.`, + instructions: `You are a fast routing agent that determines which specialized agent should handle customer messages. + +Route to "googleSheetsAgent" if the message involves: +- Google Sheets, spreadsheets, CSV, Excel, or tabular data operations +- Queries about rows, columns, cells, or data tables + +Route to "generalAgent" for all other queries.`, output: Output.object({ schema: RoutingDecisionSchema, }), stopWhen: stepCountIs(1), });lib/chat/getRoutingDecision.ts (1)
35-42: Error handling is appropriate.The error handling properly catches routing agent failures, logs for debugging, and returns a safe fallback. The
console.erroris a side effect, but logging errors is an acceptable practice for debugging and monitoring.Optional improvement: Consider using a structured logging library for production environments instead of
console.errorfor better observability.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
hooks/useVercelChat.ts(3 hunks)lib/agents/routingAgent.ts(1 hunks)lib/chat/getRoutingDecision.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- hooks/useVercelChat.ts
🧰 Additional context used
📓 Path-based instructions (1)
lib/**/*.ts
⚙️ CodeRabbit configuration file
lib/**/*.ts: For utility functions, ensure:
- Pure functions when possible
- Single responsibility per function
- Proper error handling
- Use TypeScript for type safety
- Avoid side effects
- Keep functions under 50 lines
- DRY: Consolidate similar logic into shared utilities
- KISS: Prefer simple, readable implementations over clever optimizations
Files:
lib/agents/routingAgent.tslib/chat/getRoutingDecision.ts
🧬 Code graph analysis (2)
lib/agents/routingAgent.ts (1)
lib/consts.ts (1)
LIGHTWEIGHT_MODEL(43-43)
lib/chat/getRoutingDecision.ts (2)
lib/chat/types.ts (1)
ChatRequest(12-23)lib/agents/routingAgent.ts (2)
RoutingDecision(10-10)routingAgent(16-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Vercel Agent Review
- GitHub Check: code-review
There was a problem hiding this comment.
Inline comments on changed lines:\n\n- lib/chat/getExecute.ts (await getRoutingDecision) — ⚡ Performance: This blocks chat init; run in parallel with setupChatRequest or add a short timeout and fall back.\n- lib/chat/getRoutingDecision.ts (lastMessage selection) —
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
hooks/useRoutingStatus.ts (1)
17-24: Consider memoizing handler functions for referential stability.The handler functions are recreated on every render, which could cause unnecessary re-renders in consuming components that include these handlers in dependency arrays.
Wrap handlers in
useCallback:+import { useState, useCallback } from "react"; - const handleRoutingData = (data: unknown) => { + const handleRoutingData = useCallback((data: unknown) => { const routingData = data as RoutingStatus; setRoutingStatus(routingData); - }; + }, []); - const clearRoutingStatus = () => { + const clearRoutingStatus = useCallback(() => { setRoutingStatus(null); - }; + }, []);As per coding guidelines: This improves the hook's adherence to React best practices and ensures consistent behavior for consumers.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hooks/useRoutingStatus.ts(1 hunks)hooks/useVercelChat.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- hooks/useVercelChat.ts
🧰 Additional context used
📓 Path-based instructions (1)
hooks/**/*.ts
⚙️ CodeRabbit configuration file
hooks/**/*.ts: For custom hooks, ensure:
- Single responsibility per hook
- Return consistent interface
- Handle edge cases and errors
- Use proper dependency arrays
- Keep hooks focused and reusable
- Follow naming convention (use prefix)
- DRY: Extract common hook logic into shared utilities
- KISS: Avoid complex hook compositions, prefer simple state management
Files:
hooks/useRoutingStatus.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
lib/messages/getLastMessage.ts (1)
7-19: Consider preserving original casing for routing accuracy.Lowercasing the text (line 18) may degrade routing decisions by losing proper nouns, acronyms, and other case-sensitive information. The AI model can handle case normalization internally if needed.
Based on past review feedback.
lib/agents/routingAgent/writeRoutingStatus.ts (1)
23-32: Static id may cause status collisions in concurrent scenarios.Using
id: "routing-status"means all routing status updates reconcile to the same id. If multiple messages stream concurrently, their status updates will overwrite each other. Consider appending a message or session identifier.Based on past review feedback.
export function writeRoutingStatus( writer: UIMessageStreamWriter | undefined, status: "analyzing" | "complete", message: string, agent?: string, - reason?: string + reason?: string, + messageId?: string ): void { if (!writer) return; writer.write({ type: ROUTING_STATUS_DATA_TYPE, - id: "routing-status", + id: messageId ? `routing-status-${messageId}` : "routing-status", data: { status, message, agent, reason, }, }); }lib/chat/setupChatRequest.ts (1)
29-29: Blocking await contradicts PR objective of not slowing down initial chat.The
await getRoutingDecision(body, writer)blocks setupChatRequest and adds model inference latency to every chat request, yet the routing decision isn't used. This defeats the PR's goal of adding routing "without slowing down the initial chat."Apply this diff to make it non-blocking:
- await getRoutingDecision(body, writer); + // Fire-and-forget routing for status updates only + getRoutingDecision(body, writer).catch((error) => { + console.error("Background routing failed:", error); + });Or conditionally execute only when a writer is present:
- await getRoutingDecision(body, writer); + // Only route when streaming to UI + if (writer) { + getRoutingDecision(body, writer).catch((error) => { + console.error("Routing status update failed:", error); + }); + }
🧹 Nitpick comments (5)
lib/agents/routingAgent/routingAgent.ts (2)
5-8: Both schema fields are optional, allowing empty routing decisions.Since both
agentandreasonare optional, the schema permits returning{}, which downstream code must handle explicitly. Consider making at leastagentrequired with a default value in the schema to ensure predictable behavior.Apply this diff to make
agentrequired with a default:const RoutingDecisionSchema = z.object({ - agent: z.enum(["googleSheetsAgent", "generalAgent"]).optional(), + agent: z.enum(["googleSheetsAgent", "generalAgent"]).default("generalAgent"), reason: z.string().optional(), });
16-23: Routing instructions are minimal and may impact decision quality.The single-sentence instruction (line 18) provides no context about when to route to
googleSheetsAgentvsgeneralAgent. Enriching the instructions with routing criteria could improve decision accuracy.Consider expanding the instructions:
export const routingAgent = new ToolLoopAgent({ model: FAST_MODEL, - instructions: `You are a fast routing agent that determines which specialized agent should handle customer messages.`, + instructions: `You are a fast routing agent that determines which specialized agent should handle customer messages. + +Route to googleSheetsAgent if the message involves: spreadsheet operations, Google Sheets data, or tabular data management. +Route to generalAgent for all other requests: artist analysis, fan engagement, marketing strategy, etc. + +Respond with your routing decision immediately.`, output: Output.object({ schema: RoutingDecisionSchema, }), stopWhen: stepCountIs(1), });lib/agents/routingAgent/getRoutingDecision.ts (1)
18-21: Consider early return when writer is undefined to skip unnecessary work.If
writeris undefined, the routing status updates are no-ops. You could optimize by returning early with a default decision when no UI updates are needed, avoiding the model inference cost entirely.export async function getRoutingDecision( body: ChatRequest, writer?: UIMessageStreamWriter ): Promise<RoutingDecision> { + // Skip routing inference if no UI writer present + if (!writer) { + return { + agent: "generalAgent", + reason: "no-ui-writer", + }; + } + const { messages } = body;lib/agents/routingAgent/index.ts (1)
1-2: Redundant exports ofroutingAgentmay cause confusion.Line 1 exports
routingAgentas default, and line 2 exports it as a named export. While valid, supporting both import patterns can lead to inconsistent usage across the codebase.Consider choosing one pattern:
-export { routingAgent as default, type RoutingDecision } from "./routingAgent"; -export { routingAgent } from "./routingAgent"; +export { routingAgent, type RoutingDecision } from "./routingAgent"; export { getRoutingDecision } from "./getRoutingDecision"; export { type RoutingStatusData } from "./writeRoutingStatus";Or if a default export is preferred, remove the named export on line 2.
components/VercelChat/MessageParts.tsx (1)
40-42: Type assertion could be replaced with type guard for safety.The type assertion
part.data as RoutingStatusDataassumes the shape matches. Adding a runtime type guard would prevent potential runtime errors if the data shape is incorrect.// Add type guard function function isRoutingStatusData(data: unknown): data is RoutingStatusData { return ( typeof data === "object" && data !== null && "status" in data && "message" in data ); } // Use in component if (type === ROUTING_STATUS_DATA_TYPE) { const routingData = part.data; if (!isRoutingStatusData(routingData)) { console.error("Invalid routing status data:", routingData); return null; } return ( <RoutingStatus key={key} status={routingData.status} message={routingData.message} agent={routingData.agent} /> ); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
components/VercelChat/MessageParts.tsx(1 hunks)components/VercelChat/message.tsx(2 hunks)lib/agents/routingAgent/getRoutingDecision.ts(1 hunks)lib/agents/routingAgent/index.ts(1 hunks)lib/agents/routingAgent/routingAgent.ts(1 hunks)lib/agents/routingAgent/writeRoutingStatus.ts(1 hunks)lib/chat/setupChatRequest.ts(3 hunks)lib/consts.ts(1 hunks)lib/messages/getLastMessage.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/VercelChat/message.tsx
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.ts
⚙️ CodeRabbit configuration file
lib/**/*.ts: For utility functions, ensure:
- Pure functions when possible
- Single responsibility per function
- Proper error handling
- Use TypeScript for type safety
- Avoid side effects
- Keep functions under 50 lines
- DRY: Consolidate similar logic into shared utilities
- KISS: Prefer simple, readable implementations over clever optimizations
Files:
lib/messages/getLastMessage.tslib/agents/routingAgent/index.tslib/agents/routingAgent/getRoutingDecision.tslib/consts.tslib/agents/routingAgent/routingAgent.tslib/agents/routingAgent/writeRoutingStatus.tslib/chat/setupChatRequest.ts
components/**/*.tsx
⚙️ CodeRabbit configuration file
components/**/*.tsx: For React components, ensure:
- Single responsibility per component
- Proper prop interfaces (ISP)
- Use composition over inheritance
- Avoid prop drilling (use context or state management)
- Keep components under 200 lines
- Extract custom hooks for complex logic
- Use TypeScript interfaces for props
- DRY: Extract common UI patterns into reusable components
- KISS: Prefer simple component structure over complex abstractions
Files:
components/VercelChat/MessageParts.tsx
🧬 Code graph analysis (5)
lib/agents/routingAgent/getRoutingDecision.ts (4)
lib/chat/types.ts (1)
ChatRequest(12-23)lib/agents/routingAgent/routingAgent.ts (2)
RoutingDecision(10-10)routingAgent(16-23)lib/agents/routingAgent/writeRoutingStatus.ts (1)
writeRoutingStatus(14-33)lib/messages/getLastMessage.ts (1)
getLastMessageText(7-19)
components/VercelChat/MessageParts.tsx (5)
lib/consts.ts (1)
ROUTING_STATUS_DATA_TYPE(45-45)components/VercelChat/RoutingStatus.tsx (1)
RoutingStatus(11-37)components/reasoning/EnhancedReasoning.tsx (1)
EnhancedReasoning(37-132)components/actions.tsx (2)
Actions(15-19)Action(26-65)components/VercelChat/ToolComponents.tsx (2)
getToolCallComponent(104-285)getToolResultComponent(287-539)
lib/agents/routingAgent/routingAgent.ts (2)
lib/agents/routingAgent/index.ts (3)
RoutingDecision(1-1)routingAgent(1-1)routingAgent(2-2)lib/consts.ts (1)
FAST_MODEL(42-42)
lib/agents/routingAgent/writeRoutingStatus.ts (2)
lib/agents/routingAgent/index.ts (1)
RoutingStatusData(4-4)lib/consts.ts (1)
ROUTING_STATUS_DATA_TYPE(45-45)
lib/chat/setupChatRequest.ts (2)
lib/chat/types.ts (2)
ChatRequest(12-23)ChatConfig(25-42)lib/agents/routingAgent/getRoutingDecision.ts (1)
getRoutingDecision(12-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Vercel Agent Review
- GitHub Check: code-review
🔇 Additional comments (3)
lib/agents/routingAgent/getRoutingDecision.ts (1)
12-66: Well-structured routing logic with proper error handling.The function correctly handles edge cases with fallback decisions and writes persistent status updates to the UI. Error handling is comprehensive, and the message truncation (200 chars) is reasonable for routing.
components/VercelChat/MessageParts.tsx (1)
26-137: Well-structured component with clear delegation to specialized renderers.The component maintains single responsibility by delegating to specialized sub-components (RoutingStatus, EnhancedReasoning, MessageFileViewer, etc.) for each part type. The defensive optional chaining and proper TypeScript typing are good practices.
lib/consts.ts (1)
42-42: The review comment is outdated—GPT-5-nano is now a valid OpenAI model.The original concern was accurate for March 2025 (GPT-5 didn't exist then), but circumstances have changed. GPT-5 with nano and mini versions is now available, released August 7, 2025. The model identifier format
"openai/gpt-5-nano"is correct for the Vercel AI library (evident from the consistent use of the"openai/"prefix across other constants like"openai/gpt-5-mini"and the import ofToolLoopAgentfrom the"ai"library). The code requires no changes.
…etmantech/myc-3473-add-a-routing-agent-without-slowing-down-the-initial-chat
There was a problem hiding this comment.
🚨 I reviewed the latest diff and left notes focused on high-severity issues only.
⚠️ Logic — components/VercelChat/MessageParts.tsx:129: Elements returned by getToolCallComponent/getToolResultComponent lack a key. Add a stable key to avoid React reconciliation issues in mapped lists.⚠️ Logic — lib/agents/routingAgent/routingAgent.ts:6: The agent enum is very restrictive (googleSheetsAgent|generalAgent). If routing returns a new agent, schema validation will drop it. Consider widening to z.string() or include all valid agents.⚠️ Logic — lib/agents/routingAgent/writeRoutingStatus.ts:25: Using a static id ("routing-status") can collide across messages; include the message id or a unique suffix to ensure proper UI reconciliation.⚠️ Logic — lib/messages/getLastMessage.ts:13: Lowercasing user input may harm routing intent for proper nouns/acronyms. Consider preserving case or normalizing more selectively.
No previously raised issues to mark as resolved were found in this PR.
Summary: The routing integration looks good overall. Tighten the schema to avoid unintended fallbacks, add missing keys in the UI map, and make the routing status id unique to prevent collisions.
There was a problem hiding this comment.
⚠️ Logic: Add stable React keys to tool UI branches incomponents/VercelChat/MessageParts.tsxto avoid reconciliation issues.⚠️ Logic: Use a uniqueidfor routing status writes inlib/agents/routingAgent/writeRoutingStatus.tsto prevent collisions.⚠️ Logic: WrapgetRoutingDecisioninlib/chat/setupChatRequest.tswith error handling so routing failures don’t block chat setup.
Everything else looks solid and aligns with KISS/DRY refactor goals.
| if (isToolUIPart(part)) { | ||
| const { state } = part as ToolUIPart; | ||
| if (state !== "output-available") { | ||
| return getToolCallComponent(part as ToolUIPart); |
There was a problem hiding this comment.
|
|
||
| writer.write({ | ||
| type: ROUTING_STATUS_DATA_TYPE, | ||
| id: "routing-status", // Use ID for reconciliation |
There was a problem hiding this comment.
| timezone, | ||
| } = body; | ||
|
|
||
| await getRoutingDecision(body, writer); |
There was a problem hiding this comment.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
components/VercelChat/MessageParts.tsx (1)
26-137: Consider strategy pattern for improved extensibility.The current if/else chain for handling different part types works but could become harder to maintain as more part types are added. Consider using a strategy pattern or component registry for cleaner extensibility.
Here's an example approach:
Create a new file
lib/messagePartRenderers.tsx:type PartRenderer = (props: { part: any; key: string; message: UIMessage; status: ChatStatus; partIndex: number; totalParts: number; mode: "view" | "edit"; setMode: Dispatch<SetStateAction<"view" | "edit">>; setMessages: UseChatHelpers<UIMessage>["setMessages"]; reload: () => void; }) => JSX.Element | null; export const partRenderers: Record<string, PartRenderer> = { [ROUTING_STATUS_DATA_TYPE]: ({ part, key }) => { const routingData = part.data as RoutingStatusData; return <RoutingStatus key={key} {...routingData} />; }, reasoning: ({ part, key, status, partIndex, totalParts }) => ( <EnhancedReasoning key={key} content={part.text} isStreaming={status === "streaming" && partIndex === totalParts - 1} defaultOpen={true} /> ), // ... other renderers };Then simplify the component:
const renderer = partRenderers[type]; if (renderer) { return renderer({ part, key, message, status, partIndex, totalParts: message.parts.length, mode, setMode, setMessages, reload }); }This makes adding new part types a matter of registering a new renderer rather than modifying the core component logic.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.github/workflows/eval.yamlis excluded by none and included by none
📒 Files selected for processing (1)
components/VercelChat/MessageParts.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
components/**/*.tsx
⚙️ CodeRabbit configuration file
components/**/*.tsx: For React components, ensure:
- Single responsibility per component
- Proper prop interfaces (ISP)
- Use composition over inheritance
- Avoid prop drilling (use context or state management)
- Keep components under 200 lines
- Extract custom hooks for complex logic
- Use TypeScript interfaces for props
- DRY: Extract common UI patterns into reusable components
- KISS: Prefer simple component structure over complex abstractions
Files:
components/VercelChat/MessageParts.tsx
🧬 Code graph analysis (1)
components/VercelChat/MessageParts.tsx (5)
lib/consts.ts (1)
ROUTING_STATUS_DATA_TYPE(45-45)components/VercelChat/RoutingStatus.tsx (1)
RoutingStatus(11-37)components/reasoning/EnhancedReasoning.tsx (1)
EnhancedReasoning(37-132)components/actions.tsx (2)
Actions(15-19)Action(26-65)components/VercelChat/ToolComponents.tsx (2)
getToolCallComponent(104-285)getToolResultComponent(287-539)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Vercel Agent Review
- GitHub Check: code-review
- GitHub Check: Run evaluations
🔇 Additional comments (3)
components/VercelChat/MessageParts.tsx (3)
17-24: LGTM! Clean interface definition.The
MessagePartsPropsinterface is well-structured with clear, focused props that follow TypeScript best practices and the Interface Segregation Principle.
52-64: LGTM! Reasoning part handling is well-implemented.The reasoning part rendering correctly uses the computed
key, properly detects streaming state, and appropriately delegates to theEnhancedReasoningcomponent.
66-68: LGTM! Clean delegation to MessageFileViewer.The file part handling is simple, correct, and follows composition principles well.
| {message.parts?.map((part, partIndex) => { | ||
| const { type } = part; | ||
| const key = `message-${message.id}-part-${partIndex}`; | ||
|
|
||
| if (type === ROUTING_STATUS_DATA_TYPE) { | ||
| const routingData = part.data as RoutingStatusData; | ||
| return ( | ||
| <RoutingStatus | ||
| key={ROUTING_STATUS_DATA_TYPE} | ||
| status={routingData.status} | ||
| message={routingData.message} | ||
| agent={routingData.agent} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| if (type === "reasoning") { | ||
| return ( | ||
| <EnhancedReasoning | ||
| key={key} | ||
| className="w-full" | ||
| content={part.text} | ||
| isStreaming={ | ||
| status === "streaming" && partIndex === message.parts.length - 1 | ||
| } | ||
| defaultOpen={true} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| if (type === "file") { | ||
| return <MessageFileViewer key={key} part={part} />; | ||
| } | ||
|
|
||
| if (type === "text") { | ||
| const isLastMessage = | ||
| message.role === "assistant" && | ||
| status !== "streaming" && | ||
| partIndex === message.parts.length - 1; | ||
|
|
||
| if (mode === "view") { | ||
| return ( | ||
| <div key={key}> | ||
| <ViewingMessage message={message} partText={part?.text || ""} /> | ||
| <Actions | ||
| className={cn( | ||
| "mt-0.5 gap-0.5 opacity-0 pointer-events-none group-hover:opacity-100 group-hover:pointer-events-auto transition-opacity", | ||
| { | ||
| "justify-start": message.role === "assistant", | ||
| "justify-end": message.role === "user", | ||
| } | ||
| )} | ||
| > | ||
| {message.role === "user" && ( | ||
| <Action | ||
| onClick={() => setMode("edit")} | ||
| label="Edit" | ||
| tooltip="Edit message" | ||
| > | ||
| <Pencil className="!w-3 !h-3" /> | ||
| </Action> | ||
| )} | ||
| {isLastMessage && ( | ||
| <Action | ||
| onClick={() => reload()} | ||
| label="Retry" | ||
| tooltip="Regenerate this response" | ||
| > | ||
| <RefreshCcwIcon className="!w-3 !h-3" /> | ||
| </Action> | ||
| )} | ||
| <CopyAction text={part?.text || ""} /> | ||
| </Actions> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| if (mode === "edit") { | ||
| return ( | ||
| <EditingMessage | ||
| key={key} | ||
| message={message} | ||
| setMode={setMode} | ||
| setMessages={setMessages} | ||
| reload={reload} | ||
| /> | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| if (isToolUIPart(part)) { | ||
| const { state } = part as ToolUIPart; | ||
| if (state !== "output-available") { | ||
| return getToolCallComponent(part as ToolUIPart); | ||
| } else { | ||
| return getToolResultComponent(part as ToolUIPart); | ||
| } | ||
| } | ||
| })} |
There was a problem hiding this comment.
Add explicit return for unhandled part types.
The map function doesn't have complete return coverage. If a part's type doesn't match any of the checked conditions (routing status, reasoning, file, text, or tool UI), the function implicitly returns undefined for that iteration. This violates React best practices and could cause rendering issues or warnings.
Add an explicit fallback at the end of the map function:
if (isToolUIPart(part)) {
const toolPart = part as ToolUIPart;
const { state } = toolPart;
if (state !== "output-available") {
return <div key={key}>{getToolCallComponent(toolPart)}</div>;
} else {
return <div key={key}>{getToolResultComponent(toolPart)}</div>;
}
}
+
+ // Log unhandled part types for debugging
+ console.warn(`Unhandled message part type: ${type}`, part);
+ return null;
})}
</div>
);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/VercelChat/MessageParts.tsx around lines 36 to 134, the parts.map
callback can implicitly return undefined for unknown part types which may
produce React warnings; add an explicit fallback return (e.g., return null) at
the end of the map callback so every branch returns a valid React node, using
the existing key variable for the returned element if applicable to preserve
stable keys.
| if (type === "text") { | ||
| const isLastMessage = | ||
| message.role === "assistant" && | ||
| status !== "streaming" && | ||
| partIndex === message.parts.length - 1; | ||
|
|
||
| if (mode === "view") { | ||
| return ( | ||
| <div key={key}> | ||
| <ViewingMessage message={message} partText={part?.text || ""} /> | ||
| <Actions | ||
| className={cn( | ||
| "mt-0.5 gap-0.5 opacity-0 pointer-events-none group-hover:opacity-100 group-hover:pointer-events-auto transition-opacity", | ||
| { | ||
| "justify-start": message.role === "assistant", | ||
| "justify-end": message.role === "user", | ||
| } | ||
| )} | ||
| > | ||
| {message.role === "user" && ( | ||
| <Action | ||
| onClick={() => setMode("edit")} | ||
| label="Edit" | ||
| tooltip="Edit message" | ||
| > | ||
| <Pencil className="!w-3 !h-3" /> | ||
| </Action> | ||
| )} | ||
| {isLastMessage && ( | ||
| <Action | ||
| onClick={() => reload()} | ||
| label="Retry" | ||
| tooltip="Regenerate this response" | ||
| > | ||
| <RefreshCcwIcon className="!w-3 !h-3" /> | ||
| </Action> | ||
| )} | ||
| <CopyAction text={part?.text || ""} /> | ||
| </Actions> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| if (mode === "edit") { | ||
| return ( | ||
| <EditingMessage | ||
| key={key} | ||
| message={message} | ||
| setMode={setMode} | ||
| setMessages={setMessages} | ||
| reload={reload} | ||
| /> | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Add explicit return for text parts and verify isLastMessage logic.
Two concerns:
-
Missing return statement: If
modeis neither"view"nor"edit", the function implicitly returnsundefined. While the TypeScript types suggest this shouldn't happen, defensive programming would add an explicit fallback. -
Verify isLastMessage logic: Lines 71-74 check if this is the last part of the current message. Is this the intended behavior, or should it check if this is the last message in the entire conversation? The retry action typically applies to the last assistant message in the conversation, not the last part of a message.
For issue #1, consider adding an explicit return:
if (mode === "edit") {
return (
<EditingMessage
key={key}
message={message}
setMode={setMode}
setMessages={setMessages}
reload={reload}
/>
);
}
+
+ // Fallback for unexpected mode
+ return null;
}For issue #2, please verify the intended behavior. If it should check the last message in the conversation:
🏁 Script executed:
#!/bin/bash
# Search for how other components determine "last message" behavior
rg -n -C3 "last.*message|reload" --type=tsx --type=ts -g '!node_modules'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for reload/retry patterns with correct file type
rg -n -C3 "reload|isLastMessage" --type=typescript -g '!node_modules'Length of output: 34
🏁 Script executed:
#!/bin/bash
# Also check the function signature and return type of the parent function
rg -n -B10 "if.*type.*===.*text" components/VercelChat/MessageParts.tsx | head -50Length of output: 255
🏁 Script executed:
#!/bin/bash
# Search for how messages array is used to understand the conversation context
rg -n "message.parts|partIndex" components/VercelChat/MessageParts.tsxLength of output: 256
🏁 Script executed:
#!/bin/bash
# Get the full function signature and understand return type
cat components/VercelChat/MessageParts.tsx | head -50Length of output: 1771
🏁 Script executed:
#!/bin/bash
# Find where reload comes from and what it does
rg -n "reload" components/VercelChat/MessageParts.tsx -B5 -A2Length of output: 962
🏁 Script executed:
#!/bin/bash
# Check if this component receives a "messages" array that would let it know if this message is last in conversation
rg -n "interface|type.*Props|export.*function|const.*MessageParts" components/VercelChat/MessageParts.tsx | head -20Length of output: 66
🏁 Script executed:
#!/bin/bash
# Find all possible part types handled and unhandled
rg -n 'type === "' components/VercelChat/MessageParts.tsxLength of output: 106
🏁 Script executed:
#!/bin/bash
# Check the UIMessage/part type definition to see all possible types
rg -n "type.*ToolUIPart|UIMessage" components/VercelChat/MessageParts.tsx -A5Length of output: 587
🏁 Script executed:
#!/bin/bash
# See the complete if/if/if chain to check for all branches
sed -n '36,130p' components/VercelChat/MessageParts.tsxLength of output: 3094
Add explicit return for text parts when mode is unexpected; clarify isLastMessage semantics.
Two issues verified:
- Missing return for text mode edge case (lines 70–124): If
type === "text"butmodeis neither"view"nor"edit", the callback returnsundefined. While TypeScript constrainsmodeto these two values, defensive programming should handle unexpected states:
if (mode === "edit") {
return (
<EditingMessage
key={key}
message={message}
setMode={setMode}
setMessages={setMessages}
reload={reload}
/>
);
}
+
+ return null;
}- isLastMessage checks last part, not last message (lines 71–74): The retry button appears when
partIndex === message.parts.length - 1, meaning the last part of the current message. However, the component receives no prop indicating whether this message is the last in the conversation. If the intended behavior is "retry only regenerates the final assistant message," the component needs either:- An
isLastMessageInConversationprop from the parent, or - Documentation clarifying this is intentional (retry regenerates the last part of any assistant message)
- An
| if (isToolUIPart(part)) { | ||
| const { state } = part as ToolUIPart; | ||
| if (state !== "output-available") { | ||
| return getToolCallComponent(part as ToolUIPart); | ||
| } else { | ||
| return getToolResultComponent(part as ToolUIPart); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add React keys to tool component returns and reduce type casting.
Two issues:
-
Missing React keys (critical): Lines 129 and 131 return JSX elements without the
keyprop. React requires unique keys for elements in a list/map to properly track component identity and optimize re-renders. -
Repeated type casting: The
part as ToolUIPartcast is repeated three times (lines 127, 129, 131), violating DRY principles.
Apply this diff to fix both issues:
if (isToolUIPart(part)) {
- const { state } = part as ToolUIPart;
+ const toolPart = part as ToolUIPart;
+ const { state } = toolPart;
if (state !== "output-available") {
- return getToolCallComponent(part as ToolUIPart);
+ return <div key={key}>{getToolCallComponent(toolPart)}</div>;
} else {
- return getToolResultComponent(part as ToolUIPart);
+ return <div key={key}>{getToolResultComponent(toolPart)}</div>;
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/VercelChat/MessageParts.tsx around lines 126 to 133, replace the
repeated casts and missing React keys by first assigning const toolPart = part
as ToolUIPart and then return the tool components using that single variable,
adding a stable unique key prop to each returned element (e.g. key={toolPart.id}
— if id is not available fall back to a composite like
key={`${toolPart.tool}-${toolPart.state}`} or another stable identifier). This
removes repeated type casts and ensures each returned JSX element in the list
has a unique key.
There was a problem hiding this comment.
Inline notes on changed lines:\n\n- 🔒 Security — components/VercelChat/MessageParts.tsx:36: Avoid logging full chat state to the console; this can leak sensitive user data and adds noise.\n-
| const shouldHide = | ||
| routingData.status === "complete" && | ||
| message.parts?.length === 1 && | ||
| messages[2]?.parts?.[0]?.data?.status === "complete"; |
There was a problem hiding this comment.
| messages[2]?.parts?.[0]?.data?.status === "complete"; | |
| messages.some((msg) => { | |
| const otherPart = msg.parts?.[0]; | |
| return ( | |
| msg.id !== message.id && | |
| otherPart && | |
| "data" in otherPart && | |
| (otherPart.data as RoutingStatusData)?.status === "complete" | |
| ); | |
| }); |
Hardcoded array index for checking routing status will fail or behave unpredictably with different message counts.
View Details
Analysis
Hardcoded array index breaks routing status hiding logic in multi-turn conversations
What fails: MessageParts.tsx line 48 uses hardcoded messages[2] to check for previous complete routing statuses, which fails in multi-turn conversations and doesn't reliably identify previous routing statuses.
How to reproduce:
- Start a new conversation and send a message to the AI
- Observe that a routing status message appears (e.g., "Determining optimal agent...")
- Send a second message and observe the routing status message for the second turn
- The routing status for turn 2 will be incorrectly checked against
messages[2], which contains the routing status from turn 1 (a completely different decision)
In conversations with 3+ turns, messages[2] no longer refers to any routing status related to the current message.
What happens vs expected behavior:
Current code:
const shouldHide =
routingData.status === "complete" &&
message.parts?.length === 1 &&
messages[2]?.parts?.[0]?.data?.status === "complete";This hardcoded index assumes:
- messages[2] will always be a complete routing status
- But in a 2nd user turn, messages[2] is a routing status from turn 1, not the previous routing status for the current message
- Result: The hiding logic checks the wrong message and doesn't work correctly
Fix implemented: Changed the logic to search through all messages for ANY OTHER message with a complete routing status, rather than checking a hardcoded position:
const shouldHide =
routingData.status === "complete" &&
message.parts?.length === 1 &&
messages.some((msg) => {
const otherPart = msg.parts?.[0];
return (
msg.id !== message.id &&
otherPart &&
"data" in otherPart &&
(otherPart.data as RoutingStatusData)?.status === "complete"
);
});This properly searches for ANY message (other than the current one) that has a complete routing status, making the hiding logic work correctly regardless of conversation length.
Related commit: 776226b ("attempting to hide") - the hardcoded index was added hastily without considering multi-turn conversations.
| timezone, | ||
| } = body; | ||
|
|
||
| await getRoutingDecision(body, writer); |
There was a problem hiding this comment.
| }: MessagePartsProps) { | ||
| return ( | ||
| <div className={cn("flex flex-col gap-4 w-full group")}> | ||
| {message.parts?.map((part, partIndex) => { |
There was a problem hiding this comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
lib/chat/setupChatRequest.ts (1)
29-29: Blocking call with unused return value degrades response time.This
awaitblocks the entire chat setup and incurs model cost, yet theRoutingDecisionresult is never captured or used to influence the configuration. If routing is only for streaming status updates viawriter, consider removingawaitto prevent it from slowing down the initial chat response. If the routing decision should influence model or tool selection, capture and use the return value.From the coding guidelines, functions should avoid unnecessary side effects and maintain single responsibility—currently, this mixes routing with configuration setup.
Consider one of these approaches:
Option 1: Fire-and-forget (if result isn't needed):
- await getRoutingDecision(body, writer); + getRoutingDecision(body, writer); // non-blockingOption 2: Use the result to influence config:
- await getRoutingDecision(body, writer); + const routingDecision = await getRoutingDecision(body, writer); + // Then use routingDecision.agent to modify tools, model, etc.
🧹 Nitpick comments (3)
lib/chat/setupChatRequest.ts (1)
15-106: Consider decomposing this function to improve maintainability.Per the coding guidelines, utility functions should maintain single responsibility and stay under 50 lines. Currently,
setupChatRequesthandles routing, model configuration, tool setup, system prompt building, and message conversion—multiple distinct concerns.Consider extracting smaller, focused functions such as:
buildChatConfig(body, routingDecision)resolveModelAndTools(body, nanoBananaConfig)buildMessagesWithDownloadBehavior(body, config)This would improve testability, readability, and adherence to SOLID principles.
components/VercelChat/MessageParts.tsx (2)
55-108: Add defensive return and verify retry button semantics.Two observations:
- Defensive programming: If
modeis neither"view"nor"edit", the function implicitly returnsundefined. While TypeScript constrains the type, adding an explicit fallback improves defensive coding:if (mode === "edit") { return ( <EditingMessage key={key} message={message} setMode={setMode} setMessages={setMessages} reload={reload} /> ); } + + return null; }
- Verify retry button logic (lines 56-59): The
isLastMessagevariable checks if this is the last part of the current message (partIndex === message.parts.length - 1), not the last message in the conversation. This means the retry button appears on every assistant message's final part, not just the conversation's final response.Is this the intended behavior? If retry should only appear on the conversation's final assistant message, the component needs an additional prop (e.g.,
isLastInConversation: boolean) from the parent.
111-119: Add consistent keys and fallback for unhandled part types.Two issues need attention:
- Key consistency for tool components: While
getToolCallComponentandgetToolResultComponentreturn elements with keys (verified in relevant code snippets), using the consistentkeyvariable across all map branches improves maintainability and ensures stable reconciliation:if (isToolUIPart(part)) { - const { state } = part as ToolUIPart; + const toolPart = part as ToolUIPart; + const { state } = toolPart; if (state !== "output-available") { - return getToolCallComponent(part as ToolUIPart); + return <div key={key}>{getToolCallComponent(toolPart)}</div>; } else { - return getToolResultComponent(part as ToolUIPart); + return <div key={key}>{getToolResultComponent(toolPart)}</div>; } }
- Missing fallback for unhandled types (line 119): If a part's type doesn't match any condition (reasoning, file, text, or tool UI), the map callback implicitly returns
undefined. This violates React best practices and could cause rendering issues:} } + + // Fallback for unknown part types + console.warn(`Unhandled message part type:`, part); + return null; })} </div>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/VercelChat/MessageParts.tsx(1 hunks)lib/chat/setupChatRequest.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.ts
⚙️ CodeRabbit configuration file
lib/**/*.ts: For utility functions, ensure:
- Pure functions when possible
- Single responsibility per function
- Proper error handling
- Use TypeScript for type safety
- Avoid side effects
- Keep functions under 50 lines
- DRY: Consolidate similar logic into shared utilities
- KISS: Prefer simple, readable implementations over clever optimizations
Files:
lib/chat/setupChatRequest.ts
components/**/*.tsx
⚙️ CodeRabbit configuration file
components/**/*.tsx: For React components, ensure:
- Single responsibility per component
- Proper prop interfaces (ISP)
- Use composition over inheritance
- Avoid prop drilling (use context or state management)
- Keep components under 200 lines
- Extract custom hooks for complex logic
- Use TypeScript interfaces for props
- DRY: Extract common UI patterns into reusable components
- KISS: Prefer simple component structure over complex abstractions
Files:
components/VercelChat/MessageParts.tsx
🧬 Code graph analysis (2)
lib/chat/setupChatRequest.ts (3)
lib/chat/types.ts (2)
ChatRequest(12-23)ChatConfig(25-42)lib/agents/routingAgent/index.ts (1)
getRoutingDecision(3-3)lib/agents/routingAgent/getRoutingDecision.ts (1)
getRoutingDecision(12-66)
components/VercelChat/MessageParts.tsx (4)
components/reasoning/EnhancedReasoning.tsx (1)
EnhancedReasoning(37-132)components/actions.tsx (2)
Actions(15-19)Action(26-65)components/VercelChat/ToolComponents.tsx (2)
getToolCallComponent(104-285)getToolResultComponent(287-539)components/VercelChat/message.tsx (1)
message(16-145)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Vercel Agent Review
- GitHub Check: code-review
- GitHub Check: Run evaluations
🔇 Additional comments (3)
lib/chat/setupChatRequest.ts (3)
7-7: LGTM: Imports support new routing functionality.The new imports are properly utilized in the function signature and routing decision call.
Also applies to: 13-13
15-18: LGTM: Clean signature extension.The optional
writerparameter maintains backward compatibility while enabling streaming status updates during routing.
75-75: LGTM: Quote consistency improvement.Normalizing to double quotes improves consistency with the rest of the codebase.
There was a problem hiding this comment.
Summary:\n\n- ✅ Resolved prior feedback: AnimatePresence placement, routedBody mismatch, lastMessage parts safety, routing-status id.\n- ⚡ One high-impact item: setupChatRequest calls routing but ignores the result (extra latency/cost). Remove it or plumb the decision into config before shipping.
There was a problem hiding this comment.
Inline review:
- ⚡ lib/chat/setupChatRequest.ts:26 — getRoutingDecision is awaited but the result is unused; remove it or thread the decision into config to avoid added latency/cost.
- ✨ components/VercelChat/MessageParts.tsx:111 — Returned elements lack a stable key; wrap with a keyed element or ensure helpers accept a key to prevent list reconciliation issues.
- ✨ lib/agents/routingAgent/routingAgent.ts:6 — Make 'agent' required or set a schema default (e.g., .default('generalAgent')) to avoid undefined and simplify downstream fallbacks.
⚠️ lib/agents/routingAgent/getRoutingDecision.ts:19 — Embedding raw text in a quoted prompt can break formatting with quotes/newlines; escape or use structured fields.
Summary: Flagged one perf/logic issue and two small improvements; no prior actionable human comments appeared to require resolution.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Refactor
Documentation