Skip to content

feat: add filter/exclude/copy actions to Event Deltas#1797

Draft
alex-fedotyev wants to merge 44 commits intomainfrom
claude/issue-1796-20260225-0116
Draft

feat: add filter/exclude/copy actions to Event Deltas#1797
alex-fedotyev wants to merge 44 commits intomainfrom
claude/issue-1796-20260225-0116

Conversation

@alex-fedotyev
Copy link
Contributor

@alex-fedotyev alex-fedotyev commented Feb 25, 2026

⚠️ This PR is being split into smaller PRs

This PR grew too large during iterative development (2,900+ lines across 8 files, 40+ commits). It has been split into 7 focused issues that can be implemented and reviewed independently:

Issues (in recommended merge order)

  1. Event Deltas: Chart UX polish and heatmap fixes #1823 — Chart UX polish & heatmap fixes (no deps)
  2. Event Deltas: Field filtering and priority classification #1825 — Field filtering & prioritization (no deps)
  3. Event Deltas: Deterministic sampling with adaptive sample size #1827 — Sampling improvements (no deps)
  4. Event Deltas: Always-on attribute distribution mode (no selection required) #1824 — Always-on distribution mode (depends on Event Deltas: Chart UX polish and heatmap fixes #1823)
  5. Event Deltas: Improved attribute sorting with entropy scoring and proportional comparison #1826 — Attribute sorting & scoring (depends on Event Deltas: Field filtering and priority classification #1825)
  6. Event Deltas: Heatmap correlation highlights on attribute value hover #1828 — Hover correlation highlights (depends on Event Deltas: Chart UX polish and heatmap fixes #1823, Event Deltas: Always-on attribute distribution mode (no selection required) #1824)
  7. Event Deltas: Filter/Exclude/Copy actions on attribute values #1829 — Filter/exclude/copy actions (depends on Event Deltas: Chart UX polish and heatmap fixes #1823, Event Deltas: Always-on attribute distribution mode (no selection required) #1824)

Reference implementation

This PR branch (claude/issue-1796-20260225-0116) contains the complete working implementation with 106 unit tests covering all pure utility functions. The code in this branch serves as the reference for creating the individual PRs above.

What this PR contains (for reference)

  • deltaChartUtils.ts (636 lines) — all pure utility functions, types, and constants
  • PropertyComparisonChart.tsx (401 lines) — extracted bar chart component
  • DBDeltaChart.tsx (709 lines) — main orchestration component
  • DBHeatmapChart.tsx (+279 lines) — heatmap correlation highlights, tooltip fixes, selection UX
  • DBSearchHeatmapChart.tsx (+88 lines) — container wiring
  • DBDeltaChart.test.ts (1,090 lines) — 106 unit tests
  • app.scss (+7 lines) — selection color override
  • DBSearchPage.tsx (+1 line) — onAddFilter prop

Do not merge this PR directly. Use the individual issues above for focused, reviewable changes.


Original description: Implements #1796 — Improve Traces → Event Deltas by introducing filtering actions for continuous drill down.

Add interactive filter actions to the Event Deltas chart tooltips,
consistent with the filtering UX in the results table and trace
drilldown flyout. Users can now hover over a bar in the Event Deltas
charts and click Copy, Filter, or Exclude buttons to drill down into
the subset of spans defined by the hovered attribute value.

- Enhance HDXBarChartTooltip with copy/filter/exclude action buttons
- Use controlled tooltip state to keep tooltip interactive on hover
- Export AddFilterFn type for reuse across the component chain
- Wire onAddFilter from DBSearchPage -> DBSearchHeatmapChart -> DBDeltaChart

Closes #1796

Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
@changeset-bot
Copy link

changeset-bot bot commented Feb 25, 2026

⚠️ No Changeset found

Latest commit: 923f799

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

@vercel
Copy link

vercel bot commented Feb 25, 2026

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

Project Deployment Actions Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview, Comment Feb 27, 2026 0:14am

Request Review

@github-actions
Copy link
Contributor

github-actions bot commented Feb 25, 2026

E2E Test Results

All tests passed • 75 passed • 4 skipped • 843s

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

Tests ran across 4 shards in parallel.

View full report →

@alex-fedotyev
Copy link
Contributor Author

@claude - when I tried applying filters, it seems to be erroring out on some of them, likely related to the query generation for resource attributes or span attributes (maps). Investigate what might be causing it, and whether in different parts of the trace filter logic those filters are implemented correctly, so this code would reuse it.
I think when I try to filter by resource attributes within the Trace view flyout, the resource attribute filter is applied as SQL query and not the filter in the left panel.
image

Error when filtering using current PR:
First argument for function tupleElement must be Tuple, QBit, JSON or array of Tuple, QBit, JSON. Actual Map(String, String): In scope WITH PartIds AS (SELECT tuple(_part, _part_offset) FROM otel_v2.otel_traces WHERE ((Timestamp >= fromUnixTimestamp64Milli(_CAST(1771944864000, 'Int64'))) AND (Timestamp <= fromUnixTimestamp64Milli(_CAST(1771988064000, 'Int64')))) AND ((tupleElement(tupleElement(Events.Attributes[5], 'message'), 'type') IN ('SENT')) AND ((Timestamp >= 1771948662.1446402) AND (Timestamp <= 1771964479.396244) AND ((Duration / 1000000.) >= 103076.92307692308) AND ((Duration / 1000000.) <= 613776.2237762237))) ORDER BY rand() DESC LIMIT _CAST(1000, 'Int32')) SELECT * FROM otel_v2.otel_traces WHERE ((Timestamp >= fromUnixTimestamp64Milli(_CAST(1771944864000, 'Int64'))) AND (Timestamp <= fromUnixTimestamp64Milli(_CAST(1771988064000, 'Int64')))) AND ((tupleElement(tupleElement(Events.Attributes[5], 'message'), 'type') IN ('SENT')) AND (((Duration / 1000000.) >= 103076.92307692308) AND ((Duration / 1000000.) <= 613776.2237762237) AND (Timestamp >= 1771948662.1446402) AND (Timestamp <= 1771964479.396244)) AND indexHint((_part, _part_offset) IN (PartIds))) ORDER BY rand() DESC LIMIT _CAST(1000, 'Int32').

@claude
Copy link

claude bot commented Feb 25, 2026

Claude finished @alex-fedotyev's task in 14m 48s —— View job


Fixed filter generation for Map-type attributes in Event Deltas

  • Gather context and understand the request
  • Read DBDeltaChart.tsx to understand current filter implementation
  • Explore how trace/span attribute filters are handled elsewhere
  • Identify root cause of the tupleElement error on Map(String, String)
  • Fix the filter generation logic to correctly handle map-type attributes
  • Push changes

Root Cause

flattenData() produces dot-notation keys (e.g. ResourceAttributes.service.name) for Map-type columns. When passed directly to setFilterValue, filtersToQuery generates SQL like ResourceAttributes.service.name IN ('value'). ClickHouse interprets the dot notation as tupleElement(ResourceAttributes, 'service', 'name') and errors since ResourceAttributes is Map(String, String), not a Tuple.

The same issue affects Events.Attributes[N].key (Array of Maps), plus those had an additional 0-based (JS) vs 1-based (ClickHouse) array index discrepancy.

Fix — packages/app/src/components/DBDeltaChart.tsx

Added flattenedKeyToSqlExpression(key, columnMeta) which uses the meta array from query results ({ name, type }[]) to detect Map and Array(Map) columns and convert flattened keys to valid ClickHouse SQL:

Flattened key (before) SQL expression (after)
ResourceAttributes.service.name ResourceAttributes['service.name']
SpanAttributes.http.method SpanAttributes['http.method']
Events.Attributes[0].message.type Events.Attributes[1]['message.type']

The handleAddFilter callback in DBDeltaChart wraps onAddFilter to apply this conversion. Handles LowCardinality(Map(...)) and Nullable(Map(...)) type wrappers too.

This matches how the rest of the codebase handles map attribute filters — DBRowOverviewPanel explicitly constructs ${resourceAttributesExpression}['${key}'] bracket notation.

Unit tests added in packages/app/src/components/__tests__/DBDeltaChart.test.ts covering all conversion cases.
| Branch

… in Event Deltas filters

flattenData() produces dot-notation keys like ResourceAttributes.service.name
for Map-type columns, but ClickHouse SQL requires bracket notation for Map
access: ResourceAttributes['service.name']. The dot notation causes ClickHouse
to attempt tupleElement() on a Map type, which fails.

Adds flattenedKeyToSqlExpression() that uses the query result meta (column
name + type) to detect Map and Array(Map) columns and convert keys to valid
SQL expressions. Also fixes the 0-based (JS) to 1-based (ClickHouse) array
index for Array(Map) columns like Events.Attributes.

This matches how other parts of the codebase handle map attribute filters:
DBRowOverviewPanel uses ResourceAttributesExpression['key'] bracket notation.

Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
@alex-fedotyev
Copy link
Contributor Author

@claude - implement following improvement to help navigating the list of attributes based on how useful those values are for the analysis. Make sure to reuse existing components, code and UI as much as possible, avoid re-archtecting. Bring feedback if something isn't possible to implement without major changes.

Implement Field Filtering for Event Deltas
Context
In the Traces → Event Deltas view (packages/app/src/components/DBDeltaChart.tsx), all flattened fields are displayed as small bar charts. This creates noise — fields like TraceId, SpanId, and Events.Timestamp[0-23] are unique per span and provide no analytical value. We need to hide noisy fields by default and show them in a collapsed section.
Requirements

  1. Structural Denylist (always hidden by default)
    Hide fields matching these patterns. Use the columnMeta (already available from the useMemo that reads outlierData?.meta ?? inlierData?.meta) to detect types rather than hardcoding names where possible:

ID fields: TraceId, SpanId, ParentSpanId, Links.TraceId, Links.SpanId — match top-level String columns ending in Id or ID
Nested timestamp arrays: any field matching Events.Timestamp[N] or Links.Timestamp[N] pattern — detect from columns where the base column type is Array(DateTime64(...))

  1. Cardinality Filter (hidden by default when ratio > 0.9)
    For fields NOT caught by the denylist:

