-
Notifications
You must be signed in to change notification settings - Fork 199
feat: NvidiaChatGenerator add integration tests for mixing Tool/Toolset
#2422
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
| # Pass echo_tool as a list at runtime - runtime tools should take precedence | ||
| messages = [ChatMessage.from_user("Echo this: Hello World")] | ||
| results = component.run(messages, tools=[echo_tool]) |
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.
Same comment about mixing tools and toolsets at runtime.
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.
but this one is different: there is no mention of tools and we need to wrap single tool into a list
| def test_integration_mixing_init_and_runtime_tools(self): | ||
| """Test mixing tools from init and runtime by passing tools to both __init__ and run().""" |
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 am not sure if this test was intended. We can rather have a test for ser/deser of mixed tool lists like in GoogleGenAiChatGenerator.
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.
@vblagoje lets remove this test. I still dont see the need of keeping it. The rest looks good.
Why:
Add integration tests for
ToolsTypeHaystack 2.19 updatespart of:
What:
How did you test it: