fix: Functions without parameters #31
Conversation
WalkthroughThe pull request updates several components across the project. In the schema helper, the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant SchemaBuilder
Caller->>SchemaBuilder: ConvertToSchema(type, description)
alt No properties in type
SchemaBuilder-->>Caller: null
else Properties exist
SchemaBuilder->>SchemaBuilder: Build schema
SchemaBuilder-->>Caller: OpenApiSchema
end
sequenceDiagram
participant Test
participant OpenAIClient
participant ChatClient
participant Service
Test->>OpenAIClient: Initialize with API key
OpenAIClient->>ChatClient: Setup chat session with tools
Test->>ChatClient: Send request message (e.g., student records/books)
ChatClient->>Service: Invoke function tool
Service-->>ChatClient: Return response data
ChatClient-->>Test: Deliver response message
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
🪧 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 (
|
|
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/tests/CSharpToJsonSchema.AotTests/Services/MethodFunctionTools.cs (1)
61-65: Method naming is inconsistent with class convention.This method is defined as static but does not follow the naming convention used by other static methods in this class, which include "Static" in their name.
- public static string SampleFunctionTool_No_Parameters() + public static string SampleFunctionTool_Static_No_Parameters_2()Or if this is meant to be an instance method:
- public static string SampleFunctionTool_No_Parameters() + public string SampleFunctionTool_No_Parameters()src/tests/CSharpToJsonSchema.AotTests/MethodFunctionTools_Tests.cs (1)
83-88: Missing assertion in test method.While the test correctly invokes the method with an empty JSON object, it doesn't verify the return value. Consider adding an assertion to ensure the function returns the expected result.
public async Task Should_SampleFunctionTool_No_Parameters() { var tools = new Tools([MethodFunctionTools.SampleFunctionTool_No_Parameters]); - await tools.CallAsync(nameof(MethodFunctionTools.SampleFunctionTool_No_Parameters), "{}"); + var value = await tools.CallAsync(nameof(MethodFunctionTools.SampleFunctionTool_No_Parameters), "{}"); + Assert.Equal("\"Hello world from string return\"", value); }src/tests/CSharpToJsonSchema.MeaiTests/Services/BookService.cs (1)
41-44: Improve readability by using consistent formattingThe implementation works correctly but could benefit from consistent formatting with the rest of the codebase, particularly matching the formatting style used in the
GetAuthorBooksAsyncmethod.- public Task<List<GetAuthorBook>> GetBooksAsync(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>> GetBooksAsync(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" } + ])); + }src/tests/CSharpToJsonSchema.MeaiTests/Meai_Tests.cs (2)
67-92: Consider consolidating duplicate testsThis test is nearly identical to
ShouldInvokeTheFunctions_NoParameterswith the only difference being the method called (GetStudentsList2vsGetStudentsListAsync). Since both methods have identical functionality, consider consolidating these tests or using a parameterized test approach.+ [TestMethod] + [DataRow(nameof(StudentRecordService.GetStudentsListAsync))] + [DataRow(nameof(StudentRecordService.GetStudentsList2))] + public async Task ShouldInvokeTheFunctions_NoParameters(string methodName) + { + var key = Environment.GetEnvironmentVariable("OPEN_AI_APIKEY",EnvironmentVariableTarget.User); + if (string.IsNullOrWhiteSpace(key)) + return; + + var client = new OpenAIClient(new ApiKeyCredential(key)); + + Microsoft.Extensions.AI.OpenAIChatClient openAiClient = new OpenAIChatClient(client.GetChatClient("gpt-4o-mini")); + + var chatClient = new Microsoft.Extensions.AI.FunctionInvokingChatClient(openAiClient); + var chatOptions = new ChatOptions(); + + var service = new StudentRecordService(); + var method = typeof(StudentRecordService).GetMethod(methodName); + var tools = new Tools([method?.CreateDelegate(typeof(Func<CancellationToken, Task<List<StudentRecord>>>), service)]); + chatOptions.Tools = tools.AsMeaiTools(); + + var message = new ChatMessage(ChatRole.User, "Get all students records"); + var response = await chatClient.GetResponseAsync(message,options:chatOptions).ConfigureAwait(false); + + response.Text.Contains("John", StringComparison.InvariantCultureIgnoreCase).Should() + .Be(true); + + Console.WriteLine(response.Text); + }
122-149: Use consistent client initialization patternsWhile the test is functional, I note that line 130 uses a different approach to create the chat client compared to the other tests in this file. Consider using a consistent pattern throughout the class for better maintainability.
- var chatClient = new OpenAIClient(new ApiKeyCredential(key)).AsChatClient("gpt-4o-mini").AsBuilder().UseFunctionInvocation().Build(); + var client = new OpenAIClient(new ApiKeyCredential(key)); + Microsoft.Extensions.AI.OpenAIChatClient openAiClient = new OpenAIChatClient(client.GetChatClient("gpt-4o-mini")); + var chatClient = new Microsoft.Extensions.AI.FunctionInvokingChatClient(openAiClient);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/libs/CSharpToJsonSchema/SchemaSubsetHelper.cs(5 hunks)src/tests/CSharpToJsonSchema.AotTests/MethodFunctionTools_Tests.cs(1 hunks)src/tests/CSharpToJsonSchema.AotTests/Services/MethodFunctionTools.cs(1 hunks)src/tests/CSharpToJsonSchema.IntegrationTests/MethodFunctionTools.cs(1 hunks)src/tests/CSharpToJsonSchema.MeaiTests/Meai_Tests.cs(2 hunks)src/tests/CSharpToJsonSchema.MeaiTests/Services/BookService.cs(2 hunks)src/tests/CSharpToJsonSchema.MeaiTests/Services/StudentRecordService.cs(3 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
src/tests/CSharpToJsonSchema.AotTests/MethodFunctionTools_Tests.cs (1)
src/tests/CSharpToJsonSchema.AotTests/Services/MethodFunctionTools.cs (1)
MethodFunctionTools(6-66)
src/tests/CSharpToJsonSchema.MeaiTests/Meai_Tests.cs (2)
src/tests/CSharpToJsonSchema.MeaiTests/Services/BookService.cs (4)
Task(26-34)Task(36-39)Task(41-44)BookStoreService(24-45)src/tests/CSharpToJsonSchema.MeaiTests/Services/StudentRecordService.cs (1)
StudentRecordService(5-104)
src/tests/CSharpToJsonSchema.MeaiTests/Services/StudentRecordService.cs (1)
src/tests/CSharpToJsonSchema.MeaiTests/Meai_Tests.cs (2)
Task(14-38)Task(95-121)
🔇 Additional comments (11)
src/tests/CSharpToJsonSchema.IntegrationTests/MethodFunctionTools.cs (1)
56-57: Minor formatting adjustment.The addition of these blank lines appears to be a formatting improvement for better code readability, separating the method implementations.
src/tests/CSharpToJsonSchema.AotTests/Services/MethodFunctionTools.cs (1)
56-60: LGTM! Added support for parameterless static functions.The implementation correctly adds a static function with no parameters, following the existing pattern for function tools.
src/tests/CSharpToJsonSchema.AotTests/MethodFunctionTools_Tests.cs (1)
76-81: LGTM! Test for parameterless static function.The test correctly verifies that a static function with no parameters can be invoked with an empty JSON object.
src/libs/CSharpToJsonSchema/SchemaSubsetHelper.cs (3)
12-16: Good fix for handling types with no properties.Making the return type nullable and adding an early exit when a type has no properties is a good improvement. This prevents attempting to create schemas for empty types.
47-54: Improved readability of return statement.The reformatting of the return statement improves code readability by clearly structuring the object initialization.
57-58: Improved parameter formatting.The reformatting of method parameters improves readability by placing parameters on separate lines.
src/tests/CSharpToJsonSchema.MeaiTests/Services/BookService.cs (1)
20-21: LGTM: Interface method signature properly defines a parameter-less operationThe addition of the
GetBooksAsyncmethod to the interface with only a cancellation token parameter (with default value) is a good approach for implementing a parameter-less operation while maintaining cancellation support.src/tests/CSharpToJsonSchema.MeaiTests/Meai_Tests.cs (1)
40-65: LGTM: Appropriate test for parameter-less functionThis test correctly validates that the parameter-less method
GetStudentsListAsyncworks properly. The test structure follows the established pattern and verifies that the expected student data is returned.src/tests/CSharpToJsonSchema.MeaiTests/Services/StudentRecordService.cs (3)
9-10: LGTM: Improved parameter formattingThe reformatting of the method signature improves readability by placing parameters on separate lines, especially useful when parameter lists grow longer.
29-65: LGTM: Added parameter-less method to retrieve all student recordsThe implementation of
GetStudentsListAsyncprovides a good way to retrieve all student records without requiring specific parameters, which simplifies certain use cases.
137-149: LGTM: Improved readability with consistent spacingThe added whitespace between property definitions in the
QueryStudentRecordRequestclass improves readability and maintains consistency.
2 participants
Summary by CodeRabbit
New Features
Tests
Refactor