Pinned latest litellm for tool_choice=Tool's finish_reason fix
#244
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
jamesbraza
requested review from
whitead,
sidnarayanan,
mskarlin,
maykcaldas,
albertbou92,
ludomitch,
Eddie-MG and
nadolskit
There was a problem hiding this comment.
Pull Request Overview
This PR updates the dependency for litellm to a pinned version (>=1.72.0) to address issues with the finish_reason behavior when using tool_choice, and removes an outdated assignment and TODO comment in the utils module.
- Pin litellm dependency to version 1.72.0
- Remove the unused expected_finish_reason variable and its associated comment
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/aviary/tools/utils.py | Removed an unused expected_finish_reason variable and its TODO |
| pyproject.toml | Updated and simplified the litellm dependency pinning |
Comments suppressed due to low confidence (2)
src/aviary/tools/utils.py:71
- Since litellm now fixes the finish_reason behavior, consider removing the outdated TODO and verifying that no other part of the code depends on the expected_finish_reason variable.
- expected_finish_reason = {"stop"} # TODO: should this be .add("stop") too?
pyproject.toml:82
- Ensure that removing the python_version dependent conditions for litellm does not impact environments that might require a lower version; if all targeted environments are supported by this version, the change is acceptable.
+ "litellm>=1.72.0", # Pin for fixed finish_reason when specifying a tool_choice
dosubot
bot
added
the
size:XS
jamesbraza
commented
Jun 13, 2025
| @@ -69,7 +69,6 @@ async def __call__( | |||
| "type": "function", | |||
| "function": {"name": tool_choice.info.name}, | |||
| } | |||
| expected_finish_reason = {"stop"} # TODO: should this be .add("stop") too? | |||
There was a problem hiding this comment.
I never understood why the finish_reason was "stop" when we'd directly specified to call a Tool.
Turns out it was a litellm bug fixed sometime between litellm==1.71.1 and litellm==1.72.0
nadolskit
approved these changes
Jun 13, 2025
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.
Apparently the latest
litellmminor version fixed some unexpected behavior