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/7] 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/7] 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/7] 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/7] 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/7] 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/7] 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/7] 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.