Support offline Gemma model, fix scrollbar, and update build script#51
Conversation
…ed APK Co-authored-by: Android-PowerUser <88908510+Android-PowerUser@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Summary
This PR adds offline Gemma model support with download capability, implements an optimized scrollbar, and updates the build script. I've identified 4 critical defects that must be fixed before merge:
Critical Issues Found
- ModelDownloadManager.kt (Line 24-26): Missing null check for getExternalFilesDir() - will crash when external storage is unavailable
- ModelDownloadManager.kt (Line 47-48): Unsafe cast of DownloadManager - will crash if service unavailable
- PhotoReasoningViewModel.kt (Line 424): Missing null check for getApplication() - can crash during initialization/cleanup
- build_and_sign.sh (Line 42-45): Hardcoded debug credentials used for release APK signing - severe security vulnerability (CWE-798)
Additional Concern
The API key validation logic (GenerativeAiViewModelFactory.kt lines 61-67) may block offline model usage since the offline Gemma model doesn't require an API key. While not visible in the diff, this should be verified to ensure offline functionality works correctly.
All identified issues have blocking severity and must be resolved before merging.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| -keystore debug.keystore \ | ||
| -storepass android \ | ||
| -alias androiddebugkey \ | ||
| -keypass android \ |
There was a problem hiding this comment.
🛑 Security Vulnerability: Hardcoded debug credentials in production build script. The debug.keystore uses hardcoded password "android" and is used to sign release APKs. This is a severe security issue - attackers can extract the keystore password and sign malicious APKs with your key. Use a production keystore with secure credentials for release builds.1
Footnotes
-
CWE-798: Use of Hard-coded Credentials - https://cwe.mitre.org/data/definitions/798.html ↩
|
|
||
| // Check for offline model (Gemma) | ||
| if (currentModel == ModelOption.GEMMA_3N_E4B_IT) { | ||
| val context = getApplication<Application>().applicationContext |
There was a problem hiding this comment.
🛑 Crash Risk: Missing null check causes crash. getApplication() can return null in edge cases during ViewModel initialization or cleanup, causing NullPointerException when accessing applicationContext. This can crash the app when saving chat history.
| val context = getApplication<Application>().applicationContext | |
| val context = getApplication<Application>().applicationContext |
| val downloadManager = context.getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager | ||
| downloadId = downloadManager.enqueue(request) |
There was a problem hiding this comment.
🛑 Crash Risk: Unsafe type cast can crash. The getSystemService call returns Any? but is force-cast to DownloadManager without null checking. If DOWNLOAD_SERVICE is unavailable (rare but possible on some devices), this causes a crash.
| val downloadManager = context.getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager | |
| downloadId = downloadManager.enqueue(request) | |
| val downloadManager = context.getSystemService(Context.DOWNLOAD_SERVICE) as? DownloadManager | |
| ?: throw IllegalStateException("DownloadManager not available") | |
| downloadId = downloadManager.enqueue(request) |
| fun getModelFile(context: Context): File { | ||
| return File(context.getExternalFilesDir(null), MODEL_FILENAME) | ||
| } |
There was a problem hiding this comment.
🛑 Crash Risk: Missing null check causes crash. getExternalFilesDir(null) can return null when external storage is unavailable or unmounted, causing a NullPointerException when passed to the File constructor. This will crash the app during model file operations.
| fun getModelFile(context: Context): File { | |
| return File(context.getExternalFilesDir(null), MODEL_FILENAME) | |
| } | |
| fun getModelFile(context: Context): File { | |
| val externalDir = context.getExternalFilesDir(null) | |
| ?: throw IllegalStateException("External storage not available") | |
| return File(externalDir, MODEL_FILENAME) | |
| } |
…ed APK Co-authored-by: Android-PowerUser <88908510+Android-PowerUser@users.noreply.github.com>
…ed APK Co-authored-by: Android-PowerUser <88908510+Android-PowerUser@users.noreply.github.com>
…ed APK Co-authored-by: Android-PowerUser <88908510+Android-PowerUser@users.noreply.github.com>
…ed APK Co-authored-by: Android-PowerUser <88908510+Android-PowerUser@users.noreply.github.com>
PR created automatically by Jules for task 394299877912208302 started by @Android-PowerUser