feat(settings): add model management module #194
Conversation
- Add Models tab to settings page (between Team and Integrations) - Create ModelList component for viewing/managing model configurations - Create ModelEdit component for creating/editing models with: - Provider type selection (OpenAI/Anthropic) - Model ID presets and custom option - API Key input with visibility toggle - Base URL configuration - Connection test functionality - Add backend endpoints: - POST /models/test-connection for validating API credentials - GET /models/compatible for filtering models by agent type - Update TaskCreate schema with model_id and force_override_bot_model fields - Enhance executor_kinds.py model resolution logic: - Support bind_model in agent_config - Support task-level model override with force_override flag - Query kinds table first, fallback to public_models (legacy) - Maintain backward compatibility with private_model - Add i18n translations for models (zh-CN and en) This enables users to manage AI model configurations independently and bind them to Bots, supporting both OpenAI and Anthropic providers.
|
Caution Review failedThe pull request is closed. WalkthroughAdds unified model discovery/service and new adapter endpoints; wires CRD-based model resolution into executors, bots, teams, and tasks; adds API-key encryption helpers; extends frontend with model CRUD, selector, and settings UI; introduces i18n strings and UI primitives; plus tests and small infra tweaks. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Frontend (ModelList/ModelEdit/ModelSelector)
participant API as Backend API (adapter models endpoints)
participant Aggreg as ModelAggregationService
participant DB as DB (Kind CRDs & PublicModel)
participant Provider as External Provider
User->>UI: open models UI / select model / trigger test
UI->>API: GET /adapter/models or POST /adapter/models/test-connection
API->>Aggreg: list_available_models / resolve_model (db, user, agent)
Aggreg->>DB: query Kind CRDs and PublicModel
DB-->>Aggreg: return CRD / public entries
Aggreg-->>API: aggregated model list or resolved model (with config)
alt test connection
API->>Provider: probe (OpenAI/Anthropic) using decrypted key
Provider-->>API: success / error
API-->>UI: {success, message}
else CRUD
UI->>API: POST/PUT/DELETE model
API->>DB: persist Kind CRD or update PublicModel
DB-->>API: ack
API-->>UI: result
end
sequenceDiagram
participant Task as Task Runner
participant Executor as ExecutorKinds
participant Aggreg as ModelAggregationService
participant DB as DB (Kind CRDs & PublicModel)
Task->>Executor: start(task includes optional modelId/forceOverride)
Executor->>Executor: determine model_name_to_use (task override -> bot bind -> legacy)
Executor->>Aggreg: resolve_model(name, maybe type)
Aggreg->>DB: fetch Kind or PublicModel
DB-->>Aggreg: CRD or public entry
Aggreg-->>Executor: model config (api_key decrypted if present)
Executor->>Executor: apply protocol/base_url and continue execution
alt fallback path
Executor->>Executor: use original agent_config / legacy PublicModel lookup
end
Executor-->>Task: execution proceeds with resolved config
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
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: 2
🧹 Nitpick comments (5)
backend/app/services/adapters/executor_kinds.py (1)
440-443: Uselogger.exceptionto preserve stack trace.When catching exceptions in the fallback handler,
logger.exceptionwill automatically include the traceback, which aids debugging production issues.except Exception as e: - logger.error(f"Failed to resolve model config: {e}") + logger.exception(f"Failed to resolve model config: {e}") # On any error, fallback to original agent_config agent_config_data = agent_configfrontend/src/features/settings/components/ModelList.tsx (1)
27-257: ModelList logic is solid; consider aligning chip styling with design tokensThe CRUD flow, connection testing, and list/empty/loading states are wired correctly against the CRD APIs and i18n keys; the per-row action handling looks sound. For the provider badges (
getProviderStyle), you might later swap the raw Tailwind greens/oranges for your existing theme variables (e.g.,bg-muted+ text tokens) to keep colors consistent with the rest of the app.backend/app/api/endpoints/adapter/models.py (1)
255-306: Make compatible-model filtering more robust and align with SQLAlchemy styleThe overall idea—filtering Model CRDs by
env.modelfor a given agent—is good, but a few details could be tightened:
- ModelConfig shape:
model_crd.spec.modelConfigis treated as a dict, but if the PydanticModelCRDschema uses a nested model formodelConfig,isinstance(..., dict)will be false and those models will be silently skipped. Consider handling both cases (e.g.,env = model_crd.spec.modelConfig.envwhen it’s a model, else dict.get("env")), so compatible models aren’t dropped by accident.- Agent name matching: Right now
agent_namemust be exactly"Agno"or"ClaudeCode". It may be safer to normalizeagent_name(e.g.,.lower()) or centralize these identifiers to avoid subtle mismatches with frontend strings.- SQLAlchemy boolean comparison:
Kind.is_active == Truecan be simplified toKind.is_active.is_(True)or justKind.is_activeper SQLAlchemy best practices, which will also quiet static analysis.These are mostly robustness and style tweaks; the core behavior is fine.
frontend/src/features/settings/components/ModelEdit.tsx (1)
29-217: ModelEdit flow is correct; refine validation messages and small nitsThe create/edit and test-connection flows, CRD construction, and i18n usage all line up with the new backend APIs and types. A few minor points you might adjust:
- In
handleTestConnection, when eitherfinalModelIdorapiKeyis missing you always showmodels.errors.model_id_required. Splitting the check so missing API key usesmodels.errors.api_key_requiredwould give clearer feedback.isPresetModelIdis computed but never used; it can be removed or leveraged (e.g., to conditionally show the custom ID input).- When switching provider types on an existing model,
handleProviderChangeoverwrites any existingbaseUrlwith the provider default. If users might have custom proxies per model, you may want to only set a default when creating (or whenbaseUrlis empty) rather than on every provider change.None of these block merging, but they’ll smooth the UX and keep the component tidy.
Also applies to: 233-395
frontend/src/apis/models.ts (1)
7-33: CRD types and model APIs align well with the backend designThe
ModelCRD/ModelListResponse/test-connection/compatible-model types and correspondingmodelApismethods match the CRD-centric backend endpoints and what ModelList/ModelEdit need. This fits the Kubernetes-style CRD pattern noted for the project. As a tiny future polish, you might centralize the repeated'/kinds/namespaces/default/models'path/namespace into a constant to avoid drift if namespaces ever change.Also applies to: 44-65, 68-123
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/app/api/endpoints/adapter/models.py(3 hunks)backend/app/schemas/task.py(1 hunks)backend/app/services/adapters/executor_kinds.py(1 hunks)frontend/src/apis/models.ts(2 hunks)frontend/src/app/settings/page.tsx(7 hunks)frontend/src/features/settings/components/ModelEdit.tsx(1 hunks)frontend/src/features/settings/components/ModelList.tsx(1 hunks)frontend/src/i18n/locales/en/common.json(2 hunks)frontend/src/i18n/locales/zh-CN/common.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
frontend/**
📄 CodeRabbit inference engine (AGENTS.md)
Frontend: Only use
NEXT_PUBLIC_*for client-safe environment variables
Files:
frontend/src/i18n/locales/en/common.jsonfrontend/src/features/settings/components/ModelList.tsxfrontend/src/i18n/locales/zh-CN/common.jsonfrontend/src/features/settings/components/ModelEdit.tsxfrontend/src/apis/models.tsfrontend/src/app/settings/page.tsx
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/**/*.{ts,tsx}: TypeScript strict mode must be enabled in Frontend
Use functional components with hooks in React
Use Prettier formatter in Frontend with single quotes and no semicolons
Use ESLint with Next.js configuration in Frontend
Useconstoverlet, never usevarin TypeScript
Component names must use PascalCase; file names must use kebab-case in Frontend
Use mobile-first responsive design approach in Frontend components
Frontend design: Low saturation + low contrast for reduced eye strain with generous whitespace
Frontend design: Minimal shadows, subtle component differentiation (<10% background variance)
Frontend design: Use mint blue (#14B8A6) as primary accent sparingly
Use Tailwind CSS variables for colors: --color-bg-base, --color-text-primary, --color-border, etc.
Frontend spacing: Use standard units - p-2 (8px), p-4 (16px), p-6 (24px), gap-3 (12px)
Frontend border-radius: rounded-2xl (16px) for large containers, rounded-lg (12px) for cards, rounded-md (6px) for buttons
Frontend typography: H1 text-xl font-semibold, H2 text-lg font-semibold, H3 text-base font-medium, Body text-sm
Use shadcn/ui Button component with variants: default, secondary, ghost, outline, link
Use shadcn/ui Form component with react-hook-form + zod validation
Files:
frontend/src/features/settings/components/ModelList.tsxfrontend/src/features/settings/components/ModelEdit.tsxfrontend/src/apis/models.tsfrontend/src/app/settings/page.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code must be PEP 8 compliant with Black formatter (line length: 88)
Use isort for import organization in Python
Python functions and variables must have descriptive names
Docstrings required for all public Python functions and classes
Extract magic numbers to named constants in Python
Python functions must not exceed 50 lines
Type hints required for all Python function signatures
Run pylint and flake8 for Python code quality checks
Files:
backend/app/api/endpoints/adapter/models.pybackend/app/services/adapters/executor_kinds.pybackend/app/schemas/task.py
🧠 Learnings (1)
📚 Learning: 2025-11-26T07:34:32.573Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.573Z
Learning: Use Kubernetes-inspired CRD design (Ghost, Model, Shell, Bot, Team, Task)
Applied to files:
frontend/src/apis/models.ts
🧬 Code graph analysis (6)
frontend/src/features/settings/components/ModelList.tsx (4)
frontend/src/hooks/use-toast.ts (2)
useToast(189-189)toast(189-189)frontend/src/apis/models.ts (2)
ModelCRD(8-28)modelApis(67-123)frontend/src/components/ui/button.tsx (1)
Button(55-55)frontend/src/components/ui/card.tsx (1)
Card(75-75)
frontend/src/features/settings/components/ModelEdit.tsx (1)
frontend/src/apis/models.ts (2)
ModelCRD(8-28)modelApis(67-123)
backend/app/api/endpoints/adapter/models.py (3)
backend/app/models/kind.py (1)
Kind(14-30)backend/app/core/security.py (1)
get_current_user(27-50)frontend/src/apis/models.ts (2)
Model(36-38)ModelCRD(8-28)
frontend/src/apis/models.ts (1)
frontend/src/apis/client.ts (1)
apiClient(105-105)
backend/app/services/adapters/executor_kinds.py (2)
backend/app/models/kind.py (1)
Kind(14-30)backend/app/schemas/kind.py (1)
Model(68-74)
frontend/src/app/settings/page.tsx (1)
backend/app/schemas/kind.py (1)
ModelList(77-81)
🪛 Ruff (0.14.6)
backend/app/api/endpoints/adapter/models.py
177-177: Unused function argument: current_user
(ARG001)
177-177: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
231-231: Local variable response is assigned to but never used
Remove assignment to unused variable response
(F841)
247-247: Do not catch blind exception: Exception
(BLE001)
248-248: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
248-248: Use explicit conversion flag
Replace with conversion flag
(RUF010)
251-251: Use explicit conversion flag
Replace with conversion flag
(RUF010)
258-258: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
259-259: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
282-282: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
302-302: Do not catch blind exception: Exception
(BLE001)
backend/app/services/adapters/executor_kinds.py
414-414: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
424-424: Do not catch blind exception: Exception
(BLE001)
440-440: Do not catch blind exception: Exception
(BLE001)
441-441: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🔇 Additional comments (7)
backend/app/schemas/task.py (1)
57-59: LGTM!The new model selection fields are well-defined with appropriate type hints, sensible defaults, and helpful documentation clarifying that
model_idrepresents a model name rather than a database ID.backend/app/services/adapters/executor_kinds.py (3)
418-425: Inner exception handling looks appropriate.The try-except with
logger.warningis suitable here since CRD parsing failures gracefully fall back to thepublic_modelstable. The exception details are preserved in the warning message.
372-405: Model resolution priority logic is well-structured.The resolution hierarchy (force override → bind_model → private_model → task fallback) is clearly documented and handles backward compatibility with the legacy
private_modelpath.
427-438: PublicModel fallback path looks correct.The import-inside-function pattern is acceptable for this fallback dependency. The query correctly omits user filtering since public models are shared across users.
frontend/src/i18n/locales/zh-CN/common.json (1)
270-270: Models i18n entries look consistent and completeThe new
settings.modelslabel andmodels.*block align well with the EN locale and the components’ usage (including allerrors.*keys). I don’t see missing or inconsistent keys on the zh-CN side.Also applies to: 431-469
frontend/src/i18n/locales/en/common.json (1)
269-269: English models i18n block matches UI and validation behaviorThe
settings.modelslabel andmodels.*strings cover all flows (list, create/edit, delete, test, and errors) and are consistent with the zh-CN locale and the form validation logic.Also applies to: 431-469
frontend/src/app/settings/page.tsx (1)
12-18: Models tab wiring (index, labels, and panels) is coherentThe tab index/name mappings, desktop and mobile Tab.List order, and Tab.Panels are all aligned with Models at index 2, so
?tab=modelsdeep-linking and navigation state should behave correctly without impacting existing tabs.Also applies to: 33-36, 45-48, 134-145, 182-184, 225-236, 274-276
| public_model_service.delete_model(db=db, model_id=model_id, current_user=current_user) | ||
| return {"message": "Model deleted successfully"} | ||
| return {"message": "Model deleted successfully"} | ||
|
|
||
|
|
||
| @router.post("/test-connection") | ||
| def test_model_connection( | ||
| test_data: dict, | ||
| current_user: User = Depends(security.get_current_user), | ||
| ): | ||
| """ | ||
| Test model connection | ||
| Request body: | ||
| { | ||
| "provider_type": "openai" | "anthropic", | ||
| "model_id": "gpt-4", | ||
| "api_key": "sk-...", | ||
| "base_url": "https://api.openai.com/v1" // optional | ||
| } | ||
| Response: | ||
| { | ||
| "success": true | false, | ||
| "message": "Connection successful" | "Error message" | ||
| } | ||
| """ | ||
| provider_type = test_data.get("provider_type") | ||
| model_id = test_data.get("model_id") | ||
| api_key = test_data.get("api_key") | ||
| base_url = test_data.get("base_url") | ||
|
|
||
| if not provider_type or not model_id or not api_key: | ||
| return { | ||
| "success": False, | ||
| "message": "Missing required fields: provider_type, model_id, api_key" | ||
| } | ||
|
|
||
| try: | ||
| if provider_type == "openai": | ||
| import openai | ||
| client = openai.OpenAI( | ||
| api_key=api_key, | ||
| base_url=base_url or "https://api.openai.com/v1" | ||
| ) | ||
| # Send minimal test request | ||
| response = client.chat.completions.create( | ||
| model=model_id, | ||
| messages=[{"role": "user", "content": "hi"}], | ||
| max_tokens=1 | ||
| ) | ||
| return { | ||
| "success": True, | ||
| "message": f"Successfully connected to {model_id}" | ||
| } | ||
|
|
||
| elif provider_type == "anthropic": | ||
| import anthropic | ||
| client = anthropic.Anthropic(api_key=api_key) | ||
| if base_url: | ||
| client.base_url = base_url | ||
|
|
||
| response = client.messages.create( | ||
| model=model_id, | ||
| max_tokens=1, | ||
| messages=[{"role": "user", "content": "hi"}] | ||
| ) | ||
| return { | ||
| "success": True, | ||
| "message": f"Successfully connected to {model_id}" | ||
| } | ||
|
|
||
| else: | ||
| return { | ||
| "success": False, | ||
| "message": "Unsupported provider type" | ||
| } | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Model connection test failed: {str(e)}") | ||
| return { | ||
| "success": False, | ||
| "message": f"Connection failed: {str(e)}" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the file size and read the full file to understand context
wc -l backend/app/api/endpoints/adapter/models.pyRepository: wecode-ai/Wegent
Length of output: 107
🏁 Script executed:
#!/bin/bash
# Read the file to see the full context around the flagged lines
head -n 260 backend/app/api/endpoints/adapter/models.py | tail -n 100Repository: wecode-ai/Wegent
Length of output: 2984
🏁 Script executed:
#!/bin/bash
# Search for URL validation patterns in the codebase
rg -n "base_url|validate.*url|SSRF|localhost|127\.0\.0\.1" backend/ -t py | head -30Repository: wecode-ai/Wegent
Length of output: 3058
🏁 Script executed:
#!/bin/bash
# Check for similar endpoint patterns and exception handling
rg -n "except Exception" backend/app/api/endpoints/ -t py -A 2 | head -40Repository: wecode-ai/Wegent
Length of output: 2688
🏁 Script executed:
#!/bin/bash
# Check for type hints usage in this file
rg -n "def " backend/app/api/endpoints/adapter/models.py | head -20Repository: wecode-ai/Wegent
Length of output: 277
🏁 Script executed:
#!/bin/bash
# Check imports and function signatures in the file
head -n 50 backend/app/api/endpoints/adapter/models.pyRepository: wecode-ai/Wegent
Length of output: 1520
🏁 Script executed:
#!/bin/bash
# Check if there's a Pydantic model for test connection request
rg -n "class.*Test.*Connection|class.*Model.*Test" backend/ -t py -A 3Repository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Verify the coding guidelines requirement for type hints
grep -r "Type hints required" backend/ || echo "Checking guidelines in documentation"
find . -name "*.md" -o -name "GUIDELINES*" | head -5Repository: wecode-ai/Wegent
Length of output: 197
Harden /test-connection against SSRF and tighten error handling/logging
Functionality is correct for both OpenAI and Anthropic, but there are important concerns:
- SSRF risk via
base_url: Passing user-suppliedbase_urldirectly into the OpenAI/Anthropic clients lets authenticated users make the server call arbitrary hosts/ports. At minimum, validatebase_url(schemehttpsonly, no localhost/private IPs, or restrict to an allowlist / configured proxy domains) before using it, or consider droppingbase_urlfrom this endpoint and deriving it server-side from a controlled config. - Overly broad exception handling and leakage to clients: Catching
Exceptionand returningf"Connection failed: {str(e)}"to the client exposes internal SDK error details. Prefer logging the full exception withlogger.exception("Model connection test failed: %s", e)and returning a generic message (optionally plus a short, sanitized hint). - Unused variables: The
responsevariables for both provider branches (lines 221, 236) are unused; remove assignments. - Missing type hints: Add explicit return type annotation (
-> dict) and consider creating a Pydantic model fortest_datainstead of usingdictfor consistency with other endpoints in the file.
🧰 Tools
🪛 Ruff (0.14.6)
177-177: Unused function argument: current_user
(ARG001)
177-177: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
231-231: Local variable response is assigned to but never used
Remove assignment to unused variable response
(F841)
247-247: Do not catch blind exception: Exception
(BLE001)
248-248: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
248-248: Use explicit conversion flag
Replace with conversion flag
(RUF010)
251-251: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In backend/app/api/endpoints/adapter/models.py around lines 170 to 252, harden
the test_model_connection endpoint by validating or removing user-supplied
base_url to prevent SSRF (accept only https, reject localhost/private
IPs/resolved addresses or prefer server-side configured/allowlisted proxy
domains), stop passing arbitrary base_url directly to SDK clients, replace broad
except usage with logger.exception("Model connection test failed") while
returning a generic sanitized error message to clients (do not include str(e)),
remove the unused response assignments in both provider branches, and add
explicit return type annotation (-> dict) and replace the plain dict request
body with a Pydantic request model (or at least add type hints) for consistent
validation.
Change API paths from /kinds/namespaces/... to /v1/namespaces/... to match the backend k_router prefix configuration.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/apis/models.ts (2)
16-21: Align provider naming betweenenv.modeldocs andprovider_typeunionRight now the inline comment on
env.modelsuggests'openai' | 'claude', whileTestConnectionRequest.provider_typeuses'openai' | 'anthropic'. That mismatch is a bit confusing.Consider:
- Standardizing on one provider identifier set (likely
'openai' | 'anthropic') across both places, and- If the provider set is stable, narrowing
env.modelfromstringto the same union so config drift is caught at compile time.If
env.modelis intentionally looser, at least updating the comment to reflect the real accepted values would avoid ambiguity.Also applies to: 44-50
75-123: API surface is clear; consider minor reuse of CRD base pathThe new CRUD methods and helper endpoints read clearly and use
encodeURIComponentcorrectly on dynamic segments. One small maintainability tweak you might consider:
- Extract a shared CRD base path constant to avoid repeating
'/v1/namespaces/default/models':-import { apiClient } from './client' +import { apiClient } from './client' + +const MODEL_CRD_BASE_PATH = '/v1/namespaces/default/models' @@ - async getAllModels(): Promise<ModelListResponse> { - return apiClient.get('/v1/namespaces/default/models') - }, + async getAllModels(): Promise<ModelListResponse> { + return apiClient.get(MODEL_CRD_BASE_PATH) + }, @@ - async getModel(name: string): Promise<ModelCRD> { - return apiClient.get(`/v1/namespaces/default/models/${encodeURIComponent(name)}`) - }, + async getModel(name: string): Promise<ModelCRD> { + return apiClient.get(`${MODEL_CRD_BASE_PATH}/${encodeURIComponent(name)}`) + }, @@ - async createModel(model: ModelCRD): Promise<ModelCRD> { - return apiClient.post('/v1/namespaces/default/models', model) - }, + async createModel(model: ModelCRD): Promise<ModelCRD> { + return apiClient.post(MODEL_CRD_BASE_PATH, model) + }, @@ - async updateModel(name: string, model: ModelCRD): Promise<ModelCRD> { - return apiClient.put(`/v1/namespaces/default/models/${encodeURIComponent(name)}`, model) - }, + async updateModel(name: string, model: ModelCRD): Promise<ModelCRD> { + return apiClient.put(`${MODEL_CRD_BASE_PATH}/${encodeURIComponent(name)}`, model) + }, @@ - async deleteModel(name: string): Promise<void> { - return apiClient.delete(`/v1/namespaces/default/models/${encodeURIComponent(name)}`) - }, + async deleteModel(name: string): Promise<void> { + return apiClient.delete(`${MODEL_CRD_BASE_PATH}/${encodeURIComponent(name)}`) + },This keeps the versioned CRD prefix in exactly one place if/when you need to support multiple namespaces or API versions later.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/apis/models.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/**/*.{ts,tsx}: TypeScript strict mode must be enabled in Frontend
Use functional components with hooks in React
Use Prettier formatter in Frontend with single quotes and no semicolons
Use ESLint with Next.js configuration in Frontend
Useconstoverlet, never usevarin TypeScript
Component names must use PascalCase; file names must use kebab-case in Frontend
Use mobile-first responsive design approach in Frontend components
Frontend design: Low saturation + low contrast for reduced eye strain with generous whitespace
Frontend design: Minimal shadows, subtle component differentiation (<10% background variance)
Frontend design: Use mint blue (#14B8A6) as primary accent sparingly
Use Tailwind CSS variables for colors: --color-bg-base, --color-text-primary, --color-border, etc.
Frontend spacing: Use standard units - p-2 (8px), p-4 (16px), p-6 (24px), gap-3 (12px)
Frontend border-radius: rounded-2xl (16px) for large containers, rounded-lg (12px) for cards, rounded-md (6px) for buttons
Frontend typography: H1 text-xl font-semibold, H2 text-lg font-semibold, H3 text-base font-medium, Body text-sm
Use shadcn/ui Button component with variants: default, secondary, ghost, outline, link
Use shadcn/ui Form component with react-hook-form + zod validation
Files:
frontend/src/apis/models.ts
frontend/**
📄 CodeRabbit inference engine (AGENTS.md)
Frontend: Only use
NEXT_PUBLIC_*for client-safe environment variables
Files:
frontend/src/apis/models.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.573Z
Learning: Use Kubernetes-inspired CRD design (Ghost, Model, Shell, Bot, Team, Task)
📚 Learning: 2025-11-26T07:34:32.573Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.573Z
Learning: Use Kubernetes-inspired CRD design (Ghost, Model, Shell, Bot, Team, Task)
Applied to files:
frontend/src/apis/models.ts
🧬 Code graph analysis (1)
frontend/src/apis/models.ts (1)
frontend/src/apis/client.ts (1)
apiClient(105-105)
🔇 Additional comments (2)
frontend/src/apis/models.ts (2)
7-33: ModelCRD and list response align well with CRD conventionsThe
ModelCRDshape (metadata/spec/status) andModelListResponsewrapper look consistent with the Kubernetes‑style CRD pattern you’re using elsewhere (Ghost, Model, Shell, Bot, Team, Task). This should make it straightforward to share generic CRD utilities across resources; no changes needed here.Based on learnings, this matches the intended CRD design.
44-65: Connection and compatibility types look coherent with the described APIs
TestConnectionRequest/ResponseandCompatibleModel(s)map cleanly onto the new/models/test-connectionand/models/compatibleendpoints described in the PR summary. The shapes are minimal but sufficient for the UI use cases (feedback message + list of compatible names), and the snake_case fields on the request side match typical backend JSON conventions.
- Add 'solo' option to team collaboration mode selection - Implement simplified UI for solo mode with single bot selector - Hide Transfer component and team prompts link in solo mode - Add i18n support for solo mode in both Chinese and English - Add solo.png placeholder image for mode illustration
There was a problem hiding this comment.
Actionable comments posted: 11
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)
471-489: Prevent accidental mutation/deletion ofPublicModelor shared user models inupdate_with_userThe
agent_configbranch inupdate_with_user(lines 549-633) assumes any non-Nonemodelreturned by_get_bot_componentsis a private, bot-dedicatedKind(Model). However,_get_bot_componentsexplicitly documents (line 36) that "Model can be from kinds table (private) or public_models table," creating two critical vulnerabilities:
Deleting global/public or shared models
At line 510, the code deletes without type verification:
if model and model.name != model_name: db.delete(model)If a bot previously pointed to a
PublicModel(which lacksuser_idownership), this will permanently delete that model, affecting all users/bots referencing it.Mutating global/public models with private config
At lines 525-530, the code mutates the model's CRD without guarding:
model_crd = Model.model_validate(model.json) model_crd.spec.modelConfig = model_config model_crd.spec.isCustomConfig = True model_crd.spec.protocol = protocol model.json = model_crd.model_dump()If
modelis aPublicModel, this overwrites the shared model with per-bot secrets and flags, corrupting it for all referencing bots/users.Silently swallowed DB errors
At lines 665-677, bare
except: passhides real database issues:if model and hasattr(model, 'id'): try: db.refresh(model) except Exception: pass # PublicModel may not need refreshRequired fix: Before deletion or mutation, verify
modelis a dedicated privateKindby checkingisinstance(model, Kind)and confirmingmodel.kind == "Model"andmodel.user_id == user_id. Additionally, replace bareexcept: passwith logged exception handling to surface DB issues.
♻️ Duplicate comments (2)
backend/app/services/adapters/executor_kinds.py (1)
409-414: Missinguser_idfilter may cause cross-user model access.This query doesn't filter by
Kind.user_id, unlike the existing model query at lines 329-334 which properly usesKind.user_id == team.user_id. This could return another user's model configuration when names collide.model_kind = db.query(Kind).filter( + Kind.user_id == team.user_id, Kind.kind == "Model", Kind.name == model_name_to_use, Kind.namespace == "default", - Kind.is_active == True + Kind.is_active ).first()backend/app/api/endpoints/adapter/models.py (1)
336-341: Uselogging.exceptionand sanitize error messages.Per static analysis and best practices:
except Exception as e: - logger.error(f"Model connection test failed: {str(e)}") + logger.exception("Model connection test failed") return { "success": False, - "message": f"Connection failed: {str(e)}" + "message": "Connection failed. Check logs for details." }Returning raw exception messages to clients can leak internal details. The previous review also flagged this.
🧹 Nitpick comments (37)
frontend/src/features/settings/components/team-modes/PipelineModeEditor.tsx (1)
11-25: Consider using shared type fromtypes.tsinstead of defining props inline.The
types.tsfile in the same directory already definesTeamModeEditorPropsand related interfaces. This inline interface duplicates the type definition and may diverge over time. Additionally, thetoastprop is declared but never used within this component.Consider either:
- Extending from
TransferModeEditorPropsintypes.ts, or- Removing the unused
toastprop from the interface if it's not needed.frontend/src/features/settings/components/team-modes/BotTransfer.tsx (1)
228-241: Add accessible labels to icon buttons.The Edit and Copy icons function as interactive buttons but lack accessible labels. Screen reader users won't understand their purpose.
<Edit className="ml-2 h-4 w-4 text-muted-foreground hover:text-foreground cursor-pointer" + aria-label={t('common.edit')} + role="button" + tabIndex={0} onClick={e => { e.stopPropagation(); onEditBot(Number(item.key)); }} /> <Copy className="ml-3 h-4 w-4 text-muted-foreground hover:text-foreground cursor-pointer" + aria-label={t('common.copy')} + role="button" + tabIndex={0} onClick={e => { e.stopPropagation(); onCloneBot(Number(item.key)); }} />frontend/src/features/settings/components/team-modes/LeaderModeEditor.tsx (2)
25-42: Unusedtoastprop in interface.The
toastprop is declared inLeaderModeEditorPropsbut is not used within the component. Consider removing it to keep the interface clean, or add the intended usage.export interface LeaderModeEditorProps { bots: Bot[]; selectedBotKeys: React.Key[]; setSelectedBotKeys: React.Dispatch<React.SetStateAction<React.Key[]>>; leaderBotId: number | null; setLeaderBotId: React.Dispatch<React.SetStateAction<number | null>>; unsavedPrompts: Record<string, string>; teamPromptMap: Map<number, boolean>; isDifyLeader: boolean; selectedAgentName: string | null; leaderOptions: Bot[]; - toast: ReturnType<typeof import('@/hooks/use-toast').useToast>['toast']; onEditBot: (botId: number) => void; onCreateBot: () => void; onCloneBot: (botId: number) => void; onOpenPromptDrawer: () => void; onLeaderChange: (botId: number) => void; }
79-88: Optimize repeated bot lookup.
bots.find(b => b.id === leaderBotId)is called twice in adjacent lines. Consider memoizing the selected leader bot to avoid redundant iterations.+ const selectedLeader = useMemo( + () => bots.find(b => b.id === leaderBotId), + [bots, leaderBotId] + ); + return ( <div className="rounded-md border border-border bg-base p-4 flex flex-col flex-1 min-h-0"> {/* Leader select */} ... {leaderBotId !== null ? ( <div className="flex items-center justify-between w-full"> <div className="flex items-center gap-2 flex-1 min-w-0"> <RiRobot2Line className="w-4 h-4 text-text-muted flex-shrink-0" /> <span className="truncate max-w-[200px]"> - {bots.find(b => b.id === leaderBotId)?.name || ''} + {selectedLeader?.name || ''} <span className="text-text-muted text-xs ml-1"> - ({bots.find(b => b.id === leaderBotId)?.agent_name || ''}) + ({selectedLeader?.agent_name || ''}) </span> </span> </div>frontend/src/apis/models.ts (2)
7-107: Consider moving type definitions tosrc/types/directory.Per coding guidelines, TypeScript types should be defined in the
src/types/directory. These interfaces (ModelCRD,UnifiedModel,TestConnectionRequest, etc.) could be extracted to a dedicated types file (e.g.,src/types/models.ts) and imported here.This is a recommended refactor for better organization and reusability across the codebase. Based on learnings from coding guidelines.
109-110: Remove duplicate comment.The
// Model Servicescomment appears twice consecutively.-// Model Services // Model Services export const modelApis = {backend/app/schemas/kind.py (1)
62-63: Consider adding validation for requiredprotocolwhenisCustomConfigis true.The comment states protocol is "Required for custom configs", but the field is
Optional[str]. If this requirement should be enforced, consider using a Pydantic model validator:from pydantic import model_validator class ModelSpec(BaseModel): """Model specification""" modelConfig: Dict[str, Any] isCustomConfig: Optional[bool] = None protocol: Optional[str] = None # Model protocol type: 'openai', 'claude', etc. @model_validator(mode='after') def validate_protocol_for_custom(self) -> 'ModelSpec': if self.isCustomConfig and not self.protocol: raise ValueError('protocol is required when isCustomConfig is True') return selfAlternatively, update the comment to clarify the field is optional but recommended for custom configs.
frontend/src/features/tasks/service/messageService.ts (1)
20-21: LGTM!The new
model_idandforce_override_bot_modelparameters correctly extend the message service to support task-level model overrides, aligning with theCreateTaskRequestinterface.Minor nit: Lines 65-66 could use ES6 shorthand property syntax:
- model_id: model_id, - force_override_bot_model: force_override_bot_model, + model_id, + force_override_bot_model,Also applies to: 65-66
frontend/src/features/settings/components/ModelList.tsx (3)
82-84: Remove duplicate comment.There's a duplicate comment on lines 82-83.
- // Convert to unified display format // Convert to unified display format const displayModels: DisplayModel[] = React.useMemo(() => {
131-131: Provider type detection may silently default to 'anthropic' for unknown providers.The ternary
env.model === 'openai' ? 'openai' : 'anthropic'assumes all non-OpenAI models are Anthropic. If other providers are added in the future (e.g., 'claude', 'gemini'), this could cause incorrect test connection requests.Consider explicit provider handling:
- provider_type: env.model === 'openai' ? 'openai' : 'anthropic', + provider_type: env.model === 'openai' ? 'openai' : env.model === 'claude' ? 'anthropic' : 'anthropic',Or validate and warn for unsupported providers before making the API call.
160-177: Consider adding loading state to prevent double-click during delete.The
handleDeletefunction doesn't disable the button while the delete operation is in progress, allowing potential duplicate requests if the user clicks multiple times.+ const [isDeleting, setIsDeleting] = useState(false); const handleDelete = async () => { if (!deleteConfirmModel) return; + setIsDeleting(true); try { await modelApis.deleteModel(deleteConfirmModel.metadata.name); // ... } catch (error) { // ... } finally { + setIsDeleting(false); } }; // In AlertDialogAction: - <AlertDialogAction onClick={handleDelete} className="bg-error hover:bg-error/90"> + <AlertDialogAction onClick={handleDelete} disabled={isDeleting} className="bg-error hover:bg-error/90"> + {isDeleting ? <Loader2 className="w-4 h-4 animate-spin mr-2" /> : null} {t('actions.delete')} </AlertDialogAction>backend/app/services/kind_impl.py (1)
58-61: Fix f-string and uselogging.exceptionfor better error diagnostics.The f-string on line 58 has no placeholders, and
logger.errorshould belogger.exceptionto capture the stack trace when re-raising.- logger.info(f"Encrypted API key for Model resource") + logger.info("Encrypted API key for Model resource") except Exception as e: - logger.error(f"Failed to encrypt API key: {str(e)}") + logger.exception("Failed to encrypt API key") raisefrontend/src/app/settings/page.tsx (1)
255-256: Stale comment references Bot instead of Model.The comment mentions "Bot Creation Modal" but this tab now displays
ModelList. Update for clarity.- {/* No Bot Creation Modal needed here as it's now part of the BotList component */} + {/* Model management is handled within the ModelList component */}backend/app/services/adapters/executor_kinds.py (1)
445-448: Uselogger.exceptionto capture stack trace on resolution failure.When model resolution fails, the stack trace would help diagnose issues in production.
except Exception as e: - logger.error(f"Failed to resolve model config: {e}") + logger.exception("Failed to resolve model config") # On any error, fallback to original agent_config agent_config_data = agent_configbackend/app/services/adapters/public_model.py (2)
238-248: Avoid bareexceptand clarify function purpose.The bare
except:catches all exceptions includingKeyboardInterrupt. Also,get_model_providerreturnsenv.get("model")which appears to return a model name (e.g., "claude") rather than a provider—consider renaming for clarity.def get_model_provider(json_data: Optional[Dict]) -> Optional[str]: + """Extract model protocol/provider from model config (e.g., 'openai', 'claude')""" if not isinstance(json_data, dict): return None try: model_crd = Model.model_validate(json_data) env = model_crd.spec.modelConfig.get("env", {}) if not isinstance(env, dict): return None return env.get("model") - except: + except (ValueError, KeyError, AttributeError): return None
250-258: Replace bareexceptwith specific exception types.def get_model_display_name(json_data: Optional[Dict]) -> Optional[str]: """Extract displayName from model metadata""" if not isinstance(json_data, dict): return None try: model_crd = Model.model_validate(json_data) return model_crd.metadata.displayName - except: + except (ValueError, KeyError, AttributeError): return Nonefrontend/src/features/settings/components/ModelEdit.tsx (1)
73-85: Potential issue withmodelOptionsin dependency array.The
modelOptionsvariable changes whenproviderTypechanges. Including it in the dependency array causes this effect to re-run when the provider switches, which may unintentionally reset the model selection state. Consider using a ref or checking ifmodelhas changed to avoid re-running logic meant only for initialization.+ const modelOptionsRef = useRef(modelOptions); + modelOptionsRef.current = modelOptions; + useEffect(() => { if (model?.spec.modelConfig?.env?.model_id) { const id = model.spec.modelConfig.env.model_id; - const isPreset = modelOptions.some(opt => opt.value === id && opt.value !== 'custom'); + const isPreset = modelOptionsRef.current.some(opt => opt.value === id && opt.value !== 'custom'); if (isPreset) { setModelId(id); setCustomModelId(''); } else { setModelId('custom'); setCustomModelId(id); } } - }, [model, modelOptions]); + }, [model]);frontend/src/features/settings/components/team-modes/SoloModeEditor.tsx (2)
37-46: Unused propeditingTeamin destructuring.The
editingTeamprop is defined inSoloModeEditorProps(line 30) and passed to the component but never used in the function body.export default function SoloModeEditor({ bots, setBots, selectedBotId, setSelectedBotId, - editingTeam, toast, unsavedPrompts = {}, teamPromptMap, onOpenPromptDrawer, }: SoloModeEditorProps) {Also remove from interface if not needed:
export interface SoloModeEditorProps { bots: Bot[]; setBots: React.Dispatch<React.SetStateAction<Bot[]>>; selectedBotId: number | null; setSelectedBotId: React.Dispatch<React.SetStateAction<number | null>>; - editingTeam: Team | null; toast: ReturnType<typeof import('@/hooks/use-toast').useToast>['toast'];
127-139: Reliance on bot array ordering may be fragile.The logic assumes the newest bot is at
bots[0], but this depends on how the parent component sorts the bots array. Consider finding the bot with the highest ID or most recentcreated_atinstead for more reliable selection after creation.frontend/src/features/tasks/components/ChatArea.tsx (1)
520-529: Consider extracting duplicated ModelSelector rendering.The ModelSelector is rendered identically in two places (lines 520-529 and 660-669). Consider extracting this into a reusable element or variable to reduce duplication and ensure consistency.
const modelSelectorElement = selectedTeam && ( <ModelSelector selectedModel={selectedModel} setSelectedModel={setSelectedModel} forceOverride={forceOverride} setForceOverride={setForceOverride} selectedTeam={selectedTeam} disabled={hasMessages || isLoading} /> ); // Then use {modelSelectorElement} in both locationsAlso applies to: 660-669
backend/app/services/adapters/team_kinds.py (3)
852-852: Remove unnecessary f-string prefix.The string at line 852 has an
fprefix but contains no placeholders. Remove thefprefix.- logger.info(f"[_get_bot_summary] Custom config (isCustomConfig=True), returning empty agent_config") + logger.info("[_get_bot_summary] Custom config (isCustomConfig=True), returning empty agent_config")Note: Same issue exists at line 936 in the duplicate method which should be removed.
802-803: Use module-level logger instead of creating new ones inside methods.The method creates a new logger inside the function, but a module-level logger already exists at line 11. Reuse the existing logger for consistency.
def _get_bot_summary(self, bot: Kind, db: Session, user_id: int) -> Dict[str, Any]: """ Get a summary of bot information including agent_config with only necessary fields. This is used for team list to determine if bots have predefined models. """ - import logging - logger = logging.getLogger(__name__) - from app.models.public_model import PublicModelThe module-level
loggerat line 11 will be used automatically.
749-780: Unnecessary re-query for first bot.The code queries
first_botby ID (lines 751-756), but this bot was already fetched in the loop above when processing members. Consider storing the first bot during iteration to avoid the extra database query.bots = [] agent_names = set() + first_bot_data = None # Store first bot for agent_type calculation for member in team_crd.spec.members: # Find bot in kinds table bot = db.query(Kind).filter( Kind.user_id == user_id, Kind.kind == "Bot", Kind.name == member.botRef.name, Kind.namespace == member.botRef.namespace, Kind.is_active == True ).first() if bot: + if first_bot_data is None: + first_bot_data = bot bot_summary = self._get_bot_summary(bot, db, user_id) # ... rest of loop ... # Get agent_type from the first bot's shell agent_type = None - if bots: - first_bot_id = bots[0]["bot_id"] - first_bot = db.query(Kind).filter( - Kind.id == first_bot_id, - Kind.user_id == user_id, - Kind.kind == "Bot", - Kind.is_active == True - ).first() - - if first_bot: + if first_bot_data: + bot_crd = Bot.model_validate(first_bot_data.json)shared/utils/crypto.py (2)
80-82: Uselogging.exceptionfor better error diagnostics.When logging exceptions,
logging.exceptionautomatically includes the stack trace, making debugging easier.except Exception as e: - logger.error(f"Failed to encrypt sensitive data: {str(e)}") + logger.exception("Failed to encrypt sensitive data") raise
126-129: Silent fallback to original text may hide decryption issues.Returning the original encrypted text on failure provides backward compatibility but could mask configuration issues (wrong keys). The warning log is appropriate, but consider using
logging.exceptionfor full stack trace:except Exception as e: - logger.warning(f"Failed to decrypt sensitive data: {str(e)}") + logger.warning("Failed to decrypt sensitive data: %s", e, exc_info=True) # Return the original text as fallback for backward compatibility return encrypted_textfrontend/src/features/settings/components/TeamEdit.tsx (1)
607-853: Consider extracting inline CSS to a separate stylesheet or CSS module.While the
dangerouslySetInnerHTMLusage here is safe (static CSS strings, no user input), it's a code smell that affects maintainability:
- CSS is duplicated/interleaved with JSX
- Static analysis tools flag it as potential XSS
- Makes the component harder to read
Consider moving these styles to a CSS module or using Tailwind's
@applyin a global stylesheet. This would eliminate the security warnings and improve maintainability.frontend/src/features/tasks/components/ModelSelector.tsx (1)
59-61: Consider using a constant or config file for localStorage keys.These localStorage keys are defined inline. If they need to be shared or accessed elsewhere, consider extracting to a shared constants file.
backend/app/api/endpoints/adapter/models.py (3)
139-139: Move import to module level.The
HTTPExceptionimport should be at the module level with other imports, not inside the function.-from fastapi import APIRouter, Depends, Query, status +from fastapi import APIRouter, Depends, HTTPException, Query, statusThen remove Line 139.
304-308: Remove unusedresponsevariable assignments.The
responsevariables are assigned but never used.- response = client.chat.completions.create( + client.chat.completions.create( model=model_id, messages=[{"role": "user", "content": "hi"}], max_tokens=1 )Same for the Anthropic branch at Line 320.
Also applies to: 320-324
344-394: Consider usingmodel_aggregation_service.list_available_modelsinstead.This endpoint duplicates logic that already exists in
model_aggregation_service.list_available_modelswithagent_namefiltering. Consider refactoring to use the existing service:@router.get("/compatible") def get_compatible_models( agent_name: str = Query(..., description="Agent name (Agno or ClaudeCode)"), current_user: User = Depends(security.get_current_user), db: Session = Depends(get_db) ): models = model_aggregation_service.list_available_models( db=db, current_user=current_user, agent_name=agent_name ) return {"models": [{"name": m["name"]} for m in models]}This reduces code duplication and ensures consistent filtering logic.
backend/app/services/model_aggregation_service.py (2)
265-270: Consider making the public models limit configurable or paginated.The hardcoded
limit=1000works for current scale but may become a performance issue as the number of public models grows. Consider making this configurable or implementing proper pagination.# At class level or module level PUBLIC_MODELS_FETCH_LIMIT = 1000
358-380: Inefficient public model lookup - fetches all models to find one.When looking up a public model by name, this fetches up to 1000 models and filters client-side. Consider adding a direct lookup method to
public_model_service:# In public_model_service def get_by_name(self, db: Session, name: str, current_user: User) -> Optional[Dict]: ...This would be more efficient for single-model lookups.
backend/app/services/adapters/bot_kinds.py (5)
105-195: Model lookup helper works but could be cleaned up (logging &== Truestyle)The
_get_model_by_name_and_type/_get_model_by_namehelpers look functionally correct and align with the “user first, then public” resolution strategy.Two niceties you might want to address:
- You’re importing
loggingand building the logger on every call. Consider a module-level logger (logger = logging.getLogger(__name__)) instead, reusing it in this and other helpers.- Ruff flags
== Truecomparisons on ORM columns (E712). For SQLAlchemy it’s safer/more explicit to use.is_(True)rather than a direct equality. For the new queries in this helper you could do:- Kind.is_active == True + Kind.is_active.is_(True) ... - PublicModel.is_active == True + PublicModel.is_active.is_(True) ... - Kind.is_active == True + Kind.is_active.is_(True) ... - PublicModel.is_active == True + PublicModel.is_active.is_(True)This keeps Ruff happy and avoids any ambiguity around boolean column comparisons.
254-300: Create path correctly wires modelRef, but consider existence checks and encryption usageThe new
create_with_userflow for models behaves as intended:
- Predefined config (
{bind_model, bind_model_type?}) → just setmodelRefto that name/namespace.- Custom config → create a private
ModelKind withmodelConfig,protocol, andisCustomConfig=True, then pointmodelRefto it.Two points worth tightening:
No validation that the referenced
bind_modelexistsWhen
_is_predefined_model(obj_in.agent_config)is true,create_with_usernever checks that aKind(Model)orPublicModelwith that name/namespace actually exists before committing the Bot. You only attempt to load the model afterward for the response:if model is None: model = self._get_model_by_name(db, model_ref_name, model_ref_namespace, user_id)which can return
Nonewithout failing the request. That leaves persisted bots with danglingmodelRefs if someone POSTs an arbitrarybind_modelvia the API.Consider validating and raising 4xx on create if
_get_model_by_name(...)returnsNonein the predefined branch.Encrypted config isn’t used when persisting the private Model
You compute:
encrypted_agent_config = self._encrypt_agent_config(obj_in.agent_config)but the
modelConfigfor private models is built fromobj_in.agent_configinstead. If the intention is to encrypt sensitive fields (e.g. keys underenv), you likely want:
protocol = self._get_protocol_from_config(obj_in.agent_config)model_config = {k: v for k, v in obj_in.agent_config.items() if k != "protocol"}
protocol = self._get_protocol_from_config(encrypted_agent_config)model_config = {k: vfor k, v in encrypted_agent_config.items()if k != "protocol"}so that the stored `Model` CRD benefits from the same encryption as other agent configs.Both are behavioral changes, so please double-check against how other model-management paths handle validation and encryption before applying.
Also applies to: 345-360, 385-393
742-783: Model resolution in_get_bot_componentsis correct; ensure legacy bots always havemodelRefSwitching
_get_bot_componentsto use_get_model_by_nameis a good way to support both privateKind(Model)andPublicModelsources with a single call, and the extra logging will help with debugging.One thing to verify: this now assumes that every bot CRD has a valid
spec.modelRef. For any legacy bots that previously relied on inlineagent_configorprivate_model-style fields,Bot.model_validate(bot.json)followed bybot_crd.spec.modelRefwill fail ifmodelRefis missing. If you still have such data in production, you might need a migration or a fallback path here to keep those bots loadable.
785-852: Batch model resolution is efficient; minor style tweak forPublicModel.is_activeThe batched fetch strategy is solid: you avoid N+1 queries, use key sets for Ghost/Shell/Model, and then fill in any missing models from
PublicModel. The combinedmodel_map: {(name, namespace): Kind or PublicModel}nicely lines up with how_convert_to_bot_dicttreats themodelparameter.Ruff’s E712 warning about
PublicModel.is_active == Truealso applies here; for SQLAlchemy, it’s clearer (and warning-free) to use:- public_models = db.query(PublicModel).filter( - PublicModel.is_active == True - ).filter(public_model_filter).all() + public_models = ( + db.query(PublicModel) + .filter(PublicModel.is_active.is_(True)) + .filter(public_model_filter) + .all() + )to match the style update you may apply in
_get_model_by_name_and_type.
854-945:_convert_to_bot_dictlogic matches the new model semantics; tighten type hints per Ruff and guidelinesThe expanded
_convert_to_bot_dictcorrectly encodes the various scenarios:
override_agent_config→ echo{bind_model, bind_model_type}back (predefined).modelisPublicModel→{bind_model, bind_model_type="public"}.modelRef.name != "{bot.name}-model"→ treat as shared user model (bind_model_type="user").- Dedicated private model with
isCustomConfig=True→ full config +protocol.- Dedicated private but not custom → bind-by-name for backward compatibility.
Two small improvements:
Make optionals explicit and type
modelRuff’s RUF013 warning is about implicit
Optional. You can also fully typemodelas “private or public or None” and the override config as optional to align with your code style guide (“type hints required for all Python function signatures”):
- def _convert_to_bot_dict(self, bot: Kind, ghost: Kind = None, shell: Kind = None, model = None, override_agent_config: Dict[str, Any] = None) -> Dict[str, Any]:
- def _convert_to_bot_dict(
self,bot: Kind,ghost: Kind | None = None,shell: Kind | None = None,model: Kind | PublicModel | None = None,override_agent_config: Dict[str, Any] | None = None,- ) -> Dict[str, Any]:
This matches how you actually use the parameters and satisfies Ruff’s Optional guidance. 2. **Reuse a shared logger instead of per-call imports (optional)** As with the other helpers, you’re creating a logger inside the function. If you move to a module-level logger (`logger = logging.getLogger(__name__)`), this function can just reference it and avoid repeated imports. Overall the branching looks correct; once the update/delete semantics in `update_with_user` are fixed, this method should give the frontend a consistent view of how each bot is bound to its model. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between c988717ad2a0b25ff233f9ac1411da86ce9ff54e and 5227c0bdf8126eee8be212f23db8ad07488e7576. </details> <details> <summary>⛔ Files ignored due to path filters (2)</summary> * `frontend/package-lock.json` is excluded by `!**/package-lock.json` * `frontend/public/settings/solo.png` is excluded by `!**/*.png` </details> <details> <summary>📒 Files selected for processing (37)</summary> * `backend/app/api/endpoints/adapter/bots.py` (1 hunks) * `backend/app/api/endpoints/adapter/models.py` (4 hunks) * `backend/app/schemas/kind.py` (2 hunks) * `backend/app/schemas/team.py` (1 hunks) * `backend/app/services/adapters/bot_kinds.py` (16 hunks) * `backend/app/services/adapters/executor_kinds.py` (3 hunks) * `backend/app/services/adapters/public_model.py` (4 hunks) * `backend/app/services/adapters/task_kinds.py` (1 hunks) * `backend/app/services/adapters/team_kinds.py` (3 hunks) * `backend/app/services/kind_impl.py` (2 hunks) * `backend/app/services/model_aggregation_service.py` (1 hunks) * `frontend/package.json` (1 hunks) * `frontend/src/apis/models.ts` (1 hunks) * `frontend/src/apis/tasks.ts` (1 hunks) * `frontend/src/app/settings/page.tsx` (8 hunks) * `frontend/src/components/ui/alert-dialog.tsx` (1 hunks) * `frontend/src/features/settings/components/BotEdit.tsx` (25 hunks) * `frontend/src/features/settings/components/ModelEdit.tsx` (1 hunks) * `frontend/src/features/settings/components/ModelList.tsx` (1 hunks) * `frontend/src/features/settings/components/TeamEdit.tsx` (16 hunks) * `frontend/src/features/settings/components/team-modes/BotTransfer.tsx` (1 hunks) * `frontend/src/features/settings/components/team-modes/LeaderModeEditor.tsx` (1 hunks) * `frontend/src/features/settings/components/team-modes/PipelineModeEditor.tsx` (1 hunks) * `frontend/src/features/settings/components/team-modes/SoloModeEditor.tsx` (1 hunks) * `frontend/src/features/settings/components/team-modes/index.ts` (1 hunks) * `frontend/src/features/settings/components/team-modes/types.ts` (1 hunks) * `frontend/src/features/settings/services/bots.ts` (3 hunks) * `frontend/src/features/tasks/components/ChatArea.tsx` (7 hunks) * `frontend/src/features/tasks/components/ClarificationForm.tsx` (1 hunks) * `frontend/src/features/tasks/components/ModelSelector.tsx` (1 hunks) * `frontend/src/features/tasks/components/TeamSelector.tsx` (1 hunks) * `frontend/src/features/tasks/service/messageService.ts` (4 hunks) * `frontend/src/i18n/locales/en/common.json` (6 hunks) * `frontend/src/i18n/locales/zh-CN/common.json` (6 hunks) * `frontend/src/types/api.ts` (1 hunks) * `shared/tests/utils/test_crypto.py` (2 hunks) * `shared/utils/crypto.py` (4 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (5)</summary> <details> <summary>frontend/**/*.{ts,tsx}</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > `frontend/**/*.{ts,tsx}`: TypeScript strict mode must be enabled in Frontend > Use functional components with hooks in React > Use Prettier formatter in Frontend with single quotes and no semicolons > Use ESLint with Next.js configuration in Frontend > Use `const` over `let`, never use `var` in TypeScript > Component names must use PascalCase; file names must use kebab-case in Frontend > Use mobile-first responsive design approach in Frontend components > Frontend design: Low saturation + low contrast for reduced eye strain with generous whitespace > Frontend design: Minimal shadows, subtle component differentiation (<10% background variance) > Frontend design: Use mint blue (#14B8A6) as primary accent sparingly > Use Tailwind CSS variables for colors: --color-bg-base, --color-text-primary, --color-border, etc. > Frontend spacing: Use standard units - p-2 (8px), p-4 (16px), p-6 (24px), gap-3 (12px) > Frontend border-radius: rounded-2xl (16px) for large containers, rounded-lg (12px) for cards, rounded-md (6px) for buttons > Frontend typography: H1 text-xl font-semibold, H2 text-lg font-semibold, H3 text-base font-medium, Body text-sm > Use shadcn/ui Button component with variants: default, secondary, ghost, outline, link > Use shadcn/ui Form component with react-hook-form + zod validation Files: - `frontend/src/features/settings/components/team-modes/PipelineModeEditor.tsx` - `frontend/src/apis/tasks.ts` - `frontend/src/features/tasks/components/ClarificationForm.tsx` - `frontend/src/features/settings/components/team-modes/types.ts` - `frontend/src/features/tasks/components/TeamSelector.tsx` - `frontend/src/features/settings/components/team-modes/BotTransfer.tsx` - `frontend/src/types/api.ts` - `frontend/src/features/settings/components/team-modes/SoloModeEditor.tsx` - `frontend/src/features/settings/components/team-modes/index.ts` - `frontend/src/features/settings/components/ModelEdit.tsx` - `frontend/src/features/settings/components/team-modes/LeaderModeEditor.tsx` - `frontend/src/features/settings/components/BotEdit.tsx` - `frontend/src/features/tasks/service/messageService.ts` - `frontend/src/features/settings/components/ModelList.tsx` - `frontend/src/features/tasks/components/ModelSelector.tsx` - `frontend/src/features/tasks/components/ChatArea.tsx` - `frontend/src/components/ui/alert-dialog.tsx` - `frontend/src/features/settings/services/bots.ts` - `frontend/src/features/settings/components/TeamEdit.tsx` - `frontend/src/app/settings/page.tsx` - `frontend/src/apis/models.ts` </details> <details> <summary>frontend/**</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > Frontend: Only use `NEXT_PUBLIC_*` for client-safe environment variables Files: - `frontend/src/features/settings/components/team-modes/PipelineModeEditor.tsx` - `frontend/src/apis/tasks.ts` - `frontend/src/features/tasks/components/ClarificationForm.tsx` - `frontend/src/features/settings/components/team-modes/types.ts` - `frontend/package.json` - `frontend/src/features/tasks/components/TeamSelector.tsx` - `frontend/src/features/settings/components/team-modes/BotTransfer.tsx` - `frontend/src/types/api.ts` - `frontend/src/features/settings/components/team-modes/SoloModeEditor.tsx` - `frontend/src/features/settings/components/team-modes/index.ts` - `frontend/src/features/settings/components/ModelEdit.tsx` - `frontend/src/features/settings/components/team-modes/LeaderModeEditor.tsx` - `frontend/src/features/settings/components/BotEdit.tsx` - `frontend/src/features/tasks/service/messageService.ts` - `frontend/src/features/settings/components/ModelList.tsx` - `frontend/src/features/tasks/components/ModelSelector.tsx` - `frontend/src/features/tasks/components/ChatArea.tsx` - `frontend/src/components/ui/alert-dialog.tsx` - `frontend/src/i18n/locales/en/common.json` - `frontend/src/features/settings/services/bots.ts` - `frontend/src/features/settings/components/TeamEdit.tsx` - `frontend/src/app/settings/page.tsx` - `frontend/src/apis/models.ts` - `frontend/src/i18n/locales/zh-CN/common.json` </details> <details> <summary>**/*.py</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > `**/*.py`: Python code must be PEP 8 compliant with Black formatter (line length: 88) > Use isort for import organization in Python > Python functions and variables must have descriptive names > Docstrings required for all public Python functions and classes > Extract magic numbers to named constants in Python > Python functions must not exceed 50 lines > Type hints required for all Python function signatures > Run pylint and flake8 for Python code quality checks Files: - `backend/app/schemas/kind.py` - `backend/app/api/endpoints/adapter/bots.py` - `backend/app/services/kind_impl.py` - `backend/app/schemas/team.py` - `backend/app/services/adapters/task_kinds.py` - `shared/utils/crypto.py` - `backend/app/services/adapters/team_kinds.py` - `backend/app/services/adapters/public_model.py` - `backend/app/api/endpoints/adapter/models.py` - `backend/app/services/adapters/executor_kinds.py` - `backend/app/services/adapters/bot_kinds.py` - `backend/app/services/model_aggregation_service.py` - `shared/tests/utils/test_crypto.py` </details> <details> <summary>frontend/src/types/**/*.{ts,tsx}</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > Define TypeScript types in `src/types/` directory in Frontend Files: - `frontend/src/types/api.ts` </details> <details> <summary>**/test_*.py</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > `**/test_*.py`: Follow AAA pattern in tests: Arrange, Act, Assert > Mock external services (Anthropic, OpenAI, Docker, APIs) in Python tests > Use descriptive test names explaining what is tested in Python tests > Test edge cases and error conditions in Python tests > Keep Python tests independent and isolated Files: - `shared/tests/utils/test_crypto.py` </details> </details><details> <summary>🧠 Learnings (9)</summary> <details> <summary>📚 Learning: 2025-11-26T07:34:32.591Z</summary>Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/src/types/**/*.{ts,tsx} : Define TypeScript types insrc/types/directory in Frontend**Applied to files:** - `frontend/src/features/settings/components/team-modes/types.ts` - `frontend/src/features/settings/components/team-modes/index.ts` - `frontend/src/apis/models.ts` </details> <details> <summary>📚 Learning: 2025-11-26T07:34:32.591Z</summary>Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : TypeScript strict mode must be enabled in Frontend**Applied to files:** - `frontend/src/features/settings/components/team-modes/types.ts` </details> <details> <summary>📚 Learning: 2025-11-26T07:34:32.591Z</summary>Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use shadcn/ui Form component with react-hook-form + zod validation**Applied to files:** - `frontend/package.json` </details> <details> <summary>📚 Learning: 2025-11-26T07:34:32.591Z</summary>Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use mobile-first responsive design approach in Frontend components**Applied to files:** - `frontend/src/features/tasks/components/TeamSelector.tsx` - `frontend/src/features/settings/components/TeamEdit.tsx` </details> <details> <summary>📚 Learning: 2025-11-26T07:34:32.591Z</summary>Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Frontend design: Minimal shadows, subtle component differentiation (<10% background variance)**Applied to files:** - `frontend/src/features/tasks/components/TeamSelector.tsx` - `frontend/src/components/ui/alert-dialog.tsx` </details> <details> <summary>📚 Learning: 2025-11-26T07:34:32.591Z</summary>Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Frontend border-radius: rounded-2xl (16px) for large containers, rounded-lg (12px) for cards, rounded-md (6px) for buttons**Applied to files:** - `frontend/src/features/tasks/components/TeamSelector.tsx` </details> <details> <summary>📚 Learning: 2025-11-26T07:34:32.591Z</summary>Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Frontend design: Low saturation + low contrast for reduced eye strain with generous whitespace**Applied to files:** - `frontend/src/features/tasks/components/TeamSelector.tsx` </details> <details> <summary>📚 Learning: 2025-11-26T07:34:32.591Z</summary>Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use functional components with hooks in React**Applied to files:** - `frontend/src/features/settings/components/BotEdit.tsx` - `frontend/src/features/settings/components/TeamEdit.tsx` </details> <details> <summary>📚 Learning: 2025-11-26T07:34:32.591Z</summary>Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use shadcn/ui Button component with variants: default, secondary, ghost, outline, link**Applied to files:** - `frontend/src/features/settings/components/BotEdit.tsx` - `frontend/src/components/ui/alert-dialog.tsx` </details> </details><details> <summary>🧬 Code graph analysis (14)</summary> <details> <summary>frontend/src/features/settings/components/team-modes/PipelineModeEditor.tsx (1)</summary><blockquote> <details> <summary>frontend/src/features/settings/components/team-modes/BotTransfer.tsx (1)</summary> * `BotTransfer` (38-267) </details> </blockquote></details> <details> <summary>frontend/src/features/settings/components/team-modes/types.ts (2)</summary><blockquote> <details> <summary>frontend/src/features/settings/components/team-modes/SoloModeEditor.tsx (1)</summary> * `SoloModeEditorProps` (25-35) </details> <details> <summary>frontend/src/features/settings/components/team-modes/LeaderModeEditor.tsx (1)</summary> * `LeaderModeEditorProps` (25-42) </details> </blockquote></details> <details> <summary>frontend/src/types/api.ts (1)</summary><blockquote> <details> <summary>backend/app/schemas/team.py (1)</summary> * `BotSummary` (13-16) </details> </blockquote></details> <details> <summary>frontend/src/features/settings/components/team-modes/LeaderModeEditor.tsx (2)</summary><blockquote> <details> <summary>frontend/src/utils/styles.ts (1)</summary> * `getPromptBadgeStyle` (44-74) </details> <details> <summary>frontend/src/features/settings/components/team-modes/BotTransfer.tsx (1)</summary> * `BotTransfer` (38-267) </details> </blockquote></details> <details> <summary>frontend/src/features/tasks/service/messageService.ts (1)</summary><blockquote> <details> <summary>frontend/src/apis/tasks.ts (1)</summary> * `CreateTaskRequest` (9-24) </details> </blockquote></details> <details> <summary>frontend/src/features/tasks/components/ChatArea.tsx (4)</summary><blockquote> <details> <summary>backend/app/schemas/kind.py (1)</summary> * `Model` (71-77) </details> <details> <summary>frontend/src/apis/models.ts (1)</summary> * `Model` (61-64) </details> <details> <summary>frontend/src/features/tasks/components/ModelSelector.tsx (3)</summary> * `Model` (29-34) * `DEFAULT_MODEL_NAME` (37-37) * `ModelSelector` (92-420) </details> <details> <summary>frontend/src/features/tasks/service/messageService.ts (1)</summary> * `sendMessage` (13-75) </details> </blockquote></details> <details> <summary>backend/app/api/endpoints/adapter/models.py (3)</summary><blockquote> <details> <summary>backend/app/models/kind.py (1)</summary> * `Kind` (14-30) </details> <details> <summary>backend/app/services/model_aggregation_service.py (3)</summary> * `ModelType` (29-37) * `list_available_models` (196-312) * `resolve_model` (384-421) </details> <details> <summary>backend/app/services/adapters/public_model.py (1)</summary> * `list_model_names` (201-298) </details> </blockquote></details> <details> <summary>backend/app/services/adapters/executor_kinds.py (3)</summary><blockquote> <details> <summary>shared/utils/crypto.py (1)</summary> * `decrypt_api_key` (222-239) </details> <details> <summary>backend/app/models/kind.py (1)</summary> * `Kind` (14-30) </details> <details> <summary>backend/app/schemas/kind.py (1)</summary> * `Model` (71-77) </details> </blockquote></details> <details> <summary>backend/app/services/adapters/bot_kinds.py (2)</summary><blockquote> <details> <summary>backend/app/models/kind.py (1)</summary> * `Kind` (14-30) </details> <details> <summary>backend/app/schemas/kind.py (2)</summary> * `Bot` (146-152) * `Model` (71-77) </details> </blockquote></details> <details> <summary>frontend/src/components/ui/alert-dialog.tsx (2)</summary><blockquote> <details> <summary>frontend/src/lib/utils.ts (1)</summary> * `cn` (8-10) </details> <details> <summary>frontend/src/components/ui/button.tsx (1)</summary> * `buttonVariants` (55-55) </details> </blockquote></details> <details> <summary>frontend/src/features/settings/components/TeamEdit.tsx (4)</summary><blockquote> <details> <summary>frontend/src/features/settings/components/team-modes/SoloModeEditor.tsx (1)</summary> * `SoloModeEditor` (37-277) </details> <details> <summary>frontend/src/features/settings/components/team-modes/PipelineModeEditor.tsx (1)</summary> * `PipelineModeEditor` (27-60) </details> <details> <summary>frontend/src/features/settings/components/team-modes/LeaderModeEditor.tsx (1)</summary> * `LeaderModeEditor` (44-206) </details> <details> <summary>frontend/src/components/ui/dialog.tsx (6)</summary> * `Dialog` (92-92) * `DialogContent` (97-97) * `DialogHeader` (98-98) * `DialogTitle` (100-100) * `DialogDescription` (101-101) * `DialogFooter` (99-99) </details> </blockquote></details> <details> <summary>frontend/src/app/settings/page.tsx (1)</summary><blockquote> <details> <summary>backend/app/schemas/kind.py (1)</summary> * `ModelList` (80-84) </details> </blockquote></details> <details> <summary>frontend/src/apis/models.ts (4)</summary><blockquote> <details> <summary>backend/app/schemas/kind.py (1)</summary> * `Model` (71-77) </details> <details> <summary>frontend/src/features/tasks/components/ModelSelector.tsx (1)</summary> * `Model` (29-34) </details> <details> <summary>backend/app/services/model_aggregation_service.py (1)</summary> * `UnifiedModel` (40-92) </details> <details> <summary>frontend/src/apis/client.ts (1)</summary> * `apiClient` (105-105) </details> </blockquote></details> <details> <summary>shared/tests/utils/test_crypto.py (1)</summary><blockquote> <details> <summary>shared/utils/crypto.py (10)</summary> * `is_token_encrypted` (185-195) * `encrypt_api_key` (202-219) * `decrypt_api_key` (222-239) * `is_api_key_encrypted` (242-261) * `mask_api_key` (264-285) * `encrypt_sensitive_data` (42-82) * `decrypt_sensitive_data` (85-129) * `is_data_encrypted` (132-152) * `encrypt_git_token` (159-169) * `decrypt_git_token` (172-182) </details> </blockquote></details> </details><details> <summary>🪛 ast-grep (0.40.0)</summary> <details> <summary>frontend/src/features/settings/components/TeamEdit.tsx</summary> [warning] 608-608: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks. Context: dangerouslySetInnerHTML Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES] - https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml - https://cwe.mitre.org/data/definitions/79.html (react-unsafe-html-injection) --- [warning] 856-856: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks. Context: dangerouslySetInnerHTML Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES] - https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml - https://cwe.mitre.org/data/definitions/79.html (react-unsafe-html-injection) </details> </details> <details> <summary>🪛 Biome (2.1.2)</summary> <details> <summary>frontend/src/features/settings/components/TeamEdit.tsx</summary> [error] 609-609: Avoid passing content using the dangerouslySetInnerHTML prop. Setting content using code can expose users to cross-site scripting (XSS) attacks (lint/security/noDangerouslySetInnerHtml) --- [error] 857-857: Avoid passing content using the dangerouslySetInnerHTML prop. Setting content using code can expose users to cross-site scripting (XSS) attacks (lint/security/noDangerouslySetInnerHtml) </details> </details> <details> <summary>🪛 Ruff (0.14.6)</summary> <details> <summary>backend/app/services/kind_impl.py</summary> 58-58: f-string without any placeholders Remove extraneous `f` prefix (F541) --- 60-60: Use `logging.exception` instead of `logging.error` Replace with `exception` (TRY400) --- 60-60: Use explicit conversion flag Replace with conversion flag (RUF010) --- 78-78: Do not catch blind exception: `Exception` (BLE001) --- 79-79: Use explicit conversion flag Replace with conversion flag (RUF010) </details> <details> <summary>shared/utils/crypto.py</summary> 81-81: Use `logging.exception` instead of `logging.error` Replace with `exception` (TRY400) --- 81-81: Use explicit conversion flag Replace with conversion flag (RUF010) --- 127-127: Use explicit conversion flag Replace with conversion flag (RUF010) --- 151-151: Do not catch blind exception: `Exception` (BLE001) </details> <details> <summary>backend/app/services/adapters/team_kinds.py</summary> 755-755: Avoid equality comparisons to `True`; use `Kind.is_active:` for truth checks Replace with `Kind.is_active` (E712) --- 765-765: Avoid equality comparisons to `True`; use `Kind.is_active:` for truth checks Replace with `Kind.is_active` (E712) --- 817-817: Avoid equality comparisons to `True`; use `Kind.is_active:` for truth checks Replace with `Kind.is_active` (E712) --- 835-835: Avoid equality comparisons to `True`; use `Kind.is_active:` for truth checks Replace with `Kind.is_active` (E712) --- 852-852: f-string without any placeholders Remove extraneous `f` prefix (F541) --- 862-862: Avoid equality comparisons to `True`; use `PublicModel.is_active:` for truth checks Replace with `PublicModel.is_active` (E712) --- 901-901: Avoid equality comparisons to `True`; use `Kind.is_active:` for truth checks Replace with `Kind.is_active` (E712) --- 919-919: Avoid equality comparisons to `True`; use `Kind.is_active:` for truth checks Replace with `Kind.is_active` (E712) --- 936-936: f-string without any placeholders Remove extraneous `f` prefix (F541) --- 946-946: Avoid equality comparisons to `True`; use `PublicModel.is_active:` for truth checks Replace with `PublicModel.is_active` (E712) </details> <details> <summary>backend/app/services/adapters/public_model.py</summary> 256-256: Consider moving this statement to an `else` block (TRY300) --- 257-257: Do not use bare `except` (E722) --- 280-280: Avoid equality comparisons to `True`; use `Kind.is_active:` for truth checks Replace with `Kind.is_active` (E712) --- 289-289: Avoid equality comparisons to `True`; use `PublicModel.is_active:` for truth checks Replace with `PublicModel.is_active` (E712) </details> <details> <summary>backend/app/api/endpoints/adapter/models.py</summary> 70-70: Do not perform function call `Depends` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008) --- 71-71: Do not perform function call `Depends` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008) --- 113-113: Do not perform function call `Depends` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008) --- 114-114: Do not perform function call `Depends` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008) --- 266-266: Unused function argument: `current_user` (ARG001) --- 266-266: Do not perform function call `Depends` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008) --- 320-320: Local variable `response` is assigned to but never used Remove assignment to unused variable `response` (F841) --- 336-336: Do not catch blind exception: `Exception` (BLE001) --- 337-337: Use `logging.exception` instead of `logging.error` Replace with `exception` (TRY400) --- 337-337: Use explicit conversion flag Replace with conversion flag (RUF010) --- 340-340: Use explicit conversion flag Replace with conversion flag (RUF010) --- 347-347: Do not perform function call `Depends` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008) --- 348-348: Do not perform function call `Depends` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008) --- 371-371: Avoid equality comparisons to `True`; use `Kind.is_active:` for truth checks Replace with `Kind.is_active` (E712) --- 391-391: Do not catch blind exception: `Exception` (BLE001) </details> <details> <summary>backend/app/services/adapters/executor_kinds.py</summary> 413-413: Avoid equality comparisons to `True`; use `Kind.is_active:` for truth checks Replace with `Kind.is_active` (E712) --- 426-426: Do not catch blind exception: `Exception` (BLE001) --- 445-445: Do not catch blind exception: `Exception` (BLE001) --- 446-446: Use `logging.exception` instead of `logging.error` Replace with `exception` (TRY400) </details> <details> <summary>backend/app/services/adapters/bot_kinds.py</summary> 138-138: Avoid equality comparisons to `True`; use `Kind.is_active:` for truth checks Replace with `Kind.is_active` (E712) --- 151-151: Avoid equality comparisons to `True`; use `PublicModel.is_active:` for truth checks Replace with `PublicModel.is_active` (E712) --- 166-166: Avoid equality comparisons to `True`; use `Kind.is_active:` for truth checks Replace with `Kind.is_active` (E712) --- 177-177: Avoid equality comparisons to `True`; use `PublicModel.is_active:` for truth checks Replace with `PublicModel.is_active` (E712) --- 577-577: f-string without any placeholders Remove extraneous `f` prefix (F541) --- 593-593: f-string without any placeholders Remove extraneous `f` prefix (F541) --- 676-677: `try`-`except`-`pass` detected, consider logging the exception (S110) --- 676-676: Do not catch blind exception: `Exception` (BLE001) --- 846-846: Avoid equality comparisons to `True`; use `PublicModel.is_active:` for truth checks Replace with `PublicModel.is_active` (E712) --- 854-854: PEP 484 prohibits implicit `Optional` Convert to `T | None` (RUF013) </details> <details> <summary>backend/app/services/model_aggregation_service.py</summary> 130-130: Do not catch blind exception: `Exception` (BLE001) --- 191-191: Do not catch blind exception: `Exception` (BLE001) </details> <details> <summary>shared/tests/utils/test_crypto.py</summary> 187-187: Possible hardcoded password assigned to: "token" (S105) </details> </details> </details> <details> <summary>⏰ 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)</summary> * GitHub Check: Test Executor </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| import logging | ||
| logger = logging.getLogger(__name__) | ||
| logger.info(f"[DEBUG] update_bot called with bot_id={bot_id}") | ||
| logger.info(f"[DEBUG] bot_update raw: {bot_update}") | ||
| logger.info(f"[DEBUG] bot_update.agent_config: {bot_update.agent_config}") | ||
| logger.info(f"[DEBUG] bot_update.model_dump(exclude_unset=True): {bot_update.model_dump(exclude_unset=True)}") |
There was a problem hiding this comment.
Remove development debug logging before merging.
This appears to be temporary debug code:
- Import should be at module level, not inside the function.
- Using
logger.infowith "[DEBUG]" prefix is semantically incorrect—uselogger.debugfor debug-level logs. - Logging full
bot_updatepayloads (includingagent_config) may expose sensitive configuration data in production logs.
If logging is needed for observability, consider a minimal, structured approach:
+import logging
+
+logger = logging.getLogger(__name__)
+
router = APIRouter()
# ... in update_bot function:
- import logging
- logger = logging.getLogger(__name__)
- logger.info(f"[DEBUG] update_bot called with bot_id={bot_id}")
- logger.info(f"[DEBUG] bot_update raw: {bot_update}")
- logger.info(f"[DEBUG] bot_update.agent_config: {bot_update.agent_config}")
- logger.info(f"[DEBUG] bot_update.model_dump(exclude_unset=True): {bot_update.model_dump(exclude_unset=True)}")
-
+ logger.debug("update_bot called with bot_id=%s", bot_id)🤖 Prompt for AI Agents
In backend/app/api/endpoints/adapter/bots.py around lines 67 to 72, remove the
ad-hoc development debug logging: move the logging import to the module top if
not already, remove the four info statements that print the full bot_update and
agent_config, and if observability is required replace them with a single
logger.debug call that logs only non-sensitive minimal fields (e.g., bot_id and
a trimmed status or masked/hashed indicator for agent_config) or structured
metadata; ensure you use logger.debug (not logger.info with “[DEBUG]”) and do
not emit full payloads or sensitive configuration data.
| logger.info(f"[_get_bot_summary] bot.name={bot.name}, modelRef.name={bot_crd.spec.modelRef.name}, modelRef.namespace={bot_crd.spec.modelRef.namespace}") | ||
|
|
||
| # Get shell to extract agent_name | ||
| shell = db.query(Kind).filter( | ||
| Kind.user_id == user_id, | ||
| Kind.kind == "Shell", | ||
| Kind.name == bot_crd.spec.shellRef.name, | ||
| Kind.namespace == bot_crd.spec.shellRef.namespace, | ||
| Kind.is_active == True | ||
| ).first() | ||
|
|
||
| agent_name = "" | ||
| if shell and shell.json: | ||
| shell_crd = Shell.model_validate(shell.json) | ||
| agent_name = shell_crd.spec.runtime | ||
|
|
||
| # Get model reference | ||
| model_ref_name = bot_crd.spec.modelRef.name | ||
| model_ref_namespace = bot_crd.spec.modelRef.namespace | ||
|
|
||
| # Try to find model in user's private models first | ||
| model = db.query(Kind).filter( | ||
| Kind.user_id == user_id, | ||
| Kind.kind == "Model", | ||
| Kind.name == model_ref_name, | ||
| Kind.namespace == model_ref_namespace, | ||
| Kind.is_active == True | ||
| ).first() | ||
|
|
||
| logger.info(f"[_get_bot_summary] Private model found: {model is not None}") | ||
|
|
||
| agent_config = {} | ||
|
|
||
| if model: | ||
| # Private model - check if it's a custom config or predefined model | ||
| model_crd = Model.model_validate(model.json) | ||
| is_custom_config = model_crd.spec.isCustomConfig | ||
|
|
||
| logger.info(f"[_get_bot_summary] Private model isCustomConfig: {is_custom_config}") | ||
|
|
||
| if is_custom_config: | ||
| # Custom config - don't include bind_model | ||
| agent_config = {} | ||
| logger.info(f"[_get_bot_summary] Custom config (isCustomConfig=True), returning empty agent_config") | ||
| else: | ||
| # Not custom config = predefined model, return bind_model format | ||
| agent_config = {"bind_model": model_ref_name} | ||
| logger.info(f"[_get_bot_summary] Predefined model (isCustomConfig=False), returning bind_model: {agent_config}") | ||
| else: | ||
| # Try to find in public_models table | ||
| public_model = db.query(PublicModel).filter( | ||
| PublicModel.name == model_ref_name, | ||
| PublicModel.namespace == model_ref_namespace, | ||
| PublicModel.is_active == True | ||
| ).first() | ||
|
|
||
| logger.info(f"[_get_bot_summary] Public model found: {public_model is not None}") | ||
|
|
||
| if public_model: | ||
| # Public model - return bind_model format | ||
| agent_config = {"bind_model": public_model.name} | ||
| logger.info(f"[_get_bot_summary] Using bind_model from public model: {agent_config}") | ||
| else: | ||
| logger.info(f"[_get_bot_summary] No model found for modelRef.name={model_ref_name}, modelRef.namespace={model_ref_namespace}") | ||
|
|
||
| result = { | ||
| "agent_config": agent_config, | ||
| "agent_name": agent_name | ||
| } | ||
| logger.info(f"[_get_bot_summary] Returning: {result}") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Excessive INFO-level logging in _get_bot_summary.
The method contains multiple logger.info() calls that will generate significant log volume in production for every bot summary request. Consider:
- Using
logger.debug()for detailed diagnostic logs - Removing redundant log statements after development is complete
- logger.info(f"[_get_bot_summary] bot.name={bot.name}, modelRef.name={bot_crd.spec.modelRef.name}, modelRef.namespace={bot_crd.spec.modelRef.namespace}")
+ logger.debug(f"[_get_bot_summary] bot.name={bot.name}, modelRef.name={bot_crd.spec.modelRef.name}")
# ... other code ...
- logger.info(f"[_get_bot_summary] Private model found: {model is not None}")
+ logger.debug(f"[_get_bot_summary] Private model found: {model is not None}")📝 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.
| logger.info(f"[_get_bot_summary] bot.name={bot.name}, modelRef.name={bot_crd.spec.modelRef.name}, modelRef.namespace={bot_crd.spec.modelRef.namespace}") | |
| # Get shell to extract agent_name | |
| shell = db.query(Kind).filter( | |
| Kind.user_id == user_id, | |
| Kind.kind == "Shell", | |
| Kind.name == bot_crd.spec.shellRef.name, | |
| Kind.namespace == bot_crd.spec.shellRef.namespace, | |
| Kind.is_active == True | |
| ).first() | |
| agent_name = "" | |
| if shell and shell.json: | |
| shell_crd = Shell.model_validate(shell.json) | |
| agent_name = shell_crd.spec.runtime | |
| # Get model reference | |
| model_ref_name = bot_crd.spec.modelRef.name | |
| model_ref_namespace = bot_crd.spec.modelRef.namespace | |
| # Try to find model in user's private models first | |
| model = db.query(Kind).filter( | |
| Kind.user_id == user_id, | |
| Kind.kind == "Model", | |
| Kind.name == model_ref_name, | |
| Kind.namespace == model_ref_namespace, | |
| Kind.is_active == True | |
| ).first() | |
| logger.info(f"[_get_bot_summary] Private model found: {model is not None}") | |
| agent_config = {} | |
| if model: | |
| # Private model - check if it's a custom config or predefined model | |
| model_crd = Model.model_validate(model.json) | |
| is_custom_config = model_crd.spec.isCustomConfig | |
| logger.info(f"[_get_bot_summary] Private model isCustomConfig: {is_custom_config}") | |
| if is_custom_config: | |
| # Custom config - don't include bind_model | |
| agent_config = {} | |
| logger.info(f"[_get_bot_summary] Custom config (isCustomConfig=True), returning empty agent_config") | |
| else: | |
| # Not custom config = predefined model, return bind_model format | |
| agent_config = {"bind_model": model_ref_name} | |
| logger.info(f"[_get_bot_summary] Predefined model (isCustomConfig=False), returning bind_model: {agent_config}") | |
| else: | |
| # Try to find in public_models table | |
| public_model = db.query(PublicModel).filter( | |
| PublicModel.name == model_ref_name, | |
| PublicModel.namespace == model_ref_namespace, | |
| PublicModel.is_active == True | |
| ).first() | |
| logger.info(f"[_get_bot_summary] Public model found: {public_model is not None}") | |
| if public_model: | |
| # Public model - return bind_model format | |
| agent_config = {"bind_model": public_model.name} | |
| logger.info(f"[_get_bot_summary] Using bind_model from public model: {agent_config}") | |
| else: | |
| logger.info(f"[_get_bot_summary] No model found for modelRef.name={model_ref_name}, modelRef.namespace={model_ref_namespace}") | |
| result = { | |
| "agent_config": agent_config, | |
| "agent_name": agent_name | |
| } | |
| logger.info(f"[_get_bot_summary] Returning: {result}") | |
| logger.debug(f"[_get_bot_summary] bot.name={bot.name}, modelRef.name={bot_crd.spec.modelRef.name}") | |
| # Get shell to extract agent_name | |
| shell = db.query(Kind).filter( | |
| Kind.user_id == user_id, | |
| Kind.kind == "Shell", | |
| Kind.name == bot_crd.spec.shellRef.name, | |
| Kind.namespace == bot_crd.spec.shellRef.namespace, | |
| Kind.is_active == True | |
| ).first() | |
| agent_name = "" | |
| if shell and shell.json: | |
| shell_crd = Shell.model_validate(shell.json) | |
| agent_name = shell_crd.spec.runtime | |
| # Get model reference | |
| model_ref_name = bot_crd.spec.modelRef.name | |
| model_ref_namespace = bot_crd.spec.modelRef.namespace | |
| # Try to find model in user's private models first | |
| model = db.query(Kind).filter( | |
| Kind.user_id == user_id, | |
| Kind.kind == "Model", | |
| Kind.name == model_ref_name, | |
| Kind.namespace == model_ref_namespace, | |
| Kind.is_active == True | |
| ).first() | |
| logger.debug(f"[_get_bot_summary] Private model found: {model is not None}") | |
| agent_config = {} | |
| if model: | |
| # Private model - check if it's a custom config or predefined model | |
| model_crd = Model.model_validate(model.json) | |
| is_custom_config = model_crd.spec.isCustomConfig | |
| logger.info(f"[_get_bot_summary] Private model isCustomConfig: {is_custom_config}") | |
| if is_custom_config: | |
| # Custom config - don't include bind_model | |
| agent_config = {} | |
| logger.info(f"[_get_bot_summary] Custom config (isCustomConfig=True), returning empty agent_config") | |
| else: | |
| # Not custom config = predefined model, return bind_model format | |
| agent_config = {"bind_model": model_ref_name} | |
| logger.info(f"[_get_bot_summary] Predefined model (isCustomConfig=False), returning bind_model: {agent_config}") | |
| else: | |
| # Try to find in public_models table | |
| public_model = db.query(PublicModel).filter( | |
| PublicModel.name == model_ref_name, | |
| PublicModel.namespace == model_ref_namespace, | |
| PublicModel.is_active == True | |
| ).first() | |
| logger.info(f"[_get_bot_summary] Public model found: {public_model is not None}") | |
| if public_model: | |
| # Public model - return bind_model format | |
| agent_config = {"bind_model": public_model.name} | |
| logger.info(f"[_get_bot_summary] Using bind_model from public model: {agent_config}") | |
| else: | |
| logger.info(f"[_get_bot_summary] No model found for modelRef.name={model_ref_name}, modelRef.namespace={model_ref_namespace}") | |
| result = { | |
| "agent_config": agent_config, | |
| "agent_name": agent_name | |
| } | |
| logger.info(f"[_get_bot_summary] Returning: {result}") |
🧰 Tools
🪛 Ruff (0.14.6)
817-817: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
835-835: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
852-852: f-string without any placeholders
Remove extraneous f prefix
(F541)
862-862: Avoid equality comparisons to True; use PublicModel.is_active: for truth checks
Replace with PublicModel.is_active
(E712)
🤖 Prompt for AI Agents
In backend/app/services/adapters/team_kinds.py around lines 809 to 878, there
are many logger.info() calls generating verbose output for every bot summary
request; change diagnostic/tracing logs (those that report "Private model
found", "Private model isCustomConfig", "Custom config...", "Predefined
model...", "Public model found", "Using bind_model...", and the middle "No model
found..." messages) from logger.info() to logger.debug(), and remove or keep
only one concise logger.info() (e.g., a single final summary or error) to avoid
redundant info-level noise in production.
| try: | ||
| if 'spec' in result and 'modelConfig' in result['spec']: | ||
| model_config = result['spec']['modelConfig'] | ||
| if 'env' in model_config and 'api_key' in model_config['env']: | ||
| api_key = model_config['env']['api_key'] | ||
| if api_key: | ||
| result['spec']['modelConfig']['env']['api_key'] = decrypt_api_key(api_key) | ||
| except Exception as e: | ||
| logger.warning(f"Failed to decrypt API key: {str(e)}") | ||
|
|
There was a problem hiding this comment.
Decryption failure may expose encrypted data to client.
When decryption fails, the method logs a warning but returns the result with the still-encrypted (or corrupted) API key intact. This could leak the encrypted value to the frontend.
Consider masking the API key on decryption failure:
try:
if 'spec' in result and 'modelConfig' in result['spec']:
model_config = result['spec']['modelConfig']
if 'env' in model_config and 'api_key' in model_config['env']:
api_key = model_config['env']['api_key']
if api_key:
result['spec']['modelConfig']['env']['api_key'] = decrypt_api_key(api_key)
except Exception as e:
- logger.warning(f"Failed to decrypt API key: {str(e)}")
+ logger.warning("Failed to decrypt API key: %s", e)
+ # Mask the key to avoid exposing encrypted data
+ if 'spec' in result and 'modelConfig' in result['spec']:
+ model_config = result['spec']['modelConfig']
+ if 'env' in model_config and 'api_key' in model_config['env']:
+ result['spec']['modelConfig']['env']['api_key'] = "***"🧰 Tools
🪛 Ruff (0.14.6)
78-78: Do not catch blind exception: Exception
(BLE001)
79-79: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In backend/app/services/kind_impl.py around lines 71 to 80, the code logs
decryption failures but leaves the encrypted API key in the returned result
which can leak to clients; modify the exception handler so that on any
decryption error you replace/mask the API key in
result['spec']['modelConfig']['env'] (e.g., set it to None or a constant like
"****masked****") instead of leaving the original encrypted value, keep the
warning log (including error details) and ensure the structure still exists
before assigning the masked value to avoid KeyError.
| console.log('[DEBUG] fetchModels useEffect triggered', { | ||
| agentName, | ||
| baseBot_agent_name: baseBot?.agent_name, | ||
| baseBot_agent_config: baseBot?.agent_config, | ||
| }); |
There was a problem hiding this comment.
Remove debug console.log statements before merging.
Multiple [DEBUG] log statements are scattered throughout the component (lines 293-297, 309, 320-325, 330, 340-345, 349, 435-441, 513, 552, 557, 562, 567). These should be removed or replaced with a conditional debug utility for production.
Example locations to clean up:
- Lines 293-297:
fetchModels useEffectdebug - Lines 435-441:
handleSavedebug - Lines 552, 557, 562: Save request/response debug
🤖 Prompt for AI Agents
In frontend/src/features/settings/components/BotEdit.tsx around lines 293-297
(and also clean up the other reported locations: 309, 320-325, 330, 340-345,
349, 435-441, 513, 552, 557, 562, 567), remove the leftover
console.log('[DEBUG]...') statements; either delete them or replace them with a
conditional debug utility/logger that only emits when a debug flag or NODE_ENV
permits it (e.g., a simple isDebug check or use existing app logger), and ensure
no plain console.debug/console.log calls remain in these lines before merging.
| const OPENAI_MODEL_OPTIONS = [ | ||
| { value: 'gpt-4o', label: 'gpt-4o (Recommended)' }, | ||
| { value: 'gpt-4-turbo', label: 'gpt-4-turbo' }, | ||
| { value: 'gpt-4', label: 'gpt-4' }, | ||
| { value: 'gpt-3.5-turbo', label: 'gpt-3.5-turbo' }, | ||
| { value: 'custom', label: 'Custom...' }, | ||
| ]; | ||
|
|
||
| const ANTHROPIC_MODEL_OPTIONS = [ | ||
| { value: 'claude-sonnet-4', label: 'claude-sonnet-4 (Recommended)' }, | ||
| { value: 'claude-opus-4', label: 'claude-opus-4' }, | ||
| { value: 'claude-haiku-4.5', label: 'claude-haiku-4.5' }, | ||
| { value: 'custom', label: 'Custom...' }, | ||
| ]; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Anthropic Claude API current model IDs list 2025
💡 Result:
Here are the Anthropic Claude API model IDs current in 2025 (major/recent snapshots and aliases):
- claude-opus-4-5-20251101 (alias: claude-opus-4-5). [news release]. [1]
- claude-sonnet-4-5-20250929 (alias: claude-sonnet-4-5). [2]
- claude-haiku-4-5-20251001 (alias: claude-haiku-4-5). [2]
- claude-opus-4-1-20250805 (alias: claude-opus-4-1). [2]
- claude-opus-4-20250514 (alias: claude-opus-4). [2]
- claude-sonnet-4-20250514 (alias: claude-sonnet-4). [2][3]
- claude-3-7-sonnet-20250219 (alias: claude-3-7-sonnet-latest). [3]
- claude-3-5-sonnet-20241022 (alias: claude-3-5-sonnet-latest). [3]
- claude-3-5-haiku-20241022 (alias: claude-3-5-haiku-latest). [3]
Sources: Anthropic model list / docs and recent Anthropic announcements. [1] Times/BI report on Opus 4.5 release; [2] Anthropic models overview; [3] Anthropic API models list.
Update the Anthropic model option claude-haiku-4.5 to use the correct naming convention.
The Anthropic API currently uses model IDs with hyphens (e.g., claude-opus-4, claude-sonnet-4, claude-haiku-4-5), not periods. The claude-haiku-4.5 identifier should be claude-haiku-4-5. Additionally, consider whether to use versioned aliases (claude-opus-4-5, claude-sonnet-4-5) or dated full model IDs depending on your stability/recency requirements.
Current model options (lines 39–43):
const ANTHROPIC_MODEL_OPTIONS = [
{ value: 'claude-sonnet-4', label: 'claude-sonnet-4 (Recommended)' },
{ value: 'claude-opus-4', label: 'claude-opus-4' },
{ value: 'claude-haiku-4.5', label: 'claude-haiku-4.5' }, // ← Fix: should be 'claude-haiku-4-5'
{ value: 'custom', label: 'Custom...' },
];
🤖 Prompt for AI Agents
In frontend/src/features/settings/components/ModelEdit.tsx around lines 30 to
43, the Anthropic model option uses the incorrect identifier "claude-haiku-4.5";
update the value and label to use the hyphenated model ID "claude-haiku-4-5"
(i.e., change both the value and visible label), and while editing consider
whether you want to prefer versioned aliases or dated full model IDs for
consistency across the ANTHROPIC_MODEL_OPTIONS array.
| <Button | ||
| variant="outline" | ||
| onClick={handleTestConnection} | ||
| disabled={testing || !modelId || !apiKey} | ||
| > | ||
| {testing && <Loader2 className="mr-2 h-4 w-4 animate-spin" />} | ||
| <BeakerIcon className="w-4 h-4 mr-1" /> | ||
| {t('models.test_connection')} | ||
| </Button> |
There was a problem hiding this comment.
Test Connection button enabled incorrectly when "Custom" model is selected but empty.
The disabled condition checks !modelId, but when modelId === 'custom', it should check customModelId instead. Currently, the button is enabled when "Custom" is selected with an empty custom model ID.
<Button
variant="outline"
onClick={handleTestConnection}
- disabled={testing || !modelId || !apiKey}
+ disabled={testing || !apiKey || (modelId === 'custom' ? !customModelId : !modelId)}
>📝 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.
| <Button | |
| variant="outline" | |
| onClick={handleTestConnection} | |
| disabled={testing || !modelId || !apiKey} | |
| > | |
| {testing && <Loader2 className="mr-2 h-4 w-4 animate-spin" />} | |
| <BeakerIcon className="w-4 h-4 mr-1" /> | |
| {t('models.test_connection')} | |
| </Button> | |
| <Button | |
| variant="outline" | |
| onClick={handleTestConnection} | |
| disabled={testing || !apiKey || (modelId === 'custom' ? !customModelId : !modelId)} | |
| > | |
| {testing && <Loader2 className="mr-2 h-4 w-4 animate-spin" />} | |
| <BeakerIcon className="w-4 h-4 mr-1" /> | |
| {t('models.test_connection')} | |
| </Button> |
🤖 Prompt for AI Agents
In frontend/src/features/settings/components/ModelEdit.tsx around lines 312 to
320, the Test Connection button's disabled check only uses !modelId so when
modelId === 'custom' and customModelId is empty the button is incorrectly
enabled; update the disabled prop to also require a non-empty customModelId when
modelId === 'custom' (e.g. disabled={testing || !modelId || (modelId ===
'custom' && !customModelId) || !apiKey}) so the button stays disabled if the
custom model ID is empty.
| {t('team.dify_no_members_hint') || | ||
| 'Dify bots handle execution independently and do not support team members.'} | ||
| </div> |
There was a problem hiding this comment.
Avoid hardcoded English fallback strings.
The fallback string after || bypasses i18n and will always display English even for non-English locales if the translation key is missing. Either ensure the translation key exists for all supported locales, or remove the fallback to surface missing translations during development.
- {t('team.dify_no_members_hint') ||
- 'Dify bots handle execution independently and do not support team members.'}
+ {t('team.dify_no_members_hint')}📝 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.
| {t('team.dify_no_members_hint') || | |
| 'Dify bots handle execution independently and do not support team members.'} | |
| </div> | |
| {t('team.dify_no_members_hint')} | |
| </div> |
🤖 Prompt for AI Agents
In frontend/src/features/settings/components/team-modes/BotTransfer.tsx around
lines 186 to 188, the JSX uses a hardcoded English fallback after the i18n call
(t('team.dify_no_members_hint') || '...') which forces English when a key is
missing; remove the hardcoded fallback so the component only uses
t('team.dify_no_members_hint') (or alternatively add the missing translation to
all locale files if you prefer a fallback) so missing translations surface
during development and locales remain consistent.
| console.log('[DEBUG] fetchBotsList response:', JSON.stringify(botsData, null, 2)); | ||
| const items = Array.isArray(botsData.items) ? botsData.items : []; | ||
| // Log each bot's agent_config for debugging | ||
| items.forEach((bot, index) => { | ||
| console.log(`[DEBUG] Bot ${index} (${bot.name}):`, { | ||
| id: bot.id, | ||
| agent_name: bot.agent_name, | ||
| agent_config: bot.agent_config, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Remove debug logging before merging to production.
The console.log statements with JSON.stringify will pollute browser console output in production. Additionally, logging the full API response and per-bot details (including agent_config) may inadvertently expose sensitive configuration data.
export async function fetchBotsList(): Promise<Bot[]> {
const params: PaginationParams = {};
const botsData = await botApis.getBots(params);
- console.log('[DEBUG] fetchBotsList response:', JSON.stringify(botsData, null, 2));
const items = Array.isArray(botsData.items) ? botsData.items : [];
- // Log each bot's agent_config for debugging
- items.forEach((bot, index) => {
- console.log(`[DEBUG] Bot ${index} (${bot.name}):`, {
- id: bot.id,
- agent_name: bot.agent_name,
- agent_config: bot.agent_config,
- });
- });
return items;
}📝 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.
| console.log('[DEBUG] fetchBotsList response:', JSON.stringify(botsData, null, 2)); | |
| const items = Array.isArray(botsData.items) ? botsData.items : []; | |
| // Log each bot's agent_config for debugging | |
| items.forEach((bot, index) => { | |
| console.log(`[DEBUG] Bot ${index} (${bot.name}):`, { | |
| id: bot.id, | |
| agent_name: bot.agent_name, | |
| agent_config: bot.agent_config, | |
| }); | |
| }); | |
| const items = Array.isArray(botsData.items) ? botsData.items : []; |
🤖 Prompt for AI Agents
In frontend/src/features/settings/services/bots.ts around lines 15 to 24, remove
the debug console.log calls that print the entire botsData and per-bot
agent_config (these leak sensitive config and clutter production consoles);
either delete those console.log lines or wrap them in an environment check
(e.g., only run in development) and if you must log per-bot info redact or omit
agent_config and any sensitive fields before logging.
| aes_key = os.environ.get('GIT_TOKEN_AES_KEY', '12345678901234567890123456789012') | ||
| aes_iv = os.environ.get('GIT_TOKEN_AES_IV', '1234567890123456') |
There was a problem hiding this comment.
Hardcoded default encryption keys pose a security risk.
Using hardcoded fallback values for AES key and IV means data encrypted with defaults is trivially decryptable by anyone with access to the source code. If environment variables are not set, this effectively provides no security.
Consider failing loudly when keys are not configured, or at minimum log a security warning:
- aes_key = os.environ.get('GIT_TOKEN_AES_KEY', '12345678901234567890123456789012')
- aes_iv = os.environ.get('GIT_TOKEN_AES_IV', '1234567890123456')
+ aes_key = os.environ.get('GIT_TOKEN_AES_KEY')
+ aes_iv = os.environ.get('GIT_TOKEN_AES_IV')
+ if not aes_key or not aes_iv:
+ logger.warning(
+ "Encryption keys not configured via GIT_TOKEN_AES_KEY/GIT_TOKEN_AES_IV. "
+ "Using insecure defaults - DO NOT USE IN PRODUCTION."
+ )
+ aes_key = '12345678901234567890123456789012'
+ aes_iv = '1234567890123456'Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In shared/utils/crypto.py around lines 30-31, the AES key and IV are using
insecure hardcoded defaults from os.environ.get(..., '<default>'); remove the
fallback defaults and instead require the environment variables to be present —
validate that GIT_TOKEN_AES_KEY and GIT_TOKEN_AES_IV exist and meet expected
lengths (32 bytes for AES-256 key, 16 bytes for IV), and if they are missing or
invalid raise an explicit exception (or at startup log a security error and
exit) so the application fails loudly rather than silently using insecure
defaults.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/api/endpoints/kind/kinds.py (1)
140-159: Perfect. I now have comprehensive evidence to verify and rewrite the review comment. The findings confirm the concerns raised but with additional context about the specific pattern deviation.Missing status code and inconsistent with REST conventions for DELETE operations
This endpoint returns HTTP 200 with a JSON body, but should follow the established pattern in the codebase: use
status_code=status.HTTP_204_NO_CONTENT(as inbackend/app/api/endpoints/admin.py:298for the identical DELETE operation on kind resources). Returning JSON on a DELETE also violates REST semantics—a 204 response has no body.Additionally, add return type hint (
-> None) to align with coding guidelines requiring type hints on all Python function signatures. All endpoints in this file currently lack return type annotations.
♻️ Duplicate comments (5)
backend/app/services/kind_impl.py (1)
71-80: Decryption failure may expose encrypted data to client.When decryption fails, the method logs a warning but returns the result with the still-encrypted (or corrupted) API key intact. This could leak the encrypted value to the frontend.
Consider masking the API key on decryption failure:
try: if 'spec' in result and 'modelConfig' in result['spec']: model_config = result['spec']['modelConfig'] if 'env' in model_config and 'api_key' in model_config['env']: api_key = model_config['env']['api_key'] if api_key: result['spec']['modelConfig']['env']['api_key'] = decrypt_api_key(api_key) except Exception as e: - logger.warning(f"Failed to decrypt API key: {str(e)}") + logger.warning("Failed to decrypt API key: %s", e) + # Mask the key to avoid exposing encrypted data + if 'spec' in result and 'modelConfig' in result['spec']: + model_config = result['spec']['modelConfig'] + if 'env' in model_config and 'api_key' in model_config['env']: + result['spec']['modelConfig']['env']['api_key'] = "***"frontend/src/features/settings/components/team-modes/BotTransfer.tsx (1)
196-200: Hardcoded English fallback string.This issue was previously flagged and should be addressed by removing the fallback to ensure proper i18n.
backend/app/services/adapters/team_kinds.py (1)
797-887: Excessive INFO-level logging should be reduced.The method contains many
logger.info()calls that will generate significant log noise in production. This was previously flagged and should be addressed by usinglogger.debug()for diagnostic logs.frontend/src/features/settings/components/BotEdit.tsx (2)
313-317: Debug logging still present.Multiple
[DEBUG]console.log statements remain in the code. These were flagged in a previous review.
932-939: Composite key splitting vulnerability remains.The value format using
:as delimiter will produce incorrect results if a model name contains a colon character. This was flagged in a previous review.
🧹 Nitpick comments (25)
frontend/src/features/settings/components/TeamList.tsx (1)
11-12: Bot management dialog wiring looks solid; consider bots-state refresh to avoid stalenessThe new
Manage Botsbutton,botListVisiblestate, andDialogwith<BotList />are wired correctly and match existing shadcn/ui patterns; imports and UI composition look good overall.One thing to double‑check:
TeamListowns a localbotsstate that is passed intoTeamEdit, while<BotList />is rendered without any props here. IfBotListperforms create/update/delete operations with its own data source,TeamList’sbotsarray may become stale after changes made in the Bot dialog (e.g., a newly created bot not appearing when openingTeamEditwithout a full page reload).If
BotListis not already backed by a shared store (SWR/React Query/global context), consider either:
- Exposing a callback prop such as
onBotsChangedfromBotListthat re-runs thefetchBotsList/setBotsSortedflow inTeamList, or- Refactoring both
TeamListandBotListto consume a shared bots source so they stay in sync automatically.This would keep the team‑editing bot selector consistent with the standalone bot management dialog.
Also applies to: 24-25, 58-59, 352-363, 401-412
frontend/src/components/ui/alert-dialog.tsx (2)
13-26: Consider reducing overlay opacity to align with low-contrast design guidelines.The
bg-black/80creates a relatively high-contrast overlay. Per the coding guidelines emphasizing low saturation and low contrast for reduced eye strain, consider using a lower opacity value likebg-black/60orbg-black/50.Based on coding guidelines: "Frontend design: Low saturation + low contrast for reduced eye strain"
<AlertDialogPrimitive.Overlay className={cn( - 'fixed inset-0 z-50 bg-black/80 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0', + 'fixed inset-0 z-50 bg-black/60 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0', className )}
28-44: Consider using a more subtle shadow to align with minimal design guidelines.The
shadow-lgcreates a prominent shadow effect. Per the coding guidelines emphasizing minimal shadows and subtle component differentiation, consider usingshadow-smor removing the shadow entirely since the border provides differentiation.Based on coding guidelines: "Frontend design: Minimal shadows, subtle component differentiation (<10% background variance)"
className={cn( - 'fixed left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border border-border bg-base p-6 shadow-lg duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] sm:rounded-lg', + 'fixed left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border border-border bg-base p-6 shadow-sm duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] sm:rounded-lg', className )}backend/app/api/endpoints/kind/kinds.py (1)
140-159: Aligndelete_resourcewith project typing guidelinesPer the Python guidelines (type hints required), consider adding annotations here, e.g.:
async def delete_resource( namespace: str = Path(..., description="Resource namespace"), kinds: str = Path(..., description="Resource type. Valid options: ghosts, models, shells, bots, teams, workspaces, tasks"), name: str = Path(..., description="Resource name"), current_user: User = Depends(get_current_user), ) -> dict[str, str]: ...(Adjust the return type alias as needed if you introduce a dedicated response model.)
frontend/src/features/settings/components/TeamEditDrawer.tsx (1)
18-18: Consider consolidating AgentType definition.
AgentTypeis defined identically in bothBotEdit.tsx(line 34) andteam-modes/index.ts(line 17). Consider exporting from a single source to avoid duplication and potential drift.// In a shared types file like frontend/src/features/settings/types.ts export type AgentType = 'ClaudeCode' | 'Agno' | 'Dify';Then import from that shared location in both files.
backend/app/services/kind_impl.py (1)
58-61: Fix f-string and logging patterns.Static analysis correctly identifies issues on these lines.
- logger.info(f"Encrypted API key for Model resource") + logger.info("Encrypted API key for Model resource") except Exception as e: - logger.error(f"Failed to encrypt API key: {str(e)}") + logger.exception("Failed to encrypt API key") raiseUsing
logger.exceptionautomatically includes the stack trace and exception details.frontend/src/features/settings/components/DifyBotConfig.tsx (1)
212-218: Consider simplifying the readOnly guard pattern.The early-return guard (
if (readOnly) return;) is redundant whendisabled={readOnly}is also set since the browser won't fire change events on disabled inputs.Either approach works, but you could simplify by relying solely on the
disabledattribute:onChange={e => { - if (readOnly) return; setDifyApiKey(e.target.value); }} disabled={readOnly}The current approach is more defensive and still valid if you prefer the explicit guard.
frontend/src/components/ui/transfer.tsx (1)
66-72: Good refactoring to preserve targetKeys order, minor redundancy.The refactored
rightDataSourcecorrectly preserves the order oftargetKeysfor sortable functionality. However, thetargetSet.has(key)check on line 70 is redundant sincetargetKeysis iterated directly.const rightDataSource = React.useMemo(() => { - const targetSet = new Set(targetKeys); const itemMap = new Map(dataSource.map(item => [item.key, item])); return targetKeys - .filter(key => targetSet.has(key) && itemMap.has(key)) + .filter(key => itemMap.has(key)) .map(key => itemMap.get(key)!); }, [dataSource, targetKeys]);frontend/src/features/settings/components/team-modes/BotTransfer.tsx (1)
102-119: Potential edge case:leaderBotIdtype mismatch in filter condition.On line 106,
leaderBotId !== nulldoesn't account for theundefinedcase since the prop type isnumber | null. WhenleaderBotIdisundefined, the condition is still true and the filter runs, butbots.filter(b => b.id !== undefined)will filter nothing, which is correct. However, the condition could be more explicit.- if (excludeLeader && leaderBotId !== null) { + if (excludeLeader && leaderBotId != null) {frontend/src/features/settings/components/team-modes/index.ts (2)
13-18: DuplicateAgentTypedefinition across files.
AgentTypeis also defined inBotEdit.tsx(line 34). Consider consolidating this type definition into a shared types file to avoid inconsistencies and ensure single source of truth.Based on the relevant code snippets, both files define identical
AgentType = 'ClaudeCode' | 'Agno' | 'Dify'. Extract this to a shared location likefrontend/src/types/api.tsor create a dedicatedfrontend/src/types/agents.ts.
40-50: Type assertion could mask runtime mismatches.The filter uses
bot.agent_name as AgentType, but ifbot.agent_namecontains a value not in theAgentTypeunion (e.g., a new agent type added later), the filter will incorrectly exclude valid bots without any compile-time warning.- return bots.filter(bot => allowedAgents.includes(bot.agent_name as AgentType)); + return bots.filter(bot => { + const agentName = bot.agent_name as string; + return allowedAgents.some(allowed => allowed === agentName); + });Alternatively, add a type guard or validation function to handle unknown agent types gracefully.
frontend/src/features/settings/components/team-modes/SoloModeEditor.tsx (3)
25-39: Unused prop:editingTeam.The
editingTeamprop is declared in the interface but never used in the component body. Either remove it from the props or document why it's needed for future use.export interface SoloModeEditorProps { bots: Bot[]; setBots: React.Dispatch<React.SetStateAction<Bot[]>>; selectedBotId: number | null; setSelectedBotId: React.Dispatch<React.SetStateAction<number | null>>; - editingTeam: Team | null; toast: ReturnType<typeof import('@/hooks/use-toast').useToast>['toast']; // ... }
139-145: Empty callback:handleBotCreateClosewith misleading comment.The callback body is intentionally empty with a comment explaining the rationale, but an empty function that does nothing can be confusing. Consider either:
- Removing the callback and passing
undefined/() => {}inline- Adding actual logic if needed
- // Handle bot creation close - just close the creation mode (used for cancel) - const handleBotCreateClose = useCallback(() => { - // This is called when BotEdit's onClose is triggered - // We don't set isCreatingBot to false here because the useEffect will handle it - // when a new bot is detected. If no new bot is created (e.g., validation failed), - // we keep the creation mode open. - }, []); + // Empty callback - creation mode is handled by the useEffect that detects new bots + const handleBotCreateClose = useCallback(() => undefined, []);
276-276: Consider making minimum height configurable.The hardcoded
min-h-[500px]may not be appropriate for all screen sizes or contexts. Consider making this a prop or using a more responsive approach.frontend/src/features/settings/components/TeamEdit.tsx (3)
94-104: DuplicateMODE_AGENT_FILTERdefinition.This constant is identical to the one in
index.ts. Import it from there instead of redefining it to maintain DRY principle and ensure consistency.+import { TeamMode, getFilteredBotsForMode, AgentType, MODE_AGENT_FILTER } from './team-modes'; - - // Get allowed agents for current mode - const allowedAgentsForMode = useMemo((): AgentType[] | undefined => { - const MODE_AGENT_FILTER: Record<TeamMode, AgentType[] | null> = { - solo: null, // null means all agents are allowed - pipeline: ['ClaudeCode', 'Agno'], - route: ['Agno'], - coordinate: ['Agno'], - collaborate: ['Agno'], - }; - const allowed = MODE_AGENT_FILTER[mode]; - return allowed === null ? undefined : allowed; - }, [mode]);Note: You'll need to export
MODE_AGENT_FILTERfromindex.ts.
274-279: Dead code:selectedAgentNamealways returnsnull.This memoized value always returns
nullwith a comment indicating the restriction was removed. If the logic is no longer needed, remove the variable entirely rather than keeping dead code.- // Get currently selected agent_name (from leader or selected bot) - // Note: agent_name restriction has been removed - users can now select any mode - const selectedAgentName = useMemo(() => { - // No agent_name restriction - always return null - return null; - }, []); + // agent_name restriction has been removed - users can now select any mode + const selectedAgentName = null;Then update the references to pass
nulldirectly or remove the prop if consumers don't need it.
617-863: Consider extracting inline CSS to a separate stylesheet.The extensive use of
dangerouslySetInnerHTMLfor CSS is not a security risk here (static content), but it's a code quality concern:
- CSS is not type-checked or linted
- Large inline styles hurt readability
- CSS cannot be tree-shaken or cached separately
Consider moving these styles to:
- A CSS module (
TeamEdit.module.css)- A global stylesheet for the Transfer component overrides
- Using Tailwind's
@applydirective in a styles fileThis would improve maintainability and follow the frontend coding guidelines.
backend/app/services/adapters/team_kinds.py (3)
802-803: Avoid importing inside method.The import of
logginginside_get_bot_summaryis redundant sinceloggeris already defined at module level (line 11). Remove this local import.def _get_bot_summary(self, bot: Kind, db: Session, user_id: int) -> Dict[str, Any]: """ Get a summary of bot information including agent_config with only necessary fields. This is used for team list to determine if bots have predefined models. """ - import logging - logger = logging.getLogger(__name__) - from app.models.public_model import PublicModel
821-821: Use Pythonic truth checks instead of== True.Per Ruff E712 and PEP 8, avoid equality comparisons to
True. Use the attribute directly for truth checks.- Kind.is_active == True + Kind.is_active.is_(True) # For SQLAlchemy, use is_() for explicit NULL-safe comparisonOr if SQLAlchemy's implicit boolean handling is acceptable:
Kind.is_active # Without comparisonNote: In SQLAlchemy filter contexts,
== Trueworks butis_(True)is more explicit and avoids the linter warning.Also applies to: 839-839, 868-868
748-779: Redundant database query for first bot.The first bot's ID is obtained from
bots[0]["bot_id"], then a new query fetches the same bot that was already processed in the loop above. Consider storing the first bot's shell/agent_type during the initial loop to avoid this extra query.bots = [] agent_names = set() + first_agent_type = None for member in team_crd.spec.members: # Find bot in kinds table bot = db.query(Kind).filter( # ... ).first() if bot: bot_summary = self._get_bot_summary(bot, db, user_id) bot_info = { "bot_id": bot.id, "bot_prompt": member.prompt or "", "role": member.role or "", "bot": bot_summary } bots.append(bot_info) # Collect agent_name for is_mix_team calculation if bot_summary.get("agent_name"): agent_names.add(bot_summary["agent_name"]) + # Capture first agent type during iteration + if first_agent_type is None: + first_agent_type = self._get_agent_type_from_bot(bot, db, user_id) - # Get agent_type from the first bot's shell (duplicate query removed) - agent_type = None - if bots: - # ... redundant query logic ... + agent_type = first_agent_typefrontend/src/features/settings/components/BotEdit.tsx (1)
34-36: DuplicateAgentTypedefinition creates inconsistency.
AgentTypeis already defined infrontend/src/features/settings/utils/mcpTypeAdapter.ts(line 14) as'ClaudeCode' | 'Agno'. This file adds'Dify'but duplicates the type definition. This can lead to type mismatches and maintenance issues.Consider extending the existing type in
mcpTypeAdapter.tsand importing it here:-/** Agent types supported by the system */ -export type AgentType = 'ClaudeCode' | 'Agno' | 'Dify'; +import { AgentType } from '../utils/mcpTypeAdapter';And update
mcpTypeAdapter.ts:export type AgentType = 'ClaudeCode' | 'Agno' | 'Dify';backend/app/services/model_aggregation_service.py (3)
130-132: Consider catching specific exceptions instead of bareException.Catching
Exceptionbroadly can mask unexpected errors. Consider catching more specific exceptions likeValidationErrorfrom Pydantic.- except Exception as e: - logger.warning(f"Failed to extract model info: {e}") + except (ValueError, TypeError, AttributeError) as e: + logger.warning(f"Failed to extract model info from CRD: {e}")Alternatively, if Pydantic
ValidationErroris the expected failure mode:from pydantic import ValidationError try: model_crd = Model.model_validate(model_data) # ... except ValidationError as e: logger.warning(f"Failed to validate model CRD: {e}") return {"provider": None, "model_id": None, "display_name": None, "config": {}}
297-302: Hard-coded limit of 1000 may cause issues at scale.The
limit=1000assumes there won't be more than 1000 public models. Consider either:
- Implementing proper pagination in the aggregation service
- Using a configurable constant
- Adding a warning log if the limit is reached
+ MAX_PUBLIC_MODELS = 1000 + public_models = public_model_service.get_models( db=db, skip=0, - limit=1000, # Get all public models + limit=MAX_PUBLIC_MODELS, current_user=current_user ) + + if len(public_models) >= MAX_PUBLIC_MODELS: + logger.warning(f"Public models limit ({MAX_PUBLIC_MODELS}) reached, some models may be missing")
213-218: Consider restructuring try/except for clarity.The
returnstatement inside thetryblock can be moved to improve readability.try: model_crd = Model.model_validate(model_data) - return model_crd.spec.isCustomConfig is True + is_custom = model_crd.spec.isCustomConfig is True except Exception as e: logger.warning(f"Failed to check if model is custom: {e}") return False + + return is_custombackend/app/services/adapters/bot_kinds.py (1)
651-651: Remove unnecessary f-string prefix.This log message has no placeholders, so the
fprefix is extraneous.- logger.info(f"[DEBUG] Creating new private model for custom config") + logger.info("[DEBUG] Creating new private model for custom config")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
backend/app/api/endpoints/kind/kinds.py(2 hunks)backend/app/schemas/kind.py(3 hunks)backend/app/services/adapters/bot_kinds.py(18 hunks)backend/app/services/adapters/team_kinds.py(5 hunks)backend/app/services/kind_impl.py(3 hunks)backend/app/services/model_aggregation_service.py(1 hunks)backend/init_data/01-default-resources.yaml(1 hunks)frontend/src/components/ui/alert-dialog.tsx(1 hunks)frontend/src/components/ui/transfer.tsx(8 hunks)frontend/src/features/settings/components/BotEdit.tsx(31 hunks)frontend/src/features/settings/components/DifyBotConfig.tsx(7 hunks)frontend/src/features/settings/components/ModelList.tsx(1 hunks)frontend/src/features/settings/components/TeamEdit.tsx(17 hunks)frontend/src/features/settings/components/TeamEditDrawer.tsx(3 hunks)frontend/src/features/settings/components/TeamList.tsx(5 hunks)frontend/src/features/settings/components/team-modes/BotTransfer.tsx(1 hunks)frontend/src/features/settings/components/team-modes/PipelineModeEditor.tsx(1 hunks)frontend/src/features/settings/components/team-modes/SoloModeEditor.tsx(1 hunks)frontend/src/features/settings/components/team-modes/index.ts(1 hunks)frontend/src/i18n/locales/en/common.json(7 hunks)frontend/src/i18n/locales/zh-CN/common.json(7 hunks)frontend/tailwind.config.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/features/settings/components/team-modes/PipelineModeEditor.tsx
- frontend/src/features/settings/components/ModelList.tsx
🧰 Additional context used
📓 Path-based instructions (3)
frontend/**
📄 CodeRabbit inference engine (AGENTS.md)
Frontend: Only use
NEXT_PUBLIC_*for client-safe environment variables
Files:
frontend/tailwind.config.jsfrontend/src/features/settings/components/DifyBotConfig.tsxfrontend/src/components/ui/transfer.tsxfrontend/src/features/settings/components/TeamEditDrawer.tsxfrontend/src/features/settings/components/team-modes/SoloModeEditor.tsxfrontend/src/features/settings/components/team-modes/BotTransfer.tsxfrontend/src/features/settings/components/TeamList.tsxfrontend/src/features/settings/components/BotEdit.tsxfrontend/src/features/settings/components/team-modes/index.tsfrontend/src/features/settings/components/TeamEdit.tsxfrontend/src/components/ui/alert-dialog.tsxfrontend/src/i18n/locales/zh-CN/common.jsonfrontend/src/i18n/locales/en/common.json
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/**/*.{ts,tsx}: TypeScript strict mode must be enabled in Frontend
Use functional components with hooks in React
Use Prettier formatter in Frontend with single quotes and no semicolons
Use ESLint with Next.js configuration in Frontend
Useconstoverlet, never usevarin TypeScript
Component names must use PascalCase; file names must use kebab-case in Frontend
Use mobile-first responsive design approach in Frontend components
Frontend design: Low saturation + low contrast for reduced eye strain with generous whitespace
Frontend design: Minimal shadows, subtle component differentiation (<10% background variance)
Frontend design: Use mint blue (#14B8A6) as primary accent sparingly
Use Tailwind CSS variables for colors: --color-bg-base, --color-text-primary, --color-border, etc.
Frontend spacing: Use standard units - p-2 (8px), p-4 (16px), p-6 (24px), gap-3 (12px)
Frontend border-radius: rounded-2xl (16px) for large containers, rounded-lg (12px) for cards, rounded-md (6px) for buttons
Frontend typography: H1 text-xl font-semibold, H2 text-lg font-semibold, H3 text-base font-medium, Body text-sm
Use shadcn/ui Button component with variants: default, secondary, ghost, outline, link
Use shadcn/ui Form component with react-hook-form + zod validation
Files:
frontend/src/features/settings/components/DifyBotConfig.tsxfrontend/src/components/ui/transfer.tsxfrontend/src/features/settings/components/TeamEditDrawer.tsxfrontend/src/features/settings/components/team-modes/SoloModeEditor.tsxfrontend/src/features/settings/components/team-modes/BotTransfer.tsxfrontend/src/features/settings/components/TeamList.tsxfrontend/src/features/settings/components/BotEdit.tsxfrontend/src/features/settings/components/team-modes/index.tsfrontend/src/features/settings/components/TeamEdit.tsxfrontend/src/components/ui/alert-dialog.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code must be PEP 8 compliant with Black formatter (line length: 88)
Use isort for import organization in Python
Python functions and variables must have descriptive names
Docstrings required for all public Python functions and classes
Extract magic numbers to named constants in Python
Python functions must not exceed 50 lines
Type hints required for all Python function signatures
Run pylint and flake8 for Python code quality checks
Files:
backend/app/services/kind_impl.pybackend/app/api/endpoints/kind/kinds.pybackend/app/services/model_aggregation_service.pybackend/app/schemas/kind.pybackend/app/services/adapters/bot_kinds.pybackend/app/services/adapters/team_kinds.py
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Use Kubernetes-inspired CRD design (Ghost, Model, Shell, Bot, Team, Task)
📚 Learning: 2025-11-26T07:34:32.591Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use Tailwind CSS variables for colors: --color-bg-base, --color-text-primary, --color-border, etc.
Applied to files:
frontend/tailwind.config.js
📚 Learning: 2025-11-26T07:34:32.591Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Frontend design: Minimal shadows, subtle component differentiation (<10% background variance)
Applied to files:
frontend/tailwind.config.jsfrontend/src/components/ui/alert-dialog.tsx
📚 Learning: 2025-11-26T07:34:32.591Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Frontend design: Low saturation + low contrast for reduced eye strain with generous whitespace
Applied to files:
frontend/tailwind.config.js
📚 Learning: 2025-11-26T07:34:32.591Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use functional components with hooks in React
Applied to files:
frontend/src/components/ui/transfer.tsxfrontend/src/features/settings/components/BotEdit.tsxfrontend/src/features/settings/components/TeamEdit.tsx
📚 Learning: 2025-11-26T07:34:32.591Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Use Kubernetes-inspired CRD design (Ghost, Model, Shell, Bot, Team, Task)
Applied to files:
backend/app/services/kind_impl.pybackend/init_data/01-default-resources.yamlbackend/app/services/adapters/bot_kinds.py
📚 Learning: 2025-11-26T07:34:32.591Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use shadcn/ui Button component with variants: default, secondary, ghost, outline, link
Applied to files:
frontend/src/features/settings/components/BotEdit.tsxfrontend/src/components/ui/alert-dialog.tsx
📚 Learning: 2025-11-26T07:34:32.591Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use mobile-first responsive design approach in Frontend components
Applied to files:
frontend/src/features/settings/components/TeamEdit.tsx
🧬 Code graph analysis (10)
frontend/src/components/ui/transfer.tsx (2)
frontend/src/lib/utils.ts (1)
cn(8-10)frontend/src/components/ui/checkbox.tsx (1)
Checkbox(32-32)
frontend/src/features/settings/components/TeamEditDrawer.tsx (2)
frontend/src/features/settings/components/BotEdit.tsx (1)
AgentType(35-35)frontend/src/features/settings/components/team-modes/index.ts (1)
AgentType(18-18)
frontend/src/features/settings/components/team-modes/SoloModeEditor.tsx (4)
frontend/src/features/settings/components/team-modes/types.ts (1)
SoloModeEditorProps(27-31)frontend/src/features/settings/components/BotEdit.tsx (1)
AgentType(35-35)frontend/src/features/settings/components/team-modes/index.ts (1)
AgentType(18-18)frontend/src/utils/styles.ts (2)
PromptBadgeVariant(39-39)getPromptBadgeStyle(44-74)
frontend/src/features/settings/components/team-modes/BotTransfer.tsx (2)
frontend/src/utils/styles.ts (2)
getPromptBadgeStyle(44-74)PromptBadgeVariant(39-39)frontend/src/components/ui/transfer.tsx (1)
Transfer(36-344)
frontend/src/features/settings/components/BotEdit.tsx (4)
frontend/src/features/settings/utils/mcpTypeAdapter.ts (1)
AgentType(15-15)frontend/src/types/api.ts (1)
Bot(29-40)frontend/src/apis/models.ts (3)
UnifiedModel(73-81)ModelTypeEnum(71-71)modelApis(111-214)frontend/src/features/settings/services/bots.ts (4)
isPredefinedModel(54-60)getModelFromConfig(67-70)getModelTypeFromConfig(77-86)createPredefinedModelConfig(94-103)
frontend/src/features/settings/components/team-modes/index.ts (1)
backend/app/schemas/kind.py (1)
Bot(146-152)
backend/app/api/endpoints/kind/kinds.py (1)
backend/app/core/cache.py (1)
delete(90-101)
frontend/src/components/ui/alert-dialog.tsx (2)
frontend/src/lib/utils.ts (1)
cn(8-10)frontend/src/components/ui/button.tsx (1)
buttonVariants(55-55)
backend/app/services/model_aggregation_service.py (3)
backend/app/schemas/kind.py (2)
Model(71-77)Shell(105-111)backend/app/services/kind_impl.py (2)
_format_resource(65-81)_format_resource(339-387)backend/app/services/adapters/public_model.py (1)
get_models(179-193)
backend/app/services/adapters/bot_kinds.py (2)
backend/app/models/kind.py (1)
Kind(14-30)backend/app/schemas/kind.py (3)
Bot(146-152)Model(71-77)ModelRef(88-91)
🪛 ast-grep (0.40.0)
frontend/src/features/settings/components/TeamEdit.tsx
[warning] 618-618: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
[warning] 866-866: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
frontend/src/features/settings/components/TeamEdit.tsx
[error] 619-619: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 867-867: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 Ruff (0.14.6)
backend/app/services/kind_impl.py
58-58: f-string without any placeholders
Remove extraneous f prefix
(F541)
60-60: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
60-60: Use explicit conversion flag
Replace with conversion flag
(RUF010)
78-78: Do not catch blind exception: Exception
(BLE001)
79-79: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/services/model_aggregation_service.py
130-130: Do not catch blind exception: Exception
(BLE001)
191-191: Do not catch blind exception: Exception
(BLE001)
215-215: Consider moving this statement to an else block
(TRY300)
216-216: Do not catch blind exception: Exception
(BLE001)
backend/app/services/adapters/bot_kinds.py
138-138: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
151-151: Avoid equality comparisons to True; use PublicModel.is_active: for truth checks
Replace with PublicModel.is_active
(E712)
166-166: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
177-177: Avoid equality comparisons to True; use PublicModel.is_active: for truth checks
Replace with PublicModel.is_active
(E712)
651-651: f-string without any placeholders
Remove extraneous f prefix
(F541)
734-735: try-except-pass detected, consider logging the exception
(S110)
734-734: Do not catch blind exception: Exception
(BLE001)
910-910: Avoid equality comparisons to True; use PublicModel.is_active: for truth checks
Replace with PublicModel.is_active
(E712)
918-918: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
backend/app/services/adapters/team_kinds.py
755-755: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
765-765: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
821-821: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
839-839: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
868-868: Avoid equality comparisons to True; use PublicModel.is_active: for truth checks
Replace with PublicModel.is_active
(E712)
921-921: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
⏰ 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: Test Executor
🔇 Additional comments (34)
frontend/tailwind.config.js (3)
48-48: Good organizational improvement.The comment clarifies the distinction between custom project colors and the shadcn/ui aliases added below, improving config readability.
74-94: LGTM! Logical shadcn/ui integration.The shadcn/ui color aliases are correctly mapped to existing project CSS variables, enabling seamless component integration while maintaining the established color system. The mappings align well with the learnings about using Tailwind CSS variables.
95-95: The--color-focus-ringCSS variable is properly defined.The variable is correctly defined in
frontend/src/app/globals.cssfor both light and dark themes (line ~21 and ~42 respectively), set to20 184 166(teal color matching--color-primary). The Tailwind config ring mapping to this variable is correct and will function as intended for focus states.frontend/src/components/ui/alert-dialog.tsx (1)
1-115: Well-structured shadcn/ui AlertDialog component implementation.The component follows most coding guidelines effectively:
- ✓ Uses functional components with forwardRef
- ✓ Single quotes, no semicolons
- ✓ Mobile-first responsive design
- ✓ Proper spacing units and border-radius
- ✓ Correct typography hierarchy
- ✓ Uses Tailwind CSS variables and buttonVariants
- ✓ PascalCase components, kebab-case filename
The two optional refactors suggested above (overlay opacity and shadow) are minor design consistency improvements.
frontend/src/i18n/locales/zh-CN/common.json (4)
314-315: Settings section updated with new Models tab.Lines 314–315 add "team" and "models" keys to the settings navigation, aligning with the PR objective to add a Models tab between Team and Integrations.
118-122: Bot and team sections updated to support protocol selection and mode switching.Lines 118–122 add protocol-related keys (protocol, protocol_select, protocol_hint, select_executor_first, model_select) to the bot section, supporting runtime model resolution and provider selection (OpenAI vs. Anthropic). Lines 240–246 add keys for team mode switching UI (mode_change_confirm_title, mode_change_confirm_message, solo_max_one_bot, select_bot, etc.).
Also applies to: 240-246
531-539: Task submission UI strings added for model selection override.The new
task_submitsection covers UI labels for model selection (select_model, search_model), force override (force_override_model, override_short), and handling mixed executor types (mixed_team_warning). These align with task-level model override functionality.
477-530: English and Chinese translations are fully aligned; all component i18n references are correctly mapped.Verification confirms:
Locale parity verified:
frontend/src/i18n/locales/en/common.jsonandfrontend/src/i18n/locales/zh-CN/common.jsonboth contain identicalmodelsandtask_submitkey structures.Component key alignment verified: All three Model-related components correctly reference the defined translations:
- ModelEdit.tsx uses 34
models.*keys (form fields, hints, success/error messages)- ModelList.tsx uses 15
models.*keys (CRUD operations, titles, descriptions, errors)- ModelSelector.tsx uses 6
task_submit.*keys (model selection UI)No gaps detected: Every
t()call in the components matches an existing translation key. No missing or unused translation entries found.Both locales complete: Chinese and English translations are identical in structure and comprehensive in coverage.
backend/init_data/01-default-resources.yaml (1)
76-85: LGTM! Well-structured Shell CRD resource.The new
claude-shellresource follows the Kubernetes-inspired CRD design pattern consistently with other resources, properly decoupling shell configuration from model references.backend/app/schemas/kind.py (3)
17-17: LGTM! Good addition of displayName for human-readable resource names.The
displayNamefield inObjectMetaprovides a clean way to display user-friendly names while preserving the machine-readablenamefield for internal references.
62-63: LGTM! Model configuration flexibility.The
isCustomConfigandprotocolfields properly support distinguishing between predefined and custom model configurations, with appropriate typing.
138-138: LGTM! Making modelRef optional aligns with the decoupled model design.This change supports the new model resolution flow where Bots can operate without a direct model reference, relying on the shell-based or unified model discovery approach.
frontend/src/features/settings/components/TeamEditDrawer.tsx (1)
38-39: LGTM! Clean prop forwarding for agent type filtering.The
allowedAgentsprop is properly typed and documented, and correctly passed through to theBotEditcomponent.Also applies to: 359-359
frontend/src/features/settings/components/DifyBotConfig.tsx (2)
25-26: LGTM! Clean addition of readOnly mode.The
readOnlyprop with proper default value follows React patterns and is well-documented with JSDoc.Also applies to: 48-48
250-253: LGTM! Validation button properly respects readOnly state.The disabled condition correctly combines all relevant states:
isValidating || !difyApiKey || !difyBaseUrl || readOnly.frontend/src/components/ui/transfer.tsx (3)
120-172: LGTM! Well-implemented drag-and-drop handlers.The drag handlers properly:
- Set appropriate
effectAllowedanddropEffect- Handle edge cases (same item drop, invalid indices)
- Clean up drag state on completion/cancellation
- Use immutable array operations for reordering
245-281: LGTM! Clean drag-enabled item rendering.The conditional drag behavior based on
canDragis well-implemented:
- Drag attributes only applied when appropriate
- Visual feedback via CSS classes for dragging/drag-over states
- GripVertical handle provides clear affordance
- Click-to-select preserved alongside drag functionality
338-339: LGTM! Correct wiring of isRightList for right panel.The
isRightList: trueparameter is correctly passed to enable drag-and-drop only on the right side.frontend/src/features/settings/components/team-modes/BotTransfer.tsx (4)
1-17: LGTM: Clean imports and proper component setup.The component follows React best practices with proper imports, use of
useMemofor memoization, and appropriate typing. The imports are well-organized and use the correct path aliases.
18-38: Well-defined props interface with good documentation.The
BotTransferPropsinterface is comprehensive with JSDoc comments for the optional props. Consider adding JSDoc comments to the remaining props for consistency.
122-150: Transfer change handler logic is correct.The
onTransferChangehandler properly handles both directions, auto-sets leader when appropriate, and maintains state consistency when the leader is removed. Good use ofSetfor deduplication.
261-264: Consider using CSS variables consistently.The
listStyleusesrgb(var(--color-bg-surface))format, which is consistent with the coding guidelines for Tailwind CSS variables. Good adherence to the design system.frontend/src/features/settings/components/team-modes/SoloModeEditor.tsx (1)
152-171: Effect correctly handles new bot detection.The effect properly tracks previous bot IDs using a ref and auto-selects newly created bots. The logic is sound, and the dependency array is correct. Good pattern for detecting external state changes.
frontend/src/features/settings/components/TeamEdit.tsx (1)
1033-1047: Mode change confirmation dialog is well implemented.The dialog properly uses shadcn/ui components, handles both confirm and cancel actions, and integrates with the state management. Good UX pattern for preventing accidental data loss.
backend/app/services/adapters/team_kinds.py (1)
1071-1077: Good guard for optional modelRef.The early return when
modelRefis missing prevents null pointer errors in the subsequent query. This is a proper defensive coding pattern.frontend/src/features/settings/components/BotEdit.tsx (3)
877-887: LGTM on protocol filtering logic.The agent-specific protocol filtering correctly limits ClaudeCode to Claude protocol and Agno to OpenAI protocol, with fallback to showing all options for unknown/unselected agents.
353-373: LGTM on model type-aware restoration.The model restoration logic correctly handles type disambiguation when restoring saved predefined models. It matches by both name and type when type is specified, falling back to name-only matching when type is absent.
636-653: LGTM on read-only mode implementation.The read-only mode is consistently implemented across the component, with proper button state handling and visual feedback. The edit button correctly triggers
onEditClickcallback and the cancel button only appears whenembedded && onCancelEdit.backend/app/services/model_aggregation_service.py (1)
416-453: LGTM on resolve_model implementation.The
resolve_modelmethod correctly implements type-aware resolution with sensible fallback behavior (user models first, then public models). The error handling for invalid model types is appropriate.backend/app/services/adapters/bot_kinds.py (3)
61-76: LGTM on predefined model detection.The logic correctly identifies predefined model configurations by checking that
bind_modelis present and no other keys exist besides the optionalbind_model_type. This mirrors the frontend implementation infrontend/src/features/settings/services/bots.ts.
901-914: LGTM on batch public model fallback.The implementation correctly handles missing models by batch-fetching from the public_models table, maintaining the N+1 query optimization while supporting the hybrid model architecture.
959-1012: LGTM on agent_config determination logic.The logic correctly handles all model configuration scenarios:
- Override agent_config (for predefined models)
- PublicModel instances (returns
bind_model_type: "public")- Shared/predefined models (returns
bind_model_type: "user")- Custom config models (returns full modelConfig with protocol)
- Dedicated models without isCustomConfig (backward compatibility)
The
bind_model_typefield is essential for the frontend to correctly identify model sources and avoid naming conflicts. Based on learnings, this aligns with the Kubernetes-inspired CRD design pattern.frontend/src/i18n/locales/en/common.json (2)
476-529: LGTM on Model Management i18n translations.The new
modelssection provides comprehensive translations for the Model Management module:
- Clear, user-friendly labels and hints
- Complete error message coverage
- Consistent naming conventions with the rest of the file
248-260: LGTM on team mode translations.The new
solomode translations are well-written and clearly explain the single-bot independent handling pattern. The description aligns with the collaboration mode semantics.
| import logging | ||
| logger = logging.getLogger(__name__) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move logger initialization to module level.
Importing and initializing the logger inside methods is inefficient and unconventional. The logger should be initialized once at the module level.
Add at the top of the file (after imports):
import logging
logger = logging.getLogger(__name__)Then remove the inline imports at lines 128-129, 473-474, 805-806, and 930-931.
🤖 Prompt for AI Agents
In backend/app/services/adapters/bot_kinds.py around lines 128-129 (also
affecting inline logger inits at 473-474, 805-806, and 930-931), the code
imports and initializes the logging.Logger inside functions; move the logging
setup to module scope by adding a single import logging and logger =
logging.getLogger(__name__) at the top of the file (after other imports) and
remove the inline import logging / logger = ... statements at the listed line
ranges so all functions use the module-level logger.
| "team_model_desc": { | ||
| "solo": "A single bot handles the task independently, suitable for simple scenarios or single-responsibility workflows.", | ||
| "pipeline": "Agents are chained one after another, where each agent refines or transforms the result in turn.", | ||
| "route": "A central agent acts as a message router, directing communication between agents without them talking directiy.", |
There was a problem hiding this comment.
Typo in route mode description.
"directiy" should be "directly".
- "route": "A central agent acts as a message router, directing communication between agents without them talking directiy.",
+ "route": "A central agent acts as a message router, directing communication between agents without them talking directly.",📝 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.
| "route": "A central agent acts as a message router, directing communication between agents without them talking directiy.", | |
| "route": "A central agent acts as a message router, directing communication between agents without them talking directly.", |
🤖 Prompt for AI Agents
In frontend/src/i18n/locales/en/common.json around line 257, the string value
for "route" contains a typo: "directiy" should be "directly"; update the JSON
entry to replace "directiy" with "directly" and ensure the surrounding JSON
punctuation and escaping remain valid.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
backend/app/services/kind_impl.py (2)
137-137: Verify removal of Model reference validation - downstream code may have unsafe access.The Model existence check was removed from
_validate_referencesinBotKindService. As noted in a previous review, downstream code intask_kinds.pyandexecutor_kinds.pymay directly accessbot_crd.spec.modelRef.namewithout None checks.Please verify that all downstream code handles
modelRef=Nonesafely. Run this script to check for unsafe access patterns:#!/bin/bash # Search for direct modelRef attribute access without None checks rg -n -C3 'modelRef\.(name|namespace)' backend/app/services/adapters/
71-80: Decryption failure may expose encrypted data to client.When decryption fails, the method logs a warning but returns the result with the still-encrypted (or corrupted) API key intact. This could leak the encrypted value to the frontend.
Apply this diff to mask the API key on decryption failure:
try: if 'spec' in result and 'modelConfig' in result['spec']: model_config = result['spec']['modelConfig'] if 'env' in model_config and 'api_key' in model_config['env']: api_key = model_config['env']['api_key'] if api_key: result['spec']['modelConfig']['env']['api_key'] = decrypt_api_key(api_key) except (ValueError, KeyError) as e: logger.warning("Failed to decrypt API key: %r", e) + # Mask the key to avoid exposing encrypted data + if 'spec' in result and 'modelConfig' in result['spec']: + model_config = result['spec']['modelConfig'] + if 'env' in model_config and 'api_key' in model_config['env']: + result['spec']['modelConfig']['env']['api_key'] = "***"backend/app/services/adapters/team_kinds.py (1)
796-886: Excessive INFO-level logging in_get_bot_summary.The method contains 10+
logger.info()calls with DEBUG-level prefixes that will generate significant log volume in production. These should belogger.debug().Apply this diff to use appropriate log levels:
- logger.info(f"[_get_bot_summary] bot.name={bot.name}, modelRef.name={model_ref_name}, modelRef.namespace={model_ref_namespace}") + logger.debug(f"[_get_bot_summary] bot.name={bot.name}, modelRef.name={model_ref_name}") # ... (apply similar changes to all other logger.info calls in this method) - logger.info(f"[_get_bot_summary] Private model found: {model is not None}") + logger.debug(f"[_get_bot_summary] Private model found: {model is not None}")backend/app/services/adapters/bot_kinds.py (1)
128-129: Move logger initialization to module level.The logger is imported and initialized inside methods at multiple locations (lines 128-129, 473-474, 805-806, 930-931). This is inefficient and unconventional.
Add the logger at module level after imports (around line 24):
+import logging + +logger = logging.getLogger(__name__) + class BotKindsService(BaseService[Kind, BotCreate, BotUpdate]):Then remove the inline imports at lines 128-129, 473-474, 805-806, and 930-931.
🧹 Nitpick comments (9)
backend/app/services/adapters/team_kinds.py (1)
748-778: Redundant database query for first_bot.The code queries
first_botby ID at lines 751-756, but this bot was already fetched in the loop at lines 722-728 and stored inbots[0]. This results in an unnecessary database round trip.Refactor to reuse the already-fetched bot:
# Get agent_type from the first bot's shell agent_type = None - if bots: - first_bot_id = bots[0]["bot_id"] - first_bot = db.query(Kind).filter( - Kind.id == first_bot_id, - Kind.user_id == user_id, - Kind.kind == "Bot", - Kind.is_active.is_(True) - ).first() - - if first_bot: + if team_crd.spec.members: + first_member = team_crd.spec.members[0] + first_bot = db.query(Kind).filter( + Kind.user_id == user_id, + Kind.kind == "Bot", + Kind.name == first_member.botRef.name, + Kind.namespace == first_member.botRef.namespace, + Kind.is_active.is_(True) + ).first() + if first_bot: bot_crd = Bot.model_validate(first_bot.json)Alternatively, extract agent_type from the first bot_summary's agent_name since it's already computed.
backend/app/services/model_aggregation_service.py (3)
268-275: Accessing private method_format_resourcefrom external service.Line 270 calls
kind_service._format_resource("Model", resource), which is a private method (indicated by the underscore prefix). This creates tight coupling and may break if the internal implementation changes.Consider either:
- Make
_format_resourcea public method if it's intended for external use- Add a public method in
kind_servicethat returns formatted resource data- Extract the model info directly from
resource.jsonhere# Alternative approach - extract directly from resource model_data = resource.json # resource.json contains the CRD data
297-303: Hardcoded limit=1000 may miss models or cause performance issues.The hardcoded
limit=1000could miss models if there are more than 1000 public models, or cause memory pressure if there are many models.Consider:
- Using pagination and aggregating results
- Making the limit configurable
- Adding a warning if the limit is reached
MAX_PUBLIC_MODELS = 1000 # Extract to constant public_models = public_model_service.get_models( db=db, skip=0, limit=MAX_PUBLIC_MODELS, current_user=current_user ) if len(public_models) >= MAX_PUBLIC_MODELS: logger.warning("Public models limit reached, some models may be missing")
391-413: Inefficient lookup for public model by name.The
get_model_by_name_and_typemethod fetches all public models (up to 1000) to find one by name. This is inefficient when only a single model is needed.Add a dedicated method in
public_model_serviceto fetch by name, or query directly:elif model_type == ModelType.PUBLIC: from app.models.public_model import PublicModel public_model = db.query(PublicModel).filter( PublicModel.name == name, PublicModel.is_active.is_(True) ).first() if public_model: config = public_model.json.get("spec", {}).get("modelConfig", {}) if public_model.json else {} env = config.get("env", {}) if isinstance(config, dict) else {} return UnifiedModel( name=public_model.name, model_type=ModelType.PUBLIC, display_name=None, provider=env.get("model") if isinstance(env, dict) else None, model_id=env.get("model_id") if isinstance(env, dict) else None, config=config, is_active=public_model.is_active ).to_full_dict()backend/app/services/adapters/bot_kinds.py (1)
467-737: Methodupdate_with_userexceeds recommended length (~270 lines).This method handles many concerns: name validation, model switching (predefined vs custom), protocol extraction, shell updates, ghost updates, and timestamp management. This exceeds the 50-line guideline significantly and makes the code harder to maintain.
Consider extracting into smaller focused methods:
def update_with_user(self, db: Session, *, bot_id: int, obj_in: BotUpdate, user_id: int) -> Dict[str, Any]: bot = self._get_bot_for_update(db, bot_id, user_id) update_data = obj_in.model_dump(exclude_unset=True) self._handle_name_update(db, bot, update_data, user_id) ghost, shell, model = self._get_bot_components(db, bot, user_id) self._handle_agent_name_update(db, shell, update_data) model, return_agent_config = self._handle_agent_config_update( db, bot, model, update_data, user_id ) self._handle_ghost_updates(ghost, update_data, db, user_id) self._update_timestamps(bot, ghost, shell, model) db.commit() # ... refresh and returnAs per coding guidelines, Python functions should not exceed 50 lines.
frontend/src/features/settings/components/TeamEdit.module.css (1)
5-386: Consider refactoring for maintainability.This CSS module contains extensive responsive overrides (382 lines) with significant duplication across breakpoints and heavy
!importantusage. While the comprehensive mobile/tablet/desktop coverage is good, consider:
- Extracting common patterns into reusable rules
- Documenting why specific
!importantdeclarations are needed for Ant Design overrides- Consolidating duplicate Transfer component rules that appear across multiple breakpoints
For example, Transfer list overflow prevention appears in lines 29-37, 180-193, 255-265, 282-288, and 308-317. These could potentially be consolidated with breakpoint-specific variations only where needed.
frontend/src/features/settings/components/TeamEdit.tsx (3)
333-421: Consider adding Zod validation for the form.The save handler validates inputs using inline checks, but the coding guidelines specify using
react-hook-form + zod validationfor forms. While the current validation works, adopting the prescribed approach would provide:
- Type-safe schema validation
- Better error handling and user feedback
- Consistency with the project's form handling patterns
As per coding guidelines: "Use shadcn/ui Form component with react-hook-form + zod validation"
Example structure:
import { useForm } from 'react-hook-form' import { zodResolver } from '@hookform/resolvers/zod' import { z } from 'zod' const teamSchema = z.object({ name: z.string().min(1, 'Team name is required'), mode: z.enum(['solo', 'pipeline', 'route', 'coordinate', 'collaborate']), leaderBotId: z.number().nullable().refine(val => val !== null, { message: 'Bot is required' }), // ... other fields }) const form = useForm({ resolver: zodResolver(teamSchema), defaultValues: { ... } })
439-440: Simplify the long className string.The className combines multiple utilities with template literal and CSS module reference, reducing readability.
Consider using the
clsxutility (already imported in dependencies) to improve clarity:+import { clsx } from 'clsx' - <div - className={`flex flex-col flex-1 min-h-0 items-stretch bg-surface rounded-lg pt-0 pb-4 relative w-full max-w-none px-0 md:px-4 overflow-hidden ${styles.teamEditContainer}`} - > + <div + className={clsx( + 'flex flex-col flex-1 min-h-0 items-stretch bg-surface rounded-lg', + 'pt-0 pb-4 relative w-full max-w-none px-0 md:px-4 overflow-hidden', + styles.teamEditContainer + )} + >
277-282: Remove confusing comment about agent_name restriction.The comment states "agent_name restriction has been removed" and "No agent_name restriction - always return null", but the code simply returns
nullwithout any conditional logic. This creates confusion about the purpose of this memoized value.If
selectedAgentNameis no longer needed, consider removing it entirely along with its usages (line 609). If it's needed for backward compatibility or future use, clarify the comment:- // Get currently selected agent_name (from leader or selected bot) - // Note: agent_name restriction has been removed - users can now select any mode const selectedAgentName = useMemo(() => { - // No agent_name restriction - always return null + // Agent name filtering is no longer enforced for mode selection return null; - }, []); + }, [])Or remove if unused:
- const selectedAgentName = useMemo(() => { - return null; - }, []); - // ... later in code <LeaderModeEditor // ... - selectedAgentName={selectedAgentName} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/app/services/adapters/bot_kinds.py(18 hunks)backend/app/services/adapters/team_kinds.py(5 hunks)backend/app/services/kind_impl.py(3 hunks)backend/app/services/model_aggregation_service.py(1 hunks)frontend/src/features/settings/components/TeamEdit.module.css(1 hunks)frontend/src/features/settings/components/TeamEdit.tsx(15 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code must be PEP 8 compliant with Black formatter (line length: 88)
Use isort for import organization in Python
Python functions and variables must have descriptive names
Docstrings required for all public Python functions and classes
Extract magic numbers to named constants in Python
Python functions must not exceed 50 lines
Type hints required for all Python function signatures
Run pylint and flake8 for Python code quality checks
Files:
backend/app/services/kind_impl.pybackend/app/services/adapters/team_kinds.pybackend/app/services/model_aggregation_service.pybackend/app/services/adapters/bot_kinds.py
frontend/**
📄 CodeRabbit inference engine (AGENTS.md)
Frontend: Only use
NEXT_PUBLIC_*for client-safe environment variables
Files:
frontend/src/features/settings/components/TeamEdit.module.cssfrontend/src/features/settings/components/TeamEdit.tsx
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/**/*.{ts,tsx}: TypeScript strict mode must be enabled in Frontend
Use functional components with hooks in React
Use Prettier formatter in Frontend with single quotes and no semicolons
Use ESLint with Next.js configuration in Frontend
Useconstoverlet, never usevarin TypeScript
Component names must use PascalCase; file names must use kebab-case in Frontend
Use mobile-first responsive design approach in Frontend components
Frontend design: Low saturation + low contrast for reduced eye strain with generous whitespace
Frontend design: Minimal shadows, subtle component differentiation (<10% background variance)
Frontend design: Use mint blue (#14B8A6) as primary accent sparingly
Use Tailwind CSS variables for colors: --color-bg-base, --color-text-primary, --color-border, etc.
Frontend spacing: Use standard units - p-2 (8px), p-4 (16px), p-6 (24px), gap-3 (12px)
Frontend border-radius: rounded-2xl (16px) for large containers, rounded-lg (12px) for cards, rounded-md (6px) for buttons
Frontend typography: H1 text-xl font-semibold, H2 text-lg font-semibold, H3 text-base font-medium, Body text-sm
Use shadcn/ui Button component with variants: default, secondary, ghost, outline, link
Use shadcn/ui Form component with react-hook-form + zod validation
Files:
frontend/src/features/settings/components/TeamEdit.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Use Kubernetes-inspired CRD design (Ghost, Model, Shell, Bot, Team, Task)
📚 Learning: 2025-11-26T07:34:32.591Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Use Kubernetes-inspired CRD design (Ghost, Model, Shell, Bot, Team, Task)
Applied to files:
backend/app/services/kind_impl.pybackend/app/services/adapters/bot_kinds.py
📚 Learning: 2025-11-26T07:34:32.591Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use mobile-first responsive design approach in Frontend components
Applied to files:
frontend/src/features/settings/components/TeamEdit.module.cssfrontend/src/features/settings/components/TeamEdit.tsx
📚 Learning: 2025-11-26T07:34:32.591Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T07:34:32.591Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use functional components with hooks in React
Applied to files:
frontend/src/features/settings/components/TeamEdit.tsx
🧬 Code graph analysis (3)
backend/app/services/adapters/team_kinds.py (3)
backend/app/schemas/kind.py (4)
Team(187-193)Bot(146-152)Shell(105-111)Model(71-77)frontend/src/types/api.ts (3)
Team(86-102)Bot(29-40)Shell(75-84)backend/app/models/kind.py (1)
Kind(14-30)
backend/app/services/model_aggregation_service.py (3)
backend/app/schemas/kind.py (2)
Model(71-77)Shell(105-111)backend/app/services/kind_impl.py (2)
_format_resource(65-81)_format_resource(339-387)backend/app/services/adapters/public_model.py (1)
get_models(179-193)
backend/app/services/adapters/bot_kinds.py (2)
backend/app/models/kind.py (1)
Kind(14-30)backend/app/schemas/kind.py (3)
Bot(146-152)Model(71-77)ModelRef(88-91)
🪛 Ruff (0.14.6)
backend/app/services/kind_impl.py
59-59: Redundant exception object included in logging.exception call
(TRY401)
⏰ 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: Test Executor
🔇 Additional comments (13)
backend/app/services/kind_impl.py (1)
42-63: LGTM for API key encryption logic.The
_extract_resource_datamethod correctly encrypts API keys before storage, with proper checks for already-encrypted values and the masked placeholder"***". The exception handling re-raises ValueError appropriately (after fixing the duplicate raise).backend/app/services/adapters/team_kinds.py (2)
912-921: LGTM for optional modelRef handling.The
_convert_bot_to_dictmethod now correctly handles the optionalmodelRefby checkingif bot_crd.spec.modelRefbefore querying for the model.
1070-1076: LGTM for get_team_input_parameters modelRef handling.The method correctly handles optional
modelRefby returning early withhas_parameters: Falsewhen no model reference exists.backend/app/services/model_aggregation_service.py (2)
29-93: LGTM for ModelType enum and UnifiedModel class.The
ModelTypeenum andUnifiedModelclass are well-designed with clear documentation. Theto_dictandto_full_dictmethods provide clean API response formatting with the importanttypefield for distinguishing model sources.
134-167: LGTM for agent compatibility check.The
_is_model_compatible_with_agentmethod provides a clean fallback chain: shell'ssupportModel→ agent provider mapping → allow all. This is well-structured and documented.backend/app/services/adapters/bot_kinds.py (5)
731-735: LGTM for exception handling improvement.The try-except block now properly logs the exception with
logger.debuginstead of silently passing. This addresses the previous review comment.
61-104: LGTM for predefined model helper methods.The new helper methods (
_is_predefined_model,_get_model_name_from_config,_get_model_type_from_config,_get_protocol_from_config) are well-designed with clear single responsibilities and proper docstrings.
105-194: LGTM for model lookup methods with type support.The
_get_model_by_name_and_typeand_get_model_by_namemethods provide clean fallback logic from user models to public models, with proper type hints and docstrings.Note: The DEBUG-level logging (lines 142, 155, 170, 180, 184) should remain as
logger.debugonce the module-level logger is added.
918-1012: LGTM for_convert_to_bot_dictmodel type differentiation.The method correctly handles multiple scenarios:
- Override agent_config for explicit predefined models
- Public models → returns
bind_model_type: "public"- Shared/predefined user models → returns
bind_model_type: "user"- Dedicated custom models → returns full config with protocol
The
bind_model_typefield properly identifies the model source to avoid naming conflicts.
901-914: LGTM for batch public model lookup.The
_get_bot_components_batchmethod now efficiently fetches missing models from thepublic_modelstable using a single query with OR filters, avoiding N+1 queries.frontend/src/features/settings/components/TeamEdit.tsx (3)
87-276: Well-implemented mode change confirmation flow.The mode change confirmation logic is well structured with proper state management, clear separation between checking conditions (
needsModeChangeConfirmation), executing changes (executeModeChange), and handling user actions (handleModeChange,handleConfirmModeChange,handleCancelModeChange). The confirmation properly resets team members and prompts when the mode changes.
502-503: Verify mobile layout for 5-column mode selector.The RadioGroup uses
grid-cols-5to display five mode options in a single row. On mobile screens (especially devices <375px wide), this may result in cramped buttons with truncated text or poor touch targets.Consider testing on small mobile devices or using a responsive grid pattern:
- className="w-full grid grid-cols-5 gap-2" + className="w-full grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-5 gap-2"This would display 2 columns on mobile, 3 on small tablets, and 5 on larger screens, ensuring better usability across all viewport sizes.
561-618: Well-structured mode-specific editor rendering.The conditional rendering of mode-specific editors (SoloModeEditor, PipelineModeEditor, LeaderModeEditor) is clean and maintainable. Each editor receives the appropriate props and is wrapped in consistent styling. The separation of concerns between different modes improves code organization.
| import logging | ||
| logger = logging.getLogger(__name__) | ||
|
|
||
| from app.models.public_model import PublicModel |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move logger initialization to module level.
Importing and initializing the logger inside methods is inefficient. The logger is already initialized at module level (line 11), so these inline imports are redundant.
Remove the inline imports at lines 801-802 and 804:
def _get_bot_summary(self, bot: Kind, db: Session, user_id: int) -> Dict[str, Any]:
"""
Get a summary of bot information including agent_config with only necessary fields.
This is used for team list to determine if bots have predefined models.
"""
- import logging
- logger = logging.getLogger(__name__)
-
- from app.models.public_model import PublicModel
+ from app.models.public_model import PublicModelNote: PublicModel import should be moved to module level (line 12 area) alongside other imports, or kept here if needed to avoid circular imports.
🤖 Prompt for AI Agents
In backend/app/services/adapters/team_kinds.py around lines 801 to 804, remove
the inline logger import/initialization and the inline PublicModel import and
instead use the module-level logger already defined at the top of the file (line
~11); also move the PublicModel import to module level near the other imports
(around line ~12) unless doing so creates a circular import—in that case keep
PublicModel import at this location but remove the redundant logger import;
ensure no other references rely on the removed local logger variable.
| except ValueError as e: | ||
| logger.exception("Failed to encrypt API key: %r", e) | ||
| raise | ||
| raise | ||
|
|
There was a problem hiding this comment.
Critical: Duplicate raise statement causes syntax error.
Line 61 contains a duplicate raise statement that will cause a syntax error when executed. This appears to be a copy-paste mistake.
Apply this diff to fix the duplicate raise:
except ValueError as e:
logger.exception("Failed to encrypt API key: %r", e)
raise
- raise🧰 Tools
🪛 Ruff (0.14.6)
59-59: Redundant exception object included in logging.exception call
(TRY401)
🤖 Prompt for AI Agents
In backend/app/services/kind_impl.py around lines 58 to 62, there's a duplicate
`raise` after catching ValueError which causes a syntax error; remove the extra
`raise` so only a single `raise` remains (preserve the logger.exception call and
re-raise the original exception once).
| .teamEditContainer :global(body), | ||
| .teamEditContainer :global(html) { | ||
| overflow-x: hidden !important; | ||
| } |
There was a problem hiding this comment.
Invalid selector scope for body and html elements.
These selectors attempt to apply overflow-x: hidden to body and html elements from within the .teamEditContainer scope. However, since body and html are ancestors of .teamEditContainer, this scoped selector won't match these elements correctly. The :global() wrapper doesn't change the ancestor relationship.
If you need to prevent horizontal overflow globally, consider one of these approaches:
- Apply the style directly in a global stylesheet (not scoped to
.teamEditContainer) - Use a layout wrapper component that applies these styles at the correct level
- Remove these rules if the other overflow prevention rules (lines 120-123, 237-240, etc.) are sufficient
- /* Prevent horizontal scroll on mobile */
- .teamEditContainer :global(body),
- .teamEditContainer :global(html) {
- overflow-x: hidden !important;
- }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/features/settings/components/TeamEdit.module.css around lines
65-68, the selectors ".teamEditContainer :global(body)" and ".teamEditContainer
:global(html)" are invalid because body and html are ancestors and cannot be
targeted from a child scope; move the overflow-x: hidden rule to a global
stylesheet or a top-level layout wrapper (or remove these rules if other global
overflow rules suffice) so the styles apply at the correct level rather than
inside the module scope.
- Update AGENTS.md with new Model Management section - Document model types (public/user) and resolution order - Add key API endpoints for model operations - Document bind_model configuration pattern - Update version to 1.0.8 - Update configuring-models.md (en/zh) - Add model types documentation (public/user) - Update configuration steps for new Model Management UI - Add connection test feature documentation - Add per-task model selection documentation - Document model resolution priority - Update creating-bots.md (en/zh) - Document bind_model as recommended model binding method - Make modelRef optional in spec section - Add bind_model_type for model type hints - Update managing-tasks.md (en/zh) - Add model_id and force_override_bot_model fields - Document per-task model selection feature These documentation updates reflect the changes from PR #194 which added the model management module with unified model view, connection testing, and per-task model selection.
- Update AGENTS.md with new Model Management section - Document model types (public/user) and resolution order - Add key API endpoints for model operations - Document bind_model configuration pattern - Update version to 1.0.8 - Update configuring-models.md (en/zh) - Add model types documentation (public/user) - Update configuration steps for new Model Management UI - Add connection test feature documentation - Add per-task model selection documentation - Document model resolution priority - Update creating-bots.md (en/zh) - Document bind_model as recommended model binding method - Make modelRef optional in spec section - Add bind_model_type for model type hints - Update managing-tasks.md (en/zh) - Add model_id and force_override_bot_model fields - Document per-task model selection feature These documentation updates reflect the changes from PR #194 which added the model management module with unified model view, connection testing, and per-task model selection. Co-authored-by: qdaxb <4157870+qdaxb@users.noreply.github.com>
Summary
ModelListcomponent for viewing/managing model configurationsModelEditcomponent for creating/editing models with:POST /models/test-connectionfor validating API credentialsGET /models/compatiblefor filtering models by agent typeTaskCreateschema withmodel_idandforce_override_bot_modelfieldsexecutor_kinds.pymodel resolution logic:bind_modelin agent_configprivate_modelTest plan
private_modelshould continue to workSummary by CodeRabbit
New Features
UX
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.