Compute uniqueness ratio: uniqueValues / totalRows using the existing outlierValueOccurences and inlierValueOccurences Maps
Compute the ratio for inliers and outliers separately, take the minimum of the two (if either group clusters, the field is worth showing)
If min(inlierUniqueness, outlierUniqueness) > 0.9 AND combined sample size > 20 → hide by default
Fields with 0 occurrences in both groups should not be shown at all

  1. UI: Collapsed Section (Option A)
    After all visible delta charts (and after pagination), render a collapsed section showing hidden fields.
    Collapsed state:
    ▶ N hidden fields (high cardinality)
    TraceId · SpanId · Events.Timestamp[0-23] · ... ← preview first ~5 names, truncate with "..."
    Expanded state (click to toggle):
    ▼ N hidden fields (high cardinality)

TraceId — 1000 unique values [Show chart]
SpanId — 1000 unique values [Show chart]
Events.Timestamp[0] — 998 unique values [Show chart]
http.url — 912 unique values [Show chart]
...

Clicking "Show chart" on an individual row expands the existing HDXBarChart inline for that field
The collapsed/expanded state should be managed with useState, no persistence needed
Style consistently with the existing UI — use the same font sizes, colors, and spacing as the delta chart titles. Use muted/secondary text color for the hidden section.

  1. Update pagination
    The existing pagination (activePage, setPage) should only count visible fields. Add a note near pagination: "Showing X of Y fields" so users understand why there are fewer pages.
    Implementation Notes

All filtering logic should happen client-side in the existing component. No backend/API changes needed.
The split between "visible" and "hidden" fields should be computed in a useMemo that depends on the flattened property list, outlierValueOccurences, inlierValueOccurences, and columnMeta.
Export the denylist patterns and the isHighCardinality helper so they can be unit tested.
Add unit tests for: denylist matching, cardinality ratio computation, and the split logic (which fields end up visible vs hidden).

File Changes Expected

packages/app/src/components/DBDeltaChart.tsx — filtering logic + UI
packages/app/src/components/tests/DBDeltaChart.test.ts — add tests for denylist and cardinality filter

@claude
Copy link

claude bot commented Feb 25, 2026

Claude finished @alex-fedotyev's task in 15m 14s —— View job


Implemented Field Filtering for Event Deltas

  • Gather context and read DBDeltaChart.tsx
  • Understand current data structures and patterns
  • Implement structural denylist logic (isIdField, isTimestampArrayField, isDenylisted)
  • Implement cardinality filter logic (isHighCardinality)
  • Modify getPropertyStatistics to expose property occurrence counts
  • Merge useMemos and compute visible/hidden field split
  • Build collapsed/expanded hidden fields UI section (HiddenFieldsSection)
  • Update pagination to count only visible fields + "Showing X of Y" note
  • Add unit tests for all new helpers
  • Push changes

What was implemented

packages/app/src/components/DBDeltaChart.tsx

1. Structural Denylist — always hidden regardless of cardinality:

Helper What it hides
isIdField Top-level String columns ending in Id/ID (TraceId, SpanId, ParentSpanId), and Array(String) column elements with the same suffix (Links.TraceId[0], Links.SpanId[0])
isTimestampArrayField Per-index elements and plain references of Array(DateTime64(...)) columns (Events.Timestamp[N], Links.Timestamp[N])
isDenylisted Union of the above two

Detection is type-based using the columnMeta already available from query results — no hardcoded name lists.

2. Cardinality Filter — hidden when values are too unique to be useful:

isHighCardinality(key, outlierValues, inlierValues, outlierOccurrences, inlierOccurrences):

  • Computes uniqueness = uniqueValues / totalOccurrences for each group
  • Uses min(outlierUniqueness, inlierUniqueness) — if either group clusters, the field is kept visible
  • Hides the field if min > 0.9 AND combinedSampleSize > 20
  • If only one group has data, uses that group's uniqueness alone

getPropertyStatistics was updated to also return propertyOccurences (raw counts per property) for use in the ratio calculation.

3. HiddenFieldsSection component:

  • Collapsed by default: ▶ N hidden fields (high cardinality) + italic preview of first 5 field names
  • Expanded: lists each hidden field with unique value count and a Show chart link that inlines the full PropertyComparisonChart
  • State managed with useState (no persistence)
  • Styled consistently using Text size="xs" c="dimmed" and monospace font

4. Pagination update:

  • totalPages now counts only visibleProperties (not all sorted properties)
  • A "Showing X of Y fields" note appears in the pagination row whenever fields are hidden

packages/app/src/components/__tests__/DBDeltaChart.test.ts

New test suites added:

  • isIdField — 8 test cases covering String columns, Array(String) elements, non-ID columns, Map columns, sub-key paths
  • isTimestampArrayField — 7 test cases covering indexed and plain references, non-timestamp arrays, timezone variants
  • isDenylisted — union tests
  • isHighCardinality — 6 test cases: all-unique, low cardinality, min-group logic, both-high-cardinality, small-sample boundary, single-group
  • field split logic — integration test showing correct classification of a mixed field set
    | Branch

Implements field filtering to reduce noise in the Event Deltas chart view:

