feat: add --from/--to date-range filtering for daily and monthly views - #224
Open
TobiWo wants to merge 4 commits into
Open
feat: add --from/--to date-range filtering for daily and monthly views#224TobiWo wants to merge 4 commits into
TobiWo wants to merge 4 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds ChangesDate Range Filtering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as CLI args
participant Settings as Settings
participant Aggregator as UsageAggregator
participant TableView as daily/monthly table view
CLI->>Settings: parse --from/--to
Settings->>Settings: validate range for view
CLI->>Aggregator: pass date_from/date_to
Aggregator->>TableView: aggregate with bounds
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/tests/test_aggregator.py (1)
403-516: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding non-UTC timezone coverage for
_range_bounds().Every new test pins
timezone="UTC", so the timezone-aware bound construction in_range_bounds()(which builds boundaries in the display timezone viamake_aware) is never exercised against entries in a different zone. A test with e.g.timezone="America/New_York"would confirm that a local-day boundary correctly includes/excludes UTC-stamped entries near midnight.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tests/test_aggregator.py` around lines 403 - 516, Add a test that exercises UsageAggregator._range_bounds() with a non-UTC timezone, since the current cases only use timezone="UTC" and do not verify make_aware-boundary behavior in a local zone. Use the existing UsageAggregator, _range_bounds, and aggregate_daily/aggregate_monthly patterns to assert that a timezone like America/New_York correctly includes or excludes UTC-stamped UsageEntry values near midnight based on the local-day boundary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/claude_monitor/core/settings.py`:
- Around line 277-292: The last-used replay in load_with_last_used() is
comparing raw CLI flag names to cls.model_fields, so the aliased --from/--to
inputs are not normalized to date_from/date_to and can be overwritten by stale
saved values. Update the CLI field collection logic in load_with_last_used()
(and any helper it uses) to map aliases to their model field names before
deciding which settings were provided, using the date_from/date_to fields and
their from/to aliases as the reference points. Ensure explicit CLI range values
always take precedence over previously saved settings during replay.
- Around line 495-510: The date validation in the settings logic still accepts
non-canonical inputs and compares raw strings, so update the validation in the
code around the date parsing loop to parse each of `self.date_from` and
`self.date_to` once, require that formatting back with `strftime(...)` matches
the original value, and then use the parsed date objects for the `self.date_from
> self.date_to` check instead of comparing the original strings.
In `@src/claude_monitor/data/aggregator.py`:
- Around line 342-393: The `_range_bounds()` filtering logic in `Aggregator` is
using calendar start/end datetimes that don’t match how period keys are
generated, so boundary entries can be mis-bucketed or dropped. Update the range
filtering to follow the same semantics as the period-key logic used later in
aggregation (including `reset_hour` and timezone-aware daily/monthly keying), or
filter directly against the computed period keys in the aggregation flow. Make
sure the fix is applied consistently in `_range_bounds()` and the surrounding
aggregation path so the bounds and keys stay aligned.
---
Nitpick comments:
In `@src/tests/test_aggregator.py`:
- Around line 403-516: Add a test that exercises UsageAggregator._range_bounds()
with a non-UTC timezone, since the current cases only use timezone="UTC" and do
not verify make_aware-boundary behavior in a local zone. Use the existing
UsageAggregator, _range_bounds, and aggregate_daily/aggregate_monthly patterns
to assert that a timezone like America/New_York correctly includes or excludes
UTC-stamped UsageEntry values near midnight based on the local-day boundary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2a003fc-334e-47b3-a443-bfb1fd2232d5
📒 Files selected for processing (6)
README.mdsrc/claude_monitor/cli/main.pysrc/claude_monitor/core/settings.pysrc/claude_monitor/data/aggregator.pysrc/tests/test_aggregator.pysrc/tests/test_settings.py
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/claude_monitor/core/settings.py (1)
495-523: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReplay saved
viewbefore date-range validation.load_with_last_used()applies persisted values after themode="after"validator runs, so--from/--toare still checked against the defaultrealtimeview and fail unless--view daily|monthlyis repeated on the same invocation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/claude_monitor/core/settings.py` around lines 495 - 523, The date-range validation in settings is running before the persisted view is restored, so `--from` and `--to` are being validated against the default `realtime` mode instead of the saved `view`. Update the `load_with_last_used()` flow and the `mode="after"` validator in `src/claude_monitor/core/settings.py` so the replayed `view` is applied before date validation, ensuring the `date_pattern`/`expected` logic in the date-range check uses the effective view.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/claude_monitor/core/settings.py`:
- Around line 495-523: The date-range validation in settings is running before
the persisted view is restored, so `--from` and `--to` are being validated
against the default `realtime` mode instead of the saved `view`. Update the
`load_with_last_used()` flow and the `mode="after"` validator in
`src/claude_monitor/core/settings.py` so the replayed `view` is applied before
date validation, ensuring the `date_pattern`/`expected` logic in the date-range
check uses the effective view.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a7a44d14-ca7b-45fa-827d-3914c6669490
📒 Files selected for processing (4)
src/claude_monitor/core/settings.pysrc/claude_monitor/data/aggregator.pysrc/tests/test_aggregator.pysrc/tests/test_settings.py
🚧 Files skipped from review as they are similar to previous changes (3)
- src/claude_monitor/data/aggregator.py
- src/tests/test_aggregator.py
- src/tests/test_settings.py
TobiWo
force-pushed
the
feature/add-range-flags-for-view
branch
from
July 1, 2026 10:27
7c9f70b to
9e3a6f4
Compare
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TobiWo
force-pushed
the
feature/add-range-flags-for-view
branch
from
July 1, 2026 10:31
9e3a6f4 to
994aaa6
Compare
Author
|
@Maciek-roboblog All comments raised by coderabbit have been adressed. |
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
1 participant
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.
Summary
Adds
--from/--toto restrict thedailyandmonthlytables to an inclusive date range.Format follows the view:
dailytakesYYYY-MM-DD,monthlytakesYYYY-MM. Either bound works alone. The flags only apply to those two views — using them elsewhere is a validation error.Most of the plumbing already existed.
UsageAggregatoracceptsstart_date/end_dateand filters on the entry timestamp; the publicaggregate()just never passed them. So this connects the flags to that filter instead of writing new aggregation. The totals row and period header follow automatically, since both come from the rows the aggregator returns.One thing reviewers should know: these views read
~/.claude/projects/, which Claude Code prunes aftercleanupPeriodDays(default 30). A range older than that returns no rows, not an error. The README explains how to raise the retention for longer monthly history.Changes: the validated
--from/--tofields onSettings, a_range_bounds()helper that builds timezone-aware inclusive bound (month-end viacalendar.monthrange), tests for both views including boundary inclusivity and the error cases, and the README entries.Closes #223
Summary by CodeRabbit
--from/--todate-range filters for daily and monthly table views (aliases supported).