fix: stop audio stream when input_mode is text in TUI (fixes #188)#192
Open
octo-patch wants to merge 2 commits intodnhkng:mainfrom
Open
fix: stop audio stream when input_mode is text in TUI (fixes #188)#192octo-patch wants to merge 2 commits intodnhkng:mainfrom
octo-patch wants to merge 2 commits intodnhkng:mainfrom
Conversation
Fixes dnhkng#188 - When input_mode is set to 'text' in the TUI, the TUI was previously converting it to 'audio' to avoid stdin contention between Textual and TextListener. However, this kept the audio input stream active, causing high CPU usage (related to dnhkng#187) which in turn made keyboard registration sluggish in the TUI. This commit introduces a new 'none' input mode that starts neither the SpeechListener (audio) nor the TextListener (stdin). In TUI mode, the Textual Input widget already handles all text submission, so no listener is needed. Using 'none' for TUI+text mode stops the audio stream entirely, resolving both the high CPU usage and the resulting keyboard lag.
Previously, when input_mode was set to 'text' in the TUI, it was converted to 'audio' to avoid stdin contention. This kept the audio stream active, causing high CPU usage and sluggish keyboard response. Now 'text' maps to 'none' (no SpeechListener, no TextListener) since the Textual Input widget already handles all text submission. The 'both' mode correctly maps to 'audio' (ASR active, TUI handles text). Fixes dnhkng#188
Contributor
📝 WalkthroughWalkthroughThe changes introduce a new "none" input mode to the engine configuration and adjust TUI input handling logic to resolve keyboard input registration issues. When TUI operates in text mode, it now uses the internal "none" mode to disable audio and text listeners, preventing stdin contention. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
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)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/glados/core/engine.py`:
- Around line 713-714: The emotion agent wiring in _register_subagents() is
skipped because self.autonomy_loop isn't created yet; ensure
set_emotion_agent(emotion_agent) is called after the loop is instantiated (or
defer wiring by registering the emotion_agent to be set once the loop is
created). Concretely, either move the set_emotion_agent(...) call to the block
where self.autonomy_loop is initialized (the code that constructs/assigns
self.autonomy_loop) or add a short helper (e.g., a pending_emotion_agent
attribute and a post-initialize step) that calls
self.autonomy_loop.set_emotion_agent(emotion_agent) once self.autonomy_loop
exists; reference _register_subagents, autonomy_loop, set_emotion_agent, and
autonomy_config.enabled when making the change.
In `@src/glados/tui.py`:
- Around line 1405-1416: The TUI currently only remaps input modes when
self._input_mode_override is set, so a config-only input_mode: "text" stays as
"text"; change the logic to read the effective mode from
self._input_mode_override or the configured value (e.g.
self._config.get("input_mode") or self.config.input_mode) and apply the same
remapping branches (treat "text" -> "none", "both" -> "audio", else passthrough)
before writing updates["input_mode"]; update the block around
self._input_mode_override and updates["input_mode"] to compute mode =
self._input_mode_override or config_mode and use that for the remap.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4e75f81-ac51-4f35-bf45-23b253bec6ec
📒 Files selected for processing (2)
src/glados/core/engine.pysrc/glados/tui.py
Comment on lines
+713
to
714
| if self.autonomy_config.enabled and self.autonomy_loop: | ||
| self.autonomy_loop.set_emotion_agent(emotion_agent) |
Contributor
There was a problem hiding this comment.
Emotion agent wiring is skipped because autonomy_loop is not initialized yet.
At Line 713, this check runs inside _register_subagents(), but self.autonomy_loop is created later (Line 511), so set_emotion_agent(...) is never called.
Suggested fix (defer wiring until loop is created)
- # Wire emotion agent to autonomy loop for vision events
- if self.autonomy_config.enabled and self.autonomy_loop:
- self.autonomy_loop.set_emotion_agent(emotion_agent)
+ # Wire emotion agent to autonomy loop after autonomy_loop is initialized. if self.autonomy_config.enabled:
assert self.autonomy_event_bus is not None
assert self.autonomy_slots is not None
self.autonomy_loop = AutonomyLoop(
config=self.autonomy_config,
event_bus=self.autonomy_event_bus,
interaction_state=self.interaction_state,
vision_state=self.vision_state,
slot_store=self.autonomy_slots,
llm_queue=self.llm_queue_autonomy,
processing_active_event=self.processing_active_event,
currently_speaking_event=self.currently_speaking_event,
shutdown_event=self.shutdown_event,
observability_bus=self.observability_bus,
inflight_counter=self._autonomy_inflight,
pause_time=self.PAUSE_TIME,
)
+ if self._emotion_agent is not None:
+ self.autonomy_loop.set_emotion_agent(self._emotion_agent)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/glados/core/engine.py` around lines 713 - 714, The emotion agent wiring
in _register_subagents() is skipped because self.autonomy_loop isn't created
yet; ensure set_emotion_agent(emotion_agent) is called after the loop is
instantiated (or defer wiring by registering the emotion_agent to be set once
the loop is created). Concretely, either move the set_emotion_agent(...) call to
the block where self.autonomy_loop is initialized (the code that
constructs/assigns self.autonomy_loop) or add a short helper (e.g., a
pending_emotion_agent attribute and a post-initialize step) that calls
self.autonomy_loop.set_emotion_agent(emotion_agent) once self.autonomy_loop
exists; reference _register_subagents, autonomy_loop, set_emotion_agent, and
autonomy_config.enabled when making the change.
Comment on lines
1405
to
1416
| if self._input_mode_override: | ||
| if self._input_mode_override in {"text", "both"}: | ||
| # Avoid stdin contention between Textual and TextListener. | ||
| if self._input_mode_override == "text": | ||
| # In TUI text mode, use "none" so neither audio stream nor TextListener | ||
| # is started. The TUI Input widget handles all text input, avoiding | ||
| # both high CPU from audio and stdin contention with TextListener. | ||
| updates["input_mode"] = "none" | ||
| elif self._input_mode_override == "both": | ||
| # In TUI+both mode, keep audio ASR active; the TUI Input widget handles | ||
| # text input, so TextListener is not needed (avoids stdin contention). | ||
| updates["input_mode"] = "audio" | ||
| else: | ||
| updates["input_mode"] = self._input_mode_override |
Contributor
There was a problem hiding this comment.
Config-only input_mode: "text" is not remapped for TUI.
At Line 1405, remapping happens only when self._input_mode_override is set. If a user sets input_mode: "text" in YAML and launches TUI without CLI override, the engine keeps "text" instead of "none".
Suggested fix
glados_config = GladosConfig.from_yaml(str(config_path))
updates: dict[str, object] = {}
- if self._input_mode_override:
- if self._input_mode_override == "text":
+ effective_input_mode = self._input_mode_override or glados_config.input_mode
+ if effective_input_mode:
+ if effective_input_mode == "text":
# In TUI text mode, use "none" so neither audio stream nor TextListener
# is started. The TUI Input widget handles all text input, avoiding
# both high CPU from audio and stdin contention with TextListener.
updates["input_mode"] = "none"
- elif self._input_mode_override == "both":
+ elif effective_input_mode == "both":
# In TUI+both mode, keep audio ASR active; the TUI Input widget handles
# text input, so TextListener is not needed (avoids stdin contention).
updates["input_mode"] = "audio"
else:
- updates["input_mode"] = self._input_mode_override
+ updates["input_mode"] = effective_input_mode📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if self._input_mode_override: | |
| if self._input_mode_override in {"text", "both"}: | |
| # Avoid stdin contention between Textual and TextListener. | |
| if self._input_mode_override == "text": | |
| # In TUI text mode, use "none" so neither audio stream nor TextListener | |
| # is started. The TUI Input widget handles all text input, avoiding | |
| # both high CPU from audio and stdin contention with TextListener. | |
| updates["input_mode"] = "none" | |
| elif self._input_mode_override == "both": | |
| # In TUI+both mode, keep audio ASR active; the TUI Input widget handles | |
| # text input, so TextListener is not needed (avoids stdin contention). | |
| updates["input_mode"] = "audio" | |
| else: | |
| updates["input_mode"] = self._input_mode_override | |
| effective_input_mode = self._input_mode_override or glados_config.input_mode | |
| if effective_input_mode: | |
| if effective_input_mode == "text": | |
| # In TUI text mode, use "none" so neither audio stream nor TextListener | |
| # is started. The TUI Input widget handles all text input, avoiding | |
| # both high CPU from audio and stdin contention with TextListener. | |
| updates["input_mode"] = "none" | |
| elif effective_input_mode == "both": | |
| # In TUI+both mode, keep audio ASR active; the TUI Input widget handles | |
| # text input, so TextListener is not needed (avoids stdin contention). | |
| updates["input_mode"] = "audio" | |
| else: | |
| updates["input_mode"] = effective_input_mode |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/glados/tui.py` around lines 1405 - 1416, The TUI currently only remaps
input modes when self._input_mode_override is set, so a config-only input_mode:
"text" stays as "text"; change the logic to read the effective mode from
self._input_mode_override or the configured value (e.g.
self._config.get("input_mode") or self.config.input_mode) and apply the same
remapping branches (treat "text" -> "none", "both" -> "audio", else passthrough)
before writing updates["input_mode"]; update the block around
self._input_mode_override and updates["input_mode"] to compute mode =
self._input_mode_override or config_mode and use that for the remap.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #188
Problem
When
input_mode: "text"is set in the config and GLaDOS is launched via the TUI (glados tui), the TUI was silently converting the mode to"audio"to avoid stdin contention between Textual and theTextListener. However, this kept the audio input stream running, causing:Solution
Introduced a new
"none"input mode in the engine that starts neitherSpeechListener(audio) norTextListener(stdin). In TUI text mode, the TextualInputwidget already handles all user text submission viasubmit_text_input(), so no additional listener is needed.The mapping in the TUI is now:
input_mode: "text"→ engine uses"none"(no audio stream, no stdin listener → low CPU, responsive keyboard)input_mode: "both"→ engine uses"audio"(ASR active, TUI Input handles text → was already correct)Testing
input_mode: "text"inglados_config.yamlglados tuiSummary by CodeRabbit
New Features
Improvements