fix(cli): preserve explicit T00:00:00 in --until (#870)#872
Merged
Conversation
Replace click.DateTime for --until with a custom _DateTimeOrDate ParamType that distinguishes date-only from full-datetime inputs. _normalize_until now only expands to end-of-day when the user supplied a date without a time component (has_explicit_time=False). An explicit --until 2025-01-15T00:00:00 is left as-is, giving strict before-midnight semantics. --since is unaffected. Tests cover: - _DateTimeOrDate param type parsing (date-only, datetime, midnight, invalid) - _normalize_until with _ParsedDateArg (parametrised 3 cases) - CLI-level regression via CliRunner for summary and cost commands - --since sanity check Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes the CLI --until handling so that an explicitly provided midnight timestamp (YYYY-MM-DDT00:00:00) is preserved (strict cutoff), while date-only inputs (YYYY-MM-DD) continue to expand to end-of-day for inclusive “whole day” behavior.
Changes:
- Added a
_ParsedDateArgcarrier and a custom Click param type_DateTimeOrDateto distinguish date-only vs datetime input for--until. - Updated
_normalize_until/_validate_since_untilto only expand midnight when the user did not supply an explicit time. - Expanded unit + CLI regression tests to cover explicit-midnight boundary inclusion/exclusion for
summaryandcost.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/copilot_usage/cli.py |
Introduces _DateTimeOrDate + _ParsedDateArg and updates --until parsing/normalization semantics. |
tests/copilot_usage/test_cli.py |
Updates existing tests and adds new unit/CLI regression coverage for issue #870 behavior. |
- Derive _DateTimeOrDate._try_parse formats from shared _FORMAT_SPECS constant instead of hardcoding, creating a single source of truth with _DATE_FORMATS - Reword --until help text on summary and cost commands to describe timestamp cutoff semantics accurately - Fix docstring count mismatch (four → three) in TestNormalizeUntilParametrised Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Commit pushed:
|
- Replace standalone triple-quoted docstring on _FORMAT_SPECS with # comment (B018) - Rename _DateTimeOrDate.name from 'datetime' to 'datetime-or-date' - Fix --until help text on cost command to match inclusive semantics - Unify --until help text wording between summary and cost commands Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Commit pushed:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Quality Gate: APPROVED (medium-impact bug fix)
Evaluated across code quality and blast radius dimensions:
- Code quality: Well-structured fix introducing
_ParsedDateArg(frozen dataclass) and_DateTimeOrDate(ClickParamType) to distinguish date-only from datetime--untilinputs. Follows all coding guidelines (strict typing,isinstanceat I/O boundary only,Finalconstants, frozen dataclass withslots=True). - Tests: 233 lines of new/updated tests covering the custom param type, parametrized
_normalize_untilbehavior, and CLI-level regression tests for bothsummaryandcostcommands. All meaningful — no no-ops. - Impact: Medium — focused fix to
--untilparsing incli.py. No changes to data models, parsers, or external API contracts.--sinceis unaffected. All existing tests updated to use new type. - CI: All 10 checks passing.
Auto-approving for merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #870
Problem
_normalize_untilexpanded any midnight datetime to end-of-day (23:59:59.999999), regardless of whether the user typed--until 2025-01-15(date-only) or--until 2025-01-15T00:00:00(explicit midnight). Becauseclick.DateTimediscards the original string format, the function could not distinguish the two cases.A user who typed
--until 2025-01-15T00:00:00intending "strictly before midnight" would silently get all January 15 sessions included.Solution
_ParsedDateArgfrozen dataclass carryingvalue: datetimeandhas_explicit_time: bool.click.DateTimefor--untilon bothsummaryandcostcommands with a custom_DateTimeOrDate(click.ParamType)that parses%Y-%m-%das date-only (has_explicit_time=False) and%Y-%m-%dT%H:%M:%Sas explicit time (has_explicit_time=True)._normalize_untilto only expand to end-of-day whenhas_explicit_time is False.--sinceparameters remain unchanged (usingclick.DateTime).Tests Added
TestDateTimeOrDateParamType— unit tests for the custom Click param type (date-only, datetime, explicit midnight, invalid input, datetime passthrough).TestNormalizeUntilParametrised— parametrised tests covering the 3 key cases: date-only expanded, explicit midnight not expanded, non-midnight unchanged.TestIssue870ExplicitMidnight— CLI-level regression tests viaCliRunnerfor bothsummaryandcostcommands, verifying boundary inclusion/exclusion and--sincesanity check.TestNormalizeUntil,TestNormalizeUntilNonUtcTimezone, andTestValidateSinceUntilto use_ParsedDateArg.All 1258 unit tests and 86 e2e tests pass. Coverage remains at ~99%.