Avoid listing models for explicit text model IDs#231
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @felixarntz. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #231 +/- ##
============================================
- Coverage 88.12% 87.72% -0.41%
- Complexity 1213 1217 +4
============================================
Files 60 60
Lines 3934 4121 +187
============================================
+ Hits 3467 3615 +148
- Misses 467 506 +39
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
chubes4
force-pushed
the
explicit-model-no-list
branch
from
4b2547d to
3e79f62
Compare
| /** | ||
| * Checks whether a model ID is safe to treat as a text generation model without listing models. | ||
| * | ||
| * @since n.e.x.t | ||
| * | ||
| * @param string $modelId The explicit model ID. | ||
| * @return bool True if the model ID matches common OpenAI-compatible text generation model families. | ||
| */ | ||
| protected function isExplicitTextGenerationModelId(string $modelId): bool | ||
| { | ||
| if (str_starts_with($modelId, 'gpt-image-') || str_starts_with($modelId, 'dall-e-')) { | ||
| return false; | ||
| } | ||
|
|
||
| if (str_starts_with($modelId, 'gpt-') || str_starts_with($modelId, 'chatgpt-')) { | ||
| return true; | ||
| } | ||
|
|
||
| return preg_match('/^o\d(?:-|$)/', $modelId) === 1; | ||
| } |
There was a problem hiding this comment.
I don't think this should live in this codebase, rather the OpenAI Provider repository. While we do have "OpenAI Compatible" classes, that's about the shape of the API, not coupling to any specific models.
Curious if you agree, @felixarntz.
There was a problem hiding this comment.
Yes, model IDs should only be in the provider specific repos.
There was a problem hiding this comment.
I've made the requested changes and opened a follow-up draft in the OpenAI provider: WordPress/ai-provider-for-openai#23
This PR now just provides the hook and abstract test coverage. Thank you!
The OpenAI-compatible abstraction is about HTTP/JSON shape, not OpenAI's model namespace. Drop the gpt-/o3/dall-e prefix gating and the synthetic text-generation metadata override; the per-provider override belongs in ai-provider-for-openai (and similar repos for other compatible providers). The generic createModelMetadataForExplicitModelId() hook on AbstractApiBasedModelMetadataDirectory is unchanged so providers can still opt in.
3 participants
Summary
gpt-image-*still fall back to listed metadata.Fixes #230.
Why
Explicit provider/model selection currently calls the provider model-list endpoint before the model can be instantiated. For OpenAI-compatible providers, that means
getProviderModel( 'openai', 'gpt-5.4' )can fail on a transientGET /modelsissue before the actual generation endpoint has a chance to validate the explicitly requested model.Testing
composer test -- --filter 'GetProviderModelWithExplicitOpenAiModelIdDoesNotListModels|GetModelMetadataForExplicitModelIdDoesNotListModels|GetModelMetadataForNonTextExplicitModelIdListsModels|GetModelMetadataUsesCachedListedMetadataWhenAvailable'composer testcurrently runs all 1091 tests locally, but exits non-zero on PHP 8.5 because existing reflection tests emit deprecation output and PHPUnit marks 83 tests risky. No assertion failures were reported.AI assistance