Skip to content

[Stepping] Correctness sweep across the optimization loop: measurement integrity, phase budgets, resume safety, and a large dead-code removal - #1188

Merged
ZhengGong-amd merged 73 commits into
mainfrom
feat/zgong/explore-opt-17
Aug 19, 2026
Merged

[Stepping] Correctness sweep across the optimization loop: measurement integrity, phase budgets, resume safety, and a large dead-code removal#1188
ZhengGong-amd merged 73 commits into
mainfrom
feat/zgong/explore-opt-17

Conversation

@ZhengGong-amd

@ZhengGong-amd ZhengGong-amd commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

This branch is an end-to-end audit pass over the orchestrator. It started from
one question — is every number the loop promotes actually measured? — and the
answer pulled in four adjacent areas: the promotion path into current_best,
the phase budget gates, the resume/session identity path, and the benchmark
environment contract. Along the way it removes ~10k lines of code that nothing
reaches: an undeployed robustness-server data path, an orphaned kernel harness,
and a long tail of symbols, artifacts and state fields with no reader.

The net is negative by 8k lines, but this is not a cleanup PR. Roughly half
the commits fix live defects, several of which silently corrupted the session's
reported gain. The removals are what made those defects visible.


1. Measurement integrity: nothing enters current_best unmeasured

The central defect class: several paths wrote a throughput onto current_best
that no benchmark ever produced, which then raised the grading anchor for every
later candidate and rewrote the session's reported gain.

  • b4ff1b297 — GEMM tuning KEEPs are now measured end-to-end. The GEAK
    branch derived a throughput as baseline_tput * best_speedup and wrote it
    straight onto current_best. A GEMM speedup of 1.1 would overwrite a
    current_best already holding an explore win of 1.5×, and rewrite the
    session's validated cumulative gain as that single GEMM's speedup. Both
    result shapes now reduce to a common candidate list and route through the
    per-tuner validator (previously forge-only), which measures each candidate
    against current_best via integrate_handler.
  • 045f7d08b — profiler-on measurements no longer promote. A profile run
    benches with the torch profiler attached, so its throughput is not comparable
    with un-profiled measurements. Promoting it on a private +1% rule bypassed the
    anchor gate, inflated the anchor for every later candidate, and left no
    optimization_stack entry — so the stale-stack warning never fired and a
    genuinely better KEEP measured below the inflated anchor was silently refused.
  • 227471e3d — one writer for current_best. integrate, geak_e2e,
    forge GEMM, fusion and warm_replay each hand-built their own current_best
    and stack entry; four replaced current_best outright, so a KEEP could drop
    cumulative server args or resurrect an env an earlier ablation had removed.
    All five now hand a winner dict to _lift_to_current_best. Two behaviour
    changes fall out: integrate and fusion gain the anchor guard they lacked
    (a KEEP measured below the established recipe is now refused rather than
    lowering current_best), and warm_replay stops hand-appending a rounded
    percentage to gain_per_stack_entry.
  • cc9f8843f, 82318eabe, 6389b5df4 — the stack replay is gone. Resume
    rebuilt current_best from an optimization_stack replay and overwrote it on
    any disagreement. The replay unioned every entry's extra_envs without
    applying unset_envs, so resume "repaired" a correct current_best into a
    wrong one. With every config KEEP going through the lift, the two are written
    together and cannot drift. The GEAK handoff and post-resume rebench now read
    current_best directly; the launch normalization that survives also had a
    bug where a --prefixed env key routed to server args replaced the arg
    string instead of appending to it.
  • de10702d2target_reached gates on the validated gain.
    TargetGainObjective read cumulative_gain, which any promotion could move
    without a stack rebench behind it, so a run could stop on a number no
    full-stack measurement supported. Every other loop decision already read
    cumulative_gain_validated.
  • eee047f26, b81dc3b35, cc30655cbcumulative_gain removed. It and
    cumulative_gain_validated computed the same expression; the only way they
    could differ was a write path recording a gain it should not have. The
    documented "per-round sum" never existed in any revision. One of the three
    removed "validated or raw" fallbacks published the raw value under the key
    validated_e2e_gain into the cross-session recipe KB whenever the validated
    gain was exactly 0.0 — a legal result, not a missing one.
  • beadc4c8a — a refused promotion journals as no_promote. The lift can
    decline a KEEP the executor granted; the audit row said no_promote but the
    journal keyed only on executor status and wrote KEEP, so a session audit
    counted an adoption that never happened.

2. Phase budgets: stop evicting phases that have done no work

  • fdadb6500 fixes three defects that let a phase be skipped or starved
    without spending any of its own budget:
    • normalize_budget_pct dropped 0.0 as missing, so the sentinel written for
      a disabled phase was overlaid with the library default and its revived share
      stayed in the charge-back denominator — shrinking exactly the phases the
      redistribution was meant to fund. Measured: under --no-kernel, EXPLORE was
      allotted 324.8 min where 498.8 min was intended.
    • The EXPLORE force-exit compared session wall-clock against a fixed 3 h, so
      any run of 3 h or less force-exited EXPLORE on its first tick with
      phase_remaining_pct == 1.0. The surviving budget-fraction arm reproduces
      every long-run case the hours arm caught.
    • The FRAMEWORK force-exit compared time-left against a fraction of the
      session total, so every macro-cycle reloop into FRAMEWORK — the preferred
      reloop target — entered and force-exited on the same tick.
    • Also retires --explore-force-exit-hours-remaining from the 3 h demo, which
      could only run because it disabled the gate.
  • 504bb245d puts framework_agent KEEP on the decaying threshold curve;
    _materialize_approved_proposal returned before the injection ran, so it
    always fell back to the static 1.0% default.
  • 27ad67737 routes all four dispatch sites through one
    resolve_keep_threshold(state), replacing an except Exception: pass wrapper
    and a baked-in literal dict. Prompt copy no longer states a number that drifts
    from the live curve.

3. Resume and session identity

  • 8f5532572TERMINAL_STATES derived from the transition table. The
    failed -> running edge was removed when retries moved to a fresh idempotency
    key, but the hand-listed set kept its old membership, so a failed row
    reported as still live. Three guards wedged on this: the watermark roofline
    gate never released (and the marker is persisted, so the next resume inherits
    the wedge), resume recovery never cleared
    enablement_validation_pending, and the FRAMEWORK authoring pump re-selected
    a dead candidate every tick.
  • c29169601 — never roll back a pending integrate on an unreadable event
    log.
    "No KEEP found" and "could not read the event log" were the same
    outcome, so a transient DB error during the first resume after a hard crash
    would reverse-apply every recorded patch, discard the attempt venv, report the
    loss as a fix, and clear the sentinel so no later resume could retry.
  • 2d7667a02 — session dirs are unique within a UTC second. The name is the
    de-facto session id (it feeds KB fact writes, critic requests and per-session
    sinks), so a same-second collision merged two runs' identity, not just their
    artifacts.
  • 493e801db — bare --resume removed (BREAKING). It scanned every model
    subtree under $USER_DATA_PATH, attached to the newest timestamp, and
    re-exported that session's model as MODEL_PATH without a word — silently
    continuing someone else's run on a different model, which SKILL.md already
    forbids. --resume-from is the only entry point; c70e31548 re-anchors the
    crash-recovery monitor, 7c13a720a pins that legacy command lines exit 2
    (argparse resolves --resume as a prefix of --resume-from), and 7f615df2b
    fixes the docs that documented timestamp-guessing in the same file that
    forbade it.
  • ab9568ca7 — a relative $USER_DATA_PATH is absolutised at the launcher
    boundary.
    Subprocesses spawn with their own cwds, so parent and child
    resolved different workspace roots while both believed they agreed.
  • a4620b32f, 78189a8ea, b9a4b09d5 — blank path-id components are
    refused outright instead of mapping onto a shared unknown bucket that had no
    producer and forced an early return past the traversal check.

4. Benchmark environment contract

  • 76b03786b — the bypass server gets the materialized envs. The bypass
    serving path hand-picked ROCR_VISIBLE_DEVICES out of benchmark.envs and
    dropped the rest, so a candidate whose only difference from baseline was an env
    launched a byte-identical server — same argv, same environment. The
    measured delta was run-to-run noise, the 1% keep threshold promoted it, and the
    env landed in current_best and in the reference script handed to the user.
    Kernel bundles and framework levers are activated by exactly such switches, so
    their attribution was measuring nothing. Behaviour change: configs carrying a
    static PATH under benchmark.envs now have it reach the server, matching
    Magpie.
  • 8aa97021d — per-variant env overrides hardened. GridVariant.extra_envs
    was filtered for control-plane credentials only, so a proposed variant could
    carry LD_PRELOAD / PATH / PYTHONPATH into the benchmark config — inert
    while bypass exported nothing, a live injection surface the moment it does.
    An allowlist isn't usable (grids legitimately set CONC/ISL/TP/…), so
    shell and loader hijacks plus control-plane credentials are blocked instead.
  • 6e0740cdfunset_envs cannot drop pinned workload envs. It popped any
    name given, so a variant could remove TP, CONC, ISL, RUN_EVAL or the GPU mask.
    That doesn't toggle a knob, it retargets the benchmark — dropping TP also
    shrinks the Ray lease to a single GPU, since the lease size is read back out of
    benchmark.envs.
  • 8167b8b72 — the breakdown recorder stopped dropping workload fields. A
    recorder fragment replaces its whole section rather than merging, and the
    workload fragment wrote framework where both renderers expect
    framework_name, omitted framework_version and objective, and coerced
    unset tp/conc/isl/osl from None to 0 (which reads as a real value). The
    recorder is on by default.

5. Robustness agent

  • e01bbcca1 (BREAKING) — the remote cluster data path is removed. No
    robustness-server is deployed, ROBUSTNESS_SERVER_URL is never set, and the
    five workload-uid env keys the hierarchy lookup depends on have read sites but
    no writers. The client was GET-only with no registration endpoint, so the
    server could not have learned this session either way — yet Config.discover()
    paid two doomed health probes every tick. Removed with it: the Source
    adapter, the cluster payload decoder, and the cluster_fault /
    pod_not_running symptoms that had no other producer. Four CLI flags go with
    it (9d700952a). ReactorBundle.aclose() now closes the RCA engine's HTTP
    client, which was never released.
  • 9d700952a — multi-node runs get the real agent, not the heartbeat mock.
    The downgrade guarded against LocalProbe false positives, but
    disable_local_probe already defaults to True on multi-node and swaps in a
    silent stub — so the downgrade was discarding every signal read straight off
    the Coordinator prompt and inbox, which cannot false-fire on a worker node.
    Multi-node produced no robustness symptoms at all, including
    deadline_imminent, the one that drives the delegate(report) wind-down.

    Expect alerts where there were none; --robustness-mock restores the old
    behaviour.
  • 95d4646f0 — a tick that harvested nothing no longer counts as a healthy
    primary, so the router degrades to the local probe instead of reporting a clean
    bill of health.
  • 96d89e51e — the runtime CLI fallback that backfilled a shadow
    session_id rebuilt the context by hand-listing members; that list was
    complete at 7 fields and the snapshot has since grown to 19, so a tick taking
    the fallback silently lost stop_reason, the wall-clock budget, validated
    gain, tick and macro_cycle — muting postmortem finalization and the deadline
    wind-down.
  • b70177075 — the SKILL.md symptom table listed ~20 of the 52 symptoms the
    rules emit. It is now a complete inventory keyed on the SignalSpec.name the
    classifier and factory both read (verified by set-comparison against the
    name= literals in signals/), so it cannot drift. cd7616103 drops the six
    families of milestone codes that said when a feature was built rather than
    what it does — three of which pointed at nothing.

6. Dead code and unread artifacts

  • 02162bce8 retires parallel_e2e_runner.py (538 lines), the
    self-validation harness from before a KERNEL phase existed. Its baseline step
    was removed in May and --backends defaults empty, so a plain invocation only
    raised. Its deletion also closes an aggregation bug rather than fixing it: every
    attempt's exception flattened to a row-level failed while the aggregate
    printed "status": "succeeded" and returned 0 (measured: 2/2 failed, exit 0).
    Its load_env_file duplicated ray_runtime.py's credential-alias derivation,
    which stays and is pinned by seven tests.
  • 78f162afe removes four categories of write-only output: the critic
    events.jsonl audit trail (7 call sites, no reader), framework_agent
    decision.json / semantic_audit.json (not in the session package glob, no
    skill points at them), bypass_trace_analysis kernel_sequence.json (already
    inlined in the result), and the extra_sglang_args shim.
  • fd81233c8 deletes 15 symbols with zero production callers, including the
    knowledge_plane typed read/write wrappers the codebase never adopted.
  • 4b1ee80dd removes three reads of attributes SharedState doesn't define,
    each absorbed by a getattr default — most consequentially, the framework
    ranker prompt read best_throughput / baseline_throughput and therefore
    never told the model the current best throughput. It now shares
    resolve_grading_anchor_tput with the admission path.
  • 2cb82ad7c collapses the ordinal kernel_opt_attempts index onto the
    stable kernel_opt_task_attempts ledger; the two diverged through three
    asymmetric cleanup paths and consumers fell back between them in inconsistent
    directions. A SharedState property keeps existing test doubles working.
  • 212e6c8ab converges genuinely duplicated helpers: the lease-reap +
    task-reclaim + DB-retention sequence (copied verbatim between the maintenance
    tick and the cycle soft-restart), five Claude-SDK message-text extractors with
    slightly different gaps, and three identical _phase_at binary searches. Four
    other near-duplicates are left merged-in-comment-only with the reason recorded
    (differing error contracts, undocumented threshold differences, and one that
    must stay importable on remote nodes without the hyperloom package).
  • fa264d8e6, 473aa8c8f, 5d49d0fdb, e509017d3 clear the remaining
    unread aliases, stale mirrors and follow-up breakages. e509017d3 is a review
    pass over the branch itself and fixes two defects the earlier mechanical stages
    introduced (an AttributeError on every GEMM-Roofline state sync, and a
    double-seeded plateau test).

Breaking changes

Change Migration
Bare --resume removed Use --resume-from <session-dir>; stale invocations exit 2
--robustness-server-url, --robustness-workload-uid, --robustness-enable-cluster-pod-metrics, --robustness-pod-metrics-categories removed Drop the flags; they fed an undeployed server
$ROBUSTNESS_SERVER_URL, $ROBUSTNESS_ENABLE_CLUSTER_POD_METRICS no longer read None
cluster_fault, pod_not_running, pod_no_metrics symptoms removed None — they had no producer
Multi-node no longer implies --robustness-mock Pass --robustness-mock explicitly for the old behaviour
parallel_e2e_runner.py removed Covered by the KERNEL phase and ci-e2e.yml
session_breakdown.json loses final.cumulative_gain_pct_per_round_sum Read cumulative_gain_pct_validated — it was equal in every session current code can produce
Bypass server now receives all of benchmark.envs A static PATH in a shipped config now wins over the caller's inherited value, matching Magpie

Legacy state.json files load unchanged — from_dict drops unknown keys.

