From 1f6718b0ffda673475818fc2ed2005fb385e9fae Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 9 Sep 2026 11:38:52 +0530 Subject: [PATCH] test(agent): prove the re-anchor through prepare_iteration; reuse the compaction _reanchor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- agent/turn_iteration_prep.py | 44 ++++------ .../test_export_current_turn_boundary.py | 71 ++++++---------- ..._repair_reanchors_current_turn_user_idx.py | 85 +++++++++---------- 3 files changed, 79 insertions(+), 121 deletions(-) diff --git a/agent/turn_iteration_prep.py b/agent/turn_iteration_prep.py index 70c126512f..a48299ea3d 100644 --- a/agent/turn_iteration_prep.py +++ b/agent/turn_iteration_prep.py @@ -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: diff --git a/tests/run_agent/test_export_current_turn_boundary.py b/tests/run_agent/test_export_current_turn_boundary.py index 65f07accd2..117dfb70ea 100644 --- a/tests/run_agent/test_export_current_turn_boundary.py +++ b/tests/run_agent/test_export_current_turn_boundary.py @@ -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() diff --git a/tests/run_agent/test_repair_reanchors_current_turn_user_idx.py b/tests/run_agent/test_repair_reanchors_current_turn_user_idx.py index a307794117..2d1aa24db4 100644 --- a/tests/run_agent/test_repair_reanchors_current_turn_user_idx.py +++ b/tests/run_agent/test_repair_reanchors_current_turn_user_idx.py @@ -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()