-
Notifications
You must be signed in to change notification settings - Fork 113
Fix/frontend rendering on browser for development. #132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughExpands repository ignore rules, removes the production config file, promotes two backend dependencies from indirect to direct, and refactors multiple frontend components to strengthen WebSocket parsing, match log validation, team-building defaults, and the TeamDebateRoom WebSocket/types and state synchronization. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
frontend/src/Pages/MatchLogs.tsx (1)
142-232: Fix potential runtime crash in Total column highlight logic (log.score.totalunchecked).Left total highlight still calls
log.score.total.split(...)without guardingtotal(it only checkslog.score). Ifscoreexists buttotalis missing, this will throw.- <span - className={`font-semibold ${ - log.score && - parseInt(log.score.total.split("-")[0]) > - parseInt(log.score.total.split("-")[1]) - ? "text-primary" - : "" - }`} - > + <span + className={`font-semibold ${ + Number(totalParts[0]) > Number(totalParts[1]) ? "text-primary" : "" + }`} + > {totalParts[0]} </span>frontend/src/Pages/TeamDebateRoom.tsx (4)
525-599: Bug risk:fetchDebate()can “lock in”hasDeterminedTeamwithout auserId.With a token present but
currentUser?.idnot loaded yet, this code proceeds, can’t determine team membership, logs an error, then setshasDeterminedTeam=true/isLoading=falseanyway—potentially leaving the UI in a bad state for the rest of the session.- if (!debateId || (!token && !currentUser?.id)) { + const userId = currentUser?.id || user?.id; + if (!debateId || !token || !userId) { console.log("[TeamDebateRoom] Waiting for user/token...", { debateId, - userId: currentUser?.id, + userId, hasToken: !!token, isUserLoading, userFromAtom: user?.id, userFromHook: userFromHook?.id }); return; } ... - const userId = currentUser?.id || user?.id; const userTeam1 = debateData.team1Members?.some( (member: TeamMember) => member.userId === userId );
703-724: WebSocket handler needs defensive JSON parsing (and type validation).
JSON.parse(event.data)without a try/catch can break message handling entirely if the server ever sends a non-JSON frame (or malformed payload).- ws.onmessage = async (event) => { - const data: WSMessage = JSON.parse(event.data); + ws.onmessage = async (event) => { + let data: WSMessage; + try { + data = JSON.parse(event.data); + } catch (e) { + console.warn("Ignoring non-JSON WS message", event.data, e); + return; + } + if (!data || typeof data.type !== "string") { + console.warn("Ignoring WS message without a valid type", data); + return; + } console.log("Received WebSocket message:", data);Also applies to: 1164-1168
703-707: Avoid putting auth tokens in the WebSocket URL query string.Query-string tokens are easy to leak via logs/proxies/devtools screenshots and can persist in tooling. Prefer a cookie-based session, or pass a short-lived WS ticket, or use
Sec-WebSocket-Protocolif your backend supports it.
1426-1437: Remove hardcodedhttp://localhost:1313insubmit-transcripts; useBASE_URL/ env config.This undermines the
.envinstructions and breaks non-1313 dev/prod environments.- const response = await fetch(`http://localhost:1313/submit-transcripts`, { + const response = await fetch(new URL("/submit-transcripts", BASE_URL).toString(), { method: "POST", headers: { "Content-Type": "application/json", Authorization: `Bearer ${token}`, },
🧹 Nitpick comments (9)
.gitignore (2)
1-1: Remove duplicate*.localpattern.The
*.localpattern is defined at both line 1 and line 15. Keep one and remove the duplicate.Apply this diff:
*.local # Logs logs *.log npm-debug.log* yarn-debug.log* yarn-error.log* pnpm-debug.log* lerna-debug.log* node_modules dist dist-ssr -*.local # Editor directories and filesAlso applies to: 15-15
28-28: Consider adding.env.exampleto guide frontend setup.Per the PR objectives, frontend setup requires creating a
.envfile withVITE_BASE_URLandVITE_GOOGLE_CLIENT_ID. While ignoring.envis correct for security (preventing credential leaks), the codebase should include a.env.example(or similar template) to guide developers on required variables.Ensure a
.env.examplefile exists in thefrontend/directory with placeholders for required variables. If not present, this should be added in a separate commit or as part of this PR.Example
.env.examplefor frontend:VITE_BASE_URL=http://localhost:1313 VITE_GOOGLE_CLIENT_ID=<YOUR_GOOGLE_OAUTH_CLIENT_ID>backend/go.mod (1)
23-79: Confirm the necessity of the indirect dependencycasbin/govaluate.Line 31 shows
github.com/casbin/govaluate v1.3.0 // indirect. This is a transitive dependency of Casbin. Since Casbin is now a direct dependency (lines 8), verify thatgovaluateshould remain indirect or if it should be audited for removal if unused directly in the codebase.frontend/src/Pages/MatchLogs.tsx (3)
104-107: Hardenmatchparsing beyond simplesplit()assumptions.Current parsing will behave oddly if the match title contains extra
": "segments or if" vs "is missing/multiple. Consider stripping the prefix via regex and splitting on the last" vs "occurrence.- const [leftRaw, rightRaw] = log.match.split(" vs "); - const player1 = leftRaw.split(": ")[1] || leftRaw; - const player2 = rightRaw || ""; + const vsIdx = log.match.lastIndexOf(" vs "); + const leftRaw = vsIdx >= 0 ? log.match.slice(0, vsIdx) : log.match; + const rightRaw = vsIdx >= 0 ? log.match.slice(vsIdx + 4) : ""; + const player1 = leftRaw.replace(/^.*?:\s*/, ""); + const player2 = rightRaw;
113-135: Winner/tiebreaker computation is safer now; keep the “Match 3 tiebreak” rule data-driven.The new numeric validation is good, but hardcoding a special-case winner based on string matching will be brittle as real logs come from backend. Prefer a backend-provided flag/reason for tiebreak winners when available.
181-209: Per-category score parsing can still renderundefinedcells when input is malformed.
const [score1, score2] = value.split("-")assumes the delimiter exists; if not, you’ll renderscore2asundefinedand styling compares onNaN. Consider the same “2 parts + numeric” guard you added for totals.frontend/src/Pages/TeamDebateRoom.tsx (3)
73-115: Stop relying on(data as any)for fields that are now part of the “messaging contract”.You’re still reading
team1Name,team2Name,team1ReadyStatus,team2ReadyStatus, etc. viaany. Add them toWSMessage(typed) so state sync can’t silently regress.Also applies to: 768-822
898-1008: Strongly consider gating the very verbose console logging behind a dev flag.There’s a lot of detailed per-message logging (user IDs, team IDs, counts). It’s useful now, but it will be noisy in real usage and can become a privacy/ops concern.
Also applies to: 1500-1512
1208-1236: Typing cleanup:SpeechRecognitionResult.isFinalshouldn’t need(result as any).Prefer the standard property (and/or a local type guard) to avoid masking real runtime shape issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
backend/go.sumis excluded by!**/*.sumfrontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
.gitignore(1 hunks)backend/config/config.prod.yml(1 hunks)backend/go.mod(1 hunks)frontend/src/Pages/Game.tsx(0 hunks)frontend/src/Pages/MatchLogs.tsx(4 hunks)frontend/src/Pages/TeamBuilder.tsx(0 hunks)frontend/src/Pages/TeamDebateRoom.tsx(35 hunks)
💤 Files with no reviewable changes (2)
- frontend/src/Pages/Game.tsx
- frontend/src/Pages/TeamBuilder.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/Pages/TeamDebateRoom.tsx (1)
frontend/src/utils/auth.ts (1)
getAuthToken(5-7)
🪛 Gitleaks (8.30.0)
backend/config/config.prod.yml
[high] 6-6: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
[high] 8-8: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (1)
frontend/src/Pages/TeamDebateRoom.tsx (1)
12-23: Verify backend contract matches the expandedDebatePhaseenum values.The frontend enum defines 11 phase strings (
"setup","openingFor","openingAgainst","crossForQuestion","crossAgainstAnswer","crossAgainstQuestion","crossForAnswer","closingFor","closingAgainst","finished"), but these values are not validated at runtime when received from WebSocket messages. A mismatch between backend phase names and these string values will cause silent failures in phase gating and turn logic, as TypeScript cannot enforce the contract at runtime for incoming data.
.gitignore
Outdated
|
|
||
|
|
||
|
|
||
| kjn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove stray .gitignore entry.
Line 32 contains kjn, which appears to be an accidental entry that should not be in the ignore file.
Apply this diff:
-.env
-
-
-
-kjn
+.envCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .gitignore around line 32, remove the stray entry "kjn" which appears
accidental; edit the file to delete that line so only intentional ignore
patterns remain, save the file, and run git status to verify no unintended
ignores remain.
| github.com/casbin/casbin/v2 v2.132.0 | ||
| github.com/casbin/mongodb-adapter/v3 v3.7.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
casbin/casbin v2 latest version 2025 security advisories
💡 Result:
Summary (as of Dec 12, 2025):
- Latest casbin/casbin v2 release: v2.134.0 (released Nov 14, 2025). [1]
- Security advisories in 2025 for casbin/casbin: I found no GitHub Advisory / NVD CVE entries or project security advisories specifically for casbin/casbin in 2025. Snyk and the GitHub Advisory Database show no recorded vulnerabilities for the casbin packages for 2025. [2][3]
Sources
- Casbin v2.134.0 (release mirror listing, Nov 14, 2025). [SourceForge]. [4]
- Snyk package page for casbin — no known security issues (last updated 2025). [5]
- GitHub Advisory Database / search (no casbin/casbin advisories found for 2025). [6]
If you want, I can (a) check the casbin/casbin GitHub releases/tags page directly and link specific tags, or (b) run a deeper scan across NVD, GitHub Advisories, and vendor advisories for any related ecosystem packages (node-casbin, jcasbin, casbin-adapters). Which would you prefer?
🌐 Web query:
casbin mongodb-adapter v3 latest version 2025 security advisories
💡 Result:
Short answer: There is no published "casbin-mongodb-adapter v3" on npm as of Dec 12, 2025 — the casbin-mongodb-adapter package latest release is 1.3.0 and Snyk lists no direct vulnerabilities for it. Relevant MongoDB server/driver CVEs (which affect the underlying database, not the adapter code itself) include CVE-2024-1351 and CVE-2025-6713 — you should track/apply MongoDB/driver fixes if you use MongoDB. [1][2][3][4]
Sources
- casbin-mongodb-adapter (package info, latest 1.3.0; no direct vulnerabilities reported). [1]
- casbin adapters list (official docs). [2]
- CVE-2024-1351 (MongoDB TLS/peer-certificate validation advisory). [3]
- CVE-2025-6713 (MongoDB CVE record). [4]
🏁 Script executed:
# Check pkg.go.dev for current versions and check go.dev for security info
curl -s "https://pkg.go.dev/github.com/casbin/casbin/v2" 2>/dev/null | grep -i "v2\." | head -20 || echo "Cannot fetch pkg.go.dev"Repository: AOSSIE-Org/DebateAI
Length of output: 3334
Update casbin dependencies to current versions and verify mongodb-adapter version.
The casbin/casbin/v2 dependency at v2.132.0 is outdated; the latest version is v2.135.0 (released Nov 2025). Additionally, casbin/mongodb-adapter/v3 v3.7.0 does not exist—the MongoDB adapter latest release is v1.3.0. No security vulnerabilities are reported for either package in 2025, but the versions should be corrected and updated to current releases.
🤖 Prompt for AI Agents
In backend/go.mod around lines 8-9, the casbin and MongoDB adapter module
versions are incorrect: change github.com/casbin/casbin/v2 to the current
v2.135.0 and replace the nonexistent github.com/casbin/mongodb-adapter/v3 entry
with the correct adapter module/version (github.com/casbin/mongodb-adapter or
github.com/casbin/mongodb-adapter/v1) pinned to v1.3.0; after editing go.mod run
`go get github.com/casbin/casbin/v2@v2.135.0
github.com/casbin/mongodb-adapter@v1.3.0` (or include /v1 if your imports use
it), then run `go mod tidy` and `go test` / `go build` to verify imports and
update go.sum.
🧩 Analysis chain
🏁 Script executed:
# Search for issue #131 references in the repository to understand the actual problem
find . -type f -name "*.md" -o -name "*.txt" -o -name "*.go" | head -20Repository: AOSSIE-Org/DebateAI
Length of output: 833
🏁 Script executed:
# Look for PR description or issue context in common locations
ls -la | grep -i readme
git log --oneline -20 2>/dev/null || echo "Git not available"Repository: AOSSIE-Org/DebateAI
Length of output: 171
🏁 Script executed:
# Search for casbin usage in the codebase to understand if promotion is necessary
rg "casbin" --type go -lRepository: AOSSIE-Org/DebateAI
Length of output: 90
🏁 Script executed:
# Check the full go.mod to understand dependency context
head -100 backend/go.modRepository: AOSSIE-Org/DebateAI
Length of output: 3433
🏁 Script executed:
# Search for any MongoDB or adapter usage
rg "mongodb" --type go -lRepository: AOSSIE-Org/DebateAI
Length of output: 1530
🏁 Script executed:
# Check the rbac.go file to see how casbin is being used
cat -n backend/middlewares/rbac.go | head -50Repository: AOSSIE-Org/DebateAI
Length of output: 1834
🏁 Script executed:
# Search more thoroughly for mongodb-adapter usage
rg "mongodb-adapter|mongodb" -i --type goRepository: AOSSIE-Org/DebateAI
Length of output: 9131
🏁 Script executed:
# Check if there's a CHANGELOG or PR description
find . -name "CHANGELOG*" -o -name "*.md" | grep -v node_modules | head -10
cat README.md | head -100Repository: AOSSIE-Org/DebateAI
Length of output: 2680
🏁 Script executed:
# Look for recent changes to go.mod to understand context
git log --oneline backend/go.mod 2>/dev/null | head -10 || echo "Git unavailable"Repository: AOSSIE-Org/DebateAI
Length of output: 111
🏁 Script executed:
# Search for any imports of the mongodb-adapter package in the codebase
rg "mongodb-adapter" -iRepository: AOSSIE-Org/DebateAI
Length of output: 426
Clarify why backend dependency promotion is included in this frontend-rendering PR.
These dependencies are actively used in backend/middlewares/rbac.go (Casbin for access control, MongoDB adapter for policy storage), but the PR title focuses on fixing frontend rendering in development. The changes promote casbin/casbin/v2 and mongodb-adapter/v3 from indirect to direct dependencies—a backend infrastructure change.
Explain:
- Whether this backend change is a prerequisite for the frontend rendering fix or included by mistake
- Why this promotion was necessary (e.g., does the development environment require direct RBAC initialization?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.gitignore (1)
32-32: Remove straykjnignore entry (looks accidental).
This was flagged in a prior review; keeping it risks unintentionally ignoring a real file namedkjn.-kjn
🧹 Nitpick comments (1)
.gitignore (1)
1-16: Optional: dedupe*.localand trim extra blank lines for readability.
Not functionally wrong, but reduces noise/merge churn.*.local # Logs logs *.log npm-debug.log* yarn-debug.log* yarn-error.log* pnpm-debug.log* lerna-debug.log* node_modules dist dist-ssr -*.local + # Editor directories and files .vscode/* !.vscode/extensions.json .idea .DS_Store *.suo *.ntvs* *.njsproj *.sln *.sw? .env - - -Also applies to: 28-31
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.gitignore(1 hunks)backend/config/config.prod.yml(0 hunks)
💤 Files with no reviewable changes (1)
- backend/config/config.prod.yml
|
Please descibe your fixes properly in PR desc |
Sir please check my PR description..is it ok ? |
|
@bhavik-mangla |
1 similar comment
|
@bhavik-mangla |
|
Looks good @Suraj0950 |
|
Resolve the merge conflicts |
@bhavik-mangla Sir, If you encounter any further issues, please inform me. Thankyou. |

What does this PR do?
Why is this change needed?
How was this implemented?
Testing done
npm run devFiles changed
frontend/**(configuration and rendering-related files)Fixed Build-Breaking Syntax, i removed all conflict markers across these 4 files =>
Game.tsxMatchLogs.tsxTeamBuilder.tsxTeamDebateRoom.tsxTeamDebateRoom.tsx: Removed 3 duplicate function definitions (multiple toggleCamera functions), consolidated redundant useEffect hooks, and fixed a syntax error caused by a duplicated case "ready" block.
Screenshots
Fixed Issue
#123 #131 #135 #136
Additional Notes