feat(config): render web.backend/search_backend/extract_backend as Config Form selects (#71929) - #71958
Conversation
4b30a1d to
b3dbc04
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing a verified Config Form gap: current main still infers the three web.*_backend fields as strings through hermes_cli/web_server.py:1044-1083.
Problems
hermes_cli/web_server.py:1245-1248preserves only an already-configured unknown provider. Web providers are registered and enumerable throughagent/web_search_registry.py:78-82, while the Form select renders only supplied options (web/src/components/AutoField.tsx:116-128). A newly installed, unconfigured plugin therefore cannot be selected from this UI.tests/hermes_cli/test_web_server.py:5327and:5339freeze exact provider sets.AGENTS.md:1333-1380requires behavior/invariant tests rather than change-detector snapshots.
Suggested changes
- Populate per-capability options from the web registry, filter by
supports_search()/supports_extract(), and separately retain stale configured values. Cover an unconfigured custom provider. - Replace exact-set assertions with capability-based invariants.
Automated hermes-sweeper review.
| current-value preservation used for tts/stt/memory providers. Returns the | ||
| ``base`` list unchanged when nothing needs appending. | ||
| """ | ||
| current = str(configured or "").strip() |
There was a problem hiding this comment.
This only retains a custom backend after it is already configured. agent.web_search_registry.list_providers() exposes installed providers, but a closed select cannot choose an unconfigured one unless it is added here. Enumerate registry providers per capability and preserve configured-but-undiscoverable names separately.
| from hermes_cli.web_server import CONFIG_SCHEMA | ||
|
|
||
| offered = {o for o in CONFIG_SCHEMA["web.extract_backend"]["options"] if o} | ||
| assert offered == {"firecrawl", "tavily", "exa", "parallel"} |
There was a problem hiding this comment.
Please avoid freezing this current provider enumeration. Test the relation to provider capability flags instead, including a registered custom provider; AGENTS.md prohibits change-detector snapshots for evolving provider lists.
b3dbc04 to
08b2d61
Compare
…nfig Form selects (NousResearch#71929) The Config page's Form view rendered web.backend, web.search_backend and web.extract_backend as bare text inputs, forcing users to know exact backend strings (e.g. 'brave-free', not 'brave') with no hint of valid values and no validation. Add _SCHEMA_OVERRIDES entries turning all three into selects, following the existing terminal.backend/tts.provider pattern. Option lists mirror the runtime: search/backend offer the built-in _LEGACY_WEB_BACKENDS set from tools/web_tools.py; extract_backend offers only the extract-capable subset (firecrawl, tavily, exa, parallel) since search-only providers are rejected by the runtime. A leading '' keeps the shared-backend/auto-detect fallback selectable. Web backends are plugin-extensible and the dashboard renders a select as a closed gate, so preserve a configured value outside the built-in list via _schema_with_dynamic_provider_options (same current-value preservation used for tts/stt/memory providers) — a plugin/custom backend never silently vanishes from the dropdown.
08b2d61 to
daca098
Compare
|
Both points addressed in 1. Enumerate the registry, don't just retain the configured value. You're right — the old helper only preserved a configured backend, so a closed select could never reach an installed-but-unconfigured provider.
Order is base built-ins → newly-discovered registry names → the configured-but-undiscoverable fallback (still preserved so switching away never drops it). The registry read is wrapped fail-safe: any import/registry error returns 2. Test capability flags, not a frozen list. Replaced the snapshot approach with relation tests: The two static tests ( |
SummaryOne PR addresses issue #71929. #71958 converts all three reported web backend fields from unrestricted text inputs to selects, adds capability-based discovery of installed registry providers, and preserves configured-but-undiscoverable values, but its tests still include the exact provider-set assertions challenged in review. Related pull requests
Suggested consolidationKeep #71958 open with a salvage path: retain the select overrides, capability-filtered registry enumeration, custom-provider discovery, and configured-value fallback, but replace the remaining exact built-in provider-set assertions with behavior/invariant coverage and obtain review clearance. There are no duplicate PRs to close. 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
I71929(["issue #71929 (open)"])
P71958["PR #71958 (open)"]
P71958 -->|best fix| I71929
class I71929 open
class P71958 open
class P71958 best
class P71958 target
click I71929 "https://github.com/NousResearch/hermes-agent/issues/71929"
click P71958 "https://github.com/NousResearch/hermes-agent/pull/71958"
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 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 16 kB of PR diffs, 5 kB of issue/PR text, 5 kB of discussion (5 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
What & why
Fixes #71929.
The Config page's Form view (
/config→ Web section) renderedweb.backend,web.search_backend, andweb.extract_backendas bare text<input>s. Users had to know the exact backend string (e.g.brave-free, notbrave) with no hint of valid values and no validation — a silent-failure trap, while sibling fields (terminal.backend,tts.provider,display.skin, …) have long been dropdowns.Change
Add three
_SCHEMA_OVERRIDESentries inhermes_cli/web_server.pyturning all three fields intoselects, following the existingterminal.backendpattern. The schema feeds every surface that reads/api/config/schema(desktop, CLI, dashboard), so all three get the dropdown.Option lists are mirrored from the runtime, not hand-copied from the issue:
web.backend/web.search_backend→ the built-in_LEGACY_WEB_BACKENDSset intools/web_tools.py(firecrawl,searxng,brave-free,ddgs,tavily,exa,parallel,xai). A test asserts the options equal that set so the two never drift.web.extract_backend→ only the extract-capable subset (firecrawl,tavily,exa,parallel). Search-only providers (searxng,brave-free,ddgs,xai) are rejected at runtime with a "search-only backend" error, so they're excluded here.""keeps the shared-backend / auto-detect fallback selectable.Plugin-backend safety
Web backends are plugin-extensible (
agent/web_search_registry), and the dashboard renders aselectas a closed gate (unlike the free-input voice/model fields). A naive static select would silently drop a configured plugin/custom backend and let the next save clobber it. To avoid that regression,_schema_with_dynamic_provider_options()now preserves a configured value that falls outside the built-in list — the same current-value preservation already used fortts/stt/memoryproviders. The common case (built-in or blank) returns the frozen import-time entry untouched.Tests
tests/hermes_cli/test_web_server.py:"";_LEGACY_WEB_BACKENDS(drift guard);uv run pytest tests/hermes_cli/test_web_server.py— all schema tests green.Note: the local footgun preflight flags 10 pre-existing bare
write_text()calls elsewhere intest_web_server.py; none are in this change, and the CIWindows footgunscheck scans production packages only (nottests/), so it passes clean. Those belong to the separate #71014 read_text/write_text campaign, not this PR.