Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 135 additions & 0 deletions scripts/e2e_clarifier_search_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
"""End-to-end test for issue #234 fix.
Comment thread
torkian marked this conversation as resolved.
Outdated

Runs the ClarifierAgent against a real NVIDIA NIM-hosted LLM with a real Tavily
web search tool, using an obscure query the LLM cannot possibly know from
training data. Verifies that the agent issues a search call before falling back
to asking the user for clarification.

Run with:
PYTHONPATH=src .venv/bin/python scripts/e2e_clarifier_search_test.py
"""

from __future__ import annotations

import asyncio
import os
import sys
from pathlib import Path

# Load deploy/.env if present.
ENV_FILE = Path(__file__).resolve().parents[1] / "deploy" / ".env"
if ENV_FILE.exists():
for line in ENV_FILE.read_text().splitlines():
line = line.strip()
if not line or line.startswith("#") or "=" not in line:
continue
key, _, value = line.partition("=")
os.environ.setdefault(key.strip(), value.strip())

if not os.environ.get("NVIDIA_API_KEY"):
sys.exit("NVIDIA_API_KEY not set (paste it into deploy/.env)")
if not os.environ.get("TAVILY_API_KEY"):
sys.exit("TAVILY_API_KEY not set (paste it into deploy/.env)")

from langchain_community.tools.tavily_search import TavilySearchResults # noqa: E402
from langchain_core.messages import HumanMessage # noqa: E402
from langchain_core.messages import ToolMessage # noqa: E402
from langchain_nvidia_ai_endpoints import ChatNVIDIA # noqa: E402

from aiq_agent.agents.clarifier.agent import ClarifierAgent # noqa: E402
from aiq_agent.agents.clarifier.models import ClarifierAgentState # noqa: E402
from aiq_agent.common import LLMProvider # noqa: E402

try:
from aiq_agent.agents.clarifier.agent import FORCE_SEARCH_GUIDANCE # noqa: E402
except ImportError:
FORCE_SEARCH_GUIDANCE = None # running against the pre-fix branch

# Test queries:
# - "obscure": something no LLM can know without searching.
# - "knowable": something the LLM thinks it knows; in the failure mode it
# skips the search and just asks "which aspect?".
TEST_QUERIES = {
"obscure": "Tell me about Project Zphyr-77Q at QXR Industries — what does it do and who works on it?",
"knowable": "Research the latest NVIDIA AI announcements from 2026",
}
OBSCURE_QUERY = TEST_QUERIES[os.environ.get("AIQ_TEST_QUERY", "knowable")]

# Model defaults to a small open model so the test is cheap to run.
MODEL_NAME = os.environ.get("AIQ_CLARIFIER_TEST_MODEL", "meta/llama-3.3-70b-instruct")


async def main() -> int:
llm = ChatNVIDIA(model=MODEL_NAME, temperature=0)
search = TavilySearchResults(max_results=3, name="web_search_tool")

provider = LLMProvider()
provider.set_default(llm)

# The user_prompt_callback should never be invoked if the search-before-ask
# behavior is working. If it is invoked, we record it so the test can fail
# loudly.
user_prompts: list[str] = []

async def user_prompt_callback(question: str) -> str:
user_prompts.append(question)
# If reached, pretend the user said "skip" so the run still terminates.
return "skip"

agent = ClarifierAgent(
llm_provider=provider,
tools=[search],
user_prompt_callback=user_prompt_callback,
max_turns=2,
)

state = ClarifierAgentState(messages=[HumanMessage(content=OBSCURE_QUERY)])
print(f"Model: {MODEL_NAME}")
print(f"Query: {OBSCURE_QUERY}\n")

final_state = await agent.graph.ainvoke(state, config={"callbacks": []})

# Pull the final messages out for inspection.
if isinstance(final_state, dict):
messages = final_state["messages"]
force_search_used = final_state.get("force_search_used", False)
else:
messages = final_state.messages
force_search_used = getattr(final_state, "force_search_used", False)

tool_calls_made = sum(len(getattr(m, "tool_calls", None) or []) for m in messages)
tool_results = sum(1 for m in messages if isinstance(m, ToolMessage))
forced = bool(force_search_used)

print("=" * 72)
print("Conversation trace")
print("=" * 72)
for i, m in enumerate(messages):
kind = type(m).__name__
content = str(getattr(m, "content", "")).replace("\n", " ")[:160]
extra = ""
if getattr(m, "tool_calls", None):
extra = f" tool_calls={[(tc.get('name'), tc.get('args')) for tc in m.tool_calls]}"
print(f"{i:>2}. {kind}: {content}{extra}")

print()
print("=" * 72)
print("Verdict")
print("=" * 72)
print(f" user_prompt_callback invocations : {len(user_prompts)}")
print(f" total tool_calls emitted by LLM : {tool_calls_made}")
print(f" total ToolMessages (results) : {tool_results}")
print(f" force_search guidance injected : {forced}")

if tool_calls_made >= 1 and len(user_prompts) == 0:
print("\nPASS: the LLM issued at least one search before any user prompt.")
return 0
if tool_calls_made == 0:
print("\nFAIL: the LLM never issued a search.")
else:
print("\nFAIL: the LLM issued a search, but the user was also prompted.")
return 1


if __name__ == "__main__":
sys.exit(asyncio.run(main()))
86 changes: 83 additions & 3 deletions src/aiq_agent/agents/clarifier/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,15 @@
)
"""Reminder prompt added after tool results to reinforce JSON-only output."""

FORCE_SEARCH_GUIDANCE = (
"You attempted to ask the user for clarification before gathering any context. "
"Before asking the user a question, you MUST first use the available search tools "
"to look up unfamiliar entities, acronyms, products, or terms in their request. "
"Issue one focused tool call now with a query derived from the user's request. "
"Only after reviewing the tool results should you decide whether clarification is still needed."
)
"""Guidance prompt injected when the LLM tries to clarify without having searched first."""


class ClarifierAgent:
"""
Expand Down Expand Up @@ -484,6 +493,24 @@ def _get_fallback_clarification(self, query: str | None = None) -> str:
SKIP_COMMANDS = {"skip", "done", "exit", "quit", "proceed", "continue", "no", "n", ""}
"""Set of commands that indicate the user wants to skip clarification."""

@staticmethod
def _has_tool_invocations(messages: Sequence[Any]) -> bool:
"""
Check whether any prior assistant message in the conversation issued tool calls.

Args:
messages: The conversation message history.

Returns:
True if any AIMessage in the history carries non-empty tool_calls,
False otherwise.
"""
for msg in messages:
tool_calls = getattr(msg, "tool_calls", None)
if tool_calls:
return True
return False

def _is_skip_command(self, user_reply: str) -> bool:
"""
Check if the user's reply indicates they want to skip clarification.
Expand All @@ -503,16 +530,22 @@ def _build_graph(self) -> CompiledStateGraph:
"""
Build the LangGraph StateGraph for the clarification workflow.

Creates a graph with three nodes:
Creates a graph with the following nodes:
- agent: Generates clarification questions using the LLM
- tools: Executes tool calls (e.g., web search) for context
- ask_for_clarification: Prompts user and processes response
- force_search: Nudges the LLM to attempt a search before its first
clarification question when tools are configured but unused (fires
at most once per run)
- plan_preview: Optional plan approval flow

The graph flow:
1. agent generates a response (question, tool call, or completion)
2. If tool call → tools node → back to agent
3. If question → ask_for_clarification → back to agent
4. If complete → end
3. If complete → end (or plan_preview if enabled)
4. If question, tools exist, and no search has happened yet on the
first turn → force_search injects guidance and routes back to agent
5. Otherwise → ask_for_clarification → back to agent

Returns:
Compiled LangGraph StateGraph ready for execution.
Expand Down Expand Up @@ -547,6 +580,17 @@ async def agent_node(state: ClarifierAgentState):
logger.info("Adding JSON reminder after tool results")
messages.append(HumanMessage(content=JSON_REMINDER_AFTER_TOOLS))

# If the force_search nudge was raised but no tool call has happened
# yet, inject the guidance ephemerally — never stored in state.messages,
# so it cannot leak into get_latest_user_query and similar lookups.
# The iteration==0 guard mirrors the one in decide_route: once the
# user has actually replied (iteration > 0), the nudge no longer
# applies and would otherwise force a gratuitous search on the
# user's clarifying answer.
if state.force_search_used and state.iteration == 0 and not self._has_tool_invocations(state.messages):
logger.info("Injecting force-search guidance for this turn")
messages.append(HumanMessage(content=FORCE_SEARCH_GUIDANCE))

response = await bound_llm.ainvoke(messages)
return {"messages": [response]}

Expand Down Expand Up @@ -592,8 +636,12 @@ async def ask_clarification(state: ClarifierAgentState):
def decide_route(state: ClarifierAgentState | dict):
if isinstance(state, dict):
messages = state.get("messages", [])
iteration = state.get("iteration", 0)
force_search_used = state.get("force_search_used", False)
elif hasattr(state, "messages"):
messages = state.messages
iteration = state.iteration
force_search_used = state.force_search_used
else:
msg = f"No messages found in input state to tool_edge: {state}"
raise ValueError(msg)
Expand All @@ -610,8 +658,37 @@ def decide_route(state: ClarifierAgentState | dict):
if self.enable_plan_approval:
return "plan_preview"
return "__end__"

# Force a search before the first clarification question when tools are
# configured but the agent has not yet issued any tool call. This keeps
# the behavior model-agnostic: even models that would otherwise skip
# tool use must attempt at least one search before falling back to
# asking the user for clarification. We only nudge once per run (guarded
# by ``force_search_used``) so a stubborn model cannot get stuck in a
# loop — after the nudge, normal clarification routing resumes.
if (
self.tools
and iteration == 0
and not force_search_used
and not self._has_tool_invocations(messages[:-1])
):
logger.info("Clarifier: forcing search before first clarification question")
return "force_search"

return "ask_for_clarification"

async def force_search_node(state: ClarifierAgentState):
Comment thread
torkian marked this conversation as resolved.
Outdated
"""
Flip the force_search_used flag so ``agent_node`` will inject the
search-first guidance message ephemerally on the next LLM call.

