-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Overview
Three independent but small code-quality issues that individually are nits but together form a meaningful cleanup. All are in src/copilot_usage/.
1. toolRequests uses a misleading default_factory (models.py line 107)
toolRequests: list[dict[str, object]] = Field(
default_factory=lambda: list[dict[str, object]]() # ← anti-pattern
)list[dict[str, object]]() calls the parameterized generic alias as a constructor, which happens to return [] — but this is a confusing side-effect of how Python generics work, not idiomatic code. It's also slower. The correct form is:
toolRequests: list[dict[str, object]] = Field(default_factory=list)Fix: Replace the lambda with default_factory=list.
2. model_token_map unnecessary indirection in build_session_summary (parser.py lines 191, 339–345)
In the active-session code path (no shutdown), model_token_map: dict[str, int] is declared as a variable that at most ever holds one entry, then immediately converted to active_metrics:
model_token_map: dict[str, int] = {} # declared at top of function
...
if model and total_output_tokens:
model_token_map[model] = total_output_tokens
active_metrics: dict[str, ModelMetrics] = {}
for m, tokens in model_token_map.items(): # loop over at-most-one-entry dict
active_metrics[m] = ModelMetrics(...)The intermediate dict adds cognitive overhead.
Fix: Remove model_token_map and build active_metrics directly:
active_metrics: dict[str, ModelMetrics] = {}
if model and total_output_tokens:
active_metrics[model] = ModelMetrics(usage=TokenUsage(outputTokens=total_output_tokens))3. cost CLI command inlines date filtering instead of delegating (cli.py lines 395–405)
The summary command delegates date filtering to render_summary, which calls _filter_sessions. The cost command instead manually re-implements filtering inline:
filtered = sessions
if since_aware is not None or until_aware is not None:
filtered = [
s for s in sessions
if s.start_time is not None
and (since_aware is None or s.start_time >= since_aware)
and (until_aware is None or s.start_time <= until_aware)
]
render_cost_view(filtered)If _filter_sessions logic ever changes (e.g. timezone handling, edge cases), the cost path won't get the update automatically.
Fix: Add since: datetime | None = None, until: datetime | None = None parameters to render_cost_view (in report.py), call _filter_sessions internally (same as render_summary does), and remove the inline filtering from cli.py::cost.
Testing requirement
- Item 1: No additional tests needed; existing tests cover the field. Verify with
pytest tests/copilot_usage/test_models.py -v. - Item 2: Add a unit test in
test_parser.pyfor an active session with a known model + token count that assertsactive_metricsis set correctly without intermediate dict. - Item 3: Add a test in
test_cli.py(ortest_report.py) that passessince/untildirectly torender_cost_viewand verifies filtering matches_filter_sessionsbehaviour; also verify thecostCLI command still filters correctly via the Click runner.
Generated by Code Health Analysis · ◷