Conversation
Extract a single _format_timedelta(td: timedelta) core helper and rewrite format_duration, _format_elapsed_since, and _format_detail_duration to delegate to it, eliminating duplicated hours/minutes/seconds decomposition logic. Add direct unit tests for _format_timedelta covering zero, negative, seconds-only, minutes, hours, and mixed durations. Update existing test expectations where the unified formatting now omits zero-value units (e.g. '1m 0s' → '1m', '1h 0m' → '1h'). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR consolidates the duration formatting logic in copilot_usage.report by introducing a single core _format_timedelta() helper and having existing duration-formatting helpers delegate to it, ensuring consistent output across the CLI report renderers.
Changes:
- Added
_format_timedelta(td: timedelta) -> stras the shared duration formatter (clamps negatives to0s, omits zero-value units). - Updated
format_duration(),_format_elapsed_since(), and_format_detail_duration()to delegate to_format_timedelta(). - Updated boundary expectations and added direct unit tests for
_format_timedelta.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/copilot_usage/report.py |
Introduces _format_timedelta() and routes existing duration formatting helpers through it for consistent formatting. |
tests/copilot_usage/test_report.py |
Updates expectations for zero/minute/hour boundaries and adds focused unit coverage for _format_timedelta(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Quality Gate: AUTO-APPROVED
Low-impact refactoring that consolidates three duration-formatting functions into a single _format_timedelta helper. No public API changes, no data model changes, no security concerns. 9 new unit tests with good edge-case coverage. All 8 CI checks green. Auto-approving for merge.
Closes #243
Changes
Extracted a single
_format_timedelta(td: timedelta) -> strcore helper that encapsulates the hours/minutes/seconds decomposition and formatting logic. The three existing duration-formatting functions now delegate to it:format_duration(ms)→_format_timedelta(timedelta(milliseconds=ms))_format_elapsed_since(start)→_format_timedelta(now - ensure_aware(start))_format_detail_duration(start, end)→ guards forNone, then_format_timedelta(end - start)The
format_durationpublic API signature is unchanged.Behavior change
The unified helper omits zero-value units (consistent with how
format_durationalready worked):60s→"1m"(was"1m 0s"in_format_detail_duration)3600s→"1h"(was"1h 0m"in_format_detail_duration)"0s"(was"0m 0s"in_format_elapsed_since)Tests
_format_timedeltacovering zero, negative, seconds-only, exact minute, exact hour, full h/m/s, and large durations