diff --git a/plugins/model-providers/nous/__init__.py b/plugins/model-providers/nous/__init__.py index df1c1f4674..e179e5c1b2 100644 --- a/plugins/model-providers/nous/__init__.py +++ b/plugins/model-providers/nous/__init__.py @@ -65,20 +65,53 @@ class NousProfile(ProviderProfile): body["provider"] = provider_preferences return body + @staticmethod + def _cannot_disable_reasoning(model: str | None) -> bool: + """True when a disable can't safely be sent for *model*. + + Reasoning-mandatory routes answer ``reasoning: {enabled: false}`` + with HTTP 400 ("Reasoning is mandatory for this model"), so the + catalog's ``mandatory`` flag decides. Cache-only, and an unknown + model (catalog cold, unlisted, or unreachable) also answers True: + a cold first turn errs toward the old omit-everything behavior + rather than risking a 400. + """ + try: + from hermes_cli.models import ( + nous_model_reasoning_capabilities, + warm_nous_reasoning_caps_async, + ) + + caps = nous_model_reasoning_capabilities(model) + if caps is None: + warm_nous_reasoning_caps_async() + return True + except Exception: + return True + return bool(caps.get("mandatory")) + def build_api_kwargs_extras( self, *, reasoning_config: dict | None = None, supports_reasoning: bool = False, + model: str | None = None, **context, ) -> tuple[dict[str, Any], dict[str, Any]]: - """Nous: passes full reasoning_config, but OMITS when disabled.""" + """Nous: passes the full reasoning_config, disable included. + + The Portal honors ``reasoning: {enabled: false}`` — it is the only + wire shape that does. Sending nothing means the *upstream* default, + which for a thinking-first model like ``deepseek/deepseek-v4-pro`` + (catalog: ``default_effort: high``) is thinking ON, so omitting a + disable silently ignored the user's "thinking off". + """ extra_body = {} if supports_reasoning: if reasoning_config is not None: rc = dict(reasoning_config) - if rc.get("enabled") is False: - pass # Nous omits reasoning when disabled + if rc.get("enabled") is False and self._cannot_disable_reasoning(model): + pass # route rejects a disable — let the model think else: extra_body["reasoning"] = rc else: diff --git a/tests/agent/transports/test_chat_completions.py b/tests/agent/transports/test_chat_completions.py index afa756bf50..059322ff41 100644 --- a/tests/agent/transports/test_chat_completions.py +++ b/tests/agent/transports/test_chat_completions.py @@ -262,7 +262,7 @@ class TestChatCompletionsBuildKwargs: ) assert kw["extra_body"]["reasoning"] == {"enabled": True, "effort": "medium"} - def test_nous_omits_disabled_reasoning(self, transport): + def test_nous_omits_disabled_reasoning_for_unknown_model(self, transport): from providers import get_provider_profile profile = get_provider_profile("nous") msgs = [{"role": "user", "content": "Hi"}] @@ -272,7 +272,10 @@ class TestChatCompletionsBuildKwargs: supports_reasoning=True, reasoning_config={"enabled": False}, ) - # Nous rejects enabled=false; reasoning omitted entirely + # Not a Portal model id, so the catalog can't rule out a + # reasoning-mandatory route (which 400s on a disable) — omit. + # tests/plugins/model_providers/test_nous_profile.py covers the + # catalog-known cases where the disable IS forwarded. assert "reasoning" not in kw.get("extra_body", {}) def test_ollama_num_ctx(self, transport): diff --git a/tests/plugins/model_providers/test_nous_profile.py b/tests/plugins/model_providers/test_nous_profile.py new file mode 100644 index 0000000000..68d738dcc2 --- /dev/null +++ b/tests/plugins/model_providers/test_nous_profile.py @@ -0,0 +1,123 @@ +"""Unit tests for the Nous Portal profile's reasoning wiring. + +The Portal honors ``reasoning: {enabled: false}`` — it is the only wire shape +that does, and ``extra_body.thinking`` is not forwarded upstream. The profile +used to drop a disable for every model, which on a thinking-first route like +``deepseek/deepseek-v4-pro`` (catalog: ``default_effort: high``) meant the +upstream default applied and "thinking off" burned reasoning tokens anyway. + +A disable is still dropped for reasoning-mandatory routes, which answer +``reasoning: {enabled: false}`` with HTTP 400, and for models the catalog +can't speak to — an unknown model errs toward the old behavior rather than +risking a 400 on a cold first turn. + +These tests pin that contract without going live. +""" + +from __future__ import annotations + +import pytest + + +@pytest.fixture +def nous_profile(): + """Resolve the registered Nous profile through the real discovery path.""" + # ``model_tools`` triggers plugin discovery on import, which is what + # registers the Nous profile in the global provider registry. + import model_tools # noqa: F401 + import providers + + profile = providers.get_provider_profile("nous") + assert profile is not None, "nous provider profile must be registered" + return profile + + +@pytest.fixture +def portal_catalog(monkeypatch): + """Prime the Portal reasoning-capability cache with known entries.""" + import hermes_cli.models as models_mod + + monkeypatch.setattr(models_mod, "_nous_reasoning_caps_failed_at", None) + monkeypatch.setattr(models_mod, "_nous_reasoning_caps_cache", { + "deepseek/deepseek-v4-pro": { + "supports_reasoning": True, + "supported_efforts": ["xhigh", "high"], + "mandatory": False, + }, + "arcee-ai/trinity-large-thinking": { + "supports_reasoning": True, + "supported_efforts": None, + "mandatory": True, + }, + }) + + +class TestNousReasoningWireShape: + """``build_api_kwargs_extras`` produces the Portal's wire format.""" + + def test_disable_reaches_optional_reasoning_model(self, nous_profile, portal_catalog): + """The knob the user set is the knob the Portal receives.""" + extra_body, top_level = nous_profile.build_api_kwargs_extras( + reasoning_config={"enabled": False}, + supports_reasoning=True, + model="deepseek/deepseek-v4-pro", + ) + assert extra_body == {"reasoning": {"enabled": False}} + assert top_level == {} + + def test_disable_dropped_for_mandatory_reasoning_model(self, nous_profile, portal_catalog): + """Mandatory routes 400 on a disable — send nothing instead.""" + extra_body, _ = nous_profile.build_api_kwargs_extras( + reasoning_config={"enabled": False}, + supports_reasoning=True, + model="arcee-ai/trinity-large-thinking", + ) + assert "reasoning" not in extra_body + + def test_disable_dropped_for_unknown_model(self, nous_profile, portal_catalog): + """Unlisted / cold catalog → fail safe, never risk the 400.""" + extra_body, _ = nous_profile.build_api_kwargs_extras( + reasoning_config={"enabled": False}, + supports_reasoning=True, + model="private/unlisted-route", + ) + assert "reasoning" not in extra_body + + @pytest.mark.parametrize( + "model", + ["deepseek/deepseek-v4-pro", "arcee-ai/trinity-large-thinking", "private/unlisted-route"], + ) + def test_enabled_config_always_forwarded(self, nous_profile, portal_catalog, model): + """Mandatory-ness only gates the disable; an enable always ships.""" + extra_body, _ = nous_profile.build_api_kwargs_extras( + reasoning_config={"enabled": True, "effort": "high"}, + supports_reasoning=True, + model=model, + ) + assert extra_body["reasoning"] == {"enabled": True, "effort": "high"} + + def test_no_config_defaults_to_medium(self, nous_profile, portal_catalog): + extra_body, _ = nous_profile.build_api_kwargs_extras( + reasoning_config=None, + supports_reasoning=True, + model="deepseek/deepseek-v4-pro", + ) + assert extra_body["reasoning"] == {"enabled": True, "effort": "medium"} + + def test_nothing_emitted_without_reasoning_support(self, nous_profile, portal_catalog): + extra_body, top_level = nous_profile.build_api_kwargs_extras( + reasoning_config={"enabled": False}, + supports_reasoning=False, + model="deepseek/deepseek-v4-pro", + ) + assert extra_body == {} + assert top_level == {} + + def test_caller_config_not_mutated(self, nous_profile, portal_catalog): + cfg = {"enabled": False} + nous_profile.build_api_kwargs_extras( + reasoning_config=cfg, + supports_reasoning=True, + model="deepseek/deepseek-v4-pro", + ) + assert cfg == {"enabled": False} diff --git a/tests/providers/test_transport_parity.py b/tests/providers/test_transport_parity.py index e6befad371..11a2b52d38 100644 --- a/tests/providers/test_transport_parity.py +++ b/tests/providers/test_transport_parity.py @@ -105,7 +105,7 @@ class TestOpenRouterParity: class TestNousParity: - """Nous: product tags, reasoning, omit when disabled.""" + """Nous: product tags, reasoning passthrough (disable included).""" def test_tags(self, transport): from agent.portal_tags import nous_portal_tags