-
Notifications
You must be signed in to change notification settings - Fork 798
fix: pass config params to structured_output in OpenAIResponsesModel #1910
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
| """ | ||
|
|
||
| import base64 | ||
| import copy | ||
| import json | ||
| import logging | ||
| import mimetypes | ||
|
|
@@ -356,13 +357,21 @@ async def structured_output( | |
| ContextWindowOverflowException: If the input exceeds the model's context window. | ||
| ModelThrottledException: If the request is throttled by OpenAI (rate limits). | ||
| """ | ||
| request = self._format_request(prompt, system_prompt=system_prompt) | ||
|
|
||
| excluded_keys = {"model", "input", "stream"} | ||
| parse_kwargs: dict[str, Any] = { | ||
| "model": request["model"], | ||
| "input": request["input"], | ||
| "text_format": output_model, | ||
| } | ||
| for key, value in request.items(): | ||
| if key not in excluded_keys: | ||
| parse_kwargs.setdefault(key, value) | ||
|
|
||
| async with openai.AsyncOpenAI(**self.client_args) as client: | ||
| try: | ||
| response = await client.responses.parse( | ||
| model=self.get_config()["model_id"], | ||
| input=self._format_request(prompt, system_prompt=system_prompt)["input"], | ||
| text_format=output_model, | ||
| ) | ||
| response = await client.responses.parse(**parse_kwargs) | ||
| except openai.BadRequestError as e: | ||
| if hasattr(e, "code") and e.code == "context_length_exceeded": | ||
| logger.warning(_CONTEXT_WINDOW_OVERFLOW_MSG) | ||
|
|
@@ -404,7 +413,7 @@ def _format_request( | |
| "model": self.config["model_id"], | ||
| "input": input_items, | ||
| "stream": True, | ||
| **cast(dict[str, Any], self.config.get("params", {})), | ||
| **copy.deepcopy(cast(dict[str, Any], self.config.get("params", {}))), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Issue: The Suggestion: If the intent is to protect |
||
| } | ||
|
|
||
| if system_prompt: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue: The
setdefaultloop pattern works correctly but is a bit indirect. Sinceexcluded_keysalready filters outmodel,input, andstream, and the only additional key inparse_kwargsistext_format(which won't be inrequest),setdefaultbehaves identically to a direct assignment here.Suggestion: Consider simplifying to a dict comprehension for clarity:
This makes the logic more explicit: take everything from
requestexcept the excluded keys, then addtext_format.