From 3b3d3072b723fbe9bd9d7688745ed3cb64e3b301 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 22 Mar 2026 18:03:53 +0000 Subject: [PATCH] refactor: extract _effective_stats to deduplicate active-vs-totals branching (#242) Add frozen _EffectiveStats dataclass and _effective_stats() helper that returns active-period stats when available, otherwise session totals. Replace three duplicated if/else branches in render_live_sessions, _render_active_section, and render_cost_view with calls to _effective_stats(). Add unit tests covering both branches (active period and session totals) plus frozen-dataclass immutability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/copilot_usage/report.py | 67 ++++++++++++++++-------------- tests/copilot_usage/test_report.py | 63 ++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 32 deletions(-) diff --git a/src/copilot_usage/report.py b/src/copilot_usage/report.py index 9cf0c5c..0678901 100644 --- a/src/copilot_usage/report.py +++ b/src/copilot_usage/report.py @@ -126,6 +126,30 @@ def _has_active_period_stats(session: SessionSummary) -> bool: ) +@dataclass(frozen=True) +class _EffectiveStats: + """Active-period stats when available, otherwise session totals.""" + + model_calls: int + user_messages: int + output_tokens: int + + +def _effective_stats(session: SessionSummary) -> _EffectiveStats: + """Return active-period stats if available, otherwise session totals.""" + if _has_active_period_stats(session): + return _EffectiveStats( + model_calls=session.active_model_calls, + user_messages=session.active_user_messages, + output_tokens=session.active_output_tokens, + ) + return _EffectiveStats( + model_calls=session.model_calls, + user_messages=session.user_messages, + output_tokens=_estimated_output_tokens(session), + ) + + @dataclass(frozen=True) class _SessionTotals: """Aggregated totals across a list of sessions.""" @@ -218,18 +242,10 @@ def render_live_sessions( model = s.model or "—" running = _format_session_running_time(s) - if _has_active_period_stats(s): - # Resumed/active session with post-resume stats (even when 0) - messages = str(s.active_user_messages) - output_tok = s.active_output_tokens - est_cost = _estimate_premium_cost(s.model, s.active_model_calls) - else: - # Pure-active (never shut down): totals are already in model_metrics - messages = str(s.user_messages) - output_tok = _estimated_output_tokens(s) - est_cost = _estimate_premium_cost(s.model, s.model_calls) - - tokens = format_tokens(output_tok) + stats = _effective_stats(s) + messages = str(stats.user_messages) + est_cost = _estimate_premium_cost(s.model, stats.model_calls) + tokens = format_tokens(stats.output_tokens) cwd = s.cwd or "—" table.add_row( @@ -879,20 +895,10 @@ def _render_active_section( model = s.model or "—" running = _format_session_running_time(s) - # Use active_* fields when they are populated (resumed sessions - # or pure-active sessions processed by the current parser). - # Fall back to session totals for older or externally-constructed - # SessionSummary objects whose active_* fields may still be at - # their defaults (the current parser always populates active_* - # for pure-active sessions via build_session_summary). - if _has_active_period_stats(s): - model_calls = str(s.active_model_calls) - user_msgs = str(s.active_user_messages) - output_tokens = format_tokens(s.active_output_tokens) - else: - model_calls = str(s.model_calls) - user_msgs = str(s.user_messages) - output_tokens = format_tokens(_estimated_output_tokens(s)) + stats = _effective_stats(s) + model_calls = str(stats.model_calls) + user_msgs = str(stats.user_messages) + output_tokens = format_tokens(stats.output_tokens) table.add_row( name, @@ -1000,13 +1006,10 @@ def render_cost_view( grand_model_calls += s.model_calls if s.is_active: + stats = _effective_stats(s) + cost_calls = stats.model_calls + cost_tokens = stats.output_tokens has_active = _has_active_period_stats(s) - if has_active: - cost_calls = s.active_model_calls - cost_tokens = s.active_output_tokens - else: - cost_calls = s.model_calls - cost_tokens = _estimated_output_tokens(s) est = _estimate_premium_cost(s.model, cost_calls) table.add_row( " ↳ Since last shutdown", diff --git a/tests/copilot_usage/test_report.py b/tests/copilot_usage/test_report.py index 61d23d9..a8efdc1 100644 --- a/tests/copilot_usage/test_report.py +++ b/tests/copilot_usage/test_report.py @@ -24,6 +24,7 @@ _aggregate_model_metrics, _build_event_details, _compute_session_totals, + _effective_stats, _estimate_premium_cost, _event_type_label, _filter_sessions, @@ -2621,6 +2622,68 @@ def test_returns_false_for_pure_active_never_shutdown(self) -> None: assert _has_active_period_stats(session) is False +class TestEffectiveStats: + """Tests for the _effective_stats helper.""" + + def test_returns_active_stats_when_active_period(self) -> None: + """Session with active-period stats returns active_* values.""" + now = datetime.now(tz=UTC) + session = SessionSummary( + session_id="resumed-session-1234", + is_active=True, + start_time=now - timedelta(hours=2), + last_resume_time=now - timedelta(minutes=5), + user_messages=50, + model_calls=30, + active_user_messages=3, + active_output_tokens=500, + active_model_calls=2, + model_metrics={ + "gpt-4": ModelMetrics( + usage=TokenUsage(outputTokens=9999), + ), + }, + ) + stats = _effective_stats(session) + assert stats.model_calls == 2 + assert stats.user_messages == 3 + assert stats.output_tokens == 500 + + def test_returns_session_totals_when_no_active_period(self) -> None: + """Pure-active session without active_* counters falls back to totals.""" + session = SessionSummary( + session_id="pure-active-1234", + is_active=True, + start_time=datetime.now(tz=UTC) - timedelta(minutes=10), + user_messages=8, + model_calls=5, + active_user_messages=0, + active_output_tokens=0, + active_model_calls=0, + model_metrics={ + "gpt-4": ModelMetrics( + usage=TokenUsage(outputTokens=1200), + ), + }, + ) + stats = _effective_stats(session) + assert stats.model_calls == 5 + assert stats.user_messages == 8 + assert stats.output_tokens == 1200 + + def test_frozen_dataclass(self) -> None: + """_EffectiveStats is frozen (immutable).""" + session = SessionSummary( + session_id="frozen-test-1234", + is_active=True, + user_messages=1, + model_calls=1, + ) + stats = _effective_stats(session) + with pytest.raises(AttributeError): + stats.model_calls = 99 # type: ignore[misc] + + class TestComputeSessionTotals: """Tests for _compute_session_totals helper."""