diff --git a/features/steps/tui_llm_dispatch_steps.py b/features/steps/tui_llm_dispatch_steps.py index ee9f3b815..af86e43d2 100644 --- a/features/steps/tui_llm_dispatch_steps.py +++ b/features/steps/tui_llm_dispatch_steps.py @@ -17,7 +17,7 @@ from unittest.mock import MagicMock, patch from behave import given, then, when -from cleveragents.tui.app import _run_llm_dispatch +from cleveragents.tui.app import _format_worker_outcome, _run_llm_dispatch # --------------------------------------------------------------------------- @@ -57,9 +57,7 @@ def step_init_without_facade(context: Any) -> None: @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, - ) + from cleveragents.domain.models.core.session import SessionActorNotConfiguredError facade = MagicMock() facade.dispatch.side_effect = SessionActorNotConfiguredError("no actor") @@ -67,6 +65,28 @@ def step_facade_raises_no_actor(context: Any) -> None: 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.""" @@ -109,6 +129,7 @@ def step_session_service_raises(context: Any) -> None: @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 @@ -121,11 +142,26 @@ def step_submit_text(context: Any, text: str) -> None: @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 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 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.""" @@ -174,7 +210,10 @@ def step_facade_received_request(context: Any) -> None: f"Expected method='message/send', got {request.method!r}" ) assert request.params["session_id"] == context._session_id - assert request.params["message"] == "hello there" + 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") @@ -230,6 +269,39 @@ def step_facade_received_request_with_actor(context: Any, actor: str) -> None: ) +@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 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.""" diff --git a/features/tui_llm_dispatch.feature b/features/tui_llm_dispatch.feature index 8b86f0b34..b02bcad85 100644 --- a/features/tui_llm_dispatch.feature +++ b/features/tui_llm_dispatch.feature @@ -40,6 +40,24 @@ Feature: TUI normal text input dispatches to LLM via A2A facade 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: _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" diff --git a/robot/helper_tui_llm_dispatch.py b/robot/helper_tui_llm_dispatch.py index f6f966f04..dd4ff5acb 100644 --- a/robot/helper_tui_llm_dispatch.py +++ b/robot/helper_tui_llm_dispatch.py @@ -168,6 +168,9 @@ def cmd_dispatch_message() -> None: 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 diff --git a/src/cleveragents/tui/app.py b/src/cleveragents/tui/app.py index c7177e7c8..d5dd43b63 100644 --- a/src/cleveragents/tui/app.py +++ b/src/cleveragents/tui/app.py @@ -57,11 +57,15 @@ def _run_llm_dispatch( Returns a formatted string ready for the conversation widget: - On success: ``"You: \\n\\nAssistant: "`` - - On no-actor: friendly guidance message + - On no-actor / session-not-found / database error: friendly guidance - On any other error: ``"[Error] "`` """ from cleveragents.a2a.models import A2aRequest - from cleveragents.domain.models.core.session import SessionActorNotConfiguredError + from cleveragents.core.exceptions import DatabaseError + from cleveragents.domain.models.core.session import ( + SessionActorNotConfiguredError, + SessionNotFoundError, + ) params: dict[str, Any] = {"session_id": session_id, "message": message} if actor_override: @@ -76,10 +80,38 @@ def _run_llm_dispatch( return f"You: {message}\n\nAssistant: {assistant_text}" except SessionActorNotConfiguredError: return "No actor configured. Use /persona set or select an actor first." + except SessionNotFoundError: + return ( + "[Error] Session not found. Please restart the TUI to create a new session." + ) + except DatabaseError as exc: + return f"[Error] Database error: {exc}" except Exception as exc: # pragma: no cover 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. + + 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``). + """ + if result is not None: + return result + if error is not None: + return f"[Error] {error}" + return None # pragma: no cover + + _TEXTUAL_AVAILABLE = False _TextualApp: type[Any] = object _Vertical: type[Any] = object @@ -289,8 +321,9 @@ if _TEXTUAL_AVAILABLE: def _on_llm_done(worker: Any) -> None: """Update conversation widget once worker finishes.""" - if worker.result is not None: - conversation.update(worker.result) + text = _format_worker_outcome(worker.result, worker.error) + if text is not None: + conversation.update(text) worker = self.run_worker(_dispatch_llm, thread=True, exclusive=False) worker.done_callback = _on_llm_done