Skip to content

fix(tui): persist model switch marker as user message - #48355

Closed
lucas-nicolas wants to merge 1 commit into
NousResearch:mainfrom
lucas-nicolas:fix/model-switch-marker-role
Closed

fix(tui): persist model switch marker as user message#48355
lucas-nicolas wants to merge 1 commit into
NousResearch:mainfrom
lucas-nicolas:fix/model-switch-marker-role

Conversation

@lucas-nicolas

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a gateway/TUI model-switch bug where _append_model_switch_marker() persisted an informational model-switch marker as role: "system" after prior conversation turns.

Strict OpenAI-compatible providers such as vLLM reject message arrays with system messages outside the beginning of the conversation, returning:

HTTP 400: System message must be at the beginning.

The marker is runtime metadata for the next model, not a privileged instruction. This PR persists it as a user message instead, so it can safely appear mid-history and later be merged with adjacent user messages by the existing sanitizer.

Related Issue

Fixes #48338

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • tui_gateway/server.py
    • Persist model-switch markers as role: "user" instead of role: "system" in both in-memory history and SQLite session persistence.
    • Added a comment explaining why strict OpenAI-compatible providers require this.
  • tests/tui_gateway/test_model_switch_marker.py
    • Added a regression test that verifies model-switch markers are appended and persisted as user messages, not mid-history system messages.

How to Test

  1. Run the focused regression:
    scripts/run_tests.sh tests/tui_gateway/test_model_switch_marker.py tests/gateway/test_model_switch_persistence.py
  2. Run the relevant gateway/TUI slice:
    scripts/run_tests.sh tests/tui_gateway/ tests/gateway/test_model_switch_persistence.py
  3. Reproduce against a strict provider by sending messages shaped like:
    [
        {"role": "system", "content": "..."},
        {"role": "user", "content": "hello"},
        {"role": "assistant", "content": "hi"},
        {"role": "system", "content": "[System: The active model ...]"},
        {"role": "user", "content": "say hi again"},
    ]
    Before this fix, vLLM returns HTTP 400: System message must be at the beginning. With this change, the gateway no longer creates that mid-history system role.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Linux

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Test Results

Passed:

scripts/run_tests.sh tests/tui_gateway/test_model_switch_marker.py tests/gateway/test_model_switch_persistence.py
# 2 files, 10 tests passed, 0 failed

scripts/run_tests.sh tests/tui_gateway/ tests/gateway/test_model_switch_persistence.py
# 13 files, 143 tests passed, 0 failed

git diff --check
# passed

python3 scripts/check-windows-footguns.py --diff origin/main
# passed; no Windows footguns found in changed diff

Full-suite note:

I attempted a full scripts/run_tests.sh run after installing [all,dev] per CONTRIBUTING. The full suite is very large; I stopped it after the relevant suites had already passed and the run had progressed past 22k passing tests with several failures in unrelated test files. I am not claiming full-suite green for this PR.

Screenshots / Logs

Relevant failure reproduced before the fix:

HTTP 400: {"error":{"message":"System message must be at the beginning.","type":"BadRequestError","param":null,"code":400}}

@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/gateway Gateway runner, session dispatch, delivery provider/qwen Qwen / Alibaba Cloud (OAuth) P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jun 18, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #48346 — both fix _append_model_switch_marker() in tui_gateway/server.py by persisting the model-switch marker as role: user instead of role: system, so it no longer lands mid-history and triggers HTTP 400: System message must be at the beginning on strict providers (vLLM/Qwen). #48346 is the earlier open PR for the same fix. Both address #48338; related to the broader symptom anchor #20866.

@lucas-nicolas
lucas-nicolas force-pushed the fix/model-switch-marker-role branch from c1232c2 to af89638 Compare June 18, 2026 10:00
@lucas-nicolas

Copy link
Copy Markdown
Contributor Author

Duplicate of #48346 — both fix _append_model_switch_marker() in tui_gateway/server.py by persisting the model-switch marker as role: user instead of role: system, so it no longer lands mid-history and triggers HTTP 400: System message must be at the beginning on strict providers (vLLM/Qwen). #48346 is the earlier open PR for the same fix. Both address #48338; related to the broader symptom anchor #20866.

Sorry did not see the PR at the time I started trying to fix this issue. Was hoping to make my first contribution with this PR :). There is an assertion failing on the previously opened PR that needs to be changed. changed it here and left a comment on the other PR.

Have a nice day and thank you for working on Hermes Agent

@lucas-nicolas lucas-nicolas changed the title fix(gateway): persist model switch marker as user message fix(tui): persist model switch marker as user message Jun 18, 2026
@alaamohanad169-ship-it

Copy link
Copy Markdown
Contributor

Note: This is NOT a duplicate of the prior closed PRs (#48346, #48346, #48338, #20866). The earlier PRs were closed because they were stale (far behind upstream main). This version has been cherry-picked onto the latest origin/main with a clean diff containing only the fix changes. No conflicts. CI green. Ready for review.

@lucas-nicolas
lucas-nicolas force-pushed the fix/model-switch-marker-role branch from af89638 to 8e5b1cc Compare June 18, 2026 15:12
@lucas-nicolas

Copy link
Copy Markdown
Contributor Author

Rebased this branch onto current upstream/main and re-ran the focused checks:

env -u VIRTUAL_ENV scripts/run_tests.sh tests/tui_gateway/test_model_switch_marker.py tests/test_tui_gateway_server.py
# 2 files, 279 tests passed, 0 failed

git diff --check upstream/main..HEAD
# passed

python3 scripts/check-windows-footguns.py --diff upstream/main
# passed

@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #54210 (commit 61622bb). Thanks for the report (#48338) and the fix — your explanatory comment was folded into the salvaged change and you are credited via Co-authored-by. The cleanest minimal version came from the earliest duplicate (#48346); both of you are on the commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/tui Terminal UI (ui-tui/ + tui_gateway/) duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists provider/qwen Qwen / Alibaba Cloud (OAuth) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: _append_model_switch_marker injects role:"system" mid-conversation → HTTP 400 on strict providers (vLLM, Qwen)

4 participants