fix(tui): use role=user for model switch marker to avoid HTTP 400 on strict providers - #48361
Closed
Elshayib wants to merge 2 commits into
Closed
fix(tui): use role=user for model switch marker to avoid HTTP 400 on strict providers#48361Elshayib wants to merge 2 commits into
Elshayib wants to merge 2 commits into
Conversation
…l picker When the current provider is a custom endpoint (custom or custom:*), the model switch pipeline must NOT auto-switch to a native provider or OpenRouter based on static catalog matches. The user explicitly configured their own endpoint — the same model name may be served there, and silently rewriting model.provider destroys their config. Changes: - detect_static_provider_for_model(): skip static catalog scan when current provider is custom/custom:* - switch_model() step e guard: extend is_custom to cover custom:* providers, preventing detect_provider_for_model() last-resort OpenRouter fallback from firing - _persist_model_switch(): use load_config() instead of _load_cfg() for a fully merged config that preserves all sections on save Fixes #48305 Co-authored-by: Islam Elshayib <islam.elshayib@gmail.com>
…strict providers _append_model_switch_marker injected role:"system" mid-conversation, which violates strict provider requirements (vLLM, Qwen) that only allow system as the first message. The marker is an in-conversation annotation, not a true system prompt, so role:"user" is correct. Changes: - tui_gateway/server.py: entry role changed from "system" to "user" in _append_model_switch_marker (3 sites: in-memory history + 2 DB append_message calls) - tests/test_tui_gateway_server.py: updated existing test assertions from role=system to role=user; added TestAppendModelSwitchMarker regression class (4 tests) Fixes #48338
alt-glitch
added
type/bug
Contributor
|
Merged the model-switch marker fix via PR #54210 (commit 61622bb), using the earliest duplicate (#48346) as the base. Closing this one: the marker fix is now on main, and the bundled custom-provider catalog changes in models.py / model_switch.py are unrelated to #48338 — happy to review those as a separate focused PR if you'd like to reopen just that part. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
_append_model_switch_markerintui_gateway/server.pyinjects a{"role": "system", ...}entry into the conversation history mid-session after a live model switch. Strict providers (vLLM, Qwen) reject anysystemrole message that isn't the first message in the conversation, returning HTTP 400.Root Cause
The marker is an in-conversation annotation informing the model about a runtime switch -- it is NOT a true system prompt. Using
role: "system"violates the strict message ordering requirements of these providers.Fix
Changed
role: "system"torole: "user"at all 3 sites in_append_model_switch_marker:db.append_message()via agent's session DBscoped_db.append_message()via fallback session DB pathTests
test_config_set_model_switches_agent_without_touching_envto expectrole: "user"TestAppendModelSwitchMarkerregression class (4 tests):test_marker_uses_user_role_not_system- verifies role and contenttest_marker_without_provider- verifies no "via provider" when provider is emptytest_marker_no_session- no-op when session is Nonetest_marker_empty_session_key- no-op when session_key is emptyAll 282 tests in
tests/test_tui_gateway_server.pypass.Fixes #48338