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>
This commit is contained in:
@@ -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()
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user