fix(openai): structured output #32551
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
CodSpeed WallTime Performance ReportMerging #32551 will not alter performanceComparing
|
CodSpeed Instrumentation Performance ReportMerging #32551 will not alter performanceComparing Summary
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…langchain into mdrxy/openai-fix-struc-out
|
How do I request that this same fix get made to the js lib? Running into this issue currently |
|
@alexdlugo I've opened an issue on the js repo |
|
@mdrxy hi! |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug with structured output when using the OpenAI chat model with text parameters in model_kwargs. The fix ensures proper merging of text parameters when using structured output with the json_schema method.
- Removes the previous error that prevented combining
response_formatandtextparameters - Implements proper merging logic for
textparameters with structured output format - Adds comprehensive test coverage for the fixed functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
libs/partners/openai/langchain_openai/chat_models/base.py |
Fixed parameter merging logic in _construct_responses_api_payload to properly handle text parameters with structured output |
libs/partners/openai/tests/unit_tests/chat_models/test_base.py |
Added test case to verify structured output works correctly with text parameters in model_kwargs |
| "format": {"type": "json_schema", **response_format["json_schema"]} | ||
| } | ||
| else: | ||
| structured_text = {} |
There was a problem hiding this comment.
The else clause after line 3622 sets structured_text = {} but this empty dict serves no functional purpose since it's only used when there are existing text parameters or actual structured text content to merge. Consider removing this else clause and initializing structured_text = {} at the beginning of the responses.create path for clarity.
Copilot uses AI. Check for mistakes.
| # Type cast for mypy - structured_llm is a RunnableSequence with a | ||
| # RunnableBinding first element |
There was a problem hiding this comment.
The comment mentions "Type cast for mypy" but the actual type casting occurs on lines 2751-2753. The comment should be moved closer to the actual casting operations or rewritten to better explain the structure being accessed.
| # Type cast for mypy - structured_llm is a RunnableSequence with a | |
| # RunnableBinding first element | |
| # Type cast for mypy: structured_llm is a RunnableSequence, | |
| # its first element is a RunnableBinding, and its bound is a ChatOpenAI. |
Copilot uses AI. Check for mistakes.
Fixes #32492