fix: do not mutate Tool schemas (and handle empty tools) in OpenAIResponsesChatGenerator - #12067
Conversation
_prepare_api_call wrote additionalProperties into the Tool.parameters dict that Tool.tool_spec exposes by reference, permanently altering the user's Tool (and any other generator sharing it) and making serialization round trips unstable. Build the tool definition on a copied parameters dict instead. Also guard _warm_up_tools against an empty tools list, which previously raised IndexError on the unguarded self.tools[0] access.
|
@TimurRakhmatullin86 is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi @TimurRakhmatullin86, thanks a lot for your contribution! 🙏 We noticed that the Contributor License Agreement (CLA) check ( To get your PR reviewed, please sign the CLA via the link in the |
|
Thanks for signing the CLA, @TimurRakhmatullin86! 🎉 This PR is now ready for review again and the reviewer has been re-assigned. |
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
Related Issues
None — two self-contained bugs found in
OpenAIResponsesChatGeneratortool handling.Proposed Changes
1. Do not mutate the user's
Toolschema in place.In
_prepare_api_call, tool definitions were built like:Tool.tool_specreturns{"name": ..., "description": ..., "parameters": self.parameters}—parametersby reference — and{**t.tool_spec}only shallow-copies the outer dict. SoadditionalProperties: Falsewas written permanently into the caller's liveTool.parameters. Consequences: the sameToolpassed to another generator (e.g.OpenAIChatGeneratorwithtools_strict=False, which intentionally leaves schemas open) is contaminated, andtool.to_dict()/ a serialize→load→serialize round trip is no longer stable. Fixed by building the definition on a copied parameters dict:{**function_spec["parameters"], "additionalProperties": False}.2. Guard
_warm_up_toolsagainst an empty tools list.is_openai_tool = isinstance(self.tools, list) and isinstance(self.tools[0], dict)raisedIndexErrorfortools=[](an ordinary value when tools are built programmatically). Every sibling check already guards emptiness; this one now does too viabool(self.tools).How did you test it?
Unit tests in
test/components/generators/chat/test_openai_responses.py:test_prepare_api_call_does_not_mutate_tool_parameters— asserts the originalTool.parametersis untouched while the API payload still carriesadditionalProperties=False.test_warm_up_with_empty_tools_list_does_not_raise— warms up withtools=[]without error.Both fail against the current code (
AssertionError/IndexErrorrespectively) and pass with the fix. Fulltest_openai_responses.pyunit suite passes;ruff check/ruff format --checkclean.Checklist