-
Notifications
You must be signed in to change notification settings - Fork 34
feat: Upgrade Vercel AI SDK to v5 #27
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
base: new-backend
Are you sure you want to change the base?
Conversation
I've upgraded the Vercel AI SDK from v4 to v5 in the `service` subfolder. Here are the changes I made: - Updated `ai`, `@ai-sdk/google`, and `zod` packages to the latest versions. - Ran the `v5` codemod to automatically migrate the code. - Fixed all type errors that arose from the upgrade.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe updates include dependency version changes in the package manifest, a bug fix in file extension extraction, and a comprehensive refactor of agent and tool type signatures. Notably, message types are updated from Changes
Sequence Diagram(s)sequenceDiagram
participant AdminUI
participant Agent
participant Tool
participant Model
AdminUI->>Agent: Send user message (ModelMessage[])
Agent->>Model: Run model with ModelMessage[]
Model-->>Agent: Model response (may include tool calls)
Agent->>Tool: Execute tool with input (from toolCall.input)
Tool-->>Agent: Return { output, isError? }
Agent->>Model: Continue with updated ModelMessage[]
Model-->>Agent: Final response
Agent-->>AdminUI: Render response, token usage (inputTokens/outputTokens)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 6
🧹 Nitpick comments (3)
service/src/adapters/line.ts (1)
29-31
: Optional: sanitize filenames and be defensive with extensions.
- Consider stripping path separators from event.message.fileName before using it in blobName to avoid path traversal injection into blob keys.
- getFileExtension returns 'bin' fallback; ensure filename defaulting cannot produce double dots.
Example hardening (outside shown ranges):
const safeFileName = String(event.message.fileName || '') .replace(/[/\\]/g, '') .slice(-200); // cap length const blobName = `${event.source.userId}/${Date.now()}-${event.message.id}.${getFileExtension(event.message)}`Also applies to: 47-49, 80-101
service/src/admin/chat.ts (1)
2-2
: Alignments look good; minor UI text nit for clarity.
- ModelMessage import and input/output token renaming are consistent with the v5 refactor.
- Tool call/result render now uses input/output fields—matches agent changes.
Nit: Update table labels “Prompt” and “Completion” to “Input” and “Output” to match the new terminology.
Suggested minimal change (outside ranges):
-<td>Prompt</td> +<td>Input</td> ... -<td>Completion</td> +<td>Output</td>Also applies to: 118-120, 140-142, 155-157, 163-165, 211-215, 219-229, 347-350, 354-357, 395-395
service/src/agent/index.ts (1)
345-362
: Good: explicit MCP client close with guard.Nice explicit cleanup of the MCP client. Consider downgrading the warning when close() isn’t available to debug to reduce noise in normal runs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
service/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
service/package.json
(3 hunks)service/src/adapters/line.ts
(1 hunks)service/src/admin/chat.ts
(7 hunks)service/src/agent/index.ts
(15 hunks)service/src/agent/tools.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
service/src/adapters/line.ts
[error] 73-73: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (2)
service/src/agent/index.ts (2)
57-62
: providerOptions vs providerMetadata mismatch in logs.You’re storing result.providerMetadata under providerOptions in the log. If the intent is to record provider response metadata, consider keeping the name providerMetadata (or mapping to a new log field) to avoid confusion with the request-time providerOptions.
If provider metadata was renamed in v5, confirm the correct field on the result object and adjust accordingly. Consider:
- providerOptions: result.providerMetadata, + providerMetadata: result.providerMetadata,Or, if a new result field exists:
- providerOptions: result.providerMetadata, + providerOptions: result.providerOptions ?? undefined,
24-25
: Model selection and provider options: confirm v5 compatibility.The model id gemini-1.5-flash-preview-04-17 and google.thinkingConfig usage should be validated against @ai-sdk/google@2.0.0 and ai@5.0.0 to avoid runtime errors.
Please verify in the docs or by a quick run that:
- model('gemini-1.5-flash-preview-04-17') is valid
- providerOptions.google.thinkingConfig is supported in v2.0.0
If unsure, I can adjust to a documented, stable model and remove providerOptions until needed.
Also applies to: 149-157
"repository": "https://github.com/dtinth/automatron", | ||
"dependencies": { | ||
"@ai-sdk/google": "^1.2.17", | ||
"@ai-sdk/google": "2.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify moving zod to devDependencies won’t break runtime.
Types-only usage is fine, but if any runtime schema validation relies on zod, this move will cause production failures. Also, you’re pinning ai to 5.0.0 and @ai-sdk/google to 2.0.0—good for reproducibility, but please confirm all v5 APIs used exist in the code paths shipped by service/.
Run this to confirm zod is imported only as a type (no runtime value import) under service/:
If any non type-only zod import appears, keep zod in dependencies or refactor to type-only imports.
Additionally, consider whether you want to pin ws/yargs like others for consistency.
Also applies to: 18-18, 37-39
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Zod imports under service/:"
rg -n --no-heading -g "service/**" --replace '$0' -- \
$'import\\s+(type\\s+)?{[^}]*}\\s+from\\s+[\'"]zod[\'"]|import\\s+(type\\s+)?\\*\\s+as\\s+z\\s+from\\s+[\'"]zod[\'"]'
echo
echo "Non type-only zod imports (should be empty):"
rg -n --no-heading -g "service/**" -- \
$'import\\s+\\{[^}]*\\}\\s+from\\s+[\'"]zod[\'"]' | grep -v 'import type' || true
Length of output: 455
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "All zod imports under service/:"
rg -n --no-heading -g 'service/**' "from ['\"]zod['\"]"
echo
echo "Non-type-only zod imports (should be empty):"
rg -n --no-heading -g 'service/**' "import .* from ['\"]zod['\"]" | grep -v '^import type' || true
Length of output: 498
Action Required: Restore Zod to dependencies to avoid runtime failures
Zod is imported at runtime in service/src/agent/tools.ts:2
(import { z, … } from 'zod'
), so moving it to devDependencies will break production builds. Please:
- In
service/package.json
, move Zod back underdependencies
. - (Optional) Pin
ws
andyargs
versions for consistency with other dependencies. - Confirm that all v5 APIs from
ai@5.0.0
and v2 APIs from@ai-sdk/google@2.0.0
used byservice/
actually exist in the shipped code.
🤖 Prompt for AI Agents
In service/package.json at line 8, Zod was moved to devDependencies but is
needed at runtime as imported in service/src/agent/tools.ts line 2. Move Zod
back from devDependencies to dependencies to prevent runtime failures.
Optionally, pin the versions of ws and yargs to maintain consistency with other
dependencies. Also verify that the v5 APIs from ai@5.0.0 and v2 APIs from
@ai-sdk/google@2.0.0 used in service/ are present in the shipped code.
const extension = getFileExtension(message.file).toLowerCase() | ||
return getMimeTypeFromExtension(extension) || 'application/octet-stream' |
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.
Bug: getFileExtension called with message.file; switch declaration lint issue.
- getFileExtension expects a full message (uses message.type and message.fileName). Passing message.file will break and return incorrect content types for file messages.
- Biome flagged noSwitchDeclarations: wrap the declaration in a block.
Apply this diff:
- case 'file':
- // Try to determine from file extension
- const extension = getFileExtension(message.file).toLowerCase()
- return getMimeTypeFromExtension(extension) || 'application/octet-stream'
+ case 'file': {
+ // Try to determine from file extension
+ const extension = getFileExtension(message).toLowerCase()
+ return getMimeTypeFromExtension(extension) || 'application/octet-stream'
+ }
📝 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 extension = getFileExtension(message.file).toLowerCase() | |
return getMimeTypeFromExtension(extension) || 'application/octet-stream' | |
case 'file': { | |
// Try to determine from file extension | |
const extension = getFileExtension(message).toLowerCase() | |
return getMimeTypeFromExtension(extension) || 'application/octet-stream' | |
} |
🧰 Tools
🪛 Biome (2.1.2)
[error] 73-73: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In service/src/adapters/line.ts around lines 73 to 74, getFileExtension is
incorrectly called with message.file instead of the full message object, causing
wrong content types for file messages. Also, the switch declaration needs to be
wrapped in a block to satisfy the noSwitchDeclarations lint rule. Fix this by
passing the entire message object to getFileExtension and enclosing the switch
statement inside a block.
const toolDefinititions: ToolDefinitionsMap = Object.fromEntries( | ||
Object.entries(await mcpClient.tools()).map( // Corrected: mcpClient is not a promise here, mcpClient.tools() is | ||
Object.entries(await mcpClient.tools()).map( | ||
// Corrected: mcpClient is not a promise here, mcpClient.tools() is | ||
([name, toolEntry]: [string, any]) => { | ||
const { execute, ...definitionPart } = toolEntry | ||
return [name, definitionPart as Tool<any, any>] | ||
} | ||
) | ||
) | ||
const toolImplementations: ToolImplementationsMap = Object.fromEntries( | ||
Object.entries(await mcpClient.tools()).map(([name, toolEntry]: [string, any]) => { // Corrected: mcpClient.tools() is a promise | ||
const { execute } = toolEntry; // toolEntry here is ToolExecution which has execute | ||
return [ | ||
name, | ||
async (args: any, options: ToolExecutionOptions) => { | ||
// Ensure execute is a function before calling | ||
if (typeof execute === 'function') { | ||
const result = await execute(args, options) | ||
Object.entries(await mcpClient.tools()).map( | ||
([name, toolEntry]: [string, any]) => { |
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.
🛠️ Refactor suggestion
Avoid calling mcpClient.tools() twice; fetch once and reuse.
This avoids duplicate network calls and keeps definitions/implementations in sync.
Example refactor (outside exact lines, illustrating intent):
const toolsMap = await mcpClient.tools()
const toolDefinititions: ToolDefinitionsMap = Object.fromEntries(
Object.entries(toolsMap).map(([name, toolEntry]: [string, any]) => {
const { execute, ...definitionPart } = toolEntry
return [name, definitionPart as Tool<any, any>]
})
)
const toolImplementations: ToolImplementationsMap = Object.fromEntries(
Object.entries(toolsMap).map(([name, toolEntry]: [string, any]) => {
const { execute } = toolEntry
return [
name,
async (args: any) => {
if (typeof execute === 'function') {
const out = await execute(args)
return { isError: false, result: out }
}
return {
isError: true,
result: `Tool ${name} does not have a valid execute method.`,
}
},
]
})
)
🤖 Prompt for AI Agents
In service/src/agent/index.ts around lines 100 to 111, the code calls
mcpClient.tools() twice, causing duplicate network calls and potential
inconsistency. To fix this, call mcpClient.tools() once, store the result in a
variable (e.g., toolsMap), and then use this variable to create both
toolDefinitions and toolImplementations maps by mapping over
Object.entries(toolsMap).
async (args: any) => { | ||
// Ensure execute is a function before calling | ||
if (typeof execute === 'function') { | ||
const result = await execute(args) | ||
return { | ||
isError: false, | ||
result, // Comma removed here | ||
} | ||
} |
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.
🛠️ Refactor suggestion
Unify tool return shape to use output instead of result.
Admin UI and ToolResultPart use output. Returning result here forces extra mapping and causes casts elsewhere.
Apply this diff:
- const result = await execute(args)
- return {
- isError: false,
- result, // Comma removed here
- }
+ const output = await execute(args)
+ return {
+ isError: false,
+ output,
+ }
- return {
- isError: true,
- result: `Tool ${name} does not have a valid execute method.`,
- }
+ return {
+ isError: true,
+ output: `Tool ${name} does not have a valid execute method.`,
+ }
Then, update the ToolImplementationsMap type accordingly, or import ToolResult from tools.ts and reuse it for consistency.
Also applies to: 127-131
🤖 Prompt for AI Agents
In service/src/agent/index.ts around lines 116 to 124 and 127 to 131, the
returned object uses the key "result" instead of "output," which is inconsistent
with Admin UI and ToolResultPart expectations. Change the key from "result" to
"output" in the returned object to unify the return shape. Additionally, update
the ToolImplementationsMap type to reflect this change or import and reuse the
ToolResult type from tools.ts for consistency across the codebase.
const result = await toolImplementations[toolName]!(toolCall.input as any) | ||
|
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.
🛠️ Refactor suggestion
Follow-through: processToolCalls must handle the unified output shape.
After unifying to output, adjust usage here and remove the any-cast by serializing errors safely.
Apply this diff:
- const result = await toolImplementations[toolName]!(toolCall.input as any)
+ const result = await toolImplementations[toolName]!(toolCall.input as any)
- output: result.result,
+ output: result.output,
- // 🚨 TODO: Fix this type error
- output: { error: new Error(`Internal error: ${error}`) } as any,
+ output: { error: String(error) },
Also applies to: 426-427, 440-442
🤖 Prompt for AI Agents
In service/src/agent/index.ts around lines 414-415 (also apply similar changes
at 426-427 and 440-442), the call to toolImplementations uses an unsafe any-cast
for input and does not handle the unified output shape properly. Remove the
any-cast by ensuring the input is correctly typed and adjust the handling of the
result to accommodate the unified output format, including safely serializing
any errors in the output to avoid runtime issues.
} from 'ai' | ||
import { z } from 'zod' | ||
import { type ToolResultPart, type ToolSet } from 'ai' | ||
import { z, type ZodTypeAny } from 'zod' |
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.
🛠️ Refactor suggestion
Make zod a type-only dependency and align tool result shape.
- Use type-only imports so zod isn’t required at runtime. Replace z.infer with the exported infer type to avoid importing z as a value.
- Optional: align tool implementation return to use output (not result) to match ToolResult and downstream expectations.
Apply this diff within the shown ranges:
-import { z, type ZodTypeAny } from 'zod'
+import type { ZodTypeAny, infer as ZodInfer } from 'zod'
-export type ToolResult = { output: any; isError?: boolean }
+export type ToolResult = { output: unknown; isError?: boolean }
- args: TOOLS[K]['inputSchema'] extends z.ZodTypeAny
- ? z.infer<TOOLS[K]['inputSchema']>
+ args: TOOLS[K]['inputSchema'] extends ZodTypeAny
+ ? ZodInfer<TOOLS[K]['inputSchema']>
: unknown
Nit: type ToolResultPart isn’t used here; consider removing the import to reduce noise.
Also applies to: 6-6, 10-12
🤖 Prompt for AI Agents
In service/src/agent/tools.ts at lines 2, 6, and 10-12, change the import of zod
to be type-only by using 'import type' syntax and replace all uses of z.infer
with the exported infer type to avoid importing z as a runtime value. Also,
update the tool implementation return to use 'output' instead of 'result' to
align with the ToolResult type and downstream expectations. Finally, remove the
unused import of ToolResultPart to clean up the code.
This commit upgrades the Vercel AI SDK from v4 to v5 in the service subfolder.
Summary by CodeRabbit
Bug Fixes
Refactor
Chores