Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,17 @@ private static void LogFunctionResultValueInternal(this ILogger logger, string?
}
catch (NotSupportedException ex)
{
s_logFunctionResultValue(logger, pluginName, functionName, "Failed to log function result value", ex);
// Fall back to ToString() when JSON serialization isn't supported for this type
// (e.g. Microsoft.Extensions.AI.TextContent is not registered in AbstractionsJsonContext)
try
{
var toStringValue = resultValue?.Value?.ToString() ?? string.Empty;
s_logFunctionResultValue(logger, pluginName, functionName, toStringValue, null);
}
catch
{
s_logFunctionResultValue(logger, pluginName, functionName, "Failed to log function result value", ex);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft. All rights reserved.

using System;
using System.Collections.Generic;
using System.Text.Json.Serialization;
using Microsoft.Extensions.Logging;
using Microsoft.SemanticKernel;
Expand Down Expand Up @@ -48,9 +49,46 @@ public void ItShouldLogFunctionResultOfAnyType(Type resultType)
It.IsAny<Func<It.IsAnyType, Exception?, string>>()));
}

[Fact]
public void ItShouldFallBackToToStringWhenJsonSerializationIsNotSupported()
{
// Arrange
var logger = new Mock<ILogger>();
logger.Setup(l => l.IsEnabled(It.IsAny<LogLevel>())).Returns(true);

// TypeNotInJsonContext cannot be cast to string and is not registered in the restricted JSON context
var unserializableValue = new TypeNotInJsonContext();
var functionResult = new FunctionResult(KernelFunctionFactory.CreateFromMethod(() => { }), unserializableValue);

// Use a restricted JsonSerializerOptions that knows about object but not TypeNotInJsonContext,
// simulating the AOT scenario where AbstractionsJsonContext is used and an unregistered
// MEAItype (e.g. Microsoft.Extensions.AI.TextContent) is returned from an MCP tool.
var restrictedOptions = RestrictedJsonContext.Default.Options;

// Act
logger.Object.LogFunctionResultValue("p1", "f1", functionResult, restrictedOptions);

// Assert - ToString() fallback should have been used, not the error message
logger.Verify(l => l.Log(
LogLevel.Trace,
0,
It.Is<It.IsAnyType>((o, _) => o.ToString() == "Function p1-f1 result: TypeNotInJsonContext()"),
null,
It.IsAny<Func<It.IsAnyType, Exception?, string>>()));
}

private sealed class User
{
[JsonPropertyName("name")]
public string? Name { get; set; }
}

private sealed class TypeNotInJsonContext
{
public override string ToString() => "TypeNotInJsonContext()";
}

[JsonSerializable(typeof(IDictionary<string, object?>))]
[JsonSerializable(typeof(object))]
private sealed partial class RestrictedJsonContext : JsonSerializerContext { }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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 { }

}
26 changes: 25 additions & 1 deletion python/semantic_kernel/agents/azure_ai/agent_thread_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
from azure.ai.agents.models import (
AgentsNamedToolChoiceType,
AgentStreamEvent,
AgentsResponseFormat,
AgentsResponseFormatMode,
AsyncAgentEventHandler,
AsyncAgentRunStream,
BaseAsyncAgentEventHandler,
Expand Down Expand Up @@ -923,7 +925,7 @@ def _generate_options(cls: type[_T], **kwargs: Any) -> dict[str, Any]:
return {
"model": merged.get("model"),
"top_p": merged.get("top_p"),
"response_format": merged.get("response_format"),
"response_format": cls._coerce_response_format(merged.get("response_format")),
"temperature": merged.get("temperature"),
"truncation_strategy": truncation_strategy,
"metadata": merged.get("metadata"),
Expand All @@ -933,6 +935,28 @@ def _generate_options(cls: type[_T], **kwargs: Any) -> dict[str, Any]:
"additional_messages": additional_messages,
}

@staticmethod
def _coerce_response_format(
response_format: Any,
) -> "str | AgentsResponseFormatMode | AgentsResponseFormat | ResponseFormatJsonSchemaType | None":
"""Coerce a plain dict response_format to the appropriate Azure AI typed model.

When users supply ``response_format`` as a plain Python :class:`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. This method converts plain dicts to the correct type so
that the Azure AI telemetry layer can serialize them without error.
"""
if response_format is None or isinstance(
response_format, (str, AgentsResponseFormatMode, AgentsResponseFormat, ResponseFormatJsonSchemaType)
):
return response_format
if isinstance(response_format, dict):
if response_format.get("type") == "json_schema":
return ResponseFormatJsonSchemaType(response_format)
return AgentsResponseFormat(response_format)
return response_format

@classmethod
def _translate_additional_messages(
cls: type[_T], messages: "list[ChatMessageContent] | None"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@
from datetime import datetime, timezone
from unittest.mock import AsyncMock, MagicMock, patch

import pytest
from azure.ai.agents.models import (
AgentsResponseFormat,
AgentsResponseFormatMode,
MessageTextContent,
MessageTextDetails,
RequiredFunctionToolCall,
RequiredFunctionToolCallDetails,
ResponseFormatJsonSchemaType,
RunStep,
RunStepCodeInterpreterToolCall,
RunStepCodeInterpreterToolCallDetails,
Expand Down Expand Up @@ -354,3 +358,52 @@ async def test_agent_thread_actions_invoke_stream(ai_project_client, ai_agent_de
collected_messages.append(content)
assert isinstance(content, ChatMessageContent)
assert content.metadata.get("message_id") == "msg_1"


# region _coerce_response_format tests


@pytest.mark.parametrize(
"input_value, expected_type",
[
(None, type(None)),
("auto", str),
(AgentsResponseFormatMode.AUTO, AgentsResponseFormatMode),
(AgentsResponseFormat({"type": "json_object"}), AgentsResponseFormat),
(
ResponseFormatJsonSchemaType({"type": "json_schema", "json_schema": {"name": "t", "schema": {}}}),
ResponseFormatJsonSchemaType,
),
],
)
def test_coerce_response_format_passthrough(input_value, expected_type):
"""Values that are already the correct type should be returned unchanged."""
result = AgentThreadActions._coerce_response_format(input_value)
assert isinstance(result, expected_type) or result is None


def test_coerce_response_format_dict_json_object():
"""A plain dict with type 'json_object' should become AgentsResponseFormat."""
result = AgentThreadActions._coerce_response_format({"type": "json_object"})
assert isinstance(result, AgentsResponseFormat)


def test_coerce_response_format_dict_json_schema():
"""A plain dict with type 'json_schema' should become ResponseFormatJsonSchemaType."""
payload = {"type": "json_schema", "json_schema": {"name": "MySchema", "strict": True, "schema": {}}}
result = AgentThreadActions._coerce_response_format(payload)
assert isinstance(result, ResponseFormatJsonSchemaType)


def test_coerce_response_format_unknown_type_passthrough():
"""An unknown (non-dict) type should be returned as-is to let the SDK handle it."""

class Custom:
pass

custom = Custom()
result = AgentThreadActions._coerce_response_format(custom)
assert result is custom


# endregion
Loading