Feat/role based dabate mode #199
Conversation
📝 WalkthroughWalkthroughAdds role and persona support: backend models and controllers persist Role; bot prompt generation accepts role to customize responses; WebSocket supports personaSelection and broadcasts persona; frontend adds role/persona selection, types, and UI wiring. Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend as Frontend Client
participant API as HTTP API
participant Controller as Debate Controller
participant DB as Database
participant Service as Bot Service
Frontend->>API: POST /debates { role: "historian", ... }
API->>Controller: CreateDebate(req)
Controller->>DB: Insert Debate (role="historian")
DB-->>Controller: Inserted
Controller-->>API: 201 { debateId, role }
Frontend->>API: POST /debates/message { debateId, message, role:"historian" }
API->>Controller: SendDebateMessage(req)
Controller->>Service: GenerateBotResponse(history, ..., role="historian")
Service->>Service: constructPrompt(history, role)
Service-->>Controller: BotResponse
Controller->>DB: Save message with Role
Controller-->>API: DebateMessageResponse (includes Role)
sequenceDiagram
participant Frontend as Frontend Client
participant WS as WebSocket Server
participant Room as Room Manager
participant Peers as Other Clients
Frontend->>WS: {"type":"personaSelection","persona":"scientist"}
WS->>Room: handlePersonaSelection(conn, message)
Room->>Room: set client.PersonaRole = "scientist"
Room->>Peers: broadcastParticipants(updated participants with persona)
Peers->>Frontend: receive participants update (show persona badges)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
backend/routes/rooms.go (1)
29-36: Role is added but not defaulted nor returned by/rooms/:id/participants(badges can’t reliably render).
Line 35 addsrole, but it’s never set for new/joining participants, and Line 311-317 omits it from the participants payload. Also, frontend online room code usespersonanaming, while this endpoint exposesrole, which is easy to confuse with “for/against”.Proposed fix (default + include field; optionally align naming)
--- a/backend/routes/rooms.go +++ b/backend/routes/rooms.go @@ type Participant struct { ID string `json:"id" bson:"id"` Username string `json:"username" bson:"username"` Elo int `json:"elo" bson:"elo"` AvatarURL string `json:"avatarUrl" bson:"avatarUrl,omitempty"` Email string `json:"email" bson:"email,omitempty"` Role string `json:"role" bson:"role,omitempty"` // historian, scientist, lawyer, neutral } @@ creatorParticipant := Participant{ ID: user.ID.Hex(), Username: user.DisplayName, Elo: int(math.Round(user.Rating)), AvatarURL: user.AvatarURL, Email: user.Email, + Role: "neutral", } @@ participant := Participant{ ID: user.ID.Hex(), Username: user.DisplayName, Elo: int(math.Round(user.Rating)), AvatarURL: user.AvatarURL, Email: user.Email, + Role: "neutral", } @@ participantsWithDetails = append(participantsWithDetails, gin.H{ "id": participant.ID, "username": displayName, "displayName": displayName, "elo": elo, "avatarUrl": avatarURL, + "role": participant.Role, })Also applies to: 83-90, 168-176, 311-318
frontend/src/Pages/DebateRoom.tsx (1)
140-149: Role is displayed, but not sent on subsequent turns (bot may revert to neutral style).
Line 148 addsrole, and the UI renders it (Line 817), butsendDebateMessagepayload (Line 529-536) omits it.Proposed fix
@@ const { response } = await sendDebateMessage({ botLevel: debateData.botLevel, topic: debateData.topic, history: state.messages, botName: debateData.botName, stance: state.botStance, context, + role: debateData.role, });Also applies to: 529-537, 815-822
backend/controllers/debatevsbot_controller.go (1)
21-30: Normalize + default + validateroleserver-side (prevents empty role and keeps API truly backward-compatible).
As-is, Role can be""and you return it (Line 47 / Line 57), which contradicts the “neutral default” behavior.Proposed fix (CreateDebate + SendDebateMessage)
@@ func CreateDebate(c *gin.Context) { @@ var req DebateRequest if err := c.ShouldBindJSON(&req); err != nil { c.JSON(400, gin.H{"error": "Invalid request payload: " + err.Error()}) return } + + req.Role = strings.ToLower(strings.TrimSpace(req.Role)) + if req.Role == "" { + req.Role = "neutral" + } + switch req.Role { + case "neutral", "historian", "scientist", "lawyer": + default: + c.JSON(400, gin.H{"error": "Invalid role"}) + return + } @@ func SendDebateMessage(c *gin.Context) { @@ var req DebateRequest if err := c.ShouldBindJSON(&req); err != nil { c.JSON(400, gin.H{"error": "Invalid request payload: " + err.Error()}) return } + + req.Role = strings.ToLower(strings.TrimSpace(req.Role)) + if req.Role == "" { + req.Role = "neutral" + } + switch req.Role { + case "neutral", "historian", "scientist", "lawyer": + default: + c.JSON(400, gin.H{"error": "Invalid role"}) + return + }Also applies to: 97-108, 115-125, 141-166, 147-149, 175-185
🤖 Fix all issues with AI agents
In @backend/models/debatevsbot.go:
- Line 30: The Role field on the DebateVsBot model can be empty/invalid and
roleInstruction in the switch inside backend/services/debatevsbot.go (the logic
handling historian/scientist/lawyer) ends up empty; either validate Role on
incoming requests (reject if not one of
"historian","scientist","lawyer","neutral") in the controller that constructs
the model, or make Role default to "neutral" when missing and add a "neutral"
case to the switch that sets a proper neutral roleInstruction so prompts never
get an empty roleInstruction.
In @backend/structs/auth.go:
- Line 5: Update the password validation minimum from 6 to 12 for all affected
request structs: change the struct tags on SignUpRequest.Password,
LoginRequest.Password, and VerifyForgotPasswordRequest.NewPassword to use
`binding:"required,min=12"`, and update any corresponding manual validation
logic or tests that assert the old minimum so they reflect the new 12-character
requirement. Ensure any error messages returned to clients reference the new
minimum length.
In @backend/websocket/websocket.go:
- Around line 692-712: handlePersonaSelection currently defers room.Mutex.Unlock
while later calling snapshotRecipients and broadcastParticipants which reacquire
the same mutex, causing a deadlock; change the locking to only protect the
critical update (lookup and set client.PersonaRole) and release the mutex before
calling snapshotRecipients and broadcastParticipants (remove the deferred unlock
and use a short lock/unlock around the client update), and apply the same
pattern to handleRoleSelection so both functions unlock the room.Mutex before
any calls to snapshotRecipients or broadcastParticipants.
In @frontend/src/Pages/BotSelection.tsx:
- Line 553: The JSX contains a stray space in the closing tag "</div >" (in the
BotSelection component/JSX template) which can break linters/formatters; remove
the extra space so the tag reads "</div>" and run the formatter to ensure no
other JSX closing tags contain trailing spaces.
In @frontend/src/Pages/OnlineDebateRoom.tsx:
- Around line 1320-1324: The loop over pending ICE candidates currently swallows
errors from spectatorPc.addIceCandidate(candidate); update the catch to surface
failures instead of ignoring them: catch the error and log it (including
spectatorPc id/context and the candidate) using the app logger or console.error,
and consider handling or re-throwing fatal errors so spectator streams can retry
or fail visibly; keep the for...of loop and await
spectatorPc.addIceCandidate(candidate) but replace the empty catch block with a
detailed error handling/logging statement referencing spectatorPc, pending and
candidate.
🧹 Nitpick comments (3)
backend/test_server.go (1)
38-39: Remove empty loops or implement actual test logic.These empty loops serve no purpose. If this is test scaffolding, consider either:
- Removing the loops entirely, or
- Implementing actual verification logic (e.g., checking pool length, asserting expected state)
♻️ Proposed cleanup
- for range pool { - }Or implement actual test assertions:
- for range pool { - } + if len(pool) != 2 { + panic(fmt.Sprintf("Expected 2 users in pool, got %d", len(pool))) + }Also applies to: 47-48
frontend/src/services/vsbot.ts (1)
16-25: Prefer a constrainedRoletype (avoid backend/frontend string drift).
Right nowrole?: stringcan silently diverge ("Neutral","neutral ", etc.).Possible typing tweak
+export type Role = "neutral" | "historian" | "scientist" | "lawyer"; + export type DebateRequest = { @@ - role?: string; // Added role field + role?: Role; }; @@ export type DebateResponse = { @@ - role?: string; + role?: Role; phaseTimings?: PhaseTiming[]; // Included in response for consistency };Also applies to: 27-35
backend/services/debatevsbot.go (1)
113-114: Role whitelisting via switch is solid; consider a neutral/default instruction to avoid blank “Role Instructions”.
Right now unknown/empty roles render an empty Role Instructions line in the prompt.Possible tweak
@@ roleInstruction := "" switch strings.ToLower(role) { + case "", "neutral": + roleInstruction = "Act as Neutral: be balanced, fair, and avoid relying on domain-specific authority." case "historian": roleInstruction = "Act as a Historian: use historical examples, timelines, and precedents to support your arguments." case "scientist": roleInstruction = "Act as a Scientist: use empirical evidence, data, and experimental results to support your arguments." case "lawyer": roleInstruction = "Act as a Lawyer: use persuasion, airtight logic, and sharp rebuttals to support your arguments." }Also applies to: 172-181, 188-219, 247-281, 285-306
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.env.examplebackend/config/config.prod.sample.ymlbackend/controllers/debatevsbot_controller.gobackend/models/debate.gobackend/models/debatevsbot.gobackend/routes/rooms.gobackend/services/debatevsbot.gobackend/structs/auth.gobackend/test_server.gobackend/websocket/websocket.gofrontend/src/Pages/Authentication/forms.tsxfrontend/src/Pages/BotSelection.tsxfrontend/src/Pages/DebateRoom.tsxfrontend/src/Pages/OnlineDebateRoom.tsxfrontend/src/services/vsbot.ts
💤 Files with no reviewable changes (2)
- .env.example
- backend/config/config.prod.sample.yml
🧰 Additional context used
🧬 Code graph analysis (5)
backend/services/debatevsbot.go (2)
backend/services/personalities.go (2)
BotPersonality(3-26)GetBotPersonality(28-1028)backend/models/debatevsbot.go (1)
Message(6-10)
frontend/src/Pages/DebateRoom.tsx (1)
frontend/src/services/vsbot.ts (1)
concedeDebate(105-120)
backend/websocket/websocket.go (2)
backend/routes/rooms.go (1)
Room(21-26)backend/structs/websocket.go (2)
Room(30-37)Message(10-13)
frontend/src/Pages/BotSelection.tsx (1)
frontend/src/components/ui/select.tsx (5)
Select(146-146)SelectTrigger(149-149)SelectValue(148-148)SelectContent(150-150)SelectItem(152-152)
backend/controllers/debatevsbot_controller.go (1)
backend/services/debatevsbot.go (1)
GenerateBotResponse(285-306)
🔇 Additional comments (9)
frontend/src/Pages/Authentication/forms.tsx (3)
8-8: Good centralization of password length validation.Defining MIN_PASSWORD_LENGTH as a constant provides a single source of truth and makes future updates easier. The implementation is consistent across all authentication forms.
However, note the security concern raised in
backend/structs/auth.goregarding the 6-character minimum.
29-37: LGTM: Consistent password validation across forms.The password length validation is correctly implemented across LoginForm, SignUpForm, and ResetPasswordForm, with clear error messages that reference the MIN_PASSWORD_LENGTH constant.
Also applies to: 143-146, 352-355
42-48: LGTM: Formatting improvements.The handleGoogleLogin callback reformatting improves readability without changing behavior.
Also applies to: 156-162
backend/models/debate.go (1)
24-25: LGTM: Role fields support multiplayer debate tracking.Adding Role and OpponentRole fields enables tracking roles for both participants in online debates, which aligns with the PR's role-based debate mode objectives. The fields are properly tagged for persistence and serialization.
The implementation safely handles empty Role/OpponentRole values through defensive checks (e.g.,
if opponentRole != ""), so existing debates without these fields will continue to work without issues.frontend/src/Pages/BotSelection.tsx (1)
212-213: Role selection plumbing looks consistent end-to-end.
State defaulting to"neutral"+ sending in payload + passing through navigation is aligned with the new badge rendering.Also applies to: 278-287, 292-301, 495-511, 553-553
backend/websocket/websocket.go (4)
53-53: LGTM!The new
PersonaRolefield follows the existing pattern used for theRolefield and correctly defaults to an empty string.
86-90: LGTM!The debate-specific fields are properly structured. Using
*boolforReadycorrectly distinguishes betweenfalseand not-set values.
178-178: LGTM!The persona field is correctly included in the participant data, following the same pattern as the existing
rolefield.
472-473: LGTM!The message routing follows the established pattern for other message types.
| History []Message `json:"history" bson:"history"` | ||
| PhaseTimings []PhaseTiming `json:"phaseTimings" bson:"phaseTimings"` // Added for custom timings | ||
| Outcome string `json:"outcome" bson:"outcome"` // Result of the debate (e.g., "User wins") | ||
| Role string `json:"role" bson:"role"` // User's role (historian, scientist, lawyer, neutral) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find the backend/models/debatevsbot.go file and examine the struct
echo "=== Checking DebateVsBot struct ==="
cat -n backend/models/debatevsbot.go | head -50
echo -e "\n=== Searching for DebateVsBot creation patterns ==="
ast-grep --pattern $'DebateVsBot{
$$$
}'
echo -e "\n=== Searching for Role field usage ==="
rg -nP --type=go '\bRole\b' -A2 -B2
echo -e "\n=== Searching for DebateVsBot initialization ==="
rg -nP --type=go 'new\(|DebateVsBot\s*\{|&DebateVsBot' -A5Repository: AOSSIE-Org/DebateAI
Length of output: 20321
🏁 Script executed:
# Check if there's any validation logic for Role field in controllers or services
rg -nP --type=go 'if.*Role|Role.*!=' -A2 -B2 | head -40
# Check the switch statement in debatevsbot.go service to see all handled cases
rg -nP --type=go 'switch.*role|case.*historian|case.*scientist|case.*lawyer|case.*neutral' -A1 -B1Repository: AOSSIE-Org/DebateAI
Length of output: 2946
🏁 Script executed:
# Check the full switch statement in debatevsbot.go service to see if "neutral" case exists
sed -n '173,195p' backend/services/debatevsbot.go
# Also check if there's validation elsewhere in the debatevsbot controller
rg -nP --type=go 'CreateDebateVsBot|UpdateDebateVsBot' -A30 | grep -i "valid\|error\|role"Repository: AOSSIE-Org/DebateAI
Length of output: 1760
🏁 Script executed:
# Check if "neutral" is explicitly handled anywhere
rg -nP --type=go '"neutral"' | head -20
# Get full context of the switch statement and what happens after it
sed -n '173,200p' backend/services/debatevsbot.goRepository: AOSSIE-Org/DebateAI
Length of output: 1477
Add validation for Role field or implement default "neutral" handling.
The Role field is properly tagged for serialization, but the code lacks validation and doesn't implement the documented "neutral" role. The switch statement in backend/services/debatevsbot.go (lines 173–184) handles only historian, scientist, and lawyer—if Role is empty or invalid, roleInstruction remains empty, producing degraded prompt instructions.
Recommend either:
- Validating Role values in the controller and rejecting invalid requests, or
- Defaulting empty/missing Role to "neutral" and adding the corresponding case to the switch statement for backward compatibility with existing debates.
🤖 Prompt for AI Agents
In @backend/models/debatevsbot.go at line 30, The Role field on the DebateVsBot
model can be empty/invalid and roleInstruction in the switch inside
backend/services/debatevsbot.go (the logic handling historian/scientist/lawyer)
ends up empty; either validate Role on incoming requests (reject if not one of
"historian","scientist","lawyer","neutral") in the controller that constructs
the model, or make Role default to "neutral" when missing and add a "neutral"
case to the switch that sets a proper neutral roleInstruction so prompts never
get an empty roleInstruction.
| type SignUpRequest struct { | ||
| Email string `json:"email" binding:"required,email"` | ||
| Password string `json:"password" binding:"required,min=8"` | ||
| Password string `json:"password" binding:"required,min=6"` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n backend/structs/auth.goRepository: AOSSIE-Org/DebateAI
Length of output: 1043
🌐 Web query:
NIST SP 800-63B password minimum length requirements user authentication
💡 Result:
Short answer:
- For memorized secrets (passwords): NIST SP 800-63B requires a minimum of 8 characters, and for single‑factor password authentication it mandates a minimum of 15 characters; verifiers SHOULD permit a maximum of at least 64 characters and SHOULD accept ASCII (including space) and Unicode. [1]. (pages.nist.gov)
If you want the exact text/section references, I can pull and quote them.
Citations:
🌐 Web query:
OWASP password minimum length requirements recommendations
💡 Result:
Short summary of OWASP recommendations for password minimum length and related rules:
- Minimum length: OWASP AASVS recommends at least 12 characters for user-set passwords. [1] (owasp-aasvs4.readthedocs.io)
- OWASP Authentication Cheat Sheet references NIST SP 800-63B guidance: with MFA, passwords shorter than 8 chars are weak; without MFA, passwords shorter than 15 chars are weak. (Use this to set minimums appropriate to your threat model.) [2] (cheatsheetseries.owasp.org)
- Related guidance: allow long passphrases (typical max ≥64–128 chars), permit spaces and Unicode, do not silently truncate, and check passwords against breached/commonly-used lists. [3] (owasp.deteact.com)
If you want, I can produce a short policy (minimum + max + checks + UX text) based on these recommendations.
Citations:
- 1: https://owasp-aasvs4.readthedocs.io/en/latest/2.1.1.html?utm_source=openai
- 2: https://cheatsheetseries.owasp.org/cheatsheets/Authentication_Cheat_Sheet.html?utm_source=openai
- 3: https://owasp.deteact.com/cheat/cheatsheets/Password_Storage_Cheat_Sheet.html?utm_source=openai
Increase minimum password length to at least 12 characters.
The current 6-character minimum is below security best practices. NIST SP 800-63B recommends a minimum of 8 characters for memorized secrets, or 15 characters for single-factor password authentication without MFA. OWASP AASVS explicitly recommends a minimum of 12 characters for user-set passwords. A 6-character password significantly reduces the keyspace and increases vulnerability to brute-force attacks.
This applies to all password fields: SignUpRequest.Password (line 5), LoginRequest.Password (line 15), and VerifyForgotPasswordRequest.NewPassword (line 25).
🤖 Prompt for AI Agents
In @backend/structs/auth.go at line 5, Update the password validation minimum
from 6 to 12 for all affected request structs: change the struct tags on
SignUpRequest.Password, LoginRequest.Password, and
VerifyForgotPasswordRequest.NewPassword to use `binding:"required,min=12"`, and
update any corresponding manual validation logic or tests that assert the old
minimum so they reflect the new 12-character requirement. Ensure any error
messages returned to clients reference the new minimum length.
| // handlePersonaSelection handles persona role selection | ||
| func handlePersonaSelection(room *Room, conn *websocket.Conn, message Message, roomID string) { | ||
| // Store the persona in the client | ||
| room.Mutex.Lock() | ||
| defer room.Mutex.Unlock() | ||
| if client, exists := room.Clients[conn]; exists { | ||
| if client.IsSpectator { | ||
| return | ||
| } | ||
| client.PersonaRole = message.Persona | ||
| } | ||
|
|
||
| // Broadcast persona selection to other clients | ||
| for _, r := range snapshotRecipients(room, conn) { | ||
| if err := r.SafeWriteJSON(message); err != nil { | ||
| } | ||
| } | ||
|
|
||
| // Send updated participant snapshot to everyone | ||
| broadcastParticipants(room) | ||
| } |
There was a problem hiding this comment.
Deadlock: mutex held while calling locking functions.
The function holds room.Mutex via defer while calling snapshotRecipients and broadcastParticipants, both of which attempt to acquire the same mutex. Since Go's sync.Mutex is not reentrant, this causes a deadlock when any client sends a personaSelection message.
Note: The same bug exists in handleRoleSelection (lines 671-690). Compare with handlePhaseChange which correctly releases the lock before broadcasting.
🔒️ Proposed fix to release lock before broadcasting
func handlePersonaSelection(room *Room, conn *websocket.Conn, message Message, roomID string) {
// Store the persona in the client
room.Mutex.Lock()
- defer room.Mutex.Unlock()
if client, exists := room.Clients[conn]; exists {
if client.IsSpectator {
+ room.Mutex.Unlock()
return
}
client.PersonaRole = message.Persona
}
+ room.Mutex.Unlock()
// Broadcast persona selection to other clients
for _, r := range snapshotRecipients(room, conn) {
if err := r.SafeWriteJSON(message); err != nil {
}
}
// Send updated participant snapshot to everyone
broadcastParticipants(room)
}Consider also fixing the same issue in handleRoleSelection:
func handleRoleSelection(room *Room, conn *websocket.Conn, message Message, roomID string) {
room.Mutex.Lock()
- defer room.Mutex.Unlock()
if client, exists := room.Clients[conn]; exists {
if client.IsSpectator {
+ room.Mutex.Unlock()
return
}
client.Role = message.Role
}
+ room.Mutex.Unlock()
for _, r := range snapshotRecipients(room, conn) {
if err := r.SafeWriteJSON(message); err != nil {
}
}
broadcastParticipants(room)
}📝 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.
| // handlePersonaSelection handles persona role selection | |
| func handlePersonaSelection(room *Room, conn *websocket.Conn, message Message, roomID string) { | |
| // Store the persona in the client | |
| room.Mutex.Lock() | |
| defer room.Mutex.Unlock() | |
| if client, exists := room.Clients[conn]; exists { | |
| if client.IsSpectator { | |
| return | |
| } | |
| client.PersonaRole = message.Persona | |
| } | |
| // Broadcast persona selection to other clients | |
| for _, r := range snapshotRecipients(room, conn) { | |
| if err := r.SafeWriteJSON(message); err != nil { | |
| } | |
| } | |
| // Send updated participant snapshot to everyone | |
| broadcastParticipants(room) | |
| } | |
| // handlePersonaSelection handles persona role selection | |
| func handlePersonaSelection(room *Room, conn *websocket.Conn, message Message, roomID string) { | |
| // Store the persona in the client | |
| room.Mutex.Lock() | |
| if client, exists := room.Clients[conn]; exists { | |
| if client.IsSpectator { | |
| room.Mutex.Unlock() | |
| return | |
| } | |
| client.PersonaRole = message.Persona | |
| } | |
| room.Mutex.Unlock() | |
| // Broadcast persona selection to other clients | |
| for _, r := range snapshotRecipients(room, conn) { | |
| if err := r.SafeWriteJSON(message); err != nil { | |
| } | |
| } | |
| // Send updated participant snapshot to everyone | |
| broadcastParticipants(room) | |
| } |
🤖 Prompt for AI Agents
In @backend/websocket/websocket.go around lines 692 - 712,
handlePersonaSelection currently defers room.Mutex.Unlock while later calling
snapshotRecipients and broadcastParticipants which reacquire the same mutex,
causing a deadlock; change the locking to only protect the critical update
(lookup and set client.PersonaRole) and release the mutex before calling
snapshotRecipients and broadcastParticipants (remove the deferred unlock and use
a short lock/unlock around the client update), and apply the same pattern to
handleRoleSelection so both functions unlock the room.Mutex before any calls to
snapshotRecipients or broadcastParticipants.
| </div> | ||
| </div> | ||
| </div> | ||
| </div > |
There was a problem hiding this comment.
Suspicious JSX closing tag spacing (</div >).
This can trip formatters/linters; safest to normalize.
Proposed fix
- </div >
+ </div>📝 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.
| </div > | |
| </div> |
🤖 Prompt for AI Agents
In @frontend/src/Pages/BotSelection.tsx at line 553, The JSX contains a stray
space in the closing tag "</div >" (in the BotSelection component/JSX template)
which can break linters/formatters; remove the extra space so the tag reads
"</div>" and run the formatter to ensure no other JSX closing tags contain
trailing spaces.
| break; | ||
| case "personaSelection": | ||
| if (data.persona) setPeerPersonaRole(data.persona); | ||
| break; | ||
| case "phaseChange": |
There was a problem hiding this comment.
Persona sync depends on server payload shape; currently brittle.
You consume opponentParticipant.persona (Line 1263) and a WS personaSelection event (Line 1150), but the REST participants endpoint in backend/routes/rooms.go exposes role (and currently doesn’t include it in the response body), not persona.
Also applies to: 1263-1266, 2010-2014
| for (const candidate of pending) { | ||
| try { | ||
| await spectatorPc.addIceCandidate(candidate); | ||
| } catch (err) {} | ||
| } catch (err) { } | ||
| } |
There was a problem hiding this comment.
Don’t swallow addIceCandidate errors silently.
Line 1323 hides failures that can break spectator streams and makes issues un-debuggable.
Proposed fix
- } catch (err) { }
+ } catch (err) {
+ console.debug("Failed to add pending ICE candidate:", err);
+ }📝 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.
| for (const candidate of pending) { | |
| try { | |
| await spectatorPc.addIceCandidate(candidate); | |
| } catch (err) {} | |
| } catch (err) { } | |
| } | |
| for (const candidate of pending) { | |
| try { | |
| await spectatorPc.addIceCandidate(candidate); | |
| } catch (err) { | |
| console.debug("Failed to add pending ICE candidate:", err); | |
| } | |
| } |
🤖 Prompt for AI Agents
In @frontend/src/Pages/OnlineDebateRoom.tsx around lines 1320 - 1324, The loop
over pending ICE candidates currently swallows errors from
spectatorPc.addIceCandidate(candidate); update the catch to surface failures
instead of ignoring them: catch the error and log it (including spectatorPc
id/context and the candidate) using the app logger or console.error, and
consider handling or re-throwing fatal errors so spectator streams can retry or
fail visibly; keep the for...of loop and await
spectatorPc.addIceCandidate(candidate) but replace the empty catch block with a
detailed error handling/logging statement referencing spectatorPc, pending and
candidate.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/src/components/JudgementPopup.tsx:
- Line 185: player2Desc currently uses resolvedAgainstName as a fallback which
duplicates player2Name; change the fallback so player2Desc is role-specific or
distinct (e.g., use botDesc when isUserBotFormat is true, otherwise use a
neutral role label like 'Opponent' or an empty string) so player2Name
(resolvedAgainstName) is not repeated; update the assignment of player2Desc (the
const player2Desc = ...) accordingly and ensure any UI code expecting a
description handles the new fallback.
🧹 Nitpick comments (2)
frontend/src/components/JudgementPopup.tsx (1)
146-165: Consider extracting name resolution logic for readability.The nested ternaries correctly handle all cases but are difficult to follow. Consider extracting into a helper function or using early returns for clarity.
♻️ Optional refactor for readability
const resolvePlayerNames = () => { if (isUserBotFormat) { return { forName: defaultForName, againstName: defaultAgainstName, }; } if (localRole === 'for') { return { forName: resolvedLocalName, againstName: resolvedOpponentName }; } if (localRole === 'against') { return { forName: resolvedOpponentName, againstName: resolvedLocalName }; } return { forName: defaultForName, againstName: defaultAgainstName }; }; const { forName: resolvedForName, againstName: resolvedAgainstName } = resolvePlayerNames();backend/services/debatevsbot.go (1)
172-181: Role-based instruction mapping looks good.The case-insensitive switch correctly maps the three specialized roles to their argumentation styles. The default empty string for neutral/unknown roles provides graceful degradation.
Consider adding a brief comment clarifying that "neutral" intentionally maps to no special instruction:
💡 Optional: Add clarifying comment
// Role-based instructions roleInstruction := "" switch strings.ToLower(role) { case "historian": roleInstruction = "Act as a Historian: use historical examples, timelines, and precedents to support your arguments." case "scientist": roleInstruction = "Act as a Scientist: use empirical evidence, data, and experimental results to support your arguments." case "lawyer": roleInstruction = "Act as a Lawyer: use persuasion, airtight logic, and sharp rebuttals to support your arguments." +// "neutral" or empty role intentionally receives no special instruction }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/services/debatevsbot.gobackend/services/matchmaking.gofrontend/src/components/JudgementPopup.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
backend/services/debatevsbot.go (4)
backend/services/personalities.go (1)
BotPersonality(3-26)backend/models/debatevsbot.go (1)
Message(6-10)backend/websocket/websocket.go (1)
Message(72-96)backend/services/ai.go (1)
Message(21-24)
🔇 Additional comments (6)
frontend/src/components/JudgementPopup.tsx (3)
137-144: LGTM!The type guard on line 137 correctly discriminates between
JudgmentDataUserBotandJudgmentDataForAgainstformats. The fallback chains for names and avatars are well-structured.
287-295: Formatting-only changes.These are indentation adjustments with no functional impact.
654-676: Formatting-only changes in className template literals.No functional changes; just indentation adjustments in the conditional class assignments.
backend/services/matchmaking.go (1)
93-98: LGTM!Good simplification. Go's built-in
deletefunction is safely a no-op when the key doesn't exist in the map, so the prior existence check was redundant. The mutex protection is correctly maintained.backend/services/debatevsbot.go (2)
188-218: LGTM!The opening statement prompt template correctly integrates role instructions. Format string argument count is correct (13 placeholders, 13 arguments).
283-306: LGTM!The
GenerateBotResponsefunction correctly propagates the newroleparameter toconstructPrompt. The format string arguments in the continuation template are correctly balanced (15 placeholders, 15 arguments).
|
|
||
| const player1Avatar = resolvedForAvatar || localAvatar; | ||
| const player2Avatar = resolvedAgainstAvatar || opponentAvatar; | ||
| const player2Desc = isUserBotFormat ? botDesc : resolvedAgainstName || 'Debater'; |
There was a problem hiding this comment.
player2Desc fallback may display redundant information.
For non-bot format, player2Desc falls back to resolvedAgainstName, which is already displayed as player2Name (line 163). This could result in showing the opponent's name twice in the profile section (lines 363 and 371).
Consider using a role-specific description or a distinct fallback:
-const player2Desc = isUserBotFormat ? botDesc : resolvedAgainstName || 'Debater';
+const player2Desc = isUserBotFormat ? botDesc : 'Debater';📝 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 player2Desc = isUserBotFormat ? botDesc : resolvedAgainstName || 'Debater'; | |
| const player2Desc = isUserBotFormat ? botDesc : 'Debater'; |
🤖 Prompt for AI Agents
In @frontend/src/components/JudgementPopup.tsx at line 185, player2Desc
currently uses resolvedAgainstName as a fallback which duplicates player2Name;
change the fallback so player2Desc is role-specific or distinct (e.g., use
botDesc when isUserBotFormat is true, otherwise use a neutral role label like
'Opponent' or an empty string) so player2Name (resolvedAgainstName) is not
repeated; update the assignment of player2Desc (the const player2Desc = ...)
accordingly and ensure any UI code expecting a description handles the new
fallback.
1 participant
🎭 Role-Based Debate Mode (Historian / Scientist / Lawyer)
Summary
This PR introduces Role-Based Debate Mode, adding a layer of specialization to both AI bot debates and online multiplayer rooms. Participants can now adopt predefined personas that either:
This enhancement improves educational depth, argument diversity, and debate structure while remaining fully backward-compatible.
🎯 Core Concept
Participants can select one of four roles before a debate begins:
If no role is selected, the system safely defaults to Neutral.
🤖 Bot Debate Logic (AI Influence)
In single-player (User vs Bot) debates, the selected role is passed directly to the AI.
How it works
Prompt Injection
The backend dynamically modifies the system prompt based on the selected role.
Behavioral Adaptation
This allows the same AI model to exhibit different debate styles without introducing new models or breaking existing logic.
##Screenshot
🎨 User Interface Integration
Role Selection
Live Debate Room
Once the debate starts:
Role Badges
A small, high-contrast badge (e.g.,
HISTORIAN,SCIENTIST) appears next to each participant’s name.Real-Time Synchronization
In online rooms, persona changes during setup are synchronized instantly via WebSockets.
⚙️ Technical Implementation
Backend (Go)
Models
DebateDebateVsBotRolefield with safe defaultsAI Prompt Logic
services/debatevsbot.goconstructPromptto:WebSockets
websocket/websocket.gopersonaSelectionmessage typeControllers
controllers/debatevsbot_controller.goRolefieldFrontend (React + TypeScript)
Services
services/vsbot.tsUI Changes
BotSelection.tsx
DebateRoom.tsx (Bot Debate)
OnlineDebateRoom.tsx (User vs User)
✅ Verification & Testing
Backend Build
go build ./...
Exit code: 0
Minor pre-existing lint issues in test_server.go were resolved to ensure successful compilation.
Prompt Construction
Verified that the AI receives correct role-specific instructions:
Historian → historical precedents
Scientist → empirical reasoning
Lawyer → structured rebuttals
Backward Compatibility
Existing debates without roles default to Neutral
No breaking API changes
Old data remains fully functional
🔁 How to Reproduce
Observe:
📈 Impact
🧩 Notes
closes #200
Summary by CodeRabbit
New Features
Other Changes
✏️ Tip: You can customize this high-level summary in your review settings.