fix: honor datetime_frequency_interval in datetime_frequency aggregation#1139
Merged
Conversation
The `datetime_frequency` aggregation hardcoded `calendar_interval: 'month'`, so the `datetime_frequency_interval` parameter was ignored and buckets were always monthly (#1117). Add `extractDatetimeFrequencyInterval` (validates minute/hour/day/week/ month/quarter/year, defaults to month) mirroring the geo-grid precision params, thread it through `backend.aggregate`, and override the date_histogram's `calendar_interval` accordingly. Invalid intervals now return a 400. Default behavior (month) is unchanged. Closes #1117. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes datetime_frequency aggregation so it honors the datetime_frequency_interval request parameter (previously hardcoded to monthly buckets), aligning behavior with the STAC aggregation extension expectations and OpenSearch date_histogram capabilities.
Changes:
- Added
extractDatetimeFrequencyIntervalto validate/normalize the interval (defaulting tomonth, invalid values → 400). - Threaded the interval through the API → backend aggregate call chain.
- Overrode the
datetime_frequencydate_histogram.calendar_intervalin the OpenSearch query when requested. - Added system tests for valid (
day) and invalid (fortnight) intervals, and documented the fix in the changelog.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/system/test-api-get-aggregate.ts |
Adds system tests verifying interval bucketing and invalid interval rejection. |
src/lib/types.ts |
Extends API/backend types to carry the new interval parameter through the stack. |
src/lib/database.ts |
Applies the requested calendar_interval to the datetime_frequency aggregation query. |
src/lib/api.ts |
Extracts the interval from request params and passes it to the backend. |
src/lib/api-utils.ts |
Adds interval validation/extraction helper and supported-interval list. |
CHANGELOG.md |
Documents the behavioral fix and the 400 response for invalid intervals. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1054
to
+1062
| // datetime_frequency honors the requested calendar interval (default month) | ||
| if (aggregations.includes('datetime_frequency')) { | ||
| searchParams.body.aggs['datetime_frequency'] = { | ||
| date_histogram: { | ||
| field: 'properties.datetime', | ||
| calendar_interval: datetimeFrequencyInterval | ||
| } | ||
| } | ||
| } |
Per feedback: drop sub-daily (minute/hour) granularity — day is the finest useful bucket here — and skip decade (no native OpenSearch support). Supported: day, week, month, quarter, year. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
Updated per feedback: supported intervals are now day, week, month, quarter, year — dropped sub-daily (minute/hour) and skipped decade (no native OpenSearch |
Per review: avoid duplicating the field name so it can't drift from ALL_AGGREGATIONS.datetime_frequency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jkeifer
requested changes
Jul 1, 2026
jkeifer
left a comment
jkeifer
left a comment
Member
There was a problem hiding this comment.
Looks good, but the CHANGELOG entry disagrees with the code in one key detail.
Co-authored-by: Jarrett Keifer <jkeifer@users.noreply.github.com>
jkeifer
approved these changes
Jul 1, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1117. The
datetime_frequencyaggregation hardcodedcalendar_interval: 'month'inALL_AGGREGATIONS, so thedatetime_frequency_intervalparameter was silently ignored — every request returned monthly buckets.Change
extractDatetimeFrequencyIntervalinapi-utils.ts— validates againstminute/hour/day/week/month/quarter/year, defaults tomonth(so existing behavior is unchanged), and returns 400 on an invalid value.api.aggregate→backend.aggregate(mirroring the existing geo-grid precision params).database.ts, override thedatetime_frequencydate_histogram.calendar_intervalwith the requested interval (same override pattern the geo-grid aggregations already use).Tests
datetime_frequency_interval=dayyields day-aligned buckets (2015-02-19,2015-03-23) instead of month-aligned (2015-02-01,2015-03-01).fortnight) returns 400.Verified: typecheck, lint, and the full
test-api-get-aggregatesuite (12 tests) pass.Note: like the existing aggregate precision params, this parameter isn't enumerated in
openapi.yaml, so I left the spec untouched for consistency.Closes #1117
🤖 Generated with Claude Code