Skip to content

feat: signals UI enhancement#1383

Merged
skull8888888 merged 31 commits intodevfrom
feat/signals-visualization
Mar 13, 2026
Merged

feat: signals UI enhancement#1383
skull8888888 merged 31 commits intodevfrom
feat/signals-visualization

Conversation

@kolbeyang
Copy link
Collaborator

@kolbeyang kolbeyang commented Mar 11, 2026

Note

Medium Risk
Touches core Signals events browsing by adding cluster-based filtering, new stats queries, and a redesigned events table; errors here could change query results or UI navigation. Changes span both frontend state management and ClickHouse query construction.

Overview
Signals Events UI is reworked around event clusters. The old clusters table + single-series events chart are removed and replaced with a new resizable ClustersSection (drill-down list, breadcrumb navigation, and stacked time-series chart) driven by new Zustand cluster state/selectors and URL clusterId query state (via nuqs).

Events table becomes schema-driven and supports cluster filtering. Columns/filters are now generated from the signal output schema (payload fields become typed columns with tooltips and boolean/enum rendering), row selection opens a new event detail side panel, and trace navigation is improved with copy/open-trace controls.

Backend/query changes to support the UI. Cluster APIs now return total vs clustered counts and add a new /events/clusters/stats endpoint for per-cluster time-series; events pagination adds clusterId[] and unclustered filters (ClickHouse hasAny(clusters, …) / empty(clusters)), payload filtering is updated to understand payload.<field> with typed comparisons, and the query engine validator allows the signal_events.clusters column.

Written by Cursor Bugbot for commit 3dfe965. This will update automatically on new commits. Configure here.

@kolbeyang kolbeyang self-assigned this Mar 11, 2026
@CLAassistant
Copy link

CLAassistant commented Mar 11, 2026

CLA assistant check
All committers have signed the CLA.

@kolbeyang kolbeyang changed the base branch from main to dev March 11, 2026 18:50
@kolbeyang kolbeyang changed the title feat: signals UI enhancement [WIP] feat: signals UI enhancement Mar 11, 2026
kolbeyang and others added 13 commits March 11, 2026 11:51
… buckets, FINAL for dedup

- Max height 300px on cluster container, chart fills available height
- Cluster list scrolls when overflow
- Cluster stats query uses WITH FILL to fill empty time buckets with zeros
- Clusters query uses FINAL to deduplicate ReplacingMergeTree rows
- TimeSeriesChart accepts className prop to override chart height

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Breadcrumb uses outer AnimatePresence (positional keys) for level
  add/remove and inner AnimatePresence (node ID keys) for sibling swaps
- Fix unclustered event count using clusteredEventCount from query engine
  instead of summing tree nodes (avoids double-counting from flattening)
- Match skeleton height to loaded state

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract ClusterItem into cluster-list/cluster-item.tsx (one component per file)
- Add 500ms framer motion delay before tooltip appears on hover
- Add overflow-hidden to tooltip to prevent text overflow during grow animation
- Use React state for button hover highlight instead of CSS :hover to handle portal overlay

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…eusable ClusterItem

- Convert event detail panel from inline sidebar to animated full-screen drawer
- Add selectedEvent/setSelectedEvent to signal store for shared state
- Fix cluster stats to use executeQuery (query engine) instead of direct clickhouseClient,
  which was querying raw signal_events table missing the clusters column
- Fix unclustered stats to query empty(clusters) directly instead of subtraction
  (subtraction overcounted due to events belonging to multiple clusters)
- Use clusters array column on signal_events (via query engine view) for event filtering
- Make ClusterItem reusable with iconVariant/color props, use for unclustered button
- Always show filtered vs total event counts in cluster tooltips when time range active
- Add clusters to query engine allowed columns for signal_events

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove console.log debug statements from events-table and events action
- Add cancellation flag to stats-fetching useEffect to prevent stale data
- Stabilize filter array reference with useMemo to prevent infinite re-fetches
- Fix unsafe EventCluster-to-ClusterNode casts with proper typing
- Add timeout cleanup on unmount in ClusterItem hover tooltip
- Use stable node.id keys instead of index-based keys in breadcrumb animation
- Fix breadcrumb reset to depend on clusterPath content, not just length
- Validate scale param in signals stats route instead of bare type assertion
- Fix duplicate fuchsia-500 color in palette, replace with violet-500
- Remove unused overlayRef and redundant XIcon import
- Fix missing space in cluster-stacked-chart import
- Remove stale TODO comments

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ehavior

- Use nuqs useClusterId hook as single source of truth for cluster navigation
- Parameterize store selectors as factory functions accepting clusterId
- Handle leaf node and unclustered selection: chart shows selected data,
  list stays at parent level, deselect returns to parent
- Build color map in parent so chart colors match list colors
- Rename clusters-table to clusters-section
- Use ResizablePanelGroup for cluster list/chart layout
- Trace ID column: show tail with RTL truncation, use ArrowUpRight icon

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… and Button