Reverted in-branch

dd163f2c9 recorded two blockers (an unwired MoE guard in compose_server_args,
and stop_ray_if_owned having no production caller so no path stops a Ray head
this process started). It was reverted in e39b39bbf — both findings stand but
belong in the issue tracker, not the changelog.

Review notes

The commits are ordered to be readable one at a time and each is
self-justifying; e509017d3 is a review of the rest of the branch and is worth
reading first for the list of what the earlier stages got wrong. The largest
single-file diffs (phases/kernel.py, loop/writeback.py,
phases/machine_state.py) are dominated by §1 — the convergence onto one
current_best writer — and are best reviewed against 227471e3d and
b4ff1b297 together.

ZhengGong-amd and others added 6 commits August 13, 2026 11:45
GridVariant.extra_envs was filtered for control-plane credentials only, so a
proposed variant could carry LD_PRELOAD, PATH or PYTHONPATH into the benchmark
config. That is inert while the bypass serving path exports nothing but the GPU
pin, and becomes a live injection surface the moment it exports the mapping.

An allowlist is not usable here: the sweep, conc-sweep and shape-capture grids
build variants from code and legitimately set CONC/ISL/OSL/NUM_PROMPTS/RUN_EVAL/
PORT/TP/MAX_MODEL_LEN, then read them back. Block the shell/loader hijacks and
the control-plane credentials instead, and leave workload pins alone.

filter_benchmark_env_mapping loses both call sites and is removed; the YAML
persistence site keeps its credential-only scope through the shared filter,
which also gains it a key-syntax check.

Co-authored-by: Cursor <cursoragent@cursor.com>
…server

The bypass serving path hand-picked ROCR_VISIBLE_DEVICES out of benchmark.envs
and dropped the rest, so a candidate whose only difference from the baseline was
an env launched a byte-identical server: same argv, same environment. The
measured delta was run-to-run noise, the default 1% keep threshold promoted it,
and the env then landed in current_best and in the reference script handed to
the user. Kernel bundles and framework levers are activated by exactly such
switches, so their attribution was measuring nothing.

The scriptable path already exported the whole mapping. Converge both onto one
branchless helper that layers mappings over the parent env rather than adding a
third hand-picked key. _run_subprocess and the GEAK sweep share the same shape
and now go through it as well.

Behaviour change: every shipped config carries a static PATH under
benchmark.envs, which now reaches the server and wins over the caller's
inherited-and-prepended value. That is what the Magpie backend has always done,
and bypass documents itself as accepting the same environment contract.

Co-authored-by: Cursor <cursoragent@cursor.com>
unset_envs popped any name it was given, so a variant could drop TP, CONC, ISL,
RUN_EVAL or the GPU mask from the materialized config. That does not toggle a
knob, it retargets the benchmark: dropping TP also shrinks the Ray lease to a
single GPU, because the lease size is read back out of benchmark.envs.

Reuse BLOCKED_EXTERNAL_ENV_NAMES, the list that already says which names an
untrusted source may not set. Setting and unsetting are two directions of the
same change, so they answer to one list. Ordinary tuning knobs stay removable.

Co-authored-by: Cursor <cursoragent@cursor.com>
Both statements described the bypass backend as honouring a single hand-picked
key while Magpie re-exported the whole benchmark.envs mapping. Bypass now
exports the mapping too, so the only remaining difference is that it also uses
the interpreter to launch the server.

Co-authored-by: Cursor <cursoragent@cursor.com>
…itial toolkit

Introduced in 9167263 (2026-04-30) as the self-validation driver for the
then-new kernel-agent, when no KERNEL phase existed to prove the toolkit on a
real model. Its purpose window closed in three steps: 068c667 dropped the
run_baseline.sh step so it stopped being end-to-end and required a trace
produced elsewhere, ca99ad7 removed the backend-drop logic and left
backends_dropped永 empty behind a misleading error string, and #840 emptied the
--backends default so a plain invocation only ever raised. What it was built to
prove is now covered by the orchestrator's own KERNEL phase and ci-e2e.yml.

No in-repo caller; operator confirmed no out-of-repo caller. Its artifacts
parallel_e2e_summary.{json,md} had no reader either -- the breakdown collector
reads reports/kernel_optimization_summary.json and reports/conc_sweep_summary.json
by exact relative path and no glob reaches them.

The deletion also closes an aggregation bug rather than fixing it: every
attempt's exception was flattened into a row-level failed, but the aggregate
printed "status": "succeeded" and returned 0 unconditionally, and never wrote a
status key into the summary on the success path at all. Measured with all
attempts failing: 2/2 failed, top-level succeeded, exit 0.

load_env_file was a second implementation of the credential-alias derivation in
tools/backends/ray_runtime.py:452-472 (same rule, verbatim the same comment).
That one stays and is pinned by seven tests in test_ray_runtime_env.py, four of
which cover the "GEAK aliases are never derived, forwarded verbatim" boundary
the deleted test file covered with three -- so the guardrail is preserved.

test_kernel_agent_live.py keeps its mirrored derivation and now points at
ray_runtime; whether that dormant opt-in harness should live is a separate call.
Coverage is unaffected: the module was already in [tool.coverage.run].omit, so
its tests contributed nothing to the gate; the dangling omit entry goes too.
_collective_names, _io_utils and _paths all keep other consumers.

Co-authored-by: Cursor <cursoragent@cursor.com>
… warnings

Aggregate the dropped-key warning into one line per variant rather than one per
key, collapse a comment that was duplicated verbatim at both unset sites, and
record the new filtering in the docstrings that still promised unconditional
behaviour.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ZhengGong-amd
ZhengGong-amd requested a review from a team as a code owner August 14, 2026 03:12
@ZhengGong-amd
ZhengGong-amd marked this pull request as draft August 14, 2026 03:12
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown

CI E2E report — ✅ Succeeded

item value
result ✅ Succeeded
model Qwen/Qwen3-0.6B (dense)
resources 1× GPU, TP=1
PR branch feat/zgong/explore-opt-17
commit 48ed267e3eb0494707943b0c9935fb5906d72a52
session_id 4ef6d426-a39b-4902-8b98-d59e49e3948c
queue → dispatch 0s
run time 171m 3s
total 171m 3s

details

ZhengGong-amd and others added 21 commits August 14, 2026 06:40
The relaunch passed both --resume and --resume-from. Drop the redundant
flag and re-anchor the pgrep pattern that resolves the resumed pid, so the
monitor keeps working once bare --resume stops being accepted.

Co-authored-by: Cursor <cursoragent@cursor.com>
Bare --resume scanned every model subtree under $USER_DATA_PATH and
attached to whichever session had the newest timestamp, then re-exported
that session's model as MODEL_PATH without a word. On a shared workspace
that silently continues someone else's run on a different model, which is
what SKILL.md already forbids under "never guess by timestamp".

--resume-from becomes the only way to resume, so the session is always
named by the operator. The flag it used to imply is gone; a stale
--resume now fails at argument parsing instead of picking a session.
Also retires the workspace-root fallback (unreachable: the sole
production make_session_dir call always passes a model, so no manifest
ever lands there) and the quantization resume guard (unreachable: the
prelude only runs on the fresh-launch path).

Co-authored-by: Cursor <cursoragent@cursor.com>
SKILL.md forbade guessing a session dir from timestamps in three places
while its own resume section documented exactly that, and described the
scan as model-scoped when it never was. Drop the contradiction and name
--resume-from as the only resume entry point across the skill files,
references, README, and the how-to/reference docs.

CHANGELOG and release notes keep their original wording; they record what
shipped at the time.

Co-authored-by: Cursor <cursoragent@cursor.com>
…d placeholder

