From 8cd7aef5789cfda758a83985e02edf8a63d6233f Mon Sep 17 00:00:00 2001 From: "Grapette.L" <90967079+Lesereingrape@users.noreply.github.com> Date: Fri, 2 Oct 2026 23:49:22 +0800 Subject: [PATCH] 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 --- .../deerflow/community/fastcrw/tools.py | 24 +++++++++- .../deerflow/community/firecrawl/tools.py | 23 +++++++++- backend/tests/test_fastcrw_tools.py | 45 +++++++++++++++++++ backend/tests/test_firecrawl_tools.py | 44 ++++++++++++++++++ 4 files changed, 132 insertions(+), 4 deletions(-) diff --git a/backend/packages/harness/deerflow/community/fastcrw/tools.py b/backend/packages/harness/deerflow/community/fastcrw/tools.py index 5bcdc18afe..8035eaa085 100644 --- a/backend/packages/harness/deerflow/community/fastcrw/tools.py +++ b/backend/packages/harness/deerflow/community/fastcrw/tools.py @@ -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) diff --git a/backend/packages/harness/deerflow/community/firecrawl/tools.py b/backend/packages/harness/deerflow/community/firecrawl/tools.py index ad969b1c6d..0837119d83 100644 --- a/backend/packages/harness/deerflow/community/firecrawl/tools.py +++ b/backend/packages/harness/deerflow/community/firecrawl/tools.py @@ -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) diff --git a/backend/tests/test_fastcrw_tools.py b/backend/tests/test_fastcrw_tools.py index 00afab9e7b..0aeb4c98b3 100644 --- a/backend/tests/test_fastcrw_tools.py +++ b/backend/tests/test_fastcrw_tools.py @@ -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 diff --git a/backend/tests/test_firecrawl_tools.py b/backend/tests/test_firecrawl_tools.py index 99afefacc4..cf8850f71e 100644 --- a/backend/tests/test_firecrawl_tools.py +++ b/backend/tests/test_firecrawl_tools.py @@ -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