feat(models): device-fit chips — show loadable models with Easy/Fits/Tight instead of hiding them#561
Closed
alichherawalla wants to merge 4 commits into
Closed
feat(models): device-fit chips — show loadable models with Easy/Fits/Tight instead of hiding them#561alichherawalla wants to merge 4 commits into
alichherawalla wants to merge 4 commits into
Conversation
…fit) from the owned budgets
…w snug the fit is Browse used to HIDE any model whose quants all exceeded the balanced budget (fileExceedsBudget), so a model that's loadable via reclaim credit + Load Anyway just vanished. Now browse keeps every model with a quant that isn't 'wontFit' (past the aggressive ceiling) and tags it with a small device-fit chip: Easy / Fits / Tight (fitTier owner). Only genuinely-too-big models are hidden. - useTextModels: relax the browse hide-filter to noLoadableQuant (every quant 'wontFit'); attach each model's best-quant fitTier for the chip. Recommended stays filtered by deviceRecommendation (curated cap) and search/sort are untouched. - ModelCardContent: render the fit chip in the badges row (InfoBadgesRow extracted to module scope to stay under the complexity gate); emerald for easy/fits, muted for a snug 'tight'. - TextModelsTab detail 'Available Files': relax the same way so a 'tight' model opened from browse still lists a downloadable quant (isCompatible still flags snug ones for the Load-Anyway warning). Rendered tests: a tight model now shows in browse with its chip (red-verified: revert → hidden); the detail list offers loadable quants and hides only 'wontFit' (boundary pinned at 8GB×0.75 = exactly 6.0GB). Note: cosmetic chip colors/placement want an on-device eyeball. Image-tab parity + the Home picker chip are follow-ups (same owner).
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughModel file sizing now produces RAM fit tiers. Browse results retain models with at least one loadable quant and expose fit-tier metadata for compact-card chips. Detail file lists use the same loadability verdict, with integration tests covering tight models and aggressive-ceiling boundaries. ChangesFit-tier model browsing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant HuggingFace
participant useTextModels
participant fitTier
participant ModelsScreen
participant ModelCardContent
HuggingFace->>useTextModels: model metadata and quantized file sizes
useTextModels->>fitTier: classify quant file sizes
fitTier-->>useTextModels: fit tiers
useTextModels-->>ModelsScreen: filtered models with fitTier
ModelsScreen-->>ModelCardContent: render model card
ModelCardContent-->>ModelCardContent: display fit-tier chip
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: 1
🤖 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 `@src/components/ModelCardContent.tsx`:
- Around line 129-134: Update the conditional rendering for model.paramCount and
model.minRamGB in ModelCardContent so numeric zero values do not render directly
as children of View. Use explicit presence checks that render each badge only
when its value is defined or otherwise valid, while preserving the existing
badge text and behavior for positive values.
🪄 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: 6e3d6e99-16db-4702-9d59-795d7965244f
📒 Files selected for processing (7)
__tests__/integration/models/browseFitChipShowsLoadableModels.rendered.test.tsx__tests__/integration/models/detailFilesFitHintUsesOwnedBudget.rendered.test.tsxsrc/components/ModelCardContent.tsxsrc/screens/ModelsScreen/TextModelsTab.tsxsrc/screens/ModelsScreen/useTextModels.tssrc/services/memoryBudget.tssrc/types/index.ts
The 'loadable = fitTier !== wontFit' rule was written inline in two callers (useTextModels browse filter + TextModelsTab detail list). Move it to one owned predicate memoryBudget.isLoadableOnDevice so 'what counts as loadable on this device' is defined once; both surfaces call it. fitTier still owns the tier value for the chip. No behaviour change (verified: fit tests green).
…CodeRabbit)
{model.paramCount && …} rendered a bare '0' text node outside <Text> when paramCount/minRamGB was 0,
which crashes RN. Coerce with !! so 0/undefined render nothing (a 0-value badge is meaningless anyway).
|
26 tasks
Collaborator
Author
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.
The parked fit-chip UX, built fresh as its own feature PR.
Problem
Browse HID any model whose quants all exceeded the balanced budget (
fileExceedsBudget). But the load path can reach the aggressive ceiling (Android reclaim credit + Load Anyway), so genuinely-loadable models just vanished from browse — and if one slipped through, tapping it opened an empty file list (its quants were hidden too).Change
Browse now keeps every model with a quant that isn't
wontFit(past the aggressive ceiling) and tags it with a small device-fit chip — Easy / Fits / Tight. Only genuinely-too-big models are hidden.memoryBudget.fitTier(Easy/Fits/Tight/Won't fit) from the SAME owned budgets — soft = balanced fraction, ceil = aggressive fraction. No new magic numbers.noLoadableQuant(hide only when every quant is wontFit); attach each model's best-quant tier for the chip. Recommended stays filtered (bydeviceRecommendation— the curated param cap) and search/sort are untouched.InfoBadgesRowto module scope to stay under the complexity gate.isCompatiblestill flags snug ones for the Load-Anyway warning (the 'slight relaxation', not a free pass).Tests (rendered, red-verified)
wontFit; the boundary is pinned at 8GB × 0.75 = exactly 6.0GB (integer bytes) to kill a</<=mutant.Follow-ups (noted, not in scope)
fitTierowner).Summary by CodeRabbit