mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-15 14:22:43 +00:00
fix: follow-up for salvaged #60347/#43653/#47437
- derive the compact_rows projection from SCHEMA_SQL (parse once, cache) instead of a hardcoded column list: the original #47437 list was cut against a June schema and silently dropped session_key/chat_id/chat_type/ thread_id/display_name/origin_json/expiry_finalized/git_branch/ git_repo_root/compression_failure_* — including desktop sidebar fields. Schema-derived means declaratively reconciled new columns are included automatically; only system_prompt is excluded. - guard test pinning the schema<->projection contract (mutation-verified: dropping a column from the projection fails it) - wire compact_rows=(not full) into /api/sessions and /api/profiles/sessions so the SQL projection pairs with the API-level field strip (?full=1 still returns complete rows end-to-end) - pass compact_rows at the remaining hot list callers: /api/status active count, _session_latest_descendant fallback, /api/sessions/stats by-source - thread compact_rows through the compression-tip projection (_get_session_rich_row) so projected tips can't reintroduce the blob - add pagination tests for get_messages (#60347 shipped none): paging order, offset-past-end, active-flag interaction; add tip-projection compact test - AUTHOR_MAP entries for mahdiwafy + CodeForgeNet (plain emails)
This commit is contained in:
parent
22eb1af23a
commit
4f220fc88b
4 changed files with 120 additions and 23 deletions
|
|
@ -1193,7 +1193,7 @@ def _count_status_active_sessions() -> int:
|
|||
|
||||
db = SessionDB(read_only=True)
|
||||
try:
|
||||
sessions = db.list_sessions_rich(limit=50)
|
||||
sessions = db.list_sessions_rich(limit=50, compact_rows=True)
|
||||
now = time.time()
|
||||
return sum(
|
||||
1 for s in sessions
|
||||
|
|
@ -3913,7 +3913,10 @@ def get_sessions(
|
|||
include_archived=include_archived,
|
||||
archived_only=archived_only,
|
||||
order_by_last_active=order == "recent",
|
||||
compact_rows=True,
|
||||
# SQL-level projection: when the caller didn't ask for full
|
||||
# rows, skip the system_prompt blob inside SQLite too (pairs
|
||||
# with the API-level _strip_session_list_rows below).
|
||||
compact_rows=not full,
|
||||
)
|
||||
total = db.session_count(
|
||||
source=source or None,
|
||||
|
|
@ -4032,7 +4035,8 @@ def get_profiles_sessions(
|
|||
include_archived=include_archived,
|
||||
archived_only=archived_only,
|
||||
order_by_last_active=order == "recent",
|
||||
compact_rows=True,
|
||||
# Same SQL-level blob skip as /api/sessions (see above).
|
||||
compact_rows=not full,
|
||||
)
|
||||
profile_total = db.session_count(
|
||||
source=source_filter,
|
||||
|
|
@ -9358,7 +9362,7 @@ def _session_latest_descendant(session_id: str, db):
|
|||
"started_at": row_get(row, "started_at", 2),
|
||||
})
|
||||
else:
|
||||
rows = db.list_sessions_rich(limit=10000, offset=0)
|
||||
rows = db.list_sessions_rich(limit=10000, offset=0, compact_rows=True)
|
||||
|
||||
children = {}
|
||||
for row in rows:
|
||||
|
|
@ -9512,7 +9516,7 @@ async def get_session_stats(profile: Optional[str] = None):
|
|||
messages = db.message_count()
|
||||
by_source: Dict[str, int] = {}
|
||||
try:
|
||||
for s in db.list_sessions_rich(limit=10000, include_archived=True):
|
||||
for s in db.list_sessions_rich(limit=10000, include_archived=True, compact_rows=True):
|
||||
src = str(s.get("source") or "cli")
|
||||
by_source[src] = by_source.get(src, 0) + 1
|
||||
except Exception:
|
||||
|
|
|
|||
|
|
@ -2947,20 +2947,27 @@ class SessionDB:
|
|||
current = child_id
|
||||
return current
|
||||
|
||||
# All sessions columns except the large system_prompt blob, each prefixed
|
||||
# with the "s" table alias used in list_sessions_rich/_get_session_rich_row
|
||||
# queries. Used when compact_rows=True to avoid reading the blob for
|
||||
# dashboard and picker callers that only need lightweight metadata.
|
||||
_SESSION_COMPACT_COLS = (
|
||||
"s.id, s.source, s.user_id, s.model, s.model_config, "
|
||||
"s.parent_session_id, s.started_at, s.ended_at, s.end_reason, "
|
||||
"s.message_count, s.tool_call_count, s.input_tokens, s.output_tokens, "
|
||||
"s.cache_read_tokens, s.cache_write_tokens, s.reasoning_tokens, "
|
||||
"s.cwd, s.billing_provider, s.billing_base_url, s.billing_mode, "
|
||||
"s.estimated_cost_usd, s.actual_cost_usd, s.cost_status, s.cost_source, "
|
||||
"s.pricing_version, s.title, s.api_call_count, s.handoff_state, "
|
||||
"s.handoff_platform, s.handoff_error, s.rewind_count, s.archived"
|
||||
)
|
||||
# Columns excluded from compact_rows projections: only the payload-heavy
|
||||
# blob no list consumer renders. Everything else — including gateway
|
||||
# routing fields and desktop sidebar fields like git_branch — stays, and
|
||||
# the projection is derived from SCHEMA_SQL so columns added later via
|
||||
# declarative reconciliation are included automatically instead of
|
||||
# silently dropping out of list rows.
|
||||
_SESSION_COMPACT_EXCLUDED = frozenset({"system_prompt"})
|
||||
_session_compact_cols_sql: Optional[str] = None
|
||||
|
||||
@classmethod
|
||||
def _compact_session_cols(cls) -> str:
|
||||
"""SELECT list for compact_rows: every ``sessions`` column declared in
|
||||
SCHEMA_SQL except the ``system_prompt`` blob, aliased with the ``s``
|
||||
prefix used by list_sessions_rich/_get_session_rich_row queries."""
|
||||
if cls._session_compact_cols_sql is None:
|
||||
declared = cls._parse_schema_columns(SCHEMA_SQL)["sessions"]
|
||||
cls._session_compact_cols_sql = ", ".join(
|
||||
f"s.{name}" for name in declared
|
||||
if name not in cls._SESSION_COMPACT_EXCLUDED
|
||||
)
|
||||
return cls._session_compact_cols_sql
|
||||
|
||||
def distinct_session_cwds(self, include_archived: bool = False) -> List[Dict[str, Any]]:
|
||||
"""Distinct non-empty session cwds with usage stats, for repo discovery.
|
||||
|
|
@ -3160,7 +3167,7 @@ class SessionDB:
|
|||
outer_where = (
|
||||
f"{where_sql} AND {combined}" if where_sql else f"WHERE {combined}"
|
||||
)
|
||||
_sel = self._SESSION_COMPACT_COLS if compact_rows else "s.*"
|
||||
_sel = self._compact_session_cols() if compact_rows else "s.*"
|
||||
query = f"""
|
||||
WITH RECURSIVE chain(root_id, cur_id) AS (
|
||||
SELECT s.id, s.id FROM sessions s {where_sql}
|
||||
|
|
@ -3207,7 +3214,7 @@ class SessionDB:
|
|||
# only applies to the outer select.
|
||||
params = params + params + id_params + [limit, offset]
|
||||
else:
|
||||
_sel = self._SESSION_COMPACT_COLS if compact_rows else "s.*"
|
||||
_sel = self._compact_session_cols() if compact_rows else "s.*"
|
||||
query = f"""
|
||||
SELECT {_sel},
|
||||
COALESCE(
|
||||
|
|
@ -3260,7 +3267,7 @@ class SessionDB:
|
|||
if tip_id == s["id"]:
|
||||
projected.append(s)
|
||||
continue
|
||||
tip_row = self._get_session_rich_row(tip_id)
|
||||
tip_row = self._get_session_rich_row(tip_id, compact_rows=compact_rows)
|
||||
if not tip_row:
|
||||
projected.append(s)
|
||||
continue
|
||||
|
|
@ -3354,7 +3361,7 @@ class SessionDB:
|
|||
Pass ``compact_rows=True`` to omit the ``system_prompt`` blob (see
|
||||
``list_sessions_rich`` for details).
|
||||
"""
|
||||
_sel = self._SESSION_COMPACT_COLS if compact_rows else "s.*"
|
||||
_sel = self._compact_session_cols() if compact_rows else "s.*"
|
||||
query = f"""
|
||||
SELECT {_sel},
|
||||
COALESCE(
|
||||
|
|
|
|||
|
|
@ -1929,6 +1929,8 @@ AUTHOR_MAP = {
|
|||
"rodisoft1@gmail.com": "0disoft", # PR #53511 salvage (gateway PID probe TTL cache)
|
||||
"craigs.seller.sixx@gmail.com": "0-CYBERDYNE-SYSTEMS-0", # PR #53966 salvage (session DB reads off event loop)
|
||||
"sebastianlutycz@users.noreply.github.com": "sebastianlutycz", # PR #39140 salvage (descendant CTE); bare noreply (no NNN+ prefix) needs explicit mapping
|
||||
"wafy.081107@gmail.com": "mahdiwafy", # PR #60347 salvage (session messages pagination)
|
||||
"codeforgenet@icloud.com": "CodeForgeNet", # PR #47437 salvage (compact_rows blob skip)
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -5422,3 +5422,87 @@ class TestCompactRows:
|
|||
self._create(db, "s1", system_prompt="present")
|
||||
rows = db.list_sessions_rich()
|
||||
assert "system_prompt" in rows[0]
|
||||
|
||||
def test_compact_projection_tracks_schema(self, db):
|
||||
"""Behavior contract: compact rows carry EVERY sessions column except
|
||||
the excluded blob — including gateway/desktop fields (git_branch,
|
||||
session_key) and any column added later via declarative
|
||||
reconciliation. Guards against a hardcoded column list going stale."""
|
||||
self._create(db, "s1")
|
||||
live_cols = {
|
||||
row[1] for row in db._conn.execute("PRAGMA table_info(sessions)")
|
||||
}
|
||||
row = db.list_sessions_rich(compact_rows=True)[0]
|
||||
# Hardcode the one sanctioned exclusion: if the excluded set ever
|
||||
# widens (or the projection silently drops a column), this fails and
|
||||
# forces a conscious review of what list consumers lose.
|
||||
missing = live_cols - set(row) - {"system_prompt"}
|
||||
assert not missing, f"compact projection lost schema columns: {missing}"
|
||||
assert "system_prompt" not in row
|
||||
|
||||
def test_compact_rows_tip_projection_omits_system_prompt(self, db):
|
||||
"""Compression-tip projection must not reintroduce the blob: the
|
||||
merged tip row is fetched with the same compact_rows flag (salvage
|
||||
follow-up for #47437)."""
|
||||
import time as _time
|
||||
t0 = _time.time() - 3600
|
||||
db.create_session("root", "cli")
|
||||
db.update_system_prompt("root", "root blob " * 200)
|
||||
db._conn.execute("UPDATE sessions SET started_at=? WHERE id=?", (t0, "root"))
|
||||
db._conn.execute(
|
||||
"UPDATE sessions SET ended_at=?, end_reason='compression' WHERE id=?",
|
||||
(t0 + 100, "root"),
|
||||
)
|
||||
db.create_session("tip", "cli", parent_session_id="root")
|
||||
db.update_system_prompt("tip", "tip blob " * 200)
|
||||
db._conn.execute("UPDATE sessions SET started_at=? WHERE id=?", (t0 + 200, "tip"))
|
||||
db._conn.commit()
|
||||
|
||||
rows = db.list_sessions_rich(compact_rows=True)
|
||||
projected = [r for r in rows if r.get("_lineage_root_id") == "root"]
|
||||
assert projected, "compression root should be projected to its tip"
|
||||
assert all("system_prompt" not in r for r in rows)
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# get_messages pagination (salvage follow-up for #60347)
|
||||
# =========================================================================
|
||||
|
||||
class TestGetMessagesPagination:
|
||||
"""get_messages(limit=, offset=) pages in insertion order; the default
|
||||
(limit=None) returns the full transcript unchanged."""
|
||||
|
||||
def _seed(self, db, n=10):
|
||||
db.create_session(session_id="s1", source="cli")
|
||||
for i in range(n):
|
||||
db.append_message("s1", "user" if i % 2 == 0 else "assistant", f"msg-{i}")
|
||||
|
||||
def test_default_returns_all_messages(self, db):
|
||||
self._seed(db)
|
||||
messages = db.get_messages("s1")
|
||||
assert [m["content"] for m in messages] == [f"msg-{i}" for i in range(10)]
|
||||
|
||||
def test_limit_pages_in_insertion_order(self, db):
|
||||
self._seed(db)
|
||||
page1 = db.get_messages("s1", limit=4, offset=0)
|
||||
page2 = db.get_messages("s1", limit=4, offset=4)
|
||||
page3 = db.get_messages("s1", limit=4, offset=8)
|
||||
assert [m["content"] for m in page1] == ["msg-0", "msg-1", "msg-2", "msg-3"]
|
||||
assert [m["content"] for m in page2] == ["msg-4", "msg-5", "msg-6", "msg-7"]
|
||||
assert [m["content"] for m in page3] == ["msg-8", "msg-9"]
|
||||
|
||||
def test_offset_past_end_returns_empty(self, db):
|
||||
self._seed(db, n=3)
|
||||
assert db.get_messages("s1", limit=5, offset=10) == []
|
||||
|
||||
def test_pagination_respects_active_flag(self, db):
|
||||
"""Soft-deleted (inactive) rows must not consume page slots."""
|
||||
self._seed(db, n=6)
|
||||
# Soft-delete the first two rows the way rewind does.
|
||||
db._conn.execute(
|
||||
"UPDATE messages SET active = 0 WHERE session_id = 's1' "
|
||||
"AND id IN (SELECT id FROM messages WHERE session_id = 's1' ORDER BY id LIMIT 2)"
|
||||
)
|
||||
db._conn.commit()
|
||||
page = db.get_messages("s1", limit=3, offset=0)
|
||||
assert [m["content"] for m in page] == ["msg-2", "msg-3", "msg-4"]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue