From 7d5a9d52d63307b47cdd71c50a0065bae37e8c6d Mon Sep 17 00:00:00 2001 From: Ondrej Popelka Date: Tue, 18 Feb 2025 10:52:05 +0100 Subject: [PATCH 1/2] Fix: support finish reason "length" The reason is real https://community.openai.com/t/bug-in-api-response-finish-reason-field/287212 and is even mentioned here https://github.com/php-llm/llm-chain/blob/5da000ec0fb4fcc4d78399e40eac05506cfff6fc/src/Bridge/OpenAI/GPT/ResponseConverter.php#L159 Plus added some tests for the ResponseConverter --- src/Bridge/OpenAI/GPT/ResponseConverter.php | 2 +- .../OpenAI/GPT/ResponseConverterTest.php | 177 ++++++++++++++++++ 2 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 tests/Bridge/OpenAI/GPT/ResponseConverterTest.php diff --git a/src/Bridge/OpenAI/GPT/ResponseConverter.php b/src/Bridge/OpenAI/GPT/ResponseConverter.php index c3daa655..11b9584a 100644 --- a/src/Bridge/OpenAI/GPT/ResponseConverter.php +++ b/src/Bridge/OpenAI/GPT/ResponseConverter.php @@ -165,7 +165,7 @@ private function convertChoice(array $choice): Choice return new Choice(toolCalls: array_map([$this, 'convertToolCall'], $choice['message']['tool_calls'])); } - if ('stop' === $choice['finish_reason']) { + if (in_array($choice['finish_reason'], ['stop', 'length'], true)) { return new Choice($choice['message']['content']); } diff --git a/tests/Bridge/OpenAI/GPT/ResponseConverterTest.php b/tests/Bridge/OpenAI/GPT/ResponseConverterTest.php new file mode 100644 index 00000000..3ea23495 --- /dev/null +++ b/tests/Bridge/OpenAI/GPT/ResponseConverterTest.php @@ -0,0 +1,177 @@ +createMock(ResponseInterface::class); + $httpResponse->method('toArray')->willReturn([ + 'choices' => [ + [ + 'message' => [ + 'role' => 'assistant', + 'content' => 'Hello world', + ], + 'finish_reason' => 'stop', + ], + ], + ]); + + $response = $converter->convert($httpResponse); + + $this->assertInstanceOf(TextResponse::class, $response); + $this->assertEquals('Hello world', $response->getContent()); + } + + public function testConvertToolCallResponse(): void + { + $converter = new ResponseConverter(); + $httpResponse = $this->createMock(ResponseInterface::class); + $httpResponse->method('toArray')->willReturn([ + 'choices' => [ + [ + 'message' => [ + 'role' => 'assistant', + 'content' => null, + 'tool_calls' => [ + [ + 'id' => 'call_123', + 'type' => 'function', + 'function' => [ + 'name' => 'test_function', + 'arguments' => '{"arg1": "value1"}', + ], + ], + ], + ], + 'finish_reason' => 'tool_calls', + ], + ], + ]); + + $response = $converter->convert($httpResponse); + + $this->assertInstanceOf(ToolCallResponse::class, $response); + $toolCalls = $response->getContent(); + $this->assertCount(1, $toolCalls); + $this->assertEquals('call_123', $toolCalls[0]->id); + $this->assertEquals('test_function', $toolCalls[0]->name); + $this->assertEquals(['arg1' => 'value1'], $toolCalls[0]->arguments); + } + + public function testConvertMultipleChoices(): void + { + $converter = new ResponseConverter(); + $httpResponse = $this->createMock(ResponseInterface::class); + $httpResponse->method('toArray')->willReturn([ + 'choices' => [ + [ + 'message' => [ + 'role' => 'assistant', + 'content' => 'Choice 1', + ], + 'finish_reason' => 'stop', + ], + [ + 'message' => [ + 'role' => 'assistant', + 'content' => 'Choice 2', + ], + 'finish_reason' => 'stop', + ], + ], + ]); + + $response = $converter->convert($httpResponse); + + $this->assertInstanceOf(ChoiceResponse::class, $response); + $choices = $response->getContent(); + $this->assertCount(2, $choices); + $this->assertEquals('Choice 1', $choices[0]->getContent()); + $this->assertEquals('Choice 2', $choices[1]->getContent()); + } + + public function testContentFilterException(): void + { + $converter = new ResponseConverter(); + $httpResponse = $this->createMock(ResponseInterface::class); + + $httpResponse->expects($this->exactly(2)) + ->method('toArray') + ->willReturnCallback(function ($throw = true) { + if ($throw) { + throw new class extends Exception implements ClientExceptionInterface { + public function getResponse(): ResponseInterface + { + throw new RuntimeException('Not implemented'); + } + }; + } + return [ + 'error' => [ + 'code' => 'content_filter', + 'message' => 'Content was filtered', + ], + ]; + }); + + $this->expectException(ContentFilterException::class); + $this->expectExceptionMessage('Content was filtered'); + + $converter->convert($httpResponse); + } + + public function testThrowsExceptionWhenNoChoices(): void + { + $converter = new ResponseConverter(); + $httpResponse = $this->createMock(ResponseInterface::class); + $httpResponse->method('toArray')->willReturn([]); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Response does not contain choices'); + + $converter->convert($httpResponse); + } + + public function testThrowsExceptionForUnsupportedFinishReason(): void + { + $converter = new ResponseConverter(); + $httpResponse = $this->createMock(ResponseInterface::class); + $httpResponse->method('toArray')->willReturn([ + 'choices' => [ + [ + 'message' => [ + 'role' => 'assistant', + 'content' => 'Test content', + ], + 'finish_reason' => 'unsupported_reason', + ], + ], + ]); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Unsupported finish reason "unsupported_reason"'); + + $converter->convert($httpResponse); + } +} From 0ab559d8ea46e587326d4a878d76d33d68564e5b Mon Sep 17 00:00:00 2001 From: Ondrej Popelka Date: Tue, 18 Feb 2025 12:24:33 +0100 Subject: [PATCH 2/2] chore: fix code style --- tests/Bridge/OpenAI/GPT/ResponseConverterTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Bridge/OpenAI/GPT/ResponseConverterTest.php b/tests/Bridge/OpenAI/GPT/ResponseConverterTest.php index 3ea23495..c64867e7 100644 --- a/tests/Bridge/OpenAI/GPT/ResponseConverterTest.php +++ b/tests/Bridge/OpenAI/GPT/ResponseConverterTest.php @@ -4,7 +4,6 @@ namespace PhpLlm\LlmChain\Tests\Bridge\OpenAI\GPT; -use Exception; use PhpLlm\LlmChain\Bridge\OpenAI\GPT\ResponseConverter; use PhpLlm\LlmChain\Exception\ContentFilterException; use PhpLlm\LlmChain\Exception\RuntimeException; @@ -120,13 +119,14 @@ public function testContentFilterException(): void ->method('toArray') ->willReturnCallback(function ($throw = true) { if ($throw) { - throw new class extends Exception implements ClientExceptionInterface { + throw new class extends \Exception implements ClientExceptionInterface { public function getResponse(): ResponseInterface { throw new RuntimeException('Not implemented'); } }; } + return [ 'error' => [ 'code' => 'content_filter',