The blank-id fallback mapped every empty task/session/kernel id onto a shared
"unknown" directory. Nothing could reach it: registry task ids are uuid4 hex,
and every other call site either substitutes its own placeholder or returns
early, so the bucket had no producer and no reader beyond three assertions.

Keeping it also forced an early return that skipped the traversal check, so a
containment helper carried a bypass branch. Blank is now refused alongside
path-like values, which is the only correct terminal behaviour here: pathlib
swallows an empty segment, so a blank id would resolve to the parent action
directory and collide with every sibling task.

Co-authored-by: Cursor <cursoragent@cursor.com>
…rate_patch

Each of these three sites substituted its own name for a blank task id, but
the id is a registry uuid4 hex that cannot be blank, so the substitutes were
unreachable. Three different placeholders for the same non-existent case only
obscured that the workspace is always keyed by the real task.

The path helper now refuses a blank component outright, so no local fallback
is needed to keep the workspace off the shared action directory.

Co-authored-by: Cursor <cursoragent@cursor.com>
argparse resolves --resume as a prefix of --resume-from rather than
rejecting it outright, so pin the guarantee that matters: every legacy
invocation, including the one already-deployed monitor copies send, exits
2 instead of choosing a session.

Co-authored-by: Cursor <cursoragent@cursor.com>
…used

RunnerContext.task is typed Task and task_id is a required field, so the
getattr default could never fire — it only implied the id might be absent.
The path helper refuses a blank component outright now, so nothing downstream
needs a guard of its own.

Also trims the two comments that grew with the previous change back to the
length they had before it.

Co-authored-by: Cursor <cursoragent@cursor.com>
Trim the duplicated pin comment and the session-discovery chain that the
skill docs already carry, and drop two tests that asserted a symbol was
absent or that argparse round-trips a string rather than any behaviour of
ours. Restores the blank line the quantization guard removal ate.

Co-authored-by: Cursor <cursoragent@cursor.com>
The per-launch dir name was a second-granular UTC timestamp, so two launches
of one model in the same second resolved to one session dir. That name is also
the de-facto session id (session_dir.name feeds KB fact writes, critic requests
and per-session sinks), so a collision merged two runs' identity as well as
their artifacts. Uniqueness now comes from a random suffix; the fixed-width
timestamp prefix keeps lexical order chronological.

Co-authored-by: Cursor <cursoragent@cursor.com>
Subprocesses are spawned with explicit cwds of their own, so a relative
$USER_DATA_PATH sent parent and child to different workspace roots while both
believed they agreed. Absolutising on read cannot fix that — each process
re-expands the relative value against its own cwd — so the launcher now rewrites
the env var once, before the parser defaults or any session path derive from it.

The manifest snapshot drops its own duplicate env read and goes through
workspace_root(), which is the same value with one less way to disagree.

Co-authored-by: Cursor <cursoragent@cursor.com>
…omments

The dir name gained a random suffix but four places still spelled the old
shape: two skeleton/layout comments and the layout tree plus literal session
path example in SKILL.md and references/paths.md, both of which ship as agent
instructions.

Also drops what the code already says: the whitespace-strip the authority
itself does not do, two assertions implied by their neighbours, and the
rationale paragraphs.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ver source

The session-scoped endpoints were keyed off SharedState.session_id, which
build_session_id() generates locally as "{model}_{ts}_{uuid8}". Nothing ever
registers that id with the robustness server -- the client is GET-only and the
platform's claw_session_id is never forwarded to the agent -- so
/api/v1/sessions/{id}/{pods,events,summary} could only ever 404. Those 404s were
folded into empty results, which the source then returned as a successful tick.

Pod discovery already had a working route: the workload_uid hierarchy lookup,
whose rows were merged into session_pods anyway. That is now the only route.

Drops list_session_pods, list_session_events, get_session_summary,
_extract_session_id, the now-unused _to_iso helper and events_limit knob, and
the SourceData.session_events / session_metrics fields, which had no readers.
_merge_pods loses its session-side half and becomes _wrap_hierarchy_pods.

Co-authored-by: Cursor <cursoragent@cursor.com>
The snapshot carried a second, weaker copy of the session id: parsed out of the
prompt text while the authoritative one already arrives on the request and is
handed to build_reactor_components and the emit payload. Its only reader was the
server source's session lookup, removed in the previous commit.

The runtime CLI's fallback existed solely to backfill that field, and rebuilt the
context by hand-listing members. That list was complete when written (the
snapshot had exactly 7 fields) but the snapshot has since grown to 19, so a tick
taking the fallback silently lost stop_reason, the wall-clock budget, validated
gain, tick, macro_cycle and the rest -- muting postmortem finalization and the
deadline wind-down. Deleting the branch outright removes both the shadow field
and the field-loss bug.

Replaces the fallback's vacuous test (it asserted an emit key sourced from the
request, so it passed with the branch deleted) with one that pins the real
contract: every parsed shared-state field reaches the reactor.

Co-authored-by: Cursor <cursoragent@cursor.com>
The symptom read SourceData.session_summary, whose only producer was the session
summary endpoint removed earlier. With no producer left it could never fire, so
the rule, the field, and the HealthConfig slot that existed solely to tune it all
go. SignalSpec already allows a config-less row, so the classifier keeps calling
evaluate_health_signals for pod_not_running with no other change.

Co-authored-by: Cursor <cursoragent@cursor.com>
…othing

A tick that fetched no pods, no faults and no GPU metrics still returned a
SourceData, which the DegradeRouter counted as a healthy primary. The local probe
was then never consulted, so GPU leak, log-error, disk, ray and state-integrity
signals went silent alongside the cluster ones -- and silently, since sources_used
still named the server.

Failing the tick instead lets the router degrade after its usual threshold and
log the transition. Deployments with no workload uid configured will now fall
back to the local probe rather than reporting a clean bill of health.

Co-authored-by: Cursor <cursoragent@cursor.com>
The `failed -> running` retry edge was removed when retries moved to a fresh
idempotency key, but the hand-listed TERMINAL_STATES kept its old membership. A
failed row therefore has no outgoing transition at all yet reports as still
live, and every "is this row finished?" guard reads it wrong.

Three of them wedge. The watermark roofline gate never releases, so a roofline
reclaimed by the TTL watchdog -- which reports no result, leaving the release as
the only thing that could clear the marker -- gates every later refresh; the
marker is persisted state, so the next resume inherits the wedge. Resume
recovery never clears enablement_validation_pending for a failed revalidation.
And the FRAMEWORK authoring pump keeps re-selecting a candidate whose specialist
row is dead, logging a dispatch every tick while nothing runs.

Deriving the set from _TRANSITIONS instead of restating it means the two cannot
drift apart again. proposals.py had already worked around the stale constant
with a private copy that did include failed, so it now imports the shared one;
the import left unused in framework.py goes with it.

The gate's failed branch was never exercised -- the existing test only set up a
succeeded row -- so the two near-identical gate tests collapse into one
parametrized case that pins all five states.

Co-authored-by: Cursor <cursoragent@cursor.com>
…nt log

The pending-integrate recovery treated "no KEEP found" and "could not read
the event log" as the same outcome: a swallowed scan error left kept_res as
None, which fell through to reverse-applying every recorded patch and then
cleared the sentinel unconditionally, so a transient DB error during the
first resume after a hard crash would destroy a validated patch, discard the
attempt venv, report the loss as a fix, and leave nothing for a later resume
to retry.

Track whether the scan completed and bail out with a warning that retains the
sentinel when it did not, so the destructive branch runs only on evidence.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ng it

