Update the ClawBench benchmark pin - #172
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
2f84af0 to
bc58b95
Compare
bmsaadat
left a comment
There was a problem hiding this comment.
Approve with one non-blocking ask and a few optional notes.
Verified the pin itself checks out: c7feaa2 is the 4 control-arm commits rebased onto the v0.10.0 release merge, the correctness changes really are upstream (ClawBench #324, inside v0.10.0), and a full tree diff between the two pins shows everything on the benchmark path is byte-identical (harbor_adapter.py, providers.py, task.py, the whole runtime/ tree, all task assets). The lenient/strict reward reporting made it upstream, so results.ts is unaffected, and the --clawbench-sha provenance chain stamps the new pin into Braintrust correctly. All 6 SHA occurrences agree and no stale 45a71c4 references remain.
The one ask: nothing has actually run against the new pin. The validation list is static-only, and a /benchmark comment can't cover it either, since comment-triggered runs execute main's workflow file, which still carries the old SHA (the one run on this PR resolved and skipped). A single-task workflow_dispatch launched from this branch with pr_number=172 would put one live run on the new pin before the Monday cron does, and would match the live-validation bar #162/#171 set. Fine to do right after merge instead. The only residual risk is host-side: the adapter step now resolves deps from the new top-level uv.lock.
Optional:
- The pin is only reachable via
hypeship/harbor-kernel-control(head of upstream PR TIGER-AI-Lab/ClawBench#330). If upstream review forces a rebase, the SHA goes unreachable, and45a71c4has already rotted exactly this way (head of no branch, contained by none, repo has no tags). A lightweight tag in kernel/ClawBench on the pinned commit would be cheap insurance until #330 lands and we can repin to upstream. - The two new assertions hardcode the SHA a 5th and 6th time, and the README short SHA is the one copy with no guard. Deriving the ref once from run.sh (
/CLAWBENCH_REF:-([0-9a-f]{40})/) and asserting the workflow contains it twice plus the README contains its 7-char prefix would need zero test edits on future bumps and fail with a pin-drift-shaped message. toHaveLength(2)locks in the workflow's intra-file duplication. The job already has anenv:block single-sourcing every other pin;CLAWBENCH_REFcould live there once, referenced from the checkout'swith: ref:.- Pre-existing, but this bump re-exposes it: the README says "a checkout containing pinned commit c7feaa2" without naming the branch, and links the upstream repo where the commit doesn't exist. A fresh clone on main fails run.sh's
is-ancestorcheck with a misleading message. One clause naming the branch would save the next person a round-trip.
ClawBench · PR #172 (bc58b95) vs merge base (7454ab2)
Candidate minus baseline: +1 lenient, +1 strict, +0 intercepted. Open the Braintrust experiment Lenient reward is the primary ClawBench score. Infrastructure failures remain in the intended-task denominator. |
Summary
The repository remains
kernel/ClawBenchuntil the remaining Kernel-backed Harbor control arm is accepted upstream. The generic Harbor correctness changes are already upstream.Validation
bun test— 263 testsbunx tsc --noEmit --incremental falsebash -n benchmarks/harbor/clawbench/run.shgit diff --check