test(agent): prove the re-anchor through prepare_iteration; reuse the compaction _reanchor
The salvaged regression test exercised only repair_message_sequence and reanchor_current_turn_user_idx — pre-existing helpers — so reverting the fix left it green. It now drives prepare_iteration on a real AIAgent with adjacent user rows and asserts the returned index addresses this turn's row and mirrors into _persist_user_message_idx (red without the re-anchor: IndexError). Both re-anchor sites (repair and compression restart) call turn_context_compaction._reanchor instead of inlining "reanchor + mirror", so they cannot drift. The export tests fold into one parametrized invariant plus the run_conversation envelope test; the WHAT-restating comment shrinks to the WHY.
This commit is contained in:
@@ -17,7 +17,7 @@ from dataclasses import dataclass
|
||||
from typing import Any, Dict
|
||||
|
||||
from agent.display import KawaiiSpinner
|
||||
from agent.turn_context import reanchor_current_turn_user_idx
|
||||
from agent.turn_context_compaction import _reanchor
|
||||
|
||||
logger = logging.getLogger("agent.conversation_loop")
|
||||
|
||||
@@ -185,32 +185,19 @@ def prepare_iteration(
|
||||
repaired_seq,
|
||||
agent.session_id or "-",
|
||||
)
|
||||
# The repair merges adjacent user rows in place (after a compaction the role=user
|
||||
# summary sits next to the protected first user message), so the list shrinks and
|
||||
# the index recorded at turn start points past this turn's user row. Re-anchor it
|
||||
# like the compression restart path does, and mirror the value into
|
||||
# _persist_user_message_idx, which hosts read when the result carries no index:
|
||||
# a stale anchor injects prefetch into a historical row and makes hosts that settle
|
||||
# their transcript by this index (hermes-webui) write the current turn to the FRONT
|
||||
# of the context, rewriting the prompt's leading messages every turn.
|
||||
# reanchor_current_turn_user_idx scans from the tail: with two identical user
|
||||
# rows it resolves to the LAST one, i.e. this turn, never the historical copy.
|
||||
# Without the message text the index cannot be re-derived; leave it (an
|
||||
# out-of-range index is detectably stale, a clamped one would silently target
|
||||
# a wrong row).
|
||||
_reanchored_idx = (
|
||||
reanchor_current_turn_user_idx(messages, user_message) if user_message is not None
|
||||
else current_turn_user_idx
|
||||
)
|
||||
if _reanchored_idx != current_turn_user_idx:
|
||||
request_logger.info(
|
||||
"Re-anchored current_turn_user_idx %s -> %s after alternation repair (session=%s)",
|
||||
current_turn_user_idx,
|
||||
_reanchored_idx,
|
||||
agent.session_id or "-",
|
||||
)
|
||||
current_turn_user_idx = _reanchored_idx
|
||||
agent._persist_user_message_idx = _reanchored_idx
|
||||
# The merge shrank the list, so the index recorded at turn start can point past this
|
||||
# turn's user row: prefetch would inject into a historical row and index-settling hosts
|
||||
# (hermes-webui) would write the current turn to the FRONT of the context. Re-anchor as
|
||||
# the compression-restart path does (last verbatim row wins, never a historical copy);
|
||||
# without the text the index cannot be re-derived and is left detectably stale.
|
||||
if user_message is not None:
|
||||
_reanchored_idx = _reanchor(agent, messages, user_message)
|
||||
if _reanchored_idx != current_turn_user_idx:
|
||||
request_logger.info(
|
||||
"Re-anchored current_turn_user_idx %s -> %s after alternation repair (session=%s)",
|
||||
current_turn_user_idx, _reanchored_idx, agent.session_id or "-",
|
||||
)
|
||||
current_turn_user_idx = _reanchored_idx
|
||||
return IterationPrep(
|
||||
action="fallthrough", messages=messages, request_logger=request_logger,
|
||||
current_turn_user_idx=current_turn_user_idx,
|
||||
@@ -470,8 +457,7 @@ def apply_retry_restarts(
|
||||
# In-loop compression rebuilt `messages`; re-anchor the current-turn index
|
||||
# like the prologue, AFTER the handoff guard (it may re-append this turn's
|
||||
# ask). A stale anchor injects prefetch into a historical row.
|
||||
current_turn_user_idx = reanchor_current_turn_user_idx(messages, user_message)
|
||||
agent._persist_user_message_idx = current_turn_user_idx
|
||||
current_turn_user_idx = _reanchor(agent, messages, user_message)
|
||||
return _verdict("continue")
|
||||
|
||||
if _retry.restart_with_rebuilt_messages:
|
||||
|
||||
@@ -24,54 +24,31 @@ class _Agent:
|
||||
self._persist_user_message_idx = None
|
||||
|
||||
|
||||
def test_repeated_prompt_resolves_to_the_last_verbatim_row():
|
||||
messages = [
|
||||
{"role": "user", "content": "same question"},
|
||||
{"role": "assistant", "content": "old answer"},
|
||||
{"role": "user", "content": "same question"},
|
||||
{"role": "assistant", "content": "new answer"},
|
||||
]
|
||||
@pytest.mark.parametrize("user_message, messages, expected_idx", [
|
||||
# a repeated prompt resolves to the LAST verbatim row, never the historical copy
|
||||
("same question", [
|
||||
{"role": "user", "content": "same question"}, {"role": "assistant", "content": "old answer"},
|
||||
{"role": "user", "content": "same question"}, {"role": "assistant", "content": "new answer"},
|
||||
], 2),
|
||||
# multimodal content matches by structural equality
|
||||
([{"type": "text", "text": "look"}, {"type": "image_url", "image_url": {"url": "data:x"}}],
|
||||
[{"role": "user", "content": [{"type": "text", "text": "look"}, {"type": "image_url", "image_url": {"url": "data:x"}}]},
|
||||
{"role": "assistant", "content": "ok"}], 0),
|
||||
# a row the repair rewrote (merge-into-tail) is not a proven boundary: export nothing
|
||||
("same question", [{"role": "user", "content": "summary\n\nsame question"}, {"role": "assistant", "content": "answer"}], None),
|
||||
# no current row at all: export nothing, persist override untouched
|
||||
("another question", [{"role": "user", "content": "same question"}, {"role": "assistant", "content": "old answer"}], None),
|
||||
])
|
||||
def test_boundary_is_exported_only_for_the_verbatim_current_row(user_message, messages, expected_idx):
|
||||
agent = _Agent()
|
||||
result = export_current_turn_boundary(agent, {"messages": messages}, "same question")
|
||||
assert result["current_turn_user_idx"] == 2
|
||||
assert result["turn_id"] == agent._current_turn_id
|
||||
assert agent._persist_user_message_idx == 2
|
||||
|
||||
|
||||
def test_missing_current_row_exports_nothing():
|
||||
messages = [
|
||||
{"role": "user", "content": "same question"},
|
||||
{"role": "assistant", "content": "old answer"},
|
||||
]
|
||||
agent = _Agent()
|
||||
result = export_current_turn_boundary(agent, {"messages": messages}, "another question")
|
||||
assert "current_turn_user_idx" not in result and "turn_id" not in result
|
||||
assert agent._persist_user_message_idx is None
|
||||
|
||||
|
||||
def test_rewritten_row_is_not_a_proven_boundary():
|
||||
# merge-into-tail rewrote the surviving row's content: reanchor would fall back to it,
|
||||
# but the export refuses to claim a row that is not the verbatim message.
|
||||
messages = [
|
||||
{"role": "user", "content": "summary\n\nsame question"},
|
||||
{"role": "assistant", "content": "answer"},
|
||||
]
|
||||
result = export_current_turn_boundary(_Agent(), {"messages": messages}, "same question")
|
||||
assert "current_turn_user_idx" not in result
|
||||
|
||||
|
||||
def test_multimodal_content_is_matched_verbatim():
|
||||
content = [{"type": "text", "text": "look"}, {"type": "image_url", "image_url": {"url": "data:x"}}]
|
||||
messages = [{"role": "user", "content": content}, {"role": "assistant", "content": "ok"}]
|
||||
result = export_current_turn_boundary(_Agent(), {"messages": messages}, content)
|
||||
assert result["current_turn_user_idx"] == 0
|
||||
|
||||
|
||||
def test_no_turn_id_or_non_dict_result_is_left_alone():
|
||||
assert export_current_turn_boundary(_Agent(turn_id=""), {"messages": [{"role": "user", "content": "q"}]}, "q") == {
|
||||
"messages": [{"role": "user", "content": "q"}]
|
||||
}
|
||||
assert export_current_turn_boundary(_Agent(), None, "q") is None
|
||||
result = export_current_turn_boundary(agent, {"messages": messages}, user_message)
|
||||
if expected_idx is None:
|
||||
assert "current_turn_user_idx" not in result and "turn_id" not in result
|
||||
assert agent._persist_user_message_idx is None
|
||||
else:
|
||||
assert result["current_turn_user_idx"] == expected_idx
|
||||
assert result["turn_id"] == agent._current_turn_id
|
||||
assert agent._persist_user_message_idx == expected_idx
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
|
||||
@@ -1,50 +1,45 @@
|
||||
"""Regression: ``repair_message_sequence`` merges adjacent rows *before* the current
|
||||
turn's user message (a role=user compaction summary next to the protected first
|
||||
user message), so the index recorded at turn start drifts past the current row.
|
||||
Hosts that settle the transcript by that index (WebUI) then write the current
|
||||
user turn to the FRONT of the context. ``run_conversation`` must re-anchor the
|
||||
index after a repair that changed the list.
|
||||
"""
|
||||
from agent.agent_runtime_helpers import repair_message_sequence
|
||||
from agent.turn_context import reanchor_current_turn_user_idx
|
||||
"""``prepare_iteration`` runs the alternation repair, which merges adjacent user rows in place
|
||||
(after a compaction the role=user summary sits next to the protected first user message). The
|
||||
index recorded at turn start then points past this turn's user row; hosts that settle the
|
||||
transcript by that index (WebUI) write the current turn to the FRONT of the context. The
|
||||
iteration prep must hand back a re-anchored index and mirror it into the persist override."""
|
||||
|
||||
|
||||
class _Agent:
|
||||
session_id = "s"
|
||||
def _agent(tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
from run_agent import AIAgent
|
||||
from hermes_state import SessionDB
|
||||
|
||||
return AIAgent(session_db=SessionDB(db_path=tmp_path / "proof.db"),
|
||||
model="test-model", provider="openai-compat", api_key="test",
|
||||
base_url="http://127.0.0.1:1/v1", max_iterations=4,
|
||||
quiet_mode=True, skip_context_files=True, skip_memory=True)
|
||||
|
||||
|
||||
def _history_with_adjacent_users():
|
||||
return [
|
||||
{"role": "assistant", "content": "**Context snapshot**"},
|
||||
{"role": "user", "content": "compaction summary written as a user row"},
|
||||
{"role": "user", "content": "first protected user message"},
|
||||
{"role": "assistant", "content": "ok",
|
||||
"tool_calls": [{"id": "c1", "type": "function", "function": {"name": "t", "arguments": "{}"}}]},
|
||||
{"role": "tool", "tool_call_id": "c1", "content": "out"},
|
||||
{"role": "user", "content": "NEW question"},
|
||||
]
|
||||
def test_prepare_iteration_reanchors_after_the_repair_merges_rows(tmp_path, monkeypatch):
|
||||
from agent.turn_context import _reset_per_turn_agent_state
|
||||
from agent.turn_iteration_prep import prepare_iteration
|
||||
|
||||
|
||||
def test_repair_shifts_recorded_index_and_reanchor_recovers_it():
|
||||
messages = _history_with_adjacent_users()
|
||||
recorded_idx = len(messages) - 1 # what run_conversation records at turn start
|
||||
assert messages[recorded_idx]["content"] == "NEW question"
|
||||
repairs = repair_message_sequence(_Agent(), messages)
|
||||
assert repairs >= 1
|
||||
# the recorded index no longer addresses the current user row
|
||||
assert recorded_idx >= len(messages) or messages[recorded_idx]["content"] != "NEW question"
|
||||
reanchored = reanchor_current_turn_user_idx(messages, "NEW question")
|
||||
assert messages[reanchored]["role"] == "user"
|
||||
assert messages[reanchored]["content"] == "NEW question"
|
||||
assert reanchored == len(messages) - 1
|
||||
|
||||
|
||||
def test_repair_without_changes_keeps_index():
|
||||
messages = [
|
||||
{"role": "user", "content": "hello"},
|
||||
{"role": "assistant", "content": "hi"},
|
||||
{"role": "user", "content": "NEW question"},
|
||||
]
|
||||
recorded_idx = len(messages) - 1
|
||||
assert repair_message_sequence(_Agent(), messages) == 0
|
||||
assert reanchor_current_turn_user_idx(messages, "NEW question") == recorded_idx
|
||||
agent = _agent(tmp_path, monkeypatch)
|
||||
try:
|
||||
_reset_per_turn_agent_state(agent)
|
||||
messages = [
|
||||
{"role": "assistant", "content": "**Context snapshot**"},
|
||||
{"role": "user", "content": "compaction summary written as a user row"},
|
||||
{"role": "user", "content": "first protected user message"},
|
||||
{"role": "assistant", "content": "ok",
|
||||
"tool_calls": [{"id": "c1", "type": "function", "function": {"name": "t", "arguments": "{}"}}]},
|
||||
{"role": "tool", "tool_call_id": "c1", "content": "out"},
|
||||
{"role": "user", "content": "NEW question"},
|
||||
]
|
||||
recorded_idx = len(messages) - 1 # what run_conversation records at turn start
|
||||
prep = prepare_iteration(
|
||||
agent, messages=messages, api_call_count=1,
|
||||
user_message="NEW question", current_turn_user_idx=recorded_idx,
|
||||
)
|
||||
assert prep.action == "fallthrough"
|
||||
assert len(prep.messages) < len(messages) + 1 and recorded_idx >= len(prep.messages)
|
||||
assert prep.messages[prep.current_turn_user_idx]["content"] == "NEW question"
|
||||
assert agent._persist_user_message_idx == prep.current_turn_user_idx
|
||||
finally:
|
||||
agent._session_db.close()
|
||||
|
||||
Reference in New Issue
Block a user