feat(openai): Update with_structured_output default for OpenAI #28947
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
dosubot
bot
added
the
size:XS
dosubot
bot
added
size:L
dosubot
bot
added
size:XL
| @@ -1310,6 +1325,24 @@ class Joke(BaseModel): | |||
| joke_result = chat.invoke("Give me a joke about cats, include the punchline.") | |||
| assert isinstance(joke_result, Joke) | |||
|
|
|||
| # Schema | |||
There was a problem hiding this comment.
These new tests fail unless strict is False.
#29075 is a hack (?) that will pass them if strict is True. But strict=True will break things in other ways (e.g., schemas with nested TypedDicts).
| @property | ||
| def structured_output_kwargs(self) -> dict: | ||
| return {"method": "function_calling"} |
There was a problem hiding this comment.
Added this property to chat model standard tests. Azure legacy model tests don't support method="json_schema".
There was a problem hiding this comment.
do we actually want this to be configurable, or should we just have standard tests test the default?
There was a problem hiding this comment.
TestAzureOpenAIStandardLegacy breaks with default. Seems wasteful to xfail its structured output tests instead of running them with the supported method. Can also delete TestAzureOpenAIStandardLegacy but that didn't feel great either. lmk if there are other options.
|
Integration tests pass: https://github.com/langchain-ai/langchain/actions/runs/12678107252 |
| @property | ||
| def structured_output_kwargs(self) -> dict: | ||
| return {"method": "function_calling"} |
There was a problem hiding this comment.
do we actually want this to be configurable, or should we just have standard tests test the default?
| ("model", "method", "strict"), | ||
| [("gpt-4o", "function_calling", True), ("gpt-4o-2024-08-06", "json_schema", None)], | ||
| ("model", "method"), | ||
| [("gpt-4o", "function_calling"), ("gpt-4o-2024-08-06", "json_schema")], |
There was a problem hiding this comment.
given strict is passed in should this still test both?
There was a problem hiding this comment.
strict defaults to False, so most other tests cover it. I added some "invalid" schemas to the standard test as well (test_structured_output_optional_param) to add coverage there.
Should be accompanied by a minor bump
Should we also set
strict=Trueby default?