Skip to content

Conversation

@rixitgithub
Copy link
Contributor

@rixitgithub rixitgithub commented Nov 8, 2025

Summary by CodeRabbit

Release Notes

  • New Features

    • Complete admin panel with login, dashboard, and moderation tools
    • Manage debates and comments with bulk deletion capabilities
    • Real-time and historical analytics dashboard
    • Admin action audit logs
    • Camera control in team debates
  • Improvements

    • Enhanced typing and speaking status indicators
    • Improved WebSocket synchronization for team discussions
  • UI Enhancements

    • New Checkbox and Alert components

- Added admin signup and login functionality.
- Created admin dashboard for managing debates, comments, and viewing analytics.
- Integrated Casbin for role-based access control.
- Developed admin action logging for auditing purposes.
- Updated frontend to include admin routes and components for signup and dashboard.
- Enhanced backend with new controllers and routes for admin operations.
- Added necessary dependencies for Casbin and MongoDB adapter.
@coderabbitai
Copy link

coderabbitai bot commented Nov 8, 2025

Walkthrough

Implements a comprehensive admin and analytics system with RBAC, authentication, moderation tools for managing debates and comments, real-time analytics tracking, and a corresponding frontend admin dashboard with authentication pages.

Changes

Cohort / File(s) Summary
Admin & Authentication System
backend/models/admin.go, backend/controllers/admin_controller.go, backend/middlewares/rbac.go, backend/routes/admin.go, backend/cmd/addadmin/main.go
Introduces Admin, Comment, AdminActionLog models; implements AdminSignup/AdminLogin handlers with JWT generation; adds Casbin-based RBAC middleware with MongoDB adapter; registers protected/unprotected admin routes; provides CLI tool to create initial admin users via password hashing and MongoDB insertion.
Analytics Tracking System
backend/controllers/analytics_controller.go
Implements GetAnalytics (real-time metrics aggregation from debates/users/comments), GetAnalyticsHistory (historical snapshots with gap-filling), and GetAdminActionLogs (paginated admin action retrieval); persists analytics snapshots to MongoDB for historical tracking.
Admin Frontend Pages
frontend/src/Pages/Admin/AdminSignup.tsx, frontend/src/Pages/Admin/AdminDashboard.tsx
Adds admin login form component and comprehensive admin dashboard with analytics visualization (summary cards, multi-tab charts), debates/comments/logs management with per-item and bulk deletion, role-based access control enforcement on UI actions.
Frontend Admin Integration
frontend/src/App.tsx, frontend/src/services/adminService.ts
Adds /admin/login and /admin/dashboard routes; introduces adminService with data models and API wrappers for authentication, analytics retrieval, debates/comments/logs CRUD operations with Bearer token authorization.
UI Components
frontend/src/components/ui/alert.tsx, frontend/src/components/ui/checkbox.tsx
Adds Alert (variant-driven), AlertTitle, AlertDescription components; introduces Checkbox wrapper around Radix UI with styling and Check icon indicator.
Server Integration
backend/cmd/server/main.go
Registers admin routes, initializes Casbin RBAC post-MongoDB connection, adds logging for connection and initialization status.
WebSocket Thread-Safety
backend/websocket/team_websocket.go
Introduces snapshotAllTeamClients helper for thread-safe client iteration; refactors broadcast points to use snapshot instead of direct map iteration; adds phase change logging.
Dependencies & Config
backend/go.mod, backend/rbac_model.conf
Adds Casbin and MongoDB adapter indirect dependencies; introduces RBAC model configuration with standard role-definition, policy-effect, and matcher sections.
Error Handling & Service Improvements
backend/services/rating_service.go, frontend/src/Pages/DebateRoom.tsx, frontend/src/Pages/Game.tsx, frontend/src/Pages/MatchLogs.tsx, frontend/src/Pages/TeamDebateRoom.tsx
Sanitizes NaN/Inf rating metrics; improves error logging in speech recognition; refactors WebSocket messaging with useCallback and state refs for stability; enhances tie-breaker logic with generalized conditions; synchronizes team/identity state with refs to stabilize WebSocket handlers and adds camera toggle functionality.
Package Dependencies
frontend/package.json
Adds @radix-ui/react-checkbox dependency.

Sequence Diagrams

sequenceDiagram
    participant Admin as Admin User
    participant Frontend as Frontend App
    participant Backend as Backend Server
    participant Casbin as Casbin Enforcer
    participant MongoDB as MongoDB
    
    Admin->>Frontend: Enter email & password
    Frontend->>Backend: POST /admin/login
    Backend->>MongoDB: Query admins collection by email
    MongoDB-->>Backend: Admin document
    Backend->>Backend: Verify password hash
    Backend->>Backend: Generate JWT token
    Backend-->>Frontend: { token, admin }
    Frontend->>Frontend: Store in localStorage
    Frontend->>Frontend: Redirect to /admin/dashboard
Loading
sequenceDiagram
    participant Admin as Admin User
    participant Frontend as Frontend App
    participant Backend as Backend Server
    participant Casbin as Casbin Enforcer
    participant MongoDB as MongoDB
    
    Admin->>Frontend: Request to delete debate
    Frontend->>Backend: DELETE /admin/debates/:id (with JWT)
    Backend->>Backend: AdminAuthMiddleware validates JWT
    Backend->>Casbin: Enforce(role, "debates", "delete")
    Casbin-->>Backend: Permission granted/denied
    alt Denied
        Backend-->>Frontend: 403 Forbidden
    else Allowed
        Backend->>MongoDB: Delete from debates collection
        Backend->>MongoDB: Log admin action to admin_action_logs
        Backend-->>Frontend: { success: true }
    end
    Frontend->>Frontend: Remove from debates list
