Skip to content

Conversation

@Amnah199
Copy link
Contributor

@Amnah199 Amnah199 commented Sep 22, 2025

Related Issues

Proposed Changes:

  • Add a chat generator that uses openai's responses endpoint.
  • The tools param in init and run methods allows openai and MCP tools besides haystack Tools.
  • Structured outputs can be enabled by passing text_format in generation_kwargs.
  • Reasoning can be enabled by setting reasoning params in generation_kwargs.
  • There are other features in Responses API like background requests etc, which are not supported in this iteration.
  • With every ToolCall sent back, the API also expects the reasoning id even if the reasoning object was empty.

How did you test it?

  • Add new unit tests to test features.
  • Add new integration tests.

Notes for the reviewer

Using OpenAI tools and MCP tools doesn’t behave like standard function calls.

  • Tool calls are usually identified by type="function_call". However, for OpenAI/MCP tools, the type value varies depending on the tool being used — for example, type="web_search_call".
  • Currently, we only return standard ResponseFunctionToolCall objects. As a result, tool calls from OpenAI/MCP tools won’t appear in the ChatMessage, so the user won’t see them.
  • This is fine, since these tool calls are handled internally by OpenAI to generate the final response. The user already receives the output.

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

@github-actions github-actions bot added the type:documentation Improvements on the docs label Sep 22, 2025
@coveralls
Copy link
Collaborator

coveralls commented Sep 22, 2025

Pull Request Test Coverage Report for Build 19115469262

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.7%) to 91.53%

Totals Coverage Status
Change from base Build 19102178311: -0.7%
Covered Lines: 13659
Relevant Lines: 14923

💛 - Coveralls

@Amnah199 Amnah199 requested a review from vblagoje October 13, 2025 07:57
@Amnah199 Amnah199 marked this pull request as ready for review October 13, 2025 10:15
@Amnah199 Amnah199 requested a review from a team as a code owner October 13, 2025 10:15
@anakin87
Copy link
Member

We discussed with @Amnah199 about Function tool call call_id and id. In short, we need to store both.

We considered two possible solutions:

  • Store one of them in ToolCall.id and use ChatMessage.meta to keep the other (or a mapping between them). Since a single ChatMessage can contain multiple tool calls, this approach isn't straightforward but might work.

  • If the previous solution is too complex, add an extra dictionary field to ToolCall (and ToolCallDelta) to store additional info (e.g. provider-specific info). This seems preferable to introducing a dedicated call_id field, which could be confusing for users, and it also allows for future extensibility.

Since I'll be on PTO next week, feel free to merge this PR when it's ready.

@Amnah199
Copy link
Contributor Author

Amnah199 commented Oct 31, 2025

@sjrl Would love your input here.
If we plan to evetually add an extra field to ToolCall, I would implement it that way now as it makes the task much simpler overall.

However, if that’s unlikely (or a definite no), I can try to make the first option work instead.

Context: Here’s the commit showing what the first approach might look like.
On top of that, we’d need to handle lists in convert_messages_to_responses_api_format(messages: list[ChatMessage], …) (as suggested by @anakin87) to correctly map call_id to ToolCallResult as the ChatMessage.meta containing tool results dont have this mapping.

@sjrl
Copy link
Contributor

sjrl commented Nov 3, 2025

@sjrl Would love your input here. If we plan to evetually add an extra field to ToolCall, I would implement it that way now as it makes the task much simpler overall.

However, if that’s unlikely (or a definite no), I can try to make the first option work instead.

Context: Here’s the commit showing what the first approach might look like. On top of that, we’d need to handle lists in convert_messages_to_responses_api_format(messages: list[ChatMessage], …) (as suggested by @anakin87) to correctly map call_id to ToolCallResult as the ChatMessage.meta containing tool results dont have this mapping.

I like the idea of adding the extra field now if it makes the implementation simpler.

Data that is sent to Posthog for usage analytics.
"""
return {"model": self.model}

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we make sure to add a warm_up function that looks like

    def warm_up(self):
        """
        Warm up the Azure OpenAI chat generator.

        This will warm up the tools registered in the chat generator.
        This method is idempotent and will only warm up the tools once.
        """
        if not self._is_warmed_up:
            warm_up_tools(self.tools)
            self._is_warmed_up = True

also make sure to add self._is_warmed_up = False to the init method. This change is related to this PR #9942

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

@Amnah199 Amnah199 requested a review from vblagoje November 5, 2025 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for OpenAI's Responses API

6 participants