fix: stats logging, bonfire animation, timezone and streak fixes#30
fix: stats logging, bonfire animation, timezone and streak fixes#30
Conversation
Stats: - Watchers in host/jam mode now get pomodoros logged (applyState was blocking onExpire for remote timer completions) - Dedup guard by startedAt prevents double-counting in jam mode races - Pass client local date (p_today) to increment_profile_stats so streak day attribution uses the user timezone, not server UTC - Drop stale function overloads from migrations 001 and 005 Bonfire: - Flame now grows from small (0.12) to large (0.85) as timer progresses - Resets to low on completion instead of surging; each session independent - Fix missing mode prop on BonfireScene Timezone: - buildCalendarCells and buildWeekDays use UTC arithmetic to match toDayKey output; fixes off-by-one cells for non-UTC users - Month labels use noon UTC to avoid day boundary misread - Today count query uses UTC midnight Profile: - Dead streak shows 0 if last_active_date is stale (2-day UTC window covers all timezones) - Hours display standardized to Math.floor across all three components - StreakCalendar legend fixed: 5 distinct swatches instead of duplicate - Add last_active_date to Profile type and useProfile schema - Create KeyboardShortcutsModal (missing component causing build error) Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 29 minutes and 33 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThis PR overhauls streak tracking and display logic to use UTC-based dates, introduces a keyboard shortcuts modal, refactors session management with OAuth signin and theme toggle, rewrites flame intensity calculation based on timer progress, and adds a database function to atomically update streak stats with client-provided dates. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/session/SessionProvider.tsx (2)
856-886:⚠️ Potential issue | 🟠 MajorFocus-skip increments local counters but never logs to the DB — stats will silently diverge.
handleSkipon a focus mode now bumpsfocusCountRefandtodayCount(and writespomos_doneinto the session row), so the UI shows "N pomodoros completed today", butincrement_profile_statsis never called on this path. Consequence: the badge and the profile page will disagree for anyone who skips instead of letting the timer run out, and the streak/day attribution this PR was designed to fix is skipped entirely on this branch.Either call the RPC here too (with the actual elapsed minutes, not the configured duration), or don't bump the "today" counter on skip — the current state is the worst of both.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/session/SessionProvider.tsx` around lines 856 - 886, handleSkip currently increments focusCountRef and todayCount and enqueuesSessionUpdate but never calls the backend RPC to persist the skipped pomodoro; update handleSkip to call increment_profile_stats (or the existing RPC wrapper used elsewhere) when skipping a focus round, passing the actual elapsed minutes computed from the new timer state (use newState.totalTime - newState.timeLeft, converted to minutes) rather than sessionSettings.durations, and include the same pomos_done value (focusCountRef.current) so DB stats stay in sync with the UI.
470-501:⚠️ Potential issue | 🔴 CriticalHost never broadcasts
status: "finished"—the watcher-log fix is inert.The new watcher-log code at lines 481–498 depends on receiving a remote broadcast with
state.status === "finished" && state.mode === "focus". However,handleExpire(238–303) only broadcasts the next state after auto-starting: either a break (mode: "short"|"long", status: "running") or the next focus session (mode: "focus", status: "running"). The intermediate finished state is created locally inuseTimeron tick-to-zero but never broadcast.Since watchers only receive the transitions between running modes (never the finished state itself), the condition at line 481 will never match and remote watchers will not have their stats logged via this path. The original issue for watcher stats-logging remains unresolved.
To fix: Broadcast the finished state before transitioning to the next mode, or log stats when receiving the post-expiry running state by deduping on
(startedAt, mode)pairs instead of relying on a finished-state broadcast.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/session/SessionProvider.tsx` around lines 470 - 501, The watcher logging path never sees status === "finished" because handleExpire triggers broadcasting of the next running state; update the logic so watchers can log: either modify handleExpire to emit the intermediate finished state before emitting the next running state (reference handleExpire and the broadcast function used there), or change the onTimerUpdate logging callback to detect the post-expiry running broadcast and dedupe by (startedAt, mode) instead of only startedAt (update the condition in the onTimerUpdate callback that uses lastLoggedTimerRef and minutes calculation to key by startedAt + mode and use lastLoggedTimerRef/currentLoggedKey accordingly). Ensure applyState, onTimerUpdate, and lastLoggedTimerRef are used consistently so the same session isn't double-counted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/profile/`[username]/page.tsx:
- Around line 56-70: buildCalendarCells currently computes start from
utcDay(-371) which, after rewinding to the previous Monday, makes the grid end
before today; change the logic to anchor the grid on today: compute end =
utcDay(0) (today), set start = new Date(end) and subtract (53*7 - 1) days to get
the window start, then rewind that start to the Monday on-or-before as before
and iterate forward 53*7 cells; update buildCalendarCells (and keep
toDayKey/utcDay usage) so the last pushed cell can equal todayStr used in
StreakCalendar.
In `@components/profile/StatsGrid.tsx`:
- Around line 6-14: Update the comment above function effectiveStreak to
accurately describe the grace period as a ~3-day UTC window (today, yesterday,
and 2-days-ago) rather than "2-day", and note this extra day accommodates
timezone skew (clients can be up to ±14 hours or a full calendar day off server
UTC); leave the function logic (effectiveStreak, toDayKey, comparison with
twoDaysAgo) unchanged.
In `@components/session/KeyboardShortcutsModal.tsx`:
- Around line 24-51: The KeyboardShortcutsModal lacks dialog semantics and focus
management: update the root modal container rendered by KeyboardShortcutsModal
to include role="dialog", aria-modal="true" and aria-labelledby pointing to the
header (give the <h2> a stable id), add a ref for the close <button> (the X
button) and on mount save document.activeElement, focus the close button, and on
unmount restore the saved element via onClose cleanup; additionally implement a
minimal focus trap inside the modal container so Tab/Shift+Tab cycle between
focusable elements in the modal (prevent Tab from escaping to background).
Ensure the close button retains an aria-label and the header id matches
aria-labelledby.
In `@components/session/SessionProvider.tsx`:
- Around line 248-265: The current handleExpire block uses
Math.round(settings.durations.focus) which diverges from the onTimerUpdate path
that uses the broadcasted state.totalTime/60; change handleExpire (the code
using timerStartedAtRef, lastLoggedTimerRef and the
supabase.rpc("increment_profile_stats") call) to compute minutes from the
broadcasted elapsed seconds (or state.totalTime) rather than
settings.durations.focus, and drop the redundant Math.round; ensure the same
source (broadcasted totalTime converted to minutes or actual elapsed seconds) is
used in both onTimerUpdate and this rpc call so logged minutes are consistent.
- Around line 481-498: The current onTimerUpdate block unconditionally calls
supabase.rpc("increment_profile_stats") for any authenticated user when a focus
session finishes; change it to only log stats for active participants by gating
the RPC call behind an "active participation" check: inside the same function
(onTimerUpdate) require either state.mode === "jam" OR a new boolean/condition
like isActiveParticipant === true (where isActiveParticipant is determined from
presence/heartbeat duration >= N seconds or a session-specific flag) before
updating lastLoggedTimerRef and calling increment_profile_stats; make the
active-threshold configurable (e.g.,
sessionSettingsRef.current.activeThresholdSeconds) and ensure callers that
manage presence set/update isActiveParticipant via heartbeat/presence logic so
passive watchers do not earn credit.
In `@hooks/useBonfireState.ts`:
- Around line 73-81: The useEffect that watches focusCount schedules
surgeTimerRef and can call setIsSurging after unmount; add a cleanup to that
effect that clears surgeTimerRef (if set) and prevents the pending callback from
running on an unmounted component. Specifically, inside the effect that
references prevFocusCountRef, surgeTimerRef, and setIsSurging, return a cleanup
function that clearsTimeout(surgeTimerRef.current) and nulls the ref (or use an
isMounted ref to guard setIsSurging) so the timeout is removed on unmount and no
state update occurs after the component is unmounted.
In `@supabase/migrations/012_streak_client_date.sql`:
- Line 24: The comparison uses timestamp arithmetic causing an implicit cast;
update the conditional that currently reads "if v_last_active = v_today -
interval '1 day' then" to use date arithmetic instead by replacing the RHS with
"v_today - 1" so v_today and v_last_active remain dates; locate the conditional
referencing v_last_active and v_today and make this single-token replacement to
avoid the timestamp cast.
- Around line 5-22: The SECURITY DEFINER function increment_profile_stats
currently trusts the caller-supplied p_today and p_minutes; clamp p_today to a
small window around the server date (e.g., if abs(coalesce(p_today,current_date)
- current_date) > 1 day then set v_today := current_date else set v_today :=
coalesce(p_today,current_date)) so client-supplied dates outside ±1 day are
rejected, and also cap p_minutes to a sane maximum (e.g., if p_minutes > 180
then set p_minutes := 180) before any streak/last_active_date logic runs to
prevent forged streaks or inflated session times; make these adjustments at the
start of increment_profile_stats (refer to parameters p_today/p_minutes and
local variable v_today) so all subsequent logic uses the sanitized values.
---
Outside diff comments:
In `@components/session/SessionProvider.tsx`:
- Around line 856-886: handleSkip currently increments focusCountRef and
todayCount and enqueuesSessionUpdate but never calls the backend RPC to persist
the skipped pomodoro; update handleSkip to call increment_profile_stats (or the
existing RPC wrapper used elsewhere) when skipping a focus round, passing the
actual elapsed minutes computed from the new timer state (use newState.totalTime
- newState.timeLeft, converted to minutes) rather than
sessionSettings.durations, and include the same pomos_done value
(focusCountRef.current) so DB stats stay in sync with the UI.
- Around line 470-501: The watcher logging path never sees status === "finished"
because handleExpire triggers broadcasting of the next running state; update the
logic so watchers can log: either modify handleExpire to emit the intermediate
finished state before emitting the next running state (reference handleExpire
and the broadcast function used there), or change the onTimerUpdate logging
callback to detect the post-expiry running broadcast and dedupe by (startedAt,
mode) instead of only startedAt (update the condition in the onTimerUpdate
callback that uses lastLoggedTimerRef and minutes calculation to key by
startedAt + mode and use lastLoggedTimerRef/currentLoggedKey accordingly).
Ensure applyState, onTimerUpdate, and lastLoggedTimerRef are used consistently
so the same session isn't double-counted.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9336759f-3d1d-4362-9b42-4e509cea3ab8
📒 Files selected for processing (11)
app/profile/[username]/page.tsxcomponents/profile/StatsGrid.tsxcomponents/profile/StreakCalendar.tsxcomponents/profile/WeeklyChart.tsxcomponents/session/KeyboardShortcutsModal.tsxcomponents/session/SessionProvider.tsxhooks/useBonfireState.tshooks/useProfile.tssupabase/migrations/012_streak_client_date.sqlsupabase/migrations/013_drop_stale_stat_overloads.sqltypes/index.ts
| // Returns 0 if the streak hasn't been continued recently enough to still be alive. | ||
| // Uses a 2-day UTC window to accommodate all timezones (UTC-12 to UTC+14): | ||
| // a user in UTC+14 stores last_active_date as their local date which can be | ||
| // 1 day ahead of server UTC, so we check >= 2 days ago UTC. | ||
| function effectiveStreak(currentStreak: number, lastActiveDate: string | null): number { | ||
| if (currentStreak === 0 || !lastActiveDate) return currentStreak | ||
| const twoDaysAgo = toDayKey(new Date(Date.now() - 2 * 24 * 60 * 60 * 1000)) | ||
| return lastActiveDate >= twoDaysAgo ? currentStreak : 0 | ||
| } |
There was a problem hiding this comment.
Minor: comment undersells the window width.
lastActiveDate >= twoDaysAgo admits three UTC calendar days (today, yesterday, and 2-days-ago), so the effective grace window is ~3 days, not 2. The extra day is defensible as timezone slack (UTC±14 can put the client's stored date up to one day off server UTC), but the comment should reflect reality so the next reader doesn't "fix" it to match the comment:
📝 Suggested comment tweak
-// Returns 0 if the streak hasn't been continued recently enough to still be alive.
-// Uses a 2-day UTC window to accommodate all timezones (UTC-12 to UTC+14):
-// a user in UTC+14 stores last_active_date as their local date which can be
-// 1 day ahead of server UTC, so we check >= 2 days ago UTC.
+// Returns 0 if the streak hasn't been continued recently enough to still be alive.
+// Allows last_active_date as old as 2 days ago UTC (i.e. a ~3-day grace window)
+// to accommodate timezone skew: clients store their local YYYY-MM-DD, which can
+// be ±1 day from server UTC in UTC-12…UTC+14.Logic itself is fine.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/profile/StatsGrid.tsx` around lines 6 - 14, Update the comment
above function effectiveStreak to accurately describe the grace period as a
~3-day UTC window (today, yesterday, and 2-days-ago) rather than "2-day", and
note this extra day accommodates timezone skew (clients can be up to ±14 hours
or a full calendar day off server UTC); leave the function logic
(effectiveStreak, toDayKey, comparison with twoDaysAgo) unchanged.
| return ( | ||
| <div | ||
| className="fixed inset-0 z-50 flex items-center justify-center" | ||
| style={{ background: 'rgba(0,0,0,0.55)' }} | ||
| onClick={onClose} | ||
| > | ||
| <div | ||
| className="relative rounded-2xl p-6 flex flex-col gap-4 w-full max-w-xs mx-4" | ||
| style={{ | ||
| background: 'var(--bg-elevated)', | ||
| border: '1px solid var(--border)', | ||
| boxShadow: 'var(--shadow-lg, 0 8px 32px rgba(0,0,0,0.4))', | ||
| }} | ||
| onClick={e => e.stopPropagation()} | ||
| > | ||
| <div className="flex items-center justify-between"> | ||
| <h2 className="text-sm font-semibold" style={{ color: 'var(--text-primary)' }}> | ||
| Keyboard shortcuts | ||
| </h2> | ||
| <button | ||
| onClick={onClose} | ||
| className="rounded-lg p-1 transition-colors" | ||
| style={{ color: 'var(--text-muted)' }} | ||
| aria-label="Close" | ||
| > | ||
| <X className="w-4 h-4" /> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
Modal is missing dialog semantics and focus management.
Screen readers won't announce this as a dialog, and keyboard users aren't trapped inside it (Tab will escape to the background, and focus isn't restored to the trigger on close). For a modal overlay this is a real a11y gap.
♿ Minimal fix for dialog semantics
<div
className="fixed inset-0 z-50 flex items-center justify-center"
style={{ background: 'rgba(0,0,0,0.55)' }}
onClick={onClose}
>
<div
+ role="dialog"
+ aria-modal="true"
+ aria-labelledby="kbd-shortcuts-title"
className="relative rounded-2xl p-6 flex flex-col gap-4 w-full max-w-xs mx-4"
...
onClick={e => e.stopPropagation()}
>
<div className="flex items-center justify-between">
- <h2 className="text-sm font-semibold" style={{ color: 'var(--text-primary)' }}>
+ <h2 id="kbd-shortcuts-title" className="text-sm font-semibold" style={{ color: 'var(--text-primary)' }}>
Keyboard shortcuts
</h2>Consider also auto-focusing the close button on mount and restoring focus to the previously focused element on unmount (or adopting a small focus-trap helper) — otherwise Tab will wander into the background UI behind the overlay.
As per coding guidelines: "Accessibility: interactive elements need aria-labels where text is absent."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ( | |
| <div | |
| className="fixed inset-0 z-50 flex items-center justify-center" | |
| style={{ background: 'rgba(0,0,0,0.55)' }} | |
| onClick={onClose} | |
| > | |
| <div | |
| className="relative rounded-2xl p-6 flex flex-col gap-4 w-full max-w-xs mx-4" | |
| style={{ | |
| background: 'var(--bg-elevated)', | |
| border: '1px solid var(--border)', | |
| boxShadow: 'var(--shadow-lg, 0 8px 32px rgba(0,0,0,0.4))', | |
| }} | |
| onClick={e => e.stopPropagation()} | |
| > | |
| <div className="flex items-center justify-between"> | |
| <h2 className="text-sm font-semibold" style={{ color: 'var(--text-primary)' }}> | |
| Keyboard shortcuts | |
| </h2> | |
| <button | |
| onClick={onClose} | |
| className="rounded-lg p-1 transition-colors" | |
| style={{ color: 'var(--text-muted)' }} | |
| aria-label="Close" | |
| > | |
| <X className="w-4 h-4" /> | |
| </button> | |
| </div> | |
| return ( | |
| <div | |
| className="fixed inset-0 z-50 flex items-center justify-center" | |
| style={{ background: 'rgba(0,0,0,0.55)' }} | |
| onClick={onClose} | |
| > | |
| <div | |
| role="dialog" | |
| aria-modal="true" | |
| aria-labelledby="kbd-shortcuts-title" | |
| className="relative rounded-2xl p-6 flex flex-col gap-4 w-full max-w-xs mx-4" | |
| style={{ | |
| background: 'var(--bg-elevated)', | |
| border: '1px solid var(--border)', | |
| boxShadow: 'var(--shadow-lg, 0 8px 32px rgba(0,0,0,0.4))', | |
| }} | |
| onClick={e => e.stopPropagation()} | |
| > | |
| <div className="flex items-center justify-between"> | |
| <h2 id="kbd-shortcuts-title" className="text-sm font-semibold" style={{ color: 'var(--text-primary)' }}> | |
| Keyboard shortcuts | |
| </h2> | |
| <button | |
| onClick={onClose} | |
| className="rounded-lg p-1 transition-colors" | |
| style={{ color: 'var(--text-muted)' }} | |
| aria-label="Close" | |
| > | |
| <X className="w-4 h-4" /> | |
| </button> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/session/KeyboardShortcutsModal.tsx` around lines 24 - 51, The
KeyboardShortcutsModal lacks dialog semantics and focus management: update the
root modal container rendered by KeyboardShortcutsModal to include
role="dialog", aria-modal="true" and aria-labelledby pointing to the header
(give the <h2> a stable id), add a ref for the close <button> (the X button) and
on mount save document.activeElement, focus the close button, and on unmount
restore the saved element via onClose cleanup; additionally implement a minimal
focus trap inside the modal container so Tab/Shift+Tab cycle between focusable
elements in the modal (prevent Tab from escaping to background). Ensure the
close button retains an aria-label and the header id matches aria-labelledby.
| if (userId && currentMode === "focus") { | ||
| const minutes = Math.round(settings.durations.focus); | ||
| supabase | ||
| .rpc("increment_profile_stats", { | ||
| p_user_id: userId, | ||
| p_minutes: minutes, | ||
| p_session_id: session.id, | ||
| }) | ||
| .then(({ error }) => { | ||
| if (error) | ||
| console.error("[handleExpire] Failed to log pomodoro:", error); | ||
| }); | ||
| const startedAt = timerStartedAtRef.current; | ||
| if (startedAt !== null && lastLoggedTimerRef.current !== startedAt) { | ||
| lastLoggedTimerRef.current = startedAt; | ||
| const minutes = Math.round(settings.durations.focus); | ||
| supabase | ||
| .rpc("increment_profile_stats", { | ||
| p_user_id: userId, | ||
| p_minutes: minutes, | ||
| p_session_id: session.id, | ||
| p_today: new Date().toLocaleDateString("en-CA"), | ||
| }) | ||
| .then(({ error }) => { | ||
| if (error) | ||
| console.error("[handleExpire] Failed to log pomodoro:", error); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: minutes source diverges between the two log sites.
Here you use settings.durations.focus (local configured minutes); in onTimerUpdate at line 485 you use the broadcasted state.totalTime / 60. For watchers whose local settings differ from the host's actual run, these two paths will log different durations for the same timer if (pathologically) both paths ever fire. Using the broadcasted totalTime on both sides — or the actual elapsed seconds — would make the two paths consistent and honest about what was actually run.
Also, Math.round(settings.durations.focus) is redundant (duration is already an integer); harmless but noise.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/session/SessionProvider.tsx` around lines 248 - 265, The current
handleExpire block uses Math.round(settings.durations.focus) which diverges from
the onTimerUpdate path that uses the broadcasted state.totalTime/60; change
handleExpire (the code using timerStartedAtRef, lastLoggedTimerRef and the
supabase.rpc("increment_profile_stats") call) to compute minutes from the
broadcasted elapsed seconds (or state.totalTime) rather than
settings.durations.focus, and drop the redundant Math.round; ensure the same
source (broadcasted totalTime converted to minutes or actual elapsed seconds) is
used in both onTimerUpdate and this rpc call so logged minutes are consistent.
| create or replace function public.increment_profile_stats( | ||
| p_user_id uuid, | ||
| p_minutes integer, | ||
| p_session_id text default null, | ||
| p_today date default null | ||
| ) | ||
| returns void language plpgsql security definer set search_path = public as $$ | ||
| declare | ||
| v_last_active date; | ||
| v_today date := coalesce(p_today, current_date); | ||
| begin | ||
| if p_minutes <= 0 then return; end if; | ||
|
|
||
| -- Lock the row to prevent concurrent streak miscalculation | ||
| select last_active_date into v_last_active | ||
| from public.profiles | ||
| where id = p_user_id | ||
| for update; |
There was a problem hiding this comment.
p_today is fully client-controlled in a SECURITY DEFINER function — streaks can be forged.
Because this function runs with definer privileges and uses the caller-supplied p_today verbatim for all streak attribution, a user can trivially call the RPC with arbitrary dates (e.g. last_active_date + 1 day) to keep a streak alive indefinitely without ever completing a session on the right day. If streaks surface on public profiles or leaderboards (they do — see components/profile/StatsGrid.tsx), this is a data-integrity issue, not just a self-foot-gun.
Clamp p_today to a small window around the server date so legitimate timezone differences still work, but arbitrary dates are rejected.
🔒 Suggested clamp against server date (±1 day covers all timezones)
declare
v_last_active date;
- v_today date := coalesce(p_today, current_date);
+ v_today date := coalesce(p_today, current_date);
begin
if p_minutes <= 0 then return; end if;
+
+ -- p_today is client-supplied; clamp to ±1 day of server date so timezone
+ -- skew is honored but arbitrary dates can't forge streaks.
+ if v_today < current_date - 1 or v_today > current_date + 1 then
+ v_today := current_date;
+ end if;Also worth considering: cap p_minutes at a sane maximum (e.g. 180) for the same reason — nothing server-side prevents a client from submitting p_minutes = 999999.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/012_streak_client_date.sql` around lines 5 - 22, The
SECURITY DEFINER function increment_profile_stats currently trusts the
caller-supplied p_today and p_minutes; clamp p_today to a small window around
the server date (e.g., if abs(coalesce(p_today,current_date) - current_date) > 1
day then set v_today := current_date else set v_today :=
coalesce(p_today,current_date)) so client-supplied dates outside ±1 day are
rejected, and also cap p_minutes to a sane maximum (e.g., if p_minutes > 180
then set p_minutes := 180) before any streak/last_active_date logic runs to
prevent forged streaks or inflated session times; make these adjustments at the
start of increment_profile_stats (refer to parameters p_today/p_minutes and
local variable v_today) so all subsequent logic uses the sanitized values.
| where id = p_user_id | ||
| for update; | ||
|
|
||
| if v_last_active = v_today - interval '1 day' then |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Nit: v_today - 1 is the idiomatic date arithmetic.
v_today - interval '1 day' returns a timestamp, which then implicitly casts back to date for the comparison. date - integer stays in date and avoids the cast surprise.
- if v_last_active = v_today - interval '1 day' then
+ if v_last_active = v_today - 1 then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/012_streak_client_date.sql` at line 24, The comparison
uses timestamp arithmetic causing an implicit cast; update the conditional that
currently reads "if v_last_active = v_today - interval '1 day' then" to use date
arithmetic instead by replacing the RHS with "v_today - 1" so v_today and
v_last_active remain dates; locate the conditional referencing v_last_active and
v_today and make this single-token replacement to avoid the timestamp cast.
- Watcher stats: detect focus->break transition via focusCount delta (handles autoStartBreaks=true where host never broadcasts finished state) - handleSkip: remove counter increments - skip = no credit, no DB write - Calendar grid: anchor end on coming Sunday so today always appears in a cell - BonfireState: add surge timer cleanup on unmount to prevent leak - KeyboardShortcutsModal: add role/aria-modal/aria-labelledby + focus management (save/restore on open/close) - Migration 014: sanitize p_minutes (clamp 1-180) and p_today (clamp +-1 day) in increment_profile_stats Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
1 participant
Summary
applyStateblockedonExpirefor remote completions. Fixed by logging inonTimerUpdatewith astartedAtdedup guard to prevent jam-mode double-countingincrement_profile_statswas using server UTC (current_date), causing wrong day attribution for non-UTC users. Now acceptsp_today(client local date) in both RPC call sitesbuildCalendarCellsandbuildWeekDaysused local date arithmetic but data keys are UTC - fixed to use UTC throughoutMath.flooracross StatsGrid, WeeklyChart, StreakCalendar[0, 1, 25, 60, 120]matching actual color bucketsKeyboardShortcutsModalwas imported but didn't exist, causing build failure012addsp_todayparam to function,013drops stale overloads from prior migrationsTest plan
npx supabase db pushis already done -increment_profile_statshas 4 params includingp_today🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
?key, close withEscape)Bug Fixes
UI/UX Improvements