Timing bounds sit between the deadline and the body, not close to either - #106
Merged
Conversation
The five `max_seconds` timing tests fail intermittently on a loaded machine and
pass reliably on a quiet one: three suites running at once turned exactly these
five red on an untouched `main`, and the same files run 16 consecutive times on
the same idle box were 16 x exit 0. A test that fails only under load is
indistinguishable from a real regression in the deadline machinery, which is
the subsystem where a true red matters most.
The deadline machinery was never the thing failing. The *assertion* was. Each
test bounds how long an interrupted node took, and every bound was a hand-picked
constant sitting close to the deadline: `< 2.0` on a 0.2s deadline, `< 1.0` on
0.2s, `< 3.0` on 1.5s. What the assertion means to ask is "was this interrupted,
or did it run to completion" — and a bound with 0.8s of room above the deadline
answers a question about machine load instead, because scheduling delays under
contention run 2-3x.
So every bound now sits between the deadline and the uninterrupted body, with
seconds of slack on both sides:
syscall interrupt 0.2s deadline, 5s body < 2.0 -> < 3.0
fan-out worker 1.5s deadline, 15s body < 3.0 -> < 6.0
unreachable ceiling 0.2s deadline, 10s body < 1.0 -> < 4.0
agent wall clock 0.5s ceiling, 30s call < 10 -> < 15
cookbook nap 0.25s ceiling, 5s sleep < 1.0 -> < 3.0
Two bodies were lengthened rather than their bounds widened, because the gap
was too narrow to put a bound inside. The fan-out workers spin for 15s instead
of 5s, and the unreachable-ceiling test grew a second body: its two runs want
opposite things from one, the first having to run to completion (so short, and
now 0.05s instead of 2.0s) and the second having to be cut off (so long, and now
10s). An uninterrupted body only ever runs its full length when the test is
failing, so lengthening one costs nothing on the green path and widens the gap
the assertion is reading.
The cookbook page's own numbers are untouched — `Budget(max_seconds=0.25)` and
`time.sleep(5.0)` are what the page prints and what this file exists to run
verbatim. Only the test's own elapsed bound moved.
Verified: 20/20 consecutive runs of all five with three full suites and 30 CPU
spinners on the same 20-core box, at load average 33-35. And the bounds have not
gone vacuous — reverting the guard's exit check in `deadline_guard` turns four
tests in this file red, green again on restore.
Honest limit: the original flake did not reproduce here. The old bounds also
passed 8/8 under that same load, so this removes the mechanism by which a
scheduling delay could flip these assertions rather than turning an observed red
green. The issue's reproduction was three suites in separate worktrees and
separate venvs, which is a different pressure shape from CPU oversubscription.
Closes #40
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merged
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.
Closes #40.
What was actually flaking
Not the deadline machinery — the assertions about it. Each of the five tests bounds how long an interrupted node took, and every bound was a hand-picked constant sitting close to the deadline.
< 2.0on a 0.2s deadline leaves 1.8s of room; a 2-3× scheduling delay under contention eats that, and the test answers a question about machine load instead of about the guard.The change
Every bound now sits between the deadline and the uninterrupted body, with seconds of slack on both sides:
< 2.0< 3.0< 3.0< 6.0< 1.0< 4.0< 10< 15< 1.0< 3.0Two bodies were lengthened rather than their bounds widened, because there was no room to put a bound inside the gap. An uninterrupted body only ever runs its full length when the test is failing, so lengthening one costs nothing on the green path and widens the signal the assertion reads.
The unreachable-ceiling test grew a second body: its two runs want opposite things from one. The first must run to completion (so it wants to be short — now 0.05s, down from 2.0s, which makes this test faster) and the second must be cut off (so it wants to be long — now 10s).
The cookbook page's own numbers are untouched.
Budget(max_seconds=0.25)andtime.sleep(5.0)are what the page prints and whattests/test_cookbook_basics.pyexists to run verbatim. Only the test's own elapsed bound moved.Verification
pytestsuites and 30 CPU spinners on the same 20-core box, at load average 33–35.deadline_guard(if state["fired"] or (left is not None and left <= 0)) turns four tests intest_budget_enforcement.pyred, and restoring it turns them green.uv run pytestfull suite green;uv run ruff check grapharc testsclean.Honest limit
The original flake did not reproduce here. The old bounds also passed 8/8 under that same load. So this removes the mechanism by which a scheduling delay could flip these assertions — it does not turn an observed red green. The issue's reproduction was three suites in separate worktrees with separate venvs, which is a different pressure shape (I/O and memory, not just CPU oversubscription) from what I could generate.
I took the issue's first option (widen the margins) rather than the marker/serialisation or loadavg-skip options, because it is the only one of the three that keeps the tests running everywhere and keeps the signal honest — a skip-under-load test proves nothing on exactly the machine most likely to be loaded, i.e. CI.
🤖 Generated with Claude Code