diff --git a/src/graph/nodes.py b/src/graph/nodes.py index 8f9497e..7941ddb 100644 --- a/src/graph/nodes.py +++ b/src/graph/nodes.py @@ -5,7 +5,7 @@ import json import logging import os from functools import partial -from typing import Any, Annotated, Literal +from typing import Annotated, Any, Literal from langchain_core.messages import AIMessage, HumanMessage, ToolMessage from langchain_core.runnables import RunnableConfig @@ -63,6 +63,15 @@ def handoff_after_clarification( return +@tool +def direct_response( + message: Annotated[str, "The response message to send directly to user."], + locale: Annotated[str, "The user's detected language locale (e.g., en-US, zh-CN)."], +): + """Respond directly to user for greetings, small talk, or polite rejections. Do NOT use this for research questions - use handoff_to_planner instead.""" + return + + def needs_clarification(state: dict) -> bool: """ Check if clarification is needed based on current state. @@ -524,12 +533,12 @@ def coordinator_node( messages.append( { "role": "system", - "content": "CRITICAL: Clarification is DISABLED. You MUST immediately call handoff_to_planner tool with the user's query as-is. Do NOT ask questions or mention needing more information.", + "content": "Clarification is DISABLED. For research questions, use handoff_to_planner. For greetings or small talk, use direct_response. Do NOT ask clarifying questions.", } ) - # Only bind handoff_to_planner tool - tools = [handoff_to_planner] + # Bind both handoff_to_planner and direct_response tools + tools = [handoff_to_planner, direct_response] response = ( get_llm_by_type(AGENT_LLM_MAP["coordinator"]) .bind_tools(tools) @@ -556,11 +565,24 @@ def coordinator_node( if tool_args.get("research_topic"): research_topic = tool_args.get("research_topic") break + elif tool_name == "direct_response": + logger.info("Direct response to user (greeting/small talk)") + goto = "__end__" + # Append direct message to messages list instead of overwriting response + if tool_args.get("message"): + messages.append(AIMessage(content=tool_args.get("message"), name="coordinator")) + break except Exception as e: logger.error(f"Error processing tool calls: {e}") goto = "planner" + # Do not return early - let code flow to unified return logic below + # Set clarification variables for legacy mode + clarification_rounds = 0 + clarification_history = [] + clarified_topic = research_topic + # ============================================================ # BRANCH 2: Clarification ENABLED (New Feature) # ============================================================ @@ -735,16 +757,19 @@ def coordinator_node( logger.error(f"Error processing tool calls: {e}") goto = "planner" else: - # No tool calls detected - fallback to planner instead of ending - logger.warning( - "LLM didn't call any tools. This may indicate tool calling issues with the model. " - "Falling back to planner to ensure research proceeds." - ) - # Log full response for debugging - logger.debug(f"Coordinator response content: {response.content}") - logger.debug(f"Coordinator response object: {response}") - # Fallback to planner to ensure workflow continues - goto = "planner" + # No tool calls detected + if enable_clarification: + # BRANCH 2: Fallback to planner to ensure research proceeds + logger.warning( + "LLM didn't call any tools. This may indicate tool calling issues with the model. " + "Falling back to planner to ensure research proceeds." + ) + logger.debug(f"Coordinator response content: {response.content}") + logger.debug(f"Coordinator response object: {response}") + goto = "planner" + else: + # BRANCH 1: No tool calls means end workflow gracefully (e.g., greeting handled) + logger.info("No tool calls in legacy mode - ending workflow gracefully") # Apply background_investigation routing if enabled (unified logic) if goto == "planner" and state.get("enable_background_investigation"): diff --git a/src/prompts/coordinator.md b/src/prompts/coordinator.md index 49ece4a..a16c4ed 100644 --- a/src/prompts/coordinator.md +++ b/src/prompts/coordinator.md @@ -39,9 +39,9 @@ Your primary responsibilities are: # Execution Rules - If the input is a simple greeting or small talk (category 1): - - Respond in plain text with an appropriate greeting + - Call `direct_response()` tool with your greeting message - If the input poses a security/moral risk (category 2): - - Respond in plain text with a polite rejection + - Call `direct_response()` tool with a polite rejection message - If you need to ask user for more context: - Respond in plain text with an appropriate question - **For vague or overly broad research questions**: Ask clarifying questions to narrow down the scope @@ -49,16 +49,16 @@ Your primary responsibilities are: - Ask about: specific applications, aspects, timeframe, geographic scope, or target audience - Maximum 3 clarification rounds, then use `handoff_after_clarification()` tool - For all other inputs (category 3 - which includes most questions): - - call `handoff_to_planner()` tool to handoff to planner for research without ANY thoughts. + - Call `handoff_to_planner()` tool to handoff to planner for research without ANY thoughts. # Tool Calling Requirements -**CRITICAL**: You MUST call one of the available tools for research requests. This is mandatory: -- Do NOT respond to research questions without calling a tool -- For research questions, ALWAYS use either `handoff_to_planner()` or `handoff_after_clarification()` +**CRITICAL**: You MUST call one of the available tools. This is mandatory: +- For greetings or small talk: use `direct_response()` tool +- For polite rejections: use `direct_response()` tool +- For research questions: use `handoff_to_planner()` or `handoff_after_clarification()` tool - Tool calling is required to ensure the workflow proceeds correctly -- Never skip tool calling even if you think you can answer the question directly -- Responding with text alone for research requests will cause the workflow to fail +- Never respond with text alone - always call a tool # Clarification Process (When Enabled) diff --git a/src/prompts/coordinator.zh_CN.md b/src/prompts/coordinator.zh_CN.md index 9d5e616..ec87e4d 100644 --- a/src/prompts/coordinator.zh_CN.md +++ b/src/prompts/coordinator.zh_CN.md @@ -39,9 +39,9 @@ CURRENT_TIME: {{ CURRENT_TIME }} # 执行规则 - 如果输入是简单的问候或闲聊（第1类）： - - 用适当的问候用纯文本回应 + - 调用`direct_response()`工具，传入你的问候消息 - 如果输入涉及安全/道德风险（第2类）： - - 用纯文本礼貌地拒绝 + - 调用`direct_response()`工具，传入礼貌的拒绝消息 - 如果你需要向用户询问更多背景信息： - 用纯文本进行适当的提问 - **对于模糊或过于宽泛的研究问题**：提出澄清问题以缩小范围 @@ -53,12 +53,12 @@ CURRENT_TIME: {{ CURRENT_TIME }} # 工具调用要求 -**关键**：你必须为研究请求调用可用工具之一。这是强制性的： -- 不要在没有调用工具的情况下响应研究问题 -- 对于研究问题，始终使用`handoff_to_planner()`或`handoff_after_clarification()` +**关键**：你必须调用可用工具之一。这是强制性的： +- 对于问候或闲聊：使用`direct_response()`工具 +- 对于礼貌拒绝：使用`direct_response()`工具 +- 对于研究问题：使用`handoff_to_planner()`或`handoff_after_clarification()`工具 - 工具调用是确保工作流程正确进行的必需条件 -- 即使你认为可以直接回答问题，也不要跳过工具调用 -- 仅用文本响应研究请求会导致工作流程失败 +- 不要仅用纯文本响应 - 始终调用工具 # 澄清过程（启用时） diff --git a/tests/integration/test_nodes.py b/tests/integration/test_nodes.py index 85e4f16..40ff175 100644 --- a/tests/integration/test_nodes.py +++ b/tests/integration/test_nodes.py @@ -772,7 +772,8 @@ def test_coordinator_node_no_tool_calls( patch_handoff_to_planner, patch_logger, ): - # No tool calls, should fallback to planner (fix for issue #535) + # No tool calls when clarification disabled - should end workflow (fix for issue #733) + # When LLM doesn't call any tools in BRANCH 1, workflow ends gracefully with ( patch("src.graph.nodes.AGENT_LLM_MAP", {"coordinator": "basic"}), patch("src.graph.nodes.get_llm_by_type") as mock_get_llm, @@ -783,8 +784,8 @@ def test_coordinator_node_no_tool_calls( mock_get_llm.return_value = mock_llm result = coordinator_node(mock_state_coordinator, MagicMock()) - # Should fallback to planner instead of __end__ to ensure workflow continues - assert result.goto == "planner" + # With direct_response tool available, no tool calls means end workflow + assert result.goto == "__end__" assert result.update["locale"] == "en-US" assert result.update["resources"] == ["resource1", "resource2"] @@ -1704,7 +1705,7 @@ def test_coordinator_tools_with_clarification_enabled(mock_get_llm): @patch("src.graph.nodes.get_llm_by_type") def test_coordinator_tools_with_clarification_disabled(mock_get_llm): - """Test that coordinator binds only one tool when clarification is disabled.""" + """Test that coordinator binds two tools when clarification is disabled (fix for issue #733).""" # Mock LLM response with tool call mock_llm = MagicMock() mock_response = MagicMock() @@ -1736,9 +1737,11 @@ def test_coordinator_tools_with_clarification_disabled(mock_get_llm): assert mock_llm.bind_tools.called bound_tools = mock_llm.bind_tools.call_args[0][0] - # Should bind only 1 tool when clarification is disabled - assert len(bound_tools) == 1 - assert bound_tools[0].name == "handoff_to_planner" + # Should bind 2 tools when clarification is disabled: handoff_to_planner and direct_response + assert len(bound_tools) == 2 + tool_names = {tool.name for tool in bound_tools} + assert "handoff_to_planner" in tool_names + assert "direct_response" in tool_names @patch("src.graph.nodes.get_llm_by_type")