Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 28 additions & 11 deletions report/src/components/ChartSelector.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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],
);

Expand All @@ -59,6 +75,7 @@ const ChartSelector = ({
filterOptions,
matchedRuns,
filterSelections,
byMetric,
setFilters,
setByMetric,
role,
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -119,15 +136,15 @@ const ChartSelector = ({
lastSentDataRef.current = dataToSend;
onChangeDataQuery({ data: dataToSend, role });
}
}, [matchedRuns, filterSelections.byMetric, role, onChangeDataQuery]);
}, [matchedRuns, byMetric, role, onChangeDataQuery]);

return (
<div className="flex items-start">
<div className="flex flex-wrap gap-4 pb-4 items-center flex-grow">
<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) => (
Expand All @@ -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];
Expand Down
41 changes: 35 additions & 6 deletions report/src/hooks/useBenchmarkFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions report/src/pages/RunComparison.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
24 changes: 17 additions & 7 deletions report/src/utils/useDataSeries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down
Loading