Skip to content

feat(schedules): add sunrise() for the sunrises seen from a place on Earth in @observerly/orderly - #69

Merged
michealroberts merged 1 commit into
mainfrom
feature/schedules/sunrise
Sep 7, 2026
Merged

michealroberts merged 1 commit into
mainfrom
feature/schedules/sunrise

Conversation

@michealroberts

@michealroberts michealroberts commented Sep 6, 2026 •

Copy link
Copy Markdown
Member

What This Changes

  • Adds sunrise(observer) to the schedules module: the sunrises seen from a place on Earth, one a day on the days the Sun rises, resolved with @observerly/astrometry to the standard almanac convention, the upper limb of the Sun touching the horizon with refraction and the observer's elevation included. The exported Observer type is { latitude, longitude, elevation? }: degrees north and east positive, metres above sea level, sea level when omitted.
  • Honours the schedule contract: next(after) lies strictly after the instant, is pure, and returns null only where no sunrise falls within 400 days. That is the case at the poles, where astrometry places no sunrise on any day, and can be within a tenth of a degree of them, where its day-by-day search misses the one crossing in some years. The days of a polar day or a polar night are walked past.
  • Validates the observer at construction with a RangeError naming sunrise(): a latitude outside -90 to 90, a longitude outside -180 to 180, or an elevation that is not finite. Only a finite number passes, so a coordinate that is missing or null, as data read without a type may carry, is refused rather than read as zero. astrometry answers what it cannot place with silence, which would otherwise be a schedule that never fires and never says why.
  • Marks astrometry external in the rolldown build, so dist/index.js imports @observerly/astrometry/sun rather than bundling a copy of it.
  • Documents it in the README: a row in the At A Glance table and a "The Sun" subsection, ending with a deployable worker that hands each day's sunrises to the queue from a cron trigger and starts a Workflow when one is delivered.

Notes For Reviewers

  • The walk asks astrometry a UTC day at a time, starting from the day before the instant's, because an observer far east of Greenwich sees the day's sunrise before the UTC date astrometry files it under: Auckland's June solstice sunrise is 19:33 UTC the day before. Scanning 33 latitudes by 13 longitudes over 800 days found no day whose sunrise precedes the day before's and no duplicates, and the sunrise filed under a date lies between 12 hours before its midnight and 24.1 hours after, so one day of slack is exactly what the walk needs.
  • The walk keeps two days clear of either end of the range a Date can hold, one more than the calendar walks keep, and that margin is measured rather than assumed. astrometry resolves a date's event about the solar transit nearest the date's mean solar noon, which for an observer at the far west lies a full day past the date's midnight, and it throws once that search leaves the Date range. Scanning every whole degree of longitude at eleven latitudes: asked for the last representable day, it throws for every observer more than 150 degrees west; asked for the day before, it never throws anywhere. At the start of the range one day would do, but one constant keeps the edge simple. The day or two surrendered at the end of the year 275760 is the same deliberate trade candidates.ts makes for the calendar walks, and a test pins it: an observer at 180 degrees west, asked from a day and a millisecond before the end of time, exhausts rather than throws. With a one-day margin that test throws Invalid time value.
  • The 400-day bound covers every latitude to 89.9 degrees, where the longest gap astrometry leaves between sunrises is a year. Closer to the poles its daily search can leave gaps of two or three years, which read as exhaustion; that is astrometry's resolution rather than orderly's walk, and the README says so.
  • An elevation left out reads as sea level through a destructuring default, and a day without a sunrise reads as NaN, which lies after nothing, so neither needs a case of its own.
  • Expected instants in the tests are astrometry 0.69.0's own for the UTC date each sunrise is filed under, pinned to the millisecond and checked against the almanac to the minute: London 04:43 BST at the solstice and 08:06 GMT at New Year, Auckland 07:33 NZST, Honolulu 05:50 HST, and Tromsø's polar night from late November to mid January. A year-long oracle test replays astrometry day by day for Tromsø and checks the walk reproduces every sunrise, polar gaps included.
  • The README worker takes the coming day's sunrises with preview and take: 2 rather than a single next(), because for an observer whose sunrise sits near midnight UTC a UTC day holds two sunrises once or twice a year, and a single next() from a daily cron would drop one. It bounds the schedule with between to the day a queue can hold a message back, so a polar night yields nothing rather than the RangeError send() raises for an instant out of reach, and it includes the Workflow class the wrangler config names, so it deploys as written. The whole example was typechecked against @cloudflare/workers-types and orderly's source.
  • The pole test makes 400 astrometry calls, about 200 ms, since the walk searches its full bound before exhausting.
  • pnpm test now prints seven Vite notices that astrometry's shipped sourcemaps point at source files it does not publish. They are harmless; the fix belongs upstream in astrometry.
  • The type-aware lint reports six pedantic prefer-readonly-parameter-types warnings in sun.ts, the kind every schedule module already carries, the contract's own Date parameter included. No other warning is new.

