-
-
Notifications
You must be signed in to change notification settings - Fork 2
feat: Added M.E.A.I. AIFunction Generation support #24
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
Conversation
…vidual methods to Tools.
…emaAttribute.cs and FunctionToolAttribute.cs
# Conflicts: # src/tests/CSharpToJsonSchema.AotTests/JsonSerializationTests.cs
|
Gunpal Jain seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
WalkthroughThis pull request updates the JSON serialization and schema conversion components. In the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Converter as ConvertToSchema
participant Transformer as TransformSchemaNode
Caller->>Converter: Call ConvertToSchema(type, description)
Converter->>Transformer: Process each property conversion
alt Property is enum
Note right of Converter: Enum detected
Transformer-->>Converter: Return schema type "string"
else Property is not enum
Transformer-->>Converter: Process normally
end
Converter->>Caller: Return OpenApiSchema ("object")
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/libs/CSharpToJsonSchema/OpenApiSchemaJsonContext.cs(1 hunks)src/libs/CSharpToJsonSchema/SchemaSubsetHelper.cs(1 hunks)
🔇 Additional comments (4)
src/libs/CSharpToJsonSchema/OpenApiSchemaJsonContext.cs (1)
10-10: Good addition of JSON serialization optionsAdding strict number handling and null value ignoring options will ensure more consistent JSON serialization behavior. These options will apply globally to all serialization operations in this context.
src/libs/CSharpToJsonSchema/SchemaSubsetHelper.cs (3)
122-125: Proper handling of enum typesGood fix for enum representation in schema generation. By mapping enum types to "string" in the schema, you ensure that enum values are properly represented in the generated JSON schema.
152-152: Explicit schema type settingGood addition - explicitly setting the schema type to "object" ensures the schema conforms to expected structures, which is particularly important for LLM API compatibility.
114-116: Improved code formattingThe code formatting of the JsonSerializer.Deserialize call has been improved for better readability without changing functionality.
| [JsonSerializable(typeof(Tool))] | ||
| [JsonSerializable(typeof(List<Tool>))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing import for Tool class
The code adds JSON serialization for Tool and List<Tool> types, but there's no import statement for the Tool class. This could cause compilation errors.
1
using System.Text.Json.Serialization;
2
+using System.Collections.Generic;
+// Add the appropriate namespace for the Tool class📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [JsonSerializable(typeof(Tool))] | |
| [JsonSerializable(typeof(List<Tool>))] | |
| using System.Text.Json.Serialization; | |
| using System.Collections.Generic; | |
| // Add the appropriate namespace for the Tool class | |
| [JsonSerializable(typeof(Tool))] | |
| [JsonSerializable(typeof(List<Tool>))] |
|
|
||
| var mainDescription =x.Description ?? dics["mainFunction_Desc"]; | ||
|
|
||
| var mainDescription = x.Description ?? dics["mainFunction_Desc"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling for dictionary key lookup
The code assumes that the 'dics' dictionary contains a "mainFunction_Desc" key, but doesn't handle the case where this key might be missing. This could lead to a KeyNotFoundException at runtime.
-var mainDescription = x.Description ?? dics["mainFunction_Desc"];
+var mainDescription = x.Description ?? (dics.TryGetValue("mainFunction_Desc", out var desc) ? desc : "");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var mainDescription = x.Description ?? dics["mainFunction_Desc"]; | |
| var mainDescription = x.Description ?? (dics.TryGetValue("mainFunction_Desc", out var desc) ? desc : ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (11)
src/tests/CSharpToJsonSchema.MeaiTests/Services/StudentRecordService.cs (1)
4-27: Missing cancellation token usage in student service methodThe method accepts a cancellation token parameter but doesn't use it in the implementation. Since this is a synchronous operation returning a hardcoded object wrapped in a Task, you should either use the token or remove it.
-public async Task<StudentRecord> GetStudentRecordAsync(QueryStudentRecordRequest query, CancellationToken cancellationToken = default) +public Task<StudentRecord> GetStudentRecordAsync(QueryStudentRecordRequest query, CancellationToken cancellationToken = default) { - return new StudentRecord + return Task.FromResult(new StudentRecord { StudentId = "12345", FullName = query.FullName, Level = StudentRecord.GradeLevel.Senior, EnrolledCourses = new List<string> { "Math 101", "Physics 202", "History 303" }, Grades = new Dictionary<string, double> { { "Math 101", 3.5 }, { "Physics 202", 3.8 }, { "History 303", 3.9 } }, EnrollmentDate = new DateTime(2020, 9, 1), IsActive = true - }; + }); }src/libs/CSharpToJsonSchema.Generators/Sources.Method.MeaiTools.cs (2)
9-11: Check method signature's conditional logicThe check for
@interface.Methods.Count == 0 || [email protected]will correctly return empty when there are no methods or the tool isn't enabled. However, consider adding a null check for@interface.Methodsto prevent potential NullReferenceException if Methods collection is null.- if(@interface.Methods.Count == 0 || [email protected]) + if(@interface.Methods?.Count == 0 || [email protected])
43-45: Simple condition could be more readableThe condition check for
MeaiFunctionToolis straightforward, but adding a comment explaining what this flag signifies would improve readability and maintainability.- if([email protected]) + // Skip generation if MEAI function tool attribute is not enabled + if([email protected])src/libs/CSharpToJsonSchema.Generators/Conversion/ToModels.cs (2)
21-22: Inconsistent spacing in variable assignmentThere's an inconsistent spacing in the variable assignment where
meaiFunctionTool=has no space after the variable name unlike the other assignments in this method.- var meaiFunctionTool= attributeData.NamedArguments.FirstOrDefault(x => x.Key == "MeaiFunctionTool").Value.Value is bool meaift && + var meaiFunctionTool = attributeData.NamedArguments.FirstOrDefault(x => x.Key == "MeaiFunctionTool").Value.Value is bool meaift &&
66-81: Added MEAI function tools support for methodsThe implementation of MEAI function tool support for methods is correct. The naming of the boolean variable (
meaiFunctionTools) is slightly inconsistent with the singular form used elsewhere (meaiFunctionTool), but the functionality is correctly implemented.- bool meaiFunctionTools = false; + bool meaiFunctionTool = false; - if(meai) - meaiFunctionTools = true; + if(meai) + meaiFunctionTool = true;src/tests/CSharpToJsonSchema.MeaiTests/Meai_Tests.cs (4)
12-13: Commented test attributeThe
[TestMethod]attribute is commented out, which means this test won't be automatically discovered and run by the test runner. This is likely intentional as it may require specific environment setup, but consider documenting why it's commented out or use conditional test attributes if available.
15-18: API key retrieval could be improvedThe test is retrieving the API key from environment variables but only checking for null or whitespace. Consider adding a more informative error message or skip message for test runners.
- var key = Environment.GetEnvironmentVariable("OPEN_AI_APIKEY",EnvironmentVariableTarget.User); - if (string.IsNullOrWhiteSpace(key)) - return; + var key = Environment.GetEnvironmentVariable("OPEN_AI_APIKEY",EnvironmentVariableTarget.User); + if (string.IsNullOrWhiteSpace(key)) + { + Console.WriteLine("Test skipped: OPEN_AI_APIKEY environment variable not set"); + return; + }
40-41: Commented test attribute (again)Similar to the first test method, this test method also has its
[TestMethod]attribute commented out. This pattern suggests these are integration tests that require specific environment setup.
62-63: Test assertion could be more specificThe test is checking if the response contains "damdamadum", but this is a very broad assertion. Consider adding more specific checks for the actual content of page 96 if known or making the assertion more precise.
src/libs/CSharpToJsonSchema/MeaiFunction.cs (1)
40-57: Handle non-JSON argument values or log potential anomalies.The current
GetArgsStringmethod only processesJsonElementorJsonNodeobjects and silently ignores other object types. This could lead to lost data if an argument is neither of these types. Consider either logging a warning or throwing an exception for unsupported argument value types.foreach (var args in arguments) { if (args.Value is JsonElement element) { // ... } else if (args.Value is JsonNode node) { // ... } + else if (args.Value != null) + { + // Logging or converting other object types to a JSON string + jsonObject[args.Key] = JsonValue.Create(args.Value.ToString()); + } }src/tests/CSharpToJsonSchema.MeaiTests/Services/BookService.cs (1)
34-37: Leverage method parameters or indicate a placeholder implementation.The methods do not utilize
authorName,bookPageNumber, orbookName. This might be intentional for demonstration, but leaving them unused can cause confusion. Update the implementation or mark them with a comment (e.g.,// TODO: Implement logic using authorName) if future expansions are planned.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
CSharpToJsonSchema.sln(3 hunks)src/libs/CSharpToJsonSchema.Generators/Conversion/ToModels.cs(5 hunks)src/libs/CSharpToJsonSchema.Generators/JsonSchemaGenerator.cs(3 hunks)src/libs/CSharpToJsonSchema.Generators/Models/InterfaceData.cs(1 hunks)src/libs/CSharpToJsonSchema.Generators/Sources.Method.Calls.cs(1 hunks)src/libs/CSharpToJsonSchema.Generators/Sources.Method.GoogleFunctionTools.cs(1 hunks)src/libs/CSharpToJsonSchema.Generators/Sources.Method.MeaiTools.cs(1 hunks)src/libs/CSharpToJsonSchema/CSharpToJsonSchema.csproj(1 hunks)src/libs/CSharpToJsonSchema/FunctionToolAttribute.cs(1 hunks)src/libs/CSharpToJsonSchema/GenerateJsonSchemaAttribute.cs(1 hunks)src/libs/CSharpToJsonSchema/MeaiFunction.cs(1 hunks)src/libs/CSharpToJsonSchema/OpenApiSchemaJsonContext.cs(1 hunks)src/tests/CSharpToJsonSchema.MeaiTests/CSharpToJsonSchema.MeaiTests.csproj(1 hunks)src/tests/CSharpToJsonSchema.MeaiTests/Meai_Tests.cs(1 hunks)src/tests/CSharpToJsonSchema.MeaiTests/Services/BookService.cs(1 hunks)src/tests/CSharpToJsonSchema.MeaiTests/Services/StudentRecordService.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.g.received.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.g.received.cs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/libs/CSharpToJsonSchema.Generators/Sources.Method.Calls.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/libs/CSharpToJsonSchema/OpenApiSchemaJsonContext.cs
🔇 Additional comments (24)
src/libs/CSharpToJsonSchema.Generators/Models/InterfaceData.cs (1)
7-7: Added support for Microsoft.Extensions.AI function toolsThe addition of the
MeaiFunctionToolboolean field to theInterfaceDatarecord struct complements the existingGoogleFunctionToolfield, extending the schema generation capabilities to support Microsoft.Extensions.AI compatible function tools.src/libs/CSharpToJsonSchema/GenerateJsonSchemaAttribute.cs (1)
20-24: Good addition of Microsoft.Extensions.AI supportThe new
MeaiFunctionToolproperty with proper XML documentation provides a clear interface for generating Microsoft.Extensions.AI compatible function tools at the attribute level.src/libs/CSharpToJsonSchema/FunctionToolAttribute.cs (1)
19-23: Consistent implementation of MEAI supportThe addition of the
MeaiFunctionToolproperty with XML documentation in theFunctionToolAttributeclass maintains consistency with the same property inGenerateJsonSchemaAttribute, allowing for method-level control over MEAI function tool generation.src/libs/CSharpToJsonSchema/CSharpToJsonSchema.csproj (2)
20-20: Verify preview package version compatibilityThe added reference to
Microsoft.Extensions.AI.Abstractionsis using a preview version (9.3.0-preview.1.25114.11). While this is necessary for the new MEAI functionality, be aware that preview packages may change before final release.Consider documenting this dependency on a preview package in the README or release notes, and ensure there's a plan to update to the stable version when it becomes available.
4-4: Multi-target framework support looks goodThe project targets multiple frameworks (
netstandard2.0;net4.6.2;net8.0;net9.0), which is good for compatibility. Make sure that the newMicrosoft.Extensions.AI.Abstractionspackage is compatible with all these frameworks, especially the older ones likenet4.6.2andnetstandard2.0.You can verify compatibility by checking if the package lists these frameworks as supported targets in its NuGet package metadata.
CSharpToJsonSchema.sln (1)
45-46: LGTM: New test project added properly to solutionThe addition of the new
CSharpToJsonSchema.MeaiTestsproject is properly integrated into the solution structure with appropriate project references, build configurations, and placement in the tests folder.Also applies to: 81-84, 98-98
src/tests/CSharpToJsonSchema.MeaiTests/CSharpToJsonSchema.MeaiTests.csproj (1)
1-42: Properly structured test project with appropriate dependenciesThe new test project is well-structured with all necessary dependencies, including testing frameworks, Microsoft.Extensions.AI libraries, and project references to the main libraries. The global usings are appropriately set up for testing.
src/tests/CSharpToJsonSchema.MeaiTests/Services/StudentRecordService.cs (2)
29-53: LGTM: Well-structured student record classThe
StudentRecordclass is well-designed with appropriate properties and a usefulCalculateGPAmethod. The enum forGradeLevelis properly defined.
55-72: LGTM: Well-documented query request classThe
QueryStudentRecordRequestclass is well-structured with descriptive attributes that will be useful for schema generation, clearly explaining the purpose of each property.src/libs/CSharpToJsonSchema.Generators/Sources.Method.GoogleFunctionTools.cs (1)
45-47: LGTM: Fix for extension methods class and method definitionAdding the
staticmodifier to both the extension class and the extension method is correct and follows C# extension method requirements. This was likely a bug in the original code since extension methods must be defined within static classes.src/libs/CSharpToJsonSchema.Generators/Sources.Method.MeaiTools.cs (2)
25-36: Consistent implementation with Google function toolsThe
AsMeaiTools()method implementation looks correct and follows the same pattern as the equivalent GoogleFunctionTools implementations. It properly converts tools to MEAI functions and returns them as a list.
54-65: AsMeaiTools extension method implementation looks goodThe extension method for converting interfaces to MEAI tools is properly implemented using the
thiskeyword and correctly follows the same implementation pattern as the instance method.src/libs/CSharpToJsonSchema.Generators/JsonSchemaGenerator.cs (3)
53-55: MEAI function tools integration looks goodThe integration of the MEAI function tools generation follows the same pattern as the Google function tools. The code properly selects and processes attributes, and adds the source to the context.
79-81: Consistent implementation for interfacesThe implementation for processing MEAI function tools for interfaces correctly follows the same pattern as the Google function tools implementation.
136-148: Well-structured helper methods for MEAI toolsThe implementation of helper methods
AsMeaiFunctionToolsForMethodsandAsMeaiFunctionToolsForInterfaceis correct and follows the same pattern as the equivalents for Google function tools. The naming convention for the generated files is consistent with the existing codebase.src/libs/CSharpToJsonSchema.Generators/Conversion/ToModels.cs (2)
50-51: MEAI function tool property added correctlyThe addition of the
MeaiFunctionToolproperty to theInterfaceDatareturn statement is correctly implemented.
106-107: Consistent property name in return statementThe property name in the return statement is correct, though it would be preferable to maintain consistent naming with the variable above.
src/tests/CSharpToJsonSchema.MeaiTests/Meai_Tests.cs (2)
27-28: Good use of MEAI Tools extensionThe code correctly demonstrates how to use the new MEAI tools functionality by creating tools from the service and converting them to MEAI tools.
56-57: Elegant usage of extension methodThe test shows a more elegant way to convert a service to MEAI tools directly using the extension method, which is a good demonstration of the new functionality.
src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.g.received.cs (1)
1-112: Auto-generated code acknowledgment.This file appears to be auto-generated code for JSON serialization context. Typically, such files are not manually modified. Ensure that any manual edits here are intended and consider adding a comment or warning at the top to clarify that changes might be overwritten by future generation steps.
src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.g.received.cs (4)
15-38: Well-configured JSON serialization options.The default options are comprehensive and follow best practices for JSON serialization, including:
- Proper null handling with
DefaultIgnoreCondition = WhenWritingNull- Security-conscious settings like
ReadCommentHandling = Disallow- Strict number handling with
NumberHandling = Strict- Consistent casing with
PropertyNamingPolicy = JsonNamingPolicy.CamelCaseThese settings should provide reliable serialization behavior for the weather tools implementation.
92-109: Good error handling in converter expansion logic.The
ExpandConvertermethod properly validates converters and provides clear error messages when:
- A converter isn't compatible with the specified type
- A factory returns null or another factory instance
This ensures robust converter resolution during serialization.
13-14:Details
❓ Verification inconclusive
Verify JsonSerializerContext inheritance and attribute generation.
This class appears to inherit from a base serializer context (using
baseandoverride), but the specific type isn't visible. As this is auto-generated code, ensure that:
- The correct base type is being inherited
- Any necessary type-specific serializer attributes are generated properly
For a complete serializer context, you would normally expect to see
[JsonSerializable]attributes for the types being serialized.
🏁 Script executed:
#!/bin/bash # Look for JsonSerializable attributes in related files rg --type csharp "\[JsonSerializable.*Weather" -A 3 -B 3Length of output: 800
Action: Confirm Correct Base Inheritance & Serializer Attributes
Please verify the following in the auto-generated partial class
WeatherToolsExtensionsJsonSerializerContext(file:
src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.g.received.cs):
- Ensure that the context inherits from the intended base type (likely
JsonSerializerContextas indicated in similar unit tests).- Confirm that the necessary type-specific serializer attributes (e.g.,
[JsonSerializable(typeof(Weather))]) are generated. For instance, the unit test file (src/tests/CSharpToJsonSchema.UnitTests/Tests.AsJsonSchema.cs) correctly includes[JsonSerializable(typeof(Weather))].If the base type or attributes are missing or incorrect, please make the appropriate adjustments.
1-2:Details
❓ Verification inconclusive
Verify file naming vs. generated content.
The filename suggests this should implement
IWeatherTools, but the class is actuallyWeatherToolsExtensionsJsonSerializerContext. Since this is a snapshot test file (as indicated by.received.in the filename), please ensure it's being compared against the correct expected output file.
🏁 Script executed:
#!/bin/bash # Check if there's a corresponding approved snapshot file ls -la src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather*Length of output: 6991
Action: Confirm Snapshot Naming & Comparison Consistency
Review the snapshot file
src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.g.received.cs.
The file header shows//HintName: IWeatherTools.g.cs, which suggests an implementation forIWeatherTools. However, the generated content actually relates toWeatherToolsExtensionsJsonSerializerContext. Given that there’s a corresponding verified snapshot file (ToolTests.Weather#IWeatherTools.g.verified.cs), please ensure that:
- The naming convention accurately reflects the generated type.
- Snapshot comparisons are correctly configured against the expected (verified) output.
- Any discrepancy from recent refactoring is intentional and all related tests have been updated accordingly.
| <PackageReference Include="Microsoft.Extensions.AI" Version="9.3.0-preview.1.25114.11" /> | ||
| <PackageReference Include="Microsoft.Extensions.AI.OpenAI" Version="9.3.0-preview.1.25114.11" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify the preview version of Microsoft.Extensions.AI packages
The project references preview versions of Microsoft.Extensions.AI packages. While this is fine for testing, it's worth checking if more stable versions are available or noting any potential compatibility issues.
🌐 Web query:
What is the latest stable version of Microsoft.Extensions.AI and Microsoft.Extensions.AI.OpenAI packages?
💡 Result:
The latest stable version of Microsoft.Extensions.AI and Microsoft.Extensions.AI.OpenAI packages is not yet available, as both remain in preview as of March 2025. Here's the breakdown:
Microsoft.Extensions.AI
- Latest Preview:
9.3.0-preview.1.25114.11(released February 2025) [1][3]. - Key Features:
- Utilities for generative AI integration in .NET applications.
- Support for caching (
DistributedCachingChatClient) and automated function invocation (FunctionInvokingChatClient) [4]. - Used by projects like Semantic Kernel, eShop, and Aspire [1][3].
Microsoft.Extensions.AI.OpenAI
- Latest Preview:
9.0.1-preview.1.24570.5(released November 2024) [6]. - Key Features:
- Integrates Azure OpenAI models via
IChatClientabstractions. - Simplifies authentication with Azure using
DefaultAzureCredential[6][7].
- Integrates Azure OpenAI models via
Context
- The libraries are part of .NET 9's AI building blocks, introduced as a preview in October 2024 [7][12][19].
- Microsoft emphasizes collaboration with Semantic Kernel and ecosystem adoption before stabilizing the APIs [7][12].
- For stable Azure OpenAI integration, use Azure.AI.OpenAI 2.1.0 [11], which supports GPT-4o, Assistants v2, and structured outputs [8][14].
Recommendation
While awaiting stable releases, developers can:
- Use preview versions with caution.
- Leverage Azure's official SDKs (
Azure.AI.OpenAI 2.1.0) for production-ready scenarios [11]. - Monitor the Microsoft.Extensions.AI GitHub repositories for updates.
Citations:
- 1: https://www.nuget.org/packages/Microsoft.Extensions.AI/
- 2: https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/openai/Azure.AI.OpenAI/CHANGELOG.md
- 3: https://www.nuget.org/packages/Microsoft.Extensions.AI/9.3.0-preview.1.25114.11
- 4: https://www.nuget.org/packages/Microsoft.Extensions.AI.Abstractions/
- 5: https://www.nuget.org/packages/OpenAI/
- 6: https://markheath.net/post/getting-started-ai-extensions
- 7: https://devblogs.microsoft.com/dotnet/introducing-microsoft-extensions-ai-preview/
- 8: https://learn.microsoft.com/en-us/azure/ai-services/openai/api-version-deprecation
- 9: https://azure.github.io/azure-sdk/releases/latest/
- 10: https://devblogs.microsoft.com/dotnet/announcing-the-stable-release-of-the-official-open-ai-library-for-dotnet/
- 11: https://www.nuget.org/packages/Azure.AI.OpenAI
- 12: https://www.infoq.com/news/2024/10/dotnet-ai-integration-libraries/
- 13: https://learn.microsoft.com/en-us/answers/questions/1631409/openai-extension-api-version
- 14: https://learn.microsoft.com/en-us/azure/ai-services/openai/whats-new
- 15: https://learn.microsoft.com/en-us/azure/ai-services/openai/reference
- 16: https://www.youtube.com/watch?v=zrPtp00aUX0
- 17: https://platform.openai.com/docs/libraries
- 18: https://learn.microsoft.com/en-us/answers/questions/1527594/after-upgrading-microsoft-extensions-logging-abstr
- 19: https://learn.microsoft.com/en-us/dotnet/core/whats-new/dotnet-9/overview
Action Required: Verify and Update Microsoft.Extensions.AI Package Versions
- The project currently references preview versions, which is appropriate since stable releases aren’t available as of now.
- Microsoft.Extensions.AI is using version
9.3.0-preview.1.25114.11, which matches the latest preview release. - Microsoft.Extensions.AI.OpenAI is also referenced with version
9.3.0-preview.1.25114.11, yet the latest preview available is9.0.1-preview.1.24570.5. Please confirm if this version discrepancy is intentional or if the reference should be updated to align with the officially published preview. - Given that both packages remain in preview, it’s important to document any potential compatibility risks.
| protected override async Task<object?> InvokeCoreAsync(IEnumerable<KeyValuePair<string, object?>> arguments, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| try | ||
| { | ||
| var json = GetArgsString(arguments); | ||
|
|
||
| var call = await _call(json, cancellationToken); | ||
|
|
||
| return JsonSerializer.Deserialize(call, OpenApiSchemaJsonContext.Default.JsonElement); | ||
| }catch(Exception e) | ||
| { | ||
| return e.Message; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider unifying or clarifying the return type of InvokeCoreAsync.
Currently, when the call succeeds, the method returns a JsonElement, but in the catch block, it returns a string holding the exception message. Mixing these return types (both raw JSON and a string) might create ambiguity for consumers of this method if they expect a consistent data structure. Additionally, returning the full exception message could expose internal information.
-protected override async Task<object?> InvokeCoreAsync(IEnumerable<KeyValuePair<string, object?>> arguments,
- CancellationToken cancellationToken)
+protected override async Task<JsonElement> InvokeCoreAsync(
+ IEnumerable<KeyValuePair<string, object?>> arguments,
+ CancellationToken cancellationToken)
{
try
{
var json = GetArgsString(arguments);
var call = await _call(json, cancellationToken);
return JsonSerializer.Deserialize<JsonElement>(
call, OpenApiSchemaJsonContext.Default.JsonElement);
}
catch (Exception e)
{
- return e.Message;
+ // Optionally: wrap error info within a JSON structure or throw a custom exception
+ throw new InvalidOperationException("Invocation failed.", e);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| protected override async Task<object?> InvokeCoreAsync(IEnumerable<KeyValuePair<string, object?>> arguments, | |
| CancellationToken cancellationToken) | |
| { | |
| try | |
| { | |
| var json = GetArgsString(arguments); | |
| var call = await _call(json, cancellationToken); | |
| return JsonSerializer.Deserialize(call, OpenApiSchemaJsonContext.Default.JsonElement); | |
| }catch(Exception e) | |
| { | |
| return e.Message; | |
| } | |
| } | |
| protected override async Task<JsonElement> InvokeCoreAsync( | |
| IEnumerable<KeyValuePair<string, object?>> arguments, | |
| CancellationToken cancellationToken) | |
| { | |
| try | |
| { | |
| var json = GetArgsString(arguments); | |
| var call = await _call(json, cancellationToken); | |
| return JsonSerializer.Deserialize<JsonElement>( | |
| call, OpenApiSchemaJsonContext.Default.JsonElement); | |
| } | |
| catch (Exception e) | |
| { | |
| // Optionally: wrap error info within a JSON structure or throw a custom exception | |
| throw new InvalidOperationException("Invocation failed.", e); | |
| } | |
| } |
| public Task<List<GetAuthorBook>> GetAuthorBooksAsync(string authorName, CancellationToken cancellationToken = default) | ||
| { | ||
| return Task.FromResult(new List<GetAuthorBook>([ | ||
| new GetAuthorBook | ||
| { Title = "Five point someone", Description = "This book is about 3 college friends" }, | ||
| new GetAuthorBook | ||
| { Title = "Two States", Description = "This book is about intercast marriage in India" } | ||
| ])); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify the list initialization syntax.
The usage of [ ] within new List<GetAuthorBook>([ ... ]) is atypical in most C# versions and may cause a compile-time error unless using very recent or experimental language features. If this is unintentional, switch to a standard list initializer:
return Task.FromResult(new List<GetAuthorBook>([
new GetAuthorBook
{ Title = "Five point someone", Description = "This book is about 3 college friends" },
new GetAuthorBook
{ Title = "Two States", Description = "This book is about intercast marriage in India" }
]));Refactor to:
-return Task.FromResult(new List<GetAuthorBook>([
+return Task.FromResult(new List<GetAuthorBook>
{
new GetAuthorBook
{ Title = "Five point someone", Description = "This book is about 3 college friends" },
new GetAuthorBook
{ Title = "Two States", Description = "This book is about intercast marriage in India" }
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public Task<List<GetAuthorBook>> GetAuthorBooksAsync(string authorName, CancellationToken cancellationToken = default) | |
| { | |
| return Task.FromResult(new List<GetAuthorBook>([ | |
| new GetAuthorBook | |
| { Title = "Five point someone", Description = "This book is about 3 college friends" }, | |
| new GetAuthorBook | |
| { Title = "Two States", Description = "This book is about intercast marriage in India" } | |
| ])); | |
| } | |
| public Task<List<GetAuthorBook>> GetAuthorBooksAsync(string authorName, CancellationToken cancellationToken = default) | |
| { | |
| return Task.FromResult(new List<GetAuthorBook> | |
| { | |
| new GetAuthorBook | |
| { Title = "Five point someone", Description = "This book is about 3 college friends" }, | |
| new GetAuthorBook | |
| { Title = "Two States", Description = "This book is about intercast marriage in India" } | |
| }); | |
| } |
Replaced direct dictionary access with TryGetValue to prevent potential KeyNotFound exceptions. This ensures safer and more robust handling of missing keys when fetching "mainFunction_Desc".
Removed unnecessary try-catch block around async call invocation and adjusted formatting for better code clarity. Minor whitespace and indentation improvements were also made to enhance consistency and maintainability.
Added detailed XML documentation for MeaiFunction class and its members, improving code readability and maintainability. Introduced support for strict mode and updated method implementations to enhance functionality. These changes provide better structure and guidance for future development.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/libs/CSharpToJsonSchema/MeaiFunction.cs (1)
64-72: 🛠️ Refactor suggestionReturn a more specific type instead of
object?.
The method always deserializes into aJsonElement, so returningobject?may create confusion. Consider returningTask<JsonElement>to clarify the expected return type.-protected override async Task<object?> InvokeCoreAsync( +protected override async Task<JsonElement> InvokeCoreAsync( IEnumerable<KeyValuePair<string, object?>> arguments, CancellationToken cancellationToken) { var json = GetArgsString(arguments); var call = await _call(json, cancellationToken); return JsonSerializer.Deserialize(call, OpenApiSchemaJsonContext.Default.JsonElement); }
🧹 Nitpick comments (1)
src/libs/CSharpToJsonSchema/MeaiFunction.cs (1)
36-36: Avoid creating a newReadOnlyDictionaryon every property access.
Constructing a newReadOnlyDictionaryeach timeAdditionalPropertiesis accessed can be wasteful. Consider caching or returning the underlying dictionary directly if immutability is guaranteed.-public override IReadOnlyDictionary<string, object?> AdditionalProperties - => new ReadOnlyDictionary<string, object?>(_tool.AdditionalProperties); +private readonly IReadOnlyDictionary<string, object?> _readOnlyProperties; public MeaiFunction(Tool tool, Func<string, CancellationToken, Task<string>> call) { this._tool = tool; this._call = call; _readOnlyProperties = new ReadOnlyDictionary<string, object?>(tool.AdditionalProperties); ... } public override IReadOnlyDictionary<string, object?> AdditionalProperties => _readOnlyProperties;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/libs/CSharpToJsonSchema/MeaiFunction.cs(1 hunks)
🔇 Additional comments (1)
src/libs/CSharpToJsonSchema/MeaiFunction.cs (1)
52-55: Include"Strict"in_jsonSchemabefore serialization if needed.
Currently, you add"Strict"totool.AdditionalPropertiesafter serializingtool.Parametersinto_jsonSchema. If you intend for"Strict"to be reflected in the schema, add it beforehand or re-generate_jsonSchemaafter adding it.
| if (args.Value is JsonElement element) | ||
| { | ||
| if (element.ValueKind == JsonValueKind.Array) | ||
| jsonObject[args.Key] = JsonArray.Create(element); | ||
| else if (element.ValueKind == JsonValueKind.Object) | ||
| jsonObject[args.Key] = JsonObject.Create(element); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle non-array/object JsonElement cases.
If the JsonElement represents a string, number, or boolean, it will be ignored. Consider adding a fallback to capture these value kinds:
if (element.ValueKind == JsonValueKind.Array)
jsonObject[args.Key] = JsonArray.Create(element);
else if (element.ValueKind == JsonValueKind.Object)
jsonObject[args.Key] = JsonObject.Create(element);
+else
+ jsonObject[args.Key] = element.GetRawText();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (args.Value is JsonElement element) | |
| { | |
| if (element.ValueKind == JsonValueKind.Array) | |
| jsonObject[args.Key] = JsonArray.Create(element); | |
| else if (element.ValueKind == JsonValueKind.Object) | |
| jsonObject[args.Key] = JsonObject.Create(element); | |
| } | |
| if (args.Value is JsonElement element) | |
| { | |
| if (element.ValueKind == JsonValueKind.Array) | |
| jsonObject[args.Key] = JsonArray.Create(element); | |
| else if (element.ValueKind == JsonValueKind.Object) | |
| jsonObject[args.Key] = JsonObject.Create(element); | |
| else | |
| jsonObject[args.Key] = element.GetRawText(); | |
| } |
Replaced GenericFunctionTool with GoogleFunctionTool to align with the updated namespace and class structure. This ensures compatibility with the latest library changes and maintains consistency in functionality.
Summary by CodeRabbit
New Features
Bug Fixes
Tests