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

fix: anthropic tool use #168

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

edisontim
Copy link
Contributor

anthropic currently sends back a vec on Content structs. Before the fix we were matching on the first element of that. The first element of a ToolUse is rarely ever the tool but more often text so we were never actually getting the tool use as a CompletionResponse

The PR first tries to find a tool use in the vec then some text

@0xMochan
Copy link
Contributor

Thanks for this PR! I think this is something that plagues all providers atm, but it seems like it affects Anthropic a bit more thoroughly. In general, we try to encourage tracking issues to be made before a PR is written so that we can properly understand the bug (it also helps in future maintenance). In this situation, I'd wonder if this bug fix should be applied for every provider.

@edisontim
Copy link
Contributor Author

I've only every used anthropic for my project so haven't noticed it happening in other providers but can change that yeah

Copy link

@kloudsamurai kloudsamurai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for tackling this. I really like Rig but was trying to figure out tools and got snagged here as well. is there a reason tool_use only supports 3 items? would it be more applicable to support n number of tools?

@edisontim
Copy link
Contributor Author

Thank you very much for tackling this. I really like Rig but was trying to figure out tools and got snagged here as well. is there a reason tool_use only supports 3 items? would it be more applicable to support n number of tools?

Maybe I didn't understand this correctly, it supports n tools currently.

Also the ID I've added is for anthropic tool_result response. See here

@cvauclair
Copy link
Contributor

Hey @edisontim sorry for the delay, the new year has been super busy for us!

We're ready to merge your PR! Just need to address the failing CI checks and rebase on main

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.

4 participants