From d2027033666f1e1ce4b1f86fd2dca41ea03af610 Mon Sep 17 00:00:00 2001 From: dfliess Date: Tue, 18 Aug 2026 14:23:38 +0200 Subject: [PATCH 1/2] Compare a canvas KPI against a target measure A canvas KPI can only compare a measure with itself, over an earlier time range. But a budget, a target or a forecast is a second measure, so there is no way to render "revenue vs target" today, and those cards end up drawn by hand as Vega custom charts. `measure_comparisons` says which measure a KPI is compared against: ```yaml kpi_grid: metrics_view: sales measures: [revenue] measure_comparisons: - measure: revenue compare_to: target_revenue comparison: [previous, percent_change] ``` The comparison query then asks for that measure over the same time range, instead of the same measure over an earlier range. The result is stored under the name of the first measure, so the existing rendering path does not change. Each pair is a list item, because other canvas widgets already store per-measure settings that way. Both measures must belong to the same metrics view, and the reconciler checks `compare_to`. An entry for a measure the grid no longer shows is ignored, and not an error: removing a measure in the inspector leaves its entry behind, and breaking the resource for that would leave a state nobody can fix from the UI, since the inspector does not show this option. It also works without a time dimension, which the time comparison does not. For a percentage measure use `delta`, since Rill already hides `percent_change` there, and `delta` gives the difference in points. --- .../build/dashboards/canvas-widgets/data.md | 17 +++ .../ai/instructions/data/resources/canvas.md | 27 ++++ runtime/canvas/component.go | 21 +++ runtime/canvas/component_test.go | 48 +++++++ .../canvas/components/kpi-grid/KPIGrid.svelte | 3 + .../canvas/components/kpi-grid/index.ts | 11 +- .../features/canvas/components/kpi/KPI.svelte | 4 +- .../canvas/components/kpi/KPIProvider.svelte | 127 ++++++++++++++---- .../features/canvas/components/kpi/index.ts | 3 + 9 files changed, 229 insertions(+), 32 deletions(-) diff --git a/docs/docs/developers/build/dashboards/canvas-widgets/data.md b/docs/docs/developers/build/dashboards/canvas-widgets/data.md index 6c3809743e4a..c9844b4c1906 100644 --- a/docs/docs/developers/build/dashboards/canvas-widgets/data.md +++ b/docs/docs/developers/build/dashboards/canvas-widgets/data.md @@ -25,6 +25,23 @@ KPI grids display key performance indicators in a compact grid format with compa codeLanguage="yaml" /> +To compare a measure against a target instead of against an earlier period, add +`measure_comparisons`. Both measures must belong to the same metrics view, and +the comparison is made over the selected time range: + +```yaml +- kpi_grid: + metrics_view: auction_metrics + measures: + - requests + measure_comparisons: + - measure: requests + compare_to: target_requests + comparison: + - previous + - percent_change +``` + ## Leaderboard Leaderboards show ranked data with the top performers highlighted. diff --git a/runtime/ai/instructions/data/resources/canvas.md b/runtime/ai/instructions/data/resources/canvas.md index b6c603263a4a..fb997a79082b 100644 --- a/runtime/ai/instructions/data/resources/canvas.md +++ b/runtime/ai/instructions/data/resources/canvas.md @@ -299,6 +299,33 @@ kpi_grid: hide_time_range: true ``` +**Against a target instead of the past:** + +Use `measure_comparisons` when the thing to compare against is another measure +of the same metrics view, such as a budget or a forecast. The comparison is +then made over the selected time range rather than an earlier one, and the +time comparison toggle no longer applies to that measure. + +```yaml +kpi_grid: + metrics_view: sales_metrics + measures: + - total_revenue + - gross_margin_pct + measure_comparisons: + - measure: total_revenue + compare_to: target_revenue + - measure: gross_margin_pct + compare_to: target_margin_pct + comparison: + - previous # here, the target's value + - percent_change +``` + +Both measures must live in the same metrics view. For a percentage measure use +`delta` rather than `percent_change`: the relative change of a percentage is +misleading, and Rill omits it, so `delta` gives the difference in points. + ### Leaderboard Display ranked dimension values by measures: diff --git a/runtime/canvas/component.go b/runtime/canvas/component.go index 09e3c346e5de..004aef3fbd34 100644 --- a/runtime/canvas/component.go +++ b/runtime/canvas/component.go @@ -324,6 +324,27 @@ func validateKPIGrid(props map[string]any, metricsViews map[string]*runtimev1.Me } } + comparisons, ok := props["measure_comparisons"].([]any) + if !ok && props["measure_comparisons"] != nil { + return errors.New("renderer properties for kpi_grid must have 'measure_comparisons' as an array") + } + for _, c := range comparisons { + entry, ok := c.(map[string]any) + if !ok { + return errors.New("each entry in 'measure_comparisons' must be an object with 'measure' and 'compare_to'") + } + if _, ok := pathutil.GetPathString(entry, "measure"); !ok { + return errors.New("each entry in 'measure_comparisons' must include a 'measure' string") + } + compareTo, ok := pathutil.GetPathString(entry, "compare_to") + if !ok { + return errors.New("each entry in 'measure_comparisons' must include a 'compare_to' string") + } + if !metricsViewHasMeasure(mv, compareTo) { + return fmt.Errorf("referenced compare_to value %q is not a measure in metrics view %q", compareTo, mvn) + } + } + return nil } diff --git a/runtime/canvas/component_test.go b/runtime/canvas/component_test.go index 5d6b6662f20c..24eb78a5be9d 100644 --- a/runtime/canvas/component_test.go +++ b/runtime/canvas/component_test.go @@ -724,6 +724,54 @@ kpi_grid: testruntime.ReconcileParserAndWait(t, rt, id) testruntime.RequireReconcileState(t, rt, id, 4, 1, 0) testruntime.RequireReconcileErrorContains(t, rt, id, runtime.ResourceKindComponent, "c1", "is not a measure") + + // Valid: a measure compared against another measure. + testruntime.PutFiles(t, rt, id, map[string]string{ + "c1.yaml": ` +type: component +kpi_grid: + metrics_view: mv1 + measures: + - y + measure_comparisons: + - measure: y + compare_to: z +`}) + testruntime.ReconcileParserAndWait(t, rt, id) + testruntime.RequireReconcileState(t, rt, id, 4, 0, 0) + + // Invalid: compare_to isn't a measure of the metrics view. + testruntime.PutFiles(t, rt, id, map[string]string{ + "c1.yaml": ` +type: component +kpi_grid: + metrics_view: mv1 + measures: + - y + measure_comparisons: + - measure: y + compare_to: nonexistent +`}) + testruntime.ReconcileParserAndWait(t, rt, id) + testruntime.RequireReconcileState(t, rt, id, 4, 1, 0) + testruntime.RequireReconcileErrorContains(t, rt, id, runtime.ResourceKindComponent, "c1", "compare_to") + + // Valid: an entry for a measure the grid no longer shows is inert, not an + // error. Removing a measure from the visual editor leaves one behind, and + // failing the resource for it would be a state the editor cannot undo. + testruntime.PutFiles(t, rt, id, map[string]string{ + "c1.yaml": ` +type: component +kpi_grid: + metrics_view: mv1 + measures: + - y + measure_comparisons: + - measure: z + compare_to: y +`}) + testruntime.ReconcileParserAndWait(t, rt, id) + testruntime.RequireReconcileState(t, rt, id, 4, 0, 0) } func TestValidateTable(t *testing.T) { diff --git a/web-common/src/features/canvas/components/kpi-grid/KPIGrid.svelte b/web-common/src/features/canvas/components/kpi-grid/KPIGrid.svelte index ad5ac82e7912..28c1fb7481b8 100644 --- a/web-common/src/features/canvas/components/kpi-grid/KPIGrid.svelte +++ b/web-common/src/features/canvas/components/kpi-grid/KPIGrid.svelte @@ -27,6 +27,9 @@ sparkline: kpiGridProperties.sparkline, hide_time_range: kpiGridProperties.hide_time_range, comparison: kpiGridProperties.comparison, + comparison_measure: kpiGridProperties.measure_comparisons?.find( + (comparison) => comparison?.measure === measure, + )?.compare_to, dimension_filters: kpiGridProperties.dimension_filters, time_filters: kpiGridProperties.time_filters, })); diff --git a/web-common/src/features/canvas/components/kpi-grid/index.ts b/web-common/src/features/canvas/components/kpi-grid/index.ts index eeb5079ed138..bc323cdc56e8 100644 --- a/web-common/src/features/canvas/components/kpi-grid/index.ts +++ b/web-common/src/features/canvas/components/kpi-grid/index.ts @@ -34,6 +34,14 @@ export const defaultComparisonOptions: ComponentComparisonOptions[] = [ "percent_change", ]; +// Per-measure comparison target persisted in the canvas YAML. A list (not a +// map) mirrors how per-measure config is expressed elsewhere in canvas. A +// measure listed here ignores the time comparison toggle. +export interface KPIMeasureComparisonSpec { + measure: string; + compare_to: string; +} + export interface KPIGridSpec extends ComponentCommonProperties, ComponentFilterProperties { @@ -47,12 +55,13 @@ export interface KPIGridSpec hide_time_range?: boolean; // Defaults to "delta" and "percent_change" comparison?: ComponentComparisonOptions[]; + measure_comparisons?: KPIMeasureComparisonSpec[]; } export class KPIGridComponent extends BaseCanvasComponent { minSize = { width: 2, height: 2 }; defaultSize = { width: 6, height: 4 }; - resetParams = ["measures", "adhoc_measures"]; + resetParams = ["measures", "adhoc_measures", "measure_comparisons"]; type: CanvasComponentType = "kpi_grid"; component = KPIGrid; diff --git a/web-common/src/features/canvas/components/kpi/KPI.svelte b/web-common/src/features/canvas/components/kpi/KPI.svelte index 3e39562425ad..46b4e44d510c 100644 --- a/web-common/src/features/canvas/components/kpi/KPI.svelte +++ b/web-common/src/features/canvas/components/kpi/KPI.svelte @@ -332,9 +332,7 @@ {#if comparisonLabel}

