Skip to content

fix: narrow or remove the broad excepts the 10-29 report judges non-compliant - #2320

Open
lishuoshuo-amd wants to merge 41 commits into
mainfrom
fix/broad-except-1029
Open

lishuoshuo-amd wants to merge 41 commits into
mainfrom
fix/broad-except-1029

Conversation

@lishuoshuo-amd

@lishuoshuo-amd lishuoshuo-amd commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator
  • Description: the 10-29 code-health report judges every broad catch (except Exception / BaseException, bare except, contextlib.suppress(Exception)) against its call chain. A broad catch is compliant only when it re-raises on every path, is an outermost entry that records the traceback and ends in failure, or isolates one loop item, records the traceback and writes the failure to a field the caller reads. The report lists 184 violations. This PR handles 146 of them plus the 23 compliant catches the report suggests narrowing, site by site from the try body and its callers (one of the 23, the outermost handler in scripts/partition_mode_sweep.py, is kept broad: narrowing it would let a pre-sweep bug exit 1, which the script documents as "no mode produced a valid measurement", instead of 4):

    • 108 narrowed to the types the body actually raises (OSError, sqlite3.Error, subprocess.SubprocessError, json.JSONDecodeError, KBStoreError, the SDKs' own error bases, ...). Cleanup and telemetry keep "an expected failure does not abort the main path"; an unexpected one now propagates.
    • 36 try blocks removed where the body raises nothing but defects (several wrap writers that already handle their own I/O errors, such as append_llm_call).
    • 8 made compliant in place: the handler stays broad because the callee raises bare Exception (the quantization runner's claude-agent-sdk calls) or an open set (an agent session, a retried LLM call, a generated script's main, an out-of-tree plug's close), and the site now records the full traceback and hands the failure to a field or exception the caller reads; PlugHost.close closes every started plug and then raises the first non-I/O close error.
    • 3 left as on main after review, all already compliant boundaries that record the traceback: the optional report-narration pass in render_session_report (its callee chain reaches claude-agent-sdk, which raises bare Exception, and Python 3.10's asyncio.TimeoutError); the MCP heartbeat thread; and the hosted reply annotator, where any error costs the reply its block and never the result of a tool that has already run (an error there must not tempt a client to re-run a side-effecting tool).
    • Not changed, and why:
      • 33 of the 37 "mid-chain explicit failure" sites (a handler deep in a call chain that converts the error into a failed result the caller reads); whether these are compliant is being settled with the report owner, and changing them is a behaviour change per site, so they are a separate PR;
      • 8 catches in the two vendored KB-store clients (_vendor/kb_store_client.py, kb_store/remote_client.py), pinned byte-for-byte to the upstream blob by test_vendored_sdk_matches_upstream_git_blob; they have to be narrowed upstream;
      • 4 catches around an agent session or forge-loop subprocess whose error set is open and which already hand the failure to the caller as a result (InSessionGate._on_stop, _source_attempt, _run_supervisor_review, dispatch_single_task); ClaudeBackend._query_once, where claude-agent-sdk re-raises every stream error as bare Exception and the failure becomes a resumable sdk_error, keeps its broad catch and now logs the traceback (below); making them compliant needs a decision on what the caller does with the failure, which belongs with the mid-chain set;
      • the one site in kernelforge/examples, outside the gate's scope.
    • The other four mid-chain sites and _query_once, five handlers that hand an explicit failure to their caller but did not record it at warning with its traceback now do: LangfuseEmitter._create_score (narrowed to the module's _SDK_FAILURES, debug -> warning), OutcomeMixin._request_lesson (debug -> warning), _FusedMoeAdapter._ops (info -> warning), the rewrite controller's checkpoint probe (still one warning per run, since the probe fires about once a second, and that warning now carries the last failure's traceback) and ClaudeBackend._query_once's stream-failure warning (now with exc_info).
    • Close-out paths keep their release steps when a defect now surfaces: the CLI's _close_optimize_run releases the session lock, writes the terminal artifacts and saves the state before Coordinator.stop()'s error propagates, and dispatch_single_task releases the operator worktree before a last-recovery defect propagates, while an operational git failure in that last-chance recovery (subprocess.SubprocessError, OSError) is logged with its traceback and the task's result stands; the round-recorder fanout closes every recorder before raising the first close error.
  • Linked issue(s): none

  • Tests: every changed or added assertion was run on the code before its fix and failed there. Added tests pin the newly visible behaviour with exact assertions, for example: a failed close sequence fails the run after releasing it; a failed pre-reactor phase advance fails the tick; a raising round recorder fails the round; a raising compute-bound re-profile fails the roofline action; the vLLM ROCm probe exits 1 with the traceback when is_rocm() raises (it used to fall back to the class name and print vllm_rocm_ok); a defect in the last-chance recovery still releases the operator worktree; the quantization runner logs the SDK traceback without a log callback; a raising Coordinator.stop() still releases the session lock and writes the terminal artifacts; a bare Exception from the narration client still yields the deterministic report; a non-I/O plug close error still closes the other plugs, and after a failed start rides on the PlugError; a heartbeat hook that raises once keeps beating; a git timeout in the last-chance recovery keeps the task result and releases the tree; a failing recorder close still closes the others; the five explicit-failure handlers above log at warning with the exception (test_a_score_the_sdk_refused_is_logged_at_warning_with_its_traceback, test_a_summarizer_that_raises_is_logged_at_warning_with_its_traceback, and level or exc_info assertions added to the existing fused-MoE operand, checkpoint-probe and Claude stream-error tests). Three test setups that relied on a broad catch absorbing a missing piece of production wiring now carry it: the roofline executor in the kernel-entry tests (production always registers it) and _enqueue_on_validated_board on the bringup stand-in. Tests that injected a now-uncaught type (RuntimeError, KeyError) inject a realistic caught one instead. Three fixtures that the broad catches had been hiding are fixed (_StubRecipeKB, _ReapStub, _FakeKbEmitter lacked methods the code calls). One test is deleted: test_run_with_session_kill_survives_a_broken_liveness_callback, since every production liveness callback is a counter increment. Run, after rebasing on main: the 52 changed test files, 1905 passed, 8 skipped (one DeepSeek test that fails identically on main locally deselected); the _run_optimize drivers (test_sbd_v6_initial.py, test_cli_resume_launch_shape.py, test_preflight_server_memory.py, test_session_lock.py, test_resume_guard.py), 236 passed; the Codex teardown test also on Python 3.10; the test files that cover each touched module, per area; ruff check ., ruff format --check ., lint-imports (11 kept).

  • Size/complexity triggers crossed: the gate reports 9 units worse than main, each by 1-6 lines, all already over their limit. Each line is the visible-failure part of a fix (a traceback print, an exception chain, a logged rollback, an import for the narrowed type). Splitting these units is a separate refactor; this PR carries baseline-raise for:

    Unit Main Now Why it grew
    codex.py CodexBackend._execute 90 93 one tuple naming the teardown errors, and a rollback failure that is logged instead of suppressed
    discover.py registered_agent_llm_fn._fn 92 94 keeps the last exception for exc_info and raise ... from
    discover.py (module) 1434 1436 the same two lines
    vllm_dense_tunableop.py _generate_tunableop_script 85 87 the generated script imports traceback and prints it on failure
    dispatcher.py dispatch_single_task 108 114 an inner try/finally so the worktree is released even when the last recovery raises, and a handler that logs an operational git failure there
    coordinator.py (module) 1302 1304 the close-sequence and pre-reactor-advance failures are re-raised
    prelude.py (module) 1784 1785 imports KBStoreError for the narrowed handler
    specialists/subprocess_.py (module) 1356 1357 imports sqlite3 for the narrowed handler
    cli/__init__.py (module) 1860 1864 _close_optimize_run, the CLI close-out moved out of _run_optimize so the lock and artifacts survive a raising stop()

    Nine other modules and functions get shorter, _run_optimize among them. The diff is 1149 production lines by pr-hygiene's count, over the 1000-line budget, but about half is re-indentation where a try was removed (git diff -w counts 603); it is one concern across 130 files, so it needs size-exception from a maintainer.

  • If this simplifies or refactors: 35 try blocks are removed; the rest is narrowing. No contract changes on the normal path.

  • Observable effect: a failure that used to be logged (often at DEBUG) and swallowed now surfaces when it is a defect rather than an expected I/O, store or SDK error. The ones an operator can see:

    • a run whose close sequence fails now exits as failed, after everything is released; a defect while Coordinator.stop() cancels in-flight actions now fails the CLI too, after the session lock is released and the terminal artifacts are written;
    • a phase advance that fails before the reactors fails the tick, recorded at the existing tick boundary;
    • a raising compute-bound re-profile fails the roofline action instead of silently keeping the original analysis (expected failures still return a status and keep it);
    • a round recorder that raises fails the benchmark round;
    • the vLLM ROCm probe fails when vLLM's own platform check raises, instead of passing on the platform class name;
    • progress emit now lets an out-of-tree events implementation's unexpected error reach the caller; a plug whose close raises a non-I/O error no longer stops the other plugs closing, and that error is raised once all are closed, or, after a failed start, appended to the start's PlugError;
    • hyperloom recover narrows its Langfuse-push and repackaging handlers but still exits 0 after an expected failure of either; changing that exit code is left for a separate change.
  • Breaking changes: no public API change. Out-of-tree plug and events implementations that raise anything other than I/O errors now see the error instead of a log line.

  • PR addresses single concern: yes (broad-exception remediation from one report).

  • Root cause is upstream (Magpie/TraceLens/GEAK/IntelliKit/AgentKernelArena), ticket filed: the 8 vendored KB-store client catches need the upstream KB Store SDK narrowed and re-vendored; not filed yet.

@lishuoshuo-amd lishuoshuo-amd added the baseline-raise Waives new, worse and baseline-growth code-metrics findings; reason required in the PR description label Oct 29, 2026
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Code metrics gate: PASSED with waived findings (baseline-raise)

Judged: the 130 files this change touches. Files it does not touch never fail it; anything found there is listed under Outside this change for information.

Dimension Limit New Worse Improved Over the limit vs base
Cyclomatic complexity fails > 20 0 0 0 0 +0
Cognitive complexity fails > 30 0 0 0 34 +0
Function length (lines) fails > 60 0 4 3 249 +0
Nested block depth fails > 4 0 0 0 37 +0
Parameter count fails > 5 0 0 0 0 +0
Module length (lines) warns > 800, fails > 1200 0 5 6 42 +0
Duplicated lines per file fails > 0 0 0 0 76 +0
Dead code (vulture) fails > 0 0 0 0 2 +0
Directory size (entries) fails > 50 0 0 0 1 +0
Check Findings Effect
Comments 0
Broad catch without a reason 0
Broad catch that records nothing 0
English only 0
Production code imports test code 0
Repeated literals 0
A1 Private name of another package 0
A8 Flag argument 0
A9 Tuple return 0
A10 Raw vocabulary comparison 0
A11 Hidden global write 0
A12 Pure forwarder 0
A12 Single-implementation abstraction 0

Worse than at the base (waived by the baseline-raise label): 9

Unit Dimension Base Now
src/kernelforge/agent_backends/codex.py:713 CodexBackend._execute Function length (lines) 90 93
src/kernelforge/workflows/forge_fusion/discover.py:1285 registered_agent_llm_fn._fn Function length (lines) 92 94
src/kernelforge/workflows/gemm_tune/tuners/vllm_dense_tunableop.py:122 _generate_tunableop_script Function length (lines) 85 87
src/kernelforge/workflows/rewrite_controller/dispatcher.py:164 dispatch_single_task Function length (lines) 108 114
src/hyperloom/inference_optimizer/cli/__init__.py:1 Module length (lines) 1860 1864
src/hyperloom/orchestrator/loop/coordinator.py:1 Module length (lines) 1302 1304
src/hyperloom/orchestrator/phases/prelude.py:1 Module length (lines) 1784 1785
src/hyperloom/orchestrator/specialists/subprocess_.py:1 Module length (lines) 1356 1357
src/kernelforge/workflows/forge_fusion/discover.py:1 Module length (lines) 1434 1436

How to fix

  • New or worse: split the unit (extract a function, a class, a module) until it is back under the limit, or no worse than the base branch has it. There is nothing to record: the base is the reference.
  • Override: the baseline-raise PR label waives new and worse findings and the failing design checks (they stay listed here). Use it only with the reason in the PR description; it waives nothing else.
  • Tools for a local run: pip install ruff==0.16.2 complexipy==8.0.1 radon==6.0.1 vulture==2.16 and npm install -g jscpd@5.4.1 (or point CODE_METRICS_JSCPD at another jscpd command, such as npx --yes jscpd@<version>).

Module length warning: over 800 lines, within the 1200-line failure limit (not failing): 12

Module Lines Limit
src/hyperloom/inference_optimizer/cli/bootstrap.py 1085 warns > 800
src/hyperloom/inference_optimizer/trace/langfuse_emitter.py 1136 warns > 800
src/hyperloom/orchestrator/actions/executors/_ray_serving.py 930 warns > 800
src/hyperloom/orchestrator/actions/executors/_subprocess_kill.py 1144 warns > 800
src/hyperloom/orchestrator/enablement/lane.py 888 warns > 800
src/hyperloom/orchestrator/knowledge/remote_recipe/values.py 1138 warns > 800
src/hyperloom/orchestrator/roles/claude.py 876 warns > 800
src/hyperloom/orchestrator/roles/critic_agent.py 1058 warns > 800
src/kernelforge/agent_backends/claude.py 874 warns > 800
src/kernelforge/agent_backends/codex.py 863 warns > 800
src/kernelforge/workflows/forge_loop/preparation/task_preflight.py 820 warns > 800
src/kernelforge/workflows/gemm_tune/tier3/dispatch.py 902 warns > 800

Improved by this change (informational): 9

Unit Dimension Base Now
src/hyperloom/inference_optimizer/cli/__init__.py _run_optimize Function length (lines) 153 145
src/kernelforge/agent_backends/claude.py ClaudeBackend._query_once Function length (lines) 89 88
src/kernelforge/workflows/forge_loop/iteration/outcome_runtime.py OutcomeMixin._record_lesson Function length (lines) 86 82
src/hyperloom/inference_optimizer/cli/preflight.py Module length (lines) 1749 1746
src/hyperloom/orchestrator/actions/executors/roofline.py Module length (lines) 1277 1271
src/hyperloom/orchestrator/kernel/conc_sweep.py Module length (lines) 1371 1360
src/hyperloom/orchestrator/loop/intent_router.py Module length (lines) 1357 1352
src/hyperloom/orchestrator/specialists/runner.py Module length (lines) 1405 1397
src/hyperloom/orchestrator/trace_analysis/tracelens_skill_runner.py Module length (lines) 1288 1282

Verdict computed by the gate scripts in /home/runner/work/_temp/code-metrics-gate, not by this tree's copy.

Thresholds and sources: [tool.hyperloom.code_metrics] in pyproject.toml. Tools: complexipy 8.0.1, jscpd 5.4.1, radon 6.0.1, ruff 0.16.2, vulture 2.16.

try:
from .langfuse_emitter import get_emitter
# Second sink (opt-in): mirror the call to Langfuse live.
from .langfuse_emitter import get_emitter
@lishuoshuo-amd lishuoshuo-amd added the size-exception Waives the PR diff budget; added by a maintainer with a reason in the PR description label Oct 30, 2026
@lishuoshuo-amd
lishuoshuo-amd marked this pull request as ready for review October 30, 2026 02:04
@lishuoshuo-amd lishuoshuo-amd added size-exception Waives the PR diff budget; added by a maintainer with a reason in the PR description and removed size-exception Waives the PR diff budget; added by a maintainer with a reason in the PR description labels Oct 30, 2026
@chaojhou

Copy link
Copy Markdown
Collaborator

@lishuoshuo-amd this PR conflicts with main (f418170), so I have not reviewed it yet. The conflicting files are:

  • src/hyperloom/inference_optimizer/cli/kb.py
  • src/kernelforge/agent_backends/claude.py
  • src/kernelforge/workflows/forge_loop/campaign/reporting.py

Could you rebase onto the latest main, or merge it in, and push? The review will follow once it merges cleanly.

@lishuoshuo-amd

Copy link
Copy Markdown
Collaborator Author

@chaojhou rebased onto main (f418170) and pushed; head is now 6e0d3b7 and the PR merges cleanly. The three conflicts all came from #2308 moving modules into hyperloom_common:

  • src/hyperloom/inference_optimizer/cli/kb.py and src/kernelforge/agent_backends/claude.py: kept the new hyperloom_common import paths, plus the imports this PR adds for the narrowed handlers (LocalRecipeStoreError, WorkspaceSafetyError).
  • src/kernelforge/workflows/forge_loop/campaign/reporting.py: took main's read_json_strict version, which already catches (OSError, ValueError), the same narrowing this PR made.

After the rebase every changed production module imports, and no added import points at a module #2308 moved. The CodeQL py/cyclic-import note on llm_trace.py:227 is the existing alert #3243 on main (same from .langfuse_emitter import get_emitter, one line lower there); removing the surrounding try only dedented it.

@chaojhou chaojhou left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #2320 -- fix: narrow or remove the broad excepts the 10-29 report judges non-compliant

What it does: Works through the broad catches the 10-29 code-health report judges non-compliant with S8, site by site across 80 production files (net −72 lines): most are narrowed to what the try body raises or removed so the error reaches its owner, 7 stay broad and now record the traceback, and the close-out paths keep their release steps when a defect now propagates.

Scenario: HYPERLOOM_REPORT_LLM_BACKEND=anthropic with only a Claude subscription token set, then hyperloom report <session_breakdown.json>, and the Claude CLI stream fails. claude-agent-sdk raises a bare Exception, which the head lets escape render_session_report and dump_session_report.main. The command exits with a traceback and writes no session_report.md; base wrote the deterministic report. Every other report-listed site checked does end its violation.
Layer: consumer (each handler around its try body) -> consumer (the same handler is where the error is admitted).

Risk: high -- Tier 1 files cli/init.py, loop/coordinator.py, loop/dispatcher.py and state/shared_state.py change what their failures do
Decisions for a human:

  • src/hyperloom/inference_optimizer/cli/init.py -- a raising Coordinator.stop() now ends the CLI with that error after the lock release, terminal artifacts and save; the Langfuse session_start marker is no longer guarded at session creation; terminal-artifact writes catch only I/O, value and type errors -- is a traceback after a fully written session the exit operators should see?
  • src/hyperloom/orchestrator/loop/coordinator.py -- a failed pre-reactor phase advance now fails the tick instead of being recorded and skipped; stop() re-raises a cancel error after closing the DB; the MCP setup diagnostic write is unguarded -- should a phase-advance defect end the session?
  • src/hyperloom/orchestrator/loop/dispatcher.py -- the self-heal pump steps absorb only sqlite3.Error, ValueError and OSError -- should any other defect in a self-heal stop the pump it keeps running?
  • src/hyperloom/orchestrator/state/shared_state.py -- the Langfuse status mirror after save() catches only TypeError/ValueError -- is a save that raises from the telemetry mirror acceptable?
  • src/hyperloom/hosted/mcp_server.py:209 -- a non-I/O reply-annotator error now becomes JSON-RPC -32603 after the tool ran, so its result is discarded although its side effects happened (stated, pinned by test_an_annotator_bug_is_answered_as_an_internal_error) -- is a client retry of a side-effecting tool acceptable?
  • src/kernelforge/workflows/rewrite_controller/dispatcher.py:283-291 -- the last-chance recover_task_result is no longer suppressed; _trusted_manifest -> commit_exists -> git cat-file runs outside recovery's own SubprocessError/OSError handler, so a git timeout or spawn failure replaces a returned SUCCEEDED result and stops dispatch_prepared_tasks' sequential loop -- should an operational git failure here count as a defect?
  • src/hyperloom/orchestrator/actions/executors/_subprocess_kill.py:827-829 -- a raising KvMetricsRecorder.close() fails the round (stated) and also skips GpuPowerRecorder.close() (not stated) -- should the fanout close every recorder before raising?
  • src/kernelforge/workflows/flydsl_rewrite/flydsl_rewrite_driver_preparation.py:570 -- a session that raises after writing a conforming driver is no longer preflighted, so the last attempt fails where base could succeed (stated in the table) -- is that the intended trade?

Blocking issues: 3

  1. [S8] The narration catch no longer covers what the claude-agent-sdk transport raises, so hyperloom report crashes instead of falling back [verified]
    Problem: render_session_report now catches _narration_failures(), which is OSError, RuntimeError, ValueError, TimeoutError, AttributeError plus the SDK base classes already imported (src/hyperloom/inference_optimizer/breakdown/reporters/compose.py:100-104, :126).

    • With HYPERLOOM_REPORT_LLM_BACKEND=anthropic and a subscription token, AnthropicClient.complete goes through anthropic_completion → ClaudeOneShotClient.messages → sdk.query(...).
    • This PR's own description says claude-agent-sdk "re-raises every stream error as bare Exception", and keeps ClaudeBackend._query_once broad for that reason.
    • On Python 3.10, which requires-python allows, asyncio.wait_for in amessages raises asyncio.TimeoutError. Before 3.11 that is not the builtin TimeoutError.
    • The base handler was already compliant. It logged with exc_info and wrote <llm_error: …> into llm_raw, from which used_llm is derived.
      Impact: dump_session_report.main calls render_session_report with no handler (tools/dump_session_report.py:69). A CLI stream error or a 3.10 timeout in the optional narrative pass ends hyperloom report with a traceback, and no session_report.md is written. At base, the same run wrote the deterministic report. The description states no change to report narration.
      Action: Keep this handler broad: it is the isolation boundary of one optional pass, and it already records the traceback and the failure. Or add Exception coverage only for the SDK transport and asyncio.TimeoutError. Then delete _narration_failures.
  2. [S8] PlugHost.close() now stops at the first non-I/O close error, leaving the other started plugs open [verified]
    Problem: the close loop catches only OSError around closer() (src/hyperloom/plugs/host.py:122). Plug implementations come from entry points (plugs/registry.py), so a closer can raise anything.

    • A RuntimeError from one plug leaves while self._order. That plug has already been popped. Every plug started before it stays in _order and is never closed.
    • close() is also called from start()'s failure path (host.py:66-71). There the closer's error replaces the PlugError that carries the socket name and plugin_errors() diagnostics.
    • start() itself still catches Exception around the same out-of-tree code.
    • The description says a plug close error "now reaches the caller". It does not say that the remaining plugs stop being closed.
      Impact: an operator with one faulty plug loses the teardown of every plug started before it. When the failure is at start, they see the close error instead of which socket failed to start and why.
      Action: Keep closing the rest, then raise. For example, collect the first non-OSError exception in the loop, close every remaining plug, and re-raise it after the loop; on the start() path, chain it under the PlugError. Or restore the base handler, which was a compliant loop isolation recorded in close_errors().
  3. [S8] A heartbeat hook's non-I/O error now kills the beat thread instead of reaching the caller [verified]
    Problem: _call_heartbeat.beat() catches only OSError around hook() (src/hyperloom/hosted/mcp_server.py:468). hook is whatever was passed to the public attach_heartbeat, and nothing in the tree registers one.

    • beat is the target of a daemon threading.Thread (:475). An unhandled error goes to threading.excepthook and the thread exits.
    • The finally joins the dead thread, and the request completes normally.
    • The description says heartbeat hooks "now let an out-of-tree implementation's unexpected error reach the caller". No caller ever sees it.
    • The base handler was the compliant outermost-thread form.
      Impact: one failing beat silently stops all further beats for the rest of a long tool call. Whatever liveness record the hook feeds stops advancing while the specialist is still working; the only trace is the thread's traceback on stderr, and the request succeeds.
      Action: Restore the outermost-thread handler with the traceback. Or, to make the stated behaviour true, store the error in beat, stop beating, and raise it from _call_heartbeat's finally after join().

Trim in this PR:

  • src/hyperloom/inference_optimizer/breakdown/reporters/compose.py:100-104 -- _narration_failures and the sys import -- finding 1 keeps the handler broad, and the helper exists only to approximate that open error set

Checked: the 18 candidate sites from area readers against what each try body and callee raises at head (10 refuted, 3 blocking, 4 decisions, 1 nit: codex_session.py:662-664 narrows its two teardown suppresses to different tuples where kernelforge's CodexBackend._execute uses one); build_client_from_env -> anthropic_completion -> ClaudeOneShotClient; PlugHost.start/close and plugs/registry.py; attach_heartbeat -> _call_heartbeat; the lock, worktree and session releases on the new raise paths; the body's stated effects against the diff | Ran: git diff d3df5b5 6966f0e per area, caller searches at head, CI at 6966f0e (all test shards, coverage 3.10/3.11, code-metrics, pr-hygiene, CodeQL pass), an independent skeptic agent per candidate | Skill: 3eec7f38c6e3 | Model: claude-opus-5-5 | Base: d3df5b5 | Head: 6966f0e

…wn and trace sinks

Catch only the errors each body can raise: torch attribute errors in the host
probe, OSError for its report, the decision-trace and narration failure types,
the Langfuse SDK failure set in the emitter, and SQLite/JSON errors in the
progress sink. The Langfuse mirror in the LLM-call and conversation ledgers no
longer hides emitter defects, and a parked failure note that cannot be written
is logged from the writer's own result.
…l gate, KB bootstrap and recover

Catch the filesystem, JSON and serialisation errors the terminal-artifact,
fail-fast and recover writers raise, the KB Store and local store errors of
the T0 warm-start, and the read/fetch errors of a reference script. Drop the
handlers around the Langfuse session_start marker and the install-step failure
record, whose bodies raise nothing but defects.
Rollback-after-verify failures are caught as workspace/OS/subprocess errors and
logged with a traceback; SDK teardown suppressions name the SDK, OS, runtime and
timeout errors they expect; the progress recorder and the usage observer no
longer swallow errors; the usage ledger only absorbs I/O failures.
…ion discovery and the vLLM pass probe

A crashed driver-authoring session becomes the attempt's failure instead of
preflighting an untouched driver; exhausted discovery retries chain the last
backend error and log each attempt's traceback; the pass probe narrows its
level-API read and re-raises after publishing its payload so the traceback
reaches stderr.
…start and campaign state

Warm-start publication and its rollback, the derived best view, and the campaign
config / best manifest readers catch only the I/O, git and validation errors
their bodies raise; an experiment's on-complete callback is no longer
silenced.
…owing lesson scope errors

The non-durable outcome reduction absorbs only git, I/O and journal-conflict
errors; the lesson scope step catches nothing, since the lesson store already
handles its own I/O failures.
…ck and graph-probe handlers

The graph probe's sitecustomize catches only the /proc, torch-patch and harness
read errors it expects, and reports a replay shard it cannot write on stderr.
… recovery and non-I/O progress failures

recover_task_result already turns every expected publication failure into a
result, so the dispatcher no longer wraps it; the scheduler's progress callback
only absorbs a snapshot write that fails with OSError.
The Langfuse score, lesson summarizer and fused-MoE operand handlers logged below warning; the checkpoint probe's one warning and the Claude stream-failure warning carried no traceback.
…ry plug and recorder before raising

Review of #2320: the narration pass and the heartbeat thread were already compliant boundaries and stay broad; PlugHost.close and the round-recorder fanout now close every member before raising the first error; the rewrite controller's last-chance recovery logs an operational git failure instead of replacing the task result.
@lishuoshuo-amd

Copy link
Copy Markdown
Collaborator Author

@chaojhou thanks, all three blockers were real and are fixed in b9974ed (rebased onto main a646a9c), each with a test that fails on the previous head:

  1. Report narration. render_session_report is back to main's handler; _narration_failures and the sys import are gone. test_a_bare_sdk_exception_in_the_narration_falls_back_to_the_deterministic_report raises a bare Exception from the client and asserts the deterministic markdown and <llm_error: Exception: ...>.
  2. PlugHost.close. Every started plug is closed; an I/O error is recorded as before, and the first non-I/O error is raised after the loop. On the start() path a close error is appended to the PlugError (socket name and plugin errors kept) instead of replacing it. Tests: test_a_non_io_close_error_still_closes_the_rest_then_raises, test_a_close_error_after_a_failed_start_is_reported_with_the_start_failure.
  3. Heartbeat. Back to main's outermost-thread handler with the traceback; test_a_beat_that_raises_once_keeps_beating pins that beats continue. The description no longer claims heartbeat errors reach the caller.

Two of the decisions were changes I had not intended, so they are fixed rather than left for a decision:

  • rewrite_controller/dispatcher.py: the last-chance recover_task_result now catches subprocess.SubprocessError / OSError and logs with the traceback, so a git timeout keeps the task result and the loop goes on; a defect still propagates after the worktree is released (test_a_git_failure_in_the_last_recovery_keeps_the_result_and_releases_the_tree).
  • _subprocess_kill._RecorderFanout.close: closes every recorder, then raises the first close error (test_fanout_closes_every_recorder_before_raising_the_first_failure).

The other decisions are as described in the body. Gate on the new head: the same 10 baseline-raise units (table updated: dispatch_single_task 108 -> 114), no broad-catch findings; size 1166 / 620 with -w; the 52 changed test files pass locally.

…e authoring session raised

An annotator failure after the tool ran again costs only the reply's block instead of the tool result, and a driver written before its authoring session raised is again judged by the deterministic preflight.
@lishuoshuo-amd

Copy link
Copy Markdown
Collaborator Author

@chaojhou follow-up on the decisions in your card, settled in 6aef958 rather than left open:

  • Reply annotator (hosted/mcp_server.py): restored main's handler. Answering -32603 after the tool has run discards a result whose side effects already happened and invites a client retry, which is worse than losing the annotator's block. test_a_failing_annotator_costs_the_reply_its_block_and_nothing_more again injects a RuntimeError; the -32603 test is gone. mcp_server.py now has no diff against main.
  • Driver preparation (flydsl_rewrite_driver_preparation.py): a driver written before its authoring session raised is preflighted again, so the last attempt can still succeed; the session error is still logged with its traceback, audited, and kept as prior_failure. Pinned by test_a_session_that_crashes_after_writing_a_conforming_driver_still_has_it_preflighted, which fails on the previous head. _DriverAuthoring.run drops out of the baseline-raise table (9 units now).
  • Kept, by design: a close-out defect (_close_run, Coordinator.stop()) ends hyperloom optimize with exit 1 after the lock, terminal artifacts and state are written -- that is the run's outermost boundary recording the traceback and ending in failure; and the dispatcher self-heal steps absorb only store, value and I/O errors, so a defect fails the tick at the existing tick boundary and counts toward the crash-rate emergency stop instead of being swallowed every tick.

Body updated (108 narrowed, 3 left as on main, size 1149 / 603 with -w).

@chaojhou chaojhou left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #2320 -- fix: narrow or remove the broad excepts the 10-29 report judges non-compliant

What it does: Works through the broad catches the 10-29 code-health report judges non-compliant with S8, site by site across 78 production files (net −57 lines). Most are narrowed to what the try body raises or removed so the error reaches its owner. Eight are kept broad and now record the traceback. Close-out paths (_close_optimize_run, dispatch_single_task, PlugHost.close, the round-recorder fanout) release everything before a defect propagates.

Scenario: a framework-rewrite evidence run, where _framework_rewrite_evidence.py:206 sets HYPERLOOM_HOST_PROBE_DEEP=1. A function under the vLLM/SGLang roots is called with an np.int64 positional argument while its argument samples are not yet full. At head, _profile_hook -> _sample_args -> _fingerprint(strict=True) raises AttributeError at value.data_ptr(), and the hook catches only RuntimeError. The exception therefore propagates into the profiled framework call. Every other report-listed site checked does end its violation.
Layer: consumer (each handler around its try body) -> consumer (the same handler is where the error is admitted).

Risk: high -- Tier 1 files cli/init.py, loop/coordinator.py, loop/dispatcher.py and state/shared_state.py change what their failures do
Decisions for a human:

  • src/hyperloom/inference_optimizer/cli/init.py -- a raising Coordinator.stop() ends hyperloom optimize with exit 1 after the lock release, terminal artifacts and save (author: by design); the Langfuse session_start marker is no longer guarded at session creation -- is a traceback after a fully written session the exit operators should see?
  • src/hyperloom/orchestrator/loop/coordinator.py -- a failed pre-reactor phase advance now fails the tick. _close_run re-raises a close-sequence error after releasing resources, which also skips run()'s _close_undecided_proposals, so pending proposals keep open trajectory spans (trace rows only) -- should the undecided-proposal close-out move into a finally?
  • src/hyperloom/orchestrator/loop/dispatcher.py -- the self-heal pump steps absorb only store, value and I/O errors, so a defect fails the tick and counts toward the crash-rate emergency stop (author: by design) -- accept?
  • src/hyperloom/orchestrator/state/shared_state.py -- the Langfuse status mirror after save() catches only TypeError/ValueError -- is a save that raises from the telemetry mirror acceptable?

Blocking issues: 2

  1. [S8] The deep host probe's profile hook now raises into the framework call it is profiling [verified]
    Problem: HostProbe._profile_hook is installed with sys.setprofile and threading.setprofile and runs inside every Python call under the framework roots. This PR narrows its handler from Exception to RuntimeError (src/hyperloom/inference_optimizer/assets/host_probe/hl_host_probe.py:538).

    • Its callee _fingerprint(strict=True) (hl_host_probe.py:326-342) treats any value with shape, dtype and device as a tensor.
    • When weakref.ref fails, it falls back to value.data_ptr(), catching only RuntimeError.
    • A NumPy 2 scalar has all three attributes, cannot be weak-referenced, and has no data_ptr, so AttributeError escapes both handlers.
    • An exception raised in a profile function propagates into the profiled frame.
    • Reproduced with the head and base files: a fixture function step(np.int64(3)) under the probe's roots with deep=True raises AttributeError: 'numpy.int64' object has no attribute 'data_ptr' at head and returns 4 at base.
    • The description does not mention the host probe.
      Impact: a framework-rewrite evidence run (deep probe on) breaks the vLLM/SGLang server code it is measuring the first time a sampled function receives a NumPy scalar or any other non-tensor with those three attributes. The run then fails or measures a crashed server instead of collecting evidence. At base, the probe skipped that sample at debug level.
      Action: Restore except Exception on _profile_hook, with the debug log and traceback: it is the outermost telemetry sink of a hook that runs inside code it does not own, which S8 keeps broad. Catching AttributeError beside RuntimeError at the data_ptr() fallback (hl_host_probe.py:341) would also stop recording a recycled-address generation for non-tensors, but on its own it leaves the hook's error set open.
  2. [X2] The description's remediation counts contradict each other [verified]
    Problem: body.txt states:

    • "36 try blocks removed" (line 3, the remediation buckets) and "35 try blocks are removed" (line 30, the simplification section);
    • "146 of them plus the 23 compliant catches" (line 1, 169 sites), while the buckets list 108 narrowed + 36 removed + 8 made compliant + 3 left as on main = 155.
      Impact: the release notes are built from this description and would carry two different removal counts, and a total the buckets do not reach.
      Action: Reconcile the numbers against the head: one removal count, and buckets that sum to the stated total, or a line saying which sites the buckets do not count.

Trim in this PR:

  • src/kernelforge/workflows/flydsl_rewrite/flydsl_rewrite_driver_preparation.py:646 -- the self.prior_failure = "the previous authoring session failed: ..." assignment in _author's except -- after _author returns, every path in _DriverAuthoring.run (:561-603) overwrites prior_failure or returns, so the value is never read; the error stays logged and audited in agent_event.json

Checked: prior findings at 6aef958: compose.py is byte-identical to main, PlugHost.close and _close_after_failed_start (host.py:78-139), mcp_server.py has no diff against main; the rebase onto a646a9c against git merge-tree of the old head (only the 12 files of b9974ed and 6aef958 differ) and the 8 files #2212 also touched; the whole diff by area (Tier 1 CLI/loop/state, orchestrator actions/phases/kernel/enablement/bringup, knowledge/roles/specialists/trace/breakdown/assets, kernelforge forge_loop, kernelforge rewrite_controller/gemm_tune/forge_fusion/flydsl/agent_backends, plugs/hosted/codex_session/quantization/hyperloom_common/scripts) and the body against the diff; _fingerprint with NumPy 2.5.1 scalars; the code-metrics waiver against D12/D14 and the body's stated reason | Ran: git diff a646a9c 6aef958 per area; the base/head host-probe reproduction under sys.setprofile; CI at 6aef958 (all 12 test shards, coverage 3.10/3.11, code-metrics, pr-hygiene, CodeQL, mypy, import-linter pass); an independent skeptic agent per candidate (5 candidates, 4 survived; 1 blocking, 1 trim, 1 decision, 1 merged into finding 2) | Skill: 3eec7f38c6e3 | Model: claude-opus-5-5 | Prior: S8 fixed, S8 fixed, S8 fixed | Base: a646a9c | Head: 6aef958

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

baseline-raise Waives new, worse and baseline-growth code-metrics findings; reason required in the PR description size-exception Waives the PR diff budget; added by a maintainer with a reason in the PR description

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants