fix(cli): reapply date-bound validation - #1608
Conversation
Reapply documented date-bound validation and reject impossible calendar dates in CLI and configuration files. Co-authored-by: dok123 <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe change validates ChangesDate-bound validation
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR reapplies documented date-bound validation without any supplied evidence of a current correctness, security, availability, or deployment risk; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant CLIUser
participant CLIParser
participant normalize_date_bound
participant ConfigContext
CLIUser->>CLIParser: provide --since or --until
CLIParser->>normalize_date_bound: validate date bound
normalize_date_bound-->>CLIParser: normalized YYYYMMDD or validation failure
ConfigContext->>normalize_date_bound: validate configured date bound
normalize_date_bound-->>ConfigContext: normalized value or date-bound error
CLIParser-->>CLIUser: parsed option or error
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
ccusage-guide | 6d00c28 | Commit Preview URL Branch Preview URL |
Aug 15 2026, 06:50 PM |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
This PR can reject a valid effective configuration bound when an overridden lower-precedence value is invalid.
Reviewed changes — Reviewed the complete date-bound validation reapplication across the CLI parser, shared normalization, configuration loading, tests, and documentation.
- CLI validation — Restricts
--sinceand--untilto the documented formats and real calendar dates, then normalizes them toYYYYMMDD. - Configuration validation — Applies the same validation to raw config values and preserves command discovery for value-taking root options.
- Regression coverage and documentation — Adds validation tests, normalization tests, and user-facing documentation for accepted formats and rejection behavior.
@v0 or keep the SHA fresh with Dependabot ｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using GPT Luna (free via Pullfrog for OSS) ｜ 𝕏
| /// so the check runs here and surfaces through `config_error` for every | ||
| /// command that applies shared options, not just the pi-store readers. | ||
| fn detect_date_bound_error(&self) -> Option<String> { | ||
| self.option_maps().into_iter().find_map(|options| { |
There was a problem hiding this comment.
This scans the maps in lower-to-higher precedence order and returns the first invalid value, while apply_config_to_shared() lets later maps override earlier ones. For example, an invalid defaults.since combined with a valid commands.daily.since now rejects ccusage daily before the valid effective bound is applied, so please validate the effective bound or otherwise ignore invalid values that are shadowed by a higher-precedence map.
ccusage
@ccusage/ccusage-darwin-arm64
@ccusage/ccusage-darwin-x64
@ccusage/ccusage-linux-arm64
@ccusage/ccusage-linux-x64
@ccusage/ccusage-win32-x64
commit: |
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="rust/crates/ccusage-config/src/config.rs">
<violation number="1" location="rust/crates/ccusage-config/src/config.rs:69">
P2: Config date-bound validation ignores config-layer and CLI precedence, so an invalid bound in a lower-precedence layer fails a command even when a valid value would override it. `detect_date_bound_error` scans every `option_maps()` layer and reports the first invalid `since`/`until`, while `apply_shared_options` deliberately drops unnormalizable low-layer values in favor of higher-precedence ones. Because `config_error()` is consulted in `parser.rs` before `apply_shared` and before the CLI `--since`/`--until` is parsed, an invalid `since` under `defaults` (or an unconsumed layer) now hard-fails commands that never apply it, including `statusline`/`blocks`, and can no longer be rescued by a valid command-specific config or CLI override. Validate against the effective merged value (the value that actually wins after layering) rather than each raw layer.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| /// parser gives `--since` / `--until`. `apply_shared_options` cannot fail, | ||
| /// so the check runs here and surfaces through `config_error` for every | ||
| /// command that applies shared options, not just the pi-store readers. | ||
| fn detect_date_bound_error(&self) -> Option<String> { |
There was a problem hiding this comment.
P2: Config date-bound validation ignores config-layer and CLI precedence, so an invalid bound in a lower-precedence layer fails a command even when a valid value would override it. detect_date_bound_error scans every option_maps() layer and reports the first invalid since/until, while apply_shared_options deliberately drops unnormalizable low-layer values in favor of higher-precedence ones. Because config_error() is consulted in parser.rs before apply_shared and before the CLI --since/--until is parsed, an invalid since under defaults (or an unconsumed layer) now hard-fails commands that never apply it, including statusline/blocks, and can no longer be rescued by a valid command-specific config or CLI override. Validate against the effective merged value (the value that actually wins after layering) rather than each raw layer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At rust/crates/ccusage-config/src/config.rs, line 69:
<comment>Config date-bound validation ignores config-layer and CLI precedence, so an invalid bound in a lower-precedence layer fails a command even when a valid value would override it. `detect_date_bound_error` scans every `option_maps()` layer and reports the first invalid `since`/`until`, while `apply_shared_options` deliberately drops unnormalizable low-layer values in favor of higher-precedence ones. Because `config_error()` is consulted in `parser.rs` before `apply_shared` and before the CLI `--since`/`--until` is parsed, an invalid `since` under `defaults` (or an unconsumed layer) now hard-fails commands that never apply it, including `statusline`/`blocks`, and can no longer be rescued by a valid command-specific config or CLI override. Validate against the effective merged value (the value that actually wins after layering) rather than each raw layer.</comment>
<file context>
@@ -29,25 +29,64 @@ pub struct ConfigContext {
+ /// parser gives `--since` / `--until`. `apply_shared_options` cannot fail,
+ /// so the check runs here and surfaces through `config_error` for every
+ /// command that applies shared options, not just the pi-store readers.
+ fn detect_date_bound_error(&self) -> Option<String> {
+ self.option_maps().into_iter().find_map(|options| {
+ [
</file context>
1 participant
Reapply the date-bound validation removed by the preceding revert.
This restores the documented CLI and configuration validation while recording dok123 as a co-author of the reapplication.
Testing: pre-push clippy, treefmt, gitleaks, and cargo test passed.
Summary by cubic
Reapplies strict validation of
--since/--untildate bounds in the CLI and config. Previously we accepted loosely formatted values by stripping dashes, which could silently change which rows a report kept; now onlyYYYY-MM-DDorYYYYMMDDare accepted, dates must be real calendar dates, and invalid values exit with an error. The same check applies tosince/untilin config.DATE_BOUND_FORMATSfromccusage-cliand updatesnormalize_date_boundto returnOption<String>after validating and normalizing toYYYYMMDD.ccusage-cli-parser) errors with “Expected YYYY-MM-DD or YYYYMMDD” and rejects undocumented spellings.ccusage-config) validatessince/untilearly, surfaces a single config error, and keeps pi-store errors gated to readers; valid bounds are normalized before applying shared options.Bold sections aren't required here, but migration notes follow.
YYYY-MM-DDorYYYYMMDD; invalid values now fail with a non-zero exit.since/untilas strings using those formats; non-strings or invalid dates fail config validation.ccusage-cli: update call sites to handlenormalize_date_boundreturningOption<String>.Written for commit 6d00c28. Summary will update on new commits.
Summary by CodeRabbit
YYYY-MM-DDandYYYYMMDDformats.