fix(nous): stop dropping "thinking off" on Portal models that can honor it
reasoning: {enabled: false} is the only shape the Portal honors, and the
profile refused to send it for every model. Sending nothing means the
upstream default instead, which on a thinking-first route like
deepseek/deepseek-v4-pro (catalog: default_effort high) is thinking ON — so
turning thinking off kept billing reasoning tokens on every turn.
The blanket omission was over-broad. The Portal only rejects a disable on
reasoning-mandatory routes ("Reasoning is mandatory for this model"), which
its catalog flags per model, so that flag now gates the omission. Models the
catalog can't speak to keep the old behavior rather than risk the 400.
extra_body.thinking, DeepSeek's own disable shape, is not forwarded upstream
by the Portal and is not an option here.
This commit is contained in:
committed by
brooklyn!
parent
608fa9c7af
commit
d39a031329
@@ -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:
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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}
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user