ci: align benchmark and experiment runners - #203
Merged
Conversation
This was referenced Sep 4, 2026
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 01:19
e75b031 to
2b31767
Compare
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 01:38
2b31767 to
672f2f5
Compare
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 02:06
672f2f5 to
9de6e9b
Compare
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 02:17
9de6e9b to
d8db66f
Compare
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 02:33
d8db66f to
6ff6f4a
Compare
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 02:34
6ff6f4a to
7f4e22f
Compare
joshblack
changed the base branch from
feat/add-benchmark-scenarios
to
feat/prebuild-sandbox-image
September 4, 2026 02:44
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 02:44
7f4e22f to
454150d
Compare
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 02:48
454150d to
973609d
Compare
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 02:49
973609d to
70ca2c5
Compare
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 03:32
70ca2c5 to
4f59fc2
Compare
joshblack
marked this pull request as ready for review
September 4, 2026 03:36
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 03:36
4f59fc2 to
ab623ba
Compare
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The runner scripts and workflows are consistent with the intended unified result layout and artifact retention behavior, and no correctness issues were found in the updated execution paths.
Review tier: Lite
Findings: None
What changed in this PR
This PR aligns experiment execution with the newer benchmark runner conventions by introducing a dedicated experiment runner script, standardizing result bundle layout, and simplifying the experiment GitHub Actions workflow to produce a single retained artifact per run.
Changes:
- Add
script/run-experiment.sh, mirroringscript/run-benchmark.shconventions (positional name arg, results underresults/.../<name>/<date>, single portable bundle per run). - Require benchmark/experiment names as positional runner arguments and update the benchmark workflow to pass
design-systemexplicitly. - Simplify the experiment workflow by removing shard/merge jobs and uploading a single retained artifact; update
script/setupto restore the newest benchmark and experiment bundles.
| File | Description |
|---|---|
script/setup |
Expands artifact restore to include both benchmark and experiment retained result bundles. |
script/run-experiment.sh |
New experiment runner script producing results in the standard portable directory layout. |
script/run-benchmark.sh |
Switches to requiring a positional benchmark name argument (no default). |
.github/workflows/experiment.yml |
Runs the new experiment runner and uploads a single long-retention artifact (no sharding/merge). |
.github/workflows/benchmark.yml |
Updates workflow to run design-system benchmark explicitly via the runner arg. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The benchmark runner script includes a duplicated assignment that should be cleaned up before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
script/run-benchmark.sh — benchmark_name is assigned twice, which is redundant and can be misleading when editing the… |
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 04:28
66ce648 to
b541df4
Compare
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 04:29
b541df4 to
b28638a
Compare
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The runner/workflow changes are consistent across scripts and Actions, and the updated artifact restore filter matches the new artifact naming scheme.
Review tier: Lite
Findings: None
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
script/run-benchmark.sh — benchmark_name is assigned twice, which is redundant and can be misleading when editing the… View resolved comment |
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 04:34
b28638a to
5d3a3c1
Compare
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 04:38
5d3a3c1 to
a397ce2
Compare
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 04:41
a397ce2 to
bf2f20f
Compare
Add a shared experiment runner convention, make runner names configurable, and restore the 10 newest benchmark or experiment result bundles during setup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6d12a97a-b8d3-45ad-a09a-3c76d1411705
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
joshblack
force-pushed
the
feat/add-experiment-runner-script
branch
from
September 4, 2026 04:43
bf2f20f to
b792663
Compare
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The changes are cohesive and low-risk, with only small wrapper-script ergonomics suggested to improve future flexibility.
Review tier: Lite
Findings: 2
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
 |
script/run-benchmark.sh — The wrapper currently requires exactly one positional argument and rejects any extra args, which… |
|
script/run-experiment.sh — The wrapper currently requires exactly one positional argument and rejects any extra args, which… |
Comment on lines
+7
to
+11
| if [[ $# -ne 1 ]]; then | ||
| echo "Usage: $0 <benchmark-name>" >&2 | ||
| exit 1 | ||
| fi | ||
| benchmark_name="$1" |
Comment on lines
+7
to
+32
| if [[ $# -ne 1 ]]; then | ||
| echo "Usage: $0 <experiment-name>" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| experiment_name="$1" | ||
| run_date="${RUN_DATE:-$(date -u +%F)}" | ||
| run_directory="$repository_root/results/experiments/$experiment_name/$run_date" | ||
|
|
||
| if [[ ! "$experiment_name" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]]; then | ||
| echo "Experiment name must be a file name without its extension" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if [[ ! "$run_date" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}$ ]]; then | ||
| echo "RUN_DATE must use the YYYY-MM-DD format" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| node "$repository_root/packages/agent-eval/bin/agent-eval" \ | ||
| --experiment "$experiment_name" \ | ||
| --experiments "$repository_root/experiments" \ | ||
| --concurrency "${CONCURRENCY:-1}" \ | ||
| --docker-image "${DOCKER_IMAGE:-node:26.5.0-slim}" \ | ||
| --output-dir "$run_directory" \ | ||
| --scenarios "$repository_root/scenarios" |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The changes align runners/workflows as intended; remaining feedback is limited to minor error-message/validation consistency.
Review tier: Lite
Findings: 2
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
|
script/run-benchmark.sh — This error message says the benchmark name must be a file name without an extension, but the… |
|
script/run-experiment.sh — This error message says the experiment name must be a file name without an extension, but the… |
Pre-existing issues (2)
| Severity | Finding |
|---|---|
|
script/run-experiment.sh — The wrapper currently requires exactly one positional argument and rejects any extra args, which… View comment |
|
script/run-benchmark.sh — The wrapper currently requires exactly one positional argument and rejects any extra args, which… View comment |
|
|
||
| if [[ ! "$benchmark_name" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]]; then | ||
| echo "BENCHMARK_NAME must be a benchmark file name without its extension" >&2 | ||
| echo "Benchmark name must be a file name without its extension" >&2 |
| run_directory="$repository_root/results/experiments/$experiment_name/$run_date" | ||
|
|
||
| if [[ ! "$experiment_name" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]]; then | ||
| echo "Experiment name must be a file name without its extension" >&2 |
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.
Follow up to #201.
This PR aligns benchmark and experiment execution around the newer benchmark runner conventions. Both scripts now accept a configurable name, own their result directory layout, and produce one portable output bundle per run.
This is the fourth entry in stack 214. #204 adds durable plan sharding next, and #199 adds the benchmark scenarios last.
Changelog
New
script/run-experiment.shwith the same setup and result layout conventions asscript/run-benchmark.sh.Changed
design-systembenchmark explicitly.script/setupto restore the 10 newest retained benchmark or experiment result bundles combined.Removed