-
Notifications
You must be signed in to change notification settings - Fork 82
Update documentation for the genui implemention - 'Getting Started wi… #573
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
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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.
Code Review
This pull request updates the documentation to align with recent API changes in FirebaseAiContentGenerator. The changes are accurate, but I've identified a couple of areas in the README where comments could be clearer or are outdated. My suggestions aim to improve the clarity and correctness of the documentation for developers using the library.
| _genUiManager = GenUiManager(catalog: CoreCatalogItems.asCatalog()); | ||
| // Create a ContentGenerator to communicate with the LLM. | ||
| // Provide system instructions and the tools from the GenUiManager. |
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.
This comment is now outdated. The code no longer passes tools from _genUiManager. The core UI tools are now inferred from the catalog parameter within FirebaseAiContentGenerator. You should update this comment to reflect the current implementation.
| // Provide system instructions and the tools from the GenUiManager. | |
| // Provide system instructions and any additional tools. |
packages/genui/README.md
Outdated
| additionalTools: [ | ||
| // Additional tools to be provided to the AI model. Schema [AiTool] (required String name, required String description, Schema? parameters, String? prefix) | ||
| ], |
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.
Placing a long comment inside an empty list can be hard to read. It would be more conventional and readable to place the explanatory comments above the additionalTools parameter.
| additionalTools: [ | |
| // Additional tools to be provided to the AI model. Schema [AiTool] (required String name, required String description, Schema? parameters, String? prefix) | |
| ], | |
| // Additional tools to be provided to the AI model. The schema for an AiTool is: | |
| // required String name, required String description, Schema? parameters, String? prefix | |
| additionalTools: [], |
jacobsimionato
left a comment
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.
Thank you so much for fixing the documentation here!
packages/genui/README.md
Outdated
| ''', | ||
| tools: _genUiManager.getTools(), | ||
| additionalTools: [ | ||
| // Additional tools to be provided to the AI model. Schema [AiTool] (required String name, required String description, Schema? parameters, String? prefix) |
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.
I think we can remove the 'additionalTools' parameter from the example here, seeing as it's not necessary for the most simple setup.
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.
That makes sense. I have push the change.
Description
This PR updates the documentation in the
genuipackage README to reflect recent API changes in theFirebaseAiContentGeneratorconstructor.Changes:
catalogparameter showing how to provideCoreCatalogItems.asCatalog()to the content generatortoolsparameter withadditionalToolsparameter to better reflect the current API structureadditionalToolsparameter schemaThese documentation updates ensure that developers following the "Getting Started" guide will use the correct, current API when integrating GenUI into their Flutter applications.
Why these changes:
The API structure has evolved to separate core catalog items from additional tools, making it clearer how to configure the content generator. This documentation update ensures pub.dev shows the correct usage pattern.
Pre-launch Checklist
///).Fixes #572