The GEAK branch never measured anything: it derived a throughput as
baseline_tput * best_speedup and wrote that straight onto current_best, so a
GEMM speedup of 1.1 would overwrite a current_best already carrying an explore
win of 1.5x and rewrite the session's validated cumulative gain as the single
GEMM's speedup. Only forge results reached the per-tuner validator, which
already measures each candidate against current_best via integrate_handler.

Reduce both result shapes to a common candidate list, route every backend
through that validator, and delete the synthetic-throughput promoter. The
validator is no longer forge-specific, so it and its log lines lose the forge
name; stack entries and current_best now carry the backend that produced them.

Co-authored-by: Cursor <cursoragent@cursor.com>
integrate, geak_e2e, forge GEMM, fusion and warm_replay each hand-built their
own current_best and stack entry. Four of them replaced current_best outright,
so a KEEP could drop the cumulative server args or resurrect an env an earlier
ablation had removed, and only _lift_to_current_best merged onto the previous
config or applied unset_envs at all.

All five now build a winner dict and hand it to the lift, which gains an
entry_extra parameter for the artifact and provenance handles the breakdown
collectors join on, plus final_overlay so the active overlay survives a later
winner that carries none. current_best keeps only config; tuned_file, engine,
patch_path, geak_alignment and the rest move to the stack entry or go away
where nothing read them.

Two behaviour changes fall out. integrate and fusion had no anchor guard, so a
KEEP measured below the established recipe used to lower current_best; the
lift now refuses it. warm_replay hand-appended a rounded percentage to
gain_per_stack_entry instead of calling append_stack_gain_entry, so that list
was not computed the same way as every other entry's.

Baseline and profile still assign current_best directly, which stays correct:
baseline only writes it while the stack is empty, and profile copies the
existing config and re-stamps the measured tput.

Co-authored-by: Cursor <cursoragent@cursor.com>
Resume compared current_best against a config replayed from optimization_stack
and, on any disagreement, overwrote current_best with the replay. The replay
was a second implementation of a merge only the lift performs correctly: it
unioned every entry's extra_envs without applying unset_envs, so an env a later
ablation had deliberately removed came back, and resume "repaired" a correct
current_best into a wrong one.

With every config KEEP going through the lift, current_best and the stack are
written together and cannot drift, so the comparison has nothing left to fix.
The one shape still worth reporting is a config with no stack behind it.

Co-authored-by: Cursor <cursoragent@cursor.com>
…eplaying

_materialize_stack_config_for_resume walked optimization_stack to reconstruct
cumulative args and envs, which is the merge the lift already performed on the
way in. Its two remaining consumers -- the GEAK handoff's baseline_env_spec and
the post-resume full-stack rebench -- now read current_best directly, so the
GEAK baseline is built from the config that was actually measured rather than
a replay that unioned every entry's envs and lost ablations. Despite the name,
this ran on the normal path too, not just resume.

What survives is the launch normalization: a "-"-prefixed key under extra_envs
is a server arg, and exporting it as an env would silently drop it. That
routing also had the arg merged as the full replacement string rather than an
addition, so it discarded the args it was meant to extend; it now appends.

values._config_from stays: it slices entries per origin and applies the publish
allowlist, neither of which current_best can express.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ZhengGong-amd ZhengGong-amd changed the title Feat/zgong/explore opt 17 [CLEAN] Correctness sweep across the optimization loop: measurement integrity, phase budgets, resume safety, and a large dead-code removal Aug 17, 2026
ZhengGong-amd and others added 2 commits August 17, 2026 08:05
Twenty conflicts, all in the areas the two sides both rewrote: main's Recipe
KB unification, the combined warm recipe+kernel replay, and the collective
forge lane against this branch's current_best convergence, dead-code removal,
and robustness-server removal.

How the conflicts were settled:

- warm_kernel_kb_plan is restored. This branch dropped it as an intermediate
  with no reader; main's combined promote now reads it back to record which
  kernel columns were replayed, so the premise no longer holds. Its twin
  warm_kernel_kb_outcome stays deleted -- main independently reached the same
  conclusion and routes the outcome through warm_replay_outcome, the field
  that already had consumers.

- The warm-kernel KEEP threshold keeps this branch's decaying curve and gains
  main's non-finite guard. Main's four new call sites now pass state.

- The warm-replay promote keeps _lift_to_current_best. Main's new stack fields
  (inferencex_path, kernel_replay, recipe_patch_statuses, replayed_patch_refs)
  ride along as entry_extra rather than being written onto current_best, which
  stays a pure config record. The promoted checkout is unaffected: its
  authoritative carriers are active_inferencex_path and $INFERENCEX_PATH.

- GEMM tuning keeps the single per-candidate E2E validator. Main's forge-only
  routing condition is subsumed -- the CK block-scale switch it gated is
  already injected by _gemm_e2e_candidates as a candidate of its own.

- _format_last_kernel_opt follows main's move into the render mixin, which
  reads the stable ledger through this branch's compatibility property.

- The robustness factory keeps the local probe as primary and adopts main's
  _build_local_probe_config (which also carries its server_process_patterns
  fix) plus local_processes_known=False on the quiet source.

- The SKILL.md symptom table stays a complete inventory and gains main's
  agent_quiet_work_progressing row and the local_server_unreachable
  suppression note; cluster_fault stays out, having no producer.

Mirrored onto this branch's conventions:

- The collective lane promote (new on main) hand-built current_best, appended
  its own stack entry, and stamped the removed cumulative_gain field. It now
  goes through _lift_to_current_best and _update_cumulative_gain_validated
  like every other config KEEP, which also gives it the anchor guard and the
  merge-onto-previous-config semantics it was missing.

- The collective rollback snapshot no longer captures cumulative_gain.

- Tests carried over from main were retargeted where they pinned removed
  surfaces: SharedState(cumulative_gain=...), SharedStateSnapshot(session_id=),
  current_best["engine"], and the resume pass rebuilding current_best from an
  optimization_stack replay.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main advanced while the previous merge was being resolved. Two conflicts, both
where #1197's launch-shape persistence meets this branch's resume rework.

- cli/__init__.py: take resolve_robustness_options, drop
  _robustness_server_configured. It gated the multi-node auto-downgrade to
  --robustness-mock on a configured robustness-server; this branch removed both
  the server and the downgrade, so the flag and the env it reads no longer
  exist. #1197's own layering (an explicit flag on a resume wins over the
  persisted mapping) is unaffected and is what this branch wants: the monitor's
  auto-resume re-passes nothing.

- SKILL.md: keep #1197's persisted --extra-env wording, spelled --resume-from,
  and this branch's "never runs on a resume" for the quantization prelude,
  which is accurate now that the prelude only runs on the fresh-launch path.

- test_grid_runner's new session-ownership test dropped
  SharedStateSnapshot(session_id=): the rule matches on the LocalHealthConfig
  session dir, and the snapshot no longer carries a second copy of the id.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ZhengGong-amd
ZhengGong-amd marked this pull request as ready for review August 17, 2026 08:31
ZhengGong-amd and others added 3 commits August 17, 2026 08:44
CI on the merged branch surfaced 23 failures. One is a live defect; the rest
are earlier cleanup commits that deleted a symbol and left its callers or its
tests pointing at it.

Live defect:

- The coordinator prompt's acceptance-threshold advisory called
  self._decaying_keep_threshold_pct(), which 27ad677 removed in favour of
  resolve_keep_threshold(state). The delegation entry still routed the name to
  the proposals collaborator, so every render of that block raised
  AttributeError into the prompt builder. It now calls the shared helper, and
  the stale delegation entry goes with it. The dead `if keep is None` guard
  goes too: the curve always returns a float.