1. Structural denylist: ID fields (TraceId, SpanId, ParentSpanId, Links.TraceId[N],
   Links.SpanId[N]) and per-index timestamp arrays (Events.Timestamp[N],
   Links.Timestamp[N]) are hidden by default. Detection is type-based using
   columnMeta (String / Array(String) columns ending in Id/ID; Array(DateTime64)
   columns for timestamps).

2. Cardinality filter: Fields where min(outlierUniqueness, inlierUniqueness) > 0.9
   AND combined sample size > 20 are hidden. Uses uniqueValues/propertyOccurences
   ratio derived from getPropertyStatistics output.

3. HiddenFieldsSection UI: Collapsed by default showing count + preview of first 5
   names. Expanded view lists each hidden field with its unique value count and a
   "Show chart" toggle to inline the full PropertyComparisonChart.

4. Pagination update: Counts only visible fields. A "Showing X of Y fields" note
   appears near pagination when fields are hidden.

Exports isIdField, isTimestampArrayField, isDenylisted, isHighCardinality for unit
testing. Unit tests added covering all helpers and the split logic.

Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
@alex-fedotyev
Copy link
Contributor Author

@claude - denylist and cardinality filters seem to work well mostly. Change the UX to appreciate the existing pagination, as current implementation didn't follow by adding views at the bottom of it. I suggest to add the denylist to the last page within existing pagination. Charts need to appear exactly same as other charts. Question is whether section needs to have separator - I like it. Should it be collapsed by default? If user expands it, will it change the number of pages depending on how many new attributes need to be displayed and how they fit on the page? Or just keep separator as a visual and logical splitter with lower importance attributes listed at the bottom?

@claude
Copy link

claude bot commented Feb 25, 2026

Claude finished @alex-fedotyev's task in 5m 43s —— View job


Redesigned lower-priority fields UX

  • Read current implementation and understand pagination/layout
  • Implement: move lower-priority fields into last page (before pagination)
  • Remove HiddenFieldsSection component (replaced with inline same-style charts)
  • Add Divider separator between primary and lower-priority fields
  • Simplify footer — remove "Showing X of Y" text; pagination row only visible when totalPages > 1
  • Push changes

What changed

Design decisions:

  • No collapse — lower-priority fields are always visible on the last page as static content, no toggle needed
  • Page count unchanged — pagination still counts only primary (visible) fields; lower-priority fields appear as bonus content at the bottom of the last page
  • Same charts — uses the exact same PropertyComparisonChart component and same CSS grid layout (same columns count) as primary fields
  • Separator — a Mantine Divider with label "Lower-priority fields (N)" provides the visual/logical split

Layout on last page:

[ chart ] [ chart ] [ chart ]   ← primary fields (paginated)
[ chart ] [ chart ]

───── Lower-priority fields (12) ─────

[ chart ] [ chart ] [ chart ]   ← same chart component, same grid
[ chart ] [ chart ] [ chart ]
...
                        [ < 1 2 > ]  ← pagination pinned to bottom

Code cleanup: Removed HiddenProperty type, HiddenFieldsSection component (~110 lines removed), simplified hiddenProperties to string[].
| Branch

…ination

Replace the HiddenFieldsSection component (which rendered below pagination)
with an inline section that appears on the last page. Lower-priority fields
use the same PropertyComparisonChart component and same CSS grid layout as
primary fields. A Divider with label 'Lower-priority fields (N)' provides a
clear visual and logical separator. No expand/collapse -- fields are always
visible on the last page as static content.

Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
@alex-fedotyev alex-fedotyev changed the title feat: add filter/exclude/copy actions to Event Deltas tooltip feat: add filter/exclude/copy actions to Event Deltas Feb 25, 2026
@alex-fedotyev alex-fedotyev self-assigned this Feb 25, 2026
@alex-fedotyev alex-fedotyev marked this pull request as draft February 25, 2026 18:55
@alex-fedotyev
Copy link
Contributor Author

@claude : Improve Event Deltas Chart Tooltip and Legend UX
Context
In the Traces → Event Deltas view (packages/app/src/components/DBDeltaChart.tsx), each field renders a small bar chart (HDXBarChart) showing value distributions for inliers vs outliers. PR #1797 added filter/exclude/copy action buttons to the chart tooltip (HDXBarChartTooltip).
Current problems:

The tooltip follows the mouse cursor, so action buttons move as the user tries to click them
X-axis labels (value names) overflow or collide when values are long or numerous
With 7+ values, the chart becomes unreadable

Requirements

Split Hover and Click into Separate Interactions
Problem: The tooltip currently shows both informational content (value name, count) and action buttons (filter/exclude/copy) on hover. Because it follows the cursor, the action buttons are impossible to click reliably.
Fix: Separate reading from acting:
Hover (info-only tooltip):
Shows on mouse enter for a bar, follows cursor (existing Recharts default behavior)
Content: full untruncated value name, count, percentage of group (e.g., "fraud-detection — 734 spans (73%)")
No action buttons
Disappears on mouse leave (standard tooltip behavior, no delay needed)
This is lightweight — reuse the existing Recharts with a simplified custom content renderer

Click (action popover):

Shows when the user clicks a bar
Anchored to the clicked bar's position (does NOT follow cursor)
Content: full value name + the action buttons: Filter → Exclude → Copy (in this order, left to right)
Stays visible until the user clicks outside the popover or clicks a different bar
Position the popover above or to the right of the clicked bar. Use the bar's coordinates from the Recharts click event to calculate placement. If the popover would overflow the chart container, flip to the opposite side.
Clicking an action (Filter/Exclude) should execute the action AND dismiss the popover
Clicking Copy should execute the copy and show brief feedback ("Copied!") but keep the popover open

Implementation approach:

Add onClick handler to the component (Recharts supports this via the onClick prop on )
Track clickedBarData in state: useState<{ value: string; property: string; x: number; y: number } | null>(null)
Render the action popover as an absolutely-positioned div (or portal) using the stored coordinates when clickedBarData is non-null
Add a click-outside handler (e.g., useEffect with document click listener) to dismiss
Keep the existing for hover but strip out the action buttons from its content
The existing handleAddFilter callback from PR #1797 stays the same — it's just triggered from the click popover instead of the hover tooltip

Fix Axis Labels for Long Values and Many Values
Problem: Value names are shown as X-axis labels under the bars. Long text values overflow due to limited space in the small-multiple layout. With many values, labels collide.
Fix — custom tick renderer with truncation:
Replace the default tick with a custom tick renderer:
The TruncatedTick component receives standard Recharts tick props (x, y, payload.value) and renders:

Truncated label: max 12 characters, append "…" for longer values

<title> SVG element inside the so the full value appears as a native browser tooltip on hover (this complements the Recharts hover tooltip — belt and suspenders) Use the same font size and color as current axis labels This is a small, self-contained functional component — ~15 lines of code

For 7+ values — show top N with "other" bucket:

When a field has more than 6 distinct values, render bars for only the top 6 values by combined frequency (inlier + outlier counts)
Aggregate remaining values into a single "Other (N)" bar using a neutral gray color
Add this aggregation in the data preparation step where outlierValueOccurences and inlierValueOccurences maps are processed, before passing to the chart component
Disable click actions (filter/exclude/copy) on the "Other" bar — the click handler should check for the "Other" sentinel value and no-op. The hover tooltip should still show "Other (N values)" with the combined count.

Order Action Buttons by Importance
In the new click popover (not the hover tooltip), order the buttons left-to-right as: Filter → Exclude → Copy.
Filter: primary visual weight (solid/filled button or primary color) — this is the most common action
Exclude: secondary weight (outlined or ghost variant)
Copy: most subtle (icon-only with a small copy icon, no text label, with native tooltip "Copy value")

Match button styling to the patterns already used in the results table and trace flyout for consistency. Look at how those components render their filter/exclude/copy actions and reuse the same button components/styles.
Undo PR #1797 Hover Tooltip Actions
PR #1797 added filter/exclude/copy action buttons to the hover tooltip (HDXBarChartTooltip) and introduced controlled tooltip state with hover delays to keep the tooltip interactive while the user moves toward it. Revert these changes. Specifically:

Remove the action buttons from HDXBarChartTooltip — it should only render info (value name, count, percentage)
Remove the controlled tooltip open/close state and hover delay logic — let Recharts manage tooltip visibility with its default behavior
Keep the onAddFilter prop wiring and the handleAddFilter / flattenedKeyToSqlExpression fix from PR #1797 — those are still needed, they'll just be called from the click popover instead of the hover tooltip
Keep the exported AddFilterFn type

The click popover (Section 1 above) replaces the hover-based actions with a cleaner interaction model.
Implementation Notes

Minimize changes to shared components. If HDXBarChart is used outside Event Deltas, add the click behavior via optional props (e.g., onBarClick?: (data) => void) rather than changing default behavior. The click popover state and rendering should live in DBDeltaChart, not in the shared chart component.
Reuse existing Recharts patterns. for click events, for hover, for custom ticks. No new dependencies.
Dismiss popover on scroll. Since these are small multiples that the user scrolls through, add a scroll listener that dismisses the click popover. Otherwise a pinned popover from a chart that scrolled offscreen would look broken.
All changes should be in existing component files. No backend changes.

File Changes Expected

packages/app/src/components/DBDeltaChart.tsx — click popover state/rendering, data aggregation for top-N, TruncatedTick component
HDXBarChartTooltip component — simplify to info-only (remove action buttons from hover tooltip)
HDXBarChart component — add onBarClick prop passthrough if not already present

@claude
Copy link

claude bot commented Feb 25, 2026

Claude finished @alex-fedotyev's task in 9m 10s —— View job


Improved Event Deltas Chart Tooltip and Legend UX

  • Gather context and read relevant files
  • Understand current implementation
  • Simplify HDXBarChartTooltip to info-only (remove action buttons + controlled hover state)
  • Add TruncatedTick and applyTopNAggregation helpers
  • Add click popover with Filter → Exclude → Copy buttons
  • Update recharts import to include Cell for per-bar coloring
  • Update unit tests for applyTopNAggregation
  • Push changes

What changed (packages/app/src/components/DBDeltaChart.tsx)

1. Info-only hover tooltip
Stripped action buttons, onAddFilter/onMouseEnter/onMouseLeave props, and isCopied state from HDXBarChartTooltip. Removed the controlled tooltipState / tooltipHideTimeoutRef / keepTooltipOpen / scheduleTooltipClose mechanism entirely — Recharts now manages tooltip visibility with default behavior.

2. Click popover (anchored, not following cursor)
Added clickedBar state (value, clientX, clientY) to PropertyComparisonChart. Clicking a bar sets this state and renders a position: fixed popover anchored to the cursor's position at click time:

  • FilterActionIcon variant="primary" (filled, highest visual weight) — executes filter and dismisses popover
  • ExcludeActionIcon variant="secondary" — executes exclude and dismisses popover
  • CopyActionIcon variant="secondary" icon-only — copies value, shows "Copied!" feedback, keeps popover open
  • Dismisses on click-outside (via mousedown listener that excludes both popover and chart wrapper) and on scroll (capture-phase listener)

3. TruncatedTick custom axis tick
Replaces default Recharts XAxis tick. Truncates labels to 12 characters with , wraps the SVG <text> in a <g> with a <title> element for native browser tooltip on hover.

4. Top-6 + "Other (N)" aggregation
applyTopNAggregation() sorts values by combined frequency (outlier + inlier count) descending, keeps top 6, and collapses the rest into a single "Other (N)" entry rendered in neutral gray (#868e96). Per-bar colors use <Cell> components. Click handler no-ops when payload.isOther is true.

5. Tests
7 new test cases for applyTopNAggregation in DBDeltaChart.test.ts: pass-through when ≤ MAX_CHART_VALUES, correct bucket name/count, correct top-N selection, correct accumulation of outlier/inlier counts into the Other bucket.
| Branch

- Split hover/click interactions: hover tooltip is now info-only (value
  name + percentages), click opens an anchored action popover
- Click popover (position:fixed, anchored to bar click coordinates) shows
  Filter -> Exclude -> Copy buttons; Filter/Exclude dismiss popover on
  action, Copy keeps it open with Copied! feedback
- Popover dismisses on click-outside and scroll
- Added TruncatedTick custom XAxis tick: truncates to 12 chars + ...,
  includes native SVG title for full value on hover
- Top-6 + Other(N) aggregation: fields with >6 distinct values show
  top 6 by combined frequency and collapse the rest into a neutral gray
  Other(N) bucket; click actions disabled on Other bar
- Button order: Filter (primary variant) -> Exclude (secondary) -> Copy
  (secondary, icon-only) - consistent with codebase action patterns
- Removed controlled tooltip hover-delay state
- Exported applyTopNAggregation and MAX_CHART_VALUES for unit testing
- Added unit tests for applyTopNAggregation (7 test cases)

Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
@alex-fedotyev
Copy link
Contributor Author

@claude - fix bugs:

  1. Popup is rendered under the chart from the component below the current mouse over on.
image
  1. Filter box is not rendered properly using the light theme.
image
  1. Title is missing on the click action, needs to have details about the value and maybe some counts and distribution as well?
image
  1. After applying filtering, the top chart loses the selection although the comparison is still enabled (which is good). Need to add persistence of the selection on the chart, which likely needs to be recalculated on vertical axes since duration values would likely change when filters are applied. Looking for consistent experience, while applying filters is the actionable step to drill down attribute by attribute.
image

@claude
Copy link

claude bot commented Feb 25, 2026

Claude finished @alex-fedotyev's task in 9m 20s —— View job


Fixed 4 popup and chart bugs

  • Gather context and read DBDeltaChart.tsx
  • Fix Bug 1: Popup z-index — render popover via createPortal into document.body (z-index: 9999)
  • Fix Bug 2: Popover light theme — apply styles.chartTooltip CSS class which uses --color-bg-surface / --color-border theme-aware variables
  • Fix Bug 3: Add property name header + outlier/inlier counts with percentages to click popover
  • Fix Bug 4: Persist histogram selection — pass initialSelection from URL params through DBHeatmapChartHeatmap; capture uPlot instance via init plugin hook and restore selection overlay with correct pixel coordinates after remount
  • Push changes

What changed

DBDeltaChart.tsx

  • Bug 1 (z-index): The popover was rendered inside the chart's DOM subtree, causing adjacent chart grid siblings (later in DOM order) to paint over it. Fixed by wrapping the popover in createPortal(…, document.body) with z-index: 9999.

  • Bug 2 (light theme): The popover had hardcoded var(--mantine-color-dark-7) background and var(--mantine-color-dark-4) border — both invisible on light backgrounds. Replaced with className={styles.chartTooltip} (the same class used by the hover tooltip), which uses --color-bg-surface and --color-border and adapts to the active theme.

  • Bug 3 (missing title/context): The popover now shows three sections:

    • Property name (dimmed, bold header) — e.g., service.name
    • Clicked value — full text with word-break
    • Counts rowOutliers: N (X%) in error color + Inliers: N (X%) in success color, computed by looking up the value in the outlierValueOccurences / inlierValueOccurences maps

DBSearchHeatmapChart.tsx

  • Bug 4: Passes initialSelection to DBHeatmapChart from the already-persisted URL params (fields.xMin/xMax/yMin/yMax). Previously these values were only used to decide whether to show DBDeltaChart — now they're also fed back into the heatmap to restore the selection overlay.

DBHeatmapChart.tsx

  • Bug 4: HeatmapContainer accepts and passes initialSelection to Heatmap.
  • Bug 4: In Heatmap:
    • uplotRef (ref) + uplotInitCount (state counter) are added. A plugin init hook sets the ref and increments the counter on each uPlot initialization.
    • A useEffect([uplotInitCount]) runs after each uPlot init: converts data-unit values (xMin/xMax in seconds, yMin/yMax in value units) to pixel coordinates using u.valToPos() + u.over.offsetLeft/Top, then calls setSelectingInfo to restore the visual selection rectangle.
    • The Y-axis inversion is handled correctly: top = valToPos(yMax), height = valToPos(yMin) - valToPos(yMax).
      | Branch

… persistence bugs

- Bug 1 (z-index): Render click popover via createPortal into document.body so it
  always appears above all chart siblings in the grid layout
- Bug 2 (light theme): Replace hardcoded dark-mode CSS variables with theme-aware
  chartTooltip CSS class (uses --color-bg-surface / --color-border)
- Bug 3 (missing title): Add property name header and outlier/inlier counts with
  percentages to the click popover content
- Bug 4 (selection persistence): Pass initialSelection prop from DBSearchHeatmapChart
  through DBHeatmapChart (HeatmapContainer) to Heatmap; capture uPlot instance via
  plugin init hook and restore the selection overlay with correct pixel coordinates
  after the component remounts on filter change

Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
@github-actions
Copy link
Contributor

PR Review

  • 🔒 SQL injection via unescaped single quotes in flattenedKeyToSqlExpression -> Map attribute keys are interpolated directly into SQL without escaping. If a key contains a single quote (e.g. "it's.key"), this produces malformed/injectable SQL like MapCol['it's.key']. Fix: escape single quotes in mapKey before interpolating (mapKey.replace(/'/g, "''")).

  • ⚠️ Percentage math in popover is misleading -> totalOutliers sums percentage values from percentageOccurences (each entry is already a percentage ~0-100), then computes (outlierCount / totalOutliers) * 100. The denominator is the sum of all percentages (~100), so the result approximates the original value, making the parenthetical in "Outliers: 23.5 (23.5%)" redundant and the math semantically wrong. Either display the raw percentage directly or compute from actual occurrence counts via propertyOccurences.

  • ⚠️ File size violation -> DBDeltaChart.tsx is now ~1168 lines, well over the 300-line guideline in CLAUDE.md. PropertyComparisonChart and the exported helpers (applyTopNAggregation, flattenedKeyToSqlExpression, field filtering functions) should be extracted into separate files.

  • Good test coverage for all exported helpers.

  • createPortal for the click popover z-index is the right approach.

  • Selection persistence via uplotInitCount trigger is clean.

