From c634d6bc524b048f995f1d1a548050b753475f49 Mon Sep 17 00:00:00 2001 From: Will Manning Date: Wed, 4 Nov 2026 06:51:18 -0500 Subject: [PATCH 1/6] Colour tooltip deltas by unit, and name the axis unit on the chart page The tooltip marked every rise as worse. On a throughput chart a rise is a gain, so the delta's colour now follows higherIsBetter(unit_kind), which the chart page's description also uses. The chart page printed the stored unit ("unit: ns") over an axis drawn in the display unit (ms). It now prints the display unit that the chart picks from the same values. Signed-off-by: Will Manning --- src/app/(bench)/bench/chart/[slug]/page.tsx | 4 +- src/bench/components/chart/tooltip.test.tsx | 79 +++++++++++++++++++++ src/bench/components/chart/tooltip.ts | 12 ++-- src/bench/lib/chart-format.test.ts | 24 ++++++- src/bench/lib/chart-format.ts | 27 ++++++- src/bench/lib/site-files.ts | 3 +- 6 files changed, 139 insertions(+), 10 deletions(-) create mode 100644 src/bench/components/chart/tooltip.test.tsx diff --git a/src/app/(bench)/bench/chart/[slug]/page.tsx b/src/app/(bench)/bench/chart/[slug]/page.tsx index efdb5c2f..3a8aacb0 100644 --- a/src/app/(bench)/bench/chart/[slug]/page.tsx +++ b/src/app/(bench)/bench/chart/[slug]/page.tsx @@ -10,9 +10,9 @@ import { Footer } from '@/bench/components/Footer'; import { BenchHeader } from '@/bench/components/BenchHeader'; import { canonicalFilterCsv, + chartUnitLabel, parseFilterCsv, singleSearchParam, - unitKindLabel, } from '@/bench/lib/chart-format'; import { cachedDefaultChartPayload, cachedFilterUniverse } from '@/bench/lib/data-cache'; import { chartPayload, type ChartResponse } from '@/bench/lib/queries'; @@ -204,7 +204,7 @@ export default async function ChartPage({

{'unit: '} - {unitKindLabel(chart.unit_kind)} + {chartUnitLabel(chart)} {' · '} {seriesCount} series · {commitCount} commit{commitCount !== 1 ? 's' : ''}

diff --git a/src/bench/components/chart/tooltip.test.tsx b/src/bench/components/chart/tooltip.test.tsx new file mode 100644 index 00000000..f02bbfaa --- /dev/null +++ b/src/bench/components/chart/tooltip.test.tsx @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +// @vitest-environment jsdom + +import type { Chart as ChartJs, TooltipModel } from 'chart.js'; +import { describe, expect, it } from 'vitest'; + +import { pickDisplayUnit, type NormalizedChartPayload } from '@/bench/lib/chart-format'; +import type { UnitKind } from '@/bench/lib/queries'; + +import { externalTooltipHandler } from './tooltip'; +import type { CardState } from './types'; + +/** + * The tooltip for one series that moves from `before` to `after` between two + * commits, hovered at the newer commit, on a chart whose values are `unitKind`. + */ +function tooltipFor(unitKind: UnitKind, before: number, after: number): HTMLDivElement { + const payload = { + display_name: 'chart', + unit_kind: unitKind, + history: { total_commits: 2, start_index: 0, loaded_commits: 2, complete: true }, + commits: [ + { sha: 'a'.repeat(40), timestamp: '2026-11-01T00:00:00Z', message: 'one', url: '' }, + { sha: 'b'.repeat(40), timestamp: '2026-11-02T00:00:00Z', message: 'two', url: '' }, + ], + series: { vortex: [before, after] }, + } satisfies NormalizedChartPayload; + const state = { + payload, + displayUnit: pickDisplayUnit(unitKind, [before, after]), + } as unknown as CardState; + const parent = document.createElement('div'); + const host = document.createElement('div'); + parent.append(host); + const chart = { + data: { + datasets: [{ label: 'vortex', borderColor: '#000', rawData: [before, after], benchMeta: {} }], + }, + getDatasetMeta: () => ({ hidden: false }), + canvas: document.createElement('canvas'), + } as unknown as ChartJs; + const tooltip = { + opacity: 1, + caretX: 0, + caretY: 0, + dataPoints: [{ dataIndex: 1 }], + } as unknown as TooltipModel<'line'>; + externalTooltipHandler(state, host)({ chart, tooltip }); + return host; +} + +function deltaClass(host: HTMLDivElement): string | undefined { + return host.querySelector('.tt-delta')?.className; +} + +describe('the tooltip delta', () => { + it('marks a slower time as worse and a faster one as better', () => { + expect(deltaClass(tooltipFor('time_ns', 100, 120))).toBe('tt-delta tt-delta--worse'); + expect(deltaClass(tooltipFor('time_ns', 120, 100))).toBe('tt-delta tt-delta--better'); + }); + + it('marks a larger size as worse', () => { + expect(deltaClass(tooltipFor('bytes', 100, 120))).toBe('tt-delta tt-delta--worse'); + }); + + // Higher is better on a throughput chart. + it('marks a rise in throughput as better and a fall as worse', () => { + const rise = tooltipFor('throughput_mb_s', 100, 120); + expect(deltaClass(rise)).toBe('tt-delta tt-delta--better'); + expect(rise.querySelector('.tt-delta')?.textContent).toBe('+20.0%'); + expect(deltaClass(tooltipFor('throughput_mb_s', 120, 100))).toBe('tt-delta tt-delta--worse'); + }); + + it('leaves no change uncoloured', () => { + expect(deltaClass(tooltipFor('throughput_mb_s', 100, 100))).toBe('tt-delta'); + }); +}); diff --git a/src/bench/components/chart/tooltip.ts b/src/bench/components/chart/tooltip.ts index 6117e66e..26c4e43c 100644 --- a/src/bench/components/chart/tooltip.ts +++ b/src/bench/components/chart/tooltip.ts @@ -8,6 +8,7 @@ import { escapeHtml, firstLine, formatDisplayValue, + higherIsBetter, IDENTITY_UNIT, predecessorValue, shortDate, @@ -49,6 +50,8 @@ export function externalTooltipHandler(state: CardState, host: HTMLDivElement | const idx = firstDp.dataIndex; const commit = state.payload?.commits?.[idx] ?? null; const displayUnit = state.displayUnit ?? IDENTITY_UNIT; + // A rise is a gain on a throughput chart and a loss on every other one. + const riseIsBetter = higherIsBetter(state.payload?.unit_kind); // Build one row per dataset from each series' `rawData` so the tooltip // shows raw measurements even when LTTB nulled out `dataset.data[idx]`. @@ -71,12 +74,13 @@ export function externalTooltipHandler(state: CardState, host: HTMLDivElement | let deltaHtml = ''; if (prevRaw !== null && prevRaw !== 0) { const pct = ((raw - prevRaw) / prevRaw) * 100; + const better = riseIsBetter ? pct > 0 : pct < 0; const cls = - pct > 0 - ? 'tt-delta tt-delta--worse' - : pct < 0 + pct === 0 + ? 'tt-delta' + : better ? 'tt-delta tt-delta--better' - : 'tt-delta'; + : 'tt-delta tt-delta--worse'; const sign = pct > 0 ? '+' : ''; deltaHtml = `${sign}${pct.toFixed(1)}%`; } diff --git a/src/bench/lib/chart-format.test.ts b/src/bench/lib/chart-format.test.ts index d92c6248..18c4d7d2 100644 --- a/src/bench/lib/chart-format.test.ts +++ b/src/bench/lib/chart-format.test.ts @@ -7,6 +7,7 @@ import { canonicalHistory, chartAriaLabel, chartHasRecentData, + chartUnitLabel, chartPriority, clampRangeWindow, collectAllValues, @@ -48,7 +49,7 @@ import { truncate, visibleRange, } from './chart-format'; -import type { ChartResponse, CommitPoint } from './queries'; +import type { ChartResponse, CommitPoint, UnitKind } from './queries'; import { defined } from '@/lib/defined'; function commit(sha: string): CommitPoint { @@ -627,6 +628,27 @@ describe('pickDisplayUnit', () => { }); }); +// The chart page's meta line names the unit its chart's axis is drawn in. +describe('chartUnitLabel', () => { + const chart = (unit_kind: UnitKind, values: number[]) => ({ + unit_kind, + series: { vortex: values, parquet: [null, ...values] }, + }); + + it("names the axis's display unit, not the stored one", () => { + expect(chartUnitLabel(chart('time_ns', [4e6, 6e6]))).toBe('ms'); + expect(chartUnitLabel(chart('time_ns', [4e3]))).toBe('µs'); + expect(chartUnitLabel(chart('time_ns', [400]))).toBe('ns'); + expect(chartUnitLabel(chart('bytes', [3e9]))).toBe('GB'); + expect(chartUnitLabel(chart('throughput_mb_s', [100]))).toBe('MB/s'); + }); + + it('names a dimensionless kind, whose axis has no unit', () => { + expect(chartUnitLabel(chart('ratio', [1.5]))).toBe('ratio'); + expect(chartUnitLabel(chart('count', [3]))).toBe('count'); + }); +}); + describe('formatDisplayValue', () => { it('applies the locked multiplier, decimals, and suffix', () => { const unit = pickDisplayUnit('time_ns', [12e9]); diff --git a/src/bench/lib/chart-format.ts b/src/bench/lib/chart-format.ts index ddbd7125..f7dc7709 100644 --- a/src/bench/lib/chart-format.ts +++ b/src/bench/lib/chart-format.ts @@ -404,9 +404,19 @@ export function singleSearchParam(v: string | string[] | undefined): string | nu return typeof v === 'string' ? v : null; } +/** + * Whether a larger value is the better result for a wire `unit_kind`. + * Throughput is best high; times and sizes, the units bench charts today, are + * best low. The chart tooltip colours each delta by it, and the chart page's + * description ranks series by it. + */ +export function higherIsBetter(unitKind: UnitKind | string | undefined): boolean { + return unitKind === 'throughput_mb_s'; +} + /** The short human label for a wire `unit_kind`, ported from the Rust - * `UnitKind::label` (used by the chart permalink page's meta line). */ -export function unitKindLabel(unitKind: UnitKind): string { + * `UnitKind::label`. */ +function unitKindLabel(unitKind: UnitKind): string { switch (unitKind) { case 'time_ns': return 'ns'; @@ -566,6 +576,19 @@ export function pickDisplayUnit( return IDENTITY_UNIT; } +/** + * The unit a chart's y-axis is drawn in, for the chart page's meta line: the + * display unit [`pickDisplayUnit`] picks from the payload's values (`ms` for + * a chart stored in nanoseconds), or the kind's own word for a dimensionless + * kind, whose axis has no unit. + */ +export function chartUnitLabel(chart: Pick): string { + return ( + pickDisplayUnit(chart.unit_kind, collectAllValues(chart)).suffix || + unitKindLabel(chart.unit_kind) + ); +} + /** * Tooltip formatter: applies the chart's locked display unit so the tooltip * value matches the y-axis tick numbers exactly. Raw `null`/`NaN` collapse to diff --git a/src/bench/lib/site-files.ts b/src/bench/lib/site-files.ts index 36880f8a..cccbd22f 100644 --- a/src/bench/lib/site-files.ts +++ b/src/bench/lib/site-files.ts @@ -15,6 +15,7 @@ import { groupAnchor, groupAnchors } from '@/bench/lib/anchor'; import { groupDocUrl } from '@/bench/lib/benchmark-docs'; +import { higherIsBetter } from '@/bench/lib/chart-format'; import { formatCompressionSizeRatio, formatThroughput, formatTimeNs } from '@/bench/lib/format'; import { groupHeadline, headlineText } from '@/bench/lib/headline'; import { METHODOLOGY, SCORES } from '@/bench/lib/methodology'; @@ -80,7 +81,7 @@ export function chartDescription(chart: ChartResponse, kind: ChartKind): string const value = values.findLast((v): v is number => v !== null && Number.isFinite(v)); return value === undefined ? [] : [{ name, value }]; }); - const sign = chart.unit_kind === 'throughput_mb_s' ? -1 : 1; + const sign = higherIsBetter(chart.unit_kind) ? -1 : 1; latest.sort((a, b) => sign * (a.value - b.value)); const commit = chart.commits.at(-1); const values = latest From 7f0eb2ce82b03abab850f47d53a50dc13e8f1ebf Mon Sep 17 00:00:00 2001 From: Will Manning Date: Wed, 4 Nov 2026 06:51:18 -0500 Subject: [PATCH 2/6] Mark a summary total over part of the buckets as partial A series measured in 20 of 22 queries showed a smaller total beside the complete totals, and only its hover title said why. Its total now reads "(partial)", the word a group's headline uses, in the query, vector search, and random access cards. Signed-off-by: Will Manning --- src/bench/components/SummaryCard.test.tsx | 23 +++++++++++++++++++++++ src/bench/components/SummaryCard.tsx | 21 +++++++++++++++------ 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/bench/components/SummaryCard.test.tsx b/src/bench/components/SummaryCard.test.tsx index 29d1993f..96e49611 100644 --- a/src/bench/components/SummaryCard.test.tsx +++ b/src/bench/components/SummaryCard.test.tsx @@ -219,6 +219,29 @@ describe('SummaryCard', () => { expect(html).toContain('5.00 s'); }); + // A total over part of the queries is smaller than a complete one, so it says + // it is partial beside the complete totals, as a group's headline does. + it('marks a total over part of the buckets as partial, in every timing card', () => { + const rankings = [ + { + name: 'duckdb:vortex-file', + score: 1, + totalRuntime: 5_000_000_000, + measured: 22, + total: 22, + }, + { name: 'duckdb:lance', score: 2, totalRuntime: 3_000_000_000, measured: 20, total: 22 }, + ]; + for (const html of [ + render({ type: 'queryBenchmark', title: 'Performance Summary', rankings, explanation: 'e' }), + render({ type: 'vectorSearch', title: 'Vector Search', rankings, explanation: 'e' }), + render({ type: 'randomAccess', hotRankings: rankings, coldRankings: [], explanation: 'e' }), + ]) { + expect(html).toContain('5.00 s'); + expect(html).toContain('3.00 s (partial)'); + } + }); + // Blue is for Vortex only: a row is Vortex's when its format, after any `engine:`, is one of // Vortex's, so DuckDB on Vortex counts and Vortex's engine on Parquet would not. it("marks Vortex's rows, and only them, as ours", () => { diff --git a/src/bench/components/SummaryCard.tsx b/src/bench/components/SummaryCard.tsx index c51bad4f..423430da 100644 --- a/src/bench/components/SummaryCard.tsx +++ b/src/bench/components/SummaryCard.tsx @@ -31,6 +31,19 @@ function seriesTitle(item: SeriesRanking, coverageUnit: string): string { : `${label} - measured in ${item.measured} of ${item.total} ${coverageUnit}; the rest scored by the missing-series penalty`; } +/** + * A ranked series' total time. A series measured in only some buckets has a + * smaller total than a complete one, so its total says "(partial)", as a + * group's headline does, and one measured in none shows a dash. + */ +function runtimeText(item: SeriesRanking): string { + if (item.measured <= 0) { + return '—'; + } + const total = formatTimeNs(item.totalRuntime); + return item.measured < item.total ? `${total} (partial)` : total; +} + /** * The per-group summary card. * @@ -221,9 +234,7 @@ export function SummaryCard({ summary }: { summary?: Summary | undefined }) { {item.score.toFixed(2)}x - - {item.measured > 0 ? formatTimeNs(item.totalRuntime) : '—'} - + {runtimeText(item)} ))} @@ -257,9 +268,7 @@ export function SummaryCard({ summary }: { summary?: Summary | undefined }) { {item.score.toFixed(2)}x - - {item.measured > 0 ? formatTimeNs(item.totalRuntime) : '—'} - + {runtimeText(item)} ))} From 457348b3ff7299bb2d1de783f099074548264b52 Mon Sep 17 00:00:00 2001 From: Will Manning Date: Wed, 4 Nov 2026 06:51:18 -0500 Subject: [PATCH 3/6] Show Expand all and Collapse all on the landing page only The chart page, the methodology page, and the loading view have no groups, so the two buttons did nothing there. The landing page now asks for them when it has groups. Signed-off-by: Will Manning --- src/app/(bench)/bench/page.tsx | 1 + src/bench/components/Toolbar.test.tsx | 17 +++++++- src/bench/components/Toolbar.tsx | 60 +++++++++++++++++---------- 3 files changed, 54 insertions(+), 24 deletions(-) diff --git a/src/app/(bench)/bench/page.tsx b/src/app/(bench)/bench/page.tsx index a570c2ff..eaea910e 100644 --- a/src/app/(bench)/bench/page.tsx +++ b/src/app/(bench)/bench/page.tsx @@ -127,6 +127,7 @@ function HomeContent({ data: [params, groups, universe] }: { data: LandingData } return ( <> 0} universe={universe} initialEngines={initialEngines} initialFormats={initialFormats} diff --git a/src/bench/components/Toolbar.test.tsx b/src/bench/components/Toolbar.test.tsx index 4d395f72..3bac4d06 100644 --- a/src/bench/components/Toolbar.test.tsx +++ b/src/bench/components/Toolbar.test.tsx @@ -8,9 +8,24 @@ import { Toolbar } from '@/bench/components/Toolbar'; const UNIVERSE = { engines: ['datafusion', 'duckdb'], formats: ['parquet', 'vortex'] }; +// Expand all and Collapse all open and close the landing page's groups, so +// they render only where the page asks for them: the chart page, the +// methodology page and the loading view have no groups. +describe('Toolbar group controls', () => { + it('renders Expand all and Collapse all only when the page has groups', () => { + const without = renderToStaticMarkup( + , + ); + expect(without).not.toContain('data-action="expand-all"'); + expect(without).not.toContain('data-action="collapse-all"'); + expect(without).toContain('data-role="global-filter-bar"'); + expect(renderToStaticMarkup()).not.toContain('Expand all'); + }); +}); + describe('Toolbar (server-rendered)', () => { const html = renderToStaticMarkup( - , + , ); // The site's header holds the page's title, the theme toggle and the GitHub button. diff --git a/src/bench/components/Toolbar.tsx b/src/bench/components/Toolbar.tsx index 37d9c9ef..701e0ded 100644 --- a/src/bench/components/Toolbar.tsx +++ b/src/bench/components/Toolbar.tsx @@ -12,6 +12,9 @@ import { setAllGroupsOpen } from '@/bench/lib/open-groups'; /** What a page passes for the filter dropdown. */ export type ToolbarProps = { + /** Draw Expand all and Collapse all: only the landing page, with its + * groups, passes it. */ + groupControls?: boolean; universe?: FilterUniverse | undefined; initialEngines?: string[] | null; initialFormats?: string[] | null; @@ -31,17 +34,24 @@ export type ToolbarProps = { * kept at least 300 ms apart. The native `toggle` event * (which also fires for scripted changes) is how each group body learns its * group opened, so no shared React tree is needed between the toolbar and the - * groups. On a page without groups (the chart page) the buttons do nothing. + * groups. The two buttons render only where the page passes `groupControls`: + * the landing page, when it has groups. The chart page, the methodology page + * and the loading view have no groups for them to open or close. * * The filter dropdown renders only when the universe has at least one chip, * matching v3's `show_filters` guard. Pages without chart data (or without the - * universe fetched) pass an empty universe and get the two buttons. + * universe fetched) pass an empty universe and get no dropdown. * * Without JavaScript none of the buttons can work, so each carries * `data-requires-js`, which hides it under the root's `no-js` class * (globals.css), as does the filter dropdown ([`FilterBar`]). */ -export function Toolbar({ universe, initialEngines, initialFormats }: ToolbarProps) { +export function Toolbar({ + groupControls = false, + universe, + initialEngines, + initialFormats, +}: ToolbarProps) { // The pre-paint script has swapped `no-js` for `js` already. If React ever // renders the root `` afresh (a root-level hydration failure does), it // puts the server's `no-js` back, which would hide every control that needs @@ -56,26 +66,30 @@ export function Toolbar({ universe, initialEngines, initialFormats }: ToolbarPro return ( // biome-ignore lint/a11y/useAriaPropsSupportedByRole: deliberate markup, as bench has always rendered the group controls.
- - + {groupControls && ( + <> + + + + )} {showFilters && ( Date: Wed, 4 Nov 2026 06:51:18 -0500 Subject: [PATCH 4/6] Make each group filter row's "all" chip clear only its own row The Engine, Format, and Series rows all wrote one per-series list, so each row's "all" chip cleared all three. The group store now keeps the Engine and Format choices apart from the Series choices, and each "all" chip clears its own row. A chip shows its own row's choice, so turning a format off leaves the engine chips on. A permalink still names series: it lists every series that any row hides. Signed-off-by: Will Manning --- src/bench/components/Chart.tsx | 8 +- src/bench/components/GroupPermalink.test.tsx | 20 ++ src/bench/components/GroupPermalink.tsx | 9 +- .../components/GroupToolbar.rows.test.tsx | 84 +++++++++ src/bench/components/GroupToolbar.test.tsx | 26 +++ src/bench/components/GroupToolbar.tsx | 23 +-- src/bench/components/chart/controller.ts | 6 +- src/bench/lib/chart-store.test.ts | 121 ++++++++++-- src/bench/lib/chart-store/group-store.ts | 178 +++++++++++++++--- 9 files changed, 406 insertions(+), 69 deletions(-) create mode 100644 src/bench/components/GroupToolbar.rows.test.tsx diff --git a/src/bench/components/Chart.tsx b/src/bench/components/Chart.tsx index 210cef80..57d57b40 100644 --- a/src/bench/components/Chart.tsx +++ b/src/bench/components/Chart.tsx @@ -134,7 +134,13 @@ export function Chart({ slug, name, index, groupSlug, initialPayload }: ChartIsl // biome-ignore lint/correctness/useExhaustiveDependencies: the filter state is the trigger; `applyFilters` reads the stores itself useEffect(() => { controllerRef.current?.applyFilters(); - }, [globalFilter, groupState.hiddenSeries, groupState.shownSeries]); + }, [ + globalFilter, + groupState.hiddenSeries, + groupState.shownSeries, + groupState.hiddenEngines, + groupState.hiddenFormats, + ]); // Broadcast the per-group Y override to non-sticky charts; `null` (the // resting default and the post-Reset state) reverts to linear. diff --git a/src/bench/components/GroupPermalink.test.tsx b/src/bench/components/GroupPermalink.test.tsx index c204032a..ce5369b1 100644 --- a/src/bench/components/GroupPermalink.test.tsx +++ b/src/bench/components/GroupPermalink.test.tsx @@ -10,9 +10,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { GroupPermalink, groupPermalinkUrl } from '@/bench/components/GroupPermalink'; import { + applyGroupMacro, initGlobalFilter, initGroupFilter, noteGroupSeries, + resetGroup, toggleGroupSeries, } from '@/bench/lib/chart-store'; @@ -89,6 +91,7 @@ describe('GroupPermalink click behavior', () => { Reflect.deleteProperty(navigator, 'clipboard'); window.history.replaceState(null, '', '/'); initGroupFilter('random_access', [], []); + resetGroup('random_access'); }); /** Mount the link inside a real `
`/`` host, as on the @@ -259,6 +262,23 @@ describe('GroupPermalink click behavior', () => { ]); }); + // A permalink names series, so an engine or format the group's filter hides + // reaches it as the series that engine or format covers. + it('copies the series an engine or format choice hides', async () => { + const { link } = mount(); + noteGroupSeries('random_access', { + 'datafusion:parquet': { engine: 'datafusion', format: 'parquet' }, + 'duckdb:parquet': { engine: 'duckdb', format: 'parquet' }, + }); + applyGroupMacro('random_access', 'engine', 'datafusion'); + + await click(link); + + expect(copiedText).toEqual([ + `${window.location.origin}/?group=random-access&hide=datafusion%3Aparquet#random-access`, + ]); + }); + it('preserves a local override that restores a globally hidden series', async () => { const { link } = mount(); initGlobalFilter({ engines: [], formats: ['parquet', 'lance'] }, [], []); diff --git a/src/bench/components/GroupPermalink.tsx b/src/bench/components/GroupPermalink.tsx index 1b3a8923..c43ab514 100644 --- a/src/bench/components/GroupPermalink.tsx +++ b/src/bench/components/GroupPermalink.tsx @@ -6,7 +6,7 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { useBenchPath } from '@/bench/components/BenchBase'; -import { getGroupSnapshot } from '@/bench/lib/chart-store'; +import { getGroupSnapshot, groupHiddenSeries } from '@/bench/lib/chart-store'; import { GROUP_FILTER_PARAM, HIDDEN_SERIES_PARAM, @@ -103,7 +103,12 @@ export function GroupPermalink({ const [href, setHref] = useState(path(`/#${encodeURIComponent(anchor)}`)); const permalink = useCallback((): string => { const snapshot = getGroupSnapshot(groupSlug); - return groupPermalinkUrl(anchor, snapshot.hiddenSeries, snapshot.shownSeries, window.location); + return groupPermalinkUrl( + anchor, + groupHiddenSeries(snapshot), + snapshot.shownSeries, + window.location, + ); }, [anchor, groupSlug]); const refreshHref = useCallback((): void => { setHref(permalink().slice(window.location.origin.length)); diff --git a/src/bench/components/GroupToolbar.rows.test.tsx b/src/bench/components/GroupToolbar.rows.test.tsx new file mode 100644 index 00000000..2bf61a7e --- /dev/null +++ b/src/bench/components/GroupToolbar.rows.test.tsx @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +// @vitest-environment jsdom + +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { GroupToolbar } from '@/bench/components/GroupToolbar'; +import { defined } from '@/lib/defined'; +import { + applyGroupMacro, + getGroupSnapshot, + initGlobalFilter, + noteGroupSeries, + toggleGroupSeries, +} from '@/bench/lib/chart-store'; + +const UNIVERSE = { engines: ['datafusion', 'duckdb'], formats: ['parquet', 'vortex'] }; + +// Each row's "all" chip turns its own row back on and leaves the other rows' +// choices alone. +describe("GroupToolbar's all chips", () => { + let root: Root | null = null; + const host = document.createElement('div'); + + beforeEach(() => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + initGlobalFilter(UNIVERSE, null, null); + }); + + afterEach(() => { + act(() => root?.unmount()); + root = null; + host.remove(); + }); + + /** Mount a group with one choice in each row, and click one row's "all" chip. */ + function clickAll(slug: string, dim: 'engine' | 'format' | 'series'): void { + noteGroupSeries(slug, { + 'datafusion:parquet': { engine: 'datafusion', format: 'parquet' }, + 'datafusion:vortex': { engine: 'datafusion', format: 'vortex' }, + 'duckdb:parquet': { engine: 'duckdb', format: 'parquet' }, + 'duckdb:vortex': { engine: 'duckdb', format: 'vortex' }, + }); + applyGroupMacro(slug, 'engine', 'datafusion'); + applyGroupMacro(slug, 'format', 'parquet'); + toggleGroupSeries(slug, 'duckdb:vortex'); + document.body.appendChild(host); + root = createRoot(host); + act(() => { + root?.render(); + }); + const chip = defined( + host.querySelector(`[data-group-filter="${dim}"][data-value="*"]`), + ); + act(() => chip.click()); + } + + it('in the Engine row clears only the engines', () => { + clickAll('rows-engine', 'engine'); + const snapshot = getGroupSnapshot('rows-engine'); + expect(snapshot.hiddenEngines).toEqual([]); + expect(snapshot.hiddenFormats).toEqual(['parquet']); + expect(snapshot.hiddenSeries).toEqual(['duckdb:vortex']); + }); + + it('in the Format row clears only the formats', () => { + clickAll('rows-format', 'format'); + const snapshot = getGroupSnapshot('rows-format'); + expect(snapshot.hiddenEngines).toEqual(['datafusion']); + expect(snapshot.hiddenFormats).toEqual([]); + expect(snapshot.hiddenSeries).toEqual(['duckdb:vortex']); + }); + + it('in the Series row clears only the series', () => { + clickAll('rows-series', 'series'); + const snapshot = getGroupSnapshot('rows-series'); + expect(snapshot.hiddenEngines).toEqual(['datafusion']); + expect(snapshot.hiddenFormats).toEqual(['parquet']); + expect(snapshot.hiddenSeries).toEqual([]); + }); +}); diff --git a/src/bench/components/GroupToolbar.test.tsx b/src/bench/components/GroupToolbar.test.tsx index efd40117..40a2f577 100644 --- a/src/bench/components/GroupToolbar.test.tsx +++ b/src/bench/components/GroupToolbar.test.tsx @@ -6,6 +6,7 @@ import { describe, expect, it } from 'vitest'; import { GroupToolbar } from '@/bench/components/GroupToolbar'; import { + applyGroupMacro, initGlobalFilter, noteChartRecentData, noteGroupSeries, @@ -97,3 +98,28 @@ describe('GroupToolbar cascading filters', () => { expect(html).toMatch(/data-group-filter="series" data-value="lance" aria-pressed="true"/); }); }); + +// Each row's chips show that row's choice: turning Parquet off leaves every +// engine chip on, though each engine then has a hidden series. +describe('GroupToolbar filter rows', () => { + it("shows each row's own choices", () => { + const slug = 'toolbar-independent-rows'; + initGlobalFilter(UNIVERSE, null, null); + noteGroupSeries(slug, { + 'datafusion:parquet': { engine: 'datafusion', format: 'parquet' }, + 'datafusion:vortex': { engine: 'datafusion', format: 'vortex' }, + 'duckdb:parquet': { engine: 'duckdb', format: 'parquet' }, + 'duckdb:vortex': { engine: 'duckdb', format: 'vortex' }, + }); + applyGroupMacro(slug, 'format', 'parquet'); + + const html = renderToStaticMarkup(); + expect(html).toMatch(/data-group-filter="engine" data-value="datafusion" aria-pressed="true"/); + expect(html).toMatch(/data-group-filter="engine" data-value="duckdb" aria-pressed="true"/); + expect(html).toMatch(/data-group-filter="format" data-value="parquet" aria-pressed="false"/); + expect(html).toMatch(/data-group-filter="format" data-value="vortex" aria-pressed="true"/); + expect(html).toMatch( + /data-group-filter="series" data-value="duckdb:parquet" aria-pressed="false"/, + ); + }); +}); diff --git a/src/bench/components/GroupToolbar.tsx b/src/bench/components/GroupToolbar.tsx index 01c3fe4e..50318fa6 100644 --- a/src/bench/components/GroupToolbar.tsx +++ b/src/bench/components/GroupToolbar.tsx @@ -9,9 +9,10 @@ import { FilterIcon } from '@/bench/components/icons'; import { DEFAULT_VISIBLE, type FilterUniverse } from '@/bench/lib/chart-format'; import { applyGroupMacro, - clearGroupSeriesFilter, + clearGroupFilterRow, getGlobalFilterSnapshot, getGroupSnapshot, + groupMacroIsActive, groupSeriesIsVisible, resetGroup, setGroupY, @@ -165,7 +166,6 @@ export function GroupToolbar({ groupSlug={groupSlug} snapshot={snapshot} globalSnapshot={globalSnapshot} - knownSeries={snapshot.knownSeries} />
Series @@ -184,7 +183,7 @@ export function GroupToolbar({ data-group-filter="series" data-value="*" aria-pressed="false" - onClick={() => clearGroupSeriesFilter(groupSlug)} + onClick={() => clearGroupFilterRow(groupSlug, 'series')} > all @@ -246,9 +245,10 @@ export function GroupToolbar({ /** * An engine/format macro row inside the per-group filter panel. A macro chip is - * active iff at least one known series matches this tag AND every match is - * currently visible; with no matching series the chip is inert and renders - * inactive so the dropdown does not falsely advertise irrelevant filters. + * active when its row does not hide it and at least one series it covers shows + * ([`groupMacroIsActive`]); with no matching series the chip is inert and + * renders inactive so the dropdown does not falsely advertise irrelevant + * filters. The row's "all" chip clears this row's choices only. */ function MacroRow({ label, @@ -257,7 +257,6 @@ function MacroRow({ groupSlug, snapshot, globalSnapshot, - knownSeries, }: { label: string; dim: 'engine' | 'format'; @@ -265,7 +264,6 @@ function MacroRow({ groupSlug: string; snapshot: ReturnType; globalSnapshot: ReturnType; - knownSeries: Record; }) { return (
@@ -276,15 +274,12 @@ function MacroRow({ data-group-filter={dim} data-value="*" aria-pressed="false" - onClick={() => clearGroupSeriesFilter(groupSlug)} + onClick={() => clearGroupFilterRow(groupSlug, dim)} > all {universe.map((value) => { - const matching = Object.keys(knownSeries).filter((l) => knownSeries[l]?.[dim] === value); - const isActive = - matching.length > 0 && - matching.every((value) => groupSeriesIsVisible(snapshot, value, globalSnapshot)); + const isActive = groupMacroIsActive(snapshot, dim, value, globalSnapshot); return (