Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
…4b50-93c4-0e11b13594f4 Clarify error handler ordering is correct - no changes needed
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
Co-authored-by: pladisdev <127021507+pladisdev@users.noreply.github.com>
…47b6-b204-5bf8892748d3 Extract magic numbers to named constants in audio playback code
…4606-8149-97ceaa22873d Use platform.system() instead of sys.platform for OS detection
Extract duplicated hex opacity calculation to utility function
Extract avatar active offset magic number into configurable setting
Fix audio reference cleanup on play() failure in popup mode
Fix race condition in popup avatar lifecycle
Fix memory leaks in audio error handlers
…into development
…into development
There was a problem hiding this comment.
Pull request overview
This pull request introduces several user-facing improvements and backend reliability enhancements for the Chat Yapper TTS application. The changes focus on improving the user experience with better notifications, concurrent message limiting, and enhanced OAuth token management for Twitch and YouTube integrations.
Key Changes:
- Toast notification system for user feedback on browser autoplay restrictions and authentication issues
- Parallel message limiting with configurable yapper limits and queue overflow handling
- Enhanced authentication with automatic OAuth token refresh for Twitch and YouTube
- UI improvements including username display in chat bubbles, text size adjustment, and quick status view
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/pages/YappersPage.jsx |
Adds toast notifications for autoplay errors, username display in chat bubbles, and user interaction tracking |
frontend/src/pages/SettingsPage.jsx |
Introduces quick status view sidebar, global TTS toggle button, and improved layout |
frontend/src/index.css |
Implements text size scaling system with CSS custom properties |
frontend/src/hooks/useToast.js |
New custom hook for toast notification state management |
frontend/src/components/ui/toast.jsx |
Toast notification UI component with auto-close functionality |
frontend/src/components/ui/badge.jsx |
New badge component for status indicators |
frontend/src/components/settings/TwitchIntegration.jsx |
Adds auth error detection/handling, redeem filter management, and connection testing |
frontend/src/components/settings/QuickStatusView.jsx |
New status sidebar showing system state at a glance |
frontend/src/components/settings/GeneralSettings.jsx |
Adds yapper limit controls, font size toggle, and queue overflow settings |
frontend/src/components/settings/ChatBubbleSettings.jsx |
Adds username display toggle and color customization |
frontend/src/components/VoiceManager.jsx |
Adds callback for voice changes to update parent components |
backend/app.py |
Implements parallel message limiting with audio duration-based counter, OAuth token refresh, and comprehensive auth error handling |
backend/routers/auth.py |
Adds automatic token refresh for Twitch/YouTube, auth error endpoints, and connection testing |
backend/routers/system.py |
Adds debug endpoints for TTS state inspection and parallel limit testing |
backend/modules/youtube_listener.py |
Improves error handling for 401 auth errors with automatic credential refresh |
backend/modules/twitch_listener.py |
Adds TwitchIO 3.x credential validation |
backend/modules/settings_defaults.json |
Adds defaults for new settings (text size, parallel limits, username display, redeem filter) |
backend/tests/conftest.py |
Adds mock Twitch credentials for TwitchIO 3.x compatibility in tests |
deployment/build.py |
Adds TwitchIO version checking and credential validation warnings |
README.md |
Updates changelog with v1.2.2 features |
Comments suppressed due to low confidence (5)
frontend/src/pages/YappersPage.jsx:1491
- The condition check
chatMessages[id].usernameon line 1491 could fail ifchatMessages[id]is undefined. Consider using optional chaining for safety:
(settings?.bubbleShowUsername ?? true) && chatMessages[id]?.usernameThis prevents runtime errors when accessing properties of potentially undefined objects.
{(settings?.bubbleShowUsername ?? true) && chatMessages[id].username && (
frontend/src/pages/YappersPage.jsx:1514
- Similar issue on line 1514 - the check needs optional chaining to handle cases where
chatMessages[id]might be undefined:
paddingTop: (settings?.bubbleShowUsername ?? true) && chatMessages[id]?.username ? '4px' : '8px'This prevents errors when the chat message object doesn't exist.
paddingTop: (settings?.bubbleShowUsername ?? true) && chatMessages[id].username ? '4px' : '8px'
frontend/src/pages/YappersPage.jsx:1485
- Similar to the grid mode, the condition check for
chatMessages[id]on line 1485 should also check for the message property since the data structure changed from string to object:
chatMessages[id]?.messageWithout this fix, the chat bubble won't render properly in popup mode when there's an empty message object.
{settings?.chatBubblesEnabled !== false && chatMessages[id] && (
frontend/src/pages/YappersPage.jsx:1136
- The condition check for
chatMessages[slot.id]on line 1136 will fail whenchatMessages[slot.id]is falsy (undefined, null, or an empty object). SincechatMessageswas changed from storing strings to storing objects with{ message, username }structure, this check should be updated to:
chatMessages[slot.id]?.messageThis ensures the check properly validates that a message exists in the object structure.
{(settings?.bubbleShowUsername ?? true) && chatMessages[slot.id].username && (
backend/app.py:654
- Variable result is not used.
result = task.result()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if filtered_text != original_text: | ||
| logger.info(f"Text after filtering: '{filtered_text}'") | ||
| await process_tts_message(evt_filtered) | ||
| raise |
There was a problem hiding this comment.
The raise statement on line 1645 appears to be misplaced. This line is in the middle of the handle_event function, after checking if filtered text differs from original text, but it's not inside any exception handler. This will cause an unhandled exception to be raised whenever text filtering modifies the message.
The code should either:
- Remove this
raisestatement if it was added by mistake - Replace it with appropriate logic (e.g., logging or continuing execution)
| raise |
| - **New Features:** | ||
| - Quick status view | ||
| - Limit concurrent TTS messages | ||
| - Some more twitch fixes and improved notifcations |
There was a problem hiding this comment.
Spelling error: "notifcations" should be "notifications"
| - Some more twitch fixes and improved notifcations | |
| - Some more twitch fixes and improved notifications |
| await asyncio.sleep(decrement_delay) | ||
| decrement_tts_count() | ||
| # Process any queued messages now that a slot is free | ||
| process_parallel_message_queue() |
There was a problem hiding this comment.
The decrement_after_audio function is created as a fire-and-forget task without any error handling. If an exception occurs during the sleep or when calling decrement_tts_count() or process_parallel_message_queue(), it will be silently swallowed.
Consider wrapping the async function with try-except:
async def decrement_after_audio():
try:
await asyncio.sleep(decrement_delay)
decrement_tts_count()
process_parallel_message_queue()
except Exception as e:
logger.error(f"Error in decrement_after_audio: {e}", exc_info=True)
# Still decrement to prevent counter drift
decrement_tts_count()| await asyncio.sleep(decrement_delay) | |
| decrement_tts_count() | |
| # Process any queued messages now that a slot is free | |
| process_parallel_message_queue() | |
| try: | |
| await asyncio.sleep(decrement_delay) | |
| decrement_tts_count() | |
| # Process any queued messages now that a slot is free | |
| process_parallel_message_queue() | |
| except Exception as e: | |
| logger.error(f"Error in decrement_after_audio: {e}", exc_info=True) | |
| # Still decrement to prevent counter drift | |
| decrement_tts_count() |
| try: | ||
| await process_tts_message(evt) | ||
| return True | ||
| except Exception as e: | ||
| # If processing failed, decrement counter | ||
| decrement_tts_count() | ||
| logger.error(f"TTS processing failed for {username}: {e}", exc_info=True) | ||
| return False |
There was a problem hiding this comment.
There's a potential race condition in the parallel limiting logic. The counter is incremented in check_parallel_limits_and_process (line 1599), but if the TTS processing fails early (before the async task completes), the decrement happens in the exception handler. However, if process_tts_message is called directly from check_parallel_limits_and_process and fails synchronously before reaching the fire-and-forget decrement task, the counter could be decremented twice: once in the exception handler and once by the scheduled task.
Consider tracking the scheduled decrement tasks so they can be cancelled if an early error occurs, or use a flag to prevent double-decrement.
| try: | |
| await process_tts_message(evt) | |
| return True | |
| except Exception as e: | |
| # If processing failed, decrement counter | |
| decrement_tts_count() | |
| logger.error(f"TTS processing failed for {username}: {e}", exc_info=True) | |
| return False | |
| decremented = False | |
| try: | |
| await process_tts_message(evt) | |
| except Exception as e: | |
| # If processing failed, decrement counter | |
| decrement_tts_count() | |
| decremented = True | |
| logger.error(f"TTS processing failed for {username}: {e}", exc_info=True) | |
| return False | |
| # Only decrement if not already done in exception | |
| if not decremented: | |
| decrement_tts_count() | |
| return True |
| const [toasts, setToasts] = useState([]) | ||
|
|
||
| const showToast = useCallback((message, type = 'info', options = {}) => { | ||
| const id = Date.now() + Math.random() |
There was a problem hiding this comment.
The toast ID generation using Date.now() + Math.random() on line 7 could potentially create collisions if multiple toasts are created in rapid succession within the same millisecond. While unlikely, a more robust approach would be to use a counter or UUID.
Consider:
const id = `${Date.now()}-${Math.random()}`or use a counter-based approach for guaranteed uniqueness.
| const id = Date.now() + Math.random() | |
| const id = crypto.randomUUID() |
| @@ -0,0 +1,109 @@ | |||
| import React from 'react' | |||
| import { Card, CardContent, CardHeader, CardTitle } from '../ui/card' | |||
There was a problem hiding this comment.
Unused imports CardHeader, CardTitle.
| import { Card, CardContent, CardHeader, CardTitle } from '../ui/card' | |
| import { Card, CardContent } from '../ui/card' |
| Users, | ||
| CheckCircle2, | ||
| XCircle |
There was a problem hiding this comment.
Unused imports CheckCircle2, XCircle.
| Users, | |
| CheckCircle2, | |
| XCircle | |
| Users |
| @@ -0,0 +1,63 @@ | |||
| import React, { useState, useEffect } from 'react' | |||
There was a problem hiding this comment.
Unused import useState.
| import React, { useState, useEffect } from 'react' | |
| import React, { useEffect } from 'react' |
| </div> | ||
|
|
||
| {/* Global Stop TTS Button */} | ||
| {settings && ( |
There was a problem hiding this comment.
This use of variable 'settings' always evaluates to true.
| if twitch_refresh_attempted: | ||
| logger.info("Resetting token refresh attempt tracking due to new WebSocket connection (page refresh)") | ||
| twitch_refresh_attempted = False | ||
| youtube_refresh_attempted = False |
There was a problem hiding this comment.
Variable youtube_refresh_attempted is not used.
| youtube_refresh_attempted = False |
✅ Windows Build SuccessfulExecutable: Build Status
Download the artifacts from the workflow run to test before merging. Once merged to |
🐳 Docker Image Built SuccessfullyImage: Test this PR with Docker:docker pull ghcr.io/pladisdev/chat-yapper:pr-78504eb
docker run -d \
--name chat-yapper-pr \
-p 8069:8008 \
-e TWITCH_CLIENT_ID=your_id \
-e TWITCH_CLIENT_SECRET=your_secret \
ghcr.io/pladisdev/chat-yapper:pr-78504ebAccess at: http://localhost:8069 The Docker image will be published to the GitHub Container Registry when merged to |
No description provided.