diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index fa7cdfada533..540ea37ed221 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -8009,150 +8009,188 @@ def test_mirror_slash_compress_does_not_prelock_history(monkeypatch): assert "tokens" in warning -def test_mirror_slash_compress_here_triggers_partial_compress(monkeypatch): - """/compress here [N] must split history into head/tail and rejoin after - compression — the partial_compress module is used, not full compress. +_PARTIAL_FAKE_HISTORY = [ + {"role": "user", "content": "msg1"}, + {"role": "assistant", "content": "resp1"}, + {"role": "user", "content": "msg2"}, + {"role": "assistant", "content": "resp2"}, + {"role": "user", "content": "keep this"}, + {"role": "assistant", "content": "keep this too"}, +] +_PARTIAL_COMPRESSED_HEAD = [ + {"role": "user", "content": "[summary]"}, + {"role": "assistant", "content": "ok"}, +] - Before this fix, /compress here 3 passed "here 3" as focus_topic to the - full compress, silently ignoring the boundary intent. - """ - import types - - _FAKE_HISTORY = [ - {"role": "user", "content": "msg1"}, - {"role": "assistant", "content": "resp1"}, - {"role": "user", "content": "msg2"}, - {"role": "assistant", "content": "resp2"}, - {"role": "user", "content": "keep this"}, - {"role": "assistant", "content": "keep this too"}, - ] - _COMPRESSED_HEAD = [{"role": "user", "content": "[summary]"}, - {"role": "assistant", "content": "ok"}] - _REJOINED = _COMPRESSED_HEAD + _FAKE_HISTORY[-2:] - - compress_context_calls = [] - rejoin_calls = [] - full_compress_calls = [] +def _partial_compress_agent(compress_context_calls): + """Agent stub whose _compress_context records (history, focus_topic).""" agent = types.SimpleNamespace( _cached_system_prompt=None, tools=None, session_id="s1", + context_compressor=None, # keep _get_usage on the simple path ) def _fake_compress_context(history, sys, approx_tokens=0, focus_topic=None, **kw): compress_context_calls.append((list(history), focus_topic)) - return _COMPRESSED_HEAD, {} + return list(_PARTIAL_COMPRESSED_HEAD), {} agent._compress_context = _fake_compress_context + return agent - def _fake_full_compress(session, focus_topic=None, **_kw): - full_compress_calls.append(focus_topic) - return (0, {}) - def _fake_rejoin(head, tail): - rejoin_calls.append((list(head), list(tail))) - return head + tail +def test_compress_session_history_here_triggers_partial_compress(): + """/compress here [N] must split history into head/tail and rejoin after + compression — the partial_compress module is used, not full compress. - monkeypatch.setattr(server, "_compress_session_history", _fake_full_compress) - monkeypatch.setattr(server, "_sync_session_key_after_compress", lambda *a, **kw: None) - monkeypatch.setattr(server, "_session_info", lambda _agent: {"model": "x"}) - monkeypatch.setattr(server, "_emit", lambda *args: None) - monkeypatch.setattr( - "hermes_cli.partial_compress.rejoin_compressed_head_and_tail", - _fake_rejoin, - ) + Before this fix, /compress here 3 passed "here 3" as focus_topic to the + full compress, silently ignoring the boundary intent. The parsing lives + in _compress_session_history — the choke point every manual-compress + route (session.compress RPC, command.dispatch, slash-exec mirror) + converges on — so 'here [N]' works everywhere (#35533). + """ + compress_context_calls = [] + agent = _partial_compress_agent(compress_context_calls) - import threading - lock = threading.Lock() - session = _session(running=False) - session["history_lock"] = lock - session["history"] = list(_FAKE_HISTORY) + session = _session(agent=agent) + session["history"] = list(_PARTIAL_FAKE_HISTORY) session["history_version"] = 7 - session["agent"] = agent - warning = server._mirror_slash_side_effects("sid", session, "/compress here 1") + removed, _usage = server._compress_session_history(session, "here 1") - assert warning == "" - # Partial compress must NOT fall back to full _compress_session_history - assert full_compress_calls == [], ( - "full compress was called — partial compress path not taken" - ) # agent._compress_context must have been called with the HEAD only assert len(compress_context_calls) == 1 head_passed, focus_passed = compress_context_calls[0] + assert head_passed == _PARTIAL_FAKE_HISTORY[:-2] assert focus_passed is None # partial compress has no focus topic - # rejoin must have been called to re-attach the tail - assert len(rejoin_calls) == 1 - # Session history must now contain the rejoined transcript - assert session["history"] == _REJOINED + # Session history must now contain the rejoined transcript: compressed + # head + the last exchange verbatim. + assert session["history"] == _PARTIAL_COMPRESSED_HEAD + _PARTIAL_FAKE_HISTORY[-2:] assert session["history_version"] == 8 + assert removed == len(_PARTIAL_FAKE_HISTORY) - len(session["history"]) -def test_mirror_slash_compress_here_falls_back_on_degenerate_split(monkeypatch): - """/compress here on a very short history with keep_last >= exchanges - produces an empty tail — must fall back to full compress.""" - import types +def test_compress_session_history_here_falls_back_on_degenerate_split(): + """/compress here with keep_last >= exchanges produces an empty tail — + must fall back to full compression (whole history, no rejoined tail).""" + compress_context_calls = [] + agent = _partial_compress_agent(compress_context_calls) - # Only 2 messages — keep_last=5 means nothing to compress - _SHORT_HISTORY = [ - {"role": "user", "content": "hi"}, - {"role": "assistant", "content": "hello"}, - ] - full_compress_calls = [] + # 4 messages = 2 exchanges; keep_last=5 leaves nothing to compress. + short_history = _PARTIAL_FAKE_HISTORY[:4] + session = _session(agent=agent) + session["history"] = list(short_history) - agent = types.SimpleNamespace( - _cached_system_prompt=None, tools=None, session_id="s1" - ) + server._compress_session_history(session, "here 5") - def _fake_full_compress(session, focus_topic=None, **_kw): - full_compress_calls.append(focus_topic) - return (0, {}) - - monkeypatch.setattr(server, "_compress_session_history", _fake_full_compress) - monkeypatch.setattr(server, "_sync_session_key_after_compress", lambda *a, **kw: None) - monkeypatch.setattr(server, "_session_info", lambda _agent: {"model": "x"}) - monkeypatch.setattr(server, "_emit", lambda *args: None) - - import threading - session = _session(running=False) - session["history_lock"] = threading.Lock() - session["history"] = list(_SHORT_HISTORY) - session["history_version"] = 0 - session["agent"] = agent - - server._mirror_slash_side_effects("sid", session, "/compress here 5") - - # Degenerate split → full compress, focus_topic=None - assert full_compress_calls == [None] + # Degenerate split → full compress of the whole history, focus_topic=None + assert len(compress_context_calls) == 1 + head_passed, focus_passed = compress_context_calls[0] + assert head_passed == short_history + assert focus_passed is None + assert session["history"] == _PARTIAL_COMPRESSED_HEAD -def test_mirror_slash_compress_plain_focus_topic_not_parsed_as_partial(monkeypatch): +def test_compress_session_history_plain_focus_topic_not_parsed_as_partial(): """/compress my topic must still do full compress with focus_topic set.""" - import types + compress_context_calls = [] + agent = _partial_compress_agent(compress_context_calls) - full_compress_calls = [] - agent = types.SimpleNamespace(session_id="s1") + session = _session(agent=agent) + session["history"] = list(_PARTIAL_FAKE_HISTORY) - def _fake_full_compress(session, focus_topic=None, **_kw): - full_compress_calls.append(focus_topic) - return (0, {}) + server._compress_session_history(session, "my topic") - monkeypatch.setattr(server, "_compress_session_history", _fake_full_compress) + assert len(compress_context_calls) == 1 + head_passed, focus_passed = compress_context_calls[0] + assert head_passed == _PARTIAL_FAKE_HISTORY # full history, no split + assert focus_passed == "my topic" + assert session["history"] == _PARTIAL_COMPRESSED_HEAD + + +def test_session_compress_rpc_honors_here_argument(monkeypatch): + """Route 1/3: the session.compress RPC must honor 'here [N]'.""" + compress_context_calls = [] + agent = _partial_compress_agent(compress_context_calls) + session = _session(agent=agent) + session["history"] = list(_PARTIAL_FAKE_HISTORY) + server._sessions["sid"] = session + + monkeypatch.setattr(server, "_session_info", lambda *_a, **_kw: {"model": "x"}) monkeypatch.setattr(server, "_sync_session_key_after_compress", lambda *a, **kw: None) - monkeypatch.setattr(server, "_session_info", lambda _agent: {"model": "x"}) monkeypatch.setattr(server, "_emit", lambda *args: None) - import threading - session = _session(running=False) - session["history_lock"] = threading.Lock() - session["history"] = [] - session["history_version"] = 0 - session["agent"] = agent + try: + resp = server.handle_request( + { + "id": "1", + "method": "session.compress", + "params": {"session_id": "sid", "focus_topic": "here 1"}, + } + ) + finally: + server._sessions.pop("sid", None) - server._mirror_slash_side_effects("sid", session, "/compress my topic") + assert resp["result"]["status"] == "compressed" + assert len(compress_context_calls) == 1 + head_passed, focus_passed = compress_context_calls[0] + assert head_passed == _PARTIAL_FAKE_HISTORY[:-2] + assert focus_passed is None + assert session["history"] == _PARTIAL_COMPRESSED_HEAD + _PARTIAL_FAKE_HISTORY[-2:] - assert full_compress_calls == ["my topic"] + +def test_command_dispatch_compress_honors_here_argument(monkeypatch): + """Route 2/3: command.dispatch /compress must honor 'here [N]'.""" + compress_context_calls = [] + agent = _partial_compress_agent(compress_context_calls) + session = _session(agent=agent) + session["history"] = list(_PARTIAL_FAKE_HISTORY) + server._sessions["sid"] = session + + monkeypatch.setattr(server, "_session_uses_compute_host", lambda *_a, **_kw: False) + monkeypatch.setattr(server, "_session_info", lambda *_a, **_kw: {"model": "x"}) + monkeypatch.setattr(server, "_sync_session_key_after_compress", lambda *a, **kw: None) + monkeypatch.setattr(server, "_emit", lambda *args: None) + + try: + resp = server.handle_request( + { + "id": "1", + "method": "command.dispatch", + "params": {"session_id": "sid", "name": "compress", "arg": "here 1"}, + } + ) + finally: + server._sessions.pop("sid", None) + + assert resp["result"]["type"] == "exec" + assert len(compress_context_calls) == 1 + head_passed, focus_passed = compress_context_calls[0] + assert head_passed == _PARTIAL_FAKE_HISTORY[:-2] + assert focus_passed is None + assert session["history"] == _PARTIAL_COMPRESSED_HEAD + _PARTIAL_FAKE_HISTORY[-2:] + + +def test_mirror_slash_compress_honors_here_argument(monkeypatch): + """Route 3/3: the slash-exec mirror must honor 'here [N]'.""" + compress_context_calls = [] + agent = _partial_compress_agent(compress_context_calls) + session = _session(agent=agent) + session["history"] = list(_PARTIAL_FAKE_HISTORY) + + monkeypatch.setattr(server, "_session_info", lambda *_a, **_kw: {"model": "x"}) + monkeypatch.setattr(server, "_sync_session_key_after_compress", lambda *a, **kw: None) + monkeypatch.setattr(server, "_emit", lambda *args: None) + + warning = server._mirror_slash_side_effects("sid", session, "/compress here 1") + + assert "Compressed:" in warning + assert len(compress_context_calls) == 1 + head_passed, focus_passed = compress_context_calls[0] + assert head_passed == _PARTIAL_FAKE_HISTORY[:-2] + assert focus_passed is None + assert session["history"] == _PARTIAL_COMPRESSED_HEAD + _PARTIAL_FAKE_HISTORY[-2:] # --------------------------------------------------------------------------- diff --git a/tui_gateway/server.py b/tui_gateway/server.py index f3047d47f028..96e7a9447693 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -3649,10 +3649,29 @@ def _compress_session_history( before_messages: list | None = None, history_version: int | None = None, ) -> tuple[int, dict]: + """Compress a session's history — the single choke point shared by all + three manual-compress routes (session.compress RPC, command.dispatch + /compress|/compact, and the slash-exec mirror). + + ``focus_topic`` is the RAW argument string after ``/compress``. It is + parsed here with :func:`parse_partial_compress_args` so boundary-aware + forms (``here [N]``, ``up to here``, ``--keep N``) trigger a partial + compress — head summarized, most recent ``keep_last`` exchanges kept + verbatim — on EVERY route, mirroring cli.py's ``_manual_compress`` and + gateway/slash_commands.py (PR #35252). Parsing at the choke point (not + per-route) is what fixes #35533: previously "/compress here 3" reached + this helper unparsed and ran a FULL compress focused on the literal + text "here 3". + """ from agent.conversation_compression import ( finalize_context_engine_compression_notification, ) from agent.model_metadata import estimate_request_tokens_rough + from hermes_cli.partial_compress import ( + parse_partial_compress_args, + rejoin_compressed_head_and_tail, + split_history_for_partial_compress, + ) agent = session["agent"] # Snapshot history under the lock so the LLM-bound compression call @@ -3667,6 +3686,18 @@ def _compress_session_history( if len(history) < 4: usage = _get_usage(agent) return 0, usage + partial, keep_last, focus_topic = parse_partial_compress_args(focus_topic or "") + # Boundary-aware split: only the head is summarized; the most recent + # `keep_last` exchanges ride along verbatim. A degenerate split (empty + # tail — everything would be kept, or no head left to compress) falls + # back to full compression so the user still gets an action. + tail: list = [] + head = history + if partial: + head, tail = split_history_for_partial_compress(history, keep_last) + if not tail: + partial = False + head = history if approx_tokens is None: # Include system prompt + tool schemas so the figure reflects real # request pressure, not a transcript-only underestimate (#6217). @@ -3687,9 +3718,12 @@ def _compress_session_history( # and gateway handlers. try: compressed, _ = agent._compress_context( - history, + head, None, approx_tokens=approx_tokens, + # Partial compress has no focus topic (the modes are exclusive; + # parse_partial_compress_args returns focus_topic=None for the + # boundary-aware forms). focus_topic=focus_topic or None, force=True, defer_context_engine_notification=True, @@ -3700,6 +3734,8 @@ def _compress_session_history( committed=False, ) raise + if partial and tail: + compressed = rejoin_compressed_head_and_tail(compressed, tail) with session["history_lock"]: if int(session.get("history_version", 0)) != history_version: # External mutation during compaction — drop the compressed @@ -15446,15 +15482,9 @@ def _mirror_slash_side_effects(sid: str, session: dict, command: str) -> str: from agent.conversation_compression import ( finalize_context_engine_compression_notification, ) - from hermes_cli.partial_compress import ( - parse_partial_compress_args, - rejoin_compressed_head_and_tail, - split_history_for_partial_compress, - ) with session["history_lock"]: _before_messages = list(session.get("history", [])) - _hv = int(session.get("history_version", 0)) _before_count = len(_before_messages) _sys_prompt = getattr(agent, "_cached_system_prompt", "") or "" _tools = getattr(agent, "tools", None) or None @@ -15466,31 +15496,11 @@ def _mirror_slash_side_effects(sid: str, session: dict, command: str) -> str: else 0 ) - # Boundary-aware forms (here [N], up to here, --keep N) split the - # history and compress only the head, keeping the most recent - # exchanges verbatim — mirroring cli.py and gateway/run.py's - # /compress here implementation (PR #35252). Before this fix the - # raw argument was passed straight through as a focus topic, so - # "/compress here 3" silently did a FULL compress focused on the - # literal text "here 3". - partial, keep_last, focus_topic = parse_partial_compress_args(arg or "") - if partial: - head, tail = split_history_for_partial_compress( - _before_messages, keep_last - ) - if not tail: - partial = False # degenerate split — fall back to full compress - else: - _compressed_head, _ = agent._compress_context( - head, None, approx_tokens=_before_tokens, focus_topic=None - ) - _rejoined = rejoin_compressed_head_and_tail(_compressed_head, tail) - with session["history_lock"]: - if int(session.get("history_version", 0)) == _hv: - session["history"] = _rejoined - session["history_version"] = _hv + 1 - if not partial: - _compress_session_history(session, focus_topic) + # The raw argument goes through unparsed: _compress_session_history + # (the choke point shared by all three manual-compress routes) + # parses the boundary-aware forms (here [N], up to here, --keep N) + # and does the partial head/tail split there (#35533). + _compress_session_history(session, arg) _sync_session_key_after_compress(sid, session) with session["history_lock"]: