Polished the user vs bot debate flow #102
Conversation
WalkthroughThe changes unify and update the user rating system across backend and frontend components. Backend controllers, models, and utilities rename Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DebateRoom (Frontend)
participant Bot
participant Backend
User->>DebateRoom: Sends message / starts turn
DebateRoom->>Bot: Sends user message
Bot-->>DebateRoom: Returns bot response
DebateRoom->>User: Displays bot response, disables input, shows "Next Turn" button
User->>DebateRoom: Clicks "Next Turn"
DebateRoom->>DebateRoom: Advances to next turn, enables input
Possibly related PRs
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (2)
backend/rating/glicko2.go (1)
52-52: Consider configuration immutability.Making the
Configfield public enables external access, which supports the usage pattern inpopulate.go. However, consider making the configuration immutable after initialization to prevent accidental modifications.Example approach:
+// GetConfig returns a copy of the configuration to prevent modifications +func (g *Glicko2) GetConfig() Config { + return *g.Config +}frontend/src/Pages/DebateRoom.tsx (1)
339-340: Simplify the boolean expression.The ternary operator is unnecessary when the result is a boolean.
Apply this diff to simplify:
- isBotTurn: - prevState.userStance === phaseSequences[newPhase][0] ? false : true, + isBotTurn: prevState.userStance !== phaseSequences[newPhase][0],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
backend/controllers/auth.go(2 hunks)backend/controllers/leaderboard.go(2 hunks)backend/controllers/profile_controller.go(6 hunks)backend/models/debate.go(1 hunks)backend/rating/glicko2.go(3 hunks)backend/routes/debate.go(2 hunks)backend/routes/leaderboard.go(0 hunks)backend/routes/rooms.go(2 hunks)backend/services/rating_service.go(4 hunks)backend/utils/debate.go(1 hunks)backend/utils/populate.go(1 hunks)backend/utils/user.go(2 hunks)frontend/src/Pages/DebateRoom.tsx(19 hunks)
💤 Files with no reviewable changes (1)
- backend/routes/leaderboard.go
🧰 Additional context used
🧬 Code Graph Analysis (4)
backend/routes/debate.go (1)
backend/db/db.go (1)
MongoDatabase(17-17)
backend/services/rating_service.go (3)
backend/rating/glicko2.go (2)
Glicko2(51-53)Player(21-26)backend/models/debate.go (1)
Debate(10-25)backend/db/db.go (1)
MongoDatabase(17-17)
frontend/src/Pages/DebateRoom.tsx (3)
backend/models/debatevsbot.go (1)
Message(6-10)frontend/src/components/ui/button.tsx (1)
Button(57-57)frontend/src/components/ui/input.tsx (1)
Input(25-25)
backend/controllers/profile_controller.go (2)
backend/db/db.go (1)
MongoDatabase(17-17)backend/models/user.go (1)
User(10-30)
🪛 Biome (1.9.4)
frontend/src/Pages/DebateRoom.tsx
[error] 340-340: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
🔇 Additional comments (28)
backend/routes/debate.go (2)
4-4: LGTM: Import updates support the new database access pattern.The addition of "context" and "arguehub/db" imports aligns with the direct database access pattern being implemented across the codebase.
Also applies to: 9-9
60-60: Direct Database Access Pattern VerifiedThe search confirmed there are no remaining
db.GetCollectioncalls and all collection usages now usedb.MongoDatabase.Collection(). The direct access pattern is consistently applied across the codebase.backend/utils/debate.go (1)
30-30: LGTM: Consistent field renaming from EloChange to RatingChange.All sample debate entries have been updated consistently to use the new
RatingChangefield name, aligning with the broader rating terminology standardization across the codebase.Also applies to: 37-37, 44-44, 51-51, 58-58
backend/controllers/auth.go (2)
78-78: LGTM: Consistent field renaming in Google login user creation.The field rename from
EloRatingtoRatingmaintains the same initialization value (1200) and aligns with the codebase-wide rating terminology standardization.
147-147: LGTM: Consistent field renaming in sign-up user creation.The field rename from
EloRatingtoRatingmaintains the same initialization value (1200) and aligns with the codebase-wide rating terminology standardization.backend/routes/rooms.go (2)
67-67: Excellent approach: Preserving database compatibility while updating field naming.The field rename from
EloRatingtoRatingwhile maintaining the BSON tag as"eloRating"is a best practice that ensures backward compatibility with existing database records while adopting the new rating terminology in the codebase.
80-80: LGTM: Assignment updated to match the new field name.The assignment correctly uses
user.Ratingto match the updated struct field name while maintaining the same functionality.backend/utils/user.go (1)
24-24: LGTM: Consistent field renaming in sample user data.All sample users have been updated consistently to use the new
Ratingfield name, maintaining the same rating values while aligning with the codebase-wide rating terminology standardization.Also applies to: 32-32, 40-40
backend/models/debate.go (1)
13-13: Confirm database migration strategy for renamed fieldNo remaining code references to the old
userEmailfield were found, but this change still breaks existing documents and API clients. Please ensure:
- A migration exists to rename or backfill the BSON field from
userEmail→- API clients that expect
userEmailare either updated or the service continues to accept/emit both names until the migration is complete- Any tests, fixtures or serialized schemas (e.g. GraphQL, OpenAPI, mock data) are updated to use
backend/controllers/leaderboard.go (1)
49-49: LGTM on comment update.The comment correctly reflects the unified rating field naming.
backend/rating/glicko2.go (2)
60-60: LGTM on constructor update.Correctly updated to match the public field name.
66-68: LGTM on config field access updates.All method updates correctly reference the public
Configfield. The logic remains unchanged.Also applies to: 109-109, 115-115, 121-121, 126-126, 171-171
backend/services/rating_service.go (5)
10-10: LGTM on import cleanup.Correctly removed unused imports and added the necessary database import.
22-24: LGTM on getter function addition.The
GetRatingSystem()function provides proper access to the singleton rating system instance and follows Go conventions.
66-66: LGTM on field name consistency.Correctly updated to match the
58-58: LGTM on code cleanup.Minor whitespace cleanup improves readability.
94-94: MongoDatabase initialization verified. TheMongoDatabaseis properly set inbackend/db/db.go:52viaMongoDatabase = client.Database(dbName), anddb.ConnectMongoDB(cfg.Database.URI)is called incmd/server/main.gobefore any request handlers run. No further action required.backend/utils/populate.go (3)
23-23: LGTM on database access standardization.Consistent with the direct database collection access pattern used across the codebase.
31-32: LGTM on rating system initialization.Proper initialization of the rating service with configuration and use of the new getter function. The pattern ensures the rating system is properly configured before use.
39-41: LGTM on config field access.Correctly updated to use the public
Configfield, consistent with the changes in the rating system.Also applies to: 48-50
frontend/src/Pages/DebateRoom.tsx (6)
162-162: Good addition of manual turn control state.The
nextTurnPendingstate effectively tracks when the bot has completed its turn and is waiting for user confirmation to proceed.
268-270: Timer logic correctly updated for manual turn control.The bot turn timer expiration now properly sets
nextTurnPendinginstead of automatically advancing, aligning with the new manual control flow.
357-362: Clean implementation of manual turn advancement.The
handleNextTurnfunction properly uses the setState callback pattern to ensure state consistency when advancing turns.
424-428: Robust bot response handling.Excellent error handling - setting
nextTurnPendingin both success and error cases ensures the user can always advance the debate even if the bot fails.
600-607: Well-implemented Next Turn button UI.The button is properly positioned, conditionally rendered, and styled with an intuitive green color to indicate the user should proceed.
672-674: Consistent input disabling during pending state.All user inputs (text input, microphone, send button) are properly disabled when
nextTurnPendingis true, preventing accidental inputs while waiting for turn advancement.Also applies to: 686-688, 699-701
backend/controllers/profile_controller.go (2)
20-30: Well-implemented Elo rating calculation.The function correctly implements the standard Elo rating formula with float64 precision for more accurate calculations. The K-factor of 32 is appropriate for a competitive system.
247-262: Clean debate logging implementation.Using BSON maps for debate insertion is efficient and the documents properly capture all necessary fields including the float64 rating values.
|
|
||
| // PopulateTestUsers inserts sample users into the database | ||
| func PopulateTestUsers() { | ||
| func PopulateTestUsers1() { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Clarify the function rename from PopulateTestUsers to PopulateTestUsers1.
The function renaming to PopulateTestUsers1 may indicate multiple versions, incomplete migration, or testing scenarios. Please clarify if this is intentional or if there are additional related functions that need to be updated consistently.
🏁 Script executed:
#!/bin/bash
# Description: Check for other PopulateTestUsers functions and their usage
# Expected: Understand if there are multiple versions and their usage patterns
# Search for any PopulateTestUsers functions
rg "PopulateTestUsers" --type go -A 2 -B 2
# Search for function calls to understand usage
rg "PopulateTestUsers" --type goLength of output: 1261
Remove or rename the unused PopulateTestUsers1 helper
It looks like you’ve added a second test-user population function with a “1” suffix that isn’t called anywhere, while the original PopulateTestUsers in backend/utils/populate.go is still being used in backend/cmd/server/main.go. This can be confusing and may be a leftover from testing.
• backend/utils/user.go (around line 14): func PopulateTestUsers1() { … } is never invoked
• backend/utils/populate.go: func PopulateTestUsers() { … } remains the only one used in main.go
• backend/cmd/server/main.go: still calls utils.PopulateTestUsers(), not the new variant
Please clarify your intent:
- If
PopulateTestUsers1contains improved logic, replace the call inmain.go(and remove the old version) or give it a descriptive name. - Otherwise, remove
PopulateTestUsers1to avoid dead code and confusion.
🤖 Prompt for AI Agents
In backend/utils/user.go at line 14, the function PopulateTestUsers1 is defined
but never called, causing dead code and potential confusion. Review the
function's logic: if it improves upon the existing PopulateTestUsers in
backend/utils/populate.go, update main.go to call PopulateTestUsers1 instead and
remove the old function; otherwise, delete PopulateTestUsers1 entirely to clean
up unused code.
| Rank: i + 1, | ||
| Name: name, | ||
| Score: user.EloRating, | ||
| Score: int(user.Rating), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify database field consistency.
The code uses user.Rating (line 86) but the MongoDB query sorts by eloRating (line 51). This inconsistency suggests either:
- The database field is still named
eloRatingbut the Go struct usesRating - The query needs updating to use
rating
Ensure the BSON tags in the User model match the actual database field names.
🏁 Script executed:
#!/bin/bash
# Description: Check User model BSON tags and database field consistency
# Expected: Verify the actual field name used in database vs Go struct
echo "=== Checking User model rating field BSON tags ==="
ast-grep --pattern 'type User struct {
$$$
}'
echo "=== Checking for rating field references ==="
rg -A 2 -B 2 "eloRating|rating.*bson" --type goLength of output: 5182
Ensure consistent use of rating instead of eloRating.
Your User model now uses bson:"rating", but the code still references eloRating in multiple places. Update all of these to avoid mismatches at query and marshalling time:
• backend/routes/rooms.go
– Change the struct tag on the Rating field:
diff - Rating int `bson:"eloRating"` + Rating int `bson:"rating"`
• backend/controllers/leaderboard.go
– Update the sort key in findOptions:
diff - SetSort(bson.D{{"eloRating", -1}}) + SetSort(bson.D{{"rating", -1}})
• backend/controllers/profile_controller.go
– Change all occurrences of "eloRating" to "rating" in:
• the initial Find().SetSort(...) call
• the BSON payload in Create/Log operations ("eloRating": int(user.Rating))
• the UpdateOne calls ("$set": bson.M{"eloRating": newWinnerElo} → {"rating": newWinnerElo})
• the logged document fields ("eloRating": newWinnerElo → "rating": newWinnerElo)
After updating, verify your MongoDB collection’s field name (and any existing indexes) have been migrated from eloRating to rating.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/controllers/leaderboard.go at line 86 and related files, replace all
references to "eloRating" with "rating" to match the updated User model's BSON
tag. Specifically, update the struct tag in backend/routes/rooms.go, change the
sort key in findOptions in backend/controllers/leaderboard.go, and modify all
occurrences in backend/controllers/profile_controller.go including
Find().SetSort calls, BSON payloads in Create/Log operations, UpdateOne calls,
and logged document fields. After these code changes, verify that the MongoDB
collection fields and indexes have been migrated from "eloRating" to "rating" to
ensure consistency and prevent query or marshalling errors.
| cfg, err := config.LoadConfig("./config/config.prod.yml") | ||
| if err != nil { | ||
| log.Fatalf("Failed to load config: %v", err) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make config path environment-aware.
While explicit config loading is good, hardcoding the production config path (config.prod.yml) may not be appropriate for all environments. Consider making this configurable based on environment variables or build tags.
-cfg, err := config.LoadConfig("./config/config.prod.yml")
+configPath := os.Getenv("CONFIG_PATH")
+if configPath == "" {
+ configPath = "./config/config.dev.yml" // default for development
+}
+cfg, err := config.LoadConfig(configPath)🤖 Prompt for AI Agents
In backend/utils/populate.go around lines 18 to 21, the config file path is
hardcoded to "./config/config.prod.yml", which is not flexible for different
environments. Modify the code to read the config file path from an environment
variable, falling back to a default if the variable is not set. This will make
the config loading environment-aware and adaptable to various deployment
scenarios.
| dbCtx, | ||
| bson.M{}, | ||
| options.Find().SetSort(bson.D{{"eloRating", -1}}).SetLimit(5), | ||
| ) |
There was a problem hiding this comment.
Fix field name inconsistency in leaderboard query.
The query sorts by "eloRating" but based on the User model, the field is named "rating".
Apply this diff to fix the field name:
- options.Find().SetSort(bson.D{{"eloRating", -1}}).SetLimit(5),
+ options.Find().SetSort(bson.D{{"rating", -1}}).SetLimit(5),📝 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.
| dbCtx, | |
| bson.M{}, | |
| options.Find().SetSort(bson.D{{"eloRating", -1}}).SetLimit(5), | |
| ) | |
| dbCtx, | |
| bson.M{}, | |
| options.Find().SetSort(bson.D{{"rating", -1}}).SetLimit(5), | |
| ) |
🤖 Prompt for AI Agents
In backend/controllers/profile_controller.go around lines 69 to 72, the
leaderboard query sorts by the field "eloRating" which is inconsistent with the
User model where the field is named "rating". Update the sort field name in the
options.Find().SetSort call from "eloRating" to "rating" to fix this
inconsistency.
| db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": winnerID}, bson.M{"$set": bson.M{"eloRating": newWinnerElo}}) | ||
| db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": loserID}, bson.M{"$set": bson.M{"eloRating": newLoserElo}}) |
There was a problem hiding this comment.
Fix field name inconsistency in rating updates.
The updates use "eloRating" but the User model field is "rating".
Apply this diff to fix the field names:
- db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": winnerID}, bson.M{"$set": bson.M{"eloRating": newWinnerElo}})
- db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": loserID}, bson.M{"$set": bson.M{"eloRating": newLoserElo}})
+ db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": winnerID}, bson.M{"$set": bson.M{"rating": newWinnerElo}})
+ db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": loserID}, bson.M{"$set": bson.M{"rating": newLoserElo}})📝 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.
| db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": winnerID}, bson.M{"$set": bson.M{"eloRating": newWinnerElo}}) | |
| db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": loserID}, bson.M{"$set": bson.M{"eloRating": newLoserElo}}) | |
| db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": winnerID}, bson.M{"$set": bson.M{"rating": newWinnerElo}}) | |
| db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": loserID}, bson.M{"$set": bson.M{"rating": newLoserElo}}) |
🤖 Prompt for AI Agents
In backend/controllers/profile_controller.go around lines 242 to 243, the field
name used in the UpdateOne calls is "eloRating" but it should be "rating" to
match the User model. Change the field name in both update queries from
"eloRating" to "rating" to ensure consistency and correct database updates.
| winnerID, _ := primitive.ObjectIDFromHex(req.WinnerID) | ||
| loserID, _ := primitive.ObjectIDFromHex(req.LoserID) | ||
|
|
||
| var winner, loser models.User | ||
| if err = db.MongoDatabase.Collection("users").FindOne(dbCtx, bson.M{"_id": winnerObjID}).Decode(&winner); err != nil { | ||
| if err == mongo.ErrNoDocuments { | ||
| ctx.JSON(http.StatusNotFound, gin.H{"error": "Winner not found"}) | ||
| } else { | ||
| ctx.JSON(http.StatusInternalServerError, gin.H{"error": "Error fetching winner from DB"}) | ||
| } | ||
| return | ||
| } | ||
| _ = db.MongoDatabase.Collection("users").FindOne(dbCtx, bson.M{"_id": winnerID}).Decode(&winner) | ||
| _ = db.MongoDatabase.Collection("users").FindOne(dbCtx, bson.M{"_id": loserID}).Decode(&loser) |
There was a problem hiding this comment.
Add error handling for user fetching.
The function doesn't check if ObjectID parsing succeeds or if users exist. This could lead to incorrect Elo calculations with zero-valued ratings.
Apply this diff to add proper error handling:
- winnerID, _ := primitive.ObjectIDFromHex(req.WinnerID)
- loserID, _ := primitive.ObjectIDFromHex(req.LoserID)
+ winnerID, err := primitive.ObjectIDFromHex(req.WinnerID)
+ if err != nil {
+ ctx.JSON(http.StatusBadRequest, gin.H{"error": "Invalid winner ID"})
+ return
+ }
+ loserID, err := primitive.ObjectIDFromHex(req.LoserID)
+ if err != nil {
+ ctx.JSON(http.StatusBadRequest, gin.H{"error": "Invalid loser ID"})
+ return
+ }
var winner, loser models.User
- _ = db.MongoDatabase.Collection("users").FindOne(dbCtx, bson.M{"_id": winnerID}).Decode(&winner)
- _ = db.MongoDatabase.Collection("users").FindOne(dbCtx, bson.M{"_id": loserID}).Decode(&loser)
+ if err := db.MongoDatabase.Collection("users").FindOne(dbCtx, bson.M{"_id": winnerID}).Decode(&winner); err != nil {
+ ctx.JSON(http.StatusNotFound, gin.H{"error": "Winner not found"})
+ return
+ }
+ if err := db.MongoDatabase.Collection("users").FindOne(dbCtx, bson.M{"_id": loserID}).Decode(&loser); err != nil {
+ ctx.JSON(http.StatusNotFound, gin.H{"error": "Loser not found"})
+ return
+ }🤖 Prompt for AI Agents
In backend/controllers/profile_controller.go around lines 229 to 234, the code
does not check for errors when parsing ObjectIDs or fetching users from the
database, which can cause issues if invalid IDs are provided or users are not
found. Update the code to handle errors returned by ObjectIDFromHex and
FindOne(). If parsing fails or a user is not found, return an appropriate error
response or handle the error to prevent proceeding with zero-valued user data.
2 participants
Summary by CodeRabbit
New Features
Improvements
Removals