perf(dashboard): aggregate session stats by source in SQL - #61120
Closed
mijanx wants to merge 1 commit into
Closed
Conversation
alt-glitch
added
type/perf
Collaborator
Duplicate of #48921 — both add |
Contributor
Author
|
Closing as duplicate of #48921, which is the earlier open PR for the same /api/sessions/stats GROUP BY source optimization. The active_store semantics in this PR were intentionally left unchanged; any useful test/semantics notes can be carried over there if needed. |
13 tasks
kshitijk4poor
pushed a commit
to kshitijk4poor/hermes-agent
that referenced
this pull request
Jul 28, 2026
…0k rows Replaces the O(N) list_sessions_rich histogram in /api/sessions/stats with a single GROUP BY query, reducing response time from ~575ms to <1ms on large databases. Original PR NousResearch#48921 by @liuhao1024. Salvage fixes based on review feedback from teknium1 and @wernerhp: 1. Preserve try/except guard — a DB error still degrades to empty by_source instead of failing the whole stats response. 2. GROUP BY COALESCE(source, 'cli') — the original GROUP BY source could emit duplicate 'cli' keys (NULL group + literal 'cli' group) that the dict comprehension silently dropped. 3. Add exclude_children=True — list_sessions_rich excludes subagent runs, delegates, and compression continuations by default; the bare GROUP BY counted all rows, inflating source counts. Aggregate shape (exclude_children/include_archived/limit params) adapted from closed duplicate NousResearch#61120 by @mijanx. Closes NousResearch#48914 Co-authored-by: mijanx <mijanx@users.noreply.github.com>
Collaborator
|
Your aggregate shape (exclude_children/include_archived params, COALESCE in GROUP BY, test verifying list_sessions_rich is not called) was adapted into #73362. Credited as co-author in the commit. Thanks for the more thorough implementation! |
kshitijk4poor
pushed a commit
that referenced
this pull request
Jul 28, 2026
…0k rows Replaces the O(N) list_sessions_rich histogram in /api/sessions/stats with a single GROUP BY query, reducing response time from ~575ms to <1ms on large databases. Original PR #48921 by @liuhao1024. Salvage fixes based on review feedback from teknium1 and @wernerhp: 1. Preserve try/except guard — a DB error still degrades to empty by_source instead of failing the whole stats response. 2. GROUP BY COALESCE(source, 'cli') — the original GROUP BY source could emit duplicate 'cli' keys (NULL group + literal 'cli' group) that the dict comprehension silently dropped. 3. Add exclude_children=True — list_sessions_rich excludes subagent runs, delegates, and compression continuations by default; the bare GROUP BY counted all rows, inflating source counts. Aggregate shape (exclude_children/include_archived/limit params) adapted from closed duplicate #61120 by @mijanx. Closes #48914 Co-authored-by: mijanx <mijanx@users.noreply.github.com>
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
…0k rows Replaces the O(N) list_sessions_rich histogram in /api/sessions/stats with a single GROUP BY query, reducing response time from ~575ms to <1ms on large databases. Original PR NousResearch#48921 by @liuhao1024. Salvage fixes based on review feedback from teknium1 and @wernerhp: 1. Preserve try/except guard — a DB error still degrades to empty by_source instead of failing the whole stats response. 2. GROUP BY COALESCE(source, 'cli') — the original GROUP BY source could emit duplicate 'cli' keys (NULL group + literal 'cli' group) that the dict comprehension silently dropped. 3. Add exclude_children=True — list_sessions_rich excludes subagent runs, delegates, and compression continuations by default; the bare GROUP BY counted all rows, inflating source counts. Aggregate shape (exclude_children/include_archived/limit params) adapted from closed duplicate NousResearch#61120 by @mijanx. Closes NousResearch#48914 Co-authored-by: mijanx <mijanx@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SessionDB.session_count_by_source()so source badges can be counted with a grouped SQL aggregate/api/sessions/statsto use that aggregate instead of materializing up to 10klist_sessions_rich()rowsactive_storesemantics and API shape unchangedWhy
/api/sessions/statsonly needs grouped source counts for small dashboard badges. Before this change it built rich session-list rows — including preview/last-active projection and compression-tip handling — just to countsourcelabels.That is unnecessary work on large stores. On one real store with 24,691 session rows, the old path took a median ~665ms and also only counted the first 10k materialized rows. The direct aggregate took ~29ms and counts the full listable store. On a 9,009-row profile, the old path was ~883ms vs ~17ms for the aggregate.
This PR deliberately does not change
active_store; the existing field continues to mean the current non-archived store count. If the dashboard wants a separate open/unclosed-session metric later, that should be an explicit additive API/UI decision rather than hidden inside this perf fix.Duplicate check
Searched open and closed PRs/issues in
NousResearch/hermes-agentfor:active_store sessions stats"Active in store""sessions/stats" "by_source""end_reason IS NULL" "sessions" "stats""unarchived" "sessions/stats"No duplicate PRs or issues found.
Validation
python -m py_compile hermes_state.py hermes_cli/web_server.py tests/hermes_cli/test_dashboard_admin_endpoints.pypytest tests/hermes_cli/test_dashboard_admin_endpoints.py::TestSessionManagementEndpoints::test_stats_not_shadowed_by_session_id_route tests/hermes_cli/test_dashboard_admin_endpoints.py::TestSessionManagementEndpoints::test_stats_source_counts_use_direct_aggregate -q -o 'addopts='→ 2 passedpytest tests/hermes_cli/test_dashboard_admin_endpoints.py -q -o 'addopts='→ 74 passedpytest tests/test_hermes_state.py -q -o 'addopts='→ 339 passedruff check hermes_state.py hermes_cli/web_server.py tests/hermes_cli/test_dashboard_admin_endpoints.py→ cleangit diff --check→ clean