- Split manage-signal-sheet.tsx into folder with one component per file
- Group template-picker and test-results-view into subfolders with index.tsx pattern
- Add xs/sm/md size prop to Input component (xs default, preserves existing behavior)
- Add md size variant to Button component
- Use size sm for signal name input, md for Test/Create/Run test buttons
- Right-justify footer buttons, disable Test when Create is disabled
- Reduce sheet width by 10%, remove selected trace chip, remove test flask icon
- Rename template label to "Template" with consistent header styling

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…d trace-picker

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@kolbeyang kolbeyang force-pushed the feat/signals-visualization branch from be7dcad to 9a72aec Compare March 11, 2026 18:52
@kolbeyang
Copy link
Collaborator Author

bugbot run

… prevention

- Store clusterTree as state, computed once when rawClusters changes,
  so selectTree returns stable references for Zustand equality checks
- Replace no-op cancelled variable with AbortController pattern to
  cancel in-flight cluster stats requests on dependency changes
- Rethrow AbortError past per-fetch .catch() so it propagates correctly

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@kolbeyang
Copy link
Collaborator Author

bugbot run

kolbeyang and others added 2 commits March 11, 2026 15:26
Replace tab bar with a breadcrumb dropdown that opens a 3-column
overview popover (Input → Definition → Output). Move "Edit Signal"
to the far right, opening the sheet directly. Support controlled
mode in ManageSignalSheet by conditionally rendering SheetTrigger.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@kolbeyang
Copy link
Collaborator Author

bugbot run

ClickHouse may return count() as strings; wrap with Number() to ensure
correct strict equality checks. Also extract individual search params
(pastHours, startDate, endDate) so the stats-fetch effect only re-fires
when those values change, not on every unrelated URL param update.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines +91 to +93
export const selectBreadcrumb =
(clusterId: string | null) =>
(state: Store): ClusterNode[] => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@olzhik11 is there a better way to handle UNCLESTERED_ID thing here?

@kolbeyang kolbeyang changed the title [WIP] feat: signals UI enhancement feat: signals UI enhancement Mar 12, 2026
@kolbeyang
Copy link
Collaborator Author

bugbot run

import { prettifyError, z, ZodError } from "zod/v4";

import { generateSql } from "@/lib/actions/sql";
import { generateSql } from "@/lib/actions/sql/generate";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm generateSql actually isn't exported from @/lib/actions/sql so this seems correct

import { prettifyError, ZodError } from "zod/v4";

import { createExportJob } from "@/lib/actions/sql";
import { createExportJob } from "@/lib/actions/sql/export-job";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same here, this is the correct import because it's not exported from actions/sql/index.ts

…se columns

- Replace signal header popover/dropdown with tooltip overlay on TabsList
- Remove TracePicker dual-mode (url/state), keep only state-based filtering
- Reuse trace-picker columns in debugger sidebar instead of duplicate columns.tsx
- Restructure test-panel and test-results-view into folders
- Fix filter memoization key: use JSON.stringify instead of join(",")
- Rename "All Events" breadcrumb to "Event clusters"

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kolbeyang and others added 2 commits March 12, 2026 10:51
…nal-sheet restructure

Reverts signals list page, manage-signal-sheet folder restructure,
signal-sparkline, empty-row, trace-picker, and debugger-sessions
sidebar trace-picker usage back to dev versions to reduce PR scope.
These changes will be stacked in a follow-up PR.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… query

Move cluster event counting from events/stats.ts into lib/actions/clusters
and return both clustered and unclustered counts in a single API call,
eliminating the need for separate requests and clusterIds filtering.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@kolbeyang kolbeyang requested a review from olzhik11 March 12, 2026 21:36
}),
]);

