fix(search): preserve incompatible filters as inactive when switching data sources

[elizabetdev]

Summary

When the user switched the active source on the search page (e.g. Logs → Traces), any filter whose field doesn't exist on the new source was silently dropped. A transient yellow toast announced it, but the user's selection — and the context for switching back — was gone.

This PR keeps those filters visible in the ActiveFilterPills bar with a muted, strikethrough, dashed-border style and a tooltip explaining why they aren't applied. They're excluded from the rendered query so it stays valid, and re-apply automatically if the user switches back to a compatible source.

Visual treatment

For an inactive pill:

• Strikethrough on field + operator + value
• Opacity 0.75
• Dashed border (var(--color-border-emphasis))
• Tooltip: "Filter not applied: 'SeverityText' isn't a column on the current source. It will reapply if you switch back."

No yellow warning color / warning icon — nothing is broken, it's just informational. Yellow gets noisy when many filters become invalid at once.

API changes

• ActiveFilterPills — new optional props:
  • invalidFields?: Set<string> — fields whose pills should render in the inactive state
  • invalidFieldReason?: (field: string) => string — override the tooltip copy
• useSearchPageFilterState — new optional validFields?: Set<string> input; exposes computed invalidFields. Invalid keys are stripped at query-serialization time so UI state and URL stay decoupled.
• DBSearchPage — passes current source columns as validFields and removes the imperative retainCompatibleFilters drop-on-source-change flow (and its toast notification).

All colors and borders use semantic CSS vars (--color-bg-danger, --color-text-danger, --color-border-emphasis, etc.) per agent_docs/themes.md.

Test plan

• Apply a SeverityText filter on a Logs source, switch to a Traces source. The pill should remain visible with a muted/strikethrough/dashed appearance. Tooltip should explain why.
• Switch back to the Logs source. The pill should reactivate (full opacity, no strikethrough) and the filter should re-apply to the query.
• Apply filters on a nested map/JSON key (e.g. LogAttributes.user). It should be treated as valid as long as the root column exists.
• Click the X on an inactive pill — it removes the filter from state cleanly.
• Clear all clears both active and inactive pills.
• yarn ci:unit (added/updated tests for ActiveFilterPills and useSearchPageFilterState)
• App typecheck + lint

Made with Cursor

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview, Comment	May 28, 2026 10:38am

[changeset-bot]

🦋 Changeset detected

Latest commit: f68cdad

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

[github-actions]

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

• Diff size: 276 production lines changed (Tier 2 max: < 250)

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats

• Production files changed: 3
• Production lines changed: 276 (+ 327 in test files, excluded from tier calculation)
• Branch: fix-filter-pills-inactive-state
• Author: elizabetdev

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

[github-actions]

E2E Test Results

✅ All tests passed • 192 passed • 3 skipped • 1295s

Status	Count
✅ Passed	192
❌ Failed	0
⚠️ Flaky	4
⏭️ Skipped	3

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

Deep Review

Scope: PR #2360 — preserve incompatible filters as inactive when switching data sources. ~250 changed non-test source lines across 3 files plus tests.
Reviewers (10): correctness, testing, maintainability, project-standards, agent-native, learnings-researcher, kieran-typescript, julik-frontend-races, api-contract, adversarial.

🔴 P0/P1 — must fix

• packages/app/src/searchFilters.tsx:459 — On first render, the merge effect at line 428 queues setFilters(merged) but state has not committed yet; the re-emit effect immediately reads filters from its closure as {}, computes stripInvalidFilters({}) === {}, sees it differs from a non-empty parsedQuery.filters, and calls onFilterChange([]) — wiping the URL filters the user just navigated with. Reproduces whenever useColumns returns a cached entry synchronously (common SPA-navigation case: back/forward, saved-search link, recent source).
  • Fix: Gate the re-emit effect on a hasHydratedRef flag that the merge effect sets after its first commit, or compute strippedFromState from parsedQuery.filters when filters is still the initial {}.
  • _{julik-frontend-races, adversarial, kieran-typescript}

🟡 P2 — recommended

• packages/app/src/components/ActiveFilterPills.tsx:137 — Every X button gets the identical aria-label="Remove filter", including across multiple pills on the same page. Screen-reader users cannot distinguish targets, and the test at ActiveFilterPills.test.tsx:304 already has to pick getAllByRole('button', { name: /remove filter/i })[0] blindly.

  • Fix: Interpolate the field and value, e.g. aria-label={\Remove filter ${pill.field} ${operator} ${pill.value}${isInvalid ? ' (inactive)' : ''}`}`.
  • _{kieran-typescript, adversarial}

• packages/app/src/components/ActiveFilterPills.tsx:154 — ActiveFilterPills accepts searchFilters: FilterStateHook (which now exposes invalidFields) and also a separate invalidFields?: Set<string> prop. DBSearchPage.tsx passes both — <ActiveFilterPills searchFilters={searchFilters} invalidFields={searchFilters.invalidFields} ... /> — establishing two sources of truth that callers can desync.

  • Fix: Pick one boundary — drop the separate prop and read searchFilters.invalidFields inside the component, or keep the prop and stop relying on the hook field.
  • _{maintainability, kieran-typescript}

🔵 P3 nitpicks (10)

• packages/app/src/searchFilters.tsx:461 — The re-emit effect calls stripInvalidFilters(filters) but the useCallback defining it sits at line 473, below the use site. Runtime-safe via closure binding at effect-execution time, but any refactor that runs this logic synchronously during render (e.g. useLayoutEffect, flushSync) — including the fix suggested for the P1 above — will hit a TDZ ReferenceError.

  • Fix: Hoist the stripInvalidFilters useCallback above the two effects (re-emit and SQL→Lucene migration) that depend on it.
  • _{correctness, maintainability, kieran-typescript, adversarial}

• packages/app/src/searchFilters.tsx:468 — Re-emit effect deps [validFields] with eslint-disable-next-line react-hooks/exhaustive-deps, but the closure reads filters, parsedQuery.filters, parsedQuery.passthroughFilters, onFilterChange, and stripInvalidFilters. Behavior depends on React giving a fresh closure each render plus the [validFields] trigger — a non-obvious invariant.

  • Fix: Add a comment near the eslint-disable spelling out why stale captures are safe, or move the body into a ref-read so the invariant is self-evident.
  • _{correctness, maintainability, kieran-typescript, julik-frontend-races}

• packages/app/src/searchFilters.tsx:459 — A degenerate watchedSourceColumns = [] makes validFields = new Set() (defined but empty). isKeyAllowed(any, emptySet) is always false, so the re-emit effect strips every filter and emits onFilterChange([]). If the user navigates away before the columns query refetches with real data, the URL is permanently cleared.

  • Fix: Treat an empty validFields as undefined — either gate the effect (if (!validFields || validFields.size === 0) return) or memoize validFields to undefined when watchedSourceColumns?.length === 0 at the call site.
  • _{correctness, adversarial}

• packages/app/src/searchFilters.tsx:610 — retainFiltersByColumns is now dead in production (the only callers after this PR are its own unit tests and the ActiveFilterPills test mock). Comment marks it deprecated but it remains exported.

  • Fix: Delete the method, its dedicated unit tests, and the invalidFields: new Set() mock fixture parity entry; or replace the comment with a @deprecated JSDoc and a removal target.
  • _{correctness, maintainability}

• packages/app/src/searchFilters.tsx:638 — invalidFields is returned as Set<string>, the same memoized reference each render. Consumers can mutate it via .add() / .delete() and corrupt subsequent reads.

  • Fix: Type the return as ReadonlySet<string> — ActiveFilterPills only calls .has().
  • _{kieran-typescript}

• packages/app/src/searchFilters.tsx:459 — Navigating from a search URL to a saved-search whose source has cached columns can drop the saved search's filters: same-render parsedQuery.filters change + validFields change makes the re-emit effect see stale filters (pre-merge) and emit onFilterChange([]). Same closure-capture root cause as the P1 but a different trigger.

  • Fix: Same hasHydratedRef gate fixes both cases.
  • _adversarial

• packages/app/src/components/ActiveFilterPills.tsx:91 — Excluded-but-not-invalid pills shift visual treatment: var(--mantine-color-red-light) → var(--color-bg-danger) for background, and red.4 → var(--color-text-danger) for text/icon. In hyperdx-light --color-bg-danger resolves to red-8, a much darker shade than red-light — likely a visible regression for existing excluded pills, separate from the intentional inactive-pill change.

  • Fix: If the color shift on non-invalid excluded pills was unintentional, gate the new tokens behind isInvalid and keep mantine-color-red-light / red.4 for the existing excluded case.
  • _api-contract

• packages/app/src/components/ActiveFilterPills.tsx:115 — Operator <Text> now passes c="dimmed" and an inline style={{ color: showDangerAccent ? 'var(--color-text-danger)' : undefined }}. Mantine applies c via a generated CSS class; the inline style wins by specificity today but couples the visual to Mantine's class-vs-inline order.

  • Fix: Switch to c={showDangerAccent ? 'var(--color-text-danger)' : 'dimmed'} (or pass the color via c only), so the operator color flows through one Mantine prop instead of two competing mechanisms.
  • _api-contract

• packages/app/src/components/ActiveFilterPills.tsx:135 — <ActionIcon variant="transparent" color="gray"> violates agent_docs/code_style.md: the allowed variant set is primary | secondary | danger | link | subtle, and color="gray" is listed under forbidden patterns. The diff modified these lines without aligning to the standard.

  • Fix: Switch to variant="subtle" (called out for close buttons) and drop color="gray"; danger styling is already applied via the inline style override.
  • _{project-standards}

• packages/app/src/searchFilters.tsx:391 — isKeyAllowed only splits on the first dot, so a column literally named with a dot (e.g. My.Col) plus a nested filter key like My.Col.field falsely fails both has('My.Col.field') and has('My') and gets marked invalid. Pre-existing logic, but its blast radius widens — the new re-emit effect strips on every render, not just on source switch.

  • Fix: Iterate dot-prefixes of the key (My, My.Col, My.Col.field) and accept if any are in allowedColumnNames, or document the limitation.
  • _adversarial

---

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

Testing gaps:

• No test for the cached-columns initial-mount path (the trigger for the P1 wipe).
• No test for the SQL→Lucene migration interacting with validFields (the migration effect's dep on stripInvalidFilters is uncovered).
• No test for clearAllFilters while invalid filters are present.
• No negative test for nested keys (LogAttributes.user when LogAttributes is NOT a valid column).
• No multi-value X-button-removal test for inactive pills (the existing test uses ambiguous getAllByRole(...)[0]).
• No assertion that the e2e round-trip reactivation preserves the originally selected value, only that the field name reappears in the URL.

Note (pre-existing, not blocking): A user switching to a source whose useColumns takes >1s to return can trigger a ClickHouse error toast because debouncedSubmit fires the chart query with the still-stale filters before the re-emit effect strips them. The prior imperative flow had the same race; the new reactive flow makes it more visible because there is no longer a UI signal explaining the discrepancy.