Skip to content

feat(alerts): add scheduleStartAt + scheduleOffsetMinutes#1745

Open
mlsalcedo wants to merge 31 commits intohyperdxio:mainfrom
mlsalcedo:startTime-offset
Open

feat(alerts): add scheduleStartAt + scheduleOffsetMinutes#1745
mlsalcedo wants to merge 31 commits intohyperdxio:mainfrom
mlsalcedo:startTime-offset

Conversation

@mlsalcedo
Copy link

@mlsalcedo mlsalcedo commented Feb 17, 2026

Closes #1715

Summary

  • Add scheduleStartAt and scheduleOffsetMinutes to alert schemas and API validation.
  • Update alert evaluation scheduling to anchor windows by scheduleStartAt when set.
  • Skip evaluations before scheduleStartAt.
  • Keep current behavior unchanged when scheduling fields are unset.
  • Add UI fields and API/OpenAPI/external API support for the new schedule options.
  • Add alert scheduler tests for anchored windows and pre-start skip behavior.

Notes

  • This enables Splunk-style scheduled monitor migration where checks must run on isolated, periodic windows anchored to specific times.
  • scheduleStartAt is the primary anchor; scheduleOffsetMinutes remains optional for backward-compatible alignment.

@vercel
Copy link

vercel bot commented Feb 17, 2026

@melsalcedo is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot
Copy link

changeset-bot bot commented Feb 17, 2026

🦋 Changeset detected

Latest commit: 2f3c582

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@hyperdx/api Minor
@hyperdx/app Minor
@hyperdx/common-utils Minor
@hyperdx/otel-collector Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@sgarfinkel sgarfinkel left a comment

Choose a reason for hiding this comment

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

Left some feedback

mlsalcedo and others added 4 commits February 17, 2026 10:36
Co-authored-by: Sam Garfinkel <10210461+sgarfinkel@users.noreply.github.com>
Co-authored-by: Sam Garfinkel <10210461+sgarfinkel@users.noreply.github.com>
@teeohhem teeohhem self-requested a review February 19, 2026 18:09
@github-actions
Copy link
Contributor

github-actions bot commented Feb 19, 2026

PR Review: feat(alerts): add scheduleStartAt + scheduleOffsetMinutes

No critical issues found. Implementation is well-structured with solid test coverage.

A few items worth noting:

  • WARNING: validateAlertScheduleOffsetMinutes skips offset validation when scheduleStartAt is set — The check scheduleOffsetMinutes >= intervalMinutes still runs even when scheduleStartAt is provided (though it correctly does nothing at that point since offset is forced to 0). Not a bug, just a minor clarity issue.

  • WARNING: Date range limits use approximate year lengths (365 * 24 * 60 * 60 * 1000) — won't account for leap years. Negligible in practice but worth noting.

  • INFO: Redundant max(1439) on base Zod schema in zod.ts and types.ts — the superRefine already enforces the dynamic per-interval max. Not harmful, just redundant.

  • INFO: makeAlert calls new Date(alert.scheduleStartAt) without error handling — safe because alertSchema.parse() is now called before makeAlert in all routers, ensuring scheduleStartAt is a valid ISO string. The recent change to explicitly parse in routers (previously req.body was used directly) is a good fix.

The backward-compat strategy via normalizeNoOpAlertScheduleFields and the null-vs-undefined distinction for scheduleStartAt serialization are well thought out.

Reviewed by Claude Sonnet 4.6

@github-actions
Copy link
Contributor

Code Review

  • Critical schema bug in packages/api/src/utils/zod.ts → The new .superRefine(validateAlertScheduleOffsetMinutes) is inserted with a semicolon before the existing .and(zSavedSearchAlert.or(zTileAlert)) line, making the trailing .and(...) a dead no-op expression. The alertSchema ends up missing the source discriminator after superRefine, meaning savedSearchId/dashboardId/tileId validation is silently skipped on the internal API. Remove the stale .and(zSavedSearchAlert.or(zTileAlert)) line that follows the semicolon.

  • ⚠️ normalizeNoOpAlertScheduleFields silently drops scheduleOffsetMinutes: 0 reset (packages/app/src/utils/alerts.ts) → When previousAlert is null/undefined and the user explicitly sets offset to 0, previousHadOffset is false so the field is deleted instead of being sent to the API to clear a stale DB value. The user's explicit reset is silently ignored.

  • ⚠️ (alert as any) casts for createdBy in DBEditTimeChartForm.tsx → Extend the local alert type to include createdBy instead of bypassing TypeScript.

  • ⚠️ Offset-cap enforcement (scheduleOffsetMinutes < intervalMinutes) is only applied via superRefine → Since this sits after the broken schema chain (see item 1), invalid offsets can pass validation on the internal API. Fixing item 1 resolves this automatically.

  • ℹ️ Missing test: scheduleOffsetMinutes surviving an update when scheduleStartAt is absent → Add a test to confirm the existing DB offset is not zeroed on a partial update that omits the offset field.

🤖 Generated with Claude Code

@teeohhem
Copy link
Contributor

Thanks for this. I plan on finishing up my review Monday morning with some feedback.

…arttime-offset

# Conflicts:
#	packages/api/src/utils/zod.ts
#	packages/app/src/DBSearchPageAlertModal.tsx
#	packages/app/src/components/DBEditTimeChartForm.tsx
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.

Feature Request: Alert Scheduling with Absolute Start Time (plus optional offset)

4 participants