mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-21 16:18:55 +00:00
Merge pull request #67247 from NousResearch/bb/desktop-resume-single-read
perf(gateway): serve session.resume model + display history from one SELECT
This commit is contained in:
commit
ea1cc1dd4b
5 changed files with 224 additions and 11 deletions
|
|
@ -4814,6 +4814,37 @@ class SessionDB:
|
|||
tuple(session_ids),
|
||||
).fetchall()
|
||||
|
||||
return self._rows_to_conversation(
|
||||
rows,
|
||||
session_id=session_id,
|
||||
include_ancestors=include_ancestors,
|
||||
repair_alternation=repair_alternation,
|
||||
)
|
||||
|
||||
# Columns every conversation projection decodes. Shared by
|
||||
# get_messages_as_conversation and get_resume_conversations so a single
|
||||
# SELECT can feed both the model-fed and display views.
|
||||
_CONVERSATION_ROW_COLUMNS = (
|
||||
"role, content, tool_call_id, tool_calls, tool_name, effect_disposition, "
|
||||
"finish_reason, reasoning, reasoning_content, reasoning_details, "
|
||||
"codex_reasoning_items, codex_message_items, platform_message_id, observed, timestamp"
|
||||
)
|
||||
|
||||
def _rows_to_conversation(
|
||||
self,
|
||||
rows,
|
||||
*,
|
||||
session_id: str,
|
||||
include_ancestors: bool,
|
||||
repair_alternation: bool,
|
||||
) -> List[Dict[str, Any]]:
|
||||
"""Decode fetched message rows into the OpenAI conversation format.
|
||||
|
||||
Extracted from get_messages_as_conversation so get_resume_conversations
|
||||
can build the model-fed and display views from one SELECT. ``rows`` must
|
||||
already be ordered by ``id`` (insertion order) and filtered to the
|
||||
desired session set / active state by the caller.
|
||||
"""
|
||||
messages = []
|
||||
for row in rows:
|
||||
content = self._decode_content(row["content"])
|
||||
|
|
@ -4902,6 +4933,55 @@ class SessionDB:
|
|||
)
|
||||
return messages
|
||||
|
||||
def get_resume_conversations(
|
||||
self, session_id: str
|
||||
) -> Tuple[List[Dict[str, Any]], List[Dict[str, Any]]]:
|
||||
"""Return ``(model_history, display_history)`` for a session resume in ONE SELECT.
|
||||
|
||||
``session.resume`` needs two projections of the same lineage:
|
||||
|
||||
- ``model_history`` — the tip session's active rows, alternation-repaired
|
||||
(the live-replay working conversation). Equivalent to
|
||||
``get_messages_as_conversation(session_id, repair_alternation=True)``.
|
||||
- ``display_history`` — the full lineage (ancestors → tip), verbatim, with
|
||||
replayed-user dedup. Equivalent to
|
||||
``get_messages_as_conversation(session_id, include_ancestors=True)``.
|
||||
|
||||
The display fetch already reads a superset of the model fetch (the tip
|
||||
rows are part of the lineage), so serving both from one lineage SELECT
|
||||
halves the resume's DB work versus two separate calls, with byte-identical
|
||||
output (see test_get_resume_conversations_matches_separate_reads).
|
||||
"""
|
||||
session_ids = self._session_lineage_root_to_tip(session_id)
|
||||
with self._lock:
|
||||
placeholders = ",".join("?" for _ in session_ids)
|
||||
rows = self._conn.execute(
|
||||
f"SELECT session_id, {self._CONVERSATION_ROW_COLUMNS} "
|
||||
f"FROM messages WHERE session_id IN ({placeholders}) AND active = 1 "
|
||||
# ORDER BY id (insertion order) — see get_messages_as_conversation
|
||||
# for why timestamp ordering is unsafe.
|
||||
"ORDER BY id",
|
||||
tuple(session_ids),
|
||||
).fetchall()
|
||||
|
||||
# Tip rows are exactly the model-fed set (get_messages_as_conversation
|
||||
# with session_ids=[session_id]); filtering the lineage fetch preserves
|
||||
# their relative id order.
|
||||
tip_rows = [r for r in rows if r["session_id"] == session_id]
|
||||
model_history = self._rows_to_conversation(
|
||||
tip_rows,
|
||||
session_id=session_id,
|
||||
include_ancestors=False,
|
||||
repair_alternation=True,
|
||||
)
|
||||
display_history = self._rows_to_conversation(
|
||||
rows,
|
||||
session_id=session_id,
|
||||
include_ancestors=True,
|
||||
repair_alternation=False,
|
||||
)
|
||||
return model_history, display_history
|
||||
|
||||
def get_conversation_root(self, session_id: str) -> str:
|
||||
"""Return the ROOT id of *session_id*'s lineage chain.
|
||||
|
||||
|
|
|
|||
|
|
@ -1302,6 +1302,69 @@ class TestMessageStorage:
|
|||
|
||||
assert [m["content"] for m in conv if m["role"] == "user"] == ["same prompt", "next prompt"]
|
||||
|
||||
def test_get_resume_conversations_matches_separate_reads(self, db):
|
||||
"""The one-fetch resume projections must be byte-identical to the two
|
||||
separate get_messages_as_conversation reads they replace — the whole
|
||||
point of the single-SELECT optimization (desktop audit P1). Includes a
|
||||
dangling tool-call tail so repair_alternation drops rows and the model /
|
||||
display lengths diverge (exercises session.resume's prefix computation).
|
||||
"""
|
||||
db.create_session("root", "tui")
|
||||
db.append_message("root", role="user", content="first prompt")
|
||||
db.append_message("root", role="assistant", content="first answer")
|
||||
db.create_session("child", "tui", parent_session_id="root")
|
||||
db.append_message("child", role="user", content="second prompt")
|
||||
db.append_message(
|
||||
"child", role="assistant", content="second answer", finish_reason="stop"
|
||||
)
|
||||
# Dangling assistant(tool_calls) tail with no tool response → repair
|
||||
# drops it, so model_history is shorter than display_history.
|
||||
db.append_message(
|
||||
"child",
|
||||
role="assistant",
|
||||
content="",
|
||||
tool_calls=[
|
||||
{"id": "t1", "type": "function", "function": {"name": "x", "arguments": "{}"}}
|
||||
],
|
||||
)
|
||||
|
||||
model_expected = db.get_messages_as_conversation("child", repair_alternation=True)
|
||||
display_expected = db.get_messages_as_conversation("child", include_ancestors=True)
|
||||
|
||||
model_history, display_history = db.get_resume_conversations("child")
|
||||
|
||||
assert model_history == model_expected
|
||||
assert display_history == display_expected
|
||||
# Sanity: the tail really did diverge the two projections.
|
||||
assert len(display_history) > len(model_history)
|
||||
|
||||
def test_get_resume_conversations_single_session_no_ancestors(self, db):
|
||||
db.create_session("solo", "cli")
|
||||
db.append_message("solo", role="user", content="hi")
|
||||
db.append_message("solo", role="assistant", content="hello")
|
||||
|
||||
model_expected = db.get_messages_as_conversation("solo", repair_alternation=True)
|
||||
display_expected = db.get_messages_as_conversation("solo", include_ancestors=True)
|
||||
model_history, display_history = db.get_resume_conversations("solo")
|
||||
|
||||
assert model_history == model_expected
|
||||
assert display_history == display_expected
|
||||
|
||||
def test_get_resume_conversations_dedupes_replayed_ancestor_user(self, db):
|
||||
db.create_session("root", "tui")
|
||||
db.append_message("root", role="user", content="same prompt")
|
||||
db.append_message("root", role="user", content="same prompt")
|
||||
db.append_message("root", role="assistant", content="answer")
|
||||
db.create_session("child", "tui", parent_session_id="root")
|
||||
db.append_message("child", role="user", content="next prompt")
|
||||
|
||||
model_expected = db.get_messages_as_conversation("child", repair_alternation=True)
|
||||
display_expected = db.get_messages_as_conversation("child", include_ancestors=True)
|
||||
model_history, display_history = db.get_resume_conversations("child")
|
||||
|
||||
assert model_history == model_expected
|
||||
assert display_history == display_expected
|
||||
|
||||
def test_finish_reason_stored(self, db):
|
||||
db.create_session(session_id="s1", source="cli")
|
||||
db.append_message("s1", role="assistant", content="Done", finish_reason="stop")
|
||||
|
|
|
|||
|
|
@ -1412,6 +1412,12 @@ def test_session_resume_uses_parent_lineage_for_display(monkeypatch):
|
|||
def reopen_session(self, target):
|
||||
captured["reopened"] = target
|
||||
|
||||
def get_resume_conversations(self, session_id):
|
||||
return (
|
||||
self.get_messages_as_conversation(session_id, repair_alternation=True),
|
||||
self.get_messages_as_conversation(session_id, include_ancestors=True),
|
||||
)
|
||||
|
||||
def get_messages_as_conversation(self, target, include_ancestors=False, repair_alternation=False):
|
||||
captured.setdefault("history_calls", []).append((target, include_ancestors))
|
||||
return (
|
||||
|
|
@ -1543,6 +1549,12 @@ def test_session_resume_passes_stored_runtime_to_agent(monkeypatch):
|
|||
def reopen_session(self, target):
|
||||
pass
|
||||
|
||||
def get_resume_conversations(self, session_id):
|
||||
return (
|
||||
self.get_messages_as_conversation(session_id, repair_alternation=True),
|
||||
self.get_messages_as_conversation(session_id, include_ancestors=True),
|
||||
)
|
||||
|
||||
def get_messages_as_conversation(self, target, include_ancestors=False, repair_alternation=False):
|
||||
return [{"role": "user", "content": "hello"}]
|
||||
|
||||
|
|
@ -1603,6 +1615,12 @@ def test_session_resume_profile_uses_profile_db_cwd(monkeypatch, tmp_path):
|
|||
def reopen_session(self, _target):
|
||||
captured["reopened"] = _target
|
||||
|
||||
def get_resume_conversations(self, session_id):
|
||||
return (
|
||||
self.get_messages_as_conversation(session_id, repair_alternation=True),
|
||||
self.get_messages_as_conversation(session_id, include_ancestors=True),
|
||||
)
|
||||
|
||||
def get_messages_as_conversation(self, _target, include_ancestors=False, repair_alternation=False):
|
||||
return [{"role": "user", "content": "hello"}]
|
||||
|
||||
|
|
@ -5540,6 +5558,12 @@ def test_slash_exec_r7_read_commands_use_metadata_mirror_flag_on(monkeypatch):
|
|||
"pinned": True,
|
||||
}
|
||||
|
||||
def get_resume_conversations(self, session_id):
|
||||
return (
|
||||
self.get_messages_as_conversation(session_id, repair_alternation=True),
|
||||
self.get_messages_as_conversation(session_id, include_ancestors=True),
|
||||
)
|
||||
|
||||
def get_messages_as_conversation(self, key, include_ancestors=True, repair_alternation=False):
|
||||
assert key == "session-key"
|
||||
assert include_ancestors is True
|
||||
|
|
|
|||
|
|
@ -346,6 +346,12 @@ def test_session_resume_returns_hydrated_messages(server, monkeypatch):
|
|||
def reopen_session(self, _sid):
|
||||
return None
|
||||
|
||||
def get_resume_conversations(self, session_id):
|
||||
return (
|
||||
self.get_messages_as_conversation(session_id, repair_alternation=True),
|
||||
self.get_messages_as_conversation(session_id, include_ancestors=True),
|
||||
)
|
||||
|
||||
def get_messages_as_conversation(self, _sid, include_ancestors=False, repair_alternation=False):
|
||||
return [
|
||||
{"role": "user", "content": "hello"},
|
||||
|
|
@ -406,6 +412,12 @@ def test_session_resume_defaults_to_deferred_build(server, monkeypatch):
|
|||
def reopen_session(self, _sid):
|
||||
return None
|
||||
|
||||
def get_resume_conversations(self, session_id):
|
||||
return (
|
||||
self.get_messages_as_conversation(session_id, repair_alternation=True),
|
||||
self.get_messages_as_conversation(session_id, include_ancestors=True),
|
||||
)
|
||||
|
||||
def get_messages_as_conversation(self, _sid, include_ancestors=False, repair_alternation=False):
|
||||
return [
|
||||
{"role": "user", "content": "hello"},
|
||||
|
|
@ -547,6 +559,12 @@ def test_session_resume_handles_multimodal_list_content(server, monkeypatch):
|
|||
def reopen_session(self, _sid):
|
||||
return None
|
||||
|
||||
def get_resume_conversations(self, session_id):
|
||||
return (
|
||||
self.get_messages_as_conversation(session_id, repair_alternation=True),
|
||||
self.get_messages_as_conversation(session_id, include_ancestors=True),
|
||||
)
|
||||
|
||||
def get_messages_as_conversation(self, _sid, include_ancestors=False, repair_alternation=False):
|
||||
return [multimodal_user, text_only_assistant]
|
||||
|
||||
|
|
@ -597,6 +615,12 @@ def test_session_resume_lazy_registers_watch_session_without_agent(server, monke
|
|||
def reopen_session(self, _sid):
|
||||
return None
|
||||
|
||||
def get_resume_conversations(self, session_id):
|
||||
return (
|
||||
self.get_messages_as_conversation(session_id, repair_alternation=True),
|
||||
self.get_messages_as_conversation(session_id, include_ancestors=True),
|
||||
)
|
||||
|
||||
def get_messages_as_conversation(self, _sid, include_ancestors=False, repair_alternation=False):
|
||||
return [
|
||||
{"role": "user", "content": "delegated goal"},
|
||||
|
|
@ -670,6 +694,12 @@ def test_session_resume_lazy_reports_running_for_inflight_child(server, monkeypa
|
|||
def reopen_session(self, _sid):
|
||||
return None
|
||||
|
||||
def get_resume_conversations(self, session_id):
|
||||
return (
|
||||
self.get_messages_as_conversation(session_id, repair_alternation=True),
|
||||
self.get_messages_as_conversation(session_id, include_ancestors=True),
|
||||
)
|
||||
|
||||
def get_messages_as_conversation(self, _sid, include_ancestors=False, repair_alternation=False):
|
||||
return [{"role": "user", "content": "delegated goal"}]
|
||||
|
||||
|
|
@ -721,6 +751,12 @@ def test_session_resume_lazy_tolerates_missing_row_for_active_child(server, monk
|
|||
def reopen_session(self, _sid):
|
||||
return None
|
||||
|
||||
def get_resume_conversations(self, session_id):
|
||||
return (
|
||||
self.get_messages_as_conversation(session_id, repair_alternation=True),
|
||||
self.get_messages_as_conversation(session_id, include_ancestors=True),
|
||||
)
|
||||
|
||||
def get_messages_as_conversation(self, _sid, include_ancestors=False, repair_alternation=False):
|
||||
# No rows for an unwritten session.
|
||||
return []
|
||||
|
|
@ -818,6 +854,12 @@ def test_session_resume_reuses_existing_live_session(server, monkeypatch):
|
|||
def reopen_session(self, _sid):
|
||||
return None
|
||||
|
||||
def get_resume_conversations(self, session_id):
|
||||
return (
|
||||
self.get_messages_as_conversation(session_id, repair_alternation=True),
|
||||
self.get_messages_as_conversation(session_id, include_ancestors=True),
|
||||
)
|
||||
|
||||
def get_messages_as_conversation(self, _sid, include_ancestors=False, repair_alternation=False):
|
||||
return [
|
||||
{"role": "user", "content": "hello"},
|
||||
|
|
@ -1035,6 +1077,12 @@ def test_session_resume_live_payload_uses_current_history_with_ancestors(server,
|
|||
def reopen_session(self, _sid):
|
||||
return None
|
||||
|
||||
def get_resume_conversations(self, session_id):
|
||||
return (
|
||||
self.get_messages_as_conversation(session_id, repair_alternation=True),
|
||||
self.get_messages_as_conversation(session_id, include_ancestors=True),
|
||||
)
|
||||
|
||||
def get_messages_as_conversation(self, _sid, include_ancestors=False, repair_alternation=False):
|
||||
if include_ancestors:
|
||||
return ancestor_history + current_history
|
||||
|
|
|
|||
|
|
@ -6237,12 +6237,12 @@ def _(rid, params: dict) -> dict:
|
|||
_enable_gateway_prompts()
|
||||
try:
|
||||
db.reopen_session(target)
|
||||
# repair_alternation on the model-fed copy only: this resume feeds
|
||||
# LIVE REPLAY (raw_history → sanitize_replay_history → the resumed
|
||||
# session's working conversation). display_history stays verbatim —
|
||||
# One lineage SELECT feeds both projections (#67142-adjacent perf,
|
||||
# from the desktop audit): the model-fed copy is alternation-repaired
|
||||
# (raw_history → sanitize_replay_history → the resumed session's
|
||||
# working conversation) and the display copy stays verbatim —
|
||||
# inspection/export must show what is actually stored.
|
||||
raw_history = db.get_messages_as_conversation(target, repair_alternation=True)
|
||||
display_history = db.get_messages_as_conversation(target, include_ancestors=True)
|
||||
raw_history, display_history = db.get_resume_conversations(target)
|
||||
except Exception as e:
|
||||
if lease is not None:
|
||||
lease.release()
|
||||
|
|
@ -6315,12 +6315,10 @@ def _(rid, params: dict) -> dict:
|
|||
)
|
||||
try:
|
||||
db.reopen_session(target)
|
||||
# repair_alternation on the model-fed copy only (see the interactive
|
||||
# resume above): this loads LIVE REPLAY history; display stays verbatim.
|
||||
raw_history = db.get_messages_as_conversation(target, repair_alternation=True)
|
||||
display_history = db.get_messages_as_conversation(
|
||||
target, include_ancestors=True
|
||||
)
|
||||
# One lineage SELECT feeds both projections (see the interactive resume
|
||||
# above): the model-fed copy is alternation-repaired for LIVE REPLAY, the
|
||||
# display copy stays verbatim.
|
||||
raw_history, display_history = db.get_resume_conversations(target)
|
||||
# The display transcript keeps every row so the user still sees their
|
||||
# full history. The model-fed history is sanitized: a session whose
|
||||
# last turn died mid-tool-loop persists a dangling assistant(tool_calls)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue