From 1fa4cd64ab4564342fd8e2ebb0112419bc51465e Mon Sep 17 00:00:00 2001
From: Julian Meyer <julian.meyer@coinbase.com>
Date: Mon, 20 Jul 2026 12:10:17 -0700
Subject: [PATCH] fix(report): render charts for single-role runs and missing
 validator metrics

The run-comparison charts page assumed every run had both sequencer and
validator metrics. For sequencer-only runs (no metrics-validator.json)
two things broke:

- ChartSelector fabricated a validator series for every run; fetching its
  missing metrics 404'd and rejected the whole Promise.all batch, blanking
  the page. It now emits a role series only when that role's metrics exist,
  and useMultipleDataSeries degrades an individual failed fetch to an empty
  series instead of failing the batch.
- Grouping fell back to the 'role' dimension, which collapses to a single
  value when only one role is present, so it was dropped from the available
  variables. The default then pointed at an unavailable dimension and the
  role memo threw on variables.role[0]. useBenchmarkFilters now falls back
  to the first available variable (e.g. TransactionPayload) and guards the
  role lookup, so you can compare by transaction payload with a single role.
---
 report/src/components/ChartSelector.tsx | 39 ++++++++++++++++-------
 report/src/hooks/useBenchmarkFilters.ts | 41 +++++++++++++++++++++----
 report/src/pages/RunComparison.tsx      |  3 +-
 report/src/utils/useDataSeries.ts       | 24 ++++++++++-----
 4 files changed, 81 insertions(+), 26 deletions(-)

diff --git a/report/src/components/ChartSelector.tsx b/report/src/components/ChartSelector.tsx
index cd9d4666..22f937f4 100644
--- a/report/src/components/ChartSelector.tsx
+++ b/report/src/components/ChartSelector.tsx
@@ -1,5 +1,5 @@
 import { useEffect, useMemo, useRef } from "react";
-import { BenchmarkRuns } from "../types";
+import { BenchmarkRun, BenchmarkRuns } from "../types";
 import { isEqual } from "lodash";
 import {
   camelToTitleCase,
@@ -47,10 +47,26 @@ const ChartSelector = ({
 
   const runsWithRoles = useMemo(
     () =>
-      benchmarkRuns.runs.flatMap((r) => [
-        { ...r, testConfig: { ...r.testConfig, role: "sequencer" } },
-        { ...r, testConfig: { ...r.testConfig, role: "validator" } },
-      ]),
+      benchmarkRuns.runs.flatMap((r) => {
+        // Only expose a role variant when that role actually produced metrics.
+        // Sequencer-only runs have no metrics-validator.json, and fetching a
+        // missing role 404s and rejects the whole chart batch, so emitting a
+        // phantom validator variant would blank the charts entirely.
+        const variants: BenchmarkRun[] = [];
+        if (r.result?.sequencerMetrics) {
+          variants.push({
+            ...r,
+            testConfig: { ...r.testConfig, role: "sequencer" },
+          });
+        }
+        if (r.result?.validatorMetrics) {
+          variants.push({
+            ...r,
+            testConfig: { ...r.testConfig, role: "validator" },
+          });
+        }
+        return variants;
+      }),
     [benchmarkRuns.runs],
   );
 
@@ -59,6 +75,7 @@ const ChartSelector = ({
     filterOptions,
     matchedRuns,
     filterSelections,
+    byMetric,
     setFilters,
     setByMetric,
     role,
@@ -69,7 +86,7 @@ const ChartSelector = ({
   useEffect(() => {
     let colorMap: ((val: number) => string) | undefined = undefined;
 
-    if (filterSelections.byMetric === "GasLimit" && matchedRuns.length > 0) {
+    if (byMetric === "GasLimit" && matchedRuns.length > 0) {
       const gasLimits = matchedRuns.map((r) => Number(r.testConfig.GasLimit));
       const min = Math.min(...gasLimits);
       const max = Math.max(...gasLimits);
@@ -87,9 +104,9 @@ const ChartSelector = ({
 
         let seriesName: string;
         let color: string | undefined = undefined;
-        const byMetricValue = run.testConfig[filterSelections.byMetric];
+        const byMetricValue = run.testConfig[byMetric];
 
-        if (filterSelections.byMetric === "GasLimit") {
+        if (byMetric === "GasLimit") {
           const gasLimitNum = Number(byMetricValue);
           seriesName = formatValue(gasLimitNum, "gas");
           color = colorMap?.(gasLimitNum);
@@ -119,7 +136,7 @@ const ChartSelector = ({
       lastSentDataRef.current = dataToSend;
       onChangeDataQuery({ data: dataToSend, role });
     }
-  }, [matchedRuns, filterSelections.byMetric, role, onChangeDataQuery]);
+  }, [matchedRuns, byMetric, role, onChangeDataQuery]);
 
   return (
     <div className="flex items-start">
@@ -127,7 +144,7 @@ const ChartSelector = ({
         <div>
           <div className="text-sm text-slate-500 mb-1">Show Line Per</div>
           <Select
-            value={filterSelections.byMetric}
+            value={byMetric}
             onChange={(e) => setByMetric(e.target.value)}
           >
             {Object.keys(variables).map((k) => (
@@ -139,7 +156,7 @@ const ChartSelector = ({
         </div>
         {Object.entries(filterOptions)
           .sort((a, b) => a[0].localeCompare(b[0]))
-          .filter(([k]) => k !== filterSelections.byMetric)
+          .filter(([k]) => k !== byMetric)
           .map(([key, availableValues]) => {
             const currentValue =
               filterSelections.params[key] ?? availableValues[0];
diff --git a/report/src/hooks/useBenchmarkFilters.ts b/report/src/hooks/useBenchmarkFilters.ts
index 51993a15..059d99b8 100644
--- a/report/src/hooks/useBenchmarkFilters.ts
+++ b/report/src/hooks/useBenchmarkFilters.ts
@@ -46,12 +46,25 @@ export function useBenchmarkFilters(
     );
   }, [runsWithRoles]);
 
+  // Group-by dimension to actually use: the selected/default metric only works
+  // if it is a real multi-value variable. When it is not present (e.g. the
+  // "role" default collapses to a single value for sequencer-only runs), fall
+  // back to the first available variable so grouping still works — this is what
+  // lets you compare by Transaction Payload when there is only one role.
+  const effectiveByMetric = useMemo(() => {
+    const requested = filterSelections.byMetric || defaultMetric;
+    if (variables[requested]) {
+      return requested;
+    }
+    const available = Object.keys(variables);
+    return available.length > 0 ? available[0] : requested;
+  }, [filterSelections.byMetric, defaultMetric, variables]);
+
   // Calculate current options and matched runs based on current selections + variables
   const { filterOptions, matchedRuns } = useMemo(() => {
-    // Ensure a default byMetric if somehow cleared
     const currentSelections = {
       ...filterSelections,
-      byMetric: filterSelections.byMetric || defaultMetric,
+      byMetric: effectiveByMetric,
     };
     // Pass memoized variables to avoid recalculating them inside
     return getBenchmarkVariables(
@@ -60,7 +73,7 @@ export function useBenchmarkFilters(
       variables,
       "first",
     );
-  }, [runsWithRoles, filterSelections, defaultMetric, variables]);
+  }, [runsWithRoles, filterSelections, effectiveByMetric, variables]);
 
   // Define the setter function (simplified: no adjustment logic)
   const setFilters = useCallback(
@@ -98,21 +111,37 @@ export function useBenchmarkFilters(
 
   // get the role if not grouped by role
   const role = useMemo(() => {
-    if (filterSelections.byMetric === "role") {
+    if (effectiveByMetric === "role") {
       return null;
     }
 
+    // variables.role is absent when every run shares a single role (a
+    // one-value dimension is dropped from `variables`), so fall back to the
+    // role carried on the runs themselves, then to "sequencer". Reading
+    // variables.role[0] directly would throw when only one role exists and
+    // break every non-role group-by (e.g. line-per-TransactionPayload).
+    const roleFromData = runsWithRoles.find((r) => r.testConfig.role)
+      ?.testConfig.role as "sequencer" | "validator" | undefined;
+
     return (
       (filterSelections.params.role as "sequencer" | "validator") ??
-      variables.role[0]
+      variables.role?.[0] ??
+      roleFromData ??
+      "sequencer"
     );
-  }, [filterSelections.byMetric, filterSelections.params.role, variables]);
+  }, [
+    effectiveByMetric,
+    filterSelections.params.role,
+    variables,
+    runsWithRoles,
+  ]);
 
   return {
     variables,
     filterOptions,
     matchedRuns,
     filterSelections, // Return current selections for UI binding
+    byMetric: effectiveByMetric, // Effective group-by (falls back when default is unavailable)
     setFilters, // Return the simplified setter
     setByMetric,
     role,
diff --git a/report/src/pages/RunComparison.tsx b/report/src/pages/RunComparison.tsx
index 759bcc0b..01d7981f 100644
--- a/report/src/pages/RunComparison.tsx
+++ b/report/src/pages/RunComparison.tsx
@@ -38,8 +38,7 @@ function RunComparison() {
         allBenchmarkRuns?.runs.filter(
           (run) =>
             run.testConfig.BenchmarkRun === benchmarkRunId &&
-            run.result?.complete &&
-            run.result.success,
+            run.result?.complete,
         ) ?? [],
     };
   }, [allBenchmarkRuns, benchmarkRunId]);
diff --git a/report/src/utils/useDataSeries.ts b/report/src/utils/useDataSeries.ts
index 24f08eca..3b60cd74 100644
--- a/report/src/utils/useDataSeries.ts
+++ b/report/src/utils/useDataSeries.ts
@@ -77,13 +77,23 @@ export const useMultipleDataSeries = (
   const multiFetcher = async (
     urlsToFetch: [runId: string, outputDir: string, role: string][],
   ) => {
-    // Fetch all metrics in parallel
-    const promises = urlsToFetch.map((url) => {
-      const [runId, outputDir, role] = url;
-      return fetcher([runId, outputDir, role]);
-    });
-
-    return Promise.all(promises);
+    // Fetch all metrics in parallel, but tolerate individual failures: a run
+    // that is missing metrics for the requested role (e.g. a sequencer-only run
+    // grouped by role, which has no metrics-validator.json) 404s here. Degrade
+    // that one series to empty instead of rejecting the batch, so the remaining
+    // series still render. ChartGrid already skips series with no data points.
+    const results = await Promise.all(
+      urlsToFetch.map(async (url) => {
+        const [runId, outputDir, role] = url;
+        try {
+          return await fetcher([runId, outputDir, role]);
+        } catch {
+          return [] as MetricData[];
+        }
+      }),
+    );
+
+    return results;
   };
 
   return useSWR(urlsToFetch, multiFetcher, {