diff --git a/python/packages/openai/AGENTS.md b/python/packages/openai/AGENTS.md index da64504352..1c8b0f4280 100644 --- a/python/packages/openai/AGENTS.md +++ b/python/packages/openai/AGENTS.md @@ -36,6 +36,10 @@ The generic OpenAI clients support both OpenAI and Azure OpenAI routing. Precede explicit Azure inputs (`credential`, `azure_endpoint`, `api_version`) → OpenAI API key (`OPENAI_API_KEY`) → Azure environment fallback (`AZURE_OPENAI_*`). +Request-model precedence is separate. An explicit `model=` argument wins. Otherwise an injected +`AsyncAzureOpenAI` client's configured deployment wins over `AZURE_OPENAI_*` model variables. +An injected `AsyncOpenAI` client has no deployment metadata, so OpenAI model variables still apply. + ## Adapting the Chat Completions client to OpenAI-compatible endpoints `OpenAIChatCompletionClient` targets the OpenAI Chat Completions wire format and is intentionally diff --git a/python/packages/openai/README.md b/python/packages/openai/README.md index 9198b8f101..e29bf9ff1a 100644 --- a/python/packages/openai/README.md +++ b/python/packages/openai/README.md @@ -100,7 +100,13 @@ Deployment lookup order: - `OpenAIEmbeddingClient`: `AZURE_OPENAI_EMBEDDING_MODEL` -> `AZURE_OPENAI_MODEL` For Azure routing, the same rule applies: the client-specific deployment variable is checked first, -then the generic `AZURE_OPENAI_MODEL` fallback. Passing `model=` overrides both environment variables. +then the generic `AZURE_OPENAI_MODEL` fallback. Passing `model=` overrides both environment variables +and the deployment configured on an injected client. + +When `model=` is omitted and an `AsyncAzureOpenAI` client is injected, that client's configured +deployment is used instead of the `AZURE_OPENAI_*` model variables. A plain `AsyncOpenAI` client +has no Azure deployment, so the OpenAI model variables above still apply. The injected client +stays caller-owned. When both OpenAI and Azure environment variables are present, the generic clients prefer OpenAI when `OPENAI_API_KEY` is configured. To use Azure explicitly, pass `azure_endpoint` or diff --git a/python/packages/openai/agent_framework_openai/_chat_client.py b/python/packages/openai/agent_framework_openai/_chat_client.py index 8f1a799bc2..c71d20d0d2 100644 --- a/python/packages/openai/agent_framework_openai/_chat_client.py +++ b/python/packages/openai/agent_framework_openai/_chat_client.py @@ -756,9 +756,12 @@ def __init__( """Initialize a raw OpenAI Chat client. Keyword Args: - model: Model identifier to use for the request. When not provided, the constructor - reads ``OPENAI_CHAT_MODEL`` and then ``OPENAI_MODEL`` for OpenAI, - or ``AZURE_OPENAI_CHAT_MODEL`` and then ``AZURE_OPENAI_MODEL`` for Azure. + model: Model identifier to use for the request. An explicit value wins over an + injected ``AsyncAzureOpenAI`` deployment, which wins over environment variables. + Otherwise the constructor reads ``OPENAI_CHAT_MODEL`` and then ``OPENAI_MODEL`` + for OpenAI, or ``AZURE_OPENAI_CHAT_MODEL`` and then ``AZURE_OPENAI_MODEL`` for + Azure. An injected ``AsyncOpenAI`` client has no deployment, so the OpenAI + variables still apply. api_key: API key override. For OpenAI this maps to ``OPENAI_API_KEY``. For Azure this can be used instead of ``AZURE_OPENAI_API_KEY`` for key auth. A callable token provider is also accepted for backwards compatibility, @@ -4825,10 +4828,12 @@ def __init__( """Initialize an OpenAI Responses client. Keyword Args: - model: Model identifier to use for the request. When not provided, the constructor - reads ``OPENAI_CHAT_MODEL`` and then ``OPENAI_MODEL`` for OpenAI - routing, or ``AZURE_OPENAI_CHAT_MODEL`` and then - ``AZURE_OPENAI_MODEL`` for Azure routing. + model: Model identifier to use for the request. An explicit value wins over an + injected ``AsyncAzureOpenAI`` deployment, which wins over environment variables. + Otherwise the constructor reads ``OPENAI_CHAT_MODEL`` and then ``OPENAI_MODEL`` + for OpenAI routing, or ``AZURE_OPENAI_CHAT_MODEL`` and then ``AZURE_OPENAI_MODEL`` + for Azure routing. An injected ``AsyncOpenAI`` client has no deployment, so the + OpenAI variables still apply. api_key: API key override. For OpenAI routing this maps to ``OPENAI_API_KEY``. For Azure routing this can be used instead of ``AZURE_OPENAI_API_KEY`` for key auth. A callable token provider is also accepted for backwards compatibility, diff --git a/python/packages/openai/agent_framework_openai/_chat_completion_client.py b/python/packages/openai/agent_framework_openai/_chat_completion_client.py index a9b7c9c106..c0f41763a0 100644 --- a/python/packages/openai/agent_framework_openai/_chat_completion_client.py +++ b/python/packages/openai/agent_framework_openai/_chat_completion_client.py @@ -414,9 +414,12 @@ def __init__( """Initialize a raw OpenAI Chat completion client. Keyword Args: - model: Model identifier to use for the request. When not provided, the constructor - reads ``OPENAI_CHAT_COMPLETION_MODEL`` and then ``OPENAI_MODEL`` for OpenAI routing, - or ``AZURE_OPENAI_CHAT_COMPLETION_MODEL`` and then ``AZURE_OPENAI_MODEL`` for Azure routing. + model: Model identifier to use for the request. An explicit value wins over an + injected ``AsyncAzureOpenAI`` deployment, which wins over environment variables. + Otherwise the constructor reads ``OPENAI_CHAT_COMPLETION_MODEL`` and then + ``OPENAI_MODEL`` for OpenAI routing, or ``AZURE_OPENAI_CHAT_COMPLETION_MODEL`` + and then ``AZURE_OPENAI_MODEL`` for Azure routing. An injected ``AsyncOpenAI`` + client has no deployment, so the OpenAI variables still apply. api_key: API key override. For OpenAI routing this maps to ``OPENAI_API_KEY``. For Azure routing this can be used instead of ``AZURE_OPENAI_API_KEY`` for key auth. A callable token provider is also accepted for backwards compatibility, @@ -1564,10 +1567,12 @@ def __init__( """Initialize an OpenAI Chat completion client. Keyword Args: - model: Model identifier to use for the request. When not provided, the constructor - reads ``OPENAI_CHAT_COMPLETION_MODEL`` and then ``OPENAI_MODEL`` for OpenAI routing, - or ``AZURE_OPENAI_CHAT_COMPLETION_MODEL`` and then - ``AZURE_OPENAI_MODEL`` for Azure routing. + model: Model identifier to use for the request. An explicit value wins over an + injected ``AsyncAzureOpenAI`` deployment, which wins over environment variables. + Otherwise the constructor reads ``OPENAI_CHAT_COMPLETION_MODEL`` and then + ``OPENAI_MODEL`` for OpenAI routing, or ``AZURE_OPENAI_CHAT_COMPLETION_MODEL`` + and then ``AZURE_OPENAI_MODEL`` for Azure routing. An injected ``AsyncOpenAI`` + client has no deployment, so the OpenAI variables still apply. api_key: API key override. For OpenAI routing this maps to ``OPENAI_API_KEY``. For Azure routing this can be used instead of ``AZURE_OPENAI_API_KEY`` for key auth. A callable token provider is also accepted for backwards compatibility, diff --git a/python/packages/openai/agent_framework_openai/_embedding_client.py b/python/packages/openai/agent_framework_openai/_embedding_client.py index 9560f99cc9..20823c0b95 100644 --- a/python/packages/openai/agent_framework_openai/_embedding_client.py +++ b/python/packages/openai/agent_framework_openai/_embedding_client.py @@ -168,10 +168,12 @@ def __init__( """Initialize a raw OpenAI embedding client. Keyword Args: - model: Embedding model or Azure OpenAI deployment name. When not provided, the - constructor reads ``OPENAI_EMBEDDING_MODEL`` and then ``OPENAI_MODEL`` - for OpenAI. For Azure it first checks ``AZURE_OPENAI_EMBEDDING_MODEL`` - and then ``AZURE_OPENAI_MODEL``. + model: Embedding model or Azure OpenAI deployment name. An explicit value wins + over an injected ``AsyncAzureOpenAI`` deployment, which wins over environment + variables. Otherwise the constructor reads ``OPENAI_EMBEDDING_MODEL`` and then + ``OPENAI_MODEL`` for OpenAI. For Azure it first checks + ``AZURE_OPENAI_EMBEDDING_MODEL`` and then ``AZURE_OPENAI_MODEL``. An injected + ``AsyncOpenAI`` client has no deployment, so the OpenAI variables still apply. api_key: API key override. For OpenAI this maps to ``OPENAI_API_KEY``. For Azure this can be used instead of ``AZURE_OPENAI_API_KEY`` for key auth. A callable token provider is also accepted for backwards compatibility, @@ -454,10 +456,12 @@ def __init__( """Initialize an OpenAI embedding client. Keyword Args: - model: Embedding model or Azure OpenAI deployment name. When not provided, the - constructor reads ``OPENAI_EMBEDDING_MODEL`` and then ``OPENAI_MODEL`` - for OpenAI. For Azure it first checks ``AZURE_OPENAI_EMBEDDING_MODEL`` - and then ``AZURE_OPENAI_MODEL``. + model: Embedding model or Azure OpenAI deployment name. An explicit value wins + over an injected ``AsyncAzureOpenAI`` deployment, which wins over environment + variables. Otherwise the constructor reads ``OPENAI_EMBEDDING_MODEL`` and then + ``OPENAI_MODEL`` for OpenAI. For Azure it first checks + ``AZURE_OPENAI_EMBEDDING_MODEL`` and then ``AZURE_OPENAI_MODEL``. An injected + ``AsyncOpenAI`` client has no deployment, so the OpenAI variables still apply. api_key: API key override. For OpenAI this maps to ``OPENAI_API_KEY``. For Azure this can be used instead of ``AZURE_OPENAI_API_KEY`` for key auth. A callable token provider is also accepted for backwards compatibility, diff --git a/python/packages/openai/agent_framework_openai/_shared.py b/python/packages/openai/agent_framework_openai/_shared.py index 68085e5b42..62cec90697 100644 --- a/python/packages/openai/agent_framework_openai/_shared.py +++ b/python/packages/openai/agent_framework_openai/_shared.py @@ -218,6 +218,16 @@ def _resolve_named_setting( return None +def _injected_azure_deployment(client: AsyncOpenAI | None) -> str | None: + """Return the deployment configured on an injected Azure client, if any.""" + if not isinstance(client, AsyncAzureOpenAI): + return None + azure_deployment = getattr(client, "_azure_deployment", None) + if isinstance(azure_deployment, str) and azure_deployment: + return azure_deployment + return None + + def _join_env_names(env_names: Sequence[str]) -> str: """Format env var names for user-facing error messages.""" return ", ".join(f"'{env_name}'" for env_name in env_names) @@ -268,6 +278,12 @@ def load_openai_service_settings( environment variables are used only when an explicit Azure signal is present (``endpoint`` or ``credential``) or when no explicit OpenAI API key is available. + + Model precedence on the Azure path is an explicit ``model`` argument, then the + deployment configured on an injected ``AsyncAzureOpenAI`` client, then + ``AZURE_OPENAI_*`` model aliases. An injected ``AsyncOpenAI`` client has no + deployment metadata, so it still resolves the model from the explicit argument + or the OpenAI environment variables. """ # Merge APP_INFO into the headers merged_headers = dict(copy(default_headers)) if default_headers else {} @@ -339,11 +355,15 @@ def load_openai_service_settings( if model is not None: azure_settings[azure_model_fields[0]] = model client_args = {} - resolved_azure_model = _resolve_named_setting(azure_settings, azure_model_fields) - if resolved_azure_model is None and client: - azure_deployment = getattr(client, "_azure_deployment", None) - if isinstance(azure_deployment, str) and azure_deployment: - resolved_azure_model = azure_deployment + # An explicit model wins. Otherwise keep the wrapper on the deployment the + # caller already configured on the injected client, ahead of ambient aliases. + explicit_model = model if isinstance(model, str) and model else None + injected_deployment = _injected_azure_deployment(client) if explicit_model is None else None + if injected_deployment is not None: + azure_settings[azure_model_fields[0]] = injected_deployment + resolved_azure_model = ( + explicit_model or injected_deployment or _resolve_named_setting(azure_settings, azure_model_fields) + ) if resolved_azure_model: azure_settings["model"] = resolved_azure_model client_args["azure_deployment"] = resolved_azure_model diff --git a/python/packages/openai/tests/openai/conftest.py b/python/packages/openai/tests/openai/conftest.py index 2181e1d67d..c68e523bf6 100644 --- a/python/packages/openai/tests/openai/conftest.py +++ b/python/packages/openai/tests/openai/conftest.py @@ -1,9 +1,10 @@ # Copyright (c) Microsoft. All rights reserved. -from collections.abc import Generator +from collections.abc import Callable, Generator from typing import Any, cast from unittest.mock import patch +from openai import AsyncAzureOpenAI from opentelemetry.sdk.trace.export import SimpleSpanProcessor, SpanExporter from opentelemetry.sdk.trace.export.in_memory_span_exporter import InMemorySpanExporter from pytest import FixtureRequest, fixture, skip @@ -148,6 +149,23 @@ def azure_openai_unit_test_env(monkeypatch, exclude_list, override_env_param_dic return env_vars +@fixture +def azure_sdk_client_factory() -> Callable[[str | None], AsyncAzureOpenAI]: + """Return a factory for dummy AsyncAzureOpenAI clients with an optional deployment.""" + + def create_client(deployment: str | None) -> AsyncAzureOpenAI: + kwargs: dict[str, Any] = { + "api_key": "dummy", + "azure_endpoint": "https://example.openai.azure.com", + "api_version": "2025-01-01-preview", + } + if deployment is not None: + kwargs["azure_deployment"] = deployment + return AsyncAzureOpenAI(**kwargs) + + return create_client + + # region Observability fixtures @fixture def enable_instrumentation(request: Any) -> bool: diff --git a/python/packages/openai/tests/openai/test_injected_client_model.py b/python/packages/openai/tests/openai/test_injected_client_model.py new file mode 100644 index 0000000000..4c9502570b --- /dev/null +++ b/python/packages/openai/tests/openai/test_injected_client_model.py @@ -0,0 +1,118 @@ +# Copyright (c) Microsoft. All rights reserved. + +from __future__ import annotations + +from collections.abc import Callable +from typing import Any + +import pytest +from openai import AsyncAzureOpenAI, AsyncOpenAI + +from agent_framework_openai import OpenAIChatClient, OpenAIChatCompletionClient, OpenAIEmbeddingClient + +pytestmark = pytest.mark.azure + +_AZURE_CLIENTS = [ + pytest.param(OpenAIChatClient, "AZURE_OPENAI_CHAT_MODEL", id="chat"), + pytest.param(OpenAIChatCompletionClient, "AZURE_OPENAI_CHAT_COMPLETION_MODEL", id="chat-completion"), + pytest.param(OpenAIEmbeddingClient, "AZURE_OPENAI_EMBEDDING_MODEL", id="embedding"), +] + +_OPENAI_CLIENTS = [ + pytest.param(OpenAIChatClient, "OPENAI_CHAT_MODEL", id="chat"), + pytest.param(OpenAIChatCompletionClient, "OPENAI_CHAT_COMPLETION_MODEL", id="chat-completion"), + pytest.param(OpenAIEmbeddingClient, "OPENAI_EMBEDDING_MODEL", id="embedding"), +] + + +@pytest.mark.parametrize(("client_cls", "specific_env"), _AZURE_CLIENTS) +def test_injected_azure_deployment_wins_over_environment( + client_cls: type[Any], + specific_env: str, + monkeypatch: pytest.MonkeyPatch, + azure_openai_unit_test_env: dict[str, str], + azure_sdk_client_factory: Callable[[str | None], AsyncAzureOpenAI], +) -> None: + monkeypatch.setenv(specific_env, "env-specific") + monkeypatch.setenv("AZURE_OPENAI_MODEL", "env-generic") + sdk_client = azure_sdk_client_factory("client-deployment") + + client = client_cls(async_client=sdk_client) + + assert client.model == "client-deployment" + assert client.client is sdk_client + assert sdk_client._azure_deployment == "client-deployment" + assert azure_openai_unit_test_env[specific_env] + + +@pytest.mark.parametrize(("client_cls", "specific_env"), _AZURE_CLIENTS) +def test_explicit_model_wins_over_injected_deployment_and_environment( + client_cls: type[Any], + specific_env: str, + monkeypatch: pytest.MonkeyPatch, + azure_openai_unit_test_env: dict[str, str], + azure_sdk_client_factory: Callable[[str | None], AsyncAzureOpenAI], +) -> None: + monkeypatch.setenv(specific_env, "env-specific") + monkeypatch.setenv("AZURE_OPENAI_MODEL", "env-generic") + sdk_client = azure_sdk_client_factory("client-deployment") + + client = client_cls(model="explicit-deployment", async_client=sdk_client) + + assert client.model == "explicit-deployment" + assert client.client is sdk_client + assert sdk_client._azure_deployment == "client-deployment" + assert azure_openai_unit_test_env["AZURE_OPENAI_MODEL"] + + +@pytest.mark.parametrize(("client_cls", "specific_env"), _AZURE_CLIENTS) +def test_injected_azure_client_without_deployment_keeps_environment_fallback( + client_cls: type[Any], + specific_env: str, + azure_openai_unit_test_env: dict[str, str], + azure_sdk_client_factory: Callable[[str | None], AsyncAzureOpenAI], +) -> None: + sdk_client = azure_sdk_client_factory(None) + + client = client_cls(async_client=sdk_client) + + assert client.model == azure_openai_unit_test_env[specific_env] + assert client.client is sdk_client + + +@pytest.mark.parametrize(("client_cls", "specific_env"), _OPENAI_CLIENTS) +def test_injected_openai_client_uses_openai_environment( + client_cls: type[Any], + specific_env: str, + monkeypatch: pytest.MonkeyPatch, + openai_unit_test_env: dict[str, str], +) -> None: + monkeypatch.setenv(specific_env, "openai-specific") + monkeypatch.setenv("AZURE_OPENAI_CHAT_MODEL", "azure-chat") + monkeypatch.setenv("AZURE_OPENAI_CHAT_COMPLETION_MODEL", "azure-chat-completion") + monkeypatch.setenv("AZURE_OPENAI_EMBEDDING_MODEL", "azure-embedding") + monkeypatch.setenv("AZURE_OPENAI_MODEL", "azure-generic") + sdk_client = AsyncOpenAI(api_key="dummy") + + client = client_cls(async_client=sdk_client) + + assert client.model == "openai-specific" + assert client.client is sdk_client + assert openai_unit_test_env["OPENAI_MODEL"] + + +@pytest.mark.parametrize(("client_cls", "specific_env"), _OPENAI_CLIENTS) +def test_explicit_model_wins_for_injected_openai_client( + client_cls: type[Any], + specific_env: str, + monkeypatch: pytest.MonkeyPatch, + openai_unit_test_env: dict[str, str], +) -> None: + monkeypatch.setenv(specific_env, "openai-specific") + sdk_client = AsyncOpenAI(api_key="dummy") + + client = client_cls(model="explicit-model", async_client=sdk_client) + + assert client.model == "explicit-model" + assert client.client is sdk_client + assert openai_unit_test_env["OPENAI_API_KEY"]