From 9c3d8c795808dc4b992ecc7781b6859ad960e414 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Thu, 26 Feb 2026 20:18:19 +0900 Subject: [PATCH] Address review: replace walrus with explicit None check, add edge-case tests (#4299) - Replace walrus operator with explicit assignment and 'is not None' check to avoid boolean-coercion pitfalls (empty string now correctly surfaces as ValueError instead of silently falling back) - Add test: deployment_name takes precedence over model_id kwarg - Add test: model_id='' raises ValueError - Add test: model_id=None falls back to env var Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../azure/_responses_client.py | 3 ++- .../azure/test_azure_responses_client.py | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/python/packages/core/agent_framework/azure/_responses_client.py b/python/packages/core/agent_framework/azure/_responses_client.py index 2debbd7b21..a96d596176 100644 --- a/python/packages/core/agent_framework/azure/_responses_client.py +++ b/python/packages/core/agent_framework/azure/_responses_client.py @@ -180,7 +180,8 @@ class AzureOpenAIResponsesClient( # type: ignore[misc] client: AzureOpenAIResponsesClient[MyOptions] = AzureOpenAIResponsesClient() response = await client.get_response("Hello", options={"my_custom_option": "value"}) """ - if (model_id := kwargs.pop("model_id", None)) and not deployment_name: + model_id = kwargs.pop("model_id", None) + if model_id is not None and deployment_name is None: deployment_name = str(model_id) # Project client path: create OpenAI client from an Azure AI Foundry project diff --git a/python/packages/core/tests/azure/test_azure_responses_client.py b/python/packages/core/tests/azure/test_azure_responses_client.py index 0b1e076d08..0bc043f4fe 100644 --- a/python/packages/core/tests/azure/test_azure_responses_client.py +++ b/python/packages/core/tests/azure/test_azure_responses_client.py @@ -98,6 +98,27 @@ def test_init_model_id_kwarg(azure_openai_unit_test_env: dict[str, str]) -> None assert isinstance(azure_responses_client, SupportsChatGetResponse) +def test_init_model_id_kwarg_does_not_override_deployment_name(azure_openai_unit_test_env: dict[str, str]) -> None: + """Test that deployment_name takes precedence over model_id kwarg (issue #4299).""" + azure_responses_client = AzureOpenAIResponsesClient(deployment_name="my-deployment", model_id="gpt-4o") + + assert azure_responses_client.model_id == "my-deployment" + assert isinstance(azure_responses_client, SupportsChatGetResponse) + + +def test_init_model_id_kwarg_empty_string(azure_openai_unit_test_env: dict[str, str]) -> None: + """Test that model_id="" surfaces as invalid rather than silently falling back.""" + with pytest.raises(ValueError): + AzureOpenAIResponsesClient(model_id="") + + +def test_init_model_id_kwarg_none(azure_openai_unit_test_env: dict[str, str]) -> None: + """Test that model_id=None does not override the env-var deployment name.""" + azure_responses_client = AzureOpenAIResponsesClient(model_id=None) + + assert azure_responses_client.model_id == azure_openai_unit_test_env["AZURE_OPENAI_RESPONSES_DEPLOYMENT_NAME"] + + def test_init_with_default_header(azure_openai_unit_test_env: dict[str, str]) -> None: default_headers = {"X-Unit-Test": "test-guid"}