mirrors/hermes-agent
1
0
Fork 0
mirror of https://github.com/NousResearch/hermes-agent.git synced 2026-07-31 19:16:29 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

fix(compression): choose summary role by template-visible alternation

Browse source 
The compaction summary's role was selected against the LITERAL
neighbouring messages (compressed[-1] / tail_messages[0]). Mistral-family
chat templates (Devstral, Mistral Small 3.x, Magistral) enforce
user/assistant alternation but exempt the tool flow (tool results and
assistant messages carrying tool_calls) from the check, so a protected
head ending [user, assistant(tool_calls), tool] pinned the summary to
role="user" while the last role the template counts is "user": the
backend rejects the whole request with a Jinja alternation error
(HTTP 500). The summary persists in the stored conversation, every
retry replays the identical poisoned history, and the session is
permanently unrecoverable. Fires on EVERY compaction against a
Mistral-strict backend, captured byte-exact via a tee-proxy in front of
a llama.cpp/llama-swap Devstral deployment.

Fix: compute both neighbour roles through _template_visible_role(),
which skips template-exempt messages. The #52160 (Anthropic user-first)
and #58753 (zero-user-turn) forced-user guards are preserved; their
forced shapes (summary-user followed only by exempt messages) are
alternation-safe. When the visible head ends "assistant" and the
visible tail opens "user", no standalone role can alternate and the
existing merge-into-tail fallback now correctly fires (the literal
logic emitted a standalone user summary there: a second poisoning
shape).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LGN45sMMbwM8cW9T9ga4ou
This commit is contained in:
Matt Ezell 2026-07-29 05:06:16 -05:00 • committed by kshitij
parent 36e41c09ed
commit 7965462d6c
2 changed files with 323 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

89
agent/context_compressor.py
View file

@ -170,6 +170,35 @@ def _fresh_compaction_message_copy(msg: Dict[str, Any]) -> Dict[str, Any]:
return fresh
def _template_visible_role(message: Any) -> Optional[str]:
"""Role as counted by strict chat-template alternation checks.
Mistral-family templates (Devstral, Mistral Small 3.x, Magistral)
enforce user/assistant alternation at render time but EXEMPT the tool
flow from the check: ``tool`` results and assistant messages carrying
``tool_calls`` are skipped. A summary role chosen against the *literal*
neighbouring roles can therefore still violate alternation as the
template sees it. The canonical failure: the protected head ends
``[user, assistant(tool_calls), tool]``, so the literal last role is
``tool`` and the summary is pinned to ``role="user"`` -- but the last
role the template counts is ``user``, the template sees user -> user,
and llama.cpp / Mistral-hosted backends reject the ENTIRE request with
a Jinja alternation error (HTTP 500). Because the summary persists in
the stored conversation, every retry replays the same poisoned history
and the session is unrecoverable.
Returns ``None`` for messages the alternation check skips.
"""
if not isinstance(message, dict):
return None
role = message.get("role")
if role == "tool":
return None
if role == "assistant" and message.get("tool_calls"):
return None
return role
def _strip_persistence_markers(messages: List[Dict[str, Any]]) -> None:
"""Enforce the compaction invariant: no assembled message carries a
session-store persistence marker.
@ -5420,8 +5449,43 @@ This compaction should PRIORITISE preserving all information related to the focu
# last_head_role reads the assembled (post-strip) head; first_tail_role
# reads the assembled (post-strip) tail_messages — a stripped stale
# handoff must not influence alternation-safe role selection.
last_head_role = compressed[-1].get("role", "user") if compressed else "user"
first_tail_role = tail_messages[0].get("role", "user") if tail_messages else None
# Both are TEMPLATE-VISIBLE roles (``_template_visible_role``), not the
# literal list neighbours: strict Mistral-style templates skip tool
# results and assistant tool-call messages when enforcing
# user/assistant alternation, so the summary must alternate against
# the nearest message the template actually counts. Selecting against
# the literal neighbour (previously ``compressed[-1]``) emitted the
# summary as role="user" behind a ``[user, assistant(tool_calls),
# tool]`` head — which every Mistral-strict backend rejects with a
# Jinja alternation 500, permanently poisoning the session.
last_head_role: Optional[str] = "user"
if compressed:
last_head_role = next(
(
role
for role in (
_template_visible_role(m) for m in reversed(compressed)
)
if role is not None
),
# Head holds only template-exempt messages: the summary will
# be the first message the template counts, and the sequence
# must open with "user" (handled below alongside the forced
# cases).
None,
)
first_tail_role = None
if tail_messages:
first_tail_role = next(
(
role
for role in (
_template_visible_role(m) for m in tail_messages
)
if role is not None
),
None,
)
# When the only protected head message is the system prompt, the
# summary becomes the first *visible* message in the API request
# (most adapters — Anthropic, Bedrock — send the system prompt as
@ -5455,9 +5519,15 @@ This compaction should PRIORITISE preserving all information related to the focu
)
if not _user_survives:
_force_user_leading = True
# Pick a role that avoids consecutive same-role with both neighbors.
# Priority: avoid colliding with head (already committed), then tail.
if last_head_role in {"assistant", "tool"} or _force_user_leading:
# Pick a role that alternates with both template-visible neighbors.
# Priority: alternate against the head (already committed), then tail.
# ``None`` (all-exempt head) means the summary opens the visible
# sequence, which strict templates require to start with "user".
if (
last_head_role is None
or last_head_role in {"assistant", "tool"}
or _force_user_leading
):
summary_role = "user"
else:
summary_role = "assistant"
@ -5465,7 +5535,14 @@ This compaction should PRIORITISE preserving all information related to the focu
# collide with the head, flip it.
if first_tail_role is not None and summary_role == first_tail_role:
flipped = "assistant" if summary_role == "user" else "user"
if flipped != last_head_role and not _force_user_leading:
# ``last_head_role is None`` (all-exempt head) pins the summary to
# "user" above; flipping to "assistant" would make the visible
# sequence open with "assistant", which strict templates reject.
if (
flipped != last_head_role
and last_head_role is not None
and not _force_user_leading
):
summary_role = flipped
else:
# Both roles would create consecutive same-role messages

240
tests/agent/test_summary_role_template_alternation.py Normal file
View file

@ -0,0 +1,240 @@
"""Regression coverage: the compaction summary role must alternate against
TEMPLATE-VISIBLE neighbours, not literal list neighbours.
Mistral-family chat templates (Devstral, Mistral Small 3.x, Magistral)
enforce user/assistant alternation at render time but exempt the tool flow
from the check: ``tool`` results and assistant messages carrying
``tool_calls`` are skipped. The summary-role selection previously keyed off
the LITERAL last head message (``compressed[-1]``), producing this captured
failure (Hermes Desktop v0.19.0 against llama.cpp-served Devstral):
[0] system
[1] user <- original first user turn, protected
[2] assistant (tool_calls, exempt)
[3] tool (exempt)
[4] user <- COMPACTION SUMMARY, pinned to "user" because the
literal previous role was "tool"
[5..] assistant(tool_calls)/tool pairs
The template counts only [1] and [4]: user -> user, and the backend rejects
the ENTIRE request with a Jinja alternation error (HTTP 500):
"After the optional system message, conversation roles must alternate
user and assistant roles except for tool calls and results"
Because the summary persists in the stored conversation, every retry
replays the same poisoned history: the session is permanently broken, on
EVERY compaction, overflow or not.
These tests pin the fix: neighbour roles for summary-role selection are
computed by ``_template_visible_role`` (skipping the exempt tool flow), and
the assembled output always satisfies the Mistral alternation check. The
existing #52160 / #58753 forced-user guards must keep winning: their forced
shapes (summary-user followed only by exempt messages) are alternation-safe.
"""
from __future__ import annotations
from unittest.mock import patch
import pytest
from agent.context_compressor import (
COMPRESSED_SUMMARY_METADATA_KEY,
SUMMARY_PREFIX,
_template_visible_role,
)
@pytest.fixture()
def compressor():
from agent.context_compressor import ContextCompressor
with patch(
"agent.context_compressor.get_model_context_length",
return_value=100_000,
):
c = ContextCompressor(
model="test/model",
threshold_percent=0.50,
protect_first_n=3,
protect_last_n=20,
quiet_mode=True,
)
c.tail_token_budget = 40
return c
def _tool_turns(start: int, n: int, payload: str = "x" * 300) -> list[dict]:
out: list[dict] = []
for i in range(start, start + n):
out.append(
{
"role": "assistant",
"content": None,
"tool_calls": [
{
"id": f"c{i}",
"function": {"name": "terminal", "arguments": "{}"},
}
],
}
)
out.append({"role": "tool", "content": payload, "tool_call_id": f"c{i}"})
return out
def _summary_rows(messages: list[dict]) -> list[dict]:
return [
m
for m in messages
if isinstance(m, dict) and m.get(COMPRESSED_SUMMARY_METADATA_KEY)
]
def _mistral_alternation_ok(messages: list[dict]) -> bool:
"""Replay the Mistral template's pre-flight alternation check: count
only user and assistant-without-tool_calls messages; the counted
sequence must go user, assistant, user, assistant, ..."""
expect = "user"
for m in messages:
role = _template_visible_role(m)
if role in (None, "system"):
continue
if role != expect:
return False
expect = "assistant" if expect == "user" else "user"
return True
class TestTemplateVisibleRoleHelper:
def test_tool_flow_is_exempt(self):
assert _template_visible_role({"role": "tool", "content": "r"}) is None
assert (
_template_visible_role(
{"role": "assistant", "content": "", "tool_calls": [{"id": "c"}]}
)
is None
)
def test_counted_roles_pass_through(self):
assert _template_visible_role({"role": "user", "content": "q"}) == "user"
assert (
_template_visible_role({"role": "assistant", "content": "a"})
== "assistant"
)
assert _template_visible_role({"role": "system", "content": "s"}) == "system"
def test_non_dict_is_exempt(self):
assert _template_visible_role(None) is None
assert _template_visible_role("not a message") is None
class TestSummaryRoleAlternatesAgainstVisibleNeighbours:
def test_captured_devstral_shape_emits_assistant_summary(self, compressor):
"""The byte-captured poisoning shape: protected head ends
``[user, assistant(tool_calls), tool]``, tail is all tool flow.
The literal previous role is ``tool`` (which used to pin the
summary to "user"); the template-visible previous role is
``user``, so the summary must be emitted as ``assistant``."""
c = compressor
messages = [{"role": "user", "content": "run a full systems diagnostic"}]
messages += _tool_turns(0, 30)
mocked = f"{SUMMARY_PREFIX}\nrolled-up summary of the tool work"
with patch.object(c, "_generate_summary", return_value=mocked):
out = c.compress(messages, current_tokens=90_000)
rows = _summary_rows(out)
assert len(rows) == 1
assert rows[0].get("role") == "assistant", (
"REGRESSION: compaction summary emitted as role=user directly "
"after a template-visible user turn (only exempt tool-flow "
"messages between). Mistral-strict templates reject the whole "
"request with a Jinja alternation 500 and the stored session is "
f"poisoned permanently. Got role={rows[0].get('role')!r}."
)
def test_captured_shape_passes_mistral_alternation(self, compressor):
c = compressor
messages = [{"role": "user", "content": "run a full systems diagnostic"}]
messages += _tool_turns(0, 30)
mocked = f"{SUMMARY_PREFIX}\nrolled-up summary of the tool work"
with patch.object(c, "_generate_summary", return_value=mocked):
out = c.compress(messages, current_tokens=90_000)
assert _mistral_alternation_ok(out), (
"Compressed transcript fails the Mistral template alternation "
"check. Visible-role sequence: "
f"{[_template_visible_role(m) for m in out if _template_visible_role(m)]}"
)
def test_visible_head_assistant_visible_tail_user_merges(self, compressor):
"""When the visible head ends ``assistant`` and the visible tail
opens ``user``, NO standalone role can satisfy alternation (user
collides with the tail, assistant with the head -- and the
intervening tool flow is template-exempt, so literal separation
does not help). The existing merge-into-tail fallback must fire.
The literal-role logic used to emit a standalone role="user"
summary here (literal previous role was ``tool``), which is a
second poisoning shape: visible user(summary) -> user(tail)."""
c = compressor
messages = [
{"role": "user", "content": "question " + "q" * 200},
{"role": "assistant", "content": "answer " + "a" * 200},
]
messages += _tool_turns(0, 30)
# A recent user turn preserved in the tail keeps the zero-user
# guard out of the picture.
messages += [
{"role": "user", "content": "latest question"},
{"role": "assistant", "content": "on it"},
]
mocked = f"{SUMMARY_PREFIX}\nsummary body"
with patch.object(c, "_generate_summary", return_value=mocked):
out = c.compress(messages, current_tokens=90_000)
rows = _summary_rows(out)
assert len(rows) == 1
# Merged into a template-exempt tail message: invisible to the
# alternation check, summary content still delivered.
assert _template_visible_role(rows[0]) is None
assert _mistral_alternation_ok(out)
class TestForcedUserGuardsStillWin:
def test_zero_user_guard_still_forces_user(self, compressor):
"""#58753: when no genuine user turn survives, the summary must
still be pinned to role=user (and that shape is alternation-safe
because everything after it is template-exempt)."""
c = compressor
c.compression_count = 1 # protect_first_n decays -> no head
messages = [{"role": "user", "content": "work kanban task 42"}]
messages += _tool_turns(0, 12)
mocked = f"{SUMMARY_PREFIX}\nsummary body"
with patch.object(c, "_generate_summary", return_value=mocked):
out = c.compress(messages, current_tokens=90_000)
rows = _summary_rows(out)
assert len(rows) == 1
assert rows[0].get("role") == "user"
assert _mistral_alternation_ok(out)
def test_no_literal_consecutive_user_roles(self, compressor):
"""The pre-existing literal invariant still holds alongside the
template-visible one."""
c = compressor
messages = [{"role": "user", "content": "run diagnostics"}]
messages += _tool_turns(0, 30)
mocked = f"{SUMMARY_PREFIX}\nsummary body"
with patch.object(c, "_generate_summary", return_value=mocked):
out = c.compress(messages, current_tokens=90_000)
for prev, cur in zip(out, out[1:]):
assert not (
prev.get("role") == "user" and cur.get("role") == "user"
), "compression introduced literal consecutive user-role messages"