Skip to content

Fix hourly bucket mapping across DST transitions - #6626

Open
jaideeppyne wants to merge 2 commits into
plausible:masterfrom
jaideeppyne:fix-hourly-time-labels-dst
Open

Fix hourly bucket mapping across DST transitions#6626
jaideeppyne wants to merge 2 commits into
plausible:masterfrom
jaideeppyne:fix-hourly-time-labels-dst

Conversation

@jaideeppyne

@jaideeppyne jaideeppyne commented Aug 29, 2026

Copy link
Copy Markdown

Changes

Fix hourly bucket identity and the response spine together, including sparse data on 23-hour and 25-hour days and multi-day ranges.

  • Query ClickHouse's hourly bucket as a Unix timestamp in the existing dimension, for both events and smeared sessions.
  • Keep that absolute key through result indexing and comparison matching; convert to a local display string only when constructing the response.
  • Generate the hourly spine from each local day's start in elapsed-hour steps and filter by absolute overlap with the query range. This also covers partial-hour DST transitions and sub-day windows whose local endpoints run backward.
  • Map legacy timeseries, sparklines and CSV rows through result indices, so two occurrences of the same local hour do not collapse or duplicate metrics.
  • Compute the current-hour index using absolute bucket intervals.

Comparison behavior

Preserve the existing positional alignment: bucket 0 compares with bucket 0, bucket 1 with bucket 1, and so on. This is elapsed-bucket alignment, not same-wall-clock-hour alignment after a DST transition. Both series keep their own length; an unmatched final comparison bucket retains its metrics and has change: nil. The frontend already uses the longer series length.

Why not plain WITH FILL?

A single WITH FILL STEP toIntervalHour(1) over a multi-day range does not necessarily match toStartOfHour around half-hour offset changes. In a local ClickHouse 26.7.5.10 experiment spanning October 6-7, 2024 in Australia/Lord_Howe, a sparse fill produced 49 buckets, including 24 spurious half-hour buckets on October 7; aggregating minute samples produced 48 buckets and no such October 7 buckets.

This implementation leaves metric rows sparse and replaces the existing dimension value rather than adding a second timestamp column. It does not claim a production bandwidth benchmark or rule out a more elaborate ClickHouse-generated spine.

Validation

Prepared with AI assistance; these are tool-run results, not a claim of independent human review.

  • 41 focused time/DST tests pass, covering sparse repeated hours, events and sessions, legacy/sparkline/CSV mapping, comparisons, partial windows, and eight direct ClickHouse bucket/spine comparisons.
  • Statistics tests plus main graph, dashboard CSV and external Stats API tests: 1,346 / 1,347 passed, with two excluded tests.
  • The one failure, ExplorationTest at exploration_test.exs:781, reproduces unchanged on the original PR head c92fcfd in the same environment.
  • Formatting and git diff --check pass.
  • Local environment: Elixir 1.20.4, PostgreSQL 17.11, ClickHouse 26.7.5.10. This is not validation against the project's ClickHouse 25.11 service image.

The earlier Tzdata-only exhaustive-sweep claim is not used as evidence for this revision: the relevant oracle is ClickHouse's actual bucket behavior.

Tests

  • Automated tests have been added

Changelog

  • Entry has been added to changelog

Documentation

  • This change does not need a documentation update

Dark mode

  • This PR does not change UI styling

time_labels/1 built the time:hour labels by adding a fixed number of hours
to the naive start of the range, with the count taken from the absolute
length of the range. ClickHouse buckets these rows with
toStartOfHour(toTimeZone(timestamp, tz)), so on a day that gains or loses
an hour the two disagree: the last hour of a spring-forward day gets no
label and its traffic drops off the graph, and a nonexistent hour is
labelled instead.

Walk the local hours of the range and keep the ones that occur in the
site timezone.
@CLAassistant

CLAassistant commented Aug 29, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@jaideeppyne
jaideeppyne force-pushed the fix-hourly-time-labels-dst branch from da29f1d to c92fcfd Compare August 31, 2026 13:41
@apata

apata commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Thanks, @jaideeppyne! The buckets we generate app-side (for the response spine) are wrong indeed.

Steps to reproduce

  • Visit dashboard of site with time zone that has DST
  • Select custom period 1 day that has DST switchover (e.g. Mar 27 2026 Europe/Tallinn)

Expected

  • 23 buckets of data
  • Time labels: 00:00, 01:00, 02:00, 04:00, 05:00, 06:00, ... 23:00
  • All buckets have at least some visitors (for a popular site)

Actual

  • 23 buckets of data
  • Time labels: 00:00, 01:00, 02:00, 03:00, 04:00, 05:00, ... 22:00
  • One bucket value is 0, there's a dip in the chart in the 3rd bucket

Extra info

  • Same class of issue for the switch at the other end (e.g. Oct 26 2025 Europe/Tallinn), except the buckets are mislabeled (the hour 3 should appear twice in a row 00:00, 01:00, 02:00, 03:00, 03:00, 04:00, ..., 23:00).
  • Same class of issue isn't just with 1-day-long queries, also "Last 7 days" or any other multi-day query that supports time:hour interval. The stored data in continuous in Clickhouse, but it's being mislabeled app side and some of it is dropped.

How does this PR handle queries with an hourly interval that cover days that are 25h long locally?

It seems to me we can't handle them properly without making changes in what we query from Clickhouse (~CH).

If the data from CH is labeled with local timestamps, and local timestamp is the basis for mapping the CH value to the response spine, then for a sparse CH response returning one item local datetime: 2025-10-26 03:00:00, visitors: 10, we don't know which 03:00:00 bucket in the response spine to account those visitors in.

My gut feel is that a full fix would involve

  1. fixing the buckets in the response spine, for queries containing 23h long days and 25h long days
  2. requesting an extra column from CH that allows us to distinguish what response bucket a value is for, e.g. local datetime: 2025-10-26 03:00:00, datetime: 2025-10-26 00:00:00, visitors: 10, sample below
SELECT toStartOfHour(toTimeZone(timestamp, 'Europe/Tallinn')) h,
       toDateTime(toUnixTimestamp(h)) d,
       uniq(user_id) visitors
FROM events_v2
WHERE site_id = 1
  AND name != 'engagement'
  AND timestamp >= toDateTime('2025-10-25 21:00:00')   -- 2025-10-26 00:00:00 EEST
  AND timestamp <= toDateTime('2025-10-26 21:59:59')   -- 2025-10-26 23:59:59 EET
GROUP BY h
ORDER BY h
[{
  "h": "2025-10-26 03:00:00",
  "d": "2025-10-26 00:00:00",
  "visitors": 219
},
{
  "h": "2025-10-26 03:00:00",
  "d": "2025-10-26 01:00:00",
  "visitors": 196
}]

@jaideeppyne

Copy link
Copy Markdown
Author

Thanks, that reading is right. This PR fixes the app-side hourly spine generation, including not inventing nonexistent spring-forward hours and not stopping at 22:00 on 23-hour days, but it does not fully solve the 25-hour fall-back case if result mapping is keyed only by the local timestamp string.

For a fall-back day with two 03:00:00 local buckets, the response spine can contain two labels, but the CH rows still need an unambiguous bucket identity. If the aggregation result only returns the local hour, a sparse row for 2025-10-26 03:00:00 cannot tell which occurrence it belongs to. So I agree the full fix needs an additional absolute bucket key from CH, for example UTC bucket start / unix timestamp alongside the display-local label, and the response mapper should join on that key rather than only on local datetime text.

I can take either direction from here: broaden this PR to add that CH-side disambiguator and cover both 23h and 25h days end-to-end, or narrow this PR to the spring-forward/dropped-bucket case and leave the fall-back ambiguity for a separate change.

@apata

apata commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

@jaideeppyne Thanks for your quick reply!

Of the two, I'd personally prefer the comprehensive solution. I think the ideal solution highlights how important it is to build the response spine correctly app-side if we don't want to use WITH FILL Clickhouse-side. At the moment and even after this PR, it seems to fly under the radar as labels-related, when in reality, as you found, invalid buckets are injected and legitimate buckets returned by CH are dropped in-app.

It's a significant change though and work will likely spill over to comparisons. Comparisons are a bit of a sore spot with difficult decisions. For example, when comparing hourly visitors for the last 7 days with hourly visitors from day -15 to day -8, and one of those periods has more or less hours than the other, how to align the comparison?

TBH I'm actually curious if it'd be possible to get CH to do the spine using WITH FILL. It would eliminate a class of errors completely. If most of the stats requests are not sparse, are increased HTTP payloads between CH / app from use of WITH FILL comparable to requesting an additional disambiguating timestamp column?

@jaideeppyne

Copy link
Copy Markdown
Author

Agreed, I’ll broaden this beyond a label-only fix. I’ll first check whether ClickHouse WITH FILL can generate the hourly spine correctly for the requested timezone without making sparse series/comparison payloads worse. If that does not hold up, I’ll switch the app-side mapping to a stable absolute bucket key and keep the local hour label display-only, with comparison behavior called out explicitly rather than hidden in this PR.

@jaideeppyne jaideeppyne changed the title Fix hourly time labels dropping an hour on DST transition days Fix hourly bucket mapping across DST transitions Sep 3, 2026
@jaideeppyne

Copy link
Copy Markdown
Author

Implemented the comprehensive follow-up in 1af1f55, and rewrote the PR description to match the actual scope and validation.

Hourly event/session queries now return an absolute Unix bucket key in the existing dimension. It remains the lookup key through sparse result mapping and comparisons; local timestamps are display-only. The per-day response spine matches ClickHouse buckets across 23h/25h days, multi-day ranges and partial-hour transitions. Legacy timeseries, sparklines and CSV exports now use result indices too.

For comparisons, this preserves positional/elapsed-bucket alignment rather than silently switching to wall-clock alignment. Unequal series lengths remain intact, and an extra comparison bucket keeps its metrics with change: nil. The new test pins the two repeated hours separately.

I also tested plain WITH FILL STEP toIntervalHour(1): on the October 6-7, 2024 Lord Howe range, a sparse fill yielded 49 buckets, versus 48 actual toStartOfHour buckets, because the half-hour transition shifts the fill grid into the next day. That is why this revision keeps the spine app-side, with direct ClickHouse oracle tests. It avoids both filled metric rows and an additional timestamp column; I have not measured production payloads.

Validation: 41 focused time/DST tests pass. The broader statistics/main-graph/CSV/external-API run passed 1,346 of 1,347 tests; the remaining journey-exploration failure also reproduces on the original c92fcfd head in the same environment. Formatting and diff checks pass. These runs used ClickHouse 26.7.5.10 rather than the project's 25.11 image. Prepared and validated with AI assistance.

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.

3 participants