From cba60d6e1857dfdd0e30bc2bb84928c152409c26 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Fri, 28 Aug 2026 17:38:35 +0200 Subject: [PATCH] refactor: show the warning in the schedule editor, fed by the caller's runs --- .../schedules/scheduleDrift.test.ts | 28 ++++++------- .../lib/components/schedules/scheduleDrift.ts | 39 +++++++++--------- .../triggers/schedules/ScheduleEditor.svelte | 11 ++++- .../schedules/ScheduleEditorInner.svelte | 37 ++++++++++++++++- .../(root)/(logged)/schedules/+page.svelte | 40 +++++-------------- 5 files changed, 89 insertions(+), 66 deletions(-) diff --git a/frontend/src/lib/components/schedules/scheduleDrift.test.ts b/frontend/src/lib/components/schedules/scheduleDrift.test.ts index 747e3248b1..c170712552 100644 --- a/frontend/src/lib/components/schedules/scheduleDrift.test.ts +++ b/frontend/src/lib/components/schedules/scheduleDrift.test.ts @@ -1,48 +1,48 @@ import { describe, it, expect } from 'vitest' -import { scheduleOutlastsItsInterval } from './scheduleDrift' +import { runsOutlastingInterval } from './scheduleDrift' const runs = (duration_ms: number) => Array.from({ length: 5 }, () => ({ duration_ms })) // Each exemption below is a schedule that is genuinely running less often than // its cron reads, and is still not something to report. Losing one of them turns // the badge into noise on a correctly configured schedule. -describe('scheduleOutlastsItsInterval', () => { - it('flags runs that outlast the gap between slots', () => { - expect(scheduleOutlastsItsInterval({ enabled: true, interval_s: 20, jobs: runs(50_000) })).toBe( - true +describe('runsOutlastingInterval', () => { + it('reports how long runs that outlast the gap between slots are taking', () => { + expect(runsOutlastingInterval({ enabled: true, interval_s: 20, jobs: runs(50_000) })).toBe( + 50_000 ) }) it('says nothing while the runs still fit', () => { - expect(scheduleOutlastsItsInterval({ enabled: true, interval_s: 20, jobs: runs(5_000) })).toBe( - false - ) + expect( + runsOutlastingInterval({ enabled: true, interval_s: 20, jobs: runs(5_000) }) + ).toBeUndefined() }) it('exempts a schedule that queues its next run as the previous one starts', () => { expect( - scheduleOutlastsItsInterval({ + runsOutlastingInterval({ enabled: true, queues_next_run_at_start: true, interval_s: 20, jobs: runs(50_000) }) - ).toBe(false) + ).toBeUndefined() }) it('exempts a disabled schedule, which is not running at all', () => { expect( - scheduleOutlastsItsInterval({ enabled: false, interval_s: 20, jobs: runs(50_000) }) - ).toBe(false) + runsOutlastingInterval({ enabled: false, interval_s: 20, jobs: runs(50_000) }) + ).toBeUndefined() }) it('waits for more than one run before calling it a pattern', () => { expect( - scheduleOutlastsItsInterval({ + runsOutlastingInterval({ enabled: true, interval_s: 20, jobs: [{ duration_ms: 50_000 }] }) - ).toBe(false) + ).toBeUndefined() }) }) diff --git a/frontend/src/lib/components/schedules/scheduleDrift.ts b/frontend/src/lib/components/schedules/scheduleDrift.ts index ec58d40be4..b4b54f44ee 100644 --- a/frontend/src/lib/components/schedules/scheduleDrift.ts +++ b/frontend/src/lib/components/schedules/scheduleDrift.ts @@ -1,26 +1,29 @@ const MIN_RUNS = 3 -/** - * Whether a schedule's runs are taking longer than the gap between its slots. - * - * A plain script schedule queues its next run only once the previous one has - * completed, so a run that outlasts the interval necessarily pushes the next - * one to a later slot: the schedule quietly runs less often than its cron says. - * Schedules that queue the next run as the previous one starts are exempt, and - * the server says which those are. - * - * Reads the runs the schedules page has already loaded, and asks for a few of - * them so that one slow run is not read as a change of cadence. - */ -export function scheduleOutlastsItsInterval(schedule: { +/** What the schedules page has already loaded about how a schedule is running. */ +export type ScheduleRunsSample = { queues_next_run_at_start?: boolean enabled?: boolean interval_s?: number jobs?: Array<{ duration_ms: number }> -}): boolean { - const { queues_next_run_at_start, enabled, interval_s, jobs } = schedule - if (queues_next_run_at_start || !enabled || !interval_s || (jobs?.length ?? 0) < MIN_RUNS) - return false +} + +/** + * How long a schedule's runs have been taking, when that is longer than the gap + * between its slots, and `undefined` otherwise. + * + * A plain script schedule queues its next run only once the previous one has + * completed, so a run that outlasts the interval necessarily pushes the next one + * to a later slot: the schedule quietly runs less often than its cron says. + * Schedules that queue the next run as the previous one starts are exempt, and + * the server says which those are. + */ +export function runsOutlastingInterval(sample: ScheduleRunsSample): number | undefined { + const { queues_next_run_at_start, enabled, interval_s, jobs } = sample + if (queues_next_run_at_start || !enabled || !interval_s || (jobs?.length ?? 0) < MIN_RUNS) { + return undefined + } const durations = jobs!.map((j) => j.duration_ms).sort((a, b) => a - b) - return durations[durations.length >> 1] > interval_s * 1000 + const median = durations[durations.length >> 1] + return median > interval_s * 1000 ? median : undefined } diff --git a/frontend/src/lib/components/triggers/schedules/ScheduleEditor.svelte b/frontend/src/lib/components/triggers/schedules/ScheduleEditor.svelte index 5869225ff2..ed8d8b0b2d 100644 --- a/frontend/src/lib/components/triggers/schedules/ScheduleEditor.svelte +++ b/frontend/src/lib/components/triggers/schedules/ScheduleEditor.svelte @@ -1,8 +1,15 @@ {#if open} - + {/if} diff --git a/frontend/src/lib/components/triggers/schedules/ScheduleEditorInner.svelte b/frontend/src/lib/components/triggers/schedules/ScheduleEditorInner.svelte index 7aa83cbbee..8c3a190ee1 100644 --- a/frontend/src/lib/components/triggers/schedules/ScheduleEditorInner.svelte +++ b/frontend/src/lib/components/triggers/schedules/ScheduleEditorInner.svelte @@ -30,7 +30,15 @@ type ErrorHandler } from '$lib/gen' import { enterpriseLicense, userStore, workspaceStore } from '$lib/stores' - import { canWrite, emptyString, formatCron, sendUserToast, cronV1toV2 } from '$lib/utils' + import { + canWrite, + emptyString, + formatCron, + msToReadableTime, + msToReadableTimeShort, + sendUserToast, + cronV1toV2 + } from '$lib/utils' import { base } from '$lib/base' import Section from '$lib/components/Section.svelte' import { List, Loader2, Save, AlertTriangle } from 'lucide-svelte' @@ -50,6 +58,10 @@ import { twMerge } from 'tailwind-merge' import PermissionedAsLine from '../PermissionedAsLine.svelte' import { getTriggerWorkspace } from '$lib/components/triggers/triggerWorkspace' + import { + runsOutlastingInterval, + type ScheduleRunsSample + } from '$lib/components/schedules/scheduleDrift' let { useDrawer = true, @@ -63,7 +75,14 @@ onConfigChange = undefined, onDelete = undefined, onReset = undefined, - trigger = undefined + trigger = undefined, + getRunsSample = undefined + }: { + [key: string]: any + /// Supplied by callers that already hold the schedule's recent runs, so the + /// warning below costs no fetch of its own. Absent everywhere else, and the + /// warning simply does not appear. + getRunsSample?: (path: string) => ScheduleRunsSample | undefined } = $props() let optionTabSelected: @@ -123,6 +142,10 @@ let labels: string[] | undefined = $state(undefined) let description = $state('') let no_flow_overlap = $state(false) + // Measured, not configured: it describes the runs the deployed schedule has + // already had, so it is read from the caller's sample rather than the form. + let runsSample = $derived(initialPath ? getRunsSample?.(initialPath) : undefined) + let outlastingMs = $derived(runsSample ? runsOutlastingInterval(runsSample) : undefined) let tag: string | undefined = $state(undefined) let validCRON = $state(true) let isValid = $state(true) @@ -919,6 +942,16 @@ bind:validCRON bind:cronVersion /> + {#if outlastingMs && runsSample?.interval_s} + + Recent runs have been taking about {msToReadableTimeShort(outlastingMs, 0)}, against {msToReadableTime( + runsSample.interval_s * 1000 + )} between slots. Script runs never overlap, so the next run is only queued once the + previous one has completed: this schedule is running less often than its cron asks + for. To keep the cadence, schedule a flow instead, which queues its next run when the + previous one starts. + + {/if}
s.path === path) + return row && { ...row, jobs: row.jobs } + } + // Deep link: # opens that schedule's edit drawer. Tracks the last // handled hash (not a one-shot flag) so a hash change on the already-mounted // page (e.g. the AI session preview re-pointing its tab) opens the drawer @@ -342,7 +342,7 @@ - + {#if $userStore?.operator && $workspaceStore && !$userWorkspaces.find((_) => _.id === $workspaceStore)?.operator_settings?.schedules}