Fix bot name flickering during session initialization - #304
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthrough
ChangesVoice Chat Query and Playback Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.42.2)src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.jsThanks 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js (1)
1502-1525:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse compound guard to check if intro query is actively fetching.
In TanStack Query v5, a disabled query with no cached data reports
isPending: trueeven thoughisFetchingis false. Using only!isIntroMessagePendinghere blocks speaker auto-play even when the intro query is merely disabled, not actively loading. Replace with the compound guardisIntroQueryEnabled && isIntroMessagePendingthat's already established in the loader (line 2279).Three instances need updating (lines 1502, 1513, 1517):
🔧 Suggested change
useEffect(() => { + const isIntroFetchInFlight = isIntroQueryEnabled && isIntroMessagePending + let shouldPlay = false if (showFileInput) { shouldPlay = true - } else if ((noStoryFound || noStoryFound === null) && !isIntroMessagePending && !isLoading && !endStoryMutation.isPending) { + } else if ((noStoryFound || noStoryFound === null) && !isIntroFetchInFlight && !isLoading && !endStoryMutation.isPending) { const currentFlow = storageFlow if (currentFlow) { if (chatHistory.length > 0) { if (isStreamingComplete && chatHistory[chatHistory.length - 1]?.source === "bot") { shouldPlay = true } } else { shouldPlay = true } - } else if (chatHistory && chatHistory.length > 0 && chatHistory[chatHistory.length - 1]?.source === "bot" && !isIntroMessagePending && !isLoading && !endStoryMutation.isPending) { + } else if (chatHistory && chatHistory.length > 0 && chatHistory[chatHistory.length - 1]?.source === "bot" && !isIntroFetchInFlight && !isLoading && !endStoryMutation.isPending) { shouldPlay = true } } - if (isStreamingComplete && shouldPlay && !endStoryMutation.isPending && !isLoading && !isPdfDownloading && isMute && acceptedTnc && acceptedTnc !== "ONGOING" && !isIntroMessagePending) { + if (isStreamingComplete && shouldPlay && !endStoryMutation.isPending && !isLoading && !isPdfDownloading && isMute && acceptedTnc && acceptedTnc !== "ONGOING" && !isIntroFetchInFlight) { const speakerButtons = document.querySelectorAll(".button-11.button-3") const lastSpeakerButton = speakerButtons[speakerButtons.length - 1]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js` around lines 1502 - 1525, The effect currently uses the simple guard !isIntroMessagePending which treats a disabled intro query as "pending"; replace each usage so the code checks that the intro query is actively fetching by substituting the compound guard !(isIntroQueryEnabled && isIntroMessagePending) (i.e., use isIntroQueryEnabled && isIntroMessagePending to mean actively fetching). Update all three occurrences inside the useEffect where !isIntroMessagePending appears (the conditions that decide shouldPlay and the final autoplay guard) to use !(isIntroQueryEnabled && isIntroMessagePending) instead, referencing the existing variables isIntroMessagePending and isIntroQueryEnabled.
🧹 Nitpick comments (1)
src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js (1)
824-829: ⚡ Quick winUse
default_nameas the post-load fallback.Once the intro payload has resolved,
namecan still be empty whiledefault_nameis available on the same response. Falling back todefault_namehere avoids rendering a blank bot label without reintroducing the pre-load flicker.💡 Suggested change
- const botName = introMessageData[0]?.name || "" + const botName = introMessageData[0]?.name || introMessageData[0]?.default_name || ""🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js` around lines 824 - 829, The code sets botName from introMessageData[0]?.name which can be empty even when introMessageData[0]?.default_name exists; update the assignment and the display setter to use default_name as the post-load fallback (i.e., derive botName and the value passed to setBotNameToDisplay from introMessageData[0]?.name || introMessageData[0]?.default_name || ""), keep setDefaultBotName(introMessageData[0]?.default_name) as-is, and ensure you reference introMessageData, setBotName, setDefaultBotName, and setBotNameToDisplay when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js`:
- Around line 1502-1525: The effect currently uses the simple guard
!isIntroMessagePending which treats a disabled intro query as "pending"; replace
each usage so the code checks that the intro query is actively fetching by
substituting the compound guard !(isIntroQueryEnabled && isIntroMessagePending)
(i.e., use isIntroQueryEnabled && isIntroMessagePending to mean actively
fetching). Update all three occurrences inside the useEffect where
!isIntroMessagePending appears (the conditions that decide shouldPlay and the
final autoplay guard) to use !(isIntroQueryEnabled && isIntroMessagePending)
instead, referencing the existing variables isIntroMessagePending and
isIntroQueryEnabled.
---
Nitpick comments:
In `@src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js`:
- Around line 824-829: The code sets botName from introMessageData[0]?.name
which can be empty even when introMessageData[0]?.default_name exists; update
the assignment and the display setter to use default_name as the post-load
fallback (i.e., derive botName and the value passed to setBotNameToDisplay from
introMessageData[0]?.name || introMessageData[0]?.default_name || ""), keep
setDefaultBotName(introMessageData[0]?.default_name) as-is, and ensure you
reference introMessageData, setBotName, setDefaultBotName, and
setBotNameToDisplay when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e6e51cd-99ab-4c99-8224-889c317705aa
📒 Files selected for processing (1)
src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js
Summary
Testing
Summary by CodeRabbit
Bug Fixes
Refactor