-
Notifications
You must be signed in to change notification settings - Fork 79
Added Gamification System with Badge Bitmask, Live Leaderboards, and Anti-Cheat Mechanisms #113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added Gamification System with Badge Bitmask, Live Leaderboards, and Anti-Cheat Mechanisms #113
Conversation
…sistency, including formatting adjustments and type definitions for JudgmentData. - Enhanced score retrieval logic for better clarity and maintainability.
WalkthroughAdds a gamification system: new models, controllers, routes, WebSocket hub and handler, leaderboard/profile/frontend UI updates, and debate flow integration to award scores/badges. Also updates team code generation, team routes, JWT helper, and frontend build deps. Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend as Client UI
participant Backend as API Server
participant DB as MongoDB
participant WS as Gamification Hub
Frontend->>Backend: POST /api/update-score or debate ends
Backend->>DB: Fetch user, atomically increment score, insert ScoreUpdate
DB-->>Backend: Updated user score
Backend->>DB: Check/insert badges (UserBadge), update user.Badges
DB-->>Backend: Badge records
Backend->>WS: BroadcastGamificationEvent (score_updated / badge_awarded)
WS->>Frontend: Push event to connected clients
Frontend->>Frontend: Update leaderboard/profile UI, show BadgeUnlocked
sequenceDiagram
participant Client as Leaderboard Page
participant WSHandler as /ws/gamification
participant Hub as Client Registry
participant Backend as API Server
Client->>WSHandler: WebSocket connect (token)
WSHandler->>Backend: Validate JWT & lookup user
WSHandler->>Hub: RegisterGamificationClient
Backend->>Hub: Trigger BroadcastGamificationEvent
Hub->>Client: Deliver event (badge_awarded / score_updated)
Client->>Client: Update UI in real time
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 35
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/Matchmaking.tsx (1)
133-142: Restore matchmaking reconnection after socket closesOnce the backend closes the socket (or the network blips),
onclosedrivesisConnectedto false, which disables the “Start Matchmaking” CTA. But because the effect only runs on the initial mount (its deps never change) and we keep holding the closed socket inwsRef, no new connection is ever created. Users are stuck in a disconnected state until they hard-refresh—this is a regression introduced by the new guard. Please clear the stale reference and trigger the effect to spin up a fresh socket after closure (e.g., null outwsRefand includeisConnectedin the dependency list so a close causes a reconnect pass).@@ - ws.onclose = (event) => { + ws.onclose = (event) => { setIsConnected(false); setIsInPool(false); + if (wsRef.current === ws) { + wsRef.current = null; + } @@ - }, [navigate, user?.id, user?.displayName, isLoading]); + }, [navigate, user?.id, user?.displayName, isLoading, isConnected]);Also applies to: 155-155
🧹 Nitpick comments (15)
frontend/src/components/ChatRoom.tsx (3)
110-129: Consider removing redundant null check.Lines 123-127 check
wsRef.currentagain after it was already verified on lines 112-113. If the WebSocket is in the OPEN state (line 113),wsRef.currentcannot be null.You can simplify by removing the redundant check:
const timestamp = Math.floor(Date.now() / 1000); setMessages((prev) => [ ...prev, { username, message: messageInput, timestamp }, ]); - if (wsRef.current) { - wsRef.current.send( - JSON.stringify({ type: 'chatMessage', content: messageInput }) - ); - } + wsRef.current.send( + JSON.stringify({ type: 'chatMessage', content: messageInput }) + ); setMessageInput('');
131-141: Consider removing redundant null check.Lines 136-140 check
wsRef.currentagain after verifyingreadyStateon line 132. The redundant check can be removed for cleaner code.Apply this diff:
const sendReaction = (reactionType: string) => { if (!wsRef.current || wsRef.current.readyState !== WebSocket.OPEN) { console.warn('Cannot send reaction: WebSocket not open'); return; } - if (wsRef.current) { - wsRef.current.send( - JSON.stringify({ type: 'reaction', extra: { reaction: reactionType } }) - ); - } + wsRef.current.send( + JSON.stringify({ type: 'reaction', extra: { reaction: reactionType } }) + ); };
151-168: Consider removing redundant null check.Lines 160-162 check
wsRef.currentagain after already verifyingreadyStateon line 155.Apply this diff:
if ( userVoted || !wsRef.current || wsRef.current.readyState !== WebSocket.OPEN ) { console.warn('Cannot vote: WebSocket not open or already voted'); return; } - if (wsRef.current) { - wsRef.current.send(JSON.stringify({ type: 'vote', extra: { option } })); - } + wsRef.current.send(JSON.stringify({ type: 'vote', extra: { option } })); setVoteResults((prev) => ({ ...prev, [option]: (prev[option] || 0) + 1, }));backend/controllers/profile_controller.go (1)
193-201: Normalize badges to an empty array.For legacy users the
badgesslice will benil, so this response serializesbadgesas JSONnull. New UI that expects an array (e.g.,badges.map(...)) will crash unless it defensively checks for null. Please default to an empty slice just before building the payload.+ badges := user.Badges + if badges == nil { + badges = []string{} + } c.JSON(http.StatusOK, gin.H{ "profile": gin.H{ "id": user.ID.Hex(), "displayName": displayName, "email": user.Email, "bio": user.Bio, "rating": int(user.Rating), - "score": user.Score, - "badges": user.Badges, + "score": user.Score, + "badges": badges, "currentStreak": user.CurrentStreak,backend/routes/team.go (1)
10-27: Consider standardizing route parameter names.The routes use both
:idand:teamIdfor team identifiers. This inconsistency can be confusing.Consider standardizing to
:teamIdthroughout:- teamRoutes.GET("/:id", controllers.GetTeam) - teamRoutes.POST("/:id/join", controllers.JoinTeam) - teamRoutes.POST("/:id/leave", controllers.LeaveTeam) + teamRoutes.GET("/:teamId", controllers.GetTeam) + teamRoutes.POST("/:teamId/join", controllers.JoinTeam) + teamRoutes.POST("/:teamId/leave", controllers.LeaveTeam)Update the corresponding controller methods to use
c.Param("teamId")instead ofc.Param("id").frontend/src/components/BadgeUnlocked.tsx (2)
13-27: Consider externalizing badge configuration.The badge icons and descriptions are hardcoded. For better maintainability as the badge system grows, consider moving these to a configuration file or fetching them from the backend.
47-54: Consider conditional resize listener.The resize listener runs continuously even when the dialog is closed. For a minor optimization, you could conditionally add/remove the listener based on
isOpen:useEffect(() => { + if (!isOpen) return; + const handleResize = () => { setWindowSize({ width: window.innerWidth, height: window.innerHeight }); }; window.addEventListener("resize", handleResize); return () => window.removeEventListener("resize", handleResize); -}, []); +}, [isOpen]);backend/models/gamification.go (1)
10-55: Consider adding database indexes and note type inconsistency.
Index Recommendations: For frequently queried fields, consider adding indexes in your database setup:
UserBadge.UserIDandUserBadge.BadgeName(composite index for lookups)ScoreUpdate.UserIDandScoreUpdate.CreatedAt(for user score history)RateLimitEntry.UserIDandRateLimitEntry.Action(composite for rate limit checks)Type Inconsistency:
GamificationEvent.UserID(line 49) is astringwhile other structs useprimitive.ObjectID. This is likely intentional for WebSocket serialization, but consider documenting this decision.frontend/src/components/TeamMatchmaking.tsx (5)
50-65: Complex captain check logic.The captain verification logic handles multiple ID formats with type assertions. While functional, this suggests inconsistent data structures from the backend. Consider standardizing the API response format to always return strings for IDs to simplify this logic.
71-106: Consider optimizing debate status polling.The 2-second polling interval (line 103) for all team members could be optimized:
- Increase interval: For non-real-time updates, consider 5-10 seconds.
- Use WebSocket: Since the PR adds WebSocket support, consider using it for real-time debate notifications instead of polling.
- Conditional polling: Only poll when the tab is visible using the Page Visibility API.
109-124: Matchmaking polling could use WebSocket.Similar to the debate polling, this 3-second interval could be replaced with WebSocket notifications for a more responsive and efficient experience. The backend already has WebSocket infrastructure at
/ws/team.
156-175: Hardcoded debate topic limits functionality.Line 160 uses a hardcoded topic "Should AI be regulated?" with a comment noting it "Could be dynamic". Consider allowing the captain to select or input a topic before creating the debate to improve user experience.
126-142: Replace alert() with toast notifications.Lines 128, 139, 152, and 173 use browser
alert()for error messages. Consider using a toast notification system (like the one likely available in your UI library) for better UX.backend/websocket/gamification.go (1)
41-47: Consider defensive connection closing.Line 45 closes the WebSocket connection without checking if it's already closed. While
gorilla/websockettypically handles this gracefully, consider adding error handling for robustness:func UnregisterGamificationClient(client *GamificationClient) { gamificationMutex.Lock() defer gamificationMutex.Unlock() delete(gamificationClients, client) - client.Conn.Close() + if err := client.Conn.Close(); err != nil { + log.Printf("Error closing gamification client connection: %v", err) + } log.Printf("Gamification client unregistered. Total clients: %d", len(gamificationClients)) }frontend/tsconfig.app.tsbuildinfo (1)
1-1: Consider excluding build artifacts from version control.The
tsconfig.app.tsbuildinfofile is a TypeScript build artifact that gets regenerated on each build. Consider adding*.tsbuildinfoto.gitignoreto avoid unnecessary merge conflicts and repository bloat.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (58)
backend/cmd/server/main.go(3 hunks)backend/controllers/auth.go(2 hunks)backend/controllers/debatevsbot_controller.go(3 hunks)backend/controllers/gamification_controller.go(1 hunks)backend/controllers/leaderboard.go(3 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/middlewares/auth.go(2 hunks)backend/models/gamification.go(1 hunks)backend/models/team.go(1 hunks)backend/models/user.go(1 hunks)backend/routes/gamification.go(1 hunks)backend/routes/team.go(1 hunks)backend/services/coach.go(2 hunks)backend/services/debatevsbot.go(2 hunks)backend/services/gemini.go(1 hunks)backend/services/pros_cons.go(2 hunks)backend/services/team_matchmaking.go(1 hunks)backend/services/team_turn_service.go(1 hunks)backend/services/transcriptservice.go(1 hunks)backend/utils/auth.go(1 hunks)backend/websocket/gamification.go(1 hunks)backend/websocket/gamification_handler.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/DebateRoom.tsx(5 hunks)frontend/src/Pages/Game.tsx(6 hunks)frontend/src/Pages/Leaderboard.tsx(8 hunks)frontend/src/Pages/MatchLogs.tsx(5 hunks)frontend/src/Pages/OnlineDebateRoom.tsx(15 hunks)frontend/src/Pages/Profile.tsx(4 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/BadgeUnlocked.tsx(1 hunks)frontend/src/components/ChatRoom.tsx(6 hunks)frontend/src/components/JudgementPopup.tsx(8 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/hooks/useUser.ts(1 hunks)frontend/src/services/gamificationService.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 (32)
frontend/src/components/BadgeUnlocked.tsx (2)
frontend/src/components/ui/dialog.tsx (4)
Dialog(109-109)DialogContent(114-114)DialogTitle(117-117)DialogDescription(118-118)frontend/src/components/ui/button.tsx (1)
Button(57-57)
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)
backend/websocket/matchmaking.go (1)
backend/services/matchmaking.go (1)
GetMatchmakingService(40-49)
backend/middlewares/auth.go (3)
backend/config/config.go (1)
LoadConfig(53-65)backend/models/user.go (1)
User(10-35)backend/db/db.go (1)
MongoDatabase(17-17)
backend/websocket/gamification.go (2)
backend/models/gamification.go (1)
GamificationEvent(47-55)frontend/src/services/gamificationService.ts (1)
GamificationEvent(3-12)
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)
frontend/src/components/ChatRoom.tsx (2)
backend/websocket/websocket.go (1)
Message(63-86)backend/structs/websocket.go (1)
Message(10-13)
backend/controllers/leaderboard.go (1)
backend/db/db.go (1)
MongoDatabase(17-17)
backend/routes/gamification.go (1)
backend/controllers/gamification_controller.go (3)
AwardBadge(49-161)UpdateScore(164-257)GetGamificationLeaderboard(364-431)
backend/websocket/gamification_handler.go (4)
backend/utils/auth.go (1)
GetJWTSecret(39-41)backend/models/user.go (1)
User(10-35)backend/db/db.go (1)
MongoDatabase(17-17)backend/websocket/gamification.go (3)
GamificationClient(13-17)RegisterGamificationClient(33-38)UnregisterGamificationClient(41-47)
backend/controllers/team_matchmaking.go (4)
backend/db/db.go (1)
GetCollection(21-23)backend/models/team.go (1)
Team(11-22)frontend/src/services/teamService.ts (1)
Team(4-15)backend/services/team_matchmaking.go (4)
StartTeamMatchmaking(31-62)RemoveFromMatchmaking(115-122)GetMatchmakingPool(125-133)FindMatchingTeam(65-112)
frontend/src/App.tsx (1)
backend/websocket/team_debate_handler.go (1)
TeamDebateRoom(32-36)
frontend/src/components/Matchmaking.tsx (2)
backend/websocket/matchmaking.go (1)
MatchmakingMessage(49-57)backend/services/matchmaking.go (1)
MatchmakingPool(17-26)
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/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)
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)
frontend/src/components/MatchmakingPool.tsx (2)
backend/services/matchmaking.go (1)
MatchmakingPool(17-26)frontend/src/services/teamDebateService.ts (1)
getMatchmakingPool(117-130)
frontend/src/services/gamificationService.ts (3)
backend/models/gamification.go (1)
GamificationEvent(47-55)backend/controllers/gamification_controller.go (2)
AwardBadgeRequest(21-25)UpdateScoreRequest(28-32)backend/controllers/profile_controller.go (1)
LeaderboardEntry(99-105)
backend/controllers/debatevsbot_controller.go (5)
backend/db/db.go (1)
MongoDatabase(17-17)backend/models/user.go (1)
User(10-35)backend/models/gamification.go (3)
ScoreUpdate(29-36)UserBadge(20-26)GamificationEvent(47-55)backend/websocket/gamification.go (1)
BroadcastGamificationEvent(50-83)frontend/src/services/gamificationService.ts (1)
GamificationEvent(3-12)
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/DebateRoom.tsx (1)
frontend/src/services/vsbot.ts (1)
judgeDebate(105-123)
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/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(136-149)backend/db/db.go (1)
MongoDatabase(17-17)backend/models/team.go (2)
TeamDebate(61-81)Team(11-22)
backend/cmd/server/main.go (4)
backend/websocket/gamification_handler.go (1)
GamificationWebSocketHandler(21-131)backend/routes/gamification.go (3)
AwardBadgeRouteHandler(9-11)UpdateScoreRouteHandler(13-15)GetGamificationLeaderboardRouteHandler(17-19)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/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/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/controllers/gamification_controller.go (6)
frontend/src/services/gamificationService.ts (3)
AwardBadgeRequest(14-18)GamificationEvent(3-12)UpdateScoreRequest(20-24)backend/db/db.go (1)
MongoDatabase(17-17)backend/models/user.go (1)
User(10-35)backend/models/gamification.go (4)
UserBadge(20-26)GamificationEvent(47-55)ScoreUpdate(29-36)RateLimitEntry(39-44)backend/websocket/gamification.go (1)
BroadcastGamificationEvent(50-83)backend/controllers/leaderboard.go (1)
Debater(26-34)
backend/websocket/websocket.go (1)
backend/structs/websocket.go (2)
Room(30-37)Message(10-13)
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/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)
frontend/src/Pages/Leaderboard.tsx (4)
backend/controllers/leaderboard.go (3)
Debater(26-34)Stat(37-41)LeaderboardData(20-23)frontend/src/hooks/useUser.ts (1)
useUser(7-60)frontend/src/services/gamificationService.ts (3)
fetchGamificationLeaderboard(41-55)createGamificationWebSocket(93-126)GamificationEvent(3-12)frontend/src/services/leaderboardService.ts (1)
fetchLeaderboardData(3-17)
🪛 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 (31)
frontend/src/Pages/About.tsx (1)
1-121: Styling and formatting improvements look good; no concerns.The changes are purely cosmetic refinements: React import removal aligns with React 18+ JSX transform, text reflow improves readability without altering semantic structure, and the semicolon on the export statement improves consistency with modern JavaScript conventions. The heading text update ("Get Involved" is more concise than "Contributing / Community Involvement") is a reasonable UX improvement. All markup and Tailwind classes remain intact and appropriate.
frontend/src/Pages/MatchLogs.tsx (3)
121-126: LGTM!The explicit return object formatting improves readability.
174-194: LGTM!The score comparison logic correctly highlights the higher score with proper conditional styling.
199-220: LGTM!The total score highlighting logic is correct and consistent with the individual score rendering approach.
frontend/src/Pages/Game.tsx (2)
167-179: Well implemented functional state updates.The camera and microphone state setters correctly support both direct values and functional updates, enabling flexible usage patterns like
setCameraOn(true)orsetCameraOn(prev => !prev). This is a good React practice.
189-195: Verify hardcoded Chatbox props and stub implementations.The Chatbox integration has several concerns that should be verified:
isMyTurn={true}is hardcoded but should likely bestate.isTurnto reflect actual turn statedisabled={true}hardcodes the chatbox as always disabled, preventing user interaction- All three handlers (
onSendMessage,onTypingChange,onSpeakingChange) are empty stubs that don't perform any actionsPlease confirm whether:
- These are intentional placeholders for features not yet fully implemented, or
- The hardcoded values and empty handlers should be connected to actual functionality
If these should be functional, consider this implementation:
- onSendMessage={() => {}} - onTypingChange={() => {}} - onSpeakingChange={() => {}} + onSendMessage={(message) => { + // TODO: Implement message sending via WebSocket + }} + onTypingChange={(isTyping) => { + // TODO: Broadcast typing status + }} + onSpeakingChange={(isSpeaking) => { + // TODO: Broadcast speaking status + }} typingIndicators={[]} - isMyTurn={true} - disabled={true} + isMyTurn={state.isTurn} + disabled={!state.isTurn || state.gameEnded}frontend/src/components/ChatRoom.tsx (4)
7-18: Well-structured TypeScript interfaces.The type definitions improve type safety throughout the component and align well with the WebSocket message schema.
34-40: Good cleanup implementation.The WebSocket cleanup on unmount prevents memory leaks and dangling connections.
52-57: Defensive null check is good practice.While
wsRef.currentshould be non-null immediately after instantiation, the explicit check improves code safety.
216-239: Type annotation is redundant but harmless.The explicit
msg: Messagetype annotation on line 219 can be inferred by TypeScript from themessagesarray type. However, explicit typing can improve readability and is not incorrect.The conditional styling with
clsxand timestamp handling are well implemented.frontend/package.json (1)
26-26: New dependencies appropriately support gamification features.The additions of react-dnd packages and react-confetti align well with the PR objectives for team workflows, drag-and-drop interactions, and celebratory UI effects. The versions are consistent (react-dnd and react-dnd-html5-backend both at ^16.0.1), and the type definitions are included for TypeScript support.
Also applies to: 35-35, 37-38
frontend/src/components/Sidebar.tsx (1)
41-45: LGTM!The new "Team Debates" navigation item is well-integrated, consistent with existing navigation items, and properly uses the imported
Usersicon.backend/services/coach.go (2)
147-147: Same model version used consistently.The same
gemini-2.5-flashmodel version is used here as inGenerateWeakStatement. Ensure this is verified along with line 49.
49-49: Model version verified as valid and compatible.The
gemini-2.5-flashmodel is an official Gemini model and is compatible with generative-ai-go v0.19.0, which accepts Gemini model names as strings. No changes needed.backend/db/db.go (1)
20-23: LGTM!The
GetCollectionhelper function provides a clean API for accessing collections. Note that this assumesMongoDatabasehas been initialized viaConnectMongoDB- which is an acceptable fail-fast pattern for database operations.backend/services/debatevsbot.go (2)
271-271: Model version update is consistent with coach.go.The Gemini model version has been updated to
gemini-2.5-flashhere as well. This should be verified alongside the changes inbackend/services/coach.go(line 49).
484-484: Consistent model version in JudgeDebate.The same
gemini-2.5-flashmodel version is used consistently throughout the file.backend/utils/auth.go (1)
38-41: LGTM!The public
GetJWTSecret()wrapper provides external packages access to the JWT secret for authentication operations (e.g., WebSocket authentication, middleware validation). The internalgetJWTSecret()already includes validation to ensure the secret is set before use.frontend/src/components/Layout.tsx (1)
2-20: LGTM!The changes are purely stylistic (quote normalization and semicolons), improving code consistency without altering functionality.
backend/websocket/websocket.go (4)
85-85: LGTM!The new
LiveTranscriptfield is properly defined with JSON tags and theomitemptydirective, consistent with other optional Message fields.
232-233: Proper message routing added.The new
liveTranscriptmessage type is correctly routed to thehandleLiveTranscripthandler, consistent with the existing message routing pattern.
344-350: Phase information now included in speech text responses.Adding the
phasefield to thespeechTextresponse enhances context for real-time transcription, aligning with the newliveTranscriptfeature.
352-367: Well-structured live transcript handler.The
handleLiveTranscriptfunction follows the established pattern of other broadcast handlers, usingSafeWriteJSONfor thread-safe writes and including proper error logging. The implementation correctly broadcasts interim transcripts to other clients while including user metadata and phase information.backend/services/pros_cons.go (1)
37-37: Model version bump looks consistent.Both entry points now rely on
gemini-2.5-flash, keeping this service aligned with the wider backend upgrade. Nothing else to change from my side.Also applies to: 115-115
backend/routes/team.go (3)
30-37: LGTM!The team debate routes are well-organized with consistent parameter naming.
40-48: Verify authorization for the matchmaking pool endpoint.Line 46 exposes
GET /poolwhich returns the entire matchmaking pool. Based on the controller comment at backend/controllers/team_matchmaking.go line 84, this is "for debugging". Ensure this endpoint is either restricted to admin users or removed in production.
51-53: LGTM - Placeholder for future functionality.The empty implementation with a clear comment is acceptable for deferring team chat functionality.
frontend/src/components/BadgeUnlocked.tsx (1)
33-45: LGTM!The confetti lifecycle is properly managed with appropriate cleanup.
backend/controllers/team_matchmaking.go (2)
85-106: Debug endpoint should be restricted.As noted in line 84, this endpoint is for debugging. Ensure it's either restricted to admin users or removed before production deployment.
17-69: The review comment is incorrect.The
StartTeamMatchmakingfunction already handles duplicate entries gracefully. The matchmaking pool uses a map withteamID.Hex()as the key, so calling the function multiple times with the same team ID will simply overwrite the previous entry with an updated timestamp—not create duplicates. This is the correct and intended behavior for a matchmaking pool.Likely an incorrect or invalid review comment.
backend/websocket/gamification.go (1)
13-24: LGTM!The per-client mutex in
SafeWriteJSONis a good pattern for preventing concurrent write issues on WebSocket connections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
♻️ Duplicate comments (1)
frontend/src/Pages/TeamDebateRoom.tsx (1)
424-764: Wrap switch-case variable declarations in blocks.The switch statement declares variables directly in case clauses without wrapping them in blocks. This violates
lint/correctness/noSwitchDeclarationsand will cause build failures. Each case that declares variables must be wrapped in braces.This was flagged in a previous review and remains unresolved.
Apply blocks to each affected case:
- case "stateSync": + case "stateSync": { const backendPhase = data.phase as DebatePhase; ... break; + } - case "teamMembers": + case "teamMembers": { if (data.team1Members) { ... break; + }Repeat for all cases:
roleSelection,countdownStart,ready,phaseChange,speechText,liveTranscript,teamStatus.
🧹 Nitpick comments (12)
frontend/src/services/gamificationService.ts (4)
67-70: Guard against non-JSON error responses.If the server or proxy returns a non-JSON error (e.g., 502 Bad Gateway HTML),
await response.json()will throw and mask the original error. Consider wrapping the error parsing in a try-catch block.Apply this diff to handle non-JSON error responses:
if (!response.ok) { - const error = await response.json(); - throw new Error(error.error || `Failed to award badge: ${response.status}`); + try { + const error = await response.json(); + throw new Error(error.error || `Failed to award badge: ${response.status}`); + } catch (parseError) { + throw new Error(`Failed to award badge: ${response.status} ${response.statusText}`); + } }
85-88: Guard against non-JSON error responses.Same as the
awardBadgefunction: non-JSON error responses will causeresponse.json()to throw and mask the original HTTP error.Apply this diff:
if (!response.ok) { - const error = await response.json(); - throw new Error(error.error || `Failed to update score: ${response.status}`); + try { + const error = await response.json(); + throw new Error(error.error || `Failed to update score: ${response.status}`); + } catch (parseError) { + throw new Error(`Failed to update score: ${response.status} ${response.statusText}`); + } }
57-73: Consider adding explicit return type annotation.The function returns
response.json()but lacks an explicit return type annotation. Adding one (e.g.,Promise<void>if no response data is expected, or a specific type if the backend returns data) would improve type safety.
93-129: Consider implementing automatic reconnection for production resilience.The WebSocket connection lacks automatic reconnection logic. For production deployments, consider adding exponential backoff reconnection in the consuming components or wrapping this helper with reconnection logic to handle network interruptions gracefully.
frontend/src/Pages/TeamBuilder.tsx (3)
206-206: Consider replacing browserconfirm()with a proper dialog component.Using browser
confirm()(lines 206, 221–225, 303) blocks the main thread and provides a poor UX. Since you've already importedDialogcomponents, consider creating a reusable confirmation dialog for delete/remove/leave actions.Also applies to: 221-225, 303-303
156-156: Consider extracting message timeout durations to named constants.Success (3000ms) and error (5000ms) timeouts are repeated throughout the file. Extracting these to constants (e.g.,
MESSAGE_DISPLAY_DURATION_MS) would improve maintainability.Also applies to: 171-171, 175-175, 213-213, 216-216, 230-230, 236-236, 273-273, 276-276, 289-289, 292-292, 299-299, 311-311, 314-314, 332-332, 335-335
385-385: Consider replacing deprecatedonKeyPresswithonKeyDown.
onKeyPressis deprecated in React 18. While it still works, preferonKeyDownfor better compatibility:- onKeyPress={(e) => e.key === "Enter" && handleJoinByCode()} + onKeyDown={(e) => e.key === "Enter" && handleJoinByCode()}Apply similar changes on lines 425 and 504–506.
Also applies to: 425-425, 504-506
frontend/src/Pages/TeamDebateRoom.tsx (5)
803-914: Consider using debatePhaseRef for speech recognition handlers.The speech recognition effect recreates the recognition instance whenever
debatePhasechanges (line 914). Theonresulthandler (lines 828-877) capturesdebatePhasein its closure and uses it when sending WebSocket messages (lines 857, 872).While the current implementation works, it recreates the SpeechRecognition instance on every phase change. Consider using
debatePhaseRef.currentinstead to avoid recreating the instance and to match the pattern used in the WebSocket effect.Apply this pattern:
- }, [debatePhase, isMyTurn, currentUser?.id, currentUser?.displayName]); + }, [isMyTurn, currentUser?.id, currentUser?.displayName]);And read phase from ref:
- phase: debatePhase, + phase: debatePhaseRef.current,
183-198: WebRTC peer connections are stubbed out.Lines 183, 196:
pcRefsandremoteStreamsare defined but never populated. Lines 755-763: WebRTC message handlers (offer,answer,candidate) are empty. This means video/audio from remote team members won't be transmitted or received.If WebRTC is intentionally deferred, consider adding a TODO comment. Otherwise, remote team members won't see each other's video streams.
Do you want me to implement the WebRTC connection logic, or should this be tracked as a known limitation?
Also applies to: 755-763
1686-1701: Consider moving inline styles to CSS module or Tailwind config.The inline
<style>tag with theglowanimation could be moved to a CSS module or Tailwind configuration for better maintainability and performance. Inline styles aren't optimized by build tools and are duplicated if the component mounts multiple times.Option 1: Add to
tailwind.config.js:module.exports = { theme: { extend: { keyframes: { glow: { '0%, 100%': { boxShadow: '0 0 5px rgba(255, 149, 0, 0.5)' }, '50%': { boxShadow: '0 0 20px rgba(255, 149, 0, 0.8)' }, } }, animation: { glow: 'glow 2s infinite', } } } }Option 2: Create a CSS module file.
154-227: Consider refactoring to useReducer for complex state management.The component manages 20+ state variables with complex interdependencies. Lines 1082-1141 show a useEffect with intricate logic deriving values from multiple state variables. This makes the component harder to maintain and test.
Consider refactoring to
useReduceror a state machine (e.g., XState) to centralize state transitions and make the logic more predictable.
144-152: Remove or gate console.log statements for production.The code contains extensive
console.logstatements (lines 144-152, 276-284, 401-404, 599-690, and many others) that will clutter production logs and potentially expose sensitive information.Consider one of these approaches:
- Remove debug logs before production
- Use a logging library with levels (debug/info/warn/error)
- Gate logs with an environment check:
if (import.meta.env.DEV) { console.log(...) }Also applies to: 276-284, 599-690
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/controllers/debatevsbot_controller.go(3 hunks)backend/controllers/team_controller.go(1 hunks)frontend/src/Pages/TeamBuilder.tsx(1 hunks)frontend/src/Pages/TeamDebateRoom.tsx(1 hunks)frontend/src/services/gamificationService.ts(1 hunks)frontend/src/services/teamDebateService.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- backend/controllers/team_controller.go
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/services/teamDebateService.ts
🧰 Additional context used
🧬 Code graph analysis (4)
frontend/src/Pages/TeamBuilder.tsx (3)
backend/models/team.go (2)
TeamMember(25-32)Team(11-22)frontend/src/services/teamService.ts (12)
TeamMember(17-24)Team(4-15)getAvailableTeams(142-155)getUserTeams(126-139)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/controllers/debatevsbot_controller.go (4)
backend/db/db.go (1)
MongoDatabase(17-17)backend/models/user.go (1)
User(10-35)backend/models/gamification.go (3)
ScoreUpdate(29-36)UserBadge(20-26)GamificationEvent(47-55)backend/websocket/gamification.go (1)
BroadcastGamificationEvent(50-83)
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(65-78)
frontend/src/services/gamificationService.ts (3)
backend/models/gamification.go (1)
GamificationEvent(47-55)backend/controllers/gamification_controller.go (2)
AwardBadgeRequest(21-25)UpdateScoreRequest(28-32)backend/controllers/profile_controller.go (1)
LeaderboardEntry(99-105)
🔇 Additional comments (6)
frontend/src/services/gamificationService.ts (2)
99-102: Previous WebSocket security concern has been addressed.The hardcoded
ws://localhost:1313issue flagged in the previous review has been fixed. The code now correctly derives the WebSocket protocol frombaseURL, switching towss://for HTTPS deployments.
26-34: The backend correctly provides all required fields includingidandrating.The backend's
Debaterstruct (leaderboard.go:26-34) includes all fields expected by the frontendLeaderboardEntryinterface:id,rank,name,score,rating,avatarUrl, andcurrentUser. TheGetGamificationLeaderboardendpoint (gamification_controller.go:416-424) populates each field correctly, includingIDfromuser.ID.Hex()andRatingfromuser.Rating. The response structure matches what the frontend expects.Likely an incorrect or invalid review comment.
frontend/src/Pages/TeamBuilder.tsx (4)
818-821: Previous issue resolved: full-team detection now uses team.maxSize.The hard-coded capacity value flagged in the previous review has been fixed. The code now correctly derives
capacityfromteam.maxSizewith a proper fallback, ensuring accurate full-team detection for teams of any size.
548-548: Verify that displaying user email addresses aligns with privacy requirements.Captain and member emails are displayed in several places (lines 548, 733, 860). Ensure this complies with your privacy policy and user expectations, especially if this UI is accessible beyond team members.
Also applies to: 733-733, 860-860
582-634: LGTM: Team size editing properly constrained.The logic correctly prevents captains from reducing team size below the current member count and only allows editing when the team is empty. This prevents data inconsistencies and provides clear UX feedback.
240-255: Captain check logic is defensively implemented.The
isCaptainfunction handles both string and object ID formats for backwards compatibility with MongoDB ObjectId representations. The fallback to email comparison provides additional robustness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/Pages/Profile.tsx (1)
434-434: Fix the malformed className.The className contains corrupted text
"flex gap- personally2"which will break the intended layout.Apply this diff to fix the className:
- <div className="flex gap- personally2"> + <div className="flex gap-2">frontend/src/Pages/DebateRoom.tsx (1)
699-699: Fix Tailwind CSS class name.The class
border-blue500is missing a hyphen and should beborder-blue-500. This will cause the border color styling to not apply to the loading spinner.-<div className="animate-spin rounded-full h-16 w-16 border-t-4 border-blue500 mb-4"></div> +<div className="animate-spin rounded-full h-16 w-16 border-t-4 border-blue-500 mb-4"></div>
♻️ Duplicate comments (3)
frontend/src/Pages/TeamDebateRoom.tsx (1)
1005-1005: Replace hardcoded localhost URL with BASE_URL.Line 1005 still uses a hardcoded
http://localhost:1313/submit-transcriptsURL. This should use theBASE_URLconstant defined at lines 121-123.Apply this diff:
- const response = await fetch(`http://localhost:1313/submit-transcripts`, { + const response = await fetch(`${BASE_URL}/submit-transcripts`, {backend/controllers/debatevsbot_controller.go (2)
417-447: Wait for successful FirstWin persistence before broadcasting.
UpdateOne’s result is ignored, so a write failure (timeout, retry, duplicate) still appends the badge locally and fires the WebSocket event. Clients think the badge is earned while the DB never changed, and the user can’t re-earn it. Capture(res, err), verifyerr == nilandres.ModifiedCount > 0, and only then append/broadcast; otherwise log and exit that branch.- badgeUpdate := bson.M{"$addToSet": bson.M{"badges": "FirstWin"}} - userCollection.UpdateOne(ctx, bson.M{"_id": userID}, badgeUpdate) - - // Update the updatedUser object to include the new badge - updatedUser.Badges = append(updatedUser.Badges, "FirstWin") + badgeUpdate := bson.M{"$addToSet": bson.M{"badges": "FirstWin"}} + if res, err := userCollection.UpdateOne(ctx, bson.M{"_id": userID}, badgeUpdate); err != nil || res.ModifiedCount == 0 { + log.Printf("Failed to persist FirstWin badge for %s: err=%v modified=%d", userID.Hex(), err, res.ModifiedCount) + return + } + + // Update the updatedUser object to include the new badge + updatedUser.Badges = append(updatedUser.Badges, "FirstWin")
474-511: Verify automatic badge writes before announcing.
checkAndAwardAutomaticBadgesfollows the same pattern: it broadcasts Novice/Streak5/FactMaster even ifUpdateOnefails, creating phantom badges for clients. Mirror the FirstWin fix—capture(res, err), ensure the update actually modified the document, and only then emit the WebSocket event (log and return otherwise). Applying that pattern to each badge branch keeps client state consistent with Mongo.
🧹 Nitpick comments (8)
frontend/src/Pages/Profile.tsx (2)
768-768: Use badge name as key instead of index.Using
indexas a key is suboptimal when you have a more stable identifier available. Since badge names should be unique, use the badge string itself as the key.Apply this diff:
- {profile.badges.map((badge, index) => { + {profile.badges.map((badge) => { const badgeIcons: Record<string, React.ReactNode> = { Novice: <FaAward className="w-6 h-6 text-blue-500" />, Streak5: <FaMedal className="w-6 h-6 text-yellow-500" />, FactMaster: <FaTrophy className="w-6 h-6 text-purple-500" />, FirstWin: <FaTrophy className="w-6 h-6 text-green-500" />, Debater10: <FaMedal className="w-6 h-6 text-orange-500" />, }; const badgeDescriptions: Record<string, string> = { Novice: "Completed first debate", Streak5: "5-day streak achieved", FactMaster: "Master of facts (500+ points)", FirstWin: "First victory earned", Debater10: "10 debates completed", }; const badgeIcon = badgeIcons[badge] || <FaAward className="w-6 h-6 text-primary" />; const badgeDescription = badgeDescriptions[badge] || "Achievement unlocked"; return ( <div - key={index} + key={badge} className="flex flex-col items-center justify-center p-3 bg-muted rounded-lg border border-border hover:bg-accent transition-colors cursor-pointer group"
769-784: Consider centralizing badge configuration to improve maintainability.The badge icons and descriptions are hardcoded, requiring frontend changes whenever new badges are added on the backend. While a fallback exists, consider one of these approaches:
- Move badge metadata to a shared configuration file (e.g.,
badgeConfig.ts)- Fetch badge metadata from the backend alongside badge names
- At minimum, document the expected badge names and coordinate with backend
Example refactor to a shared config file:
// badgeConfig.ts export const BADGE_CONFIG: Record<string, { icon: React.ReactNode; description: string }> = { Novice: { icon: <FaAward className="w-6 h-6 text-blue-500" />, description: "Completed first debate" }, Streak5: { icon: <FaMedal className="w-6 h-6 text-yellow-500" />, description: "5-day streak achieved" }, // ... other badges }; export const DEFAULT_BADGE = { icon: <FaAward className="w-6 h-6 text-primary" />, description: "Achievement unlocked" };Then import and use in the component:
const { icon, description } = BADGE_CONFIG[badge] || DEFAULT_BADGE;frontend/src/Pages/TeamDebateRoom.tsx (3)
421-523: Switch case properly wrapped; consider reducing debug logging.The case block is now correctly wrapped in braces, addressing the previous lint error. The state synchronization logic handles late joiners well.
Consider removing or conditionally disabling the verbose console.log statements (e.g., lines 430-432) for production builds to reduce noise.
597-688: Switch case properly wrapped, but excessive logging present.The case block is correctly wrapped. However, lines 598-617 contain 15+ debug console.log statements that add significant noise. These should be removed or wrapped in a conditional check (e.g.,
if (DEBUG_MODE)) for production.Apply this pattern to reduce logging:
- case "ready": { - console.log("=== READY MESSAGE RECEIVED ==="); - console.log("Received ready message:", data); - console.log("Current user:", currentUser?.id); - console.log("Message userId:", data.userId); - // ... (remove or conditionalize remaining logs) + case "ready": { + if (import.meta.env.DEV) { + console.debug("Ready message:", { userId: data.userId, teamId: data.teamId, ready: data.ready }); + }
689-710: Switch case properly wrapped; reduce verbose logging.The case block is correctly wrapped and uses
debatePhaseRef.currentappropriately. Consider consolidating the multiple console.log statements (lines 692-696) into a single conditional debug statement.frontend/src/Pages/DebateRoom.tsx (1)
598-627: Improve UX for judgment failure scenario.When judgment fails, showing numerical scores of
0with "Error occurred during judgment" may confuse users into thinking they received zero points rather than that judgment failed.Consider either:
- Hiding the detailed scores and showing only the error message
- Using
nullor-1for scores to clearly indicate no judgment occurred- Adding a prominent error banner in the JudgmentPopup component
Additionally, the 3-second delay before showing the judgment popup seems arbitrary. Consider using a shorter delay (1-2 seconds) or dismissing immediately with a clear error state.
backend/cmd/server/main.go (2)
38-42: Avoid hardcoded config path duplication.The config path
"./config/config.prod.yml"is hardcoded in multiple locations (lines 23, 39, 110, 151). Consider extracting this to a constant or passing the already-loadedcfgobject toInitCasbininstead of the file path.Apply this diff to extract the config path:
+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) if err != nil { panic("Failed to load config: " + err.Error()) }Then update other occurrences to use
configPath.
34-40: Consider consistent error handling for initialization failures.MongoDB connection errors use
panic(line 34) while Casbin initialization useslog.Fatalf(line 40). Both immediately terminate the program, but mixing styles reduces code consistency. Consider standardizing on one approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
backend/cmd/server/main.go(5 hunks)backend/controllers/auth.go(2 hunks)backend/controllers/debatevsbot_controller.go(2 hunks)backend/controllers/leaderboard.go(3 hunks)backend/controllers/profile_controller.go(1 hunks)backend/controllers/team_controller.go(5 hunks)backend/models/user.go(1 hunks)backend/services/debatevsbot.go(2 hunks)backend/services/gemini.go(1 hunks)backend/services/pros_cons.go(2 hunks)backend/utils/auth.go(1 hunks)frontend/package.json(2 hunks)frontend/src/Pages/DebateRoom.tsx(4 hunks)frontend/src/Pages/Profile.tsx(4 hunks)frontend/src/Pages/TeamDebateRoom.tsx(13 hunks)frontend/src/components/JudgementPopup.tsx(3 hunks)frontend/src/services/teamDebateService.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- backend/utils/auth.go
- backend/services/gemini.go
- backend/models/user.go
- backend/controllers/auth.go
- frontend/src/components/JudgementPopup.tsx
- frontend/src/services/teamDebateService.ts
- backend/controllers/profile_controller.go
🧰 Additional context used
🧬 Code graph analysis (4)
frontend/src/Pages/DebateRoom.tsx (2)
backend/models/debatevsbot.go (1)
Message(6-10)frontend/src/services/vsbot.ts (1)
judgeDebate(104-121)
backend/controllers/leaderboard.go (1)
backend/db/db.go (1)
MongoDatabase(16-16)
backend/cmd/server/main.go (2)
backend/websocket/gamification_handler.go (1)
GamificationWebSocketHandler(21-131)backend/routes/gamification.go (3)
AwardBadgeRouteHandler(9-11)UpdateScoreRouteHandler(13-15)GetGamificationLeaderboardRouteHandler(17-19)
backend/controllers/debatevsbot_controller.go (6)
frontend/src/services/vsbot.ts (1)
JudgeResponse(40-42)backend/db/db.go (1)
MongoDatabase(16-16)backend/models/user.go (1)
User(10-35)backend/models/gamification.go (3)
ScoreUpdate(29-36)UserBadge(20-26)GamificationEvent(47-55)backend/websocket/gamification.go (1)
BroadcastGamificationEvent(50-83)frontend/src/services/gamificationService.ts (1)
GamificationEvent(3-12)
🔇 Additional comments (17)
frontend/src/Pages/Profile.tsx (2)
715-725: LGTM! Score and streak display implementation is solid.The conditional rendering correctly handles edge cases (undefined values and zero scores), and the Flame icon is a nice touch for the streak visualization.
769-784: Badge names are correctly synchronized with backend.Verification confirms all five badge names in the frontend (Novice, Streak5, FactMaster, FirstWin, Debater10) match those validated in the backend gamification system (
backend/controllers/gamification_controller.go, lines 87–92). No action required.frontend/src/Pages/TeamDebateRoom.tsx (10)
121-123: LGTM! Environment-aware BASE_URL setup.The BASE_URL constant correctly derives from the environment variable with a sensible fallback, and the trailing slash removal prevents double-slash issues in URL construction.
186-190: Correctly implements debatePhaseRef to stabilize WebSocket connection.This ref pattern allows WebSocket handlers to access the current debate phase without triggering reconnections. This addresses the previous critical issue about WebSocket recreation on phase changes.
404-409: LGTM! Proper WebSocket URL construction with protocol switching.The WebSocket URL correctly derives from BASE_URL with automatic protocol selection (wss for https, ws otherwise) and properly sets query parameters.
524-565: LGTM! Proper team member synchronization.The case block is correctly wrapped, and the logic properly initializes ready status for new team members while updating team rosters.
569-584: LGTM! Correct role selection handling.The case block is properly wrapped and correctly determines whether the role selection is from the local or opponent team.
585-592: LGTM! Countdown initialization.The case block is properly wrapped and correctly handles the countdown start event.
711-721: LGTM! Correct use of debatePhaseRef in WebSocket handler.The case block is properly wrapped and correctly uses
debatePhaseRef.currentas a fallback when the phase is not provided in the message data.
722-731: LGTM! Proper live transcript handling.The case block is correctly wrapped and properly filters out the current user's own transcripts.
732-749: LGTM! Team status update handling.The case block is properly wrapped and correctly updates team member lists.
793-793: Correctly removes phase dependencies to stabilize WebSocket connection.This change addresses the previous critical issue by removing
debatePhaseanddebatefrom the dependency array, preventing WebSocket reconnection on every phase change. ThedebatePhaseRefprovides access to the current phase within handlers.frontend/src/Pages/DebateRoom.tsx (1)
533-548: Good error recovery, but inherits state management issue.The error handling correctly prevents the debate from getting stuck by inserting an error message and resetting
isBotTurn. However, this code shares the same state management pattern issue noted in lines 521-531 (callingadvanceTurnimmediately aftersetStatewithout waiting for the update to commit).Consider applying the same
useEffect-based solution recommended for the success case to ensure consistent state handling across both paths.backend/cmd/server/main.go (3)
106-106: LGTM!The gamification WebSocket endpoint is correctly placed outside the auth middleware group, consistent with the matchmaking handler pattern. The
GamificationWebSocketHandlerhandles authentication internally via JWT validation.
118-119: ****The endpoints do have proper authorization.
AwardBadgeexplicitly enforces role-based checks (line 76-77) to prevent non-admin users from awarding badges to other users.UpdateScoreonly allows users to modify their own scores (line 211), not arbitrary users. Both endpoints include anti-cheat measures:UpdateScorevalidates point ranges (0-1000) and enforces rate limiting, whileAwardBadgevalidates badge names against an allowlist.Likely an incorrect or invalid review comment.
114-120: Endpoints intentionally serve different purposes; add clarifying comments.The two leaderboard endpoints are not accidental duplicates:
/leaderboard(line 114): Traditional debate leaderboard sorted by user rating, returns all users with comprehensive stats (debates today, debating now, experts online)/api/leaderboard(line 120): Gamification leaderboard sorted by user score, returns limited users (default 50) with a simple responseThe implementations confirm this is intentional design. However, add inline comments to each handler clarifying their distinct purposes and response formats so future developers understand why both endpoints exist.
frontend/package.json (1)
36-36: ✓ react-confetti addition aligns with gamification feature.The new dependency is appropriate for the badge celebration UI (BadgeUnlocked.tsx) and the version constraint is reasonable.
| // Admin routes | ||
| routes.SetupAdminRoutes(router, "./config/config.prod.yml") | ||
| log.Println("Admin routes registered") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that admin routes apply authentication internally.
The SetupAdminRoutes function receives the base router instead of the auth group, meaning it doesn't automatically inherit JWT authentication middleware. Ensure that SetupAdminRoutes applies proper authentication and RBAC authorization internally.
Additionally, note that this uses the hardcoded config path "./config/config.prod.yml" mentioned in an earlier comment.
🏁 Script executed:
#!/bin/bash
# Description: Verify that SetupAdminRoutes applies authentication and authorization
echo "=== Checking SetupAdminRoutes implementation ==="
ast-grep --pattern $'func SetupAdminRoutes($$$) {
$$$
}'
echo -e "\n=== Searching for middleware usage in SetupAdminRoutes ==="
rg -n -C10 "func SetupAdminRoutes" backend/routes/
echo -e "\n=== Checking for AuthMiddleware or Casbin usage ==="
rg -n "AuthMiddleware|casbin|Enforce" backend/routes/admin*.goLength of output: 226
I need to search more broadly. Let me locate the SetupAdminRoutes function and understand the routes structure:
🏁 Script executed:
#!/bin/bash
# Search for SetupAdminRoutes across the entire codebase
echo "=== Searching for SetupAdminRoutes function ==="
rg -n "SetupAdminRoutes" --type go
echo -e "\n=== Finding route-related Go files ==="
fd --type f --extension go "route" backend/
echo -e "\n=== Checking backend directory structure ==="
find backend -type d -name "routes" 2>/dev/null | head -20
echo -e "\n=== List files in backend/routes if it exists ==="
if [ -d "backend/routes" ]; then
ls -la backend/routes/
fi
echo -e "\n=== Checking main.go imports ==="
rg -n "^import|^\\s+\"" backend/cmd/server/main.go | head -30Length of output: 1389
Let me verify if SetupAdminRoutes is actually defined in any routes file:
🏁 Script executed:
#!/bin/bash
# Check all functions defined in routes package
echo "=== All exported functions in routes package ==="
rg -n "^func [A-Z]" backend/routes/ --type go
echo -e "\n=== Search for 'AdminRoutes' or 'Admin' in routes files ==="
rg -n -i "admin" backend/routes/ --type go
echo -e "\n=== Check all file contents for function definitions ==="
rg -n "^func " backend/routes/ --type go | head -30Length of output: 4668
SetupAdminRoutes function is not defined in the codebase.
The call to routes.SetupAdminRoutes(router, "./config/config.prod.yml") references a function that does not exist. All exported functions in backend/routes/ are defined in auth.go, coach.go, debate.go, debatevsbot.go, gamification.go, leaderboard.go, profile.go, rooms.go, team.go, and transcriptroutes.go, but none of these files define SetupAdminRoutes. This will cause a compilation error.
You must create the SetupAdminRoutes function with proper authentication and authorization middleware applied before registering admin routes.
| "typescript": "^5.6.3", | ||
| "typescript-eslint": "^8.0.1", | ||
| "vite": "^5.4.1" | ||
| "vite": "^7.2.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
The upgrade from vite ^5.4.1 → ^7.2.2 spans two major versions and introduces breaking changes. Your previous review flagged three critical issues that must be resolved before merging:
-
Browser target default change (critical): Vite 7 defaults
build.targetto"baseline-widely-available"instead of"modules", raising minimum browser versions (Chrome 87→107, Safari 14→16). You must add an explicitbuild.targetto vite.config.ts matching your target browsers—otherwise the default will silently break support for older browsers. -
@vitejs/plugin-react-swc archived (line 59): The plugin repository was archived July 24, 2025—after Vite 7's June 24 release—with no official Vite 7 support. This is a maintenance and compatibility risk. Migrate to
@vitejs/plugin-react(maintained) or thoroughly test before deploy. -
@svgr/rollup incompatible (line 55): Rollup plugins don't work reliably in Vite's dev server. Replace with the Vite-native
vite-plugin-svgrfor HMR and Vite 7 compatibility.
Required actions:
- Update vite.config.ts: add explicit
build.target(or confirm current minimum browser support and set accordingly) - Replace
@vitejs/plugin-react-swcwith@vitejs/plugin-reactand update vite.config.ts imports - Replace
@svgr/rollupwithvite-plugin-svgrand update plugin configuration - Run full
npm run devandnpm run buildto verify HMR, dev server, and production bundle
Please confirm these migrations are complete and tested before this PR is merged.
I'll verify the claims in this review comment by checking the codebase and researching the specific issues mentioned.
Now I need to check the actual codebase to verify what's currently configured:
This will help verify the current state before finalizing the review comment.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check vite.config.ts content and frontend/package.json dependencies
echo "=== vite.config.ts ==="
find . -name "vite.config.ts" -o -name "vite.config.js" | head -1 | xargs cat 2>/dev/null || echo "vite.config not found"
echo -e "\n=== package.json (relevant deps) ==="
cat frontend/package.json | grep -A 2 -B 2 '"@vitejs/\|"@svgr/\|"vite"' || echo "Not found"
Length of output: 475
I need to locate the package.json to verify the exact dependency versions and confirm which packages are actually being used.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find package.json in the repo
echo "=== Locating package.json ==="
find . -name "package.json" -type f | head -5
echo -e "\n=== Checking repo root ==="
ls -la | grep -E "package.json|frontend"
Length of output: 140
---
🏁 Script executed:
```shell
#!/bin/bash
# Extract dependency versions from frontend/package.json
echo "=== Relevant dependencies from package.json ==="
grep -E '"vite"|"@vitejs/plugin-react|"@svgr/rollup"' ./frontend/package.json
echo -e "\n=== Full dependencies section ==="
sed -n '/"dependencies":/,/^ }/p' ./frontend/package.json | head -20
echo -e "\n=== Full devDependencies section (excerpt) ==="
sed -n '/"devDependencies":/,/^ }/p' ./frontend/package.json | grep -E '"vite"|"@vitejs|"@svgr"' -A 0 -B 0
Length of output: 1028
I need to verify the specific status and availability of the suggested alternatives.
🌐 Web query:
@vitejs/plugin-react-swc archived deprecated status 2025
💡 Result:
Short answer: The @vitejs/plugin-react-swc repository was archived (made read‑only) by the Vite org on July 24, 2025; users are directed to the consolidated @vitejs/plugin-react (monorepo) packages and the Vite docs recommend switching to @vitejs/plugin-react (or @vitejs/plugin-react with Oxc/rolldown guidance). [1][2]
Sources:
- GitHub repository archive notice (archived Jul 24, 2025). [1]
- Vite docs noting migration / use of @vitejs/plugin-react and related guidance. [2]
🌐 Web query:
vite-plugin-svgr alternative @svgr/rollup
💡 Result:
Here are practical alternatives to vite-plugin-svgr / @svgr/rollup for using SVGs as components or sprites in Vite projects:
- vite-svg-loader — loads SVGs as (React/Vue) components, supports SVGO and TS types. [1]
- vite-plugin-svg-icons (createSvgIconsPlugin) — generates SVG sprite maps and injects a virtual sprite (good for icon sets). [2]
- vite-plugin-svg-sprite / @spiriit/vite-plugin-svg-spritemap — sprite-based plugins that support importing symbols or fragments. [3][4]
- unplugin-icons — on‑demand icon components (Iconify + custom collections), usable with Vite and React. [5]
- Built-in Vite handling (import as URL/raw) — import SVGs as image URLs or raw strings and inline/parse them yourself (no plugin). (Vite docs / community guidance). [6]
If you need SVGR-specific JSX transformations (feature parity with @svgr/*), vite-svg-loader and some sprite plugins provide component exports; otherwise keep using @svgr/rollup but combine it with Vite’s plugin stack. Pick sprite-based plugins for many small icons (better runtime performance) and component loaders for single-file, JSX-editable SVGs.
Sources:
[1] vite-svg-loader (npm/github).
[2] vite-plugin-svg-icons (createSvgIconsPlugin) repo.
[3] vite-plugin-svg-sprite (npm).
[4] @spiriit/vite-plugin-svg-spritemap (GitHub).
[5] unplugin-icons (GitHub).
[6] Vite community/FAQ (discussion threads about importing SVGs).
---
🌐 Web query:
```web
vite-plugin-svgr npm package
💡 Result:
vite-plugin-svgr — quick summary
- What it is: a Vite plugin that transforms SVG files into React components using SVGR. [1]
- Install: npm i -D vite-plugin-svgr (or yarn/pnpm). [1]
- Basic usage (vite.config.ts/js):
import svgr from 'vite-plugin-svgr'
export default { plugins: [svgr()] }
Then import SVGs as components, e.g. import Logo from './logo.svg?react'. [1] - Options: supports svgrOptions, esbuildOptions, include/exclude patterns, and SVGO via @svgr/plugin-svgo. See package docs for details. [1]
- TypeScript: add /// for better typing. [1]
- Notes/compatibility: import query syntax and default-export behavior have varied across versions — check the repo/issues if you see import errors like “does not provide an export named 'ReactComponent'”. [2]
Sources:
[1] vite-plugin-svgr npm package page.
[2] pd4d10/vite-plugin-svgr GitHub (issues/discussion).
Vite 7 upgrade requires three critical fixes: missing build.target configuration and two unmaintained plugins.
The upgrade to Vite 7.2.2 introduces breaking changes that must be addressed:
-
Missing build.target in vite.config.ts: Vite 7 defaults build.target to 'baseline-widely-available', which raises minimum browser versions (Chrome 87→107, Safari 14→16). Add explicit
build.target: "modules"(or your target) to vite.config.ts to preserve current browser support. -
@vitejs/plugin-react-swc archived: The plugin repository was archived (made read‑only) on July 24, 2025. Replace with the maintained
@vitejs/plugin-reactin package.json and update the import in vite.config.ts. -
@svgr/rollup → vite-plugin-svgr: vite-plugin-svgr is a Vite plugin that transforms SVG files into React components using SVGR. Replace @svgr/rollup (a Rollup-specific plugin) with vite-plugin-svgr for Vite compatibility. Update vite.config.ts imports and test the import syntax (e.g.,
?reactquery modifier).
Required actions:
- vite.config.ts: Add
build: { target: "modules" }(confirm your actual target browsers) - package.json: Replace
@vitejs/plugin-react-swcwith@vitejs/plugin-react; replace@svgr/rollupwithvite-plugin-svgr - vite.config.ts: Update plugin imports and configuration accordingly
- Test: Run
npm run devandnpm run buildto verify
🤖 Prompt for AI Agents
In frontend/package.json around line 74, the project upgraded to Vite 7 but
package.json still references unmaintained/incompatible plugins and
vite.config.ts lacks an explicit build.target; replace the dependency
"@vitejs/plugin-react-swc" with the maintained "@vitejs/plugin-react" and
replace "@svgr/rollup" with "vite-plugin-svgr" in package.json, then update
vite.config.ts to import the new @vitejs/plugin-react and vite-plugin-svgr, add
build: { target: "modules" } (or your chosen target) to the vite configuration,
adjust any SVGR usage to the Vite plugin pattern (or ?react query as needed),
and finally run npm run dev and npm run build to verify everything works.
| const extractJSON = (response: string): string => { | ||
| if (!response) return "{}"; | ||
|
|
||
| // Try to extract JSON from markdown code fences | ||
| const fenceRegex = /```(?:json)?\s*([\s\S]*?)\s*```/; | ||
| const match = fenceRegex.exec(response); | ||
| if (match && match[1]) return match[1].trim(); | ||
| return response; | ||
| if (match && match[1]) { | ||
| return match[1].trim(); | ||
| } | ||
|
|
||
| // Try to find JSON object in the response | ||
| const jsonMatch = response.match(/\{[\s\S]*\}/); | ||
| if (jsonMatch) { | ||
| return jsonMatch[0]; | ||
| } | ||
|
|
||
| // If no JSON found, return empty object | ||
| console.warn("No JSON found in response:", response); | ||
| return "{}"; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider throwing an error instead of silently returning empty JSON.
Returning "{}" when no JSON is found masks extraction failures. The caller at line 563 may not detect this failure case, potentially leading to subtle bugs when the empty object is parsed and used as judgment data.
Consider this approach:
const extractJSON = (response: string): string => {
- if (!response) return "{}";
+ if (!response) throw new Error("Empty response received");
// Try to extract JSON from markdown code fences
const fenceRegex = /```(?:json)?\s*([\s\S]*?)\s*```/;
const match = fenceRegex.exec(response);
if (match && match[1]) {
return match[1].trim();
}
// Try to find JSON object in the response
const jsonMatch = response.match(/\{[\s\S]*\}/);
if (jsonMatch) {
return jsonMatch[0];
}
// If no JSON found, return empty object
- console.warn("No JSON found in response:", response);
- return "{}";
+ throw new Error("No JSON found in response");
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const extractJSON = (response: string): string => { | |
| if (!response) return "{}"; | |
| // Try to extract JSON from markdown code fences | |
| const fenceRegex = /```(?:json)?\s*([\s\S]*?)\s*```/; | |
| const match = fenceRegex.exec(response); | |
| if (match && match[1]) return match[1].trim(); | |
| return response; | |
| if (match && match[1]) { | |
| return match[1].trim(); | |
| } | |
| // Try to find JSON object in the response | |
| const jsonMatch = response.match(/\{[\s\S]*\}/); | |
| if (jsonMatch) { | |
| return jsonMatch[0]; | |
| } | |
| // If no JSON found, return empty object | |
| console.warn("No JSON found in response:", response); | |
| return "{}"; | |
| }; | |
| const extractJSON = (response: string): string => { | |
| if (!response) throw new Error("Empty response received"); | |
| // Try to extract JSON from markdown code fences | |
| const fenceRegex = / |
🤖 Prompt for AI Agents
frontend/src/Pages/DebateRoom.tsx around lines 198 to 217: the extractJSON
function currently logs a warning and returns the string "{}" when no JSON is
found, which hides extraction failures; replace the final console.warn + return
"{}" with throwing a descriptive error (e.g., throw new Error("No JSON found in
response")) so failures bubble up, and update the caller(s) (notably around line
563) to catch this error and handle it (validate/notify/abort) instead of
assuming valid JSON.
| setState((prev) => { | ||
| const updatedState = { | ||
| ...prev, | ||
| messages: [...prev.messages, botMessage], | ||
| }; | ||
| // Advance turn after bot responds | ||
| setTimeout(() => { | ||
| advanceTurn(updatedState); | ||
| }, 100); // Small delay to ensure state is updated | ||
| return updatedState; | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid setTimeout to sequence state updates.
The setTimeout with 100ms delay is a workaround for React's asynchronous setState and does not guarantee the state is committed. This pattern can lead to race conditions and will fail if the component unmounts during the timeout.
Instead, rely on React's state update guarantees by moving the advance logic into a useEffect that reacts to the messages array:
setState((prev) => {
const updatedState = {
...prev,
messages: [...prev.messages, botMessage],
};
- // Advance turn after bot responds
- setTimeout(() => {
- advanceTurn(updatedState);
- }, 100); // Small delay to ensure state is updated
return updatedState;
});Then add a useEffect to handle advancement after bot messages:
useEffect(() => {
const lastMessage = state.messages[state.messages.length - 1];
if (lastMessage?.sender === "Bot" && !state.isDebateEnded) {
advanceTurn(state);
}
}, [state.messages]);🤖 Prompt for AI Agents
In frontend/src/Pages/DebateRoom.tsx around lines 521 to 531, remove the
setTimeout-based sequencing inside setState and instead trigger advanceTurn from
a useEffect that watches state.messages; specifically, stop calling advanceTurn
inside the state updater, add a useEffect that reads the last message after
messages change, checks that lastMessage?.sender === "Bot" and that debate
hasn't ended, and then calls advanceTurn with the current/latest state (or
dispatch) so the turn advancement runs reliably without timeouts or race
conditions.
| try { | ||
| judgment = JSON.parse(jsonString); | ||
| } catch (parseError) { | ||
| console.error("JSON parse error:", parseError, "Trying to fix JSON..."); | ||
| // Try to fix common JSON issues | ||
| const fixedJson = jsonString | ||
| .replace(/'/g, '"') // Replace single quotes with double quotes | ||
| .replace(/(\w+):/g, '"$1":') // Add quotes to keys | ||
| .replace(/,\s*}/g, '}') // Remove trailing commas | ||
| .replace(/,\s*]/g, ']'); // Remove trailing commas in arrays | ||
| try { | ||
| judgment = JSON.parse(fixedJson); | ||
| } catch (e) { | ||
| throw new Error(`Failed to parse JSON: ${e}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brittle JSON repair logic with regex replacements.
The regex-based JSON fixing approach is fragile and can corrupt valid JSON:
replace(/'/g, '"')breaks strings containing apostrophesreplace(/(\w+):/g, '"$1":')fails on already-quoted keys or special characters- These transformations are not safe for arbitrary JSON structures
Consider these alternatives:
- Fix the backend to return valid JSON consistently (preferred)
- Remove the repair logic and let it fail fast with a clear error message
- Use a robust JSON repair library like
json-repairif backend fixes are not feasible
let judgment: JudgmentData;
try {
judgment = JSON.parse(jsonString);
} catch (parseError) {
- console.error("JSON parse error:", parseError, "Trying to fix JSON...");
- // Try to fix common JSON issues
- const fixedJson = jsonString
- .replace(/'/g, '"') // Replace single quotes with double quotes
- .replace(/(\w+):/g, '"$1":') // Add quotes to keys
- .replace(/,\s*}/g, '}') // Remove trailing commas
- .replace(/,\s*]/g, ']'); // Remove trailing commas in arrays
- try {
- judgment = JSON.parse(fixedJson);
- } catch (e) {
- throw new Error(`Failed to parse JSON: ${e}`);
- }
+ console.error("JSON parse error:", parseError);
+ throw new Error(`Invalid JSON from judge: ${parseError.message}`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| judgment = JSON.parse(jsonString); | |
| } catch (parseError) { | |
| console.error("JSON parse error:", parseError, "Trying to fix JSON..."); | |
| // Try to fix common JSON issues | |
| const fixedJson = jsonString | |
| .replace(/'/g, '"') // Replace single quotes with double quotes | |
| .replace(/(\w+):/g, '"$1":') // Add quotes to keys | |
| .replace(/,\s*}/g, '}') // Remove trailing commas | |
| .replace(/,\s*]/g, ']'); // Remove trailing commas in arrays | |
| try { | |
| judgment = JSON.parse(fixedJson); | |
| } catch (e) { | |
| throw new Error(`Failed to parse JSON: ${e}`); | |
| } | |
| } | |
| try { | |
| judgment = JSON.parse(jsonString); | |
| } catch (parseError) { | |
| console.error("JSON parse error:", parseError); | |
| throw new Error(`Invalid JSON from judge: ${parseError.message}`); | |
| } |
🤖 Prompt for AI Agents
frontend/src/Pages/DebateRoom.tsx around lines 567-582: the current regex-based
“repair” of JSON (replacing single quotes, mass-quoting keys, stripping trailing
commas) is brittle and can corrupt valid input; remove this regex repair block
and either (A) fix the backend to always return valid JSON (preferred), or (B)
fail fast by throwing a clear, contextual error when JSON.parse(jsonString)
fails (include the raw jsonString and parseError in the thrown/logged message),
or (C) if you must attempt recovery, replace the ad-hoc fixes with a tested
library such as json-repair: import the library, call its repair function on the
raw string, then JSON.parse the repaired output and handle any errors similarly.
Ensure only one of these strategies is implemented and remove the regex
replacements to avoid corrupting valid payloads.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
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)
196-204: Critical: DuplicatetoggleCamerafunction declaration.
toggleCamerais defined twice—at lines 196-204 and again at lines 967-985. This causes a redeclaration error at runtime.Keep only one implementation. The second one (lines 967-985) has better guard logic, so remove the first:
- const toggleCamera = useCallback(() => { - const stream = localStreamRef.current; - if (stream) { - stream.getVideoTracks().forEach((track) => { - track.enabled = !track.enabled; - }); - } - setIsCameraOn((prev) => !prev); - }, []);Also applies to: 967-985
♻️ Duplicate comments (4)
frontend/src/Pages/TeamDebateRoom.tsx (1)
1058-1071: Replace hardcoded localhost URL withBASE_URL.Line 1060 still uses
http://localhost:1313/submit-transcripts, which will fail in production. UseBASE_URLas introduced at the top of the file.- const response = await fetch(`http://localhost:1313/submit-transcripts`, { + const response = await fetch(`${BASE_URL}/submit-transcripts`, {frontend/src/Pages/DebateRoom.tsx (3)
198-217: Returning"{}"masks extraction failures.When no JSON is found, returning
"{}"silently allows the caller to proceed with an empty object, which may cause subtle bugs downstream when parsed asJudgmentData.Consider throwing an error instead:
- // If no JSON found, return empty object - console.warn("No JSON found in response:", response); - return "{}"; + throw new Error("No JSON found in response");Then handle the error in
judgeDebateResultto show a meaningful message.
519-529:setTimeoutto sequence state updates is unreliable.The 100ms delay (lines 525-527) does not guarantee state is committed before
advanceTurnruns. This can cause race conditions or fail if the component unmounts during the timeout.Move advancement logic to a
useEffectthat reacts to message changes:useEffect(() => { const lastMessage = state.messages[state.messages.length - 1]; if (lastMessage?.sender === "Bot" && !state.isDebateEnded && !botTurnRef.current) { advanceTurn(state); } }, [state.messages]);And remove the
setTimeoutcall from the bot handler.
565-580: Brittle regex-based JSON repair can corrupt valid JSON.The regex fixes at lines 570-574 are unsafe:
replace(/'/g, '"')breaks strings containing apostrophes (e.g.,"It's"→"It"s")replace(/(\w+):/g, '"$1":')double-quotes already-quoted keysEither fix the backend to return valid JSON, or use a tested library like
json-repair:- const fixedJson = jsonString - .replace(/'/g, '"') - .replace(/(\w+):/g, '"$1":') - .replace(/,\s*}/g, '}') - .replace(/,\s*]/g, ']'); - try { - judgment = JSON.parse(fixedJson); - } catch (e) { - throw new Error(`Failed to parse JSON: ${e}`); - } + throw new Error(`Invalid JSON from judge: ${parseError.message}`);
🧹 Nitpick comments (2)
frontend/src/Pages/TeamDebateRoom.tsx (2)
626-721: Excessive console.log statements in production code.Lines 627-646 contain verbose debugging output with
===banners and multipleconsole.logcalls. These should be removed or gated behind a debug flag before merging to production.- console.log("=== READY MESSAGE RECEIVED ==="); - console.log("Received ready message:", data); - console.log("Current user:", currentUser?.id); - console.log("Message userId:", data.userId); - console.log("Message teamId:", data.teamId); - console.log("Message assignedToTeam:", (data as any).assignedToTeam); - console.log("isTeam1:", isTeam1); - console.log("myTeamId:", myTeamId); - console.log( - "Team1Ready:", - data.team1Ready, - "Team2Ready:", - data.team2Ready - ); - console.log( - "Team1MembersCount:", - data.team1MembersCount, - "Team2MembersCount:", - data.team2MembersCount - ); + // Debug logs removed for production
722-743: Debug logs in phaseChange handler should be removed.Lines 725-728 contain verbose console logs with special characters. Remove for production.
- console.log( - `✓✓✓ RECEIVED PHASE CHANGE: ${newPhase} (previous: ${debatePhaseRef.current})` - ); - console.log(`Phase change data:`, data);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
frontend/package.json(2 hunks)frontend/src/Pages/DebateRoom.tsx(4 hunks)frontend/src/Pages/TeamDebateRoom.tsx(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/Pages/DebateRoom.tsx (3)
backend/services/ai.go (1)
Message(21-24)backend/models/debatevsbot.go (1)
Message(6-10)frontend/src/services/vsbot.ts (1)
judgeDebate(104-121)
🔇 Additional comments (5)
frontend/src/Pages/TeamDebateRoom.tsx (3)
121-123: BASE_URL and WebSocket URL construction look correct.The
BASE_URLconstant properly strips trailing slashes and falls back towindow.location.origin. The WebSocket URL correctly switches betweenwss:andws:based on the page protocol.Also applies to: 425-430
446-549: Switch case blocks correctly scope declarations.The
stateSynccase now properly wraps its body in braces, addressing the previous lint errors about switch-case declarations leaking scope.
744-764: Good use ofdebatePhaseReffor phase consistency.The
speechTexthandler correctly usesdebatePhaseRef.currentas the fallback phase (line 746), ensuring transcripts are associated with the correct phase even during async updates.frontend/src/Pages/DebateRoom.tsx (2)
530-546: Error recovery advances turn correctly.The error handler (lines 530-546) properly inserts an error message, resets
isBotTurn, and advances the turn to prevent the debate from getting stuck. This is a good resilience pattern.
586-625: Default judgment fallback provides graceful degradation.When judgment parsing fails, the code shows an error popup, sets default judgment data with clear error messages, and proceeds to show results after a delay. This prevents the UI from hanging.
| setState({ | ||
| ...currentState, | ||
| phaseStep: nextStep, | ||
| isBotTurn: nextEntity === "Bot", | ||
| timer: phases[currentState.currentPhase].time, | ||
| })); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale state risk when using spread with currentState.
Line 418-423 uses setState({ ...currentState, ... }) instead of the functional updater setState(prev => ...). Since currentState is captured at call time, rapid successive calls to advanceTurn may overwrite intermediate state updates.
Use the functional form to ensure state consistency:
- setState({
- ...currentState,
+ setState((prev) => ({
+ ...prev,
phaseStep: nextStep,
- isBotTurn: nextEntity === "Bot",
- timer: phases[currentState.currentPhase].time,
- });
+ isBotTurn: phaseSequences[prev.currentPhase][nextStep] === prev.userStance ? false : true,
+ timer: phases[prev.currentPhase].time,
+ }));Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/Pages/DebateRoom.tsx around lines 418-423, the setState call
uses the captured currentState and spread syntax which can cause stale state
when advanceTurn is called rapidly; replace it with the functional updater form
setState(prev => ({ ...prev, phaseStep: nextStep, isBotTurn: nextEntity ===
"Bot", timer: phases[prev.currentPhase].time })) so updates are applied to the
latest state and use prev.currentPhase for deriving timer.
| const debatePhaseRef = useRef<DebatePhase>(debatePhase); | ||
|
|
||
| useEffect(() => { | ||
| debatePhaseRef.current = debatePhase; | ||
| }, [debatePhase]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Duplicate debatePhaseRef declaration causes runtime error.
debatePhaseRef is declared twice—once at lines 186-190 and again at lines 217-220. This will cause a JavaScript runtime error ("Identifier 'debatePhaseRef' has already been declared").
Remove the duplicate declaration at lines 217-220:
- const debatePhaseRef = useRef<DebatePhase>(debatePhase);
- useEffect(() => {
- debatePhaseRef.current = debatePhase;
- }, [debatePhase]);
-Also applies to: 217-220
🤖 Prompt for AI Agents
In frontend/src/Pages/TeamDebateRoom.tsx around lines 186-190 and 217-220, there
are two declarations of debatePhaseRef which causes a runtime "Identifier ...
already been declared" error; remove the duplicate declaration at lines 217-220
so only the initial useRef at 186-190 remains, and verify any subsequent code
that referred to the removed declaration now uses the original debatePhaseRef
(update imports/closures or references if needed) so the ref assignment in the
existing useEffect continues to keep debatePhaseRef.current in sync with
debatePhase.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.