[REQ-002] Add completionsInRange and last7Count pure helpers#10
Conversation
markoub
left a comment
There was a problem hiding this comment.
Review: PR #10 — completionsInRange and last7Count pure helpers
Overall: this is solid work — clean implementation, well-tested, perfectly scoped. ✅ Ready to merge.
Correctness vs acceptance criteria
All five acceptance criteria from issue #5 are satisfied:
| Criterion | Result |
|---|---|
completionsInRange(habit, fromDay, toDay): number in src/habits.ts |
✅ |
last7Count(habit, today): number delegates to completionsInRange |
✅ |
Both helpers are pure (no DOM / localStorage / window) |
✅ |
| Tests cover zero completions, boundary dates, full & partial windows | ✅ |
npm test and npm run build pass |
✅ (CI green) |
Purity (src/habits.ts)
completionsInRange (habits.ts:75-83) is a one-liner .filter() on an in-memory array — perfectly pure. last7Count (habits.ts:88-91) delegates to shiftDay (UTC-safe date arithmetic, no browser APIs) and completionsInRange. Nothing here touches DOM, storage, or the clock. The golden rule in CLAUDE.md is upheld.
Implementation correctness
The boundary math in last7Count is right: shiftDay(today, -6) gives the date 6 days before today, making a closed window of exactly 7 days (today − 6 … today). The test at habits.test.ts:151 ("includes today and the oldest day of the 7-day window") confirms this with 2026-06-04 as the lower bound when today = "2026-06-10". ✅
ISO lexicographic comparison (d >= fromDay && d <= toDay) is valid for YYYY-MM-DD strings — good use of the existing convention already present in currentStreak.
Test coverage
10 new tests across two describe blocks — all edge cases called out in the acceptance criteria are hit:
completionsInRange: empty habit, strictly inside,fromDayboundary,toDayboundary, outside range.last7Count: empty habit, all 7 days, partial window, day −6 boundary, day −7 excluded.
The "excludes completions older than 7 days" test correctly pins 2026-06-03 (day −7, outside the window) and expects 0. ✅
Style & scope
- Diff touches exactly two files (
src/habits.ts,src/habits.test.ts) — focused and on-topic. - Single commit on
feat/issue-5-completions-in-range— conventional naming. - JSDoc style matches the existing multi-line blocks already present for
currentStreak(habits.ts:55-59) andbestStreak(habits.ts:75-78). The second line incompletionsInRange's doc ("ISO lexicographic order is used") records a non-obvious implementation invariant — justified per CLAUDE.md's "only add a comment when the WHY is non-obvious." - No new dependencies introduced.
Non-blocking nit (no action required)
shiftDay is unexported (habits.ts:49) and used internally by currentStreak, bestStreak, and now last7Count. That's fine for now. If a future issue requires date arithmetic in the UI layer it'll naturally be promoted then — no action needed here.
Verdict: All criteria met, CI green, mergeable. Labelling ready-to-merge and squash-merging.
Generated by Claude Code
Summary
completionsInRange(habit, fromDay, toDay): numbertosrc/habits.ts— counts completions within a YYYY-MM-DD range (both ends inclusive) using ISO string comparison.last7Count(habit, today): number— thin wrapper that callscompletionsInRangeover the 7-day window ending ontoday.How I tested
Added 10 new unit tests in
src/habits.test.tscovering:All 27 tests pass:
npm test✅Build clean:
npm run build✅Closes #5
Generated by Claude Code