Skip to content

Commit 8f38615

Browse files
fix(inference): stop stacking duplicate line labels per hardware (#470) (#504)
With a single precision selected, the scatter chart de-duplicates line labels to one per hardware type (e.g. the b300_sglang fp4/fp8/bf16 curves all collapse to a single "B300 (SGLang)" label). The render hides the duplicates via a per-label `visible` flag, but three visibility-sync code paths (legend hover, hover-end reset, filter-change effect) re-derived line-label opacity from hardware membership alone. Because every duplicate shares the same `data-hw-key`, all of them got re-shown — stacking three identical "B300 (SGLang)" labels on the curve. Persist the render's decision as a `data-visible` attribute and fold it into every opacity decision via new pure helpers in line-label-visibility.ts (unit-tested). Parallelism labels take the precision branch and are unaffected. (cherry picked from commit aff62bb) Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Bryan Shan <Oseltamivir@users.noreply.github.com>
1 parent 8f612d6 commit 8f38615

3 files changed

Lines changed: 157 additions & 16 deletions

File tree

packages/app/src/components/inference/ui/ScatterGraph.tsx

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef } from
77
import { GRADIENT_NUDGE_EVENT } from '@/lib/nudges/registry';
88
import { useInference } from '@/components/inference/InferenceContext';
99
import { pointNearestX } from '@/components/inference/ui/line-label-anchor';
10+
import {
11+
labelOpacityForActiveState,
12+
labelOpacityForHover,
13+
} from '@/components/inference/ui/line-label-visibility';
1014
import ChartLegend from '@/components/ui/chart-legend';
1115
import { useUnofficialRun } from '@/components/unofficial-run-provider';
1216
import { computeToggle } from '@/hooks/useTogglableSet';
@@ -676,9 +680,7 @@ const ScatterGraph = React.memo(
676680
.transition('legend-hover')
677681
.duration(150)
678682
.style('opacity', function () {
679-
const hw = (this as SVGGElement).dataset.hwKey;
680-
if (!hw) return 0;
681-
return hw === hwKey ? 1 : 0;
683+
return labelOpacityForHover((this as SVGGElement).dataset, hwKey);
682684
});
683685
},
684686
[isPointVisible, isRooflineVisible],
@@ -705,12 +707,11 @@ const ScatterGraph = React.memo(
705707
.transition('legend-hover')
706708
.duration(150)
707709
.style('opacity', function () {
708-
const hw = (this as SVGGElement).dataset.hwKey;
709-
const prec = (this as SVGGElement).dataset.precision;
710-
if (!hw) return 0;
711-
// Line labels have no precision attr — always visible if hw is active
712-
if (!prec) return effectiveActiveHwTypes.has(hw) ? 1 : 0;
713-
return effectiveActiveHwTypes.has(hw) && selectedPrecisions.includes(prec) ? 1 : 0;
710+
return labelOpacityForActiveState(
711+
(this as SVGGElement).dataset,
712+
effectiveActiveHwTypes,
713+
selectedPrecisions,
714+
);
714715
});
715716
}, [isPointVisible, isRooflineVisible, effectiveActiveHwTypes, selectedPrecisions]);
716717

@@ -1312,6 +1313,11 @@ const ScatterGraph = React.memo(
13121313
)
13131314
.attr('data-line-key', (d) => d.key)
13141315
.attr('data-hw-key', (d) => d.hw)
1316+
// Persist the per-label visibility decision (one label per hw group;
1317+
// de-duplicated / collision-hidden curves are `false`) so the
1318+
// visibility-sync effects below don't re-show hidden duplicates that
1319+
// share the same `data-hw-key`. See GH #470.
1320+
.attr('data-visible', (d) => (d.visible ? '1' : '0'))
13151321
.attr('transform', (d) => `translate(${d.x + 8},${d.y - 14})`)
13161322
.style('opacity', (d) => (d.visible ? 1 : 0))
13171323
.each(function (d) {
@@ -2124,13 +2130,11 @@ const ScatterGraph = React.memo(
21242130
zoomGroup
21252131
.selectAll<SVGGElement, unknown>('.parallelism-label, .line-label')
21262132
.style('opacity', function () {
2127-
const hw = (this as SVGGElement).dataset.hwKey;
2128-
const precision = (this as SVGGElement).dataset.precision;
2129-
if (!hw) return 0;
2130-
if (!precision) return ir.effectiveActiveHwTypes.has(hw) ? 1 : 0;
2131-
return ir.effectiveActiveHwTypes.has(hw) && ir.selectedPrecisions.includes(precision)
2132-
? 1
2133-
: 0;
2133+
return labelOpacityForActiveState(
2134+
(this as SVGGElement).dataset,
2135+
ir.effectiveActiveHwTypes,
2136+
ir.selectedPrecisions,
2137+
);
21342138
});
21352139

21362140
// Label placement (greedy collision layout) depends on the visible set,
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import {
4+
labelOpacityForActiveState,
5+
labelOpacityForHover,
6+
} from '@/components/inference/ui/line-label-visibility';
7+
8+
// Regression coverage for GH #470: when one hardware type contributes several
9+
// curves (e.g. b300_sglang_fp4 / _fp8 / _bf16) the render keeps a single line
10+
// label ("B300 (SGLang)") and hides the rest via `data-visible="0"`. Every
11+
// duplicate shares the same `data-hw-key`, so the visibility-sync paths must
12+
// honour `data-visible` or they re-show the hidden duplicates.
13+
14+
describe('labelOpacityForActiveState', () => {
15+
const active = new Set(['b300_sglang', 'gb300_dynamo-sglang']);
16+
const precisions = ['fp4'];
17+
18+
it('shows the kept line label for an active hardware type', () => {
19+
expect(
20+
labelOpacityForActiveState({ hwKey: 'b300_sglang', visible: '1' }, active, precisions),
21+
).toBe(1);
22+
});
23+
24+
it('keeps hidden duplicate line labels hidden even though their hw is active', () => {
25+
// The fp8 / bf16 B300 curves: same data-hw-key as the visible fp4 label,
26+
// but the render hid them. They must NOT be re-shown (the #470 bug).
27+
expect(
28+
labelOpacityForActiveState({ hwKey: 'b300_sglang', visible: '0' }, active, precisions),
29+
).toBe(0);
30+
});
31+
32+
it('hides line labels for inactive hardware types', () => {
33+
expect(
34+
labelOpacityForActiveState({ hwKey: 'h100_sglang', visible: '1' }, active, precisions),
35+
).toBe(0);
36+
});
37+
38+
it('treats a missing data-visible attribute as kept (parallelism labels)', () => {
39+
// Parallelism labels carry a precision and no data-visible gate.
40+
expect(
41+
labelOpacityForActiveState({ hwKey: 'b300_sglang', precision: 'fp4' }, active, precisions),
42+
).toBe(1);
43+
});
44+
45+
it('hides parallelism labels whose precision is not selected', () => {
46+
expect(
47+
labelOpacityForActiveState({ hwKey: 'b300_sglang', precision: 'fp8' }, active, precisions),
48+
).toBe(0);
49+
});
50+
51+
it('returns 0 when there is no hardware key', () => {
52+
expect(labelOpacityForActiveState({}, active, precisions)).toBe(0);
53+
});
54+
});
55+
56+
describe('labelOpacityForHover', () => {
57+
it('lights up the kept label for the hovered hardware', () => {
58+
expect(labelOpacityForHover({ hwKey: 'b300_sglang', visible: '1' }, 'b300_sglang')).toBe(1);
59+
});
60+
61+
it('does not re-show a hidden duplicate when its hardware is hovered', () => {
62+
expect(labelOpacityForHover({ hwKey: 'b300_sglang', visible: '0' }, 'b300_sglang')).toBe(0);
63+
});
64+
65+
it('hides labels for non-hovered hardware', () => {
66+
expect(labelOpacityForHover({ hwKey: 'h100_sglang', visible: '1' }, 'b300_sglang')).toBe(0);
67+
});
68+
69+
it('returns 0 when there is no hardware key', () => {
70+
expect(labelOpacityForHover({}, 'b300_sglang')).toBe(0);
71+
});
72+
});
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/**
2+
* Opacity decisions for the in-chart label overlays (line labels + parallelism
3+
* labels) rendered by `ScatterGraph`. Extracted as pure helpers so the
4+
* visibility invariant is unit-testable independent of the D3 render.
5+
*
6+
* Why this exists (GH #470): with a single precision selected the chart
7+
* de-duplicates line labels to **one per hardware type** — e.g. the
8+
* `b300_sglang_fp4`, `b300_sglang_fp8` and `b300_sglang_bf16` curves all share
9+
* the label text "B300 (SGLang)", so only one is kept (`visible: true`) and the
10+
* rest are hidden (`visible: false`). The render writes that decision to a
11+
* `data-visible` attribute on each `.line-label`.
12+
*
13+
* The bug: several visibility-sync code paths (legend hover, hover-end, and the
14+
* filter-change effect) re-derived line-label opacity from hardware membership
15+
* **alone**, ignoring `data-visible`. Because every duplicate carries the same
16+
* `data-hw-key` (e.g. `b300_sglang`), all of them got re-shown — stacking three
17+
* identical "B300 (SGLang)" labels on the curve. These helpers fold the
18+
* `data-visible` flag back into every decision so hidden duplicates stay hidden.
19+
*
20+
* Parallelism labels carry a `data-precision` attribute and intentionally have
21+
* no `data-visible` gate (multiple segments per curve are expected); they take
22+
* the precision branch and are unaffected.
23+
*/
24+
25+
export interface LabelAttrs {
26+
/** `data-hw-key` — base hardware key, shared across a hw's curves. */
27+
hwKey?: string;
28+
/** `data-precision` — set on parallelism labels, absent on line labels. */
29+
precision?: string;
30+
/** `data-visible` — `'1'`/`'0'`; only line labels set this. */
31+
visible?: string;
32+
}
33+
34+
/** True unless the render explicitly hid this label (`data-visible="0"`). */
35+
const renderKept = (attrs: LabelAttrs): boolean => attrs.visible !== '0';
36+
37+
/**
38+
* Opacity for a label while a legend row is hovered. Line/parallelism labels
39+
* whose hardware matches the hovered row light up — but a label the render hid
40+
* (a de-duplicated duplicate) stays hidden.
41+
*/
42+
export const labelOpacityForHover = (attrs: LabelAttrs, hoveredHwKey: string): 0 | 1 => {
43+
if (!attrs.hwKey) return 0;
44+
if (!renderKept(attrs)) return 0;
45+
return attrs.hwKey === hoveredHwKey ? 1 : 0;
46+
};
47+
48+
/**
49+
* Steady-state opacity (no hover): used by both the hover-end reset and the
50+
* filter-change sync effect. Line labels (no precision) show when their
51+
* hardware is active **and** the render kept them; parallelism labels show when
52+
* their hardware is active and their precision is selected.
53+
*/
54+
export const labelOpacityForActiveState = (
55+
attrs: LabelAttrs,
56+
activeHwTypes: ReadonlySet<string>,
57+
selectedPrecisions: readonly string[],
58+
): 0 | 1 => {
59+
const { hwKey, precision } = attrs;
60+
if (!hwKey) return 0;
61+
if (!precision) {
62+
return activeHwTypes.has(hwKey) && renderKept(attrs) ? 1 : 0;
63+
}
64+
return activeHwTypes.has(hwKey) && selectedPrecisions.includes(precision) ? 1 : 0;
65+
};

0 commit comments

Comments
 (0)