From 71bb4b406eeef5485a3cff49e150653392177857 Mon Sep 17 00:00:00 2001 From: Matt Elliott Date: Thu, 6 Aug 2026 14:08:08 +0000 Subject: [PATCH 1/6] Fix three orchestrator bugs found in an overnight --framework atom run Root-caused by the orchestrator's own self-diagnosis in reports/final.md after a 9h08m run found zero validated gain despite 95.8% roofline headroom: - PolicyGate rejected every specialist delegate this session because a placeholder string ("Not found") in a source_file field isn't empty, so it reached the trusted-scope check and was denied identically 15 times. Treat known absent-value sentinels as an omitted field instead of a bogus path. - EXPLORE could exit via a stale pending_escalate_hint left over from a different phase or macro-cycle, before ever dispatching a round (explore_search.tested=0), which was the direct cause of cumulative_gain_validated=0.00%. Clear the hint whenever a phase transition fires for an unrelated reason, and on macro-cycle reload. - local_server_unreachable had no remediation branch in the action ladder; its own suggestion text pointed at "server_lifecycle", which isn't a real dispatchable action. Wire it to the existing recover action instead, mirroring the gpu_memory_leaked branch. Also threads a PolicyGate rejection's specific rule through SubAgentResult into the gap ledger as error_class instead of the generic "unknown_error", so future policy denials are recognizable instead of blending into normal retries. --- .../robustness/decision/action_ladder.py | 22 ++++++++++++ src/hyperloom/orchestrator/loop/dispatcher.py | 11 +++++- .../orchestrator/loop/sub_agent_runner.py | 7 ++++ src/hyperloom/orchestrator/phases/machine.py | 6 ++++ src/hyperloom/orchestrator/policy/gate.py | 35 +++++++++++++++++++ .../state/_shared_state/explore_state.py | 4 +++ 6 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src/hyperloom/agents/robustness/decision/action_ladder.py b/src/hyperloom/agents/robustness/decision/action_ladder.py index 3e025f8535..8875893d1e 100644 --- a/src/hyperloom/agents/robustness/decision/action_ladder.py +++ b/src/hyperloom/agents/robustness/decision/action_ladder.py @@ -303,6 +303,28 @@ def _recommend(self, sym: Symptom) -> list[Intent]: ) ) return intents + # Dead inference server -> ``delegate(recover, force_gpu_cleanup=True)``. + # ``recover``'s owner-pattern kill list already covers the atom/Magpie + # server process names (Magpie, EngineCore); this is the same remedy + # as gpu_memory_leaked, just triggered by an unreachable health check + # instead of a VRAM leak. ("server_lifecycle", named in this + # symptom's own suggestion text, is an internal warm-reuse config + # helper, not a real dispatchable action — PolicyGate would reject it + # as unknown_action.) + if sym.name == "local_server_unreachable": + evidence = dict(sym.evidence) if isinstance(sym.evidence, dict) else {} + intents.append( + build_delegate( + action_name="recover", + params={ + "reason": "local_server_unreachable", + "force_gpu_cleanup": True, + "evidence": evidence, + }, + idempotency_key=(f"recover-server-unreachable-tick-{self._last_tick_index}"), + ) + ) + return intents # Wall-clock wind-down: ``delegate(report)`` lands a deterministic # report in the remaining budget before the deadline supervisor # SIGTERMs work; ``recover_unsuccessful`` is the finalization path. diff --git a/src/hyperloom/orchestrator/loop/dispatcher.py b/src/hyperloom/orchestrator/loop/dispatcher.py index 37d053246a..515e27f8ee 100644 --- a/src/hyperloom/orchestrator/loop/dispatcher.py +++ b/src/hyperloom/orchestrator/loop/dispatcher.py @@ -1019,7 +1019,16 @@ async def _reap_dispatched_task( task=task, ) elif task.task_id not in self._dead_holder_accounted: - await self._handle_unpromotable_result(task, result_payload) + unpromotable_result = dict(result.result or {}) + # Surface a PolicyGate dispatch rejection's specific rule + # (e.g. "policy_source_file_outside_trusted_scope") into + # the gap ledger instead of letting it default to + # "unknown_error" — result.result is {} for these + # (rejected before the executor ever ran), so error_class + # would otherwise be silently dropped here. + if result.error_class and not unpromotable_result.get("error_class"): + unpromotable_result["error_class"] = result.error_class + await self._handle_unpromotable_result(task, unpromotable_result) except Exception as exc: # noqa: BLE001 log.exception( "dispatcher: promotion/unpromotable handling failed for task=%s", diff --git a/src/hyperloom/orchestrator/loop/sub_agent_runner.py b/src/hyperloom/orchestrator/loop/sub_agent_runner.py index de1bef9ebc..6419e66a83 100644 --- a/src/hyperloom/orchestrator/loop/sub_agent_runner.py +++ b/src/hyperloom/orchestrator/loop/sub_agent_runner.py @@ -67,12 +67,17 @@ class SubAgentResult: state (str): Terminal state — ``"succeeded"`` / ``"failed"``. result (dict): Executor result payload (empty on failure). error (str | None): Error string when the task failed, else None. + error_class (str): Machine-readable failure category (e.g. + ``"policy_source_file_outside_trusted_scope"``) for a + ``PolicyDenied`` dispatch rejection; empty for other failures + (executors set their own ``error_class`` inside ``result``). """ task_id: str state: str # "succeeded" / "failed" result: dict error: str | None = None + error_class: str = "" class SubAgentRunner: @@ -238,11 +243,13 @@ async def run_task( ) if prebound_lease is not None: await self.locks.release(prebound_lease) + rule = getattr(denied, "rule", "") or "denied" return SubAgentResult( task_id=task.task_id, state="failed", result={}, error=str(denied), + error_class=f"policy_{rule}", ) await self._transition_resilient( diff --git a/src/hyperloom/orchestrator/phases/machine.py b/src/hyperloom/orchestrator/phases/machine.py index 0922c91e23..6a68623ec9 100644 --- a/src/hyperloom/orchestrator/phases/machine.py +++ b/src/hyperloom/orchestrator/phases/machine.py @@ -224,6 +224,12 @@ async def _advance_phase_if_needed(self) -> None: # Consume escalate hint after a hint-driven transition. if isinstance(evidence, dict) and (evidence.get("evidence") == "llm_escalation" or "hint" in evidence): state.consume_pending_escalate_hint() + elif getattr(state, "pending_escalate_hint", ""): + # A phase change fired for a reason unrelated to the hint while + # one was still pending (e.g. set by a different phase's agent + # turn and never claimed) — clear it so it isn't inherited by the + # next phase's own exit checks as if it earned that leverage. + state.consume_pending_escalate_hint() # Terminal transition (target=CLOSE): mirror the stop_reason onto state. if ( target == _phase_state.PHASE_CLOSE diff --git a/src/hyperloom/orchestrator/policy/gate.py b/src/hyperloom/orchestrator/policy/gate.py index 3d3dd1424c..96932c5aee 100644 --- a/src/hyperloom/orchestrator/policy/gate.py +++ b/src/hyperloom/orchestrator/policy/gate.py @@ -441,6 +441,39 @@ def _whole_machine_pool_size() -> int: _REMOTE_RECIPE_FILES_PARTS = ("runtime", "remote_recipe", "files") _MAX_POLICY_PATCH_BYTES = 4 * 1024 * 1024 +# Placeholder/not-found sentinels that upstream lookups (or an LLM restating +# a miss as prose) can leave in a SOURCE_LIKE_FIELDS value instead of leaving +# the field empty. Treated as an absent field, not a bogus path: a resolver +# miss should degrade the delegate gracefully, not deny the whole intent. +# Includes the vendor-label and TraceLens placeholder forms pinned by +# reject_non_path_source()'s own test (test_source_resolution_guards.py +# _SENTINELS) -- those reach here verbatim when a stale/cached candidate +# still carries a placeholder TraceLens meant to zero at the producer. +# +# Not made redundant by tracelens_analysis.reject_non_path_source(): that +# guard only runs inside _finalize_candidates(), so it only protects +# source_file values that flowed through the TraceLens candidate pipeline. A +# delegate request can still carry one of these placeholders some other way +# (an LLM restating a miss as prose directly into a task field, or a resumed +# session replaying kernel_candidates.json written before this producer guard +# existed) and this is the last check before PolicyGate would otherwise deny +# or admit it as a bogus path. +_SOURCE_FILE_ABSENT_SENTINELS: frozenset[str] = frozenset( + { + "not found", + "none", + "n/a", + "null", + "unknown", + "unresolved", + "missing", + "tbd", + "", + "aiter (vendor)", + "triton (vendor)", + } +) + # Multi-node profile trace dirs live outside session_dir but must be referenceable by trace_dir / main_trace_path / trace_input (runtime-resolved). def _trace_path_allowlist() -> tuple[str, ...]: @@ -2335,6 +2368,8 @@ def visit(node: Any, path_keys: tuple[str, ...]) -> None: return key = path_keys[-1] if path_keys else "" if key in SOURCE_LIKE_FIELDS: + if node.strip().lower() in _SOURCE_FILE_ABSENT_SENTINELS: + return if any( self._path_in_source_allowlist(c) or self._path_under_session(c) for c in _source_file_candidates(node) diff --git a/src/hyperloom/orchestrator/state/_shared_state/explore_state.py b/src/hyperloom/orchestrator/state/_shared_state/explore_state.py index 1c04a41b7b..87fa6590df 100644 --- a/src/hyperloom/orchestrator/state/_shared_state/explore_state.py +++ b/src/hyperloom/orchestrator/state/_shared_state/explore_state.py @@ -411,6 +411,10 @@ def reset_per_cycle_plateau_state(self) -> None: self.rounds_since_last_keep = {} self.last_sweep = {} self.last_conc_sweep = {} + # A hint set during the prior macro-cycle (e.g. by kernel_agent + # completion) and never claimed by the transition it caused must not + # survive into the next cycle's phases as if they'd earned it. + self.pending_escalate_hint = "" def note_explore_outcome(self, *, promoted: bool) -> None: """Update the plateau proxy after one explore task (KEEP resets, no-promote increments). From c0b2af53de6f0a58bbcd5e4b18e1a79d12210223 Mon Sep 17 00:00:00 2001 From: Matt Elliott Date: Wed, 12 Aug 2026 15:50:47 +0000 Subject: [PATCH 2/6] fix(orchestrator): close the skip_to_kernel hint regression + zero-round EXPLORE exit Review feedback on #1125 found the unrelated-transition hint cleanup added in the prior commit was itself a regression, and that it hadn't actually closed the zero-round EXPLORE bug it was meant to fix: - machine.py's cleanup discarded a skip_to_kernel/skip_to_close hint on ANY phase transition fired for an unrelated reason, including one where the hint was still legitimately in flight toward its only consumer (exit_normal_explore, which only runs once phase == EXPLORE). A hint set during FRAMEWORK_AGENT no longer survived the FRAMEWORK_AGENT -> EXPLORE transition to reach it. Only discard now when the transition target isn't EXPLORE, and log the discard. - exit_normal_explore still honored skip_to_kernel unconditionally, ahead of compute_plateau_explore's own evidence requirement -- so a hint that arrived before EXPLORE had dispatched any specialist round this cycle still exited with explore_search.tested=0 (the direct cause of a prior cumulative_gain_validated=0.00% session). Gate the hint on at least one specialist round having run this macro-cycle. Co-Authored-By: Claude Sonnet 5 --- .../test_coordinator_async_batch2_unit.py | 43 +++++++++++++++ .../tests/test_phase_state_machine.py | 54 +++++++++++++++++++ .../test_policy_source_file_from_trace.py | 28 ++++++++++ src/hyperloom/orchestrator/phases/machine.py | 22 +++++--- .../orchestrator/phases/machine_state.py | 16 ++++-- 5 files changed, 153 insertions(+), 10 deletions(-) diff --git a/src/hyperloom/inference_optimizer/tests/test_coordinator_async_batch2_unit.py b/src/hyperloom/inference_optimizer/tests/test_coordinator_async_batch2_unit.py index 0c818c0cc1..c1527cf849 100644 --- a/src/hyperloom/inference_optimizer/tests/test_coordinator_async_batch2_unit.py +++ b/src/hyperloom/inference_optimizer/tests/test_coordinator_async_batch2_unit.py @@ -2018,6 +2018,49 @@ async def _entered(*, from_phase, to_phase): assert coord.shared_state.stop_reason == "target_reached" +@pytest.mark.asyncio +async def test_advance_phase_hint_survives_transition_toward_explore(coord: Coordinator, monkeypatch) -> None: + """A skip_to_kernel hint set during FRAMEWORK_AGENT must survive an unrelated + FRAMEWORK_AGENT -> EXPLORE transition, since exit_normal_explore (the hint's + only consumer) only ever checks it once the phase is EXPLORE. + """ + import hyperloom.orchestrator.phases.machine_state as ps + + coord.shared_state.phase = "FRAMEWORK_AGENT" + coord.shared_state.pending_escalate_hint = "skip_to_kernel" + monkeypatch.setattr(ps, "compute_next_phase", lambda *a, **k: ("EXPLORE", "framework_phase_done", {})) + + async def _entered(*, from_phase, to_phase): + return None + + monkeypatch.setattr(coord.phase_machine, "_on_phase_entered", _entered) + await coord._advance_phase_if_needed() + assert (coord.shared_state.phase or "").upper() == "EXPLORE" + assert coord.shared_state.pending_escalate_hint == "skip_to_kernel" + + +@pytest.mark.asyncio +async def test_advance_phase_hint_discarded_when_not_headed_to_explore(coord: Coordinator, monkeypatch) -> None: + """A pending hint is genuinely stale once the transition target isn't + EXPLORE -- it can never reach exit_normal_explore's check again -- so this + is the one case the unrelated-transition cleanup should still clear it. + """ + import hyperloom.orchestrator.phases.machine_state as ps + + coord.shared_state.phase = "FRAMEWORK_AGENT" + coord.shared_state.pending_escalate_hint = "skip_to_kernel" + monkeypatch.setattr(ps, "compute_next_phase", lambda *a, **k: ("SWEEP", "some_other_reason", {})) + + async def _entered(*, from_phase, to_phase): + return None + + monkeypatch.setattr(coord.phase_machine, "_on_phase_entered", _entered) + await coord._advance_phase_if_needed() + assert (coord.shared_state.phase or "").upper() == "SWEEP" + assert coord.shared_state.pending_escalate_hint == "" + assert coord.shared_state.last_consumed_escalate_hint == "skip_to_kernel" + + # -- _materialize_approved_proposal ----------------------------------------- def _pending(action_name: str, payload: dict, msg_id: str = "prop-1"): from hyperloom.orchestrator.loop.coordinator import PendingProposal diff --git a/src/hyperloom/inference_optimizer/tests/test_phase_state_machine.py b/src/hyperloom/inference_optimizer/tests/test_phase_state_machine.py index 5b833e28c8..3f27135796 100644 --- a/src/hyperloom/inference_optimizer/tests/test_phase_state_machine.py +++ b/src/hyperloom/inference_optimizer/tests/test_phase_state_machine.py @@ -257,6 +257,9 @@ def test_compute_next_phase_no_kernel_skips_kernel_phase(): stop_reason="", pending_escalate_hint="skip_to_kernel", explore_search={}, + # At least one specialist round this cycle, required for skip_to_kernel + # to fire at all (see test_exit_normal_explore_skip_to_kernel_*). + specialist_rounds=[{"proposals_total": 1, "proposals_kept": 0}], optimization_stack=[{"action": "explore"}], ) out = phase_state.compute_next_phase(state, kernel_enabled=False) @@ -267,6 +270,57 @@ def test_compute_next_phase_no_kernel_skips_kernel_phase(): assert evidence.get("passed_through_reason") == "plateau_explore" +def test_exit_normal_explore_skip_to_kernel_requires_a_tested_round(): + """A skip_to_kernel hint must not end EXPLORE with zero validated work. + + Reproduces the cumulative_gain_validated=0.00% session: the hint arrived + before EXPLORE ever dispatched a specialist round this cycle, and must not + be honored until one actually has. + """ + state = SimpleNamespace( + phase="EXPLORE", + phase_started_unix=1_000_000.0, + max_minutes=0, + phase_budget_pct={}, + pending_escalate_hint="skip_to_kernel", + explore_search={}, + specialist_rounds=[], + macro_cycle=0, + optimization_stack=[{"action": "explore"}], + _now_unix=lambda: 1_000_000.0, + ) + out = phase_state.exit_normal_explore( + state, + force_exit_hours_remaining=0.0, + force_exit_budget_pct=0.0, + ) + assert out is None + + +def test_exit_normal_explore_skip_to_kernel_fires_once_a_round_ran(): + state = SimpleNamespace( + phase="EXPLORE", + phase_started_unix=1_000_000.0, + max_minutes=0, + phase_budget_pct={}, + pending_escalate_hint="skip_to_kernel", + explore_search={}, + specialist_rounds=[{"proposals_total": 1, "proposals_kept": 0}], + macro_cycle=0, + optimization_stack=[{"action": "explore"}], + _now_unix=lambda: 1_000_000.0, + ) + out = phase_state.exit_normal_explore( + state, + force_exit_hours_remaining=0.0, + force_exit_budget_pct=0.0, + ) + assert out is not None + reason, evidence = out + assert reason == "plateau_explore" + assert evidence.get("hint") == "skip_to_kernel" + + def test_compute_next_phase_terminal_overrides_phase(): state = SimpleNamespace( phase="EXPLORE", diff --git a/src/hyperloom/inference_optimizer/tests/test_policy_source_file_from_trace.py b/src/hyperloom/inference_optimizer/tests/test_policy_source_file_from_trace.py index 5a6396fadb..081d99fb70 100644 --- a/src/hyperloom/inference_optimizer/tests/test_policy_source_file_from_trace.py +++ b/src/hyperloom/inference_optimizer/tests/test_policy_source_file_from_trace.py @@ -113,3 +113,31 @@ def test_absolute_path_outside_every_scope_is_still_denied(tmp_path, monkeypatch with pytest.raises(PolicyDenied) as exc: _gate(tmp_path).validate_intent("orchestration", _dispatch_intent("/etc/passwd")) assert exc.value.rule == "source_file_outside_trusted_scope" + + +# Placeholder/vendor-label forms TraceLens can leave in source_file instead of +# an empty string (test_source_resolution_guards.py _SENTINELS covers the +# producer side; this covers the gate degrading them to an omitted field +# rather than denying the whole delegate as a bogus path). +_ABSENT_SENTINELS = ( + "Not found", + "N/A", + "none", + "unknown", + "TBD", + "", + "AITER (vendor)", + "Triton (vendor)", +) + + +@pytest.mark.parametrize("sentinel", _ABSENT_SENTINELS) +def test_absent_value_sentinel_is_accepted_not_denied(tmp_path, monkeypatch, sentinel): + """A known placeholder degrades the delegate gracefully instead of denying it.""" + _framework_tree(tmp_path, monkeypatch) + _gate(tmp_path).validate_intent("orchestration", _dispatch_intent(sentinel)) + + +def test_sentinel_match_is_case_insensitive(tmp_path, monkeypatch): + _framework_tree(tmp_path, monkeypatch) + _gate(tmp_path).validate_intent("orchestration", _dispatch_intent("NOT FOUND")) diff --git a/src/hyperloom/orchestrator/phases/machine.py b/src/hyperloom/orchestrator/phases/machine.py index 6a68623ec9..ac2897ad2a 100644 --- a/src/hyperloom/orchestrator/phases/machine.py +++ b/src/hyperloom/orchestrator/phases/machine.py @@ -224,12 +224,22 @@ async def _advance_phase_if_needed(self) -> None: # Consume escalate hint after a hint-driven transition. if isinstance(evidence, dict) and (evidence.get("evidence") == "llm_escalation" or "hint" in evidence): state.consume_pending_escalate_hint() - elif getattr(state, "pending_escalate_hint", ""): - # A phase change fired for a reason unrelated to the hint while - # one was still pending (e.g. set by a different phase's agent - # turn and never claimed) — clear it so it isn't inherited by the - # next phase's own exit checks as if it earned that leverage. - state.consume_pending_escalate_hint() + elif getattr(state, "pending_escalate_hint", "") and target != _phase_state.PHASE_EXPLORE: + # A phase change fired for a reason unrelated to the hint while one + # was still pending. exit_normal_explore is the hint's only + # consumer, so a hint riding toward EXPLORE (target == PHASE_EXPLORE) + # is still legitimately in flight and must survive this transition + # to be checked there — only discard when the transition is NOT + # headed to EXPLORE (including EXPLORE itself force-exiting past + # the hint via IR-6), since it can then never reach that check. + discarded_hint = state.consume_pending_escalate_hint() + log.info( + "phase_machine: discarded stale pending_escalate_hint=%r on unrelated transition %s -> %s (reason=%s)", + discarded_hint, + prior, + target, + reason, + ) # Terminal transition (target=CLOSE): mirror the stop_reason onto state. if ( target == _phase_state.PHASE_CLOSE diff --git a/src/hyperloom/orchestrator/phases/machine_state.py b/src/hyperloom/orchestrator/phases/machine_state.py index ecc647bff2..e141640095 100644 --- a/src/hyperloom/orchestrator/phases/machine_state.py +++ b/src/hyperloom/orchestrator/phases/machine_state.py @@ -2029,10 +2029,18 @@ def exit_normal_explore( hint = _pending_escalate_hint(state) if hint == ESCALATE_HINT_SKIP_TO_KERNEL: - return "plateau_explore", { - "evidence": "llm_escalation", - "hint": hint, - } + # A skip_to_kernel hint that arrives before EXPLORE has actually + # dispatched a specialist round this cycle must not end the phase + # with zero validated work (the direct cause of a prior + # cumulative_gain_validated=0.00% session) -- leave it pending so it + # fires once a round has actually run, instead of honoring it + # unconditionally ahead of compute_plateau_explore's own evidence + # requirement. + if _rows_for_current_cycle(getattr(state, "specialist_rounds", None) or [], state): + return "plateau_explore", { + "evidence": "llm_escalation", + "hint": hint, + } if hint == ESCALATE_HINT_SKIP_TO_SWEEP: return "explore_no_more_leverage", { "evidence": "explore_no_more_leverage", From 53126cd91afd25514beadb472c4d5b60232d04e7 Mon Sep 17 00:00:00 2001 From: Matt Elliott Date: Fri, 14 Aug 2026 19:13:22 +0000 Subject: [PATCH 3/6] fix(robustness): route local_server_unreachable to recover, not server_lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review item #3: local_health.py still constructed suggestion="delegate(server_lifecycle)..." for local_server_unreachable (HIGH) and log_error_pattern (HIGH), even though action_ladder.py already routes local_server_unreachable to a real delegate(recover, force_gpu_cleanup=True). That stale string reaches the orchestration prompt via alert detail.suggestion, telling the model to reach for a non-dispatchable action. Point both suggestions at the real remedy. Review item #4: action_ladder.py's local_server_unreachable idempotency key only carried the tick. _server_unreachable emits one symptom per unreachable probe target and marks all of them HIGH together, so two dead targets in one tick produced two delegates with the same idempotency_key — the first creates the recovery task, the second comes back as a duplicate-idempotency PolicyDenied that pollutes repeated_policy_denied tracking. Disambiguate the key with a short hash of the target URL. Also updates action_ladder.py's stale module docstring (review item #9), which still listed delegate(recover) as gpu_memory_leaked-only. Adds direct test coverage for the local_server_unreachable -> recover mapping (review item #11), none of which existed before. --- .../robustness/decision/action_ladder.py | 22 ++++-- .../agents/robustness/signals/local_health.py | 4 +- .../tests/test_decision_action_ladder.py | 79 +++++++++++++++++++ .../tests/test_signals_local_health.py | 31 ++++++++ 4 files changed, 129 insertions(+), 7 deletions(-) diff --git a/src/hyperloom/agents/robustness/decision/action_ladder.py b/src/hyperloom/agents/robustness/decision/action_ladder.py index 8875893d1e..0b815c735d 100644 --- a/src/hyperloom/agents/robustness/decision/action_ladder.py +++ b/src/hyperloom/agents/robustness/decision/action_ladder.py @@ -9,10 +9,11 @@ 2. **diagnose** (medium) — ``alert(severity="medium")`` carrying evidence. 3. **recommend** (high) — ``alert(severity="high")`` plus, for some symptoms, a symptom-specific remediation intent: ``kill_task`` (stale_lease), - ``delegate(recover)`` (gpu_memory_leaked), ``delegate(report)`` - (``deadline_*`` wind-down and ``recover_unsuccessful`` finalization), or - ``prune_branch`` (stuck / no-lever families in ``_PRUNE_SYMPTOMS``). Every - other HIGH symptom is strategic: the alert alone, and Orchestration decides. + ``delegate(recover)`` (gpu_memory_leaked, local_server_unreachable), + ``delegate(report)`` (``deadline_*`` wind-down and ``recover_unsuccessful`` + finalization), or ``prune_branch`` (stuck / no-lever families in + ``_PRUNE_SYMPTOMS``). Every other HIGH symptom is strategic: the alert + alone, and Orchestration decides. Strategic suggestions ride the alert ``detail.suggestion`` field. A per-key cooldown (``Symptom.dedup_key`` × ``cooldown_ticks``) prevents inbox flooding. @@ -21,6 +22,7 @@ from __future__ import annotations +import hashlib import logging from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any, Iterable @@ -313,6 +315,14 @@ def _recommend(self, sym: Symptom) -> list[Intent]: # as unknown_action.) if sym.name == "local_server_unreachable": evidence = dict(sym.evidence) if isinstance(sym.evidence, dict) else {} + # `_server_unreachable` emits one symptom per unreachable probe + # target and marks all of them HIGH together, so a tick-only key + # would collide across targets: the first delegate creates the + # task and the rest come back as duplicate-idempotency + # PolicyDenied, which incorrectly books a working recovery as a + # repeated policy denial. Disambiguate with the target itself. + target = str(sym.subject.get("url") or evidence.get("url") or "unknown") + target_key = hashlib.sha1(target.encode("utf-8")).hexdigest()[:8] intents.append( build_delegate( action_name="recover", @@ -321,7 +331,9 @@ def _recommend(self, sym: Symptom) -> list[Intent]: "force_gpu_cleanup": True, "evidence": evidence, }, - idempotency_key=(f"recover-server-unreachable-tick-{self._last_tick_index}"), + idempotency_key=( + f"recover-server-unreachable-tick-{self._last_tick_index}-{target_key}" + ), ) ) return intents diff --git a/src/hyperloom/agents/robustness/signals/local_health.py b/src/hyperloom/agents/robustness/signals/local_health.py index a4e805c44b..fdf28c665d 100644 --- a/src/hyperloom/agents/robustness/signals/local_health.py +++ b/src/hyperloom/agents/robustness/signals/local_health.py @@ -202,7 +202,7 @@ def _server_unreachable(data: SourceData, cfg: LocalHealthConfig) -> list[Sympto subject={"url": url}, source="local", suggestion=( - "delegate(server_lifecycle) to restart the inference server" + "delegate(recover, force_gpu_cleanup=True) to restart the inference server" if severity is SymptomSeverity.HIGH else "monitor; alert orchestration if it persists" ), @@ -342,7 +342,7 @@ def _log_error_symptoms(data: SourceData) -> list[Symptom]: subject={"pattern": pattern}, source="local", suggestion=( - "delegate(server_lifecycle) or escalate strategy" + "delegate(recover, force_gpu_cleanup=True) or escalate strategy" if severity is SymptomSeverity.HIGH else "review log evidence with RCA before further action" ), diff --git a/src/hyperloom/agents/robustness/tests/test_decision_action_ladder.py b/src/hyperloom/agents/robustness/tests/test_decision_action_ladder.py index 54d4dc0d9c..42bf324190 100644 --- a/src/hyperloom/agents/robustness/tests/test_decision_action_ladder.py +++ b/src/hyperloom/agents/robustness/tests/test_decision_action_ladder.py @@ -1216,3 +1216,82 @@ async def test_gpu_memory_leaked_idempotency_key_advances_with_tick(): second_delegate = next(i for i in second.intents if i.type is IntentType.DELEGATE) assert first_delegate.payload["idempotency_key"] == "recover-gpu-leak-tick-0" assert second_delegate.payload["idempotency_key"] == "recover-gpu-leak-tick-5" + + +# local_server_unreachable -> delegate(recover) + + +def _server_unreachable_symptom(url: str, evidence: dict | None = None) -> Symptom: + return Symptom( + name="local_server_unreachable", + severity=SymptomSeverity.HIGH, + summary=f"local server probe {url} status=down", + evidence=evidence or {"url": url}, + subject={"url": url}, + source="local", + suggestion="delegate(recover, force_gpu_cleanup=True) to restart the inference server", + ) + + +async def test_local_server_unreachable_emits_alert_and_delegate_recover(): + """``local_server_unreachable`` must route to the real ``recover`` action, + not the non-dispatchable ``server_lifecycle`` its own suggestion text names. + """ + ladder = ActionLadder() + out = await ladder.decide( + [_server_unreachable_symptom("http://127.0.0.1:8000/health")], + tick_index=4, + now_unix=1.0, + ) + types = [i.type for i in out.intents] + assert types == [IntentType.ALERT, IntentType.DELEGATE] + + delegate = out.intents[1] + assert delegate.payload["action_name"] == "recover" + assert delegate.payload["params"]["force_gpu_cleanup"] is True + assert delegate.payload["params"]["reason"] == "local_server_unreachable" + assert delegate.payload["idempotency_key"].startswith("recover-server-unreachable-tick-4-") + + +async def test_local_server_unreachable_idempotency_key_disambiguates_targets(): + """Two unreachable targets in the same tick must not collide on idempotency_key, + or the second delegate comes back as a duplicate-idempotency PolicyDenied + that pollutes repeated_policy_denied tracking instead of just recovering. + """ + ladder = ActionLadder() + out = await ladder.decide( + [ + _server_unreachable_symptom("http://127.0.0.1:8000/health"), + _server_unreachable_symptom("http://127.0.0.1:8001/health"), + ], + tick_index=4, + now_unix=1.0, + ) + delegate_keys = [i.payload["idempotency_key"] for i in out.intents if i.type is IntentType.DELEGATE] + assert len(delegate_keys) == 2 + assert len(set(delegate_keys)) == 2 + assert all(key.startswith("recover-server-unreachable-tick-4-") for key in delegate_keys) + + +async def test_local_server_unreachable_idempotency_key_stable_for_same_target(): + """The same target re-firing (e.g. a later tick after cooldown) must derive + the same per-target suffix, since that's what makes the disambiguator + deterministic rather than a source of new spurious duplicates. + """ + ladder = ActionLadder() + first = await ladder.decide( + [_server_unreachable_symptom("http://127.0.0.1:8000/health")], + tick_index=4, + now_unix=1.0, + ) + second = await ladder.decide( + [_server_unreachable_symptom("http://127.0.0.1:8000/health")], + tick_index=9, + now_unix=2.0, + ) + first_key = next(i.payload["idempotency_key"] for i in first.intents if i.type is IntentType.DELEGATE) + second_key = next(i.payload["idempotency_key"] for i in second.intents if i.type is IntentType.DELEGATE) + first_suffix = first_key.rsplit("-", 1)[-1] + second_suffix = second_key.rsplit("-", 1)[-1] + assert first_suffix == second_suffix + assert first_key != second_key diff --git a/src/hyperloom/agents/robustness/tests/test_signals_local_health.py b/src/hyperloom/agents/robustness/tests/test_signals_local_health.py index ca88b34a4e..58e01d0039 100644 --- a/src/hyperloom/agents/robustness/tests/test_signals_local_health.py +++ b/src/hyperloom/agents/robustness/tests/test_signals_local_health.py @@ -206,6 +206,29 @@ def test_a_seen_server_is_recorded_in_the_evidence(): assert matched and matched[0].evidence["server_process_seen"] is True +def test_all_targets_down_suggests_the_real_dispatchable_action(): + """The HIGH suggestion must not point at ``server_lifecycle`` -- PolicyGate + rejects it as unknown_action; action_ladder routes this symptom to a real + ``delegate(recover, force_gpu_cleanup=True)``, and the suggestion text + reaching the orchestration prompt must say so instead. + + A live server process is included so the idle-server-detection added + upstream (a refused port with no server and no benchmark client behind it + is treated as an expected idle stretch, not a fault) doesn't suppress the + symptom this test exists to check. + """ + data = SourceData( + local_processes=_live_server(), + local_server_health=[ + {"url": "http://localhost:30000", "reachable": False, "status": "error"}, + ], + ) + out = evaluate_local_health_signals(_ctx(), data) + matched = next(s for s in out if s.name == "local_server_unreachable") + assert "server_lifecycle" not in matched.suggestion + assert "delegate(recover" in matched.suggestion + + def test_no_unreachable_targets_is_silent(): data = SourceData( local_server_health=[ @@ -230,6 +253,14 @@ def test_runtimeerror_pattern_is_medium_severity(): assert matched and matched[0].severity is SymptomSeverity.MEDIUM +def test_oom_pattern_suggests_the_real_dispatchable_action(): + data = SourceData(local_log_errors=[{"pattern": "CUDA out of memory", "line": "torch ... CUDA out of memory ..."}]) + out = evaluate_local_health_signals(_ctx(), data) + matched = next(s for s in out if s.name == "log_error_pattern") + assert "server_lifecycle" not in matched.suggestion + assert "delegate(recover" in matched.suggestion + + def test_log_error_groups_samples_by_pattern(): data = SourceData(local_log_errors=[{"pattern": "RuntimeError", "line": f"err {i}"} for i in range(5)]) out = evaluate_local_health_signals(_ctx(), data) From fb243e44580c330ab5b0803cf4cf9afc986f85e4 Mon Sep 17 00:00:00 2001 From: Matt Elliott Date: Fri, 14 Aug 2026 19:13:53 +0000 Subject: [PATCH 4/6] fix(orchestrator): distinguish discarded escalate hints from consumed ones Review item #7: machine.py's unrelated-transition cleanup dropped a pending escalate hint through consume_pending_escalate_hint(), which records the hint in last_consumed_escalate_hint -- an audit field documented as meaning "this hint drove a transition." A hint that was thrown away without acting on it is a different event; recording it as consumed told the breakdown the opposite of what happened. Add a sibling discard_pending_escalate_hint() that records into new last_discarded_escalate_hint(_ts) fields instead, wire machine.py's discard branch to it, and register the new fields in both LLM-write-blocked state allowlists (gate.py, robustness envelope.py) so an LLM-authored patch can't forge them. reset_per_cycle_plateau_state() also discarded a hint directly (bypassing both audit paths); route it through the new method too since that clear is semantically the same event. Also fixes review item #9's remaining stale comments: pending_escalate_hint's field comment ("cleared once acted on"), consume_pending_escalate_hint()'s own docstring, and reset_per_cycle_plateau_state()'s docstring (it resets pending_escalate_hint too, which isn't "plateau and dispatch state"). Review item #6: gate.py's sentinel pass-through returned silently on a match. Add a log line so a sentinel-driven accept is visible in logs instead of looking identical to a normal accept. Adds direct test coverage (review item #11) distinguishing the discard and consume paths by their respective audit fields, and covering the sentinel log line, none of which existed before. Co-Authored-By: Claude Sonnet 5 --- .../agents/robustness/role/envelope.py | 2 ++ .../test_coordinator_async_batch2_unit.py | 33 ++++++++++++++++++ .../test_policy_source_file_from_trace.py | 13 +++++++ src/hyperloom/orchestrator/phases/machine.py | 4 +-- src/hyperloom/orchestrator/policy/gate.py | 10 ++++++ .../state/_shared_state/explore_state.py | 14 +++++--- .../orchestrator/state/shared_state.py | 34 +++++++++++++++++-- 7 files changed, 100 insertions(+), 10 deletions(-) diff --git a/src/hyperloom/agents/robustness/role/envelope.py b/src/hyperloom/agents/robustness/role/envelope.py index ec205187e7..c65e6d0e0d 100644 --- a/src/hyperloom/agents/robustness/role/envelope.py +++ b/src/hyperloom/agents/robustness/role/envelope.py @@ -174,6 +174,8 @@ class IntentType(str, Enum): "pending_escalate_hint", "last_consumed_escalate_hint", "last_consumed_escalate_hint_ts", + "last_discarded_escalate_hint", + "last_discarded_escalate_hint_ts", "plateau_overrides", # CLOSE phase sequencer flag. "close_sequence_done", diff --git a/src/hyperloom/inference_optimizer/tests/test_coordinator_async_batch2_unit.py b/src/hyperloom/inference_optimizer/tests/test_coordinator_async_batch2_unit.py index c1527cf849..d2478ed826 100644 --- a/src/hyperloom/inference_optimizer/tests/test_coordinator_async_batch2_unit.py +++ b/src/hyperloom/inference_optimizer/tests/test_coordinator_async_batch2_unit.py @@ -2044,6 +2044,10 @@ async def test_advance_phase_hint_discarded_when_not_headed_to_explore(coord: Co """A pending hint is genuinely stale once the transition target isn't EXPLORE -- it can never reach exit_normal_explore's check again -- so this is the one case the unrelated-transition cleanup should still clear it. + + A discard is not a consumption: it must land in last_discarded_escalate_hint, + not last_consumed_escalate_hint, which specifically means "this hint drove + a transition" and this one never did. """ import hyperloom.orchestrator.phases.machine_state as ps @@ -2058,7 +2062,36 @@ async def _entered(*, from_phase, to_phase): await coord._advance_phase_if_needed() assert (coord.shared_state.phase or "").upper() == "SWEEP" assert coord.shared_state.pending_escalate_hint == "" + assert coord.shared_state.last_discarded_escalate_hint == "skip_to_kernel" + assert coord.shared_state.last_discarded_escalate_hint_ts + assert coord.shared_state.last_consumed_escalate_hint == "" + + +@pytest.mark.asyncio +async def test_advance_phase_hint_consumed_when_it_drove_the_transition(coord: Coordinator, monkeypatch) -> None: + """The complementary case: a hint-driven transition must record consumption, + not a discard, so the two are distinguishable in the breakdown. + """ + import hyperloom.orchestrator.phases.machine_state as ps + + coord.shared_state.phase = "EXPLORE" + coord.shared_state.pending_escalate_hint = "skip_to_kernel" + monkeypatch.setattr( + ps, + "compute_next_phase", + lambda *a, **k: ("KERNEL_AGENT", "skip_to_kernel", {"hint": "skip_to_kernel"}), + ) + + async def _entered(*, from_phase, to_phase): + return None + + monkeypatch.setattr(coord.phase_machine, "_on_phase_entered", _entered) + await coord._advance_phase_if_needed() + assert (coord.shared_state.phase or "").upper() == "KERNEL_AGENT" + assert coord.shared_state.pending_escalate_hint == "" assert coord.shared_state.last_consumed_escalate_hint == "skip_to_kernel" + assert coord.shared_state.last_consumed_escalate_hint_ts + assert coord.shared_state.last_discarded_escalate_hint == "" # -- _materialize_approved_proposal ----------------------------------------- diff --git a/src/hyperloom/inference_optimizer/tests/test_policy_source_file_from_trace.py b/src/hyperloom/inference_optimizer/tests/test_policy_source_file_from_trace.py index 081d99fb70..bd57ca6f92 100644 --- a/src/hyperloom/inference_optimizer/tests/test_policy_source_file_from_trace.py +++ b/src/hyperloom/inference_optimizer/tests/test_policy_source_file_from_trace.py @@ -141,3 +141,16 @@ def test_absent_value_sentinel_is_accepted_not_denied(tmp_path, monkeypatch, sen def test_sentinel_match_is_case_insensitive(tmp_path, monkeypatch): _framework_tree(tmp_path, monkeypatch) _gate(tmp_path).validate_intent("orchestration", _dispatch_intent("NOT FOUND")) + + +def test_sentinel_pass_through_is_logged(tmp_path, monkeypatch, caplog): + """A sentinel-driven accept must be visible in logs, not indistinguishable + from a normal accept -- previously this branch returned silently. + """ + _framework_tree(tmp_path, monkeypatch) + with caplog.at_level("INFO", logger="hyperloom.orchestrator.policy.gate"): + _gate(tmp_path).validate_intent("orchestration", _dispatch_intent("Not found")) + assert any( + "absent-value sentinel" in record.message and "Not found" in record.message + for record in caplog.records + ) diff --git a/src/hyperloom/orchestrator/phases/machine.py b/src/hyperloom/orchestrator/phases/machine.py index ac2897ad2a..87cdc8dc4f 100644 --- a/src/hyperloom/orchestrator/phases/machine.py +++ b/src/hyperloom/orchestrator/phases/machine.py @@ -224,7 +224,7 @@ async def _advance_phase_if_needed(self) -> None: # Consume escalate hint after a hint-driven transition. if isinstance(evidence, dict) and (evidence.get("evidence") == "llm_escalation" or "hint" in evidence): state.consume_pending_escalate_hint() - elif getattr(state, "pending_escalate_hint", "") and target != _phase_state.PHASE_EXPLORE: + elif state.pending_escalate_hint and target != _phase_state.PHASE_EXPLORE: # A phase change fired for a reason unrelated to the hint while one # was still pending. exit_normal_explore is the hint's only # consumer, so a hint riding toward EXPLORE (target == PHASE_EXPLORE) @@ -232,7 +232,7 @@ async def _advance_phase_if_needed(self) -> None: # to be checked there — only discard when the transition is NOT # headed to EXPLORE (including EXPLORE itself force-exiting past # the hint via IR-6), since it can then never reach that check. - discarded_hint = state.consume_pending_escalate_hint() + discarded_hint = state.discard_pending_escalate_hint() log.info( "phase_machine: discarded stale pending_escalate_hint=%r on unrelated transition %s -> %s (reason=%s)", discarded_hint, diff --git a/src/hyperloom/orchestrator/policy/gate.py b/src/hyperloom/orchestrator/policy/gate.py index 96932c5aee..e13c88cf1b 100644 --- a/src/hyperloom/orchestrator/policy/gate.py +++ b/src/hyperloom/orchestrator/policy/gate.py @@ -650,6 +650,8 @@ def _source_file_candidates(value: str) -> tuple[str, ...]: "pending_escalate_hint", "last_consumed_escalate_hint", "last_consumed_escalate_hint_ts", + "last_discarded_escalate_hint", + "last_discarded_escalate_hint_ts", "plateau_overrides", # CLOSE-phase sequencer flag; LLM must not toggle it. "close_sequence_done", @@ -2369,6 +2371,14 @@ def visit(node: Any, path_keys: tuple[str, ...]) -> None: key = path_keys[-1] if path_keys else "" if key in SOURCE_LIKE_FIELDS: if node.strip().lower() in _SOURCE_FILE_ABSENT_SENTINELS: + log.info( + "role=%r %s payload field %r=%r is an absent-value " + "sentinel; treating as omitted and admitting the delegate", + role.name, + intent_type.value, + key, + node, + ) return if any( self._path_in_source_allowlist(c) or self._path_under_session(c) diff --git a/src/hyperloom/orchestrator/state/_shared_state/explore_state.py b/src/hyperloom/orchestrator/state/_shared_state/explore_state.py index 87fa6590df..d565c30f22 100644 --- a/src/hyperloom/orchestrator/state/_shared_state/explore_state.py +++ b/src/hyperloom/orchestrator/state/_shared_state/explore_state.py @@ -397,7 +397,14 @@ def reset_explore_plateau_proxy(self) -> None: self.params_no_promote_streak = 0 def reset_per_cycle_plateau_state(self) -> None: - """Reset transient plateau and dispatch state for a macro-cycle.""" + """Reset transient plateau and dispatch state for a macro-cycle, plus any stale escalate hint. + + ``pending_escalate_hint`` isn't plateau or dispatch state, but it is + reset here too: a hint set during the prior macro-cycle (e.g. by + kernel_agent completion) and never claimed by the transition it + caused must not survive into the next cycle's phases as if they'd + earned it. + """ self.params_no_promote_streak = 0 self.explore_specialist_dispatched_count = 0 self.framework_agent_phase_done = False @@ -411,10 +418,7 @@ def reset_per_cycle_plateau_state(self) -> None: self.rounds_since_last_keep = {} self.last_sweep = {} self.last_conc_sweep = {} - # A hint set during the prior macro-cycle (e.g. by kernel_agent - # completion) and never claimed by the transition it caused must not - # survive into the next cycle's phases as if they'd earned it. - self.pending_escalate_hint = "" + self.discard_pending_escalate_hint() def note_explore_outcome(self, *, promoted: bool) -> None: """Update the plateau proxy after one explore task (KEEP resets, no-promote increments). diff --git a/src/hyperloom/orchestrator/state/shared_state.py b/src/hyperloom/orchestrator/state/shared_state.py index 6d945a34b0..74333a67f8 100644 --- a/src/hyperloom/orchestrator/state/shared_state.py +++ b/src/hyperloom/orchestrator/state/shared_state.py @@ -857,11 +857,14 @@ class SharedState(_RenderMixin, _ExploreStateMixin): # session start. The dataclass default is a placeholder for tests/direct # construction; the CLI/manifest default is whole-machine GPU detection. gpu_specialist_capacity: int = 0 - # escalate_strategy_change carry-over: Coordinator writes validated next_action_hint here for compute_next_phase, then clears it once acted on. + # escalate_strategy_change carry-over: Coordinator writes validated next_action_hint here for compute_next_phase, then clears it either by consuming it (drove a transition) or discarding it (an unrelated transition fired while it was pending). pending_escalate_hint: str = "" - # last cleared escalate hint (audit only) for the breakdown. + # last hint that actually drove a phase transition (audit only) for the breakdown. last_consumed_escalate_hint: str = "" last_consumed_escalate_hint_ts: str = "" + # last hint thrown away by an unrelated transition, never acted on (audit only) for the breakdown. Distinct from last_consumed_escalate_hint: that field means "this drove a transition", which a discarded hint never did. + last_discarded_escalate_hint: str = "" + last_discarded_escalate_hint_ts: str = "" # per-phase plateau threshold overrides locked at session start (CLI flags); empty => library defaults. plateau_overrides: dict[str, Any] = field(default_factory=dict) # E2E integrate bookkeeping keyed by kernel_id+patch_path+args; prevents re-validating the same patch after NEEDS_REVIEW/REVERT. @@ -1807,7 +1810,13 @@ def set_pending_escalate_hint(self, hint: str) -> str: return text def consume_pending_escalate_hint(self) -> str: - """Pop the pending hint (recording consumption in audit fields) so the next tick doesn't re-trigger; returns cleared hint. + """Pop the pending hint because it drove a phase transition; returns cleared hint. + + Records the hint into ``last_consumed_escalate_hint`` — an audit field + that specifically means "this hint drove a transition". A hint that + was thrown away without acting on it is a different event and must go + through :meth:`discard_pending_escalate_hint` instead, or a discard + would misreport itself as a consumption in the breakdown. Returns: str: The consumed hint (``""`` when none was pending). @@ -1820,6 +1829,25 @@ def consume_pending_escalate_hint(self) -> str: self.last_consumed_escalate_hint_ts = _now_iso() return hint + def discard_pending_escalate_hint(self) -> str: + """Pop the pending hint because an unrelated transition fired without acting on it; returns cleared hint. + + Records the hint into ``last_discarded_escalate_hint`` rather than + ``last_consumed_escalate_hint`` — the hint never drove anything, so + recording it as consumed would tell the breakdown the opposite of + what happened. + + Returns: + str: The discarded hint (``""`` when none was pending). + """ + hint = (self.pending_escalate_hint or "").strip() + if not hint: + return "" + self.pending_escalate_hint = "" + self.last_discarded_escalate_hint = hint + self.last_discarded_escalate_hint_ts = _now_iso() + return hint + def enablement_close_guard_active(self) -> bool: """True while a not-yet-enabled run must be protected from premature close. From a613b99202cdecb0dbbb1cb37cef19622b6b78f1 Mon Sep 17 00:00:00 2001 From: Matt Elliott Date: Fri, 14 Aug 2026 19:14:27 +0000 Subject: [PATCH 5/6] refactor(orchestrator): drop no-op getattr defaults, document error_class vocabulary Review item #10: getattr(denied, "rule", ...) at both call sites default a PolicyDenied.rule attribute that __init__ always assigns (gate.py:119) -- the getattr default can never fire. Simplify to direct attribute access; the "or 'denied'" still does real work for the actual None case. Review item #8: "policy_{rule}" error_class values (e.g. policy_source_file_outside_trusted_scope) don't match any of the field's existing exact-match buckets (crash/oom/hang/detokenizer_stall), and nothing documents that this is an open, not closed, vocabulary. Checked both consumers: writeback._pitfall_severity_for correctly excludes policy_* from crash-severity (a policy denial isn't a runtime crash), and explore._extract_gaps_from_attempts groups it into its own gap key just from the raw string -- neither needs new handling. Document the field's known producers/consumers on SubAgentResult.error_class instead, so a future prefix family is discoverable from one place. --- .../orchestrator/loop/sub_agent_runner.py | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/hyperloom/orchestrator/loop/sub_agent_runner.py b/src/hyperloom/orchestrator/loop/sub_agent_runner.py index 6419e66a83..687d305e85 100644 --- a/src/hyperloom/orchestrator/loop/sub_agent_runner.py +++ b/src/hyperloom/orchestrator/loop/sub_agent_runner.py @@ -67,10 +67,28 @@ class SubAgentResult: state (str): Terminal state — ``"succeeded"`` / ``"failed"``. result (dict): Executor result payload (empty on failure). error (str | None): Error string when the task failed, else None. - error_class (str): Machine-readable failure category (e.g. - ``"policy_source_file_outside_trusted_scope"``) for a - ``PolicyDenied`` dispatch rejection; empty for other failures - (executors set their own ``error_class`` inside ``result``). + error_class (str): Machine-readable failure category. Not a closed + enum — each producer mints its own values, so a new prefix + family here is discoverable only via its consumers, not via a + central registry: + + * ``"policy_{rule}"`` (e.g. + ``"policy_source_file_outside_trusted_scope"``): a + ``PolicyDenied`` dispatch rejection, keyed on + :attr:`PolicyDenied.rule <..policy.gate.PolicyDenied.rule>`. + Falls through any exact-match bucket below by design — a + policy denial isn't a runtime crash/oom/hang, so + :meth:`writeback._pitfall_severity_for` correctly excludes it + from ``SEVERITY_CRASH``. Still lands in the gap ledger as its + own ``(action, error_class)`` key + (:meth:`explore._extract_gaps_from_attempts`), which is enough + to group repeat denials without a dedicated bucket. + * ``"crash"`` / ``"oom"`` / ``"hang"`` / ``"detokenizer_stall"``: + exact-matched by :meth:`writeback._pitfall_severity_for` to + classify a failure as crash-severity for the KB. + * Anything else (including empty): executors set their own + ``error_class`` inside ``result``, or leave it unset, in which + case the gap ledger buckets it as ``"unknown_error"``. """ task_id: str @@ -236,14 +254,14 @@ async def run_task( "cancelled", evidence={ "reason": "policy_denied", - "rule": getattr(denied, "rule", None), + "rule": denied.rule, "error": str(denied), }, context="dispatch_policy_denied", ) if prebound_lease is not None: await self.locks.release(prebound_lease) - rule = getattr(denied, "rule", "") or "denied" + rule = denied.rule or "denied" return SubAgentResult( task_id=task.task_id, state="failed", From c064fa63fa0025c5c7870fd118c219215e4c8071 Mon Sep 17 00:00:00 2001 From: Matt Elliott Date: Fri, 14 Aug 2026 22:32:51 +0000 Subject: [PATCH 6/6] fix(orchestrator): thread error_class through all three run_task failure exits Review item #5: run_task has three return SubAgentResult(state="failed", result={}, ...) sites; only the PolicyDenied one set error_class. The no_executor and executor-exception exits still returned an empty result with no class, so they kept collapsing into unknown_error at the gap key (explore._extract_gaps_from_attempts) -- the same generic bucket this item was meant to get callers out of. no_executor now sets error_class="no_executor" (matches the transition evidence's own "reason" value at the same site). The executor-exception exit sets error_class to the raised exception's own class name, following the same convention already used elsewhere in this codebase (e.g. collective_driver_generator.py, forge_fusion.py) for exception-derived classes -- more specific than a flat string, and free since the exception object is already in hand. Extends SubAgentResult.error_class's docstring (added for item #8 in the previous commit) with these two producers. Adds direct test coverage for both paths; the no_executor test previously asserted nothing about error_class. --- .../tests/test_coordinator_runtime.py | 22 +++++++++++++++++++ .../orchestrator/loop/sub_agent_runner.py | 10 +++++++++ 2 files changed, 32 insertions(+) diff --git a/src/hyperloom/inference_optimizer/tests/test_coordinator_runtime.py b/src/hyperloom/inference_optimizer/tests/test_coordinator_runtime.py index 3e21e6baad..067219f3e7 100644 --- a/src/hyperloom/inference_optimizer/tests/test_coordinator_runtime.py +++ b/src/hyperloom/inference_optimizer/tests/test_coordinator_runtime.py @@ -121,6 +121,28 @@ async def test_sub_agent_runner_no_executor_fails(tmp_path): res = await sub.run_task(task) assert res.state == "failed" assert "no runner" in res.error + assert res.error_class == "no_executor" + db.close() + + +@pytest.mark.asyncio +async def test_sub_agent_runner_executor_exception_sets_error_class(tmp_path): + """A raised executor exception must not collapse into the generic + unknown_error gap bucket -- error_class carries the exception's own class name. + """ + db = SqliteConnection(tmp_path / "x.db") + locks = ResourceLockManager(SqliteLeaseBackend(db)) + tr = TaskRegistry(db) + sub = SubAgentRunner(locks, tr) + + async def exe(ctx): + raise TimeoutError("benchmark server never came up") + + sub.register_executor("bench_runner", exe) + task = await tr.create(kind="bench_runner", params={}, idempotency_key="k-y") + res = await sub.run_task(task) + assert res.state == "failed" + assert res.error_class == "TimeoutError" db.close() diff --git a/src/hyperloom/orchestrator/loop/sub_agent_runner.py b/src/hyperloom/orchestrator/loop/sub_agent_runner.py index 687d305e85..b3e6e34fe4 100644 --- a/src/hyperloom/orchestrator/loop/sub_agent_runner.py +++ b/src/hyperloom/orchestrator/loop/sub_agent_runner.py @@ -86,6 +86,14 @@ class SubAgentResult: * ``"crash"`` / ``"oom"`` / ``"hang"`` / ``"detokenizer_stall"``: exact-matched by :meth:`writeback._pitfall_severity_for` to classify a failure as crash-severity for the KB. + * ``"no_executor"``: no runner registered for the task's + ``kind`` — set directly on this dataclass, same site as + ``policy_{rule}``, so this exit no longer collapses into + ``"unknown_error"`` either. + * The raised exception's ``__class__.__name__`` (e.g. + ``"TimeoutError"``): an executor raised instead of returning a + result. Same reasoning — a real class beats the generic + bucket, even though the exact name isn't enumerable up front. * Anything else (including empty): executors set their own ``error_class`` inside ``result``, or leave it unset, in which case the gap ledger buckets it as ``"unknown_error"``. @@ -291,6 +299,7 @@ async def run_task( state="failed", result={}, error=f"no runner registered for kind={task.kind!r}", + error_class="no_executor", ) lease: Lease | None = prebound_lease @@ -331,6 +340,7 @@ async def run_task( state="failed", result={}, error=repr(exc), + error_class=exc.__class__.__name__, ) await self._transition_resilient( task.task_id,