FirebaseAI: template chat and function calling api#1416
FirebaseAI: template chat and function calling api#1416cynthiajoan wants to merge 1 commit intomainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Firebase AI SDK by introducing a comprehensive chat session management system and integrating robust function calling capabilities. It allows developers to maintain conversational context across multiple turns, define and automatically execute functions based on AI model prompts, and configure these tools, thereby expanding the interactivity and utility of AI models within applications. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a TemplateChatSession for managing conversations with template-based models, including support for multi-turn function calling. It also adds the necessary TemplateTool and related classes for defining and using tools. The changes are well-structured. I've provided a few suggestions to improve API design, robustness, and code clarity, particularly around function call handling and serialization.
| return new Dictionary<string, object>() | ||
| { | ||
| { "name", Name }, | ||
| { "input_schema", _schemaObject != null ? _schemaObject.ToJson() : "" } |
There was a problem hiding this comment.
When _schemaObject is null, input_schema is serialized as an empty string (""). An empty string is not a valid JSON Schema and will likely cause a request failure. A more correct representation for a schema with no properties is an empty object ({}).
{ "input_schema", _schemaObject != null ? _schemaObject.ToJson() : new Dictionary<string, object>() }| if (function.Callable != null) | ||
| { | ||
| result = await function.Callable(call.Args.ToDictionary(k => k.Key, k => k.Value)); | ||
| } | ||
| else | ||
| { | ||
| result = null; | ||
| } |
There was a problem hiding this comment.
The current implementation for a null Callable on a TemplateAutoFunctionDeclaration will result in a null result for the function call. This could be confusing or hide an implementation error. It would be better to throw an informative exception if a function is called without a Callable implementation. The surrounding try-catch block will then handle this exception and provide a clear error message to the developer.
if (function.Callable != null)
{
result = await function.Callable(call.Args.ToDictionary(k => k.Key, k => k.Value));
}
else
{
throw new InvalidOperationException($"Auto function '{function.Name}' is missing a callable implementation.");
}| public Func<IDictionary<string, object>, Task<IDictionary<string, object>>> Callable { get; } | ||
|
|
||
| /// <summary> | ||
| /// Constructs a TemplateAutoFunctionDeclaration. | ||
| /// </summary> | ||
| /// <param name="name">The name of the function.</param> | ||
| /// <param name="callable">The function to execute when requested by the model.</param> | ||
| /// <param name="parameters">Optional dictionary of parameters schema.</param> | ||
| /// <param name="optionalParameters">Optional list of parameter names that are not required.</param> | ||
| public TemplateAutoFunctionDeclaration( | ||
| string name, | ||
| Func<IDictionary<string, object>, Task<IDictionary<string, object>>> callable, | ||
| IDictionary<string, Schema> parameters = null, | ||
| IEnumerable<string> optionalParameters = null) | ||
| : base(name, parameters, optionalParameters) | ||
| { | ||
| Callable = callable; | ||
| } |
There was a problem hiding this comment.
The Callable property on TemplateAutoFunctionDeclaration is defined to return a Task<IDictionary<string, object>>. This forces developers to wrap any simple return value (like a string or number) into a dictionary. For better flexibility and developer experience, consider changing the return type to Task<object>. The calling code in TemplateChatSession already handles an object result, so this change would make the API more ergonomic for developers implementing tool functions.
public Func<IDictionary<string, object>, Task<object>> Callable { get; }
/// <summary>
/// Constructs a TemplateAutoFunctionDeclaration.
/// </summary>
/// <param name="name">The name of the function.</param>
/// <param name="callable">The function to execute when requested by the model.</param>
/// <param name="parameters">Optional dictionary of parameters schema.</param>
/// <param name="optionalParameters">Optional list of parameter names that are not required.</param>
public TemplateAutoFunctionDeclaration(
string name,
Func<IDictionary<string, object>, Task<object>> callable,
IDictionary<string, Schema> parameters = null,
IEnumerable<string> optionalParameters = null)
: base(name, parameters, optionalParameters)
{
Callable = callable;
}| if (_functionDeclarations == null) return Enumerable.Empty<TemplateAutoFunctionDeclaration>(); | ||
| return _functionDeclarations.OfType<TemplateAutoFunctionDeclaration>(); |
There was a problem hiding this comment.
The constructor for TemplateTool ensures that _functionDeclarations is never null. Therefore, the null check _functionDeclarations == null is redundant and can be removed for cleaner code.
return _functionDeclarations.OfType<TemplateAutoFunctionDeclaration>();| internal Dictionary<string, object> ToJson() | ||
| { | ||
| var json = new Dictionary<string, object>(); | ||
| if (_functionDeclarations != null && _functionDeclarations.Any()) |
There was a problem hiding this comment.
The constructor for TemplateTool ensures that _functionDeclarations is never null. Therefore, the null check _functionDeclarations != null is redundant and can be removed for cleaner code.
if (_functionDeclarations.Any())1 participant
Description
Testing
Type of Change
Place an
xthe applicable box: