-
Notifications
You must be signed in to change notification settings - Fork 0
Development #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Development #34
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request is a development release (version 1.3.1) that includes bug fixes, error handling improvements, and refactoring to resolve circular import dependencies. The changes focus on improving Twitch bot lifecycle management, YouTube authentication error handling, and cleaning up the module structure.
Key changes:
- Refactored queue imports from
appmodule tomodules.queue_managerto eliminate circular dependencies - Enhanced YouTube listener with specific
RefreshErrorhandling and improved error messages - Improved Twitch bot lifecycle with duplicate start prevention and cleaner shutdown handling
- Merged settings defaults into database settings for backward compatibility with new settings
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| deployment/ChatYapper.wxs | Version bump to 1.3.1 |
| backend/version.py | Version bump to 1.3.1 |
| frontend/src/components/settings/GeneralSettings.jsx | Added undefined check for parallelMessageLimit |
| backend/tests/test_clearchat.py | Added credential check and skip decorator for TwitchBot tests |
| backend/routers/system.py | Refactored parallel_message_queue import to use queue_manager module |
| backend/routers/config_backup.py | Refactored avatar_message_queue import to use queue_manager module |
| backend/routers/avatars.py | Refactored queue imports to use queue_manager module; minor formatting fix |
| backend/modules/youtube_listener.py | Added RefreshError exception handling with clearer error messages |
| backend/modules/persistent_data.py | Enhanced get_settings() to merge database settings with defaults |
| backend/app.py | Improved Twitch bot lifecycle with duplicate prevention, custom exception handler, and additional logging |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
|
||
| # Event router to handle different event types | ||
| async def route_twitch_event(e): | ||
| logger.info(f"[EVENT ROUTER] Received event: type={e.get('type')}, user={e.get('user')}, text={e.get('text', '')[:50]}") |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug logging statement appears to be left in production code. The "[EVENT ROUTER]" prefix and verbose logging suggest this was added for debugging. Consider removing this line or reducing its log level to logger.debug() if it's intended for troubleshooting purposes only.
| logger.info(f"[EVENT ROUTER] Received event: type={e.get('type')}, user={e.get('user')}, text={e.get('text', '')[:50]}") | |
| logger.debug(f"[EVENT ROUTER] Received event: type={e.get('type')}, user={e.get('user')}, text={e.get('text', '')[:50]}") |
|
|
||
| # Event router to handle different event types | ||
| async def route_twitch_event(e): | ||
| logger.info(f"[EVENT ROUTER] Received event: type={e.get('type')}, user={e.get('user')}, text={e.get('text', '')[:50]}") |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate debug logging statement. This appears to be the same debug log as line 521 in the restart function. Consider removing this line or reducing its log level to logger.debug() if it's intended for troubleshooting purposes only.
| logger.info(f"[EVENT ROUTER] Received event: type={e.get('type')}, user={e.get('user')}, text={e.get('text', '')[:50]}") | |
| logger.debug(f"[EVENT ROUTER] Received event: type={e.get('type')}, user={e.get('user')}, text={e.get('text', '')[:50]}") |
✅ 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-c19975a
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-c19975aAccess at: http://localhost:8069 The Docker image will be published to the GitHub Container Registry when merged to |
No description provided.