-
Notifications
You must be signed in to change notification settings - Fork 35
Fix empty tool calls and arguments in OpenAI compatible models #80
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
Changes from 1 commit
b1a5c47
7c2fff1
f043ca6
133e576
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -236,17 +236,24 @@ function (Message $message): array { | |
| 'tool_call_id' => $functionResponse->getId(), | ||
| ]; | ||
| } | ||
| return [ | ||
| $messageData = [ | ||
| 'role' => $this->getMessageRoleString($message->getRole()), | ||
| 'content' => array_values(array_filter(array_map( | ||
| [$this, 'getMessagePartContentData'], | ||
| $messageParts | ||
| ))), | ||
| 'tool_calls' => array_values(array_filter(array_map( | ||
| [$this, 'getMessagePartToolCallData'], | ||
| $messageParts | ||
| ))), | ||
| ]; | ||
|
|
||
| // Only include tool_calls if there are any (OpenAI rejects empty arrays). | ||
| $toolCalls = array_values(array_filter(array_map( | ||
| [$this, 'getMessagePartToolCallData'], | ||
| $messageParts | ||
| ))); | ||
| if (!empty($toolCalls)) { | ||
| $messageData['tool_calls'] = $toolCalls; | ||
| } | ||
|
|
||
| return $messageData; | ||
| }, | ||
| $messages | ||
| ); | ||
|
|
@@ -394,12 +401,17 @@ protected function getMessagePartToolCallData(MessagePart $part): ?array | |
| 'The function call typed message part must contain a function call.' | ||
| ); | ||
| } | ||
| $args = $functionCall->getArgs(); | ||
| // Ensure empty arrays become empty objects for JSON encoding. | ||
| if (is_array($args) && empty($args)) { | ||
| $args = (object) array(); | ||
| } | ||
|
||
| return [ | ||
| 'type' => 'function', | ||
| 'id' => $functionCall->getId(), | ||
| 'function' => [ | ||
| 'name' => $functionCall->getName(), | ||
| 'arguments' => json_encode($functionCall->getArgs()), | ||
| 'arguments' => json_encode($args), | ||
| ], | ||
| ]; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -518,6 +518,26 @@ public function testPrepareMessagesParamModelMessageWithFunctionCall(): void | |||||||||||||||||
| ); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Tests prepareMessagesParam with message having no function calls (tool_calls should not be included). | ||||||||||||||||||
| * | ||||||||||||||||||
| * @return void | ||||||||||||||||||
| */ | ||||||||||||||||||
| public function testPrepareMessagesParamNoToolCalls(): void | ||||||||||||||||||
| { | ||||||||||||||||||
| $message = new Message( | ||||||||||||||||||
| MessageRoleEnum::model(), | ||||||||||||||||||
| [new MessagePart('Hello, I am a simple text response.')] | ||||||||||||||||||
| ); | ||||||||||||||||||
|
|
||||||||||||||||||
| $model = $this->createModel(); | ||||||||||||||||||
| $prepared = $model->exposePrepareMessagesParam([$message], null); | ||||||||||||||||||
|
|
||||||||||||||||||
|
||||||||||||||||||
| $model = $this->createModel(); | |
| $prepared = $model->exposePrepareMessagesParam([$message], null); | |
| $model = $this->createModel(); | |
| $prepared = $model->exposePrepareMessagesParam([$message], null); | |
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.
[nitpick] The
(object) array()conversion can be simplified tonew \stdClass()for better readability and clarity of intent when creating an empty object.