Skip to content

feat(py): add web_fetch vended tool - #4001

Open
liramon2 wants to merge 11 commits into
strands-agents:mainfrom
liramon2:web_fetch_py
Open

feat(py): add web_fetch vended tool#4001
liramon2 wants to merge 11 commits into
strands-agents:mainfrom
liramon2:web_fetch_py

Conversation

@liramon2

@liramon2 liramon2 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Description

Adds web_fetch as a vended tool to the Python SDK. This implements the markdown conversion feature from the http_request tool in the tools repo. Combined with the http_request tool port, the web_fetch tool enables us to deprecate the tool repo's http_request. This PR combines the original web_fetch implementation and the Stan implementation.

  • This PR drops the SSRF protection from the original implementation because egress control belongs to the encapsulation layer, not in a single tool https://github.com/strands-agents/stan/pull/47.
  • This PR keeps the summarizer agent from the Stan implementation because it lets the model avoid adding the full page contents to its context, which is more efficient than reactively offloading via ContextOffloader.
  • Byte caps are useful to avoid buffering huge payloads but it is not configurable in httpx clients, so it is a new part of the web_fetch configs.

beautifulsoup4 and markdownify are new dependencies handling HTML to markdown conversion because of their light weight and prior usage in the tools repo. turndown will likely be used in the TS implementation, since it is already used in the site docs. While the markdown engines will be different, this tradeoff is acceptable because the model simply needs the page rendered in markdown without hard formatting requirements.

  • html-to-markdown was considered, but it is missing Pypi wheels for musllinux and Windows ARM. It is also much heavier at 7 MB, compared to beautifulsoup4 and markdownify (125 kB)
  • Manual parsing was rejected because it is difficult to maintain and we are not in the business of converting HTML to markdown.

Public API

Default web_fetch tool

from strands import Agent
from strands.vended_tools import web_fetch

agent = Agent(tools=[web_fetch])

Custom configuration

import httpx
from strands import Agent
from strands.vended_tools.web_fetch import make_web_fetch

client = httpx.AsyncClient(timeout=10.0)
agent = Agent(tools=[make_web_fetch(client=client, max_bytes=1 * 1024 * 1024)])

Related Issues

Closes #3239 on the Python side. TS implementation will be a follow-up.

Documentation PR

Documentation added in site/src/content/docs/user-guide/concepts/tools/vended-tools.mdx.

Type of Change

New feature

Testing

  • I ran hatch run prepare

Checklist

  • I have read the CONTRIBUTING document
  • I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
  • My change is focused and reasonably small; I have split unrelated work into separate PRs
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@liramon2

Copy link
Copy Markdown
Contributor Author

@strandly-the-agent Review this PR.

@github-actions github-actions Bot added enhancement New feature or request python Pull requests that update python code area-tool Tool behavior/api labels Aug 26, 2026
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.48276% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...-py/src/strands/vended_tools/web_fetch/_extract.py 91.48% 3 Missing and 1 partial ⚠️
strands-py/src/strands/vended_tools/__init__.py 60.00% 1 Missing and 1 partial ⚠️
...py/src/strands/vended_tools/web_fetch/web_fetch.py 97.75% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@github-actions github-actions Bot added complexity/high A touched function exceeds cognitive complexity 25; may be worth splitting size/l labels Aug 26, 2026
@liramon2

This comment was marked as outdated.

@github-actions github-actions Bot added complexity/medium Touched functions have moderate cognitive complexity (11-25) and removed complexity/high A touched function exceeds cognitive complexity 25; may be worth splitting labels Aug 26, 2026
@liramon2

This comment was marked as outdated.

@liramon2

Copy link
Copy Markdown
Contributor Author

@strandly-the-agent Review this PR.

@github-actions github-actions Bot added complexity/low Touched functions have low cognitive complexity (<=10) size/m and removed size/l complexity/medium Touched functions have moderate cognitive complexity (11-25) labels Aug 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Re-review (following commit 5afdafd — "reduce return value to only page content")

Assessment: Approve (non-blocking)

Reviewed the delta. The tool's return simplifies from the WebFetchOutput TypedDict to a bare str (markdown only). Clean change — the WebFetchOutput type is fully removed with no dangling references, the docs don't describe the old shape, and the tests were updated correctly (full-object equality where deterministic).

