From fc635377bc6f1d07e4f06cab63494e36b1c34c1a Mon Sep 17 00:00:00 2001 From: chengyongru <2755839590@qq.com> Date: Mon, 15 Jun 2026 22:51:07 +0800 Subject: [PATCH] fix: avoid replaying older long turns Treat the current live user message as the replay boundary for normal user turns, while keeping user-turn extension for history and consolidation paths that need it. Add regression coverage for the user-triggered long tool-turn case. --- nanobot/agent/loop.py | 6 +-- nanobot/utils/helpers.py | 4 +- tests/agent/test_consolidator.py | 32 ++++++++++++ tests/agent/test_max_messages_config.py | 55 ++++++++++++++++++++- tests/agent/test_session_manager_history.py | 17 +++++++ 5 files changed, 108 insertions(+), 6 deletions(-) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 7b281df1..059770b1 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -1168,14 +1168,14 @@ class AgentLoop: channel, chat_id, msg.metadata.get("message_id"), msg.metadata, session_key=key, ) + current_role = "assistant" if is_subagent else "user" _hist_kwargs: dict[str, Any] = { "max_messages": self._max_messages, "max_tokens": self._replay_token_budget(), "include_timestamps": True, - "extend_to_user": True, + "extend_to_user": is_subagent, } history = session.get_history(**_hist_kwargs) - current_role = "assistant" if is_subagent else "user" workspace_scope = self.workspace_scopes.for_message(msg, session.metadata) messages = self.context.build_messages( @@ -1448,7 +1448,7 @@ class AgentLoop: "max_messages": self._max_messages, "max_tokens": self._replay_token_budget(), "include_timestamps": True, - "extend_to_user": True, + "extend_to_user": False, } ctx.history = ctx.session.get_history(**_hist_kwargs) self._runtime_events().record_turn_runtime( diff --git a/nanobot/utils/helpers.py b/nanobot/utils/helpers.py index 34177bef..a1ec6076 100644 --- a/nanobot/utils/helpers.py +++ b/nanobot/utils/helpers.py @@ -282,9 +282,11 @@ def recent_message_start_index( start_idx = max(0, len(messages) - max_messages) if not extend_to_user or len(messages) <= max_messages: return start_idx + if any(messages[i].get("role") == "user" for i in range(start_idx, len(messages))): + return start_idx recovered_user = next( - (i for i in range(start_idx, -1, -1) if messages[i].get("role") == "user"), + (i for i in range(start_idx - 1, -1, -1) if messages[i].get("role") == "user"), None, ) if recovered_user is None: diff --git a/tests/agent/test_consolidator.py b/tests/agent/test_consolidator.py index da79f600..a95b9638 100644 --- a/tests/agent/test_consolidator.py +++ b/tests/agent/test_consolidator.py @@ -263,6 +263,38 @@ class TestConsolidatorTokenBudget: assert history[0]["content"] == "record this" assert history[-1]["content"] == "final answer" + async def test_replay_window_overflow_uses_newer_user_inside_window( + self, + consolidator, + ): + """Do not extend to an older long turn when the hard window has a newer user.""" + session = Session(key="test:replay-newer-user") + session.add_message("user", "old") + session.add_message("assistant", "old answer") + session.add_message("user", "long older turn") + for i in range(8): + session.messages.extend(_tool_round(f"older-{i}")) + session.add_message("assistant", "older final") + session.add_message("user", "new question") + session.add_message("assistant", "new answer") + + consolidator.sessions._session_cache[session.key] = session + consolidator.estimate_session_prompt_tokens = MagicMock(return_value=(100, "tiktoken")) + consolidator.archive = AsyncMock(return_value="older turn summary") + + await consolidator.maybe_consolidate_by_tokens( + session, + replay_max_messages=6, + ) + + archived_chunk = consolidator.archive.await_args.args[0] + assert archived_chunk[2]["content"] == "long older turn" + assert archived_chunk[-1]["content"] == "older final" + assert session.last_consolidated == len(session.messages) - 2 + + history = session.get_history(max_messages=6, extend_to_user=True) + assert [m["content"] for m in history] == ["new question", "new answer"] + async def test_large_chunk_archived_without_cap(self, consolidator): """Without chunk cap, the full range from pick_consolidation_boundary is archived.""" consolidator._SAFETY_BUFFER = 0 diff --git a/tests/agent/test_max_messages_config.py b/tests/agent/test_max_messages_config.py index 2a598ce4..e7adccec 100644 --- a/tests/agent/test_max_messages_config.py +++ b/tests/agent/test_max_messages_config.py @@ -37,6 +37,19 @@ def _populated_session(n: int) -> Session: return session +def _tool_round(call_id: str) -> list[dict]: + return [ + { + "role": "assistant", + "content": None, + "tool_calls": [ + {"id": call_id, "type": "function", "function": {"name": "x", "arguments": "{}"}} + ], + }, + {"role": "tool", "tool_call_id": call_id, "name": "x", "content": "ok"}, + ] + + class TestMaxMessagesInit: """Verify AgentLoop stores the config value correctly.""" @@ -111,7 +124,7 @@ class TestMaxMessagesIntegration: assert result is not None assert mock_hist.call_count == 1 assert mock_hist.call_args.kwargs["max_messages"] == 25 - assert mock_hist.call_args.kwargs["extend_to_user"] is True + assert mock_hist.call_args.kwargs["extend_to_user"] is False @pytest.mark.asyncio async def test_zero_config_passes_builtin_limit_to_history_call(self, tmp_path: Path) -> None: @@ -130,7 +143,45 @@ class TestMaxMessagesIntegration: assert result is not None assert mock_hist.call_args.kwargs["max_messages"] == DEFAULT_MAX_MESSAGES - assert mock_hist.call_args.kwargs["extend_to_user"] is True + assert mock_hist.call_args.kwargs["extend_to_user"] is False + + @pytest.mark.asyncio + async def test_process_message_uses_current_user_as_replay_boundary( + self, + tmp_path: Path, + ) -> None: + """A live user turn should not extend history to an older long tool turn.""" + loop = _make_loop(tmp_path, max_messages=6) + loop.provider.chat_with_retry = AsyncMock( + return_value=LLMResponse(content="ok", tool_calls=[], usage={}) + ) + loop.tools.get_definitions = MagicMock(return_value=[]) + loop.consolidator.maybe_consolidate_by_tokens = AsyncMock(return_value=False) # type: ignore[method-assign] + + session = loop.sessions.get_or_create("cli:test") + session.add_message("user", "old") + session.add_message("assistant", "old answer") + session.add_message("user", "long older turn") + for i in range(8): + session.messages.extend(_tool_round(f"older-{i}")) + session.add_message("assistant", "older final") + + with patch.object(session, "get_history", wraps=session.get_history) as mock_hist: + result = await loop._process_message( + InboundMessage( + channel="cli", + sender_id="user", + chat_id="test", + content="new question", + ) + ) + + assert result is not None + assert mock_hist.call_args.kwargs["extend_to_user"] is False + sent_messages = loop.provider.chat_with_retry.await_args.kwargs["messages"] + sent_text = "\n".join(str(message.get("content")) for message in sent_messages) + assert "new question" in sent_text + assert "long older turn" not in sent_text class TestSchemaConfig: diff --git a/tests/agent/test_session_manager_history.py b/tests/agent/test_session_manager_history.py index 4b8fbbdf..dc9c648a 100644 --- a/tests/agent/test_session_manager_history.py +++ b/tests/agent/test_session_manager_history.py @@ -660,6 +660,23 @@ def test_get_history_can_extend_to_user_for_long_recent_turn(): _assert_no_orphans(extended) +def test_get_history_extend_to_user_keeps_newer_user_inside_window(): + session = Session(key="test:history-extend-newer-user") + session.messages.append({"role": "user", "content": "old"}) + session.messages.append({"role": "assistant", "content": "old answer"}) + session.messages.append({"role": "user", "content": "long older turn"}) + for i in range(8): + session.messages.extend(_tool_turn("older", i)) + session.messages.append({"role": "assistant", "content": "older final"}) + session.messages.append({"role": "user", "content": "new question"}) + session.messages.append({"role": "assistant", "content": "new answer"}) + + history = session.get_history(max_messages=6, extend_to_user=True) + + assert [m["content"] for m in history] == ["new question", "new answer"] + _assert_no_orphans(history) + + # --- enforce_file_cap archive correctness (issue #4128) ---