Recursive filter_none in Inference Providers #3178
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
hanouticelina
approved these changes
Jun 25, 2025
Wauplin
commented
Jun 25, 2025
Wauplin
commented
Jun 25, 2025
| ), | ||
| ], | ||
| ) | ||
| def test_prepare_payload_filters_messages(self, raw_messages, expected_messages): |
There was a problem hiding this comment.
thanks for adding this :)
hanouticelina
added a commit
that referenced
this pull request
Jun 25, 2025
* Recursive filter_none in Inference Providers * filter none values from messages as well * update filter_none and add test cases * don't drop empty dicts in list * better typing and refactor logic into a list of comrpehension --------- Co-authored-by: Celina Hanouti <hanouticelina@gmail.com>
mintyleaf
pushed a commit
to Swarmind/huggingface_hub
that referenced
this pull request
Jul 11, 2025
* Recursive filter_none in Inference Providers * filter none values from messages as well * update filter_none and add test cases * don't drop empty dicts in list * better typing and refactor logic into a list of comrpehension --------- Co-authored-by: Celina Hanouti <hanouticelina@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Nonevalues in payload can lead to a BadRequest error if the Inference Provider did not expect the field and is doing input validation. This is for instance what's happening in #3170 with groq complaining iftool_callsandtool_call_idare both set (with None values in them). Groq is complying to the OpenAI specs, this is more a problem on our side but won't be in this PR (*).What we currently do to avoid these issues is to filter out None values using
filter_none. However this helper only removes None values at the top-level of the dictionary. This PR updatesfilter_noneto recursively remove None values. I added some tests to ensure that values like"",0and[]are not removed. This PR should fix the Groq issue and will likely avoid some other errors in the future.(*) regarding why we don't exactly follow the OpenAI specs in our inference types is that we don't have a good way to generate Union type. If a schema is defined as "oneOf", our script will generate a class that is in fact a superset of all possible values, with all of them being optional. See huggingface/huggingface.js#1537 or glideapps/quicktype#1266 for more details. This issue might be annoying on the long run but there is no easy way to address it. Pydantic defines discriminators unions for that which we could also implement. What's harder is to have a reliable and generic
parse_objmethod extending a Union type (which doesn't seem possible). Let's see if/when it becomes for urgent to tackle it.cc @Vaibhavs10 @burtenshaw