Repository navigation
Add timezone support for accurate time-of-day context in AI responses - #372
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
⌛ 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. 📝 WalkthroughWalkthroughThis PR adds comprehensive timezone support to the application. It introduces database schema changes (timezone column), new timezone utility functions for validation and time calculations, updates pulse generation and cron routes to perform timezone-aware time computations, captures browser timezone from the frontend, and updates test data to include timezone fields. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend as Pulse.tsx
participant Browser as Browser API
participant PulseAPI as /api/pulse/generate
participant TimezoneUtils as Timezone Utils
participant Database as DB
User->>Frontend: Trigger pulse refresh
Frontend->>Browser: Get resolved timezone
Browser-->>Frontend: Browser timezone (e.g., "America/New_York")
Frontend->>PulseAPI: POST with { timezone }
PulseAPI->>TimezoneUtils: normalizeTimezone(clientTimezone)
TimezoneUtils-->>PulseAPI: Validated timezone
PulseAPI->>TimezoneUtils: getUserTimeOfDay(userTimezone)
TimezoneUtils-->>PulseAPI: { hour, timeOfDay }
PulseAPI->>TimezoneUtils: getStartOfTodayInTimezone(userTimezone)
TimezoneUtils-->>PulseAPI: Start of day in UTC
PulseAPI->>PulseAPI: Build timezone-aware prompt with timeOfDay
alt Timezone differs from stored
PulseAPI->>Database: Update user.timezone
Database-->>PulseAPI: Success
end
PulseAPI->>PulseAPI: Generate pulse with timezone context
PulseAPI-->>Frontend: Pulse response
Frontend-->>User: Updated pulse
sequenceDiagram
participant CronJob as Pulse Cron
participant UserService as User Service
participant TimezoneUtils as Timezone Utils
participant PromptBuilder as Prompt Builder
participant ActivityData as Activity Data
CronJob->>UserService: Fetch all users
UserService-->>CronJob: User list with timezone
loop For each user
CronJob->>TimezoneUtils: normalizeTimezone(user.timezone)
TimezoneUtils-->>CronJob: Normalized timezone
CronJob->>TimezoneUtils: getStartOfTodayInTimezone(timezone)
TimezoneUtils-->>CronJob: Today's start in UTC
CronJob->>TimezoneUtils: getUserTimeOfDay(timezone)
TimezoneUtils-->>CronJob: { hour, timeOfDay }
CronJob->>ActivityData: Fetch activities since today
ActivityData-->>CronJob: Drive activities with drive scopes
CronJob->>PromptBuilder: buildTimestampSystemPrompt(userTimezone)
PromptBuilder-->>CronJob: Timezone-aware system prompt
CronJob->>CronJob: Generate pulse with timezone context
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 2
🤖 Fix all issues with AI agents
In `@apps/web/src/app/api/pulse/generate/route.ts`:
- Around line 109-128: Validate any clientTimezone string before using or
persisting it: implement an isValidTimezone(tz: string) helper that returns true
only if new Intl.DateTimeFormat('en-US', { timeZone: tz }) does not throw (catch
RangeError). Use this helper when parsing req.json() to only set clientTimezone
if valid, fall back to storing/using user?.timezone only if that value also
passes validation, and otherwise default to 'UTC'. Apply the same validation
before calling db.update(users).set({ timezone: clientTimezone }) so you never
persist an invalid timezone.
In `@apps/web/src/lib/ai/core/timestamp-utils.ts`:
- Around line 53-78: The code constructs localMidnight with new Date(dateStr)
which is parsed in the server's local timezone; instead build the timestamp as
UTC using Date.UTC(year, month, day, 0, 0, 0) (i.e., replace new Date(dateStr)
with new Date(Date.UTC(year, month, day, 0, 0, 0))) and then use that Date
instance when calling tzFormatter.formatToParts(localMidnight) to obtain
tzOffset and compute offsetMs as before (variables: dateStr, localMidnight,
tzFormatter, tzOffset, offsetMatch); this ensures the midnight candidate is
constructed in UTC instead of the server local timezone before applying the
timezone offset to produce the correct UTC moment for the user's midnight.
🧹 Nitpick comments (4)
apps/web/src/lib/ai/core/timestamp-utils.ts (2)
23-26: Time-of-day categorization has overlapping logic.The
timeOfDayvariable is initialized to'day'but this value can never be returned because the conditions cover all hours (0-11 → morning, 12-16 → afternoon, 17+ → evening). The'day'initialization is dead code.While this doesn't cause a bug, consider removing the unreachable default or clarifying intent:
♻️ Suggested simplification
- let timeOfDay = 'day'; - if (hour < 12) timeOfDay = 'morning'; - else if (hour < 17) timeOfDay = 'afternoon'; - else timeOfDay = 'evening'; + let timeOfDay: string; + if (hour < 12) timeOfDay = 'morning'; + else if (hour < 17) timeOfDay = 'afternoon'; + else timeOfDay = 'evening';
66-78: Edge case: "GMT" without offset may not match regex.Some runtimes format UTC timezone as simply
"GMT"rather than"GMT+00:00". The regex at Line 69 requires the+/-and digits, so it would fall through to the fallback at Line 81. While the fallback is correct for UTC, consider adding explicit handling or documenting this behavior.apps/web/src/lib/ai/core/__tests__/timestamp-utils.test.ts (1)
14-26: Consider usingvi.useFakeTimers()for cleaner Date mocking.The manual Date class override pattern is verbose and requires cleanup in each test. Vitest provides built-in timer mocking that's cleaner and safer:
♻️ Suggested refactor using Vitest fake timers
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { describe, it, expect } from 'vitest'; import { getUserTimeOfDay, getStartOfTodayInTimezone, buildTimestampSystemPrompt, } from '../timestamp-utils'; describe('timestamp-utils', () => { + afterEach(() => { + vi.useRealTimers(); + }); + describe('getUserTimeOfDay', () => { it('should return morning for hours before 12', () => { - // Mock the current time to 9 AM UTC - const testDate = new Date('2024-06-15T09:00:00Z'); - const originalDate = Date; - global.Date = class extends originalDate { - constructor(...args: unknown[]) { - if (args.length === 0) { - super(testDate); - } else { - // `@ts-expect-error` - spread args - super(...args); - } - } - static now() { - return testDate.getTime(); - } - } as DateConstructor; + vi.useFakeTimers(); + vi.setSystemTime(new Date('2024-06-15T09:00:00Z')); const result = getUserTimeOfDay('UTC'); expect(result.hour).toBe(9); expect(result.timeOfDay).toBe('morning'); - - global.Date = originalDate; });apps/web/src/app/api/pulse/cron/route.ts (1)
311-311: Non-null assertion ondriveIdmay cause runtime errors.Line 311 uses
activity.driveId!with a non-null assertion, butdriveIdcould potentially be null based on the query. Consider adding a filter or guard:♻️ Suggested defensive check
activitiesForDiff.push({ id: activity.id, timestamp: activity.timestamp, pageId: activity.pageId, resourceTitle: activity.resourceTitle, changeGroupId: activity.changeGroupId, aiConversationId: activity.aiConversationId, isAiGenerated: activity.isAiGenerated, actorEmail: activity.actorEmail, actorDisplayName: activity.actorName, content: activity.contentSnapshot ?? null, - driveId: activity.driveId!, + driveId: activity.driveId ?? '', });Or filter out activities without driveId earlier in the pipeline.
The pulse feature was using server time for all calculations, causing issues like "good morning" messages appearing at the wrong time of day for users in different timezones. Changes: - Add timezone field to users table schema - Update buildTimestampSystemPrompt to accept timezone parameter - Update pulse/generate route to use user timezone for time-of-day calculations and pass timezone to AI context - Update Pulse component to send browser timezone on refresh - Update pulse cron route to use stored user timezone - Auto-save user timezone preference when provided by client The browser timezone is detected using Intl.DateTimeFormat and stored in the user profile for use by the cron job when generating scheduled pulses. https://claude.ai/code/session_014W8BwjdD8e5Jo1kYM45ey9
45af101 to
397fb3c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/lib/ai/core/timestamp-utils.ts`:
- Around line 38-55: The getUserTimeOfDay function can receive "24" at midnight
because Intl.DateTimeFormat with hour12: false may use h24; update the
DateTimeFormat options in getUserTimeOfDay (where formatter is constructed) to
include hourCycle: 'h23' (after calling normalizeTimezone(timezone)) so the
formatter always returns 0–23, then continue parsing as before; no other logic
changes needed.
| export function getUserTimeOfDay(timezone?: string | null): { hour: number; timeOfDay: TimeOfDay } { | ||
| const tz = normalizeTimezone(timezone); | ||
|
|
||
| // Get the hour in the user's timezone | ||
| const formatter = new Intl.DateTimeFormat('en-US', { | ||
| timeZone: tz, | ||
| hour: 'numeric', | ||
| hour12: false, | ||
| }); | ||
|
|
||
| const hour = parseInt(formatter.format(new Date()), 10); | ||
|
|
||
| let timeOfDay: TimeOfDay; | ||
| if (hour < 12) timeOfDay = 'morning'; | ||
| else if (hour < 17) timeOfDay = 'afternoon'; | ||
| else timeOfDay = 'evening'; | ||
|
|
||
| return { hour, timeOfDay }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/src/lib/ai/core/timestamp-utils.ts | head -60Repository: 2witstudios/PageSpace
Length of output: 2215
🌐 Web query:
Intl.DateTimeFormat hour12 false behavior midnight "24" ECMAScript specification
💡 Result:
In ECMA-402 (Intl), hour12: false does not mandate "00" vs "24" at midnight. Instead, it forces Intl.DateTimeFormat to use the locale’s 24‑hour hour cycle ([[hourCycle24]]), which is allowed to be either:
"h23"= hours 0–23 (midnight formats as 00:00)"h24"= hours 1–24 (midnight can format as 24:00) (402.ecma-international.org)
Which one you get is therefore locale-data / implementation dependent (within the spec’s constraints), because [[hourCycle24]] is implementation-defined but constrained to "h23" or "h24". (402.ecma-international.org)
Also, resolvedOptions().hour12 is derived from the resolved hourCycle for web-compat (true for h11/h12, false for h23/h24). (402.ecma-international.org)
Practical note: "h24" (24:00 midnight) is generally not used in common locale data, so most engines/locales will yield "h23" → 00:00. (developer.mozilla.org)
Citations:
- 1: https://402.ecma-international.org/12.0/?utm_source=openai
- 2: https://402.ecma-international.org/12.0/?utm_source=openai
- 3: https://402.ecma-international.org/?utm_source=openai
- 4: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/Locale/getHourCycles?utm_source=openai
Add explicit hourCycle: 'h23' to guard against edge-case "24" return at midnight.
Per ECMA-402, hour12: false allows implementations to use either h23 (0–23) or h24 (1–24) hour cycles. While h24 is uncommon in locale data, the spec permits it, which would cause midnight to return "24". Add the hourCycle option to enforce consistent behavior.
🔧 Suggested fix
const formatter = new Intl.DateTimeFormat('en-US', {
timeZone: tz,
hour: 'numeric',
hour12: false,
+ hourCycle: 'h23',
});🤖 Prompt for AI Agents
In `@apps/web/src/lib/ai/core/timestamp-utils.ts` around lines 38 - 55, The
getUserTimeOfDay function can receive "24" at midnight because
Intl.DateTimeFormat with hour12: false may use h24; update the DateTimeFormat
options in getUserTimeOfDay (where formatter is constructed) to include
hourCycle: 'h23' (after calling normalizeTimezone(timezone)) so the formatter
always returns 0–23, then continue parsing as before; no other logic changes
needed.
2 participants
Summary
This PR adds timezone support to the application to ensure AI-generated pulse summaries use accurate time-of-day context based on the user's actual location, rather than server time.
Key Changes
Database Schema: Added
timezonecolumn to theuserstable to store user's IANA timezone preference (e.g., "America/New_York")Timezone Utilities: Created new
timestamp-utils.tsmodule with three core functions:getUserTimeOfDay(): Determines time period (morning/afternoon/evening) based on user's timezonegetStartOfTodayInTimezone(): Calculates midnight in user's timezone for accurate "today" calculationsbuildTimestampSystemPrompt(): Enhanced to include timezone and time-of-day context in AI system promptsClient-Side Detection: Updated the Pulse component to detect browser timezone using
Intl.DateTimeFormat().resolvedOptions().timeZoneand send it to the APIAPI Endpoints: Modified both
/api/pulse/generateand/api/pulse/cronto:Test Coverage: Added comprehensive test suite for timezone utilities covering:
timezonefieldImplementation Details
Intl.DateTimeFormatAPI for reliable timezone handling across different environmentshttps://claude.ai/code/session_014W8BwjdD8e5Jo1kYM45ey9
Summary by CodeRabbit
Release Notes
New Features
Tests