Skip to content

Commit 1e405e3

Browse files
skylenetskylenet
authored
fix(ui): compare-groups crash with 6+ groups + responsive filter UX (#235)
## Summary - **Crash fix:** `RUN_SLOTS` extended from 5 to 10 entries (A–J) so URLs comparing more than 5 groups (e.g. all six EL clients) no longer crash on `RUN_SLOTS[r.index].label` being undefined. `GroupBuilder` cap raised from 5 to `RUN_SLOTS.length`. - **Per-group loading spinners** on the compare-groups builder: each card now shows a small \"Loading…\" spinner in its header while that group's config or any of its sampled result fetches are in-flight, instead of relying solely on the global state. - **Non-blocking filter updates** on both compare pages: filter URL writes (search box, regex toggle, chip toggles, gas-bucket buttons, diff filter) now run inside `useTransition`. Chip clicks and keystrokes paint immediately while the fan-out re-renders (charts × runs × thousands of tests) happen as an interruptible transition — rapid clicks cancel in-flight work instead of queueing 4s hangs per click. - **Quieter dimension breakdown:** when only 1–2 dimensions are available, the \"Show more / Show fewer dimensions\" toggle is hidden entirely (nothing to collapse). Reappears automatically with 3+ dimensions. Applied in both `DimensionInsights` and `CompareDimensionInsights`. ## Test plan - [x] Open `/compare/groups?suite=…&groups=besu:;;erigon:;;ethrex:;;geth:;;nethermind:;;reth:` — confirm the page renders without throwing on `slot.label` and each group gets a distinct color (A–F). - [x] In the group builder, while results are still loading, confirm each group card shows a small spinner in its header alongside the \"X runs found\" badge. - [x] On `/compare?runs=…` and `/compare/groups?…` with thousands of tests, click a facet chip rapidly several times — the click should feel instant; older work should be cancelled rather than queued. - [ ] In the dimension breakdown panel: with a filter narrow enough that only 1–2 dimensions remain, confirm both are visible and no toggle button is shown. Loosen the filter so 3+ dimensions appear and confirm the toggle reappears.
1 parent 4533a67 commit 1e405e3

6 files changed

Lines changed: 158 additions & 28 deletions

File tree

‎ui/src/components/compare/CompareDimensionInsights.tsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,12 +274,15 @@ export function CompareDimensionInsights({
274274

275275
const activeTerms = splitQuery(query)
276276
const hasFileFilter = activeTerms.some((t) => queryTermDimension(t) === 'file')
277-
const previewDims = dimensions.filter(({ def, values }) =>
277+
// With only 1–2 dimensions total, collapsing adds no value — just show
278+
// everything and skip the toggle.
279+
const skipPreview = dimensions.length <= 2
280+
const previewDims = skipPreview ? dimensions : dimensions.filter(({ def, values }) =>
278281
BARS_PREVIEW_KEYS.has(def.key) ||
279282
(hasFileFilter && def.key === 'fn') ||
280283
values.some((v) => searchQueryContains(query, `${def.emitKey}=${v.value}`)),
281284
)
282-
const hiddenDims = dimensions.filter((d) => !previewDims.includes(d))
285+
const hiddenDims = skipPreview ? [] : dimensions.filter((d) => !previewDims.includes(d))
283286
const visibleDims = showAllBars ? dimensions : previewDims
284287

285288
const renderBarsForValue = (def: DimensionDef, v: ValueAgg) => {

‎ui/src/components/compare/GroupBuilder.tsx‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ import { useState } from 'react'
22
import clsx from 'clsx'
33
import { Plus, Trash2 } from 'lucide-react'
44
import { JDenticon } from '@/components/shared/JDenticon'
5+
import { Spinner } from '@/components/shared/Spinner'
56
import { type IndexEntry, getIndexAggregatedStats } from '@/api/types'
67
import { formatTimestamp } from '@/utils/date'
78
import { formatDuration } from '@/utils/format'
89
import type { GroupDef } from './groupUtils'
10+
import { RUN_SLOTS } from './constants'
911

1012
// ── Props ────────────────────────────────────────────────────────
1113

@@ -25,6 +27,8 @@ interface GroupBuilderProps {
2527
groupRunCounts: number[]
2628
/** Matched index entries per group (sorted newest-first, full list before sample-size truncation). */
2729
groupMatchedRuns: IndexEntry[][]
30+
/** Per-group loading flag — true while this group's config or any of its result queries are in-flight. */
31+
groupLoadingFlags: boolean[]
2832
}
2933

3034
// ── Component ────────────────────────────────────────────────────
@@ -44,6 +48,7 @@ export function GroupBuilder({
4448
onAggModeChange,
4549
groupRunCounts,
4650
groupMatchedRuns,
51+
groupLoadingFlags,
4752
}: GroupBuilderProps) {
4853
const addGroup = () => {
4954
const nextClient = availableClients.find((c) => !groups.some((g) => g.client === c)) ?? availableClients[0] ?? ''
@@ -138,14 +143,15 @@ export function GroupBuilder({
138143
runCount={groupRunCounts[idx] ?? 0}
139144
sampleSize={sampleSize}
140145
matchedRuns={groupMatchedRuns[idx] ?? []}
146+
loading={groupLoadingFlags[idx] ?? false}
141147
onClientChange={(client) => updateGroup(idx, { client, metadata: {} })}
142148
onAddMetadata={(key, val) => addMetadata(idx, key, val)}
143149
onRemoveMetadata={(key) => removeMetadata(idx, key)}
144150
onRemove={() => removeGroup(idx)}
145151
canRemove={groups.length > 1}
146152
/>
147153
))}
148-
{availableClients.length > 0 && groups.length < 5 && (
154+
{availableClients.length > 0 && groups.length < RUN_SLOTS.length && (
149155
<button
150156
onClick={addGroup}
151157
className="flex items-center gap-1.5 self-start rounded-xs border border-dashed border-gray-300 px-3 py-1.5 text-sm/6 text-gray-600 hover:border-gray-400 hover:text-gray-800 dark:border-gray-600 dark:text-gray-400 dark:hover:border-gray-500 dark:hover:text-gray-200"
@@ -170,6 +176,7 @@ function GroupCard({
170176
runCount,
171177
sampleSize,
172178
matchedRuns,
179+
loading,
173180
onClientChange,
174181
onAddMetadata,
175182
onRemoveMetadata,
@@ -183,6 +190,7 @@ function GroupCard({
183190
runCount: number
184191
sampleSize: number
185192
matchedRuns: IndexEntry[]
193+
loading: boolean
186194
onClientChange: (client: string) => void
187195
onAddMetadata: (key: string, value: string) => void
188196
onRemoveMetadata: (key: string) => void
@@ -222,8 +230,16 @@ function GroupCard({
222230
/>
223231
)}
224232

233+
{loading && (
234+
<span className="ml-auto inline-flex items-center gap-1.5 text-xs/5 text-gray-500 dark:text-gray-400">
235+
<Spinner size="sm" />
236+
Loading…
237+
</span>
238+
)}
239+
225240
<span className={clsx(
226-
'ml-auto rounded-xs px-2 py-0.5 text-xs/5 font-medium',
241+
'rounded-xs px-2 py-0.5 text-xs/5 font-medium',
242+
!loading && 'ml-auto',
227243
runCount >= sampleSize
228244
? 'bg-green-100 text-green-800 dark:bg-green-900/40 dark:text-green-200'
229245
: runCount > 0

‎ui/src/components/compare/constants.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,66 @@ export const RUN_SLOTS: RunSlot[] = [
7777
badgeBgClass: 'bg-red-100 dark:bg-red-900/50',
7878
badgeTextClass: 'text-red-700 dark:text-red-300',
7979
},
80+
{
81+
label: 'F',
82+
color: '#06b6d4',
83+
colorLight: '#22d3ee',
84+
borderClass: 'border-cyan-500',
85+
textClass: 'text-cyan-600',
86+
textDarkClass: 'text-cyan-400',
87+
bgDotClass: 'bg-cyan-500',
88+
diffTextClass: 'text-cyan-700 dark:text-cyan-300',
89+
badgeBgClass: 'bg-cyan-100 dark:bg-cyan-900/50',
90+
badgeTextClass: 'text-cyan-700 dark:text-cyan-300',
91+
},
92+
{
93+
label: 'G',
94+
color: '#ec4899',
95+
colorLight: '#f472b6',
96+
borderClass: 'border-pink-500',
97+
textClass: 'text-pink-600',
98+
textDarkClass: 'text-pink-400',
99+
bgDotClass: 'bg-pink-500',
100+
diffTextClass: 'text-pink-700 dark:text-pink-300',
101+
badgeBgClass: 'bg-pink-100 dark:bg-pink-900/50',
102+
badgeTextClass: 'text-pink-700 dark:text-pink-300',
103+
},
104+
{
105+
label: 'H',
106+
color: '#f97316',
107+
colorLight: '#fb923c',
108+
borderClass: 'border-orange-500',
109+
textClass: 'text-orange-600',
110+
textDarkClass: 'text-orange-400',
111+
bgDotClass: 'bg-orange-500',
112+
diffTextClass: 'text-orange-700 dark:text-orange-300',
113+
badgeBgClass: 'bg-orange-100 dark:bg-orange-900/50',
114+
badgeTextClass: 'text-orange-700 dark:text-orange-300',
115+
},
116+
{
117+
label: 'I',
118+
color: '#84cc16',
119+
colorLight: '#a3e635',
120+
borderClass: 'border-lime-500',
121+
textClass: 'text-lime-600',
122+
textDarkClass: 'text-lime-400',
123+
bgDotClass: 'bg-lime-500',
124+
diffTextClass: 'text-lime-700 dark:text-lime-300',
125+
badgeBgClass: 'bg-lime-100 dark:bg-lime-900/50',
126+
badgeTextClass: 'text-lime-700 dark:text-lime-300',
127+
},
128+
{
129+
label: 'J',
130+
color: '#6366f1',
131+
colorLight: '#818cf8',
132+
borderClass: 'border-indigo-500',
133+
textClass: 'text-indigo-600',
134+
textDarkClass: 'text-indigo-400',
135+
bgDotClass: 'bg-indigo-500',
136+
diffTextClass: 'text-indigo-700 dark:text-indigo-300',
137+
badgeBgClass: 'bg-indigo-100 dark:bg-indigo-900/50',
138+
badgeTextClass: 'text-indigo-700 dark:text-indigo-300',
139+
},
80140
]
81141

82142
export interface CompareRun {

‎ui/src/components/run-detail/DimensionInsights.tsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,12 +305,15 @@ export function DimensionInsights({
305305
// next drill-down — surface it in the preview alongside File and
306306
// Gas without requiring "Show more dimensions".
307307
const hasFileFilter = activeTerms.some((t) => queryTermDimension(t) === 'file')
308-
const previewDims = dimensions.filter(({ def, values }) =>
308+
// With only 1–2 dimensions total, collapsing adds no value —
309+
// just show everything and skip the toggle.
310+
const skipPreview = dimensions.length <= 2
311+
const previewDims = skipPreview ? dimensions : dimensions.filter(({ def, values }) =>
309312
BARS_PREVIEW_KEYS.has(def.key) ||
310313
(hasFileFilter && def.key === 'fn') ||
311314
values.some((v) => searchQueryContains(query, `${def.emitKey}=${v.value}`)),
312315
)
313-
const hiddenDims = dimensions.filter((d) => !previewDims.includes(d))
316+
const hiddenDims = skipPreview ? [] : dimensions.filter((d) => !previewDims.includes(d))
314317
const visibleDims = showAllBars ? dimensions : previewDims
315318

316319
const renderDim = ({ def, values }: DimensionAgg) => {

‎ui/src/pages/CompareGroupsPage.tsx‎

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
1+
import { useCallback, useEffect, useMemo, useRef, useState, useTransition } from 'react'
22
import { Link, useSearch, useNavigate } from '@tanstack/react-router'
33
import { useQueries } from '@tanstack/react-query'
44
import clsx from 'clsx'
@@ -85,6 +85,19 @@ export function CompareGroupsPage() {
8585
[navigate, search],
8686
)
8787

88+
// Filter changes fan out into many synchronous re-renders (charts, table,
89+
// facet panel, dimension insights, with N runs each). Wrapping the URL
90+
// update in `startTransition` marks the resulting work as interruptible
91+
// so chip clicks and keystrokes feel instant even when the downstream
92+
// work takes hundreds of ms.
93+
const [, startFilterTransition] = useTransition()
94+
const updateFilterSearch = useCallback(
95+
(patch: Record<string, string | undefined>) => {
96+
startFilterTransition(() => updateSearch(patch))
97+
},
98+
[updateSearch],
99+
)
100+
88101
const setSuiteHash = useCallback(
89102
(hash: string) => updateSearch({ suite: hash || undefined, groups: undefined }),
90103
[updateSearch],
@@ -161,6 +174,28 @@ export function CompareGroupsPage() {
161174

162175
const isLoading = configQueries.some((q) => q.isLoading) || resultQueries.some((q) => q.isLoading)
163176

177+
// Per-group loading flag: true when this group's config or any of its
178+
// result queries are still in-flight. Used to show spinners on the
179+
// individual group cards.
180+
const groupLoadingFlags = useMemo(() => {
181+
const flags: boolean[] = []
182+
let resultOffset = 0
183+
for (let gi = 0; gi < groups.length; gi++) {
184+
const runIds = groupRuns[gi] ?? []
185+
const configLoading = configQueries[gi]?.isLoading ?? false
186+
let resultsLoading = false
187+
for (let ri = 0; ri < runIds.length; ri++) {
188+
if (resultQueries[resultOffset + ri]?.isLoading) {
189+
resultsLoading = true
190+
break
191+
}
192+
}
193+
resultOffset += runIds.length
194+
flags.push(configLoading || resultsLoading)
195+
}
196+
return flags
197+
}, [groups, groupRuns, configQueries, resultQueries])
198+
164199
// ─── Compute averages and build synthetic CompareRun[] ─────────
165200
const { syntheticRuns, varianceMap } = useMemo(() => {
166201
if (groups.length === 0 || isLoading) return { syntheticRuns: [] as CompareRun[], varianceMap: new Map() }
@@ -410,6 +445,7 @@ export function CompareGroupsPage() {
410445
onAggModeChange={setAggMode}
411446
groupRunCounts={groupRuns.map((ids) => ids.length)}
412447
groupMatchedRuns={groupMatchedEntries}
448+
groupLoadingFlags={groupLoadingFlags}
413449
/>
414450
</div>
415451

@@ -454,14 +490,14 @@ export function CompareGroupsPage() {
454490
? 'Regex against the raw test name.'
455491
: TEST_FILTER_HINT}
456492
value={testFilter}
457-
onChange={(e) => updateSearch({ filter: e.target.value || undefined })}
493+
onChange={(e) => updateFilterSearch({ filter: e.target.value || undefined })}
458494
className={clsx(
459495
'w-36 rounded-xs border bg-white px-2 py-0.5 text-xs/5 placeholder-gray-400 focus:outline-hidden focus:ring-1 dark:bg-gray-700 dark:text-gray-100 dark:placeholder-gray-500',
460496
'border-gray-300 focus:border-blue-500 focus:ring-blue-500 dark:border-gray-600',
461497
)}
462498
/>
463499
<button
464-
onClick={() => updateSearch({ filterRegex: testFilterRegex ? undefined : '1' })}
500+
onClick={() => updateFilterSearch({ filterRegex: testFilterRegex ? undefined : '1' })}
465501
title={testFilterRegex ? 'Regex mode' : 'Text mode'}
466502
className={clsx(
467503
'rounded-xs px-1 py-0.5 font-mono text-xs/5 transition-colors',
@@ -478,7 +514,7 @@ export function CompareGroupsPage() {
478514
<span>Gas:</span>
479515
<div className="flex flex-wrap gap-1">
480516
<button
481-
onClick={() => updateSearch({ gasBuckets: undefined })}
517+
onClick={() => updateFilterSearch({ gasBuckets: undefined })}
482518
className={clsx(
483519
'rounded-xs px-2 py-0.5 text-xs/5 font-medium transition-colors',
484520
selectedGasBuckets.size === 0
@@ -497,7 +533,7 @@ export function CompareGroupsPage() {
497533
const next = new Set(selectedGasBuckets)
498534
if (isSelected) next.delete(bucket); else next.add(bucket)
499535
const sorted = [...next].sort((a, b) => a - b)
500-
updateSearch({ gasBuckets: sorted.length > 0 ? sorted.map((v) => String(v / 1_000_000)).join(',') : undefined })
536+
updateFilterSearch({ gasBuckets: sorted.length > 0 ? sorted.map((v) => String(v / 1_000_000)).join(',') : undefined })
501537
}}
502538
className={clsx(
503539
'rounded-xs px-2 py-0.5 text-xs/5 font-medium transition-colors',
@@ -583,7 +619,7 @@ export function CompareGroupsPage() {
583619
placeholder={testFilterRegex ? 'Regex pattern...' : 'Filter… or e.g. opcode:ORIGIN'}
584620
title={testFilterRegex ? 'Regex against the raw test name.' : TEST_FILTER_HINT}
585621
value={testFilter}
586-
onChange={(e) => updateSearch({ filter: e.target.value || undefined })}
622+
onChange={(e) => updateFilterSearch({ filter: e.target.value || undefined })}
587623
className={clsx(
588624
'w-36 rounded-xs border bg-white px-2 py-0.5 text-xs/5 placeholder-gray-400 focus:outline-hidden focus:ring-1 dark:bg-gray-700 dark:text-gray-100 dark:placeholder-gray-500',
589625
testFilterRegex && testFilter && (() => { try { new RegExp(testFilter); return false } catch { return true } })()
@@ -592,7 +628,7 @@ export function CompareGroupsPage() {
592628
)}
593629
/>
594630
<button
595-
onClick={() => updateSearch({ filterRegex: testFilterRegex ? undefined : '1' })}
631+
onClick={() => updateFilterSearch({ filterRegex: testFilterRegex ? undefined : '1' })}
596632
title={testFilterRegex ? 'Regex mode (click to switch to text)' : 'Text mode (click to switch to regex)'}
597633
className={clsx(
598634
'rounded-xs px-1.5 py-0.5 font-mono text-xs/5 transition-colors',
@@ -609,7 +645,7 @@ export function CompareGroupsPage() {
609645
<span>Gas:</span>
610646
<div className="flex flex-wrap gap-1">
611647
<button
612-
onClick={() => updateSearch({ gasBuckets: undefined })}
648+
onClick={() => updateFilterSearch({ gasBuckets: undefined })}
613649
className={clsx(
614650
'rounded-xs px-2 py-0.5 text-xs/5 font-medium transition-colors',
615651
selectedGasBuckets.size === 0
@@ -628,7 +664,7 @@ export function CompareGroupsPage() {
628664
const next = new Set(selectedGasBuckets)
629665
if (isSelected) next.delete(bucket); else next.add(bucket)
630666
const sorted = [...next].sort((a, b) => a - b)
631-
updateSearch({ gasBuckets: sorted.length > 0 ? sorted.map((v) => String(v / 1_000_000)).join(',') : undefined })
667+
updateFilterSearch({ gasBuckets: sorted.length > 0 ? sorted.map((v) => String(v / 1_000_000)).join(',') : undefined })
632668
}}
633669
className={clsx(
634670
'rounded-xs px-2 py-0.5 text-xs/5 font-medium transition-colors',
@@ -649,7 +685,7 @@ export function CompareGroupsPage() {
649685
<FacetPanel
650686
testNames={[...testGasMap.keys()]}
651687
query={testFilter}
652-
onToggle={(term) => updateSearch({ filter: toggleSearchTerm(testFilter, term) || undefined })}
688+
onToggle={(term) => updateFilterSearch({ filter: toggleSearchTerm(testFilter, term) || undefined })}
653689
/>
654690

655691
<MetricsComparison
@@ -682,7 +718,7 @@ export function CompareGroupsPage() {
682718
onBaselineChange={(idx) => updateSearch({ baseline: idx > 0 ? String(idx) : undefined })}
683719
labelMode="instance-id"
684720
diffFilter={search.diffFilter === 'faster' || search.diffFilter === 'slower' ? search.diffFilter : 'all'}
685-
onDiffFilterChange={(val) => updateSearch({ diffFilter: val === 'all' ? undefined : val })}
721+
onDiffFilterChange={(val) => updateFilterSearch({ diffFilter: val === 'all' ? undefined : val })}
686722
testNameFilter={testNameFilter}
687723
zoomRange={sharedZoom ? chartZoom : undefined}
688724
onZoomChange={sharedZoom ? setChartZoom : undefined}
@@ -721,7 +757,7 @@ export function CompareGroupsPage() {
721757
labelMode="instance-id"
722758
testNameFilter={testNameFilter}
723759
query={testFilter}
724-
onToggle={(term) => updateSearch({ filter: toggleSearchTerm(testFilter, term) || undefined })}
760+
onToggle={(term) => updateFilterSearch({ filter: toggleSearchTerm(testFilter, term) || undefined })}
725761
onTestClick={setSelectedTest}
726762
/>
727763

@@ -759,7 +795,7 @@ export function CompareGroupsPage() {
759795
stepFilter={stepFilter}
760796
sampleSize={sampleSize}
761797
searchQuery={testFilter}
762-
onChipFilterToggle={(term) => updateSearch({ filter: toggleSearchTerm(testFilter, term) || undefined })}
798+
onChipFilterToggle={(term) => updateFilterSearch({ filter: toggleSearchTerm(testFilter, term) || undefined })}
763799
onClose={() => setSelectedTest(null)}
764800
/>
765801
)}

0 commit comments

Comments
 (0)