Skip to content

[Agent] Code grooming around tool call #1010

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.

Sign up for GitHub

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

Open
lyrixx wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Agent] Code grooming around tool call #1010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 30 additions & 29 deletions src/agent/src/Toolbox/AgentProcessor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ public function processOutput(Output $output): void

if ($result instanceof GenericStreamResponse) {
$output->setResult(
new ToolboxStreamResponse($result->getContent(), $this->handleToolCallsCallback($output))
new ToolboxStreamResponse(
$result->getContent(),
fn (ToolCallResult $result, ?AssistantMessage $streamedAssistantResponse = null) => $this->handleToolCallsCallback($output, $result, $streamedAssistantResponse)
)
);

return;
Expand All @@ -90,7 +93,7 @@ public function processOutput(Output $output): void
return;
}

$output->setResult($this->handleToolCallsCallback($output)($result));
$output->setResult($this->handleToolCallsCallback($output, $result));
}

/**
Expand All @@ -101,40 +104,38 @@ private function isFlatStringArray(array $tools): bool
return array_reduce($tools, fn (bool $carry, mixed $item) => $carry && \is_string($item), true);
}

private function handleToolCallsCallback(Output $output): \Closure
private function handleToolCallsCallback(Output $output, ToolCallResult $result, ?AssistantMessage $streamedAssistantResponse = null): ResultInterface
Copy link
Member Author

@lyrixx lyrixx Nov 27, 2025

Choose a reason for hiding this comment

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

I reduced the indirection. This reduce the depth in the stack.

{
return function (ToolCallResult $result, ?AssistantMessage $streamedAssistantResponse = null) use ($output): ResultInterface {
++$this->nestingLevel;
$messages = $this->keepToolMessages ? $output->getMessageBag() : clone $output->getMessageBag();
++$this->nestingLevel;
$messages = $this->keepToolMessages ? $output->getMessageBag() : clone $output->getMessageBag();

if (null !== $streamedAssistantResponse && '' !== $streamedAssistantResponse->getContent()) {
$messages->add($streamedAssistantResponse);
}
if (null !== $streamedAssistantResponse && '' !== $streamedAssistantResponse->getContent()) {
$messages->add($streamedAssistantResponse);
}

do {
$toolCalls = $result->getContent();
$messages->add(Message::ofAssistant(toolCalls: $toolCalls));
do {
$toolCalls = $result->getContent();
$messages->add(Message::ofAssistant(toolCalls: $toolCalls));

$results = [];
foreach ($toolCalls as $toolCall) {
$results[] = $toolResult = $this->toolbox->execute($toolCall);
$messages->add(Message::ofToolCall($toolCall, $this->resultConverter->convert($toolResult)));
array_push($this->sources, ...$toolResult->getSources());
}
$results = [];
foreach ($toolCalls as $toolCall) {
$results[] = $toolResult = $this->toolbox->execute($toolCall);
$messages->add(Message::ofToolCall($toolCall, $this->resultConverter->convert($toolResult)));
array_push($this->sources, ...$toolResult->getSources());
}

$event = new ToolCallsExecuted(...$results);
$this->eventDispatcher?->dispatch($event);
$event = new ToolCallsExecuted(...$results);
$this->eventDispatcher?->dispatch($event);

$result = $event->hasResult() ? $event->getResult() : $this->agent->call($messages, $output->getOptions());
} while ($result instanceof ToolCallResult);
$result = $event->hasResult() ? $event->getResult() : $this->agent->call($messages, $output->getOptions());
} while ($result instanceof ToolCallResult);

--$this->nestingLevel;
if ($this->includeSources && 0 === $this->nestingLevel) {
$result->getMetadata()->add('sources', $this->sources);
$this->sources = [];
}
--$this->nestingLevel;
if ($this->includeSources && 0 === $this->nestingLevel) {
$result->getMetadata()->add('sources', $this->sources);
$this->sources = [];
}

return $result;
};
return $result;
}
}
12 changes: 6 additions & 6 deletions src/agent/src/Toolbox/ToolFactory/MemoryToolFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,16 @@ public function addTool(string|object $class, string $name, string $description,
}

/**
* @param class-string $reference
* @param class-string $className
Copy link
Member Author

@lyrixx lyrixx Nov 27, 2025

Choose a reason for hiding this comment

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

Let's use only one name for a "thing". Line 30, this is called className.
I think it's better to be consistant, it's easier to understand

Copy link
Member

@chr-hertel chr-hertel Nov 29, 2025

Choose a reason for hiding this comment

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

doesn't have to be a class tho, right?

Copy link
Member Author

@lyrixx lyrixx Dec 2, 2025

Choose a reason for hiding this comment

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

I'm not sure to understand. What could it be, if it's not a class, but the type is class-string?

*/
public function getTool(string $reference): iterable
public function getTool(string $className): iterable
{
if (!isset($this->tools[$reference])) {
throw ToolException::invalidReference($reference);
if (!isset($this->tools[$className])) {
throw ToolException::invalidReference($className);
}

foreach ($this->tools[$reference] as $tool) {
yield $this->convertAttribute($reference, $tool);
foreach ($this->tools[$className] as $tool) {
yield $this->convertAttribute($className, $tool);
}
}
}
12 changes: 6 additions & 6 deletions src/agent/src/Toolbox/Toolbox.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ final class Toolbox implements ToolboxInterface
*
* @var Tool[]
*/
private array $map;
private array $toolsMetadata;

/**
* @param iterable<object> $tools
Expand All @@ -60,18 +60,18 @@ public function __construct(

public function getTools(): array
Copy link
Member Author

@lyrixx lyrixx Nov 27, 2025 •
edited by OskarStark
Loading

Choose a reason for hiding this comment

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

There is something a bit hard to understand.
getTools does not return tool, but tools metadata !

I would like to rebame Tool to ToolMetadata, getTools() to getToolsMetadata()

It will really ease on-boarding I guess.

And it will fix inconsistency. For example :

private function getMetadata(ToolCall $toolCall): Tool

{
if (isset($this->map)) {
return $this->map;
if (isset($this->toolsMetadata)) {
return $this->toolsMetadata;
}

$map = [];
$toolsMetadata = [];
foreach ($this->tools as $tool) {
foreach ($this->toolFactory->getTool($tool::class) as $metadata) {
$map[] = $metadata;
$toolsMetadata[] = $metadata;
}
}

return $this->map = $map;
return $this->toolsMetadata = $toolsMetadata;
}

public function execute(ToolCall $toolCall): ToolResult
Expand Down
2 changes: 1 addition & 1 deletion src/platform/src/Bridge/OpenAi/Gpt/ResultConverter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ private function isToolCallsStreamFinished(array $data): bool
private function convertChoice(array $choice): ToolCallResult|TextResult
{
if ('tool_calls' === $choice['finish_reason']) {
return new ToolCallResult(...array_map([$this, 'convertToolCall'], $choice['message']['tool_calls']));
return new ToolCallResult(...array_map($this->convertToolCall(...), $choice['message']['tool_calls']));
}

if (\in_array($choice['finish_reason'], ['stop', 'length'], true)) {
Expand Down