Skip to content

fix: reject configs where timeouts.total_seconds can't cover member+judge - #151

Open
shrdgn wants to merge 1 commit into
mainfrom
claude/vibrant-archimedes-0n4hq4
Open

fix: reject configs where timeouts.total_seconds can't cover member+judge#151
shrdgn wants to merge 1 commit into
mainfrom
claude/vibrant-archimedes-0n4hq4

Conversation

@shrdgn

@shrdgn shrdgn commented Aug 18, 2026

Copy link
Copy Markdown
Owner

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 below member_seconds + judge_seconds — a value no legitimately-slow-but-successful request could ever satisfy:

Example member + judge total_seconds (before)
draco.yaml.example 420 350
panel-strongsynth.yaml.example 360 340
panel-tools.yaml.example 360 340

This 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):

  • TimeoutsConfig gets a model validator rejecting total_seconds < member_seconds + judge_seconds at config load, with a docstring clarifying total_seconds isn't enforced as a request deadline yet.
  • Fixed the three examples with headroom (draco: 450, panel-strongsynth/panel-tools: 420).
  • Added a smoke test (test_every_example_config_loads) that loads every examples/*.yaml.example, so this class of drift can't silently ship again.
  • Fixing those examples' widened per-phase budgets surfaced a second, related bug: worst-case request latency (member_seconds + judge_seconds) can now exceed bench.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.py and bench/research_eval.py now size their own client timeout off the config under test (member_seconds + judge_seconds + 30s margin) instead of assuming 360s covers it, each with its own offline test.

Closes part of #140.

How it was tested

  • ruff check . passes
  • pytest -q passes (506 passed, no live network)
  • New behavior has a test (tests/test_config.py, tests/test_research_eval.py, tests/test_draco_eval.py)
  • Docs updated (CHANGELOG.md, TimeoutsConfig docstring, updated example comments)
  • No secrets, prompts, or response bodies added to logs or metrics
  • Quality/cost claims backed by a bench/run.py number — n/a, no quality/cost change
$ mypy openfusion bench scripts
bench/run.py:120: error: ... [pre-existing, unrelated to this change]
Found 1 error in 1 file (checked 31 source files)

$ python -m pytest --cov=openfusion --cov-report=term-missing -q
506 passed
Required test coverage of 80.0% reached. Total coverage: 99.41%

Notes for reviewers

  • total_seconds is still not enforced as an actual request-level deadline anywhere — only member_seconds/judge_seconds bound live calls. That's the harder half of timeouts.total_seconds is 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.
  • The bench-harness timeout fix follows the same pattern in both draco_eval.py and research_eval.py for consistency, since both hit the identical class of bug once their configs' worst-case latency could exceed the hardcoded default.

Generated by Claude Code

…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants