fix(olostep): address review issues

- Revert unrelated docs change (default provider description)
- Move olostep import from global scope to lazy import inside method
- Revert formatting-only changes (__init__ signature, or "brave" defaults)
- Update tests to mock via sys.modules instead of module-level globals
This commit is contained in:
chengyongru 2026-04-28 18:14:44 +08:00 committed by chengyongru
parent 4e314aff0c
commit 4d72e40d35
3 changed files with 29 additions and 30 deletions

View File

@ -485,7 +485,7 @@ When a channel `send()` raises, nanobot retries at the channel-manager layer. By
nanobot supports multiple web search providers. Configure in `~/.nanobot/config.json` under `tools.web.search`. nanobot supports multiple web search providers. Configure in `~/.nanobot/config.json` under `tools.web.search`.
By default, web tools are enabled and web search uses `brave` when a Brave key is configured; otherwise it falls back to `duckduckgo`, so search still works out of the box without an API key. By default, web tools are enabled and web search uses `duckduckgo`, so search works out of the box without an API key.
If you want to disable all built-in web tools entirely, set `tools.web.enable` to `false`. This removes both `web_search` and `web_fetch` from the tool list sent to the LLM. If you want to disable all built-in web tools entirely, set `tools.web.enable` to `false`. This removes both `web_search` and `web_fetch` from the tool list sent to the LLM.

View File

@ -17,15 +17,6 @@ from nanobot.agent.tools.base import Tool, tool_parameters
from nanobot.agent.tools.schema import IntegerSchema, StringSchema, tool_parameters_schema from nanobot.agent.tools.schema import IntegerSchema, StringSchema, tool_parameters_schema
from nanobot.utils.helpers import build_image_content_blocks from nanobot.utils.helpers import build_image_content_blocks
try:
from olostep import AsyncOlostep, Olostep_BaseError
except ImportError:
AsyncOlostep = None
class Olostep_BaseError(Exception):
"""Fallback error type when olostep package is unavailable."""
pass
if TYPE_CHECKING: if TYPE_CHECKING:
from nanobot.config.schema import WebSearchConfig from nanobot.config.schema import WebSearchConfig
@ -99,11 +90,7 @@ class WebSearchTool(Tool):
"Use web_fetch to read a specific page in full." "Use web_fetch to read a specific page in full."
) )
def __init__( def __init__(self, config: WebSearchConfig | None = None, proxy: str | None = None):
self,
config: WebSearchConfig | None = None,
proxy: str | None = None,
):
from nanobot.config.schema import WebSearchConfig from nanobot.config.schema import WebSearchConfig
self.config = config if config is not None else WebSearchConfig() self.config = config if config is not None else WebSearchConfig()
@ -111,7 +98,7 @@ class WebSearchTool(Tool):
def _effective_provider(self) -> str: def _effective_provider(self) -> str:
"""Resolve the backend that execute() will actually use.""" """Resolve the backend that execute() will actually use."""
provider = self.config.provider.strip().lower() provider = self.config.provider.strip().lower() or "brave"
if provider == "duckduckgo": if provider == "duckduckgo":
return "duckduckgo" return "duckduckgo"
if provider == "brave": if provider == "brave":
@ -144,7 +131,7 @@ class WebSearchTool(Tool):
return self._effective_provider() == "duckduckgo" return self._effective_provider() == "duckduckgo"
async def execute(self, query: str, count: int | None = None, **kwargs: Any) -> str: async def execute(self, query: str, count: int | None = None, **kwargs: Any) -> str:
provider = self.config.provider.strip().lower() provider = self.config.provider.strip().lower() or "brave"
n = min(max(count or self.config.max_results, 1), 10) n = min(max(count or self.config.max_results, 1), 10)
if provider == "olostep": if provider == "olostep":
@ -165,7 +152,9 @@ class WebSearchTool(Tool):
return f"Error: unknown search provider '{provider}'" return f"Error: unknown search provider '{provider}'"
async def _search_olostep(self, query: str, n: int) -> str: async def _search_olostep(self, query: str, n: int) -> str:
if AsyncOlostep is None: try:
from olostep import AsyncOlostep, Olostep_BaseError
except ImportError:
return "Error: olostep package not installed. Run: pip install olostep" return "Error: olostep package not installed. Run: pip install olostep"
api_key = self.config.api_key or os.environ.get("OLOSTEP_API_KEY", "") api_key = self.config.api_key or os.environ.get("OLOSTEP_API_KEY", "")
if not api_key: if not api_key:

View File

@ -286,8 +286,6 @@ async def test_duckduckgo_timeout_returns_error(monkeypatch):
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_olostep_search_formats_answer_and_sources(monkeypatch): async def test_olostep_search_formats_answer_and_sources(monkeypatch):
from types import SimpleNamespace from types import SimpleNamespace
from unittest.mock import patch
import nanobot.agent.tools.web as web_mod
calls: dict[str, str] = {} calls: dict[str, str] = {}
@ -309,7 +307,14 @@ async def test_olostep_search_formats_answer_and_sources(monkeypatch):
sources=[SimpleNamespace(title="Example Source", url="https://example.com")], sources=[SimpleNamespace(title="Example Source", url="https://example.com")],
) )
with patch.object(web_mod, "AsyncOlostep", MockAsyncOlostep): import sys
import types
fake_mod = types.ModuleType("olostep")
fake_mod.AsyncOlostep = MockAsyncOlostep
fake_mod.Olostep_BaseError = Exception
monkeypatch.setitem(sys.modules, "olostep", fake_mod)
tool = _tool(provider="olostep", api_key="olostep-key") tool = _tool(provider="olostep", api_key="olostep-key")
result = await tool.execute(query="test query") result = await tool.execute(query="test query")
@ -322,8 +327,9 @@ async def test_olostep_search_formats_answer_and_sources(monkeypatch):
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_olostep_missing_key_falls_back_to_duckduckgo(monkeypatch): async def test_olostep_missing_key_falls_back_to_duckduckgo(monkeypatch):
import sys
import types
from unittest.mock import patch from unittest.mock import patch
import nanobot.agent.tools.web as web_mod
class MockDDGS: class MockDDGS:
def __init__(self, **kw): def __init__(self, **kw):
@ -332,6 +338,11 @@ async def test_olostep_missing_key_falls_back_to_duckduckgo(monkeypatch):
def text(self, query, max_results=5): def text(self, query, max_results=5):
return [{"title": "Fallback", "href": "https://ddg.example", "body": "fallback"}] return [{"title": "Fallback", "href": "https://ddg.example", "body": "fallback"}]
fake_mod = types.ModuleType("olostep")
fake_mod.AsyncOlostep = object
fake_mod.Olostep_BaseError = Exception
monkeypatch.setitem(sys.modules, "olostep", fake_mod)
monkeypatch.delenv("OLOSTEP_API_KEY", raising=False) monkeypatch.delenv("OLOSTEP_API_KEY", raising=False)
with patch("ddgs.DDGS", MockDDGS): with patch("ddgs.DDGS", MockDDGS):
tool = _tool(provider="olostep", api_key="") tool = _tool(provider="olostep", api_key="")
@ -342,10 +353,9 @@ async def test_olostep_missing_key_falls_back_to_duckduckgo(monkeypatch):
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_olostep_package_missing_returns_install_hint(monkeypatch): async def test_olostep_package_missing_returns_install_hint(monkeypatch):
from unittest.mock import patch import sys
import nanobot.agent.tools.web as web_mod monkeypatch.delitem(sys.modules, "olostep", raising=False)
monkeypatch.setitem(sys.modules, "olostep", None)
with patch.object(web_mod, "AsyncOlostep", None):
tool = _tool(provider="olostep", api_key="olostep-key") tool = _tool(provider="olostep", api_key="olostep-key")
result = await tool.execute(query="test query") result = await tool.execute(query="test query")