feat: Persist and read variable-keyed filter state - #2964
Conversation
|
| Name | Type |
|---|---|
| @hyperdx/common-utils | Patch |
| @hyperdx/api | Patch |
| @hyperdx/app | Patch |
| @hyperdx/otel-collector | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
a39697b to
cc522fb
Compare
Greptile SummaryThis PR migrates dashboard filter selections toward variable-name-keyed persistence while preserving expression-keyed compatibility. The disjoint-selection repair remains incomplete because an empty intersection is still serialized as no constraint.
Confidence Score: 4/5The PR is not yet safe to merge because disjoint sibling filter selections still remove the intended constraint from dependent dropdown queries. The attempted intersection fix produces an empty inclusion set for disjoint selections, and the downstream filter serializer drops that state as unconstrained, preserving the previously reported incorrect dropdown behavior. Files Needing Attention: packages/app/src/hooks/useDashboardFilterValues.tsx
|
| Filename | Overview |
|---|---|
| packages/app/src/hooks/useDashboardFilters.tsx | Implements variable-keyed persistence, legacy migration, per-filter selection state, and source-aware query broadcasting without a newly eligible blocking issue. |
| packages/app/src/hooks/useDashboardFilterValues.tsx | Converts faceted filtering to filter-ID selections, but disjoint sibling inclusions still become an omitted constraint. |
| packages/app/src/DBDashboardPage.tsx | Integrates the new filter-state API and expands ignored-filter warnings; the previously reported semantic warning variant is fixed. |
| packages/app/src/utils/queryParsers.ts | Extends URL parsing for the variable-keyed filter-value representation and legacy compatibility. |
| packages/app/tests/e2e/features/dashboard-filter-value-format.spec.ts | Adds end-to-end coverage for variable-keyed persistence and compatibility flows. |
| packages/app/tests/e2e/features/dashboard-table-linking.spec.ts | Extends dashboard-linking coverage; the dynamic URL regex uses a hexadecimal ObjectId and does not create a realistic regex-security path. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Filter selections keyed by filter ID] --> B[Combine sibling selections]
B --> C{Inclusions overlap?}
C -->|Yes| D[Emit intersected predicate]
C -->|No| E[Empty included set]
E --> F[Serializer drops constraint]
F --> G[Dependent dropdown is unfiltered]
Reviews (8): Last reviewed commit: "feat: Persist and read variable-keyed fi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
pulpdrew
changed the title
E2E Test Results✅ All tests passed • 315 passed • 1 skipped • 1155s
Tests ran across 4 shards in parallel. |
cc522fb to
4f7dd76
Compare
4f7dd76 to
5c7ba6a
Compare
🟡 Tier 3 — StandardIntroduces new logic, modifies core functionality, or touches areas with non-trivial risk. Why this tier:
Review process: Full human review — logic, architecture, edge cases. Stats
|
Deep ReviewPart 2/2 of the variable-keyed dashboard filter state migration. The change is well-structured, carries good inline documentation, and ships substantial unit + e2e coverage. No ship-blockers found. Two correctness edge cases around filters that share a SQL expression remain, both narrow and both introduced by this diff's new ✅ No critical issues found. 🟡 P2 -- recommended
Reviewers (3): correctness, testing, previous-comments. Testing gaps:
|
5c7ba6a to
95c5be9
Compare
95c5be9 to
3020823
Compare
3020823 to
cb98bb3
Compare
cb98bb3 to
d1f98a8
Compare
d1f98a8 to
6eb83ac
Compare
| included: | ||
| a.included.size === 0 | ||
| ? b.included | ||
| : b.included.size === 0 | ||
| ? a.included | ||
| : new Set([...a.included].filter(v => b.included.has(v))), |
There was a problem hiding this comment.
When linked sibling filters sharing an expression have disjoint non-empty selections, intersectSelections produces an empty inclusion set that downstream serialization treats as no constraint, causing the dependent dropdown to display unfiltered values instead of no matching values.
Knowledge Base Used:
1 participant
Summary
Part 2/2 in enabling variable-name-based dashboard filter state. This part updates the dashboard filter hooks to read and write state in the new variable-keyed format.
Why
Historically, dashboard filter selection state has been persisted (in the URL, dashboard documents, and exports) as keyed by the filter's expression, in the same format used by search-page filters. This has two problems:
What
This change will persist dashboard filter selection state (that is, what values are selected in the filter's drop-down) based on the variableName of the filter, when it has one.
Screenshots or video
Updated banner showing URL state that doesn't correspond to the declared filters
How to test
Use a URL Decoder if you want to inspect the URL
References