diff --git a/src/band/adapters/crewai.py b/src/band/adapters/crewai.py index 1a53d8ba7..f49924fe1 100644 --- a/src/band/adapters/crewai.py +++ b/src/band/adapters/crewai.py @@ -52,15 +52,29 @@ ) +# CrewAI offers no error type or code for an empty completion, so its message +# is the only discriminator. One definition, matched here and faked in tests. +EMPTY_LLM_RESPONSE_MARKER = "Invalid response from LLM call" + + +def _is_empty_llm_response(exc: Exception) -> bool: + """Whether ``exc`` is CrewAI reporting that an LLM call came back empty. + + ``crewai.utilities.agent_utils`` raises this bare ``ValueError`` for every + empty completion in its loop, not only the forced final-answer step — so a + match means "no text came back", never "the turn is healthy". + """ + return isinstance(exc, ValueError) and EMPTY_LLM_RESPONSE_MARKER in str(exc) + + def _silence_lite_agent_error_panel() -> None: """Deregister CrewAI's benign red "LiteAgent Failed" console panel. - The agent replies via the band_send_message tool, so CrewAI's post-tool step - returns an empty final answer and raises the "Invalid response from LLM call" - ValueError that on_message already swallows — yet its global console listener - prints an alarming panel anyway (regardless of verbose). Remove only that - handler; tracing and genuine errors are untouched. Idempotent (a later call - finds nothing) and best-effort (leave the panel if CrewAI internals move). + This agent answers only through band_send_message, so most turns end on an + empty completion and CrewAI's global console listener prints an alarming + panel anyway (regardless of verbose). Remove only that handler; tracing and + genuine errors are untouched. Idempotent (a later call finds nothing) and + best-effort (leave the panel if CrewAI internals move). """ try: # event_listener is imported for its side effect: registering the handlers. @@ -318,7 +332,7 @@ async def _process_message( *, is_session_bootstrap: bool, room_id: str, - reply_tracker: ReplyTracker | None = None, + reply_tracker: ReplyTracker, ) -> None: """Internal message processing logic.""" assert self._crewai_agent is not None, "on_message already checked this" @@ -393,63 +407,51 @@ async def _process_message( prompt = "\n\n".join(sections) result = await self._crewai_agent.kickoff_async(prompt) - if result and result.raw: - self._message_history[room_id].append( - { - "role": "assistant", - "content": result.raw, - } - ) + except Exception as e: + # An empty response is benign only once some tool ran this turn -- + # otherwise the model's very first call came back empty, which is + # indistinguishable from a genuine provider failure and must keep + # failing the delivery so the platform retries it. + if not (_is_empty_llm_response(e) and reply_tracker.any_tool_ran): + logger.error("Error processing message: %s", e, exc_info=True) + await self._report_error(tools, str(e)) + raise + # Keep the exception text: it is the only record that CrewAI raised, + # and this turn is no longer marked failed for the runtime to log. + logger.debug("Room %s: CrewAI returned no text: %s", room_id, e) + result = None + + final_text = (result.raw or "") if result else "" + if final_text: + self._message_history[room_id].append( + { + "role": "assistant", + "content": final_text, + } + ) - if not (reply_tracker is not None and reply_tracker.replied): - await self._report_error( - tools, - missing_reply_error( - "CrewAI", - detail=( - "Repeated tool failures may also have exhausted " - f"max_iter={self.max_iter}." - ), + if not reply_tracker.did_productive_work: + # Warn, not debug: nothing reached the room, and the delivery is + # still acked as processed, so this log is the only operator signal. + logger.warning( + "Room %s: CrewAI turn produced nothing for the room", room_id + ) + await self._report_error( + tools, + missing_reply_error( + "CrewAI", + detail=( + "Repeated tool failures may also have exhausted " + f"max_iter={self.max_iter}." ), - ) - - logger.info( - "Room %s: CrewAI agent completed (output_length=%s)", - room_id, - len(result.raw) if result and result.raw else 0, + ), ) - except Exception as e: - # CrewAI raises ValueError("Invalid response from LLM call - None or - # empty.") when its ReAct loop yields an empty final answer. In this - # adapter the agent acts via tools (band_send_message to reply, - # band_store_memory, etc.), so an empty final answer AFTER the agent - # already did productive work is benign noise — a reply went out, or a - # tool-only turn (e.g. a memory store the user told it not to follow - # with a message) completed and there is simply nothing left to say. - # Match that specific ValueError narrowly so genuine no-response - # failures (the LLM returned empty without doing anything) still - # surface as error events and propagate. - if ( - reply_tracker is not None - and (reply_tracker.replied or reply_tracker.tool_executed) - and isinstance(e, ValueError) - and "Invalid response from LLM call" in str(e) - ): - logger.warning( - "Room %s: CrewAI returned an empty final answer after the agent " - "already did productive work this turn; treating as non-fatal: %s", - room_id, - e, - ) - return - logger.error("Error processing message: %s", e, exc_info=True) - await self._report_error(tools, str(e)) - raise - - logger.debug( - "Message %s processed successfully (history now has %s messages)", + logger.info( + "Room %s: CrewAI turn over for %s (output=%s chars, history=%s)", + room_id, msg.id, + len(final_text), len(self._message_history[room_id]), ) diff --git a/src/band/adapters/pydantic_ai.py b/src/band/adapters/pydantic_ai.py index b45f3b7c2..d48ab550c 100644 --- a/src/band/adapters/pydantic_ai.py +++ b/src/band/adapters/pydantic_ai.py @@ -1032,9 +1032,10 @@ async def on_message( # other response the run cannot turn into output can still spend the # refused output budget. Once a terminal tool has run (a # band_send_message reply, a band_store_memory, ...) the work already went - # out, so that exhaustion is benign — mirror the crewai adapter and - # swallow it. Genuine no-response failures (no terminal tool ran — only - # read-only lookups or failed tools) still propagate. + # out, so that exhaustion is benign — swallow it. Genuine no-response + # failures (no terminal tool ran — only read-only lookups or failed + # tools) still propagate here, unlike the crewai adapter, which cannot + # tell them apart from the empty completion that ends its every turn. if tool_executed and _is_output_retries_exhausted(e): logger.warning( "Room %s: Pydantic AI exhausted its output retries after " diff --git a/src/band/integrations/crewai/reporting.py b/src/band/integrations/crewai/reporting.py index 1216728da..d8a56adeb 100644 --- a/src/band/integrations/crewai/reporting.py +++ b/src/band/integrations/crewai/reporting.py @@ -26,11 +26,18 @@ class ReplyTracker: """Mutable per-turn markers shared (by reference) with the tool wrappers. ``replied`` flips once ``band_send_message`` succeeds; ``tool_executed`` flips - once any terminal tool succeeds. + once any terminal tool succeeds; ``any_tool_ran`` flips on any tool call at + all, success or failure, terminal or not. """ replied: bool = False tool_executed: bool = False + any_tool_ran: bool = False + + @property + def did_productive_work(self) -> bool: + """Whether the turn left something behind for the room to show for it.""" + return self.replied or self.tool_executed @dataclass(frozen=True) diff --git a/src/band/integrations/crewai/tools.py b/src/band/integrations/crewai/tools.py index d3c0c10b6..351e5df01 100644 --- a/src/band/integrations/crewai/tools.py +++ b/src/band/integrations/crewai/tools.py @@ -132,8 +132,12 @@ def _mark_productive_work( CrewAI raises on an empty final answer; that is a genuine no-response failure only when nothing terminal ran. ``is_terminal_success`` is the shared rule for what counts (read-only Band tools and undeclared custom - tools do not). + tools do not) toward the missing-reply decision. ``any_tool_ran`` is + separate and coarser: it flips on this call alone, whether the tool + succeeded or not, so a turn that tried *something* is never mistaken for + one where the model's very first response came back empty. """ + tracker.any_tool_ran = True try: if json.loads(result).get("status") != "success": return diff --git a/tests/adapters/test_crewai_adapter.py b/tests/adapters/test_crewai_adapter.py index 6c4cf66df..92c53642a 100644 --- a/tests/adapters/test_crewai_adapter.py +++ b/tests/adapters/test_crewai_adapter.py @@ -24,8 +24,13 @@ import pytest from pydantic import BaseModel, Field +from band.adapters.crewai import EMPTY_LLM_RESPONSE_MARKER from band.core.types import Capability, Emit, PlatformMessage from band.runtime.prompts import render_system_prompt +from band.runtime.tools import BandTool, missing_reply_error + +# The exact text CrewAI raises; the marker is the part the adapter matches on. +EMPTY_LLM_RESPONSE_ERROR = f"{EMPTY_LLM_RESPONSE_MARKER} - None or empty." if TYPE_CHECKING: from band.adapters.crewai import CrewAIAdapter as CrewAIAdapterType @@ -39,6 +44,15 @@ def __init__(self): pass +def error_events(mock_tools: Any) -> list[str]: + """The content of every error event the adapter posted to the room.""" + return [ + call.kwargs["content"] + for call in mock_tools.send_event.await_args_list + if call.kwargs.get("message_type") == "error" + ] + + @pytest.fixture def crewai_mocks(monkeypatch): @@ -557,20 +571,24 @@ async def test_suppresses_empty_final_answer_after_reply( """CrewAI raising an empty final answer AFTER the agent already replied via band_send_message is non-fatal: no error event, no re-raise. - Regression: CrewAI 1.14.3 raises ValueError("Invalid response from LLM - call - None or empty.") on its forced final-answer step. Because this - adapter replies through the tool, that fired on essentially every turn, - posting a spurious error event alongside each (successful) reply. + This adapter replies only through band_send_message, so CrewAI's + empty-final-answer ValueError fires on essentially every turn -- + including this successful one. Routes through the real + ``_mark_productive_work`` (not hand-set flags) so the test fails if a + future change stops SEND_MESSAGE from marking a turn productive. """ module = importlib.import_module("band.adapters.crewai") - - async def _kickoff(_messages): - # Simulate band_send_message having succeeded earlier this turn. - tracker = module._reply_tracker_var.get() - if tracker is not None: - tracker.replied = True - raise ValueError("Invalid response from LLM call - None or empty.") + tools_module = importlib.import_module("band.integrations.crewai.tools") + + async def _kickoff(_prompt): + tools_module._mark_productive_work( + module._reply_tracker_var.get(), + BandTool.SEND_MESSAGE, + json.dumps({"status": "success"}), + custom_terminal=False, + ) + raise ValueError(EMPTY_LLM_RESPONSE_ERROR) mock_crewai_agent.kickoff_async = AsyncMock(side_effect=_kickoff) @@ -603,18 +621,22 @@ async def test_suppresses_empty_final_answer_after_tool_only_turn( nothing left to say — CrewAI then raises ValueError("Invalid response from LLM call - None or empty.") on its forced final-answer step. Because a tool already executed successfully this turn, that empty answer is - benign: no error event, no re-raise. + benign: no error event, no re-raise. Routes through the real + ``_mark_productive_work`` (not hand-set flags) so the test fails if a + future change stops a terminal tool from marking a turn productive. """ module = importlib.import_module("band.adapters.crewai") - - async def _kickoff(_messages): - # Simulate a non-reply tool (e.g. band_store_memory) having succeeded - # earlier this turn — tool_executed flips, replied does not. - tracker = module._reply_tracker_var.get() - if tracker is not None: - tracker.tool_executed = True - raise ValueError("Invalid response from LLM call - None or empty.") + tools_module = importlib.import_module("band.integrations.crewai.tools") + + async def _kickoff(_prompt): + tools_module._mark_productive_work( + module._reply_tracker_var.get(), + BandTool.STORE_MEMORY, + json.dumps({"status": "success"}), + custom_terminal=False, + ) + raise ValueError(EMPTY_LLM_RESPONSE_ERROR) mock_crewai_agent.kickoff_async = AsyncMock(side_effect=_kickoff) @@ -636,6 +658,126 @@ async def _kickoff(_messages): # No error event posted to the room. mock_tools.send_event.assert_not_called() + @pytest.mark.asyncio + async def test_read_only_turn_with_empty_final_answer_completes( + self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent + ): + """A turn whose only work was read-only must finish, not fail the delivery. + + Told to run read tools and nothing else ("call band_list_tasks ... do not + call any other tool"), the agent does exactly that and stays silent: + fetching state is not terminal work, so the real marker leaves both + tracker flags down. Re-raising CrewAI's empty-response ValueError would + mark the delivery failed for a turn that did precisely what it was asked. + """ + module = importlib.import_module("band.adapters.crewai") + tools_module = importlib.import_module("band.integrations.crewai.tools") + + async def _kickoff(_prompt): + # Route through the real marker so the read-only classification -- + # not a hand-set flag -- is what keeps this turn "unproductive". + tools_module._mark_productive_work( + module._reply_tracker_var.get(), + BandTool.LIST_TASKS, + json.dumps({"status": "success", "data": []}), + custom_terminal=False, + ) + raise ValueError(EMPTY_LLM_RESPONSE_ERROR) + + mock_crewai_agent.kickoff_async = AsyncMock(side_effect=_kickoff) + + adapter = CrewAIAdapter() + await adapter.on_started("TestBot", "Test bot") + adapter._crewai_agent = mock_crewai_agent + + # Must NOT raise — the delivery completes. + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + # Exactly one event, carrying the shared missing-reply wording -- the + # room hears about the missing reply, not CrewAI's internal error. + [error] = error_events(mock_tools) + assert missing_reply_error("CrewAI") in error + mock_tools.send_event.assert_awaited_once() + + @pytest.mark.asyncio + async def test_empty_answer_with_no_tool_call_still_raises( + self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent + ): + """An empty answer with zero tool activity is a genuine failure, not a + finished turn -- it must still fail the delivery so the platform retries. + + Nothing distinguishes "the model correctly stopped after read-only + work" from "the very first LLM call came back empty" by the exception + alone -- both raise CrewAI's identical ValueError. ``any_tool_ran`` is + that distinction: it is the only turn where this adapter can rule out + a genuine no-response failure, so it is the only one that must not be + silently marked processed. + """ + mock_crewai_agent.kickoff_async = AsyncMock( + side_effect=ValueError(EMPTY_LLM_RESPONSE_ERROR) + ) + + adapter = CrewAIAdapter() + await adapter.on_started("TestBot", "Test bot") + adapter._crewai_agent = mock_crewai_agent + + with pytest.raises(ValueError, match=EMPTY_LLM_RESPONSE_ERROR): + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + mock_tools.send_event.assert_awaited_once() + + @pytest.mark.asyncio + async def test_no_missing_reply_error_after_clean_tool_only_return( + self, CrewAIAdapter, sample_message, mock_tools, mock_crewai_agent + ): + """Terminal tool work answers for a turn even when kickoff returns cleanly. + + The empty-response path is not the only ending without a reply: CrewAI + can also return normally after a tool-only turn. One rule judges both, so + neither posts a missing-reply error once terminal work has landed. + """ + module = importlib.import_module("band.adapters.crewai") + mock_result = MagicMock() + mock_result.raw = "Stored it." + + async def _kickoff(_prompt): + module._reply_tracker_var.get().tool_executed = True + return mock_result + + mock_crewai_agent.kickoff_async = AsyncMock(side_effect=_kickoff) + + adapter = CrewAIAdapter() + await adapter.on_started("TestBot", "Test bot") + adapter._crewai_agent = mock_crewai_agent + + await adapter.on_message( + msg=sample_message, + tools=mock_tools, + history=[], + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id="room-123", + ) + + assert error_events(mock_tools) == [] + @pytest.mark.asyncio @pytest.mark.parametrize( "error", diff --git a/tests/e2e/baseline/smoke/matrix/test_capability_matrix.py b/tests/e2e/baseline/smoke/matrix/test_capability_matrix.py index 81f571d5b..de15cb55e 100644 --- a/tests/e2e/baseline/smoke/matrix/test_capability_matrix.py +++ b/tests/e2e/baseline/smoke/matrix/test_capability_matrix.py @@ -137,9 +137,9 @@ async def test_recall_memory_across_memory_adapters( exclude=[ ExcludedAdapter( Adapter.CREWAI, - "the second, post-reboot retrieval turn returns an empty completion " - "('Invalid response from LLM call - None or empty'), so the turn never " - "finishes; reproduced on every attempt, not a transient", + "the second, post-reboot retrieval turn reads the memory but ends on " + "an empty completion before band_send_message runs, so no reply ever " + "reaches the room; reproduced on every attempt, not a transient", ) ], **MEMORY_AGENT,