-
-
Notifications
You must be signed in to change notification settings - Fork 50
[Platform] [Anthropic] Allow beta feature flags to be passed into platform invocations #274
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
base: main
Are you sure you want to change the base?
Conversation
I am fine using the Can you please add a test case? Thanks |
Sure thing. |
@OskarStark Done. |
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.
Good catch, those specific things slip easily through, thanks!
minor comments from my end
@@ -65,7 +65,7 @@ start for vendor-specific models and their capabilities, see ``Symfony\AI\Platfo | |||
supports a specific feature, like ``Capability::INPUT_AUDIO`` or ``Capability::OUTPUT_IMAGE``. | |||
|
|||
**Options** are additional parameters that can be passed to the model, like ``temperature`` or ``max_tokens``, and are | |||
usually defined by the specific models and their documentation. | |||
usually defined by the specific models and their documentation. Flags for beta features are also passed in here, handled, and then removed before being sent to the provider (beta feature flags are currently only supported for the `Anthropic` platform). A more robust approach for handling provider feature flags may be implemented in the future. |
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.
Please remove this again, I don't think we should give those model specific examples in this rather generic section.
if (isset($options['tools'])) { | ||
$options['tool_choice'] = ['type' => 'auto']; | ||
} | ||
|
||
if (isset($options['beta_features']) && !empty($options['beta_features'])) { |
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.
what do you think about also checking for array here?
$this->model = new Claude(); | ||
} | ||
|
||
private function parseHeaders(array $headers): array |
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.
please move private
methods below public
methods
i wonder why php-cs-fixer doesn't complain 🤔
|
||
$this->assertArrayNotHasKey('anthropic-beta', $headers); | ||
|
||
return new MockResponse('{"success": true}'); |
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.
Please use JsonMockResponse
Will address these issues on Monday. |
Anthropic supports passing flags for beta features via beta headers in requests to its API. This PR allows for passing in the desired beta features via the
$options
parameter in theinvoke
function like so:If the beta features option is set and contains at least one element, the beta header is constructed inside the model client and then the option is removed from
$options
before the request is built and sent to Anthropic.If we want to support beta feature flags for other providers in the future, we should perhaps add another parameter to the
invoke
function for platforms and therequest
function for model clients in order to not pollute$options
(after all,$options
is meant to hold model options not platform options).