fix(kernel): default integrate base_tput to current_best, not raw baseline - #1074
Merged
Merged
Conversation
…eline _fill_integrate_defaults_from_state filled the integrate base_tput from state.baseline_tput, so the kernel/fusion integrate KEEP gate judged a candidate against the raw baseline instead of the current best recipe it stacks onto. A kernel/fusion that beats baseline but REGRESSES vs the established best (e.g. a warm-replay recipe) was therefore KEEP'd instead of REVERT'd, dragging the final recipe down while the attribution ledger recorded a negative "kernel gain". Observed on three sessions where a forge_fusion entry was adopted at negative gain vs current_best while still positive vs baseline: Mixtral-8x7B -7.46% (recipe 10.66% -> 3.21%), gemma-4-26B -6.72%, Qwen3-0.6B -3.73%. Prefer current_best.tput as the base_tput fallback (mirroring integrate_patch's rebind to the live stack top); fall back to baseline_tput only before any current_best exists. Explicit payload base_tput still wins. Adds a regression test for current_best precedence plus the no-current_best baseline fallback; updates the existing all-defaults test which encoded the baseline behaviour.
CI E2E report — ✅ Succeeded
|
chaojhou
approved these changes
Aug 2, 2026
ZhengGong-amd
added a commit
that referenced
this pull request
Aug 3, 2026
A candidate is launched with current_best's args/envs but several dispatch paths graded it against the bare baseline_tput, so anything that beat the baseline while regressing against the established recipe read as a win. The same fix had already been applied twice in isolation (integrate_patch, then kernel integrate in #1074) without the other seeding sites following. Add resolve_grading_anchor_tput() next to _first_positive_tput and route the explore/framework phase dispatch, the kernel integrate defaults, the approved proposal and delegate paths, and the kernel-stack keep decision through it. A free function rather than a SharedState method: half the callers hold a possibly-None state and the phase unit tests pass duck-typed doubles. The two resume/geak revalidation tasks keep baseline_tput on purpose - they reproduce the whole stack, so their gain is cumulative rather than a delta over current_best. Co-authored-by: Cursor <cursoragent@cursor.com>
ZhengGong-amd
added a commit
that referenced
this pull request
Aug 3, 2026
…ke a zero accuracy explain itself (#1079) * refactor(state): resolve the grading anchor in one place A candidate is launched with current_best's args/envs but several dispatch paths graded it against the bare baseline_tput, so anything that beat the baseline while regressing against the established recipe read as a win. The same fix had already been applied twice in isolation (integrate_patch, then kernel integrate in #1074) without the other seeding sites following. Add resolve_grading_anchor_tput() next to _first_positive_tput and route the explore/framework phase dispatch, the kernel integrate defaults, the approved proposal and delegate paths, and the kernel-stack keep decision through it. A free function rather than a SharedState method: half the callers hold a possibly-None state and the phase unit tests pass duck-typed doubles. The two resume/geak revalidation tasks keep baseline_tput on purpose - they reproduce the whole stack, so their gain is cumulative rather than a delta over current_best. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(writeback): never lower current_best when lifting a winner _lift_to_current_best overwrote current_best unconditionally, so a winner whose executor called it a KEEP against a stale task-level base_tput could regress the recipe. Session MiniMax-M3-MXFP8 lost 0.14% that way: a warm replay had reached 2358.8 and an explore variant measuring 2355.5 was still lifted, leaving the run reporting less gain than it had already achieved. Refuse the lift when the winner does not beat the anchor it was composed on, and return whether it landed so the callers stop stamping a promotion, advancing cumulative_gain_validated or firing a watermark roofline for work that was refused. The audit row records no_promote rather than discarded, matching _promote_baseline, so orchestration reads it as measured-but-flat instead of retrying it as a failure. No allow_regression escape: all four call sites were audited and none needs one. Revalidation is skipped earlier, geak 2b has its own no_promote branch, and the framework/integrate keep flags already come from an executor that compared against current_best. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(explore,integrate): re-resolve the grading anchor at execution time base_tput is snapshotted into task params when the task is queued, and the backstop only consulted live state when that value was missing. A task that waits behind others - PRELUDE ran six hours in the session that surfaced this - therefore grades against an anchor that current_best has since moved past, and the regression reads as a win. Take the live anchor whenever it exceeds the snapshot, warning on the drift so the next occurrence is one grep away rather than an investigation. This also feeds the stack rebench a correct stability floor, which on its own would have rejected the variant that started this. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(baseline): drain the queued backlog once the anchor is established baseline is LLM-proposable and nothing retired it after it succeeded, so a run could keep queueing reference measurements while the first was still running. Session MiniMax-M3-MXFP8 executed ten of them, eight after the anchor already existed, at roughly nineteen GPU-minutes each. Cancel the queued baselines when baseline_tput turns positive and report the cancellation as a baseline_drain observation. The enablement revalidation baseline is spared: it re-anchors a stack the specialist changed rather than re-measuring the one the session has, identified the same way _promote_baseline identifies it. cancel_family grows a reason and an exclusion set - it previously hardcoded prune_branch into every cancellation's history evidence, which would have misattributed these. The revalidation params reason becomes a shared constant instead of a literal repeated across four call sites. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(policy): retire baseline once the session has an anchor PRELUDE allows baseline so a run can reach baseline_tput > 0, and nothing retired it afterwards. _sequence_denial_for_action exempts baseline by design - it exists to hold everything else back until the anchor lands - and PolicyGate carried a singleton rule only for sweep, so a repeat baseline was denied by nothing and reviewed against an empty evidence checklist. Add baseline_phase_singleton on the same shape as sweep_phase_singleton: denied on both the propose_action and delegate channels once the anchor is positive, with params.bypass_baseline_singleton as the recorded escape for an operator who genuinely wants a fresh reference. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(prune): let orchestration drain a queue without retiring the family Orchestration could already emit prune_branch, but the handler always added the family to the persistent pruned set. That is the right move for an action that has to stop and the wrong one for a backlog that merely outlived its purpose - draining stale baselines should not cost the run the ability to re-baseline later. Add a scope field, following the convention kill_task already established. The default "family" keeps today's behaviour; "queued" cancels the queued tasks and leaves the pruned set alone, routing a baseline drain through the same helper the Coordinator uses so the enablement revalidation is spared there too. Documented for the model alongside kill_task / send_message / extend_lease, which are the moves it already reaches for when a queue needs attention. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(baseline): report a measured zero accuracy as below-floor, not unavailable The sibling-accuracy salvage filtered on `accuracy > 0.0`, so a baseline that genuinely scored gsm8k=0.0 was indistinguishable from one whose eval never ran. The cold-start guard evaluates only in warmup_round, so the deciding measure_round always salvages: a real zero was dropped, `acc` stayed None, and `classify_accuracy_failure` stamped `accuracy_unavailable` with evidence `accuracy=None ... source=None`. The enablement specialist was handed "the eval produced no number" while a full gsm8k run of degenerate output sat unreferenced, and the "salvaged but below the floor" branch was unreachable for exactly zero. Salvage now returns any finite score and the callers decide usability through the existing `accuracy_meets_floor`, which already means "finite, strictly positive and >= floor". `_request_eval_rooted_baseline_stop` still stops the run on a zero, now with the score and its source on record. `_finite_score` replaces a hand-rolled float() guard that would have admitted NaN/inf, and the write-back block duplicated across both call sites moves into `_apply_salvaged_accuracy`. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(reference-script): lift the literal default out of ${FOO:-value} exports `export FOO=${FOO:-1}` is the idiomatic overridable default in InferenceX recipes, but `_extract_envs` skipped every value containing `$`, so those settings never reached the lifted recipe even though the script applies them whenever the caller does not override. The whitelisted default is now resolved to its literal, and quote stripping runs before the check so the quoted form is covered too. Only the self-referential form counts: `export FOO=${BAR:-1}` depends on an unrelated variable, so its default is not FOO's effective value here and that line is still skipped, as is any default that itself contains a `$`. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(policy): admit tracked enablement revalidation baselines Keep the baseline singleton for normal work while allowing the Coordinator's identified accuracy revalidation to reach its executor. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(trace): preserve orchestration turn diagnostics Persist redacted Claude turn state and MCP setup so failed control-plane calls can be diagnosed after a session ends. Co-authored-by: Cursor <cursoragent@cursor.com> * test(trace): cover orchestration diagnostics Verify no-intent details, durable turn records, MCP snapshots, and the new session paths. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(orchestrator): preserve valid revalidation anchors Keep full-stack revalidation and framework patch decisions aligned with live state while preventing agent-controlled baseline bypasses and unnecessary backend diagnostics. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(eval): add generation-pathology kind and probe sidecar readers Adds EVAL_KIND_GENERATION_PATHOLOGY to the existing eval-failure taxonomy, plus read_eval_probe / eval_probe_summary for the sidecar the lm-eval probe writes when it cuts a runaway eval short. The probe knobs join the eval-contract keys so a baseline and a variant evaluated under different settings no longer fingerprint as comparable. No caller yet; the probe that produces the sidecar lands separately. * feat(breakdown): carry writeback audit extras into baseline attempt history record_action_attempt already persists an arbitrary extras dict per attempt, but collect_baseline dropped it, so anything the writeback audit recorded was invisible in session_breakdown.json. Pass it through and declare it on BaselineAttemptSummary. Additive only: schema.py states schema_version bumps on breaking changes, so SCHEMA_VERSION is unchanged. * feat(eval): cut short an accuracy eval whose model never emits EOS InferenceX runs lm-eval with max_tokens=min(16384, ctx-4096), so a model that never emits EOS burns that budget on all 1319 GSM8K docs -- ~21.6M decode tokens against ~0.26M for a healthy model. Nothing bounded it: the soft deadline is retired at eval start by design, the stall watchdog needs total log silence, and --max-hours is only checked between coordinator ticks. The 7800s baseline timeout was the only backstop, and blowing it also discards the throughput benchmark that had already finished. Inject a probe into the sitecustomize.py InferenceX already writes in _patch_lm_eval. It watches finish_reason and, once the sample is decisive, answers the remaining generate requests with an empty string so lm-eval still writes a results*.json scoring ~0 -- the same verdict a full run would reach, minutes instead of hours. Downstream is unchanged: baseline stops on the existing baseline_accuracy_failed, a variant REVERTs through accuracy_keep_block. No new stop_reason. The short-circuit hooks amodel_call rather than _create_payload because get_batched_requests builds every payload before awaiting the inner semaphore, so lowering max_tokens after the fact is a no-op. An equally sized outer gate parks the tasks instead; it is loop-keyed because asyncio.run() builds a fresh loop per batch. Tunable via HYPERLOOM_EVAL_PROBE{,_MIN_SAMPLES,_LENGTH_RATIO}. * fix(tests): stop seeding baseline_tput before the baseline proposal baseline_phase_singleton (254fd8b) denies a `baseline` propose_action once baseline_tput > 0. Two tests seeded baseline_tput up front for convenience and then proposed `baseline` as part of their normal flow, so PolicyGate silently dropped that proposal and the tests undercounted/indexed past the bus. Set baseline_tput only after the baseline proposal lands, matching what the real baseline action would do on completion. * fix(orchestrator): require salvaged sibling accuracy to meet floor before setting shared baseline Salvaged sibling accuracy is always recorded on the result as evidence, but it is now only copied to `SharedState.baseline_accuracy` when `accuracy_meets_floor` passes. This prevents a zero or negative salvaged score from becoming the "no baseline, skip the check" sentinel and silently bypassing every subsequent accuracy gate. * fix(eval): require ceiling hits before the probe cuts an eval short The trip test was a bare finish_reason=length ratio over the first 16 responses, which cannot separate a model that never terminates from one whose long answers are truncated: lm-eval sizes max_tokens per request from the remaining context, so a terminating model legitimately produces capped responses at several lengths. The ceiling was already tracked and then never consulted. The trip now counts only responses that stopped AT the largest observed cap, over a default window of 128. The knobs had no range validation either. LENGTH_RATIO=0 -- the value an operator reaches for to turn the probe off -- made the ratio test vacuously true and ended every eval at min_samples. Out-of-range values now fall back to the default rather than to the nearest legal one, since clamping RATIO=0 would do the opposite of what it asks for. Two artifact fixes alongside. With no $RESULT_DIR the sidecar landed in the cwd, i.e. InferenceX's checkout, which is the escape the _EVAL_DEST_* patch exists to prevent; stderr already carries the record, so nothing is written there now. And the probe drops a sidecar left in its $RESULT_DIR by a previous attempt, since the eval-failure retry reuses the slot, while read_eval_probe picks the newest by mtime rather than by path -- integrate_patch searches a grid slot whose sibling variants each own one. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(enablement): tell the specialist the eval was cut short, not answered wrong EVAL_KIND_GENERATION_PATHOLOGY was defined and stamped onto the probe record, but no decision ever read it: classify_accuracy_failure never returns it, and its only consumers append it to a log line or to a revert reason that had already been decided. A truncated eval was therefore indistinguishable from a model that answered and got them wrong -- same ~0 score, same accuracy_below_floor routing -- and the authoring specialist received evidence reading only "accuracy=0.0", sending it after a quality regression that never happened. The baseline executor now stamps the pathology kind and appends the probe summary to the evidence when the probe tripped. classify_accuracy_failure stays pure: it is handed a number and cannot see the probe. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(eval): trim the probe record and its comments to what is read Follow-up to 53cbc82 / 32c50f1. Three sidecar fields were written and never read: written_at (read_eval_probe orders by mtime), cap_hit_ratio and length_ratio (both derivable from counts already in the record). The remaining threshold is renamed to say which ratio it bounds, now that the trip test keys off cap hits. eval_probe_summary drops its finish_reason_length fallback and the routing drops an "unclassified" default for a kind that cannot be empty there -- neither case is reachable, since the probe writes and reads within one run and the classifier only returns None above the floor. The rest is comment length: six explanatory blocks cut to the constraint they state, and _maybe_stop_on_missing_baseline_accuracy now documents that a tripped probe re-labels the eval-failure contract, which the code already did without saying so. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(enablement): classify a cut-short eval as its own failure kind The baseline stamps eval_generation_pathology, but the specialist's kind comes from classify_failure over the evidence text, and that text also carries the "accuracy did not meet floor" phrasing -- so a truncated eval arrived labelled accuracy_below_floor and the specialist was pointed at answer quality instead of at generation that never terminates. Declaring the kind in FAILURE_KINDS is not enough on its own: classify_failure elects a primary by position in _RULES, so the kind needs a rule of its own, ahead of the below-floor rule it shadows. The ladder book now names the kind alongside the other eval triggers, since a kind the classifier can return with no methodology entry is the same gap one step further along. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(trace): keep gateway credentials out of the orchestration trace urlsplit().netloc includes URL userinfo, so a base URL configured as https://user:key@gateway/... put the key straight into an on-disk trace row. The identifier is the host, so read parts.hostname instead. gateway_endpoint was also the one free-text string in to_row() not passed through _safe_value, in a method whose whole contract is "serialize a redacted row"; every other string field there is wrapped. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
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
_fill_integrate_defaults_from_statefilled the integratebase_tputfromstate.baseline_tput, so the kernel/fusion integrate KEEP gate judged a candidate against the raw baseline instead of the current best recipe it stacks onto. A kernel/fusion that beats baseline but regresses vs the established best (e.g. a warm-replay recipe) was therefore KEEP'd instead of REVERT'd — it dragged the final recipe down while the attribution ledger correctly recorded a negative "kernel gain".The explore/framework path (
integrate_patch) already rebindsbase_tputtocurrent_best.tput("measure against the real stack top"); the kernel/fusion path missed the same rebind. Root: PR #313 added the baseline-defaulting helper, which predates a stackedcurrent_best.Observed on three sessions where a
forge_fusionentry was adopted at negative gain vs current_best while still positive vs baseline:Changes
orchestrator/kernel/request_handlers.py(_fill_integrate_defaults_from_state): prefercurrent_best.tputas thebase_tputfallback; fall back tobaseline_tputonly before anycurrent_bestexists. Explicit payloadbase_tputstill wins.test_base_tput_prefers_current_best_over_baseline(regression) andtest_base_tput_falls_back_to_baseline_without_current_best; updatetest_all_three_defaults_fired, which encoded the old baseline behaviour.Test plan
pytest test_integrate_payload_defaults.py— 13 passed (incl. the new current_best-precedence repro).test_kernel_request_handlers_units + test_kernel_integrate_and_report + test_integrate_patch_executor + test_auto_integrate_kernel_retry— 367 passed.