Skip to content

feat(dashboards): cascading (faceted) filter values#2423

Draft
teeohhem wants to merge 8 commits into
mainfrom
la-paz
Draft

feat(dashboards): cascading (faceted) filter values#2423
teeohhem wants to merge 8 commits into
mainfrom
la-paz

Conversation

@teeohhem
Copy link
Copy Markdown
Contributor

@teeohhem teeohhem commented Jun 5, 2026

Summary

Adds opt-in linked (faceted) filter values to the dashboard and Kubernetes filter bars. A bidirectional-arrow "link filters" toggle at the end of each bar (off by default) turns on filter-awareness: each dropdown then only shows values that co-occur with the other current selections — e.g. picking a cluster narrows the namespace dropdown to namespaces in that cluster (the K8s bar also factors in the free-text search). A filter never constrains its own options, so multi-select still works. When linked, a dropdown's narrowed values are fetched lazily only when it is opened, which bounds the cost of these contingent lookups — they can't be served from the cheap per-key rollups and are far more expensive at scale, so they stay opt-in and off by default. Search-page filters are intentionally untouched. Addresses HDX-4462.

Screenshots or video

No screenshots: the local/preview demo dataset has no Kubernetes telemetry, so the K8s dropdowns are empty there. The narrowing was instead verified against live ClickHouse using the traces source — selecting ServiceName = hdx-oss-dev-app narrowed the SpanName dropdown from 21 → 9 co-occurring values (and the reverse narrowed ServiceName to a single service).

How to test on Vercel preview

Preview routes: /kubernetes

Steps:

  1. Open /kubernetes.
  2. Verify the filter bar renders the Cluster and Namespace dropdowns (cluster-filter-select, namespace-filter-select) and the link-filters toggle (k8s-filters-link-toggle).
  3. Click the link-filters toggle and confirm it becomes active (pressed) without any console errors.

References

Dashboard filter dropdowns now narrow one another: selecting a value in one filter constrains the options shown by the others to values that co-occur with the current selection (e.g. picking a cluster limits the namespace dropdown to namespaces in that cluster). Applies to both manually-created dashboards and the bundled Kubernetes page (which also honors its free-text search). A filter never constrains its own options, so multi-select within a filter still works.

HDX-4462
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 5, 2026

🦋 Changeset detected

Latest commit: d6e724a

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
Copy link
Copy Markdown

vercel Bot commented Jun 5, 2026

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

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Jun 5, 2026 7:37pm
hyperdx-storybook Ready Ready Preview, Comment Jun 5, 2026 7:37pm

Request Review

@github-actions github-actions Bot added the review/tier-2 Low risk — AI review + quick human skim label Jun 5, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

🔵 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: 3
  • Production lines changed: 179 (+ 265 in test files, excluded from tier calculation)
  • Branch: la-paz
  • Author: teeohhem

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
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

E2E Test Results

1 test failed • 196 passed • 3 skipped • 1362s

Status Count
✅ Passed 196
❌ Failed 1
⚠️ Flaky 5
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

Deep Review

✅ No critical issues found. The faceting design is sound — constraints are AND-combined in renderChartConfig, cross-source leakage is correctly prevented (source/sourceMetricType guard), exclude-self preserves multi-select, and value strings are SQL-escaped by filtersToQuery. The items below are recommended improvements; none is a ship-blocker.

🟡 P2 — recommended

  • packages/app/src/hooks/useDashboardFilterValues.tsx:244placeholderData keys on a prefix (from + keys) that omits the faceting constraints, so on the first selection the constrained query reuses the previously-cached unconstrained result and the dropdown momentarily shows the full list before narrowing.
    • Fix: Include the constraint signature in queryKeyPrefix, or replace the manual placeholderData factory with keepPreviousData so only a matching constraint set is reused.
    • julik-frontend-races
  • packages/app/src/hooks/useDashboardFilterValues.tsx:95 — Faceting builds constraints with filtersToQuery(prunedState, { stringifyKeys: false }), but the canonical tile-application path uses stringifyKeys: true (wrapping keys in toString() for JSON/Dynamic columns), so a faceted value query can match differently than the filter it mirrors and narrow a sibling dropdown to the wrong or empty set.
    • Fix: Use the same stringifyKeys value as the filter-application path so faceted narrowing is consistent with how the selection is applied to tiles.
    • correctness, maintainability
  • packages/app/src/components/KubernetesFilters.tsx:40stripFieldClause (and extractValueFromSearchQuery at line 135) interpolate the dotted attribute into new RegExp without escaping, so the literal dots act as wildcards and can over-match an unrelated clause; stripFieldClause now runs on every render via buildChartConfigForField.
    • Fix: Escape resourceAttr and field (e.g. s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) before constructing the RegExp in both functions.
    • correctness, adversarial, testing, maintainability, kieran-typescript, julik-frontend-races, project-standards
  • packages/app/src/hooks/useDashboardFilterValues.tsx:107 — Each selected filter now gets a distinct constraint signature and splits into its own getKeyValues query (up to K constrained scans for K selections where one batched scan ran before), each with disableRowLimit: true.
    • Fix: Confirm the per-selection query fan-out is acceptable for large dashboards, or debounce/coalesce constrained value queries on rapid selection changes.
    • performance
  • packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx:1059 — Faceting tests cover only included selections; the excluded (NOT IN) and range (BETWEEN) constraint branches and the multi-selection grouping path (3+ distinct constraint sets) have no coverage.
    • Fix: Add faceting tests asserting the constraint SQL for excluded-only and range selections, and for two simultaneously-selected same-source filters.
    • testing, kieran-typescript, maintainability
🔵 P3 nitpicks (6)
  • packages/app/src/components/KubernetesFilters.tsx:179buildChartConfigForField is called inline in JSX, creating five fresh chartConfig objects on every render; React Query deep-compares the queryKey so no request storm results, but every parent re-render re-evaluates all five keys.
    • Fix: Memoize the per-field configs with useMemo keyed on [searchQuery, metricSource, dateRange].
    • maintainability, kieran-typescript, julik-frontend-races
  • packages/app/src/hooks/useDashboardFilterValues.tsx:218 — The filterValues: FilterState = {} default allocates a new object on every call that omits the argument, re-running the constraintsByFilterId memo (which lists filterValues in its deps) unnecessarily.
    • Fix: Hoist a module-level EMPTY_FILTER_STATE sentinel and default to it.
    • kieran-typescript
  • packages/app/src/hooks/useDashboardFilterValues.tsx:114 — The constraint signature joins serialized filters with a bare |, which can appear inside a SQL condition string and collide two distinct constraint sets into one group.
    • Fix: JSON.stringify the sorted array of serialized filters instead of join('|').
  • packages/app/src/hooks/useDashboardFilterValues.tsx:139 — The whereLanguage ?? 'sql' fallback is dead code; DashboardFilter.whereLanguage is a required schema field.
    • Fix: Drop the fallback, or add a comment if it is intentional defense-in-depth.
  • packages/app/src/hooks/useDashboardFilterValues.tsx:313 — The file is now 313 lines, over the repo's 300-line file limit (AGENTS.md, agent_docs/code_style.md).
    • Fix: Extract the inner useOptimizedKeyValuesCalls hook into its own module.
    • project-standards
  • packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx:1022 — The new filtersForKeys helper casts mock.calls as any[], so a typo in the property path would silently return undefined and make assertions vacuously pass.
    • Fix: Type the mock accessor from jest.mocked(...) instead of as any[].
    • project-standards, kieran-typescript

Reviewers (8): correctness, adversarial, testing, maintainability, kieran-typescript, julik-frontend-races, performance, project-standards.

Testing gaps:

  • No component/integration test for KubernetesFilters wiring (buildChartConfigForField, whereLanguage: 'lucene'); only the pure stripFieldClause helper is tested.
  • extractValueFromSearchQuery has no tests (absent field, value-with-spaces, prefix-sharing siblings).
  • No stripFieldClause test exercises an unescaped-dot over-match or regex-special characters in the attribute.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 5, 2026

Greptile Summary

This PR adds opt-in linked (faceted) filter values to the Dashboard and Kubernetes filter bars. A toggle at the end of each bar enables filter-awareness: each dropdown only shows values that co-occur with the other current selections. Lazy fetching bounds the cost — a dropdown's constrained values are only fetched when it is opened.

  • useDashboardFilterValues is a new hook that computes per-filter constraint sets, groups filters by (source, metricType, where, constraint-sig) for batching, and wires lazy-mode gating via activeFilterIds.
  • KubernetesFilters gains a FilterLinkToggle and a local opened state per FilterSelect; stripFieldClause (now with escapeRegExp) strips each field's own clause from the shared Lucene where when building faceted configs.
  • VirtualMultiSelect receives new onDropdownOpen/onDropdownClose callbacks and a loading prop that surfaces a loading hint in the empty state.

Confidence Score: 5/5

Safe to merge; the new faceted-filter logic is well-tested, off by default, and isolated from the search-page filters.

The constraint-building, batching, and lazy-fetch logic are all correctly implemented and covered by comprehensive unit tests. The two isLoading vacuous-truth edge cases and the openFilterIds stale-state edge case are minor quality issues that don't affect observable behaviour in current consumers.

No files require special attention.

Important Files Changed

