feat: add quick team cards feature for fast team switching #299
Conversation
- Backend: - Add SystemConfig model for storing system-wide configuration - Add user role field (user/admin) to User model - Create quick teams API endpoints (/quick-teams and /admin/quick-teams/*) - Update admin permission check to use role field - Add database migration for user role and system_config table - Frontend: - Add QuickTeamCards component showing quick team selection below input - Integrate QuickTeamCards in ChatArea for chat and code scenes - Create admin page (/admin) with QuickTeamsConfig management - Add IconPicker component for selecting Lucide icons - Add admin entry in TaskSidebar (only visible to admins) - Add admin API client for quick teams configuration - Update User type to include role field
WalkthroughAdds a Quick Teams feature: DB migration (SystemConfig table, user.role), backend endpoints and schemas for quick teams, admin authorization update, and frontend admin UI plus in-chat quick team cards and related API client and types. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin UI
participant FE as Frontend
participant BE as Backend API
participant DB as Database
Admin->>FE: Open /admin?tab=quick-teams
FE->>BE: GET /admin/quick-teams/config
BE->>DB: SELECT * FROM system_config WHERE config_key='quick_teams'
DB-->>BE: config row or empty
BE-->>FE: QuickTeamsConfig (or default)
FE->>BE: GET /admin/quick-teams/available-teams
BE->>DB: SELECT * FROM kind WHERE kind='Team'
DB-->>BE: teams
BE-->>FE: AvailableTeamsResponse
Admin->>FE: Modify config (add/remove/reorder/change icon)
FE->>BE: PUT /admin/quick-teams/config (payload)
BE->>DB: INSERT/UPDATE system_config for key='quick_teams'
DB-->>BE: OK
BE-->>FE: Success
FE->>FE: Invalidate caches / show toast
sequenceDiagram
participant User as End User
participant Chat as Chat UI
participant FE as Frontend
participant BE as Backend API
participant DB as Database
User->>Chat: Open chat (no messages)
Chat->>FE: request quick teams (scene=chat)
FE->>BE: GET /quick-teams?scene=chat
BE->>DB: SELECT config_value FROM system_config WHERE config_key='quick_teams'
DB-->>BE: config
BE->>DB: SELECT * FROM kind WHERE id IN (configured IDs) AND kind='Team'
DB-->>BE: matching teams
BE-->>FE: QuickTeamsListResponse
FE-->>Chat: render QuickTeamCards
User->>Chat: Click quick team card
Chat->>FE: select team (change context)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (11)
backend/app/models/__init__.py (1)
12-12: ExportingSystemConfigfrom models package is fine; consider sorting__all__Importing and exposing
SystemConfigvia__all__is correct. To satisfy Ruff’sRUF022and keep things tidy, you can sort__all__alphabetically.-__all__ = ["User", "Kind", "Subtask", "SharedTeam", "SkillBinary", "SystemConfig"] +__all__ = ["Kind", "SharedTeam", "SkillBinary", "Subtask", "SystemConfig", "User"]Also applies to: 18-18
frontend/src/types/api.ts (1)
13-13: Userrolefield aligns with backend; consider narrowing the typeAdding
role?: stringmatches the new backend field and keeps backward compatibility. If you only expect a small fixed set of roles (e.g."user"and"admin"), consider tightening the type to a string-literal union to catch typos at compile time:- role?: string; + role?: 'user' | 'admin';This is optional but makes role handling safer end-to-end.
frontend/src/features/tasks/components/ChatArea.tsx (1)
12-12: QuickTeamCards integration fits the blank-state flow;disabledprop is redundant hereWiring
QuickTeamCardsinto the no-messages branch withscene={taskType}andonTeamSelect={handleTeamChange}matches the intended fast team-switch UX for new chats/code tasks.One minor cleanup: inside
!hasMessagesthedisabled={hasMessages}prop will always befalse, so you could drop the prop or hardcodedisabled={false}for clarity.Also applies to: 1075-1083
backend/app/models/user.py (1)
25-26: Role column addition is correct; consider constraining allowed valuesAdding a non-null
rolecolumn with default"user"matches the new authorization logic and schema changes.To avoid bad data (e.g. typos like
"admni"), consider enforcing the allowed set at the DB/ORM layer, for example with anEnumor a CHECK constraint restrictingroleto the supported values ('user','admin', etc.). This is not strictly required but will make admin checks more robust over time.backend/app/core/security.py (1)
207-213: Admin check correctly usesrole; plan to phase out username special-casing laterThe updated
get_admin_usercorrectly gates admin access oncurrent_user.role == "admin"while still allowing a legacy"admin"username for backward compatibility.Once all real admin users have proper
role="admin"set, consider deprecating the username special case and relying solely onroleto reduce the risk of any future coupling between “being named admin” and “being an admin”.frontend/src/features/tasks/components/QuickTeamCards.tsx (1)
28-28: RenameMapimport to avoid shadowing global.The
Mapimport from lucide-react shadows the JavaScript globalMapobject. While this doesn't cause a runtime error in this file, it can create confusion and potential issues if you need to use the native Map.Apply this diff to rename the icon import:
Compass, - Map, + Map as MapIcon, Book, LucideIcon, } from 'lucide-react'And update the ICON_MAP:
Target, Compass, - Map, + Map: MapIcon, Book, }frontend/src/features/admin/components/IconPicker.tsx (1)
27-27: RenameMapimport to avoid shadowing global.Similar to QuickTeamCards, the
Mapimport shadows the JavaScript globalMapobject.Apply this diff to rename the icon import:
Compass, - Map, + Map as MapIcon, Book, LucideIcon, } from 'lucide-react'And update the AVAILABLE_ICONS array and ICON_MAP:
{ name: 'Compass', icon: Compass }, - { name: 'Map', icon: Map }, + { name: 'Map', icon: MapIcon }, { name: 'Book', icon: Book }, ]backend/app/api/endpoints/admin.py (1)
483-486: Consider using Pydantic's model_dump() instead of dict().Lines 484-485 use
.dict()which is deprecated in Pydantic v2. The code should use.model_dump()for forward compatibility.Apply this diff:
config_value = { - "chat": [item.dict() for item in config.chat], - "code": [item.dict() for item in config.code], + "chat": [item.model_dump() for item in config.chat], + "code": [item.model_dump() for item in config.code], }backend/app/api/endpoints/quick_teams.py (3)
25-34: Consider adding validation for config_value structure.The function returns
config.config_valuedirectly without validating its structure. If the stored value is malformed (not a dict, or missing expected keys), downstream code inget_quick_teamscould fail.Consider adding a validation check:
def get_quick_teams_config(db: Session) -> dict: """Get quick teams configuration from system config""" config = ( db.query(SystemConfig) .filter(SystemConfig.config_key == QUICK_TEAMS_CONFIG_KEY) .first() ) - if config and config.config_value: + if config and config.config_value and isinstance(config.config_value, dict): return config.config_value return {"chat": [], "code": []}
37-42: Add validation for the scene parameter.The
sceneparameter accepts any string, but only "chat" or "code" are valid. Invalid values silently return an empty list. Consider validating the parameter to provide clearer API contract and better error messages.Apply this diff to add validation:
+from fastapi import APIRouter, Depends, HTTPException, Query +from enum import Enum + +class SceneType(str, Enum): + chat = "chat" + code = "code" + @router.get("/quick-teams", response_model=QuickTeamsListResponse) async def get_quick_teams( - scene: str = Query(..., description="Scene type: chat or code"), + scene: SceneType = Query(..., description="Scene type: chat or code"), db: Session = Depends(get_db), current_user: User = Depends(get_current_user), ):
48-56: Add defensive handling for malformed config items.Line 52 assumes each item in
scene_configis a dict with a "team_id" key. If the stored configuration is malformed, this could raise a KeyError.Consider adding validation or error handling:
config = get_quick_teams_config(db) scene_config = config.get(scene, []) # Build team_id to config mapping - team_configs = {item["team_id"]: item for item in scene_config} + team_configs = { + item["team_id"]: item + for item in scene_config + if isinstance(item, dict) and "team_id" in item + } team_ids = list(team_configs.keys())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
backend/alembic/versions/add_role_and_system_config.py(1 hunks)backend/app/api/api.py(2 hunks)backend/app/api/endpoints/admin.py(3 hunks)backend/app/api/endpoints/quick_teams.py(1 hunks)backend/app/core/security.py(1 hunks)backend/app/models/__init__.py(1 hunks)backend/app/models/system_config.py(1 hunks)backend/app/models/user.py(1 hunks)backend/app/schemas/quick_teams.py(1 hunks)backend/app/schemas/user.py(2 hunks)frontend/src/apis/admin.ts(1 hunks)frontend/src/app/admin/page.tsx(1 hunks)frontend/src/config/paths.ts(1 hunks)frontend/src/features/admin/components/IconPicker.tsx(1 hunks)frontend/src/features/admin/components/QuickTeamsConfig.tsx(1 hunks)frontend/src/features/tasks/components/ChatArea.tsx(2 hunks)frontend/src/features/tasks/components/QuickTeamCards.tsx(1 hunks)frontend/src/features/tasks/components/TaskSidebar.tsx(3 hunks)frontend/src/types/api.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
frontend/src/types/api.tsfrontend/src/config/paths.tsfrontend/src/app/admin/page.tsxfrontend/src/features/admin/components/IconPicker.tsxbackend/app/models/system_config.pybackend/app/models/user.pyfrontend/src/features/admin/components/QuickTeamsConfig.tsxbackend/app/schemas/quick_teams.pyfrontend/src/features/tasks/components/QuickTeamCards.tsxfrontend/src/apis/admin.tsbackend/app/core/security.pybackend/app/schemas/user.pyfrontend/src/features/tasks/components/ChatArea.tsxbackend/app/api/api.pybackend/app/api/endpoints/quick_teams.pyfrontend/src/features/tasks/components/TaskSidebar.tsxbackend/alembic/versions/add_role_and_system_config.pybackend/app/models/__init__.pybackend/app/api/endpoints/admin.py
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/src/types/api.tsfrontend/src/config/paths.tsfrontend/src/app/admin/page.tsxfrontend/src/features/admin/components/IconPicker.tsxfrontend/src/features/admin/components/QuickTeamsConfig.tsxfrontend/src/features/tasks/components/QuickTeamCards.tsxfrontend/src/apis/admin.tsfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/TaskSidebar.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/src/types/api.tsfrontend/src/config/paths.tsfrontend/src/app/admin/page.tsxfrontend/src/features/admin/components/IconPicker.tsxfrontend/src/features/admin/components/QuickTeamsConfig.tsxfrontend/src/features/tasks/components/QuickTeamCards.tsxfrontend/src/apis/admin.tsfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/TaskSidebar.tsx
**/types/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript type definitions MUST be organized in
src/types/directory
Files:
frontend/src/types/api.ts
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/app/admin/page.tsxfrontend/src/features/admin/components/IconPicker.tsxfrontend/src/features/admin/components/QuickTeamsConfig.tsxfrontend/src/features/tasks/components/QuickTeamCards.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/TaskSidebar.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/app/admin/page.tsxfrontend/src/features/admin/components/IconPicker.tsxfrontend/src/features/admin/components/QuickTeamsConfig.tsxfrontend/src/features/tasks/components/QuickTeamCards.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/TaskSidebar.tsx
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React component files MUST use kebab-case naming convention
Files:
frontend/src/features/admin/components/IconPicker.tsxfrontend/src/features/admin/components/QuickTeamsConfig.tsxfrontend/src/features/tasks/components/QuickTeamCards.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/TaskSidebar.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST be PEP 8 compliant with Black formatter (line length: 88) and isort for import organization
Python code MUST include type hints for all functions and variables
Python functions SHOULD NOT exceed 50 lines (preferred maximum)
Python functions and classes MUST have descriptive names and public functions/classes MUST include docstrings
Python code MUST extract magic numbers to named constants
Files:
backend/app/models/system_config.pybackend/app/models/user.pybackend/app/schemas/quick_teams.pybackend/app/core/security.pybackend/app/schemas/user.pybackend/app/api/api.pybackend/app/api/endpoints/quick_teams.pybackend/alembic/versions/add_role_and_system_config.pybackend/app/models/__init__.pybackend/app/api/endpoints/admin.py
**/backend/app/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend SQLAlchemy models MUST be created in
app/models/directory
Files:
backend/app/models/system_config.pybackend/app/models/user.pybackend/app/models/__init__.py
**/backend/app/schemas/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend Pydantic schemas MUST be created in
app/schemas/directory
Files:
backend/app/schemas/quick_teams.pybackend/app/schemas/user.py
**/backend/app/api/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend API route handlers MUST be created in
app/api/directory
Files:
backend/app/api/api.pybackend/app/api/endpoints/quick_teams.pybackend/app/api/endpoints/admin.py
**/backend/alembic/versions/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend Alembic database migrations MUST be reviewed before applying, especially auto-generated migrations
Files:
backend/alembic/versions/add_role_and_system_config.py
🧠 Learnings (2)
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/app/schemas/**/*.py : Backend Pydantic schemas MUST be created in `app/schemas/` directory
Applied to files:
backend/app/schemas/quick_teams.py
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/alembic/versions/*.py : Backend Alembic database migrations MUST be reviewed before applying, especially auto-generated migrations
Applied to files:
backend/alembic/versions/add_role_and_system_config.py
🧬 Code graph analysis (6)
frontend/src/features/admin/components/IconPicker.tsx (1)
frontend/src/components/ui/popover.tsx (3)
Popover(29-29)PopoverTrigger(29-29)PopoverContent(29-29)
frontend/src/features/admin/components/QuickTeamsConfig.tsx (3)
backend/app/schemas/quick_teams.py (2)
QuickTeamsConfig(21-25)QuickTeamItem(13-18)frontend/src/apis/admin.ts (4)
QuickTeamsConfig(14-17)adminApis(44-64)AvailableTeam(32-38)QuickTeamItem(8-12)frontend/src/features/admin/components/IconPicker.tsx (1)
IconPicker(71-105)
backend/app/schemas/quick_teams.py (1)
frontend/src/apis/admin.ts (4)
QuickTeamItem(8-12)QuickTeamsConfig(14-17)QuickTeamResponse(19-26)QuickTeamsListResponse(28-30)
frontend/src/apis/admin.ts (2)
backend/app/schemas/quick_teams.py (4)
QuickTeamItem(13-18)QuickTeamsConfig(21-25)QuickTeamResponse(28-36)QuickTeamsListResponse(39-42)frontend/src/apis/client.ts (1)
apiClient(105-105)
frontend/src/features/tasks/components/ChatArea.tsx (1)
frontend/src/features/tasks/components/QuickTeamCards.tsx (1)
QuickTeamCards(67-142)
backend/app/models/__init__.py (2)
backend/app/models/system_config.py (1)
SystemConfig(14-32)backend/app/models/user.py (1)
User(14-37)
🪛 Biome (2.1.2)
frontend/src/features/admin/components/IconPicker.tsx
[error] 27-27: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
frontend/src/features/admin/components/QuickTeamsConfig.tsx
[error] 19-20: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 22-22: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
frontend/src/features/tasks/components/QuickTeamCards.tsx
[error] 28-28: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 Ruff (0.14.7)
backend/app/api/endpoints/quick_teams.py
40-40: 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)
41-41: Unused function argument: current_user
(ARG001)
41-41: 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)
backend/app/models/__init__.py
18-18: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
backend/app/api/endpoints/admin.py
452-452: 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)
453-453: Unused function argument: current_user
(ARG001)
453-453: 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)
471-471: 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)
472-472: Unused function argument: current_user
(ARG001)
472-472: 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)
503-503: 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)
504-504: Unused function argument: current_user
(ARG001)
504-504: 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)
⏰ 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). (2)
- GitHub Check: Test wegent CLI Integration
- GitHub Check: e2e-tests
🔇 Additional comments (17)
backend/app/models/system_config.py (1)
5-32: SystemConfig model looks solid and consistent with existing modelsKey/JSON structure, unique indexed key, and timestamp defaults all look good and match the existing ORM patterns in this project. No issues from a correctness or schema-design standpoint.
backend/app/api/api.py (1)
5-5: Quick-teams router integration is correctImporting
quick_teamsand includingquick_teams.routerwith its own internal prefix cleanly exposes the new/quick-teamsAPI without disturbing existing routes.Also applies to: 37-37
frontend/src/features/tasks/components/TaskSidebar.tsx (2)
38-39: Admin check logic is consistent with admin page.The admin permission check using
user?.role === 'admin' || user?.user_name === 'admin'matches the pattern in the admin page, providing backward compatibility. This is good for consistency.
359-398: Admin entry placement and implementation look good.The admin section is correctly positioned above Settings for visibility and includes proper tooltip support in collapsed mode. Navigation closes the mobile sidebar appropriately.
frontend/src/config/paths.ts (1)
47-54: LGTM! Admin paths follow existing conventions.The admin path structure mirrors the settings paths pattern with tab-based navigation, maintaining consistency across the codebase.
frontend/src/features/tasks/components/QuickTeamCards.tsx (1)
67-98: Component logic and rendering look good.The query caching with 5-minute stale time is appropriate, and the team matching logic correctly handles missing teams. The null return for empty states is a clean approach.
frontend/src/features/admin/components/IconPicker.tsx (1)
71-104: Icon picker implementation is clean and functional.The popover-based icon selection with visual feedback for the current selection works well. State management is straightforward.
frontend/src/features/admin/components/QuickTeamsConfig.tsx (3)
22-22: Empty interface and props pattern is intentional.The empty interface and props destructuring on line 22 indicate this component takes no props currently. This is a valid pattern for future extensibility.
114-148: Move up/down logic correctly maintains sort order.The reordering logic properly:
- Creates a copy of the array before swapping
- Updates sort_order values to match new positions
- Maintains immutability through setState
159-290: Configuration UI provides comprehensive management capabilities.The component offers a complete admin interface with:
- Scene-based tabbing
- Visual team cards with icons
- Add/remove functionality
- Drag indicators and reordering controls
- Dirty state tracking with save confirmation
backend/app/api/endpoints/admin.py (2)
35-35: Good practice: Configuration key as constant.Defining
QUICK_TEAMS_CONFIG_KEYas a module-level constant prevents typos and makes refactoring easier.
450-466: No changes needed: Router is correctly configured with/adminprefix.The router in
admin.pyis defined without a prefix on line 33, but it is mounted with the/adminprefix inbackend/app/api/api.pyline 24:api_router.include_router(admin.router, prefix="/admin", tags=["admin"])This means the endpoint
@router.get("/quick-teams/config")is correctly accessible at/admin/quick-teams/config, matching the frontend expectations. No endpoint path modifications are required.Likely an incorrect or invalid review comment.
frontend/src/app/admin/page.tsx (1)
73-76: Consider consolidating admin check logic.The dual check
user.role === 'admin' || user.user_name === 'admin'provides backward compatibility but may create inconsistent behavior. Consider whether the username fallback is necessary long-term, especially after the migration adds the role field to all users. Search the codebase for other admin permission checks to assess consistency and determine if this pattern appears elsewhere or if consolidation to a single method is appropriate.frontend/src/apis/admin.ts (2)
5-42: LGTM! Type definitions align with backend schemas.The TypeScript interfaces correctly mirror the backend Pydantic models with proper type mappings (int→number, str→string, Optional[str]→string | null). The structure supports both public quick-teams access and admin configuration workflows.
44-64: LGTM! API methods follow clean patterns.The adminApis object provides a well-structured interface to the backend endpoints with proper HTTP verbs, type safety, and scene query parameter handling.
backend/app/schemas/quick_teams.py (1)
1-49: LGTM! Schemas are well-structured and compliant.The Pydantic models follow all coding guidelines: proper docstrings, complete type hints, PEP 8 formatting, and correct placement in
app/schemas/directory. The schema structure aligns with frontend TypeScript types and properly supports the Quick Teams configuration feature.Based on learnings, backend Pydantic schemas are correctly located in
app/schemas/directory.backend/app/api/endpoints/quick_teams.py (1)
59-86: LGTM! Query and response building logic is solid.The code correctly queries teams from the Kind table, safely accesses nested dictionary fields with appropriate defaults, and sorts results by sort_order. The use of
.get()with defaults prevents errors from missing metadata/spec fields.
|
|
||
| def upgrade(): | ||
| # Add role column to users table | ||
| op.add_column('users', sa.Column('role', sa.String(20), nullable=False, server_default='user')) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for follow-up migrations that update admin roles
echo "=== Checking for admin role updates in migrations ==="
rg -nP --type=py "UPDATE.*users.*SET.*role|admin" backend/alembic/versions/ | head -50
echo ""
echo "=== Checking for admin role initialization in app code ==="
rg -nP --type=py "role.*=.*['\"]admin['\"]" backend/app/ | head -50
echo ""
echo "=== Listing all migration files to check context ==="
ls -la backend/alembic/versions/*.py 2>/dev/null | head -20Repository: wecode-ai/Wegent
Length of output: 884
🏁 Script executed:
cat -n backend/alembic/versions/add_role_and_system_config.pyRepository: wecode-ai/Wegent
Length of output: 1866
Add data migration to set admin user role.
The migration adds a role column with default value 'user' but does not update the existing admin user (identified by user_name == 'admin') to have role = 'admin'. This creates a data inconsistency. Add an op.execute() call in the upgrade() function to update the admin user's role:
op.execute("UPDATE users SET role = 'admin' WHERE user_name = 'admin'")Also update the downgrade() function to revert this if needed.
🤖 Prompt for AI Agents
In backend/alembic/versions/add_role_and_system_config.py around line 21, the
migration adds a non-nullable role column defaulting to 'user' but does not
update the existing admin user; add an op.execute(...) in upgrade() to set
role='admin' for WHERE user_name='admin' immediately after adding the column,
and in downgrade() run an op.execute(...) to reset any admin row back to 'user'
(or otherwise revert the change) before dropping the column so the schema
rollback restores prior data state.
| sa.PrimaryKeyConstraint('id'), | ||
| ) | ||
| op.create_index(op.f('ix_system_config_config_key'), 'system_config', ['config_key'], unique=True) | ||
| op.create_index(op.f('ix_system_config_id'), 'system_config', ['id'], unique=False) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove redundant index on primary key.
Line 34 creates a non-unique index on the id column, but primary keys are automatically indexed by the database. This index is redundant and wastes storage.
Apply this diff to remove the redundant index:
op.create_index(op.f('ix_system_config_config_key'), 'system_config', ['config_key'], unique=True)
- op.create_index(op.f('ix_system_config_id'), 'system_config', ['id'], unique=False)
def downgrade():
# Drop system_config table
- op.drop_index(op.f('ix_system_config_id'), table_name='system_config')
op.drop_index(op.f('ix_system_config_config_key'), table_name='system_config')Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/alembic/versions/add_role_and_system_config.py at line 34, remove the
redundant non-unique index creation on the primary key by deleting the line
"op.create_index(op.f('ix_system_config_id'), 'system_config', ['id'],
unique=False)"; also remove or update any corresponding index drop in the
downgrade block if present so the migration stays consistent, then
re-run/refresh the migration to ensure no unused index operations remain.
| user_name: str | ||
| email: Optional[EmailStr] = None | ||
| is_active: Optional[bool] = True | ||
| role: Optional[str] = "user" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the schema file to understand the current field definitions
cat -n backend/app/schemas/user.pyRepository: wecode-ai/Wegent
Length of output: 3281
🏁 Script executed:
# Search for where UserCreate and UserBase are used in the backend
rg -n "UserCreate|UserBase" backend/app --type py -A 2 -B 2Repository: wecode-ai/Wegent
Length of output: 2168
🏁 Script executed:
# Look for endpoint handlers that accept UserCreate to understand usage context
rg -n "def.*create.*user|def.*update.*user" backend/app --type py -i -A 5Repository: wecode-ai/Wegent
Length of output: 7355
🏁 Script executed:
# Examine the create_user endpoint implementation fully
sed -n '70,90p' backend/app/api/endpoints/users.pyRepository: wecode-ai/Wegent
Length of output: 399
🏁 Script executed:
# Examine the service create_user implementation
sed -n '110,150p' backend/app/services/user.pyRepository: wecode-ai/Wegent
Length of output: 1368
🏁 Script executed:
# Check if there's any admin authorization or role validation in the service
rg -n "role" backend/app/services/user.py -A 3 -B 3Repository: wecode-ai/Wegent
Length of output: 42
Remove role from UserCreate schema to prevent confusion about client-supplied values
The role field in UserBase (and thus UserCreate) suggests that clients can specify their role during account creation. However, the create_user service method never uses obj_in.role—it silently ignores it and always creates users with the default database role. While this prevents accidental privilege escalation at the service layer, it creates confusing API contracts where clients may supply a value that has no effect.
To clarify intent and prevent future bugs:
- Remove
rolefromUserBaseandUserCreate; keep it only inUserInDBas a database-only field. - Create a separate admin-only schema (e.g.,
AdminUserCreateorAdminUserUpdate) that includesroleif admin-driven role assignment is needed. - Ensure
UserUpdatecontinues to excluderoleto prevent non-admin users from attempting role changes.
This aligns the schema with actual implementation behavior and makes security guarantees explicit.
🤖 Prompt for AI Agents
In backend/app/schemas/user.py around line 43, remove the role field from
UserBase/UserCreate (delete the Optional[str] = "user" declaration) so clients
cannot supply a role that is ignored by the service; keep role only on UserInDB
as a DB-only field, create a separate AdminUserCreate/AdminUserUpdate schema
that includes role for admin-driven creation/updates, and ensure UserUpdate does
not include role; update any imports/type references and tests to reflect the
new admin schema and the removal of client-visible role fields.
| if (!loading && !isAdmin) { | ||
| router.push('/chat') | ||
| } | ||
| }, [loading, isAdmin, router]) |
There was a problem hiding this comment.
Critical: undefined variable loading causing runtime error.
Lines 80 and 83 reference loading, but the variable destructured from useUser() on line 20 is isLoading. This will cause a runtime error.
Apply this diff to fix the variable name:
// Redirect non-admin users
useEffect(() => {
- if (!loading && !isAdmin) {
+ if (!isLoading && !isAdmin) {
router.push('/chat')
}
- }, [loading, isAdmin, router])
+ }, [isLoading, isAdmin, router])
- if (loading) {
+ if (isLoading) {
return (📝 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.
| if (!loading && !isAdmin) { | |
| router.push('/chat') | |
| } | |
| }, [loading, isAdmin, router]) | |
| if (!isLoading && !isAdmin) { | |
| router.push('/chat') | |
| } | |
| }, [isLoading, isAdmin, router]) |
🤖 Prompt for AI Agents
In frontend/src/app/admin/page.tsx around lines 80 to 83, the hook state
variable `loading` is referenced but the component actually destructures
`isLoading` from useUser() (around line 20); replace occurrences of `loading`
with `isLoading` in the conditional and the effect dependency array so the code
reads `if (!isLoading && !isAdmin) { router.push('/chat') }` and the
dependencies use `[isLoading, isAdmin, router]`.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
backend/app/api/api.py (1)
39-39: Consider adding a prefix or consolidating admin routes for clearer API organization.The router registration is functional. However, registering without a prefix means routes are fully defined within the
quick_teamsrouter, including admin endpoints (/admin/quick-teams/*) that overlap with the existing admin router's prefix space.For better API organization and discoverability, consider:
- Adding
prefix="/quick-teams"and moving admin endpoints to theadmin.routerfor consistency with the pattern where admin routes are consolidated under/admin- Or if grouping all quick-teams logic together is preferred (current approach), this is acceptable but may make admin endpoint discovery slightly less straightforward
frontend/src/features/tasks/components/TaskSidebar.tsx (2)
13-40: Centralize admin detection logic for consistency
isAdmincurrently encodes both the new role-based check and the legacyuser_name === 'admin'fallback inline. That works, but this logic will likely be needed in multiple places (e.g., guarding routes, other nav entries). To avoid drift and keep the criteria in one place, consider moving this into a shared helper/hook (e.g.,useIsAdmin()or a method on the user context) and using it here instead of inlining the condition.
363-402: Localize “Admin” label, tooltip, and aria-labelThe new Admin entry looks consistent with the New Task / Settings controls, but all visible strings (
aria-label="Admin", tooltip<p>Admin</p>, and the button label) are hardcoded. Since the rest of this component already usest(...)for UI text, consider wiring these through your i18n layer as well (e.g., at('navigation.admin')key) so the Admin entry is localizable and consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/app/api/api.py(2 hunks)backend/app/models/user.py(1 hunks)frontend/src/features/tasks/components/ChatArea.tsx(2 hunks)frontend/src/features/tasks/components/TaskSidebar.tsx(3 hunks)frontend/src/types/api.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/types/api.ts
- frontend/src/features/tasks/components/ChatArea.tsx
- backend/app/models/user.py
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
frontend/src/features/tasks/components/TaskSidebar.tsxbackend/app/api/api.py
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/src/features/tasks/components/TaskSidebar.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/src/features/tasks/components/TaskSidebar.tsx
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React component files MUST use kebab-case naming convention
Files:
frontend/src/features/tasks/components/TaskSidebar.tsx
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/features/tasks/components/TaskSidebar.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/features/tasks/components/TaskSidebar.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST be PEP 8 compliant with Black formatter (line length: 88) and isort for import organization
Python code MUST include type hints for all functions and variables
Python functions SHOULD NOT exceed 50 lines (preferred maximum)
Python functions and classes MUST have descriptive names and public functions/classes MUST include docstrings
Python code MUST extract magic numbers to named constants
Files:
backend/app/api/api.py
**/backend/app/api/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend API route handlers MUST be created in
app/api/directory
Files:
backend/app/api/api.py
🧬 Code graph analysis (1)
frontend/src/features/tasks/components/TaskSidebar.tsx (4)
frontend/src/features/common/UserContext.tsx (1)
useUser(148-148)frontend/src/components/ui/tooltip.tsx (4)
TooltipProvider(34-34)Tooltip(34-34)TooltipTrigger(34-34)TooltipContent(34-34)frontend/src/components/ui/button.tsx (1)
Button(55-55)frontend/src/config/paths.ts (1)
paths(5-55)
⏰ 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). (2)
- GitHub Check: Test wegent CLI Integration
- GitHub Check: e2e-tests
🔇 Additional comments (1)
backend/app/api/api.py (1)
5-5: LGTM! Import addition is correct.The
quick_teamsimport is properly added in alphabetical order and follows the existing pattern for endpoint imports.
Summary
Changes
Backend
SystemConfigmodel for storing system-wide configuration (quick teams config)rolefield to User model (user/admin)GET /api/quick-teams?scene=chat|code- Get quick teams list for scene (public)GET /api/admin/quick-teams/config- Get full configuration (admin only)PUT /api/admin/quick-teams/config- Update configuration (admin only)GET /api/admin/quick-teams/available-teams- Get all teams for config (admin only)Frontend
QuickTeamCardscomponent showing quick team selection below input area/admin) withQuickTeamsConfigmanagement componentIconPickercomponent for selecting Lucide icons for teamsrolefieldTest plan
Summary by CodeRabbit
New Features
Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.