OE-02/03/04: Add is_reasoning_model config flag for non-OpenAI thinking models

[MateoTTR]

Summary

Closes #2

• OE-02: Extract OPENAI_REASONING_MODEL_PREFIXES to a module-level constant and create a standalone is_reasoning_model() function in openevolve/llm/openai.py
• OE-03: Add is_reasoning_model: Optional[bool] = None field to LLMModelConfig in openevolve/config.py
• OE-04: Add comprehensive unit tests in tests/test_reasoning_model_detection.py (12 test cases)

Design decision

The is_reasoning_model config field uses 3-state logic:

Value	Behavior
None (default)	Auto-detect via OpenAI prefix matching (backward compatible)
True	Force reasoning-model parameter conventions (max_completion_tokens, no temperature/top_p)
False	Force standard parameter conventions (even for OpenAI reasoning models)

This is fully backward compatible: existing configs without the field behave exactly as before.

Config example

llm:
  models:
    # Auto-detected as reasoning model (no config needed)
    - name: o3-mini
      weight: 0.8

    # Gemini thinking model — must be explicitly marked
    - name: gemini-2.5-flash
      api_base: https://generativelanguage.googleapis.com/v1beta/openai/
      api_key: ${GEMINI_API_KEY}
      is_reasoning_model: true
      weight: 0.2

    # DeepSeek R1 — must be explicitly marked
    - name: deepseek-r1
      api_base: https://api.deepseek.com/v1
      api_key: ${DEEPSEEK_API_KEY}
      is_reasoning_model: true
      weight: 0.1

Test plan

• 12 new unit tests covering auto-detect, explicit override, case insensitivity, and backward compatibility
• Full test suite passes (382 tests, 0 failures)
• Black + isort formatting verified on changed files
• Manual validation with Gemini 2.5 Flash + is_reasoning_model: true

🤖 Generated with Claude Code

[MateoTTR]

Code Review — PR #4

Summary

Extracts the reasoning-model detection logic from a local variable inside generate_with_context() into a module-level constant + public function, then adds an is_reasoning_model: Optional[bool] = None field to LLMModelConfig that allows per-model override. The design is clean and the 3-state logic is correctly implemented.

Verdict: COMMENT (no changes required before merge, but see warnings below)
Upstream-ready: YES — branch contains only openevolve/, tests/, no fork artifacts. One pre-existing test file (test_openai_model_detection.py) needs a follow-up fix.

Stats

• Files reviewed: 3 changed + 1 pre-existing test
• Critical: 0 | Warnings: 3 | Suggestions: 2

---

CRITICAL — none

---

WARNINGS

W1 — test_openai_model_detection.py now tests phantom behaviour (pre-existing file, not changed by this PR)

The old test file (tests/test_openai_model_detection.py) defines a local is_reasoning_model() helper that includes an api_base == "https://api.openai.com/v1" guard. The real production code NEVER had that guard — it was always pure prefix matching. test_non_openai_api_base therefore tests an invariant that does not exist in the actual code (the test passes only because it calls its own local stub, not the real function). This was a latent issue before this PR, but it becomes more confusing now that there is a real is_reasoning_model() exported by the module. The test should be rewritten to call the real function and verify that is_reasoning_model("o1-mini", config_flag=None) returns True regardless of api_base (which is no longer a parameter at all), and that is_reasoning_model("o1-mini", config_flag=False) can be used by callers who need the old "non-OpenAI endpoint" behaviour.

W2 — gpt-oss-* entries are specific model names, not a prefix — any future gpt-oss-* variant will silently fall through to non-reasoning

"gpt-oss-120b" and "gpt-oss-20b" are used as prefixes via str.startswith(), which means they match those exact strings (and any longer name that starts with them, e.g. "gpt-oss-120b-2025"). However "gpt-oss-30b" or any other gpt-oss-* variant would not match. If the intention is to cover the whole gpt-oss family, the entry should be "gpt-oss-" instead.

W3 — Redundant prefixes in OPENAI_REASONING_MODEL_PREFIXES

