feat(frontend): display model as displayName(modelId) in task model selector #229
Conversation
…elector - Add displayName field to Model interface - Add getModelDisplayText helper function to format model display - Update trigger button to show displayName(modelId) format - Update dropdown list items to show displayName(modelId) format - Include displayName in search value for better searchability
WalkthroughAdds optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/features/tasks/components/ModelSelector.tsx (2)
31-37: ConsistentdisplayNameplumbing from backend to UI modelAdding
displayNameto the localModelinterface and mapping it inunifiedToModelkeeps this component aligned with the backend/APIModelshape and enables the new display behavior without breaking existing usage. One optional improvement would be to derive this interface from the APIModeltype (e.g., viaPick/intersection) to avoid future divergence.Also applies to: 66-74
266-278: Align trigger tooltip with new display text for better UX
getTriggerDisplayTextnow usesgetModelDisplayText, but the trigger’stitleattribute still shows onlyselectedModel?.name. This can confuse users whendisplayNamediffers fromnameor whenmodelIdis appended.Consider updating the title to use the same helper:
- <span className="truncate flex-1 min-w-0" title={selectedModel?.name || ''}> - {getTriggerDisplayText()} - </span> + <span + className="truncate flex-1 min-w-0" + title={selectedModel ? getModelDisplayText(selectedModel) : ''} + > + {getTriggerDisplayText()} + </span>Also applies to: 306-308
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/features/tasks/components/ModelSelector.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{py,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments MUST be written in English, including inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions
Files:
frontend/src/features/tasks/components/ModelSelector.tsx
**/frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript/React code must use strict mode, functional components with hooks, Prettier formatter, ESLint (Next.js config), single quotes, and no semicolons
Files:
frontend/src/features/tasks/components/ModelSelector.tsx
**/frontend/**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
constoverlet, nevervarin TypeScript/JavaScript
Files:
frontend/src/features/tasks/components/ModelSelector.tsx
**/frontend/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/frontend/**/*.{tsx,jsx}: Use Tailwind CSS utility classes with the color system CSS variables for consistent styling across the calm UI philosophy
Frontend mobile-first responsive design approach should be used with Tailwind CSS breakpoints (sm, md, lg, etc.)
Files:
frontend/src/features/tasks/components/ModelSelector.tsx
**/frontend/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
React hooks from react-hook-form and zod validation must be used for form implementation in TypeScript components
Files:
frontend/src/features/tasks/components/ModelSelector.tsx
🧬 Code graph analysis (1)
frontend/src/features/tasks/components/ModelSelector.tsx (2)
backend/app/schemas/kind.py (1)
Model(84-91)frontend/src/apis/models.ts (1)
Model(61-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-tests
🔇 Additional comments (3)
frontend/src/features/tasks/components/ModelSelector.tsx (3)
76-80:getModelDisplayTextmatches the desired display formatThe helper correctly implements the
displayName(modelId)/name(modelId)behavior and centralizes the formatting, which reduces duplication and future change surface. No issues from a correctness or readability standpoint.
379-383: IncludingdisplayNameinCommandItem.valuenicely improves searchabilityExtending the
valuestring withmodel.displayName(with a safe empty-string fallback) means command search can now match on the human-readable name without affecting selection logic (sinceonSelectstill passesgetModelKey(model)). This is a clean, low-risk enhancement.
406-411: Dropdown label and tooltip correctly reusegetModelDisplayTextUsing
getModelDisplayTextfor both the visible label and thetitleensures consistent rendering ofdisplayName(modelId)across the list, and keeps future changes to the format localized to one helper.
…-displayname-format
…fined models Legacy teams created before PR#194 may have Bots without agent_config (bind_model). When using these teams, selecting "Default Model" option would cause agent_config to be empty and result in task execution errors. This fix: - Export allBotsHavePredefinedModel function from ModelSelector for reuse - Add isModelSelectionRequired check in ChatArea to disable send button when model selection is required but not fulfilled - Display error-styled "Please select a model" prompt in ModelSelector for legacy teams - Add i18n translations (model_required) for both zh-CN and en locales
…-displayname-format
…/model-selector-displayname-format
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 (1)
backend/app/services/adapters/bot_kinds.py (1)
654-658: Validate that the model exists before updating the reference.After fetching the model at line 655, there's no validation that it was found. If
_get_model_by_name_and_typereturnsNone, the bot's modelRef will point to a non-existent model, which could cause runtime errors.Apply this diff to add validation:
# Get the new model for response using type hint model = self._get_model_by_name_and_type( db, model_name, "default", user_id, model_type ) + if model is None: + raise HTTPException( + status_code=404, + detail=f"Model '{model_name}' not found", + ) return_agent_config = new_agent_config
🧹 Nitpick comments (4)
backend/app/services/adapters/bot_kinds.py (2)
608-617: Extract duplicated ModelRef update logic into a helper method.The pattern of checking if
bot_crd.spec.modelRefexists and creating/updating it appears twice (lines 608-617 and 750-759). Extract this into a private helper method to eliminate duplication and improve maintainability.Add this helper method to the class:
def _update_bot_model_ref( self, bot: Kind, model_name: str, namespace: str = "default" ) -> None: """ Update or create modelRef for a bot. Args: bot: The Bot Kind object model_name: Name of the model to reference namespace: Model namespace (defaults to "default") """ from app.schemas.kind import ModelRef bot_crd = Bot.model_validate(bot.json) if bot_crd.spec.modelRef: bot_crd.spec.modelRef.name = model_name bot_crd.spec.modelRef.namespace = namespace else: bot_crd.spec.modelRef = ModelRef(name=model_name, namespace=namespace) bot.json = bot_crd.model_dump() flag_modified(bot, "json")Then replace both occurrences:
# Update bot's modelRef - bot_crd = Bot.model_validate(bot.json) - from app.schemas.kind import ModelRef - - if bot_crd.spec.modelRef: - bot_crd.spec.modelRef.name = model_name - bot_crd.spec.modelRef.namespace = "default" - else: - # Create new modelRef if it doesn't exist - bot_crd.spec.modelRef = ModelRef( - name=model_name, namespace="default" - ) - bot.json = bot_crd.model_dump() - flag_modified(bot, "json") + self._update_bot_model_ref(bot, model_name, "default")# Update bot's modelRef to point to the new dedicated model - bot_crd = Bot.model_validate(bot.json) - from app.schemas.kind import ModelRef - - if bot_crd.spec.modelRef: - bot_crd.spec.modelRef.name = dedicated_model_name - bot_crd.spec.modelRef.namespace = "default" - else: - # Create new modelRef - bot_crd.spec.modelRef = ModelRef( - name=dedicated_model_name, namespace="default" - ) - bot.json = bot_crd.model_dump() - flag_modified(bot, "json") + self._update_bot_model_ref(bot, dedicated_model_name, "default")Also applies to: 750-759
495-808: Refactor method to reduce length and complexity.The
update_with_usermethod is approximately 313 lines, significantly exceeding the guideline maximum of 50 lines. Consider breaking it into smaller, focused methods to improve readability and maintainability.As per coding guidelines, Python functions should not exceed 50 lines.
Suggested decomposition:
_update_bot_name- Handle name updates and conflict checking (lines 523-542)_update_shell_and_agent- Handle shell and agent_name updates (lines 557-590)_update_agent_config_predefined_model- Handle predefined model updates (lines 596-658)_update_agent_config_custom_model- Handle custom model config updates (lines 659-759)_update_ghost_config- Handle ghost (system_prompt, mcp_servers, skills) updates (lines 761-783)_update_timestamps- Update all component timestamps (lines 785-792)The main
update_with_usermethod would then orchestrate these smaller operations, making the logic flow more apparent and each piece easier to test independently.frontend/src/features/tasks/components/ModelSelector.tsx (2)
43-50: Type aliasTeamWithBotDetailsvs.Teamacross components could be simplified
ModelSelectorintroduces a localTeamWithBotDetails extends Teamto express thebotsshape needed byallBotsHavePredefinedModel, whileChatAreapasses plainTeaminstances intoModelSelector.This is fine structurally as long as the runtime
Teamobjects always satisfy theTeamWithBotDetailsshape, but it does mean:
- Props in
ModelSelectorare stricter than the type used inChatArea, which can surface as a TS prop‑type mismatch ifTeamdoes not already have a compatiblebotsdefinition.- There are now two slightly different notions of “team” around the model selector.
If
Teamfrom@/types/apialready hasbotswith the right structure, you could simplify by droppingTeamWithBotDetailsand typing the prop and helper directly onTeam | null, which keeps the API surface betweenChatAreaandModelSelectoraligned and avoids future divergence.If
Teamdoes not yet carry the full detail, another option is to centralizeTeamWithBotDetailsin the shared types module and use that consistently in both places.Also applies to: 81-87, 123-137
296-315: Minor UX nit: fallback text language ingetTriggerDisplayTextdefaults to Chinese
getTriggerDisplayTextuses:if (isModelRequired) { return t('task_submit.model_required', '请选择模型'); } return t('task_submit.select_model', '选择模型');The translation keys exist in both locales, so the Chinese fallback should almost never be hit. Still, if you want the default to be language‑agnostic, you might prefer an English fallback here (or drop the fallback entirely and rely on the JSON definitions).
Not a blocker, just something to consider for consistency.
Also applies to: 323-339
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/app/services/adapters/bot_kinds.py(1 hunks)frontend/src/features/tasks/components/ChatArea.tsx(4 hunks)frontend/src/features/tasks/components/ModelSelector.tsx(8 hunks)frontend/src/i18n/locales/en/common.json(1 hunks)frontend/src/i18n/locales/zh-CN/common.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
frontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/ModelSelector.tsxbackend/app/services/adapters/bot_kinds.py
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/ModelSelector.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/ModelSelector.tsx
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React component files MUST use kebab-case naming convention
Files:
frontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/ModelSelector.tsx
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/ModelSelector.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/ModelSelector.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST be PEP 8 compliant with Black formatter (line length: 88) and isort for import organization
Python code MUST include type hints for all functions and variables
Python functions SHOULD NOT exceed 50 lines (preferred maximum)
Python functions and classes MUST have descriptive names and public functions/classes MUST include docstrings
Python code MUST extract magic numbers to named constants
Files:
backend/app/services/adapters/bot_kinds.py
**/backend/app/services/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend business logic MUST be placed in
app/services/directory
Files:
backend/app/services/adapters/bot_kinds.py
🧬 Code graph analysis (3)
frontend/src/features/tasks/components/ChatArea.tsx (1)
frontend/src/features/tasks/components/ModelSelector.tsx (1)
allBotsHavePredefinedModel(83-100)
frontend/src/features/tasks/components/ModelSelector.tsx (1)
frontend/src/apis/models.ts (1)
Model(61-64)
backend/app/services/adapters/bot_kinds.py (1)
backend/app/schemas/kind.py (1)
ModelRef(103-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-tests
🔇 Additional comments (5)
frontend/src/i18n/locales/zh-CN/common.json (1)
539-541:task_submit.model_requiredkey addition looks correct and aligned with EN localeThe new
model_requiredentry is consistent with the existingtask_submitkeys and matches the semantics of “Please select a model”; no issues from an i18n standpoint.frontend/src/i18n/locales/en/common.json (1)
532-540: Englishtask_submit.model_requiredentry is well-placed and consistentThe new
model_requiredkey cleanly extends thetask_submitblock and matches the usage in the UI (“Please select a model”); no issues spotted.frontend/src/features/tasks/components/ModelSelector.tsx (2)
30-37: displayName plumbing and display text helper look correct and match the PR goalThe
displayNamefield is correctly:
- Added to the local
Modelshape.- Mapped from
UnifiedModelinunifiedToModel.- Used via
getModelDisplayTextfor both the trigger text/title and list items.- Included in the
CommandItem.valuestring so search covers bothnameanddisplayName.The fallback to
model.namewhendisplayNameis absent ensures old models continue to render and search correctly. This aligns well with the “displayName(modelId)” requirement.Also applies to: 66-73, 76-80, 420-421, 445-449, 456-463
81-100: AlignisModelRequiredUI state with ChatArea logic and mixed/dify team behaviorRight now:
isModelRequiredis computed as!showDefaultOption && !selectedModel, regardless ofagent_typeorisMixedTeam.- ChatArea's
isModelSelectionRequiredexplicitly skips dify teams, and (after the suggested fix) should also skip mixed teams.- For dify teams, the UI can show the red CPU icon and "Please select a model" hint even though ChatArea does not actually require a model.
- For mixed teams, the selector is disabled via
isMixedTeam, butisModelRequiredcan still be true, which makes the control look invalid/required even when the user cannot interact with it.To keep behavior coherent across components and avoid confusing states, consider tightening
isModelRequiredto the same conditions you intend to enforce in ChatArea, e.g.:- // Check if model selection is required (for legacy teams without predefined models) - const isModelRequired = !showDefaultOption && !selectedModel; + // Check if model selection is required (for legacy non-dify, non-mixed teams without predefined models) + const isModelRequired = + !!selectedTeam && + selectedTeam.agent_type !== 'dify' && + !isMixedTeam && + !showDefaultOption && + !selectedModel;This will:
- Prevent dify teams from being visually marked as "required" when ChatArea does not block sending.
- Avoid showing an unsatisfiable "required" state for mixed teams where the selector is intentionally disabled.
frontend/src/features/tasks/components/ChatArea.tsx (1)
12-16: Model-required gating can hard‑block mixed teams with no prior selection; consider broadening the skip conditionsThe new
isModelSelectionRequiredgate and its use in the send buttondisabledprop are generally sound, but there is a corner case:
- For teams with
is_mix_team === true,ModelSelectorinternally disables the selector (isMixedTeam), so the user cannot change the model.- At the same time,
isModelSelectionRequiredonly skips dify and "has default option" teams. For a mixed team without a stored last‑selected model,selectedModelremainsnull, soisModelSelectionRequiredbecomestrueand the send button stays permanently disabled with no way to satisfy the requirement.To avoid blocking mixed teams, extend the early‑return conditions to also skip mixed teams (and keep the dify behavior you already added):
- const isModelSelectionRequired = React.useMemo(() => { - // Skip check if team is not selected, or if team type is 'dify' (external API) - if (!selectedTeam || selectedTeam.agent_type === 'dify') return false; + const isModelSelectionRequired = React.useMemo(() => { + // Skip check if team is not selected, is a Dify team, or is a mixed team + if ( + !selectedTeam || + selectedTeam.agent_type === 'dify' || + selectedTeam.is_mix_team + ) { + return false; + } @@ - const hasDefaultOption = allBotsHavePredefinedModel(selectedTeam); + const hasDefaultOption = allBotsHavePredefinedModel(selectedTeam); @@ - return !selectedModel; - }, [selectedTeam, selectedModel]); + return !selectedModel; + }, [selectedTeam, selectedModel]);This keeps the "legacy team must pick a model" behavior while avoiding a dead‑end UX for mixed teams where the selector is disabled by design.
|
|
||
| # Update bot's modelRef | ||
| bot_crd = Bot.model_validate(bot.json) | ||
| from app.schemas.kind import ModelRef |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Consolidate duplicate imports to function or file level.
The import from app.schemas.kind import ModelRef appears twice in this function (lines 606 and 748). Move the import to the top of the function to avoid redundancy.
Apply this diff to move the import to the function start:
) -> Dict[str, Any]:
"""
Update user Bot
"""
import logging
logger = logging.getLogger(__name__)
+ from app.schemas.kind import ModelRef
bot = (Then remove both inline imports:
# Update bot's modelRef
bot_crd = Bot.model_validate(bot.json)
- from app.schemas.kind import ModelRef
-
if bot_crd.spec.modelRef: # Update bot's modelRef to point to the new dedicated model
bot_crd = Bot.model_validate(bot.json)
- from app.schemas.kind import ModelRef
-
if bot_crd.spec.modelRef:Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/app/services/adapters/bot_kinds.py around lines 606 and 748, the
import "from app.schemas.kind import ModelRef" is duplicated inline; move a
single "from app.schemas.kind import ModelRef" to the top of the enclosing
function (immediately after the function signature) and remove the two inline
import statements at lines ~606 and ~748 so ModelRef is imported once per
function scope.
Summary
displayNamefield to Model interface to support human-readable model namesgetModelDisplayTexthelper function that formats model display asdisplayName(modelId)orname(modelId)Changes
frontend/src/features/tasks/components/ModelSelector.tsxTest plan
displayName(modelId)format in the trigger buttondisplayName(modelId)format for each model itemname(modelId)Summary by CodeRabbit
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.