From 828c2bcc943d67ce74da0666d536146883e3c317 Mon Sep 17 00:00:00 2001 From: Frank Midigo Date: Thu, 17 Sep 2026 09:08:21 +0300 Subject: [PATCH 1/7] Bucket the runs volume chart on the reader's clock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The grid was laid on the raw epoch, so a "daily" bar was a UTC day — 03:00 to 03:00 in Nairobi, 05:30 in Delhi — and the day a clock changed split across two bars. Buckets are now cut with date_bin on the local wall clock, in the zone the browser sends as an x-timezone header: a boundary lands on local midnight, London's 25-hour October day is one bar, and every axis and tooltip label is read in that same zone rather than in UTC. Bar widths are unchanged — 2-hourly over a day, half-days over a week, daily over a month — but they are now wall-clock hours, so a daily bar is 23 or 25 hours on the day the clocks move. The response carries bucket_hours and timezone alongside the buckets. The chart used to measure its own bar width by subtracting the first two timestamps, which reads 25 on a clock-change day and mislabels the axis; the server cut the grid, so the server names the width and the clock. The timezone joins the cache key, because it moves every boundary and Cachex keys an in-flight fetch by key alone — without it the first reader to load the page would pin their grid on everyone else's for the TTL. It is validated against the tz database before it gets there, since :workflow_stats has no size limit and an arbitrary header value would let one authenticated reader mint unbounded entries. An unknown or missing zone falls back to UTC rather than 400ing: a bad window cannot be drawn, but a bad zone still has a drawable answer. Configures Tzdata as the time zone database, without which every DateTime.shift_zone/2 by name returns :utc_only_time_zone_database. --- CHANGELOG.md | 5 + assets/js/health/WorkflowHealth.tsx | 6 +- assets/js/health/charts/VolumeBars.tsx | 105 ++++++++------ assets/js/health/useHealthQuery.ts | 12 +- assets/test/health/WorkflowHealth.test.tsx | 25 +++- assets/test/health/charts/VolumeBars.test.tsx | 98 ++++++++----- config/config.exs | 5 + lib/lightning/workflows/stats.ex | 130 ++++++++++++------ .../api/workflow_health_controller.ex | 30 +++- test/lightning/workflows/stats_test.exs | 122 +++++++++++++--- .../api/workflow_health_controller_test.exs | 74 +++++++++- 11 files changed, 466 insertions(+), 146 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13dd5f9949..38aaf19d6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,11 @@ and this project adheres to ### Changed +- The runs volume chart on the workflow health page is now bucketed in your own + timezone rather than UTC, so a daily bar covers your day, the week view's bars + split at your noon, and a day the clocks change is still one bar. Bar widths + are unchanged, and the card names the clock they are drawn on. + ### Fixed ## [2.19.0-pre1] - 2026-09-15 diff --git a/assets/js/health/WorkflowHealth.tsx b/assets/js/health/WorkflowHealth.tsx index 6bed9cd67e..37d7a98a29 100644 --- a/assets/js/health/WorkflowHealth.tsx +++ b/assets/js/health/WorkflowHealth.tsx @@ -95,12 +95,14 @@ export const WorkflowHealth = ({ - {({ buckets, window }) => ( + {({ buckets, window, timezone, bucket_hours }) => ( )} diff --git a/assets/js/health/charts/VolumeBars.tsx b/assets/js/health/charts/VolumeBars.tsx index 12eea2aff5..43aa55c947 100644 --- a/assets/js/health/charts/VolumeBars.tsx +++ b/assets/js/health/charts/VolumeBars.tsx @@ -20,8 +20,9 @@ import { CANCELLED, FAILED, SUCCESS } from './OutcomesDonut'; * * Counts runs where the donuts beside it count work orders, so the two differ * on purpose and the card carries no total. Buckets arrive already counted and - * zero-filled from `Stats.runs/2` — one row per bar, one key per run state, - * which is the shape Recharts takes as `data`. + * zero-filled from `Stats.runs/3` — one row per bar, one key per run state, + * which is the shape Recharts takes as `data` — on a grid cut to the reader's + * calendar, so every label here follows `timezone` rather than the browser. */ // A run has no `rejected` state — a work order rejected on arrival never @@ -47,6 +48,10 @@ export interface RunBucket { export interface RunVolume { window: { from: string; to: string }; + /** The clock the server cut the grid on. Every label follows it. */ + timezone: string; + /** Bar width, in wall-clock hours on that clock: 2, 12 or 24. */ + bucket_hours: number; buckets: RunBucket[]; } @@ -60,10 +65,17 @@ const SERIES = [ interface VolumeBarsProps { buckets: RunBucket[]; + timezone: string; + hours: number; emptyMessage: string; } -export const VolumeBars = ({ buckets, emptyMessage }: VolumeBarsProps) => { +export const VolumeBars = ({ + buckets, + timezone, + hours, + emptyMessage, +}: VolumeBarsProps) => { const rows = buckets.map(bucket => ({ at: bucket.at, success: bucket.success, @@ -84,8 +96,6 @@ export const VolumeBars = ({ buckets, emptyMessage }: VolumeBarsProps) => { return

{emptyMessage}

