From 0eb43937cdcb6b0a9db9ab346753ce4045004661 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Thu, 11 Jun 2026 12:06:32 -0700 Subject: [PATCH 1/9] fix(charts): keep a consistent top-N series set across chunked queries Each time-window chunk's __hdx_series_limit CTE ranked the top N within its own window, so the union across chunks could exceed seriesLimit and adjacent windows disagreed on which series to keep. Chunked queries now carry the full chart range as seriesLimitDateRange and the CTE ranks over that pinned range, so every chunk keeps the identical top-N set. Follow-up to HDX-4499. --- .../hooks/__tests__/useChartConfig.test.tsx | 86 ++++++++++++++++++ packages/app/src/hooks/useChartConfig.tsx | 27 ++++-- .../__tests__/queryChartConfig.int.test.ts | 91 +++++++++++++++++++ .../src/__tests__/renderChartConfig.test.ts | 68 ++++++++++++++ .../src/core/renderChartConfig.ts | 26 +++++- packages/common-utils/src/types.ts | 4 + 6 files changed, 289 insertions(+), 13 deletions(-) diff --git a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx index 5371cf9c1a..05bf82a53b 100644 --- a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx +++ b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx @@ -802,6 +802,65 @@ describe('useChartConfig', () => { expect(result.current.isPending).toBe(false); }); + it('pins the series-limit ranking to the full date range on each chunk', async () => { + const fullRange: [Date, Date] = [ + new Date('2025-10-01 00:00:00Z'), + new Date('2025-10-02 00:00:00Z'), + ]; + const config = createMockChartConfig({ + dateRange: fullRange, + granularity: '3 hour', + seriesLimit: 3, + }); + + mockClickhouseClient.queryChartConfig.mockResolvedValue( + createMockQueryResponse([]), + ); + + const { result } = renderHook( + () => useQueriedChartConfig(config, { enableQueryChunking: true }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + // Each chunk queries its own window but must rank top-N series over + // the full chart range, or the union across chunks exceeds the limit. + const calls = mockClickhouseClient.queryChartConfig.mock.calls; + expect(calls).toHaveLength(3); + for (const [{ config: windowed }] of calls) { + expect(windowed.seriesLimitDateRange).toEqual(fullRange); + expect(windowed.dateRange).not.toEqual(fullRange); + } + }); + + it('does not set seriesLimitDateRange when the query is not chunked', async () => { + const config = createMockChartConfig({ + dateRange: [ + new Date('2025-10-01 00:00:00Z'), + new Date('2025-10-02 00:00:00Z'), + ], + granularity: '3 hour', + seriesLimit: 3, + }); + + mockClickhouseClient.queryChartConfig.mockResolvedValue( + createMockQueryResponse([]), + ); + + const { result } = renderHook(() => useQueriedChartConfig(config), { + wrapper, + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + const calls = mockClickhouseClient.queryChartConfig.mock.calls; + expect(calls).toHaveLength(1); + expect('seriesLimitDateRange' in calls[0][0].config).toBe(false); + }); + it('remains in a fetching state, with partial data until all data is loaded', async () => { const config = createMockChartConfig({ dateRange: [ @@ -1327,6 +1386,33 @@ describe('useChartConfig', () => { }); }); + it('pins the series-limit ranking to the full date range with parallel queries', async () => { + const { config } = setupParallelQueries(); + const configWithLimit = { ...config, seriesLimit: 3 }; + + mockClickhouseClient.queryChartConfig.mockResolvedValue( + createMockQueryResponse([]), + ); + + const { result } = renderHook( + () => + useQueriedChartConfig(configWithLimit, { + enableQueryChunking: true, + enableParallelQueries: true, + }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + const calls = mockClickhouseClient.queryChartConfig.mock.calls; + expect(calls).toHaveLength(3); + for (const [{ config: windowed }] of calls) { + expect(windowed.seriesLimitDateRange).toEqual(config.dateRange); + } + }); + it('should not execute query while useMVOptimizationExplanation is in loading state', async () => { const config = createMockChartConfig({ dateRange: [ diff --git a/packages/app/src/hooks/useChartConfig.tsx b/packages/app/src/hooks/useChartConfig.tsx index 26c1b1fd36..617e09566f 100644 --- a/packages/app/src/hooks/useChartConfig.tsx +++ b/packages/app/src/hooks/useChartConfig.tsx @@ -153,6 +153,21 @@ async function* fetchDataInChunks({ ? getGranularityAlignedTimeWindows(config) : [undefined]; + // Narrowing dateRange to a window must not change which top-N series the + // __hdx_series_limit CTE keeps, or the union across chunks would exceed + // seriesLimit — pin the ranking to the full chart range. + const fullDateRange = config.dateRange; + const seriesLimit = isBuilderChartConfig(config) + ? config.seriesLimit + : undefined; + const windowedConfigFor = (w: (typeof windows)[number]) => ({ + ...config, + ...(w ?? {}), + ...(w != null && seriesLimit != null && fullDateRange != null + ? { seriesLimitDateRange: fullDateRange } + : {}), + }); + if (IS_MTVIEWS_ENABLED && isBuilderChartConfig(config)) { const { dataTableDDL, mtViewDDL, renderMTViewConfig } = await buildMTViewSelectQuery(config, metadata, querySettings); @@ -172,10 +187,7 @@ async function* fetchDataInChunks({ if (enableParallelQueries) { // fetch in parallel const promises = windows.map(async (w, index) => { - const windowedConfig = { - ...config, - ...(w ?? {}), - }; + const windowedConfig = windowedConfigFor(w); return { index, queryResult: await clickhouseClient.queryChartConfig({ @@ -214,12 +226,7 @@ async function* fetchDataInChunks({ // fetch in series for (let i = 0; i < windows.length; i++) { - const window = windows[i]; - - const windowedConfig = { - ...config, - ...(window ?? {}), - }; + const windowedConfig = windowedConfigFor(windows[i]); const result = await clickhouseClient.queryChartConfig({ config: windowedConfig, diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index fc50abd854..55c9c679b4 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -461,4 +461,95 @@ describe('queryChartConfig Integration Tests', () => { }); } }); + + // Chunked fetches narrow dateRange per window; seriesLimitDateRange pins the + // top-N ranking to the full chart range so every chunk keeps the SAME group + // set — otherwise the union of per-window top-N sets exceeds the limit. + it('keeps a consistent top-N group set across chunked windows via seriesLimitDateRange', async () => { + const TABLE = 'logs_chunked_series_limit_int_test'; + await client.command({ + query: `CREATE OR REPLACE TABLE ${DATABASE}.${TABLE} ( + Timestamp DateTime CODEC(ZSTD(1)), + ServiceName String CODEC(ZSTD(1)) + ) ENGINE = MergeTree ORDER BY (ServiceName, Timestamp)`, + }); + + // Window 1 (00:00-00:30): svcA dominates. Window 2 (00:30-01:00): svcB + // dominates locally, but svcA's window-1 peak wins globally. + const rows = [ + ...Array.from({ length: 100 }, () => ({ + Timestamp: '2025-04-15 00:10:00', + ServiceName: 'svcA', + })), + { Timestamp: '2025-04-15 00:10:00', ServiceName: 'svcB' }, + { Timestamp: '2025-04-15 00:40:00', ServiceName: 'svcA' }, + ...Array.from({ length: 50 }, () => ({ + Timestamp: '2025-04-15 00:40:00', + ServiceName: 'svcB', + })), + ]; + await client.insert({ + table: `${DATABASE}.${TABLE}`, + values: rows, + format: 'JSONEachRow', + }); + + try { + const fullRange: [Date, Date] = [ + new Date('2025-04-15T00:00:00Z'), + new Date('2025-04-15T01:00:00Z'), + ]; + const windows: Array<{ + dateRange: [Date, Date]; + dateRangeEndInclusive: boolean; + }> = [ + { + dateRange: [new Date('2025-04-15T00:30:00Z'), fullRange[1]], + dateRangeEndInclusive: true, + }, + { + dateRange: [fullRange[0], new Date('2025-04-15T00:30:00Z')], + dateRangeEndInclusive: false, + }, + ]; + + const groupsPerWindow = await Promise.all( + windows.map(async window => { + const config: ChartConfigWithOptDateRange = { + displayType: DisplayType.Line, + connection: 'test-connection', + from: { databaseName: DATABASE, tableName: TABLE }, + select: [{ aggFn: 'count', aggCondition: '', valueExpression: '' }], + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + where: '', + whereLanguage: 'sql', + timestampValueExpression: 'Timestamp', + granularity: '5 minute', + seriesLimit: 1, + ...window, + seriesLimitDateRange: fullRange, + }; + const result = await hdxClient.queryChartConfig({ + config, + metadata, + querySettings: undefined, + }); + return new Set( + (result.data as Array<{ ServiceName: string }>).map( + r => r.ServiceName, + ), + ); + }), + ); + + // Both windows keep the global winner only — without the pinned range, + // window 2 would keep svcB (its local top-1) and the union would be 2. + expect(groupsPerWindow[0]).toEqual(new Set(['svcA'])); + expect(groupsPerWindow[1]).toEqual(new Set(['svcA'])); + } finally { + await client.command({ + query: `DROP TABLE IF EXISTS ${DATABASE}.${TABLE}`, + }); + } + }); }); diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 13a588900c..592c0744b1 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -510,6 +510,74 @@ describe('renderChartConfig', () => { await renderChartConfig(baseLogsConfig, mockMetadata, querySettings), ); expect(sql).not.toContain('__hdx_series_limit'); + + // seriesLimitDateRange alone (no seriesLimit) must not emit a CTE either. + const sqlWithRangeOnly = parameterizedQueryToSql( + await renderChartConfig( + { + ...baseLogsConfig, + seriesLimitDateRange: baseLogsConfig.dateRange, + }, + mockMetadata, + querySettings, + ), + ); + expect(sqlWithRangeOnly).not.toContain('__hdx_series_limit'); + }); + + it('pins the series-limit CTE to seriesLimitDateRange while the outer query stays windowed', async () => { + const fullRange: [Date, Date] = [ + new Date('2025-02-12T00:00:00Z'), + new Date('2025-02-13T00:00:00Z'), + ]; + const renderWindow = async ( + dateRange: [Date, Date], + dateRangeEndInclusive: boolean, + ) => + parameterizedQueryToSql( + await renderChartConfig( + { + ...baseLogsConfig, + seriesLimit: 60, + dateRange, + dateRangeEndInclusive, + seriesLimitDateRange: fullRange, + }, + mockMetadata, + querySettings, + ), + ); + + // Two chunked windows of the same chart (most recent window first, + // older windows are end-exclusive — mirrors fetchDataInChunks). + const recentChunk = await renderWindow( + [new Date('2025-02-12T18:00:00Z'), fullRange[1]], + true, + ); + const olderChunk = await renderWindow( + [fullRange[0], new Date('2025-02-12T18:00:00Z')], + false, + ); + + // The CTE end is the first `) SELECT` — the outer query starts there. + const cteOf = (sql: string) => { + const start = sql.indexOf('`__hdx_series_limit` AS ('); + const end = sql.indexOf(') SELECT '); + expect(start).toBeGreaterThanOrEqual(0); + expect(end).toBeGreaterThan(start); + return sql.slice(start, end); + }; + + // Both chunks rank over the identical full range, so they keep the + // same top-N set; the windowed range only applies to the outer query. + expect(cteOf(recentChunk)).toBe(cteOf(olderChunk)); + expect(cteOf(recentChunk)).toContain(String(fullRange[0].getTime())); + expect(cteOf(recentChunk)).toContain(String(fullRange[1].getTime())); + const outerOf = (sql: string) => sql.slice(sql.indexOf(') SELECT ')); + expect(outerOf(olderChunk)).toContain( + String(new Date('2025-02-12T18:00:00Z').getTime()), + ); + expect(outerOf(olderChunk)).not.toContain(String(fullRange[1].getTime())); }); it('does not emit a series-limit CTE without a group-by', async () => { diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index c25e985890..b7b35b366d 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -1235,6 +1235,26 @@ async function renderSeriesLimitCte( return undefined; } + // When the query was chunked into time windows, rank over the full chart + // range instead of the window — otherwise each chunk keeps its own top-N + // and the union across chunks exceeds N. Inclusivity is normalized so all + // chunks emit an identical CTE (non-first windows set + // dateRangeEndInclusive=false). + const cteConfig = chartConfig.seriesLimitDateRange + ? { + ...chartConfig, + dateRange: chartConfig.seriesLimitDateRange, + dateRangeStartInclusive: true, + dateRangeEndInclusive: true, + } + : undefined; + const cteWhere = cteConfig ? await renderWhere(cteConfig, metadata) : where; + // Re-rendered because timeBucketExpr derives the bucket size from dateRange + // when granularity is 'auto'. + const cteGroupBy = + (cteConfig ? await renderGroupBy(cteConfig, metadata) : undefined) ?? + groupBy; + // One ChSql per group-by column (groupBy may be an array or a comma-separated // string). splitAndTrimWithBracket respects []/()/quotes so it won't split // inside Map['a,b']; the per-column null filter below needs them separated. @@ -1275,8 +1295,8 @@ async function renderSeriesLimitCte( ' AND ', groupByCols.map(g => chSql`${g} IS NOT NULL`), ); - const innerWhere = where.sql - ? concatChSql(' AND ', where, groupByNotNullFilter) + const innerWhere = cteWhere.sql + ? concatChSql(' AND ', cteWhere, groupByNotNullFilter) : groupByNotNullFilter; // Per-(group, bucket) aggregate, then max per group, keeping the top N. @@ -1286,7 +1306,7 @@ async function renderSeriesLimitCte( SELECT tuple(${groupByTuple}) AS \`group\`, ${rankValue} AS \`__hdx_series_rank\` FROM ${from} WHERE ${innerWhere} - GROUP BY ${groupBy} + GROUP BY ${cteGroupBy} ) GROUP BY \`group\` ORDER BY max(\`__hdx_series_rank\`) DESC, \`group\` diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index dc38e71b2f..8cd56d2e26 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -1229,6 +1229,10 @@ export type DateRange = { dateRange: [Date, Date]; dateRangeStartInclusive?: boolean; // default true dateRangeEndInclusive?: boolean; // default true + // Runtime-only, set by query chunking when dateRange is narrowed to a + // window: the full chart range used by the `__hdx_series_limit` CTE so + // every chunk ranks (and keeps) the same top-N series. Never persisted. + seriesLimitDateRange?: [Date, Date]; }; export type ChartConfigWithDateRange = ChartConfig & DateRange; From 396549ffcc8927d1a39f8ec1207696c2da5c7344 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Thu, 11 Jun 2026 12:35:48 -0700 Subject: [PATCH 2/9] fix(charts): use the team series limit in the generated SQL preview buildChartConfigForExplanations called convertToTimeChartConfig without the team's seriesLimit, so the Generated SQL section always rendered the default LIMIT 100 even when the team setting was lower. Pass the same value DBTimeChart uses so the preview matches the executed query. --- .../DBEditTimeChartForm/EditTimeChartForm.tsx | 5 +++ .../__tests__/utils.test.ts | 32 +++++++++++++++++++ .../components/DBEditTimeChartForm/utils.ts | 7 +++- 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx index 574cce2a5b..26f794d0af 100644 --- a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx @@ -41,6 +41,7 @@ import { IconTable, } from '@tabler/icons-react'; +import api from '@/api'; import { getPreviousDateRange } from '@/ChartUtils'; import ChartDisplaySettingsDrawer, { ChartConfigDisplaySettings, @@ -514,6 +515,8 @@ export default function EditTimeChartForm({ }); }, [dateRange]); + const { data: me } = api.useMe(); + const chartConfigForExplanations = useMemo( () => buildChartConfigForExplanations({ @@ -524,6 +527,7 @@ export default function EditTimeChartForm({ dateRange, activeTab, dbTimeChartConfig, + teamSeriesLimit: me?.team?.seriesLimit, }), [ queriedConfig, @@ -533,6 +537,7 @@ export default function EditTimeChartForm({ dateRange, activeTab, dbTimeChartConfig, + me?.team?.seriesLimit, ], ); diff --git a/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts b/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts index 67f96b4aca..646ce55f4f 100644 --- a/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts +++ b/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts @@ -6,6 +6,7 @@ import { TSource, } from '@hyperdx/common-utils/dist/types'; +import { MAX_TIME_CHART_SERIES } from '@/ChartUtils'; import { ChartEditorFormState } from '@/components/ChartEditor/types'; import { @@ -411,6 +412,37 @@ describe('buildChartConfigForExplanations', () => { expect(result).toBeDefined(); }); + it('applies the team series limit so the SQL preview matches the chart query', () => { + const result = buildChartConfigForExplanations({ + ...baseParams, + queriedConfig: builderConfig, + queriedSourceId: logSource.id, + tableSource: logSource, + activeTab: 'time', + dbTimeChartConfig: builderConfig, + teamSeriesLimit: 3, + }); + + expect(result).toBeDefined(); + // @ts-expect-error union types.. + expect(result!.seriesLimit).toBe(3); + }); + + it('falls back to the default series limit when no team limit is set', () => { + const result = buildChartConfigForExplanations({ + ...baseParams, + queriedConfig: builderConfig, + queriedSourceId: logSource.id, + tableSource: logSource, + activeTab: 'time', + dbTimeChartConfig: builderConfig, + }); + + expect(result).toBeDefined(); + // @ts-expect-error union types.. + expect(result!.seriesLimit).toBe(MAX_TIME_CHART_SERIES); + }); + it.each(['table', 'number', 'pie'] as const)( 'uses queriedConfig for activeTab=%s and applies tab transform', activeTab => { diff --git a/packages/app/src/components/DBEditTimeChartForm/utils.ts b/packages/app/src/components/DBEditTimeChartForm/utils.ts index 883fb01cf0..c998ce51d1 100644 --- a/packages/app/src/components/DBEditTimeChartForm/utils.ts +++ b/packages/app/src/components/DBEditTimeChartForm/utils.ts @@ -184,6 +184,10 @@ type BuildChartConfigForExplanationsParams = { dateRange: [Date, Date]; activeTab: string; dbTimeChartConfig?: ChartConfigWithDateRange; + // The team's configured series cap — must match what DBTimeChart passes to + // convertToTimeChartConfig or the generated SQL preview shows the wrong + // seriesLimit. + teamSeriesLimit?: number; }; export function buildChartConfigForExplanations({ @@ -194,6 +198,7 @@ export function buildChartConfigForExplanations({ dateRange, activeTab, dbTimeChartConfig, + teamSeriesLimit, }: BuildChartConfigForExplanationsParams): | ChartConfigWithOptTimestamp | undefined { @@ -239,7 +244,7 @@ export function buildChartConfigForExplanations({ const builderConfig = config as BuilderChartConfigWithDateRange; if (activeTab === 'time') { - return convertToTimeChartConfig(builderConfig); + return convertToTimeChartConfig(builderConfig, teamSeriesLimit); } else if (activeTab === 'number') { return convertToNumberChartConfig(builderConfig); } else if (activeTab === 'table') { From cdd05a79d4ce574ddd28d49359c024acfb753e5c Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Thu, 11 Jun 2026 12:40:25 -0700 Subject: [PATCH 3/9] chore: add changeset for series-limit chunk consistency fix --- .changeset/fix-series-limit-chunk-consistency.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/fix-series-limit-chunk-consistency.md diff --git a/.changeset/fix-series-limit-chunk-consistency.md b/.changeset/fix-series-limit-chunk-consistency.md new file mode 100644 index 0000000000..3956d49e26 --- /dev/null +++ b/.changeset/fix-series-limit-chunk-consistency.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/common-utils': patch +'@hyperdx/app': patch +--- + +fix(charts): group-by time charts could render more series than the configured series limit because each time-window chunk ranked its own top-N; the ranking is now pinned to the full chart range so every chunk keeps the same series set. Also fixes the chart editor's "Generated SQL" preview, which always showed the default series limit of 100 instead of the team's configured value. From 14275a104110d6c005ef5a8e568bf82a0cb634a7 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Thu, 11 Jun 2026 12:43:17 -0700 Subject: [PATCH 4/9] refactor(charts): render series-limit CTE where/groupBy in parallel --- .../common-utils/src/core/renderChartConfig.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index b7b35b366d..6d3af301c2 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -1248,12 +1248,14 @@ async function renderSeriesLimitCte( dateRangeEndInclusive: true, } : undefined; - const cteWhere = cteConfig ? await renderWhere(cteConfig, metadata) : where; - // Re-rendered because timeBucketExpr derives the bucket size from dateRange - // when granularity is 'auto'. - const cteGroupBy = - (cteConfig ? await renderGroupBy(cteConfig, metadata) : undefined) ?? - groupBy; + // groupBy is re-rendered (not reused) because timeBucketExpr derives the + // bucket size from dateRange when granularity is 'auto'. + const [cteWhere = where, cteGroupBy = groupBy] = cteConfig + ? await Promise.all([ + renderWhere(cteConfig, metadata), + renderGroupBy(cteConfig, metadata), + ]) + : []; // One ChSql per group-by column (groupBy may be an array or a comma-separated // string). splitAndTrimWithBracket respects []/()/quotes so it won't split From eb983eb45081e17e516760efc0df22f42c898536 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Thu, 11 Jun 2026 13:27:19 -0700 Subject: [PATCH 5/9] perf(charts): rank the series-limit CTE over the newest chunk window Pinning the __hdx_series_limit ranking to the full chart range made every chunk re-scan the whole range. Rank over the newest window instead: still one fixed range shared by all chunks (so the top-N set stays consistent), but the scan is bounded by the smallest window. Trade-off: series are picked by recent activity, so groups with no events in the newest window are dropped from the chart. --- .../fix-series-limit-chunk-consistency.md | 2 +- .../hooks/__tests__/useChartConfig.test.tsx | 29 ++++++++++++------- packages/app/src/hooks/useChartConfig.tsx | 15 ++++++---- .../__tests__/queryChartConfig.int.test.ts | 29 ++++++++++--------- .../src/__tests__/renderChartConfig.test.ts | 20 ++++++++----- .../src/core/renderChartConfig.ts | 10 +++---- packages/common-utils/src/types.ts | 5 ++-- 7 files changed, 64 insertions(+), 46 deletions(-) diff --git a/.changeset/fix-series-limit-chunk-consistency.md b/.changeset/fix-series-limit-chunk-consistency.md index 3956d49e26..220a717583 100644 --- a/.changeset/fix-series-limit-chunk-consistency.md +++ b/.changeset/fix-series-limit-chunk-consistency.md @@ -3,4 +3,4 @@ '@hyperdx/app': patch --- -fix(charts): group-by time charts could render more series than the configured series limit because each time-window chunk ranked its own top-N; the ranking is now pinned to the full chart range so every chunk keeps the same series set. Also fixes the chart editor's "Generated SQL" preview, which always showed the default series limit of 100 instead of the team's configured value. +fix(charts): group-by time charts could render more series than the configured series limit because each time-window chunk ranked its own top-N; the ranking is now pinned to the newest chunk window so every chunk keeps the same series set. Also fixes the chart editor's "Generated SQL" preview, which always showed the default series limit of 100 instead of the team's configured value. diff --git a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx index 05bf82a53b..40981a1e7b 100644 --- a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx +++ b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx @@ -802,13 +802,12 @@ describe('useChartConfig', () => { expect(result.current.isPending).toBe(false); }); - it('pins the series-limit ranking to the full date range on each chunk', async () => { - const fullRange: [Date, Date] = [ - new Date('2025-10-01 00:00:00Z'), - new Date('2025-10-02 00:00:00Z'), - ]; + it('pins the series-limit ranking to the newest window on each chunk', async () => { const config = createMockChartConfig({ - dateRange: fullRange, + dateRange: [ + new Date('2025-10-01 00:00:00Z'), + new Date('2025-10-02 00:00:00Z'), + ], granularity: '3 hour', seriesLimit: 3, }); @@ -826,12 +825,16 @@ describe('useChartConfig', () => { await waitFor(() => expect(result.current.isFetching).toBe(false)); // Each chunk queries its own window but must rank top-N series over - // the full chart range, or the union across chunks exceeds the limit. + // the same fixed range, or the union across chunks exceeds the limit. + // The newest window (first 6h mock window) bounds the ranking scan. + const newestWindow: [Date, Date] = [ + new Date('2025-10-01T18:00:00.000Z'), + new Date('2025-10-02T00:00:00.000Z'), + ]; const calls = mockClickhouseClient.queryChartConfig.mock.calls; expect(calls).toHaveLength(3); for (const [{ config: windowed }] of calls) { - expect(windowed.seriesLimitDateRange).toEqual(fullRange); - expect(windowed.dateRange).not.toEqual(fullRange); + expect(windowed.seriesLimitDateRange).toEqual(newestWindow); } }); @@ -1386,7 +1389,7 @@ describe('useChartConfig', () => { }); }); - it('pins the series-limit ranking to the full date range with parallel queries', async () => { + it('pins the series-limit ranking to the newest window with parallel queries', async () => { const { config } = setupParallelQueries(); const configWithLimit = { ...config, seriesLimit: 3 }; @@ -1406,10 +1409,14 @@ describe('useChartConfig', () => { await waitFor(() => expect(result.current.isSuccess).toBe(true)); await waitFor(() => expect(result.current.isFetching).toBe(false)); + const newestWindow: [Date, Date] = [ + new Date('2025-10-01T18:00:00.000Z'), + new Date('2025-10-02T00:00:00.000Z'), + ]; const calls = mockClickhouseClient.queryChartConfig.mock.calls; expect(calls).toHaveLength(3); for (const [{ config: windowed }] of calls) { - expect(windowed.seriesLimitDateRange).toEqual(config.dateRange); + expect(windowed.seriesLimitDateRange).toEqual(newestWindow); } }); diff --git a/packages/app/src/hooks/useChartConfig.tsx b/packages/app/src/hooks/useChartConfig.tsx index 617e09566f..e2cb3c3c14 100644 --- a/packages/app/src/hooks/useChartConfig.tsx +++ b/packages/app/src/hooks/useChartConfig.tsx @@ -153,18 +153,21 @@ async function* fetchDataInChunks({ ? getGranularityAlignedTimeWindows(config) : [undefined]; - // Narrowing dateRange to a window must not change which top-N series the - // __hdx_series_limit CTE keeps, or the union across chunks would exceed - // seriesLimit — pin the ranking to the full chart range. - const fullDateRange = config.dateRange; + // Every chunk must rank the __hdx_series_limit CTE over the same fixed + // range, or each window keeps its own top-N and the union across chunks + // exceeds seriesLimit. The newest window is used (rather than the full + // chart range) to bound the ranking scan; the trade-off is that series + // are picked by recent activity, so groups with no events in the newest + // window are dropped from the chart. + const rankingDateRange = windows[0]?.dateRange; const seriesLimit = isBuilderChartConfig(config) ? config.seriesLimit : undefined; const windowedConfigFor = (w: (typeof windows)[number]) => ({ ...config, ...(w ?? {}), - ...(w != null && seriesLimit != null && fullDateRange != null - ? { seriesLimitDateRange: fullDateRange } + ...(w != null && seriesLimit != null && rankingDateRange != null + ? { seriesLimitDateRange: rankingDateRange } : {}), }); diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index 55c9c679b4..50015052e3 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -463,8 +463,9 @@ describe('queryChartConfig Integration Tests', () => { }); // Chunked fetches narrow dateRange per window; seriesLimitDateRange pins the - // top-N ranking to the full chart range so every chunk keeps the SAME group - // set — otherwise the union of per-window top-N sets exceeds the limit. + // top-N ranking to one shared range (the newest window) so every chunk keeps + // the SAME group set — otherwise the union of per-window top-N sets exceeds + // the limit. it('keeps a consistent top-N group set across chunked windows via seriesLimitDateRange', async () => { const TABLE = 'logs_chunked_series_limit_int_test'; await client.command({ @@ -474,8 +475,9 @@ describe('queryChartConfig Integration Tests', () => { ) ENGINE = MergeTree ORDER BY (ServiceName, Timestamp)`, }); - // Window 1 (00:00-00:30): svcA dominates. Window 2 (00:30-01:00): svcB - // dominates locally, but svcA's window-1 peak wins globally. + // Older window (00:00-00:30): svcA dominates. Newest window (00:30-01:00): + // svcB dominates — the ranking is pinned to the newest window, so svcB + // must win in BOTH chunks even though svcA's older peak is larger. const rows = [ ...Array.from({ length: 100 }, () => ({ Timestamp: '2025-04-15 00:10:00', @@ -495,8 +497,8 @@ describe('queryChartConfig Integration Tests', () => { }); try { - const fullRange: [Date, Date] = [ - new Date('2025-04-15T00:00:00Z'), + const newestWindow: [Date, Date] = [ + new Date('2025-04-15T00:30:00Z'), new Date('2025-04-15T01:00:00Z'), ]; const windows: Array<{ @@ -504,11 +506,11 @@ describe('queryChartConfig Integration Tests', () => { dateRangeEndInclusive: boolean; }> = [ { - dateRange: [new Date('2025-04-15T00:30:00Z'), fullRange[1]], + dateRange: newestWindow, dateRangeEndInclusive: true, }, { - dateRange: [fullRange[0], new Date('2025-04-15T00:30:00Z')], + dateRange: [new Date('2025-04-15T00:00:00Z'), newestWindow[0]], dateRangeEndInclusive: false, }, ]; @@ -527,7 +529,7 @@ describe('queryChartConfig Integration Tests', () => { granularity: '5 minute', seriesLimit: 1, ...window, - seriesLimitDateRange: fullRange, + seriesLimitDateRange: newestWindow, }; const result = await hdxClient.queryChartConfig({ config, @@ -542,10 +544,11 @@ describe('queryChartConfig Integration Tests', () => { }), ); - // Both windows keep the global winner only — without the pinned range, - // window 2 would keep svcB (its local top-1) and the union would be 2. - expect(groupsPerWindow[0]).toEqual(new Set(['svcA'])); - expect(groupsPerWindow[1]).toEqual(new Set(['svcA'])); + // Both windows keep the newest-window winner only — without the pinned + // range, the older window would keep svcA (its local top-1) and the + // union would be 2. + expect(groupsPerWindow[0]).toEqual(new Set(['svcB'])); + expect(groupsPerWindow[1]).toEqual(new Set(['svcB'])); } finally { await client.command({ query: `DROP TABLE IF EXISTS ${DATABASE}.${TABLE}`, diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 592c0744b1..2e7f42d875 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -526,7 +526,9 @@ describe('renderChartConfig', () => { }); it('pins the series-limit CTE to seriesLimitDateRange while the outer query stays windowed', async () => { - const fullRange: [Date, Date] = [ + // The chunking caller pins all chunks to one shared ranking range + // (the newest window); the render layer is agnostic to which range. + const rankingRange: [Date, Date] = [ new Date('2025-02-12T00:00:00Z'), new Date('2025-02-13T00:00:00Z'), ]; @@ -541,7 +543,7 @@ describe('renderChartConfig', () => { seriesLimit: 60, dateRange, dateRangeEndInclusive, - seriesLimitDateRange: fullRange, + seriesLimitDateRange: rankingRange, }, mockMetadata, querySettings, @@ -551,11 +553,11 @@ describe('renderChartConfig', () => { // Two chunked windows of the same chart (most recent window first, // older windows are end-exclusive — mirrors fetchDataInChunks). const recentChunk = await renderWindow( - [new Date('2025-02-12T18:00:00Z'), fullRange[1]], + [new Date('2025-02-12T18:00:00Z'), rankingRange[1]], true, ); const olderChunk = await renderWindow( - [fullRange[0], new Date('2025-02-12T18:00:00Z')], + [rankingRange[0], new Date('2025-02-12T18:00:00Z')], false, ); @@ -568,16 +570,18 @@ describe('renderChartConfig', () => { return sql.slice(start, end); }; - // Both chunks rank over the identical full range, so they keep the + // Both chunks rank over the identical pinned range, so they keep the // same top-N set; the windowed range only applies to the outer query. expect(cteOf(recentChunk)).toBe(cteOf(olderChunk)); - expect(cteOf(recentChunk)).toContain(String(fullRange[0].getTime())); - expect(cteOf(recentChunk)).toContain(String(fullRange[1].getTime())); + expect(cteOf(recentChunk)).toContain(String(rankingRange[0].getTime())); + expect(cteOf(recentChunk)).toContain(String(rankingRange[1].getTime())); const outerOf = (sql: string) => sql.slice(sql.indexOf(') SELECT ')); expect(outerOf(olderChunk)).toContain( String(new Date('2025-02-12T18:00:00Z').getTime()), ); - expect(outerOf(olderChunk)).not.toContain(String(fullRange[1].getTime())); + expect(outerOf(olderChunk)).not.toContain( + String(rankingRange[1].getTime()), + ); }); it('does not emit a series-limit CTE without a group-by', async () => { diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index 6d3af301c2..903bfd67ab 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -1235,11 +1235,11 @@ async function renderSeriesLimitCte( return undefined; } - // When the query was chunked into time windows, rank over the full chart - // range instead of the window — otherwise each chunk keeps its own top-N - // and the union across chunks exceeds N. Inclusivity is normalized so all - // chunks emit an identical CTE (non-first windows set - // dateRangeEndInclusive=false). + // When the query was chunked into time windows, rank over the shared + // range the caller pinned (the newest window) instead of each chunk's own + // window — otherwise each chunk keeps its own top-N and the union across + // chunks exceeds N. Inclusivity is normalized so all chunks emit an + // identical CTE (non-first windows set dateRangeEndInclusive=false). const cteConfig = chartConfig.seriesLimitDateRange ? { ...chartConfig, diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 8cd56d2e26..514983635f 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -1230,8 +1230,9 @@ export type DateRange = { dateRangeStartInclusive?: boolean; // default true dateRangeEndInclusive?: boolean; // default true // Runtime-only, set by query chunking when dateRange is narrowed to a - // window: the full chart range used by the `__hdx_series_limit` CTE so - // every chunk ranks (and keeps) the same top-N series. Never persisted. + // window: a fixed ranking range (the newest chunk window) used by the + // `__hdx_series_limit` CTE so every chunk ranks (and keeps) the same + // top-N series. Never persisted. seriesLimitDateRange?: [Date, Date]; }; From 0e33b0d64e849b1fb43a557451b0eb9f2f5f28d2 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Thu, 11 Jun 2026 14:40:20 -0700 Subject: [PATCH 6/9] feat(team): make the time-chart series limit opt-in The team seriesLimit setting no longer falls back to 100: when unset, charts fetch every series and no __hdx_series_limit CTE is emitted. The team settings page shows the setting as "Disabled" by default and adds a Disable button to clear a configured value back to undefined. --- .../fix-series-limit-chunk-consistency.md | 2 +- packages/app/src/ChartUtils.tsx | 7 ++- packages/app/src/__tests__/ChartUtils.test.ts | 5 +- .../__tests__/utils.test.ts | 5 +- .../TeamSettings/TeamQueryConfigSection.tsx | 52 +++++++++++++------ 5 files changed, 45 insertions(+), 26 deletions(-) diff --git a/.changeset/fix-series-limit-chunk-consistency.md b/.changeset/fix-series-limit-chunk-consistency.md index 220a717583..b0655c85a5 100644 --- a/.changeset/fix-series-limit-chunk-consistency.md +++ b/.changeset/fix-series-limit-chunk-consistency.md @@ -3,4 +3,4 @@ '@hyperdx/app': patch --- -fix(charts): group-by time charts could render more series than the configured series limit because each time-window chunk ranked its own top-N; the ranking is now pinned to the newest chunk window so every chunk keeps the same series set. Also fixes the chart editor's "Generated SQL" preview, which always showed the default series limit of 100 instead of the team's configured value. +fix(charts): group-by time charts could render more series than the configured series limit because each time-window chunk ranked its own top-N; the ranking is now pinned to the newest chunk window so every chunk keeps the same series set. The team "Time Chart Series Limit" setting is now opt-in: it defaults to disabled (charts fetch every series, no limit CTE) and can be disabled again from the team settings page. Also fixes the chart editor's "Generated SQL" preview, which always showed a series limit of 100 instead of the team's configured value. diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index b284670182..2d1eca34a2 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -110,7 +110,10 @@ export function convertToTimeChartConfig( config: ChartConfigWithDateRange, teamSeriesLimit?: number, ): ChartConfigWithDateRange { - const seriesLimit = Math.max(1, teamSeriesLimit ?? MAX_TIME_CHART_SERIES); + // Series capping is opt-in via the team setting; when unset, no + // __hdx_series_limit CTE is emitted and charts fetch every series. + const seriesLimit = + teamSeriesLimit != null ? Math.max(1, teamSeriesLimit) : undefined; const granularity = getTimeChartGranularity( config.granularity, @@ -139,7 +142,7 @@ export function convertToTimeChartConfig( dateRangeEndInclusive, granularity, limit: { limit: 100000 }, - seriesLimit, + ...(seriesLimit != null ? { seriesLimit } : {}), } : { ...config, diff --git a/packages/app/src/__tests__/ChartUtils.test.ts b/packages/app/src/__tests__/ChartUtils.test.ts index ab9acf58e7..9a573103d0 100644 --- a/packages/app/src/__tests__/ChartUtils.test.ts +++ b/packages/app/src/__tests__/ChartUtils.test.ts @@ -11,7 +11,6 @@ import { formatResponseForPieChart, formatResponseForTimeChart, } from '@/ChartUtils'; -import { DEFAULT_SERIES_LIMIT } from '@/defaults'; import { COLORS } from '@/utils'; // Anchor info/error to concrete hexes rather than `getChartColorInfo()` / @@ -826,8 +825,8 @@ describe('ChartUtils', () => { ) as BuilderChartConfigWithDateRange ).seriesLimit; - it('defaults seriesLimit to DEFAULT_SERIES_LIMIT when no team value is given', () => { - expect(seriesLimitOf()).toBe(DEFAULT_SERIES_LIMIT); + it('omits seriesLimit (capping disabled) when no team value is given', () => { + expect(seriesLimitOf()).toBeUndefined(); }); it('uses the team seriesLimit when provided', () => { diff --git a/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts b/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts index 646ce55f4f..8b58abe468 100644 --- a/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts +++ b/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts @@ -6,7 +6,6 @@ import { TSource, } from '@hyperdx/common-utils/dist/types'; -import { MAX_TIME_CHART_SERIES } from '@/ChartUtils'; import { ChartEditorFormState } from '@/components/ChartEditor/types'; import { @@ -428,7 +427,7 @@ describe('buildChartConfigForExplanations', () => { expect(result!.seriesLimit).toBe(3); }); - it('falls back to the default series limit when no team limit is set', () => { + it('omits seriesLimit (capping disabled) when no team limit is set', () => { const result = buildChartConfigForExplanations({ ...baseParams, queriedConfig: builderConfig, @@ -440,7 +439,7 @@ describe('buildChartConfigForExplanations', () => { expect(result).toBeDefined(); // @ts-expect-error union types.. - expect(result!.seriesLimit).toBe(MAX_TIME_CHART_SERIES); + expect(result!.seriesLimit).toBeUndefined(); }); it.each(['table', 'number', 'pie'] as const)( diff --git a/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx b/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx index 2cfc66c9b4..f5535bf484 100644 --- a/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx +++ b/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx @@ -41,6 +41,11 @@ interface ClickhouseSettingFormProps { max?: number; displayValue?: (value: any, defaultValue?: any) => string; description?: string; + // For settings with no server-side default: lets the user clear the value + // back to undefined (which disables the feature) instead of resetting to + // a defaultValue. + allowUnset?: boolean; + resetLabel?: string; } function getFieldErrorMessage(error: unknown): string | undefined { @@ -63,6 +68,8 @@ function ClickhouseSettingForm({ max, displayValue, description, + allowUnset = false, + resetLabel = 'Reset to default', }: ClickhouseSettingFormProps) { const { data: me, refetch: refetchMe } = api.useMe(); const updateClickhouseSettings = api.useUpdateClickhouseSettings(); @@ -124,7 +131,7 @@ function ClickhouseSettingForm({ ); const handleReset = useCallback(() => { - if (defaultValue == null) return; + if (defaultValue == null && !allowUnset) return; updateClickhouseSettings.mutate( { [settingKey]: null }, { @@ -137,9 +144,12 @@ function ClickhouseSettingForm({ onSuccess: () => { notifications.show({ color: 'green', - message: `Reset ${label} to default`, + message: + defaultValue != null + ? `Reset ${label} to default` + : `Updated ${label}`, }); - form.reset({ value: defaultValue }); + form.reset({ value: defaultValue ?? '' }); refetchMe(); setIsEditing(false); }, @@ -151,6 +161,7 @@ function ClickhouseSettingForm({ settingKey, label, defaultValue, + allowUnset, form, ]); @@ -255,16 +266,18 @@ function ClickhouseSettingForm({ Change )} - {hasAdminAccess && isCustomValue && defaultValue != null && ( - - )} + {hasAdminAccess && + isCustomValue && + (defaultValue != null || allowUnset) && ( + + )} )} @@ -347,12 +360,17 @@ export default function TeamQueryConfigSection() { + value == null + ? 'Disabled' + : `${Number(value).toLocaleString()} series` + } + allowUnset + resetLabel="Disable" /> From e40772c4cbd6c2aed75889ee2adb48be097d4373 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Thu, 11 Jun 2026 14:44:47 -0700 Subject: [PATCH 7/9] chore: update changeset for opt-in series limit --- .changeset/fix-series-limit-chunk-consistency.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/fix-series-limit-chunk-consistency.md b/.changeset/fix-series-limit-chunk-consistency.md index b0655c85a5..471be9200a 100644 --- a/.changeset/fix-series-limit-chunk-consistency.md +++ b/.changeset/fix-series-limit-chunk-consistency.md @@ -3,4 +3,4 @@ '@hyperdx/app': patch --- -fix(charts): group-by time charts could render more series than the configured series limit because each time-window chunk ranked its own top-N; the ranking is now pinned to the newest chunk window so every chunk keeps the same series set. The team "Time Chart Series Limit" setting is now opt-in: it defaults to disabled (charts fetch every series, no limit CTE) and can be disabled again from the team settings page. Also fixes the chart editor's "Generated SQL" preview, which always showed a series limit of 100 instead of the team's configured value. +feat(charts): the team "Time Chart Series Limit" setting is now opt-in — it defaults to disabled (charts fetch every series, no limit CTE) and a configured value can be cleared back to disabled from the team settings page. When a limit is set, chunked time-chart queries now keep a consistent top-N series set: previously each time-window chunk ranked its own top-N, so charts could render more series than the limit and adjacent windows disagreed; the ranking is now pinned to the newest chunk window for every chunk. The chart editor's "Generated SQL" preview also reflects the team's configured limit instead of always showing 100. From 74caf21cd0e373f4917cea9c290b04eb04f1e680 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Mon, 15 Jun 2026 11:22:57 -0700 Subject: [PATCH 8/9] feat(charts): move time-chart series limit from team setting to per-tile Display Settings The series limit is now configured per chart in the Display Settings drawer instead of as a workspace-wide team setting. The value already persisted on the chart config, so convertToTimeChartConfig now reads config.seriesLimit directly; the team seriesLimit field, its ClickHouse settings schema, and the Team Settings UI control are removed. The chunk-consistency behavior (seriesLimitDateRange pinned to the newest window) is unchanged and now driven per tile. HDX-4499 --- .../fix-series-limit-chunk-consistency.md | 2 +- packages/api/src/models/team.ts | 1 - packages/app/src/ChartUtils.tsx | 12 +-- packages/app/src/__tests__/ChartUtils.test.ts | 31 +++---- .../components/ChartDisplaySettingsDrawer.tsx | 32 +++++++ .../DBEditTimeChartForm/EditTimeChartForm.tsx | 11 +-- .../__tests__/utils.test.ts | 10 ++- .../components/DBEditTimeChartForm/utils.ts | 7 +- packages/app/src/components/DBTimeChart.tsx | 4 +- .../src/components/SearchTotalCountChart.tsx | 4 +- .../TeamSettings/TeamQueryConfigSection.tsx | 55 +++--------- .../ChartDisplaySettingsDrawer.test.tsx | 85 +++++++++++++++++++ packages/common-utils/src/types.ts | 2 - 13 files changed, 169 insertions(+), 87 deletions(-) diff --git a/.changeset/fix-series-limit-chunk-consistency.md b/.changeset/fix-series-limit-chunk-consistency.md index 471be9200a..255666fc5a 100644 --- a/.changeset/fix-series-limit-chunk-consistency.md +++ b/.changeset/fix-series-limit-chunk-consistency.md @@ -3,4 +3,4 @@ '@hyperdx/app': patch --- -feat(charts): the team "Time Chart Series Limit" setting is now opt-in — it defaults to disabled (charts fetch every series, no limit CTE) and a configured value can be cleared back to disabled from the team settings page. When a limit is set, chunked time-chart queries now keep a consistent top-N series set: previously each time-window chunk ranked its own top-N, so charts could render more series than the limit and adjacent windows disagreed; the ranking is now pinned to the newest chunk window for every chunk. The chart editor's "Generated SQL" preview also reflects the team's configured limit instead of always showing 100. +feat(charts): the time-chart series limit is now configured per chart in the Display Settings drawer instead of as a workspace-wide team setting (the team "Time Chart Series Limit" setting is removed). It is disabled by default — charts fetch every series and no `__hdx_series_limit` CTE is emitted — and is cleared back to disabled by emptying the field. The control only appears for builder line/bar charts; the limit and its Generated SQL preview now come from the chart's own config. When a limit is set, chunked time-chart queries keep a consistent top-N series set: previously each time-window chunk ranked its own top-N, so charts could render more series than the limit and adjacent windows disagreed; the ranking is now pinned to the newest chunk window for every chunk so the union across chunks equals the limit. diff --git a/packages/api/src/models/team.ts b/packages/api/src/models/team.ts index 15e92ec72c..4d6b620162 100644 --- a/packages/api/src/models/team.ts +++ b/packages/api/src/models/team.ts @@ -42,7 +42,6 @@ export default mongoose.model( fieldMetadataDisabled: Boolean, parallelizeWhenPossible: Boolean, filterKeysFetchLimit: Number, - seriesLimit: Number, }, { timestamps: true, diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index 2d1eca34a2..86bdaf169f 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -108,12 +108,14 @@ export const MAX_TIME_CHART_SERIES = DEFAULT_SERIES_LIMIT; export function convertToTimeChartConfig( config: ChartConfigWithDateRange, - teamSeriesLimit?: number, ): ChartConfigWithDateRange { - // Series capping is opt-in via the team setting; when unset, no - // __hdx_series_limit CTE is emitted and charts fetch every series. - const seriesLimit = - teamSeriesLimit != null ? Math.max(1, teamSeriesLimit) : undefined; + // Series capping is opt-in per tile via the chart's Display Settings; when + // unset, no __hdx_series_limit CTE is emitted and every series is fetched. + const seriesLimit = isBuilderChartConfig(config) + ? config.seriesLimit != null + ? Math.max(1, config.seriesLimit) + : undefined + : undefined; const granularity = getTimeChartGranularity( config.granularity, diff --git a/packages/app/src/__tests__/ChartUtils.test.ts b/packages/app/src/__tests__/ChartUtils.test.ts index 9a573103d0..74dd619dff 100644 --- a/packages/app/src/__tests__/ChartUtils.test.ts +++ b/packages/app/src/__tests__/ChartUtils.test.ts @@ -807,33 +807,30 @@ describe('ChartUtils', () => { expect(granularityFromFunction).toBe('5 minute'); }); - const seriesLimitConfig = { - granularity: '5 minute', - dateRange: [ - new Date('2025-11-26T00:00:00Z'), - new Date('2025-11-27T00:00:00Z'), - ], - } as BuilderChartConfigWithDateRange; - // seriesLimit lives on the builder member of the ChartConfigWithDateRange - // union, so narrow the result before reading it. - const seriesLimitOf = (teamSeriesLimit?: number) => + // union, so narrow the result before reading it. The per-tile value is + // read from the config itself (no team override anymore). + const seriesLimitOf = (seriesLimit?: number) => ( - convertToTimeChartConfig( - seriesLimitConfig, - teamSeriesLimit, - ) as BuilderChartConfigWithDateRange + convertToTimeChartConfig({ + granularity: '5 minute', + dateRange: [ + new Date('2025-11-26T00:00:00Z'), + new Date('2025-11-27T00:00:00Z'), + ], + ...(seriesLimit != null ? { seriesLimit } : {}), + } as BuilderChartConfigWithDateRange) as BuilderChartConfigWithDateRange ).seriesLimit; - it('omits seriesLimit (capping disabled) when no team value is given', () => { + it('omits seriesLimit (capping disabled) when the tile has no limit', () => { expect(seriesLimitOf()).toBeUndefined(); }); - it('uses the team seriesLimit when provided', () => { + it('uses the tile seriesLimit when provided', () => { expect(seriesLimitOf(5)).toBe(5); }); - it('passes a large team seriesLimit through unbounded', () => { + it('passes a large tile seriesLimit through unbounded', () => { expect(seriesLimitOf(100000)).toBe(100000); }); }); diff --git a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx index 04660d7be1..330266a7ac 100644 --- a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx +++ b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx @@ -13,11 +13,13 @@ import { Divider, Drawer, Group, + NumberInput, Stack, Text, } from '@mantine/core'; import { shouldFillNullsWithZero } from '@/ChartUtils'; +import { DEFAULT_SERIES_LIMIT } from '@/defaults'; import { FormatTime } from '@/useFormatTime'; import { @@ -41,6 +43,9 @@ export type ChartConfigDisplaySettings = Pick< | 'colorRules' > & { groupByColumnsOnLeft?: boolean; + // Per-tile cap on the number of series fetched for a group-by time chart. + // Undefined = disabled (no __hdx_series_limit CTE; every series is fetched). + seriesLimit?: number; }; /** @@ -81,6 +86,7 @@ function applyDefaultSettings( compareToPreviousPeriod: settings.compareToPreviousPeriod ?? false, fitYAxisToData: settings.fitYAxisToData ?? false, groupByColumnsOnLeft: settings.groupByColumnsOnLeft ?? false, + seriesLimit: settings.seriesLimit, color: settings.color, colorRules: settings.colorRules ? attachLocalIds(settings.colorRules) @@ -144,6 +150,10 @@ export default function ChartDisplaySettingsDrawer({ const isTimeChart = displayType === DisplayType.Line || displayType === DisplayType.StackedBar; + // The series-limit CTE is only emitted for builder group-by time charts; + // raw SQL configs author their own LIMIT logic directly. + const showSeriesLimit = isTimeChart && configType !== 'sql'; + // Group By column ordering only applies to builder table charts; raw SQL // configs let the user author whatever column order they want directly. const showGroupByColumnsOnLeft = @@ -203,6 +213,28 @@ export default function ChartDisplaySettingsDrawer({ label="Fit Y-Axis to Data" description="Start the y-axis at the minimum of the displayed data instead of zero. Only applicable to line charts." /> + {showSeriesLimit && ( + + ( + + onChange(v === '' || v == null ? undefined : Number(v)) + } + /> + )} + /> + + )} )} diff --git a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx index 26f794d0af..e3f0970dee 100644 --- a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx @@ -41,7 +41,6 @@ import { IconTable, } from '@tabler/icons-react'; -import api from '@/api'; import { getPreviousDateRange } from '@/ChartUtils'; import ChartDisplaySettingsDrawer, { ChartConfigDisplaySettings, @@ -225,6 +224,7 @@ export default function EditTimeChartForm({ fitYAxisToData, numberFormat, groupByColumnsOnLeft, + seriesLimit, color, colorRules, ] = useWatch({ @@ -236,6 +236,7 @@ export default function EditTimeChartForm({ 'fitYAxisToData', 'numberFormat', 'groupByColumnsOnLeft', + 'seriesLimit', 'color', 'colorRules', ], @@ -258,6 +259,7 @@ export default function EditTimeChartForm({ fitYAxisToData, numberFormat, groupByColumnsOnLeft, + seriesLimit, color, colorRules, }), @@ -268,6 +270,7 @@ export default function EditTimeChartForm({ fitYAxisToData, numberFormat, groupByColumnsOnLeft, + seriesLimit, color, colorRules, ], @@ -515,8 +518,6 @@ export default function EditTimeChartForm({ }); }, [dateRange]); - const { data: me } = api.useMe(); - const chartConfigForExplanations = useMemo( () => buildChartConfigForExplanations({ @@ -527,7 +528,6 @@ export default function EditTimeChartForm({ dateRange, activeTab, dbTimeChartConfig, - teamSeriesLimit: me?.team?.seriesLimit, }), [ queriedConfig, @@ -537,7 +537,6 @@ export default function EditTimeChartForm({ dateRange, activeTab, dbTimeChartConfig, - me?.team?.seriesLimit, ], ); @@ -554,6 +553,7 @@ export default function EditTimeChartForm({ compareToPreviousPeriod, fitYAxisToData, groupByColumnsOnLeft, + seriesLimit, color, colorRules, }: ChartConfigDisplaySettings) => { @@ -563,6 +563,7 @@ export default function EditTimeChartForm({ setValue('compareToPreviousPeriod', compareToPreviousPeriod); setValue('fitYAxisToData', fitYAxisToData); setValue('groupByColumnsOnLeft', groupByColumnsOnLeft); + setValue('seriesLimit', seriesLimit); setValue('color', color); setValue('colorRules', colorRules); onSubmit(); diff --git a/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts b/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts index 8b58abe468..ef8f02a269 100644 --- a/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts +++ b/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts @@ -411,15 +411,17 @@ describe('buildChartConfigForExplanations', () => { expect(result).toBeDefined(); }); - it('applies the team series limit so the SQL preview matches the chart query', () => { + it("applies the tile's series limit so the SQL preview matches the chart query", () => { const result = buildChartConfigForExplanations({ ...baseParams, queriedConfig: builderConfig, queriedSourceId: logSource.id, tableSource: logSource, activeTab: 'time', - dbTimeChartConfig: builderConfig, - teamSeriesLimit: 3, + dbTimeChartConfig: { + ...builderConfig, + seriesLimit: 3, + } as ChartConfigWithDateRange, }); expect(result).toBeDefined(); @@ -427,7 +429,7 @@ describe('buildChartConfigForExplanations', () => { expect(result!.seriesLimit).toBe(3); }); - it('omits seriesLimit (capping disabled) when no team limit is set', () => { + it('omits seriesLimit (capping disabled) when the tile has no limit', () => { const result = buildChartConfigForExplanations({ ...baseParams, queriedConfig: builderConfig, diff --git a/packages/app/src/components/DBEditTimeChartForm/utils.ts b/packages/app/src/components/DBEditTimeChartForm/utils.ts index c998ce51d1..883fb01cf0 100644 --- a/packages/app/src/components/DBEditTimeChartForm/utils.ts +++ b/packages/app/src/components/DBEditTimeChartForm/utils.ts @@ -184,10 +184,6 @@ type BuildChartConfigForExplanationsParams = { dateRange: [Date, Date]; activeTab: string; dbTimeChartConfig?: ChartConfigWithDateRange; - // The team's configured series cap — must match what DBTimeChart passes to - // convertToTimeChartConfig or the generated SQL preview shows the wrong - // seriesLimit. - teamSeriesLimit?: number; }; export function buildChartConfigForExplanations({ @@ -198,7 +194,6 @@ export function buildChartConfigForExplanations({ dateRange, activeTab, dbTimeChartConfig, - teamSeriesLimit, }: BuildChartConfigForExplanationsParams): | ChartConfigWithOptTimestamp | undefined { @@ -244,7 +239,7 @@ export function buildChartConfigForExplanations({ const builderConfig = config as BuilderChartConfigWithDateRange; if (activeTab === 'time') { - return convertToTimeChartConfig(builderConfig, teamSeriesLimit); + return convertToTimeChartConfig(builderConfig); } else if (activeTab === 'number') { return convertToNumberChartConfig(builderConfig); } else if (activeTab === 'table') { diff --git a/packages/app/src/components/DBTimeChart.tsx b/packages/app/src/components/DBTimeChart.tsx index 9ae6f1a54c..6783eb8b6e 100644 --- a/packages/app/src/components/DBTimeChart.tsx +++ b/packages/app/src/components/DBTimeChart.tsx @@ -290,8 +290,8 @@ function DBTimeChartComponent({ const { data: me, isLoading: isLoadingMe } = api.useMe(); const queriedConfig = useMemo( - () => convertToTimeChartConfig(config, me?.team?.seriesLimit), - [config, me?.team?.seriesLimit], + () => convertToTimeChartConfig(config), + [config], ); // Determine whether the config can be optimized with an MV, to determine whether diff --git a/packages/app/src/components/SearchTotalCountChart.tsx b/packages/app/src/components/SearchTotalCountChart.tsx index a6be03f89d..ec42f131fa 100644 --- a/packages/app/src/components/SearchTotalCountChart.tsx +++ b/packages/app/src/components/SearchTotalCountChart.tsx @@ -39,8 +39,8 @@ export function useSearchTotalCount( // queriedConfig, queryKey, and enableQueryChunking match DBTimeChart so that react query can de-dupe these queries. const queriedConfig = useMemo( - () => convertToTimeChartConfig(config, me?.team?.seriesLimit), - [config, me?.team?.seriesLimit], + () => convertToTimeChartConfig(config), + [config], ); const { data: totalCountData, diff --git a/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx b/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx index f5535bf484..de850d2d6f 100644 --- a/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx +++ b/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx @@ -24,7 +24,6 @@ import { DEFAULT_FILTER_KEYS_FETCH_LIMIT_WITH_MVS, DEFAULT_QUERY_TIMEOUT, DEFAULT_SEARCH_ROW_LIMIT, - DEFAULT_SERIES_LIMIT, } from '@/defaults'; import { useBrandDisplayName } from '@/theme/ThemeProvider'; @@ -41,11 +40,6 @@ interface ClickhouseSettingFormProps { max?: number; displayValue?: (value: any, defaultValue?: any) => string; description?: string; - // For settings with no server-side default: lets the user clear the value - // back to undefined (which disables the feature) instead of resetting to - // a defaultValue. - allowUnset?: boolean; - resetLabel?: string; } function getFieldErrorMessage(error: unknown): string | undefined { @@ -68,8 +62,6 @@ function ClickhouseSettingForm({ max, displayValue, description, - allowUnset = false, - resetLabel = 'Reset to default', }: ClickhouseSettingFormProps) { const { data: me, refetch: refetchMe } = api.useMe(); const updateClickhouseSettings = api.useUpdateClickhouseSettings(); @@ -131,7 +123,7 @@ function ClickhouseSettingForm({ ); const handleReset = useCallback(() => { - if (defaultValue == null && !allowUnset) return; + if (defaultValue == null) return; updateClickhouseSettings.mutate( { [settingKey]: null }, { @@ -144,12 +136,9 @@ function ClickhouseSettingForm({ onSuccess: () => { notifications.show({ color: 'green', - message: - defaultValue != null - ? `Reset ${label} to default` - : `Updated ${label}`, + message: `Reset ${label} to default`, }); - form.reset({ value: defaultValue ?? '' }); + form.reset({ value: defaultValue }); refetchMe(); setIsEditing(false); }, @@ -161,7 +150,6 @@ function ClickhouseSettingForm({ settingKey, label, defaultValue, - allowUnset, form, ]); @@ -266,18 +254,16 @@ function ClickhouseSettingForm({ Change )} - {hasAdminAccess && - isCustomValue && - (defaultValue != null || allowUnset) && ( - - )} + {hasAdminAccess && isCustomValue && defaultValue != null && ( + + )} )} @@ -357,21 +343,6 @@ export default function TeamQueryConfigSection() { type="boolean" displayValue={value => (value ? 'Enabled' : 'Disabled')} /> - - value == null - ? 'Disabled' - : `${Number(value).toLocaleString()} series` - } - allowUnset - resetLabel="Disable" - /> diff --git a/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx b/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx index ec6b681bcd..d44cabf59c 100644 --- a/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx +++ b/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx @@ -175,4 +175,89 @@ describe('ChartDisplaySettingsDrawer', () => { }); }); }); + + describe('series limit setting', () => { + const builderProps = { ...baseProps, configType: 'builder' as const }; + + it('shows the Series Limit input for builder line charts', () => { + renderWithMantine( + , + ); + + expect( + screen.getByRole('textbox', { name: /series limit/i }), + ).toBeInTheDocument(); + }); + + it('does not show the Series Limit input for raw SQL line charts', () => { + renderWithMantine( + , + ); + + expect( + screen.queryByRole('textbox', { name: /series limit/i }), + ).not.toBeInTheDocument(); + }); + + it('does not show the Series Limit input for table charts', () => { + renderWithMantine( + , + ); + + expect( + screen.queryByRole('textbox', { name: /series limit/i }), + ).not.toBeInTheDocument(); + }); + + it('calls onChange with the entered seriesLimit when applied', async () => { + const onChange = jest.fn(); + const user = userEvent.setup(); + + renderWithMantine( + , + ); + + await user.type( + screen.getByRole('textbox', { name: /series limit/i }), + '25', + ); + await user.click(screen.getByRole('button', { name: /apply/i })); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange.mock.calls[0][0]).toMatchObject({ seriesLimit: 25 }); + }); + + it('clears seriesLimit to undefined (disabled) when emptied', async () => { + const onChange = jest.fn(); + const user = userEvent.setup(); + + renderWithMantine( + , + ); + + await user.clear(screen.getByRole('textbox', { name: /series limit/i })); + await user.click(screen.getByRole('button', { name: /apply/i })); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange.mock.calls[0][0].seriesLimit).toBeUndefined(); + }); + }); }); diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 514983635f..89d546b22c 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -1528,7 +1528,6 @@ export const TeamClickHouseSettingsSchema = z.object({ metadataMaxRowsToRead: z.number().optional(), parallelizeWhenPossible: z.boolean().optional(), filterKeysFetchLimit: z.number().optional(), - seriesLimit: z.number().int().positive().optional(), }); /** Accepts null to unset (reset to default) a setting. */ @@ -1539,7 +1538,6 @@ export const TeamClickHouseSettingsUpdateSchema = z.object({ metadataMaxRowsToRead: z.number().nullish(), parallelizeWhenPossible: z.boolean().nullish(), filterKeysFetchLimit: z.number().nullish(), - seriesLimit: z.number().int().positive().nullish(), }); export type TeamClickHouseSettingsUpdate = z.infer< typeof TeamClickHouseSettingsUpdateSchema From 92f80c28b31555eb8db7d7d0c76aab4b5f439ad0 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Mon, 15 Jun 2026 13:32:59 -0700 Subject: [PATCH 9/9] fix(charts): persist cleared series limit as null so it stays disabled Clearing the per-chart Series Limit in Display Settings updated the chart query but reappeared when the drawer was reopened. In Chart Explorer the config round-trips through the URL query state (JSON), which drops keys with undefined values; react-hook-form's `values` prop then leaves the stale field in place because the key is absent. Clear to null instead, which survives JSON serialization and forces RHF to reset the field. seriesLimit is now nullish in the schema, and convertToTimeChartConfig normalizes a cleared null back to undefined (no CTE). HDX-4499 --- packages/app/src/ChartUtils.tsx | 4 +++- packages/app/src/__tests__/ChartUtils.test.ts | 8 ++++++-- .../src/components/ChartDisplaySettingsDrawer.tsx | 12 ++++++++---- .../DBEditTimeChartForm/EditTimeChartForm.tsx | 5 ++++- .../__tests__/ChartDisplaySettingsDrawer.test.tsx | 7 +++++-- packages/common-utils/src/types.ts | 5 ++++- 6 files changed, 30 insertions(+), 11 deletions(-) diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index 86bdaf169f..b339e64650 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -144,7 +144,9 @@ export function convertToTimeChartConfig( dateRangeEndInclusive, granularity, limit: { limit: 100000 }, - ...(seriesLimit != null ? { seriesLimit } : {}), + // Overwrite (not conditionally spread) so a cleared `null` from the + // source config is normalized to undefined rather than carried over. + seriesLimit, } : { ...config, diff --git a/packages/app/src/__tests__/ChartUtils.test.ts b/packages/app/src/__tests__/ChartUtils.test.ts index 74dd619dff..400c0ca6d8 100644 --- a/packages/app/src/__tests__/ChartUtils.test.ts +++ b/packages/app/src/__tests__/ChartUtils.test.ts @@ -810,7 +810,7 @@ describe('ChartUtils', () => { // seriesLimit lives on the builder member of the ChartConfigWithDateRange // union, so narrow the result before reading it. The per-tile value is // read from the config itself (no team override anymore). - const seriesLimitOf = (seriesLimit?: number) => + const seriesLimitOf = (seriesLimit?: number | null) => ( convertToTimeChartConfig({ granularity: '5 minute', @@ -818,7 +818,7 @@ describe('ChartUtils', () => { new Date('2025-11-26T00:00:00Z'), new Date('2025-11-27T00:00:00Z'), ], - ...(seriesLimit != null ? { seriesLimit } : {}), + ...(seriesLimit !== undefined ? { seriesLimit } : {}), } as BuilderChartConfigWithDateRange) as BuilderChartConfigWithDateRange ).seriesLimit; @@ -826,6 +826,10 @@ describe('ChartUtils', () => { expect(seriesLimitOf()).toBeUndefined(); }); + it('normalizes a cleared (null) seriesLimit to undefined (disabled)', () => { + expect(seriesLimitOf(null)).toBeUndefined(); + }); + it('uses the tile seriesLimit when provided', () => { expect(seriesLimitOf(5)).toBe(5); }); diff --git a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx index 330266a7ac..a1aa110401 100644 --- a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx +++ b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx @@ -44,8 +44,10 @@ export type ChartConfigDisplaySettings = Pick< > & { groupByColumnsOnLeft?: boolean; // Per-tile cap on the number of series fetched for a group-by time chart. - // Undefined = disabled (no __hdx_series_limit CTE; every series is fetched). - seriesLimit?: number; + // null/undefined = disabled (no __hdx_series_limit CTE; every series is + // fetched). The editor clears to `null` (not `undefined`) so the cleared + // state survives JSON round-tripping through the URL query state. + seriesLimit?: number | null; }; /** @@ -86,7 +88,9 @@ function applyDefaultSettings( compareToPreviousPeriod: settings.compareToPreviousPeriod ?? false, fitYAxisToData: settings.fitYAxisToData ?? false, groupByColumnsOnLeft: settings.groupByColumnsOnLeft ?? false, - seriesLimit: settings.seriesLimit, + // Coerce to null so `reset` clears the input; undefined leaves the + // previously registered field value in place. + seriesLimit: settings.seriesLimit ?? null, color: settings.color, colorRules: settings.colorRules ? attachLocalIds(settings.colorRules) @@ -228,7 +232,7 @@ export default function ChartDisplaySettingsDrawer({ allowDecimal={false} value={value ?? ''} onChange={v => - onChange(v === '' || v == null ? undefined : Number(v)) + onChange(v === '' || v == null ? null : Number(v)) } /> )} diff --git a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx index e3f0970dee..84a11ec5f3 100644 --- a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx @@ -563,7 +563,10 @@ export default function EditTimeChartForm({ setValue('compareToPreviousPeriod', compareToPreviousPeriod); setValue('fitYAxisToData', fitYAxisToData); setValue('groupByColumnsOnLeft', groupByColumnsOnLeft); - setValue('seriesLimit', seriesLimit); + // Persist `null` (not undefined) when cleared so the disabled state + // survives JSON round-tripping through the URL query state — otherwise + // the dropped key lets RHF's `values` sync restore the stale value. + setValue('seriesLimit', seriesLimit ?? null); setValue('color', color); setValue('colorRules', colorRules); onSubmit(); diff --git a/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx b/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx index d44cabf59c..fa388de29f 100644 --- a/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx +++ b/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx @@ -240,7 +240,10 @@ describe('ChartDisplaySettingsDrawer', () => { expect(onChange.mock.calls[0][0]).toMatchObject({ seriesLimit: 25 }); }); - it('clears seriesLimit to undefined (disabled) when emptied', async () => { + // Emits null (not undefined) so the cleared/disabled state survives JSON + // round-tripping through the URL query state; undefined would be dropped, + // letting RHF's `values` sync restore the stale value. + it('clears seriesLimit to null (disabled) when emptied', async () => { const onChange = jest.fn(); const user = userEvent.setup(); @@ -257,7 +260,7 @@ describe('ChartDisplaySettingsDrawer', () => { await user.click(screen.getByRole('button', { name: /apply/i })); expect(onChange).toHaveBeenCalledTimes(1); - expect(onChange.mock.calls[0][0].seriesLimit).toBeUndefined(); + expect(onChange.mock.calls[0][0].seriesLimit).toBeNull(); }); }); }); diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 89d546b22c..c93cfadf25 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -275,7 +275,10 @@ export const SelectSQLStatementSchema = z.object({ havingLanguage: SearchConditionLanguageSchema.optional(), orderBy: SortSpecificationListSchema.optional(), limit: LimitSchema.optional(), - seriesLimit: z.number().int().positive().optional(), + // Nullish (not just optional): the chart editor clears the value to `null` + // so the cleared state survives JSON round-tripping (e.g. through the URL + // query state). `null` and `undefined` both mean "disabled" downstream. + seriesLimit: z.number().int().positive().nullish(), }); export type SQLInterval = z.infer;