Add tool/function calling support for Anthropic adapter#146
Conversation
WalkthroughAdds ToolFormatter to emit Anthropic-compatible tool schemas, extends AnthropicChatAdapter to send/receive tool_use/tool_result blocks (including TOOL role mapping), widens streaming/message PHPDoc shapes, and adds unit tests plus PHPStan baseline updates. ChangesTool Integration
Sequence DiagramsequenceDiagram
participant Client
participant Adapter as AnthropicChatAdapter
participant Formatter as ToolFormatter
participant API as Anthropic API
Client->>Adapter: create(request with tools)
Adapter->>Formatter: formatTools(toolInfo[])
Formatter->>Formatter: formatTool -> formatParameter (recursive)
Formatter-->>Adapter: formatted tools array
Adapter->>API: POST /messages (with tools and messages)
API-->>Adapter: response with content_blocks (text, tool_use, etc.)
Adapter->>Adapter: aggregate text and build AIChatToolCall[] from tool_use
Adapter-->>Client: AIChatResponse (text + optional toolCalls)
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/anthropic-adapter/src/Chat/AnthropicChatAdapter.php (2)
41-46:⚠️ Potential issue | 🟡 MinorRemove
AIChatMessageRoleEnum::TOOLfromEXPECTED_ROLES.Anthropic's API only accepts
userandassistantroles in messages. The adapter currently allowsTOOLrole to pass validation (line 80), which would then be sent directly to the API at line 85 as'role' => 'tool', causing a rejection.Tool result messages should use
USERrole withToolCallPart, as the tests correctly demonstrate. TheTOOLenum value should not appear inEXPECTED_ROLESsince it cannot be used at the message level.
220-256:⚠️ Potential issue | 🟠 MajorStreaming path does not handle
tool_usecontent blocks from streamed responses.The
createStreamedMessagesmethod only extracts$delta->text(Line 244). Anthropic's streaming API supports sending tool_use content blocks viacontent_block_startevents (with id/name fields) andcontent_block_deltaevents (withinput_json_deltatype for partial input). Currently, these tool call events will be silently dropped during streaming while the non-streaming path correctly handles them (Lines 168-175).Consider either:
- Accumulating tool call data from streamed
content_block_start(typetool_use) andcontent_block_delta(typeinput_json_delta) events and yielding them after stream completion, or- Documenting that streamed tool use is not yet supported and throwing an exception if tools are present in a streamed request.
packages/anthropic/src/Resources/Messages.php (1)
234-234:⚠️ Potential issue | 🟠 MajorRemove invalid
'tool'role from message role validator — it will cause API errors.The validator at line 234 accepts
'tool'as a valid message role, but the Anthropic Messages API only supports'user'and'assistant'roles. Tool interactions are represented as content blocks (tool_useandtool_resulttypes within messages), not as separate message roles. Any message with role'tool'will be rejected by the API at runtime.Remove
'tool'from the allowed roles array:['user', 'assistant'](note:'system'is correctly extracted and moved to the top-levelsystemparameter byextractSystemPrompts()).
🤖 Fix all issues with AI agents
In `@packages/anthropic/src/Resources/MessagesInterface.php`:
- Line 25: Remove the unsupported "tool" role from the Message type alias and
runtime validation: update the phpstan-type Message definition (the Message type
alias) to use the union "system"|"assistant"|"user" (remove "tool") and change
the role validation in Messages.php (the validation that currently allows
'tool') to only allow ['system','user','assistant'] so messages conform to
Anthropic's API.
🧹 Nitpick comments (3)
packages/anthropic-adapter/src/Chat/ToolFormatter.php (2)
80-92: Use specific exception classes instead of generic\Exception.Lines 92 and 122 throw
\Exception. Per coding guidelines, specific exception classes should be used for error handling.♻️ Proposed fix
- throw new \Exception('Array type parameter must have items description. Define a type or use the Parameter class for object.'); + throw new \InvalidArgumentException('Array type parameter must have items description. Define a type or use the Parameter class for object.');- throw new \Exception('Object type parameter must have properties description. You need to pass an array of Parameter.'); + throw new \InvalidArgumentException('Object type parameter must have properties description. You need to pass an array of Parameter.');As per coding guidelines, "Use exception hierarchies with specific exception classes for error handling".
82-83: Consider removing defensiveproperty_existschecks.
Parameteris a typed class with known public properties (nullable,required). Using\property_exists()at Lines 82, 111, and 133 is a defensive pattern — likely carried over from the OpenAI adapter for backward compatibility with olderParameterversions. If theParameterclass is stable (which it appears to be given the0.4.0version), direct property access would be cleaner.♻️ Example simplification
- $nullable = \property_exists($parameter, 'nullable') ? $parameter->nullable : false; + $nullable = $parameter->nullable;packages/anthropic-adapter/tests/Unit/Chat/AnthropicChatAdapterTest.php (1)
89-90: Consider adding a streaming + tools test.The
supports()method now returnstruefor allAIChatRequestinstances, including streamed requests with tools. However, as noted in the adapter review, the streaming path doesn't handletool_usecontent blocks. A test for this scenario would help document the current behavior (or catch future regressions once streaming tool support is added).
wachterjohannes
force-pushed
the
feature/anthropic-tool-support
branch
from
9114527 to
4ebe02b
Compare
- Create ToolFormatter for Anthropic-specific tool format (uses input_schema instead of parameters, no type/function wrapper like OpenAI) - Update AnthropicChatAdapter to handle tool requests and responses: - Add tools to request parameters when hasTools() is true - Handle ToolCallsPart (converts to tool_use content blocks) - Handle ToolCallPart (converts to tool_result content blocks) - Parse tool_use from response to create AIChatToolCall objects - Update Messages.php to allow streaming with tools - Update MessagesInterface types to include tool role and improved input_schema - Add comprehensive tests for tool support
wachterjohannes
force-pushed
the
feature/anthropic-tool-support
branch
from
4ebe02b to
b8fd359
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/anthropic-adapter/tests/Unit/Chat/AnthropicChatAdapterTest.php (1)
618-623: ⚡ Quick winDrop redundant
@paramannotations fromgetWeatherMethoddocblock.The method already has explicit scalar types and return type, so the parameter tags are unnecessary noise.
As per coding guidelines, "Type hints are documentation - Modern PHP 8.2+ with strict types makes
@param/@return tags redundant".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/anthropic-adapter/tests/Unit/Chat/AnthropicChatAdapterTest.php` around lines 618 - 623, Remove the redundant `@param` annotations from the getWeatherMethod docblock in AnthropicChatAdapterTest.php: since the method signature already uses explicit scalar types and a return type, delete the `@param` (and any redundant `@return`) tags so the docblock only contains meaningful description text; locate the docblock immediately above the getWeatherMethod function and remove those parameter annotations.packages/anthropic-adapter/tests/Unit/Chat/ToolFormatterTest.php (1)
102-107: ⚡ Quick winRemove redundant
@paramtags in helper method docblock.With strict types and full parameter/return type hints already present, this docblock can be simplified to description-only (or removed if unnecessary).
As per coding guidelines, "Type hints are documentation - Modern PHP 8.2+ with strict types makes
@param/@return tags redundant".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/anthropic-adapter/tests/Unit/Chat/ToolFormatterTest.php` around lines 102 - 107, Remove the redundant `@param` tags from the helper method docblock in the ToolFormatterTest class: delete the two "`@param` string $required ..." and "`@param` string $optional ..." lines so the docblock is description-only (or remove the entire docblock if it's unnecessary), keeping the method's strict types and return type intact; locate the block above the helper method in packages/anthropic-adapter/tests/Unit/Chat/ToolFormatterTest.php and update the comment accordingly.packages/anthropic-adapter/phpstan-baseline.neon (1)
36-43: ⚡ Quick winAvoid expanding baseline for tautological test assertions.
These new ignores are for always-true assertions; prefer fixing the corresponding tests (remove or replace tautological checks) instead of suppressing, to keep PHPStan signal clean.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/anthropic-adapter/phpstan-baseline.neon` around lines 36 - 43, The baseline entry suppresses a tautological assertion in tests/Unit/Chat/AnthropicChatAdapterTest.php where PHPUnit\Framework\Assert::assertSame() is being called with ModelflowAi\Chat\ToolInfo\ToolTypeEnum::FUNCTION for both expected and actual; locate the failing assertion in AnthropicChatAdapterTest (search for assertSame and ToolTypeEnum::FUNCTION) and fix the test by removing or replacing the tautology — either assert the actual value produced by the code (use assertSame(expectedValue, $actualVariable) where $actualVariable comes from the adapter under test) or replace with a meaningful check (e.g., assertEquals/assertInstanceOf/assertContains on the adapter output) so the test asserts real behavior instead of comparing the same constant to itself.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/anthropic-adapter/phpstan-baseline.neon`:
- Around line 36-43: The baseline entry suppresses a tautological assertion in
tests/Unit/Chat/AnthropicChatAdapterTest.php where
PHPUnit\Framework\Assert::assertSame() is being called with
ModelflowAi\Chat\ToolInfo\ToolTypeEnum::FUNCTION for both expected and actual;
locate the failing assertion in AnthropicChatAdapterTest (search for assertSame
and ToolTypeEnum::FUNCTION) and fix the test by removing or replacing the
tautology — either assert the actual value produced by the code (use
assertSame(expectedValue, $actualVariable) where $actualVariable comes from the
adapter under test) or replace with a meaningful check (e.g.,
assertEquals/assertInstanceOf/assertContains on the adapter output) so the test
asserts real behavior instead of comparing the same constant to itself.
In `@packages/anthropic-adapter/tests/Unit/Chat/AnthropicChatAdapterTest.php`:
- Around line 618-623: Remove the redundant `@param` annotations from the
getWeatherMethod docblock in AnthropicChatAdapterTest.php: since the method
signature already uses explicit scalar types and a return type, delete the
`@param` (and any redundant `@return`) tags so the docblock only contains meaningful
description text; locate the docblock immediately above the getWeatherMethod
function and remove those parameter annotations.
In `@packages/anthropic-adapter/tests/Unit/Chat/ToolFormatterTest.php`:
- Around line 102-107: Remove the redundant `@param` tags from the helper method
docblock in the ToolFormatterTest class: delete the two "`@param` string $required
..." and "`@param` string $optional ..." lines so the docblock is description-only
(or remove the entire docblock if it's unnecessary), keeping the method's strict
types and return type intact; locate the block above the helper method in
packages/anthropic-adapter/tests/Unit/Chat/ToolFormatterTest.php and update the
comment accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7f88e229-73d2-4615-a573-2d5ec97ba9c3
⛔ Files ignored due to path filters (1)
packages/anthropic/composer.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
packages/anthropic-adapter/phpstan-baseline.neonpackages/anthropic-adapter/src/Chat/AnthropicChatAdapter.phppackages/anthropic-adapter/src/Chat/ToolFormatter.phppackages/anthropic-adapter/tests/Unit/Chat/AnthropicChatAdapterTest.phppackages/anthropic-adapter/tests/Unit/Chat/ToolFormatterTest.phppackages/anthropic/phpstan-baseline.neonpackages/anthropic/src/Resources/Messages.phppackages/anthropic/src/Resources/MessagesInterface.php
✅ Files skipped from review due to trivial changes (1)
- packages/anthropic/src/Resources/MessagesInterface.php
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/anthropic/phpstan-baseline.neon
- packages/anthropic-adapter/src/Chat/ToolFormatter.php
- packages/anthropic/src/Resources/Messages.php
- packages/anthropic-adapter/src/Chat/AnthropicChatAdapter.php
1 participant
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests