diff --git a/hermes_state.py b/hermes_state.py index ff27d5b13e39..86618cbed48f 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -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. diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index c12b17a7f805..5561c93d4eaf 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -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") diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 5c6dc79765c9..71d709f5dc6a 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -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 diff --git a/tests/tui_gateway/test_protocol.py b/tests/tui_gateway/test_protocol.py index 49177c7694e0..4fcce61e2dce 100644 --- a/tests/tui_gateway/test_protocol.py +++ b/tests/tui_gateway/test_protocol.py @@ -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 diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 5f90717da6b4..cba10c4aad00 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -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)