Filename Overview
packages/app/src/hooks/useDashboardFilterValues.tsx New hook implementing faceted/constrained filter-value fetching; constraints-by-filter-id logic is correct, lazy-mode gating is correct, but results.every(r => r.isLoading) returns true vacuously for an empty array.
packages/app/src/DashboardFilters.tsx Adds linked-mode state, openFilterIds tracking, and lazy-open callbacks; logic is sound but openFilterIds is never cleared when linked mode is toggled off.
packages/app/src/components/KubernetesFilters.tsx Adds FilterLinkToggle and local opened state to each FilterSelect; escapeRegExp applied correctly; chartConfigs memoized; lazy fetch logic is correct.
packages/app/src/components/FilterLinkToggle.tsx Simple, well-scoped toggle component with accessible aria-pressed and aria-label attributes.
packages/app/src/components/VirtualMultiSelect/VirtualMultiSelect.tsx Adds onDropdownOpen/onDropdownClose callbacks and a loading prop; delegated correctly to the Mantine Combobox store.
packages/app/src/hooks/tests/useDashboardFilterValues.test.tsx Comprehensive test coverage including faceted filtering, lazy mode, error paths, and placeholder-data behaviour.
packages/app/src/components/tests/KubernetesFilters.test.ts Tests stripFieldClause for prefix-match safety and regex metacharacter escaping; good edge-case coverage.

Sequence Diagram

sequenceDiagram
    participant U as User
    participant DF as DashboardFilters
    participant UDFV as useDashboardFilterValues
    participant OKVC as useOptimizedKeyValuesCalls
    participant RQ as React Query

    U->>DF: Toggle Link filters ON
    DF->>UDFV: filterValues, activeFilterIds empty
    Note over UDFV: No dropdowns open — no fetches

    U->>DF: Open dropdown for Filter A
    DF->>UDFV: activeFilterIds includes A
    UDFV->>OKVC: Build constraints (sibling selections, exclude self)
    OKVC->>RQ: queryKey with constraints_for_A
    RQ-->>UDFV: constrained values for A
    UDFV-->>DF: Map with A values
    DF-->>U: Dropdown shows narrowed options

    U->>DF: Select value in Filter A
    DF->>UDFV: filterValues updated
    Note over UDFV: Constraints for open siblings recalculated
    UDFV->>OKVC: New constraint sig for open filters
    OKVC->>RQ: New queryKey for affected open filters
    RQ-->>UDFV: Re-narrowed values
    DF-->>U: Other open dropdowns refresh
Loading

Fix All in Claude Code Fix All in Conductor Fix All in Cursor Fix All in Codex

Reviews (5): Last reviewed commit: "style(dashboards): use themed 'secondary..." | Re-trigger Greptile

Comment thread packages/app/src/components/KubernetesFilters.tsx
Comment thread packages/app/src/components/KubernetesFilters.tsx Outdated
stripFieldClause and extractValueFromSearchQuery interpolated the attribute name straight into a RegExp. Escape it (lodash escapeRegExp) so dots match literally instead of as wildcards, and metacharacters like '(' or '[' in a resource-attribute expression can't throw a SyntaxError.

Memoize the five FilterSelect chart configs in a single useMemo (keyed on searchQuery, dateRange, metricSource) so they keep a stable identity across re-renders unrelated to the filters, avoiding repeated React Query key serialization. (useCallback on the builder wouldn't help: calling it still mints a new object per render.)

Add tests covering literal-dot matching and the metacharacter case.
@teeohhem
Copy link
Copy Markdown
Contributor Author

teeohhem commented Jun 5, 2026

Outstanding discussions:

  1. Make this an option users can turn on?
  2. Performance implications

teeohhem added 2 commits June 5, 2026 14:52
Add a bidirectional-arrow toggle at the end of the dashboard and Kubernetes filter bars that opts into linked (faceted) filter values, off by default. When linked, each dropdown's values are narrowed by the other selections (and the K8s free-text search) and fetched lazily only when the dropdown is opened, bounding the cost of contingent value lookups that can't use per-key rollups. Search-page filters are untouched.
Copy link
Copy Markdown
Contributor

@pulpdrew pulpdrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work well! I left a couple of ideas, mostly related to performance (cc @knudtty) in case he wants to comment on that as well.

const { data: sources, isLoading: isLoadingSources } = useSources();

// Group filters by (source, metricType, where, whereLanguage) so that we can test each group for MV compatibility separately.
// Faceted filtering: each filter's selectable values are narrowed by the
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One perf-related downside to this is that it makes it less likely that filters can leverage materialized views (since any selected filters being applied to the filter must be a dimension in the materialized view). Fine at reasonable scales, less so at scales that actually require MVs.

Maybe providing the option like we do on the search page is the right move, as Alex (I think?) suggested

Comment on lines +105 to +106
// single key-values query; each selected filter (which omits its own
// expression) splits into its own query.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another potential idea, to reduce the number of queries / performance impact, could be to do what we do for source filters:

SELECT
   groupUniqArrayIf(columnA, conditions on columnB),
   groupUniqArrayIf(columnB, conditions on columnA)
FROM ...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also - there might be some additional code re-use opportunities now between dashboard filters and source filters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-2 Low risk — AI review + quick human skim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants