fix(ai): detect callable ArkType schemas in convertSchemaToJsonSchema (#276)#662
Merged
Merged
Conversation
…#276) ArkType's type() returns a callable function with ~standard attached, not a plain object. The Standard Schema detection guards required `typeof schema === 'object'`, so ArkType schemas matched neither branch and fell through the JSONSchema pass-through, returning the raw validator function instead of a converted JSON Schema object. This also meant tool input/output validation silently skipped ArkType schemas. Add an isPropertyCarrier helper that accepts both objects and callable functions, and route both isStandardJSONSchema and isStandardSchema through it. Add unit coverage (conversion, guards, structured output, parse/validate) plus an end-to-end wire-format regression asserting the converted JSON Schema reaches the provider.
Contributor
📝 WalkthroughWalkthroughThis PR fixes ChangesArkType Callable Schema Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
Contributor
🚀 Changeset Version Preview1 package(s) bumped directly, 18 bumped as dependents. 🟩 Patch bumps
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
View your CI Pipeline Execution ↗ for commit 32b980d
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-skills
@tanstack/ai-devtools-core
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-openrouter
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
![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
🧹 Nitpick comments (1)
testing/e2e/src/routes/api.arktype-tool-wire.ts (1)
73-79: 💤 Low valueConsider using appropriate HTTP status code for errors.
The error handler returns status
200even when an error occurs. While this might be intentional for the test endpoint to ensure the test framework can always read the response, it's unconventional and could mask failures in test infrastructure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@testing/e2e/src/routes/api.arktype-tool-wire.ts` around lines 73 - 79, The error branch currently always returns HTTP 200; change it to return an appropriate error status (e.g., use (error as any).status || 500) instead of 200 so failures surface correctly: update the Response creation that JSON.stringifies { ok:false, error: ... } to set the status to the error's status when available or 500 as a default, and preserve the JSON content-type header.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@testing/e2e/src/routes/api.arktype-tool-wire.ts`:
- Line 3: The import is using the package root (createOpenRouterText from
'`@tanstack/ai-openrouter`') which prevents tree-shaking; update the import to use
the adapters subpath (import createOpenRouterText from
'`@tanstack/ai-openrouter/adapters`') so the provider adapter is
tree-shakeable—locate the createOpenRouterText import and replace the root
package import with the '/adapters' subpath.
- Line 51: The model argument to createOpenRouterText is being force-cast with
"as never", which bypasses its generic type OpenRouterTextModels; remove the "as
never" cast and pass 'openai/gpt-4o' directly (or, if necessary, cast to the
correct OpenRouterTextModels member) so the call to createOpenRouterText<TModel
extends OpenRouterTextModels>(model: TModel, ...) retains proper type safety.
---
Nitpick comments:
In `@testing/e2e/src/routes/api.arktype-tool-wire.ts`:
- Around line 73-79: The error branch currently always returns HTTP 200; change
it to return an appropriate error status (e.g., use (error as any).status ||
500) instead of 200 so failures surface correctly: update the Response creation
that JSON.stringifies { ok:false, error: ... } to set the status to the error's
status when available or 500 as a default, and preserve the JSON content-type
header.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c86cd97b-9806-4b7d-a80f-055ecc5b3901
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.changeset/arktype-callable-schema.mdpackages/ai/package.jsonpackages/ai/src/activities/chat/tools/schema-converter.tspackages/ai/tests/standard-converter.test.tstesting/e2e/package.jsontesting/e2e/src/routeTree.gen.tstesting/e2e/src/routes/api.arktype-tool-wire.tstesting/e2e/tests/arktype-tool-wire.spec.ts
| @@ -0,0 +1,89 @@ | |||
| import { createFileRoute } from '@tanstack/react-router' | |||
| import { chat, createChatOptions, toolDefinition } from '@tanstack/ai' | |||
| import { createOpenRouterText } from '@tanstack/ai-openrouter' | |||
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Prefer /adapters subpath import for tree-shakeable provider adapters.
As per coding guidelines, provider adapters should be imported from the /adapters subpath rather than the root package for better tree-shaking.
♻️ Proposed fix
-import { createOpenRouterText } from '`@tanstack/ai-openrouter`'
+import { createOpenRouterText } from '`@tanstack/ai-openrouter/adapters`'As per coding guidelines: "Import tree-shakeable provider adapters from /adapters subpath (e.g., @tanstack/ai-openai/adapters) rather than default imports"
📝 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.
Suggested change
| import { createOpenRouterText } from '@tanstack/ai-openrouter' | |
| import { createOpenRouterText } from '`@tanstack/ai-openrouter/adapters`' |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@testing/e2e/src/routes/api.arktype-tool-wire.ts` at line 3, The import is
using the package root (createOpenRouterText from '`@tanstack/ai-openrouter`')
which prevents tree-shaking; update the import to use the adapters subpath
(import createOpenRouterText from '`@tanstack/ai-openrouter/adapters`') so the
provider adapter is tree-shakeable—locate the createOpenRouterText import and
replace the root package import with the '/adapters' subpath.
| } | ||
|
|
||
| const adapter = createOpenRouterText( | ||
| 'openai/gpt-4o' as never, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the type signature of createOpenRouterText to determine the correct model type
ast-grep --pattern 'function createOpenRouterText($$$) {
$$$
}'
# Also search for type definitions
rg -nP --type=ts 'createOpenRouterText.*\(' -A 5Repository: TanStack/ai
Length of output: 5883
Avoid as never on the createOpenRouterText model argument
createOpenRouterText is generic over OpenRouterTextModels (createOpenRouterText<TModel extends OpenRouterTextModels>(model: TModel, apiKey: string, ...)), so as never bypasses the intended type safety. Remove the cast and pass 'openai/gpt-4o' directly (or cast to the correct OpenRouterTextModels member instead).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@testing/e2e/src/routes/api.arktype-tool-wire.ts` at line 51, The model
argument to createOpenRouterText is being force-cast with "as never", which
bypasses its generic type OpenRouterTextModels; remove the "as never" cast and
pass 'openai/gpt-4o' directly (or, if necessary, cast to the correct
OpenRouterTextModels member) so the call to createOpenRouterText<TModel extends
OpenRouterTextModels>(model: TModel, ...) retains proper type safety.
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
1 participant
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.
Fixes #276
convertSchemaToJsonSchemareturned the raw ArkType validator (a function) instead of a JSON Schema object, so providers that validate the tool/output schema (e.g. OpenRouter) rejected it.Root cause
ArkType's
type()returns a callable function with~standardattached as a property — not a plain object. Both Standard Schema detection guards (isStandardJSONSchema,isStandardSchema) requiredtypeof schema === 'object', which isfalsefor a function. ArkType schemas therefore matched neither branch and fell through to the JSONSchema pass-through, which returned the function untouched. The same guard also gated tool input/output validation, so validation was silently skipped for ArkType tools.Verified empirically against the reporter's version (arktype
2.1.28):typeof type({...}) === 'function', and~standard.jsonSchema.input()is present and works — so the only needed change is widening the guards.Fix
isPropertyCarrierhelper that accepts both'object'and'function'(non-null).isStandardJSONSchemaandisStandardSchemathrough it. RewroteisStandardJSONSchemacast-free, which also drops theeslint-disableand turns a latentnull.jsonSchemacrash into a cleanfalse.Because tool input/output conversion/validation and structured output all route through these two functions, ArkType now works end-to-end for tools and structured output too — previously validation was silently skipped.
Tests
packages/ai/tests/standard-converter.test.ts): the reporter's exact repro plus guard detection, optional fields, structured-output transformation, and directparseWithStandardSchema/validateWithStandardSchemacoverage with ArkType.testing/e2e/tests/arktype-tool-wire.spec.ts+ route): drives the OpenRouter chat adapter with an ArkType-schema function tool against aimock and inspects the journal to assert the converted JSON Schema reaches the wire (pre-fix the function-valued schema serialized to{}). Deterministic — no recorded LLM fixture needed.Verification
@tanstack/aiunit suite green (63 tests in the converter spec);test:typesclean;test:eslint0 errors for the touched file.arktype — tool schema wire formatpasses.test:sherifand Prettier clean.A patch changeset is included.
Summary by CodeRabbit