From 9cf7b1a43d8579778e1095d550ed63a95be78f04 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Wed, 26 Nov 2025 11:02:23 -0800 Subject: [PATCH] fix(aci): use aggregate output type for % change detection charts --- .../utils/detectorChartFormatting.spec.tsx | 19 ++++++++++++++++--- .../utils/detectorChartFormatting.tsx | 16 ++++++++-------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/static/app/views/detectors/utils/detectorChartFormatting.spec.tsx b/static/app/views/detectors/utils/detectorChartFormatting.spec.tsx index 97b456d3a1950c..3f8f05b4759e7a 100644 --- a/static/app/views/detectors/utils/detectorChartFormatting.spec.tsx +++ b/static/app/views/detectors/utils/detectorChartFormatting.spec.tsx @@ -23,14 +23,27 @@ describe('getDetectorChartFormatters', () => { expect(result.formatTooltipValue(2000)).toBe('2.00s'); }); - it('percent detection: returns percentage type and % suffix', () => { + it('percent detection: uses aggregate output type and does not append % to formatters', () => { const result = getDetectorChartFormatters({ detectionType: 'percent', aggregate: 'count()', }); - expect(result.outputType).toBe('percentage'); + // % change detection should use the aggregate's actual type (number), not percentage + expect(result.outputType).toBe('number'); + // unitSuffix is still % for threshold display purposes expect(result.unitSuffix).toBe('%'); - expect(result.formatTooltipValue(0.25)).toBe('25%'); + // But formatters should NOT append % since primary series shows actual metric values + expect(result.formatTooltipValue(1000)).toBe('1,000'); + expect(result.formatYAxisLabel(1000)).toBe('1k'); + }); + + it('percentage aggregate (crash_free_rate): returns percentage type', () => { + const result = getDetectorChartFormatters({ + detectionType: 'static', + aggregate: 'crash_free_rate(session)', + }); + + expect(result.outputType).toBe('percentage'); }); }); diff --git a/static/app/views/detectors/utils/detectorChartFormatting.tsx b/static/app/views/detectors/utils/detectorChartFormatting.tsx index 969ca8a085220b..78d73fb55ad528 100644 --- a/static/app/views/detectors/utils/detectorChartFormatting.tsx +++ b/static/app/views/detectors/utils/detectorChartFormatting.tsx @@ -17,22 +17,22 @@ export function getDetectorChartFormatters({ detectionType, aggregate, }: DetectorChartFormatterOptions) { - const outputType = - detectionType === 'percent' ? 'percentage' : aggregateOutputType(aggregate); + const outputType = aggregateOutputType(aggregate); const unitSuffix = getMetricDetectorSuffix(detectionType, aggregate); + // For % change detection, the primary series shows actual metric values (counts, durations, etc.) + // The % suffix is only for threshold display, not for y-axis/tooltip formatting + const shouldAppendSuffix = + detectionType !== 'percent' && (outputType === 'number' || outputType === 'integer'); + const formatYAxisLabel = (value: number): string => { const base = axisLabelFormatterUsingAggregateOutputType(value, outputType, true); - return outputType === 'number' || outputType === 'integer' - ? `${base}${unitSuffix}` - : base; + return shouldAppendSuffix ? `${base}${unitSuffix}` : base; }; const formatTooltipValue = (value: number): string => { const base = tooltipFormatterUsingAggregateOutputType(value, outputType); - return outputType === 'number' || outputType === 'integer' - ? `${base}${unitSuffix}` - : base; + return shouldAppendSuffix ? `${base}${unitSuffix}` : base; }; return {