-
Notifications
You must be signed in to change notification settings - Fork 361
[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
GemmaModelwrapper class is included, abstracting the complexities of MediaPipe'sLlmInferenceAPI. 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.mdfor 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.