Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 46 additions & 85 deletions frontend/src/components/ModelSelector.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,4 @@
import {
ChevronDown,
Check,
Lock,
Camera,
ChevronLeft,
Sparkles,
Zap,
Code,
Image,
Brain
} from "lucide-react";
import { ChevronDown, Check, Lock, Camera, ChevronLeft, Zap, Brain } from "lucide-react";
import { Button } from "@/components/ui/button";
import {
DropdownMenu,
Expand Down Expand Up @@ -79,8 +68,6 @@ export const MODEL_CONFIG: Record<string, ModelCfg> = {
"gpt-oss-120b": {
displayName: "OpenAI GPT-OSS 120B",
shortName: "GPT-OSS",
badges: ["Pro"],
requiresPro: true,
tokenLimit: 128000
},
"qwen3-vl-30b": {
Expand All @@ -100,42 +87,22 @@ export function getModelTokenLimit(modelId: string): number {
return MODEL_CONFIG[modelId]?.tokenLimit || DEFAULT_TOKEN_LIMIT;
}

// Model categories for simplified UI
type ModelCategory = "free" | "quick" | "reasoning" | "math" | "image" | "advanced";

export const CATEGORY_MODELS = {
free: "llama-3.3-70b",
// Primary model options
const PRIMARY_MODELS = {
quick: "gpt-oss-120b",
reasoning: "deepseek-r1-0528",
math: "kimi-k2-5",
image: "qwen3-vl-30b" // Qwen3-VL for image analysis
powerful: "kimi-k2-5"
};

const CATEGORY_INFO = {
free: {
label: "General",
icon: Sparkles,
description: "Balanced & capable"
},
const PRIMARY_INFO = {
quick: {
label: "Quick",
icon: Zap,
description: "Fastest responses"
description: "Fast, everyday responses"
},
reasoning: {
label: "Reasoning",
powerful: {
label: "Powerful",
icon: Brain,
description: "Deep analysis"
},
math: {
label: "Math/Coding",
icon: Code,
description: "Technical tasks"
},
image: {
label: "Image Analysis",
icon: Image,
description: "Vision & analysis"
description: "Deeper thinking & analysis"
}
};

Expand Down Expand Up @@ -233,26 +200,32 @@ export function ModelSelector({ hasImages = false }: { hasImages?: boolean }) {
}
}, [os, setAvailableModels, setHasWhisperModel]);

// Auto-switch to image analysis when images are uploaded
// 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]);
Comment on lines 204 to 223
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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]);
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pre-existing issue and out of scope for this PR.

Comment on lines +203 to 223
Copy link

@coderabbitai coderabbitai bot Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pre-existing issue and out of scope for this PR.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marksftw, got it—I'll leave this as-is. Feel free to reach out if you need anything else!


// Get current category based on selected model
const getCurrentCategory = (): string => {
if (model === CATEGORY_MODELS.free) return "General";
if (model === CATEGORY_MODELS.quick) return "Quick";
if (model === CATEGORY_MODELS.reasoning) return "Reasoning";
if (model === CATEGORY_MODELS.math) return "Math/Coding";
if (model === CATEGORY_MODELS.image) return "Image Analysis";
// If in advanced mode, show model name
// Get dropdown button label based on selected model
const getDropdownLabel = (): string => {
if (model === PRIMARY_MODELS.quick) return "Quick";
if (model === PRIMARY_MODELS.powerful) return "Powerful";
const config = MODEL_CONFIG[model];
return config?.displayName || model;
};
Expand Down Expand Up @@ -284,14 +257,9 @@ export function ModelSelector({ hasImages = false }: { hasImages?: boolean }) {
return true;
};

// Handle category selection
const handleCategorySelect = (category: ModelCategory) => {
if (category === "advanced") {
setShowAdvanced(true);
return;
}

const targetModel = CATEGORY_MODELS[category as keyof typeof CATEGORY_MODELS];
// Handle primary option selection
const handlePrimarySelect = (key: "quick" | "powerful") => {
const targetModel = PRIMARY_MODELS[key];

// Prevent switching to non-vision models if chat has images
const targetModelConfig = MODEL_CONFIG[targetModel];
Expand Down Expand Up @@ -386,7 +354,7 @@ export function ModelSelector({ hasImages = false }: { hasImages?: boolean }) {
// Show current category or model name in the collapsed view
const modelDisplay = (
<div className="flex items-center gap-1">
<div className="text-xs font-medium">{getCurrentCategory()}</div>
<div className="text-sm font-medium">{getDropdownLabel()}</div>
</div>
);

Expand All @@ -409,32 +377,25 @@ export function ModelSelector({ hasImages = false }: { hasImages?: boolean }) {
</DropdownMenuTrigger>
<DropdownMenuContent align="start" className="w-64 p-0">
{!showAdvanced ? (
<div className="p-1 h-[300px] flex flex-col">
{/* Category options */}
{(["free", "quick", "reasoning", "math", "image"] as const).map((category) => {
const info = CATEGORY_INFO[category];
<div className="p-1 flex flex-col">
{/* Primary options */}
{(["quick", "powerful"] as const).map((key) => {
const info = PRIMARY_INFO[key];
const Icon = info.icon;
const isActive =
(category === "free" && model === CATEGORY_MODELS.free) ||
(category === "quick" && model === CATEGORY_MODELS.quick) ||
(category === "reasoning" && model === CATEGORY_MODELS.reasoning) ||
(category === "math" && model === CATEGORY_MODELS.math) ||
(category === "image" && model === CATEGORY_MODELS.image);

// Check if user has access to this category's model
const targetModel = CATEGORY_MODELS[category];
const targetModel = PRIMARY_MODELS[key];
const isActive = model === targetModel;
const hasAccess = hasAccessToModel(targetModel);
const targetModelConfig = MODEL_CONFIG[targetModel];
const requiresUpgrade = !hasAccess;

// Disable non-vision categories if chat has images
// Disable non-vision options if chat has images
const isDisabledDueToImages = chatHasImages && !targetModelConfig?.supportsVision;
const isDisabled = isDisabledDueToImages || targetModelConfig?.disabled;

return (
<DropdownMenuItem
key={category}
onClick={() => handleCategorySelect(category)}
key={key}
onClick={() => handlePrimarySelect(key)}
className={`flex items-center gap-2 px-3 py-1.5 cursor-pointer ${
isDisabled ? "opacity-50 cursor-not-allowed" : ""
} ${requiresUpgrade ? "hover:bg-purple-50 dark:hover:bg-purple-950/20" : ""}`}
Expand All @@ -455,7 +416,7 @@ export function ModelSelector({ hasImages = false }: { hasImages?: boolean }) {

<DropdownMenuSeparator />

{/* Advanced option */}
{/* More models option */}
<DropdownMenuItem
onSelect={(e) => {
e.preventDefault();
Expand All @@ -465,13 +426,13 @@ export function ModelSelector({ hasImages = false }: { hasImages?: boolean }) {
>
<ChevronLeft className="h-4 w-4 opacity-70 rotate-180" />
<div className="flex-1">
<span className="text-sm font-medium">Advanced</span>
<span className="text-sm font-medium">More models</span>
<div className="text-xs text-muted-foreground">All models</div>
</div>
</DropdownMenuItem>
</div>
) : (
<div className="p-1 h-[300px] flex flex-col">
<div className="p-1 flex flex-col">
{/* Back button */}
<DropdownMenuItem
onSelect={(e) => {
Expand Down
15 changes: 7 additions & 8 deletions frontend/src/state/LocalStateContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ export {
type LocalState
} from "./LocalStateContextDef";

export const DEFAULT_MODEL_ID = "llama-3.3-70b";
const QUICK_MODEL_ID = "gpt-oss-120b";
export const DEFAULT_MODEL_ID = "gpt-oss-120b";

// Helper to get default model based on cached billing status
function getInitialModel(): string {
Expand All @@ -34,9 +33,9 @@ function getInitialModel(): string {
const cachedBilling = JSON.parse(cachedBillingStr) as BillingStatus;
const planName = cachedBilling.product_name?.toLowerCase() || "";

// Pro, Max, or Team users get Quick model
// Pro, Max, or Team users get default model
if (planName.includes("pro") || planName.includes("max") || planName.includes("team")) {
return QUICK_MODEL_ID;
return DEFAULT_MODEL_ID;
}
}
} catch (error) {
Expand All @@ -49,11 +48,11 @@ function getInitialModel(): string {

export const LocalStateProvider = ({ children }: { children: React.ReactNode }) => {
/** The model that should be assumed when a chat doesn't yet have one */
const llamaModel: OpenSecretModel = {
const defaultModel: OpenSecretModel = {
id: DEFAULT_MODEL_ID,
object: "model",
created: Date.now(),
owned_by: "meta",
owned_by: "openai",
tasks: ["generate"]
};

Expand All @@ -63,7 +62,7 @@ export const LocalStateProvider = ({ children }: { children: React.ReactNode })
userImages: [] as File[],
sentViaVoice: false,
model: getInitialModel(),
availableModels: [llamaModel] as OpenSecretModel[],
availableModels: [defaultModel] as OpenSecretModel[],
hasWhisperModel: true, // Default to true to avoid hiding button during loading
billingStatus: null as BillingStatus | null,
searchQuery: "",
Expand Down Expand Up @@ -182,7 +181,7 @@ export const LocalStateProvider = ({ children }: { children: React.ReactNode })

if (shouldUpdateModel) {
if (isProMaxOrTeam) {
setModel(QUICK_MODEL_ID);
setModel(DEFAULT_MODEL_ID);
} else if (billingChanged) {
// User downgraded, switch back to free model
setModel(DEFAULT_MODEL_ID);
Expand Down
Loading