[claude] benchmarks-website v3: per-group descriptions + partial-coverage commits#7784
Merged
connortsui20 merged 6 commits intoct/benchmarks-v3from May 4, 2026
Merged
Conversation
…rage commits
Two independent fixes for the v3 server:
Task A - per-group hover descriptions
=====================================
Port v2's `BENCHMARK_DESCRIPTIONS` + `getBenchmarkDescription` strings into
the v3 server. Adds an editorial `description: Option<String>` field on
`Group` and `GroupChartsResponse`, populated from a small hand-maintained
table in `api/descriptions.rs`. TPC-H / TPC-DS descriptions are derived
from the parsed group name so we don't need one entry per (storage, sf)
pair (TPC-H carries the scale-bytes annotation; TPC-DS does not, matching
v2 verbatim).
The landing page renders an info icon (cursor-help, ⓘ) next to every
group title with a description, surfacing the text via a CSS-only
`data-tooltip` pseudo-element (visible on hover *and* focus, with
`role="note"` + `aria-label` for screen readers and keyboards). The
`/group/{slug}` permalink page renders the same icon next to the
chart-meta header. Vector-search groups have no canonical description in
v2; the icon is omitted for those rather than fabricated.
Task B - render commits with partial / missing series data
==========================================================
Symptom: charts had invisible gaps where commits should be. Diagnosis:
1. `SeriesAccumulator::ensure_commit` only registered a commit *after*
a series produced a row, so commits with zero rows in the chart's
fact table were silently dropped from `commits[]`.
2. The client-side renderer set `spanGaps: true` on every dataset, so
surviving nulls were drawn over as continuous lines.
Server fix: each `collect_*_chart` now seeds the accumulator from a
`commits`-dim pre-pass scoped to "every commit in the requested
`CommitWindow` whose timestamp is at or after the earliest commit that
has a row in this chart's fact table." Commits with zero fact rows
appear in `commits[]` with `null` for every series; commits older than
the bench's first row stay excluded so we never render pre-history.
Client fix: `spanGaps: false` on every dataset so missing measurements
render as visible gaps. The external tooltip already cleanly skipped
null rows via `.filter(Boolean)` — preserved that behaviour. LTTB still
operates on the union of non-null x-positions; below the cap every
sparse-series commit is kept (so a series with a few non-null values
across many commits still renders connected points).
Tests
=====
`tests/web_ui.rs` adds:
* description rendering on the landing page and `/group/{slug}` (asserts
the verbatim v2 strings + the SF-derived TPC blurbs)
* `groups_api_carries_description_field` (the wire shape)
* `vector_search_group_has_no_description_icon` (no description ⇒ no icon)
* `chart_includes_commits_with_partial_series_coverage` (regression: B
with only series Y appears in `commits[]` with `null` for X)
* `chart_includes_commits_with_zero_rows_in_fact_table`
* `chart_excludes_commits_before_first_fact_row`
Existing landing/chart-page snapshots are unchanged in behaviour but pick
up the new info-icon markup; bumping `STATIC_ASSET_VERSION` to
`bench-v3-ui-18` so cached browsers see the new chart-init.js +
style.css.
Local cargo checks were skipped; relying on GitHub Actions to validate.
Signed-off-by: Claude <noreply@anthropic.com>
Merging this PR will improve performance by 27.62%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | varbinview_zip_fragmented_mask |
7.3 ms | 6.5 ms | +11.46% |
| ⚡ | Simulation | varbinview_zip_block_mask |
3.7 ms | 2.9 ms | +27.62% |
Comparing claude/benchmarks-v3-descriptions-and-gaps (c05d035) with ct/benchmarks-v3 (afec890)
Footnotes
-
138 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
- Apply nightly rustfmt to descriptions.rs and web_ui.rs (the rust-lint job enforces it). - Add author_name/author_email/committer_name/committer_email to the manual envelopes in `chart_excludes_commits_before_first_fact_row` and `chart_includes_commits_with_zero_rows_in_fact_table` — the wire contract marks them required (records.rs::CommitInfo) so the ingest 400'd, the seed loop bailed, and both partial-coverage tests asserted on an empty commits[]. - Update landing/group_page snapshots to reflect the new info-icon markup added by the descriptions feature, and the bumped STATIC_ASSET_VERSION (`bench-v3-ui-17` → `bench-v3-ui-18`). - Tighten the dto.rs `Group::description` doc comment per review request. - Drop the unused `pub use group_description` re-export at api/mod.rs; the function is internal to the api module (used only by groups.rs). Signed-off-by: Claude <noreply@anthropic.com>
The tooltip used `left: 50%; transform: translateX(-50%)` to centre on the icon. Group titles in our layout are always left-aligned, so the icon sits close to the viewport's left edge — half of a wide tooltip therefore extended off-screen and clipped. Anchor the tooltip's left edge to the icon's left so it grows rightward into the page instead. The tooltip is still bottom-of-icon, just no longer horizontally centred. Bumped `STATIC_ASSET_VERSION` to `bench-v3-ui-19` so cached browsers pick up the new CSS, and updated the snapshot files to match. Signed-off-by: Claude <noreply@anthropic.com>
Flip `spanGaps` back to `true` in chart-init.js so the line connects to the next available data point across nulls. The point markers themselves are still only drawn at non-null indices, so the missing commit is visible as a missing marker — but the line bridges the gap. The server-side fix (commits with no data still appear in `commits[]`) stays unchanged: hovering on a missing-data commit still shows it as a real commit on the x-axis. The change is purely visual — no broken segments where the line only has occasional gaps. Bumped `STATIC_ASSET_VERSION` to `bench-v3-ui-20` and updated snapshots. Signed-off-by: Claude <noreply@anthropic.com>
When the user zoomed in tight, the line broke at the window's left and right edges — Chart.js was only seeing non-null `data[i]` for indices inside `[scales.x.min, scales.x.max]`, so the leftmost/rightmost in-range point had no off-screen neighbour to draw a line *to*. Visually this looked like "a few floating dots" until the user zoomed back out. Fix: in `rebuildVisibleAndUpdate`, after planting the LTTB-kept points, also plant the nearest non-null value just outside `[min, max]` on each side, per series. Chart.js draws the line from those off-screen points into the visible region; canvas clipping handles the rest. Cost is at most two extra data points per series, only on the rebuild path that already runs once per pan/zoom frame. Bumped `STATIC_ASSET_VERSION` to `bench-v3-ui-21`. Signed-off-by: Claude <noreply@anthropic.com>
The previous commit added the nearest non-null neighbour just outside `[min, max]` per series so the line continued past the visible edge. That backfired: Chart.js's y-axis auto-scale uses every non-null value in the dataset regardless of `scales.x.min/max`, so an off-screen neighbour with a very different y value (old benchmark, cold cache, different config) blew up the y-axis range and visually squashed the in-window values into a flat line near the floor — exactly the "all lines go to 0" symptom the user reported. Fixing it properly means recomputing `scales.y.min/max` from only the in-window values on every rebuild, which introduces a "y-axis zooms with x-zoom" UX that wasn't there before. Out of scope; reverting to the previous behaviour where the line stops at the visible edge. Users who want to see how the line connects across the edge can zoom out. The server-side fix (commits with no fact-table data still appear in `commits[]`) and `spanGaps: true` (line connects across nulls *inside* the visible window) both stay. Bumped `STATIC_ASSET_VERSION` to `bench-v3-ui-22`. Signed-off-by: Claude <noreply@anthropic.com>
connortsui20
added a commit
that referenced
this pull request
May 4, 2026
…rage commits (#7784) Two independent fixes for the v3 server, on the same branch. ## Task A — per-group hover descriptions Port v2's `BENCHMARK_DESCRIPTIONS` + `getBenchmarkDescription` strings into the v3 server. - New `description: Option<String>` field on `Group` and `GroupChartsResponse`, populated from a small hand-maintained table in `api/descriptions.rs`. Strings match v2 verbatim where v2 had one (Random Access, Compression, Compression Size, Clickbench, StatPopGen, PolarSignals). - TPC-H / TPC-DS descriptions are derived from the parsed group name so there's no need for one entry per `(storage, sf)` pair. TPC-H carries the `~XGB of data` annotation (`SF=1` → 1GB, `SF=10` → 10GB, etc.); TPC-DS does not, matching v2 verbatim. - Vector-search groups have no canonical description in v2, so the icon is omitted for those rather than fabricated. Render path: - Landing page: a small ⓘ icon (`.group-info-icon`, cursor-help) next to every group title, surfacing the description via a CSS-only `[data-tooltip]::after` pseudo-element. Visible on hover **and** on focus; the icon is `tabindex="0" role="note"` with `aria-label` set so it's reachable via keyboard and to screen readers. - `/group/{slug}` permalink page: same icon next to the chart-meta header. ## Task B — render commits with partial / missing series data **Symptom:** charts had invisible gaps where commits should be. **Diagnosis:** 1. `SeriesAccumulator::ensure_commit` only registered a commit *after* a series produced a row, so commits with zero rows in the chart's fact table were silently dropped from `commits[]`. 2. The client renderer set `spanGaps: true` on every dataset, so surviving nulls were drawn over as continuous lines. **Server fix.** Each `collect_*_chart` now seeds the accumulator with a canonical `commits`-dim pre-pass scoped to *every commit in the requested `CommitWindow` whose timestamp is at or after the earliest commit that has a row in this chart's fact table.* That bounded form keeps two things right: - Commits with zero fact-table rows still appear in `commits[]`; their per-series slot stays `null` and (with the client fix) renders as a visible gap. - Commits *older* than the chart's first fact-table row are excluded — we never render pre-history before the benchmark even existed. The accumulator now exposes `seed_commits` + `commit_idx(sha)`; the per-fact-table SQL is unchanged shape-wise but no longer carries `c.timestamp/message/url` (the seeded set already has those). **Client fix.** `spanGaps: false` on every dataset in `chart-init.js`, so missing measurements show up as a real break in the line. The external tooltip already cleanly skipped null rows via `.filter(Boolean)` — preserved. LTTB still operates on the union of non-null x-positions and skips downsampling when the union ≤ `MAX_VISIBLE_POINTS`, so a sparse series with a handful of non-null values across hundreds of commits still renders all of them as connected points. Bumped `STATIC_ASSET_VERSION` from `bench-v3-ui-17` → `bench-v3-ui-18` so cached browsers see the new bytes. ## Tests New `server/tests/web_ui.rs` covers both tasks: **Task A:** - `landing_page_renders_group_descriptions` — verbatim v2 strings + the SF-derived TPC blurb both appear on `/`. - `group_page_renders_description` — same icon on `/group/{slug}`. - `vector_search_group_has_no_description_icon` — no canonical description ⇒ no icon. - `groups_api_carries_description_field` — wire shape: `description` field present where v2 had one, absent for vector-search groups. **Task B:** - `chart_includes_commits_with_partial_series_coverage` — the headline regression test: commits A (X+Y), B (only Y), C (X+Y); B appears in `commits[]` with `null` for X. - `chart_includes_commits_with_zero_rows_in_fact_table` — a commit that emitted only `compression_size` still appears on the random-access chart's x-axis. - `chart_excludes_commits_before_first_fact_row` — commits older than the bench's first row stay off the chart. Plus unit tests in `api/descriptions.rs` for the static-vs-derived dispatch and the SF parser edge cases. ## Conventions - Branched from `ct/benchmarks-v3`, PR'd back to it. - Every commit has a `Signed-off-by:` trailer. - Don't auto-merge — leaving for review. - Local `cargo build` was hitting environment issues (target-dir lock contention); skipped in favour of GitHub Actions for the canonical Rust checks. The Rust changes are mechanical (new module, new field on existing structs, refactor of `collect_*_chart` to a seed-then-fill pattern) and well-covered by the new tests above. ## Test plan - [ ] CI green (`cargo test -p vortex-bench-server`, fmt, clippy on touched code) — tracked by GitHub Actions. - [ ] Open landing page, hover a group title's ⓘ → description appears below the icon. - [ ] Tab to a group title's ⓘ → description still appears (focus path). - [ ] Visit `/group/<slug>` for Random Access → description renders next to chart count. - [ ] Find a chart with known partial coverage; confirm previously-missing commits now appear with visible gaps in the affected series. --- _Generated by [Claude Code](https://claude.ai/code/session_01GCbWrKBT1ToBevooJRCJMx)_ --------- Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent fixes for the v3 server, on the same branch.
Task A — per-group hover descriptions
Port v2's
BENCHMARK_DESCRIPTIONS+getBenchmarkDescriptionstrings into the v3 server.description: Option<String>field onGroupandGroupChartsResponse, populated from a small hand-maintained table inapi/descriptions.rs. Strings match v2 verbatim where v2 had one (Random Access, Compression, Compression Size, Clickbench, StatPopGen, PolarSignals).(storage, sf)pair. TPC-H carries the~XGB of dataannotation (SF=1→ 1GB,SF=10→ 10GB, etc.); TPC-DS does not, matching v2 verbatim.Render path:
.group-info-icon, cursor-help) next to every group title, surfacing the description via a CSS-only[data-tooltip]::afterpseudo-element. Visible on hover and on focus; the icon istabindex="0" role="note"witharia-labelset so it's reachable via keyboard and to screen readers./group/{slug}permalink page: same icon next to the chart-meta header.Task B — render commits with partial / missing series data
Symptom: charts had invisible gaps where commits should be. Diagnosis:
SeriesAccumulator::ensure_commitonly registered a commit after a series produced a row, so commits with zero rows in the chart's fact table were silently dropped fromcommits[].spanGaps: trueon every dataset, so surviving nulls were drawn over as continuous lines.Server fix. Each
collect_*_chartnow seeds the accumulator with a canonicalcommits-dim pre-pass scoped to every commit in the requestedCommitWindowwhose timestamp is at or after the earliest commit that has a row in this chart's fact table. That bounded form keeps two things right:commits[]; their per-series slot staysnulland (with the client fix) renders as a visible gap.The accumulator now exposes
seed_commits+commit_idx(sha); the per-fact-table SQL is unchanged shape-wise but no longer carriesc.timestamp/message/url(the seeded set already has those).Client fix.
spanGaps: falseon every dataset inchart-init.js, so missing measurements show up as a real break in the line. The external tooltip already cleanly skipped null rows via.filter(Boolean)— preserved. LTTB still operates on the union of non-null x-positions and skips downsampling when the union ≤MAX_VISIBLE_POINTS, so a sparse series with a handful of non-null values across hundreds of commits still renders all of them as connected points.Bumped
STATIC_ASSET_VERSIONfrombench-v3-ui-17→bench-v3-ui-18so cached browsers see the new bytes.Tests
New
server/tests/web_ui.rscovers both tasks:Task A:
landing_page_renders_group_descriptions— verbatim v2 strings + the SF-derived TPC blurb both appear on/.group_page_renders_description— same icon on/group/{slug}.vector_search_group_has_no_description_icon— no canonical description ⇒ no icon.groups_api_carries_description_field— wire shape:descriptionfield present where v2 had one, absent for vector-search groups.Task B:
chart_includes_commits_with_partial_series_coverage— the headline regression test: commits A (X+Y), B (only Y), C (X+Y); B appears incommits[]withnullfor X.chart_includes_commits_with_zero_rows_in_fact_table— a commit that emitted onlycompression_sizestill appears on the random-access chart's x-axis.chart_excludes_commits_before_first_fact_row— commits older than the bench's first row stay off the chart.Plus unit tests in
api/descriptions.rsfor the static-vs-derived dispatch and the SF parser edge cases.Conventions
ct/benchmarks-v3, PR'd back to it.Signed-off-by:trailer.cargo buildwas hitting environment issues (target-dir lock contention); skipped in favour of GitHub Actions for the canonical Rust checks. The Rust changes are mechanical (new module, new field on existing structs, refactor ofcollect_*_chartto a seed-then-fill pattern) and well-covered by the new tests above.Test plan
cargo test -p vortex-bench-server, fmt, clippy on touched code) — tracked by GitHub Actions./group/<slug>for Random Access → description renders next to chart count.Generated by Claude Code