fix(gateway): keep the personality pivot out of the truncate ordinal space (#82756)
`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) <noreply@anthropic.com>
This commit is contained in:
@@ -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'
|
||||
])
|
||||
})
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
JoaoMarcos44 # fix(gateway): personality pivot must not consume a truncate ordinal slot (#82756)
|
||||
@@ -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)
|
||||
|
||||
|
||||
+10
-1
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user