mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-07-22 15:25:09 +00:00
fix(flow): stop replaying previous turn's intent when route_turn() returns falsy (#6505)
In conversational flows, a falsy return from an overridden route_turn() fell back to the sticky state.last_intent from a previous turn, silently re-running the prior turn's handler for an unhandled input. The fallback exists for the legacy default_intents path, where receive_user_message() classifies the intent fresh each turn. Track that per-turn classification in _turn_classified_intent (cleared on every turn reset) and route on it instead, so a falsy route_turn() now falls through to the built-in answer_from_history/converse defaults and never reuses stale routing state. Fixes EPD-176. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Lorenze Jay <63378463+lorenzejay@users.noreply.github.com>
This commit is contained in:
@@ -133,6 +133,7 @@ class _ConversationalMixin:
|
||||
_pending_user_message: str | dict[str, Any] | None
|
||||
_pending_intents: Sequence[str] | None
|
||||
_pending_intent_llm: str | BaseLLM | None
|
||||
_turn_classified_intent: str | None
|
||||
|
||||
def _clear_or_listeners(self) -> None:
|
||||
pass
|
||||
@@ -185,12 +186,22 @@ class _ConversationalMixin:
|
||||
)
|
||||
return configured_route
|
||||
|
||||
if state.last_intent:
|
||||
turn_intent = self._turn_classified_intent
|
||||
if turn_intent:
|
||||
state.last_intent = turn_intent
|
||||
self._emit_conversation_route_selected(
|
||||
state.last_intent,
|
||||
turn_intent,
|
||||
previous_intent=previous_intent,
|
||||
)
|
||||
return state.last_intent
|
||||
return turn_intent
|
||||
|
||||
if previous_intent:
|
||||
logger.debug(
|
||||
"route_turn() returned no route and no intent was classified "
|
||||
"this turn; ignoring stale last_intent=%r from a previous turn "
|
||||
"and falling back to built-in routing",
|
||||
previous_intent,
|
||||
)
|
||||
|
||||
if self.can_answer_from_history(context):
|
||||
state.last_intent = "answer_from_history"
|
||||
@@ -550,6 +561,11 @@ class _ConversationalMixin:
|
||||
supply per-route descriptions, or change the default/fallback intent.
|
||||
Override this method to bypass the LLM router entirely (e.g.,
|
||||
permission gates before the LLM decision).
|
||||
|
||||
Returning a falsy value means "no routing decision": the turn falls
|
||||
through to the built-in defaults (``answer_from_history`` when
|
||||
configured, else ``converse``). It never replays a previous turn's
|
||||
intent.
|
||||
"""
|
||||
config = self._conversation_config
|
||||
if config is None:
|
||||
@@ -722,6 +738,7 @@ class _ConversationalMixin:
|
||||
context=self.conversation_messages,
|
||||
)
|
||||
state.last_intent = intent
|
||||
object.__setattr__(self, "_turn_classified_intent", intent)
|
||||
return intent
|
||||
return text
|
||||
|
||||
@@ -788,6 +805,8 @@ class _ConversationalMixin:
|
||||
object.__setattr__(self, "_pending_intent_llm", None)
|
||||
if not hasattr(self, "_streaming_conversation_turn"):
|
||||
object.__setattr__(self, "_streaming_conversation_turn", False)
|
||||
if not hasattr(self, "_turn_classified_intent"):
|
||||
object.__setattr__(self, "_turn_classified_intent", None)
|
||||
|
||||
def _create_default_extension_state(self) -> ConversationState | None:
|
||||
initial_state_t = getattr(self, "_initial_state_t", None)
|
||||
@@ -852,6 +871,7 @@ class _ConversationalMixin:
|
||||
self._method_call_counts.clear()
|
||||
self._clear_or_listeners()
|
||||
self._is_execution_resuming = False
|
||||
object.__setattr__(self, "_turn_classified_intent", None)
|
||||
|
||||
def _apply_pending_conversational_turn(self) -> None:
|
||||
"""Drain the stashed user message + classify if intents configured.
|
||||
@@ -859,6 +879,7 @@ class _ConversationalMixin:
|
||||
Called from ``Flow.kickoff_async`` AFTER persist state restore so
|
||||
the appended message survives ``self.persistence.load_state(...)``.
|
||||
"""
|
||||
object.__setattr__(self, "_turn_classified_intent", None)
|
||||
if self._pending_user_message is None:
|
||||
return
|
||||
|
||||
|
||||
@@ -1555,6 +1555,115 @@ class TestConversationalFlow:
|
||||
)
|
||||
|
||||
|
||||
class TestFalsyRouteTurnFallback:
|
||||
"""A falsy ``route_turn()`` must never replay a previous turn's intent.
|
||||
|
||||
Regression tests for EPD-176: an overridden ``route_turn()`` returning
|
||||
``None`` on an unhandled input used to silently reuse the sticky
|
||||
``state.last_intent`` from the *previous* turn, running the wrong handler
|
||||
with no error or warning.
|
||||
"""
|
||||
|
||||
def test_falsy_route_turn_does_not_replay_previous_turns_intent(self) -> None:
|
||||
ran: list[str] = []
|
||||
|
||||
class Bot(ConversationalFlow):
|
||||
def route_turn(self, context: dict[str, Any]) -> str | None:
|
||||
message = context.get("current_user_message") or ""
|
||||
if "hello" in message.lower():
|
||||
return "GREETING"
|
||||
return None # unhandled input -> falsy return
|
||||
|
||||
@listen("GREETING")
|
||||
def greeting(self) -> str:
|
||||
ran.append("GREETING")
|
||||
reply = "Hi! I only do greetings."
|
||||
self.append_assistant_message(reply)
|
||||
return reply
|
||||
|
||||
@listen("WEATHER")
|
||||
def weather(self) -> str:
|
||||
ran.append("WEATHER")
|
||||
reply = "It is sunny."
|
||||
self.append_assistant_message(reply)
|
||||
return reply
|
||||
|
||||
flow = Bot()
|
||||
flow.handle_turn("hello there")
|
||||
assert ran == ["GREETING"]
|
||||
assert flow.state.last_intent == "GREETING"
|
||||
|
||||
flow.handle_turn("what is the meaning of life?")
|
||||
assert ran == ["GREETING"], (
|
||||
"an unhandled turn must not re-run the previous turn's handler"
|
||||
)
|
||||
# With no routing decision the turn falls through to the built-in
|
||||
# 'converse' default instead of replaying the stale intent.
|
||||
assert flow.state.last_intent == "converse"
|
||||
assert flow.state.messages[-1].content != "Hi! I only do greetings."
|
||||
|
||||
def test_stale_intent_ignored_but_route_selected_event_still_emitted(
|
||||
self,
|
||||
) -> None:
|
||||
class Bot(ConversationalFlow):
|
||||
def route_turn(self, context: dict[str, Any]) -> str | None:
|
||||
message = context.get("current_user_message") or ""
|
||||
return "work" if "work" in message else None
|
||||
|
||||
@listen("work")
|
||||
def do_work(self) -> str:
|
||||
self.append_assistant_message("worked")
|
||||
return "worked"
|
||||
|
||||
flow = Bot()
|
||||
routes: list[ConversationRouteSelectedEvent] = []
|
||||
with crewai_event_bus.scoped_handlers():
|
||||
|
||||
@crewai_event_bus.on(ConversationRouteSelectedEvent)
|
||||
def capture(_: Any, event: ConversationRouteSelectedEvent) -> None:
|
||||
routes.append(event)
|
||||
|
||||
flow.handle_turn("work please")
|
||||
flow.handle_turn("something unrelated")
|
||||
crewai_event_bus.flush()
|
||||
|
||||
assert [event.route for event in routes] == ["work", "converse"]
|
||||
# The fallback decision still reports the prior intent for visibility.
|
||||
assert routes[1].previous_intent == "work"
|
||||
|
||||
def test_fresh_intent_classified_this_turn_still_routes(self) -> None:
|
||||
"""The legacy ``default_intents`` path classifies per turn and must
|
||||
keep routing on the freshly classified intent — including when the
|
||||
intent changes between turns."""
|
||||
ran: list[str] = []
|
||||
|
||||
@ConversationConfig(
|
||||
default_intents=["search", "weather"], intent_llm="gpt-4o-mini"
|
||||
)
|
||||
class LegacyFlow(ConversationalFlow):
|
||||
@listen("search")
|
||||
def handle_search(self) -> str:
|
||||
ran.append("search")
|
||||
self.append_assistant_message("searched")
|
||||
return "searched"
|
||||
|
||||
@listen("weather")
|
||||
def handle_weather(self) -> str:
|
||||
ran.append("weather")
|
||||
self.append_assistant_message("sunny")
|
||||
return "sunny"
|
||||
|
||||
flow = LegacyFlow()
|
||||
with patch.object(
|
||||
flow, "_collapse_to_outcome", side_effect=["search", "weather"]
|
||||
):
|
||||
flow.handle_turn("look up crewai")
|
||||
flow.handle_turn("how is the weather?")
|
||||
|
||||
assert ran == ["search", "weather"]
|
||||
assert flow.state.last_intent == "weather"
|
||||
|
||||
|
||||
class TestFlowTracingWhenSuppressed:
|
||||
def test_flow_started_emitted_when_panel_events_suppressed(self) -> None:
|
||||
class QuietFlow(Flow[ChatState]):
|
||||
|
||||
Reference in New Issue
Block a user