feat(livecodebench): replace the whole-suite timeout with upstream's per-case rule - #66
Merged
Merged
Conversation
LiveCodeBench budgets each test case, not the suite. `lcb_runner` re-arms `signal.alarm(timeout)` inside the case loop of `grade_call_based` / `grade_stdio` (`lcb_runner/evaluation/testing_util.py`), `codegen_metrics(..., timeout=6)` supplies the default, and `check_correctness` joins its worker at `(timeout + 1) * n + 5` only as a backstop. We had only the backstop: one wall for the whole suite, `timeout + 2 * n_cases`. That is a different rule, not a looser version of the same one -- a 43-case suite where one case takes 200s and the rest take 1s fits inside a 258s whole-suite wall and fails a 6s-per-case one. It also grades tighter than the benchmark defines: at the typical 43 cases we allow 116s against upstream's 306s backstop, ~0.38x. Adds an opt-in `timeout_per_case`, threaded task -> request -> worker: * `vendor/code-evaluator/app/exec_py_test.py` — each case runs inside `_case_time_limit`, a `signal.setitimer` guard armed around the case body and cancelled after it. `CaseTimeout` derives from `BaseException` so neither the submitted code's `except Exception` nor this module's own swallows it. * `vendor/code-evaluator/app/server.py` — `Sample.timeout_per_case`. When only that is supplied, the suite wall is derived as upstream's `(timeout_per_case + 1) * n + 5`. * `sieval/tasks/livecodebench_code_generation_0shot_gen.py` — task arg of the same name; set it to 6.0 to grade the way the benchmark defines. Second effect, and the reason it also improves reporting: a per-case timeout returns normally, so `n_passed` survives it -- only the whole-suite wall loses the count, because it kills the worker. On a 90-rollout run every rollout missing `n_passed` was a timeout and no timeout carried one, so a timed-out submission's progress was unknowable. Now it is the case index, like every other failure. Opt-in throughout: absent the field, behaviour is byte-for-byte what it was, and the first test asserts exactly that, including that a whole-suite kill still loses the count. Verified on a submission whose second of three cases spins forever: per-case 2s gives `n_passed=1` and `failed: case timeout: 2.0s`; the old 8s suite wall gives `n_passed=None` and `failed: subprocess timeout: 8.0s`; a correct submission is unaffected either way. Six tests, the four that spawn subprocesses marked `stress`. `tests/unit/tasks` + `tests/unit/core`: 1305 passed, 1 skipped. Sizing, measured before writing this: on a 90-rollout LiveCodeBench run 6 rollouts (6.7%) hit the whole-suite wall. Re-running those six at 6s/case, one passes -- correct code that needed 151.5s of a 102s budget -- and five still exceed 252-258s, i.e. they are genuinely too slow, which is a real failure on a competitive-programming benchmark. So this is worth ~1pp on that lane, not the ~7pp a naive "every timeout would have passed" ceiling suggests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sizing in the previous commit was measured with the wrong intervention. That probe
sent `timeout=6*n` -- a bigger WHOLE-SUITE wall -- and found 1 of 6 timeouts rescued,
which I reported as "worth ~1pp". That is a true statement about raising the budget and
says nothing about changing the rule.
Re-graded the same 90-rollout lane through the actual per-case path:
unchanged 88/90 | pass -> FAIL 2 | fail -> pass 0 | net -2.22 pp
Per-case bites in both directions, and on this data only one of them fires. The
submission the earlier probe "rescued" (s5, 36 cases, 151.5s under a 216s wall) does
NOT pass per-case -- at least one of its cases exceeds 6s. Meanwhile s11 and s57, which
had completed every case inside the old wall (42/42 in 114s, 44/44 in 118s), now fail
because one case each runs long.
The change is still right: it is the rule the benchmark defines, and our old wall was
simultaneously too generous to a submission with one slow case and too harsh on one
that is uniformly slowish. But it is a re-baseline, not a free point, and VENDORED.md
now says so where someone deciding whether to enable it will read it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review follow-ups on the per-case timeout. Three of these are about the rule
being claimed in more places than it was actually applied.
**The tests that verified this did not run in CI.** `addopts` carries
`-m "not stress"` and CI adds `not benchmark`, so all four subprocess tests
were deselected; the two that did run asserted arithmetic on literals
(`30.0 + 43 * 2.0 == 116.0`) without calling task code, and survived replacing
both wall formulas with `0.001`. Task-side tests now drive `feedback()` through
a stub evaluator and assert the posted body, so they fail on that mutation;
the guard's own semantics -- including the `BaseException` choice the module
depends on -- are covered in-process in milliseconds. The subprocess tests stay
`stress`, which is the right marker for what they do.
Rehomed onto the mirror rule while there: task-side into the per-module files
(`test_livecodebench_code_generation_{0shot_gen,kshot_base_gen}.py`),
evaluator-side into `tests/unit/vendor/code_evaluator/`, since it tests
`vendor/`, not `sieval/tasks/`. The vendor-root `sys.path` insert is now undone
on teardown rather than left for the session -- `app` is a name that can collide.
**The k-shot base variant carried the identical formula.** It grades the same
problems with the same suites off `self._timeout + len(inputs) * 2.0`, so
shipping per-case on only the 0-shot task would have split the family. It gets
the same opt-in param, and its "Deviations (complete list)" item 3 -- which
justified the whole-suite wall by saying the service runs all cases under one
sequential budget -- is no longer true, so it is rewritten and the budget
promoted to its own numbered deviation.
**`reference_impl` still advertised a faithful port.** Both tasks now record
the divergence where the alignment metadata is read, not only in `VENDORED.md`;
`meta/index.json` regenerated. Left `status="stable"` alone -- flipping it is a
catalog-visible call for the author, and making `timeout_per_case=6.0` the
default would settle it properly.
Also, in the evaluator:
- `compile_code` now runs on the per-case clock, which is where upstream arms it
(`testing_util.py` `compile_code`). On the call-based path that `exec` runs the
submission's module-level statements, so a hang there is inside no case and
escaped the budget to the whole-suite wall -- killing the worker and losing
`n_passed`, the one outcome this feature exists to prevent. The vendored copy
had kept upstream's `finally` with a bare `pass` where the alarm was cancelled.
- `_subprocess_target` catches `CaseTimeout` explicitly. It is a `BaseException`,
so `except Exception` did not cover it, and cancelling the timer cannot
un-deliver a signal the kernel already sent: a late alarm could escape
`_unsafe_execute` and leave the queue empty, stranding the parent until the
suite wall.
- Noted the one deliberate remaining difference from upstream: the guard wraps
call and comparison together where upstream compares off the clock.
Verified: upstream re-arm/default/backstop claims checked against
LiveCodeBench@28fef95e. 2937 passed / 6 stress passed, ruff + ty clean, full
preflight green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BREAKING CHANGE: `timeout` is gone from both LiveCodeBench code-generation tasks, replaced by `timeout_per_case` (default 6.0). LiveCodeBench scores recorded before this are not comparable with scores recorded after it -- measured -2.22 pp on a 90-rollout lane. Release this as a MINOR bump (0.8.0), not a patch: the resume gate's break axis under 1.0 is (major, minor), so a 0.7.x -> 0.7.y resume is only warned about, while 0.7.x -> 0.8.0 is rejected. That rejection is what stops a run from being continued under a different grading rule than it started with. The whole-suite wall was never LiveCodeBench's rule. Keeping it as the default and hiding the real rule behind an opt-in meant shipping a known-wrong default and asking every caller to remember which of two `timeout`s meant what. So the opt-in is gone: - `timeout_per_case` is the only knob, and upstream's own parameter and default (`codegen_metrics(..., timeout=6)`). - The whole-suite wall is no longer configurable. It is derived as upstream's backstop, `(timeout_per_case + 1) * n + 5`, and is only a backstop -- for a worker wedged where a per-case signal cannot reach it. - The old name is removed rather than redefined. Reusing `timeout` for a per-case budget would silently regrade any config still setting it (a stale `timeout: 30.0` would go from a 30 s suite base to 30 s *per case*); now it raises `TypeError` at construction. A test pins that. No shipped config set it -- the only `timeout:` in `leaderboards/` is a model arg. Both tasks, not one: the k-shot base variant graded the same problems off the same formula, so leaving it behind would have split the family. Documentation follows the code rather than the other way round: both tasks' `reference_impl.notes` now say the budget MATCHES upstream and that older runs are not comparable, and the k-shot "Deviations (complete list)" loses the budget entry it had just gained -- the list gets shorter because the divergence is gone. `meta/index.json` regenerated. The evaluator keeps `timeout_per_case` optional on its API, so a client that predates the field is unaffected; sieval's tasks simply always send it now. Verified: 2940 passed / 6 stress passed, ruff + ty clean, 21/21 preflight checks pass. Upstream rule re-checked against LiveCodeBench@28fef95e. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`tests/unit/` mirrors `sieval/`, and `vendor/code-evaluator` is neither: it is a separately deployed service whose local patches are meant to land in `scitix/code-evaluator` and be re-vendored. A test tree under `tests/unit/vendor/` had no precedent in this repo -- the previous commit invented it -- and the next re-vendor would orphan it. Tests for the guard belong in the evaluator's own repo, alongside the patch. Removed with it: the `sys.path` insert of the evaluator root (needed so a spawned worker could re-import `app.exec_py_test` by dotted name), and the `stress` marker usage that came with the subprocess tests. The task-side tests stay. They cover what sieval actually owns -- which budget goes on the wire -- and run in CI in under a second, which was the point of the earlier commit. VENDORED.md now records where the thirteen deleted tests live (`tests/unit/vendor/` at 7c426a6) so they can be ported upstream with the patch rather than rewritten. They passed before removal, including the two that pinned the compile-time budget. Verified: 2933 passed, ruff + ty clean, 21/21 preflight checks pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Trims what the review iterations accumulated, with no behaviour change. The VENDORED.md entry had grown to six paragraphs that each re-argued the same two points -- per-case is a different rule, and scores drop -- so it is now five tight ones with each fact stated once. Task docstrings, the wall comments, and the test module docstring lose the passages that justified decisions to a reviewer rather than telling a reader what the code does. The k-shot "Deviations (complete list)" loses the trailing essay: the budget is no longer a deviation, so it belongs inside item 3 as a clause, not as a paragraph hanging off a numbered list. Two real fixes fell out of the pass: - `README.md` pointed at a "Per-case timeout" section that was never written. Made the field's bullet self-contained instead of adding one. - The README's timeout defaults listed only `6s + 2s * n`, not the `(timeout_per_case + 1) * n + 5` the server derives when the field is sent. `meta/index.json` regenerated for the shortened notes. Verified: 409 task tests pass, ruff + ty clean, 21/21 preflight checks pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"a 90-rollout lane" is not a term this repo uses anywhere -- `git grep lane` against main returns nothing. It entered in this PR's own commit messages and I carried it into `VENDORED.md` and the 0-shot task's `reference_impl.notes`, which meant it reached `meta/index.json` and so the public task catalog: a number qualified by a word with no definition in the codebase. Replaced with what it actually denotes -- 90 recorded rollouts, the stored submissions from an earlier run, re-graded without re-generating them. That is the replay the measurement was done by, and it says how someone would reproduce it. `meta/index.json` regenerated. Verified: 409 task tests pass, ruff clean, no `lane` left in shipped text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 6, 2026
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.
Type
Summary
lcb_runnerre-armssignal.alarm(timeout)inside the case loop ofgrade_call_based/grade_stdioand incompile_code(testing_util.py), withcodegen_metrics(..., timeout=6)the default (compute_code_generation_metrics.py). We had only the outer backstop: one wall for the whole suite.timeout_per_case: float = 6.0, upstream's own parameter and default. The whole-suite wall is no longer configurable — it's derived as upstream's backstop(timeout_per_case + 1) * n + 5and is only a backstop.timeoutis removed, not redefined. Reusing the name would silently regrade a config still setting it (timeout: 30.0would go from a 30 s suite base to 30 s per case); it now raisesTypeError. No shipped config set it.(major, minor), so0.7.x → 0.7.yonly warns while0.7.x → 0.8.0rejects — that rejection is what stops a run being continued under a different grading rule than it started with.n_passedsurvives, where the suite wall kills the worker and loses it. On the measured run, every rollout missingn_passedwas a timeout and no timeout carried one.timeout_per_caseoptional on its API, so other clients are unaffected; sieval's tasks simply always send it.Expect this to LOWER the score — merge it as a re-baseline
Per-case is a different rule, not a looser one, and it bites both ways:
Replaying 90 recorded rollouts — the stored submissions from an earlier run, re-graded through the per-case path without re-generating them:
Both regressions finished every case inside the old wall (s11 42/42 in 114 s, s57 44/44 in 118 s) and own at least one case over 6 s. Nothing was rescued the other way on this data. LiveCodeBench numbers from before this are not comparable with numbers after it.
An earlier revision of this PR claimed "+≈1 pp". That was measured wrong.
That probe sent
timeout=6*n— a bigger whole-suite wall — and found 1 of 6 timeouts rescued. A true statement about raising the budget; it says nothing about changing the rule. The submission it "rescued" (s5, 36 cases, 151.5 s under a 216 s wall) does not pass per-case. Corrected in b271c5c.Migration
Drop
timeoutfrom any LiveCodeBench taskargs; settimeout_per_caseinstead if you need something other than upstream's 6 s. Re-run rather than resume: existing result dirs were graded by the old rule.Test Plan
Automated
ruff check && ruff format --check)ty check)check_preflight.pychecks passTask-side tests run in CI (sub-second): they drive
feedback()through a stub evaluator and assert the posted body — the default is upstream's 6 s, the wall is the derived backstop, the HTTP deadline stays outside it, and a staletimeoutraises. Tests for the guard itself belong inscitix/code-evaluatorwith the patch, not undertests/(which mirrorssieval/) where the next re-vendor would orphan them; thirteen were written and passed, and are recoverable fromtests/unit/vendor/at7c426a69.Manual
timeout=6default and the(timeout + 1) * n + 5join against LiveCodeBench@28fef95e.compile timeoutwithn_passed=0under per-case, andsubprocess timeoutwithn_passed=Nonewithout it.Checklist
Required (all PRs)
AI-Generated Code - <model> (<provider>)in module docstringcore/timeouthad no remaining call sitesIf: New or Modified Benchmark
__init__.py(unchanged)If: Breaking Change
Note on the vendored evaluator
vendor/code-evaluatorcarries local patches by the existing convention, andVENDORED.mdrecords this one with the same caveat as the others: not yet upstream — land inscitix/code-evaluatorand re-vendor. This patch also putscompile_codeon the per-case clock, where upstream arms it too — the vendored copy had kept upstream'sfinallywith a barepassafter the alarm was dropped.🤖 Generated with Claude Code