fix: size the DRACO bench harness's HTTP timeout off the config under test - #146
Open
shrdgn wants to merge 1 commit into
Open
fix: size the DRACO bench harness's HTTP timeout off the config under test#146shrdgn wants to merge 1 commit into
shrdgn wants to merge 1 commit into
Conversation
… test bench/draco_eval.py called bench.run._chat without an explicit timeout, so it inherited that function's shared 360s default. examples/draco.yaml.example deliberately widens member_seconds/judge_seconds to 240/180 (worst case 420s) for slow agentic tool loops, so a legitimately slow-but-successful DRACO run could be killed client-side by the 360s timeout and misrecorded as a task failure. run_eval now derives its request timeout from the loaded config's own member_seconds + judge_seconds, and draco.yaml.example's total_seconds (previously 350, below its own 420s worst case) is raised to 450 to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfTJhjkqkDfTMzufuBcLtQ
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.
What & why
bench/draco_eval.pycallsbench.run._chat(via_chat_with_retry) to hit the openfusionserver under test, without ever passing an explicit
timeout=, so every call — the soloanswer, the fusion answer, and every grading call — inherited
_chat's shared default of360s (
bench/run.py:97).examples/draco.yaml.exampledeliberately widensmember_seconds/judge_secondsto240/180"because agentic tool loops + long answers are slow" (its own comment). Sincepanel members run concurrently and judge synthesis follows sequentially
(
panel.py::_call_member's retry loop shares one deadline, so a member's own worst caseis bounded by
member_seconds, not a multiple of it), the server-side worst case for afusion request against this config is
member_seconds + judge_seconds= 420s —comfortably past the bench client's hardcoded 360s HTTP timeout. A DRACO run that
legitimately takes, say, 380s to produce a good answer would get killed client-side by
httpx.ReadTimeoutand either get spuriously retried (_chat_with_retry, up to 4 attempts)or recorded as a task failure (
errors.append(...)inrun_eval) — silently corrupting thebenchmark's own results for exactly the slow-but-good responses the widened budgets exist to
allow.
This is also the concrete, narrow, self-contained piece of #140 flagged as fair game there:
draco.yaml.example'stotal_seconds: 350was itself below its ownmember_seconds + judge_seconds(420), inconsistent with the field's documented "aggregate wall-clock cap"semantics (full enforcement of that field is out of scope here, deliberately deferred in
#140 pending a product decision on streaming semantics).
Fix
bench/draco_eval.py::run_evalnow computes `request_timeout = config.timeouts.member_secondsfrom the config it just loaded, and passes it explicitly to every_chat_with_retrycall (_gradenow takes and forwards atimeout`param too). The harness sizes itself off whatever config it's pointed at instead of assuming
a fixed 360s across every config.
examples/draco.yaml.example'stotal_secondsis raised from350to450(above the420s worst case, with headroom), and the comment now states the real invariant instead of
the stale "keep under the bench client's 360s timeout" claim (which the harness no longer
needs, since it derives its timeout from these same values).
Not touched:
member_seconds/judge_secondsthemselves (240/180) — those are enforcedserver-side today and presumably tuned for real agentic tool-loop durations, so shrinking them
is a behavior change with real product trade-offs, out of scope for this fix. Also not touched:
bench/run.py's own_chatdefault orexamples/panel-strongsynth.yaml.example(which has anarrower, less severe version of the same 360s-vs-
member+judgegap — a possible follow-up).How it was tested
ruff check .passespytest -qpasses (491 passed, no live network)tests/test_draco_eval.py::test_run_eval_sizes_request_timeout_from_configured_member_and_judge_secondsmonkeypatches
load_config/load_draco/_chat_with_retryand asserts every call receivestimeout == member_seconds + judge_seconds + 30.harness itself is willing to wait for a response
Notes for reviewers
Found via an automated repo-review scheduled task, verified by hand: read
bench/draco_eval.pyand confirmed it imports
_chatfrombench/run.py(defaulttimeout=360.0, never overriddenin any
_chat_with_retrycall), then confirmedpanel.py::_call_member's retry loop shares asingle
member_secondsdeadline (so 240 + 180 = 420 is the real server-side worst case, not anunderestimate), then cross-checked
examples/draco.yaml.example's current committed values onmain.Generated by Claude Code