diff --git a/src/hyperloom/inference_optimizer/tests/test_kernel_request_handlers_units.py b/src/hyperloom/inference_optimizer/tests/test_kernel_request_handlers_units.py index ad855c6d33..a82d7f5f5c 100644 --- a/src/hyperloom/inference_optimizer/tests/test_kernel_request_handlers_units.py +++ b/src/hyperloom/inference_optimizer/tests/test_kernel_request_handlers_units.py @@ -5601,3 +5601,124 @@ def test_a_longer_explicit_budget_is_never_shortened(monkeypatch): generous = aiter_jit.BASELINE_COLD_START_TIMEOUT_SEC + 1200 assert rh._cold_start_rebaseline_timeout(generous) == generous + + +def _gemm_trace_rows(tmp_path: Path, result: dict) -> list[dict]: + """Run the GEMM-tuning audit and return the rows it appended.""" + krh._trace_gemm_tuning_run(result, session_dir=tmp_path) + path = tmp_path / "reports" / "trace" / "gemm_tuning.jsonl" + if not path.is_file(): + return [] + return [json.loads(line) for line in path.read_text(encoding="utf-8").splitlines() if line.strip()] + + +def test_gemm_trace_keeps_tuner_failure(tmp_path: Path) -> None: + # A tuner killed by the OS and a tuner that found nothing both arrive with no + # speedup. The audit row must keep the tuner's own status, cost and error, or + # the two are indistinguishable in the trace. + rows = _gemm_trace_rows( + tmp_path, + { + "engine": "forge", + "status": "failed", + "tuners_run": [ + { + "tuner": "a8w8", + "status": "failed", + "elapsed_s": 500.42, + "error": "Tuner exited with code -9: " + "x" * 5000, + "error_class": "subprocess_error", + } + ], + }, + ) + assert len(rows) == 1 + tuner = rows[0]["tuners_run"][0] + assert tuner["status"] == "failed" + assert tuner["elapsed_s"] == 500.42 + assert tuner["error_class"] == "subprocess_error" + assert tuner["error"].startswith("Tuner exited with code -9") + assert len(tuner["error"]) == krh._TRACE_ERROR_CHARS + + +def test_gemm_trace_promotes_tuner_error_class_to_envelope(tmp_path: Path) -> None: + # The run envelope leaves error_class unset even when the tuner failed, so a + # failed run was not greppable by class. Promote the first tuner's class. + rows = _gemm_trace_rows( + tmp_path, + { + "engine": "forge", + "status": "failed", + "error_class": None, + "tuners_run": [ + {"tuner": "fmoe_ck", "status": "failed", "error_class": "subprocess_error", "error": "boom"} + ], + }, + ) + assert rows[0]["error_class"] == "subprocess_error" + + +def test_gemm_trace_envelope_error_class_wins_over_tuner(tmp_path: Path) -> None: + rows = _gemm_trace_rows( + tmp_path, + { + "engine": "forge", + "status": "failed", + "error_class": "global_timeout", + "tuners_run": [{"tuner": "a8w8", "status": "failed", "error_class": "subprocess_error"}], + }, + ) + assert rows[0]["error_class"] == "global_timeout" + + +def test_gemm_trace_clean_run_row_stays_compact(tmp_path: Path) -> None: + # A tuner that simply found nothing carries no error, so the new keys are + # omitted rather than written as nulls. The legacy keys stay unconditional. + rows = _gemm_trace_rows( + tmp_path, + { + "engine": "forge", + "status": "ok", + "tuners_run": [{"tuner": "sglang_dense_bf16", "status": "no_improvement", "elapsed_s": 11.19}], + }, + ) + tuner = rows[0]["tuners_run"][0] + assert tuner == { + "tuner": "sglang_dense_bf16", + "best_micro_speedup": None, + "kept": None, + "status": "no_improvement", + "elapsed_s": 11.19, + } + assert "error_class" not in rows[0] + + +def test_gemm_trace_survives_non_mapping_tuner_entries(tmp_path: Path) -> None: + rows = _gemm_trace_rows( + tmp_path, + {"engine": "forge", "status": "ok", "tuners_run": ["junk", None, {"tuner": "a8w8"}]}, + ) + assert [t["tuner"] for t in rows[0]["tuners_run"]] == ["a8w8"] + + +def test_gemm_trace_does_not_stamp_a_successful_run_with_a_tuner_error(tmp_path: Path) -> None: + # A run can succeed on one tuner while another fails. Promoting that tuner's + # class onto the row would report the whole run as failed by it. Across the + # 320 traces on record no ok row carries an error_class, and 119 of them hold + # a tuner that returned no speedup -- so this is the common case, not a corner. + rows = _gemm_trace_rows( + tmp_path, + { + "engine": "forge", + "status": "ok", + "error_class": None, + "tuners_run": [ + {"tuner": "a8w8", "status": "failed", "error_class": "subprocess_error", "error": "boom"}, + {"tuner": "fmoe_ck", "status": "ok", "best_micro_speedup": 1.31}, + ], + }, + ) + # The row is clean... + assert "error_class" not in rows[0] + # ...and the failure is not lost, only kept where it belongs. + assert rows[0]["tuners_run"][0]["error_class"] == "subprocess_error" diff --git a/src/hyperloom/orchestrator/kernel/request_handlers.py b/src/hyperloom/orchestrator/kernel/request_handlers.py index 13227856b6..962b8a113a 100644 --- a/src/hyperloom/orchestrator/kernel/request_handlers.py +++ b/src/hyperloom/orchestrator/kernel/request_handlers.py @@ -4874,6 +4874,29 @@ async def run_collective_handler(payload: dict, *, session_dir: Path) -> Handler return result +#: Characters of a tuner's error kept in the audit row. Enough to identify the +#: failure (a memory fault, a kill, a missing file) without copying the tuner's +#: whole tail into an append-only trace. +#: +#: This bounds an error that exists; it does not create one. A rejected argument +#: reaches neither side of it -- the downstream tuner's rejection is a returncode +#: from ``subprocess.run``, and the wrapper's own is a ``SystemExit``, which its +#: ``except Exception`` does not catch. Such a run arrives with no ``error`` at +#: all, and is identified by ``status`` and the absent speedup instead. +_TRACE_ERROR_CHARS = 400 + +#: Audit keys emitted even when null, so the entry keeps the shape readers have +#: always seen. ``kept`` is in here for that reason alone: nothing on the GEMM +#: path writes it -- re-measured 18 Aug 2026, it is null in all 366 tuner +#: entries across the 322 traces on record -- so no reader can be indexing on +#: its value, only on its presence. The write-back that would populate it, +#: ``_validate_forge_gemm_tuning_e2e``, writes a separate ``e2e_results`` list +#: and runs after this trace fires; that gap is real and is not fixed here. +#: Every other key is dropped when empty, which keeps a failure's row about as +#: small as a clean one's rather than making either smaller than before. +_TRACE_TUNER_KEEP_NULL = frozenset({"tuner", "best_micro_speedup", "kept"}) + + def _trace_gemm_tuning_run(result: Any, *, session_dir: Path) -> None: """Append one ``gemm_tuning.jsonl`` audit row for a GEMM-tuning run. @@ -4892,17 +4915,29 @@ def _trace_gemm_tuning_run(result: Any, *, session_dir: Path) -> None: from hyperloom.inference_optimizer.session.session_paths import gemm_tuning_steps_path engine = str(result.get("engine") or result.get("backend") or "").strip().lower() or "unknown" + envelope_failed = str(result.get("status") or "").strip().lower() == "failed" tuners: list[dict[str, Any]] = [] + tuner_error_class: str | None = None for t in result.get("tuners_run") or []: if not isinstance(t, dict): continue - tuners.append( - { - "tuner": t.get("tuner") or t.get("name"), - "best_micro_speedup": t.get("best_micro_speedup"), - "kept": t.get("kept"), - } - ) + # A tuner that crashed and a tuner that found nothing both arrive with no + # speedup. Carrying only the speedup made them indistinguishable in the + # audit, so an environment failure read as a tuning verdict. Keep the + # tuner's own outcome, its cost, and the head of its error. + error = t.get("error") + entry = { + "tuner": t.get("tuner") or t.get("name"), + "best_micro_speedup": t.get("best_micro_speedup"), + "kept": t.get("kept"), + "status": t.get("status"), + "elapsed_s": t.get("elapsed_s"), + "error_class": t.get("error_class"), + "error": str(error)[:_TRACE_ERROR_CHARS] if error else None, + } + tuners.append({k: v for k, v in entry.items() if v is not None or k in _TRACE_TUNER_KEEP_NULL}) + if tuner_error_class is None and t.get("error_class"): + tuner_error_class = str(t.get("error_class")) row = { "kind": "gemm_tuning", "ts": datetime.now(timezone.utc).isoformat(timespec="microseconds"), @@ -4919,7 +4954,15 @@ def _trace_gemm_tuning_run(result: Any, *, session_dir: Path) -> None: "workspace": result.get("workspace"), "requires_e2e_validation": result.get("requires_e2e_validation"), "tuners_run": tuners, - "error_class": result.get("error_class"), + # The envelope leaves ``error_class`` unset even when every tuner failed; + # promote the first tuner's class so a failed run is greppable. Only for a + # run that actually failed: a tuner may fail inside a run that succeeds, + # and stamping that row would break the invariant readers rely on, that + # ``error_class`` means the run did not succeed. It holds in every trace on + # record -- re-measured 18 Aug 2026, 0 of 320 ``ok`` rows carry one, + # while 121 of them have a tuner that returned no speedup and would be + # promoted from here ungated. + "error_class": result.get("error_class") or (tuner_error_class if envelope_failed else None), } row = {k: v for k, v in row.items() if v is not None} try: