Apply saved-search filters to alert sample queries

[Tyagiquamar]

Fixes #3059.

Why

A saved search scoped through the filter bar evaluated correctly, but the sample events rendered into the notification came from an unfiltered query, so alerts showed unrelated log lines as evidence next to a correct count.

What changed

The sample fetch in renderAlertTemplate hand-built its chart config from where/whereLanguage only. It now also passes savedSearch.filters, the same input the evaluation path feeds buildSearchChartConfig, so renderChartConfig narrows both queries to the same row set. TILE and INLINE paths are untouched. Includes a changeset (patch, @hyperdx/api).

Tests

• New integration case in renderAlertTemplate.int.test.ts renders a saved-search alert whose search carries a pinned filter and asserts the generated sample SQL contains the filter condition. It fails on unpatched main and passes with the fix.
• Targeted run against local ClickHouse/Mongo services: 1 passed.
• ESLint on touched files: 0 errors (8 pre-existing warnings elsewhere in the files); Prettier clean.

[changeset-bot]

🦋 Changeset detected

Latest commit: 3b15bea

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@hyperdx/api	Patch
@hyperdx/app	Patch
@hyperdx/otel-collector	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

@Tyagiquamar is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions]

Hi @Tyagiquamar, thanks for the pull request!

Before we review code from a first-time contributor we ask that a maintainer vouches for you, and you're not on our list yet. This PR stays open — it just isn't in the review queue until someone vouches.

To get vouched, open an issue saying hello and what you're working on:

https://github.com/hyperdxio/hyperdx/issues/new?template=introduce-yourself.md

A maintainer will usually reply within a day or two, and then this PR gets picked up as normal. More detail in our contributing guide.

[greptile-apps]

Greptile Summary

The PR routes saved-search alert sample queries through the shared search-chart builder so pinned and source-level filters match the evaluated row set.

• Adds pinned filters to notification sample queries.
• Adds integration coverage for the generated ClickHouse SQL.
• Adds a patch changeset for @hyperdx/api.

Confidence Score: 4/5

The PR should not merge until the ClickHouse test mock is assignable to the renderAlertTemplate client parameter.

The production query change consistently applies the shared saved-search configuration, but the test's new Pick<ClickhouseClient, 'query'> annotation cannot be passed where a full ClickhouseClient is required and breaks type-checking. The earlier untyped-mock finding was manually resolved without explanation.

Files Needing Attention: packages/api/src/tasks/checkAlerts/tests/renderAlertTemplate.int.test.ts

Important Files Changed

Filename	Overview
packages/api/src/tasks/checkAlerts/template.ts	Replaces hand-built sample-query configuration with the shared saved-search chart builder and passes pinned filters.
packages/api/src/tasks/checkAlerts/tests/renderAlertTemplate.int.test.ts	Adds pinned-filter SQL coverage, but the newly narrowed client type is incompatible with the function parameter.
.changeset/silent-pandas-search.md	Records the alert sample-query correction as an API patch release.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Saved search] --> B[Shared search chart-config builder]
    B --> C[Alert evaluation query]
    B --> D[Notification sample query]
    C --> E[Alert count]
    D --> F[Filtered sample log lines]

Loading

_{Reviews (2): Last reviewed commit: "fix(alerts): reuse search config for sam..." | Re-trigger Greptile}

[github-actions]

Deep Review

✅ No critical issues found. The core change is correct: the sample-log fetch in the SAVED_SEARCH branch of renderAlertTemplate now routes through buildSearchChartConfig(source, {...}) — the same builder the evaluation/count path (getChartConfigFromAlert) uses — so both queries narrow to the same row set. This resolves the earlier "sample query isn't the counted row set" feedback: source.tableFilterExpression is now prepended as the first filter and bodyExpression is passed for bare-text Lucene, both of which the prior hand-built config omitted.

Verified independently:

