-
-
Notifications
You must be signed in to change notification settings - Fork 108
[Platform] Fix structured output capability check for unsupported models #794
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?
[Platform] Fix structured output capability check for unsupported models #794
Conversation
Griffon-Weglot
commented
Oct 21, 2025
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| Docs? | no |
| Issues | Fix #791 |
| License | MIT |
src/agent/src/Agent.php
Outdated
| $messages = $input->getMessageBag(); | ||
| $options = $input->getOptions(); | ||
|
|
||
| if (isset($options['output_structure'])) { |
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.
IIRC we had some kind of check before, right @chr-hertel ?
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.
The way it was checked before is directly in the AgentProcessor
https://github.com/symfony/ai/pull/728/files#diff-212a36313084d7a7912d6ed09c262e978715148ce2d045aee724cfc3ec9b342cL77-L79
I think it would be better like it was before @Griffon-Weglot
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.
The only issue is that it requires to call $this->platform->getModelCatalog()->getModel($input->getModel()); in the OutputProcessor (and therefor inject the platform inside the OutputProcessor).
I don't know if it's was a good win to change $input->getModel from the object to the string.
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.
Ok i've made the changes, i agree with @VincentLanglet, injecting platform is maybe a problem
b3b7c7d to
0d976ce
Compare
| $modelObject = $this->platform->getModelCatalog()->getModel($input->getModel()); | ||
| if (!\in_array(Capability::OUTPUT_STRUCTURED, $modelObject->getCapabilities(), true)) { | ||
| throw MissingModelSupportException::forStructuredOutput($modelObject->getName()); |
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.
this would be enough for me
| $modelObject = $this->platform->getModelCatalog()->getModel($input->getModel()); | |
| if (!\in_array(Capability::OUTPUT_STRUCTURED, $modelObject->getCapabilities(), true)) { | |
| throw MissingModelSupportException::forStructuredOutput($modelObject->getName()); | |
| $model = $this->platform->getModelCatalog()->getModel($input->getModel()); | |
| if (!\in_array(Capability::OUTPUT_STRUCTURED, $model->getCapabilities(), true)) { | |
| throw MissingModelSupportException::forStructuredOutput($model->getName()); |
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.
I'm not familiar with the new pattern.
Is it ok to inject the platform every time we need the capabilities ; or would it be better to have the model instance directly in the input like it was before @chr-hertel ?
| if (!class_exists(__NAMESPACE__.'\ConfigurableResponseFormatFactory')) { | ||
| class ConfigurableResponseFormatFactory implements ResponseFormatFactoryInterface { | ||
| public function __construct(private array $format = []) {} | ||
| public function create(string $structure): array { return $this->format; } | ||
| } | ||
| } |
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.
do we need this? there is already Symfony\AI\Agent\Tests\StructuredOutput\ConfigurableResponseFormatFactory
|
Let's please put this to hold - most likely gonna be replaced with #802 Thanks anyways @Griffon-Weglot - also for second the necessity to bring back the validation! |
Am I wrong or @Griffon-Weglot could/should still open a PR after #802 in order to update/fix the supported capabilities of Gemini ? (And this can be done already since it shouldn't conflict with your work @chr-hertel) |
|
@VincentLanglet that is correct |
…Platform component (chr-hertel) This PR was merged into the main branch. Discussion ---------- [Agent][Platform] Shift structured output from Agent to Platform component | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Docs? | no | Issues | | License | MIT The main differentiator between the Agent and the Platform component is that the Agent is designed for multi-step model interaction and this is not the case for structured output. In the beginning the feature was also a bit more heavy than the light platform, since it was using the Serializer, and the Platform not having extension points for it. Nowadays the Platform uses the Serializer anyways for the contract handling, and relies on a optional EventDispatcher, that now also can provide the needed extension points. Secondary goals where to introduce a `ResultEvent` and bring back user-land validation for structured output capability of model. Fixes #791, replaces #794 <img width="1765" height="359" alt="image" src="https://github.com/user-attachments/assets/df9fc29f-fc90-4094-8c2f-10f01535dbfa" /> Commits ------- 8110d13 Shift structured output from Agent to Platform component
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.
Hey @Griffon-Weglot, i just merged #802 - can you please rebase and reduce the scope of this PR to fix the capabilities in the Gemini and VertexAI model catalog?