fix(gateway): offload /model context-length resolution off the event loop - #74155
fix(gateway): offload /model context-length resolution off the event loop#74155Drexuxux wants to merge 1 commit into
Conversation
…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.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the remaining display-resolution calls. The direct premise is still present on current main at gateway/slash_commands.py:2028 and :2350, and the proposed wrapper follows the established asyncio.to_thread pattern in agent/model_metadata.py:2761-2787.
Problems
- The two handlers still synchronously call
enrich_model_switch_warnings_for_gateway()before the changed display calls (gateway/slash_commands.py:1843,:2148). That helper reachesmerge_preflight_compression_warning()(hermes_cli/context_switch_guard.py:194), which invokes synchronousresolve_display_context_length()at:94. Thus cached, compression-enabled sessions can still execute the probe ladder on the event loop. tests/hermes_cli/test_model_switch_context_offload.py:108usesinspect.getsource()to assert implementation text.AGENTS.md:1382-1435explicitly prohibits source-reading tests.
Suggested changes
- Make the preflight-warning resolver path non-blocking for both gateway callers and cover it with the same regression guarantee.
- Replace the source assertion with a real handler-seam test using a blocking resolver plus a worker-thread or heartbeat assertion.
Automated hermes-sweeper review.
|
|
||
| from gateway import slash_commands | ||
|
|
||
| source = inspect.getsource(slash_commands) |
There was a problem hiding this comment.
inspect.getsource() is a source-shape assertion, which AGENTS.md prohibits. Replace this with a behavioral test that drives the actual handler paths and proves a deliberately blocking resolver runs off the event-loop thread.
…ioral offload tests Follow-ups to the previous commit (#74155 by @Drexuxux): - enrich_model_switch_warnings_for_gateway() -> merge_preflight_compression_warning() still called the sync resolve_display_context_length() provider probe ladder inline in both async /model call sites; dispatch it via asyncio.to_thread. - Replace the inspect.getsource() test (source-reading tests are banned by AGENTS.md) with behavioral tests that drive the real _handle_model_command: assert the resolver runs off the loop thread and that the warning enrichment is dispatched through asyncio.to_thread.
|
Merged via salvage PR #75888 (#75888) — your asyncio.to_thread offload was cherry-picked with authorship preserved, plus coverage of the enrich_model_switch_warnings_for_gateway() path that still hit the sync resolver, and the inspect.getsource() test replaced with behavioral ones (source-reading tests are banned by repo policy). Thanks! |
…ioral offload tests Follow-ups to the previous commit (NousResearch#74155 by @Drexuxux): - enrich_model_switch_warnings_for_gateway() -> merge_preflight_compression_warning() still called the sync resolve_display_context_length() provider probe ladder inline in both async /model call sites; dispatch it via asyncio.to_thread. - Replace the inspect.getsource() test (source-reading tests are banned by AGENTS.md) with behavioral tests that drive the real _handle_model_command: assert the resolver runs off the loop thread and that the warning enrichment is dispatched through asyncio.to_thread.
…ioral offload tests Follow-ups to the previous commit (NousResearch#74155 by @Drexuxux): - enrich_model_switch_warnings_for_gateway() -> merge_preflight_compression_warning() still called the sync resolve_display_context_length() provider probe ladder inline in both async /model call sites; dispatch it via asyncio.to_thread. - Replace the inspect.getsource() test (source-reading tests are banned by AGENTS.md) with behavioral tests that drive the real _handle_model_command: assert the resolver runs off the loop thread and that the warning enrichment is dispatched through asyncio.to_thread.
What
/modelon the gateway freezes the entire event loop while it resolves the context length to display.resolve_display_context_length()runs two blocking chains — the route comparison inshould_clear_context_pin()and the provider probe ladder inget_model_context_length()(blockingrequestscalls 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()andshould_clear_context_pin_async(). The/modelslash-command handlers —_handle_model_command(gateway/slash_commands.py:2072) and_finish_switch(:2395) — call the sync helper bare insideasync def.While one user runs
/model, no messages are processed on any platform.get_model_context_length_async()'s own docstring names the failure mode it exists to prevent: "does not freeze the asyncio event loop and cause Discord heartbeat timeouts."Measured with a 50 ms heartbeat and a single 2 s probe standing in for the ladder:
Both resolve the same value in the same wall-clock time — the work simply stops blocking the loop.
Fix
Add
resolve_display_context_length_async()— a thinasyncio.to_threadwrapper mirroring the two existing*_asynchelpers, sharing all logic with the sync version (no duplication) — andawaitit at both handlers.Tests
tests/hermes_cli/test_model_switch_context_offload.py(4 tests, all fail without the fix):/modelhandlers no longer reach the sync helperExisting suites unaffected:
188 passed, 4 skippedacross themodel_switch/context_display/apply_model_switchselections;283 passedacross the gateway slash-command selection (the 3 failures there are present onmainunchanged, intest_complete_path_at_filter,test_feishuandtest_telegram_slash_confirm).