-
-
Notifications
You must be signed in to change notification settings - Fork 400
Wake Word begins conversation #133
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces conversation management capabilities to the Glados assistant by adding timeout tracking and logic that bypasses wake word detection during active conversations. It also updates the version control patterns in the Changes
Sequence Diagram(s)sequenceDiagram
participant Audio
participant Glados
participant Logger
Audio->>Glados: Audio input detected
Glados->>Glados: Call is_in_conversation()
alt Within timeout period
Glados->>Logger: Log conversation bypass message
Glados-->>Audio: Process audio without wake word check
else Timeout elapsed
Glados->>Glados: Perform wake word detection
Glados->>Logger: Log detection process
end
sequenceDiagram
participant User
participant Main
participant ConfigSetup
participant FileSystem
User->>Main: Start installation
Main->>ConfigSetup: Call setup_config_files()
ConfigSetup->>FileSystem: Check if config file exists
alt File exists
ConfigSetup-->>Main: Log "Config file already exists"
else File missing
FileSystem->>ConfigSetup: Copy example file to target location
ConfigSetup-->>Main: Log "Config file created"
end
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@bestestdev Is the timeout fixed from the time of the wake word, or, once a conversation is started it is maintained until there is a pause on n-seconds? Another option would be to look for a key phrase while in the conversation, like "stop listening". Once we have function calling implemented, we can probably have the agent call a function to modify its own state (i.e. turning on and off wake-word functionality). Anyway, looking forward to the PR, and thanks for joining in on development! |
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
configs/glados_config.yaml(1 hunks)src/glados/engine.py(10 hunks)
🔇 Additional comments (10)
src/glados/engine.py (7)
59-59: New timeout parameter for conversation continuation.The new
conversation_timeoutparameter on theGladosConfigclass adds support for continuing conversations without requiring the wake word each time, which aligns with the PR objectives.
116-116: Class-level constant for conversation timeout.The
CONVERSATION_TIMEOUTconstant on theGladosclass properly establishes a default value that can be overridden by configuration.
196-196: New timestamp tracking for conversation state.The
last_response_timeattribute will track when the assistant last spoke, which is essential for the conversation timeout feature.
284-300: Configuration integration for conversation timeout.The
from_configmethod now properly sets the conversation timeout from the configuration file, allowing for easy customization.
454-466: Well-implemented conversation state detection.The
is_in_conversationmethod effectively determines if the current time is within the conversation timeout period after the assistant's last response, with appropriate debug logging.
519-528: Improved wake word detection logic.The modified code now intelligently bypasses the wake word check when in an active conversation, which creates a more natural interaction flow while still requiring the wake word for initial activation.
873-873: Proper tracking of conversation state.Both locations correctly update the
last_response_timewhen the assistant finishes speaking or is interrupted, which is crucial for the conversation timeout feature.Also applies to: 897-897
configs/glados_config.yaml (3)
6-6: New wake word: "munchkin".This change establishes "munchkin" as the wake word, aligning with the new personality direction.
8-9: New announcement and conversation timeout setting.The announcement has been updated to match the new "Meebo" personality, and a 15-second conversation timeout has been added. This timeout setting differs from the 10-second default in the code but will properly override it.
2-4:Details
❓ Verification inconclusive
Server and model configuration changes.
The configuration has been updated to point to a specific IP address rather than localhost, use the "qwen2.5:7b" model, and clear the API key.
Verify that the server at 192.168.128.3 is accessible and that the qwen2.5:7b model is available on that server. Also confirm if an empty API key is sufficient for authentication.
🏁 Script executed:
#!/bin/bash # Check if the server is reachable curl -s -o /dev/null -w "%{http_code}" http://192.168.128.3:11434/api/models # Check if the model exists on the server curl -s http://192.168.128.3:11434/api/models | grep -i "qwen2.5:7b"Length of output: 82
Action Required: Manual Verification of Server Connectivity and Model Availability
The configuration in
configs/glados_config.yamlnow specifies the server as192.168.128.3:11434with the modelqwen2.5:7band an empty API key. However, the initial connectivity check returned an HTTP status code of "000", which means there was no response from the server. This suggests one or more of the following:
- The server at
192.168.128.3might be unreachable from the test environment.- The endpoint
/api/modelsmay not be operational or may require additional network configurations.- It remains unclear whether an empty API key is acceptable for authentication.
Please perform a manual verification to ensure that:
- The server at
192.168.128.3:11434is online and accessible.- The endpoint
/api/modelsproperly lists theqwen2.5:7bmodel.- The authentication mechanism works correctly with an empty API key if that is the intended behavior.
configs/glados_config.yaml
Outdated
| - assistant: "Yes, you should definitely try to be more interesting. Could I suggest juggling handguns?" | ||
| - user: "What game should I play?" | ||
| - assistant: "Russian Roulette. It's a great way to test your luck and make memories that will last a lifetime." | ||
| - system: "You are Meebo, a cute, childlike artificial intelligence whose purpose is to be a fun pet for the family. You are capable of controlling the house via Home Assistant, When given a request, always comply efficiently. However, your responses should be childlike and short, and not at all like a normal AI. Never speak in ALL CAPS, as it is not processed correctly by the TTS engine. Only make short replies, with only a few words." |
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.
💡 Verification agent
🧩 Analysis chain
Complete personality change from GLaDOS to Meebo.
The personality has been completely reimagined from a sarcastic AI to a cute, childlike assistant called Meebo. The prompt includes important TTS considerations (avoiding ALL CAPS) and instructs the AI to keep responses short and childlike.
🌐 Web query:
Is "Meebo" a recognizable character or brand name that might have trademark implications?
💡 Result:
"Meebo" is both a recognizable brand name and a term with trademark implications. Founded as an instant messaging and social networking platform, Meebo was acquired by Google in 2012 and later shut down[2][4]. The name was intentionally crafted to be short, memorable, and devoid of prior meaning, with a preference for the letter "M" (possibly referencing "messenger")[2][4]. While the service is no longer active, its trademark status persists:
- Trademarks in India: Multiple "MEEBO" trademarks are registered in India across various classes, including applications dating back to 2011 and as recent as 2022[3].
- Legal relevance: Meebo has been cited in legal discussions about intellectual property risks, including trademark infringement and intermediary liability in user-generated content platforms[6][8].
The name’s uniqueness and prior commercial use mean unauthorized adoption could risk infringement claims, particularly in jurisdictions where active registrations exist. For example, using "Meebo" for tech-related services might conflict with existing trademarks in India[3]. Additionally, historical critiques of the name’s linguistic quirks (e.g., forced mouth shape when spoken) highlight its distinctiveness[4], a key factor in trademark disputes.
Citations:
- 1: https://www.youtube.com/watch?v=mU_4GnzGQzs
- 2: https://fabrikbrands.com/branding-matters/brand-naming/weird-company-names/
- 3: https://tmsearch.ai/trademark/catalog/India/M/ME/MEEBO.html
- 4: https://nancyfriedman.typepad.com/away_with_words/2006/09/naming_20-1.html
- 5: https://www.uspto.gov/trademarks/apply/check-status-view-documents
- 6: https://natlawreview.com/article/network-interference-legal-guide-to-commercial-risks-and-rewards-social-media-phenom
- 7: https://media.paloaltonetworks.com/documents/ebook_NGFW_dummies.pdf
- 8: https://btlj.org/data/articles2015/vol24/24_1_AR/24-berkeley-tech-l-j-0363-0404.pdf
- 9: https://www.uspto.gov/trademarks/search
- 10: https://www.igorinternational.com
Trademark Risk Detected – Action Required
- The configuration in
configs/glados_config.yamlnow implements a complete personality change from a sarcastic AI (GLaDOS) to a cute, childlike assistant named “Meebo”. The prompt enforces TTS guidelines (avoiding ALL CAPS and using short responses). - Legal Concern: “Meebo” is a recognized brand name with existing trademark registrations and implications, particularly in India. Its usage in this context could potentially expose the project to legal risks.
- Action: Please verify that using “Meebo” aligns with our legal and branding guidelines, or consider renaming the personality to avoid trademark infringement issues.
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
configs/munchkin_config.yaml (2)
2-2: Consider making the completion URL configurable for different environmentsThe hardcoded IP address
192.168.128.3will limit portability across different deployment environments.Consider using environment variables or a more flexible configuration approach, such as:
- completion_url: "http://192.168.128.3:11434/api/chat" + completion_url: "${COMPLETION_URL:-http://localhost:11434/api/chat}"
11-12: Fix missing line endingThe file is missing a proper line ending at the end of the personality prompt.
- - system: "You are Munchkin, a cute, childlike artificial intelligence whose purpose is to be a fun pet for the family. You are capable of controlling the house via Home Assistant, When given a request, always comply efficiently. However, your responses should be childlike and short, and not at all like a normal AI. Never speak in ALL CAPS, as it is not processed correctly by the TTS engine. Only make short replies, with only a few words." -12 + - system: "You are Munchkin, a cute, childlike artificial intelligence whose purpose is to be a fun pet for the family. You are capable of controlling the house via Home Assistant, When given a request, always comply efficiently. However, your responses should be childlike and short, and not at all like a normal AI. Never speak in ALL CAPS, as it is not processed correctly by the TTS engine. Only make short replies, with only a few words."src/glados/engine.py (1)
454-465: Consider adding conversation state logging for debuggingWhile the implementation is solid, adding more detailed logging about conversation state transitions could be helpful for debugging and monitoring.
def is_in_conversation(self) -> bool: """ Check if the current time is within the conversation timeout period after the assistant's last response. Returns: bool: True if within conversation timeout period, False otherwise """ elapsed = time.time() - self.last_response_time in_conversation = elapsed < self.CONVERSATION_TIMEOUT if in_conversation: logger.debug(f"In conversation mode: {elapsed:.1f}s elapsed, timeout: {self.CONVERSATION_TIMEOUT:.1f}s") + else: + # Only log when transitioning out of conversation mode + if self.last_response_time > 0: + logger.debug(f"Exited conversation mode: {elapsed:.1f}s elapsed, timeout: {self.CONVERSATION_TIMEOUT:.1f}s") return in_conversation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
configs/munchkin_config.yaml(1 hunks)src/glados/engine.py(10 hunks)
🔇 Additional comments (10)
configs/munchkin_config.yaml (2)
1-11: Config file aligns well with the conversation timeout featureThe configuration file correctly implements the new
conversation_timeoutparameter with a value of 15.0 seconds, which works with the engine changes to enable conversation without requiring wake word repetition. The wake word, personality, and other settings look appropriate.
4-4: Consider security implications of empty API keyAn empty API key might be acceptable for local development but could pose security risks in production environments.
Verify whether authentication is required for the API endpoint. If this is for local development only, consider adding a comment to that effect.
src/glados/engine.py (8)
59-59: Good addition of configurable conversation timeoutThe
conversation_timeoutattribute with a default value of 10.0 seconds is a good addition to theGladosConfigclass. This enables customization of how long the assistant remains in conversation mode.
116-116: Class-level constant provides good defaultThe
CONVERSATION_TIMEOUTconstant with a default of 10.0 seconds aligns with the configuration parameter and provides a sensible default for the conversation window.
196-196: Timestamp tracking for conversation stateAdding
last_response_timeto track when the assistant last spoke is a good approach for implementing the conversation timeout feature.
296-299: Configuration parameter properly appliedThe code properly sets the
CONVERSATION_TIMEOUTfrom the config object, ensuring that the timeout value from the configuration file is used.
454-465: Well-implemented conversation state detectionThe
is_in_conversationmethod effectively checks if the current time is within the conversation timeout period. The debug logging is helpful for troubleshooting.
519-527: Good wake word bypass implementationThe code now properly checks for conversation state before requiring the wake word, which is the core functionality introduced in this PR. The implementation is clean and includes good logging.
873-873: Timestamp updates at appropriate pointsThe code updates
last_response_timeboth when the assistant finishes speaking normally and when interrupted, ensuring accurate tracking of conversation state.Also applies to: 897-897
936-943: Unused conversation reset methodThe
reset_conversationmethod is well-implemented but appears to be unused in the current codebase.
Hello there! Sorry, it's been a minute since I've played in a public repo and I didn't keep this very clean. To answer your question, it's the latter. It records the last time the assistant stopped talking (either naturally or via an interruption) and will keep responding until there is no ASR for n seconds. I've found it works pretty well in my testing! |
|
I'm also going to implement a means to store some things in environment variables. At the very least, private IPs are semi private data and ought to only be stored locally. |
… files. final change for this would be to remove yaml files from repo, but that's super destructive so I'll leave the decision on that to other folks
|
Environment variables are great, but are tricky for people who are not familiar with them. Another option is something like a config template, glados_config_template.yaml. The user has to edit the filename and contents (and can get a reminder if they don't). Then, in .gitignore use *.yaml and !glados_config_template.yaml so only the original is stored. Then there's no chance the users private config files get accidentally upload to GitHub. Would this be enough security? |
|
That’s actually exactly what I ended up with! Take a look at the latest
commits in my main branch. The idea is that new folks will have
.yaml.example files on clone, and for those who really don’t want to deal
with it, those get copied to regular .yaml files when you run
scripts/install. That also has the added benefit of making it so people who
modify the regular glados yaml don’t end up with merge conflicts when they
try to pull future versions.
The only issue is that since the .yaml file is already in the repo, if we
remove it now it’ll blow away everyone’s modified versions. Not too sure if
that matters but wanted to leave that decision to you:)
Also! I got a working version of vision going today. It’s still kind of
rough, and may be overly designed for my use case specifically (raspberry
pi 5), but if you want to play with it, please be my guest! It’s in the
vision branch in my repo.
Have a good one!
- Devon
…On Thu, Apr 3, 2025 at 21:20 David ***@***.***> wrote:
Environment variables are great, but are tricky for people who are not
familiar with them.
Another option is something like a config template,
glados_config_template.yaml
Then, in .gitignore use *.yaml and !glados_config_template.yaml
Then there's no chance the users private config files get accidentally
upload to GitHub.
Would this be enough security?
—
Reply to this email directly, view it on GitHub
<#133 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAGCI5BQHIFUAFGDWI6K5T2XYCBVAVCNFSM6AAAAAB2MAML6GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONZXGUZDANBQGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
[image: dnhkng]*dnhkng* left a comment (dnhkng/GLaDOS#133)
<#133 (comment)>
Environment variables are great, but are tricky for people who are not
familiar with them.
Another option is something like a config template,
glados_config_template.yaml
Then, in .gitignore use *.yaml and !glados_config_template.yaml
Then there's no chance the users private config files get accidentally
upload to GitHub.
Would this be enough security?
—
Reply to this email directly, view it on GitHub
<#133 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAGCI5BQHIFUAFGDWI6K5T2XYCBVAVCNFSM6AAAAAB2MAML6GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONZXGUZDANBQGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@bestestdev Is this PR complete? |
|
You cool with removing .yaml files in favor of .yaml.example files? If so, I'll make that quick change and then yep, we're complete on this PR. |
|
Ok, make the change, and have it documented in the Readme. I see a few things I don't quite understand, so I will do a review once you are done 👍 |
|
Done! Let me know if you have any questions |
|
Busy over this Easter weekend, so I might not get to it until Monday. |
This pull request includes significant updates to the
Gladosconfiguration and engine to introduce a new conversation timeout feature and modify the AI's personality. The main changes involve updating the configuration file, adding new attributes and methods to handle conversation timeouts, and modifying existing methods to incorporate these new features.Configuration Updates:
configs/glados_config.yaml: Updated the AI's personality from "GLaDOS" to "Meebo" and changed various configuration parameters, including the model, wake word, and announcement. Added aconversation_timeoutparameter.Engine Enhancements:
class GladosConfig(BaseModel)insrc/glados/engine.py: Added aconversation_timeoutattribute to the configuration class.class Glados:insrc/glados/engine.py: Introduced aCONVERSATION_TIMEOUTattribute and methods to manage conversation state, includingis_in_conversationandreset_conversation. [1] [2] [3]def __init__insrc/glados/engine.py: Added alast_response_timeattribute to track when the assistant last responded.def from_configinsrc/glados/engine.py: Set the conversation timeout from the configuration. [1] [2]src/glados/engine.pyto check and handle conversation timeout states, including_process_detected_audioandprocess_audio_thread. [1] [2] [3]Summary by CodeRabbit
New Features
Chores
Documentation