-
Notifications
You must be signed in to change notification settings - Fork 2.8k
re-introduce openai's zodFunction function helper as it will ensure t… #8389
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
re-introduce openai's zodFunction function helper as it will ensure t… #8389
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
…hat tool call schemas conform to the subset of the JSON Schema spec that they support
bab94c6
to
a9b07ed
Compare
@hntrl ready for another 👀 |
This is a breaking change for any LangChain tools with zod schema fields that are Since the API doesn't return errors with my schemas, maybe some way to configure this would be in order? Even the Calculator tool from @langchain/community will error after this change.
|
@danny-avila out of curiosity were you getting warnings like this when making calls with these tools before the upgrade? These errors were getting surfaced internally by openai when trying to reproduce your case on an older version. I have the feeling that the error you're seeing now is because optional handling got nixed in the latest openai sdk version.
|
Yes, that's the error. Since it's not breaking yet, would like some time to migrate everything to |
Thanks for flagging this. We didn't have any integrations tests for optional fields which means this skipped our radar, so apologies for that! Correcting PR (#8402) @infiton Unfortunately I think these are mutually exclusive details, and I want to trend towards not breaking existing code. We can't have optional handling and ref forwarding like this using zods inlined serializer. If these forward facing refs are important to you I might recommend something like this until we can do a more permanent fix. import { zodToJsonSchema } from "openai/_vendor/zod-to-json-schema"
import { z } from "zod";
const schema = z.object({
choice: ChoiceSchema,
});
// with tools
const choiceTool = tool((x) => x, {
name: "choice",
description: "A tool to choose a color",
schema: zodToJsonSchema(
z.object({
choice: ChoiceSchema,
}),
{
openaiStrictMode: true,
name: "choice",
nameStrategy: "duplicate-ref",
$refStrategy: "extract-to-root",
nullableStrategy: "property",
}
),
});
// withStructuredOutput
model.withStructuredOutput<z.output<typeof schema>>(
zodToJsonSchema(
z.object({
choice: ChoiceSchema,
}),
{
openaiStrictMode: true,
name: "choice",
nameStrategy: "duplicate-ref",
$refStrategy: "extract-to-root",
nullableStrategy: "property",
}
)
); Conclusion of this is there's some more work to be done surrounding how we handle schema serializing. There were a couple of other things that I found are broken (not by this PR) surrounding zod schemas, like how you can't have optional fields in |
Over at gadget.dev we were upgrading langchain packages, specifically we were upgrading
@langchain/openai
to0.5.13
and found that one of tools consistently failed OpenAI's schema validation.The error we hit was:
The issue is that OpenAI does not support the entire JSONSchema spec and they provide their own zod -> json helper that was previously in langchain and removed in https://github.com/langchain-ai/langchainjs/pull/7973/files#diff-dce785f9dfdad205c5d1752f94e8b55fa98f0b863b0eb1d53410de156b183580
I think that OpenAI is likely to provide the most reliable support for this conversion so if we can use their helper we should
I've added a test, that mimics the failure we were seeing
the schema generated on main:
with this change:
you can reach me at
@kttttate
or@gadget_dev