fix(gateway): dedup model-switch markers so they don't accumulate in history
Each mid-session `/model` switch appended a `[System: The active model for this chat has changed to …]` user-role marker to the live conversation history and never removed the prior one. N switches left N stale markers, all re-sent to the provider on every subsequent turn — the issue measured ~80-120 tokens each (Chinese+English), so 5 MoA-preset switches burned ~400-600 context tokens per API call, permanently. Only the newest marker is meaningful (it names the currently-active model); the rest are pure waste. Fixes #65891. `_append_model_switch_marker` now strips any earlier markers from `session["history"]` (in place, under the existing history_lock) before appending the new one, so the live payload carries exactly one marker. This is the payload re-sent each turn, and it's self-healing across resumes: whatever markers a history reload brings back, the next switch collapses to one. A stable `_MODEL_SWITCH_MARKER_PREFIX` constant is shared by the builder and the `_is_model_switch_marker` predicate so the two can't drift; the marker string itself is byte-for-byte unchanged. Existing behavior (role='user' per the single db.append_message persistence) is preserved. Scope note: this dedups the in-memory history (the per-turn cost the issue quantifies). The DB still persists one row per switch for audit/resume; I did NOT soft-archive prior marker rows because `active=0, compacted=0` is the repo's rewind/undo sentinel (hermes_state.py get_messages docs), so deactivating a marker naively would make it look rewound — a follow-up that needs its own archive semantics. Tests (tests/tui_gateway/test_model_switch_marker_role.py, +5): second switch replaces (not stacks) the marker; the issue's exact 5-MoA-switch sequence leaves one marker naming the last model; real conversation turns are preserved in order; a stale marker sitting between turns is stripped; history_version increments once per switch. `pytest test_model_switch_marker_role.py` → 13 passed (8 existing unchanged). Authored on Windows. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013b1XyXitAxV7phGmKWigJX
This commit is contained in:
@@ -36,3 +36,84 @@ class TestAppendModelSwitchMarkerRole:
|
||||
_append_model_switch_marker(None, model="gpt-4o", provider="openai")
|
||||
|
||||
|
||||
class TestModelSwitchMarkerDedup:
|
||||
"""#65891: only the newest marker is meaningful; older ones must not
|
||||
accumulate in the live history and burn context tokens every turn."""
|
||||
|
||||
@staticmethod
|
||||
def _markers(session: dict) -> list:
|
||||
from tui_gateway.server import _is_model_switch_marker
|
||||
|
||||
return [h for h in session["history"] if _is_model_switch_marker(h)]
|
||||
|
||||
def test_second_switch_replaces_first_marker(self) -> None:
|
||||
session: dict = {"session_key": "s", "history": []}
|
||||
_append_model_switch_marker(session, model="model-a", provider="p")
|
||||
_append_model_switch_marker(session, model="model-b", provider="p")
|
||||
markers = self._markers(session)
|
||||
assert len(markers) == 1, "a second switch must replace, not stack, the marker"
|
||||
assert "model-b" in markers[0]["content"]
|
||||
assert "model-a" not in markers[0]["content"]
|
||||
# The surviving marker is the last history entry.
|
||||
assert session["history"][-1] is markers[0]
|
||||
|
||||
def test_five_switches_leave_one_marker(self) -> None:
|
||||
# Mirrors the issue's screenshot: 5 consecutive MoA preset switches.
|
||||
session: dict = {"session_key": "s", "history": []}
|
||||
for name in ("质量-非高峰", "省钱-非高峰", "代码编程-非高峰", "日常对话-非高峰", "智能-高峰"):
|
||||
_append_model_switch_marker(session, model=name, provider="moa")
|
||||
markers = self._markers(session)
|
||||
assert len(markers) == 1
|
||||
assert "智能-高峰" in markers[0]["content"]
|
||||
|
||||
def test_dedup_preserves_real_conversation_turns(self) -> None:
|
||||
session: dict = {
|
||||
"session_key": "s",
|
||||
"history": [
|
||||
{"role": "user", "content": "hello"},
|
||||
{"role": "assistant", "content": "hi"},
|
||||
],
|
||||
}
|
||||
_append_model_switch_marker(session, model="model-a", provider="p")
|
||||
_append_model_switch_marker(session, model="model-b", provider="p")
|
||||
# Real turns untouched; exactly one marker, appended at the end.
|
||||
assert session["history"][0] == {"role": "user", "content": "hello"}
|
||||
assert session["history"][1] == {"role": "assistant", "content": "hi"}
|
||||
assert len(self._markers(session)) == 1
|
||||
assert len(session["history"]) == 3
|
||||
|
||||
def test_prior_marker_between_turns_is_removed(self) -> None:
|
||||
# A stale marker not at the tail (a later turn followed it) is still
|
||||
# stripped on the next switch.
|
||||
session: dict = {
|
||||
"session_key": "s",
|
||||
"history": [
|
||||
{"role": "user", "content": "q1"},
|
||||
_make_marker_entry("model-a"),
|
||||
{"role": "assistant", "content": "a1"},
|
||||
],
|
||||
}
|
||||
_append_model_switch_marker(session, model="model-b", provider="p")
|
||||
markers = self._markers(session)
|
||||
assert len(markers) == 1
|
||||
assert "model-b" in markers[0]["content"]
|
||||
# The real turns are preserved in order.
|
||||
assert [h["content"] for h in session["history"] if not _is_marker(h)] == ["q1", "a1"]
|
||||
|
||||
def test_history_version_increments_once_on_replace(self) -> None:
|
||||
session: dict = {"session_key": "s", "history": [], "history_version": 0}
|
||||
_append_model_switch_marker(session, model="model-a", provider="p")
|
||||
_append_model_switch_marker(session, model="model-b", provider="p")
|
||||
assert session["history_version"] == 2 # one increment per switch
|
||||
|
||||
|
||||
def _make_marker_entry(model: str) -> dict:
|
||||
from tui_gateway.server import _MODEL_SWITCH_MARKER_PREFIX
|
||||
|
||||
return {"role": "user", "content": f"{_MODEL_SWITCH_MARKER_PREFIX}{model}.]"}
|
||||
|
||||
|
||||
def _is_marker(entry: dict) -> bool:
|
||||
from tui_gateway.server import _is_model_switch_marker
|
||||
|
||||
return _is_model_switch_marker(entry)
|
||||
|
||||
@@ -3595,8 +3595,31 @@ def _persist_live_session_system_prompt(session: dict | None) -> None:
|
||||
logger.debug("failed to persist live session system prompt", exc_info=True)
|
||||
|
||||
|
||||
# Stable leading text of the model-switch marker, shared by the builder and the
|
||||
# dedup below. Only the newest marker is meaningful (it names the *currently*
|
||||
# active model); older ones are stale and would otherwise be re-sent to the
|
||||
# provider on every turn (#65891).
|
||||
_MODEL_SWITCH_MARKER_PREFIX = "[System: The active model for this chat has changed to "
|
||||
|
||||
|
||||
def _is_model_switch_marker(entry: Any) -> bool:
|
||||
"""Whether a history entry is a (self-replacing) model-switch marker."""
|
||||
if not isinstance(entry, dict):
|
||||
return False
|
||||
content = entry.get("content")
|
||||
return isinstance(content, str) and content.startswith(_MODEL_SWITCH_MARKER_PREFIX)
|
||||
|
||||
|
||||
def _append_model_switch_marker(session: dict | None, *, model: str, provider: str) -> None:
|
||||
"""Record a real system-history pivot after a live model switch."""
|
||||
"""Record a real system-history pivot after a live model switch.
|
||||
|
||||
Only the most recent marker is kept: each new switch first strips any
|
||||
prior model-switch markers from the live history, so N switches leave one
|
||||
marker (naming the active model), not N stale ones accumulating tokens on
|
||||
every subsequent API call (#65891). The in-memory history is the payload
|
||||
re-sent each turn; the dedup is self-healing across resumes because the
|
||||
next switch collapses whatever markers a reload brought back.
|
||||
"""
|
||||
if not session:
|
||||
return
|
||||
session_key = str(session.get("session_key") or "").strip()
|
||||
@@ -3605,7 +3628,7 @@ def _append_model_switch_marker(session: dict | None, *, model: str, provider: s
|
||||
|
||||
provider_part = f" via provider {provider}" if provider else ""
|
||||
marker = (
|
||||
"[System: The active model for this chat has changed to "
|
||||
f"{_MODEL_SWITCH_MARKER_PREFIX}"
|
||||
f"{model}{provider_part}. From this point forward, use this runtime "
|
||||
"metadata when answering questions about what model/provider is active.]"
|
||||
)
|
||||
@@ -3615,14 +3638,19 @@ def _append_model_switch_marker(session: dict | None, *, model: str, provider: s
|
||||
# beginning of the API message list (#48338).
|
||||
entry = {"role": "user", "content": marker, "display_kind": "model_switch"}
|
||||
|
||||
def _replace_markers() -> None:
|
||||
history = session.setdefault("history", [])
|
||||
# Drop any earlier markers in place before appending the new one.
|
||||
history[:] = [h for h in history if not _is_model_switch_marker(h)]
|
||||
history.append(entry)
|
||||
session["history_version"] = int(session.get("history_version", 0)) + 1
|
||||
|
||||
lock = session.get("history_lock")
|
||||
if lock is not None:
|
||||
with lock:
|
||||
session.setdefault("history", []).append(entry)
|
||||
session["history_version"] = int(session.get("history_version", 0)) + 1
|
||||
_replace_markers()
|
||||
else:
|
||||
session.setdefault("history", []).append(entry)
|
||||
session["history_version"] = int(session.get("history_version", 0)) + 1
|
||||
_replace_markers()
|
||||
|
||||
try:
|
||||
agent = session.get("agent")
|
||||
|
||||
Reference in New Issue
Block a user