Update useChatWebhook to initialize reconnectCount at 1 and adjust re… - #213
Conversation
…connect logic. Enhance ShikshalokamVoiceChat by adding homepage visibility condition and updating WebSocket retry attempts from environment variable.
WalkthroughThe changes refactor WebSocket reconnection logic in the hook by reorganizing conditional checks into an if/else-if chain and adjusting the initial reconnect counter. The voice chat page adds reconnectAttempts configuration, modifies chat history cleanup behavior, and adjusts language selection triggers. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/hooks/useChatWebhook.js(2 hunks)src/pages/ShikshalokamVoiceChat/voice-chat.js(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/ShikshalokamVoiceChat/voice-chat.js (2)
src/hooks/useSmartChatStorage.js (1)
chatHistory(4-4)src/pages/UnifiedChat/UnifiedVoiceBasedChat.jsx (1)
chatHistory(41-41)
🔇 Additional comments (4)
src/hooks/useChatWebhook.js (1)
49-56: LGTM! Correctly prevents double execution of final reconnect logic.The change to
else ifensures thatonFinalReconnectAttemptis only called when the reconnect limit has been reached, and not during the sameoncloseevent where a reconnection is attempted. This prevents potential race conditions or duplicate final-attempt notifications.src/pages/ShikshalokamVoiceChat/voice-chat.js (3)
193-196: LGTM! Appropriate homepage display after reconnection cleanup.When the WebSocket reconnection fails and chat history is cleaned, showing the homepage if only the intro message remains (length === 1) provides a clean user experience and prevents displaying a partial conversation state.
303-303: LGTM! WebSocket retry attempts now configurable via environment variable.Sourcing
reconnectAttemptsfromenv.WEBSOCKET_RETRY_NUM()makes the retry behavior configurable per environment. The variable is properly defined insrc/utils/env.jswith a sensible default value of2and correctly parses theREACT_APP_WEBSOCKET_RETRY_NUMenvironment variable. Usage is consistent across lines 303 and 309.
1021-1021: Verify the language selection trigger condition change.The condition was changed from
chatHistory?.length <= 1to!chatHistory.length, which significantly alters when language selection logic executes:
- Old behavior: Triggers when chatHistory has 0 or 1 items (empty or with intro message)
- New behavior: Triggers only when chatHistory is completely empty (0 items)
This means language selection will no longer re-trigger when the intro message is present. Confirm whether this aligns with the intended UX—specifically, that users who have already received the intro message should not have their language selection reset or re-initialized.
|
|
||
| const ws = useRef(null) | ||
| const reconnectCount = useRef(0) | ||
| const reconnectCount = useRef(1) |
There was a problem hiding this comment.
Inconsistent reconnectCount initialization.
Line 7 initializes reconnectCount.current to 1, but line 83 in the useEffect immediately resets it to 0 on mount. This makes the initial value of 1 effectively unused and creates confusion about the intended behavior.
If the intention is to start at 1 (as per the PR objectives), remove the reset on line 83:
useEffect(() => {
- reconnectCount.current = 0
if (autoConnect) connect()Otherwise, revert line 7 to initialize at 0 for consistency:
-const reconnectCount = useRef(1)
+const reconnectCount = useRef(0)Also applies to: 83-83
🤖 Prompt for AI Agents
In src/hooks/useChatWebhook.js around lines 7 and 83, the ref reconnectCount is
initialized to 1 at line 7 but immediately reset to 0 on mount at line 83,
creating inconsistency; if the intended behavior is to start at 1 (per PR
objectives) remove the reset at line 83 so reconnectCount.current remains 1 on
mount, otherwise change the initialization at line 7 to useRef(0) to match the
reset and keep the existing line 83 as-is.
…connect logic. Enhance ShikshalokamVoiceChat by adding homepage visibility condition and updating WebSocket retry attempts from environment variable.
Summary by CodeRabbit
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.