Fix backend build errors and restore server startup #130
Conversation
…i service, restore config structure, and correct websocket handlers
WalkthroughThis PR introduces multiple backend changes including Redis configuration renaming, Google OAuth configuration addition, model structure expansions (Comment struct with new fields), API handler stubs (Gemini and Debate WebSocket), dependency updates for Casbin integration, and WebSocket broadcast logic refinements following merge conflict resolution. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/cmd/server/main.go (1)
44-58: Redis URL fallback is currently unreachableThe inner fallback to
"localhost:6379"can never run because of the outerif cfg.Redis.URL != "":if cfg.Redis.URL != "" { redisURL := cfg.Redis.URL if redisURL == "" { // unreachable redisURL = "localhost:6379" } ... }If you want a real default to localhost when
cfg.Redis.URLis empty, consider:redisURL := cfg.Redis.URL if redisURL == "" { redisURL = "localhost:6379" } if err := debate.InitRedis(redisURL, cfg.Redis.Password, cfg.Redis.DB); err != nil { ... }Otherwise, you can safely drop the inner
if redisURL == ""as dead code.
🧹 Nitpick comments (6)
backend/config/config.go (1)
3-8: Config structure and loader are fine; consider modernizing file I/OThe expanded
Configshape and YAML unmarshalling look consistent and should work as intended. You might later replaceioutil.ReadFilewithos.ReadFilesinceio/ioutilis deprecated in newer Go versions, but that’s purely a cleanup.Also applies to: 10-57, 59-72
backend/controllers/transcript_controller.go (1)
407-415: Remove or replace the email stub inUpdatePendingTranscriptsHandlerThe
email := "" // or load actual emailfollowed by_ = emailis effectively a no-op and may confuse future maintainers. Either:
- Capture the email from
ValidateTokenAndFetchEmailand use it if/when the service needs it, or- Drop the local
backend/cmd/server/main.go (1)
164-175: StubDebateWebsocketHandleris fine as a temporary placeholderWiring
/ws/debate/:debateIDto a 501 JSON stub unblocks routing and build. Just keep in mind this endpoint is effectively disabled until real WebSocket logic is hooked up, so any callers should expect the 501 for now.If helpful, I can help adapt the existing debate WebSocket logic into a
websocket.DebateWebsocketHandlerand wire it here when you’re ready.backend/controllers/team_controller.go (1)
354-377: Deduplicate capacity calculation inJoinTeamNormalizing
capacitywith a fallback to 4 is a good safeguard, but you do it twice:capacity := team.MaxSize if capacity <= 0 { capacity = 4 } if len(team.Members) >= capacity { ... } ... capacity = team.MaxSize if capacity <= 0 { capacity = 4 } if len(team.Members) >= capacity { ... }You can compute
capacityonce and reuse it:capacity := team.MaxSize if capacity <= 0 { capacity = 4 } if len(team.Members) >= capacity { c.JSON(http.StatusBadRequest, gin.H{"error": "Team is already full"}) return } totalElo := 0.0 for _, member := range team.Members { totalElo += member.Elo } totalElo += newMember.Elo newAverageElo := totalElo / float64(len(team.Members)+1)This keeps behavior the same and removes redundant logic.
backend/websocket/team_websocket.go (1)
488-507:snapshotAllTeamClientshelper is a good concurrency-friendly additionCapturing all
TeamClientpointers underroom.Mutexand then broadcasting outside the lock viasnapshotAllTeamClientsis a nice improvement over writing while holding the mutex. This pattern should scale better under load.You might later migrate other broadcast paths that still iterate
room.Clientsunderroom.Mutex(e.g., inhandleTeamReadyStatus/handleCheckStart) to reuse this helper for consistency and to keep I/O outside the critical section.backend/models/comment.go (1)
12-29: Admin metadata fields look correct; consider clarifying dual email fields and deletion flag semanticsThe added admin fields (
Type,UserEmail,DebateID,TeamID,IsDeleted) have consistent BSON/JSON tags and should work fine withadmin_controller.go. However, having bothUserEmailplus anIsDeletedbool withomitemptycan be ambiguous for future readers/clients. Consider:
- Adding brief doc comments (or renaming) to distinguish
UserEmail.- Confirming that omitting
isDeletedentirely (for false) is acceptable to all API consumers, vs always returning an explicitfalse.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
backend/cmd/server/main.go(1 hunks)backend/config/config.go(1 hunks)backend/controllers/auth.go(1 hunks)backend/controllers/team_controller.go(2 hunks)backend/controllers/transcript_controller.go(1 hunks)backend/db/db.go(1 hunks)backend/go.mod(4 hunks)backend/models/admin.go(1 hunks)backend/models/comment.go(2 hunks)backend/services/gemini.go(1 hunks)backend/websocket/team_websocket.go(2 hunks)backend/websocket/websocket.go(0 hunks)
💤 Files with no reviewable changes (1)
- backend/websocket/websocket.go
🧰 Additional context used
🧬 Code graph analysis (1)
backend/cmd/server/main.go (1)
backend/cmd/server/ws_debate_handler.go (1)
DebateWebsocketHandler(196-264)
🔇 Additional comments (9)
backend/db/db.go (1)
7-15: Redis client wiring and logging look correctThe switch to
github.com/redis/go-redis/v9plus thePingcheck and success log inConnectRedisare idiomatic and should be reliable as-is.Also applies to: 99-118
backend/models/admin.go (1)
23-36:AdminActionLog.Detailsaddition is a good, flexible extensionAdding the
Detailsfield withomitemptytags gives you room for richer audit data without breaking existing documents or responses. Looks good.backend/controllers/team_controller.go (1)
625-635: Available-teams$exprquery usingbson.Alooks correctThe
$expr/$ltfilter withbson.A{ bson.M{"$size": "$members"}, "$maxSize" }is valid and matches the intended “members count < maxSize” semantics.backend/websocket/team_websocket.go (2)
510-529: Team join broadcast now correctly syncs all clients with team statusUsing
snapshotAllTeamClientsinhandleTeamJointo send the"teamStatus"payload to everyone (including the joiner) is reasonable and avoids per-call map locking. No correctness issues spotted.
949-970: Turn-status broadcast changes inhandleTeamTurnRequestlook soundComputing
currentTurnonce and then broadcasting"teamStatus"only to clients wherer.TeamID == client.TeamIDviasnapshotAllTeamClientskeeps the update scoped to the relevant team while reducing redundant locking. Behavior matches the per-team semantics of turns.backend/services/gemini.go (1)
10-35: Verify error handling at all call sites of Gemini stub functions
initGeminireturns(nil, nil)andgenerateModelTextguards ongeminiClient == nil, so any invocation ofgenerateDefaultModelTextorgenerateModelTextwill return an error unless a real client is initialized elsewhere. This changes behavior from silent no-op to explicit error.Confirm that all call sites either:
- Handle the "gemini client not initialized" error gracefully, or
- Skip calling these helpers when Gemini is not available.
backend/models/comment.go (2)
40-51: MarshalJSON remains compatible; new fields are automatically includedThe Alias-based
MarshalJSONstill correctly convertsID,TranscriptID,ParentID, andUserIDto hex strings while delegating the rest of the fields (including the new admin metadata) to the embeddedAlias. This keeps existing JSON shape for IDs and automatically serializes the new fields without extra code. No changes needed here.
56-70: DebatePost struct/tag setup is consistent and sane
DebatePost’s ObjectID fields, user info, and audit timestamps all have aligned BSON/JSON tags and mirror the patterns used onComment. This should keep the public post API stable and predictable. Looks good as-is.backend/go.mod (1)
8-9: Validate new Casbin/genai dependencies and clean up any legacy Gemini client usagePromoting
github.com/casbin/casbin/v2andgithub.com/casbin/mongodb-adapter/v3to direct deps and bumpinggoogle.golang.org/genaitov1.37.0matches the new auth and Gemini stub work. Two follow-ups to keep the module graph tidy:
- Verify all call sites are written against
google.golang.org/genai v1.37.0's API surface (breaking changes exist across minor versions).- You have both
google.golang.org/genaiandgithub.com/google/generative-ai-goin the graph; if fully migrated togoogle.golang.org/genai, remove remaininggenerative-ai-goimports and rungo mod tidylocally so the old client drops out.
| "os" | ||
| "strings" | ||
| "time" | ||
| "log" |
There was a problem hiding this comment.
PII logging creates compliance risk.
The log import enables logging statements in generateJWT (line 546) that log user email addresses. Logging PII such as emails may violate GDPR/CCPA requirements, especially if logs are retained, aggregated, or transmitted to third-party systems without proper safeguards.
Consider one of the following approaches:
- Remove email from log statements and use anonymized user IDs instead
- Implement structured logging with PII redaction/masking
- Ensure logs containing PII have appropriate retention policies and access controls
Apply this diff to remove email from the log statement:
- log.Printf("JWT Generation - Email: %s, Now: %s, Expiry: %s (in %d minutes)", email, now.Format(time.RFC3339), expirationTime.Format(time.RFC3339), expiryMinutes)
+ log.Printf("JWT Generation - Now: %s, Expiry: %s (in %d minutes)", now.Format(time.RFC3339), expirationTime.Format(time.RFC3339), expiryMinutes)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/controllers/auth.go around lines 11 and 546, the file currently
imports the standard "log" (line 11) and later logs user email in generateJWT
(around line 546), which exposes PII; remove or change the log to avoid logging
the raw email — replace any email output with a non-PII identifier (e.g., user
ID or hashed/masked email) or remove the log entirely, and if the "log" import
becomes unused remove that import line; ensure any structured logging uses the
non-PII field and that the code still compiles after these changes.
|
Include testing evidences |
2 participants
Title: Fix backend build errors and restore server startup
Summary
This PR resolves multiple critical issues that prevented the backend from compiling and starting.
The project had several unresolved merge conflicts, missing structs, mismatched types, and incomplete API integrations that caused the build to fail.
Key Fixes
Cleaned and merged conflict blocks in:
team_websocket.go
team_controller.go
websocket.go
others as necessary
Ensured consistent logic (e.g., team capacity, bson filters).
Removed duplicate Comment struct definitions.
Unified Comment into a single model with fields required by controllers:
Type
UserEmail
DebateID
TeamID
IsDeleted
plus existing transcript/comment fields.
This restores compatibility with admin_controller.go, transcript_controller.go, and others.
Removed duplicate nested Redis blocks in config.go.
Restored valid Config struct shape.
Removed duplicate fields (IsSpectator).
Corrected missing/broken handler definitions.
Added stub for DebateWebsocketHandler.
Restored missing "log" imports where needed.
The project was using an outdated Gemini API that no longer exists.
To restore build:
Removed broken API calls.
Implemented stub functions:
initGemini
generateModelText
generateDefaultModelText
Ensured the correct type (*genai.Client) matches the rest of the codebase.
Restored consistent import paths.
This allows the backend to compile even without full Gemini support.
All compile-time issues are resolved; the project now proceeds to runtime, failing only on invalid MongoDB config (expected).
Why This PR Is Important
The backend was previously not buildable at all.
Merge conflicts and inconsistent models made it impossible for contributors to run or test.
This PR restores the project to a functional, maintainable state.
Enables future work such as improving Debate AI, fixing endpoints, or implementing real Gemini logic.
What Still Needs Work (Optional Future Tasks)
Implement real Gemini model integration.
Provide a valid MongoDB config or add a fallback local DB.
Improve websocket error handling and room management.
Add unit tests for Comment flows and team logic.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.