Dead call sites (all inside `except Exception: log.debug(...)`, so they failed
silently rather than crashing, once per candidate event):

- Four blocks importing write_decision_json / write_semantic_audit from
  framework.artifacts, whose definitions 78f162a removed. The module
  docstring still advertised both writers.

Tests pointing at removed surfaces. Retargeted where the behaviour they cover
still exists, deleted where the thing under test is gone:

- collect_final replaces the recorder's _snapshot_final for the three
  scriptable-latency assertions. The e2el derivation is asserted through
  _backfill_scriptable_latency, which is where it actually happens -- on save,
  before any collector reads state.json.
- The fusion test keeps the counts the result dict inlines and pins the absence
  of the kernel_sequence.json artifact; the per-cluster breakdown it also
  asserted only ever lived in that unread file.
- The shape-contract tests assert membership in DISPATCHABLE_SHAPE_PROVENANCE,
  which is what the dispatch gate reads now that the wrapper is gone.
- The GPU-lease test keeps its start_async / poll_started coverage and drops
  only the pending_seconds assertions.
- The roofline audit-fields test now pins roofline_attempts and that
  last_roofline is absent.
- Deleted: test_payload_aliases_shim.py (78f162a said the shim and its guard
  test were deleted; only the shim was), the three probe_ssh tests, the four
  infera discover_role_pods / frontend_service_url tests, the
  framework_levers_by_state test, the reset_specialist_dispatched test, and the
  two write_semantic_audit error-path tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
monkeypatch.delenv(name, raising=False) records nothing when the name is
already absent, so it does not arm an undo. The resume pass under test then
exports INFERENCEX_PATH itself, and that value -- a tmp_path that pytest
deletes -- outlived the test and reached every later one in the same process.

The victim was test_profile_executor_extracts_vllm_capture_traces, which then
resolved a dead InferenceX checkout, failed inside the executor, and returned a
failure payload with no "framework" key. It only reproduced under full-suite
ordering, and only through this one predecessor (bisected).

setenv arms the undo whatever the prior state, and the assertion is unchanged.

Two notes for whoever reads that failure next:

- ``SubAgentResult.state`` is "succeeded" whenever the executor returns at all,
  including when its payload is {"status": "failed"}. Asserting state alone
  does not establish that the run worked.
- ``PROFILE_DEFAULT_CONFIG`` freezes ``asset_root()`` at import time, so an
  $INFERENCE_OPTIMIZER_ASSET_ROOT override that is live during the first import
  of profile.py sticks for the whole process. Benign in production, where the
  variable does not move; worth knowing when a path-shaped test goes strange.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Self-review of the two preceding commits. No behaviour change beyond removing
code and a result key that nothing reaches.

Orphaned by the decision.json / semantic_audit.json writer removal:

- artifacts.candidate_slug existed to build runs/framework_agent/<slug>/ for
  those writers and has no production caller left (the slug helper the pump
  still uses is framework_agent._candidate_slug, a different function). It goes
  with its __all__ entry and the test file that covered nothing else.
- The module logger went unused with the writers.

Orphaned by the kernel_sequence.json removal:

- bypass_trace_analysis still computed kernel_sequence_path and published it in
  the result, advertising a file that is never written and that no consumer
  reads.

Orphaned by the pending_seconds removal:

- A test double still implemented the method; nothing calls it.

Redundant:

- The collective promote kept an isinstance precondition on
  gain_per_stack_entry, which guarded a direct append the lift now owns.
- _warm_kernel_keep_threshold_pct had two near-identical warning branches for
  one "override is unusable" case, and a function-local machine_state import
  where the module already imports it at the top.
- Three imports left unused by the test deletions.

Prose:

- Comments that restated the docstring directly above them (GEMM routing) or a
  callee's own contract (the lift) are gone.
- Docstrings that narrated what was removed rather than describing live code
  now state the contract; that history belongs in these commit messages.
- _promote_collective_integrate_keep documents that it is a no-op on an
  already-stacked patch or a lift refusal, which its one-line summary implied
  did not happen.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ZhengGong-amd ZhengGong-amd changed the title [CLEAN] Correctness sweep across the optimization loop: measurement integrity, phase budgets, resume safety, and a large dead-code removal [Stepping] Correctness sweep across the optimization loop: measurement integrity, phase budgets, resume safety, and a large dead-code removal Aug 17, 2026
ZhengGong-amd and others added 5 commits August 18, 2026 02:48
…ape it never renders

The profile/roofline path passed --profiler-config.capture_torch_profiler True.
No vLLM ProfilerConfig has ever carried that boolean: the TraceLens patch adds
capture_torch_profiler_dir, a path Magpie's TraceLens route already supplies.
vLLM rejected the argument outright, so every profile attempt died before the
server booted, trace_analyze stayed empty, and kernel-opt had nothing to
dispatch on. Drop the flag and keep the real detailed_trace_annotation. The
patch sentinel matched the same phantom name as a bare substring, so it also
accepted a tree holding only the upstream _dir field; anchor it on the full
field name.

Robustness parsed the coordinator inbox with a regex demanding payload= right
after topic=, but _format_inbox_event renders a per-topic field set and
delegated_result emits kind/state/error with no payload= at all, dropping
msg_id when a message carries none. Nearly every row was discarded, so the
agent could not see the delegated_result stream proving orchestration was
alive and raised a high-severity stall alert against a healthy run. Anchor on
the fields the renderer always emits, then decode the tail: payload= wins when
present and the summary fields fold in underneath, which the repeated-failure
signal reads. Quoted values match whole so a k=v inside a rendered error string
cannot split it into a corrupt value plus a bogus key.

Co-authored-by: Cursor <cursoragent@cursor.com>
…explore-opt-17

Four conflicts, all where main's new work meets this branch's current_best
convergence and its de-duplication of the journal outcome helper.

Conflicts:

- derive_journal_outcome takes main's was_skipped -> OUTCOME_SKIP branch on top
  of this branch's hoisted `result` local, so the new branch reads the same
  local instead of re-deriving `result_dict or {}` a fourth time.
- The fusion promote keeps _lift_to_current_best and drops target_file from its
  stack entry: #1199 made the unified patch the sole target authority, resolved
  from diff headers at replay. Scoped to fusion, as main scoped it -- the
  collective entry keeps its target_file.
- The warm-replay promote keeps the lift and adopts main's framework_source_root
  spelling for the promoted checkout, and its new workload_compatibility param.
- test_breakdown_exporter_unit takes both sides' new tests.

Mirrored onto this branch:

- #1199 renamed the promoted-checkout keys and dropped the only writer of
  SharedState.active_inferencex_path, leaving its resume guard inert: the
  reader, the active_inferencex_checkout_missing warning, its entry in the
  terminal stop-reason registry, its operator-facing explanation in report.py
  and a test asserting the restore all survive. PRELUDE writes the field again
  so a resume re-points $INFERENCEX_PATH and stops when the checkout is gone.

Fixed here:

- last_roofline was removed as unread, but roofline is one of the five
  _AUDIT_ACTIONS and record_action_attempt sets `last_<action>` by name. On a
  plain dataclass that assignment silently created an undeclared attribute, so
  the row was written in memory and dropped by to_dict on every save while its
  sibling roofline_attempts persisted. Redeclared; the test now pins that both
  mirrors reach to_dict.

- The recorder's sweep fragment replaced the whole section with last_sweep
  verbatim, dropping all_variants -- the shaped variant points only the
  collector can scan off disk. Same defect main just fixed for `session` with
  _merge_session, and this branch fixed for `workload`. Here the fragment adds
  nothing at all: snapshot_state_sections runs immediately after state.save, so
  the collector always reads a last_sweep at least as fresh. Removed, with a
  regression test that fails without it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fd81233 removed _resolve_effective_concurrency for having no production
