feat(agent): track per-model token usage for mid-session model switches - #51634
feat(agent): track per-model token usage for mid-session model switches#51634tcconnally wants to merge 1 commit into
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Well-implemented per-model token usage tracking that fixes issue #51607. The design is solid:
- New
session_model_usagetable with composite PK (session_id, model, billing_provider) and proper indexes - Schema migration v17 with idempotent INSERT OR IGNORE backfill for legacy sessions
- Only incremental path records here (absolute=True calls excluded to avoid double-counting)
- Clean fallback: sessions without per-model rows fall back to their aggregate so totals never regress
- Comprehensive test coverage: single-model usage, mid-session switch, insights breakdown, and schema migration
- Good documentation in docstrings explaining the design rationale
No security concerns. The COALESCE fallback for sessions without per-model data is a nice safety net.
Reviewed by Hermes Agent
8437e8d to
4483e5d
Compare
The `sessions` table records only the initial (model, billing_provider) for a session, so when a user switches models mid-session (via `/model` or programmatically) every token — including the switched model's — is attributed to the first model. Insights/billing reports then hide the cost of the new model entirely (e.g. a session that started on deepseek and switched to opus shows $0 for opus). Add a `session_model_usage` table keyed (session_id, model, billing_provider) that accumulates each per-API-call delta under the model active at the time of the call. `update_token_counts()` is the single chokepoint every per-call delta flows through (CLI, gateway, cron, delegated, codex), so recording there captures accurate attribution on every platform. Only the incremental path records — the gateway's `absolute=True` summary overwrite is skipped to avoid double-counting cumulative totals that can't be split per model. When a call omits the model, it falls back to the session's recorded model, matching the existing COALESCE-from-session summary behaviour. Insights `_compute_model_breakdown` now aggregates tokens and cost from `session_model_usage`, so a switched session splits correctly across models, with a defensive fallback to the per-session aggregate for any session lacking usage rows. A v17 migration backfills one usage row per existing token-bearing session from its aggregate totals (idempotent via INSERT OR IGNORE), validated lossless against a 1.3 GB production DB. Tests: per-model recording, mid-session split, model fallback, absolute no-double-count, v17 backfill, and an insights-level switch breakdown. Fixes #51607. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
4483e5d to
7743955
Compare
|
Rebased onto latest |
|
Merged via PR #62610 after salvaging your per-model usage design onto current main and preserving your authorship in git history. The landing also adds route identity, migration/deletion safety, cumulative-total reconciliation, stored cost attribution, and fallback coverage found during review. Thanks for building the core accounting path and its initial state/Insights tests. |
What does this PR do?
The
sessionstable records only the initial(model, billing_provider)for a session. When a user switches models mid-session (via/modelor programmatically), every token — including the switched model's — is attributed to the first model. Insights/billing reports then hide the new model's cost entirely.This adds per-model usage attribution so token/cost metrics stay accurate across any number of mid-session switches, while leaving the
sessionssummary row (and its "latest model" display) unchanged.Related Issue
Fixes #51607
(A prior attempt, #28842, was closed without merging and never landed — confirmed no
session_model_usage/usage_by_modelsymbol exists onmain.)Type of Change
Changes Made
hermes_state.pysession_model_usagetable keyed(session_id, model, billing_provider)(+ token/cost counters,api_call_count,first_seen/last_seen) and two indexes.update_token_counts()now upserts each per-API-call delta into the table under the model active at that call — the single chokepoint every platform's per-call delta flows through (CLI / gateway / cron / delegated / codex). Recording happens only on the incremental path: the gateway'sabsolute=Truesummary overwrite is intentionally skipped, since cumulative totals can't be split per model and would double-count. When a call omits the model, it falls back to the session's recorded model (matches the existingCOALESCE-from-session summary behaviour, so current callers that pass no model keep working).SCHEMA_VERSION16 → 17 with an idempotent backfill (INSERT OR IGNORE) seeding one usage row per existing token-bearing session from its aggregate totals.agent/insights.py_compute_model_breakdownaggregates tokens & cost fromsession_model_usage, so a switched session splits across the models it used. Defensive fallback to the per-session aggregate for any session lacking usage rows (totals never regress). Tool-call counts remain attributed to the session's recorded model (tool calls aren't tied to a specific API invocation).tests/test_hermes_state.pyandtests/agent/test_insights.py.How to Test
pytest tests/test_hermes_state.py tests/agent/test_insights.py -qtest_mid_session_switch_splits_per_model_usage— pre/post-switch deltas land on the correct model; summary row keeps combined totals + latest model.test_model_breakdown_splits_mid_session_switch— insights report shows both models with correctly-attributed tokens/cost.test_per_model_usage_falls_back_to_session_model,test_absolute_update_does_not_record_per_model,test_v17_backfill_seeds_existing_session_usage.state.db— all 1,257 token-bearing sessions migrate losslessly (per-model token sums reconcile exactly with thesessionsaggregate; distinct-session count matches).Checklist
Code
pytest tests/ -qand all tests pass — ran the relevanttest_hermes_state.py+test_insights.pysuites (344 pass) and validated the migration on production data; full suite runs in CIDocumentation & Housekeeping
cli-config.yaml.example— N/A (no config keys added)CONTRIBUTING.md/AGENTS.md— N/Asqlite3+timeonly)