Set --pids-limit in docker-mode launch templates (fixes silent hangs + OpenBLAS crashes under Ray + TP>1) - #1276
Open
skariapaul wants to merge 1178 commits into
Open
Set --pids-limit in docker-mode launch templates (fixes silent hangs + OpenBLAS crashes under Ray + TP>1)#1276skariapaul wants to merge 1178 commits into
skariapaul wants to merge 1178 commits into
Conversation
Detect the v0.5.17+ AOT kernel at python/sglang/kernels/aot/setup_rocm.py instead of falling back to CUDA-only PyPI sglang-kernel wheels.
AgentX was not producing leaderboard-comparable numbers, and the reasons were not only the missing upstream flags -- the measurement itself was wrong in ways that produced plausible results rather than errors. Four defects, each of which silently reported a clean run: - The replay context was capped from $MAX_MODEL_LEN, which is derived from the synthetic ISL+OSL shape (6144 at the defaults). aiperf DROPS traces above the cap rather than truncating them, so the 393-trace corpus was reduced to its short-trace tail. Two independent paths did this: the client's --max-context-length, and sglang's --context-length via an ISL+OSL ceiling. MAX_MODEL_LEN under AgentX now resolves to the model's own context window. - --failed-request-threshold was never passed, and aiperf defaults it to None (check disabled), so a run whose requests mostly errored still exited 0 and was scored. map_aiperf.py carries no error counters, so nothing downstream could notice. - AgentX sets RUN_EVAL=false in the YAML, which the baseline's missing-accuracy guard explicitly rejects as an excuse. A good AgentX baseline was stamped an eval failure, never anchored baseline_tput, and left every variant's gain None. AgentX now routes through the same deliberate opt-out as --no-eval. - The scenario's own submission_valid verdict was produced by aiperf and never read. It is now carried through and gates KEEP; absent is treated as unknown, not valid. Command line now mirrors upstream build_replay_cmd() (scenario, 062126 corpus by the upstream model-family whitelist, 3600s window, seed 42, trajectory ratios, per-lane warmup, error-rate gate, 393 entries). aiperf and InferenceX are re-pinned to the leaderboard's current head; the previous aiperf predates the 062126 corpus and its scenario allowlist rejects it outright. Because a round now costs orders of magnitude more wall-clock, the per-variant budgets are widened under AgentX and the search scope is reduced: the concurrency sweep is off by default (16 rounds would consume the entire budget, and the leaderboard treats each concurrency as its own row) and the ISL/OSL fan-out collapses to one point (the agentic client never reads ISL/OSL, so the three points ran byte-identical work under different labels). --max-hours is deliberately NOT raised: it is the operator's contract with the scheduler. Guards added for the ways a run could still measure something other than what it claims: AgentX + the bypass backend is refused outright, resumes assert the benchmark mode and an AgentX measurement epoch, and variant proposals are stripped of knobs that redefine the workload (corpus, concurrency, context contract) rather than tune it. All AgentX behaviour is gated on HYPERLOOM_AGENTX; the synthetic path is unchanged (349 shared-module tests pass with the switch off). Co-Authored-By: Claude <noreply@anthropic.com>
Treat empty geak_pending as untracked-by-design for promote and failure paths, apply the orphan gate to no_material and no_promote, and finalize geak_pending when CLOSE cancels queued rebench tasks. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…7-kernel-install Fix baremetal SGLang install for v0.5.17 kernel layout
The platform record globbed `0000:*` under the amdgpu driver directory, hardcoding PCI domain 0000. A host with more devices than one domain can address puts its GPUs under `0002:`, `0003:` and so on -- the normal layout on an MI300/MI350-class node -- so the count came back empty there and the run report read `accelerators: ?x gfx950` on an 8-GPU host. The field failed on exactly the hardware it exists to describe. Move the count into `amdgpu_device_count()` so it follows the injectable `root` convention the rest of this module documents, which is also why the hardcoded absolute path had no test covering it. Co-authored-by: Cursor <cursoragent@cursor.com>
`detect_stack_fingerprint` resolves versions with `importlib.metadata`, which only sees the interpreter running the orchestrator. `--framework-env isolated` is the default for vLLM -- its ROCm wheel pins its own torch, so it must not share that environment -- which means the framework is installed where this lookup cannot reach it. Every bare-metal vLLM run therefore recorded the framework it actually served with as `vllm unknown`. Fall back to the venv root setup already records in `.env` (`VLLM_VENV_ROOT`) and read the distribution metadata from its site-packages. Env pins keep priority and the scan stays behind `probe=True`, so the hermetic contract of `probe=False` is unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
Five defects found while replaying the claw campaign against this branch. Each is keyed on evidence that holds campaign-wide, not on one session. B1 attribution: a GEAK revalidation dispatches as a plain `explore` task, so its action label says `explore` and only its provenance says GEAK. `_entry_family` now consults provenance, reusing the existing `_CONFIG_REPLAY_PROVENANCE` set rather than adding a second list. Without this every GEAK-driven gain is bucketed as generic exploration. B2 alias twins: GEAK records one acceptance twice -- the candidate-slot row carries `gpu_pct`, the resolved profiler symbol carries `gpu_pct: null`. `_collapse_journey_aliases` keeps the measured row (it is the only one holding the number) and names it by the symbol (it is the id the acceptance ledger keeps). Naming it by the slot tag put one kernel under two names in two tables of the same report. B3 kernel_integrate_attempts: the GEAK path never wrote the ledger, so dead-run recovery read a file that is never produced. `_record_geak_ candidate` now writes it. Recovers `MiniMax-M3-MXFP8/20260731T182731Z` (`_mxfp8_linear_kernel`, +40.626%). B4 status gate: `no_gain` is GEAK's verdict on its own headline number, not on the kernels it accepted. A run can report `no_gain` on the promoted basis while carrying an accepted kernel with a positive parity-checked same-config A/B. The gate now admits on `_geak_has_accepted_kernel`, and the rebench downstream still decides. Of 38 `no_gain` results in the campaign exactly one flips (`GLM-5.2-MXFP4/20260814T163244Z`, `dsa_sparse_attn_prefill_main_kernel`, +29.994%); the other 37 stay declined. B5 overlay gate: a config-only `_overlay_manifest.json` imports cleanly and installs nothing, so "the overlay imported" was never evidence a kernel ran. `_geak_overlay_is_loadable` requires `sitecustomize.py` and at least one module or rebind; `overlay_loaded_2a` also checks the digest matches what the task was dispatched with. 64 runs declare `final_overlay`, 9 are loadable -- 55 false "kernel ran" credits removed. `_geak_has_accepted_kernel`, `_geak_overlay_is_loadable` and `_geak_overlay_digest` live in `loop/coordinator_helpers.py` as the single owner; `phases/kernel.py` and `loop/writeback.py` import them. Tests: three new files (37 tests) plus two assertion updates in `test_geak_breakdown_unit.py` that still expected the pre-B2 spelling. 80 tests pass across the five GEAK test files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Re-running the collector A/B over all 98 claw sessions showed ``kernels_optimized`` falling 31 -> 26 on the recovery path, and one session (Qwen3-14B-FP8/20260814T163051Z) going 2 -> 0 while discarding a +14.924% KEEP. That read as a regression. It is not. The journey holds an alias twin whose resolved symbol is a CK library GEMM. ``result.json`` ships an empty ``accepted_kernels`` and puts the win in ``accepted_heads`` with ``kind: env``. The collapse names the surviving row by the symbol, the kind join then finds GEAK's own declaration, and the row is excluded from the kernel bucket -- correctly. The e2e gain is untouched; it is attributed to config, which is what it is. ``main`` reported 2 only because it neither collapsed the twin nor read the kind. Checked campaign-wide rather than on this one run: of every symbol ``result.json`` names across the 68 sessions holding one, exactly 2 are not covered by a kept row, and both are declared ``env``. Non-env exclusions: 0. The two tests pin both directions, because the exclusion is only safe while it stays narrow: a declared ``env`` twin collapses to 0, and the same twin declared ``authored`` survives at 1 with ``kind_source: result_json``. A row no lane names stays admitted as ``kind_source: absent`` -- guessing "env" there would delete real kernels from dead runs, which is the loss this collector recovers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The kind join read only `result.json`. That file is rewritten once per cycle and the last write wins, so a later cycle that accepts nothing blanks the lanes an earlier cycle declared. On Qwen3-14B-FP8/20260816T050457Z it names 0 lanes while the run accepted three rows: every recovered row came out `kind_source: absent` and the `kind == "env"` exclusion could not run on them at all. The `action == "geak_e2e"` entries of `state.optimization_stack` carry the same two lanes in the same spelling -- KernelPhase copies them from the result of *that* cycle -- and the stack is append-only, so it keeps what the file lost. `collect_geak` already receives the parsed state, so the second source costs one argument threaded through, not a new reader. Precedence keeps the run's own words authoritative: a declared kind beats an undeclared one whichever artifact holds it, and between two declarations `result.json` wins. So the stack can only fill gaps, never overwrite a published kind. `kind_source` gains `stack` / `stack_undeclared` beside the existing `result_json` forms, so a stack-sourced kind is never reported as something result.json said, and the untyped residual stays countable. Replay over all 98 claw sessions is byte-identical (26 recover, 23 live): every campaign `state.json` is root mode 600, so the replay synthesises state from `result.json` and cannot reach a stack. The change is strictly additive there; the three new unit tests cover the behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reconciles 15 commits from docs/1.0.0a1 (2026-07-22 to 2026-07-23, from peer/tech review that was never merged) against main, which had advanced roughly 800 commits past the merge-base in the interim. Conflicts were resolved file-by-file: main's content was kept throughout except in docs/conf.py, where the "hyperloom" theme flavor and header button/title settings from the review branch were preserved since main had not reintroduced them. Peer/tech-reviewed wording elsewhere had already been superseded by later, distinct work on main (feature rewrites, cleanup PRs, or the reviewing author's own follow-up commits).
A real AgentX round (Qwen3.6-35B-A3B, TP8, conc 64, 3600s window) takes 111 min: 46 min of per-lane warmup, the 60 min measurement window, ~5 min of boot, corpus load and mapping. Two corrections fall out of that number. The overtime kill ratio should never have been raised. The reasoning was that trace replay has a wider runtime spread than a fixed synthetic shape, but a DURATION-based replay does the opposite: the measurement window is fixed, so only warmup scales with how slow a config is. A variant with 3x slower warmup lands at ~1.8x the baseline total, comfortably inside the stock 2.0x guard. What actually needed adjusting is the per-variant hard cap. Its ceiling assumes a synthetic round measured in minutes, so at AgentX baselines it clamps the hard cap BELOW the soft kill and inverts the layering _compute_explore_variant_timeout documents -- the generic timeout fires first and the round is recorded as a plain timeout instead of KILLED_OVERTIME with its diagnostic ratio. At the measured baseline the stock ceiling leaves 240 min of hard cap against a 222 min soft kill (barely intact); at a 2 h baseline the two are equal and beyond that it is inverted. Raising the ceiling under AgentX restores the ordering. Verified on the E4 run: 393/393 traces loaded with rejected_by_maxctx 0 and --max-context-length absent, metadata.submission_valid true, and both result shapes emitted (Magpie's and the leaderboard aggregate). Co-Authored-By: Claude <noreply@anthropic.com>
The AgentX ceiling branch called agentx_enabled() without importing it, so every explore round raised NameError. The executor wraps task bodies, which turned the crash into a plain state="failed" with no traceback -- five explore tests went red with `assert 'failed' == 'succeeded'` and nothing pointing at the cause. Only caught by diffing the full suite against the base commit: the AgentX-focused tests all passed, because they exercise the helpers directly rather than through a task. Co-Authored-By: Claude <noreply@anthropic.com>
Nine cases including the one that matters most: a longer flag sharing the same prefix (--max-model-len-extra) must survive untouched. Getting that wrong would silently delete an unrelated tuning knob from every variant. Co-Authored-By: Claude <noreply@anthropic.com>
The coverage gate failed at 89.98% against fail-under=90.00. The two units
this PR adds to carry a GEAK kernel from an acceptance to a named row were
the largest uncovered blocks it introduced:
* _geak_accepted_kernel_specs -- the whole selection loop (lanes, env
exclusion, non-positive deltas, alias-twin collapse) was reached only
through its early returns.
* KernelPhase._record_geak_adopted_kernels -- the per-kernel ledger writer,
entirely uncovered. by_kernel, kernel_lifecycle.adopted and the
attribution split all read what it writes.
Writing the ledger tests surfaced a real defect: best_gain_pct was set to the
current rebench gain, while the canonical writer in _kernel_decisions.py
computes it as a max over attempts. Both feed the same downstream field, so a
second, worse rebench lowered the kernel's recorded best. Now a max, keeping
None (rather than that writer's 0.0 default) for the unattributable case --
"not attributable" is not the same claim as "no gain".
Measured by combining this run's four CI coverage shards with the new tests:
89.98% -> 90.04% (7485 -> 7438 missing lines).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four issues, found by reviewing the completed diff rather than by writing it. The InferenceX pin exists in TWO places and only one was moved. install.sh went to 3d558156 while cli/preflight.py kept a4bb43af, and they are independent clone paths -- install.sh's ensure_inferencex, and _clone_inferencex when a session starts without $INFERENCEX_PATH. The synthetic path sources benchmark_lib.sh from whichever checkout wins, so the same Hyperloom commit could run two different measurement harnesses with nothing in the logs saying which. Not AgentX-scoped, and the more dangerous half: it pairs the new aiperf with the old InferenceX, exactly the mismatch install.sh warns about. That drift also blinded the guard built to catch it. The anchor contract asserts the recorded ref equals _INFERENCEX_REF_DEFAULT, so with both sides left at a4bb43af it passed vacuously -- and the "zero regression across 10000+ tests" evidence said nothing about the new pin. Re-verified all four patch anchors against 3d558156 (each still matches exactly once) and refreshed the record; only benchmark_lib.sh's digest moved. --trace-idle-gap-cap-seconds was missing. It is not scenario-locked -- upstream passes it explicitly alongside the scenario, defaulting to 300 -- and it caps per-trajectory idle, distinct from the scenario's 10s whole-system cap. Without it a trace carrying a long recorded idle gap replays that gap in full, and since --benchmark-duration is a fixed window the lost time comes straight out of measured requests: throughput would land systematically below the published row for the same server config, defeating the point of the change. Also: the workload guard stripped AIPERF_BIN from extra_envs but not from unset_envs, and the leaderboard aggregate now records which InferenceX checkout produced it (an operator-pinned $INFERENCEX_PATH is supported, so the aggregator is not necessarily the pinned one). Co-Authored-By: Claude <noreply@anthropic.com>
…e-docs Updates to docs to prepare for release
github-code-quality[bot] and github-advanced-security[bot] (CodeQL) both flagged the bare `except OSError: pass` around the scratch temp-file unlink in _steal_stale_claim() as an empty except with no explanation. The behavior is intentional (the unlink is best-effort cleanup of a file that has, by that point, already been atomically replaced onto claim_path or never fully written -- it must never raise out of a claim-stealing attempt) -- add a comment documenting why, no behavior change. Co-authored-by: Cursor <cursoragent@cursor.com>
…uction Tech-lead review finding on PR AMD-AGI#1191: SharedState._build_hot_kernel_ summaries() projects each hot kernel into an explicit key whitelist (summary_entry) that becomes last_trace_analyze["hot_kernels_top15"]. That whitelist never carried patch_strategy, vendor_playbook_group_id, vendor_playbook_aggregate_gpu_pct, or vendor_playbook_min_gpu_pct_floor. untried_hot_reusable_kernels() reads hot_kernels_top15 in preference to raw hot_kernels, and hot_kernels_top15 is always populated in production, so effective_hot_kernel_gpu_pct()/effective_hot_kernel_min_gpu_pct() silently degraded to bare gpu_pct/min_gpu_pct at that call site -- the only one of the three gate call sites affected (the two in _batch_kernel_candidates read full candidate dicts off candidates_path instead, so they were never broken). This broke both directions of the intended behavior: a split-load vendor-playbook group (e.g. dispatch=7%, combine=5%) clearing a 10% floor together was still dropped as below-threshold on both members, and the playbook's own min_gpu_pct_floor was not enforced either, so a loosened HYPERLOOM_KERNEL_OPT_MIN_GPU_PCT could let a below-floor group burn a whole forge-loop session. Fix: add the four fields to summary_entry. Existing coverage (test_untried_hot_kernels_vendor_playbook_group_gated_on_aggregate/ _floor_still_applies) could not catch this because their _set_trace() helper assigns last_trace_analyze directly and never populates hot_kernels_top15, so they fell through to the unprojected raw hot_kernels list. Added two new regression tests that go through the real record_trace_analyze() entry point instead (one per direction); both fail with a KeyError without this fix and pass with it. Also strengthened the existing aggregate test's weak `assert untried` (which passed even if only one member cleared the gate) to assert the specific kernel id set, and fixed its comment, which incorrectly claimed the two rows share (source_file, name) -- they do not; the differing names (::dispatch vs ::combine) are exactly what keeps them distinct instead of collapsing under the identity-dedup fallback. Verified: the two new tests fail with KeyError without the shared_state.py fix and pass with it. Full agents/kernel/tests/ + inference_optimizer/tests/ suites show the identical 1612 pre-existing, unrelated failures (missing pytest-asyncio plugin) both before and after this change -- no regressions. Co-authored-by: Cursor <cursoragent@cursor.com>
…patch_and_combine feat(kernel): route mori EP dispatch/combine to the KernelForge vendor playbook
Reviewing the change set against what was actually asked for, roughly half the non-test code was mine rather than the requirement's. Two pieces do not earn their keep. The variant workload guard is removed. It defended a path with a measured occurrence rate of zero: nothing in this tree -- no prompt, no lever list, no default grid, no knowledge-base entry -- ever proposes the knobs it stripped, and vllm/sglang have no seed grid at all. The failure it prevented was real but its probability was not, and --failed-request-threshold already catches the same class. Arguing it from "could permanently contaminate RecipeKB" used the severity of the consequence to paper over the missing likelihood. The leaderboard-shaped aggregate is removed. A leaderboard row is identified by 18 fields; the record we produced left 9 of them empty (hw, image, recipe_fingerprint, infmax_model_prefix, precision, disagg, is_multinode, kv_offload_backend, allocated_cpu_dram_gb) and carried a local filesystem path where the model id belongs. Without a row identity there is nothing to compare it against, so the artifact was not usable -- and shipping it invites the conclusion that comparing against the leaderboard is a solved problem when it is not. Emitting it again should follow, not precede, filling in that metadata. What the requirement asked for is untouched: the upstream invocation, the corpus selection, the context fix that makes 393 traces reachable at all, the budgets that let a round survive, and the guards against a session silently measuring something other than what it reports. Co-Authored-By: Claude <noreply@anthropic.com>
Two options this repository sends to KernelForge no longer exist there. `forge-fuse` renamed `--llm-model` to `--model`, and unlike `forge-loop` it is not a tolerant command: an unknown option is a click UsageError, so the child exited 2 before authoring anything and the wrapper reported the outcome as a missing `fusion_manifest.json` — an infrastructure-shaped error for what was really a stale flag. Only the argv string moves; the `llm_model` key in the wrapper's own input JSON is what the orchestrator writes and stays as it is. `--gpu-arch` was renamed alongside it, but this repository never passed it. `--max-iters` was deleted from `forge-loop` and `forge-rewrite-by-flydsl`. Those two are tolerant, so sending it was never fatal — it was reported as an ignored option and dropped. It is gone from all three argv builders here, along with the parameters that carried it and the `FORGE_MAX_ITERS` / `FORGE_COMPILED_MAX_ITERS` environment variables behind them. The compiled/ASM fellow cap those fed could not have worked: KernelForge stopped acting on the value when the supervisor's intervention limit was removed, so the block logged a cap it never applied. Campaign length is `--max-hours` and the hard kill, which is what it already was. `forge_collective` had already stopped sending the flag for the same reason. The argv tests now assert the absence of both flags rather than their values, so neither can return unnoticed. Not changed, having been checked: the fusion manifest reader keys off `fusion_loop.kept`, which is exactly the field KernelForge tightened to require a commit, so it inherits the stricter verdict with no edit; nothing here imports from `kernel_agents` or `forge_llm`, so the workspace guard moving out of the Codex backend and the deletion of `CodexSafetyError` reach nothing; and every other option in all five argv builders is still declared upstream. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… 19th The Unreleased heading was added with a pending item that the 1.0.0b1 cut folded into the release section, leaving a bare heading that has rendered empty on the docs site since. Release notes are read top-down, so the heading only earns its place when it has content; 1.0.0a3 had none either. Co-authored-by: Cursor <cursoragent@cursor.com>
_validated_forge_best_result gated on schema_version == 1. KernelForge has stamped 2 into best_result.json since 2026-08-13, so the gate rejected every published best and the kernel backend fell through to the caller checkpoint or the stdout sentinel — losing the one record that survives a hard kill, which is the case that file exists for. The gate now names the producer's constant instead of a bare literal, and a test pins the two together so the next bump cannot drift unnoticed. The checkpoint gate beside it stays at 1: the checkpoint really does carry that version. Eight tests already covered this salvage path in full; they were passing only because their own fixtures repeated the wrong version. They now publish what the producer publishes, which is why fixing the gate needed no other change: the behaviour was specified and tested all along, just never reachable. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
chore(release): cut 1.0.0b2
SGLang rocm720 images still need the host libamdhip64/libroctracer swap; bring Phase 3 back while keeping SGLang 0.5.17 / vLLM 0.27.1 defaults.
SGLang takes its profiler capture bounds in the /start_profile POST body, not on the serve line. Hyperloom computes them into $PROFILE_EXTRA_BODY but only InferenceX's own client ever posted it, so a bare POST left the capture unbounded and the worker accumulated profiler events in host RAM until the cgroup OOM-killer took it out mid-run -- surfacing as an unexplained server death rather than a profiling bug. The client now forwards the body when there is one. These cases hold that: one asserting the bounds reach curl with a JSON content type, and a parametrized pair asserting an empty or absent body posts bare rather than sending a meaningless payload (vLLM carries its bounds on --profiler-config). The fake aiperf gains an opt-in sleep so the PROFILE branch finds it running; it stays 0 for every other case. Co-Authored-By: Claude <noreply@anthropic.com>
…aremetal-rocm-hotfix Restore bare-metal ROCm profiler hotfix
Review pass over the third merge. * The policy-denied exit goes back to main's exact two lines. Inlining ``denied.rule or "denied"`` into the f-string was a deviation this merge had no reason to make: every production ``raise PolicyDenied`` passes a rule, so the default is main's call to keep or drop, not the merge's. Only the indentation now differs, and that is forced by ada71eb's outer ``try``. * ``test_coordinator_runtime`` stopped naming ``kill_task`` as the reason a row was cancelled. The intent left the vocabulary in 8bd7a94; the test only needs some terminal reason, so it names one that still exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
[Stepping] Containment sweep across the orchestration loop: authority gates, task-row lifecycle, patch revert integrity, and path boundaries
…1.0.0 Update TraceLens commit SHA for v1.0.0
…gpie-nonzero-invalid-measurement fix(custom): restore operator bench scripts on --resume-from
Rebased onto main to resolve merge conflicts. Applies mechanical ruff format (366 files), enables ruff-format in pre-commit, excludes Markdown from ruff format, updates the vendored SDK blob checksum, makes Ruff a hard CI gate, and adds advisory mypy config plus lint job. Co-authored-by: Cursor <cursoragent@cursor.com>
Use the digest of the committed kb_store_client.py blob (d8a790b...) so CI matches the gate on Linux. Co-authored-by: Cursor <cursoragent@cursor.com>
Scan oldest unrouted targeted builds instead of returning when the newest build is already accounted for, and ack failed builds only after rearm. Record framework phase markers through append_phase_history_event so rows use the canonical schema, ts_unix, and history cap. Treat in-phase markers (from_phase == to_phase) as sub-events for duration and breakdown readers.
Keep build_novelty unchanged when rearm raises so a retried novel compile_error still routes as advanced instead of reverted.
…ical chore: apply ruff format repo-wide and enable ruff-format hook
ROCm docs review for style and clarity
Resolve framework.py conflict by keeping ack-after-rearm ordering for artifact-unreadable targeted builds. Co-authored-by: Cursor <cursoragent@cursor.com>
…ct (P2-36, P2-37) (AMD-AGI#1271) * Tighten freeform specialist wave validation and proposal scorer contracts. Reject malformed wave entries at PolicyGate and fan-out, harden scorer input/output handling (stable IDs, duplicate rejection, tail JSON envelope), and align max_turns=0 resolution in SpecialistRunner. * Address PR review: freeform max_turns, fan-out re-gate, scorer JSON helper. Apply shared max_turns validation to freeform and wave subtasks, re-validate each fanned sub-intent through PolicyGate, reuse extract_last_json_with_key, preserve original proposal names in score keys, persist scorer input errors, and update legacy proposal-scorer and fan-out tests. * Fix fan-out wave test and ruff format for CI. Add action_name to test_fan_out_wave_dispatches_valid_task so fan-out re-gating matches production delegate intents; apply ruff format to gate.py and test_common_jsonio.py flagged by the lint job. --------- Co-authored-by: xiaofei-zheng <XiaoFei.Zheng@amd.com>
…k-routing Fix framework build routing starvation and phase_history markers.
…ts (AMD-AGI#1272) * fix(breakdown): make reported counts and evidence match their contracts The pipeline stated four contracts in comments and enforced none of them. Both ends leaked, each in its own way. P2-42 collectors/timeline.py, schema.py `keeps` is documented as "kernels adopted at integrate (NOT micro-only KEEP)" while counting exactly those; it counted invocation rows instead of distinct kernels; and `task_id` never reached the de-dup key because `change` is always set, so two tasks sharing a second collapsed into one row. A lane could report keeps=8 for three kernels, none of which ever passed integrate. Micro-only KEEPs now have their own tally and no longer make a lane read as "kept". P2-43 reporters/cross_section.py, reporters/compose.py A skipped section's key facts and warnings were discarded, making "this never ran" indistinguishable from "this ran clean". They now reach data_quality_flags. Sections nothing produces are held apart from sections with no data this session -- four permanent flags would have drowned the ones that describe the run. geak/forge invocations rendered but had no SECTION_GROUPS entry, so the report quoted their adoption counts while showing none of the attempts behind them. P2-44 reporters/llm_prompt.py Model prose was pasted verbatim beneath a heading the composer owns; a line opening a heading, code fence or block element re-parents every deterministic block that follows. Such lines are stripped, prose past the length the prompt asks for is dropped whole rather than truncated, and both fall back to the deterministic path that already existed. P2-38, P2-39 recorder/recorder.py The module docstring promised cross-process safety that record_upsert_* cannot provide (read-merge-write under an in-process lock, on a network filesystem where advisory locks are unreliable). It now says which methods are safe, and a guard test keeps subprocess packages from importing the recorder. Keyless items drew two sequence numbers, so a filename never matched its own envelope. Stable filenames now carry a digest of the untouched key, because sanitizing folded `a/b`, `a:b` and `a b` onto one file; fragments already written under the old name keep it, so resuming a session updates its fragment instead of forking one. Tests: 24 cases in test_breakdown_report_integrity.py, plus a guard test and a test that the guard can fail. test_forge_lane pinned the micro-only-counts-as-adopted behaviour and now supplies the integrate verdict that makes its KEEP a real adoption. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(breakdown): address review — verdicts, shared identity, output guarantees All eight review points reproduced; none were rejected. The first pass fixed the counts but left four more ways for the same class of bug to survive, and introduced two of its own. Introduced by the first pass: The exporter kept its own timeline identity while the collector's changed, with a docstring still claiming the two matched. Two recorder-only rows one second apart merged into one event. Both sides now share TimelineDedup. The legacy-name fallback re-opened the collision the digest had closed: `a/b` and `a:b` both adopt a legacy `a-b.json`. Reuse is now limited to keys sanitizing left untouched -- the only case where the old name is unambiguous -- and the ambiguous case logs why it wrote a new file instead. The HTML blacklist missed `<!--`, `<?php`, `<!DOCTYPE`, `<![CDATA[`, `<p>`, `<pre>` and `<blockquote>`; an unterminated comment comments out the rest of the report. Enumerating safe HTML was the wrong shape. The rule now matches the act of opening a block and rejects the narrative whole: 50 lines became 10, and ordinary prose containing `<` still passes. The subprocess guard inspected only `node.module`, so `from ...breakdown import recorder` walked past it. Its own self-test had picked three cases that all landed on the branch that worked. A new docstring claimed `attempts` counts distinct kernels. It counts invocation rows, deliberately -- the comment was wrong, not the code. Pre-existing, surfaced by making `keeps` mean something precise: `NEEDS_REVIEW` and an empty decision were counted as adoptions. Neither is a verdict, and a NEEDS_REVIEW with gain <= 0 is never retried, so the report claimed a win for the rest of the session. They now have their own tally. `integ_by_kid[kid] = ...` overwrote. The ledger is keyed `<kernel_id>|<patch_path>|<extra_args>`, so one kernel holds several rows and whichever sorted last decided the outcome -- a REVERT could erase a KEEP. Verdicts fold now, the more decided one winning. data_quality_flags rendered only inside the deterministic summary, which the LLM summary replaces wholesale, and Key Facts explicitly declines to repeat them. A model ignoring one line of the prompt erased the skipped-section evidence the previous commit had just rescued. Rendered deterministically on both paths now. `micro_only_keeps`, `reverts` and `e2e_gain_pct` had no renderer, so a reader could not tell a failed lane from one whose wins are unvalidated. Deliberately not fixed: an integrate verdict still joins on kernel_id alone, so two lanes that optimised the same kernel in different runs can both claim it. The suggested patch/integration identity has no common key today -- integrate rows carry no backend, invocation rows no patch_path -- so it needs a bridging field first, and that is its own change. Tests: 27 new, including the collector/exporter agreement the split had broken. Remaining failures on this branch match main exactly (Windows-only APIs and path separators). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: zengleixin-amd <lzeng@amd.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ilure modes
Ray's prestarted worker pool (--num-cpus workers) plus a TP>1 framework
server (one heavy process per GPU, each with its own OpenBLAS thread pool)
reliably exceeds Docker/Podman's default pids.max cgroup limit (2048)
before the server finishes booting. Depending on which thread loses the
race for a PID slot, this surfaces either as a loud OpenBLAS
`pthread_create failed: Resource temporarily unavailable` crash in the
server, or as complete silence: the orchestrator logs the benchmark launch
and then never produces a completion, failure, or exception anywhere,
because its own subprocess-management thread lost the race instead.
Verified end-to-end on 8x AMD MI325X (gfx942) running moonshotai/Kimi-K3:
recreating the container with --pids-limit -1 immediately fixed the
silent-hang case (real Magpie/vllm processes appeared, Ray showed the
GPUs in use, checkpoint loaded normally).
Also documents two related failure modes hit in the same session:
- A stale enablement.inflight_task_id in state.json survives --resume
even with --enablement off, since off only prevents new authoring
rounds rather than reaping an already in-flight one.
- baseline_accuracy_failed / Explore never proposing anything, for models
without an accuracy-eval task mapping yet (--no-eval is the fix, not a
bug workaround) versus a genuinely idle orchestrator under a very small
workload + reduced flag set.
Changes:
- examples/hyperloom-custom-advanced/SKILL.md
- examples/hyperloom-qwen3-8b-3h/SKILL.md
- examples/hyperloom-qwen3-14b-fp8-12h/SKILL.md
Add --pids-limit "${HYPERLOOM_PIDS_LIMIT:--1}" to the docker run
template, plus an explanatory note.
- docs/reference/troubleshooting.md
New entries for all three failure modes above, matching the existing
sibling entry for the analogous Ray fd-limit problem.
xiaofei-zheng
requested review from
Ahmedhasssan-aig,
devalshahamd and
tsrikris
as code owners
August 25, 2026 10:22
Collaborator
|
@skariapaul Can you pls confirm why so many changes are required across files? @xiaofei-zheng there are a significant number of changes on TraceLens, etc. which I'm not sure how to sign off on, it should require a much shorter fix and not involve changing 60% of the files |
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
20 participants
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
Ray's prestarted worker pool (
--num-cpusworkers) plus a TP>1 framework server (one heavy process per GPU, each with its own OpenBLAS thread pool) reliably exceeds Docker/Podman's defaultpids.maxcgroup limit (2048) before the server finishes booting in docker-mode. Depending on which thread loses the race for a PID slot, this surfaces either as a loud OpenBLASpthread_create failed: Resource temporarily unavailablecrash in the server, or as complete silence: the orchestrator logs the benchmark launch (baseline_executor: launching Magpie cmd=[...]) and then never produces a completion, failure, or exception anywhere — because its own subprocess-management thread lost the race for a PID slot instead of the server.Verified end-to-end running
moonshotai/Kimi-K3(TP=8) on 8x AMD Instinct MI325X (gfx942) via Podman:cat /sys/fs/cgroup/pids.current /sys/fs/cgroup/pids.maxinside the container:pids.currentwas already at 1659/2048 from Ray alone before vLLM even tried to start.OpenBLAS blas_thread_init: pthread_create failed ... Resource temporarily unavailable.--pids-limit -1fixed it immediately: realMagpie/vllm serveprocesses appeared,ray statusshowed8.0/8.0 GPUin use, and the checkpoint loaded normally.Changes
examples/hyperloom-custom-advanced/SKILL.md,examples/hyperloom-qwen3-8b-3h/SKILL.md,examples/hyperloom-qwen3-14b-fp8-12h/SKILL.md: add--pids-limit "${HYPERLOOM_PIDS_LIMIT:--1}"to the docker-run template, plus an explanatory note (why it's required, not cosmetic — mirrors the existing sibling note for the fd-limit/Ray raylet issue).docs/reference/troubleshooting.md: three new entries, matching the style of the existing sibling entry for the analogous Ray fd-limit problem:pids.current/pids.maxdiagnostic command).enablement.inflight_task_idinstate.jsonsurvives--resumeeven with--enablement off(which only prevents new authoring rounds, not reaping an already in-flight one) — includes a snippet to clear it directly.baseline_accuracy_failed/ Explore never proposing anything — for models without an accuracy-eval task mapping yet,--no-evalis the correct fix (not a workaround for a bug), documented alongside how to distinguish that from a genuinely idle orchestrator under a very small workload + reduced flag set.Test plan
pids-limit) with Ray (128 prestarted workers) + vLLM TP=8.pids.current/pids.maxinspection and a synchronous out-of-band repro of the exact subprocess call.--pids-limit -1and confirmed the benchmark subprocess launches and progresses normally (checkpoint loads, GPUs show 100% utilization, real throughput numbers recorded).