Repository navigation
Config-driven override for the Gemma picker-visibility filter - #62746
rrschott-ai wants to merge 2 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for making the existing picker filter configurable; the underlying limitation is present on current main (agent/models_dev.py:567-597).
Problems
- The new override un-hides every
_GOOGLE_HIDDEN_MODELSmember. That set also contains stale/retired Gemini slugs that are explicitly documented as 404ing atagent/models_dev.py:557-563, not only low-TPM Gemma IDs. Preserve the stale-model filter while allowing the intended Gemma subset. - Commit
d5b3da2also addsagent.skill_auto_patch, which is outside the PR summary and has no implementation consumer on current main (rg -n 'skill_auto_patch' .returned no matches). Please remove this unrelated config addition from the change. - No tests are included. Extend
tests/hermes_cli/test_gemini_provider.py:309-356to cover the configured Gemma override on both catalog paths and verify stale Gemini IDs remain hidden.
Suggested changes
- Separate the low-TPM Gemma IDs from stale Google IDs, and consult the opt-in list only for the former.
- Keep the change scoped to the picker override and add targeted regression coverage.
Automated hermes-sweeper review.
| def _should_hide_from_provider_catalog(provider: str, model_id: str) -> bool: | ||
| provider_lower = (provider or "").strip().lower() | ||
| model_lower = (model_id or "").strip().lower() | ||
| if provider_lower in {"gemini", "google"} and model_lower in _GOOGLE_HIDDEN_MODELS: | ||
| return True | ||
| return model_lower not in _google_show_hidden_overrides() | ||
| return False |
There was a problem hiding this comment.
_GOOGLE_HIDDEN_MODELS also contains stale/retired Gemini IDs that main documents as 404ing (agent/models_dev.py:557-563). This permits users to re-add known-invalid models; restrict this override to the low-TPM Gemma subset and keep stale IDs unconditionally hidden.
… filter _GOOGLE_HIDDEN_MODELS (agent/models_dev.py) hides several Gemma IDs from the interactive model/fallback picker by default - their low TPM quotas trip under agent-style traffic for most users, a reasonable default. The only way to see a hidden model in the picker was editing the frozenset directly, which a `hermes update` silently wipes on the next release (source is reset to upstream), forcing a user who deliberately wants one of these models to reapply a source patch after every update. Add agent.models (config.yaml) -> models.show_hidden_google_models: a list of model IDs to force-show despite the built-in filter. Config lives outside the package and survives updates. Raw config entries (fallback_providers, a manually-typed model id) already bypassed this filter entirely and are unaffected - this only changes picker visibility. Empty by default, no behavior change for users who don't set it.
…, add tests Per hermes-sweeper's automated review on PR NousResearch#62746: - Split _GOOGLE_HIDDEN_MODELS into _GOOGLE_LOW_TPM_MODELS (the low-TPM Gemma ids the override is meant for) and _GOOGLE_STALE_MODELS (retired Google slugs that 404 on current endpoints - a fact, not a posture, so never eligible for the config override). _should_hide_from_provider_catalog now only consults show_hidden_google_models for the low-TPM set; a stale slug in that config list is silently ignored rather than un-hiding a broken model. - Dropped the agent.skill_auto_patch addition from hermes_cli/config.py - it leaked into this PR from an unrelated, separately-committed change in the same working file during branch prep. Not part of this PR's scope. - Added three tests to tests/hermes_cli/test_gemini_provider.py covering both catalog paths reviewer asked for: the override un-hiding a configured Gemma id while leaving other Gemma ids hidden, a stale slug in the override list staying hidden regardless, and default (no config) behavior unchanged. All pass, plus the existing suite in this file (4 pre-existing failures are an unrelated missing concurrent_log_handler dependency in this environment, not touched by this change).
d5b3da2 to
b9ef402
Compare
|
Addressed all three points, pushed as a new commit (b9ef402):
Thanks for catching the cross-contamination — verified with |
Summary
_GOOGLE_HIDDEN_MODELS(agent/models_dev.py) hides several Gemma IDs from the interactive model/fallback picker by default — their low TPM quotas trip under agent-style traffic for most users, which is a reasonable default.hermes updateresets that file to upstream, silently wiping the edit — a user who deliberately wants one of these models has to reapply a source patch after every release, with no warning it happened.models.show_hidden_google_modelstoconfig.yaml(default[], no behavior change unless set): a list of model IDs to force-show despite the built-in filter. Config lives outside the package, so it survives updates.fallback_providers, a manually-typed model id) already bypassed this filter entirely and worked fine — this only changes what the picker UI shows, matching the intent of the original filter (a default for typical users, not a hard block).Test plan
ast.parse)models.show_hidden_google_modelscontaining it, it's shown; an unrelated hidden id (not in the override list) stays hidden; a non-Google provider is unaffected; a config-load failure fails safe (stays hidden, no exception)config.yamlwith the override set — picker filter correctly un-hides only the configured idsmodels.show_hidden_google_modelsis the right home/name for this vs. e.g. folding it into the existingprovidersconfig dict