add bocha web search tool - #207
Conversation
|
@weijintaocode Heya, thanks for your contribution, could you please resolve the build errors? |
Eigenwise
left a comment
There was a problem hiding this comment.
PR Review: Add Bocha Web Search Tool
Thanks for contributing a new search tool! The overall structure follows the existing patterns well. However, there are several issues that need to be addressed before this can be merged.
Critical Issues
1. Wrong BaseTool generic type parameters — tool/bocha_search.py:54
The first type parameter must be the input schema, not the result item schema. This breaks input_schema, tool_name, and tool_description — fundamental to how agents discover and use tools.
# Current (wrong):
class BoChaSearchTool(BaseTool[BoChaSearchResultItemSchema, BoChaSearchToolOutputSchema]):
# Should be:
class BoChaSearchTool(BaseTool[BoChaSearchToolInputSchema, BoChaSearchToolOutputSchema]):Compare with TavilySearchTool and SearXNGSearchTool which both use BaseTool[InputSchema, OutputSchema].
2. Unsafe dictionary access — tool/bocha_search.py:101
results = data["data"]["webPages"]["value"]If the API returns an error response or unexpected structure, this crashes with a KeyError. Use .get() with validation.
3. Test file name typo — tests/test_bocha_seach.py
Missing 'r' in "search". Should be test_bocha_search.py.
4. Unused dependency — requirements.txt
sympy>=1.12,<2.0.0 is listed but never used in the tool.
README Issues
5. Parameter documented as api_url but the actual config field is api_key (README line 22)
6. Listed dependency is requests but the code uses aiohttp (README line 9)
7. Python version says "3.9 or later" but pyproject.toml requires >=3.12 (README line 7)
8. Example imports BoChaTool but the actual class is BoChaSearchTool (README line 56)
Code Issues
9. Type mismatch on snippet — tool/bocha_search.py:32
snippet: str = Field(None, ...) — type is str but default is None. Should be Optional[str].
10. Inconsistent env var naming — tool/bocha_search.py:73
BoCha_API_KEY uses mixed case. All other tools use uppercase: SEARXNG_BASE_URL, TAVILY_API_KEY. Should be BOCHA_API_KEY.
11. Hardcoded API key placeholder in __main__ block — tool/bocha_search.py:178
Uses api_key="sk-**************" instead of os.getenv("BOCHA_API_KEY"). Also passes max_results=2 but the config field is named count.
12. freshness field has no type constraint — tool/bocha_search.py:47
freshness: str accepts any string. Consider using Literal["noLimit", "day", "week", "month"] (similar to Tavily's search_depth: Literal[...] pattern).
13. asyncio.gather without return_exceptions=True — tool/bocha_search.py:115
If one query fails, all results are lost. Consider handling partial failures gracefully.
14. include/exclude sent as None — tool/bocha_search.py:89-90
When these are None, they're still included in the JSON payload. The API may reject null values — conditionally include them.
Test Coverage Gaps
The tests cover basic scenarios well but are missing:
- Empty queries list (
queries=[]) - Multiple concurrent queries (all tests use single query —
asyncio.gatheruntested) - Config params
freshness,include,excludebeing passed correctly - Malformed API response (missing
data/webPageskeys) - Verification that correct headers and payload are sent to the API
Minor
- Docstring at line 62 mentions
countas a public attribute — this is an internal config detail uv.lockdiff includes unrelated changes (greenlet wheel removals, pre-commit addition)
What looks good
- Directory structure follows existing tool patterns correctly
- Async/sync pattern matches SearXNG and Tavily
- Good use of
aiohttp.ClientSessionas context manager - Proper
pyproject.tomlwith workspace source - Tests for missing fields, empty results, and HTTP errors are solid
…type; update README and tests
Add a Bocha web search tool that enables agents to perform real-time, asynchronous web queries via the Bocha API.