Skip to content

Conversation

@sjrl
Copy link
Contributor

@sjrl sjrl commented Nov 5, 2025

Related Issues

Proposed Changes:

  • Adds AzureOpenAIResponsesChatGenerator which inherits from OpenAIResponsesChatGenerator to use Azure OpenAI's Responses API

How did you test it?

  • Added unit tests and integration tests

Notes for the reviewer

  • As a heads up Azure OpenAI officially recommends using the OpenAI client to connect to Azure now so we do run super().__init__() in the component. It still makes sense to make a dedicated component since specifying the base_url is a little clunky and it's easier to keep the old parameter azure_deployment which is then used to create the base_url.

Checklist

  • I have read the contributors guidelines and the code of conduct
  • I have updated the related issue with new insights and changes
  • I added unit tests and updated the docstrings
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I documented my code
  • I ran pre-commit hooks and fixed any issue

@sjrl sjrl force-pushed the azure-openai-responses branch from 149c315 to 94508be Compare November 6, 2025 10:48
Copy link
Contributor

@mpangrazzi mpangrazzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks already good! Left some minor comments.

Also, I think it would be nice to have a live run integration test with reasoning model (like gpt-5-nano), WDYT?

Base automatically changed from openai-responses to main November 6, 2025 11:55
@coveralls
Copy link
Collaborator

coveralls commented Nov 6, 2025

Pull Request Test Coverage Report for Build 19134929429

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 91.578%

Totals Coverage Status
Change from base Build 19134808494: 0.01%
Covered Lines: 13711
Relevant Lines: 14972

💛 - Coveralls

@sjrl
Copy link
Contributor Author

sjrl commented Nov 6, 2025

Looks already good! Left some minor comments.

Also, I think it would be nice to have a live run integration test with reasoning model (like gpt-5-nano), WDYT?

@mpangrazzi I was able to test with gpt-5-mini using a separate Azure deployment than the one we have in our CI (our CI doesn't have access to GPT-5) and the integration test test_live_run_with_tools passed. If there are no complaints on your end I'll go ahead and merge as-is and we can add a proper integration test in the future.

@mpangrazzi
Copy link
Contributor

@sjrl sounds good!

@sjrl
Copy link
Contributor Author

sjrl commented Nov 6, 2025

@mpangrazzi feel free to approve ;)

@mpangrazzi mpangrazzi self-requested a review November 6, 2025 13:22
@sjrl sjrl merged commit bd927da into main Nov 6, 2025
23 checks passed
@sjrl sjrl deleted the azure-openai-responses branch November 6, 2025 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add an AzureOpenAIResponsesChatGenerator based on the OpenAIResponsesChatGenerator

5 participants