mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(community): normalize max_results before handing it to Firecrawl and fastCRW (#6175)
* fix(community): normalize max_results before handing it to Firecrawl and fastCRW * test(community): pin the explicit-null max_results shape The parametrized `omitted` case reused None as its "key absent" marker, so `extra["max_results"] = configured` never ran for None and the key-present, value-null YAML shape never reached _coerce_max_results. Use a distinct sentinel for absence and add an explicit-null case per provider. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
fafe904106
commit
8cd7aef578
@ -1,4 +1,5 @@
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
|
||||
from firecrawl import FirecrawlApp
|
||||
@ -7,11 +8,30 @@ from langchain.tools import tool
|
||||
from deerflow.community.url_safety import validate_public_http_url
|
||||
from deerflow.config import get_app_config
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# fastCRW is a Firecrawl-compatible web data engine (single Rust binary; self-host
|
||||
# or cloud). Because the REST API is Firecrawl-compatible, this provider reuses the
|
||||
# Firecrawl client and only swaps the base URL. Cloud default points at the managed
|
||||
# service; override `base_url` in the tool config (or set CRW_API_URL) for self-host.
|
||||
DEFAULT_BASE_URL = "https://fastcrw.com/api"
|
||||
DEFAULT_MAX_RESULTS = 5
|
||||
|
||||
|
||||
def _coerce_max_results(value: object) -> int:
|
||||
"""Normalize the configured ``max_results`` before handing it to the client."""
|
||||
if isinstance(value, bool) or (isinstance(value, float) and not value.is_integer()):
|
||||
# int() accepts booleans and silently truncates a YAML value such as 3.5.
|
||||
count = 0
|
||||
else:
|
||||
try:
|
||||
count = int(value) # type: ignore[call-overload]
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
count = 0
|
||||
if count <= 0:
|
||||
logger.warning("Invalid fastCRW max_results=%r; using default %s", value, DEFAULT_MAX_RESULTS)
|
||||
return DEFAULT_MAX_RESULTS
|
||||
return count
|
||||
|
||||
|
||||
def _get_fastcrw_client(tool_name: str = "web_search") -> FirecrawlApp:
|
||||
@ -56,9 +76,9 @@ def web_search_tool(query: str) -> str:
|
||||
"""
|
||||
try:
|
||||
config = get_app_config().get_tool_config("web_search")
|
||||
max_results = 5
|
||||
max_results = DEFAULT_MAX_RESULTS
|
||||
if config is not None:
|
||||
max_results = config.model_extra.get("max_results", max_results)
|
||||
max_results = _coerce_max_results(config.model_extra.get("max_results", max_results))
|
||||
|
||||
client = _get_fastcrw_client("web_search")
|
||||
result = client.search(query, limit=max_results)
|
||||
|
||||
@ -34,6 +34,25 @@ async def _aclose_firecrawl_client(client: AsyncFirecrawlApp) -> None:
|
||||
logger.warning("Failed to close the Firecrawl async HTTP pool", exc_info=True)
|
||||
|
||||
|
||||
DEFAULT_MAX_RESULTS = 5
|
||||
|
||||
|
||||
def _coerce_max_results(value: object) -> int:
|
||||
"""Normalize the configured ``max_results`` before handing it to Firecrawl."""
|
||||
if isinstance(value, bool) or (isinstance(value, float) and not value.is_integer()):
|
||||
# int() accepts booleans and silently truncates a YAML value such as 3.5.
|
||||
count = 0
|
||||
else:
|
||||
try:
|
||||
count = int(value) # type: ignore[call-overload]
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
count = 0
|
||||
if count <= 0:
|
||||
logger.warning("Invalid Firecrawl max_results=%r; using default %s", value, DEFAULT_MAX_RESULTS)
|
||||
return DEFAULT_MAX_RESULTS
|
||||
return count
|
||||
|
||||
|
||||
def _get_firecrawl_client(tool_name: str = "web_search") -> AsyncFirecrawlApp:
|
||||
config = get_app_config().get_tool_config(tool_name)
|
||||
api_key = None
|
||||
@ -59,9 +78,9 @@ async def web_search_tool(query: str) -> str:
|
||||
client: AsyncFirecrawlApp | None = None
|
||||
try:
|
||||
config = get_app_config().get_tool_config("web_search")
|
||||
max_results = 5
|
||||
max_results = DEFAULT_MAX_RESULTS
|
||||
if config is not None:
|
||||
max_results = config.model_extra.get("max_results", max_results)
|
||||
max_results = _coerce_max_results(config.model_extra.get("max_results", max_results))
|
||||
|
||||
client = _get_firecrawl_client("web_search")
|
||||
result = await client.search(query, limit=max_results)
|
||||
|
||||
@ -2,6 +2,7 @@
|
||||
|
||||
import ipaddress
|
||||
import json
|
||||
import logging
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
@ -190,3 +191,47 @@ class TestWebFetchTool:
|
||||
"http://10.0.0.5/dashboard",
|
||||
formats=["markdown"],
|
||||
)
|
||||
|
||||
|
||||
# `None` is itself a configured value (`max_results:` with nothing after it in YAML), so an absent key
|
||||
# needs its own sentinel — otherwise the key-present case is never built and never tested.
|
||||
_OMITTED = object()
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("configured", "expected_limit", "warns"),
|
||||
[
|
||||
pytest.param(True, 5, True, id="bool"),
|
||||
pytest.param(0, 5, True, id="zero"),
|
||||
pytest.param(-3, 5, True, id="negative"),
|
||||
pytest.param(3.5, 5, True, id="fractional"),
|
||||
pytest.param("many", 5, True, id="non-numeric-string"),
|
||||
pytest.param("8", 8, False, id="integer-string"),
|
||||
pytest.param(7, 7, False, id="plain-int"),
|
||||
pytest.param(None, 5, True, id="explicit-null"),
|
||||
pytest.param(_OMITTED, 5, False, id="omitted"),
|
||||
],
|
||||
)
|
||||
@patch.dict("os.environ", {}, clear=True)
|
||||
@patch("deerflow.community.fastcrw.tools.FirecrawlApp")
|
||||
@patch("deerflow.community.fastcrw.tools.get_app_config")
|
||||
def test_search_normalizes_max_results_before_calling_the_client(mock_get_app_config, mock_fastcrw_cls, caplog, configured, expected_limit, warns):
|
||||
search_config = MagicMock()
|
||||
extra: dict[str, object] = {"api_key": "fastcrw-search-key"}
|
||||
if configured is not _OMITTED:
|
||||
extra["max_results"] = configured
|
||||
search_config.model_extra = extra
|
||||
mock_get_app_config.return_value.get_tool_config.return_value = search_config
|
||||
|
||||
mock_result = MagicMock()
|
||||
mock_result.web = []
|
||||
mock_fastcrw_cls.return_value.search.return_value = mock_result
|
||||
|
||||
from deerflow.community.fastcrw.tools import web_search_tool
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="deerflow.community.fastcrw.tools"):
|
||||
result = web_search_tool.invoke({"query": "q"})
|
||||
|
||||
assert result == "[]"
|
||||
mock_fastcrw_cls.return_value.search.assert_called_once_with("q", limit=expected_limit)
|
||||
assert ("Invalid fastCRW max_results" in caplog.text) is warns
|
||||
|
||||
@ -279,3 +279,47 @@ class TestPerCallClientTeardown:
|
||||
result = await web_search_tool.ainvoke({"query": "test query"})
|
||||
|
||||
assert result == "[]"
|
||||
|
||||
|
||||
# `None` is itself a configured value (`max_results:` with nothing after it in YAML), so an absent key
|
||||
# needs its own sentinel — otherwise the key-present case is never built and never tested.
|
||||
_OMITTED = object()
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("configured", "expected_limit", "warns"),
|
||||
[
|
||||
pytest.param(True, 5, True, id="bool"),
|
||||
pytest.param(0, 5, True, id="zero"),
|
||||
pytest.param(-3, 5, True, id="negative"),
|
||||
pytest.param(3.5, 5, True, id="fractional"),
|
||||
pytest.param("many", 5, True, id="non-numeric-string"),
|
||||
pytest.param("8", 8, False, id="integer-string"),
|
||||
pytest.param(7, 7, False, id="plain-int"),
|
||||
pytest.param(None, 5, True, id="explicit-null"),
|
||||
pytest.param(_OMITTED, 5, False, id="omitted"),
|
||||
],
|
||||
)
|
||||
@patch("deerflow.community.firecrawl.tools.AsyncFirecrawlApp")
|
||||
@patch("deerflow.community.firecrawl.tools.get_app_config")
|
||||
@pytest.mark.anyio
|
||||
async def test_search_normalizes_max_results_before_calling_the_client(mock_get_app_config, mock_firecrawl_cls, caplog, configured, expected_limit, warns):
|
||||
search_config = MagicMock()
|
||||
extra: dict[str, object] = {"api_key": "firecrawl-search-key"}
|
||||
if configured is not _OMITTED:
|
||||
extra["max_results"] = configured
|
||||
search_config.model_extra = extra
|
||||
mock_get_app_config.return_value.get_tool_config.return_value = search_config
|
||||
|
||||
mock_result = MagicMock()
|
||||
mock_result.web = []
|
||||
mock_firecrawl_cls.return_value.search = AsyncMock(return_value=mock_result)
|
||||
|
||||
from deerflow.community.firecrawl.tools import web_search_tool
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="deerflow.community.firecrawl.tools"):
|
||||
result = await web_search_tool.ainvoke({"query": "q"})
|
||||
|
||||
assert result == "[]"
|
||||
mock_firecrawl_cls.return_value.search.assert_called_once_with("q", limit=expected_limit)
|
||||
assert ("Invalid Firecrawl max_results" in caplog.text) is warns
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user