fix(copilot): derive runtime api_mode from active target model - #32308
fix(copilot): derive runtime api_mode from active target model#32308dandomin wants to merge 2 commits into
Conversation
When a user switches models mid-session via /model (e.g. config default is
gpt-5.5 but the active model is claude-opus-4.7-1m-internal via Copilot),
_copilot_runtime_api_mode() was deriving api_mode from model_cfg['default']
or a stale persisted model.api_mode rather than the currently active model.
This caused resolver and normalizer to disagree on api_mode every turn,
triggering constant OpenAI client rebuilds ('agent_init' on every turn)
and, for some model/endpoint pairs, outright HTTP failures because Copilot
serves different model families through different API surfaces:
- GPT-5.x non-mini / -codex -> codex_responses
- GPT-5-mini / 4.1 / 4o / Gemini -> chat_completions
- Claude -> chat_completions (or anthropic_messages depending on catalog)
Fix: thread the resolved target_model through _resolve_explicit_runtime,
_resolve_runtime_from_pool_entry, and the legacy creds path into
_copilot_runtime_api_mode, and have that helper prefer copilot_model_api_mode
inference on the active model over a stale config api_mode. The stale
configured api_mode is still honored only when no target_model is supplied
(so non-switching callers keep their existing behavior).
Verified locally: after gateway restart with the patch loaded, a fresh
session shows exactly one agent_init at session startup and zero on
subsequent turns, where pre-patch the same flow logged a fresh
'OpenAI client created (agent_init, ...)' on every conversation turn.
Tests:
- test_copilot_api_mode_recomputes_from_target_model_even_with_matching_config
- test_resolve_runtime_provider_threads_target_model_to_copilot_explicit_path
Related upstream PRs working the same area:
NousResearch#27267 (closest match), NousResearch#17622, NousResearch#24251, NousResearch#9033, NousResearch#6647
…vider
Follow-up to the runtime_provider.py change in this PR.
resolve_runtime_provider now accepts target_model and uses it to derive
Copilot api_mode from the active/switched model. The gateway and CLI
were still calling it without that argument, so:
- gateway/run.py: _resolve_runtime_agent_kwargs now accepts and
forwards target_model. All four call sites (build_agent, runtime
snapshot for /msg context, gateway-model probe, and the model
diagnostic path) pass the gateway's active model.
- cli.py: HermesCLI passes self.model when it resolves runtime
credentials for the interactive session.
Without these, a mid-session /model switch in the gateway/CLI hits the
no-target_model path in _copilot_runtime_api_mode and falls back to
model_cfg['default'], which is exactly the resolver/normalizer
disagreement this PR is trying to eliminate.
|
Pushed a follow-up commit (f45dfe9) that threads the resolved active model into
Without these, mid-session |
|
Related to #17622, #6647, #9033, #27267, #24251 — same architectural pattern (stale api_mode derived from config default instead of active target model) across different entry points. This PR fixes the CLI/runtime path specifically. #6647 covers delegation, #9033 covers cron, #24251 covers subagents, #27267 covers --model CLI override. |
|
Confirming this also breaks the Hit this on The bridge already threads the active model through correctly: Minimal repro on current from hermes_cli.runtime_provider import resolve_runtime_provider
rt = resolve_runtime_provider(requested="copilot", target_model="gpt-5.5")
print(rt["api_mode"]) # -> 'chat_completions' (bug; should be 'codex_responses')With this PR applied it returns So the impact surface is broader than CLI |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the active-model routing issue. The premise still holds on current main: hermes_cli/runtime_provider.py:320-335 derives Copilot mode from persisted config/default, while hermes_cli/model_switch.py:1214-1217 already passes the switched model into the resolver.
Problems
- In the pool path, the PR passes
effective_modelastarget_model(hermes_cli/runtime_provider.py, PR line 361).effective_modelfalls back tomodel_cfg["default"], so ordinary calls without a target model no longer take the PR helper's configured-api_modepreservation branch. Pass the originaltarget_modelinstead; the helper can still usemodel.defaultfor inference. - The live CLI resolver has moved to
hermes_cli/cli_agent_setup_mixin.py:41-45, and current gateway session setup invokes_resolve_runtime_agent_kwargs()without its resolvedmodelatgateway/run.py:3801. The submitted locations therefore do not cover the current CLI/gateway paths.
Suggested changes
- Port the active-model wiring to those current call sites and add a pool-path regression where
target_modelis omitted butmodel.api_modeis set.
Automated hermes-sweeper review.
| api_mode = _copilot_runtime_api_mode( | ||
| model_cfg, | ||
| getattr(entry, "runtime_api_key", ""), | ||
| target_model=effective_model, |
There was a problem hiding this comment.
Please pass the original target_model here, not effective_model. effective_model falls back to model_cfg["default"], so this is non-null even when the caller supplied no target; that bypasses the helper's new configured-api_mode preservation branch and changes normal pool resolution behavior.
|
Closing — this fix landed via PR #60086 (merged, commit 51083d2), which threads target_model through all three Copilot resolver paths with resolver-level tests. Your PR fixed the same bug; the earliest submitter (#27267) and the first complete version (#47090) are credited alongside. Thanks for the contribution! |
Summary
When a user switches models mid-session via
/model(e.g. config default isgpt-5.5but the active model isclaude-opus-4.7-1m-internalvia Copilot),_copilot_runtime_api_mode()was derivingapi_modefrommodel_cfg["default"]or a stale persistedmodel.api_moderather than the currently active model.This caused the resolver and the normalizer to disagree on
api_modeon every turn, which in turn triggered constant OpenAI client rebuilds (a freshagent_initlog line on every conversation turn) and, for some model/endpoint pairs, outright HTTP failures because Copilot serves different model families through different API surfaces:-codex→codex_responseschat_completionschat_completions(oranthropic_messagesdepending on catalog)Fix
Thread the resolved
target_modelthrough_resolve_explicit_runtime,_resolve_runtime_from_pool_entry, and the legacy creds path into_copilot_runtime_api_mode, and have that helper prefercopilot_model_api_mode(active_model)inference over a stale configuredapi_mode. The configuredapi_modeis still honored when notarget_modelis supplied, so non-switching callers keep their existing behavior.Why
The same architectural smell — "runtime routing derives
api_modefrom stale config / default / main model instead of the actual target / active model" — shows up in several open PRs covering different surfaces (CLI switch, gateway switch, delegation, subagents, cron). This PR fixes the CLI/runtime path specifically and is intentionally scoped to one helper + its callers so it can land independently of the broader Copilot routing cleanups.Verification
Locally, after restarting the gateway with this patch loaded:
agent_initat session startup and zero on subsequent turns.OpenAI client created (agent_init, shared=True) provider=copilot ...line on every conversation turn.Tests
Two new unit tests added to
tests/hermes_cli/test_runtime_provider_resolution.py:test_copilot_api_mode_recomputes_from_target_model_even_with_matching_config— proves that even when the configuredapi_modematches the configured provider, an explicittarget_modelof a different family (Claude vs. GPT‑5.x) overrides it.test_resolve_runtime_provider_threads_target_model_to_copilot_explicit_path— proves the non-pool / explicit Copilot resolver path also threadstarget_modelthrough.Both pass:
Related
Open PRs working the same area / same bug class:
Happy to rebase on or close in favor of #27267 if the maintainers prefer that landing path.