diff --git a/tests/tui_gateway/test_model_switch_marker_role.py b/tests/tui_gateway/test_model_switch_marker_role.py index c854b5e1ce..9513efc20f 100644 --- a/tests/tui_gateway/test_model_switch_marker_role.py +++ b/tests/tui_gateway/test_model_switch_marker_role.py @@ -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) diff --git a/tui_gateway/server.py b/tui_gateway/server.py index e83277fa19..d049c7556d 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -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")