fix(community): normalize max_results before handing it to Firecrawl and fastCRW - #6175

This is now the third near-identical copy of _coerce_max_results (ddg_search, firecrawl, fastcrw), on top of the ~10 per-provider _coerce_* guards already merged. deerflow.community already hosts shared helpers (url_safety, search_time_range), so a single coerce_max_results(value, default, label) there would keep the bool/float/int rules from drifting apart the next time one copy gets a fix and the others don't. Following the existing per-provider convention is fine, so deferring to a follow-up is reasonable — just noting the duplication has now tripled.

Agreed, and it is the reason I hesitated before adding a third copy. I stayed with the per-provider shape because the existing guards are not one function wearing three hats — measured on this tree: ddg_search and image_search are the unbounded form my two copy (reject bool, reject fractional float, <= 0 -> warn + default, no upper bound); serper and serply take default/max_allowed and clamp with min(...) instead of rejecting, and serper falls back without a log line; exa._coerce_positive_int accepts on isinstance(value, int) plus str.isdecimal() and threads an option name through its message.

So a shared coerce_max_results(value, default, label) has to settle three policies at once — whether an upper bound exists, whether a silent fallback is allowed, and whether fractional floats truncate or warn — across six call sites, which is its own change with its own tests rather than something to fold into this diff. Your drift argument is exactly why that refactor deserves to be reviewed on its own. Happy to take it on as a follow-up if you want it; otherwise I will leave this as it stands.

omitted uses None as the key-absent sentinel, so the explicit max_results: null YAML shape — key present with a None value, which reaches _coerce_max_results and exercises the int(None) -> TypeError path — is never tested. It behaves correctly today (warning + default), but a dedicated case with a distinct sentinel object instead of None would pin that path down. Same applies to the mirrored test in test_fastcrw_tools.py.

Good catch — None was doing double duty, so extra["max_results"] = configured never ran for it and the key-present / value-null shape was never built. 24f8fb5 switches absence to a _OMITTED sentinel and adds an explicit-null case here and in test_fastcrw_tools.py. The two cases differ in the observable that matters: explicit-null warns, omitted does not, which is what pins the int(None) -> TypeError branch down (measured: _coerce_max_results(None) returns 5 with a warning in both providers).

pytest tests/test_firecrawl_tools.py tests/test_fastcrw_tools.py -k normalizes_max_results is 18 passed, up from 16; the only failures left in those two files are the three TestWebFetchTool cases that need a resolver, which this sandbox does not have.