"o1-" is fully redundant: "o1" already matches "o1-mini", "o1-preview", etc. because str.startswith("o1") is true for any string beginning with "o1". Same for "o3-" and "gpt-5-". The redundant entries have no effect on runtime behaviour (confirmed by test), but they add noise and could mislead future maintainers into thinking they are load-bearing. Suggesting removal of "o1-", "o3-", and "gpt-5-".

---

SUGGESTIONS

S1 — New test (test_reasoning_model_detection.py) does not cover gpt-oss-* models

TestIsReasoningModel tests o1, o3, o4, gpt-5, gemini, claude, and deepseek, but never exercises the two gpt-oss-* entries that were added to OPENAI_REASONING_MODEL_PREFIXES. A subTest loop over ["gpt-oss-120b", "gpt-oss-20b"] should be added.

S2 — is_reasoning_model not propagated through LLMConfig.update_model_params (intentional but undocumented)

The field is correctly omitted from shared_config in LLMConfig.__post_init__() and rebuild_models() — per-model overrides should not be overwritten by the top-level defaults. But there is no comment explaining why. A one-line comment (# is_reasoning_model is intentionally per-model only — not propagated from shared config) would prevent a future contributor from "fixing" the omission.

---

GOOD

• 3-state Optional[bool] design is idiomatic and backward compatible: existing YAML configs without the field produce None which falls through to auto-detection, preserving the old behaviour exactly.
• getattr(model_cfg, "is_reasoning_model", None) in OpenAILLM.__init__ is the right defensive pattern for forward/backward compatibility with configs that predate the field.
• Test coverage is comprehensive for the happy path: auto-detect (positive + negative), explicit override in both directions, case insensitivity, and config_flag=None equivalence. 12 targeted subtests is appropriate scope for this utility function.
• Branch is clean: only openevolve/config.py, openevolve/llm/openai.py, and tests/test_reasoning_model_detection.py — no fork-internal files.
• CI passes (unit + integration).

Automated review by Claude Code

[MateoTTR]

WARNING W2 — "gpt-oss-120b" and "gpt-oss-20b" are used as startswith prefixes, which means they only match those two exact model families (plus any longer name that starts with them). A future "gpt-oss-30b" or "gpt-oss-mini" would silently fall through to non-reasoning.

If the intention is to cover the entire gpt-oss family, replace both entries with the single prefix "gpt-oss-":

# The GPT OSS series are also reasoning models
"gpt-oss-",

If only these two specific variants are known reasoning models, add a comment clarifying that the list is exhaustive by intent, to prevent a future "fix".

[MateoTTR]

WARNING W3 — "o1-" is redundant: since "o1" is already in the tuple, str.startswith("o1") matches "o1-mini", "o1-preview", etc. just as well. Same applies to "o3-" (covered by "o3") and "gpt-5-" (covered by "gpt-5"). Confirmed with tests — removing the three dash-suffixed variants produces identical results for all known model names.

Suggest removing "o1-", "o3-", and "gpt-5-" to keep the constant minimal and avoid misleading future maintainers.

[MateoTTR]

SUGGESTION S1 — The new OPENAI_REASONING_MODEL_PREFIXES constant includes "gpt-oss-120b" and "gpt-oss-20b", but neither is exercised by TestIsReasoningModel. Add a subTest loop:

def test_gpt_oss_auto_detected(self):
    for model in ["gpt-oss-120b", "gpt-oss-20b"]:
        with self.subTest(model=model):
            self.assertTrue(is_reasoning_model(model))

This would also catch a future regression if one of the specific-name entries is accidentally removed.

[MateoTTR]

SUGGESTION S2 — This field is correctly omitted from shared_config inside LLMConfig.__post_init__() and rebuild_models() (lines ~186 and ~240). However there is no comment explaining the omission, which may look like a bug to a future contributor. Suggest adding a short inline comment in shared_config in both places:

shared_config = {
    ...
    "reasoning_effort": self.reasoning_effort,
    # is_reasoning_model is intentionally per-model only — not propagated from shared config
    "manual_mode": self.manual_mode,
}