fix(tui): correct preset cycling keybinding to ctrl+tab and add persona tab-cycling

This PR fixes two keybinding issues in the TUI application to align with ADR-045 specifications:
1. Corrects the preset cycling keybinding from CTRL+t to CTRL+Tab as required by the specification
2. Adds the missing TAB keybinding for cycling through personas

These changes ensure the TUI keybindings match the documented behavior in ADR-045 and provide users with the expected keyboard shortcuts for navigating presets and personas.

Changes:

- Fixed preset cycling keybinding: Changed CTRL+T to CTRL+Tab in BINDINGS to match ADR-045 specification
- Added persona cycling keybinding: Introduced new TAB binding that maps to action_cycle_persona action
- Implemented action_cycle_persona method: New method cycles through personas with cycle_order > 0, sorted by cycle_order value
- Updated persona bar refresh: Ensured _refresh_persona_bar() is called after persona cycling to reflect UI changes
- Added BDD test scenarios: Comprehensive test coverage verifying both the preset cycling (CTRL+Tab) and persona cycling (TAB) keybindings work correctly

Testing:

- BDD scenarios added to verify CTRL+Tab keybinding correctly cycles through presets
- BDD scenarios added to verify TAB keybinding correctly cycles through personas
- Persona cycling respects the cycle_order attribute and only cycles through personas with cycle_order > 0
- UI refresh is properly triggered after persona changes

Parent Epic: #8601 (TUI Implementation — milestone v3.7.0)
Related ADRs: ADR-044 (TUI Architecture), ADR-045 (Persona System), ADR-046 (Reference/Command System)

CHANGELOG.md