@alex-fedotyev
Copy link
Contributor Author

@claude - fix couple following issues:

  1. popup for the actions like "filter" and "exclude" hints are shows below the visible are (wrong z-index). And their style doesn't seem to follow the light/dark design themes.
image
  1. Chart selection doesn't persist after applying a filter, chart is rendered as new without any selection although "filter by selection" button is visible.
image
  1. I suggest that low priority fields need to be account in the pages (pagination controls), otherwise they break the experience and make last page very long, pagination control render below the fold in my example. Make sure to account for use cases when N of chart is uneven and then the horizontal separator makes it appear like a new row has started... need to account for the fact that new row is started for those charts, and make sure that separator height isn't screwing up the vertical alignment of the screen.
image

@claude
Copy link

claude bot commented Feb 25, 2026

Claude finished @alex-fedotyev's task in 7m 46s —— View job


Fixed popup tooltip z-index, chart selection persistence, and lower-priority field pagination

  • Gather context and read relevant files
  • Fix Bug 1: Add zIndex={10000} to MantineTooltips inside click popover
  • Fix Bug 2: Chart selection persistence — add dataLength dependency to restore effect
  • Fix Bug 3: Include lower-priority fields in pagination with two-grid layout
  • Push changes

What changed

Bug 1 — Popup tooltip hints rendering below/behind popover (DBDeltaChart.tsx)

Mantine Tooltip components render into a separate Mantine portal that had no explicit z-index. Since the click popover itself is rendered via createPortal(…, document.body) at z-index: 9999, the Mantine Tooltip portal rendered at a lower effective stacking context. Added zIndex={10000} to all three MantineTooltip wrappers (Filter, Exclude, Copy) inside the click popover so they always render above it.

Bug 2 — Chart selection not restored after applying filter (DBHeatmapChart.tsx)

