Skip to content

fix: resolve the per-script timeout parent-side and keep timeout output - #227

Merged
Jammy2211 merged 1 commit into
mainfrom
feature/jax-grad-smoke-timeout-budget
Aug 4, 2026
Merged

fix: resolve the per-script timeout parent-side and keep timeout output#227
Jammy2211 merged 1 commit into
mainfrom
feature/jax-grad-smoke-timeout-budget

Conversation

@Jammy2211

Copy link
Copy Markdown
Collaborator

Phase 1 of #226. Runner-side only — no workspace profile is changed here, so no
script's cap moves yet. Phase 2 sets the actual jax_grad/ budget once measured.

Why

Three autolens_workspace_test jax_grad scripts TIMEOUT at the 300s smoke cap
(PyAutoHeart workspace-smoke run 30858578587). Two defects in the same code path
made that both unfixable and undiagnosable:

1. A per-script budget was impossible. BUILD_SCRIPT_TIMEOUT is read once at
import into TIMEOUT_SECS and passed straight to subprocess.run(timeout=...).
The kill timer lives in the PARENT, but the per-script env from
env_config.build_env_for_script is handed only to the CHILD — so a profile
setting BUILD_SCRIPT_TIMEOUT on an overrides pattern was silently ignored.
There was no way to budget one script class without moving the cap for the
entire run.

2. A TIMEOUT could not say where it died. The TimeoutExpired handler
discarded e.stdout/e.stderr and recorded only "Timed out after Ns". A
killed script cannot report its own progress, so the CI artefact named the file
and nothing else. This is why the three timeouts could not be diagnosed from CI
output at all — and why the underlying question (real slowdown, or a cap that
never fitted?) is still open for one of the three.

Fixing (2) is what makes the measurement in phase 2 legible.

What changed

  • build_util.timeout_for(env) resolves the effective cap parent-side; used by
    both execute_script and execute_notebook.
  • The TimeoutExpired handlers keep a truncated tail of each captured stream
    and record the cap in force.
  • slow_skip_check banner now says "default" cap rather than asserting one cap
    for every script.

Precedence: profile value > ambient global > 300 default. A profile value
deliberately beats the ambient global, because run_all.py:256 exports
BUILD_SCRIPT_TIMEOUT unconditionally — even when 300 was only its CLI default
— so the parent cannot distinguish a deliberate operator cap from the default.
The opposite rule would make per-script budgets work under Heart's workflow
(which does not go through run_all) while being silently ignored locally: the
exact silent-divergence class this change exists to remove.

Malformed, zero and negative values fall back to the global rather than
disabling the cap.

API Changes

No breaking changes. Behaviour additions only:

  • New: build_util.timeout_for(env=None) -> int. Previously callers read
    build_util.TIMEOUT_SECS directly; that constant is unchanged and still the
    fallback, so existing callers keep working.
  • Behaviour: an env profile may now express a per-script cap with the
    EXISTING set: key — set: {BUILD_SCRIPT_TIMEOUT: "1800"} on an overrides
    pattern is now honoured by the parent, where before it was silently dropped.
    No profile schema change: validate_env_profiles.ALLOWED_OVERRIDE_KEYS is
    {pattern, set, unset}, so a new timeout: key would have been rejected —
    reusing set: keeps every existing profile valid.
  • Report: ScriptResult.error_message for a TIMEOUT now carries the cap in
    force plus a truncated output tail. Anything parsing that string for exactly
    "Timed out after Ns" should match a prefix, not the whole value.

No workspace or library source is affected; no release is required.

Testing

tests/test_script_timeout.py — 25 tests: resolution, malformed/zero/negative
fallback, precedence vs the ambient global, the mode=release 1800s case,
output rendering/decoding/truncation, and end-to-end kills through a real
subprocess for BOTH a matching and a non-matching script.

Control-tested against the unfixed behaviour first: with timeout_for and
_timeout_output stubbed back to the old semantics, 11 of the 25 fail
including every end-to-end case. The negative-control test (non-matching script
keeps the global cap) correctly passes in both states.

Full suite: 281 passed, 2 failed. Both failures are in
test_python_matrix_workflow.py (experimental_python_314 job key) and were
verified to fail identically on unmodified main at 9601f18 — pre-existing
and unrelated to this change.

Refs #226

The kill timer lives in the parent: build_util read BUILD_SCRIPT_TIMEOUT once
at import into TIMEOUT_SECS and passed that straight to subprocess.run, while
the per-script env from env_config.build_env_for_script went only to the CHILD.
A profile that set BUILD_SCRIPT_TIMEOUT on an overrides pattern was therefore
silently ignored — there was no way to budget one script class without moving
the cap for every script in the run.

timeout_for(env) resolves it parent-side, used by both execute_script and
execute_notebook. Precedence is profile value > ambient global > 300 default.
A profile value deliberately beats the ambient global because run_all exports
BUILD_SCRIPT_TIMEOUT unconditionally, even when 300 was only its CLI default —
so the parent cannot distinguish a deliberate operator cap from the default,
and the opposite rule would make per-script budgets work under CI while being
silently ignored under run_all. Malformed, zero and negative values fall back
to the global rather than disabling the cap.

Second defect, same path: the TimeoutExpired handler discarded e.stdout and
e.stderr and recorded only "Timed out after Ns". A killed script cannot report
its own progress, so a TIMEOUT artefact could not say WHICH block was running
— which is why the three jax_grad timeouts in PyAutoHeart workspace-smoke run
30858578587 could not be diagnosed from CI output at all. The handler now keeps
a truncated tail of each captured stream and records the cap that was in force.

No schema change: profiles express this with the existing `set:` key, which
validate_env_profiles already allows (ALLOWED_OVERRIDE_KEYS is pattern/set/
unset, so a new `timeout:` key would have been rejected).

The SLOW banner in slow_skip_check quotes build_util.TIMEOUT_SECS as the cap;
with per-script budgets that figure is the run-wide DEFAULT, not necessarily
the cap a given script ran under, and the text now says so. Understating a cap
is not cosmetic — it biases every "too slow to un-skip?" call toward parking
scripts that would pass (the 60s-cap myth, #172).

Tests: 25 covering resolution, malformed/zero/negative fallback, precedence vs
the ambient global, the release 1800s case, output rendering/decoding/
truncation, and end-to-end kills through a real subprocess for both the
matching and non-matching script. Verified against the unfixed behaviour first:
11 of them fail without this change.

Refs #226

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant