Repository navigation
Promote Azure served model header for Responses - #3545
scarab-systems wants to merge 4 commits into
Conversation
Promote non-empty x-ms-served-model values into Azure Responses model fields before response construction. This keeps the behavior scoped to Azure clients and covers both parsed Responses objects and streamed response events without touching generated resource files. Add sync and async Azure regression coverage for non-streaming and streaming Responses calls.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
The production change is focused and behaves correctly in the exercised sync/async and streaming paths, but the test structure needs one cleanup before merge.
tests/lib/test_azure.py grows from 955 to 1,084 lines in this PR, crossing the 1k-line boundary, and the added 130 lines repeat nearly identical client/router setup across four tests. Please move this feature coverage into a focused module (for example, tests/lib/test_azure_responses.py) or collapse the duplication with shared fixtures/helpers so the existing catch-all file does not keep sprawling past 1k lines. This is a clear decomposition boundary and will also make the important absent/blank-header and non-Responses guards easier to cover without making the original file larger.
Validation on exact head d5a658b6125d5d877aaf04c9b6db72eae4255fcb: all 63 Azure tests passed; Ruff lint and format checks passed; Pyright passed; mypy passed; git diff --check passed.
|
Addressed in the latest push ( I moved the Azure Responses served-model coverage out of Local validation run:
|
|
AI-assisted independent offline check at 22/22 controls pass on this head versus 14/22 on pinned main. Added controls cover Responses retrieval paths, nested response events, trimmed headers, a similar non-Responses suffix, non-mapping/list/delta payloads and non-mapping nested response fields. Both sync and async client objects process these inputs; status/usage/sequence fields remain unchanged and source dictionaries are not mutated. Scope: Actual _process_response_data into object with constructed response headers and URLs. No request transport, model-specific parsing, Azure header provenance or live provider behavior is established. This supplements the existing public-client tests with payload-shape and mutation controls. Python 3.14.7 / Pydantic 2.13.5 and shared local dependencies; this is a bounded matrix, not a full SDK suite or historical dependency matrix. Network denied by the sandbox. Standalone reproducer (run separately with each source tree on PYTHONPATH)import json,copy,asyncio,importlib
from openai.lib.azure import AzureOpenAI,AsyncAzureOpenAI
m=importlib.import_module('openai.lib.azure');http=getattr(m,'httpx2',None) or m.httpx
cases=[('create','/openai/responses','served',{'model':'body','usage':{'total_tokens':7}},True),('retrieve','/openai/responses/fictional','served',{'model':'body','status':'incomplete'},True),('nested_event','/openai/responses/fictional/events','served',{'type':'response.created','sequence_number':3,'response':{'model':'body','usage':{'total_tokens':7}}},True),('trim','/openai/responses/',' served ',{'model':'body'},True),('blank','/openai/responses',' ',{'model':'body'},False),('absent','/openai/responses',None,{'model':'body'},False),('chat','/openai/chat/completions','served',{'model':'body'},False),('similar_suffix','/openai/not-responses','served',{'model':'body'},False),('list','/openai/responses','served',[{'model':'body'}],False),('delta','/openai/responses','served',{'type':'response.output_text.delta','delta':'fictional'},False),('nonnested_mapping','/openai/responses','served',{'response':'fictional','type':'fictional'},False)]
rows=[]
for async_ in [False,True]:
c=(AsyncAzureOpenAI if async_ else AzureOpenAI)(api_key='fictional',api_version='fictional',azure_endpoint='https://example.invalid')
for name,path,header,data,replace in cases:
before=copy.deepcopy(data);expected=copy.deepcopy(data)
if replace:
target=expected.get('response',expected);target['model']='served'
response=http.Response(200,request=http.Request('GET','https://example.invalid'+path),headers={'x-ms-served-model':header} if header is not None else {})
actual=c._process_response_data(data=data,cast_to=object,response=response)
rows.append({'case':name,'async_client':async_,'actual':actual,'expected':expected,'input_unchanged':data==before,'pass':actual==expected and data==before})
if async_:asyncio.run(c.close())
else:c.close()
print(json.dumps({'cases':rows,'passed':sum(x['pass'] for x in rows),'failed':sum(not x['pass'] for x in rows),'scope':'Actual Azure client response processing into object; synthetic response headers/paths, no transport/provider header provenance claim.'})) |
Move Azure Responses served-model header coverage out of the catch-all Azure test module into a focused tests/lib/test_azure_responses.py module. Keep tests/lib/test_azure.py below the 1k-line decomposition boundary and add focused guard coverage for absent or blank served-model headers and non-Responses resources.
51de468 to
6425870
Compare
Resolve the Azure imports while preserving upstream authentication, redirects, deployment paths, and data residency. Adapt the served-model response hook and focused regression tests to HTTPX2 and the current test router. Validation: Azure tests passed on Python 3.10 and 3.13 with Pydantic v2, and on Python 3.13 with Pydantic v1 (840 passed, 83 skipped per run). Full Ruff lint, changed-file formatting, full ty checks, import smoke check, and whitespace checks passed.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5e7b1cbd9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def _is_responses_request(response: httpx2.Response) -> bool: | ||
| path = response.request.url.path.rstrip("/") | ||
| return path.endswith("/responses") or "/responses/" in path |
There was a problem hiding this comment.
With an Azure deployment named responses, Chat Completions requests use /openai/deployments/responses/chat/completions, which this substring check incorrectly identifies as a Responses endpoint. If the response includes x-ms-served-model, the new processing hook overwrites ChatCompletion.model and streamed chunk models, violating the Responses-only scope explicitly tested in this change. Match the actual resource route or Responses result type rather than accepting any /responses/ path segment.
Useful? React with 👍 / 👎.
Inspect the encoded resource path after its deployment segment so a deployment named responses cannot change Chat Completions model values. Preserve genuine Responses routes, including deployment-scoped and proxy-prefixed base URLs. Add synchronous and asynchronous JSON, streaming, and retrieval regressions. Validation: - 12 new Chat Completions cases failed before the fix; all 25 focused tests pass. - Full SDK suite: 21,457 passed and 205 skipped with Pydantic v2; 21,441 passed and 221 skipped with Pydantic v1. - Ruff lint and formatting, full type checking, import smoke test, and diff checks pass.
Fixes #3271.
Summary
x-ms-served-modelresponse headers into Responses APIResponse.modelevent.response.modelTesting
Azure regression command (run in each locked environment):
python -m pytest -o addopts= --tb=short tests/lib/test_azure*.py -qAlso passed:
src/openai/lib/azure.pyandtests/lib/test_azure_responses.pygit diff --checkTests use mocked HTTP or local fixture servers; no live Azure API calls were made.