Improve perf#381
Conversation
LiteRT runs the tool loop natively via automaticToolCalling, so the JS MAX_TOTAL_TOOL_CALLS cap never applied to it — a single message could trigger unbounded tool calls and overflow the ~4096-token KV cache mid-turn, producing degenerate output or crashing. Add a per-turn counter in buildLiteRTToolCallHandler: calls 1-3 run normally; the 4th+ skips execution and returns a 'stop, answer now' nudge to the model. Counter resets each turn (closure rebuilt per generation). Loop stays native. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…ume retry When a download fails on the Models screen, the card now renders the error message, a red partial-progress bar, and Retry / Remove buttons directly inside the card boundary — matching the Download Manager UI. Tapping Retry calls backgroundDownloadService.retryDownload with the existing download ID so the native WorkManager resumes from the partial file via HTTP Range instead of starting a fresh download from 0. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Remove the two logs that fire in tight loops: - llm.ts: reasoning_content chunk received (fired on every thinking token — O(N²) string work serializing accumulated text each call) - useDownloads.ts: mmproj progress and missed-entry debug logs (fired every 1.5s during download and on every progress event miss) All other diagnostic logs (model load, download lifecycle, tool calls) are untouched — they fire once per user action and are useful for diagnosing real issues. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
ChatsListScreen was subscribing to the entire chatStore and appStore with no selectors, causing it to re-render on every streaming token while mounted in the tab navigator. Actions moved to getState() and data fields use targeted selectors. Adds an informational banner above the chat input when a llama model is loaded with OpenCL selected as the inference backend, nudging users to switch to CPU in Settings. Does not show for LiteRT models or remote models. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
… StyleSheet Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a failed download state UI to the ModelCard component, optimizes download listeners to prevent unnecessary root re-renders, adds an OpenCL warning banner in the chat area, limits native LiteRT tool calls to prevent KV cache overflow, and optimizes store subscriptions in ChatsListScreen. Feedback on these changes highlights a duplicate event listener registration issue in useDownloads, an Android-only API call in the retry download handler that will fail on iOS, and an opportunity to use an existing reactive store selector instead of bypassing it.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| export function useDownloads() { | ||
| useDownloadListeners(); | ||
|
|
There was a problem hiding this comment.
Calling useDownloadListeners() inside useDownloads() causes duplicate event listeners to be registered every time the useDownloads hook is mounted in any component or screen. Since useDownloadListeners() is already registered at the App root (as intended for global background download event handling), calling it here is redundant and leads to multiple active listeners receiving the same progress/complete/error events. This results in duplicate store updates and performance degradation during active downloads.
| export function useDownloads() { | |
| useDownloadListeners(); | |
| export function useDownloads() { |
| const handleRetryDownload = async (modelKey: string, downloadId: string) => { | ||
| const store = useDownloadStore.getState(); | ||
| store.setStatus(downloadId, 'pending'); | ||
| try { | ||
| await backgroundDownloadService.retryDownload(downloadId); | ||
| modelManager.watchDownload( | ||
| downloadId, | ||
| async () => { | ||
| const models = await modelManager.getDownloadedModels(); | ||
| setDownloadedModels(models); | ||
| const key = store.downloadIdIndex[downloadId] ?? modelKey; | ||
| if (key) store.remove(key); | ||
| }, | ||
| (error: Error) => { | ||
| store.setStatus(downloadId, 'failed', { message: error.message }); | ||
| }, | ||
| ); | ||
| backgroundDownloadService.startProgressPolling(); | ||
| } catch (error: any) { | ||
| store.setStatus(downloadId, 'failed', { message: error?.message ?? 'Retry failed' }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The handleRetryDownload function calls backgroundDownloadService.retryDownload(downloadId) directly, which is an Android-only API and throws an error on iOS. This causes the "Retry" button on iOS to always fail immediately with an error. Additionally, if the download has an associated mmproj sidecar that failed, we must call modelManager.resetMmProjForRetry(downloadId) to reset the sidecar state so that finalization waits for the retried sidecar.
On iOS, since native retry by ID is not supported, we should cancel the failed download and remove it from the store so the user can trigger a fresh download. Ensure that any caught errors during cancellation are logged rather than swallowed.
const handleRetryDownload = async (modelKey: string, downloadId: string) => {
const store = useDownloadStore.getState();
store.setStatus(downloadId, 'pending');
modelManager.resetMmProjForRetry(downloadId);
try {
if (Platform.OS === 'android') {
await backgroundDownloadService.retryDownload(downloadId);
modelManager.watchDownload(
downloadId,
async () => {
const models = await modelManager.getDownloadedModels();
setDownloadedModels(models);
const key = store.downloadIdIndex[downloadId] ?? modelKey;
if (key) store.remove(key);
},
(error: Error) => {
store.setStatus(downloadId, 'failed', { message: error.message });
},
);
backgroundDownloadService.startProgressPolling();
} else {
await backgroundDownloadService.cancelDownload(downloadId).catch((err) => console.error('Failed to cancel download:', err));
store.remove(modelKey);
}
} catch (error: any) {
store.setStatus(downloadId, 'failed', { message: error?.message ?? 'Retry failed' });
}
};
References
- When catching errors, log them instead of swallowing them to ensure failures are visible and to aid in debugging.
| ? { pillLabel: 'Recommended', highlightText: liteRTMeta.highlight } | ||
| : undefined; | ||
| const card = ( | ||
| const storeEntry = useDownloadStore.getState().downloads[s.downloadKey]; |
There was a problem hiding this comment.
Instead of calling useDownloadStore.getState().downloads[s.downloadKey] which bypasses the reactive subscription, you can directly use the already subscribed storeDownloads selector defined at line 90 (const storeDownloads = useDownloadStore(state => state.downloads);). This is cleaner, more idiomatic, and avoids redundant store access during render.
| const storeEntry = useDownloadStore.getState().downloads[s.downloadKey]; | |
| const storeEntry = storeDownloads[s.downloadKey]; |
…stener split useDownloads.ts: remove useDownloadListeners() call — now fully independent. App.tsx: mount useDownloadListeners() directly at root so listener registration is not lost after the split. TextModelsTab handleRetryDownload: - Android-only guard; iOS falls back to proceedDownload (fresh download) - mmproj sidecar retry: set pending before retry, only call resetMmProjForRetry if native retry succeeded, set failed on error. Matches retryAndroidDownload in useDownloadManager exactly — prevents silent vision loss on retry from the Models screen. - onRetry branches on Platform.OS - Use storeDownloads selector instead of getState() snapshot for storeEntry Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…callback Replace captured store.downloadIdIndex snapshot with a live getState() call inside the async callback, matching the pattern in reattachRetriedTextDownload in useDownloadManager.ts. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
- ModelCard: 8 new tests covering failedState / FailedSection (new inline retry UI from fix/69d17d28) - generationToolLoop: 4 new tests for LiteRT native tool-call cap introduced in fix/73f85ff8 — verifies cap at 3, Aborted fast-path, and per-generation counter reset - activeModelService loaders: fix stale-path test (add isVisionModel:true), add guard tests for text-only model and mmProjFileName repair sentinel - scan.test.ts (new): unit tests for extractBaseName and findMatchingMmProj, plus curatedLiteRTRegistry entry lookup - visionRepair: 3 additional branch tests (name-lookup false path, catalog-no-mmproj path, fileName vl-detection path) Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…e ID - Create shared test utilities: mocks.ts with AsyncStorage, logger, whisper service, and HTTP client factories - Add store-specific reset helpers (resetDownloadStore, resetRemoteServerStore, resetWhisperStore, etc) - Add act() wrapper utilities (actStoreUpdate, actAsyncStoreUpdate) to reduce boilerplate - Refactor remoteServerStore.test.ts to use shared actStoreUpdate() instead of 50+ act() calls - Refactor whisperStore.test.ts to use resetWhisperStore() from shared utilities - Change litert bundle ID from ai.offgridmobile to ai.offgridmobile.litert (allows side-by-side install with Play Store version) Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
|
|
Addressed Gemini feedback on improve-perf branch:
|
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (48.33%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## litertsupport #381 +/- ##
=================================================
+ Coverage 81.74% 81.80% +0.06%
=================================================
Files 241 241
Lines 12788 12836 +48
Branches 3521 3536 +15
=================================================
+ Hits 10453 10501 +48
+ Misses 1407 1403 -4
- Partials 928 932 +4
🚀 New features to boost your workflow:
|
1 participant
Summary
Type of Change
Overview
Performance optimization and stability improvements for LiteRT implementation with comprehensive test coverage additions.
Commit Breakdown (15 commits)
🎯 Feature Additions
feat(tools): on-device HTML parsing for read_url, improve tool prompts
read_urltoolfeat(perf): enable React 19 compiler
🔧 Core Fixes & Improvements
fix(litert): cap native tool calls at 3 per response
fix(models): show failed download state inline in model card with resume retry
fix(downloads): align ModelCard retry with DownloadManager and fix listener split
fix(downloads): use fresh store snapshot in watchDownload completion callback
fix(tools): fix amber dot position on settings gear icon
⚡ Performance Optimizations
perf(chat): fix ChatsListScreen re-renders and add OpenCL warning banner
perf: remove high-frequency debug logs from hot paths
🧹 Code Quality & Testing
test: disable React Compiler for Jest
test: push branch coverage to 80% with tests for recent changes
refactor(tests): consolidate test duplication and update litert bundle ID
mocks.ts, store-specific reset helpers)ai.offgridmobile.litertfor parallel installs🐛 Bug Fixes & Linting
fix(lint): remove unused colors variable and extract inline styles to StyleSheet
fix test (2x commits)
fix sonar errors
Object.assignwith object spread syntaxKey Improvements
Testing & Quality Gates
Notes for Review
Bundle ID changed to
ai.offgridmobile.litertto allow side-by-side installation with Play Store versionTest consolidation establishes shared utilities for future test development
All Gemini Code Assist feedback has been addressed
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactor (code change that neither fixes a bug nor adds a feature)
Chore (build process, CI, dependency updates, etc.)
Screenshots / Screen Recordings
Android
iOS
Checklist
General
Testing
npm test)React Native Specific
project.pbxproj)SPACING/TYPOGRAPHYconstants from the themeuseThemedStylespattern (not inline or staticStyleSheet.create)FlatList/FlashList(not.map()insideScrollView)Performance & Models
/vs\\)Security
Related Issues
Additional Notes