-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Root Cause
All three of the "complex" public render functions in report.py accept an optional target_console: Console | None = None parameter that lets callers control where output goes:
render_full_summary(sessions, *, target_console)— ✅ has itrender_cost_view(sessions, *, since, until, target_console)— ✅ has itrender_session_detail(events, summary, *, target_console)— ✅ has it
But the two "simple" public render functions do not:
render_live_sessions(sessions)— ❌ creates its ownConsole()internally, no overriderender_summary(sessions, since, until)— ❌ creates its ownConsole()internally, no override
Impact
1. Inconsistent API contract. All five functions are part of the same module and documented in __all__. Callers can't treat them uniformly.
2. Forces test workaround. Tests that need to capture output from these functions must monkeypatch Console itself rather than passing a pre-configured console:
# test_report.py — forced workaround
with patch("copilot_usage.report.Console", return_value=console):
render_live_sessions(sessions)Every other render function is tested cleanly by passing target_console=console.
3. Prevents caller-level console sharing. cli.py creates a Console() instance in _interactive_loop and passes it to render_full_summary, render_cost_view, and render_session_detail. But it can't pass the same console to render_live_sessions or render_summary — each invocation creates a fresh Console() with potentially different settings (width, color support, etc.).
Spec
Add target_console: Console | None = None as a keyword-only parameter to both functions:
def render_live_sessions(
sessions: list[SessionSummary],
*,
target_console: Console | None = None,
) -> None:
console = target_console or Console()
...
def render_summary(
sessions: list[SessionSummary],
since: datetime | None = None,
until: datetime | None = None,
*,
target_console: Console | None = None,
) -> None:
console = target_console or Console()
...- Both functions already have an internal
console = Console()that just needs to be replaced with the pattern above. - The
*separator is already used in all other target_console-accepting functions; follow the same convention. - No callers in
cli.pycurrently passtarget_consoleto these, so there are no call-site changes required (backward-compatible addition). - Update
__all__is not needed (no API surface change).
Testing Requirement
- Remove the
patch("copilot_usage.report.Console", ...)workaround fromtest_report.py's_capture_outputhelper and replace with a directtarget_console=argument. - Add tests that pass an explicit
target_consoleto both functions and assert output is captured correctly — analogous to existing tests forrender_full_summaryandrender_cost_view. - Regression: all existing render tests must continue to pass.
Generated by Code Health Analysis · ◷