Skip to content

fix(web): implement extract() for the ddgs backend - #65197

Open
wreed4 wants to merge 1 commit into
NousResearch:mainfrom
wreed4:fix/ddgs-extract-support
Open

wreed4 wants to merge 1 commit into
NousResearch:mainfrom
wreed4:fix/ddgs-extract-support

Conversation

@wreed4

@wreed4 wreed4 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Problem

The ddgs web backend is registered as search-only (supports_extract() -> False), so web_extract always fails with:

DuckDuckGo (ddgs) is a search-only backend and cannot extract URL content. Set web.extract_backend to firecrawl, tavily, exa, or parallel.

But the ddgs package itself ships a working extract() (also exposed as ddgs extract on the CLI) that fetches a URL and returns clean markdown/text via readability parsing — confirmed manually:

$ ddgs extract -u "https://example.com/some-post" -f text_markdown
# ... clean markdown output ...

So the plugin was leaving a real, no-API-key capability on the table and forcing users who prefer ddgs (no key, no vendor lock-in) into paid extract backends or full browser automation for pages that a plain HTTP fetch would have handled fine.

Fix

  • DDGSWebSearchProvider.supports_extract() -> True
  • Added DDGSWebSearchProvider.extract(urls, **kwargs) backed by DDGS().extract(url, fmt=...)
    • Per-URL fetch in its own worker thread with a hard wall-clock cap (_EXTRACT_TIMEOUT_SECS), mirroring the existing search() timeout guard from ddgs web search provider hangs indefinitely — no overall timeout on search calls #36776 — a hung fetch must not block the shared agent loop.
    • Per-URL failures (missing package, timeout, ddgs exceptions) become result entries with an error field instead of aborting the whole batch, matching the tavily/exa provider contract.
    • format="markdown"/"html"/"text" maps to ddgs's fmt="text_markdown"/"text"/"text_plain".
  • Updated get_setup_schema() copy to reflect the new capability and document the known limitation below.

Known limitation

ddgs does a plain HTTP GET + HTML parse — no JS rendering. Client-rendered SPAs will only yield page-shell content (nav/loading skeleton). That's inherent to the approach, not a regression; users who need JS rendering should still reach for a browser-based extraction path or a rendering-capable backend (firecrawl/parallel/etc). Documented in the provider's get_setup_schema() tag.

Testing

