feat(replay): foreground line labels, stable anchors, and fixed axes#434
feat(replay): foreground line labels, stable anchors, and fixed axes#434cquil11 wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| const result: LayerConfig<InferenceData>[] = [rooflineLayer, scatterLayer]; | ||
| if (overlayLayer) result.push(overlayLayer); | ||
| result.push(speedOverlayLayer); | ||
| result.push(lineLabelForegroundLayer); |
There was a problem hiding this comment.
Labels raised before dot groups
Medium Severity
The new line-label-foreground layer calls raise() on .line-label during layer rendering, but D3Chart always runs selectAll('.dot-group').raise() immediately afterward in the same paint. That second pass moves scatter groups after the labels again, so line labels can still paint underneath points even though the layer is meant to keep them in the foreground.
Reviewed by Cursor Bugbot for commit 030a299. Configure here.
cquil11
changed the title
Inference performance scatter charts create line labels inside the rooflines layer, which renders before the dot groups so roofline paths stay behind the points. That left the labels painted under the scatter points and overlay marks. Add a trailing custom layer that re-raises every `.line-label` to the end of the zoomGroup after all layers render (and on zoom), so they always read as foreground — mirroring GPUGraph, whose line-label layer already renders last. `.raise()` only changes z-order; label placement and the existing de-overlap (hide-on-collision for interactivity, vertical nudge for TTFT/E2EL) are untouched, so labels still never overlap one another. Selects overlay line labels (`overlay-*`) too, so unofficial-run overlays get the same treatment. Adds an E2E assertion that visible labels follow the dot groups in DOM order. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In "Replay over time", line labels were re-placed every frame by the greedy placement algorithm, so they teleported between candidate positions (and the TTFT/E2EL vertical nudge reshuffled them) as the rooflines animated — visually noisy. Give each label a positional "affinity": add an opt-in `pinLineLabels` prop (set by ReplayPanel, like `transitionDuration`/`niceAxes`). When pinned, each label remembers a data-space x anchor the first time its series is seen and, on every later frame, resolves that anchor to the nearest current point on the line — so the label tracks the same spot as the line moves instead of hopping. The TTFT/E2EL de-overlap nudge is skipped while pinned so endpoint labels stay glued to their (smoothly moving) endpoints. Anchors are pruned when a series disappears. The static (non-pinned) chart is unchanged: the default branch of the shared placeLabel helper keeps the exact greedy-place + hide-on-collision behavior, preserving the no-overlap guarantee. Works for both official and overlay (`?unofficialrun=`) line labels. Extracts pointNearestX into its own module with unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cquil11
force-pushed
the
feat/line-labels-foreground
branch
from
50fb34f to
ed3e7a3
Compare
| // Stay visible across frames — positional stability is the goal | ||
| // during animation, so we don't hide on transient collisions. | ||
| lineLabels.push({ key, hw, label, color, x: px, y: py, visible: true }); | ||
| return; |
There was a problem hiding this comment.
Pinned labels ignored on zoom
Low Severity
pinLineLabels and lineLabelAnchorRef/pointNearestX apply only in the rooflines layer’s main render path. The same layer’s onZoom handler still recomputes interactivity labels with greedy placement and still runs TTFT/E2EL vertical nudge without checking pinLineLabels, so zooming during replay can make labels hop again.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ed3e7a3. Configure here.
By default the replay axes now stay fixed to the whole run's extent (for the active hardware) instead of refitting to each frame, so you can watch the frontier expand toward a constant coordinate space over time. A "Fixed axes" switch in the replay controls flips back to per-frame (dynamic) axes that hug the current frontier. - buildReplayTimeline: add computeFullRunDomain(timeline, hwFilter) — the bounding box across every step for the filtered configs. - ScatterGraph: add optional xExtentOverride/yExtentOverride; when set, the axis domain is based on them (normal padding + log/zero-baseline handling still applied) instead of the current points. Undefined for every non-replay caller, so the static chart is unchanged. - ReplayPanel: compute the full-run extent for the active hardware and pass it when "Fixed axes" is on (default); pass undefined when off. Unit tests cover computeFullRunDomain (spans all steps, respects the hw filter). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cquil11
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ac07c81. Configure here.
| // refitting each frame. Recomputed when the legend's hw filter changes. | ||
| const fixedExtent = useMemo( | ||
| () => (timeline ? computeFullRunDomain(timeline, (hw) => activeHwTypes.has(hw)) : null), | ||
| [timeline, activeHwTypes], |
There was a problem hiding this comment.
Fixed axes ignore legend override
Low Severity
computeFullRunDomain for fixed replay axes filters with activeHwTypes, while the embedded ScatterGraph plots and scales visible points using effectiveOfficialHwTypes (localOfficialOverride ?? activeHwTypes). After unofficial-run legend toggles set localOfficialOverride, fixed extents can include hardware the replay chart hides (or miss the visible set), so axes no longer match what is drawn.
Reviewed by Cursor Bugbot for commit ac07c81. Configure here.
1 participant
Improvements to inference performance chart line labels and the Replay over time view. Rebased on latest
master(preserves #426's per-precision line labels).1. Line labels always render in the foreground
Line labels are built in the rooflines layer (which renders before the dot groups), so they were painted under the scatter points/overlay marks. A trailing custom layer re-raises every
.line-labelto the top after all layers render (and on zoom), mirroring GPUGraph..raise()only changes z-order, so the existing label de-overlap is preserved.2. Positional "affinity" for line labels during replay
In replay, labels were re-placed every frame by the greedy algorithm, so they teleported between candidate positions. New
pinLineLabelsprop (set byReplayPanel): each label remembers a data-space x anchor the first time its series appears, then resolves it to the nearest current point each frame so it tracks its line instead of hopping. TTFT/E2EL endpoint labels skip the vertical de-overlap nudge while pinned. The static chart keeps its exact greedy + hide-on-collision behavior.3. Fixed axes across the whole run (with a toggle)
Previously the replay axes refit to each frame. Now, by default, the axes stay fixed to the whole run's extent (for the active hardware), so you can watch the frontier expand toward a constant coordinate space over time. A "Fixed axes" switch in the replay controls flips back to per-frame dynamic axes that hug the current frontier.
computeFullRunDomain(timeline, hwFilter)— bounding box across every step for the filtered configs.ScatterGraphgains optionalxExtentOverride/yExtentOverride; undefined for every non-replay caller, so the static chart is unchanged.Overlay support
Foreground raise + per-precision labels + affinity all apply to overlay (
?unofficialrun=…) line labels.Tests
line-labels.cy.ts): labels render in DOM order after the scatter points (foreground); plus master's per-precision test.pointNearestX(affinity anchor resolution) andcomputeFullRunDomain(spans all steps, respects hw filter).pnpm typecheck,pnpm lint,pnpm fmt, fullpnpm test:unit(2020 app tests) all pass.🤖 Generated with Claude Code
Note
Low Risk
Inference chart rendering and replay UX only; default scatter behavior is unchanged unless replay opts into pinning and fixed extents.
Overview
Line labels now paint in front of scatter points and overlays via a trailing D3 layer that
.raise()s every.line-labelafter other layers render (and on zoom). Cypress asserts labels follow dot groups in SVG DOM order.Replay gains smoother line labels and steadier axes:
ScatterGraphaccepts optionalpinLineLabels(replay enables it) so each label keeps a data-space x anchor resolved with newpointNearestX, skipping per-frame greedy placement and TTFT/E2EL vertical nudge while animating.computeFullRunDomaindrives optionalxExtentOverride/yExtentOverride; replay defaults to fixed axes with a Fixed axes toggle and analytics on change.Static charts stay on the existing greedy placement when
pinLineLabelsis off. Overlay line labels share the foreground raise and pinned path.Reviewed by Cursor Bugbot for commit ac07c81. Bugbot is set up for automated code reviews on this repo. Configure here.