Simplify model selector to Quick/Powerful#432
Merged
AnthonyRonning merged 3 commits intomasterfrom Feb 24, 2026
Merged
Conversation
Replace 5 task-based categories with 2 primary options (Quick, Powerful) and a "More models" submenu. Quick maps to llama-3.3-70b, Powerful maps to kimi-k2-5. Image auto-switch is now tier-aware (Pro/Max/Team gets Powerful, Starter gets Qwen3-VL, Free gets no auto-switch). Remove unused QUICK_MODEL_ID from LocalStateContext. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis change replaces category-based model selection with two primary options (Quick, Powerful), relabels the advanced view to "More models", adds billing-aware vision-capable auto-switching, and consolidates defaults by replacing QUICK_MODEL_ID with DEFAULT_MODEL_ID in local state handling. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as ModelSelector (UI)
participant State as LocalStateContext
participant Billing as BillingService
participant Registry as ModelRegistry
UI->>State: user selects "Quick" or "Powerful" / opens "More models"
UI->>Registry: request model metadata (vision support, permissions)
Registry-->>UI: model metadata
alt image input present
UI->>Billing: fetch billing status
Billing-->>UI: billing tier (Free / Pro/Max/Team)
UI->>State: request vision-capable model switch (if needed and allowed)
State->>Registry: validate target model availability/permissions
Registry-->>State: allowed? / metadata
State-->>UI: confirm switch or show locked badge
end
UI->>State: set selected model
State-->>UI: update active model / availableModels
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
206
to
225
| useEffect(() => { | ||
| if (chatHasImages && hasAccessToModel(CATEGORY_MODELS.image)) { | ||
| // Only auto-switch if not already on a vision-capable model | ||
| const currentModelConfig = MODEL_CONFIG[model]; | ||
| if (!currentModelConfig?.supportsVision) { | ||
| setModel(CATEGORY_MODELS.image); | ||
| } | ||
| if (!chatHasImages) return; | ||
| const currentModelConfig = MODEL_CONFIG[model]; | ||
| if (currentModelConfig?.supportsVision) return; // Already on a vision model | ||
|
|
||
| const planName = billingStatus?.product_name?.toLowerCase() || ""; | ||
| const isProMaxOrTeam = | ||
| planName.includes("pro") || planName.includes("max") || planName.includes("team"); | ||
| const isStarter = planName.includes("starter"); | ||
|
|
||
| if (isProMaxOrTeam) { | ||
| // Pro/Max/Team: switch to Powerful (kimi-k2-5 has vision) | ||
| setModel(PRIMARY_MODELS.powerful); | ||
| } else if (isStarter) { | ||
| // Starter: switch to qwen3-vl-30b | ||
| setModel("qwen3-vl-30b"); | ||
| } | ||
| // Free: no auto-switch (existing upgrade prompt handles it) | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [chatHasImages]); |
Contributor
There was a problem hiding this comment.
🔴 Image auto-switch useEffect captures stale billingStatus (null) due to missing dependency
When a user uploads an image, the auto-switch useEffect reads billingStatus from its closure, but the dependency array is only [chatHasImages]. Since billingStatus starts as null (LocalStateContext.tsx:67) and is populated asynchronously via setBillingStatus after an API call, there is a race condition: if images are added before billing status loads, planName will be "", neither isProMaxOrTeam nor isStarter will be true, and no auto-switch occurs for any paid user. Once billingStatus later arrives, the effect does not re-run because chatHasImages hasn't changed.
Root Cause and Impact
The effect at ModelSelector.tsx:206-225 uses billingStatus (line 211) and model (line 208) from the component closure but only lists [chatHasImages] as a dependency (line 225), suppressing the exhaustive-deps lint rule.
The old code had the same [chatHasImages] dependency pattern, but it only called hasAccessToModel(CATEGORY_MODELS.image) which was a single yes/no gate. The new code introduces tier-aware branching (isProMaxOrTeam → kimi-k2-5, isStarter → qwen3-vl-30b) that is entirely dependent on billingStatus being populated. With a stale null value, the entire auto-switch feature silently fails for all paid tiers.
Impact: Pro/Max/Team users who upload images before billing status loads will not get auto-switched to the Powerful (kimi-k2-5) vision model. Starter users will similarly not get auto-switched to qwen3-vl-30b. The user would need to manually select a vision-capable model.
Suggested change
| useEffect(() => { | |
| if (chatHasImages && hasAccessToModel(CATEGORY_MODELS.image)) { | |
| // Only auto-switch if not already on a vision-capable model | |
| const currentModelConfig = MODEL_CONFIG[model]; | |
| if (!currentModelConfig?.supportsVision) { | |
| setModel(CATEGORY_MODELS.image); | |
| } | |
| if (!chatHasImages) return; | |
| const currentModelConfig = MODEL_CONFIG[model]; | |
| if (currentModelConfig?.supportsVision) return; // Already on a vision model | |
| const planName = billingStatus?.product_name?.toLowerCase() || ""; | |
| const isProMaxOrTeam = | |
| planName.includes("pro") || planName.includes("max") || planName.includes("team"); | |
| const isStarter = planName.includes("starter"); | |
| if (isProMaxOrTeam) { | |
| // Pro/Max/Team: switch to Powerful (kimi-k2-5 has vision) | |
| setModel(PRIMARY_MODELS.powerful); | |
| } else if (isStarter) { | |
| // Starter: switch to qwen3-vl-30b | |
| setModel("qwen3-vl-30b"); | |
| } | |
| // Free: no auto-switch (existing upgrade prompt handles it) | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [chatHasImages]); | |
| // Auto-switch to a vision-capable model when images are uploaded | |
| useEffect(() => { | |
| if (!chatHasImages) return; | |
| const currentModelConfig = MODEL_CONFIG[model]; | |
| if (currentModelConfig?.supportsVision) return; // Already on a vision model | |
| const planName = billingStatus?.product_name?.toLowerCase() || ""; | |
| const isProMaxOrTeam = | |
| planName.includes("pro") || planName.includes("max") || planName.includes("team"); | |
| const isStarter = planName.includes("starter"); | |
| if (isProMaxOrTeam) { | |
| // Pro/Max/Team: switch to Powerful (kimi-k2-5 has vision) | |
| setModel(PRIMARY_MODELS.powerful); | |
| } else if (isStarter) { | |
| // Starter: switch to qwen3-vl-30b | |
| setModel("qwen3-vl-30b"); | |
| } | |
| // Free: no auto-switch (existing upgrade prompt handles it) | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [chatHasImages, billingStatus]); | |
Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
There was a problem hiding this comment.
This is a pre-existing issue and out of scope for this PR.
Remove requiresPro restriction from gpt-oss-120b so it's free for everyone. Set it as DEFAULT_MODEL_ID and the Quick primary option, replacing llama-3.3-70b as the default. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Deploying maple with
|
| Latest commit: |
7959cb4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a510c74f.maple-ca8.pages.dev |
| Branch Preview URL: | https://model-selector-quick-powerfu.maple-ca8.pages.dev |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/state/LocalStateContext.tsx (1)
30-46:⚠️ Potential issue | 🟡 MinorPriority 2 billing check is now dead code; comment on line 45 is stale.
Since both Priority 2 (
planNamematches pro/max/team) and Priority 3 (the unconditional fallback) returnDEFAULT_MODEL_ID, the entirecachedBillingStatusblock is redundant — the result is the same regardless of cached billing plan. Additionally, the comment on line 45 says "Default to free model" butDEFAULT_MODEL_IDis now the universal default for all tiers, making it misleading.♻️ Proposed simplification
function getInitialModel(): string { // Check for dev override first if (import.meta.env.VITE_DEV_MODEL_OVERRIDE) { return aliasModelName(import.meta.env.VITE_DEV_MODEL_OVERRIDE); } try { - // Priority 1: Check local storage for last used model + // Check local storage for last used model const selectedModel = localStorage.getItem("selectedModel"); if (selectedModel) { return aliasModelName(selectedModel); } - - // Priority 2: Check cached billing status for pro/max/team users - const cachedBillingStr = localStorage.getItem("cachedBillingStatus"); - if (cachedBillingStr) { - const cachedBilling = JSON.parse(cachedBillingStr) as BillingStatus; - const planName = cachedBilling.product_name?.toLowerCase() || ""; - - // Pro, Max, or Team users get default model - if (planName.includes("pro") || planName.includes("max") || planName.includes("team")) { - return DEFAULT_MODEL_ID; - } - } } catch (error) { console.error("Failed to load initial model:", error); } - // Priority 3: Default to free model + // Default model for all users return DEFAULT_MODEL_ID; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/state/LocalStateContext.tsx` around lines 30 - 46, The cached billing branch (variables cachedBillingStr, cachedBilling, planName) is dead code because it ultimately returns DEFAULT_MODEL_ID just like the unconditional fallback; remove that entire Priority 2 block and its misleading comment, then update the remaining comment to state that DEFAULT_MODEL_ID is the universal default (or adjust the fallback if you intended different per-plan behavior); ensure any JSON.parse usages around cachedBillingStr are also removed so there are no unused variables left.frontend/src/components/ModelSelector.tsx (1)
363-366:⚠️ Potential issue | 🟡 Minor
showAdvancedis never reset when the dropdown dismisses without a selection
showAdvancedis component-level state that persists across dropdown open/close cycles. If the user navigates to "More models" then closes the dropdown by clicking outside or pressing Escape,showAdvancedremainstrue. The next time they open the dropdown they land directly on the "More models" panel instead of the primary Quick/Powerful view.🛠️ Proposed fix — reset on close via `onOpenChange`
- <DropdownMenu> + <DropdownMenu onOpenChange={(open) => { if (!open) setShowAdvanced(false); }}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ModelSelector.tsx` around lines 363 - 366, The component-level boolean showAdvanced persists when the dropdown closes without selection; add an onOpenChange handler to the DropdownMenu component to reset it: in DropdownMenu (wrapping DropdownMenuTrigger) implement onOpenChange={(open) => { if (!open) setShowAdvanced(false); }} so whenever the menu closes (including click-outside or Escape) you call setShowAdvanced(false) and restore the primary Quick/Powerful panel.
🧹 Nitpick comments (4)
frontend/src/state/LocalStateContext.tsx (2)
51-57:defaultModelis redefined on every render unnecessarily.The object depends only on the module-level constant
DEFAULT_MODEL_IDand fixed string literals, so it can be hoisted to module scope.♻️ Proposed fix — extract to module level
Add before
LocalStateProvider:+const defaultModel: OpenSecretModel = { + id: DEFAULT_MODEL_ID, + object: "model", + created: Date.now(), + owned_by: "openai", + tasks: ["generate"] +}; + export const LocalStateProvider = ({ children }: { children: React.ReactNode }) => { - /** The model that should be assumed when a chat doesn't yet have one */ - const defaultModel: OpenSecretModel = { - id: DEFAULT_MODEL_ID, - object: "model", - created: Date.now(), - owned_by: "openai", - tasks: ["generate"] - };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/state/LocalStateContext.tsx` around lines 51 - 57, The defaultModel object is being recreated on every render inside LocalStateProvider; hoist it to module scope by declaring a top-level const defaultModel: OpenSecretModel = { id: DEFAULT_MODEL_ID, object: "model", created: Date.now(), owned_by: "openai", tasks: ["generate"] } before the LocalStateProvider definition and then remove the in-component declaration so LocalStateProvider uses the module-level defaultModel constant.
183-188: Both branches callsetModel(DEFAULT_MODEL_ID)— simplify into a single condition.♻️ Proposed simplification
if (shouldUpdateModel) { - if (isProMaxOrTeam) { - setModel(DEFAULT_MODEL_ID); - } else if (billingChanged) { - // User downgraded, switch back to free model - setModel(DEFAULT_MODEL_ID); - } + if (isProMaxOrTeam || billingChanged) { + setModel(DEFAULT_MODEL_ID); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/state/LocalStateContext.tsx` around lines 183 - 188, The two branches both call setModel(DEFAULT_MODEL_ID); simplify by combining their conditions into a single if that checks (isProMaxOrTeam || billingChanged) and then calls setModel(DEFAULT_MODEL_ID). Update the logic around the existing conditional in LocalStateContext (referencing the isProMaxOrTeam and billingChanged variables and the setModel(DEFAULT_MODEL_ID) call) so there is no duplicate call path.frontend/src/components/ModelSelector.tsx (2)
214-220: Bare string"qwen3-vl-30b"in auto-switch duplicates a known model ID
PRIMARY_MODELSalready centralises the Quick/Powerful IDs. The Starter vision model used in auto-switch is a raw string that could silently diverge fromMODEL_CONFIGif the model is ever renamed.♻️ Suggested extraction
const PRIMARY_MODELS = { quick: "gpt-oss-120b", - powerful: "kimi-k2-5" + powerful: "kimi-k2-5", + starterVision: "qwen3-vl-30b" }; // ... in the effect: - setModel("qwen3-vl-30b"); + setModel(PRIMARY_MODELS.starterVision);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ModelSelector.tsx` around lines 214 - 220, The auto-switch currently uses a hard-coded string "qwen3-vl-30b" in the isStarter branch which risks drifting from your canonical model IDs; replace that literal with the centralized identifier (e.g., use PRIMARY_MODELS.starterVision or the corresponding entry from MODEL_CONFIG) so the starter vision model is sourced from the shared config instead of a bare string, updating the isStarter -> setModel call to reference that constant (keep references to isProMaxOrTeam, isStarter, setModel, and PRIMARY_MODELS/MODEL_CONFIG to locate the change).
457-470:modelcallback parameter shadows the outermodelstate variableIn three
.some()/.findIndex()callbacks the parameter is namedmodel, which shadows themodelvalue fromuseLocalState(). It doesn't produce a bug here, but it reduces clarity and can catch future refactors off-guard.♻️ Suggested rename
- .filter((m) => { - if (m.id === "leon-se/gemma-3-27b-it-fp8-dynamic") { - return !availableModels.some((model) => model.id === "gemma-3-27b"); - } - if (m.id === "ibnzterrell/Meta-Llama-3.3-70B-Instruct-AWQ-INT4") { - return !availableModels.some((model) => model.id === "llama-3.3-70b"); - } - return true; - }) + .filter((m) => { + if (m.id === "leon-se/gemma-3-27b-it-fp8-dynamic") { + return !availableModels.some((am) => am.id === "gemma-3-27b"); + } + if (m.id === "ibnzterrell/Meta-Llama-3.3-70B-Instruct-AWQ-INT4") { + return !availableModels.some((am) => am.id === "llama-3.3-70b"); + } + return true; + }) // Remove duplicates by id .filter( (m, index, self) => MODEL_CONFIG[m.id] !== undefined && - self.findIndex((model) => model.id === m.id) === index + self.findIndex((am) => am.id === m.id) === index )Also, the
MODEL_CONFIG[m.id] !== undefinedcheck on line 469 is redundant — the first.filteron line 455 already guarantees this.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ModelSelector.tsx` around lines 457 - 470, The callbacks inside the two filters in ModelSelector.tsx shadow the outer "model" state from useLocalState() — rename the inner callback parameters in the .some() and .findIndex() calls (e.g., use "candidate" or "otherModel" instead of "model") to avoid shadowing and improve clarity; additionally remove the redundant MODEL_CONFIG[m.id] !== undefined check from the second .filter since the earlier filter already enforces valid MODEL_CONFIG entries. Ensure references to availableModels.some(...) and self.findIndex(...) use the new parameter names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/ModelSelector.tsx`:
- Around line 203-223: The useEffect that auto-switches models when images are
uploaded reads billingStatus from a stale closure because its dependency array
only contains chatHasImages; add billingStatus to the dependency array and
restore the linter (remove the eslint-disable) so the effect re-runs when
billingStatus becomes available, and add an early return guard (e.g., if
(billingStatus == null) return) before computing planName to avoid premature
empty-plan behavior; keep model out of deps so user manual model changes aren't
overwritten, and continue using MODEL_CONFIG, setModel, and PRIMARY_MODELS as in
the existing code.
---
Outside diff comments:
In `@frontend/src/components/ModelSelector.tsx`:
- Around line 363-366: The component-level boolean showAdvanced persists when
the dropdown closes without selection; add an onOpenChange handler to the
DropdownMenu component to reset it: in DropdownMenu (wrapping
DropdownMenuTrigger) implement onOpenChange={(open) => { if (!open)
setShowAdvanced(false); }} so whenever the menu closes (including click-outside
or Escape) you call setShowAdvanced(false) and restore the primary
Quick/Powerful panel.
In `@frontend/src/state/LocalStateContext.tsx`:
- Around line 30-46: The cached billing branch (variables cachedBillingStr,
cachedBilling, planName) is dead code because it ultimately returns
DEFAULT_MODEL_ID just like the unconditional fallback; remove that entire
Priority 2 block and its misleading comment, then update the remaining comment
to state that DEFAULT_MODEL_ID is the universal default (or adjust the fallback
if you intended different per-plan behavior); ensure any JSON.parse usages
around cachedBillingStr are also removed so there are no unused variables left.
---
Nitpick comments:
In `@frontend/src/components/ModelSelector.tsx`:
- Around line 214-220: The auto-switch currently uses a hard-coded string
"qwen3-vl-30b" in the isStarter branch which risks drifting from your canonical
model IDs; replace that literal with the centralized identifier (e.g., use
PRIMARY_MODELS.starterVision or the corresponding entry from MODEL_CONFIG) so
the starter vision model is sourced from the shared config instead of a bare
string, updating the isStarter -> setModel call to reference that constant (keep
references to isProMaxOrTeam, isStarter, setModel, and
PRIMARY_MODELS/MODEL_CONFIG to locate the change).
- Around line 457-470: The callbacks inside the two filters in ModelSelector.tsx
shadow the outer "model" state from useLocalState() — rename the inner callback
parameters in the .some() and .findIndex() calls (e.g., use "candidate" or
"otherModel" instead of "model") to avoid shadowing and improve clarity;
additionally remove the redundant MODEL_CONFIG[m.id] !== undefined check from
the second .filter since the earlier filter already enforces valid MODEL_CONFIG
entries. Ensure references to availableModels.some(...) and self.findIndex(...)
use the new parameter names.
In `@frontend/src/state/LocalStateContext.tsx`:
- Around line 51-57: The defaultModel object is being recreated on every render
inside LocalStateProvider; hoist it to module scope by declaring a top-level
const defaultModel: OpenSecretModel = { id: DEFAULT_MODEL_ID, object: "model",
created: Date.now(), owned_by: "openai", tasks: ["generate"] } before the
LocalStateProvider definition and then remove the in-component declaration so
LocalStateProvider uses the module-level defaultModel constant.
- Around line 183-188: The two branches both call setModel(DEFAULT_MODEL_ID);
simplify by combining their conditions into a single if that checks
(isProMaxOrTeam || billingChanged) and then calls setModel(DEFAULT_MODEL_ID).
Update the logic around the existing conditional in LocalStateContext
(referencing the isProMaxOrTeam and billingChanged variables and the
setModel(DEFAULT_MODEL_ID) call) so there is no duplicate call path.
Comment on lines
+203
to
223
| // Auto-switch to a vision-capable model when images are uploaded | ||
| useEffect(() => { | ||
| if (chatHasImages && hasAccessToModel(CATEGORY_MODELS.image)) { | ||
| // Only auto-switch if not already on a vision-capable model | ||
| const currentModelConfig = MODEL_CONFIG[model]; | ||
| if (!currentModelConfig?.supportsVision) { | ||
| setModel(CATEGORY_MODELS.image); | ||
| } | ||
| if (!chatHasImages) return; | ||
| const currentModelConfig = MODEL_CONFIG[model]; | ||
| if (currentModelConfig?.supportsVision) return; // Already on a vision model | ||
|
|
||
| const planName = billingStatus?.product_name?.toLowerCase() || ""; | ||
| const isProMaxOrTeam = | ||
| planName.includes("pro") || planName.includes("max") || planName.includes("team"); | ||
| const isStarter = planName.includes("starter"); | ||
|
|
||
| if (isProMaxOrTeam) { | ||
| // Pro/Max/Team: switch to Powerful (kimi-k2-5 has vision) | ||
| setModel(PRIMARY_MODELS.powerful); | ||
| } else if (isStarter) { | ||
| // Starter: switch to qwen3-vl-30b | ||
| setModel("qwen3-vl-30b"); | ||
| } | ||
| // Free: no auto-switch (existing upgrade prompt handles it) | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [chatHasImages]); |
There was a problem hiding this comment.
Stale billingStatus closure can silently skip the auto-switch
The effect's dependency array is [chatHasImages] only, but billingStatus is read from a stale closure. If billingStatus hasn't loaded yet when chatHasImages first becomes true (e.g., an image is pasted immediately on page load), planName evaluates to "", neither branch fires, and the effect never re-runs because chatHasImages stays true. The // eslint-disable-next-line react-hooks/exhaustive-deps comment is the direct signal of this problem.
Adding billingStatus to the dep array (with an early-return guard) fixes the race without the need to suppress the linter:
🛠️ Proposed fix
useEffect(() => {
if (!chatHasImages) return;
+ if (!billingStatus) return; // wait for billing to load before deciding
const currentModelConfig = MODEL_CONFIG[model];
if (currentModelConfig?.supportsVision) return;
const planName = billingStatus?.product_name?.toLowerCase() || "";
const isProMaxOrTeam =
planName.includes("pro") || planName.includes("max") || planName.includes("team");
const isStarter = planName.includes("starter");
if (isProMaxOrTeam) {
setModel(PRIMARY_MODELS.powerful);
} else if (isStarter) {
setModel("qwen3-vl-30b");
}
- // eslint-disable-next-line react-hooks/exhaustive-deps
- }, [chatHasImages]);
+ }, [chatHasImages, billingStatus]);Note: intentionally keeping model out of deps is correct — including it would cause the effect to fight any manual model switch the user makes while images are present.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Auto-switch to a vision-capable model when images are uploaded | |
| useEffect(() => { | |
| if (chatHasImages && hasAccessToModel(CATEGORY_MODELS.image)) { | |
| // Only auto-switch if not already on a vision-capable model | |
| const currentModelConfig = MODEL_CONFIG[model]; | |
| if (!currentModelConfig?.supportsVision) { | |
| setModel(CATEGORY_MODELS.image); | |
| } | |
| if (!chatHasImages) return; | |
| const currentModelConfig = MODEL_CONFIG[model]; | |
| if (currentModelConfig?.supportsVision) return; // Already on a vision model | |
| const planName = billingStatus?.product_name?.toLowerCase() || ""; | |
| const isProMaxOrTeam = | |
| planName.includes("pro") || planName.includes("max") || planName.includes("team"); | |
| const isStarter = planName.includes("starter"); | |
| if (isProMaxOrTeam) { | |
| // Pro/Max/Team: switch to Powerful (kimi-k2-5 has vision) | |
| setModel(PRIMARY_MODELS.powerful); | |
| } else if (isStarter) { | |
| // Starter: switch to qwen3-vl-30b | |
| setModel("qwen3-vl-30b"); | |
| } | |
| // Free: no auto-switch (existing upgrade prompt handles it) | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [chatHasImages]); | |
| // Auto-switch to a vision-capable model when images are uploaded | |
| useEffect(() => { | |
| if (!chatHasImages) return; | |
| if (!billingStatus) return; // wait for billing to load before deciding | |
| const currentModelConfig = MODEL_CONFIG[model]; | |
| if (currentModelConfig?.supportsVision) return; // Already on a vision model | |
| const planName = billingStatus?.product_name?.toLowerCase() || ""; | |
| const isProMaxOrTeam = | |
| planName.includes("pro") || planName.includes("max") || planName.includes("team"); | |
| const isStarter = planName.includes("starter"); | |
| if (isProMaxOrTeam) { | |
| // Pro/Max/Team: switch to Powerful (kimi-k2-5 has vision) | |
| setModel(PRIMARY_MODELS.powerful); | |
| } else if (isStarter) { | |
| // Starter: switch to qwen3-vl-30b | |
| setModel("qwen3-vl-30b"); | |
| } | |
| // Free: no auto-switch (existing upgrade prompt handles it) | |
| }, [chatHasImages, billingStatus]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/ModelSelector.tsx` around lines 203 - 223, The
useEffect that auto-switches models when images are uploaded reads billingStatus
from a stale closure because its dependency array only contains chatHasImages;
add billingStatus to the dependency array and restore the linter (remove the
eslint-disable) so the effect re-runs when billingStatus becomes available, and
add an early return guard (e.g., if (billingStatus == null) return) before
computing planName to avoid premature empty-plan behavior; keep model out of
deps so user manual model changes aren't overwritten, and continue using
MODEL_CONFIG, setModel, and PRIMARY_MODELS as in the existing code.
Contributor
Author
There was a problem hiding this comment.
This is a pre-existing issue and out of scope for this PR.
There was a problem hiding this comment.
@marksftw, understood! Since this touches on functionality that could affect the image auto-switch behavior, would you like me to open a follow-up issue to track this for a future PR?
There was a problem hiding this comment.
@marksftw, got it—I'll leave this as-is. Feel free to reach out if you need anything else!
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/ModelSelector.tsx (1)
363-366:⚠️ Potential issue | 🟡 Minor
showAdvancedpersists across dropdown sessions — reset it on close.If a user navigates to "More models" and dismisses the dropdown (Escape or outside-click) without selecting, the next open still shows the full model list instead of the primary Quick/Powerful view.
🛠️ Proposed fix
- <DropdownMenu> + <DropdownMenu onOpenChange={(open) => { if (!open) setShowAdvanced(false); }}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ModelSelector.tsx` around lines 363 - 366, The dropdown's showAdvanced state (showAdvanced) isn't reset when the menu closes, so add a close handler on the DropdownMenu (use its onOpenChange or equivalent prop) that sets showAdvanced to false whenever open becomes false; update the component to pass a function like onOpenChange={(open) => { if (!open) setShowAdvanced(false); }} to the DropdownMenu (the DropdownMenu and showAdvanced state/setShowAdvanced variables identify where to make the change).
🧹 Nitpick comments (2)
frontend/src/components/ModelSelector.tsx (2)
391-393: Inconsistentboolean | undefinedtype forisDisabled; use|| falseto match the advanced list pattern.
targetModelConfig?.disabledisboolean | undefined, makingisDisabledboolean | undefined. The advanced-list code (line 505) correctly normalises with|| false. Aligning here satisfies strict TypeScript and avoids a type mismatch with thedisabledprop.♻️ Proposed fix
- const isDisabled = isDisabledDueToImages || targetModelConfig?.disabled; + const isDisabled = isDisabledDueToImages || (targetModelConfig?.disabled ?? false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ModelSelector.tsx` around lines 391 - 393, isDisabled is inferred as boolean | undefined because targetModelConfig?.disabled can be undefined; change its assignment in the ModelSelector component so it always yields a boolean by normalizing the optional value (e.g., use "|| false"). Specifically update the expression that builds isDisabled from isDisabledDueToImages and targetModelConfig?.disabled (and reference the isDisabledDueToImages variable and targetModelConfig?.disabled) to coerce the latter to a boolean (targetModelConfig?.disabled || false) so isDisabled is strictly boolean and matches the advanced-list pattern.
1-1: PreferChevronRightoverChevronLeft rotate-180for the "More models" row.The rotation works but
ChevronRightis semantically correct and avoids a confusing CSS transform for future readers.♻️ Proposed fix
-import { ChevronDown, Check, Lock, Camera, ChevronLeft, Zap, Brain } from "lucide-react"; +import { ChevronDown, Check, Lock, Camera, ChevronLeft, ChevronRight, Zap, Brain } from "lucide-react";- <ChevronLeft className="h-4 w-4 opacity-70 rotate-180" /> + <ChevronRight className="h-4 w-4 opacity-70" />Also applies to: 427-427
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ModelSelector.tsx` at line 1, Replace the rotated ChevronLeft icon usage with the semantically correct ChevronRight to represent the "More models" row; find the occurrence of ChevronLeft (used with a rotate-180 CSS transform) in the ModelSelector component and swap it for ChevronRight (imported from lucide-react) and remove the rotate-180 class/transform so the icon is not rotated manually.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend/src/components/ModelSelector.tsx`:
- Around line 363-366: The dropdown's showAdvanced state (showAdvanced) isn't
reset when the menu closes, so add a close handler on the DropdownMenu (use its
onOpenChange or equivalent prop) that sets showAdvanced to false whenever open
becomes false; update the component to pass a function like onOpenChange={(open)
=> { if (!open) setShowAdvanced(false); }} to the DropdownMenu (the DropdownMenu
and showAdvanced state/setShowAdvanced variables identify where to make the
change).
---
Nitpick comments:
In `@frontend/src/components/ModelSelector.tsx`:
- Around line 391-393: isDisabled is inferred as boolean | undefined because
targetModelConfig?.disabled can be undefined; change its assignment in the
ModelSelector component so it always yields a boolean by normalizing the
optional value (e.g., use "|| false"). Specifically update the expression that
builds isDisabled from isDisabledDueToImages and targetModelConfig?.disabled
(and reference the isDisabledDueToImages variable and
targetModelConfig?.disabled) to coerce the latter to a boolean
(targetModelConfig?.disabled || false) so isDisabled is strictly boolean and
matches the advanced-list pattern.
- Line 1: Replace the rotated ChevronLeft icon usage with the semantically
correct ChevronRight to represent the "More models" row; find the occurrence of
ChevronLeft (used with a rotate-180 CSS transform) in the ModelSelector
component and swap it for ChevronRight (imported from lucide-react) and remove
the rotate-180 class/transform so the icon is not rotated manually.
AnthonyRonning
approved these changes
Feb 24, 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
2 participants
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.
Closes #431
Summary
QUICK_MODEL_IDfrom LocalStateContextTest plan
Summary by CodeRabbit