mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-24 16:54:43 +00:00
refactor(tui): relocate /compress arg parsing into _compress_session_history choke point
Follow-up to the salvaged #35533 fix: instead of parsing 'here [N]' only in _mirror_slash_side_effects, parse it inside _compress_session_history — the single helper all three manual-compress routes converge on (session.compress RPC, command.dispatch /compress|/compact, slash-exec mirror). Every route now honors the boundary-aware forms (here [N], up to here, --keep N) with the same head/tail split + rejoin as cli.py and gateway/slash_commands.py, and keeps the choke point's existing guards (lock-free LLM call, history_version race check, deferred context-engine notification). Tests: choke-point unit tests for 'here N', degenerate-split fallback, and focus-topic passthrough, plus endpoint-level tests on each of the three routes.
This commit is contained in:
Teknium
parent
9284a3402f
commit
2cabeeabca
2 changed files with 184 additions and 136 deletions
|
|
@ -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:]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -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"]:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue