Show visualization brush filters in the filter toolbar#37
Open
Dev-Lan wants to merge 10 commits into
Open
Conversation
Mirror each viz's own brush selection from selectionsStore into dataFiltersStore.internalDataSelections under a `viz-brush-<uuid>` key so brushes appear as removable chips alongside LLM-originated filters. When a user clicks a brush chip's X button, the FilterToolbar clears both stores: selectionsStore (the authoritative brush state that UDIVis reads) by setting selection to null, and dataFiltersStore via clearFilter. Cross-chart filtering continues to flow through the Pinia DataSourcesStore + named-filter entries in each viz's interactiveSpec.transformation — the mirror is display-only. Deferred: a chat-side adjustment widget for brush filters. Each FilterComponent today is tied to a FilterData tool-call message, so surfacing brush filters there needs more design work and is out of scope. Refs #29 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines
+55
to
+56
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| selectionsStore.getState().updateSelections({ [uuid]: { selection: null } as any }); |
Collaborator
There was a problem hiding this comment.
Could we annotate this with as ActiveDataSelection instead? That should help avoid the any.
Collaborator
There was a problem hiding this comment.
actually may be worth it to even just add a updateSelection function to the selectionsStore which just takes a uuid and an ActiveDataSelection so we don't have to do these assertions anywhere
The X button on a viz-brush chip clears selectionsStore[viz.uuid] and the chip-mirror in dataFiltersStore, but Vega manages its own brush rectangle internally and there's no API on UDIVis to ask the chart to drop the rectangle. Detect the own-brush-going-from-present-to-absent transition during render and bump a brushClearedCount that's part of the UDIVis key. The chart then remounts with no brush state, so the visible rectangle disappears together with the chip. Uses React's "store information from previous renders" pattern (conditional setState during render gated by a transition check) so the cascade-renders lint rule doesn't fire. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Brushes now appear in the chat as user-side filter bubbles that share the same FilterComponent UI as LLM-originated filter widgets. The flow: - DashboardCard's brush mirror now writes to dataFiltersStore.dataSelections under `viz-brush-<uuid>` (instead of internalDataSelections), so FilterComponent — which keys off dataSelections — can read brush state from the existing lookup. - FilterToolbar's chip filter now lets `viz-brush-*` entries through the prefix check; previous reliance on internalDataSelections is preserved for any other internal filter sources. - MessageList synthesizes a Message via generateFilterMessage for each viz-brush entry with non-empty selection and renders it through MessageBubble at the end of the conversation. The synthetic messages live only at render time — they aren't appended to conversationStore, so they don't bleed into the LLM prompt. - FilterComponent disables tweakable for brush-derived widgets so the entity/field dropdowns are hidden; rebinding a brush to a different entity would silently detach it from its source viz. - UDIChat adds a mirror effect that round-trips dataSelections[viz-brush-*] back to selectionsStore so chat-side slider drags propagate to cross-chart filtering. updateSelections' JSON-equality check short-circuits no-op writes. - generateFilterMessage is exported from the dashboard feature barrel. Limitation: Vega manages each chart's brush rectangle internally, so a chat-side slider drag updates the data filtering but does not move the visible rectangle on the source viz. Clearing via the FilterToolbar X remounts the source viz (handled in 381628e), but no equivalent path exists for incremental adjustments. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ers-in-toolbar-and-chat # Conflicts: # src/app/UDIChat.tsx # src/features/chat/components/MessageList.tsx # src/features/dashboard/components/DashboardCard.tsx # src/features/dashboard/components/FilterToolbar.tsx
Synthetic brush-filter adjustment widgets were all rendered in a single block after the entire conversation. Map each brush's source-viz UUID to the message index that produced it and render the widget inline right after that visualization, so it keeps its position in the conversation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Vega re-emits a viz's brush selection on every data update, and DashboardCard mirrors each emit into dataFiltersStore via setDataSelection, which unconditionally allocated a new dataSelections object. That reference change re-rendered every dashboard card, prompting Vega to re-emit again — a feedback loop that locked up the page after interacting with a couple of visualizations. Guard setDataSelection so an identical value is a no-op, breaking the loop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The brush->selections mirror skipped any viz-brush entry whose selection was empty, so unchecking every value in a categorical filter widget (or clearing a range) never reached selectionsStore: the stale filter stayed applied and the adjustment appeared to do nothing. Mirror an emptied selection as an explicit null instead, which updateSelections turns into a filter removal (and lets DashboardCard remount to drop the chart's brush). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The stack trace pinned the "Maximum update depth exceeded" crash to IntervalFilterComponent's base-ui Slider: a quantitative brush widget would hard-crash the app (taking down the whole UI via the ErrorBoundary) as soon as it mounted. Cause: localRange was synced from the store via useEffect(() => setLocalRange(storeRange), [storeRange]) where storeRange was a freshly-allocated array memoized on dataSelection.selection. That selection object gets replaced with an equal-valued copy on unrelated store updates, so the effect fired every render and handed the controlled Slider a new value array each time, driving base-ui's thumb effect into an unbounded setState loop. Sync during render instead (React's "info from previous render" pattern, as already used in DashboardCard), keyed on the primitive min/max bounds so equal-valued churn no longer reaches the Slider. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…op crash)
thumbAlignment="edge" puts base-ui's Slider in "inset" mode, which:
- positions the filled Indicator from a DOM-measured indicatorPosition
rather than the values, and that measurement was rendering the range
fill inverted (selected middle empty, outside filled); and
- runs a layout effect (getInsetPosition) that calls setIndicatorPosition
/setPositionPercent off getBoundingClientRect on every value change. When
a viz brush streams rapid value updates into the controlled Slider, that
effect re-fires unboundedly -> "Maximum update depth exceeded", which the
ErrorBoundary turns into a full-app crash.
Removing the prop falls back to the default "center" alignment (inset=false):
the Indicator fill is computed directly from valueToPercent(values) — filling
between the two thumbs — and the inset measurement effect is inert, so there
is no setState loop. Fixes the inverted range fill and the crash.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Temporary visual marker to confirm the running app reflects the latest pushed commits. Revert once verified. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
selectionsStoreintodataFiltersStore.internalDataSelectionsunder aviz-brush-<uuid>key so brushes render as removable chips inFilterToolbaralongside LLM-originated filters.selectionsStore(the authoritative brush state that UDIVis reads, set to null) anddataFiltersStorevia the existingclearFilteraction.DataSourcesStore+ named-filter entries in each viz'sinteractiveSpec.transformation. The mirror is purely for toolbar display.Refs hms-dbmi/udi-yac#24
Partial: chat adjustment widget deferred
The spec also asked for a chat-side adjustment widget for brush filters. Each
FilterComponenttoday is tied to aFilterDatatool-call message (messageFilterKey), so surfacing brush filters there needs a design decision (synthetic message? floating widget? new sibling panel?). I left the issue open (Refs hms-dbmi/udi-yac#24rather thanFixes) so the remaining half can be scoped and shipped in a follow-up — happy to sketch options if useful.Changes
src/features/dashboard/components/DashboardCard.tsx—handleSelectionChangenow additionally callsdataFiltersStore.updateInternalDataSelections({ 'viz-brush-<uuid>': own })with the viz's own brush (empty selection when the brush clears). Comment updated to describe the new display-only mirror.src/features/dashboard/components/FilterToolbar.tsx— add aBRUSH_KEY_PREFIXconstant and ahandleClearChiphelper that clearsselectionsStoreas well when the chip's key starts withviz-brush-, then delegates to the existingclearFilter.Spec
.claude/todo/done/show-viz-filters-in-toolbar-and-chat.mdTest plan
FilterDatatool call): it continues to appear in the toolbar as before, no regression.npm run testpasses (145 tests).Notes
main(17 errors in dashboard/data-package stores and tests) are unrelated; no new typecheck errors introduced.as anycast inFilterToolbar.handleClearChipis narrowly scoped and documented — it exists becauseselectionsStore.updateSelectionsuses udi-toolkit's stricterDataSelectionsshape, while the clear path only needs to write{ selection: null }to trigger a remove.🤖 Generated with Claude Code