; } - const hours = bucketHours(buckets); - return ( <> {/* The donut's `FRAME` is a fixed box and gains nothing from extra @@ -104,7 +114,7 @@ export const VolumeBars = ({ buckets, emptyMessage }: VolumeBarsProps) => { to thin the axis itself Recharts drops named bars too. */} tickLabel(at as string, hours)} + tickFormatter={at => tickLabel(at as string, hours, timezone)} tickLine={false} axisLine={false} interval={hours >= 12 && hours < 24 ? 0 : 'preserveEnd'} @@ -132,7 +142,7 @@ export const VolumeBars = ({ buckets, emptyMessage }: VolumeBarsProps) => { content={ rangeLabel(at, hours)} + formatLabel={at => rangeLabel(at, hours, timezone)} /> } /> @@ -183,69 +193,76 @@ export const VolumeBars = ({ buckets, emptyMessage }: VolumeBarsProps) => { ); }; -// Read off the data rather than passed in, so nothing naming the bucket width -// can disagree with the bars under it. A window too short to measure reads as -// daily, which only ever costs a coarser label. -const bucketHours = (buckets: RunBucket[]) => { - const [first, second] = buckets; - - return first && second - ? (Date.parse(second.at) - Date.parse(first.at)) / 3_600_000 - : 24; -}; - /** - * The card's meta line, measured off the same buckets the chart draws. + * The card's meta line: the bar width this range draws, and the clock it is + * drawn on. * - * Names the zone as well as the width: the axis and the tooltip are in UTC, so - * a reader whose own day starts hours either side of it can tell which day a - * bar is counting. + * Names the timezone because the grid is cut on the reader's own clock rather + * than UTC, and a bar labelled `Mar 3` is only unambiguous once the reader + * knows whose `Mar 3` it is. */ -export const bucketMeta = (buckets: RunBucket[]) => { - const hours = bucketHours(buckets); - - return `${hours >= 24 ? 'daily' : `${hours}-hour`} buckets · UTC`; -}; +export const bucketMeta = ({ timezone, bucket_hours: hours }: RunVolume) => + `${hours >= 24 ? 'daily' : `${hours}-hour`} buckets · ${timezone}`; const TICK_FILL = '#6b7280'; -// Rendered in UTC because that is where the buckets are: the server lays the -// grid on the raw epoch, so a day starts at UTC midnight — 03:00 in Nairobi, -// 05:30 in Delhi. The tooltip says UTC for the same reason. -const dayLabel = (date: Date) => +// In the timezone the server cut the grid on, not the browser's own: a bar starts +// at a local whole hour there, so a Delhi bar opening at UTC 18:30 labels as +// 00:00 and no label ever needs minutes. +const dayLabel = (date: Date, timezone: string) => date.toLocaleDateString(undefined, { month: 'short', day: 'numeric', - timeZone: 'UTC', + timeZone: timezone, }); -const hourLabel = (date: Date) => `${date.getUTCHours()}:00`; +// `hourCycle` rather than `hour12: false` because it is the explicit way to +// ask for 00-23 and does not depend on the locale's own preference. Every +// boundary is a local whole hour, so no label ever needs minutes. +const localHour = (date: Date, timezone: string) => + Number( + date.toLocaleString('en-GB', { + hour: '2-digit', + hourCycle: 'h23', + timeZone: timezone, + }) + ); + +const hourLabel = (hour: number) => `${String(hour).padStart(2, '0')}:00`; -export const tickLabel = (at: string, hours: number) => { +export const tickLabel = (at: string, hours: number, timezone: string) => { const date = new Date(at); - if (hours >= 24) return dayLabel(date); + if (hours >= 24) return dayLabel(date, timezone); // Two bars to a day at this width, so the date goes on the morning bar, // where the day starts, and the afternoon is left blank. A window opening // mid-day leaves that first bar unlabelled, which is honest — its morning // is outside the window — and keeps every date two bars from the last. - if (hours >= 12) return date.getUTCHours() < 12 ? dayLabel(date) : ''; + if (hours >= 12) { + return localHour(date, timezone) < 12 ? dayLabel(date, timezone) : ''; + } - return hourLabel(date); + return hourLabel(localHour(date, timezone)); }; // The axis is thinned and its labels name only where a bucket starts, so the // tooltip names the whole slot rather than leaving the reader to add the width -// to the start. -export const rangeLabel = (at: string, hours: number) => { +// to the start. No timezone suffix: the card's meta line names it once, and +// repeating it on every hover is noise. +export const rangeLabel = (at: string, hours: number, timezone: string) => { const start = new Date(at); - // A daily bar is a whole UTC day, and `00:00 – 00:00` invites the question - // of whether the closing midnight is inside the bar or the next one. - if (hours >= 24) return `${dayLabel(start)} UTC`; + // A daily bar is a whole local day, and `00:00 – 00:00` invites the question + // of whether the closing midnight is inside the bar or the next one. It is + // also the branch that keeps a 25-hour clock-change day honest, by printing + // no end at all. + if (hours >= 24) return dayLabel(start, timezone); - const end = new Date(start.getTime() + hours * 3_600_000); + // The end hour is the start's plus the width on the wall clock, not + // `start + hours` in real time: a bar spanning a clock change is 11 or 13 + // real hours, and only the wall clock closes it where the next bar opens. + const from = localHour(start, timezone); - return `${dayLabel(start)}, ${hourLabel(start)} – ${hourLabel(end)} UTC`; + return `${dayLabel(start, timezone)}, ${hourLabel(from)} – ${hourLabel((from + hours) % 24)}`; }; diff --git a/assets/js/health/useHealthQuery.ts b/assets/js/health/useHealthQuery.ts index 1709cbc207..c2e25effaf 100644 --- a/assets/js/health/useHealthQuery.ts +++ b/assets/js/health/useHealthQuery.ts @@ -52,7 +52,17 @@ export function useHealthQuery(url: string): Query { setInFlight(true); - fetch(url, { credentials: 'same-origin', signal: controller.signal }) + // Read inside the effect, so a reader whose machine changes timezone + // mid-session picks it up on the next poll. Sent to all three endpoints; + // only `runs` reads it, which is cheaper than threading a per-endpoint + // flag through a hook whose job is to be indifferent to the endpoint. + fetch(url, { + credentials: 'same-origin', + signal: controller.signal, + headers: { + 'x-timezone': Intl.DateTimeFormat().resolvedOptions().timeZone, + }, + }) .then(response => { if (!response.ok) throw new Error('Could not load workflow stats'); diff --git a/assets/test/health/WorkflowHealth.test.tsx b/assets/test/health/WorkflowHealth.test.tsx index dd49065c7a..3bb5a20e05 100644 --- a/assets/test/health/WorkflowHealth.test.tsx +++ b/assets/test/health/WorkflowHealth.test.tsx @@ -34,13 +34,15 @@ const errorSignatures = { ], }; -// Two buckets is the least the chart can measure its own width from. What the -// bars look like is `VolumeBars`'s own test; the page only hands them through. +// Bars as the server cuts them, on the reader's clock. What they look like +// drawn is `VolumeBars`'s own test; the page only hands them through. const runVolume = { window: outcomes.window, + timezone: 'Africa/Nairobi', + bucket_hours: 2, buckets: [ bucket('2026-08-30T00:00:00Z', { success: 40, failed: 3 }), - bucket('2026-08-31T00:00:00Z', { success: 60, failed: 1 }), + bucket('2026-08-30T01:00:00Z', { success: 60, failed: 1 }), ], }; @@ -127,6 +129,23 @@ describe('WorkflowHealth', () => { ); }); + // Nothing server-side records a reader's timezone, so the request has to carry + // it or the chart's buckets go back to being cut on UTC. + test("tells the server the reader's timezone", async () => { + const { fetchMock } = mount(both); + + await screen.findAllByText('Success'); + + expect(fetchMock).toHaveBeenCalledWith( + '/api/projects/proj-1/workflows/wf-1/health/runs?days=30', + expect.objectContaining({ + headers: { + 'x-timezone': Intl.DateTimeFormat().resolvedOptions().timeZone, + }, + }) + ); + }); + test('polls every 30 seconds while the tab is visible, and stops while hidden', async () => { // Mutable so the polled request can answer differently. const responses: Record = { ...both }; diff --git a/assets/test/health/charts/VolumeBars.test.tsx b/assets/test/health/charts/VolumeBars.test.tsx index 5d447d9d0e..2474eefe6f 100644 --- a/assets/test/health/charts/VolumeBars.test.tsx +++ b/assets/test/health/charts/VolumeBars.test.tsx @@ -20,7 +20,7 @@ describe('VolumeBars', () => { failed: 3, crashed: 2, }), - bucket('2026-09-09T00:00:00Z', { + bucket('2026-09-08T01:00:00Z', { success: 60, killed: 1, exception: 1, @@ -28,6 +28,8 @@ describe('VolumeBars', () => { cancelled: 4, }), ]} + timezone="Etc/UTC" + hours={2} emptyMessage="No runs" /> ); @@ -49,8 +51,10 @@ describe('VolumeBars', () => { ); @@ -67,8 +71,10 @@ describe('VolumeBars', () => { ); @@ -78,54 +84,76 @@ describe('VolumeBars', () => { }); }); -// Measured off the gap between the first two buckets, so the meta line follows -// the bars rather than the range the reader picked. +// Read off the payload, since the server cuts the grid and names its width. describe('bucketMeta', () => { - test('names the bucket width the server actually sent', () => { - const hourly = (hours: number) => [ - bucket('2026-09-09T00:00:00Z'), - bucket( - new Date( - Date.parse('2026-09-09T00:00:00Z') + hours * 3_600_000 - ).toISOString() - ), - ]; - - expect(bucketMeta(hourly(2))).toBe('2-hour buckets · UTC'); - expect(bucketMeta(hourly(12))).toBe('12-hour buckets · UTC'); - expect(bucketMeta(hourly(24))).toBe('daily buckets · UTC'); + const volume = (bucket_hours: number) => ({ + window: { from: '2026-09-08T21:00:00Z', to: '2026-09-09T21:00:00Z' }, + timezone: 'Africa/Nairobi', + bucket_hours, + buckets: [], }); - // One bucket has no width to disagree with, so a coarser label is the worst - // this can cost. - test('falls back to daily when there is nothing to measure', () => { - expect(bucketMeta([bucket('2026-09-09T00:00:00Z')])).toBe( - 'daily buckets · UTC' - ); + test('names the bar width and the clock it is cut on', () => { + expect(bucketMeta(volume(2))).toBe('2-hour buckets · Africa/Nairobi'); + expect(bucketMeta(volume(12))).toBe('12-hour buckets · Africa/Nairobi'); + expect(bucketMeta(volume(24))).toBe('daily buckets · Africa/Nairobi'); }); }); -// The axis names where a bucket starts; the tooltip names the whole slot. The -// day part is left to the locale, so only the clock is asserted literally. +// The axis names where a bar starts; the tooltip names the whole slot. The day +// part is left to the locale, so only the clock is asserted literally. describe('tickLabel and rangeLabel', () => { const at = '2026-09-09T14:00:00Z'; - const day = tickLabel(at, 24); + const day = tickLabel(at, 24, 'Etc/UTC'); - test('clocks the narrow buckets and dates the wide ones', () => { - expect(tickLabel(at, 2)).toBe('14:00'); + test('clocks the narrow bars and dates the wide ones', () => { + expect(tickLabel(at, 2, 'Etc/UTC')).toBe('14:00'); expect(day).not.toBe(''); }); // The date names where a day starts, so it goes on the morning bar and the // afternoon is left blank — a date under the afternoon bar would put the // day's start half a day late. - test('dates a half-day bucket on its morning bar only', () => { - expect(tickLabel('2026-09-09T00:00:00Z', 12)).toBe(day); - expect(tickLabel(at, 12)).toBe(''); + test('dates a half-day bar on its morning bar only', () => { + expect(tickLabel('2026-09-09T00:00:00Z', 12, 'Etc/UTC')).toBe(day); + expect(tickLabel(at, 12, 'Etc/UTC')).toBe(''); + }); + + test('gives the tooltip the whole slot, with no timezone suffix', () => { + expect(rangeLabel(at, 24, 'Etc/UTC')).toBe(day); + expect(rangeLabel(at, 2, 'Etc/UTC')).toBe(`${day}, 14:00 – 16:00`); + }); + + // Closed on the wall clock, not on `start + hours`: the last bar of a local + // day ends at midnight, and a bar spanning a clock change is 11 or 13 real + // hours but still 12 on the clock its labels are drawn on. + test('closes the last bar of a day at midnight', () => { + const lastBar = '2026-10-25T12:00:00Z'; + + expect(rangeLabel('2026-09-09T22:00:00Z', 2, 'Etc/UTC')).toMatch( + /22:00 – 00:00$/ + ); + expect(rangeLabel(lastBar, 12, 'Europe/London')).toMatch(/12:00 – 00:00$/); + }); + + // A bar opening at 21:00Z is the start of the next day in Nairobi, and it is + // that date the reader has to see. + test("dates a bar on the reader's calendar, not on UTC", () => { + expect(tickLabel('2026-09-08T21:00:00Z', 24, 'Africa/Nairobi')).toBe( + tickLabel('2026-09-09T00:00:00Z', 24, 'Etc/UTC') + ); + expect(tickLabel('2026-09-08T21:00:00Z', 12, 'Africa/Nairobi')).toBe( + tickLabel('2026-09-09T00:00:00Z', 24, 'Etc/UTC') + ); }); - test('gives the tooltip the whole slot, in UTC', () => { - expect(rangeLabel(at, 24)).toBe(`${day} UTC`); - expect(rangeLabel(at, 2)).toBe(`${day}, 14:00 – 16:00 UTC`); + // The half- and quarter-hour zones. The grid is floored on the local clock, + // so a boundary is a local whole hour and no label ever needs minutes — the + // thing that broke when the hour was read off UTC. + test('clocks the off-the-hour zones as whole local hours', () => { + expect(tickLabel('2026-09-08T18:30:00Z', 2, 'Asia/Kolkata')).toBe('00:00'); + expect(tickLabel('2026-09-08T18:15:00Z', 2, 'Asia/Kathmandu')).toBe( + '00:00' + ); }); }); diff --git a/config/config.exs b/config/config.exs index 6c5f0388ec..f093a38c4e 100644 --- a/config/config.exs +++ b/config/config.exs @@ -173,6 +173,11 @@ config :logger, :console, # Use Jason for JSON parsing in Phoenix config :phoenix, :json_library, Jason +# Without this every `DateTime.shift_zone/2` with a zone name returns +# `{:error, :utc_only_time_zone_database}`. Needed by the workflow health +# charts, which bucket on the reader's clock. +config :elixir, :time_zone_database, Tzdata.TimeZoneDatabase + config :lightning, Lightning.Vault, json_library: Jason config :lightning, Lightning.FailureAlerter, diff --git a/lib/lightning/workflows/stats.ex b/lib/lightning/workflows/stats.ex index 2c96b5f736..218502ca99 100644 --- a/lib/lightning/workflows/stats.ex +++ b/lib/lightning/workflows/stats.ex @@ -92,16 +92,20 @@ defmodule Lightning.Workflows.Stats do end) end - # How the runs chart slices each window: `{bucket_seconds, bucket_count}`. - # Every width divides a day evenly, so a window whose `from` sits on the grid - # keeps every bucket boundary on the clock — 2-hourly, AM/PM, midnight. + # How the runs chart slices each window: `{bucket_hours, bucket_count}`. + # Widths are wall-clock hours on the reader's calendar, not fixed durations — + # a "daily" bar is a local day, which is 23 or 25 hours across a clock change. # # One bucket more than the width divides into: `now` sits mid-bucket, so a - # grid of exactly `days_back / width` bars would start *after* + # grid of exactly `days_back * 24 / width` bars would start *after* # `now - days_back` and leave the oldest hours of the window undrawn while # the donuts beside it counted them. The oldest bar instead reaches back # past `from`, and `window` reports the range actually covered. - @buckets %{1 => {7_200, 13}, 7 => {43_200, 15}, 30 => {86_400, 31}} + # + # ponytail: the counts are wall-clock, so on a spring-forward day the grid + # covers an hour less real time than the donuts beside it. Add a bucket to + # each width if anyone ever notices. + @buckets %{1 => {2, 13}, 7 => {12, 15}, 30 => {24, 31}} @zero_run_counts Map.new(Run.final_states(), &{&1, 0}) @@ -112,69 +116,115 @@ defmodule Lightning.Workflows.Stats do } @doc """ - Final run counts per state, bucketed across the last `days_back` days: - 2-hourly over a day, AM/PM over a week, daily over a month. + Final run counts per state, bucketed across the last `days_back` days on a + grid cut to `timezone`'s clock: local 2-hourly over a day, local half-days + over a week, local days over a month. Bucketed here rather than in the browser, because the alternative is shipping every run in the window — six figures of rows on a busy workflow, cached whole and JSON-encoded — to draw thirty bars. + Bucketed on the local calendar rather than on the epoch, which is what makes + it daylight-saving-correct: `date_bin` bins the *local* timestamp, so + London's 25-hour October day is one bar 25 hours wide and every boundary + stays on local midnight. + Buckets are counted on `inserted_at` — when the attempt started, not when it settled — so a run stays in the bar the traffic arrived in. Every bucket and every state is present, zero-filled: the chart draws a flat window without reasoning about which bars are missing. The last bucket is the one `now` falls in, so it is still filling; the first - reaches back past `now - days_back`, so nothing in the window goes undrawn. - `window` is the range the bars actually cover. + reaches back past `now - days_back`, so nothing in the window goes undrawn — + except on a spring-forward day in the reader's zone, where the grid is a + wall-clock hour wide but an hour short in real time. + `window` is the range the bars actually cover, `bucket_hours` is the width + the chart labels, and `timezone` is the clock both were cut on. A bucket is `at` alongside one key per state, flat rather than nested, which is the row shape Recharts takes as `data` — the list goes to the chart untouched, and each `Bar` names the state it draws. """ - @spec runs(Workflow.t(), 1 | 7 | 30) :: %{ + @spec runs(Workflow.t(), 1 | 7 | 30, Calendar.time_zone()) :: %{ window: %{from: DateTime.t(), to: DateTime.t()}, + timezone: Calendar.time_zone(), + bucket_hours: pos_integer(), buckets: [run_bucket()] } - def runs(%Workflow{id: workflow_id}, days_back \\ @default_days_back) + def runs( + %Workflow{id: workflow_id}, + days_back \\ @default_days_back, + timezone \\ "Etc/UTC" + ) when is_map_key(@buckets, days_back) do # No change marker in this key — a busy workflow moves the marker faster # than the page polls, so every poll would miss (211 ms and 1.4 GiB of # buffer traffic per recompute at 44k work orders). Costs up to # `@runs_ttl` of staleness on a chart whose finest bar is two hours wide. + # + # The timezone *is* in the key: it moves every boundary, and Cachex keys a + # fetch in flight by key alone (see `cached_until_ttl/3` below), so without + # it the first reader to load the page would pin their grid on every other + # timezone for the TTL. cached_until_ttl( - {:runs, workflow_id, days_back}, + {:runs, workflow_id, days_back, timezone}, fn -> - {seconds, count} = Map.fetch!(@buckets, days_back) - window = bucket_window(seconds, count) + {hours, count} = Map.fetch!(@buckets, days_back) + to = DateTime.utc_now() + starts = bucket_starts(to, hours, count, timezone) + from = resolve(hd(starts), timezone) %{ - window: window, - buckets: bucket_runs(workflow_id, window.from, seconds, count) + window: %{from: from, to: to}, + timezone: timezone, + bucket_hours: hours, + buckets: bucket_runs(workflow_id, starts, from, hours, timezone) } end, @runs_ttl ) end + # The local wall-clock start of every bar, oldest first, as naive local + # datetimes — the same domain `date_bin` bins in below, so a tally matches a + # bar by equality and no timezone arithmetic is needed to line them up. + # # Anchored on the bucket `now` is in, not on `now` itself: a window that ends # mid-bucket would put every boundary at whatever minute the request landed - # on, and the labels the chart draws — "2am", "PM", a date — would be lies. - defp bucket_window(seconds, count) do - to = DateTime.utc_now() - current = DateTime.from_unix!(div(DateTime.to_unix(to), seconds) * seconds) + # on, and the labels the chart draws — an hour, a date — would be lies. + defp bucket_starts(to, hours, count, timezone) do + local = to |> DateTime.shift_zone!(timezone) |> DateTime.to_naive() - %{from: DateTime.add(current, -(count - 1) * seconds, :second), to: to} + current = + local + |> NaiveDateTime.beginning_of_day() + |> NaiveDateTime.add(div(local.hour, hours) * hours, :hour) + |> NaiveDateTime.truncate(:second) + + Enum.map((count - 1)..0//-1, &NaiveDateTime.add(current, -&1 * hours, :hour)) + end + + # A local wall-clock time back to a real instant, reported in UTC like every + # other timestamp on this page. A spring-forward gap has no such instant — + # take the one the clock jumps to; an autumn repeat has two — take the first, + # so the bar opens when its label says it does. + defp resolve(naive, timezone) do + case DateTime.from_naive(naive, timezone) do + {:ok, datetime} -> datetime + {:ambiguous, first, _second} -> first + {:gap, _before, after_gap} -> after_gap + end + |> DateTime.shift_zone!("Etc/UTC") end - defp bucket_runs(workflow_id, from, seconds, count) do - tallies = tally_runs(workflow_id, from, seconds) + defp bucket_runs(workflow_id, starts, from, hours, timezone) do + tallies = tally_runs(workflow_id, from, hours, timezone) - Enum.map(0..(count - 1), fn index -> + Enum.map(starts, fn start -> tallies - |> Map.get(index, []) + |> Map.get(start, []) |> Enum.into(@zero_run_counts) - |> Map.put(:at, DateTime.add(from, index * seconds, :second)) + |> Map.put(:at, resolve(start, timezone)) end) end @@ -185,11 +235,12 @@ defmodule Lightning.Workflows.Stats do # the window before the nested loop into `runs(work_order_id, inserted_at)`, # instead of probing every work order the workflow ever had. # - # The grid is aligned, so integer division by the bucket width is the whole - # of the bucketing — no `date_trunc` special case per width. `floor` before - # the cast because `extract` yields `numeric` and `numeric::bigint` rounds: - # without it a run at 01:59:59.7 is counted in the 02:00 bar. - defp tally_runs(workflow_id, from, seconds) do + # `inserted_at` is a naked `timestamp` holding UTC, so it is labelled UTC and + # then read on the reader's clock before binning: `date_bin` bins the local + # wall clock, which is what puts a boundary on local midnight and makes a + # 25-hour day one bar. The origin is a midnight, so every width — 2, 12, 24 — + # aligns to one. + defp tally_runs(workflow_id, from, hours, timezone) do from(r in Run, join: wo in WorkOrder, on: wo.id == r.work_order_id, @@ -200,10 +251,10 @@ defmodule Lightning.Workflows.Stats do select: { selected_as( fragment( - "div(floor(extract(epoch from ? - ?))::bigint, ?)::int", + "date_bin(make_interval(hours => ?::int), ? at time zone 'UTC' at time zone ?, timestamp '2000-01-01')", + ^hours, r.inserted_at, - type(^from, :utc_datetime_usec), - type(^seconds, :integer) + ^timezone ), :bucket ), @@ -212,9 +263,10 @@ defmodule Lightning.Workflows.Stats do } ) |> Repo.all() - |> Enum.group_by(fn {bucket, _, _} -> bucket end, fn {_, state, count} -> - {state, count} - end) + |> Enum.group_by( + fn {bucket, _, _} -> NaiveDateTime.truncate(bucket, :second) end, + fn {_, state, count} -> {state, count} end + ) end defp window(days_back) do @@ -229,7 +281,7 @@ defmodule Lightning.Workflows.Stats do # something settles: a poll that finds nothing has moved is a hit on every # pod, because the answer is read from Postgres and not from one node's ETS. # The trade is that a workflow settling work orders faster than the poll - # recomputes on every poll, which is why `runs/2` opts out and takes + # recomputes on every poll, which is why `runs/3` opts out and takes # `@ttl`-bounded staleness instead. # # The marker only moves when a work order settles, so a stat counting unsettled @@ -239,7 +291,7 @@ defmodule Lightning.Workflows.Stats do # flight by key alone and ignores the fallback closure # (`deps/cachex/lib/cachex/services/courier.ex:60-62`), so a second caller # with a different closure for the same key never runs its own and is handed - # the first one's answer. `outcomes/2`, `error_signatures/2` and `runs/2` are + # the first one's answer. `outcomes/2`, `error_signatures/2` and `runs/3` are # safe because their key prefixes differ. defp cached({_slice, workflow_id, _days} = key, fun) do key diff --git a/lib/lightning_web/controllers/api/workflow_health_controller.ex b/lib/lightning_web/controllers/api/workflow_health_controller.ex index 9b353a42c5..3a61060acb 100644 --- a/lib/lightning_web/controllers/api/workflow_health_controller.ex +++ b/lib/lightning_web/controllers/api/workflow_health_controller.ex @@ -19,6 +19,8 @@ defmodule LightningWeb.API.WorkflowHealthController do plug :authorize_workflow # After :authorize_workflow so a 404 wins over a 400. plug :validate_days + # Auth, then params, then presentation. + plug :validate_timezone when action in [:runs] def outcomes(conn, _params) do json( @@ -40,13 +42,18 @@ defmodule LightningWeb.API.WorkflowHealthController do def runs(conn, _params) do json( conn, - Workflows.Stats.runs(conn.assigns.workflow, conn.assigns.days_back) + Workflows.Stats.runs( + conn.assigns.workflow, + conn.assigns.days_back, + conn.assigns.timezone + ) ) end # Closed set, string-matched — no free integer, no parse to defend. @days %{"1" => 1, "7" => 7, "30" => 30} @default_days "30" + @default_timezone "Etc/UTC" defp validate_days(conn, _opts) do case Map.fetch(@days, conn.params["days"] || @default_days) do @@ -61,6 +68,27 @@ defmodule LightningWeb.API.WorkflowHealthController do end end + # The reader's timezone, because nothing in Lightning records one. Unknown + # or absent falls back to UTC rather than 400ing: a bad `days` means a window + # we cannot draw, but a bad timezone still has a drawable answer, and an old + # cached bundle that sends no header has to keep working through a deploy. + # + # Validated before it reaches the cache key. `:workflow_stats` has no size + # limit (`application.ex:62-65`), so an arbitrary string would let one + # authenticated reader mint unbounded entries; `Tzdata.zone_exists?/1` bounds + # the key to the tz database. + defp validate_timezone(conn, _opts) do + timezone = + with [timezone] <- get_req_header(conn, "x-timezone"), + true <- Tzdata.zone_exists?(timezone) do + timezone + else + _ -> @default_timezone + end + + assign(conn, :timezone, timezone) + end + defp authorize_workflow(conn, _opts) do %{"project_id" => project_id, "workflow_id" => workflow_id} = conn.params diff --git a/test/lightning/workflows/stats_test.exs b/test/lightning/workflows/stats_test.exs index 065da9c927..11d8e149fe 100644 --- a/test/lightning/workflows/stats_test.exs +++ b/test/lightning/workflows/stats_test.exs @@ -685,7 +685,7 @@ defmodule Lightning.Workflows.StatsTest do end end - describe "runs/2" do + describe "runs/3" do # A run whose `inserted_at` we choose, on a work order active at the same # moment — the shape the window needs and `insert_run/4` can't give. defp run_at(workflow, trigger, state, at) do @@ -716,8 +716,8 @@ defmodule Lightning.Workflows.StatsTest do older = DateTime.add(DateTime.utc_now(), -5, :hour) newer = DateTime.add(DateTime.utc_now(), -90, :minute) - # A hair inside a bucket, not in the next one: `extract(epoch ...)` is - # `numeric` and casting it rounds, so the query has to floor first. + # A hair inside a bucket, not in the next one: `date_bin` floors, and this + # is the run that would move if it ever stopped. edge = DateTime.utc_now() |> DateTime.to_unix() @@ -743,40 +743,123 @@ defmodule Lightning.Workflows.StatsTest do assert %{crashed: 1} = Enum.at(buckets, bucket_of(result, edge)) end - # Boundaries on the clock, so the chart can label a bar "2am" or "Tuesday" - # and be telling the truth. And a bar chart with holes in it is a different - # chart, so every bucket carries every final state, zero-filled. + # Boundaries on the reader's clock, so the chart can label a bar with an + # hour or a date and be telling the truth. And a bar chart with holes in it + # is a different chart, so every bucket carries every final state, + # zero-filled. + # + # Kolkata and Kathmandu are the zones that prove the grid really moved: + # their local whole hour is UTC :30 and :15. Europe/London is here because + # a 30 day window may straddle a clock change, and every boundary has to + # stay on a local midnight when it does. test "cuts each window into clock-aligned, zero-filled buckets", ctx do %{workflow: workflow} = ctx zeroed = Map.new(Run.final_states(), &{&1, 0}) - for {days, seconds, count} <- [ - {1, 7_200, 13}, - {7, 43_200, 15}, - {30, 86_400, 31} - ] do - assert %{buckets: buckets, window: window} = Stats.runs(workflow, days) + for timezone <- [ + "Etc/UTC", + "Africa/Nairobi", + "Asia/Kolkata", + "Asia/Kathmandu", + "Europe/London" + ], + {days, hours, count} <- [{1, 2, 13}, {7, 12, 15}, {30, 24, 31}] do + assert %{buckets: buckets, window: window} = + Stats.runs(workflow, days, timezone) assert length(buckets) == count - assert rem(DateTime.to_unix(window.from), seconds) == 0 + assert DateTime.compare(window.from, hd(buckets).at) == :eq + + # Every bar opens on a local whole hour that the width divides — local + # midnight for a daily bar, local noon or midnight for a half-day. + for bucket <- buckets do + local = DateTime.shift_zone!(bucket.at, timezone) + + assert local.minute == 0 and local.second == 0 + assert rem(local.hour, hours) == 0 + end + # An hour's grace: the grid is `count` wall-clock steps wide, and a + # spring-forward day makes that an hour less in real time. assert DateTime.compare( window.from, - DateTime.add(window.to, -days, :day) + window.to |> DateTime.add(-days, :day) |> DateTime.add(1, :hour) ) != :gt - assert DateTime.diff(Enum.at(buckets, 1).at, hd(buckets).at) == seconds - for bucket <- buckets, do: assert(Map.delete(bucket, :at) == zeroed) # The window ends inside the last bucket, which is still filling. last = List.last(buckets).at assert DateTime.compare(last, window.to) == :lt - assert DateTime.diff(window.to, last, :second) < seconds + assert DateTime.diff(window.to, last, :second) < hours * 3_600 + end + end + + # The half- and quarter-hour timezones. A whole-hour one cannot prove the + # grid moved, because at these widths its boundaries are the same list of + # instants as UTC's — the offset cancels out of the arithmetic. These two + # can: a local whole hour is not a UTC whole hour. + test "anchors the grid off the hour for offsets that are", ctx do + %{workflow: workflow} = ctx + + for {timezone, minute} <- [{"Asia/Kolkata", 30}, {"Asia/Kathmandu", 15}] do + assert %{window: window} = Stats.runs(workflow, 1, timezone) + + assert window.from.minute == minute + assert window.from.second == 0 end end + # The claim the local-calendar grid rests on, and the one a UTC grid gets + # wrong: London's October Sunday is 25 hours long, and both sides of the + # change belong to the same bar. Asserted against the binning expression + # itself, because the window is always the last 30 days and no fixed + # transition stays inside it. + test "bins a 25-hour local day as a single bucket" do + %{rows: rows} = + Repo.query!(""" + SELECT date_bin( + make_interval(hours => 24), + t at time zone 'UTC' at time zone 'Europe/London', + timestamp '2000-01-01' + ) + FROM (VALUES (timestamp '2025-10-26 00:30'), + (timestamp '2025-10-26 23:30')) AS v(t) + """) + + assert rows == List.duplicate([~N[2025-10-26 00:00:00.000000]], 2) + end + + # Without the timezone in the key, the first reader to load the page would + # pin their grid on every other timezone for the TTL — and on one machine + # that is invisible. + test "caches each timezone's grid separately", ctx do + %{workflow: workflow} = ctx + + utc = Stats.runs(workflow, 1, "Etc/UTC") + kolkata = Stats.runs(workflow, 1, "Asia/Kolkata") + + assert utc.window.from != kolkata.window.from + assert utc.timezone == "Etc/UTC" + assert kolkata.timezone == "Asia/Kolkata" + + for timezone <- ["Etc/UTC", "Asia/Kolkata"] do + assert {:ok, true} = + Cachex.exists?( + :workflow_stats, + {:runs, workflow.id, 1, timezone} + ) + end + end + + test "defaults to a UTC grid when no timezone is given", ctx do + %{workflow: workflow} = ctx + + assert %{window: window, timezone: "Etc/UTC"} = Stats.runs(workflow, 1) + assert rem(DateTime.to_unix(window.from), 7_200) == 0 + end + # Keyed without the change marker — a settle must not mint a new key. test "serves the same answer after a work order settles", ctx do %{workflow: workflow, trigger: trigger} = ctx @@ -789,7 +872,10 @@ defmodule Lightning.Workflows.StatsTest do assert Stats.runs(workflow, 1) == first assert {:ok, true} = - Cachex.exists?(:workflow_stats, {:runs, workflow.id, 1}) + Cachex.exists?( + :workflow_stats, + {:runs, workflow.id, 1, "Etc/UTC"} + ) end test "skips runs outside the window, in flight, or on another workflow", diff --git a/test/lightning_web/controllers/api/workflow_health_controller_test.exs b/test/lightning_web/controllers/api/workflow_health_controller_test.exs index 9a5f9ba167..8ffb997f69 100644 --- a/test/lightning_web/controllers/api/workflow_health_controller_test.exs +++ b/test/lightning_web/controllers/api/workflow_health_controller_test.exs @@ -33,9 +33,17 @@ defmodule LightningWeb.API.WorkflowHealthControllerTest do ) end - defp get_runs(conn, user, project_id, workflow_id, params \\ %{}) do - conn - |> log_in_user(user) + defp get_runs( + conn, + user, + project_id, + workflow_id, + params \\ %{}, + headers \\ [] + ) do + Enum.reduce(headers, log_in_user(conn, user), fn {name, value}, conn -> + put_req_header(conn, name, value) + end) |> get( ~p"/api/projects/#{project_id}/workflows/#{workflow_id}/health/runs", params @@ -395,6 +403,66 @@ defmodule LightningWeb.API.WorkflowHealthControllerTest do end end + # Nothing in Lightning records a reader's timezone, so the browser says. A bad + # value still has a drawable answer, so it falls back to UTC rather than + # 400ing the way a bad `days` does. + describe "x-timezone" do + test "cuts the grid on the timezone the reader sent", %{ + conn: conn, + user: user, + project: project, + workflow: workflow + } do + response = + conn + |> get_runs(user, project.id, workflow.id, %{"days" => "1"}, [ + {"x-timezone", "Africa/Nairobi"} + ]) + |> json_response(200) + + assert response["timezone"] == "Africa/Nairobi" + + {:ok, from, 0} = DateTime.from_iso8601(response["window"]["from"]) + local = DateTime.shift_zone!(from, "Africa/Nairobi") + + assert local.minute == 0 and local.second == 0 + end + + test "falls back to UTC without the header", %{ + conn: conn, + user: user, + project: project, + workflow: workflow + } do + response = + conn + |> get_runs(user, project.id, workflow.id, %{"days" => "1"}) + |> json_response(200) + + assert response["timezone"] == "Etc/UTC" + end + + # Unknown, empty, and long — the cache key has to stay bounded to the tz + # database whatever arrives. + test "falls back to UTC on a value it cannot use", %{ + conn: conn, + user: user, + project: project, + workflow: workflow + } do + for value <- ["Mars/Olympus", "", String.duplicate("x", 5_000)] do + response = + conn + |> get_runs(user, project.id, workflow.id, %{"days" => "1"}, [ + {"x-timezone", value} + ]) + |> json_response(200) + + assert response["timezone"] == "Etc/UTC" + end + end + end + describe "?days=" do @accepted_days [1, 7, 30] From 7e3e2dcda46a14f7a3262093fb47ccf250d64a85 Mon Sep 17 00:00:00 2001 From: Frank Midigo Date: Thu, 17 Sep 2026 17:20:12 +0300 Subject: [PATCH 2/7] Fall back to UTC when Postgres rejects the reader's timezone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tzdata and pg_timezone_names are versioned apart: :tzdata autoupdates at runtime, while Postgres' zone set is fixed at the server's build. So a zone the controller validated can still be one the binning query has never heard of — America/Coyhaique against PG 15 is exactly that, and it is what Chrome reports for readers in Coyhaique, Chile. Rescue invalid_parameter_value and redraw the window on UTC rather than 500 the endpoint. This matches the policy the controller already sets for unusable timezones: a bad window cannot be drawn, a bad clock still can. The result caches under the requested zone, so the failing query runs once per TTL rather than once per poll. --- lib/lightning/workflows/stats.ex | 50 ++++++++++++++----- .../api/workflow_health_controller.ex | 2 + 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/lib/lightning/workflows/stats.ex b/lib/lightning/workflows/stats.ex index 218502ca99..4b41d28c96 100644 --- a/lib/lightning/workflows/stats.ex +++ b/lib/lightning/workflows/stats.ex @@ -168,23 +168,47 @@ defmodule Lightning.Workflows.Stats do # timezone for the TTL. cached_until_ttl( {:runs, workflow_id, days_back, timezone}, - fn -> - {hours, count} = Map.fetch!(@buckets, days_back) - to = DateTime.utc_now() - starts = bucket_starts(to, hours, count, timezone) - from = resolve(hd(starts), timezone) - - %{ - window: %{from: from, to: to}, - timezone: timezone, - bucket_hours: hours, - buckets: bucket_runs(workflow_id, starts, from, hours, timezone) - } - end, + fn -> bucket_window(workflow_id, days_back, timezone) end, @runs_ttl ) end + # Postgres, not Tzdata, is the authority on what `at time zone` accepts, and + # the two zone sets are versioned apart: `:tzdata` autoupdates at runtime + # everywhere but test, while `pg_timezone_names` is fixed at the server's + # build. So a zone the controller validated can still be one this server has + # never heard of — `America/Coyhaique` is exactly that against PG 15, and it + # is what Chrome reports in Coyhaique, Chile. + # + # Redrawn on UTC rather than left to raise, which keeps the policy the + # controller already sets for every other unusable timezone: a bad window + # cannot be drawn, a bad clock still can. The result is cached under the + # requested zone, so the dead query runs once per `@runs_ttl`, not per poll. + # + # Not checked up front: `pg_timezone_names` costs ~90 ms to scan, more than + # the query it would guard. + defp bucket_window(workflow_id, days_back, timezone) do + {hours, count} = Map.fetch!(@buckets, days_back) + to = DateTime.utc_now() + starts = bucket_starts(to, hours, count, timezone) + from = resolve(hd(starts), timezone) + + %{ + window: %{from: from, to: to}, + timezone: timezone, + bucket_hours: hours, + buckets: bucket_runs(workflow_id, starts, from, hours, timezone) + } + rescue + error in Postgrex.Error -> + if error.postgres[:code] == :invalid_parameter_value and + timezone != "Etc/UTC" do + bucket_window(workflow_id, days_back, "Etc/UTC") + else + reraise error, __STACKTRACE__ + end + end + # The local wall-clock start of every bar, oldest first, as naive local # datetimes — the same domain `date_bin` bins in below, so a tally matches a # bar by equality and no timezone arithmetic is needed to line them up. diff --git a/lib/lightning_web/controllers/api/workflow_health_controller.ex b/lib/lightning_web/controllers/api/workflow_health_controller.ex index 3a61060acb..59be25d922 100644 --- a/lib/lightning_web/controllers/api/workflow_health_controller.ex +++ b/lib/lightning_web/controllers/api/workflow_health_controller.ex @@ -77,6 +77,8 @@ defmodule LightningWeb.API.WorkflowHealthController do # limit (`application.ex:62-65`), so an arbitrary string would let one # authenticated reader mint unbounded entries; `Tzdata.zone_exists?/1` bounds # the key to the tz database. + # Postgres, which does the binning, knows a narrower set — `Stats.runs/3` + # falls back to UTC for the difference. defp validate_timezone(conn, _opts) do timezone = with [timezone] <- get_req_header(conn, "x-timezone"), From 86b36c91c4ac9b1c5bce8fe6e2545f0803ddcc7b Mon Sep 17 00:00:00 2001 From: Frank Midigo Date: Thu, 17 Sep 2026 20:57:12 +0300 Subject: [PATCH 3/7] Click through the runs volume chart to the history page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each band of each bar links to the work orders behind it: the bar's slot, and the states that band stacked. The red band folds five run states, so it carries all five — a column-wide link would land the reader on the answer they came for mixed back in with the successes. The history page had no run-level filter to link to. run_date_after, run_date_before and run_status bound a run's inserted_at and its state, matched with an EXISTS over the work order's runs. The existing status filter is the work order's own state, which is its latest run: a failure since retried to success is a success there, and the band that counted the failure still has to reach it. run_status takes final states only, since an unsettled run has no outcome to have been counted. A bar's slot ends at the next bar's start rather than its own plus a width. The grid is cut on the reader's clock, so a bar spanning a clock change is 11 or 13 real hours wide, and a run landing on a boundary belongs to the same bar date_bin counted it in. The newest bar goes over open-ended — it is still filling. historyUrl now repeats key[] for a list value, which is what Plug decodes back into something an {:array, _} field can cast. The chips read the range in UTC and say so. Nothing in Lightning records what clock the bars were cut on, and a bare "14:00" under a bar labelled "16:00" is worse than a suffix. --- assets/js/health/WorkflowHealth.tsx | 2 + assets/js/health/charts/VolumeBars.tsx | 91 ++++++++++-- assets/js/health/historyUrl.ts | 12 +- assets/test/health/charts/VolumeBars.test.tsx | 62 ++++++++ lib/lightning/invocation.ex | 37 +++++ lib/lightning/workorders/search_params.ex | 18 +++ lib/lightning_web/live/run_live/index.ex | 18 +++ .../live/run_live/index.html.heex | 38 +++++ test/lightning/invocation_test.exs | 134 ++++++++++++++++++ .../live/run_live/index_test.exs | 79 +++++++++++ 10 files changed, 473 insertions(+), 18 deletions(-) diff --git a/assets/js/health/WorkflowHealth.tsx b/assets/js/health/WorkflowHealth.tsx index 37d7a98a29..2fa9f7d1cb 100644 --- a/assets/js/health/WorkflowHealth.tsx +++ b/assets/js/health/WorkflowHealth.tsx @@ -104,6 +104,8 @@ export const WorkflowHealth = ({ timezone={timezone} hours={bucket_hours} emptyMessage={emptyMessage(window, 'runs')} + projectId={projectId} + workflowId={workflowId} /> )} diff --git a/assets/js/health/charts/VolumeBars.tsx b/assets/js/health/charts/VolumeBars.tsx index 43aa55c947..c3179f83bf 100644 --- a/assets/js/health/charts/VolumeBars.tsx +++ b/assets/js/health/charts/VolumeBars.tsx @@ -8,6 +8,7 @@ import { YAxis, } from 'recharts'; +import { historyUrl } from '../historyUrl'; import type { FailureState } from '../types'; import { FAILURE_STATES } from '../types'; @@ -30,7 +31,7 @@ import { CANCELLED, FAILED, SUCCESS } from './OutcomesDonut'; // both. type RunFailureState = Exclude; -const RUN_FAILURE_STATES = FAILURE_STATES.filter( +export const RUN_FAILURE_STATES = FAILURE_STATES.filter( (state): state is RunFailureState => state !== 'rejected' ); @@ -57,10 +58,19 @@ export interface RunVolume { // Top to bottom, as the legend and the spoken totals read. The bars draw from // the reverse of it, since Recharts puts the first `Bar` on the axis. +// +// `states` is what the band actually stacked, and is what its history link +// filters on — the red one folds five, and it is the only place those five can +// be named. const SERIES = [ - { key: 'success', label: 'Success', color: SUCCESS }, - { key: 'cancelled', label: 'Cancelled', color: CANCELLED }, - { key: 'failed', label: 'Failed', color: FAILED }, + { key: 'success', label: 'Success', color: SUCCESS, states: ['success'] }, + { + key: 'cancelled', + label: 'Cancelled', + color: CANCELLED, + states: ['cancelled'], + }, + { key: 'failed', label: 'Failed', color: FAILED, states: RUN_FAILURE_STATES }, ] as const; interface VolumeBarsProps { @@ -68,6 +78,8 @@ interface VolumeBarsProps { timezone: string; hours: number; emptyMessage: string; + projectId: string; + workflowId: string; } export const VolumeBars = ({ @@ -75,6 +87,8 @@ export const VolumeBars = ({ timezone, hours, emptyMessage, + projectId, + workflowId, }: VolumeBarsProps) => { const rows = buckets.map(bucket => ({ at: bucket.at, @@ -147,17 +161,37 @@ export const VolumeBars = ({ } /> {/* Reversed, so failures land on the axis and can be read against - a fixed baseline day to day rather than judged by thickness. */} - {[...totals].reverse().map(({ key, label: name, color }) => ( - - ))} + a fixed baseline day to day rather than judged by thickness. + + Each band links to its own states rather than the chart + carrying one link for the whole column: the red band is the + answer most readers came for, and a column-wide link would + make them filter the failures out again on arrival. Clicking + is a mouse affordance only — the frame is `aria-hidden`, and + thirty bars times three bands is not a link list. */} + {[...totals] + .reverse() + .map(({ key, label: name, color, states }) => ( + { + const href = bucketUrl( + projectId, + workflowId, + buckets, + index, + states + ); + if (href) window.open(href, '_blank'); + }} + /> + ))} @@ -204,6 +238,33 @@ export const VolumeBars = ({ export const bucketMeta = ({ timezone, bucket_hours: hours }: RunVolume) => `${hours >= 24 ? 'daily' : `${hours}-hour`} buckets · ${timezone}`; +/** + * The history link for one band of one bar: the work orders with a run + * created inside the bar's slot that settled in one of the band's states. + * + * Half-open, and the slot's end is the *next* bar's start rather than its own + * plus a width: no timezone arithmetic happens here, and a bar spanning a + * clock change is 11 or 13 real hours wide. The newest bar has no upper bound + * — it is still filling. + */ +export const bucketUrl = ( + projectId: string, + workflowId: string, + buckets: RunBucket[], + index: number, + states: readonly string[] +) => { + const bucket = buckets[index]; + + if (!bucket) return null; + + return historyUrl(projectId, workflowId, { + run_date_after: bucket.at, + run_date_before: buckets[index + 1]?.at, + run_status: states, + }); +}; + const TICK_FILL = '#6b7280'; // In the timezone the server cut the grid on, not the browser's own: a bar starts diff --git a/assets/js/health/historyUrl.ts b/assets/js/health/historyUrl.ts index 47fce45e83..8f9fd589b0 100644 --- a/assets/js/health/historyUrl.ts +++ b/assets/js/health/historyUrl.ts @@ -13,7 +13,7 @@ import type { WorkOrderStateCounts } from './types'; export const historyUrl = ( projectId: string, workflowId: string, - filters: Record + filters: Record ) => { const params = new URLSearchParams({ 'filters[workflow_id]': workflowId, @@ -21,9 +21,15 @@ export const historyUrl = ( }); // Absent parts of a filter are skipped, so a caller can hand over an - // optional field without guarding it. + // optional field without guarding it. A list goes over as `key[]` repeated, + // which is what Plug decodes back into a list for a `{:array, _}` field — + // one joined string would fail to cast. for (const [key, value] of Object.entries(filters)) { - if (value) params.set(`filters[${key}]`, value); + if (Array.isArray(value)) { + for (const item of value) params.append(`filters[${key}][]`, item); + } else if (typeof value === 'string' && value) { + params.set(`filters[${key}]`, value); + } } return `/projects/${projectId}/history?${params.toString()}`; diff --git a/assets/test/health/charts/VolumeBars.test.tsx b/assets/test/health/charts/VolumeBars.test.tsx index 2474eefe6f..9da6f0cffe 100644 --- a/assets/test/health/charts/VolumeBars.test.tsx +++ b/assets/test/health/charts/VolumeBars.test.tsx @@ -3,6 +3,8 @@ import { describe, expect, test } from 'vitest'; import { bucketMeta, + bucketUrl, + RUN_FAILURE_STATES, rangeLabel, tickLabel, VolumeBars, @@ -10,6 +12,8 @@ import { import { bucket } from './counts'; +const links = { projectId: 'proj-1', workflowId: 'wf-1' }; + describe('VolumeBars', () => { test('folds every run failure state into one Failed total', () => { render( @@ -31,6 +35,7 @@ describe('VolumeBars', () => { timezone="Etc/UTC" hours={2} emptyMessage="No runs" + {...links} /> ); @@ -56,6 +61,7 @@ describe('VolumeBars', () => { timezone="Etc/UTC" hours={2} emptyMessage="No runs" + {...links} /> ); @@ -76,6 +82,7 @@ describe('VolumeBars', () => { timezone="Etc/UTC" hours={2} emptyMessage="No runs in the last 30 days" + {...links} /> ); @@ -157,3 +164,58 @@ describe('tickLabel and rangeLabel', () => { ); }); }); + +// The bar's own link. Its end is the next bar's start, so the boundary a run +// lands on belongs to exactly one bar — the same half-open slot `date_bin` +// counted it in. +describe('bucketUrl', () => { + const buckets = [ + bucket('2026-09-08T00:00:00Z'), + bucket('2026-09-08T02:00:00Z'), + bucket('2026-09-08T04:00:00Z'), + ]; + + const params = (index: number, states: readonly string[] = ['success']) => { + const href = bucketUrl( + links.projectId, + links.workflowId, + buckets, + index, + states + ); + + return href ? new URLSearchParams(href.split('?')[1]) : null; + }; + + test('closes a bar on the next one’s start', () => { + expect(params(1)?.get('filters[run_date_after]')).toBe( + '2026-09-08T02:00:00Z' + ); + expect(params(1)?.get('filters[run_date_before]')).toBe( + '2026-09-08T04:00:00Z' + ); + }); + + // Still filling, and an upper bound would be whenever the response was + // computed rather than now. + test('leaves the newest bar open-ended', () => { + expect(params(2)?.get('filters[run_date_after]')).toBe( + '2026-09-08T04:00:00Z' + ); + expect(params(2)?.has('filters[run_date_before]')).toBe(false); + }); + + test('scopes the link to the workflow', () => { + expect(params(0)?.get('filters[workflow_id]')).toBe('wf-1'); + }); + + // The band, not the bar. `run_status` and not `status`, so a failure since + // retried to success still shows up under the band that counted it. + test('filters on the band’s own run states', () => { + expect( + params(1, RUN_FAILURE_STATES)?.getAll('filters[run_status][]') + ).toEqual([...RUN_FAILURE_STATES]); + expect(params(1)?.getAll('filters[run_status][]')).toEqual(['success']); + expect(params(1)?.has('filters[failed]')).toBe(false); + }); +}); diff --git a/lib/lightning/invocation.ex b/lib/lightning/invocation.ex index 0c921fda43..97a039930a 100644 --- a/lib/lightning/invocation.ex +++ b/lib/lightning/invocation.ex @@ -657,6 +657,7 @@ defmodule Lightning.Invocation do |> filter_by_wo_date_before(search_params.wo_date_before) |> filter_by_date_after(search_params.date_after) |> filter_by_date_before(search_params.date_before) + |> filter_by_runs(search_params) |> filter_by_error_signature(search_params) |> filter_by_body_or_log_or_id( search_params.search_fields, @@ -753,6 +754,42 @@ defmodule Lightning.Invocation do ) end + defp filter_by_runs(query, %SearchParams{ + run_date_after: nil, + run_date_before: nil, + run_status: [] + }), + do: query + + defp filter_by_runs(query, %SearchParams{ + run_date_after: run_date_after, + run_date_before: run_date_before, + run_status: run_status + }) do + runs = + from(r in Run, where: r.work_order_id == parent_as(:workorder).id) + |> filter_run_inserted_after(run_date_after) + |> filter_run_inserted_before(run_date_before) + |> filter_run_statuses(run_status) + + from([workorder: _workorder] in query, where: exists(runs)) + end + + defp filter_run_inserted_after(query, nil), do: query + + defp filter_run_inserted_after(query, run_date_after), + do: where(query, [r], r.inserted_at >= ^run_date_after) + + defp filter_run_inserted_before(query, nil), do: query + + defp filter_run_inserted_before(query, run_date_before), + do: where(query, [r], r.inserted_at < ^run_date_before) + + defp filter_run_statuses(query, []), do: query + + defp filter_run_statuses(query, states), + do: where(query, [r], r.state in ^states) + # The inverse of `Run.state_reasons/0`, for reading a run-level signature's # `exit_reason` back into the state it came from. `"rejected"` is not a # value in that map — it is `to_signature/2`'s own literal for a work order diff --git a/lib/lightning/workorders/search_params.ex b/lib/lightning/workorders/search_params.ex index 6b3a082247..0ee17f36dd 100644 --- a/lib/lightning/workorders/search_params.ex +++ b/lib/lightning/workorders/search_params.ex @@ -16,6 +16,9 @@ defmodule Lightning.WorkOrders.SearchParams do :date_before, :wo_date_after, :wo_date_before, + :run_date_after, + :run_date_before, + :run_status, :sort_by, :sort_direction, :error_signature_exit_reason, @@ -30,6 +33,9 @@ defmodule Lightning.WorkOrders.SearchParams do @derive {JSON.Encoder, only: @fields} @status_values Lightning.WorkOrder.states() + # Run states, not work order states: the workflow health page's runs chart + # counts runs, and only a *final* run has an outcome to have been counted. + @run_status_values Lightning.Run.final_states() @search_field_values [:id, :body, :log, :dataclip_name] # String forms for the URI/flag params new/1 receives from the UI. @@ -53,6 +59,9 @@ defmodule Lightning.WorkOrders.SearchParams do date_before: DateTime.t(), wo_date_after: DateTime.t(), wo_date_before: DateTime.t(), + run_date_after: DateTime.t(), + run_date_before: DateTime.t(), + run_status: [atom()], sort_by: String.t(), sort_direction: String.t(), error_signature_exit_reason: String.t(), @@ -79,6 +88,15 @@ defmodule Lightning.WorkOrders.SearchParams do field(:sort_by, :string) field(:sort_direction, :string) + # Workflow health page filters + field(:run_date_after, :utc_datetime_usec) + field(:run_date_before, :utc_datetime_usec) + + field(:run_status, {:array, Ecto.Enum}, + values: @run_status_values, + default: [] + ) + # The error signature the workflow health page's triage row draws its # "View" button from. `error_signature_exit_reason` switches the # filter on; a present `error_signature_job_id` is a step-level row, diff --git a/lib/lightning_web/live/run_live/index.ex b/lib/lightning_web/live/run_live/index.ex index 79614fd706..d0e2e16c64 100644 --- a/lib/lightning_web/live/run_live/index.ex +++ b/lib/lightning_web/live/run_live/index.ex @@ -36,6 +36,9 @@ defmodule LightningWeb.RunLive.Index do date_before: :utc_datetime, wo_date_after: :utc_datetime, wo_date_before: :utc_datetime, + run_date_after: :utc_datetime, + run_date_before: :utc_datetime, + run_status: {:array, :string}, pending: :boolean, running: :boolean, success: :boolean, @@ -844,6 +847,21 @@ defmodule LightningWeb.RunLive.Index do end end + # The run-date chip's range. + # + # Stamped in UTC and says so: the bars this comes from are cut on the + # reader's clock, nothing in Lightning records what that clock is, and a bare + # "14:00" under a bar labelled "16:00" is worse than a suffix. The suffix + # goes on once, at the end, rather than on both ends of a range. + defp format_run_range(nil, nil), do: "any time" + defp format_run_range(from, nil), do: "after #{run_stamp(from)} UTC" + defp format_run_range(nil, to), do: "before #{run_stamp(to)} UTC" + + defp format_run_range(from, to), + do: "#{run_stamp(from)} – #{run_stamp(to)} UTC" + + defp run_stamp(date), do: Timex.format!(date, "{D}-{Mshort} {h24}:{m}") + defp format_date_range(date_after, date_before) do case {date_after, date_before} do {nil, nil} -> diff --git a/lib/lightning_web/live/run_live/index.html.heex b/lib/lightning_web/live/run_live/index.html.heex index 323403cdf2..a3f85a6df5 100644 --- a/lib/lightning_web/live/run_live/index.html.heex +++ b/lib/lightning_web/live/run_live/index.html.heex @@ -246,6 +246,44 @@ + <%!-- Run status chip. Only when set: a runs-chart band is the only + thing that sets it, and unlike the two date chips above there + is no dropdown behind it to open. Its own chip rather than a + clause on the run-date one, so a reader who wants the whole + slot can drop the band without losing the slot. --%> + <% run_status = get_change(@filters_changeset, :run_status) || [] %> + <%= if run_status != [] do %> + <.filter_chip + id="run-status-filter-chip" + active={true} + clear_fields={[{:run_status, nil}]} + > + Run status: {Enum.map_join( + run_status, + ", ", + &String.capitalize/1 + )} + + <% end %> + + <%!-- Run date chip. "Created", because these bound a run's + `inserted_at` — the chips above bound the work order's + arrival and its last activity. --%> + <% run_date_after = get_change(@filters_changeset, :run_date_after) + run_date_before = get_change(@filters_changeset, :run_date_before) %> + <%= if run_date_after || run_date_before do %> + <.filter_chip + id="run-dates-filter-chip" + active={true} + clear_fields={[ + {:run_date_after, nil}, + {:run_date_before, nil} + ]} + > + Run created {format_run_range(run_date_after, run_date_before)} + + <% end %> + <%!-- Work order ID chip (only when workorder_id filter is active) --%> <%= if workorder_id = get_change(@filters_changeset, :workorder_id) do %> <.filter_chip diff --git a/test/lightning/invocation_test.exs b/test/lightning/invocation_test.exs index 3aa62fcaea..9b335e2181 100644 --- a/test/lightning/invocation_test.exs +++ b/test/lightning/invocation_test.exs @@ -1781,6 +1781,140 @@ defmodule Lightning.InvocationTest do assert found_workorder.id == wo_now.id end + # The runs chart's bar link. Not expressible with either date filter above: + # both work orders here arrived and last moved at the same times, and only + # their runs differ. + test "filters workorders by when a run was created, half-open on the end" do + project = insert(:project) + workflow = insert(:workflow, project: project) + trigger = insert(:trigger, workflow: workflow) + + bar_start = ~U[2026-09-08T02:00:00.000000Z] + bar_end = ~U[2026-09-08T04:00:00.000000Z] + + run_at = fn at -> + work_order = + insert(:workorder, + workflow: workflow, + trigger: trigger, + inserted_at: bar_start, + last_activity: bar_end + ) + + insert(:run, + work_order: work_order, + dataclip: build(:dataclip), + starting_trigger: trigger, + inserted_at: at + ) + + work_order.id + end + + before_bar = run_at.(DateTime.add(bar_start, -1, :second)) + inside_bar = run_at.(DateTime.add(bar_start, 30, :minute)) + on_start = run_at.(bar_start) + # `date_bin` puts a run landing exactly on a boundary in the *later* + # bucket, so the bar that closes here must not claim it. + on_end = run_at.(bar_end) + + found = + Invocation.search_workorders( + project, + SearchParams.new(%{ + "run_date_after" => bar_start, + "run_date_before" => bar_end + }) + ).entries + |> Enum.map(& &1.id) + + assert Enum.sort(found) == Enum.sort([inside_bar, on_start]) + refute before_bar in found + refute on_end in found + end + + # A band of that bar. `run_status` and not `status`, which is the work + # order's *current* state: the retried work order here is a success now, + # and the red band that counted its failed run still has to reach it. + test "filters workorders by the run's own state, not the work order's" do + project = insert(:project) + workflow = insert(:workflow, project: project) + trigger = insert(:trigger, workflow: workflow) + + bar_start = ~U[2026-09-08T02:00:00.000000Z] + bar_end = ~U[2026-09-08T04:00:00.000000Z] + + run_at = fn wo_state, run_status -> + work_order = + insert(:workorder, + workflow: workflow, + trigger: trigger, + state: wo_state, + last_activity: bar_end + ) + + insert(:run, + work_order: work_order, + dataclip: build(:dataclip), + starting_trigger: trigger, + state: run_status, + inserted_at: DateTime.add(bar_start, 30, :minute) + ) + + work_order.id + end + + retried = run_at.(:success, :failed) + still_failed = run_at.(:failed, :failed) + succeeded = run_at.(:success, :success) + + failures = + Invocation.search_workorders( + project, + SearchParams.new(%{ + "run_date_after" => bar_start, + "run_date_before" => bar_end, + "run_status" => ["failed", "crashed", "killed", "exception", "lost"] + }) + ).entries + |> Enum.map(& &1.id) + + assert Enum.sort(failures) == Enum.sort([retried, still_failed]) + refute succeeded in failures + end + + # The newest bar is still filling, so its link carries no upper bound. + test "filters workorders by an open-ended run window" do + project = insert(:project) + workflow = insert(:workflow, project: project) + trigger = insert(:trigger, workflow: workflow) + + now = DateTime.utc_now() + + work_order = insert(:workorder, workflow: workflow, trigger: trigger) + + # Two runs, one either side of the bound: the work order matches on the + # newer one, and is listed once rather than twice. + for at <- [DateTime.add(now, -1, :hour), now] do + insert(:run, + work_order: work_order, + dataclip: build(:dataclip), + starting_trigger: trigger, + inserted_at: at + ) + end + + assert [found] = + Invocation.search_workorders( + project, + SearchParams.new(%{ + "run_date_after" => DateTime.add(now, -1, :minute) + }) + ).entries + + assert found.id == work_order.id + end + # to be replaced by paginator unit tests @tag :skip test "filters workorders sets timeout" do diff --git a/test/lightning_web/live/run_live/index_test.exs b/test/lightning_web/live/run_live/index_test.exs index 6305e5c43f..848d89fdf8 100644 --- a/test/lightning_web/live/run_live/index_test.exs +++ b/test/lightning_web/live/run_live/index_test.exs @@ -1049,6 +1049,85 @@ defmodule LightningWeb.RunLive.IndexTest do refute has_element?(view, "#error-signature-filter-chip") end + + # A band of a bar on the workflow health page's runs chart links here, and + # sets both. Two chips, so either can be dropped without the other: the + # band without the slot is every failure in the range, the slot without + # the band is the whole bar. + test "a runs-chart band renders a run status chip and a run date chip", %{ + conn: conn, + project: project + } do + {:ok, view, _html} = + live_async( + conn, + Routes.project_run_index_path(conn, :index, project.id, + filters: %{ + run_date_after: "2026-09-08T02:00:00Z", + run_date_before: "2026-09-08T04:00:00Z", + run_status: ["failed", "crashed"] + } + ) + ) + + assert render(element(view, "#run-status-filter-chip")) =~ + "Run status: Failed, Crashed" + + dates = render(element(view, "#run-dates-filter-chip")) + + # The stamp is UTC and says so: the bars are cut on the reader's clock + # and nothing here records what that clock is. + assert dates =~ "Run created 8-Sep 02:00" + assert dates =~ "8-Sep 04:00 UTC" + end + + test "a run status filter renders no run date chip", %{ + conn: conn, + project: project + } do + {:ok, view, _html} = + live_async( + conn, + Routes.project_run_index_path(conn, :index, project.id, + filters: %{run_status: ["success"]} + ) + ) + + assert render(element(view, "#run-status-filter-chip")) =~ + "Run status: Success" + + refute has_element?(view, "#run-dates-filter-chip") + end + + # The newest bar is still filling, so its link carries no upper bound. + test "run date chip reads open-ended without a run_date_before", %{ + conn: conn, + project: project + } do + {:ok, view, _html} = + live_async( + conn, + Routes.project_run_index_path(conn, :index, project.id, + filters: %{run_date_after: "2026-09-08T02:00:00Z"} + ) + ) + + assert render(element(view, "#run-dates-filter-chip")) =~ + "Run created after 8-Sep 02:00 UTC" + + refute has_element?(view, "#run-status-filter-chip") + end + + test "run chips are absent when neither filter is set", %{ + conn: conn, + project: project + } do + {:ok, view, _html} = + live_async(conn, Routes.project_run_index_path(conn, :index, project.id)) + + refute has_element?(view, "#run-status-filter-chip") + refute has_element?(view, "#run-dates-filter-chip") + end end describe "cancel work orders" do From 75c74524ee15e929d181118c6221c4a1ef1aeb4a Mon Sep 17 00:00:00 2001 From: Frank Midigo Date: Thu, 17 Sep 2026 21:11:51 +0300 Subject: [PATCH 4/7] Note the volume chart click-through in the changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38aaf19d6f..767d46b49e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,11 @@ and this project adheres to [#5188](https://github.com/OpenFn/lightning/pull/5188) - A "Steps with failures" card on the workflow health page, ranking the jobs the window's failures landed on, heaviest first. +- A band of a bar in the runs volume chart now opens the work orders it counted, + filtered to that bar's window and the run states the band stacked. History + gained run-level filters to carry it, so a failure that was later retried to + success is still reachable from the band that counted it. + [#5191](https://github.com/OpenFn/lightning/pull/5191) ### Changed From 5fad068103e15d959b1b05c48298418baff4d799 Mon Sep 17 00:00:00 2001 From: Stuart Corbishley Date: Fri, 18 Sep 2026 16:21:56 +0200 Subject: [PATCH 5/7] Cut the volume chart's grid with one clock database MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit date_bin cut the buckets with Postgres' zone rules while bucket_starts and resolve cut the same grid with Tzdata's. The two are versioned apart, so a transition they disagreed about put a tally under a local timestamp absent from the bar list, and Map.get dropped those runs. The bar read zero rather than wrong, which is the kind of wrong nobody reports. width_bucket takes the resolved boundaries instead and returns the slot, so Postgres never sees a zone name and Tzdata alone decides where a bar opens. The Coyhaique case goes with it: nothing asks Postgres about a zone it may not know, so there is nothing to rescue. Those boundaries have to be strictly ascending for width_bucket to mean anything, and Postgres answers an unsorted array with a plausible number rather than an error. Antarctica/Troll shifts two hours, so a transition can be as wide as the narrowest bucket — the property holds because of where the transitions land, not because of any margin, and the boundary test is what holds it. boundaries/3 is public so the grid can be checked without a database. That is the only way to put a fixed clock change inside the window, which is otherwise always relative to now, and it reaches the gap and ambiguous branches of resolve that nothing exercised before. Every run test now asserts the bars sum to the runs in the window. --- assets/js/health/charts/VolumeBars.tsx | 6 +- assets/test/health/WorkflowHealth.test.tsx | 4 +- assets/test/health/charts/VolumeBars.test.tsx | 5 +- lib/lightning/workflows/stats.ex | 99 +++++----- test/lightning/invocation_test.exs | 6 +- test/lightning/workflows/stats_test.exs | 174 +++++++++++++----- 6 files changed, 191 insertions(+), 103 deletions(-) diff --git a/assets/js/health/charts/VolumeBars.tsx b/assets/js/health/charts/VolumeBars.tsx index c3179f83bf..495e3604a6 100644 --- a/assets/js/health/charts/VolumeBars.tsx +++ b/assets/js/health/charts/VolumeBars.tsx @@ -267,9 +267,9 @@ export const bucketUrl = ( const TICK_FILL = '#6b7280'; -// In the timezone the server cut the grid on, not the browser's own: a bar starts -// at a local whole hour there, so a Delhi bar opening at UTC 18:30 labels as -// 00:00 and no label ever needs minutes. +// In the timezone the server cut the grid on, not the browser's own: a bar +// starts at a local whole hour there, so a Delhi bar opening at UTC 18:30 +// labels as 00:00 and no label ever needs minutes. const dayLabel = (date: Date, timezone: string) => date.toLocaleDateString(undefined, { month: 'short', diff --git a/assets/test/health/WorkflowHealth.test.tsx b/assets/test/health/WorkflowHealth.test.tsx index 3bb5a20e05..94a3de4ee8 100644 --- a/assets/test/health/WorkflowHealth.test.tsx +++ b/assets/test/health/WorkflowHealth.test.tsx @@ -129,8 +129,8 @@ describe('WorkflowHealth', () => { ); }); - // Nothing server-side records a reader's timezone, so the request has to carry - // it or the chart's buckets go back to being cut on UTC. + // Nothing server-side records a reader's timezone, so the request has to + // carry it or the chart's buckets are cut on UTC. test("tells the server the reader's timezone", async () => { const { fetchMock } = mount(both); diff --git a/assets/test/health/charts/VolumeBars.test.tsx b/assets/test/health/charts/VolumeBars.test.tsx index 9da6f0cffe..782cf8dcf5 100644 --- a/assets/test/health/charts/VolumeBars.test.tsx +++ b/assets/test/health/charts/VolumeBars.test.tsx @@ -155,8 +155,7 @@ describe('tickLabel and rangeLabel', () => { }); // The half- and quarter-hour zones. The grid is floored on the local clock, - // so a boundary is a local whole hour and no label ever needs minutes — the - // thing that broke when the hour was read off UTC. + // so a boundary is a local whole hour and no label ever needs minutes. test('clocks the off-the-hour zones as whole local hours', () => { expect(tickLabel('2026-09-08T18:30:00Z', 2, 'Asia/Kolkata')).toBe('00:00'); expect(tickLabel('2026-09-08T18:15:00Z', 2, 'Asia/Kathmandu')).toBe( @@ -166,7 +165,7 @@ describe('tickLabel and rangeLabel', () => { }); // The bar's own link. Its end is the next bar's start, so the boundary a run -// lands on belongs to exactly one bar — the same half-open slot `date_bin` +// lands on belongs to exactly one bar — the same half-open slot the server // counted it in. describe('bucketUrl', () => { const buckets = [ diff --git a/lib/lightning/workflows/stats.ex b/lib/lightning/workflows/stats.ex index 4b41d28c96..18b52e6f68 100644 --- a/lib/lightning/workflows/stats.ex +++ b/lib/lightning/workflows/stats.ex @@ -125,9 +125,9 @@ defmodule Lightning.Workflows.Stats do whole and JSON-encoded — to draw thirty bars. Bucketed on the local calendar rather than on the epoch, which is what makes - it daylight-saving-correct: `date_bin` bins the *local* timestamp, so - London's 25-hour October day is one bar 25 hours wide and every boundary - stays on local midnight. + it daylight-saving-correct: every boundary is a local whole hour turned into + the instant it names, so London's 25-hour October day is one bar 25 hours + wide and every daily boundary stays on local midnight. Buckets are counted on `inserted_at` — when the attempt started, not when it settled — so a run stays in the bar the traffic arrived in. Every bucket and @@ -151,11 +151,7 @@ defmodule Lightning.Workflows.Stats do bucket_hours: pos_integer(), buckets: [run_bucket()] } - def runs( - %Workflow{id: workflow_id}, - days_back \\ @default_days_back, - timezone \\ "Etc/UTC" - ) + def runs(%Workflow{id: workflow_id}, days_back, timezone) when is_map_key(@buckets, days_back) do # No change marker in this key — a busy workflow moves the marker faster # than the page polls, so every poll would miss (211 ms and 1.4 GiB of @@ -173,45 +169,43 @@ defmodule Lightning.Workflows.Stats do ) end - # Postgres, not Tzdata, is the authority on what `at time zone` accepts, and - # the two zone sets are versioned apart: `:tzdata` autoupdates at runtime - # everywhere but test, while `pg_timezone_names` is fixed at the server's - # build. So a zone the controller validated can still be one this server has - # never heard of — `America/Coyhaique` is exactly that against PG 15, and it - # is what Chrome reports in Coyhaique, Chile. - # - # Redrawn on UTC rather than left to raise, which keeps the policy the - # controller already sets for every other unusable timezone: a bad window - # cannot be drawn, a bad clock still can. The result is cached under the - # requested zone, so the dead query runs once per `@runs_ttl`, not per poll. + # The start of every bar as a real instant, oldest first, in UTC. Resolved + # here and nowhere else: Tzdata and `pg_timezone_names` are versioned apart, + # so a boundary the two databases place differently would be runs tallied + # into a bar that is never drawn. # - # Not checked up front: `pg_timezone_names` costs ~90 ms to scan, more than - # the query it would guard. - defp bucket_window(workflow_id, days_back, timezone) do + # The list has to be strictly ascending: `width_bucket` reads an unsorted + # array without complaining and returns a plausible number, so a boundary + # that landed at or before its neighbour would be wrong counts per bar and no + # error. The boundary test holds that for the zones it names — London, + # Havana, Troll, Lord Howe, Chatham — not for all of tzdata. + @doc false + @spec boundaries(DateTime.t(), 1 | 7 | 30, Calendar.time_zone()) :: [ + DateTime.t() + ] + def boundaries(to, days_back, timezone) do {hours, count} = Map.fetch!(@buckets, days_back) + + to + |> bucket_starts(hours, count, timezone) + |> Enum.map(&resolve(&1, timezone)) + end + + defp bucket_window(workflow_id, days_back, timezone) do + {hours, _count} = Map.fetch!(@buckets, days_back) to = DateTime.utc_now() - starts = bucket_starts(to, hours, count, timezone) - from = resolve(hd(starts), timezone) + starts = boundaries(to, days_back, timezone) %{ - window: %{from: from, to: to}, + window: %{from: hd(starts), to: to}, timezone: timezone, bucket_hours: hours, - buckets: bucket_runs(workflow_id, starts, from, hours, timezone) + buckets: bucket_runs(workflow_id, starts) } - rescue - error in Postgrex.Error -> - if error.postgres[:code] == :invalid_parameter_value and - timezone != "Etc/UTC" do - bucket_window(workflow_id, days_back, "Etc/UTC") - else - reraise error, __STACKTRACE__ - end end # The local wall-clock start of every bar, oldest first, as naive local - # datetimes — the same domain `date_bin` bins in below, so a tally matches a - # bar by equality and no timezone arithmetic is needed to line them up. + # datetimes, for `boundaries/3` to resolve. # # Anchored on the bucket `now` is in, not on `now` itself: a window that ends # mid-bucket would put every boundary at whatever minute the request landed @@ -241,14 +235,16 @@ defmodule Lightning.Workflows.Stats do |> DateTime.shift_zone!("Etc/UTC") end - defp bucket_runs(workflow_id, starts, from, hours, timezone) do - tallies = tally_runs(workflow_id, from, hours, timezone) + defp bucket_runs(workflow_id, starts) do + tallies = tally_runs(workflow_id, starts) - Enum.map(starts, fn start -> + starts + |> Enum.with_index(1) + |> Enum.map(fn {start, bucket} -> tallies - |> Map.get(start, []) + |> Map.get(bucket, []) |> Enum.into(@zero_run_counts) - |> Map.put(:at, resolve(start, timezone)) + |> Map.put(:at, start) end) end @@ -259,12 +255,14 @@ defmodule Lightning.Workflows.Stats do # the window before the nested loop into `runs(work_order_id, inserted_at)`, # instead of probing every work order the workflow ever had. # - # `inserted_at` is a naked `timestamp` holding UTC, so it is labelled UTC and - # then read on the reader's clock before binning: `date_bin` bins the local - # wall clock, which is what puts a boundary on local midnight and makes a - # 25-hour day one bar. The origin is a midnight, so every width — 2, 12, 24 — - # aligns to one. - defp tally_runs(workflow_id, from, hours, timezone) do + # `inserted_at` is a naked `timestamp` holding UTC, so the boundaries go down + # as naive UTC datetimes to match it, and `width_bucket` assigns each row the + # 1-based slot it falls in — lower edge inclusive. Slot 0 is below the oldest + # boundary, which the `>= from` filter has already excluded; the highest slot + # is the bar `now` is in, still filling. + defp tally_runs(workflow_id, [from | _] = starts) do + edges = Enum.map(starts, &DateTime.to_naive/1) + from(r in Run, join: wo in WorkOrder, on: wo.id == r.work_order_id, @@ -275,10 +273,9 @@ defmodule Lightning.Workflows.Stats do select: { selected_as( fragment( - "date_bin(make_interval(hours => ?::int), ? at time zone 'UTC' at time zone ?, timestamp '2000-01-01')", - ^hours, + "width_bucket(?, ?)", r.inserted_at, - ^timezone + ^edges ), :bucket ), @@ -288,7 +285,7 @@ defmodule Lightning.Workflows.Stats do ) |> Repo.all() |> Enum.group_by( - fn {bucket, _, _} -> NaiveDateTime.truncate(bucket, :second) end, + fn {bucket, _, _} -> bucket end, fn {_, state, count} -> {state, count} end ) end diff --git a/test/lightning/invocation_test.exs b/test/lightning/invocation_test.exs index 9b335e2181..856b6bc3f1 100644 --- a/test/lightning/invocation_test.exs +++ b/test/lightning/invocation_test.exs @@ -1814,8 +1814,8 @@ defmodule Lightning.InvocationTest do before_bar = run_at.(DateTime.add(bar_start, -1, :second)) inside_bar = run_at.(DateTime.add(bar_start, 30, :minute)) on_start = run_at.(bar_start) - # `date_bin` puts a run landing exactly on a boundary in the *later* - # bucket, so the bar that closes here must not claim it. + # The chart puts a run landing exactly on a boundary in the *later* bar, + # so the bar that closes here must not claim it. on_end = run_at.(bar_end) found = @@ -2966,7 +2966,7 @@ defmodule Lightning.InvocationTest do %{project: project, workflow: workflow, snapshot: snapshot, run: run} end - defp add_step(ctx, name, opts \\ []) do + defp add_step(ctx, name, opts) do step = insert( :step, diff --git a/test/lightning/workflows/stats_test.exs b/test/lightning/workflows/stats_test.exs index 11d8e149fe..9aeb03a2b8 100644 --- a/test/lightning/workflows/stats_test.exs +++ b/test/lightning/workflows/stats_test.exs @@ -129,9 +129,9 @@ defmodule Lightning.Workflows.StatsTest do assert %{signatures: []} = Stats.error_signatures(workflow) end - # The review comment this whole unit change is for: retrying a failure until - # it works has to make the page's failure count fall, and only a work order's - # state can fall — the failed run stays failed forever. + # Retrying a failure until it works has to make the page's failure count + # fall, and only a work order's state can fall — the failed run stays failed + # forever. test "counts a work order retried to success once, as a success", %{ workflow: workflow, trigger: trigger @@ -636,7 +636,7 @@ defmodule Lightning.Workflows.StatsTest do assert %{signatures: []} = Stats.error_signatures(workflow) end - # Both render as `unknown`, so ungrouped the table drew two rows under one + # Both render as `unknown`, so ungrouped they are two table rows under one # React key. test "groups an empty error type with a missing one", %{ workflow: workflow, @@ -656,7 +656,7 @@ defmodule Lightning.Workflows.StatsTest do assert %{count: 2, error_type: nil} = signature end - # `""` is truthy, so it won the `||` in `to_signature/2`. + # `""` is truthy, so it wins the `||` in `to_signature/2`. test "does not let an empty step error type mask the run's", %{ workflow: workflow, trigger: trigger @@ -710,14 +710,23 @@ defmodule Lightning.Workflows.StatsTest do |> div(DateTime.diff(b.at, a.at, :second)) end + # Every run in the window lands in some bar. A grid whose edges don't line + # up with the tally drops runs silently, and no per-bar assertion catches + # that, so the total is asserted everywhere the bars are. + defp total(buckets) do + Enum.sum_by(buckets, fn bucket -> + bucket |> Map.delete(:at) |> Map.values() |> Enum.sum() + end) + end + test "counts each state into the bucket its run started in", ctx do %{workflow: workflow, trigger: trigger} = ctx older = DateTime.add(DateTime.utc_now(), -5, :hour) newer = DateTime.add(DateTime.utc_now(), -90, :minute) - # A hair inside a bucket, not in the next one: `date_bin` floors, and this - # is the run that would move if it ever stopped. + # A hair inside a bucket, so it stays there rather than opening the next + # one. edge = DateTime.utc_now() |> DateTime.to_unix() @@ -732,7 +741,7 @@ defmodule Lightning.Workflows.StatsTest do run_at(workflow, trigger, :success, older) run_at(workflow, trigger, :crashed, edge) - assert %{buckets: buckets} = result = Stats.runs(workflow, 1) + assert %{buckets: buckets} = result = Stats.runs(workflow, 1, "Etc/UTC") assert %{success: 2, failed: 0} = Enum.at(buckets, bucket_of(result, older)) @@ -741,6 +750,24 @@ defmodule Lightning.Workflows.StatsTest do Enum.at(buckets, bucket_of(result, newer)) assert %{crashed: 1} = Enum.at(buckets, bucket_of(result, edge)) + assert total(buckets) == 4 + end + + # `width_bucket` takes the lower edge, so a run at the boundary instant + # opens that bar. The slot below the oldest boundary is drawn by no bar at + # all, so a run falling into it would leave the chart without a trace. + test "counts a run landing exactly on a boundary into that bar", ctx do + %{workflow: workflow, trigger: trigger} = ctx + + [from | _] = Stats.boundaries(DateTime.utc_now(), 1, "Etc/UTC") + + run_at(workflow, trigger, :success, from) + + assert %{buckets: [first | _] = buckets} = + Stats.runs(workflow, 1, "Etc/UTC") + + assert %{success: 1} = first + assert total(buckets) == 1 end # Boundaries on the reader's clock, so the chart can label a bar with an @@ -811,26 +838,6 @@ defmodule Lightning.Workflows.StatsTest do end end - # The claim the local-calendar grid rests on, and the one a UTC grid gets - # wrong: London's October Sunday is 25 hours long, and both sides of the - # change belong to the same bar. Asserted against the binning expression - # itself, because the window is always the last 30 days and no fixed - # transition stays inside it. - test "bins a 25-hour local day as a single bucket" do - %{rows: rows} = - Repo.query!(""" - SELECT date_bin( - make_interval(hours => 24), - t at time zone 'UTC' at time zone 'Europe/London', - timestamp '2000-01-01' - ) - FROM (VALUES (timestamp '2025-10-26 00:30'), - (timestamp '2025-10-26 23:30')) AS v(t) - """) - - assert rows == List.duplicate([~N[2025-10-26 00:00:00.000000]], 2) - end - # Without the timezone in the key, the first reader to load the page would # pin their grid on every other timezone for the TTL — and on one machine # that is invisible. @@ -853,23 +860,16 @@ defmodule Lightning.Workflows.StatsTest do end end - test "defaults to a UTC grid when no timezone is given", ctx do - %{workflow: workflow} = ctx - - assert %{window: window, timezone: "Etc/UTC"} = Stats.runs(workflow, 1) - assert rem(DateTime.to_unix(window.from), 7_200) == 0 - end - # Keyed without the change marker — a settle must not mint a new key. test "serves the same answer after a work order settles", ctx do %{workflow: workflow, trigger: trigger} = ctx run_at(workflow, trigger, :success, DateTime.utc_now()) - first = Stats.runs(workflow, 1) + first = Stats.runs(workflow, 1, "Etc/UTC") run_at(workflow, trigger, :failed, DateTime.utc_now()) - assert Stats.runs(workflow, 1) == first + assert Stats.runs(workflow, 1, "Etc/UTC") == first assert {:ok, true} = Cachex.exists?( @@ -888,11 +888,103 @@ defmodule Lightning.Workflows.StatsTest do other = insert(:simple_workflow) run_at(other, hd(other.triggers), :success, DateTime.utc_now()) - assert %{buckets: buckets} = Stats.runs(workflow, 1) + assert %{buckets: buckets} = Stats.runs(workflow, 1, "Etc/UTC") + + assert total(buckets) == 0 + end - assert Enum.sum_by(buckets, fn bucket -> - bucket |> Map.delete(:at) |> Map.values() |> Enum.sum() - end) == 0 + # Tzdata knows this zone and PG 15 does not. Nothing sends a zone name to + # Postgres, so the two databases disagreeing about it cannot reach the + # chart. + test "draws a zone this Postgres has never heard of", ctx do + %{workflow: workflow, trigger: trigger} = ctx + + run_at(workflow, trigger, :success, DateTime.utc_now()) + + run_at( + workflow, + trigger, + :failed, + DateTime.add(DateTime.utc_now(), -5, :hour) + ) + + assert %{buckets: buckets, timezone: "America/Coyhaique"} = + Stats.runs(workflow, 1, "America/Coyhaique") + + assert total(buckets) == 2 + end + end + + # `runs/3` always ends its window at `DateTime.utc_now()` and nothing here can + # move that clock, so the clock changes are exercised against the grid itself + # at fixed dates. + describe "boundaries/3" do + defp spans(boundaries) do + boundaries + |> Enum.chunk_every(2, 1, :discard) + |> Enum.map(fn [a, b] -> DateTime.diff(b, a, :hour) end) + end + + # The claim the local-calendar grid rests on, and the one a UTC grid gets + # wrong: London's October Sunday is 25 hours long, and both sides of the + # change belong to the same bar. + test "gives a 25-hour day one 24-hour-labelled bar" do + boundaries = + Stats.boundaries(~U[2025-10-27 12:00:00Z], 30, "Europe/London") + + assert Enum.count(spans(boundaries), &(&1 == 25)) == 1 + assert Enum.all?(spans(boundaries), &(&1 in [24, 25])) + assert ~U[2025-10-25 23:00:00Z] in boundaries + assert ~U[2025-10-27 00:00:00Z] in boundaries + end + + # Havana springs forward at local midnight, so the daily boundary itself is + # the hour that never happened. It opens when the clock reaches 01:00. + test "opens a bar whose local start never happened at the jump" do + boundaries = + Stats.boundaries(~U[2026-03-09 12:00:00Z], 30, "America/Havana") + + assert Enum.count(spans(boundaries), &(&1 == 23)) == 1 + assert ~U[2026-03-08 05:00:00Z] in boundaries + end + + # The same midnight twice in November. The bar opens at the first, so it + # opens when its label says it does. + test "opens a bar on the first of two identical local starts" do + boundaries = + Stats.boundaries(~U[2025-11-02 12:00:00Z], 30, "America/Havana") + + assert ~U[2025-11-02 04:00:00Z] in boundaries + refute ~U[2025-11-02 05:00:00Z] in boundaries + end + + # `width_bucket` reads an unsorted array without complaining and returns a + # plausible slot for every row, so a clock change that moved a boundary past + # its neighbour would be silently wrong counts rather than an error. Troll + # shifts two hours, Lord Howe thirty minutes, and Havana's daily boundary + # lands in the change itself. + test "never places a boundary at or before the one before it" do + dates = + for day <- 0..364, + do: DateTime.add(~U[2026-01-01 12:00:00Z], day, :day) + + for zone <- [ + "Europe/London", + "America/Havana", + "Antarctica/Troll", + "Australia/Lord_Howe", + "Pacific/Chatham" + ], + days <- [1, 7, 30], + to <- dates do + boundaries = Stats.boundaries(to, days, zone) + where = "#{zone}, #{days}d, ending #{to}" + + assert boundaries == Enum.sort(boundaries, DateTime), + "out of order: #{where}" + + assert boundaries == Enum.uniq(boundaries), "repeated boundary: #{where}" + end end end end From 36c29fd4a1f99697d7c813ef2069a1ceac6da4bb Mon Sep 17 00:00:00 2001 From: Stuart Corbishley Date: Fri, 18 Sep 2026 16:22:07 +0200 Subject: [PATCH 6/7] Decide the reader's timezone in one place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three places chose UTC: the controller's default, the rescue that redrew the window when Postgres rejected a zone, and a default argument on Stats.runs/3. The rescue went with the binning change and the default argument goes here, leaving the plug. No header is UTC, because a cached bundle from before the header existed has to keep working through a deploy. Etc/Unknown is UTC too — it is CLDR's sentinel for a host clock it could not map, so a browser sending it is saying it does not know rather than naming a zone we failed to recognise. Anything else the tz database does not know is a 400: the browser picked it, and drawing someone else's clock would hide that rather than fix it. vary, because this is the one action whose body turns on a request header and nothing between the browser and here would guess it. The stats cache stays unbounded. The timezone is in its key, so the key space is wider than it was, but an entry is ~16 KiB against the ~200 ms aggregate it takes to fill one — the database is the scarce resource here, and a size limit would bound the wrong one. --- lib/lightning/application.ex | 5 ++ .../api/workflow_health_controller.ex | 60 ++++++++++++------- .../api/workflow_health_controller_test.exs | 43 +++++++++++-- 3 files changed, 80 insertions(+), 28 deletions(-) diff --git a/lib/lightning/application.ex b/lib/lightning/application.ex index 5c8f1d7370..d5d85e43e8 100644 --- a/lib/lightning/application.ex +++ b/lib/lightning/application.ex @@ -59,6 +59,11 @@ defmodule Lightning.Application do # Workflow health page stats, cached briefly to dedupe bursts on the same # workflow. See `Lightning.Workflows.Stats`. + # + # Unbounded, keyed by workflow, window and timezone. An entry is ~16 KiB + # and lives two minutes, and filling one costs a ~200 ms aggregate, so the + # database is the scarce resource here and a size limit would bound the + # wrong one. workflow_stats_cache_childspec = Supervisor.child_spec({Cachex, name: :workflow_stats}, id: :workflow_stats_cache diff --git a/lib/lightning_web/controllers/api/workflow_health_controller.ex b/lib/lightning_web/controllers/api/workflow_health_controller.ex index 59be25d922..126ea99072 100644 --- a/lib/lightning_web/controllers/api/workflow_health_controller.ex +++ b/lib/lightning_web/controllers/api/workflow_health_controller.ex @@ -19,7 +19,6 @@ defmodule LightningWeb.API.WorkflowHealthController do plug :authorize_workflow # After :authorize_workflow so a 404 wins over a 400. plug :validate_days - # Auth, then params, then presentation. plug :validate_timezone when action in [:runs] def outcomes(conn, _params) do @@ -39,9 +38,12 @@ defmodule LightningWeb.API.WorkflowHealthController do ) end + # `vary` because this is the one action whose body depends on a request + # header, and nothing between the browser and here would guess that. def runs(conn, _params) do - json( - conn, + conn + |> put_resp_header("vary", "x-timezone") + |> json( Workflows.Stats.runs( conn.assigns.workflow, conn.assigns.days_back, @@ -55,6 +57,11 @@ defmodule LightningWeb.API.WorkflowHealthController do @default_days "30" @default_timezone "Etc/UTC" + # CLDR's sentinel for a host clock it could not map to an IANA zone. A + # browser sending it is telling us it does not know, which is the same thing + # as not telling us. + @unknown_timezone "Etc/Unknown" + defp validate_days(conn, _opts) do case Map.fetch(@days, conn.params["days"] || @default_days) do {:ok, days_back} -> @@ -68,27 +75,36 @@ defmodule LightningWeb.API.WorkflowHealthController do end end - # The reader's timezone, because nothing in Lightning records one. Unknown - # or absent falls back to UTC rather than 400ing: a bad `days` means a window - # we cannot draw, but a bad timezone still has a drawable answer, and an old - # cached bundle that sends no header has to keep working through a deploy. + # The reader's timezone, because nothing in Lightning records one. The only + # place a default is chosen: a browser that sends no header, or says it does + # not know, gets UTC; anything else that is not a zone is a 400, because the + # browser picked it and drawing someone else's clock would hide that. # - # Validated before it reaches the cache key. `:workflow_stats` has no size - # limit (`application.ex:62-65`), so an arbitrary string would let one - # authenticated reader mint unbounded entries; `Tzdata.zone_exists?/1` bounds - # the key to the tz database. - # Postgres, which does the binning, knows a narrower set — `Stats.runs/3` - # falls back to UTC for the difference. + # Validated before it reaches the cache key, so `:workflow_stats` is keyed on + # the tz database rather than on anything a header can carry. defp validate_timezone(conn, _opts) do - timezone = - with [timezone] <- get_req_header(conn, "x-timezone"), - true <- Tzdata.zone_exists?(timezone) do - timezone - else - _ -> @default_timezone - end - - assign(conn, :timezone, timezone) + case get_req_header(conn, "x-timezone") do + [] -> + assign(conn, :timezone, @default_timezone) + + [@unknown_timezone] -> + assign(conn, :timezone, @default_timezone) + + [timezone] -> + if Tzdata.zone_exists?(timezone), + do: assign(conn, :timezone, timezone), + else: reject_timezone(conn) + + _ -> + reject_timezone(conn) + end + end + + defp reject_timezone(conn) do + conn + |> put_status(:bad_request) + |> json(%{error: "x-timezone must be an IANA timezone name"}) + |> halt() end defp authorize_workflow(conn, _opts) do diff --git a/test/lightning_web/controllers/api/workflow_health_controller_test.exs b/test/lightning_web/controllers/api/workflow_health_controller_test.exs index 8ffb997f69..e741355486 100644 --- a/test/lightning_web/controllers/api/workflow_health_controller_test.exs +++ b/test/lightning_web/controllers/api/workflow_health_controller_test.exs @@ -403,9 +403,9 @@ defmodule LightningWeb.API.WorkflowHealthControllerTest do end end - # Nothing in Lightning records a reader's timezone, so the browser says. A bad - # value still has a drawable answer, so it falls back to UTC rather than - # 400ing the way a bad `days` does. + # Nothing in Lightning records a reader's timezone, so the browser says. No + # header, or one saying the browser does not know, is UTC; anything else the + # tz database does not know is a 400, because the browser chose it. describe "x-timezone" do test "cuts the grid on the timezone the reader sent", %{ conn: conn, @@ -442,9 +442,38 @@ defmodule LightningWeb.API.WorkflowHealthControllerTest do assert response["timezone"] == "Etc/UTC" end + # A host clock CLDR could not map to a zone. The browser is saying it does + # not know, not naming one we failed to recognise. + test "falls back to UTC when the browser says it does not know", %{ + conn: conn, + user: user, + project: project, + workflow: workflow + } do + response = + conn + |> get_runs(user, project.id, workflow.id, %{"days" => "1"}, [ + {"x-timezone", "Etc/Unknown"} + ]) + |> json_response(200) + + assert response["timezone"] == "Etc/UTC" + end + + test "tells caches the body turns on the header", %{ + conn: conn, + user: user, + project: project, + workflow: workflow + } do + conn = get_runs(conn, user, project.id, workflow.id, %{"days" => "1"}) + + assert get_resp_header(conn, "vary") == ["x-timezone"] + end + # Unknown, empty, and long — the cache key has to stay bounded to the tz # database whatever arrives. - test "falls back to UTC on a value it cannot use", %{ + test "rejects a value it cannot use", %{ conn: conn, user: user, project: project, @@ -456,9 +485,11 @@ defmodule LightningWeb.API.WorkflowHealthControllerTest do |> get_runs(user, project.id, workflow.id, %{"days" => "1"}, [ {"x-timezone", value} ]) - |> json_response(200) + |> json_response(400) - assert response["timezone"] == "Etc/UTC" + assert response == %{ + "error" => "x-timezone must be an IANA timezone name" + } end end end From 0d634a515b9ea89d28c3e1405eddd9034070b17e Mon Sep 17 00:00:00 2001 From: Stuart Corbishley Date: Fri, 18 Sep 2026 16:22:17 +0200 Subject: [PATCH 7/7] Rewrite the changelog entries for the volume chart work The click-through and the history filters land separately for a reader, so they read as separate entries rather than one. The timezone entry now says what a browser gets when it sends no zone or one the tz database does not know. --- CHANGELOG.md | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 767d46b49e..43fb7d04a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,18 +23,23 @@ and this project adheres to [#5188](https://github.com/OpenFn/lightning/pull/5188) - A "Steps with failures" card on the workflow health page, ranking the jobs the window's failures landed on, heaviest first. -- A band of a bar in the runs volume chart now opens the work orders it counted, - filtered to that bar's window and the run states the band stacked. History - gained run-level filters to carry it, so a failure that was later retried to - success is still reachable from the band that counted it. +- A band of a bar in the runs volume chart on the workflow health page now opens + the work orders it counted, filtered to that bar's slot and the run states the + band stacked. [#5191](https://github.com/OpenFn/lightning/pull/5191) +- History can now filter on when a run was created and on the state a run + finished in, each as its own chip, so a failure later retried to success is + still reachable from the band that counted it. [#5191](https://github.com/OpenFn/lightning/pull/5191) ### Changed -- The runs volume chart on the workflow health page is now bucketed in your own +- The runs volume chart on the workflow health page is now bucketed on your own timezone rather than UTC, so a daily bar covers your day, the week view's bars - split at your noon, and a day the clocks change is still one bar. Bar widths - are unchanged, and the card names the clock they are drawn on. + split at your noon, a day the clocks change is still one bar, and the card + names the clock it was drawn on. A browser that sends no timezone, or says it + does not know one, still gets UTC; one that sends a zone the tz database does + not know now gets an error. + [#5191](https://github.com/OpenFn/lightning/pull/5191) ### Fixed