-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat!: HuggingFaceLocalChatGenerator unified support for tools #8827
base: main
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 13200235309Details
💛 - Coveralls |
@davidsbatista asking @anakin87 for review here as he knows this code intimately 🙏 |
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.
Nice work! I particularly appreciated the notebook...
I left some comments about points we can improve.
Have a look at the updated run of the notebook as well @anakin87 🙏 |
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.
I left a small comment to address.
Plus, "feat!:" in the PR title indicates a breaking change, and this does not seem breaking.
If I am right, let's simply change this to "feat:"
Feel free to merge once these small aspects are fixed.
Good work!
generator = HuggingFaceLocalChatGenerator( | ||
model="meta-llama/Llama-2-13b-chat-hf", tools=tools, tool_parsing_function=custom_tool_parser | ||
) | ||
generator.pipeline = Mock(return_value=[{"generated_text": "Let me check the weather for you"}]) |
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.
this generated_text
is confusing. Please use something like "mock response. We don't use it"
Uses DEFAULT_TOOL_PATTERN to extract tool calls. | ||
:param text: The text to parse for tool calls. | ||
:returns: A list containing a single ToolCall if a valid tool call is found, None otherwise. |
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.
we are only extracting a single tool call, but this is clearly explained.
We can improve this in future if there are requests from the community.
Why:
Adds universal tools support to
HuggingFaceLocalChatGenerator
What:
How can it be used:
The addition allows for parsing tool calls from generated text, identified by tool names and their arguments. This can be set up during initialization:
How did you test it:
Notes for the reviewer: