mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
.NET: Fix function_call_output.output to be a JSON string on the wire (#5705)
* Fix function_call_output.output to be a JSON string on the wire
OutputConverter was passing the JSON serialization of complex tool results (e.g. List<TodoItem>) directly into OutputItemFunctionToolCallOutput via BinaryData.FromString. The Responses SDK treats that BinaryData as the *raw JSON value* for the field, so non-string results landed on the wire as an unquoted JSON array (e.g. `"output":[{...}]`) instead of a JSON string.
The OpenAI Responses spec requires `function_call_output.output` to be a JSON string. The strict-parsing OpenAI .NET client (FunctionCallOutputResponseItem) consequently failed when threading a follow-up turn that replayed such an item, with: `The JSON value could not be converted... requires an element of type 'String', but the target element has type 'Array'`.
Always wrap the payload as a JSON string literal:
- string s -> JSON-encode s (quoted, with escapes)
- object o -> JSON-serialize o, then JSON-encode the resulting text
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address PR feedback: JsonElement special-case, symmetric inbound unwrap, tests
OutputConverter: extract EncodeFunctionResultAsJsonStringPayload helper
that special-cases JsonElement / JsonDocument so a string-kind element
does not get double-encoded into "\"value\"". Other JsonElement kinds
(object/array/number/bool) round-trip via GetRawText() and are then
JSON-string-wrapped, matching the spec.
InputConverter: symmetric DecodeFunctionResultPayload added to
ConvertFunctionCallOutput and ConvertFunctionToolCallOutput so
previously-stored function_call_output items replayed via
previous_response_id unwrap back to the original tool result text
instead of leaking the JSON-encoded form into FunctionResultContent.Result.
Legacy non-conforming raw-JSON-value payloads pass through unchanged.
Tests:
- Replace ConvertUpdatesToEventsAsync_FunctionResultStringPayload_EmittedAsRawTextAsync
with EmittedAsJsonStringAsync asserting the new wire contract ("sunny" -> "\"sunny\"").
- Add coverage for object payloads, JsonElement string kind (no double-encoding),
and JsonElement array kind (JSON-stringified).
- Add InputConverter round-trip tests for spec-compliant JSON-string payloads
and legacy raw-JSON-array payloads.
All 663 tests pass on net8/net9/net10. Verified end-to-end against the local
hosted-harness sample: T1-T4 (incl. TodoList tool replay across turns) all
succeed with no SDK parse errors.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: alliscode <bentho@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Ben Thomas
committed by
GitHub
Unverified
GitHub
parent
27324a8013
commit
76772ffc19
@@ -190,7 +190,7 @@ internal static class InputConverter
|
||||
|
||||
private static ChatMessage ConvertFunctionCallOutput(FunctionCallOutputItemParam funcOutput)
|
||||
{
|
||||
var output = funcOutput.Output?.ToString() ?? string.Empty;
|
||||
var output = DecodeFunctionResultPayload(funcOutput.Output);
|
||||
return new ChatMessage(
|
||||
ChatRole.Tool,
|
||||
[new FunctionResultContent(funcOutput.CallId, output)]);
|
||||
@@ -482,9 +482,54 @@ internal static class InputConverter
|
||||
|
||||
private static ChatMessage ConvertFunctionToolCallOutput(OutputItemFunctionToolCallOutput funcOutput)
|
||||
{
|
||||
var output = DecodeFunctionResultPayload(funcOutput.Output);
|
||||
return new ChatMessage(
|
||||
ChatRole.Tool,
|
||||
[new FunctionResultContent(funcOutput.CallId, funcOutput.Output)]);
|
||||
[new FunctionResultContent(funcOutput.CallId, output)]);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Decodes the wire payload of a <c>function_call_output.output</c> field back into the
|
||||
/// underlying tool-result text suitable for replay as <see cref="FunctionResultContent.Result"/>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Mirrors <c>OutputConverter.EncodeFunctionResultAsJsonStringPayload</c>. Per the OpenAI
|
||||
/// Responses spec, <c>output</c> is a JSON string; we extract its underlying value. Legacy
|
||||
/// producers that emitted raw JSON values (arrays/objects) are tolerated by passing the raw
|
||||
/// bytes through unchanged.
|
||||
/// </remarks>
|
||||
private static string DecodeFunctionResultPayload(BinaryData? rawOutput)
|
||||
{
|
||||
if (rawOutput is null)
|
||||
{
|
||||
return string.Empty;
|
||||
}
|
||||
|
||||
var raw = rawOutput.ToString();
|
||||
if (string.IsNullOrEmpty(raw))
|
||||
{
|
||||
return string.Empty;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
using var doc = JsonDocument.Parse(raw);
|
||||
if (doc.RootElement.ValueKind == JsonValueKind.String)
|
||||
{
|
||||
return doc.RootElement.GetString() ?? string.Empty;
|
||||
}
|
||||
|
||||
// Legacy/non-conforming producers may have emitted a raw JSON value
|
||||
// (array/object/number/bool/null). Pass the raw text through as the
|
||||
// payload so the replayed FunctionResultContent.Result preserves the
|
||||
// original tool output shape.
|
||||
return raw;
|
||||
}
|
||||
catch (JsonException)
|
||||
{
|
||||
// Not valid JSON — treat the bytes as a literal string payload.
|
||||
return raw;
|
||||
}
|
||||
}
|
||||
|
||||
private static ChatRole ConvertMessageRole(MessageRole role)
|
||||
|
||||
@@ -279,12 +279,7 @@ internal static class OutputConverter
|
||||
accumulatedText = null;
|
||||
previousMessageId = null;
|
||||
|
||||
var outputText = functionResult.Result switch
|
||||
{
|
||||
null => string.Empty,
|
||||
string s => s,
|
||||
_ => JsonSerializer.Serialize(functionResult.Result),
|
||||
};
|
||||
var outputText = EncodeFunctionResultAsJsonStringPayload(functionResult.Result);
|
||||
|
||||
var itemId = GenerateItemId("fc");
|
||||
var outputItem = new OutputItemFunctionToolCallOutput(
|
||||
@@ -448,4 +443,44 @@ internal static class OutputConverter
|
||||
var body = Convert.ToHexString(bytes); // 50 hex chars, uppercase
|
||||
return $"{prefix}_{body}";
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Encodes a <see cref="FunctionResultContent.Result"/> value into the wire payload for
|
||||
/// the OpenAI Responses <c>function_call_output.output</c> field.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The OpenAI Responses spec requires <c>output</c> to be a JSON string. The Responses
|
||||
/// SDK's <see cref="OutputItemFunctionToolCallOutput"/> accepts a <see cref="BinaryData"/>
|
||||
/// containing the *raw JSON value* for the field, so the returned text is always a JSON
|
||||
/// string literal (quoted, with escapes). This avoids two bugs:
|
||||
/// <list type="bullet">
|
||||
/// <item>Complex results (e.g. <c>List<TodoItem></c>) landing on the wire as an
|
||||
/// unquoted JSON array, which the strict-parsing OpenAI .NET client
|
||||
/// (<c>FunctionCallOutputResponseItem</c>) rejects with
|
||||
/// "requires an element of type 'String', but the target element has type 'Array'".</item>
|
||||
/// <item>Numeric- or JSON-shaped string results (e.g. <c>"42"</c> or <c>"{\"k\":1}"</c>)
|
||||
/// silently changing type on the wire because <c>BinaryData</c> auto-detects JSON.</item>
|
||||
/// </list>
|
||||
/// <see cref="JsonElement"/> / <see cref="JsonDocument"/> values are unwrapped first so
|
||||
/// a string-kind element does not get double-encoded into <c>"\"value\""</c>.
|
||||
/// </remarks>
|
||||
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "Serializing function call result payload.")]
|
||||
[UnconditionalSuppressMessage("AOT", "IL3050", Justification = "Serializing function call result payload.")]
|
||||
private static string EncodeFunctionResultAsJsonStringPayload(object? result)
|
||||
{
|
||||
string innerText = result switch
|
||||
{
|
||||
null => string.Empty,
|
||||
string s => s,
|
||||
JsonElement je => je.ValueKind == JsonValueKind.String
|
||||
? (je.GetString() ?? string.Empty)
|
||||
: je.GetRawText(),
|
||||
JsonDocument jd => jd.RootElement.ValueKind == JsonValueKind.String
|
||||
? (jd.RootElement.GetString() ?? string.Empty)
|
||||
: jd.RootElement.GetRawText(),
|
||||
_ => JsonSerializer.Serialize(result),
|
||||
};
|
||||
|
||||
return JsonSerializer.Serialize(innerText);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -207,9 +207,10 @@ public class InputConverterTests
|
||||
[Fact]
|
||||
public void ConvertOutputItemsToMessages_FunctionToolCallOutput_ReturnsToolMessage()
|
||||
{
|
||||
// Spec-compliant payload: a JSON string literal.
|
||||
var funcOutput = new OutputItemFunctionToolCallOutput(
|
||||
callId: "call_def",
|
||||
output: BinaryData.FromString("result data"));
|
||||
output: BinaryData.FromString("\"result data\""));
|
||||
|
||||
var messages = InputConverter.ConvertOutputItemsToMessages([funcOutput]);
|
||||
|
||||
@@ -218,6 +219,52 @@ public class InputConverterTests
|
||||
var result = messages[0].Contents.OfType<FunctionResultContent>().FirstOrDefault();
|
||||
Assert.NotNull(result);
|
||||
Assert.Equal("call_def", result.CallId);
|
||||
// Round-trip: the JSON-string wire payload is unwrapped to the original tool result text.
|
||||
Assert.Equal("result data", result.Result as string);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ConvertOutputItemsToMessages_FunctionToolCallOutput_LegacyRawJsonArray_PassesThrough()
|
||||
{
|
||||
// Legacy/non-conforming producers that emitted a raw JSON value (array/object) in
|
||||
// `output` are tolerated: the raw text is forwarded as the FunctionResultContent.Result
|
||||
// so the model still sees the original tool-output shape on replay.
|
||||
var funcOutput = new OutputItemFunctionToolCallOutput(
|
||||
callId: "call_legacy",
|
||||
output: BinaryData.FromString("[{\"id\":1}]"));
|
||||
|
||||
var messages = InputConverter.ConvertOutputItemsToMessages([funcOutput]);
|
||||
|
||||
var result = messages[0].Contents.OfType<FunctionResultContent>().FirstOrDefault();
|
||||
Assert.NotNull(result);
|
||||
Assert.Equal("[{\"id\":1}]", result.Result as string);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ConvertInputToMessages_FunctionCallOutput_JsonStringPayload_Unwraps()
|
||||
{
|
||||
// Spec-compliant inbound payload — a JSON string literal — must be unwrapped so
|
||||
// FunctionResultContent.Result is the original tool result text, not the JSON-encoded form.
|
||||
var input = new[]
|
||||
{
|
||||
new
|
||||
{
|
||||
type = "function_call_output",
|
||||
id = "fc_out_002",
|
||||
call_id = "call_456",
|
||||
output = "sunny"
|
||||
}
|
||||
};
|
||||
|
||||
var request = new CreateResponse();
|
||||
request.Input = BinaryData.FromObjectAsJson(input);
|
||||
|
||||
var messages = InputConverter.ConvertInputToMessages(request);
|
||||
|
||||
Assert.Single(messages);
|
||||
var funcResult = messages[0].Contents.OfType<FunctionResultContent>().FirstOrDefault();
|
||||
Assert.NotNull(funcResult);
|
||||
Assert.Equal("sunny", funcResult.Result as string);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
||||
@@ -616,9 +616,10 @@ public class OutputConverterTests
|
||||
Assert.IsType<ResponseCompletedEvent>(events[^1]);
|
||||
}
|
||||
|
||||
// K-06: FRC string results are emitted as raw text on the wire (not JSON-quoted).
|
||||
// K-06: FRC payloads are wrapped as JSON string literals on the wire so the field is
|
||||
// always a spec-compliant OpenAI Responses `function_call_output.output` string value.
|
||||
[Fact]
|
||||
public async Task ConvertUpdatesToEventsAsync_FunctionResultStringPayload_EmittedAsRawTextAsync()
|
||||
public async Task ConvertUpdatesToEventsAsync_FunctionResultStringPayload_EmittedAsJsonStringAsync()
|
||||
{
|
||||
var (stream, _) = CreateTestStream();
|
||||
var update = new AgentResponseUpdate { Contents = [new FunctionResultContent("call_1", "sunny")] };
|
||||
@@ -631,8 +632,76 @@ public class OutputConverterTests
|
||||
|
||||
var added = Assert.Single(events.OfType<ResponseOutputItemAddedEvent>());
|
||||
var output = Assert.IsType<OutputItemFunctionToolCallOutput>(added.Item);
|
||||
// String FRC payloads must not be double-encoded — `sunny`, not `"sunny"`.
|
||||
Assert.Equal("sunny", output.Output.ToString());
|
||||
// The wire payload is a JSON string literal — `"sunny"`, not the bare bytes `sunny`.
|
||||
Assert.Equal("\"sunny\"", output.Output.ToString());
|
||||
}
|
||||
|
||||
// K-06b: List/object FRC payloads must be JSON-stringified into a JSON string value
|
||||
// so the OpenAI .NET client (FunctionCallOutputResponseItem.Output: string) can parse them.
|
||||
[Fact]
|
||||
public async Task ConvertUpdatesToEventsAsync_FunctionResultObjectPayload_EmittedAsJsonStringAsync()
|
||||
{
|
||||
var (stream, _) = CreateTestStream();
|
||||
var todoList = new[] { new { id = 1, text = "Buy milk" } };
|
||||
var update = new AgentResponseUpdate { Contents = [new FunctionResultContent("call_1", todoList)] };
|
||||
|
||||
var events = new List<ResponseStreamEvent>();
|
||||
await foreach (var evt in OutputConverter.ConvertUpdatesToEventsAsync(ToAsync(new[] { update }), stream))
|
||||
{
|
||||
events.Add(evt);
|
||||
}
|
||||
|
||||
var added = Assert.Single(events.OfType<ResponseOutputItemAddedEvent>());
|
||||
var output = Assert.IsType<OutputItemFunctionToolCallOutput>(added.Item);
|
||||
// The wire payload must be a quoted JSON string containing the JSON-serialized object.
|
||||
var raw = output.Output.ToString();
|
||||
Assert.StartsWith("\"", raw);
|
||||
Assert.EndsWith("\"", raw);
|
||||
// The unwrapped value must round-trip back to the original JSON.
|
||||
var inner = System.Text.Json.JsonSerializer.Deserialize<string>(raw);
|
||||
Assert.Equal("[{\"id\":1,\"text\":\"Buy milk\"}]", inner);
|
||||
}
|
||||
|
||||
// K-06c: A JsonElement of kind String must not be double-encoded.
|
||||
[Fact]
|
||||
public async Task ConvertUpdatesToEventsAsync_FunctionResultJsonElementStringPayload_NotDoubleEncodedAsync()
|
||||
{
|
||||
var (stream, _) = CreateTestStream();
|
||||
using var doc = System.Text.Json.JsonDocument.Parse("\"sunny\"");
|
||||
var update = new AgentResponseUpdate { Contents = [new FunctionResultContent("call_1", doc.RootElement.Clone())] };
|
||||
|
||||
var events = new List<ResponseStreamEvent>();
|
||||
await foreach (var evt in OutputConverter.ConvertUpdatesToEventsAsync(ToAsync(new[] { update }), stream))
|
||||
{
|
||||
events.Add(evt);
|
||||
}
|
||||
|
||||
var added = Assert.Single(events.OfType<ResponseOutputItemAddedEvent>());
|
||||
var output = Assert.IsType<OutputItemFunctionToolCallOutput>(added.Item);
|
||||
// Must be `"sunny"`, not `"\"sunny\""`.
|
||||
Assert.Equal("\"sunny\"", output.Output.ToString());
|
||||
}
|
||||
|
||||
// K-06d: A JsonElement of non-string kind (e.g. array) must be JSON-stringified, not
|
||||
// emitted as a raw JSON array on the wire.
|
||||
[Fact]
|
||||
public async Task ConvertUpdatesToEventsAsync_FunctionResultJsonElementArrayPayload_EmittedAsJsonStringAsync()
|
||||
{
|
||||
var (stream, _) = CreateTestStream();
|
||||
using var doc = System.Text.Json.JsonDocument.Parse("[{\"id\":1}]");
|
||||
var update = new AgentResponseUpdate { Contents = [new FunctionResultContent("call_1", doc.RootElement.Clone())] };
|
||||
|
||||
var events = new List<ResponseStreamEvent>();
|
||||
await foreach (var evt in OutputConverter.ConvertUpdatesToEventsAsync(ToAsync(new[] { update }), stream))
|
||||
{
|
||||
events.Add(evt);
|
||||
}
|
||||
|
||||
var added = Assert.Single(events.OfType<ResponseOutputItemAddedEvent>());
|
||||
var output = Assert.IsType<OutputItemFunctionToolCallOutput>(added.Item);
|
||||
var raw = output.Output.ToString();
|
||||
var inner = System.Text.Json.JsonSerializer.Deserialize<string>(raw);
|
||||
Assert.Equal("[{\"id\":1}]", inner);
|
||||
}
|
||||
|
||||
// L-01
|
||||
|
||||
Reference in New Issue
Block a user