feat: add recording timezone and UTC offset to sleep entries - #2070
Conversation
PR Validation ResultsChange Detection
✅ All checks passed. Thank you! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughSleep entries now preserve recording timezone metadata across imports, manual entry, integrations, storage, analytics, and frontend displays. Shared utilities provide IANA and fixed-offset fallback behavior. ChangesSleep timezone support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SleepSource
participant SleepProcessing
participant SleepRepository
participant Database
participant SleepReports
SleepSource->>SleepProcessing: provide timestamps and recording-zone data
SleepProcessing->>SleepRepository: persist recording timezone or UTC offset
SleepRepository->>Database: insert or update sleep metadata
Database-->>SleepRepository: return stored metadata
SleepRepository-->>SleepReports: provide entries with recording-zone fields
SleepReports->>SleepReports: resolve zone and format sleep times
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@CodeWithCJ I was unable to verify this will work for Withings. Do you have mock data for them? |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
SparkyFitnessServer/services/sleepScienceService.ts (1)
87-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
anywith a sleep-history input type.
getWakeHourandgetSleepHouracceptentry: any. Define a narrow type forsleepStartTimestampGMT,sleepEndTimestampGMT,record_timezone, andrecord_utc_offset_minutes. This preserves the new recording-zone behavior and prevents unchecked property access.As per coding guidelines,
SparkyFitnessServer/{models,integrations,services}/**/*.tsmust avoidanydeclarations in services.🤖 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 `@SparkyFitnessServer/services/sleepScienceService.ts` around lines 87 - 107, Replace the any parameter types in getWakeHour and getSleepHour with a shared narrow sleep-history input type containing sleepStartTimestampGMT, sleepEndTimestampGMT, record_timezone, and record_utc_offset_minutes with appropriate nullable or optional value types. Keep the existing resolveRecordZone and timestamp validation behavior unchanged, and remove the explicit-any suppression once both helpers are typed.Source: Coding guidelines
🤖 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 `@shared/src/schemas/database/SleepEntries.zod.ts`:
- Around line 42-43: Update all three record_utc_offset_minutes fields across
the write schemas to validate whole-minute values with z.number().int(),
including a dedicated integer validator for
ImportHealthDataItemSchema.record_utc_offset_minutes. Leave the shared
nullableOptionalLegacyNumber unchanged because it supports fractional
measurements.
In `@shared/src/utils/timezone.ts`:
- Around line 257-261: Restrict offset metadata in the shared timezone resolver
to integer minute values from -840 through 840 before returning the "offset"
result; otherwise fall back to the existing profile-timezone behavior. Also
update the calculated-offset acceptance condition in
SparkyFitnessGarmin/routes.py lines 579-584 to require -840 <= offset_min <=
840, preserving the existing handling for invalid offsets.
- Around line 286-294: Update the offset parsing logic in the timezone utility
to validate the complete datetime structure before extracting the trailing
offset, so malformed inputs such as “T+05:30” return null. Preserve valid offset
handling and add regression coverage for malformed strings containing +HH:MM
suffixes.
In `@SparkyFitnessFrontend/src/tests/components/SleepEntrySection.test.tsx`:
- Around line 1-3: Move the SleepEntrySection test into the
src/tests/pages/CheckIn/ subtree, preserving the existing test coverage. Replace
the direct `@testing-library/react` render import with the shared render exported
by test-utils.tsx, while retaining the other required testing imports.
In `@SparkyFitnessFrontend/src/tests/utils/formatTimeInZoneDstGap.test.ts`:
- Around line 1-4: Update the test suite setup around the process.env['TZ']
assignment to capture the existing TZ value, set America/New_York in beforeAll,
and restore the captured value or delete the variable in afterAll. Ensure
cleanup runs after the suite so later Jest tests do not inherit the forced
timezone.
In `@SparkyFitnessFrontend/src/utils/healthDataImport.ts`:
- Around line 216-224: Validate recording-zone metadata before persistence
across SparkyFitnessFrontend/src/utils/healthDataImport.ts:216-224,
SparkyFitnessServer/routes/sleepRoutes.ts:135-152 and :360-376, and the import
schema; require record_timezone to be a valid IANA zone and
record_utc_offset_minutes to be an integer whole-minute value within the
supported range. Update readNumberCell and nullableOptionalLegacyNumber
validation as needed, and ensure both sleep create/update handlers and
SparkyFitnessServer/integrations/healthData/healthDataRoutes.ts reject invalid
raw zone fields before forwarding them to persistence.
In `@SparkyFitnessServer/tests/chatbotToolsCheckin.test.ts`:
- Around line 645-661: Extend the test “leaves the stamp unset for Z-suffixed
and generated timestamps” with a separate invocation of
sparky_manage_checkin.execute for log_sleep that omits both bedtime and
wake_time, then assert the resulting processSleepEntry payload has
record_utc_offset_minutes undefined. Keep the existing Z-suffixed timestamp
assertion intact.
In `@SparkyFitnessServer/tests/sleepRepository.placeholders.test.ts`:
- Around line 73-79: Replace the any-typed mockClient, queryCalls call
arguments, and all corresponding values types in sleep repository tests with
explicit mock/query types and unknown where values are not known. Remove every
`@typescript-eslint/no-explicit-any` suppression at the referenced locations while
preserving the existing mock behavior and query-call assertions.
---
Nitpick comments:
In `@SparkyFitnessServer/services/sleepScienceService.ts`:
- Around line 87-107: Replace the any parameter types in getWakeHour and
getSleepHour with a shared narrow sleep-history input type containing
sleepStartTimestampGMT, sleepEndTimestampGMT, record_timezone, and
record_utc_offset_minutes with appropriate nullable or optional value types.
Keep the existing resolveRecordZone and timestamp validation behavior unchanged,
and remove the explicit-any suppression once both helpers are typed.
🪄 Autofix
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: ac6a6c31-80e2-4ab1-b27e-82a4ea63409b
📒 Files selected for processing (40)
SparkyFitnessFrontend/src/constants/healthDataImport.tsSparkyFitnessFrontend/src/pages/CheckIn/SleepEntrySection.tsxSparkyFitnessFrontend/src/pages/Reports/SleepAnalyticsCharts.tsxSparkyFitnessFrontend/src/pages/Reports/SleepAnalyticsTable.tsxSparkyFitnessFrontend/src/pages/Reports/SleepReport.tsxSparkyFitnessFrontend/src/pages/Reports/SleepStageChart.tsxSparkyFitnessFrontend/src/tests/components/SleepEntrySection.test.tsxSparkyFitnessFrontend/src/tests/utils/formatTimeInZoneDstGap.test.tsSparkyFitnessFrontend/src/tests/utils/healthDataImport.test.tsSparkyFitnessFrontend/src/tests/utils/timeFormatters.test.tsSparkyFitnessFrontend/src/types.tsSparkyFitnessFrontend/src/utils/healthDataImport.tsSparkyFitnessFrontend/src/utils/timeFormatters.tsSparkyFitnessGarmin/routes.pySparkyFitnessServer/ai/tools/checkinTools.tsSparkyFitnessServer/config/swagger.tsSparkyFitnessServer/db/migrations/20260807000000_add_record_timezone_to_sleep_entries.sqlSparkyFitnessServer/integrations/fitbit/fitbitDataProcessor.tsSparkyFitnessServer/integrations/googlehealth/googleHealthDataProcessor.tsSparkyFitnessServer/integrations/healthData/healthDataRoutes.tsSparkyFitnessServer/integrations/oura/ouraDataProcessor.tsSparkyFitnessServer/integrations/polar/polarDataProcessor.tsSparkyFitnessServer/integrations/withings/withingsDataProcessor.tsSparkyFitnessServer/models/sleepRepository.tsSparkyFitnessServer/models/sleepScienceRepository.tsSparkyFitnessServer/routes/sleepRoutes.tsSparkyFitnessServer/services/fitbitService.tsSparkyFitnessServer/services/healthDataHandlers.tsSparkyFitnessServer/services/sleepScienceService.tsSparkyFitnessServer/tests/chatbotToolsCheckin.test.tsSparkyFitnessServer/tests/fitbitDataProcessor.test.tsSparkyFitnessServer/tests/googleHealthDataProcessor.test.tsSparkyFitnessServer/tests/measurementService.healthConnectSleepStages.test.tsSparkyFitnessServer/tests/ouraDataProcessor.test.tsSparkyFitnessServer/tests/polarDataProcessor.test.tsSparkyFitnessServer/tests/sleepRepository.placeholders.test.tsSparkyFitnessServer/tests/sleepScienceService.recordZone.test.tsSparkyFitnessServer/tests/timezone.test.tsshared/src/schemas/database/SleepEntries.zod.tsshared/src/utils/timezone.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
SparkyFitnessServer/schemas/measurementSchemas.ts (1)
58-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression tests for legacy integer parsing.
Cover whole-number numbers and strings,
null, empty strings, and fractional numbers and numeric strings. This verifies that legacy coercion remains compatible while fractional UTC offsets fail validation.🤖 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 `@SparkyFitnessServer/schemas/measurementSchemas.ts` around lines 58 - 72, Add regression tests for nullableOptionalLegacyInteger covering whole-number numeric and string inputs, null, and empty strings as accepted values, plus fractional numeric and numeric-string inputs as rejected values. Exercise the schema directly and verify fractional UTC offsets fail validation while preserving legacy coercion behavior.
🤖 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 `@shared/src/utils/timezone.ts`:
- Around line 299-303: Update the offset parsing logic near the hours/minutes
validation to return null when the parsed sign is "-" and total equals zero,
covering both -00:00 and -0000 while preserving valid nonzero offsets. Add
regression tests for both input forms and verify resolveRecordZone uses the
profile-timezone fallback.
In `@SparkyFitnessServer/tests/measurementImportRoutes.test.ts`:
- Around line 204-218: The SleepSession import test does not verify that
record_utc_offset_minutes is forwarded unchanged. Extend the assertions after
the processHealthData call in the post test to inspect its argument and confirm
the forwarded item retains record_utc_offset_minutes: -300, while preserving the
existing status and call-count checks.
---
Nitpick comments:
In `@SparkyFitnessServer/schemas/measurementSchemas.ts`:
- Around line 58-72: Add regression tests for nullableOptionalLegacyInteger
covering whole-number numeric and string inputs, null, and empty strings as
accepted values, plus fractional numeric and numeric-string inputs as rejected
values. Exercise the schema directly and verify fractional UTC offsets fail
validation while preserving legacy coercion behavior.
🪄 Autofix
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: 7fb223d5-7349-4717-bce3-f5ecd5e07625
📒 Files selected for processing (8)
SparkyFitnessFrontend/src/tests/utils/formatTimeInZoneDstGap.test.tsSparkyFitnessGarmin/routes.pySparkyFitnessServer/schemas/measurementSchemas.tsSparkyFitnessServer/services/sleepScienceService.tsSparkyFitnessServer/tests/measurementImportRoutes.test.tsSparkyFitnessServer/tests/sleepRepository.placeholders.test.tsSparkyFitnessServer/tests/timezone.test.tsshared/src/utils/timezone.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- SparkyFitnessFrontend/src/tests/utils/formatTimeInZoneDstGap.test.ts
- SparkyFitnessServer/tests/timezone.test.ts
- SparkyFitnessServer/services/sleepScienceService.ts
- SparkyFitnessGarmin/routes.py
- SparkyFitnessServer/tests/sleepRepository.placeholders.test.ts
Tip
Help us review and merge your PR faster!
Please ensure you have completed the Checklist below.
For Frontend changes, please run
pnpm run validateto check for any errors.PRs that include tests and clear screenshots are highly preferred!
Note: AI-generated descriptions must be manually edited for conciseness. Do not paste raw AI summaries.
Description
What problem does this PR solve?
Sleep bed/wake times were stored as UTC instants without data on how they were recorded so every display falls back to the user's current time zone. Travel across time zones relabeled history.
How did you implement the solution?
Add two columns to sleep_entries, record_timezone (IANA) and record_utc_offset_minutes. When importing the system will fill in the timezone based on the data provided and the priority: IANA timezone, then the offset, and finally use the users saved time zone preference. Entry date bucketing is untouched and there's no backfill. This will only apply to new sleep entries.
Known limitations to be addressed in a follow up PR:
Linked Issue: Closes #2033
How to Test
PR Type
Checklist
All PRs:
New features only:
Frontend changes (
SparkyFitnessFrontend/):pnpm run validateand it passes.en) translation file.Backend changes (
SparkyFitnessServer/):rls_policies.sqlfor any new user-specific tables.UI changes (components, screens, pages):
Mobile changes (
SparkyFitnessMobile/):Screenshots
Click to expand
Before
After
Notes for Reviewers
Summary by CodeRabbit
New Features
Bug Fixes