Skip to content

fix(backend): weekly score window spans 8 days instead of the documented 7 (off-by-one)#10015

Merged
kodjima33 merged 1 commit into
BasedHardware:mainfrom
arhxam:fix/weekly-score-window-off-by-one
Jul 21, 2026
Merged

fix(backend): weekly score window spans 8 days instead of the documented 7 (off-by-one)#10015
kodjima33 merged 1 commit into
BasedHardware:mainfrom
arhxam:fix/weekly-score-window-off-by-one

Conversation

@arhxam

@arhxam arhxam commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Problem

get_scores() (database/action_items.py) documents its weekly window as "tasks created in the 7 days ending on that date", but builds it a day too wide:

day_start  = day
day_end    = day + timedelta(days=1)
week_start = day - timedelta(days=7)
...
weekly_q = col.where(FieldFilter('created_at', '>=', week_start)).where(FieldFilter('created_at', '<', day_end))

created_at ∈ [day-7, day+1) spans 8 calendar days (day-7 … day inclusive). So every task created on the day-7 date is pulled into the weekly totals, inflating weekly.total_tasks and skewing weekly.score.

Root cause

The adjacent daily window is built as [day, day+1) — exactly one day. That establishes the intended construction: an N-day window ending on day is [day-(N-1), day+1). For the 7-day weekly window that is [day-6, day+1), but the code uses day-7.

Concrete: for date="2026-07-19", the query lower bound is 2026-07-12T00:00Z, so a task created 2026-07-12 is counted — even though "7 days ending 2026-07-19" is 2026-07-13 … 2026-07-19.

Fix

-week_start = day - timedelta(days=7)
+week_start = day - timedelta(days=6)

This preserves the created_at field choice — the "matches Rust backend which uses created_at" comment is about the field, not the window size, and the Rust backend (desktop/macos/Backend-Rust) has no weekly-score window at all — and only corrects the span from 8 days to 7.

Tests

The existing get_scores coverage (test_desktop_migration.py::TestScoreComputation) only asserted the weekly query uses the created_at field, never the window bounds. Added test_weekly_window_spans_seven_days_ending_on_date, which captures the FieldFilter calls and asserts the created_at >= lower bound for date=2026-07-19 is 2026-07-13 (day-6), not 2026-07-12 (day-7).

Verification

Exercised the real get_scores with a mocked db + a FieldFilter tracker:

fixed:    created_at >= 2026-07-13T00:00Z   (window spans 7 days)
reverted: created_at >= 2026-07-12T00:00Z   (window spans 8 days) -> new assertion fails

black --line-length 120 --check database/action_items.py tests/unit/test_desktop_migration.py → clean.


🤖 Generated with Claude Code

Review in cubic

get_scores() documents its weekly window as "tasks created in the 7 days
ending on that date", but builds it as:

    day_end   = day + timedelta(days=1)
    week_start = day - timedelta(days=7)   # created_at in [day-7, day+1)

[day-7, day+1) spans 8 calendar days (day-7 .. day inclusive), so every task
created on the day-7 date is counted in the weekly totals, inflating
weekly.total_tasks and skewing weekly.score.

The adjacent daily window is built as [day, day+1) — exactly one day — which
fixes the intended construction: an N-day window ending on `day` is
[day-(N-1), day+1). For the 7-day weekly window that is [day-6, day+1).

Fix: week_start = day - timedelta(days=6). This preserves the created_at
field choice (the "matches Rust backend which uses created_at" comment is
about the field, not the window; the Rust backend has no weekly-score
window) and only corrects the span from 8 to 7 days.

Test: the existing get_scores coverage only asserted the query uses the
created_at field, never the window bounds. Added
test_weekly_window_spans_seven_days_ending_on_date, which captures the
FieldFilter calls and asserts the created_at lower bound for date=2026-07-19
is 2026-07-13 (day-6), not 2026-07-12 (day-7).

Verification: exercised the real get_scores with a mocked db + FieldFilter
tracker -> created_at >= 2026-07-13 (7-day span); reverting the one-line fix
yields 2026-07-12 (8-day span) and the new assertion fails.
black --line-length 120 --check: clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Git-on-my-level Git-on-my-level added candidate-clean-pr Small, safe, well-scoped, well-tested contribution; model clean PR needs-maintainer-review Needs a human maintainer to sign off before merge labels Jul 19, 2026

@Git-on-my-level Git-on-my-level left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused fix — this looks like the right direction.

I reviewed the weekly score window change and the added regression coverage. The previous [day-7, day+1) range did span 8 calendar days for a "7 days ending on that date" window, and changing the lower bound to day - timedelta(days=6) makes the query match the documented inclusive 7-day window. The new test also checks the actual created_at >= bound, which covers the off-by-one rather than only checking the queried field.

I did not find a blocking code issue in this diff. Because this touches backend action-item scoring/user-data logic and the author is a first-time contributor, I’m leaving this as a positive maintainer signal rather than a formal approval; a human maintainer should still do the final merge decision.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Backend off-by-one fix (weekly score window 8→7 days) with test. Approve-only — no CI checks ran for contributor.

@kodjima33
kodjima33 merged commit 1abb045 into BasedHardware:main Jul 21, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

candidate-clean-pr Small, safe, well-scoped, well-tested contribution; model clean PR needs-maintainer-review Needs a human maintainer to sign off before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants