Skip to content

fix(aci): Translate percent threshold value correctly for metric monitors#111259

Merged
malwilley merged 5 commits intomasterfrom
malwilley/percent-threshold-fix
Mar 23, 2026
Merged

fix(aci): Translate percent threshold value correctly for metric monitors#111259
malwilley merged 5 commits intomasterfrom
malwilley/percent-threshold-fix

Conversation

@malwilley
Copy link
Member

Fixes ISWF-2278

Metric monitors with percent change thresholds were translating the value incorrectly. 110 should display as 10% higher, not 110% higher.

See this for an example: https://sentry.sentry.io/monitors/2809101/

Before:
CleanShot 2026-03-20 at 15 10 34@2x
CleanShot 2026-03-20 at 15 10 44@2x

After:
CleanShot 2026-03-20 at 15 10 52@2x
CleanShot 2026-03-20 at 15 11 01@2x

@malwilley malwilley requested a review from a team as a code owner March 20, 2026 22:11
@linear-code
Copy link

linear-code bot commented Mar 20, 2026

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 20, 2026
Copy link
Contributor

@saponifi3d saponifi3d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - there might be a few nits / refactor things we might be able to decompose, but none of it really feels related to this work. :shipit:

Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Preview chart shows wrong threshold for percent detectors
    • Added isConditionsInDeltaFormat parameter to skip the absolute-to-delta conversion when conditions are already in delta format from the form.

Create PR

Or push these changes by commenting:

@cursor push 50dfe91189
Preview (50dfe91189)
diff --git a/static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx b/static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx
--- a/static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx
+++ b/static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx
@@ -209,6 +209,7 @@
       conditions,
       detectionType,
       comparisonSeries,
+      isConditionsInDeltaFormat: true,
     });
 
   const isAnomalyDetection = detectionType === 'dynamic';

diff --git a/static/app/views/detectors/hooks/useMetricDetectorThresholdSeries.tsx b/static/app/views/detectors/hooks/useMetricDetectorThresholdSeries.tsx
--- a/static/app/views/detectors/hooks/useMetricDetectorThresholdSeries.tsx
+++ b/static/app/views/detectors/hooks/useMetricDetectorThresholdSeries.tsx
@@ -106,7 +106,8 @@
 function extractThresholdsFromConditions(
   conditions: Array<Omit<MetricCondition, 'id'>>,
   aggregate: string,
-  detectionType: MetricDetectorConfig['detectionType']
+  detectionType: MetricDetectorConfig['detectionType'],
+  isConditionsInDeltaFormat: boolean
 ): {
   thresholds: Array<{
     priority: DetectorPriorityLevel;
@@ -123,7 +124,7 @@
     )
     .map(condition => {
       let value = normalizePercentageThreshold(aggregate, Number(condition.comparison));
-      if (detectionType === 'percent') {
+      if (detectionType === 'percent' && !isConditionsInDeltaFormat) {
         value = percentThresholdAbsoluteToDelta(value);
       }
       return {
@@ -143,7 +144,7 @@
       ? {
           type: resolutionCondition.type,
           value:
-            detectionType === 'percent'
+            detectionType === 'percent' && !isConditionsInDeltaFormat
               ? percentThresholdAbsoluteToDelta(
                   normalizePercentageThreshold(
                     aggregate,
@@ -168,6 +169,11 @@
   conditions: Array<Omit<MetricCondition, 'id'>> | undefined;
   detectionType: MetricDetectorConfig['detectionType'];
   comparisonSeries?: Series[];
+  /**
+   * When true, conditions are already in user-facing delta format (e.g., 10 for "10% higher").
+   * When false (default), conditions are in backend absolute format (e.g., 110) and need conversion.
+   */
+  isConditionsInDeltaFormat?: boolean;
 }
 
 interface UseMetricDetectorThresholdSeriesResult {
@@ -186,6 +192,7 @@
   detectionType,
   aggregate,
   comparisonSeries = [],
+  isConditionsInDeltaFormat = false,
 }: UseMetricDetectorThresholdSeriesProps): UseMetricDetectorThresholdSeriesResult {
   const theme = useTheme();
 
@@ -197,7 +204,8 @@
     const {thresholds, resolution} = extractThresholdsFromConditions(
       conditions,
       aggregate,
-      detectionType
+      detectionType,
+      isConditionsInDeltaFormat
     );
     const additional: LineSeriesOption[] = [];
 
@@ -315,5 +323,5 @@
 
     // Other detection types not supported yet
     return {maxValue: undefined, additionalSeries: additional};
-  }, [aggregate, conditions, detectionType, comparisonSeries, theme]);
+  }, [aggregate, conditions, detectionType, comparisonSeries, isConditionsInDeltaFormat, theme]);
 }

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

@malwilley malwilley merged commit 3bcf506 into master Mar 23, 2026
69 of 70 checks passed
@malwilley malwilley deleted the malwilley/percent-threshold-fix branch March 23, 2026 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants