fix: resolve DST bug in sunrise/sunset trigger timing (#3737)

[darinpope]

The original implementation of sunrise/sunset event timing had a critical DST bug where event times would jump 1+ hours across DST transitions (issue #3737). The root cause was using getTimezoneOffset() in day-of-year calculations, which changes during DST transitions.

Key Changes

DST Fix Implementation (Timer.ts)

• Replaced timezone-offset-based day calculation with dayjs for robust calendar math
• Changed time creation from local timezone to UTC-based Date.UTC() approach
• Applied offset in UTC minutes instead of local minutes to avoid DST issues
• This ensures sunrise/sunset times remain stable across DST boundaries

Testing

• Added comprehensive test suite with 536 tests covering:
  • 20 geographic locations across 6 continents (20 locations × 10 years × 2 event types = 400 baseline tests)
  • DST transitions in both spring and fall for multiple hemispheres
  • Non-DST timezones (Hawaii, Arizona, Iceland, Moscow, India, Nepal)
  • Edge cases: leap years, year boundaries, polar regions, equinoxes/solstices
  • Offset precision: positive/negative offsets, boundary values, extreme values
  • Multiple Timer instances, idempotency, concurrent updates
  • Invalid inputs: NaN, Infinity, extreme coordinates (±90°, ±180°)

Debug Logging

• Added informative debug logging on setSun() with:
  • Event type (Sunrise/Sunset)
  • Location coordinates (latitude, longitude)
  • Offset in minutes
  • Next scheduled time in both local and UTC formats

New Test Categories

• Offset Precision Near DST Boundaries
• Multiple Independent Timer Instances
• Large Negative Offset Edge Cases
• Absolute Idempotency with State Verification
• Maximum Valid Coordinate Precision

In addition to automated tests, also tested interactively with numerous lat/long combinations.

🤖 Generated with Claude Code

[CLAassistant]

All committers have signed the CLA.

[peternewman]

I'll admit I've not looked through the 3.5k lines of tests, and I'm not sure I could tell you what the correct behaviour should actually be, but have you got a test for when the offset to sunset/sunrise takes it into the magic DST hour that doesn't exist/exists twice depending on which way the clock is going?

[darinpope]

Assuming I understand your question, the tests you are looking for are in the "DST Boundary Jump Verification" test at line 1226.

[peternewman]

Assuming I understand your question, the tests you are looking for are in the "DST Boundary Jump Verification" test at line 1226.

I was meaning more "should handle offset that causes day wraparound during DST" but with an offset that takes it into the repeated/dropped hour depending on the DST direction.

[coderabbitai[bot]]

[!NOTE]

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Modified sunrise/sunset calculation in the Timer event handler to use UTC-based day-of-year logic instead of DST/timezone-dependent calculations, eliminating offset drift. Reworked sun event time construction to compute UTC timestamps, precompute nextExecute values, and introduced a new public helper method getSunNextExecuteTime for testing.

Changes

Cohort / File(s)

Summary

Sun Event Scheduling Logic companion/lib/Controls/ControlTypes/Triggers/Events/Timer.ts

Replaced DST/timezone-dependent sunrise/sunset calculation with UTC-based day-of-year logic using dayjs. Reworked sun event time construction to compute UTC timestamps from local date components. Precomputed nextExecute values and stored them in sun event entries. Added new public method getSunNextExecuteTime(id: string): number | null for retrieving scheduled sun event times. Enhanced debug logging with verbose output for both local and UTC representations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Timezone/DST logic verification: Reviewers must validate that the UTC-based day-of-year approach correctly eliminates DST offset drift and handles edge cases (e.g., daylight saving transitions, date boundary conditions)
• Timestamp construction accuracy: Careful verification needed for the UTC timestamp computation path and offset application logic
• Data flow consistency: Ensure precomputed nextExecute values are correctly threaded through and used consistently across the module
• Test coverage: The new getSunNextExecuteTime public method requires review for its contract and test-ability implications

Poem

🐰 The sun's great journey, once confusing with time zones and DST,
Now marches in UTC, so clean and so blessed!
No more offset drift to make bunnies bemoan,
Day-of-year calculations have claimed their own throne. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: resolve DST bug in sunrise/sunset trigger timing' directly and specifically describes the main change: fixing a critical DST (Daylight Saving Time) bug affecting sunrise/sunset event scheduling.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• [ ] 📝 Generate docstrings

🧪 Generate unit tests (beta)

• [ ] Create PR with unit tests
• [ ] Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d24595568220102dc51af0e92a24ebaa7fb2eeae and 07662fb24e259ccba3a767fbaca0ea421301d78b.

📒 Files selected for processing (1)

• companion/lib/Controls/ControlTypes/Triggers/Events/Timer.ts (4 hunks)

🔇 Additional comments (3)

companion/lib/Controls/ControlTypes/Triggers/Events/Timer.ts (3)

261-266: Good documentation of the DST fix rationale.

The comment clearly explains the root cause (timezone offset changes during DST) and the solution approach (using UTC to avoid offset variations). This helps future maintainers understand why UTC-based calculations are essential here.

---

516-532: Good refactoring and observability improvements.

Precomputing nextExecute before pushing to the array is cleaner, and the debug logging provides valuable visibility into sun event scheduling, which is particularly helpful for troubleshooting DST-related issues.

The logging includes all relevant details (event type, coordinates, offset, and both local/UTC times), making it easy to diagnose scheduling problems.

---

541-550: Clean test helper implementation.

The method is straightforward and correctly uses optional chaining with nullish coalescing. Documenting it as "for testing purposes" is appropriate.

If this method should remain internal to tests only, consider using a different access pattern (e.g., a test-only export or making it package-private) to signal it's not part of the stable public API.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}