diff --git a/.changeset/dashboard-time-chart-series-limit.md b/.changeset/dashboard-time-chart-series-limit.md new file mode 100644 index 0000000000..d10d9e27a2 --- /dev/null +++ b/.changeset/dashboard-time-chart-series-limit.md @@ -0,0 +1,15 @@ +--- +'@hyperdx/common-utils': minor +'@hyperdx/app': minor +'@hyperdx/api': minor +--- + +Cap high-cardinality time-chart series to protect the browser from rendering +thousands of lines at once. Time charts now materialize and draw a bounded +number of series per tile, with escape hatches to reveal the rest on demand: a +"+N more" affordance in the hover and pinned tooltips, and a "load all series" +action that lifts the cap for a chart. Tooltips also cap how many rows they +render per frame so a wide bucket can't mount thousands of popovers. The +external dashboards API exposes the per-tile series limit as a three-state value +across tile types — omit for the default cap, 0 for unlimited, or a positive N +for the top N diff --git a/packages/api/openapi.json b/packages/api/openapi.json index c2fbbb9aee..98ba15d9d7 100644 --- a/packages/api/openapi.json +++ b/packages/api/openapi.json @@ -1645,8 +1645,8 @@ }, "seriesLimit": { "type": "integer", - "minimum": 1, - "description": "Maximum number of series rendered (top-N by value). Omit for no limit.", + "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", "example": 5 } } @@ -1709,8 +1709,8 @@ }, "seriesLimit": { "type": "integer", - "minimum": 1, - "description": "Maximum number of series rendered (top-N by value). Omit for no limit.", + "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.", "example": 5 } } @@ -1887,8 +1887,8 @@ }, "limit": { "type": "integer", - "minimum": 1, - "description": "Maximum number of slices (SQL LIMIT). Without a custom \"orderBy\" the query keeps the groups with the largest aggregated values; with an \"orderBy\" it keeps the first slices in that order. Omit to fetch all groups.\n", + "minimum": 0, + "description": "Maximum number of slices (SQL LIMIT). Without a custom \"orderBy\" the query keeps the groups with the largest aggregated values; with an \"orderBy\" it keeps the first slices in that order. Omit or set 0 to fetch all groups.\n", "example": 10 } } @@ -1942,8 +1942,8 @@ }, "limit": { "type": "integer", - "minimum": 1, - "description": "Maximum number of bars (SQL LIMIT). Without a custom \"orderBy\" the query keeps the groups with the largest aggregated values; with an \"orderBy\" it keeps the first bars in that order. Omit to fetch all groups.\n", + "minimum": 0, + "description": "Maximum number of bars (SQL LIMIT). Without a custom \"orderBy\" the query keeps the groups with the largest aggregated values; with an \"orderBy\" it keeps the first bars in that order. Omit or set 0 to fetch all groups.\n", "example": 10 } } diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index 75b85d89d9..82e6fdb88f 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -51,9 +51,10 @@ 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. ' + - 'Omit to fetch every series.'; + '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().positive().optional(); +const seriesLimitSchema = z.number().int().nonnegative().optional(); const numberTileColorDescription = 'Static color for the displayed number, as a palette token such as ' + @@ -661,12 +662,12 @@ const mcpPieTileSchema = mcpTileLayoutSchema.extend({ limit: z .number() .int() - .positive() + .nonnegative() .optional() .describe( 'Maximum number of slices (SQL LIMIT). Without a custom `orderBy`, keeps ' + 'the top-N groups by the aggregated value, descending; with an `orderBy` ' + - 'keeps the first N in that order. Omit to fetch all groups.', + 'keeps the first N in that order. Omit or set 0 to fetch all groups.', ), }), }); @@ -703,12 +704,12 @@ const mcpCategoricalBarTileSchema = mcpTileLayoutSchema.extend({ limit: z .number() .int() - .positive() + .nonnegative() .optional() .describe( 'Maximum number of bars (SQL LIMIT). Without a custom `orderBy`, keeps ' + 'the top-N groups by the aggregated value, descending; with an `orderBy` ' + - 'keeps the first N in that order. Omit to fetch all groups.', + 'keeps the first N in that order. Omit or set 0 to fetch all groups.', ), }), }); diff --git a/packages/api/src/routers/external-api/v2/dashboards.ts b/packages/api/src/routers/external-api/v2/dashboards.ts index f3d92bed45..5e868f85ca 100644 --- a/packages/api/src/routers/external-api/v2/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/dashboards.ts @@ -631,8 +631,11 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * default: false * seriesLimit: * type: integer - * minimum: 1 - * description: Maximum number of series rendered (top-N by value). Omit for no limit. + * 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. * example: 5 * * BarBuilderChartConfig: @@ -683,8 +686,11 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * description: Number formatting options for displayed values. * seriesLimit: * type: integer - * minimum: 1 - * description: Maximum number of series rendered (top-N by value). Omit for no limit. + * 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. * example: 5 * * TableBuilderChartConfig: @@ -832,12 +838,12 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * description: Number formatting options for displayed values. * limit: * type: integer - * minimum: 1 + * minimum: 0 * description: > * Maximum number of slices (SQL LIMIT). Without a custom "orderBy" * the query keeps the groups with the largest aggregated values; * with an "orderBy" it keeps the first slices in that order. Omit - * to fetch all groups. + * or set 0 to fetch all groups. * example: 10 * * CategoricalBarBuilderChartConfig: @@ -885,12 +891,12 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * description: Number formatting options for displayed values. * limit: * type: integer - * minimum: 1 + * minimum: 0 * description: > * Maximum number of bars (SQL LIMIT). Without a custom "orderBy" * the query keeps the groups with the largest aggregated values; - * with an "orderBy" it keeps the first bars in that order. Omit to - * fetch all groups. + * with an "orderBy" it keeps the first bars in that order. Omit or + * set 0 to fetch all groups. * example: 10 * * HeatmapSelectItem: 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 29618d8bb3..84d8bba251 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 @@ -399,4 +399,50 @@ describe('convertToExternalDashboard orphan-ref heal', () => { const ext = convertToExternalDashboard(doc); expect(ext.tiles.map(t => t.id)).toEqual(['normal-tile']); }); + + // seriesLimit is three-state (matching the internal schema): omitted = + // default cap, 0 = unlimited, positive N = top-N. All three must survive a + // GET->PUT round-trip: 0 and N pass through, null maps to absent (the + // default-cap state). + describe.each([DisplayType.Line, DisplayType.StackedBar])( + 'seriesLimit serialization for %s tiles', + displayType => { + function readSeriesLimit(seriesLimit: number | null): unknown { + const doc = makeDoc({ + tiles: [ + makeTile({ + id: 'series-limit-tile', + config: { + displayType, + source: new mongoose.Types.ObjectId().toString(), + name: 'Series limit tile', + select: [{ aggFn: 'count', valueExpression: '' }], + where: '', + seriesLimit, + }, + }), + ], + }); + // Round-trip through JSON to observe what the wire body actually + // carries: `undefined` fields are dropped, so an omitted seriesLimit + // reads back as `undefined` here. + const wire = JSON.parse( + JSON.stringify(convertToExternalDashboard(doc)), + ); + return wire.tiles[0].config.seriesLimit; + } + + it('round-trips 0 (unlimited) rather than dropping it', () => { + expect(readSeriesLimit(0)).toBe(0); + }); + + it('emits seriesLimit as absent when stored as null (default cap)', () => { + expect(readSeriesLimit(null)).toBeUndefined(); + }); + + it('passes a positive seriesLimit through unchanged', () => { + expect(readSeriesLimit(25)).toBe(25); + }); + }, + ); }); 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 42522b0abe..aec470f26f 100644 --- a/packages/api/src/routers/external-api/v2/utils/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/utils/dashboards.ts @@ -299,6 +299,8 @@ const convertToExternalTileChartConfig = ( : [DEFAULT_SELECT_ITEM], compareToPreviousPeriod: config.compareToPreviousPeriod, numberFormat: config.numberFormat, + // Three-state passthrough: 0 (unlimited) and positive N round-trip; + // null/undefined map to absent (the default-cap state). seriesLimit: config.seriesLimit ?? undefined, }; case DisplayType.StackedBar: @@ -316,6 +318,8 @@ const convertToExternalTileChartConfig = ( ? config.select.map(convertToExternalSelectItem) : [DEFAULT_SELECT_ITEM], numberFormat: config.numberFormat, + // Three-state passthrough: 0 (unlimited) and positive N round-trip; + // null/undefined map to absent (the default-cap state). seriesLimit: config.seriesLimit ?? undefined, }; case DisplayType.Number: @@ -353,6 +357,8 @@ const convertToExternalTileChartConfig = ( groupBy: stringValueOrDefault(config.groupBy, undefined), orderBy: stringValueOrDefault(config.orderBy, undefined), numberFormat: config.numberFormat, + // Three-state passthrough: 0 (unlimited) and positive N round-trip; + // null/undefined map to absent (the default-cap state). limit: config.seriesLimit ?? undefined, }; case DisplayType.Bar: @@ -365,6 +371,8 @@ const convertToExternalTileChartConfig = ( groupBy: stringValueOrDefault(config.groupBy, undefined), orderBy: stringValueOrDefault(config.orderBy, undefined), numberFormat: config.numberFormat, + // Three-state passthrough: 0 (unlimited) and positive N round-trip; + // null/undefined map to absent (the default-cap state). limit: config.seriesLimit ?? undefined, }; case DisplayType.Table: diff --git a/packages/api/src/utils/zod.ts b/packages/api/src/utils/zod.ts index 756090b69c..41fe9f7429 100644 --- a/packages/api/src/utils/zod.ts +++ b/packages/api/src/utils/zod.ts @@ -268,7 +268,9 @@ const externalDashboardLineChartConfigSchema = displayType: z.literal('line'), compareToPreviousPeriod: z.boolean().optional(), fitYAxisToData: z.boolean().optional(), - seriesLimit: z.number().int().positive().optional(), + // 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(), }); const externalDashboardLineRawSqlChartConfigSchema = @@ -283,7 +285,9 @@ const externalDashboardLineRawSqlChartConfigSchema = const externalDashboardBarChartConfigSchema = externalDashboardTimeChartConfigSchema.extend({ displayType: z.literal('stacked_bar'), - seriesLimit: z.number().int().positive().optional(), + // 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(), }); const externalDashboardBarRawSqlChartConfigSchema = @@ -375,7 +379,8 @@ const externalDashboardPieChartConfigSchema = z.object({ groupBy: z.string().max(10000).optional(), orderBy: z.string().max(10000).optional(), numberFormat: NumberFormatSchema.optional(), - limit: z.number().int().positive().optional(), + // Three-state: omitted = default cap, 0 = unlimited, positive N = top-N. + limit: z.number().int().nonnegative().optional(), }); const externalDashboardCategoricalBarChartConfigSchema = z.object({ @@ -385,7 +390,8 @@ const externalDashboardCategoricalBarChartConfigSchema = z.object({ groupBy: z.string().max(10000).optional(), orderBy: z.string().max(10000).optional(), numberFormat: NumberFormatSchema.optional(), - limit: z.number().int().positive().optional(), + // Three-state: omitted = default cap, 0 = unlimited, positive N = top-N. + limit: z.number().int().nonnegative().optional(), }); // Heatmap charts use a dedicated select item schema because they carry the diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index d9e2ad8a70..cded550780 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -18,6 +18,7 @@ import { convertToTableChartConfig, getAlignedDateRange, Granularity, + hasPositiveSeriesLimit, } from '@hyperdx/common-utils/dist/core/utils'; import { isBuilderChartConfig } from '@hyperdx/common-utils/dist/guards'; import { @@ -38,7 +39,10 @@ import { notifications } from '@mantine/notifications'; import DateRangeIndicator from './components/charts/DateRangeIndicator'; import { MVOptimizationExplanationResult } from './hooks/useMVOptimizationExplanation'; -import { DEFAULT_SERIES_LIMIT } from './defaults'; +import { + DEFAULT_SERIES_LIMIT, + MAX_RENDERED_TIME_CHART_SERIES, +} from './defaults'; import { getMetricNameSql } from './otelSemanticConventions'; import { AggFn, TableChartSeries, TimeChartSeries } from './types'; import { NumberFormat } from './types'; @@ -110,13 +114,15 @@ export const MAX_TIME_CHART_SERIES = DEFAULT_SERIES_LIMIT; export function convertToTimeChartConfig( config: ChartConfigWithDateRange, ): ChartConfigWithDateRange { - // Series capping is opt-in per tile via the chart's Display Settings; when - // unset, no __hdx_series_limit CTE is emitted and every series is fetched. - const seriesLimit = isBuilderChartConfig(config) - ? config.seriesLimit != null - ? Math.max(1, config.seriesLimit) - : undefined - : undefined; + // Builder group-by charts emit the __hdx_series_limit CTE only for a positive + // seriesLimit. null/undefined (default) and 0 (explicitly unlimited) both + // skip the CTE and fetch every series; the client-side render cap in + // formatResponseForTimeChart then applies the default/opt-out behavior + // (mirrors resolveRenderedSeriesCap on the SQL side). + const seriesLimit = + isBuilderChartConfig(config) && hasPositiveSeriesLimit(config.seriesLimit) + ? config.seriesLimit + : undefined; const granularity = getTimeChartGranularity( config.granularity, @@ -611,55 +617,83 @@ function addResponseToFormattedData({ const isSingleValueColumn = valueColumns.length === 1; const hasGroupColumns = groupColumns.length > 0; - for (const row of data) { - const date = new Date(row[timestampColumn.name]); + // Hoist per-row-loop invariants: this runs once per row × value column, + // hundreds of thousands of times on a high-cardinality group-by. + const groupColumnNames = groupColumns.map(g => g.name); + const valueColumnNames = valueColumns.map(v => v.name); + // Single value column + group-by simplifies the key to just the group. + const omitValueColumnInKey = isSingleValueColumn && hasGroupColumns; + const applyLogLevelColor = firstGroupColumnIsLogLevel(source, groupColumns); + const timestampColumnName = timestampColumn.name; + const offsetSeconds = isPreviousPeriod ? previousPeriodOffsetSeconds : 0; + + // A time chart has very few distinct bucket timestamps (one per granularity + // step) but potentially hundreds of thousands of rows, so `new Date(...)` + // parsing per row dominated the transform. Cache the parsed epoch-second + // bucket per raw timestamp value — same input always yields the same result, + // so this is behavior-preserving regardless of the value's format. + const tsSecondsByRaw = new Map(); - // Previous period data needs to be shifted forward to align with current period - const offsetSeconds = isPreviousPeriod ? previousPeriodOffsetSeconds : 0; - const ts = Math.round(date.getTime() / 1000 + offsetSeconds); + for (const row of data) { + const rawTs = row[timestampColumnName]; + let ts = tsSecondsByRaw.get(rawTs); + if (ts === undefined) { + ts = Math.round(new Date(rawTs).getTime() / 1000 + offsetSeconds); + tsSecondsByRaw.set(rawTs, ts); + } - for (const valueColumn of valueColumns) { - let tsBucket = tsBucketMap.get(ts); - if (tsBucket == null) { - tsBucket = { [timestampColumn.name]: ts }; - tsBucketMap.set(ts, tsBucket); - } + let tsBucket = tsBucketMap.get(ts); + if (tsBucket == null) { + tsBucket = { [timestampColumnName]: ts }; + tsBucketMap.set(ts, tsBucket); + } - const currentPeriodKey = [ - // Simplify the display name if there's only one series and a group by - ...(isSingleValueColumn && hasGroupColumns ? [] : [valueColumn.name]), - ...groupColumns.map(g => { - const v = row[g.name]; - return typeof v === 'object' && v !== null ? JSON.stringify(v) : v; - }), - ].join(ChartKeyJoiner); - const previousPeriodKey = `${currentPeriodKey}${PreviousPeriodSuffix}`; - const keyName = isPreviousPeriod ? previousPeriodKey : currentPeriodKey; + // Group key parts, built once per row and shared across value columns. + // Array.join renders null/undefined as '' (matches the prior behavior). + const groupKeyParts = groupColumnNames.map(name => { + const v = row[name]; + return typeof v === 'object' && v !== null ? JSON.stringify(v) : v; + }); + const groupKeyPart = groupKeyParts.join(ChartKeyJoiner); + + for (const valueColumnName of valueColumnNames) { + const currentPeriodKey = omitValueColumnInKey + ? groupKeyPart + : hasGroupColumns + ? [valueColumnName, ...groupKeyParts].join(ChartKeyJoiner) + : valueColumnName; + const keyName = isPreviousPeriod + ? `${currentPeriodKey}${PreviousPeriodSuffix}` + : currentPeriodKey; // UInt64 are returned as strings, we'll convert to number // and accept a bit of floating point error - const rawValue = row[valueColumn.name]; + const rawValue = row[valueColumnName]; const value = typeof rawValue === 'number' ? rawValue : Number.parseFloat(rawValue); // Mutate the existing bucket object to avoid repeated large object copies tsBucket[keyName] = value; - // Special handling for log level / trace severity colors - let color: string | undefined = undefined; - if (firstGroupColumnIsLogLevel(source, groupColumns)) { - color = logLevelColor(row[groupColumns[0].name]); + // Build the LineData entry once per key (not once per row): the object + // churn was the dominant cost on high-cardinality group-bys. Only the + // log-level color is row-dependent, so refresh just that on later rows. + const existing = lineDataMap[keyName]; + if (existing == null) { + lineDataMap[keyName] = { + dataKey: keyName, + currentPeriodKey, + previousPeriodKey: `${currentPeriodKey}${PreviousPeriodSuffix}`, + displayName: keyName, + valueColumnName, + color: applyLogLevelColor + ? logLevelColor(row[groupColumnNames[0]]) + : undefined, + isDashed: isPreviousPeriod, + }; + } else if (applyLogLevelColor) { + existing.color = logLevelColor(row[groupColumnNames[0]]); } - - lineDataMap[keyName] = { - dataKey: keyName, - currentPeriodKey, - previousPeriodKey, - displayName: keyName, - valueColumnName: valueColumn.name, - color, - isDashed: isPreviousPeriod, - }; } } } @@ -675,6 +709,7 @@ export function formatResponseForTimeChart({ source, hiddenSeries = [], previousPeriodOffsetSeconds = 0, + maxSeries = MAX_RENDERED_TIME_CHART_SERIES, }: { dateRange: [Date, Date]; granularity?: SQLInterval; @@ -684,6 +719,12 @@ export function formatResponseForTimeChart({ source?: TSource; hiddenSeries?: string[]; previousPeriodOffsetSeconds?: number; + /** + * Render cap for the number of series. Defaults to + * MAX_RENDERED_TIME_CHART_SERIES; pass Number.POSITIVE_INFINITY to render + * every series (the "load all" escape hatch behind the hidden-series notice). + */ + maxSeries?: number; }) { const meta = currentPeriodResponse.meta; @@ -737,13 +778,148 @@ export function formatResponseForTimeChart({ } const logLevelColorOrder = getLogLevelColorOrder(); - const sortedLineData = Object.values(lineDataMap).sort((a, b) => { + let sortedLineData = Object.values(lineDataMap).sort((a, b) => { return ( logLevelColorOrder.findIndex(color => color === a.color) - logLevelColorOrder.findIndex(color => color === b.color) ); }); + // Cap materialized series to protect browser memory: high-cardinality + // group-bys (esp. raw SQL, which has no server-side limit) can return tens of + // thousands of series while only a handful are drawn. Keep the top `maxSeries` + // by peak value; drop and count the rest. The cap counts LOGICAL series + // (grouped by `currentPeriodKey`) so a comparison chart's current/previous + // pair is kept or dropped together, not orphaned by a flat entry-list slice. + let hiddenSeriesCount = 0; + + // The cap operates on the group-by GROUP, not on each rendered series. A + // single group can yield several series that must be kept or dropped together: + // - the current + previous-period pair in comparison mode (same + // currentPeriodKey, distinguished by isDashed), and + // - one series per value column when a chart plots multiple aggregations + // (e.g. avg + max), which the key builder prefixes with valueColumnName. + // Ranking each of those independently would let, say, a large-magnitude `max` + // column evict every `avg` series, or a previous-only line evict a current + // one. Derive a group identity by stripping the leading value-column segment + // from currentPeriodKey so all series of a group share one rankable key. + const groupKeyByDataKey = new Map(); + const logicalSeriesKeys: string[] = []; + const seenLogicalKeys = new Set(); + // Groups that have a current-period (non-dashed) entry. In comparison mode the + // current and previous periods are separate queries whose kept sets can + // differ. Current-period groups get priority when the cap trips (see the + // selection below), so a previous-only group can't evict a current-period + // series — but previous-only groups still count toward the total cap. + const currentPeriodGroupKeys = new Set(); + // The value-column prefix is only added to currentPeriodKey when a chart has + // BOTH multiple value columns AND group columns (see addResponseToFormattedData: + // omitValueColumnInKey). In every other shape the key has no such prefix, so + // stripping a leading ` · ` would wrongly collapse a group + // whose own value merely starts with that text. Only strip when the builder + // actually added the prefix. + const keyHasValueColumnPrefix = + valueColumns.length > 1 && groupColumns.length > 0; + const groupKeyOf = (line: LineDataWithOptionalColor): string => { + if (!keyHasValueColumnPrefix) { + return line.currentPeriodKey; + } + const prefix = `${line.valueColumnName}${ChartKeyJoiner}`; + return line.currentPeriodKey.startsWith(prefix) + ? line.currentPeriodKey.slice(prefix.length) + : line.currentPeriodKey; + }; + for (const line of sortedLineData) { + const groupKey = groupKeyOf(line); + groupKeyByDataKey.set(line.dataKey, groupKey); + if (!seenLogicalKeys.has(groupKey)) { + seenLogicalKeys.add(groupKey); + logicalSeriesKeys.push(groupKey); + } + if (!line.isDashed) { + currentPeriodGroupKeys.add(groupKey); + } + } + + // The cap bounds the TOTAL number of logical groups materialized, so a + // comparison chart whose current and previous result sets are disjoint can't + // exceed maxSeries by keeping every previous-only group on top of the + // current-period top-N. Current-period groups still take priority: they're + // ranked and slotted first, then any remaining slots go to previous-only + // groups (also by peak). This keeps a current-period series from being + // evicted by a higher-peak previous-only one while still honoring the cap. + if (logicalSeriesKeys.length > maxSeries) { + hiddenSeriesCount = logicalSeriesKeys.length - maxSeries; + + // Peak absolute value per logical group. Iterate only each bucket's + // populated cells so sparse results cost O(populated cells), not + // O(buckets * series). + const peakByGroup = new Map(); + for (const tsBucket of tsBucketMap.values()) { + for (const [key, raw] of Object.entries(tsBucket)) { + if (typeof raw !== 'number' || !Number.isFinite(raw)) { + continue; + } + const groupKey = groupKeyByDataKey.get(key); + if (groupKey == null) { + continue; + } + const mag = Math.abs(raw); + const prev = peakByGroup.get(groupKey); + if (prev == null || mag > prev) { + peakByGroup.set(groupKey, mag); + } + } + } + + // Rank by peak desc; index tiebreak preserves log-level color ordering. + const byPeakThenIndex = ( + a: { groupKey: string; index: number }, + b: { groupKey: string; index: number }, + ) => { + const diff = + (peakByGroup.get(b.groupKey) ?? 0) - (peakByGroup.get(a.groupKey) ?? 0); + return diff !== 0 ? diff : a.index - b.index; + }; + const currentRanked = logicalSeriesKeys + .map((groupKey, index) => ({ groupKey, index })) + .filter(({ groupKey }) => currentPeriodGroupKeys.has(groupKey)) + .sort(byPeakThenIndex); + const previousOnlyRanked = logicalSeriesKeys + .map((groupKey, index) => ({ groupKey, index })) + .filter(({ groupKey }) => !currentPeriodGroupKeys.has(groupKey)) + .sort(byPeakThenIndex); + + // Current-period groups fill slots first; previous-only groups take any + // remainder. Slice the concatenation to the cap so the total is bounded. + const keptGroups = new Set( + [...currentRanked, ...previousOnlyRanked] + .slice(0, maxSeries) + .map(({ groupKey }) => groupKey), + ); + + // Keep every entry of a surviving group: its current + previous-period + // pair AND every value column, since keptGroups holds group identities. + const keptKeys = new Set( + sortedLineData + .filter(line => keptGroups.has(groupKeyOf(line))) + .map(line => line.dataKey), + ); + + sortedLineData = sortedLineData.filter(line => + keptGroups.has(groupKeyOf(line)), + ); + + // Prune dropped keys from every bucket so graphResults stays small. + for (const tsBucket of tsBucketMap.values()) { + for (const key of Object.keys(tsBucket)) { + if (key !== timestampColumn.name && !keptKeys.has(key)) { + delete tsBucket[key]; + } + } + } + } + if (generateEmptyBuckets && granularity != null) { const generatedTsBuckets = timeBucketByGranularity( dateRange[0], @@ -785,6 +961,14 @@ export function formatResponseForTimeChart({ const sortedLineDataWithColors = setLineColors(sortedLineData); + // Count of LOGICAL groups actually rendered, in the same unit as + // hiddenSeriesCount — so a comparison chart (current + previous entries per + // group) or a multi-value-column chart (one entry per value column per group) + // isn't multiply counted in the hidden-series notice. + const renderedSeriesCount = new Set( + sortedLineDataWithColors.map(line => groupKeyOf(line)), + ).size; + return { graphResults, timestampColumn, @@ -792,6 +976,8 @@ export function formatResponseForTimeChart({ groupColumns: groupColumns.map(g => g.name), valueColumns: valueColumns.map(v => v.name), isSingleValueColumn, + hiddenSeriesCount, + renderedSeriesCount, }; } diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 421e4f8351..d06e89792f 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -58,6 +58,12 @@ import styles from '@styles/HDXLineChart.module.scss'; const MAX_LEGEND_ITEMS = 4; +// Max rows rendered in a series tooltip (hover and pinned). Each row mounts a +// DOM node (the pinned one also a Mantine Tooltip), so an uncapped busy bucket +// was a jank source; the rest collapse into a "+N more" line (see +// getVisibleTooltipRows). Exported so the pinned tooltip shares the cap. +export const MAX_TOOLTIP_ROWS = 20; + // Vertical pixel distance within which a series' line counts as "near" the // cursor for tooltip highlighting. Beyond this, no row is emphasized so the // tooltip is not misleading when the pointer is in empty space. @@ -209,41 +215,59 @@ const HDXLineChartTooltip = withErrorBoundary( } : {}; + // Copy before sorting: Recharts 3 freezes the payload, so an in-place + // sort throws "this object has been frozen". + const sortedPayload = [...typedPayload].sort( + (a: TooltipPayload, b: TooltipPayload) => b.value - a.value, + ); + + // Cap how many rows are rendered per frame (see getVisibleTooltipRows). + const { rows: visiblePayload, hiddenCount: hiddenRowCount } = + getVisibleTooltipRows( + sortedPayload, + nearestSeriesKey, + MAX_TOOLTIP_ROWS, + ); + return (
- {/* Copy before sorting: Recharts 3 freezes the payload, so an - in-place sort throws "this object has been frozen". */} - {[...payload] - .sort((a: TooltipPayload, b: TooltipPayload) => b.value - a.value) - .map((p: TooltipPayload) => { - const previousKey = lineDataMap[p.dataKey]?.previousPeriodKey; - const isPreviousPeriod = previousKey === p.dataKey; - const previousPayload = - !isPreviousPeriod && previousKey - ? payloadByKey.get(previousKey) - : undefined; - const valueColumnName = - lineDataMap[p.dataKey]?.valueColumnName ?? p.dataKey; - const numberFormatForKey = - numberFormatByKey.get(valueColumnName) ?? numberFormat; + {visiblePayload.map((p: TooltipPayload) => { + const previousKey = lineDataMap[p.dataKey]?.previousPeriodKey; + const isPreviousPeriod = previousKey === p.dataKey; + const previousPayload = + !isPreviousPeriod && previousKey + ? payloadByKey.get(previousKey) + : undefined; + const valueColumnName = + lineDataMap[p.dataKey]?.valueColumnName ?? p.dataKey; + const numberFormatForKey = + numberFormatByKey.get(valueColumnName) ?? numberFormat; - return ( - - ); - })} + return ( + + ); + })} + {hiddenRowCount > 0 && ( +
+ +{hiddenRowCount.toLocaleString()} more +
+ )}
); @@ -462,6 +486,20 @@ export type ActiveClickPayload = { /** Series label shown in the legend, tooltip, and line `name`. */ const getSeriesDisplayName = (ld: LineData) => ld.displayName || ld.dataKey; +/** + * Stable, CSS-safe class for a series' , unique per chart (`id`) and + * series (`dataKey`). Lets the nearest-cursor emphasis target one line via CSS + * without changing any prop (which would rebuild every line on hover). + */ +const seriesClassName = (id: string, dataKey: string) => + `hdx-series-${id}-${dataKey.replace(/[^a-zA-Z0-9_-]/g, '_')}`; + +// The subset of recharts' loosely-typed chart mouse-event `state` we read. +type ChartMouseState = { + activeLabel?: string | number; + activeCoordinate?: { x?: number; y?: number }; +}; + /** Normalize a chart event's active label (number | string) to a string. */ const getActiveLabel = (state?: { activeLabel?: string | number; @@ -481,7 +519,11 @@ export function buildActiveClickSeries( if (activeRow == null) return []; return visibleLineData.flatMap(ld => { const value = activeRow[ld.dataKey]; - if (typeof value !== 'number') return []; + // Exclude non-finite values (NaN/±Infinity) — e.g. a ratio chart's + // zero-denominator bucket yields NaN. The tooltip already drops these + // (ChartSeriesTooltip filters on Number.isFinite), and admitting them here + // would also break the sameActiveClickSeries equality guard (NaN !== NaN). + if (typeof value !== 'number' || !Number.isFinite(value)) return []; const isPreviousPeriod = ld.previousPeriodKey === ld.dataKey; // Pair each current-period series with its previous-period value for the // percent-change chip. Only current-period rows carry a comparison. @@ -504,6 +546,35 @@ export function buildActiveClickSeries( }); } +/** + * Shallow structural equality for two click-frozen payloads, used to decide + * whether an open pin's snapshot needs rebuilding. Compares the drawn set + * (length + per-row dataKey) and the value/previousValue at the pinned bucket; + * a change in any means the tooltip's rows or its "+N more" overflow would + * differ. Cheap and order-sensitive — `buildActiveClickSeries` derives both + * sides from the same `tooltipLineData` ordering, so positions stay aligned. + * + * Uses `Object.is` for the numeric fields so a `NaN` value compares equal to + * itself (a plain `!==` would report NaN-holding snapshots as perpetually + * changed and drive the resync effect into an infinite update loop). + */ +export function sameActiveClickSeries( + a: ActiveClickSeries[] | undefined, + b: ActiveClickSeries[], +): boolean { + if (a == null || a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) { + if ( + a[i].dataKey !== b[i].dataKey || + !Object.is(a[i].value, b[i].value) || + !Object.is(a[i].previousValue, b[i].previousValue) + ) { + return false; + } + } + return true; +} + /** * The series actually drawn on the chart. Without a selection, the first * HARD_LINES_LIMIT of lineData. With a selection (legend isolate, checkbox @@ -529,14 +600,61 @@ function hasSeriesSelection( export function getVisibleLineData( lineData: LineData[], selectedSeriesNames: Set | undefined, +): LineData[] { + return getSelectedLineData(lineData, selectedSeriesNames).slice( + 0, + HARD_LINES_LIMIT, + ); +} + +/** + * The series that survive the legend/table selection, WITHOUT the + * HARD_LINES_LIMIT draw cap. Same selection semantics as getVisibleLineData + * (selection applied first), so an explicitly chosen series is always kept. + * The pinned tooltip's rows derive from this so the "load all series" escape + * hatch can list series that were materialized but not drawn — the draw cap + * (getVisibleLineData) exists to keep the chart readable/fast, not to bound the + * scrollable drill-down list. Exported for unit testing. + */ +export function getSelectedLineData( + lineData: LineData[], + selectedSeriesNames: Set | undefined, ): LineData[] { const hasSelection = hasSeriesSelection(selectedSeriesNames); if (hasSelection) { - return lineData - .filter(ld => selectedSeriesNames.has(getSeriesDisplayName(ld))) - .slice(0, HARD_LINES_LIMIT); + return lineData.filter(ld => + selectedSeriesNames.has(getSeriesDisplayName(ld)), + ); + } + return lineData; +} + +/** + * The series-tooltip rows to render (hover or pinned). `rows` must be sorted by + * value descending. Keeps the top `limit`; if the cursor-nearest series ranks + * past it, that series replaces the lowest kept row so it still shows (pass + * `undefined` for the pinned tooltip, which has no cursor). `hiddenCount` + * drives the "+N more" line. Exported for unit testing. + */ +export function getVisibleTooltipRows( + rows: T[], + nearestSeriesKey: string | undefined, + limit: number, +): { rows: T[]; hiddenCount: number } { + if (rows.length <= limit) { + return { rows, hiddenCount: 0 }; + } + const visible = rows.slice(0, limit); + if ( + nearestSeriesKey != null && + !visible.some(r => r.dataKey === nearestSeriesKey) + ) { + const nearest = rows.find(r => r.dataKey === nearestSeriesKey); + if (nearest != null) { + visible[visible.length - 1] = nearest; + } } - return lineData.slice(0, HARD_LINES_LIMIT); + return { rows: visible, hiddenCount: rows.length - visible.length }; } const StackedBarWithOverlap = (props: BarProps) => { @@ -642,6 +760,7 @@ export function collectMemoChartGradientHexes( export const MemoChart = memo(function MemoChart({ graphResults, setIsClickActive, + refreshClickActive, isClickActive, dateRange, lineData, @@ -666,6 +785,12 @@ export const MemoChart = memo(function MemoChart({ }: { graphResults: any[]; setIsClickActive: (v: ActiveClickPayload | undefined) => void; + /** + * In-place refresh of the open pin's frozen snapshot (rows only), without the + * cross-chart pin-dismiss broadcast setIsClickActive performs. Used by the + * resync effect. Falls back to setIsClickActive when not provided. + */ + refreshClickActive?: (v: ActiveClickPayload | undefined) => void; isClickActive: ActiveClickPayload | undefined; dateRange: [Date, Date] | Readonly<[Date, Date]>; lineData: LineData[]; @@ -740,16 +865,16 @@ export const MemoChart = memo(function MemoChart({ [lineData, selectedSeriesNames], ); - const lines = useMemo(() => { - // When a series is nearest the cursor (only meaningful with more than one - // line shown), thicken its line and fade the others so the eye lands on - // the same series the tooltip bolds. Mirrors the legend's selected style - // (thicker stroke) with a gentle fade that keeps the rest readable. - const hasNearest = - visibleLineData.length > 1 && - nearestSeriesKey != null && - visibleLineData.some(ld => ld.dataKey === nearestSeriesKey); + // Series for the pinned tooltip's drill-down list: selection applied but NOT + // clamped to HARD_LINES_LIMIT, so "load all series" reveals series that were + // materialized (up to the render cap) yet not drawn. Kept separate from + // visibleLineData so the drawn chart stays bounded at HARD_LINES_LIMIT. + const tooltipLineData = useMemo( + () => getSelectedLineData(lineData, selectedSeriesNames), + [lineData, selectedSeriesNames], + ); + const lines = useMemo(() => { return visibleLineData.map(ld => { const key = ld.dataKey; const color = ld.color; @@ -775,31 +900,40 @@ export const MemoChart = memo(function MemoChart({ type="monotone" stroke={color} fillOpacity={1} - strokeWidth={hasNearest && key === nearestSeriesKey ? 2.5 : undefined} - strokeOpacity={ - hasNearest && key !== nearestSeriesKey ? 0.5 : undefined - } + // Stable per-series class so the nearest-cursor emphasis can be + // applied via CSS (see nearestSeriesStyle) rather than by changing + // these props — a prop change here rebuilds every on hover. + className={seriesClassName(id, key)} activeDot={} - {...(isHovered - ? { fill: 'none', strokeDasharray } - : { - fill: `url(#time-chart-lin-grad-${id}-${color?.replace('#', '').toLowerCase()})`, - strokeDasharray, - })} + // Fill is always the gradient. Hiding it on hover is a CSS class + // toggle (styles.chartHovered), not a prop swap — swapping it here + // re-created every on each hover enter/leave (hover churn). + fill={`url(#time-chart-lin-grad-${id}-${color?.replace('#', '').toLowerCase()})`} + strokeDasharray={strokeDasharray} name={seriesName} isAnimationActive={false} connectNulls /> ); }); - }, [ - visibleLineData, - displayType, - id, - isHovered, - nearestSeriesKey, - captureActivePointY, - ]); + }, [visibleLineData, displayType, id, captureActivePointY]); + + // Nearest-cursor emphasis (thicken the nearest line, fade the rest) applied + // via a tiny scoped + ); + }, [nearestSeriesKey, visibleLineData.length, id]); const yAxisDomain: AxisDomain = useMemo(() => { const hasSelection = hasSeriesSelection(selectedSeriesNames); @@ -889,7 +1023,9 @@ export const MemoChart = memo(function MemoChart({ const activeRow = graphResults.find( row => String(row[timestampKey]) === activeLabel, ); - const activePayload = buildActiveClickSeries(visibleLineData, activeRow); + // Build from tooltipLineData (uncapped), not visibleLineData: the pinned + // drill-down list may show more series than are drawn. + const activePayload = buildActiveClickSeries(tooltipLineData, activeRow); if (activePayload.length === 0) { return undefined; } @@ -902,9 +1038,44 @@ export const MemoChart = memo(function MemoChart({ activePayload, }; }, - [graphResults, timestampKey, visibleLineData], + [graphResults, timestampKey, tooltipLineData], ); + // Keep the pinned tooltip's frozen snapshot in sync with its series set. + // The snapshot's rows are captured once at click time from `tooltipLineData`; + // when that set changes underneath an open pin — most notably after "load all + // series" materializes the previously-capped series — the frozen rows (and + // the "+N more" overflow derived from them) would otherwise stay stale, so + // clicking "load all" would leave a phantom "+N more" and never surface the + // newly-loaded rows. Rebuild the rows for the same clicked bucket from the + // current data while preserving the click-time anchor coords. + useEffect(() => { + if (isClickActive == null) return; + const activeRow = graphResults.find( + row => String(row[timestampKey]) === isClickActive.activeLabel, + ); + const nextPayload = buildActiveClickSeries(tooltipLineData, activeRow); + // No numeric value at the pinned bucket anymore (e.g. the series vanished); + // leave the existing snapshot rather than dismissing a still-anchored pin. + if (nextPayload.length === 0) return; + // Only update when the series set actually changed, so ordinary re-renders + // (hover, live-range ticks) don't churn state or reset scroll position. + if (sameActiveClickSeries(isClickActive.activePayload, nextPayload)) return; + // In-place refresh (no cross-chart pin-dismiss broadcast); fall back to + // setIsClickActive when the refresh callback isn't wired. + (refreshClickActive ?? setIsClickActive)({ + ...isClickActive, + activePayload: nextPayload, + }); + }, [ + isClickActive, + graphResults, + timestampKey, + tooltipLineData, + refreshClickActive, + setIsClickActive, + ]); + // Recharts computes bar width from the smallest gap between ticks on a // numerical XAxis. With a single data point there are no gaps, so the // computed width is 0 and bars become invisible. Provide an explicit @@ -1014,6 +1185,216 @@ export const MemoChart = memo(function MemoChart({ return map; }, [lineData]); + // Memoize the tooltip `content` element: recharts re-evaluates it every hover + // frame, so a fresh element each render defeats HDXLineChartTooltip's memo. + // Refs are stable, so only the listed values are deps. + const hoverTooltipContent = useMemo( + () => ( + + ), + [ + fallbackNumberFormat, + tooltipNumberFormatsByKey, + lineDataMap, + previousPeriodOffsetSeconds, + ], + ); + + // Latest values the mouse handlers read, in a ref so the handlers below can + // be stable useCallbacks. Recharts re-runs its event wiring when a handler + // prop's identity changes, so a stable reference avoids that per-render churn. + const handlerStateRef = useRef({ + isClickActive, + highlightStart, + highlightEnd, + dateRange, + onTimeRangeSelect, + }); + // Updated in an effect (not during render); the one-commit lag is harmless + // since these are only read in event handlers, which fire after commit. + useEffect(() => { + handlerStateRef.current = { + isClickActive, + highlightStart, + highlightEnd, + dateRange, + onTimeRangeSelect, + }; + }, [ + isClickActive, + highlightStart, + highlightEnd, + dateRange, + onTimeRangeSelect, + ]); + + const handleMouseEnter = useCallback(() => setIsHovered(true), []); + + const handleMouseLeave = useCallback(() => { + setIsHovered(false); + setNearestSeriesKey(undefined); + setHighlightStart(undefined); + setHighlightEnd(undefined); + mouseDownPosRef.current = null; + }, []); + + const handleMouseDown = useCallback( + (state: ChartMouseState, e?: { nativeEvent?: { clientX?: number } }) => { + // Record the drag start: the active bucket label and a container-relative + // pointer X (always defined, single origin) for measuring drag distance. + const chartX = getContainerX(e?.nativeEvent); + const downLabel = getActiveLabel(state); + if (downLabel != null && chartX != null) { + setHighlightStart(downLabel); + mouseDownPosRef.current = chartX; + } + }, + [getContainerX], + ); + + const handleMouseMove = useCallback( + (state: ChartMouseState) => { + setIsHovered(true); + + const { isClickActive, highlightStart } = handlerStateRef.current; + + // Track which series' line is nearest the cursor so the lines can + // emphasize it. The active dots captured their pixel Y on the prior frame; + // comparing the pointer's chartY picks the nearest line. Skip while a + // click-frozen tooltip is shown, matching the tooltip, and only set state + // when the key changes to keep re-renders rare. + const chartY = state?.activeCoordinate?.y; + const activePointYByKey = activePointYByKeyRef.current; + const nextNearest = + isClickActive == null && activePointYByKey.size > 1 && chartY != null + ? findNearestSeriesKey( + activePointYByKey, + Array.from(activePointYByKey.keys()), + chartY, + NEAREST_SERIES_MAX_DISTANCE_PX, + ) + : undefined; + setNearestSeriesKey(prev => (prev === nextNearest ? prev : nextNearest)); + + const moveLabel = getActiveLabel(state); + if (highlightStart != null && moveLabel != null) { + setHighlightEnd(moveLabel); + setIsClickActive(undefined); // Clear out any click state as we're highlighting + } + }, + [setIsClickActive], + ); + + const handleMouseUp = useCallback( + (state: ChartMouseState, e?: { nativeEvent?: { clientX?: number } }) => { + const MIN_DRAG_DISTANCE = 20; // Minimum horizontal drag distance in pixels + let dragDistance = 0; + + const { highlightStart, highlightEnd, dateRange, onTimeRangeSelect } = + handlerStateRef.current; + + // Measure against the same container-relative origin recorded on mouse + // down so the distance is never skewed or dropped when the pointer maps + // to no data point. + const chartX = getContainerX(e?.nativeEvent); + if (mouseDownPosRef.current != null && chartX != null) { + dragDistance = Math.abs(chartX - mouseDownPosRef.current); + } + + const activeLabel = getActiveLabel(state); + if (activeLabel != null && highlightStart === activeLabel) { + // If it's just a click, don't zoom + setHighlightStart(undefined); + setHighlightEnd(undefined); + mouseDownPosRef.current = null; + } else if ( + highlightStart != null && + highlightEnd != null && + dragDistance >= MIN_DRAG_DISTANCE + ) { + try { + // Remember the range we're zooming away from so "Reset zoom" can + // restore it. Keep the earliest origin across consecutive zooms. + const originStart = dateRange[0]; + const originEnd = dateRange[1]; + setZoomOrigin(prev => prev ?? [originStart, originEnd]); + // The synthetic click after this drag must be swallowed regardless of + // whether a range change follows; onClick consumes and clears this. + suppressNextClickRef.current = true; + // Only tell the [dateRange] effect to preserve zoomOrigin when a + // range change will actually happen; without onTimeRangeSelect the + // range never changes and the effect never runs. + if (onTimeRangeSelect != null) { + justZoomedRef.current = true; + } + // Order the range numerically — the labels are epoch-second strings, + // so a lexicographic compare would misorder values of differing + // digit length. + const startSec = Number(highlightStart); + const endSec = Number(highlightEnd); + const lowSec = Math.min(startSec, endSec); + const highSec = Math.max(startSec, endSec); + onTimeRangeSelect?.( + new Date(lowSec * 1000), + new Date(highSec * 1000), + ); + } catch (err) { + console.error('failed to highlight range', err); + justZoomedRef.current = false; + setZoomOrigin(null); + } + setHighlightStart(undefined); + setHighlightEnd(undefined); + mouseDownPosRef.current = null; + } else { + // Drag was too short, clear the highlight + setHighlightStart(undefined); + setHighlightEnd(undefined); + mouseDownPosRef.current = null; + } + }, + [getContainerX], + ); + + const handleClick = useCallback( + (state: ChartMouseState, e: { stopPropagation: () => void }) => { + // A brush-to-zoom ends with a synthetic click; skip that one click so we + // don't freeze a drill-down tooltip with now-stale, pre-zoom data. + // Consume-and-clear the flag here so a value-equal zoom (which never + // triggers the dateRange effect) can't leave it stuck and suppress every + // later click. + if (suppressNextClickRef.current) { + suppressNextClickRef.current = false; + e.stopPropagation(); + return; + } + const { highlightStart } = handlerStateRef.current; + // Freeze a tooltip at the clicked point. The builder mirrors the series + // actually drawn (legend selection + HARD_LINES_LIMIT). + const clickPayload = + highlightStart == null ? buildActivePayloadFromState(state) : undefined; + if (clickPayload != null) { + setIsClickActive(clickPayload); + // Pinned replaces hover; drop line emphasis to match. + setNearestSeriesKey(undefined); + } else { + // We clicked on the chart but outside of a line + setIsClickActive(undefined); + } + + // TODO: Properly detect clicks outside of the fake tooltip + e.stopPropagation(); + }, + [buildActivePayloadFromState, setIsClickActive], + ); + const xAxisDomain: AxisDomain = useMemo(() => { let startTime = toStartOfInterval(dateRange[0], granularity); let endTime = toStartOfInterval(dateRange[1], granularity); @@ -1052,8 +1433,24 @@ export const MemoChart = memo(function MemoChart({ return (
's fill prop, so the chart's ~N Area + // elements are not re-created on every hover enter/leave. + // `rr-block` tells the HyperDX/rrweb session-replay recorder to capture + // this chart as a placeholder rather than serializing its (very large) + // SVG DOM on every mutation — the dominant session-replay cost on + // high-cardinality dashboards. + className={cx( + 'rr-block', + styles.chartRoot, + isHovered && styles.chartHovered, + )} + // Scopes nearestSeriesStyle to this chart instance. + data-chart-id={id} style={{ position: 'relative', width: '100%', height: '100%' }} > + {nearestSeriesStyle} setIsHovered(true)} - onMouseLeave={() => { - setIsHovered(false); - setNearestSeriesKey(undefined); - - setHighlightStart(undefined); - setHighlightEnd(undefined); - mouseDownPosRef.current = null; - }} - onMouseDown={(state, e) => { - // Record the drag start: the active bucket label and a - // container-relative pointer X (always defined, single origin) for - // measuring drag distance on mouse up. - const chartX = getContainerX(e?.nativeEvent); - const downLabel = getActiveLabel(state); - if (downLabel != null && chartX != null) { - setHighlightStart(downLabel); - mouseDownPosRef.current = chartX; - } - }} - onMouseMove={state => { - setIsHovered(true); - - // Track which series' line is nearest the cursor so the lines can - // emphasize it. The active dots captured their pixel Y on the prior - // frame; comparing the pointer's chartY picks the nearest line. Skip - // while a click-frozen tooltip is shown, matching the tooltip, and - // only set state when the key changes to keep re-renders rare. - const chartY = state?.activeCoordinate?.y; - const activePointYByKey = activePointYByKeyRef.current; - const nextNearest = - isClickActive == null && - activePointYByKey.size > 1 && - chartY != null - ? findNearestSeriesKey( - activePointYByKey, - Array.from(activePointYByKey.keys()), - chartY, - NEAREST_SERIES_MAX_DISTANCE_PX, - ) - : undefined; - setNearestSeriesKey(prev => - prev === nextNearest ? prev : nextNearest, - ); - - const moveLabel = getActiveLabel(state); - if (highlightStart != null && moveLabel != null) { - setHighlightEnd(moveLabel); - setIsClickActive(undefined); // Clear out any click state as we're highlighting - } - }} - onMouseUp={(state, e) => { - const MIN_DRAG_DISTANCE = 20; // Minimum horizontal drag distance in pixels - let dragDistance = 0; - - // Measure against the same container-relative origin recorded on - // mouse down so the distance is never skewed or dropped when the - // pointer maps to no data point. - const chartX = getContainerX(e?.nativeEvent); - if (mouseDownPosRef.current != null && chartX != null) { - dragDistance = Math.abs(chartX - mouseDownPosRef.current); - } - - const activeLabel = getActiveLabel(state); - if (activeLabel != null && highlightStart === activeLabel) { - // If it's just a click, don't zoom - setHighlightStart(undefined); - setHighlightEnd(undefined); - mouseDownPosRef.current = null; - } else if ( - highlightStart != null && - highlightEnd != null && - dragDistance >= MIN_DRAG_DISTANCE - ) { - try { - // Remember the range we're zooming away from so "Reset zoom" can - // restore it. Keep the earliest origin across consecutive zooms. - const originStart = dateRange[0]; - const originEnd = dateRange[1]; - setZoomOrigin(prev => prev ?? [originStart, originEnd]); - // The synthetic click after this drag must be swallowed - // regardless of whether a range change follows; onClick - // consumes and clears this itself. - suppressNextClickRef.current = true; - // Only tell the [dateRange] effect to preserve zoomOrigin when a - // range change will actually happen; without onTimeRangeSelect - // the range never changes and the effect never runs. - if (onTimeRangeSelect != null) { - justZoomedRef.current = true; - } - // Order the range numerically — the labels are epoch-second - // strings, so a lexicographic compare would misorder values of - // differing digit length. - const startSec = Number(highlightStart); - const endSec = Number(highlightEnd); - const lowSec = Math.min(startSec, endSec); - const highSec = Math.max(startSec, endSec); - onTimeRangeSelect?.( - new Date(lowSec * 1000), - new Date(highSec * 1000), - ); - } catch (e) { - console.error('failed to highlight range', e); - justZoomedRef.current = false; - setZoomOrigin(null); - } - setHighlightStart(undefined); - setHighlightEnd(undefined); - mouseDownPosRef.current = null; - } else { - // Drag was too short, clear the highlight - setHighlightStart(undefined); - setHighlightEnd(undefined); - mouseDownPosRef.current = null; - } - }} - onClick={(state, e) => { - // A brush-to-zoom ends with a synthetic click; skip that one click - // so we don't freeze a drill-down tooltip with now-stale, pre-zoom - // data. Consume-and-clear the flag here so a value-equal zoom (which - // never triggers the dateRange effect) can't leave it stuck and - // suppress every later click. - if (suppressNextClickRef.current) { - suppressNextClickRef.current = false; - e.stopPropagation(); - return; - } - // Freeze a tooltip at the clicked point. The builder mirrors the - // series actually drawn (legend selection + HARD_LINES_LIMIT). - const clickPayload = - highlightStart == null - ? buildActivePayloadFromState(state) - : undefined; - if (clickPayload != null) { - setIsClickActive(clickPayload); - // Pinned replaces hover; drop line emphasis to match. - setNearestSeriesKey(undefined); - } else { - // We clicked on the chart but outside of a line - setIsClickActive(undefined); - } - - // TODO: Properly detect clicks outside of the fake tooltip - e.stopPropagation(); - }} + onMouseEnter={handleMouseEnter} + onMouseLeave={handleMouseLeave} + onMouseDown={handleMouseDown} + onMouseMove={handleMouseMove} + onMouseUp={handleMouseUp} + onClick={handleClick} > {/* Gradient defs cover every hex that any fill may reference. @@ -1291,16 +1549,7 @@ export const MemoChart = memo(function MemoChart({ docblock) and escape the chart's bounds near an edge. */} {isClickActive == null && ( - } + content={hoverTooltipContent} portal={typeof document !== 'undefined' ? document.body : null} /> )} diff --git a/packages/app/src/__tests__/ChartUtils.test.ts b/packages/app/src/__tests__/ChartUtils.test.ts index a1ab4cbe8f..4e8c8ab316 100644 --- a/packages/app/src/__tests__/ChartUtils.test.ts +++ b/packages/app/src/__tests__/ChartUtils.test.ts @@ -12,6 +12,10 @@ import { formatResponseForCategoricalChart, formatResponseForTimeChart, } from '@/ChartUtils'; +import { + MAX_RENDERED_TIME_CHART_SERIES, + resolveRenderedSeriesCap, +} from '@/defaults'; import { COLORS } from '@/utils'; // Anchor info/error to concrete hexes rather than `getChartColorInfo()` / @@ -761,6 +765,420 @@ describe('ChartUtils', () => { }, ]); }); + + it('does not cap series when the group count is within the limit', () => { + const groupCount = MAX_RENDERED_TIME_CHART_SERIES; + const data = Array.from({ length: groupCount }, (_, i) => ({ + value: i + 1, + group: `g${i}`, + __hdx_time_bucket: '2025-11-26T11:12:00Z', + })); + const meta = [ + { name: 'value', type: 'Float64' }, + { name: 'group', type: 'String' }, + { name: '__hdx_time_bucket', type: 'DateTime' }, + ]; + + const actual = formatResponseForTimeChart({ + currentPeriodResponse: { data, meta }, + dateRange: [ + new Date('2025-11-26T11:12:00Z'), + new Date('2025-11-26T11:13:00Z'), + ], + granularity: '1 minute', + generateEmptyBuckets: false, + }); + + expect(actual.hiddenSeriesCount).toBe(0); + expect(actual.lineData).toHaveLength(groupCount); + }); + + it('caps high-cardinality group-bys and keeps the highest-peak series', () => { + // One extra group over the cap so exactly one series should be dropped — + // and it should be the lowest-value one (value 0). + const groupCount = MAX_RENDERED_TIME_CHART_SERIES + 1; + const data = Array.from({ length: groupCount }, (_, i) => ({ + // group 0 has the smallest peak, so it is the one that gets dropped. + value: i, + group: `g${i}`, + __hdx_time_bucket: '2025-11-26T11:12:00Z', + })); + const meta = [ + { name: 'value', type: 'Float64' }, + { name: 'group', type: 'String' }, + { name: '__hdx_time_bucket', type: 'DateTime' }, + ]; + + const actual = formatResponseForTimeChart({ + currentPeriodResponse: { data, meta }, + dateRange: [ + new Date('2025-11-26T11:12:00Z'), + new Date('2025-11-26T11:13:00Z'), + ], + granularity: '1 minute', + generateEmptyBuckets: false, + }); + + // Exactly the overflow is hidden and the rendered set is capped. + expect(actual.hiddenSeriesCount).toBe(1); + expect(actual.lineData).toHaveLength(MAX_RENDERED_TIME_CHART_SERIES); + + // The lowest-peak series (value 0) is dropped from both lineData and the + // materialized bucket objects; a high-peak one survives. + const keptKeys = new Set(actual.lineData.map(l => l.dataKey)); + expect(keptKeys.has('g0')).toBe(false); + expect(keptKeys.has(`g${groupCount - 1}`)).toBe(true); + for (const bucket of actual.graphResults) { + expect(bucket).not.toHaveProperty('g0'); + } + }); + + it('renders every series when maxSeries is Infinity (load-all escape hatch)', () => { + const groupCount = MAX_RENDERED_TIME_CHART_SERIES + 50; + const data = Array.from({ length: groupCount }, (_, i) => ({ + value: i, + group: `g${i}`, + __hdx_time_bucket: '2025-11-26T11:12:00Z', + })); + const meta = [ + { name: 'value', type: 'Float64' }, + { name: 'group', type: 'String' }, + { name: '__hdx_time_bucket', type: 'DateTime' }, + ]; + + const actual = formatResponseForTimeChart({ + currentPeriodResponse: { data, meta }, + dateRange: [ + new Date('2025-11-26T11:12:00Z'), + new Date('2025-11-26T11:13:00Z'), + ], + granularity: '1 minute', + generateEmptyBuckets: false, + maxSeries: Number.POSITIVE_INFINITY, + }); + + expect(actual.hiddenSeriesCount).toBe(0); + expect(actual.lineData).toHaveLength(groupCount); + }); + + it('ranks by peak magnitude across all buckets (not just the last)', () => { + // g0 peaks high in bucket 1 then drops to 0; a naive "last value" ranking + // would drop it, but peak-across-buckets must keep it over a flat-low g1. + const meta = [ + { name: 'value', type: 'Float64' }, + { name: 'group', type: 'String' }, + { name: '__hdx_time_bucket', type: 'DateTime' }, + ]; + // Cap to 1 so exactly the single highest-peak series survives. + const data = [ + { value: 999, group: 'g0', __hdx_time_bucket: '2025-11-26T11:12:00Z' }, + { value: 0, group: 'g0', __hdx_time_bucket: '2025-11-26T11:13:00Z' }, + { value: 5, group: 'g1', __hdx_time_bucket: '2025-11-26T11:12:00Z' }, + { value: 5, group: 'g1', __hdx_time_bucket: '2025-11-26T11:13:00Z' }, + ]; + + const actual = formatResponseForTimeChart({ + currentPeriodResponse: { data, meta }, + dateRange: [ + new Date('2025-11-26T11:12:00Z'), + new Date('2025-11-26T11:14:00Z'), + ], + granularity: '1 minute', + generateEmptyBuckets: false, + maxSeries: 1, + }); + + expect(actual.lineData).toHaveLength(1); + expect(actual.lineData[0].dataKey).toBe('g0'); + expect(actual.hiddenSeriesCount).toBe(1); + }); + + it('breaks peak ties deterministically by first-seen (insertion) order', () => { + // Three series with identical peaks; capping to 2 must keep the first two + // encountered (g0, g1) and drop g2 — stable, order-independent of hashing. + const meta = [ + { name: 'value', type: 'Float64' }, + { name: 'group', type: 'String' }, + { name: '__hdx_time_bucket', type: 'DateTime' }, + ]; + const data = [ + { value: 7, group: 'g0', __hdx_time_bucket: '2025-11-26T11:12:00Z' }, + { value: 7, group: 'g1', __hdx_time_bucket: '2025-11-26T11:12:00Z' }, + { value: 7, group: 'g2', __hdx_time_bucket: '2025-11-26T11:12:00Z' }, + ]; + + const actual = formatResponseForTimeChart({ + currentPeriodResponse: { data, meta }, + dateRange: [ + new Date('2025-11-26T11:12:00Z'), + new Date('2025-11-26T11:13:00Z'), + ], + granularity: '1 minute', + generateEmptyBuckets: false, + maxSeries: 2, + }); + + const keptKeys = new Set(actual.lineData.map(l => l.dataKey)); + expect(keptKeys.has('g0')).toBe(true); + expect(keptKeys.has('g1')).toBe(true); + expect(keptKeys.has('g2')).toBe(false); + }); + + it('caps by logical series so comparison pairs are kept together', () => { + // Comparison mode: each group yields a current + previous-period entry + // that share a currentPeriodKey. Capping to 2 must keep 2 *logical* + // series (4 entries: 2 current + 2 previous), not slice the flat 6-entry + // list down to 2 and orphan a current line from its dashed partner. + const PREVIOUS_SUFFIX = ' (previous)'; + const meta = [ + { name: 'value', type: 'Float64' }, + { name: 'group', type: 'String' }, + { name: '__hdx_time_bucket', type: 'DateTime' }, + ]; + const bucket = '2025-11-26T11:12:00Z'; + // g2 has the lowest peak, so it is the logical series that gets dropped. + const currentData = [ + { value: 30, group: 'g0', __hdx_time_bucket: bucket }, + { value: 20, group: 'g1', __hdx_time_bucket: bucket }, + { value: 10, group: 'g2', __hdx_time_bucket: bucket }, + ]; + const previousData = [ + { value: 29, group: 'g0', __hdx_time_bucket: bucket }, + { value: 19, group: 'g1', __hdx_time_bucket: bucket }, + { value: 9, group: 'g2', __hdx_time_bucket: bucket }, + ]; + + const actual = formatResponseForTimeChart({ + currentPeriodResponse: { data: currentData, meta }, + previousPeriodResponse: { data: previousData, meta }, + dateRange: [ + new Date('2025-11-26T11:12:00Z'), + new Date('2025-11-26T11:13:00Z'), + ], + granularity: '1 minute', + generateEmptyBuckets: false, + maxSeries: 2, + }); + + // One logical series (g2 and its previous twin) is hidden. + expect(actual.hiddenSeriesCount).toBe(1); + // renderedSeriesCount counts LOGICAL series (2), not the 4 lineData + // entries (2 groups × current+previous). + expect(actual.renderedSeriesCount).toBe(2); + expect(actual.renderedSeriesCount + actual.hiddenSeriesCount).toBe(3); + + const keptKeys = new Set(actual.lineData.map(l => l.dataKey)); + // Both kept logical series retain BOTH halves of the pair. + expect(keptKeys.has('g0')).toBe(true); + expect(keptKeys.has(`g0${PREVIOUS_SUFFIX}`)).toBe(true); + expect(keptKeys.has('g1')).toBe(true); + expect(keptKeys.has(`g1${PREVIOUS_SUFFIX}`)).toBe(true); + // The dropped logical series is gone entirely (no orphaned half). + expect(keptKeys.has('g2')).toBe(false); + expect(keptKeys.has(`g2${PREVIOUS_SUFFIX}`)).toBe(false); + }); + + it('gives current-period groups priority but still bounds the total by the cap', () => { + // A high-peak previous-only group (g3) must NOT evict a current-period + // series, and previous-only groups must still count toward the cap so a + // disjoint comparison result can't exceed maxSeries. Current: g0,g1,g2; + // previous adds g3 (peak 999). Cap 2 -> current priority keeps g0,g1; + // g2 and g3 both drop; hidden = 4 total groups - 2 = 2. + const PREVIOUS_SUFFIX = ' (previous)'; + const meta = [ + { name: 'value', type: 'Float64' }, + { name: 'group', type: 'String' }, + { name: '__hdx_time_bucket', type: 'DateTime' }, + ]; + const bucket = '2025-11-26T11:12:00Z'; + const currentData = [ + { value: 30, group: 'g0', __hdx_time_bucket: bucket }, + { value: 20, group: 'g1', __hdx_time_bucket: bucket }, + { value: 10, group: 'g2', __hdx_time_bucket: bucket }, + ]; + const previousData = [ + { value: 29, group: 'g0', __hdx_time_bucket: bucket }, + { value: 19, group: 'g1', __hdx_time_bucket: bucket }, + { value: 999, group: 'g3', __hdx_time_bucket: bucket }, + ]; + + const actual = formatResponseForTimeChart({ + currentPeriodResponse: { data: currentData, meta }, + previousPeriodResponse: { data: previousData, meta }, + dateRange: [ + new Date('2025-11-26T11:12:00Z'), + new Date('2025-11-26T11:13:00Z'), + ], + granularity: '1 minute', + generateEmptyBuckets: false, + maxSeries: 2, + }); + + const keptKeys = new Set(actual.lineData.map(l => l.dataKey)); + // Current-period top-2 kept with their previous twins. + expect(keptKeys.has('g0')).toBe(true); + expect(keptKeys.has('g1')).toBe(true); + // g3's peak of 999 did NOT evict a current-period series... + expect(keptKeys.has(`g3${PREVIOUS_SUFFIX}`)).toBe(false); + // ...and the lowest current group is dropped too. + expect(keptKeys.has('g2')).toBe(false); + // Total is bounded: 4 logical groups, cap 2 -> 2 hidden (g2 + g3). + expect(actual.hiddenSeriesCount).toBe(2); + }); + + it('keeps a previous-only group when there is room under the cap', () => { + // Current: g0,g1. Previous adds g3. 3 logical groups, cap 3 -> nothing + // hidden, and the previous-only g3 survives rather than being dropped. + const PREVIOUS_SUFFIX = ' (previous)'; + const meta = [ + { name: 'value', type: 'Float64' }, + { name: 'group', type: 'String' }, + { name: '__hdx_time_bucket', type: 'DateTime' }, + ]; + const bucket = '2025-11-26T11:12:00Z'; + const currentData = [ + { value: 30, group: 'g0', __hdx_time_bucket: bucket }, + { value: 20, group: 'g1', __hdx_time_bucket: bucket }, + ]; + const previousData = [ + { value: 29, group: 'g0', __hdx_time_bucket: bucket }, + { value: 5, group: 'g3', __hdx_time_bucket: bucket }, + ]; + + const actual = formatResponseForTimeChart({ + currentPeriodResponse: { data: currentData, meta }, + previousPeriodResponse: { data: previousData, meta }, + dateRange: [ + new Date('2025-11-26T11:12:00Z'), + new Date('2025-11-26T11:13:00Z'), + ], + granularity: '1 minute', + generateEmptyBuckets: false, + maxSeries: 3, + }); + + const keptKeys = new Set(actual.lineData.map(l => l.dataKey)); + expect(keptKeys.has('g0')).toBe(true); + expect(keptKeys.has('g1')).toBe(true); + expect(keptKeys.has(`g3${PREVIOUS_SUFFIX}`)).toBe(true); + expect(actual.hiddenSeriesCount).toBe(0); + }); + + it('caps by group so all value columns of a kept group survive together', () => { + // Two value columns (avg + max) with a group-by: each group yields one + // series per value column, keyed ` · `. max >> avg in + // magnitude, so ranking each key independently by peak would keep the two + // max series and evict BOTH avg series. Capping by GROUP must instead keep + // both value columns of the surviving groups together. + const meta = [ + { name: 'avg', type: 'Float64' }, + { name: 'max', type: 'Float64' }, + { name: 'group', type: 'String' }, + { name: '__hdx_time_bucket', type: 'DateTime' }, + ]; + const bucket = '2025-11-26T11:12:00Z'; + // 3 groups, cap to 2 logical groups -> g2 (lowest) hidden. + const data = [ + { avg: 30, max: 300, group: 'g0', __hdx_time_bucket: bucket }, + { avg: 20, max: 200, group: 'g1', __hdx_time_bucket: bucket }, + { avg: 10, max: 100, group: 'g2', __hdx_time_bucket: bucket }, + ]; + + const actual = formatResponseForTimeChart({ + currentPeriodResponse: { data, meta }, + dateRange: [ + new Date('2025-11-26T11:12:00Z'), + new Date('2025-11-26T11:13:00Z'), + ], + granularity: '1 minute', + generateEmptyBuckets: false, + maxSeries: 2, + }); + + const keptKeys = new Set(actual.lineData.map(l => l.dataKey)); + // Both value columns of the two kept groups survive together. + expect(keptKeys.has('avg · g0')).toBe(true); + expect(keptKeys.has('max · g0')).toBe(true); + expect(keptKeys.has('avg · g1')).toBe(true); + expect(keptKeys.has('max · g1')).toBe(true); + // The whole g2 group is dropped (both columns), and counted once. + expect(keptKeys.has('avg · g2')).toBe(false); + expect(keptKeys.has('max · g2')).toBe(false); + expect(actual.hiddenSeriesCount).toBe(1); + // renderedSeriesCount is a LOGICAL group count (2 kept groups), not the + // flat lineData length (4 entries: 2 groups × 2 value columns). A + // regression to lineData.length would break this. + expect(actual.renderedSeriesCount).toBe(2); + expect(actual.renderedSeriesCount + actual.hiddenSeriesCount).toBe(3); + }); + + it('does not collapse single-value groups whose name starts with the value-column prefix', () => { + // Single-value charts add NO value-column prefix to the key, so a group + // literally named `value · x` must stay distinct from a group `x`. A + // naive prefix-strip would merge them and miscount series. + const meta = [ + { name: 'value', type: 'Float64' }, + { name: 'group', type: 'String' }, + { name: '__hdx_time_bucket', type: 'DateTime' }, + ]; + const bucket = '2025-11-26T11:12:00Z'; + // Two distinct groups; one collides with the strip pattern `value · `. + const data = [ + { value: 30, group: 'value · x', __hdx_time_bucket: bucket }, + { value: 20, group: 'x', __hdx_time_bucket: bucket }, + ]; + + const actual = formatResponseForTimeChart({ + currentPeriodResponse: { data, meta }, + dateRange: [ + new Date('2025-11-26T11:12:00Z'), + new Date('2025-11-26T11:13:00Z'), + ], + granularity: '1 minute', + generateEmptyBuckets: false, + // Cap high enough that nothing is dropped: this is about the group + // IDENTITY, not eviction. + maxSeries: 100, + }); + + // Both groups are kept as separate series (not collapsed into one). + const keptKeys = new Set(actual.lineData.map(l => l.dataKey)); + expect(keptKeys.has('value · x')).toBe(true); + expect(keptKeys.has('x')).toBe(true); + expect(actual.renderedSeriesCount).toBe(2); + expect(actual.hiddenSeriesCount).toBe(0); + }); + }); + + describe('resolveRenderedSeriesCap', () => { + it('returns the default cap for null/undefined', () => { + expect(resolveRenderedSeriesCap(null)).toBe( + MAX_RENDERED_TIME_CHART_SERIES, + ); + expect(resolveRenderedSeriesCap(undefined)).toBe( + MAX_RENDERED_TIME_CHART_SERIES, + ); + }); + + it('returns Infinity for 0 (unlimited)', () => { + expect(resolveRenderedSeriesCap(0)).toBe(Number.POSITIVE_INFINITY); + }); + + it('returns the value for a positive limit', () => { + expect(resolveRenderedSeriesCap(25)).toBe(25); + }); + + it('falls back to the default cap for malformed values (never disables the guard)', () => { + // Zod blocks these on persist, but Mixed Mongo storage + unvalidated form + // state can still reach here. None must resolve to Infinity (unlimited). + expect(resolveRenderedSeriesCap(NaN)).toBe( + MAX_RENDERED_TIME_CHART_SERIES, + ); + expect(resolveRenderedSeriesCap(-5)).toBe(MAX_RENDERED_TIME_CHART_SERIES); + expect(resolveRenderedSeriesCap(12.5)).toBe( + MAX_RENDERED_TIME_CHART_SERIES, + ); + }); }); describe('convertToTimeChartConfig', () => { diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 300dbe74c0..723207bbf7 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -7,11 +7,15 @@ // `getChartColorInfo()` on HyperDX) would not have a matching gradient // def after the `COLORS` palette was unified to Observable 10. import type { LineData } from '@/ChartUtils'; +import type { ActiveClickSeries } from '@/HDXMultiSeriesTimeChart'; import { buildActiveClickSeries, collectMemoChartGradientHexes, + getSelectedLineData, getVisibleLineData, + getVisibleTooltipRows, HARD_LINES_LIMIT, + sameActiveClickSeries, } from '@/HDXMultiSeriesTimeChart'; import { COLORS } from '@/utils'; @@ -132,6 +136,49 @@ describe('getVisibleLineData', () => { }); }); +describe('getSelectedLineData', () => { + const makeLine = (dataKey: string, displayName?: string): LineData => ({ + dataKey, + currentPeriodKey: dataKey, + previousPeriodKey: `${dataKey}.prev`, + displayName: displayName ?? dataKey, + valueColumnName: dataKey, + color: '#abcdef', + }); + + it('does NOT cap to HARD_LINES_LIMIT (feeds the uncapped pinned tooltip list)', () => { + // The pinned tooltip's drill-down list may show more series than are drawn, + // so selection is applied without the draw cap. This is the behavior that + // lets "load all series" reveal materialized-but-undrawn series. + const lineData = Array.from({ length: HARD_LINES_LIMIT + 50 }, (_, i) => + makeLine(`series-${i}`), + ); + expect(getSelectedLineData(lineData, undefined)).toHaveLength( + HARD_LINES_LIMIT + 50, + ); + }); + + it('applies the same name-based selection as getVisibleLineData', () => { + const lineData = [ + makeLine('a', 'Alpha'), + makeLine('b', 'Beta'), + makeLine('c', 'Gamma'), + ]; + expect( + getSelectedLineData(lineData, new Set(['Alpha', 'Gamma'])).map( + l => l.dataKey, + ), + ).toEqual(['a', 'c']); + }); + + it('returns every series when the selection is empty', () => { + const lineData = [makeLine('a'), makeLine('b')]; + expect( + getSelectedLineData(lineData, new Set()).map(l => l.dataKey), + ).toEqual(['a', 'b']); + }); +}); + // The drill-down popover payload is rebuilt from the clicked bucket row. This // pins that it only includes visible series with a numeric value, and carries // the fields the popover renders (name/color/dataKey/value). @@ -174,6 +221,15 @@ describe('buildActiveClickSeries', () => { expect(result[0].value).toBe(0); }); + it('drops non-finite values (NaN/Infinity) so the resync guard stays stable', () => { + // A ratio chart's zero-denominator bucket yields NaN; admitting it would + // break the NaN-unsafe equality guard and loop the resync effect. + const visible = [makeLine('a'), makeLine('b'), makeLine('c')]; + const row = { a: NaN, b: Infinity, c: 7 }; + const result = buildActiveClickSeries(visible, row); + expect(result.map(r => r.dataKey)).toEqual(['c']); + }); + it('pairs a current-period series with its previous-period value', () => { // makeLine sets previousPeriodKey to `${dataKey}.prev`; when the bucket row // carries a numeric value under that key, it is surfaced as previousValue @@ -218,3 +274,95 @@ describe('buildActiveClickSeries', () => { }); }); }); + +describe('sameActiveClickSeries', () => { + const row = ( + dataKey: string, + value: number, + previousValue?: number, + ): ActiveClickSeries => ({ dataKey, value, previousValue }); + + it('treats an undefined prior snapshot as changed (initial fill)', () => { + expect(sameActiveClickSeries(undefined, [row('a', 1)])).toBe(false); + }); + + it('detects added series (the "load all" case)', () => { + // A capped snapshot gains series once the render cap is lifted; the pinned + // tooltip must rebuild so its "+N more" overflow reflects the drawn set. + const before = [row('a', 5)]; + const after = [row('a', 5), row('b', 3)]; + expect(sameActiveClickSeries(before, after)).toBe(false); + }); + + it('detects a changed value at the pinned bucket', () => { + expect(sameActiveClickSeries([row('a', 5)], [row('a', 6)])).toBe(false); + }); + + it('detects a changed previous-period comparison value', () => { + expect(sameActiveClickSeries([row('a', 5, 4)], [row('a', 5, 9)])).toBe( + false, + ); + }); + + it('is stable for an identical drawn set (no churn on ordinary re-renders)', () => { + const before = [row('a', 5), row('b', 3)]; + const after = [row('a', 5), row('b', 3)]; + expect(sameActiveClickSeries(before, after)).toBe(true); + }); + + it('treats NaN-holding snapshots as equal (no infinite resync loop)', () => { + // Regression: `NaN !== NaN` would report identical NaN-holding snapshots as + // perpetually changed, driving the resync effect into an infinite loop. + // Object.is makes NaN compare equal to itself. (In practice + // buildActiveClickSeries now also drops non-finite values, but the equality + // must be NaN-safe regardless.) + const before = [row('a', NaN)]; + const after = [row('a', NaN)]; + expect(sameActiveClickSeries(before, after)).toBe(true); + + const beforePrev = [row('a', 5, NaN)]; + const afterPrev = [row('a', 5, NaN)]; + expect(sameActiveClickSeries(beforePrev, afterPrev)).toBe(true); + }); +}); + +describe('getVisibleTooltipRows', () => { + const mkRows = (n: number) => + Array.from({ length: n }, (_, i) => ({ dataKey: `s${i}` })); + + it('returns all rows and zero hidden when within the limit', () => { + const rows = mkRows(5); + const result = getVisibleTooltipRows(rows, undefined, 20); + expect(result.rows).toBe(rows); // same reference, no copy + expect(result.hiddenCount).toBe(0); + }); + + it('caps to the limit and reports the remainder as hidden', () => { + const rows = mkRows(100); + const result = getVisibleTooltipRows(rows, undefined, 20); + expect(result.rows).toHaveLength(20); + expect(result.hiddenCount).toBe(80); + // Keeps the highest-ranked (value-desc caller ordering) head. + expect(result.rows[0].dataKey).toBe('s0'); + expect(result.rows[19].dataKey).toBe('s19'); + }); + + it('keeps the cursor-nearest series even when it ranks past the cap', () => { + const rows = mkRows(100); + const result = getVisibleTooltipRows(rows, 's50', 20); + expect(result.rows).toHaveLength(20); + expect(result.hiddenCount).toBe(80); + const keys = result.rows.map(r => r.dataKey); + expect(keys).toContain('s50'); + // The lowest kept row was swapped out to make room; count stays stable. + expect(keys).not.toContain('s19'); + }); + + it('does not duplicate the nearest series when it is already visible', () => { + const rows = mkRows(100); + const result = getVisibleTooltipRows(rows, 's3', 20); + const keys = result.rows.map(r => r.dataKey); + expect(keys.filter(k => k === 's3')).toHaveLength(1); + expect(result.rows).toHaveLength(20); + }); +}); diff --git a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx index ba38dac8d1..ee8e0eca33 100644 --- a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx +++ b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx @@ -19,7 +19,7 @@ import { } from '@mantine/core'; import { shouldFillNullsWithZero } from '@/ChartUtils'; -import { DEFAULT_SERIES_LIMIT } from '@/defaults'; +import { MAX_RENDERED_TIME_CHART_SERIES } from '@/defaults'; import { FormatTime } from '@/useFormatTime'; import { BackgroundChartInput } from './BackgroundChartInput'; @@ -46,12 +46,12 @@ export type ChartConfigDisplaySettings = Pick< > & { groupByColumnsOnLeft?: boolean; alternateRowBackground?: boolean; - // Per-tile cap on the number of series fetched. On group-by time charts it - // drives the __hdx_series_limit CTE; on pie/bar builder charts it becomes a - // plain SQL LIMIT. - // null/undefined = disabled (every series is fetched). The editor clears to - // `null` (not `undefined`) so the cleared state survives JSON - // round-tripping through the URL query state. + // Per-tile series cap. On builder group-by/pie/bar charts it's a fetch cap + // (the __hdx_series_limit CTE / a SQL LIMIT); on raw SQL time charts it's a + // client-side render cap only. Three-state: null/undefined = default cap, + // 0 = unlimited, positive N = top N. See SharedChartSettingsSchema.seriesLimit + // 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; }; @@ -181,10 +181,13 @@ export default function ChartDisplaySettingsDrawer({ const isTimeChart = displayType === DisplayType.Line || displayType === DisplayType.StackedBar; - // The series-limit CTE is only emitted for builder group-by time charts; - // raw SQL configs author their own LIMIT logic directly. - const showSeriesLimit = - isTimeChart && configType !== 'sql' && configType !== 'promql'; + // Series Limit applies to every time chart. On builder group-by charts a + // positive value drives the __hdx_series_limit SQL CTE (trimming what's + // fetched); on raw SQL it drives the client-side render cap in + // `formatResponseForTimeChart` (raw SQL can't inject the CTE). PromQL is + // excluded — its series come from Prometheus, not this pipeline. + const showSeriesLimit = isTimeChart && configType !== 'promql'; + const isRawSqlTimeChart = showSeriesLimit && configType === 'sql'; // On pie/bar builder charts, seriesLimit becomes a plain SQL LIMIT on the // number of slices/bars; raw SQL configs author their own LIMIT directly. @@ -271,9 +274,13 @@ export default function ChartDisplaySettingsDrawer({ diff --git a/packages/app/src/components/ChartEditor/utils.ts b/packages/app/src/components/ChartEditor/utils.ts index 58e4d75f98..e0b7f61c74 100644 --- a/packages/app/src/components/ChartEditor/utils.ts +++ b/packages/app/src/components/ChartEditor/utils.ts @@ -146,6 +146,10 @@ export function convertFormStateToSavedChartConfig( 'fillNulls', 'alignDateRangeToGranularity', 'alternateRowBackground', + // Per-tile render cap for raw SQL time charts (drives the client-side + // series cap in formatResponseForTimeChart). See + // SharedChartSettingsSchema.seriesLimit. + 'seriesLimit', 'alert', 'onClick', ]), @@ -223,6 +227,9 @@ export function convertFormStateToChartConfig( 'fillNulls', 'alignDateRangeToGranularity', 'alternateRowBackground', + // Per-tile render cap for raw SQL time charts (see the save-config path + // above and SharedChartSettingsSchema.seriesLimit). + 'seriesLimit', 'onClick', ]), sqlTemplate: form.sqlTemplate ?? '', diff --git a/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx b/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx index f7cacf916f..98dde8110c 100644 --- a/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx @@ -228,6 +228,10 @@ export function ChartPreviewPanel({ } errorVariant="inline" showMVOptimizationIndicator={false} + // Preview doesn't need the MV indicators; disabling both lets + // DBTimeChart skip the extra MV-optimization EXPLAIN query, which + // otherwise fires on every edit-modal open / submit. + showDateRangeIndicator={false} />
)} diff --git a/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts b/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts index bce2bd0b35..971bdf2d1b 100644 --- a/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts +++ b/packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts @@ -427,7 +427,6 @@ describe('buildChartConfigForExplanations', () => { }); expect(result).toBeDefined(); - // @ts-expect-error union types.. expect(result!.seriesLimit).toBe(3); }); @@ -442,7 +441,6 @@ describe('buildChartConfigForExplanations', () => { }); expect(result).toBeDefined(); - // @ts-expect-error union types.. expect(result!.seriesLimit).toBeUndefined(); }); diff --git a/packages/app/src/components/DBTimeChart.tsx b/packages/app/src/components/DBTimeChart.tsx index 8d1b99ccb4..1538dcf7be 100644 --- a/packages/app/src/components/DBTimeChart.tsx +++ b/packages/app/src/components/DBTimeChart.tsx @@ -39,6 +39,10 @@ import { import { ChartAnnotation } from '@/components/charts/chartAnnotations'; import { ChartSeriesTooltip } from '@/components/charts/ChartSeriesTooltip'; import { useChartTooltipZIndex } from '@/components/charts/ChartTooltip'; +import { + MAX_LOADABLE_TIME_CHART_SERIES, + resolveRenderedSeriesCap, +} from '@/defaults'; import { type ActiveClickPayload, MemoChart } from '@/HDXMultiSeriesTimeChart'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { useMVOptimizationExplanation } from '@/hooks/useMVOptimizationExplanation'; @@ -51,6 +55,7 @@ import ChartErrorState, { } from './charts/ChartErrorState'; import DateRangeIndicator from './charts/DateRangeIndicator'; import DisplaySwitcher from './charts/DisplaySwitcher'; +import HiddenSeriesIndicator from './charts/HiddenSeriesIndicator'; import MVOptimizationIndicator from './MaterializedViews/MVOptimizationIndicator'; /** A single group column / value pair decoded from a chart series key. */ @@ -139,6 +144,9 @@ function ChartTooltipOverlay({ fallbackNumberFormat, numberFormatByKey, previousPeriodOffsetSeconds, + hiddenSeriesCount, + onLoadAllSeries, + expanded, }: { payload: ActiveClickPayload | undefined; buildSearchUrl: (key?: string, value?: number) => string | null; @@ -151,6 +159,12 @@ function ChartTooltipOverlay({ /** Per-value-column formats, keyed by result column name. */ numberFormatByKey: Map; previousPeriodOffsetSeconds?: number; + /** Series dropped by the chart's render cap (see ChartSeriesTooltip). */ + hiddenSeriesCount?: number; + /** Render every series on the chart, bypassing the cap. */ + onLoadAllSeries?: () => void; + /** "Load all" is active: render every row in the scrollable tooltip body. */ + expanded?: boolean; }) { const isOpen = payload != null && @@ -265,6 +279,9 @@ function ChartTooltipOverlay({ onDismiss={onDismiss} onFocusSeries={onFocusSeries} onShowAllSeries={onShowAllSeries} + hiddenSeriesCount={hiddenSeriesCount} + onLoadAllSeries={onLoadAllSeries} + expanded={expanded} /> @@ -336,6 +353,17 @@ function DBTimeChartComponent({ new Set(), ); + // When the render cap hides series, the hidden-series notice lets the user + // opt into rendering every series (accepting the memory/perf cost). Store the + // query shape the opt-in was enabled at (not a bare boolean) so it can be + // gated on the current shape during render — this resets the opt-in the + // instant the query changes, with no setState-in-effect and no one-commit + // window where a stale opt-in pairs with the new shape. `queryShapeIdentity` + // is derived below; `showAllSeries` is defined right after it. + const [showAllSeriesShape, setShowAllSeriesShape] = useState( + null, + ); + const handleClearSelectedSeries = useCallback(() => { setSelectedSeriesSet(new Set()); }, []); @@ -385,12 +413,61 @@ function DBTimeChartComponent({ [config], ); - // Determine whether the config can be optimized with an MV, to determine whether - // to show the MV optimization indicator and date range indicator in the toolbar + // Stable identity for the query's SHAPE, excluding the sliding time window. + // `queriedConfig` (and the `config` it derives from) is a fresh object literal + // on every render — dashboard tiles rebuild the tile config inline each render + // (e.g. on hover) and live ranges tick the dateRange/granularity — so keying + // effects on its object reference, or serializing the whole thing, would fire + // them on unrelated re-renders / every live tick. Stripping the time fields + // yields a value that changes only when the user re-authors the query. + const queryShapeIdentity = useMemo(() => { + // Serialize every top-level field except the sliding time window. + const shape: Record = { ...queriedConfig }; + delete shape.dateRange; + delete shape.granularity; + delete shape.dateRangeEndInclusive; + // Builder configs normalize both `0` and null seriesLimit to `undefined`, + // which JSON.stringify drops — so a null<->0 edit wouldn't change the shape + // and the reset effect below wouldn't fire. Fold in the raw value so the + // two states serialize differently. + return JSON.stringify({ + shape, + rawSeriesLimit: config.seriesLimit ?? null, + }); + }, [queriedConfig, config.seriesLimit]); + + // "Load all" is active only while the shape it was enabled at still matches. + // Deriving it (instead of resetting a boolean in an effect) means a query + // change drops the opt-in in the same render, so a stale opt-in can never + // pair with the new shape — and there's no setState-in-effect. + const showAllSeries = showAllSeriesShape === queryShapeIdentity; + const enableShowAllSeries = useCallback( + () => setShowAllSeriesShape(queryShapeIdentity), + [queryShapeIdentity], + ); + // "Load all" only helps when it actually raises the cap. If the tile's own + // seriesLimit already meets or exceeds the load-all bound (or is unlimited), + // clicking would render the same set — so don't offer the affordance rather + // than flip showAllSeries to a state where onLoadAll goes permanently + // undefined for a no-op. + const loadAllCanRaiseCap = + MAX_LOADABLE_TIME_CHART_SERIES > + resolveRenderedSeriesCap(config.seriesLimit); + const loadAllHandler = + showAllSeries || !loadAllCanRaiseCap ? undefined : enableShowAllSeries; + + // Determine whether the config can be optimized with an MV, to drive the MV + // optimization indicator and the MV-derived date-range indicator in the + // toolbar. Only those two indicators consume `mvOptimizationData`, so skip + // this extra ClickHouse EXPLAIN when both are hidden — which includes the + // edit-modal preview (ChartPreviewPanel passes showMVOptimizationIndicator and + // showDateRangeIndicator both false), so the EXPLAIN is skipped there too. const builderQueriedConfig: BuilderChartConfigWithDateRange | undefined = isBuilderChartConfig(queriedConfig) ? queriedConfig : undefined; - const { data: mvOptimizationData } = - useMVOptimizationExplanation(builderQueriedConfig); + const { data: mvOptimizationData } = useMVOptimizationExplanation( + builderQueriedConfig, + { enabled: showMVOptimizationIndicator || showDateRangeIndicator }, + ); const { data, isLoading, isError, error, isPlaceholderData, isSuccess } = useQueriedChartConfig(queriedConfig, { @@ -469,6 +546,8 @@ function DBTimeChartComponent({ valueColumns, isSingleValueColumn, lineData, + hiddenSeriesCount, + renderedSeriesCount, } = useMemo(() => { const defaultResponse = { error: null, @@ -478,6 +557,8 @@ function DBTimeChartComponent({ groupColumns: [], valueColumns: [], isSingleValueColumn: true, + hiddenSeriesCount: 0, + renderedSeriesCount: 0, }; if (data == null || !isSuccess) { @@ -496,6 +577,23 @@ function DBTimeChartComponent({ source, hiddenSeries, previousPeriodOffsetSeconds, + // "Load all" (from the warning / pinned tooltip) overrides everything; + // otherwise the per-tile Series Limit drives the cap (null = default, + // 0 = unlimited). On builder group-by charts the SQL CTE already trims + // to seriesLimit, so this is a no-op there; on raw SQL it's the only + // cardinality guard. "Load all" is bounded (not truly unlimited) so a + // runaway high-cardinality result can't exhaust browser memory; drawn + // lines stay capped at HARD_LINES_LIMIT either way. Take the max of the + // bound and the tile's own cap so load-all can only ever RAISE the + // rendered count — never reduce it when a tile's seriesLimit already + // exceeds the bound. (A `0`/unlimited tile shows no affordance, so the + // Infinity case is unreachable here.) + maxSeries: showAllSeries + ? Math.max( + MAX_LOADABLE_TIME_CHART_SERIES, + resolveRenderedSeriesCap(config.seriesLimit), + ) + : resolveRenderedSeriesCap(config.seriesLimit), }); return { ...defaultResponse, @@ -516,9 +614,11 @@ function DBTimeChartComponent({ fillNulls, source, config.compareToPreviousPeriod, + config.seriesLimit, previousPeriodData, hiddenSeries, previousPeriodOffsetSeconds, + showAllSeries, ]); // To enable backward compatibility, allow non-controlled usage of displayType @@ -556,6 +656,19 @@ function DBTimeChartComponent({ const dismissPinned = useCallback(() => setActiveClickPayload(undefined), []); const notifyTooltipPinned = useCrossChartPinDismiss(dismissPinned); + // Dismiss any open pin when the query shape changes: its frozen snapshot + // belongs to the previous query, and the resync effect would otherwise + // repaint it with the new query's series at the stale click anchor. (The + // "load all" opt-in resets on its own — it's derived from + // `showAllSeriesShape === queryShapeIdentity` above — so it isn't touched + // here.) Keyed on `queryShapeIdentity` (a stable serialization of the query + // shape) rather than the `queriedConfig` object reference — which is new + // every render — so unrelated re-renders (tile hover) and live-range ticks + // don't dismiss the pin, while re-authoring the query still does. + useEffect(() => { + dismissPinned(); + }, [queryShapeIdentity, dismissPinned]); + // Pin the tooltip on click. Not gated on `source`: source-less charts still // show values/percent-change, and the drill-down actions hide themselves when // there's no source. `disableDrillDown` stays an explicit opt-out. @@ -573,6 +686,20 @@ function DBTimeChartComponent({ [disableDrillDown, notifyTooltipPinned], ); + // In-place refresh of the already-open pin's frozen snapshot (used by the + // chart's resync effect after "load all" / live ticks). Unlike + // setPinnedPayload this does NOT broadcast the cross-chart pin-dismiss — it + // isn't opening a new pin, just repainting the current one's rows. + const refreshPinnedPayload = useCallback( + (payload: ActiveClickPayload | undefined) => { + if (disableDrillDown) { + return; + } + setActiveClickPayload(payload); + }, + [disableDrillDown], + ); + const clickedActiveLabelDate = useMemo(() => { return activeClickPayload?.activeLabel != null ? new Date(Number.parseInt(activeClickPayload.activeLabel) * 1000) @@ -778,6 +905,20 @@ function DBTimeChartComponent({ ); } + if (hiddenSeriesCount > 0) { + allToolbarItems.push( + , + ); + } + if (toolbarSuffix && toolbarSuffix.length > 0) { allToolbarItems.push(...toolbarSuffix); } @@ -796,6 +937,9 @@ function DBTimeChartComponent({ showDateRangeIndicator, mvOptimizationData, queriedConfig, + hiddenSeriesCount, + renderedSeriesCount, + loadAllHandler, ]); return ( @@ -835,6 +979,13 @@ function DBTimeChartComponent({ fallbackNumberFormat={queriedConfig.numberFormat} numberFormatByKey={formatByColumn} previousPeriodOffsetSeconds={previousPeriodOffsetSeconds} + // "+N more" in the pinned tooltip loads every series (same escape + // hatch as the hidden-series warning). Only offered while capped. + hiddenSeriesCount={hiddenSeriesCount} + onLoadAllSeries={loadAllHandler} + // Once loaded, render the full set in the scrollable tooltip body + // (not just the 20-row preview) so "load all" actually shows them. + expanded={showAllSeries} /> { ).toBeInTheDocument(); }); - it('does not show the Series Limit input for raw SQL line charts', () => { + it('shows the Series Limit input for raw SQL line charts (client render cap)', () => { renderWithMantine( { />, ); - expect( - screen.queryByRole('textbox', { name: /series limit/i }), - ).not.toBeInTheDocument(); + const input = screen.getByRole('textbox', { name: /series limit/i }); + expect(input).toBeInTheDocument(); + expect(input).toHaveAttribute( + 'placeholder', + `Default (${MAX_RENDERED_TIME_CHART_SERIES})`, + ); }); it('does not show the Series Limit input for table charts', () => { diff --git a/packages/app/src/components/__tests__/DBTimeChart.test.tsx b/packages/app/src/components/__tests__/DBTimeChart.test.tsx index 1e86e1e60d..380e832757 100644 --- a/packages/app/src/components/__tests__/DBTimeChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTimeChart.test.tsx @@ -1,4 +1,8 @@ import React from 'react'; +import { MantineProvider } from '@mantine/core'; +import { Notifications } from '@mantine/notifications'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import api from '@/api'; import { ChartKeyJoiner } from '@/ChartUtils'; @@ -8,6 +12,7 @@ import { decodeSeriesGroupFilters, } from '@/components/DBTimeChart'; import MVOptimizationIndicator from '@/components/MaterializedViews/MVOptimizationIndicator'; +import { MAX_LOADABLE_TIME_CHART_SERIES } from '@/defaults'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { useMVOptimizationExplanation } from '@/hooks/useMVOptimizationExplanation'; import { useSource } from '@/source'; @@ -177,6 +182,235 @@ describe('DBTimeChart', () => { expect(mvOptExplanationConfig).toBe(indicatorConfig); }); + it('disables the MV-optimization query when both MV and date-range indicators are hidden', () => { + jest.mocked(useSource).mockReturnValue({ + data: { id: 'test-source', name: 'Test Source' }, + } as any); + + renderWithMantine( + , + ); + + expect(jest.mocked(useMVOptimizationExplanation)).toHaveBeenCalled(); + const options = jest.mocked(useMVOptimizationExplanation).mock.calls[0][1]; + expect(options?.enabled).toBe(false); + }); + + it('keeps the MV-optimization query enabled when only the date-range indicator is shown', () => { + jest.mocked(useSource).mockReturnValue({ + data: { id: 'test-source', name: 'Test Source' }, + } as any); + + renderWithMantine( + , + ); + + const options = jest.mocked(useMVOptimizationExplanation).mock.calls[0][1]; + expect(options?.enabled).toBe(true); + }); + + describe('load-all series escape hatch', () => { + // A high-cardinality group-by response: MAX_RENDERED_TIME_CHART_SERIES (250) + // default cap + 50 extra groups, so 50 series are hidden and the + // HiddenSeriesIndicator surfaces the load-all affordance. + const HIDDEN = 50; + const GROUP_COUNT = 250 + HIDDEN; + + const highCardinalityData = Array.from({ length: GROUP_COUNT }, (_, i) => ({ + timestamp: 1704067200, + value: i + 1, + group: `g${i}`, + })); + const highCardinalityMeta = [ + { name: 'timestamp', type: 'DateTime' }, + { name: 'value', type: 'UInt64' }, + { name: 'group', type: 'String' }, + ]; + + const groupByConfig = { + ...baseTestConfig, + groupBy: 'group', + }; + + beforeEach(() => { + mockUseQueriedChartConfig.mockReturnValue({ + data: { + data: highCardinalityData, + meta: highCardinalityMeta, + rows: GROUP_COUNT, + isComplete: true, + }, + isLoading: false, + isError: false, + isSuccess: true, + isPlaceholderData: false, + }); + }); + + it('surfaces the load-all button when the render cap hides series, then hides it after loading all', async () => { + const user = userEvent.setup(); + renderWithMantine(); + + // 50 series over the default 250 cap => the load-all affordance appears. + const loadAllButton = await screen.findByRole('button', { + name: /load all .* series/i, + }); + expect(loadAllButton).toBeInTheDocument(); + + await user.click(loadAllButton); + + // After loading all, every series is materialized (bounded, but far above + // GROUP_COUNT), so nothing is hidden and the affordance disappears. + await waitFor(() => { + expect( + screen.queryByRole('button', { name: /load all .* series/i }), + ).not.toBeInTheDocument(); + }); + }); + + it('keeps the load-all opt-in across an unrelated re-render with a fresh-but-equal config', async () => { + // Regression for the reset effect firing on every render: dashboard tiles + // pass a fresh config object literal each render (e.g. on hover), so a + // reset keyed on config/queriedConfig identity would snap showAllSeries + // back to false and re-cap the chart. The opt-in must survive a re-render + // whose config is a new object with identical query shape. + const user = userEvent.setup(); + const { rerender } = renderWithMantine( + , + ); + + const loadAllButton = await screen.findByRole('button', { + name: /load all .* series/i, + }); + await user.click(loadAllButton); + await waitFor(() => { + expect( + screen.queryByRole('button', { name: /load all .* series/i }), + ).not.toBeInTheDocument(); + }); + + // Re-render with a brand-new object AND a genuinely shifted time window + // (a new dateRange, mimicking a live-range tick / zoom). dateRange is + // deliberately excluded from the shape identity, so the opt-in must + // survive — this also guards against dateRange being reintroduced into + // queryShapeIdentity, which would make live-range ticks re-cap the chart. + rerender( + + + + , + ); + + // The opt-in survives: no series are re-hidden, so the affordance stays + // gone. (Before the fix, the reset effect fired here and it reappeared.) + expect( + screen.queryByRole('button', { name: /load all .* series/i }), + ).not.toBeInTheDocument(); + }); + + it('resets the load-all opt-in when the query shape changes (e.g. seriesLimit re-authored)', async () => { + // The reset must still fire for a genuine query change: after loading all, + // re-authoring the tile (here, tightening seriesLimit) re-applies the cap. + const user = userEvent.setup(); + const { rerender } = renderWithMantine( + , + ); + + const loadAllButton = await screen.findByRole('button', { + name: /load all .* series/i, + }); + await user.click(loadAllButton); + await waitFor(() => { + expect( + screen.queryByRole('button', { name: /load all .* series/i }), + ).not.toBeInTheDocument(); + }); + + // Change the query shape (a positive seriesLimit below GROUP_COUNT keeps + // series hidden), which should reset the opt-in and re-show the affordance. + rerender( + + + + , + ); + + expect( + await screen.findByRole('button', { name: /load all .* series/i }), + ).toBeInTheDocument(); + }); + + it('does not hide series (no load-all affordance) when seriesLimit is 0 (unlimited)', () => { + renderWithMantine( + , + ); + + // seriesLimit=0 resolves to an unlimited render cap, so no series are + // dropped and the load-all affordance never appears. + expect( + screen.queryByRole('button', { name: /load all .* series/i }), + ).not.toBeInTheDocument(); + }); + + it('goes passive (non-clickable) after load-all when the result still exceeds the load-all bound', async () => { + // A result larger than MAX_LOADABLE_TIME_CHART_SERIES (5000): after + // clicking "load all", the cap is lifted to the bound but series remain + // hidden. Clicking again could not reveal more (showAllSeries is already + // true), so the indicator must drop its onLoadAll action rather than + // render a button that no-ops. + const user = userEvent.setup(); + const BIG_GROUP_COUNT = MAX_LOADABLE_TIME_CHART_SERIES + 100; + const bigData = Array.from({ length: BIG_GROUP_COUNT }, (_, i) => ({ + timestamp: 1704067200, + value: i + 1, + group: `g${i}`, + })); + mockUseQueriedChartConfig.mockReturnValue({ + data: { + data: bigData, + meta: highCardinalityMeta, + rows: BIG_GROUP_COUNT, + isComplete: true, + }, + isLoading: false, + isError: false, + isSuccess: true, + isPlaceholderData: false, + }); + + renderWithMantine(); + + const loadAllButton = await screen.findByRole('button', { + name: /load all .* series/i, + }); + await user.click(loadAllButton); + + // Series are still hidden (result > bound), but the affordance is now a + // passive warning icon, not a clickable button. + await waitFor(() => { + expect( + screen.queryByRole('button', { name: /load all .* series/i }), + ).not.toBeInTheDocument(); + }); + }); + }); + it('renders DateRangeIndicator when MV optimization returns a different date range', () => { const originalStartDate = new Date('2024-01-01T00:00:30Z'); const originalEndDate = new Date('2024-01-01T01:30:45Z'); diff --git a/packages/app/src/components/charts/ChartSeriesTooltip.tsx b/packages/app/src/components/charts/ChartSeriesTooltip.tsx index e40dc58d68..eb31ec06c0 100644 --- a/packages/app/src/components/charts/ChartSeriesTooltip.tsx +++ b/packages/app/src/components/charts/ChartSeriesTooltip.tsx @@ -16,7 +16,12 @@ import { IconSearch, } from '@tabler/icons-react'; +import { MAX_EXPANDED_TOOLTIP_ROWS } from '@/defaults'; import type { ActiveClickSeries } from '@/HDXMultiSeriesTimeChart'; +import { + getVisibleTooltipRows, + MAX_TOOLTIP_ROWS, +} from '@/HDXMultiSeriesTimeChart'; import type { NumberFormat } from '@/types'; import { @@ -167,6 +172,36 @@ export type ChartSeriesTooltipProps = { onFocusSeries?: (payload: { dataKey?: string; name: string }) => void; /** Clear an active series focus; renders a "Show All Series" footer action when set. */ onShowAllSeries?: () => void; + /** + * Series dropped by the chart's render cap (i.e. absent from activePayload + * entirely, not just beyond this tooltip's row cap). Added to the tooltip's + * own overflow to size the "+N more" affordance against the true total. + */ + hiddenSeriesCount?: number; + /** + * Render every series on the chart, bypassing the cap. When provided, the + * "+N more" line becomes a button that triggers it (the same escape hatch as + * the chart's hidden-series warning). Omit to keep "+N more" passive. + */ + onLoadAllSeries?: () => void; + /** + * When true, the caller has already loaded all series ("load all" is active), + * so the tooltip renders EVERY row (up to `expandedRowCap`) in its scrollable + * body instead of clamping to MAX_TOOLTIP_ROWS. This is what makes the pinned + * tooltip's "load all" actually reveal the extra rows: the container + * (.chartTooltipContent) already scrolls, so lifting the render cap lets the + * user scroll through the full set. Kept bounded by `expandedRowCap` so a + * runaway high-cardinality bucket can't mount thousands of row Tooltips. + */ + expanded?: boolean; + /** + * Upper bound on rows rendered when `expanded`. Defaults to + * MAX_EXPANDED_TOOLTIP_ROWS. Each row mounts several Mantine Tooltips, so this + * is deliberately well below the chart's materialize ceiling — enough to make + * "load all" meaningfully bigger than the 20-row preview without mounting + * thousands of popovers. Series beyond it stay counted in the "+N more" line. + */ + expandedRowCap?: number; }; /** @@ -185,6 +220,10 @@ export function ChartSeriesTooltip({ onDismiss, onFocusSeries, onShowAllSeries, + hiddenSeriesCount = 0, + onLoadAllSeries, + expanded = false, + expandedRowCap = MAX_EXPANDED_TOOLTIP_ROWS, }: ChartSeriesTooltipProps) { // Called before any early return to keep hook order stable. const actionTooltipZIndex = useChartTooltipActionZIndex(); @@ -201,6 +240,20 @@ export function ChartSeriesTooltip({ return null; } + // Cap rendered rows (each mounts a Mantine Tooltip); rows is value-desc, so + // this keeps the largest. No cursor concept here — the pin is frozen. Once + // "load all" is active (`expanded`), render every row (up to expandedRowCap) + // so the user can scroll the full set instead of being stuck at the 20-row + // preview; the scrollable container bounds the height either way. + const rowCap = expanded ? expandedRowCap : MAX_TOOLTIP_ROWS; + const { rows: visibleRows, hiddenCount: tooltipHiddenCount } = + getVisibleTooltipRows(rows, undefined, rowCap); + + // Rows not shown = those beyond this tooltip's cap PLUS series the chart's + // render cap dropped entirely (absent from activePayload). Clicking loads all + // series onto the chart — the same escape hatch as the hidden-series warning. + const totalHidden = tooltipHiddenCount + hiddenSeriesCount; + // Per-series actions only make sense with more than one group (a single series // is covered by the header/footer). const showPerSeriesActions = rows.length > 1; @@ -263,8 +316,9 @@ export function ChartSeriesTooltip({ return ( - - {rows.map((payload, idx) => { + {/* Height bounded by the shared .chartTooltipContent container. */} + + {visibleRows.map((payload, idx) => { const name = payload.name ?? payload.dataKey ?? ''; const rowNumberFormat = (payload.valueColumnName != null @@ -305,6 +359,21 @@ export function ChartSeriesTooltip({ /> ); })} + {totalHidden > 0 && + (onLoadAllSeries ? ( + + + +{totalHidden.toLocaleString()} more (click to load all) + + + ) : ( + + +{totalHidden.toLocaleString()} more + + ))} ); diff --git a/packages/app/src/components/charts/HiddenSeriesIndicator.tsx b/packages/app/src/components/charts/HiddenSeriesIndicator.tsx new file mode 100644 index 0000000000..56b77a849b --- /dev/null +++ b/packages/app/src/components/charts/HiddenSeriesIndicator.tsx @@ -0,0 +1,57 @@ +import { ActionIcon, Tooltip } from '@mantine/core'; +import { IconAlertTriangle } from '@tabler/icons-react'; + +interface HiddenSeriesIndicatorProps { + hiddenSeriesCount: number; + renderedSeriesCount: number; + /** Render every series, bypassing the cap. Omit to keep the notice passive. */ + onLoadAll?: () => void; +} + +/** + * Warns that the chart returned more series than the client renders. The + * transform caps series to protect memory; this surfaces the dropped ones and, + * when `onLoadAll` is provided, lets the user render all of them anyway. + */ +export default function HiddenSeriesIndicator({ + hiddenSeriesCount, + renderedSeriesCount, + onLoadAll, +}: HiddenSeriesIndicatorProps) { + if (hiddenSeriesCount <= 0) { + return null; + } + + const total = renderedSeriesCount + hiddenSeriesCount; + const label = + `This query returned ${total.toLocaleString()} series. ` + + `${hiddenSeriesCount.toLocaleString()} low-value series were hidden to keep the page responsive; ` + + `showing the top ${renderedSeriesCount.toLocaleString()} by peak value. ` + + (onLoadAll + ? `Click to load all ${total.toLocaleString()} (may be slow).` + : 'Add a stricter GROUP BY, a WHERE filter, or a series limit to reduce cardinality.'); + + const icon = ( + + ); + + return ( + + {onLoadAll ? ( + e.stopPropagation()} + aria-label={`Load all ${total.toLocaleString()} series`} + > + {icon} + + ) : ( + icon + )} + + ); +} diff --git a/packages/app/src/components/charts/__tests__/ChartSeriesTooltip.test.tsx b/packages/app/src/components/charts/__tests__/ChartSeriesTooltip.test.tsx index 549528c2a0..b3de47662e 100644 --- a/packages/app/src/components/charts/__tests__/ChartSeriesTooltip.test.tsx +++ b/packages/app/src/components/charts/__tests__/ChartSeriesTooltip.test.tsx @@ -1,8 +1,21 @@ import React from 'react'; import { fireEvent, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import { ChartSeriesTooltip } from '@/components/charts/ChartSeriesTooltip'; import type { ActiveClickSeries } from '@/HDXMultiSeriesTimeChart'; +import { MAX_TOOLTIP_ROWS } from '@/HDXMultiSeriesTimeChart'; + +// Build `count` current-period series with descending values so the tooltip's +// value-desc sort/cap is deterministic. +function makeRows(count: number): ActiveClickSeries[] { + return Array.from({ length: count }, (_, i) => ({ + value: count - i, + dataKey: `g${i}`, + name: `g${i}`, + color: '#437eef', + })); +} const multiSeriesPayload: ActiveClickSeries[] = [ { dataKey: 'error', name: 'error', value: 90, color: '#f00' }, @@ -14,12 +27,120 @@ const singleSeriesPayload: ActiveClickSeries[] = [ ]; const baseProps = { - activeLabel: '1704067200', + activeLabel: '1700000000', numberFormatByKey: new Map(), buildSearchUrl: () => null, }; describe('ChartSeriesTooltip', () => { + it('shows passive "+N more" text (not a button) without onLoadAllSeries', () => { + // 25 rows over the 20-row cap => 5 hidden by the tooltip; plus 100 dropped + // by the chart render cap => totalHidden should read 105. + renderWithMantine( + , + ); + + expect(screen.queryByRole('button', { name: /load all/i })).toBeNull(); + // tooltipHiddenCount (5) + hiddenSeriesCount (100) = 105. + expect(screen.getByText(/\+105 more/)).toBeInTheDocument(); + }); + + it('renders a clickable load-all button and fires onLoadAllSeries', async () => { + const onLoadAllSeries = jest.fn(); + renderWithMantine( + , + ); + + const button = screen.getByRole('button', { + name: /load all 105 more series/i, + }); + expect(button).toHaveTextContent(/\+105 more \(click to load all\)/); + await userEvent.click(button); + expect(onLoadAllSeries).toHaveBeenCalledTimes(1); + }); + + it('folds hiddenSeriesCount into the total even when nothing overflows the tooltip cap', () => { + // Under the tooltip cap (no tooltipHiddenCount), so the "+N more" total is + // driven entirely by the chart-level render cap. + renderWithMantine( + , + ); + + expect(screen.getByText(/\+42 more/)).toBeInTheDocument(); + }); + + it('shows no "+N more" line when nothing is hidden', () => { + renderWithMantine( + , + ); + + expect(screen.queryByText(/more/)).toBeNull(); + }); + + it('caps rendered rows at MAX_TOOLTIP_ROWS when not expanded', () => { + renderWithMantine( + , + ); + + // Only the top 20 series render; the 21st (g20) is beyond the preview cap. + expect(screen.getByText('g0')).toBeInTheDocument(); + expect(screen.getByText(`g${MAX_TOOLTIP_ROWS - 1}`)).toBeInTheDocument(); + expect(screen.queryByText(`g${MAX_TOOLTIP_ROWS}`)).toBeNull(); + // The overflow is summarized. + expect(screen.getByText(/\+30 more/)).toBeInTheDocument(); + }); + + it('renders every row (past the 20-row preview) when expanded, so "load all" reveals them', () => { + // This is the core fix: once "load all" is active the pinned tooltip shows + // the full set in its scrollable body instead of the 20-row preview. + const count = MAX_TOOLTIP_ROWS + 30; + renderWithMantine( + , + ); + + // Rows beyond the 20-preview are now present. + expect(screen.getByText(`g${MAX_TOOLTIP_ROWS}`)).toBeInTheDocument(); + expect(screen.getByText(`g${count - 1}`)).toBeInTheDocument(); + // Nothing beyond the expanded cap here, so no "+N more". + expect(screen.queryByText(/more/)).toBeNull(); + }); + + it('still summarizes rows beyond expandedRowCap when expanded', () => { + renderWithMantine( + , + ); + + // 30 rows, cap 25 => 5 summarized. + expect(screen.getByText('g24')).toBeInTheDocument(); + expect(screen.queryByText('g25')).toBeNull(); + expect(screen.getByText(/\+5 more/)).toBeInTheDocument(); + }); + it('renders one Focus button per series when there is more than one series', () => { renderWithMantine( , diff --git a/packages/app/src/components/charts/__tests__/HiddenSeriesIndicator.test.tsx b/packages/app/src/components/charts/__tests__/HiddenSeriesIndicator.test.tsx new file mode 100644 index 0000000000..672fe13828 --- /dev/null +++ b/packages/app/src/components/charts/__tests__/HiddenSeriesIndicator.test.tsx @@ -0,0 +1,45 @@ +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import HiddenSeriesIndicator from '@/components/charts/HiddenSeriesIndicator'; + +describe('HiddenSeriesIndicator', () => { + it('renders nothing when no series are hidden', () => { + renderWithMantine( + , + ); + // No warning icon/button when nothing is hidden. + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + }); + + it('is a passive icon (not a button) without onLoadAll', () => { + renderWithMantine( + , + ); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + }); + + it('renders a clickable load-all button and fires onLoadAll', async () => { + const onLoadAll = jest.fn(); + renderWithMantine( + , + ); + // aria-label reflects the total (rendered + hidden). + const button = screen.getByRole('button', { + name: /load all 1,000 series/i, + }); + await userEvent.click(button); + expect(onLoadAll).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/app/src/defaults.ts b/packages/app/src/defaults.ts index 95d151307f..5894d8b78d 100644 --- a/packages/app/src/defaults.ts +++ b/packages/app/src/defaults.ts @@ -6,6 +6,57 @@ export const DEFAULT_QUERY_TIMEOUT = 60; // max_execution_time, seconds export const DEFAULT_FILTER_KEYS_FETCH_LIMIT = 100; export const DEFAULT_SERIES_LIMIT = 100; +// Default ceiling on distinct series the time-chart transform materializes, +// across all config types, when a tile has no explicit `seriesLimit`. +// High-cardinality group-bys (esp. raw SQL) can return tens of thousands of +// series; without this cap the client holds them all in memory while only +// DEFAULT_SERIES_LIMIT are drawn. Series beyond the effective cap are dropped +// (lowest peak first) and surfaced via a hidden-series notice. Override +// per-tile via the Display Settings "Series Limit" control. +export const MAX_RENDERED_TIME_CHART_SERIES = 250; + +// Upper bound on rows the pinned tooltip renders once "load all series" is +// active. The tooltip body scrolls, but each row mounts several Mantine +// Tooltips (Search/Copy/Focus), so we don't render the full materialized set +// (up to MAX_LOADABLE_TIME_CHART_SERIES) — that could mount thousands of +// popovers and hang the tab. This ceiling is far above the 20-row preview +// (MAX_TOOLTIP_ROWS) and the drawn-line cap (HARD_LINES_LIMIT = 100), so "load +// all" reveals a meaningfully larger, scrollable list; series beyond it remain +// summarized in the tooltip's "+N more" line. +export const MAX_EXPANDED_TOOLTIP_ROWS = 500; + +// Hard ceiling for the "load all series" escape hatch. Clicking the +// hidden-series notice (or the pinned tooltip's "+N more") opts into rendering +// beyond the default cap, but we still bound materialization so a runaway +// high-cardinality raw-SQL result (tens of thousands of series) can't exhaust +// browser memory / hang the tab. 5000 is a generous ceiling far above both the +// default materialize cap (MAX_RENDERED_TIME_CHART_SERIES) and the draw cap +// (HARD_LINES_LIMIT); drawn lines remain bounded by HARD_LINES_LIMIT regardless. +export const MAX_LOADABLE_TIME_CHART_SERIES = 5000; + +/** + * Resolve the effective client-side render cap from a tile's `seriesLimit` + * (see SharedChartSettingsSchema): null/undefined → the default cap, exactly 0 → + * unlimited (Infinity), a positive integer N → N. Anything else (NaN, negative, + * non-integer — possible via the Mixed Mongo field or unvalidated form state) + * falls back to the default cap so a bad value can't silently disable the guard. + */ +export function resolveRenderedSeriesCap( + seriesLimit: number | null | undefined, +): number { + if (seriesLimit === 0) { + return Number.POSITIVE_INFINITY; + } + if ( + seriesLimit == null || + !Number.isInteger(seriesLimit) || + seriesLimit < 0 + ) { + return MAX_RENDERED_TIME_CHART_SERIES; + } + return seriesLimit; +} + export function searchChartConfigDefaults( team: any | undefined | null, ): Partial { diff --git a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx index 84651d07d4..7b9eab0b7c 100644 --- a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx +++ b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx @@ -12,6 +12,7 @@ import { renderHook, waitFor } from '@testing-library/react'; import { useClickhouseClient } from '@/clickhouse'; import { + appendChunk, getGranularityAlignedTimeWindows, useQueriedChartConfig, } from '@/hooks/useChartConfig'; @@ -1524,4 +1525,42 @@ describe('useChartConfig', () => { expect(result2.current.data?.data).toBeDefined(); }); }); + + describe('appendChunk', () => { + const empty = { data: [], meta: [], rows: 0, isComplete: false }; + + it('reuses the chunk array on the first/only chunk (no copy)', () => { + const chunkData = [{ a: 1 }, { a: 2 }]; + const chunk = { + data: chunkData, + meta: [{ name: 'a', type: 'UInt64' }], + rows: 2, + }; + const result = appendChunk(empty, { chunk, isComplete: true }); + // Same array reference — the large-array spread copy is skipped. + expect(result.data).toBe(chunkData); + expect(result.rows).toBe(2); + expect(result.isComplete).toBe(true); + expect(result.meta).toBe(chunk.meta); + }); + + it('prepends the newer chunk ahead of accumulated rows on later chunks', () => { + const older = { + data: [{ a: 3 }], + meta: [{ name: 'a', type: 'UInt64' }], + rows: 1, + isComplete: false, + }; + const chunk = { + data: [{ a: 1 }, { a: 2 }], + meta: [{ name: 'a', type: 'UInt64' }], + rows: 2, + }; + const result = appendChunk(older, { chunk, isComplete: true }); + // Newer chunk first, then accumulated (oldest-first ordering preserved). + expect(result.data).toEqual([{ a: 1 }, { a: 2 }, { a: 3 }]); + expect(result.data).not.toBe(chunk.data); // fresh array when merging + expect(result.rows).toBe(3); + }); + }); }); diff --git a/packages/app/src/hooks/useChartConfig.tsx b/packages/app/src/hooks/useChartConfig.tsx index e4393dc780..22fbd0b556 100644 --- a/packages/app/src/hooks/useChartConfig.tsx +++ b/packages/app/src/hooks/useChartConfig.tsx @@ -11,7 +11,10 @@ import { isUsingGranularity, renderChartConfig, } from '@hyperdx/common-utils/dist/core/renderChartConfig'; -import { convertDateRangeToGranularityString } from '@hyperdx/common-utils/dist/core/utils'; +import { + convertDateRangeToGranularityString, + hasPositiveSeriesLimit, +} from '@hyperdx/common-utils/dist/core/utils'; import { isBuilderChartConfig, isPromqlChartConfig, @@ -160,9 +163,12 @@ async function* fetchDataInChunks({ // 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; + // 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 = + isBuilderChartConfig(config) && hasPositiveSeriesLimit(config.seriesLimit) + ? config.seriesLimit + : undefined; const windowedConfigFor = (w: (typeof windows)[number]) => ({ ...config, ...(w ?? {}), @@ -244,13 +250,22 @@ async function* fetchDataInChunks({ } } -/** Append the given chunk to the given accumulated result */ -function appendChunk( +/** Append the given chunk to the given accumulated result. Exported for tests. */ +export function appendChunk( accumulated: TQueryFnData, { chunk, isComplete }: TChunk, ): TQueryFnData { + const chunkData = chunk.data || []; + const accumulatedData = accumulated?.data || []; + // Fast path for the first/only chunk (always the case for raw SQL, which is + // never chunked): reuse the chunk's array instead of spreading it into a new + // one. Avoids an O(rows) copy of a potentially very large (100k+) row array. + const data = + accumulatedData.length === 0 + ? chunkData + : [...chunkData, ...accumulatedData]; return { - data: [...(chunk.data || []), ...(accumulated?.data || [])], + data, meta: chunk.meta, rows: (accumulated?.rows || 0) + (chunk.rows || 0), isComplete, diff --git a/packages/app/styles/HDXLineChart.module.scss b/packages/app/styles/HDXLineChart.module.scss index 30f7df2ec7..2acc37a48c 100644 --- a/packages/app/styles/HDXLineChart.module.scss +++ b/packages/app/styles/HDXLineChart.module.scss @@ -1,3 +1,18 @@ +/* stylelint-disable selector-pseudo-class-no-unknown */ + +// Chart root wrapper. Also the scope for the nearest-series emphasis