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 4 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. |
| Sets ``additionalProperties: false`` on all objects and ensures every defined | ||
| property is listed in ``required``. Walks into nested properties, ``$defs``, | ||
| array ``items``, and ``anyOf``/``oneOf``/``allOf`` combinators. |
There was a problem hiding this comment.
We only use single back ticks in our docstrings. The double backticks are only needed for the release note.
| assert result["additionalProperties"] is False | ||
| assert result["required"] == ["name"] |
There was a problem hiding this comment.
I realize this will make it slightly more verbose but could you do a full dict comparison instead of testing a few elements. So assert result == {...} for all of the tests.
|
|
||
| def _make_schema_strict(schema: dict[str, Any]) -> dict[str, Any]: | ||
| """ | ||
| Recursively transform a JSON schema to be OpenAI strict-mode compliant. |
There was a problem hiding this comment.
Could we link the docs page from OpenAI explaining these requirements?
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
…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 |
4 participants
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