feat: add impromptu daily challenge#380
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a complete Daily Impromptu Speaking Challenge feature. It adds backend routes, controllers, and services to handle topic delivery and speech evaluation via Gemini AI, alongside a frontend page with speech recognition support, state machine flow management (IDLE → PREP → RECORDING → EVALUATING → RESULT), and sidebar navigation. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Frontend as Frontend<br/>(ImpromptuChallenge)
participant Backend as Backend<br/>(Controllers)
participant Gemini as Gemini AI
participant Database as Database
User->>Frontend: Initiates impromptu challenge
Frontend->>Backend: GET /impromptu/topic<br/>(with Bearer token)
Backend->>Frontend: Returns random topic
Frontend->>User: Displays topic + 30sec prep timer
rect rgba(100, 150, 255, 0.5)
Note over User,Frontend: Prep Phase → Recording Phase
User->>Frontend: Begins speaking (60sec timer)
Frontend->>Frontend: Captures speech via Web Speech API<br/>into transcript state
User->>Frontend: Clicks "Finish & Evaluate"
end
Frontend->>Backend: POST /impromptu/evaluate<br/>{ topic, transcript }
Backend->>Gemini: Score user speech<br/>(0–10 per section)
Gemini->>Backend: Returns JSON judgment<br/>(scores, reasons, verdict)
Backend->>Database: SaveDebateTranscript<br/>(async: updateGamification)
Backend->>Frontend: { "message": "...", "data": eval }
Frontend->>Frontend: Transitions to RESULT phase
Frontend->>User: Displays JudgmentPopup<br/>(avatars, scores, feedback)
User->>Frontend: Closes result
Frontend->>Frontend: Resets to IDLE
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
backend/services/impromptu.go (1)
12-16: Unused structImpromptuEvaluation.This struct is defined but never used in the file. The actual return type for evaluation is
ImpromptuJudgmentData. Consider removing it to reduce dead code.🗑️ Suggested removal
-type ImpromptuEvaluation struct { - Score int `json:"score"` - Result string `json:"result"` - Feedback string `json:"feedback"` -} - // Static list of impromptu topics🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/services/impromptu.go` around lines 12 - 16, The ImpromptuEvaluation type is dead code — remove the struct declaration named ImpromptuEvaluation from the file (since the code uses ImpromptuJudgmentData as the actual evaluation/return type) or, if you intended to use it, replace any references to ImpromptuJudgmentData with ImpromptuEvaluation; after the change, run a build to ensure no remaining references to ImpromptuEvaluation exist and update imports/types accordingly.backend/controllers/impromptu_controller.go (1)
92-101: IgnoringSaveDebateTranscripterror silently.If saving the transcript fails, the error is discarded with
_ =. Consider logging the error so issues with database persistence are observable.📝 Suggested fix
- _ = services.SaveDebateTranscript( + if err := services.SaveDebateTranscript( userID, email, "impromptu", req.Topic, "AI Evaluator", resultStr, history, nil, - ) + ); err != nil { + log.Printf("Failed to save impromptu transcript for user %s: %v", email, err) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/controllers/impromptu_controller.go` around lines 92 - 101, Replace the ignored return from SaveDebateTranscript with proper error handling: capture the error returned by services.SaveDebateTranscript (called with userID, email, "impromptu", req.Topic, "AI Evaluator", resultStr, history, nil) and log it instead of discarding it; e.g., assign the call to err and call the module's logger/error reporting (or fmt/log) to record a descriptive message including the error and relevant identifiers (userID or req.Topic) so persistence failures are observable.frontend/src/components/Sidebar.tsx (1)
47-51: Consider using a distinct icon for "Daily Challenge".The
MessageSquareicon is already used for "Start Debate" (line 35). Using a different icon likeZap,Clock, orSparklesfrom lucide-react could help users visually distinguish the Daily Challenge feature.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Sidebar.tsx` around lines 47 - 51, The Sidebar uses the same MessageSquare icon for both "Start Debate" and the Daily Challenge NavItem (see NavItem with to='/impromptu' and label='Daily Challenge'), which reduces visual distinction; replace the MessageSquare instance for the Daily Challenge NavItem with a different lucide-react icon such as Zap, Clock, or Sparkles (import the chosen icon at the top and update the icon prop on the NavItem) so the Daily Challenge appears visually distinct from Start Debate.frontend/src/Pages/ImpromptuChallenge.tsx (1)
84-107: Stale closure risk:handleStopRecordingis not in the dependency array.The
setTimeoutcallback on line 97 captures a stale reference tohandleStopRecording. If the component re-renders between phase changes, the callback may invoke an outdated function closure. React's exhaustive-deps rule would flag this.♻️ Suggested approach
Wrap
handleStopRecordinginuseCallbackand add it to the dependency array, or use a ref to always access the latest version:+ const handleStopRecordingRef = useRef(handleStopRecording); + useEffect(() => { + handleStopRecordingRef.current = handleStopRecording; + }); useEffect(() => { let timeout: NodeJS.Timeout; // ... } else if (phase === 'RECORDING') { setTimerValue(60); startRecognition(); timeout = setTimeout(() => { - handleStopRecording(); + handleStopRecordingRef.current(); }, 60000); } // ... - }, [phase]); + }, [phase]);Alternatively, use
useCallbackwith proper dependencies forhandleStopRecording.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/Pages/ImpromptuChallenge.tsx` around lines 84 - 107, The timeout callback in the useEffect captures a stale handleStopRecording reference — wrap handleStopRecording in useCallback (with its real dependencies) or store the latest function in a ref and read ref.current inside the timeout, then add that handleStopRecording (or the ref) to the useEffect dependency array; ensure the cleanup still clears the timeout and calls stopRecognition when phase !== 'RECORDING', and keep startRecognition, stopRecognition, phase and any other functions used in the effect in the dependency list as appropriate.
🤖 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/impromptu_controller.go`:
- Around line 80-84: The win threshold is set incorrectly (>= 70) making a win
impossible given the max score is 40; update the logic in the block that sets
resultStr (uses eval.Total.User and resultStr) to use the correct
threshold—either change the literal 70 to 20 to match the prompt's ">= 20"
verdict, or compute a 50% threshold from the maximum possible (e.g., maxScore/2)
and compare eval.Total.User against that so the win condition can actually be
reached.
In `@backend/services/impromptu.go`:
- Around line 27-31: The GetRandomImpromptuTopic function should stop calling
rand.Seed on each invocation; remove the rand.Seed(time.Now().UnixNano()) line
so the function simply returns impromptuTopics[rand.Intn(len(impromptuTopics))]
and rely on Go's automatic seeding (or, if you need a separate source for
concurrency/control, create and use a dedicated rand.New(rand.NewSource(...))
instance referenced where needed). Update the GetRandomImpromptuTopic
implementation to delete the Seed call and keep only the random selection logic.
In `@frontend/src/Pages/ImpromptuChallenge.tsx`:
- Around line 132-139: The evaluate fetch in ImpromptuChallenge.tsx currently
parses the response without checking HTTP status; update the try block where you
call fetch(...) for `${import.meta.env.VITE_BASE_URL}/impromptu/evaluate` to
check res.ok before calling await res.json(): if !res.ok, read and surface the
error (e.g., await res.text() or res.json()) and throw or handle it (so
downstream code using resJson isn't run on error). Ensure the Authorization
token retrieval (localStorage.getItem('token')), the fetch call, and the
variables res and resJson are the ones updated.
- Around line 109-123: In fetchTopic, you need to check the HTTP response status
before calling res.json(); currently fetchTopic always parses the body and may
setTopic(undefined) on non-2xx responses. Update fetchTopic to verify res.ok (or
specific status codes) after awaiting fetch, and if not ok call toast with a
descriptive error (optionally include status or error text from the response)
and return early; only call await res.json() and setTopic(data.topic) when the
response is successful. Ensure transcriptRef, setTranscript and setPhase('PREP')
only run on successful responses.
---
Nitpick comments:
In `@backend/controllers/impromptu_controller.go`:
- Around line 92-101: Replace the ignored return from SaveDebateTranscript with
proper error handling: capture the error returned by
services.SaveDebateTranscript (called with userID, email, "impromptu",
req.Topic, "AI Evaluator", resultStr, history, nil) and log it instead of
discarding it; e.g., assign the call to err and call the module's logger/error
reporting (or fmt/log) to record a descriptive message including the error and
relevant identifiers (userID or req.Topic) so persistence failures are
observable.
In `@backend/services/impromptu.go`:
- Around line 12-16: The ImpromptuEvaluation type is dead code — remove the
struct declaration named ImpromptuEvaluation from the file (since the code uses
ImpromptuJudgmentData as the actual evaluation/return type) or, if you intended
to use it, replace any references to ImpromptuJudgmentData with
ImpromptuEvaluation; after the change, run a build to ensure no remaining
references to ImpromptuEvaluation exist and update imports/types accordingly.
In `@frontend/src/components/Sidebar.tsx`:
- Around line 47-51: The Sidebar uses the same MessageSquare icon for both
"Start Debate" and the Daily Challenge NavItem (see NavItem with to='/impromptu'
and label='Daily Challenge'), which reduces visual distinction; replace the
MessageSquare instance for the Daily Challenge NavItem with a different
lucide-react icon such as Zap, Clock, or Sparkles (import the chosen icon at the
top and update the icon prop on the NavItem) so the Daily Challenge appears
visually distinct from Start Debate.
In `@frontend/src/Pages/ImpromptuChallenge.tsx`:
- Around line 84-107: The timeout callback in the useEffect captures a stale
handleStopRecording reference — wrap handleStopRecording in useCallback (with
its real dependencies) or store the latest function in a ref and read
ref.current inside the timeout, then add that handleStopRecording (or the ref)
to the useEffect dependency array; ensure the cleanup still clears the timeout
and calls stopRecognition when phase !== 'RECORDING', and keep startRecognition,
stopRecognition, phase and any other functions used in the effect in the
dependency list as appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7d58ee2c-1dcf-4409-88ae-83f0557c3562
📒 Files selected for processing (8)
backend/cmd/server/main.gobackend/controllers/impromptu_controller.gobackend/routes/impromptu.gobackend/services/impromptu.gofrontend/src/App.tsxfrontend/src/Pages/ImpromptuChallenge.tsxfrontend/src/components/JudgementPopup.tsxfrontend/src/components/Sidebar.tsx
| // Determine win/loss based on total score (>= 70 is a win) | ||
| resultStr := "loss" | ||
| if eval.Total.User >= 70 { | ||
| resultStr = "win" | ||
| } |
There was a problem hiding this comment.
Critical: Win threshold of 70 is unreachable — maximum possible score is 40.
The scoring system has 4 sections (opening, cross-examination, answers, closing) each worth 0-10 points, for a maximum total of 40. The prompt template in services/impromptu.go (line 109) even states "winner": "<'User' if total >= 20, otherwise 'AI Evaluator'>", confirming the expected range.
With >= 70 as the threshold, users can never achieve a "win" result, which will incorrectly affect gamification metrics.
🐛 Proposed fix
// Determine win/loss based on total score (>= 70 is a win)
resultStr := "loss"
- if eval.Total.User >= 70 {
+ if eval.Total.User >= 20 {
resultStr = "win"
}Align with the prompt's verdict logic (20 out of 40 = 50% threshold).
📝 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.
| // Determine win/loss based on total score (>= 70 is a win) | |
| resultStr := "loss" | |
| if eval.Total.User >= 70 { | |
| resultStr = "win" | |
| } | |
| // Determine win/loss based on total score (>= 70 is a win) | |
| resultStr := "loss" | |
| if eval.Total.User >= 20 { | |
| resultStr = "win" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/controllers/impromptu_controller.go` around lines 80 - 84, The win
threshold is set incorrectly (>= 70) making a win impossible given the max score
is 40; update the logic in the block that sets resultStr (uses eval.Total.User
and resultStr) to use the correct threshold—either change the literal 70 to 20
to match the prompt's ">= 20" verdict, or compute a 50% threshold from the
maximum possible (e.g., maxScore/2) and compare eval.Total.User against that so
the win condition can actually be reached.
| // GetRandomImpromptuTopic returns a single random topic for the impromptu session | ||
| func GetRandomImpromptuTopic() string { | ||
| rand.Seed(time.Now().UnixNano()) | ||
| return impromptuTopics[rand.Intn(len(impromptuTopics))] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is rand.Seed deprecated in Go 1.20?
💡 Result:
Yes. In Go 1.20, math/rand.Seed (the top-level rand.Seed) was deprecated because the package now automatically seeds the global generator with a random value at startup. [1] The Go team also documents this change and its rationale in the math/rand/v2 write-up. [2]
What to do instead: if you need a repeatable sequence, create your own generator, e.g. r := rand.New(rand.NewSource(seed)), rather than seeding the global one. [1]
Sources:
[1] Go 1.20 Release Notes (math/rand auto-seeding; top-level Seed deprecated) (tip.golang.org)
[2] “Evolving the Go Standard Library with math/rand/v2” (notes Go 1.20 deprecated rand.Seed) (go.dev)
Citations:
- 1: https://tip.golang.org/doc/go1.20?utm_source=openai
- 2: https://go.dev/blog/randv2?utm_source=openai
🏁 Script executed:
# Check Go version
cat go.mod | grep "^go " | head -1
# Read the impromptu.go file to verify imports and usage
head -40 backend/services/impromptu.goRepository: AOSSIE-Org/DebateAI
Length of output: 1474
🏁 Script executed:
# Find go.mod
find . -name "go.mod" -type f 2>/dev/null | head -5
# Get line count of impromptu.go to read the whole file
wc -l backend/services/impromptu.go
# Read the full impromptu.go file
cat -n backend/services/impromptu.goRepository: AOSSIE-Org/DebateAI
Length of output: 5685
🏁 Script executed:
# Check Go version
cat backend/go.mod | grep "^go " | head -1
# Search for ImpromptuEvaluation usage in the codebase
rg "ImpromptuEvaluation" --type goRepository: AOSSIE-Org/DebateAI
Length of output: 134
Remove deprecated rand.Seed call.
Since Go 1.20, the global math/rand is automatically seeded with a random value, making rand.Seed unnecessary and deprecated. Additionally, calling Seed on every function invocation is inefficient and can cause issues in concurrent scenarios.
♻️ Suggested fix
// GetRandomImpromptuTopic returns a single random topic for the impromptu session
func GetRandomImpromptuTopic() string {
- rand.Seed(time.Now().UnixNano())
return impromptuTopics[rand.Intn(len(impromptuTopics))]
}📝 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.
| // GetRandomImpromptuTopic returns a single random topic for the impromptu session | |
| func GetRandomImpromptuTopic() string { | |
| rand.Seed(time.Now().UnixNano()) | |
| return impromptuTopics[rand.Intn(len(impromptuTopics))] | |
| } | |
| // GetRandomImpromptuTopic returns a single random topic for the impromptu session | |
| func GetRandomImpromptuTopic() string { | |
| return impromptuTopics[rand.Intn(len(impromptuTopics))] | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/services/impromptu.go` around lines 27 - 31, The
GetRandomImpromptuTopic function should stop calling rand.Seed on each
invocation; remove the rand.Seed(time.Now().UnixNano()) line so the function
simply returns impromptuTopics[rand.Intn(len(impromptuTopics))] and rely on Go's
automatic seeding (or, if you need a separate source for concurrency/control,
create and use a dedicated rand.New(rand.NewSource(...)) instance referenced
where needed). Update the GetRandomImpromptuTopic implementation to delete the
Seed call and keep only the random selection logic.
| const fetchTopic = async () => { | ||
| try { | ||
| const token = localStorage.getItem('token'); | ||
| const res = await fetch(`${import.meta.env.VITE_BASE_URL}/impromptu/topic`, { | ||
| headers: { Authorization: `Bearer ${token}` } | ||
| }); | ||
| const data = await res.json(); | ||
| setTopic(data.topic); | ||
| setTranscript(''); | ||
| transcriptRef.current = ''; | ||
| setPhase('PREP'); | ||
| } catch { | ||
| toast({ title: "Failed to fetch topic", variant: "destructive" }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Missing HTTP response status check before parsing JSON.
If the API returns a non-2xx status (e.g., 401, 500), the code still attempts to parse the response as JSON, which may fail or produce unexpected results. The topic would be set to undefined.
🛡️ Suggested fix
const fetchTopic = async () => {
try {
const token = localStorage.getItem('token');
const res = await fetch(`${import.meta.env.VITE_BASE_URL}/impromptu/topic`, {
headers: { Authorization: `Bearer ${token}` }
});
+ if (!res.ok) {
+ throw new Error(`HTTP ${res.status}`);
+ }
const data = await res.json();
setTopic(data.topic);
setTranscript('');
transcriptRef.current = '';
setPhase('PREP');
} catch {
toast({ title: "Failed to fetch topic", variant: "destructive" });
}
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/Pages/ImpromptuChallenge.tsx` around lines 109 - 123, In
fetchTopic, you need to check the HTTP response status before calling
res.json(); currently fetchTopic always parses the body and may
setTopic(undefined) on non-2xx responses. Update fetchTopic to verify res.ok (or
specific status codes) after awaiting fetch, and if not ok call toast with a
descriptive error (optionally include status or error text from the response)
and return early; only call await res.json() and setTopic(data.topic) when the
response is successful. Ensure transcriptRef, setTranscript and setPhase('PREP')
only run on successful responses.
| try { | ||
| const token = localStorage.getItem('token'); | ||
| const res = await fetch(`${import.meta.env.VITE_BASE_URL}/impromptu/evaluate`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}` }, | ||
| body: JSON.stringify({ topic, transcript: finalText }), | ||
| }); | ||
| const resJson = await res.json(); |
There was a problem hiding this comment.
Missing HTTP response status check for evaluate endpoint.
Similar to fetchTopic, the evaluate call should verify res.ok before parsing the response to avoid silently processing error responses.
🛡️ Suggested fix
const res = await fetch(`${import.meta.env.VITE_BASE_URL}/impromptu/evaluate`, {
method: 'POST',
headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}` },
body: JSON.stringify({ topic, transcript: finalText }),
});
+ if (!res.ok) {
+ throw new Error(`HTTP ${res.status}`);
+ }
const resJson = await res.json();📝 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.
| try { | |
| const token = localStorage.getItem('token'); | |
| const res = await fetch(`${import.meta.env.VITE_BASE_URL}/impromptu/evaluate`, { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}` }, | |
| body: JSON.stringify({ topic, transcript: finalText }), | |
| }); | |
| const resJson = await res.json(); | |
| try { | |
| const token = localStorage.getItem('token'); | |
| const res = await fetch(`${import.meta.env.VITE_BASE_URL}/impromptu/evaluate`, { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}` }, | |
| body: JSON.stringify({ topic, transcript: finalText }), | |
| }); | |
| if (!res.ok) { | |
| throw new Error(`HTTP ${res.status}`); | |
| } | |
| const resJson = await res.json(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/Pages/ImpromptuChallenge.tsx` around lines 132 - 139, The
evaluate fetch in ImpromptuChallenge.tsx currently parses the response without
checking HTTP status; update the try block where you call fetch(...) for
`${import.meta.env.VITE_BASE_URL}/impromptu/evaluate` to check res.ok before
calling await res.json(): if !res.ok, read and surface the error (e.g., await
res.text() or res.json()) and throw or handle it (so downstream code using
resJson isn't run on error). Ensure the Authorization token retrieval
(localStorage.getItem('token')), the fetch call, and the variables res and
resJson are the ones updated.
Addressed Issues:
Fixes #379
Screenshots/Recordings:
Idle Phase

Preparation Phase

Recording Phase

Result

Result

Additional Notes:
This PR implements the "Daily Impromptu Challenge" feature as requested in #379. The implementation is designed to be highly efficient by maximizing the reuse of existing infrastructure:
Frontend Implementation:
Backend Implementation:
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: Google Gemini
Checklist
Summary by CodeRabbit