fix(dashboards): keep auto-detected number format on display settings apply

.changeset/number-tile-format-survives-apply.md

@@ -0,0 +1,12 @@
+---
+"@hyperdx/app": patch
+---
+
+fix(dashboards): keep the auto-detected number format when applying display settings
+
+Opening Display Settings on a number tile that auto-detects its format from the
+datasource (for example p95 of a trace Duration column) and clicking Apply no
+longer rewrites the format to Number. The drawer now reflects the
+datasource-derived format, and Apply persists `numberFormat` only when the user
+explicitly changes it; otherwise it stays unset so render-time auto-detection
+keeps driving the format.

packages/app/src/components/ChartDisplaySettingsDrawer.tsx

@@ -117,7 +117,13 @@ export default function ChartDisplaySettingsDrawer({
     [settings, defaultNumberFormat],
   );
 
-  const { control, handleSubmit, reset, setValue } = useForm<DrawerFormValues>({
+  const {
+    control,
+    handleSubmit,
+    reset,
+    setValue,
+    formState: { dirtyFields },
+  } = useForm<DrawerFormValues>({
     defaultValues: appliedDefaults,
   });
 
@@ -137,13 +143,23 @@ export default function ChartDisplaySettingsDrawer({
     handleSubmit(formValues => {
       // Strip client-side localIds before passing rules to the config.
       const { colorRules, ...rest } = formValues;
+      // Persist numberFormat only when the user actually chose one: either the
+      // tile already had an explicit override (settings.numberFormat) or the
+      // user changed the format control in this session (dirtyFields). Otherwise
+      // emit undefined so the datasource-derived format keeps driving render
+      // instead of freezing the drawer's inferred fallback into the config.
+      const numberFormatExplicit =
+        settings.numberFormat != null || dirtyFields.numberFormat != null;
       onChange({
         ...rest,
+        numberFormat: numberFormatExplicit
+          ? formValues.numberFormat
+          : undefined,
         colorRules: colorRules ? stripLocalIds(colorRules) : undefined,
       });
     })();
     onClose();
-  }, [onChange, handleSubmit, onClose]);
+  }, [onChange, handleSubmit, onClose, settings.numberFormat, dirtyFields]);
 
   const resetToDefaults = useCallback(() => {
     reset(

packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx

@@ -196,6 +196,7 @@ export default function EditTimeChartForm({
   }, [isDirty, onDirtyChange]);
 
   const select = useWatch({ control, name: 'select' });
+  const series = useWatch({ control, name: 'series' });
   const sourceId = useWatch({ control, name: 'source' });
   const alert = useWatch({ control, name: 'alert' });
   const seriesReturnType = useWatch({ control, name: 'seriesReturnType' });
@@ -263,13 +264,18 @@ export default function EditTimeChartForm({
     ],
   });
 
+  // Format auto-detected purely from the datasource (e.g. duration for a trace
+  // Duration column), used as the drawer's fallback when no explicit
+  // numberFormat is set. Reads the live `series`, the field the builder edits;
+  // `select` is only synced from `series` on submit and on display-type / source
+  // resets, so it goes stale after an aggregation edit and resolves undefined.
+  // The drawer prioritizes an explicit `numberFormat` over this fallback.
   const autoDetectedNumberFormat = useMemo(
     () =>
-      numberFormat ??
-      (Array.isArray(select)
-        ? getFirstSeriesNumberFormat(select, tableSource)
-        : undefined),
-    [numberFormat, select, tableSource],
+      Array.isArray(series)
+        ? getFirstSeriesNumberFormat(series, tableSource)
+        : undefined,
+    [series, tableSource],
   );
 
   const displaySettings: ChartConfigDisplaySettings = useMemo(
@@ -592,7 +598,12 @@ export default function EditTimeChartForm({
       colorRules,
       backgroundChart,
     }: ChartConfigDisplaySettings) => {
-      setValue('numberFormat', numberFormat);
+      // Only persist an explicit numberFormat. When the drawer emits undefined
+      // (the user never chose a format), leave it unset so render-time
+      // auto-detection keeps driving the format from the datasource.
+      if (numberFormat !== undefined) {
+        setValue('numberFormat', numberFormat);
+      }
       setValue('alignDateRangeToGranularity', alignDateRangeToGranularity);
       setValue('fillNulls', fillNulls);
       setValue('compareToPreviousPeriod', compareToPreviousPeriod);

packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx

@@ -263,4 +263,89 @@ describe('ChartDisplaySettingsDrawer', () => {
       expect(onChange.mock.calls[0][0].seriesLimit).toBeNull();
     });
   });
+
+  describe('number format persistence', () => {
+    // A duration number tile (e.g. p95 Duration from a trace source) auto-detects
+    // a duration format from the datasource; the drawer receives it as
+    // `defaultNumberFormat` and shows it as the fallback when no explicit
+    // numberFormat is set.
+    const durationFormat = { output: 'duration' as const, factor: 1e-9 };
+    const numberBuilderProps = {
+      ...baseProps,
+      configType: 'builder' as const,
+      displayType: DisplayType.Number,
+    };
+
+    it('does not persist the auto-detected format when Apply is clicked without changing it', async () => {
+      const onChange = jest.fn();
+      const user = userEvent.setup();
+
+      renderWithMantine(
+        <ChartDisplaySettingsDrawer
+          {...numberBuilderProps}
+          defaultNumberFormat={durationFormat}
+          onChange={onChange}
+        />,
+      );
+
+      await user.click(screen.getByRole('button', { name: /apply/i }));
+
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange.mock.calls[0][0].numberFormat).toBeUndefined();
+    });
+
+    it('persists the format when the user changes the output format', async () => {
+      const onChange = jest.fn();
+      const user = userEvent.setup();
+
+      renderWithMantine(
+        <ChartDisplaySettingsDrawer
+          {...numberBuilderProps}
+          defaultNumberFormat={durationFormat}
+          onChange={onChange}
+        />,
+      );
+
+      await user.selectOptions(
+        screen.getByRole('combobox', { name: /output format/i }),
+        'number',
+      );
+      await user.click(screen.getByRole('button', { name: /apply/i }));
+
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange.mock.calls[0][0].numberFormat).toMatchObject({
+        output: 'number',
+      });
+    });
+
+    it('preserves an existing explicit format when only another setting changes', async () => {
+      const onChange = jest.fn();
+      const user = userEvent.setup();
+
+      renderWithMantine(
+        <ChartDisplaySettingsDrawer
+          {...numberBuilderProps}
+          settings={
+            {
+              numberFormat: { output: 'currency', currencySymbol: '$' },
+            } as ChartConfigDisplaySettings
+          }
+          onChange={onChange}
+        />,
+      );
+
+      // Change the tile color, not the format.
+      await user.click(screen.getByTestId('color-swatch-input-trigger'));
+      await user.click(
+        await screen.findByTestId('color-swatch-option-chart-blue'),
+      );
+      await user.click(screen.getByRole('button', { name: /apply/i }));
+
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange.mock.calls[0][0]).toMatchObject({
+        color: 'chart-blue',
+        numberFormat: { output: 'currency' },
+      });
+    });
+  });
 });