Feat/ai debate judge post debate scorecard #164
Conversation
📝 WalkthroughWalkthroughThis PR introduces AI-powered debate judging via Gemini integration, adds centralized storage utility helpers across the frontend to standardize localStorage access, implements WebSocket URL building utilities, refactors JSON parsing with safe fallbacks across multiple components, and provides production configuration files with JWT token generation tooling. Changes
Sequence DiagramsequenceDiagram
participant Client as Client/<br/>OnlineDebateRoom
participant Server as Server/<br/>JudgeDebateHandler
participant DB as MongoDB
participant AI as Gemini AI<br/>API
participant Cache as sessionStorage/<br/>state
Client->>Server: POST /debate/:id/judge<br/>(with context)
activate Server
Server->>DB: Check for existing<br/>judgement (debateId)
activate DB
DB-->>Server: Found/Not Found
deactivate DB
alt Judgement exists
Server-->>Client: 409 Conflict
else No judgement
Server->>DB: Fetch debate transcript
activate DB
DB-->>Server: SavedDebateTranscript
deactivate DB
Server->>Server: Build formatted<br/>transcript
Server->>AI: POST with JSON schema<br/>(90s timeout)
activate AI
note over AI: Strict JSON parsing<br/>with lenient fallback
AI-->>Server: JudgementResult<br/>{winner, scores, summary}
deactivate AI
alt AI success
Server->>DB: Validate & insert<br/>debate_judgement
activate DB
DB-->>Server: Inserted / 409 race
deactivate DB
alt Insert success
Server-->>Client: 200 OK<br/>+ judgement JSON
else Race condition
Server-->>Client: 409 Conflict
end
else AI timeout/error
Server-->>Client: 202 Accepted<br/>(non-fatal message)
end
end
deactivate Server
Client->>Client: safeParse response
Client->>Cache: Store judgement result
Client->>Client: Show DebateScorecard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 14
🧹 Nitpick comments (25)
frontend/src/services/profileService.ts (1)
43-43: Align parsing pattern across all service functions.Both
updateProfileandgetLeaderboardstill callresponse.json()directly, whilegetProfilenow uses the text-then-parse pattern. For consistency and resilience, consider refactoring these functions to use the same defensive parsing approach (ideally thesafeParseutility).🔎 Proposed refactor for consistency
export const updateProfile = async ( ... ) => { const response = await fetch(`${baseURL}/user/updateprofile`, { ... }); + const text = await response.text(); + const data = safeParse(text); + if (!response.ok) { throw new Error("Failed to update profile"); } - return response.json(); + return data; }; export const getLeaderboard = async () => { const response = await fetch(`${baseURL}/leaderboard`, { method: "GET", }); + const text = await response.text(); + const data = safeParse(text); + if (!response.ok) { throw new Error("Failed to fetch leaderboard"); } - return response.json(); + return data; };Also applies to: 52-52
backend/cmd/maketoken/main.go (1)
1-24: LGTM! Useful development tool.The JWT token generator provides a simple way for developers to obtain authentication tokens for testing. The hardcoded test credentials align with seeded test data.
💡 Optional: Accept user ID and email as CLI arguments
This would make the tool more flexible for testing with different users:
package main import ( + "flag" "fmt" "arguehub/config" "arguehub/utils" ) func main() { + userID := flag.String("user", "test-user-id", "User ID for token") + email := flag.String("email", "user1@example.com", "Email for token") + flag.Parse() + cfg, err := config.LoadConfig("./config/config.prod.yml") if err != nil { fmt.Println("failed to load config:", err) return } utils.SetJWTSecret(cfg.JWT.Secret) - // Generate a token for the seeded test user (user1@example.com). - token, err := utils.GenerateJWTToken("test-user-id", "user1@example.com") + token, err := utils.GenerateJWTToken(*userID, *email) if err != nil { fmt.Println("failed to generate token:", err) return } fmt.Println(token) }backend/controllers/transcript_controller.go (1)
307-333: Mock judgement insertion logic looks reasonable for dev/test purposes.The code correctly:
- Uses a bounded context with proper cleanup
- Finds the most recent matching transcript using sort
- Constructs a mock judgement with the expected score structure
One minor note: the comment on line 331 says "Ignore duplicate key errors" but the code ignores all errors. Consider logging non-duplicate errors for debugging:
🔎 Optional: Log unexpected errors
// Ignore duplicate key errors to be idempotent - _, _ = jColl.InsertOne(ctx2, insertDoc) + if _, err := jColl.InsertOne(ctx2, insertDoc); err != nil && !mongo.IsDuplicateKeyError(err) { + log.Printf("warning: failed to insert mock judgement: %v", err) + } }backend/services/transcript.go (1)
61-69: Fallback side assignment may not correctly alternate between participants.The fallback logic assigns "Side A" only when
b.Len() == 0(first message), meaning all subsequent unmatched senders become "Side B". If the debate has multiple exchanges from two distinct participants who don't matchsideA/sideBstrings, they'll all appear as "Side B" after the first message.Consider tracking the first two distinct senders explicitly if alternation is expected.
backend/controllers/judge_controller.go (1)
81-97: Redundant struct creation:docfields are unused in favor ofinsertDoc.The
models.DebateJudgementstruct (doc) is created but onlydoc.ID,doc.DebateID, etc. are extracted and placed intoinsertDoc. Thedoc.Scoresfield (empty map) is never used. Consider simplifying by buildinginsertDocdirectly without the intermediate struct.🔎 Simplified approach
- doc := models.DebateJudgement{ - ID: primitive.NewObjectID(), - DebateID: debateID, - Winner: result.Winner, - Scores: map[string]float64{}, // keep legacy field for compatibility - Summary: result.Summary, - CreatedAt: time.Now(), - } - // Also store the detailed per-side scores in a separate field via raw BSON insert insertDoc := bson.M{ - "_id": doc.ID, - "debateId": doc.DebateID, - "winner": doc.Winner, + "_id": primitive.NewObjectID(), + "debateId": debateID, + "winner": result.Winner, "scores": scoresMap, - "summary": doc.Summary, - "createdAt": doc.CreatedAt, + "summary": result.Summary, + "createdAt": time.Now(), }frontend/src/utils/auth.ts (1)
1-15: Auth token storage refactor is correct; minor cleanup possibleDelegating
setAuthToken/getAuthTokentosetLocalString/getLocalStringpreserves semantics and centralizes error handling.clearAuthToken’s try/catch is fine for SSR/non‑browser contexts. You can optionally drop the unusedsetLocalJSONimport to keep the module tidy.frontend/src/components/Matchmaking.tsx (1)
2-2: Robust pool_update parsing; consider aligning WS URL/token handlingThe new
safeParse<MatchmakingMessage>plus normalization ofmessage.poolto an array (handling array vs. JSON string vs. null) is solid and should prevent crashes on malformed payloads. TheshouldBeInPoolcheck againststartedMatchmakingis also correct.Given the rest of the app is now standardizing on helpers, consider:
- Using
buildWsUrl('/ws/matchmaking', { token })instead of the hard‑codedws://localhost:1313/....- Optionally reading the token via
getAuthToken()orgetLocalString('token')for consistency withauth.tsand other pages.These are consistency/refactor points, not blockers.
Also applies to: 73-100
frontend/src/components/DebateScorecard.tsx (1)
1-88: Scorecard implementation is good; consider reusing shared judge typesThe component cleanly handles both “no judgement” and populated states, and the Side A/B breakdown matches the backend
JudgementResultshape.Since
SideScores,JudgementScores, andJudgementResultare already defined infrontend/src/services/judge.ts, consider importing those instead of redefining them here to avoid future drift if the API evolves.frontend/src/Pages/OnlineDebateRoom.tsx (1)
15-19: WS URL centralization and safe message parsing look solid; WS_BASE_URL can be droppedUsing
buildWsUrl('/ws', { room: roomId, token })andsafeParse<WSMessage>inrws.onmessageis a nice hardening step: invalid frames are ignored with a warning instead of crashing the room, and WS URLs are now consistent with the rest of the app.
WS_BASE_URLis now defined but unused in this file; you can safely remove it to avoid dead code.Also applies to: 146-151, 1115-1117, 1136-1141
frontend/src/components/ChatRoom.tsx (1)
2-3: Safer chat WS connection; optional consistency tweaks
- Constructing the WS URL via
buildWsUrl(/chat/${roomId}, { token })and usingsafeParsefor incoming messages nicely aligns ChatRoom with the new WS/url and parsing utilities.- Early‑returning on invalid
datakeeps the switch clean and avoids runtime errors on malformed frames.For consistency with the rest of the app, you might optionally:
- Read the token via
getAuthToken()orgetLocalString('token')instead oflocalStorage.getItem('token').Functionally this is fine as‑is.
Also applies to: 49-50, 60-63
frontend/src/Pages/Leaderboard.tsx (1)
174-186: Consider extracting the token refresh logic to avoid duplication.The token retrieval inside the
setTimeoutcallback (line 176) re-fetches to ensure freshness, which is correct. However, the cleanup function returned on line 186 is inside thescore_updatedhandler and won't properly clear the timer if the component unmounts during the 2-second delay.🔎 Suggested improvement
Store the timer ref at component level to ensure proper cleanup:
+ const reloadTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null); + // In the score_updated handler: - const reloadTimer = setTimeout(async () => { + if (reloadTimerRef.current) clearTimeout(reloadTimerRef.current); + reloadTimerRef.current = setTimeout(async () => { // ... existing logic }, 2000); - return () => clearTimeout(reloadTimer);And in the cleanup effect:
return () => { if (wsRef.current) { wsRef.current.close(); wsRef.current = null; } + if (reloadTimerRef.current) { + clearTimeout(reloadTimerRef.current); + reloadTimerRef.current = null; + } };frontend/src/services/teamDebateService.ts (1)
8-12: Consider using the sharedgetAuthTokenfrom@/utils/auth.tsinstead of a local definition.The relevant code snippets show that
frontend/src/utils/auth.tsalready exports agetAuthTokenfunction that usesgetLocalString('token'). Maintaining a separate local implementation leads to duplication and potential inconsistency (the auth.ts version returnsnull, while this returns an empty string).🔎 Proposed change
import { Team } from "./teamService"; - -// Team debate service for API calls -const API_BASE_URL = - (import.meta.env.VITE_BASE_URL as string | undefined)?.replace(/\/$/, "") ?? - window.location.origin; - -import { getLocalString } from '@/utils/storage'; - -function getAuthToken(): string { - return getLocalString('token') || ''; -} +import { getAuthToken } from '@/utils/auth'; +// Team debate service for API calls +const API_BASE_URL = + (import.meta.env.VITE_BASE_URL as string | undefined)?.replace(/\/$/, "") ?? + window.location.origin;Note: You may need to adjust callers if they rely on the empty string fallback instead of null.
frontend/src/components/DebatePopup.tsx (2)
28-33: Consider using the storage helper for consistency.The PR introduces
getLocalStringfrom@/utils/storageto standardize localStorage access across the codebase. For consistency with changes in other files (e.g., useUser.ts, AdminDashboard.tsx), consider replacing:- const token = localStorage.getItem('token'); + const token = getLocalString('token');Don't forget to add the import at the top of the file.
</review_comment_end>
45-62: Consider using safeParse and improving error UX.The nested try/catch blocks for error parsing could be simplified using the
safeParseutility introduced in this PR:🔎 Suggested refactor
if (!response.ok) { - // Try to parse an error message from the backend - let errMsg = 'Error creating room.'; - try { - const payload = await response.json(); - if (payload?.error) errMsg = payload.error; - else if (payload?.message) errMsg = payload.message; - } catch (e) { - // Not JSON, fallback to text - try { - const text = await response.text(); - if (text) errMsg = text; - } catch {} - } + const text = await response.text(); + const payload = safeParse<{error?: string; message?: string}>(text, null); + const errMsg = payload?.error || payload?.message || text || 'Error creating room.'; console.error('Create room failed:', response.status, errMsg); alert(errMsg); return; }Additionally, consider replacing
alert()with a toast notification or inline error display for better UX (alerts block user interaction and aren't styled).</review_comment_end>
frontend/src/hooks/useUser.ts (1)
19-30: Use getLocalJSON for consistency.The hydration logic at line 20 uses
getLocalString, but line 23 manually parses withJSON.parse. The PR introducesgetLocalJSONwhich handles parsing with error handling and fallback logic built-in:🔎 Suggested refactor
useEffect(() => { if (!user) { - const cachedUser = getLocalString(USER_CACHE_KEY); - if (cachedUser) { - try { - const parsedUser = JSON.parse(cachedUser); - setUser(parsedUser); - } catch (error) { - console.error("Failed to parse cached user profile:", error); - try { localStorage.removeItem(USER_CACHE_KEY); } catch {} - } - } + const cachedUser = getLocalJSON(USER_CACHE_KEY); + if (cachedUser) { + setUser(cachedUser); + } } }, [user, setUser]);This simplifies the code and leverages the built-in error handling in
getLocalJSON(which already logs warnings and removes corrupt entries).</review_comment_end>
frontend/src/Pages/DebateRoom.tsx (2)
228-261: Simplify state initialization with getLocalJSON.The initialization manually calls
JSON.parseat line 232 and implements field-by-field defaulting. This could be simplified usinggetLocalJSONfrom the storage utilities:🔎 Suggested refactor
const [state, setState] = useState<DebateState>(() => { - const savedState = getLocalString(debateKey); - if (savedState) { - try { - const parsed = JSON.parse(savedState) as Partial<DebateState> | null; - if (parsed && typeof parsed === 'object') { - return { - messages: parsed.messages ?? [], - currentPhase: parsed.currentPhase ?? 0, - phaseStep: parsed.phaseStep ?? 0, - isBotTurn: parsed.isBotTurn ?? false, - userStance: parsed.userStance ?? "", - botStance: parsed.botStance ?? "", - timer: parsed.timer ?? phases[0].time, - isDebateEnded: parsed.isDebateEnded ?? false, - } as DebateState; - } - } catch (err) { - console.warn('Failed to parse saved debate state, clearing corrupt value', err); - try { localStorage.removeItem(debateKey); } catch {} - } - } + const parsed = getLocalJSON<Partial<DebateState>>(debateKey, null); + if (parsed && typeof parsed === 'object') { + return { + messages: parsed.messages ?? [], + currentPhase: parsed.currentPhase ?? 0, + phaseStep: parsed.phaseStep ?? 0, + isBotTurn: parsed.isBotTurn ?? false, + userStance: parsed.userStance ?? "", + botStance: parsed.botStance ?? "", + timer: parsed.timer ?? phases[0].time, + isDebateEnded: parsed.isDebateEnded ?? false, + } as DebateState; + } return { messages: [], currentPhase: 0, phaseStep: 0, isBotTurn: false, userStance: "", botStance: "", timer: phases[0].time, isDebateEnded: false, }; });
getLocalJSONalready handles parse errors, logging, and corrupt data removal.</review_comment_end>
585-597: JSON repair logic is fragile—consider alternatives.The regex-based JSON repair at lines 588-592 is a brittle workaround that may not handle all malformed JSON cases (e.g., nested objects, escaped quotes, complex strings). While this defensive coding improves resilience, consider these alternatives:
- Fix the root cause: Ensure the backend (Gemini judging response) returns valid JSON
- Use a JSON repair library: Libraries like
json-repairorsecure-json-parsehandle edge cases better- Document the workaround: Add a comment explaining why repair is needed and the known limitations
If the repair is a temporary measure for development/testing, add a TODO indicating this should be removed once backend JSON is validated.
</review_comment_end>
frontend/src/Pages/Game.tsx (1)
276-310: LGTM! WebSocket refactor improves reliability.The migration to
buildWsUrlcentralizes WebSocket URL construction and the enhanced lifecycle logging (onopen, onerror, onclose) will aid debugging. The cleanup properly closes the WebSocket on unmount.Optional improvement: The nested try/catch for message parsing (lines 289-295) could leverage the
safeParseutility introduced in this PR:🔎 Suggested refactor
+import { safeParse } from '@/utils/safeParse'; + ws.onmessage = (event) => { try { - const raw = event.data; - let parsed: any = null; - try { - parsed = JSON.parse(typeof raw === 'string' ? raw : String(raw)); - } catch (err) { - console.warn('Game: failed to parse WS message', err); - return; - } + const raw = typeof event.data === 'string' ? event.data : String(event.data); + const parsed = safeParse(raw, null); + if (!parsed) return; handleWebSocketMessage(parsed); } catch (error) { console.error("Failed to handle WebSocket message:", error); } };</review_comment_end>
frontend/src/utils/storage.ts (1)
28-44: Consider adding a removeItem helper for completeness.The storage utilities provide getters and setters but no removal helper. Some files in the PR still use
localStorage.removeItemdirectly (e.g., useUser.ts line 27, AdminDashboard.tsx lines 381-382). Adding a helper would complete the abstraction:🔎 Suggested addition
export const removeLocalItem = (key: string) => { if (typeof window === 'undefined') return; try { localStorage.removeItem(key); } catch (err) { console.error('Failed to remove from localStorage', key, err); } };This would allow consistent error handling and SSR safety for removal operations.
</review_comment_end>
frontend/src/Pages/Profile.tsx (2)
200-208: Error detection via string matching is fragile.Lines 203-205 detect authentication failures by checking if the error message includes 'invalid', 'expired', or 'authorization'. This approach is brittle because:
- It's case-sensitive (
.toLowerCase()helps but English-only)- Backend message changes could break detection
- Not robust against localization or message variations
Better alternatives:
- Backend returns error codes: Modify backend to return structured errors with codes (e.g.,
{code: "TOKEN_EXPIRED", message: "..."})- Check HTTP status codes: Use response.status (e.g., 401) to detect auth failures
- Centralize error handling: Create an error handler utility that interprets backend errors consistently
For now, this works, but consider improving backend error responses for more reliable error detection.
</review_comment_end>
339-359: Use navigate() for client-side routing.Line 344 uses
window.location.href = '/authentication'instead of the importednavigate()function (line 76). This triggers a full page reload, losing React state and causing unnecessary re-initialization.🔎 Suggested refactor
+ const navigate = useNavigate(); + if (!dashboard) { return ( <div className="p-4 flex flex-col items-center justify-center min-h-[200px]"> <div className="text-center"> <div className="text-red-500 text-sm mb-4">{errorMessage}</div> <div className="flex gap-2 justify-center"> <Button - onClick={() => window.location.href = '/authentication'} + onClick={() => navigate('/authentication')} variant="default" size="sm" > Sign In </Button>This provides smoother client-side navigation without losing application state.
</review_comment_end>
frontend/src/services/judge.ts (3)
5-21: Duplicate type definitions withDebateScorecard.tsx.These interfaces (
SideScores,JudgementScores,JudgementResult) are duplicated infrontend/src/components/DebateScorecard.tsx. Consider extracting them to a shared types file (e.g.,@/types/judgement.ts) and importing from there to maintain a single source of truth.
41-46: Unused function:extractErrorMessage.This helper function is defined but never called anywhere in this file. Either remove it or integrate it into the error handling logic (lines 66-67 and 87-88).
🔎 Proposed fix: Remove or use the function
If you want to use it:
- const msg = data?.error || data?.message || res.statusText || 'Failed to judge debate'; + const msg = extractErrorMessage(data?.error || data?.message, res.statusText || 'Failed to judge debate');Or simply remove lines 41-46 if not needed.
48-68: Consider adding runtime type validation for the response.The parsed response is directly cast to
JudgementResultwithout validating its structure. If the API returns malformed data, this could cause runtime errors downstream. Consider adding basic validation or using a schema validation library (e.g., zod).frontend/src/hooks/useDebateWS.ts (1)
121-132: Fragile access to WebSocket URL property.Accessing internal/undocumented properties (
url,URL,_url) via type casting is brittle and may break with library updates. Consider storing the URL explicitly when creating the socket.🔎 Proposed fix: Track URL explicitly
const wsRef = useRef<ReconnectingWebSocket | null>(null); + const wsUrlRef = useRef<string | null>(null); const creatingRef = useRef(false);Then when creating the socket:
rws = new ReconnectingWebSocket(wsUrl, [], { ... }); + wsUrlRef.current = wsUrl;And for comparison:
- const existingUrl = (existing as any)?.url || (existing as any)?.URL || (existing as any)?._url || null; - if ( - existing && - (existing.readyState === WebSocket.OPEN || existing.readyState === WebSocket.CONNECTING) && - existingUrl === wsUrl - ) { + if ( + existing && + (existing.readyState === WebSocket.OPEN || existing.readyState === WebSocket.CONNECTING) && + wsUrlRef.current === wsUrl + ) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/server.exeis excluded by!**/*.exe
📒 Files selected for processing (41)
README.mdbackend/cmd/maketoken/main.gobackend/cmd/server/main.gobackend/config/config.prod.sample.ymlbackend/config/config.prod.ymlbackend/controllers/judge_controller.gobackend/controllers/transcript_controller.gobackend/db/db.gobackend/models/debate_judgement.gobackend/routes/debate.gobackend/routes/debug.gobackend/routes/rooms.gobackend/services/ai_judge.gobackend/services/transcript.gofrontend/src/Pages/Admin/AdminDashboard.tsxfrontend/src/Pages/CommunityFeed.tsxfrontend/src/Pages/DebateRoom.tsxfrontend/src/Pages/Game.tsxfrontend/src/Pages/Leaderboard.tsxfrontend/src/Pages/OnlineDebateRoom.tsxfrontend/src/Pages/Profile.tsxfrontend/src/Pages/StrengthenArgument.tsxfrontend/src/Pages/TeamDebateRoom.tsxfrontend/src/Pages/ViewDebate.tsxfrontend/src/components/ChatRoom.tsxfrontend/src/components/DebatePopup.tsxfrontend/src/components/DebateScorecard.tsxfrontend/src/components/Matchmaking.tsxfrontend/src/components/TeamChatSidebar.tsxfrontend/src/context/theme-provider.tsxfrontend/src/hooks/useDebateWS.tsfrontend/src/hooks/useUser.tsfrontend/src/lib/autoLoadMockAi.tsfrontend/src/lib/ws.tsfrontend/src/services/gamificationService.tsfrontend/src/services/judge.tsfrontend/src/services/profileService.tsfrontend/src/services/teamDebateService.tsfrontend/src/utils/auth.tsfrontend/src/utils/safeParse.tsfrontend/src/utils/storage.ts
🧰 Additional context used
🧬 Code graph analysis (28)
backend/cmd/server/main.go (2)
backend/routes/debug.go (1)
DebugDBInfo(16-42)backend/routes/debate.go (1)
SetupDebateJudgeRoutes(109-112)
backend/routes/debug.go (1)
backend/db/db.go (1)
MongoDatabase(18-18)
frontend/src/services/teamDebateService.ts (2)
frontend/src/utils/auth.ts (1)
getAuthToken(7-9)frontend/src/utils/storage.ts (1)
getLocalString(1-10)
frontend/src/components/ChatRoom.tsx (2)
frontend/src/lib/ws.ts (1)
buildWsUrl(6-14)frontend/src/utils/safeParse.ts (1)
safeParse(1-11)
frontend/src/services/judge.ts (3)
backend/services/ai_judge.go (2)
SideScores(17-20)JudgementResult(31-35)frontend/src/components/DebateScorecard.tsx (3)
SideScores(3-6)JudgementScores(8-13)JudgementResult(15-19)frontend/src/utils/auth.ts (1)
getAuthToken(7-9)
frontend/src/Pages/ViewDebate.tsx (2)
frontend/src/lib/ws.ts (1)
buildWsUrl(6-14)frontend/src/utils/safeParse.ts (1)
safeParse(1-11)
frontend/src/Pages/TeamDebateRoom.tsx (3)
frontend/src/lib/ws.ts (1)
buildWsUrl(6-14)frontend/src/utils/safeParse.ts (1)
safeParse(1-11)frontend/src/utils/storage.ts (1)
getLocalString(1-10)
frontend/src/context/theme-provider.tsx (1)
frontend/src/utils/storage.ts (2)
getLocalString(1-10)setLocalString(37-44)
frontend/src/components/TeamChatSidebar.tsx (1)
frontend/src/utils/safeParse.ts (1)
safeParse(1-11)
backend/services/ai_judge.go (2)
frontend/src/components/DebateScorecard.tsx (2)
SideScores(3-6)JudgementResult(15-19)frontend/src/services/judge.ts (2)
SideScores(5-8)JudgementResult(17-21)
backend/services/transcript.go (1)
backend/db/db.go (1)
MongoDatabase(18-18)
frontend/src/utils/auth.ts (1)
frontend/src/utils/storage.ts (2)
setLocalString(37-44)getLocalString(1-10)
backend/models/debate_judgement.go (1)
backend/services/ai_judge.go (1)
Scores(23-28)
frontend/src/Pages/CommunityFeed.tsx (1)
frontend/src/utils/storage.ts (1)
getLocalString(1-10)
frontend/src/components/Matchmaking.tsx (3)
frontend/src/utils/safeParse.ts (1)
safeParse(1-11)backend/websocket/matchmaking.go (1)
MatchmakingMessage(48-56)backend/services/matchmaking.go (1)
MatchmakingPool(16-25)
backend/controllers/transcript_controller.go (2)
backend/db/db.go (1)
MongoDatabase(18-18)backend/models/transcript.go (1)
SavedDebateTranscript(25-37)
frontend/src/components/DebateScorecard.tsx (2)
backend/services/ai_judge.go (2)
SideScores(17-20)JudgementResult(31-35)frontend/src/services/judge.ts (3)
SideScores(5-8)JudgementScores(10-15)JudgementResult(17-21)
frontend/src/Pages/Game.tsx (1)
frontend/src/lib/ws.ts (1)
buildWsUrl(6-14)
backend/controllers/judge_controller.go (4)
backend/db/db.go (1)
MongoDatabase(18-18)backend/models/debate_judgement.go (1)
DebateJudgement(10-17)backend/services/transcript.go (1)
GetDebateTranscript(20-100)backend/services/ai_judge.go (2)
JudgeDebateAI(38-225)Scores(23-28)
backend/cmd/maketoken/main.go (2)
backend/config/config.go (1)
LoadConfig(60-73)backend/utils/auth.go (2)
SetJWTSecret(27-29)GenerateJWTToken(75-97)
frontend/src/Pages/Profile.tsx (2)
frontend/src/utils/auth.ts (1)
clearAuthToken(11-15)frontend/src/components/ui/button.tsx (1)
Button(57-57)
frontend/src/services/gamificationService.ts (3)
frontend/src/lib/ws.ts (1)
buildWsUrl(6-14)backend/models/gamification.go (1)
GamificationEvent(47-55)frontend/src/utils/safeParse.ts (1)
safeParse(1-11)
backend/routes/debate.go (1)
backend/controllers/judge_controller.go (2)
JudgeDebateHandler(20-113)GetDebateJudgementHandler(116-141)
frontend/src/Pages/DebateRoom.tsx (2)
frontend/src/utils/storage.ts (1)
getLocalString(1-10)frontend/src/utils/safeParse.ts (1)
safeParse(1-11)
frontend/src/hooks/useUser.ts (1)
frontend/src/utils/storage.ts (2)
getLocalString(1-10)setLocalJSON(28-35)
backend/routes/rooms.go (1)
backend/db/db.go (1)
MongoDatabase(18-18)
frontend/src/Pages/StrengthenArgument.tsx (1)
backend/models/coach.go (2)
WeakStatement(4-9)Evaluation(18-21)
frontend/src/Pages/OnlineDebateRoom.tsx (6)
backend/services/ai_judge.go (1)
JudgementResult(31-35)frontend/src/components/DebateScorecard.tsx (2)
JudgementResult(15-19)DebateScorecard(38-88)frontend/src/services/judge.ts (2)
JudgementResult(17-21)judgeDebate(48-72)frontend/src/utils/safeParse.ts (1)
safeParse(1-11)frontend/src/utils/storage.ts (1)
getLocalString(1-10)frontend/src/lib/ws.ts (1)
buildWsUrl(6-14)
🪛 Gitleaks (8.30.0)
backend/config/config.prod.yml
[high] 10-10: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
[high] 15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (34)
backend/routes/rooms.go (1)
64-64: LGTM! Good refactoring to use centralized database handle.Replacing direct MongoDB client references with the centralized
db.MongoDatabasehandle eliminates hardcoded database names and improves maintainability. The refactoring is consistent across all handlers.Also applies to: 99-99, 112-112, 143-143, 177-177, 209-210
README.md (1)
61-63: LGTM! Clear documentation with security guidance.The updated MongoDB configuration instructions provide clear examples and appropriate security warnings about credential management.
backend/cmd/server/main.go (1)
130-131: LGTM! Judge routes properly protected.The debate judge endpoints are correctly registered within the authenticated route group.
backend/config/config.prod.sample.yml (1)
5-7: LGTM! Clear placeholder with helpful examples.The updated MongoDB URI documentation provides clear guidance with example formats for both local and Atlas connections.
backend/routes/debate.go (1)
108-112: LGTM! Clean route registration.The judge route setup follows the established pattern used elsewhere in the codebase and properly delegates to controller handlers.
backend/services/ai_judge.go (1)
38-104: Overall structure of AI judging flow is well-designed.Good practices observed:
- Timeout-bounded context (60s) prevents hanging
- Strict JSON parsing with
DisallowUnknownFields()as first attempt- Trailing token check to ensure exactly one JSON object
- Comprehensive range validation (1-10) for all scores
- Graceful fallback to lenient parsing
backend/controllers/judge_controller.go (2)
115-141: GET handler implementation is correct.Properly handles:
- Invalid ObjectID format (400)
- Document not found (404)
- Database errors (500)
- Bounded context timeout (10s)
19-27: Authentication is already applied via middleware.The judge endpoints are registered within the protected
authrouter group that appliesmiddlewares.AuthMiddlewareat the route level (main.go:116, 131). Authentication is enforced before either handler is invoked, so unauthenticated requests are blocked.Likely an incorrect or invalid review comment.
frontend/src/Pages/TeamDebateRoom.tsx (2)
11-13: Centralizing WS URL construction and message parsing looks goodUsing
buildWsUrlfor the team WS endpoint andsafeParse<WSMessage>for incoming messages is a solid reliability upgrade; invalid frames are now safely ignored instead of crashing the handler.Also applies to: 718-720, 733-739
1452-1455: Safer transcript sourcing and judgment parsingSwitching to
getLocalStringfor transcript reads and wrapping judgment JSON insafeParse<JudgmentData>with a user‑visible fallback avoids hard failures on malformed storage/LLM output while preserving previous behavior.Also applies to: 1484-1492
frontend/src/lib/autoLoadMockAi.ts (1)
1-29: Dev-only auto-load helper is safe and self-containedThe global declaration plus
typeof window !== 'undefined'guard and nested try/catch make this safe in non‑browser contexts. Persisting the mock judgment intosessionStorageand exposing no runtime API keeps the helper clearly dev‑scoped and easy to remove later.frontend/src/Pages/OnlineDebateRoom.tsx (1)
570-579: Judgment parsing and transcript retrieval changes are defensive and backwards-compatible
- Using
safeParse<JudgmentData>on bothpollData.resultandresult.resultensures malformed LLM output doesn’t explode the UI; the warning + user‑facing error message is a good fallback.gatherTranscriptsForRolenow reads fromgetLocalString(storageKey)instead of rawlocalStorage.getItem, while still preferring in‑memoryspeechTranscriptswhen present. This preserves previous behavior and centralizes storage error handling.No functional issues spotted here.
Also applies to: 672-677, 720-747
frontend/src/Pages/ViewDebate.tsx (1)
2-2: Spectator WS hardening is correct and consistentUsing
buildWsUrl('/ws', { room: debateID, token, spectator: 'true' })brings this path in line with the centralized WS config, and wrappingevent.datainsafeParsewith a warning/early‑return avoids crashes on invalid frames while preserving existing message handling.Also applies to: 6-6, 85-90, 271-275
frontend/src/Pages/Leaderboard.tsx (1)
4-4: LGTM! Consistent adoption of centralized storage utility.The migration from direct
localStorage.getItemtogetLocalStringprovides SSR safety and error handling. The pattern is applied consistently across the component.Also applies to: 97-98
frontend/src/context/theme-provider.tsx (1)
2-2: LGTM! Clean migration to centralized storage utilities.The theme persistence now uses
getLocalStringandsetLocalString, providing SSR-safe localStorage access with proper error handling.Also applies to: 31-31, 59-59
frontend/src/Pages/CommunityFeed.tsx (1)
3-3: LGTM! Consistent token retrieval migration.All three token access points (
fetchFeed,handleFollow,handleDeletePost) now use the centralizedgetLocalStringutility with proper null checks.Also applies to: 282-282, 360-360, 406-406
frontend/src/components/TeamChatSidebar.tsx (1)
7-7: LGTM! Proper adoption of safe JSON parsing for WebSocket messages.The migration to
safeParsewith an early return guard (line 63) prevents crashes from malformed WebSocket payloads. This aligns with the PR objective of improving WebSocket reliability.Also applies to: 61-75
frontend/src/services/gamificationService.ts (1)
1-1: LGTM! Proper adoption of centralized WebSocket URL builder.Using
buildWsUrl('/ws/gamification', { token })aligns with the PR's goal of centralizing WebSocket URL construction.Also applies to: 100-101
frontend/src/Pages/StrengthenArgument.tsx (3)
35-38: LGTM! Interface aligns with backend model.The
Evaluationinterface withscore: numbernow matches the backendEvaluationstruct inbackend/models/coach.go.
128-136: Good defensive parsing for WeakStatement response.Wrapping
JSON.parsein try/catch and validating required fields (id,text,topic,stance) prevents crashes from malformed server responses and provides clear error messaging.
178-191: Good defensive parsing for Evaluation response.The validation checks both the type (
typeof data.score !== "number") and presence of required fields, ensuring robust handling of unexpected response formats.frontend/src/lib/ws.ts (2)
6-14: LGTM! Well-implemented WebSocket URL builder.The
buildWsUrlfunction properly:
- Uses the
URLconstructor for reliable path resolution- Filters out undefined, null, and empty string query parameters
- Converts values to strings before setting
This centralizes WebSocket URL construction across the frontend.
1-4: The regex implementation is correct and does not require changes.The pattern
/^https?/uses a start-of-string anchor that only matches the protocol prefix ("http" or "https"), not occurrences of "http" elsewhere in the URL. URLs likehttp://httpserver.comare handled correctly—only the protocol prefix is replaced, producingws://httpserver.com. No false matches occur.Likely an incorrect or invalid review comment.
frontend/src/components/DebatePopup.tsx (1)
67-70: LGTM! Enhanced error logging.The improved error logging with context will aid debugging during development and production troubleshooting.
</review_comment_end>
frontend/src/hooks/useUser.ts (1)
91-91: LGTM! Consistent storage helper usage.The migration to
setLocalJSONfor persistence andgetLocalStringfor token checks aligns with the PR's goal of standardizing localStorage access.</review_comment_end>
Also applies to: 110-111
frontend/src/Pages/Admin/AdminDashboard.tsx (1)
367-378: LGTM! Clean migration to storage utilities.The consistent use of
getLocalStringandgetLocalJSON<Admin>throughout the admin dashboard initialization and refresh logic aligns well with the PR's goal of standardizing localStorage access. The generic type usage is particularly good for type safety.</review_comment_end>
Also applies to: 508-516
frontend/src/utils/storage.ts (1)
1-26: LGTM! Well-designed storage abstraction.The SSR-safe checks, consistent error handling, and automatic cleanup of corrupt JSON (line 20) make this a robust utility. The separation between string and JSON helpers provides clear semantics for callers.
</review_comment_end>
frontend/src/Pages/Profile.tsx (1)
200-208: LGTM! Enhanced authentication error handling.The integration of
clearAuthTokenand the actionable error UI significantly improve the user experience when authentication fails. Users now get clear feedback and options to sign in or retry, rather than being stuck in an error state.</review_comment_end>
Also applies to: 339-359
frontend/src/services/judge.ts (1)
23-39: LGTM on the JSON parsing helper.Good defensive parsing with fallback for markdown-fenced JSON blocks. This handles edge cases where AI models may return JSON wrapped in markdown formatting.
frontend/src/hooks/useDebateWS.ts (5)
37-38: Guard against concurrent socket creation is a good addition.The
creatingRefflag prevents race conditions during rapid debateId changes or re-renders. Well designed.Also applies to: 107-109
139-146: Good WebSocket configuration defaults.The
maxRetries: 10limit prevents infinite reconnection loops, and the exponential backoff parameters (minReconnectionDelay: 2000,maxReconnectionDelay: 10000,reconnectionDelayGrowFactor: 1.3) are reasonable for avoiding browser resource exhaustion.
153-168: Good stale socket guards throughout handlers.Checking
socket !== wsRef.currentinonopen,onmessage,onerror, andoncloseprevents processing events from orphaned sockets. This is a solid reliability improvement.Also applies to: 362-373
392-394: Intentional dependency omission is documented.The comment explaining why
wsandsetWsare omitted from dependencies is helpful for future maintainers. This prevents infinite effect loops when updating the atom inside the effect.
40-72: The cleanup approach is correct; the@ts-ignorecomments are due to incomplete type definitions.Setting handlers to
nullis the documented cleanup pattern forreconnecting-websocket. The@ts-ignorecomments are necessary because the library's TypeScript type definitions don't properly declare these handlers as nullable (e.g.,onmessage: ((ev: MessageEvent) => any) | null), even though the runtime supports it. Consider suppressing the error with a single@ts-expect-errorcomment above the cleanup block instead of repeating@ts-ignorefor each handler, or file an issue with the library's type definitions.
|
|
||
| // Debug endpoint for matchmaking pool status | ||
| router.GET("/debug/matchmaking-pool", routes.GetMatchmakingPoolStatusHandler) | ||
| router.GET("/debug/db-info", routes.DebugDBInfo) |
There was a problem hiding this comment.
Security: Public debug endpoint exposes database information.
The /debug/db-info endpoint is registered without authentication and returns the database name and sample user emails. This could leak sensitive information in production environments.
Consider:
- Moving this endpoint to the authenticated
authgroup - Adding environment-based conditional registration (only in dev/staging)
- Implementing IP-based access control for debug endpoints
🔎 Recommended fix: Restrict debug endpoint to authenticated users
- router.GET("/debug/db-info", routes.DebugDBInfo)
-
// WebSocket routes (handle auth internally)
router.GET("/ws/matchmaking", websocket.MatchmakingHandler)
router.GET("/ws/gamification", websocket.GamificationWebSocketHandler)
@@ -114,6 +112,9 @@
// Protected routes (JWT auth)
auth := router.Group("/")
auth.Use(middlewares.AuthMiddleware("./config/config.prod.yml"))
{
+ // Debug endpoint (authenticated)
+ auth.GET("/debug/db-info", routes.DebugDBInfo)
+
auth.GET("/user/fetchprofile", routes.GetProfileRouteHandler)📝 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.
| router.GET("/debug/db-info", routes.DebugDBInfo) | |
| // WebSocket routes (handle auth internally) | |
| router.GET("/ws/matchmaking", websocket.MatchmakingHandler) | |
| router.GET("/ws/gamification", websocket.GamificationWebSocketHandler) | |
| // Protected routes (JWT auth) | |
| auth := router.Group("/") | |
| auth.Use(middlewares.AuthMiddleware("./config/config.prod.yml")) | |
| { | |
| // Debug endpoint (authenticated) | |
| auth.GET("/debug/db-info", routes.DebugDBInfo) | |
| auth.GET("/user/fetchprofile", routes.GetProfileRouteHandler) |
🤖 Prompt for AI Agents
In backend/cmd/server/main.go around line 108, the public route
router.GET("/debug/db-info", routes.DebugDBInfo) exposes DB details without
protection; move its registration into the authenticated router group (e.g.,
authGroup.GET("/debug/db-info", routes.DebugDBInfo)) or wrap the registration in
an environment check so it only registers in non-production (check APP_ENV or a
DEBUG_FEATURES flag), and add an optional IP-whitelist middleware around the
handler for extra protection (apply middleware when registering or check client
IP in the handler). Ensure the chosen approach denies access by default in
production and documents the required env/role to enable the endpoint.
| server: | ||
| port: 1313 # The port number your backend server will run on | ||
|
|
||
| database: | ||
| uri: "mongodb://localhost:27017" | ||
| # Replace with your MongoDB Atlas connection string | ||
| # Get this from your MongoDB Atlas dashboard after creating a cluster and database | ||
|
|
||
| gemini: | ||
| apiKey: "AIzaSyBgwbE92uDx5Hxcrs4ZJNqKdFdcMoqoiOo" | ||
| # API key for OpenAI / Gemini model access | ||
| # Obtain from your OpenRouter.ai or OpenAI account dashboard | ||
|
|
||
| jwt: | ||
| secret: "dlblvgbk78vkzdd4335342dkvfbgdf/mvds" | ||
| # A secret string used to sign JWT tokens | ||
| # Generate a strong random string (e.g. use `openssl rand -hex 32`) | ||
|
|
||
| expiry: 1440 | ||
| # Token expiry time in minutes (e.g. 1440 = 24 hours) | ||
|
|
||
| smtp: | ||
| host: "smtp.gmail.com" | ||
| # SMTP server host for sending emails (example is Gmail SMTP) | ||
|
|
||
| port: 587 | ||
| # SMTP server port (587 for TLS) | ||
|
|
||
| username: "nilanjan.saha.csiot.2023@tint.edu.in" | ||
| # Email username (your email address) | ||
|
|
||
| password: "10052002" | ||
| # Password for the email or app-specific password if 2FA is enabled | ||
|
|
||
| senderEmail: "nilanjan.saha.csiot.2023@tint.edu.in" | ||
| # The 'from' email address used when sending mails | ||
|
|
||
| senderName: "DebateAI Team" | ||
|
|
||
| googleOAuth: | ||
| clientID: "926438884290-019tmibvc61i0vq5ra0l2pbi7b71gr5o.apps.googleusercontent.com" | ||
| # Google OAuth Client ID for OAuth login | ||
| # Obtain from Google Cloud Console (APIs & Services > Credentials > OAuth 2.0 Client IDs) |
There was a problem hiding this comment.
Critical: Real secrets and credentials committed to version control.
This file contains actual API keys, passwords, and secrets that should never be committed to a repository:
- Line 10: Gemini API key (
AIzaSy...) - Line 15: JWT secret
- Line 29, 35: Real email addresses (PII)
- Line 32: Email password
- Line 41: Google OAuth client ID
These credentials are now exposed in git history and must be rotated immediately. Use environment variables or a secrets manager instead, and keep only placeholder values in committed config files (as in config.prod.sample.yml).
🔎 Recommended approach
gemini:
- apiKey: "AIzaSyBgwbE92uDx5Hxcrs4ZJNqKdFdcMoqoiOo"
+ apiKey: "${GEMINI_API_KEY}" # Set via environment variable
jwt:
- secret: "dlblvgbk78vkzdd4335342dkvfbgdf/mvds"
+ secret: "${JWT_SECRET}" # Generate via: openssl rand -hex 32
smtp:
- username: "nilanjan.saha.csiot.2023@tint.edu.in"
- password: "10052002"
- senderEmail: "nilanjan.saha.csiot.2023@tint.edu.in"
+ username: "${SMTP_USERNAME}"
+ password: "${SMTP_PASSWORD}"
+ senderEmail: "${SMTP_SENDER_EMAIL}"
googleOAuth:
- clientID: "926438884290-019tmibvc61i0vq5ra0l2pbi7b71gr5o.apps.googleusercontent.com"
+ clientID: "${GOOGLE_OAUTH_CLIENT_ID}"Immediate actions required:
- Remove this file from the commit and add it to
.gitignore - Rotate all exposed credentials (Gemini API key, JWT secret, email password, OAuth client ID)
- Use the existing
config.prod.sample.ymlpattern with placeholders
📝 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.
| server: | |
| port: 1313 # The port number your backend server will run on | |
| database: | |
| uri: "mongodb://localhost:27017" | |
| # Replace with your MongoDB Atlas connection string | |
| # Get this from your MongoDB Atlas dashboard after creating a cluster and database | |
| gemini: | |
| apiKey: "AIzaSyBgwbE92uDx5Hxcrs4ZJNqKdFdcMoqoiOo" | |
| # API key for OpenAI / Gemini model access | |
| # Obtain from your OpenRouter.ai or OpenAI account dashboard | |
| jwt: | |
| secret: "dlblvgbk78vkzdd4335342dkvfbgdf/mvds" | |
| # A secret string used to sign JWT tokens | |
| # Generate a strong random string (e.g. use `openssl rand -hex 32`) | |
| expiry: 1440 | |
| # Token expiry time in minutes (e.g. 1440 = 24 hours) | |
| smtp: | |
| host: "smtp.gmail.com" | |
| # SMTP server host for sending emails (example is Gmail SMTP) | |
| port: 587 | |
| # SMTP server port (587 for TLS) | |
| username: "nilanjan.saha.csiot.2023@tint.edu.in" | |
| # Email username (your email address) | |
| password: "10052002" | |
| # Password for the email or app-specific password if 2FA is enabled | |
| senderEmail: "nilanjan.saha.csiot.2023@tint.edu.in" | |
| # The 'from' email address used when sending mails | |
| senderName: "DebateAI Team" | |
| googleOAuth: | |
| clientID: "926438884290-019tmibvc61i0vq5ra0l2pbi7b71gr5o.apps.googleusercontent.com" | |
| # Google OAuth Client ID for OAuth login | |
| # Obtain from Google Cloud Console (APIs & Services > Credentials > OAuth 2.0 Client IDs) | |
| server: | |
| port: 1313 # The port number your backend server will run on | |
| database: | |
| uri: "mongodb://localhost:27017" | |
| # Replace with your MongoDB Atlas connection string | |
| # Get this from your MongoDB Atlas dashboard after creating a cluster and database | |
| gemini: | |
| apiKey: "${GEMINI_API_KEY}" # Set via environment variable | |
| # API key for OpenAI / Gemini model access | |
| # Obtain from your OpenRouter.ai or OpenAI account dashboard | |
| jwt: | |
| secret: "${JWT_SECRET}" # Generate via: openssl rand -hex 32 | |
| # A secret string used to sign JWT tokens | |
| # Generate a strong random string (e.g. use `openssl rand -hex 32`) | |
| expiry: 1440 | |
| # Token expiry time in minutes (e.g. 1440 = 24 hours) | |
| smtp: | |
| host: "smtp.gmail.com" | |
| # SMTP server host for sending emails (example is Gmail SMTP) | |
| port: 587 | |
| # SMTP server port (587 for TLS) | |
| username: "${SMTP_USERNAME}" | |
| # Email username (your email address) | |
| password: "${SMTP_PASSWORD}" | |
| # Password for the email or app-specific password if 2FA is enabled | |
| senderEmail: "${SMTP_SENDER_EMAIL}" | |
| # The 'from' email address used when sending mails | |
| senderName: "DebateAI Team" | |
| googleOAuth: | |
| clientID: "${GOOGLE_OAUTH_CLIENT_ID}" | |
| # Google OAuth Client ID for OAuth login | |
| # Obtain from Google Cloud Console (APIs & Services > Credentials > OAuth 2.0 Client IDs) |
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 10-10: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
[high] 15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| if err != nil { | ||
| // Log the AI failure for later investigation, but don't crash the server. | ||
| log.Printf("AI judge failed for debate %s: %v", debateID.Hex(), err) | ||
| // Return a 202 Accepted to indicate processing failed but debate flow continues. | ||
| c.JSON(http.StatusAccepted, gin.H{"message": "AI judging failed; debate completed without AI score"}) | ||
| return | ||
| } |
There was a problem hiding this comment.
Incorrect HTTP status: 202 Accepted implies async processing will continue.
Returning 202 Accepted when the AI judge fails is semantically incorrect. HTTP 202 indicates the request was accepted for later processing, but here processing has already failed. This may confuse API consumers.
Consider 500 Internal Server Error or 503 Service Unavailable to indicate the AI service failed.
🔎 Proposed fix
if err != nil {
// Log the AI failure for later investigation, but don't crash the server.
log.Printf("AI judge failed for debate %s: %v", debateID.Hex(), err)
- // Return a 202 Accepted to indicate processing failed but debate flow continues.
- c.JSON(http.StatusAccepted, gin.H{"message": "AI judging failed; debate completed without AI score"})
+ // Return 503 to indicate the AI service is currently unavailable.
+ c.JSON(http.StatusServiceUnavailable, gin.H{"error": "AI judging temporarily unavailable", "details": err.Error()})
return
}🤖 Prompt for AI Agents
In backend/controllers/judge_controller.go around lines 58 to 64, the handler
currently returns HTTP 202 on AI judge failure which is semantically incorrect;
change the response to return an appropriate error status (prefer
http.StatusServiceUnavailable / 503 for external AI service failures, or
http.StatusInternalServerError / 500 for internal errors). Replace
c.JSON(http.StatusAccepted, ...) with c.JSON(http.StatusServiceUnavailable,
gin.H{"message":"AI judging failed; debate completed without AI score"}) (or use
500 if you determine it’s an internal error) and keep the existing log; no async
implication should be returned.
| // Ensure unique index on debate_judgements.debateId to prevent multiple judgements | ||
| go func() { | ||
| coll := MongoDatabase.Collection("debate_judgements") | ||
| indexModel := mongo.IndexModel{ | ||
| Keys: bson.D{{Key: "debateId", Value: 1}}, | ||
| Options: options.Index().SetUnique(true).SetBackground(true), | ||
| } | ||
| ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) | ||
| defer cancel() | ||
| if _, err := coll.Indexes().CreateOne(ctx, indexModel); err != nil { | ||
| // Log the error but do not fail startup; index creation can be retried later | ||
| log.Printf("warning: failed to create index on debate_judgements.debateId: %v", err) | ||
| } | ||
| }() |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
MongoDB Go driver v1.17.3 SetBackground deprecated option
💡 Result:
SetBackground on IndexOptions is deprecated because the MongoDB server deprecated/ignores the "background" index option starting in MongoDB 4.2; the Go driver therefore marked the setter deprecated (and the driver maintainers also removed deprecated-notice mismatches in GODRIVER-1736). In practice: stop calling SetBackground — createIndexes will ignore that option on servers >= 4.2 and the driver will continue to support index creation without it. [1][2][3]
Sources:
- go.mongodb.org mongo/options docs (SetBackground deprecated). [1]
- MongoDB Community discussion explaining driver deprecation (GODRIVER-1736 / SetBackground change). [2]
- MongoDB 4.2 release notes: optimized index builds; background option ignored. [3]
🏁 Script executed:
# Check the full context of db.go to understand the startup sequence
find . -name "db.go" -path "*/backend/db/*"Repository: AOSSIE-Org/DebateAI
Length of output: 81
🏁 Script executed:
# Look for judge-related operations and their initialization
rg "judge" -i --type go -A 3 -B 3Repository: AOSSIE-Org/DebateAI
Length of output: 30529
🏁 Script executed:
cat -n backend/db/db.goRepository: AOSSIE-Org/DebateAI
Length of output: 4822
🏁 Script executed:
cat -n backend/cmd/server/main.goRepository: AOSSIE-Org/DebateAI
Length of output: 7016
Remove the deprecated SetBackground option and fix the race condition on index creation.
The SetBackground(true) option is deprecated in MongoDB 4.2+; all index builds are background by default. More critically, launching index creation in a goroutine without synchronization creates a real race condition: the HTTP server starts accepting judge requests (via router.Run()) before the unique index on debate_judgements.debateId is guaranteed to exist. During this window, concurrent judge operations can bypass the unique constraint and create duplicate judgements. If index creation fails silently, the constraint is lost entirely.
The current duplicate key error handling in judge_controller.go only works if the index exists; without it, duplicates slip through undetected.
Recommended fixes:
- Create the index synchronously during startup (block until complete), or
- Use a WaitGroup or channel to synchronize goroutine completion before calling
router.Run(), or - Implement a retry mechanism with exponential backoff if index creation fails, and fail startup if it cannot be created.
| type DebateJudgement struct { | ||
| ID primitive.ObjectID `bson:"_id,omitempty" json:"id,omitempty"` | ||
| DebateID primitive.ObjectID `bson:"debateId" json:"debateId"` | ||
| Winner string `bson:"winner" json:"winner"` // "Side A" or "Side B" | ||
| Scores map[string]float64 `bson:"scores" json:"scores"` // keys: logic, evidence, persuasion, rebuttal | ||
| Summary string `bson:"summary" json:"summary"` | ||
| CreatedAt time.Time `bson:"createdAt" json:"createdAt"` | ||
| } |
There was a problem hiding this comment.
Type mismatch: Scores field won't deserialize correctly.
The Scores field is declared as map[string]float64, but the actual stored documents use nested maps:
{"logic": {"sideA": 8, "sideB": 6}, "evidence": {...}, ...}This mismatch will cause deserialization failures when reading documents via this model. The field type should match the actual stored structure.
🔎 Proposed fix
Either use a flexible type:
- Scores map[string]float64 `bson:"scores" json:"scores"`
+ Scores map[string]interface{} `bson:"scores" json:"scores"`Or define a proper nested structure matching the stored data:
type SideScores struct {
SideA int `bson:"sideA" json:"sideA"`
SideB int `bson:"sideB" json:"sideB"`
}
type JudgementScores struct {
Logic SideScores `bson:"logic" json:"logic"`
Evidence SideScores `bson:"evidence" json:"evidence"`
Persuasion SideScores `bson:"persuasion" json:"persuasion"`
Rebuttal SideScores `bson:"rebuttal" json:"rebuttal"`
}
type DebateJudgement struct {
// ...
Scores JudgementScores `bson:"scores" json:"scores"`
// ...
}| const [aiLoading, setAiLoading] = useState(false); | ||
| const [aiJudgement, setAiJudgement] = useState<JudgementResult | null>(null); | ||
| const [aiError, setAiError] = useState<string | null>(null); | ||
| const [showAiScorecard, setShowAiScorecard] = useState(false); | ||
| const judgeCalledRef = useRef(false); | ||
| const [judgmentData, setJudgmentData] = useState<JudgmentData | null>(null); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "OnlineDebateRoom.tsx" -type fRepository: AOSSIE-Org/DebateAI
Length of output: 104
🏁 Script executed:
wc -l frontend/src/Pages/OnlineDebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 107
🏁 Script executed:
sed -n '470,490p' frontend/src/Pages/OnlineDebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 885
🏁 Script executed:
grep -n "judgeCalledRef" frontend/src/Pages/OnlineDebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 191
🏁 Script executed:
grep -n "useEffect" frontend/src/Pages/OnlineDebateRoom.tsx | head -20Repository: AOSSIE-Org/DebateAI
Length of output: 544
🏁 Script executed:
sed -n '1963,2020p' frontend/src/Pages/OnlineDebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 2313
🏁 Script executed:
sed -n '1969,1993p' frontend/src/Pages/OnlineDebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 1038
🏁 Script executed:
sed -n '1995,2016p' frontend/src/Pages/OnlineDebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 1028
🏁 Script executed:
sed -n '2011,2030p' frontend/src/Pages/OnlineDebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 684
🏁 Script executed:
sed -n '2411,2423p' frontend/src/Pages/OnlineDebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 635
🏁 Script executed:
sed -n '1970,1998p' frontend/src/Pages/OnlineDebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 1285
🏁 Script executed:
grep -A5 "useEffect.*roomId" frontend/src/Pages/OnlineDebateRoom.tsx | head -30Repository: AOSSIE-Org/DebateAI
Length of output: 45
🏁 Script executed:
grep -n "return.*cleanup\|return ().*=>" frontend/src/Pages/OnlineDebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 249
🏁 Script executed:
sed -n '1,100p' frontend/src/Pages/OnlineDebateRoom.tsx | head -50Repository: AOSSIE-Org/DebateAI
Length of output: 1591
🏁 Script executed:
grep -n "useParams\|useNavigate\|router" frontend/src/Pages/OnlineDebateRoom.tsx | head -10Repository: AOSSIE-Org/DebateAI
Length of output: 168
🏁 Script executed:
grep -B5 "judgeCalledRef.current\s*=" frontend/src/Pages/OnlineDebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 279
🏁 Script executed:
grep -n "aiError" frontend/src/Pages/OnlineDebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 129
🏁 Script executed:
sed -n '155,160p' frontend/src/Pages/OnlineDebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 346
🌐 Web query:
React Router v6 component remount on route param change same path pattern
💡 Result:
Short answer: React Router v6 does not automatically unmount/remount an element when only route params change. The element is reused (props/hooks update) — if you need a full remount, either (A) respond to param changes inside the component (recommended), or (B) force a remount by giving the element a changing key based on the param/location.
Why: RRv6 reuses the same element for the same route pattern; React only remounts when the element’s key/type changes. [1][2]
Two concise patterns
A) Handle param changes inside the component (preferred)
- Use useParams/useLocation and run logic in useEffect keyed to the param.
Example:
import { useParams } from "react-router-dom";
useEffect(() => {
// load data or reset state when id changes
}, [useParams().id]);
This avoids forced remounts and preserves performance. [2]
B) Force remount using key (when you truly need a fresh instance)
- Wrap the page in a small proxy that sets key from the param or location:
import { useParams } from "react-router-dom";
function PageProxy() {
const { id } = useParams();
return ;
}
<Route path="/item/:id" element={} />
You can also use location.pathname or location.key if you want remount on any location change. [2][3]
References
- React Router FAQ / docs (v6). [1]
- StackOverflow discussion and common solutions (useEffect or key proxy). [2]
- Blog explanation of forcing remount via key. [3]
judgeCalledRef persists across room changes, blocking AI judge in multi-debate sessions
In React Router v6, navigating between routes with different params (e.g., /debate-room/1 → /debate-room/2) reuses the same component instance without remounting. Since judgeCalledRef.current is set to true when the first debate finishes and never reset, the AI judge will be skipped on all subsequent debates opened in the same session.
Add a cleanup function to reset the ref when roomId changes:
useEffect(() => {
// ... existing AI judge logic ...
}, [debatePhase, roomId, isRoomOwner]);
// Add this:
useEffect(() => {
return () => {
judgeCalledRef.current = false;
};
}, [roomId]);Additionally:
aiErroris set on failure but never displayed in the UI; either wire it into a visible error banner or remove the state to avoid confusion.- Uncomment the
sessionStorage.removeItem('mockAiJudgement')line (currently commented at line 2014) to prevent mocks from persisting across page refreshes once you're done testing.
🤖 Prompt for AI Agents
In frontend/src/Pages/OnlineDebateRoom.tsx around lines 474 to 479,
judgeCalledRef is never reset when navigating between rooms so the AI judge is
skipped for subsequent debates; add a useEffect keyed on roomId that resets
judgeCalledRef.current = false in its cleanup to ensure the judge can run for
each new room, also either surface aiError in the UI (e.g., show an error
banner) or remove the aiError state if unused to avoid confusion, and uncomment
the sessionStorage.removeItem('mockAiJudgement') call (currently commented near
line ~2014) so test mocks do not persist across refreshes.
| // parse using shared safeParse utility | ||
| // import safeParse lazily to avoid import cycles in some builds | ||
| // (top-level import kept minimal) -- but here we import directly | ||
| // to use the utility consistently. | ||
| // NOTE: safeParse returns null on failure | ||
| // (ensures consistent parsing behavior across the app) | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const { safeParse } = require("@/utils/safeParse"); | ||
| const raw = event.data; | ||
| const parsed: GamificationEvent | null = safeParse<GamificationEvent>(raw, null); | ||
| if (!parsed) { | ||
| console.warn("Failed to parse gamification event, ignoring", raw); | ||
| return; | ||
| } | ||
| onMessage(parsed); |
There was a problem hiding this comment.
Avoid using require() for dynamic imports in ES module context.
Using require() inside onmessage is problematic:
- It's called on every incoming message, adding overhead
require()is a CommonJS construct that may not work correctly with Vite/ESM bundling- Other files in this PR use top-level imports for
safeParse
🔎 Proposed fix
Move the import to the top of the file like other components:
import { buildWsUrl } from '@/lib/ws';
+import { safeParse } from '@/utils/safeParse';
const baseURL = import.meta.env.VITE_BASE_URL || "http://localhost:1313";Then simplify the onmessage handler:
ws.onmessage = (event) => {
try {
- // parse using shared safeParse utility
- // import safeParse lazily to avoid import cycles in some builds
- // (top-level import kept minimal) -- but here we import directly
- // to use the utility consistently.
- // NOTE: safeParse returns null on failure
- // (ensures consistent parsing behavior across the app)
- // eslint-disable-next-line @typescript-eslint/no-var-requires
- const { safeParse } = require("@/utils/safeParse");
- const raw = event.data;
- const parsed: GamificationEvent | null = safeParse<GamificationEvent>(raw, null);
+ const parsed = safeParse<GamificationEvent>(event.data, null);
if (!parsed) {
- console.warn("Failed to parse gamification event, ignoring", raw);
+ console.warn("Failed to parse gamification event, ignoring", event.data);
return;
}
onMessage(parsed);
} catch (error) {
console.error("Error handling gamification event:", error);
}
};📝 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.
| // parse using shared safeParse utility | |
| // import safeParse lazily to avoid import cycles in some builds | |
| // (top-level import kept minimal) -- but here we import directly | |
| // to use the utility consistently. | |
| // NOTE: safeParse returns null on failure | |
| // (ensures consistent parsing behavior across the app) | |
| // eslint-disable-next-line @typescript-eslint/no-var-requires | |
| const { safeParse } = require("@/utils/safeParse"); | |
| const raw = event.data; | |
| const parsed: GamificationEvent | null = safeParse<GamificationEvent>(raw, null); | |
| if (!parsed) { | |
| console.warn("Failed to parse gamification event, ignoring", raw); | |
| return; | |
| } | |
| onMessage(parsed); | |
| const parsed = safeParse<GamificationEvent>(event.data, null); | |
| if (!parsed) { | |
| console.warn("Failed to parse gamification event, ignoring", event.data); | |
| return; | |
| } | |
| onMessage(parsed); |
🤖 Prompt for AI Agents
In frontend/src/services/gamificationService.ts around lines 109 to 123, the
onmessage handler currently uses require() to load safeParse on each incoming
message which is inefficient and incompatible with ESM/Vite; move the safeParse
import to the top of the file using a standard TypeScript ES import (e.g. import
{ safeParse } from "@/utils/safeParse";), remove the in-handler require and its
eslint-disable line, and then call safeParse directly in the handler exactly as
before (preserving the nullable return type and the warning/early return logic).
| const data = await parseJSONResponse(res); | ||
| return data?.result || data; | ||
| } |
There was a problem hiding this comment.
Inconsistent return handling between judgeDebate and getJudgement.
judgeDebate returns data?.result || data (line 71), while getJudgement returns just data (line 92). If both endpoints can return either { result: JudgementResult } or JudgementResult directly, both functions should handle it consistently.
🔎 Proposed fix
const data = await parseJSONResponse(res);
- return data;
+ return data?.result || data;Also applies to: 91-92
🤖 Prompt for AI Agents
In frontend/src/services/judge.ts around lines 70-72 and 91-92, the two
functions handle responses inconsistently: judgeDebate returns data?.result ||
data while getJudgement returns data; make them consistent by normalizing the
parsed response the same way in both places (extract data?.result if present,
otherwise fall back to data) so callers always receive a JudgementResult object;
update both return statements to use this normalization and add a brief comment
noting the dual-shape API response.
| const text = await response.text(); | ||
| const data = text ? JSON.parse(text) : null; |
There was a problem hiding this comment.
Unhandled JSON parsing error; use the safeParse utility.
JSON.parse() throws a SyntaxError on malformed JSON. The ternary check only guards against empty strings but won't prevent parsing exceptions. Per the PR objectives, a safeParse.ts utility exists to standardize parsing with safe fallbacks across the app—this function should use it.
🔎 Proposed fix using safeParse utility
Assuming safeParse is available from a utility file:
+import { safeParse } from '../utils/safeParse';
+
export const getProfile = async (token: string) => {
const response = await fetch(`${baseURL}/user/fetchprofile`, {
method: "GET",
headers: {
Authorization: `Bearer ${token}`,
},
});
- const text = await response.text();
- const data = text ? JSON.parse(text) : null;
+ const text = await response.text();
+ const data = safeParse(text);
if (!response.ok) {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/services/profileService.ts around lines 11 to 12, the code uses
JSON.parse on response.text() which can throw on malformed JSON; replace it with
the app's safeParse utility. Import safeParse from the utility module at the top
of the file, call safeParse(text) instead of JSON.parse(text), and use the
returned safe result (or null/default) when populating `data` so parsing errors
are handled gracefully without throwing.
| export const safeParse = <T = unknown>(raw: unknown, fallback: T | null = null): T | null => { | ||
| if (raw === null || raw === undefined) return fallback; | ||
| if (typeof raw === 'object') return (raw as unknown) as T; | ||
| if (typeof raw !== 'string') return fallback; | ||
| try { | ||
| return JSON.parse(raw) as T; | ||
| } catch (err) { | ||
| console.warn('safeParse failed to parse JSON:', err); | ||
| return fallback; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Review the unsafe type cast for objects.
Line 3 performs a double type cast (raw as unknown) as T without validating that the object conforms to type T. This bypasses TypeScript's type safety and could return incorrectly shaped objects, potentially causing runtime errors in consuming code.
Consider these options:
- Remove the object path if
safeParseis only meant for parsing JSON strings:
if (raw === null || raw === undefined) return fallback;
- if (typeof raw === 'object') return (raw as unknown) as T;
if (typeof raw !== 'string') return fallback;- Add runtime validation if object pass-through is intentional:
if (typeof raw === 'object') {
// Add validation logic here to verify raw conforms to T
return raw as T;
}- Document the behavior if this is by design, noting that callers must ensure objects match the expected type.
</review_comment_end>
🤖 Prompt for AI Agents
In frontend/src/utils/safeParse.ts around lines 1 to 11 the function unsafely
casts non-string objects to T via (raw as unknown) as T which bypasses type
safety; either remove the object pass-through branch so safeParse only accepts
strings and fallbacks, or implement runtime validation before returning an
object (e.g., check required keys/types or accept a validator callback parameter
to assert raw conforms to T) and return fallback on validation failure, and if
keeping the pass-through behavior add JSDoc making callers explicitly
responsible for ensuring object shape.
2 participants
Overview
This PR adds an AI-judge scorecard flow with mock support, improves WebSocket reliability, and introduces backend dev helpers to make local UI testing easier without requiring Gemini or production credentials.
The goal is to enable end-to-end testing of the AI judge UX, harden real-time behavior, and reduce friction for contributors running the project locally.
Highlights
##🛡️ WebSocket Lifecycle Hardening
useDebateWS rewritten to:
🧯 Matchmaking Safety
🧪 Backend Dev Helpers
Key Files Changed
Frontend
ws.ts
Centralized buildWsUrl and WebSocket helpers.
useDebateWS.ts
Lifecycle fixes: cleanup, creation guard, limited reconnects.
autoLoadMockAi.ts
Temporary dev helper to persist/load a mock AI judgement.
DebateScorecard.tsx
New UI component to render AI judgement scorecards.
OnlineDebateRoom.tsx
Auto-loads mock judgement for room owners.
Matchmaking.tsx
Defensive handling for pool_update messages.
Backend
debate_judgement.go
New model for persisted AI judgements.
ai_judge.go
AI judge flow updates and mock support.
judge_controller.go
Judge creation/retrieval endpoints.
transcript_controller.go
create-test-transcript now inserts a mock judgement.
main.go
Token-generation helper (maketoken).
debug.go
Debug route for DB info.
Behavior & Testing
###🔹 Frontend-only Quick Test (No Backend Needed)
Open a debate room as the room owner
In the browser console, set:
window.__mockAiJudgement = { /* mock JSON */ }
Reload the page
##🔹 End-to-End Test (Backend + Frontend)
Start backend on :1313
Generate a JWT:
maketoken
Create a test transcript + mock judgement:
POST /create-test-transcript
Authorization: Bearer
The persisted AI scorecard will appear automatically
🔹 WebSocket Behavior
All WebSocket connections now go through buildWsUrl
useDebateWS prevents:
Duplicate connections
Orphaned sockets
Excessive reconnect attempts
Notes / Follow-ups
Future work:
This PR solves #159 issue
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.