Close the accuracy-gate gaps that let a broken model be promoted - #1216
Merged
Conversation
When the parameter search lowers --max-model-len for throughput, the gsm8k harness can no longer be answered: it asks for a 2048-token completion on top of a roughly 1k-token five-shot prompt, so at a served context of 2048 every request returns HTTP 400 before reaching the model. No verdict is produced, accuracy_pass stays None, and because a positive baseline accuracy (measured earlier, under the larger context the run started with) is read as proof that eval works here, the missing verdict blocks the KEEP. Each such round counts as a fair attempt, so after three the patch is discarded for a reason that has nothing to do with the patch. Measured directly, same model and eval, only the served context differing: served context | lm_eval exit | HTTP 400 context errors | gsm8k 2048 | 1 | 201 | none 6144 | 0 | 0 | 0.645 +/- 0.034 A scan of every runs/**/config.yaml under the session store finds 1216 configs across 195 sessions serving a context at or below the generation budget. resolve_served_context reads the context the server actually honours: the --max-model-len CLI flag outranks the MAX_MODEL_LEN env, because the search rewrites the flag and leaves the env alone. served_context_hosts_eval reports only configurations provably unable to answer any eval request; an unknown context or an unbounded generation budget is never called infeasible, so this proves impossibility rather than guessing at it. The integrate handler now returns such a round as an integration fault instead of a gate verdict. Faults already carry their own budget and never consume one of the three attempts a patch gets to prove itself, so a configuration choice no longer discards a kernel. The gate is not degraded to throughput-only when the eval cannot run. That would raise the keep count immediately, but a patch can gain throughput while destroying accuracy, so admitting unverified patches is the wrong trade. Making the impossibility visible and attributable is the part that is safe to do here. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
CI E2E report — ✅ Succeeded
|
…ontext-feasibility
Breaking a model's numerics is itself a large throughput win — across the retained session pool the collapsed runs are a median 22% faster than the healthy ones, and up to 4.9x on GLM-5.2-MXFP4 — so an optimizer scored on throughput alone actively selects for configurations that make the model emit garbage. Two lanes that can promote a serving config had nothing to stop that. Warm replay promoted on throughput alone. Its only correctness check was an image-quality gate that is non-blocking when absent, so for an LLM workload there was none at all. 45 of 241 promoted replays (19%) were promoted while the model was broken; the worst carried a +23.95% gain on a config scoring 0.0000 on gsm8k against a 0.9014 baseline. The score had been measured and written into the same round directory — the promotion path never read it. A promoted config becomes current_best, so every later measurement in that session is taken against a model that is not producing language. The gate added here mirrors ExploreExecutor exactly: it fires only for configs touching a high-risk knob and only when a positive baseline accuracy exists to compare against, so the two lanes that can promote a serving config now agree on when accuracy is owed. A high-risk config with no verdict fails closed; that "no evidence" state is precisely what let the 45 through. The recipe KB stores best_config and best_throughput and no accuracy, so once a config is written there nothing downstream can distinguish a safe one from a ruinous one — the next session simply replays it. CLOSE is the last point where that distinction still exists, so a champion carrying a high-risk knob now has to point at a recorded passing verdict before it may overwrite best_config. Experience entries are still written either way, and a champion with no high-risk knob is unaffected. Making that check possible needed the ledger to say what a KEEP was judged on. explore_search.accepted recorded no accuracy for any variant — 188 of 188 high-risk variants in the pool — so after the fact there was no way to tell whether a kept config had ever been checked. The verdict is now carried through; None means "not gated" rather than "scored zero". No threshold changes. ACCURACY_THRESHOLD stays at a 0.05 allowed drop: healthy run-to-run spread in the pool reaches 0.037, so tightening it would flag noise, and no threshold catches the real failure mode anyway — the fault is per-server-launch and bimodal, the same config measuring 0.0076 and 0.9393 in one session. Repeated sampling of high-risk configs is the follow-up that addresses that, and is deliberately not bundled here. Co-authored-by: Cursor <cursoragent@cursor.com>
zengleixin-amd
changed the title
Self-review of the previous commit found the writeback gate had a false negative it could not recover from. It looked the champion up in explore_search.accepted, so a champion promoted by warm replay — which leaves no ledger row — was treated as unverified even after the new replay gate had checked it. That silently withholds the KB write, and nothing in CI covers KB sedimentation with a high-risk champion, so it would not have surfaced. Reconstructing the evidence per lane is the wrong shape: whichever lane is overlooked silently stops contributing to the KB. Every lane that can promote a config now stamps its verdict on the stack entry it pushes, and CLOSE reads that one place. explore carries it through from the value the gate already computed; warm replay carries the score its new gate measured. Two tests added for the cases the review exposed: a warm-replay-promoted champion is judged on the same evidence as an explore-promoted one, and an earlier passing layer does not vouch for a later unverified one. Co-authored-by: Cursor <cursoragent@cursor.com>
6 tasks
The gate added for warm replay looked for its score in the wrong directory, so it almost never found one. The cold-start guard splits a replay into a warmup round that evaluates and a measure round that times hot throughput, then hands back the measure round's result -- whose accuracy is None by construction and whose workspace holds no eval output. Across 852 recorded replays the score sat in warmup_round 320 times and in measure_round zero times. Failing closed on a missing verdict therefore did not gate the risky replays; it rejected every double-run replay, sound ones included. parse_eval_results already prefers a measured round and falls back to the warmup's file, so nothing was wrong with the parser. It was handed a path two levels too deep. It is now given the task directory that holds both rounds, which is what the baseline's own sibling-salvage does with the same layout. A replay whose contract has RUN_EVAL off still produced no score at all, 115 of the 852. The warmup round is the only round that evaluates, so it is forced on for replay tasks. The salvage retry taken when the eval is itself what aborted the run keeps RUN_EVAL off: forcing it back on there reproduces the failure and loses the throughput baseline too. Every replay is now judged, not only those touching a knob on the high-risk list. A recipe is evidence from another session on another machine, so reproducing its throughput here says nothing about whether it still computes correctly here; the high-risk trigger the other lanes use assumes a candidate built from a known-good local baseline, which a replay is not. The score is recorded whether it passes or fails. A promotion that was checked is not the same record as one that never was, and the KB identity needed to trace a bad config back to its recipe row already sits in the same block -- config_source carries a full canonical_id in 448 of 469 recorded replays. eval_ran separates the two ways the score can be absent: a model that answered nothing scores 0.0, while an eval that never ran scores nothing at all, and collapsing those is what produced the false rejections. The CLOSE-time KB writeback gate is removed. It only covered one of the two paths that write best_config -- the per-KEEP runtime amend goes through _kb_best_config_overrides_for_keep on throughput alone -- and it read a stack field that only explore and warm replay populate, so a framework- or kernel-promoted champion was withheld for lack of a verdict no lane had recorded. Gating half the writes on evidence two thirds of the lanes never produce is worse than not gating them. Co-authored-by: Cursor <cursoragent@cursor.com>
A failed measurement already admitted the replay rather than stopping the run, but the record said only that no score existed. That leaves an operator unable to tell a contract with RUN_EVAL off from an eval that crashed from a results file carrying a metric this parser does not know, and those want different fixes. parse_eval_results already returns the reason it could not produce a score, so it is carried through to eval_error instead of being discarded. eval_ran still separates "an eval produced nothing" from "no eval ran", and the two fields together say which of the three cases happened. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…nce. Sessions started with --no-eval carry no baseline accuracy, so the relative 0.05 gate never fired and collapsed replays could still promote. When no baseline exists but a replay score was measured, judge it against the enablement absolute floor instead of admitting unconditionally. Add tests for that path and for replay double-run forcing RUN_EVAL on the warmup round only. Co-authored-by: Cursor <cursoragent@cursor.com>
The GitHub Docs workflow (sphinx html) passed on e47a064; the external Read the Docs check failed without any docs changes on this commit. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
/retest |
Contributor
Author
CI status update (automated triage)Code-related checks on
Two failures — not caused by this PR's code changes 1. E2E (
|
zengleixin-amd
had a problem deploying
to
github-pages
August 20, 2026 02:13 — with 
GitHub Actions
Failure
GitHub Actions
Failure
…ontext-feasibility
…ontext-feasibility
eval_ran answered "not no results", so every reason other than a missing results file counted as an eval that produced output -- including the parser itself raising, where nothing was read at all. That reads downstream as a model that answered nothing, which is the one state it has to be distinguishable from: a score of 0.0 is a broken config, an absent score is broken infrastructure, and they want different responses. Only the two reasons that prove a file was reached now count as having run: one the parser could not decode, and one carrying no metric it recognises. The sibling paths were already correct and stay that way -- a result naming no round directory, and a directory holding no results file, both already reported eval_ran False. Co-authored-by: Cursor <cursoragent@cursor.com>
The forced eval excluded the staged-accuracy lane and the salvage retry, but not --no-eval, so a session that switched eval off still paid for one on every warm replay. The flag is an operator instruction, and the baseline path on this same executor already honours it -- twelve lines above, defer_accuracy is computed as "not eval_disabled and ..." for exactly this reason. A replay that did not was the odd one out, and it overrode the instruction silently. Measured on the three arms that share the code: a replay under --no-eval asked for RUN_EVAL true then false, identical to a replay with eval on, while a baseline under the same flag asked for false twice. Such a session now promotes replays unjudged. That is the trade --no-eval already makes everywhere else, and the outcome records it: eval_ran false with the reason, rather than a score that was never taken. Also registers the four warm-replay accuracy fields, and the stack entry's accuracy, in the session-breakdown reference -- they are a downstream contract and were only described in the schema docstring. 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
2 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.
Why
Breaking a model's numerics is itself a large throughput win. Across the retained session pool the accuracy-collapsed runs are a median 22% faster than the healthy ones in the same session (77 of 101 comparable sessions; up to +392% on GLM-5.2-MXFP4). An optimizer scored on throughput therefore selects for configurations that make the model emit garbage.
Warm replay was the worst gap: 45 of 241 promoted replays (19%) were promoted while the model was broken ? the worst carrying a
+23.95%gain on a config scoring 0.0000 on gsm8k against a 0.9014 baseline. The score had been measured; the promotion path never read it from the right place.What this changes
Warm-replay accuracy gate (R1 + R2)
baseline.py). A recipe from another session is not a locally verified candidate, so the high-risk-only trigger used by explore does not apply here.--no-evalstill means no eval. The forced warmup eval is skipped when the operator disabled eval for the session, matching what the baseline path on the same executor already did. Such a session promotes replays unjudged, which is the trade that flag already makes everywhere else.warmup_round320 times, inmeasure_roundzero times.accuracy_passed(threshold=0.05)) when a positive baseline accuracy exists. Degradation above 0.05 rejects promotion; within tolerance passes.baseline_accuracy <= 0, e.g. the baseline eval failed), a measured replay score is judged against the enablement absolute floor (DEFAULT_ENABLEMENT_ACCURACY_FLOOR = 0.5) so a collapsed replay cannot promote on throughput alone.eval_errorand promotion continues.warm_replay_outcomeand flow into breakdown:eval_ran,replay_accuracy,baseline_accuracy,eval_error.eval_ranis what separates "the model answered nothing" (score 0.0) from "nothing checked it" (no score) ? collapsing those is what produced the original false rejections. Documented indocs/reference/session-breakdown.md.optimization_stackentries from warm-replay promotion carryaccuracy(the replay score).Eval-context feasibility
When
--max-model-lenis lowered below what gsm8k needs, the accuracy gate cannot run ? that is an integration fault, not a kernel verdict. Prevents charging an unrunnable eval to the patch under test.R3 ? baseline/eval failure recording
Already satisfied by the existing
enablementframework (baseline_eval_kind,observed_accuracy, evidence paths). No new code needed; confirmed across 1500 sessions.R4 ? the recipe KB is left alone
No change to the KB schema or its write paths, by design.
For transparency about the branch history: an earlier commit here (
1ae743126) added a CLOSE-time_champion_accuracy_okgate on the KB write, and a later one (e14f5fb26) removed it again after self-review found it covered only one of the two paths that writebest_configand read a stack field two thirds of the promoting lanes never populate. Both commits are internal to this branch ? the gate never existed onmain, and the net diff againstmaincontains no KB change at all.No threshold changes, deliberately
ACCURACY_THRESHOLDstays at a 0.05 allowed drop for baseline-relative comparison: healthy run-to-run spread in the pool reaches 0.037, so tightening it would flag noise. The no-baseline path reuses the existing enablement floor rather than introducing a new number.No threshold addresses the real failure mode anyway ? the fault is per-server-launch and bimodal, the same config measuring 0.0076 and 0.9393 within one session. Repeated sampling of high-risk configs is the follow-up for that and is deliberately not bundled here.
Test plan
test_warm_replay_accuracy_gate.py? 19 tests: degradation rejection, within-tolerance pass, warmup-round score location, no-baseline absolute floor pass/reject, and each way a score can be absenttest_baseline_warmup_double_run.py? replay double-run forcesRUN_EVALon the warmup round only, and honours--no-evaltest_eval_context_feasibility.py? 13 tests still passparse_eval_resultspath that yields no score, assertingeval_ranand the promote/reject decision: 11/11main(real-ROCm / real-backend cases), so zero net regressionruff checkclean on changed filesTests were run on Linux; the local Windows checkout cannot import
recipe_kb(fcntl).