Skip to content

Interactive-chart UX benchmark harness - #357

Merged
Alek99 merged 23 commits into
mainfrom
alek/ux-benchmark
Jul 28, 2026
Merged

Interactive-chart UX benchmark harness#357
Alek99 merged 23 commits into
mainfrom
alek/ux-benchmark

Conversation

@Alek99

@Alek99 Alek99 commented Jul 27, 2026

Copy link
Copy Markdown
Member

Measures what a user actually experiences on a live interactive chart across data sizes: how long until every point is on screen, how the chart behaves during a zoom, and how long after the gesture until the picture is final.

Draft: the harness is validated end-to-end at 100k and 10M, but no numbers are published yet — see Not in this PR below.

Why this exists

The existing launch suite measures a fixed output contract (build → PNG/HTML). It cannot answer "how many points can you actually work with", because it never touches the chart after first paint. This harness does, and holds every arm to one contract.

The five arms

Every arm is a real interactive deployment, driven through its own input path — no synthetic API calls.

Arm What runs Zoom input
xy production ChartView over a WebSocket bridging channel.handle_message; deep zoom drills to exact rows real WheelEvent
xy-exact same host, density=False — one marker per row, no aggregation credit real WheelEvent
plotly px.scatter HTML with scrollZoom real WheelEvent
matplotlib WebAgg server + mpl.js Zoom-tool rubber-band drags
datashader hvPlot/HoloViews on a Bokeh server, datashade=True + resample_when real WheelEvent

The standalone HTML export is deliberately not measured: its kernel-less client cannot drill to exact rows, so it does not meet the bar the other arms are held to.

The measurement contract

The clock stops only on a frame that is both correct and stable. Correct = the scripted target domain plus sentinel rows (planted at known coordinates during data generation) verified lit at their projected screen positions. Stable = full readback surface byte-identical for 10 consecutive frames.

Neither alone is sufficient, and this is not theoretical: stability alone passes a stale raster while a server is still computing — exactly the server arms' failure mode. Correctness alone passes a frame still being painted. render_incorrect and render_unstable are distinct failures.

Other rules:

  • Fixed wall-clock input schedule (42 inputs at 33 ms), never paced by the library's frame rate, so a slow arm falls behind honestly.
  • zoom_to_final_ms is measured directly, not summed. A p95 is one frame, not a duration; adding it to settle would double-count the last gesture frame.
  • Both memory pools, both peaks. Host trees polled every 50 ms from spawn so build transients count; Plotly's to_html build runs in its own child so its peak is attributable rather than charged to the harness.
  • first_frame_ms published beside visible_complete_ms — the gap measures how much a naive first-paint benchmark would have flattered a progressive renderer.

Verification screenshots are captured at every clock stop, so any published number can be eyeballed against the frame it came from.

Validation

All five arms green at 100k; four at 10M plus datashader at 10M. Two 10M failures were real bugs, both fixed: my WebSocket framing packed buffers without alignment padding (the client's decoder correctly rejected it at 10M), and the WebAgg arm drew rubber-band boxes against stale projections while the server was still redrawing.

Docs

The README benchmark section is replaced with the 10k-100M interactive ladder: two charts generated by plot_ux.py from the run JSONs, plus the full render-time table. No number in that section is hand-typed.

Not in this PR

  • ceiling-results.json stays uncommitted: its xy and datashader cells came from proxy configurations (HTML export, bare raster) that this harness supersedes.
  • Docs beyond the README still quote the launch baseline (docs/index.md, docs/overview/benchmarks.md, the docs-app demos, spec/benchmarks/results.md). They need reconciling with the README in a follow-up.
  • Repetitions per cell and the full ladder run — the interesting divergence is at 2.5M+.
  • /oracle count-equality check in the settle phase (the host serves it; the probe does not yet call it).
  • verify_benchmark_report.py kind for these artifacts, and the spec updates under spec/benchmarks/.

Upstream behaviors documented in benchmarks/README.md

Each cost a debugging cycle and is load-bearing: mpl.js assigns to the read-only button.classList (silent no-op, so toolbar buttons have no classes) and silently discards drags with a corner outside the axes bbox; Bokeh's websocket origin allowlist 403s 127.0.0.1 when it expects localhost; Bokeh 3 renders into shadow DOM; holoviews' resample_when renders nothing until a client range event arrives.

Summary by CodeRabbit

  • New Features
    • Added an Interactive ceiling benchmark suite that measures interactive limits with stability (“correct-then-stable”) and peak-memory tracking, plus automatic charting and Markdown summaries.
    • Added an Interactive UX benchmark suite covering multiple rendering modes, oracle verification, and synchronized screenshot/video grid outputs.
  • Documentation
    • Updated benchmark documentation with new live-interaction guidance and detailed measurement/interpretation rules.
  • Chores
    • Introduced a manually-triggered CI workflow to run the ceiling benchmark and publish results/artifacts.
  • Tests
    • Added coverage to ensure benchmark UX hosts use safe server port binding.

Measures what a user experiences on a live interactive chart across data
sizes: time until every point is on screen, behavior during a zoom, and
time from the last input until the picture is final. Five arms, each a
real interactive deployment driven through its own input path.

The measurement contract, in `_ux_probe.py`:

- The clock stops only on a frame that is both semantically correct and
  pixel-stable. Correct means the scripted target domain plus sentinel
  rows -- planted at known coordinates during data generation -- verified
  lit at their projected screen positions. Stable means the full readback
  surface byte-identical for 10 consecutive frames. Stability alone would
  pass a stale raster while a server is still computing; correctness alone
  would pass a frame still being painted. `render_incorrect` and
  `render_unstable` are distinct failures.
- Inputs replay on a fixed wall-clock schedule, never paced by the
  library's frame rate, so a slow arm falls behind honestly.
- Load and interaction are separate clocks, and `zoom_to_final_ms` is
  measured directly rather than summed from a p95 and a duration.
- Both memory pools report peaks: host trees are polled every 50 ms from
  spawn so build transients count, and Plotly's `to_html` build runs in
  its own child so its peak is attributable.

Arms: xy (production ChartView over a WebSocket bridging
channel.handle_message, so deep zoom drills to exact rows), xy-exact
(density=False), plotly (scrollZoom), matplotlib (WebAgg + mpl.js
rubber-band drags), datashader (hvPlot/HoloViews on a Bokeh server).
Verification screenshots are captured at each clock stop.

Also includes `bench_ceiling.py` and its summarizer/plotter, plus a
manually-triggered workflow for the macOS large runner.
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b63a4908-00ad-423b-bf54-66108b2cd661

📥 Commits

Reviewing files that changed from the base of the PR and between d5376a9 and 1ceedeb.

⛔ Files ignored due to path filters (1)
  • spec/assets/ux-render-time.png is excluded by !**/*.png
📒 Files selected for processing (5)
  • README.md
  • benchmarks/_ux_datashader_host.py
  • benchmarks/_ux_webagg_host.py
  • benchmarks/plot_ux.py
  • tests/test_benchmark_environment.py

📝 Walkthrough

Walkthrough

Adds interactive ceiling and UX benchmark suites for multiple renderers, including isolated hosts, browser probes, memory tracking, report generation, charts, video artifacts, CI execution, tests, and updated benchmark documentation.

Changes

Interactive benchmark suites

Layer / File(s) Summary
Ceiling sweep harness
benchmarks/bench_ceiling.py
Runs isolated rendering arms across size ladders, validates rendering semantics, measures visibility and gestures, enforces resource limits, and writes structured reports.
UX hosts and measurement contract
benchmarks/_ux_*.py, benchmarks/_ux_probe.py
Provides live, WebAgg, and datashader hosts plus shared correctness, stability, gesture, and settle measurement logic.
UX arm adapters and orchestration
benchmarks/bench_ux.py
Runs renderer-specific browser adapters, tracks process memory, captures measurements, and optionally records screenshots and screencasts.
Artifacts and reporting
benchmarks/make_ux_grid.py, benchmarks/plot_*.py, benchmarks/summarize_*.py, benchmarks/run_ux_suite.sh
Assembles videos, plots benchmark metrics, summarizes ceilings and timing, and runs the UX size ladder.
Workflow and documentation
.github/workflows/ceiling-benchmark.yml, README.md, benchmarks/README.md, tests/test_benchmark_environment.py
Adds manually dispatched ceiling automation, validates atomic port binding, and documents benchmark methods, outputs, renderer behavior, and results.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Runner
  participant bench_ux.py
  participant ChartHost
  participant Chromium
  participant ReportTools
  Runner->>bench_ux.py: start UX benchmark for size and arm
  bench_ux.py->>ChartHost: launch renderer host
  ChartHost-->>Chromium: serve interactive chart
  bench_ux.py->>Chromium: inject adapter and UX probe
  Chromium-->>bench_ux.py: return UX_BENCH or UX_ERROR
  bench_ux.py-->>ReportTools: write JSON results and video frames
  ReportTools-->>Runner: generate summaries, charts, and MP4 artifacts
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly captures the main change: adding an interactive-chart UX benchmark harness.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch alek/ux-benchmark

Comment @coderabbitai help to get the list of available commands.

Post-load CDP injection floored every fast arm's visible-complete at
"when the probe started looking": xy's first observed frame was already
complete and stable, and its number carried ~120 ms of target discovery,
readyState polling, and script compilation that belonged to the harness,
not the chart. Register the probe with
Page.addScriptToEvaluateOnNewDocument from about:blank and navigate
after, so the clock observes every page from its first moment. Measured
at 100k: xy 192 -> 70.5 ms, matplotlib 245 -> 154, plotly 493 -> 392,
datashader 1096 -> 937 — a uniform ~90-160 ms of harness latency removed
from all five arms.
@codspeed-hq

codspeed-hq Bot commented Jul 27, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 103 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing alek/ux-benchmark (1ceedeb) with main (fa422a9)

Open in CodSpeed

Footnotes

  1. 2 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.

Alek99 added 7 commits July 27, 2026 16:41
A benchmark that cannot be watched cannot be checked, so screencast
recording is now on by default for every arm rather than behind a flag,
and the synced grid video is assembled automatically at the end of a
sweep.

Recording starts at navigation and stops the moment that arm's chart is
rendered (visible-complete). The video is therefore the load race -- a
panel ending is that library finishing -- and the gesture and settle
clocks carry no screencast cost at all.

Screencast overhead on the load clock is measured, not assumed: across
three recorded and three unrecorded runs of all five arms at 100k, the
difference in visible_complete_ms ranged -5.9 ms to +5.7 ms, smaller
than each arm's own run-to-run spread. `--no-record` and `--no-grid`
remain for debugging.

The frames also independently corroborate the probe: on a recorded 100k
run, four of five arms placed visible_complete_ms inside the bracket
between the last blank captured frame and the first fully-drawn one --
a check that shares no clock with the probe, since frame timestamps come
from the driver and the probe's from performance.now().
Every panel is now its own stopwatch: the timer runs blue while that chart
is loading and freezes green on the arm's render time the moment it is up,
so the video reads as a race with five finish lines rather than one shared
clock nobody crosses.

The frozen figure is the measured visible_complete_ms, read from the run's
own JSON via --report rather than inferred from frame timestamps, so the
number on screen is the number the benchmark recorded and the two cannot
drift apart. It falls back to the last captured frame when no report is
given, which is the same moment since recording stops at visible-complete.

Grid assembly moved after the report is written, since it now consumes it.
run_ux_suite.sh drives the full ladder one size per invocation, smallest
first. Per-size invocation means each size's video and JSON land the moment
that size finishes rather than after the whole suite, a size that hangs
cannot take the rest of the ladder with it, and timeouts scale with N
instead of applying one worst-case value everywhere. Grid PNG staging is
cleaned per size; the mp4 is the artifact.

summarize_ux.py re-derives every table from a directory of reports, and
reads partial ladders cleanly so a run in progress can be inspected.
Failed cells print their status rather than blanking, since a failure at a
given size is a result.
The section quoted a static-PNG export time at a single size (10M). That
number is real but answers a narrow question -- how fast one file gets
written -- while the claim people actually care about is what a live
interactive chart does as data grows.

Both charts come from the 10k-100M sweep in bench_ux.py: every library gets
every row, driven through its own input path in a real browser, with the
clock stopping only on a frame that is both correct (planted sentinels
verified lit at their projected positions) and stable (drawing buffer
byte-identical for 10 frames). A progressive renderer is therefore charged
until its last chunk lands rather than at first paint.

Linear y with fixed domains: a log axis compresses a 165x difference into a
couple of gridlines, and an axis derived from the data would move whenever
an arm or a value changed, making two versions of the same chart silently
incomparable. x stays log -- the ladder spans four decades.

The pale XY line (density=False) is deliberately kept in both charts. It is
the same engine with no aggregation credit, and publishing it is what makes
the flat default line legible as a design choice rather than a trick.

Charts are generated by plot_ux.py straight from the run JSONs, so no
number in this section is hand-typed.
The ceilings table compressed ten measured sizes into two columns, and one
of its cells was actively misleading: Matplotlib's frame p95 reads 9.4 ms at
every size because the browser is smoothly displaying a stale raster while
the server re-rasterizes, so an 'interactive to 50M' entry would have been
derived from our own metric rather than from anything a user would feel.

The full table shows every measured cell instead, so the shape of each
curve is legible without trusting a threshold we chose. Failures are marked
and explained rather than summarized away, and the single-run, single-machine
caveat is stated inline.
@Alek99
Alek99 marked this pull request as ready for review July 28, 2026 00:23

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 14

🧹 Nitpick comments (5)
benchmarks/_ux_live_host.py (1)

49-61: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Sentinel planting assumes n >= len(SENTINELS).

--n 3 raises IndexError out of make_data, and the probe contract silently degrades for any n close to 5. A guard makes the requirement explicit for both this host and the WebAgg/datashader hosts that import it.

🔧 Guard
     rng = np.random.default_rng(SEED)
+    if n < len(SENTINELS):
+        raise ValueError(f"n must be >= {len(SENTINELS)} to plant sentinels")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@benchmarks/_ux_live_host.py` around lines 49 - 61, Update make_data to
validate that n is at least len(SENTINELS) before planting sentinel rows, and
fail explicitly with a clear error when the requirement is not met. Preserve the
existing data generation and sentinel replacement behavior for valid n values.
benchmarks/plot_ux.py (1)

71-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Naming nit: series returns x-values, not labels.

labels, values, stopped = series(...) shadows the categorical meaning the name implies while these are numeric row counts on a log axis. Renaming to xs matches the function's own contract.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@benchmarks/plot_ux.py` around lines 71 - 79, The loop in the plotting flow
should rename the first value returned by series from labels to xs, and update
the corresponding xy.line and xy.marker references. Preserve the existing
behavior and use xs to reflect that these are numeric x-values.
benchmarks/run_ux_suite.sh (1)

16-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Hardcoded .venv/bin/python with no existence check.

Every size fails opaquely (No such file or directory swallowed by the grep pipeline) when the venv lives elsewhere. A one-line guard, or honoring ${PYTHON:-...}, makes this portable.

🔧 Guard
-PY="$(cd "$(dirname "$0")/.." && pwd)/.venv/bin/python"
+PY="${PYTHON:-$(cd "$(dirname "$0")/.." && pwd)/.venv/bin/python}"
+[ -x "$PY" ] || { echo "python not found at $PY (set PYTHON=...)" >&2; exit 1; }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@benchmarks/run_ux_suite.sh` around lines 16 - 21, Update the PY
initialization in run_ux_suite.sh to honor an existing PYTHON override or
otherwise select the default virtual-environment interpreter, and add an
existence check before invoking the benchmark so missing interpreters fail with
a clear error instead of being swallowed by the grep pipeline.
benchmarks/bench_ceiling.py (1)

512-520: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

--repetitions 0 crashes with IndexError on attempts[-1].

The loop body never runs, so attempts is empty. Either clamp the argument (max(1, args.repetitions)) or validate it in main().

Also applies to: 616-618

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@benchmarks/bench_ceiling.py` around lines 512 - 520, Validate repetitions in
main() or clamp it to at least one before the benchmarking loop so --repetitions
0 cannot leave attempts empty and trigger attempts[-1] IndexError. Preserve the
existing loop and result handling for positive repetition counts.
benchmarks/make_ux_grid.py (1)

106-125: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Single-entry cache thrashes: every panel re-decodes and resizes its JPEG on every output frame.

cache.clear() on each miss means the cache holds one image, but the inner loop cycles through all arms per frame, so each panel always misses. With ~5 arms and thousands of frames this is thousands of redundant decode+resize operations. Key the cache per arm instead.

♻️ Per-arm cache
-                key = str(path)
-                if key not in cache:
-                    cache.clear()
-                    cache[key] = Image.open(path).convert("RGB").resize(cell)
-                canvas.paste(cache[key], (x0, y0 + label_h))
+                if cache.get(name + "_path") != path:
+                    cache[name + "_path"] = path
+                    cache[name] = Image.open(path).convert("RGB").resize(cell)
+                canvas.paste(cache[name], (x0, y0 + label_h))

(adjust the dict type hint accordingly, or use two dicts.)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@benchmarks/make_ux_grid.py` around lines 106 - 125, Update the image cache in
the grid-generation loop to retain entries for every arm instead of clearing it
on each cache miss. In the cache handling around `cache`, `key`, and
`Image.open`, use the path key per arm so previously decoded and resized images
are reused across panels and output frames, and adjust the cache type annotation
if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ceiling-benchmark.yml:
- Around line 39-42: Update the memory_gib workflow input and its handling
alongside benchmarks/bench_ceiling.py’s memory_limit_gib_per_process_tree so
enforcement covers aggregate runner RSS across Python and browser trees, keeping
the configured threshold below the runner’s 30 GB limit; otherwise revise the
description to state that it only guards each process tree.
- Around line 123-131: Update the benchmark workflow step around the uv run
command to expose each dispatch input (sizes, arms, repetitions, timeout, and
memory_gib, plus the referenced values at the additional lines) through
step-level environment variables. Replace direct `${{ inputs.* }}` interpolation
in the shell with quoted environment-variable references, preserving the
existing SOFTWARE selection and argument behavior.
- Around line 61-64: Update the actions/checkout step in the ceiling benchmark
workflow to set persist-credentials to false alongside fetch-depth, preventing
the checkout action from retaining the GITHUB_TOKEN for subsequent cargo, uv,
npm, and benchmark steps.

In `@benchmarks/_ux_probe.py`:
- Around line 140-150: Update background() to return the modal-corner reference
rather than always selecting px[0]. Compute the mode across the collected corner
pixels and return that value, preserving the existing transparent fallback when
no pixels are available.

In `@benchmarks/_ux_webagg_host.py`:
- Around line 35-38: Eliminate the duplicated TOCTOU-prone free_port helpers by
extracting one shared helper near SENTINELS/make_data in
benchmarks/_ux_live_host.py. In benchmarks/_ux_webagg_host.py lines 35-38, use
the shared helper and configure webagg.port_retries above 1, or report WebAgg’s
actually bound port. In benchmarks/_ux_datashader_host.py lines 30-33, import
the shared helper and retry bind failures instead of exiting fatally.
- Around line 62-89: Protect the entire snapshot operation in state(), including
get_xlim(), get_ylim(), transData.transform(), and bbox reads, with the same
figure lock used by the WebAgg mutation paths. Acquire the lock before reading
axes or figure state and release it only after constructing the returned
dictionary, preserving consistent /state serialization during concurrent
updates.

In `@benchmarks/bench_ceiling.py`:
- Around line 409-411: Normalize the value assigned to "python_maxrss_bytes"
before it is compared with byte-based tree_rss and peak values. In the child
high-water-mark reporting path, account for the platform-specific ru_maxrss
units—convert Linux kilobytes to bytes while preserving macOS byte values—and
apply the same normalization to the additional occurrence around the other
reported high-water mark.

In `@benchmarks/bench_ux.py`:
- Around line 636-641: Bound every host handshake read, including the
`meta_line` path and the datashader arm’s `while` loop, by `timeout_s` instead
of allowing `readline()` to block indefinitely. Add a watchdog that terminates
the host when the deadline expires, returns `{"status": "host_timeout"}`, and
preserves cleanup so the sweep continues to the next case. Apply the same
behavior to the additional handshake block around the other host launch.
- Around line 944-956: Update the frame discovery loop around sizes and staged
so video_dir.glob(f"*-{n}") excludes the existing _grid-{n} staging directory
before populating it. Preserve the current symlink staging behavior for actual
arm tracks and prevent stale grid contents from being treated as an input track.
- Around line 796-801: Update the Plotly build flow around builder.communicate()
to use a bounded timeout and handle timeout or communication failures by
stopping the poller and returning a build_failed result. Guard stdout parsing
before accessing the last line, validate the extracted value as a float, and
return build_failed with diagnostic detail when output is empty or contains
non-numeric trailing content; preserve the existing successful timing result and
nonzero-return handling.

In `@benchmarks/make_ux_grid.py`:
- Around line 145-163: Validate that ffmpeg is available before invoking it in
the subprocess flow, using shutil.which and the existing parser.error mechanism
to report an actionable failure. Add the required shutil import and keep the
current subprocess execution unchanged when ffmpeg is present.

In `@benchmarks/plot_ceiling.py`:
- Around line 131-156: The tick construction in the memory and time branches of
the plotting flow must always retain at least one tick before assigning
tick_labels[-1], even when top is below the smallest threshold. Also replace the
fixed sizes[1] and sizes[2] annotation indices with sizes[min(1, len(sizes) -
1)] so annotations remain valid when fewer than three sizes are selected.

In `@benchmarks/plot_ux.py`:
- Around line 50-53: Update the series logic around rows.get((arm, n)) so a
missing row returns a non-empty stopped status, allowing the existing “fails at”
annotation branch to mark the truncation. Preserve the current status value for
rows that exist but are not “ok”, and apply the same behavior to the
corresponding logic near the other referenced occurrence.

In `@benchmarks/README.md`:
- Around line 95-147: Remove or defer the excluded benchmark documentation
across benchmarks/README.md sections 95-147, 200-213, and 221-270, and README.md
sections 154-183: omit ceiling/specification expansion,
recorded-versus-unrecorded verification claims, UX and renderer behavior
contracts, hard-coded ladder results, memory peaks, failure cells, and one-run
narratives until the benchmark scope permits them.

---

Nitpick comments:
In `@benchmarks/_ux_live_host.py`:
- Around line 49-61: Update make_data to validate that n is at least
len(SENTINELS) before planting sentinel rows, and fail explicitly with a clear
error when the requirement is not met. Preserve the existing data generation and
sentinel replacement behavior for valid n values.

In `@benchmarks/bench_ceiling.py`:
- Around line 512-520: Validate repetitions in main() or clamp it to at least
one before the benchmarking loop so --repetitions 0 cannot leave attempts empty
and trigger attempts[-1] IndexError. Preserve the existing loop and result
handling for positive repetition counts.

In `@benchmarks/make_ux_grid.py`:
- Around line 106-125: Update the image cache in the grid-generation loop to
retain entries for every arm instead of clearing it on each cache miss. In the
cache handling around `cache`, `key`, and `Image.open`, use the path key per arm
so previously decoded and resized images are reused across panels and output
frames, and adjust the cache type annotation if needed.

In `@benchmarks/plot_ux.py`:
- Around line 71-79: The loop in the plotting flow should rename the first value
returned by series from labels to xs, and update the corresponding xy.line and
xy.marker references. Preserve the existing behavior and use xs to reflect that
these are numeric x-values.

In `@benchmarks/run_ux_suite.sh`:
- Around line 16-21: Update the PY initialization in run_ux_suite.sh to honor an
existing PYTHON override or otherwise select the default virtual-environment
interpreter, and add an existence check before invoking the benchmark so missing
interpreters fail with a clear error instead of being swallowed by the grep
pipeline.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0b2be645-7766-4734-9f6e-84cdb68707a9

📥 Commits

Reviewing files that changed from the base of the PR and between 55b0ca8 and 0b725e7.

⛔ Files ignored due to path filters (2)
  • spec/assets/ux-python-memory.png is excluded by !**/*.png
  • spec/assets/ux-render-time.png is excluded by !**/*.png
📒 Files selected for processing (15)
  • .github/workflows/ceiling-benchmark.yml
  • README.md
  • benchmarks/README.md
  • benchmarks/_ux_datashader_host.py
  • benchmarks/_ux_live_host.py
  • benchmarks/_ux_probe.py
  • benchmarks/_ux_webagg_host.py
  • benchmarks/bench_ceiling.py
  • benchmarks/bench_ux.py
  • benchmarks/make_ux_grid.py
  • benchmarks/plot_ceiling.py
  • benchmarks/plot_ux.py
  • benchmarks/run_ux_suite.sh
  • benchmarks/summarize_ceiling.py
  • benchmarks/summarize_ux.py

Comment thread .github/workflows/ceiling-benchmark.yml Outdated
Comment thread .github/workflows/ceiling-benchmark.yml
Comment thread .github/workflows/ceiling-benchmark.yml
Comment thread benchmarks/_ux_probe.py
Comment thread benchmarks/_ux_webagg_host.py Outdated
Comment thread benchmarks/bench_ux.py
Comment thread benchmarks/make_ux_grid.py
Comment thread benchmarks/plot_ceiling.py
Comment thread benchmarks/plot_ux.py Outdated
Comment thread benchmarks/README.md
Alek99 and others added 14 commits July 27, 2026 17:36
Security (workflow):
- persist-credentials: false on checkout. Benchmark steps execute repository
  and dependency code, which would otherwise have the GITHUB_TOKEN in reach.
- Dispatch inputs move to step-level env vars. `${{ }}` expands before the
  shell parses, so an input carrying shell syntax was executable on the
  runner. zizmor flagged five injection sites; none remain.
- memory_gib is a per-tree guard, not a runner-wide cap: the WebAgg arm keeps
  its server alive while the browser runs, so two trees can be resident at
  once. Documented as such and defaulted to 14 GiB on a 30 GB runner.

Correctness:
- ru_maxrss is bytes on macOS and KiB everywhere else. The field was named
  python_maxrss_bytes and mixed with byte-valued poller samples, so off macOS
  it silently under-reported by 1024x. Normalized at the source.
- The grid staging dir lived at video_dir/_grid-{n}, which the arm glob
  `*-{n}` matched -- on a rerun it was picked up as an arm and symlinked into
  itself. Moved to a sibling and excluded underscore-prefixed names.
- background() documented a modal corner but returned the first one, so a
  title or toolbar in the top-left corner would silently weaken the sentinel
  correctness gate. Computes the actual mode now.

Robustness:
- Host handshakes used a bare readline(), which blocks forever if a host dies
  or hangs before printing META, stalling the whole sweep on one cell. Reads
  through a bounded helper.
- The Plotly build child had an unbounded communicate() and an unguarded
  float() on its last stdout line; both now fail as build_timeout /
  build_failed rather than hanging or raising.
- Missing ffmpeg raised a bare FileNotFoundError after the expensive frame
  work; it now reports where the frames are and exits 2.
- plot_ux marked a stop only for measured failures, so an unmeasured size
  trailed the line off as though the ladder had simply ended.

Also fixed, found while verifying the above rather than in review: the grid
canvas could be an odd height, which libx264 with yuv420p rejects. Whether it
was even depended on arm count and screencast aspect, so a layout that
encoded with five panels failed with three. Dimensions round up to even.

Verified end to end at 100k across xy, plotly and matplotlib, including a
rerun over an existing output directory.
The memory chart was four near-parallel lines: every library pays linearly
for the source data, so the shape carried no information the render-time
chart had not already established, while the numbers people actually want
(what does 100M cost) were harder to read off it than off a table.

Dropped the unused asset. plot_ux.py still generates both charts for local
inspection.
Each dead arm's line now continues as a dashed segment climbing toward the
size that killed it, ending in a red cross labelled "Plotly fails at 50M" /
"Matplotlib fails at 100M" -- the failure reads as an event on the chart
rather than a footnote at the last surviving point. Endpoint value labels
for failed arms move below their point to stay clear of the red annotation.

The render-time table gains a derived row, XY speedup vs the next-best
other library per size: 1x at 10k growing to 177x at 50M, and a dash at
100M where no other library completed. Computed from the run JSONs like
every other number in the section.
The dashed lead-outs crossed other lines and the floating labels collided
with endpoint values, which read as clutter rather than emphasis. The line
now simply dies where it died: a red cross sits on the last successful
point with 'fails at N' directly above and the final time directly below,
each annotation attached to the thing it describes.
The reader can see exactly which sizes were run rather than inferring them
from line bends -- on a log x axis the rungs are not where the eye expects
them.
Same derivation as the render-time speedup row: XY's peak Python RSS
against the next-best other library per size, computed from the run JSONs.
Steady 1.7-2.9x rather than the render-time row's runaway curve, which is
the honest shape: memory scales linearly for everyone and XY's edge is a
constant factor, not an asymptotic one.
A dashed red horizontal runs from the last measurement out to the failing
size, ending in a red cross AT that x -- so 'fails at 50M' sits on 50M
rather than on the survivor next to it, and reading down from the cross to
the axis gives the size that killed the arm.
'fails at 50M' sat centered on its cross, straddling the Matplotlib line
climbing through that region; it now sits in the clear space beside the
cross. The 100M label tucks under its dashed segment instead of colliding
with the 13.385s value above it.
The multiplier is the row most readers come for; it now leads each table
as 'XY speedup' / 'XY advantage', with the derivation (against the next
best other library, computed from the run JSONs) unchanged.
@Alek99
Alek99 merged commit 45deedb into main Jul 28, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants