Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle empty tool call function arguments #717

Merged
merged 5 commits into from
Nov 26, 2024
Merged

Conversation

jimkring
Copy link
Contributor

@jimkring jimkring commented Nov 26, 2024

This pull request includes changes to improve the handling of tool calls in the mirascope/core package. The updates ensure that the model_json is properly initialized even when tool_call.function.arguments is empty.

Background:
LLMs sometimes use pass an empty string for tool call function args, which was causing an unhandled exception when deserializing the function arguments string to json with jiter.

Solution:
Now, we detect that tool_call.function.arguments is empty and simply initialize an empty dictionary (instead of trying to deserialize an empty string).

Changes to tool call handling:

  • mirascope/core/azure/tool.py: Added a check to initialize model_json to an empty dictionary if tool_call.function.arguments is empty in the from_tool_call method.
  • mirascope/core/groq/tool.py: Added a check to initialize model_json to an empty dictionary if tool_call.function.arguments is empty in the from_tool_call method.
  • mirascope/core/openai/tool.py: Added a check to initialize model_json to an empty dictionary if tool_call.function.arguments is empty in the from_tool_call method.

LLMs can return an empty string for no args, so need to handle that case to prevent an error trying to convert to dict from json with jiter.
@jimkring jimkring requested a review from willbakst as a code owner November 26, 2024 03:43
Copy link

codecov bot commented Nov 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (64c7ea5) to head (72b2d95).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #717   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          398       398           
  Lines        13298     13292    -6     
=========================================
- Hits         13298     13292    -6     
Flag Coverage Δ
tests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@willbakst
Copy link
Contributor

@jimkring good catch!

could you also update the remaining 7 providers?

@jimkring
Copy link
Contributor Author

@willbakst I'll take a look. I did a search and only found those three identical bits of code. Maybe the other providers are subtly different.

@willbakst
Copy link
Contributor

@willbakst I'll take a look. I did a search and only found those three identical bits of code. Maybe the other providers are subtly different.

oh you're right because of the issue being with jiter

can you update mistral to match the new recommended style from my other comment?

also, i just noticed that gemini and vertex have an issue where it will not allow empty args (when it should), would be great if you could update those as well as part of this

@jimkring
Copy link
Contributor Author

@willbakst I've updated several other files/providers. Please take a look at how the differ in subtle ways. In some cases the arguments from the tool call are coming from the provide as a string, in other cases it's a Pydantic model. I don't have a way to test all these special cases, so we'll want to be thoughtful about our review and scope of changes.

@willbakst
Copy link
Contributor

so we'll want to be thoughtful about our review and scope of changes.

happy to provide comments, also happy to take this over the finish line, just let me know :)

@jimkring
Copy link
Contributor Author

jimkring commented Nov 26, 2024

@willbakst it's all yours to finish up, now. Thanks for the style suggestions and fixes -- I learned a couple things 👍

@willbakst willbakst merged commit 4365d60 into Mirascope:main Nov 26, 2024
7 checks passed
@willbakst
Copy link
Contributor

this is now released in v1.10.1

thank you for the contribution!!

@jimkring jimkring deleted the patch-3 branch November 26, 2024 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants