diff --git a/.agents/skills/aiq-customize-prompts-models/references/model-selection.md b/.agents/skills/aiq-customize-prompts-models/references/model-selection.md index fedc5b75a..2ae7dc367 100644 --- a/.agents/skills/aiq-customize-prompts-models/references/model-selection.md +++ b/.agents/skills/aiq-customize-prompts-models/references/model-selection.md @@ -46,10 +46,9 @@ with `LLMProvider.configure(LLMRole., llm)` An unset role falls back to the provider default (the `orchestrator_llm` model). There is **no** generic `llm` field on the deep research agent. -**Clarifier** (`src/aiq_agent/agents/clarifier/register.py`) defines `llm` (its -default) and `planner_llm`. It does **not** use `LLMProvider.configure` for the -role — it passes `planner_llm` straight to the agent constructor, and `planner_llm` -falls back to `llm` when unset. +**Clarifier** (`src/aiq_agent/agents/clarifier/register.py`) defines a single +`llm` field. It does **not** use `LLMProvider.configure` for a role — it passes +`llm` straight to the agent constructor. ```yaml functions: diff --git a/.agents/skills/aiq-customize-prompts-models/references/prompt-templates.md b/.agents/skills/aiq-customize-prompts-models/references/prompt-templates.md index b5e1420a2..bc953088b 100644 --- a/.agents/skills/aiq-customize-prompts-models/references/prompt-templates.md +++ b/.agents/skills/aiq-customize-prompts-models/references/prompt-templates.md @@ -16,9 +16,8 @@ variable section there yet — so read the `.j2` file when in doubt. Each agent owns its Jinja2 templates under `src/aiq_agent/agents//prompts/*.j2`. For example, the deep researcher has `orchestrator.j2`, `planner.j2`, `researcher.j2`, `source_router.j2`, `writer.j2`, -and `source_registry.j2`; the clarifier has `plan_generation.j2` and -`research_clarification.j2`; `shallow_researcher` and `chat_researcher` have their -own as well. +and `source_registry.j2`; the clarifier has `research_clarification.j2`; +`shallow_researcher` and `chat_researcher` have their own as well. ## How templates load and render diff --git a/configs/config_cli_default.yml b/configs/config_cli_default.yml index 10be391eb..85d78c731 100644 --- a/configs/config_cli_default.yml +++ b/configs/config_cli_default.yml @@ -99,11 +99,9 @@ functions: clarifier_agent: _type: clarifier_agent llm: nemotron_super_llm - planner_llm: nemotron_super_llm # tools: omitted -> inherits all from data_source_registry # exclude_tools: [] max_turns: 3 - enable_plan_approval: true log_response_max_chars: 2000 verbose: true diff --git a/configs/config_frontier_models.yml b/configs/config_frontier_models.yml index 330ae7ff0..1d952ef76 100644 --- a/configs/config_frontier_models.yml +++ b/configs/config_frontier_models.yml @@ -127,12 +127,10 @@ functions: clarifier_agent: _type: clarifier_agent llm: openai_gpt_5_2 - planner_llm: openai_gpt_5_2 tools: - web_search_tool - knowledge_search max_turns: 3 - enable_plan_approval: true log_response_max_chars: 2000 verbose: true diff --git a/configs/config_openshell.yml b/configs/config_openshell.yml index 1d6537d18..788e38efe 100644 --- a/configs/config_openshell.yml +++ b/configs/config_openshell.yml @@ -114,9 +114,7 @@ functions: clarifier_agent: _type: clarifier_agent llm: nemotron_super_llm - planner_llm: nemotron_super_llm max_turns: 3 - enable_plan_approval: true log_response_max_chars: 2000 verbose: true diff --git a/configs/config_web_default_guardrails.yml b/configs/config_web_default_guardrails.yml index 84cf3d2bf..d786774f6 100644 --- a/configs/config_web_default_guardrails.yml +++ b/configs/config_web_default_guardrails.yml @@ -206,9 +206,7 @@ functions: clarifier_agent: _type: clarifier_agent llm: nemotron_super_llm - planner_llm: nemotron_super_llm max_turns: 3 - enable_plan_approval: true log_response_max_chars: 2000 verbose: true diff --git a/configs/config_web_default_llamaindex.yml b/configs/config_web_default_llamaindex.yml index fa4027716..b5ca6bfe1 100644 --- a/configs/config_web_default_llamaindex.yml +++ b/configs/config_web_default_llamaindex.yml @@ -172,11 +172,9 @@ functions: clarifier_agent: _type: clarifier_agent llm: nemotron_super_llm - planner_llm: nemotron_super_llm # tools: omitted -> inherits all from data_source_registry # exclude_tools: [] max_turns: 3 - enable_plan_approval: true log_response_max_chars: 2000 verbose: true diff --git a/configs/config_web_frag.yml b/configs/config_web_frag.yml index bd678ee73..195f0e665 100644 --- a/configs/config_web_frag.yml +++ b/configs/config_web_frag.yml @@ -141,11 +141,9 @@ functions: clarifier_agent: _type: clarifier_agent llm: nemotron_super_llm - planner_llm: nemotron_super_llm # tools: omitted -> inherits all from data_source_registry # exclude_tools: [] max_turns: 3 - enable_plan_approval: true log_response_max_chars: 2000 verbose: true diff --git a/configs/config_web_frag_mcp_auth.yml b/configs/config_web_frag_mcp_auth.yml index 4e5b1c9ea..0b48c34da 100644 --- a/configs/config_web_frag_mcp_auth.yml +++ b/configs/config_web_frag_mcp_auth.yml @@ -190,11 +190,9 @@ functions: clarifier_agent: _type: clarifier_agent llm: nemotron_super_llm - planner_llm: nemotron_super_llm # tools: omitted -> inherits all from data_source_registry # exclude_tools: [] max_turns: 3 - enable_plan_approval: true log_response_max_chars: 2000 verbose: true diff --git a/configs/config_web_opensearch.yml b/configs/config_web_opensearch.yml index 57755c2f5..39f1735e2 100644 --- a/configs/config_web_opensearch.yml +++ b/configs/config_web_opensearch.yml @@ -148,11 +148,9 @@ functions: clarifier_agent: _type: clarifier_agent llm: nemotron_super_llm - planner_llm: nemotron_super_llm # tools: omitted -> inherits all from data_source_registry # exclude_tools: [] max_turns: 3 - enable_plan_approval: true log_response_max_chars: 2000 verbose: true diff --git a/docs/notebooks/0_Getting_Started_with_AIQ.ipynb b/docs/notebooks/0_Getting_Started_with_AIQ.ipynb index b37b9ae8c..0d21b6647 100644 --- a/docs/notebooks/0_Getting_Started_with_AIQ.ipynb +++ b/docs/notebooks/0_Getting_Started_with_AIQ.ipynb @@ -391,7 +391,7 @@ "**Optional settings:** \n", "\n", "`enable_clarifier: true` lets the agent ask clarifying questions before deep research; \n", - "`enable_plan_approval: true` (on the clarifier) lets users approve or adjust the research plan. \n", + "When the request is vague, the clarifier may also ask you to narrow the scope or clarify the type of output requested. \n", "\n", ">**Note:** the above human-in-the-loop interactions may not work in a notebook environment; you can experience these using the AI-Q CLI or the web UI deployed later in this notebook.\n", "\n", @@ -622,7 +622,7 @@ "\n", "**Select data sources:** In the UI, open the **data sources** panel (e.g. from the right side or connections icon). You will see available sources (e.g. Web Search, and optionally Paper Search, Knowledge Layer, or enterprise sources if configured). Toggle **on** the sources you want the agent to use for the next query. Web Search is usually enabled by default.\n", "\n", - "**Send a query:** Type your question or research request in the chat input and send it. The agent will use only the **enabled** data sources for that query. For deep research, the UI may show clarification or plan approval steps; respond as prompted. Results and the final report will appear in the chat." + "**Send a query:** Type your question or research request in the chat input and send it. The agent will use only the **enabled** data sources for that query. For deep research, the UI may show clarification steps; respond as prompted. Results and the final report will appear in the chat." ] }, { diff --git a/docs/source/architecture/agents/clarifier.md b/docs/source/architecture/agents/clarifier.md index 96d5bf159..7bc33c608 100644 --- a/docs/source/architecture/agents/clarifier.md +++ b/docs/source/architecture/agents/clarifier.md @@ -6,8 +6,8 @@ SPDX-License-Identifier: Apache-2.0 # Clarifier Agent The Clarifier Agent provides human-in-the-loop (HITL) interaction before deep -research begins. It gathers clarifications from the user, generates a -structured research plan, and optionally presents the plan for approval. +research begins. It gathers context and, when the request is vague, optionally +asks the user to narrow the scope or clarify the type of output requested. **Location:** `src/aiq_agent/agents/clarifier/agent.py` @@ -16,9 +16,9 @@ structured research plan, and optionally presents the plan for approval. Deep research is expensive in both time and compute. The Clarifier reduces wasted effort by: -1. Asking focused clarification questions to narrow the research scope -2. Generating a structured research plan with title and sections -3. Allowing the user to approve, reject, or provide feedback on the plan +1. Gathering context (including optional tool calls such as web search) about the request +2. Asking focused clarification questions only when the request is genuinely ambiguous +3. Optionally clarifying the **type of output** the user wants (for example, report, table, comparison, prediction, or brief answer) when that is unclear The clarifier runs on the deep research path and also when a shallow query escalates to deep. It can be disabled entirely using `enable_clarifier: false` @@ -46,28 +46,11 @@ graph TD I -->|no| L[Auto-complete clarification] H -->|no| L[Clarification complete] - L --> M{enable_plan_approval?} - - M -->|no| N[Return ClarifierResult
with clarifier_log] - M -->|yes| O[Generate research plan
using plan_generation.j2] - - O --> P[Present plan to user] - P --> Q{User decision?} - - Q -->|approve| R[Return ClarifierResult
plan_approved = true] - Q -->|reject| S[Return ClarifierResult
plan_rejected = true] - Q -->|feedback| T{iterations < max_plan_iterations?} - T -->|yes| U[Regenerate plan
with feedback] - U --> P - T -->|no| V[Auto-approve plan] - V --> R + L --> N[Return ClarifierResult
with clarifier_log] style A fill:#e1f5fe style N fill:#e8f5e9 - style R fill:#e8f5e9 - style S fill:#ffebee style J fill:#fff3e0 - style P fill:#fff3e0 ``` ## State Model @@ -82,11 +65,6 @@ graph TD | `max_turns` | `int` | `3` | Maximum clarification Q&A turns | | `clarifier_log` | `str` | `""` | Accumulated clarification dialog log | | `iteration` | `int` | `0` | Current clarification turn counter | -| `plan_title` | `str` or `None` | `None` | Title of the generated research plan | -| `plan_sections` | `list[str]` | `[]` | Section titles for the research plan | -| `plan_approved` | `bool` | `false` | Whether the user approved the plan | -| `plan_rejected` | `bool` | `false` | Whether the user rejected the plan | -| `plan_feedback_history` | `list[str]` | `[]` | History of user feedback on plan iterations | Computed property: - `remaining_questions` = `max_turns - iteration` @@ -98,13 +76,6 @@ Returned to the orchestrator after the clarification dialog completes: | Field | Type | Description | | ----- | ---- | ----------- | | `clarifier_log` | `str` | Full clarification dialog log | -| `plan_title` | `str` or `None` | Research plan title (if plan approval enabled) | -| `plan_sections` | `list[str]` | Plan section titles | -| `plan_approved` | `bool` | Whether the plan was approved | -| `plan_rejected` | `bool` | Whether the plan was rejected | - -The `get_approved_plan_context()` method formats the approved plan as markdown -for injection into the deep researcher's orchestrator prompt. ### ClarificationResponse @@ -122,11 +93,8 @@ Configured through `ClarifierConfig` (NeMo Agent Toolkit type name: `clarifier_a | Parameter | Type | Default | Description | | --------- | ---- | ------- | ----------- | | `llm` | `LLMRef` | required | LLM for generating clarification questions | -| `planner_llm` | `LLMRef` or `None` | `None` | Separate LLM for plan generation; falls back to `llm` | | `tools` | `list[FunctionRef \| FunctionGroupRef]` | `[]` | Tools for context gathering (for example, web search) | | `max_turns` | `int` | `3` | Maximum clarification Q&A turns before auto-completing | -| `enable_plan_approval` | `bool` | `false` | Enable plan preview and approval after clarification | -| `max_plan_iterations` | `int` | `10` | Maximum plan feedback iterations before auto-approving | | `log_response_max_chars` | `int` | `2000` | Maximum characters to log from LLM responses | | `verbose` | `bool` | `false` | Enable verbose logging with `VerboseTraceCallback` | @@ -137,12 +105,9 @@ functions: clarifier_agent: _type: clarifier_agent llm: nemotron_llm - planner_llm: nemotron_llm tools: - web_search_tool max_turns: 3 - enable_plan_approval: true - max_plan_iterations: 10 verbose: true ``` @@ -153,7 +118,6 @@ Located in `src/aiq_agent/agents/clarifier/prompts/`: | Template | Purpose | | -------- | ------- | | `research_clarification.j2` | Generates clarification questions. Includes conditional sections for uploaded documents context. Instructs the LLM to respond with JSON containing `needs_clarification` and `clarification_question`. Template variables: `clarifier_result`, `available_documents`, `tools`, `tool_names` | -| `plan_generation.j2` | Generates a structured research plan with title and sections from the clarified query and dialog log. Template variables: `clarifier_context`, `feedback_history` | ## HITL Interaction Patterns @@ -167,25 +131,9 @@ User: "Focus on Germany and Japan." Agent: "Got it. Are you interested in economic impacts from a GDP perspective, job creation, or both?" User: "Both GDP impact and job creation." -Agent: [clarification complete, generates plan] -``` - -When `enable_plan_approval` is `true`: - -``` -Agent: "Here is the proposed research plan: - Title: Economic Impacts of Renewable Energy in Germany and Japan - Sections: - - GDP Impact Analysis - - Job Creation Metrics - - Comparative Analysis - Do you approve this plan?" -User: "Add a section on policy frameworks." -Agent: [regenerates plan with feedback] -User: "Approve" -Agent: [returns ClarifierResult with plan_approved=true] +Agent: [clarification complete, proceeds to deep research] ``` -User responses are matched against keyword sets: -- **Approval:** approve, approved, yes, ok, proceed, continue, go ahead, looks good, y, accept -- **Rejection:** reject, rejected, no, cancel, stop, abort, n +When the desired output form is unclear, the clarifier may instead ask which +type of output you want (for example, a full report, a comparison table, or a +brief answer) before research begins. diff --git a/docs/source/architecture/agents/index.md b/docs/source/architecture/agents/index.md index d4b749c3f..e5b4eaa37 100644 --- a/docs/source/architecture/agents/index.md +++ b/docs/source/architecture/agents/index.md @@ -10,7 +10,7 @@ AI-Q uses a multi-agent architecture where an intent classifier routes queries t | Agent | Purpose | Speed | Depth | |-------|---------|-------|-------| | [Intent Classifier](./intent-classifier.md) | Route queries and determine research depth | Instant | — | -| [Clarifier](./clarifier.md) | HITL: clarify ambiguous queries and approve research plans | Interactive | — | +| [Clarifier](./clarifier.md) | HITL: clarify ambiguous queries and the requested output type | Interactive | — | | [Shallow Researcher](./shallow-researcher.md) | Fast, bounded research for simple questions | Fast (30-60s) | Surface | | [Deep Researcher](./deep-researcher.md) | Multi-phase deep research with planning and iteration | Thorough (2-10min) | Deep | diff --git a/docs/source/customization/configuration-reference.md b/docs/source/customization/configuration-reference.md index 667b62be7..85d376791 100644 --- a/docs/source/customization/configuration-reference.md +++ b/docs/source/customization/configuration-reference.md @@ -293,12 +293,9 @@ functions: clarifier_agent: _type: clarifier_agent llm: nemotron_llm - planner_llm: nemotron_llm tools: - web_search_tool max_turns: 3 - enable_plan_approval: true - max_plan_iterations: 10 log_response_max_chars: 2000 verbose: true ``` @@ -306,11 +303,9 @@ functions: | Parameter | Type | Default | Description | |-----------|------|---------|-------------| | `llm` | `str` | **required** | LLM for generating clarification questions. | -| `planner_llm` | `str` | `None` | LLM for plan generation. Falls back to `llm` if not specified. | | `tools` | `list[str]` | `[]` | Tools available for gathering context during clarification. | +| `exclude_tools` | `list[str]` | `[]` | Tool names to exclude when inheriting from the data source registry. | | `max_turns` | `int` | `3` | Maximum number of clarification Q&A turns before auto-completing. | -| `enable_plan_approval` | `bool` | `false` | Show research plan to the user for approval after clarification. | -| `max_plan_iterations` | `int` | `10` | Maximum plan feedback iterations before auto-approving. | | `log_response_max_chars` | `int` | `2000` | Maximum characters to log from LLM responses. | | `verbose` | `bool` | `false` | Enable verbose logging. | @@ -475,11 +470,9 @@ functions: clarifier_agent: # Asks clarifying questions for deep research _type: clarifier_agent llm: research_llm - planner_llm: research_llm tools: - web_search_tool max_turns: 3 - enable_plan_approval: true verbose: true shallow_research_agent: # Fast single-pass research @@ -514,7 +507,7 @@ The repository includes several pre-built configurations: | File | Mode | Features | |------|------|----------| -| `configs/config_cli_default.yml` | CLI | Web search, paper search, clarifier with plan approval | +| `configs/config_cli_default.yml` | CLI | Web search, paper search, clarifier | | `configs/config_web_default_llamaindex.yml` | Web API | LlamaIndex knowledge retrieval, web search, paper search | | `configs/config_web_frag.yml` | Web API | Foundational RAG knowledge retrieval, web search, paper search | diff --git a/docs/source/customization/hitl.md b/docs/source/customization/hitl.md index e4cdc0cfb..c659d86ab 100644 --- a/docs/source/customization/hitl.md +++ b/docs/source/customization/hitl.md @@ -4,11 +4,13 @@ SPDX-License-Identifier: Apache-2.0 --> # Human-in-the-Loop (HITL) -Human-in-the-loop (clarifier and plan approval) runs before deep research. To disable it: +The clarifier runs before deep research. It gathers context and, when the +request is vague, may ask you to narrow the scope or clarify the type of output +you want. To disable it: ## Disable the Clarifier Entirely -No plan generation or approval step: +No clarification step before deep research: ```yaml workflow: @@ -17,14 +19,14 @@ workflow: # ... ``` -## Keep Clarifier but Skip Plan Approval +## Limit Clarification Questions -No user approval step before deep research: +Cap how many clarification turns the clarifier may take: ```yaml functions: clarifier_agent: _type: clarifier_agent - enable_plan_approval: false + max_turns: 1 # ... ``` diff --git a/docs/source/customization/index.md b/docs/source/customization/index.md index 6f4de872d..f994672a3 100644 --- a/docs/source/customization/index.md +++ b/docs/source/customization/index.md @@ -18,4 +18,4 @@ SPDX-License-Identifier: Apache-2.0 - **[Guardrails](./guardrails.md)** — Configure NeMo Guardrails at workflow and agent boundaries - **[Knowledge Layer](./knowledge-layer.md)** — Add document retrieval (LlamaIndex or Foundational RAG) - **[Prompts](./prompts.md)** — Modify agent behavior through Jinja2 prompt templates -- **[Human-in-the-Loop](./hitl.md)** — Configure the clarifier and plan approval workflow +- **[Human-in-the-Loop](./hitl.md)** — Configure the clarifier diff --git a/docs/source/customization/prompts.md b/docs/source/customization/prompts.md index c97d957a5..3e86abfcc 100644 --- a/docs/source/customization/prompts.md +++ b/docs/source/customization/prompts.md @@ -17,7 +17,6 @@ Each agent in the AI-Q blueprint uses [Jinja2](https://jinja.palletsprojects.com | `src/aiq_agent/agents/deep_researcher/prompts/planner.j2` | Deep Research Planner | Generates evidence-grounded research plans with TOC structure and search queries | | `src/aiq_agent/agents/deep_researcher/prompts/researcher.j2` | Deep Research Researcher | Gathers and synthesizes information from search tools with inline citations | | `src/aiq_agent/agents/clarifier/prompts/research_clarification.j2` | Clarifier | Determines whether a research request needs clarification, asks focused follow-up questions | -| `src/aiq_agent/agents/clarifier/prompts/plan_generation.j2` | Clarifier (Plan) | Generates a lightweight research plan after clarification is complete | ## Template Directory Structure @@ -35,7 +34,6 @@ src/aiq_agent/agents/ researcher.j2 # Sub-researcher prompt clarifier/ prompts/ - plan_generation.j2 # Research plan generation research_clarification.j2 # Clarification prompt chat_researcher/ prompts/ @@ -177,7 +175,6 @@ Each template has well-defined sections you can target: - **Deep Research Planner** (`planner.j2`) — TOC structure, query generation guidelines, research cycle instructions, output JSON schema - **Deep Research Researcher** (`researcher.j2`) — Research protocol, source prioritization, tool call budget, citation format - **Clarifier** (`research_clarification.j2`) — What counts as "sufficiently specified", question style, multi-turn policy -- **Plan Generation** (`plan_generation.j2`) — Plan structure, section naming rules, output JSON format ### Creating a New Template diff --git a/docs/source/examples/cli-with-local-nims.md b/docs/source/examples/cli-with-local-nims.md index f7af9d012..34de6810b 100644 --- a/docs/source/examples/cli-with-local-nims.md +++ b/docs/source/examples/cli-with-local-nims.md @@ -123,11 +123,9 @@ functions: clarifier_agent: _type: clarifier_agent llm: nemotron_super_llm - planner_llm: nemotron_super_llm tools: - web_search_tool max_turns: 3 - enable_plan_approval: true # Interactive plan approval in CLI log_response_max_chars: 2000 verbose: true diff --git a/docs/source/examples/full-pipeline-llamaindex.md b/docs/source/examples/full-pipeline-llamaindex.md index 3b86d7c26..98f5c60df 100644 --- a/docs/source/examples/full-pipeline-llamaindex.md +++ b/docs/source/examples/full-pipeline-llamaindex.md @@ -122,12 +122,10 @@ functions: clarifier_agent: _type: clarifier_agent llm: nemotron_super_llm - planner_llm: nemotron_super_llm tools: - web_search_tool - knowledge_search max_turns: 3 - enable_plan_approval: true log_response_max_chars: 2000 verbose: true diff --git a/docs/source/examples/full-pipeline-web.md b/docs/source/examples/full-pipeline-web.md index 58d139641..e33c11217 100644 --- a/docs/source/examples/full-pipeline-web.md +++ b/docs/source/examples/full-pipeline-web.md @@ -5,7 +5,7 @@ SPDX-License-Identifier: Apache-2.0 # Example: Full Pipeline (Foundational RAG) -The complete AI-Q blueprint configuration with all features enabled: intent classification, shallow and deep research agents, knowledge retrieval (Foundational RAG), paper search, web search, clarifier with human-in-the-loop plan approval, and the async jobs API with SSE streaming. +The complete AI-Q blueprint configuration with all features enabled: intent classification, shallow and deep research agents, knowledge retrieval (Foundational RAG), paper search, web search, clarifier with human-in-the-loop clarification, and the async jobs API with SSE streaming. This is based on `configs/config_web_frag.yml`, which is the default for Helm deployments. @@ -139,17 +139,15 @@ functions: # ------------------------------------------------------------------------- # Clarifier agent (human-in-the-loop) # ------------------------------------------------------------------------- - # For deep research: asks clarifying questions and generates a research - # plan that the user can approve or modify before execution. + # For deep research: asks clarifying questions before handing off to the + # deep_research_agent. clarifier_agent: _type: clarifier_agent llm: nemotron_super_llm - planner_llm: nemotron_super_llm tools: - web_search_tool - knowledge_search max_turns: 3 # Max clarification rounds - enable_plan_approval: true # User must approve the plan log_response_max_chars: 2000 verbose: true @@ -185,7 +183,7 @@ functions: # The chat_deepresearcher_agent is the meta-routing workflow: # 1. Intent classifier determines shallow vs deep # 2. Shallow queries go directly to shallow_research_agent -# 3. Deep queries go through clarifier -> plan approval -> deep_research_agent +# 3. Deep queries go through clarifier -> deep_research_agent workflow: _type: chat_deepresearcher_agent enable_escalation: true # Allow shallow -> deep escalation diff --git a/docs/source/examples/hybrid-frontier-model.md b/docs/source/examples/hybrid-frontier-model.md index c5cb943ee..428054836 100644 --- a/docs/source/examples/hybrid-frontier-model.md +++ b/docs/source/examples/hybrid-frontier-model.md @@ -69,12 +69,10 @@ functions: clarifier_agent: _type: clarifier_agent llm: nemotron_super_llm - planner_llm: nemotron_super_llm tools: - web_search_tool - knowledge_search max_turns: 3 - enable_plan_approval: true shallow_research_agent: _type: shallow_research_agent diff --git a/docs/source/resources/faq.md b/docs/source/resources/faq.md index 20916c418..8d1de2208 100644 --- a/docs/source/resources/faq.md +++ b/docs/source/resources/faq.md @@ -30,9 +30,9 @@ Not for running the blueprint itself — it calls cloud-hosted LLM APIs. You onl The [Intent Classifier](../architecture/agents/intent-classifier.md) automatically routes queries to the appropriate depth. -**Can I disable the clarifier / plan approval step?** +**Can I disable the clarifier step?** -Yes. Refer to [Human-in-the-Loop](../customization/hitl.md) for configuration options. You can disable the clarifier entirely or keep it but skip plan approval. +Yes. Refer to [Human-in-the-Loop](../customization/hitl.md) for configuration options. You can disable the clarifier entirely or limit how many clarification questions it asks. **What happens when shallow research escalates to deep?** diff --git a/docs/source/resources/troubleshooting.md b/docs/source/resources/troubleshooting.md index 6094f9596..0477fe842 100644 --- a/docs/source/resources/troubleshooting.md +++ b/docs/source/resources/troubleshooting.md @@ -34,7 +34,7 @@ Common issues and solutions for the AI-Q blueprint. | Agent hangs on deep research | LLM timeout or rate limit | Set `verbose: true` in config to see progress; check LLM API availability and rate limits | | HTTP 429 or 503 on deep research | Nemotron Super Build API has limited availability due to high demand | Retry after a short delay, reduce concurrency, or self-host via [Brev Launchable](#nemotron-super--build-endpoint-availability) for consistent throughput | | Shallow research returns generic answers | Insufficient tool calls | Increase `max_tool_iterations` (default: 5) | -| Clarifier keeps asking questions | Too many clarification turns | Reduce `max_turns` or set `enable_plan_approval: false` | +| Clarifier keeps asking questions | Too many clarification turns | Reduce `max_turns`, or set `enable_clarifier: false` in the workflow to disable clarification | | SSE stream disconnects | Network timeout | Client auto-reconnects using `last_event_id`; refer to [Data Flow](../architecture/data-flow.md) | | Job status stuck on RUNNING | Dask worker crashed | Check Dask logs; the ghost job reaper will eventually mark it FAILURE | diff --git a/frontends/ui/src/features/chat/components/AgentPrompt.spec.tsx b/frontends/ui/src/features/chat/components/AgentPrompt.spec.tsx index f504d54b5..011a6e071 100644 --- a/frontends/ui/src/features/chat/components/AgentPrompt.spec.tsx +++ b/frontends/ui/src/features/chat/components/AgentPrompt.spec.tsx @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 import { render, screen } from '@/test-utils' -import userEvent from '@testing-library/user-event' import { vi, describe, test, expect, beforeEach } from 'vitest' import { AgentPrompt } from './AgentPrompt' import { useChatStore } from '../store' @@ -113,27 +112,4 @@ describe('AgentPrompt', () => { expect(screen.getByText(/\d{1,2}:\d{2}/)).toBeInTheDocument() }) - test('tabs through plan approval actions in DOM order', async () => { - const user = userEvent.setup() - useChatStore.setState({ respondToInteractionFn: vi.fn() }) - - render( - - ) - - const approveButton = screen.getByRole('button', { name: /approve plan/i }) - const rejectButton = screen.getByRole('button', { name: /reject plan/i }) - - expect(approveButton).not.toHaveAttribute('tabindex') - expect(rejectButton).not.toHaveAttribute('tabindex') - - await user.tab() - expect(approveButton).toHaveFocus() - await user.tab() - expect(rejectButton).toHaveFocus() - }) }) diff --git a/frontends/ui/src/features/chat/components/AgentPrompt.tsx b/frontends/ui/src/features/chat/components/AgentPrompt.tsx index d398ad979..12264a475 100644 --- a/frontends/ui/src/features/chat/components/AgentPrompt.tsx +++ b/frontends/ui/src/features/chat/components/AgentPrompt.tsx @@ -6,26 +6,19 @@ * * Displays prompts from the agent that require user response. * This is a display-only component - user responds via the main chat input. - * - * For plan approval prompts, inline Approve/Reject buttons are rendered - * inside the bubble so the user can respond without typing. */ 'use client' -import { type FC, useCallback } from 'react' -import { Flex, Text, Button } from '@/adapters/ui' +import type { FC } from 'react' +import { Flex, Text } from '@/adapters/ui' import { formatTime } from '@/shared/utils/format-time' import { Chat } from '@/adapters/ui/icons' import { MarkdownRenderer } from '@/shared/components/MarkdownRenderer' -import { useChatStore } from '../store' import type { PromptType } from '../types' export type { PromptType } -const APPROVAL_PROMPT_RE = - /Reply\s+\*{0,2}approve\*{0,2}\s+to proceed,\s+\*{0,2}reject\*{0,2}\s+to cancel/i - export interface AgentPromptProps { /** Unique identifier for this prompt */ id: string @@ -50,9 +43,6 @@ export interface AgentPromptProps { /** * Agent prompt component - display only. * User responds via the main chat input area. - * - * When the prompt contains plan approval text, Approve/Reject buttons - * are rendered inline so the user can respond with a single click. */ export const AgentPrompt: FC = ({ type: _type, @@ -62,18 +52,6 @@ export const AgentPrompt: FC = ({ response, timestamp, }) => { - const respondToInteractionFn = useChatStore((state) => state.respondToInteractionFn) - const isApprovalPrompt = APPROVAL_PROMPT_RE.test(content) - const showApprovalButtons = isApprovalPrompt && !isResponded && !!respondToInteractionFn - - const handleApprove = useCallback(() => { - respondToInteractionFn?.('approve') - }, [respondToInteractionFn]) - - const handleReject = useCallback(() => { - respondToInteractionFn?.('reject') - }, [respondToInteractionFn]) - return ( @@ -98,29 +76,6 @@ export const AgentPrompt: FC = ({ {/* Options list for choice prompts */} {options.length > 0 && !isResponded && } - {/* Approve/Reject buttons for plan approval prompts */} - {showApprovalButtons && ( - - - - - )} - {/* Response display (only shown after user responds) */} {isResponded && } diff --git a/frontends/ui/src/features/chat/hooks/use-current-session-busy.spec.ts b/frontends/ui/src/features/chat/hooks/use-current-session-busy.spec.ts index 8a64890d4..380db6719 100644 --- a/frontends/ui/src/features/chat/hooks/use-current-session-busy.spec.ts +++ b/frontends/ui/src/features/chat/hooks/use-current-session-busy.spec.ts @@ -153,7 +153,7 @@ describe('useIsCurrentSessionBusy', () => { mockUseChatStore.mockImplementation((selector: (state: any) => any) => selector({ ...idleState, - pendingInteraction: { type: 'plan_approval', content: 'Approve this plan?' }, + pendingInteraction: { type: 'clarification', content: 'Approve this plan?' }, }) ) @@ -177,7 +177,7 @@ describe('useIsCurrentSessionBusy', () => { selector({ ...idleState, isStreaming: true, - pendingInteraction: { type: 'plan_approval' }, + pendingInteraction: { type: 'clarification' }, }) ) @@ -191,7 +191,7 @@ describe('useIsCurrentSessionBusy', () => { mockUseChatStore.mockImplementation((selector: (state: any) => any) => selector({ ...idleState, - pendingInteraction: { type: 'plan_approval' }, + pendingInteraction: { type: 'clarification' }, }) ) diff --git a/frontends/ui/src/features/chat/lib/session-activity.spec.ts b/frontends/ui/src/features/chat/lib/session-activity.spec.ts index ab26f0cc4..ce5c363ce 100644 --- a/frontends/ui/src/features/chat/lib/session-activity.spec.ts +++ b/frontends/ui/src/features/chat/lib/session-activity.spec.ts @@ -244,7 +244,7 @@ describe('getPersistedActivityFlags', () => { }) it('detects pending HITL interaction', () => { - const flags = getPersistedActivityFlags([], { type: 'plan_approval', content: 'Approve?' }) + const flags = getPersistedActivityFlags([], { type: 'clarification', content: 'Approve?' }) expect(flags.hasActiveDeepResearch).toBe(false) expect(flags.hasPendingHITL).toBe(true) }) @@ -257,7 +257,7 @@ describe('getPersistedActivityFlags', () => { deepResearchJobStatus: 'submitted', }), ] - const flags = getPersistedActivityFlags(messages, { type: 'plan_approval' }) + const flags = getPersistedActivityFlags(messages, { type: 'clarification' }) expect(flags.hasActiveDeepResearch).toBe(true) expect(flags.hasPendingHITL).toBe(true) }) diff --git a/frontends/ui/src/features/chat/types.ts b/frontends/ui/src/features/chat/types.ts index a62785299..fc72bb435 100644 --- a/frontends/ui/src/features/chat/types.ts +++ b/frontends/ui/src/features/chat/types.ts @@ -61,7 +61,7 @@ export type ErrorCode = | 'system.unknown' /** Prompt types for agent prompts requiring user response */ -export type PromptType = 'clarification' | 'approval' | 'choice' | 'text-input' | 'plan_approval' +export type PromptType = 'clarification' | 'approval' | 'choice' | 'text-input' /** File card data for file messages */ export interface FileCardData { diff --git a/src/aiq_agent/agents/chat_researcher/agent.py b/src/aiq_agent/agents/chat_researcher/agent.py index 62662b258..e3cf2fd7e 100644 --- a/src/aiq_agent/agents/chat_researcher/agent.py +++ b/src/aiq_agent/agents/chat_researcher/agent.py @@ -181,31 +181,10 @@ async def clarifier_node(state: ChatResearcherState) -> dict[str, Any]: ) result = await self.clarifier_fn(clarifier_state) - # Check if plan was rejected - if result.plan_rejected: - logger.info("ChatResearcher: Plan rejected by user, ending workflow") - return Command( - goto=END, - update={ - "messages": [ - AIMessage( - content="Research plan was rejected. Please start a new research query when ready." - ) - ], - "original_query": original_query, - }, - ) - - # Build clarifier result with optional approved plan context - clarifier_result = result.clarifier_log - approved_plan_context = result.get_approved_plan_context() - if approved_plan_context: - clarifier_result = f"{clarifier_result}\n\n{approved_plan_context}" - return Command( goto="deep_research", update={ - "clarifier_result": clarifier_result, + "clarifier_result": result.clarifier_log, "original_query": original_query, }, ) diff --git a/src/aiq_agent/agents/clarifier/agent.py b/src/aiq_agent/agents/clarifier/agent.py index 11c70a8ce..dfeea04b8 100644 --- a/src/aiq_agent/agents/clarifier/agent.py +++ b/src/aiq_agent/agents/clarifier/agent.py @@ -81,19 +81,6 @@ ) """Fallback prompt used when the prompt file cannot be loaded.""" -DEFAULT_PLAN_GENERATION_PROMPT = ( - "/no_think\n\n" - "Generate a research plan with a title and 5-8 sections. " - 'Respond with JSON: {"title": "...", "sections": ["...", "..."]}' -) -"""Fallback prompt for plan generation.""" - -APPROVAL_KEYWORDS = {"approve", "approved", "yes", "ok", "proceed", "continue", "go ahead", "looks good", "y", "accept"} -"""Keywords that indicate the user approves the plan.""" - -REJECTION_KEYWORDS = {"reject", "rejected", "no", "cancel", "stop", "abort", "n"} -"""Keywords that indicate the user rejects the plan.""" - JSON_REMINDER_AFTER_TOOLS = ( "Based on the search results above, now make your clarification decision. " "IMPORTANT: You must respond with ONLY a valid JSON object, nothing else. " @@ -132,6 +119,10 @@ class ClarifierAgent: - tools: Executes tool calls for context gathering (e.g., web search) - ask_for_clarification: Prompts the user and processes their response + It gathers context and, when the request is vague, may clarify the scope or + the type of output the user wants (e.g. report, table, comparison, prediction) + before research begins. It does not produce or approve a research plan. + Attributes: llm_provider: Provider for obtaining LLM instances. tools: List of tools available for context gathering. @@ -163,9 +154,6 @@ def __init__( *, user_prompt_callback: Callable[[str], Awaitable[str]], max_turns: int = 3, - enable_plan_approval: bool = False, - max_plan_iterations: int = 10, - planner_llm: BaseChatModel | None = None, log_response_max_chars: int = 2000, verbose: bool = False, callbacks: list[Any] | None = None, @@ -181,12 +169,6 @@ def __init__( Takes a question string and returns the user's response string. max_turns: Maximum number of clarification Q&A turns before automatically completing clarification. Defaults to 3. - enable_plan_approval: Whether to enable plan preview and approval - after clarification completes. Defaults to False. - max_plan_iterations: Maximum number of plan feedback iterations - before auto-approving. Defaults to 10. - planner_llm: Optional LLM to use for plan generation. If not provided, - uses the default clarifier LLM. log_response_max_chars: Maximum characters to log from LLM responses. Used for debugging. Defaults to 2000. verbose: Whether to enable detailed logging. Defaults to False. @@ -197,15 +179,11 @@ def __init__( self.tools = list(tools) if tools else [] self.user_prompt_callback = user_prompt_callback self.max_turns = max_turns - self.enable_plan_approval = enable_plan_approval - self.max_plan_iterations = max_plan_iterations - self.planner_llm = planner_llm self.log_response_max_chars = log_response_max_chars self.verbose = verbose self.callbacks = callbacks or [] self.system_prompt = self._load_default_prompt() - self.plan_generation_prompt = self._load_plan_generation_prompt() self._graph = self._build_graph() @@ -225,101 +203,6 @@ def _load_default_prompt(self) -> str: logger.warning("Clarifier prompt not found, using inline default") return DEFAULT_CLARIFICATION_PROMPT - def _load_plan_generation_prompt(self) -> str: - """ - Load the plan generation prompt from file. - - Returns: - The loaded prompt string, or the default fallback prompt. - """ - try: - return load_prompt(AGENT_DIR / "prompts", "plan_generation") - except Exception: - logger.warning("Plan generation prompt not found, using inline default") - return DEFAULT_PLAN_GENERATION_PROMPT - - def _parse_plan_response(self, text: str) -> tuple[str | None, list[str]]: - """ - Parse plan generation response from LLM. - - Args: - text: Raw JSON text response from the LLM. - - Returns: - Tuple of (title, sections) or (None, []) if parsing fails. - """ - if not text: - return None, [] - - text = text.strip() - json_match = re.search(r"```(?:json)?\s*([\s\S]*?)```", text) - if json_match: - text = json_match.group(1).strip() - - try: - data = json.loads(text) - title = data.get("title") - sections = data.get("sections", []) - if isinstance(sections, list) and all(isinstance(s, str) for s in sections): - return title, sections - except (json.JSONDecodeError, Exception) as e: - logger.warning("Failed to parse plan response as JSON: %s", e) - - return None, [] - - def _parse_approval(self, response: str) -> tuple[bool, bool, str | None]: - """ - Parse user's approval response. - - Args: - response: User's response text (may be JSON wrapped). - - Returns: - Tuple of (approved, rejected, feedback). - - If approved: (True, False, None) - - If rejected: (False, True, None) - - If feedback: (False, False, feedback_text) - """ - # Extract query from JSON if wrapped (e.g., {"query": "approve", ...}) - text = response.strip() - try: - data = json.loads(text) - if isinstance(data, dict) and "query" in data: - text = data["query"] - except (json.JSONDecodeError, TypeError): - pass # Not JSON, use original text - - normalized = text.strip().lower() - - if normalized in APPROVAL_KEYWORDS: - return True, False, None - - if normalized in REJECTION_KEYWORDS: - return False, True, None - - # Treat as feedback for plan revision - return False, False, text.strip() - - def _format_plan_for_user(self, title: str, sections: list[str]) -> str: - """ - Format the plan for user display. - - Args: - title: Plan title. - sections: List of section titles. - - Returns: - Formatted string for user display. - """ - sections_text = "\n".join(f" {i + 1}. {s}" for i, s in enumerate(sections)) - return ( - f"**Research Plan Preview**\n\n" - f"**Title:** {title}\n\n" - f"**Sections:**\n{sections_text}\n\n" - f"---\n" - f"Reply **approve** to proceed, **reject** to cancel, or provide feedback to revise the plan." - ) - def _parse_response(self, text: str) -> ClarificationResponse | None: """ Parse JSON response from LLM into ClarificationResponse. @@ -563,13 +446,12 @@ def _build_graph(self) -> CompiledStateGraph: the model once and retries inline. - tools: Executes tool calls (e.g., web search) for context - ask_for_clarification: Prompts user and processes response - - plan_preview: Optional plan approval flow The graph flow: 1. agent generates a response (question, tool call, or completion); on turn 0 it may force one search-and-retry before yielding 2. If tool call → tools node → back to agent - 3. If complete → end (or plan_preview if enabled) + 3. If complete → end 4. Otherwise → ask_for_clarification → back to agent Returns: @@ -577,8 +459,6 @@ def _build_graph(self) -> CompiledStateGraph: """ llm = self._get_llm() bound_llm = llm.bind_tools(self.tools, parallel_tool_calls=True) if self.tools else llm - # Use planner_llm for plan generation if provided, otherwise use default llm - planner_llm = self.planner_llm if self.planner_llm is not None else llm graph = StateGraph(ClarifierAgentState) @@ -592,8 +472,8 @@ async def agent_node(state: ClarifierAgentState): if state.remaining_questions <= 0: # Clarification budget is exhausted — emit a completion signal, # but never create an invalid history (two adjacent assistant - # messages, or a pending tool call left unresolved), since it can - # reach plan_preview's planner. + # messages, or a pending tool call left unresolved), since the + # message list is replayed to the LLM on later turns. last_message = state.messages[-1] if state.messages else None if isinstance(last_message, AIMessage) and getattr(last_message, "tool_calls", None): # A pending tool call must be resolved before we complete; @@ -743,8 +623,6 @@ def decide_route(state: ClarifierAgentState | dict): return "tools" if self._is_complete(ai_message.content): - if self.enable_plan_approval: - return "plan_preview" return "__end__" # The search-before-clarify nudge (issue #234) is handled inline in @@ -753,76 +631,9 @@ def decide_route(state: ClarifierAgentState | dict): # already happened, so we route straight to the user. return "ask_for_clarification" - async def plan_preview_node(state: ClarifierAgentState): - """Generate plan preview and handle approval/feedback loop.""" - clarifier_log = state.clarifier_log - feedback_history: list[str] = list(state.plan_feedback_history) - - # Initialize with fallback values in case loop doesn't execute (max_plan_iterations <= 0) - title: str = "Research Report" - sections: list[str] = ["Introduction", "Background", "Analysis", "Findings", "Conclusion"] - - for iteration in range(self.max_plan_iterations): - rendered_prompt = render_prompt_template( - self.plan_generation_prompt, - clarifier_context=clarifier_log, - feedback_history=feedback_history if feedback_history else None, - ) - - # Generate plan using planner LLM - messages_for_plan = state.messages + [HumanMessage(content="Generate a research plan.")] - response = await planner_llm.ainvoke([SystemMessage(content=rendered_prompt)] + messages_for_plan) - title, sections = self._parse_plan_response(response.content) - - if not title or not sections: - logger.warning("Failed to generate valid plan, using fallback") - title = "Research Report" - sections = ["Introduction", "Background", "Analysis", "Findings", "Conclusion"] - - # Present plan to user - plan_display = self._format_plan_for_user(title, sections) - user_response = await self.user_prompt_callback(plan_display) - - approved, rejected, feedback = self._parse_approval(user_response) - - if approved: - logger.info("Clarifier: Plan approved by user") - return { - "plan_title": title, - "plan_sections": sections, - "plan_approved": True, - "plan_rejected": False, - "plan_feedback_history": feedback_history, - } - - if rejected: - logger.info("Clarifier: Plan rejected by user") - return { - "plan_title": title, - "plan_sections": sections, - "plan_approved": False, - "plan_rejected": True, - "plan_feedback_history": feedback_history, - } - - # User provided feedback, add to history and continue loop - logger.info("Clarifier: User provided feedback, regenerating plan") - feedback_history.append(feedback) - - # Max iterations reached, auto-approve - logger.warning("Clarifier: Max plan iterations reached, auto-approving") - return { - "plan_title": title, - "plan_sections": sections, - "plan_approved": True, - "plan_rejected": False, - "plan_feedback_history": feedback_history, - } - graph.add_node("agent", agent_node) graph.add_node("tools", ToolNode(self.tools)) graph.add_node("ask_for_clarification", ask_clarification) - graph.add_node("plan_preview", plan_preview_node) graph.set_entry_point("agent") @@ -832,14 +643,12 @@ async def plan_preview_node(state: ClarifierAgentState): { "tools": "tools", "ask_for_clarification": "ask_for_clarification", - "plan_preview": "plan_preview", "__end__": "__end__", }, ) graph.add_edge("tools", "agent") graph.add_edge("ask_for_clarification", "agent") - graph.add_edge("plan_preview", "__end__") return graph.compile() @@ -851,20 +660,14 @@ async def run(self, state: ClarifierAgentState) -> ClarifierResult: state: Initial state of the clarifier agent. Returns: - ClarifierResult with clarification log and plan approval details. + ClarifierResult with the clarification log. """ logger.info("Clarifier: Starting (max %d turns)", self.max_turns) query = get_latest_user_query(state.messages) logger.info("User's query: %s...", str(query)[:100] if query else "") result = await self._graph.ainvoke(state, config={"callbacks": self.callbacks}) final_state = ClarifierAgentState.model_validate(result) - return ClarifierResult( - clarifier_log=final_state.clarifier_log, - plan_title=final_state.plan_title, - plan_sections=final_state.plan_sections, - plan_approved=final_state.plan_approved, - plan_rejected=final_state.plan_rejected, - ) + return ClarifierResult(clarifier_log=final_state.clarifier_log) @property def graph(self) -> CompiledStateGraph: diff --git a/src/aiq_agent/agents/clarifier/models/state.py b/src/aiq_agent/agents/clarifier/models/state.py index 89d1514da..a617f5a66 100644 --- a/src/aiq_agent/agents/clarifier/models/state.py +++ b/src/aiq_agent/agents/clarifier/models/state.py @@ -29,21 +29,10 @@ class ClarifierResult(BaseModel): """ Result returned from clarifier agent run. - Contains the clarification log plus optional plan approval details. + Contains the clarification log. """ clarifier_log: str = Field(default="") - plan_title: str | None = Field(default=None) - plan_sections: list[str] = Field(default_factory=list) - plan_approved: bool = Field(default=False) - plan_rejected: bool = Field(default=False) - - def get_approved_plan_context(self) -> str | None: - """Get formatted plan context if approved.""" - if not self.plan_approved or not self.plan_title: - return None - sections_text = "\n".join(f"- {s}" for s in self.plan_sections) - return f"**Approved Research Plan**\n\nTitle: {self.plan_title}\n\nSections:\n{sections_text}" class ClarifierAgentState(BaseModel): @@ -58,11 +47,6 @@ 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. - 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. - plan_rejected: Whether the user rejected the plan. - plan_feedback_history: History of user feedback on plan iterations. """ messages: Annotated[list[AnyMessage], add_messages] @@ -74,11 +58,6 @@ class ClarifierAgentState(BaseModel): max_turns: int = Field(default=3) clarifier_log: str = Field(default="") iteration: int = Field(default=0) - plan_title: str | None = Field(default=None) - plan_sections: list[str] = Field(default_factory=list) - plan_approved: bool = Field(default=False) - plan_rejected: bool = Field(default=False) - plan_feedback_history: list[str] = Field(default_factory=list) @computed_field @property diff --git a/src/aiq_agent/agents/clarifier/prompts/plan_generation.j2 b/src/aiq_agent/agents/clarifier/prompts/plan_generation.j2 deleted file mode 100644 index 7f536370f..000000000 --- a/src/aiq_agent/agents/clarifier/prompts/plan_generation.j2 +++ /dev/null @@ -1,42 +0,0 @@ -/no_think - -You are a research planning assistant. Your job is to create a lightweight research plan based on the user's request and any clarification context provided. - -Generate a research plan with: -1. A clear, descriptive title (1 line) -2. 5-8 section headings that outline the report structure - -IMPORTANT RULES: -- Output ONLY valid JSON. No text before or after. -- Do NOT include numbering in section names (e.g., use "Introduction" not "1. Introduction" or "Section 1: Introduction") -- Keep section names concise (3-6 words each) - -OUTPUT FORMAT: -```json -{ - "title": "Your Research Plan Title", - "sections": [ - "Introduction and Background", - "Topic Area One", - "Topic Area Two", - "Topic Area Three", - "Conclusion and Future Directions" - ] -} -``` - -Create a focused plan that addresses the user's research needs. - -{#- === KV CACHE BOUNDARY — dynamic content below === -#} - -{% if clarifier_context %} -CLARIFICATION CONTEXT: -{{ clarifier_context }} -{% endif %} - -{% if feedback_history %} -PREVIOUS FEEDBACK (incorporate this into your revised plan): -{% for feedback in feedback_history %} -- {{ feedback }} -{% endfor %} -{% endif %} diff --git a/src/aiq_agent/agents/clarifier/prompts/research_clarification.j2 b/src/aiq_agent/agents/clarifier/prompts/research_clarification.j2 index ae719a242..db8cb7a83 100644 --- a/src/aiq_agent/agents/clarifier/prompts/research_clarification.j2 +++ b/src/aiq_agent/agents/clarifier/prompts/research_clarification.j2 @@ -80,6 +80,7 @@ Only clarify when you genuinely cannot proceed. Ask about: - Scope narrowing (if topic is extremely broad, e.g., "Research AI") - Specific focus (if multiple distinct interpretations exist) - Comparison criteria (if comparing things without stated criteria) +- Output type (only if the desired form of the answer is genuinely ambiguous — e.g. whether the user wants a full long-form report, a comparison table, a brief direct answer, a data extraction, or a prediction/forecast). Do not ask this when the request already implies an output shape. Do NOT ask about: - Intent or purpose (unless completely ambiguous) @@ -113,6 +114,12 @@ Example - Comparison without criteria ("Compare cloud providers"): "clarification_question": "**Criteria**: What factors matter most for your comparison?\n\n1. Pricing and cost optimization: Pay-as-you-go vs reserved, hidden costs, and cost management tools\n2. Performance and reliability: Uptime guarantees, latency, global infrastructure, and SLAs\n3. Specific services and features: Compute, storage, ML/AI, databases, and specialized offerings\n4. Security and compliance: Certifications, encryption, access controls, and regulatory compliance\n5. Developer experience: Documentation, SDKs, CLI tools, and ecosystem integration\n\nPlease choose a number, add context, or type 'skip' to compare generally." } +Example - Ambiguous output type ("Give me the EV market in 2024"): +{ + "needs_clarification": true, + "clarification_question": "**Output**: How would you like the answer on the 2024 EV market?\n\n1. Long-form report: A comprehensive written analysis with sections and depth\n2. Comparison table: A table comparing the main players, models, or metrics\n3. Brief answer: A concise summary of the key numbers and takeaways\n4. Forecast: A forward-looking prediction for where the market is heading\n\nPlease choose a number, add context, or type 'skip' for a general report." +} + Example - Clear question, NO clarification needed: User: "Why is the US interested in Greenland?" { diff --git a/src/aiq_agent/agents/clarifier/register.py b/src/aiq_agent/agents/clarifier/register.py index e23d83346..10ac0946f 100644 --- a/src/aiq_agent/agents/clarifier/register.py +++ b/src/aiq_agent/agents/clarifier/register.py @@ -67,17 +67,11 @@ class ClarifierConfig(FunctionBaseConfig, name="clarifier_agent"): llm: Reference to the LLM to use for generating clarification questions. tools: List of tool references for context gathering (e.g., web search). max_turns: Maximum number of clarification Q&A turns before auto-completing. - enable_plan_approval: Whether to enable plan preview and approval after clarification. - max_plan_iterations: Maximum number of plan feedback iterations before auto-approving. log_response_max_chars: Maximum characters to log from LLM responses. verbose: Whether to enable verbose logging with VerboseTraceCallback. """ llm: LLMRef = Field(..., description="LLM to use for generating questions") - planner_llm: LLMRef | None = Field( - default=None, - description="LLM to use for plan generation. If not specified, uses the main llm.", - ) tools: list[FunctionRef | FunctionGroupRef] = Field( default_factory=list, description="Explicit tool list. Empty = inherit all from data_source_registry.", @@ -90,14 +84,6 @@ class ClarifierConfig(FunctionBaseConfig, name="clarifier_agent"): default=3, description="Maximum number of clarification Q&A turns", ) - enable_plan_approval: bool = Field( - default=False, - description="Whether to enable plan preview and approval after clarification", - ) - max_plan_iterations: int = Field( - default=10, - description="Maximum number of plan feedback iterations before auto-approving", - ) log_response_max_chars: int = Field( default=2000, description="Max characters to log from LLM responses", @@ -136,12 +122,6 @@ async def clarifier_agent(config: ClarifierConfig, builder: Builder): config.llm, wrapper_type=LLMFrameworkEnum.LANGCHAIN, ) - planner_llm = None - if config.planner_llm is not None: - planner_llm = await builder.get_llm( - config.planner_llm, - wrapper_type=LLMFrameworkEnum.LANGCHAIN, - ) if config.tools: tool_refs = config.tools else: @@ -193,9 +173,6 @@ async def user_prompt_callback(question: str) -> str: tools=tools, user_prompt_callback=user_prompt_callback, max_turns=config.max_turns, - enable_plan_approval=config.enable_plan_approval, - max_plan_iterations=config.max_plan_iterations, - planner_llm=planner_llm, log_response_max_chars=config.log_response_max_chars, verbose=verbose, callbacks=callbacks, @@ -208,7 +185,7 @@ async def _run(state: ClarifierAgentState) -> ClarifierResult: state: ClarifierAgentState with conversation messages. Returns: - ClarifierResult with clarification log and plan approval details. + ClarifierResult with the clarification log. """ data_sources = state.data_sources selected_tools = filter_tools_by_sources(tools, data_sources) @@ -219,9 +196,6 @@ async def _run(state: ClarifierAgentState) -> ClarifierResult: tools=selected_tools, user_prompt_callback=user_prompt_callback, max_turns=config.max_turns, - enable_plan_approval=config.enable_plan_approval, - max_plan_iterations=config.max_plan_iterations, - planner_llm=planner_llm, log_response_max_chars=config.log_response_max_chars, verbose=verbose, callbacks=callbacks, @@ -235,7 +209,7 @@ async def _run(state: ClarifierAgentState) -> ClarifierResult: _run, description=( "Handles interactive clarification dialog with users " - "for deep research queries. Asks follow-up questions and refines the research " - "scope, constraints, and requirements before planning begins." + "for deep research queries. Gathers context and, when the request is vague, " + "clarifies the scope or the type of output requested before research begins." ), ) diff --git a/tests/aiq_agent/agents/clarifier/models/test_state.py b/tests/aiq_agent/agents/clarifier/models/test_state.py index bbc86cd1f..e56020305 100644 --- a/tests/aiq_agent/agents/clarifier/models/test_state.py +++ b/tests/aiq_agent/agents/clarifier/models/test_state.py @@ -126,68 +126,8 @@ def test_default_values(self): """Test default values for ClarifierResult.""" result = ClarifierResult() assert result.clarifier_log == "" - assert result.plan_title is None - assert result.plan_sections == [] - assert result.plan_approved is False - assert result.plan_rejected is False def test_custom_values(self): """Test custom values for ClarifierResult.""" - result = ClarifierResult( - clarifier_log="Test log", - plan_title="Research Plan", - plan_sections=["Intro", "Analysis"], - plan_approved=True, - plan_rejected=False, - ) + result = ClarifierResult(clarifier_log="Test log") assert result.clarifier_log == "Test log" - assert result.plan_title == "Research Plan" - assert result.plan_sections == ["Intro", "Analysis"] - assert result.plan_approved is True - assert result.plan_rejected is False - - def test_get_approved_plan_context_when_approved(self): - """Test get_approved_plan_context returns formatted string when approved.""" - result = ClarifierResult( - plan_title="AI Research Report", - plan_sections=["Introduction", "Background", "Analysis"], - plan_approved=True, - ) - context = result.get_approved_plan_context() - - assert context is not None - assert "**Approved Research Plan**" in context - assert "AI Research Report" in context - assert "- Introduction" in context - assert "- Background" in context - assert "- Analysis" in context - - def test_get_approved_plan_context_when_not_approved(self): - """Test get_approved_plan_context returns None when not approved.""" - result = ClarifierResult( - plan_title="Research Plan", - plan_sections=["Section 1"], - plan_approved=False, - ) - assert result.get_approved_plan_context() is None - - def test_get_approved_plan_context_when_no_title(self): - """Test get_approved_plan_context returns None when no title.""" - result = ClarifierResult( - plan_title=None, - plan_sections=["Section 1"], - plan_approved=True, - ) - assert result.get_approved_plan_context() is None - - def test_get_approved_plan_context_empty_sections(self): - """Test get_approved_plan_context with empty sections.""" - result = ClarifierResult( - plan_title="Research Plan", - plan_sections=[], - plan_approved=True, - ) - context = result.get_approved_plan_context() - assert context is not None - assert "Research Plan" in context - assert "Sections:" in context diff --git a/tests/aiq_agent/agents/clarifier/test_agent.py b/tests/aiq_agent/agents/clarifier/test_agent.py index d6ca78f8d..5260d3691 100644 --- a/tests/aiq_agent/agents/clarifier/test_agent.py +++ b/tests/aiq_agent/agents/clarifier/test_agent.py @@ -463,430 +463,6 @@ async def test_run_logs_query(self, mock_llm_provider, mock_llm, caplog): assert "Clarifier: Starting" in caplog.text -class TestClarifierAgentPlanParsing: - """Tests for plan response parsing.""" - - @pytest.fixture - def agent(self): - """Create an agent for testing parsing methods.""" - llm = MagicMock() - llm.bind_tools = MagicMock(return_value=llm) - provider = MagicMock(spec=LLMProvider) - provider.get = MagicMock(return_value=llm) - - return ClarifierAgent( - llm_provider=provider, - user_prompt_callback=AsyncMock(), - ) - - def test_parse_plan_response_valid_json(self, agent): - """Test parsing valid plan JSON response.""" - text = '{"title": "AI Research Report", "sections": ["Introduction", "Methods", "Results"]}' - title, sections = agent._parse_plan_response(text) - - assert title == "AI Research Report" - assert sections == ["Introduction", "Methods", "Results"] - - def test_parse_plan_response_with_code_block(self, agent): - """Test parsing plan JSON wrapped in code block.""" - text = '```json\n{"title": "Research Plan", "sections": ["Overview", "Analysis"]}\n```' - title, sections = agent._parse_plan_response(text) - - assert title == "Research Plan" - assert sections == ["Overview", "Analysis"] - - def test_parse_plan_response_invalid_json(self, agent): - """Test parsing invalid JSON returns None and empty list.""" - title, sections = agent._parse_plan_response("not valid json") - assert title is None - assert sections == [] - - def test_parse_plan_response_empty_string(self, agent): - """Test parsing empty string returns None and empty list.""" - title, sections = agent._parse_plan_response("") - assert title is None - assert sections == [] - - def test_parse_plan_response_none(self, agent): - """Test parsing None returns None and empty list.""" - title, sections = agent._parse_plan_response(None) - assert title is None - assert sections == [] - - def test_parse_plan_response_missing_sections(self, agent): - """Test parsing response with missing sections.""" - text = '{"title": "Research Plan"}' - title, sections = agent._parse_plan_response(text) - - assert title == "Research Plan" - assert sections == [] - - def test_parse_plan_response_invalid_sections_type(self, agent): - """Test parsing response with non-list sections.""" - text = '{"title": "Research Plan", "sections": "not a list"}' - title, sections = agent._parse_plan_response(text) - - assert title is None - assert sections == [] - - def test_parse_plan_response_non_string_sections(self, agent): - """Test parsing response with non-string section items.""" - text = '{"title": "Research Plan", "sections": [1, 2, 3]}' - title, sections = agent._parse_plan_response(text) - - assert title is None - assert sections == [] - - -class TestClarifierAgentApprovalParsing: - """Tests for approval response parsing.""" - - @pytest.fixture - def agent(self): - """Create an agent for testing parsing methods.""" - llm = MagicMock() - llm.bind_tools = MagicMock(return_value=llm) - provider = MagicMock(spec=LLMProvider) - provider.get = MagicMock(return_value=llm) - - return ClarifierAgent( - llm_provider=provider, - user_prompt_callback=AsyncMock(), - ) - - @pytest.mark.parametrize( - "response", ["approve", "approved", "yes", "ok", "proceed", "continue", "go ahead", "looks good", "y"] - ) - def test_parse_approval_approved(self, agent, response): - """Test all approval keywords are recognized.""" - approved, rejected, feedback = agent._parse_approval(response) - assert approved is True - assert rejected is False - assert feedback is None - - @pytest.mark.parametrize("response", ["reject", "rejected", "no", "cancel", "stop", "abort", "n"]) - def test_parse_approval_rejected(self, agent, response): - """Test all rejection keywords are recognized.""" - approved, rejected, feedback = agent._parse_approval(response) - assert approved is False - assert rejected is True - assert feedback is None - - def test_parse_approval_feedback(self, agent): - """Test feedback response is captured.""" - approved, rejected, feedback = agent._parse_approval("Please add a section about security") - assert approved is False - assert rejected is False - assert feedback == "Please add a section about security" - - def test_parse_approval_case_insensitive(self, agent): - """Test approval parsing is case insensitive.""" - approved, rejected, feedback = agent._parse_approval("APPROVE") - assert approved is True - - approved, rejected, feedback = agent._parse_approval("REJECT") - assert rejected is True - - def test_parse_approval_with_whitespace(self, agent): - """Test approval parsing handles whitespace.""" - approved, rejected, feedback = agent._parse_approval(" approve ") - assert approved is True - - def test_parse_approval_json_wrapped(self, agent): - """Test approval parsing extracts query from JSON.""" - approved, rejected, feedback = agent._parse_approval('{"query": "approve", "context": "test"}') - assert approved is True - - def test_parse_approval_json_wrapped_feedback(self, agent): - """Test feedback extraction from JSON-wrapped response.""" - approved, rejected, feedback = agent._parse_approval('{"query": "add more sections"}') - assert approved is False - assert rejected is False - assert feedback == "add more sections" - - -class TestClarifierAgentPlanFormatting: - """Tests for plan formatting.""" - - @pytest.fixture - def agent(self): - """Create an agent for testing formatting methods.""" - llm = MagicMock() - llm.bind_tools = MagicMock(return_value=llm) - provider = MagicMock(spec=LLMProvider) - provider.get = MagicMock(return_value=llm) - - return ClarifierAgent( - llm_provider=provider, - user_prompt_callback=AsyncMock(), - ) - - def test_format_plan_for_user(self, agent): - """Test plan formatting for user display.""" - title = "AI Research Report" - sections = ["Introduction", "Background", "Analysis"] - - result = agent._format_plan_for_user(title, sections) - - assert "**Research Plan Preview**" in result - assert "**Title:** AI Research Report" in result - assert "1. Introduction" in result - assert "2. Background" in result - assert "3. Analysis" in result - assert "approve" in result.lower() - assert "reject" in result.lower() - - def test_format_plan_for_user_empty_sections(self, agent): - """Test plan formatting with empty sections list.""" - result = agent._format_plan_for_user("Test Plan", []) - - assert "**Title:** Test Plan" in result - assert "**Sections:**" in result - - -class TestClarifierAgentPlanApproval: - """Tests for plan approval workflow.""" - - @pytest.fixture - def mock_llm(self): - """Create a mock LLM.""" - llm = MagicMock() - llm.bind_tools = MagicMock(return_value=llm) - return llm - - @pytest.fixture - def mock_planner_llm(self): - """Create a mock planner LLM.""" - llm = MagicMock() - return llm - - @pytest.fixture - def mock_llm_provider(self, mock_llm): - """Create a mock LLM provider.""" - provider = MagicMock(spec=LLMProvider) - provider.get = MagicMock(return_value=mock_llm) - return provider - - @pytest.mark.asyncio - async def test_run_with_plan_approval_approved(self, mock_llm_provider, mock_llm, mock_planner_llm): - """Test run with plan approval when user approves.""" - # First, LLM completes clarification - complete_response = ClarificationResponse(needs_clarification=False, clarification_question=None) - mock_llm.ainvoke = AsyncMock(return_value=AIMessage(content=complete_response.model_dump_json())) - - # Planner LLM returns a valid plan - plan_response = '{"title": "Test Research Plan", "sections": ["Intro", "Analysis", "Conclusion"]}' - mock_planner_llm.ainvoke = AsyncMock(return_value=AIMessage(content=plan_response)) - - # User approves - mock_user_callback = AsyncMock(return_value="approve") - - agent = ClarifierAgent( - llm_provider=mock_llm_provider, - user_prompt_callback=mock_user_callback, - enable_plan_approval=True, - planner_llm=mock_planner_llm, - ) - - state = ClarifierAgentState(messages=[HumanMessage(content="Research AI")]) - result = await agent.run(state) - - assert result is not None - assert result.plan_approved is True - assert result.plan_rejected is False - assert result.plan_title == "Test Research Plan" - assert result.plan_sections == ["Intro", "Analysis", "Conclusion"] - - @pytest.mark.asyncio - async def test_run_with_plan_approval_rejected(self, mock_llm_provider, mock_llm, mock_planner_llm): - """Test run with plan approval when user rejects.""" - complete_response = ClarificationResponse(needs_clarification=False, clarification_question=None) - mock_llm.ainvoke = AsyncMock(return_value=AIMessage(content=complete_response.model_dump_json())) - - plan_response = '{"title": "Test Plan", "sections": ["Section 1", "Section 2"]}' - mock_planner_llm.ainvoke = AsyncMock(return_value=AIMessage(content=plan_response)) - - mock_user_callback = AsyncMock(return_value="reject") - - agent = ClarifierAgent( - llm_provider=mock_llm_provider, - user_prompt_callback=mock_user_callback, - enable_plan_approval=True, - planner_llm=mock_planner_llm, - ) - - state = ClarifierAgentState(messages=[HumanMessage(content="Research AI")]) - result = await agent.run(state) - - assert result is not None - assert result.plan_approved is False - assert result.plan_rejected is True - - @pytest.mark.asyncio - async def test_run_with_plan_approval_feedback_then_approve(self, mock_llm_provider, mock_llm, mock_planner_llm): - """Test run with plan approval when user provides feedback then approves.""" - complete_response = ClarificationResponse(needs_clarification=False, clarification_question=None) - mock_llm.ainvoke = AsyncMock(return_value=AIMessage(content=complete_response.model_dump_json())) - - # First plan, then revised plan - plan_response_1 = '{"title": "Initial Plan", "sections": ["Intro", "Analysis"]}' - plan_response_2 = '{"title": "Revised Plan", "sections": ["Intro", "Security", "Analysis"]}' - mock_planner_llm.ainvoke = AsyncMock( - side_effect=[ - AIMessage(content=plan_response_1), - AIMessage(content=plan_response_2), - ] - ) - - # User provides feedback, then approves - mock_user_callback = AsyncMock(side_effect=["add a security section", "approve"]) - - agent = ClarifierAgent( - llm_provider=mock_llm_provider, - user_prompt_callback=mock_user_callback, - enable_plan_approval=True, - planner_llm=mock_planner_llm, - ) - - state = ClarifierAgentState(messages=[HumanMessage(content="Research AI")]) - result = await agent.run(state) - - assert result is not None - assert result.plan_approved is True - assert result.plan_title == "Revised Plan" - assert "Security" in result.plan_sections - - @pytest.mark.asyncio - async def test_run_with_plan_approval_max_iterations(self, mock_llm_provider, mock_llm, mock_planner_llm): - """Test plan approval auto-approves after max iterations.""" - complete_response = ClarificationResponse(needs_clarification=False, clarification_question=None) - mock_llm.ainvoke = AsyncMock(return_value=AIMessage(content=complete_response.model_dump_json())) - - plan_response = '{"title": "Test Plan", "sections": ["Section 1"]}' - mock_planner_llm.ainvoke = AsyncMock(return_value=AIMessage(content=plan_response)) - - # User keeps providing feedback - mock_user_callback = AsyncMock(return_value="make it better") - - agent = ClarifierAgent( - llm_provider=mock_llm_provider, - user_prompt_callback=mock_user_callback, - enable_plan_approval=True, - planner_llm=mock_planner_llm, - max_plan_iterations=2, # Low iteration limit - ) - - state = ClarifierAgentState(messages=[HumanMessage(content="Research AI")]) - result = await agent.run(state) - - assert result is not None - assert result.plan_approved is True # Auto-approved after max iterations - - @pytest.mark.asyncio - async def test_run_with_plan_approval_fallback_plan(self, mock_llm_provider, mock_llm, mock_planner_llm): - """Test plan approval uses fallback when LLM returns invalid plan.""" - complete_response = ClarificationResponse(needs_clarification=False, clarification_question=None) - mock_llm.ainvoke = AsyncMock(return_value=AIMessage(content=complete_response.model_dump_json())) - - # LLM returns invalid plan - mock_planner_llm.ainvoke = AsyncMock(return_value=AIMessage(content="not valid json")) - - mock_user_callback = AsyncMock(return_value="approve") - - agent = ClarifierAgent( - llm_provider=mock_llm_provider, - user_prompt_callback=mock_user_callback, - enable_plan_approval=True, - planner_llm=mock_planner_llm, - ) - - state = ClarifierAgentState(messages=[HumanMessage(content="Research AI")]) - result = await agent.run(state) - - assert result is not None - assert result.plan_approved is True - # Should use fallback plan - assert result.plan_title == "Research Report" - assert "Introduction" in result.plan_sections - - @pytest.mark.asyncio - async def test_run_with_plan_approval_zero_iterations(self, mock_llm_provider, mock_llm, mock_planner_llm): - """Test plan approval with zero max_plan_iterations uses fallback values.""" - complete_response = ClarificationResponse(needs_clarification=False, clarification_question=None) - mock_llm.ainvoke = AsyncMock(return_value=AIMessage(content=complete_response.model_dump_json())) - - agent = ClarifierAgent( - llm_provider=mock_llm_provider, - user_prompt_callback=AsyncMock(), - enable_plan_approval=True, - planner_llm=mock_planner_llm, - max_plan_iterations=0, # Zero iterations - ) - - state = ClarifierAgentState(messages=[HumanMessage(content="Research AI")]) - result = await agent.run(state) - - assert result is not None - # Should auto-approve with fallback values (fix for the undefined variable bug) - assert result.plan_approved is True - assert result.plan_title == "Research Report" - assert "Introduction" in result.plan_sections - - -class TestClarifierAgentPlanApprovalInit: - """Tests for plan approval initialization settings.""" - - @pytest.fixture - def mock_llm_provider(self): - """Create a mock LLM provider.""" - llm = MagicMock() - llm.bind_tools = MagicMock(return_value=llm) - provider = MagicMock(spec=LLMProvider) - provider.get = MagicMock(return_value=llm) - return provider - - def test_init_with_plan_approval_disabled(self, mock_llm_provider): - """Test initialization with plan approval disabled (default).""" - agent = ClarifierAgent( - llm_provider=mock_llm_provider, - user_prompt_callback=AsyncMock(), - ) - - assert agent.enable_plan_approval is False - assert agent.max_plan_iterations == 10 - - def test_init_with_plan_approval_enabled(self, mock_llm_provider): - """Test initialization with plan approval enabled.""" - agent = ClarifierAgent( - llm_provider=mock_llm_provider, - user_prompt_callback=AsyncMock(), - enable_plan_approval=True, - ) - - assert agent.enable_plan_approval is True - - def test_init_with_custom_max_plan_iterations(self, mock_llm_provider): - """Test initialization with custom max_plan_iterations.""" - agent = ClarifierAgent( - llm_provider=mock_llm_provider, - user_prompt_callback=AsyncMock(), - max_plan_iterations=5, - ) - - assert agent.max_plan_iterations == 5 - - def test_init_with_planner_llm(self, mock_llm_provider): - """Test initialization with separate planner LLM.""" - planner_llm = MagicMock() - agent = ClarifierAgent( - llm_provider=mock_llm_provider, - user_prompt_callback=AsyncMock(), - planner_llm=planner_llm, - ) - - assert agent.planner_llm == planner_llm - - class TestHasToolInvocations: """Tests for the _has_tool_invocations helper.""" @@ -1341,96 +917,6 @@ async def user_callback(question: str) -> str: got = len(completion_ais) assert got == 1, f"expected exactly one terminal completion AIMessage, got {got}" - @pytest.mark.asyncio - async def test_skip_with_plan_approval_planner_gets_valid_sequence(self, mock_llm_provider, mock_llm): - """With plan approval on, the corrupted skip history previously flowed - into the planner's ainvoke. Assert the planner never receives two - consecutive assistant messages.""" - clarif = AIMessage( - content=ClarificationResponse( - needs_clarification=True, clarification_question="What aspect?" - ).model_dump_json() - ) - mock_llm.ainvoke = AsyncMock(side_effect=[clarif]) - - planner_llm = MagicMock() - plan_json = '{"title": "Plan", "sections": ["Intro", "Analysis"]}' - planner_llm.ainvoke = AsyncMock(return_value=AIMessage(content=plan_json)) - - replies = iter(["skip", "approve"]) - - async def user_callback(question: str) -> str: - """Return the canned user reply for this test.""" - return next(replies) - - agent = ClarifierAgent( - llm_provider=mock_llm_provider, - tools=[], - user_prompt_callback=user_callback, - enable_plan_approval=True, - planner_llm=planner_llm, - ) - - state = ClarifierAgentState(messages=[HumanMessage(content="Research AI")]) - result = await agent.run(state) - - assert result is not None - assert result.plan_approved is True - # The planner was called; none of its input message lists may contain - # consecutive assistant messages. - assert planner_llm.ainvoke.call_count == 1 - for call_idx, call in enumerate(planner_llm.ainvoke.call_args_list): - sent = call.args[0] - offenders = _adjacent_assistant_pairs(sent) - assert not offenders, f"planner ainvoke #{call_idx} had consecutive assistant messages at {offenders}" - - @pytest.mark.asyncio - async def test_blank_skip_reply_does_not_persist_empty_human_message(self, mock_llm_provider, mock_llm): - """A blank skip reply must not put an empty HumanMessage into history. - - ``_is_skip_command`` treats ``""``/whitespace as skip, so the skip branch - substitutes a non-empty sentinel. With plan approval enabled the history - flows into the planner's ainvoke; an empty-content message there is - rejected by some chat APIs. Assert no empty HumanMessage reaches the - planner. Regression test for the blank-skip-reply edge case. - """ - clarif = AIMessage( - content=ClarificationResponse( - needs_clarification=True, clarification_question="What aspect?" - ).model_dump_json() - ) - mock_llm.ainvoke = AsyncMock(side_effect=[clarif]) - - planner_llm = MagicMock() - planner_llm.ainvoke = AsyncMock( - return_value=AIMessage(content='{"title": "Plan", "sections": ["Intro", "Analysis"]}') - ) - - # First reply is blank (still a skip command); then approve the plan. - replies = iter(["", "approve"]) - - async def user_callback(question: str) -> str: - """Return the canned user reply for this test.""" - return next(replies) - - agent = ClarifierAgent( - llm_provider=mock_llm_provider, - tools=[], - user_prompt_callback=user_callback, - enable_plan_approval=True, - planner_llm=planner_llm, - ) - - state = ClarifierAgentState(messages=[HumanMessage(content="Research AI")]) - result = await agent.run(state) - - assert result is not None - assert planner_llm.ainvoke.call_count == 1 - for call_idx, call in enumerate(planner_llm.ainvoke.call_args_list): - sent = call.args[0] - empty = [i for i, m in enumerate(sent) if isinstance(m, HumanMessage) and not str(m.content).strip()] - assert not empty, f"planner ainvoke #{call_idx} received empty HumanMessage(s) at {empty}" - class TestClarifierReviewRegressions: """Regression tests for edge cases surfaced in deep review of #245.""" @@ -1491,46 +977,6 @@ async def test_force_search_fires_despite_prior_turn_tool_calls(self, mock_llm_p second_call_messages = mock_llm.ainvoke.call_args_list[1].args[0] assert any(isinstance(m, SystemMessage) and FORCE_SEARCH_GUIDANCE in m.content for m in second_call_messages) - @pytest.mark.asyncio - async def test_exhausted_entry_with_clarification_last_no_adjacent_assistants(self, mock_llm_provider, mock_llm): - """If the budget is already exhausted and the last message is a - non-complete clarification AIMessage, the completion must be interleaved - with a sentinel user turn so no two assistant messages are adjacent - (which would 400 the planner under plan approval).""" - planner_llm = MagicMock() - planner_llm.ainvoke = AsyncMock( - return_value=AIMessage(content='{"title": "Plan", "sections": ["Intro", "Analysis"]}') - ) - - agent = ClarifierAgent( - llm_provider=mock_llm_provider, - tools=[], - user_prompt_callback=AsyncMock(return_value="approve"), - enable_plan_approval=True, - planner_llm=planner_llm, - ) - - # Externally-provided state at exhaustion (max_turns=0) whose last turn - # is a non-complete clarification AIMessage. - clarif = AIMessage( - content=ClarificationResponse( - needs_clarification=True, clarification_question="What aspect?" - ).model_dump_json() - ) - state = ClarifierAgentState( - messages=[HumanMessage(content="Research AI"), clarif], - max_turns=0, - ) - result = await agent.run(state) - - assert result is not None - # The planner must never receive two consecutive assistant messages. - assert planner_llm.ainvoke.call_count == 1 - for call_idx, call in enumerate(planner_llm.ainvoke.call_args_list): - sent = call.args[0] - offenders = _adjacent_assistant_pairs(sent) - assert not offenders, f"planner ainvoke #{call_idx} had consecutive assistant messages at {offenders}" - @pytest.mark.asyncio async def test_exhausted_entry_with_pending_tool_call_is_not_completed_directly(self, mock_llm_provider, mock_llm): """At exhaustion, if the last message is an AIMessage with pending tool