@@ -7,6 +7,8 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- **TUI keybinding corrections** (#9358): Fixed preset cycling keybinding from `ctrl+t` to `ctrl+tab` and added persona tab-cycling with `tab` key. Personas with `cycle_order > 0` are cycled in order.
+
 - **Cross-actor subgraph cycle detection reads actor_ref field** (#1431): Fixed
   `_detect_subgraph_cycles()`, `_map_node()`, and the `compile_actor()` main loop
   in `src/cleveragents/actor/compiler.py` to read `actor_ref` from the top-level

CONTRIBUTORS.md

@@ -4,6 +4,7 @@
 * Brent E. Edwards <brent.edwards@cleverthis.com>
 * HAL 9000 <hal9000@cleverthis.com>
 * Hamza Khyari <hamza.khyari@cleverthis.com>
+* HAL9000 <hal9000@cleverthis.com>
 * Jeffrey Phillips Freeman <jeffrey.freeman@syncleus.com>
 * Luis Mendes <luis.p.mendes@gmail.com>
 * Rui Hu <rui.hu@cleverthis.com>

features/steps/tui_app_coverage_steps.py

@@ -23,6 +23,10 @@ from unittest.mock import MagicMock, patch
 
 from behave import given, then, when
 
+from cleveragents.tui.persona.registry import PersonaRegistry
+from cleveragents.tui.persona.schema import Persona
+from cleveragents.tui.persona.state import PersonaState
+
 # ---------------------------------------------------------------------------
 # Mock Textual infrastructure
 # ---------------------------------------------------------------------------
@@ -162,9 +166,6 @@ def _restore_modules(context):
 
 def _make_persona_state(context):
     """Create a real PersonaState backed by a temp directory."""
-    from cleveragents.tui.persona.registry import PersonaRegistry
-    from cleveragents.tui.persona.state import PersonaState
-
     tmp = tempfile.mkdtemp()
     context._tui_tmpdir = tmp
     registry = PersonaRegistry(config_dir=Path(tmp))
@@ -520,3 +521,70 @@ def step_theme_class_var(context, theme: str) -> None:
     assert theme == context._tui_app.THEME, (
         f"Expected THEME='{theme}', got '{context._tui_app.THEME}'"
     )
+
+
+# --- action_cycle_persona keybinding ---
+@then('the app class should have a keybinding for "{key}" to "{action}"')
+def step_check_keybinding(context, key, action):
+    """Verify that a specific keybinding exists."""
+    app_class = context._tui_app_mod._TextualCleverAgentsTuiApp
+    bindings = app_class.BINDINGS
+    found = any(b[0] == key and b[1] == action for b in bindings)
+    assert found, f"Keybinding {key} -> {action} not found in {bindings}"
+
+
+@given("a mock command router and persona state with cycleable personas")
+def step_create_cycleable_personas(context):
+    """Create a persona state with multiple personas that have cycle_order > 0."""
+    tmp = tempfile.mkdtemp()
+    context._tui_tmpdir = tmp
+    registry = PersonaRegistry(config_dir=Path(tmp))
+
+    # Create default persona
+    registry.ensure_default()
+
+    # Create cycleable personas
+    persona1 = Persona(
+        name="persona1",
+        actor="local/test",
+        cycle_order=1,
+    )
+    persona2 = Persona(
+        name="persona2",
+        actor="local/test",
+        cycle_order=2,
+    )
+    registry.save(persona1)
+    registry.save(persona2)
+
+    context._tui_cmd_router = _FakeCommandRouter()
+    context._tui_persona_state = PersonaState(registry=registry)
+
+
+@when("I call action_cycle_persona on the app")
+def step_call_action_cycle_persona(context):
+    """Call the action_cycle_persona method on the app."""
+    # Capture the active persona name before cycling so the then-step can
+    # assert that it actually changed.
+    context._tui_persona_before_cycle = context._tui_persona_state.active_name(
+        context._tui_app._session.session_id
+    )
+    context._tui_app.action_cycle_persona()
+
+
+@then("the active persona should have changed")
+def step_check_persona_changed(context):
+    """Verify that the active persona has changed after cycling."""
+    session_id = context._tui_app._session.session_id
+    active_after = context._tui_persona_state.active_name(session_id)
+    active_before = getattr(context, "_tui_persona_before_cycle", None)
+    # The active persona must differ from the pre-cycle value.
+    assert active_after != active_before, (
+        f"Expected persona to change but it stayed '{active_after}'"
+    )
+    # The new active persona must be one of the cycleable personas.
+    personas = context._tui_persona_state.registry.list_personas()
+    cycleable = [p for p in personas if p.cycle_order > 0]
+    assert any(p.name == active_after for p in cycleable), (
+        f"Active persona '{active_after}' is not cycleable; cycleable={[p.name for p in cycleable]}"
+    )

features/steps/tui_persona_state_coverage_steps.py

@@ -9,6 +9,7 @@ These steps target specific uncovered lines in tui/persona/state.py:
 - Lines 54-55: cycle_preset() when persona has no argument_presets
 - Lines 59-60: cycle_preset() when current preset not in names list
 - Lines 67-69: effective_arguments() delegation
+- cycle_persona(): no cycleable personas, advance, and wrap-around
 """
 
 from types import SimpleNamespace
@@ -353,3 +354,54 @@ def step_verify_effective_arguments(context):
     assert context.result_eff_args["model"] == "gpt-4"
     assert context.result_eff_args["temp"] == 0.7
     assert context.result_eff_args["speed"] == "fast"
+
+
+# ---------------------------------------------------------------------------
+# Scenario: cycle_persona (new method)
+# ---------------------------------------------------------------------------
+
+
+@when('I cycle the persona for session "{session_id}"')
+def step_cycle_persona(context, session_id):
+    context.result_cycle_persona = context.state.cycle_persona(session_id)
+
+
+@then("the cycle_persona result should be None")
+def step_verify_cycle_persona_none(context):
+    assert context.result_cycle_persona is None
+
+
+@then('the cycle_persona result should be "{expected}"')
+def step_verify_cycle_persona_result(context, expected):
+    assert context.result_cycle_persona == expected, (
+        f"Expected cycle_persona to return '{expected}', got '{context.result_cycle_persona}'"
+    )
+
+
+@given('cycleable personas "{names_csv}" are registered for session "{session_id}"')
+def step_register_cycleable_personas(context, names_csv, session_id):
+    """Register multiple cycleable personas with ascending cycle_order values.
+
+    A non-cycleable "default" persona is used as the registry default so that
+    when no active persona has been set for the session, ``_resolve_default_name``
+    returns "default" (not in the cycleable list).  This means the first call to
+    ``cycle_persona`` starts from the beginning of the cycleable list (index 0).
+    """
+    names = [n.strip() for n in names_csv.split(",")]
+    registry = MagicMock()
+    personas = [
+        Persona(name=name, actor="ns/actor", cycle_order=idx + 1)
+        for idx, name in enumerate(names)
+    ]
+    # Non-cycleable default persona — ensures _resolve_default_name() returns a
+    # name that is NOT in the cycleable list, so cycling starts from index 0.
+    default_persona = Persona(name="default", actor="ns/actor", cycle_order=0)
+    registry.list_personas.return_value = personas
+    registry.get.side_effect = lambda name: next(
+        (p for p in personas if p.name == name), None
+    )
+    registry.ensure_default.return_value = default_persona
+    registry.get_last_persona.return_value = None
+    registry.set_last_persona = MagicMock()
+    context.state = PersonaState(registry=registry)
+    context.mock_registry = registry

features/tui_app_coverage.feature

@@ -47,7 +47,7 @@ Feature: TUI App Coverage
     Given a mock command router and persona state
     When I instantiate the Textual TUI app
     Then the app class should have CSS_PATH set to "cleveragents.tcss"
-    And the app class should have 3 key bindings
+    And the app class should have 4 key bindings
 
   # --- compose method (lines 102-112) ---
 
@@ -111,6 +111,29 @@ Feature: TUI App Coverage
     And I call action_cycle_preset on the app
     Then the persona bar content should be refreshed
 
+  # --- action_cycle_preset method (lines 127-129) ---
+
+  Scenario: action_cycle_preset uses ctrl+tab keybinding
+    Given a mock command router and persona state
+    When I instantiate the Textual TUI app
+    Then the app class should have a keybinding for "ctrl+tab" to "cycle_preset"
+
+  # --- action_cycle_preset method (lines 127-129) ---
+
+  # --- action_cycle_persona method (new) ---
+
+  Scenario: action_cycle_persona uses tab keybinding
+    Given a mock command router and persona state
+    When I instantiate the Textual TUI app
+    Then the app class should have a keybinding for "tab" to "cycle_persona"
+
+  Scenario: action_cycle_persona cycles through personas with cycle_order > 0
+    Given a mock command router and persona state with cycleable personas
+    When I instantiate the Textual TUI app
+    And I call on_mount on the app
+    And I call action_cycle_persona on the app
+    Then the active persona should have changed
+
   # --- _refresh_persona_bar method (lines 131-142) ---
 
   Scenario: _refresh_persona_bar builds scope text from active persona

features/tui_persona_state_coverage.feature

@@ -66,3 +66,19 @@ Feature: TUI Persona State Coverage
     Given a persona with base arguments and presets is active for session "sess-11"
     When I request the effective arguments for session "sess-11"
     Then the effective arguments should merge base and preset overrides
+
+  Scenario: cycle_persona returns None when no cycleable personas exist
+    When I cycle the persona for session "sess-12"
+    Then the cycle_persona result should be None
+
+  Scenario: cycle_persona advances to the next cycleable persona
+    Given cycleable personas "alpha,beta,gamma" are registered for session "sess-13"
+    When I cycle the persona for session "sess-13"
+    Then the cycle_persona result should be "alpha"
+    And session "sess-13" should have active persona "alpha"
+
+  Scenario: cycle_persona wraps around to the first persona
+    Given cycleable personas "alpha,beta,gamma" are registered for session "sess-14"
+    And session "sess-14" already has active persona "gamma"
+    When I cycle the persona for session "sess-14"
+    Then the cycle_persona result should be "alpha"

src/cleveragents/tui/app.py

@@ -93,7 +93,8 @@ if _TEXTUAL_AVAILABLE:
         BINDINGS: ClassVar[list[tuple[str, str, str]]] = [
             ("ctrl+q", "quit", "Quit"),
             ("f1", "help", "Help"),
-            ("ctrl+t", "cycle_preset", "Cycle Preset"),
+            ("ctrl+tab", "cycle_preset", "Cycle Preset"),
+            ("tab", "cycle_persona", "Cycle Persona"),
         ]
 
         def __init__(
@@ -153,6 +154,11 @@ if _TEXTUAL_AVAILABLE:
             self._persona_state.cycle_preset(self._session.session_id)
             self._refresh_persona_bar()
 
+        def action_cycle_persona(self) -> None:
+            """Cycle through personas with cycle_order > 0, sorted by cycle_order."""
+            self._persona_state.cycle_persona(self._session.session_id)
+            self._refresh_persona_bar()
+
         def _refresh_persona_bar(self) -> None:
             persona = self._persona_state.active_persona(self._session.session_id)
             preset = self._persona_state.current_preset(self._session.session_id)

src/cleveragents/tui/persona/state.py

@@ -63,6 +63,27 @@ class PersonaState:
         self.preset_by_session[session_id] = next_name
         return next_name
 
+    def cycle_persona(self, session_id: str) -> str | None:
+        """Cycle to the next persona with cycle_order > 0, sorted by cycle_order.
+
+        Returns the name of the newly active persona, or None if no cycleable
+        personas exist.
+        """
+        personas = self.registry.list_personas()
+        cycleable = sorted(
+            (p for p in personas if p.cycle_order > 0),
+            key=lambda p: p.cycle_order,
+        )
+        if not cycleable:
+            return None
+        current_name = self.active_name(session_id)
+        current_idx = next(
+            (i for i, p in enumerate(cycleable) if p.name == current_name), -1
+        )
+        next_persona = cycleable[(current_idx + 1) % len(cycleable)]
+        self.set_active_persona(session_id, next_persona.name)
+        return next_persona.name
+
     def effective_arguments(self, session_id: str) -> dict[str, object]:
         persona = self.active_persona(session_id)
         preset = self.current_preset(session_id)