release: onboarding reliability and post-0.0.103 fixes#571
Open
alichherawalla wants to merge 300 commits into
Open
release: onboarding reliability and post-0.0.103 fixes#571alichherawalla wants to merge 300 commits into
alichherawalla wants to merge 300 commits into
Conversation
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
📝 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 | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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
Release candidate consolidating the onboarding reliability fix and all post-0.0.103 work onto
main.Headline changes:
11.00 GB, not the 4.5–4.9 GB process-available ceiling).Easy,Fits,Tight) instead of hiding every model beyond the balanced target.AGENTS.mdthe canonical engineering contract;CLAUDE.mdpoints to it.Type of Change
Test coverage — automated vs. manual
Coverage is mapped per check in
docs/RELEASE_TEST_CHECKLIST.csv(247 rows, each tagged with an Automation %). The targeted device pass isdocs/PR_571_TARGETED_MANUAL_TESTS.md; the tonight-ship check-off sheet isdocs/RELEASE_TONIGHT_CHECKOFF.md.Covered by automated integration tests
327 real-screen / real-navigation / real-gesture integration suites (fakes only at the device boundary). Of 247 release checks: 72 fully automated, 107 at 70–99%, 55 partial, 13 at 0%. Strongest coverage: prompt enhancement (100%), image gen (90%), tools (90%), KB/projects (86%), remote (83%), text gen (80%).
Must be validated manually on device
Checklist
Testing
npm test)Security
Related Issues
Supersedes #560, #561, #568, #569.
Additional Notes
Local verification:
BUILD SUCCESSFUL, 589 tasks).