[HDX-4111] refactor(alerts/search): consolidate saved-search → chart-config builder (#2166)

## Summary

Refs [HDX-4111](https://linear.app/clickhouse/issue/HDX-4111/private-instance-alert-system-triggering-false-positive-alerts).

Consolidate the saved-search → chart-config builder into a single shared helper. The alert task, the alert preview chart, and the app search page each had their own inline builder, and only the search page applied `source.tableFilterExpression`. On Log sources where that field is set, the alert path could legitimately count rows the search page hides — same false-positive class as HDX-4111, different cause from the underlying ClickHouse bug. Routing all three paths through the shared builder eliminates this drift class.

## Changes

### Shared chart-config builder

`packages/common-utils/src/core/searchChartConfig.ts` (new) exports `buildSearchChartConfig(source, input)` — the single source of truth for assembling a `BuilderChartConfig` from a `TSource` plus saved-search-style fields. Three call sites now route through it:

| Call site | Before | After |
| --- | --- | --- |
| `packages/app/src/DBSearchPage.tsx` (`useSearchedConfigToChartConfig`) | inline; applied `tableFilterExpression`; latently dropped it when user filters were non-null | shared builder |
| `packages/app/src/components/AlertPreviewChart.tsx` | inline; did NOT apply `tableFilterExpression` | shared builder |
| `packages/api/src/tasks/checkAlerts/index.ts` (`SAVED_SEARCH` branch) | inline; did NOT apply `tableFilterExpression` | shared builder |

Behavior fixes that fall out of consolidation:
- `tableFilterExpression` is now applied uniformly on Log sources by the alert task and the alert preview.
- A latent bug in the search-page builder is fixed: a non-null `filters` array no longer silently drops the table filter via spread-overwrite.
- The alert preview chart now matches what the alert task will actually count.

### Shared default SELECT for count alerts

`ALERT_COUNT_DEFAULT_SELECT` (in the same module) — both the alert task and the alert preview chart use this constant for the `count()` aggregate, so the two paths produce byte-identical SELECT shapes.

### `SearchChartConfig` return type

Narrower than `BuilderChartConfigWithOptDateRange` (keeps `timestampValueExpression` required, since the builder always sets it from the source). Lets downstream callers spread-and-add `dateRange` into `BuilderChartConfigWithDateRange` without TS errors.

## Risk / behavior change

Alerts on Log sources with `tableFilterExpression` set will now evaluate against a strictly smaller row set — the same rows the search page was already showing. Any alert that was quietly firing only because of rows hidden by `tableFilterExpression` will stop firing. This is a correctness fix.

Follow-up structural work (TILE alert chart-config consolidation, `computeAliasWithClauses` extraction) is tracked in [HDX-4113](https://linear.app/clickhouse/issue/HDX-4113/consolidate-alert-chart-config-assembly-into-hyperdxcommon-utils).

Author: wrn14897

.changeset/hdx-4111-alert-table-filter-expression.md

@@ -0,0 +1,20 @@
+---
+'@hyperdx/common-utils': minor
+'@hyperdx/api': minor
+'@hyperdx/app': minor
+---
+
+refactor(alerts/search): consolidate the saved-search → chart-config builder
+into a single shared helper, `buildSearchChartConfig`, in
+`@hyperdx/common-utils/core/searchChartConfig.ts`. The app search page, the
+alert preview chart, and the scheduled alert task's `SAVED_SEARCH` branch now
+all route through it, so `tableFilterExpression`, `implicitColumnExpression`,
+sample-weight expressions, SELECT precedence, and the `count()` default
+SELECT shape are applied identically by construction.
+
+Behavior fixes that fall out of consolidation:
+- The alert task and the alert preview now apply `source.tableFilterExpression`
+  on Log sources, matching what the search page already did.
+- A latent bug in the search-page builder is fixed: a non-null `filters`
+  array no longer silently drops the `tableFilterExpression` SQL filter via
+  spread-overwrite.

packages/api/src/models/source.ts

@@ -134,6 +134,7 @@ export const LogSource = Source.discriminator<ILogSource>(
     spanIdExpression: String,
     implicitColumnExpression: String,
     uniqueRowIdExpression: String,
+    /** @deprecated See LogSourceSchema in @hyperdx/common-utils/types.ts. */
     tableFilterExpression: String,
     highlightedTraceAttributeExpressions: {
       type: mongoose.Schema.Types.Array,

packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.test.ts

@@ -30,7 +30,7 @@ import AlertHistory from '@/models/alertHistory';
 import Connection, { IConnection } from '@/models/connection';
 import Dashboard, { IDashboard } from '@/models/dashboard';
 import { ISavedSearch, SavedSearch } from '@/models/savedSearch';
-import { ISource, Source } from '@/models/source';
+import { ISource, LogSource, Source } from '@/models/source';
 import { ITeam } from '@/models/team';
 import Webhook, { IWebhook } from '@/models/webhook';
 import * as checkAlert from '@/tasks/checkAlerts';
@@ -2314,6 +2314,110 @@ describe('checkAlerts', () => {
       );
     });
 
+    // The scheduled alert task and the app search page both go through
+    // `buildSearchChartConfig`, which prepends the Log source's
+    // `tableFilterExpression` (when set) as a SQL filter. This regression
+    // test pins that contract for the alert task: rows excluded by
+    // `tableFilterExpression` must not be counted toward the alert
+    // threshold.
+    it('SAVED_SEARCH alert honors source.tableFilterExpression', async () => {
+      const {
+        team,
+        webhook,
+        connection,
+        source,
+        savedSearch,
+        teamWebhooksById,
+        clickhouseClient,
+      } = await setupSavedSearchAlertTest();
+
+      // Configure the source to hide the 'excluded' service, matching the
+      // semantics of a Log source whose UI view filters it out.
+      // `tableFilterExpression` is declared on the Log discriminator schema
+      // (not the base Source schema), so we must update through `LogSource`
+      // — `Source.updateOne` would silently drop the field under Mongoose's
+      // default strict mode.
+      await LogSource.updateOne(
+        { _id: source._id },
+        { $set: { tableFilterExpression: "ServiceName != 'excluded'" } },
+      );
+      const filteredSource = await Source.findById(source._id);
+
+      const details = await createAlertDetails(
+        team,
+        filteredSource ?? undefined,
+        {
+          source: AlertSource.SAVED_SEARCH,
+          channel: {
+            type: 'webhook',
+            webhookId: webhook._id.toString(),
+          },
+          interval: '5m',
+          thresholdType: AlertThresholdType.ABOVE_EXCLUSIVE,
+          threshold: 1,
+          savedSearchId: savedSearch.id,
+        },
+        {
+          taskType: AlertTaskType.SAVED_SEARCH,
+          savedSearch,
+        },
+      );
+
+      const now = new Date('2023-11-16T22:12:00.000Z');
+      const eventMs = new Date('2023-11-16T22:05:00.000Z');
+
+      // Insert two log rows in the alert window:
+      //   1. 'excluded' service — matches the saved search `where` but is
+      //      hidden by the source's `tableFilterExpression`.
+      //   2. 'api' service — matches the saved search `where` and passes
+      //      the table filter. Threshold uses ABOVE_EXCLUSIVE (strict `>`),
+      //      so with the filter applied (count = 1) `1 > 1` is false and
+      //      the alert stays OK. If the filter were dropped (count = 2),
+      //      `2 > 1` would fire — the failure mode this test guards.
+      await bulkInsertLogs([
+        {
+          ServiceName: 'excluded',
+          Timestamp: eventMs,
+          SeverityText: 'error',
+          Body: 'This row should be hidden by tableFilterExpression',
+        },
+        {
+          ServiceName: 'api',
+          Timestamp: eventMs,
+          SeverityText: 'error',
+          Body: 'This row should pass the filter',
+        },
+      ]);
+
+      await processAlertAtTime(
+        now,
+        details,
+        clickhouseClient,
+        connection.id,
+        alertProvider,
+        teamWebhooksById,
+      );
+
+      // Threshold is `> 1`, so with the fix (1 row counted) the alert stays
+      // OK. Without the fix (2 rows counted — including the excluded one),
+      // the alert would transition to ALERT and fire the webhook.
+      const alert = await Alert.findById(details.alert.id);
+      expect(alert!.state).toBe('OK');
+
+      const alertHistories = await AlertHistory.find({
+        alert: details.alert.id,
+      }).sort({ createdAt: 1 });
+      expect(alertHistories.length).toBe(1);
+      expect(alertHistories[0].state).toBe('OK');
+      // Exactly one row counted — the 'excluded' row is filtered out.
+      expect(alertHistories[0].lastValues).toEqual([
+        expect.objectContaining({ count: 1 }),
+      ]);
+
+      // No webhook should have fired.
+      expect(slack.postMessageToWebhook).not.toHaveBeenCalled();
+    });
+
     it('TILE alert (events) - slack webhook', async () => {
       const {
         team,

packages/api/src/tasks/checkAlerts/index.ts

@@ -14,6 +14,10 @@ import {
   Metadata,
 } from '@hyperdx/common-utils/dist/core/metadata';
 import { renderChartConfig } from '@hyperdx/common-utils/dist/core/renderChartConfig';
+import {
+  ALERT_COUNT_DEFAULT_SELECT,
+  buildSearchChartConfig,
+} from '@hyperdx/common-utils/dist/core/searchChartConfig';
 import {
   aliasMapToWithClauses,
   displayTypeSupportsRawSqlAlerts,
@@ -512,32 +516,23 @@ const getChartConfigFromAlert = (
   if (details.taskType === AlertTaskType.SAVED_SEARCH) {
     const { source } = details;
     const savedSearch = details.savedSearch;
-    return {
-      connection,
+    // Delegate to the shared builder (in @hyperdx/common-utils) so the alert
+    // task, the alert preview chart, and the main app search page all
+    // assemble saved-search chart configs identically — keeping source-level
+    // fields like `tableFilterExpression` applied uniformly across paths.
+    return buildSearchChartConfig(source, {
+      where: savedSearch.where,
+      whereLanguage: savedSearch.whereLanguage,
+      filters: savedSearch.filters?.map(f => ({ ...f })),
+      groupBy: alert.groupBy,
+      select: ALERT_COUNT_DEFAULT_SELECT,
       displayType: DisplayType.Line,
+      connection,
       dateRange,
       dateRangeStartInclusive: true,
       dateRangeEndInclusive: false,
-      from: source.from,
       granularity: `${windowSizeInMins} minute`,
-      select: [
-        {
-          aggFn: 'count',
-          aggCondition: '',
-          valueExpression: '',
-        },
-      ],
-      where: savedSearch.where,
-      whereLanguage: savedSearch.whereLanguage,
-      filters: savedSearch.filters?.map(f => ({ ...f })),
-      groupBy: alert.groupBy,
-      implicitColumnExpression:
-        source.kind === SourceKind.Log || source.kind === SourceKind.Trace
-          ? source.implicitColumnExpression
-          : undefined,
-      ...pickSampleWeightExpressionProps(source),
-      timestampValueExpression: source.timestampValueExpression,
-    };
+    });
   } else if (details.taskType === AlertTaskType.TILE) {
     const tile = details.tile;
 

packages/app/src/DBSearchPage.tsx

@@ -27,6 +27,7 @@ import { z } from 'zod';
 import { zodResolver } from '@hookform/resolvers/zod';
 import { ClickHouseQueryError } from '@hyperdx/common-utils/dist/clickhouse';
 import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata';
+import { buildSearchChartConfig } from '@hyperdx/common-utils/dist/core/searchChartConfig';
 import {
   aliasMapToWithClauses,
   isBrowser,
@@ -37,9 +38,7 @@ import {
   ChartConfigWithDateRange,
   DisplayType,
   Filter,
-  isLogSource,
   isTraceSource,
-  pickSampleWeightExpressionProps,
   SourceKind,
   TSource,
 } from '@hyperdx/common-utils/dist/types';
@@ -511,7 +510,7 @@ function SaveSearchModalComponent({
               <Text size="xs" mb="xs" mt="sm">
                 ORDER BY
               </Text>
-              <Text size="xs">{chartConfig.orderBy}</Text>
+              <Text size="xs">{`${chartConfig.orderBy ?? ''}`}</Text>
               {searchedConfig.filters && searchedConfig.filters.length > 0 && (
                 <>
                   <Text size="xs" mb="xs" mt="sm">
@@ -690,35 +689,20 @@ function useSearchedConfigToChartConfig(
 
   return useMemo(() => {
     if (sourceObj != null) {
+      const resolvedOrderBy =
+        orderBy || defaultSearchConfig?.orderBy || defaultOrderBy;
+
+      const chartConfig = buildSearchChartConfig(sourceObj, {
+        where,
+        whereLanguage,
+        filters,
+        select: select || defaultSearchConfig?.select || null,
+        displayType: DisplayType.Search,
+        ...(resolvedOrderBy != null ? { orderBy: resolvedOrderBy } : {}),
+      });
+
       return {
-        data: {
-          select:
-            select ||
-            defaultSearchConfig?.select ||
-            sourceObj.defaultTableSelectExpression,
-          from: sourceObj.from,
-          source: sourceObj.id,
-          ...(isLogSource(sourceObj) && sourceObj.tableFilterExpression != null
-            ? {
-                filters: [
-                  {
-                    type: 'sql' as const,
-                    condition: sourceObj.tableFilterExpression,
-                  },
-                  ...(filters ?? []),
-                ],
-              }
-            : {}),
-          ...(filters != null ? { filters } : {}),
-          where: where ?? '',
-          whereLanguage: whereLanguage ?? 'sql',
-          timestampValueExpression: sourceObj.timestampValueExpression,
-          implicitColumnExpression: sourceObj.implicitColumnExpression,
-          ...pickSampleWeightExpressionProps(sourceObj),
-          connection: sourceObj.connection,
-          displayType: DisplayType.Search,
-          orderBy: orderBy || defaultSearchConfig?.orderBy || defaultOrderBy,
-        },
+        data: chartConfig,
       };
     }
 
@@ -1324,13 +1308,15 @@ export function DBSearchPage() {
     };
   }, [chartConfig, searchedTimeRange]);
 
-  const displayedColumns = useMemo(
-    () =>
-      splitAndTrimWithBracket(
-        dbSqlRowTableConfig?.select ?? defaultSearchConfig.select ?? '',
-      ),
-    [dbSqlRowTableConfig?.select, defaultSearchConfig.select],
-  );
+  const displayedColumns = useMemo(() => {
+    // `select` is typed as `string | DerivedColumn[]` upstream, but in the
+    // search page we always supply a string. Guard for type safety.
+    const rawSelect =
+      dbSqlRowTableConfig?.select ?? defaultSearchConfig.select ?? '';
+    return splitAndTrimWithBracket(
+      typeof rawSelect === 'string' ? rawSelect : '',
+    );
+  }, [dbSqlRowTableConfig?.select, defaultSearchConfig.select]);
 
   const toggleColumn = useCallback(
     (column: string) => {