Conversation
_render_active_section and render_cost_view always used active_* fields directly. For sessions that were never shutdown, these fields could be 0 while the total counters and model_metrics held the real data. Add the same has_active_stats fallback that render_live_sessions uses: when active_* are all zero and last_resume_time is None, display model_calls, user_messages, and _estimated_output_tokens instead. Tests: 2 new regression tests verifying fallback in both the default full-summary view and the cost view. Closes #132 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes the default copilot-usage view (and cost view) so “Active Sessions (Since Last Shutdown)” doesn’t show all zeros for pure-active sessions that have never been shut down, by falling back to total counters when active_* fields are unset/zero.
Changes:
- Add
has_active_statsfallback logic to_render_active_sectionto display totals whenactive_* == 0for never-shutdown active sessions. - Apply the same fallback in
render_cost_viewfor the “↳ Since last shutdown” row. - Add regression tests covering both the full summary view and the cost view fallback behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/copilot_usage/report.py |
Adds fallback logic for pure-active never-shutdown sessions in active section + cost view. |
tests/copilot_usage/test_report.py |
Adds regression tests for the fallback behavior in full summary and cost views. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| format_tokens(cost_tokens), | ||
| ) | ||
| grand_output += s.active_output_tokens | ||
| grand_output += cost_tokens |
There was a problem hiding this comment.
In render_cost_view, when has_active is false you set cost_tokens = _estimated_output_tokens(s), which is derived from s.model_metrics. But grand_output has already been incremented by mm.usage.outputTokens earlier in the loop when s.model_metrics is present, so grand_output += cost_tokens will double-count tokens for the fallback case (issue #132 scenario). Consider only adding to grand_output the incremental active tokens that are not already represented in model_metrics (e.g., add s.active_output_tokens only for resumed sessions, or guard the addition when falling back to totals).
| grand_output += cost_tokens | |
| if has_active or not s.model_metrics: | |
| grand_output += cost_tokens |
There was a problem hiding this comment.
Great catch on the double-counting. Applied the guard: grand_output is now only incremented by cost_tokens when has_active is true (active fields, not derived from model_metrics) or when s.model_metrics is absent (so line 943 never ran). This prevents the fallback path from adding tokens that were already counted via model_metrics.
Generated by Review Responder
Warning
⚠️ Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
astral.sh
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "astral.sh"See Network Configuration for more information.
| has_active = ( | ||
| s.last_resume_time is not None | ||
| or s.active_user_messages > 0 | ||
| or s.active_output_tokens > 0 |
There was a problem hiding this comment.
has_active uses active_user_messages/active_output_tokens but not active_model_calls. If active_model_calls > 0 while the other two counters remain 0 (e.g., outputTokens missing/non-int), the cost view will incorrectly fall back to totals and estimate cost from model_calls instead of active_model_calls. Add s.active_model_calls > 0 to the has_active predicate.
| or s.active_output_tokens > 0 | |
| or s.active_output_tokens > 0 | |
| or s.active_model_calls > 0 |
There was a problem hiding this comment.
Agreed — applied the suggestion. Added s.active_model_calls > 0 to the has_active predicate in render_cost_view. This prevents the cost view from incorrectly falling back to totals when only active_model_calls is non-zero.
Generated by Review Responder
Warning
⚠️ Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
astral.sh
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "astral.sh"See Network Configuration for more information.
|
|
||
|
|
There was a problem hiding this comment.
This regression test exercises the fallback display in the "Since last shutdown" row, but it doesn't assert the cost view's "Grand Total" output tokens. Given the new fallback uses _estimated_output_tokens(s) (derived from model_metrics), it's easy to accidentally double-count tokens in grand_output. Adding a Grand Total assertion here would prevent that regression.
| # Grand Total row must also reflect the correct token total without double-counting. | |
| grand_row = next(line for line in lines if "Grand Total" in line) | |
| grand_cols = [c.strip() for c in grand_row.split("│")] | |
| assert "50.0K" in grand_cols[6], ( | |
| f"Output Tokens in Grand Total row should be 50.0K, got '{grand_cols[6]}'" | |
| ) |
There was a problem hiding this comment.
Good idea — added the Grand Total assertion to test_pure_active_never_shutdown_cost_falls_back. This verifies that the Grand Total output tokens are 50.0K (matching the single session's model_metrics), catching any double-counting regression in grand_output.
Generated by Review Responder
Warning
⚠️ Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
astral.sh
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "astral.sh"See Network Configuration for more information.
| has_active_stats = ( | ||
| s.last_resume_time is not None | ||
| or s.active_user_messages > 0 | ||
| or s.active_output_tokens > 0 |
There was a problem hiding this comment.
has_active_stats ignores active_model_calls. A resumed session can have post-shutdown assistant.turn_start events (so active_model_calls > 0) while active_user_messages/active_output_tokens remain 0 (e.g., missing/non-int outputTokens). In that case this code will incorrectly fall back to totals and display the wrong "since last shutdown" model-call count. Include s.active_model_calls > 0 in the has_active_stats predicate (matching what you later read/display).
| or s.active_output_tokens > 0 | |
| or s.active_output_tokens > 0 | |
| or s.active_model_calls > 0 |
There was a problem hiding this comment.
Good catch. Applied the suggested fix — added s.active_model_calls > 0 to the has_active_stats predicate in both render_live_sessions (line 178) and _render_active_section (line 840). This ensures that a resumed session with post-shutdown assistant.turn_start events but zero active_user_messages/active_output_tokens correctly uses the active fields instead of falling back to totals.
Generated by Review Responder
Warning
⚠️ Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
astral.sh
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "astral.sh"See Network Configuration for more information.
There was a problem hiding this comment.
Quality Gate: APPROVE ✅
Low-impact bug fix with good test coverage. Auto-approving for merge.
Evaluated:
- Adds
has_active_statsfallback to_render_active_sectionandrender_cost_view, matching the existing pattern inrender_live_sessions— all three instances use identical condition logic - Two meaningful regression tests verify the fallback for both the default full-summary view and cost view
- Only touches display/rendering logic in
report.py— no changes to data models, parsers, or API contracts - CI passing (check, CodeQL, analyze, review all green)
Impact: LOW — display-only fix for an edge case where pure-active sessions showed zeros instead of totals.
Summary
Fixes #132 — the default view (
copilot-usage) showed all 0s for the "Active Sessions (Since Last Shutdown)" section when a session had never been shutdown.Root cause
_render_active_sectionandrender_cost_viewalways used theactive_*fields directly (active_model_calls,active_user_messages,active_output_tokens). For pure-active sessions where these fields are 0 (e.g. older parser output or directSessionSummaryconstruction), the display showed zeros while the total counters andmodel_metricsheld the real data.Meanwhile,
render_live_sessionsalready had ahas_active_statsfallback that correctly displayed totals in this scenario.Fix
Added the same
has_active_statsfallback to both_render_active_sectionandrender_cost_view:active_*fields are all zero andlast_resume_timeisNone, fall back tomodel_calls,user_messages, and_estimated_output_tokens()active_*field is non-zero orlast_resume_timeis set (resumed sessions), use theactive_*fields as beforeThis makes all three views (
render_live_sessions,_render_active_section,render_cost_view) consistent.Tests
test_pure_active_never_shutdown_falls_back_to_totals— verifies the default full-summary view shows totals (58 model calls, 382 user msgs, 204.0K tokens) whenactive_*are 0test_pure_active_never_shutdown_cost_falls_back— verifies the cost view's "↳ Since last shutdown" row shows totals whenactive_*are 0All 430 tests pass. Coverage: 98%.