Echarts shared module - #1558
Conversation
There was a problem hiding this comment.
Pull request overview
Introduces a new shared ECharts charting module for the frontend (builders, layouts, utilities, and interaction hooks), plus an EChartsDemo module to exercise the shared API and persist zoom state.
Changes:
- Added
@modules/_shared/eChartspackage-style module with chart builders (timeseries, histogram, density, exceedance, convergence, etc.), common layout/composition, and interaction hooks. - Added
EChartsDemomodule showcasing charts, linked hover, click-to-timestamp, and persisted zoom state. - Added unit tests for core math/utilities and shared-axis behavior; added ECharts dependencies.
Reviewed changes
Copilot reviewed 99 out of 100 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/tests/unit/eCharts/statistics.test.ts | Adds unit tests for point/timeseries statistics helpers. |
| frontend/tests/unit/eCharts/sharedAxes.test.ts | Adds tests validating shared axis extent linking across subplots. |
| frontend/tests/unit/eCharts/kde.test.ts | Adds unit tests for KDE computation helper. |
| frontend/tests/unit/eCharts/histogram.test.ts | Adds tests for histogram binning/layout utilities. |
| frontend/tests/unit/eCharts/convergence.test.ts | Adds tests for convergence calculations. |
| frontend/tests/unit/eCharts/closestMemberTooltip.test.ts | Adds tests for closest-member selection used in tooltip snapping. |
| frontend/src/modules/registerAllModules.ts | Registers the new EChartsDemo module in dev mode. |
| frontend/src/modules/_shared/eCharts/utils/statistics.ts | Implements point and per-timestep timeseries statistics helpers. |
| frontend/src/modules/_shared/eCharts/utils/kde.ts | Implements Gaussian KDE helper. |
| frontend/src/modules/_shared/eCharts/utils/index.ts | Re-exports shared ECharts utility functions. |
| frontend/src/modules/_shared/eCharts/utils/histogram.ts | Implements histogram binning and bar layout computation. |
| frontend/src/modules/_shared/eCharts/utils/convergence.ts | Implements convergence series computation utility. |
| frontend/src/modules/_shared/eCharts/types.ts | Defines shared trace/config/types for all chart families. |
| frontend/src/modules/_shared/eCharts/overlays/index.ts | Exposes overlay helpers from a single entrypoint. |
| frontend/src/modules/_shared/eCharts/overlays/activeTimestampMarker.ts | Adds markLine overlay for an active timestamp. |
| frontend/src/modules/_shared/eCharts/layout/subplotGridLayout.ts | Adds responsive subplot grid layout computation. |
| frontend/src/modules/_shared/eCharts/layout/subplotAxes.ts | Adds builder for per-subplot axes and titles. |
| frontend/src/modules/_shared/eCharts/layout/responsiveConfig.ts | Adds responsive feature toggles (legend/toolbox/sliders) by container size. |
| frontend/src/modules/_shared/eCharts/layout/index.ts | Re-exports layout helpers/types. |
| frontend/src/modules/_shared/eCharts/index.ts | Adds the public root API surface for the shared ECharts module. |
| frontend/src/modules/_shared/eCharts/hooks/useMemberInteraction.ts | Adds unified hover/highlight + closest-member tooltip behavior. |
| frontend/src/modules/_shared/eCharts/hooks/useEchartsViewState.ts | Adds zoom state persistence handler for dataZoom events. |
| frontend/src/modules/_shared/eCharts/hooks/useClickToTimestamp.ts | Adds ctrl/meta-click to resolve and toggle timestamp selection. |
| frontend/src/modules/_shared/eCharts/hooks/index.ts | Re-exports shared ECharts hooks. |
| frontend/src/modules/_shared/eCharts/core/tooltip.ts | Adds compact tooltip formatting utilities. |
| frontend/src/modules/_shared/eCharts/core/seriesId.ts | Adds structured series ID creation/parsing for stable interaction/tooltip logic. |
| frontend/src/modules/_shared/eCharts/core/index.ts | Re-exports core composition/building utilities. |
| frontend/src/modules/_shared/eCharts/core/composeChartOption.ts | Composes final ECharts options and defines zoom state types. |
| frontend/src/modules/_shared/eCharts/core/cartesianSubplotChart.ts | Adds shared cartesian subplot chart builder with shared axis linking + zoom state restore. |
| frontend/src/modules/_shared/eCharts/core/aggregateSubplotTraces.ts | Adds helper to aggregate per-trace series/legend building across subplots. |
| frontend/src/modules/_shared/eCharts/charts/timeseries/tooltips.ts | Implements timeseries tooltips for members/statistics/observations. |
| frontend/src/modules/_shared/eCharts/charts/timeseries/statisticsSeries.ts | Builds statistics lines and fanchart bands. |
| frontend/src/modules/_shared/eCharts/charts/timeseries/observationSeries.ts | Builds custom observation/errorbar series. |
| frontend/src/modules/_shared/eCharts/charts/timeseries/memberSeries.ts | Builds per-realization member line series. |
| frontend/src/modules/_shared/eCharts/charts/timeseries/index.ts | Re-exports timeseries chart family symbols. |
| frontend/src/modules/_shared/eCharts/charts/timeseries/ids.ts | Defines series-id helpers for timeseries family. |
| frontend/src/modules/_shared/eCharts/charts/timeseries/historySeries.ts | Builds historical reference line series. |
| frontend/src/modules/_shared/eCharts/charts/timeseries/builder.ts | Builds complete timeseries chart options from traces/config. |
| frontend/src/modules/_shared/eCharts/charts/percentileRange/tooltips.ts | Implements percentile-range tooltip formatters. |
| frontend/src/modules/_shared/eCharts/charts/percentileRange/series.ts | Builds percentile-range glyph + realization scatter series. |
| frontend/src/modules/_shared/eCharts/charts/percentileRange/index.ts | Re-exports percentile-range chart family symbols. |
| frontend/src/modules/_shared/eCharts/charts/percentileRange/ids.ts | Defines series-id helpers for percentile-range family. |
| frontend/src/modules/_shared/eCharts/charts/percentileRange/builder.ts | Builds percentile-range chart options for grouped traces. |
| frontend/src/modules/_shared/eCharts/charts/memberScatter/tooltips.ts | Implements member-scatter item tooltip. |
| frontend/src/modules/_shared/eCharts/charts/memberScatter/series.ts | Builds member scatter series (per member). |
| frontend/src/modules/_shared/eCharts/charts/memberScatter/index.ts | Re-exports member-scatter chart family symbols. |
| frontend/src/modules/_shared/eCharts/charts/memberScatter/ids.ts | Defines series-id helpers for member-scatter family. |
| frontend/src/modules/_shared/eCharts/charts/memberScatter/builder.ts | Builds member-scatter chart options and tooltip config. |
| frontend/src/modules/_shared/eCharts/charts/index.ts | Re-exports all chart family builders from a single entrypoint. |
| frontend/src/modules/_shared/eCharts/charts/histogram/utils.ts | Implements histogram axis extent and opacity helpers. |
| frontend/src/modules/_shared/eCharts/charts/histogram/tooltips.ts | Implements histogram bar/rug tooltips. |
| frontend/src/modules/_shared/eCharts/charts/histogram/series.ts | Builds histogram bars + optional rug points series. |
| frontend/src/modules/_shared/eCharts/charts/histogram/index.ts | Re-exports histogram chart family symbols. |
| frontend/src/modules/_shared/eCharts/charts/histogram/ids.ts | Defines series-id helpers for histogram family. |
| frontend/src/modules/_shared/eCharts/charts/histogram/builder.ts | Builds histogram chart options and y-axis post-processing. |
| frontend/src/modules/_shared/eCharts/charts/heatmap/utils.ts | Adds heatmap subplot normalization + visualMap helpers. |
| frontend/src/modules/_shared/eCharts/charts/heatmap/tooltips.ts | Implements heatmap tooltips. |
| frontend/src/modules/_shared/eCharts/charts/heatmap/series.ts | Builds heatmap series. |
| frontend/src/modules/_shared/eCharts/charts/heatmap/index.ts | Re-exports heatmap chart family symbols. |
| frontend/src/modules/_shared/eCharts/charts/heatmap/ids.ts | Defines series-id helpers for heatmap family. |
| frontend/src/modules/_shared/eCharts/charts/heatmap/builder.ts | Builds heatmap chart options including visualMap + tooltip datasets. |
| frontend/src/modules/_shared/eCharts/charts/exceedance/tooltips.ts | Implements exceedance axis tooltip formatting. |
| frontend/src/modules/_shared/eCharts/charts/exceedance/series.ts | Builds exceedance curve series. |
| frontend/src/modules/_shared/eCharts/charts/exceedance/index.ts | Re-exports exceedance chart family symbols. |
| frontend/src/modules/_shared/eCharts/charts/exceedance/ids.ts | Defines series-id helpers for exceedance family. |
| frontend/src/modules/_shared/eCharts/charts/exceedance/builder.ts | Builds exceedance chart options (incl. y-axis constraints + tooltip). |
| frontend/src/modules/_shared/eCharts/charts/density/series.ts | Builds density (KDE) series + optional realization points. |
| frontend/src/modules/_shared/eCharts/charts/density/index.ts | Re-exports density chart family symbols. |
| frontend/src/modules/_shared/eCharts/charts/density/ids.ts | Defines series-id helpers for density family. |
| frontend/src/modules/_shared/eCharts/charts/density/builder.ts | Builds density chart options. |
| frontend/src/modules/_shared/eCharts/charts/convergence/tooltips.ts | Implements convergence axis tooltip formatting. |
| frontend/src/modules/_shared/eCharts/charts/convergence/series.ts | Builds convergence lines + band series. |
| frontend/src/modules/_shared/eCharts/charts/convergence/index.ts | Re-exports convergence chart family symbols. |
| frontend/src/modules/_shared/eCharts/charts/convergence/ids.ts | Defines series-id helpers for convergence family. |
| frontend/src/modules/_shared/eCharts/charts/convergence/builder.ts | Builds convergence chart options and tooltip. |
| frontend/src/modules/_shared/eCharts/charts/bar/tooltips.ts | Implements bar axis/mean tooltips. |
| frontend/src/modules/_shared/eCharts/charts/bar/series.ts | Builds bar series + optional mean reference line. |
| frontend/src/modules/_shared/eCharts/charts/bar/index.ts | Re-exports bar chart family symbols. |
| frontend/src/modules/_shared/eCharts/charts/bar/ids.ts | Defines series-id helpers for bar family. |
| frontend/src/modules/_shared/eCharts/charts/bar/builder.ts | Builds bar chart options and tooltip config. |
| frontend/src/modules/_shared/eCharts/README.md | Documents shared module purpose, architecture, and consumption. |
| frontend/src/modules/_shared/eCharts/AGENTS.md | Adds module-specific engineering guidance (API, typing, testing). |
| frontend/src/modules/ModuleSerializedStateMap.ts | Adds serialized state mapping for EChartsDemo. |
| frontend/src/modules/EChartsDemo/view/view.tsx | Implements demo UI view, including hover/zoom/timestamp interactions. |
| frontend/src/modules/EChartsDemo/view/useEcharts.tsx | Orchestrates which shared chart builder to use per selected plot type. |
| frontend/src/modules/EChartsDemo/view/plotOrchestrator.ts | Builds chart options + synthetic traces for each demo plot type. |
| frontend/src/modules/EChartsDemo/view/persistence.ts | Adds view-state persistence schema + (de)serialization for zoom. |
| frontend/src/modules/EChartsDemo/view/atoms/baseAtoms.ts | Stores zoom state atom for the demo view. |
| frontend/src/modules/EChartsDemo/utils/syntheticData.ts | Generates synthetic traces/overlays for demo charts. |
| frontend/src/modules/EChartsDemo/typesAndEnums.ts | Declares demo plot types and UI labels. |
| frontend/src/modules/EChartsDemo/settings/settings.tsx | Implements demo settings UI for plot/data/layout toggles. |
| frontend/src/modules/EChartsDemo/settings/persistence.ts | Adds settings persistence schema + (de)serialization. |
| frontend/src/modules/EChartsDemo/settings/atoms/baseAtoms.ts | Stores demo settings atoms (plot type, counts, toggles). |
| frontend/src/modules/EChartsDemo/registerModule.ts | Registers the demo module with module registry. |
| frontend/src/modules/EChartsDemo/preview.tsx | Adds preview rendering for module picker. |
| frontend/src/modules/EChartsDemo/persistence.ts | Wires module-level settings/view persistence functions. |
| frontend/src/modules/EChartsDemo/loadModule.tsx | Initializes module components and binds view/settings FCs. |
| frontend/src/modules/EChartsDemo/interfaces.ts | Defines module interfaces and settings-to-view mapping. |
| frontend/package.json | Adds echarts and echarts-for-react dependencies (+ @types/echarts). |
| frontend/package-lock.json | Locks new dependencies and updates engine metadata. |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function computeKde(sorted: number[], numPoints: number): [number, number][] { | ||
| const n = sorted.length; | ||
| if (n === 0) return []; | ||
|
|
||
| const min = sorted[0]; | ||
| const max = sorted[n - 1]; | ||
| const range = max - min; | ||
|
|
||
| // O(n) mean + stdDev | ||
| const sum = sorted.reduce((a, b) => a + b, 0); | ||
| const mean = sum / n; | ||
| const variance = sorted.reduce((s, v) => s + (v - mean) ** 2, 0) / n; | ||
| const stdDev = Math.sqrt(variance); | ||
|
|
||
| const iqr = sorted[Math.floor(n * 0.75)] - sorted[Math.floor(n * 0.25)]; | ||
|
|
||
| // Silverman's rule of thumb; fall back to epsilon when all values are identical | ||
| const bandwidth = 0.9 * Math.min(stdDev, iqr / 1.34) * Math.pow(n, -0.2); | ||
| const h = bandwidth || 1e-4; | ||
|
|
||
| const pad = range * 0.1 || h * 3; | ||
| const step = (range + 2 * pad) / (numPoints - 1); | ||
| const result: [number, number][] = []; |
There was a problem hiding this comment.
step is computed as (range + 2 * pad) / (numPoints - 1). When numPoints is 1 (or 0), this produces Infinity/-Infinity and the loop will emit invalid coordinates. Please guard numPoints (e.g. return [[min, normalization]] for 1 point, or return [] for non-positive) before computing step.
| import React from "react"; | ||
|
|
||
| import type { ViewContext } from "@framework/ModuleContext"; | ||
| import type { ChartZoomState } from "@modules/_shared/eCharts/core/composeChartOption"; |
There was a problem hiding this comment.
This demo imports ChartZoomState via a deep internal path (@modules/_shared/eCharts/core/composeChartOption). Per the shared module guidance (AGENTS.md:9-10), consumers should rely on the root @modules/_shared/eCharts entrypoint for public types. Consider re-exporting ChartZoomState from @modules/_shared/eCharts (or from types.ts) so consumers don’t need internal imports.
| import type { ChartZoomState } from "@modules/_shared/eCharts/core/composeChartOption"; | |
| import type { ChartZoomState } from "@modules/_shared/eCharts"; |
|
|
||
| import type { DeserializeStateFunction, SerializeStateFunction } from "@framework/Module"; | ||
| import { setIfDefined } from "@framework/utils/atomUtils"; | ||
| import type { ChartZoomState } from "@modules/_shared/eCharts/core/composeChartOption"; |
There was a problem hiding this comment.
This file also depends on a deep internal type import (@modules/_shared/eCharts/core/composeChartOption). If ChartZoomState is intended for consumers (e.g. persistence schemas), it should be re-exported from the shared module root (@modules/_shared/eCharts) to avoid leaking internal paths and to follow the module’s import guidance (AGENTS.md:9-10).
| import type { ChartZoomState } from "@modules/_shared/eCharts/core/composeChartOption"; | |
| import type { ChartZoomState } from "@modules/_shared/eCharts"; |
| // Core | ||
| export { composeChartOption } from "./core"; | ||
| export type { ChartSeriesOption, ComposeChartConfig, SeriesBuildResult } from "./core"; | ||
| export { makeSeriesId, parseSeriesId } from "./core"; | ||
| export type { SeriesIdFields } from "./core"; | ||
|
|
||
| // Series Builders & Chart Options | ||
| export { buildBarSeries, type BarSortBy } from "./charts/bar"; | ||
| export { buildConvergenceSeries } from "./charts/convergence"; | ||
| export { buildDensitySeries, type DensityDisplayOptions } from "./charts/density"; | ||
| export { buildExceedanceSeries } from "./charts/exceedance"; | ||
| export { buildHeatmapSeries } from "./charts/heatmap"; | ||
| export { buildHistogramSeries, type HistogramDisplayOptions } from "./charts/histogram"; | ||
| export { buildMemberScatterSeries } from "./charts/memberScatter"; | ||
| export { buildPercentileRangeSeries, type PercentileRangeCenterStatistic, type PercentileRangeDisplayOptions } from "./charts/percentileRange"; | ||
| export { | ||
| buildFanchartSeries, | ||
| buildHistorySeries, | ||
| buildMemberSeries, | ||
| buildObservationSeries, | ||
| buildStatisticsSeries, | ||
| } from "./charts/timeseries"; | ||
|
|
||
| // Chart Composition Builders | ||
| export { | ||
| buildBarChart, | ||
| buildConvergenceChart, | ||
| buildDensityChart, | ||
| buildExceedanceChart, | ||
| buildHeatmapChart, | ||
| buildHistogramChart, | ||
| buildMemberScatterChart, | ||
| buildPercentileRangeChart, | ||
| buildTimeseriesChart, | ||
| extractTimeseriesCategoryData, | ||
| } from "./charts/"; | ||
|
|
||
| // Overlays | ||
| export { applyActiveTimestampMarker, createTimestampMarkLine } from "./overlays"; | ||
|
|
||
| // Hooks | ||
| export { useClickToTimestamp, useMemberInteraction } from "./hooks"; | ||
| export type { HoveredMemberInfo, MemberInteractionEvents, MemberInteractionOptions } from "./hooks"; | ||
|
|
There was a problem hiding this comment.
The demo (and likely other consumers) currently need deep imports for ChartZoomState and useEChartsViewState. If these are intended as public APIs, please re-export them from this root entrypoint (or from types.ts / hooks/index.ts) so consumers can follow the module’s import guidance (AGENTS.md:9-10) and avoid coupling to internal paths.
| if (histogramType === HistogramType.Stack || histogramType === HistogramType.Relative) { | ||
| const yStart = cumulativeY; | ||
| const yEnd = cumulativeY + percentage; | ||
| barsByTrace[traceIndex].push({ | ||
| xStart: referenceBin.start, | ||
| xEnd: referenceBin.end, | ||
| yStart, | ||
| yEnd, | ||
| count: bin.count, | ||
| percentage, | ||
| }); | ||
| cumulativeY = yEnd; | ||
| yMax = Math.max(yMax, yEnd); | ||
| continue; |
There was a problem hiding this comment.
HistogramType.Relative is currently handled identically to Stack (it accumulates each trace’s already-normalized percentages), so stacked bars can exceed 100% and the "Relative" mode doesn’t provide per-bin relative composition. If "Relative" is meant to show relative contributions within each bin, the layout needs to normalize counts by the total count across traces for that bin before stacking (or remove Relative if it’s not supported).
| import { computeSubplotGridLayout, useClickToTimestamp, useMemberInteraction } from "@modules/_shared/eCharts"; | ||
| import { useEChartsViewState } from "@modules/_shared/eCharts/hooks/useEchartsViewState"; |
There was a problem hiding this comment.
Consumer code is using a deep internal import (@modules/_shared/eCharts/hooks/useEchartsViewState). The shared module guidance says consumers should prefer @modules/_shared/eCharts for public symbols and only use deep imports when intentionally not part of the public API (AGENTS.md:9-10). Either export useEChartsViewState from the shared module root entrypoint or move this hook into the demo module if it’s meant to be demo-only.
| import { computeSubplotGridLayout, useClickToTimestamp, useMemberInteraction } from "@modules/_shared/eCharts"; | |
| import { useEChartsViewState } from "@modules/_shared/eCharts/hooks/useEchartsViewState"; | |
| import { computeSubplotGridLayout, useClickToTimestamp, useMemberInteraction, useEChartsViewState } from "@modules/_shared/eCharts"; |
| fontSize: 11, | ||
| } | ||
| : undefined, | ||
| encode: { x: 0, y: 1 }, |
There was a problem hiding this comment.
The bar series data is a plain number[], but encode is set to { x: 0, y: 1 }. With 1D data there is no dimension 1, and forcing an encode mapping can lead to incorrect dimension binding in ECharts (especially if you later switch to datasets). Consider removing encode entirely here, or set it consistently with the data shape (e.g. y: 0).
| encode: { x: 0, y: 1 }, |
| import { useCallback, useRef } from "react"; | ||
|
|
||
| import type { ChartZoomState } from "../core/composeChartOption"; | ||
|
|
||
|
|
||
| export function useEChartsViewState( | ||
| setZoomState: React.Dispatch<React.SetStateAction<ChartZoomState>> | ||
| ) { | ||
| const debounceTimer = useRef<NodeJS.Timeout | null>(null); | ||
|
|
There was a problem hiding this comment.
This hook references React.Dispatch/React.SetStateAction but only imports named hooks from react, so React is undefined here and will fail type-checking. Import type React from "react" (or import Dispatch/SetStateAction types directly) and consider typing the timeout as ReturnType<typeof setTimeout> instead of NodeJS.Timeout for browser compatibility.
| const handleDataZoom = useCallback((params: any) => { | ||
| const updates = params.batch ?? [params]; | ||
|
|
||
| if (debounceTimer.current) clearTimeout(debounceTimer.current); | ||
|
|
||
| debounceTimer.current = setTimeout(() => { | ||
| setZoomState((prev) => { | ||
| const next = { ...prev }; | ||
| updates.forEach((item: any) => { | ||
| const axisKey = item.dataZoomId === "y" ? "y" : "x"; | ||
| next[axisKey] = { |
There was a problem hiding this comment.
This introduces any for params and for items in updates. The shared eCharts module's agent guidance explicitly says to avoid any (AGENTS.md:12). Please replace any with unknown and narrow/validate the shape you read (batch, dataZoomId, start/end, etc.), or define a minimal DataZoomEvent type that matches the ECharts event payload you rely on.
| import { buildExceedanceChart } from "@modules/_shared/eCharts/charts//exceedance"; | ||
| import { buildHistogramChart } from "@modules/_shared/eCharts/charts//histogram"; | ||
| import { buildPercentileRangeChart } from "@modules/_shared/eCharts/charts//percentileRange"; |
There was a problem hiding this comment.
These import specifiers contain a double slash (charts//...). While many resolvers normalize this, it's easy to break path aliasing and makes the imports harder to grep. Please change to @modules/_shared/eCharts/charts/exceedance (single /) consistently.
1969b81 to
2d8b91e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 121 out of 122 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mean[t] = col.length > 0 ? sum / col.length : 0; | ||
| const minMax = MinMax.fromNumericValues(col); | ||
| min[t] = minMax.min; | ||
| max[t] = minMax.max; | ||
| p10[t] = computeReservesP10(col); |
There was a problem hiding this comment.
In computeTimeseriesStatistics, col can be empty for a timestep where all member values are non-finite. MinMax.fromNumericValues([]) returns an invalid range (min=+∞, max=-∞), so this writes infinities into the returned min/max arrays. Consider guarding on col.length === 0 (set min/max to 0 or keep previous), or using minMax.isValid() before assigning.
| function computeHistogramBins(values: number[], binEdges: number[]): HistogramBin[] { | ||
| const counts = new Array(Math.max(binEdges.length - 1, 0)).fill(0); | ||
| const min = binEdges[0] ?? 0; | ||
| const binSize = (binEdges[1] ?? min + 1) - min || 1; | ||
|
|
||
| for (const value of values) { | ||
| const index = Math.min(Math.floor((value - min) / binSize), counts.length - 1); | ||
| counts[Math.max(index, 0)]++; | ||
| } |
There was a problem hiding this comment.
computeHistogramBins iterates over raw values without filtering non-finite numbers. If values contains NaN/Infinity, the computed bin index becomes NaN/out of range and the counts[...]++ write will corrupt the array (e.g. creates a counts["NaN"] property) and percentages will be wrong. Suggest filtering to finite values first (and use the finite count as the percentage denominator).
| for (let r = 0; r < numMembers; r++) { | ||
| const vals = memberValues[r]; | ||
| let min = Infinity; | ||
| let max = -Infinity; | ||
| for (let t = 0; t < numTimesteps; t++) { | ||
| const v = vals[t]; | ||
| if (v < min) min = v; | ||
| if (v > max) max = v; | ||
| } | ||
| data.push([r, min, max]); | ||
| } |
There was a problem hiding this comment.
buildMemberSeriesLarge computes per-member min/max without skipping non-finite values. If a member series contains NaN/Infinity, min/max can remain ±Infinity and get written into data, which can break axis extent calculation. Consider checking Number.isFinite(v) before updating min/max, and fall back to 0 (or skip the member) when no finite values exist.
| export function computeKde(values: number[], numPoints: number): [number, number][] { | ||
| const sorted = [...values].sort((a, b) => a - b); | ||
| const n = sorted.length; | ||
| if (n === 0) return []; | ||
|
|
||
| const min = sorted[0]; | ||
| const max = sorted[n - 1]; | ||
| const range = max - min; | ||
|
|
||
| // O(n) mean + stdDev | ||
| const sum = sorted.reduce((a, b) => a + b, 0); | ||
| const mean = sum / n; | ||
| const variance = sorted.reduce((s, v) => s + (v - mean) ** 2, 0) / n; | ||
| const stdDev = Math.sqrt(variance); | ||
|
|
||
| const iqr = sorted[Math.floor(n * 0.75)] - sorted[Math.floor(n * 0.25)]; | ||
|
|
||
| // Silverman's rule of thumb; fall back to epsilon when all values are identical | ||
| const bandwidth = 0.9 * Math.min(stdDev, iqr / 1.34) * Math.pow(n, -0.2); | ||
| const h = bandwidth || 1e-4; | ||
|
|
||
| const pad = range * 0.1 || h * 3; | ||
| const step = (range + 2 * pad) / (numPoints - 1); | ||
| const result: [number, number][] = []; |
There was a problem hiding this comment.
computeKde assumes numPoints >= 2 (it computes step = ... / (numPoints - 1)). If numPoints is 0 or 1, this produces Infinity/NaN x-coordinates (since 0 * Infinity becomes NaN). Consider validating numPoints up front (e.g. return [] for numPoints <= 0, and a single [mean, density]/[min, ...] point or [] for numPoints === 1).
| const categoryOrder = computeCategoryOrder(group.traces, options.sortBy ?? "categories"); | ||
| const optionsWithOrder: BuildBarSeriesOptions = { ...options, categoryOrder }; | ||
| const series = []; | ||
| const legendData: string[] = []; | ||
| const seenLegend = new Set<string>(); | ||
|
|
||
| for (const trace of group.traces) { | ||
| const result = buildBarSeries(trace, axisIndex, optionsWithOrder); | ||
| series.push(...result.series); |
There was a problem hiding this comment.
buildBarSubplot initializes const series = []; without a type annotation, which makes it any[] and loses type-safety when pushing ECharts series objects. Suggest typing it (e.g. ChartSeriesOption[] or CartesianSubplotBuildResult["series"]) so incorrect series shapes are caught by TS.
Introduces a shared ECharts charting library and an EChartsDemo module, with typed chart builders for timeseries, histogram, density, percentile range, exceedance, convergence, bar, heatmap, and member scatter. The branch adds shared subplot/layout composition, tooltip infrastructure, zoom and timestamp-selection hooks, cross-subplot interaction support, and focused unit coverage for the new chart layer.