From d9bbeecd4825e113b168986e551a73e61455c321 Mon Sep 17 00:00:00 2001 From: Marcello Duarte Date: Sat, 25 Jul 2026 20:46:56 +0100 Subject: [PATCH] Harden core against provider-specific coupling (OCP/DIP) Add a neutral Usage value object; Response and EmbeddingResponse store it and accept Usage|array, with ArrayAccess preserving legacy key access. This fixes token counts returning 0 for OpenAI-family providers and Cohere. Remove toAnthropic() and toOpenAI() from ToolInterface; keep them deprecated on the concrete Tool. Agent now emits a neutral tool definition (name, description, parameters) and each provider formats it. Deprecate Response::fromAnthropic() and ToolCall::fromAnthropic(); provider response mapping now lives in the provider packages. No public method removed; fully backward compatible. --- src/Agent.php | 7 +- src/Contracts/ToolInterface.php | 18 +---- src/EmbeddingResponse.php | 19 +++-- src/Response.php | 32 ++++++--- src/Tool.php | 14 +++- src/ToolCall.php | 3 + src/Usage.php | 123 ++++++++++++++++++++++++++++++++ tests/Unit/AgentBuilderTest.php | 3 +- tests/Unit/AgentTest.php | 27 +++++++ tests/Unit/UsageTest.php | 99 +++++++++++++++++++++++++ 10 files changed, 310 insertions(+), 35 deletions(-) create mode 100644 src/Usage.php create mode 100644 tests/Unit/UsageTest.php diff --git a/src/Agent.php b/src/Agent.php index 3c966f9..2111168 100644 --- a/src/Agent.php +++ b/src/Agent.php @@ -294,8 +294,13 @@ private function getProviderOptions(): array } if (!empty($this->tools)) { + // Pass a neutral tool definition; each provider formats it to its own wire shape. $options['tools'] = array_values(array_map( - fn (ToolInterface $tool) => $tool->toAnthropic(), + fn (ToolInterface $tool) => [ + 'name' => $tool->getName(), + 'description' => $tool->getDescription(), + 'parameters' => $tool->getParameterSchema(), + ], $this->tools )); } diff --git a/src/Contracts/ToolInterface.php b/src/Contracts/ToolInterface.php index 41be1ce..5cd5259 100644 --- a/src/Contracts/ToolInterface.php +++ b/src/Contracts/ToolInterface.php @@ -18,8 +18,8 @@ * Contract for tools that can be invoked by an LLM agent. * * Tools expose a name, description, and parameter schema so the LLM can decide - * when and how to call them. Implementations must be serialisable to both - * Anthropic and OpenAI API formats. + * when and how to call them. Each provider translates that neutral definition + * into its own wire format. */ interface ToolInterface { @@ -52,18 +52,4 @@ public function getParameterSchema(): array; * @return mixed The tool result */ public function execute(array $arguments, mixed $context = null): mixed; - - /** - * Convert to Anthropic API tool format. - * - * @return array{name: string, description: string, input_schema: array} - */ - public function toAnthropic(): array; - - /** - * Convert to OpenAI API tool format. - * - * @return array{type: string, function: array{name: string, description: string, parameters: array}} - */ - public function toOpenAI(): array; } diff --git a/src/EmbeddingResponse.php b/src/EmbeddingResponse.php index 3842f2a..f65ae80 100644 --- a/src/EmbeddingResponse.php +++ b/src/EmbeddingResponse.php @@ -22,16 +22,25 @@ */ final class EmbeddingResponse { + /** + * Token usage for the request. + * + * Typed as the neutral Usage value object; it is still array-accessible + * (`$response->usage['prompt_tokens']`) for backward compatibility. + */ + public readonly Usage $usage; + /** * @param array> $embeddings The embedding vectors - * @param string $model The model used - * @param array{prompt_tokens?: int, total_tokens?: int} $usage Token usage + * @param string $model The model used + * @param Usage|array $usage Token usage (a raw provider array is accepted and normalised) */ public function __construct( public readonly array $embeddings, public readonly string $model, - public readonly array $usage = [], + Usage|array $usage = [], ) { + $this->usage = is_array($usage) ? Usage::fromArray($usage) : $usage; } /** @@ -71,7 +80,7 @@ public function dimensions(): int */ public function getPromptTokens(): int { - return $this->usage['prompt_tokens'] ?? 0; + return $this->usage->inputTokens; } /** @@ -81,6 +90,6 @@ public function getPromptTokens(): int */ public function getTotalTokens(): int { - return $this->usage['total_tokens'] ?? 0; + return $this->usage->totalTokens; } } diff --git a/src/Response.php b/src/Response.php index 13f7c66..dfa33ec 100644 --- a/src/Response.php +++ b/src/Response.php @@ -23,21 +23,30 @@ final class Response { /** - * @param string $text The text response from the LLM - * @param array|null $data Parsed structured data (if output schema was used) - * @param array $toolCalls Tool calls made by the LLM - * @param array $messages Full conversation history - * @param array $usage Token usage statistics - * @param string|null $stopReason Why the response ended + * Token usage for the request. + * + * Typed as the neutral Usage value object; it is still array-accessible + * (`$response->usage['input_tokens']`) for backward compatibility. + */ + public readonly Usage $usage; + + /** + * @param string $text The text response from the LLM + * @param array|null $data Parsed structured data (if output schema was used) + * @param array $toolCalls Tool calls made by the LLM + * @param array $messages Full conversation history + * @param Usage|array $usage Token usage (a raw provider array is accepted and normalised) + * @param string|null $stopReason Why the response ended */ public function __construct( public readonly string $text, public readonly ?array $data = null, public readonly array $toolCalls = [], public readonly array $messages = [], - public readonly array $usage = [], + Usage|array $usage = [], public readonly ?string $stopReason = null, ) { + $this->usage = is_array($usage) ? Usage::fromArray($usage) : $usage; } /** @@ -67,7 +76,7 @@ public function hasData(): bool */ public function getInputTokens(): int { - return $this->usage['input_tokens'] ?? 0; + return $this->usage->inputTokens; } /** @@ -77,7 +86,7 @@ public function getInputTokens(): int */ public function getOutputTokens(): int { - return $this->usage['output_tokens'] ?? 0; + return $this->usage->outputTokens; } /** @@ -87,12 +96,15 @@ public function getOutputTokens(): int */ public function getTotalTokens(): int { - return $this->getInputTokens() + $this->getOutputTokens(); + return $this->usage->totalTokens; } /** * Create from an Anthropic API response payload. * + * @deprecated Response mapping now lives in each provider package (see AnthropicProvider); build a + * Response with the neutral constructor instead. Kept for backward compatibility. + * * @param array $response Raw Anthropic API response * @param array $messages Conversation history to attach * diff --git a/src/Tool.php b/src/Tool.php index 71784f5..ee4f228 100644 --- a/src/Tool.php +++ b/src/Tool.php @@ -209,7 +209,12 @@ public function execute(array $arguments, mixed $context = null): mixed } /** - * Convert to format expected by Anthropic API. + * Convert to the format expected by the Anthropic API. + * + * @deprecated Tool wire formatting now lives in each provider package; the Agent passes a neutral + * definition (name, description, parameters) and the provider formats it. Kept for BC. + * + * @return array{name: string, description: string, input_schema: array} */ public function toAnthropic(): array { @@ -221,7 +226,12 @@ public function toAnthropic(): array } /** - * Convert to format expected by OpenAI API. + * Convert to the format expected by the OpenAI API. + * + * @deprecated Tool wire formatting now lives in each provider package; the Agent passes a neutral + * definition (name, description, parameters) and the provider formats it. Kept for BC. + * + * @return array{type: string, function: array{name: string, description: string, parameters: array}} */ public function toOpenAI(): array { diff --git a/src/ToolCall.php b/src/ToolCall.php index 4c20baa..43b1d83 100644 --- a/src/ToolCall.php +++ b/src/ToolCall.php @@ -37,6 +37,9 @@ public function __construct( /** * Create from an Anthropic API tool_use content block. * + * @deprecated Provider mapping now lives in each provider package; construct ToolCall directly. + * Kept for backward compatibility. + * * @param array{id: string, name: string, input?: array} $data Anthropic tool_use block * * @return self Parsed tool call diff --git a/src/Usage.php b/src/Usage.php new file mode 100644 index 0000000..ce4b2c4 --- /dev/null +++ b/src/Usage.php @@ -0,0 +1,123 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace PapiAI\Core; + +use ArrayAccess; +use LogicException; + +/** + * Immutable, provider-neutral token usage for a request. + * + * Normalises the different key names providers use (Anthropic `input_tokens`/`output_tokens`, + * OpenAI `prompt_tokens`/`completion_tokens`) into `inputTokens`/`outputTokens`/`totalTokens`. + * + * Implements ArrayAccess against the legacy key names purely for backward compatibility, so code + * written against the old raw usage array (for example `$response->usage['input_tokens']`) keeps + * working. New code should read the typed properties, or the getters on Response/EmbeddingResponse. + * + * @implements ArrayAccess + */ +final class Usage implements ArrayAccess +{ + public readonly int $totalTokens; + + /** + * @param int $inputTokens Prompt/input tokens + * @param int $outputTokens Completion/output tokens + * @param int|null $totalTokens Total tokens; defaults to inputTokens + outputTokens + * @param array $raw The original provider usage payload, preserved verbatim + */ + public function __construct( + public readonly int $inputTokens = 0, + public readonly int $outputTokens = 0, + ?int $totalTokens = null, + private readonly array $raw = [], + ) { + $this->totalTokens = $totalTokens ?? ($this->inputTokens + $this->outputTokens); + } + + /** + * Build from a raw provider usage array, mapping the known key aliases. + * + * @param array $usage Provider usage payload in any of the known key styles + */ + public static function fromArray(array $usage): self + { + $input = $usage['input_tokens'] ?? $usage['prompt_tokens'] ?? 0; + $output = $usage['output_tokens'] ?? $usage['completion_tokens'] ?? 0; + $total = $usage['total_tokens'] ?? null; + + return new self( + (int) $input, + (int) $output, + $total !== null ? (int) $total : null, + $usage, + ); + } + + /** + * The original provider usage payload, preserved verbatim. + * + * @return array + */ + public function toArray(): array + { + return $this->raw; + } + + /** + * @deprecated Legacy array-access shim; read the typed properties (inputTokens, etc.) instead. + */ + public function offsetExists(mixed $offset): bool + { + return match ($offset) { + 'input_tokens', 'prompt_tokens', 'output_tokens', 'completion_tokens', 'total_tokens' => true, + default => isset($this->raw[$offset]), + }; + } + + /** + * @deprecated Legacy array-access shim; read the typed properties (inputTokens, etc.) instead. + */ + public function offsetGet(mixed $offset): mixed + { + return match ($offset) { + 'input_tokens', 'prompt_tokens' => $this->inputTokens, + 'output_tokens', 'completion_tokens' => $this->outputTokens, + 'total_tokens' => $this->totalTokens, + default => $this->raw[$offset] ?? null, + }; + } + + /** + * @deprecated Usage is immutable; this shim only exists to satisfy ArrayAccess. + * + * @throws LogicException Always, because Usage is immutable + */ + public function offsetSet(mixed $offset, mixed $value): void + { + throw new LogicException('Usage is immutable.'); + } + + /** + * @deprecated Usage is immutable; this shim only exists to satisfy ArrayAccess. + * + * @throws LogicException Always, because Usage is immutable + */ + public function offsetUnset(mixed $offset): void + { + throw new LogicException('Usage is immutable.'); + } +} diff --git a/tests/Unit/AgentBuilderTest.php b/tests/Unit/AgentBuilderTest.php index d51b620..524891e 100644 --- a/tests/Unit/AgentBuilderTest.php +++ b/tests/Unit/AgentBuilderTest.php @@ -44,7 +44,8 @@ it('accepts tools', function () { $tool = Mockery::mock(ToolInterface::class); $tool->allows('getName')->andReturn('test_tool'); - $tool->allows('toAnthropic')->andReturn([]); + $tool->allows('getDescription')->andReturn('A test tool'); + $tool->allows('getParameterSchema')->andReturn(['type' => 'object', 'properties' => []]); $this->provider->expects('chat')->andReturn(new Response(text: 'OK')); diff --git a/tests/Unit/AgentTest.php b/tests/Unit/AgentTest.php index 05785d9..eeb6843 100644 --- a/tests/Unit/AgentTest.php +++ b/tests/Unit/AgentTest.php @@ -124,6 +124,33 @@ $agent->run('Hello'); }); + it('passes neutral tool definitions to the provider (not the Anthropic shape)', function () { + $tool = Tool::make( + name: 'lookup', + description: 'Look something up', + parameters: ['query' => ['type' => 'string', 'description' => 'the query']], + handler: fn () => 'ok', + ); + + $this->mockProvider + ->expects('chat') + ->withArgs(function ($messages, $options) use ($tool) { + $def = $options['tools'][0] ?? []; + + return $def === [ + 'name' => 'lookup', + 'description' => 'Look something up', + 'parameters' => $tool->getParameterSchema(), + ] + && !array_key_exists('input_schema', $def); + }) + ->andReturn(new Response(text: 'OK')); + + $agent = new Agent(provider: $this->mockProvider, model: 'test-model', tools: [$tool]); + + $agent->run('Hi'); + }); + it('executes tools when provider requests them', function () { $toolExecuted = false; diff --git a/tests/Unit/UsageTest.php b/tests/Unit/UsageTest.php new file mode 100644 index 0000000..a85dbe1 --- /dev/null +++ b/tests/Unit/UsageTest.php @@ -0,0 +1,99 @@ +inputTokens)->toBe(100); + expect($usage->outputTokens)->toBe(20); + expect($usage->totalTokens)->toBe(120); + }); + + it('respects an explicit total when given', function () { + $usage = new Usage(inputTokens: 100, outputTokens: 20, totalTokens: 130); + + expect($usage->totalTokens)->toBe(130); + }); + + it('normalises Anthropic-style keys', function () { + $usage = Usage::fromArray(['input_tokens' => 100, 'output_tokens' => 20]); + + expect($usage->inputTokens)->toBe(100); + expect($usage->outputTokens)->toBe(20); + }); + + it('normalises OpenAI-style keys', function () { + $usage = Usage::fromArray(['prompt_tokens' => 100, 'completion_tokens' => 20, 'total_tokens' => 120]); + + expect($usage->inputTokens)->toBe(100); + expect($usage->outputTokens)->toBe(20); + expect($usage->totalTokens)->toBe(120); + }); + + it('preserves the raw payload', function () { + $usage = Usage::fromArray(['prompt_tokens' => 5, 'extra' => 'kept']); + + expect($usage->toArray())->toBe(['prompt_tokens' => 5, 'extra' => 'kept']); + }); + + describe('legacy array-access (backward compatibility)', function () { + it('reads legacy keys through both naming styles', function () { + $usage = Usage::fromArray(['input_tokens' => 100, 'output_tokens' => 20]); + + expect($usage['input_tokens'])->toBe(100); + expect($usage['prompt_tokens'])->toBe(100); + expect($usage['output_tokens'])->toBe(20); + expect($usage['completion_tokens'])->toBe(20); + expect($usage['total_tokens'])->toBe(120); + }); + + it('reports offset existence for known and raw keys', function () { + $usage = Usage::fromArray(['input_tokens' => 1, 'custom' => 'x']); + + expect(isset($usage['input_tokens']))->toBeTrue(); + expect(isset($usage['custom']))->toBeTrue(); + expect(isset($usage['missing']))->toBeFalse(); + }); + + it('is immutable', function () { + $usage = new Usage(1, 2); + + expect(fn () => $usage['input_tokens'] = 5)->toThrow(LogicException::class); + }); + }); + + describe('Response integration', function () { + it('accepts a raw array and keeps the getters working (Anthropic keys)', function () { + $response = new Response(text: 'hi', usage: ['input_tokens' => 100, 'output_tokens' => 20]); + + expect($response->getInputTokens())->toBe(100); + expect($response->getOutputTokens())->toBe(20); + expect($response->getTotalTokens())->toBe(120); + // legacy array access still resolves + expect($response->usage['input_tokens'])->toBe(100); + }); + + it('fixes the token counts for OpenAI-style usage (previously returned 0)', function () { + $response = new Response( + text: 'hi', + usage: ['prompt_tokens' => 100, 'completion_tokens' => 20, 'total_tokens' => 120], + ); + + expect($response->getInputTokens())->toBe(100); + expect($response->getOutputTokens())->toBe(20); + expect($response->getTotalTokens())->toBe(120); + }); + + it('accepts a Usage object directly', function () { + $response = new Response(text: 'hi', usage: new Usage(7, 3)); + + expect($response->getInputTokens())->toBe(7); + expect($response->usage)->toBeInstanceOf(Usage::class); + }); + }); +});