fix(pivot-table): fix combineMetric denominator pruning and multi-metric fraction cross-talk

Three follow-ups from review on the show-values-as-fraction feature:
- `showValuesAs` was `renderTrigger: true`, but it changes which rollup
  levels `buildGroupbyCombinations` requests for non-additive metrics, so
  switching it needs a real requery, not a client-side-only re-render.
- The `combineMetric` filter in `buildGroupbyCombinations` stripped the
  percent-mode denominator level back out whenever it landed on the
  opposite axis from the kept metrics-layout level, blanking percentages
  with Combine metrics enabled.
- With multiple metrics sharing an axis, `fractionOf`'s grand/row/column
  total lookup hit the same metric-blind "Metric-collapse totals" slot for
  every metric, so all but the last metric divided by the wrong total.
  `fractionOf` now keeps the pushing record's own metric key in the
  selector so the lookup resolves to that metric's own total.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: rusackas

superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx

@@ -250,7 +250,10 @@ const config: ControlPanelConfig = {
               type: 'SelectControl',
               label: t('Show values as'),
               default: ShowValuesAsEnum.ACTUAL,
-              renderTrigger: true,
+              // Not a renderTrigger: for non-additive metrics, a percent choice
+              // here can require a rollup level (see `buildGroupbyCombinations`)
+              // that a prior query never fetched, so switching it needs a real
+              // requery rather than a client-side-only re-render.
               choices: [
                 [ShowValuesAsEnum.ACTUAL, t('Actual values')],
                 [ShowValuesAsEnum.PERCENT_OF_ROW, t('% of row total')],

superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/utilities.ts

@@ -250,13 +250,25 @@ export default function buildGroupbyCombinations(
     );
 
   if (formData.combineMetric) {
+    // A forced-in percent-mode denominator level (above) is collapsed on the
+    // opposite axis from a "normal" subtotal, so it can be mistaken for one
+    // and stripped back out here. Exempt it explicitly so combining metrics
+    // doesn't blank out the percent denominator.
+    const isForcedDenominatorLevel = (combination: Groupby): boolean =>
+      (needsRowsCollapsed && combination.rows.length === 0) ||
+      (needsColumnsCollapsed && combination.columns.length === 0);
+
     if (formData.metricsLayout === MetricsLayoutEnum.ROWS) {
       groupbyCombinations = groupbyCombinations.filter(
-        combination => combination.rows.length === rows.length,
+        combination =>
+          combination.rows.length === rows.length ||
+          isForcedDenominatorLevel(combination),
       );
     } else {
       groupbyCombinations = groupbyCombinations.filter(
-        combination => combination.columns.length === columns.length,
+        combination =>
+          combination.columns.length === columns.length ||
+          isForcedDenominatorLevel(combination),
       );
     }
   }

superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.ts

@@ -748,7 +748,39 @@ const baseAggregatorTemplates = {
             type
           ],
           inner: wrapped(...Array.from(x || []))(data, rowKey, colKey),
+          // The metric this cell belongs to, and which axis carries it (see the
+          // "Metric" pseudo-dimension in PivotTableChart). Captured from the
+          // first pushed record. With multiple metrics, the axis holding the
+          // metric is never actually empty, so collapsing it to `[]` (as the
+          // `selector` above does) would route every metric's lookup to the
+          // same shared total slot -- see `processRecord`'s "Metric-collapse
+          // totals". Keeping the metric's own key segment instead routes the
+          // lookup to the per-metric total that's already correctly split out.
+          metricAxis: undefined as
+            | { axis: 'row' | 'col'; value: string }
+            | null
+            | undefined,
           push(record: PivotRecord) {
+            if (this.metricAxis === undefined) {
+              const metricDim = record.__metricKey as unknown as
+                | string
+                | undefined;
+              const cols = data.props.cols as string[] | undefined;
+              const rows = data.props.rows as string[] | undefined;
+              if (metricDim && cols?.includes(metricDim)) {
+                this.metricAxis = {
+                  axis: 'col',
+                  value: String(record[metricDim]),
+                };
+              } else if (metricDim && rows?.includes(metricDim)) {
+                this.metricAxis = {
+                  axis: 'row',
+                  value: String(record[metricDim]),
+                };
+              } else {
+                this.metricAxis = null;
+              }
+            }
             this.inner.push(record);
           },
           format: fmtNonString(formatter),
@@ -758,9 +790,21 @@ const baseAggregatorTemplates = {
             // back to `null` (rendered blank) instead of throwing if it is
             // ever missing -- e.g. a denominator aggregator with no matching
             // rows in the response.
-            const denominatorAggregator = data.getAggregator(
-              ...Array.from(this.selector || []),
-            );
+            let [selRow, selCol] = (this.selector || [[], []]) as [
+              string[],
+              string[],
+            ];
+            if (this.metricAxis) {
+              if (this.metricAxis.axis === 'col' && selCol.length === 0) {
+                selCol = [this.metricAxis.value];
+              } else if (
+                this.metricAxis.axis === 'row' &&
+                selRow.length === 0
+              ) {
+                selRow = [this.metricAxis.value];
+              }
+            }
+            const denominatorAggregator = data.getAggregator(selRow, selCol);
             if (!denominatorAggregator.inner) {
               return null;
             }

superset-frontend/plugins/plugin-chart-pivot-table/test/react-pivottable/tableRenders.test.tsx

@@ -837,6 +837,88 @@ test('TableRenderer shows values as a percentage of the column total', () => {
   expect(getCellTexts('pvtTotal')).toEqual(expect.arrayContaining(['100.0%']));
 });
 
+/**
+ * Regression guard: when the metric pseudo-dimension collapses to the only
+ * thing on an axis (the grand-total level), each metric's own grand total
+ * must be used as the `showValuesAs` denominator -- not whichever metric's
+ * record was pushed last into the shared "Metric-collapse totals" slot (see
+ * `processRecord` in ../../src/react-pivottable/utilities.ts).
+ */
+const TAGGED_MULTI_METRIC_ON_COLUMNS = [
+  // leaf cells for metric m1 (grand total 30)
+  {
+    color: 'blue',
+    Metric: 'm1',
+    value: 10,
+    __rows: ['color'],
+    __columns: ['Metric'],
+    __metricKey: 'Metric',
+  },
+  {
+    color: 'red',
+    Metric: 'm1',
+    value: 20,
+    __rows: ['color'],
+    __columns: ['Metric'],
+    __metricKey: 'Metric',
+  },
+  // leaf cells for metric m2 (grand total 300) -- a different ratio so a
+  // cross-metric mixup produces a distinctly wrong percentage.
+  {
+    color: 'blue',
+    Metric: 'm2',
+    value: 250,
+    __rows: ['color'],
+    __columns: ['Metric'],
+    __metricKey: 'Metric',
+  },
+  {
+    color: 'red',
+    Metric: 'm2',
+    value: 50,
+    __rows: ['color'],
+    __columns: ['Metric'],
+    __metricKey: 'Metric',
+  },
+  // grand total level: rows = [], columns = [Metric]. m2 is pushed last.
+  {
+    Metric: 'm1',
+    value: 30,
+    __rows: [],
+    __columns: ['Metric'],
+    __metricKey: 'Metric',
+  },
+  {
+    Metric: 'm2',
+    value: 300,
+    __rows: [],
+    __columns: ['Metric'],
+    __metricKey: 'Metric',
+  },
+];
+
+test("TableRenderer divides percent_total by each metric's own grand total", () => {
+  const props = buildDefaultProps({
+    data: TAGGED_MULTI_METRIC_ON_COLUMNS,
+    rows: ['color'],
+    cols: ['Metric'],
+    vals: ['value'],
+    tableOptions: { rowTotals: true, colTotals: true },
+    showValuesAs: 'percent_total',
+  });
+  renderWithTheme(<TableRenderer {...props} />);
+
+  const cellTexts = getCellTexts('pvtVal');
+  // m1: 10/30 and 20/30 -- correct only if m1's own grand total (30) is used.
+  expect(cellTexts).toEqual(expect.arrayContaining(['33.3%', '66.7%']));
+  // m2: 250/300 and 50/300.
+  expect(cellTexts).toEqual(expect.arrayContaining(['83.3%', '16.7%']));
+  // A "last metric wins" bug would divide m1's cells by m2's grand total
+  // (300) instead, producing 3.3%/6.7%.
+  expect(cellTexts).not.toEqual(expect.arrayContaining(['3.3%']));
+  expect(cellTexts).not.toEqual(expect.arrayContaining(['6.7%']));
+});
+
 test('TableRenderer shows actual values when showValuesAs is unset (default)', () => {
   const props = buildDefaultProps({
     data: TAGGED_COUNT_DATA,