-
Notifications
You must be signed in to change notification settings - Fork 24
Classify gpt-6-astra as reasoning + text/image-capable in OpenAI model metadata
#54
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: trunk
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -33,17 +33,19 @@ class OpenAiModelMetadataDirectory extends AbstractOpenAiCompatibleModelMetadata | |||||||||||||
| /** | ||||||||||||||
| * Regular expression matching the model ID prefixes of OpenAI reasoning models. | ||||||||||||||
| * | ||||||||||||||
| * Reasoning models (codex-mini-latest, the versioned GPT-5 family, and the verified o1, o3, | ||||||||||||||
| * and o4 families) must be classified separately from standard GPT models. Only o-families | ||||||||||||||
| * whose behavior has been verified are recognized; new o-families require documentation and | ||||||||||||||
| * tests before they are added. GPT-5 chat aliases share a reasoning-family prefix but are | ||||||||||||||
| * non-reasoning models and are handled separately by self::isNonReasoningChatModel(). | ||||||||||||||
| * Reasoning models (codex-mini-latest, the versioned GPT-5 family, GPT-6 Astra, and the | ||||||||||||||
| * verified o1, o3, and o4 families) must be classified separately from standard GPT models. | ||||||||||||||
| * Only o-families whose behavior has been verified are recognized; new o-families require | ||||||||||||||
| * documentation and tests before they are added. GPT-5 chat aliases share a | ||||||||||||||
| * reasoning-family prefix but are non-reasoning models and are handled separately by | ||||||||||||||
| * self::isNonReasoningChatModel(). | ||||||||||||||
| * | ||||||||||||||
| * @since 1.1.0 | ||||||||||||||
| * | ||||||||||||||
| * @var string | ||||||||||||||
| */ | ||||||||||||||
| private const REASONING_MODEL_ID_PATTERN = '/^(?:codex-mini-latest|gpt-5(?:\.\d+)?|o(?:1|3|4))(?:-|$)/'; | ||||||||||||||
| private const REASONING_MODEL_ID_PATTERN = | ||||||||||||||
| '/^(?:codex-mini-latest|gpt-5(?:\.\d+)?|gpt-6-astra(?:-\d{4}-\d{2}-\d{2})?|o(?:1|3|4))(?:-|$)/'; | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Regular expression matching the IDs of reasoning models that use reasoning effort `none` by default. | ||||||||||||||
|
|
@@ -157,6 +159,26 @@ protected function parseResponseToModelMetadataList(Response $response): array | |||||||||||||
| ), | ||||||||||||||
| new SupportedOption(OptionEnum::outputModalities(), [[ModalityEnum::text()]]), | ||||||||||||||
| ]); | ||||||||||||||
| $gptTextAndImageInputOptions = array_merge($gptBaseOptions, $gptSamplingOptions, [ | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please read this together with my comment on line 424 before deleting anything. As the PR stands, this array is never selected. For every ID where
so It's unreachable because the PR only handles Astra, which is the one GPT-6 model that can't use So there are two consistent options:
The current state, where the array looks intentional but nothing can reach it, is the one to avoid. Separately: this method now has three pairs of option arrays with the same structure ( array_merge(
$gptBaseOptions,
self::supportsSamplingOptions($modelId) ? $gptSamplingOptions : [],
[
new SupportedOption(OptionEnum::inputModalities(), self::inputModalitiesFor($modelId)),
new SupportedOption(OptionEnum::outputModalities(), [[ModalityEnum::text()]]),
]
); |
||||||||||||||
| new SupportedOption( | ||||||||||||||
| OptionEnum::inputModalities(), | ||||||||||||||
| [ | ||||||||||||||
| [ModalityEnum::text()], | ||||||||||||||
| [ModalityEnum::text(), ModalityEnum::image()], | ||||||||||||||
| ] | ||||||||||||||
| ), | ||||||||||||||
| new SupportedOption(OptionEnum::outputModalities(), [[ModalityEnum::text()]]), | ||||||||||||||
| ]); | ||||||||||||||
| $gptReasoningTextAndImageInputOptions = array_merge($gptBaseOptions, [ | ||||||||||||||
| new SupportedOption( | ||||||||||||||
| OptionEnum::inputModalities(), | ||||||||||||||
| [ | ||||||||||||||
| [ModalityEnum::text()], | ||||||||||||||
| [ModalityEnum::text(), ModalityEnum::image()], | ||||||||||||||
| ] | ||||||||||||||
| ), | ||||||||||||||
| new SupportedOption(OptionEnum::outputModalities(), [[ModalityEnum::text()]]), | ||||||||||||||
| ]); | ||||||||||||||
| $gptMultimodalSpeechOutputOptions = array_merge($gptBaseOptions, $gptSamplingOptions, [ | ||||||||||||||
| new SupportedOption( | ||||||||||||||
| OptionEnum::inputModalities(), | ||||||||||||||
|
|
@@ -271,6 +293,8 @@ static function (array $modelData) use ( | |||||||||||||
| $gptReasoningOptions, | ||||||||||||||
| $gptMultimodalInputOptions, | ||||||||||||||
| $gptReasoningMultimodalInputOptions, | ||||||||||||||
| $gptTextAndImageInputOptions, | ||||||||||||||
| $gptReasoningTextAndImageInputOptions, | ||||||||||||||
| $gptMultimodalSpeechOutputOptions, | ||||||||||||||
| $gptSearchOptions, | ||||||||||||||
| $imageCapabilities, | ||||||||||||||
|
|
@@ -325,7 +349,12 @@ static function (array $modelData) use ( | |||||||||||||
| && !str_contains($modelId, '-realtime') | ||||||||||||||
| && !str_contains($modelId, '-transcribe') | ||||||||||||||
| ) { | ||||||||||||||
| if (self::supportsMultimodalTextInput($modelId)) { | ||||||||||||||
| if (self::supportsTextAndImageInputOnly($modelId)) { | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This only matters once line 424 is widened, so please treat it as part of that change. This branch runs before the If the pattern is widened to the prefix form suggested on line 424, a GPT-6 ID with a Since the suffix checks apply across families, they could move ahead of the family dispatch instead of being nested inside one branch: if (str_contains($modelId, '-audio')) {
// ... existing audio handling
} elseif (str_contains($modelId, '-search')) {
// ... existing search handling
} elseif (self::supportsTextAndImageInputOnly($modelId)) {
// ... GPT-6
} elseif (self::supportsMultimodalTextInput($modelId)) {
// ... existing multimodal handling
}A regression test with a synthetic |
||||||||||||||
| $modelCaps = $gptCapabilities; | ||||||||||||||
| $modelOptions = self::supportsSamplingOptions($modelId) | ||||||||||||||
| ? $gptTextAndImageInputOptions | ||||||||||||||
| : $gptReasoningTextAndImageInputOptions; | ||||||||||||||
| } elseif (self::supportsMultimodalTextInput($modelId)) { | ||||||||||||||
| $modelCaps = $gptCapabilities; | ||||||||||||||
| $modelOptions = $gptMultimodalInputOptions; | ||||||||||||||
| // New multimodal output model for audio generation. | ||||||||||||||
|
|
@@ -382,12 +411,25 @@ private static function supportsMultimodalTextInput(string $modelId): bool | |||||||||||||
| ); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Checks whether an OpenAI text generation model supports text and image input only. | ||||||||||||||
| * | ||||||||||||||
| * @since n.e.x.t | ||||||||||||||
| * | ||||||||||||||
| * @param string $modelId The model ID. | ||||||||||||||
| * @return bool True if the model supports text and image input only, false otherwise. | ||||||||||||||
| */ | ||||||||||||||
| private static function supportsTextAndImageInputOnly(string $modelId): bool | ||||||||||||||
| { | ||||||||||||||
| return (bool) preg_match('/^gpt-6-astra(?:-\d{4}-\d{2}-\d{2})?$/', $modelId); | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking. This matches a single model ID, and the rest of the generation is left out.
Luna and Sol end up where Astra was before this PR. With text-only input, a client that requires According to OpenAI's latest-model guide, both of those are wrong:
All three are reasoning models with text + image input. The PR handles Astra correctly, including leaving out sampling options since Astra can't use The PR description calls the root cause "model-ID patterning that did not include GPT-6 Astra in multimodal/reasoning capability paths." I'd describe it as the patterning not including GPT-6 at all. Astra is just the one that was reported. Suggestion: a generation-level constant shared by /**
* Regular expression matching the model IDs of the GPT-6 generation.
*
* @since n.e.x.t
*
* @var string
*/
private const GPT6_MODEL_ID_PATTERN = '/^gpt-6-(?:astra|luna|sol)(?:-|$)/';With this applied locally, all three resolve to Open question. The docs say Sol and Luna support Also on this line. This pattern is exact-anchored ( The existing test data suggests the prefix form: Related: If you relax this anchor, please also look at my comment on line 352. That change needs to go in at the same time. |
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Checks whether an OpenAI text generation model is a reasoning model. | ||||||||||||||
| * | ||||||||||||||
| * Reasoning model families include codex-mini-latest, versioned GPT-5 models (e.g. `gpt-5`, | ||||||||||||||
| * `gpt-5.5`), and the verified o1, o3, and o4 families. GPT-5 chat aliases are non-reasoning | ||||||||||||||
| * models and are handled separately; see {@see self::isNonReasoningChatModel()} and | ||||||||||||||
| * `gpt-5.5`), GPT-6 Astra, and the verified o1, o3, and o4 families. GPT-5 chat aliases are | ||||||||||||||
| * non-reasoning models and are handled separately; see {@see self::isNonReasoningChatModel()} and | ||||||||||||||
| * {@see self::supportsSamplingOptions()}. | ||||||||||||||
| * | ||||||||||||||
| * @since 1.1.0 | ||||||||||||||
|
|
||||||||||||||
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
(?:-\d{4}-\d{2}-\d{2})?group added aftergpt-6-astrahas no effect here. The alternation is followed by(?:-|$), sogpt-6-astraon its own already matches anything that starts withgpt-6-astra-, includinggpt-6-astra-2026-09-01. I checked, and classification is the same with or without the group.It also makes the pattern look like only bare and dated Astra IDs count as reasoning models. That's the assumption behind the mismatch I mentioned on line 424, so the group is misleading as well as redundant.
For comparison, the same group is needed in
EFFORT_NONE_DEFAULT_MODEL_ID_PATTERN(line 61) because that pattern ends in$. Andgpt-5(?:\.\d+)?is needed here because.isn't-, so(?:-|$)wouldn't matchgpt-5.2.I'd drop the group, which also lets the constant fit on one line again:
(If you go with the generation-level constant from line 424, this line would reference that instead.)