Extended tests/tools/test_web_providers_ddgs.py:

  • supports_extract() now True
  • happy path, multi-URL batch, format→fmt mapping
  • missing package / runtime error / timeout -> per-URL error entries (batch doesn't abort on one bad URL)
  • replaced the now-stale "search-only error" dispatch test with one asserting web_extract_tool successfully dispatches to and returns content from ddgs
$ pytest tests/tools/test_web_providers_ddgs.py tests/integration/test_web_tools.py -q
27 passed in 0.92s

Also manually verified against the real ddgs package (not just the test stub) via ddgs extract -u <url> -f text_markdown against both a static/server-rendered page (clean full extraction) and a client-rendered SPA (confirmed page-shell-only, as expected/documented).

The DDGS package (via ``ddgs extract``/``DDGS().extract()``) already
supports fetching and readability-parsing a URL into clean markdown/text,
but the ddgs plugin's WebSearchProvider hardcoded supports_extract() ->
False and web_extract_tool surfaced a blanket "DuckDuckGo (ddgs) is a
search-only backend" error for every URL regardless of page type.

Add DDGSWebSearchProvider.extract(), backed by DDGS().extract(url, fmt=...):
- Per-URL fetch in its own worker thread with a hard wall-clock cap
  (_EXTRACT_TIMEOUT_SECS), mirroring the existing search() timeout guard
  for NousResearch#36776 — a hung fetch must not block the shared agent loop.
- Per-URL failures (missing package, timeout, ddgs exceptions) become
  result entries with an "error" field rather than aborting the whole
  batch, matching the tavily/exa provider contract.
- format="markdown"/"html"/"text" maps to ddgs's fmt="text_markdown"/
  "text"/"text_plain".
- supports_extract() now returns True, so web_extract_tool dispatches to
  ddgs like any other extract-capable provider instead of returning the
  search-only error.

Known limitation (documented in get_setup_schema's tag): ddgs does a
plain HTTP fetch + HTML parse, no JS rendering, so client-rendered SPAs
still return only page-shell content. That's an inherent ddgs limitation,
not a regression — users needing JS rendering should pair ddgs with a
browser-based extraction path or a rendering-capable backend.

Tests: extended tests/tools/test_web_providers_ddgs.py with extract()
coverage (happy path, multi-URL, format mapping, missing package,
runtime errors, one-bad-url-doesn't-abort-batch, timeout) and replaced
the now-stale "search-only error" dispatch test with one asserting ddgs
successfully serves web_extract_tool.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins tool/web Web search and extraction sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades needs-decision Awaiting maintainer decision before any implementation labels Jul 15, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for identifying a real DDGS capability gap: current main returns False from DDGSWebSearchProvider.supports_extract() at plugins/web/ddgs/provider.py:91-92, and the dispatcher rejects it as search-only (tools/web_tools.py:875-894). The upstream DDGS API does provide extract(url, fmt).

Problems

  • The new direct fetch at plugins/web/ddgs/provider.py:206 has no redirect/final-URL safety or website-policy revalidation. web_extract_tool validates only the submitted URL (tools/web_tools.py:840-852), while Firecrawl explicitly re-checks redirected final URLs (plugins/web/firecrawl/provider.py:527-550).
  • The PR leaves public documentation and setup output claiming DDGS is search-only: website/docs/user-guide/features/web-search.md:24-30, website/docs/developer-guide/web-search-provider-plugin.md:160, and hermes_cli/tools_config.py:1364.

Suggested changes

  • Add redirect-aware SSRF/policy enforcement and regression tests before enabling local DDGS extraction.
  • Update the affected user/developer docs, translations, optional DDGS skill guidance, and post-setup wording.

Automated hermes-sweeper review.

for url in urls:
pool = _cf.ThreadPoolExecutor(max_workers=1)
try:
future = pool.submit(_run_ddgs_extract, url, fmt)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The dispatcher validates only the submitted URL (tools/web_tools.py:840-852); this direct local fetch has no redirect or final-URL revalidation. Firecrawl performs both checks (plugins/web/firecrawl/provider.py:527-550). Add an equivalent redirect-safe fetch/guard and regression test before enabling DDGS extraction.


def supports_extract(self) -> bool:
return False
return True

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This capability change leaves current user and developer docs plus the DDGS post-setup message incorrectly describing DDGS as search-only (website/docs/user-guide/features/web-search.md:24-30, website/docs/developer-guide/web-search-provider-plugin.md:160, hermes_cli/tools_config.py:1364). Please update those surfaces and their translations.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary

Three PRs are considered: #60528 and #60552 address #60425 by registering DDGS for lazy installation, while #65197 implements the separate DDGS extraction capability and does not change the sealed-venv dependency path. #60528 provides the shared provider/tool availability chokepoint and regression coverage; #60552 applies a similar mechanism but deletes most of the lazy-deps implementation; #65197 enables extraction without redirect-aware policy revalidation.

Related pull requests

Duplicates

#60552 substantially duplicates #60528's search.ddgs allowlist, pinned extra, and provider lazy-install path; #60552 is already closed in favor of the safer and better-tested #60528. #65197 is not a duplicate because it changes DDGS extraction rather than sealed-venv availability.

Suggested consolidation

Keep #60528 open with a salvage path: preserve its recorded best-fix implementation and have the author regenerate and commit uv.lock before reevaluation. Keep #60552 closed as a duplicate of #60528, and handle #65197 independently with the redirect-safety, policy-revalidation, test, and documentation actions identified in its keep_open review.

Cross-PR triage: Reviewed 3 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 56 kB of PR diffs, 6 kB of issue/PR text, 4 kB of discussion (5 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

This branch has not been deployed

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

Labels

comp/plugins Plugin system and bundled plugins needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/web Web search and extraction type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants