-
Notifications
You must be signed in to change notification settings - Fork 918
Support structured and manual JSON output_type modes in addition to tool calls #1628
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?
Conversation
Docs Preview
|
elif structured_outputs: | ||
# No events are emitted during the handling of structured outputs, so we don't need to yield anything | ||
self._next_node = await self._handle_structured_outputs(ctx, structured_outputs) |
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.
It seems like this code is assuming that you don't have both tool calls and structured outputs, but I would think that this is possible. Obviously you aren't going to have output tool calls, but you could have non-output tool calls right? My intuition is that we should rework it so that we process any tool calls and then handle the structured outputs.
If it's impossible to use tool calls while also using structured output then maybe this is fine but I don't see why that should be the case in principle. (But maybe it's the case in all existing APIs? Or maybe it isn't?)
try: | ||
result_data = output_schema.validate(structured_output) | ||
result_data = await _validate_output(result_data, ctx, None) | ||
except _output.ToolRetryError as e: |
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.
unfortunate exception name now
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.
Could we use ModelRetry directly here? Maybe that's hard/annoying if things are set up to convert those into ToolRetryError (because it was assumed to be handled in a tool). I don't remember the details..
f305aa5
to
b892fc8
Compare
# Conflicts: # tests/models/test_openai.py
Right now, this only implements
response_format=json_schema
structured output for OpenAI chat completions in non-streaming mode.output_type
in a dict (as we do with output tools), because while OpenAI supports them, our schema generation doesn'tToolOutput
andStructuredOutput
not preferred but forced, erroring if model doesn't support itStructuredOutput
TODO
sresponse_format=json_object
for older OpenAI models?This test illustrates the behavior, including the fact that OpenAI supports tool calls alongside structured output (as noted in #582 (comment)):