Skip to content

fix: remap text/both input_mode from config file in TUI to avoid stdin conflict #193

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.

Sign up for GitHub

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

Open
octo-patch wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: remap text/both input_mode from config file in TUI to avoid stdin conflict #193

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions src/glados/tui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,11 +1403,28 @@ def instantiate_glados(self) -> None:
glados_config = GladosConfig.from_yaml(str(config_path))
updates: dict[str, object] = {}
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
elif glados_config.input_mode == "text":
# Config file sets input_mode: "text" but TUI is running — remap to "none"
# to prevent TextListener from competing with Textual for stdin, which causes
# keyboard input to require multiple keystrokes. The TUI Input widget submits
# text via submit_text_input() instead.
updates["input_mode"] = "none"
elif glados_config.input_mode == "both":
# Config file sets input_mode: "both" but TUI is running — remap to "audio"
# so audio ASR stays active while TextListener is suppressed to avoid stdin
# contention with Textual. Text input is handled by the TUI Input widget.
updates["input_mode"] = "audio"
Comment on lines +1406 to +1427
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "GladosConfig.input_mode declaration:"
sed -n '104,112p' src/glados/core/engine.py

echo
echo "Engine input_mode branches:"
rg -nP -C2 'self\.input_mode\s*(==|in)' src/glados/core/engine.py

echo
echo 'Current TUI remap block:'
sed -n '1403,1434p' src/glados/tui.py

echo
echo 'Any existing engine-side support for "none":'
rg -nP '"none"' src/glados/core/engine.py src/glados/tui.py

Repository: dnhkng/GLaDOS

Length of output: 4007

Don't inject an undeclared input_mode value.

Lines 1410 and 1422 write "none" to input_mode, but GladosConfig.input_mode is declared as Literal["audio", "text", "both"] and the engine only branches on those three values. When this config is passed to Glados.from_config(), the "none" value will violate the type contract and cause both speech_listener and text_listener to be silently skipped without explicit intent.

Either extend the input_mode Literal type to include "none" across config and engine layers, or suppress TextListener using a separate TUI-only flag while keeping input_mode within its declared contract.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/glados/tui.py` around lines 1406 - 1427, The code writes "none" into
updates["input_mode"], which violates GladosConfig.input_mode's
Literal["audio","text","both"]; instead avoid injecting "none" and add a
TUI-only flag to suppress the TextListener: leave updates["input_mode"] set to
one of "audio"/"text"/"both" (use the same mapping logic in the branches around
_input_mode_override and glados_config.input_mode) and add a separate boolean
key such as updates["tui_suppress_text_listener"] (or
self._tui_suppress_text_listener) where you currently set "none" to indicate the
TUI should not start TextListener; update Glados.from_config()/engine code to
read that flag to conditionally skip TextListener (affecting
speech_listener/text_listener startup) rather than changing input_mode's value.

Copy link
Copy Markdown
Owner

@dnhkng dnhkng Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the options to fix this? Pros and cons for both please

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== GladosConfig declaration ==="
rg -nP -A5 -B2 'input_mode' src/glados/core/engine.py | head -80

echo
echo "=== Engine branching on input_mode ==="
rg -nP -C5 'input_mode' src/glados/core/engine.py

echo
echo "=== TextListener / speech_listener startup ==="
rg -nP -C5 'TextListener|text_listener|speech_listener' src/glados/core/engine.py | head -100

echo
echo "=== from_config method ==="
ast-grep --pattern 'def from_config($$$) {
  $$$
}'

Length of output: 11804

@dnhkng, here's a breakdown of both options with the engine code as context.

The engine's __init__ decides what to start based on input_mode via two independent guards:

if self.input_mode in {"audio", "both"}:   # → starts SpeechListener + audio stream
    ...
if self.input_mode in {"text", "both"}:    # → starts TextListener
    ...

When "none" is passed today, neither branch matches, so both listeners are silently skipped — it works, but only by accident.

Option A — Extend input_mode to formally include "none"

Make "none" a first-class value in GladosConfig, Glados.__init__, and the YAML schema.

Changes required:

  • GladosConfig.input_mode: Literal["audio", "text", "both", "none"] = "audio" (engine.py line 110)
  • Same in Glados.__init__() parameter (line 201)
  • Optionally add an explicit elif self.input_mode == "none": logger.info("Input disabled.") branch for clarity

Pros:

  • ✅ Minimal diff — only type annotation changes needed; the engine logic already handles it implicitly
  • ✅ Makes the intent explicit and self-documenting
  • ✅ Useful beyond TUI — e.g. for testing, headless batch processing, or future use cases that need no input
  • ✅ No TUI concepts leak into the engine layer
  • ✅ Pydantic will validate the YAML value correctly once added

Cons:

  • ⚠️ Changes the public config schema — users' YAML files that specify input_mode: none will now be valid (could be surprising)
  • ⚠️ Slightly broadens the engine API surface

Option B — Add a TUI-only suppress_text_listener flag

Keep input_mode as Literal["audio", "text", "both"] and introduce a separate boolean in GladosConfig (e.g. tui_mode: bool = False) that the engine checks when starting TextListener.

Changes required:

  • Add tui_mode: bool = False to GladosConfig and Glados.__init__()
  • Change the TextListener guard in engine to if self.input_mode in {"text", "both"} and not self.tui_mode
  • In tui.py, set updates["tui_mode"] = True instead of remapping input_mode to "none" or "audio"; keep input_mode unchanged

Pros:

  • ✅ input_mode contract stays strictly intact
  • ✅ The remapping logic in tui.py becomes simpler (no need to translate values)
  • ✅ Engine status/config commands (/status, /config) still report the user's original intent

Cons:

  • ⚠️ TUI-specific concern (tui_mode) leaks into the engine and config schema — breaks separation of concerns
  • ⚠️ More code to change: engine guard, config model, __init__ signature, and TUI
  • ⚠️ The flag name is ambiguous — future developers may not know why it suppresses TextListener
  • ⚠️ If you later add other TUI-incompatible listeners, you'd need to thread this flag further

Recommendation

Option A is the cleaner path. The engine's existing if ... in {"audio", "both"} / if ... in {"text", "both"} pattern already excludes "none" correctly — the only gap is the missing type declaration. A two-line type annotation change formally closes that gap without introducing cross-layer coupling.

if self._tts_enabled_override is not None:
updates["tts_enabled"] = self._tts_enabled_override
if self._asr_muted_override is not None:
Expand Down