-
-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Update model list in settings #379
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.
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
base: main
Are you sure you want to change the base?
Conversation
This commit updates the list of available models in the settings UI to include Grok 4.1, GPT 5.1, Opus 4.5, and Deepseek 4.5. It also removes the old models and updates the corresponding icons.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. New to Jules? Learn more at jules.google/docs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
|
|
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main technical concern is the change to model id values, which can break backward compatibility if these IDs are persisted or used in cross-layer contracts. Badge text has been simplified to "New" for all models, which may reduce the UI's ability to convey meaningful distinctions between them. Removal of QCX-Terra and its icon may represent a product/UX regression if that model is still required for geospatial or in-house scenarios. No obvious runtime or performance issues are visible within the scope of this diff.
Additional notes (1)
- Readability |
components/settings/components/model-selection-form.tsx:27-51
All four models now use the"New"badge, which may quickly become stale and provide limited information value to users. If badges are meant to communicate meaningful distinctions (e.g.,Recommended,Fast,Open Source,Advanced), collapsing everything into"New"reduces the UI's ability to guide model choice.
Consider whether you want to preserve some differentiating metadata (capabilities, performance, cost, or openness) and reflect that in the badges or descriptions.
Summary of changes
Summary of Changes
- Updated the imported icon list from
lucide-react, removingEarthand reordering remaining icons. - Replaced the previous model list (
QCX-Terra,Grok-3,claude-4-sonnet,llama-4) with a new set of models:Grok 4.1,GPT 5.1,Opus 4.5, andDeepseek 4.5. - Updated each model's
id,name,description,icon, andbadgemetadata to match the new lineup. - Adjusted badge variants:
secondary,default, andoutlineare used for the updated models to reflect their status as new offerings.
| id: "grok-4.1", | ||
| name: "Grok 4.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The id for the Grok model has been changed from a capitalized Grok-3 style to lowercased with a version ("grok-4.1"). If this id is persisted (e.g., user settings, URLs, backend contracts, or feature flags), this change can silently break existing users' saved preferences or any code that still expects the old identifiers.
It would be safer to either:
- Confirm that
selectedModelnever leaves the UI layer and has no backward-compatibility constraints, or - Introduce a migration/compat compatibility mapping from old IDs to new ones to avoid breaking existing stored values.
Suggestion
If these model IDs are stored or used across layers, consider adding a mapping layer to translate old IDs to new ones and keep the UI flexible:
// Example mapping for backward compatibility
const MODEL_ID_MAPPINGS: Record<string, string> = {
"Grok-3": "grok-4.1",
"claude-4-sonnet": "gpt-5.1",
"llama-4": "deepseek-4.5",
// "QCX-Terra" could map to a default or be handled explicitly
};
function normalizeModelId(id: string): string {
return MODEL_ID_MAPPINGS[id] ?? id;
}
// Wherever `selectedModel` is initialized/loaded:
const normalized = normalizeModelId(savedSelectedModelId);
form.setValue("selectedModel", normalized);This keeps the new IDs in the UI while not breaking existing users. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion and wire it into the appropriate initialization path.
| import { Card, CardContent } from "@/components/ui/card"; | ||
| import { Badge } from "@/components/ui/badge"; | ||
| import { Sparkles, Zap, Rocket, Cpu, Earth } from "lucide-react"; | ||
| import { Zap, Rocket, Cpu, Sparkles } from "lucide-react"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Earth icon import was removed entirely, which is appropriate given that QCX-Terra was deleted, but it also means there is no longer any in-house or geospatial model in the list. If QCX-Terra is part of your product identity or needed for certain use cases, its removal (with no replacement) might be a product/UX regression rather than a purely technical one.
Double-check with product requirements that dropping QCX-Terra from the selectable models is intentional and that any dependent flows (e.g., geospatial-specific features) are either deprecated or mapped to another model.
Suggestion
If QCX-Terra or a similar in-house/geospatial model should remain available, consider keeping it in the list with an updated description and potentially a different badge to de-emphasize it if necessary. For instance:
{
id: "QCX-Terra",
name: "QCX-Terra",
description: "QCX's geospatial foundational model for map and location-heavy tasks.",
icon: Earth,
badge: "Geospatial",
badgeVariant: "secondary" as const,
},You can also group or visually separate internal vs external models in the UI if that aligns with product goals. Reply with "@CharlieHelps yes please" if you'd like me to add a commit that restores or repositions QCX-Terra (or a renamed successor) in the model list.
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
User description
This commit updates the list of available models in the settings UI to include Grok 4.1, GPT 5.1, Opus 4.5, and Deepseek 4.5. It also removes the old models and updates the corresponding icons.
PR created automatically by Jules for task 6756121818968058353 started by @ngoiyaeric
PR Type
Enhancement
Description
Updates model list with four new AI models (Grok 4.1, GPT 5.1, Opus 4.5, Deepseek 4.5)
Replaces outdated models (QCX-Terra, Grok-3, Claude-4-sonnet, Llama-4)
Refreshes icon imports and badge labels for consistency
Marks all new models with "New" badge indicator
Diagram Walkthrough
File Walkthrough
model-selection-form.tsx
Replace legacy models with latest AI versionscomponents/settings/components/model-selection-form.tsx
versions
Earthand reordering remaining iconsbadge
providers