feat: update tsdown version, add zod dependency, and enhance type def…#1343
Merged
sergiofilhowz merged 2 commits intomainfrom Mar 24, 2026
Merged
feat: update tsdown version, add zod dependency, and enhance type def…#1343sergiofilhowz merged 2 commits intomainfrom
sergiofilhowz merged 2 commits intomainfrom
Conversation
…initions in SDK (#1341)
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a Zod-based example and zod dependency to the Node example package, updates SDK types for function registration, tweaks a build config and dev dependency, and applies small formatting/type-check suppressions in example sources. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
ytallo
previously approved these changes
Mar 23, 2026
guibeira
previously approved these changes
Mar 23, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk/packages/node/iii/src/iii-types.ts (1)
86-111:⚠️ Potential issue | 🟠 MajorType mismatch:
propertiesshould be a Record, not an array, andrequiredshould support string arrays for object-level requirements.The
propertiesfield is typed asRegisterFunctionFormat[](array), but standard JSON Schema (and Zod'stoJSONSchema()output) definespropertiesas aRecord<string, Schema>(object map). Additionally,requiredis typed asboolean, but JSON Schema usesstring[]for object-level property requirements.This causes incompatibility with the Zod example in
iii-zod-example.ts, which passestoJSONSchema(inputSchema)directly torequest_format. The engine also expects JSON Schema format, as confirmed by its use of.as_object()on the properties field.Align with JSON Schema conventions:
export type RegisterFunctionFormat = { name?: string description?: string type: 'string' | 'number' | 'boolean' | 'object' | 'array' | 'null' | 'map' - properties?: RegisterFunctionFormat[] + properties?: Record<string, RegisterFunctionFormat> items?: RegisterFunctionFormat - required?: boolean + required?: boolean | string[] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/packages/node/iii/src/iii-types.ts` around lines 86 - 111, The RegisterFunctionFormat type's properties and required fields are incorrect for JSON Schema: change properties from an array to a map (properties: Record<string, RegisterFunctionFormat> | undefined) and make required accept JSON Schema-style string arrays (required?: string[]), or to preserve backwards compatibility allow required?: boolean | string[]; update any consumers expecting the old array/boolean shapes (references to RegisterFunctionFormat.properties and RegisterFunctionFormat.required) to handle the new types accordingly.
🧹 Nitpick comments (1)
sdk/packages/node/iii-example/src/iii.ts (1)
4-6: Consider using standard//@ts-expect-error`` format.The JSDoc-style
/**@ts-expect-error*/works but is unconventional. The standard format is a single-line comment directly above the offending line:// `@ts-expect-error` process.env is not typed const engineWsUrl = process.env.III_URL ?? 'ws://localhost:49134'Alternatively, for better type safety in the example code, consider declaring the environment variable type or using a type assertion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/packages/node/iii-example/src/iii.ts` around lines 4 - 6, Replace the JSDoc-style suppression before the engineWsUrl declaration with the standard single-line TypeScript suppression or add a proper type assertion: change the comment above the const engineWsUrl declaration to use "// `@ts-expect-error` process.env is not typed" or alternatively declare/augment the environment variable types (or cast process.env.III_URL as string | undefined) so the const engineWsUrl = process.env.III_URL ?? 'ws://localhost:49134' no longer relies on the unconventional /** `@ts-expect-error` */ form.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk/packages/node/iii-example/src/iii-zod-example.ts`:
- Line 1: The import currently pulls toJSONSchema as a named import (import {
toJSONSchema, z } from 'zod') which is invalid for Zod v4; update the import to
only import z (import { z } from 'zod') and then replace any direct uses of
toJSONSchema (e.g., toJSONSchema(schema) or standalone toJSONSchema calls) to
call the method on the z object (z.toJSONSchema(schema) or
schemaInstance?.toJSONSchema()) as appropriate so all references use
z.toJSONSchema.
- Around line 17-25: The call to iii.registerFunction is passing Zod's
toJSONSchema(inputSchema)/toJSONSchema(outputSchema) which produce standard JSON
Schema shapes that don't match the project's RegisterFunctionFormat (differences
in properties, required, type structure); add a conversion step before
registering: implement a utility (e.g., jsonSchemaToFormat) that takes the
return of toJSONSchema and maps fields into RegisterFunctionFormat (map
schema.type -> type, convert schema.properties object into
RegisterFunctionFormat[] with name and recursively converted children, convert
schema.required array into per-property boolean required) and use
jsonSchemaToFormat(toJSONSchema(inputSchema)) and
jsonSchemaToFormat(toJSONSchema(outputSchema)) as request_format/response_format
when calling iii.registerFunction (or alternatively align the
RegisterFunctionFormat type to accept JSON Schema if you control that type).
---
Outside diff comments:
In `@sdk/packages/node/iii/src/iii-types.ts`:
- Around line 86-111: The RegisterFunctionFormat type's properties and required
fields are incorrect for JSON Schema: change properties from an array to a map
(properties: Record<string, RegisterFunctionFormat> | undefined) and make
required accept JSON Schema-style string arrays (required?: string[]), or to
preserve backwards compatibility allow required?: boolean | string[]; update any
consumers expecting the old array/boolean shapes (references to
RegisterFunctionFormat.properties and RegisterFunctionFormat.required) to handle
the new types accordingly.
---
Nitpick comments:
In `@sdk/packages/node/iii-example/src/iii.ts`:
- Around line 4-6: Replace the JSDoc-style suppression before the engineWsUrl
declaration with the standard single-line TypeScript suppression or add a proper
type assertion: change the comment above the const engineWsUrl declaration to
use "// `@ts-expect-error` process.env is not typed" or alternatively
declare/augment the environment variable types (or cast process.env.III_URL as
string | undefined) so the const engineWsUrl = process.env.III_URL ??
'ws://localhost:49134' no longer relies on the unconventional /**
`@ts-expect-error` */ form.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7c9b776d-1cf7-47b4-aff2-80cda464f245
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
sdk/packages/node/iii-example/package.jsonsdk/packages/node/iii-example/src/iii-zod-example.tssdk/packages/node/iii-example/src/iii.tssdk/packages/node/iii-example/src/index.tssdk/packages/node/iii-example/src/state.tssdk/packages/node/iii/package.jsonsdk/packages/node/iii/src/iii-types.tssdk/packages/node/iii/tsdown.config.ts
…and adjust zod import in examples
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…initions in SDK (#1341)
Summary by CodeRabbit
New Features
Changes
nameoptional.bodywith keyedproperties.requiredfrom a boolean to a list of required field names.Chores