Repository navigation
Add thinking mode for ai logic hybrid - #8678
VinayGuthal wants to merge 7 commits into
Conversation
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 the 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 counterproductive. 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. |
📝 PRs merging into main branchOur main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for thinking mode in on-device content generation, introducing configuration options like enableThinking, tracking the model's reasoning process via thoughtProcess, and adding availability checks. The review feedback highlights two key improvements: first, updating the candidate mapping logic to accept a list of thoughts rather than a single thought to prevent reasoning steps from being discarded when there is only one candidate; second, disabling the thinking mode check during token counting to avoid unnecessary failures, as tokenization does not execute the model's thinking process.
…nerativemodel/OnDeviceGenerativeModelProvider.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for thinking mode in on-device content generation, adding enableThinking to configuration and request classes, thoughtProcess to response classes, and isThinkingModeAvailable to the generative model interface. Feedback on these changes highlights a critical issue in OnDeviceGenerativeModelProvider.toPublicResponse where sequential thought steps are incorrectly sliced across multiple candidates or mapped to separate candidates when the response candidates list is empty. Refactoring is suggested to associate the entire thoughtProcess with each candidate, which would also allow the removal of the now-redundant fromInteropThought helper function.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for "thinking mode" in on-device content generation by adding enableThinking to request configurations, thoughtProcess to responses, and isThinkingModeAvailable to model interfaces across the firebase-ai and firebase-ai-ondevice modules. Feedback recommends wrapping the initialization block of generateContentStream in a try-catch to ensure setup exceptions are properly wrapped in FirebaseAIException, and filtering out empty thoughts in fromInteropThought to maintain consistency with fromInterop.
…pe/Candidate.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for thinking mode in on-device content generation. It adds enableThinking to request configurations, exposes thoughtProcess in responses, and provides a method to check if thinking mode is available on the model. These changes are propagated through the interop layers, public APIs, and converters, accompanied by corresponding unit tests. Feedback on the changes suggests removing a fragile assertion in ConvertersTest.kt that checks the default value of enableThinking when not explicitly set, as it contradicts the test's stated design principle.
| @@ -1,5 +1,6 @@ | |||
| # Unreleased | |||
|
|
|||
| - [feature] Added `enableThinking` to `GenerateContentRequest`, `thoughtProcess` to `GenerateContentResponse`, and `isThinkingModeAvailable` to `GenerativeModel` to support thinking mode. | |||
There was a problem hiding this comment.
Max 80 chars per line. Also, add PR number
| topK = this@toMlKit.topK | ||
| seed = this@toMlKit.seed | ||
| systemInstruction = this@toMlKit.systemInstruction?.toMlKitSystemInstruction() | ||
| this@toMlKit.enableThinking?.let { enableThinking = it } |
There was a problem hiding this comment.
This statement logic is reversed to all the others (not a direct assignment)
| @@ -1,5 +1,6 @@ | |||
| # Unreleased | |||
|
|
|||
| - [feature] Added support for thinking mode in on-device content generation requests and responses. | |||
| nonEmptyThoughts.forEach { thought -> | ||
| part(TextPart(text = thought.text, isThought = true, thoughtSignature = null)) | ||
| } | ||
| if (candidate.text.isNotEmpty() || nonEmptyThoughts.isEmpty()) { |
There was a problem hiding this comment.
could you walk me through this part? All thoughts are included in the content, but if there are no thoughts, an empty text is used?
| @@ -1,5 +1,6 @@ | |||
| # Unreleased | |||
|
|
|||
| - [feature] Added `enableThinking` to `OnDeviceConfig` to support [thinking mode](https://firebase.google.com/docs/ai-logic/thinking) in on-device and hybrid inference modes, with automatic cloud fallback when the on-device model does not support thinking. | |||
There was a problem hiding this comment.
Max length 80, include PR number
Adds support for thinking mode (
enableThinking) when generating content using on-device and hybrid inference modes.Summary of Changes
firebase-ai-ondevice-interop:enableThinking: Boolean?toGenerateContentRequest.thoughtProcess: List<Candidate>toGenerateContentResponse.isThinkingModeAvailable(): BooleantoGenerativeModel.api.txt.firebase-ai-ondevice:GenerativeModelImpl.isThinkingModeAvailable()delegating to ML Kit'sGenerativeModel.isThinkingModeAvailable().Converters.ktto passenableThinkingto the ML Kit request and mapthoughtProcesscandidates to the interopGenerateContentResponse.firebase-ai:enableThinking: Boolean? = nulltoOnDeviceConfigand updatedapi.txt.OnDeviceGenerativeModelProviderto checkonDeviceModel.isThinkingModeAvailable()whenenableThinking == true(throwingFirebaseAIOnDeviceNotAvailableExceptionsoPREFER_ON_DEVICEfalls back to cloud when unsupported) and forwardenableThinkingto the on-device request.CandidateandOnDeviceGenerativeModelProviderresponse mapping to convert interopthoughtProcesscandidates intoTextPartentries withisThought = true(populatingGenerateContentResponse.thoughtSummaryfor both unary and streaming responses).ConvertersTest,GenerativeModelImplTest, andOnDeviceGenerativeModelProviderTests, and updatedCHANGELOG.mdacross all three modules.Tested at firebase/quickstart-android#2848