Checklist

  • Tests cover the change, and run inside workerd
  • A changeset is included, or the change is not one a consumer would notice
  • No Node built-ins were introduced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new observer validation accepts non-numeric latitude/longitude values like undefined/null at runtime due to Number.isNaN checks, which can violate the stated “refuse invalid observers” contract.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a new sunrise(observer) schedule to the schedules module, backed by @observerly/astrometry, and wires it through the public API/build/docs so consumers can schedule daily sunrises for a location (including polar-day/night gaps and pole exhaustion).

Changes:

  • Introduces src/schedules/sun.ts with sunrise() plus exported Observer type and day-walk logic.
  • Adds comprehensive Vitest coverage for day-to-day behavior, global longitude offsets, elevation effects, polar gaps, and Date-horizon behavior.
  • Updates public exports, docs, changeset, and marks @observerly/astrometry as external in the rolldown build.
File summaries
File Description
tests/sunrise.spec.ts Adds functional tests for sunrise sequencing, strict “after”, global locations, elevation, purity, and input refusal.
tests/sunrise-horizons.spec.ts Adds tests for polar day/night gaps, pole exhaustion, and Date min/max edge behavior (plus oracle replay vs astrometry).
src/schedules/sun.ts Implements Observer + sunrise() via a bounded day-walk using getSunrise.
src/schedules/index.ts Re-exports sunrise and Observer from schedules entrypoint.
src/index.ts Exposes sunrise and Observer from the package root API.
rolldown.config.ts Marks @observerly/astrometry as external so it isn’t bundled into dist.
README.md Documents the new sunrise() schedule and adds a “The Sun” section with example usage.
.changeset/add-sunrise.md Declares a minor bump for adding sunrise(observer) to the public API.
Review details

Suppressed comments (1)

src/schedules/sun.ts:67

  • Same issue as latitude: using Number.isNaN(longitude) allows undefined through and treats null as 0. Using Number.isFinite(longitude) makes the constructor reliably reject non-numeric inputs at runtime.
  if (Number.isNaN(longitude) || longitude < -180 || longitude > 180) {
    throw new RangeError(`${method} requires a longitude in degrees between -180 and 180`);
  }
  • Files reviewed: 8/8 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/schedules/sun.ts Outdated
Comment thread README.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The sunrise schedule’s horizon is set two days inside the Date limit, which can prematurely exhaust and skip a valid last-day occurrence near the end of time.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/schedules/sun.ts Outdated
@michealroberts
michealroberts force-pushed the feature/schedules/sunrise branch from 2aafa87 to cc74766 Compare September 6, 2026 22:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The feature is isolated, thoroughly tested (including edge cases), and the only feedback is a minor maintainability naming clarification.

Review details

Suppressed comments (2)

src/schedules/sun.ts:51

  • This file defines a local HORIZON_IN_MILLISECONDS with different semantics (MAX − 2 days) than the exported HORIZON_IN_MILLISECONDS used by other schedule walks (MAX − 1 day). Reusing the same name for a different horizon makes code search and future refactors error-prone; consider renaming this constant to clearly indicate it is the astrometry-specific horizon.
// astrometry resolves a date's event about the solar transit nearest the date's mean solar noon, which for an
// observer at the far west falls a full day past the date's midnight, and it refuses a date whose search
// leaves the range a Date can hold. Asked for the last day of that range by an observer more than 150 degrees
// west, it throws; asked for the day before, it never does. So the walk keeps two days clear of either end,
// one more than the calendar walks keep, and surrenders those days at the end of the year 275760 deliberately,
// as they do, rather than resolving the edge by a strategy of its own.
const HORIZON_IN_MILLISECONDS = MAXIMUM_INSTANT_IN_MILLISECONDS - 2 * MILLISECONDS_IN_DAY;

src/schedules/sun.ts:114

  • This reference also needs to use the renamed astrometry-specific horizon constant to avoid confusion with the shared calendar horizon used elsewhere in schedules.
        // A day before the horizon cannot be asked for either, but the days after it can.
        if (midnight < -HORIZON_IN_MILLISECONDS) {
          continue;
        }
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/schedules/sun.ts
Comment on lines +106 to +109
// Nothing past the horizon can be asked for, so nothing past it can follow.
if (midnight > HORIZON_IN_MILLISECONDS) {
return null;
}
@michealroberts
michealroberts merged commit 06cceb9 into main Sep 7, 2026
7 checks passed
@michealroberts michealroberts mentioned this pull request Sep 14, 2026
3 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants