feat(metrics): Dashboard charts performance improvements#3083
feat(metrics): Dashboard charts performance improvements#3083
Conversation
|
WalkthroughThis pull request introduces a series-limiting feature to chart rendering, capping the number of rendered series at 50 (MAX_SERIES) to manage SVG rendering budget. The changes compute the total series count from the data and filter to show only the top series by absolute total values. A Callout component is displayed when truncation occurs. Simultaneously, the activePayload state management is refactored from the HighlightState hook into a separate PayloadContext to optimize re-renders. All chart components (Bar, Line, Legend) are updated to consume visibleSeries instead of all dataKeys for rendering, and to use the new setActivePayload function. The ChartRoot component gains visibleSeries and beforeLegend props that are threaded through the provider and inner components. Dynamic effective max points are calculated based on the filtered series count to maintain SVG budget constraints. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Key changes across files
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| ...totals, | ||
| ...hoverData, | ||
| }; | ||
| }, [highlight.activePayload, totals]); | ||
| }, [activePayload, totals]); |
There was a problem hiding this comment.
🚩 Legend shows stale hover data for non-visible (truncated) series when hovering chart
When visibleSeries is a strict subset of dataKeys (i.e., some series are rendered in the legend but not on the chart), hovering a data point on the chart only produces activePayload entries for the visible/rendered series. The legend's currentData computation at ChartLegendCompound.tsx:113-133 merges hover data with totals, so non-visible series fall back to their aggregate totals while visible series show the hovered point's value.
This means the legend mixes point-specific values (for visible series) with aggregate totals (for non-visible series) during hover. This is arguably the best available behavior given that non-visible series have no SVG element to provide point-level data, but it could be confusing if users notice the numbers don't sum correctly. Worth considering whether non-visible series should show a dash (–) during hover instead.
(Refers to lines 112-133)
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/webapp/app/components/primitives/charts/ChartLine.tsx (1)
127-131: Minor:setTooltipActive(false)is redundant beforereset().In
handleMouseLeave,highlight.reset()already setstooltipActive: false(viainitialState). The precedingsetTooltipActive(false)is harmless but unnecessary.Also applies to: 144-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/primitives/charts/ChartLine.tsx` around lines 127 - 131, Remove the redundant explicit tooltip toggle before calling reset: in the handleMouseLeave handler, drop the highlight.setTooltipActive(false) call and just call highlight.reset(); do the same for the other similar handler block around the 144-151 region (remove the setTooltipActive(false) before highlight.reset()). This keeps the code concise because highlight.reset() already restores tooltipActive via the component's initial state.apps/webapp/app/components/primitives/charts/ChartContext.tsx (1)
52-52: Consider a narrower type thanany[]for payload.
PayloadContextanduseActivePayloadboth useany[] | null. Since this is the recharts tooltip payload, you could use recharts'Payloadtype for slightly better type safety. Not critical given the existinganyusage across chart components, but worth considering if you tighten types later.Also applies to: 63-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/primitives/charts/ChartContext.tsx` at line 52, Replace the broad any[]|null with Recharts' Payload type: update PayloadContext (createContext<any[] | null>(null)) and the useActivePayload return/type annotations to use Payload[] | null, and add an import for the Payload type from Recharts (e.g., import { Payload } from 'recharts/types/component/DefaultTooltipContent' or the appropriate recharts export) so the tooltip payload has stronger typing.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/primitives/charts/ChartContext.tsxapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsxapps/webapp/app/components/primitives/charts/hooks/useHighlightState.tsapps/webapp/app/components/primitives/charts/hooks/useZoomSelection.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/components/primitives/charts/hooks/useZoomSelection.tsapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartContext.tsxapps/webapp/app/components/primitives/charts/hooks/useHighlightState.tsapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/primitives/charts/hooks/useZoomSelection.tsapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartContext.tsxapps/webapp/app/components/primitives/charts/hooks/useHighlightState.tsapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/components/primitives/charts/hooks/useZoomSelection.tsapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartContext.tsxapps/webapp/app/components/primitives/charts/hooks/useHighlightState.tsapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/primitives/charts/hooks/useZoomSelection.tsapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartContext.tsxapps/webapp/app/components/primitives/charts/hooks/useHighlightState.tsapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/components/primitives/charts/hooks/useZoomSelection.tsapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartContext.tsxapps/webapp/app/components/primitives/charts/hooks/useHighlightState.tsapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/components/primitives/charts/hooks/useZoomSelection.tsapps/webapp/app/components/primitives/charts/hooks/useHighlightState.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/components/primitives/charts/hooks/useZoomSelection.tsapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartContext.tsxapps/webapp/app/components/primitives/charts/hooks/useHighlightState.tsapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsx
🧠 Learnings (3)
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartContext.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsx
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Use `useRun`, `useRealtimeRun` and other SWR/realtime hooks from `trigger.dev/react-hooks` for data fetching
Applied to files:
apps/webapp/app/components/primitives/charts/ChartContext.tsx
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Use `trigger.dev/react-hooks` package for realtime subscriptions in React components
Applied to files:
apps/webapp/app/components/primitives/charts/ChartContext.tsx
🧬 Code graph analysis (3)
apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsx (1)
apps/webapp/app/components/primitives/charts/ChartContext.tsx (1)
useActivePayload(63-65)
apps/webapp/app/components/code/QueryResultsChart.tsx (3)
apps/webapp/app/components/primitives/Callout.tsx (1)
Callout(73-170)apps/webapp/app/components/primitives/charts/ChartBlankState.tsx (1)
ChartBlankState(4-23)apps/webapp/app/components/primitives/charts/ChartCompound.tsx (1)
Chart(92-98)
apps/webapp/app/components/primitives/charts/ChartContext.tsx (2)
apps/webapp/app/components/primitives/charts/hooks/useHighlightState.ts (1)
UseHighlightStateReturn(23-23)apps/webapp/app/components/primitives/charts/Chart.tsx (1)
ChartConfig(11-20)
🔇 Additional comments (12)
apps/webapp/app/components/code/QueryResultsChart.tsx (2)
609-665: Good approach: computing group totals in a single pass before building heavy data objects.Accumulating
groupTotalsalongside the row iteration and then selecting only the top-N groups before constructing the per-point data objects avoids allocating thousands of keys per data point. Clean optimization.
562-566: Dynamic SVG budget calculation looks correct.The
effectiveMaxPointscalculation is sound:MAX_SVG_ELEMENT_BUDGET / seriesCount, clamped betweenMIN_DATA_POINTSandMAX_DATA_POINTS. Both the grouped and non-grouped paths use consistent logic.Also applies to: 667-671
apps/webapp/app/components/primitives/charts/hooks/useHighlightState.ts (1)
38-77: Clean refactor: memoized return and functional updaters to reduce re-renders.The early-return checks in
setHoveredLegendItemandsetTooltipActiveprevent unnecessary state transitions, and theuseMemowrapper gives consumers a stable object reference. Well done.apps/webapp/app/components/primitives/charts/hooks/useZoomSelection.ts (1)
178-190: Consistent memoization pattern withuseHighlightState.Stable callbacks (via
useCallback+stateRef) mean the memo only recomputes on state changes. Good alignment with the broader re-render reduction effort.apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsx (1)
56-57: Good separation: legend consumes payload fromPayloadContextindependently.This isolates legend re-renders from chart element re-renders. The
activePayloadfromuseActivePayload()correctly replaces all formerhighlight.activePayloadreferences, whilehighlight.activeBarKeyis still used for series-level highlighting (lines 153, 203). Clean split.apps/webapp/app/components/primitives/charts/ChartLine.tsx (1)
81-81: Rendering switched tovisibleSeriesthroughout — looks correct.The stacked guard, Area map, and Line map all consistently use
visibleSeriesfrom context, ensuring only the capped subset of series generates SVG elements.Also applies to: 133-133, 164-164, 212-212
apps/webapp/app/components/primitives/charts/ChartRoot.tsx (1)
69-120: Clean prop threading ofvisibleSeriesandbeforeLegend.Both new props are correctly forwarded:
visibleSeries→ChartProvider(context),beforeLegend→ChartRootInner(render). The existing hooks (useHasNoData,useSeriesTotal) correctly continue to usedataKeys(all series) rather thanvisibleSeries.apps/webapp/app/components/primitives/charts/ChartContext.tsx (2)
48-65: Effective context split for payload isolation.Separating
activePayloadinto its ownPayloadContextis a well-targeted optimization — frequent mouse-move updates only re-render the legend (viauseActivePayload), not the chart SVG elements consumingChartCompoundContext. The dedup viaactiveTooltipIndexRef(lines 107–117) further reduces state churn on same-index mouse moves.
119-130: The code correctly maintains referential identity for non-reset properties.The
highlightWithResetimplementation properly spreadshighlight(which includesactiveBarKey,activeDataPointIndex,tooltipActive,setHoveredBar,setHoveredLegendItem, andsetTooltipActive) and overrides only theresetmethod. SinceresetWithPayloaddepends onoriginalResetand the entire object is memoized with[highlight, resetWithPayload]as dependencies, the identity updates appropriately whenever either source changes—the correct and expected behavior.apps/webapp/app/components/primitives/charts/ChartBar.tsx (3)
86-89:handleMouseLeavecorrectly chains zoom + highlight reset.Since
highlight.resetis nowresetWithPayload(viaChartContext), this also clearsactivePayloadand the tooltip index ref. Good integration.
68-68: Context consumption expanded cleanly.
visibleSeriesandsetActivePayloadare destructured from context and used consistently:visibleSeriesfor the Bar map,setActivePayloadfor mouse-move events.
187-220: Behavior change: dimming now applies per-series, not per-cell.The previous
Cell-based rendering allowed highlighting a single bar at a specific data point (viagetBarOpacitychecking bothactiveBarKeyandactiveDataPointIndex). The newfillOpacityon theBarcomponent dims/brightens the entire series at once. This is a good trade-off for performance with many series, but note that hovering a specific bar now highlights the entire series strip rather than just that cell.Also,
getBarOpacityinuseHighlightState.tsis now unused and can be removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/webapp/app/components/code/QueryResultsChart.tsx`:
- Around line 820-835: The callout logic currently compares totalSeriesCount to
series.length which is always false in the non-grouped path; update the
condition that builds seriesLimitCallout to compare totalSeriesCount against
visibleSeries.length (or sortedSeries.length) instead so the warning is shown
whenever visibleSeries is truncated; locate the relevant variables
visibleSeries, totalSeriesCount, series, sortedSeries and change the conditional
expression used to render seriesLimitCallout accordingly.
---
Nitpick comments:
In `@apps/webapp/app/components/primitives/charts/ChartContext.tsx`:
- Line 52: Replace the broad any[]|null with Recharts' Payload type: update
PayloadContext (createContext<any[] | null>(null)) and the useActivePayload
return/type annotations to use Payload[] | null, and add an import for the
Payload type from Recharts (e.g., import { Payload } from
'recharts/types/component/DefaultTooltipContent' or the appropriate recharts
export) so the tooltip payload has stronger typing.
In `@apps/webapp/app/components/primitives/charts/ChartLine.tsx`:
- Around line 127-131: Remove the redundant explicit tooltip toggle before
calling reset: in the handleMouseLeave handler, drop the
highlight.setTooltipActive(false) call and just call highlight.reset(); do the
same for the other similar handler block around the 144-151 region (remove the
setTooltipActive(false) before highlight.reset()). This keeps the code concise
because highlight.reset() already restores tooltipActive via the component's
initial state.
ericallam
changed the title
2 participants
Summary