fix: recursively apply strict schema constraints for tools_strict=True#11232
fix: recursively apply strict schema constraints for tools_strict=True#11232ArkaD171717 wants to merge 5 commits intodeepset-ai:mainfrom
Conversation
The existing code only set additionalProperties: false at the top level of tool parameter schemas. OpenAI strict mode requires it recursively on all nested objects, $defs, array items, and anyOf/oneOf/allOf branches. Add _make_schema_strict() that walks the schema recursively and sets additionalProperties: false + required on every object. Replace the single-line top-level-only fix in _prepare_api_call. 10 new tests including a 4-level-deep ComponentTool-style schema. Closes deepset-ai#9411
|
@ArkaD171717 is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
|
Fixed ruff formatting and added release note |
|
@sjrl, could you please take a look at this PR? |
|
Hey I noticed the reno check failed, it was a UID collision on the release note filename and i pushed the fix. |
There was a problem hiding this comment.
Thanks for all of the unit tests, but could we also make one integration test that would have failed before this change? Perhaps take a complicated one like test_complex_schema_with_defs_and_combinators and make an integration test version of it as well.
There was a problem hiding this comment.
I pushed a fix where I added test_prepare_api_call_strict_complex_tool; it takes the same schema structure from test_complex_schema_with_defs_and_combinators and puts it thru _prepare_api_call with tools_strict=True
Fails on the old code b/c nested objects dont have additionalProperties: false
There was a problem hiding this comment.
Right thanks but we should still add an integration test so we can see if the schema is actually accepted by OpenAI. We have other integration tests in this file to see how to structure them to be skipped if an API key is missing.
There was a problem hiding this comment.
Added test_live_run_strict_nested_tool and test_prepare_api_call_strict_component_tool
…gration test - single backticks in docstring instead of double - link to OpenAI structured outputs docs - all unit tests use full dict comparison instead of checking individual keys - added integration test with complex schema through _prepare_api_call
|
Pushed fix for backtick formatting changed to full dict comparisons, added the schemas link in the docstring and added a harder integration test that wouldve failed on the old version |
- test_prepare_api_call_strict_component_tool verifies ComponentTool with ChatMessage params gets all nested $defs strictified - test_live_run_strict_nested_tool hits the OpenAI API with a nested tool schema under tools_strict=True to confirm acceptance
tools_strict=Truewas only settingadditionalProperties: falseat the toplevel of tool parameter schemas
OpenAI's strict mode rejects anything with nested objects that don't also have
additionalProperties: falseand a completerequiredlistSo any tool with nested params (like a ComponentTool wrapping a component that
takes ChatMessage-shaped input) would fail at the API
Adds
_make_schema_strict()that walks the schema recursively: nestedproperties,
$defs, arrayitems,anyOf/oneOf/allOfbranchesSets the strict constraints at every level
Replaces the single-line top-level-only fix in
_prepare_api_callReturns a copy, doesn't mutate the original schema dict
10 new tests including a 4-level-deep ComponentTool-style schema
All 37 existing tests still pass
Closes #9411