Skip to content
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

openai: fix json_schema mode in AzureChatOpenAI.with_structured_output #26086

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Skar0
Copy link
Contributor

@Skar0 Skar0 commented Sep 5, 2024

This PR fixes an issue with json_schema mode in AzureChatOpenAI.with_structured_output.

Following the release of structured output support in Azure OpenAI, I was able to test #25591 where the main proposed change is to use BaseChatOpenAI.with_structured_output and not override it in AzureChatOpenAI.with_structured_output like currently implemented. When testing json_schema mode with those proposed changes, I was getting ValueError: Structured Output response does not have a 'parsed' field nor a 'refusal' field..

After some investigation, here is an analysis of the issue:

  • In BaseChatOpenAI._create_chat_result, the parsed and refusal properties of the completion generated by BaseChatOpenAI._generate are correctly added as additional_kwargs to the generation.
  • In that case, BaseChatOpenAI._create_chat_result is called with the argument response of type openai.BaseModel.
  • However, AzureChatOpenAI._create_chat_result overrides BaseChatOpenAI._create_chat_result like so
    if not isinstance(response, dict):
    response = response.model_dump()
    for res in response["choices"]:
    if res.get("finish_reason", None) == "content_filter":
    raise ValueError(
    "Azure has not provided the response due to a content filter "
    "being triggered"
    )
    chat_result = super()._create_chat_result(response, generation_info)
  • BaseChatOpenAI._create_chat_result is thus called with the argument response of type dict (following response.model_dump()).
  • It follows that the condition below in the implementation of BaseChatOpenAI._create_chat_result is not satisfied and thus parsed and refusal are not added to the generation.
    if isinstance(response, openai.BaseModel) and getattr(
    response, "choices", None
    ):
    message = response.choices[0].message # type: ignore[attr-defined]
    if hasattr(message, "parsed"):
    generations[0].message.additional_kwargs["parsed"] = message.parsed
    if hasattr(message, "refusal"):
    generations[0].message.additional_kwargs["refusal"] = message.refusal

I propose to change the implementation of that check so that it works on the response_dict that is created anyways in BaseChatOpenAI._create_chat_result so that the parsed and refusal properties of the completion are correctly added as additional_kwargs to the generation whatever the type of response.

@baskaryan I believe it would be valuable for you to review to avoid any regression and confirm that this PR should be merged at the same time as #25591 to avoid introducing a bug.

@efriis efriis added the partner label Sep 5, 2024
@efriis efriis self-assigned this Sep 5, 2024
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Sep 5, 2024
Copy link

vercel bot commented Sep 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Sep 5, 2024 0:02am

@dosubot dosubot bot added langchain Related to the langchain package 🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature labels Sep 5, 2024
@efriis efriis assigned baskaryan and unassigned efriis Sep 8, 2024
@ccurme ccurme removed the langchain Related to the langchain package label Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature partner size:S This PR changes 10-29 lines, ignoring generated files.
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

4 participants