Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions dspy/clients/_openai_model_family.py
Original file line number Diff line number Diff line change
@@ -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)(?:-.*)?)$"
)
Comment thread
greptile-apps[bot] marked this conversation as resolved.


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
16 changes: 4 additions & 12 deletions dspy/clients/lm.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import functools
import logging
import os
import re
import threading
import warnings
from typing import Any, Literal, cast
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 "
Expand Down Expand Up @@ -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

Expand All @@ -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")
Expand Down
14 changes: 3 additions & 11 deletions dspy/clients/openai_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import pydantic

from dspy.clients._openai_model_family import is_openai_reasoning_model
from dspy.core.types import (
LMAudioPart,
LMBinaryPart,
Expand Down Expand Up @@ -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"}:
Expand All @@ -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]:
Expand Down
76 changes: 75 additions & 1 deletion tests/clients/test_lm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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(
Expand Down Expand Up @@ -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)
Expand Down