Skip to content

fix(core): handle missing web search query params#7981

Open
whatevertogo wants to merge 5 commits intoAstrBotDevs:masterfrom
whatevertogo:fix/search-tool-empty-params-7499
Open

fix(core): handle missing web search query params#7981
whatevertogo wants to merge 5 commits intoAstrBotDevs:masterfrom
whatevertogo:fix/search-tool-empty-params-7499

Conversation

@whatevertogo
Copy link
Copy Markdown
Contributor

@whatevertogo whatevertogo commented May 3, 2026

Fixes #7499.

This PR prevents built-in web search tools from crashing when the LLM emits an empty or invalid query argument. The original issue showed web_search_tavily being called with {}, which raised KeyError: 'query' and fed an error back into the tool loop repeatedly.

Modifications / 改动点

  • This is NOT a breaking change. / 这不是一个破坏性变更。

  • Validate query before building provider payloads for Tavily, BoCha, Brave, Firecrawl, and Baidu AI Search.

  • Return a clear tool error when query is missing, null, or blank instead of raising KeyError/AttributeError.

  • Keep normal non-empty queries flowing into each provider payload unchanged, aside from trimming surrounding whitespace.

  • Add a regression test covering the affected built-in web search tools.

Screenshots or Test Results / 运行截图或测试结果

uv run pytest tests/unit/test_web_search_tools.py -q
...............                                                          [100%]
15 passed in 2.35s
uv run ruff check astrbot/core/tools/web_search_tools.py tests/unit/test_web_search_tools.py
All checks passed!

Checklist / 检查清单

  • 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
    / 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。

  • 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
    / 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”

  • 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.toml 文件相应位置。

  • 😮 My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码。

Summary by Sourcery

Handle invalid or missing search queries in built-in web search tools to avoid crashes and return clear error messages.

Bug Fixes:

  • Prevent Tavily, BoCha, Brave, Firecrawl, and Baidu web search tools from raising errors when the LLM omits or sends an empty/invalid query parameter.

Tests:

  • Add regression tests ensuring each built-in web search tool returns a friendly error string when the query argument is missing, null, or blank.

Web search tools previously indexed query directly, so model-generated empty arguments raised KeyError and could trigger repeated tool-call failures. Validate query before building provider payloads and return a clear tool error for missing, null, or blank inputs.

Constraint: Keep the fix scoped to built-in web search tools and their direct regression test.
Rejected: Do not add a broader schema-aware argument normalizer in this PR.
Directive: Treat missing, null, and blank web search query inputs as friendly tool errors.
Confidence: high
Scope-risk: low
Reversibility: straightforward
Tested: uv run pytest tests/unit/test_web_search_tools.py -q
Tested: uv run ruff check astrbot/core/tools/web_search_tools.py tests/unit/test_web_search_tools.py
Tested: git diff --check and git diff --cached --check
Related: AstrBotDevs#7499
Co-authored-by: OmX <omx@oh-my-codex.local>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces validation for the query parameter across several web search tools, including Tavily, BoCha, Brave, Firecrawl, and Baidu, ensuring that empty or missing queries return a friendly error message instead of causing a crash. Corresponding unit tests were added to verify this behavior. Feedback suggests refactoring the duplicated validation logic into a shared helper function to improve maintainability and removing a redundant str() conversion in the Baidu tool implementation.

Comment thread astrbot/core/tools/web_search_tools.py Outdated
Comment thread astrbot/core/tools/web_search_tools.py Outdated
@whatevertogo whatevertogo marked this pull request as ready for review May 3, 2026 17:30
@whatevertogo whatevertogo requested review from Copilot May 3, 2026 17:30
@auto-assign auto-assign Bot requested review from Soulter and advent259141 May 3, 2026 17:30
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. area:core The bug / feature is about astrbot's core, backend labels May 3, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The query normalization and error handling logic is duplicated across all the web search tools; consider extracting this into a shared helper (e.g., normalize_query(kwargs) returning either a trimmed query or a standardized error) to reduce repetition and keep behavior consistent.
  • In the Baidu tool test setup you have to special-case websearch_baidu_app_builder_key because it uses a string while the others use lists; aligning the provider_settings structure across tools would simplify this and avoid hidden assumptions about value types.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `query` normalization and error handling logic is duplicated across all the web search tools; consider extracting this into a shared helper (e.g., `normalize_query(kwargs)` returning either a trimmed query or a standardized error) to reduce repetition and keep behavior consistent.
