fix: reject configs where timeouts.total_seconds can't cover member+judge - #151
Open
shrdgn wants to merge 1 commit into
Open
fix: reject configs where timeouts.total_seconds can't cover member+judge#151shrdgn wants to merge 1 commit into
shrdgn wants to merge 1 commit into
Conversation
…udge timeouts.total_seconds is documented and set in every example config as an apparent aggregate wall-clock cap, but three shipped examples set it below member_seconds + judge_seconds (draco.yaml.example, panel-strongsynth, and panel-tools) — a value no legitimately-slow-but-successful request could ever satisfy. Add a TimeoutsConfig model validator that rejects this at config load, fix the three examples, and add a smoke test that loads every examples/*.yaml.example. Fixing those examples' budgets exposed a matching bug in the bench harness: worst-case request latency (member_seconds + judge_seconds) can now exceed bench.run._chat's hardcoded 360s default HTTP timeout, so draco_eval.py and research_eval.py size their own client timeout off the config under test instead of assuming 360s covers it. total_seconds still isn't enforced as an aggregate request-level deadline (that needs a product decision on streaming semantics — see #140); this closes the "impossible config" half of that gap. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PtWSAe6pa3DpECaDR1RMyK
6 tasks
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
timeouts.total_seconds(openfusion/config.py) is documented and set in every example config, and reads as an aggregate wall-clock cap on a request (member + judge phases). But three shipped examples set it belowmember_seconds + judge_seconds— a value no legitimately-slow-but-successful request could ever satisfy:draco.yaml.examplepanel-strongsynth.yaml.examplepanel-tools.yaml.exampleThis closes the "impossible config" half of #140 (full aggregate-deadline enforcement still needs a product decision on streaming semantics, per that issue — not attempted here):
TimeoutsConfiggets a model validator rejectingtotal_seconds < member_seconds + judge_secondsat config load, with a docstring clarifyingtotal_secondsisn't enforced as a request deadline yet.draco: 450,panel-strongsynth/panel-tools: 420).test_every_example_config_loads) that loads everyexamples/*.yaml.example, so this class of drift can't silently ship again.member_seconds + judge_seconds) can now exceedbench.run._chat's hardcoded 360s default HTTP timeout — so a slow-but-successful bench run against these configs would get killed client-side and misrecorded as a failure.bench/draco_eval.pyandbench/research_eval.pynow size their own client timeout off the config under test (member_seconds + judge_seconds + 30smargin) instead of assuming 360s covers it, each with its own offline test.Closes part of #140.
How it was tested
ruff check .passespytest -qpasses (506 passed, no live network)tests/test_config.py,tests/test_research_eval.py,tests/test_draco_eval.py)CHANGELOG.md,TimeoutsConfigdocstring, updated example comments)bench/run.pynumber — n/a, no quality/cost changeNotes for reviewers
total_secondsis still not enforced as an actual request-level deadline anywhere — onlymember_seconds/judge_secondsbound live calls. That's the harder half oftimeouts.total_secondsis documented and set in every example config, but never enforced #140 (streaming needs a timeout-watcher task alongside_stream_with_cancellation's disconnect watcher, plus a decision on hard-cancel vs. soft/logged-warning semantics) and is intentionally out of scope here.draco_eval.pyandresearch_eval.pyfor consistency, since both hit the identical class of bug once their configs' worst-case latency could exceed the hardcoded default.Generated by Claude Code