feat(web): expose limit for web_search#16808
Conversation
7c7c0ce to
8d63e82
Compare
|
CI note after rebasing onto latest upstream main ( |
There was a problem hiding this comment.
Pull request overview
This PR enhances the web_search tool by exposing an optional limit parameter in the tool schema and wiring it through to web_search_tool(), while keeping the default behavior (5 results) unchanged.
Changes:
- Added optional
limit(min 1, max 100, default 5) toWEB_SEARCH_SCHEMA, plus updated schema/query descriptions to mention common query operators. - Plumbed
limitthrough the registeredweb_searchhandler intoweb_search_tool(). - Added runtime coercion/clamping for
limitand added tests covering schema shape, handler wiring/defaulting, and clamping behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tools/web_tools.py |
Adds limit to schema, passes it through registry handler, and clamps/coerces runtime limit to 1..100. |
tests/tools/test_web_tools_config.py |
Adds targeted tests for schema/handler wiring and runtime clamping behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "limit": { | ||
| "type": "integer", | ||
| "description": "Maximum number of results to return. Defaults to 5.", | ||
| "minimum": 1, | ||
| "maximum": 100, | ||
| "default": 5 |
There was a problem hiding this comment.
WEB_SEARCH_SCHEMA hardcodes limit bounds/default (1..100, default 5) separately from the runtime clamp and handler default. To prevent future inconsistencies, consider referencing shared constants for these values (or generating the schema from them).
| toolset="web", | ||
| schema=WEB_SEARCH_SCHEMA, | ||
| handler=lambda args, **kw: web_search_tool(args.get("query", ""), limit=5), | ||
| handler=lambda args, **kw: web_search_tool(args.get("query", ""), limit=args.get("limit", 5)), |
There was a problem hiding this comment.
The handler default for limit is another hardcoded '5'. If you introduce shared constants for limit defaults/bounds, wire them through here as well so schema + runtime behavior stay in sync.
| try: | ||
| limit = int(limit) | ||
| except (TypeError, ValueError): | ||
| limit = 5 | ||
| limit = min(max(limit, 1), 100) |
There was a problem hiding this comment.
The limit sanitization duplicates the default value (5) and clamp bounds (1..100) as literals. Consider defining module-level constants (e.g., WEB_SEARCH_DEFAULT_LIMIT/MIN/MAX) and using them here to avoid the schema/handler/default getting out of sync if the defaults change later.
What does this PR do?
Adds an optional
limitparameter to theweb_searchtool schema and wires it through to the existingweb_search_tool()handler.The default stays at 5 results, so existing tool calls keep the same behavior. Callers can now request a larger or smaller result set when needed.
Related Issue
Fixes #16696
Type of Change
Changes Made
limitto theweb_searchtool schema with min1, max100, and default5.web_search_tool().1..100before calling the configured backend.site:,filetype:,intitle:,-term, and exact phrases, while noting that support depends on the backend.How to Test
source /Users/blackishgreen03/workspace/hermes-agent/venv/bin/activate && python -m pytest tests/tools/test_web_tools_config.py -q49 passedpython -m pytest tests/ -q, but this local environment is not clean for the whole repository: it fails with missing optional packages such asacp,numpy, andfastapi, plus many unrelated existing gateway/web/tool failures (16556 passed, 117 skipped, 191 failed, 66 errors).Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
N/A
Screenshots / Logs
Targeted test: