Python: fix(python): normalize Azure AI agent response_format#13891
Python: fix(python): normalize Azure AI agent response_format#13891MukundaKatta wants to merge 3 commits intomicrosoft:mainfrom
Conversation
github-actions
Bot
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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.
…xt dict shapes
Per @Copilot review:
- widen _merge_options and _normalize_response_format to accept str | ResponseFormatJsonSchemaType | dict | None, matching AzureAIAgent's AgentsApiResponseFormatOption
- map {'type': 'json_object'} and {'type': 'text'} dict shapes to their canonical string form so the Azure SDK sees the expected type
- narrow return type to str | ResponseFormatJsonSchemaType | None (dict is only passed through as an unrecognized escape hatch, never intentionally)
Avoids importing AgentsApiResponseFormatOption directly (would be circular — azure_ai_agent imports from this module). The inline union keeps type checkers satisfied without duplicating the alias definition.
|
Addressed @copilot's review in 7ad6447: Type widening: Unknown dict shapes: Return type narrowed to |
moonbox3
left a comment
moonbox3
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 82%
✓ Correctness
The PR correctly widens
_merge_options'sresponse_formatparameter and introduces_normalize_response_formatto convert dict-shaped response formats into the Azure SDK types before they reachruns.create/stream. The import ofResponseFormatJsonSchemafromazure.ai.agents.modelsis consistent with existing usage in the samples codebase. The normalization logic is sound for the cases it handles:None,ResponseFormatJsonSchemaType, bare strings, and thejson_object/text/json_schemadict variants. The new test covers the primary agent-level dict normalization path. The return type annotation on_normalize_response_formatstill omitsdict[str, Any]for the two fallthrough branches (rf_type != 'json_schema'andjson_schemanot being a dict), but that concern is already captured in the existing unresolved comment at line 937 and is not a new regression. No new blocking correctness issues were found.
✓ Security Reliability
The diff widens
_merge_options/_normalize_response_formatto acceptstr | ResponseFormatJsonSchemaType | dict[str, Any] | None, addressing the first unresolved thread. It maps{"type": "json_object"}and{"type": "text"}dict shapes to their string equivalents and constructs a typedResponseFormatJsonSchemaTypeforjson_schemadicts, which directly aligns withAgentsApiResponseFormatOption = str | ResponseFormatJsonSchemaType(azure_ai_agent.py:79). No injection risks, resource leaks, or credential exposure were found. The second unresolved thread (raw dict passthrough for unrecognized/malformedtypevalues) is partially mitigated but not fully resolved — unknowntypevalues andjson_schemadicts with a non-dictjson_schemafield still fall through as raw dicts. One reliability concern exists:json_schema.get("name")can beNoneif the caller omitsname, which may fail during SDK serialization sincenameappears to be a required field inResponseFormatJsonSchema(per the sample at azure_ai_agent_structured_outputs.py:49-53), but SDK enforcement could not be confirmed without the installed package source.
✗ Test Coverage
The PR introduces
_normalize_response_formatwith 7 distinct branches, but the single new test only exercises branch 7 (dict withtype == "json_schema"→ResponseFormatJsonSchemaType). The explicitly documented transformations for{"type": "json_object"}and{"type": "text"}(which map a dict to the canonical string the Azure SDK expects) are entirely untested, as are the passthrough branches forResponseFormatJsonSchemaType, bare strings, unknown dict types, and a malformedjson_schemavalue. Additionally, the test exercises normalization only viaagent.definition.response_format; the run-levelresponse_formatargument path through_merge_optionshas no coverage.
✓ Design Approach
No additional design-approach issues found in this diff beyond the already-open response_format validation discussion. Normalizing
response_formatinAgentThreadActionsis a reasonable boundary fix here because that is the only local place whereagent.definition.response_formatis consumed beforeruns.create/stream, and the added test covers the dict-backedjson_schemacase.
Suggestions
- Add a direct unit test for
_normalize_response_formatcovering each branch:None, aResponseFormatJsonSchemaTypeinstance, a bare string,{"type": "json_object"},{"type": "text"}, an unknown dict type, a malformedjson_schemavalue (non-dict inner field), and thejson_schemahappy-path already covered. - Add a test where a run-level
response_formatdict is passed directly to_generate_options/_merge_options(not only viaagent.definition.response_format) to verify the run-level normalization path (diff line ~905–908).
Automated review by moonbox3's agents
Summary
Testing