Python: Fix walrus operator precedence for model_id kwarg in AzureOpenAIResponsesClient (#4310)

* Fix walrus operator precedence for model_id in AzureOpenAIResponsesClient (#4299)

Add parentheses around the walrus assignment so model_id receives the
actual string value instead of the boolean result of
`kwargs.pop(...) and not deployment_name`.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* 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>

* Add explicit validation for empty model_id in AzureOpenAIResponsesClient

Reject empty or whitespace-only model_id with ValueError instead of
silently passing an empty deployment name downstream. This ensures the
test_init_model_id_kwarg_empty_string test correctly validates behavior
defined in production code rather than relying on downstream validation.

Addresses PR review feedback for #4299.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Simplify model_id handling using walrus operator

Addresses review comment on PR #4310.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Restore explicit model_id validation to fix test failures (#4299)

The walrus operator refactor silently dropped the empty-string validation,
causing test_init_model_id_kwarg_empty_string to fail. Restore the explicit
None check and ValueError raise for empty model_id.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Revert "Restore explicit model_id validation to fix test failures (#4299)"

This reverts commit 1d2965fff6.

* Revert to walrus operator fix per review feedback

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Evan Mattson
2026-03-02 23:06:58 +09:00
committed by GitHub
Unverified
parent 26cef555ce
commit c4f643a750
2 changed files with 24 additions and 1 deletions
@@ -180,7 +180,7 @@ 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:
if (model_id := kwargs.pop("model_id", None)) and not deployment_name:
deployment_name = str(model_id)
# Project client path: create OpenAI client from an Azure AI Foundry project
@@ -90,6 +90,29 @@ def test_init_model_id_constructor(azure_openai_unit_test_env: dict[str, str]) -
assert isinstance(azure_responses_client, SupportsChatGetResponse)
def test_init_model_id_kwarg(azure_openai_unit_test_env: dict[str, str]) -> None:
"""Test that model_id kwarg correctly sets the deployment name (issue #4299)."""
azure_responses_client = AzureOpenAIResponsesClient(model_id="gpt-4o")
assert azure_responses_client.model_id == "gpt-4o"
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_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"}