release: onboarding reliability and post-0.0.103 fixes#571
Open
alichherawalla wants to merge 299 commits into
Open
release: onboarding reliability and post-0.0.103 fixes#571alichherawalla wants to merge 299 commits into
alichherawalla wants to merge 299 commits into
Conversation
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Important Review skippedToo many files! This PR contains 451 files, which is 151 over the limit of 300. To get a review, narrow the scope: Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (524)
You can disable this status message by setting the 📝 WalkthroughWalkthroughChangesThe pull request adds RAM-based model fit tiers and compact fit badges, introduces timeout-aware model-file loading with inline retry UI, makes onboarding device analysis non-blocking, updates total-RAM display, hardens download and release scripts, and modernizes numerous integration assertions and repository instructions. Model fit and onboarding
Reliability and maintenance
Repository guidance
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/services/huggingface.ts (1)
30-88: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winTimeout clears prematurely, leaving the body stream unprotected.
fetchWithTimeoutclears the timer as soon asfetchresolves (which happens when the HTTP headers are received). Consequently,await response.json()is executed after the timeout has been cleared. If the network connection drops or the server stalls while streaming the JSON body, the app will hang indefinitely onresponse.json(), bypassing the intended 5-second limit entirely.Refactor the timeout logic into a wrapper that keeps the
AbortSignalactive until the body is fully read.🛠️ Proposed fix using a holistic timeout wrapper
Replace
fetchWithTimeoutwithwithTimeout, and updatefetchJsonandgetModelFilesto wrap the entire parse operation:- private async fetchWithTimeout(url: string, timeoutMs = HF_REQUEST_TIMEOUT_MS): Promise<Response> { + private async withTimeout<T>(operation: (signal: AbortSignal) => Promise<T>, timeoutMs = HF_REQUEST_TIMEOUT_MS): Promise<T> { const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), timeoutMs); try { - return await fetch(url, { headers: { Accept: 'application/json' }, signal: controller.signal }); + return await operation(controller.signal); } finally { clearTimeout(timer); } } private async fetchJson<T>(url: string): Promise<T> { - const response = await this.fetchWithTimeout(url); - if (!response.ok) throw new Error(`API error: ${response.status}`); - return response.json() as Promise<T>; + return this.withTimeout(async (signal) => { + const response = await fetch(url, { headers: { Accept: 'application/json' }, signal }); + if (!response.ok) throw new Error(`API error: ${response.status}`); + return response.json(); + }); } // ... async getModelFiles(modelId: string): Promise<ModelFile[]> { try { - const response = await this.fetchWithTimeout(`${this.apiUrl}/models/${modelId}/tree/main`); - if (!response.ok) return this.getModelFilesFromSiblings(modelId); - const files: Array<{ type: string; path: string; size?: number; lfs?: { size: number } }> = await response.json(); - const allGguf = files.filter(f => f.type === 'file' && f.path.endsWith('.gguf')); - const mmProjFiles = allGguf.filter(f => this.isMMProjFile(f.path)); - const modelFiles = allGguf.filter(f => !this.isMMProjFile(f.path)); - return modelFiles - .map(file => ({ - name: file.path, - size: file.lfs?.size || file.size || 0, - quantization: this.extractQuantization(file.path), - downloadUrl: this.getDownloadUrl(modelId, file.path), - mmProjFile: this.findMatchingMMProj(file.path, mmProjFiles, modelId), - })) - .sort((a, b) => a.size - b.size); + return await this.withTimeout(async (signal) => { + const response = await fetch(`${this.apiUrl}/models/${modelId}/tree/main`, { headers: { Accept: 'application/json' }, signal }); + if (!response.ok) throw new Error('API Error'); // Triggers the catch block to run the sibling fallback + const files: Array<{ type: string; path: string; size?: number; lfs?: { size: number } }> = await response.json(); + const allGguf = files.filter(f => f.type === 'file' && f.path.endsWith('.gguf')); + const mmProjFiles = allGguf.filter(f => this.isMMProjFile(f.path)); + const modelFiles = allGguf.filter(f => !this.isMMProjFile(f.path)); + return modelFiles + .map(file => ({ + name: file.path, + size: file.lfs?.size || file.size || 0, + quantization: this.extractQuantization(file.path), + downloadUrl: this.getDownloadUrl(modelId, file.path), + mmProjFile: this.findMatchingMMProj(file.path, mmProjFiles, modelId), + })) + .sort((a, b) => a.size - b.size); + }); } catch (e) { // A timed-out/aborted request means the network is down — don't pay a second 5s on the🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/huggingface.ts` around lines 30 - 88, Refactor fetchWithTimeout into a withTimeout wrapper that keeps the AbortController active until the entire asynchronous operation completes, including response.json(). Update fetchJson and getModelFiles to wrap both fetching and body parsing in this timeout, while preserving existing non-OK handling, abort propagation, and siblings fallback behavior.src/screens/ModelsScreen/useTextModels.ts (1)
209-231: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winRace condition in
handleSelectModelstate updates.State updates are applied unconditionally after the async file fetch. If a user quickly navigates between different models, concurrent fetch responses can resolve out-of-order. This TOCTOU (Time-of-Check to Time-of-Use) race condition will overwrite
modelFileswith stale data, causing the detail view to display and eventually download files belonging to a completely different model.Guard the state updates to ensure only the latest active selection modifies the state.
🔒️ Proposed fix to ignore stale responses
(Note: ensure
useRefis included in yourreactimports)+ const fetchIdRef = useRef(0); + const handleSelectModel = async (model: ModelInfo) => { + fetchIdRef.current += 1; + const currentFetchId = fetchIdRef.current; + setSelectedModel(model); setIsLoadingFiles(true); setFilesLoadError(false); // Curated entries under the offgrid/ namespace (e.g. the synthetic LiteRT // parent) ship with their files baked into the ModelInfo — skip the // HuggingFace fetch and use them as-is. Real HF models always go through // the fetch path even when factories/mocks pre-populate model.files. if (model.id.startsWith('offgrid/') && model.files && model.files.length > 0) { setModelFiles(model.files); setIsLoadingFiles(false); return; } try { const files = await huggingFaceService.getModelFiles(model.id); + if (fetchIdRef.current !== currentFetchId) return; setModelFiles(files); } catch { // Fetch failed (offline / HF unreachable / timeout). Surface an inline retry state rather // than a transient modal — the detail view reads filesLoadError and offers Retry. + if (fetchIdRef.current !== currentFetchId) return; setModelFiles([]); setFilesLoadError(true); } finally { + if (fetchIdRef.current === currentFetchId) { setIsLoadingFiles(false); + } } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/screens/ModelsScreen/useTextModels.ts` around lines 209 - 231, Update handleSelectModel to track the latest selection with a useRef-based request or selection token, and verify that token before applying fetched files, errors, or loading-state updates. Ensure stale responses from prior model selections cannot modify modelFiles, filesLoadError, or loading state, while preserving the existing offgrid fast path and current behavior for the active selection.
🧹 Nitpick comments (1)
src/types/index.ts (1)
25-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer top-level
import typeover inline imports.Using a top-level type import is more idiomatic and improves readability compared to an inline
import()type inside the interface definition.♻️ Proposed refactor
Add the type import at the top of the file:
+import type { FitTier } from '../services/memoryBudget';And update the field definition:
/** Device-fit tier of this model's BEST (most-fitting) quant, for the browse fit chip. Set by the * models VM from memoryBudget.fitTier; undefined until the file sizes are known. */ - fitTier?: import('../services/memoryBudget').FitTier; + fitTier?: FitTier;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/types/index.ts` around lines 25 - 27, Replace the inline import type used by the fitTier property in the relevant interface with a top-level import type for FitTier from the memoryBudget service, then reference FitTier directly in the property declaration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@__tests__/unit/services/huggingface.test.ts`:
- Around line 596-604: Update the test around the custom global.fetch stub to
capture the original fetch implementation and restore it in a finally block,
ensuring restoration occurs whether the test passes, fails, or aborts.
Alternatively, replace the direct assignment with a jest.spyOn mock configured
with the existing behavior and ensure it is restored after the test.
In `@CLAUDE.md`:
- Around line 1-4: Update the references in batch9-diagnostics-debuglog.test.ts
and DEVICE_TEST_LOG.md that point to CLAUDE.md, replacing them with AGENTS.md so
debug-log and gate guidance directs contributors to the canonical source. Leave
unrelated repository instructions unchanged.
---
Outside diff comments:
In `@src/screens/ModelsScreen/useTextModels.ts`:
- Around line 209-231: Update handleSelectModel to track the latest selection
with a useRef-based request or selection token, and verify that token before
applying fetched files, errors, or loading-state updates. Ensure stale responses
from prior model selections cannot modify modelFiles, filesLoadError, or loading
state, while preserving the existing offgrid fast path and current behavior for
the active selection.
In `@src/services/huggingface.ts`:
- Around line 30-88: Refactor fetchWithTimeout into a withTimeout wrapper that
keeps the AbortController active until the entire asynchronous operation
completes, including response.json(). Update fetchJson and getModelFiles to wrap
both fetching and body parsing in this timeout, while preserving existing non-OK
handling, abort propagation, and siblings fallback behavior.
---
Nitpick comments:
In `@src/types/index.ts`:
- Around line 25-27: Replace the inline import type used by the fitTier property
in the relevant interface with a top-level import type for FitTier from the
memoryBudget service, then reference FitTier directly in the property
declaration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4b441aab-6882-473d-8878-0f40b74e98ce
📒 Files selected for processing (63)
AGENTS.mdCLAUDE.md__tests__/integration/audio/voiceModeImageJourney.rendered.happy.test.tsx__tests__/integration/audio/voiceModeResendEnhancedImage.rendered.redflow.test.tsx__tests__/integration/audio/voiceModeResendImageRoutes.rendered.redflow.test.tsx__tests__/integration/chat/voiceNoteChatModeEmptyTurn.redflow.test.ts__tests__/integration/downloads/downloadedCountBadge.rendered.happy.test.tsx__tests__/integration/downloads/iosImageStagingPurgedRedownloads.redflow.test.ts__tests__/integration/downloads/iosImageStagingPurgedRedownloads.rendered.redflow.test.tsx__tests__/integration/downloads/iosInterruptedNoFailedEntry.redflow.test.ts__tests__/integration/downloads/iosTextRetryReissues.rendered.redflow.test.tsx__tests__/integration/downloads/queuedDownloadsSurviveKill.redflow.test.ts__tests__/integration/downloads/textRetryReissuesOnIosLostDownloadId.rendered.redflow.test.tsx__tests__/integration/generation/enhancementNoThinking.rendered.redflow.test.tsx__tests__/integration/generation/enhancementStreamingProgress.rendered.redflow.test.tsx__tests__/integration/generation/generationFlow.test.ts__tests__/integration/generation/imageGenerationFlow.test.ts__tests__/integration/generation/queuedForceImagePreservesMode.rendered.redflow.test.tsx__tests__/integration/generation/queuedSendFeedback.test.ts__tests__/integration/generation/reloadRaceKeepsThinking.rendered.redflow.test.tsx__tests__/integration/generation/remoteParallelTools.rendered.happy.test.tsx__tests__/integration/generation/resendImageRoutes.rendered.redflow.test.tsx__tests__/integration/generation/resendImageRoutesLlama.rendered.redflow.test.tsx__tests__/integration/generation/resendImageTurnFallsBackToText.rendered.redflow.test.tsx__tests__/integration/happy/imageBackends.happy.test.tsx__tests__/integration/happy/imageIntentRouting.happy.test.tsx__tests__/integration/happy/imageLightbox.happy.test.tsx__tests__/integration/happy/imageModeToggle.happy.test.tsx__tests__/integration/happy/imageOomCard.happy.test.tsx__tests__/integration/happy/smartBudgeting.happy.test.tsx__tests__/integration/happy/speakMessage.happy.test.tsx__tests__/integration/happy/tools.happy.test.tsx__tests__/integration/image/imageTunablesReadFreshFromStore.redflow.test.ts__tests__/integration/models/activeModelService.test.ts__tests__/integration/models/browseFitChipShowsLoadableModels.rendered.test.tsx__tests__/integration/models/detailFilesFitHintUsesOwnedBudget.rendered.test.tsx__tests__/integration/models/modelFilesLoadErrorShowsRetry.rendered.test.tsx__tests__/integration/onboarding/deviceAnalysisDoesNotWaitForNetwork.rendered.test.tsx__tests__/integration/onboarding/deviceCardShowsTotalRam.rendered.test.tsx__tests__/integration/onboarding/spotlightFlowIntegration.test.ts__tests__/integration/rag/embeddingFlow.test.ts__tests__/integration/rag/ragFlow.test.ts__tests__/integration/stores/chatStoreIntegration.test.ts__tests__/rntl/screens/ModelDownloadScreen.test.tsx__tests__/unit/services/huggingface.test.ts__tests__/unit/services/modelManager.test.tsandroid/app/src/main/java/ai/offgridmobile/download/DownloadManagerModule.ktandroid/app/src/main/java/ai/offgridmobile/download/WorkerDownload.ktfastlane/Fastfilescripts/uat.shsrc/components/ModelCardContent.tsxsrc/screens/DownloadManagerScreen/useVoiceDownloadItems.tssrc/screens/ModelDownloadScreen.tsxsrc/screens/ModelsScreen/TextModelsTab.tsxsrc/screens/ModelsScreen/index.tsxsrc/screens/ModelsScreen/styles.tssrc/screens/ModelsScreen/useModelsScreen.tssrc/screens/ModelsScreen/useTextModels.tssrc/services/activeDownloadPersistence.tssrc/services/huggingface.tssrc/services/memoryBudget.tssrc/services/modelPreloader.tssrc/types/index.ts
This was referenced Jul 16, 2026
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Consolidates the onboarding reliability fix and the recent post-0.0.103 work into one release candidate.
Easy,Fits,Tight) instead of hiding every model beyond the balanced target.AGENTS.mdthe canonical concise engineering contract: SOLID/DRY/separation of concerns, no mocks of Off Grid code, real screen/navigation/gesture integration tests, and fakes only at uncontrollable boundaries.CLAUDE.mdnow points to it.Type of Change
Screenshots / Screen Recordings
The UI regressions are covered by rendered integration tests. Manual screenshot capture remains for release QA.
Android
Analyzing your device...11.00 GB RAMiOS
Analyzing your device...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
Supersedes #560, #561, #568, and #569.
Additional Notes
Local verification:
BUILD SUCCESSFUL, 589 tasks).Summary by CodeRabbit
New Features
Bug Fixes
Documentation