Conversation
There was a problem hiding this comment.
Pull request overview
Fixes an edge case in report rendering where sessions with active_model_calls > 0 (but active_user_messages == 0 and active_output_tokens == 0) were incorrectly treated as having no active-period stats, causing the UI to fall back to historical totals.
Changes:
- Update active-period predicates to include
active_model_calls > 0in live sessions, full summary active section, and cost view. - Add regression tests covering the “model calls only” active-period scenario across all three views.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/copilot_usage/report.py |
Includes active_model_calls > 0 in the active-period detection predicates so resumed/active sessions take the correct rendering path. |
tests/copilot_usage/test_report.py |
Adds edge-case tests ensuring the renderers use active-period fields when only active_model_calls is non-zero. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
Changes reviewed:
src/copilot_usage/report.py: 3 one-line additions ofor s.active_model_calls > 0tohas_active_stats/has_activepredicates inrender_live_sessions,_render_active_section, andrender_cost_viewtests/copilot_usage/test_report.py: 3 new edge-case tests coveringactive_model_calls > 0withactive_user_messages=0,active_output_tokens=0,last_resume_time=None
Assessment:
- Code quality: Good — follows existing predicate pattern exactly, logically correct fix for issue #196
- Impact: LOW — display-only predicate fix in rendering layer, no API/data model/security changes
- Tests: Meaningful coverage of the exact edge case across all 3 rendering paths (103 lines of tests for 3 lines of production code)
Low-impact bug fix with thorough test coverage. Auto-approving for merge.
microsasa
added
aw-review-response-attempted
|
Commit pushed:
|
microsasa
force-pushed
the
fix/196-active-model-calls-predicate-2ade8f9efa9a6696
branch
from
2dd15af to
a49e937
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
microsasa
added
aw-review-response-2
|
Commit pushed:
|
microsasa
force-pushed
the
fix/196-active-model-calls-predicate-2ade8f9efa9a6696
branch
from
e847169 to
0de95c3
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…#196) Add s.active_model_calls > 0 to the has_active_stats predicate in render_live_sessions (live view and full summary) and the has_active predicate in render_cost_view. Without this, a resumed session with post-shutdown assistant.turn_start events but no user messages or output tokens would incorrectly fall back to historical totals. Add tests for the edge case: last_resume_time=None, active_model_calls > 0, active_user_messages=0, active_output_tokens=0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract duplicated has_active_stats/has_active predicate into a _has_active_period_stats() helper and reuse it across all three renderers (render_live_sessions, render_full_summary, render_cost_view) to prevent future drift when active_* fields change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add Output Tokens assertion to full summary edge-case test - Make Model Calls assertion exact (== instead of 'in') in cost view test - Add Output Tokens assertion to cost view edge-case test (cols[6]) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
microsasa
force-pushed
the
fix/196-active-model-calls-predicate-2ade8f9efa9a6696
branch
from
0de95c3 to
86fcc23
Compare
There was a problem hiding this comment.
Quality Gate: APPROVED ✅
What was evaluated:
- Bug fix adding missing
active_model_calls > 0condition to thehas_active_stats/has_activepredicates inreport.py(3 call sites) - Refactored duplicated inline predicates into shared
_has_active_period_stats()helper (DRY improvement) - 3 new edge-case tests covering the exact bug scenario (active_model_calls > 0, user_messages=0, output_tokens=0, last_resume_time=None) across
TestRenderLiveSessions,TestRenderFullSummary, andTestRenderCostView
Impact: LOW — Display/rendering logic only. No API contracts, data models, or security-sensitive code affected. 2 files changed (source + tests), net +111 lines.
Auto-approving for merge.
microsasa
deleted the
fix/196-active-model-calls-predicate-2ade8f9efa9a6696
branch
Closes #196
Problem
The
has_active_statspredicate inrender_live_sessions(both live view and full summary) and thehas_activepredicate inrender_cost_viewcheckedactive_user_messages > 0andactive_output_tokens > 0but did not checkactive_model_calls > 0.A resumed session can have post-shutdown
assistant.turn_startevents (soactive_model_calls > 0) whileactive_user_messagesandactive_output_tokensremain 0. In that case the code incorrectly fell back to historical totals instead of showing the active-period values.Fix
Added
s.active_model_calls > 0to all three predicate locations inreport.py:has_active_statsinrender_live_sessions(live view)has_active_statsin the full summary active sessions sectionhas_activeinrender_cost_viewTests
Added edge-case tests for
last_resume_time=None,active_model_calls > 0,active_user_messages=0,active_output_tokens=0to:TestRenderLiveSessionsTestRenderFullSummaryTestRenderCostViewAll 438 tests pass, coverage at 98%.
Warning
The following domain was blocked by the firewall during workflow execution:
astral.shTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.