diff --git a/src/hyperloom/inference_optimizer/tests/test_accuracy_gate_units.py b/src/hyperloom/inference_optimizer/tests/test_accuracy_gate_units.py index fe6a9df5a3..d5a722df00 100644 --- a/src/hyperloom/inference_optimizer/tests/test_accuracy_gate_units.py +++ b/src/hyperloom/inference_optimizer/tests/test_accuracy_gate_units.py @@ -13,32 +13,6 @@ from hyperloom.orchestrator.actions.executors import _accuracy_gate as ag -class TestIsHighAccuracyRisk: - def test_no_risk_when_empty(self): - assert ag.is_high_accuracy_risk("", None) is False - assert ag.is_high_accuracy_risk(None or "", {}) is False - - @pytest.mark.parametrize( - "args", - [ - "--kv-cache-dtype fp8_e4m3", - "--enforce-eager", - "--compilation-config '{\"x\": 1}'", - "--attention-backend aiter", - "--decode-attention-backend triton", - ], - ) - def test_high_risk_cli_flags(self, args): - assert ag.is_high_accuracy_risk(args, None) is True - - def test_high_risk_env_keys(self): - assert ag.is_high_accuracy_risk("", {"VLLM_ROCM_USE_AITER": "1"}) is True - assert ag.is_high_accuracy_risk("", {"SGLANG_USE_AITER": "1"}) is True - - def test_neutral_inputs_are_low_risk(self): - assert ag.is_high_accuracy_risk("--max-num-seqs 64", {"FOO": "BAR"}) is False - - class TestParseEvalResults: def test_returns_error_when_no_results_dir(self, tmp_path): out = ag.parse_eval_results(tmp_path) diff --git a/src/hyperloom/inference_optimizer/tests/test_eval_result_dir_wiring.py b/src/hyperloom/inference_optimizer/tests/test_eval_result_dir_wiring.py index e353c704b2..409b4494c9 100644 --- a/src/hyperloom/inference_optimizer/tests/test_eval_result_dir_wiring.py +++ b/src/hyperloom/inference_optimizer/tests/test_eval_result_dir_wiring.py @@ -31,7 +31,6 @@ from hyperloom.orchestrator.actions.executors._accuracy_gate import ( accuracy_passed, - is_high_accuracy_risk, parse_eval_results, ) from hyperloom.orchestrator.actions.executors._grid_runner import _run_magpie @@ -546,9 +545,9 @@ def test_parse_eval_results_keeps_results_when_root_is_warmup_slot(tmp_path): assert out.get("accuracy") == pytest.approx(0.77) -def test_warm_decision_high_risk_variant_grades_from_warmup_round(tmp_path): +def test_warm_decision_gated_variant_grades_from_warmup_round(tmp_path): """Warm-decision explore runs the decision round with ``RUN_EVAL=false``, so a - high-risk variant's only score sits under ``warmup_round/``. The gate must + gated variant's only score sits under ``warmup_round/``. The gate must grade from it and PASS rather than REVERT as ``accuracy_unavailable``. """ slot = tmp_path / "variant_00_kv" @@ -556,8 +555,6 @@ def test_warm_decision_high_risk_variant_grades_from_warmup_round(tmp_path): slot / "warmup_round" / "Qwen__model" / "results_2026-07-15T09-00-00.000000.json", 0.9462, ) - # The flag that makes this variant gated in the first place. - assert is_high_accuracy_risk(extra_args="--kv-cache-dtype fp8", extra_envs={}) is True out = parse_eval_results(slot, framework="vllm") accuracy = out.get("accuracy") diff --git a/src/hyperloom/inference_optimizer/tests/test_explore_executor.py b/src/hyperloom/inference_optimizer/tests/test_explore_executor.py index 37e861723d..46e25356d4 100644 --- a/src/hyperloom/inference_optimizer/tests/test_explore_executor.py +++ b/src/hyperloom/inference_optimizer/tests/test_explore_executor.py @@ -584,6 +584,65 @@ def _fake_run(cmd, *args, **kwargs): assert state.stop_reason == "" +@pytest.mark.asyncio +async def test_explore_gates_a_variant_no_flag_catalogue_would_have_caught(sub_agent_runner, tmp_path): + """The gate no longer asks which knobs look risky. + + ``--online_quant_config`` changes numeric precision directly, and no entry of + the deleted high-risk catalogue matched it, so a variant carrying it cleared + on throughput alone with its measured accuracy discarded. With a baseline on + the state it is now gated like any other variant, and no eval verdict is a + REVERT rather than a KEEP. + """ + sub, tr, _ = sub_agent_runner + state = SharedState() + state.baseline_tput = 800.0 + state.baseline_accuracy = 0.80 + sub.shared_state = state + + base = tmp_path / "base.yaml" + _write_baseline_yaml(base) + + def _fake_run(cmd, *args, **kwargs): + out_idx = cmd.index("--output-dir") + slot = Path(cmd[out_idx + 1]) + _fake_workspace(slot, tput=840.0) # +5% vs base 800 (clears throughput) + return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="ok", stderr="") + + extra_args = '--online_quant_config {"global_quant_config":"ptpc_fp8"}' + task = await tr.create( + kind="explore", + params={ + "config_path": str(base), + "output_dir": str(tmp_path / "explore-uncatalogued"), + "base_tput": 800.0, + "accuracy_baseline": 0.80, + "grid": [ + { + "name": "v_quant", + "extra_args": extra_args, + "extra_envs": {}, + "provenance": "llm_direct", + } + ], + "variant_timeout_sec": 10, + }, + idempotency_key="ex-uncatalogued-acc", + ) + sub.register_executor("explore", ExploreExecutor(session_dir=tmp_path)) + with patch( + "hyperloom.orchestrator.actions.executors._grid_runner.run_with_session_kill", + side_effect=_fake_run, + ): + res = await sub.run_task(task) + + tested = res.result["explore_search_update"]["tested"][canonical_fingerprint(extra_args, {})] + assert tested["outcome"] == "REVERT" + reasons = {lr["name"]: lr.get("reason") for lr in res.result["losers"]} + assert reasons.get("v_quant") == "accuracy_unavailable" + assert state.stop_reason == "" + + @pytest.mark.asyncio async def test_explore_accuracy_gate_falls_back_to_shared_state(sub_agent_runner, tmp_path): sub, tr, _ = sub_agent_runner diff --git a/src/hyperloom/inference_optimizer/tests/test_grid_runner_helpers_coverage_unit.py b/src/hyperloom/inference_optimizer/tests/test_grid_runner_helpers_coverage_unit.py index 73637a5ee2..de3f37574a 100644 --- a/src/hyperloom/inference_optimizer/tests/test_grid_runner_helpers_coverage_unit.py +++ b/src/hyperloom/inference_optimizer/tests/test_grid_runner_helpers_coverage_unit.py @@ -427,3 +427,34 @@ def test_remove_server_args_accepts_multi_flag_string() -> None: "--flag-a --flag-b --flag-c", ) assert out == "--keep 4" + + +def test_remove_server_args_keeps_a_sibling_json_value_parseable() -> None: + """Removing one flag must not corrupt a JSON-valued sibling. + + The JSON arrives with no shell wrapper because ``compact_json_server_args`` + strips it upstream, so the POSIX shlex round-trip used to eat the JSON's own + double quotes. The bareword repair could not put them back around atom's + ``exclude_layer`` wildcards, and ``strip_benchmark_harness_flags`` puts this + call on every launch path, so ``--online_quant_config`` reached the server + unparseable and every conc_sweep launch died in ``json.loads``. + """ + args = ( + '--online_quant_config {"global_quant_config":"ptpc_fp8",' + '"exclude_layer":["*.mlp.gate","*expert*"]} ' + "--no-enable-prefix-caching --tp 8" + ) + out = gr.remove_server_args(args, ["--no-enable-prefix-caching"]) + tokens = out.split(" ") + assert tokens[0] == "--online_quant_config" + assert json.loads(tokens[1]) == { + "global_quant_config": "ptpc_fp8", + "exclude_layer": ["*.mlp.gate", "*expert*"], + } + assert tokens[2:] == ["--tp", "8"] + + +def test_remove_server_args_unbalanced_brace_drops_only_its_own_flag() -> None: + """A stray ``}`` is not a JSON blob and must not take the tail with it.""" + out = gr.remove_server_args("--foo a} --tp 8 --max-num-seqs 64", ["--foo"]) + assert out == "--tp 8 --max-num-seqs 64" diff --git a/src/hyperloom/orchestrator/actions/executors/_accuracy_gate.py b/src/hyperloom/orchestrator/actions/executors/_accuracy_gate.py index 914df442d7..5e0704951f 100644 --- a/src/hyperloom/orchestrator/actions/executors/_accuracy_gate.py +++ b/src/hyperloom/orchestrator/actions/executors/_accuracy_gate.py @@ -3,10 +3,12 @@ """Accuracy gate — GSM8K eval integration for hyperloom.inference_optimizer. -Baseline always runs GSM8K; high-risk variants too. Threshold is -``baseline_accuracy - new_accuracy <= 0.05`` (5% tolerance), REVERT otherwise. -High-risk = precision/compute-path changes; kernel patches handled by -kernel-agent. +Baseline always runs GSM8K, and so does every variant whose round has ``RUN_EVAL`` +on -- the default. The gate reads that score for every variant rather than +guessing from flag names which ones deserve reading. Threshold is +``baseline_accuracy - new_accuracy <= 0.05`` (5% tolerance), REVERT otherwise. A +session that opts out of eval records no baseline accuracy, and that is what +leaves serving ungated there. Kernel patches are handled by kernel-agent. """ from __future__ import annotations @@ -486,54 +488,15 @@ def accuracy_keep_block( return False, "", True -# Flags indicating accuracy risk; matching variants must pass the gate. -_HIGH_RISK_CLI_PATTERNS: tuple[str, ...] = ( - "--kv-cache-dtype", - "--enforce-eager", - "--compilation-config", - "--attention-backend", - "--decode-attention-backend", -) - -_HIGH_RISK_ENV_KEYS: frozenset[str] = frozenset( - { - "VLLM_ROCM_USE_AITER", - "VLLM_ROCM_USE_AITER_LINEAR", - "VLLM_ROCM_USE_AITER_RMSNORM", - "VLLM_ROCM_USE_AITER_FP8BMM", - "VLLM_ROCM_USE_AITER_FP4_ASM_GEMM", - "VLLM_ROCM_USE_AITER_TRITON_ROPE", - "VLLM_ROCM_QUICK_REDUCE_QUANTIZATION", - "VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT", - "AMDGCN_USE_BUFFER_OPS", - "SGLANG_USE_AITER", - } -) - - -def is_high_accuracy_risk( - extra_args: str = "", - extra_envs: dict[str, str] | None = None, -) -> bool: - """Return True if the variant changes precision or compute paths. - - Args: - extra_args (str): The variant's server args to scan for high-risk - CLI flags. - extra_envs (dict[str, str] | None): The variant's env overrides to scan - for high-risk keys. - - Returns: - bool: True when the variant matches any high-risk flag / env key. - """ - args_lower = extra_args.lower() - for pattern in _HIGH_RISK_CLI_PATTERNS: - if pattern in args_lower: - return True - if extra_envs: - if set(extra_envs.keys()) & _HIGH_RISK_ENV_KEYS: - return True - return False +# There is deliberately no "high accuracy risk" predicate here any more. It used +# to decide whether EXPLORE bothered to parse a variant's eval result, matching a +# hardcoded list of vLLM/SGLang flag names and VLLM_*/SGLANG_* env keys as +# substrings. Two ways that silently under-reported: a framework spelling the +# same knob differently (atom's ``--kv_cache_dtype`` never matched +# ``--kv-cache-dtype``) and a framework-specific knob nobody enrolled (atom's +# ``--online_quant_config``, which changes numeric precision directly). Since the +# round runs the eval whenever ``RUN_EVAL`` is on, the result is already on disk +# and the only thing the predicate bought was discarding it. def parse_quality_gate(workspace: Path | str) -> dict[str, Any]: @@ -844,7 +807,6 @@ def accuracy_passed( "eval_contract_fingerprint", "eval_enablement_allowed", "eval_probe_summary", - "is_high_accuracy_risk", "launch_enablement_allowed", "parse_eval_results", "read_eval_probe", diff --git a/src/hyperloom/orchestrator/actions/executors/_grid_server_args.py b/src/hyperloom/orchestrator/actions/executors/_grid_server_args.py index 71c188cb92..d115c6e072 100644 --- a/src/hyperloom/orchestrator/actions/executors/_grid_server_args.py +++ b/src/hyperloom/orchestrator/actions/executors/_grid_server_args.py @@ -100,6 +100,17 @@ def merge_server_args(*parts: str | None) -> str: return " ".join(str(p).strip() for p in parts if str(p or "").strip()) +def _unquote_token(token: str) -> str: + """Drop one layer of matching shell quotes from a non-POSIX-split token. + + The removal specs are still POSIX-split, so they arrive unquoted; this puts + both sides of a pair comparison in the same shape. + """ + if len(token) >= 2 and token[0] == token[-1] and token[0] in "\"'": + return token[1:-1] + return token + + def remove_server_args(server_args: str | None, remove_args: Any) -> str: """Remove flag specs from a server-arg string. @@ -108,12 +119,20 @@ def remove_server_args(server_args: str | None, remove_args: Any) -> str: token shape; ``"--foo bar"`` removes the exact flag/value pair. Unknown / unparseable inputs are left untouched rather than guessed. """ - args = str(server_args or "").strip() + # Compact the JSON values first, then split without POSIX quote processing. + # Compacting leaves every JSON value as one whitespace-free word, so the + # non-POSIX split keeps it whole AND keeps its inner double quotes, which + # the POSIX split eats (``{"a":"b"}`` -> ``{a:b}``, rejected by vLLM's + # ``json.loads`` at boot). Re-quoting afterwards cannot recover every value: + # _repair_unquoted_json has to guess where the quotes went, and atom's + # ``--online_quant_config`` wildcards (``*.mlp.gate``) fall outside that + # guess, so they reached the server unparseable. + args = _reserialize_json_blobs(str(server_args or "").strip()) removes = to_str_list(remove_args) if not args or not removes: return args try: - tokens = shlex.split(args) + tokens = shlex.split(args, posix=False) except ValueError: return args @@ -148,12 +167,12 @@ def remove_server_args(server_args: str | None, remove_args: Any) -> str: flag = tok.split("=", 1)[0] if tok.startswith("--") else "" if flag and "=" in tok: _flag, _, value = tok.partition("=") - if _flag in remove_flags or (_flag, value) in remove_pairs: + if _flag in remove_flags or (_flag, _unquote_token(value)) in remove_pairs: i += 1 continue if flag and i + 1 < len(tokens) and not tokens[i + 1].startswith("--"): value = tokens[i + 1] - if flag in remove_flags or (flag, value) in remove_pairs: + if flag in remove_flags or (flag, _unquote_token(value)) in remove_pairs: i += 2 continue if flag and flag in remove_flags: @@ -161,11 +180,9 @@ def remove_server_args(server_args: str | None, remove_args: Any) -> str: continue out.append(tok) i += 1 - # ``shlex.split`` above strips the inner double quotes of any JSON-valued - # flag (``--compilation-config {"cudagraph_mode":"FULL"}`` -> - # ``{cudagraph_mode:FULL}``); re-quote/compact the JSON blobs so removal - # never corrupts a sibling flag that vLLM parses with ``json.loads``. - return _reserialize_json_blobs(" ".join(out)) + # No re-serialisation on the way out: the non-POSIX split kept every token + # byte-for-byte, so re-joining the survivors cannot corrupt a sibling flag. + return " ".join(out) # Serving-ineligible harness flags. Enroll here; compose_server_args strips them diff --git a/src/hyperloom/orchestrator/actions/executors/explore.py b/src/hyperloom/orchestrator/actions/executors/explore.py index 886d722a06..de8d7365ea 100644 --- a/src/hyperloom/orchestrator/actions/executors/explore.py +++ b/src/hyperloom/orchestrator/actions/executors/explore.py @@ -12,7 +12,7 @@ ``explore_search`` results are evidence, never an eligibility gate. 2. Render the variant's Magpie YAML, run E2E bench. 3. Immediate KEEP/REVERT decision (``DEFAULT_KEEP_THRESHOLD_PCT`` gain - threshold + accuracy gate when ``is_high_accuracy_risk``). + threshold + accuracy gate on every variant that has a reference). Follows the "one change at a time" rule (single-tenant serving GPU). ``provenance`` passes through to the ledger unchanged so the specialist @@ -53,7 +53,6 @@ ) from ._accuracy_gate import ( accuracy_passed, - is_high_accuracy_risk, parse_eval_results, ) from . import _framework_switch_manifest as _switch_manifest @@ -768,11 +767,18 @@ async def __call__(self, ctx) -> dict[str, Any]: base_unset_envs = to_str_list(params.get("base_unset_envs")) base_args_mode = str(params.get("base_args_mode") or "append").strip().lower() base_tput = float(params.get("base_tput") or 0.0) - baseline_accuracy = float(params.get("accuracy_baseline") or 0.0) or float( - params.get("baseline_accuracy") or 0.0 - ) - if baseline_accuracy <= 0 and ss is not None: - baseline_accuracy = float(getattr(ss, "baseline_accuracy", 0.0) or 0.0) + # The measured baseline outranks a proposed one. ``accuracy_baseline`` is + # offered to the LLM in the action schema while every in-tree writer only + # copies ``SharedState.baseline_accuracy``, so a proposed figure that + # disagrees is a hallucination -- and now that every variant carrying a + # reference is gated, one bad number fails a whole grid where it used to + # reach only the few variants a flag catalogue called risky. params stay + # as the fallback for an external invocation that has no state. + baseline_accuracy = float(getattr(ss, "baseline_accuracy", 0.0) or 0.0) if ss is not None else 0.0 + if baseline_accuracy <= 0: + baseline_accuracy = float(params.get("accuracy_baseline") or 0.0) or float( + params.get("baseline_accuracy") or 0.0 + ) keep_threshold_pct = float( params.get( "keep_threshold_pct", @@ -1492,24 +1498,23 @@ def _stopped_by_the_run(result: Any, *, variant: GridVariant, idx: int, round_la outcome = "REVERT" reason = "gain_below_threshold" else: - # Accuracy gate. For serving it runs only for high-risk - # variants. For scriptable frameworks the image-quality - # gate is the sole correctness signal, so every variant is - # gated and a missing gate fails closed. + # Accuracy gate. Every variant is gated: the round already + # ran the eval, so the score is on disk and the flag + # catalogue that used to decide whether to read it only + # discarded numbers already paid for -- and missed atom's + # precision knobs entirely. A session that opted out of + # eval has no baseline accuracy, which is what leaves + # serving ungated below. For scriptable frameworks the + # image-quality gate is the sole correctness signal, so a + # missing gate fails closed. from hyperloom.inference_optimizer import framework_registry scriptable = framework_registry.is_scriptable(framework) accuracy_ok = True accuracy_value: float | None = None - # Scriptable: gate every variant. Serving: only high-risk - # variants, and only when a baseline accuracy was recorded. - if scriptable or ( - baseline_accuracy > 0 - and is_high_accuracy_risk( - extra_args=gv.extra_server_args, - extra_envs=gv.extra_envs, - ) - ): + # Serving still needs a measured baseline to compare + # against; scriptable compares against a fixed 1.0. + if scriptable or baseline_accuracy > 0: eval_out = parse_eval_results(slot, framework=framework) accuracy_value = eval_out.get("accuracy") if isinstance(accuracy_value, (int, float)): @@ -1524,8 +1529,8 @@ def _stopped_by_the_run(result: Any, *, variant: GridVariant, idx: int, round_la else: # No eval result. Both scriptable and serving # fail closed: a gated variant (scriptable, or a - # high-risk serving variant with a baseline) that - # yields no accuracy verdict likely broke the eval + # serving variant with a baseline) that yields no + # accuracy verdict likely broke the eval # path, so the change is reverted. The former # serving throughput-only skip is removed. Baseline # is where a missing accuracy result halts the run;