fix(session_search): order recent mode by last activity instead of start time
- order session_search recent-mode results by last activity instead of session start time - add an opt-in `order_by_last_active` path to `SessionDB.list_sessions_rich` - add regression coverage for both the database ordering and recent-mode call path
This commit is contained in:
parent
96691268df
commit
142b4bf3ce
@ -933,6 +933,7 @@ class SessionDB:
|
||||
offset: int = 0,
|
||||
include_children: bool = False,
|
||||
project_compression_tips: bool = True,
|
||||
order_by_last_active: bool = False,
|
||||
) -> List[Dict[str, Any]]:
|
||||
"""List sessions with preview (first user message) and last active timestamp.
|
||||
|
||||
@ -952,6 +953,11 @@ class SessionDB:
|
||||
compressed continuations from being invisible to users while keeping
|
||||
delegate subagents and branches hidden. Pass ``False`` to return the
|
||||
raw root rows (useful for admin/debug UIs).
|
||||
|
||||
Pass ``order_by_last_active=True`` to sort by most-recent activity
|
||||
instead of original conversation start time. This is computed after
|
||||
compression-tip projection so "recent sessions" surfaces the live tip
|
||||
of a compressed conversation in the correct slot.
|
||||
"""
|
||||
where_clauses = []
|
||||
params = []
|
||||
@ -979,6 +985,15 @@ class SessionDB:
|
||||
params.extend(exclude_sources)
|
||||
|
||||
where_sql = f"WHERE {' AND '.join(where_clauses)}" if where_clauses else ""
|
||||
order_sql = (
|
||||
"ORDER BY last_active DESC, s.started_at DESC, s.id DESC"
|
||||
if order_by_last_active
|
||||
else "ORDER BY s.started_at DESC"
|
||||
)
|
||||
limit_sql = ""
|
||||
if not order_by_last_active:
|
||||
limit_sql = "LIMIT ? OFFSET ?"
|
||||
params.extend([limit, offset])
|
||||
query = f"""
|
||||
SELECT s.*,
|
||||
COALESCE(
|
||||
@ -994,10 +1009,9 @@ class SessionDB:
|
||||
) AS last_active
|
||||
FROM sessions s
|
||||
{where_sql}
|
||||
ORDER BY s.started_at DESC
|
||||
LIMIT ? OFFSET ?
|
||||
{order_sql}
|
||||
{limit_sql}
|
||||
"""
|
||||
params.extend([limit, offset])
|
||||
with self._lock:
|
||||
cursor = self._conn.execute(query, params)
|
||||
rows = cursor.fetchall()
|
||||
@ -1047,6 +1061,17 @@ class SessionDB:
|
||||
projected.append(merged)
|
||||
sessions = projected
|
||||
|
||||
if order_by_last_active:
|
||||
sessions.sort(
|
||||
key=lambda s: (
|
||||
s.get("last_active") or s.get("started_at") or 0,
|
||||
s.get("started_at") or 0,
|
||||
s.get("id") or "",
|
||||
),
|
||||
reverse=True,
|
||||
)
|
||||
sessions = sessions[offset:offset + limit]
|
||||
|
||||
return sessions
|
||||
|
||||
def _get_session_rich_row(self, session_id: str) -> Optional[Dict[str, Any]]:
|
||||
|
||||
@ -1719,6 +1719,39 @@ class TestListSessionsRich:
|
||||
# No messages, so last_active falls back to started_at
|
||||
assert sessions[0]["last_active"] == sessions[0]["started_at"]
|
||||
|
||||
def test_order_by_last_active_surfaces_recently_touched_older_session_first(self, db):
|
||||
t0 = 1709500000.0
|
||||
db.create_session("old", "cli")
|
||||
db.create_session("new", "cli")
|
||||
|
||||
with db._lock:
|
||||
db._conn.execute("UPDATE sessions SET started_at=? WHERE id=?", (t0, "old"))
|
||||
db._conn.execute("UPDATE sessions SET started_at=? WHERE id=?", (t0 + 10, "new"))
|
||||
|
||||
db.append_message("old", "user", "old first")
|
||||
db.append_message("new", "user", "new first")
|
||||
db.append_message("old", "assistant", "old touched later")
|
||||
|
||||
with db._lock:
|
||||
db._conn.execute(
|
||||
"UPDATE messages SET timestamp=? WHERE session_id=? AND role=? AND content=?",
|
||||
(t0 + 1, "old", "user", "old first"),
|
||||
)
|
||||
db._conn.execute(
|
||||
"UPDATE messages SET timestamp=? WHERE session_id=? AND role=? AND content=?",
|
||||
(t0 + 11, "new", "user", "new first"),
|
||||
)
|
||||
db._conn.execute(
|
||||
"UPDATE messages SET timestamp=? WHERE session_id=? AND role=? AND content=?",
|
||||
(t0 + 20, "old", "assistant", "old touched later"),
|
||||
)
|
||||
db._conn.commit()
|
||||
|
||||
assert [s["id"] for s in db.list_sessions_rich(limit=5)] == ["new", "old"]
|
||||
assert [
|
||||
s["id"] for s in db.list_sessions_rich(limit=5, order_by_last_active=True)
|
||||
] == ["old", "new"]
|
||||
|
||||
def test_rich_list_includes_title(self, db):
|
||||
db.create_session("s1", "cli")
|
||||
db.set_session_title("s1", "refactoring auth")
|
||||
|
||||
@ -242,6 +242,21 @@ class TestSessionSearchConcurrency:
|
||||
|
||||
|
||||
class TestRecentSessionListing:
|
||||
def test_recent_mode_requests_last_active_ordering(self):
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
mock_db = MagicMock()
|
||||
mock_db.list_sessions_rich.return_value = []
|
||||
|
||||
result = json.loads(_list_recent_sessions(mock_db, limit=5))
|
||||
|
||||
assert result["success"] is True
|
||||
mock_db.list_sessions_rich.assert_called_once_with(
|
||||
limit=10,
|
||||
exclude_sources=["tool"],
|
||||
order_by_last_active=True,
|
||||
)
|
||||
|
||||
def test_current_child_session_excludes_root_lineage_even_when_child_id_is_longer(self):
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
|
||||
@ -266,7 +266,11 @@ _HIDDEN_SESSION_SOURCES = ("tool",)
|
||||
def _list_recent_sessions(db, limit: int, current_session_id: str = None) -> str:
|
||||
"""Return metadata for the most recent sessions (no LLM calls)."""
|
||||
try:
|
||||
sessions = db.list_sessions_rich(limit=limit + 5, exclude_sources=list(_HIDDEN_SESSION_SOURCES)) # fetch extra to skip current
|
||||
sessions = db.list_sessions_rich(
|
||||
limit=limit + 5,
|
||||
exclude_sources=list(_HIDDEN_SESSION_SOURCES),
|
||||
order_by_last_active=True,
|
||||
) # fetch extra to skip current
|
||||
|
||||
# Resolve current session lineage to exclude it
|
||||
current_root = None
|
||||
|
||||
Loading…
Reference in New Issue
Block a user