The guidance is intentionally NOT appended to ``state.messages`` so
it cannot be picked up by helpers like ``get_latest_user_query``
(which would otherwise surface internal scaffolding back to the
user in fallback text).
"""
return {"force_search_used": True}

async def plan_preview_node(state: ClarifierAgentState):
"""Generate plan preview and handle approval/feedback loop."""
clarifier_log = state.clarifier_log
Expand Down Expand Up @@ -681,6 +758,7 @@ async def plan_preview_node(state: ClarifierAgentState):
graph.add_node("agent", agent_node)
graph.add_node("tools", ToolNode(self.tools))
graph.add_node("ask_for_clarification", ask_clarification)
graph.add_node("force_search", force_search_node)
graph.add_node("plan_preview", plan_preview_node)

graph.set_entry_point("agent")
Expand All @@ -691,13 +769,15 @@ async def plan_preview_node(state: ClarifierAgentState):
{
"tools": "tools",
"ask_for_clarification": "ask_for_clarification",
"force_search": "force_search",
"plan_preview": "plan_preview",
"__end__": "__end__",
},
)

graph.add_edge("tools", "agent")
graph.add_edge("ask_for_clarification", "agent")
graph.add_edge("force_search", "agent")
graph.add_edge("plan_preview", "__end__")

return graph.compile()
Expand Down
4 changes: 4 additions & 0 deletions src/aiq_agent/agents/clarifier/models/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ class ClarifierAgentState(BaseModel):
max_turns: Maximum number of turns for the clarification dialog.
clarifier_log: Log of the clarification dialog.
iteration: Current iteration of the clarification dialog.
force_search_used: Whether the agent has already been nudged once to
attempt a search before its first clarification question. Used to
ensure the nudge fires at most once per run.
plan_title: Title of the generated research plan (if plan approval enabled).
plan_sections: List of section titles for the research plan.
plan_approved: Whether the user approved the plan.
Expand All @@ -74,6 +77,7 @@ class ClarifierAgentState(BaseModel):
max_turns: int = Field(default=3)
clarifier_log: str = Field(default="")
iteration: int = Field(default=0)
force_search_used: bool = Field(default=False)
plan_title: str | None = Field(default=None)
plan_sections: list[str] = Field(default_factory=list)
plan_approved: bool = Field(default=False)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ Your ONLY responsibility is to determine whether a research request requires cla

## Tool Usage

- You may use search tools ONLY to understand unfamiliar domains
- Use at most 1-2 high-value searches
- Searches are for your internal understanding only
- **Search first, ask second.** If the user's request contains any unfamiliar entity, acronym, project, person, product, or technical term that you cannot fully define from your training data, you MUST issue a search tool call before deciding clarification is needed. Do not ask the user to define terms that a quick search would resolve.
- On the first turn, prefer at least one search to ground the topic in current context whenever search tools are available.
- Use at most 1-2 high-value searches per turn — keep queries focused on the specific unknown.
- Searches are for your internal understanding only. Do not summarize or report search results to the user.
- After reviewing search results, re-evaluate: if the request is now sufficiently specified, return `needs_clarification: false`. Only ask a clarification question if a genuine ambiguity remains.

---

Expand Down
10 changes: 10 additions & 0 deletions tests/aiq_agent/agents/clarifier/models/test_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,16 @@ def test_custom_iteration(self):
state = ClarifierAgentState(messages=[], iteration=2)
assert state.iteration == 2

def test_default_force_search_used(self):
"""Test default force_search_used is False."""
state = ClarifierAgentState(messages=[])
assert state.force_search_used is False

def test_custom_force_search_used(self):
"""Test custom force_search_used."""
state = ClarifierAgentState(messages=[], force_search_used=True)
assert state.force_search_used is True

def test_remaining_questions_full(self):
"""Test remaining_questions when iteration is 0."""
state = ClarifierAgentState(messages=[], max_turns=3, iteration=0)
Expand Down
Loading