Repository navigation
fix: collection period adjustment - #20
Conversation
|
@maximgorbatyuk please, cleanup / reopen this MR. 
|
a608816 to
775f7d8
Compare
drugoi
left a comment
drugoi
left a comment
There was a problem hiding this comment.
Code review (PR #20)
What looks good
- Copy in
interactive.rsand theWeek/Monthdoc comments now match the new rolling-window intent. Datelikeimport removal is correct after dropping calendar-week / month-start logic.- Tests were updated consistently with the implementation;
cargo testpasses.
Issue: off-by-one vs “last 7 / 30 days”
load_commits_for_window uses query_date_range(from, to), and date_range_bounds_in_timezone includes both from and to as full local calendar days (committed_at >= start_of(from) and < start_of(to.next_day())).
So with week_start = today - Duration::days(7) and to = today, the inclusive calendar span is 8 days (e.g. 2026-03-05 … 2026-03-12), not 7. Similarly today - 30 through today is 31 inclusive days in typical month boundaries (e.g. Feb 10 … Mar 12 in the test), not 30.
If the product intent is exactly seven / thirty calendar days ending today, the starts are usually today - 6 and today - 29 respectively (with matching test expectations and labels).
Worth fixing so CLI copy, AI ai_period text, and actual query windows stay aligned.
Nit (out of diff)
README.md still mentions this week / this month in the template-variable example; consider updating when you touch docs.
Week and Month periods used days(7)/days(30) offsets but date_range_bounds_in_timezone includes both endpoints as full calendar days, resulting in 8/31 day spans. Changed to days(6)/days(29) so the inclusive range is exactly 7/30 days as labeled.
775f7d8 to
65dbded
Compare
|
fixed, commits history is rewritten |
3 participants
'Week' meaning 'this week' -> last 7 days
'Month' meaning 'this month' -> last 30 days