fix(agentx): close AgentX round timeout gaps + overhead-cap visibility - #1309
Merged
Conversation
All three were found by running the AgentX path end to end on GLM-5.2-MXFP4 and DeepSeek-V4-Pro (8x MI355X, TP=8, full 062126 corpus). Each one silently cost a multi-hour round. 1. Warmup was missing from the non-canonical list. The agentic warmup is what puts the cache under realistic pressure before the window opens, so 1 request/lane instead of 10 measures a materially emptier cache. aiperf has no notion of "enough warmup", so such a round came back submission_valid=true and looked publishable. On a 743B model the canonical 10/lane is a ~2h warmup, which is exactly when an operator reaches for the knob -- the hole was reachable in practice, not theoretical. Both AGENTX_WARMUP_REQUESTS_PER_LANE and AGENTX_WARMUP_GRACE_PERIOD now register as deviations. 2. The self-bracketed profile delay could open after the round had ended. AGENTX_PROFILE_WARMUP_S is a blind wall clock with no idea which phase aiperf is in, and the two failure directions are not symmetric: opening late yields NO trace (aiperf exits, the branch only warns), opening early yields a capture of a still-loaded system that TraceLens can use. Observed both ways on the same model in one day. Clamp toward early at DURATION - window - 60s and say so when the clamp bites. 3. The profile capture bound is calibrated on the synthetic shape. 128 decode steps is serialization-safe at 1024/1024; an agentic step carries a measured ISL p50 of 56k-96k tokens. On DeepSeek-V4-Pro that put each of the eight vLLM workers at 113-127 GB of HOST RAM -- Ray reported 1012/1024 GB and killed the capture, three attempts running, so the roofline arm produced no trace at all. AgentX now caps at 8; the client bounds the window by wall clock anyway, so the extra steps only inflate the in-memory event buffer. HYPERLOOM_PROFILE_MAX_ITERS still overrides. Tests: three new cases in test_aiperf_client_sh.py, negative-verified by reverting the client fix (the two "must flag" cases go red, the no-false-positive case stays green). Full file: 37 passed on a POSIX box. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d cap Found by running the AgentX path end to end with no phase disabled -- the first time that has been done, because every earlier run in this campaign passed --no-kernel and friends. A GLM-5.2 variant launched 09:47:41 died at 11:47:41.575: twenty-plus connections dropped in the SAME millisecond while the server was healthy and still prefilling with 55 requests running. A simultaneous mass disconnect against a live server is a subprocess kill, not a workload problem -- but aiperf treats a cancelled root warmup credit as terminal, so it surfaces as `warmup_failure` and the real cause never appears in the abort reason. Every variant cap is sized for the synthetic 1024/1024 shape: 7800s for integrate, 2400s for explore, 1800s for the conc sweep. A canonical AgentX warmup is 10 requests per lane over real agentic traces and runs past two hours on a 700B-class model before the measured round begins, so those caps kill the round by arithmetic rather than by chance. baseline already derives an AgentX-aware cap; only that path got it. Raised at the single choke point every variant round resolves through, reusing baseline's own resolver so there is not a second number to keep in sync, and never lowering a cap an operator asked for. AgentX is an opt-in benchmark branch: the gate means that with it off this is a no-op and the default path is untouched -- asserted directly, including with stale AGENTX_* vars present. This also retires three hypotheses that looked right and were not: concurrency contention (canonical conc=64 clears baseline with zero errors), a mis-passed --warmup-grace-period (removing it still failed at 706/707), and poisoned corpus entries (the recurring trace ids are an artefact of seed 42 fixing dispatch order; their sizes are the 9th and 88th percentile). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Second half of the variant-cap defect, found by running the fixed path further. Raising the variant cap carried GLM-5.2 to the roofline round for the first time, and then rounds kept dying with `warmup_failure` anyway. The reason is not in the round: a Qwen3.8 baseline whose server answered all 685 chat/completions with 200 was cut at exactly its 7200s timeout, mid-warmup, after which the client could no longer connect. The log names it plainly -- `baseline_executor: timeout=7200s (explicit task param)`, and 9000s elsewhere. Integrate passes an explicit timeout_sec, sized for the synthetic shape, and a canonical AgentX warmup (10/lane over real agentic traces) does not fit either value. aiperf reports the cancelled warmup credit as `warmup_failure`, so the timeout never appears in the abort reason and it reads as a workload problem. Raised at the producer, not the consumer. `_resolve_timeout` deliberately lets an explicit param outrank the AgentX derivation and has a test pinning that contract; the first attempt here overrode it and broke test_explicit_task_param_still_outranks_agentx, which is exactly the kind of deliberate decision a test exists to defend. So this sits next to _cold_start_rebaseline_timeout, which was written for the very same reason: an explicit param suppresses the executor's own sizing branch. Gated on agentx_enabled(), never lowers a larger value, and follows baseline's own resolver so there is no second number to keep in sync. AgentX is an opt-in benchmark branch and must not move the default path -- asserted directly, including with stale AGENTX_* vars in the environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…r a model AGENTX_BASELINE_OVERHEAD_SEC's default (7200s) is calibrated on GLM-5.2/Qwen3.8 measurements. A raw aiperf run against Kimi-K3 at concurrency=64 measured warmup alone draining in ~12075s, already past the whole default cap before profiling even starts. Warn when the default is used unmodified so an operator sees this before the round is silently killed hours in, instead of only after. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
|
||
| if not agentx_enabled(): | ||
| return cap | ||
| from .baseline import agentx_baseline_timeout_sec |
- baseline.py: treat an invalid AGENTX_BASELINE_OVERHEAD_SEC override (non-integer, zero, or negative) the same as unset, so the "using the default overhead" warning still fires instead of being silently suppressed by a broken value. - _grid_runner.py: _skip_rest_for_budget now gates admission on the AgentX-raised variant cap (agentx_variant_timeout_sec), not the declared one, when no per-variant estimate is available -- gating on the declared cap could admit a variant that the raised cap then gets clamped back down for, reproducing the mid-warmup kill this cap-raise exists to prevent. - _grid_runner.py: capture the measure round's actual granted timeout in a local variable and log that value on TimeoutExpired, instead of the outer variant_timeout_sec, so the timeout reported during diagnosis matches what was actually enforced. - _workload_envs.py: warn when AgentX's max-iters clamp lands below the steady-state floor, mirroring the existing warning on the manual HYPERLOOM_PROFILE_MAX_ITERS override path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- aiperf_client.sh: the _pmax safe bound for the self-bracketing profile delay only accounted for DURATION (the measurement window), not the warmup drain (bounded by WARMGRACE) that precedes it. On a long-warmup model this clamped an operator-tuned PWARM down to a fraction of what it needed to be, forcing the capture to fire mid-warmup -- the exact failure this self-bracketing exists to prevent. - aiperf_client.sh: WARMLANE/WARMGRACE non-canon checks used `!=`, so raising either above canonical (e.g. a longer grace period to let a large model's warmup fully drain) was flagged as a deviation even though it doesn't change what gets replayed. Changed to `-lt` so only a reduction is flagged. - aiperf_client.sh: CANON_WARMUP_PER_LANE/CANON_WARMUP_GRACE were declared twice with duplicated literals; now declared once, next to the WARMLANE/WARMGRACE defaults that derive from them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ate aware - request_handlers.py: _agentx_rebaseline_timeout only checked the ambient HYPERLOOM_AGENTX env var, so an integrate call driven from a subprocess that did not inherit it silently fell back to the non-agentic timeout. Now checks agentx_active(), which also honors the persisted benchmark_mode on SharedState. - _workload_envs.py: extracted agentx_active() as the shared persisted-state check (agentx_kb_write_blocked already had this logic inline) so both call sites agree on what "AgentX is on" means. - _workload_envs.py: when HYPERLOOM_PROFILE_MAX_STEPS_CAP is set explicitly and AgentX's clamp overrides it anyway, warn instead of silently overriding -- otherwise a deliberate operator setting appears to have no effect with no trace of why. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tion gaps Addresses the M1/M2/M3 findings of the PR 1309 architecture review. - baseline.py: AGENTX_BASELINE_OVERHEAD_SEC was a flat 7200s covering setup, corpus load, warmup and first-compile, calibrated on GLM-5.2/Qwen3.8. Warmup is the share that varies by model, and it already has an operator-visible bound in the client -- AGENTX_WARMUP_GRACE_PERIOD. A model whose warmup runs long is a model whose operator has already had to raise that knob, so the overhead now derives from it (5400s non-warmup + grace) instead of asking for a second, independent number meaning the same thing. At canonical settings the sum is unchanged, preserving the measured calibration. Every input is logged, and the "nothing has been tuned for this model" warning now fires only when neither knob is set. - _workload_envs.py: HYPERLOOM_PROFILE_MAX_ITERS is applied after the AgentX capture clamp and wins, which is intended -- but it lifts a host-RAM bound, not a serialization one, and neither existing warning could report it. `cap` defaults to _DEFAULT_PROFILE_MAX_STEPS (128), so the obvious override of 128 was neither below the steady-state floor nor above the cap and restored the full OOM exposure in silence. Still honoured verbatim; now warns. - aiperf_client.sh: AGENTX_FAILED_REQUEST_THRESHOLD was missing from the non-canonical list. Raising it keeps alive a run upstream's 0.10 would have aborted, and aiperf stamps no scenario marker for it, so the round came back submission_valid=true. Only a larger ratio is flagged; tightening it measures a strictly cleaner run. CANON_FRT is declared once and feeds the default, so the canonical value and the default cannot drift apart. - Cross-referenced the three synthetic-sized variant-timeout defaults with agentx_variant_timeout_sec, and added a CHANGELOG entry with operator notes for the two behaviour changes (derived cap, submission_valid on a raised failure threshold). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The AgentX-off counterpart of the MAX_ITERS override test asserts only on log records, so it never renders the materialized config and does not need yaml. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI runs `ruff format --check .` (lint.yml), which this branch has been failing since 0c15594 -- the warmup non-canon assertions added there, and the failed-request-threshold ones added on top of them, were hand-wrapped rather than formatted. Formatting only, no behaviour change; the three files pass on a clean checkout (69 passed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… scrub TCP_USER_TIMEOUT bounds how long Linux tolerates an established connection making no progress, and an agentic turn against a long-context model makes none for as long as the server is prefill-bound. aiperf's stock 30s therefore aborts otherwise-live connections mid-prefill, which surfaces as a warmup failure with no server-side error to match it. Upstream's Kimi-K3 arms (benchmarks/single_node/agentic/kimik3_fp4_mi355x_mtp.sh and the atom/b300 variants) and the DSv4 SGLang arms all export 900000 for exactly this reason -- the DSv4 comment names the 30s default it is overriding. This file scrubs every inherited AIPERF_* except AIPERF_BIN, so an operator who exported it upstream had no effect and the client kept the stock bound. Exported after the scrub like every other AIPERF_ setting here, tunable through AGENTX_HTTP_TCP_USER_TIMEOUT. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI E2E report — ❌ Timeout
|
…-closeloop-fixes # Conflicts: # CHANGELOG.md # src/hyperloom/orchestrator/actions/executors/_grid_base.py # src/hyperloom/orchestrator/actions/executors/integrate_patch.py
The Magpie benchmark config's flat timeout_seconds (7200s default from baseline_vllm.yaml) is one wall-clock deadline over server boot + warmup + the measurement window + result export. At the model's native context an AgentX baseline's boot+warmup alone overruns it, so Magpie SIGKILLs the benchmark before aiperf writes inferencex_result.json -- a 0-tput baseline that fails the session, while the raised outer subprocess cap goes unused (the two layers were left inconsistent). Set bench["timeout_seconds"] in apply_agentx_switch to the same agentx_baseline_timeout_sec() the outer cap already uses, so the inner and outer caps stay consistent. AgentX-only: the switch returns early when AgentX is off, so the default (synthetic) cap is untouched. Measured on Kimi-K3 (vLLM, TP8, conc8, 1M ctx): boot+warmup ~46 min + a 3600s window overran the 7200s cap and the baseline landed at 0 tok/s despite a clean profiling run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-closeloop-fixes Resolve _grid_base.py conflict on the DEFAULT_VARIANT_TIMEOUT_SEC comment: keep this branch's AgentX-aware note (points at agentx_variant_timeout_sec, explains the synthetic-shape sizing) as a superset of main's shorter reword. Code line unchanged. All other files auto-merged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The AgentX baseline cap derives its warmup share from AGENTX_WARMUP_GRACE_PERIOD, one flat number. But the client builds warmup as CANON_WARMUP_PER_LANE requests per lane across CONC lanes, so the work is linear in CONC by construction: a grace calibrated at one concurrency under-budgets every higher one, and the round is killed mid-warmup -- the exact failure the cap-raise exists to prevent, moved one axis over. Measured on Kimi-K3: conc=8 -> 87 warmup requests (~3000s), conc=16 -> 177 (~5000s), conc=64 -> the ~12075s the existing warning already cites. The warmup share now carries a CONC-scaled floor anchored at CONC=8, the lowest concurrency this repo has a measured agentic warmup for. At or below the anchor the derivation is unchanged, so every previously-validated round keeps its exact cap; above it the floor only ever raises. An over-large cap costs a longer wait on a round that was hung anyway (and the session budget clamps it regardless), while an under-sized one kills a round that would have finished. AgentX-only: nothing on the synthetic path reads this derivation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four budget gates -- one in the grid runner's _skip_rest_for_budget and three in the conc sweep, plus the sweep's session soft deadline -- priced a variant at the DECLARED variant_timeout_sec. Under AgentX that is not what the round gets: agentx_variant_timeout_sec raises the cap before launch. Pricing at 1800s while granting 10800s admits a variant the budget cannot pay for; the round then has its cap clamped back to the remaining time by session_clamped_timeout_sec and is killed mid-warmup. Same for the soft deadline: 1800s ends an agentic round before its measurement window opens. All five sites now go through the raised cap. In the grid runner the estimate branch takes the max only when the cap was actually raised -- with AgentX off the helper is the identity, and without that gate the default path would start charging every variant its full backstop instead of its expected cost, skipping variants that fit comfortably. The sweep's helper is likewise the identity with AgentX off, so the synthetic sweep prices and paces exactly as before. Verified: 146 grid-runner and 89 conc-sweep cases pass, including the pre-existing budget-admission and session-kill-attribution locks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two defects in the AgentX client wrapper. 1. The client sets AIPERF_HTTP_TCP_USER_TIMEOUT to 900s to tolerate a slow first prefill, but nothing raised the SERVER's keep-alive: vLLM defaults to 5s. The server closes an idle connection long before the client gives up, which surfaces as ServerDisconnectedError during warmup -- terminal, after a full weight load. The wrapper now defaults the framework's own keep-alive knob (VLLM_HTTP_TIMEOUT_KEEP_ALIVE / SGLANG_TIMEOUT_KEEP_ALIVE) to the same tolerance the client was already given, overridable via AGENTX_HTTP_KEEP_ALIVE_S and never overwriting an explicit setting. 2. AGENTX_FAILED_REQUEST_THRESHOLD was interpolated straight into an awk program, so its value was executable code. It and the two warmup knobs are now validated as numbers before use, and the comparison passes the value through awk -v instead of the program text. Verified by revert: neutering the validators fails 6 of the 56 cases in this file; with them, 56 pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
shlex.split defaults to posix=True, which strips the quote bytes -- so a flag whose value is a JSON blob (the KV-connector extra_config among them) came back out of remove_server_args as unparseable text. Splitting now preserves the quoting and warns when a JSON value did not survive the round trip. Taken from origin/feat/zgong/agentX-kimi-k3, which hit the same wall on the same model. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The AgentX inner-timeout raise was an unconditional assignment, so a config that declared MORE than the AgentX derivation had its timeout cut -- profile_sglang's 14400s became 10800s. The comment already said "raise". Now it takes the max and logs when the config's own number wins. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This branch introduced a second "## [v1.0.0] - 2026-08-26" heading with a four-line preamble; origin/main has one. Restores the single-heading structure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three corrections to the Kimi-K3 launch guide, all from measurements taken on an 8xMI355X pod over the last two days. CPU KV offload (new section). Without the official SimpleCPUOffloadConnector, conc=8 measured prefix_cache_hit=4.0% against a 94% theoretical ceiling; with it, 68.5% plus an ext_cache_hit of 80.7%. The guide now states the official three-part configuration verbatim and, more importantly, how to size cpu_bytes_to_use_per_rank: from the CGROUP limit, not from `free`. Inside a container `free` reports the host -- 2751 GiB on that pod against a 2048 GiB cgroup cap -- and a pool sized off the host number put worker RSS at 222.9 GiB/rank with cgroup usage pinned at 99.998%, leaving no page-cache headroom. Also records the two things that bite: expandable_segments:True is a hard incompatibility with any KV connector, and ext_cache_hit was still climbing when host memory ran out. Concurrency table. 8/16/32 were listed as untested; 8 and 16 now have evidence (8 is clean, 16 carries a ~30% error floor), and the entry says why "fixing" that error floor by raising AGENTX_FAILED_REQUEST_THRESHOLD makes the round submission_valid=false and loops PRELUDE forever. Timeout escape hatch. The old advice -- pin AGENTX_BASELINE_OVERHEAD_SEC, "no validated formula" -- predates the derivation. The cap is now built from AGENTX_WARMUP_GRACE_PERIOD with the warmup share scaled by CONC/8, so the guide points at that input, says to anchor it at the CONC=8 measurement (the floor multiplies whatever it is given), and notes that AGENTX_BASELINE_TIMEOUT_SEC short-circuits the scaling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The CONC scaling added in 46dd949 lived inside agentx_baseline_timeout_sec, so only the subprocess cap saw it. aiperf_client.sh reads the SAME env var and passes it to aiperf as --warmup-grace-period -- and that is the number that actually stops the warmup. The AGENTX_* forwarding loop in apply_agentx_switch copies the operator's raw value, so the client stayed bounded at the unscaled grace while the cap paid for the scaled one. Caught live on a Kimi-K3 conc=32 round: cap 23400s (of which 14400s warmup) against a client bound of 3600s. Warmup was measured returning 1.76 req/min against a 354-request corpus, so the client would have cut it at ~106/354 -- not a crash, which is what makes it dangerous: the round completes and reports a prefix-reuse figure taken before the cache held anything. The server was still showing External prefix cache hit rate 0.0% at that point. The scaling now lives in one function, agentx_warmup_grace_sec, that both consumers call: the cap derives its warmup share from it, and apply_agentx_switch exports its result into the benchmark env instead of forwarding the raw variable. The cap's docstring has claimed since M1 that "the client honours it too, so the warmup and this cap stay consistent" -- this makes that true rather than aspirational. Default path untouched, and locked by test: apply_agentx_switch returns early when AgentX is off and for scriptable frameworks, so neither gets an envs block at all. Both early returns now have a test that sets a grace and a CONC and asserts nothing is written. Verified: 88 cases on the two touched files, 534 across the AgentX and default-path suites. Revert-proof -- restoring the raw forward fails exactly the two export tests and nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
apply_agentx_switch exports the CONC-scaled warmup grace during the config rebuild -- which happens BEFORE _build_variant_yaml merges the variant's extra_envs, and the variant's CONC only exists as of that merge. So the grace every variant received was scaled by the SESSION concurrency (cli writes --conc into os.environ), not by the concurrency the round actually runs at. For a conc sweep that is wrong by construction: the ladder walks 256..2 while the session sits at one value. Verified against the real path -- session CONC=8 with a 3600s anchor handed a CONC=128 variant a 3600s bound where it needs 57600s. The client's --warmup-grace-period is what stops the warmup, so that variant's warmup ends mid-corpus and the round reports numbers taken before the cache was populated. The grace is now re-derived after the merge, from the OPERATOR's anchor plus the VARIANT's CONC. Not from the value already in envs: that one has been scaled once, and feeding it back would scale it twice -- a CONC=32 session exporting 14400s would hand a CONC=128 variant 230400s instead of 57600s. There is a test for that specific arithmetic, because it is the obvious way to write this wrong. Only ever upward, so a low-CONC variant keeps a larger bound the session already paid for rather than having it taken back. Default path untouched: the branch is gated on agentx_enabled(), and nothing writes AGENTX_WARMUP_GRACE_PERIOD into a synthetic variant's env anyway. Locked by test_the_default_grid_never_re_derives_a_grace, which sets both the anchor and a CONC with AgentX off and asserts the key is absent. Verified: 35 cases on the touched file, 656 across the grid, conc-sweep, AgentX and default-path suites. Revert-proof -- disabling the re-derivation fails exactly the two variant tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…asured at AGENTX_CANON_WARMUP_CONC was hardcoded at 8 and not overridable, so the CONC scaling encoded an assumption nobody ever states: that AGENTX_WARMUP_GRACE_PERIOD was measured at concurrency 8. An operator who measured 14400s at CONC=16 and passed it in got 28800s back, silently. That is a coupling to this repo's own measurement history rather than to anything about the workload -- 8 is simply the lowest concurrency we happen to have an agentic warmup for. The scaling itself is model-independent (warmup is per-lane requests x CONC lanes, linear by construction); only the anchor was parochial. Evidence it is a bad interface: it forced me to hand-convert. env.sh carried AGENTX_WARMUP_GRACE_PERIOD=14400, tuned for CONC=16, and to get correct scaling I had to enter 3600 instead -- a number that is not a measurement of anything, just 14400 divided by the anchor I could not declare. AGENTX_WARMUP_GRACE_CONC now declares the anchor, defaulting to 8 so every existing configuration derives exactly what it derived before. The ratio becomes CONC / declared-anchor, and the operator enters the number they measured next to the concurrency they measured it at. The anchor travels with the grace into the per-variant re-derivation too; dropping it there would leave a variant anchored at the repo default while the baseline used the operator's -- two rounds of one session disagreeing about what the same number means. Validation is fail-safe: an unset, zero, negative or non-integer anchor falls back to the default rather than reaching the division. Verified in a detached worktree so the live benchmark's checkout was not touched: 106 anchor cases, 726 across the AgentX, grid, conc-sweep, aiperf-client and default-path suites. Revert-proof -- restoring the hardcoded anchor fails exactly the six anchor-dependent cases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-closeloop-fixes # Conflicts: # CHANGELOG.md # src/hyperloom/orchestrator/actions/executors/_grid_server_args.py
Comment on lines
+216
to
+219
| from hyperloom.orchestrator.actions.executors.baseline import ( | ||
| agentx_baseline_timeout_sec, | ||
| agentx_warmup_grace_sec, | ||
| ) |
…an main Four defects from the previous four commits, all of which happened to be no-ops at CONC=8 -- the canonical setting the closed-loop run was validated on, and the one point where every scaling factor is 1. 1. A sweep variant's grace was re-derived from its own CONC while the two caps that bound it stayed session-scaled. Reproduced: at session CONC=8 a CONC=256 variant got a 115200s grace against a 12600s subprocess cap, so the round is SIGKILLed at 12600s while warmup is still draining. That is strictly worse than the 3600s grace that shipped before, i.e. a regression. Reverted; all three numbers are session-scaled and consistent again. Making them variant-aware means threading the merged CONC into agentx_variant_timeout_sec and _round_timeout_sec, which is a separate change. 2. The same block's log call had four format placeholders and three arguments, so every AgentX variant raised TypeError inside logging and printed a traceback instead of the one line that would have diagnosed (1). Gone with the revert. 3. agentx_warmup_grace_sec scaled the canonical default even when nothing was set. An untuned CONC=64 round derived a 23400s cap where the documented canonical total is 10800s -- 3.2x -- and the comment claiming "unchanged at canonical settings" was only true at CONC=8. A constant is not a measurement: with no operator value there is nothing to scale, so the canonical grace is returned flat at every concurrency and the existing "nothing has been tuned for this model" warning does the talking. That also restores the warning's own arithmetic, which printed a scaled overhead against fixed decomposition terms. 4. conc_sweep priced every rung at the granted AgentX cap (correct -- run_grid does grant it) against DEFAULT_TOTAL_BUDGET_SEC=9000, which cannot fund one rung at 10800s. Every rung was skipped as insufficient_remaining_for_variant and the sweep produced zero measurements, which reads as a benchmark failure rather than a budget never sized for this workload. The CLI already raises this knob for AgentX; callers reaching run_conc_sweep directly did not. The floor is now applied at the point of use, only when the caller left the default in place, and it buys two rungs rather than the whole ladder. Also: _agentx_positive_int accepted only bare integers, so an anchor written "16.0" was discarded and the grace silently re-anchored at 8 and doubled -- the exact failure AGENTX_WARMUP_GRACE_CONC exists to prevent. Whole numbers in decimal or exponent form are now accepted; fractional ones are still rejected. agentx_baseline_timeout_sec's two local parsers now delegate to it, so a value one accepts and the other rejects cannot resurrect the suppressed-warning mismatch. KIMI_K3_LAUNCH_GUIDE.md is removed from the repository root; it is operator documentation for one model, not part of this change, and now lives outside the tree. Verified against the merged HEAD in a detached worktree: 704 cases across the AgentX, grid, conc-sweep, server-args and default-path suites. Each of the four defects has a reproduction recorded above and a test that fails without the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…idate the
profile-window knobs
Two client-side defects.
The keep-alive arm matched against "${FRAMEWORK}${BUILTIN}" -- a concatenation,
not a choice. BUILTIN defaults to ${FRAMEWORK}_${GPU}.sh, so the two can only
disagree through AGENTX_SERVER_SCRIPT, which is exactly how it happens in the
field: a stale FRAMEWORK reaches the client through persisted state while the
operator pins the script explicitly (the launch recipes do pin it, precisely
because of that leak). FRAMEWORK=vllm with an sglang script formed
"vllmsglang_mi300x.sh", hit the *vllm* arm first, and exported the vllm knob --
leaving SGLang on its 5s idle default while the very next log line reported
900s. The server then closed the socket mid-warmup and the round died as "root
AgentX warmup request failed", with the one diagnostic line actively denying the
cause. The script that boots now decides; FRAMEWORK is a fallback for a script
name that carries no framework, and a disagreement is logged rather than
resolved silently. A script matching neither says so instead of leaving the
reader to infer that a knob was set.
AGENTX_PROFILE_WARMUP_S / AGENTX_PROFILE_WINDOW_S / AGENTX_DURATION now go
through the same _require_uint the other measurement knobs got. All three reach
both `$(( ))` and `[ -gt ]` in the profile-delay clamp, and those two constructs
mishandle a non-integer in opposite directions: `$(( ))` aborts the whole round
under set -e, minutes from the measurement window, while `[ -gt ]` exits 2 --
which set -e exempts as an `if` condition, so the clamp silently does not fire
and the capture lands after the round ended. No trace, and the "exceeds the safe
bound" warning never printed. Those are the two failure modes the adjacent
comment already described; they are now rejected at the door with the knob named.
Verified: 62 cases. Revert-proof -- restoring the concatenation fails exactly the
two mismatch cases and nothing else.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…uiet the derivation logs The JSON-damage tripwire compared ``composed`` -- already the output of the one or two ``remove_server_args`` calls above it -- against the final string. Damage done inside those calls therefore made the "before" side unparseable too, ``healthy_before`` False, and the tripwire silent on exactly the failure its own docstring describes. It now compares against the raw inputs, so the removal pass is inside the window. Flags a removal spec deliberately dropped are still not reported: the loop walks what survived. The same file's tokenizer is on the DEFAULT path, which the PR note glossed: ``compose_server_args`` always calls ``strip_benchmark_harness_flags``, i.e. ``remove_server_args`` with a non-empty denylist, so every synthetic grid variant goes through it. Five cases now lock that behaviour with HYPERLOOM_AGENTX unset -- plain args round-trip, the denylisted flag is dropped, quoted operands lose their wrappers (Magpie expands EXTRA_*_ARGS unquoted, so a wrapper would reach argv literally), an unbalanced quote leaves the string untouched, and a synthetic JSON value survives a removal. The integrate path called SharedState.load_or_init unguarded, after the kernel patch had already been applied, to answer the advisory question "is this session AgentX". A truncated or concurrently-written state.json would raise there and discard work that had succeeded. It now falls back to the HYPERLOOM_AGENTX env signal with a warning, which differs only for a run resumed into a shell that lost the variable -- and there the cost is the un-raised timeout that shipped before. Finally, both derivation log lines are emitted once per distinct payload rather than once per call. An 8-rung, 2-arm sweep printed the eight-line untuned-overhead warning 16 times and buried the per-variant progress this PR added -- a regression in the thing the logging was for. Keyed on the formatted values, so a configuration that actually changes still speaks up; tests clear the ledger in their existing _clear() helper. Verified on top of the previous two batches: 774 cases across 16 suites. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…st env var
Two defects from the review, both verified by reproduction before the fix.
The warmup floor scaled on an ASSUMED anchor. A ratio needs two numbers, and
defaulting the second to 8 made the same value mean different things depending
on whether it was typed: with CONC=64, leaving AGENTX_WARMUP_GRACE_PERIOD unset
derived a 10800s cap while writing the canonical 1800 explicitly derived 23400s.
The conc sweep then prices every rung against the inflated number and skips most
of the ladder. Scaling now requires AGENTX_WARMUP_GRACE_CONC to be declared:
no anchor, no ratio. Measured after the fix, unset and explicit-1800 agree at
10800s for CONC=8/16/64/256, and declaring the anchor still yields 14400s/23400s
at CONC=32.
An unusable anchor ("abc", "0", "-8", "8.5") now disables scaling rather than
falling back to the default. A typo in a declaration is not a declaration, so
the grace passes through unscaled instead of being multiplied against a number
the operator never wrote.
agentx_variant_timeout_sec consulted only HYPERLOOM_AGENTX. That is the original
report's own scenario: a session resumed into a shell without the variable, or a
variant round driven from a subprocess that did not inherit it, reads as
synthetic and the round is killed by the synthetic cap mid-warmup -- the failure
this helper exists to prevent, reached by the one route it did not cover.
benchmark_mode is stamped at seed for exactly this. It now takes an optional
shared_state and asks agentx_active. Reproduced: with no env var and no state
the cap stays 1800s; with state.benchmark_mode="agentx" it rises to 10800s; with
"synthetic" it stays 1800s.
The conc sweep passes its state at all five gates. run_grid's own call sites do
not yet -- threading state through nine call sites in six files is a change of
its own, and agentx_active(None) is exactly today's behaviour, so those paths are
no worse than before while the sweep gets the durable signal. The gap is named
in the docstring rather than left to be rediscovered.
Default path untouched: both changes sit behind the AgentX check, and
agentx_variant_timeout_sec with shared_state=None or a synthetic benchmark_mode
returns its argument unchanged.
Verified: 716 cases across 15 suites. Revert-proof -- removing the anchor gate
fails 9 cases, restoring agentx_enabled fails exactly the persisted-state case.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| # Local import: baseline imports from this module, and the rest of the file | ||
| # already resolves _workload_envs this way. | ||
| from ._workload_envs import agentx_active | ||
|
|
…ced nothing
The stability gate requires a nonzero file count, so a profiled round where no
trace file ever lands can never satisfy it -- the loop spins out the whole
AGENTX_TRACE_FLUSH_TIMEOUT_S (1800s by default) and, on a sweep, pays that once
per profiled round. It also broke three existing profile tests in CI, whose
sandbox has no profiler output directory at all: they hit pytest's 120s cap.
Two bounds, for the two ways "no files" happens:
- No profiler output directory is configured at all: nothing was ever pointed
anywhere, so there is nothing to flush. Return before entering the loop.
- The directory exists but stays empty: a rejected /start_profile or an
unwritable dir. Bound that case with AGENTX_TRACE_FIRST_FILE_TIMEOUT_S
(default 900, clamped to the flush budget so lowering only the budget still
takes effect). The first rank file landed at t+350s on the GLM-5.3 TP=8
capture, so 900 keeps real margin over a merely slow flush.
Neither path fails the round: the measurement is unaffected by a failed capture.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
xiaofei-zheng
approved these changes
Sep 2, 2026
2 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.
Summary
The AgentX closed loop could not complete a run. Rounds were killed mid-warmup by caps sized for synthetic workloads, the budget admission check priced rounds at a cap they were never granted, JSON-valued server args were corrupted on the way to the launcher, and a profiled round tore the server down while the trace was still being written. This PR closes those gaps.
Everything here is scoped to the AgentX path. There is no runtime flag: isolation comes from the
assets/agentx/asset directory plusagentx_*-named functions that return early whenagentx_enabled()is false. The one file that touches shared code is called out under Known limitations.What changed
Round timeouts no longer kill a canonical warmup
_grid_runner.py/baseline.py: an AgentX variant's hard cap is raised to the AgentX baseline cap rather than the synthetic 1800s/2400s/7800s. A raw aiperf run against Kimi-K3 at concurrency=64 measured warmup alone at ~12075s — past the entire default cap before the measurement window even opens.request_handlers.py: the integrate/re-baseline timeout is raised where its explicittimeout_secis produced, preserving the existing "explicit param outranks AgentX derivation" precedence._grid_base.py: an outer cap is never lowered below what the config already declared.timeout_seconds=7200) is raised to match. Without this it silently nullified the raised outer cap — the outer cap was correct and the inner one killed the round anyway.Warmup grace is derived, not guessed
AGENTX_WARMUP_GRACE_PERIODnow scales only when the operator also declares the concurrency it was measured at (AGENTX_WARMUP_GRACE_CONC). Without an anchor there is nothing to scale from, and the earlier unconditional scaling made CONC>8 rounds worse thanmain._agentx_positive_intaccepts whole numbers written in decimal or exponent form ("16.0","1e3") and rejects fractional ones.Budget admission is priced at the cap the round is actually granted
conc_sweep.py:_granted_cap_sec()takes the raised cap, not the declared one, and a floor keeps at least two rungs fundable when the default budget cannot fund even one. The raise is gated on_cap_was_raisedso non-AgentX variants keep being priced exactly as before.Profiled rounds keep their trace
aiperf_client.sh: after/stop_profile, wait for the per-rank files to be complete (one per rank) and stable (unchanged size across three samples) before letting teardown run. A 200 from/stop_profileonly means the tracer was told to stop. Measured on GLM-5.3, sglang, TP=8, one 20s window: the first rank file appeared 350s after the call returned, all eight at 391s, and the set was still growing at 546s on its way to 5.1 GB.cleanup()allows 20s — a 27× gap, so every capture before this was killed mid-write and failedgzip -t.AGENTX_TRACE_FIRST_FILE_TIMEOUT_S(default 900) rather than the full 1800s flush budget. A capture that produced nothing is a failed capture, not a slow one.Knob handling
AGENTX_PROFILE_WARMUP_S/AGENTX_PROFILE_WINDOW_S/AGENTX_DURATIONare validated as unsigned integers instead of being fed straight into arithmetic.AIPERF_HTTP_TCP_USER_TIMEOUTis re-stated after theAIPERF_*scrub that was silently dropping it._say_once) — they were firing once per call.Default-path guarantee
The stated constraint on this work is that nothing may change for a non-AgentX run.
agentx_active(shared_state)/agentx_enabled()early return, or behind a knob that is unset by default.agentx_active()prefers the persistedbenchmark_modeover the ambientHYPERLOOM_AGENTX, so a stale environment variable cannot switch behavior mid-run.max()broke 13 default-path tests and was fixed._grid_runner.py; it was reverted in this branch and replaced with a comment explaining why, after review flagged it as reaching outside the AgentX path.E2E evidence
GLM-5.3-Flash, sglang, MI355X — full closed loop.
GLM_RC=0,stop_reason=conc_sweep_done, phasesPRELUDE → KERNEL_AGENT → SWEEP → CLOSE, baseline 76.6 tok/s/GPU, TTFT 682ms. KV pool 8,761,024 tokens / 103.75 GB, matching the official run exactly.Trace capture proven possible, separately: 8 rank files / 5.1 GB landed from an isolated capture. Those particular files are gzip-corrupt because the server was killed at t+546s — which is the measurement that motivated the flush fix above, not a result produced with it.
Known limitations
_grid_server_args.pyis the one file here that is not AgentX-specific. It fixes JSON-valued server args (--compilation-config {...}) being corrupted by a POSIX split followed by a quote-rebuilding heuristic that cannot reconstruct["+fused_rms_norm_gated"]. K3's real launch args need it. A 13-case matrix run against both this branch andmainshows this version scoring 9/13 vsmain's 10/13 — it fixes the JSON casemainbreaks, but regresses quoted-value spec matching. A strictly better implementation exists (12/13, losing to neither, and fixing an unquoted multi-word spec case that both versions fail) and is not in this PR. Splitting this file into its own PR is the cleaner call if reviewers prefer.worker_crashduring profiling at 18/172 failed requests (10.5%, over the canonical 10% threshold). Not root-caused, deliberately not guessed at.by_kernelentries; the flush fix has not yet been exercised by a complete run.--enable-shape-discovery-for-cuda-graph-profileinjection has no capability probe; a framework that does not support it fails the round.Test plan
Run on Linux (these tests do not collect on Windows —
fcntl).test_aiperf_client_sh.py— 69 passed (includes the 3 profile tests that CI failed on before this fix, plus 4 new ones covering the two empty-capture bounds)src/hyperloom/inference_optimizer/tests/regression — running; result to be posted