fix(llms): narrow _is_reasoning_model to not match gpt-5.x variants#4746
Merged
kartik-mem0 merged 3 commits intomem0ai:mainfrom Apr 23, 2026
Merged
Conversation
The substring match 'gpt-5 in model_lower' incorrectly classified gpt-5.4-mini and other gpt-5.x variants as reasoning models, causing temperature and other standard parameters to be silently stripped. Replace the broad substring match with: - Exact match against known reasoning models (o1, o3, gpt-5, gpt-5o) - Prefix match for dated variants (o1-2024-12-17, o3-2025-04-16) - Provider prefix stripping (openai/o3-mini -> o3-mini) gpt-5.x models (gpt-5.4-mini, gpt-5.4) support temperature and are no longer misclassified. Fixes mem0ai#4738 Co-Authored-By: Claude <noreply@anthropic.com>
kartik-mem0
previously approved these changes
Apr 13, 2026
Contributor
|
please address the ci failing here |
test_is_reasoning_model_classification created OpenAILLM without mocking the OpenAI client, causing CI to fail with: openai.OpenAIError: The api_key client option must be set Added the mock_openai_client fixture (same pattern as all other tests in this file) so the real OpenAI client is never initialized.
Merged upstream main which added test_store_not_sent_by_default, test_store_sent_when_explicitly_true, and test_store_sent_when_explicitly_false (from mem0ai#4709 fix). Kept both upstream store tests and our reasoning model tests.
Contributor
Author
|
Thanks @kartik-mem0, resolved the merge conflict kept both the upstream store tests from #4709 and the reasoning model classification tests from this PR. Also fixed the CI failure (missing mock_openai_client fixture on the classification test). should be green now |
kartik-mem0
approved these changes
Apr 23, 2026
This was referenced Apr 23, 2026
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked Issue
Closes #4738
Description
_is_reasoning_model()used a substring match ("gpt-5" in model_lower) that incorrectly classifiedgpt-5.4-miniand other gpt-5.x variants as reasoning models. This causedtemperatureand other standard parameters to be silently stripped from API calls.Root Cause
gpt-5.4-minicontains"gpt-5"as a substring, so it was misclassified. The_get_supported_params()method then stripped all parameters exceptmessages,response_format,tools, andtool_choice— includingtemperature.Fix
Replaced the broad substring match with:
o1,o3,gpt-5,gpt-5o, etc.)o1-2024-12-17,o3-2025-04-16)openai/o3-mini→o3-mini)"gpt-5"substring match entirelyType of Change
Test Coverage
Added 2 tests:
test_gpt5_mini_not_classified_as_reasoning— verifies gpt-5.4-mini passes temperature throughtest_is_reasoning_model_classification— comprehensive classification test covering reasoning models (o1, o3, gpt-5, prefixed variants) and non-reasoning models (gpt-5.4-mini, gpt-5.4, gpt-4.1)Checklist