Web Speech API continuous mode stops automatically after silence#330
Open
Abhishek2005-ard wants to merge 18 commits intoAOSSIE-Org:mainfrom
Open
Web Speech API continuous mode stops automatically after silence#330Abhishek2005-ard wants to merge 18 commits intoAOSSIE-Org:mainfrom
Abhishek2005-ard wants to merge 18 commits intoAOSSIE-Org:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an EloChange field to saved debate transcripts and propagates it through save and judge flows; enhances frontend speech-recognition (async permission checks, auto-restart guards); introduces JSON5 parsing and dark-mode/contrast styling; refactors route structure and updates several UI components. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
backend/services/transcriptservice.go (2)
968-976:⚠️ Potential issue | 🟡 Minor
GetDebateStatsstill hardcodeseloChange: 0.This function mirrors the logic in
GetProfilebut wasn't updated to usetranscript.EloChange. The TODO on line 975 is now stale since the model has the field.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 968 - 976, GetDebateStats is still hardcoding "eloChange": 0 when building recentDebates; update the map construction in GetDebateStats to use the transcript.EloChange field (e.g., replace the hardcoded 0 with transcript.EloChange) so recentDebates mirrors the GetProfile logic and returns the actual Elo delta stored on the Transcript model.
180-206:⚠️ Potential issue | 🔴 CriticalCompilation errors and ordering bug in EloChange propagation.
Three issues in this block:
forRecordis undefined (line 189). The variable doesn't exist in scope —UpdateRatingson line 217 returnsdebateRecord/opponentRecord, notforRecord. This won't compile.Missing
eloChangeargument for the "against" user call (lines 195–204).SaveDebateTranscriptnow requires aneloChange float64parameter per line 663, but this call site still uses the old arity. This also won't compile.Ordering problem: Both
SaveDebateTranscriptcalls happen beforeUpdateRatings(line 217), so the rating change values haven't been computed yet. The saves need to happen after the Elo computation, or the transcripts need a follow-up update.Proposed fix: move saves after rating computation
- // Save transcript for "for" user - err = SaveDebateTranscript( - forUser.ID, - forUser.Email, - "user_vs_user", - topic, - againstUser.Email, - resultFor, - []models.Message{}, // You might want to reconstruct messages from transcripts - forSubmission.Transcripts, - forRecord.RatingChange - ) - if err != nil { - } - - // Save transcript for "against" user - err = SaveDebateTranscript( - againstUser.ID, - againstUser.Email, - "user_vs_user", - topic, - forUser.Email, - resultAgainst, - []models.Message{}, // You might want to reconstruct messages from transcripts - againstSubmission.Transcripts, - ) - if err != nil { - } - // Update ratings based on the result outcomeFor := 0.5 switch strings.ToLower(resultFor) { case "win": outcomeFor = 1.0 case "loss": outcomeFor = 0.0 } debateRecord, opponentRecord, ratingErr := UpdateRatings(forUser.ID, againstUser.ID, outcomeFor, time.Now()) if ratingErr != nil { } else { debateRecord.Topic = topic debateRecord.Result = resultFor opponentRecord.Topic = topic opponentRecord.Result = resultAgainst records := []interface{}{debateRecord, opponentRecord} if _, insertErr := db.MongoDatabase.Collection("debates").InsertMany(ctx, records); insertErr != nil { } ratingSummary = map[string]interface{}{ "for": map[string]float64{ "rating": debateRecord.PostRating, "change": debateRecord.RatingChange, }, "against": map[string]float64{ "rating": opponentRecord.PostRating, "change": opponentRecord.RatingChange, }, } + + // Save transcript for "for" user (after Elo is computed) + err = SaveDebateTranscript( + forUser.ID, + forUser.Email, + "user_vs_user", + topic, + againstUser.Email, + resultFor, + []models.Message{}, + forSubmission.Transcripts, + debateRecord.RatingChange, + ) + if err != nil { + log.Printf("failed to save 'for' transcript: %v", err) + } + + // Save transcript for "against" user + err = SaveDebateTranscript( + againstUser.ID, + againstUser.Email, + "user_vs_user", + topic, + forUser.Email, + resultAgainst, + []models.Message{}, + againstSubmission.Transcripts, + opponentRecord.RatingChange, + ) + if err != nil { + log.Printf("failed to save 'against' transcript: %v", err) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/services/transcriptservice.go` around lines 180 - 206, The three issues are: 1) replace the undefined forRecord with the actual return values from UpdateRatings (use debateRecord and opponentRecord returned by UpdateRatings) when reading RatingChange; 2) supply the missing eloChange float64 argument to the second SaveDebateTranscript call (pass opponentRecord.RatingChange or the appropriate RatingChange field); and 3) move both SaveDebateTranscript calls to after the UpdateRatings invocation so you pass computed rating changes. Concretely, call UpdateRatings(...) first, capture debateRecord and opponentRecord, then call SaveDebateTranscript(forUser.ID, forUser.Email, ..., debateRecord.RatingChange, ...) and SaveDebateTranscript(againstUser.ID, againstUser.Email, ..., opponentRecord.RatingChange, ...), using forSubmission.Transcripts / againstSubmission.Transcripts and forUser/againstUser to populate the other args.backend/controllers/profile_controller.go (3)
50-53:⚠️ Potential issue | 🟠 MajorExcessive PII logging in request handler.
These
log.Printfcalls log the full request URL (which may contain user IDs and tokens), raw query strings, and all query parameters on everyGetProfilecall. This is a compliance/privacy risk in production and will generate high log volume. Consider gating behind a debug flag or removing entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/controllers/profile_controller.go` around lines 50 - 53, The GetProfile handler currently logs full request details (c.Request.URL.String, c.Request.URL.RawQuery, c.Request.URL.Query and userIDParam) which may leak PII; remove these unconditional log.Printf calls or gate them behind a debug/verbose flag; update the GetProfile function to either delete those four logging lines or wrap them with a check (e.g., if debugEnabled) so only non-production debug runs emit them, and keep only minimal, non-sensitive logs (e.g., request received for GetProfile with sanitized user id or request ID) to locate the handler during normal operation.
366-367:⚠️ Potential issue | 🟠 MajorErrors from
ObjectIDFromHexare silently discarded.If
req.WinnerIDorreq.LoserIDare not valid hex strings,winnerID/loserIDwill be zero-value ObjectIDs, leading to silent data corruption (wrong users looked up / updated). Validate and return early on error.Proposed fix
- winnerID, _ := primitive.ObjectIDFromHex(req.WinnerID) - loserID, _ := primitive.ObjectIDFromHex(req.LoserID) + winnerID, err := primitive.ObjectIDFromHex(req.WinnerID) + if err != nil { + ctx.JSON(http.StatusBadRequest, gin.H{"error": "Invalid winner ID"}) + return + } + loserID, err := primitive.ObjectIDFromHex(req.LoserID) + if err != nil { + ctx.JSON(http.StatusBadRequest, gin.H{"error": "Invalid loser ID"}) + return + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/controllers/profile_controller.go` around lines 366 - 367, The code currently discards errors from primitive.ObjectIDFromHex when converting req.WinnerID and req.LoserID (the variables winnerID and loserID), which can produce zero-value ObjectIDs; update the handler to check the returned error for each call to primitive.ObjectIDFromHex, and if either conversion fails return early with a client error (e.g., HTTP 400 / bad request) and a clear message about the invalid ID instead of proceeding; locate the conversion calls around winnerID, loserID and add the validation and early return logic there.
104-104:⚠️ Potential issue | 🟠 MajorUser email and display name logged at INFO level.
Line 104 logs
user.Emailanduser.DisplayName— both are PII. This should be removed or moved behind a debug flag.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/controllers/profile_controller.go` at line 104, The log in GetProfile currently prints PII (user.Email and user.DisplayName) via log.Printf; remove those fields from the info-level log and only log non-PII (e.g., user.ID via user.ID.Hex()), or wrap the detailed log in a debug-only guard (e.g., if debug or logger.IsDebug() then log user.Email and user.DisplayName). Update the log call in GetProfile (where log.Printf is used) to either omit user.Email and user.DisplayName or conditionalize their inclusion behind a debug flag.frontend/src/Pages/DebateRoom.tsx (1)
322-322:⚠️ Potential issue | 🔴 CriticalPR objective not met: speech recognition auto-restart on silence is not implemented.
The PR title states this fixes "Web Speech API continuous mode stops automatically after silence," and the description mentions the workaround of "manually restarting recognition inside the
onendevent." However,onendonly setssetIsRecognizing(false)— it never restarts recognition. When the browser silently ends recognition (which happens even withcontinuous = true), the user must manually click the mic button again.To implement the stated fix:
Proposed auto-restart implementation
- recognitionRef.current.onend = () => setIsRecognizing(false); + recognitionRef.current.onend = () => { + // Browser may stop recognition due to silence even in continuous mode. + // Auto-restart if we're still supposed to be recognizing. + if (isRecognizing && recognitionRef.current) { + try { + recognitionRef.current.start(); + } catch (e) { + console.warn("Failed to restart speech recognition:", e); + setIsRecognizing(false); + } + } else { + setIsRecognizing(false); + } + };Note: Since
isRecognizingis React state, you'll need a ref to track the "should be listening" intent to avoid stale closure issues inside theonendcallback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/Pages/DebateRoom.tsx` at line 322, The onend handler currently only calls setIsRecognizing(false) but never restarts the Web Speech recognition; add a ref (e.g., shouldBeListeningRef) that mirrors the user's intent to keep listening (updated wherever you toggle listening and when isRecognizing would be set) to avoid stale closures, then update recognitionRef.current.onend to check shouldBeListeningRef.current and, if true, attempt to restart recognition by calling recognitionRef.current.start() (wrap in a try/catch and setIsRecognizing(true) on success); ensure recognitionRef and shouldBeListeningRef are cleaned up on unmount/stop to avoid leaks.frontend/src/components/JudgementPopup.tsx (1)
94-107:⚠️ Potential issue | 🟠 MajorHardcoded
localhost:5173URLs will break in any deployed environment.The
coachSkillsarray uses absolutehttp://localhost:5173/coach/...URLs. These should be relative paths (e.g.,/coach/strengthen-argument) since react-router handles navigation, or usenavigate()instead of<a href>links.Proposed fix
const coachSkills: CoachSkill[] = [ { title: 'Strengthen Argument', description: 'Master the art of crafting compelling, persuasive arguments that win debates.', - url: 'http://localhost:5173/coach/strengthen-argument', + url: '/coach/strengthen-argument', }, { title: 'Pros and Cons Challenge', description: 'Test your critical thinking by crafting up to 5 pros and cons for engaging debate topics.', - url: 'http://localhost:5173/coach/pros-cons', + url: '/coach/pros-cons', }, ];And update the link rendering (lines 720–727) to use
react-router'sLinkornavigate()instead of<a href>withtarget="_blank".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/JudgementPopup.tsx` around lines 94 - 107, The coachSkills array contains hardcoded absolute localhost URLs which will break in production; update the url values in the coachSkills constant to use relative paths (e.g., "/coach/strengthen-argument", "/coach/pros-cons") and replace the existing anchor-based navigation in the JudgementPopup link rendering (the code that currently renders <a href=... target="_blank">) with react-router navigation—either render <Link to={skill.url}> from react-router-dom or invoke navigate(skill.url) (imported from useNavigate) so routing is handled client-side and no absolute host is baked into the bundle.
🧹 Nitpick comments (5)
backend/controllers/profile_controller.go (1)
257-258: Remove the stale TODO comment.
transcript.EloChangenow provides the actual value — the// TODO: Add actual Elo change trackingcomment is outdated.Proposed fix
- "eloChange": transcript.EloChange,// 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/controllers/profile_controller.go` around lines 257 - 258, Remove the outdated trailing comment after the eloChange field: in the response map where "eloChange" is set to transcript.EloChange, delete the "// TODO: Add actual Elo change tracking" comment so the code reflects that transcript.EloChange now contains the real value (reference symbol: transcript.EloChange).backend/models/transcript.go (1)
41-54: Redundant NilObjectID assignments in MarshalJSON.Lines 44–45 set
a.IDanda.UserIDtoNilObjectID, but those fields are already shadowed by the outer struct'sID stringandUserID stringat a shallower embedding depth — Go'sencoding/jsonwill always pick the shallower field. The assignments are harmless but unnecessary.Simplify by removing the NilObjectID assignments
func (s SavedDebateTranscript) MarshalJSON() ([]byte, error) { type Alias SavedDebateTranscript - a := Alias(s) - a.ID = primitive.NilObjectID - a.UserID = primitive.NilObjectID return json.Marshal(&struct { ID string `json:"id"` UserID string `json:"userId"` Alias }{ ID: s.ID.Hex(), UserID: s.UserID.Hex(), - Alias: a, + Alias: Alias(s), }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/models/transcript.go` around lines 41 - 54, The MarshalJSON implementation for SavedDebateTranscript redundantly assigns NilObjectID to a.ID and a.UserID before embedding Alias; remove the unnecessary assignments in SavedDebateTranscript.MarshalJSON (the variables a.ID and a.UserID) since the outer struct's ID and UserID string fields already shadow them for JSON encoding, leaving the rest of the function (Alias type, outer struct with ID/UserID string and json.Marshal call) unchanged.frontend/src/App.tsx (1)
97-98: DuplicateViewDebateroutes — clarify intent or consolidate.Two routes render the same
<ViewDebate />component:
debate/:debateID/view(line 97)view-debate/:debateID(line 98)If one is a legacy path for backward compatibility, add a comment. Otherwise, consolidate to a single canonical route and use
<Navigate>for the old path.🤖 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, There are two routes rendering the same component (Route path="debate/:debateID/view" and Route path="view-debate/:debateID" both using <ViewDebate />); decide which path is canonical and either remove the duplicate or keep the legacy path and replace it with a redirect using <Navigate> to the canonical route; update or add a comment explaining if the second path is kept for backward compatibility and reference the Route entries and the ViewDebate component when making the change.frontend/src/Pages/DebateRoom.tsx (1)
579-579: Inconsistent indentation: function body uses mixed indentation.Line 579 has extra leading spaces (
const judgeDebateResult) compared to other top-level function declarations in this component (e.g.,handleBotTurnat line 516,sendMessageat line 491 which use 2-space indent). This suggests a copy-paste or merge artifact.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/Pages/DebateRoom.tsx` at line 579, The function declaration for judgeDebateResult has inconsistent leading spaces compared to other top-level functions (e.g., handleBotTurn, sendMessage); fix by normalizing indentation to match the component's 2-space top-level style—adjust the declaration "const judgeDebateResult = async (messages: Message[])" and its entire body to use the same 2-space indentation as handleBotTurn and sendMessage so there are no mixed indent levels.frontend/src/components/Header.tsx (1)
130-191: Notification popover still uses hardcoded colors, inconsistent with the token migration.The notification popover content (e.g.,
text-gray-900,bg-gray-50,hover:bg-gray-50,bg-blue-50/50,border-gray-100) hasn't been migrated to design tokens. In dark mode, these will render as light-themed elements inside a dark header. The same applies to the user profile popover (lines 205–240).This could be addressed in a follow-up if it's out of scope for this PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Header.tsx` around lines 130 - 191, Replace hardcoded Tailwind color classes in the notification popover with the project's design token classes so colors respond to light/dark theme; specifically update className usages inside PopoverContent, the notification item wrapper (the div with className starting with "p-4 hover:bg-gray-50..."), the unread background ("bg-blue-50/50" and the unread dot "bg-blue-500"), the header border ("border-gray-100"), text classes like "text-gray-900"/"text-gray-500"/"text-gray-400", and the empty-state text; mirror the same token replacements in the user profile popover (the popover rendered around lines ~205–240) so both popovers use tokens (e.g., text-foreground / text-muted, bg-card / bg-muted, border-muted, hover:bg-muted, accent/unread token) instead of hardcoded gray/blue classes.
🤖 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/components/Matchmaking.tsx`:
- Around line 38-42: The useEffect that resets matchmaking state on mount is
redundant because the state variables are already initialized via
useState(false) and useState(0); remove the entire effect block (the useEffect
wrapper and its callback that calls setIsInPool(false) and setWaitTime(0)),
along with the accompanying comment and extra blank lines, leaving the useState
declarations (isInPool, setIsInPool and waitTime, setWaitTime) intact.
In `@frontend/src/index.css`:
- Around line 88-91: Register the "high-contrast" Tailwind variant and move the
.high-contrast rule into the base layer: add an addVariant('high-contrast',
'@media (prefers-contrast: more)') plugin entry in tailwind.config.js so usages
like high-contrast:data-[highlighted]:bg-... in select.tsx are preserved, and
relocate the .high-contrast CSS block into `@layer` base alongside the .contrast
theme block to ensure correct cascade ordering and consistency.
In `@frontend/src/Pages/DebateRoom.tsx`:
- Around line 759-762: The onClose handler in DebateRoom (the anonymous function
using setShowJudgment and navigate) currently navigates to "/" which causes
authenticated users to be redirected to "/startDebate" and produces a visible
flash; change the navigate target to "/startDebate" (or "/game" if preferred) so
the handler calls navigate("/startDebate") after setShowJudgment(false) to avoid
the redundant redirect.
- Around line 599-604: The code calls JSON5.parse (used to produce JudgmentData
in the DebateRoom component), but json5 is not listed in frontend/package.json
dependencies; add "json5": "^2.2.3" to the dependencies section of
frontend/package.json and run your package manager to install (e.g., npm install
or yarn install) so the existing import and the JSON5.parse usage resolve at
runtime.
---
Outside diff comments:
In `@backend/controllers/profile_controller.go`:
- Around line 50-53: The GetProfile handler currently logs full request details
(c.Request.URL.String, c.Request.URL.RawQuery, c.Request.URL.Query and
userIDParam) which may leak PII; remove these unconditional log.Printf calls or
gate them behind a debug/verbose flag; update the GetProfile function to either
delete those four logging lines or wrap them with a check (e.g., if
debugEnabled) so only non-production debug runs emit them, and keep only
minimal, non-sensitive logs (e.g., request received for GetProfile with
sanitized user id or request ID) to locate the handler during normal operation.
- Around line 366-367: The code currently discards errors from
primitive.ObjectIDFromHex when converting req.WinnerID and req.LoserID (the
variables winnerID and loserID), which can produce zero-value ObjectIDs; update
the handler to check the returned error for each call to
primitive.ObjectIDFromHex, and if either conversion fails return early with a
client error (e.g., HTTP 400 / bad request) and a clear message about the
invalid ID instead of proceeding; locate the conversion calls around winnerID,
loserID and add the validation and early return logic there.
- Line 104: The log in GetProfile currently prints PII (user.Email and
user.DisplayName) via log.Printf; remove those fields from the info-level log
and only log non-PII (e.g., user.ID via user.ID.Hex()), or wrap the detailed log
in a debug-only guard (e.g., if debug or logger.IsDebug() then log user.Email
and user.DisplayName). Update the log call in GetProfile (where log.Printf is
used) to either omit user.Email and user.DisplayName or conditionalize their
inclusion behind a debug flag.
In `@backend/services/transcriptservice.go`:
- Around line 968-976: GetDebateStats is still hardcoding "eloChange": 0 when
building recentDebates; update the map construction in GetDebateStats to use the
transcript.EloChange field (e.g., replace the hardcoded 0 with
transcript.EloChange) so recentDebates mirrors the GetProfile logic and returns
the actual Elo delta stored on the Transcript model.
- Around line 180-206: The three issues are: 1) replace the undefined forRecord
with the actual return values from UpdateRatings (use debateRecord and
opponentRecord returned by UpdateRatings) when reading RatingChange; 2) supply
the missing eloChange float64 argument to the second SaveDebateTranscript call
(pass opponentRecord.RatingChange or the appropriate RatingChange field); and 3)
move both SaveDebateTranscript calls to after the UpdateRatings invocation so
you pass computed rating changes. Concretely, call UpdateRatings(...) first,
capture debateRecord and opponentRecord, then call
SaveDebateTranscript(forUser.ID, forUser.Email, ..., debateRecord.RatingChange,
...) and SaveDebateTranscript(againstUser.ID, againstUser.Email, ...,
opponentRecord.RatingChange, ...), using forSubmission.Transcripts /
againstSubmission.Transcripts and forUser/againstUser to populate the other
args.
In `@frontend/src/components/JudgementPopup.tsx`:
- Around line 94-107: The coachSkills array contains hardcoded absolute
localhost URLs which will break in production; update the url values in the
coachSkills constant to use relative paths (e.g., "/coach/strengthen-argument",
"/coach/pros-cons") and replace the existing anchor-based navigation in the
JudgementPopup link rendering (the code that currently renders <a href=...
target="_blank">) with react-router navigation—either render <Link
to={skill.url}> from react-router-dom or invoke navigate(skill.url) (imported
from useNavigate) so routing is handled client-side and no absolute host is
baked into the bundle.
In `@frontend/src/Pages/DebateRoom.tsx`:
- Line 322: The onend handler currently only calls setIsRecognizing(false) but
never restarts the Web Speech recognition; add a ref (e.g.,
shouldBeListeningRef) that mirrors the user's intent to keep listening (updated
wherever you toggle listening and when isRecognizing would be set) to avoid
stale closures, then update recognitionRef.current.onend to check
shouldBeListeningRef.current and, if true, attempt to restart recognition by
calling recognitionRef.current.start() (wrap in a try/catch and
setIsRecognizing(true) on success); ensure recognitionRef and
shouldBeListeningRef are cleaned up on unmount/stop to avoid leaks.
---
Nitpick comments:
In `@backend/controllers/profile_controller.go`:
- Around line 257-258: Remove the outdated trailing comment after the eloChange
field: in the response map where "eloChange" is set to transcript.EloChange,
delete the "// TODO: Add actual Elo change tracking" comment so the code
reflects that transcript.EloChange now contains the real value (reference
symbol: transcript.EloChange).
In `@backend/models/transcript.go`:
- Around line 41-54: The MarshalJSON implementation for SavedDebateTranscript
redundantly assigns NilObjectID to a.ID and a.UserID before embedding Alias;
remove the unnecessary assignments in SavedDebateTranscript.MarshalJSON (the
variables a.ID and a.UserID) since the outer struct's ID and UserID string
fields already shadow them for JSON encoding, leaving the rest of the function
(Alias type, outer struct with ID/UserID string and json.Marshal call)
unchanged.
In `@frontend/src/App.tsx`:
- Around line 97-98: There are two routes rendering the same component (Route
path="debate/:debateID/view" and Route path="view-debate/:debateID" both using
<ViewDebate />); decide which path is canonical and either remove the duplicate
or keep the legacy path and replace it with a redirect using <Navigate> to the
canonical route; update or add a comment explaining if the second path is kept
for backward compatibility and reference the Route entries and the ViewDebate
component when making the change.
In `@frontend/src/components/Header.tsx`:
- Around line 130-191: Replace hardcoded Tailwind color classes in the
notification popover with the project's design token classes so colors respond
to light/dark theme; specifically update className usages inside PopoverContent,
the notification item wrapper (the div with className starting with "p-4
hover:bg-gray-50..."), the unread background ("bg-blue-50/50" and the unread dot
"bg-blue-500"), the header border ("border-gray-100"), text classes like
"text-gray-900"/"text-gray-500"/"text-gray-400", and the empty-state text;
mirror the same token replacements in the user profile popover (the popover
rendered around lines ~205–240) so both popovers use tokens (e.g.,
text-foreground / text-muted, bg-card / bg-muted, border-muted, hover:bg-muted,
accent/unread token) instead of hardcoded gray/blue classes.
In `@frontend/src/Pages/DebateRoom.tsx`:
- Line 579: The function declaration for judgeDebateResult has inconsistent
leading spaces compared to other top-level functions (e.g., handleBotTurn,
sendMessage); fix by normalizing indentation to match the component's 2-space
top-level style—adjust the declaration "const judgeDebateResult = async
(messages: Message[])" and its entire body to use the same 2-space indentation
as handleBotTurn and sendMessage so there are no mixed indent levels.
… initialized via useState)
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 (4)
frontend/src/Pages/DebateRoom.tsx (2)
812-815:⚠️ Potential issue | 🟡 MinorUser panel is missing
dark:classes that were added to the bot panel.The bot section (line 768) has
dark:bg-zinc-900anddark:border-zinc-700, but the user section here does not. This will cause a visual mismatch in dark mode.Proposed fix
<div className={`relative w-full md:w-1/2 ${ !state.isBotTurn && !state.isDebateEnded ? "animate-glow" : "" - } 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 812 - 815, The user panel div (the JSX element whose className begins with "relative w-full md:w-1/2") is missing the dark-mode utility classes present on the bot panel; update its className to include dark:bg-zinc-900 and dark:border-zinc-700 so the user panel matches the bot panel in dark mode, keeping the existing conditional "animate-glow", bg-white, border, and other classes intact.
288-334:⚠️ Potential issue | 🔴 CriticalThe
onendhandler does not auto-restart recognition — the stated PR objective is unmet.The PR's core objective is to fix continuous speech recognition stopping after silence. However, the
onendhandler on line 322 simply setsisRecognizingtofalse, meaning when Chrome's Web Speech API internally stops (due to silence/timeout — a known browser behavior even withcontinuous = true), the mic turns off and the user must manually re-enable it.To actually keep recognition alive until the user explicitly stops it, you need to auto-restart in
onendwhen the stop wasn't user-initiated. Use a ref to track explicit stops:Proposed fix: auto-restart recognition on browser-initiated stops
Add a ref to track whether the user explicitly requested a stop:
const recognitionRef = useRef<SpeechRecognition | null>(null); + const manualStopRef = useRef(false);Update the
onendhandler to auto-restart when the stop was not user-initiated:- recognitionRef.current.onend = () => setIsRecognizing(false); + recognitionRef.current.onend = () => { + if (manualStopRef.current) { + manualStopRef.current = false; + setIsRecognizing(false); + } else if (recognitionRef.current) { + // Browser stopped due to silence/timeout — restart automatically + try { + recognitionRef.current.start(); + } catch (e) { + console.error("Failed to restart recognition:", e); + setIsRecognizing(false); + } + } + };Update
stopRecognitionto signal an explicit stop:const stopRecognition = () => { if (recognitionRef.current && isRecognizing) { + manualStopRef.current = true; recognitionRef.current.stop(); setIsRecognizing(false); } };Also update the cleanup to avoid unintended restarts on unmount:
return () => { - if (recognitionRef.current) recognitionRef.current.stop(); + if (recognitionRef.current) { + manualStopRef.current = true; + recognitionRef.current.stop(); + } };🤖 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 288 - 334, The onend handler currently only sets isRecognizing to false, so browser-initiated stops (silence/timeouts) aren't auto-restarted; add a userStoppedRef boolean ref to distinguish user-initiated stops from browser-initiated ones, update onend (recognitionRef.current.onend) to auto-restart recognitionRef.current.start() and keep isRecognizing true when userStoppedRef.current is false, update your stopRecognition function to set userStoppedRef.current = true before calling recognitionRef.current.stop() and set isRecognizing false, and update the cleanup in the effect to set userStoppedRef.current = true before stopping to prevent restarts on unmount; also ensure startRecognition clears userStoppedRef.current = false and calls recognitionRef.current.start().backend/controllers/debatevsbot_controller.go (1)
588-597:⚠️ Potential issue | 🔴 CriticalCompilation error —
SaveDebateTranscriptcall inConcedeDebateis missing the requiredeloChange float64argument.The updated signature now has 9 parameters, but this call site only supplies 8.
🐛 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 SaveDebateTranscript in ConcedeDebate is missing the new eloChange float64 parameter (SaveDebateTranscript now expects 9 args); update the call site in ConcedeDebate to pass a float64 elo change value (either the computed elo change variable if available or a default like 0.0) as the penultimate argument so the final nil matches the updated signature for the optional field; locate the call to services.SaveDebateTranscript and insert the appropriate float64 value (e.g., eloChange) before the nil.backend/services/transcriptservice.go (1)
181-244:⚠️ Potential issue | 🟡 Minor
eloChangewill always be persisted as0.0— Elo data is computed after the transcript saves.The
SaveDebateTranscriptcalls (lines 181–211) execute beforeUpdateRatings(line 222), sodebateRecord.RatingChangeandopponentRecord.RatingChangeare not yet available when the transcripts are written. The simplest fix is to reorder: compute ratings first, then save transcripts using the actual deltas.♻️ Proposed reordering
- // Save transcript for "for" user - err = SaveDebateTranscript(... 0.0 ...) - // Save transcript for "against" user - err = SaveDebateTranscript(... 0.0 ...) - // Update ratings based on the result outcomeFor := 0.5 ... debateRecord, opponentRecord, ratingErr := UpdateRatings(forUser.ID, againstUser.ID, outcomeFor, time.Now()) + + eloFor := 0.0 + eloAgainst := 0.0 if ratingErr != nil { - } else { + } else { debateRecord.Topic = topic ... + eloFor = debateRecord.RatingChange + eloAgainst = opponentRecord.RatingChange ... } + + // Save transcript for "for" user + err = SaveDebateTranscript(... eloFor ...) + // Save transcript for "against" user + err = SaveDebateTranscript(... eloAgainst ...)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/services/transcriptservice.go` around lines 181 - 244, The transcript saves call SaveDebateTranscript with a hardcoded eloChange (0.0) before UpdateRatings runs, so saved transcripts never include the computed RatingChange; call UpdateRatings(forUser.ID, againstUser.ID, outcomeFor, time.Now()) first, handle ratingErr and set debateRecord/opponentRecord, then pass debateRecord.RatingChange and opponentRecord.RatingChange (instead of 0.0) into the two SaveDebateTranscript calls and only after that insert the debate records into the "debates" collection and build ratingSummary.
🧹 Nitpick comments (7)
frontend/src/Pages/DebateRoom.tsx (2)
579-656: Good addition of thejudgingRefconcurrency guard andJSON5.parsefor resilient parsing.The guard prevents duplicate judgment calls, and JSON5 handles non-strict JSON from LLM output gracefully. The fallback judgment data in the catch block is solid UX.
Minor nit: inconsistent indentation on line 579 (
const— extra leading spaces vs. surrounding function declarations) and missing space on line 602 (}catch→} catch).🤖 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 579 - 656, The function declaration for judgeDebateResult has inconsistent leading indentation and there's a missing space in the inner catch (`}catch`) which affects readability; open frontend/src/Pages/DebateRoom.tsx, locate the judgeDebateResult function (referencing judgingRef, judgeDebate, JSON5.parse) and normalize indentation to match surrounding function declarations (remove the extra leading spaces before "const judgeDebateResult") and change `}catch` to `} catch` inside the JSON5.parse try/catch block so the spacing is consistent.
323-327: UseSpeechRecognitionErrorEventtype for theonerrorhandler.The project has a
SpeechRecognitionErrorEventtype definition available infrontend/src/types/speech-recognition.d.ts. The current manual castevent as Event & { error?: string }should be replaced with the proper type. This improves consistency with other files in the codebase (e.g.,SpeechTest.tsxalready usesSpeechRecognitionErrorEvent) and provides better type safety:Suggested change
- recognitionRef.current.onerror = (event: Event) => { - const errorEvent = event as Event & { error?: string }; - console.error("Speech recognition error:", errorEvent.error ?? event); + recognitionRef.current.onerror = (event: SpeechRecognitionErrorEvent) => { + console.error("Speech recognition error:", event.error);🤖 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 323 - 327, Replace the manual cast in the onerror handler with the project's SpeechRecognitionErrorEvent type: update recognitionRef.current.onerror to accept (event: SpeechRecognitionErrorEvent) and use event.error (or fallback to event) when logging, then call setIsRecognizing(false); this keeps types consistent with SpeechTest.tsx and uses the SpeechRecognitionErrorEvent definition from frontend/src/types/speech-recognition.d.ts.frontend/src/utils/speechTest.ts (4)
78-85: Non-transient errors (network,audio-capture) trigger an unbounded rapid restart loop.After
onerror, the browser firesonend, which schedules another restart in 500ms, encounters the same error, firesonendagain — infinite loop at 500ms intervals. Onlynot-allowedandservice-not-allowedcurrently break the cycle.♻️ Proposed fix — stop restart for errors unlikely to self-resolve
this.recognition.onerror = (event: any) => { console.error("Speech error:", event.error, event.message); - // stop infinite restart loop if permission denied if ( event.error === "not-allowed" || - event.error === "service-not-allowed" + event.error === "service-not-allowed" || + event.error === "network" || + event.error === "audio-capture" ) { this.shouldRestart = false; } };🤖 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 78 - 85, The onerror handler in speechTest.ts currently only disables restarts for permission errors, causing rapid 500ms restart loops for non-transient errors; update the recognition.onerror implementation (the handler attached to this.recognition and code that sets this.shouldRestart) to also set this.shouldRestart = false when event.error is "network" or "audio-capture" (and keep the existing "not-allowed" / "service-not-allowed" checks), and ensure you still log the error; this prevents unbounded restart retries triggered by onend while keeping transient errors restartable.
127-135:checkMicrophonePermission()activates mic hardware on every call — prefer the Permissions API for read-only state checks.
getUserMediabriefly opens a live audio stream (triggering the OS mic indicator light and browser tab icon) even though the tracks are stopped immediately. For the target Chrome/Edge environment,navigator.permissions.queryaccepts"microphone"as a permission name and returns the grant state without touching hardware. If permissions are granted you don't need to ask again;"prompt"is the default state where the browser asks the user; and the most useful state is"denied", meaning the user has explicitly blocked the page.♻️ Proposed refactor using the Permissions API
public async checkMicrophonePermission(): Promise<boolean> { try { - const stream = await navigator.mediaDevices.getUserMedia({ audio: true }); - stream.getTracks().forEach(track => track.stop()); - return true; - } catch { - return false; + const result = await navigator.permissions.query({ + name: 'microphone' as PermissionName, + }); + if (result.state === 'denied') return false; + if (result.state === 'granted') return true; + // 'prompt' — trigger the actual browser permission dialog via getUserMedia + const stream = await navigator.mediaDevices.getUserMedia({ audio: true }); + stream.getTracks().forEach(track => track.stop()); + return true; + } catch { + return false; } }🤖 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 127 - 135, Replace the current hardware-activating approach in checkMicrophonePermission() with a Permissions API query: call navigator.permissions.query({ name: "microphone" }) and return true only when the returned PermissionStatus.state === "granted", return false for "denied" or "prompt"; if navigator.permissions is undefined (older browsers), keep a guarded fallback to the existing getUserMedia try/catch but ensure the Permissions path is preferred so you don't open the mic unnecessarily. Ensure you reference and update the checkMicrophonePermission method to implement this logic and handle errors from permissions.query gracefully.
78-85: Persistent errors (network,audio-capture) cause an unbounded rapid restart loop.Only
not-allowedandservice-not-alloweddisable auto-restart. Errors likenetworkoraudio-captureleaveshouldRestart = true, so everyonend(which fires after eachonerror) immediately schedules anotherstart()500ms later — producing an infinite retry loop with no backoff.♻️ Proposed fix — disable restart for non-transient errors
this.recognition.onerror = (event: any) => { console.error("Speech error:", event.error, event.message); - // stop infinite restart loop if permission denied - if (event.error === "not-allowed" || event.error === "service-not-allowed") { + // stop restart for errors that won't self-resolve + if ( + event.error === "not-allowed" || + event.error === "service-not-allowed" || + event.error === "network" || + event.error === "audio-capture" + ) { this.shouldRestart = false; } };
127-135:checkMicrophonePermissionbriefly activates the microphone on everystart()call.Calling
getUserMediajust to check permission activates the device (indicator light, OS audio graph), even though the tracks are immediately stopped. For Chrome/Edge, the Permissions API provides a read-only permission check without hardware activation.♻️ Proposed refactor using the Permissions API
public async checkMicrophonePermission(): Promise<boolean> { try { - const stream = await navigator.mediaDevices.getUserMedia({ audio: true }); - stream.getTracks().forEach(track => track.stop()); - return true; - } catch { - return false; + const result = await navigator.permissions.query({ name: 'microphone' as PermissionName }); + if (result.state === 'granted') return true; + if (result.state === 'denied') return false; + // 'prompt' — fall back to getUserMedia to trigger the actual prompt + const stream = await navigator.mediaDevices.getUserMedia({ audio: true }); + stream.getTracks().forEach(track => track.stop()); + return true; + } catch { + return false; } }🤖 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 127 - 135, The checkMicrophonePermission function currently calls navigator.mediaDevices.getUserMedia which activates hardware; change it to first use the Permissions API by calling navigator.permissions.query({ name: 'microphone' }) and return true when permissionState is 'granted', false when 'denied', and treat 'prompt' as false (or fall back to a non-activating behavior). If navigator.permissions is unavailable (older browsers), fall back to the existing getUserMedia approach as a last resort; ensure you stop any tracks if fallback is used and keep the method signature public async checkMicrophonePermission(): Promise<boolean>.backend/services/transcriptservice.go (1)
973-981: Resolve the staleeloChangeTODO now thatEloChangeis available on the model.
transcript.EloChangeis now populated (when the reordering above is applied); the hardcoded0placeholder can be replaced.♻️ 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, Replace the hardcoded eloChange placeholder with the actual value from the model: when building recentDebates append map, use transcript.EloChange (or convert it to the expected type/format if necessary) instead of 0 so the "eloChange" field reflects the real Elo change from the Transcript struct; update the entry where recentDebates is appended (the map literal that currently sets "eloChange": 0) to pull the value from transcript.EloChange and ensure any type conversions (e.g., to int or float) match the surrounding JSON/consumer expectations.
🤖 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/config/config.prod.sample.yml`:
- Around line 14-15: The JWT secret value jwt.secret should not contain a real
or guessable key; replace the hardcoded "debateai_secret_key_123" with a clear
placeholder (e.g., a descriptive token like "<YOUR_JWT_SECRET_HERE>" or similar)
and add a comment prompting maintainers to set a strong, randomly generated
secret in production (update the jwt.secret entry in the sample to use the
placeholder so consumers must explicitly provide a secure key).
- Around line 4-6: Replace the production sample database URI that currently
points to "mongodb://localhost:27017/debateai" with a clear production
placeholder and an example Atlas template: update the "database:" block and its
"uri:" entry to use a "<REPLACE_ME>" or commented MongoDB Atlas connection
string example (including <username>, <password>, <cluster-url>, and <dbname>
placeholders) so the sample config no longer suggests localhost for production
and guides users to provide proper credentials.
In `@backend/controllers/debatevsbot_controller.go`:
- Around line 270-280: In JudgeDebate, fix the SaveDebateTranscript call by
using the correct locals: replace user.ID and user.Email with the existing
userID and email variables, replace topic with latestDebate.Topic, and replace
messages with req.History; also pass resultStatus (the normalized
"win"/"loss"/"draw"/"pending" string) instead of the raw result JSON, and
capture the error return from services.SaveDebateTranscript (e.g., err :=
services.SaveDebateTranscript(...) ) then handle it (log and/or return) instead
of discarding it.
In `@frontend/src/utils/speechTest.ts`:
- Around line 100-108: When calling this.recognition.start() in the start
routine, ensure this.shouldRestart is cleared if start() throws: move or update
the try/catch so that on any exception you set this.shouldRestart = false before
logging and returning false, so stray onend handlers won't trigger an unwanted
restart; locate the block that calls this.recognition.start() and adjust the
error path to reset this.shouldRestart.
- Around line 100-108: The code sets this.shouldRestart = true before calling
this.recognition.start(), so if start() throws the flag remains true and the
existing onend handler may later restart unexpectedly; fix by only setting
this.shouldRestart to true after a successful start (i.e., move the assignment
to after this.recognition.start() succeeds) or ensure the catch block resets it
(add this.shouldRestart = false in the catch), and verify the onend restart
logic (the onend handler) relies on that flag.
- Around line 62-74: When scheduling the automatic restart in
this.recognition.onend, avoid the race where stop() can prevent
recognition.stop() but not the pending restart: add a class field (e.g.,
restartTimerId) to hold the timeout ID when you call setTimeout in onend and
call clearTimeout(this.restartTimerId) inside stop(); also, inside the scheduled
callback re-check this.shouldRestart before calling this.recognition?.start()
(so update onend’s setTimeout to assign restartTimerId and guard by
shouldRestart), and ensure stop() still sets this.shouldRestart = false and
clears restartTimerId to prevent the pending restart.
- Around line 62-74: The onend handler queues a restart setTimeout but stop()
can be called during that 500ms window and allow recognition to restart; add a
class field (e.g., restartTimerId) to store the timer ID when calling setTimeout
in the onend handler, clearTimeout(this.restartTimerId) and set it to null
inside stop(), and also guard the timeout callback by re-checking
this.shouldRestart before calling this.recognition?.start(); update references
in the onend handler and stop() methods (recognition, shouldRestart,
isListening) accordingly to prevent the race.
In `@frontend/tailwind.config.js`:
- Around line 68-69: The plugins array contains duplicate registrations of
require("tailwindcss-animate") and require("tailwind-scrollbar-hide"), which
causes double-processing and bloated/conflicting CSS; remove the second
occurrences of require("tailwindcss-animate") and
require("tailwind-scrollbar-hide") from the plugins array so each plugin is
required only once (keep the first instances and delete the duplicate
require(...) entries).
---
Outside diff comments:
In `@backend/controllers/debatevsbot_controller.go`:
- Around line 588-597: The call to SaveDebateTranscript in ConcedeDebate is
missing the new eloChange float64 parameter (SaveDebateTranscript now expects 9
args); update the call site in ConcedeDebate to pass a float64 elo change value
(either the computed elo change variable if available or a default like 0.0) as
the penultimate argument so the final nil matches the updated signature for the
optional field; locate the call to services.SaveDebateTranscript and insert the
appropriate float64 value (e.g., eloChange) before the nil.
In `@backend/services/transcriptservice.go`:
- Around line 181-244: The transcript saves call SaveDebateTranscript with a
hardcoded eloChange (0.0) before UpdateRatings runs, so saved transcripts never
include the computed RatingChange; call UpdateRatings(forUser.ID,
againstUser.ID, outcomeFor, time.Now()) first, handle ratingErr and set
debateRecord/opponentRecord, then pass debateRecord.RatingChange and
opponentRecord.RatingChange (instead of 0.0) into the two SaveDebateTranscript
calls and only after that insert the debate records into the "debates"
collection and build ratingSummary.
In `@frontend/src/Pages/DebateRoom.tsx`:
- Around line 812-815: The user panel div (the JSX element whose className
begins with "relative w-full md:w-1/2") is missing the dark-mode utility classes
present on the bot panel; update its className to include dark:bg-zinc-900 and
dark:border-zinc-700 so the user panel matches the bot panel in dark mode,
keeping the existing conditional "animate-glow", bg-white, border, and other
classes intact.
- Around line 288-334: The onend handler currently only sets isRecognizing to
false, so browser-initiated stops (silence/timeouts) aren't auto-restarted; add
a userStoppedRef boolean ref to distinguish user-initiated stops from
browser-initiated ones, update onend (recognitionRef.current.onend) to
auto-restart recognitionRef.current.start() and keep isRecognizing true when
userStoppedRef.current is false, update your stopRecognition function to set
userStoppedRef.current = true before calling recognitionRef.current.stop() and
set isRecognizing false, and update the cleanup in the effect to set
userStoppedRef.current = true before stopping to prevent restarts on unmount;
also ensure startRecognition clears userStoppedRef.current = false and calls
recognitionRef.current.start().
---
Duplicate comments:
In `@frontend/src/Pages/DebateRoom.tsx`:
- Around line 759-762: Previous feedback is addressed: the onClose handler
correctly calls setShowJudgment(false) and then navigate("/startDebate"); no
code changes required — keep the onClose implementation in the component (the
function using setShowJudgment and navigate) as-is.
---
Nitpick comments:
In `@backend/services/transcriptservice.go`:
- Around line 973-981: Replace the hardcoded eloChange placeholder with the
actual value from the model: when building recentDebates append map, use
transcript.EloChange (or convert it to the expected type/format if necessary)
instead of 0 so the "eloChange" field reflects the real Elo change from the
Transcript struct; update the entry where recentDebates is appended (the map
literal that currently sets "eloChange": 0) to pull the value from
transcript.EloChange and ensure any type conversions (e.g., to int or float)
match the surrounding JSON/consumer expectations.
In `@frontend/src/Pages/DebateRoom.tsx`:
- Around line 579-656: The function declaration for judgeDebateResult has
inconsistent leading indentation and there's a missing space in the inner catch
(`}catch`) which affects readability; open frontend/src/Pages/DebateRoom.tsx,
locate the judgeDebateResult function (referencing judgingRef, judgeDebate,
JSON5.parse) and normalize indentation to match surrounding function
declarations (remove the extra leading spaces before "const judgeDebateResult")
and change `}catch` to `} catch` inside the JSON5.parse try/catch block so the
spacing is consistent.
- Around line 323-327: Replace the manual cast in the onerror handler with the
project's SpeechRecognitionErrorEvent type: update
recognitionRef.current.onerror to accept (event: SpeechRecognitionErrorEvent)
and use event.error (or fallback to event) when logging, then call
setIsRecognizing(false); this keeps types consistent with SpeechTest.tsx and
uses the SpeechRecognitionErrorEvent definition from
frontend/src/types/speech-recognition.d.ts.
In `@frontend/src/utils/speechTest.ts`:
- Around line 78-85: The onerror handler in speechTest.ts currently only
disables restarts for permission errors, causing rapid 500ms restart loops for
non-transient errors; update the recognition.onerror implementation (the handler
attached to this.recognition and code that sets this.shouldRestart) to also set
this.shouldRestart = false when event.error is "network" or "audio-capture" (and
keep the existing "not-allowed" / "service-not-allowed" checks), and ensure you
still log the error; this prevents unbounded restart retries triggered by onend
while keeping transient errors restartable.
- Around line 127-135: Replace the current hardware-activating approach in
checkMicrophonePermission() with a Permissions API query: call
navigator.permissions.query({ name: "microphone" }) and return true only when
the returned PermissionStatus.state === "granted", return false for "denied" or
"prompt"; if navigator.permissions is undefined (older browsers), keep a guarded
fallback to the existing getUserMedia try/catch but ensure the Permissions path
is preferred so you don't open the mic unnecessarily. Ensure you reference and
update the checkMicrophonePermission method to implement this logic and handle
errors from permissions.query gracefully.
- Around line 127-135: The checkMicrophonePermission function currently calls
navigator.mediaDevices.getUserMedia which activates hardware; change it to first
use the Permissions API by calling navigator.permissions.query({ name:
'microphone' }) and return true when permissionState is 'granted', false when
'denied', and treat 'prompt' as false (or fall back to a non-activating
behavior). If navigator.permissions is unavailable (older browsers), fall back
to the existing getUserMedia approach as a last resort; ensure you stop any
tracks if fallback is used and keep the method signature public async
checkMicrophonePermission(): Promise<boolean>.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/utils/speechTest.ts`:
- Around line 77-81: When this.recognition?.start() throws inside the restart
block, the instance state is left inconsistent; update the catch to reset the
same state as the start() method's catch by setting shouldRestart = false,
restartTimeout = null and isListening = false before logging the error so stray
onend events won't queue another restart; locate the restart call around
this.recognition?.start() and mirror the state-reset behavior from the start()
method's catch handler.
- Around line 86-98: Update the recognition.onerror handler: change the event
typing from any to SpeechRecognitionErrorEvent, and remove the "aborted" error
from the set of errors that flip this.shouldRestart to false (keep not-allowed,
service-not-allowed, audio-capture); the stop() path already sets shouldRestart
= false before calling recognition.stop(), so avoid treating browser-initiated
aborts as permanent fatal errors—let the normal restart logic handle them and
only treat explicit fatal errors as shown in the recognition.onerror handler.
- Around line 107-126: The race occurs because shouldRestart is set after await
checkMicrophonePermission(), allowing stop() to race; move the assignment of
shouldRestart = true to before calling checkMicrophonePermission() (i.e., before
await this.checkMicrophonePermission()) and then immediately re-check
shouldRestart after the await resolves before calling this.recognition.start();
also on the error path ensure shouldRestart is reset to false and isListening
remains false; reference: checkMicrophonePermission(), shouldRestart, stop(),
recognition.start(), isListening.
…anted restart loop
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/utils/speechTest.ts (1)
62-72:restartTimeoutnot cleared after the timeout callback fires successfully.When
this.recognition?.start()succeeds at Line 66,restartTimeoutretains the stale timer ID. The failure path (Lines 69–70) correctly nulls it, but the success path does not. WhileclearTimeouton a stale ID is a harmless no-op andshouldRestartis the primary guard, nulling it keeps the state consistent and signals clearly that no restart is pending.♻️ Proposed fix
this.restartTimeout = window.setTimeout(() => { if (!this.shouldRestart) return; + this.restartTimeout = null; try { this.recognition?.start(); } catch (error) { console.error("Restart failed:", error); this.shouldRestart = false; - this.restartTimeout = null; } }, 500);🤖 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 62 - 72, The timeout handler for this.restartTimeout does not clear the stored timer ID on the success path—inside the try block where you call this.recognition?.start(), after a successful start set this.restartTimeout = null (and optionally clearTimeout(this.restartTimeout) before nulling if you prefer) so the state is consistent; leave the failure path that sets this.shouldRestart = false and this.restartTimeout = null intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@frontend/src/utils/speechTest.ts`:
- Around line 76-78: The onerror handler currently types its parameter as Event
and casts to SpeechRecognitionErrorEvent; change the handler signature to accept
(ev: SpeechRecognitionErrorEvent) directly on this.recognition.onerror to enable
compile-time checking, i.e., replace the (ev: Event) cast pattern in the onerror
assignment with (ev: SpeechRecognitionErrorEvent) => { ... }; if you had to use
the cast due to webkitSpeechRecognition typings, add a short inline comment
referencing that specific TS/typing limitation and why the cast is kept so
future readers understand the workaround.
---
Nitpick comments:
In `@frontend/src/utils/speechTest.ts`:
- Around line 62-72: The timeout handler for this.restartTimeout does not clear
the stored timer ID on the success path—inside the try block where you call
this.recognition?.start(), after a successful start set this.restartTimeout =
null (and optionally clearTimeout(this.restartTimeout) before nulling if you
prefer) so the state is consistent; leave the failure path that sets
this.shouldRestart = false and this.restartTimeout = null intact.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/src/utils/speechTest.ts (2)
76-89:onerrorparameter typed asEventthen manually cast — can useSpeechRecognitionErrorEventdirectly.Since
this.recognitionis typed asSpeechRecognition, theonerrorhandler already expectsSpeechRecognitionErrorEvent. The intermediateEventtype and manual cast on line 77 are unnecessary.♻️ Proposed fix
- this.recognition.onerror = (ev: Event) => { - const event = ev as SpeechRecognitionErrorEvent; - - console.error("Speech error:", event.error, event.message); + this.recognition.onerror = (event: SpeechRecognitionErrorEvent) => { + console.error("Speech error:", event.error, event.message);🤖 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 76 - 89, The onerror handler currently types the parameter as Event and then casts it to SpeechRecognitionErrorEvent; change the onerror signature to accept a SpeechRecognitionErrorEvent directly (i.e., in the this.recognition.onerror assignment) so you can remove the manual cast and use event.error and event.message with correct typing in the handler body; update any related imports/types if needed to ensure SpeechRecognitionErrorEvent is available in the scope where this.recognition.onerror is defined.
62-72:restartTimeoutnot nulled after the callback succeeds — stale timer ID remains.When the timeout callback fires and
recognition.start()succeeds (line 66),restartTimeoutstill holds the (now-expired) timer ID. The error path correctly nulls it (line 70), but the happy path doesn't. This is mostly a cleanliness concern, but it also meansstop()will callclearTimeouton an already-fired timer (harmless no-op) and then null it, rather than seeing it's already null.♻️ Proposed fix — null the timer at the top of the callback
this.restartTimeout = window.setTimeout(() => { + this.restartTimeout = null; if (!this.shouldRestart) return; try { this.recognition?.start(); } catch (error) { console.error("Restart failed:", error); this.shouldRestart = false; - this.restartTimeout = null; } }, 500);🤖 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 62 - 72, In the timeout callback that restarts recognition (the function that calls this.recognition?.start()), clear the stored timer ID immediately when the callback begins by setting this.restartTimeout = null so the property no longer holds a stale ID; keep the existing error handling that sets this.shouldRestart = false and leaves restartTimeout null on failure. Locate the restart logic using symbols this.restartTimeout, this.shouldRestart and this.recognition.start() and ensure the assignment happens before calling recognition.start() (or right after successful start) so stop() sees a null timer regardless of success or failure.
🤖 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/utils/speechTest.ts`:
- Around line 93-123: The start() method must clear any pending restartTimeout
to avoid a stale timeout triggering a second recognition.start() and breaking
auto-restart; at the top of start() (before permission checks and before setting
shouldRestart) cancel the pending timeout (clearTimeout(this.restartTimeout))
and reset the field (e.g., this.restartTimeout = undefined/null) so onend’s
500ms callback cannot fire after a manual start; keep references to
this.restartTimeout, this.shouldRestart, this.recognition.start(), and the onend
restart flow when making the change.
---
Duplicate comments:
In `@frontend/src/utils/speechTest.ts`:
- Around line 65-71: Ensure that when this.recognition?.start() throws, the
catch block resets the restart guards by setting shouldRestart = false and
restartTimeout = null so no further onend-driven restarts are queued; locate the
start call in the speech restart logic (the this.recognition?.start()
invocation) and verify the catch block performs those resets and logs the error
(console.error("Restart failed:", error))—if missing, add the resets and keep
the error log.
---
Nitpick comments:
In `@frontend/src/utils/speechTest.ts`:
- Around line 76-89: The onerror handler currently types the parameter as Event
and then casts it to SpeechRecognitionErrorEvent; change the onerror signature
to accept a SpeechRecognitionErrorEvent directly (i.e., in the
this.recognition.onerror assignment) so you can remove the manual cast and use
event.error and event.message with correct typing in the handler body; update
any related imports/types if needed to ensure SpeechRecognitionErrorEvent is
available in the scope where this.recognition.onerror is defined.
- Around line 62-72: In the timeout callback that restarts recognition (the
function that calls this.recognition?.start()), clear the stored timer ID
immediately when the callback begins by setting this.restartTimeout = null so
the property no longer holds a stale ID; keep the existing error handling that
sets this.shouldRestart = false and leaves restartTimeout null on failure.
Locate the restart logic using symbols this.restartTimeout, this.shouldRestart
and this.recognition.start() and ensure the assignment happens before calling
recognition.start() (or right after successful start) so stop() sees a null
timer regardless of success or failure.
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.
When using the Web Speech API with recognition.continuous = true, speech recognition still stops automatically after a short period of silence or inactivity. This breaks the expected continuous listening behavior, as recognition should remain active until manually stopped by the application.
Steps to reproduce:
Expected behavior:
Recognition should continue listening until manually stopped.
Actual behavior:
Recognition stops automatically due to silence, timeout, or internal browser reset.
Environment:
Workaround:
Manually restarting recognition inside the onend event.
Impact:
Critical – breaks continuous listening functionality.
close #329
Summary by CodeRabbit
New Features
Improvements
Chores