-
Notifications
You must be signed in to change notification settings - Fork 162
feat: OllamaChatGenerator - add Toolset support #1765
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
Conversation
@mpangrazzi let's have @anakin87 look at these changes - he's familiar with the code and the context of deepset-ai/haystack#9418 |
@vblagoje lets make sure to pin Haystack in the deps to when we added Toolset |
Looks good in general. I would only update this aspect in the PR description
I understand that this is something you had to introduce to make Langfuse work, but in the current version of this PR we are not introducing this change. |
Right, spot on! I had to remove it pending a proper resolution of deepset-ai/haystack#9418 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from pinning Haystack (see #1765 (comment)), looks good
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, sorry. Looks good!
Why:
Enhance
OllamaChatGenerator
by allowing it to accept aToolset
instance in addition to a list ofTool
objects. This brings it in line with other chat generators and improves consistency and flexibility in tool usage.What:
OllamaChatGenerator
to supporttools
parameter as either aList[Tool]
or aToolset
Toolset
usingserialize_tools_or_toolset
deserialize_tools_or_toolset_inplace
How can it be used:
The
OllamaChatGenerator
can now be used with aToolset
for better tool management and compatibility across generators:It also allows seamless deserialization and execution with
Toolset
as part of the component configuration.How did you test it:
Added tests for:
Toolset
Toolset
-based componentsrun()
execution flow using aToolset
and mocked response fromOllama
clientManually:
Notes for the reviewer:
Please confirm:
ToolCall.id
fallback generation doesn't break traceability or downstream logicNote:
ToolCall
ID requirement into_openai_dict_format
causes failure with non-OpenAI models and Langfuse haystack#9418