mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(agent): make micro-compaction alternation-safe and defrag user-preserving
Two integration bugs found during review of #74522, both confirmed with empirical probes against the production message-repair path: 1. Alternation: the summary marker was role="user" and an exchange was a single assistant+tools group, so splicing between two user turns produced user -> marker(user) -> user. The pre-request repair_message_sequence pass (conversation_loop.py, runs before EVERY API call) then merged the marker into the neighbouring real user message: metadata gone, cursor unrecoverable on resume, and the summary text duplicated into the transcript on every later pass (the transcript GREW every turn). Fix: an exchange is now a full agent turn (assistant + tools + follow-up assistant iterations, bounded by user messages), the marker is assistant-role, and superseding an old marker deliberately merges the two adjacent real user turns (plain-text \n\n-join, identical to repair pass 2) so the returned transcript is alternation-valid by construction. Probe result: repairs 0 (was 2), marker survives, no summary leakage. 2. Defrag destroyed user messages: _defrag_rolling_summary serialized the whole remaining middle (user turns included) and spliced it away — 8 of 10 user prompts destroyed in one pass, contradicting the feature's "your messages are never compacted" invariant. Fix: defrag now re-summarizes only the rolling summary TEXT and rewrites the marker content in place; transcript shape, cursor, and user turns untouched. Probe result: 10 of 10 user prompts survive. Also: marker provenance is now COMPRESSED_SUMMARY_HAS_USER_TURN_KEY=False — micro markers absorb only assistant/tool content (#64650 invariant), and real user turns remain in the transcript for provenance detection. Adds 5 regression tests (repair-pass integration, alternation on multi-iteration tool turns, defrag user survival, defrag input scope, marker provenance); updates the two existing tests and the design doc to the corrected semantics. 28 tests pass.
This commit is contained in:
kshitijk4poor
kshitij
parent
9ca4ee72ca
commit
b8bfd68af1
3 changed files with 349 additions and 77 deletions
|
|
@ -5069,36 +5069,46 @@ This compaction should PRIORITISE preserving all information related to the focu
|
|||
) -> Optional[tuple[int, int]]:
|
||||
"""Find the next complete exchange starting at *start*.
|
||||
|
||||
An exchange is an assistant message plus its tool results. Returns
|
||||
``(exchange_start, exchange_end)`` indices into *messages*, or ``None``
|
||||
if no complete exchange is available before *tail_start*.
|
||||
An exchange is one full agent turn: the first assistant message after
|
||||
*start* plus everything through the end of that turn — tool results
|
||||
and any follow-up assistant iterations — up to (exclusive) the next
|
||||
``user`` message. Returns ``(exchange_start, exchange_end)`` indices
|
||||
into *messages*, or ``None`` if no complete, safely-spliceable turn is
|
||||
available before *tail_start*.
|
||||
|
||||
Tool results are consumed as a group following the assistant message
|
||||
(consecutive ``tool``-role messages). The assistant message itself must
|
||||
exist; without one there is nothing to summarise.
|
||||
The full-turn shape is an alternation-safety requirement, not a
|
||||
convenience: the splice replaces the span with a single
|
||||
``assistant``-role summary marker, so the span must be bounded by
|
||||
user messages on the right (``messages[exchange_end]`` is ``user``).
|
||||
Absorbing only the first assistant+tools group of a multi-iteration
|
||||
turn would leave the marker adjacent to the turn's next assistant
|
||||
message — two consecutive assistant turns, which strict providers
|
||||
reject and ``repair_message_sequence`` would then mangle.
|
||||
|
||||
User messages are deliberately NOT part of an exchange. The walk skips
|
||||
past them to reach the assistant message, and ``exchange_start`` is that
|
||||
assistant index, so user turns are never absorbed into the rolling
|
||||
summary and stay verbatim for the life of the session. This is the
|
||||
intended behaviour, not an oversight: what the assistant emits is
|
||||
largely an account of what it did, which survives summarising, while the
|
||||
user's own words are the instructions everything else is derived from
|
||||
and are the one thing that cannot be reconstructed from context. They
|
||||
are also cheap — a prompt is normally a tiny fraction of the tokens a
|
||||
single tool result costs.
|
||||
summary and their text stays verbatim for the life of the session.
|
||||
This is the intended behaviour, not an oversight: what the assistant
|
||||
emits is largely an account of what it did, which survives summarising,
|
||||
while the user's own words are the instructions everything else is
|
||||
derived from and are the one thing that cannot be reconstructed from
|
||||
context. They are also cheap — a prompt is normally a tiny fraction
|
||||
of the tokens a single tool result costs.
|
||||
"""
|
||||
idx = start
|
||||
n = len(messages)
|
||||
if idx >= n or idx >= tail_start:
|
||||
return None
|
||||
|
||||
# Walk past any user messages or summary markers until we hit an
|
||||
# assistant message with actual output (content or tool_calls).
|
||||
# Walk past user messages and existing summary markers until we hit a
|
||||
# real assistant message with actual output (content or tool_calls).
|
||||
# Summary markers are assistant-role themselves, so they must be
|
||||
# skipped explicitly or a rehydrated cursor could try to absorb the
|
||||
# marker that carries the compacted history.
|
||||
while idx < tail_start and idx < n:
|
||||
msg = messages[idx]
|
||||
role = msg.get("role")
|
||||
if role == "assistant":
|
||||
if msg.get("role") == "assistant" and not self._is_context_summary_message(msg):
|
||||
break
|
||||
idx += 1
|
||||
|
||||
|
|
@ -5107,18 +5117,33 @@ This compaction should PRIORITISE preserving all information related to the focu
|
|||
|
||||
exchange_start = idx
|
||||
|
||||
# Advance past the assistant message
|
||||
# Consume the full turn: assistant / tool messages until the next
|
||||
# user message (or an existing summary marker) ends the turn.
|
||||
idx += 1
|
||||
|
||||
# Consume following tool results as part of the same exchange
|
||||
while idx < tail_start and idx < n:
|
||||
if messages[idx].get("role") == "tool":
|
||||
idx += 1
|
||||
else:
|
||||
msg = messages[idx]
|
||||
role = msg.get("role")
|
||||
if role not in ("assistant", "tool"):
|
||||
break
|
||||
if self._is_context_summary_message(msg):
|
||||
break
|
||||
idx += 1
|
||||
|
||||
if idx <= exchange_start:
|
||||
return None
|
||||
|
||||
# Splice-boundary guard: the message right after the exchange must be
|
||||
# a user turn (or a summary marker, which stands in for one
|
||||
# structurally). If the walk stopped because it ran into *tail_start*
|
||||
# mid-turn, splicing here would leave the assistant-role marker
|
||||
# adjacent to the turn's remaining assistant/tool messages — invalid
|
||||
# alternation. Skip this pass; the tail recedes as the conversation
|
||||
# grows and the turn becomes absorbable later.
|
||||
if idx >= n:
|
||||
return None
|
||||
boundary = messages[idx]
|
||||
if not isinstance(boundary, dict) or boundary.get("role") != "user":
|
||||
return None
|
||||
return (exchange_start, idx)
|
||||
|
||||
def _serialize_one_exchange(
|
||||
|
|
@ -5294,25 +5319,51 @@ This compaction should PRIORITISE preserving all information related to the focu
|
|||
def _defrag_rolling_summary(
|
||||
self,
|
||||
messages: List[Dict[str, Any]],
|
||||
head_end: int,
|
||||
tail_start: int,
|
||||
) -> None:
|
||||
"""Re-summarize the rolling summary + remaining middle in one shot.
|
||||
) -> bool:
|
||||
"""Re-summarize the rolling summary TEXT and rewrite the marker in place.
|
||||
|
||||
This is a lightweight batch compaction on just the summary and the
|
||||
remaining un-compacted region \u2014 NOT the full transcript. It replaces
|
||||
the rolling summary with a fresh, compact version and advances the
|
||||
cursor to *tail_start*.
|
||||
Merging exchange after exchange makes the rolling summary baggy —
|
||||
repetitive, and larger than the material justifies. Defrag compacts
|
||||
the summary *itself*: one aux call over the accumulated summary text,
|
||||
then the existing marker's content is rewritten in place.
|
||||
|
||||
Deliberately transcript-shape-neutral: no messages are spliced, no
|
||||
user turns are touched, and the cursor does not move. The original
|
||||
implementation serialized the whole remaining middle (user turns
|
||||
included) and spliced it into the marker, which silently absorbed
|
||||
user messages — violating the feature's core "your messages are never
|
||||
compacted" invariant. Un-absorbed exchanges stay where they are and
|
||||
get absorbed by later per-exchange passes.
|
||||
|
||||
Returns True when a pass actually rewrote the summary.
|
||||
"""
|
||||
middle_content = self._serialize_one_exchange(messages, head_end, tail_start)
|
||||
fresh_summary = self._micro_summarize_one(middle_content)
|
||||
if fresh_summary:
|
||||
self._micro_compact_rolling_summary = fresh_summary
|
||||
self._micro_compact_cursor = tail_start
|
||||
logger.info(
|
||||
"Micro-compaction defrag: rolling summary re-summarized "
|
||||
"(%d chars)", len(fresh_summary),
|
||||
)
|
||||
old_summary = self._micro_compact_rolling_summary
|
||||
if not old_summary.strip():
|
||||
return False
|
||||
# Feed the old summary through the merge prompt with an empty base:
|
||||
# "merge these decisions into (no previous summary)" is exactly a
|
||||
# rewrite-compactly instruction for the accumulated text.
|
||||
self._micro_compact_rolling_summary = ""
|
||||
fresh_summary = self._micro_summarize_one(old_summary)
|
||||
if not fresh_summary:
|
||||
self._micro_compact_rolling_summary = old_summary
|
||||
return False
|
||||
self._micro_compact_rolling_summary = fresh_summary
|
||||
# Rewrite the newest marker's content in place so the transcript and
|
||||
# the in-memory summary stay in step (resume rehydrates from it).
|
||||
for idx in range(len(messages) - 1, -1, -1):
|
||||
entry = messages[idx]
|
||||
if isinstance(entry, dict) and entry.get(COMPRESSED_SUMMARY_METADATA_KEY):
|
||||
entry["content"] = self._render_micro_marker_content(fresh_summary)
|
||||
# Content changed after a possible flush — clear the persisted
|
||||
# stamp so the DB sync/flush rewrites the row.
|
||||
entry.pop(_DB_PERSISTED_MARKER, None)
|
||||
break
|
||||
logger.info(
|
||||
"Micro-compaction defrag: rolling summary re-summarized "
|
||||
"(%d -> %d chars)", len(old_summary), len(fresh_summary),
|
||||
)
|
||||
return True
|
||||
|
||||
def _micro_compact(
|
||||
self,
|
||||
|
|
@ -5379,23 +5430,26 @@ This compaction should PRIORITISE preserving all information related to the focu
|
|||
def _elapsed_ms() -> int:
|
||||
return int((time.monotonic() - _started_at) * 1000)
|
||||
|
||||
# Check for defrag trigger
|
||||
# Check for defrag trigger: the rolling summary itself has grown
|
||||
# baggy. Defrag rewrites the summary text and the existing marker in
|
||||
# place — no splice, no cursor movement, no user turns touched — so
|
||||
# the transcript shape is unchanged and this pass does not also
|
||||
# absorb an exchange (one aux call per turn either way).
|
||||
if self._needs_defrag():
|
||||
self._defrag_rolling_summary(messages, exchange_start, compress_end)
|
||||
result = self._splice_micro_compact_result(messages, exchange_start, compress_end)
|
||||
self._micro_compact_cursor = self._cursor_after_splice(result, exchange_start + 1)
|
||||
self._sync_micro_compact_to_db(result)
|
||||
self._micro_compact_consecutive_failures = 0
|
||||
self._micro_compact_last_failure_cursor = -1
|
||||
defragged = self._defrag_rolling_summary(messages)
|
||||
if defragged:
|
||||
self._sync_micro_compact_to_db(messages)
|
||||
self._micro_compact_consecutive_failures = 0
|
||||
self._micro_compact_last_failure_cursor = -1
|
||||
self._emit_micro_compaction_telemetry(
|
||||
outcome="defrag",
|
||||
outcome="defrag" if defragged else "defrag_failed",
|
||||
messages_before=_messages_before,
|
||||
messages_after=len(result),
|
||||
messages_after=len(messages),
|
||||
tokens_before=_tokens_before,
|
||||
tokens_after=estimate_messages_tokens_rough(result),
|
||||
tokens_after=estimate_messages_tokens_rough(messages),
|
||||
duration_ms=_elapsed_ms(),
|
||||
)
|
||||
return result
|
||||
return messages
|
||||
|
||||
# Whether this pass's summary will be cumulative — i.e. whether it
|
||||
# subsumes any earlier marker. Captured before summarizing.
|
||||
|
|
@ -5621,23 +5675,36 @@ This compaction should PRIORITISE preserving all information related to the focu
|
|||
``_compressed_summary`` metadata flag so downstream consumers
|
||||
(resume, handoff, /compress) handle it identically to batch
|
||||
compaction summaries.
|
||||
|
||||
Alternation safety: the marker is ``assistant``-role. An exchange is
|
||||
a full agent turn bounded by user messages on both sides (see
|
||||
``_find_one_exchange``), so the spliced result is
|
||||
``user → marker(assistant) → user`` — valid alternation that the
|
||||
pre-request ``repair_message_sequence`` pass leaves untouched. A
|
||||
``user``-role marker in that position produced ``user → user → user``,
|
||||
and repair then merged the marker into the neighbouring real user
|
||||
message: metadata gone, cursor unrecoverable, and the summary text
|
||||
duplicated into the transcript on every subsequent pass.
|
||||
|
||||
Superseding an earlier marker removes the assistant turn that stood
|
||||
between two real user messages, leaving them adjacent. Those two are
|
||||
merged (plain-text only, ``\\n\\n``-joined — the same repair pass 2
|
||||
would apply) so the transcript is alternation-valid as returned
|
||||
rather than relying on downstream repair to fix it up.
|
||||
"""
|
||||
summary_text = self._micro_compact_rolling_summary
|
||||
if not summary_text.strip():
|
||||
return messages
|
||||
|
||||
content = (
|
||||
f"{SUMMARY_PREFIX}\n\n"
|
||||
f"{HISTORICAL_TASK_HEADING}\n"
|
||||
)
|
||||
content += summary_text.strip()
|
||||
content += f"\n\n{_SUMMARY_END_MARKER}"
|
||||
|
||||
summary_msg = {
|
||||
"role": "user",
|
||||
"content": content,
|
||||
"role": "assistant",
|
||||
"content": self._render_micro_marker_content(summary_text),
|
||||
COMPRESSED_SUMMARY_METADATA_KEY: True,
|
||||
COMPRESSED_SUMMARY_HAS_USER_TURN_KEY: True,
|
||||
# Honest provenance (#64650): this marker absorbs only
|
||||
# assistant/tool content — user turns are never micro-compacted,
|
||||
# so they remain in the transcript and _transcript_has_real_user_turn
|
||||
# keeps reporting them directly.
|
||||
COMPRESSED_SUMMARY_HAS_USER_TURN_KEY: False,
|
||||
}
|
||||
|
||||
result = messages[:splice_start] + [summary_msg] + messages[splice_end:]
|
||||
|
|
@ -5661,10 +5728,63 @@ This compaction should PRIORITISE preserving all information related to the focu
|
|||
if len(marker_idxs) > 1:
|
||||
superseded = set(marker_idxs[:-1])
|
||||
result = [m for i, m in enumerate(result) if i not in superseded]
|
||||
result = self._merge_adjacent_user_turns(result)
|
||||
|
||||
_strip_persistence_markers(result)
|
||||
return result
|
||||
|
||||
@staticmethod
|
||||
def _render_micro_marker_content(summary_text: str) -> str:
|
||||
"""Assemble the marker content wrapper around *summary_text*."""
|
||||
return (
|
||||
f"{SUMMARY_PREFIX}\n\n"
|
||||
f"{HISTORICAL_TASK_HEADING}\n"
|
||||
f"{summary_text.strip()}"
|
||||
f"\n\n{_SUMMARY_END_MARKER}"
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _merge_adjacent_user_turns(
|
||||
result: List[Dict[str, Any]],
|
||||
) -> List[Dict[str, Any]]:
|
||||
"""Merge consecutive plain-text real user turns left by a supersede.
|
||||
|
||||
Dropping a superseded marker removes the assistant turn that separated
|
||||
two real user messages. Merging them here (``\\n\\n``-joined, exactly
|
||||
what ``repair_message_sequence`` pass 2 does) keeps every byte the
|
||||
user typed while restoring alternation deliberately, so the marker
|
||||
and cursor state are never collateral damage of the downstream repair.
|
||||
Multimodal (list) content is left alone, mirroring the repair pass.
|
||||
"""
|
||||
from agent.turn_context import drop_stale_api_content
|
||||
|
||||
merged: List[Dict[str, Any]] = []
|
||||
for msg in result:
|
||||
prev = merged[-1] if merged else None
|
||||
if (
|
||||
isinstance(msg, dict)
|
||||
and isinstance(prev, dict)
|
||||
and msg.get("role") == "user"
|
||||
and prev.get("role") == "user"
|
||||
and not msg.get(COMPRESSED_SUMMARY_METADATA_KEY)
|
||||
and not prev.get(COMPRESSED_SUMMARY_METADATA_KEY)
|
||||
and isinstance(prev.get("content"), str)
|
||||
and isinstance(msg.get("content"), str)
|
||||
):
|
||||
prev_content = prev["content"]
|
||||
new_content = msg["content"]
|
||||
prev["content"] = (
|
||||
(prev_content + "\n\n" + new_content)
|
||||
if prev_content and new_content
|
||||
else (prev_content or new_content)
|
||||
)
|
||||
# Merged content invalidates the api_content sidecar (exact
|
||||
# bytes previously sent for the pre-merge message).
|
||||
drop_stale_api_content(prev)
|
||||
continue
|
||||
merged.append(msg)
|
||||
return merged
|
||||
|
||||
def compress(
|
||||
self,
|
||||
messages: List[Dict[str, Any]],
|
||||
|
|
|
|||
|
|
@ -51,10 +51,14 @@ compressor to absorb **one** exchange:
|
|||
One exchange per turn. The per-turn cost stays bounded no matter how long the
|
||||
conversation gets.
|
||||
|
||||
An **exchange** is an assistant message together with any tool results that
|
||||
followed it. In tool-heavy work that's where the bulk of the tokens live — a
|
||||
An **exchange** is one full agent turn: an assistant message together with its
|
||||
tool results and any follow-up assistant iterations, up to the next user
|
||||
message. In tool-heavy work that's where the bulk of the tokens live — a
|
||||
file read or a command's output dwarfs the surrounding prose — which is why
|
||||
absorbing one exchange at a time is worth doing at all.
|
||||
absorbing one exchange at a time is worth doing at all. Taking the whole turn
|
||||
(rather than a single assistant+tools group) also keeps the transcript's role
|
||||
alternation strictly valid: the summary marker is an assistant-role message,
|
||||
and a full turn is always bounded by user messages on both sides.
|
||||
|
||||
## Your messages are never compacted
|
||||
|
||||
|
|
@ -123,12 +127,14 @@ transcript grows on every turn instead of shrinking.
|
|||
|
||||
Merge into a summary often enough and it gets baggy — repetitive, and larger
|
||||
than the material justifies. When the running summary crosses a token threshold
|
||||
(2000 by default), the next pass **defrags**: it re-summarizes the summary and
|
||||
whatever middle remains in one shot, replacing it with a fresh compact version
|
||||
and advancing the cursor to the tail.
|
||||
(2000 by default), the next pass **defrags**: one auxiliary call re-summarizes
|
||||
the running summary *itself* into a fresh compact version, and the summary
|
||||
marker in the transcript is rewritten in place.
|
||||
|
||||
This is still much cheaper than full batch compaction. It only ever processes the
|
||||
summary plus the un-absorbed middle, never the whole transcript.
|
||||
Defrag never touches the transcript's structure — no messages are absorbed or
|
||||
spliced, the cursor doesn't move, and user turns are untouched. It processes
|
||||
only the accumulated summary text, never conversation messages, so the
|
||||
"your messages are never compacted" guarantee holds through it.
|
||||
|
||||
### Staying in step with the session database
|
||||
|
||||
|
|
@ -152,7 +158,7 @@ untouched and the failure is counted.
|
|||
If the *same* exchange fails three times in a row, the cursor is advanced past it
|
||||
anyway. Without that, one bad exchange would be retried on every single turn
|
||||
forever. Those skipped messages stay in the transcript and get picked up by the
|
||||
next defrag or batch compaction.
|
||||
next batch compaction.
|
||||
|
||||
## Interaction with batch compaction
|
||||
|
||||
|
|
|
|||
|
|
@ -70,8 +70,12 @@ class TestMicroCompaction:
|
|||
markers = _summary_markers(result)
|
||||
assert len(markers) == 1
|
||||
assert "ROLLING SUMMARY" in markers[0]["content"]
|
||||
# The marker stands in for a user turn, like batch compaction's does.
|
||||
assert markers[0]["role"] == "user"
|
||||
# The marker is assistant-role: an exchange is a full agent turn
|
||||
# bounded by user messages, so user → marker(assistant) → user keeps
|
||||
# strict alternation. A user-role marker produced user → user → user,
|
||||
# and the pre-request repair_message_sequence pass merged the marker
|
||||
# into the neighbouring real user turn — metadata gone, cursor lost.
|
||||
assert markers[0]["role"] == "assistant"
|
||||
|
||||
def test_disabled_is_a_no_op(self):
|
||||
cc = _compressor()
|
||||
|
|
@ -168,12 +172,17 @@ class TestMicroCompaction:
|
|||
assert result[-1] == messages[-1], "most recent turn must be preserved"
|
||||
|
||||
def test_user_messages_are_never_absorbed(self):
|
||||
"""User turns stay verbatim for the life of the session — by design.
|
||||
"""Every byte the user typed stays in the transcript — by design.
|
||||
|
||||
Assistant output is largely an account of what was done and survives
|
||||
summarising; the user's own words are the intent everything else is
|
||||
derived from and can't be reconstructed from it. So an exchange starts
|
||||
at the assistant message and the walk skips past user turns.
|
||||
|
||||
The invariant is on user TEXT, not message-list shape: superseding an
|
||||
old marker leaves two real user turns adjacent, and they are merged
|
||||
(\\n\\n-joined, same as repair_message_sequence pass 2) to keep strict
|
||||
alternation. Text is never summarized or dropped.
|
||||
"""
|
||||
cc = _compressor()
|
||||
messages = _conversation(exchanges=10)
|
||||
|
|
@ -182,11 +191,14 @@ class TestMicroCompaction:
|
|||
for _ in range(5):
|
||||
messages = cc._micro_compact(messages)
|
||||
|
||||
surviving = [
|
||||
surviving_text = "\n\n".join(
|
||||
m["content"] for m in messages
|
||||
if m.get("role") == "user" and not m.get(COMPRESSED_SUMMARY_METADATA_KEY)
|
||||
]
|
||||
assert surviving == originals, "user turns must survive verbatim"
|
||||
)
|
||||
for original in originals:
|
||||
assert original in surviving_text, (
|
||||
f"user text {original!r} must survive verbatim"
|
||||
)
|
||||
|
||||
def test_cursor_is_derived_from_the_spliced_list(self):
|
||||
"""The cursor must never carry over a pre-splice index.
|
||||
|
|
@ -474,9 +486,20 @@ class TestMicroCompaction:
|
|||
assert cc._micro_compact_tokens_saved_total > 0
|
||||
|
||||
def test_defrag_triggers_once_the_rolling_summary_grows(self):
|
||||
"""Defrag rewrites the summary text and the marker — nothing else.
|
||||
|
||||
The original implementation spliced the whole remaining middle (user
|
||||
turns included) into the marker, silently absorbing user messages.
|
||||
Defrag is now transcript-shape-neutral: same message list, same
|
||||
cursor, marker content rewritten in place.
|
||||
"""
|
||||
cc = _compressor(summary="FRESH DEFRAGGED SUMMARY")
|
||||
cc._micro_compact_rolling_summary = "x" * 40_000 # far over the threshold
|
||||
messages = _conversation(exchanges=8)
|
||||
# Seed a real marker + oversized rolling summary, as after many passes.
|
||||
messages = cc._micro_compact(list(messages))
|
||||
cc._micro_compact_rolling_summary = "x" * 40_000 # far over the threshold
|
||||
cursor_before = cc._micro_compact_cursor
|
||||
shape_before = [m.get("role") for m in messages]
|
||||
|
||||
assert cc._needs_defrag() is True
|
||||
result = cc._micro_compact(list(messages))
|
||||
|
|
@ -485,3 +508,126 @@ class TestMicroCompaction:
|
|||
markers = _summary_markers(result)
|
||||
assert len(markers) == 1
|
||||
assert "FRESH DEFRAGGED SUMMARY" in markers[0]["content"]
|
||||
# Shape-neutral: no messages absorbed or spliced, cursor unmoved.
|
||||
assert [m.get("role") for m in result] == shape_before
|
||||
assert cc._micro_compact_cursor == cursor_before
|
||||
|
||||
def test_defrag_never_absorbs_user_messages(self):
|
||||
"""Defrag must not touch user turns — the feature's core invariant.
|
||||
|
||||
The original implementation serialized head..tail (user turns
|
||||
included) and spliced it away: 8 of 10 user prompts were destroyed in
|
||||
one pass. Defrag now only rewrites the rolling summary text.
|
||||
"""
|
||||
cc = _compressor(summary="DEFRAGGED")
|
||||
messages = [{"role": "system", "content": "sys"}]
|
||||
for i in range(10):
|
||||
messages.append({"role": "user", "content": f"UNIQUE-USER-PROMPT-{i}"})
|
||||
messages.append({"role": "assistant", "content": f"answer {i} " + "z" * 400})
|
||||
|
||||
cc._micro_compact_rolling_summary = "x" * 40_000 # force defrag
|
||||
result = cc._micro_compact(list(messages))
|
||||
|
||||
surviving = [
|
||||
m["content"] for m in result
|
||||
if m.get("role") == "user" and not m.get(COMPRESSED_SUMMARY_METADATA_KEY)
|
||||
]
|
||||
for i in range(10):
|
||||
assert any(f"UNIQUE-USER-PROMPT-{i}" in s for s in surviving), (
|
||||
f"user prompt {i} was absorbed by defrag"
|
||||
)
|
||||
|
||||
def test_defrag_summarizes_only_the_summary_text(self):
|
||||
"""The defrag aux call receives the rolling summary, not the transcript."""
|
||||
cc = _compressor()
|
||||
captured = {}
|
||||
|
||||
def capture(text):
|
||||
captured["text"] = text
|
||||
return "DEFRAGGED"
|
||||
|
||||
cc._micro_summarize_one = capture
|
||||
cc._micro_compact_rolling_summary = "OLD-SUMMARY " + "x" * 40_000
|
||||
messages = _conversation(exchanges=8)
|
||||
cc._micro_compact(list(messages))
|
||||
|
||||
assert "OLD-SUMMARY" in captured["text"]
|
||||
assert "[USER]" not in captured["text"], (
|
||||
"defrag must never serialize transcript user turns"
|
||||
)
|
||||
|
||||
def test_spliced_transcript_survives_repair_message_sequence(self):
|
||||
"""The compacted transcript must survive the production repair pass.
|
||||
|
||||
conversation_loop runs repair_message_sequence before EVERY API call.
|
||||
With the old user-role marker, splicing next to a real user turn made
|
||||
user → user → user; repair merged the marker into the real user
|
||||
message — metadata gone, cursor unrecoverable, summary text duplicated
|
||||
into the transcript on every later pass. Pin the integration: markers
|
||||
survive repair untouched, and no summary text leaks into real user
|
||||
messages.
|
||||
"""
|
||||
from agent.agent_runtime_helpers import repair_message_sequence
|
||||
|
||||
class _DummyAgent:
|
||||
session_id = "probe"
|
||||
_last_flushed_db_idx = 0
|
||||
|
||||
cc = _compressor()
|
||||
messages = _conversation(exchanges=8)
|
||||
|
||||
for _ in range(3):
|
||||
messages = cc._micro_compact(messages)
|
||||
repairs = repair_message_sequence(_DummyAgent(), messages)
|
||||
assert repairs == 0, (
|
||||
"micro-compacted transcript must already be alternation-valid"
|
||||
)
|
||||
markers = _summary_markers(messages)
|
||||
assert len(markers) == 1, "marker destroyed by repair pass"
|
||||
polluted = [
|
||||
m for m in messages
|
||||
if m.get("role") == "user"
|
||||
and not m.get(COMPRESSED_SUMMARY_METADATA_KEY)
|
||||
and "ROLLING SUMMARY" in str(m.get("content"))
|
||||
]
|
||||
assert not polluted, "summary text leaked into a real user message"
|
||||
|
||||
def test_spliced_transcript_has_no_consecutive_same_role_messages(self):
|
||||
"""Alternation invariant, checked directly on tool-bearing turns."""
|
||||
cc = _compressor()
|
||||
msgs = [{"role": "system", "content": "sys"}]
|
||||
for i in range(8):
|
||||
msgs.append({"role": "user", "content": f"q{i}"})
|
||||
msgs.append({
|
||||
"role": "assistant",
|
||||
"content": f"a{i}",
|
||||
"tool_calls": [
|
||||
{"id": f"c{i}-{j}", "type": "function",
|
||||
"function": {"name": "f", "arguments": "{}"}}
|
||||
for j in range(2)
|
||||
],
|
||||
})
|
||||
for j in range(2):
|
||||
msgs.append({"role": "tool", "tool_call_id": f"c{i}-{j}",
|
||||
"content": "T" * 400})
|
||||
# Multi-iteration turn: a second assistant+tools group before the
|
||||
# next user message — the splice must absorb the WHOLE turn.
|
||||
msgs.append({"role": "assistant", "content": f"followup {i} " + "y" * 200})
|
||||
|
||||
for _ in range(4):
|
||||
msgs = cc._micro_compact(msgs)
|
||||
for a, b in zip(msgs, msgs[1:]):
|
||||
ra, rb = a.get("role"), b.get("role")
|
||||
assert not (ra == rb and ra in ("user", "assistant")), (
|
||||
f"consecutive {ra} messages after micro-compaction"
|
||||
)
|
||||
|
||||
def test_marker_reports_no_user_provenance(self):
|
||||
"""Micro markers absorb only assistant/tool content (#64650)."""
|
||||
from agent.context_compressor import COMPRESSED_SUMMARY_HAS_USER_TURN_KEY
|
||||
|
||||
cc = _compressor()
|
||||
result = cc._micro_compact(_conversation(exchanges=6))
|
||||
marker = _summary_markers(result)[0]
|
||||
|
||||
assert marker[COMPRESSED_SUMMARY_HAS_USER_TURN_KEY] is False
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue