From 4daed4a6447a41d53af7dd88ca8bf322633f8cb0 Mon Sep 17 00:00:00 2001 From: Noor Nabi <44670749+nabi-noor@users.noreply.github.com> Date: Tue, 29 Sep 2026 21:05:12 +0500 Subject: [PATCH 1/3] feat(charts): choose the range the Series Limit ranks over A chunked time chart with a positive Series Limit ranked its top N groups over the newest chunk window only. A group with no events in that window was dropped, even when it was the largest in the chart range. The docs said the ranking covered the whole queried range. Add seriesLimitRankingRange with two values. "recent" is the default and keeps the current behaviour. "full" ranks over the whole chart range, and every chunk still shares that one ranking range. - Display Settings gets a "Rank Series Over" control, shown for builder time charts with a positive Series Limit. - The external API and MCP schemas accept the new field, and their Series Limit descriptions now say how the ranking works. Closes #3204 Co-Authored-By: Claude Fable 5.1 --- .changeset/series-limit-ranking-range.md | 14 ++ packages/api/openapi.json | 24 +++- .../api/src/mcp/tools/dashboards/schemas.ts | 22 ++- .../src/routers/external-api/v2/dashboards.ts | 30 +++- .../v2/utils/__tests__/dashboards.test.ts | 46 +++++++ .../external-api/v2/utils/alertChartConfig.ts | 3 + .../external-api/v2/utils/dashboards.ts | 5 + packages/api/src/utils/zod.ts | 7 + .../components/ChartDisplaySettingsDrawer.tsx | 51 ++++++- .../DBEditTimeChartForm/EditTimeChartForm.tsx | 7 + .../ChartDisplaySettingsDrawer.test.tsx | 128 ++++++++++++++++++ .../hooks/__tests__/useChartConfig.test.tsx | 128 ++++++++++++++++++ packages/app/src/hooks/useChartConfig.tsx | 17 ++- packages/common-utils/src/types.ts | 22 ++- 14 files changed, 487 insertions(+), 17 deletions(-) create mode 100644 .changeset/series-limit-ranking-range.md diff --git a/.changeset/series-limit-ranking-range.md b/.changeset/series-limit-ranking-range.md new file mode 100644 index 0000000000..c062670bea --- /dev/null +++ b/.changeset/series-limit-ranking-range.md @@ -0,0 +1,14 @@ +--- +'@hyperdx/common-utils': patch +'@hyperdx/app': patch +'@hyperdx/api': patch +--- + +feat(charts): choose the range the Series Limit ranks over + +A time chart with a Series Limit picks its top N series from the most recent +part of the time range, so a series with no recent events was dropped even when +it was the largest overall. Display Settings now has a "Rank Series Over" +option: "Most recent window" (the default, unchanged behaviour) or "Full range". +The option is also available as `seriesLimitRankingRange` in the external API +and MCP, and the Series Limit descriptions now say how the ranking works. diff --git a/packages/api/openapi.json b/packages/api/openapi.json index 0a7b87fb39..40e9007bd0 100644 --- a/packages/api/openapi.json +++ b/packages/api/openapi.json @@ -1845,9 +1845,19 @@ "seriesLimit": { "type": "integer", "minimum": 0, - "description": "Maximum number of series rendered (top-N by value). Omit to use the default render cap, set 0 for unlimited, or a positive N to keep the top N series.\n", + "description": "Maximum number of series rendered (top-N by value). Omit to use the default render cap, set 0 for unlimited, or a positive N to keep the top N series. The range the ranking covers is set by seriesLimitRankingRange.\n", "example": 5 }, + "seriesLimitRankingRange": { + "type": "string", + "enum": [ + "recent", + "full" + ], + "description": "Range the top-N ranking for seriesLimit covers when the chart is fetched in chunks. \"recent\" ranks over the newest chunk window only, so a series with no events in that window is dropped. \"full\" ranks over the whole time range, which is accurate but scans more data. Ignored without a positive seriesLimit and a groupBy.\n", + "default": "recent", + "example": "full" + }, "formulas": { "type": "array", "maxItems": 10, @@ -1922,9 +1932,19 @@ "seriesLimit": { "type": "integer", "minimum": 0, - "description": "Maximum number of series rendered (top-N by value). Omit to use the default render cap, set 0 for unlimited, or a positive N to keep the top N series.", + "description": "Maximum number of series rendered (top-N by value). Omit to use the default render cap, set 0 for unlimited, or a positive N to keep the top N series. The range the ranking covers is set by seriesLimitRankingRange.", "example": 5 }, + "seriesLimitRankingRange": { + "type": "string", + "enum": [ + "recent", + "full" + ], + "description": "Range the top-N ranking for seriesLimit covers when the chart is fetched in chunks. \"recent\" ranks over the newest chunk window only, so a series with no events in that window is dropped. \"full\" ranks over the whole time range, which is accurate but scans more data. Ignored without a positive seriesLimit and a groupBy.\n", + "default": "recent", + "example": "full" + }, "formulas": { "type": "array", "maxItems": 10, diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index a944ff5fff..c370f60e9a 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -16,6 +16,7 @@ import { MetricsDataType, NumberTileColorConditionSchema, SearchConditionTrimmedLanguageSchema, + SeriesLimitRankingRangeSchema, StaticListDashboardFilterSchema, } from '@hyperdx/common-utils/dist/types'; import { z } from 'zod'; @@ -103,13 +104,24 @@ const sqlOnlyVariableMacroDescription = const timeChartSeriesLimitDescription = 'Maximum number of series to fetch (the "Series Limit" display setting). ' + - 'Keeps the top-N groups by aggregated value over the queried range and ' + - 'drops the rest. Requires `groupBy`; ignored on a chart without one. ' + - 'Three-state: omit to apply the default render cap, 0 for unlimited, or a ' + - 'positive N to keep the top N.'; + 'Keeps the top-N groups by aggregated value and drops the rest. The ' + + 'ranking covers the range set by `seriesLimitRankingRange`, which defaults ' + + 'to the most recent part of the queried range. Requires `groupBy`; ignored ' + + 'on a chart without one. Three-state: omit to apply the default render ' + + 'cap, 0 for unlimited, or a positive N to keep the top N.'; const seriesLimitSchema = z.number().int().nonnegative().optional(); +const seriesLimitRankingRangeSchema = + SeriesLimitRankingRangeSchema.optional().describe( + 'Range the top-N ranking for `seriesLimit` covers (the "Rank Series ' + + 'Over" display setting). "recent" (default) ranks over the newest ' + + 'chunk of the queried range only, so a group with no recent events is ' + + 'dropped even if it is the largest overall. "full" ranks over the ' + + 'whole queried range, which is accurate but scans more data. Ignored ' + + 'without a positive `seriesLimit` and a `groupBy`.', + ); + const numberTileColorDescription = 'Static color for the displayed number, as a palette token such as ' + '"chart-blue", "chart-green", or "chart-success" (see the enum for the ' + @@ -705,6 +717,7 @@ export const mcpLineTileSchema = mcpTileLayoutSchema.extend({ 'Scale the y-axis to the data range instead of starting at zero.', ), seriesLimit: seriesLimitSchema.describe(timeChartSeriesLimitDescription), + seriesLimitRankingRange: seriesLimitRankingRangeSchema, formulas: mcpTileFormulasSchema, showOperandSeries: mcpShowOperandSeriesSchema, }), @@ -726,6 +739,7 @@ export const mcpBarTileSchema = mcpTileLayoutSchema.extend({ .optional() .describe(tileLevelNumberFormatDescription), seriesLimit: seriesLimitSchema.describe(timeChartSeriesLimitDescription), + seriesLimitRankingRange: seriesLimitRankingRangeSchema, formulas: mcpTileFormulasSchema, showOperandSeries: mcpShowOperandSeriesSchema, }), diff --git a/packages/api/src/routers/external-api/v2/dashboards.ts b/packages/api/src/routers/external-api/v2/dashboards.ts index 51546ca632..6d67ff613d 100644 --- a/packages/api/src/routers/external-api/v2/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/dashboards.ts @@ -703,8 +703,21 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * description: > * Maximum number of series rendered (top-N by value). Omit to use * the default render cap, set 0 for unlimited, or a positive N to - * keep the top N series. + * keep the top N series. The range the ranking covers is set by + * seriesLimitRankingRange. * example: 5 + * seriesLimitRankingRange: + * type: string + * enum: [recent, full] + * description: > + * Range the top-N ranking for seriesLimit covers when the chart is + * fetched in chunks. "recent" ranks over the newest chunk window + * only, so a series with no events in that window is dropped. + * "full" ranks over the whole time range, which is accurate but + * scans more data. Ignored without a positive seriesLimit and a + * groupBy. + * default: recent + * example: full * formulas: * type: array * maxItems: 10 @@ -773,8 +786,21 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * description: >- * Maximum number of series rendered (top-N by value). Omit to use * the default render cap, set 0 for unlimited, or a positive N to - * keep the top N series. + * keep the top N series. The range the ranking covers is set by + * seriesLimitRankingRange. * example: 5 + * seriesLimitRankingRange: + * type: string + * enum: [recent, full] + * description: > + * Range the top-N ranking for seriesLimit covers when the chart is + * fetched in chunks. "recent" ranks over the newest chunk window + * only, so a series with no events in that window is dropped. + * "full" ranks over the whole time range, which is accurate but + * scans more data. Ignored without a positive seriesLimit and a + * groupBy. + * default: recent + * example: full * formulas: * type: array * maxItems: 10 diff --git a/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts index 3306ab728d..80c117e6ce 100644 --- a/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts +++ b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts @@ -448,6 +448,52 @@ describe('convertToExternalDashboard orphan-ref heal', () => { }); }, ); + + // seriesLimitRankingRange defaults to 'recent' when absent. An explicit + // value must survive a GET->PUT round-trip; null maps to absent. + describe.each([DisplayType.Line, DisplayType.StackedBar])( + 'seriesLimitRankingRange serialization for %s tiles', + displayType => { + function readRankingRange( + seriesLimitRankingRange: 'recent' | 'full' | null | undefined, + ): unknown { + const doc = makeDoc({ + tiles: [ + makeTile({ + id: 'ranking-range-tile', + config: { + displayType, + source: new mongoose.Types.ObjectId().toString(), + name: 'Ranking range tile', + select: [{ aggFn: 'count', valueExpression: '' }], + where: '', + groupBy: 'ServiceName', + seriesLimit: 5, + seriesLimitRankingRange, + }, + }), + ], + }); + const wire = JSON.parse( + JSON.stringify(convertToExternalDashboard(doc)), + ); + return wire.tiles[0].config.seriesLimitRankingRange; + } + + it('passes full through unchanged', () => { + expect(readRankingRange('full')).toBe('full'); + }); + + it('passes an explicit recent through unchanged', () => { + expect(readRankingRange('recent')).toBe('recent'); + }); + + it('emits it as absent when stored as null or not set', () => { + expect(readRankingRange(null)).toBeUndefined(); + expect(readRankingRange(undefined)).toBeUndefined(); + }); + }, + ); }); describe('convertToExternalDashboard stale aggregation params', () => { diff --git a/packages/api/src/routers/external-api/v2/utils/alertChartConfig.ts b/packages/api/src/routers/external-api/v2/utils/alertChartConfig.ts index f7356078f9..032dba6c2d 100644 --- a/packages/api/src/routers/external-api/v2/utils/alertChartConfig.ts +++ b/packages/api/src/routers/external-api/v2/utils/alertChartConfig.ts @@ -112,6 +112,9 @@ export const EVALUATION_INERT_CONFIG_KEYS: ReadonlySet = new Set([ // Evaluation-relevant only with a groupBy, which (on variants whose shape // lacks seriesLimit, i.e. number) is itself refused when present. 'seriesLimit', + // Only read by the chunked time-chart fetch in the app; the alert task + // does not chunk its queries. + 'seriesLimitRankingRange', // The alert task derives granularity from alert.interval // (tasks/checkAlerts/index.ts: `${windowSizeInMins} minute`) and the alert // detail chart from intervalToGranularity, so the stored value only affects diff --git a/packages/api/src/routers/external-api/v2/utils/dashboards.ts b/packages/api/src/routers/external-api/v2/utils/dashboards.ts index fa3a4eb311..a0f37b034c 100644 --- a/packages/api/src/routers/external-api/v2/utils/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/utils/dashboards.ts @@ -339,6 +339,8 @@ export const convertToExternalTileChartConfig = ( // Three-state passthrough: 0 (unlimited) and positive N round-trip; // null/undefined map to absent (the default-cap state). seriesLimit: config.seriesLimit ?? undefined, + // null/undefined map to absent (the default, 'recent'). + seriesLimitRankingRange: config.seriesLimitRankingRange ?? undefined, ...externalFormulaFields, ...externalShowOperandSeriesField, }; @@ -360,6 +362,8 @@ export const convertToExternalTileChartConfig = ( // Three-state passthrough: 0 (unlimited) and positive N round-trip; // null/undefined map to absent (the default-cap state). seriesLimit: config.seriesLimit ?? undefined, + // null/undefined map to absent (the default, 'recent'). + seriesLimitRankingRange: config.seriesLimitRankingRange ?? undefined, ...externalFormulaFields, ...externalShowOperandSeriesField, }; @@ -775,6 +779,7 @@ export function convertToInternalTileConfig( fillNulls: externalConfig.fillNulls === false ? false : undefined, seriesReturnType: externalConfig.asRatio ? 'ratio' : undefined, seriesLimit: externalConfig.seriesLimit, + seriesLimitRankingRange: externalConfig.seriesLimitRankingRange, name, } satisfies BuilderSavedChartConfig; break; diff --git a/packages/api/src/utils/zod.ts b/packages/api/src/utils/zod.ts index 78622d9c1a..37f482e930 100644 --- a/packages/api/src/utils/zod.ts +++ b/packages/api/src/utils/zod.ts @@ -24,6 +24,7 @@ import { QueryExpressionDashboardFilterSchema, scheduleStartAtSchema, SearchConditionLanguageSchema as whereLanguageSchema, + SeriesLimitRankingRangeSchema, StaticListDashboardFilterSchema, tagsSchema, validateAlertChannelSelection, @@ -354,6 +355,9 @@ const externalDashboardLineChartConfigSchema = // Three-state, matching the internal SharedChartSettingsSchema.seriesLimit: // omitted = default render cap, 0 = unlimited, positive N = top-N by peak. seriesLimit: z.number().int().nonnegative().optional(), + // Matches the internal SelectSQLStatementSchema.seriesLimitRankingRange: + // omitted = 'recent' (rank over the newest chunk window). + seriesLimitRankingRange: SeriesLimitRankingRangeSchema.optional(), }); const externalDashboardLineRawSqlChartConfigSchema = @@ -376,6 +380,9 @@ const externalDashboardBarChartConfigSchema = // Three-state, matching the internal SharedChartSettingsSchema.seriesLimit: // omitted = default render cap, 0 = unlimited, positive N = top-N by peak. seriesLimit: z.number().int().nonnegative().optional(), + // Matches the internal SelectSQLStatementSchema.seriesLimitRankingRange: + // omitted = 'recent' (rank over the newest chunk window). + seriesLimitRankingRange: SeriesLimitRankingRangeSchema.optional(), }); const externalDashboardBarRawSqlChartConfigSchema = diff --git a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx index 20f3ee0894..70dc5755f6 100644 --- a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx +++ b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx @@ -9,6 +9,7 @@ import { DisplayType, MAX_LEGEND_TEMPLATE_LENGTH, NumberFormat, + SeriesLimitRankingRange, } from '@hyperdx/common-utils/dist/types'; import { Alert, @@ -19,6 +20,7 @@ import { Drawer, Group, NumberInput, + SegmentedControl, Stack, Text, } from '@mantine/core'; @@ -58,6 +60,10 @@ export type ChartConfigDisplaySettings = Pick< // for the authoritative semantics. The editor clears to `null` (not // `undefined`) so the cleared state survives JSON round-tripping via the URL. seriesLimit?: number | null; + // Builder group-by time charts only: the range the top-N ranking covers when + // the chart is fetched in chunks. null/undefined = 'recent' (the newest + // chunk window). See SeriesLimitRankingRangeSchema. + seriesLimitRankingRange?: SeriesLimitRankingRange | null; // PromQL-only: Handlebars template over each series' Prometheus label set // that renders the legend/tooltip name. legendTemplate?: string; @@ -107,6 +113,7 @@ function applyDefaultSettings( // Coerce to null so `reset` clears the input; undefined leaves the // previously registered field value in place. seriesLimit: settings.seriesLimit ?? null, + seriesLimitRankingRange: settings.seriesLimitRankingRange ?? 'recent', legendTemplate: settings.legendTemplate ?? '', color: settings.color, colorRules: settings.colorRules @@ -148,6 +155,7 @@ export default function ChartDisplaySettingsDrawer({ }, [appliedDefaults, reset]); const fillNulls = useWatch({ control, name: 'fillNulls' }); + const seriesLimit = useWatch({ control, name: 'seriesLimit' }); const isFillNullsEnabled = shouldFillNullsWithZero(fillNulls); const handleClose = useCallback(() => { @@ -170,6 +178,10 @@ export default function ChartDisplaySettingsDrawer({ onChange( { ...rest, + // 'recent' is the default, so persist it as null to keep saved + // configs unchanged unless the user opts into the full range. + seriesLimitRankingRange: + formValues.seriesLimitRankingRange === 'full' ? 'full' : null, numberFormat: numberFormatExplicit ? formValues.numberFormat : undefined, @@ -201,6 +213,13 @@ export default function ChartDisplaySettingsDrawer({ // excluded — its series come from Prometheus, not this pipeline. const showSeriesLimit = isTimeChart && configType !== 'promql'; const isRawSqlTimeChart = showSeriesLimit && configType === 'sql'; + // The ranking range only matters where the __hdx_series_limit CTE runs: + // builder time charts with a positive Series Limit. + const showSeriesLimitRankingRange = + showSeriesLimit && + !isRawSqlTimeChart && + seriesLimit != null && + seriesLimit > 0; // Every PromQL display that surfaces a series name. A number tile shows one // value and a table gives each label its own column, so neither has a legend. @@ -304,7 +323,7 @@ export default function ChartDisplaySettingsDrawer({ description={ isRawSqlTimeChart ? `Maximum number of series rendered, keeping those with the largest values. Leave empty for the default (${MAX_RENDERED_TIME_CHART_SERIES}); set 0 for unlimited.` - : `Maximum number of series fetched for a group-by chart, keeping those with the largest values. Leave empty for the default (${MAX_RENDERED_TIME_CHART_SERIES}); set 0 for unlimited.` + : `Maximum number of series fetched for a group-by chart, keeping those with the largest values in the ranking range below. Leave empty for the default (${MAX_RENDERED_TIME_CHART_SERIES}); set 0 for unlimited.` } placeholder={`Default (${MAX_RENDERED_TIME_CHART_SERIES})`} min={0} @@ -318,6 +337,36 @@ export default function ChartDisplaySettingsDrawer({ /> )} + {showSeriesLimitRankingRange && ( + + + Rank Series Over + + + Most recent window ranks series by the newest part of the time + range only, so a series with no recent events is dropped. Full + range ranks over the whole time range, which is accurate but + slower on long ranges. + + ( + + )} + /> + + )} )} diff --git a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx index 46cad955c2..61d60e52e1 100644 --- a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx @@ -342,6 +342,7 @@ export default function EditTimeChartForm({ groupByColumnsOnLeft, alternateRowBackground, seriesLimit, + seriesLimitRankingRange, color, colorRules, backgroundChart, @@ -357,6 +358,7 @@ export default function EditTimeChartForm({ 'groupByColumnsOnLeft', 'alternateRowBackground', 'seriesLimit', + 'seriesLimitRankingRange', 'color', 'colorRules', 'backgroundChart', @@ -388,6 +390,7 @@ export default function EditTimeChartForm({ groupByColumnsOnLeft, alternateRowBackground, seriesLimit, + seriesLimitRankingRange, color, colorRules, backgroundChart, @@ -402,6 +405,7 @@ export default function EditTimeChartForm({ groupByColumnsOnLeft, alternateRowBackground, seriesLimit, + seriesLimitRankingRange, color, colorRules, backgroundChart, @@ -883,6 +887,7 @@ export default function EditTimeChartForm({ groupByColumnsOnLeft, alternateRowBackground, seriesLimit, + seriesLimitRankingRange, color, colorRules, backgroundChart, @@ -906,6 +911,8 @@ export default function EditTimeChartForm({ // 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); + // Same null-not-undefined rule as seriesLimit above. + setValue('seriesLimitRankingRange', seriesLimitRankingRange ?? null); setValue('color', color); setValue('colorRules', colorRules); setValue('backgroundChart', backgroundChart); diff --git a/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx b/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx index cc0a906752..8db9b2312d 100644 --- a/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx +++ b/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx @@ -271,6 +271,134 @@ describe('ChartDisplaySettingsDrawer', () => { }); }); + describe('series limit ranking range setting', () => { + const builderProps = { ...baseProps, configType: 'builder' as const }; + + it('is hidden until a positive Series Limit is set', async () => { + const user = userEvent.setup(); + + renderWithMantine( + , + ); + + expect( + screen.queryByTestId('series-limit-ranking-range'), + ).not.toBeInTheDocument(); + + await user.type( + screen.getByRole('textbox', { name: /series limit/i }), + '5', + ); + + expect( + screen.getByTestId('series-limit-ranking-range'), + ).toBeInTheDocument(); + }); + + it('is hidden when Series Limit is 0 (unlimited)', () => { + renderWithMantine( + , + ); + + expect( + screen.queryByTestId('series-limit-ranking-range'), + ).not.toBeInTheDocument(); + }); + + it('is hidden for raw SQL time charts, which rank client-side', () => { + renderWithMantine( + , + ); + + expect( + screen.queryByTestId('series-limit-ranking-range'), + ).not.toBeInTheDocument(); + }); + + it('defaults to the most recent window', () => { + renderWithMantine( + , + ); + + expect( + screen.getByRole('radio', { name: /most recent window/i }), + ).toBeChecked(); + expect( + screen.getByRole('radio', { name: /full range/i }), + ).not.toBeChecked(); + }); + + it('calls onChange with seriesLimitRankingRange full when selected', async () => { + const onChange = jest.fn(); + const user = userEvent.setup(); + + renderWithMantine( + , + ); + + await user.click(screen.getByRole('radio', { name: /full range/i })); + await user.click(screen.getByRole('button', { name: /apply/i })); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange.mock.calls[0][0]).toMatchObject({ + seriesLimit: 5, + seriesLimitRankingRange: 'full', + }); + expect(onChange.mock.calls[0][1]).toBe(true); + }); + + // The default is stored as null so existing saved configs are unchanged + // and the cleared state survives the URL round-trip. + it('emits null when switched back to the most recent window', async () => { + const onChange = jest.fn(); + const user = userEvent.setup(); + + renderWithMantine( + , + ); + + expect(screen.getByRole('radio', { name: /full range/i })).toBeChecked(); + + await user.click( + screen.getByRole('radio', { name: /most recent window/i }), + ); + await user.click(screen.getByRole('button', { name: /apply/i })); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange.mock.calls[0][0].seriesLimitRankingRange).toBeNull(); + }); + }); + describe('categorical series limit setting (pie/bar)', () => { const builderProps = { ...baseProps, configType: 'builder' as const }; diff --git a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx index 0793af11e0..85d1629306 100644 --- a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx +++ b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx @@ -1566,6 +1566,103 @@ describe('useChartConfig', () => { } }); + it('ranks the series limit over the full range on each chunk when seriesLimitRankingRange is full', async () => { + const dateRange: [Date, Date] = [ + new Date('2025-10-01 00:00:00Z'), + new Date('2025-10-02 00:00:00Z'), + ]; + const config = createMockChartConfig({ + dateRange, + granularity: '3 hour', + seriesLimit: 3, + seriesLimitRankingRange: 'full', + }); + + 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)); + + // Every chunk still shares one ranking range, so the union across + // chunks stays within the limit. It is the whole chart range, so a + // group that is only busy early in the range is kept. + const calls = mockClickhouseClient.queryChartConfig.mock.calls; + expect(calls).toHaveLength(3); + for (const [{ config: windowed }] of calls) { + expect(windowed.seriesLimitDateRange).toEqual(dateRange); + } + }); + + it('pins the series-limit ranking to the newest window when seriesLimitRankingRange is recent', 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, + seriesLimitRankingRange: 'recent', + }); + + 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)); + + 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(newestWindow); + } + }); + + it('does not set seriesLimitDateRange without a positive seriesLimit, even when seriesLimitRankingRange is full', async () => { + const config = createMockChartConfig({ + dateRange: [ + new Date('2025-10-01 00:00:00Z'), + new Date('2025-10-02 00:00:00Z'), + ], + granularity: '3 hour', + seriesLimitRankingRange: 'full', + }); + + 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)); + + const calls = mockClickhouseClient.queryChartConfig.mock.calls; + expect(calls).toHaveLength(3); + for (const [{ config: windowed }] of calls) { + expect('seriesLimitDateRange' in windowed).toBe(false); + } + }); + it('does not set seriesLimitDateRange when the query is not chunked', async () => { const config = createMockChartConfig({ dateRange: [ @@ -2117,6 +2214,37 @@ describe('useChartConfig', () => { }); }); + it('ranks the series limit over the full range with parallel queries when seriesLimitRankingRange is full', async () => { + const { config } = setupParallelQueries(); + const configWithLimit = { + ...config, + seriesLimit: 3, + seriesLimitRankingRange: 'full' as const, + }; + + 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('pins the series-limit ranking to the newest window with parallel queries', async () => { const { config } = setupParallelQueries(); const configWithLimit = { ...config, seriesLimit: 3 }; diff --git a/packages/app/src/hooks/useChartConfig.tsx b/packages/app/src/hooks/useChartConfig.tsx index ea96708564..3ea62a550f 100644 --- a/packages/app/src/hooks/useChartConfig.tsx +++ b/packages/app/src/hooks/useChartConfig.tsx @@ -192,11 +192,18 @@ async function* fetchDataInChunks({ // 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; + // exceeds seriesLimit. By default 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. A tile can opt into ranking + // over the full chart range with `seriesLimitRankingRange: 'full'`. + const fullDateRange = config.dateRange; + const rankOverFullRange = + isBuilderChartConfig(config) && config.seriesLimitRankingRange === 'full'; + const rankingDateRange = + rankOverFullRange && fullDateRange != null + ? fullDateRange + : windows[0]?.dateRange; // Only a positive seriesLimit emits the __hdx_series_limit CTE (0 = unlimited, // null = default), so only then does the ranking need a pinned date range. const seriesLimit = diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 69c26e4699..e90092ae28 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -471,6 +471,18 @@ export const ChSqlSchema = z.object({ // When making changes here, consider if they need to be made to the external API // schema as well (packages/api/src/utils/zod.ts). +// Which range the `__hdx_series_limit` CTE ranks groups over when a time +// chart is fetched in chunks: +// - `recent` (default): the newest chunk window only. Cheap, but groups with +// no events in that window are dropped, however large they are earlier on. +// - `full`: the whole chart date range. Accurate, but every chunk scans the +// full range to rank. +// A query that is not chunked always ranks over its whole date range. +export const SeriesLimitRankingRangeSchema = z.enum(['recent', 'full']); +export type SeriesLimitRankingRange = z.infer< + typeof SeriesLimitRankingRangeSchema +>; + export const SelectSQLStatementSchema = z.object({ select: SelectListSchema, from: z.object({ @@ -490,6 +502,9 @@ export const SelectSQLStatementSchema = z.object({ // (0 = unlimited, null/undefined = default cap). On builder group-by time // charts a positive value additionally drives the __hdx_series_limit CTE. seriesLimit: z.number().int().nonnegative().nullish(), + // Builder group-by time charts only: the range the top-N ranking covers. + // Nullish = `recent`. See SeriesLimitRankingRangeSchema. + seriesLimitRankingRange: SeriesLimitRankingRangeSchema.nullish(), }); export type SQLInterval = z.infer; @@ -1883,9 +1898,10 @@ export type DateRange = { dateRangeStartInclusive?: boolean; // default true dateRangeEndInclusive?: boolean; // default true // Runtime-only, set by query chunking when dateRange is narrowed to a - // 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. + // window: a fixed ranking range used by the `__hdx_series_limit` CTE so + // every chunk ranks (and keeps) the same top-N series. It is the newest + // chunk window, or the full chart range when `seriesLimitRankingRange` is + // `full`. Never persisted. seriesLimitDateRange?: [Date, Date]; // Runtime-only, populated from the queried MetricSource's // `minAutoGranularity` (when set) by whichever caller resolves the source From f6c0337c9ce1de141513e0746eab7f5726acd8e2 Mon Sep 17 00:00:00 2001 From: Noor Nabi <44670749+nabi-noor@users.noreply.github.com> Date: Tue, 29 Sep 2026 22:44:11 +0500 Subject: [PATCH 2/3] fix(charts): address review feedback on series limit ranking range - Use sentence case for the new setting label, "Rank series over", as the UI text style guide requires. - Test the PUT side of the external API. The earlier test only read the GET-side value, so a save that dropped seriesLimitRankingRange would have passed. - Test that a series active only early in the range is returned when the ranking covers the full range, and that every chunk's ranking CTE scans the whole chart range while its outer query stays windowed. Co-Authored-By: Claude Fable 5.1 --- .changeset/series-limit-ranking-range.md | 2 +- .../api/src/mcp/tools/dashboards/schemas.ts | 4 +- .../v2/utils/__tests__/dashboards.test.ts | 78 +++++++++++++++++++ .../components/ChartDisplaySettingsDrawer.tsx | 2 +- .../hooks/__tests__/useChartConfig.test.tsx | 54 +++++++++++++ .../src/__tests__/renderChartConfig.test.ts | 68 ++++++++++++++++ 6 files changed, 204 insertions(+), 4 deletions(-) diff --git a/.changeset/series-limit-ranking-range.md b/.changeset/series-limit-ranking-range.md index c062670bea..385ae5f87c 100644 --- a/.changeset/series-limit-ranking-range.md +++ b/.changeset/series-limit-ranking-range.md @@ -8,7 +8,7 @@ feat(charts): choose the range the Series Limit ranks over A time chart with a Series Limit picks its top N series from the most recent part of the time range, so a series with no recent events was dropped even when -it was the largest overall. Display Settings now has a "Rank Series Over" +it was the largest overall. Display Settings now has a "Rank series over" option: "Most recent window" (the default, unchanged behaviour) or "Full range". The option is also available as `seriesLimitRankingRange` in the external API and MCP, and the Series Limit descriptions now say how the ranking works. diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index c370f60e9a..8773fde484 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -114,8 +114,8 @@ const seriesLimitSchema = z.number().int().nonnegative().optional(); const seriesLimitRankingRangeSchema = SeriesLimitRankingRangeSchema.optional().describe( - 'Range the top-N ranking for `seriesLimit` covers (the "Rank Series ' + - 'Over" display setting). "recent" (default) ranks over the newest ' + + 'Range the top-N ranking for `seriesLimit` covers (the "Rank series ' + + 'over" display setting). "recent" (default) ranks over the newest ' + 'chunk of the queried range only, so a group with no recent events is ' + 'dropped even if it is the largest overall. "full" ranks over the ' + 'whole queried range, which is accurate but scans more data. Ignored ' + diff --git a/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts index 80c117e6ce..5297c92d4a 100644 --- a/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts +++ b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts @@ -492,6 +492,84 @@ describe('convertToExternalDashboard orphan-ref heal', () => { expect(readRankingRange(null)).toBeUndefined(); expect(readRankingRange(undefined)).toBeUndefined(); }); + + // The PUT side: what convertToInternalTileConfig stores for the value + // a client sends. Without this, a save that drops the field would pass. + function writeRankingRange( + seriesLimitRankingRange: 'recent' | 'full' | undefined, + ): unknown { + const externalDisplayType = + displayType === DisplayType.StackedBar ? 'stacked_bar' : 'line'; + const internal = convertToInternalTileConfig({ + id: 'ranking-range-tile', + x: 0, + y: 0, + w: 12, + h: 4, + name: 'Ranking range tile', + config: { + displayType: externalDisplayType, + sourceId: new mongoose.Types.ObjectId().toString(), + select: [{ aggFn: 'count', where: '' }], + groupBy: 'ServiceName', + seriesLimit: 5, + seriesLimitRankingRange, + }, + }); + if (!isBuilderSavedChartConfig(internal.config)) { + throw new Error('Expected a builder config for a time chart tile'); + } + return internal.config.seriesLimitRankingRange; + } + + it('stores full when a client sends it', () => { + expect(writeRankingRange('full')).toBe('full'); + }); + + it('stores an explicit recent when a client sends it', () => { + expect(writeRankingRange('recent')).toBe('recent'); + }); + + it('stores nothing when a client omits it', () => { + expect(writeRankingRange(undefined)).toBeUndefined(); + }); + + it.each(['recent', 'full'] as const)( + 'keeps %s through GET then PUT', + value => { + const doc = makeDoc({ + tiles: [ + makeTile({ + id: 'ranking-range-tile', + config: { + displayType, + source: new mongoose.Types.ObjectId().toString(), + name: 'Ranking range tile', + select: [{ aggFn: 'count', valueExpression: '' }], + where: '', + groupBy: 'ServiceName', + seriesLimit: 5, + seriesLimitRankingRange: value, + }, + }), + ], + }); + const [externalTile] = convertToExternalDashboard(doc).tiles; + const externalConfig = externalTile.config; + if (!externalConfig) { + throw new Error('Expected the tile to have an external config'); + } + const internal = convertToInternalTileConfig({ + ...externalTile, + config: externalConfig, + }); + if (!isBuilderSavedChartConfig(internal.config)) { + throw new Error('Expected a builder config for a time chart tile'); + } + expect(internal.config.seriesLimitRankingRange).toBe(value); + expect(internal.config.seriesLimit).toBe(5); + }, + ); }, ); }); diff --git a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx index 70dc5755f6..8a87a9d405 100644 --- a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx +++ b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx @@ -340,7 +340,7 @@ export default function ChartDisplaySettingsDrawer({ {showSeriesLimitRankingRange && ( - Rank Series Over + Rank series over Most recent window ranks series by the newest part of the time diff --git a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx index 85d1629306..9a03ec5dbe 100644 --- a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx +++ b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx @@ -1600,6 +1600,60 @@ describe('useChartConfig', () => { } }); + it('returns a series that is only active early in the range when seriesLimitRankingRange is full', async () => { + const dateRange: [Date, Date] = [ + new Date('2025-10-01 00:00:00Z'), + new Date('2025-10-02 00:00:00Z'), + ]; + const config = createMockChartConfig({ + dateRange, + granularity: '3 hour', + seriesLimit: 1, + seriesLimitRankingRange: 'full', + }); + + // The ranking is resolved in SQL. With the full range pinned, the one + // kept group is the early-only one, so the newest chunks come back + // empty and only the oldest chunk returns rows for it. + const earlyOnlyRows = [ + { + 'count()': '10000', + SeverityText: 'early-only', + __hdx_time_bucket: '2025-10-01T03:00:00Z', + }, + { + 'count()': '9000', + SeverityText: 'early-only', + __hdx_time_bucket: '2025-10-01T06:00:00Z', + }, + ]; + mockClickhouseClient.queryChartConfig + .mockResolvedValueOnce(createMockQueryResponse([])) + .mockResolvedValueOnce(createMockQueryResponse([])) + .mockResolvedValueOnce(createMockQueryResponse(earlyOnlyRows)); + + const { result } = renderHook( + () => useQueriedChartConfig(config, { enableQueryChunking: 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); + // The oldest chunk, the only one with rows, ranked over the full range. + expect(calls[2][0].config.seriesLimitDateRange).toEqual(dateRange); + expect(calls[2][0].config.seriesLimit).toBe(1); + + // The early-only series reaches the chart, and it is the only one. + const rows = result.current.data?.data ?? []; + expect(rows).toEqual(earlyOnlyRows); + const series = new Set(rows.map(r => r.SeverityText)); + expect([...series]).toEqual(['early-only']); + expect(series.size).toBeLessThanOrEqual(config.seriesLimit ?? 0); + }); + it('pins the series-limit ranking to the newest window when seriesLimitRankingRange is recent', async () => { const config = createMockChartConfig({ dateRange: [ diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 2264838536..86359655d2 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -771,6 +771,74 @@ describe('renderChartConfig', () => { ); }); + it('ranks over the full chart range from every chunk when the caller pins it', async () => { + // seriesLimitRankingRange: 'full' pins the ranking to the whole chart + // range. A group that is only active in the oldest chunk is then inside + // the ranking scan of every chunk, the newest one included. + const chartRange: [Date, Date] = [ + new Date('2025-02-12T00:00:00Z'), + new Date('2025-02-13T00:00:00Z'), + ]; + const newestWindow: [Date, Date] = [ + new Date('2025-02-12T18:00:00Z'), + chartRange[1], + ]; + const oldestWindow: [Date, Date] = [ + chartRange[0], + new Date('2025-02-12T06:00:00Z'), + ]; + const render = async ( + dateRange: [Date, Date], + dateRangeEndInclusive: boolean, + seriesLimitDateRange: [Date, Date], + ) => + parameterizedQueryToSql( + await renderChartConfig( + { + ...baseLogsConfig, + seriesLimit: 1, + dateRange, + dateRangeEndInclusive, + seriesLimitDateRange, + }, + mockMetadata, + querySettings, + ), + ); + 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); + }; + const outerOf = (sql: string) => sql.slice(sql.indexOf(') SELECT ')); + const chartStart = String(chartRange[0].getTime()); + const chartEnd = String(chartRange[1].getTime()); + const newestStart = String(newestWindow[0].getTime()); + + const newestFull = await render(newestWindow, true, chartRange); + const oldestFull = await render(oldestWindow, false, chartRange); + + // Both chunks rank over the whole chart range and keep the same top N. + expect(cteOf(newestFull)).toBe(cteOf(oldestFull)); + expect(cteOf(newestFull)).toContain(chartStart); + expect(cteOf(newestFull)).toContain(chartEnd); + // The ranking scan is not narrowed to the newest window. + expect(cteOf(newestFull)).not.toContain(newestStart); + // The outer query still reads only its own chunk. + expect(outerOf(newestFull)).toContain(newestStart); + expect(outerOf(oldestFull)).not.toContain(chartEnd); + // The limit applies to the ranking, so the union across chunks is N. + expect(cteOf(newestFull)).toMatch(/LIMIT 1\b/); + + // Contrast: pinned to the newest window, the ranking scan starts at + // that window and cannot see the oldest chunk's groups. + const newestRecent = await render(newestWindow, true, newestWindow); + expect(cteOf(newestRecent)).toContain(newestStart); + expect(cteOf(newestRecent)).not.toContain(chartStart); + }); + it('does not emit a series-limit CTE without a group-by', async () => { const sql = parameterizedQueryToSql( await renderChartConfig( From 80cfa079cde7c171cf6c7dcc2b97c4b665db3782 Mon Sep 17 00:00:00 2001 From: Noor Nabi <44670749+nabi-noor@users.noreply.github.com> Date: Tue, 29 Sep 2026 22:45:10 +0500 Subject: [PATCH 3/3] test(api): fix types in the series limit ranking range PUT test Build the external tile config per display type so it narrows to one member of the tile config union. Co-Authored-By: Claude Fable 5.1 --- .../v2/utils/__tests__/dashboards.test.ts | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts index 5297c92d4a..119b31d7bc 100644 --- a/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts +++ b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts @@ -498,8 +498,13 @@ describe('convertToExternalDashboard orphan-ref heal', () => { function writeRankingRange( seriesLimitRankingRange: 'recent' | 'full' | undefined, ): unknown { - const externalDisplayType = - displayType === DisplayType.StackedBar ? 'stacked_bar' : 'line'; + const shared = { + sourceId: new mongoose.Types.ObjectId().toString(), + select: [{ aggFn: 'count' as const, where: '' }], + groupBy: 'ServiceName', + seriesLimit: 5, + seriesLimitRankingRange, + }; const internal = convertToInternalTileConfig({ id: 'ranking-range-tile', x: 0, @@ -507,14 +512,10 @@ describe('convertToExternalDashboard orphan-ref heal', () => { w: 12, h: 4, name: 'Ranking range tile', - config: { - displayType: externalDisplayType, - sourceId: new mongoose.Types.ObjectId().toString(), - select: [{ aggFn: 'count', where: '' }], - groupBy: 'ServiceName', - seriesLimit: 5, - seriesLimitRankingRange, - }, + config: + displayType === DisplayType.StackedBar + ? { ...shared, displayType: 'stacked_bar' as const } + : { ...shared, displayType: 'line' as const }, }); if (!isBuilderSavedChartConfig(internal.config)) { throw new Error('Expected a builder config for a time chart tile');