diff --git a/packages/app/cypress/e2e/overlay-legend-remove.cy.ts b/packages/app/cypress/e2e/overlay-legend-remove.cy.ts index 3be788fb..2192ab3d 100644 --- a/packages/app/cypress/e2e/overlay-legend-remove.cy.ts +++ b/packages/app/cypress/e2e/overlay-legend-remove.cy.ts @@ -92,11 +92,32 @@ describe('Official legend X works while an unofficial overlay is loaded', () => // Inactive row: the hover affordance flips to the "+" restore indicator // (explicit "clicking the name brings it back"), and the Hide X is gone. cy.get('[data-testid="chart-legend"] [title^="Show B300"]').should('exist'); + cy.get('[data-testid="scatter-best-per-sku"]').should('have.attr', 'data-state', 'unchecked'); cy.get( '[data-testid="chart-legend"] [role="button"][aria-label^="Hide"][aria-label*="B300"]', ).should('not.exist'); }); + it('keeps the official SKU hidden when chart metrics change', () => { + cy.get('[data-testid="yaxis-metric-selector"]').click({ force: true }); + cy.contains('[role="option"]', 'Cost per Million Total Tokens (Owning - Hyperscaler)').click({ + force: true, + }); + + cy.get('[data-testid="chart-legend"] [title^="Show B300"]').should('exist'); + cy.get('[data-testid="inference-chart-display"] svg .dot-group').should(($dots) => { + expect(countVisible($dots), 'visible official points after Y-axis change').to.eq(0); + }); + + cy.get('[data-testid="x-axis-mode-ttft"]').click().should('have.attr', 'data-state', 'active'); + cy.get('[data-testid="chart-legend"] [title^="Show B300"]').should('exist'); + cy.get('[data-testid="inference-chart-display"] svg .unofficial-overlay-pt').should(($pts) => { + expect(countVisible($pts), 'visible overlay points after metric changes').to.be.greaterThan( + 0, + ); + }); + }); + it('re-activating the SKU from the legend restores the official points', () => { cy.get('[data-testid="chart-legend"]').contains('B300').click(); cy.get('[data-testid="inference-chart-display"] svg .dot-group').should(($dots) => { diff --git a/packages/app/cypress/e2e/url-params.cy.ts b/packages/app/cypress/e2e/url-params.cy.ts index 6c827218..4c59df6e 100644 --- a/packages/app/cypress/e2e/url-params.cy.ts +++ b/packages/app/cypress/e2e/url-params.cy.ts @@ -43,6 +43,71 @@ describe('URL Parameter Persistence', () => { cy.get('.sidebar-legend').first().should('be.visible'); cy.get('.sidebar-legend').first().should('not.have.class', 'bg-accent'); }); + + it('preserves a legend subset when chart metrics change', () => { + visitWithDismissedModal('/inference'); + + cy.get('[data-testid="chart-legend"] input[type="checkbox"]:checked').should( + 'have.length.greaterThan', + 1, + ); + cy.get('[data-testid="chart-legend"] [role="button"][aria-label^="Hide "]') + .first() + .closest('li') + .find('input[type="checkbox"]') + .invoke('attr', 'id') + .then((hiddenInputId) => { + expect(hiddenInputId).to.be.a('string'); + expect(hiddenInputId).to.have.length.greaterThan(0); + const selector = `#${CSS.escape(hiddenInputId!)}`; + + cy.get(selector).parent().find('[role="button"][aria-label^="Hide "]').click(); + cy.get(selector).should('not.be.checked'); + + cy.get('[data-testid="yaxis-metric-selector"]').click({ force: true }); + cy.contains('[role="option"]', 'All-in Provisioned Joules per Total Token').click({ + force: true, + }); + cy.get(selector).should('not.be.checked'); + + cy.get('[data-testid="x-axis-mode-ttft"]').click(); + cy.get('[data-testid="x-axis-mode-ttft"]').should('have.attr', 'aria-selected', 'true'); + cy.get(selector).should('not.be.checked'); + }); + }); + + it('refreshes the automatic Best per SKU selection when the metric changes', () => { + visitWithDismissedModal('/inference'); + + cy.get('[data-testid="scatter-best-per-sku"]') + .should('have.attr', 'data-state', 'checked') + .then(() => + cy + .get('[data-testid="chart-legend"] ul input[type="checkbox"]:checked') + .then(($inputs) => [...$inputs].map((input) => input.id).toSorted()), + ) + .then((before) => { + cy.get('[data-testid="yaxis-metric-selector"]').click({ force: true }); + cy.contains( + '[role="option"]', + 'Cost per Million Total Tokens (Owning - Hyperscaler)', + ).click({ force: true }); + + cy.get('[data-testid="scatter-best-per-sku"]').should( + 'have.attr', + 'data-state', + 'checked', + ); + cy.get('[data-testid="x-axis-mode-ttft"]').click(); + cy.get('[data-testid="x-axis-mode-ttft"]').should('have.attr', 'aria-selected', 'true'); + cy.get('[data-testid="chart-legend"] ul input[type="checkbox"]:checked').then( + ($inputs) => { + const after = [...$inputs].map((input) => input.id).toSorted(); + expect(after, 'metric-specific Best per SKU winners').not.to.deep.equal(before); + }, + ); + }); + }); }); describe('Inference Y-axis metric', () => { diff --git a/packages/app/src/components/inference/InferenceContext.tsx b/packages/app/src/components/inference/InferenceContext.tsx index fb10ffa0..48d24a52 100644 --- a/packages/app/src/components/inference/InferenceContext.tsx +++ b/packages/app/src/components/inference/InferenceContext.tsx @@ -946,8 +946,12 @@ export function InferenceProvider({ }, [graphs, hwTypesWithData, selectedXAxisMode, selectedYAxisMetric]); const setBestPerSkuAndApply = useCallback( - (enabled: boolean) => { + (enabled: boolean, options?: { applySelection?: boolean }) => { setBestPerSku(enabled); + // Overlay-mode legend edits own a temporary unified selection. They can + // disable the automatic mode without replacing the context selection + // that should be restored when the overlay is dismissed. + if (options?.applySelection === false) return; const target = enabled ? bestHwTypes : selectableHwTypes; setActiveHwTypes(resolveHwSelection(target).result); setActivePresetId(null); @@ -1050,7 +1054,7 @@ export function InferenceProvider({ const precisionsKey = effectivePrecisions.join(','); const hwResetKey = `${selectedModel}|${effectiveSequence}|${precisionsKey}|${ isUnofficialRun ? 'preview' : 'official' - }|${selectedYAxisMetric}|${selectedXAxisMode}`; + }`; const lastHwResetKeyRef = useRef(''); // Restore legend-active selection from URL on first availability of @@ -1104,9 +1108,14 @@ export function InferenceProvider({ if (pendingHwFilterRef.current) return; if (pendingActiveHwTypes) return; if (selectableHwTypes.size === 0) return; - if (lastHwResetKeyRef.current === hwResetKey) return; - lastHwResetKeyRef.current = hwResetKey; + const scopeChanged = lastHwResetKeyRef.current !== hwResetKey; const presetFilter = presetHwFilterRef.current; + // Metric changes must preserve manual legend subsets, but automatic + // selections still need to follow the newly selected axes. In particular, + // Best per SKU is metric-aware and would otherwise keep the previous + // metric's winners while its toggle remained enabled. + if (!scopeChanged && !bestPerSku && !presetFilter) return; + lastHwResetKeyRef.current = hwResetKey; if (presetFilter) { const filtered = new Set( [...selectableHwTypes].filter((k) => matchesPresetHwFilter(k, presetFilter, selectedModel)), diff --git a/packages/app/src/components/inference/types.ts b/packages/app/src/components/inference/types.ts index b5554a68..97e3e97c 100644 --- a/packages/app/src/components/inference/types.ts +++ b/packages/app/src/components/inference/types.ts @@ -755,7 +755,7 @@ export interface InferenceChartContextType { selectAllHwTypes: () => void; /** Whether clean dashboard loads automatically keep the best configuration per physical SKU. */ bestPerSku: boolean; - setBestPerSku: (enabled: boolean) => void; + setBestPerSku: (enabled: boolean, options?: { applySelection?: boolean }) => void; /** Resolve automatic official + `overlay:` hardware selections under the active scope rule. */ resolveComparisonSelection: ( proposed: Set, diff --git a/packages/app/src/components/inference/ui/ScatterGraph.decoration.test.tsx b/packages/app/src/components/inference/ui/ScatterGraph.decoration.test.tsx index 57320c91..e9f45a25 100644 --- a/packages/app/src/components/inference/ui/ScatterGraph.decoration.test.tsx +++ b/packages/app/src/components/inference/ui/ScatterGraph.decoration.test.tsx @@ -22,8 +22,12 @@ vi.mock('@/lib/d3-chart/chart-setup', { spy: true }); vi.mock('@/lib/analytics', () => ({ track: vi.fn() })); vi.mock('next-themes', () => ({ useTheme: () => ({ resolvedTheme: 'dark' }) })); // The legend is React-rendered (covered elsewhere) — keep the tree light. +const legendState = vi.hoisted(() => ({ current: null as Record | null })); vi.mock('@/components/ui/chart-legend', () => ({ - default: ({ keyIndicators }: { keyIndicators?: React.ReactNode }) => keyIndicators ?? null, + default: (props: Record) => { + legendState.current = props; + return props.keyIndicators ?? null; + }, })); const inferenceState = vi.hoisted(() => ({ current: {} as Record })); @@ -218,6 +222,7 @@ beforeEach(() => { } as DOMRect); inferenceState.current = baseInferenceState(); overlayState.current = baseOverlayState(); + legendState.current = null; vi.mocked(setupChartStructure).mockClear(); }); @@ -442,6 +447,149 @@ describe('ScatterGraph toggle decoration', () => { unmount(); }); + it('refreshes official and overlay winners while Best per SKU is enabled', () => { + const setLocalOfficialOverride = vi.fn(); + const setActiveOverlayHwTypes = vi.fn(); + const officialPoints = [ + point('h100_vllm', 'fp8', 10, 10, 1), + point('h100_vllm', 'fp8', 20, 10, 2), + point('h100_trt', 'fp8', 10, 20, 1), + point('h100_trt', 'fp8', 20, 20, 2), + ]; + const runUrl = 'https://github.com/o/r/actions/runs/123'; + const overlayPoints = [ + point('b200_vllm', 'fp8', 10, 5, 1), + point('b200_vllm', 'fp8', 20, 5, 2), + point('b200_trt', 'fp8', 10, 15, 1), + point('b200_trt', 'fp8', 20, 15, 2), + ].map((entry) => ({ ...entry, run_url: runUrl })); + + inferenceState.current = { + ...baseInferenceState(), + activeHwTypes: new Set(['h100_vllm']), + hwTypesWithData: new Set(['h100_vllm', 'h100_trt']), + bestPerSku: true, + setBestPerSku: noop, + selectedYAxisMetric: 'y', + }; + overlayState.current = { + ...baseOverlayState(), + isUnofficialRun: true, + activeOverlayHwTypes: new Set(['b200_vllm']), + allOverlayHwTypes: new Set(['b200_vllm', 'b200_trt']), + localOfficialOverride: new Set(['h100_vllm']), + setLocalOfficialOverride, + setActiveOverlayHwTypes, + runIndexByUrl: { [runUrl]: 0 }, + unofficialRunInfos: [{ id: '123', branch: 'test-branch', url: runUrl }], + }; + + const { unmount } = mountChart({ + data: officialPoints, + chartDefinition: { + chartType: 'interactivity', + y_roofline: 'upper_right', + } as unknown as ChartDefinition, + overlayData: { + data: overlayPoints, + hardwareConfig: HARDWARE_CONFIG, + } as unknown as Parameters[0]['overlayData'], + }); + + expect(setLocalOfficialOverride).toHaveBeenCalledWith(new Set(['h100_trt'])); + expect(setActiveOverlayHwTypes).toHaveBeenCalledWith(new Set(['b200_trt'])); + unmount(); + }); + + it('falls back to the full official and overlay scopes when no best series is scoreable', () => { + const setLocalOfficialOverride = vi.fn(); + const setActiveOverlayHwTypes = vi.fn(); + const officialPoints = [ + point('h100_vllm', 'fp8', 0, 10, 1), + point('h100_trt', 'fp8', 0, 20, 1), + ]; + const runUrl = 'https://github.com/o/r/actions/runs/123'; + const overlayPoints = [ + point('b200_vllm', 'fp8', 0, 5, 1), + point('b200_trt', 'fp8', 0, 15, 1), + ].map((entry) => ({ ...entry, run_url: runUrl })); + + inferenceState.current = { + ...baseInferenceState(), + activeHwTypes: new Set(['h100_vllm']), + hwTypesWithData: new Set(['h100_vllm', 'h100_trt']), + bestPerSku: true, + setBestPerSku: noop, + selectedYAxisMetric: 'y', + }; + overlayState.current = { + ...baseOverlayState(), + isUnofficialRun: true, + activeOverlayHwTypes: new Set(['b200_vllm']), + allOverlayHwTypes: new Set(['b200_vllm', 'b200_trt']), + localOfficialOverride: new Set(['h100_vllm']), + setLocalOfficialOverride, + setActiveOverlayHwTypes, + runIndexByUrl: { [runUrl]: 0 }, + unofficialRunInfos: [{ id: '123', branch: 'test-branch', url: runUrl }], + }; + + const { unmount } = mountChart({ + data: officialPoints, + chartDefinition: { + chartType: 'interactivity', + y_roofline: 'upper_right', + } as unknown as ChartDefinition, + overlayData: { + data: overlayPoints, + hardwareConfig: HARDWARE_CONFIG, + } as unknown as Parameters[0]['overlayData'], + }); + + expect(setLocalOfficialOverride).toHaveBeenCalledWith(new Set(['h100_vllm', 'h100_trt'])); + expect(setActiveOverlayHwTypes).toHaveBeenCalledWith(new Set(['b200_vllm', 'b200_trt'])); + unmount(); + }); + + it('disables Best per SKU for overlay edits without applying a context selection', () => { + const setBestPerSku = vi.fn(); + const runUrl = 'https://github.com/o/r/actions/runs/123'; + const overlayPoints = [ + { ...point('h100', 'fp8', 30, 300, 2), run_url: runUrl }, + { ...point('h100', 'fp8', 35, 350, 4), run_url: runUrl }, + ]; + inferenceState.current = { + ...baseInferenceState(), + bestPerSku: true, + setBestPerSku, + }; + overlayState.current = { + ...baseOverlayState(), + isUnofficialRun: true, + activeOverlayHwTypes: new Set(['h100']), + allOverlayHwTypes: new Set(['h100']), + runIndexByUrl: { [runUrl]: 0 }, + unofficialRunInfos: [{ id: '123', branch: 'test-branch', url: runUrl }], + }; + + const { unmount } = mountChart({ + overlayData: { + data: overlayPoints, + hardwareConfig: HARDWARE_CONFIG, + } as unknown as Parameters[0]['overlayData'], + }); + const officialItem = legendState.current!.legendItems.find( + (item: { hw: string }) => item.hw === 'h100', + ); + + act(() => officialItem.onClick()); + expect(setBestPerSku).toHaveBeenLastCalledWith(false, { applySelection: false }); + + act(() => legendState.current!.onItemRemove('h100')); + expect(setBestPerSku).toHaveBeenLastCalledWith(false, { applySelection: false }); + unmount(); + }); + it('keeps speculative decoding out of unofficial-run point decorations', () => { const runUrl = 'https://github.com/o/r/actions/runs/123'; const overlayPoints = [ diff --git a/packages/app/src/components/inference/ui/ScatterGraph.tsx b/packages/app/src/components/inference/ui/ScatterGraph.tsx index 2baf295a..29c3ba5c 100644 --- a/packages/app/src/components/inference/ui/ScatterGraph.tsx +++ b/packages/app/src/components/inference/ui/ScatterGraph.tsx @@ -681,6 +681,36 @@ const ScatterGraph = React.memo( }, [setLocalOfficialOverride, setActiveOverlayHwTypes, mergeScopedOverlaySelection], ); + useEffect(() => { + if (!overlayData || !bestPerSku || overlayScopeChanged) return; + const direction = chartDefinition[`${selectedYAxisMetric}_roofline` as keyof ChartDefinition]; + if ( + direction !== 'upper_right' && + direction !== 'upper_left' && + direction !== 'lower_left' && + direction !== 'lower_right' + ) { + return; + } + const officialBest = bestSeriesPerSku(data, direction); + const overlayBest = bestSeriesPerSku(overlayData.data, direction); + const selection = new Set(officialBest.size > 0 ? officialBest : hwTypesWithData); + for (const key of overlayBest.size > 0 ? overlayBest : scopedOverlayHwTypes) { + selection.add(`overlay:${key}`); + } + if (!setsEqual(rawUnifiedSelection, selection)) commitUnifiedSelection(selection); + }, [ + overlayData, + bestPerSku, + overlayScopeChanged, + chartDefinition, + selectedYAxisMetric, + data, + hwTypesWithData, + scopedOverlayHwTypes, + rawUnifiedSelection, + commitUnifiedSelection, + ]); const unifiedToggle = useCallback( (key: string, isOverlay: boolean) => { const prefixedKey = isOverlay ? `overlay:${key}` : key; @@ -707,8 +737,15 @@ const ScatterGraph = React.memo( // When no overlay data, delegate to context's toggleHwType (preserves setActivePresetId) const handleToggleHwType = useCallback( - (key: string) => (overlayData ? unifiedToggle(key, false) : toggleHwType(key)), - [overlayData, unifiedToggle, toggleHwType], + (key: string) => { + if (!overlayData) { + toggleHwType(key); + return; + } + setBestPerSku(false, { applySelection: false }); + unifiedToggle(key, false); + }, + [overlayData, setBestPerSku, unifiedToggle, toggleHwType], ); // Legend "X" (remove) — same overlay split as handleToggleHwType. With an @@ -724,11 +761,12 @@ const ScatterGraph = React.memo( removeHwType(key); return; } + setBestPerSku(false, { applySelection: false }); const next = new Set(resolvedUnifiedSelection); next.delete(key); commitUnifiedSelection(next); }, - [overlayData, removeHwType, resolvedUnifiedSelection, commitUnifiedSelection], + [overlayData, setBestPerSku, removeHwType, resolvedUnifiedSelection, commitUnifiedSelection], ); // --- Theme ---