From 5692f7a68ab991d8679136944ef295bae95b0dba Mon Sep 17 00:00:00 2001 From: axelray-dev <110029405+axelray-dev@users.noreply.github.com> Date: Sun, 28 Jun 2026 13:44:19 +0800 Subject: [PATCH] refactor(session): return RetentionResult instead of bare tuple Replace the tuple(list[dict], int) return of Session.retain_recent_legal_suffix with a named RetentionResult dataclass that exposes retained, dropped, already_consolidated_count, and new_last_consolidated fields. The tuple return was easy to misuse because the second value only made sense relative to the first and the old last_consolidated cursor. The named fields make the archive-skip semantics explicit at every call site. No behavior change. All existing tests pass unchanged in semantics. Refs #4136 Signed-off-by: axelray-dev <110029405+axelray-dev@users.noreply.github.com> --- nanobot/agent/memory.py | 4 +- nanobot/session/manager.py | 46 +++++++++++++++------ tests/agent/test_auto_compact.py | 4 +- tests/agent/test_session_manager_history.py | 24 +++++------ 4 files changed, 49 insertions(+), 29 deletions(-) diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index 94916e2f..df4ac140 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -1018,9 +1018,9 @@ class Consolidator: metadata={}, last_consolidated=0, ) - dropped, already_consolidated = probe.retain_recent_legal_suffix(max_suffix, extend_to_user=True) + result = probe.retain_recent_legal_suffix(max_suffix, extend_to_user=True) messages_to_keep = probe.messages - messages_to_remove = dropped[already_consolidated:] + messages_to_remove = result.dropped[result.already_consolidated_count:] if not messages_to_remove and not messages_to_keep: session.updated_at = datetime.now() diff --git a/nanobot/session/manager.py b/nanobot/session/manager.py index fc90fea0..b6e80c5c 100644 --- a/nanobot/session/manager.py +++ b/nanobot/session/manager.py @@ -110,6 +110,14 @@ def _metadata_title(metadata: Any) -> str: return strip_think(title) +@dataclass +class RetentionResult: + retained: list[dict] + dropped: list[dict] + already_consolidated_count: int + new_last_consolidated: int + + @dataclass class Session: """A conversation session.""" @@ -289,22 +297,29 @@ class Session: max_messages: int, *, extend_to_user: bool = False, - ) -> tuple[list[dict], int]: + ) -> RetentionResult: """Keep a legal recent suffix, optionally extending it back to a user turn. - Returns ``(dropped, already_consolidated_count)`` where *dropped* is - the list of removed messages (in original order) and - *already_consolidated_count* is how many of those were inside the - pre-existing ``last_consolidated`` prefix and therefore do not need - raw archiving. + Returns a RetentionResult describing retained messages, removed messages, + and how the last_consolidated cursor changed. """ if max_messages <= 0: dropped = list(self.messages) lc = self.last_consolidated self.clear() - return dropped, min(lc, len(dropped)) + return RetentionResult( + retained=self.messages, + dropped=dropped, + already_consolidated_count=min(lc, len(dropped)), + new_last_consolidated=self.last_consolidated, + ) if len(self.messages) <= max_messages: - return [], 0 + return RetentionResult( + retained=self.messages, + dropped=[], + already_consolidated_count=0, + new_last_consolidated=self.last_consolidated, + ) original = list(self.messages) before_lc = self.last_consolidated @@ -370,7 +385,12 @@ class Session: self.messages = retained self.last_consolidated = new_lc self.updated_at = datetime.now() - return dropped, already_consolidated + return RetentionResult( + retained=retained, + dropped=dropped, + already_consolidated_count=already_consolidated, + new_last_consolidated=new_lc, + ) def enforce_file_cap( self, @@ -381,17 +401,17 @@ class Session: if limit <= 0 or len(self.messages) <= limit: return - dropped, already_consolidated = self.retain_recent_legal_suffix(limit) - if not dropped: + result = self.retain_recent_legal_suffix(limit) + if not result.dropped: return - archive_chunk = dropped[already_consolidated:] + archive_chunk = result.dropped[result.already_consolidated_count:] if archive_chunk and on_archive: on_archive(archive_chunk) logger.info( "Session file cap hit for {}: dropped {}, raw-archived {}, kept {}", self.key, - len(dropped), + len(result.dropped), len(archive_chunk), len(self.messages), ) diff --git a/tests/agent/test_auto_compact.py b/tests/agent/test_auto_compact.py index fe63d7da..20cb648d 100644 --- a/tests/agent/test_auto_compact.py +++ b/tests/agent/test_auto_compact.py @@ -103,12 +103,12 @@ def _make_fake_compact( metadata={}, last_consolidated=0, ) - dropped, already_consolidated = probe.retain_recent_legal_suffix( + result = probe.retain_recent_legal_suffix( max_suffix, extend_to_user=True, ) kept = probe.messages - archive_msgs = dropped[already_consolidated:] + archive_msgs = result.dropped[result.already_consolidated_count:] if not archive_msgs and not kept: session.updated_at = datetime.now() diff --git a/tests/agent/test_session_manager_history.py b/tests/agent/test_session_manager_history.py index ab619cab..066499c2 100644 --- a/tests/agent/test_session_manager_history.py +++ b/tests/agent/test_session_manager_history.py @@ -685,12 +685,12 @@ def test_retain_recent_legal_suffix_returns_dropped_messages(): for i in range(10): session.messages.append({"role": "user", "content": f"msg{i}"}) - dropped, already_cons = session.retain_recent_legal_suffix(4) + result = session.retain_recent_legal_suffix(4) - assert len(dropped) == 6 - assert [m["content"] for m in dropped] == [f"msg{i}" for i in range(6)] + assert len(result.dropped) == 6 + assert [m["content"] for m in result.dropped] == [f"msg{i}" for i in range(6)] assert len(session.messages) == 4 - assert already_cons == 0 + assert result.already_consolidated_count == 0 def test_retain_recent_legal_suffix_returns_empty_when_no_drop(): @@ -699,10 +699,10 @@ def test_retain_recent_legal_suffix_returns_empty_when_no_drop(): for i in range(3): session.messages.append({"role": "user", "content": f"msg{i}"}) - dropped, already_cons = session.retain_recent_legal_suffix(4) + result = session.retain_recent_legal_suffix(4) - assert dropped == [] - assert already_cons == 0 + assert result.dropped == [] + assert result.already_consolidated_count == 0 assert len(session.messages) == 3 @@ -713,10 +713,10 @@ def test_retain_recent_legal_suffix_returns_all_on_zero(): session.messages.append({"role": "user", "content": f"msg{i}"}) session.last_consolidated = 3 - dropped, already_cons = session.retain_recent_legal_suffix(0) + result = session.retain_recent_legal_suffix(0) - assert len(dropped) == 5 - assert already_cons == 3 + assert len(result.dropped) == 5 + assert result.already_consolidated_count == 3 assert session.messages == [] @@ -820,11 +820,11 @@ def test_retain_recent_legal_suffix_last_consolidated_correct_in_else_branch(): session.messages.append({"role": "assistant", "content": f"a{i}"}) session.last_consolidated = 12 # u0..u9, a0, a1 consolidated - dropped, already_cons = session.retain_recent_legal_suffix(4) + result = session.retain_recent_legal_suffix(4) # Retained messages start from latest user (u9) + max_messages forward # so retained = [u9, a0..a9][:4] → but these are from original indices 9..12 # Of those, indices 9,10,11 are < 12 (before_lc), so new_lc = 3 assert session.last_consolidated == 3 # already_cons should count dropped messages with original index < 12 - assert already_cons == 9 + assert result.already_consolidated_count == 9