diff --git a/src/app/api/series/[slug]/route.ts b/src/app/api/series/[slug]/route.ts index fbcfd1d92..06537e0af 100644 --- a/src/app/api/series/[slug]/route.ts +++ b/src/app/api/series/[slug]/route.ts @@ -2,7 +2,13 @@ import { NextResponse } from "next/server"; import { hasSeriesHistory, loadSeriesHistory } from "@/lib/series-history"; import { unstable_cache } from "next/cache"; import { getBenchmark } from "@/data/benchmarks"; -import { filterSig, loadSpecsUncached, specToBenchmark } from "@/lib/materialize/load"; +import { + dimensionLabels, + filterSig, + loadSpecsUncached, + panelMetricQuery, + specToBenchmark, +} from "@/lib/materialize/load"; import { readMaterialized } from "@/lib/materialize/store"; import { loadSnapshotFromBlob } from "@/lib/bench-blob"; import { buildProviderColors } from "@/lib/series-colors"; @@ -95,13 +101,17 @@ const getSeriesMapCached = unstable_cache( const windowSec = range === "7d" ? 7 * 86_400 : 30 * 86_400; const numPoints = range === "7d" ? 84 : 60; const labelKey = panel.label_key ?? "builder"; + // The spec here is the raw one off disk, never through + // applyDimensionsToSpec, so the reader's chain/region have to be + // injected explicitly. Omitting them pooled every chain into one + // line while the head-lag view beside it moved per chain. + const dims = dimensionLabels({ chain, region, kind, venue, tier }); + const scoped = panelMetricQuery(panel, dims); const result: Record = {}; await Promise.all( spec.providers.map(async (p) => { const sel = `${labelKey}="${escapePromLabelValue(p.slug)}"`; - const q = panel.metric.includes("{") - ? panel.metric.replace("{", `{${sel},`) - : `${panel.metric}{${sel}}`; + const q = scoped.replace("{", `{${sel},`); const s = await prom.series(q, windowSec, numPoints); if (s && s.length > 0) result[p.slug] = s; }), @@ -143,9 +153,9 @@ const getSeriesMapCached = unstable_cache( if (!panel) return; const labelKey = panel.label_key ?? "builder"; const sel = `${labelKey}="${escapePromLabelValue(p.slug)}"`; - q = panel.metric.includes("{") - ? panel.metric.replace("{", `{${sel},`) - : `${panel.metric}{${sel}}`; + // Same raw spec, same reason as the 7d/30d path above. + const dims = dimensionLabels({ chain, region, kind, venue, tier }); + q = panelMetricQuery(panel, dims).replace("{", `{${sel},`); } else { q = p.queries?.series; } @@ -158,7 +168,9 @@ const getSeriesMapCached = unstable_cache( return Object.keys(result).length > 0 ? result : null; }, // v8: denser 90d/1y resolution (6h / 18h step) so young benches render at full fidelity. - ["series-by-range-v9"], + // v10: panel series carry the reader's chain/region. Entries written + // before this hold one cross-chain average under a per-chain key. + ["series-by-range-v10"], { revalidate: 300, tags: ["benchmarks"] }, ); diff --git a/src/components/time-series-chart/index.tsx b/src/components/time-series-chart/index.tsx index 13b3e209f..4a1321e57 100644 --- a/src/components/time-series-chart/index.tsx +++ b/src/components/time-series-chart/index.tsx @@ -18,6 +18,7 @@ import { RANGE_LABEL, REGION_LABEL, pickSeries, + panelSeriesCacheKey, mean, type Range, type LineWithColor, @@ -186,14 +187,28 @@ export function TimeSeriesChart({ const [lazySeries1y, setLazySeries1y] = useState | null>(null); // Declared early so effects below can guard on it without TDZ issues. const panelActive = !!seriesOverride; - // Per-panel lazy series, keyed by panel id. Cached across a panel - // switch so flipping back to a previously-viewed panel is instant. + // Per-panel lazy series, keyed by panel id AND by the filters the series + // was fetched under. Cached across a panel switch so flipping back to a + // previously-viewed panel is instant. // 7d/30d: fetched on panel activation. 90d/1y: fetched on-demand when // the user clicks those tabs (Prom-backed, not archive-backed). + // + // The filters have to be in the key. Keyed on the panel alone, the first + // chain a reader opened was served back on every other chain tab: the + // effects below DO re-run on a chain change (chainProp is in their deps) + // and then return early on their own cache hit, so the fetch never + // happened. 1H/6H/24H never showed it because those render from the + // embedded per-filter series, which is why the bug presented as "only 7D + // and 30D are wrong". Lucas reported it on bench 001, where the First to + // report shares are a per-chain race, so one chain's numbers appearing on + // four tabs named the wrong leader on three of them. const [lazyPanel7d, setLazyPanel7d] = useState>>({}); const [lazyPanel30d, setLazyPanel30d] = useState>>({}); const [lazyPanel90d, setLazyPanel90d] = useState>>({}); const [lazyPanel1y, setLazyPanel1y] = useState>>({}); + const panelCacheKey = activePanelId + ? panelSeriesCacheKey(activePanelId, chainProp, regionProp, tierProp) + : null; // Pre-fetch 7d AND 30d in the background as soon as the chart mounts, // not just when the user clicks the tab. The fetches are non-blocking @@ -318,14 +333,14 @@ export function TimeSeriesChart({ // case since slimBenchmarkForCache drops them). Cache-keyed per panel so // switching between panels re-uses previously loaded data instantly. useEffect(() => { - if (!activePanelId) return; + if (!activePanelId || !panelCacheKey) return; // Inline overrides win — skip the fetch when the parent already // supplied panel long-range series (e.g. an unshimmed test caller). if (seriesOverride7d && seriesOverride30d) return; - if (lazyPanel7d[activePanelId] && lazyPanel30d[activePanelId]) return; + if (lazyPanel7d[panelCacheKey] && lazyPanel30d[panelCacheKey]) return; let cancelled = false; - const need7d = !seriesOverride7d && !lazyPanel7d[activePanelId]; - const need30d = !seriesOverride30d && !lazyPanel30d[activePanelId]; + const need7d = !seriesOverride7d && !lazyPanel7d[panelCacheKey]; + const need30d = !seriesOverride30d && !lazyPanel30d[panelCacheKey]; const done: Record<"7d" | "30d", boolean> = { "7d": !need7d, "30d": !need30d, @@ -349,9 +364,9 @@ export function TimeSeriesChart({ const map: Record = {}; for (const p of data.providers) map[p.slug] = p.values; if (range === "7d") { - setLazyPanel7d((prev) => ({ ...prev, [activePanelId]: map })); + setLazyPanel7d((prev) => ({ ...prev, [panelCacheKey]: map })); } else { - setLazyPanel30d((prev) => ({ ...prev, [activePanelId]: map })); + setLazyPanel30d((prev) => ({ ...prev, [panelCacheKey]: map })); } }) .catch(() => { @@ -361,9 +376,9 @@ export function TimeSeriesChart({ } else { done[range] = true; if (range === "7d") { - setLazyPanel7d((prev) => ({ ...prev, [activePanelId]: {} })); + setLazyPanel7d((prev) => ({ ...prev, [panelCacheKey]: {} })); } else { - setLazyPanel30d((prev) => ({ ...prev, [activePanelId]: {} })); + setLazyPanel30d((prev) => ({ ...prev, [panelCacheKey]: {} })); } } }); @@ -384,9 +399,9 @@ export function TimeSeriesChart({ // Triggered the first time a panel becomes active — we prefetch both // ranges so the user can switch between them without extra round-trips. useEffect(() => { - if (!activePanelId) return; - const need90d = !lazyPanel90d[activePanelId]; - const need1y = !lazyPanel1y[activePanelId]; + if (!activePanelId || !panelCacheKey) return; + const need90d = !lazyPanel90d[panelCacheKey]; + const need1y = !lazyPanel1y[panelCacheKey]; if (!need90d && !need1y) return; let cancelled = false; const buildQs = (r: "90d" | "1y") => { @@ -406,13 +421,13 @@ export function TimeSeriesChart({ if (cancelled) return; const map: Record = {}; for (const p of data.providers) map[p.slug] = p.values; - if (r === "90d") setLazyPanel90d((prev) => ({ ...prev, [activePanelId]: map })); - else setLazyPanel1y((prev) => ({ ...prev, [activePanelId]: map })); + if (r === "90d") setLazyPanel90d((prev) => ({ ...prev, [panelCacheKey]: map })); + else setLazyPanel1y((prev) => ({ ...prev, [panelCacheKey]: map })); }) .catch(() => { if (cancelled) return; - if (r === "90d") setLazyPanel90d((prev) => ({ ...prev, [activePanelId]: {} })); - else setLazyPanel1y((prev) => ({ ...prev, [activePanelId]: {} })); + if (r === "90d") setLazyPanel90d((prev) => ({ ...prev, [panelCacheKey]: {} })); + else setLazyPanel1y((prev) => ({ ...prev, [panelCacheKey]: {} })); }); }; if (need90d) fetchOne("90d"); @@ -421,10 +436,10 @@ export function TimeSeriesChart({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [activePanelId, regionProp, chainProp, tierProp, benchmark.slug]); - const panelLazy7d = activePanelId ? lazyPanel7d[activePanelId] : undefined; - const panelLazy30d = activePanelId ? lazyPanel30d[activePanelId] : undefined; - const panelLazy90d = activePanelId ? lazyPanel90d[activePanelId] : undefined; - const panelLazy1y = activePanelId ? lazyPanel1y[activePanelId] : undefined; + const panelLazy7d = panelCacheKey ? lazyPanel7d[panelCacheKey] : undefined; + const panelLazy30d = panelCacheKey ? lazyPanel30d[panelCacheKey] : undefined; + const panelLazy90d = panelCacheKey ? lazyPanel90d[panelCacheKey] : undefined; + const panelLazy1y = panelCacheKey ? lazyPanel1y[panelCacheKey] : undefined; // null = still loading → show optimistically; {} = confirmed empty → hide const seriesHasData = (m: Record | null) => diff --git a/src/components/time-series-chart/panel-cache-key.test.ts b/src/components/time-series-chart/panel-cache-key.test.ts new file mode 100644 index 000000000..6ff7b342c --- /dev/null +++ b/src/components/time-series-chart/panel-cache-key.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, test } from "bun:test"; +import { panelSeriesCacheKey } from "@/components/time-series-chart/scales"; + +/** + * The chart caches a panel's lazily fetched 7d/30d/90d/1y series, and the key + * decides whether switching a tab refetches. + * + * Keyed on the panel id alone, it did not. The effects re-ran on a chain + * change, hit their own "already cached?" guard, and returned before + * fetching, so the first chain a reader opened stayed on screen for every + * other chain. On bench 001 that put one chain's First to report shares on + * four tabs and named the wrong leader on three of them (Base is Serialized + * at 74.7 %, BNB is Mobula at 64.8 %, Robinhood is Serialized at 93.3 %; + * only Solana was Codex). 1H/6H/24H were always right because they render + * from the embedded per-filter series, which is exactly why the report was + * "only 7D and 30D". + * + * So the property under test is simply: two different filter sets must never + * share a key. + */ +describe("panelSeriesCacheKey", () => { + test("a chain change produces a different key", () => { + expect(panelSeriesCacheKey("first_share", "base")).not.toBe( + panelSeriesCacheKey("first_share", "bnb"), + ); + }); + + test("a region change produces a different key", () => { + expect(panelSeriesCacheKey("first_share", "base", "eu-west")).not.toBe( + panelSeriesCacheKey("first_share", "base", "sgp"), + ); + }); + + test("a tier change produces a different key", () => { + expect(panelSeriesCacheKey("p50", "ethereum", "eu-west", "keyed")).not.toBe( + panelSeriesCacheKey("p50", "ethereum", "eu-west", "public"), + ); + }); + + test("a panel change still produces a different key", () => { + expect(panelSeriesCacheKey("first_share", "base")).not.toBe( + panelSeriesCacheKey("head_lag", "base"), + ); + }); + + test("the same filters produce the same key, so the cache still works", () => { + expect(panelSeriesCacheKey("first_share", "base", "eu-west", "public")).toBe( + panelSeriesCacheKey("first_share", "base", "eu-west", "public"), + ); + }); + + test("an absent filter is distinct from a present one", () => { + // The unfiltered view and a chain tab must not share an entry either. + expect(panelSeriesCacheKey("first_share")).not.toBe( + panelSeriesCacheKey("first_share", "base"), + ); + }); + + test("filters cannot collide across positions", () => { + // A naive concatenation would make (chain=a, region=b) equal to + // (chain=ab) and serve one tab's series on another. + expect(panelSeriesCacheKey("p", "a", "b")).not.toBe(panelSeriesCacheKey("p", "ab")); + expect(panelSeriesCacheKey("p", "", "x")).not.toBe(panelSeriesCacheKey("p", "x", "")); + }); +}); diff --git a/src/components/time-series-chart/scales.ts b/src/components/time-series-chart/scales.ts index f4bd88911..1b8fd9efb 100644 --- a/src/components/time-series-chart/scales.ts +++ b/src/components/time-series-chart/scales.ts @@ -274,3 +274,27 @@ export function fmtTick(v: number, unit: string) { if (v >= 1000) return `${(v / 1000).toFixed(1)}s`; return `${Math.round(v)}ms`; } + +/** Cache key for a metric panel's lazily fetched long-range series. + * + * The filters belong in the key, not just the panel id. Keyed on the panel + * alone, the chart's own cache-hit guard returned before fetching whenever a + * reader switched chain, so the first chain opened was served back on every + * other tab. The effects DO re-run on a chain change; it is the early return + * that swallowed it. 1H/6H/24H never showed the fault because those render + * from the embedded per-filter series, which is why it presented as "only 7D + * and 30D are wrong". Found on bench 001 on 2026-10-29, where the First to + * report shares are a per-chain race, so one chain's numbers on four tabs + * named the wrong leader on three of them. + * + * The same class of bug was fixed once for the main series (see the `chain` + * prop docs in index.tsx) and left standing for panels. + */ +export function panelSeriesCacheKey( + panelId: string, + chain?: string, + region?: string, + tier?: string, +): string { + return `${panelId}|${chain ?? ""}|${region ?? ""}|${tier ?? ""}`; +} diff --git a/src/lib/materialize/load.ts b/src/lib/materialize/load.ts index ff2707dbc..89375e3ce 100644 --- a/src/lib/materialize/load.ts +++ b/src/lib/materialize/load.ts @@ -1011,25 +1011,10 @@ function applyDimensionsToSpec(spec: Spec, labels: Record): Spec // braced form so dimension labels (chain=..., region=...) reach them // like every provider query. Without this a panel on a chain-dimensioned // bench silently mixes every chain's series. - metric_panels: spec.metric_panels?.map((panel) => { - // A panel may declare the dimensions its metric does not carry. - // Appending one of those matches nothing and empties the panel in - // silence, which is how bench 282 shipped a "Billing units per - // request" chart that was blank on 71 of 75 views. - const ignored = new Set(panel.ignore_dimensions ?? []); - const panelLabels = ignored.size - ? Object.fromEntries( - Object.entries(labels).filter(([k]) => !ignored.has(k)), - ) - : labels; - return { - ...panel, - metric: injectLabels( - panel.metric.includes("{") ? panel.metric : `${panel.metric}{}`, - panelLabels, - ), - }; - }), + metric_panels: spec.metric_panels?.map((panel) => ({ + ...panel, + metric: panelMetricQuery(panel, labels), + })), providers: spec.providers.map((p) => ({ ...p, queries: p.queries @@ -1059,6 +1044,52 @@ function applyDimensionsToSpec(spec: Spec, labels: Record): Spec }; } +/** The dimension labels a reader's filters select, ready for injectLabels. + * + * `tier` is excluded because it picks the cohort rather than narrowing a + * series, and an `all` value is not a filter. Exported so a caller holding + * loose filter strings can build the same label set the loader uses. + */ +export function dimensionLabels(f: BenchmarkFilters): Record { + return activeFilterLabels(labelFilters(f)); +} + +/** The PromQL a metric panel runs under the reader's active filters. + * + * A panel declares a bare metric name or `metric{sel}`; normalise to the + * braced form so the dimension labels reach it like every provider query. + * Without this, a panel on a chain-dimensioned bench silently pools every + * chain into one line. + * + * Exported because TWO paths need this exact string and had drifted. The + * loader builds panel values through applyDimensionsToSpec, but the 7d and + * 30d panel SERIES are stripped from both the cache form and the snapshot on + * purpose (slimBenchmarkForCache, toSnapshot) and refetched by + * /api/series/[slug]?panel=..., which rebuilds them from the raw spec. That + * route injected only the provider selector, so from the day panels got a + * chart, every chain tab of bench 001's "First to report" view published the + * same cross-chain average, while the head-lag view beside it moved per chain + * because it goes through specToBenchmark. Lucas spotted it on 2026-10-29. + * + * A panel may also declare dimensions its metric does not carry: appending one + * matches nothing and empties the panel in silence, which is how bench 282 + * shipped a "Billing units per request" chart blank on 71 of 75 views. Hence + * ignore_dimensions, honoured here so both callers honour it. + */ +export function panelMetricQuery( + panel: { metric: string; ignore_dimensions?: string[] }, + labels: Record, +): string { + const ignored = new Set(panel.ignore_dimensions ?? []); + const panelLabels = ignored.size + ? Object.fromEntries(Object.entries(labels).filter(([k]) => !ignored.has(k))) + : labels; + return injectLabels( + panel.metric.includes("{") ? panel.metric : `${panel.metric}{}`, + panelLabels, + ); +} + export function injectLabels(query: string, labels: Record): string { return query.replace(/\{([^}]*)\}/g, (_, inside: string) => { const additions: string[] = []; diff --git a/src/lib/materialize/panel-metric-query.test.ts b/src/lib/materialize/panel-metric-query.test.ts new file mode 100644 index 000000000..f294adf1d --- /dev/null +++ b/src/lib/materialize/panel-metric-query.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, test } from "bun:test"; +import { dimensionLabels, panelMetricQuery } from "@/lib/materialize/load"; + +/** + * A metric panel's query has to carry the reader's chain and region, and for a + * long time one of the two paths that build it did not. + * + * Panel VALUES go through applyDimensionsToSpec, so they were always scoped. + * Panel SERIES for 7d and 30d are stripped from the cache form and from the + * snapshot on purpose and refetched by /api/series/[slug]?panel=..., which + * rebuilds them from the raw spec on disk. That route injected only the + * provider selector, so bench 001's "First to report" chart published one + * cross-chain average under every chain tab: Codex 68.8% and Serialized 29.5% + * identical on Base, BNB, Solana and Robinhood, while the head-lag view + * beside it moved per chain because it goes through specToBenchmark. + * + * Nothing caught it because both paths hand-rolled the same string and only + * one of them was ever read in a test. They share one function now, and this + * asserts the property that was silently false. + */ +describe("panelMetricQuery", () => { + test("a bare metric name gets the active dimensions", () => { + expect(panelMetricQuery({ metric: "head_lag_first_share_pct" }, { chain: "base" })) + .toBe('head_lag_first_share_pct{chain="base"}'); + }); + + test("an empty selector is filled rather than left empty", () => { + // This is the exact shape bench 001 declares, and the shape that made the + // bug invisible: `{}` looks filtered and matches everything. + expect( + panelMetricQuery( + { metric: "avg by (aggregator) (head_lag_first_share_pct{})" }, + { chain: "solana", region: "eu-west" }, + ), + ).toBe( + 'avg by (aggregator) (head_lag_first_share_pct{chain="solana",region="eu-west"})', + ); + }); + + test("an existing selector keeps its matchers and gains the dimensions", () => { + expect( + panelMetricQuery({ metric: 'rpc_call_total{result="ok"}' }, { chain: "base" }), + ).toBe('rpc_call_total{result="ok",chain="base"}'); + }); + + test("a label the panel already pins is not duplicated", () => { + expect( + panelMetricQuery({ metric: 'head_lag_seconds{chain="bnb"}' }, { chain: "base" }), + ).toBe('head_lag_seconds{chain="bnb"}'); + }); + + test("ignore_dimensions drops the labels the metric does not carry", () => { + // Appending a dimension a metric has no label for matches nothing and + // empties the panel in silence (bench 282, blank on 71 of 75 views). + expect( + panelMetricQuery( + { metric: "rpc_billing_units", ignore_dimensions: ["region"] }, + { chain: "base", region: "eu-west" }, + ), + ).toBe('rpc_billing_units{chain="base"}'); + }); + + test("no active filter leaves the metric queryable", () => { + expect(panelMetricQuery({ metric: "head_lag_first_share_pct" }, {})).toBe( + "head_lag_first_share_pct{}", + ); + }); + + test("the provider selector the route prepends survives the scoping", () => { + // The route does scoped.replace("{", `{${sel},`), which only works + // because panelMetricQuery always returns a braced form. + const scoped = panelMetricQuery( + { metric: "avg by (aggregator) (head_lag_first_share_pct{})" }, + { chain: "robinhood" }, + ); + expect(scoped.replace("{", '{aggregator="codex",')).toBe( + 'avg by (aggregator) (head_lag_first_share_pct{aggregator="codex",chain="robinhood"})', + ); + }); +}); + +describe("dimensionLabels", () => { + test("tier is excluded because it selects the cohort, not a slice", () => { + expect(dimensionLabels({ chain: "base", tier: "keyed" })).toEqual({ chain: "base" }); + }); + + test('"all" is not a filter', () => { + expect(dimensionLabels({ chain: "all", region: "sgp" })).toEqual({ region: "sgp" }); + }); + + test("an empty filter set yields no labels", () => { + expect(dimensionLabels({})).toEqual({}); + }); +});