Repository navigation
optimizer: fix structurally-uneconomical screening, promote control-relative as primary gate signal - #689
Merged
Conversation
skillberry-bot
added
bug
Contributor
|
🏷️ Automatic Labeling I've analyzed this pull request and added the following labels:
These labels were selected based on the PR title, description, and changed files. If you believe any labels are incorrect or missing, feel free to adjust them manually. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| import json | ||
| import shutil | ||
| import sys |
| import json | ||
| import shutil | ||
| import sys | ||
| from pathlib import Path |
| import sys | ||
| from pathlib import Path | ||
|
|
||
| from test_round_requires_screen_ladder import CORE, SCRIPTS, _project, _run |
…elative as primary gate signal Issue #684 items 5-6, from forensic diagnosis of two real runs. Item 5 (screen.py / subsample.py): tier-1 screen subsets (5-11 tasks) measured SEs of 0.11-0.17 while the round deltas that mattered were 0.02-0.09, so the old significance-based kill rule (mean(delta) + k_se*SE < 0) could never clear its own confidence bar. Result: 0 screen-kills across 7 candidates in that run (0/8 in an earlier one), with screening costing more rollouts than it saved. Per GEPA's own validated design (a cheap minibatch gate with no significance test; the full/expensive eval is the real decision point), screen_decision now kills on a GROSS negative signal (mean(delta) <= GROSS_KILL_DELTA, default -0.15) regardless of SE, and otherwise promotes. -0.15 sits above the ~0.10 drift measured between byte-identical controls and below the ~0.2 of the one confirmed gross failure (cand_7), so it catches genuinely broken candidates without tripping on noise or a genuine close call. Documented prominently in subsample.py's module docstring, screen.py's own docstring, and references/algorithm.md and references/measured-lessons.md so this is not a silent behavior change. Added a screening_economics audit (test_subsample.py) that reproduces cand_7's real screen delta as a fixture, confirms the new rule would have killed it while the old significance rule would have missed it, and confirms genuinely close calls (0.02-0.09) still promote. Item 6 (gate.py / round.py): measurement drift between rounds (~0.10) is comparable in magnitude to the real effects being chased (~0.02-0.09). round.py used to gate each candidate's top-level verdict against the parent's STORED reward (which can carry that drift) as primary, with the delta against a freshly-measured byte-identical control kept as a secondary cross-check. Inverted: round.py now gates primarily against the pooled null-control replicates whenever one was measured this round (the standard case), with the raw comparison against the stored parent kept as a secondary diagnostic field (raw_vs_parent) for detecting drift itself. The pre-existing control_relative field keeps its old shape for backward compatibility with commit.py's --reject-basis drift_control and the dashboard's control-relative panels. Falls back to the unchanged parent-primary behavior when no control exists this round (--no-control). Added test_round_control_relative_primary.py, which reproduces the real run's exact ambiguous scenario end to end through round.py (reward tie against the stored parent, a real win against fresh control replicates) and asserts the primary verdict now resolves ACCEPT directly, with no grow.py escalation needed. core/tests -q passes except the 2 known pre-existing test_tau2_airline_eval_cost.py failures (unrelated to this change). Signed-off-by: Osher Elhadad <Osher.Elhadad@ibm.com>
OsherElhadad
force-pushed
the
worktree-agent-abac93057aaa4d9f3
branch
from
October 8, 2026 11:36
335d211 to
0e3fc4b
Compare
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
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
Fixes issue #684 items 5 and 6 only (screening redesign, control-relative-as-primary
gating). Does not touch the Pareto/archive gate (items 1-2),
merge_search.py(item 4),plan_round.py's branch-count/ownership logic (item 3), the dashboard, hill-climb, orcost-reduction guidance — those are owned by other in-flight PRs against #684.
Item 5 — screening redesign (
screen.py/subsample.py)Confirmed bug: tier-1 screen subsets (5-11 tasks) measured SEs of 0.11-0.17 while the
round deltas that actually mattered were 0.02-0.09, so the old significance-based kill
rule (
mean(Δ) + k_se·SE < 0) could never clear its own confidence bar. Result: 0screen-kills across 7 candidates in that run (0/8 in an earlier one), with screening
costing more rollouts than it saved.
Per GEPA's own validated design (a cheap minibatch gate that just checks "did it beat
the parent, yes/no" — no significance test, because the full/expensive eval is the real
decision point),
screen_decisionnow kills on a gross negative signal —mean(Δ) <= GROSS_KILL_DELTA(default -0.15), regardless of SE — and otherwisepromotes.
Threshold reasoning: -0.15 sits strictly between the ~0.10 drift measured between
byte-identical control replicates (so ordinary re-measurement noise never trips it) and
the ~0.2 of the one confirmed gross failure in a real run (
cand_7, an infra-brokenedit), so it still catches genuinely broken candidates without being trigger-happy on
real noise or a genuine close call. Configurable via
--gross-kill-threshold.This is a deliberate, documented behavior change, not a silent one — called out
prominently in
subsample.py's module docstring,screen.py's own docstring, andreferences/algorithm.md/references/measured-lessons.md.Added a
screening_economicsaudit (test_subsample.py): a fixture reproducingcand_7's real screen delta confirms the new rule kills it (and that the oldsignificance rule would have missed it, at the SEs that run actually measured), while
genuinely close-call effect sizes (0.02-0.09) still promote.
Item 6 — control-relative as primary (
gate.py/round.py)Confirmed bug: measurement drift between rounds (~0.10) is comparable in magnitude to
the real effects being chased (~0.02-0.09).
round.pytreated the delta against theparent's STORED reward (which can carry that drift) as primary, with the delta against
a freshly-measured byte-identical control kept as a secondary cross-check — backwards,
and it required a manual
grow.pyescalation to resolve real disagreements twice inone run.
Inverted:
round.pynow gates PRIMARILY against the pooled null-control replicateswhenever one was measured this round (
--control-replicates 2is the default — thestandard case), with the raw comparison against the stored parent kept as a secondary
diagnostic field (
raw_vs_parent) for detecting drift itself. The pre-existingcontrol_relativefield keeps its old shape for backward compatibility withcommit.py's--reject-basis drift_controland the dashboard's control-relativepanels — it is simply redundant with the primary verdict under the new default, since
there's no longer a disagreement for it to resolve. Falls back to the unchanged
parent-primary behavior whenever no control exists this round (
--no-control).Added
test_round_control_relative_primary.py: reproduces the real run's exactambiguous scenario end to end through a real
round.pyinvocation (reward tie againstthe stored parent, a real significant win against fresh control replicates) and asserts
the primary verdict now resolves ACCEPT directly, with no
grow.pygrowth round needed.Test plan
python -m pytest core/tests -q— 1917 passed, 1 skipped, 2 failed (the 2 knownpre-existing
test_tau2_airline_eval_cost.pyfailures, unrelated to this change)issue agent-optimize v3: native Pareto gating, archive-based search, real merging, cost-reduction levers #684
round.py's Pareto/archive logic,merge_search.py,plan_round.py's branch-count/ownership logic, or any dashboard fileCloses part of #684 (items 5, 6).