-
Notifications
You must be signed in to change notification settings - Fork 52.7k
feat(model-switch): add excluded_providers config + fix custom provider grouping for shared-endpoint proxies #28218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
craigdfrench
wants to merge
2
commits into
NousResearch:main
from
craigdfrench:feat/excluded-providers-and-proxy-grouping
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
128 changes: 128 additions & 0 deletions
128
tests/hermes_cli/test_model_picker_excluded_providers.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| """Tests that ``model_catalog.excluded_providers`` hides providers from the | ||
| interactive ``hermes model`` CLI picker. | ||
|
|
||
| The CLI picker (``hermes_cli.main.select_provider_and_model``) builds its | ||
| provider menu from ``CANONICAL_PROVIDERS`` via ``group_providers`` — a | ||
| separate code path from ``list_authenticated_providers``. These tests | ||
| verify the exclusion config is honored there too, matching the | ||
| gateway/TUI picker behavior. | ||
| """ | ||
|
|
||
| from unittest.mock import patch | ||
|
|
||
| import pytest | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def config_home(tmp_path, monkeypatch): | ||
| """Isolated HERMES_HOME with a minimal config.""" | ||
| home = tmp_path / "hermes" | ||
| home.mkdir() | ||
| config_yaml = home / "config.yaml" | ||
| config_yaml.write_text("model: old-model\ncustom_providers: []\n") | ||
| env_file = home / ".env" | ||
| env_file.write_text("") | ||
| monkeypatch.setenv("HERMES_HOME", str(home)) | ||
| monkeypatch.delenv("HERMES_MODEL", raising=False) | ||
| monkeypatch.delenv("LLM_MODEL", raising=False) | ||
| monkeypatch.delenv("HERMES_INFERENCE_PROVIDER", raising=False) | ||
| monkeypatch.delenv("OPENAI_BASE_URL", raising=False) | ||
| monkeypatch.delenv("OPENAI_API_KEY", raising=False) | ||
| return home | ||
|
|
||
|
|
||
| def _write_config(home, **top_level): | ||
| import yaml | ||
| cfg = {"model": "old-model", "custom_providers": []} | ||
| cfg.update(top_level) | ||
| (home / "config.yaml").write_text(yaml.safe_dump(cfg)) | ||
|
|
||
|
|
||
| def _capture_provider_labels(config_home): | ||
| """Drive ``select_provider_and_model`` and return the provider-menu labels | ||
| shown to the user (the first ``_prompt_provider_choice`` call). Cancels | ||
| immediately after capturing.""" | ||
| from hermes_cli.main import select_provider_and_model | ||
|
|
||
| captured: dict = {} | ||
|
|
||
| def _capture_and_cancel(labels, default=0, title=None): | ||
| # Only capture the top-level provider menu (the first call). | ||
| if "labels" not in captured: | ||
| captured["labels"] = list(labels) | ||
| return None # cancel | ||
|
|
||
| with patch("hermes_cli.main._prompt_provider_choice", | ||
| side_effect=_capture_and_cancel), \ | ||
| patch("builtins.print"): | ||
| select_provider_and_model() | ||
|
|
||
| return captured.get("labels", []) | ||
|
|
||
|
|
||
| def test_cli_picker_hides_excluded_provider(config_home): | ||
| """``excluded_providers: [openrouter]`` must remove the OpenRouter row | ||
| from the ``hermes model`` provider menu.""" | ||
| _write_config(config_home, **{"model_catalog": {"excluded_providers": ["openrouter"]}}) | ||
|
|
||
| labels = _capture_provider_labels(config_home) | ||
| assert labels, "provider menu was empty" | ||
| assert not any("OpenRouter" in lbl for lbl in labels), ( | ||
| f"OpenRouter should be hidden by excluded_providers, got: {labels}" | ||
| ) | ||
|
|
||
|
|
||
| def test_cli_picker_hides_excluded_provider_by_alias(config_home): | ||
| """Exclusion by an alias (not the canonical slug) must also hide the | ||
| provider, matching ``list_authenticated_providers``' matching against | ||
| hermes_id / alias names.""" | ||
| # 'openai' is an alias-style hermes id; ensure excluding it hides the | ||
| # canonical openai provider row if present. Use the canonical slug's | ||
| # alias from _PROVIDER_ALIASES to stay robust to renames. | ||
| from hermes_cli.models import _PROVIDER_ALIASES, CANONICAL_PROVIDERS | ||
|
|
||
| # Find a canonical provider that has at least one alias and is a leaf | ||
| # row (not folded into a multi-member group) so its label appears | ||
| # directly. Pick the first such provider. | ||
| target_slug = None | ||
| target_alias = None | ||
| for alias, canon in _PROVIDER_ALIASES.items(): | ||
| if canon and any(p.slug == canon for p in CANONICAL_PROVIDERS): | ||
| target_slug = canon | ||
| target_alias = alias | ||
| break | ||
| if target_slug is None: | ||
| pytest.skip("no aliased canonical provider available to test") | ||
|
|
||
| from hermes_cli.models import _PROVIDER_LABELS | ||
| target_label_fragment = _PROVIDER_LABELS.get(target_slug, target_slug) | ||
|
|
||
| # Baseline: the provider appears without exclusion. | ||
| _write_config(config_home) | ||
| baseline = _capture_provider_labels(config_home) | ||
| assert any(target_label_fragment in lbl for lbl in baseline), ( | ||
| f"sanity: {target_slug} ({target_label_fragment!r}) should appear by " | ||
| f"default; labels={baseline}" | ||
| ) | ||
|
|
||
| # Excluding by alias hides it. | ||
| _write_config( | ||
| config_home, | ||
| **{"model_catalog": {"excluded_providers": [target_alias]}}, | ||
| ) | ||
| excluded_labels = _capture_provider_labels(config_home) | ||
| assert not any(target_label_fragment in lbl for lbl in excluded_labels), ( | ||
| f"excluding alias {target_alias!r} should hide {target_slug}; " | ||
| f"labels={excluded_labels}" | ||
| ) | ||
|
|
||
|
|
||
| def test_cli_picker_empty_excluded_is_noop(config_home): | ||
| """An empty ``excluded_providers`` list must not change the menu.""" | ||
| _write_config(config_home, **{"model_catalog": {"excluded_providers": []}}) | ||
| excluded_labels = _capture_provider_labels(config_home) | ||
|
|
||
| _write_config(config_home) | ||
| baseline_labels = _capture_provider_labels(config_home) | ||
|
|
||
| assert excluded_labels == baseline_labels |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only feeds inventory consumers. The gateway
/modelpath still callslist_picker_providers/list_authenticated_providersdirectly, so the new config would not hide providers on that picker surface unless those call paths also receive the exclusion list.