caller and deleted test_roofline_ceiling.py with it, but that file was the
module's coverage, not just that symbol's: roofline_ceiling fell to 43.50%
(352 of 623 statements), the worst in the tree, and took the repo under the
fail_under gate at 89.84%.

Two files, 55 cases, covering 243 of those statements. The module is pure
arithmetic over a config dir, so these assert the contracts rather than
recorded outputs -- a golden number cannot tell a corrected formula from a
broken one.

PerfModel and the HF metadata it reads:

- The op formulas against the arithmetic in their own docstrings: GEMM 2MNK,
  SDPA's two matmuls, causal halving that applies to prefill and not decode,
  K/V sized by the KV-head count so GQA is visible, and the MoE
  coupon-collector expert count saturating from topk at one token to every
  expert at a large batch.
- Activation and weight precision stay separable, which is what keeps an fp8
  checkpoint from reading its bf16 activations at the weight width.
- compute_roofline_from_perfmodel: the decode rate is the slower of its own two
  ceilings and bound_kind agrees with it, per-op shares normalise to 1, a MoE
  model routes through moe_fused instead of the three dense FFN GEMMs, the
  hardware scales with the GPU count, and every incomplete-input path declines.
- load_model_meta: the safetensors index outranks the shard sizes and a
  malformed one falls back, quant_method outranks torch_dtype, and the MoE
  decomposition charges a token only the experts it activates -- including
  fp4 experts under an fp8 model, whose bytes the global dtype would overstate
  enough to trip the safe-degrade and drop the MoE from the ceiling entirely.

The diffusion arm:

- VAE downscale, the latent-grid token count (FLUX's 2x2 pack, and the default
  when the channel ratio is not a square), and DiT geometry including
  dual-stream blocks counted twice and the sample_size-less path that takes its
  sequence length from the runtime resolution.
- Both ceilings: weights read once per denoising step, linear plus attention
  FLOPs per image, GPU-count scaling, and the degenerate inputs that yield 0.0.

Coverage: 89.90% -> 90.23% locally (7392 -> 7149 missed).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…explore-opt-17

#1194 rebuilds the optimization read model from what producers recorded rather
than re-inferring ownership from phase timestamps, and reaches the same
diagnosis this branch did about unmeasured gain -- from the other side. Five
conflicts, all where those two meet.

Conflicts:

- optimizations.py drops the phase-timestamp inference block for main's
  recorded ownership. phase_at stays in collectors/_common: attribution.py and
  decision.py still share it.
- exporter.py takes main's removal of the v4 canonical-stream integrity
  machinery, and test_breakdown_v4_core.py goes with it. This branch had
  touched neither; the conflict was adjacency to the alias removal above it.
- schema.py restores SCHEMA_VERSION_V2 / V3. This branch dropped them as
  unused, but #1194 gives them a stated job -- telling a recorder-aggregated
  archive apart from a collector fallback -- and documents them in SKILL.md,
  so the premise for removing them is gone.
- _promote_gemm_tuning_keep stays deleted. Main now labels what it produces
  measurement_basis="derived_speedup"; b4ff1b2 removed the path instead,
  because baseline * best_speedup is not a measurement. Every GEMM KEEP here
  goes through _validate_gemm_tuning_e2e, so what it stamps is an e2e_rebench.
  Main's new _journal_gemm_tuning_keep hook is wired into that validator, so
  the journalling survives the promoter it was written against.
- The geak_e2e, forge-GEMM and fusion promotes keep the lift and carry main's
  new source= label onto the validated stamp; cumulative_gain and
  cumulative_gain_provenance stay removed.

Mirrored onto this branch:

- The collective promote keeps main's record_collective_promotion and
  record_session_validation calls. That lane settles its own verdict outside
  the kernel integrate queue, so without them its gain reports as belonging to
  no step -- which is the defect #1194 exists to fix. Adapted to the lift:
  recorded only once the winner is actually lifted, target_file resolved from
  the result rather than a hand-built entry, and one ts minted for the
  validated stamp and both recorder writes so the promotion lands on a single
  instant.

The other paths need no wiring: #1194 put record_session_validation inside
_update_cumulative_gain_validated, which every lift-based promote here calls.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The #1194 merge landed the recorder rebuild, which brought ~154 newly missed
statements in with it (assembler 19 -> 86, instrument 126 -> 203, trace 10 new)
and put the repo back under the gate at 89.98% -- the earlier roofline work
holds, roofline_ceiling is 43.50% -> 82.66% in that same report, but a margin
that thin does not survive a merge this size.

roofline_ceiling, the state-level entry points the pure-function tests did not
reach:

- resolve_runtime_workload: the baseline yaml is the geometry of record and
  state attributes only fill its gaps, concurrency floors at 1, and an
  unreadable yaml still yields a usable workload. gpu_type comes from state or
  the benchmark runner_type, not from a benchmark key.
- compute_roofline_breakdown_from_state on both arms: a serving run reports the
  lower of its two projections, an xdit run routes to the images/sec ceiling,
  and an unreadable model yields the empty breakdown rather than a zero that
  reads as measured.
- The diffusion arm end to end: steps and resolution from XDIT_* with the
  CUSTOM_* aliases an operator workload uses, both ceilings binding together,
  and the memory-only degrade when the DiT transformer config is unreadable.
- select_peak_and_bound treats a zero projection as unknown rather than as
  infinitely slow, so it cannot win the min.

The assembler's fragment merge, which is what decides whether a recorded fact
survives to the archive. A producer writes partial fragments from inside its
own work and a second write of the same entity id merges into the first:

- Nested dicts merge key by key rather than wholesale -- whole-value
  replacement is what drops a sibling the update never mentioned.
- List rows merge on the first recognised nested id, including an id the
  update itself introduced; rows without one append at most once.
- Filling an absent field is not a conflict and says nothing; replacing an
  answer with a different one is reported at its full path, last writer wins.
- The versions and critic/robustness substreams fold into their sections, and
  a directly recorded section outranks the substreams that would rebuild it.

Coverage: 89.98% -> 90.20% (7344 -> 7189 missed of 73321), leaving roughly
150 statements of headroom over the gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ZhengGong-amd and others added 7 commits August 19, 2026 06:22
…lore-opt-17

~130 commits, dominated by the cooperative-cancellation and session-clock work.
It reaches the same conclusions this branch did from the other side -- "a round
the run stopped is not a baseline that failed", "a variant the run reaped is not
a variant that failed" -- so most of it merged clean. Five conflicts.

Reversed here, deliberately: the EXPLORE hours force-exit is back

fdadb65 removed DEFAULT_EXPLORE_FORCE_EXIT_HOURS_REMAINING because any run of
3h or less force-exited EXPLORE on its first tick, and argued the gate was
redundant once charge-back reserves the later phases' share. 07e23b9 found the
same defect and fixed it instead of removing it: _explore_hours_leavebehind_
applies disables the gate when the leave-behind is not strictly smaller than the
session, with disabled_leavebehind_covers_session in the evidence and 56 lines
of new tests.

Main's version is taken. It closes the failure this branch removed it for, it
belongs to a 130-commit effort by the owner of that machinery, and the removal
rested on redundancy rather than incorrectness -- so keeping it costs a
redundant check, while dropping it would delete another team's tested work
mid-merge. should_force_exit_explore and exit_normal_explore carry the
parameter again; a non-positive threshold still disables the gate.

The other two fixes in fdadb65 are untouched: normalize_budget_pct still
keeps an explicit 0.0, and the FRAMEWORK force-exit stays removed.

