diff --git a/.changeset/number-tile-sparkline-groupby.md b/.changeset/number-tile-sparkline-groupby.md new file mode 100644 index 0000000000..375d52826c --- /dev/null +++ b/.changeset/number-tile-sparkline-groupby.md @@ -0,0 +1,7 @@ +--- +"@hyperdx/app": patch +--- + +fix(dashboards): match the number-tile background sparkline to the displayed value + +The big number on a number tile is a single aggregate (its query drops `groupBy`), but the background sparkline kept any `groupBy` the tile carried over from a prior Line display type. It then plotted only the first group's trend behind a value that aggregates every group. The sparkline now drops `groupBy` as well, so its trend reflects the same single series as the value it sits behind. diff --git a/packages/app/src/components/NumberTileBackgroundChart.tsx b/packages/app/src/components/NumberTileBackgroundChart.tsx index 22d57fdc46..0a7ad0b98b 100644 --- a/packages/app/src/components/NumberTileBackgroundChart.tsx +++ b/packages/app/src/components/NumberTileBackgroundChart.tsx @@ -70,6 +70,44 @@ export function sparklinePointsFromGraphResults( return points; } +/** + * Derive the sparkline's time-series query config from a number tile's config. + * + * The big number strips both `granularity` and `groupBy` + * (`convertToNumberChartConfig`), collapsing the query to a single aggregate. + * The sparkline must plot that same single series, so it strips `groupBy` too; + * otherwise a tile carrying a residual `groupBy` (left over from a prior Line + * display type) would query multiple series, and the renderer plots only the + * first, which would not match the value. `granularity` is kept (auto when + * unset) to recover the temporal trend behind the value. + * + * Number-tile display-only fields are dropped as well: they flow into the + * query key (via `convertToTimeChartConfig`), so leaving them in would refetch + * identical time-series data on every purely visual edit (sparkline type, tile + * color, color rules, number format). Exported for unit testing. + */ +export function buildSparklineTimeConfig( + config: ChartConfigWithDateRange, +): ChartConfigWithDateRange { + const { + backgroundChart: _backgroundChart, + color: _color, + colorRules: _colorRules, + numberFormat: _numberFormat, + ...rest + } = config; + const timeConfig: ChartConfigWithDateRange = { + ...rest, + displayType: DisplayType.Line, + granularity: config.granularity ?? 'auto', + }; + // `groupBy` exists only on builder configs, so drop it under the guard. + if (isBuilderChartConfig(timeConfig)) { + delete timeConfig.groupBy; + } + return timeConfig; +} + function NumberTileBackgroundChartInner({ config, backgroundChart, @@ -77,28 +115,7 @@ function NumberTileBackgroundChartInner({ config: ChartConfigWithDateRange; backgroundChart: BackgroundChart; }) { - // Number tiles strip granularity / group-by (`convertToNumberChartConfig`), - // collapsing the query to a single value. Request a line series at the - // tile's granularity (auto when unset) to recover the temporal trend. - // - // Drop number-tile display-only fields first: they flow into the query key - // (via `convertToTimeChartConfig`), so leaving them in would refetch - // identical time-series data on every purely visual edit (sparkline type, - // tile color, number format). - const timeConfig = useMemo(() => { - const { - backgroundChart: _backgroundChart, - color: _color, - colorRules: _colorRules, - numberFormat: _numberFormat, - ...rest - } = config; - return { - ...rest, - displayType: DisplayType.Line, - granularity: config.granularity ?? 'auto', - }; - }, [config]); + const timeConfig = useMemo(() => buildSparklineTimeConfig(config), [config]); const { dateRange, granularity, fillNulls } = useTimeChartSettings(timeConfig); diff --git a/packages/app/src/components/__tests__/NumberTileBackgroundChart.test.tsx b/packages/app/src/components/__tests__/NumberTileBackgroundChart.test.tsx index 6f603867a2..38b95c7010 100644 --- a/packages/app/src/components/__tests__/NumberTileBackgroundChart.test.tsx +++ b/packages/app/src/components/__tests__/NumberTileBackgroundChart.test.tsx @@ -1,4 +1,20 @@ -import { sparklinePointsFromGraphResults } from '@/components/NumberTileBackgroundChart'; +import React from 'react'; +import { DisplayType } from '@hyperdx/common-utils/dist/types'; + +import NumberTileBackgroundChart, { + buildSparklineTimeConfig, + sparklinePointsFromGraphResults, +} from '@/components/NumberTileBackgroundChart'; +import { useQueriedChartConfig } from '@/hooks/useChartConfig'; +import { useSource } from '@/source'; + +jest.mock('@/hooks/useChartConfig', () => ({ + useQueriedChartConfig: jest.fn(), +})); + +jest.mock('@/source', () => ({ + useSource: jest.fn(), +})); describe('sparklinePointsFromGraphResults', () => { const ts = '__hdx_time_bucket'; @@ -55,3 +71,82 @@ describe('sparklinePointsFromGraphResults', () => { ]); }); }); + +describe('buildSparklineTimeConfig', () => { + const baseConfig = { + dateRange: [ + new Date('2024-01-01T00:00:00Z'), + new Date('2024-01-01T01:00:00Z'), + ] as [Date, Date], + from: { databaseName: 'test', tableName: 'test' }, + timestampValueExpression: 'timestamp', + connection: 'test-connection', + select: '', + where: '', + }; + + it('drops groupBy and the display-only fields, keeps granularity, and forces a Line display type', () => { + const result = buildSparklineTimeConfig({ + ...baseConfig, + granularity: '5 minute', + groupBy: 'ServiceName', + backgroundChart: { type: 'area' as const }, + color: 'chart-success' as const, + colorRules: [ + { operator: 'gte' as const, value: 1, color: 'chart-error' as const }, + ], + numberFormat: { output: 'percent' as const, mantissa: 2 }, + }); + + expect(result).not.toHaveProperty('groupBy'); + expect(result).not.toHaveProperty('backgroundChart'); + expect(result).not.toHaveProperty('color'); + expect(result).not.toHaveProperty('colorRules'); + expect(result).not.toHaveProperty('numberFormat'); + expect(result.granularity).toBe('5 minute'); + expect(result.displayType).toBe(DisplayType.Line); + }); + + it("defaults granularity to 'auto' when the tile has none", () => { + const result = buildSparklineTimeConfig(baseConfig); + expect(result.granularity).toBe('auto'); + expect(result.displayType).toBe(DisplayType.Line); + }); +}); + +describe('NumberTileBackgroundChart', () => { + const mockUseQueriedChartConfig = useQueriedChartConfig as jest.Mock; + + beforeEach(() => { + jest.clearAllMocks(); + mockUseQueriedChartConfig.mockReturnValue({ data: undefined }); + (useSource as jest.Mock).mockReturnValue({ data: null }); + }); + + it('strips groupBy from the issued query so the sparkline matches the single displayed aggregate', () => { + const config = { + dateRange: [ + new Date('2024-01-01T00:00:00Z'), + new Date('2024-01-01T01:00:00Z'), + ] as [Date, Date], + from: { databaseName: 'test', tableName: 'test' }, + timestampValueExpression: 'timestamp', + connection: 'test-connection', + select: '', + where: '', + groupBy: 'ServiceName', + backgroundChart: { type: 'area' as const }, + }; + + renderWithMantine( + , + ); + + expect(mockUseQueriedChartConfig).toHaveBeenCalled(); + const queriedConfig = mockUseQueriedChartConfig.mock.calls[0][0]; + expect(queriedConfig).not.toHaveProperty('groupBy'); + }); +});