- In the Baidu tool test setup you have to special-case `websearch_baidu_app_builder_key` because it uses a string while the others use lists; aligning the provider_settings structure across tools would simplify this and avoid hidden assumptions about value types.

## Individual Comments

### Comment 1
<location path="astrbot/core/tools/web_search_tools.py" line_range="427" />
<code_context>
         if not provider_settings.get("websearch_tavily_key", []):
             return "Error: Tavily API key is not configured in AstrBot."

+        query = str(kwargs.get("query") or "").strip()
+        if not query:
+            return "Error: 'query' parameter is required but was not provided."
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the repeated `query` parsing and validation into a shared helper function and calling it from each tool’s `call` method to avoid duplication.

You can reduce duplication and keep behavior identical by centralizing the `query` extraction/validation into a small helper, then using it in each tool.

For example, at module level:

```python
def _extract_query(kwargs: dict) -> str | None:
    query = str(kwargs.get("query") or "").strip()
    if not query:
        return None
    return query
```

Then in each `call` method:

```python
async def call(self, context, **kwargs) -> ToolExecResult:
    _, provider_settings, _ = _get_runtime(context)
    if not provider_settings.get("websearch_tavily_key", []):
        return "Error: Tavily API key is not configured in AstrBot."

    query = _extract_query(kwargs)
    if query is None:
        return "Error: 'query' parameter is required but was not provided."

    # rest of logic unchanged...
    payload = {
        "query": query,
        "max_results": kwargs.get("max_results", 7),
        "include_favicon": True,
        "search_depth": search_depth,
        "topic": topic,
    }
```

For Baidu:

```python
async def call(self, context, **kwargs) -> ToolExecResult:
    _, provider_settings, _ = _get_runtime(context)
    if not provider_settings.get("websearch_baidu_app_builder_key", ""):
        return "Error: Baidu AI Search API key is not configured in AstrBot."

    query = _extract_query(kwargs)
    if query is None:
        return "Error: 'query' parameter is required but was not provided."

    top_k = int(kwargs.get("top_k", 10))
    # ... bounds logic unchanged ...

    payload = {
        "messages": [{"role": "user", "content": query[:72]}],
        "search_source": "baidu_search_v2",
        "resource_type_filter": [{"type": "web", "top_k": top_k}],
    }
```

This keeps all current behavior (string coercion, stripping, error text) but moves the logic to a single place, lowering maintenance and cognitive overhead.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread astrbot/core/tools/web_search_tools.py Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a crash loop in AstrBot’s built-in web search tools when the LLM calls them with a missing/blank query, returning a friendly tool error instead of raising exceptions.

Changes:

  • Add query normalization/validation (missing/None/blank) in Tavily, BoCha, Brave, Firecrawl, and Baidu search tools.
  • Ensure provider payloads use the normalized query (trimmed) rather than direct kwargs["query"] access.
  • Add a regression test covering invalid query inputs across the affected tools.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
astrbot/core/tools/web_search_tools.py Adds defensive query validation to prevent KeyError/AttributeError and standardizes payload building.
tests/unit/test_web_search_tools.py Adds a parameterized async regression test to ensure tools return a friendly error string when query is invalid.

Comment thread astrbot/core/tools/web_search_tools.py Outdated
Comment thread astrbot/core/tools/web_search_tools.py Outdated
whatevertogo and others added 2 commits May 4, 2026 01:33
The PR repeated the same query normalization block in every web search provider. Extract the shared behavior into a named helper so missing, null, and blank query inputs keep one consistent validation path and error response.

Constraint: Keep the refactor local to astrbot/core/tools/web_search_tools.py.
Rejected: Do not introduce a broader tool-argument normalization layer for this review comment.
Directive: Reuse _validate_search_query for built-in web search providers before payload construction.
Confidence: high
Scope-risk: low
Reversibility: straightforward
Tested: uv run pytest tests/unit/test_web_search_tools.py -q
Tested: uv run ruff check astrbot/core/tools/web_search_tools.py tests/unit/test_web_search_tools.py
Tested: git diff --check
Related: AstrBotDevs#7499
Co-authored-by: OmX <omx@oh-my-codex.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core The bug / feature is about astrbot's core, backend size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]当给bot发送“查一下昨天发生的大事”之类的网页查询命令,会无限轮回报错。

2 participants