From abd85a94bcb10b300f3938889dfd2ae1b0a7b97b Mon Sep 17 00:00:00 2001 From: joaomarcos Date: Sun, 9 Aug 2026 21:01:49 -0300 Subject: [PATCH] fix(gateway): keep the personality pivot out of the truncate ordinal space (#82756) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `truncate_before_user_ordinal` is an index into the list of *real* user turns. The gateway builds that list with `role == "user" and not display_kind`, and `test_prompt_submit_truncate_ordinal_skips_display_kind_rows` already pins why: "Without the filter, a trailing marker shifts the ordinal so the wrong message is targeted for truncation." `_apply_personality_to_session` broke that invariant at the producer. Its pivot marker rides as `role=user` — deliberately, so strict OpenAI-compatible providers accept it mid-conversation (the same reason `_append_model_switch_marker` does) — but unlike the model-switch marker it carried no `display_kind`. The gateway therefore counted it as a real user turn while no client ever renders it as one. After a personality change the two sides address different lists: every later rewind/edit/regenerate resolves one slot too early, and `replace_messages()` hard-DELETEs the extra span. That is the reported signature — an in-range, valid ordinal, `confirm_truncate: true`, and a cut that moved backwards with no user rewind action. Tag the pivot like the model-switch marker, and teach the desktop to project the kind as a timeline row so a persisted marker is never rendered — or counted — as a user turn on the client side either. Both ends must exclude it; excluding it on only one end just inverts the drift. The regression test drives the real injection point rather than a hand-written marker dict. Without the fix it fails with "the pivot shifted the ordinal: the cut landed at 3 instead of 5", losing a turn the user never asked to drop. Co-Authored-By: Claude Opus 5 (1M context) --- apps/desktop/src/lib/chat-messages.test.ts | 18 ++- apps/desktop/src/lib/chat-messages.ts | 7 +- apps/desktop/src/types/hermes.ts | 8 +- .../emails/joaomarcosdias444@gmail.com | 1 + tests/test_tui_gateway_server.py | 113 ++++++++++++++++++ tui_gateway/server.py | 11 +- 6 files changed, 153 insertions(+), 5 deletions(-) create mode 100644 contributors/emails/joaomarcosdias444@gmail.com diff --git a/apps/desktop/src/lib/chat-messages.test.ts b/apps/desktop/src/lib/chat-messages.test.ts index 6c959326e7..95235b4468 100644 --- a/apps/desktop/src/lib/chat-messages.test.ts +++ b/apps/desktop/src/lib/chat-messages.test.ts @@ -324,16 +324,30 @@ describe('toChatMessages', () => { content: '[System note: Your previous turn was interrupted mid-run…]\n\noriginal prompt', display_kind: 'auto_continue', timestamp: 6 + }, + { + role: 'user', + content: "[System: The user has changed the assistant's personality…]", + display_kind: 'personality_switch', + timestamp: 7 } ]) - expect(messages.map(message => message.role)).toEqual(['user', 'assistant', 'system', 'system', 'system']) + expect(messages.map(message => message.role)).toEqual([ + 'user', + 'assistant', + 'system', + 'system', + 'system', + 'system' + ]) expect(messages.map(chatMessageText)).toEqual([ 'real user turn', 'real assistant reply', 'model changed', 'background agent work finished', - 'resumed interrupted turn' + 'resumed interrupted turn', + 'personality changed' ]) }) diff --git a/apps/desktop/src/lib/chat-messages.ts b/apps/desktop/src/lib/chat-messages.ts index 0978683ca3..c55dbf6c3e 100644 --- a/apps/desktop/src/lib/chat-messages.ts +++ b/apps/desktop/src/lib/chat-messages.ts @@ -387,6 +387,10 @@ function timelineDisplayContent(message: SessionMessage, content: string): strin return 'resumed interrupted turn' } + if (message.display_kind === 'personality_switch') { + return 'personality changed' + } + if (message.display_kind === 'async_delegation_complete') { const count = timelineTaskCount(message.display_metadata) @@ -993,7 +997,8 @@ export function toChatMessages(messages: SessionMessage[]): ChatMessage[] { const displayRole = message.display_kind === 'model_switch' || message.display_kind === 'async_delegation_complete' || - message.display_kind === 'auto_continue' + message.display_kind === 'auto_continue' || + message.display_kind === 'personality_switch' ? 'system' : message.role diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index ff45339ee7..39c9faedb5 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -555,7 +555,13 @@ export interface SessionMessage { reasoning?: null | string reasoning_content?: null | string reasoning_details?: unknown - display_kind?: 'async_delegation_complete' | 'auto_continue' | 'hidden' | 'model_switch' | string + display_kind?: + | 'async_delegation_complete' + | 'auto_continue' + | 'hidden' + | 'model_switch' + | 'personality_switch' + | string /** * A backend older than this app can still serve this as unparsed JSON text, * so readers must narrow before indexing into it. diff --git a/contributors/emails/joaomarcosdias444@gmail.com b/contributors/emails/joaomarcosdias444@gmail.com new file mode 100644 index 0000000000..63b9e7b46c --- /dev/null +++ b/contributors/emails/joaomarcosdias444@gmail.com @@ -0,0 +1 @@ +JoaoMarcos44 # fix(gateway): personality pivot must not consume a truncate ordinal slot (#82756) diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 2ca998519f..8bb8188be6 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -16723,3 +16723,116 @@ def test_save_cfg_keeps_unicode_personalities_readable(tmp_path, monkeypatch): assert "(=^･ω･^=)" in text assert "\\u4f60" not in text + +def test_personality_marker_does_not_shift_truncate_ordinal(monkeypatch): + """A personality pivot must not occupy a slot in the ordinal address space. + + ``_apply_personality_to_session`` injects its pivot as ``role=user`` so + strict OpenAI-compatible providers accept it mid-conversation. Untagged, the + ordinal filter (``role == "user" and not display_kind``) counted it as a + real user turn while no client renders it as one, so every rewind issued + after a personality change resolved one turn too early and + ``replace_messages()`` hard-deleted the extra span (#82756, third occurrence + after #70516 / #80763). + + The sibling ``test_prompt_submit_truncate_ordinal_skips_display_kind_rows`` + pins the filter itself; this one pins the producer, which is where the + invariant was actually broken. + """ + + class _Agent: + def run_conversation(self, prompt, conversation_history=None, stream_callback=None, **_kwargs): + return { + "final_response": "reply", + "messages": [ + *(conversation_history or []), + {"role": "user", "content": prompt}, + {"role": "assistant", "content": "reply"}, + ], + } + + class _ImmediateThread: + def __init__(self, target=None, daemon=None): + self._target = target + + def start(self): + self._target() + + class _StubDb: + def __init__(self): + self.replaced = [] + + def replace_messages(self, session_id, messages, active_only=False): + self.replaced.append((session_id, list(messages))) + + session = _session( + agent=_Agent(), + history=[ + {"role": "user", "content": "first"}, + {"role": "assistant", "content": "first reply"}, + ], + ) + server._sessions["personality-ordinal-sid"] = session + stub_db = _StubDb() + + try: + monkeypatch.setattr(server, "_session_info", lambda *a, **k: {}) + monkeypatch.setattr(server, "_emit", lambda *a: None) + + # Real production injection point — not a hand-written marker dict. + server._apply_personality_to_session( + "personality-ordinal-sid", session, "talk like a pirate", "pirate" + ) + + marker = session["history"][-1] + assert marker["role"] == "user", "provider compatibility: pivot rides as a user turn" + + # Two more real turns land after the personality change. + session["history"].extend( + [ + {"role": "user", "content": "second"}, + {"role": "assistant", "content": "second reply"}, + {"role": "user", "content": "third"}, + {"role": "assistant", "content": "third reply"}, + ] + ) + history_before = list(session["history"]) + third_index = history_before.index({"role": "user", "content": "third"}) + + monkeypatch.setattr(server.threading, "Thread", _ImmediateThread) + monkeypatch.setattr(server, "_get_usage", lambda _a: {}) + monkeypatch.setattr(server, "render_message", lambda _t, _c: "") + monkeypatch.setattr(server, "_get_db", lambda: stub_db) + + # The client counts three user bubbles (first=0, second=1, third=2) — + # it never sees the pivot. Rewinding to "third" must cut exactly there. + resp = server.handle_request( + { + "id": "1", + "method": "prompt.submit", + "params": { + "session_id": "personality-ordinal-sid", + "text": "third, reworded", + "truncate_before_user_ordinal": 2, + "confirm_truncate": True, + }, + } + ) + assert resp.get("result"), f"got error: {resp.get('error')}" + + expected = history_before[:third_index] + assert stub_db.replaced == [("session-key", expected)], ( + "the pivot shifted the ordinal: the cut landed at " + f"{len(stub_db.replaced[0][1]) if stub_db.replaced else None} instead of {third_index}" + ) + # The turn before the target must survive — that is the span the three + # reported incidents lost. + assert {"role": "user", "content": "second"} in expected + # And the mechanism that keeps it out of the address space, so a future + # producer cannot regress this by dropping the tag. + assert marker.get("display_kind"), ( + "an untagged role=user pivot silently consumes a truncate ordinal slot" + ) + finally: + server._sessions.pop("personality-ordinal-sid", None) + diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 842e6e0c3f..6bbbb260cb 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -6065,8 +6065,17 @@ def _apply_personality_to_session( "[System: The user has cleared the personality overlay. " "From this point forward, respond in your normal default style.]" ) + # Tagged like the model-switch marker (`_append_model_switch_marker`): + # the marker rides as role=user so strict OpenAI-compatible providers + # accept it mid-conversation, but `display_kind` keeps it out of the + # `truncate_before_user_ordinal` addressing space. Untagged, it counts + # as a real user turn on the gateway side while no client counts it, so + # every later rewind resolves one turn too early and `replace_messages` + # hard-deletes the difference (#82756). with session["history_lock"]: - session["history"].append({"role": "user", "content": marker}) + session["history"].append( + {"role": "user", "content": marker, "display_kind": "personality_switch"} + ) session["history_version"] = int(session.get("history_version", 0)) + 1 info = _session_info(agent) _emit("session.info", sid, info)