-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add a toggle for googleAI or vertexAI backend #1690
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
Adds a picker UI to the main `FirebaseAISample` screen allowing users to select between Google AI and Vertex AI backends. - Introduced an `AIBackend` enum to represent the choices. - Added a `@State` variable in `FirebaseAISampleApp` to manage the selection. - Added a `Picker` in `ContentView` bound to the state variable. - Passed the selected backend down to each sample view (`Summarize`, `PhotoReasoning`, `Chat`, `FunctionCalling`, `Imagen`). - Updated the ViewModels for each sample to accept the backend choice and initialize `FirebaseAI.firebaseAI(backend:)` accordingly. - Refactored the `AIBackend` enum to a common location.
Adds a Picker UI to the main ContentView allowing users to select between Google AI and Vertex AI as the backend for the Generative AI samples. - Introduced a `BackendOption` enum. - Added a Picker in `ContentView` bound to a state variable holding the selected backend. - Modified sample Screens (`SummarizeScreen`, `PhotoReasoningScreen`, `ConversationScreen`, `FunctionCallingScreen`, `ImagenScreen`) to accept the chosen `FirebaseAIBackend` in their initializers. - Updated corresponding ViewModels (`SummarizeViewModel`, `PhotoReasoningViewModel`, `ConversationViewModel`, `FunctionCallingViewModel`, `ImagenViewModel`) to receive the backend instance via their initializers and use it when initializing the Firebase AI service, removing the previously hardcoded `.googleAI()` backend.
Refactors the codebase based on feedback to initialize the FirebaseAI service instance once in ContentView and pass it down the hierarchy, instead of passing the backend option and initializing the service in each ViewModel. - ContentView now manages a @State variable `firebaseService: FirebaseAI` which is updated when the backend Picker selection changes. - Sample Screens now receive the `firebaseService` instance in their initializers. - ViewModels now receive the `firebaseService` instance from their respective Screens and use it directly to obtain generative/Imagen models, removing the internal service initialization call.
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.
Hello @paulb777, 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!
Summary of Changes
This pull request introduces a toggle to switch between Google AI and Vertex AI backends for the Generative AI samples. It modifies the ContentView to include a Picker that allows the user to select the backend. The selected backend is then used to initialize the FirebaseAI service, which is passed to the various screens and view models. This allows users to easily test the samples with different backends. The changes involve modifying the initializers of the view models to accept a FirebaseAI instance, and updating the screen previews to pass in a FirebaseAI instance.
Highlights
- Backend Toggle: Adds a
PickerinContentViewto select between Google AI and Vertex AI backends. - Dependency Injection: Modifies view model initializers to accept a
FirebaseAIinstance, enabling backend selection. - Screen Updates: Updates screen previews to pass in a
FirebaseAIinstance for proper initialization.
Changelog
Click here to see the changelog
- firebaseai/ChatSample/Screens/ConversationScreen.swift
- Modified initializer to accept
FirebaseAIinstance. - Updated previews to pass in a
FirebaseAIinstance.
- Modified initializer to accept
- firebaseai/ChatSample/ViewModels/ConversationViewModel.swift
- Modified initializer to accept
FirebaseAIinstance and use it to create the generative model.
- Modified initializer to accept
- firebaseai/FirebaseAISample/ContentView.swift
- Added
BackendOptionenum to represent the available backends. - Added a
Pickerto select the backend. - Updated the
firebaseServicestate to reflect the selected backend. - Passed the
firebaseServiceinstance to the various screens.
- Added
- firebaseai/FunctionCallingSample/Screens/FunctionCallingScreen.swift
- Modified initializer to accept
FirebaseAIinstance. - Updated previews to pass in a
FirebaseAIinstance.
- Modified initializer to accept
- firebaseai/FunctionCallingSample/ViewModels/FunctionCallingViewModel.swift
- Modified initializer to accept
FirebaseAIinstance and use it to create the generative model.
- Modified initializer to accept
- firebaseai/GenerativeAIMultimodalSample/Screens/PhotoReasoningScreen.swift
- Modified initializer to accept
FirebaseAIinstance. - Updated previews to pass in a
FirebaseAIinstance.
- Modified initializer to accept
- firebaseai/GenerativeAIMultimodalSample/ViewModels/PhotoReasoningViewModel.swift
- Modified initializer to accept
FirebaseAIinstance and use it to create the generative model.
- Modified initializer to accept
- firebaseai/GenerativeAITextSample/Screens/SummarizeScreen.swift
- Modified initializer to accept
FirebaseAIinstance. - Updated previews to pass in a
FirebaseAIinstance.
- Modified initializer to accept
- firebaseai/GenerativeAITextSample/ViewModels/SummarizeViewModel.swift
- Modified initializer to accept
FirebaseAIinstance and use it to create the generative model.
- Modified initializer to accept
- firebaseai/ImagenScreen/ImagenScreen.swift
- Modified initializer to accept
FirebaseAIinstance. - Updated previews to pass in a
FirebaseAIinstance.
- Modified initializer to accept
- firebaseai/ImagenScreen/ImagenViewModel.swift
- Modified initializer to accept
FirebaseAIinstance and use it to create the imagen model.
- Modified initializer to accept
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A toggle's gentle sway,
Backend choices light the way,
AI's path unfolds.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
|
/gemini summary |
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 introduces a toggle for Google AI or Vertex AI backends, enhancing the flexibility of the application. The changes involve modifying the ContentView to include a picker for selecting the backend and passing the selected FirebaseAI service instance to the various screens. The ViewModel classes are also updated to accept a FirebaseAI instance in their initializers. Overall, the changes seem well-structured and address the objective effectively.
Summary of Findings
- Potential retain cycle: The
onChangehandler inContentViewmight cause a retain cycle iffirebaseServicecapturesselectedBackendstrongly. Consider using[weak self]or[unowned self]to avoid this. - Force-unwrapping: The code relies on force-unwrapping in several places. This could lead to runtime crashes if the unwrapped values are nil. Consider using optional binding or providing default values to handle potential nil values gracefully.
- Inconsistent initialization: The
firebaseServiceis initialized twice inContentView, once with a default value and again inonAppear. This could lead to unexpected behavior if the initial value is used beforeonAppearis called.
Merge Readiness
The pull request introduces a valuable feature by allowing users to select between Google AI and Vertex AI backends. However, there are some potential issues that need to be addressed before merging, such as the potential retain cycle in ContentView and the reliance on force-unwrapping. Addressing these issues will improve the stability and maintainability of the code. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging.
|
Thanks for the ping, and for running the review and summary commands, @paulb777. It looks like this pull request introduces a toggle to switch between Google AI and Vertex AI backends for the Generative AI samples. The core change involves modifying the I don't have any specific feedback to add at this time. |
|
Did you try injecting it into the Environment instead? I'm curious if that'd clean up a bunch of instantiations of StateObjects. IIRC the general rule of thumb was around |
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.
LGTM. Just highlighted a few potential remaining Jules comments but feel free to ignore. Thanks!
firebaseai/FunctionCallingSample/Screens/FunctionCallingScreen.swift
Outdated
Show resolved
Hide resolved
Co-authored-by: Andrew Heard <andrewheard@google.com>
Co-authored-by: Andrew Heard <andrewheard@google.com>
….swift Co-authored-by: Andrew Heard <andrewheard@google.com>
No description provided.