Repository navigation
Fix session analytics accuracy: dedupe usage, current pricing, active time - #106
Conversation
JayantDevkar
left a comment
JayantDevkar
left a comment
There was a problem hiding this comment.
Inline context for reviewers, one note per change area.
| if msg.usage is None: | ||
| return | ||
| key = msg.message_id or msg.request_id or msg.uuid | ||
| self._responses[key] = (msg.usage, msg.model) |
There was a problem hiding this comment.
Last line wins, not first: in 16,207 local duplicate lines the later one always had more output tokens, because streaming updates the count. Keeping the first line undercounted subagent output 4.4x on the 45-subagent session.
| """Extract (family, major, minor) from a model ID; date suffixes are not versions.""" | ||
|
|
||
| def version(part: Optional[str]) -> Optional[int]: | ||
| return int(part) if part and len(part) < 8 else None |
There was a problem hiding this comment.
Parts of 8+ digits are date suffixes, not versions, so claude-opus-5-20261201 resolves to Opus 5 rather than falling back to the newest Opus. The known-version table is built from MODEL_PRICING keys, so adding a model there is the only edit needed.
| pricing.get("output_long", pricing["output"]) if is_long else pricing["output"] | ||
| ) | ||
|
|
||
| cache_write_1h = min(self.cache_creation_1h_input_tokens, self.cache_creation_input_tokens) |
There was a problem hiding this comment.
The min() guards against a 1h count larger than the total. Usage with no cache_creation breakdown (older transcripts) prices everything at the 5-minute 1.25x rate, as before.
|
@JayantDevkar I reviewed this PR. Fixing the duplicate usage counting and the pricing is the right call, and checking it against an independent recount was well done. The PR's own tests pass (395 across the touched suites), and ruff shows nothing new. I found the following issues. Could you work through them, starting with #1, which blocks the merge? Blocker1. The v20/v21 migrations are skipped on any DB whose version is already 21 or higher ( Should fix before merge2. The Overview tab changes too, not just Analytics ( 3. Totals mix scopes ( 4. The unknown-version fallback can cross major versions ( 5. Active-time duration is only applied in one place ( Lower priority6. 7. In older-format sessions, where subagent turns are written inline with 8. On the JSONL fallback, 9. v20 is redundant: v21 also lowers Not covered by this review: how long the forced reindex takes on a large Happy to pair on #1 if useful. |
…108) The v19 migration re-enabled PRAGMA foreign_keys inside an open transaction, where SQLite ignores it, so the writer connection ran with cascades disabled for the rest of the process. Combined with delete paths that relied on the cascade, child rows could outlive their parents. - schema: commit before toggling foreign_keys in v19; re-enable after the final commit; v22 migration deletes orphaned rows in every child table - indexer: one explicit delete helper for the empty-session and stale paths (covers shells/cron too); session row uses ON CONFLICT DO UPDATE so re-indexing no longer cascade-deletes children; health check reports orphan counts - tests: upgrade paths keep FK on, delete paths work with FK off, repair migration, health metric, re-index preserves child ids v20/v21 are reserved for PR #106; this must land after it.
… time Claude Code repeats a response's usage on every content-block line, so tokens and cost were summed 2.5-3x. Usage is now counted once per API response (last line wins), priced with current models, per-model cache read rates and 2x 1-hour cache writes. Session duration is active time (idle gaps capped at 30 min). Session cards and project/global totals include subagent cost. Schema v20/v21 force a reindex.
Subagents have their own analytics pages, so the session cards no longer fold in subagent tokens, cost or cache hit rate. Drops the subagent_* fields from SessionDetail and the multi-file ETag that only they needed. Project and global totals still include subagent cost.
GET /sessions/{uuid}/tools takes include_subagents (default true, so
other callers are unchanged); the session page passes false, so Tools
Used and the tool table match the other session-only cards.
…t subagent_cost - Unknown model versions bill like the nearest known version at or below them (oldest known if none), instead of the newest model in the family - Index each subagent file once per session: the cost sum and the invocation rows share one list_subagents() call - Add subagent_cost to the session upsert SET list (lost in the rebase onto the ON CONFLICT upsert), so re-indexing refreshes it - "Active time over ..." footnote compares seconds with a 60s threshold instead of formatted strings
11e91cd to
e994853
Compare
JayantDevkar
left a comment
JayantDevkar
left a comment
There was a problem hiding this comment.
Inline context for reviewers, one note per change area.
| logger.info("Migrating → v22: remove orphaned child rows (issue #107)") | ||
| _delete_orphan_rows(conn) | ||
|
|
||
| if current_version < 23: |
There was a problem hiding this comment.
One v23 block for the re-pricing reindex. The column adds moved out of the version gate into _ensure_session_columns (see the note there), so this block only nudges mtimes, and skips that when the guard already did.
| _KNOWN_VERSIONS, _VERSIONS_BY_FAMILY = _index_known_versions() | ||
|
|
||
|
|
||
| def _nearest_known(parsed: ModelVersion) -> str: |
There was a problem hiding this comment.
Prefers the same major version: claude-haiku-4 bills as Haiku 4.5 and claude-sonnet-4-9 as Sonnet 4.6, not the 5.5 models. Only with no known version in that major does it fall back across majors, and each guessed model logs one warning.
| if subagents_dir.exists(): | ||
| subagent_count = len(list(subagents_dir.glob("agent-*.jsonl"))) | ||
| # One parse of each subagent file feeds both the cost sum and the invocation rows below | ||
| subagents = session.list_subagents() if subagent_count else [] |
There was a problem hiding this comment.
Correction to my earlier note: Agent metadata is cached by file path, so the old code wasn't re-parsing subagent files. This listing now feeds the cost, the token totals and the invocation rows; the change is about one listing, not about parse count.
| "slug project_encoded_name project_path start_time end_time message_count " | ||
| "duration_seconds input_tokens output_tokens cache_creation_tokens cache_read_tokens " | ||
| "total_cost initial_prompt git_branch models_used session_titles is_continuation_marker " | ||
| "total_cost subagent_cost initial_prompt git_branch models_used session_titles is_continuation_marker " |
There was a problem hiding this comment.
Added subagent_cost here. Main's ON CONFLICT DO UPDATE statement enumerates the columns to refresh, and the rebase merged the INSERT list but not this SET list, so a re-indexed session would have kept its old cost. test_reindex_overwrites_stale_subagent_cost covers it.
…icing, active time - Sessions columns added after v19 are ensured on every startup, before the version check, so a DB stamped higher by another branch still gets them; adding one forces a reindex - Overview and live polling use the default /tools response again; the Analytics tab derives session-only rows from by_session (include_subagents param removed) - Store subagent token totals per session; project/global token totals include them, matching the cost that already did (SQLite and JSONL fallback paths) - Unknown model versions stay within their major version and log one warning - Session, Agent, index entries and the JSONL sessions list share one active-time calculation; subagent durations are active time
JayantDevkar
left a comment
JayantDevkar
left a comment
There was a problem hiding this comment.
Inline context for reviewers, one note per change area.
| ] | ||
|
|
||
|
|
||
| def _ensure_session_columns(conn: sqlite3.Connection) -> bool: |
There was a problem hiding this comment.
Runs on every startup before the version check, like the tickets and shells tables, so a DB stamped higher by another branch (e.g. v24 from #85) still gets these columns. Adding a column also forces one reindex so the new values get filled in.
| return | ||
| if self._latest is None: | ||
| self._latest = timestamp | ||
| elif timestamp > self._latest: |
There was a problem hiding this comment.
Active time advances from the max timestamp seen, so an out-of-order line can't add a negative gap or double-count one. Session and Agent now share this, which is what makes subagent durations active time too.
| } | ||
|
|
||
|
|
||
| def active_duration_seconds( |
There was a problem hiding this comment.
For callers that only know a session's span (stale sessions-index.json entries, the JSONL sessions list). Under 30 minutes the wall clock is exact, so the file is only parsed for longer spans; a missing file returns None rather than an inflated wall-clock number.
| let totalToolCalls = $derived(toolsArray.reduce((acc, t) => acc + t.count, 0)); | ||
|
|
||
| // Overview counts subagent tool calls; the Analytics tab is the session's own work | ||
| let analyticsTools = $derived( |
There was a problem hiding this comment.
The page is back on the default /tools response, so Overview and live-poll change detection see subagent tool calls again. The Analytics tab derives its session-only rows from by_session, which that response already carried, so there is no second fetch.
|
@the-non-expert thanks for the thorough pass. Items 1–5 are fixed in
Verification on
Could you take another look? |
|
@the-non-expert heads up: I merged this before your re-review so it ships with the schema fix. Your items 1–5 are addressed as described above. If anything in that reply looks off, comment here or open an issue and I'll follow up in a new PR. |
2 participants
Problem
The session Analytics tab overstated cost and tokens 2–6x. Claude Code writes one JSONL line per content block and repeats the response's
usageon each, and we summed every line. Pricing also stopped at the 4.6 models, so Opus 4.8 was billed as Opus 4 ($15/$75) and Fable 5.1 as Sonnet 4.6. Resumed sessions showed durations of up to 109 days.Changes
UsageAccumulatorcounts usage once per API response; the last line wins because streamed output counts grow.by_sessionsplit already in the/toolsresponse. The Overview tab and live polling still count subagent tool calls. Each subagent's usage is on its own analytics page.sessions.subagent_costandsubagent_*_tokens), since that is real spend. Per-session rows stay session-only, so they don't sum to the project total.StatsCardrendered descriptions with no space before them.Component
Type
Testing
Checked against an independent recount of the raw JSONL, after a local reindex of 978 sessions:
Follow-ups
_categorize_model).subagent_costonly refreshes when the parent JSONL changes, so a background subagent that keeps writing after the parent's last turn is under-counted until the next parent write.isSidechaincount that cost in the session-only tab but not its tool calls. Not reproducible on my data.