• Row-set parity — both call sites pass where, whereLanguage, filters, and dateRangeStartInclusive/EndInclusive identically; the sample path correctly diverges only on select (raw columns vs ALERT_COUNT_DEFAULT_SELECT), displayType, and granularity, which is intended.
• Select fallback equivalence — the removed savedSearch.select || source.defaultTableSelectExpression || '' is behaviorally preserved by resolveSelect, which treats empty string and empty array as "fall back to default."
• No mutation hazard — passing savedSearch.filters by reference is safe because buildSearchChartConfig spreads filters into a new array rather than mutating the input, so omitting the eval path's defensive .map(f => ({...f})) clone does not introduce shared-state risk.
• Regression test — the new case asserts the generated sample SQL contains the pinned filter condition, captures the only ClickHouse query issued on this path, and fails on unpatched main.

🔵 P3 nitpicks (1)

• packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts:728 — the new capturingClickhouseClient mock is typed Pick<ClickhouseClient, 'query'> but then cast as any with an eslint-disable for no-unsafe-type-assertion, which loosens the type contract the Pick annotation implies; this matches the existing as any mock convention already used throughout this file (lines 77, 86–91, 338) and is test-only, so it changes no behavior.
  • Fix: narrow the assertion to as unknown as Pick<ClickhouseClient, 'query'> so the mock keeps a checked shape without the blanket any.

---

Reviewers (5): correctness, testing, project-standards, kieran-typescript, previous-comments.

Previous comments: The major "sample query still isn't the counted row set (tableFilterExpression/bodyExpression missing)" comment is now addressed by routing through buildSearchChartConfig. The prior P2 "untyped ClickHouse mock" comment remains as the P3 above.

[github-actions]

🟠 major — Sample query still isn't the counted row set: tableFilterExpression (and bodyExpression) are missing

Build this config with buildSearchChartConfig(source, { where, whereLanguage, filters, select: savedSearch.select, orderBy, connection: '', dateRange: [startTime, endTime], dateRangeStartInclusive: true, dateRangeEndInclusive: false }) (packages/common-utils/src/core/searchChartConfig.ts:120) and then add limit/with, instead of adding filters to a hand-built twin. The evaluation path goes through that builder (packages/api/src/tasks/checkAlerts/index.ts:817), which (a) prepends a Log source's tableFilterExpression as the first SQL filter — pinned by checkAlerts.int.test.ts:2486, so a source with tableFilterExpression: "ServiceName != 'excluded'" counts 1 row while this sample query still returns the excluded row into the webhook body — and (b) passes bodyExpression, without which a Log source that sets only bodyExpression (no implicitColumnExpression) makes getColumnForField throw "Can not search bare text without an implicit column set" (packages/common-utils/src/queryParser.ts:1942) for a bare-text Lucene search or Lucene filter; the throw is swallowed by the catch at line 853 and the notification renders an empty code block. Reusing the builder also removes the duplicated select fallback at line 800 (resolveSelect, searchChartConfig.ts:94).

[github-actions]

PR Review

3 finding(s): 🔴 0 critical · 🟠 2 major · 🔵 1 minor

2 posted as inline comment(s) on the changed lines. 1 listed below.

Findings outside the changed lines

• 🟠 packages/api/src/tasks/checkAlerts/template.ts:14 — DisplayType and pickSampleWeightExpressionProps imports are now unused — yarn lint fails → The hand-built config was the only user of both symbols (grep shows DisplayType only at line 14 and pickSampleWeightExpressionProps only at line 17 now that buildSearchChartConfig supplies displayType and the sample-weight props). packages/api/eslint.config.mjs:47 sets @typescript-eslint/no-unused-vars to error, and .github/workflows/main.yml:51 runs ci:lint (yarn lint && yarn tsc --noEmit), so this fails CI. Remove both names from the @hyperdx/common-utils/dist/types import.

---

_{Severity is the reviewer's own estimate and is used for ordering, not filtering.}

[pulpdrew]

/vouch @Tyagiquamar

[pulpdrew]

Please fix the lint errors (unused and out of order imports, and an incorrect type for capturingClickhouseClient)