const items: ClusterStatsDataPoint[] = clusterRows.map((row) => ({
Copy link
Member

Choose a reason for hiding this comment

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

@kolbeyang i think here we don't need to loop again, it should return as number already if not mistaken, and we can return unclustered id right away in query,

SELECT
  '00000000-0000-0000-0000-000000000000' as cluster_id,
  toStartOfInterval(timestamp, toInterval({intervalValue: UInt32}, {intervalUnit: String})) as ts,
  count() as count

or modify type to make it optional, this would ease logic

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@olzhik11 For this one do you think unclustered should be unified with the other clusters using UNCLUSTERED_ID?

Currently I treat it as kind of its own thing in the store

        set({
          rawClusters: data.items,
          clusterTree: buildTree(data.items),
          totalEventCount: data.totalEventCount,
          clusteredEventCount: data.clusteredEventCount,
        });

Copy link
Member

Choose a reason for hiding this comment

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

separately is fine i think

return findNodeById(selectTree(state), clusterId);
};

export const selectBreadcrumb =
Copy link
Member

Choose a reason for hiding this comment

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

I like the selectors approach, very clean, just for reference we can do it this way:

export const getBreadcrumb = (state: Store, clusterId: string | null): ClusterNode[] => { ... };

const breadcrumb = useSignalStoreContext((state) => getBreadcrumb(state, clusterId));

here we would not need to use useMemo

kolbeyang and others added 5 commits March 13, 2026 08:22
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ckHouse queries

Adds an optional `dataType` discriminator to the filter schema so consumers
(especially the events defaultProcessor) can use the correct ClickHouse
extraction function for payload fields (e.g. simpleJSONExtractFloat for
numbers, simpleJSONExtractBool for booleans). The field is optional to
avoid breaking persisted triggers or bookmarked URL params.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ClickHouse returns count() as JSON numbers (output_format_json_quote_64bit_integers=0),
so type them as number directly instead of string + parseInt mapping.

Also minor tooltip styling tweaks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix renamed store exports (selectBreadcrumb → getBreadcrumb, etc.)
- Type cluster integer fields (level, numChildrenClusters, numEvents)
  as number, removing redundant parseInt wrappers

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Missing shallow equality causes unnecessary event refetches
    • Added Zustand shallow equality to the selectedClusterIds selector so unchanged cluster ID arrays no longer trigger infinite-scroll dependency resets and redundant event refetches.

Create PR

Or push these changes by commenting:

@cursor push 4451e1e41b
Preview (4451e1e41b)
diff --git a/frontend/components/signal/events-table/index.tsx b/frontend/components/signal/events-table/index.tsx
--- a/frontend/components/signal/events-table/index.tsx
+++ b/frontend/components/signal/events-table/index.tsx
@@ -3,6 +3,7 @@
 import { type Row } from "@tanstack/react-table";
 import { useParams, usePathname, useRouter, useSearchParams } from "next/navigation";
 import { useCallback, useEffect, useMemo } from "react";
+import { shallow } from "zustand/shallow";
 
 import ClustersSection from "@/components/signal/clusters-section";
 import ClusterBreadcrumbs from "@/components/signal/clusters-section/cluster-breadcrumbs";
@@ -59,7 +60,7 @@
   const [clusterId] = useClusterId();
   const signal = useSignalStoreContext((state) => state.signal);
   const selectedEvent = useSignalStoreContext((state) => state.selectedEvent);
-  const selectedClusterIds = useSignalStoreContext((state) => getFilterClusterIds(state, clusterId));
+  const selectedClusterIds = useSignalStoreContext((state) => getFilterClusterIds(state, clusterId), shallow);
   const isUnclusteredFilter = clusterId === UNCLUSTERED_ID;
   const searchParams = useSearchParams();
   const pathName = usePathname();

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

kolbeyang and others added 2 commits March 13, 2026 09:45
…d EventsStatsDataPoint import)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
GROUP BY cluster_id, timestamp
ORDER BY cluster_id, timestamp ASC
${withFillClause}
`;
Copy link

Choose a reason for hiding this comment

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

ClickHouse WITH FILL creates incorrect cross-cluster rows

Medium Severity

The cluster time-series query orders by cluster_id, timestamp and appends a WITH FILL clause. In ClickHouse, WITH FILL fills gaps between consecutive rows in the result. Because ordering is by cluster_id first, the fill will generate spurious rows between the last timestamp of one cluster and the first timestamp of the next, assigning incorrect cluster_id values to the filled rows. The unclustered query doesn't have this issue since it has no grouping column.

Fix in Cursor Fix in Web

kolbeyang and others added 4 commits March 13, 2026 10:05
Prevents unnecessary event refetches and re-renders caused by new
references from getFilterClusterIds and getFilteredCountByCluster.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

for (const key of fieldKeys) {
(point as Record<string, unknown>)[key] = counts[key] || 0;
}
return point;
Copy link

Choose a reason for hiding this comment

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

Stacked chart data points missing required count field

Low Severity

Data points created for the stacked chart are cast as TimeSeriesDataPoint but never set the required count property — only dynamic cluster-ID keys are populated. This works now because showTotal={false} is passed, but if showTotal were ever omitted or set to true, accessing d.count would yield undefined, producing NaN in totals. The as cast silently masks this type violation.

Fix in Cursor Fix in Web

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not a real issue. TimeSeriesDataPoint is typed as { timestamp: string } & Record<string, number> — there is no required count field. The dynamic cluster-ID keys satisfy the type correctly.

Additionally, totalCount (used when showTotal is true) iterates Object.entries(dataPoint) and sums all numeric values, skipping "timestamp" — it never accesses a .count property. So even with showTotal={true}, this would work fine.

Copy link
Member

@olzhik11 olzhik11 left a comment

Choose a reason for hiding this comment

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

lgtm @kolbeyang, lets check last comment

@skull8888888 skull8888888 merged commit 220209b into dev Mar 13, 2026
4 checks passed
@skull8888888 skull8888888 deleted the feat/signals-visualization branch March 13, 2026 18:43
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.

4 participants