Skip to content

feat: implement default model selection based on user plan #186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
marksftw wants to merge 2 commits into from
Closed

feat: implement default model selection based on user plan #186

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e54922b
feat: implement default model selection based on user plan
claude[bot] Aug 9, 2025
fb2997c
fix: address code review comments and implement plan upgrade scenario
claude[bot] Aug 10, 2025
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
25 changes: 2 additions & 23 deletions frontend/src/components/ModelSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { useOpenSecret } from "@opensecret/react";
import { useEffect, useRef } from "react";
import { useNavigate } from "@tanstack/react-router";
import type { Model } from "openai/resources/models.js";
import { hasAccessToModel as checkModelAccess } from "@/utils/modelDefaults";

Comment thread
coderabbitai[bot] marked this conversation as resolved.
// Model configuration for display names, badges, and token limits
type ModelCfg = {
Expand Down Expand Up @@ -177,29 +178,7 @@ export function ModelSelector({

// Check if user has access to a model based on their plan
const hasAccessToModel = (modelId: string) => {
const config = MODEL_CONFIG[modelId];

// If no restrictions, allow access
if (!config?.requiresPro && !config?.requiresStarter) return true;

const planName = billingStatus?.product_name?.toLowerCase() || "";

// Check if user is on Pro, Max, or Team plan (for requiresPro models)
if (config?.requiresPro) {
return planName.includes("pro") || planName.includes("max") || planName.includes("team");
}

// Check if user is on Starter, Pro, Max, or Team plan (for requiresStarter models)
if (config?.requiresStarter) {
return (
planName.includes("starter") ||
planName.includes("pro") ||
planName.includes("max") ||
planName.includes("team")
);
}

return true;
return checkModelAccess(modelId, billingStatus);
};

const getDisplayName = (modelId: string, showLock = false) => {
Expand Down
61 changes: 59 additions & 2 deletions frontend/src/state/LocalStateContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { useOpenSecret } from "@opensecret/react";
import { useState } from "react";
import { useState, useEffect } from "react";
import { BillingStatus } from "@/billing/billingApi";
import { LocalStateContext, Chat, HistoryItem, OpenSecretModel } from "./LocalStateContextDef";
import { aliasModelName } from "@/utils/utils";
import { getDefaultModelForUser } from "@/utils/modelDefaults";

export {
LocalStateContext,
Expand Down Expand Up @@ -38,6 +39,55 @@ export const LocalStateProvider = ({ children }: { children: React.ReactNode })

const { get, put, list, del } = useOpenSecret();

// Load last used model when component mounts
useEffect(() => {
async function loadLastUsedModel() {
try {
const lastUsedModel = await get("last_used_model");
if (lastUsedModel) {
const aliasedModel = aliasModelName(lastUsedModel);
setLocalState((prev) => ({
...prev,
model: aliasedModel
}));
}
Comment on lines +70 to +88

@coderabbitai coderabbitai Bot Aug 10, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix type mismatch for previousPlanTier (resolves TS2345 at Lines 77 and 84)

get("previous_plan_tier") returns string | null, but state expects "free" | "starter" | "pro" | null. Validate/narrow before setting.

-        const [lastUsedModel, storedPlanTier] = await Promise.all([
+        const [lastUsedModel, storedPlanTier] = await Promise.all([
           get("last_used_model"),
           get("previous_plan_tier")
         ]);
 
-        if (lastUsedModel) {
+        const parsedPlanTier =
+          storedPlanTier === "free" || storedPlanTier === "starter" || storedPlanTier === "pro"
+            ? (storedPlanTier as "free" | "starter" | "pro")
+            : null;
+
+        if (lastUsedModel) {
           const aliasedModel = aliasModelName(lastUsedModel);
           // Validate stored model before using it
           if (
             MODEL_CONFIG[aliasedModel] &&
             hasAccessToModel(aliasedModel, localState.billingStatus)
           ) {
             setLocalState((prev) => ({
               ...prev,
               model: aliasedModel,
-              previousPlanTier: storedPlanTier || null
+              previousPlanTier: parsedPlanTier
             }));
           }
-        } else if (storedPlanTier) {
+        } else if (parsedPlanTier) {
           setLocalState((prev) => ({
             ...prev,
-            previousPlanTier: storedPlanTier
+            previousPlanTier: parsedPlanTier
           }));
         }
📝 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
if (lastUsedModel) {
const aliasedModel = aliasModelName(lastUsedModel);
// Validate stored model before using it
if (
MODEL_CONFIG[aliasedModel] &&
hasAccessToModel(aliasedModel, localState.billingStatus)
) {
setLocalState((prev) => ({
...prev,
model: aliasedModel,
previousPlanTier: storedPlanTier || null
}));
}
} else if (storedPlanTier) {
setLocalState((prev) => ({
...prev,
previousPlanTier: storedPlanTier
}));
}
const [lastUsedModel, storedPlanTier] = await Promise.all([
get("last_used_model"),
get("previous_plan_tier")
]);
const parsedPlanTier =
storedPlanTier === "free" ||
storedPlanTier === "starter" ||
storedPlanTier === "pro"
? (storedPlanTier as "free" | "starter" | "pro")
: null;
if (lastUsedModel) {
const aliasedModel = aliasModelName(lastUsedModel);
// Validate stored model before using it
if (
MODEL_CONFIG[aliasedModel] &&
hasAccessToModel(aliasedModel, localState.billingStatus)
) {
setLocalState((prev) => ({
...prev,
model: aliasedModel,
previousPlanTier: parsedPlanTier
}));
}
} else if (parsedPlanTier) {
setLocalState((prev) => ({
...prev,
previousPlanTier: parsedPlanTier
}));
}
🧰 Tools
🪛 GitHub Actions: BrowserUse Tests

[error] 77-77: TypeScript error TS2345: Argument of type '(prev: {...}) => {...}' is not assignable to parameter of type 'SetStateAction<{...}>' in state setter function.

🪛 GitHub Actions: Mobile App CI

[error] 77-77: TypeScript error TS2345: Argument type is not assignable to parameter type due to incompatible 'previousPlanTier' types. Expected '"free" | "starter" | "pro" | null' but got 'string | null'.

[error] 84-84: TypeScript error TS2345: Argument type is not assignable to parameter type due to incompatible 'previousPlanTier' types. Expected '"free" | "starter" | "pro" | null' but got 'string'.

🤖 Prompt for AI Agents 
In frontend/src/state/LocalStateContext.tsx around lines 70 to 88, the value
assigned to previousPlanTier comes from get("previous_plan_tier") which returns
string | null, but the state expects a more specific union type "free" |
"starter" | "pro" | null. To fix this, validate or narrow the storedPlanTier
value before setting it in state by checking if it matches one of the allowed
string literals; if it does, assign it, otherwise assign null.

} catch (error) {
console.error("Error loading last used model:", error);
}
}

loadLastUsedModel();
}, [get]);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

// Update default model when billing status changes
useEffect(() => {
async function updateDefaultModel() {
if (localState.billingStatus) {
try {
const lastUsedModel = await get("last_used_model");
const defaultModel = getDefaultModelForUser(
localState.billingStatus,
lastUsedModel || null
);

// Only update if the current model is still the initial default
// and we have a better default based on the user's plan
if (localState.model === DEFAULT_MODEL_ID || !lastUsedModel) {
const aliasedModel = aliasModelName(defaultModel);
setLocalState((prev) => ({
...prev,
model: aliasedModel
}));
}
} catch (error) {
console.error("Error updating default model:", error);
}
}
}

updateDefaultModel();
}, [localState.billingStatus, localState.model, get]);

@greptile-apps greptile-apps Bot Aug 9, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

logic: The dependency array includes localState.model which could cause the effect to run when the model changes, potentially creating a feedback loop if getDefaultModelForUser returns a different model than expected.


async function persistChat(chat: Chat) {
const chatToSave = {
/** If a model is missing, assume the default Llama and write it now */
Expand Down Expand Up @@ -258,11 +308,18 @@ export const LocalStateProvider = ({ children }: { children: React.ReactNode })
});
}

function setModel(model: string) {
async function setModel(model: string) {
const aliasedModel = aliasModelName(model);
setLocalState((prev) =>
prev.model === aliasedModel ? prev : { ...prev, model: aliasedModel }
);

// Persist the last used model
try {
await put("last_used_model", aliasedModel);
} catch (error) {
console.error("Error saving last used model:", error);
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +368 to 379

@coderabbitai coderabbitai Bot Aug 10, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Guard setModel with access checks before persisting

Prevent selecting an inaccessible/unknown model and failing later. Early reject with a clear error.

   async function setModel(model: string) {
     const aliasedModel = aliasModelName(model);
-    setLocalState((prev) =>
-      prev.model === aliasedModel ? prev : { ...prev, model: aliasedModel }
-    );
+    // Validate before updating state/persisting
+    if (!MODEL_CONFIG[aliasedModel]) {
+      throw new Error(`Unknown model: ${aliasedModel}`);
+    }
+    if (!hasAccessToModel(aliasedModel, localState.billingStatus)) {
+      throw new Error(`No access to model: ${aliasedModel}`);
+    }
+    setLocalState((prev) =>
+      prev.model === aliasedModel ? prev : { ...prev, model: aliasedModel }
+    );
 
     // Persist the last used model
     await put("last_used_model", aliasedModel).catch((err) => {
       console.error("Error saving last used model:", err);
       throw err; // Propagate error to caller
     });
   }
📝 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
async function setModel(model: string) {
const aliasedModel = aliasModelName(model);
setLocalState((prev) =>
prev.model === aliasedModel ? prev : { ...prev, model: aliasedModel }
);
// Persist the last used model
await put("last_used_model", aliasedModel).catch((err) => {
console.error("Error saving last used model:", err);
throw err; // Propagate error to caller
});
}
async function setModel(model: string) {
const aliasedModel = aliasModelName(model);
// Validate before updating state/persisting
if (!MODEL_CONFIG[aliasedModel]) {
throw new Error(`Unknown model: ${aliasedModel}`);
}
if (!hasAccessToModel(aliasedModel, localState.billingStatus)) {
throw new Error(`No access to model: ${aliasedModel}`);
}
setLocalState((prev) =>
prev.model === aliasedModel ? prev : { ...prev, model: aliasedModel }
);
// Persist the last used model
await put("last_used_model", aliasedModel).catch((err) => {
console.error("Error saving last used model:", err);
throw err; // Propagate error to caller
});
}
🤖 Prompt for AI Agents 
In frontend/src/state/LocalStateContext.tsx around lines 368 to 379, the
setModel function currently persists the model without verifying access rights.
To fix this, add a check before persisting to confirm the model is accessible
and known. If the model is not accessible, immediately throw an error to prevent
setting and persisting an invalid model. This early guard will avoid later
failures and provide clearer error feedback.


function setAvailableModels(models: OpenSecretModel[]) {
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/state/LocalStateContextDef.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export type HistoryItem = {
export type LocalState = {
model: string;
availableModels: OpenSecretModel[];
setModel: (model: string) => void;
setModel: (model: string) => Promise<void>;
setAvailableModels: (models: OpenSecretModel[]) => void;
userPrompt: string;
systemPrompt: string | null;
Expand Down Expand Up @@ -75,7 +75,7 @@ export type LocalState = {
export const LocalStateContext = createContext<LocalState>({
model: "",
availableModels: [],
setModel: () => void 0,
setModel: async () => void 0,
setAvailableModels: () => void 0,
userPrompt: "",
systemPrompt: null,
Expand Down
64 changes: 64 additions & 0 deletions frontend/src/utils/modelDefaults.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { BillingStatus } from "@/billing/billingApi";
import { MODEL_CONFIG } from "@/components/ModelSelector";
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

/**
* Determines the default model for a user based on their plan type and usage history
*/
export function getDefaultModelForUser(
billingStatus: BillingStatus | null,
lastUsedModel: string | null
): string {
// If user has a last used model, prefer that
if (lastUsedModel && MODEL_CONFIG[lastUsedModel]) {
return lastUsedModel;
}

// If no billing status, assume free plan
if (!billingStatus) {
return "llama3-3-70b"; // Llama 3.3 70B for free users
}

const planName = billingStatus.product_name?.toLowerCase() || "";

// Pro, Max, or Team plan users get GPT-OSS 120B
if (planName.includes("pro") || planName.includes("max") || planName.includes("team")) {
return "gpt-oss-120b";
}

// Starter plan users get Gemma 3
if (planName.includes("starter")) {
return "google/gemma-3-27b-it";
}

// Free plan users get Llama 3.3 (default fallback)
return "llama3-3-70b";
}

/**
* Checks if a user has access to a specific model based on their billing status
*/
export function hasAccessToModel(modelId: string, billingStatus: BillingStatus | null): boolean {
const config = MODEL_CONFIG[modelId];

// If no restrictions, allow access
if (!config?.requiresPro && !config?.requiresStarter) return true;

const planName = billingStatus?.product_name?.toLowerCase() || "";

// Check if user is on Pro, Max, or Team plan (for requiresPro models)
if (config?.requiresPro) {
return planName.includes("pro") || planName.includes("max") || planName.includes("team");
}

// Check if user is on Starter, Pro, Max, or Team plan (for requiresStarter models)
if (config?.requiresStarter) {
return (
planName.includes("starter") ||
planName.includes("pro") ||
planName.includes("max") ||
planName.includes("team")
);
}

return true;
}
Comment on lines +65 to +83

@coderabbitai coderabbitai Bot Aug 10, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Respect disabled models in access checks

ModelCfg has disabled?: boolean, but hasAccessToModel doesn’t consider it. Disallow disabled models early.

 export function hasAccessToModel(modelId: string, billingStatus: BillingStatus | null): boolean {
   const config = MODEL_CONFIG[modelId];

   // If no restrictions, allow access
-  if (!config?.requiresPro && !config?.requiresStarter) return true;
+  if (!config) return false;               // unknown model
+  if (config.disabled) return false;       // disabled models are not accessible
+  if (!config.requiresPro && !config.requiresStarter) return true;

   const planTier = getPlanTier(billingStatus);
📝 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
export function hasAccessToModel(modelId: string, billingStatus: BillingStatus | null): boolean {
const config = MODEL_CONFIG[modelId];
// If no restrictions, allow access
if (!config?.requiresPro && !config?.requiresStarter) return true;
const planTier = getPlanTier(billingStatus);
// Check if user has required plan tier
if (config?.requiresPro) {
return planTier === "pro";
}
if (config?.requiresStarter) {
return planTier === "starter" || planTier === "pro";
}
return true;
}
export function hasAccessToModel(modelId: string, billingStatus: BillingStatus | null): boolean {
const config = MODEL_CONFIG[modelId];
// If no restrictions, allow access
if (!config) return false; // unknown model
if (config.disabled) return false; // disabled models are not accessible
if (!config.requiresPro && !config.requiresStarter) return true;
const planTier = getPlanTier(billingStatus);
// Check if user has required plan tier
if (config.requiresPro) {
return planTier === "pro";
}
if (config.requiresStarter) {
return planTier === "starter" || planTier === "pro";
}
return true;
}
🤖 Prompt for AI Agents 
In frontend/src/utils/modelDefaults.ts lines 65 to 83, the hasAccessToModel
function does not check if a model is disabled using the disabled property from
ModelCfg. Update the function to first check if config.disabled is true and if
so, immediately return false to disallow access to disabled models before any
other access checks.