fix(forge-gemm): MoE runtime key, E2E faults, warm-replay nogit, model path - #1238
Merged
Conversation
The regex meant to read aiter's fused-MoE dispatch tuple never matched a real log line: it expected six leading integers, while every actual line starts with a quoted gfx name. Its own fixture omitted that field, so the pattern passed its tests while the dtype gate it feeds silently degraded to 'always allow' in production -- combos was always empty, so the bf16-times-fp4 rejection it exists to enforce never fired.
Anchor the tuple on ' for (' instead of the wording, which also covers the two other forms aiter emits, including the one that interposes its own parenthesised kernel names. Verified against 2948 real lines across three sessions: 2948 matched. Fixtures are now verbatim log lines for exactly this reason.
Use the parsed tuple as the MoE tuning input. The quantisation pair, the per-partition inter_dim and the EP path's inflated expert/topk are all runtime properties that no derivation from the model config recovers. Rows whose dtype pair aiter's codegen refuses are dropped, because one such row aborts the entire tuner run; the per-problem filter is why the gate can now ask whether any problem is tunable rather than blocking the whole model on the worst one.
Co-authored-by: Cursor <cursoragent@cursor.com>
Stop forge GEMM E2E from recording boot/measurement failures as REVERT with 0% gain; route them through e2e_results.faults instead. Teach the tuned-config coverage check to match MoE dispatch tuples from server.log against fmoe CSV rows rather than treating every MoE table as empty. Co-authored-by: Cursor <cursoragent@cursor.com>
Give each forge GEMM tuner up to _MAX_INTEGRATE_FAULT_ATTEMPTS integrate attempts before recording an integrate_fault, matching the kernel_opt fault retry semantics. Repair the stopped-run unit test that was merged into the bench-fault case during the prior edit. Co-authored-by: Cursor <cursoragent@cursor.com>
Warm-replay patches now fall back to the shared nogit patch applier for non-git install trees and unborn git repos, so enablement overlays reach runtime instead of failing with missing_git_head. Co-authored-by: Cursor <cursoragent@cursor.com>
The gemm_tuning breakdown now prefers e2e_gain_pct when present so KEEP runs report the validated end-to-end delta rather than the micro benchmark speedup alone. Co-authored-by: Cursor <cursoragent@cursor.com>
Forge KEEP now copies fmoe and dense tuned tables into the serving aiter config tree and snapshots them together, instead of hardcoding only the a8w8 blockscale env key. Co-authored-by: Cursor <cursoragent@cursor.com>
…grate Add shared session model-path precedence with HL_MODEL_BASE and HF-cache fallback, re-export MODEL_PATH at CLI startup, and pass the resolved path into forge GEMM E2E integrate calls. Co-authored-by: Cursor <cursoragent@cursor.com>
Baseline, explore, integrate, sweep, and conc_sweep now share the same params -> MODEL_PATH -> state precedence and serving-path normalization. Co-authored-by: Cursor <cursoragent@cursor.com>
CI E2E report — ✅ Succeeded
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| _log(tmp_path, REAL_2STAGE_DEFAULT), [4, 512], tmp_path / "ws" | ||
| ) | ||
| rows = [ | ||
| line for line in open(path, encoding="utf-8").read().splitlines() if line |
| ) | ||
| path, report = krh._write_fmoe_untuned_csv_from_log(log, [8], tmp_path / "ws") | ||
|
|
||
| body = open(path, encoding="utf-8").read() |
|
|
||
|
|
||
| def test_persist_copies_dense_and_fmoe_together(tmp_path, monkeypatch): | ||
| aiter_pkg = _fake_aiter(monkeypatch, tmp_path) |
| direct = Path(text).expanduser() | ||
| if direct.is_dir(): | ||
| return str(direct) | ||
| except OSError: |
| try: | ||
| if candidate.is_dir(): | ||
| return str(candidate) | ||
| except OSError: |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| _log(tmp_path, REAL_2STAGE_DEFAULT), [4, 512], tmp_path / "ws" | ||
| ) | ||
| rows = [ | ||
| line for line in open(path, encoding="utf-8").read().splitlines() if line |
| ) | ||
| path, report = krh._write_fmoe_untuned_csv_from_log(log, [8], tmp_path / "ws") | ||
|
|
||
| body = open(path, encoding="utf-8").read() |
| direct = Path(text).expanduser() | ||
| if direct.is_dir(): | ||
| return str(direct) | ||
| except OSError: |
| try: | ||
| if candidate.is_dir(): | ||
| return str(candidate) | ||
| except OSError: |
|
|
||
|
|
||
| def test_persist_copies_dense_and_fmoe_together(tmp_path, monkeypatch): | ||
| aiter_pkg = _fake_aiter(monkeypatch, tmp_path) |
| """Return the framework root whose tree holds the warm-replay patch targets.""" | ||
| if not params.get("patches"): | ||
| return "" | ||
| from .integrate_patch import _resolve_framework_root |
GNU patch honours the `index <old>..<new>` header and reads an all-zero old blob hash as a file creation, so a modification hunk written as `index 0000000..1111111` alongside `--- a/path` was refused with "which already exists!" at every strip level. git apply ignores the header, so such patches applied through the git channel and failed only through the nogit one -- surfacing as a bad patch rather than a header disagreement. Specialists emit placeholder hashes rather than real blob hashes, which makes this shape common. Drop index headers that contradict their `---` header before handing the patch to the CLI; genuine creations (`--- /dev/null`) keep theirs, and a patch whose context truly does not match still fails closed. Fixes the two warm-replay nogit tests that only run where the patch CLI is present, and were therefore green on Windows but red on Linux CI. Verified against real GNU patch 2.7.6 on Linux: both CI scenarios apply and revert cleanly, real creations are untouched, and a mismatched-context patch still fails closed. Co-authored-by: Cursor <cursoragent@cursor.com>
Brings the branch base forward 45 commits. Picks up the docs header-button and project-metadata fixes (4be7e9d, f17222c) that the sphinx html job needs -- without them sphinx_book_theme's add_source_buttons raised "cannot unpack non-iterable NoneType" and the build failed on this branch while passing on branches with a newer base. All five overlapping files (cli/__init__.py, baseline.py, request_handlers.py, phases/kernel.py, test_coordinator_gemm_promote_units.py) merged without conflict. Verified: the change-related suites report 5 failed / 183 passed / 11 skipped. All 5 failures are pre-existing and reproduce identically on both the pre-merge branch head (3 failed / 62 passed on the two affected files) and on pristine origin/main (3 failed / 47 passed on the same two files); the remaining 2 are the Windows-only missing-patch-CLI cases. No regression. Windows cannot collect test_coordinator_gemm_promote_units.py after this merge because main's recipe_kb/local_store.py imports fcntl, which is POSIX-only; that suite is covered by CI on Linux. Co-authored-by: Cursor <cursoragent@cursor.com>
Brings the base forward 49 commits, onto 4731f47. That head is the #1230 merge, which carries the docs/conf.py repository-key fix the sphinx html job needs -- the job failed on this branch only because sphinx-book-theme's get_repo_parts() returned None and its caller unpacked it unconditionally. One conflict, in test_geak_breakdown_unit.py: both sides appended tests at the end of the file (26 lines here, 380 on main). Pure append collision, no shared semantics; both sets are kept. Verified against pristine 4731f47 on the same seven suites: main reports 7 failed / 168 passed / 7 skipped, this branch 5 failed / 196 passed / 11 skipped. The failures here are a strict subset of main's -- the two warm-patch cases main fails are fixed on this branch -- so no regression. Remaining failures are Windows-only: no patch CLI, and main's cli/kb.py imports fcntl. Co-authored-by: Cursor <cursoragent@cursor.com>
Four guards, all on paths this branch introduced or newly made load-bearing. Every one turns a failure to reach a verdict into "undetermined" instead of into a verdict, which is the same conflation the branch exists to remove: a measurement that never happened must not read as a measurement of zero. * E2E validation is now guarded as a whole. Both entrypoints wrap only the tuning call, so an exception from the validation that follows -- server restarts, log parsing, CSV merges -- took the KERNEL phase down over a candidate that had simply gone unmeasured. It now records an ``e2e_validation_exception`` fault and lets the phase continue. * The tuned-config coverage report no longer answers when it cannot read its own artifact. An unreadable or schema-shifted CSV yields no keys, which the report scored as 0% coverage -- and 0% blocks a KEEP. That let a corrupt file revert a candidate whose throughput genuinely improved. An empty key set now returns "undetermined", matching how an empty request set was already handled. A readable CSV whose keys miss still reports 0%, so the real check is unweakened. * That report also gets the blanket guard its sibling ``_gemm_apply_verdict`` already carries, plus a safe mtime helper: sorting logs by mtime races the round still writing them, and an ``exists()`` check does not close the window. * Writing the MoE untuned CSV tolerates a full disk. ``mkdir``/``write_text`` were unguarded, so an OSError escaped into the tuning run; the dense tuners take their shapes from elsewhere and can still deliver something. Forge's model-path probe now matches bootstrap's. Bootstrap walks HL_MODEL_BASE and the hub cache and falls back to the raw string, so a repo id the running server resolved fine was rejected here by a hub-cache-only probe. Unresolvable input is now ``skipped`` rather than ``failed``: forge never ran, so it has no verdict, and reporting one spends a REVERT on an experiment that never started. Tests: the coverage and E2E guards are pinned by mutation (reverting either one fails 3 and 2 cases respectively). Affected suites hold at 5 pre-existing Windows-only failures against pristine origin/main, with 12 added cases. Co-authored-by: Cursor <cursoragent@cursor.com>
zengleixin-amd
marked this pull request as draft
August 20, 2026 09:44
… model Two independent gaps, both where a value was recomputed instead of read back. The breakdown decides whether a GEMM run was adopted by matching the history row's ``tuned_file`` against the artifact the optimization stack recorded. Forge reports per-tuner envs and never set that field, so the history row carried "" -- and the stack lookup skips empty keys, so no forge KEEP could ever match. Across 419 real forge attempts in hyperloom-claw, all 419 had a null ``tuned_file`` and every one of the 20 KEEPs was reported unadopted, including runs measuring +20%, +33% and +49%. It cannot be reconstructed either: one KEEP is described by three different path strings -- the durable copy in aiter's config tree, the tuner-workspace original, and the E2E merge product -- so the fix reads back the one the stack entry actually holds, taking the newest GEMM entry because an older one names a previous run's artifact. The same lookup also gates "prefer the stack's validated gain", which had therefore never fired. Separately, the framework bench resolved its model path with a local ``params -> $MODEL_PATH`` two-step while the other five executors use the shared resolver. That skipped both the ``SharedState`` fallback and, more importantly, the serving normalization that walks HL_MODEL_BASE and the hub cache -- so a bare repo id went straight to a server it cannot authenticate against. Its single caller already had the shared state in scope. Tests: the backfill helper is pinned by mutation (reading the oldest stack entry instead of the newest fails a case). Two cases in the coordinator suite assert the history row and stack entry name the same artifact on KEEP and that a REVERT claims none; those run on Linux CI only, since Windows cannot import that module (main's cli/kb.py imports fcntl). Co-authored-by: Cursor <cursoragent@cursor.com>
aiter merges every ``model_configs/*{table}*.csv`` it can glob whenever the
matching env var is unset, which is the ordinary case for a plain server start.
The durable copy landed directly in that directory, and it landed during the
micro phase -- before E2E has ruled on the candidate. So a candidate E2E went
on to reject still reached every later server: the verdict read REVERT while
the table was silently in effect, poisoning the baseline that subsequent gains
are measured against, and persisting across sessions because it lives in the
installed package rather than the session.
The scan does not discriminate by model either. A real V4-Flash run merged
dsv3's table, so one model's tuning reaches another's serving.
Replay does not need the scan: it restores the env var explicitly from
``e2e_results.kept[].env_var`` and defers a GEMM column that has no env at all
(``prelude._warm_kernel_extra_envs``). The auto-merge was a side effect, not the
mechanism durability relies on -- so moving the copy one level down costs
nothing and ends the leak. The glob is not recursive.
Verified against the installed aiter on gfx950: a probe in the subdirectory is
absent from both aiter's own selection expression and its merge banner, while
the same filename directly in ``model_configs/`` is picked up by both -- so the
scan is live and the subdirectory is genuinely out of its reach.
Co-authored-by: Cursor <cursoragent@cursor.com>
The session already knows its isl -- the GEAK GEMM path passes it -- but the forge path never did, so forge fell back to inferring one from the token coverage list. That list is capped by conc (about 512 at conc=64), so a long-context arm was described as a workload roughly 16x shorter than it is. What that costs is not a missing top band: forge's ``conc * 128`` term reaches the 8192 ceiling regardless. It is budget spent tuning mid bands (512, 2048) the workload never prefills in, on a shape list forge then trims to fit the window -- so the wasted entries can displace ones that matter. Sent only when actually known. ``SharedState.isl`` defaults to 0, and at 0 the option is omitted rather than sent as a zero, which keeps forge on its existing inference and keeps the command accepted by forge builds that predate the option. Pairs with the KernelForge side that adds ``--isl``; without it click rejects the unknown option, so the two land together. Co-authored-by: Cursor <cursoragent@cursor.com>
Bypass trace analysis can write a weighted, variant-discriminating TraceShapeManifest, and forge has accepted --shapes-manifest since it was added, but nothing ever passed it: the manifest was produced and dropped. Resolve it from the latest trace analysis (the artifact_paths entry, the trace_shape_manifest block, or beside the candidates file) and forward it. Resolution is fail-open at every step -- a missing, unreadable, or wrong-kind file yields "", leaving forge on demand, shapes JSON, untuned CSV, or config derivation exactly as before. The manifest stays behind two gates in practice: its production is opt-in via HYPERLOOM_TRACE_SHAPE_MANIFEST, and forge short-circuits on demand before reaching it, so a serving log carrying GEMM misses still wins. This wires up the fallback rather than changing that precedence. Co-authored-by: Cursor <cursoragent@cursor.com>
…e exists" This reverts commit 660973d. Wiring the manifest up bought almost nothing and cost three safety checks. It buys almost nothing because forge short-circuits on demand before it ever reaches the manifest, and the demand file is derived from the serving log Hyperloom already passes -- so on any run whose log carries GEMM misses (the case this whole line exists for) the manifest is never read. The bf16 dense tuner does not read it at all. The cost is that the resolution skipped guards its sibling shape sources honour. It did not take require_fresh_profile, so a manifest from another workload arm could be handed to the tuner; it sat before the framework != vllm branch, making a trace manifest a shape source on the one framework that branch exists to keep trace evidence out of; and it bypassed _align_forge_shapes_for_aiter, so a freshly captured, workload-matched, dispatch-aligned shape list could be silently replaced by an unaligned one -- which then tunes rows no runtime lookup reaches. Hardening it would mean three additions guarding a path that is dead in practice. Reverting is the smaller surface and the honest one. Forge keeps accepting --shapes-manifest, so nothing has to change there if this comes back with the guards and a reason to trust it over demand. Co-authored-by: Cursor <cursoragent@cursor.com>
…ngth" This reverts commit 31f6cd3. Two reasons, either of which is enough on its own. It breaks the whole forge GEMM lane on any deployment whose forge predates the matching option. click rejects an unknown option and exits 2, forge never prints its sentinel, and the run is recorded as a tuning failure with no error_class -- indistinguishable from a crash while measuring. The trigger is not an edge case: ISL comes from the workload env and defaults to 1024, so isl > 0 holds on every real workload. The shared KernelForge checkout on the serving box is on main at 6cdc7c4 and has no --isl (--shapes-manifest greps 11 hits there, --isl none), so this is the state a merge would land into, and nothing here makes Hyperloom wait for the other half to arrive. The option may also be the wrong shape to begin with. Prefill step M is the chunked prefill size, not the input length, and Hyperloom does configure --chunked-prefill-size. Where that setting is below ISL, declaring the ISL collapses the mid bands (min(isl, 8192) folds them onto one) and leaves a row at M=8192 that the runtime never looks up -- tuning wasted, by a different route than a wrong key. Sizing prefill bands from the workload is still worth doing. It needs the value the scheduler actually batches, and a forge that accepts it. Co-authored-by: Cursor <cursoragent@cursor.com>
Reading it back off the stack could name the wrong round's file. _lift_to_current_best skips the stack append when (action, variant_name) already matches, and a GEMM variant is named <backend>_<tuner>. So when a second macro cycle re-tunes the same tuner and keeps it, nothing is appended and the newest gemm_tuning entry still describes round one. Taking the artifact from there had the second attempt claim the first one's path, and the breakdown then credits it with the first one's gain -- the same misreport the backfill was added to remove, pointing the other way. The value never had to be looked up. The E2E loop already holds it when it builds the stack entry: it is the candidate's env var, or the only value its env carries. _candidate_tuned_file returns exactly that, and both the stack entry and the attempt row take it from the same call, so they are the same string by construction. _adopted_tuned_file is gone. The earlier reasoning was half right: one KEEP really is described by three different path strings (durable copy, tuner workspace, E2E merge product), so re-deriving it does fail. The wrong step was concluding that the stack had to be read -- the way out is to take it from where it is already known. Verified by mutation, which also caught a hole in the test: dropping the env_var preference left every case passing, because the expected path happened to be the dict's first value and the fallback returned it anyway. With the target key moved off the front, the mutation fails the case it should. Controlled comparison on the four affected suites: 6 failed / 383 passed before, 6 failed / 385 passed after, the same six Windows platform failures. The false-claim path itself is covered in test_coordinator_gemm_promote_units, which cannot be collected on Windows (recipe_kb imports fcntl), so that case is exercised by Linux CI only. Co-authored-by: Cursor <cursoragent@cursor.com>
The reason this lane exists is that MoE tuning keyed on the model config wrote tables no runtime lookup could reach, so the key has to come from the dispatch tuple the runtime logged. Both ends of that were covered -- the CSV writer in test_gemm_bf16_aiter_routing, KernelForge's preference for a caller-supplied CSV in test_fmoe_ck -- and the handoff between them was not covered at all. Measured with mutations against the previous suite: setting the payload field to "", removing the derivation from the log, removing the caller-CSV existence check, and removing the argv option each left every test passing. The string moe_untuned_csv did not appear in a single Hyperloom test payload. Deleting this feature's plumbing outright was a green run. Four handler-level cases now assert the chain: the derived CSV exists and its fields equal the logged tuple rather than anything derivable from the config (inter_dim sharded, both quant dtypes, quant type); the CSV the payload names is the one written to the workspace; a caller-supplied CSV wins; and a path that no longer exists falls back to deriving from the log instead of being forwarded dead. Two tool-level cases assert the option reaches forge's argv, and one asserts its absence when no key was observed. test_build_cmd_maps_all_options also stops overstating itself. It claimed to map all options while asserting 10 of the ones _build_cmd emits, which is how the MoE option went unasserted while being the point of the lane -- it now checks --untuned-csv, --kernel-signature-log, --tp, --conc and both timeouts too, and a meta case fails when _build_cmd emits a flag this file does not declare, so the next omission surfaces here. All four mutations are now caught (4, 3, 1 and 1 failures respectively). Controlled comparison: the 7 failures in the affected suites are identical before and after, all of them the Windows-only fcntl / patch / path-separator platform limits. Co-authored-by: Cursor <cursoragent@cursor.com>
forge reports seven micro_decision wordings. The bridge to the coordinator schema handled four, so partial_failure, empty_output and partial_output left decision unset and status at "ok" -- which in the breakdown is exactly what a genuine no_improvement looks like. Those three wordings exist to draw that distinction, and the envelope was erasing it. Two of them are worse than a lost label. partial_failure means one tuner crashed while another delivered, and partial_output means a tuner wrote fewer rows than it had shapes for -- the rows it wrote are deployable. Both arrive with a recommended_env, and both were dropped on the floor: no decision, no requires_e2e_validation, so a usable artifact was never measured. Bridging on "delivered an env" rather than on the single word candidate is what those cases needed. The barren wordings now reach the envelope as an error_class. A crash, a tuner that wrote zero rows, and a partial run whose survivors produced nothing are three different outcomes, and none of them is an honest no_improvement -- which stays deliberately unadorned, because the others are only legible against it. Separately, a tuner that named its own failure was invisible above itself. The jsonl audit row already lifted error_class out of tuners_run, with a comment saying a crashed run and a barren one otherwise look alike; the breakdown and the optimization stack read the envelope instead, where a run with every tuner crashed arrived as status="failed" plus two empty strings. The same lift now happens on the envelope, before the bridge, so a specific class outranks the generic wording and the message travels with it. Mutation-verified, five for five: re-gating the bridge on candidate fails 2 cases, removing the error_class for barren wordings fails 1, dropping either tuner lift fails 1 each, and letting no_improvement acquire an error_class fails 1. Controlled comparison across four suites: 8 failed / 422 passed before, 8 failed / 428 passed after, the same eight Windows platform limits. Co-authored-by: Cursor <cursoragent@cursor.com>
Found while tracing the previous commit end to end. tuners_run is forge's own
JSON, so its shape is not guaranteed, and the two loops that lift an error out
of it iterated it directly: a scalar there raises TypeError, which the caller's
catch-all turns into "the tuning run failed" with a Python exception name for a
cause. That is precisely the misattribution this lane exists to remove, and the
previous commit introduced it while fixing a neighbouring instance of it.
Coerce a non-list to empty before either loop. The verdict still lands and the
cause stays honest.
Two cases added from the same trace. One pins the combination the change makes
reachable: partial_failure with an env and a crashed sibling now yields KEEP,
requires_e2e_validation, the crashed tuner's error_class, and a status that is
not "failed" -- promotability is decided on status, so a named crash must not
demote a run that delivered. The other feeds four malformed tuners_run shapes
(scalar, string, dict, list with non-dicts) and asserts a verdict still lands
with no exception class as the cause.
Also traced the downstream of the previous commit and found nothing else to
change. _should_run_bf16_dense_gemm_fallback rejects on micro_decision before it
ever reads extra_envs, so routing partial_* through the KEEP branch cannot
disturb it. _gemm_e2e_candidates filters on `status not in ("ok",
"partial_output")`, so the crashed tuner inside a partial_failure is excluded
while its delivering sibling becomes a candidate -- which is what makes routing
that wording to E2E correct rather than merely permissive. error_class has no
decision-making consumer on this path: the collective handler's check at 5146 is
a different lane, and promotability keys on status.
Co-authored-by: Cursor <cursoragent@cursor.com>
… KEEP
Two defects found reviewing this branch, both on the MoE path this lane exists
to fix, and both invisible to the tests because the tests disagreed with the
caller.
The token column was wrong or fatal. _write_fmoe_untuned_csv_from_log annotated
tokens as list[int] and iterated it, but its only production caller builds them
with _normalize_tokens, which returns forge's comma-separated string. A real
multi-token workload reached int(',') and lost the whole MoE tuning to a
ValueError the envelope then reported as a forge crash; a single-token workload
silently wrote each digit as its own token, producing rows no runtime lookup can
reach. Measured: '1,32,64' raises, '64' yields tokens 4 and 6. Every existing
case passed a list, so the suite agreed with the annotation rather than with the
caller. _fmoe_token_list now accepts either shape and drops unparseable or
non-positive entries instead of raising -- this is a token sweep for a tuning
input, and one bad entry is not worth the run.
Coverage treated the token count as part of a problem's identity. The tuner
sweeps token and emits one row per batch size it chose; the runtime asks for
whichever batch size it is running. Requiring them to be equal made a table that
does serve the problem report zero coverage, and a zero there goes into
apply_blockers and vetoes a KEEP whose throughput really improved -- exactly the
misjudgement this module was added to prevent, reproduced on the MoE path. The
two constants in this change disagreed with each other about it:
_FMOE_SHAPE_FIELDS omits token with the comment "which the tuner sweeps", while
_FMOE_DISPATCH_COLUMNS included it "because the tuner emits one row per swept
batch size" -- the same fact, read as the opposite conclusion. Identity is now
_FMOE_PROBLEM_COLUMNS; _FMOE_DISPATCH_COLUMNS keeps its job of locating and
validating a row's fields.
Mutation-verified, four for four: iterating tokens as a list, dropping the string
branch, and keeping non-positive tokens each fail a case, and putting token back
into the identity fails three. Controlled comparison across five suites: 8 failed
/ 465 passed before, 8 failed / 468 passed after, the same eight Windows platform
limits.
Co-authored-by: Cursor <cursoragent@cursor.com>
…ead branch Two review findings, one of them mine. The guard around _validate_gemm_tuning_e2e recorded the fault and left the envelope alone. But the forge bridge had already stamped decision="KEEP", requires_e2e_validation=True and the raw combined recommended_env on the strength of the micro result, and the normal exit of validation rewrites all three precisely so Orchestration never sees an unmeasured candidate and issues a bundled integrate against it. An arm that raised was not measured, so it now reads as REVERT with the envs cleared and micro_decision naming the exception. The fault record stays: the point is that the reason is legible, not that the run looks clean. The other finding is a branch I added a few commits ago that cannot execute. _FORGE_DELIVERING_MICRO_DECISIONS listed partial_failure and partial_output on the theory that either can arrive with a deployable env. forge's build_report checks has_candidate ahead of both, so any run that produced an env reports "candidate" instead -- verified against the real build_report across seven scenarios, including "one tuner crashed while another delivered", which reports candidate, not partial_failure. The branch was unreachable, its comment described a scenario that cannot occur, and two tests asserted an input forge never emits while passing. Reverted to gating on candidate; the wordings still get their verdict and their error_class through the barren branch, which is where they actually arrive. The surviving case now pins the real contract, and the crashed-sibling case is expressed as what forge really sends. Worth recording: those two tests were the same defect I had flagged in someone else's fixture hours earlier -- an assertion whose input the producer cannot generate. Passing tests were the reason it went unnoticed. Mutation-verified: dropping the decision reset, keeping requires_e2e_validation, or leaving the stale envs each fail a case. Suites: test_gemm_shape_coverage 38 passed, the bridge cases 7 passed. Co-authored-by: Cursor <cursoragent@cursor.com>
The comments I wrote around these fixes were carrying the commit messages. _fmoe_token_list had an 11-line docstring over 18 lines of code, most of it retelling the defect; _FORGE_BARREN_MICRO_DECISIONS spent 6 of 11 lines on how the contract was verified; several test docstrings ran 6-9 lines explaining consequences already argued in the commit that introduced them. Each now states the constraint a reader cannot get from the code -- that build_report checks has_candidate first, that token is swept by the tuner and therefore not an identity, that validate is inside the guard because it now runs int() over raw config -- and nothing else. No behaviour change; suites and lint unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
…efore Two defects, both from routing the required timeline through the nogit applier. prelude promotes that tree only against a pre_sha and a git snapshot manifest. nogit produces neither, so a replay that measured successfully then failed downstream with validated_recipe_checkout_incomplete, and the rollback that followed did not recognise nogit's backups either -- a path that can reach set_stop_reason. main refused up front with missing_git_head; that guard is restored, and nogit keeps serving the legacy list, where nothing downstream needs a sha. The revert on the way out was also lost. main exempts a required timeline from the finally-block revert because prelude promotes the tree after baseline returns; that exemption was dropped from both revert sites, so the tree handed over was clean and the replay silently disappeared. Restored at both. The two nogit tests asserted the goal this removes, so they now assert the contract that holds: both a non-git install tree and an unborn repo refuse with missing_git_head and leave the tree unpatched. A third keeps nogit covered on the legacy path. Two of the three need no patch CLI, so they run on Windows too -- the old pair skipped there, which is why the gap was never seen locally. Scope note: an integral revert of the nogit commit was tried first and rejected. It auto-merged cleanly but broke two tests that came from origin/main (063a49c) and depend on the refactor that commit also carried -- the merges in between had brought main's baseline.py changes in, so reverting produced a state that was neither main nor this branch. Only the two defects are addressed here; the refactor stays. Suites: 2 failed / 92 passed, the two being the pre-existing Windows patch-CLI failures, identical to before the change. Co-authored-by: Cursor <cursoragent@cursor.com>
Mutating the nogit branch of the revert trigger and of _revert_warm_patch_state left the whole warm suite green, so widening the trigger to (pre_sha or nogit_backups) was unguarded: a nogit apply has no sha, and skipping its revert leaks the patch into later tasks that reuse the same checkout. Cover both ends -- apply records the backups, revert restores from them. The apply-side case needs the patch CLI and skips on Windows; the apply/revert round-trip was verified against a real POSIX patch on Linux (modify, create, multi-file; tree byte-identical after revert). Co-authored-by: Cursor <cursoragent@cursor.com>
main grew its own fused-MoE coverage check (PR #1245) while this branch carried an independent one. git merged both happily because they sit at different offsets in the same module, which is worse than a conflict: our fmoe_tuned_config_coverage and _normalize_fmoe_field were defined second and silently shadowed main's, dropping its kernelName check, q_dtype/act_type normalisation and merged-CSV resolution with nothing to show for it. main's implementation wins. Removed our _FMOE_DISPATCH_COLUMNS, _FMOE_PROBLEM_COLUMNS, fmoe_dispatch_key, tuned_fmoe_csv_keys, the duplicate _normalize_fmoe_field and the duplicate fmoe_tuned_config_coverage, and routed fmoe_ck to main's _fmoe_tuned_config_coverage from inside _gemm_tuned_config_coverage_impl -- so it also inherits our wrapper's degrade-to-undetermined guard. Dropped our token-agnostic matching. Real dispatch logs plus aiter's own lookup settle it against us: aiter normalises token to nextPow2 and only falls back across tiers above 32768, so token really is part of the key. Of 15 logged dispatches, 7 (token 1, 2 and 1024-16384) have no CSV row and genuinely ran default -- main's strict key reports that honestly, and excluding token would have hidden it. Kept what main does not have: the coverage wrapper's exception degradation, the dense fail-open when a CSV yields no keys (main still reports 0% there and lets an unreadable artifact veto a KEEP), and the MoE runtime-key extraction feeding --moe-untuned-csv. Retargeted the fail-open tests at the dense path they now guard. Co-authored-by: Cursor <cursoragent@cursor.com>
Both nogit tests I added asserted result["status"], but only the required-timeline path returns a dict; the legacy path has returned the applied list since #808. On Linux both raised TypeError. They passed locally only because they skip without a patch CLI, and this box had none on PATH -- so the suite was green here and red in CI. Git for Windows ships patch.exe under usr/bin; with that on PATH the file runs 33 passed / 1 skipped instead of 31 / 3, and these two now actually execute. Co-authored-by: Cursor <cursoragent@cursor.com>
zengleixin-amd
marked this pull request as ready for review
August 21, 2026 09:24
xiaofei-zheng
approved these changes
Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
feature/leixin/forge-fmoe-runtime-key).e2e_gain_pctin breakdown, and unify model-path resolution across session executors.feature/leixin/forge-fmoe-runtime-key(4 commits: MoE runtime key, separate MoE untuned CSV field, fail-closed keying, DeepSeek-V4 dense shapes).Test plan
test_warm_patch_apply.py(28 passed, 3 skipped on Windows without POSIX patch)test_forge_gemm_durable_persist.pytest_model_path_resolver.pytest_geak_breakdown_unit.py::test_collect_gemm_tuning_prefers_e2e_gain_over_micro_speedupTestValidateForgeGemmTuningE2E,test_gemm_shape_coverage.pyMODEL_PATHresolves viaHL_MODEL_BASEfor repo-id sessions (#100200 class)EOF