fix(gateway): offload /model context-length resolution off the event loop
resolve_display_context_length() runs two blocking chains: the route comparison in should_clear_context_pin() and the provider probe ladder in get_model_context_length() (blocking requests calls to Anthropic /v1/models, Copilot, Nous, Codex, GMI, Ollama, models.dev and OpenRouter). The gateway message path already offloads both via get_model_context_length_async() and should_clear_context_pin_async(), but the /model slash-command handlers (_handle_model_command, _finish_switch) called the sync helper directly, freezing the whole event loop for the duration of the probe ladder - no messages processed on any platform, and the Discord heartbeat timeouts that get_model_context_length_async() was introduced to prevent. Add resolve_display_context_length_async(), a thin asyncio.to_thread wrapper mirroring the two existing *_async helpers (no logic duplication), and await it at both handlers.
This commit is contained in:
parent
4aa029bfab
commit
95eae03883
|
|
@ -2011,7 +2011,7 @@ class GatewaySlashCommandsMixin:
|
|||
lines = [t("gateway.model.switched", model=format_model_for_display(result.new_model))]
|
||||
lines.append(t("gateway.model.provider_label", provider=plabel))
|
||||
mi = result.model_info
|
||||
from hermes_cli.model_switch import resolve_display_context_length
|
||||
from hermes_cli.model_switch import resolve_display_context_length_async
|
||||
_sw_config_ctx = None
|
||||
_sw_model_cfg = {}
|
||||
try:
|
||||
|
|
@ -2025,7 +2025,7 @@ class GatewaySlashCommandsMixin:
|
|||
pass
|
||||
if not isinstance(_sw_model_cfg, dict):
|
||||
_sw_model_cfg = {}
|
||||
ctx = resolve_display_context_length(
|
||||
ctx = await resolve_display_context_length_async(
|
||||
result.new_model,
|
||||
result.target_provider,
|
||||
base_url=result.base_url or current_base_url or "",
|
||||
|
|
@ -2333,7 +2333,7 @@ class GatewaySlashCommandsMixin:
|
|||
# Context: always resolve via the provider-aware chain so Codex OAuth,
|
||||
# Copilot, and Nous-enforced caps win over the raw models.dev entry.
|
||||
mi = result.model_info
|
||||
from hermes_cli.model_switch import resolve_display_context_length
|
||||
from hermes_cli.model_switch import resolve_display_context_length_async
|
||||
_sw2_config_ctx = None
|
||||
_sw2_model_cfg = {}
|
||||
try:
|
||||
|
|
@ -2347,7 +2347,7 @@ class GatewaySlashCommandsMixin:
|
|||
pass
|
||||
if not isinstance(_sw2_model_cfg, dict):
|
||||
_sw2_model_cfg = {}
|
||||
ctx = resolve_display_context_length(
|
||||
ctx = await resolve_display_context_length_async(
|
||||
result.new_model,
|
||||
result.target_provider,
|
||||
base_url=result.base_url or current_base_url or "",
|
||||
|
|
|
|||
|
|
@ -1058,6 +1058,48 @@ def resolve_display_context_length(
|
|||
return None
|
||||
|
||||
|
||||
async def resolve_display_context_length_async(
|
||||
model: str,
|
||||
provider: str,
|
||||
base_url: str = "",
|
||||
api_key: str = "",
|
||||
model_info: Optional[ModelInfo] = None,
|
||||
custom_providers: list | None = None,
|
||||
config_context_length: int | None = None,
|
||||
configured_model: str | None = None,
|
||||
configured_provider: str | None = None,
|
||||
configured_base_url: str | None = None,
|
||||
) -> Optional[int]:
|
||||
"""Async variant of :func:`resolve_display_context_length`.
|
||||
|
||||
The sync version runs two blocking chains: the route comparison in
|
||||
``should_clear_context_pin`` and the full provider probe ladder in
|
||||
``get_model_context_length`` (blocking ``requests`` calls to Anthropic
|
||||
``/v1/models``, Copilot, Nous, Codex, GMI, Ollama, models.dev and
|
||||
OpenRouter). Async gateway handlers must not run either on the event
|
||||
loop — see ``agent.model_metadata.get_model_context_length_async`` and
|
||||
``hermes_cli.route_identity.should_clear_context_pin_async``, which
|
||||
offload the same chains for the message path.
|
||||
|
||||
Shares all logic with the sync version — no code duplication.
|
||||
"""
|
||||
import asyncio
|
||||
|
||||
return await asyncio.to_thread(
|
||||
resolve_display_context_length,
|
||||
model,
|
||||
provider,
|
||||
base_url=base_url,
|
||||
api_key=api_key,
|
||||
model_info=model_info,
|
||||
custom_providers=custom_providers,
|
||||
config_context_length=config_context_length,
|
||||
configured_model=configured_model,
|
||||
configured_provider=configured_provider,
|
||||
configured_base_url=configured_base_url,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Configured-provider detection for typed model names
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -0,0 +1,111 @@
|
|||
"""``/model`` context-length resolution must not run on the gateway event loop.
|
||||
|
||||
``resolve_display_context_length`` runs two blocking chains — the route
|
||||
comparison in ``should_clear_context_pin`` and the provider probe ladder in
|
||||
``get_model_context_length`` (blocking ``requests`` calls to Anthropic
|
||||
``/v1/models``, Copilot, Nous, Codex, GMI, Ollama, models.dev and OpenRouter).
|
||||
|
||||
The gateway message path already offloads both (``get_model_context_length_async``,
|
||||
``should_clear_context_pin_async``); the ``/model`` slash-command handlers called
|
||||
the sync helper directly, freezing the loop for every user on every platform for
|
||||
the duration of the probe ladder.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import threading
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
import agent.model_metadata as model_meta_mod
|
||||
from hermes_cli import model_switch
|
||||
|
||||
PROBE_SECONDS = 0.4
|
||||
|
||||
RESOLVE_ARGS = dict(
|
||||
model="claude-opus-4",
|
||||
provider="anthropic",
|
||||
base_url="",
|
||||
api_key="",
|
||||
custom_providers=None,
|
||||
config_context_length=None,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def slow_probe(monkeypatch):
|
||||
"""Stand in for one blocking provider probe inside the resolution chain."""
|
||||
calls = {}
|
||||
|
||||
def _probe(model, **kwargs):
|
||||
calls["thread"] = threading.current_thread()
|
||||
time.sleep(PROBE_SECONDS)
|
||||
return 128000
|
||||
|
||||
monkeypatch.setattr(model_meta_mod, "get_model_context_length", _probe)
|
||||
return calls
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_async_variant_matches_sync(slow_probe):
|
||||
"""The async wrapper resolves the same value as the sync helper."""
|
||||
sync_value = model_switch.resolve_display_context_length(**RESOLVE_ARGS)
|
||||
async_value = await model_switch.resolve_display_context_length_async(
|
||||
**RESOLVE_ARGS
|
||||
)
|
||||
assert async_value == sync_value == 128000
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolution_runs_off_the_event_loop_thread(slow_probe):
|
||||
"""The blocking chain must execute on a worker thread, not the loop thread."""
|
||||
loop_thread = threading.current_thread()
|
||||
await model_switch.resolve_display_context_length_async(**RESOLVE_ARGS)
|
||||
assert slow_probe["thread"] is not loop_thread
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_event_loop_stays_responsive_during_resolution(slow_probe):
|
||||
"""A concurrent heartbeat keeps ticking while the probe ladder runs.
|
||||
|
||||
This is the regression: with the bare sync call the loop stalled for the
|
||||
full probe duration, which is what times out Discord heartbeats and stalls
|
||||
Telegram polling for every other chat.
|
||||
"""
|
||||
lags = []
|
||||
stop = asyncio.Event()
|
||||
|
||||
async def heartbeat():
|
||||
interval = 0.02
|
||||
while not stop.is_set():
|
||||
t0 = time.monotonic()
|
||||
try:
|
||||
await asyncio.wait_for(stop.wait(), timeout=interval)
|
||||
except asyncio.TimeoutError:
|
||||
pass
|
||||
lags.append(time.monotonic() - t0 - interval)
|
||||
|
||||
hb = asyncio.create_task(heartbeat())
|
||||
await asyncio.sleep(0.05) # let the heartbeat settle
|
||||
|
||||
ctx = await model_switch.resolve_display_context_length_async(**RESOLVE_ARGS)
|
||||
|
||||
stop.set()
|
||||
await hb
|
||||
|
||||
assert ctx == 128000
|
||||
# The loop was never blocked for anything close to the probe duration.
|
||||
assert max(lags) < PROBE_SECONDS / 2, f"event loop stalled {max(lags):.3f}s"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_gateway_model_handlers_await_the_async_variant():
|
||||
"""The ``/model`` handlers must not reach the sync helper again."""
|
||||
import inspect
|
||||
|
||||
from gateway import slash_commands
|
||||
|
||||
source = inspect.getsource(slash_commands)
|
||||
assert "resolve_display_context_length_async(" in source
|
||||
# No bare sync call: every occurrence carries the _async suffix.
|
||||
assert "resolve_display_context_length(" not in source
|
||||
Loading…
Reference in New Issue