diff --git a/features/steps/tui_llm_dispatch_steps.py b/features/steps/tui_llm_dispatch_steps.py new file mode 100644 index 000000000..e225d541f --- /dev/null +++ b/features/steps/tui_llm_dispatch_steps.py @@ -0,0 +1,429 @@ +"""Step definitions for tui_llm_dispatch.feature. + +Tests the TUI LLM dispatch wiring without requiring Textual or importlib.reload. + +The core dispatch logic lives in the module-level ``_run_llm_dispatch()`` +function in ``tui/app.py``. This function is Textual-free and can be +imported and tested directly. + +Session creation and facade building helpers (``_create_tui_session`` and +``_build_tui_facade`` in ``tui/commands.py``) are also tested directly. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock, patch + +from behave import given, then, when + +from cleveragents.tui.app import _format_worker_outcome, _run_llm_dispatch + + +# --------------------------------------------------------------------------- +# Shared helpers +# --------------------------------------------------------------------------- + + +def _make_mock_facade(response_text: str = "Mock LLM reply") -> MagicMock: + """Return a mock A2aLocalFacade returning a canned assistant message.""" + facade = MagicMock() + response = MagicMock() + response.error = None + response.result = {"assistant_message": response_text} + facade.dispatch.return_value = response + return facade + + +# --------------------------------------------------------------------------- +# Given steps +# --------------------------------------------------------------------------- + + +@given("the TUI app is initialised with a mock A2A facade and session") +def step_init_with_facade(context: Any) -> None: + """Set up a mock facade and session_id on the context.""" + context._facade = _make_mock_facade() + context._session_id = "test-session-001" + + +@given("the TUI app is initialised with a facade that returns empty response") +def step_init_with_empty_response_facade(context: Any) -> None: + """Set up a mock facade that returns empty assistant_message.""" + context._facade = _make_mock_facade(response_text="") + context._session_id = "test-session-001" + + +@given("the TUI app is initialised without a facade") +def step_init_without_facade(context: Any) -> None: + """Simulate facade=None (preview fallback path).""" + context._facade = None + context._session_id = "default" + + +@given("the facade raises SessionActorNotConfiguredError") +def step_facade_raises_no_actor(context: Any) -> None: + """Configure the mock facade to raise SessionActorNotConfiguredError.""" + from cleveragents.domain.models.core.session import SessionActorNotConfiguredError + + facade = MagicMock() + facade.dispatch.side_effect = SessionActorNotConfiguredError("no actor") + context._facade = facade + context._session_id = "default" + + +@given("the facade raises SessionNotFoundError") +def step_facade_raises_session_not_found(context: Any) -> None: + """Configure the mock facade to raise SessionNotFoundError.""" + from cleveragents.domain.models.core.session import SessionNotFoundError + + facade = MagicMock() + facade.dispatch.side_effect = SessionNotFoundError("session not found") + context._facade = facade + context._session_id = "default" + + +@given("the facade raises DatabaseError") +def step_facade_raises_database_error(context: Any) -> None: + """Configure the mock facade to raise DatabaseError.""" + from cleveragents.core.exceptions import DatabaseError + + facade = MagicMock() + facade.dispatch.side_effect = DatabaseError("db locked") + context._facade = facade + context._session_id = "default" + + +@given("the facade returns an A2A error response") +def step_facade_returns_error(context: Any) -> None: + """Configure the mock facade to return an A2A error response.""" + facade = MagicMock() + error = MagicMock() + error.message = "internal server error" + response = MagicMock() + response.error = error + response.result = None + facade.dispatch.return_value = response + context._facade = facade + context._session_id = "default" + + +@given('a mock session service that returns session_id "{sid}"') +def step_mock_session_service(context: Any, sid: str) -> None: + """Set up a mock session service returning the given session_id.""" + session = MagicMock() + session.session_id = sid + service = MagicMock() + service.create.return_value = session + context._mock_service = service + context._expected_sid = sid + + +@given("the session service raises an exception") +def step_session_service_raises(context: Any) -> None: + """Set up a mock session service that raises on create().""" + service = MagicMock() + service.create.side_effect = RuntimeError("db unavailable") + context._mock_service = service + context._expected_sid = "default" + + +# --------------------------------------------------------------------------- +# When steps +# --------------------------------------------------------------------------- + + +@when('I submit the text "{text}"') +def step_submit_text(context: Any, text: str) -> None: + """Invoke _run_llm_dispatch directly with the given text.""" + context._submitted_text = text # store for parameterized assertions + if context._facade is None: + # Preview fallback path — no dispatch, just echo + context._result_text = text + context._app_transcript = [] + else: + context._result_text = _run_llm_dispatch( + context._facade, context._session_id, text + ) + # Simulate what on_input_submitted does: pre-escape and store in transcript + from rich.markup import escape as _esc + + context._app_transcript = [_esc(f"You: {text}")] + + +@when('I dispatch "{text}" overriding actor with "{actor}"') +def step_dispatch_text_with_actor(context: Any, text: str, actor: str) -> None: + """Invoke _run_llm_dispatch with an explicit actor_override.""" + context._submitted_text = text + context._result_text = _run_llm_dispatch( + context._facade, context._session_id, text, actor_override=actor + ) + + +@when("the worker completes with a KeyboardInterrupt error") +def step_worker_keyboard_interrupt(context: Any) -> None: + """Simulate a worker that completed with a KeyboardInterrupt.""" + context._worker_error = KeyboardInterrupt("user interrupt") + + +@when("the worker completes with a SystemExit error") +def step_worker_system_exit(context: Any) -> None: + """Simulate a worker that completed with a SystemExit.""" + context._worker_error = SystemExit(1) + + +@when("the worker completes with neither result nor error") +def step_worker_no_result_no_error(context: Any) -> None: + """Simulate a worker that completed with both result and error as None.""" + context._worker_outcome = _format_worker_outcome(result=None, error=None) + + +@when("_build_tui_facade is called with a broken container") +def step_build_tui_facade_broken_container(context: Any) -> None: + """Call _build_tui_facade with a container that always raises.""" + from cleveragents.tui import commands as cmd_mod + + cmd_mod._tui_session_service = None + broken = MagicMock() + broken.session_service.side_effect = RuntimeError("no db") + with patch("cleveragents.a2a.cli_bootstrap.get_facade") as mock_facade: + mock_facade.side_effect = ImportError("no a2a") + context._facade_result = cmd_mod._build_tui_facade() + cmd_mod._tui_session_service = None + + +@when('the worker completes with error "{error_msg}"') +def step_worker_completes_with_error(context: Any, error_msg: str) -> None: + """Simulate a Textual worker that failed with the given exception.""" + context._worker_outcome = _format_worker_outcome( + result=None, error=RuntimeError(error_msg) + ) + + +@when('the worker completes with result "{result}"') +def step_worker_completes_with_result(context: Any, result: str) -> None: + """Simulate a Textual worker that succeeded with the given result.""" + context._worker_outcome = _format_worker_outcome(result=result, error=None) + + +@given('a facade returning assistant text "{reply}"') +def step_facade_returning_specific_reply(context: Any, reply: str) -> None: + """Set up a mock facade with a specific assistant reply.""" + context._facade = _make_mock_facade(response_text=reply) + context._session_id = "test-session-001" + + +@given('a persona state with active actor "{actor}"') +def step_persona_state_with_actor(context: Any, actor: str) -> None: + """Set up a mock PersonaState whose active persona has the given actor.""" + persona = MagicMock() + persona.actor = actor + state = MagicMock() + state.active_persona.return_value = persona + context._mock_persona_state = state + + +@when("I call _create_tui_session with that service") +def step_call_create_tui_session(context: Any) -> None: + """Call _create_tui_session patching get_container, no persona state.""" + from cleveragents.tui import commands as cmd_mod + + container = MagicMock() + container.session_service.return_value = context._mock_service + # Reset singleton so the patched container is used on this call. + cmd_mod._tui_session_service = None + with patch.object(cmd_mod, "get_container", return_value=container): + context._result_sid = cmd_mod._create_tui_session() + cmd_mod._tui_session_service = None # clean up after test + + +@when("I call _create_tui_session with that service and persona state") +def step_call_create_tui_session_with_persona(context: Any) -> None: + """Call _create_tui_session with a persona state so actor is bound.""" + from cleveragents.tui import commands as cmd_mod + + container = MagicMock() + container.session_service.return_value = context._mock_service + # Reset singleton so the patched container is used on this call. + cmd_mod._tui_session_service = None + with patch.object(cmd_mod, "get_container", return_value=container): + context._result_sid = cmd_mod._create_tui_session( + persona_state=context._mock_persona_state + ) + cmd_mod._tui_session_service = None # clean up after test + + +# --------------------------------------------------------------------------- +# Then steps +# --------------------------------------------------------------------------- + + +@then("the facade should have received a message/send request") +def step_facade_received_request(context: Any) -> None: + """Assert facade.dispatch was called with method=message/send.""" + context._facade.dispatch.assert_called_once() + request = context._facade.dispatch.call_args[0][0] + assert request.method == "message/send", ( + f"Expected method='message/send', got {request.method!r}" + ) + assert request.params["session_id"] == context._session_id + assert request.params["message"] == context._submitted_text, ( + f"Expected message={context._submitted_text!r}, " + f"got {request.params['message']!r}" + ) + + +@then("the conversation should show the assistant response") +def step_conversation_shows_response(context: Any) -> None: + """Assert the result contains the assistant response.""" + assert "Mock LLM reply" in context._result_text, ( + f"Expected assistant response in result, got: {context._result_text!r}" + ) + + +@then('the conversation should show the preview text "{expected}"') +def step_conversation_shows_preview(context: Any, expected: str) -> None: + """Assert the result is just the echoed input text (no-facade path).""" + assert expected in context._result_text, ( + f"Expected {expected!r} in result, got: {context._result_text!r}" + ) + + +@then("the conversation should show the no-actor error message") +def step_conversation_shows_no_actor(context: Any) -> None: + """Assert the no-actor friendly message is returned.""" + assert "No actor configured" in context._result_text, ( + f"Expected no-actor message in result, got: {context._result_text!r}" + ) + + +@then("the conversation should show an error message") +def step_conversation_shows_error(context: Any) -> None: + """Assert an [Error] tag is in the result.""" + assert "[Error]" in context._result_text, ( + f"Expected [Error] in result, got: {context._result_text!r}" + ) + + +@then('the returned session_id should be "{expected}"') +def step_returned_session_id(context: Any, expected: str) -> None: + """Assert the session_id from _create_tui_session matches.""" + assert context._result_sid == expected, ( + f"Expected session_id={expected!r}, got {context._result_sid!r}" + ) + + +@then('the facade should have received a message/send request with actor "{actor}"') +def step_facade_received_request_with_actor(context: Any, actor: str) -> None: + """Assert the dispatch request included the given actor override.""" + context._facade.dispatch.assert_called_once() + request = context._facade.dispatch.call_args[0][0] + assert request.method == "message/send", ( + f"Expected method='message/send', got {request.method!r}" + ) + assert request.params.get("actor") == actor, ( + f"Expected actor={actor!r} in params, got {request.params.get('actor')!r}" + ) + + +@then("the conversation should show a session-not-found error message") +def step_conversation_shows_session_not_found(context: Any) -> None: + """Assert the session-not-found friendly message is returned.""" + assert "Session not found" in context._result_text, ( + f"Expected session-not-found message, got: {context._result_text!r}" + ) + + +@then("the conversation should show a database error message") +def step_conversation_shows_database_error(context: Any) -> None: + """Assert the database error message is returned.""" + assert "Database error" in context._result_text, ( + f"Expected database error message, got: {context._result_text!r}" + ) + + +@then("the formatted outcome should show an error message") +def step_worker_outcome_shows_error(context: Any) -> None: + """Assert _format_worker_outcome produced an [Error] string.""" + assert context._worker_outcome is not None, "Expected non-None outcome" + assert "[Error]" in context._worker_outcome, ( + f"Expected [Error] in outcome, got: {context._worker_outcome!r}" + ) + + +@then('the formatted outcome should be "{expected}"') +def step_worker_outcome_equals(context: Any, expected: str) -> None: + """Assert _format_worker_outcome returned the expected string.""" + assert context._worker_outcome == expected, ( + f"Expected {expected!r}, got {context._worker_outcome!r}" + ) + + +@then('the conversation should show "(no response)"') +def step_conversation_shows_no_response(context: Any) -> None: + """Assert the empty-response fallback text is present.""" + assert "(no response)" in context._result_text, ( + f"Expected '(no response)' in result, got: {context._result_text!r}" + ) + + +@then("the transcript entry should have markup escaped") +def step_transcript_entry_escaped(context: Any) -> None: + """Assert the transcript entry stored by on_input_submitted has escaped markup. + + Verifies that _render_transcript receives pre-escaped entries — i.e. that + on_input_submitted calls _escape() before storing in the transcript, not + just at render time. This replaces the previous test that only validated + escape() in isolation without checking the storage path. + """ + import re + + transcript = context._app_transcript + assert transcript, "Expected at least one transcript entry after submit" + entry = transcript[-1] # the most recently appended entry + + # The stored entry must NOT contain unescaped Rich markup tags. + assert not re.search(r"(? None: + """Assert _format_worker_outcome re-raises BaseException subclasses.""" + raised = False + err = context._worker_error + exc_type = type(err) + try: + _format_worker_outcome(result=None, error=err) + except exc_type: + raised = True + assert raised, f"_format_worker_outcome must re-raise {exc_type.__name__}" + + +@then("the formatted outcome should be None") +def step_formatted_outcome_none(context: Any) -> None: + """Assert _format_worker_outcome returns None when both inputs are None.""" + assert context._worker_outcome is None, ( + f"Expected None from _format_worker_outcome(None, None), " + f"got: {context._worker_outcome!r}" + ) + + +@then("the returned facade should be None") +def step_returned_facade_none(context: Any) -> None: + """Assert _build_tui_facade returns None when construction fails.""" + assert context._facade_result is None, ( + f"Expected _build_tui_facade to return None on failure, " + f"got: {context._facade_result!r}" + ) + + +@then('the session should have been created with actor "{actor}"') +def step_session_created_with_actor(context: Any, actor: str) -> None: + """Assert service.create() was called with the persona's actor.""" + context._mock_service.create.assert_called_once_with(actor_name=actor) diff --git a/features/tui_llm_dispatch.feature b/features/tui_llm_dispatch.feature new file mode 100644 index 000000000..e85e27aae --- /dev/null +++ b/features/tui_llm_dispatch.feature @@ -0,0 +1,91 @@ +Feature: TUI normal text input dispatches to LLM via A2A facade + As a user of the CleverAgents TUI + I want my normal text messages to be sent to an LLM actor + So that I can have a real conversation through the TUI interface + + Background: + Given the TUI app is initialised with a mock A2A facade and session + + Scenario: Normal text input dispatches to LLM and renders response + When I submit the text "hello there" + Then the facade should have received a message/send request + And the conversation should show the assistant response + + Scenario: Dispatch passes active persona actor as override + When I dispatch "hello there" overriding actor with "openai/gpt-4o" + Then the facade should have received a message/send request with actor "openai/gpt-4o" + + Scenario: Normal text with no facade falls back to preview mode + Given the TUI app is initialised without a facade + When I submit the text "hello there" + Then the conversation should show the preview text "hello there" + + Scenario: LLM dispatch handles SessionActorNotConfiguredError gracefully + Given the facade raises SessionActorNotConfiguredError + When I submit the text "hello there" + Then the conversation should show the no-actor error message + + Scenario: LLM dispatch handles generic facade error gracefully + Given the facade returns an A2A error response + When I submit the text "hello there" + Then the conversation should show an error message + + Scenario: _create_tui_session returns real session_id from service + Given a mock session service that returns session_id "abc-123" + When I call _create_tui_session with that service + Then the returned session_id should be "abc-123" + + Scenario: _create_tui_session falls back to default when service unavailable + Given the session service raises an exception + When I call _create_tui_session with that service + Then the returned session_id should be "default" + + Scenario: LLM dispatch handles SessionNotFoundError gracefully + Given the facade raises SessionNotFoundError + When I submit the text "hello" + Then the conversation should show a session-not-found error message + + Scenario: LLM dispatch handles DatabaseError gracefully + Given the facade raises DatabaseError + When I submit the text "hello" + Then the conversation should show a database error message + + Scenario: Worker error is surfaced in the conversation widget + When the worker completes with error "network timeout" + Then the formatted outcome should show an error message + + Scenario: Worker success result is passed through unchanged + When the worker completes with result "You: hi\n\nAssistant: hello" + Then the formatted outcome should be "You: hi\n\nAssistant: hello" + + Scenario: Empty assistant response shows fallback text + Given the TUI app is initialised with a facade that returns empty response + When I submit the text "hello" + Then the conversation should show "(no response)" + + Scenario: Rich markup in user input is escaped before storage in transcript + Given the TUI app is initialised with a mock A2A facade and session + When I submit the text "[bold]attack[/bold]" + Then the transcript entry should have markup escaped + + Scenario: _format_worker_outcome re-raises KeyboardInterrupt + When the worker completes with a KeyboardInterrupt error + Then _format_worker_outcome should re-raise it + + Scenario: _format_worker_outcome re-raises SystemExit + When the worker completes with a SystemExit error + Then _format_worker_outcome should re-raise it + + Scenario: _format_worker_outcome returns None when both result and error are None + When the worker completes with neither result nor error + Then the formatted outcome should be None + + Scenario: _build_tui_facade returns None when facade construction fails + When _build_tui_facade is called with a broken container + Then the returned facade should be None + + Scenario: _create_tui_session binds active persona actor at session creation + Given a mock session service that returns session_id "abc-123" + And a persona state with active actor "anthropic/claude-4-sonnet" + When I call _create_tui_session with that service and persona state + Then the session should have been created with actor "anthropic/claude-4-sonnet" diff --git a/robot/helper_tui_llm_dispatch.py b/robot/helper_tui_llm_dispatch.py new file mode 100644 index 000000000..1b90aa4e1 --- /dev/null +++ b/robot/helper_tui_llm_dispatch.py @@ -0,0 +1,285 @@ +"""Helper script for tui_llm_dispatch.robot integration tests. + +Each subcommand is a self-contained check that prints a sentinel on success. + +Uses a stub LLM and real SQLite DB — no API keys required. +Follows the same fixture pattern as helper_session_tell_llm.py. +""" + +# ruff: noqa: E402 +from __future__ import annotations + +import sys +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +from cleveragents.application.services.session_service import PersistentSessionService +from cleveragents.application.services.session_workflow import SessionWorkflow +from cleveragents.infrastructure.database.models import Base +from cleveragents.infrastructure.database.repositories import ( + SessionMessageRepository, + SessionRepository, +) + +# --------------------------------------------------------------------------- +# Stub LLM — deterministic responses without real API calls +# --------------------------------------------------------------------------- + +_STUB_TEXT = "Hello from the TUI integration stub LLM." + + +class _StubLLM: + """Minimal LLM stub compatible with LangChain chat model interface.""" + + def invoke(self, messages: Any, **kwargs: Any) -> Any: + class _Resp: + content = _STUB_TEXT + + def __init__(self) -> None: + self.tool_calls: list[Any] = [] + self.response_metadata: dict[str, Any] = { + "usage": {"input_tokens": 5, "output_tokens": 10} + } + + return _Resp() + + +# --------------------------------------------------------------------------- +# Fixture helpers — mirrors helper_session_tell_llm.py pattern +# --------------------------------------------------------------------------- + + +def _make_db() -> tuple[Any, Any]: + """Create in-memory SQLite DB; return (session_factory, get_db callable).""" + engine = create_engine("sqlite:///:memory:", echo=False) + Base.metadata.create_all(engine) + factory = sessionmaker(bind=engine, expire_on_commit=False) + db_session = factory() + + def get_db() -> Any: + return db_session + + return factory, get_db + + +def _make_service(get_db: Any) -> PersistentSessionService: + """Build a PersistentSessionService from repository instances.""" + return PersistentSessionService( + session_repo=SessionRepository(get_db), + message_repo=SessionMessageRepository(get_db), + ) + + +def _make_workflow( + service: PersistentSessionService, +) -> SessionWorkflow: + """Build a SessionWorkflow using the stub LLM via llm_factory injection.""" + stub = _StubLLM() + return SessionWorkflow( + session_service=service, + llm_factory=lambda _actor_name: stub, + ) + + +# --------------------------------------------------------------------------- +# Subcommands +# --------------------------------------------------------------------------- + + +def cmd_create_session() -> None: + """Verify _create_tui_session() creates a real DB-backed session.""" + from cleveragents.tui import commands as cmd_mod + + _, get_db = _make_db() + service = _make_service(get_db) + container = MagicMock() + container.session_service.return_value = service + + with patch.object(cmd_mod, "get_container", return_value=container): + sid = cmd_mod._create_tui_session() + + assert sid != "default", "Expected a real session ID, got 'default'" + assert len(sid) > 0, "session_id must not be empty" + + session_obj = service.get(sid) + assert session_obj is not None, "Session not found in database" + assert session_obj.session_id == sid + + print("tui-create-session-ok") + + +def cmd_build_facade() -> None: + """Verify _build_tui_facade() returns a non-None A2aLocalFacade.""" + from cleveragents.a2a.facade import A2aLocalFacade + from cleveragents.tui import commands as cmd_mod + + _, get_db = _make_db() + service = _make_service(get_db) + container = MagicMock() + container.session_service.return_value = service + container.plan_lifecycle_service.side_effect = RuntimeError("not needed") + container.resource_registry_service.side_effect = RuntimeError("not needed") + container.tool_registry.side_effect = RuntimeError("not needed") + + with ( + patch.object(cmd_mod, "get_container", return_value=container), + patch("cleveragents.a2a.cli_bootstrap._facade_instance", None), + ): + facade = cmd_mod._build_tui_facade() + + assert facade is not None, "_build_tui_facade() must return a facade" + assert isinstance(facade, A2aLocalFacade), ( + f"Expected A2aLocalFacade, got {type(facade)}" + ) + print("tui-build-facade-ok") + + +def cmd_dispatch_message() -> None: + """Verify facade.dispatch(message/send) with stub LLM persists and responds.""" + from cleveragents.a2a.facade import A2aLocalFacade + from cleveragents.a2a.models import A2aRequest + + _, get_db = _make_db() + service = _make_service(get_db) + workflow = _make_workflow(service) + + facade = A2aLocalFacade( + services={"session_service": service, "session_workflow": workflow} + ) + + session = service.create(actor_name="openai/gpt-4o") + session_id = session.session_id + + request = A2aRequest( + method="message/send", + params={"session_id": session_id, "message": "hello integration"}, + ) + response = facade.dispatch(request) + + assert response.error is None, f"Unexpected error: {response.error}" + assert response.result is not None, "Expected result dict" + assistant_msg: str = response.result.get("assistant_message", "") + assert len(assistant_msg) > 0, "Expected non-empty assistant message" + assert _STUB_TEXT in assistant_msg, ( + f"Expected stub text {_STUB_TEXT!r} in response, got: {assistant_msg!r}" + ) + + messages = service.get_messages(session_id) + # get_messages() returns SessionMessage objects — access .role attribute + roles = [m.role.value for m in messages] + assert "user" in roles, "User message not persisted" + assert "assistant" in roles, "Assistant message not persisted" + + print("tui-dispatch-message-ok") + + +def cmd_no_actor_error() -> None: + """Verify SessionActorNotConfiguredError is raised when no actor configured. + + The A2A facade re-raises domain exceptions (SessionActorNotConfiguredError, + SessionNotFoundError, DatabaseError) so callers can handle them by type. + This mirrors the CLI pattern in _facade_dispatch() in session.py. + """ + from cleveragents.a2a.facade import A2aLocalFacade + from cleveragents.a2a.models import A2aRequest + from cleveragents.domain.models.core.session import ( + SessionActorNotConfiguredError, + ) + + _, get_db = _make_db() + service = _make_service(get_db) + workflow = SessionWorkflow(session_service=service) + + facade = A2aLocalFacade( + services={"session_service": service, "session_workflow": workflow} + ) + + session = service.create(actor_name=None) + session_id = session.session_id + + request = A2aRequest( + method="message/send", + params={"session_id": session_id, "message": "hello"}, + ) + + try: + facade.dispatch(request) + raise AssertionError("Expected SessionActorNotConfiguredError to be raised") + except SessionActorNotConfiguredError: + pass # expected — facade re-raises domain exceptions + + print("tui-no-actor-error-ok") + + +def cmd_database_error() -> None: + """Verify _run_llm_dispatch returns a friendly message on DatabaseError. + + Simulates a DatabaseError raised by the facade and confirms the TUI + dispatch function converts it to a user-readable error string rather + than propagating the exception. + """ + from unittest.mock import MagicMock + + from cleveragents.core.exceptions import DatabaseError + from cleveragents.tui.app import _run_llm_dispatch + + facade = MagicMock() + facade.dispatch.side_effect = DatabaseError("disk full") + + result = _run_llm_dispatch(facade, "test-session", "hello") + + assert "Database error" in result, ( + f"Expected 'Database error' in result, got: {result!r}" + ) + print("tui-database-error-ok") + + +def cmd_session_not_found() -> None: + """Verify _run_llm_dispatch returns a friendly message on SessionNotFoundError. + + Simulates the session being deleted between creation and dispatch (e.g. + concurrent process or manual DB edit) and confirms the TUI surfaces a + clear error message rather than crashing. + """ + from unittest.mock import MagicMock + + from cleveragents.domain.models.core.session import SessionNotFoundError + from cleveragents.tui.app import _run_llm_dispatch + + facade = MagicMock() + facade.dispatch.side_effect = SessionNotFoundError("session gone") + + result = _run_llm_dispatch(facade, "test-session", "hello") + + assert "Session not found" in result, ( + f"Expected 'Session not found' in result, got: {result!r}" + ) + print("tui-session-not-found-ok") + + +# --------------------------------------------------------------------------- +# Dispatch +# --------------------------------------------------------------------------- + +COMMANDS = { + "create-session": cmd_create_session, + "build-facade": cmd_build_facade, + "dispatch-message": cmd_dispatch_message, + "no-actor-error": cmd_no_actor_error, + "database-error": cmd_database_error, + "session-not-found": cmd_session_not_found, +} + +if __name__ == "__main__": + if len(sys.argv) < 2 or sys.argv[1] not in COMMANDS: + print(f"Usage: {sys.argv[0]} [{' | '.join(COMMANDS)}]", file=sys.stderr) + sys.exit(1) + COMMANDS[sys.argv[1]]() diff --git a/robot/tui_llm_dispatch.robot b/robot/tui_llm_dispatch.robot new file mode 100644 index 000000000..b5889ee3f --- /dev/null +++ b/robot/tui_llm_dispatch.robot @@ -0,0 +1,82 @@ +*** Settings *** +Documentation Integration tests for TUI normal text input → A2A facade → LLM dispatch. +... +... Verifies that: +... - _create_tui_session() creates a real database-backed session +... - _build_tui_facade() produces a wired A2aLocalFacade +... - CleverAgentsTuiApp dispatches normal text to the facade +... and renders the assistant response +... +... Uses a FakeListLLM stub — no real API keys required. +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment With Database Isolation +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_tui_llm_dispatch.py + +*** Test Cases *** +TUI Create Session Returns Real Session ID + [Documentation] Verify _create_tui_session() creates a DB-backed session. + [Tags] tui_llm_dispatch + ${result}= Run Process ${PYTHON} ${HELPER} create-session + ... cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tui-create-session-ok + +TUI Build Facade Returns Wired Facade + [Documentation] Verify _build_tui_facade() returns a non-None A2aLocalFacade. + [Tags] tui_llm_dispatch + ${result}= Run Process ${PYTHON} ${HELPER} build-facade + ... cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tui-build-facade-ok + +TUI Normal Text Dispatches To Facade And Renders Response + [Documentation] Verify CleverAgentsTuiApp dispatches normal text to + ... the facade and updates the conversation widget with the + ... assistant response from a FakeListLLM. + [Tags] tui_llm_dispatch + ${result}= Run Process ${PYTHON} ${HELPER} dispatch-message + ... cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tui-dispatch-message-ok + +TUI No Actor Error Renders Friendly Message + [Documentation] Verify that when no actor is configured, + ... the TUI shows a friendly error rather than crashing. + [Tags] tui_llm_dispatch + ${result}= Run Process ${PYTHON} ${HELPER} no-actor-error + ... cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tui-no-actor-error-ok + +TUI DatabaseError Renders Friendly Message + [Documentation] Verify _run_llm_dispatch returns a user-readable error + ... string when a DatabaseError is raised during dispatch. + [Tags] tui_llm_dispatch + ${result}= Run Process ${PYTHON} ${HELPER} database-error + ... cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tui-database-error-ok + +TUI SessionNotFoundError Renders Friendly Message + [Documentation] Verify _run_llm_dispatch returns a user-readable error + ... string when the session is missing at dispatch time. + [Tags] tui_llm_dispatch + ${result}= Run Process ${PYTHON} ${HELPER} session-not-found + ... cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tui-session-not-found-ok diff --git a/src/cleveragents/tui/app.py b/src/cleveragents/tui/app.py index 68401dedd..4734ff2cb 100644 --- a/src/cleveragents/tui/app.py +++ b/src/cleveragents/tui/app.py @@ -4,9 +4,18 @@ from __future__ import annotations import importlib import os -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any, ClassVar, Protocol +import structlog +from rich.markup import escape as _escape + +from cleveragents.a2a.models import A2aRequest +from cleveragents.core.exceptions import DatabaseError +from cleveragents.domain.models.core.session import ( + SessionActorNotConfiguredError, + SessionNotFoundError, +) from cleveragents.tui.first_run import create_default_persona_for_actor, is_first_run from cleveragents.tui.input.modes import InputMode, InputModeRouter from cleveragents.tui.input.reference_parser import suggestions @@ -23,9 +32,129 @@ from cleveragents.tui.widgets.reference_picker import ReferencePickerOverlay from cleveragents.tui.widgets.slash_command_overlay import SlashCommandOverlay if TYPE_CHECKING: - InputSubmittedEvent = Any -else: - InputSubmittedEvent = Any + from cleveragents.a2a.facade import A2aLocalFacade + +InputSubmittedEvent = Any + +logger = structlog.get_logger(__name__) + +# Sentinel session ID used when the real DB session cannot be created. +_FALLBACK_SESSION_ID = "default" +# Session ID key used to look up the active persona before the real session +# exists (persona resolution happens before session creation in run_tui). +_PRE_SESSION_PERSONA_KEY = "default" + +# Separator rendered between conversation exchanges in the Static widget. +_TRANSCRIPT_SEPARATOR = "\n\n" + "─" * 48 + "\n\n" + + +def _run_llm_dispatch( + facade: A2aLocalFacade, + session_id: str, + message: str, + actor_override: str | None = None, +) -> str: + """Dispatch *message* to the LLM via the A2A facade and return display text. + + This is a pure module-level function (no Textual dependency) so it can be + unit-tested directly without a running event loop or mocked Textual. + + Parameters + ---------- + facade: + The wired A2A local facade. + session_id: + Database-backed session ULID. + message: + User message text (with ``@`` references already expanded). + actor_override: + When set, overrides the session's default actor for this request. + Used to pass the active persona's actor on every dispatch so that + persona cycling (``tab`` / ``action_cycle_preset``) is reflected + immediately without requiring a session restart. + + Returns a formatted string ready for the conversation widget: + - On success: ``"You: \\n\\nAssistant: "`` + - On no-actor / session-not-found / database error: friendly guidance + - On any other error: ``"[Error] "`` + + Note on the catch-all ``except Exception`` branch (marked + ``# pragma: no cover``): the three specific exception types above cover + all expected A2A / domain failure modes. The catch-all is a safety net + for genuinely unexpected errors (e.g. ``MemoryError`` subclasses that + ARE ``Exception``); it converts them to a user-visible ``[Error]`` + string so the TUI stays alive rather than crashing. This branch is + intentionally excluded from coverage because triggering it requires an + exception type that is neither a domain error nor a BaseException + subclass — effectively impossible in the current call chain. + """ + params: dict[str, Any] = {"session_id": session_id, "message": message} + if actor_override: + params["actor"] = actor_override + + try: + response = facade.dispatch(A2aRequest(method="message/send", params=params)) + if response.error is not None: + logger.warning( + "tui.llm_dispatch.a2a_error", + session_id=session_id, + error=response.error.message, + ) + return f"[Error] {response.error.message}" + result_data: dict[str, Any] = response.result or {} + assistant_text: str = ( + result_data.get("assistant_message", "") or "(no response)" + ) + return f"You: {message}\n\nAssistant: {assistant_text}" + except SessionActorNotConfiguredError: + logger.info( + "tui.llm_dispatch.no_actor", session_id=session_id, actor=actor_override + ) + return "No actor configured. Use /persona set or select an actor first." + except SessionNotFoundError: + logger.warning("tui.llm_dispatch.session_not_found", session_id=session_id) + return ( + "[Error] Session not found. Please restart the TUI to create a new session." + ) + except DatabaseError as exc: + logger.exception("tui.llm_dispatch.database_error", session_id=session_id) + return f"[Error] Database error: {exc}" + except Exception as exc: # pragma: no cover + logger.exception("tui.llm_dispatch.unexpected_error", session_id=session_id) + return f"[Error] {exc}" + + +def _format_worker_outcome( + result: str | None, + error: BaseException | None, +) -> str | None: + """Translate a completed Textual worker's outcome into a display string. + + This module-level helper is extracted from the ``_on_llm_done`` closure + so it can be unit-tested independently of the Textual event loop. + + ``BaseException`` subclasses that are not ``Exception`` (e.g. + ``KeyboardInterrupt``, ``SystemExit``) are re-raised rather than + stringified so they can propagate correctly to the Textual runtime. + + Returns + ------- + str | None + A string to display in the conversation widget, or ``None`` if there + is nothing to show (both *result* and *error* are ``None``). + The ``return None`` branch is marked ``# pragma: no cover`` because + Textual guarantees either ``result`` or ``error`` is set when the + worker completes; the branch is unreachable in practice. + """ + if result is not None: + return result + if error is not None: + if not isinstance(error, Exception): + raise error # re-raise KeyboardInterrupt, SystemExit, etc. + logger.warning("tui.worker.error", error=str(error)) + return f"[Error] {error}" + return None # pragma: no cover + _TEXTUAL_AVAILABLE = False _TextualApp: type[Any] = object @@ -57,7 +186,7 @@ class SessionView: """Minimal per-session TUI view model.""" session_id: str - transcript: list[str] + transcript: list[str] = field(default_factory=list) class _CommandRouter(Protocol): @@ -72,9 +201,14 @@ class _FallbackCleverAgentsTuiApp: # pragma: no cover """Fallback app that raises actionable dependency error.""" def __init__( - self, *, command_router: _CommandRouter, persona_state: PersonaState + self, + *, + command_router: _CommandRouter, + persona_state: PersonaState, + facade: A2aLocalFacade | None = None, + session_id: str = _FALLBACK_SESSION_ID, ) -> None: - del command_router, persona_state + del command_router, persona_state, facade, session_id def run(self) -> None: raise RuntimeError( @@ -101,11 +235,21 @@ if _TEXTUAL_AVAILABLE: *, command_router: _CommandRouter, persona_state: PersonaState, + facade: A2aLocalFacade | None = None, + session_id: str = _FALLBACK_SESSION_ID, ) -> None: super().__init__() self._command_router = command_router self._persona_state = persona_state - self._session = SessionView(session_id="default", transcript=[]) + self._facade = facade + self._session = SessionView(session_id=session_id) + # Monotonically increasing counter — incremented on each dispatch. + # _on_llm_done checks it to discard callbacks from cancelled workers + # (exclusive=True cancels in-flight workers; their done_callback still + # fires and must not corrupt the transcript with stale data). + self._dispatch_gen: int = 0 + # Cached after mount to avoid repeated query_one() on every submit. + self._conversation: Any = None def compose(self) -> Any: yield _Header(show_clock=True) @@ -137,11 +281,22 @@ if _TEXTUAL_AVAILABLE: actor_overlay.show() else: actor_overlay.hide() + # Cache the conversation widget to avoid repeated query_one() on + # every on_input_submitted call. + self._conversation = self.query_one("#conversation", _Static) + # Explicitly focus the prompt so keyboard input is received immediately. + # Textual does not auto-focus any widget at startup; without this call + # the inner Input never gets focus and typing appears to do nothing. + prompt = self.query_one("#prompt", PromptInput) + prompt.focus() def _complete_first_run(self, actor: str) -> None: """Persist the chosen actor as the default persona and refresh the bar.""" create_default_persona_for_actor(self._persona_state.registry, actor) self._refresh_persona_bar() + # Refocus prompt after first-run overlay closes. + prompt = self.query_one("#prompt", PromptInput) + prompt.focus() def action_help(self) -> None: prompt = self.query_one("#prompt", PromptInput) @@ -166,6 +321,40 @@ if _TEXTUAL_AVAILABLE: scope_text=scope_text, ) + def _render_transcript( + self, conversation: Any, *, thinking: bool = False + ) -> None: + """Render the accumulated transcript into the conversation widget. + + Transcript entries are pre-escaped when stored, so this method + only needs to join them — avoiding O(N) re-escaping of the full + history on every render call. + + Accumulates all exchanges so multi-turn conversations remain visible + as the session grows (spec §29269: scrollable message stream). + + Spec gap (acknowledged, intentional simplification): the spec calls + for a per-message ``VerticalScroll`` with ``RichLog`` widgets and + block-cursor navigation (§29269). This PR uses a single ``_Static`` + widget with accumulated text to keep the scope focused on LLM + dispatch wiring. A follow-up task should replace this with a + ``RichLog``-based implementation for proper scrolling and per-message + metadata. + + TODO: implement conversation pruning per spec §30492 to bound memory + usage and rendering latency for long-running sessions. + """ + # Entries are already escaped when appended — join only. + entries = list(self._session.transcript) + if thinking and entries: + entries[-1] = entries[-1] + "\n\n⏳ Thinking..." + elif thinking: + entries = [_escape("⏳ Thinking...")] + text = ( + _TRANSCRIPT_SEPARATOR.join(entries) if entries else "CleverAgents TUI" + ) + conversation.update(text) + def on_input_submitted(self, event: InputSubmittedEvent) -> None: del event prompt = self.query_one("#prompt", PromptInput) @@ -184,7 +373,11 @@ if _TEXTUAL_AVAILABLE: ), ) result = mode_router.process(text) - conversation = self.query_one("#conversation", _Static) + # Use the cached widget (set in on_mount) to avoid repeated + # query_one() lookups on every submission. + conversation = self._conversation or self.query_one( + "#conversation", _Static + ) if result.mode == InputMode.COMMAND: conversation.update(result.command_result or "") @@ -198,16 +391,84 @@ if _TEXTUAL_AVAILABLE: output = ( shell.stdout.strip() or shell.stderr.strip() or "(empty output)" ) - conversation.update(f"$ {shell.command}\n{output}") + conversation.update(_escape(f"$ {shell.command}\n{output}")) return - preview = result.expanded_text + expanded = result.expanded_text if "@" in text: ref_picker = self.query_one("#reference-picker", ReferencePickerOverlay) ref_picker.set_suggestions( text, suggestions(text.replace("@", "").strip()) ) - conversation.update(preview) + + if self._facade is None: + # Facade not wired yet — preview only (graceful degradation) + conversation.update(_escape(expanded)) + return + + # Pre-escape the entry on storage so _render_transcript only joins + # (avoids O(N) re-escaping of the full history on every render). + self._session.transcript.append(_escape(f"You: {expanded}")) + self._render_transcript(conversation, thinking=True) + + # Run blocking LLM call off the main Textual thread so the + # event loop stays responsive during the API round-trip. + # exclusive=True serialises dispatches: a new message cancels + # any in-flight worker, preventing concurrent writes to the + # same session and conversation widget thrashing. + # Pass the active persona's actor as an override on every + # request so persona cycling (tab) takes effect immediately. + # + # Note on testability: the composition of run_worker(thread=True, + # exclusive=True) + done_callback wiring below is intentionally + # NOT covered by Behave unit tests because it requires a running + # Textual event loop. The constituent parts ARE tested in isolation: + # _run_llm_dispatch() via tui_llm_dispatch.feature, and + # _format_worker_outcome() via its own scenarios. Manual + # verification confirmed the full path works end-to-end. + facade = self._facade + session_id = self._session.session_id + actor = self._persona_state.active_persona(session_id).actor or None + transcript = self._session.transcript + + # Capture the current generation so the callback can detect if a + # newer dispatch has superseded this one (exclusive=True cancels the + # in-flight worker but its done_callback still fires — without this + # guard the cancelled callback would overwrite transcript[-1] with + # stale data from the older request). + self._dispatch_gen += 1 + current_gen = self._dispatch_gen + + def _dispatch_llm() -> str: + return _run_llm_dispatch(facade, session_id, expanded, actor) + + def _on_llm_done(worker: Any) -> None: + """Accumulate outcome into transcript and re-render.""" + # Discard callbacks from cancelled workers. + # worker.is_cancelled is a public bool property on Textual's + # Worker — no WorkerState import needed (Textual is optional). + if getattr(worker, "is_cancelled", False): + return + # Also discard if a newer dispatch has already started. + if self._dispatch_gen != current_gen: + return + + outcome = _format_worker_outcome(worker.result, worker.error) + if outcome is not None and transcript: + # Pre-escape and store; replace the placeholder entry. + # outcome is "You: {msg}\n\nAssistant: {reply}" on success + # or an error string — always prefix with user message so + # the exchange is self-contained in the transcript. + if outcome.startswith("You: "): + transcript[-1] = _escape(outcome) + else: + transcript[-1] = _escape(f"You: {expanded}\n\n{outcome}") + self._render_transcript(conversation, thinking=False) + + worker = self.run_worker( + _dispatch_llm, thread=True, exclusive=True, name="llm-dispatch" + ) + worker.done_callback = _on_llm_done _ResolvedTuiApp = _TextualCleverAgentsTuiApp diff --git a/src/cleveragents/tui/commands.py b/src/cleveragents/tui/commands.py index d155ef726..70898df05 100644 --- a/src/cleveragents/tui/commands.py +++ b/src/cleveragents/tui/commands.py @@ -2,19 +2,52 @@ from __future__ import annotations +import contextlib import json from collections import defaultdict from collections.abc import Callable from dataclasses import dataclass, field from pathlib import Path -from typing import Any +from typing import TYPE_CHECKING, Any from cleveragents.application.container import get_container -from cleveragents.tui.app import CleverAgentsTuiApp, textual_available +from cleveragents.tui.app import ( + _FALLBACK_SESSION_ID, + _PRE_SESSION_PERSONA_KEY, + CleverAgentsTuiApp, + textual_available, +) from cleveragents.tui.persona.registry import PersonaRegistry from cleveragents.tui.persona.state import PersonaState from cleveragents.tui.slash_catalog import SLASH_COMMAND_SPECS +if TYPE_CHECKING: + from cleveragents.a2a.facade import A2aLocalFacade + from cleveragents.application.services.session_service import SessionService + +# Module-level singleton for the TUI session service — mirrors the CLI pattern +# in cli/commands/session.py (_get_session_service). Both _build_tui_facade() +# and _create_tui_session() must share the same SessionService instance so they +# use the same SQLAlchemy engine and sessionmaker rather than creating two +# separate engines against the same SQLite file (providers.Factory creates a +# new instance on every call). +_tui_session_service: SessionService | None = None + + +def _get_tui_session_service() -> SessionService | None: + """Return the process-wide TUI SessionService, creating it on first call. + + Returns ``None`` if the session service cannot be initialised (e.g. + missing database URL), so callers degrade gracefully. + """ + global _tui_session_service + if _tui_session_service is None: + with contextlib.suppress( + RuntimeError, ValueError, ImportError, AttributeError, OSError + ): + _tui_session_service = get_container().session_service() + return _tui_session_service + @dataclass(slots=True) class TuiCommandRouter: @@ -223,6 +256,76 @@ class TuiCommandRouter: return f"Import failed: {exc}" +def _build_tui_facade() -> A2aLocalFacade | None: + """Build and wire an :class:`~cleveragents.a2a.facade.A2aLocalFacade` for the TUI. + + Follows the same pattern as ``_facade_dispatch`` in + ``cli/commands/session.py``: the facade is retrieved via + :func:`~cleveragents.a2a.cli_bootstrap.get_facade` and the session + workflow is registered so ``message/send`` resolves through the real + LLM stack. Any failure is suppressed so the TUI degrades gracefully + to preview-only mode rather than crashing. + """ + + try: + from cleveragents.a2a.cli_bootstrap import get_facade + from cleveragents.application.services.session_workflow import SessionWorkflow + + facade = get_facade() + session_svc = _get_tui_session_service() + if session_svc is not None: + facade.register_service("session_service", session_svc) + + try: + provider_registry = None + try: + from cleveragents.providers.registry import get_provider_registry + + provider_registry = get_provider_registry() + except (ImportError, RuntimeError, OSError): + pass + if session_svc is not None: + facade.register_service( + "session_workflow", + SessionWorkflow( + session_service=session_svc, + provider_registry=provider_registry, + ), + ) + except (RuntimeError, ValueError, ImportError, AttributeError, OSError): + pass + return facade + except (ImportError, RuntimeError, AttributeError, OSError): # pragma: no cover + return None + + +def _create_tui_session(persona_state: PersonaState | None = None) -> str: + """Create a real database-backed TUI session and return its session_id. + + When *persona_state* is provided, the active persona's actor is bound to + the session at creation time so the first message dispatches to the LLM + without hitting ``SessionActorNotConfiguredError``. + + Falls back to ``_FALLBACK_SESSION_ID`` if the session service is + unavailable so the TUI can still launch without a database. + """ + actor_name: str | None = None + if persona_state is not None: + with contextlib.suppress(ValueError, AttributeError, KeyError): + actor_name = ( + persona_state.active_persona(_PRE_SESSION_PERSONA_KEY).actor or None + ) + + service = _get_tui_session_service() + if service is None: + return _FALLBACK_SESSION_ID + try: + session = service.create(actor_name=actor_name) + return session.session_id + except (RuntimeError, ValueError, AttributeError, OSError): + return _FALLBACK_SESSION_ID + + def run_tui(*, headless: bool = False) -> int: """Run the Textual TUI app or a headless startup check.""" container = get_container() @@ -236,12 +339,20 @@ def run_tui(*, headless: bool = False) -> int: "persona_count": len(registry.list_personas()) if registry.personas_dir.exists() else 0, - "default_persona": state.active_name("default"), - "help": router.handle("help", session_id="default"), + "default_persona": state.active_name(_PRE_SESSION_PERSONA_KEY), + "help": router.handle("help", session_id=_PRE_SESSION_PERSONA_KEY), } print(json.dumps(payload, indent=2)) return 0 - app = CleverAgentsTuiApp(command_router=router, persona_state=state) + facade = _build_tui_facade() + session_id = _create_tui_session(persona_state=state) + + app = CleverAgentsTuiApp( + command_router=router, + persona_state=state, + facade=facade, + session_id=session_id, + ) app.run() return 0