Other conflicts:

- _grid_runner __all__ takes main's three new stop-attribution exports;
  MULTI_NODE_DEFAULT_KEEP_THRESHOLD_PCT is not among them because 27ad677
  deleted the constant, and exporting it would break the module interface.
- request_handlers keeps main's vendor-playbook deploy-blocked guard and reads
  the stable ledger through _entry_by_kernel_id rather than the ordinal
  kernel_opt_attempts dict (2cb82ad).
- machine_state also takes main's SWEEP closeout helper, so skip_to_close no
  longer maps a refused conc_sweep onto robustness_escalated.
- test_phase_force_exit takes main's cases; one had spliced this branch's
  variable name onto main's new assertions.
- CHANGELOG keeps both sides' entries.

Also drops a duplicate _update_cumulative_gain_validated key this branch had
left in the Coordinator delegation table (same value, so inert, but ruff F601).

Not addressed here, and worth an issue against #1171: integrate_patch does not
consult stop_attribution on either side of this merge, so a session-budget stop
during the GEMM E2E validator files its remaining candidates as REVERT -- a
verdict about tuners that were never measured. Extending that subsystem into
integrate is the author's call, not a merge resolution.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The hard force-exit had two arms: the unspent fraction of EXPLORE's charge-back
budget, and an absolute floor on session hours remaining. The floor was there
from when a phase got a flat max_minutes*pct allotment and could therefore hold
budget the session no longer had. Charge-back rebuilt the allotment from the
time left when the phase starts, which made the floor redundant, and left it
firing on any run shorter than the leave-behind: remaining starts at max_hours,
so a 3h session was already at or under a 3h floor on the first tick.

Guarding the floor against that case keeps a second gate whose every firing the
budget arm reaches on its own, and which grades a phase on time other phases
spent. Removed instead: should_force_exit_explore now reads the phase fraction
alone, and reports fired_reasons unchanged so the evidence shape holds.

The cases the guard protected are still pinned, against the surviving
behaviour: a 3h session reaches EXPLORE with its budget intact both at entry
and after a slow PRELUDE, and the gate fires once the phase has spent its own
share.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Work the run itself stops carries one of two error classes, and the grid runner
already assigns them. integrate dropped the class when it flattened the variant
result, so a re-baseline the session clock or a cancel ended arrived looking
like a bench that simply produced no throughput -- and every ledger downstream
read that as a verdict on the patch.

The class now travels with the rest of the bench result, and the two places
that turn a bench into a verdict consult it first:

- The perf gate returns a stopped result, with the patch and its artifacts
  taken back out of the tree exactly as a REVERT would, but no decision.
- The kernel integrate handler no longer answers REVERT when the re-baseline
  produced nothing because the run ended it; the patch is reverted and the
  result says so without grading it.

The GEMM E2E validator is the ledger this was visible in: it walks tuners one
at a time and files anything that is not a KEEP as a REVERT, so a clock running
out mid-sweep taught the ledger that every remaining tuner had been tested and
rejected. It now stops on the first stopped round, since the cause ends the
batch either way, and records nothing for the tuners it never measured.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Self-review of the two preceding commits.

The stopped results carried fields that either nobody reads or that describe
work that did not happen: ends_the_batch has no consumer on either result (the
GEMM sweep stops on any cause, because it has no fit check to leave the rest
of the batch to), and base_tput / keep_threshold_pct name a grading the round
never reached.

Docstrings that still promised only a verdict now say a round can end without
one: the perf gate, and the GEMM sweep that stops with its remaining tuners
unrecorded.

The operator explanation for explore_force_exit_low_budget described the
session wall-clock, which is what the removed floor measured. It is the phase's
own budget that fires the gate now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three CI failures, all tests carried in from main that name something no longer
there. None of them is about the behaviour it asserts.

- compute_next_phase no longer takes max_hours. Its only consumer was the
  FRAMEWORK hours force-exit, which went when the phase clock started charging
  back; the three SWEEP closeout cases pass it incidentally and assert nothing
  about it.
- _sweep_state names the validated gain, which is the only cumulative gain
  left.
- enablement_round_dir refuses a blank task id rather than mapping every round
  onto one directory. snapshot_round already skips a round that carries no id
  for that reason, so the fallback the test expected has no producer; the
  traversal case and the blank case are now the same parametrized refusal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eady read

The stopped results introduced their own status. Two discriminators key on the
established one instead, and both read the new value the wrong way:
_is_integrate_fault treats "every unmeasured path returns failed" as the signal
that a result is a fault rather than a verdict, so a stopped round was counted
against the REVERT quota it was meant to be kept off; _is_promotable_result
holds anything that is not failed to be promotable, so the same round was
routed through promotion.

Both stop returns now carry status failed, which is what an unmeasured round
has always reported, and leave the cause to error_class -- the channel
stop_attribution exists to populate, and the one the ledgers carry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It fabricated the status the production path no longer returns, and carried
setup the assertions never depend on: a warm-replay current_best, a baseline
runtime the timeout stub makes unreachable, a workspace, a precision, and two
micro-speedups. Verified by defeating the guard and watching it fail.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ZhengGong-amd
ZhengGong-amd merged commit c92784c into main Aug 19, 2026
27 of 28 checks passed
@ZhengGong-amd
ZhengGong-amd deleted the feat/zgong/explore-opt-17 branch August 19, 2026 07:22
iraj465 pushed a commit that referenced this pull request Aug 19, 2026
Resolves the two conflicts in the GEAK promote path, both created by #1188's
correctness sweep landing on the same function this branch extends.

phases/kernel.py, _promote_geak_from_candidate:
  main added a KEEP guard (a rebench that does not beat current_best must not
  overwrite the headline) and dropped the then-unused `provenance` parameter.
  This branch needs `provenance` and `overlay_loaded` to record the adoption
  ledger. Kept both sides: main's KEEP guard is untouched, and the two
  parameters return -- with `provenance` now DEFAULTED rather than required,
  so main's callers in writeback.py and test_geak_gain_alignment.py continue
  to work unchanged.

loop/writeback.py:
  restores the two keyword arguments at the promote call sites, matching the
  signature above.

143 GEAK tests pass, including main's test_geak_gain_alignment.py, which
omits `provenance` and so exercises the new default.
chennyiiis added a commit that referenced this pull request Aug 19, 2026
…rift

The branch was cut before #1188, which deleted _promote_gemm_tuning_keep --
the synthetic-throughput promoter this branch had been carefully teaching not
to stamp self-reported speedups as validated. Rebasing brings that deletion
back, so the two hunks guarding it and the two tests covering it are gone;
#1188 solved the same problem the stronger way, by measuring every candidate
end to end instead of declining to certify a ratio. What survives is the
layer above it: an end-to-end number can be measured correctly and still be
measuring the wrong thing, because the tuned table may never have been read.

The apply-verification test module skipped on the top-level forge package
while production imports forge_gemm_tune.evidence. A box with the package but
not that submodule passed the skip, got a None parser, scored every verdict
'unknown', and failed eleven cases for reasons unrelated to what they test.
Now 44 pass and 14 skip without forge, 58 pass with it.

The env-var-to-table map existed twice in this file, and a name that drifts
makes the apply check compare our deployed file against the wrong table,
conclude the artifact never arrived, and revert a candidate that was fine.
One constant now, with a test pinning the third copy in KernelForge wherever
it is importable.

AITER_LOG_TUNED_CONFIG and HYPERLOOM_GEMM_PAIRED_PAIRS are documented. The
first changes the environment of every serving run, so an operator wanting to
turn it off has to be able to find out it exists.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants