Repository navigation
fix(web): refresh sidebar/stat queries on day rollover - #390
Conversation
The desktop Today sidebar (TodayRail) captured its notion of "today" once at mount via `useMemo(() => formatAPIDate(new Date()), [])` and never subscribed to the day-rollover mechanism. When a day passed with the tab left open, the habit list re-keyed to the new day (the Today page owned its own `useTodayTick` subscription) but the rail kept the previous day's completion ring, remaining-today, streak, level/XP, and achievements until a manual refresh. Introduce a single-owner TodayProvider at the app layout that holds the current local day and owns the one `useTodayTick` subscription. On a genuine rollover it advances the day (re-keying every date-keyed habit query that consumers derive from it) and invalidates the gamification queries, which are not date-keyed and otherwise would not refetch at a silent local midnight. Both the Today page and the rail now read the day via `useToday`, so a single trigger refreshes everything date-dependent and the two surfaces can no longer drift. Mobile is unaffected: its Today screen already runs a midnight timer plus an AppState-resume rollover, and its stat surfaces are server-sourced with refetch-on-foreground rather than a client-captured date. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: PR #390 — fix(web): refresh sidebar/stat queries on day rollover
Recommendation: Request changes
Summary
TodayProvider correctly centralizes the app's local-day rollover and invalidates
gamificationKeys.all when the day genuinely changes, fixing the reported symptom
(stale sidebar completion ring / streak / level / achievements until manual refresh).
The mechanism is sound (ref-guarded, single subscription, well-tested). However, the
fix is incomplete on two fronts detailed below.
Findings
High
1. Mobile has the identical silent-midnight staleness bug; cross-platform parity is required (root CLAUDE.md: "Fix a bug in one platform? Check and fix in the other too").
- File:
apps/mobile/app/(tabs)/index.tsx:241-244(rollover handler),apps/mobile/app/(tabs)/index.tsx:347-374(midnight timer +AppStatelistener) - If a user leaves the mobile app foregrounded across local midnight (no background→foreground transition), the self-rescheduling
setTimeoutfireshandleTodayRollover, which only doessetToday(getTodayDate())— it never invalidatesgamificationKeys.all(already available viauseGamificationProfile'sinvalidateinapps/mobile/hooks/use-gamification.ts:77-79, but not wired into the rollover path). ThefocusManagerbridge only refetches on an actualAppStatechange event, which doesn't fire in this scenario. Streak/level/achievements on mobile show yesterday's data until the next real background/foreground cycle — the same bug this PR fixes on web. - Fix: in
apps/mobile/app/(tabs)/index.tsx, havehandleTodayRolloveralso invalidategamificationKeys.allviauseQueryClient(), mirroringtoday-provider.tsx'shandleRollover.
2. The web fix itself misses a second date-dependent stat on the same page: TodayHeader's streak badge.
- File:
apps/web/app/(app)/today-provider.tsx:24-30(only invalidatesgamificationKeys.all); consumed atapps/web/app/(app)/page.tsx:74,422(useProfile()→<TodayHeader streak={profile?.currentStreak ?? 0} />); source query atapps/web/hooks/use-profile.ts:36(profileKeys.detail()— a separate TanStack Query cache fromgamificationKeys, invalidated only via mutations, not rollover). - A silent local-midnight rollover (tab stays open/focused) advances
todayand invalidatesgamificationKeys.all(fixingTodayRail's streak), but leavesprofileKeysuntouched, soTodayHeader's streak badge keeps showing yesterday's number until an unrelated focus/mutation event happens to invalidateprofileKeys. This reproduces the exact bug being fixed, on the same page, in an adjacent component. - Fix: also invalidate
profileKeys.all(orprofileKeys.detail()) inTodayProvider'shandleRollover.
Both findings were verified directly against the current file contents (not just inferred from the PR description), including the mobile rollover handler and the separate profileKeys/gamificationKeys query caches.
What's good
- Clean single-owner pattern: one
useTodayTicksubscription for the whole app instead of duplicated per-page subscriptions. handleRollover's ref-guard (if (next === todayRef.current) return) correctly avoids redundant re-renders/invalidations on repeated same-day focus events.- Solid test coverage for the new provider: day-advance + gamification invalidation on both visibility-change and local-midnight timer paths, no-op on same-day focus, and the
useToday-without-provider throw. - Comment policy compliant: only JSDoc on the two exported symbols, no narration.
- No
any, noconsole.log, no dead code left behind.
Not verifiable in this environment
- orbit-api / contract-aligner: N/A — no
packages/sharedtype or DTO changes; orbit-api not checked out in this job. - Lint/type-check/tests: skipped per this workflow's CI adaptation (separate required checks already run Build/Unit Tests/SonarCloud).
Recommendation
Before merge: (1) wire gamificationKeys.all invalidation into mobile's existing rollover handler in apps/mobile/app/(tabs)/index.tsx, and (2) extend TodayProvider's handleRollover to also invalidate profileKeys.all so TodayHeader's streak badge doesn't go stale after the exact rollover this PR targets. Both are scoped follow-ups to the call sites already touched by this change.
| * (`YYYY-MM-DD`) and, when the day actually changes, advances it and refreshes | ||
| * date-dependent server state that isn't itself date-keyed (gamification: | ||
| * streak, level/XP, achievements). Date-keyed queries (habits) refresh on their | ||
| * own once consumers re-key off the new day. Consumers read the day via | ||
| * `useToday`; a manually pinned `?date=` param is resolved by the caller. | ||
| */ | ||
| export function TodayProvider({ children }: Readonly<{ children: ReactNode }>) { |
There was a problem hiding this comment.
High: This only invalidates gamificationKeys.all, but TodayHeader on the same page reads its streak from useProfile() → profileKeys.detail() (apps/web/app/(app)/page.tsx:74,422) — a separate query cache. A silent midnight rollover (tab left open/focused) will refresh TodayRail's streak but leave TodayHeader's streak badge showing yesterday's value until an unrelated focus/mutation event invalidates profileKeys. Suggest also invalidating profileKeys.all (or profileKeys.detail()) here.
There was a problem hiding this comment.
Code Review: fix(web): refresh sidebar/stat queries on day rollover
Scope: PR #390 in thomasluizon/orbit-ui-mobile
Recommendation: NEEDS WORK
Summary
The PR consolidates day-rollover ownership into a single TodayProvider (apps/web/app/(app)/today-provider.tsx) that both the Today page and the sidebar (today-rail.tsx) now read via useToday(), fixing the reported bug where the sidebar froze its date and never invalidated gamification (streak/level/achievements) queries at a silent local midnight. The web-side implementation is correct, well-tested (new today-provider.test.tsx covers visibility-change rollover, midnight-timer rollover, same-day no-op, and the missing-provider throw), and the removed dead code (getTodayDate duplicate in page.tsx) is cleaned up properly. One parity gap survived review: the PR's claim that mobile needs no equivalent fix doesn't hold for the specific defect being fixed (silent-midnight gamification staleness), because mobile's handleTodayRollover has the identical gap.
Findings
Critical
None.
High
[High] Mobile day-rollover doesn't invalidate gamification queries, mirroring the exact bug just fixed on web
· dimension: Parity (#9)
· location: apps/mobile/app/(tabs)/index.tsx:242-246 (handleTodayRollover), also wired at :355 (midnight timer) and :364 (AppState 'active' resume)
· issue: The PR body argues mobile needs no change because it has no frozen client-side "today" (true) and its gamification surfaces read via TanStack Query with `refetchOnWindowFocus` bridged to `AppState` (also true). But the root cause this PR actually fixes is broader than the frozen date: per the PR's own description, "the gamification queries... are not date-keyed... never refetched at a silent local midnight (their refetchOnWindowFocus only fires on an actual focus event...)". Mobile's focus bridge (`apps/mobile/lib/query-client.ts:8-13`, `focusManager.setEventListener` on `AppState` `'change'`) has the identical limitation: it only fires on an actual foreground/background transition. Mobile already has a self-rescheduling midnight timer (`index.tsx:242` `handleTodayRollover`) that runs at exactly the same moment the web fix targets, but it only calls `setToday(...)` — it does not invalidate `gamificationKeys.all` the way the new web `TodayProvider.handleRollover` does (`today-provider.tsx:35-41`). Confirmed no other invalidation path covers this: `apps/mobile/hooks/use-gamification.ts` only calls `invalidateQueries({ queryKey: gamificationKeys.all })` from mutation success handlers (logging/reporting events), never from the rollover path, and there is no `refetchInterval` configured (`apps/mobile/lib/query-client.ts:56-70`).
· risk: A phone/tablet left foregrounded across local midnight without an intervening lock/background transition (e.g., screen-timeout disabled, kiosk/tablet use, "stay awake while charging") keeps showing the previous day's streak/level/achievements on the profile screen and achievements screen (`apps/mobile/app/(tabs)/profile.tsx`, `apps/mobile/app/achievements.tsx`) until the app is backgrounded or a gamification mutation happens — the same stale-stats bug this PR fixes on web, just gated behind a rarer device-stays-active window instead of a common left-open-desktop-tab window.
· fix: In `apps/mobile/app/(tabs)/index.tsx`, add the same `queryClient.invalidateQueries({ queryKey: gamificationKeys.all })` call inside `handleTodayRollover` (mirroring `today-provider.tsx:35-41`), guarded the same way (only on an actual day change, which the mobile `today`/`getTodayDate` comparison already gives for free by only calling `setToday` — add an equality check before invalidating to avoid invalidating on every AppState resume).
· reference: root CLAUDE.md "Cross-platform parity (MANDATORY)" — "Fix a bug in one platform? Check and fix in the other too."; rubric dimension 9 ("PARTIAL is High until proven intentional").
Medium
None (concretely-actionable Medium bar not otherwise met — see Deferred below for what wasn't independently re-derived).
Low / Info
None posted per the rubric's signal gate.
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PARTIAL — see High finding above (manual equivalent of the agent's check; not separately spawned since the diff is small and fully read) |
| i18n-syncer | N/A — no user-facing strings added or changed |
| contract-aligner | N/A — no packages/shared/src/types/* or endpoints.ts change, and orbit-api is not part of this diff |
| security-reviewer | N/A — no orbit-api code in this diff |
Validation
| Check | Result |
|---|---|
| Lint | N/A — CI runs this as a separate required check |
| Type check | N/A — CI runs this as a separate required check |
| Tests | N/A — CI runs this as a separate required check (PR body reports 40 affected vitest tests green locally) |
| Build (api) | N/A — orbit-api not touched, not checked out in this job |
Deferred — N/A dimensions & files not verdicted
- DESIGN.md / AI-slop (#8): gated in (touches
apps/web/components/shell/today-rail.tsx) but the change there is a pure data-source swap (useMemo(formatAPIDate...)→useToday()) with zero visual/markup change — no AI-slop surface to evaluate. - Contract drift + backward-compat (#11): N/A, no shared-type or DTO field changes in this diff.
- Security (#12) and Backend hard rules (#13): N/A,
orbit-apinot touched and not checked out in this CI job — not verifiable here regardless. - FEATURES.md parity (#14): N/A, this is a bugfix with no user-facing feature/gating/platform change.
- Every changed file in the diff (
today-page.test.tsx,today-provider.test.tsx,today-rail.test.tsx,layout.tsx,page.tsx,today-provider.tsx,today-rail.tsx) was read in full and given a verdict; nothing left unverdicted.
What's good
- Single-owner
TodayProvideris the right fix shape: it removes the duplicate rollover-tracking logic frompage.tsxand the frozenuseMemofromtoday-rail.tsx, and both now derive from one source of truth, so they structurally cannot drift from each other again. handleRollover'stodayRefequality guard correctly avoids redundant invalidation on same-day focus/visibility events — matches the new testdoes not refresh gamification when a focus event fires on the same day.- Test coverage is genuinely behavioral (dispatches real
visibilitychange/focusevents and advances fake timers), not implementation-detail assertions, and includes theuseTodaywithout-provider throw case. - Comment/JSDoc usage on the two new exports is compliant with the lint-enforced comment policy (short paragraph on intent/contract, nothing narrative).
Recommendation
Add the mirrored gamification-invalidation call to apps/mobile/app/(tabs)/index.tsx's handleTodayRollover before merge, so the fix is complete on both platforms per the mandatory parity rule. Everything else in the web-side change is solid and ready to ship once that's added.
|
There was a problem hiding this comment.
Code Review: fix(web): refresh sidebar/stat queries on day rollover
Scope: PR #390 in thomasluizon/orbit-ui-mobile
Recommendation: NEEDS WORK
Summary
The PR consolidates day-rollover ownership into a single TodayProvider (apps/web/app/(app)/today-provider.tsx) that both the Today page and the sidebar (today-rail.tsx) now read via useToday(), fixing the reported bug where the sidebar froze its date and never invalidated gamification (streak/level/achievements) queries at a silent local midnight. The mechanism is sound (ref-guarded, single subscription, well-tested), and the removed dead code (getTodayDate duplicate in page.tsx) is cleaned up properly. Two gaps survive review — one is the same defect recurring on mobile, the other is the same defect recurring on an adjacent web component that reads a different query cache.
Findings
Critical
None.
High
[High] Mobile day-rollover doesn't invalidate gamification queries, mirroring the exact bug just fixed on web
· dimension: Parity (#9)
· location: apps/mobile/app/(tabs)/index.tsx:242-246 (handleTodayRollover), also wired at :355 (midnight timer) and :364 (AppState 'active' resume)
· issue: The PR body argues mobile needs no change because it has no frozen client-side "today" (true) and its gamification surfaces read via TanStack Query with `refetchOnWindowFocus` bridged to `AppState` (also true). But the root cause this PR actually fixes is broader than the frozen date: the gamification queries are not date-keyed and are never refetched at a silent local midnight, because `refetchOnWindowFocus` only fires on an actual focus event. Mobile's focus bridge (`apps/mobile/lib/query-client.ts:8-13`, `focusManager.setEventListener` on `AppState` `'change'`) has the identical limitation. Mobile already has a self-rescheduling midnight timer (`index.tsx:242`, `handleTodayRollover`) that runs at exactly the same moment the web fix targets, but it only calls `setToday(...)` — it never invalidates `gamificationKeys.all` the way the new web `TodayProvider.handleRollover` does (`today-provider.tsx:35-41`). `apps/mobile/hooks/use-gamification.ts` only invalidates `gamificationKeys.all` from mutation success handlers, never from the rollover path, and there is no `refetchInterval` configured (`apps/mobile/lib/query-client.ts:56-70`).
· risk: A phone/tablet left foregrounded across local midnight without an intervening lock/background transition (screen-timeout disabled, kiosk/tablet use, "stay awake while charging") keeps showing the previous day's streak/level/achievements on the profile and achievements screens until the app is backgrounded or a gamification mutation happens — the same stale-stats bug this PR fixes on web, gated behind a rarer device-stays-active window.
· fix: In `apps/mobile/app/(tabs)/index.tsx`, add `queryClient.invalidateQueries({ queryKey: gamificationKeys.all })` inside `handleTodayRollover`, mirroring `today-provider.tsx:35-41`.
· reference: root CLAUDE.md "Cross-platform parity (MANDATORY)" — "Fix a bug in one platform? Check and fix in the other too."
[High] The web fix itself misses a second date-dependent stat on the same page: TodayHeader's streak badge (profileKeys, not gamificationKeys)
· dimension: Correctness / completeness of the fix (#1, #9)
· location: apps/web/app/(app)/today-provider.tsx:35-41 (handleRollover only invalidates gamificationKeys.all); consumed at apps/web/app/(app)/page.tsx:74,422 (useProfile() -> <TodayHeader streak={profile?.currentStreak ?? 0} />); source query at apps/web/hooks/use-profile.ts:36 (profileKeys.detail(), refetchOnWindowFocus: true) — a separate TanStack Query cache from gamificationKeys, invalidated only via mutations, not rollover.
· issue: The exact scenario this PR fixes is a tab left open and focused across local midnight: use-follow-today-sync.ts's setTimeout fires onRollover() directly with no visibilitychange/focus event, so profile's own refetchOnWindowFocus never fires. TodayProvider's handleRollover advances `today` and invalidates gamificationKeys.all (fixing today-rail.tsx's streak), but never touches profileKeys, so page.tsx's TodayHeader streak badge — reading the same page, same rollover — keeps showing yesterday's number until an unrelated focus/mutation event happens to invalidate profileKeys. This reproduces the bug being fixed, on the same page, in an adjacent component, via the second server-state cache the fix didn't cover. Confirmed unresolved: no functional commit has landed on today-provider.tsx since this was first flagged (only a `main` merge commit, no content change).
· fix: Also invalidate `profileKeys.all` (or `profileKeys.detail()`) in TodayProvider's handleRollover, alongside gamificationKeys.all.
Medium
None (concretely-actionable Medium bar not otherwise met).
Low / Info
None posted per the rubric's signal gate.
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PARTIAL — see first High finding above |
| i18n-syncer | N/A — no user-facing strings added or changed |
| contract-aligner | N/A — no packages/shared/src/types/* or endpoints.ts change, and orbit-api is not checked out in this job |
| security-reviewer | N/A — no orbit-api code in this diff |
Validation
| Check | Result |
|---|---|
| Lint | N/A — CI runs this as a separate required check |
| Type check | N/A — CI runs this as a separate required check |
| Tests | N/A — CI runs this as a separate required check |
| Build (api) | N/A — orbit-api not touched, not checked out in this job |
Not verifiable in this environment
- Contract drift + backward-compat (#11): N/A, no shared-type or DTO field changes in this diff.
- Security (#12) and Backend hard rules (#13): N/A,
orbit-apinot touched and not checked out in this CI job. - FEATURES.md parity (#14): N/A, this is a bugfix with no user-facing feature/gating/platform change.
What's good
- Single-owner
TodayProvideris the right fix shape: it removes the duplicate rollover-tracking logic frompage.tsxand the frozenuseMemofromtoday-rail.tsx, so they structurally cannot drift from each other again. handleRollover'stodayRefequality guard correctly avoids redundant invalidation on same-day focus/visibility events.- Test coverage is genuinely behavioral (dispatches real
visibilitychange/focusevents, advances fake timers), including theuseTodaywithout-provider throw case. - Comment/JSDoc usage on the two new exports is compliant with the lint-enforced comment policy.
Recommendation
Before merge: (1) wire gamificationKeys.all invalidation into mobile's existing rollover handler in apps/mobile/app/(tabs)/index.tsx, and (2) extend TodayProvider's handleRollover to also invalidate profileKeys.all so TodayHeader's streak badge doesn't go stale after the exact rollover this PR targets. Both are scoped, one-line follow-ups to call sites already touched by this change.
Main's #390 replay work added a bulk delete test at the same place as this branch's dropped-delete Undo test; keep both tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Root cause
When a day passes with the web app tab left open (
app.useorbit.org), the habit list correctly re-updates to the new day but the right sidebar keeps showing the previous day's stats (completion ring, "Restantes hoje", "Sequência", "Nível", "Conquistas") until a manual page refresh.The Today page (
apps/web/app/(app)/page.tsx) owned its ownuseTodayTicksubscription and advanced a localtodaystate at local midnight / on focus, which re-keyed its habit query. The sidebar (apps/web/components/shell/today-rail.tsx) instead froze its day at mount:with an empty dependency array, so it never participated in rollover. Its habit query stayed keyed to the stale day (completion ring + remaining), and the gamification queries (streak / level / achievements) — which are not date-keyed — never refetched at a silent local midnight (their
refetchOnWindowFocusonly fires on an actual focus event, which does not occur while the tab stays focused across midnight).Fix
Introduce a single-owner
TodayProvider(apps/web/app/(app)/today-provider.tsx) mounted at the app layout. It:YYYY-MM-DD) and owns the oneuseTodayTicksubscription for the app;gamificationKeys.allso streak / level / XP / achievements refetch;Both the Today page and the sidebar now read the day via
useToday(), so a single rollover trigger refreshes everything date-dependent and the two surfaces can no longer drift.Cross-platform parity
Mobile is unaffected and needs no change.
apps/mobile/app/(tabs)/index.tsxalready runs a self-rescheduling midnight timer plus anAppStateactive(resume) rollover, and its stat surfaces (today-header streak, profile streak/level/achievements) are server-sourced through TanStack Query withrefetchOnWindowFocusbridged toAppState— none capture a client-side "today", so there is no frozen-date equivalent to fix.Tests
apps/web/__tests__/app/today-provider.test.tsx: day advances + gamification invalidated on visibility-change on a new day and at local midnight; no invalidation on same-day focus;useTodaythrows without a provider.today-page.test.tsxandtoday-rail.test.tsxto render insideTodayProvider(the page's local rollover ownership moved into the provider).apps/web: type-check, lint, and the affected vitest suites (40 tests) all green.🤖 Generated with Claude Code