Python: fix: coerce dict response_format to typed Azure AI model to fix Azure Monitor compatibility#13912
Conversation
…n result logging (fixes microsoft#13681) When a KernelFunction returns a type not registered in AbstractionsJsonContext (e.g. Microsoft.Extensions.AI.TextContent from MCP tools), and JSON serialization is attempted with AOT-safe JsonSerializerOptions, a NotSupportedException is thrown and swallowed, causing the log message to show "Failed to log function result value" instead of useful content. This change adds a ToString() fallback in the NotSupportedException catch block so that result values still produce meaningful log output even when their runtime type is absent from the source-generated JSON context.
…ix Azure Monitor compatibility
When response_format is passed as a plain Python dict (e.g. {"type":
"json_object"}), the Azure AI telemetry instrumentor raises
ValueError: Unknown response format <class 'dict'> because it only
handles the typed SDK models (AgentsResponseFormat,
ResponseFormatJsonSchemaType, AgentsResponseFormatMode, str).
Add AgentThreadActions._coerce_response_format() to convert plain
dicts to the appropriate typed model before the options dict is
forwarded to agents.runs.create(), preventing the telemetry error
when Azure Monitor is enabled.
Fixes microsoft#13715
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 90%
✓ Correctness
This PR adds two independent improvements: (1) a C# fallback to ToString() when JSON serialization fails with NotSupportedException in KernelFunctionLogMessages, and (2) a Python _coerce_response_format method that converts plain dict response_format values to typed Azure SDK models to avoid telemetry errors. Both changes are correct. The C# catch chain properly captures the NotSupportedException and falls back gracefully. The Python coerce method correctly handles all response_format variants — I verified that AgentsResponseFormat(dict) and ResponseFormatJsonSchemaType(dict) constructors work correctly with the installed Azure AI agents SDK (1.2.0b6). The isinstance checks in the passthrough path are also valid. Tests are comprehensive and cover passthrough, conversion, and unknown-type scenarios.
✓ Security Reliability
This PR makes two defensive improvements: (1) a C# logging fallback that uses ToString() when JSON serialization throws NotSupportedException (e.g., in AOT scenarios with unregistered types), and (2) a Python coercion helper that converts plain-dict response_format values into typed Azure AI SDK models to avoid ValueError in telemetry instrumentation. Both changes are well-scoped, maintain existing error-handling patterns, and include good test coverage. No security or reliability issues were identified.
✓ Test Coverage
The PR adds a ToString() fallback in the .NET KernelFunctionLogMessages when JSON serialization throws NotSupportedException, and adds a _coerce_response_format helper in the Python Azure AI agent thread actions to convert plain dicts to typed SDK models. Both changes include new tests. The .NET test covers the happy-path ToString() fallback but not the inner catch block where ToString() itself throws. The Python tests thoroughly cover _coerce_response_format in isolation but don't verify the integration point in _generate_options. Overall, test quality is good with meaningful assertions, but one untested code path in the .NET change is worth noting.
✗ Design Approach
I found one blocking design issue in the .NET change: the new serialization fallback bypasses
FunctionResult's own string-conversion abstraction and reaches directly intoValue.ToString(), which can produce different output than the rest of Semantic Kernel for culture-sensitive or custom-converted values. The Python change looks directionally reasonable for the telemetry problem, but it currently widens acceptedresponse_formatshapes only inside an internal helper while the Azure AI public API still advertises a narrower contract.
Suggestions
- [dotnet] The inner catch block (lines 171-174 in the diff) that handles the case where ToString() itself throws is not covered by any test. Consider adding a test with a type whose ToString() throws, verifying the log message falls back to "Failed to log function result value" with the original NotSupportedException.
- [python] Consider moving the response_format normalization to the Azure AI public API boundary as well.
AgentThreadActionsnow accepts/coerces plain dicts internally, but the exposed contract is stillAgentsApiResponseFormatOption = str | ResponseFormatJsonSchemaType, so the widened accepted shapes remain undocumented and unvalidated until the internal helper call.
Automated review by octo-patch's agents
|
|
||
| [JsonSerializable(typeof(IDictionary<string, object?>))] | ||
| [JsonSerializable(typeof(object))] | ||
| private sealed partial class RestrictedJsonContext : JsonSerializerContext { } |
There was a problem hiding this comment.
Test coverage gap: there is no test for the inner catch block in LogFunctionResultValueInternal (lines 171-174 of KernelFunctionLogMessages.cs). When ToString() itself throws, the code should fall back to logging "Failed to log function result value" with the original NotSupportedException. Consider adding a test with a type whose ToString() throws, using the same RestrictedJsonContext, and asserting that the error message and exception are logged.
| private sealed partial class RestrictedJsonContext : JsonSerializerContext { } | |
| [Fact] | |
| public void ItShouldLogFailureMessageWhenToStringAlsoFails() | |
| { | |
| // Arrange | |
| var logger = new Mock<ILogger>(); | |
| logger.Setup(l => l.IsEnabled(It.IsAny<LogLevel>()).Returns(true); | |
| var throwingValue = new TypeWhoseToStringThrows(); | |
| var functionResult = new FunctionResult(KernelFunctionFactory.CreateFromMethod(() => { }), throwingValue); | |
| var restrictedOptions = RestrictedJsonContext.Default.Options; | |
| // Act | |
| logger.Object.LogFunctionResultValue("p1", "f1", functionResult, restrictedOptions); | |
| // Assert - should fall back to the error message | |
| logger.Verify(l => l.Log( | |
| LogLevel.Trace, | |
| 0, | |
| It.Is<It.IsAnyType>((o, _) => o.ToString() == "Function p1-f1 result: Failed to log function result value"), | |
| It.IsAny<NotSupportedException>(), | |
| It.IsAny<Func<It.IsAnyType, Exception?, string>>())); | |
| } | |
| private sealed class TypeWhoseToStringThrows | |
| { | |
| public override string ToString() => throw new InvalidOperationException("ToString failed"); | |
| } | |
| [JsonSerializable(typeof(IDictionary<string, object?>))] | |
| [JsonSerializable(typeof(object))] | |
| private sealed partial class RestrictedJsonContext : JsonSerializerContext { } |
Fixes #13715
Problem
When
AzureAIAgentis used insideAgentGroupChatwith Azure Monitor (configure_azure_monitor) enabled, all agent executions fail with:The Azure AI telemetry instrumentor (
_AIAgentsInstrumentorPreview.agent_api_response_to_str) only handlesstr,AgentsResponseFormatMode,AgentsResponseFormat, andResponseFormatJsonSchemaType. Whenresponse_formatis a plain Pythondict(e.g.{"type": "json_object"}), none of the isinstance checks match and the instrumentor raisesValueError.SK's
AgentThreadActions._generate_optionspassesresponse_formatas-is from the merged options dict, which can be a plaindictwhen users supply it that way or whenagent.definition.response_formatreturns a raw mapping.Solution
Add a
_coerce_response_format()static method toAgentThreadActionsthat converts a plaindictto the appropriate typed Azure AI SDK model before it is forwarded toagents.runs.create():{"type": "json_schema", ...}→ResponseFormatJsonSchemaTypedict→AgentsResponseFormatstr,AgentsResponseFormatMode,AgentsResponseFormat,ResponseFormatJsonSchemaType,None) pass through unchanged._generate_optionsnow calls_coerce_response_formaton the mergedresponse_formatvalue, ensuring the telemetry instrumentor always receives a supported type.Testing
test_agent_thread_actions.pycovering:None,str,AgentsResponseFormatMode,AgentsResponseFormat,ResponseFormatJsonSchemaType)"type": "json_object"→AgentsResponseFormat"type": "json_schema"→ResponseFormatJsonSchemaTypetest_agent_thread_actions.pycontinue to pass.