From 42c3a69d0131c9aa011feedf35b2f9e7df6a5a3b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 27 Mar 2026 22:14:23 +0000 Subject: [PATCH 1/3] feat: add 'Other' grouping and hover percentages for pie/donut charts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add smart 'Other' grouping algorithm (target ≤20% of total, max 10 slices) - Support explicit limit via YAML config and showOther toggle - Add percentage display in tooltips (1 decimal place) - Add mini-leaderboard tooltip for 'Other' slice (top 5 + N more) - Muted color for 'Other' slice (light/dark mode aware) - Add data transformer pipeline for circular chart data - Custom tooltip formatter for pie/donut charts Closes APP-818 Co-authored-by: ericokuma --- .../canvas/components/charts/BaseChart.ts | 14 +- .../components/charts/CanvasChart.svelte | 1 + .../canvas/components/charts/selector.ts | 1 + .../charts/variants/CircularChart.ts | 12 ++ .../features/components/charts/Chart.svelte | 79 +++++++++- .../charts/circular/CircularChartProvider.ts | 57 ++++++- .../charts/circular/other-grouping.ts | 148 ++++++++++++++++++ .../components/charts/circular/pie-tooltip.ts | 116 ++++++++++++++ .../components/charts/circular/pie.ts | 110 +++++++++++-- .../components/charts/data-provider.ts | 10 ++ 10 files changed, 530 insertions(+), 18 deletions(-) create mode 100644 web-common/src/features/components/charts/circular/other-grouping.ts create mode 100644 web-common/src/features/components/charts/circular/pie-tooltip.ts diff --git a/web-common/src/features/canvas/components/charts/BaseChart.ts b/web-common/src/features/canvas/components/charts/BaseChart.ts index e1756ade86a..8859bf4be63 100644 --- a/web-common/src/features/canvas/components/charts/BaseChart.ts +++ b/web-common/src/features/canvas/components/charts/BaseChart.ts @@ -26,6 +26,8 @@ import type { V1Resource, } from "@rilldata/web-common/runtime-client"; import { get, writable, type Readable, type Writable } from "svelte/store"; +import type { V1MetricsViewAggregationResponseDataItem } from "@rilldata/web-common/runtime-client"; +import type { OtherGroupResult } from "../../../components/charts/circular/other-grouping"; import type { ChartDataQuery, ChartDomainValues, @@ -104,11 +106,19 @@ export abstract class BaseChart< abstract chartTitle(fields: ChartFieldsMap): string; getChartDomainValues(): ChartDomainValues { - // Default implementation returns empty metadata - // Subclasses can override to provide specific metadata return {}; } + getDataTransformer(): (( + data: V1MetricsViewAggregationResponseDataItem[], + ) => V1MetricsViewAggregationResponseDataItem[]) | undefined { + return undefined; + } + + getOtherGroupResult(): OtherGroupResult | undefined { + return undefined; + } + protected getDefaultFieldConfig(): Partial { return { showAxisTitle: true, diff --git a/web-common/src/features/canvas/components/charts/CanvasChart.svelte b/web-common/src/features/canvas/components/charts/CanvasChart.svelte index 71b47806d2d..2d751031423 100644 --- a/web-common/src/features/canvas/components/charts/CanvasChart.svelte +++ b/web-common/src/features/canvas/components/charts/CanvasChart.svelte @@ -108,6 +108,7 @@ bind:view={viewVL} themeMode={$isThemeModeDark ? "dark" : "light"} theme={currentTheme} + otherGroupResult={component.getOtherGroupResult()} /> {/if} {:else} diff --git a/web-common/src/features/canvas/components/charts/selector.ts b/web-common/src/features/canvas/components/charts/selector.ts index e68b2fe426a..8201c65d403 100644 --- a/web-common/src/features/canvas/components/charts/selector.ts +++ b/web-common/src/features/canvas/components/charts/selector.ts @@ -32,5 +32,6 @@ export function getChartDataForCanvas( themeStore: ctx.canvasEntity.theme, timeAndFilterStore, themeModeStore, + dataTransformer: component.getDataTransformer(), }); } diff --git a/web-common/src/features/canvas/components/charts/variants/CircularChart.ts b/web-common/src/features/canvas/components/charts/variants/CircularChart.ts index e46ec7c41c6..1b312a96ad3 100644 --- a/web-common/src/features/canvas/components/charts/variants/CircularChart.ts +++ b/web-common/src/features/canvas/components/charts/variants/CircularChart.ts @@ -4,6 +4,7 @@ import { CircularChartProvider, type CircularChartSpec as CircularChartSpecBase, } from "@rilldata/web-common/features/components/charts/circular/CircularChartProvider"; +import type { OtherGroupResult } from "@rilldata/web-common/features/components/charts/circular/other-grouping"; import { ChartSortType, type ChartFieldsMap, @@ -11,6 +12,7 @@ import { import type { TimeAndFilterStore } from "@rilldata/web-common/features/dashboards/time-controls/time-control-store"; import { MetricsViewSpecDimensionType, + type V1MetricsViewAggregationResponseDataItem, type V1MetricsViewSpec, type V1Resource, } from "@rilldata/web-common/runtime-client"; @@ -112,6 +114,16 @@ export class CircularChartComponent extends BaseChart { return this.provider.getChartDomainValues(); } + getDataTransformer(): (( + data: V1MetricsViewAggregationResponseDataItem[], + ) => V1MetricsViewAggregationResponseDataItem[]) | undefined { + return (data) => this.provider.transformData(data); + } + + getOtherGroupResult(): OtherGroupResult | undefined { + return this.provider.otherGroupResult; + } + chartTitle(fields: ChartFieldsMap) { return this.provider.chartTitle(fields); } diff --git a/web-common/src/features/components/charts/Chart.svelte b/web-common/src/features/components/charts/Chart.svelte index faff8305ab9..ca697d25b73 100644 --- a/web-common/src/features/components/charts/Chart.svelte +++ b/web-common/src/features/components/charts/Chart.svelte @@ -28,7 +28,18 @@ compileToBrushedVegaSpec, createAdaptiveScrubHandler, } from "./brush-builder"; - import type { ChartDataResult, ChartType } from "./types"; + import type { VLTooltipFormatter } from "@rilldata/web-common/components/vega/types"; + import { + OTHER_SLICE_COLOR_DARK, + OTHER_SLICE_COLOR_LIGHT, + OTHER_SLICE_LABEL, + } from "./circular/other-grouping"; + import type { OtherGroupResult } from "./circular/other-grouping"; + import { + createPieTooltipFormatter, + type PieTooltipConfig, + } from "./circular/pie-tooltip"; + import type { ChartDataResult, ChartType, ColorMapping } from "./types"; import { generateSpec, getColorMappingForChart } from "./util"; export let chartType: ChartType; @@ -42,6 +53,7 @@ */ export let theme: Record | undefined = undefined; export let isCanvas: boolean; + export let otherGroupResult: OtherGroupResult | undefined = undefined; export let isScrubbing: boolean = false; export let temporalField: string | undefined = undefined; @@ -130,12 +142,72 @@ // Color mapping needs to be reactive to theme mode changes (light/dark) // because colors are resolved differently for each mode $: isThemeModeDark = themeMode === "dark"; - $: colorMapping = getColorMappingForChart( + $: colorMapping = applyOtherColor( + getColorMappingForChart(chartSpec, domainValues, isThemeModeDark), + isThemeModeDark, + ); + + $: isCircularChart = chartType === "donut_chart" || chartType === "pie_chart"; + + $: pieTooltipFormatter = buildPieTooltipFormatter( + isCircularChart, chartSpec, - domainValues, + otherGroupResult, + colorMapping, isThemeModeDark, + measureFormatters, + chartDataWithTheme, ); + function buildPieTooltipFormatter( + isCircular: boolean, + spec: CanvasChartSpec, + otherResult: OtherGroupResult | undefined, + mapping: ColorMapping | undefined, + isDark: boolean, + formatters: Record string>, + chartDataResult: ChartDataResult, + ): VLTooltipFormatter | undefined { + if (!isCircular || !("measure" in spec) || !("color" in spec)) return undefined; + const measureField = (spec.measure as { field?: string })?.field; + const colorField = (spec.color as { field?: string })?.field; + if (!measureField || !colorField) return undefined; + + const grandTotal = otherResult?.total ?? + (chartDataResult.domainValues?.["__otherTotal"]?.[0] as number | undefined) ?? 0; + if (grandTotal <= 0) return undefined; + + const measureMeta = chartDataResult.fields[measureField]; + const colorMeta = chartDataResult.fields[colorField]; + const formatter = formatters[sanitizeFieldName(measureField)]; + + const cfg: PieTooltipConfig = { + colorField, + measureField, + colorFieldLabel: colorMeta?.displayName || colorField, + measureFieldLabel: measureMeta?.displayName || measureField, + otherItems: otherResult?.otherItems ?? [], + grandTotal, + colorMapping: mapping ?? [], + isDarkMode: isDark, + formatValue: (val) => (formatter ? formatter(val) : String(val)), + }; + + return createPieTooltipFormatter(cfg); + } + + function applyOtherColor( + mapping: ColorMapping | undefined, + isDark: boolean, + ): ColorMapping | undefined { + if (!mapping) return mapping; + return mapping.map((m) => + m.value === OTHER_SLICE_LABEL + ? { ...m, color: isDark ? OTHER_SLICE_COLOR_DARK : OTHER_SLICE_COLOR_LIGHT } + : m, + ); + } + const scrubHandler = createAdaptiveScrubHandler((interval) => onBrush?.(interval), ); @@ -227,6 +299,7 @@ renderer="canvas" {expressionFunctions} {hasComparison} + tooltipFormatter={pieTooltipFormatter} config={getRillTheme(isThemeModeDark, theme)} /> {/if} diff --git a/web-common/src/features/components/charts/circular/CircularChartProvider.ts b/web-common/src/features/components/charts/circular/CircularChartProvider.ts index d7e0ab84453..f01fbc2a2f5 100644 --- a/web-common/src/features/components/charts/circular/CircularChartProvider.ts +++ b/web-common/src/features/components/charts/circular/CircularChartProvider.ts @@ -14,6 +14,7 @@ import type { V1Expression, V1MetricsViewAggregationDimension, V1MetricsViewAggregationMeasure, + V1MetricsViewAggregationResponseDataItem, V1MetricsViewAggregationSort, } from "@rilldata/web-common/runtime-client"; import { getQueryServiceMetricsViewAggregationQueryOptions } from "@rilldata/web-common/runtime-client"; @@ -28,12 +29,18 @@ import { type Writable, } from "svelte/store"; import { getFilterWithNullHandling } from "../query-util"; +import { + computeOtherGrouping, + OTHER_SLICE_LABEL, + type OtherGroupResult, +} from "./other-grouping"; export type CircularChartSpec = { metrics_view: string; measure?: FieldConfig<"quantitative">; color?: FieldConfig<"nominal">; innerRadius?: number; + showOther?: boolean; }; export type CircularChartDefaultOptions = { @@ -51,6 +58,7 @@ export class CircularChartProvider { customColorValues: string[] = []; totalsValue: number | undefined = undefined; + otherGroupResult: OtherGroupResult | undefined = undefined; combinedWhere: Writable = writable(undefined); @@ -127,13 +135,16 @@ export class CircularChartProvider { const topNColorQuery = createQuery(topNColorQueryOptionsStore); + const showOther = config.showOther !== false; + const needsTotal = showTotal || showOther; + const totalQueryOptionsStore = derived( [timeAndFilterStore, visibleStore], ([$timeAndFilterStore, $visible]) => { const { timeRange, where, hasTimeSeries } = $timeAndFilterStore; const enabled = $visible && - !!showTotal && + !!needsTotal && (!hasTimeSeries || (!!timeRange?.start && !!timeRange?.end)) && !!config.measure?.field; @@ -265,6 +276,46 @@ export class CircularChartProvider { }; } + /** + * Transforms raw query data to apply "Other" grouping for pie/donut charts. + * Called by the data provider pipeline before data reaches the chart spec. + */ + transformData( + data: V1MetricsViewAggregationResponseDataItem[], + ): V1MetricsViewAggregationResponseDataItem[] { + const config = get(this.spec); + const measureField = config.measure?.field; + const colorField = config.color?.field; + + if (!measureField || !colorField) { + this.otherGroupResult = undefined; + return data; + } + + const showOther = config.showOther !== false; + const explicitLimit = config.color?.limit; + + const result = computeOtherGrouping(data, measureField, colorField, { + limit: explicitLimit, + showOther, + }); + + this.otherGroupResult = result; + + if (result.hasOther) { + this.customColorValues = result.visibleData + .map((d) => String(d[colorField] ?? "")) + .filter((v) => v !== OTHER_SLICE_LABEL); + this.customColorValues.push(OTHER_SLICE_LABEL); + } + + if (this.totalsValue === undefined && result.total > 0) { + this.totalsValue = result.total; + } + + return result.visibleData; + } + getChartDomainValues(): ChartDomainValues { const config = get(this.spec); const result: Record = {}; @@ -280,6 +331,10 @@ export class CircularChartProvider { result["total"] = [this.totalsValue]; } + if (this.otherGroupResult) { + result["__otherTotal"] = [this.otherGroupResult.total]; + } + return result; } diff --git a/web-common/src/features/components/charts/circular/other-grouping.ts b/web-common/src/features/components/charts/circular/other-grouping.ts new file mode 100644 index 00000000000..e432d514f8e --- /dev/null +++ b/web-common/src/features/components/charts/circular/other-grouping.ts @@ -0,0 +1,148 @@ +import type { V1MetricsViewAggregationResponseDataItem } from "@rilldata/web-common/runtime-client"; + +export const OTHER_SLICE_LABEL = "Other"; +export const OTHER_SLICE_COLOR_LIGHT = "#d1d5db"; +export const OTHER_SLICE_COLOR_DARK = "#4b5563"; + +const MAX_VISIBLE_SLICES = 10; +const OTHER_TARGET_PERCENT = 0.2; +const OTHER_TOOLTIP_TOP_N = 5; + +export interface OtherGroupResult { + visibleData: V1MetricsViewAggregationResponseDataItem[]; + otherItems: V1MetricsViewAggregationResponseDataItem[]; + total: number; + hasOther: boolean; +} + +/** + * Determines how many slices to show individually vs. group into "Other". + * + * Algorithm: + * - Data is assumed pre-sorted descending by measure. + * - Walk from the largest slice downward, accumulating their share. + * - Stop adding individual slices when: + * a) we've reached MAX_VISIBLE_SLICES, OR + * b) the remaining slices account for ≤ OTHER_TARGET_PERCENT of total + * and we already have at least 2 visible slices. + * - If only 0-1 items would be grouped, don't create "Other" at all. + */ +export function computeOtherGrouping( + data: V1MetricsViewAggregationResponseDataItem[], + measureField: string, + colorField: string, + options: { limit?: number; showOther?: boolean }, +): OtherGroupResult { + const total = data.reduce( + (sum, d) => sum + (Number(d[measureField]) || 0), + 0, + ); + + if (options.showOther === false || data.length <= 1) { + return { visibleData: data, otherItems: [], total, hasOther: false }; + } + + const sorted = [...data].sort( + (a, b) => (Number(b[measureField]) || 0) - (Number(a[measureField]) || 0), + ); + + let cutoff: number; + + if (options.limit !== undefined) { + cutoff = Math.max(1, Math.min(options.limit, sorted.length)); + } else { + cutoff = computeDynamicCutoff(sorted, measureField, total); + } + + if (cutoff >= sorted.length || sorted.length - cutoff <= 1) { + return { visibleData: sorted, otherItems: [], total, hasOther: false }; + } + + const visible = sorted.slice(0, cutoff); + const others = sorted.slice(cutoff); + + const otherValue = others.reduce( + (sum, d) => sum + (Number(d[measureField]) || 0), + 0, + ); + + const otherRow: V1MetricsViewAggregationResponseDataItem = { + [colorField]: OTHER_SLICE_LABEL, + [measureField]: otherValue, + }; + + return { + visibleData: [...visible, otherRow], + otherItems: others, + total, + hasOther: true, + }; +} + +function computeDynamicCutoff( + sorted: V1MetricsViewAggregationResponseDataItem[], + measureField: string, + total: number, +): number { + if (total === 0) return sorted.length; + + let accumulated = 0; + + for (let i = 0; i < sorted.length && i < MAX_VISIBLE_SLICES; i++) { + accumulated += Number(sorted[i][measureField]) || 0; + const remaining = total - accumulated; + const remainingPercent = remaining / total; + + if (remainingPercent <= OTHER_TARGET_PERCENT && i >= 1) { + return i + 1; + } + } + + return Math.min(sorted.length, MAX_VISIBLE_SLICES); +} + +export interface OtherTooltipItem { + name: string; + value: number; + percent: number; +} + +export interface OtherTooltipData { + items: OtherTooltipItem[]; + remainingCount: number; + totalValue: number; + totalPercent: number; +} + +export function getOtherTooltipData( + otherItems: V1MetricsViewAggregationResponseDataItem[], + measureField: string, + colorField: string, + grandTotal: number, +): OtherTooltipData { + const sorted = [...otherItems].sort( + (a, b) => (Number(b[measureField]) || 0) - (Number(a[measureField]) || 0), + ); + + const topItems = sorted.slice(0, OTHER_TOOLTIP_TOP_N); + const remaining = sorted.length - topItems.length; + + const totalValue = sorted.reduce( + (sum, d) => sum + (Number(d[measureField]) || 0), + 0, + ); + + return { + items: topItems.map((d) => { + const val = Number(d[measureField]) || 0; + return { + name: String(d[colorField] ?? ""), + value: val, + percent: grandTotal > 0 ? (val / grandTotal) * 100 : 0, + }; + }), + remainingCount: remaining, + totalValue, + totalPercent: grandTotal > 0 ? (totalValue / grandTotal) * 100 : 0, + }; +} diff --git a/web-common/src/features/components/charts/circular/pie-tooltip.ts b/web-common/src/features/components/charts/circular/pie-tooltip.ts new file mode 100644 index 00000000000..acdd7d71036 --- /dev/null +++ b/web-common/src/features/components/charts/circular/pie-tooltip.ts @@ -0,0 +1,116 @@ +import type { VLTooltipFormatter } from "@rilldata/web-common/components/vega/types"; +import type { V1MetricsViewAggregationResponseDataItem } from "@rilldata/web-common/runtime-client"; +import type { ColorMapping } from "../types"; +import { + OTHER_SLICE_COLOR_DARK, + OTHER_SLICE_COLOR_LIGHT, + OTHER_SLICE_LABEL, + getOtherTooltipData, +} from "./other-grouping"; + +export interface PieTooltipConfig { + colorField: string; + measureField: string; + colorFieldLabel: string; + measureFieldLabel: string; + otherItems: V1MetricsViewAggregationResponseDataItem[]; + grandTotal: number; + colorMapping: ColorMapping; + isDarkMode: boolean; + formatValue: (val: number) => string; +} + +function escapeHtml(str: string): string { + return str + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """); +} + +export function createPieTooltipFormatter( + config: PieTooltipConfig, +): VLTooltipFormatter { + const colorMap = new Map( + config.colorMapping.map((m) => [m.value, m.color]), + ); + + return (value: unknown): string => { + if (!value || typeof value !== "object") return ""; + + const record = value as Record; + const sliceName = String(record[config.colorField] ?? ""); + const sliceValue = Number(record[config.measureField] ?? 0); + const percentage = Number(record["__percentage"] ?? 0); + + if (sliceName === OTHER_SLICE_LABEL) { + return buildOtherTooltip(config, sliceValue); + } + + return buildNamedSliceTooltip( + sliceName, + sliceValue, + percentage, + colorMap.get(sliceName), + config, + ); + }; +} + +function buildNamedSliceTooltip( + name: string, + value: number, + percentage: number, + color: string | undefined, + config: PieTooltipConfig, +): string { + const formattedValue = config.formatValue(value); + const pctStr = percentage.toFixed(1) + "%"; + const colorDot = color + ? `` + : ""; + + return ` + +
${colorDot}${escapeHtml(name)}${escapeHtml(formattedValue)}${pctStr}
`; +} + +function buildOtherTooltip( + config: PieTooltipConfig, + otherValue: number, +): string { + const data = getOtherTooltipData( + config.otherItems, + config.measureField, + config.colorField, + config.grandTotal, + ); + + const otherColor = config.isDarkMode + ? OTHER_SLICE_COLOR_DARK + : OTHER_SLICE_COLOR_LIGHT; + const otherPct = data.totalPercent.toFixed(1) + "%"; + const formattedOtherValue = config.formatValue(otherValue); + + const rows: string[] = []; + + rows.push( + `${escapeHtml(OTHER_SLICE_LABEL)} — ${escapeHtml(formattedOtherValue)} (${otherPct})`, + ); + + for (const item of data.items) { + const fmtVal = config.formatValue(item.value); + const pct = item.percent.toFixed(1) + "%"; + rows.push( + `${escapeHtml(item.name)}${escapeHtml(fmtVal)}${pct}`, + ); + } + + if (data.remainingCount > 0) { + rows.push( + `and ${data.remainingCount} more`, + ); + } + + return `${rows.join("")}
`; +} diff --git a/web-common/src/features/components/charts/circular/pie.ts b/web-common/src/features/components/charts/circular/pie.ts index a92bc788542..2f7df83e909 100644 --- a/web-common/src/features/components/charts/circular/pie.ts +++ b/web-common/src/features/components/charts/circular/pie.ts @@ -14,8 +14,13 @@ import { createPositionEncoding, createSingleLayerBaseSpec, } from "../builder"; -import type { ChartDataResult } from "../types"; +import type { ChartDataResult, TooltipValue } from "../types"; import type { CircularChartSpec } from "./CircularChartProvider"; +import { + OTHER_SLICE_COLOR_DARK, + OTHER_SLICE_COLOR_LIGHT, + OTHER_SLICE_LABEL, +} from "./other-grouping"; function getInnerRadius(innerRadiusPercentage: number | undefined) { if (!innerRadiusPercentage) return 0; @@ -42,6 +47,43 @@ function getTotalFontSize(innerRadiusPercentage: number | undefined) { return { expr: `max(11, min(32, ${decimal}*min(width,height)/4))` }; } +function createPieTooltipEncoding( + config: CircularChartSpec, + data: ChartDataResult, +): TooltipValue[] { + const tooltip: TooltipValue[] = []; + + if (config.color) { + const colorMeta = data.fields[config.color.field]; + tooltip.push({ + field: config.color.field, + title: colorMeta?.displayName || config.color.field, + type: config.color.type as "nominal", + }); + } + + if (config.measure) { + const measureMeta = data.fields[config.measure.field]; + tooltip.push({ + field: config.measure.field, + title: measureMeta?.displayName || config.measure.field, + type: "quantitative", + ...(config.measure.type === "quantitative" && { + formatType: sanitizeFieldName(config.measure.field), + }), + }); + } + + tooltip.push({ + field: "__percentage", + title: "% of Total", + type: "quantitative", + format: ".1f", + }); + + return tooltip; +} + export function generateVLPieChartSpec( config: CircularChartSpec, data: ChartDataResult, @@ -76,17 +118,58 @@ export function generateVLPieChartSpec( const color = createColorEncoding(config.color, data); const order = createOrderEncoding(config.measure); - const tooltip = createDefaultTooltipEncoding( - [config.color, config.measure], - data, - ); + const hasOther = + config.showOther !== false && + data.data?.some( + (d) => + config.color?.field && + d[config.color.field] === OTHER_SLICE_LABEL, + ); + + if (hasOther) { + const colorAny = color as Record; + const scale = colorAny.scale as + | { domain?: string[]; range?: string[] } + | undefined; + if (scale?.domain && scale?.range) { + const otherIdx = scale.domain.indexOf(OTHER_SLICE_LABEL); + if (otherIdx >= 0) { + scale.range[otherIdx] = data.isDarkMode + ? OTHER_SLICE_COLOR_DARK + : OTHER_SLICE_COLOR_LIGHT; + } + } + } - const arcLayer: LayerSpec | UnitSpec = { - mark: { - type: "arc", - padAngle: 0.01, - innerRadius: getInnerRadius(config.innerRadius), - }, + const grandTotal = data.domainValues?.["__otherTotal"]?.[0] as + | number + | undefined; + const hasPercentage = !!config.measure?.field && (grandTotal ?? 0) > 0; + + const transforms = hasPercentage + ? [ + { + calculate: `datum['${config.measure!.field}'] / ${grandTotal} * 100`, + as: "__percentage", + }, + ] + : []; + + const tooltip = hasPercentage + ? createPieTooltipEncoding(config, data) + : createDefaultTooltipEncoding([config.color, config.measure], data); + + const arcMark = { + type: "arc" as const, + padAngle: 0.01, + innerRadius: getInnerRadius(config.innerRadius), + }; + + const arcLayer: UnitSpec = { + ...(transforms.length > 0 + ? { transform: transforms as UnitSpec["transform"] } + : {}), + mark: arcMark, encoding: { theta, color, @@ -134,8 +217,11 @@ export function generateVLPieChartSpec( }; } else { const spec = createSingleLayerBaseSpec("arc"); - spec.mark = arcLayer.mark; + spec.mark = arcMark; spec.encoding = arcLayer.encoding; + if (transforms.length > 0) { + (spec as unknown as Record).transform = transforms; + } return { ...spec, diff --git a/web-common/src/features/components/charts/data-provider.ts b/web-common/src/features/components/charts/data-provider.ts index 1f7f8dde0c1..4dc6bf62c76 100644 --- a/web-common/src/features/components/charts/data-provider.ts +++ b/web-common/src/features/components/charts/data-provider.ts @@ -5,6 +5,7 @@ import { TIME_GRAIN } from "@rilldata/web-common/lib/time/config"; import { type MetricsViewSpecDimension, type MetricsViewSpecMeasure, + type V1MetricsViewAggregationResponseDataItem, type V1MetricsViewSpec, } from "@rilldata/web-common/runtime-client"; import { derived, type Readable } from "svelte/store"; @@ -31,6 +32,10 @@ export interface ChartDataDependencies { /** Static theme mode flag - used in standalone chart context */ isThemeModeDark?: boolean; getDomainValues: () => ChartDomainValues; + /** Optional transformer applied to query data before it reaches the chart */ + dataTransformer?: ( + data: V1MetricsViewAggregationResponseDataItem[], + ) => V1MetricsViewAggregationResponseDataItem[]; } /** @@ -49,6 +54,7 @@ export function getChartData( timeAndFilterStore, themeModeStore, isThemeModeDark: staticThemeModeDark, + dataTransformer, } = deps; const { measures, dimensions, timeDimensions } = getFieldsByType(config); @@ -113,6 +119,10 @@ export function getChartData( ); } + if (dataTransformer && data) { + data = dataTransformer(data); + } + const domainValues = getDomainValues(); const hasComparison = $timeAndFilterStore.showTimeComparison; From 3c2b0ee7879dfa56d17314a18790f328d038af27 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 27 Mar 2026 22:21:46 +0000 Subject: [PATCH 2/3] fix: handle grandTotal for accurate percentages, add unit tests - Use grandTotal from total query for Other value calculation - Fetch more data from API when showOther is enabled with low limit - Distinguish between query limit and user's explicit limit - Add 13 unit tests for other-grouping algorithm - All 2419 existing tests continue to pass Co-authored-by: ericokuma --- .../charts/circular/CircularChartProvider.ts | 24 ++- .../charts/circular/other-grouping.spec.ts | 198 ++++++++++++++++++ .../charts/circular/other-grouping.ts | 10 +- 3 files changed, 222 insertions(+), 10 deletions(-) create mode 100644 web-common/src/features/components/charts/circular/other-grouping.spec.ts diff --git a/web-common/src/features/components/charts/circular/CircularChartProvider.ts b/web-common/src/features/components/charts/circular/CircularChartProvider.ts index f01fbc2a2f5..d02fdcdbc3e 100644 --- a/web-common/src/features/components/charts/circular/CircularChartProvider.ts +++ b/web-common/src/features/components/charts/circular/CircularChartProvider.ts @@ -89,12 +89,21 @@ export class CircularChartProvider { } let colorSort: V1MetricsViewAggregationSort | undefined; - let limit: number; + let queryLimit: number = this.defaultColorLimit; const colorDimensionName = config.color?.field; const showTotal = config.measure?.showTotal; + const userLimit = config.color?.limit; if (colorDimensionName) { - limit = config.color?.limit || this.defaultColorLimit; + const showOtherEnabled = config.showOther !== false; + if (showOtherEnabled) { + queryLimit = Math.max( + userLimit ?? this.defaultColorLimit, + this.defaultColorLimit, + ); + } else { + queryLimit = userLimit || this.defaultColorLimit; + } dimensions = [{ name: colorDimensionName }]; colorSort = this.getColorSort(config); } @@ -122,7 +131,7 @@ export class CircularChartProvider { sort: colorSort ? [colorSort] : undefined, where: topNWhere, timeRange, - limit: limit?.toString(), + limit: queryLimit?.toString(), }, { query: { @@ -216,7 +225,7 @@ export class CircularChartProvider { where: combinedWhere, sort: colorSort ? [colorSort] : undefined, timeRange, - limit: limit?.toString(), + limit: queryLimit?.toString(), }, { query: { @@ -293,11 +302,14 @@ export class CircularChartProvider { } const showOther = config.showOther !== false; - const explicitLimit = config.color?.limit; + const userLimit = config.color?.limit; + const isExplicitLimit = + userLimit !== undefined && userLimit !== this.defaultColorLimit; const result = computeOtherGrouping(data, measureField, colorField, { - limit: explicitLimit, + limit: isExplicitLimit ? userLimit : undefined, showOther, + grandTotal: this.totalsValue, }); this.otherGroupResult = result; diff --git a/web-common/src/features/components/charts/circular/other-grouping.spec.ts b/web-common/src/features/components/charts/circular/other-grouping.spec.ts new file mode 100644 index 00000000000..12818842540 --- /dev/null +++ b/web-common/src/features/components/charts/circular/other-grouping.spec.ts @@ -0,0 +1,198 @@ +import { describe, expect, it } from "vitest"; +import { + computeOtherGrouping, + getOtherTooltipData, + OTHER_SLICE_LABEL, +} from "./other-grouping"; + +function makeData( + values: Array<[string, number]>, + colorField = "category", + measureField = "value", +) { + return values.map(([name, val]) => ({ + [colorField]: name, + [measureField]: val, + })); +} + +describe("computeOtherGrouping", () => { + it("returns all slices when 3 or fewer categories", () => { + const data = makeData([ + ["A", 50], + ["B", 30], + ["C", 20], + ]); + const result = computeOtherGrouping(data, "value", "category", {}); + expect(result.hasOther).toBe(false); + expect(result.visibleData).toHaveLength(3); + expect(result.total).toBe(100); + }); + + it("groups long-tail distribution (top 3 = 80%): shows ~4-5 slices + Other", () => { + const data = makeData([ + ["A", 400], + ["B", 250], + ["C", 150], + ["D", 80], + ["E", 50], + ["F", 30], + ["G", 25], + ["H", 15], + ]); + const result = computeOtherGrouping(data, "value", "category", {}); + expect(result.hasOther).toBe(true); + const otherSlice = result.visibleData.find( + (d) => d.category === OTHER_SLICE_LABEL, + ); + expect(otherSlice).toBeDefined(); + const namedSlices = result.visibleData.filter( + (d) => d.category !== OTHER_SLICE_LABEL, + ); + expect(namedSlices.length).toBeGreaterThanOrEqual(3); + expect(namedSlices.length).toBeLessThanOrEqual(7); + }); + + it("groups even distribution (20 categories, ~5% each): shows 10 + Other", () => { + const data = makeData( + Array.from({ length: 20 }, (_, i) => [`Cat${i}`, 50]), + ); + const result = computeOtherGrouping(data, "value", "category", {}); + expect(result.hasOther).toBe(true); + const namedSlices = result.visibleData.filter( + (d) => d.category !== OTHER_SLICE_LABEL, + ); + expect(namedSlices.length).toBe(10); + }); + + it("respects explicit limit: 3", () => { + const data = makeData( + Array.from({ length: 10 }, (_, i) => [`Cat${i}`, 100 - i * 10]), + ); + const result = computeOtherGrouping(data, "value", "category", { + limit: 3, + }); + expect(result.hasOther).toBe(true); + const namedSlices = result.visibleData.filter( + (d) => d.category !== OTHER_SLICE_LABEL, + ); + expect(namedSlices.length).toBe(3); + }); + + it("respects showOther: false", () => { + const data = makeData( + Array.from({ length: 20 }, (_, i) => [`Cat${i}`, 50]), + ); + const result = computeOtherGrouping(data, "value", "category", { + showOther: false, + }); + expect(result.hasOther).toBe(false); + expect(result.visibleData).toHaveLength(20); + }); + + it("does not create Other when only 1 item would be grouped", () => { + const data = makeData([ + ["A", 400], + ["B", 300], + ["C", 200], + ["D", 100], + ]); + const result = computeOtherGrouping(data, "value", "category", { + limit: 3, + }); + expect(result.hasOther).toBe(false); + expect(result.visibleData).toHaveLength(4); + }); + + it("sums Other value correctly", () => { + const data = makeData([ + ["A", 500], + ["B", 300], + ["C", 100], + ["D", 50], + ["E", 30], + ["F", 20], + ]); + const result = computeOtherGrouping(data, "value", "category", { + limit: 3, + }); + expect(result.hasOther).toBe(true); + const otherSlice = result.visibleData.find( + (d) => d.category === OTHER_SLICE_LABEL, + ); + expect(otherSlice?.value).toBe(100); + expect(result.otherItems).toHaveLength(3); + expect(result.total).toBe(1000); + }); + + it("handles empty data", () => { + const result = computeOtherGrouping([], "value", "category", {}); + expect(result.hasOther).toBe(false); + expect(result.visibleData).toHaveLength(0); + expect(result.total).toBe(0); + }); + + it("handles single item", () => { + const data = makeData([["A", 100]]); + const result = computeOtherGrouping(data, "value", "category", {}); + expect(result.hasOther).toBe(false); + expect(result.visibleData).toHaveLength(1); + }); + + it("uses grandTotal when provided for Other value calculation", () => { + const data = makeData([ + ["A", 500], + ["B", 300], + ["C", 100], + ["D", 50], + ["E", 30], + ["F", 20], + ]); + const result = computeOtherGrouping(data, "value", "category", { + limit: 3, + grandTotal: 2000, + }); + expect(result.hasOther).toBe(true); + expect(result.total).toBe(2000); + const otherSlice = result.visibleData.find( + (d) => d.category === OTHER_SLICE_LABEL, + ); + expect(otherSlice?.value).toBe(2000 - 500 - 300 - 100); + }); +}); + +describe("getOtherTooltipData", () => { + it("returns top 5 items with percentages", () => { + const items = makeData( + Array.from({ length: 8 }, (_, i) => [`Item${i}`, 10 - i]), + ); + const result = getOtherTooltipData(items, "value", "category", 1000); + expect(result.items).toHaveLength(5); + expect(result.remainingCount).toBe(3); + expect(result.items[0].name).toBe("Item0"); + expect(result.items[0].value).toBe(10); + expect(result.items[0].percent).toBeCloseTo(1.0, 1); + }); + + it("returns all items when fewer than 5", () => { + const items = makeData([ + ["X", 20], + ["Y", 10], + ]); + const result = getOtherTooltipData(items, "value", "category", 100); + expect(result.items).toHaveLength(2); + expect(result.remainingCount).toBe(0); + expect(result.totalPercent).toBeCloseTo(30, 1); + }); + + it("calculates total value and percent", () => { + const items = makeData([ + ["A", 30], + ["B", 20], + ["C", 10], + ]); + const result = getOtherTooltipData(items, "value", "category", 200); + expect(result.totalValue).toBe(60); + expect(result.totalPercent).toBeCloseTo(30, 1); + }); +}); diff --git a/web-common/src/features/components/charts/circular/other-grouping.ts b/web-common/src/features/components/charts/circular/other-grouping.ts index e432d514f8e..dbf85efe4f4 100644 --- a/web-common/src/features/components/charts/circular/other-grouping.ts +++ b/web-common/src/features/components/charts/circular/other-grouping.ts @@ -31,12 +31,13 @@ export function computeOtherGrouping( data: V1MetricsViewAggregationResponseDataItem[], measureField: string, colorField: string, - options: { limit?: number; showOther?: boolean }, + options: { limit?: number; showOther?: boolean; grandTotal?: number }, ): OtherGroupResult { - const total = data.reduce( + const dataTotal = data.reduce( (sum, d) => sum + (Number(d[measureField]) || 0), 0, ); + const total = options.grandTotal ?? dataTotal; if (options.showOther === false || data.length <= 1) { return { visibleData: data, otherItems: [], total, hasOther: false }; @@ -51,7 +52,7 @@ export function computeOtherGrouping( if (options.limit !== undefined) { cutoff = Math.max(1, Math.min(options.limit, sorted.length)); } else { - cutoff = computeDynamicCutoff(sorted, measureField, total); + cutoff = computeDynamicCutoff(sorted, measureField, dataTotal); } if (cutoff >= sorted.length || sorted.length - cutoff <= 1) { @@ -61,10 +62,11 @@ export function computeOtherGrouping( const visible = sorted.slice(0, cutoff); const others = sorted.slice(cutoff); - const otherValue = others.reduce( + const visibleSum = visible.reduce( (sum, d) => sum + (Number(d[measureField]) || 0), 0, ); + const otherValue = total - visibleSum; const otherRow: V1MetricsViewAggregationResponseDataItem = { [colorField]: OTHER_SLICE_LABEL, From bbfaa8f072fc5df72595214ab262cdf9b930b221 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 27 Mar 2026 22:23:25 +0000 Subject: [PATCH 3/3] fix: ensure totalsValue is set when showOther is enabled without showTotal Co-authored-by: ericokuma --- .../components/charts/circular/CircularChartProvider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web-common/src/features/components/charts/circular/CircularChartProvider.ts b/web-common/src/features/components/charts/circular/CircularChartProvider.ts index d02fdcdbc3e..53541f21aa3 100644 --- a/web-common/src/features/components/charts/circular/CircularChartProvider.ts +++ b/web-common/src/features/components/charts/circular/CircularChartProvider.ts @@ -235,7 +235,7 @@ export class CircularChartProvider { }, ); - if (showTotal && config.measure?.field) { + if (needsTotal && config.measure?.field) { this.totalsValue = $totalQuery?.data?.data?.[0]?.[ config.measure?.field ] as number;