fix(web): load plugins before resolving extract backend - #67110
fix(web): load plugins before resolving extract backend#67110xzeronelysium wants to merge 1 commit into
Conversation
web_extract_tool resolved web.extract_backend before _ensure_web_plugins_loaded(). Plugin backends like "local" are not in _LEGACY_WEB_BACKENDS, so availability is a registry lookup. With an empty registry the configured extract backend looked unavailable and fell through to web.backend (e.g. searxng), which is search-only. Load plugins first — same order web_search_tool already uses — so registry-backed extract providers are visible when availability is probed.
alt-glitch
added
type/bug
Related to merged #34563: current main already performs plugin discovery before resolving the extract backend. Please rebase to demonstrate any remaining delta. |
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the resolution-order issue. The premise remains valid on current main: tools/web_tools.py:858 calls _get_extract_backend() before discovery at tools/web_tools.py:867; non-legacy backend availability uses the registry at tools/web_tools.py:324-327.
Problems
- The patch has no regression test for this ordering.
tests/tools/test_web_providers.py:350-435configures legacyfirecrawland asserts only that discovery was called, so it does not fail when backend resolution happens first.
Suggested changes
- Add a test with an initially empty registry where discovery registers an extract-only custom provider,
web.extract_backendselects it, andweb.backendissearxng. Assert extraction uses the custom provider rather than the search-only error path attools/web_tools.py:882-894.
This is an automated hermes-sweeper review.
| if not safe_urls: | ||
| results = [] | ||
| else: | ||
| # Ensure plugin-registered providers (including custom |
There was a problem hiding this comment.
Please add a regression test for this earlier placement. The existing discovery test uses legacy firecrawl and only checks that the hook ran; it does not prove that a plugin-only extract_backend is resolved after discovery rather than falling back to a shared searxng backend.
teknium1
added
sweeper:risk-compatibility
|
Thank you for working on #67110. I appreciate the effort to fix the extract-dispatcher path for plugin-registered providers. PR #67309 takes a slightly different approach by moving _ensure_web_plugins_loaded() to the two shared chokepoints _get_backend() (line ~230) and _is_backend_available() (line ~324) in tools/web_tools.py. This ensures plugin discovery happens before backend selection for all three call sites:
This matches the project's rubric in AGENTS.md for fixing the whole bug class (sibling call paths included), rather than just the single path addressed in #67110. Give PR #67309 a try and let me know if you have any issues. |
SummaryTwenty-four PRs address or reference this complex across three causes: cold-registry discovery, hardcoded provider selection/tool gates, and non-actionable failure guidance. Merged #34563 covers discovery before live dispatcher lookups and merged #57779 covers registry-aware provider selection, while #67110, #67309, #71791, and #72646 expose a remaining selection-before-discovery path and #58359 separately targets guidance. Related pull requests
Duplicates#27584, #27700, #28202, and #69144 overlap on discovery-before-dispatch, with #34563 as the merged reference; #31829, #31887, #32465, #33516, #33902, #36094, #38375, #52057, #56201, and #56761 overlap with the provider-gate class implemented by #57779. #67110, #67309, #71791, and the web-selection portion of #72646 overlap on discovery-before-selection; #58359 is the distinct failure-guidance change. Suggested consolidationClose #67110 as a duplicate of #67309 despite the keep_open review on #67110: its eight-line dispatcher move is contained by #67309's shared-gate diff, and #67309 now includes the requested cold-registry tests. Keep #67309 open with the salvage path of rebasing onto current main and validating its shared-gate tests; author action on #71791 is to reuse the existing discovery helper or close it as duplicate of #67309, while #72646 should split out the display fix and add check_web_api_key() coverage, and #58359 should address its contributor-reviewed guidance/test issues. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I27683(["issue #27683 (closed)"])
subgraph Dup67110 ["PRs duplicating each other"]
P67110["PR #67110 (open)"]
P72646["PR #72646 (open)"]
end
P67110 -.->|partial| I27683
class I27683 closed
class P67110 open
class P72646 open
class P67110 target
click I27683 "https://github.com/NousResearch/hermes-agent/issues/27683"
click P67110 "https://github.com/NousResearch/hermes-agent/pull/67110"
click P72646 "https://github.com/NousResearch/hermes-agent/pull/72646"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 24 pull requests and 8 issues in this complex. Each diff was read against this issue; Assessment working set: 147 kB of PR diffs, 118 kB of issue/PR text, 64 kB of discussion (79 comments), 96 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
alt-glitch
added
duplicate
Duplicate of #73183 — both load web plugins before resolving the configured extract backend, preventing plugin-only extract providers from falling back to the search backend on cold start. |
alt-glitch
added
comp/tools
5 participants
Summary
web_extract_toolresolvedweb.extract_backendbefore calling_ensure_web_plugins_loaded(). For plugin-registered extract backends (e.g.local/ web-local), availability is a registry lookup because the name is not in_LEGACY_WEB_BACKENDS. With plugins not loaded yet, the registry was empty, so the configured backend looked unavailable and silently fell through to sharedweb.backend(oftensearxng). SearXNG is search-only → users got:…even when extract was correctly configured, the plugin was enabled, and trafilatura was installed.
web_search_toolalready loads plugins before resolving the search backend. This PR applies the same order to extract.Change
_ensure_web_plugins_loaded()above_get_extract_backend()inweb_extract_toolTest plan
web.search_backend=searxng,web.extract_backend=local, web-local enabled:_get_extract_backend()→searxng(wrong)_get_extract_backend()→local;_get_search_backend()stillsearxngweb_extract(["https://example.com"])returns content via trafilaturaweb_searchstill succeeds via SearXNGNotes
Not a config problem — config was already correct. This is a logic/ordering bug in tool dispatch that affects any harness path using Hermes web tools with a plugin-only extract backend.