Repository navigation
fix: judge explore variants and warm recipes by what they change about the running launch - #1908
lishuoshuo-amd wants to merge 6 commits into
Conversation
Code metrics gate: PASSEDJudged: the 12 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): 1
Already over at the base (backlog, not this change's): 6
86 findings in files this change does not touchOutside this change (informational): 86
...and 61 more; the job log lists every row.
Thresholds and sources: |
lishuoshuo-amd
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…unch Explore identified a variant by the text of its own delta, over a base it never looked at: the round dedup fingerprinted extra_server_args alone, and the baseline-noop probe diffed only the settings a variant names, so it exempted removals and list-valued flags and kept every empty delta. On a MiniMax-M3 AgentX run two variants restating the operator's --max-num-seqs 64 and --kv-cache-dtype fp8 each re-measured the baseline for about 68 minutes. launch_change composes a candidate onto the current best's recorded launch (launch_evidence, written from the materialized config by every executor) and returns the full difference: settings set, rewritten or removed, envs set or unset, minus settings the engine already resolved to the requested value. plan_round drops a variant with no change and collapses variants with the same change, replacing the probe, its exemptions and the text-based round dedup. The re-measure exemption named resume_stack_revalidate, a source #1739 renamed to stack_revalidate, so it never matched; stack revalidations only ran because empty deltas were kept. It now names STACK_REVALIDATE_SOURCE.
A recipe's best_config is a delta from the session that produced it, and the replay laid it onto whatever baseline the current session has. When the operator config had since adopted those flags, the replay re-measured the baseline for a full AgentX round and settled as drift. Judge the recipe with launch_change against the current best's recorded launch and skip it as recipe_already_in_baseline when it changes nothing. A recipe carrying patches or kernel work still replays; an empty recipe still skips as best_config_empty.
…with The tick summary carried current_best without its measurement, and the orchestration sandbox cannot read the session config, so the model never saw the operator's server flags. It proposed --max-num-seqs 64 and --kv-cache-dtype fp8 as new levers on a baseline that already passed both. Render the deduplicated requested server args from the current best's launch evidence, the record launch_change judges candidates against.
90c61a7 to
48d8bf3
Compare
lishuoshuo-amd
changed the title
PR #1908 -- fix: judge explore variants and warm recipes by what they change about the running launchWhat it does: Restatements of the running launch were measured as experiments. A warm recipe and two explore rows that only re-passed the baseline's own flags cost about 3.4 h on a MiniMax-M3 run, because candidates were identified by the text of their delta and never compared with the launch the stack runs. This PR adds Blocking issues: 2
SKIPPED: V4 -- openprs.txt compared #1032, #1154, #1276, #1675 and #1869 on their first 100 changed files only, so overlap beyond those files was not checked. Checked: launch_change.py, explore.py ( |
The removal and replace rules a grid variant launches with lived inline in _build_variant_yaml. Move them to compose_variant_args so the launch and the judgement of what a variant changes compose the same way.
…onto Review of #1908: plan_round composed every variant onto current_best's recorded launch, but a round launches its variants on its own base, config_path plus base_extra_args, base_extra_envs and the base controls. When Orchestration supplies base_extra_args the two differ, and round_dup merged rows that launch different argv. round_base builds the base from the round's own inputs and the same composition the launch uses (compose_variant_args). The engine's resolved settings describe the recorded launch only, so they apply to a round base only when it launches the same args. Warm replay keeps the current best's record as its base: it is enqueued from the baseline's own writeback.
Review of #1908: both copies of the explore contract still said only exact same-grid duplicates are collapsed. A row is now judged by what it changes about the round's base launch: one changing nothing is skipped, and rows with one change are benched once.
|
Both blocking issues are addressed at
The two CodeQL cyclic-import notes are on |
chaojhou
left a comment
chaojhou
left a comment
There was a problem hiding this comment.
PR #1908 -- fix: judge explore variants and warm recipes by what they change about the running launch
What it does: Restatements of the running launch were measured as experiments, costing about 3.4 h on a MiniMax-M3 run. This PR adds launch_change, which composes a candidate onto its round's base launch with the same compose_variant_args the launch uses and returns what differs. explore's plan_round uses it to skip rows that change nothing (baseline_noop) and to bench rows with the same change once (round_dup), replacing the #1647 probe and the text-based dedup. Warm replay uses it to skip a recipe already in the baseline (recipe_already_in_baseline), and the orchestration summary now shows current_launch_flags. Both items from the earlier self-review are fixed at this head. It also fixes a live bug on main: stack revalidation rows are exempt again (STACK_REVALIDATE_SOURCE replaces the dead resume_stack_revalidate name).
Blocking issues: 3
-
[X6] The PR conflicts with main, and #1903 has since landed a second restatement mechanism there; the rebase must leave one [verified]
Problem: #1903 (8c4b640) addedactions/executors/_launch_restatement.py(named_settings,_opposite_setting,restates_the_baseline,config_launch_restated_by), a changed empty-delta branch infilter_baseline_noop_variants, andPreludePhase._skip_restated_replaywith skip coderestates_baseline, judged againstbaseline_config_path.git merge-tree origin/main 00bf8560bconflicts in five files but merges prelude.py cleanly, with both checks in one flow:replay_skip_code(recorded launch,recipe_already_in_baseline) at :1314 and_skip_restated_replay(config file,restates_baseline, exempt under reference launch controls) at :1365. The tree also carries two byte-identical argv parsers,launch_change._named_settingsand_launch_restatement.named_settings. src/hyperloom/orchestrator/phases/prelude.py:1312
Impact: one decision made twice against two bases with two skip codes. Warm-replay event consumers see either code for the same outcome, and the next parser fix (item 2) lands in one copy and is missed in the other.
Action: when rebasing,launch_changeowns the decision. Delete_skip_restated_replayand its call,restates_the_baseline/config_launch_restated_by/_opposite_settingand the duplicatenamed_settings, and keep one warm skip code. Keepconfig_envs(with #1903'sNonefilter) and the helpers #1904'sfilter_context_copy_drafts/_materialized_launchstill read. Port #1903's tests ontoplan_round/replay_skip_code, and state in the description any expectation that deliberately flips (the reference-launch-controls warm replay case). -
[D]
launch_changesees no change when the candidate flips a negated flag the base already names [verified]
Problem:_named_settingskeys--enable-prefix-cachingasenable_prefix_cachingand--no-enable-prefix-cachingasno_enable_prefix_caching, two independent settings, and composition appends without folding them. When the base passes both spellings (the engine takes the last one, so the cache is off) and a row appends--enable-prefix-caching(the cache is now on), the base and composed dicts are equal. src/hyperloom/orchestrator/actions/executors/launch_change.py:178
Impact: reproduced at head throughplan_round. A config with--enable-prefix-cachingplusbase_extra_args="--no-enable-prefix-caching", and a row--enable-prefix-caching, is skipped asbaseline_noopalthough it changes the engine's setting.replay_skip_codeskips such a warm recipe asrecipe_already_in_baselinethe same way. main pins the opposite in #1903'stest_restates_the_baseline[negated_spelling_in_the_baseline].
Action: have the one parser resolve what the engine resolves. Fold a negated boolean spelling onto its positive setting with a value (x=True/x=False, last occurrence wins), so the two dicts differ exactly when the effective setting differs, with no separate opposite-spelling exemption. Carry #1903's case over as alaunch_changetest. -
[D] Rows skipped as
baseline_noop/round_dupnever count as decided, so the FRAMEWORK_AGENT auto-bench stalls behind them [verified]
Problem: skipped rows go only toper_variant_outcomes/last_round.skipped_dup(explore.py:1578-1588).explore_search.testedis written only for benched and warmup-failed variants (:1022, :1270).FrameworkPhase._maybe_bench_untested_proposals(framework.py:2813-2852) takesuntested_proposal_rows()[:4], where "untested" means "not inexplore_search.tested", and keys its task on those four fingerprints throughcreate_or_return_existing, which returns a finished task with the same key. At the PR base, a row restating the stack produced an empty delta that the probe kept and benched, so it left the queue. #1903 has since dropped that class on main as well, so main stalls the same way, and this PR, which replaces that mechanism, is where it gets fixed. src/hyperloom/orchestrator/actions/executors/explore.py:1578
Impact: reproduced with the realFrameworkPhase,plan_roundanduntested_proposal_rows. A specialist round proposes four rows the stack already passes, thenlpm. The first auto-bench skips all four, the second computes the same keyauto-explore-c0-7d0008f798faand enqueues nothing, andlpmis never benched in that macro cycle. This is the restatement pattern the description reports from the MiniMax-M3 run, so the PR saves the hour-long runs but can starve the real proposals behind them.
Action: a row skipped because it changes nothing, or the same thing another row measures, is a decision. Record it in the explore ledger under its own fingerprint with its skip reason (and the twin's fingerprint forround_dup), sobenched_fingerprints/untested_proposal_rowsand the negative-ledger renderer read "decided" from one source. Do not add a guard in the auto-bench path. Pin it with a test: an all-restatement head followed by a real row, where the next auto-bench grid contains the real row.
Non-blocking:
launch_changedoes not modeloverlay_pythonpathorruntime_override. Today only thegeak_revalidateandstack_revalidaterows carry an overlay, and both are exempt, so this is latent. #1903'stest_restated_args_with_a_kernel_overlay_are_not_a_restatementshould still be ported or retired explicitly.round_basestringifies config envs, so a null config env becomes'None'and a row requestingFOO='None'is skipped. #1903'sconfig_envsdrops None; carry that filter over.- The engine-record gate in
round_basecompares args but not env against the recorded launch. _named_settingsreturns{}when shlex fails, which judges every row a no-op. Unreachable today becauseseal_server_argvvalidates with posix shlex, but it fails toward skipping; raise instead.
Checked: launch_change.py; explore.py plan_round, _remeasures_on_purpose, base resolution, skip handling into per_variant_outcomes / tested_update; _grid_runner.py _build_variant_yaml / compose_variant_args; grid_server_args and launch_log_evidence flag naming; prelude.py maybe_enqueue_warm_replay and warm_recipe_change.py; writeback current_best stamping and the revalidation producers; framework.py auto-bench and task_registry idempotency; render.py untested_proposal_rows / current_launch_flags; prompt_builder explore contract; #1903 (8c4b640) and #1904 on origin/main; base filter_baseline_noop_variants | Ran: git merge-tree origin/main 00bf8560b; repro scripts on archives of head and origin/main (same-effect rows, removals, list flags, round-base mismatch, stack revalidation, overlay, negated spelling, null env, shlex failure); FrameworkPhase._maybe_bench_untested_proposals with plan_round at head (same idempotency key, real row never enqueued). Not run: pytest (not installed here) | Base: e67763d | Head: 00bf856
|
Heads-up from the directory-size gate that landed in #2136 ( This PR adds To pass, either move the new module(s) into a subpackage of |
Description: what and why
On a MiniMax-M3-MXFP4 AgentX release-qualification run (
hyperloom:14-31-g6-054c763634, 720 min budget, Hyperloom123ad18), three hour-long AgentX measurements launched exactly the baseline:--kv-cache-dtype fp8 --enable-prefix-cachingmaxseqs-64-tight-graphs--max-num-seqs 64kv-cache-fp8--kv-cache-dtype fp8Each run's
launch_evidence.jsonrecords the sameobserved_server_config(22 settings) and env as the baseline's. Together they cost about 3.4 h; the session closed with 0 KEEP, and its closing summary listed "kv-cache fp8 (flat)" as a known dead end. The 2026-10-15 top-model-daily MiniMax-M3 run (hyperloom:2-19-g16-293468d5b2) ran the same baseline-identical warm replay.Root cause. Candidates were identified by the text of their own delta (
extra_server_args/extra_envs) over a base nothing looked at:best_configis a delta from the session that produced it, replayed onto whatever baseline the current session has;Fix. Judge every candidate by what it changes about the base it is composed onto, composed the same way the launch composes it.
compose_variant_args: the removal and replace rules a grid variant launches with move out of_build_variant_yamlinto one function, which the launch and the judgement both call.launch_change(newactions/executors/launch_change.py) composes a candidate onto aBaseLaunchand returns the full difference: settings set, rewritten or removed; envs set; every unset (the image env a launch inherits is not recorded). A setting the engine resolved to the requested value is not a change, which keeps feat(explore): skip baseline-noop variants #1647's engine-record rule, but only on the launch the engine was observed running.round_basebuilds the base from the round's own inputs,config_pathenvs plusbase_extra_args,base_extra_envsand the base controls, and takes the engine record of the current best's launch (launch_evidence, written from the materialized config by every executor) only when the round base launches the same args.plan_rounddrops a variant with no change (baseline_noop) and benches variants with the same change once (round_dup). It replaces the probe, its two exemptions, its empty-delta branch and the text-based round dedup. The re-measure exemption namedresume_stack_revalidate, a source [Refactor] Orchestration layer: retire delegation mechanism and converge ownership #1739 renamed tostack_revalidate, so it never matched; stack revalidations only ran because empty deltas were kept. It now namesSTACK_REVALIDATE_SOURCE.phases/warm_recipe_change.replay_skip_codejudges the recipe with the same function against the current best's recorded launch, which is the baseline's (the replay is enqueued from the baseline's own writeback), and skips it asrecipe_already_in_baselinewhen it changes nothing. A recipe carrying patches or kernel work always replays; an empty recipe still skips asbest_config_empty.to_prompt_summarycarriescurrent_launch_flags(already passed by the running stack)=, the deduplicated requested args of that same record, so the model proposes against the base the system judges against. The explore contract in the prompt now states the rule: a row changing nothing is skipped, rows with one change are benched once.Linked issue(s): none. Found through Dispatron problem Lack of dedicated resources impacts the analysis result #56 (plan
hyperloom-release-qualification).Tests: added/updated? commands run?
test_explore_executor.py::test_explore_benches_only_what_changes_the_running_launch(base: all four rows benched, including two restatements and a duplicate effect)test_explore_executor.py::test_explore_judges_rows_on_the_round_base_not_the_current_best(the review repro: a round with its ownbase_extra_argsmust benchlpmandlpm-plus-mem; fails at48d8bf3)test_explore_executor.py::test_a_stack_revalidation_runs_although_it_restates_the_stack(fails with the staleresume_stack_revalidateexemption)test_warm_replay.py::test_warm_replay_skips_a_recipe_the_baseline_already_launches_with(base: replay enqueued)test_shared_state_renderers.py::test_prompt_summary_renders_the_flags_the_current_best_launched_with(base: no such line)_ALREADY_THE_STACK,_MUST_RUN, removals, the engine-record and engine-rewrote tests) now assert exactLaunchChangevalues. Two cases changed meaning on purpose: removing a flag the stack does not pass is no change (the argv is fully recorded), and removing one it passes is a change; an unset env is always a change.test_warm_replay.py::test_warm_replay_runs_a_recipe_that_changes_the_baseline_launch. Unit:test_the_engine_record_describes_a_round_base_only_when_it_launches_the_recorded_args.pytest .../test_explore_executor.py(117 passed)test_warm_replay.py,test_sbd_v6_warm_replay_timeline.pyandtest_shared_state_renderers.py(1084 passed)pytest .../test_grid_runner.py src/hyperloom/orchestrator/actions/executors/tests/(809 passed; the twotest_kernel_tool_entry_pointscases that import TraceLens fail locally without it, at base too)pytest .../test_shared_state_renderers.pyplus the six files asserting the prompt summary (196 passed)python scripts/code_metrics.py --base-ref origin/main(PASSED),ruff check,ruff format --check,lint-imports(7 contracts kept),mypyon the new modules (clean)aiter-kernel-path-enable,gpu-mem-95-more-kv,multi-step-schedulerand skipsmaxseqs-64-tight-graphsandkv-cache-fp8; its five later rounds bench every row; its warm recipe returnsrecipe_already_in_baseline. Run 543 (Kimi-K3): all five explore rounds bench every row and the warm recipe replays.Size/complexity triggers crossed: split, or why the shape is right: production code +371/-305 across 8 files; tests +268/-100. explore.py loses the probe (1905 -> 1692 lines) and
ExploreExecutor.__call__gets simpler (length 1124 -> 1088, cognitive 369 -> 365, cyclomatic 76 -> 73);_build_variant_yamlshrinks (138 -> 133 lines) by callingcompose_variant_args.prelude.pyis over its recorded module length, so the warm-replay judgement is a 41-line module andprelude.pyswaps its 12-line empty-recipe check for one of the same length (maybe_enqueue_warm_replaycognitive 190 -> 189). The baseline is lowered for exactly these units.If this simplifies or refactors: removed
filter_baseline_noop_variants,observed_launch_from_state,_launch_delta,_settings_already_hold,_envs_already_hold,_subtracts_from_the_launch,_touches_a_multi_value_flagand the throwaway base materialization. Preserved: a variant whose settings the engine already resolved to the requested value is not benched (engine-record and engine-rewrote tests, ported); revalidations and GEAK config replays always run; an empty recipe skips withbest_config_emptyand persists the one-shot guard (test_warm_replay_skips_when_best_config_empty,test_every_refusal_persists_the_one_shot_guard, unchanged).Observable effect: what an operator will now see, and what the old behaviour cost them:
skipped_dupasbaseline_noop; a row repeating another row's change appears asround_dup. Before, each cost one full measurement.skipped/recipe_already_in_baselineinstead of a measureddrift.Breaking changes: no config, flag or env var changes. New values only: the warm-replay skip code
recipe_already_in_baseline.skipped_duprows forbaseline_noopnow carryfingerprintand nodetail.PR addresses single concern: yes. One finding, a measurement that launches what the stack already runs, fixed by judging candidates on the stack's recorded launch at the two places that launch them and showing that record to the model that proposes them.
Root cause is upstream (Magpie/TraceLens/GEAK/IntelliKit/AgentKernelArena), ticket filed: no, all changes are in Hyperloom.