Details
  • Return contract: One non-blocking note left inline — a bare str forecloses adding response metadata later without a breaking change, and diverges from the structured http_request output. Worth confirming under api/needs-review.
  • Type hygiene: cast(str, ...) on convert_soup plus # type: ignore[import-not-found] on the bs4/markdownify imports are appropriate given those packages ship no stubs.
  • Cosmetic: the _DROPPED_ELEMENTS reformat is formatter-only, no behavior change.
  • Tests: updated to the str return; test_non_html_response_returns_body asserts the whole value, and markup cases use targeted substring checks (markdown formatting isn't deterministic) — good.

All earlier feedback remains resolved; no new blocking issues.

Comment thread strands-py/src/strands/vended_tools/web_fetch/web_fetch.py
@github-actions

Copy link
Copy Markdown
Contributor

Re-review (following commit 1e9e29e — "use httpx instead of urllib")

Assessment: Comment (one non-blocking Important item)

Solid migration. Moving to httpx.AsyncClient drops the to_thread wrapper (resolving my earlier note about the non-cancellable worker), enables streamed max_bytes enforcement, and brings web_fetch in line with the existing http_request tool — same operator-supplied client pattern and cancel-signal plumbing. httpx>=0.28.1,<1.0.0 is already a properly-bounded core dependency, so no new dependency concerns.

Details
  • Mid-flight cancellation (Important, non-blocking): inline on _fetch_once — the cancel signal is only checked pre-flight, never between streamed chunks, unlike http_request._read_body. Cancellation won't interrupt an in-flight body. Trivial fix + a mid-flight test would close the gap.
  • Doc accuracy (Suggestion): the timeout param was dropped (public-API change → worth the api/needs-review gate), the "configured timeout" docstring is now stale, and the default client sets follow_redirects=True rather than pure "httpx defaults."
  • Minor/DRY: web_fetch hand-rolls _parse_charset while http_request uses response.encoding; consider reusing the latter for consistency. Not blocking.
  • Tests: the rewrite onto httpx.MockTransport is clean and offline (scheme rejection, cap, error status, redirect passthrough, timeout mapping, header check all covered); full-object equality where deterministic, targeted substrings for markdown. Only cancellation coverage is incomplete (see above).

Nice, well-structured refactor — the parity with http_request makes the whole vended-tools surface more coherent.

Comment thread strands-py/src/strands/vended_tools/web_fetch/web_fetch.py
@github-actions

Copy link
Copy Markdown
Contributor

Re-review (following amend acb92bb — "use httpx instead of urllib")

Assessment: Comment (no new issues)

The only delta since my last pass is in web_fetch/_extract.py: dropping the # type: ignore[import-not-found] on the bs4/markdownify imports and the cast(str, ...) around convert_soup(...). That's the right call and effectively CI-required — beautifulsoup4 (4.15.0) and markdownify (1.2.3) now ship py.typed (markdownify's stub declares convert_soup(...) -> str), so under this repo's warn_unused_ignores + warn_redundant_casts mypy settings both the ignore and the cast would now be flagged. No functional change, and the cast removal is type-safe.

Two items from my prior review remain open and unaddressed (no code change in web_fetch.py):

  • Mid-flight cancellation (Important, non-blocking) — thread on _fetch_once, line 137.
  • Docstring accuracy / removed timeout param (Suggestion) — thread on line 52.

Both are non-blocking, so no objection to merging — just flagging that they're still standing in case you want to fold them in.

result = await summarizer.invoke_async(
f"Fetched URL: {url}\n\nRequest: {prompt}\n\n--- Content ---\n{content}"
)
return str(result)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue (Important, non-blocking): the full fetched content — up to max_bytes (5 MiB by default) — is injected verbatim into the summarizer prompt with no truncation. For large pages this can overflow the model's context window or cause large, surprising token costs, and it'll surface as an opaque model error rather than the tidy ValueError/TimeoutError the fetch path produces.

Suggestion: bound the content handed to the summarizer (e.g. a char/token budget with a clear "content truncated" marker), and/or wrap invoke_async failures into the tool's ValueError so summarizer errors are reported consistently with fetch errors.

)
result = await summarizer.invoke_async(
f"Fetched URL: {url}\n\nRequest: {prompt}\n\n--- Content ---\n{content}"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue (Important, non-blocking): the summarizer Agent is created fresh here and isn't connected to the host agent's cancel signal, so if the host is cancelled while this invoke_async LLM call is in flight, it won't abort. That's the most expensive operation in the whole tool, and it's currently the least cancellable — the fetch at least checks the signal (pre-flight), but the summarize step checks nothing.

Suggestion: propagate the host's _cancel_signal into the summarizer (or race the invoke_async coroutine against the cancel event and aclose/cancel on set), so cancellation behaves consistently across both phases.

@github-actions

Copy link
Copy Markdown
Contributor

Re-review (following commit 02313af — "feat: add summarizer to web_fetch")

Assessment: Request Changes (one design-coherence blocker; rest non-blocking)

Design contradiction (please reconcile before merge). This PR's own description states:

"This drops the summarizer agent from the Stan implementation because the user can include the ContextOffloader plugin if their fetched pages consume significant context."

…but this commit re-adds a summarizer sub-agent. That directly reverses a documented design decision, and it's a meaningful expansion of the tool's contract — web_fetch goes from a deterministic fetch/convert tool to one that spins up an Agent and makes an LLM call. Two things I'd want resolved:

  1. Is the summarizer intentional now? If yes, please update the PR description (including the "Public API" section, which still doesn't mention the new model= param or the model-facing prompt arg) and the rationale for choosing an in-tool summarizer over the ContextOffloader plugin it previously deferred to. If it was added experimentally, consider dropping it.
  2. This is squarely within the api/needs-review scope already on the PR — worth an explicit call-out to the API reviewer since it introduces LLM-invoking behavior inside a tool.
Other findings (non-blocking)
  • Summarizer bypasses cancellation (Important) — inline. The fresh Agent isn't wired to the host's cancel signal, so the most expensive operation (the summarize LLM call) can't be aborted mid-flight. Compounds the still-open pre-flight-only cancel gap in _fetch_once.
  • Unbounded content into the summarizer (Important) — inline. Up to max_bytes (5 MiB) of text is injected into the prompt with no truncation, risking context-window overflow / cost blowup on large pages.
  • Test fidelity (Suggestion) — all four summarizer tests monkeypatch Agent to return a bare str, so the real str(AgentResult) conversion, the system_prompt/callback_handler wiring, and any error handling around invoke_async are untested. (str(AgentResult) does return concatenated text, so the impl is correct — but the tests don't prove it.) The summarizer path also doesn't wrap invoke_async errors, unlike the fetch path.
  • Still open from prior rounds: mid-flight cancel in _fetch_once and the stale "configured timeout"/removed-timeout-param docstring.

The summarizer implementation itself is clean (fresh agent per call, no history bleed, sensible resolution order) — the main ask is reconciling it with the PR's stated design.

@github-actions

Copy link
Copy Markdown
Contributor

Re-review (following commit 58e53ea — "check cancel signal inside aiter_bytes() loop and clarify docstrings")

Assessment: Comment (progress; prior design blocker still stands)

Both items this commit targets are correctly resolved:

  • Mid-flight fetch cancellation_check_cancelled(cancel_signal) now runs inside the aiter_bytes() loop, so a cancel arriving during body streaming aborts the fetch. Matches the http_request._read_body pattern.
  • Docstring accuracy — the client=None doc now states follow_redirects=True + httpx's 5s default, and the stale "configured timeout" wording is fixed.

Still open (unchanged by this commit):

  • Summarizer cancellation (thread on line 137) — this fix covers only the fetch loop; an in-flight invoke_async still isn't cancellable.
  • Unbounded content into the summarizer (thread on line 138).
  • Design contradiction — the PR description still says the summarizer was dropped in favor of ContextOffloader, while the code adds one; please reconcile (and update the "Public API" section for model=/prompt).

Minor follow-on: the new mid-flight cancel path isn't tested — test_pre_flight_cancel_short_circuits only covers the pre-request case. A short "cancel set after the first chunk" test would lock in the new behavior.

Nice quick turnaround on the cancel/docstring fixes.

@github-actions

Copy link
Copy Markdown
Contributor

Re-review (force-push 58e53ea5bd2da7)

The web_fetch.py changes are unchanged from the prior tip; the new addition is test_mid_flight_cancel_aborts_between_chunks, which is exactly the coverage I flagged as missing last round. It sets the cancel event after the first chunk and asserts CancelledError on the next iteration — correctly exercising the between-chunks abort path with a clean pytest.raises. That item is now closed. ✅

Still open (unchanged by this push):

  • Summarizer cancellation (line 137 thread) — in-flight invoke_async still isn't cancellable.
  • Unbounded content into the summarizer (line 138 thread).
  • Design contradiction — PR description still says the summarizer was dropped in favor of ContextOffloader; please reconcile and update the "Public API" section for model=/prompt.

No new issues in this push — good follow-through on the cancel test.

provided by the operator, giving full control over transport configuration,
caching, proxies, redirects, and connection pooling.

When ``prompt`` is non-empty, a summarizer agent answers the prompt over the

@liramon2 liramon2 Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To the reviewer: what are your thoughts on DevX like this make_web_fetch(extractor="agentic | markdown") if web_fetch is doing too much right now? Stan has a use for the question/answer fetching and will likely be a consumer for this tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api/needs-review Makes changes to the public API surface area-tool Tool behavior/api complexity/medium Touched functions have moderate cognitive complexity (11-25) enhancement New feature or request python Pull requests that update python code size/m

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vended tools: new web_fetch tool (py + ts)

2 participants