-
Notifications
You must be signed in to change notification settings - Fork 9
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: add tool call event #187
Conversation
16fef38
to
f096dba
Compare
Tests currently failing since i moved tool result conversion into the chain processor, but would that be a way to go for you @OskarStark ? final class SpecificToolCallSubscriber implement EventSubscriberInterface
{
public static function getSubscribedEvents()
{
return [
ToolCallExecuted::class => 'handleToolCall',
];
}
public function handleToolCall(ToolCallExecuted $event): void
{
foreach ($event->toolCallResults as $toolCallResult) {
if ('xyz' === $toolCallResult->toolCall->name) {
$event->response = new StructuredResponse($toolCallResult->result);
}
}
}
} See example: llm-chain/examples/toolbox-weather-event.php Lines 26 to 47 in f096dba
|
Uhhhhhhh that looks nice 😍 will have a detailed look when on my Mac 💻👍🏻 Thanks for taking time |
f096dba
to
9a5330e
Compare
examples/toolbox-weather-event.php
Outdated
$eventDispatcher->addListener(ToolCallExecuted::class, function (ToolCallExecuted $event): void { | ||
foreach ($event->toolCallResults as $toolCallResult) { |
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.
The event name is singular and then you iterate over all toolCallResults (plural), that feels weird. Shouldn't we throw an event for every ToolCall which was executed?
Otherwise rename it to ToolCallsExecuted
?
Or am I missing sth.?
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.
you're right. should be plural
I like it 👍 Looks like I can achieve my goals with it 🕺 |
9a5330e
to
4413820
Compare
4413820
to
30e1ff7
Compare
Closes #168