fix(sessions): compacted display history keeps protected-tail copies in original chronological order
_dedupe_display_generations chose the right representative row per logical message but sorted the survivors by that representative's id. A protected-tail copy written into a newer compaction generation has a higher id than messages emitted after the original, so include_compacted reads came back as C, A, B. Anchor the sort on the logical message's first-ever row id instead. Salvaged from #93869 (the tui_gateway half of that PR is superseded by #100504 and #104137); the code moved from hermes_state.py to hermes_state_messages.py since, so the change is re-applied to its new home with the PR's regression test verbatim.
This commit is contained in:
@@ -588,6 +588,7 @@ class SessionMessagesMixin:
|
|||||||
into each generation: same role/content/timestamp, different ``active``/id); prefer the live row, then
|
into each generation: same role/content/timestamp, different ``active``/id); prefer the live row, then
|
||||||
the newest. The ONE definition every display projection shares. *rows* must be ordered by ``id``."""
|
the newest. The ONE definition every display projection shares. *rows* must be ordered by ``id``."""
|
||||||
seen: Dict[Tuple[Any, ...], Any] = {}
|
seen: Dict[Tuple[Any, ...], Any] = {}
|
||||||
|
first_id: Dict[Tuple[Any, ...], int] = {}
|
||||||
for row in rows:
|
for row in rows:
|
||||||
dedupe_content = row["content"]
|
dedupe_content = row["content"]
|
||||||
if row["role"] == "user":
|
if row["role"] == "user":
|
||||||
@@ -604,7 +605,10 @@ class SessionMessagesMixin:
|
|||||||
cur = seen.get(key)
|
cur = seen.get(key)
|
||||||
if cur is None or (row["active"], row["id"]) > (cur["active"], cur["id"]):
|
if cur is None or (row["active"], row["id"]) > (cur["active"], cur["id"]):
|
||||||
seen[key] = row
|
seen[key] = row
|
||||||
return sorted(seen.values(), key=lambda r: r["id"])
|
first_id[key] = min(first_id.get(key, row["id"]), row["id"])
|
||||||
|
# Order by the logical message's FIRST row, not the chosen representative's: a protected-tail
|
||||||
|
# copy in a newer generation has a higher id than messages emitted after the original.
|
||||||
|
return [seen[key] for key in sorted(seen, key=first_id.__getitem__)]
|
||||||
|
|
||||||
def _row_to_message_dict(self, row, *, warn_context: str, summary_flag: bool) -> Dict[str, Any]:
|
def _row_to_message_dict(self, row, *, warn_context: str, summary_flag: bool) -> Dict[str, Any]:
|
||||||
"""``dict(row)`` with content/tool_calls/display_metadata decoded; *summary_flag* keeps
|
"""``dict(row)`` with content/tool_calls/display_metadata decoded; *summary_flag* keeps
|
||||||
|
|||||||
@@ -158,6 +158,32 @@ class TestDisplayDedupe:
|
|||||||
assert len(msgs) == 2
|
assert len(msgs) == 2
|
||||||
assert [m["content"] for m in msgs] == ["turn 1", "answer 1"]
|
assert [m["content"] for m in msgs] == ["turn 1", "answer 1"]
|
||||||
|
|
||||||
|
def test_new_generation_copy_keeps_original_chronological_position(self, db):
|
||||||
|
"""A protected-tail copy inserted after a newer message stays in its
|
||||||
|
original position in the display read (C, A, B regression)."""
|
||||||
|
sid = "s1"
|
||||||
|
db.create_session(sid, source="cli")
|
||||||
|
db.append_messages_batch(
|
||||||
|
sid,
|
||||||
|
[
|
||||||
|
{"role": "assistant", "content": "A", "timestamp": 100.0},
|
||||||
|
{"role": "assistant", "content": "B", "timestamp": 200.0},
|
||||||
|
],
|
||||||
|
)
|
||||||
|
original = _row_ids(db, sid)
|
||||||
|
db._execute_write(
|
||||||
|
lambda conn: conn.execute(
|
||||||
|
"UPDATE messages SET active = 0, compacted = 1 WHERE session_id = ?",
|
||||||
|
[sid],
|
||||||
|
)
|
||||||
|
)
|
||||||
|
db.append_message(sid, role="user", content="C", timestamp=300.0)
|
||||||
|
self._copy_tail_as_new_generation(db, sid, original)
|
||||||
|
|
||||||
|
msgs = db.get_messages(sid, include_compacted=True)
|
||||||
|
|
||||||
|
assert [m["content"] for m in msgs] == ["A", "B", "C"]
|
||||||
|
|
||||||
def test_dedupe_prefers_live_row_then_newest_generation(self, db):
|
def test_dedupe_prefers_live_row_then_newest_generation(self, db):
|
||||||
"""When generations conflict, the live row wins; otherwise the newest
|
"""When generations conflict, the live row wins; otherwise the newest
|
||||||
generation (highest id) wins."""
|
generation (highest id) wins."""
|
||||||
|
|||||||
Reference in New Issue
Block a user