Fix issue 156 gamification #198
Conversation
📝 WalkthroughWalkthroughAdded "concede" as a new debate outcome in the backend gamification system with 0 points and blocking automatic badge checks. Frontend introduces GamificationOverlay component for real-time badge/score notifications via WebSocket, centralizing gamification UI logic and adding Top10 badge for reaching leaderboard top 10. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as Frontend App
participant GOv as GamificationOverlay
participant WS as WebSocket
participant Server
participant Leaderboard as Leaderboard API
participant BadgeUI as BadgeUnlocked Modal
User->>App: Load App
App->>GOv: Mount GamificationOverlay
GOv->>WS: Connect WebSocket (with user token)
WS->>Server: Subscribe to gamification events
User->>Server: Complete debate/trigger action
Server->>WS: Emit GamificationEvent
WS->>GOv: Receive event
alt Badge Award
GOv->>BadgeUI: Open BadgeUnlocked modal (badge name)
BadgeUI->>User: Display badge
User->>BadgeUI: Close
else Score Update
GOv->>Leaderboard: Fetch user rank
Leaderboard->>GOv: Return new rank
GOv->>GOv: Compare to previous rank
alt Entered Top 10
GOv->>BadgeUI: Trigger Top10 badge modal
BadgeUI->>User: Display badge
end
GOv->>GOv: Update previousRank
end
User->>App: Unmount
GOv->>WS: Close connection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/src/components/GamificationOverlay.tsx:
- Around line 27-32: The initial leaderboard fetch
(fetchGamificationLeaderboard) can finish after the WebSocket begins delivering
score_updated events, leaving previousRankRef.current null and preventing the
first top-10 celebration; either await the initial fetch before
creating/connecting the WebSocket so previousRankRef is populated before any
events are handled, or initialize previousRankRef.current to a safe default
(e.g., Infinity) and update the score_updated handler to use that sentinel (and
compare using !== undefined/null as appropriate) so the first rank delta is
evaluated correctly; locate references to fetchGamificationLeaderboard,
previousRankRef, and the score_updated WebSocket handler to implement the chosen
fix.
🧹 Nitpick comments (5)
frontend/src/components/SavedTranscripts.tsx (2)
139-151: Concede result icon mapping looks consistent with loss UIRouting
case 'concede'to the same icon aslossmatches the stated UX requirement. Consider tightening the type to prevent future drift.Proposed refactor (optional)
- const getResultIcon = (result: string) => { + const getResultIcon = (result: SavedDebateTranscript['result']) => { switch (result) { case 'win': return <Trophy className='w-4 h-4 text-yellow-500' />; case 'loss': case 'concede': return <XCircle className='w-4 h-4 text-red-500' />; case 'draw': return <MinusCircle className='w-4 h-4 text-gray-500' />; default: return <Clock className='w-4 h-4 text-blue-500' />; } };
153-165: Concede result color mapping looks consistent with loss UISame as the icon mapping:
concedeinheritinglossstyling is consistent with the PR behavior. Same optional typing tightening applies here too.Proposed refactor (optional)
- const getResultColor = (result: string) => { + const getResultColor = (result: SavedDebateTranscript['result']) => { switch (result) { case 'win': return 'bg-green-100 text-green-800 border-green-200'; case 'loss': case 'concede': return 'bg-red-100 text-red-800 border-red-200'; case 'draw': return 'bg-gray-100 text-gray-800 border-gray-200'; default: return 'bg-blue-100 text-blue-800 border-blue-200'; } };backend/controllers/debatevsbot_controller.go (1)
583-611: Concede transcript result is now “concede” (good), but update the stale “loss” commentPersisting
"concede"(Line 597) and callingupdateGamificationAfterBotDebate(..., "concede", ...)(Line 610) aligns backend state with the new frontend union type. The comment on Line 584 (“We treat concession as a loss”) is now misleading and should be updated (even if the UI renders it like a loss).Proposed comment fix (optional)
- // Save transcript to history - // We treat concession as a loss + // Save transcript to history + // Concession is stored as a distinct result ("concede"), though the UI may render it like a loss.frontend/src/services/transcriptService.ts (1)
5-21: Good type update; consider propagating “concede” to other transcript/stat types for consistencyAdding
'concede'toSavedDebateTranscript.result(Line 12) matches the backend changes. Two follow-ups to consider:
- If
/debate-statscan return concede inrecentDebates, updateDebateStats.recentDebates.resulttoo.- Tighten
SaveTranscriptRequest.resultfromstringto the same union to prevent bad values.Proposed typing alignment
export interface SaveTranscriptRequest { debateType: 'user_vs_bot' | 'user_vs_user'; topic: string; opponent: string; - result?: string; + result?: SavedDebateTranscript['result']; messages: Array<{ sender: string; text: string; phase?: string; }>; transcripts?: Record<string, string>; } export interface DebateStats { totalDebates: number; wins: number; losses: number; draws: number; winRate: number; recentDebates: Array<{ topic: string; - result: 'win' | 'loss' | 'draw' | 'pending'; + result: SavedDebateTranscript['result']; opponent: string; debateType: 'user_vs_bot' | 'user_vs_user'; date: string; eloChange?: number; // We'll need to add this to the backend }>; }frontend/src/components/GamificationOverlay.tsx (1)
38-71: Consider adding WebSocket error handling and reconnection logic.The WebSocket is created without
onErrororonClosecallbacks. If the connection fails or drops unexpectedly (e.g., network issues), users won't receive gamification events until they refresh the page or trigger an effect re-run.Suggested improvement
const ws = createGamificationWebSocket( token, async (event: GamificationEvent) => { // ... existing handler - } + }, + (error) => { + console.error("Gamification WebSocket error:", error); + }, + () => { + // Optionally implement reconnection logic here + console.log("Gamification WebSocket closed"); + } );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/controllers/debatevsbot_controller.gofrontend/src/App.tsxfrontend/src/Pages/Leaderboard.tsxfrontend/src/components/BadgeUnlocked.tsxfrontend/src/components/GamificationOverlay.tsxfrontend/src/components/SavedTranscripts.tsxfrontend/src/services/transcriptService.ts
💤 Files with no reviewable changes (1)
- frontend/src/Pages/Leaderboard.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/App.tsx (1)
frontend/src/components/ui/toaster.tsx (1)
Toaster(11-33)
frontend/src/components/GamificationOverlay.tsx (2)
frontend/src/hooks/useUser.ts (1)
useUser(12-112)frontend/src/services/gamificationService.ts (2)
fetchGamificationLeaderboard(41-55)createGamificationWebSocket(93-129)
🔇 Additional comments (5)
backend/controllers/debatevsbot_controller.go (2)
342-361: Concede branch correctly awards 0 points, but ensure downstream UI/event handling matches “no reward” intent
case "concede"setspointsToAdd = 0and a distinctaction = "debate_concede"(Line 352-355), which is good. One thing to double-check: the function still inserts a score_updates record and broadcasts ascore_updatedevent later even when points are 0—if product wants “no gamification popup/toast at all on concede”, clients should filter byaction === "debate_concede"(or you can skip broadcasting whenpointsToAdd == 0).
453-456: Skipping automatic badge checks on concede matches the PR requirementThe
if resultStatus != "concede"guard (Line 454-456) prevents side-effect badge awarding on concede, including badges that could otherwise be granted due to existing score/streak thresholds.frontend/src/components/BadgeUnlocked.tsx (1)
1-29: Top10 badge mapping looks fine; verify badgeName string matches backend exactlyAdding
Top10tobadgeIcons/badgeDescriptionsis clean. Just ensure the emittedbadgeNamefrom backend/websocket is exactly"Top10"(same casing), otherwise the UI will fall back to the default icon/description.frontend/src/App.tsx (1)
31-33: Good global wiring; ensure GamificationOverlay is auth-gated to avoid public-page websocket churnRendering
<GamificationOverlay />at the app root is a good way to centralize gamification UI. Please verify it behaves safely when the user is logged out (e.g., no websocket connect / no errors / no toasts on the public home/auth pages). If it doesn’t already gate internally, consider gating it onAuthContext.isAuthenticated(either inside the component or via a small wrapper).Also applies to: 110-119
frontend/src/components/GamificationOverlay.tsx (1)
82-88: LGTM!The render logic correctly passes the required props to
BadgeUnlocked. TheonClosehandler appropriately preserves thebadgeNamewhile closing the modal, allowing any closing animation to display the correct badge.
| fetchGamificationLeaderboard(token).then((data) => { | ||
| const myEntry = data.debaters.find(d => d.id === user.id); | ||
| if (myEntry) { | ||
| previousRankRef.current = myEntry.rank; | ||
| } | ||
| }).catch(console.error); |
There was a problem hiding this comment.
Race condition may cause missed top 10 celebration.
The initial leaderboard fetch is asynchronous and runs concurrently with WebSocket setup. If a score_updated event arrives before this fetch completes, previousRankRef.current will still be null, causing the check at line 58 (oldRank !== null) to fail. This means the user won't see the top 10 celebration on their first ranking update.
Consider awaiting the initial rank fetch before establishing the WebSocket, or initializing previousRankRef to a safe default (e.g., Infinity) so the comparison works correctly:
Suggested approach
- const previousRankRef = useRef<number | null>(null);
+ const previousRankRef = useRef<number>(Infinity);And update the check accordingly:
- if (newRank <= 10 && oldRank !== null && oldRank > 10) {
+ if (newRank <= 10 && oldRank > 10) {🤖 Prompt for AI Agents
In @frontend/src/components/GamificationOverlay.tsx around lines 27 - 32, The
initial leaderboard fetch (fetchGamificationLeaderboard) can finish after the
WebSocket begins delivering score_updated events, leaving
previousRankRef.current null and preventing the first top-10 celebration; either
await the initial fetch before creating/connecting the WebSocket so
previousRankRef is populated before any events are handled, or initialize
previousRankRef.current to a safe default (e.g., Infinity) and update the
score_updated handler to use that sentinel (and compare using !== undefined/null
as appropriate) so the first rank delta is evaluated correctly; locate
references to fetchGamificationLeaderboard, previousRankRef, and the
score_updated WebSocket handler to implement the chosen fix.
1 participant
Changes made
Videos
533260979-2392df04-2d08-4d5b-8c3f-73c0b56b4fc1.mov
My.Movie.mp4