fix: persist [System:] markers with role=system, demote at API call time - #62598
fix: persist [System:] markers with role=system, demote at API call time#62598yingliang-zhang wants to merge 1 commit into
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Automated review by Hermes Agent: No obvious issues found.
Reviewed by Hermes Agent
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Automated review by Hermes Agent: No obvious issues found.
Reviewed by Hermes Agent
9d2f982 to
b73a714
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for separating persisted transcript roles from the strict-provider wire workaround. The current-main premise is real: tui_gateway/server.py:2433-2460 stores model-switch markers as user, and tui_gateway/server.py:4084 does the same for personality pivots.
Problems
agent/agent_runtime_helpers.py:2492demotes every system message after index 0. This also rewrites supported system-role prefill messages:agent/conversation_loop.py:878-881insertsagent.prefill_messagesafter the primary system prompt and calls this sanitizer at:900;tests/cron/test_scheduler.py:1864supplies a system-role prefill example. The new behavior should be limited to Hermes's internal marker records.
Suggested changes
- Mark internal pivot/nudge messages explicitly and demote only those records at the API boundary.
- Add a regression test proving a non-marker system prefill remains
systemwhile an internal marker is demoted.
Automated hermes-sweeper review.
7464aed to
0b06c50
Compare
e540f13 to
dcf1a01
Compare
dcf1a01 to
6fd61a9
Compare
6fd61a9 to
ab15322
Compare
701674e to
c665567
Compare
Rebuild on the current upstream snapshot: keep upstream's conversation-loop refactors, flush-to-DB batch writes, and personality-marker display_kind tagging, while switching [System:] markers from role=user to role=system via make_internal_system_marker. The pre-call sanitizer demotes tagged markers to role=user for strict-provider compatibility. Includes test updates: _DB stubs gain append_messages_batch, personality-marker assertions expect role=system, and the model-switch marker tests verify the system role + tag.
b232e36 to
447a409
Compare
|
Closing: this branch predates upstream's mid-turn guidelines + display_kind redesign (#78113 / a871948). Against latest main the runner metadata + TUI scaffolding-layer work conflicts architecturally with upstream's rollback/display_kind design, and the abandoned base-insert-in-flight flush direction was validated as unworkable (#80832 replaces it). Not resubmitting in this form. |
Problem
[System:]messages (model-switch markers, personality pivots, verification nudges, continuation prompts) are persisted asrole=userto avoid HTTP 400 on strict OpenAI-compatible providers (#48338). This causes the Desktop to render them as prominent right-aligned user message bubbles, visually pushing assistant responses out of view — users report their "response was washed away by a system message."Root Cause
PR #54210 (fixing #48338) changed model-switch markers from
role=systemtorole=userto avoid strict providers rejecting mid-array system messages. This conflated the API compatibility layer with the persistence/representation layer: the wire-format constraint was pushed into the DB, where the Desktop reads it as a real user message.The same
role=userpattern was then copied to all 7[System:]injection points acrosstui_gateway/server.pyandagent/conversation_loop.py.Fix
Separate the concerns: persist with the correct
role=system, demote torole=useronly at API call time.1. Persistence layer — use
role=system(4 files, 7 injection points)tui_gateway/server.py:~2440user→systemtui_gateway/server.py:~2448user→systemtui_gateway/server.py:~2454user→systemtui_gateway/server.py:~4098user→systemagent/conversation_loop.py:~1942user→systemagent/conversation_loop.py:~5111user→systemagent/conversation_loop.py:~5184user→systemagent/conversation_loop.py:~5242user→system2. API compatibility layer — demote in
sanitize_api_messages(1 file)Added a new step in
sanitize_api_messages(agent/agent_runtime_helpers.py) that demotes anysystemmessage at position > 0 torole=useron the per-call copy. The stored transcript and Desktop rendering keep the correctsystemrole; only the wire payload is adjusted.This is the same chokepoint already used for empty tool_calls cleanup and orphaned tool repair — it runs on every API call and only mutates the per-call copy, never the persisted trajectory.
3. Desktop rendering — zero changes needed
The Desktop already has a
SystemMessagecomponent (apps/desktop/src/components/assistant-ui/thread/system-message.tsx) that rendersrole=systemmessages as small, centered, gray text. ThetoRuntimeMessagefunction (chat-runtime.ts:355) already mapsrole=systemto this component. No frontend changes required.Related
session_metarows — this PR is simpler)role=userworkaround from fix(tui): use role=user for model switch marker to avoid HTTP 400 on strict providers (#48338) #54210 / [Bug]: _append_model_switch_marker injects role:"system" mid-conversation → HTTP 400 on strict providers (vLLM, Qwen) #48338Test
test_tui_gateway_server.py(316),test_agent_guardrails.py(39),test_verification_stop_caching.py(3)role=system