Python: fix(python): normalize Azure AI agent response_format#13891
Python: fix(python): normalize Azure AI agent response_format#13891MukundaKatta wants to merge 1 commit intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 89%
✓ Correctness
The diff adds a
_normalize_response_formatclassmethod that converts dict-basedjson_schemaresponse formats into strongly-typedResponseFormatJsonSchemaTypeobjects before passing them to the Azure SDK. The normalization is integrated into_merge_optionsand applied to both run-level and agent-level response formats. The implementation correctly handles all edge cases (None, already-typed, non-dict, non-json_schema dicts, dicts with non-dict json_schema). The new test verifies the main use case of dict-to-typed conversion via_generate_options. The import ofResponseFormatJsonSchemais consistent with existing usage in the sample code. No correctness issues found.
✓ Security Reliability
This change adds a
_normalize_response_formathelper that converts dict-based JSON schema response formats into typedResponseFormatJsonSchemaTypeSDK objects, and integrates it into_merge_options. The normalization logic is defensively written with multiple guard clauses that correctly handle None, already-typed objects, non-dict inputs, dicts without the expected structure, and dicts with a non-dictjson_schemafield. TheAgentsApiResponseFormatOptiontype alias (str | ResponseFormatJsonSchemaType) meansagent.definition.response_formatcould also be a plain string, which the normalizer handles correctly by returning it as-is (theisinstance(response_format, dict)check fails for strings). No security or reliability issues were identified.
✓ Test Coverage
The PR adds
_normalize_response_formatto convert dict-based response formats intoResponseFormatJsonSchemaTypeobjects and includes one integration test covering the happy path via_generate_options. While the happy path is covered, several branches of_normalize_response_formatlack tests (None input, already-typed input, non-json_schema dicts, missing/non-dict json_schema), and the test doesn't verify all propagated fields (description,schema). There is also no test exercising the run-levelresponse_formatparameter normalization path.
✓ Design Approach
The change adds a
_normalize_response_formathelper that converts a raw dict withtype: "json_schema"to the SDK'sResponseFormatJsonSchemaTypeobject, and calls it in_merge_optionsbefore returning the merged options. The approach is sound:agent.definition.response_formatcan arrive as a plain dict from the Azure SDK deserialization layer, and normalizing it at the single merge/consume point (_merge_options) is a defensible place. There are no fragile assumptions, no leaky abstractions, and no symptom-level masking. One minor observation worth noting: the_merge_optionssignature still declaresresponse_format: ResponseFormatJsonSchemaType | None, which is narrower than bothAgentsApiResponseFormatOption = str | ResponseFormatJsonSchemaType(used by all three publicinvoke/get_responsemethods inazure_ai_agent.py) and_normalize_response_format's own input typeResponseFormatJsonSchemaType | dict[str, Any] | None. Thestrbranch silently passes through_normalize_response_format(it hits thenot isinstance(response_format, dict)guard and returns as-is), so there is no runtime breakage, but the annotation is misleading. This is a pre-existing mismatch that the diff does not worsen, but it would be cleaner to align the annotation with the actual accepted types.
Suggestions
- Add a test that passes a dict
response_formatas a run-level override to_generate_options(or_merge_options), verifying the run-level normalization path. - The
_merge_optionsresponse_formatparameter annotation isResponseFormatJsonSchemaType | None, but callers passstr | ResponseFormatJsonSchemaType | None. Now that_normalize_response_formatformally acceptsdict[str, Any]as well, consider updating the annotation toAgentsApiResponseFormatOption | dict[str, Any] | Noneso the declared type matches what is actually handled.
Automated review by MukundaKatta's agents
There was a problem hiding this comment.
Pull request overview
This PR updates the Azure AI Agent run-option construction path to normalize dict-based structured output response_format configurations into Azure SDK model objects before invoking runs.create/stream, and adds a unit test to cover that normalization.
Changes:
- Normalize dict-based
response_format(specificallytype: "json_schema") intoResponseFormatJsonSchemaTypeinAgentThreadActionsoption merging. - Ensure
response_formatpassed to run creation is always the normalized value from merged agent/run options. - Add a focused unit test asserting
_generate_optionsconverts a dictresponse_formatintoResponseFormatJsonSchemaType.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/semantic_kernel/agents/azure_ai/agent_thread_actions.py | Adds _normalize_response_format and applies it during option merging so Azure SDK receives normalized response_format. |
| python/tests/unit/agents/azure_ai_agent/test_agent_thread_actions.py | Adds a unit test validating dict response_format gets normalized to ResponseFormatJsonSchemaType. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @classmethod | ||
| def _merge_options( | ||
| cls: type[_T], | ||
| *, | ||
| agent: "AzureAIAgent", | ||
| model: str | None = None, | ||
| response_format: ResponseFormatJsonSchemaType | None = None, | ||
| temperature: float | None = None, | ||
| top_p: float | None = None, | ||
| metadata: dict[str, str] | None = None, | ||
| **kwargs: Any, | ||
| ) -> dict[str, Any]: | ||
| """Merge run-time options with the agent-level options. | ||
|
|
||
| Run-level parameters take precedence. | ||
| """ | ||
| normalized_response_format = ( | ||
| cls._normalize_response_format(response_format) | ||
| if response_format is not None | ||
| else cls._normalize_response_format(agent.definition.response_format) | ||
| ) | ||
| return { | ||
| "model": model if model is not None else agent.definition.model, | ||
| "response_format": response_format if response_format is not None else agent.definition.response_format, | ||
| "response_format": normalized_response_format, | ||
| "temperature": temperature if temperature is not None else None, | ||
| "top_p": top_p if top_p is not None else None, | ||
| "metadata": metadata if metadata is not None else agent.definition.metadata, | ||
| **kwargs, | ||
| } | ||
|
|
||
| @classmethod | ||
| def _normalize_response_format( | ||
| cls: type[_T], response_format: ResponseFormatJsonSchemaType | dict[str, Any] | None | ||
| ) -> ResponseFormatJsonSchemaType | dict[str, Any] | None: | ||
| """Normalize structured output response formats for Azure SDK consumers.""" |
There was a problem hiding this comment.
_merge_options/_normalize_response_format currently assume response_format is ResponseFormatJsonSchemaType | dict | None, but AzureAIAgent passes response_format through as str | ResponseFormatJsonSchemaType (see AgentsApiResponseFormatOption), and this method already returns non-dict values unchanged. To keep type hints accurate (and avoid needing type: ignore), consider widening the accepted/returned types here to include str (and ideally align with AzureAIAgent.AgentsApiResponseFormatOption).
| if not isinstance(response_format, dict): | ||
| return response_format | ||
|
|
||
| if response_format.get("type") != "json_schema": | ||
| return response_format | ||
|
|
||
| json_schema = response_format.get("json_schema") | ||
| if not isinstance(json_schema, dict): | ||
| return response_format | ||
|
|
There was a problem hiding this comment.
_normalize_response_format returns the original dict for any dict shapes it doesn't recognize (including {"type": "json_object"} or other unexpected type values). Since AgentThreadActions.invoke/_generate_options feed this value directly into agent.client.agents.runs.create/stream, this can still result in passing raw dicts into the Azure SDK/monitoring pipeline. If the intent is to avoid dicts entirely, normalize additional supported shapes (e.g., map {"type": "json_object"} to the string form the SDK expects) and/or raise a clear error for unsupported dict formats instead of passing them through.
Summary
Testing