Merge remote-tracking branch 'origin/main' into ethie/pm-clean
This commit is contained in:
@@ -23,7 +23,7 @@ from agent.conversation_compression import (
|
||||
from agent.error_classifier import FailoverReason
|
||||
from agent.message_sanitization import serialized_messages_bytes
|
||||
from agent.model_metadata import (
|
||||
get_context_length_from_provider_error, is_output_cap_error,
|
||||
get_context_length_from_provider_error, is_local_endpoint, is_output_cap_error,
|
||||
parse_available_output_tokens_from_error,
|
||||
)
|
||||
from agent.turn_failure_copy import site_copy, stamp_failure
|
||||
@@ -402,11 +402,14 @@ def _recover_context_length(st: _Recovery, _retry: TurnRetryState, error_msg: st
|
||||
# request — a background review from an earlier session — holds their context. Name that,
|
||||
# keep the turn retryable and transient: no "conversation too long", no gateway auto-reset.
|
||||
# Only when the server quoted NO measurement of its own: "prompt is too long: 233153 tokens
|
||||
# > 200000" is the server's count and beats the local estimate.
|
||||
# > 200000" is the server's count and beats the local estimate. Local endpoints only: a hosted
|
||||
# route has no shared slot, so the same rejection means its real window is smaller than the one
|
||||
# Hermes assumes, and "wait and /retry" would fail identically forever; compress instead.
|
||||
window = agent.context_compressor.context_length
|
||||
request_tokens = st.request_tokens() + max(0, int(getattr(agent, "max_tokens", 0) or 0))
|
||||
if (
|
||||
not re.search(r"\d{4,}", error_msg)
|
||||
is_local_endpoint(agent.base_url)
|
||||
and not re.search(r"\d{4,}", error_msg)
|
||||
and isinstance(window, int) and window > 0
|
||||
and request_tokens < window * _UNEXPLAINED_REJECTION_FRACTION
|
||||
):
|
||||
|
||||
@@ -160,10 +160,13 @@ class GatewayAgentCacheMixin:
|
||||
return
|
||||
override: Dict[str, Any] = {k: persisted.get(k) for k in ("model", "provider", "base_url")}
|
||||
provider = persisted.get("provider")
|
||||
from hermes_cli.runtime_provider import is_foreign_provider_endpoint
|
||||
if is_foreign_provider_endpoint(provider, override.get("base_url")):
|
||||
override["base_url"] = None # left over from a switch that kept the previous provider's URL
|
||||
if provider:
|
||||
# Re-resolve credentials for the persisted provider. On failure (e.g. credentials removed
|
||||
# since the switch) keep the credential-less override — _resolve_session_agent_runtime
|
||||
# falls back to env resolution and layers model/provider.
|
||||
# retries the resolution for that provider on each turn (default route + notice meanwhile).
|
||||
try:
|
||||
runtime = _resolve_runtime_agent_kwargs_for_provider(provider, target_model=persisted.get("model") or None)
|
||||
for k in ("api_key", "api_mode", "credential_pool", "requested_provider", "max_tokens"):
|
||||
|
||||
@@ -207,7 +207,8 @@ class GatewayTurnMixin:
|
||||
skey or "", model, override_model, override_runtime.get("provider"),
|
||||
)
|
||||
return override_model, override_runtime
|
||||
# No api_key on the override: env-based resolution below, override model/provider on top.
|
||||
# No api_key on the override (credentials failed to re-resolve at rehydrate): resolve them
|
||||
# for the override's own provider below, never layer it over the default provider's runtime.
|
||||
logger.debug(
|
||||
"Session model override (no api_key, fallback): session=%s config_model=%s override_model=%s",
|
||||
skey or "", model, override_model,
|
||||
@@ -222,7 +223,19 @@ class GatewayTurnMixin:
|
||||
][:5] or "[]",
|
||||
)
|
||||
|
||||
runtime_kwargs = _resolve_runtime_agent_kwargs()
|
||||
runtime_kwargs, unavailable_override = None, None
|
||||
if override and override.get("provider"):
|
||||
try:
|
||||
runtime_kwargs = _resolve_runtime_agent_kwargs_for_provider(
|
||||
override["provider"], target_model=override.get("model") or None)
|
||||
except Exception as exc:
|
||||
# Layering the override on the default runtime sent its model to the default provider's
|
||||
# endpoint (openai-codex on the Nous URL). Run this turn on the whole default route and say
|
||||
# so; the persisted override is kept, so the next turn retries it.
|
||||
logger.warning("Session /model override provider %s unavailable: %s", override["provider"], exc)
|
||||
unavailable_override, override = override, None
|
||||
if runtime_kwargs is None:
|
||||
runtime_kwargs = _resolve_runtime_agent_kwargs()
|
||||
# Private notice metadata must never reach an ``AIAgent(**runtime_kwargs)`` spread; the turn
|
||||
# runner surfaces it through the agent's one-shot fallback notice (#74349).
|
||||
self._pre_agent_fallback_notice = runtime_kwargs.pop("_fallback_notice", None)
|
||||
@@ -230,6 +243,10 @@ class GatewayTurnMixin:
|
||||
if runtime_model:
|
||||
logger.info("Runtime provider supplied explicit model override: %s -> %s", model, runtime_model)
|
||||
model = runtime_model
|
||||
if unavailable_override and not self._pre_agent_fallback_notice:
|
||||
from hermes_cli.fallback_config import pre_agent_fallback_notice
|
||||
self._pre_agent_fallback_notice = pre_agent_fallback_notice(
|
||||
unavailable_override["provider"], unavailable_override.get("model"), runtime_kwargs.get("provider"), model)
|
||||
|
||||
cfg = getattr(self, "config", None) # getattr: bare object.__new__ test runners
|
||||
if cfg and source is not None:
|
||||
|
||||
@@ -215,6 +215,7 @@ class GatewayModelCommandsMixin:
|
||||
_sess_entry.was_auto_reset = False
|
||||
await _sess_db.update_session_model(
|
||||
_sess_entry.session_id, result.new_model, provider=result.target_provider,
|
||||
base_url=result.base_url, api_mode=result.api_mode,
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.debug("Failed to persist model switch to DB: %s", exc)
|
||||
|
||||
@@ -61,6 +61,10 @@ def stored_session_route(session_meta, *, current_model, current_provider):
|
||||
if stored_model == current_model and not provider_changed:
|
||||
return None
|
||||
api_mode = runtime.get("api_mode") or None
|
||||
from hermes_cli.runtime_provider import is_foreign_provider_endpoint
|
||||
if is_foreign_provider_endpoint(provider, base_url):
|
||||
# The endpoint and its wire belong to the provider this chat left; resolve the stored one's own.
|
||||
base_url = api_mode = None
|
||||
# A row's api_mode/base_url were written for whichever model the session last ran. Providers that
|
||||
# pick the wire per model (OpenCode Zen/Go, Copilot, Nous) re-derive both from the stored model, or a
|
||||
# resumed opencode-go session keeps a MiniMax-era anthropic_messages route for a chat_completions
|
||||
|
||||
@@ -317,6 +317,20 @@ def _config_base_url_for_provider(model_cfg: Dict[str, Any], provider: str) -> s
|
||||
return str(model_cfg.get("base_url") or "").strip().rstrip("/") if _same_registered_provider(provider, configured_provider) else ""
|
||||
|
||||
|
||||
def is_foreign_provider_endpoint(provider: Optional[str], base_url: Optional[str]) -> bool:
|
||||
"""True when ``base_url`` is another built-in provider's canonical endpoint, not ``provider``'s.
|
||||
|
||||
A persisted session route that pairs one provider with another's endpoint is left over from a
|
||||
switch that kept the old URL (openai-codex + the Nous Portal URL sent the Codex slug to the Portal).
|
||||
Only registered providers are judged: a custom or proxy URL is never another provider's canonical one.
|
||||
"""
|
||||
pconfig = PROVIDER_REGISTRY.get(str(provider or "").strip().lower())
|
||||
url = str(base_url or "").strip().rstrip("/")
|
||||
if pconfig is None or not url or url == (pconfig.inference_base_url or "").rstrip("/"):
|
||||
return False
|
||||
return any(url == (other.inference_base_url or "").rstrip("/") for other in PROVIDER_REGISTRY.values())
|
||||
|
||||
|
||||
def _anthropic_base_url_override_ok(base_url: str) -> bool:
|
||||
"""Whether a configured ``model.base_url`` plausibly speaks the Anthropic Messages protocol:
|
||||
official Anthropic/Claude hosts, Azure Foundry, or ``/anthropic`` / Kimi ``/coding`` proxies
|
||||
|
||||
@@ -667,15 +667,20 @@ class SessionSessionsMixin:
|
||||
payload = json.dumps(list(tool_names)) if tool_names is not None else None
|
||||
self._write_sql("UPDATE sessions SET tool_names = ? WHERE id = ?", (payload, session_id))
|
||||
|
||||
def update_session_model(self, session_id: str, model: str, provider: Optional[str] = None) -> None:
|
||||
def update_session_model(
|
||||
self, session_id: str, model: str, provider: Optional[str] = None, *,
|
||||
base_url: Optional[str] = None, api_mode: Optional[str] = None,
|
||||
) -> None:
|
||||
"""Set the model after a mid-session /model switch (unconditionally), null system_prompt so
|
||||
stale Model:/Provider: footers rebuild, and drop any Browser runtime lock (lineage markers
|
||||
survive). *provider* is merged into model_config so resume recombines model and provider.
|
||||
survive).
|
||||
|
||||
When *provider* is given, it is merged into ``model_config`` alongside the model (``$.model`` /
|
||||
``$.provider``) so a later resume recombines the persisted model with the provider that actually
|
||||
serves it instead of the config.yaml primary provider (#79536). Callers without provider knowledge
|
||||
leave any stored provider untouched.
|
||||
When *provider* is given the whole route is written, in both shapes resume reads (top-level
|
||||
keys for the TUI/Desktop, ``gateway_runtime`` for the CLI), so a later resume recombines the
|
||||
model with the provider that serves it (#79536). ``base_url``/``api_mode`` are always
|
||||
replaced then (``None`` deletes): the previous provider's endpoint must not survive a switch,
|
||||
or resume sends the new provider's model to the old host. Callers without provider knowledge
|
||||
leave the stored route untouched.
|
||||
"""
|
||||
# Flush first: a still-queued pre-switch delta applied after this UPDATE would trip the
|
||||
# first_accounted_route overwrite and resurrect the old route.
|
||||
@@ -684,7 +689,8 @@ class SessionSessionsMixin:
|
||||
if model:
|
||||
patch["model"] = model
|
||||
if provider:
|
||||
patch["provider"] = provider
|
||||
route = {"provider": provider, "base_url": base_url or None, "api_mode": api_mode or None}
|
||||
patch.update(route, gateway_runtime=route)
|
||||
self._write_model_config_patch(
|
||||
session_id, patch, "UPDATE sessions SET model = ?, model_config = ?, "
|
||||
"system_prompt = NULL, system_prompt_hash = NULL WHERE id = ?",
|
||||
|
||||
@@ -38,7 +38,8 @@ def test_overflow_exhaustion_is_non_retryable_context_overflow_with_slash_comman
|
||||
assert build_error_surface_from_result(result)["code"] == "context_overflow"
|
||||
|
||||
|
||||
def _context_rejection(request_tokens: int, window: int = 65_536, error="HTTP 500: Context size has been exceeded."):
|
||||
def _context_rejection(request_tokens: int, window: int = 65_536, error="HTTP 500: Context size has been exceeded.",
|
||||
base_url: str = "http://127.0.0.1:1234/v1"):
|
||||
"""Drive ``_recover_context_length`` with a provider "context exceeded" and a request the rough
|
||||
estimator prices at ``request_tokens`` against a ``window``-token model (no output cap)."""
|
||||
from unittest.mock import patch
|
||||
@@ -50,7 +51,7 @@ def _context_rejection(request_tokens: int, window: int = 65_536, error="HTTP 50
|
||||
st.compression_attempts = 0
|
||||
st.agent.max_tokens = None
|
||||
st.agent.context_compressor = SimpleNamespace(context_length=window)
|
||||
st.agent.provider, st.agent.base_url, st.agent.tools = "lmstudio", "http://127.0.0.1:1234/v1", None
|
||||
st.agent.provider, st.agent.base_url, st.agent.tools = "lmstudio", base_url, None
|
||||
st.agent._buffer_vprint = st.agent._buffer_diagnostic_status = lambda *a, **k: None
|
||||
compressed = []
|
||||
st.agent._compress_context = lambda msgs, *a, **k: (compressed.append(1) or [{"role": "user", "content": "x"}], None)
|
||||
@@ -87,6 +88,14 @@ def test_context_rejection_near_the_window_still_compresses():
|
||||
assert compressed and verdict.action == "break"
|
||||
|
||||
|
||||
def test_hosted_context_rejection_far_below_the_known_window_compresses():
|
||||
"""A hosted route has no shared slot to wait out: a small request rejected there means the
|
||||
route's real window is below the one Hermes assumes, so /retry would fail forever. Compress."""
|
||||
verdict, compressed = _context_rejection(
|
||||
47_000, window=1_000_000, base_url="https://api.anthropic.com",
|
||||
error="This model's maximum context length was exceeded. Please reduce the length of the messages.",
|
||||
)
|
||||
assert compressed and verdict.action == "break"
|
||||
|
||||
|
||||
def test_empty_response_exhaustion_has_one_text_everywhere():
|
||||
|
||||
@@ -182,6 +182,39 @@ def test_rehydrate_opencode_override_heals_relay_url_for_rederived_wire(store_fa
|
||||
assert (override["api_mode"], override["base_url"]) == ("chat_completions", "https://opencode.ai/zen/go/v1")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("codex_on_turn", ["recovers", "still_unavailable"])
|
||||
def test_codex_override_never_runs_on_the_default_providers_endpoint(store_factory, codex_on_turn):
|
||||
"""A persisted openai-codex override whose credentials fail to re-resolve used to be layered over the
|
||||
DEFAULT provider's runtime (Nous URL + Nous key + chat_completions). The turn runs on ONE coherent
|
||||
route: the override's own provider when it resolves, else the whole default route with a notice."""
|
||||
store = store_factory()
|
||||
session_key = store.get_or_create_session(_make_source()).session_key
|
||||
store.set_model_override(session_key, {"model": "gpt-6-luna-900k", "provider": "openai-codex",
|
||||
"base_url": "https://inference-api.nousresearch.com/v1"})
|
||||
runner = _make_runner(store_factory())
|
||||
codex = {"provider": "openai-codex", "api_key": "codex-tok", "api_mode": "codex_responses",
|
||||
"base_url": "https://chatgpt.com/backend-api/codex"}
|
||||
nous = {"provider": "nous", "api_key": "nous-key", "api_mode": "chat_completions",
|
||||
"base_url": "https://inference-api.nousresearch.com/v1"}
|
||||
calls = iter([RuntimeError("refresh blip"), codex if codex_on_turn == "recovers" else RuntimeError("gone")])
|
||||
|
||||
def _for_provider(provider, target_model=None):
|
||||
assert provider == "openai-codex"
|
||||
nxt = next(calls)
|
||||
if isinstance(nxt, Exception):
|
||||
raise nxt
|
||||
return dict(nxt)
|
||||
|
||||
with patch("gateway.run._resolve_runtime_agent_kwargs_for_provider", side_effect=_for_provider), \
|
||||
patch("gateway.run._resolve_runtime_agent_kwargs", return_value=dict(nous)):
|
||||
model, runtime = runner._resolve_session_agent_runtime(
|
||||
session_key=session_key, user_config={"model": {"default": "openai/gpt-6-luna", "provider": "nous"}})
|
||||
|
||||
expected_model, expected = ("gpt-6-luna-900k", codex) if codex_on_turn == "recovers" else ("openai/gpt-6-luna", nous)
|
||||
assert (model, {k: runtime[k] for k in expected}) == (expected_model, expected)
|
||||
assert bool(runner._pre_agent_fallback_notice) is (codex_on_turn == "still_unavailable")
|
||||
|
||||
|
||||
def test_sanitize_model_override():
|
||||
assert sanitize_model_override(None) is None
|
||||
assert sanitize_model_override({}) is None
|
||||
|
||||
35
tests/tui_gateway/test_resume_switched_provider_endpoint.py
Normal file
35
tests/tui_gateway/test_resume_switched_provider_endpoint.py
Normal file
@@ -0,0 +1,35 @@
|
||||
"""A chat switched to another provider must resume on THAT provider's endpoint.
|
||||
|
||||
The messaging gateway's /model wrote only ``model``/``provider`` over a row the Desktop had persisted
|
||||
with the Nous Portal route, so resuming a chat switched to openai-codex sent the Codex slug to the
|
||||
Portal's chat/completions ("Model 'gpt-6-luna-900k' isn't available on ChatGPT or Codex").
|
||||
"""
|
||||
|
||||
import json
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_cli.cli_model_switch_mixin import stored_session_route
|
||||
from hermes_state import SessionDB
|
||||
from tui_gateway.server import _stored_session_runtime_overrides
|
||||
|
||||
NOUS_ROUTE = {"base_url": "https://inference-api.nousresearch.com/v1", "api_mode": "chat_completions"}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("row_origin", ["gateway_model_switch", "row_written_by_older_build"])
|
||||
def test_resume_drops_previous_providers_endpoint(tmp_path, row_origin):
|
||||
db = SessionDB(db_path=tmp_path / "state.db")
|
||||
db.create_session("s1", source="telegram", model="openai/gpt-6-luna")
|
||||
if row_origin == "gateway_model_switch":
|
||||
db.update_session_meta("s1", json.dumps({"model": "openai/gpt-6-luna", "provider": "nous", **NOUS_ROUTE}),
|
||||
"openai/gpt-6-luna")
|
||||
db.update_session_model("s1", "gpt-6-luna-900k", provider="openai-codex")
|
||||
else:
|
||||
db.update_session_meta("s1", json.dumps({"model": "gpt-6-luna-900k", "provider": "openai-codex", **NOUS_ROUTE}),
|
||||
"gpt-6-luna-900k")
|
||||
row = db.get_session("s1")
|
||||
|
||||
desktop = _stored_session_runtime_overrides(row)["model_override"]
|
||||
assert (desktop["provider"], desktop["base_url"], desktop["api_mode"]) == ("openai-codex", None, None)
|
||||
assert stored_session_route(row, current_model="openai/gpt-6-luna", current_provider="nous") == (
|
||||
"gpt-6-luna-900k", "openai-codex", None, None, True)
|
||||
@@ -1551,6 +1551,10 @@ def _stored_session_runtime_overrides(row: dict | None) -> dict:
|
||||
provider = billing_provider
|
||||
base_url, api_mode, service_tier = field("base_url"), field("api_mode"), field("service_tier")
|
||||
reasoning_config = model_config.get("reasoning_config")
|
||||
from hermes_cli.runtime_provider import is_foreign_provider_endpoint
|
||||
if is_foreign_provider_endpoint(provider, base_url):
|
||||
# The endpoint and its wire belong to the provider this chat left; resolve the stored one's own.
|
||||
base_url = api_mode = ""
|
||||
# Heal a stale provider persisted by an older build (renamed/removed custom provider → "Unknown provider"):
|
||||
# recover ``custom:<name>`` from the stored base_url, then from the entry serving the model; else drop it.
|
||||
if provider and not _is_routable_provider(provider):
|
||||
|
||||
@@ -339,7 +339,7 @@ Look at the CLI startup line — it shows the detected context length (e.g., `
|
||||
|
||||
**Local servers (llama.cpp, Ollama) that go silent instead of erroring:** when a provider rejects a request as too large, Hermes compacts the conversation and rebuilds the request. Hermes re-measures the *complete* rebuilt request (system prompt + tool schemas + messages) before retrying, and runs further bounded compaction passes if it is still over the threshold. If the request still cannot fit, the turn ends with `Context length exceeded: compression could not reduce the rebuilt request below the safe threshold` rather than sending an oversized request that llama.cpp would silently truncate (`stop processing: n_tokens = 65535, truncated = 1` in the server log). If you hit that message, the fix is almost always the configured `context_length` above: make it match the server's actual `-c` / `--ctx-size`.
|
||||
|
||||
**"The model server rejected this request as too large, but this conversation is only about N tokens…":** the server said "context exceeded" without quoting any measurement, while Hermes's own estimate of the request is far below the window it knows for the model — so it does **not** compress or blame the conversation, and the turn stays retryable. On single-slot local servers (LM Studio, Ollama) this is almost always another request holding the server's context at that moment — typically a background memory review from an earlier session (`thread=bg-review` in `logs/agent.log`). Wait a moment and `/retry`. If it recurs with no other Hermes process running, the server is loading the model with a smaller window than Hermes assumes: raise the server's context setting or lower `model.context_length` to match it.
|
||||
**"The model server rejected this request as too large, but this conversation is only about N tokens…":** a local server (localhost, LAN, Tailscale) said "context exceeded" without quoting any measurement, while Hermes's own estimate of the request is far below the window it knows for the model — so it does **not** compress or blame the conversation, and the turn stays retryable. On single-slot local servers (LM Studio, Ollama) this is almost always another request holding the server's context at that moment — typically a background memory review from an earlier session (`thread=bg-review` in `logs/agent.log`). Wait a moment and `/retry`. If it recurs with no other Hermes process running, the server is loading the model with a smaller window than Hermes assumes: raise the server's context setting or lower `model.context_length` to match it. Hosted providers never get this message: they have no shared slot to wait out, so the same rejection there means the route's real window is smaller than Hermes assumes, and Hermes compresses and retries instead.
|
||||
|
||||
To fix context detection, set it explicitly:
|
||||
|
||||
|
||||
Reference in New Issue
Block a user