fix(dashboard): query session_model_usage for accurate model analytics (#71778) - #71802
Conversation
NousResearch#71778) The Models dashboard page (`/api/analytics/models`) queried the `sessions` table with `GROUP BY model, billing_provider`. The `sessions` table only records the *final* billing_provider after a mid-session `/model` switch, so all tokens were attributed to one provider even when multiple were used. Switch the main query to `session_model_usage` which tracks every API call individually with its own model+provider attribution. This matches the approach already used by the Insights engine (`_compute_model_breakdown`). Includes a try/except fallback to the old `sessions` query for older DBs that may not have the `session_model_usage` table. Fixes NousResearch#71778
c70d245 to
c4ad87c
Compare
|
Reviewed against #71778. The root cause is right and the core fix is correct: Two things worth tightening before merge:
Neither blocks the core fix; both are about not masking a regression. Thanks for tracing this to the right table. |
|
Reviewed against #71778 — the root-cause diagnosis and the core fix are right. One concrete regression to flag before merge:
Minor: Neither blocks the core correctness fix; the provider/token attribution is the important part and this gets it right. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the Models dashboard to the per-route accounting table. Current main still aggregates this endpoint from sessions (hermes_cli/web_server.py:13952-13969), while the write path records the active model/provider per API delta in session_model_usage (hermes_state.py:4224-4290), so the core direction is correct.
Problems
hermes_cli/web_server.py:16835hardcodestool_callsto0.session_model_usagehas no such column (hermes_state_common.py:218-238), but the dashboard renders positiveentry.tool_callsvalues (web/src/pages/ModelsPage.tsx:520-524), so this removes the stat from every main-agent card.hermes_cli/web_server.py:16845catches every exception and silently falls back to the known-inaccuratesessionsquery. The analogous Insights fallback is limited tosqlite3.OperationalError(agent/insights.py:525-541).- The updated test only changes an existing zero-API-call expectation; it does not cover a single session switching between two model/provider routes.
Suggested changes
- Preserve or explicitly redesign session-level tool-call attribution rather than returning zeros.
- Narrow the fallback to the expected SQLite compatibility error.
- Add an endpoint regression test for two model/provider rows written under one session.
Automated hermes-sweeper review.
| COALESCE(SUM(u.actual_cost_usd), 0) as actual_cost, | ||
| COUNT(DISTINCT u.session_id) as sessions, | ||
| SUM(COALESCE(u.api_call_count, 0)) as api_calls, | ||
| 0 as tool_calls, |
There was a problem hiding this comment.
session_model_usage has no tool-call field, so this makes every main-agent Models card report zero and hides the UI statistic at ModelsPage.tsx:520. Please preserve a deliberate session-level attribution (as Insights does) or remove/redesign the displayed metric explicitly.
| GROUP BY u.model, u.billing_provider | ||
| ORDER BY SUM(u.input_tokens) + SUM(u.output_tokens) DESC | ||
| """, (cutoff,)) | ||
| except Exception: |
There was a problem hiding this comment.
Please narrow this fallback to the expected SQLite compatibility failure. Catching every exception hides defects in this new query and silently reverts users to the inaccurate sessions aggregation; Insights limits its analogous fallback to sqlite3.OperationalError.
SummarySeven PRs address this issue complex across three layers: persisting the latest model switch, recording per-route usage, and consuming that accounting in Insights and the Models dashboard. #35256 landed latest-model persistence, #62610 landed per-route accounting and Insights aggregation, while #71802 is the remaining dashboard-consumer fix but has documented gaps. Related pull requests
Duplicates#35181 is the source implementation salvaged by #35256; #51634 is the core per-route design salvaged and hardened by #62610. #28842 overlaps #51634/#62610 on per-model accounting but uses a different JSON storage design, while #49682 overlaps the stale-model symptom addressed correctly by #35256; #71802 is not a duplicate because it fixes the downstream Models dashboard consumer. Suggested consolidationKeep #71802 open with a salvage path, consistent with the contributor keep_open review: retain its session_model_usage aggregation and u.task = '' scoping, narrow the fallback to sqlite3.OperationalError, deliberately preserve or redesign tool-call attribution, and add a regression test with one session switching across two model/provider routes. Treat #35256 and #62610 as merged reference implementations, and keep #28842, #35181, #49682, and #51634 closed under the explicit supersession chains above. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I28637(["issue #28637 (closed)"])
I51607(["issue #51607 (closed)"])
I71778(["issue #71778 (open)"])
P71802["PR #71802 (open)"]
P71802 -.->|partial| I28637
P71802 -.->|partial| I51607
P71802 -->|best fix| I71778
class I28637 closed
class I51607 closed
class I71778 open
class P71802 open
class P71802 best
class P71802 target
click I28637 "https://github.com/NousResearch/hermes-agent/issues/28637"
click I51607 "https://github.com/NousResearch/hermes-agent/issues/51607"
click I71778 "https://github.com/NousResearch/hermes-agent/issues/71778"
click P71802 "https://github.com/NousResearch/hermes-agent/pull/71802"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 7 pull requests and 4 issues in this complex. Each diff was read against this issue; Assessment working set: 76 kB of PR diffs, 28 kB of issue/PR text, 15 kB of discussion (22 comments), 34 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Summary
Fixes #71778
The Models dashboard page (
/api/analytics/models) queried thesessionstable withGROUP BY model, billing_provider. Thesessionstable only records the finalbilling_providerafter a mid-session/modelswitch, so all tokens were attributed to a single provider even when multiple providers were used during the session.This PR switches the main query to
session_model_usagewhich tracks every API call individually with its own model+provider attribution. This matches the approach already used by the Insights engine (_compute_model_breakdown).Changes
hermes_cli/web_server.py: ReplaceFROM sessionsquery in_get_models_analytics()withFROM session_model_usage u JOIN sessions stry/exceptfallback to oldsessionsquery for older DBs without thesession_model_usagetableCOUNT(DISTINCT u.session_id)instead ofCOUNT(*)for accurate session countu.task = ''to exclude auxiliary usage rows (already handled separately by_aux_usage_rows)Test Plan
python3 -m py_compile hermes_cli/web_server.pysession_model_usagetable still work (fallback query)