Root cause: the uPlot init hook fires immediately when the component mounts, before the heatmap data has loaded. At that point, the x/y scales aren't set, so u.valToPos(…) returns NaN/Infinity and the restore effect exits early. When data loads later, uplotInitCount doesn't increment again (the component doesn't remount), so the restore never runs.

Fix: added dataLength = data[0].length as a second dependency to the restore useEffect. The effect now re-runs when real data arrives (dataLength ≥ 2). By that point UplotReact has called u.setData() with the loaded data, scales are valid, and valToPos returns correct pixel coordinates.

Bug 3 — Lower-priority fields causing unbounded last page (DBDeltaChart.tsx)

Lower-priority fields are now part of the standard pagination:

  • totalProperties = visibleProperties.length + hiddenProperties.length (was: only visible)
  • Each page computes visibleOnPage and hiddenOnPage from their respective slices of the combined range
  • Two separate grids: primary fields grid and lower-priority fields grid — this ensures the CSS grid's auto-placement doesn't interleave them, so empty trailing cells in the primary grid row don't affect the divider's vertical position
  • A divider labeled "Lower-priority fields (N)" appears when both sections are on the same page, or when a page contains only lower-priority fields (pure hidden-fields pages)
    | Branch

@alex-fedotyev
Copy link
Contributor Author

@claude - lint still failing even after latest fixes, can you improve the rest of the items?

@claude
Copy link

claude bot commented Feb 26, 2026

Claude finished @alex-fedotyev's task in 4m 8s —— View job


Fixed remaining lint errors

  • Gather context and check CI status
  • Identify remaining lint errors (3 errors in 2 files)
  • Fix PropertyComparisonChart.tsx import sort order
  • Fix DBDeltaChart.test.ts prettier formatting at lines 213 and 1015
  • Push changes

3 errors fixed:

  1. PropertyComparisonChart.tsx:1:1simple-import-sort/imports — moved @mantine/core and @tabler/icons-react imports before react/react-dom/react-error-boundary/recharts (ESLint rule groups all external packages together, sorted alphabetically; @ < r in ASCII)

  2. DBDeltaChart.test.ts:213 — prettier — changed expect(\n isIdField(...),\n).toBe(false) to expect(isIdField(...)).toBe(\n false,\n) (prettier prefers inlining the arg into expect() when it fits within 80 chars, then wrapping the .toBe() value)

  3. DBDeltaChart.test.ts:1015 — prettier — same pattern: expect(computeYValue(...)).toBeCloseTo(\n 5,\n)
    |

…Chart and DBDeltaChart tests

Co-authored-by: Alex Fedotyev <alex-fedotyev@users.noreply.github.com>
Map keys like "process.runtime.name" are a single key with dots in it,
not a nested path. The previous approach quoted each segment separately:
  toString(ResourceAttributes.\`process\`.\`runtime\`.\`name\`)
which ClickHouse interpreted as nested tuple element access and failed
with "Unknown expression or function identifier".

Now quotes the entire key as one identifier:
  toString(ResourceAttributes.\`process.runtime.name\`)
which correctly accesses the Map key as a single string.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The sidebar wraps facet keys from useGetKeyValues as
toString(ResourceAttributes.os.version) — no backtick quoting.
Our previous backtick-quoted format didn't match, causing the filter
to appear as a separate entry and the sidebar to regenerate it
without backticks on checkbox toggle, producing invalid SQL.

Now uses the exact same format as the sidebar: toString(ColName.key)
with unquoted dot-notation, ensuring filter keys match facet keys
and checkboxes work correctly on toggle.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rgePath)

The trace flyout's "Add to Filters" uses mergePath() which produces
per-segment backtick quoting: ResourceAttributes.\`service\`.\`instance\`.\`id\`
wrapped in toString(). This format is valid ClickHouse SQL (backticks
are identifiers) and matches the sidebar's facet key format.

Previous attempts failed because:
- Unquoted dots: ClickHouse treats as nested tuple access → error
- Single whole-key backtick: didn't match sidebar facet format
- Bracket notation: didn't match sidebar facet format

Now matches the exact same format as mergePath() in utils.ts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
toString() and backtick-quoting approaches all fail for Map columns
with dotted keys — ClickHouse interprets dots as nested tuple access
regardless of quoting style.

The only format that reliably works is bracket notation:
  ResourceAttributes['service.instance.id']

This matches what the search bar produces when filtering from trace
detail view (WHERE ResourceAttributes['k8s.pod.name'] = 'value').

Trade-off: bracket-notation keys won't match sidebar facet keys
(which use toString + backtick format), so filters from the delta
chart appear as separate entries rather than matching checkboxes.
This is acceptable — the filter works correctly at the SQL level,
and improving sidebar compatibility is tracked for a follow-up PR.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix import sort order in DBDeltaChart.test.ts and PropertyComparisonChart.tsx
- Fix prettier formatting in test file
- Update docstrings in deltaChartUtils.ts and DBDeltaChart.tsx to reflect
  the bracket-notation filter key format (no longer toString/backtick)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The click popover (Filter/Exclude/Copy actions) was hard to dismiss —
clicking elsewhere in the UI didn't close it because the click-outside
handler required clicks to be outside both the popover AND the chart
wrapper, and Recharts consumed mousedown events on other charts.

Now:
- Escape key closes the popover
- Clicking anywhere outside the popover closes it (simplified check)
- Scroll still closes it (unchanged)
- All three handlers consolidated into a single useEffect

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@alex-fedotyev alex-fedotyev requested review from a team and teeohhem and removed request for a team February 27, 2026 02:41
@alex-fedotyev alex-fedotyev marked this pull request as ready for review February 27, 2026 03:42
@teeohhem
Copy link
Contributor

teeohhem commented Feb 27, 2026

image

Filtering action text appears behind the popover.

image

The X axis text is black and hard to read.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants