feat(home): streak milestones + sunday memory digest (PR-B)#39
Merged
Conversation
PR3 — Electron streak milestone notifications:
- src/lib/calc-streak.ts: TZ-safe streak/longest/tier/shown-up math
via shiftIsoDate; tiers 7 / 30 / 100 / 365 per DESIGN.md D12.
- src/hooks/useStreakNotifications.ts: composes existing
useElectronNotifications; localStorage 'streak-max-tier-notified'
dedupes per tier so milestones never re-fire after a break.
- Copy is additive only ("day 7 of showing up — the cathedral
remembers") per DESIGN.md voice; never "streak ended".
PR4 — Memory Lane Sunday digest card:
- SundayDigestCard renders only on local Sunday with a per-week
localStorage dismiss key keyed on the local Sunday's ISO date so
next Sunday auto-recovers. Surfaces 7-day total, best day, and
top categories — falls through to a quiet "the room was quiet
this week" line on zero-weeks (never failure framing).
- SSR-safe via useMounted; no extra network round-trip (shares
dataByDate with WeeklySummaryCard/ContributionGraph).
Wired into TodoList.tsx alongside the existing weekly summary; the
streak hook is fire-and-forget and no-ops outside Electron.
44 new unit tests covering empty data, grace period, tier dedupe,
TZ/DST boundaries, dismiss persistence, and the Sunday gating.
- SundayDigestCard: memoize `today` so useMemo stays stable across renders
- SundayDigestCard: anchor 7-day window on UTC midnight of local Sunday so
JST users see Sunday data (UTC-Saturday-as-anchor was the prior bug)
- SundayDigestCard: rename "best day" → "brightest day" + spell out count
("6 things") — removes ranking framing flagged by Codex against DESIGN.md
- useStreakNotifications: gate on `useIsRestoring()` so a stale TanStack
persister snapshot cannot latch a wrong tier permanently
- useStreakNotifications: validate stored tier against STREAK_TIERS so a
tampered/spurious value (e.g. "5000") does not block real milestones
- TodoList: pass `isRestoring` through to useStreakNotifications
Tests: +2 (spurious-value defense, isRestoring gate, 1-thing singular).
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdds deterministic UTC streak computation and STREAK_TIERS, a client hook that shows per-user milestone Electron notifications with localStorage dedupe, a SundayDigestCard that shows a weekly recap with per-week dismissal, and TodoList wiring to run the hook and render the card. ChangesStreak Tracking & Sunday Digest Notifications
Sequence Diagram(s)sequenceDiagram
participant TodoList as TodoList
participant Hook as useStreakNotifications
participant Calc as calcStreak
participant Store as localStorage
participant Electron as ElectronNotifier
TodoList->>Hook: useStreakNotifications(dataByDate, isLoading, isRestoring, now)
Hook->>Calc: calcStreak(dataByDate, now)
Calc-->>Hook: StreakSummary (currentTier)
Hook->>Store: readStoredTier(userKey)
Store-->>Hook: previousMaxTier
alt currentTier > previousMaxTier and not isRestoring and notifications supported
Hook->>Store: writeStoredTier(currentTier)
Store-->>Hook: ok
Hook->>Electron: showNotification(title, body, tag)
Electron-->>Hook: ack
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #39 +/- ##
==========================================
+ Coverage 72.05% 73.97% +1.92%
==========================================
Files 24 30 +6
Lines 526 734 +208
Branches 130 188 +58
==========================================
+ Hits 379 543 +164
- Misses 137 176 +39
- Partials 10 15 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/app/`(main)/home/_components/SundayDigestCard.tsx:
- Around line 215-219: The early-return checking dataByDate.size === 0 in
SundayDigestCard prevents rendering the empty-heatmap "quiet week" fallback;
remove or change that guard so the component still renders when dataByDate is
empty and lets the internal Heatmap/fallback branch show the "the room was quiet
this week" message — keep the other guards (isMounted, isLoading, isSunday,
dismissed) intact and adjust any downstream rendering logic that assumes
non-empty data (in render paths inside SundayDigestCard and any
Heatmap/QuietWeek components) to handle an empty dataByDate Map safely.
- Around line 146-152: The localSundayIso function relies on
toLocaleDateString('en-CA') which is not deterministic; replace that call with a
manual YYYY-MM-DD formatter that uses the Date getters (getFullYear, getMonth,
getDate) and pads month/day to two digits (e.g.,
String(month+1).padStart(2,'0')), so localSundayIso(now: Date) computes the
previous Sunday as it does now but returns `${year}-${mm}-${dd}` explicitly;
update any callers like shiftIsoDate/localStorage keys to continue using the new
deterministic YYYY-MM-DD string.
- Around line 182-188: The component is freezing to the mount-day because today
is memoized; remove the useMemo and compute today directly as now ?? new Date()
so a fresh Date is produced each render, which will let derived values isSunday,
weekKey (and any memoized summary that depends on today) update across day
boundaries; update any related memo/computed values (e.g., summary,
localSundayIso usages) to depend on the non-memoized today or include
appropriate dependencies so they recompute when the date changes.
In `@src/hooks/useStreakNotifications.ts`:
- Around line 13-20: STORAGE_KEY currently uses a global browser key
('corelive.streak-max-tier-notified') which causes cross-account suppression;
change the dedupe to be user-scoped by incorporating the signed-in user's stable
id (e.g., append or prefix with currentUser.id) when reading/writing in
useStreakNotifications (references: STORAGE_KEY, any get/set calls in
useStreakNotifications.ts) or alternatively clear the stored key on auth changes
by hooking into the auth state change handler used by the hook; ensure all
reads/writes and the initial default use the new namespaced key so notifications
are per-account.
🪄 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 Plus
Run ID: a080fd5b-6b30-4ee4-b697-f2fc167be5db
📒 Files selected for processing (7)
src/app/(main)/home/_components/SundayDigestCard.test.tsxsrc/app/(main)/home/_components/SundayDigestCard.tsxsrc/app/(main)/home/_components/TodoList.tsxsrc/hooks/useStreakNotifications.test.tssrc/hooks/useStreakNotifications.tssrc/lib/calc-streak.test.tssrc/lib/calc-streak.ts
- SundayDigestCard.localSundayIso: replace toLocaleDateString('en-CA')
with deterministic YYYY-MM-DD formatter. The locale's format is not
spec-stable and has flipped to M/d/yyyy in past CLDR updates.
- SundayDigestCard.today: revert the useMemo wrap. Memoizing freezes the
Date to mount-day, so the card would not appear if the home stays open
across Sat→Sun. The per-render Date is cheap.
- SundayDigestCard: remove the empty-heatmap early-return so the
"the room was quiet this week — that is fine too." fallback can render
for new users and fully quiet weeks (this was the path the copy was
added for).
- useStreakNotifications: namespace the dedupe localStorage key per
Clerk userId (corelive.streak-max-tier-notified.<user_id>). Without
this, two accounts on one Electron/browser profile suppress each
other's milestones. Hook now short-circuits when userId is null.
- useStreakNotifications: key the in-memory latch ref by userId too so
a sign-out → sign-in-as-different-user in the same session does not
leak the previous account's tier into the new account's gate.
Tests: +2 (signed-out short-circuit, per-user namespace cross-account
isolation). Empty-heatmap test now asserts the quiet-week fallback
renders instead of being suppressed.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/hooks/useStreakNotifications.test.ts (2)
230-243: ⚡ Quick winTest the
isRestoringtransition (true -> false) in one hook instance.Line 230 currently covers only the static restoring gate. A rerender transition test will catch missed-fire or double-fire regressions during hydration completion.
Proposed transition test
+ it('fires exactly once after rehydration finishes', () => { + const { rerender } = renderHook( + ({ isRestoring }) => + useStreakNotifications({ + dataByDate: buildConsecutive(7), + isLoading: false, + isRestoring, + now: TODAY, + }), + { initialProps: { isRestoring: true } }, + ) + + expect(electronMocks.showNotification).not.toHaveBeenCalled() + rerender({ isRestoring: false }) + expect(electronMocks.showNotification).toHaveBeenCalledTimes(1) + expect(window.localStorage.getItem(STORAGE_KEY)).toBe('7') + })🤖 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/hooks/useStreakNotifications.test.ts` around lines 230 - 243, Add a test that exercises the isRestoring transition on a single hook instance: renderHook(() => useStreakNotifications(...)) with isRestoring: true, assert no notification and STORAGE_KEY absent, then rerender the same hook with isRestoring: false and the same inputs and assert electronMocks.showNotification is called exactly once (no double-fire) and window.localStorage.getItem(STORAGE_KEY) is set accordingly; this verifies the hook correctly waits for the restoring -> not-restoring transition and prevents missed or duplicate notifications.
150-161: ⚡ Quick winAdd explicit coverage for the 100/365 tiers.
Right now only Line 150 (30-day) and Line 134 (7-day) milestone paths are asserted; adding 100/365 checks would protect all shipped tiers from copy/tag/storage regressions.
Proposed test addition
+ it.each([100, 365])( + 'fires the %i-day tier and stores it as max', + (tier) => { + renderHook(() => + useStreakNotifications({ + dataByDate: buildConsecutive(tier), + isLoading: false, + now: TODAY, + }), + ) + expect(electronMocks.showNotification).toHaveBeenCalledTimes(1) + expect(electronMocks.showNotification.mock.calls[0]?.[0]).toMatch( + new RegExp(`day\\s+${tier}`, 'i'), + ) + expect(window.localStorage.getItem(STORAGE_KEY)).toBe(String(tier)) + }, + )🤖 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/hooks/useStreakNotifications.test.ts` around lines 150 - 161, Add explicit test cases that assert notifications and storage updates for the 100-day and 365-day tiers in the same style as the existing 7-day and 30-day tests: render the hook via useStreakNotifications with dataByDate built by buildConsecutive(100) and buildConsecutive(365) (isLoading: false, now: TODAY), then expect electronMocks.showNotification to have been called and that the notification text matches /day 100/i and /day 365/i respectively, and finally assert window.localStorage.getItem(STORAGE_KEY) equals '100' and '365' for each test; mirror the existing test structure and cleanup to ensure isolation.
🤖 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.
Nitpick comments:
In `@src/hooks/useStreakNotifications.test.ts`:
- Around line 230-243: Add a test that exercises the isRestoring transition on a
single hook instance: renderHook(() => useStreakNotifications(...)) with
isRestoring: true, assert no notification and STORAGE_KEY absent, then rerender
the same hook with isRestoring: false and the same inputs and assert
electronMocks.showNotification is called exactly once (no double-fire) and
window.localStorage.getItem(STORAGE_KEY) is set accordingly; this verifies the
hook correctly waits for the restoring -> not-restoring transition and prevents
missed or duplicate notifications.
- Around line 150-161: Add explicit test cases that assert notifications and
storage updates for the 100-day and 365-day tiers in the same style as the
existing 7-day and 30-day tests: render the hook via useStreakNotifications with
dataByDate built by buildConsecutive(100) and buildConsecutive(365) (isLoading:
false, now: TODAY), then expect electronMocks.showNotification to have been
called and that the notification text matches /day 100/i and /day 365/i
respectively, and finally assert window.localStorage.getItem(STORAGE_KEY) equals
'100' and '365' for each test; mirror the existing test structure and cleanup to
ensure isolation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8fb9f6d3-e689-498f-96c8-8add0d61beba
📒 Files selected for processing (4)
src/app/(main)/home/_components/SundayDigestCard.test.tsxsrc/app/(main)/home/_components/SundayDigestCard.tsxsrc/hooks/useStreakNotifications.test.tssrc/hooks/useStreakNotifications.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/hooks/useStreakNotifications.ts
- src/app/(main)/home/_components/SundayDigestCard.test.tsx
6 tasks
ryota-murakami
added a commit
that referenced
this pull request
May 11, 2026
Heatmap Cathedral release — Design Doc §3.1–§3.8 fully implemented. PRs since v0.4.0 (27 commits, 4 PRs): - #38 PR-A: UNION heatmap + WeeklySummaryCard + day-nav < > + j/k + ?date= deep-link - #39 PR-B: Streak milestone notifications (Electron) + Sunday Memory Lane digest - #40 PR-C: Per-category WoW trend chips + shareable "this day" PNG + Year-in-Review modal - #41 PR-D hotfix: blank share-image PNG — wrap card in 0×0 overflow:hidden clipper
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
2 participants
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
Heatmap Cathedral PR-B — implements DESIGN.md §3.3-3.4 (Streak Tier
Notifications + Memory Lane Sunday Digest).
crosses Day 7 / 30 / 100 / 365 of showing up. localStorage-keyed dedupe
(
corelive.streak-max-tier-notified) is monotonic by tier, so a brokenstreak does not re-fire a previously celebrated tier (DESIGN.md D12:
additive, never decreases without explanation).
appears only on local Sunday, summarises the last 7 days (total + the
brightest day + top categories), and persists a per-week dismiss via
corelive.sunday-digest-dismissed.<local-Sunday-ISO>. Zero-weekfallback: "the room was quiet this week — that is fine too."
Files
src/lib/calc-streak.ts+ tests — TZ-safe streak / tier calculatorusing
shiftIsoDate(24 unit tests covering grace period, gaps,longest-preserved, leap day, DST spring-forward, all four tier
transitions)
src/hooks/useStreakNotifications.ts+ tests — Electron-only one-shotnotification hook (12 unit tests covering web no-op, loading gate,
persister-restoring gate, sub-tier suppression, dedupe, corrupted /
spurious storage values)
src/app/(main)/home/_components/SundayDigestCard.tsx+ tests —Sunday-only digest card (10 unit tests covering non-Sunday hide,
zero-week fallback, brightest-day rendering, singular "1 thing",
per-week dismiss persistence)
src/app/(main)/home/_components/TodoList.tsx— wires both consumersto the existing
useHeatmapData()query so no extra fetch is created(React Query dedupes the underlying request with ContributionGraph)
Voice (DESIGN.md §Voice)
a time."
up."
that is fine too."
Pre-PR review
Ran 3-way adversarial check (specialized agent
modern-react-pro+codex-rescue+ Claude Code advisor). All findings applied in commit676b37a:SundayDigestCard:today = now ?? new Date()was creating a new Date every render and busting
useMemo. Wrappedin
useMemoso identity is stable.SundayDigestCard: aggregation anchor wastoday(UTC), but visibility was local Sunday — for JST users at Sunday
06:00 the UTC day is still Saturday, so the digest showed wrong
data. Now anchored on UTC midnight of the local Sunday ISO so the
data window matches the visibility window.
useStreakNotifications: gated effect onuseIsRestoring()so a stale TanStack persister snapshot cannotlatch a wrong tier permanently before live data arrives.
useStreakNotifications.readStoredTier: nowvalidates against
STREAK_TIERSso a tampered / spurious"5000"cannot block every real future milestone."6 things"(DESIGN.md bans KPI / ranking framing).Deferred (documented, not fixed in this PR)
calcStreak: when the heatmap queryreturns a window shorter than 365 days,
longestStreakis boundedby that window. This is fine for tier semantics (the only thing
consumed) but could surface as a stat in a future PR — track with
src/hooks/useHeatmapData.tswindow expansion if/when needed.Test plan
pnpm validate(test + lint + build + typecheck) — green (172unit tests, all PR-B suites included)
pnpm dev:now) the digest card appears belowWeeklySummaryCard with correct copy
banner exactly once; second mount does not re-fire
🤖 PR-B of the Heatmap Cathedral 3-PR split.
Summary by CodeRabbit
New Features
Tests