Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address PR feedback from #5513 #5533

Merged
merged 1 commit into from
Oct 17, 2024
Merged
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 @@ -64,14 +64,12 @@ public FunctionCallContent(string callId, string name, IDictionary<string, objec
/// <param name="callId">The function call ID.</param>
/// <param name="name">The function name.</param>
/// <param name="argumentParser">The parsing implementation converting the encoding to a dictionary of arguments.</param>
/// <param name="exceptionFilter">Filters potential parsing exceptions that should be caught and included in the result.</param>
/// <returns>A new instance of <see cref="FunctionCallContent"/> containing the parse result.</returns>
public static FunctionCallContent CreateFromParsedArguments<TEncoding>(
TEncoding encodedArguments,
string callId,
string name,
Func<TEncoding, IDictionary<string, object?>?> argumentParser,
Func<Exception, bool>? exceptionFilter = null)
Func<TEncoding, IDictionary<string, object?>?> argumentParser)
{
_ = Throw.IfNull(callId);
_ = Throw.IfNull(name);
Expand All @@ -81,14 +79,16 @@ public static FunctionCallContent CreateFromParsedArguments<TEncoding>(
IDictionary<string, object?>? arguments = null;
Exception? parsingException = null;

#pragma warning disable CA1031 // Do not catch general exception types
try
{
arguments = argumentParser(encodedArguments);
}
catch (Exception ex) when (exceptionFilter is null || exceptionFilter(ex))
catch (Exception ex)
{
parsingException = new InvalidOperationException("Error parsing function call arguments.", ex);
}
#pragma warning restore CA1031 // Do not catch general exception types

return new FunctionCallContent(callId, name, arguments)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,7 @@ private IEnumerable<ChatRequestMessage> ToAzureAIInferenceChatMessages(IEnumerab

private static FunctionCallContent ParseCallContentFromJsonString(string json, string callId, string name) =>
FunctionCallContent.CreateFromParsedArguments(json, callId, name,
argumentParser: static json => JsonSerializer.Deserialize(json, JsonContext.Default.IDictionaryStringObject),
exceptionFilter: static ex => ex is JsonException);
argumentParser: static json => JsonSerializer.Deserialize(json, JsonContext.Default.IDictionaryStringObject));

/// <summary>Source-generated JSON type information.</summary>
[JsonSerializable(typeof(AzureAIChatToolJson))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -655,13 +655,11 @@ private sealed class OpenAIChatToolJson

private static FunctionCallContent ParseCallContentFromJsonString(string json, string callId, string name) =>
FunctionCallContent.CreateFromParsedArguments(json, callId, name,
argumentParser: static json => JsonSerializer.Deserialize(json, JsonContext.Default.IDictionaryStringObject),
exceptionFilter: static ex => ex is JsonException);
argumentParser: static json => JsonSerializer.Deserialize(json, JsonContext.Default.IDictionaryStringObject));

private static FunctionCallContent ParseCallContentFromBinaryData(BinaryData ut8Json, string callId, string name) =>
FunctionCallContent.CreateFromParsedArguments(ut8Json, callId, name,
argumentParser: static json => JsonSerializer.Deserialize(json, JsonContext.Default.IDictionaryStringObject),
exceptionFilter: static ex => ex is JsonException);
argumentParser: static json => JsonSerializer.Deserialize(json, JsonContext.Default.IDictionaryStringObject));

/// <summary>Source-generated JSON type information.</summary>
[JsonSerializable(typeof(OpenAIChatToolJson))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public static partial class AIJsonUtilities
/// <param name="serializerOptions">The options used to extract the schema from the specified type.</param>
/// <param name="inferenceOptions">The options controlling schema inference.</param>
/// <returns>A JSON schema document encoded as a <see cref="JsonElement"/>.</returns>
public static JsonElement ResolveParameterSchema(
public static JsonElement ResolveParameterJsonSchema(
AIFunctionParameterMetadata parameterMetadata,
AIFunctionMetadata functionMetadata,
JsonSerializerOptions? serializerOptions = null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public static void CreateJsonSchema_OverriddenParameters_GeneratesExpectedJsonSc
}

[Fact]
public static void ResolveJsonSchema_ReturnsExpectedValue()
public static void ResolveParameterJsonSchema_ReturnsExpectedValue()
{
JsonSerializerOptions options = new(JsonSerializerOptions.Default);
AIFunction func = AIFunctionFactory.Create((int x, int y) => x + y, serializerOptions: options);
Expand All @@ -125,11 +125,11 @@ public static void ResolveJsonSchema_ReturnsExpectedValue()
JsonElement generatedSchema = Assert.IsType<JsonElement>(param.Schema);

JsonElement resolvedSchema;
resolvedSchema = AIJsonUtilities.ResolveParameterSchema(param, metadata, options);
resolvedSchema = AIJsonUtilities.ResolveParameterJsonSchema(param, metadata, options);
Assert.True(JsonElement.DeepEquals(generatedSchema, resolvedSchema));

options = new(options) { NumberHandling = JsonNumberHandling.AllowReadingFromString };
resolvedSchema = AIJsonUtilities.ResolveParameterSchema(param, metadata, options);
resolvedSchema = AIJsonUtilities.ResolveParameterJsonSchema(param, metadata, options);
Assert.False(JsonElement.DeepEquals(generatedSchema, resolvedSchema));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,30 +316,22 @@ public static void CreateFromParsedArguments_ObjectJsonInput_ReturnsElementArgum
});
}

[Fact]
public static void CreateFromParsedArguments_ParseException_HasExpectedHandling()
[Theory]
[InlineData(typeof(JsonException))]
[InlineData(typeof(InvalidOperationException))]
[InlineData(typeof(NotSupportedException))]
public static void CreateFromParsedArguments_ParseException_HasExpectedHandling(Type exceptionType)
{
FunctionCallContent content;
JsonException exc = new();
Exception exc = (Exception)Activator.CreateInstance(exceptionType)!;
FunctionCallContent content = FunctionCallContent.CreateFromParsedArguments(exc, "callId", "functionName", ThrowingParser);

content = FunctionCallContent.CreateFromParsedArguments(exc, "callId", "functionName", ThrowingParser);
Assert.Equal("functionName", content.Name);
Assert.Equal("callId", content.CallId);
Assert.Null(content.Arguments);
Assert.IsType<InvalidOperationException>(content.Exception);
Assert.Same(exc, content.Exception.InnerException);

content = FunctionCallContent.CreateFromParsedArguments(exc, "callId", "functionName", ThrowingParser, exceptionFilter: IsJsonException);
Assert.Null(content.Arguments);
Assert.IsType<InvalidOperationException>(content.Exception);
Assert.Same(exc, content.Exception.InnerException);

NotSupportedException otherExc = new();
NotSupportedException thrownEx = Assert.Throws<NotSupportedException>(() =>
FunctionCallContent.CreateFromParsedArguments(otherExc, "callId", "functionName", ThrowingParser, exceptionFilter: IsJsonException));

Assert.Same(otherExc, thrownEx);

static Dictionary<string, object?> ThrowingParser(Exception ex) => throw ex;
static bool IsJsonException(Exception ex) => ex is JsonException;
}

[Fact]
Expand Down
Loading