mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-29 18:46:59 +00:00
fix(session): skip display_kind timeline rows in undo/retry turn targets
list_recent_user_messages and the in-memory /retry + session.undo walkers treated every role=user row as a real user turn. Timeline bookkeeping (model_switch, async_delegation_complete, auto_continue, hidden) is stored that way, so /undo soft-deleted from a marker and /retry re-sent opaque bookkeeping text. Exclude display_kind the same way CLI resume counting and the prompt.submit ordinal path do.
This commit is contained in:
parent
9e2f07e704
commit
748c12b148
6 changed files with 142 additions and 18 deletions
14
cli.py
14
cli.py
|
|
@ -8079,10 +8079,13 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
|
|||
print("(._.) No messages to retry.")
|
||||
return None
|
||||
|
||||
# Walk backwards to find the last user message
|
||||
# Walk backwards to the last *real* user message. Timeline bookkeeping
|
||||
# rows (display_kind set) are role=user but are not user turns — match
|
||||
# CLI resume counting and list_recent_user_messages.
|
||||
last_user_idx = None
|
||||
for i in range(len(self.conversation_history) - 1, -1, -1):
|
||||
if self.conversation_history[i].get("role") == "user":
|
||||
msg = self.conversation_history[i]
|
||||
if msg.get("role") == "user" and not msg.get("display_kind"):
|
||||
last_user_idx = i
|
||||
break
|
||||
|
||||
|
|
@ -8127,10 +8130,13 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
|
|||
if n < 1:
|
||||
n = 1
|
||||
|
||||
# Walk backwards collecting the indices of the last N user messages.
|
||||
# Walk backwards collecting the indices of the last N *real* user
|
||||
# messages (exclude display_kind timeline rows — same predicate as
|
||||
# list_recent_user_messages and resume turn counting).
|
||||
user_indices = []
|
||||
for i in range(len(self.conversation_history) - 1, -1, -1):
|
||||
if self.conversation_history[i].get("role") == "user":
|
||||
msg = self.conversation_history[i]
|
||||
if msg.get("role") == "user" and not msg.get("display_kind"):
|
||||
user_indices.append(i)
|
||||
if len(user_indices) >= n:
|
||||
break
|
||||
|
|
|
|||
|
|
@ -2603,12 +2603,17 @@ class GatewaySlashCommandsMixin:
|
|||
session_entry = await self.async_session_store.get_or_create_session(source)
|
||||
history = await self.async_session_store.load_transcript(session_entry.session_id)
|
||||
|
||||
# Find the last user message
|
||||
# Find the last *real* user message. Timeline bookkeeping rows carry
|
||||
# role=user + display_kind (model_switch / async_delegation_complete /
|
||||
# auto_continue / hidden); clients never count them as user turns.
|
||||
# Without this filter /retry rewrote the transcript around a marker
|
||||
# and re-sent opaque bookkeeping text (same class as the TUI ordinal).
|
||||
last_user_msg = None
|
||||
last_user_idx = None
|
||||
for i in range(len(history) - 1, -1, -1):
|
||||
if history[i].get("role") == "user":
|
||||
last_user_msg = history[i].get("content", "")
|
||||
msg = history[i]
|
||||
if msg.get("role") == "user" and not msg.get("display_kind"):
|
||||
last_user_msg = msg.get("content", "")
|
||||
last_user_idx = i
|
||||
break
|
||||
|
||||
|
|
|
|||
|
|
@ -8049,16 +8049,27 @@ class SessionDB:
|
|||
Each entry is a dict with keys ``id``, ``timestamp``, ``preview``.
|
||||
``preview`` is the first 80 characters of the message content
|
||||
(with line breaks collapsed to spaces). Used by the /rewind
|
||||
slash command picker.
|
||||
slash command picker, CLI/TUI/gateway ``/undo [N]``, and any other
|
||||
caller that needs real user-turn targets.
|
||||
|
||||
Bookkeeping timeline rows (``display_kind`` set — e.g. model_switch,
|
||||
async_delegation_complete, auto_continue, hidden) are excluded. They
|
||||
are durable ``role='user'`` rows for the API transcript, but no client
|
||||
counts them as user turns (desktop demotes them to system / drops them;
|
||||
the CLI already uses ``not m.get("display_kind")``). Including them here
|
||||
made ``/undo`` soft-delete from a marker instead of the last real turn —
|
||||
same class of index skew as the prompt.submit ordinal bug.
|
||||
|
||||
By default only active messages are returned.
|
||||
"""
|
||||
active_clause = "" if include_inactive else " AND active = 1"
|
||||
# Match CLI/desktop: only real user turns, not timeline bookkeeping.
|
||||
display_clause = " AND (display_kind IS NULL OR display_kind = '')"
|
||||
with self._lock:
|
||||
cursor = self._conn.execute(
|
||||
"SELECT id, timestamp, content FROM messages "
|
||||
"WHERE session_id = ? AND role = 'user'"
|
||||
f"{active_clause} "
|
||||
f"{active_clause}{display_clause} "
|
||||
"ORDER BY id DESC LIMIT ?",
|
||||
(session_id, int(limit)),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1908,6 +1908,45 @@ def test_command_dispatch_retry_finds_last_user_message(server):
|
|||
assert server._sessions[sid]["history_version"] == 1
|
||||
|
||||
|
||||
def test_command_dispatch_retry_skips_display_kind_timeline_rows(server):
|
||||
"""/retry must resend the last real user turn, not a display_kind marker."""
|
||||
sid = "test-session-retry-timeline"
|
||||
history = [
|
||||
{"role": "user", "content": "first question"},
|
||||
{"role": "assistant", "content": "first answer"},
|
||||
{"role": "user", "content": "second question"},
|
||||
{"role": "assistant", "content": "second answer"},
|
||||
{
|
||||
"role": "user",
|
||||
"content": "background agent work finished",
|
||||
"display_kind": "async_delegation_complete",
|
||||
},
|
||||
]
|
||||
server._sessions[sid] = {
|
||||
"session_key": sid,
|
||||
"agent": None,
|
||||
"history": history,
|
||||
"history_lock": threading.Lock(),
|
||||
"history_version": 0,
|
||||
}
|
||||
|
||||
resp = server.handle_request({
|
||||
"id": "r4b",
|
||||
"method": "command.dispatch",
|
||||
"params": {"name": "retry", "session_id": sid},
|
||||
})
|
||||
|
||||
assert "error" not in resp
|
||||
result = resp["result"]
|
||||
assert result["type"] == "send"
|
||||
assert result["message"] == "second question"
|
||||
# Truncated through the real last user turn (and the trailing marker).
|
||||
assert [m["content"] for m in server._sessions[sid]["history"]] == [
|
||||
"first question",
|
||||
"first answer",
|
||||
]
|
||||
|
||||
|
||||
def test_command_dispatch_retry_empty_history(server):
|
||||
"""command.dispatch /retry with empty history returns error."""
|
||||
sid = "test-session"
|
||||
|
|
|
|||
|
|
@ -176,6 +176,56 @@ def test_undo_refuses_when_session_busy(server, session_with_history):
|
|||
assert "busy" in resp["error"]["message"].lower()
|
||||
|
||||
|
||||
def test_undo_skips_display_kind_timeline_rows(server, db):
|
||||
"""/undo must target real user turns, not durable timeline bookkeeping.
|
||||
|
||||
async_delegation_complete / model_switch / auto_continue rows are stored
|
||||
as role=user with display_kind set. Clients never count them as user turns.
|
||||
Without the list_recent_user_messages filter, /undo 1 soft-deleted only
|
||||
the trailing marker and left the last real exchange intact.
|
||||
"""
|
||||
sid = "sid-undo-timeline"
|
||||
session_key = "tui-undo-timeline"
|
||||
db.create_session(session_key, source="tui")
|
||||
db.append_message(session_key, "user", "question 1")
|
||||
db.append_message(session_key, "assistant", "answer 1")
|
||||
db.append_message(session_key, "user", "question 2")
|
||||
db.append_message(session_key, "assistant", "answer 2")
|
||||
db.append_message(
|
||||
session_key,
|
||||
"user",
|
||||
"background agent work finished",
|
||||
display_kind="async_delegation_complete",
|
||||
)
|
||||
history = db.get_messages_as_conversation(session_key)
|
||||
agent = MagicMock()
|
||||
agent._memory_manager = MagicMock()
|
||||
agent._last_flushed_db_idx = len(history)
|
||||
s = {
|
||||
"session_key": session_key,
|
||||
"history": list(history),
|
||||
"history_lock": threading.Lock(),
|
||||
"history_version": 0,
|
||||
"running": False,
|
||||
"agent": agent,
|
||||
"attached_images": [],
|
||||
"cols": 120,
|
||||
}
|
||||
server._sessions[sid] = s
|
||||
server._db = db
|
||||
|
||||
resp = _call(server, "command.dispatch", session_id=sid, name="undo", arg="")
|
||||
result = resp["result"]
|
||||
assert result["type"] == "prefill"
|
||||
# Must prefill the last *real* user turn, not the marker text.
|
||||
assert result["message"] == "question 2"
|
||||
# Active history: only q1/a1 remain (q2+a2+marker soft-deleted).
|
||||
assert len(s["history"]) == 2
|
||||
assert [m.get("content") for m in s["history"]] == ["question 1", "answer 1"]
|
||||
active = db.get_messages(session_key, include_inactive=False)
|
||||
assert [r["content"] for r in active] == ["question 1", "answer 1"]
|
||||
|
||||
|
||||
def test_undo_errors_when_no_active_session(server):
|
||||
resp = _call(server, "command.dispatch", session_id="no-such-sid", name="undo", arg="")
|
||||
assert "error" in resp
|
||||
|
|
|
|||
|
|
@ -10122,13 +10122,20 @@ def _(rid, params: dict) -> dict:
|
|||
removed = 0
|
||||
with session["history_lock"]:
|
||||
history = session.get("history", [])
|
||||
while history and history[-1].get("role") in {"assistant", "tool"}:
|
||||
history.pop()
|
||||
removed += 1
|
||||
if history and history[-1].get("role") == "user":
|
||||
history.pop()
|
||||
removed += 1
|
||||
if removed:
|
||||
# Truncate from the last *real* user turn (no display_kind). Popping
|
||||
# only trailing assistant/tool then one user left timeline markers
|
||||
# (async_delegation_complete, model_switch, …) as the undo target —
|
||||
# so session.undo removed bookkeeping instead of the last exchange.
|
||||
# Match list_recent_user_messages / CLI turn counting.
|
||||
last_user_idx = None
|
||||
for i in range(len(history) - 1, -1, -1):
|
||||
msg = history[i]
|
||||
if msg.get("role") == "user" and not msg.get("display_kind"):
|
||||
last_user_idx = i
|
||||
break
|
||||
if last_user_idx is not None:
|
||||
removed = len(history) - last_user_idx
|
||||
del history[last_user_idx:]
|
||||
session["history_version"] = int(session.get("history_version", 0)) + 1
|
||||
return _ok(rid, {"removed": removed})
|
||||
|
||||
|
|
@ -15719,10 +15726,16 @@ def _(rid, params: dict) -> dict:
|
|||
history = session.get("history", [])
|
||||
if not history:
|
||||
return _err(rid, 4018, "no previous user message to retry")
|
||||
# Walk backwards to find the last user message
|
||||
# Walk backwards to the last *real* user turn. Timeline bookkeeping
|
||||
# rows (display_kind set) are durable role=user but no client counts
|
||||
# them as user turns — same predicate as CLI resume/count and the
|
||||
# prompt.submit ordinal fix. Without this, /retry re-sends opaque
|
||||
# markers (model_switch / async_delegation_complete / auto_continue)
|
||||
# and truncates only the marker instead of the failed exchange.
|
||||
last_user_idx = None
|
||||
for i in range(len(history) - 1, -1, -1):
|
||||
if history[i].get("role") == "user":
|
||||
msg = history[i]
|
||||
if msg.get("role") == "user" and not msg.get("display_kind"):
|
||||
last_user_idx = i
|
||||
break
|
||||
if last_user_idx is None:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue