fix: respect custom provider config and nested route denies
Resolve review feedback on PR #5313: - Read reasoning_efforts from named custom_providers entries for custom:<name> providers instead of looking only in providers:<name>. - Preserve the nested-route deny ordering before the provider-config shortcut so Gemini image/embedding routes cannot be re-enabled by config. - Deduplicate configured effort values before returning them. Add focused regression tests for bare provider config, named custom provider config, all-invalid fallthrough, ACP hard guards, and nested route denies.
This commit is contained in:
parent
b80ea016b1
commit
4f4c8fd183
2 changed files with 138 additions and 24 deletions
|
|
@ -3345,35 +3345,40 @@ def resolve_model_reasoning_efforts(
|
|||
if provider in {"cursor-acp", "copilot-acp"}:
|
||||
return []
|
||||
|
||||
# 0. Provider config: providers.<name>.reasoning_efforts
|
||||
# When the user has explicitly listed valid efforts for a provider,
|
||||
# return that list directly — no heuristics, no models.dev lookup.
|
||||
# Handles both custom:<name> and bare registered provider names (e.g. wandb).
|
||||
# Only short-circuits when the filtered list is non-empty; an all-invalid
|
||||
# list (e.g. typos) falls through to heuristics instead of hiding reasoning.
|
||||
_prov_name = None
|
||||
if provider and provider.startswith("custom:"):
|
||||
_prov_name = provider.split(":", 1)[1]
|
||||
elif provider:
|
||||
_prov_name = provider
|
||||
if _prov_name:
|
||||
try:
|
||||
_prov_entry = (cfg.get("providers") or {}).get(_prov_name, {})
|
||||
if isinstance(_prov_entry, dict):
|
||||
_re_list = _prov_entry.get("reasoning_efforts")
|
||||
if isinstance(_re_list, list) and _re_list:
|
||||
_filtered = [str(x).strip().lower() for x in _re_list
|
||||
if str(x).strip().lower() in {*VALID_REASONING_EFFORTS, "none"}]
|
||||
if _filtered:
|
||||
return _filtered
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
hinted_model = _strip_provider_hint_for_reasoning(model)
|
||||
|
||||
# Master hides reasoning controls for nested image/embedding routes. Keep
|
||||
# that hard deny above provider config so an explicit allowlist cannot
|
||||
# re-enable controls for routes that should never expose them.
|
||||
if _nested_route_reasoning_denied(hinted_model):
|
||||
return []
|
||||
|
||||
# 0. Provider config: providers.<name>.reasoning_efforts or named
|
||||
# custom_providers[].reasoning_efforts. When the user has explicitly listed
|
||||
# valid efforts for a provider, return that list directly — no heuristics,
|
||||
# no models.dev lookup.
|
||||
# Only short-circuits when the filtered list is non-empty; an all-invalid
|
||||
# list (e.g. typos) falls through to heuristics instead of hiding reasoning.
|
||||
_re_list = None
|
||||
try:
|
||||
if provider and provider.startswith("custom:"):
|
||||
for _entry in _custom_provider_entries():
|
||||
if _custom_provider_slug_from_name(_entry.get("name")) == provider:
|
||||
_re_list = _entry.get("reasoning_efforts")
|
||||
break
|
||||
elif provider:
|
||||
_prov_entry = (cfg.get("providers") or {}).get(provider, {})
|
||||
if isinstance(_prov_entry, dict):
|
||||
_re_list = _prov_entry.get("reasoning_efforts")
|
||||
if isinstance(_re_list, list) and _re_list:
|
||||
_filtered = [str(x).strip().lower() for x in _re_list
|
||||
if str(x).strip().lower() in {*VALID_REASONING_EFFORTS, "none"}]
|
||||
_filtered = list(dict.fromkeys(_filtered))
|
||||
if _filtered:
|
||||
return _filtered
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
if provider in {"copilot", "github-copilot"}:
|
||||
try:
|
||||
from hermes_cli.models import github_model_reasoning_efforts
|
||||
|
|
|
|||
|
|
@ -118,6 +118,115 @@ def test_non_reasoning_http_models_hide_reasoning_effort_levels():
|
|||
) == []
|
||||
|
||||
|
||||
def test_provider_config_reasoning_efforts_return_filtered_deduped(monkeypatch):
|
||||
original = cfg.cfg.get("providers")
|
||||
monkeypatch.setitem(
|
||||
cfg.cfg,
|
||||
"providers",
|
||||
{
|
||||
"wandb": {
|
||||
"reasoning_efforts": [
|
||||
" none ",
|
||||
"HIGH",
|
||||
"bogus",
|
||||
"high",
|
||||
"xhigh",
|
||||
]
|
||||
}
|
||||
},
|
||||
)
|
||||
try:
|
||||
assert cfg.resolve_model_reasoning_efforts(
|
||||
"zai-org/GLM-5.2",
|
||||
provider_id="wandb",
|
||||
) == ["none", "high", "xhigh"]
|
||||
finally:
|
||||
if original is None:
|
||||
cfg.cfg.pop("providers", None)
|
||||
else:
|
||||
monkeypatch.setitem(cfg.cfg, "providers", original)
|
||||
|
||||
|
||||
def test_provider_config_all_invalid_falls_through(monkeypatch):
|
||||
original = cfg.cfg.get("providers")
|
||||
monkeypatch.setitem(
|
||||
cfg.cfg,
|
||||
"providers",
|
||||
{"wandb": {"reasoning_efforts": ["bogus", "typo"]}},
|
||||
)
|
||||
try:
|
||||
result = cfg.resolve_model_reasoning_efforts(
|
||||
"zai-org/GLM-5.2",
|
||||
provider_id="wandb",
|
||||
)
|
||||
assert result != []
|
||||
assert "bogus" not in result
|
||||
assert "typo" not in result
|
||||
finally:
|
||||
if original is None:
|
||||
cfg.cfg.pop("providers", None)
|
||||
else:
|
||||
monkeypatch.setitem(cfg.cfg, "providers", original)
|
||||
|
||||
|
||||
def test_named_custom_provider_config_reasoning_efforts(monkeypatch):
|
||||
original = cfg.cfg.get("custom_providers")
|
||||
monkeypatch.setitem(
|
||||
cfg.cfg,
|
||||
"custom_providers",
|
||||
[{"name": "llm-proxy", "reasoning_efforts": ["none", "high", "xhigh"]}],
|
||||
)
|
||||
try:
|
||||
assert cfg.resolve_model_reasoning_efforts(
|
||||
"some-model",
|
||||
provider_id="custom:llm-proxy",
|
||||
) == ["none", "high", "xhigh"]
|
||||
finally:
|
||||
if original is None:
|
||||
cfg.cfg.pop("custom_providers", None)
|
||||
else:
|
||||
monkeypatch.setitem(cfg.cfg, "custom_providers", original)
|
||||
|
||||
|
||||
def test_acp_guards_win_over_configured_reasoning_efforts(monkeypatch):
|
||||
original = cfg.cfg.get("providers")
|
||||
monkeypatch.setitem(
|
||||
cfg.cfg,
|
||||
"providers",
|
||||
{"copilot-acp": {"reasoning_efforts": ["high"]}},
|
||||
)
|
||||
try:
|
||||
assert cfg.resolve_model_reasoning_efforts(
|
||||
"some-model",
|
||||
provider_id="copilot-acp",
|
||||
) == []
|
||||
finally:
|
||||
if original is None:
|
||||
cfg.cfg.pop("providers", None)
|
||||
else:
|
||||
monkeypatch.setitem(cfg.cfg, "providers", original)
|
||||
|
||||
|
||||
def test_nested_route_deny_wins_over_configured_reasoning_efforts(monkeypatch):
|
||||
original = cfg.cfg.get("custom_providers")
|
||||
monkeypatch.setitem(
|
||||
cfg.cfg,
|
||||
"custom_providers",
|
||||
[{"name": "agg", "reasoning_efforts": ["low", "high"]}],
|
||||
)
|
||||
try:
|
||||
for model in ("vertex/gemini-image-1.0", "vertex/gemini-embedding-001"):
|
||||
assert cfg.resolve_model_reasoning_efforts(
|
||||
model,
|
||||
provider_id="custom:agg",
|
||||
) == []
|
||||
finally:
|
||||
if original is None:
|
||||
cfg.cfg.pop("custom_providers", None)
|
||||
else:
|
||||
monkeypatch.setitem(cfg.cfg, "custom_providers", original)
|
||||
|
||||
|
||||
def test_get_reasoning_status_includes_supported_efforts(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
cfg,
|
||||
|
|
|
|||
Loading…
Reference in a new issue