diff --git a/dspy/clients/_openai_model_family.py b/dspy/clients/_openai_model_family.py new file mode 100644 index 0000000000..72eca6c586 --- /dev/null +++ b/dspy/clients/_openai_model_family.py @@ -0,0 +1,39 @@ +"""Shared OpenAI reasoning-model classifier. + +Both :mod:`dspy.clients.lm` and :mod:`dspy.clients.openai_format` need to decide +whether a model string names an OpenAI reasoning-family model (``o1``/``o3``/ +``o4``/``o5``, ``gpt-5`` excluding ``gpt-5-chat``). They previously carried two +separate copies that diverged in provider-prefix stripping and family grammar: + +* ``lm.py`` stripped any provider prefix (``model.split("/")[-1]``) and matched an + anchored regex. +* ``openai_format.py`` stripped only the literal ``openai/`` prefix and matched + a loose ``startswith``, so non-``openai/`` prefixes like ``azure/o3`` were + misclassified as non-reasoning. + +This module is the single source of truth so future drift is a single +definition, not two. :mod:`dspy.clients.openai_format` cannot import +:mod:`dspy.clients.lm` (which already imports the other way), hence this neutral +third module. +""" + +from __future__ import annotations + +import re + +_OPENAI_REASONING_MODEL_RE = re.compile( + r"^(?:o1-preview|o[1345](?:-(?:mini|nano|pro))?(?:-\d{4}-\d{2}-\d{2})?|gpt-5(?!-chat)(?:-.*)?)$" +) + + +def is_openai_reasoning_model(model: str | None) -> bool: + """Return ``True`` iff ``model`` names an OpenAI reasoning-family model. + + Any LiteLLM provider prefix (``openai/``, ``azure/``, ``vertex_ai/`` ...) is + stripped; the family is decided by the bare model suffix. ``None`` and + non-strings return ``False`` (the adapter path may pass a missing model). + """ + if not isinstance(model, str): + return False + model_family = model.split("/")[-1].lower() if "/" in model else model.lower() + return _OPENAI_REASONING_MODEL_RE.match(model_family) is not None diff --git a/dspy/clients/lm.py b/dspy/clients/lm.py index b6fa8ffb4a..becaeabff9 100644 --- a/dspy/clients/lm.py +++ b/dspy/clients/lm.py @@ -1,7 +1,6 @@ import functools import logging import os -import re import threading import warnings from typing import Any, Literal, cast @@ -11,6 +10,7 @@ import dspy from dspy.clients._litellm import get_litellm, is_litellm_context_window_error +from dspy.clients._openai_model_family import is_openai_reasoning_model from dspy.clients.cache import request_cache from dspy.clients.openai import OpenAIProvider from dspy.clients.openai_format import to_openai_responses_request @@ -45,14 +45,6 @@ def _get_litellm(): return get_litellm(feature="dspy.LM") -def _is_openai_reasoning_model(model: str) -> bool: - model_family = model.split("/")[-1].lower() if "/" in model else model.lower() - return re.match( - r"^(?:o[1345](?:-(?:mini|nano|pro))?(?:-\d{4}-\d{2}-\d{2})?|gpt-5(?!-chat)(?:-.*)?)$", - model_family, - ) is not None - - class LM(BaseLM): """ A language model supporting chat or text completion requests for use with DSPy modules. @@ -122,7 +114,7 @@ def __init__( def _get_initial_kwargs(self, *, temperature, max_tokens, **kwargs) -> dict[str, Any]: # Override BaseLM's default kwargs shape for LiteLLM/model-family-specific token parameters. - if _is_openai_reasoning_model(self.model): + if is_openai_reasoning_model(self.model): if (temperature and temperature != 1.0) or (max_tokens and max_tokens < 16000): raise LMConfigurationError( "OpenAI's reasoning models require passing temperature=1.0 or None and max_tokens >= 16000 or None to " @@ -418,7 +410,7 @@ def dump_state(self): ) if self.use_developer_role: state["use_developer_role"] = self.use_developer_role - if _is_openai_reasoning_model(self.model) and "max_completion_tokens" in state: + if is_openai_reasoning_model(self.model) and "max_completion_tokens" in state: state["max_tokens"] = state.pop("max_completion_tokens") return state @@ -427,7 +419,7 @@ def load_state(cls, state: dict[str, Any], *, allow_custom_lm_class: bool = Fals state = dict(state) model = state.get("model") - if isinstance(model, str) and _is_openai_reasoning_model(model) and "max_completion_tokens" in state: + if isinstance(model, str) and is_openai_reasoning_model(model) and "max_completion_tokens" in state: if "max_tokens" not in state: state["max_tokens"] = state["max_completion_tokens"] state.pop("max_completion_tokens") diff --git a/dspy/clients/openai_format.py b/dspy/clients/openai_format.py index 2a18b136ee..54ba77a76f 100644 --- a/dspy/clients/openai_format.py +++ b/dspy/clients/openai_format.py @@ -29,6 +29,7 @@ import pydantic +from dspy.clients._openai_model_family import is_openai_reasoning_model from dspy.core.types import ( LMAudioPart, LMBinaryPart, @@ -521,7 +522,7 @@ def reasoning_to_responses_kwargs(reasoning: Any) -> dict[str, Any]: def _validate_openai_reasoning_temperature(config: LMConfig, *, model: str | None, endpoint: str) -> None: - if not _is_openai_reasoning_model(model): + if not is_openai_reasoning_model(model): return effort = getattr(config.reasoning, "effort", None) if config.reasoning is not None else None if effort in {None, "none"}: @@ -544,16 +545,7 @@ def _validate_openai_reasoning_temperature(config: LMConfig, *, model: str | Non def _uses_max_completion_tokens(model: str | None) -> bool: - return _is_openai_reasoning_model(model) - - -def _is_openai_reasoning_model(model: str | None) -> bool: - if not isinstance(model, str): - return False - model_name = model.removeprefix("openai/").lower() - if "chat" in model_name: - return False - return model_name.startswith(("o1", "o3", "o4", "gpt-5")) + return is_openai_reasoning_model(model) def prompt_cache_to_kwargs(cache: Any) -> dict[str, Any]: diff --git a/tests/clients/test_lm.py b/tests/clients/test_lm.py index 6588348092..23d3f77d48 100644 --- a/tests/clients/test_lm.py +++ b/tests/clients/test_lm.py @@ -384,6 +384,8 @@ def test_reasoning_model_token_parameter(): ("openai/gpt-5", True), ("openai/gpt-5-mini", True), ("openai/gpt-5-nano", True), + ("azure/o3", True), # non-openai/ provider prefix must be classified as reasoning too + ("azure/gpt-5", True), ("azure/gpt-5-chat", False), # gpt-5-chat is NOT a reasoning model ("openai/gpt-4", False), ("anthropic/claude-2", False), @@ -410,7 +412,7 @@ def test_lm_supports_reasoning_with_litellm_capability_api(): assert lm.supports_reasoning is True -@pytest.mark.parametrize("model_name", ["openai/o1", "openai/gpt-5-nano", "openai/gpt-5-mini"]) +@pytest.mark.parametrize("model_name", ["openai/o1", "openai/gpt-5-nano", "openai/gpt-5-mini", "azure/o3"]) def test_reasoning_model_requirements(model_name): # Should raise assertion error if temperature or max_tokens requirements not met with pytest.raises( @@ -454,6 +456,78 @@ def test_gpt_5_chat_not_reasoning_model(): assert lm.kwargs["temperature"] == 0.7 +def test_is_openai_reasoning_model_is_shared_single_source_of_truth(): + """Guard against the two-module drift that caused the azure/o3 bug. + + `dspy.clients.lm` and `dspy.clients.openai_format` must use the *same* + classifier object from the shared `_openai_model_family` module; a naive + re-copy would reintroduce the prefix-strip / grammar divergence. + """ + from dspy.clients import lm as lm_mod + from dspy.clients import openai_format as openai_format_mod + from dspy.clients._openai_model_family import is_openai_reasoning_model + + assert lm_mod.is_openai_reasoning_model is is_openai_reasoning_model + assert openai_format_mod.is_openai_reasoning_model is is_openai_reasoning_model + + +@pytest.mark.parametrize( + "model, expected", + [ + # Any LiteLLM provider prefix is stripped; the bare suffix decides the family. + ("openai/o3", True), + ("azure/o3", True), + ("o3", True), + ("azure/o1", True), + ("azure/o3-mini-2023-01-01", True), + ("azure/gpt-5", True), + ("azure/gpt-5-mini", True), + ("azure/gpt-5-nano", True), + # o5 is matched by the anchored regex (o[1345]); the old loose `startswith` + # in openai_format.py did NOT list o5, so this guards against a naive revert. + ("openai/o5", True), + ("azure/o5", True), + # gpt-5-chat is excluded by the grammar's (?!-chat) negative lookahead. + ("azure/gpt-5-chat", False), + ("openai/gpt-4", False), + ("anthropic/claude-2", False), + # Strict anchored grammar rejects loose `startswith` matches; a naive + # prefix-only fix would have accepted these and re-diverged from lm.py. + ("azure/o3-preview", False), + ("azure/o1-mini-pro", False), + # The adapter path may pass a missing model; non-strings are non-reasoning. + (None, False), + ], +) +def test_is_openai_reasoning_model_parses_provider_prefixes_and_grammar(model, expected): + from dspy.clients._openai_model_family import is_openai_reasoning_model + + assert is_openai_reasoning_model(model) is expected + + +@pytest.mark.parametrize("model", ["azure/o3", "openai/o3"]) +def test_adapter_path_reasoning_model_uses_max_completion_tokens(model): + """Regression: the adapter path (`dspy.Predict`) routes through + `dspy.clients.openai_format.common_config_kwargs`, whose classifier used to + only strip the literal `openai/` prefix and misclassified `azure/o3` as + non-reasoning. The token limit set as a Predictor generation kwarg must be + sent under `max_completion_tokens` (not `max_tokens`) for any provider prefix. + """ + captured = {} + + def fake_completion(request=None, num_retries=0, cache=None, **kw): + captured.update(request) + return _model_response('{"answer": "42"}') + + lm = dspy.LM(model, temperature=None, cache=False) + pred = dspy.Predict("question -> answer", max_tokens=16_000, reasoning_effort="medium") + with mock.patch("dspy.clients.lm.litellm_completion", side_effect=fake_completion): + pred(question="hi", lm=lm) + + assert captured["max_completion_tokens"] == 16_000 + assert "max_tokens" not in captured + + def test_base_lm_init_uses_lm_defaults_and_isolates_callback_list(): callbacks = [object()] lm = dspy.BaseLM("custom-model", callbacks=callbacks)