Fix: Function Tools AOT bugs #26
Conversation
…vidual methods to Tools.
…emaAttribute.cs and FunctionToolAttribute.cs
# Conflicts: # src/tests/CSharpToJsonSchema.AotTests/JsonSerializationTests.cs
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.
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.
# Conflicts: # CSharpToJsonSchema.sln # src/libs/CSharpToJsonSchema/MeaiFunction.cs
Generated snapshot files for ToolsJsonSerializerContext to support integration tests for various tool methods and their serialization. This includes handling for different types of arguments like string, void, and async inputs.
The `MethodFunction` test method is commented out, likely due to being unused or obsolete. This change improves code clarity by removing an inactive test without deleting it entirely, preserving it for potential future use.
|
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 adds a new AOT console project to the solution and introduces several new service implementations and source modifications. It updates dynamic invocation handling and refines compiler warning management in generator files. Additionally, new auto‐generated JSON serialization context files have been added for various tool methods, while numerous obsolete snapshot and generated files for tools and weather functionalities have been removed, streamlining the serialization framework. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Program
participant OpenAIClient
participant OpenAIChatClient
participant FunctionInvokingChatClient
User->>Program: Launch application
Program->>OpenAIClient: Retrieve API key and initialize client
OpenAIClient->>OpenAIChatClient: Create chat client instance
Program->>FunctionInvokingChatClient: Configure chat options & send prompt
FunctionInvokingChatClient->>OpenAIChatClient: Process request
OpenAIChatClient-->>Program: Return response
Program->>User: Display retrieved student records
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.g.received.cs (1)
12-12: Clarify the Version Reset in GeneratedCodeAttributeThe version number is now set to
"0.0.0.0", which appears to be an intentional reset. Please confirm that this change is deliberate and that it aligns with the new AOT (ahead-of-time) tooling behavior. It may be beneficial to add a brief comment explaining the rationale for the version reset in the generated code (to help future maintainers understand its purpose).src/tests/AotConsole/Program.cs (2)
8-10: Improve user feedback for missing API key.When the API key is missing, the program silently returns. Consider adding a console message to inform the user about the missing key and suggest how to set it.
-if (string.IsNullOrWhiteSpace(key)) - return; +if (string.IsNullOrWhiteSpace(key)) +{ + Console.WriteLine("Error: OpenAI API key not found. Please set the OPEN_AI_APIKEY environment variable."); + return; +}
1-27: Consider adding comments and structuring the code.The code would benefit from additional comments explaining each section's purpose. Also, consider extracting the core functionality into methods for better organization and testability.
Example structure:
// Initialize clients and services private static OpenAIClient CreateOpenAIClient(string apiKey) { return new OpenAIClient(new ApiKeyCredential(apiKey)); } // Process the student query private static async Task<string> ProcessStudentQueryAsync(string prompt, OpenAIClient client) { // Implementation... } // Main program public static async Task<int> Main() { // Get API key and validate // Create clients and process query // Handle and display results }src/libs/CSharpToJsonSchema.Generators/Sources.Method.Tools.cs (1)
104-110: Good addition of GetDelegateInputsTypes method.This new method properly handles explicit default values in method parameters, which is important for correct delegate type representation. However, the method should include XML documentation to explain its purpose and differences from the existing
GetInputsTypesmethod.+ /// <summary> + /// Gets a string representation of method parameter types along with parameter names and default values. + /// This is different from GetInputsTypes as it includes parameter names and default values. + /// </summary> + /// <param name="first">The method data to extract types from</param> + /// <param name="addReturnType">Whether to include the return type</param> + /// <returns>A string representation of the method's parameter list</returns> private static string GetDelegateInputsTypes(MethodData first, bool addReturnType = true) { var f = first.Parameters.Select(s => $"{s.Type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)} {s.Name}"+(s.HasExplicitDefaultValue? $" = " + (s.Type.Name == "CancellationToken"?"default":s.ExplicitDefaultValue?.ToString()) :"")).ToList(); if(addReturnType) f.Add(first.ReturnType.ToDisplayString()); return string.Join(", ", f); }src/tests/AotConsole/Services/StudentRecordService.cs (2)
1-3: Check namespace alignment with project structure.The namespace
CSharpToJsonSchema.MeaiTests.Servicesdoesn't seem to align with the file path (src/tests/AotConsole/Services). Consider using a namespace that reflects the project structure.-namespace CSharpToJsonSchema.MeaiTests.Services; +namespace CSharpToJsonSchema.AotConsole.Services; using DescriptionAttribute = System.ComponentModel.DescriptionAttribute;
29-53: Consider adding nullable reference type annotations.Since this appears to be a new project in .NET 9.0, it would be beneficial to add nullable reference type annotations to the StudentRecord class properties. This would improve type safety.
- public string StudentId { get; set; } = string.Empty; - public string FullName { get; set; } = string.Empty; + public string StudentId { get; set; } = string.Empty; + public string FullName { get; set; } = string.Empty; + public required GradeLevel Level { get; set; } = GradeLevel.Freshman; - public GradeLevel Level { get; set; } = GradeLevel.Freshman; - public List<string> EnrolledCourses { get; set; } = new List<string>(); - public Dictionary<string, double> Grades { get; set; } = new Dictionary<string, double>(); + public List<string> EnrolledCourses { get; set; } = new List<string>(); + public Dictionary<string, double> Grades { get; set; } = new Dictionary<string, double>();src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_NoReturnAsyncArgs.g.received.cs (1)
1-96: Auto-generated file observation.
This file appears to be auto-generated code for JSON serialization context. Typically, auto-generated files should either remain unmodified or excluded from source control to prevent manual edits from being overwritten.src/tests/AotConsole/Services/BookService.cs (1)
34-37: Inconsistent return value for GetBookPageContentAsyncThe returned string doesn't appear to be related to books or page content - it's about weather. Consider returning content that's more relevant to the method name and purpose.
public Task<string> GetBookPageContentAsync(string bookName, int bookPageNumber, CancellationToken cancellationToken = default) { - return Task.FromResult("this is a cool weather out there, and I am stuck at home."); + return Task.FromResult($"Page {bookPageNumber} of book '{bookName}': This is sample content for the requested page."); }src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_StringArgs.g.received.cs (1)
52-61: Suggestion: Clarify non-null usage of the setter.
You are forcing non-null assignment (value!) toInput. IfInputcan be legitimately null, consider removing the null-forgiving operator or adding a piece of documentation explaining why it’s safe to assume non-null.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (68)
CSharpToJsonSchema.sln(1 hunks)src/libs/CSharpToJsonSchema.Generators/Sources.Method.Calls.cs(8 hunks)src/libs/CSharpToJsonSchema.Generators/Sources.Method.Tools.cs(5 hunks)src/libs/CSharpToJsonSchema/MeaiFunction.cs(0 hunks)src/libs/CSharpToJsonSchema/TypeToSchemaHelpers.cs(2 hunks)src/tests/AotConsole/AotConsole.csproj(1 hunks)src/tests/AotConsole/Program.cs(1 hunks)src/tests/AotConsole/Services/BookService.cs(1 hunks)src/tests/AotConsole/Services/StudentRecordService.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.PropertyNames.g.received.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.PropertyNames.g.verified.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_NoReturnAsyncArgs.g.received.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_NoReturnAsyncArgs.g.verified.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_NoReturnAsyncArgs.g.received.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_NoReturnAsyncArgs.g.verified.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_StringArgs.g.received.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_StringArgs.g.verified.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_StringAsyncArgs.g.received.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_StringAsyncArgs.g.verified.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_VoidArgs.g.received.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_VoidArgs.g.verified.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_StringArgs.g.received.cs(3 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_StringArgs.g.verified.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_StringAsyncArgs.g.received.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_StringAsyncArgs.g.verified.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_VoidArgs.g.received.cs(3 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_VoidArgs.g.verified.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.String.g.received.cs(2 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.String.g.verified.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Boolean.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Calls.generated.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.DateOnly.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.DateTime.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Double.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.GetCurrentWeather3Args.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.GetJsonTypeInfo.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.GetValueArgs.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.GetValueAsyncArgs.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Int32.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Int64.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.PropertyNames.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Single.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Tools.generated.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes_Diagnostics.verified.txt(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.Calls.generated.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.Double.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.GetCurrentWeatherArgs.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.GetCurrentWeatherAsyncArgs.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.GetJsonTypeInfo.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.PropertyNames.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.Tools.generated.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.Unit.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.Weather.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.Calls.generated.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.Double.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.GetCurrentWeather2Args.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.GetCurrentWeatherAsync2Args.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.GetJsonTypeInfo.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.PropertyNames.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.String.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.Tools.generated.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.Unit2.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.Weather2.g.verified.cs(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.g.received.cs(1 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict_Diagnostics.verified.txt(0 hunks)src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather_Diagnostics.verified.txt(0 hunks)
💤 Files with no reviewable changes (39)
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict_Diagnostics.verified.txt
- src/libs/CSharpToJsonSchema/MeaiFunction.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.PropertyNames.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.Tools.generated.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.GetJsonTypeInfo.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.Unit.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.Tools.generated.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.PropertyNames.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.GetCurrentWeatherAsyncArgs.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.DateOnly.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Boolean.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Double.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Tools.generated.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Single.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Int32.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Int64.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.DateTime.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.GetJsonTypeInfo.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.Weather.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.Unit2.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes_Diagnostics.verified.txt
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.GetCurrentWeather3Args.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.Double.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.GetCurrentWeatherAsync2Args.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.Weather2.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather_Diagnostics.verified.txt
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.GetJsonTypeInfo.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.PropertyNames.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.GetCurrentWeather2Args.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.GetValueArgs.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.Double.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.GetValueAsyncArgs.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.Calls.generated.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.Calls.generated.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.WeatherStrict#IWeatherStrictTools.String.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.Weather#IWeatherTools.GetCurrentWeatherArgs.g.verified.cs
- src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.VariousTypes#IVariousTypesTools.Calls.generated.verified.cs
🔇 Additional comments (46)
src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_VoidArgs.g.received.cs (2)
1-96: The JsonSerializerContext looks correctly structured overall.The auto-generated serialization context for
SampleFunctionTool_VoidArgsfollows the correct pattern for System.Text.Json source generators. The class properly initializes properties and handles serialization logic.
90-91:Details
✅ Verification successful
Verify the
PropName_inputconstant is properly defined.The code uses
PropName_inputwhich appears to be a constant that should be defined elsewhere in the serializer context. Make sure this constant is properly defined to avoid runtime errors.
🏁 Script executed:
#!/bin/bash # Check if PropName_input is defined in any of the context files rg "PropName_input\s*=" --type cs -A 1 -B 1 src/Length of output: 609
Verified:
PropName_inputis Defined CorrectlyThe constant is properly defined in the serializer context. The scan confirmed that in the file
src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.PropertyNames.g.received.cs
PropName_inputis set as a static readonly field usingJsonEncodedText.Encode("input"), ensuring it is correctly initialized.src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_VoidArgs.g.verified.cs (1)
1-1: Empty verified snapshot file is acceptable.Empty verified snapshot files are commonly used in snapshot testing frameworks to verify that no content should be generated in certain cases. This appears to be intentional.
src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_NoReturnAsyncArgs.g.verified.cs (1)
1-1: Empty verified snapshot file is acceptable.Empty verified snapshot files are commonly used in snapshot testing frameworks to verify that no content should be generated in certain cases. This appears to be intentional.
src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_StringAsyncArgs.g.verified.cs (1)
1-1: Empty verified snapshot file is acceptable.Empty verified snapshot files are commonly used in snapshot testing frameworks to verify that no content should be generated in certain cases. This appears to be intentional.
src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_VoidArgs.g.verified.cs (1)
1-1: Empty Snapshot File Verification
This file only contains a non-visible character (BOM) and no additional content. Please confirm that an empty snapshot (aside from the BOM) is the expected baseline output for this test.src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_NoReturnAsyncArgs.g.verified.cs (1)
1-1: Empty Snapshot File Verification
The file contains solely a BOM character. Verify that this empty state is intentional for the corresponding snapshot test and that it meets the expected verification criteria.src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.String.g.verified.cs (1)
1-1: Verified Empty Snapshot File
This verified snapshot file includes only a non-visible character (BOM). Ensure that this is the intended content for confirming the JSON serialization baseline.src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_StringArgs.g.verified.cs (1)
1-1: Empty Snapshot File Check
The file is currently empty except for a BOM character. As this file is auto‐generated for snapshot verification, please confirm that its emptiness is intentional and consistent with how other snapshot files are structured.src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.PropertyNames.g.verified.cs (1)
1-1: Snapshot File Baseline Confirmed
This new snapshot file only contains a non-visible character. Please double-check that this minimal content is by design for the verification of property names in the JSON serialization context.src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_StringAsyncArgs.g.verified.cs (1)
1-1: Verified test snapshot is set up correctly.This empty verified snapshot file is correctly structured for snapshot testing comparison.
src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_StringArgs.g.verified.cs (1)
1-1: Verified test snapshot is set up correctly.This empty verified snapshot file is correctly structured for snapshot testing comparison.
src/libs/CSharpToJsonSchema/TypeToSchemaHelpers.cs (1)
55-64: AOT compatibility fix for JSON serialization.The added pragma directives correctly suppress and restore IL2026 warnings for reflection-based code that might be affected by AOT trimming. This change aligns with similar handling at lines 32-34 and ensures consistent AOT compatibility throughout the codebase.
src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.PropertyNames.g.received.cs (1)
1-16: Generated JSON serialization infrastructure looks correct.This auto-generated code provides optimized property name encoding for JSON serialization. The structure follows best practices for generated code with appropriate nullable annotations and warning suppressions.
src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.String.g.received.cs (2)
12-12: LGTM: Class name updated to match tool organization.The class name has been updated from
WeatherToolsExtensionsJsonSerializerContexttoToolsJsonSerializerContextas noted in the AI summary, which appears to be part of a broader reorganization effort.
1-1:Details
✅ Verification successful
Verify file naming convention matches project standards.
The file is marked as a "received" snapshot test file, suggesting it's an output comparison file generated during testing. Ensure this naming pattern (
ToolsJsonSerializerContext.String.g.received.cs) follows your project's snapshot testing conventions.
🏁 Script executed:
#!/bin/bash # Check for other snapshot test files with similar naming patterns fd -e cs "\.received\.cs$" --exec grep -l "auto-generated" {} \; | head -n 5Length of output: 781
File Naming Convention Verified
The snapshot fileToolsJsonSerializerContext.String.g.received.csfollows the same naming pattern as other auto-generated snapshot test files in the repository. Our verification shows that the naming convention (including markers like.g.received.csafter the context) is consistent with the project's standards.src/libs/CSharpToJsonSchema.Generators/Sources.Method.Tools.cs (3)
1-2: LGTM: Organized imports.The addition of the Conversion namespace import makes sense as it's likely needed for the new delegate input types handling functionality.
70-71: Fixed issue with tool registration.The change correctly adds the tool to the Delegates dictionary after validation, which is a necessary step that was missing before.
76-80: Improved loop for better readability and performance.Converting from a foreach to a for loop with explicit indexing makes the code more maintainable and potentially more performant.
src/tests/AotConsole/Services/StudentRecordService.cs (3)
9-26: LGTM: Well-structured service method with async pattern.The method follows good async/await practices and includes proper CancellationToken support. The implementation returns mock data, which is appropriate for testing purposes.
48-52: Good implementation of CalculateGPA method.The method correctly handles the case when there are no grades by returning 0.0 and uses LINQ's Average method when there are grades. This is clean and concise.
55-72: Well-documented request class with proper descriptions.The QueryStudentRecordRequest class has appropriate descriptions for all properties, which will be helpful for generating good API documentation and schema information.
src/libs/CSharpToJsonSchema.Generators/Sources.Method.Calls.cs (8)
5-5: LGTM: Added explicit SymbolDisplayFormat import.This import improves code readability by reducing namespace qualification in the code.
17-17: Updated compiler warning pragma syntax.The code now correctly disables CS8600 (converting nullable to non-nullable) in addition to CS8602 (dereference of possibly null reference). This is appropriate for the dynamic invocation code that follows.
111-112: Improved dynamic invocation with explicit type casting.The changes replace the use of
(dynamic)with explicit retrieval from theDelegatesdictionary followed byDynamicInvoke, with proper type casting. This is a safer approach that provides better performance and error messages.
141-143: Consistent delegate invocation pattern applied.This change follows the same pattern improvement as seen in other methods, using dictionary access and DynamicInvoke instead of dynamic casting.
153-155: Improved async method delegate invocation.The updated code retrieves the delegate from the dictionary and applies explicit type casting for the asynchronous method call, which is consistent with the changes in other methods.
190-191: Improved void async method delegate invocation.The delegate retrieval and invocation pattern is consistently applied to void async methods as well, with proper type casting.
81-88: Enhanced pragma warning management for IL trimming.The code now correctly scopes IL2026 and IL3050 warnings (about reflection-based serialization) to the specific code blocks that need them, using proper disable/restore syntax.
Also applies to: 117-123, 160-166
213-213: Proper restoration of compiler warnings.The pragma warning restore at the end of the file correctly matches the disable at the beginning, ensuring warnings aren't accidentally disabled in downstream code.
src/tests/AotConsole/AotConsole.csproj (1)
1-23: Consider verifying compatibility and stability of .NET 9.0 AOT and preview packages.
This new project file looks structurally sound. However, .NET 9.0 and the9.3.0-preview.1.25114.11preview packages might undergo further changes before final release. Please ensure such dependencies won’t break your build pipeline or cause maintenance issues down the road.Would you like a script to check whether these preview packages have known incompatibilities or open issues?
CSharpToJsonSchema.sln (1)
47-48: Add new project reference.
Including the new “AotConsole” project in the solution is clear and properly aligned under the tests folder.src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_NoReturnAsyncArgs.g.received.cs (1)
1-96:Details
✅ Verification successful
Auto-generated code looks good
This auto-generated JSON serialization context for
SampleFunctionTool_Static_NoReturnAsyncArgsfollows standard patterns for System.Text.Json source generation. Since this is auto-generated code, I'll just verify that the serialization handler correctly refers to the property name constant.
🏁 Script executed:
#!/bin/bash # Check if PropName_input is defined in the ToolsJsonSerializerContext class rg -A 2 "PropName_input" --type csLength of output: 4989
Auto-generated JSON Serialization Context Verified
The auto-generated code for
SampleFunctionTool_Static_NoReturnAsyncArgsconforms to the expected patterns. The verification confirms that the serialization handler correctly references the property name constant (PropName_input), which is defined inToolsJsonSerializerContext.PropertyNames.g.received.cs.
- The serialization handler uses
writer.WriteString(PropName_input, __value_Input);as expected.- The
PropName_inputconstant is properly declared and reused across generated files.src/tests/AotConsole/Services/BookService.cs (2)
8-9: Good practice - initializing string propertiesInitializing string properties to
string.Emptyrather than leaving them as null is a good practice to prevent null reference exceptions.
15-20: Well-documented interface with descriptive attributesGood use of
[Description]attributes to document the purpose of the interface methods and their parameters, which will likely be reflected in the generated schemas.src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_VoidArgs.g.received.cs (1)
1-96: Auto-generated code looks consistentThis auto-generated serialization context for
SampleFunctionTool_Static_VoidArgsfollows the same pattern as the other serialization contexts. The code structure is sound, with proper null-checking and property handling.src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_StringArgs.g.received.cs (1)
1-96: Auto-generated code follows established patternsThe serialization context for
SampleFunctionTool_Static_StringArgsis consistent with the other generated serialization contexts, using the same patterns for type registration, property initialization, and serialization handling.src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_StringAsyncArgs.g.received.cs (3)
1-9: File header and warning directives look fine.
These initial lines are typical of source-generated code and handle nullable context and compiler warnings for outdated or obsolete members. No changes needed.
14-25: Lazy initialization pattern looks good.
The auto‐property_SampleFunctionTool_StringAsyncArgsand its public accessor follow the recommended pattern for JSON type info resolution.
75-94: Serialize handler implementation appears correct.
Null checks, object start/end, and property writes are in proper order. The usage ofwriter.WriteStringfor theInputproperty is clean and straightforward.src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_Static_StringAsyncArgs.g.received.cs (3)
1-9: Header and compiler directives
The auto‐generated header and associated#pragmadirectives are aligned with standard practices for generated code.
14-25: Static variant retains the same overall structure.
The_SampleFunctionTool_Static_StringAsyncArgsbacking field and its property remain consistent with the non‐static version. No issues spotted.
75-94: Serialization logic is consistent.
Checks for null values and the final JSON writing are in place. The flow mirrors best practices for source‐generated serialization handlers.src/tests/CSharpToJsonSchema.SnapshotTests/Snapshots/ToolTests.MethodFunction#ToolsJsonSerializerContext.SampleFunctionTool_StringArgs.g.received.cs (3)
1-9: Auto‐generated header is standard.
Nothing unusual about the file banner, nullable directives, or obsolete warning suppression.
14-25: Property introduction for_SampleFunctionTool_StringArgs.
This property is consistent with the pattern used in the other context partial classes. Lazy initialization is correctly applied.
75-94: Safe handling of null references and property serialization.
The null check and property writing logic inSampleFunctionTool_StringArgsSerializeHandlerreflect recommended patterns for generated code.
| chatOptions.Tools = tools.AsMeaiTools(); | ||
| var message = new ChatMessage(ChatRole.User, prompt); | ||
| var response = await chatClient.GetResponseAsync(message,options:chatOptions).ConfigureAwait(false); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for API calls.
The API call lacks try-catch error handling. Network requests can fail for various reasons, and the application should gracefully handle these failures.
-var message = new ChatMessage(ChatRole.User, prompt);
-var response = await chatClient.GetResponseAsync(message,options:chatOptions).ConfigureAwait(false);
+var message = new ChatMessage(ChatRole.User, prompt);
+try
+{
+ var response = await chatClient.GetResponseAsync(message, options: chatOptions).ConfigureAwait(false);
+ Console.WriteLine(response.Choices.LastOrDefault()?.Text ?? "No response received.");
+}
+catch (Exception ex)
+{
+ Console.WriteLine($"Error occurred while communicating with OpenAI: {ex.Message}");
+}📝 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.
| chatOptions.Tools = tools.AsMeaiTools(); | |
| var message = new ChatMessage(ChatRole.User, prompt); | |
| var response = await chatClient.GetResponseAsync(message,options:chatOptions).ConfigureAwait(false); | |
| chatOptions.Tools = tools.AsMeaiTools(); | |
| var message = new ChatMessage(ChatRole.User, prompt); | |
| try | |
| { | |
| var response = await chatClient.GetResponseAsync(message, options: chatOptions).ConfigureAwait(false); | |
| Console.WriteLine(response.Choices.LastOrDefault()?.Text ?? "No response received."); | |
| } | |
| catch (Exception ex) | |
| { | |
| Console.WriteLine($"Error occurred while communicating with OpenAI: {ex.Message}"); | |
| } |
| [FunctionTool(MeaiFunctionTool = true)] | ||
|
|
There was a problem hiding this comment.
Missing using statement for FunctionTool attribute.
The code uses the FunctionTool attribute but doesn't include a using directive for the namespace that contains it. Add the appropriate using statement.
namespace CSharpToJsonSchema.MeaiTests.Services;
using DescriptionAttribute = System.ComponentModel.DescriptionAttribute;
+using CSharpToJsonSchema; // Add this for FunctionTool attribute📝 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.
| [FunctionTool(MeaiFunctionTool = true)] | |
| namespace CSharpToJsonSchema.MeaiTests.Services; | |
| using DescriptionAttribute = System.ComponentModel.DescriptionAttribute; | |
| using CSharpToJsonSchema; // Add this for FunctionTool attribute | |
| [FunctionTool(MeaiFunctionTool = true)] |
| using DescriptionAttribute = System.ComponentModel.DescriptionAttribute; | ||
|
|
||
|
|
||
| namespace CSharpToJsonSchema.MeaiTests.Services; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Namespace mismatch with file location
The namespace CSharpToJsonSchema.MeaiTests.Services doesn't match the file location in the AotConsole project. This can lead to confusion about where the code belongs.
-namespace CSharpToJsonSchema.MeaiTests.Services;
+namespace CSharpToJsonSchema.AotConsole.Services;📝 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.
| namespace CSharpToJsonSchema.MeaiTests.Services; | |
| namespace CSharpToJsonSchema.AotConsole.Services; |
3 participants
Summary by CodeRabbit
New Features
Refactor & Maintenance
Chores