feat: add multi-type model support (LLM, TTS, STT, Embedding, Rerank) #499
Conversation
- Add ModelCategoryType enum and type-specific configs (TTSConfig, STTConfig, EmbeddingConfig, RerankConfig) in backend schema - Extend ModelSpec with modelType and type-specific configuration fields - Add model_category_type filter parameter to list_available_models API - Update UnifiedModel to include modelCategoryType in response - Add TypeScript types for model category type and configurations - Update getUnifiedModels API to support modelCategoryType filter - Add i18n translations for model type labels and filter options (zh-CN and en)
WalkthroughIntroduce model category typing across backend and frontend: new ModelCategoryType enum and per-type configs, a model_category_type query/filter on unified models, propagation through model aggregation and test-connection flows, TypeScript types and API param changes, UI filters/selectors, and i18n strings. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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
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/model_aggregation_service.py (1)
408-433: Fix public model config to include or properly accessmodelTypefield.The code attempts to extract
modelTypefrom theconfigdict usingconfig.get("modelType", "llm"), butmodelTypeis actually defined as a field inModelSpec(not inmodelConfig). SinceModelAdapter.to_model_dict()extracts onlyspec.modelConfigand returns it asconfig, themodelTypefield is lost. This means the extraction will always return the default"llm"value.To fix this, either:
- Pass both
modelConfigandmodelTypeseparately when building the model dict, or- Store
modelTypeinsidemodelConfigwhen creating/updating public models, or- Access
modelTypefrom the full CRD spec instead of the config dict
🧹 Nitpick comments (3)
frontend/src/apis/models.ts (1)
144-145: Remove duplicate comment.Line 145 duplicates the comment on line 144.
// Model Services -// Model Services export const modelApis = {backend/app/api/endpoints/adapter/models.py (1)
86-88: Consider using theModelCategoryTypeenum for parameter validation.The
model_category_typeparameter accepts any string, but the backend has aModelCategoryTypeenum that defines valid values. Using the enum would provide automatic validation and clearer API documentation.+from app.schemas.kind import ModelCategoryType + @router.get("/unified") def list_unified_models( shell_type: Optional[str] = Query( None, description="Shell type to filter compatible models (Agno, ClaudeCode)" ), include_config: bool = Query( False, description="Whether to include full config in response" ), scope: str = Query( "personal", description="Query scope: 'personal' (default), 'group', or 'all'", ), group_name: Optional[str] = Query( None, description="Group name (required when scope='group')" ), - model_category_type: Optional[str] = Query( + model_category_type: Optional[ModelCategoryType] = Query( None, description="Filter by model category type (llm, tts, stt, embedding, rerank)" ),If using the enum, you'll need to convert to string when passing to the service:
model_category_type=model_category_type.value if model_category_type else None,backend/app/schemas/kind.py (1)
34-75: Type-specific configuration models are well-defined.The config models include appropriate validation:
TTSConfig.speedhasge=0.25, le=4.0bounds- All fields are optional with sensible defaults
- Descriptions provide helpful context
Consider using
Literaltypes for fields with fixed allowed values (e.g.,output_format,transcription_format,encoding_format) to enable Pydantic validation, similar to the stricter typing on the frontend.+from typing import Any, Dict, List, Literal, Optional + class TTSConfig(BaseModel): """TTS-specific configuration""" voice: Optional[str] = Field( None, description="Voice ID (e.g., 'alloy', 'echo' for OpenAI)" ) speed: Optional[float] = Field( 1.0, ge=0.25, le=4.0, description="Speech speed (0.25-4.0)" ) - output_format: Optional[str] = Field("mp3", description="Output format (mp3, wav)") + output_format: Optional[Literal["mp3", "wav"]] = Field("mp3", description="Output format (mp3, wav)")
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/app/api/endpoints/adapter/models.py(4 hunks)backend/app/schemas/kind.py(2 hunks)backend/app/services/model_aggregation_service.py(15 hunks)frontend/src/apis/models.ts(5 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 (9)
**/*.{py,js,ts,tsx,jsx}
📄 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:
backend/app/api/endpoints/adapter/models.pybackend/app/schemas/kind.pyfrontend/src/apis/models.tsbackend/app/services/model_aggregation_service.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code must follow PEP 8 with Black formatter (line length: 88), isort for import sorting, and type hints are required
Run Black formatter and isort on Python code before committing
Run pylint and flake8 linters on Python code
Descriptive names and docstrings for public functions/classes are required in Python; extract magic numbers to constants; max 50 lines per function (preferred)
Files:
backend/app/api/endpoints/adapter/models.pybackend/app/schemas/kind.pybackend/app/services/model_aggregation_service.py
backend/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
backend/**/*.py: Backend API routes must be created inapp/api/directory, with schemas inapp/schemas/, and business logic inapp/services/
Backend must encrypt Git tokens and API keys using AES-256-CBC encryption
Files:
backend/app/api/endpoints/adapter/models.pybackend/app/schemas/kind.pybackend/app/services/model_aggregation_service.py
**/*.{py,ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Code in writing must distinguish between code-level terminology (Team, Bot, Ghost, Shell, Model) and UI-level terminology (智能体, 机器人 in Chinese; Agent, Bot in English)
Files:
backend/app/api/endpoints/adapter/models.pybackend/app/schemas/kind.pyfrontend/src/apis/models.tsbackend/app/services/model_aggregation_service.py
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
When writing code comments, use Team/Bot (CRD names) when referring to code-level resources; use 智能体/机器人 when writing Chinese UI text; use Agent/Team and Bot when writing English UI text
Files:
backend/app/api/endpoints/adapter/models.pybackend/app/schemas/kind.pyfrontend/src/apis/models.tsbackend/app/services/model_aggregation_service.py
backend/app/api/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
API route naming conventions should use CRD names: use
/api/teamsand/api/botsinstead of using other naming schemes
Files:
backend/app/api/endpoints/adapter/models.py
frontend/src/i18n/**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend i18n keys should use CRD names (e.g.,
teams.title,bots.title); i18n values in zh-CN should use UI terms (e.g., '智能体列表', '机器人')
Files:
frontend/src/i18n/locales/en/common.jsonfrontend/src/i18n/locales/zh-CN/common.json
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: TypeScript/React code must use TypeScript strict mode, functional components, Prettier formatter, ESLint, single quotes, and no semicolons
Useconstoverlet, never usevarin JavaScript/TypeScript code
Files:
frontend/src/apis/models.ts
frontend/**/*.{tsx,ts,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS with the specified color system CSS variables for styling (light theme:
--color-bg-base,--color-text-primary,--color-primary, etc.)
Files:
frontend/src/apis/models.ts
🧠 Learnings (2)
📚 Learning: 2025-12-16T11:28:47.401Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-16T11:28:47.401Z
Learning: Applies to frontend/src/i18n/**/*.{ts,tsx,json} : Frontend i18n keys should use CRD names (e.g., `teams.title`, `bots.title`); i18n values in zh-CN should use UI terms (e.g., '智能体列表', '机器人')
Applied to files:
frontend/src/i18n/locales/en/common.jsonfrontend/src/i18n/locales/zh-CN/common.json
📚 Learning: 2025-12-16T11:28:47.401Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-16T11:28:47.401Z
Learning: Applies to **/*.{py,ts,tsx,js,jsx,md} : Code in writing must distinguish between code-level terminology (Team, Bot, Ghost, Shell, Model) and UI-level terminology (智能体, 机器人 in Chinese; Agent, Bot in English)
Applied to files:
frontend/src/i18n/locales/zh-CN/common.json
🧬 Code graph analysis (3)
backend/app/schemas/kind.py (1)
frontend/src/apis/models.ts (5)
ModelCategoryType(8-8)TTSConfig(11-15)STTConfig(17-20)EmbeddingConfig(22-25)RerankConfig(27-30)
frontend/src/apis/models.ts (1)
backend/app/schemas/kind.py (5)
ModelCategoryType(16-31)TTSConfig(35-44)STTConfig(47-55)EmbeddingConfig(58-66)RerankConfig(69-75)
backend/app/services/model_aggregation_service.py (1)
backend/app/schemas/kind.py (2)
Model(166-173)ModelCategoryType(16-31)
⏰ 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). (4)
- GitHub Check: Test Frontend
- GitHub Check: Test wegent CLI Integration
- GitHub Check: E2E Tests (Chromium)
- GitHub Check: API Integration Tests
🔇 Additional comments (10)
frontend/src/i18n/locales/zh-CN/common.json (1)
626-648: LGTM! Translations follow coding guidelines perfectly.The new model category type translations adhere to the established pattern: keys use code-level identifiers while values provide clear Chinese UI terms. The translations are professional and include helpful examples in the hint texts (e.g., voice IDs, dimension values, speed ranges).
Based on learnings, this correctly applies the guideline that i18n keys should use CRD names while zh-CN values should use UI-friendly Chinese terms.
frontend/src/apis/models.ts (2)
11-30: Config interface property naming uses snake_case, which is consistent with backend API responses.The use of
output_format,transcription_format,encoding_format, etc. in snake_case matches the backend Pydantic models. This is appropriate since these interfaces represent API response data.
166-191: The API client update correctly propagates the new filter parameter.The implementation properly:
- Adds optional
modelCategoryTypeparameter- Converts to snake_case
model_category_typefor the API query string- Maintains backward compatibility by making the parameter optional
backend/app/api/endpoints/adapter/models.py (1)
128-136: Service call correctly propagates the new parameter.The
model_category_typeparameter is properly passed through to the aggregation service.backend/app/schemas/kind.py (2)
15-31: Well-structured enum with clear documentation.The
ModelCategoryTypeenum is properly defined with:
- Descriptive docstring explaining each type
- String inheritance for JSON serialization compatibility
- LLM as the default for backward compatibility
141-157: ModelSpec extension maintains backward compatibility.The new fields are properly integrated:
modelTypedefaults toLLMensuring existing models work without modification- All type-specific configs are optional
- Field descriptions are clear and accurate
backend/app/services/model_aggregation_service.py (3)
65-77: UnifiedModel correctly incorporates model category type with backward-compatible default.The implementation properly:
- Adds
model_category_typeas an optional parameter- Defaults to
"llm"for backward compatibility- Includes it in both
to_dict()andto_full_dict()outputs
147-157: CRD model category extraction handles the enum correctly.The code properly extracts the enum value as a string using
.valuefor JSON serialization compatibility.
370-372: Filter logic correctly compares model category types.The filtering uses string comparison with a sensible default fallback to
"llm".frontend/src/i18n/locales/en/common.json (1)
626-648: Comprehensive i18n keys for the new model category feature.The localization entries are well-structured:
- Clear labels for each model category type
- Helpful hints for configuration fields
- Consistent naming pattern with existing keys
As per coding guidelines, i18n keys correctly use CRD-style naming (e.g.,
models.model_category_type).
…ment - Add model category type selector (LLM/TTS/STT/Embedding/Rerank) in ModelEditDialog - Add dynamic protocol options based on selected category type - Add type-specific configuration fields: - TTS: voice, speed, output format - STT: language, transcription format - Embedding: dimensions, encoding format - Rerank: top_n, return_documents - Add category type filter dropdown in ModelList header - Add category type badges to model cards with color variants - Update form state management for category-specific configs
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/features/settings/components/ModelEditDialog.tsx (1)
211-217: Model ID options are LLM-specific but apply to all categories.
modelOptionsderives fromproviderTypebut only handles LLM providers (openai/gemini/anthropic). For non-LLM categories (e.g., TTS with Azure, Embedding with Cohere), it falls back toANTHROPIC_MODEL_OPTIONS, showing irrelevant Claude models.Consider:
- Conditionally rendering the model selector only for LLM, or
- Adding category-specific model options (e.g.,
TTS_MODEL_OPTIONS), or- Defaulting to custom-only input for non-LLM categories.
// Determine model options based on provider + // Only show preset model options for LLM category const modelOptions = - providerType === 'openai' - ? OPENAI_MODEL_OPTIONS - : providerType === 'gemini' - ? GEMINI_MODEL_OPTIONS - : ANTHROPIC_MODEL_OPTIONS; + modelCategoryType === 'llm' + ? (providerType === 'openai' + ? OPENAI_MODEL_OPTIONS + : providerType === 'gemini' + ? GEMINI_MODEL_OPTIONS + : ANTHROPIC_MODEL_OPTIONS) + : [{ value: 'custom', label: 'Custom...' }];Also applies to: 572-596
🧹 Nitpick comments (4)
frontend/src/features/settings/components/ModelEditDialog.tsx (3)
166-183: Consider validating type-specific config values before assignment.The type assertions (e.g.,
as 'mp3' | 'wav') rely on backend data being correct. If the stored value is invalid, the assertion silently coerces it. Consider adding validation:// Load type-specific configs if (model.spec.ttsConfig) { setTtsVoice(model.spec.ttsConfig.voice || ''); setTtsSpeed(model.spec.ttsConfig.speed || 1.0); - setTtsOutputFormat(model.spec.ttsConfig.output_format as 'mp3' | 'wav' || 'mp3'); + const outputFormat = model.spec.ttsConfig.output_format; + setTtsOutputFormat(outputFormat === 'mp3' || outputFormat === 'wav' ? outputFormat : 'mp3'); }
249-263: Non-LLM categories don't receive default base URLs.The handler only sets default base URLs for LLM models. For TTS/STT/Embedding/Rerank providers (Azure, ElevenLabs, Cohere, Jina), users must manually enter the base URL. Consider adding provider-specific defaults for other categories, or document this in the UI hint.
473-480: Placeholders are LLM-provider-specific.
apiKeyPlaceholderandbaseUrlPlaceholderonly handle OpenAI, Gemini, and Anthropic. For non-LLM providers (Azure, ElevenLabs, Cohere, Jina), users see the Anthropic placeholder (sk-ant-...), which may be confusing. Consider extending placeholders for category-specific providers.frontend/src/features/settings/components/ModelList.tsx (1)
54-61: Improve type safety of badge variant mapping.The
Record<string, ...>type allows any string key, which bypasses TypeScript's type checking. Since allModelCategoryTypevalues are mapped, use the specific type for stricter compile-time safety.Apply this diff:
-const MODEL_CATEGORY_BADGE_VARIANT: Record<string, 'default' | 'success' | 'info' | 'warning' | 'secondary'> = { +const MODEL_CATEGORY_BADGE_VARIANT: Record<ModelCategoryType, 'default' | 'success' | 'info' | 'warning' | 'secondary'> = { llm: 'default', tts: 'success', stt: 'info', embedding: 'warning', rerank: 'secondary', };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/features/settings/components/ModelEditDialog.tsx(12 hunks)frontend/src/features/settings/components/ModelList.tsx(9 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{py,js,ts,tsx,jsx}
📄 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/settings/components/ModelList.tsxfrontend/src/features/settings/components/ModelEditDialog.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: TypeScript/React code must use TypeScript strict mode, functional components, Prettier formatter, ESLint, single quotes, and no semicolons
Useconstoverlet, never usevarin JavaScript/TypeScript code
Files:
frontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/ModelEditDialog.tsx
**/components/**/*.{tsx,ts,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Component names must use PascalCase, and component files must use kebab-case naming convention
Files:
frontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/ModelEditDialog.tsx
frontend/src/features/**/components/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Feature-specific components should be organized in
src/features/*/components/directory
Files:
frontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/ModelEditDialog.tsx
frontend/**/*.{tsx,ts,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS with the specified color system CSS variables for styling (light theme:
--color-bg-base,--color-text-primary,--color-primary, etc.)
Files:
frontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/ModelEditDialog.tsx
**/*.{py,ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Code in writing must distinguish between code-level terminology (Team, Bot, Ghost, Shell, Model) and UI-level terminology (智能体, 机器人 in Chinese; Agent, Bot in English)
Files:
frontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/ModelEditDialog.tsx
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
When writing code comments, use Team/Bot (CRD names) when referring to code-level resources; use 智能体/机器人 when writing Chinese UI text; use Agent/Team and Bot when writing English UI text
Files:
frontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/ModelEditDialog.tsx
frontend/src/features/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
State management in frontend should use React Context API with providers defined in
src/features/(e.g., UserContext, TaskContext, ChatStreamContext, ThemeContext)
Files:
frontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/ModelEditDialog.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-16T11:28:47.401Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-16T11:28:47.401Z
Learning: Applies to frontend/src/components/**/*.{tsx,ts} : Use shadcn/ui components for UI elements with variants: `default`, `primary`, `secondary`, `outline`, `ghost`, `link`, `destructive` for buttons
Applied to files:
frontend/src/features/settings/components/ModelList.tsx
🧬 Code graph analysis (1)
frontend/src/features/settings/components/ModelList.tsx (3)
frontend/src/apis/models.ts (1)
ModelCategoryType(8-8)backend/app/schemas/kind.py (1)
ModelCategoryType(16-31)frontend/e2e/fixtures/data-builders.ts (1)
model(148-158)
⏰ 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). (3)
- GitHub Check: Test wegent CLI Integration
- GitHub Check: API Integration Tests
- GitHub Check: E2E Tests (Chromium)
🔇 Additional comments (12)
frontend/src/features/settings/components/ModelEditDialog.tsx (6)
40-79: Well-structured category-to-protocol mapping.The
MODEL_CATEGORY_OPTIONSandPROTOCOL_BY_CATEGORYconstants provide a clean, type-safe mapping for category-specific protocols. Good use of optionalhintfor LLM providers.
392-443: Clean conditional config building.The type-specific configs are built only when the corresponding category is selected, and spread into the CRD spec. This avoids polluting the model with empty config objects.
490-511: Category type selector correctly disables on edit.Preventing category type changes during edit avoids data inconsistency with existing type-specific configs. Good defensive design.
682-692: TTS speed input handles edge cases well.The
parseFloat(e.target.value) || 1.0fallback ensures invalid inputs default to 1.0. The HTMLmin/maxattributes provide client-side validation boundaries.
758-766: Integer parsing with undefined fallback is appropriate.Using
parseInt(e.target.value) || undefinedcorrectly handles empty/invalid inputs by settingundefined, allowing the backend to use defaults.Also applies to: 795-803
660-663: Hard-coded section headers should use translation keys.The section headers ("TTS Configuration", "STT Configuration", "Embedding Configuration", "Rerank Configuration") are hard-coded in English. Per the coding guidelines, UI text should use translation keys for i18n support.
- <h4 className="text-sm font-medium text-text-secondary">TTS Configuration</h4> + <h4 className="text-sm font-medium text-text-secondary">{t('models.tts_configuration')}</h4>Apply similar changes for STT, Embedding, and Rerank section headers.
Also applies to: 712-715, 749-752, 786-789
⛔ Skipped due to learnings
Learnt from: CR Repo: wecode-ai/Wegent PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-16T11:28:47.401Z Learning: Applies to frontend/src/i18n/**/*.{ts,tsx,json} : Frontend i18n keys should use CRD names (e.g., `teams.title`, `bots.title`); i18n values in zh-CN should use UI terms (e.g., '智能体列表', '机器人')Learnt from: CR Repo: wecode-ai/Wegent PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-16T11:28:47.400Z Learning: Applies to **/*.{py,js,ts,tsx,jsx} : All code comments MUST be written in English, including inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptionsLearnt from: CR Repo: wecode-ai/Wegent PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-16T11:28:47.401Z Learning: Applies to **/*.{py,ts,tsx,js,jsx,md} : Code in writing must distinguish between code-level terminology (Team, Bot, Ghost, Shell, Model) and UI-level terminology (智能体, 机器人 in Chinese; Agent, Bot in English)frontend/src/features/settings/components/ModelList.tsx (6)
34-41: LGTM! Clean imports.The Select component imports follow shadcn/ui patterns, and the ModelCategoryType is properly imported from the API layer.
73-73: LGTM! Properly typed field.The
modelCategoryTypefield is correctly typed withModelCategoryTypefrom the API layer and properly documented.
99-118: LGTM! Proper state and API integration.The category filter state is correctly typed, conditionally passed to the API only when not 'all', and properly included in the
useCallbackdependencies to trigger re-fetching when the filter changes.
147-147: LGTM! Good backward compatibility.Defaulting to
'llm'ensures existing models without an explicit category type are handled correctly, aligning with the PR's backward compatibility requirement.
390-394: LGTM! Consistent category badge rendering.The category badge is properly rendered across all three model sections (user, group, public) with:
- Dynamic translation keys for category labels
- Variant mapping with sensible fallback to
'default'- Consistent positioning as the first tag
The defensive
|| 'default'fallback ensures graceful handling if a new category is added without updating the variant mapping.Also applies to: 474-478, 555-559
327-348: Header layout implemented correctly.The flex layout properly accommodates the category filter UI. The Select component integration is correct and follows shadcn/ui patterns.
Keep the type annotation on line 335. The explicit type
(value: ModelCategoryType | 'all')is required for type safety and cannot be omitted. shadcn/ui's Select.onValueChange passes string type and does not support type inference, so the annotation is essential—not redundant.Verify translation keys are present.
All required translation keys exist in the i18n files:
models.title,models.description,models.filter_by_category_type,models.all_category_types, and allmodels.model_category_type_*keys for each category (llm, tts, stt, embedding, rerank).Likely an incorrect or invalid review comment.
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/model_aggregation_service.py (1)
415-444: IncludemodelTypein the public model data returned byModelAdapter.The code attempts to extract
modelTypefrom theconfigdict (line 418), butmodelTypeis stored atspec.modelTypein the CRD schema—not insidespec.modelConfig. TheModelAdapter.to_model_dict()method only returnsmodelConfig, somodelTypeinformation is lost. UpdateModelAdapter.to_model_dict()to includemodelTypein the returned dictionary, or extract it from the full schema before passing toUnifiedModel.
🧹 Nitpick comments (9)
frontend/src/features/settings/components/ModelEditDialog.tsx (2)
241-251: Consider resettingbaseUrlwhen model category changes.When switching categories (e.g., from LLM to Embedding), the base URL from the previous category may not be applicable. Consider clearing or updating
baseUrlto prevent confusion.const handleModelCategoryTypeChange = (value: ModelCategoryType) => { setModelCategoryType(value); // Reset provider to first available option for new category const protocols = PROTOCOL_BY_CATEGORY[value]; if (protocols && protocols.length > 0) { setProviderType(protocols[0].value); } setModelId(''); setCustomModelId(''); + setBaseUrl(''); };
703-876: Missing i18n for per-category section headers.The section headers like "TTS Configuration", "STT Configuration", "Embedding Configuration", and "Rerank Configuration" are hardcoded in English. Per coding guidelines, UI text should use translation keys.
- <h4 className="text-sm font-medium text-text-secondary">TTS Configuration</h4> + <h4 className="text-sm font-medium text-text-secondary">{t('models.tts_configuration')}</h4>Apply similar changes to STT, Embedding, and Rerank section headers.
frontend/src/apis/models.ts (1)
146-148: Duplicate comment on line 147.There's a duplicate
// Model Servicescomment.// Model Services -// Model Services export const modelApis = {backend/app/services/model_aggregation_service.py (1)
450-456: Minor: f-string in logger.debug.Using f-strings directly in logging calls prevents lazy evaluation. Consider using
%sformatting for consistency with other logging in this file (e.g., line 164).if model_name in seen_names: - logger.debug( - f"Model name '{model_name}' exists in both user and public models" - ) + logger.debug( + "Model name '%s' exists in both user and public models", + model_name, + )backend/app/api/endpoints/adapter/models.py (5)
350-352: Consider validating custom_headers values are strings.The code ensures
custom_headersis a dict but doesn't validate that values are strings. Malformed headers could cause issues downstream.# Ensure custom_headers is a dict if not isinstance(custom_headers, dict): custom_headers = {} + else: + # Filter out non-string values from custom_headers + custom_headers = { + k: v for k, v in custom_headers.items() + if isinstance(k, str) and isinstance(v, str) + }
373-375: Improve exception handling per static analysis hints.Use
logging.exceptioninstead oflogging.errorto include traceback, and avoid f-string in logging call.except Exception as e: - logger.error(f"Model connection test failed: {str(e)}") - return {"success": False, "message": f"Connection failed: {str(e)}"} + logger.exception("Model connection test failed: %s", e) + return {"success": False, "message": f"Connection failed: {e!s}"}
441-446: Remove unused variable assignment.The
responsevariable is assigned but never used, as flagged by static analysis.else: # Default: LLM - use chat completions - response = client.chat.completions.create( + client.chat.completions.create( model=model_id, messages=[{"role": "user", "content": "hi"}], max_tokens=128, ) return {"success": True, "message": f"Successfully connected to {model_id}"}
476-481: Remove unused variable assignment.Same issue as above -
responseis assigned but never used.- response = client.messages.create( + client.messages.create( model=model_id, max_tokens=128, messages=[{"role": "user", "content": "hi"}], ) return {"success": True, "message": f"Successfully connected to {model_id}"}
459-464: Anthropic model category validation is overly restrictive.The condition
if model_category_type not in ["llm", None, ""]will always be true whenmodel_category_typeis the string"llm"(which it defaults to on line 342), so this check works. However, the empty string check is unnecessary since the default is"llm", not"".# Anthropic currently only supports LLM models - if model_category_type not in ["llm", None, ""]: + if model_category_type != "llm": return { "success": False, "message": f"Anthropic provider does not support {model_category_type} models", }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/app/api/endpoints/adapter/models.py(6 hunks)backend/app/schemas/kind.py(2 hunks)backend/app/services/model_aggregation_service.py(15 hunks)frontend/src/apis/models.ts(10 hunks)frontend/src/features/settings/components/BotEdit.tsx(1 hunks)frontend/src/features/settings/components/ModelEditDialog.tsx(13 hunks)frontend/src/features/settings/components/ModelList.tsx(10 hunks)frontend/src/features/settings/components/wizard/steps/PreviewAdjustStep.tsx(1 hunks)frontend/src/features/tasks/components/ModelSelector.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/schemas/kind.py
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{py,js,ts,tsx,jsx}
📄 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/settings/components/BotEdit.tsxfrontend/src/features/tasks/components/ModelSelector.tsxfrontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/wizard/steps/PreviewAdjustStep.tsxbackend/app/services/model_aggregation_service.pyfrontend/src/apis/models.tsfrontend/src/features/settings/components/ModelEditDialog.tsxbackend/app/api/endpoints/adapter/models.py
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: TypeScript/React code must use TypeScript strict mode, functional components, Prettier formatter, ESLint, single quotes, and no semicolons
Useconstoverlet, never usevarin JavaScript/TypeScript code
Files:
frontend/src/features/settings/components/BotEdit.tsxfrontend/src/features/tasks/components/ModelSelector.tsxfrontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/wizard/steps/PreviewAdjustStep.tsxfrontend/src/apis/models.tsfrontend/src/features/settings/components/ModelEditDialog.tsx
**/components/**/*.{tsx,ts,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Component names must use PascalCase, and component files must use kebab-case naming convention
Files:
frontend/src/features/settings/components/BotEdit.tsxfrontend/src/features/tasks/components/ModelSelector.tsxfrontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/wizard/steps/PreviewAdjustStep.tsxfrontend/src/features/settings/components/ModelEditDialog.tsx
frontend/src/features/**/components/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Feature-specific components should be organized in
src/features/*/components/directory
Files:
frontend/src/features/settings/components/BotEdit.tsxfrontend/src/features/tasks/components/ModelSelector.tsxfrontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/wizard/steps/PreviewAdjustStep.tsxfrontend/src/features/settings/components/ModelEditDialog.tsx
frontend/**/*.{tsx,ts,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS with the specified color system CSS variables for styling (light theme:
--color-bg-base,--color-text-primary,--color-primary, etc.)
Files:
frontend/src/features/settings/components/BotEdit.tsxfrontend/src/features/tasks/components/ModelSelector.tsxfrontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/wizard/steps/PreviewAdjustStep.tsxfrontend/src/apis/models.tsfrontend/src/features/settings/components/ModelEditDialog.tsx
**/*.{py,ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Code in writing must distinguish between code-level terminology (Team, Bot, Ghost, Shell, Model) and UI-level terminology (智能体, 机器人 in Chinese; Agent, Bot in English)
Files:
frontend/src/features/settings/components/BotEdit.tsxfrontend/src/features/tasks/components/ModelSelector.tsxfrontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/wizard/steps/PreviewAdjustStep.tsxbackend/app/services/model_aggregation_service.pyfrontend/src/apis/models.tsfrontend/src/features/settings/components/ModelEditDialog.tsxbackend/app/api/endpoints/adapter/models.py
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
When writing code comments, use Team/Bot (CRD names) when referring to code-level resources; use 智能体/机器人 when writing Chinese UI text; use Agent/Team and Bot when writing English UI text
Files:
frontend/src/features/settings/components/BotEdit.tsxfrontend/src/features/tasks/components/ModelSelector.tsxfrontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/wizard/steps/PreviewAdjustStep.tsxbackend/app/services/model_aggregation_service.pyfrontend/src/apis/models.tsfrontend/src/features/settings/components/ModelEditDialog.tsxbackend/app/api/endpoints/adapter/models.py
frontend/src/features/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
State management in frontend should use React Context API with providers defined in
src/features/(e.g., UserContext, TaskContext, ChatStreamContext, ThemeContext)
Files:
frontend/src/features/settings/components/BotEdit.tsxfrontend/src/features/tasks/components/ModelSelector.tsxfrontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/wizard/steps/PreviewAdjustStep.tsxfrontend/src/features/settings/components/ModelEditDialog.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code must follow PEP 8 with Black formatter (line length: 88), isort for import sorting, and type hints are required
Run Black formatter and isort on Python code before committing
Run pylint and flake8 linters on Python code
Descriptive names and docstrings for public functions/classes are required in Python; extract magic numbers to constants; max 50 lines per function (preferred)
Files:
backend/app/services/model_aggregation_service.pybackend/app/api/endpoints/adapter/models.py
backend/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
backend/**/*.py: Backend API routes must be created inapp/api/directory, with schemas inapp/schemas/, and business logic inapp/services/
Backend must encrypt Git tokens and API keys using AES-256-CBC encryption
Files:
backend/app/services/model_aggregation_service.pybackend/app/api/endpoints/adapter/models.py
backend/app/api/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
API route naming conventions should use CRD names: use
/api/teamsand/api/botsinstead of using other naming schemes
Files:
backend/app/api/endpoints/adapter/models.py
🧠 Learnings (1)
📚 Learning: 2025-12-16T11:28:47.401Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-16T11:28:47.401Z
Learning: Applies to frontend/src/components/**/*.{tsx,ts} : Use shadcn/ui components for UI elements with variants: `default`, `primary`, `secondary`, `outline`, `ghost`, `link`, `destructive` for buttons
Applied to files:
frontend/src/features/settings/components/ModelList.tsx
🧬 Code graph analysis (3)
backend/app/services/model_aggregation_service.py (1)
backend/app/schemas/kind.py (2)
Model(167-174)ModelCategoryType(16-31)
frontend/src/apis/models.ts (3)
backend/app/schemas/kind.py (5)
ModelCategoryType(16-31)TTSConfig(35-44)STTConfig(47-55)EmbeddingConfig(58-66)RerankConfig(69-75)frontend/src/apis/client.ts (1)
apiClient(110-110)frontend/e2e/fixtures/data-builders.ts (1)
model(148-158)
frontend/src/features/settings/components/ModelEditDialog.tsx (1)
frontend/src/apis/models.ts (5)
ModelCategoryType(8-8)TTSConfig(11-15)STTConfig(17-20)EmbeddingConfig(22-25)RerankConfig(27-30)
🪛 Ruff (0.14.8)
backend/app/api/endpoints/adapter/models.py
373-373: Do not catch blind exception: Exception
(BLE001)
374-374: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
374-374: Use explicit conversion flag
Replace with conversion flag
(RUF010)
375-375: Use explicit conversion flag
Replace with conversion flag
(RUF010)
416-419: Consider moving this statement to an else block
(TRY300)
441-441: Local variable response is assigned to but never used
Remove assignment to unused variable response
(F841)
476-476: Local variable response is assigned to but never used
Remove assignment to unused variable response
(F841)
⏰ 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). (4)
- GitHub Check: Test wegent CLI Integration
- GitHub Check: Test Frontend
- GitHub Check: API Integration Tests
- GitHub Check: E2E Tests (Chromium)
🔇 Additional comments (23)
frontend/src/features/settings/components/BotEdit.tsx (1)
379-386: LGTM! Correctly filters models to LLM category for bot configuration.The filter ensures only LLM models are available for bot selection, which aligns with the PR objectives.
frontend/src/features/settings/components/wizard/steps/PreviewAdjustStep.tsx (1)
266-267: LGTM! Properly filters to LLM models for chat preview.The API call correctly passes all parameters including the new
'llm'category filter.frontend/src/features/tasks/components/ModelSelector.tsx (1)
157-158: LGTM! Correctly filters to LLM models for chat model selection.The change is consistent with other components and ensures only LLM models appear in the chat model selector.
frontend/src/features/settings/components/ModelList.tsx (3)
44-65: Good implementation of category filter options and badge variants.The commented-out TTS/STT options indicate these are planned for future enablement. The badge variant mapping provides visual differentiation for different model categories.
349-372: LGTM! Clean category filter UI implementation.The filter follows the established UI patterns and properly integrates with the model fetching logic via state.
233-252: LGTM! Test connection properly passes model category type.This enables the backend to use the appropriate API endpoint for testing different model categories (e.g., embeddings API vs chat completions API).
frontend/src/features/settings/components/ModelEditDialog.tsx (5)
48-90: Well-structured category and protocol configuration.The
PROTOCOL_BY_CATEGORYmapping provides clear protocol options for each model category, with helpful hints for LLM protocols. TTS/STT options are appropriately commented out for future enablement.
141-158: LGTM! Per-category state variables properly initialized with sensible defaults.The state management for TTS, STT, Embedding, and Rerank configurations follows React best practices with appropriate default values.
423-486: LGTM! Clean implementation of per-category config building.The conditional config building ensures only the relevant category's configuration is included in the CRD spec, avoiding unnecessary data.
533-554: LGTM! Model category selector appropriately disabled during editing.Preventing category changes for existing models is the correct approach to avoid configuration inconsistencies.
304-313: UpdateTestConnectionRequesttype to accept all supported provider types, not just LLM providers.The cast on line 307 masks a type safety issue:
PROTOCOL_BY_CATEGORYcontains providers like'azure','elevenlabs','google', and'custom'for TTS, STT, embedding, and rerank categories, butTestConnectionRequest.provider_typeonly accepts'openai' | 'anthropic' | 'gemini'. When users select non-LLM categories and providers, the cast silently passes invalid values to the API. Update theTestConnectionRequestinterface infrontend/src/apis/models.tsto include all valid providers fromPROTOCOL_BY_CATEGORY, then remove the unsafe cast.frontend/src/apis/models.ts (6)
7-8: LGTM! Type definition aligns with backend.The
ModelCategoryTypeunion type correctly mirrors the backendModelCategoryTypeenum values fromapp/schemas/kind.py.
10-30: Type-specific config interfaces look good.The interfaces correctly define optional properties matching the backend Pydantic models. However, note that the
output_formatinTTSConfig(line 14) uses a restricted union'mp3' | 'wav'while the backendTTSConfigaccepts any string with default "mp3". Same pattern for other format fields. This is acceptable as it provides stricter typing on the frontend.
51-58: New spec fields for multi-type model support look correct.The fields align with the backend Model schema. Consider documenting that
modelTypehere refers to the category (llm/tts/stt/embedding/rerank), to avoid confusion withModelTypeEnum(public/user/group) defined on line 104.
115-116: LGTM!The
modelCategoryTypefield correctly uses theModelCategoryTypeunion type and is optional for backward compatibility.
128-130: LGTM!New fields for
custom_headersandmodel_category_typeinTestConnectionRequestmatch the backend API expectations.
166-190: LGTM! API parameter handling is correct.The
modelCategoryTypeparameter is properly serialized asmodel_category_typein the query string (line 189), matching the backend's snake_case convention for query parameters.backend/app/services/model_aggregation_service.py (4)
65-81: LGTM! Constructor properly handles backward compatibility.The
model_category_typeparameter defaults toNoneand is set to"llm"when not provided, ensuring backward compatibility with existing models.
83-104: LGTM!The
to_dictmethod correctly outputsmodelCategoryTypeusing camelCase for the API response, consistent with other field naming conventions in the method.
127-171: LGTM! Model info extraction handles the new field correctly.The method properly extracts
model_category_typefrom the CRD spec'smodelTypefield and defaults to"llm"in all fallback scenarios for backward compatibility.
374-397: LGTM! User/group model filtering and construction.The filtering logic correctly uses
info.get("model_category_type", "llm")with a default, and passes the value through toUnifiedModelconstruction.backend/app/api/endpoints/adapter/models.py (2)
86-89: LGTM! Query parameter properly documented.The
model_category_typeparameter is correctly typed asOptional[str]with clear description. It follows the same pattern as other optional query parameters in this endpoint.
505-523: LGTM! Gemini embedding test implementation.The URL construction logic correctly handles different base URL formats and the embedding endpoint is properly tested with a minimal payload.
| elif model_category_type == "stt": | ||
| # STT models require audio input, hard to test without actual audio | ||
| # Return a message indicating manual verification is needed | ||
| return { | ||
| "success": True, | ||
| "message": f"STT model {model_id} configured. Audio transcription test requires actual audio input.", | ||
| } | ||
|
|
||
| elif model_category_type == "rerank": | ||
| # Rerank models are typically not standard OpenAI API | ||
| # They may use custom endpoints | ||
| return { | ||
| "success": True, | ||
| "message": f"Rerank model {model_id} configured. Please verify with actual rerank request.", | ||
| } |
There was a problem hiding this comment.
STT and Rerank tests return success without actual verification.
Both STT (lines 423-429) and Rerank (lines 431-437) handlers return success: True without performing any actual connection test. This could mislead users into thinking their configuration is valid when it hasn't been tested.
Consider either:
- Returning a different status like
"verified": falsealongside the message - Documenting this limitation clearly in the API response
elif model_category_type == "stt":
# STT models require audio input, hard to test without actual audio
- # Return a message indicating manual verification is needed
return {
- "success": True,
- "message": f"STT model {model_id} configured. Audio transcription test requires actual audio input.",
+ "success": True,
+ "message": f"STT model {model_id} configuration saved. Note: Connection was not verified - audio transcription test requires actual audio input.",
}
elif model_category_type == "rerank":
# Rerank models are typically not standard OpenAI API
- # They may use custom endpoints
return {
- "success": True,
- "message": f"Rerank model {model_id} configured. Please verify with actual rerank request.",
+ "success": True,
+ "message": f"Rerank model {model_id} configuration saved. Note: Connection was not verified - please test with actual rerank request.",
}📝 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.
| elif model_category_type == "stt": | |
| # STT models require audio input, hard to test without actual audio | |
| # Return a message indicating manual verification is needed | |
| return { | |
| "success": True, | |
| "message": f"STT model {model_id} configured. Audio transcription test requires actual audio input.", | |
| } | |
| elif model_category_type == "rerank": | |
| # Rerank models are typically not standard OpenAI API | |
| # They may use custom endpoints | |
| return { | |
| "success": True, | |
| "message": f"Rerank model {model_id} configured. Please verify with actual rerank request.", | |
| } | |
| elif model_category_type == "stt": | |
| # STT models require audio input, hard to test without actual audio | |
| return { | |
| "success": True, | |
| "message": f"STT model {model_id} configuration saved. Note: Connection was not verified - audio transcription test requires actual audio input.", | |
| } | |
| elif model_category_type == "rerank": | |
| # Rerank models are typically not standard OpenAI API | |
| return { | |
| "success": True, | |
| "message": f"Rerank model {model_id} configuration saved. Note: Connection was not verified - please test with actual rerank request.", | |
| } |
🤖 Prompt for AI Agents
In backend/app/api/endpoints/adapter/models.py around lines 423 to 437, the STT
and Rerank branches return success: True without any verification; change the
response to include a verification flag and clearer documentation by returning
success: True (indicating config saved) plus "verified": False and update the
message to state that no live verification was performed and manual testing is
required (e.g., "configured but not verified; manual test required"), so callers
can distinguish configured vs actually verified endpoints.
Summary
Test plan
Summary by CodeRabbit
New Features
UX / UI
Localization
✏️ Tip: You can customize this high-level summary in your review settings.