- {m.kpi_vs_comparison({ - comparison: comparisonLabel?.toLowerCase() ?? "", - })} + {m.kpi_vs_comparison({ comparison: comparisonLabel ?? "" })}

{/if} {/if} diff --git a/web-common/src/features/canvas/components/kpi/KPIProvider.svelte b/web-common/src/features/canvas/components/kpi/KPIProvider.svelte index daaea0736c53..5203aa2cac44 100644 --- a/web-common/src/features/canvas/components/kpi/KPIProvider.svelte +++ b/web-common/src/features/canvas/components/kpi/KPIProvider.svelte @@ -38,9 +38,15 @@ measure: measureName, sparkline, comparison: comparisonOptions, + comparison_measure: comparisonMeasureName, hide_time_range: hideTimeRange, } = spec); + // Compare against another measure over the primary time range, instead of + // against the same measure over the comparison range. + $: comparisonMeasureKey = comparisonMeasureName ?? ""; + $: measureComparison = comparisonMeasureKey !== ""; + $: ({ timeGrain, timeRange: { timeZone, start, end }, @@ -70,14 +76,25 @@ // also wait for it. $: supportsTotal = !!measure && measureSupportsTotalsQuery(measure); + $: comparisonMeasureStore = getMeasureForMetricView( + comparisonMeasureKey, + metricsViewName, + ); + $: comparisonMeasure = $comparisonMeasureStore; + $: showSparkline = sparkline !== "none" && hasTimeSeries; - $: showComparison = !!comparisonOptions?.length && showTimeComparison; + $: showComparison = + !!comparisonOptions?.length && (showTimeComparison || measureComparison); - $: comparisonLabel = - comparisonTimeRangeState?.selectedComparisonTimeRange?.name && - (TIME_COMPARISON[comparisonTimeRangeState?.selectedComparisonTimeRange.name] - ?.label as string | undefined); + $: comparisonLabel = measureComparison + ? (comparisonMeasure?.displayName ?? comparisonMeasureKey) + : comparisonTimeRangeState?.selectedComparisonTimeRange?.name && + ( + TIME_COMPARISON[ + comparisonTimeRangeState?.selectedComparisonTimeRange.name + ]?.label as string | undefined + )?.toLowerCase(); $: queryMeasures = mapEphemeralMeasuresForRequest( [{ name: measureName }], @@ -114,25 +131,51 @@ client, { metricsView: metricsViewName, - measures: queryMeasures, - timeRange: comparisonTimeRange, + measures: measureComparison + ? [{ name: comparisonMeasureKey }] + : queryMeasures, + timeRange: measureComparison + ? { start, end, timeZone } + : comparisonTimeRange, where, priority: 50, }, { query: { - enabled: - comparisonTimeRange && - showComparison && - isValid && - supportsTotal && - !!start && - !!end && - visible, + enabled: measureComparison + ? showComparison && + isValid && + visible && + (!hasTimeSeries || (!!start && !!end)) + : comparisonTimeRange && + showComparison && + isValid && + supportsTotal && + !!start && + !!end && + visible, }, }, ); + // KPI.svelte reads comparison values keyed by the primary measure name. + // Only rewritten once the data is in: spreading the result while loading or + // in error breaks TanStack Query's discriminated union. + $: comparisonTotalResult = !measureComparison + ? $comparisonTotalQuery + : !$comparisonTotalQuery.data + ? $comparisonTotalQuery + : { + ...$comparisonTotalQuery, + data: { + ...$comparisonTotalQuery.data, + data: $comparisonTotalQuery.data.data?.map((row) => ({ + ...row, + [measureName]: row[comparisonMeasureKey], + })), + }, + }; + $: primarySparklineQuery = createQueryServiceMetricsViewTimeSeries( client, { @@ -157,10 +200,12 @@ client, { metricsViewName, - measureNames: tsMeasureNames, - ephemeralMeasures: tsEphemeralMeasures, - timeStart: comparisonTimeRange?.start, - timeEnd: comparisonTimeRange?.end, + measureNames: measureComparison + ? [comparisonMeasureKey] + : tsMeasureNames, + ephemeralMeasures: measureComparison ? undefined : tsEphemeralMeasures, + timeStart: measureComparison ? start : comparisonTimeRange?.start, + timeEnd: measureComparison ? end : comparisonTimeRange?.end, timeGranularity: timeGrain || V1TimeGrain.TIME_GRAIN_HOUR, timeZone, where, @@ -168,16 +213,42 @@ }, { query: { - enabled: - comparisonTimeRange && - isValid && - showSparkline && - showComparison && - visible, + enabled: measureComparison + ? isValid && + showSparkline && + showComparison && + visible && + !!start && + !!end + : comparisonTimeRange && + isValid && + showSparkline && + showComparison && + visible, }, }, ); + $: comparisonSparklineResult = !measureComparison + ? $comparisonSparklineQuery + : !$comparisonSparklineQuery.data + ? $comparisonSparklineQuery + : { + ...$comparisonSparklineQuery, + data: { + ...$comparisonSparklineQuery.data, + data: $comparisonSparklineQuery.data.data?.map((point) => ({ + ...point, + records: point.records && { + ...point.records, + [measureName]: (point.records as Record)[ + comparisonMeasureKey + ], + }, + })), + }, + }; + $: interval = Interval.fromDateTimes( DateTime.fromISO(start ?? "").setZone(timeZone), DateTime.fromISO(end ?? "").setZone(timeZone), @@ -188,7 +259,7 @@ {measure} {timeGrain} {timeZone} - {showTimeComparison} + showTimeComparison={showTimeComparison || measureComparison} {hasTimeSeries} {comparisonLabel} {interval} @@ -196,7 +267,7 @@ {hideTimeRange} comparisonOptions={spec.comparison} primaryTotalResult={$totalQuery} - comparisonTotalResult={$comparisonTotalQuery} + {comparisonTotalResult} primarySparklineResult={$primarySparklineQuery} - comparisonSparklineResult={$comparisonSparklineQuery} + {comparisonSparklineResult} /> diff --git a/web-common/src/features/canvas/components/kpi/index.ts b/web-common/src/features/canvas/components/kpi/index.ts index 1005c1303c7d..fd533870a52e 100644 --- a/web-common/src/features/canvas/components/kpi/index.ts +++ b/web-common/src/features/canvas/components/kpi/index.ts @@ -102,5 +102,8 @@ export interface KPISpec sparkline?: "none" | "bottom" | "right"; // Defaults to "delta" and "percent_change" comparison?: ComponentComparisonOptions[]; + // Measure to compare against over the primary time range (e.g. a target), + // instead of the time comparison. Takes precedence over it when set. + comparison_measure?: string; hide_time_range?: boolean; } From a5fa3893638e9a9bfc5684bb1cec7c4edccc1089 Mon Sep 17 00:00:00 2001 From: dfliess Date: Mon, 21 Sep 2026 16:32:34 +0200 Subject: [PATCH 2/2] Address review on KPI target comparisons - Gate the target's totals query on supportsTotal, for the KPI's own measure and for the target. A target without a single total shows no comparison. - Send the target through mapEphemeralMeasuresForRequest and splitTimeSeriesMeasures, so compare_to can name an adhoc_measures entry. The backend now accepts that too. - Validate the measure of each measure_comparisons entry against the metrics view and the ad-hoc measures. - Deleting an ad-hoc measure also drops the entries that compare against it, so the grid does not break on a leftover compare_to. - Pass the target to KPIProvider as a prop instead of adding comparison_measure to the public KPISpec. --- .../build/dashboards/canvas-widgets/data.md | 6 ++- .../ai/instructions/data/resources/canvas.md | 4 +- runtime/canvas/component.go | 10 +++- runtime/canvas/component_test.go | 40 ++++++++++++++ .../canvas/components/kpi-grid/KPIGrid.svelte | 11 ++-- .../canvas/components/kpi/KPIProvider.svelte | 52 ++++++++++++++----- .../features/canvas/components/kpi/index.ts | 3 -- .../ephemeral-measures/canvas.spec.ts | 18 +++++++ .../dashboards/ephemeral-measures/canvas.ts | 15 ++++-- 9 files changed, 132 insertions(+), 27 deletions(-) diff --git a/docs/docs/developers/build/dashboards/canvas-widgets/data.md b/docs/docs/developers/build/dashboards/canvas-widgets/data.md index c9844b4c1906..9f7cbfe2912b 100644 --- a/docs/docs/developers/build/dashboards/canvas-widgets/data.md +++ b/docs/docs/developers/build/dashboards/canvas-widgets/data.md @@ -26,8 +26,10 @@ KPI grids display key performance indicators in a compact grid format with compa /> To compare a measure against a target instead of against an earlier period, add -`measure_comparisons`. Both measures must belong to the same metrics view, and -the comparison is made over the selected time range: +`measure_comparisons`. The comparison is made over the selected time range. +`compare_to` can be a measure of the same metrics view or one of the grid's +`adhoc_measures`. If it has no single total, because it has +`required_dimensions`, the card shows no comparison: ```yaml - kpi_grid: diff --git a/runtime/ai/instructions/data/resources/canvas.md b/runtime/ai/instructions/data/resources/canvas.md index fb997a79082b..a5549b2620f0 100644 --- a/runtime/ai/instructions/data/resources/canvas.md +++ b/runtime/ai/instructions/data/resources/canvas.md @@ -322,7 +322,9 @@ kpi_grid: - percent_change ``` -Both measures must live in the same metrics view. For a percentage measure use +`compare_to` can name a measure of the same metrics view or an entry in the +grid's `adhoc_measures`. A target with `required_dimensions` has no single +total, so the card shows no comparison for it. For a percentage measure use `delta` rather than `percent_change`: the relative change of a percentage is misleading, and Rill omits it, so `delta` gives the difference in points. diff --git a/runtime/canvas/component.go b/runtime/canvas/component.go index 004aef3fbd34..9a3b4216aa4e 100644 --- a/runtime/canvas/component.go +++ b/runtime/canvas/component.go @@ -333,14 +333,20 @@ func validateKPIGrid(props map[string]any, metricsViews map[string]*runtimev1.Me if !ok { return errors.New("each entry in 'measure_comparisons' must be an object with 'measure' and 'compare_to'") } - if _, ok := pathutil.GetPathString(entry, "measure"); !ok { + measure, ok := pathutil.GetPathString(entry, "measure") + if !ok { return errors.New("each entry in 'measure_comparisons' must include a 'measure' string") } + // Checked against the metrics view, not against 'measures': an entry for a + // measure the grid no longer shows is inert, not an error. + if !metricsViewHasMeasure(mv, measure) && !ephemeralNames[measure] { + return fmt.Errorf("referenced measure_comparisons measure %q is not a measure in metrics view %q", measure, mvn) + } compareTo, ok := pathutil.GetPathString(entry, "compare_to") if !ok { return errors.New("each entry in 'measure_comparisons' must include a 'compare_to' string") } - if !metricsViewHasMeasure(mv, compareTo) { + if !metricsViewHasMeasure(mv, compareTo) && !ephemeralNames[compareTo] { return fmt.Errorf("referenced compare_to value %q is not a measure in metrics view %q", compareTo, mvn) } } diff --git a/runtime/canvas/component_test.go b/runtime/canvas/component_test.go index 24eb78a5be9d..b556bb38f0bb 100644 --- a/runtime/canvas/component_test.go +++ b/runtime/canvas/component_test.go @@ -756,6 +756,22 @@ kpi_grid: testruntime.RequireReconcileState(t, rt, id, 4, 1, 0) testruntime.RequireReconcileErrorContains(t, rt, id, runtime.ResourceKindComponent, "c1", "compare_to") + // Invalid: measure isn't a measure of the metrics view, e.g. a typo. + testruntime.PutFiles(t, rt, id, map[string]string{ + "c1.yaml": ` +type: component +kpi_grid: + metrics_view: mv1 + measures: + - y + measure_comparisons: + - measure: nonexistent + compare_to: z +`}) + testruntime.ReconcileParserAndWait(t, rt, id) + testruntime.RequireReconcileState(t, rt, id, 4, 1, 0) + testruntime.RequireReconcileErrorContains(t, rt, id, runtime.ResourceKindComponent, "c1", `measure_comparisons measure "nonexistent"`) + // Valid: an entry for a measure the grid no longer shows is inert, not an // error. Removing a measure from the visual editor leaves one behind, and // failing the resource for it would be a state the editor cannot undo. @@ -1035,6 +1051,30 @@ kpi_grid: testruntime.ReconcileParserAndWait(t, rt, id) testruntime.RequireReconcileState(t, rt, id, 4, 0, 0) + // A kpi_grid comparing a measure against a ephemeral measure, and a ephemeral + // measure against a measure, should be valid. + testruntime.PutFiles(t, rt, id, map[string]string{ + "c1.yaml": ` +type: component +kpi_grid: + metrics_view: mv1 + measures: [y, profit] + measure_comparisons: + - measure: y + compare_to: target + - measure: profit + compare_to: z + adhoc_measures: + - name: profit + display_name: Profit + expression: y - z + - name: target + display_name: Target + expression: z * 2 +`}) + testruntime.ReconcileParserAndWait(t, rt, id) + testruntime.RequireReconcileState(t, rt, id, 4, 0, 0) + // A leaderboard referencing a ephemeral measure should be valid. testruntime.PutFiles(t, rt, id, map[string]string{ "c1.yaml": ` diff --git a/web-common/src/features/canvas/components/kpi-grid/KPIGrid.svelte b/web-common/src/features/canvas/components/kpi-grid/KPIGrid.svelte index 28c1fb7481b8..b0063b0bbbc9 100644 --- a/web-common/src/features/canvas/components/kpi-grid/KPIGrid.svelte +++ b/web-common/src/features/canvas/components/kpi-grid/KPIGrid.svelte @@ -27,13 +27,17 @@ sparkline: kpiGridProperties.sparkline, hide_time_range: kpiGridProperties.hide_time_range, comparison: kpiGridProperties.comparison, - comparison_measure: kpiGridProperties.measure_comparisons?.find( - (comparison) => comparison?.measure === measure, - )?.compare_to, dimension_filters: kpiGridProperties.dimension_filters, time_filters: kpiGridProperties.time_filters, })); + $: comparisonMeasureNames = kpis.map( + (kpi) => + kpiGridProperties.measure_comparisons?.find( + (comparison) => comparison?.measure === kpi.measure, + )?.compare_to, + ); + $: filters = { time_filters: kpiGridProperties.time_filters, dimension_filters: kpiGridProperties.dimension_filters, @@ -74,6 +78,7 @@ {#if $timeAndFilterStore} ; export let canvasName: string; export let visible: boolean; + // Measure to compare against over the primary time range (e.g. a target), + // instead of the time comparison. Set by the KPI grid from its + // `measure_comparisons`; it is not part of the `kpi` component spec. + export let comparisonMeasureName: string | undefined = undefined; const client = useRuntimeClient(); @@ -38,7 +42,6 @@ measure: measureName, sparkline, comparison: comparisonOptions, - comparison_measure: comparisonMeasureName, hide_time_range: hideTimeRange, } = spec); @@ -76,16 +79,31 @@ // also wait for it. $: supportsTotal = !!measure && measureSupportsTotalsQuery(measure); + $: comparisonEphemeralDef = ephemeralMeasures?.find( + (def) => def.name === comparisonMeasureKey, + ); $: comparisonMeasureStore = getMeasureForMetricView( comparisonMeasureKey, metricsViewName, ); - $: comparisonMeasure = $comparisonMeasureStore; + $: comparisonMeasure = + $comparisonMeasureStore ?? + (comparisonEphemeralDef + ? ephemeralMeasureToSpecMeasure(comparisonEphemeralDef) + : undefined); + + // A comparison measure without a single total has no value to compare + // against, so the card shows no comparison at all. + $: comparisonSupportsTotal = + !!comparisonMeasure && measureSupportsTotalsQuery(comparisonMeasure); + + $: comparisonEnabled = measureComparison + ? comparisonSupportsTotal + : showTimeComparison; $: showSparkline = sparkline !== "none" && hasTimeSeries; - $: showComparison = - !!comparisonOptions?.length && (showTimeComparison || measureComparison); + $: showComparison = !!comparisonOptions?.length && comparisonEnabled; $: comparisonLabel = measureComparison ? (comparisonMeasure?.displayName ?? comparisonMeasureKey) @@ -103,6 +121,19 @@ $: ({ measureNames: tsMeasureNames, ephemeralMeasures: tsEphemeralMeasures } = splitTimeSeriesMeasures([measureName], ephemeralMeasures)); + $: comparisonQueryMeasures = measureComparison + ? mapEphemeralMeasuresForRequest( + [{ name: comparisonMeasureKey }], + ephemeralMeasures, + ) + : queryMeasures; + $: ({ + measureNames: comparisonTsMeasureNames, + ephemeralMeasures: comparisonTsEphemeralMeasures, + } = measureComparison + ? splitTimeSeriesMeasures([comparisonMeasureKey], ephemeralMeasures) + : { measureNames: tsMeasureNames, ephemeralMeasures: tsEphemeralMeasures }); + $: totalQuery = createQueryServiceMetricsViewAggregation( client, { @@ -131,9 +162,7 @@ client, { metricsView: metricsViewName, - measures: measureComparison - ? [{ name: comparisonMeasureKey }] - : queryMeasures, + measures: comparisonQueryMeasures, timeRange: measureComparison ? { start, end, timeZone } : comparisonTimeRange, @@ -145,6 +174,7 @@ enabled: measureComparison ? showComparison && isValid && + supportsTotal && visible && (!hasTimeSeries || (!!start && !!end)) : comparisonTimeRange && @@ -200,10 +230,8 @@ client, { metricsViewName, - measureNames: measureComparison - ? [comparisonMeasureKey] - : tsMeasureNames, - ephemeralMeasures: measureComparison ? undefined : tsEphemeralMeasures, + measureNames: comparisonTsMeasureNames, + ephemeralMeasures: comparisonTsEphemeralMeasures, timeStart: measureComparison ? start : comparisonTimeRange?.start, timeEnd: measureComparison ? end : comparisonTimeRange?.end, timeGranularity: timeGrain || V1TimeGrain.TIME_GRAIN_HOUR, @@ -259,7 +287,7 @@ {measure} {timeGrain} {timeZone} - showTimeComparison={showTimeComparison || measureComparison} + showTimeComparison={comparisonEnabled} {hasTimeSeries} {comparisonLabel} {interval} diff --git a/web-common/src/features/canvas/components/kpi/index.ts b/web-common/src/features/canvas/components/kpi/index.ts index fd533870a52e..1005c1303c7d 100644 --- a/web-common/src/features/canvas/components/kpi/index.ts +++ b/web-common/src/features/canvas/components/kpi/index.ts @@ -102,8 +102,5 @@ export interface KPISpec sparkline?: "none" | "bottom" | "right"; // Defaults to "delta" and "percent_change" comparison?: ComponentComparisonOptions[]; - // Measure to compare against over the primary time range (e.g. a target), - // instead of the time comparison. Takes precedence over it when set. - comparison_measure?: string; hide_time_range?: boolean; } diff --git a/web-common/src/features/dashboards/ephemeral-measures/canvas.spec.ts b/web-common/src/features/dashboards/ephemeral-measures/canvas.spec.ts index 25e88c96ec92..a57391046a1f 100644 --- a/web-common/src/features/dashboards/ephemeral-measures/canvas.spec.ts +++ b/web-common/src/features/dashboards/ephemeral-measures/canvas.spec.ts @@ -70,6 +70,24 @@ describe("removeMeasureFromComponentSpec", () => { }); }); + it("drops KPI comparisons that use the measure on either side", () => { + expect( + removeMeasureFromComponentSpec( + { + measures: ["revenue", "margin"], + measure_comparisons: [ + { measure: "revenue", compare_to: "target" }, + { measure: "target", compare_to: "revenue" }, + { measure: "margin", compare_to: "margin_target" }, + ], + }, + "target", + ), + ).toEqual({ + measure_comparisons: [{ measure: "margin", compare_to: "margin_target" }], + }); + }); + it("returns nothing when the measure is unused", () => { expect( removeMeasureFromComponentSpec( diff --git a/web-common/src/features/dashboards/ephemeral-measures/canvas.ts b/web-common/src/features/dashboards/ephemeral-measures/canvas.ts index 20fe00594609..1cc30c3b0947 100644 --- a/web-common/src/features/dashboards/ephemeral-measures/canvas.ts +++ b/web-common/src/features/dashboards/ephemeral-measures/canvas.ts @@ -39,9 +39,10 @@ export function ephemeralDefsToSpecs( * `undefined` value deletes the property). Handles plain lists (`measures`, * `columns`), the single-measure `measure` property, chart field configs * (`y.field`, `y.fields`, `color.field`, ...) and per-measure entries such as - * the pivot's `conditional_format`. Other properties are never touched even - * if a value equals the name: `metrics_view`, `title`, dimension lists, or a - * `comparison` list containing "delta". A field config whose only field was + * the pivot's `conditional_format` or the KPI grid's `measure_comparisons`, + * where the measure may be on either side. Other properties are never touched + * even if a value equals the name: `metrics_view`, `title`, dimension lists, or + * a `comparison` list containing "delta". A field config whose only field was * the measure is dropped entirely, matching the inspector's own remove action. */ export function removeMeasureFromComponentSpec( @@ -56,7 +57,10 @@ export function removeMeasureFromComponentSpec( const kept = value.filter((item: unknown) => MEASURE_LIST_KEYS.has(key) ? item !== name - : !(isRecord(item) && item["measure"] === name), + : !( + isRecord(item) && + MEASURE_ENTRY_KEYS.some((entryKey) => item[entryKey] === name) + ), ); if (kept.length !== value.length) changes[key] = kept; } else if (isRecord(value)) { @@ -85,6 +89,9 @@ export function removeMeasureFromComponentSpec( // Component spec lists whose string entries are measure names. const MEASURE_LIST_KEYS = new Set(["measures", "columns"]); +// Keys of per-measure list entries whose values are measure names. +const MEASURE_ENTRY_KEYS = ["measure", "compare_to"]; + function isRecord(value: unknown): value is Record { return !!value && typeof value === "object" && !Array.isArray(value); }