fix: make copyright year dynamic in footer#332
Open
Abhishek2005-ard wants to merge 22 commits intoAOSSIE-Org:mainfrom
Open
fix: make copyright year dynamic in footer#332Abhishek2005-ard wants to merge 22 commits intoAOSSIE-Org:mainfrom
Abhishek2005-ard wants to merge 22 commits intoAOSSIE-Org:mainfrom
Conversation
… initialized via useState)
…anted restart loop
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds EloChange to transcript model and SaveDebateTranscript signature and usage; JudgeDebate now supplies topic, bot name, result, history and eloChange when saving transcripts and no longer discards the save call's return; frontend adds JSON5 parsing, admin route guard, speech-recognition auto-restart/async start, theming (dark + high-contrast), and minor UI/route tweaks. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant FE as Frontend (DebateRoom)
participant Judge as JudgeDebate Controller
participant TranscriptSvc as TranscriptService.SaveDebateTranscript
participant DB as Database
participant Profile as ProfileController
User->>FE: submit debate for judgment
FE->>Judge: POST /judge (judgment payload)
Judge->>Judge: parse result using JSON5.parse
Judge->>TranscriptSvc: SaveDebateTranscript(userID, email, debateType, topic, BotName, result, messages, req.History, nil, eloChange)
TranscriptSvc->>DB: insert/update SavedDebateTranscript (includes EloChange)
DB-->>TranscriptSvc: ok
TranscriptSvc-->>Judge: save result
Judge-->>FE: return judgement + eloChange
FE-->>User: display result and Elo delta
rect rgba(100,150,200,0.5)
User->>Profile: open profile
Profile->>DB: query recent transcripts
DB-->>Profile: transcripts (with EloChange)
Profile-->>User: show recentDebates with EloChange
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/services/transcriptservice.go (1)
973-981:⚠️ Potential issue | 🟡 Minor
GetDebateStatsstill hardcodeseloChange: 0despite the field now being available on the transcript.Line 980 still uses
"eloChange": 0with a TODO comment, buttranscript.EloChangeis now available. This should be updated to use the actual value, consistent with whatprofile_controller.goline 257 does.🔧 Proposed fix
- "eloChange": 0, // TODO: Add actual Elo change tracking + "eloChange": transcript.EloChange,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/services/transcriptservice.go` around lines 973 - 981, GetDebateStats is still hardcoding "eloChange": 0 in the recentDebates map; replace that with the actual transcript.EloChange value so the returned stats reflect real Elo changes (mirror how profile_controller uses transcript.EloChange), ensuring the map key "eloChange" is assigned transcript.EloChange and any necessary type conversion to match the map value type is handled.backend/controllers/debatevsbot_controller.go (1)
588-597:⚠️ Potential issue | 🔴 CriticalCritical: Missing
eloChangeparameter — this will not compile.
SaveDebateTranscriptnow requires 9 parameters (the neweloChange float64was added), butConcedeDebateonly passes 8 arguments.🐛 Proposed fix
_ = services.SaveDebateTranscript( user.ID, email, "user_vs_bot", debate.Topic, debate.BotName, "loss", historyToSave, nil, + 0.0, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/controllers/debatevsbot_controller.go` around lines 588 - 597, ConcedeDebate is calling SaveDebateTranscript with only 8 args but the signature now requires a ninth float64 eloChange; update ConcedeDebate to compute or provide an eloChange value (e.g., compute via your ELO helper or set eloChange := 0.0 if neutral) and pass that variable as the ninth argument to services.SaveDebateTranscript so the call matches the new signature.
🧹 Nitpick comments (11)
frontend/src/utils/speechTest.ts (1)
104-117:checkMicrophonePermission()acquires and immediately releases a real media stream on everystart()call.This adds latency and may briefly flash the browser's microphone indicator. Consider using
navigator.permissions.query({ name: "microphone" })first and only falling back togetUserMediawhen the permission state is"prompt".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/utils/speechTest.ts` around lines 104 - 117, The current start() flow calls checkMicrophonePermission() which acquires/releases a real MediaStream each time; update checkMicrophonePermission() to first use navigator.permissions.query({ name: "microphone" }) and return true when state is "granted", return false when "denied", and only call navigator.mediaDevices.getUserMedia({ audio: true }) as a fallback when the state is "prompt" (or when Permissions API is unavailable), releasing the stream immediately afterward; keep the existing shouldRestart checks in start() (the this.shouldRestart guard) so that permission requests can be cancelled mid-flow and ensure any thrown getUserMedia errors are caught and result in returning false.frontend/package.json (1)
34-34: Unrelated dependency addition — consider splitting into a separate PR.Adding
json5is unrelated to the PR objective of making the copyright year dynamic (issue#331). Including unrelated changes makes the PR harder to review and bisect. Consider moving this (and the associatedDebateRoom.tsxparsing changes) to a dedicated PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/package.json` at line 34, This PR includes an unrelated dependency addition ("json5" in package.json) and associated parsing changes in DebateRoom.tsx; split these out into a separate PR by removing the "json5" entry from package.json and reverting or isolating any changes in DebateRoom.tsx that rely on json5 so this PR only contains the copyright-year dynamic change (issue `#331`), then create a follow-up PR that adds "json5" and the DebateRoom.tsx parsing modifications with a focused description.backend/services/transcriptservice.go (2)
668-668: 9 positional parameters is a code smell — consider a struct-based parameter.The function now takes 9 positional arguments, which increases the risk of argument ordering mistakes (especially with multiple
stringparams in sequence). A params struct would be more maintainable and self-documenting.♻️ Example struct-based approach
type SaveTranscriptParams struct { UserID primitive.ObjectID Email string DebateType string Topic string Opponent string Result string Messages []models.Message Transcripts map[string]string EloChange float64 } func SaveDebateTranscript(params SaveTranscriptParams) error { // ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/services/transcriptservice.go` at line 668, The SaveDebateTranscript function currently accepts nine positional params (UserID, email, debateType, topic, opponent, result, messages, transcripts, eloChange), which is error-prone; refactor it to accept a single params struct (e.g., SaveTranscriptParams with fields UserID, Email, DebateType, Topic, Opponent, Result, Messages, Transcripts, EloChange) and update all call sites to construct and pass that struct to SaveDebateTranscript(params) to preserve clarity and prevent ordering mistakes.
182-210: Inconsistent indentation in the transcript save blocks.The
SaveDebateTranscriptcall arguments and subsequent error handling use mixed indentation (some lines at 2 tabs, some at 3 tabs). This makes the code harder to read within the deeply nested block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/services/transcriptservice.go` around lines 182 - 210, The two SaveDebateTranscript call blocks (the one forUser and the one for againstUser) have inconsistent indentation which hurts readability; locate the calls to SaveDebateTranscript and their corresponding error checks in transcriptservice.go and reformat so the argument list, closing parenthesis, and the subsequent if err { log.Println(...) } lines use the same indentation level (match the surrounding block's indentation), making both blocks visually identical and aligned (references: SaveDebateTranscript, forUser.ID/Email, againstUser.ID/Email, forSubmission.Transcripts, againstSubmission.Transcripts).frontend/tailwind.config.js (1)
65-68: High-contrast variant addition looks correct, but is unrelated to this PR's scope.The
addVariant('high-contrast', '@media (prefers-contrast: more)')is a valid Tailwind v3 plugin pattern. Consider moving accessibility/theming changes to a separate PR to keep this one focused on the copyright year fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/tailwind.config.js` around lines 65 - 68, The commit adds a Tailwind plugin variant addVariant('high-contrast', '@media (prefers-contrast: more)') inside tailwind.config.js which is unrelated to this PR; remove or revert that change from this branch and instead create a separate PR that introduces the accessibility/theme change, or if you must keep it now, add a comment and justify in the PR description—locate the addVariant('high-contrast', '@media (prefers-contrast: more)') call in tailwind.config.js and extract it to its own commit/PR.frontend/src/components/Matchmaking.tsx (1)
27-28: Whitespace-only change — no functional impact.These blank lines don't affect behavior. Consider keeping the PR focused on the copyright year fix to minimize noise.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Matchmaking.tsx` around lines 27 - 28, Whitespace-only lines were added in the Matchmaking.tsx file and create noisy, non-functional changes; remove the extra blank lines so the only meaningful change in this PR is the copyright year update. Open the Matchmaking component (Matchmaking.tsx), delete the superfluous blank lines introduced by the edit, verify formatting/linting passes, and ensure no other functional code in the Matchmaking component (or its exported component/function) was altered.frontend/src/Pages/DebateRoom.tsx (1)
599-604: Minor: missing space beforecatchkeyword.🧹 Style fix
- }catch (e) { + } catch (e) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/Pages/DebateRoom.tsx` around lines 599 - 604, The try/catch block parsing the judgment JSON in DebateRoom.tsx currently has no space before the catch keyword; update the block around the JSON5.parse call (where variable judgment: JudgmentData is assigned) to use the correct spacing: add a space between the closing brace and the catch keyword so it reads " } catch (e) {", leaving the existing error handling (throw new Error(`Failed to parse judgment JSON: ${e}`)) intact.frontend/src/index.css (1)
89-94:.high-contrastCSS variables look correct; consider merging@layer baseblocks.The scoped hover variables work correctly with the
high-contrast:Tailwind variant defined intailwind.config.js. Optionally, this second@layer baseblock can be consolidated into the existing one at line 96 to avoid repetition.🧹 Merge duplicate `@layer` base blocks (optional)
-@layer base { - .high-contrast { - --hc-hover-bg: 48 100% 50%; - --hc-hover-text: 0 0% 0%; - } -} - `@layer` base { * { `@apply` border-border; } body { `@apply` bg-background text-foreground; } + .high-contrast { + --hc-hover-bg: 48 100% 50%; + --hc-hover-text: 0 0% 0%; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/index.css` around lines 89 - 94, There are two `@layer` base blocks; merge the .high-contrast block into the existing `@layer` base to avoid duplicate base layers—move the .high-contrast { --hc-hover-bg: 48 100% 50%; --hc-hover-text: 0 0% 0%; } declarations into the other `@layer` base block (preserving the selector name .high-contrast) so the Tailwind high-contrast: variant continues to work without duplicate `@layer` base declarations (check tailwind.config.js for the high-contrast variant if needed).frontend/src/components/JudgementPopup.tsx (1)
116-116: Trailing whitespace on thebotDescdestructuring line.Minor nit — the line has trailing spaces (
botDesc,).🧹 Fix trailing whitespace
- botDesc, + botDesc,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/JudgementPopup.tsx` at line 116, Remove the trailing whitespace from the destructuring line that contains botDesc in JudgementPopup (the line reading "botDesc,"); edit the destructuring block so the token is "botDesc," with no extra spaces after it to eliminate the trailing whitespace.frontend/src/App.tsx (2)
97-98: Duplicate routes both rendering<ViewDebate />.Two routes at lines 97–98 mount the same component (
ViewDebate) with different path shapes. Ifview-debate/:debateIDis just a legacy alias, redirect it to the canonical path rather than duplicating the route.♻️ Replace duplicate with a redirect
- <Route path="debate/:debateID/view" element={<ViewDebate />} /> - <Route path="view-debate/:debateID" element={<ViewDebate />} /> + <Route path="debate/:debateID/view" element={<ViewDebate />} /> + {/* Redirect legacy path to canonical path */} + <Route path="view-debate/:debateID" element={<Navigate to="/debate/:debateID/view" replace />} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/App.tsx` around lines 97 - 98, Remove the duplicate Route that mounts <ViewDebate /> at "view-debate/:debateID" and replace it with a redirect to the canonical "debate/:debateID/view" path; specifically, keep the existing <Route path="debate/:debateID/view" element={<ViewDebate />} /> and change the other <Route path="view-debate/:debateID" ...> to render a redirect (e.g., using <Navigate> or your router's redirect mechanism) that routes to the canonical path while preserving the :debateID param and using replace to avoid history duplication.
55-103: Inconsistent path prefix convention between public and protected routes.Public routes use absolute paths (
"/auth","/admin/login") while protected routes use relative paths ("startDebate","leaderboard"). In React Router v6, both resolve identically under a pathless layout route, but the inconsistency makes the intent less clear. Prefer one convention throughout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/App.tsx` around lines 55 - 103, The route paths in App.tsx are inconsistent: public routes use absolute paths (e.g., "/auth", "/admin/login") while routes nested under <ProtectedRoute> and <Layout> use relative paths (e.g., "startDebate", "leaderboard"); pick one convention and make them consistent across the file — for example, update all nested Route path props inside the <ProtectedRoute> / <Layout> block (including "startDebate", "leaderboard", "profile", "community", "about", "team-builder", "game/:userId", "bot-selection", "tournaments", "tournament/:id/bracket", coach child routes, and all debate/spectator/view routes such as "debate/:roomId", "debate-room/:roomId", etc.) to use absolute paths (prefix with "/") so they match the public routes and clarify intent, or conversely remove leading slashes from the public routes to match the nested style; ensure any corresponding links/navigation use the same path format.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/controllers/debatevsbot_controller.go`:
- Line 273: The code is using two different debateType string literals
("user_vs_ai" in JudgeDebate vs "user_vs_bot" elsewhere), causing downstream
checks in UpdatePendingTranscripts and determineBotDebateResult to miss AI
debates; fix by standardizing to a single constant (e.g.,
DEBATE_TYPE_USER_VS_BOT) and replace all occurrences of the literals: update
JudgeDebate to use the constant (instead of "user_vs_ai"), update ConcedeDebate
and the gamification metadata usage (where "user_vs_bot" is referenced) to use
the same constant, and search/replace all places that check debateType
(including UpdatePendingTranscripts and determineBotDebateResult) so they
compare against the constant rather than hardcoded strings.
- Around line 270-280: In JudgeDebate, replace the undefined variables in the
SaveDebateTranscript call: pass userID (instead of user.ID), email (instead of
user.Email), latestDebate.Topic (instead of topic), and req.History (instead of
messages); also pass resultStatus (the "win"/"loss"/"draw"/"pending" string) as
the result parameter instead of the full JSON result variable; update the
SaveDebateTranscript invocation in JudgeDebate accordingly so it uses the
in-scope symbols userID, email, latestDebate.Topic, req.History, and
resultStatus.
In `@backend/controllers/profile_controller.go`:
- Line 257: The transcript's EloChange is never persisted: after calling
UpdateRatings (in transcriptservice.go) capture the computed rating delta it
returns and write it into the transcript model (the field referenced as
transcript.EloChange in profile_controller.go), then persist the transcript
using the existing transcript save/update routine so future reads reflect the
real Elo change; also remove the "Add actual Elo change tracking" TODO in
profile_controller.go (or, if you choose not to persist, replace the TODO with a
clear comment stating Elo change is not yet persisted).
In `@backend/models/transcript.go`:
- Line 33: The saved Transcript.EloChange is always 0.0 because
SaveDebateTranscript is called with a hardcoded 0.0 in the user-vs-user path (in
transcriptservice.SaveDebateTranscript call sites) and the actual Elo delta is
only computed later, so you must persist that computed value: update the flow so
that after the Elo delta is computed (the Elo calculation in transcriptservice
where eloChange is derived) you assign transcript.EloChange = eloChange and
either call the repository Save/Update for that transcript or change the earlier
SaveDebateTranscript call to accept and store the computed eloChange (ensure all
SaveDebateTranscript call sites pass a real value, not 0.0); this will make
profile_controller.go’s access of transcript.EloChange return the real delta.
In `@frontend/src/App.tsx`:
- Around line 62-63: Admin routes are not protected at the router level; wrap or
replace the current routes for AdminSignup and AdminDashboard with a
router-level guard by creating an AdminProtectedRoute (mirroring ProtectedRoute)
or using the shared auth context to check admin auth before rendering. Implement
AdminProtectedRoute to read admin credentials (e.g., adminToken/adminData from
the auth context or localStorage) and redirect to "/admin/login" when
unauthenticated, then use it in App.tsx to render AdminDashboard (and optionally
prevent access to /admin/login when already authenticated). Update route entries
to use <AdminProtectedRoute> for protected admin pages and keep AdminSignup
accessible only when appropriate.
In `@frontend/src/Pages/DebateRoom.tsx`:
- Line 581: Remove the development-only console.log call that prints "Judging
already in progress, skipping duplicate call" (the console.log(...) statement)
before merging; either delete this line or guard it behind a runtime debug check
such as NODE_ENV or an existing isDebug flag so it only runs in non-production
builds. Locate the exact console.log call in DebateRoom.tsx and replace/remove
it accordingly.
In `@frontend/src/Pages/TeamDebateRoom.tsx`:
- Around line 1315-1319: The catch block in the try/catch around
recognitionRef.current.start() in TeamDebateRoom.tsx is mis-indented relative to
the try; adjust the whitespace so the catch aligns with the try (same
indentation level as the try line), keeping the inner console.error line
indented one level inside the catch, ensuring consistent formatting around the
recognitionRef.current.start() call and its error handling.
In `@frontend/src/utils/speechTest.ts`:
- Around line 93-129: The start() method body is mis-indented by 2 spaces
(aligned with class members) — adjust its internal lines to use the same 4-space
indentation as stop() so the entire block inside the start() function is nested
properly; update the lines that reference this.restartTimeout,
this.shouldRestart, this.checkMicrophonePermission(), this.recognition.start(),
and the try/catch return paths to be indented inside the start() method body.
---
Outside diff comments:
In `@backend/controllers/debatevsbot_controller.go`:
- Around line 588-597: ConcedeDebate is calling SaveDebateTranscript with only 8
args but the signature now requires a ninth float64 eloChange; update
ConcedeDebate to compute or provide an eloChange value (e.g., compute via your
ELO helper or set eloChange := 0.0 if neutral) and pass that variable as the
ninth argument to services.SaveDebateTranscript so the call matches the new
signature.
In `@backend/services/transcriptservice.go`:
- Around line 973-981: GetDebateStats is still hardcoding "eloChange": 0 in the
recentDebates map; replace that with the actual transcript.EloChange value so
the returned stats reflect real Elo changes (mirror how profile_controller uses
transcript.EloChange), ensuring the map key "eloChange" is assigned
transcript.EloChange and any necessary type conversion to match the map value
type is handled.
---
Nitpick comments:
In `@backend/services/transcriptservice.go`:
- Line 668: The SaveDebateTranscript function currently accepts nine positional
params (UserID, email, debateType, topic, opponent, result, messages,
transcripts, eloChange), which is error-prone; refactor it to accept a single
params struct (e.g., SaveTranscriptParams with fields UserID, Email, DebateType,
Topic, Opponent, Result, Messages, Transcripts, EloChange) and update all call
sites to construct and pass that struct to SaveDebateTranscript(params) to
preserve clarity and prevent ordering mistakes.
- Around line 182-210: The two SaveDebateTranscript call blocks (the one forUser
and the one for againstUser) have inconsistent indentation which hurts
readability; locate the calls to SaveDebateTranscript and their corresponding
error checks in transcriptservice.go and reformat so the argument list, closing
parenthesis, and the subsequent if err { log.Println(...) } lines use the same
indentation level (match the surrounding block's indentation), making both
blocks visually identical and aligned (references: SaveDebateTranscript,
forUser.ID/Email, againstUser.ID/Email, forSubmission.Transcripts,
againstSubmission.Transcripts).
In `@frontend/package.json`:
- Line 34: This PR includes an unrelated dependency addition ("json5" in
package.json) and associated parsing changes in DebateRoom.tsx; split these out
into a separate PR by removing the "json5" entry from package.json and reverting
or isolating any changes in DebateRoom.tsx that rely on json5 so this PR only
contains the copyright-year dynamic change (issue `#331`), then create a follow-up
PR that adds "json5" and the DebateRoom.tsx parsing modifications with a focused
description.
In `@frontend/src/App.tsx`:
- Around line 97-98: Remove the duplicate Route that mounts <ViewDebate /> at
"view-debate/:debateID" and replace it with a redirect to the canonical
"debate/:debateID/view" path; specifically, keep the existing <Route
path="debate/:debateID/view" element={<ViewDebate />} /> and change the other
<Route path="view-debate/:debateID" ...> to render a redirect (e.g., using
<Navigate> or your router's redirect mechanism) that routes to the canonical
path while preserving the :debateID param and using replace to avoid history
duplication.
- Around line 55-103: The route paths in App.tsx are inconsistent: public routes
use absolute paths (e.g., "/auth", "/admin/login") while routes nested under
<ProtectedRoute> and <Layout> use relative paths (e.g., "startDebate",
"leaderboard"); pick one convention and make them consistent across the file —
for example, update all nested Route path props inside the <ProtectedRoute> /
<Layout> block (including "startDebate", "leaderboard", "profile", "community",
"about", "team-builder", "game/:userId", "bot-selection", "tournaments",
"tournament/:id/bracket", coach child routes, and all debate/spectator/view
routes such as "debate/:roomId", "debate-room/:roomId", etc.) to use absolute
paths (prefix with "/") so they match the public routes and clarify intent, or
conversely remove leading slashes from the public routes to match the nested
style; ensure any corresponding links/navigation use the same path format.
In `@frontend/src/components/JudgementPopup.tsx`:
- Line 116: Remove the trailing whitespace from the destructuring line that
contains botDesc in JudgementPopup (the line reading "botDesc,"); edit the
destructuring block so the token is "botDesc," with no extra spaces after it to
eliminate the trailing whitespace.
In `@frontend/src/components/Matchmaking.tsx`:
- Around line 27-28: Whitespace-only lines were added in the Matchmaking.tsx
file and create noisy, non-functional changes; remove the extra blank lines so
the only meaningful change in this PR is the copyright year update. Open the
Matchmaking component (Matchmaking.tsx), delete the superfluous blank lines
introduced by the edit, verify formatting/linting passes, and ensure no other
functional code in the Matchmaking component (or its exported
component/function) was altered.
In `@frontend/src/index.css`:
- Around line 89-94: There are two `@layer` base blocks; merge the .high-contrast
block into the existing `@layer` base to avoid duplicate base layers—move the
.high-contrast { --hc-hover-bg: 48 100% 50%; --hc-hover-text: 0 0% 0%; }
declarations into the other `@layer` base block (preserving the selector name
.high-contrast) so the Tailwind high-contrast: variant continues to work without
duplicate `@layer` base declarations (check tailwind.config.js for the
high-contrast variant if needed).
In `@frontend/src/Pages/DebateRoom.tsx`:
- Around line 599-604: The try/catch block parsing the judgment JSON in
DebateRoom.tsx currently has no space before the catch keyword; update the block
around the JSON5.parse call (where variable judgment: JudgmentData is assigned)
to use the correct spacing: add a space between the closing brace and the catch
keyword so it reads " } catch (e) {", leaving the existing error handling (throw
new Error(`Failed to parse judgment JSON: ${e}`)) intact.
In `@frontend/src/utils/speechTest.ts`:
- Around line 104-117: The current start() flow calls
checkMicrophonePermission() which acquires/releases a real MediaStream each
time; update checkMicrophonePermission() to first use
navigator.permissions.query({ name: "microphone" }) and return true when state
is "granted", return false when "denied", and only call
navigator.mediaDevices.getUserMedia({ audio: true }) as a fallback when the
state is "prompt" (or when Permissions API is unavailable), releasing the stream
immediately afterward; keep the existing shouldRestart checks in start() (the
this.shouldRestart guard) so that permission requests can be cancelled mid-flow
and ensure any thrown getUserMedia errors are caught and result in returning
false.
In `@frontend/tailwind.config.js`:
- Around line 65-68: The commit adds a Tailwind plugin variant
addVariant('high-contrast', '@media (prefers-contrast: more)') inside
tailwind.config.js which is unrelated to this PR; remove or revert that change
from this branch and instead create a separate PR that introduces the
accessibility/theme change, or if you must keep it now, add a comment and
justify in the PR description—locate the addVariant('high-contrast', '@media
(prefers-contrast: more)') call in tailwind.config.js and extract it to its own
commit/PR.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/controllers/debatevsbot_controller.go (1)
588-597:⚠️ Potential issue | 🔴 Critical
ConcedeDebate'sSaveDebateTranscriptcall is missing the neweloChangeargument — this will not compile.This PR updated the
SaveDebateTranscriptsignature to includeeloChange float64as the ninth parameter and correctly added0.0inJudgeDebate(line 279), but the call site inConcedeDebatestill passes only eight arguments. The Go compiler will reject this with"not enough arguments in call to services.SaveDebateTranscript", preventing the binary from building entirely.🐛 Proposed fix
_ = services.SaveDebateTranscript( user.ID, email, "user_vs_bot", debate.Topic, debate.BotName, "loss", historyToSave, nil, + 0.0, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/controllers/debatevsbot_controller.go` around lines 588 - 597, The call to services.SaveDebateTranscript inside ConcedeDebate is missing the newly added ninth parameter eloChange; update the call in the ConcedeDebate handler to pass a float64 value (e.g., 0.0) as the ninth argument to match the new SaveDebateTranscript signature, so the invocation signature matches the function declared elsewhere (SaveDebateTranscript).frontend/src/Pages/DebateRoom.tsx (1)
810-815:⚠️ Potential issue | 🟡 MinorUser panel is missing dark-mode classes that the bot panel (line 767) already has.
The bot container has
dark:bg-zinc-900 dark:border-zinc-700, but the user container retains onlybg-white border-gray-200, leaving it unstyled in dark mode.🐛 Proposed fix
- } bg-white border border-gray-200 shadow-md h-[540px] flex flex-col`} + } bg-white dark:bg-zinc-900 border border-gray-200 dark:border-zinc-700 shadow-md h-[540px] flex flex-col`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/Pages/DebateRoom.tsx` around lines 810 - 815, The User Section div in DebateRoom.tsx (the container with className starting with "relative w-full md:w-1/2") is missing dark-mode classes; update that className to mirror the bot panel by adding dark:bg-zinc-900 and dark:border-zinc-700 (and any matching dark text class if used on the bot panel) so the user container displays correctly in dark mode while preserving the existing conditional animate-glow, bg-white, border-gray-200, shadow-md, and layout classes.
🧹 Nitpick comments (3)
frontend/src/Pages/DebateRoom.tsx (1)
438-439:judgeDebateResultis called insideadvanceTurn(line 479) but declared ~140 lines later (line 579).Functionally safe — all invocations are deferred (setState callbacks, setTimeout) and
judgeDebateResultis fully initialized before any of those fire. Hoisting the declaration aboveadvanceTurnwould make the dependency direction immediately obvious to readers.Also applies to: 579-580
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/Pages/DebateRoom.tsx` around lines 438 - 439, advanceTurn currently calls judgeDebateResult but judgeDebateResult is declared much later; move the judgeDebateResult function declaration so it appears above advanceTurn (or inline a small helper) to make the dependency explicit — update the order so judgeDebateResult is defined before advanceTurn (referencing the functions advanceTurn and judgeDebateResult) and ensure any closures/props used by judgeDebateResult still resolve after the reorder.frontend/src/components/ProtectedRoute.tsx (2)
1-18: FilenameProtectedRoute.tsxonly exportsAdminProtectedRoute— consider renaming.The file name creates a false expectation of a generic
ProtectedRouteexport and clashes conceptually with the inlineProtectedRoutefunction inApp.tsx. Renaming toAdminProtectedRoute.tsxwould eliminate the ambiguity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ProtectedRoute.tsx` around lines 1 - 18, The component exported as AdminProtectedRoute is in a file named ProtectedRoute.tsx which is misleading; rename the file to AdminProtectedRoute.tsx and keep the default export AdminProtectedRoute as-is, then update any imports (e.g., in App.tsx or other modules that currently import from "ProtectedRoute") to import from "AdminProtectedRoute" instead; also search for any local identifier or inline ProtectedRoute references and ensure there are no naming collisions after the rename.
8-12:AdminProtectedRouteonly validates token presence, not validity or expiry.A stale, expired
adminTokenstill passes the guard. While API calls do validate server-side (Bearer token), an attacker who retains a previously-logged-in browser profile can access the admin dashboard UI indefinitely.Consider storing a token expiry timestamp (e.g.
adminTokenExpiry) alongsideadminTokenat login time and checking it here:♻️ Proposed improvement
const adminToken = localStorage.getItem("adminToken"); const adminData = localStorage.getItem("adminData"); + const adminTokenExpiry = localStorage.getItem("adminTokenExpiry"); + const isExpired = adminTokenExpiry && Date.now() > Number(adminTokenExpiry); - if (!adminToken || !adminData) { + if (!adminToken || !adminData || isExpired) { + if (isExpired) localStorage.removeItem("adminToken"); return <Navigate to="/admin/login" replace />; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ProtectedRoute.tsx` around lines 8 - 12, The route guard currently only checks presence of adminToken/adminData; update the AdminProtectedRoute (the component reading localStorage.getItem("adminToken") and "adminData") to also read and validate an "adminTokenExpiry" value (e.g. a numeric timestamp or ISO string), ensure Date.now() < parsed expiry, and treat missing/expired expiry as invalid: clear the adminToken/adminData/adminTokenExpiry from localStorage and return <Navigate to="/admin/login" replace />; keep server-side validation but enforce client-side expiry check here to prevent stale tokens from granting UI access.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/controllers/debatevsbot_controller.go`:
- Line 275: Replace the hardcoded "AI" with the actual bot identity (use
latestDebate.BotName as previously) when constructing the transcript in
JudgeDebate so the saved opponent matches the real bot; ensure you handle an
empty/nil BotName by falling back to a sensible default (e.g., "AI") to avoid
panics and to keep SaveDebateTranscript's opponent filter consistent with
ConcedeDebate and other flows.
In `@frontend/src/Pages/DebateRoom.tsx`:
- Around line 586-608: Remove the debug console.log calls that print sensitive
data in the judgment flow: delete the console.log statements around "Starting
judgment with messages:", "Raw judge result:", "Extracted JSON string:", and
"Parsed judgment:" inside the try block that calls judgeDebate; keep the logic
that calls judgeDebate({ history: messages, userId: debateData.userId }), parses
with extractJSON and JSON5.parse, and updates state via setJudgmentData,
setPopup and setShowJudgment. If you want optional non-sensitive logging for
debugging, replace those console.log calls with a guarded debug logger (e.g.,
isDev check) that never prints full message history or AI outputs in production.
- Line 730: In the DebateRoom component JSX, fix the Tailwind class typo on the
spinner div by replacing the invalid utility "border-blue500" with the correct
"border-blue-500" so the spinner's top border uses the intended blue color;
locate the div element containing "animate-spin rounded-full h-16 w-16
border-t-4 border-blue500 mb-4" and update that class list accordingly.
---
Outside diff comments:
In `@backend/controllers/debatevsbot_controller.go`:
- Around line 588-597: The call to services.SaveDebateTranscript inside
ConcedeDebate is missing the newly added ninth parameter eloChange; update the
call in the ConcedeDebate handler to pass a float64 value (e.g., 0.0) as the
ninth argument to match the new SaveDebateTranscript signature, so the
invocation signature matches the function declared elsewhere
(SaveDebateTranscript).
In `@frontend/src/Pages/DebateRoom.tsx`:
- Around line 810-815: The User Section div in DebateRoom.tsx (the container
with className starting with "relative w-full md:w-1/2") is missing dark-mode
classes; update that className to mirror the bot panel by adding
dark:bg-zinc-900 and dark:border-zinc-700 (and any matching dark text class if
used on the bot panel) so the user container displays correctly in dark mode
while preserving the existing conditional animate-glow, bg-white,
border-gray-200, shadow-md, and layout classes.
---
Nitpick comments:
In `@frontend/src/components/ProtectedRoute.tsx`:
- Around line 1-18: The component exported as AdminProtectedRoute is in a file
named ProtectedRoute.tsx which is misleading; rename the file to
AdminProtectedRoute.tsx and keep the default export AdminProtectedRoute as-is,
then update any imports (e.g., in App.tsx or other modules that currently import
from "ProtectedRoute") to import from "AdminProtectedRoute" instead; also search
for any local identifier or inline ProtectedRoute references and ensure there
are no naming collisions after the rename.
- Around line 8-12: The route guard currently only checks presence of
adminToken/adminData; update the AdminProtectedRoute (the component reading
localStorage.getItem("adminToken") and "adminData") to also read and validate an
"adminTokenExpiry" value (e.g. a numeric timestamp or ISO string), ensure
Date.now() < parsed expiry, and treat missing/expired expiry as invalid: clear
the adminToken/adminData/adminTokenExpiry from localStorage and return <Navigate
to="/admin/login" replace />; keep server-side validation but enforce
client-side expiry check here to prevent stale tokens from granting UI access.
In `@frontend/src/Pages/DebateRoom.tsx`:
- Around line 438-439: advanceTurn currently calls judgeDebateResult but
judgeDebateResult is declared much later; move the judgeDebateResult function
declaration so it appears above advanceTurn (or inline a small helper) to make
the dependency explicit — update the order so judgeDebateResult is defined
before advanceTurn (referencing the functions advanceTurn and judgeDebateResult)
and ensure any closures/props used by judgeDebateResult still resolve after the
reorder.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/Pages/DebateRoom.tsx`:
- Line 727: The Tailwind class on the modal div uses an invalid utility
"max-w-m" causing unconstrained width; update the class list on the div (the
element that currently has className="bg-white ... max-w-m w-full ...") to use a
valid max-width utility such as "max-w-sm" or "max-w-md" (choose based on
desired modal width) so the popup is constrained on large screens and retains
modal appearance.
---
Duplicate comments:
In `@frontend/src/Pages/DebateRoom.tsx`:
- Around line 586-608: Remove the debug console.log calls that expose full
message history and raw AI output in production (the logs around judgeDebate,
extractJSON, and parsed judgment) and replace them with non-leaking behavior:
either remove them entirely or guard them behind a development-only check (e.g.,
NODE_ENV === "development" or a feature flag) before calling console.log; keep
the logic that parses via JSON5.parse and updates state (setJudgmentData,
setPopup, setShowJudgment) unchanged so only logging is affected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaced hard-coded copyright year with dynamic year using
new Date().getFullYear() in the About page footer.
This ensures the footer always displays the current year
without requiring manual updates.
close #331
Summary by CodeRabbit
New Features
Bug Fixes
Style