Fix: Group By on ratio charts

.changeset/ratio-group-by.md

@@ -0,0 +1,33 @@
+---
+"@hyperdx/common-utils": patch
+"@hyperdx/app": patch
+---
+
+fix: support Group By on ratio charts
+
+A ratio chart (`seriesReturnType: 'ratio'`) with a Group By previously collapsed
+to a single line. Two issues in the multi-series merge: (1) rows were keyed by
+time bucket only, so groups at the same bucket overwrote each other, and (2) the
+ratio computation dropped every non-value column, discarding the group
+dimension. The merge now keys by (time bucket + group dimensions) and the ratio
+result carries the group columns through, so a grouped ratio renders one series
+per group.
+
+Grouped ratios use share-of-total semantics: each group's denominator is the
+total of the denominator column across all groups in the same time bucket, so
+the grouped lines are each group's contribution to the overall ratio and sum to
+the ungrouped value (e.g. each tenant's share of the overall error rate), rather
+than each group's in-group rate. Ungrouped ratios are unchanged (one row per
+bucket → the bucket total is that row's denominator). A group absent from the
+filtered numerator (e.g. a tenant with zero errors) contributes 0%, not N/A.
+
+Also fixed alongside grouped ratios:
+
+- A ratio whose two series resolve to the same value-column alias (e.g.
+  `count(request)` filtered / unfiltered for an error rate) previously collapsed
+  to one column and threw "Unable to compute ratio". The two operands are now
+  kept distinct through the merge.
+- The chart-level Group By for metric sources offered the union of every
+  series' fields, which could suggest a native column that exists in one metric
+  table (e.g. gauge) but not another (e.g. sum), making that series' query fail.
+  It now offers only fields valid for every series (the intersection).

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

@@ -1,10 +1,11 @@
-import { useState } from 'react';
+import { useMemo, useState } from 'react';
 import {
   Control,
   FieldArrayWithId,
   FieldErrors,
   UseFormClearErrors,
   UseFormSetValue,
+  useWatch,
 } from 'react-hook-form';
 import { TableConnection } from '@hyperdx/common-utils/dist/core/metadata';
 import {
@@ -38,6 +39,7 @@ import { OnClickFormButton } from './OnClickForm/OnClickFormButton';
 import { ChartSeriesEditor } from './ChartSeriesEditor';
 import { HeatmapSeriesEditor } from './HeatmapSeriesEditor';
 import { TileAlertEditor } from './TileAlertEditor';
+import { buildGroupByConnectionProps } from './utils';
 
 type ChartEditorControlsProps = {
   control: Control<ChartEditorFormState>;
@@ -105,6 +107,15 @@ export function ChartEditorControls({
   const [isSourceSchemaPreviewOpen, setIsSourceSchemaPreviewOpen] =
     useState(false);
 
+  // The chart-level Group By must be valid against every series query. For
+  // metric sources (which fan out to per-type tables) this means offering the
+  // intersection of each series' fields; see buildGroupByConnectionProps.
+  const series = useWatch({ control, name: 'series' });
+  const groupByConnectionProps = useMemo(
+    () => buildGroupByConnectionProps({ tableSource, series, tableConnection }),
+    [tableSource, series, tableConnection],
+  );
+
   return (
     <>
       <Flex mb="md" align="center" justify="space-between">
@@ -211,7 +222,7 @@ export function ChartEditorControls({
                 </div>
                 <div>
                   <SQLInlineEditorControlled
-                    tableConnection={tableConnection}
+                    {...groupByConnectionProps}
                     control={control}
                     name={`groupBy`}
                     placeholder="SQL Columns"

packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts

@@ -9,6 +9,7 @@ import {
 import { ChartEditorFormState } from '@/components/ChartEditor/types';
 import {
   buildChartConfigForExplanations,
+  buildGroupByConnectionProps,
   buildSampleEventsConfig,
   computeDbTimeChartConfig,
   displayTypeToActiveTab,
@@ -501,3 +502,67 @@ describe('buildChartConfigForExplanations', () => {
     expect(result!.dateRange).toBe(dateRange);
   });
 });
+
+// ---------------------------------------------------------------------------
+// buildGroupByConnectionProps
+// ---------------------------------------------------------------------------
+
+describe('buildGroupByConnectionProps', () => {
+  const tableConnection = {
+    databaseName: 'default',
+    tableName: 'logs',
+    connectionId: 'clickhouse',
+  };
+
+  it('falls back to the source tableConnection for non-metric sources', () => {
+    const result = buildGroupByConnectionProps({
+      tableSource: logSource,
+      series: [{ metricType: 'gauge', metricName: 'cpu' }],
+      tableConnection,
+    });
+
+    expect(result).toEqual({ tableConnection });
+  });
+
+  it('builds one intersected connection per distinct metric table + name for mixed-type series', () => {
+    const result = buildGroupByConnectionProps({
+      tableSource: metricSource,
+      series: [
+        { metricType: 'gauge', metricName: 'cpu' },
+        { metricType: 'sum', metricName: 'requests' },
+        // duplicate of the first series — must be deduped
+        { metricType: 'gauge', metricName: 'cpu' },
+        // incomplete series — skipped
+        { metricType: 'gauge' },
+      ],
+      tableConnection,
+    });
+
+    expect(result.tableConnection).toBeUndefined();
+    expect(result.intersectFields).toBe(true);
+    expect(result.tableConnections).toEqual([
+      {
+        databaseName: 'default',
+        tableName: 'metrics.gauge',
+        connectionId: 'clickhouse',
+        metricName: 'cpu',
+      },
+      {
+        databaseName: 'default',
+        tableName: 'metrics.sum',
+        connectionId: 'clickhouse',
+        metricName: 'requests',
+      },
+    ]);
+  });
+
+  it('falls back to the tableConnection when a metric source has no resolvable series', () => {
+    const result = buildGroupByConnectionProps({
+      tableSource: metricSource,
+      series: [{ metricName: 'cpu' }], // no metricType
+      tableConnection,
+    });
+
+    expect(result).toEqual({ tableConnection });
+  });
+});

packages/app/src/components/DBEditTimeChartForm/utils.ts

@@ -1,4 +1,8 @@
 import z from 'zod';
+import {
+  TableConnection,
+  TableConnectionChoice,
+} from '@hyperdx/common-utils/dist/core/metadata';
 import {
   isBuilderChartConfig,
   isPromqlChartConfig,
@@ -27,6 +31,7 @@ import {
 } from '@/ChartUtils';
 import { ChartEditorFormState } from '@/components/ChartEditor/types';
 import { getFirstTimestampValueExpression } from '@/source';
+import { getMetricTableName } from '@/utils';
 import {
   extendDateRangeToInterval,
   intervalToGranularity,
@@ -252,3 +257,53 @@ export function buildChartConfigForExplanations({
 
   return config;
 }
+
+/**
+ * Picks the table connection(s) that drive attribute autocomplete for the
+ * chart-level Group By.
+ *
+ * Metric sources have no single `from.tableName` (they fan out to per-type
+ * metric tables), so we build one connection per series' metric table + name
+ * (deduped) and ask the editor to offer only fields present in ALL of them
+ * (`intersectFields`). A ratio can mix metric types (e.g. gauge / sum) whose
+ * tables have different native columns — a union would suggest a column that
+ * only exists in one series and make the other series' query fail, so the Group
+ * By must be restricted to fields valid for every series.
+ *
+ * Non-metric sources (and metric sources with no resolvable series) fall back
+ * to the source's single `tableConnection`.
+ */
+export function buildGroupByConnectionProps({
+  tableSource,
+  series,
+  tableConnection,
+}: {
+  tableSource: TSource | undefined;
+  series: { metricType?: string; metricName?: string }[] | undefined;
+  tableConnection: TableConnection;
+}): TableConnectionChoice & { intersectFields?: boolean } {
+  if (tableSource?.kind !== SourceKind.Metric || !Array.isArray(series)) {
+    return { tableConnection };
+  }
+
+  const seen = new Set<string>();
+  const connections: TableConnection[] = [];
+  for (const s of series) {
+    if (!s?.metricType || !s?.metricName) continue;
+    const metricTable = getMetricTableName(tableSource, s.metricType);
+    if (!metricTable) continue;
+    const key = `${metricTable}::${s.metricName}`;
+    if (seen.has(key)) continue;
+    seen.add(key);
+    connections.push({
+      databaseName: tableSource.from.databaseName,
+      tableName: metricTable,
+      connectionId: tableSource.connection,
+      metricName: s.metricName,
+    });
+  }
+
+  return connections.length > 0
+    ? { tableConnections: connections, intersectFields: true }
+    : { tableConnection };
+}

packages/app/src/components/SQLEditor/SQLInlineEditor.tsx

@@ -65,6 +65,10 @@ type SQLInlineEditorProps = {
   allowMultiline?: boolean;
   dateRange?: [Date, Date];
   sourceId?: string;
+  // With multiple tableConnections, offer only fields present in ALL of them
+  // (intersection) rather than the union — for an expression that must be valid
+  // against every connection, e.g. a chart-level Group By over multiple series.
+  intersectFields?: boolean;
 };
 
 const MAX_EDITOR_HEIGHT = '150px';
@@ -91,6 +95,7 @@ export default function SQLInlineEditor({
   allowMultiline = true,
   dateRange,
   sourceId,
+  intersectFields,
 }: SQLInlineEditorProps & TableConnectionChoice) {
   const { colorScheme } = useMantineColorScheme();
   const _tableConnections = tableConnection
@@ -100,6 +105,7 @@ export default function SQLInlineEditor({
   const { data: fields } = useMultipleAllFields(_tableConnections ?? [], {
     dateRange,
     timestampValueExpression: source?.timestampValueExpression,
+    intersect: intersectFields,
   });
   const filteredFields = useMemo(() => {
     return filterField ? fields?.filter(filterField) : fields;

packages/app/src/hooks/__tests__/useMetadata.test.tsx

@@ -14,6 +14,7 @@ import { renderHook, waitFor } from '@testing-library/react';
 import api from '@/api';
 import {
   deduplicate2dArray,
+  intersect2dArray,
   useGetKeyValues,
   useMultipleAllFields,
   useMultipleGetKeyValues,
@@ -497,3 +498,45 @@ describe('deduplicate2dArray', () => {
     expect(result).toEqual([{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }]);
   });
 });
+
+describe('intersect2dArray', () => {
+  it('keeps only elements present in every sub-array', () => {
+    const input = [
+      [
+        { id: 1, name: 'Alice' },
+        { id: 2, name: 'Bob' },
+      ],
+      [
+        { id: 1, name: 'Alice' },
+        { id: 3, name: 'Charlie' },
+      ],
+    ];
+
+    // Only Alice appears in both — the fields valid across all connections.
+    expect(intersect2dArray(input)).toEqual([{ id: 1, name: 'Alice' }]);
+  });
+
+  it('collapses to empty when any sub-array is empty (a failed fetch fails closed)', () => {
+    const input = [[{ id: 1, name: 'Alice' }], []];
+
+    expect(intersect2dArray(input)).toEqual([]);
+  });
+
+  it('dedupes repeats within a single sub-array before intersecting', () => {
+    const input = [
+      [
+        { id: 1, name: 'Alice' },
+        { id: 1, name: 'Alice' },
+      ],
+      [{ id: 1, name: 'Alice' }],
+    ];
+
+    // A repeat within one table must not be counted twice toward the "present
+    // in every sub-array" tally.
+    expect(intersect2dArray(input)).toEqual([{ id: 1, name: 'Alice' }]);
+  });
+
+  it('returns empty for an empty 2D array', () => {
+    expect(intersect2dArray([])).toEqual([]);
+  });
+});

packages/app/src/hooks/useMetadata.tsx

@@ -224,18 +224,24 @@ export function useMultipleAllFields(
   options?: Partial<UseQueryOptions<Field[]>> & {
     dateRange?: [Date, Date];
     timestampValueExpression?: string;
+    // Return only fields present in EVERY table connection instead of the
+    // union. Use for a shared expression (e.g. a chart-level Group By over
+    // multiple series) that must be valid against all of them — the union
+    // would offer fields that exist in one table but not another.
+    intersect?: boolean;
   },
 ) {
   const metadata = useMetadataWithSettings();
   const { data: me, isFetched } = api.useMe();
-  const { dateRange, timestampValueExpression, ...queryOptions } =
+  const { dateRange, timestampValueExpression, intersect, ...queryOptions } =
     options ?? {};
   return useQuery<Field[]>({
     queryKey: [
       'useMetadata.useMultipleAllFields',
       ...tableConnections.map(tc => ({ ...tc })),
       dateRange ? [dateRange[0].getTime(), dateRange[1].getTime()] : undefined,
       timestampValueExpression,
+      intersect ?? false,
     ],
     queryFn: async () => {
       const team = me?.team;
@@ -260,10 +266,12 @@ export function useMultipleAllFields(
         return result.value;
       });
 
-      // skip deduplication if not needed
+      // skip set logic if not needed
       if (fields2d.length === 1) return fields2d[0];
 
-      return deduplicate2dArray<Field>(fields2d);
+      return intersect
+        ? intersect2dArray<Field>(fields2d)
+        : deduplicate2dArray<Field>(fields2d);
     },
     enabled:
       tableConnections.length > 0 &&
@@ -572,3 +580,25 @@ export function deduplicate2dArray<T extends object>(array2d: T[][]): T[] {
   }
   return array;
 }
+
+// Keep only elements present in every sub-array (set intersection). A rejected
+// fetch yields an empty sub-array, which correctly collapses the result to
+// empty — fail closed rather than offer a field that isn't in every table.
+export function intersect2dArray<T extends object>(array2d: T[][]): T[] {
+  if (array2d.length === 0) return [];
+  const counts = new Map<string, { elem: T; n: number }>();
+  for (const _array of array2d) {
+    const seenInArray = new Set<string>();
+    for (const elem of _array) {
+      const key = objectHash.sha1(elem);
+      if (seenInArray.has(key)) continue; // dedupe within a table first
+      seenInArray.add(key);
+      const entry = counts.get(key);
+      if (entry) entry.n += 1;
+      else counts.set(key, { elem, n: 1 });
+    }
+  }
+  return Array.from(counts.values())
+    .filter(({ n }) => n === array2d.length)
+    .map(({ elem }) => elem);
+}