feat: preserve compatible filters when switching sources

[karl-power]

Summary

Switching the active source on the /search page used to clear all sidebar filters unconditionally. This was added (HDX-1176, HDX-1258, HDX-2545) to prevent broken queries when columns from the old source didn't exist on the new one, but users keep asking for filters to survive across compatible sources.

This PR revisits that tradeoff with a per-filter rule: when the source changes, keep filters whose root column exists on the new source's schema, drop the rest, and surface a yellow toast naming the count of dropped filters.

• searchFilters.tsx — new retainFiltersByColumns(allowed: Set<string>) helper on useSearchPageFilterState. Returns the keys of dropped filters and syncs URL state via updateFilterQuery only when something actually changed. Root-column extraction handles both top-level columns (ServiceName) and nested JSON/Map keys (LogAttributes.user.id → root LogAttributes), with an exact-match fallback for the rare case of a column whose name contains dots.
• DBSearchPage.tsx — replaced the unconditional clearAllFilters() call in the source-change effect with a pendingFilterReconcileRef. A follow-up effect runs once useColumns for the new source resolves and calls retainFiltersByColumns. The deep-link "open this trace by ID" path (onDirectTraceSourceChange) is intentionally untouched — it builds a fresh SQL where clause from scratch and is a different flow.
• Toast — notifications.show({ color: 'yellow', message: 'N filter(s) didn't apply to this source and were removed.' }) fires only when at least one filter was dropped.

Coverage: 6 unit tests for retainFiltersByColumns (empty, all-kept, nested keys, all-dropped, mixed, passthrough preservation) and 3 Playwright E2E tests (compatible → preserved + no toast, mixed → toast + URL keeps shared filter, all-dropped → toast). Existing mocks in ActiveFilterPills.test.tsx and DBSearchPage.directTrace.test.tsx were updated for the new hook return shape.

Screenshots or video

Screen.Recording.2026-05-20.at.16.14.59.mov

How to test on Vercel preview

Preview routes: /search

Steps:

1. Open /search and select a logs source from the source dropdown.
2. In the sidebar, open the ServiceName filter group and check any value.
3. Open the SeverityText filter group and check info.
4. Switch the source dropdown to a traces source.
5. Verify a yellow toast appears reading 1 filter didn't apply to this source and was removed. (the SeverityText filter is dropped because traces lack that column).
6. Re-open the ServiceName filter group and confirm the URL's filters= param still contains the ServiceName filter.
7. Switch back to the logs source. Verify no toast appears (the ServiceName filter is still compatible) and the URL retains the filter.

References

• Linear Issue: Closes HDX-4254
• Related PRs: —

[changeset-bot]

🦋 Changeset detected

Latest commit: 27f5bd8

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

This PR includes changesets to release 3 packages

Name	Type
@hyperdx/app	Patch
@hyperdx/api	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]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview, Comment	May 22, 2026 3:17pm

[github-actions]

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

• Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats

• Production files changed: 2
• Production lines changed: 95 (+ 280 in test files, excluded from tier calculation)
• Branch: karl/preserve-compatible-filters-when-switching-sources
• Author: karl-power

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

[github-actions]

PR Review

✅ No critical issues found.

A few minor observations (non-blocking):

• ℹ️ Reconcile effect depends on the entire searchFilters object → effect re-runs on every render of useSearchPageFilterState (the hook returns a fresh object each render and retainFiltersByColumns is recreated whenever filters changes). The pendingFilterReconcileRef/!watchedSourceColumns guard makes this a cheap no-op, but you could depend on searchFilters.retainFiltersByColumns directly to make the intent clearer.
• ℹ️ If useColumns errors or never resolves for the new source, pendingFilterReconcileRef is never cleared and filters are silently left in place (mild behavior regression vs. the old unconditional clear). Probably acceptable since the worst case is a stale filter the user can clear manually, but worth noting — a fallback timeout or "clear on error" path would close the gap.
• ℹ️ Empty watchedSourceColumns ([] returned successfully) would drop every filter and show "N filters didn't apply" — unlikely in practice but a corner case the tests don't cover. Consider guarding on watchedSourceColumns.length > 0 if you want to be defensive.
• ℹ️ Root-column extraction uses key.indexOf('.') with an exact-match fallback. This is correct for the current parseKeyPath output (top-level columns never contain dots; bracketed keys join as Root.nested.path). Worth a brief comment pointing back to parseKeyPath so the invariant is discoverable from searchFilters.tsx.
• ℹ️ Toast message names a count but not which filters were dropped — fine per the PR description, just flagging that returning dropped from retainFiltersByColumns is currently used only for the count; a future iteration could surface the names.

Implementation looks well-scoped, with thorough unit coverage (including passthrough preservation) and three E2E scenarios. Direct-trace flow correctly left untouched.

[github-actions]

E2E Test Results

❌ 1 test failed • 180 passed • 3 skipped • 1223s

Status	Count
✅ Passed	180
❌ Failed	1
⚠️ Flaky	4
⏭️ Skipped	3

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

Deep Review

Reviewed against base 8938b05e with 10 reviewers (4 always-on, 4 stack-specific, 2 CE always-on). The dominant finding is a silent revert of a fix that landed in the very base commit: the branch was forked before PR #2329 and not rebased, so the diff against the new base inverts that fix line-for-line plus deletes its changeset. Six reviewers independently confirmed it.

🔴 P0/P1 -- must fix

• packages/app/src/components/DBSearchPageFilters.tsx:1340-1387 -- this hunk removes the strippedFilterState block, drops the filtersToQuery import, and removes filterState from the loadMoreFilterValuesForKey deps; combined with the deletion of .changeset/purple-peaches-smash.md, it line-for-line inverts PR fix: let "Load more" surface unselected values in exact filter mode #2329 (the fix that landed in this PR's own base SHA 8938b05e), reintroducing the bug where "Load more" is a silent no-op for self-restricted dropdowns in exact filter mode.

  • Fix: Rebase the branch onto current main (which already contains 8938b05e); do not re-author the DBSearchPageFilters.tsx hunk by hand. After rebase, confirm git diff main -- packages/app/src/components/DBSearchPageFilters.tsx is empty and that .changeset/purple-peaches-smash.md is restored.
  • _{correctness, maintainability, project-standards, kieran-typescript, reliability, adversarial}

• packages/app/src/hooks/useMetadata.tsx:91 -- useColumns's React-Query key is ['useMetadata.useColumns', { databaseName, tableName }] with no connectionId; this PR newly hangs a destructive operation (retainFiltersByColumns) on that result, so two sources sharing the same databaseName + tableName across connections will reconcile against the wrong cached column set and either drop valid filters or retain incompatible ones, producing Unknown identifier errors from ClickHouse.

  • Fix: Include connectionId in the query key: ['useMetadata.useColumns', { connectionId, databaseName, tableName }] — the underlying metadata.getColumns cache already keys on the full tuple, so only the React-Query layer is wrong.
  • _{kieran-typescript, julik-frontend-races, adversarial}

• packages/app/src/DBSearchPage.tsx:1117-1164 -- the non-saved-search branch fires debouncedSubmit() (1000ms debounce) at line 1129 immediately on source change, while the reconcile effect at line 1157 waits for a network round-trip to resolve useColumns; if the column fetch exceeds the debounce window, the chart re-queries the new source with the still-incompatible old filters and surfaces a ClickHouse query error before reconcile lands — pre-PR clearAllFilters() ran synchronously and avoided this window entirely.

  • Fix: Either gate the new source's first debouncedSubmit() on watchedSourceColumns having loaded, or have only the reconcile effect submit (let the trailing setValue('filters', kept) re-arm the debounce); a synchronous fast-path that reads queryClient.getQueryData(...) for already-cached columns would close the window.
  • _{correctness, julik-frontend-races}

🟡 P2 -- recommended

• packages/app/src/DBSearchPage.tsx:1092-1164 -- useColumns failures are silently swallowed: only data is destructured, so on network error / RBAC denial / dropped table the reconcile gate watchedSourceColumns never opens, pendingFilterReconcileRef.current stays set indefinitely, and the chart keeps firing queries against the new source with incompatible old filters with no fallback or recovery.

  • Fix: Destructure isError from useColumns and either fall back to clearAllFilters() on error (matches prior behavior) or surface a distinct toast that names the failure; clear pendingFilterReconcileRef.current either way so the effect doesn't dangle.
  • _reliability

• packages/app/src/DBSearchPage.tsx:1129 -- the unconditional debouncedSubmit() inside the if (newInputSourceObj != null) block now fires on the saved-search-matches branch too, contradicting the inline comment at line 1124 (Don't clear filters - we're loading from saved search) and producing a redundant URL update / chart re-query per saved-search source switch.

  • Fix: Move debouncedSubmit() inside the if (savedSearchId == null || savedSearch?.source !== watchedSource) branch so the saved-search path stays a pure load.
  • _{correctness, julik-frontend-races}

• packages/app/src/DBSearchPage.tsx:1083-1164 -- only the pure helper retainFiltersByColumns has Jest coverage; the orchestration (pendingFilterReconcileRef lifecycle, the pending === watchedSource && watchedSourceColumns gate, race protection) is verified by Playwright only, and the new mock in DBSearchPage.directTrace.test.tsx returns data: undefined which short-circuits the reconcile effect, so a future regression in the ref lifecycle would not be caught by any unit suite.

  • Fix: Add a Jest test that renders DBSearchPage with a non-undefined useColumns mock, drives a source switch, and asserts (a) compatible filters kept, (b) incompatible filters dropped, (c) toast fires with correct pluralization, (d) saved-search-source-matches branch leaves the ref null and shows no toast.
  • _{correctness, testing, maintainability}

• packages/app/src/DBSearchPage.tsx:1689-1140 -- the PR description claims onDirectTraceSourceChange is intentionally untouched, but the flow onDirectTraceSourceChange → setSearchedConfig → URL-sync effect → reset({ source }) → watchedSource changes → source-change effect at line 1101 fires the new pendingFilterReconcileRef machinery and debouncedSubmit(); today the form's filters are wiped to [] by the reset so retainFiltersByColumns no-ops, but any future change that leaves filters intact during direct-trace transition silently flows through reconcile.

  • Fix: Inside onDirectTraceSourceChange, set prevSourceRef.current = nextSource.id (and leave pendingFilterReconcileRef null) so the source-change effect short-circuits, or add an explicit unit test pinning the coupling and document it next to the inline comment.
  • _adversarial

• packages/app/src/DBSearchPage.tsx:1149-1154 -- notifications.show({ color: 'yellow', message: ... }) does not dedupe by id; a user iterating through sources to compare schemas will stack one yellow toast per source change with dropped filters, obscuring the page until each auto-closes.

  • Fix: Pass id: 'filters-dropped-on-source-change' to notifications.show (or use notifications.update once a toast already exists) so subsequent firings replace the prior toast.
  • _reliability

🔵 P3 nitpicks (4)

• packages/app/src/searchFilters.tsx:539-541 -- root-column extraction via key.indexOf('.') plus the exact-match fallback for "columns whose name contains dots" has no test exercising the fallback; a JSON column literally named My.Col with a nested key My.Col.user.id would slice to root My and miss both has(key) and has(root), silently dropping the filter.

  • Fix: Either iterate allowedColumnNames and accept any allowed such that key === allowed || key.startsWith(\${allowed}.`)`, or drop the fallback branch entirely and add a test pinning the chosen behavior.

• packages/app/src/DBSearchPage.tsx:1092-1099 -- the spread ...options sits after useColumns's built-in enabled: !!databaseName && !!tableName && !!connectionId guard, so the call-site override enabled: !!watchedSourceObj re-enables the query with the ?? '' empty-string fallbacks; the zod source schema currently prevents empty values from reaching this path, but the override removes the safety net.

  • Fix: Drop the call-site enabled and let the hook's default guard apply, or compose: enabled: !!watchedSourceObj?.from?.databaseName && !!watchedSourceObj?.from?.tableName && !!watchedSourceObj?.connection.

• packages/app/src/DBSearchPage.tsx:790-794 -- formatDroppedFiltersMessage is exercised only via Playwright's tolerant regex /filter.* didn't apply to this source/i, which matches both singular and plural; a future regression to "1 filters was removed" would not fail CI.

  • Fix: Export the helper and add a two-case Jest test for count === 1 and count > 1.

• packages/app/src/DBSearchPage.tsx:27 -- ColumnMeta is imported in value position from @hyperdx/common-utils/dist/clickhouse but is only used as a type annotation on line 1142; the bundler retains an unused module reference.

  • Fix: Move ColumnMeta to an import type line.

---

Reviewers (10): correctness, testing, maintainability, project-standards, kieran-typescript, julik-frontend-races, reliability, adversarial, agent-native, learnings-researcher.

Testing gaps:

• No Jest test exercises the full DBSearchPage reconcile orchestration (pendingFilterReconcileRef + useColumns + retainCompatibleFilters) — feature is E2E-only.
• No test covers the >1000ms column-fetch case where debouncedSubmit fires before reconcile.
• No test covers the saved-search-source-matches branch (filter preservation, no toast, no submit).
• No regression test pins loadMoreFilterValuesForKey's exact-mode behavior — its absence is what allowed the P0 revert to land without a CI failure.
• No test exercises rapid source-switching against a delayed useColumns resolve.

[knudtty]

nothing blocking