fix(openai): support gpt-5-nano parameters#22
Conversation
WalkthroughAdds support for OpenAI model Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
723f805 to
1c51274
Compare
1c51274 to
5dfa09a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/Agents/Adapters/OpenAI.php`:
- Around line 186-188: PHPStan flags a possible null dereference when calling
$this->getAgent()->getSchema(); to fix, capture the agent into a local variable
(e.g., $agent = $this->getAgent()) and perform an explicit null-check or
assertion before calling getSchema on it; replace usages of
$this->getAgent()->getSchema() with $agent->getSchema() inside the guarded block
(or add assert($this->getAgent() !== null) if you prefer) so PHPStan can see the
non-null guarantee for getSchema().
🧹 Nitpick comments (3)
src/Agents/Adapters/OpenAI.php (2)
178-185: Consider omittingtemperaturefrom the payload instead of forcing 1.0.The OpenAI API returns an error stating
'temperature' does not support 0.2 with this model. Only the default (1) value is supported.for GPT-5 models via chat completions, so sending1.0may work today. However, some GPT-5 models report'temperature' is not supported with this model, which rejects the parameter entirely. Omittingtemperaturefrom the payload for these models is safer and future-proof.♻️ Suggested change
$temperature = $this->temperature; if ($this->usesDefaultTemperatureOnly()) { if ($this->temperature !== 1.0) { $this->warnTemperatureOverride(); } - $temperature = 1.0; + // Omit temperature from payload entirely for models that only accept default + // Do not set $temperature — we'll skip adding it to the payload + } else { + $payload['temperature'] = $temperature; } - $payload['temperature'] = $temperature;
343-351:o3,o3-mini, ando4-minialso reject custom temperature values.Modern reasoning models like GPT-5, o3, and o4-mini disable sampling parameters such as temperature and top_p because their internal generation process involves multiple rounds of reasoning. Now that this helper method exists, consider expanding it to cover all affected models.
♻️ Suggested change
protected function usesDefaultTemperatureOnly(): bool { return in_array($this->model, [ self::MODEL_GPT_5_NANO, + self::MODEL_O4_MINI, + self::MODEL_O3, + self::MODEL_O3_MINI, ], true); }README.md (1)
249-249: Nit: Unrelated formatting change — verify this was intentional.The dash was removed from the MIT license line, converting it from a list item to plain text. If intentional, it's fine; if not, it may have been an accidental edit.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/Agents/Adapters/OpenAI.php`:
- Around line 344-360: The usesDefaultTemperatureOnly() method currently only
checks self::MODEL_GPT_5_NANO; update its model list to include the modern
models that disallow sampling (e.g., o3, o3-mini, o4-mini) by adding their
corresponding class constants (e.g., self::MODEL_O3, self::MODEL_O3_MINI,
self::MODEL_O4_MINI) to the in_array check inside usesDefaultTemperatureOnly(),
leaving the existing temperature-override warning logic intact so it still logs
and sets hasWarnedTemperatureOverride when a non-1.0 temperature is passed for
those models.
- Around line 179-183: The payload always includes 'temperature' which causes
errors for models that disallow it (e.g., gpt-5-nano); update the code in the
OpenAI adapter (within the method that builds $payload where $temperature is set
and usesDefaultTemperatureOnly() is used) to only add $payload['temperature']
when the current model supports that parameter—check the model identifier
available on $this (e.g., $this->model or getter) against a
whitelist/feature-check (or implement a helper like modelSupportsTemperature())
and skip adding the 'temperature' key for restricted models such as
'gpt-5-nano', preserving the existing usesDefaultTemperatureOnly() logic for
supported models.
e0d5133 to
36ac59e
Compare
36ac59e to
ecc9603
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/Agents/Adapters/XAI.php`:
- Line 27: The constant XAI::MODEL_GROK_2_IMAGE was changed to a date-pinned
string ('grok-2-image-1212') which can silently alter behavior; confirm whether
this pin is intentional and either revert to the generic identifier or
explicitly document the pin and preserve backward-compatibility by adding a
clear alias (keep XAI::MODEL_GROK_2_IMAGE as the canonical constant and, if
needed, add a new XAI::MODEL_GROK_2_IMAGE_PINNED constant or a comment next to
MODEL_GROK_2_IMAGE explaining the pin and its rationale) and update any places
that depend on MODEL_GROK_2_IMAGE to use the intended constant.
- Around line 15-22: Restore backward compatibility by reintroducing the removed
constant MODEL_GROK_2_LATEST as a deprecated alias that points to the new model
constant (choose either MODEL_GROK_3_MINI or MODEL_GROK_3 based on
compatibility) and add a deprecation doc comment; also revert or provide a
deprecated alias for the changed MODEL_GROK_2_IMAGE value so the original string
'grok-2-image' remains available (e.g., keep existing MODEL_GROK_2_IMAGE with
its new value and add MODEL_GROK_2_IMAGE_DEPRECATED or reassign the original
name to the old string while marking it deprecated), updating PHPDoc comments
for XAI::MODEL_GROK_2_LATEST and XAI::MODEL_GROK_2_IMAGE to indicate deprecation
and mapping to the new constants.
| * Grok 3 - Latest Grok model | ||
| */ | ||
| public const MODEL_GROK_2_LATEST = 'grok-2-latest'; | ||
| public const MODEL_GROK_3 = 'grok-3'; | ||
|
|
||
| /** | ||
| * Grok 3 Mini - Mini version of grok 3 | ||
| */ | ||
| public const MODEL_GROK_3_MINI = 'grok-3-mini'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'MODEL_GROK_2_LATEST' --type=phpRepository: utopia-php/agents
Length of output: 43
🏁 Script executed:
cat -n src/Agents/Adapters/XAI.php | head -100Repository: utopia-php/agents
Length of output: 2780
🏁 Script executed:
git log --oneline -n 20 src/Agents/Adapters/XAI.phpRepository: utopia-php/agents
Length of output: 91
🏁 Script executed:
git show 430ad21:src/Agents/Adapters/XAI.php | head -100Repository: utopia-php/agents
Length of output: 2080
🏁 Script executed:
git log --oneline --follow -- src/Agents/Adapters/XAI.phpRepository: utopia-php/agents
Length of output: 91
🏁 Script executed:
git log --all --diff-filter=A -- src/Agents/Adapters/XAI.phpRepository: utopia-php/agents
Length of output: 441
🏁 Script executed:
git show c22aa7a --name-status | grep -i xaiRepository: utopia-php/agents
Length of output: 142
🏁 Script executed:
git show c22aa7a:src/Agents/Adapters/XAI.php | head -100Repository: utopia-php/agents
Length of output: 2070
🏁 Script executed:
git show c22aa7a -- src/Agents/Adapters/XAI.php | grep -A 5 -B 5 "MODEL_GROK_2_LATEST"Repository: utopia-php/agents
Length of output: 901
🏁 Script executed:
git log --oneline --all -- src/Agents/Adapters/XAI.phpRepository: utopia-php/agents
Length of output: 178
Removal of MODEL_GROK_2_LATEST is a breaking change for downstream consumers.
The MODEL_GROK_2_LATEST constant was replaced as part of the Grok 2 to Grok 3 migration. Any external code referencing XAI::MODEL_GROK_2_LATEST will fail. If this library has external consumers, preserve backwards compatibility by keeping the old constant as a deprecated alias pointing to MODEL_GROK_3_MINI or MODEL_GROK_3.
Additionally, MODEL_GROK_2_IMAGE value changed from 'grok-2-image' to 'grok-2-image-1212', which is also a breaking change for external consumers relying on the old value.
🤖 Prompt for AI Agents
In `@src/Agents/Adapters/XAI.php` around lines 15 - 22, Restore backward
compatibility by reintroducing the removed constant MODEL_GROK_2_LATEST as a
deprecated alias that points to the new model constant (choose either
MODEL_GROK_3_MINI or MODEL_GROK_3 based on compatibility) and add a deprecation
doc comment; also revert or provide a deprecated alias for the changed
MODEL_GROK_2_IMAGE value so the original string 'grok-2-image' remains available
(e.g., keep existing MODEL_GROK_2_IMAGE with its new value and add
MODEL_GROK_2_IMAGE_DEPRECATED or reassign the original name to the old string
while marking it deprecated), updating PHPDoc comments for
XAI::MODEL_GROK_2_LATEST and XAI::MODEL_GROK_2_IMAGE to indicate deprecation and
mapping to the new constants.
| * Grok 2 Image - Latest Grok model with image support | ||
| */ | ||
| public const MODEL_GROK_2_IMAGE = 'grok-2-image'; | ||
| public const MODEL_GROK_2_IMAGE = 'grok-2-image-1212'; |
There was a problem hiding this comment.
MODEL_GROK_2_IMAGE value changed — verify this is intentional.
The model string changed to 'grok-2-image-1212' (a date-pinned version). This silently changes behavior for anyone using XAI::MODEL_GROK_2_IMAGE without updating their code, which may be desirable for reproducibility but should be noted.
🤖 Prompt for AI Agents
In `@src/Agents/Adapters/XAI.php` at line 27, The constant XAI::MODEL_GROK_2_IMAGE
was changed to a date-pinned string ('grok-2-image-1212') which can silently
alter behavior; confirm whether this pin is intentional and either revert to the
generic identifier or explicitly document the pin and preserve
backward-compatibility by adding a clear alias (keep XAI::MODEL_GROK_2_IMAGE as
the canonical constant and, if needed, add a new XAI::MODEL_GROK_2_IMAGE_PINNED
constant or a comment next to MODEL_GROK_2_IMAGE explaining the pin and its
rationale) and update any places that depend on MODEL_GROK_2_IMAGE to use the
intended constant.
Summary by CodeRabbit
New Features
Improvements
Chores
Documentation