Loading
sequenceDiagram
    participant Admin as Admin User
    participant Frontend as Frontend App
    participant Backend as Backend Server
    participant MongoDB as MongoDB
    
    Admin->>Frontend: Navigate to admin dashboard
    Frontend->>Backend: GET /admin/analytics (with JWT)
    Backend->>MongoDB: Aggregate from debates, users, comments
    Backend->>MongoDB: Compute totals (today's metrics, active users, etc.)
    Backend->>MongoDB: Insert snapshot to analytics_snapshots
    Backend-->>Frontend: { totalDebates, totalUsers, ... }
    
    Frontend->>Backend: GET /admin/analytics/history?days=7
    Backend->>MongoDB: Query analytics_snapshots for last 7 days
    Backend->>MongoDB: Fill gaps with per-day computed metrics
    Backend-->>Frontend: [ { timestamp, metrics }... ]
    
    Frontend->>Frontend: Render analytics charts and cards
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Areas requiring extra attention:

  • RBAC Middleware (backend/middlewares/rbac.go): Casbin enforcer initialization, default policy setup, and JWT validation logic requires careful verification of permission model and initialization order.
  • Analytics Aggregation (backend/controllers/analytics_controller.go): Complex MongoDB aggregation pipelines with multiple collections; verify NaN/Inf handling and gap-filling logic for historical data.
  • Admin Controller (backend/controllers/admin_controller.go): Multiple CRUD handlers with mixed operation patterns (single delete vs. bulk delete); verify transaction-like semantics and error rollback behavior.
  • WebSocket Thread-Safety (backend/websocket/team_websocket.go): Critical refactor from direct map iteration to snapshotAllTeamClients; verify no race conditions or missed broadcasts in concurrent scenarios.
  • Frontend Admin Dashboard (frontend/src/Pages/Admin/AdminDashboard.tsx): Large, stateful component with complex data loading, formatting, and UI interactions; verify data synchronization, null safety, and event handler bindings.
  • State Ref Synchronization (frontend/src/Pages/TeamDebateRoom.tsx): Extensive use of refs mirroring reactive state; verify consistency between refs and state in WebSocket message handlers and phase transitions.

Possibly related PRs

Suggested reviewers

  • bhavik-mangla

Poem

🐰 A rabbit hops through the admin domain,
With Casbin rules and analytics to gain,
Debates deleted, comments marked away,
Dashboards glow with metrics by day,
WebSockets snapshot safe and sound,
Admin magic all around!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: adding admin/moderator dashboard, role-based access control, analytics, and action logging. The PR introduces comprehensive admin features across backend and frontend that align directly with this description.
Docstring Coverage ✅ Passed Docstring coverage is 80.49% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 43

🧹 Nitpick comments (15)
frontend/src/components/ChatRoom.tsx (1)

52-56: Consider removing redundant WebSocket null checks.

After verifying wsRef.current in guard clauses, the subsequent checks before .send() are redundant. In the onopen callback (lines 52-56), wsRef.current is guaranteed to exist.

For example, simplify lines 123-127:

-    if (wsRef.current) {
-      wsRef.current.send(
-        JSON.stringify({ type: 'chatMessage', content: messageInput })
-      );
-    }
+    wsRef.current.send(
+      JSON.stringify({ type: 'chatMessage', content: messageInput })
+    );

Apply similar simplification at lines 52-56, 136-140, and 160-162.

Also applies to: 123-127, 136-140, 160-162

frontend/tsconfig.app.tsbuildinfo (1)

1-1: Consider excluding .tsbuildinfo files from version control.

.tsbuildinfo files are auto-generated TypeScript build artifacts used for incremental compilation. Including them in version control can lead to unnecessary merge conflicts and diffs.

Apply this change to exclude the file:

  1. Remove the file from version control:
#!/bin/bash
git rm --cached frontend/tsconfig.app.tsbuildinfo
  1. Add it to .gitignore:
+# TypeScript build info
+*.tsbuildinfo

Would you like me to open an issue to track this improvement?

frontend/src/Pages/OnlineDebateRoom.tsx (2)

616-619: Acceptable workaround for race conditions.

The 1-second delay before fetching participants addresses race conditions during room creation. While this works in practice, consider whether the backend could send a room-ready event to eliminate the need for an arbitrary delay.


1046-1133: LGTM! Comprehensive error handling with retry logic.

The error handler is well-designed with:

  • Appropriate categorization of error types (no-speech, network, permissions, etc.)
  • Differentiated retry strategies (3 attempts for network errors, 2 for others)
  • Clear user feedback on persistent errors
  • Guards to prevent retries in inappropriate states
  • Sensible retry delays (2-3 seconds)

The retry logic between network and default cases has some duplication, but given the different retry limits and delays, this may be acceptable. Consider extracting a shared retry helper if more error types need retry logic in the future.

backend/cmd/addadmin/main.go (1)

52-52: Note: defer may not execute on log.Fatalf.

The deferred db.MongoClient.Disconnect() won't execute if any of the subsequent log.Fatalf calls occur (lines 61, 64, 70, 87), as log.Fatalf calls os.Exit. This is acceptable for a CLI tool where the OS will clean up connections, but consider using log.Printf + os.Exit(1) after the defer if you need guaranteed cleanup.

backend/websocket/matchmaking.go (1)

194-199: Consider using proper JSON encoding for error messages.

Line 194 uses fmt.Sprintf to construct a JSON string, which could be fragile if the error message contains special characters. Consider using json.Marshal to construct the message properly:

-c.send <- []byte(fmt.Sprintf(`{"type":"error","error":"Failed to start matchmaking: %v"}`, err))
+errorMsg := MatchmakingMessage{Type: "error", Error: fmt.Sprintf("Failed to start matchmaking: %v", err)}
+if msgData, err := json.Marshal(errorMsg); err == nil {
+	c.send <- msgData
+}
backend/middlewares/auth.go (1)

54-73: Consider performance impact of per-request database lookups.

The middleware now performs a MongoDB query on every authenticated request to fetch user details. While the 5-second timeout provides protection, this could become a performance bottleneck under load.

Consider these alternatives:

  1. Include essential user data in JWT claims (userID, displayName, rating) to avoid the database lookup entirely
  2. Implement caching (e.g., Redis or in-memory cache with TTL) for user data
  3. Make the lookup conditional - only fetch from DB if the downstream handler needs fields beyond the email

If you need to ensure fresh data (e.g., for updated ratings), option 2 with a short TTL would balance performance and freshness.

backend/routes/admin.go (1)

22-37: Consider enforcing RBAC on read endpoints too.

Lines [23]-[37] rely solely on AdminAuthMiddleware, so every authenticated admin/moderator can hit analytics and logs even if the Casbin policy doesn’t grant that action. Wrapping the GET routes with the appropriate RBACMiddleware keeps authorization consistent and prevents policy drift.

-        admin.GET("/analytics", controllers.GetAnalytics)
+        admin.GET("/analytics", middlewares.RBACMiddleware("analytics", "view"), controllers.GetAnalytics)
-        admin.GET("/analytics/history", controllers.GetAnalyticsHistory)
+        admin.GET("/analytics/history", middlewares.RBACMiddleware("analytics", "view"), controllers.GetAnalyticsHistory)
-        admin.GET("/comments", controllers.GetComments)
+        admin.GET("/comments", middlewares.RBACMiddleware("comment", "view"), controllers.GetComments)
-        admin.GET("/logs", controllers.GetAdminActionLogs)
+        admin.GET("/logs", middlewares.RBACMiddleware("log", "view"), controllers.GetAdminActionLogs)
frontend/src/Pages/Admin/AdminDashboard.tsx (1)

129-129: Remove debug logging before release.

The console.log spam in production. Please drop it or guard it behind a dev flag.

backend/controllers/team_controller.go (2)

388-409: Average Elo recalculation leaves averageElo unset when last member leaves.

When memberCount == 0 you skip writing averageElo, leaving stale value. Set it to 0 (or null) so downstream consumers don’t see outdated numbers.


492-511: RemoveMember fails to recalc when team empties.

As above, if only captain remains and you remove someone, memberCount could be zero, leaving old average. Please handle zero-member case explicitly.

frontend/src/services/teamDebateService.ts (3)

4-6: Don’t default missing auth token to empty string.

getAuthToken silently returns "", so every call sends Authorization: Bearer , masking auth bugs and producing confusing 401s. Let it return null and gate fetches accordingly (or throw early). That better surfaces “not logged in” states.


37-52: Handle fetch error payload safely.

On non-OK responses you call response.json() without guarding against non-JSON replies. If backend returns plain text, this throws and masks original error. Use response.text() fallback or wrap in try/catch.


95-115: Surface backend error message in joinMatchmaking.

Same response.json() assumption as above and no token check; also should early exit if !token.

backend/services/team_turn_service.go (1)

37-62: Consider making token parameters configurable.

The initialization logic is correct. The hardcoded token parameters (capacity=10, refill rate=1) work for now but might benefit from being configurable per team or debate type in the future.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f29be62 and 6c9faea.

⛔ Files ignored due to path filters (2)
  • backend/go.sum is excluded by !**/*.sum
  • frontend/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (51)
  • backend/cmd/addadmin/main.go (1 hunks)
  • backend/cmd/server/main.go (2 hunks)
  • backend/controllers/admin_controller.go (1 hunks)
  • backend/controllers/analytics_controller.go (1 hunks)
  • backend/controllers/profile_controller.go (1 hunks)
  • backend/controllers/team_controller.go (1 hunks)
  • backend/controllers/team_debate_controller.go (1 hunks)
  • backend/controllers/team_matchmaking.go (1 hunks)
  • backend/db/db.go (1 hunks)
  • backend/go.mod (1 hunks)
  • backend/middlewares/auth.go (2 hunks)
  • backend/middlewares/rbac.go (1 hunks)
  • backend/models/admin.go (1 hunks)
  • backend/models/team.go (1 hunks)
  • backend/rbac_model.conf (1 hunks)
  • backend/routes/admin.go (1 hunks)
  • backend/routes/team.go (1 hunks)
  • backend/services/team_matchmaking.go (1 hunks)
  • backend/services/team_turn_service.go (1 hunks)
  • backend/websocket/matchmaking.go (2 hunks)
  • backend/websocket/team_debate_handler.go (1 hunks)
  • backend/websocket/team_websocket.go (1 hunks)
  • backend/websocket/websocket.go (3 hunks)
  • frontend/package.json (3 hunks)
  • frontend/src/App.tsx (4 hunks)
  • frontend/src/Pages/About.tsx (5 hunks)
  • frontend/src/Pages/Admin/AdminDashboard.tsx (1 hunks)
  • frontend/src/Pages/Admin/AdminSignup.tsx (1 hunks)
  • frontend/src/Pages/DebateRoom.tsx (1 hunks)
  • frontend/src/Pages/Game.tsx (6 hunks)
  • frontend/src/Pages/MatchLogs.tsx (5 hunks)
  • frontend/src/Pages/OnlineDebateRoom.tsx (15 hunks)
  • frontend/src/Pages/StartDebate.tsx (2 hunks)
  • frontend/src/Pages/TeamBuilder.tsx (1 hunks)
  • frontend/src/Pages/TeamDebateRoom.tsx (1 hunks)
  • frontend/src/Pages/TournamentHub.tsx (9 hunks)
  • frontend/src/components/ChatRoom.tsx (6 hunks)
  • frontend/src/components/Layout.tsx (1 hunks)
  • frontend/src/components/Matchmaking.tsx (6 hunks)
  • frontend/src/components/MatchmakingPool.tsx (1 hunks)
  • frontend/src/components/Sidebar.tsx (1 hunks)
  • frontend/src/components/SpeechTranscripts.tsx (2 hunks)
  • frontend/src/components/TeamChatSidebar.tsx (1 hunks)
  • frontend/src/components/TeamMatchmaking.tsx (1 hunks)
  • frontend/src/components/ui/alert.tsx (1 hunks)
  • frontend/src/components/ui/checkbox.tsx (1 hunks)
  • frontend/src/hooks/useUser.ts (1 hunks)
  • frontend/src/services/adminService.ts (1 hunks)
  • frontend/src/services/teamDebateService.ts (1 hunks)
  • frontend/src/services/teamService.ts (1 hunks)
  • frontend/tsconfig.app.tsbuildinfo (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (33)
backend/routes/team.go (4)
backend/controllers/team_controller.go (11)
  • CreateTeam (175-246)
  • GetTeam (249-270)
  • JoinTeam (273-355)
  • LeaveTeam (358-416)
  • RemoveMember (447-518)
  • UpdateTeamName (49-103)
  • UpdateTeamSize (106-172)
  • GetTeamByCode (30-46)
  • GetTeamMemberProfile (562-588)
  • GetUserTeams (419-444)
  • GetAvailableTeams (591-609)
backend/controllers/team_debate_controller.go (3)
  • CreateTeamDebate (18-94)
  • GetTeamDebate (97-114)
  • GetActiveTeamDebate (117-146)
backend/controllers/team_matchmaking.go (4)
  • JoinMatchmaking (17-69)
  • LeaveMatchmaking (72-82)
  • GetMatchmakingStatus (109-129)
  • GetMatchmakingPool (85-106)
backend/services/team_matchmaking.go (1)
  • GetMatchmakingPool (125-133)
frontend/src/Pages/Admin/AdminSignup.tsx (2)
backend/controllers/admin_controller.go (1)
  • AdminSignup (35-110)
frontend/src/services/adminService.ts (1)
  • adminLogin (78-94)
backend/controllers/analytics_controller.go (3)
backend/db/db.go (1)
  • MongoDatabase (17-17)
backend/models/admin.go (2)
  • AnalyticsSnapshot (53-63)
  • AdminActionLog (38-50)
frontend/src/services/adminService.ts (2)
  • AnalyticsSnapshot (50-60)
  • AdminActionLog (62-74)
frontend/src/components/ui/checkbox.tsx (1)
frontend/src/lib/utils.ts (1)
  • cn (4-6)
frontend/src/Pages/TeamBuilder.tsx (3)
backend/models/team.go (2)
  • TeamMember (25-32)
  • Team (11-22)
frontend/src/services/teamService.ts (13)
  • TeamMember (17-24)
  • Team (4-15)
  • getAvailableTeams (142-155)
  • getUserTeams (126-139)
  • createTeam (61-77)
  • joinTeam (96-108)
  • getTeamMemberProfile (308-321)
  • removeMember (276-289)
  • deleteTeam (292-305)
  • updateTeamName (324-339)
  • updateTeamSize (358-373)
  • leaveTeam (111-123)
  • getTeamByCode (342-355)
frontend/src/state/userAtom.ts (1)
  • userAtom (4-4)
backend/websocket/websocket.go (2)
backend/structs/websocket.go (2)
  • Room (30-37)
  • Message (10-13)
backend/models/debatevsbot.go (1)
  • Message (6-10)
backend/cmd/addadmin/main.go (3)
backend/config/config.go (1)
  • LoadConfig (53-65)
backend/db/db.go (3)
  • ConnectMongoDB (38-60)
  • MongoClient (16-16)
  • MongoDatabase (17-17)
backend/models/admin.go (1)
  • Admin (10-18)
frontend/src/components/TeamChatSidebar.tsx (4)
frontend/src/components/ui/button.tsx (1)
  • Button (57-57)
frontend/src/components/ui/scroll-area.tsx (1)
  • ScrollArea (46-46)
frontend/src/components/ui/avatar.tsx (3)
  • Avatar (48-48)
  • AvatarImage (48-48)
  • AvatarFallback (48-48)
frontend/src/components/ui/input.tsx (1)
  • Input (25-25)
frontend/src/components/TeamMatchmaking.tsx (3)
backend/models/team.go (1)
  • Team (11-22)
frontend/src/services/teamService.ts (2)
  • Team (4-15)
  • createTeamDebate (158-176)
frontend/src/services/teamDebateService.ts (5)
  • getActiveTeamDebate (71-84)
  • getMatchmakingStatus (132-145)
  • joinMatchmaking (87-100)
  • leaveMatchmaking (102-115)
  • createTeamDebate (31-52)
backend/websocket/team_debate_handler.go (2)
backend/models/team.go (2)
  • TeamDebateMessage (84-95)
  • TeamDebate (61-81)
backend/db/db.go (1)
  • GetCollection (21-23)
frontend/src/components/ui/alert.tsx (1)
frontend/src/lib/utils.ts (1)
  • cn (4-6)
frontend/src/components/MatchmakingPool.tsx (2)
backend/services/matchmaking.go (1)
  • MatchmakingPool (17-26)
frontend/src/services/teamDebateService.ts (1)
  • getMatchmakingPool (117-130)
backend/middlewares/auth.go (3)
backend/config/config.go (1)
  • LoadConfig (53-65)
backend/models/user.go (1)
  • User (10-30)
backend/db/db.go (1)
  • MongoDatabase (17-17)
backend/controllers/team_debate_controller.go (3)
backend/db/db.go (1)
  • GetCollection (21-23)
backend/models/team.go (2)
  • Team (11-22)
  • TeamDebate (61-81)
backend/services/team_matchmaking.go (1)
  • RemoveFromMatchmaking (115-122)
backend/middlewares/rbac.go (4)
backend/config/config.go (1)
  • LoadConfig (53-65)
backend/models/admin.go (2)
  • Admin (10-18)
  • AdminActionLog (38-50)
backend/db/db.go (1)
  • MongoDatabase (17-17)
backend/utils/auth.go (1)
  • Claims (64-69)
backend/controllers/team_matchmaking.go (3)
backend/db/db.go (1)
  • GetCollection (21-23)
backend/models/team.go (1)
  • Team (11-22)
backend/services/team_matchmaking.go (4)
  • StartTeamMatchmaking (31-62)
  • RemoveFromMatchmaking (115-122)
  • GetMatchmakingPool (125-133)
  • FindMatchingTeam (65-112)
frontend/src/components/ChatRoom.tsx (2)
backend/websocket/websocket.go (1)
  • Message (63-86)
backend/structs/websocket.go (1)
  • Message (10-13)
backend/controllers/admin_controller.go (6)
backend/models/admin.go (2)
  • Admin (10-18)
  • Comment (22-35)
frontend/src/services/adminService.ts (3)
  • Admin (3-8)
  • Debate (16-24)
  • Comment (26-37)
backend/db/db.go (1)
  • MongoDatabase (17-17)
backend/models/debatevsbot.go (1)
  • DebateVsBot (20-31)
backend/middlewares/rbac.go (1)
  • LogAdminAction (239-283)
backend/models/team.go (1)
  • TeamChatMessage (98-106)
frontend/src/services/adminService.ts (1)
backend/models/admin.go (4)
  • Admin (10-18)
  • Comment (22-35)
  • AnalyticsSnapshot (53-63)
  • AdminActionLog (38-50)
backend/cmd/server/main.go (4)
backend/middlewares/rbac.go (1)
  • InitCasbin (26-101)
backend/routes/team.go (4)
  • SetupTeamRoutes (10-27)
  • SetupTeamDebateRoutes (30-37)
  • SetupTeamChatRoutes (51-53)
  • SetupTeamMatchmakingRoutes (40-48)
backend/websocket/team_websocket.go (1)
  • TeamWebsocketHandler (102-388)
backend/routes/admin.go (1)
  • SetupAdminRoutes (11-39)
backend/controllers/team_controller.go (3)
backend/db/db.go (1)
  • GetCollection (21-23)
backend/models/team.go (2)
  • Team (11-22)
  • TeamMember (25-32)
frontend/src/services/teamService.ts (2)
  • Team (4-15)
  • TeamMember (17-24)
frontend/src/services/teamDebateService.ts (3)
frontend/src/utils/auth.ts (1)
  • getAuthToken (5-7)
backend/models/team.go (1)
  • TeamDebate (61-81)
frontend/src/services/teamService.ts (3)
  • TeamDebate (26-40)
  • createTeamDebate (158-176)
  • getTeamDebate (179-192)
backend/models/team.go (3)
frontend/src/services/teamService.ts (3)
  • TeamMember (17-24)
  • Team (4-15)
  • TeamDebate (26-40)
frontend/src/services/teamDebateService.ts (1)
  • TeamDebate (8-28)
backend/websocket/team_debate_handler.go (1)
  • TeamDebateMessage (38-47)
frontend/src/Pages/TeamDebateRoom.tsx (7)
backend/models/team.go (1)
  • TeamMember (25-32)
frontend/src/services/teamService.ts (2)
  • TeamMember (17-24)
  • getTeamDebate (179-192)
backend/websocket/team_debate_handler.go (1)
  • TeamDebateRoom (32-36)
frontend/src/state/userAtom.ts (1)
  • userAtom (4-4)
frontend/src/hooks/useUser.ts (1)
  • useUser (7-60)
frontend/src/utils/auth.ts (1)
  • getAuthToken (5-7)
frontend/src/services/teamDebateService.ts (1)
  • getTeamDebate (55-68)
backend/websocket/team_websocket.go (4)
backend/services/team_turn_service.go (5)
  • TeamTurnManager (132-136)
  • TokenBucket (22-28)
  • TokenBucketService (16-19)
  • NewTeamTurnManager (139-144)
  • NewTokenBucketService (31-35)
backend/utils/auth.go (1)
  • ValidateTokenAndFetchEmail (131-144)
backend/db/db.go (1)
  • MongoDatabase (17-17)
backend/models/team.go (2)
  • TeamDebate (61-81)
  • Team (11-22)
frontend/src/App.tsx (4)
backend/controllers/admin_controller.go (1)
  • AdminSignup (35-110)
frontend/src/Pages/Admin/AdminSignup.tsx (1)
  • AdminSignup (16-92)
frontend/src/Pages/Admin/AdminDashboard.tsx (1)
  • AdminDashboard (44-630)
backend/websocket/team_debate_handler.go (1)
  • TeamDebateRoom (32-36)
backend/routes/admin.go (3)
backend/controllers/admin_controller.go (7)
  • AdminLogin (113-164)
  • GetDebates (167-223)
  • DeleteDebate (226-261)
  • BulkDeleteDebates (264-315)
  • GetComments (318-392)
  • DeleteComment (395-436)
  • BulkDeleteComments (439-494)
backend/middlewares/rbac.go (2)
  • AdminAuthMiddleware (135-186)
  • RBACMiddleware (189-224)
backend/controllers/analytics_controller.go (3)
  • GetAnalytics (19-100)
  • GetAnalyticsHistory (103-213)
  • GetAdminActionLogs (216-248)
frontend/src/Pages/Admin/AdminDashboard.tsx (2)
frontend/src/services/adminService.ts (13)
  • Analytics (39-48)
  • Debate (16-24)
  • Comment (26-37)
  • AdminActionLog (62-74)
  • getAnalytics (97-108)
  • getAnalyticsHistory (110-127)
  • getDebates (130-148)
  • getComments (183-201)
  • getAdminActionLogs (241-259)
  • deleteDebate (150-163)
  • deleteComment (203-220)
  • bulkDeleteDebates (165-180)
  • bulkDeleteComments (222-238)
backend/models/admin.go (2)
  • Comment (22-35)
  • AdminActionLog (38-50)
backend/websocket/matchmaking.go (1)
backend/services/matchmaking.go (1)
  • GetMatchmakingService (40-49)
frontend/src/components/Matchmaking.tsx (2)
backend/websocket/matchmaking.go (1)
  • MatchmakingMessage (49-57)
backend/services/matchmaking.go (1)
  • MatchmakingPool (17-26)
backend/services/team_matchmaking.go (3)
backend/models/team.go (1)
  • Team (11-22)
backend/db/db.go (1)
  • GetCollection (21-23)
backend/controllers/team_matchmaking.go (1)
  • GetMatchmakingPool (85-106)
backend/services/team_turn_service.go (2)
backend/db/db.go (1)
  • GetCollection (21-23)
backend/models/team.go (1)
  • Team (11-22)
frontend/src/services/teamService.ts (3)
backend/models/team.go (3)
  • Team (11-22)
  • TeamMember (25-32)
  • TeamDebate (61-81)
frontend/src/services/teamDebateService.ts (3)
  • TeamDebate (8-28)
  • createTeamDebate (31-52)
  • getTeamDebate (55-68)
frontend/src/utils/auth.ts (1)
  • getAuthToken (5-7)
🪛 Biome (2.1.2)
frontend/src/Pages/TeamDebateRoom.tsx

[error] 494-495: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 495-496: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 571-572: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 595-596: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 596-596: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 597-598: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 628-629: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 629-631: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 649-649: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 649-650: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 650-651: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 651-652: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 652-654: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 672-673: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

🔇 Additional comments (56)
frontend/src/Pages/MatchLogs.tsx (1)

121-126: Formatting changes approved.

The reformatting of the return object, JSX div, timestamp display, score rendering, and winner text is cosmetic and preserves all functional logic.

Also applies to: 136-139, 158-160, 174-195, 201-216, 233-235

frontend/src/components/Layout.tsx (1)

2-20: Style-only changes—no functional impact.

All modifications are purely stylistic (quote style consistency and trailing semicolon). The component logic and rendering behavior remain unchanged.

frontend/src/Pages/StartDebate.tsx (1)

1-1: Good cleanup: unused default import removed.

The file only uses named imports (useContext, useState), so removing the default React import is appropriate and follows modern React best practices.

frontend/src/Pages/About.tsx (2)

1-121: Clarify the relationship between file changes and PR objectives.

This file updates the About page content with marketing copy and formatting improvements, which appears unrelated to the PR's stated objectives of adding "Admin & Moderator Dashboard with Role-Based Access, Analytics, and Action Logging."

Please clarify whether:

  1. This file was intentionally included as part of a broader release update, or
  2. These changes should be in a separate PR focused on content updates.

44-54: Two features are verified; Community-Driven Topics feature requires clarification.

The code review findings:

  • Structured Formats: ✅ Verified. Multiple debate room files (DebateRoom.tsx, OnlineDebateRoom.tsx, TeamDebateRoom.tsx) implement formal phases: "Opening Statements" (240s), "Cross-Examination" (180s), and "Closing Statements" (180s).

  • Personalized Progress Tracking: ✅ Verified. User ratings (Elo-based), ratings history, and skill tracking are implemented across multiple files (Profile.tsx, Matchmaking.tsx, context/authContext.tsx).

  • Community-Driven Topics: ⚠️ Unclear. ChatRoom.tsx includes voting functionality, but it tracks votes on debate positions (FOR/AGAINST), not on topic suggestions. The "Submit Debate Topics" mention appears only in the "Get Involved" section as a contribution method, not as a live platform feature for users.

Confirm whether community topic suggestion and voting is implemented as a user-facing feature, or if this claim should be removed/updated in the About page to avoid misleading users about available capabilities.

frontend/src/Pages/TournamentHub.tsx (1)

1-260: Formatting improvements look good!

The changes in this file are purely formatting and whitespace refinements with no functional modifications. The code structure, validation logic, and component behavior remain unchanged. These formatting improvements enhance readability without introducing any risks.

frontend/src/hooks/useUser.ts (2)

36-36: No changes needed—password field is intentionally set to empty string.

The code is correct. The password field is part of the User type definition but is deliberately set to empty string whenever user data is fetched because passwords should not be stored in client-side state. This pattern is consistent across the codebase, and the rationale is already documented in frontend/src/context/authContext.tsx line 138 with the comment: "Password should not be stored in client-side state."


24-35: Verify backend response structure—inconsistent profile field mapping confirmed across files.

The inconsistency flagged in the review comment is real and confirmed. useUser.ts (lines 24-28, 32-35) expects some fields nested under userData.profile with fallbacks, while authContext.tsx (lines 74-90) reads all fields directly from userData—yet both call the identical /user/fetchprofile endpoint.

If the backend returns fields nested in profile, authContext.tsx will receive undefined for those fields. If the backend returns a flat structure, useUser.ts will rely on fallbacks and miss actual profile data. Verify the backend response structure and ensure both files align with it, or implement consistent mapping across the codebase.

frontend/src/Pages/Game.tsx (5)

15-15: LGTM!

The addition of trailing commas follows JavaScript/TypeScript best practices and makes future diffs cleaner.

Also applies to: 19-19, 29-29


53-57: LGTM!

The multi-line formatting improves readability while maintaining the same functionality.


76-76: LGTM!

The inline comment clearly explains the state update purpose, improving code maintainability.


174-178: LGTM! Consistent functional setter pattern.

This mirrors the cameraOn setter implementation, ensuring consistent behavior across both media controls.


167-172: Implementation verified and working correctly.

PlayerCard is indeed using the functional setter pattern at lines 45 and 48, calling setCameraOn((prev) => !prev) and setMicOn((prev) => !prev) for toggle operations. The wrapper in Game.tsx correctly handles both direct values and updater functions, enabling proper state management and preventing stale closures.

frontend/tsconfig.app.tsbuildinfo (1)

1-1: Add .tsbuildinfo files to .gitignore—do not commit build artifacts.

The frontend/tsconfig.app.tsbuildinfo file is an auto-generated incremental build cache created by tsc -b. The "errors":true flag reflects the previous build state, not a current compilation failure. Build artifacts like .tsbuildinfo should be excluded from version control to prevent merge conflicts and unnecessary diffs.

Update frontend/.gitignore to include:

*.tsbuildinfo
backend/websocket/websocket.go (4)

84-85: LGTM! Clear separation of concerns.

The addition of both SpeechText (for final transcripts) and LiveTranscript (for interim transcripts) provides clear distinction between finalized and in-progress speech-to-text results. The naming conventions and JSON tags are consistent with the codebase.


232-233: LGTM! Consistent message routing.

The new liveTranscript message type is properly routed through the switch statement with the same handler signature pattern as other message types.


331-350: LGTM! Phase-aware transcript tracking.

The addition of the phase field to the speechText broadcast enables phase-specific transcript tracking on the frontend. This aligns well with the debate flow management and is consistent with the new liveTranscript handler.


352-367: LGTM! Clean interim transcript broadcasting.

The handleLiveTranscript implementation correctly:

  • Broadcasts interim transcripts to all other room participants
  • Excludes the sender from receiving their own interim transcript
  • Uses thread-safe writes via SafeWriteJSON
  • Includes phase context for proper frontend tracking
  • Avoids state mutation (appropriate for transient data)
frontend/src/components/SpeechTranscripts.tsx (2)

6-6: LGTM! Clean prop definition.

The optional liveTranscript prop is appropriately typed and integrated into the component props interface and destructuring.

Also applies to: 12-12


78-91: LGTM! Well-designed live transcript visualization.

The rendering logic effectively:

  • Shows live transcripts only for the current phase
  • Provides clear visual distinction with blue-themed styling and "Live:" label
  • Displays both final and interim transcripts simultaneously
  • Handles edge cases where only live transcript exists (no final transcript yet)
frontend/src/Pages/OnlineDebateRoom.tsx (8)

87-87: LGTM! Interface extension for live transcripts.

The liveTranscript field is correctly added to the WSMessage interface as an optional string, maintaining consistency with the backend WebSocket message structure.


165-166: LGTM! Appropriate state for error handling.

The addition of speechError state for user-facing error messages and retryCountRef for tracking retry attempts (without triggering re-renders) demonstrates proper React patterns.


462-516: LGTM! Improved resilience for participant data.

The changes improve robustness by:

  • Relaxing the participant requirement to >= 1 instead of exactly 2
  • Providing comprehensive fallbacks when participant data is missing
  • Handling race conditions during room initialization
  • Using currentUser data as a fallback for the local participant

The UI properly handles the case when opponentUser is null (e.g., showing "Waiting for opponent...").


668-693: LGTM! Correct transcript handling for both final and interim speech.

The WebSocket message handlers properly:

  • Store final speechText by phase (with sensible fallback to current phase)
  • Only process liveTranscript from the opponent (preventing echo)
  • Update the appropriate state for UI rendering

968-1022: LGTM! Complete speech recognition result processing.

The onresult handler effectively:

  • Distinguishes between final and interim results
  • Updates local state appropriately (phase-specific for final, transient for interim)
  • Broadcasts both final speechText and interim liveTranscript to the backend with phase context
  • Includes necessary metadata (userId, username, phase) for proper routing
  • Validates WebSocket state before sending

1191-1232: LGTM! Well-guarded speech recognition lifecycle.

The start/stop functions demonstrate good practices:

  • Comprehensive guards prevent starting in invalid states
  • Retry logic handles transient initialization failures
  • Retry counter reset on manual stop prevents stale retry state
  • Appropriate useCallback dependencies

965-965: LGTM! Thorough retry counter management.

The retry counter is reset at multiple appropriate points:

  • On successful recognition start
  • On manual stop
  • On phase transitions
  • On turn changes

This defensive approach ensures retry state doesn't carry over inappropriately between different contexts.

Also applies to: 1227-1227, 1269-1276


1809-1814: LGTM! Live transcript integration complete.

The liveTranscript prop correctly passes currentTranscript to the SpeechTranscripts component, completing the data flow from WebSocket messages and speech recognition to UI display. The turn-based debate structure ensures that only one participant speaks at a time, preventing conflicts between local and opponent interim transcripts.

backend/controllers/profile_controller.go (1)

193-193: Expose profile ID looks good

Including user.ID.Hex() in the profile payload gives the frontend a stable identifier to reference, which aligns with the new admin tooling.

backend/go.mod (1)

24-29: RBAC dependencies accounted for

The Casbin and adapter packages showing up here line up with the new authorization layers introduced in this PR.

backend/db/db.go (1)

20-23: Helper promotes consistent collection access

GetCollection is a sensible thin wrapper around MongoDatabase.Collection, reducing repetition in the new code.

frontend/package.json (1)

14-39: UI dependency additions make sense

The Radix checkbox and React DnD packages are consistent with the new interactive admin/team experiences added in this PR.

frontend/src/components/ui/checkbox.tsx (1)

1-29: Checkbox primitive wrapper looks solid

Forwarding the ref through Radix’s root with the composed classes keeps the checkbox consistent with the rest of the design system.

frontend/src/components/Matchmaking.tsx (1)

56-60: Connection guard prevents duplicate sockets

Short-circuiting when an OPEN socket already exists avoids redundant connections and the double-join issues they caused earlier.

backend/models/admin.go (1)

1-63: LGTM! Well-structured admin data models.

The model definitions follow MongoDB and Go best practices:

  • Proper BSON and JSON tags for all fields
  • Password field correctly excluded from JSON serialization (line 13)
  • Appropriate use of pointers for optional fields (DeletedAt, DeletedBy)
  • Flexible Details map in AdminActionLog for extensibility
backend/cmd/addadmin/main.go (1)

19-95: LGTM! CLI utility is well-structured.

The admin creation flow is solid:

  • Proper input validation
  • Duplicate admin check before insertion
  • Secure password hashing with bcrypt
  • Clear success output with all relevant details
frontend/src/components/Sidebar.tsx (1)

41-45: LGTM! Team Debates navigation item added.

The new navigation item follows the established pattern and uses an appropriate icon for team functionality.

backend/websocket/matchmaking.go (1)

188-208: Good improvement to user feedback.

Adding success confirmations and error messages improves the client experience by providing clear feedback for matchmaking state changes.

backend/cmd/server/main.go (2)

37-41: LGTM! Proper RBAC initialization sequence.

Casbin initialization is correctly placed after the MongoDB connection, which is required for the MongoDB adapter. Fatal error handling is appropriate for this critical infrastructure component.


127-140: LGTM! Team and admin routes properly registered.

The new routes are registered with appropriate logging for observability. The Team WebSocket handler is exposed at a global route as needed for WebSocket upgrades.

backend/rbac_model.conf (1)

1-14: LGTM! Standard Casbin RBAC model.

The model definition follows Casbin conventions and supports role-based access control with:

  • Subject, object, action tuple for requests and policies
  • Role inheritance via the g relation
  • Simple allow-based policy effect
  • Matcher that checks role membership and resource/action permissions

This is appropriate for the admin/moderator authorization system.

backend/middlewares/auth.go (1)

19-76: Enhanced authentication with logging.

The added logging improves observability for auth failures and JWT validation issues. The additional user context data (userID, displayName, etc.) enriches the context for downstream handlers.

frontend/src/App.tsx (2)

60-61: Verify admin dashboard protection.

The admin dashboard route (line 61) is exposed as a public route outside the ProtectedRoute. Based on the AdminDashboard component implementation (which checks localStorage for adminToken and redirects to '/admin/login' if missing), this appears to be intentional with protection handled internally.

However, for consistency with the application's routing pattern, consider moving admin routes under a protected route wrapper or clearly documenting why admin routes use client-side protection rather than route-level protection.


1-28: LGTM! New admin and team routes properly wired.

The new pages are imported and routed appropriately:

  • Team routes are protected via ProtectedRoute
  • Import change to named useContext is fine
  • Route structure follows existing patterns

Also applies to: 69-69, 92-92

frontend/src/Pages/Admin/AdminSignup.tsx (1)

28-37: Login flow integrates cleanly.

The submit handler covers loading/error state management and persists the admin session in local storage appropriately. Nicely done.

backend/routes/team.go (1)

10-26: Route surface looks comprehensive.

Nice job grouping the full set of team CRUD, membership, and discovery routes under a single router group—no concerns here.

backend/services/team_turn_service.go (10)

1-13: LGTM!

Package declaration and imports are appropriate for the functionality.


15-28: LGTM!

The two-level mutex design (service-level for bucket map access, bucket-level for token operations) provides good concurrency granularity.


30-35: LGTM!

Constructor follows standard Go patterns.


64-88: LGTM!

Proper concurrency control with two-phase locking. The method correctly handles missing buckets and safely refills tokens under write lock.


111-124: LGTM!

Refill logic correctly calculates tokens based on elapsed time and caps at capacity.


126-129: LGTM!

Simple and correct key generation using ObjectID hex strings.


131-136: LGTM!

Struct design is appropriate for managing turn state per team.


138-144: LGTM!

Constructor follows standard Go patterns.


172-183: LGTM!

Proper read-only access with appropriate fallback to NilObjectID when data is missing.


185-220: LGTM!

Circular turn advancement logic is correct with proper concurrency control and defensive checks for missing or inconsistent data.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review continued from previous batch...

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

♻️ Duplicate comments (3)
backend/controllers/analytics_controller.go (1)

57-63: Include bot debates in today's debate count.

The debatesToday metric only counts regular debates, but totalDebates (line 55) includes bot debates, and GetAnalyticsHistory (lines 217-219) counts bot debates when generating daily snapshots. This inconsistency means the dashboard will show different debate counts for today depending on whether it reads from the live snapshot or historical data.

Apply this diff to count bot debates in the same window:

 // Get debates today
 debatesToday, ok := countDocuments(debatesCollection, bson.M{
   "date": bson.M{"$gte": todayStart},
 }, "debates today")
 if !ok {
   return
 }
+
+// Get bot debates today
+botDebatesToday, ok := countDocuments(botDebatesCollection, bson.M{
+  "createdAt": bson.M{"$gte": todayStart.Unix()},
+}, "bot debates today")
+if !ok {
+  return
+}
+debatesToday += botDebatesToday
backend/websocket/team_websocket.go (2)

788-821: Use snapshot pattern in goroutine to prevent concurrent map access.

The goroutine iterates room.Clients directly at line 809, which can cause a concurrent map iteration/write panic if a client disconnects while this goroutine is broadcasting.

 			room.Mutex.Lock()
 			if room.CurrentPhase == "countdown" || room.CurrentPhase == "setup" {
 				room.CurrentPhase = "openingFor"
 				
 				// Broadcast phase change to ALL clients using proper TeamMessage format
 				phaseMessage := TeamMessage{
 					Type:  "phaseChange",
 					Phase: "openingFor",
 				}
-				for _, r := range room.Clients {
+				recipients := snapshotAllTeamClients(room)
+				room.Mutex.Unlock()
+				
+				for _, r := range recipients {
 					if err := r.SafeWriteJSON(phaseMessage); err != nil {
 						log.Printf("Team WebSocket write error in room %s: %v", roomKey, err)
 					} else {
 						log.Printf("[handleTeamReadyStatus] ✓ Phase change message sent to client")
 					}
 				}
-				log.Printf("[handleTeamReadyStatus] Debate started! Phase changed to openingFor for %d clients", len(room.Clients))
+				log.Printf("[handleTeamReadyStatus] Debate started! Phase changed to openingFor for %d clients", len(recipients))
 			} else {
 				log.Printf("[handleTeamReadyStatus] Phase already changed to %s, skipping", room.CurrentPhase)
 			}
-			room.Mutex.Unlock()
 		}()

744-750: Use snapshot pattern to prevent concurrent map access.

This code iterates room.Clients directly while holding the lock and making blocking I/O calls (SafeWriteJSON), which can cause concurrent map iteration/write panics if another goroutine modifies the clients map. The pattern should match the corrected handleTeamJoin.

+	// Release lock before broadcasting
+	room.Mutex.Unlock()
+	
 	for _, r := range room.Clients {
+	for _, r := range snapshotAllTeamClients(room) {
 		if err := r.SafeWriteJSON(readyMessage); err != nil {
 			log.Printf("Team WebSocket write error in room %s: %v", roomKey, err)
 		} else {
 			log.Printf("[handleTeamReadyStatus] ✓ Ready message sent successfully")
 		}
 	}
 	
 	// Check if all teams are ready and phase is still setup
+	room.Mutex.Lock()
 	allTeam1Ready := currentTeam1ReadyCount == currentTeam1MembersCount && currentTeam1MembersCount > 0

Note: You'll need to re-acquire the lock after broadcasting to check the ready status.

🧹 Nitpick comments (22)
backend/controllers/analytics_controller.go (2)

131-135: Consider alerting on snapshot persistence failures.

The snapshot insertion error is only logged, allowing the request to succeed even when persistence fails. While this ensures that live analytics remain available, it can lead to silent gaps in historical data that may not be noticed until someone queries GetAnalyticsHistory.

Consider:

  • Incrementing a metric/counter for monitoring
  • Or including a warning flag in the response when persistence fails

225-225: Consider removing or downgrading the verbose debug log.

This log fires for every generated snapshot (potentially dozens if a user requests 30+ days). In production, this can clutter logs. Consider using a debug-level logger or removing this line once you've validated the generation logic.

frontend/src/Pages/Game.tsx (2)

112-151: LGTM! Solid typing/speaking indicator management.

The logic correctly maintains the typing indicators array by filtering and conditionally re-adding indicators. The defensive checks (lines 123-125) and error handling (lines 147-149) are well-implemented.

Optional: Consider consistent error handling across message handlers.

This handler includes try-catch error handling, but other message handlers (TURN_START, CHAT_MESSAGE, etc.) don't. For consistency and robustness, consider wrapping JSON.parse() calls in other handlers with similar error handling.

Example pattern:

case "CHAT_MESSAGE": {
  try {
    const { sender, message: chatMessage } = JSON.parse(message.content);
    // ... rest of handler
  } catch (error) {
    console.error("Failed to handle CHAT_MESSAGE:", error);
  }
  break;
}

191-208: LGTM! Smart optimization with real-time typing preview.

The logic correctly avoids redundant messages when the typing state hasn't changed, while still allowing partialText updates for real-time typing preview. The ref tracking prevents duplicate status broadcasts.

Optional: Consider debouncing partial text updates.

If partialText updates very frequently (e.g., on every keystroke), this could generate significant WebSocket traffic. Consider debouncing these updates (e.g., 100-200ms) to reduce network overhead while maintaining a good user experience.

Example:

const debouncedPartialTextUpdate = useMemo(
  () => debounce((text: string) => {
    sendWebSocketMessage({
      type: "TYPING_STATUS",
      content: JSON.stringify({ sender: userId, isTyping: true, partialText: text })
    });
  }, 150),
  [sendWebSocketMessage, userId]
);
frontend/src/Pages/TeamDebateRoom.tsx (2)

1550-1567: Duplicate camera toggle controls may confuse users.

The camera toggle button appears both inline within each video tile (lines 1550-1567) and as a separate button at the bottom of the team section (lines 1616-1647). This duplication might confuse users about which button controls what.

Consider keeping only one camera toggle location. The bottom button is more visible and consistently positioned, making it a better choice. Remove the inline button from the video tile to simplify the UI.

Also applies to: 1616-1647


362-362: Large dependency arrays may cause unnecessary re-renders.

Several effects have large dependency arrays that might trigger more frequently than needed:

  • Line 362: includes isUserLoading which changes during auth flow
  • Line 399: includes many debate state variables
  • Line 1170: includes all ready count and member count variables

Consider whether all dependencies are truly needed. For example:

  • Line 362: isUserLoading might not be necessary if you're already checking for currentUser?.id
  • Line 399: Consider using refs for values that don't need to trigger effect re-runs
  • Line 1170: Could potentially use a derived value instead of listing all ready counts

Also applies to: 1170-1170, 1206-1206

frontend/src/Pages/TeamBuilder.tsx (11)

45-82: Import types from teamService instead of duplicating.

The TeamMember, Team, and related interfaces are duplicated from frontend/src/services/teamService.ts. This creates a maintenance burden—changes to the service types won't automatically propagate here.

Apply this diff to import and reuse the types:

+import {
+  Team,
+  TeamMember,
+  createTeam,
-import {
-  createTeam,
   getAvailableTeams,
   ...
-
-interface TeamMember {
-  userId: string;
-  email: string;
-  displayName: string;
-  elo: number;
-  joinedAt?: string;
-}
-
-interface Team {
-  id: string;
-  name: string;
-  code: string;
-  captainId: string;
-  captainEmail: string;
-  members: TeamMember[];
-  maxSize: number;
-  averageElo: number;
-  createdAt: string;
-  updatedAt: string;
-}

106-125: Provide user feedback when data fetching fails.

Both fetchAvailableTeams and fetchUserTeams silently catch errors and set empty arrays. Users can't distinguish between "no teams exist" and "fetch failed." Consider setting an error state to display a retry option or error message.

Example approach:

 const fetchAvailableTeams = useCallback(async () => {
   try {
     const teams = await getAvailableTeams();
     setAvailableTeams(teams || []);
   } catch (error) {
     console.error("Failed to fetch available teams:", error);
+    setError("Failed to load available teams. Please refresh.");
     setAvailableTeams([]);
   }
 }, []);

132-163: Add validation for team name length and characters.

The team name validation only checks if it's non-empty. Consider adding constraints for maximum length and allowed characters to prevent issues with display, storage, or potential injection attacks.

 const handleCreateTeam = async () => {
   if (!teamName.trim()) {
     setError("Please enter a team name");
     return;
   }
+
+  if (teamName.trim().length > 50) {
+    setError("Team name must be 50 characters or less");
+    return;
+  }

240-255: Document or normalize ID format inconsistencies.

The backwards‑compatibility logic for $oid suggests mixed ID formats (MongoDB ObjectId objects vs. strings) from the API. This is brittle and adds cognitive overhead. Consider normalizing IDs at the API boundary (in teamService.ts) or documenting why this is necessary.


206-206: Replace native confirm with custom dialog component.

The blocking confirm() call doesn't match your app's styling and offers limited accessibility. You're already using Radix UI Dialog elsewhere; consider a reusable confirmation dialog component.

Same issue exists at:

  • Line 221 (delete team)
  • Line 303 (leave team)

624-633: Allow captains to increase team size when members exist.

The condition team.members.length === 0 prevents any size editing when there are members, even to increase capacity. Lines 593 and 605 already disable shrinking below the current member count, so you can safely allow editing when increasing.

-                            {isCaptain(team) && team.members.length === 0 && (
+                            {isCaptain(team) && (
                               <button

532-539: Add aria-label for better accessibility.

The title attribute is not reliably announced by screen readers. Use aria-label for icon-only buttons.

 <button
   onClick={() => handleEditTeamName(team)}
   className="p-1 hover:bg-gray-100 rounded transition-colors"
-  title="Edit team name"
+  aria-label="Edit team name"
 >

Same issue exists at:

  • Lines 555-562 (copy team code button)
  • Lines 625-632 (edit team size button)

318-326: Validate team code format before API call.

The placeholder mentions a "6-character team code" and the input has maxLength={6}, but handleJoinByCode doesn't enforce this. Users could submit shorter codes and receive API errors.

 const handleJoinByCode = async (): Promise<void> => {
   if (!searchCode.trim()) {
     setError("Please enter a team code");
     return;
   }
+
+  if (searchCode.trim().length !== 6) {
+    setError("Team code must be exactly 6 characters");
+    return;
+  }

156-156: Extract message timeout pattern and add cleanup.

The setTimeout pattern for clearing messages repeats throughout the component (lines 156, 171, 175, 213, 216, 233, 236, 273, 276, 289, 292, 299, 311, 314, 332, 335). Consider a custom hook like useTimedMessage() for DRYness. Also, these lack cleanup—if the component unmounts before the timeout fires, you may get warnings or memory leaks.

Example custom hook:

const useTimedMessage = () => {
  const [message, setMessage] = useState("");
  const timeoutRef = useRef<NodeJS.Timeout>();

  const showMessage = useCallback((msg: string, duration = 3000) => {
    setMessage(msg);
    if (timeoutRef.current) clearTimeout(timeoutRef.current);
    timeoutRef.current = setTimeout(() => setMessage(""), duration);
  }, []);

  useEffect(() => {
    return () => {
      if (timeoutRef.current) clearTimeout(timeoutRef.current);
    };
  }, []);

  return { message, showMessage };
};

762-792: Simplify nested captain‑action conditionals.

The deeply nested checks (lines 762–792) to determine whether to show the "Remove Member" button are hard to follow. Consider extracting this logic into a computed boolean or separate component.

Example:

const showRemoveButton = useMemo(() => {
  if (!selectedMember?.teamId) return false;
  const team = userTeams?.find(t => t.id === selectedMember.teamId);
  if (!team || !isCaptain(team)) return false;
  return selectedMember.memberUserId !== team.captainId;
}, [selectedMember, userTeams]);

Then in JSX:

{showRemoveButton && (
  <Button variant="destructive" className="w-full" onClick={...}>
    <FaTimes className="mr-2" /> Remove Member from Team
  </Button>
)}

817-916: Consider extracting AvailableTeamCard component.

The available‑teams rendering (lines 817–916) is ~100 lines and could be extracted into a separate AvailableTeamCard component. This would improve readability, testability, and reusability.

backend/websocket/team_websocket.go (5)

115-115: Avoid hardcoded configuration paths.

The configuration file path is hardcoded, which reduces flexibility across different deployment environments.

Consider passing the config path as a parameter, environment variable, or using a global configuration service:

-	valid, email, err := utils.ValidateTokenAndFetchEmail("./config/config.prod.yml", token, c)
+	valid, email, err := utils.ValidateTokenAndFetchEmail(os.Getenv("CONFIG_PATH"), token, c)

186-217: Reduce critical section by initializing services outside the global lock.

Service initialization (lines 190-199) happens while holding teamRoomsMutex, potentially blocking other connections from creating/accessing rooms.

 	teamRoomsMutex.Lock()
 	roomKey := debateID
 	if _, exists := teamRooms[roomKey]; !exists {
+		teamRoomsMutex.Unlock()
+		
 		turnManager := services.NewTeamTurnManager()
 		tokenBucket := services.NewTokenBucketService()
 
 		// Initialize turn management for both teams
 		turnManager.InitializeTeamTurns(debate.Team1ID)
 		turnManager.InitializeTeamTurns(debate.Team2ID)
 
 		// Initialize token buckets for both teams
 		tokenBucket.InitializeTeamBuckets(debate.Team1ID)
 		tokenBucket.InitializeTeamBuckets(debate.Team2ID)
 
+		teamRoomsMutex.Lock()
+		// Double-check pattern: another goroutine might have created it
+		if _, exists := teamRooms[roomKey]; !exists {
-		teamRooms[roomKey] = &TeamRoom{
-			Clients:     make(map[*websocket.Conn]*TeamClient),
-			Team1ID:     debate.Team1ID,
-			Team2ID:     debate.Team2ID,
-			DebateID:    debateObjectID,
-			TurnManager: turnManager,
-			TokenBucket: tokenBucket,
-			CurrentTopic: debate.Topic,
-			CurrentPhase: "setup",
-			Team1Role:   debate.Team1Stance,
-			Team2Role:   debate.Team2Stance,
-			Team1Ready:  make(map[string]bool),
-			Team2Ready:  make(map[string]bool),
-		}
+			teamRooms[roomKey] = &TeamRoom{
+				Clients:     make(map[*websocket.Conn]*TeamClient),
+				Team1ID:     debate.Team1ID,
+				Team2ID:     debate.Team2ID,
+				DebateID:    debateObjectID,
+				TurnManager: turnManager,
+				TokenBucket: tokenBucket,
+				CurrentTopic: debate.Topic,
+				CurrentPhase: "setup",
+				Team1Role:   debate.Team1Stance,
+				Team2Role:   debate.Team2Stance,
+				Team1Ready:  make(map[string]bool),
+				Team2Ready:  make(map[string]bool),
+			}
+		}
 	}
 	room := teamRooms[roomKey]
 	teamRoomsMutex.Unlock()

226-236: Validate team assignment before WebSocket upgrade.

The team assignment validation occurs after the WebSocket upgrade (line 220), which means the connection is established even if validation fails. While the connection is closed on error (line 234), it's more efficient to validate before upgrading.

Consider moving this validation block before line 220 to avoid unnecessary WebSocket upgrades for invalid requests. However, you'd need to refactor to avoid using conn.Close() and instead use c.JSON() for the error response.


338-342: Log JSON unmarshaling errors for debugging.

Unmarshaling errors are silently ignored, which can make it difficult to diagnose client-side issues sending malformed messages.

 		var message TeamMessage
 		if err := json.Unmarshal(msg, &message); err != nil {
+			log.Printf("Team WebSocket message unmarshal error in room %s: %v", roomKey, err)
 			continue
 		}

636-827: Consider breaking up the long critical section.

The handleTeamReadyStatus function holds the room lock for an extended period (~190 lines), which includes counting operations, message construction, and broadcasting. This can increase lock contention and reduce concurrency.

Consider restructuring to:

  1. Lock → read/update state → unlock
  2. Perform broadcasts outside the lock using snapshots
  3. Re-lock only when needed for subsequent state checks

This would reduce the time the lock is held and improve concurrent access.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6c9faea and 0a6dea9.

📒 Files selected for processing (7)
  • backend/controllers/analytics_controller.go (1 hunks)
  • backend/websocket/team_websocket.go (1 hunks)
  • frontend/src/Pages/DebateRoom.tsx (1 hunks)
  • frontend/src/Pages/Game.tsx (8 hunks)
  • frontend/src/Pages/MatchLogs.tsx (4 hunks)
  • frontend/src/Pages/TeamBuilder.tsx (1 hunks)
  • frontend/src/Pages/TeamDebateRoom.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • frontend/src/Pages/MatchLogs.tsx
  • frontend/src/Pages/DebateRoom.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
frontend/src/Pages/Game.tsx (2)
backend/websocket/websocket.go (1)
  • TypingIndicator (88-94)
frontend/src/components/Chatbox.tsx (1)
  • TypingIndicator (13-19)
backend/websocket/team_websocket.go (4)
backend/services/team_turn_service.go (5)
  • TeamTurnManager (132-136)
  • TokenBucket (22-28)
  • TokenBucketService (16-19)
  • NewTeamTurnManager (139-144)
  • NewTokenBucketService (31-35)
backend/utils/auth.go (1)
  • ValidateTokenAndFetchEmail (131-144)
backend/db/db.go (1)
  • MongoDatabase (17-17)
backend/models/team.go (2)
  • TeamDebate (61-81)
  • Team (11-22)
backend/controllers/analytics_controller.go (3)
backend/db/db.go (1)
  • MongoDatabase (17-17)
backend/models/admin.go (2)
  • AnalyticsSnapshot (53-63)
  • AdminActionLog (38-50)
frontend/src/services/adminService.ts (2)
  • AnalyticsSnapshot (50-60)
  • AdminActionLog (62-74)
frontend/src/Pages/TeamBuilder.tsx (3)
backend/models/team.go (2)
  • TeamMember (25-32)
  • Team (11-22)
frontend/src/services/teamService.ts (13)
  • TeamMember (17-24)
  • Team (4-15)
  • getAvailableTeams (142-155)
  • getUserTeams (126-139)
  • createTeam (61-77)
  • joinTeam (96-108)
  • getTeamMemberProfile (308-321)
  • removeMember (276-289)
  • deleteTeam (292-305)
  • updateTeamName (324-339)
  • updateTeamSize (358-373)
  • leaveTeam (111-123)
  • getTeamByCode (342-355)
frontend/src/state/userAtom.ts (1)
  • userAtom (4-4)
frontend/src/Pages/TeamDebateRoom.tsx (6)
backend/models/team.go (1)
  • TeamMember (25-32)
frontend/src/services/teamService.ts (2)
  • TeamMember (17-24)
  • getTeamDebate (179-192)
frontend/src/state/userAtom.ts (1)
  • userAtom (4-4)
frontend/src/hooks/useUser.ts (1)
  • useUser (7-60)
frontend/src/utils/auth.ts (1)
  • getAuthToken (5-7)
frontend/src/services/teamDebateService.ts (1)
  • getTeamDebate (55-68)
🪛 Biome (2.1.2)
frontend/src/Pages/TeamDebateRoom.tsx

[error] 519-520: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 520-521: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 599-600: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 623-624: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 624-624: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 625-626: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 656-657: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 657-659: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 677-677: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 677-678: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 678-679: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 679-680: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 680-682: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 700-701: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

🔇 Additional comments (23)
backend/controllers/analytics_controller.go (2)

262-314: LGTM! Pagination properly implemented.

The function now correctly parses page and limit query parameters with sensible defaults and validation, addressing the previous review feedback. The 200-record cap prevents abuse, and error handling is thorough throughout.


24-36: LGTM! Clean error-handling helper.

The countDocuments helper consolidates error handling and makes GetAnalytics more maintainable. This addresses the previous review feedback about masked CountDocuments failures.

frontend/src/Pages/Game.tsx (8)

1-1: LGTM! Clean imports for enhanced functionality.

The addition of useCallback and TypingIndicator imports properly supports the new typing/speaking status features and performance optimizations implemented throughout the file.

Also applies to: 5-5


35-36: LGTM! Smart optimization to reduce redundant WebSocket traffic.

These refs effectively prevent sending duplicate typing/speaking status messages when the state hasn't changed, reducing unnecessary network overhead.


38-45: LGTM! Robust WebSocket message helper.

The defensive check for WebSocket.OPEN with warning logs provides good protection against attempting to send messages when the connection isn't ready, and the use of useCallback ensures optimal performance.


80-84: LGTM! Logical typing indicator cleanup.

Removing the sender's typing indicator when their message arrives is the correct behavior and provides a clean user experience.


96-111: LGTM! Past review comment addressed correctly.

The change from loose equality (==) to strict equality (===) on line 104 properly addresses the previous review concern about type coercion issues. This ensures the winner comparison works reliably regardless of whether the IDs are strings or numbers.


172-189: LGTM! Well-structured message handler.

The implementation includes proper input validation (trim/empty check), correct dependencies, and a clean message structure. The inclusion of the mode parameter supports different input methods (typing vs speaking).


210-226: LGTM! Clean speaking status handler.

The implementation follows the same optimization pattern as handleTypingChange, preventing redundant status broadcasts while maintaining accurate state.


278-290: LGTM! Past review comment fully addressed.

The Chatbox props are now properly connected to real functionality, addressing all concerns from the previous review:

  • onSendMessagehandleSendChatMessage (was no-op)
  • isMyTurnstate.isTurn (was hardcoded true)
  • disabledstate.gameEnded || state.loading (was hardcoded true)
  • onTypingChange and onSpeakingChange → proper handlers (were no-ops)
  • typingIndicatorsstate.typingIndicators (was empty array)

Additionally, the setCameraOn and setMicOn handlers now support both direct values and functional updates, following React best practices.

Also applies to: 297-307

frontend/src/Pages/TeamDebateRoom.tsx (6)

762-770: Empty WebRTC handlers may indicate incomplete implementation.

The offer, answer, and candidate cases are empty placeholders. If WebRTC peer connections are not being used yet, consider adding a comment explaining the roadmap. If they should be implemented, this is a functional gap.

Are WebRTC peer connections planned for team debates? If so, these handlers need implementation to exchange media streams between peers.


953-971: toggleCamera implemented correctly.

The toggleCamera handler has been properly implemented, addressing the previous review comment. It correctly toggles the video track's enabled state using functional setState.


403-410: WebSocket initialization handles auth correctly.

Good defensive programming: the effect waits for both token and debateId before connecting, and uses refs in the dependency array to avoid reconnecting on every state change. This addresses the previous review comment about WebSocket stability.


441-454: Phase reset protection is well-implemented.

The stateSync handler properly prevents phase from resetting back to Setup after the debate has started (lines 445-447). This guard rail prevents the setup popup from reopening mid-debate.


1135-1143: Debate started guard prevents setup popup from reopening.

Excellent use of debateStartedRef to permanently close the setup popup once the debate begins. This ensures users aren't interrupted by the setup UI during active debate phases.


194-212: Refs properly mirror reactive state for WebSocket handlers.

Good solution to avoid WebSocket reconnection issues. The refs (isTeam1Ref, myTeamIdRef, debatePhaseRef, currentUserIdRef) are kept in sync with their state counterparts via useEffect, allowing the WebSocket handler to access fresh values without being in the dependency array.

frontend/src/Pages/TeamBuilder.tsx (1)

818-821: Past capacity issue appears resolved.

The previous review flagged a hard-coded capacity = 4, but line 820 now correctly uses team.maxSize || 4. This properly respects each team's configured size.

backend/websocket/team_websocket.go (6)

22-38: Well-structured room state with appropriate synchronization.

The TeamRoom struct properly includes a mutex for protecting concurrent access to the Clients map and room state fields. The ready status maps (Team1Ready, Team2Ready) are correctly protected by the room's mutex throughout the code.


390-412: Excellent use of snapshot pattern for thread-safe iteration.

These helper functions properly implement the snapshot pattern to prevent concurrent map iteration and write panics. They correctly hold the room mutex while building the snapshot, then release it before returning.


414-430: Past review feedback properly addressed.

The concurrent map iteration issue from the previous review has been correctly fixed. The code now uses snapshotAllTeamClients(room) to safely iterate over clients without holding the mutex during the broadcast loop.


432-547: Well-implemented message handlers with proper concurrency control.

All message handlers correctly use snapshot functions for iteration and properly protect shared state updates with the room mutex. The timestamp addition and state resets are handled appropriately.


594-634: Correct role selection handling with proper team identification.

The handler correctly uses hex string comparison for team ID matching, updates the appropriate team's role, and safely broadcasts using the snapshot pattern after releasing the lock.


829-898: Turn management handlers correctly implement snapshot pattern.

Both handleTeamTurnRequest and handleTeamTurnEnd properly use snapshotAllTeamClients for safe iteration during broadcasts.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (4)
backend/controllers/analytics_controller.go (1)

68-74: Include bot debates in today's count for consistency.

The debatesToday metric only counts from the debates collection, while totalDebates (line 66) includes bot debates. This creates an inconsistency with the history endpoint (lines 214-236), which includes both debate types per day. Please add bot debates to the today count.

Apply this diff to include bot debates:

 // Get debates today
 debatesToday, ok := countDocuments(debatesCollection, bson.M{
 	"date": bson.M{"$gte": todayStart},
 }, "debates today")
 if !ok {
 	return
 }
+
+// Get bot debates today
+botDebatesToday, ok := countDocuments(botDebatesCollection, bson.M{
+	"createdAt": bson.M{"$gte": todayStart.Unix()},
+}, "bot debates today")
+if !ok {
+	return
+}
+debatesToday += botDebatesToday
frontend/src/Pages/Admin/AdminDashboard.tsx (1)

432-455: Fix bulk delete for mixed comment types.

The handleBulkDeleteComments function only uses the first selected comment type (line 446), causing silent failures when admins select comments of different types (e.g., mixing team_debate_message and team_chat_message). Comments not matching the first type won't be deleted.

Apply this diff to group by type and call the API for each group:

 const handleBulkDeleteComments = async () => {
   if (!token || selectedComments.size === 0) return;
   if (
     !confirm(
       `Are you sure you want to delete ${selectedComments.size} comments?`
     )
   )
     return;

   try {
-    // Get types for selected comments
-    const commentTypes = comments
-      .filter((c) => selectedComments.has(c.id))
-      .map((c) => c.type);
-    const type = commentTypes[0] || "team_debate_message";
-
-    await bulkDeleteComments(token, Array.from(selectedComments), type);
+    // Group selected comments by type
+    const groupedByType = new Map<string, string[]>();
+    comments
+      .filter((c) => selectedComments.has(c.id))
+      .forEach((c) => {
+        if (!groupedByType.has(c.type)) {
+          groupedByType.set(c.type, []);
+        }
+        groupedByType.get(c.type)!.push(c.id);
+      });
+
+    // Delete each group
+    await Promise.all(
+      Array.from(groupedByType.entries()).map(([type, ids]) =>
+        bulkDeleteComments(token, ids, type)
+      )
+    );
+
     setSelectedComments(new Set());
     loadData(token);
   } catch (err) {
     console.error("Failed to delete comments", err);
     alert("Failed to delete comments");
   }
 };
backend/controllers/admin_controller.go (2)

218-226: Return bot debates or remove the unused query.

The code queries debates_vs_bot (lines 218-226) but never merges those results into the response. The total count (line 230) only reflects human debates, and admins can't moderate bot debates. Either include bot debates in the payload with a consistent view model, or remove the dead query.

To include bot debates, apply this diff:

 // Also get debates from debates_vs_bot collection
 botCollection := db.MongoDatabase.Collection("debates_vs_bot")
-botCursor, err := botCollection.Find(dbCtx, bson.M{}, options.Find().SetSkip(int64(skip)).SetLimit(int64(limit)).SetSort(bson.M{"createdAt": -1}))
-if err == nil {
-	defer botCursor.Close(dbCtx)
-	var botDebates []models.DebateVsBot
-	botCursor.All(dbCtx, &botDebates)
-	// Merge results if needed
-}
+botTotal, err := botCollection.CountDocuments(dbCtx, bson.M{})
+if err != nil {
+	ctx.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to count bot debates", "message": err.Error()})
+	return
+}
+total += botTotal

 ctx.JSON(http.StatusOK, gin.H{
 	"debates": debates,
 	"total":   total,
 	"page":    page,
 	"limit":   limit,
 })

Note: If you want to return bot debates in the list, you'll need to convert them to a common format or return them separately.


335-410: Fix comment pagination and total count.

Applying skip/limit to each collection separately (lines 356 and 380) breaks pagination—page 1 can return up to 40 items, and total (line 402) becomes "items on this page" rather than the real count. This breaks the frontend pagination contract.

To fix this, aggregate both collections with a single pipeline:

 func GetComments(ctx *gin.Context) {
 	page := 1
 	limit := 20
 
 	if pageStr := ctx.Query("page"); pageStr != "" {
 		fmt.Sscanf(pageStr, "%d", &page)
 	}
 	if limitStr := ctx.Query("limit"); limitStr != "" {
 		fmt.Sscanf(limitStr, "%d", &limit)
 	}
 
 	skip := (page - 1) * limit
 
 	dbCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	var comments []models.Comment
-
-	// Get team debate messages
-	teamDebateCollection := db.MongoDatabase.Collection("team_debate_messages")
-	cursor1, err := teamDebateCollection.Find(dbCtx, bson.M{}, options.Find().SetSkip(int64(skip)).SetLimit(int64(limit)).SetSort(bson.M{"timestamp": -1}))
-	if err == nil {
-		defer cursor1.Close(dbCtx)
-		var messages []models.TeamDebateMessage
-		if err := cursor1.All(dbCtx, &messages); err == nil {
-			for _, msg := range messages {
-				comments = append(comments, models.Comment{
-					ID:          msg.ID,
-					Type:        "team_debate_message",
-					Content:     msg.Message,
-					UserID:      msg.UserID,
-					UserEmail:   msg.Email,
-					DisplayName: msg.DisplayName,
-					DebateID:    msg.DebateID,
-					TeamID:      msg.TeamID,
-					CreatedAt:   msg.Timestamp,
-					IsDeleted:   false,
-				})
-			}
-		}
-	}
-
-	// Get team chat messages
-	teamChatCollection := db.MongoDatabase.Collection("team_chat_messages")
-	cursor2, err := teamChatCollection.Find(dbCtx, bson.M{}, options.Find().SetSkip(int64(skip)).SetLimit(int64(limit)).SetSort(bson.M{"timestamp": -1}))
-	if err == nil {
-		defer cursor2.Close(dbCtx)
-		var chatMessages []models.TeamChatMessage
-		if err := cursor2.All(dbCtx, &chatMessages); err == nil {
-			for _, msg := range chatMessages {
-				comments = append(comments, models.Comment{
-					ID:          msg.ID,
-					Type:        "team_chat_message",
-					Content:     msg.Message,
-					UserID:      msg.UserID,
-					UserEmail:   msg.Email,
-					DisplayName: msg.DisplayName,
-					TeamID:      msg.TeamID,
-					CreatedAt:   msg.Timestamp,
-					IsDeleted:   false,
-				})
-			}
-		}
-	}
-
-	// Get total count (simplified)
-	total := int64(len(comments))
+	// Use aggregation to union both collections
+	teamDebateCollection := db.MongoDatabase.Collection("team_debate_messages")
+	pipeline := mongo.Pipeline{
+		{{Key: "$unionWith", Value: bson.M{
+			"coll": "team_chat_messages",
+		}}},
+		{{Key: "$sort", Value: bson.M{"timestamp": -1}}},
+		{{Key: "$facet", Value: bson.M{
+			"metadata": bson.A{
+				bson.M{"$count": "total"},
+			},
+			"data": bson.A{
+				bson.M{"$skip": skip},
+				bson.M{"$limit": limit},
+			},
+		}}},
+	}
+
+	cursor, err := teamDebateCollection.Aggregate(dbCtx, pipeline)
+	if err != nil {
+		ctx.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to fetch comments", "message": err.Error()})
+		return
+	}
+	defer cursor.Close(dbCtx)
+
+	var results []bson.M
+	if err := cursor.All(dbCtx, &results); err != nil {
+		ctx.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to decode comments", "message": err.Error()})
+		return
+	}
+
+	var comments []models.Comment
+	var total int64
+	if len(results) > 0 {
+		if metadata, ok := results[0]["metadata"].(bson.A); ok && len(metadata) > 0 {
+			if metaDoc, ok := metadata[0].(bson.M); ok {
+				if count, ok := metaDoc["total"].(int32); ok {
+					total = int64(count)
+				}
+			}
+		}
+		if data, ok := results[0]["data"].(bson.A); ok {
+			for _, item := range data {
+				if doc, ok := item.(bson.M); ok {
+					// Map to Comment model based on collection source
+					// You'll need to add type detection logic here
+				}
+			}
+		}
+	}
 
 	ctx.JSON(http.StatusOK, gin.H{
 		"comments": comments,
 		"total":    total,
 		"page":     page,
 		"limit":    limit,
 	})
 }
🧹 Nitpick comments (3)
backend/services/rating_service.go (1)

63-86: Consider extracting the inline closure for testability.

The sanitizePlayerMetrics closure is defined inline and handles both NaN/Inf sanitization and non-positive validation. Extracting it to a package-level function would improve testability and reusability.

Apply this diff to extract the function:

+func sanitizePlayerMetrics(player *rating.Player, config *rating.Config) {
+	if math.IsNaN(player.Rating) || math.IsInf(player.Rating, 0) {
+		player.Rating = config.InitialRating
+	}
+	if math.IsNaN(player.RD) || math.IsInf(player.RD, 0) {
+		player.RD = config.InitialRD
+	}
+	if math.IsNaN(player.Volatility) || math.IsInf(player.Volatility, 0) {
+		player.Volatility = config.InitialVol
+	}
+	if player.RD <= 0 {
+		player.RD = config.InitialRD
+	}
+	if player.Volatility <= 0 {
+		player.Volatility = config.InitialVol
+	}
+}
+
 func UpdateRatings(userID, opponentID primitive.ObjectID, outcome float64, debateTime time.Time) (*models.Debate, error) {
 	// ...
 	ratingSystem.UpdateMatch(userPlayer, opponentPlayer, outcome, debateTime)
 
 	// Sanitize rating outputs to avoid NaN/Inf values
-	sanitizePlayerMetrics := func(player *rating.Player) {
-		initialRating := ratingSystem.Config.InitialRating
-		initialRD := ratingSystem.Config.InitialRD
-		initialVol := ratingSystem.Config.InitialVol
-
-		if math.IsNaN(player.Rating) || math.IsInf(player.Rating, 0) {
-			player.Rating = initialRating
-		}
-		if math.IsNaN(player.RD) || math.IsInf(player.RD, 0) {
-			player.RD = initialRD
-		}
-		if math.IsNaN(player.Volatility) || math.IsInf(player.Volatility, 0) {
-			player.Volatility = initialVol
-		}
-		if player.RD <= 0 {
-			player.RD = initialRD
-		}
-		if player.Volatility <= 0 {
-			player.Volatility = initialVol
-		}
-	}
-	sanitizePlayerMetrics(userPlayer)
-	sanitizePlayerMetrics(opponentPlayer)
+	sanitizePlayerMetrics(userPlayer, &ratingSystem.Config)
+	sanitizePlayerMetrics(opponentPlayer, &ratingSystem.Config)
frontend/src/Pages/Admin/AdminDashboard.tsx (1)

615-661: Consider extracting duplicate chart formatting logic.

The XAxis tickFormatter logic (lines 627-636) is duplicated across all three charts (debates, comments, users at lines 675-684 and 718-727). Consider extracting this to a reusable helper function.

Add this helper function before the component:

const createTickFormatter = (history: FormattedSnapshot[]) => 
  (value: string, index: number) => {
    if (history.length <= 7) return value;
    if (index % 3 === 0 || index === history.length - 1) return value;
    return "";
  };

Then use it in each chart:

-tickFormatter={(value, index) => {
-  // Show every 3rd date or last date to avoid overcrowding
-  if (analyticsHistory.length <= 7) return value; // Show all if 7 days or less
-  if (
-    index % 3 === 0 ||
-    index === analyticsHistory.length - 1
-  )
-    return value;
-  return "";
-}}
+tickFormatter={createTickFormatter(analyticsHistory)}
backend/controllers/admin_controller.go (1)

236-241: Consider centralizing float sanitization logic.

The sanitizeFloat64 function is duplicated in both rating_service.go (as sanitizeFloatMetric) and here. Consider moving this to a shared utility package to reduce duplication.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0a6dea9 and ed4ede5.

📒 Files selected for processing (4)
  • backend/controllers/admin_controller.go (1 hunks)
  • backend/controllers/analytics_controller.go (1 hunks)
  • backend/services/rating_service.go (4 hunks)
  • frontend/src/Pages/Admin/AdminDashboard.tsx (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
backend/controllers/admin_controller.go (6)
backend/models/admin.go (2)
  • Admin (10-18)
  • Comment (22-35)
frontend/src/services/adminService.ts (3)
  • Admin (3-8)
  • Debate (16-24)
  • Comment (26-37)
backend/db/db.go (1)
  • MongoDatabase (17-17)
backend/models/debatevsbot.go (1)
  • DebateVsBot (20-31)
backend/middlewares/rbac.go (1)
  • LogAdminAction (239-283)
backend/models/team.go (1)
  • TeamChatMessage (98-106)
frontend/src/Pages/Admin/AdminDashboard.tsx (2)
backend/models/admin.go (4)
  • AnalyticsSnapshot (53-63)
  • Admin (10-18)
  • Comment (22-35)
  • AdminActionLog (38-50)
frontend/src/services/adminService.ts (14)
  • AnalyticsSnapshot (50-60)
  • Analytics (39-48)
  • Admin (3-8)
  • Debate (16-24)
  • Comment (26-37)
  • AdminActionLog (62-74)
  • getAnalytics (97-108)
  • getAnalyticsHistory (110-127)
  • getDebates (130-148)
  • getComments (183-201)
  • deleteDebate (150-163)
  • deleteComment (203-220)
  • bulkDeleteDebates (165-180)
  • bulkDeleteComments (222-238)
backend/services/rating_service.go (2)
backend/rating/glicko2.go (2)
  • Player (21-26)
  • Config (29-36)
backend/models/debate.go (1)
  • Debate (10-25)
backend/controllers/analytics_controller.go (3)
backend/db/db.go (1)
  • MongoDatabase (17-17)
backend/models/admin.go (2)
  • AnalyticsSnapshot (53-63)
  • AdminActionLog (38-50)
frontend/src/services/adminService.ts (2)
  • AnalyticsSnapshot (50-60)
  • AdminActionLog (62-74)
🔇 Additional comments (11)
backend/services/rating_service.go (2)

89-101: LGTM!

The debate record construction correctly uses sanitized player metrics and applies sanitizeFloatMetric to the rating deltas, ensuring all numeric fields are safe for JSON serialization.


116-121: LGTM!

The sanitizeFloatMetric helper correctly clamps NaN/Inf values to 0, which is appropriate for rating deltas.

backend/controllers/analytics_controller.go (3)

24-28: LGTM!

The countDocumentsWithTimeout helper provides consistent timeout handling for count queries, improving error handling across the analytics endpoints.


160-287: LGTM!

The GetAnalyticsHistory endpoint properly handles errors with countDocumentsWithTimeout, includes both debates and bot debates in daily counts, and generates complete historical snapshots.


289-341: LGTM!

The GetAdminActionLogs endpoint correctly parses pagination parameters with a reasonable max limit cap, and handles errors consistently.

frontend/src/Pages/Admin/AdminDashboard.tsx (3)

57-169: Verify manual analytics flag before production.

The USE_MANUAL_ANALYTICS flag is set to true with hardcoded test data. Ensure this is set to false before deploying to production, or consider removing the manual analytics code entirely if it's only for development.


247-364: LGTM with field normalization note.

The loadData function properly uses Promise.allSettled for resilient parallel requests and handles both manual and real analytics modes. The field name normalization (lines 319-327) suggests the backend may return inconsistent casing—consider standardizing the backend response format.


760-837: LGTM!

The debates management table properly implements role-based access control, restricting delete operations to admin users only.

backend/controllers/admin_controller.go (3)

36-111: LGTM!

The AdminSignup endpoint properly validates role, checks for existing admins, hashes passwords with bcrypt, and generates JWTs securely.


113-165: LGTM!

The AdminLogin endpoint properly authenticates admins with bcrypt password verification and returns a JWT token.


412-512: LGTM!

The DeleteComment and BulkDeleteComments endpoints properly validate comment types, handle errors, and log admin actions for audit trails.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/Pages/TeamDebateRoom.tsx (1)

187-195: Remove duplicate toggleCamera declaration.

toggleCamera is declared twice in this component—once here (lines 187-195) and again at lines 919-937. The second declaration is more complete with better error handling. This redeclaration will cause a runtime error.

Remove this first declaration:

-  const toggleCamera = useCallback(() => {
-    const stream = localStreamRef.current;
-    if (stream) {
-      stream.getVideoTracks().forEach((track) => {
-        track.enabled = !track.enabled;
-      });
-    }
-    setIsCameraOn((prev) => !prev);
-  }, []);

Keep only the more complete implementation at lines 919-937.

♻️ Duplicate comments (6)
frontend/src/Pages/TeamDebateRoom.tsx (3)

417-417: Hardcoded localhost WebSocket URL will break in production.

This issue was flagged in a previous review but remains unaddressed. The WebSocket URL is still hardcoded to ws://localhost:1313, which will fail when deployed.

Apply the previously suggested fix:

+    const wsProtocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:';
+    const wsHost = process.env.REACT_APP_WS_URL || window.location.host;
     const ws = new WebSocket(
-      `ws://localhost:1313/ws/team?debateId=${debateId}&token=${token}`
+      `${wsProtocol}//${wsHost}/ws/team?debateId=${debateId}&token=${token}`
     );

1012-1012: Hardcoded localhost API URL will break in production.

This issue was flagged in a previous review but remains unaddressed. The API endpoint is still hardcoded to http://localhost:1313, which will fail when deployed.

Consider creating a shared API configuration as previously suggested, or at minimum use an environment variable:

+      const apiBaseUrl = process.env.REACT_APP_API_URL || 'http://localhost:1313';
       const token = getAuthToken();
-      const response = await fetch(`http://localhost:1313/submit-transcripts`, {
+      const response = await fetch(`${apiBaseUrl}/submit-transcripts`, {
         method: "POST",

513-516: Wrap switch case variable declarations in blocks.

Multiple switch cases declare variables without wrapping them in blocks, allowing them to leak into other cases. This is flagged by the linter and can cause subtle bugs.

Apply this pattern to all affected cases:

         case "stateSync":
+          {
           // ... existing code with variable declarations
           const opponentReadyCount = ...
           const opponentMemberCount = ...
           // ... rest of case
           break;
+          }
         case "ready":
+          {
           const messageTeamId = data.teamId;
           const expectedTeamId = ...
           // ... rest of case
           break;
+          }

Repeat for all cases with variable declarations at lines 513-516, 611-612, and 658-669.

Also applies to: 611-612, 658-669

backend/websocket/team_websocket.go (3)

705-707: Apply snapshot pattern to both broadcasts in handleTeamReadyStatus.

Direct iteration over room.Clients while holding the mutex should be replaced with the snapshot pattern. Holding the lock during SafeWriteJSON calls blocks other operations unnecessarily and can cause performance issues.

Apply this diff to fix both broadcast loops:

 	// Broadcast ready status with accurate counts to ALL clients
 	readyMessage := map[string]interface{}{
 		"type":              "ready",
 		"ready":             message.Ready,
 		"userId":            userID,
 		"teamId":            clientTeamIDHex,
 		"assignedToTeam":    assignedToTeam,
 		"team1Ready":        currentTeam1ReadyCount,
 		"team2Ready":        currentTeam2ReadyCount,
 		"team1MembersCount": currentTeam1MembersCount,
 		"team2MembersCount": currentTeam2MembersCount,
 	}
 
-	for _, r := range room.Clients {
+	recipients := snapshotAllTeamClients(room)
+	room.Mutex.Unlock()
+	
+	for _, r := range recipients {
 		_ = r.SafeWriteJSON(readyMessage)
 	}
 
 	// Check if all teams are ready and phase is still setup
+	room.Mutex.Lock()
 	allTeam1Ready := currentTeam1ReadyCount == currentTeam1MembersCount && currentTeam1MembersCount > 0
 	allTeam2Ready := currentTeam2ReadyCount == currentTeam2MembersCount && currentTeam2MembersCount > 0
 	allReady := allTeam1Ready && allTeam2Ready
 
 	// Check if we should start countdown - use a flag to prevent multiple triggers
 	shouldStartCountdown := allReady && room.CurrentPhase == "setup"
 
 	// Check if countdown already started (phase is still setup but we have a flag in room)
 	// We'll use a simple check: if phase is setup and all ready, start countdown
 	if shouldStartCountdown {
 
 		// Broadcast countdown start to ALL clients immediately
 		countdownMessage := map[string]interface{}{
 			"type":      "countdownStart",
 			"countdown": 3,
 		}
-		for _, r := range room.Clients {
+		recipients := snapshotAllTeamClients(room)
+		room.Mutex.Unlock()
+		
+		for _, r := range recipients {
 			_ = r.SafeWriteJSON(countdownMessage)
 		}
 		
 		// Update phase immediately to prevent multiple triggers
+		room.Mutex.Lock()
 		room.CurrentPhase = "countdown"
+		room.Mutex.Unlock()

Also applies to: 726-728


754-758: Apply snapshot pattern in countdown goroutine.

The goroutine iterates room.Clients directly while holding the lock, causing the same concurrent access issue.

Apply this diff:

 			room.Mutex.Lock()
 			if room.CurrentPhase == "countdown" || room.CurrentPhase == "setup" {
 				room.CurrentPhase = "openingFor"
 				
 				// Broadcast phase change to ALL clients using proper TeamMessage format
 				phaseMessage := TeamMessage{
 					Type:  "phaseChange",
 					Phase: "openingFor",
 				}
-				for _, r := range room.Clients {
-					if err := r.SafeWriteJSON(phaseMessage); err != nil {
-					} else {
-					}
-				}
+				recipients := snapshotAllTeamClients(room)
+				room.Mutex.Unlock()
+				
+				for _, r := range recipients {
+					_ = r.SafeWriteJSON(phaseMessage)
+				}
+				return
 			} else {
 			}
 			room.Mutex.Unlock()

891-893: Apply snapshot pattern to both broadcasts in handleCheckStart.

Both countdown and phase change broadcasts iterate room.Clients directly while holding the mutex, repeating the same issue as handleTeamReadyStatus.

Apply this diff:

 	if allReady && room.CurrentPhase == "setup" {
 
 		// Update phase to prevent multiple triggers
 		room.CurrentPhase = "countdown"
 
 		// Broadcast countdown start to ALL clients immediately
 		countdownMessage := map[string]interface{}{
 			"type":      "countdownStart",
 			"countdown": 3,
 		}
-		for _, r := range room.Clients {
+		recipients := snapshotAllTeamClients(room)
+		room.Mutex.Unlock()
+		
+		for _, r := range recipients {
 			_ = r.SafeWriteJSON(countdownMessage)
 		}
 		
 		// Start countdown and phase change after 3 seconds
+		room.Mutex.Lock()
+		room.Mutex.Unlock()
+		
 		go func() {
 			time.Sleep(3 * time.Second)
 
 			teamRoomsMutex.Lock()
 			room, stillExists := teamRooms[roomKey]
 			teamRoomsMutex.Unlock()
 
 			if !stillExists {
 				return
 			}
 
 			room.Mutex.Lock()
 			if room.CurrentPhase == "countdown" || room.CurrentPhase == "setup" {
 				room.CurrentPhase = "openingFor"
 
 				// Broadcast phase change to ALL clients
 				phaseMessage := TeamMessage{
 					Type:  "phaseChange",
 					Phase: "openingFor",
 				}
-				for _, r := range room.Clients {
+				recipients := snapshotAllTeamClients(room)
+				room.Mutex.Unlock()
+				
+				for _, r := range recipients {
 					_ = r.SafeWriteJSON(phaseMessage)
 				}
+				return
 			}
 			room.Mutex.Unlock()
 		}()
+		return
 	}

Also applies to: 916-918

🧹 Nitpick comments (1)
backend/cmd/server/main.go (1)

38-42: Consider extracting the hardcoded config path to reduce duplication.

The config path "./config/config.prod.yml" is hardcoded in multiple places (lines 23, 39, 109, 145). While the initialization logic for Casbin and admin routes is correct, this duplication could lead to inconsistencies if the path needs to change.

Consider extracting the config path to a constant at the package level:

+const configPath = "./config/config.prod.yml"
+
 func main() {
 	// Load the configuration from the specified YAML file
-	cfg, err := config.LoadConfig("./config/config.prod.yml")
+	cfg, err := config.LoadConfig(configPath)

Then use configPath throughout the file:

 	// Initialize Casbin RBAC
-	if err := middlewares.InitCasbin("./config/config.prod.yml"); err != nil {
+	if err := middlewares.InitCasbin(configPath); err != nil {
 	// Admin routes
-	routes.SetupAdminRoutes(router, "./config/config.prod.yml")
+	routes.SetupAdminRoutes(router, configPath)

Also update line 109 in setupRouter to use the constant passed as a parameter or use the constant directly.

Also applies to: 144-146

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed4ede5 and dc57652.

⛔ Files ignored due to path filters (2)
  • backend/go.sum is excluded by !**/*.sum
  • frontend/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (10)
  • backend/cmd/server/main.go (3 hunks)
  • backend/go.mod (1 hunks)
  • backend/services/rating_service.go (3 hunks)
  • backend/websocket/team_websocket.go (7 hunks)
  • frontend/package.json (1 hunks)
  • frontend/src/App.tsx (2 hunks)
  • frontend/src/Pages/DebateRoom.tsx (1 hunks)
  • frontend/src/Pages/Game.tsx (6 hunks)
  • frontend/src/Pages/MatchLogs.tsx (1 hunks)
  • frontend/src/Pages/TeamDebateRoom.tsx (13 hunks)
✅ Files skipped from review due to trivial changes (1)
  • frontend/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
  • frontend/src/Pages/DebateRoom.tsx
  • frontend/src/Pages/MatchLogs.tsx
  • backend/go.mod
  • backend/services/rating_service.go
🧰 Additional context used
🧬 Code graph analysis (3)
backend/cmd/server/main.go (2)
backend/middlewares/rbac.go (1)
  • InitCasbin (26-101)
backend/routes/admin.go (1)
  • SetupAdminRoutes (11-39)
frontend/src/App.tsx (2)
frontend/src/Pages/Admin/AdminSignup.tsx (1)
  • AdminSignup (16-92)
frontend/src/Pages/Admin/AdminDashboard.tsx (1)
  • AdminDashboard (225-969)
backend/websocket/team_websocket.go (1)
backend/services/team_turn_service.go (1)
  • TokenBucket (22-28)
🪛 Biome (2.1.2)
frontend/src/Pages/TeamDebateRoom.tsx

[error] 513-514: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 515-516: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 611-611: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 612-612: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 658-660: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 661-663: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 664-666: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 667-669: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 919-919: Shouldn't redeclare 'toggleCamera'. Consider to delete it or rename it.

'toggleCamera' is defined here:

(lint/suspicious/noRedeclare)

🔇 Additional comments (16)
frontend/src/Pages/TeamDebateRoom.tsx (2)

197-216: LGTM! Refs correctly stabilize state for WebSocket handlers.

The pattern of mirroring frequently-changing reactive state (isTeam1, myTeamId, debatePhase, currentUser?.id) in refs ensures stable values inside WebSocket message handlers, preventing unnecessary reconnections.


428-431: Well done stabilizing WebSocket handlers with refs.

The use of refs (isTeam1Ref, myTeamIdRef, debatePhaseRef, currentUserIdRef) inside ws.onmessage combined with the reduced dependency array on line 780 correctly addresses the previous issue where the WebSocket would tear down and reconnect on every phase or debate state update. The derived variables (amTeam1, currentMyTeamId, etc.) provide stable, up-to-date values throughout the message handler.

Also applies to: 780-780

backend/cmd/server/main.go (4)

4-4: LGTM!

The log import is necessary for the new logging statements added throughout the file.


36-37: LGTM!

Adding logging for successful MongoDB connection improves observability and helps with debugging during startup.


38-42: LGTM!

The Casbin RBAC initialization is correctly placed after MongoDB connection (which Casbin depends on) and before route setup. Using log.Fatalf for initialization failures is appropriate for startup-critical components.


144-146: LGTM!

Admin routes are appropriately registered, and the logging statement helps track initialization progress. The placement after protected routes and before spectator WebSocket handlers is suitable.

frontend/src/Pages/Game.tsx (6)

1-1: Good addition of useCallback and debouncing refs.

The import of useCallback and the debouncing refs (lastTypingStateRef, lastSpeakingStateRef) are properly set up to optimize WebSocket message sending and avoid redundant status updates.

Also applies to: 35-36


38-45: Excellent WebSocket send helper with safety checks.

The sendWebSocketMessage helper properly checks the WebSocket ready state before sending and logs warnings for failed attempts. The use of useCallback with an empty dependency array is appropriate since it only accesses websocketRef.current.


127-142: Proper sender resolution prevents self-echo issues.

The sender resolution logic using data.userId ?? data.sender provides backward compatibility, and the filter (!senderId || senderId === userId) correctly prevents echoing the local user's typing/speaking status back to themselves.

Also applies to: 144-158


200-218: Well-structured message sending with proper validation.

The handleSendChatMessage callback correctly trims and validates the message before sending. The structured payload with sender, message, mode, and timestamp provides comprehensive context for the backend.


220-246: Verify intentionality of local user status indicators.

The debouncing logic and WebSocket integration are well-implemented. However, these handlers update the local state to include the current user's typing/speaking indicators (lines 235-243, 262-269). Typically, typing/speaking indicators are shown only for other participants, not the local user.

If this is intentional (e.g., to display "You are typing..." in the UI for user feedback), it's fine. Otherwise, consider removing the local state updates since the user already knows they're typing or speaking.

Also applies to: 248-272


343-352: Excellent resolution of previous review feedback.

The Chatbox props are now properly wired:

  • onSendMessage, onTypingChange, and onSpeakingChange are connected to real handlers
  • isMyTurn reflects actual turn state
  • disabled is conditional based on gameEnded or loading states

This fully addresses the past review comment about stub/no-op implementations and enables complete chat functionality.

backend/websocket/team_websocket.go (3)

6-6: LGTM!

The added imports are used appropriately in the code (log for logging and strings for token extraction).

Also applies to: 8-8


397-406: LGTM!

The helper function correctly implements the snapshot pattern, taking a locked copy of all clients and returning it for iteration. This prevents concurrent map access issues.


414-414: LGTM!

These handlers correctly use snapshotAllTeamClients to avoid concurrent map access issues. The pattern of capturing state under lock, releasing the lock, then iterating the snapshot is appropriate.

Also applies to: 545-558, 571-571, 606-606, 792-798, 825-825

frontend/src/App.tsx (1)

61-62: The original review comment is incorrect. Admin API endpoints are properly protected server-side.

The backend has robust authentication middleware (AdminAuthMiddleware) that validates all protected admin API calls:

  • Requires valid Authorization header with Bearer token
  • Validates JWT signature server-side
  • Queries database to confirm admin status
  • Blocks requests that fail any check

All admin endpoints (/analytics, /debates, /comments, /logs) use this middleware. Frontend route protection is a UX enhancement but not a security requirement, since backend validation is the authoritative trust boundary.

Likely an incorrect or invalid review comment.

import ProsConsChallenge from './Pages/ProsConsChallenge';
import TeamBuilder from './Pages/TeamBuilder';
import TeamDebateRoom from './Pages/TeamDebateRoom';
import AdminSignup from './Pages/Admin/AdminSignup';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Misleading component name: AdminSignup is actually a login page.

The component AdminSignup actually implements a login form (not signup/registration). Based on the component code, it displays "Sign in to your account" and calls adminLogin. Consider renaming to AdminLogin for clarity.

Apply this diff to rename the import:

-import AdminSignup from './Pages/Admin/AdminSignup';
+import AdminLogin from './Pages/Admin/AdminLogin';

And update the route on line 61:

-      <Route path='/admin/login' element={<AdminSignup />} />
+      <Route path='/admin/login' element={<AdminLogin />} />

Don't forget to rename the actual component file from AdminSignup.tsx to AdminLogin.tsx.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In frontend/src/App.tsx around line 27 and route at line 61, the imported
component AdminSignup is misnamed (it is actually a login page); rename the
import to AdminLogin, update the corresponding Route reference on line 61 to use
AdminLogin, and ensure any other references in this file are updated; also
rename the component file frontend/src/Pages/Admin/AdminSignup.tsx to
frontend/src/Pages/Admin/AdminLogin.tsx and update the default export name
inside that file (and any imports elsewhere) so the filename, component name,
and all imports consistently use AdminLogin.

@bhavik-mangla bhavik-mangla merged commit d4dcff6 into AOSSIE-Org:main Nov 23, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants