Repository navigation
[fix] close the defects behind the MuseSpark run shortfall and delete state nothing reads - #2055
Conversation
Code metrics gate: PASSEDJudged: the 155 files this change touches. Files it does not touch never fail it; anything found there is listed under Outside this change for information.
Module length warning: over 800 lines, within the 1200-line failure limit (not failing): 8
Already over at the base (backlog, not this change's): 31
...and 6 more; the job log lists every row. 58 findings in files this change does not touchOutside this change (informational): 58
...and 33 more; the job log lists every row.
Thresholds and sources: |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
ZhengGong-amd
changed the title
|
size-exception |
ZhengGong-amd
added
the
size-exception
Review of #2055What it does: traces a 660-minute MuseSpark-1.2 run that finished at +5.99% against a 10% target back to five areas of the code — what a KEEP is judged against, how a phase decides it is done, what lease expiry claims to do, whether a dead lever can be seen as dead, how the kernel arm is credited — fixes the defects that have a fix in the existing infrastructure, and deletes the mechanisms verified to have no reader, no effect, or no reachable branch. Net −3.5k lines across 160 files, 42 commits plus one Blocking issues: 3 1.
So any stream shaped
Trigger in a real config: TP=8 vLLM where one rank hits At the merge base ( 2. Documented result fields renamed without the major
In-tree consumers were all migrated and 3. The line reads "The phase exits on Checked: bring-up ladder ( Ran: static verification and targeted greps in a worktree at CI at this head: Base: |
8fdb867 to
9bea6d1
Compare
xiaofei-zheng
left a comment
xiaofei-zheng
left a comment
There was a problem hiding this comment.
Review of #2055
What it does. A 660-minute MuseSpark run finished at +5.99% against a 10% target; tracing it back through the code found defects in five areas (what a KEEP is judged against, how a phase decides it is done, what lease expiry claims to do, whether a dead lever can be seen as dead, how the kernel arm is credited). The PR fixes those in place — the KEEP bar becomes a floor an agent may only raise, SWEEP closes on the rule that actually refused the next macro-cycle, prune_branch expands a catalogue family into its member actions, an empty or undispatchable authoring round now charges an attempt row to its lever, a GEAK config that restates current_best settles before it is benchmarked, and the hot-kernel "tried" test reads from queued integrations, integrate verdicts, the optimization stack and the rewrite controller instead of a ledger nothing wrote — and deletes the mechanisms verified to have no function (the per-kernel attempt ledger, assess_convergence, the lease_expired topic, gpu_pool.extend, the per-lease heartbeat, the rocprof enrichment hook, the 1000 macro-cycle ceiling, four env vars). kernel_optimization_summary.json goes to schema_version 3, rebuilt from the timeline's V6KernelAttempt records.
Blocking issues: 2
1. A documented result field of session_breakdown.json is renamed without the major schema_version bump the contract requires.
outcome.baseline.throughput_tok_s_per_gpu and outcome.final.throughput_tok_s_per_gpu become output_throughput (breakdown/recorder/baseline_event.py:224, breakdown/collectors/v6.py:367,667,689, both renderers), while inference_optimizer/session/sbd_v6.py:22 stays hyperloom.session_breakdown.v6.0 and exporter_version is untouched.
docs/reference/session-breakdown.md states the rule twice, and this PR edits those lines without changing it: compatibility rules — "Renamed, removed, or semantically changed fields of a result section require a major bump"; stability guarantee #1 — "Never removing or renaming a documented field of a result section within a major schema_version ... every removed or renamed field is given a disposition in the section it left" (only enablement is carved out). outcome is a result section, the field was documented, and the same page named it the recommended index key ((session.session_id, final.throughput_tok_s_per_gpu, ...)) — that line is rewritten, the old key is deleted, and no disposition entry replaces it.
A consumer pinned on major version 6, as the page instructs, therefore reads null for the headline number with no version change to detect the cutover by. There is no alias: at head throughput_tok_s_per_gpu appears nowhere outside the unrelated conc-sweep input_throughput_tok_s_per_gpu. The in-repo skew case is real as well — a baseline timeline event written by a pre-rename build and exported by this build yields outcome.baseline.output_throughput = null.
On the open question the description raises: src/hyperloom_kb/mappings/hyperloom-sbd-v6.yaml reads $attempt.measurement.after_tput and $doc.metadata.grading.objective, neither of which this rename touches, so a v7 cutover costs the mapping's file name, not its body. Either cut over to v7 with the disposition entry, or keep emitting the old key for the remainder of v6.
2. Bring-up classification lost its read bound and runs on the event loop.
read_bringup_log (orchestrator/actions/executors/baseline.py:273) now does path.read_bytes() on the whole server.log. At 931af8ab the same function took 64 KiB from each edge and streamed the middle for milestone lines only, so at most ~128 KiB plus the milestones ever reached the classifier. Nothing between the read and the classifier caps it.
The text is then walked repeatedly. server_child_elapsed_sec (baseline.py:304) constructs a datetime per timestamped line over the whole file, and _LOG_LINE.finditer runs over the whole file three times — failure_region → _root_traceback (bringup/ladder.py:140) inside classify, again at ladder.py:349, and a third time in observe_bringup's closing classify_failure(failure_region(evidence)) (bringup/observe.py:80). Each failure_region also materialises a full lowered copy of the text (ladder.py:197) and scans it once per progress marker. There is no early exit.
The caller is a synchronous closure: record_bringup is defined inside async def _run_single_benchmark (baseline.py:3387) and called directly at baseline.py:3497 and :3513, while the benchmark subprocess itself is correctly offloaded with asyncio.to_thread (baseline.py:3473). integrate_patch.py:4725 pays the same cost once per bench variant. So the coordinator loop — heartbeat, lease renewal, every other in-flight lane — blocks for a pass linear in log size, once per baseline round and once per bench variant, where it was constant before.
The repo states the size this has to survive: baseline.py:149 — "Bounded per-file read so log scanning never slurps a multi-GB server.log", _LOG_SCAN_MAX_BYTES = 262_144, applied to server.log among others (baseline.py:1576); enablement/artifacts.py:38 — "Server logs routinely exceed _FILE_SIZE_LIMIT" (2 MiB) — and caps its copy at a 1 MiB tail. I could not run a production-sized log in this environment, so no measured number is quoted; the unbounded read, the three full passes and the loop placement are read off the code. What the commit wanted — a milestone or a root traceback that can lie anywhere — needs a deeper read, not an unbounded one on the event loop.
Checked: orchestrator/kernel/{attempt_summary,_kernel_decisions,geak_config,geak_runner,request_handlers}.py; phases/{framework,machine,machine_state,kernel,kernel_stack,geak_adoption}.py; state/{shared_state,_shared_state/*,failure_evidence}.py; loop/{coordinator,writeback,intent_router,proposals,dispatcher}.py; bus/*; specialists/*; common/agent_cli.py; breakdown/**; actions/executors/{baseline,report,roofline}.py; trace_analysis/**; bringup/**; cli/preflight.py; knowledge/**; docs, prompts, SKILL.md.
Verified by whole-tree grep that every removed name is swept (kernel_opt_task_attempts, last_kernel_opt, roofline_measured, rocprof_roofline, rocprof_enrich, no_kernel_skipped, predicate_inputs.phase_budget_pct, sweep_reloop.max_cycles, assess_convergence, and the four env vars) with zero surviving references in code, docs, prompts or examples; that global_converged / time_exhausted are in STOP_REASON_VOCAB, the v3 contract, the schema enum and the renderer map, and in SUCCESS_STOP_REASONS so the exit code is unchanged; that recovered is accepted by profile_trace_usable() and _promote_roofline with no surviving == "succeeded" reader; that floor_keep_threshold cannot emit NaN or inf because to_float rejects them; that common/agent_cli.py is equivalent to the two resolvers it replaces and that .importlinter removes an ignore rather than adding one; that the deleted lease machinery had no publisher or reader at the merge base; and that the v3 workflow_contract digest recomputes to the const it pins.
Also checked and clean: the "tried" reclassification cannot retire a sibling kernel (record_matches_task requires task-group or kernel-id agreement before it looks at source_file) and both rejection writers leave a record the new definition sees; the removed macro-cycle ceiling leaves SWEEP bounded by the no-gain streak, the session deadline and max_ticks; the new attempt row is written only on a newly stamped terminal row, so it cannot double-count against the plateau rule.
Considered and not reported: a family-scope prune_branch cancels a queued enablement-revalidation baseline without the exclusion drain_queued_baselines applies — pre-existing at 931af8ab via prune_branch{family: "baseline"}, which cancel_family matched by task kind there too, so this diff does not introduce it.
Ran: no test execution — this box is Windows and the suite's conftest is POSIX-only (fcntl); CI is green at head on all 12 test shards, coverage, CodeQL, ruff, pylint, mypy, import-linter and bandit. code-metrics is red at head on exactly the three module-length entries the description tables (framework 3173, preflight 2678, coordinator 1440); the baseline-raise label it asks for is not applied, so that check stays red until it is.
Base: 931af8abed2807d204c2e6db04ce0d416febe5c6 — Head: 9bea6d1e5c96fdfd0b74f41aaf48e6d6fef03970
9bea6d1 to
be99714
Compare
xiaofei-zheng
left a comment
xiaofei-zheng
left a comment
There was a problem hiding this comment.
Re-review at f2744e38
Both findings from the previous round are resolved.
1. The session_breakdown rename is no longer this PR's. #2023 landed the
throughput_tok_s_per_gpu → output_throughput rename on main with its own version decision
(hyperloom.session_breakdown.v6.1); this branch drops its copy, no longer touches sbd_v6.py,
and the description now says so and declares 6.1 in the workflow contract. I checked the body's
replacement claim: roofline_measured, rocprof_roofline and convergence appear nowhere under
docs/ at the merge base, so "none of the three is documented in the reference" holds, and the
contract digest is pinned by test_workflow_evaluation_contract.py:122, which is green at head.
2. The bring-up classification no longer runs on the event loop.
fix(bringup): classify a boot off the event loop wraps both record_bringup call sites
(baseline.py:3498, :3514) and _record_bench_bringup (integrate_patch.py:4631) in
asyncio.to_thread, so the pass sits beside the benchmark subprocess it follows rather than in
front of the heartbeat and the lease renewals. read_bringup_log's docstring now states the
linearity and the obligation.
Blocking issues: 1
- The bring-up failure-classification change is an operator-visible effect the description does
not state.failure_region/_region_start(bringup/ladder.py:194-213) do not exist at the
merge base: this PR introduces scoping of the failure rules to a region, and
fix(bringup): start the failure region at the first line of the furthest milestonethen changes
where that region begins. The outcome an operator reads changes with it —matched_marker,
env_fault,failure_textand therefore whether a failed boot stops the run as an environment
fault. The PR's own tests show the discrimination: on
test_a_rule_match_the_process_ran_past_does_not_name_the_failure, an autotuner's "out of
resource: shared memory" line that precedes a later milestone no longer names the failure, where
on main it is what the rules match first; on
test_a_milestone_another_rank_repeats_after_the_failing_line_keeps_it_in_the_region, a rank
repeating a milestone after another rank's fatal line no longer costs the observation its
env_fault.
The description's## Observable effectsection is otherwise exhaustive — every removed field,
artifact column and env var is there — but carries nothing about bring-up, and the only mention
of this work is the line in## Descriptionnaming the commit
(fix(bringup): read the server log whole …) without saying what changes for someone who will
never read the diff.AGENTS.mdEvery change states its observable effect in its own PR
description makes that statement the PR's job, because the release cut aggregates these
descriptions and cannot reconstruct them from commit subjects.
Action: add a bullet under## Observable effectfor what a failed boot now reports, and for the
classification moving to a worker thread.
Checked this round: git log and full diff from the new merge base e25cb2db2 — the five commits
added since 9bea6d1e5 (fix(bringup): start the failure region …,
fix(bringup): classify a boot off the event loop, docs(sweep), test(geak), chore(metrics))
plus the rebase onto the new main. LadderStage is an ordered enum, so the new
max(stage) / min(at) selection in _region_start picks the furthest stage and its first line as
the docstring says. The test(geak) and docs(sweep) commits are test- and docs-only; the
chore(metrics) commit only lowers baseline entries. Re-verified the reconciliation section against
main: #2023, #2024/#2052/#2056/#2057, #2040, #2041, #2049 and #2050 are each named with what was
kept from which side.
CI at head: everything green except code-metrics, which fails on the same three module-length
entries the description tables (preflight.py 2678, coordinator.py 1440, framework.py 3173).
The baseline-raise label the description asks for is still not applied, so that check stays red
until it is.
Base: e25cb2db2a572f3af70103f2b91aa3a0fa683d4c — Head: f2744e38105789e8704a45ec96b35cff08955cc5
Re-check at
|
…o the failure read_bringup_log kept 64 KiB from each end of server.log and carried at most 200 milestone lines out of the middle. On real logs that lost both halves of what the ladder needs: - A shard progress bar repeats "Loading safetensors checkpoint" hundreds of times, so the 200-line cap filled before "Application startup complete" and served GLM-5.2 boots were classified as never reaching HTTP_READY. - A worker's root traceback megabytes above the tail never reached the ladder, so the failure was keyed on the engine's EngineDeadError re-raise instead. The log is now read whole; the edge window, the middle milestone scan, the edge_bytes parameter and PROGRESS_MARKER_SUBSTRINGS are gone. Reading everything exposed the failure rule table, which takes the highest-precedence match anywhere in the text, to lines the process ran past (Triton autotune rejecting a config read as kernel_resource_limit). The rules now read failure_region(): from the last milestone line before the root traceback to the end. A later milestone proves the process got past a line; the lines between that milestone and the traceback are kept because native code prints the cause there (RCCL "HIP failure: 'out of memory'"). The observation and the verdict's signature use the same region. Co-authored-by: Cursor <cursoragent@cursor.com>
The baseline action in the session breakdown no longer carries a `convergence` block. It was built from the warm-up and the measured round alone, and with one round discarded as warm-up the verdict was always `insufficient_rounds`, so the block never said anything a reader could act on and no decision branched on it. Building it ran inside a catch-all that would also have swallowed a failure of the cold-start check sharing that block. The `baseline_cold_start_delta_high` warning and its log line are unchanged: a measured round more than 25% above its warm-up still flags the anchor as possibly low. Co-Authored-By: Claude <noreply@anthropic.com>
…constants The stalled-domain force dispatch read an enable switch and two thresholds off SharedState attributes that do not exist, so the feature was always on and the thresholds were always the module constants (8 specialist rounds, 12 rounds without a KEEP). The reads are gone and the constants are used directly; the docstring now says which counter a forced dispatch resets (the dispatch counter, not the keep counter). No behaviour change; the dispatch log line no longer repeats the fixed thresholds. Co-Authored-By: Claude <noreply@anthropic.com>
…ng source_patch A patch specialist with no git source tree to write into was refused by a deterministic preflight, and the first refusal also wrote "source_patch" into pruned_families. That entry was only a cached copy of the preflight: every reader recomputed it anyway, except that once written it stayed for the rest of the session, so a source tree that became available later never unblocked patch specialists. Both the Orchestration delegate path and the stalled-domain force dispatch now ask one SpecialistDispatchCollaborator method, which runs the preflight and records the first refusal in last_action_failures as before. The custom specialist falls back to research mode on the preflight error itself. Observable effect: "source_patch" no longer appears in pruned_families, so the state render shown to Orchestration, the final report's pruned_families line and the console session summary no longer list it; the refusal reason (no_git_framework_source_root) stays in last_action_failures, and the custom specialist's research-only observation now names that reason instead of "source_patch_pruned". Co-Authored-By: Claude <noreply@anthropic.com>
When no Critic verdict arrived within the review TTL, the reconciler wrote a reject verdict and dropped the in-memory pending proposal, but never passed through the verdict handler. The event log said rejected while the session breakdown left the proposal unsettled, so a timed-out proposal read as still pending in the framework timeline. The timeout deny now settles the proposal as dropped (reason review_timeout) through the same router method a Critic reject uses. The reconcile report's denied_reviews list, which fed only its own summary log line, is removed; each timeout deny still logs its own warning. Co-Authored-By: Claude <noreply@anthropic.com>
Preflight warned whenever node, claude or codex was missing from PATH and said the Claude/Codex backends "may fall back to direct HTTP" and that install.sh would bring all three in. No such fallback exists: specialists fail to start without their CLI. Nothing runs node, a codex runtime bundled with the Codex SDK satisfied the backend while still drawing the warning, and install.sh installs only the claude CLI. Preflight now resolves the CLI of the agent backend this deployment selects, the same way the specialist launcher does, and warns only when it is missing, naming where it looked and how to install it. It remains a warning. The Claude and Codex CLI resolvers move to hyperloom.common.agent_cli so the CLI layer, the Claude role and the specialist launcher share one resolver without the CLI importing the orchestrator. Co-Authored-By: Claude <noreply@anthropic.com>
HYPERLOOM_SPECIALIST_PERMISSION_MODE let an operator change the --permission-mode passed to Claude specialist subprocesses. Nothing sets it, and under --print any value but bypassPermissions stops the specialist from writing its Bash heartbeat, so it is reaped as stale and retried until the attempt budget is spent -- the variable could only break specialists. The variable is removed along with its documentation, and the specialist subprocess config no longer carries a permission mode: Claude specialists always run with --permission-mode bypassPermissions, which is what every run already used. Setting the variable now has no effect. Co-Authored-By: Claude <noreply@anthropic.com>
…ings Claude specialist subprocesses were started without --setting-sources while inheriting HOME and CLAUDE_CONFIG_DIR, so the CLI loaded the operator's ~/.claude/settings.json (and any .claude/ settings in the checkout it ran in): their hooks, permission rules, env overrides and model choices silently changed how an unattended specialist behaved from one host to the next. Specialists now run with --setting-sources= (no user, project or local settings), the same isolation the one-shot Claude calls already use. Credentials still reach them through the forwarded environment and the CLI's own config; an operator who relied on settings.json to configure specialists will find those settings no longer apply. Co-Authored-By: Claude <noreply@anthropic.com>
…launch _geak_result_has_material still took a prior best (flags, envs, controls) to compare GEAK's accepted config against, but no production caller passes one any more: whether a config changes current_best is decided before the rebench by comparing the launch it produces. The parameters and their fingerprint comparison are gone; the helper now answers only whether a result carries an artifact or a non-empty launch config. The comparisons those parameters pinned are now tested against the launch comparison itself: untrusted environment keys on either side, complete (replace) configs, delta configs over a complete current_best, unset environment names, and removal lists. Removal lists now count toward that launch identity, so a GEAK config whose only change is a removal the composed flags cannot show (an inherited recipe flag, or clearing an inherited removal with an explicit empty list) is benchmarked instead of being settled as no_material. Co-Authored-By: Claude <noreply@anthropic.com>
KERNEL waited for the GPUs GEAK handed back before every call into the rebench, including the ones that end without a measurement: a GEAK config that restates current_best settles as geak_no_material, and an invalid config or an invalid native source is recorded and dropped. Each of those still reaped this session's leftover servers and waited up to the GPU-free timeout for nothing. The rebench now takes the handed-off cards and drains them only once it knows a measurement follows (the explore rebench, the GEAK-harness replay or the native-source revalidation). A rebench recovered from a result.json on resume now drains the same cards as a fresh one; it did not wait at all before. Co-Authored-By: Claude <noreply@anthropic.com>
The stalled-domain dispatch no longer prunes source_patch or saves state when the patch preflight refuses a specialist; it records the refusal once in last_action_failures, which test_specialist_lifecycle already pins. The force_prune save site therefore exercised a save that no longer happens. Co-Authored-By: Claude <noreply@anthropic.com>
…est milestone The failure rules read a stream from its last milestone line before the root traceback. Milestone markers are substrings that repeat: every TP rank prints its own KV-cache and weight-loading lines, and a serving vLLM prints "GPU KV cache usage" every few seconds. When one rank's fatal line (a HIP out-of-memory abort, an NCCL error) was followed by another rank's repeat of the same milestone, the region started after the fatal line, no rule matched, and the observation lost its failure kind and env_fault, so a host fault no longer stopped the run as environment_fault. The region now starts at the first line of the furthest milestone the stream reached before the root traceback. Repeats of a stage already reached no longer move it, and a rule match before a later milestone (an autotuner's rejected config before startup completes) still stays out of it. Co-Authored-By: Claude <noreply@anthropic.com>
Reading a server log whole and classifying it is linear in the log's size, and it ran on the coordinator's event loop after every baseline benchmark and every integrate_patch bench, stalling heartbeats, lease renewals and every other lane for as long as the pass took on a large log. The baseline benchmark's boot observation and the integrate bench's now run in a worker thread, as the benchmark subprocess around them already does. Co-Authored-By: Claude <noreply@anthropic.com>
…ng rule The SWEEP section still said the phase exits on sweep_done. With FRAMEWORK_AGENT enabled, a finished ladder now either loops back or closes on the rule that refused the next macro-cycle, as the paragraph below it states; only a failed ladder closes as sweep_failed. Documentation only. Co-Authored-By: Claude <noreply@anthropic.com>
The drain test's cleanup killed the escaped descendant inside a try/except ProcessLookupError: pass, which code scanning flags as an empty except. contextlib.suppress states the same thing: the child may already be gone. Test-only change. Co-Authored-By: Claude <noreply@anthropic.com>
…up units Co-Authored-By: Claude <noreply@anthropic.com>
…ledger An authoring round that delivered nothing wrote its attempt row through a twenty-line constructor call spelling out "not adopted, no gain, no throughput". AttemptRow.undelivered_round now names that row next to the row shape, so the FRAMEWORK writer is one line, and the dispatch-failure recorder's signature fits on one line. framework.py is back under its length on main. No change in what is recorded. Co-Authored-By: Claude <noreply@anthropic.com>
…comment The reconciler's proposal links are callables because the resume replay rebuilds the pending proposals after the reconciler is built; one comment now covers both, keeping coordinator.py at its length on main. Comment only. Co-Authored-By: Claude <noreply@anthropic.com>
The agent-CLI check picked the resolver by backend and then spelled out a backend-specific message. It now selects the resolver once and prints one warning naming the backend: "no claude CLI" or "no codex CLI", that specialists cannot start without it, and that assets/install.sh installs it. preflight.py is back under its length on main. Co-Authored-By: Claude <noreply@anthropic.com>
…nstant 6.1 states every key it removes with its disposition. The roofline per-kernel rows' rocprof_roofline was never populated and joins that list; the rows' roofline_measured (always false) and the baseline action's convergence (always insufficient_rounds) are listed with the disposition "removed in 6.1, constant". The session-breakdown reference, the upgrade notes and the breakdown skill now name them. Documentation only. Co-Authored-By: Claude <noreply@anthropic.com>
…nch shrank Co-Authored-By: Claude <noreply@anthropic.com>
5495220 to
de191c3
Compare
Re-review at
|
xiaofei-zheng
left a comment
xiaofei-zheng
left a comment
There was a problem hiding this comment.
Re-reviewed at de191c3 — no blocking issues; see the detailed comment above. Every point from the earlier rounds is closed: the output_throughput rename moved to main's declared schema 6.1 exception and this branch's three removals now carry dispositions in the reference, the upgrade notes and the breakdown skill; boot classification runs off the event loop at all three call sites; the description states the bring-up effect; the rebase onto b772e94 keeps #2046's geak_spec_name and #1998's named stages with this branch's changes re-applied inside them; and code-metrics is green with the three module lengths matching the description exactly. All 33 reporting checks pass.
Approving so the earlier change requests no longer block. A code-owner review is still required by the release ruleset.
…kernel-3 Resolve the kernel attempt-summary conflict toward main: #2055 rewrote build_kernel_optimization_summary as a projection of the KERNEL timeline events and deleted the per-kernel attempt ledger this branch's restructuring had reorganised, so the branch's helpers for that ledger have nothing left to restructure. The file is main's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…kernel-2 #2055 deleted the per-kernel attempt ledger (kernel_opt_task_attempts, last_kernel_opt) and rewrote untried_hot_reusable_kernels, _queue_kernel_keep and the identity resolvers around it, so the branch's decomposition of those reads has nothing left to decompose. Main's behaviour is taken whole there, and the branch's structure is re-applied by hand on what survives: - enqueue_nominated_patch keeps its single fusion lane (no `lane` parameter, no forge_rewrite branch) over main's two-lane body. - _existing_integration_id, _pending_record_settled, _bind_result_to_pending, _attempt_totals, _record_integration_keep and _reject_integration are carried across, each minus the ledger reads #2055 removed. - untried_hot_reusable_kernels and its helpers are main's, unchanged. HL_VERIFIED_MICRO_PROMOTE_THRESHOLD and HL_PROMOTE_VERIFIED_MICRO_NEEDS_REVIEW are gone with _queue_kernel_keep; nothing reads them on main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…knowledge #2055 deletes state nothing reads, and this branch had relocated one of the readers: _build_kernel_optimizations_from_state became the module-level kept_kernel_optimizations with _index_integrate_attempts, _kernel_optimization_row and _last_rebenched_throughput beside it. A textual merge would have resurrected the whole join over the deleted kernel_opt_task_attempts ledger. Main's deletion wins and is re-applied to the branch's shape: those four units, the kernel_optimizations attrs key and the extras merge that carried it into the KB row are gone, and the config arm's attempt now copies the attribution explore stored instead of re-deriving it (_attempt_failure no longer takes the outcome). The two conflicted tests take main's side: it replaced the recipe-attrs kernel_optimizations assertions with one that pins an earlier session's rows being carried forward, and deleted the e2e_decision tests of the retired join. The branch's move from _build_recipe_attrs_from_state to recipe_attrs_from_state is carried across in the surviving cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…loop #2055's only change inside the conflicted region is comment text, and the comment sits in code this branch had moved: the GPU-lease TTL note now lives in _admit_gpu_specialist, not in the dispatch loop main edited. The branch's extraction is kept and main's wording is re-applied there, so the retired "iron law" phrasing does not survive the move. gpu_lease_ttl_sec's rewritten docstring, the unwind-window note and the conc_sweep skip comment merged into place unchanged. scripts/code_metrics_baseline.txt is resolved by the ratchet and dispatcher.py's module-lines entry lowered to the merged file's 1454. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…state Two of main's changes landed in code this branch had moved, so the merge is resolved by hand on the branch's shape: - #2060 replaced profile_workload_context's shlex round-trip with normalize_server_args, but the branch had lifted that computation above the context literal. The import merged and the call did not; the branch's joined_args now goes through normalize_server_args, so a JSON server-arg value survives the profile context, and `import shlex` is gone. - #2055 deleted the rocprof roofline enrichment, whose merge the branch had moved into the module-level _hot_kernel_row. _hot_kernel_row loses its rocprof_by_kernel_id argument and its rocprof_roofline field, _ROOFLINE_SIGNAL_KEYS loses that key, and _build_hot_kernel_summaries takes only the result, as on main. last_kernel_opt, kernel_opt_task_attempts, the kernel_opt_attempts accessors, kernel_opt_attempts_count, the _resolve_kernel_patch_identity shim and resolve_kernel_opt_max_failures stay deleted; none is read anywhere in the tree. SharedState.record_trace_analyze is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Description
A 660-minute MuseSpark-1.2 run (816B MoE MXFP4, 8× MI355X, TP=8) finished at +5.99% against a 10% target. Tracing
that run back through the code found defects in five areas: what a KEEP is judged against, how a phase decides it is
done, what lease expiry claims to do, whether a dead lever can be seen as dead, and how the kernel arm is credited.
This PR fixes the defects that have a fix in the existing infrastructure, and deletes the mechanisms that were
verified to have no function (no reader, a constant value, or an unreachable branch). Each removal was checked for a
live path and a real effect; mechanisms with a live path that changes behaviour were kept and fixed instead.
GEAK and Forge are both kept as first-class kernel backends; nothing here weakens either route.
Every commit is one concern and its message states what an operator now sees. The branch is rebased onto
origin/mainatb772e9434(no merge commit).Reconciliation with
origin/mainThe rebase was checked commit by commit against main for semantic overlap, not only textual conflicts:
branch can settle a GEAK result without measuring (
geak_no_material, invalid config).fix(geak): drain the handed-off GPUs only before a rebench that measuresmoves the drain to just before a measurement; the resumedrebench now drains the same cards.
source_patchprune block (now with anOSError-only save) is the codethis branch replaces with the recomputed preflight; its save-error test case is dropped in
test(specialists): drop the save-error case for the source-patch prune. Main's narrowed handlers intrajectory_reviewer.pyandrecipe_journal.pyare kept, on top of this branch's journal reader and without theredundant
kernel_optimizationscarry-forward.commit of this branch now only drops the defaults on values that are always set
(
refactor(framework): read the phase-done summary without defaults it never needs).dispatch_failedattempt row this branch writes is only for an authoring task that was created and ran, so abusy-domain refusal never counts against the source lever. Its proposal seeding still applies the KEEP-bar floor,
and its simplified idempotency lookup is kept beside this branch's source-patch refusal.
dropped, and the workflow contract v3 declares session breakdown 6.1 with its digest recomputed.
decision_traceandbuild_candidatesintoper-row helpers; this branch's changes are re-applied inside them (the KEEP/REVERT row now reads a
JournalEntryfrom
read_journal_entries; the bypass candidate dropsroofline_measured/rocprof_roofline).framework_repo_pathwith the promoted checkout; because thisbranch recomputes the source-patch preflight from that path every tick, patch specialists unblock once a runtime
is promoted, where the old prune flag stayed set.
geak_spec_namein_geak_accepted_kernel_specs#2046 (geak_spec_namein_geak_accepted_kernel_specs) is kept; this branch still drops the self-reporteddelta filter around it.
_bench_patch; it runs there off theevent loop through
_observe_bench_boot.rocprof_roofline,roofline_measuredand baseline-actionconvergenceare added to that list(
docs(breakdown): list the 6.1 removals of keys that only carried a constant).geak_no_materialsettle clears the slot like every other settle, and the kernel summary reads the attempt'srebench-stamped
gain_pct, which fix(breakdown): record the GEAK candidate at close and kernel e2e gain from the rebench #2048 now defines as Hyperloom's measurement.this branch's changes without overlap; the backend env var is still read only at bootstrap.
KEEP-threshold or GEAK self-report reads that this branch's rules would change.
Review follow-ups
failure_regioncould cut the failing line out (fixed,fix(bringup): start the failure region at the first line of the furthest milestone): milestone markers repeat per TP rank and while serving (GPU KV cache usage),so a later repeat could start the region after a rank's fatal line and lose its failure kind and
env_fault. Theregion now starts at the first line of the furthest milestone reached; a test pins the reverse ordering.
fix(bringup): classify a boot off the event loop): thebaseline and integrate-bench observations now run in a worker thread. The previous edge-plus-milestone reader also
streamed the whole log synchronously, so the loop placement predates this PR; the read stays whole because a root
traceback can sit megabytes above the tail.
sweep_done(fixed,docs(sweep): …).output_throughputrename without a major bump: resolved by fix(breakdown): rename throughput_tok_s_per_gpu to output_throughput (schema 6.1) and record per-GPU output on baseline and validation rows #2023 (schema 6.1); this branch's copy is dropped.exceptflagged by code scanning (fixed,test(geak): …).Bring-up classification
is no longer lost between fixed edge windows; the failure rules read only the region from the furthest milestone
reached to the root traceback, so a rule match the process ran past (an autotuner's rejected config) does not name
the failure. Classification runs off the event loop.
Measurement and KEEP
keep_threshold_pctcan only raise the session's KEEP bar. A lower, negative or non-finite value isreplaced by the session bar; before,
0kept any non-negative gain and a negative value kept regressions.assess_convergenceis deleted: it always returnedinsufficient_roundsand nothing read it.optimization_journal.jsonis read through the journal module only; failure attribution is carried from exploreinstead of being re-derived.
Phase state machine
global_convergedfor saturateddirections,
time_exhaustedfor too little time) instead ofsweep_done.--no-kernel, FRAMEWORK_AGENT's own exit reason is kept; the skip is markedkernel_skipped: true.predicate_inputs.phase_budget_pctand the 1000 macro-cycle ceiling are removed.Leases and cancellation
prune_branchnow expands a catalogue family (prep,shallow, …) into its member actions; before, pruning afamily the prompt showed cancelled nothing. Proposing a member of a pruned family draws the pruned-action advisory.
lease_expiredtopic,gpu_pool.extendand the uncalled per-lease heartbeat are deleted; fourdocstrings that promised lease reclamation now say what the code does.
Lever liveness (FRAMEWORK_AGENT)
coming back empty is seen as dry and the phase can leave on its plateau rule. In the audited run the arm was dead
from the start and FRAMEWORK_AGENT could only leave on its budget.
backend and warns
no claude CLI/no codex CLI; specialists cannot start without it (assets/install.sh).~/.claudesettings (--setting-sources=).getattrswitches that read attributesSharedStatenever had, thesource_patchprune flag(a cached copy of a deterministic preflight), a broad
except Exception: pass.Kernel arm
current_bestand brings no loadable artifact settles asgeak_no_materialbefore any benchmark; in the audited run that no-op rebench cost 16.7 minutes.throughput_speedup;GEAK's self-reported
e2e_delta_pctno longer filters which accepted kernels go to the rebench.read from a ledger nothing wrote, so
kernel_work_pendingstayed true for any ≥5% kernel.kernel_optimization_summary.jsonis built from the timeline'sV6KernelAttemptrecords; GEAK success means"adopted by Hyperloom's rebench", and GEAK's own acceptances appear as
claimed.recovered, notsucceeded.kernel_opt_task_attempts/last_kernel_optledger and its readers, the dead rocprofenrichment hook,
roofline_measuredandrocprof_roofline.Linked issue(s)
None.
Tests
each commit message). New contract tests cover the KEEP-bar floor, empty-round dryness, family pruning, the GEAK
pre-measurement no-material settle,
recoveredprofiles, the summary built from timeline records, and the boundedGEAK drain. Tests that only asserted a removed thing was absent, or duplicated another test's contract, were dropped.
imports a changed module (≈11.7k tests), every test file main had changed under
src/hyperloom, and theGEAK/specialist suites touched by the reconciliation. The rebased tree was checked with ruff,
lint-importsandscripts/code_metrics.py. The review follow-ups ran the bring-up suites and every test that observes a boot(315), the baseline and integrate executor suites (827), the GEAK drain test, and the attempt-ledger, preflight,
specialist and reconcile suites touched by the module-length trims (428). The only failures are four tests that
fail identically
on the base commit in this environment (
test_derive_runtime_paths×2,test_cli_resume_launch_shape,test_tracelens_csv— shellANTHROPIC_MODEL/ ROCm-path dependent).ruff check,ruff format --checkon touched files;lint-imports: all contracts kept.Size/complexity triggers crossed
None.
scripts/code_metrics.py --base-ref b772e9434passes: no new or worse unit, and every baseline entry thebranch touched is lowered. Three modules were already over their recorded length on
origin/main; this branchleaves each at or under its length there:
origin/mainorchestrator/phases/framework.pyinference_optimizer/cli/preflight.pyorchestrator/loop/coordinator.pySimplification
Mechanisms removed, with the contract preserved and where it is pinned:
results (
test_untried_hot_dedup.py,test_kernel_idle_streak.py,test_kernel_idle_no_progress.py).(
test_geak_revalidation_dispatch.py).read_journal_entries(test_optimization_journal.py).source_patchprune flag → the deterministic preflight it cached (test_custom_specialist.py,test_framework_*).hyperloom.common.agent_cli, one resolver for the CLI layer, the Claude roleand the specialist launcher.
Observable effect
session_breakdown.json(stays at 6.1): per-kernel roofline rows droproofline_measuredandrocprof_roofline; the baseline action dropsconvergence. None of the three is documented in the reference, andeach held a constant or unread value.
predicate_inputs.phase_budget_pctandsweep_reloop.max_cyclesremoved;no_kernel_skippedleavesstop_reasons(it was only ever a transition label).kernel_optimization_summary.jsonschema_version2 → 3:totalscarriesclaimed / adopted / rejected / failed / needs_review / not_dispatched;rejection_breakdownis gone;by_kernelhas one row per candidate.kernel_metrics.csvdropsroofline_measured; the bypasssummary.jsondropsrocprof_enrich;the roofline action can report
status: "recovered".HYPERLOOM_SPECIALIST_PERMISSION_MODE,INFERENCE_OPTIMIZER_KERNEL_OPT_MAX_FAILURES,HYPERLOOM_ROCPROF_ROOFLINE_ENRICH,HYPERLOOM_ROCPROF_ROOFLINE_TIMEOUT_SEC.rocprof_roofline(never populated) androofline_measured/ baseline-actionconvergence(removed in 6.1, constant).~/.claude/settings.json; remote-recipe kernel rewrites publish their real speedup (it was always 0.0);extend_leaseobservations no longer carrygpu_rows/gpu_refresh_error; a boot whose fatal line is followedby another rank's repeat of a milestone keeps its failure kind and
env_fault; boot classification no longerstalls the coordinator loop; the preflight agent-CLI warning text names the backend.
Breaking changes
Yes — the removed fields, the workflow contract v3 and the env vars listed above. No documented
session-breakdown result field is renamed or removed, so the breakdown stays at 6.1.
PR addresses single concern
No. The commits share one root — the defects found tracing a single run — and each commit is one concern with its
own message, so they can be reviewed commit by commit. They are kept together because several touch the same
modules (
writeback.py,kernel.py,shared_state.py) and were verified as a whole. Happy to split by area(measurement, phases, leases, framework lever, kernel arm) if preferred.
Root cause is upstream
Partly. GEAK's
run_e2e.pysetsstatus = "ok"wheneverspeedup > 1.0, bypassing its own Director verdict, andGEAK's journey file omits rejected candidates. Hyperloom no longer trusts either claim (its own rebench decides);
the upstream fixes are not part of this PR and no ticket is filed yet.
🤖 Generated with Claude Code