From 36b3f3b44059f9ea6ce5e63be8ddc606177f6b32 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Fri, 10 Jul 2026 11:35:00 -0600 Subject: [PATCH 1/2] fix(status): slide the analytics window on refresh instead of re-polling a frozen range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The [startTime, endTime] analytics window was computed once and baked into every panel's query key; refetchInterval then re-fetched that same fixed window forever, so charts never advanced and every tick was pure load on the customer's Harper while the freshness pill implied live data. A single clock in StatusTabsInner now slides the window forward each refresh period — skipping ticks while the browser tab is hidden (one catch-up tick on return), coalescing near-simultaneous refresh sources, and holding off while a fetch batch is still in flight. Manual refresh shares the same path. useAnalyticsRecords no longer polls: a fixed window is an immutable snapshot (staleTime Infinity, gcTime bounded to a minute so stranded snapshots don't accumulate), and keepPreviousData bridges the key swap. The now-unused refreshIntervalMs is removed from AnalyticsContext and its consumers. Fixes #1439 Co-Authored-By: Claude Fable 5 --- .../instance/status/analytics/StatusTabs.tsx | 54 +++++- .../StatusTabs.sliding-window.test.tsx | 168 ++++++++++++++++++ .../analytics/context/AnalyticsContext.tsx | 2 - .../hooks/useAnalyticsRecords.test.ts | 16 +- .../analytics/hooks/useAnalyticsRecords.ts | 43 +++-- .../analytics/tabs/ConnectionsPanel.tsx | 4 +- .../status/analytics/tabs/MetricPanel.tsx | 3 +- .../status/analytics/tabs/StorageTab.tsx | 3 +- .../analytics/tabs/__tests__/testHelpers.tsx | 1 - 9 files changed, 239 insertions(+), 55 deletions(-) create mode 100644 src/features/instance/status/analytics/__tests__/StatusTabs.sliding-window.test.tsx diff --git a/src/features/instance/status/analytics/StatusTabs.tsx b/src/features/instance/status/analytics/StatusTabs.tsx index 0764ed04a..a472d214d 100644 --- a/src/features/instance/status/analytics/StatusTabs.tsx +++ b/src/features/instance/status/analytics/StatusTabs.tsx @@ -2,8 +2,10 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@ import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs'; import type { InstanceClientIdConfig, InstanceTypeConfig } from '@/config/instanceClientConfig.ts'; import { useSystemTheme } from '@/hooks/useSystemTheme'; +import { ANALYTICS_QUERY_KEY_PREFIX } from '@/integrations/api/instance/status/getAnalytics.ts'; +import { useQueryClient } from '@tanstack/react-query'; import { useNavigate, useSearch } from '@tanstack/react-router'; -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { AnalyticsOnboardingHint } from './components/AnalyticsOnboardingHint.tsx'; import { TimeRangePicker } from './components/TimeRangePicker.tsx'; import { type AnalyticsContextValue, AnalyticsProvider } from './context/AnalyticsContext.tsx'; @@ -76,9 +78,46 @@ function StatusTabsInner({ instanceParams, isLocalStudio }: Props) { ? Number(raw.refresh) : DEFAULT_REFRESH_MS; - // Manual refresh ticks bump this to force a fresh window when the user - // clicks the refresh button. + // The analytics window is a fixed [start, end] snapshot baked into every + // panel's query key, so "refresh" means sliding the window forward to a + // new snapshot — re-fetching the old one could only return the same data. + // `tick` bumps recompute the window below; the interval drives it when + // auto-refresh is on, and the manual refresh button shares the same path. const [tick, setTick] = useState(0); + const queryClient = useQueryClient(); + const lastRefreshRef = useRef(Date.now()); + const refresh = useCallback(() => { + lastRefreshRef.current = Date.now(); + setTick((t) => t + 1); + }, []); + + useEffect(() => { + if (refreshMs <= 0) { return; } + // Tick guards, in order: + // - hidden tab: fetching for an invisible dashboard is pure load on + // the customer's Harper; the visibility handler below catches up + // once on return if at least one full period elapsed. + // - elapsed: coalesces near-simultaneous refresh sources — a catch-up + // or manual refresh just before a scheduled tick would otherwise + // double-fetch every panel back-to-back. + // - in-flight: when Harper is slower than the cadence, sliding the + // window again would key a fresh POST batch on top of the running + // one (new key = no dedupe); skip until the current batch settles. + const canTick = () => + !document.hidden + && queryClient.isFetching({ queryKey: [ANALYTICS_QUERY_KEY_PREFIX] }) === 0; + const intervalId = setInterval(() => { + if (canTick() && Date.now() - lastRefreshRef.current >= refreshMs - 1000) { refresh(); } + }, refreshMs); + const onVisibilityChange = () => { + if (canTick() && Date.now() - lastRefreshRef.current >= refreshMs) { refresh(); } + }; + document.addEventListener('visibilitychange', onVisibilityChange); + return () => { + clearInterval(intervalId); + document.removeEventListener('visibilitychange', onVisibilityChange); + }; + }, [refreshMs, refresh, queryClient]); const theme = useSystemTheme(); @@ -107,17 +146,16 @@ function StatusTabsInner({ instanceParams, isLocalStudio }: Props) { const preset = getPreset(presetId); const endTime = Date.now(); const startTime = endTime - preset.durationMs; - // `tick` participates in memo deps so a manual refresh produces a fresh - // window even if the user did not change presets. + // `tick` participates in memo deps so every refresh (interval or manual) + // produces a fresh window even if the user did not change presets. void tick; return { timeRange: { startTime, endTime }, bucketMs: preset.bucketMs, - refreshIntervalMs: refreshMs, theme, instanceParams, }; - }, [presetId, refreshMs, theme, instanceParams, tick]); + }, [presetId, theme, instanceParams, tick]); const showTimePicker = tab !== 'overview'; const picker = showTimePicker @@ -127,7 +165,7 @@ function StatusTabsInner({ instanceParams, isLocalStudio }: Props) { onPresetChange={updatePreset} refreshMs={refreshMs} onRefreshChange={updateRefreshMs} - onManualRefresh={() => setTick((t) => t + 1)} + onManualRefresh={refresh} /> ) : null; diff --git a/src/features/instance/status/analytics/__tests__/StatusTabs.sliding-window.test.tsx b/src/features/instance/status/analytics/__tests__/StatusTabs.sliding-window.test.tsx new file mode 100644 index 000000000..33bea80c2 --- /dev/null +++ b/src/features/instance/status/analytics/__tests__/StatusTabs.sliding-window.test.tsx @@ -0,0 +1,168 @@ +// @vitest-environment happy-dom +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, cleanup, render } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Regression coverage for the frozen-refresh-window bug (#1439): the analytics +// window is baked into every panel's query key, so auto-refresh must slide the +// window forward — re-polling a fixed [start, end] can only return stale data. +// These tests observe the windows each panel requests via the mocked records +// hook and assert the clock in StatusTabsInner advances them. + +const seenWindows: { startTime: number; endTime: number }[] = []; +vi.mock('../hooks/useAnalyticsRecords.ts', () => ({ + useAnalyticsRecords: (args: { startTime: number; endTime: number }) => { + seenWindows.push({ startTime: args.startTime, endTime: args.endTime }); + return { + data: [], + isLoading: false, + isError: false, + error: null, + isEmpty: true, + fieldKeys: new Set(), + missingFields: [], + refetch: vi.fn(), + }; + }, +})); + +vi.mock('../hooks/useAnalyticsCapability.ts', () => ({ + useAnalyticsCapability: () => ({ supported: true, isLoading: false, retry: vi.fn() }), +})); + +let currentSearch: Record = {}; +const navigateMock = vi.fn(async (opts: { search?: unknown }) => { + if (typeof opts.search === 'object' && opts.search !== null) { + currentSearch = { ...(opts.search as Record) }; + } else { + currentSearch = {}; + } +}); +vi.mock('@tanstack/react-router', () => ({ + useSearch: () => currentSearch, + useNavigate: () => navigateMock, +})); + +import { DEFAULT_REFRESH_MS } from '../context/timePresets.ts'; +import { StatusTabs } from '../StatusTabs.tsx'; + +const instanceParams = { + instanceClient: { post: vi.fn(async () => ({ data: [] })) } as never, + entityId: 'inst-A' as never, + entityType: 'instance' as const, +}; + +function mount() { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false, gcTime: 0 } }, + }); + return render( + + + , + ); +} + +function latestEndTime(): number { + return Math.max(...seenWindows.map((w) => w.endTime)); +} + +/** Force `document.hidden` (happy-dom defaults it to false, non-configurable + * access goes through visibilityState — define both). */ +function setDocumentHidden(hidden: boolean) { + Object.defineProperty(document, 'hidden', { configurable: true, get: () => hidden }); + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: () => (hidden ? 'hidden' : 'visible'), + }); +} + +beforeEach(() => { + vi.useFakeTimers(); + currentSearch = {}; + seenWindows.length = 0; + navigateMock.mockClear(); + setDocumentHidden(false); + Object.defineProperty(window, 'matchMedia', { + writable: true, + configurable: true, + value: vi.fn().mockImplementation((query: string) => ({ + matches: false, + media: query, + onchange: null, + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + addListener: vi.fn(), + removeListener: vi.fn(), + dispatchEvent: vi.fn(), + })), + }); +}); + +afterEach(() => { + cleanup(); + vi.useRealTimers(); +}); + +describe('StatusTabs sliding refresh window', () => { + it('advances the window by one refresh period per tick', async () => { + mount(); + const initialEnd = latestEndTime(); + expect(initialEnd).toBeGreaterThan(0); + + await act(async () => { + await vi.advanceTimersByTimeAsync(DEFAULT_REFRESH_MS + 5); + }); + const afterOneTick = latestEndTime(); + expect(afterOneTick).toBeGreaterThanOrEqual(initialEnd + DEFAULT_REFRESH_MS); + + await act(async () => { + await vi.advanceTimersByTimeAsync(DEFAULT_REFRESH_MS); + }); + expect(latestEndTime()).toBeGreaterThanOrEqual(afterOneTick + DEFAULT_REFRESH_MS); + }); + + it('does not tick while auto-refresh is off (refresh=0)', async () => { + currentSearch = { refresh: 0 }; + mount(); + const initialEnd = latestEndTime(); + + await act(async () => { + await vi.advanceTimersByTimeAsync(10 * DEFAULT_REFRESH_MS); + }); + expect(latestEndTime()).toBe(initialEnd); + }); + + it('pauses while the browser tab is hidden and catches up once on return', async () => { + mount(); + const initialEnd = latestEndTime(); + + setDocumentHidden(true); + await act(async () => { + await vi.advanceTimersByTimeAsync(2.5 * DEFAULT_REFRESH_MS); + }); + // Hidden: interval fires but skips the refresh — window stays frozen. + expect(latestEndTime()).toBe(initialEnd); + + setDocumentHidden(false); + await act(async () => { + document.dispatchEvent(new Event('visibilitychange')); + }); + // Visible again: one catch-up tick (not one per missed period). + const afterReturn = latestEndTime(); + expect(afterReturn).toBeGreaterThanOrEqual(initialEnd + 2 * DEFAULT_REFRESH_MS); + + // The next scheduled interval tick lands half a period after the + // catch-up; the elapsed guard must swallow it (no back-to-back burst)… + await act(async () => { + await vi.advanceTimersByTimeAsync(0.5 * DEFAULT_REFRESH_MS + 5); + }); + expect(latestEndTime()).toBe(afterReturn); + + // …while the tick after that (a full period since catch-up) fires. + await act(async () => { + await vi.advanceTimersByTimeAsync(DEFAULT_REFRESH_MS); + }); + expect(latestEndTime()).toBeGreaterThanOrEqual(afterReturn + DEFAULT_REFRESH_MS); + }); +}); diff --git a/src/features/instance/status/analytics/context/AnalyticsContext.tsx b/src/features/instance/status/analytics/context/AnalyticsContext.tsx index f61c31afc..6396ea771 100644 --- a/src/features/instance/status/analytics/context/AnalyticsContext.tsx +++ b/src/features/instance/status/analytics/context/AnalyticsContext.tsx @@ -7,7 +7,6 @@ export type AnalyticsTheme = 'light' | 'dark'; export interface AnalyticsContextValue { timeRange: TimeRange; bucketMs: number; - refreshIntervalMs: number; theme: AnalyticsTheme; instanceParams: InstanceClientIdConfig & InstanceTypeConfig; } @@ -24,7 +23,6 @@ export function AnalyticsProvider({ value, children }: ProviderProps) { value.timeRange.startTime, value.timeRange.endTime, value.bucketMs, - value.refreshIntervalMs, value.theme, value.instanceParams.entityId, ]); diff --git a/src/features/instance/status/analytics/hooks/useAnalyticsRecords.test.ts b/src/features/instance/status/analytics/hooks/useAnalyticsRecords.test.ts index ba2305845..723112dbc 100644 --- a/src/features/instance/status/analytics/hooks/useAnalyticsRecords.test.ts +++ b/src/features/instance/status/analytics/hooks/useAnalyticsRecords.test.ts @@ -48,7 +48,6 @@ describe('useAnalyticsRecords', () => { startTime: 0, endTime: 10_000, instanceParams: makeInstanceParams(rows), - refetchIntervalMs: 0, }), { wrapper: Wrapper }, ); @@ -66,7 +65,6 @@ describe('useAnalyticsRecords', () => { startTime: 0, endTime: 10_000, instanceParams: makeInstanceParams([]), - refetchIntervalMs: 0, }), { wrapper: Wrapper }, ); @@ -88,7 +86,6 @@ describe('useAnalyticsRecords', () => { startTime: 0, endTime: 10_000, instanceParams: makeInstanceParams([]), - refetchIntervalMs: 0, requiredFields: ['p95', 'path'], }), { wrapper: Wrapper }, @@ -111,7 +108,6 @@ describe('useAnalyticsRecords', () => { startTime: 0, endTime: 10_000, instanceParams: makeInstanceParams(rows), - refetchIntervalMs: 0, requiredFields: ['p95', 'p99'], }), { wrapper: Wrapper }, @@ -133,7 +129,6 @@ describe('useAnalyticsRecords', () => { startTime: 0, endTime: 10_000, instanceParams: paramsA, - refetchIntervalMs: 0, }), { wrapper: Wrapper }, ); @@ -144,7 +139,6 @@ describe('useAnalyticsRecords', () => { startTime: 0, endTime: 10_000, instanceParams: paramsB, - refetchIntervalMs: 0, }), { wrapper: Wrapper }, ); @@ -172,7 +166,6 @@ describe('useAnalyticsRecords', () => { startTime: 0, endTime: 10_000, instanceParams: makeInstanceParams([], { throwError: new Error('boom') }), - refetchIntervalMs: 0, }), { wrapper: Wrapper }, ); @@ -191,7 +184,6 @@ describe('useAnalyticsRecords', () => { endTime: 10_000, instanceParams: params, conditions: [{ attribute: 'path', value: '/api/x' }], - refetchIntervalMs: 0, }), { wrapper: Wrapper }, ); @@ -215,7 +207,6 @@ describe('useAnalyticsRecords', () => { endTime: 10_000, instanceParams: params, bucketMs: 60_000, - refetchIntervalMs: 0, }), { wrapper: Wrapper }, ); @@ -239,7 +230,6 @@ describe('useAnalyticsRecords', () => { startTime: 0, endTime: 10_000, instanceParams: makeInstanceParams([]), - refetchIntervalMs: 0, }), { wrapper: Wrapper }, ); @@ -260,7 +250,6 @@ describe('useAnalyticsRecords', () => { startTime: 100, endTime: 200, instanceParams: makeInstanceParams(driftRows), - refetchIntervalMs: 0, requiredFields: ['user'], }), { wrapper: W2 }, @@ -277,7 +266,6 @@ describe('useAnalyticsRecords', () => { startTime: 200, endTime: 300, instanceParams: makeInstanceParams([]), - refetchIntervalMs: 0, requiredFields: ['user'], }), { wrapper: W3 }, @@ -294,7 +282,7 @@ describe('useAnalyticsRecords', () => { warn.mockRestore(); }); - it('refetchIntervalMs=0 disables polling (no extra fetch after initial)', async () => { + it('never polls — no extra fetch after the initial one', async () => { vi.useFakeTimers(); try { const { Wrapper } = wrapper(); @@ -306,7 +294,6 @@ describe('useAnalyticsRecords', () => { startTime: 0, endTime: 10_000, instanceParams: params, - refetchIntervalMs: 0, }), { wrapper: Wrapper }, ); @@ -334,7 +321,6 @@ describe('useAnalyticsRecords', () => { startTime: args.startTime, endTime: args.endTime, instanceParams: params, - refetchIntervalMs: 0, }), { wrapper: Wrapper, diff --git a/src/features/instance/status/analytics/hooks/useAnalyticsRecords.ts b/src/features/instance/status/analytics/hooks/useAnalyticsRecords.ts index d7458d28b..e407287a7 100644 --- a/src/features/instance/status/analytics/hooks/useAnalyticsRecords.ts +++ b/src/features/instance/status/analytics/hooks/useAnalyticsRecords.ts @@ -4,7 +4,7 @@ import { getRawAnalyticsQueryOptions, } from '@/integrations/api/instance/status/getAnalytics.ts'; import { keepPreviousData, useQuery } from '@tanstack/react-query'; -import { useEffect, useMemo, useRef } from 'react'; +import { useEffect, useMemo } from 'react'; import type { AnalyticsDataPoint } from '../types/analytics.ts'; export interface UseAnalyticsRecordsArgs { @@ -13,8 +13,6 @@ export interface UseAnalyticsRecordsArgs { endTime: number; conditions?: AnalyticsCondition[]; instanceParams: InstanceClientIdConfig & InstanceTypeConfig; - /** Polling cadence in ms. Set to 0 to disable. */ - refetchIntervalMs?: number; /** Required keys this metric's spec depends on. Missing keys surface via * `missingFields`, which the renderer can use to show a precise empty * state instead of a blank chart. */ @@ -46,27 +44,24 @@ const RESERVED = new Set(['time', 'node']); const EMPTY: readonly AnalyticsDataPoint[] = Object.freeze([]); /** Adapter from studio's `get_analytics` operation to the analytics-viz spec - * pipeline. Passes rows through verbatim, exposes a schema-drift signal so - * callers can render an explicit "field unavailable" state, and applies - * small jitter to the polling start time so a tab's many concurrent specs - * do not fire in lockstep on every refresh tick. */ + * pipeline. Passes rows through verbatim and exposes a schema-drift signal so + * callers can render an explicit "field unavailable" state. + * + * This hook never polls: a `[startTime, endTime]` window is a fixed snapshot + * (it participates in the query key), so re-fetching it can only return the + * same range. Auto-refresh works by the caller sliding the window forward + * (see `StatusTabsInner`), which produces a new query key and one fresh + * fetch per panel; `keepPreviousData` keeps the old series on screen while + * the new window loads. */ export function useAnalyticsRecords({ metric, startTime, endTime, conditions, instanceParams, - refetchIntervalMs = 60_000, requiredFields, bucketMs, }: UseAnalyticsRecordsArgs): UseAnalyticsRecordsResult { - // Per-spec startup jitter (0–500 ms) so 5–7 concurrent specs in one tab - // do not refire in the same render frame on auto-refresh. - const jitterRef = useRef(null); - if (jitterRef.current === null) { - jitterRef.current = Math.floor(Math.random() * 500); - } - const queryOpts = getRawAnalyticsQueryOptions({ metric, startTime, @@ -78,18 +73,22 @@ export function useAnalyticsRecords({ const query = useQuery({ ...queryOpts, - staleTime: refetchIntervalMs > 0 ? refetchIntervalMs : Infinity, - refetchInterval: refetchIntervalMs > 0 ? refetchIntervalMs + jitterRef.current : false, + // A fixed window's data is effectively immutable (Harper may still be + // aggregating the newest bucket, but the next window slide re-fetches + // it anyway), so never treat a cached window as stale — remounts and + // tab switches within one refresh period are served from cache. + staleTime: Infinity, + // …but do bound retention: every window slide strands the previous + // snapshot (up to the row cap) as an inactive cache entry, and the + // default 5-minute gcTime would hold a rolling backlog of them on a + // fast refresh cadence. One minute is enough for keepPreviousData to + // bridge the swap and for quick tab flips to hit cache. + gcTime: 60_000, refetchOnWindowFocus: false, refetchOnReconnect: false, placeholderData: keepPreviousData, }); - // React Query already pauses interval refetching when the tab is hidden; - // we don't add a visibility-driven refetch ourselves because that turns - // every alt-tab into a synchronized N-panel POST burst on the customer's - // Harper, bypassing staleTime entirely. - const data = (query.data ?? EMPTY) as AnalyticsDataPoint[]; const { fieldKeys, missingFields } = useMemo(() => { diff --git a/src/features/instance/status/analytics/tabs/ConnectionsPanel.tsx b/src/features/instance/status/analytics/tabs/ConnectionsPanel.tsx index 26081794b..1e8278e76 100644 --- a/src/features/instance/status/analytics/tabs/ConnectionsPanel.tsx +++ b/src/features/instance/status/analytics/tabs/ConnectionsPanel.tsx @@ -30,7 +30,7 @@ export function ConnectionsPanel() { } function ConnectionsPanelInner() { - const { timeRange, bucketMs, refreshIntervalMs, theme, instanceParams } = useAnalyticsContext(); + const { timeRange, bucketMs, theme, instanceParams } = useAnalyticsContext(); // Chart-only ref; capturing the whole Card would include the action buttons // in the exported PNG. const chartRef = useRef(null); @@ -40,7 +40,6 @@ function ConnectionsPanelInner() { startTime: timeRange.startTime, endTime: timeRange.endTime, instanceParams, - refetchIntervalMs: refreshIntervalMs, bucketMs, }); const ws = useAnalyticsRecords({ @@ -48,7 +47,6 @@ function ConnectionsPanelInner() { startTime: timeRange.startTime, endTime: timeRange.endTime, instanceParams, - refetchIntervalMs: refreshIntervalMs, bucketMs, }); diff --git a/src/features/instance/status/analytics/tabs/MetricPanel.tsx b/src/features/instance/status/analytics/tabs/MetricPanel.tsx index 540fe7cac..cf36903fe 100644 --- a/src/features/instance/status/analytics/tabs/MetricPanel.tsx +++ b/src/features/instance/status/analytics/tabs/MetricPanel.tsx @@ -30,7 +30,7 @@ export function MetricPanel({ metric, titleOverride }: Props) { } function MetricPanelInner({ metric, titleOverride }: Props) { - const { timeRange, bucketMs, refreshIntervalMs, theme, instanceParams } = useAnalyticsContext(); + const { timeRange, bucketMs, theme, instanceParams } = useAnalyticsContext(); const sourceMetric = derivedRegistry[metric]?.sourceMetric ?? metric; const requiredFields = getSpecRequiredFields(metric); const { data, isLoading, isError, error, isEmpty, missingFields, refetch } = useAnalyticsRecords({ @@ -38,7 +38,6 @@ function MetricPanelInner({ metric, titleOverride }: Props) { startTime: timeRange.startTime, endTime: timeRange.endTime, instanceParams, - refetchIntervalMs: refreshIntervalMs, bucketMs, requiredFields, }); diff --git a/src/features/instance/status/analytics/tabs/StorageTab.tsx b/src/features/instance/status/analytics/tabs/StorageTab.tsx index 61946968a..49cf2b66d 100644 --- a/src/features/instance/status/analytics/tabs/StorageTab.tsx +++ b/src/features/instance/status/analytics/tabs/StorageTab.tsx @@ -32,13 +32,12 @@ export function StorageTab() { } function TableSizePanels() { - const { timeRange, bucketMs, refreshIntervalMs, theme, instanceParams } = useAnalyticsContext(); + const { timeRange, bucketMs, theme, instanceParams } = useAnalyticsContext(); const { data, isLoading, isError } = useAnalyticsRecords({ metric: 'table-size', startTime: timeRange.startTime, endTime: timeRange.endTime, instanceParams, - refetchIntervalMs: refreshIntervalMs, bucketMs, }); diff --git a/src/features/instance/status/analytics/tabs/__tests__/testHelpers.tsx b/src/features/instance/status/analytics/tabs/__tests__/testHelpers.tsx index 8c8c4cffd..ef791fb96 100644 --- a/src/features/instance/status/analytics/tabs/__tests__/testHelpers.tsx +++ b/src/features/instance/status/analytics/tabs/__tests__/testHelpers.tsx @@ -17,7 +17,6 @@ export function makeContextValue(overrides: Partial = {}) return { timeRange: { startTime: 0, endTime: 60_000 }, bucketMs: 60_000, - refreshIntervalMs: 0, theme: 'light', instanceParams: makeInstanceParams(), ...overrides, From 1495d47075ae719aa1240a2ab91930b3130f8757 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Fri, 10 Jul 2026 11:41:53 -0600 Subject: [PATCH 2/2] fix(status): restart the refresh interval on every tick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review suggestion (gemini): with tick in the effect deps the interval is recreated after every refresh, so the next scheduled tick is always exactly refreshMs after the last actual refresh — the elapsed-time coalescing guard becomes unnecessary and a manual refresh no longer delays the next auto-refresh by up to two periods. Co-Authored-By: Claude Fable 5 --- .../instance/status/analytics/StatusTabs.tsx | 15 +++++++++------ .../__tests__/StatusTabs.sliding-window.test.tsx | 6 +++--- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/features/instance/status/analytics/StatusTabs.tsx b/src/features/instance/status/analytics/StatusTabs.tsx index a472d214d..931a1efec 100644 --- a/src/features/instance/status/analytics/StatusTabs.tsx +++ b/src/features/instance/status/analytics/StatusTabs.tsx @@ -93,13 +93,16 @@ function StatusTabsInner({ instanceParams, isLocalStudio }: Props) { useEffect(() => { if (refreshMs <= 0) { return; } - // Tick guards, in order: + // `tick` in the deps restarts the interval on EVERY refresh (scheduled, + // manual, or visibility catch-up), so the next scheduled tick is always + // exactly refreshMs after the last actual refresh — no elapsed-time + // bookkeeping and no double-fetch when refresh sources land close + // together. + void tick; + // Tick guards: // - hidden tab: fetching for an invisible dashboard is pure load on // the customer's Harper; the visibility handler below catches up // once on return if at least one full period elapsed. - // - elapsed: coalesces near-simultaneous refresh sources — a catch-up - // or manual refresh just before a scheduled tick would otherwise - // double-fetch every panel back-to-back. // - in-flight: when Harper is slower than the cadence, sliding the // window again would key a fresh POST batch on top of the running // one (new key = no dedupe); skip until the current batch settles. @@ -107,7 +110,7 @@ function StatusTabsInner({ instanceParams, isLocalStudio }: Props) { !document.hidden && queryClient.isFetching({ queryKey: [ANALYTICS_QUERY_KEY_PREFIX] }) === 0; const intervalId = setInterval(() => { - if (canTick() && Date.now() - lastRefreshRef.current >= refreshMs - 1000) { refresh(); } + if (canTick()) { refresh(); } }, refreshMs); const onVisibilityChange = () => { if (canTick() && Date.now() - lastRefreshRef.current >= refreshMs) { refresh(); } @@ -117,7 +120,7 @@ function StatusTabsInner({ instanceParams, isLocalStudio }: Props) { clearInterval(intervalId); document.removeEventListener('visibilitychange', onVisibilityChange); }; - }, [refreshMs, refresh, queryClient]); + }, [refreshMs, refresh, queryClient, tick]); const theme = useSystemTheme(); diff --git a/src/features/instance/status/analytics/__tests__/StatusTabs.sliding-window.test.tsx b/src/features/instance/status/analytics/__tests__/StatusTabs.sliding-window.test.tsx index 33bea80c2..e32f53552 100644 --- a/src/features/instance/status/analytics/__tests__/StatusTabs.sliding-window.test.tsx +++ b/src/features/instance/status/analytics/__tests__/StatusTabs.sliding-window.test.tsx @@ -152,14 +152,14 @@ describe('StatusTabs sliding refresh window', () => { const afterReturn = latestEndTime(); expect(afterReturn).toBeGreaterThanOrEqual(initialEnd + 2 * DEFAULT_REFRESH_MS); - // The next scheduled interval tick lands half a period after the - // catch-up; the elapsed guard must swallow it (no back-to-back burst)… + // The catch-up restarts the interval, so no tick fires at the old + // half-period-away boundary (no back-to-back burst)… await act(async () => { await vi.advanceTimersByTimeAsync(0.5 * DEFAULT_REFRESH_MS + 5); }); expect(latestEndTime()).toBe(afterReturn); - // …while the tick after that (a full period since catch-up) fires. + // …and the next tick fires a full period after the catch-up. await act(async () => { await vi.advanceTimersByTimeAsync(DEFAULT_REFRESH_MS); });