-
Notifications
You must be signed in to change notification settings - Fork 344
[WIP] Add android template #233
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
base: main
Are you sure you want to change the base?
Conversation
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.
Summary of Changes
Hello @bebechien, 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!
I've added a new Android application template to the Workshops/Android
directory. This template, named GemmaAppText
, is designed to provide a quick and easy way to integrate the Gemma large language model into Android applications. It includes all necessary project files, build configurations, and a basic chat interface built with Jetpack Compose, along with a wrapper for the Gemma model, to facilitate rapid development and experimentation with on-device LLM inference.
Highlights
- New Android Application Template: This PR introduces a complete, ready-to-use Android application template (
GemmaAppText
) designed for easy integration of the Gemma large language model. It provides a foundational project structure, build configurations, and essential code components to kickstart Gemma-powered Android applications. - Jetpack Compose Chat UI: The template features a chat-like user interface built with Jetpack Compose, demonstrating how to send prompts and display streaming responses from the Gemma model. This provides a clear example of interactive AI integration within an Android app.
- Simplified Gemma Model Integration: A dedicated
GemmaModel
wrapper class is included, abstracting the complexities of MediaPipe'sLlmInference
API. This class handles model loading, session management, and asynchronous response generation, making it straightforward to interact with the Gemma model. - Detailed Setup and Usage Documentation: Comprehensive instructions are provided in the
README.md
for both quick-start (using the boilerplate) and manual setup scenarios. This includes guidance on downloading the Gemma model and deploying it to an Android device, ensuring developers can easily get the template running.
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 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 or fill out our survey 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.
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. ↩
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 adds a new Android sample application demonstrating the use of the Gemma model. The implementation is a good starting point, but I've identified several areas for improvement. My main feedback focuses on a critical bug in the model initialization logic that could cause crashes, improving thread-safety in the singleton pattern, and refactoring the UI state management to follow best practices with a single source of truth. I've also pointed out some potential build issues with dependency versions and several smaller code quality and documentation improvements.
if (!modelExists()) { | ||
Log.d(TAG, "Model not found at path: ${LLMConstants.MODEL_PATH}") | ||
//throw IllegalArgumentException("Model not found at path: ${LLMConstants.MODEL_PATH}") | ||
} else { | ||
|
||
// Set the configuration options for the LLM Inference task | ||
val taskOptions = LlmInference.LlmInferenceOptions.builder() | ||
.setModelPath(LLMConstants.MODEL_PATH) | ||
.setMaxTokens(LLMConstants.DEFAULT_MAX_TOKEN) | ||
.setPreferredBackend(LLMConstants.DEFAULT_BACKEND) | ||
.build() | ||
|
||
// Create an instance of the LLM Inference task | ||
llmInference = LlmInference.createFromOptions(context, taskOptions) | ||
createSession() | ||
} |
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.
The init
block's handling of a missing model file is unsafe. If modelExists()
returns false, it only logs a message and continues. This will cause a crash later with an UninitializedPropertyAccessException
because llmInference
will not be initialized. You should throw an exception to fail fast, similar to what you do in generateResponseAsync
.
if (!modelExists()) {
Log.e(TAG, "Model not found at path: ${LLMConstants.MODEL_PATH}")
throw IllegalArgumentException("Model not found at path: ${LLMConstants.MODEL_PATH}")
}
// Set the configuration options for the LLM Inference task
val taskOptions = LlmInference.LlmInferenceOptions.builder()
.setModelPath(LLMConstants.MODEL_PATH)
.setMaxTokens(LLMConstants.DEFAULT_MAX_TOKEN)
.setPreferredBackend(LLMConstants.DEFAULT_BACKEND)
.build()
// Create an instance of the LLM Inference task
llmInference = LlmInference.createFromOptions(context, taskOptions)
createSession()
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.
@gemini-code-assist No, I intentionally do this. The app won't crash and shows the "model not found" error message on ChatScreen.
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 for clarifying your intentional design choice, @bebechien. I understand that the ChatViewModel
's try-catch
block around generateResponseAsync
and the modelExists()
checks in close()
and resetSession()
effectively prevent UninitializedPropertyAccessException
from crashing the app in these specific scenarios, allowing you to display a user-friendly error message.
My initial comment was primarily focused on the init
block's role in ensuring the GemmaModel
object is in a fully valid and usable state immediately after construction. While your current approach handles the error gracefully in the observed UI flow, relying on lateinit
properties that might not be initialized can sometimes lead to less predictable behavior or potential UninitializedPropertyAccessException
in other parts of the codebase if new methods are added that access llmInference
or llmInferenceSession
without first checking modelExists()
.
For future maintainability and to align more closely with Kotlin's idiomatic practices for properties that might not always be initialized, consider making llmInference
and llmInferenceSession
nullable (LlmInference?
, LlmInferenceSession?
). This would explicitly communicate their potential nullability and enforce null-safety checks at compile time, making the code more robust against future modifications.
However, given your explicit design choice and the existing error handling, I will defer to your implementation for this pull request.
lifecycleRuntimeKtx = "2.9.2" | ||
lifecycleViewmodelCompose = "2.9.2" | ||
activityCompose = "1.10.1" | ||
composeBom = "2025.07.00" |
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.
val placeholderPrompt = stringResource(R.string.prompt_placeholder) | ||
val placeholderResult = stringResource(R.string.result_placeholder) | ||
var prompt by rememberSaveable { mutableStateOf(placeholderPrompt) } | ||
var result by rememberSaveable { mutableStateOf(placeholderResult) } |
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.
The result
state variable has a complex lifecycle and is updated from multiple sources (the updateResult
callback and observing uiState
). This makes the UI logic hard to follow and prone to bugs. The ViewModel
's UiState
should be the single source of truth for the UI.
I recommend refactoring to hold the generated text within the UiState
. This will centralize state management in the ViewModel
.
1. Modify UiState.kt
:
Change Generating
to a data class
to hold the partial response.
// In UiState.kt
sealed interface UiState {
// ...
data class Generating(val partialText: String) : UiState
// ...
}
2. Modify ChatViewModel.kt
:
Update sendPrompt
to emit Generating
states and remove the callback parameter.
// In ChatViewModel.kt
fun sendPrompt(prompt: String) {
_uiState.value = UiState.Loading
viewModelScope.launch(Dispatchers.Default) {
try {
var response = ""
llmModel.generateResponseAsync(prompt) { partialResult, done ->
response += partialResult
if (done) {
_uiState.value = UiState.Success(response)
} else {
_uiState.value = UiState.Generating(response)
}
}
} catch (e: Exception) {
_uiState.value = UiState.Error(e.localizedMessage ?: "Error")
}
}
}
3. Modify ChatScreen.kt
:
Remove the local result
state and derive the displayed text directly from uiState
.
// In ChatScreen.kt
// Remove: var result by rememberSaveable { ... }
// Remove: fun updateResult(...)
// Update button's onClick:
onClick = {
chatViewModel.resetSession()
chatViewModel.sendPrompt(prompt)
}
// Update text display logic:
if (uiState !is UiState.Loading) {
val (text, color) = when (val state = uiState) {
is UiState.Generating -> state.partialText to MaterialTheme.colorScheme.onSurface
is UiState.Success -> state.outputText to MaterialTheme.colorScheme.onSurface
is UiState.Error -> state.errorMessage to MaterialTheme.colorScheme.error
else -> stringResource(R.string.result_placeholder) to MaterialTheme.colorScheme.onSurface
}
// ...
Text(text = text, color = color, ...)
}
private var instance: GemmaModel? = null | ||
|
||
fun getInstance(context: Context): GemmaModel { | ||
return if (instance != null) { | ||
instance!! | ||
} else { | ||
GemmaModel(context).also { instance = it } | ||
} | ||
} |
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.
The getInstance
method is not thread-safe. If multiple threads call it simultaneously when instance
is null, it could lead to multiple instances of GemmaModel
being created. Use double-checked locking to ensure thread safety. Also, it's a good practice to use the applicationContext
to prevent memory leaks.
@Volatile
private var instance: GemmaModel? = null
fun getInstance(context: Context): GemmaModel {
return instance ?: synchronized(this) {
instance ?: GemmaModel(context.applicationContext).also { instance = it }
}
}
fun getFactory(context: Context) = object : ViewModelProvider.Factory { | ||
override fun <T : ViewModel> create(modelClass: Class<T>, extras: CreationExtras): T { | ||
val inferenceModel = GemmaModel.getInstance(context) | ||
return ChatViewModel(inferenceModel) as T | ||
} | ||
} |
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.
The ViewModelProvider.Factory
implementation can be simplified using the modern viewModelFactory
initializer from lifecycle-viewmodel-ktx
. This makes the code more concise and idiomatic. You would then use it in your ChatScreen
like this: viewModel(factory = ChatViewModel.Factory)
.
val Factory: ViewModelProvider.Factory = viewModelFactory {
initializer {
val application = checkNotNull(get(ViewModelProvider.AndroidViewModelFactory.APPLICATION_KEY))
val inferenceModel = GemmaModel.getInstance(application)
ChatViewModel(inferenceModel)
}
}
/* Other default colors to override | ||
background = Color(0xFFFFFBFE), | ||
surface = Color(0xFFFFFBFE), | ||
onPrimary = Color.White, | ||
onSecondary = Color.White, | ||
onTertiary = Color.White, | ||
onBackground = Color(0xFF1C1B1F), | ||
onSurface = Color(0xFF1C1B1F), | ||
*/ |
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.
/* Other default text styles to override | ||
titleLarge = TextStyle( | ||
fontFamily = FontFamily.Default, | ||
fontWeight = FontWeight.Normal, | ||
fontSize = 22.sp, | ||
lineHeight = 28.sp, | ||
letterSpacing = 0.sp | ||
), | ||
labelSmall = TextStyle( | ||
fontFamily = FontFamily.Default, | ||
fontWeight = FontWeight.Medium, | ||
fontSize = 11.sp, | ||
lineHeight = 16.sp, | ||
letterSpacing = 0.5.sp | ||
) | ||
*/ |
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.
|
||
buildTypes { | ||
release { | ||
isMinifyEnabled = false |
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.
For release builds, isMinifyEnabled
is set to false
. While this might be acceptable for a sample app to simplify debugging, it's not recommended for production. Minification reduces APK size and obfuscates code, which is important for security and performance in release builds. It would be helpful to either enable it or add a comment explaining that it should be enabled for production.
|
||
```kotlin | ||
setContent { | ||
MyApplicationTheme { // Or your app's theme |
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.
The theme name in the example code is MyApplicationTheme
, but the theme defined in this project is GemmaAppTextTheme
. This should be corrected to avoid confusion for users following the manual setup guide.
MyApplicationTheme { // Or your app's theme | |
GemmaAppTextTheme { // Or your app's theme |
|
||
After completing these steps, you can run your app to see the Gemma-powered chat interface in action. | ||
|
||
 |
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.
/gemini review |
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 adds two Android template projects, GemmaAppText
and GemmaAppImage
, for integrating the Gemma model, along with a README.md
file with instructions. The projects are a good starting point, but I have identified several areas for improvement regarding Android development best practices, code correctness, and thread safety. Key issues include a non-robust model initialization that can lead to crashes, non-thread-safe singletons, blocking I/O on the main thread, and side effects in Jetpack Compose UI code. I've provided specific suggestions to address these points, which will enhance the quality and reliability of the templates.
if (!modelExists()) { | ||
Log.d(TAG, "Model not found at path: ${LLMConstants.MODEL_PATH}") | ||
//throw IllegalArgumentException("Model not found at path: ${LLMConstants.MODEL_PATH}") |
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.
The init
block currently logs an error if the model file doesn't exist but allows the object construction to continue. This will lead to an UninitializedPropertyAccessException
when methods on this class are called, causing a crash.
The initialization should fail fast. Throw an exception (e.g., IllegalStateException
) if the model cannot be found to ensure the application doesn't proceed in an invalid state.
if (!modelExists()) {
val errorMessage = "Model not found at path: ${LLMConstants.MODEL_PATH}"
Log.e(TAG, errorMessage)
throw IllegalStateException(errorMessage)
}
if (!modelExists()) { | ||
Log.d(TAG, "Model not found at path: ${LLMConstants.MODEL_PATH}") | ||
//throw IllegalArgumentException("Model not found at path: ${LLMConstants.MODEL_PATH}") |
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.
The init
block currently logs an error if the model file doesn't exist but allows the object construction to continue. This will lead to an UninitializedPropertyAccessException
when methods on this class are called, causing a crash.
The initialization should fail fast. Throw an exception (e.g., IllegalStateException
) if the model cannot be found to ensure the application doesn't proceed in an invalid state.
if (!modelExists()) {
val errorMessage = "Model not found at path: ${LLMConstants.MODEL_PATH}"
Log.e(TAG, errorMessage)
throw IllegalStateException(errorMessage)
}
var textColor = MaterialTheme.colorScheme.onSurface | ||
if (uiState is UiState.Error) { | ||
textColor = MaterialTheme.colorScheme.error | ||
result = (uiState as UiState.Error).errorMessage | ||
} else if (uiState is UiState.Generating) { | ||
textColor = MaterialTheme.colorScheme.onSurface | ||
result = (uiState as UiState.Generating).partialResult | ||
} else if (uiState is UiState.Success) { | ||
textColor = MaterialTheme.colorScheme.onSurface | ||
result = (uiState as UiState.Success).outputText | ||
} | ||
val scrollState = rememberScrollState() | ||
Text( | ||
text = result, | ||
textAlign = TextAlign.Start, | ||
color = textColor, | ||
modifier = Modifier | ||
.padding(16.dp) | ||
.fillMaxSize() | ||
.verticalScroll(scrollState) | ||
) |
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 block of code mutates the result
state variable as a side effect during composition, which violates a core principle of Compose. The UI should be a direct function of the state, without side effects during composition.
Instead of mutating a rememberSaveable
variable, you should derive the textColor
and resultText
directly from the uiState
within the composition. This makes the UI more predictable and stable.
This change would also allow you to remove the result
state variable declared on line 61.
val (textColor, resultText) = when (val state = uiState) {
is UiState.Error -> MaterialTheme.colorScheme.error to state.errorMessage
is UiState.Generating -> MaterialTheme.colorScheme.onSurface to state.partialResult
is UiState.Success -> MaterialTheme.colorScheme.onSurface to state.outputText
is UiState.Initial -> MaterialTheme.colorScheme.onSurface to placeholderResult
}
val scrollState = rememberScrollState()
Text(
text = resultText,
textAlign = TextAlign.Start,
color = textColor,
modifier = Modifier
.padding(16.dp)
.fillMaxSize()
.verticalScroll(scrollState)
)
|
||
buildTypes { | ||
release { | ||
isMinifyEnabled = false |
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.
For release builds, it's highly recommended to enable minification (isMinifyEnabled = true
). This helps reduce the size of your APK by shrinking and obfuscating the code, which is a crucial optimization for production apps. While it might be disabled for easier debugging in a sample, it's a best practice to demonstrate in a template.
isMinifyEnabled = true
val bitmap = BitmapFactory.decodeResource( | ||
context.resources, | ||
images[selectedImage.intValue] | ||
) | ||
chatViewModel.resetSession() | ||
chatViewModel.sendPrompt(bitmap, prompt) | ||
}, |
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.
BitmapFactory.decodeResource
is a blocking I/O operation that runs on the main thread here. This can cause the UI to freeze or lead to an "Application Not Responding" (ANR) error, especially with large images. This operation should be moved to a background thread.
A good practice is to delegate this work to the ViewModel
, which can use viewModelScope
to handle it asynchronously without blocking the UI.
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.
@gemini-code-assist Makes sense. I'll fix it in the next commit. Thanks!
fun getFactory(context: Context) = object : ViewModelProvider.Factory { | ||
override fun <T : ViewModel> create(modelClass: Class<T>, extras: CreationExtras): T { | ||
val inferenceModel = GemmaModel.getInstance(context) | ||
return ChatViewModel(inferenceModel) as T |
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.
The unchecked cast as T
can be suppressed with @Suppress("UNCHECKED_CAST")
to acknowledge that the cast is intentional and to remove the compiler warning. This is done in the GemmaAppText
project's ChatViewModel
, and it would be good to be consistent.
@Suppress("UNCHECKED_CAST")
return ChatViewModel(inferenceModel) as T
@@ -0,0 +1,58 @@ | |||
package com.example.gemmaappimage.ui.theme | |||
|
|||
import android.app.Activity |
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.
kotlinOptions { | ||
jvmTarget = "11" | ||
} |
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.
The kotlinOptions
block is a legacy way to configure Kotlin compilation options. The modern and recommended approach is to use the kotlin
extension with compilerOptions
. This is also inconsistent with the GemmaAppText
project, which uses the modern syntax.
kotlin {
compilerOptions {
jvmTarget.set(org.jetbrains.kotlin.gradle.dsl.JvmTarget.JVM_11)
}
}
@@ -0,0 +1,58 @@ | |||
package com.example.gemmaapptext.ui.theme | |||
|
|||
import android.app.Activity |
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.
|
||
**For Image + Text Input (`GemmaAppImage`):** | ||
|
||
1. **Copy Images:** Copy sample JPEG files to the `app/src/main/res/drawable` directoy. |
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.
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.
@gemini-code-assist typo! thanks for catching it. I'll fix it.
Provide the resources and instructions to integrate the Gemma model into Android app.