Repository navigation
refactor(evaluation): one plugin class per candidate-evaluation policy, composed by mixins - #380
Merged
Merged
Conversation
|
Merge Oncall: @BobbyZhouZijian The merge oncall will route reviews and keep this pull request moving toward a clear decision. |
BobbyZhouZijian
force-pushed
the
feat/regression-gate
branch
from
September 11, 2026 10:04
612c47d to
f620109
Compare
…tion
Reef ships one built-in selection policy, AlwaysSelect, which publishes every
evaluated candidate. RegressionGateMixin is its counterpart as a mixin for a
CandidateEvaluationPlugin: it supplies decide(), selecting a candidate only
while one scalar metric has not regressed more than `margin` below the best
score selected so far — best-checkpoint selection made online, so an objective
that has passed its peak cannot compound regressing steps into serving. The bar
is seeded by the first candidate, so the initial climb is always admitted.
A recipe mixes it into a cohesive plugin class that supplies evaluate():
class MyPlugin(RegressionGateMixin, CandidateEvaluationPlugin):
def __init__(self, ...):
super().__init__(metric="clean_rate", margin=0.17)
def evaluate(self, candidate): ...
Generic and dependency-free (higher_is_better=False gates a loss or error
rate), with no probe, model, or runtime coupling. The CandidateSelector
abstraction and AlwaysSelect are untouched, so recipes that compose a selector
(cordis, gepa, meta_harness) are unaffected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SuEa3FyrkW1xc3sjmAJSEB
BobbyZhouZijian
force-pushed
the
feat/regression-gate
branch
from
September 11, 2026 10:57
f620109 to
a239271
Compare
…posed by mixins
Candidate evaluation was two objects glued together: a `CandidateEvaluator`
that measured and a `CandidateSelector` that decided, wrapped at runtime by
`DefaultCandidateEvaluationPlugin`. A method that wanted both halves had to
write two classes and a recipe had to know how to pair them, so "the policy"
was never a thing you could point at.
Now a plugin is one class that mixes in the methods it needs:
- `AlwaysSelectMixin` / `RegressionGateMixin` / `ScoreComparisonMixin` /
`GEPASelectorMixin` / `MetaHarnessSelectorMixin` supply `decide()`.
- `BackendEvaluateMixin` supplies an `evaluate()` that delegates to the
training backend, for methods whose measurement is the backend's own.
- A shipped plugin is the pairing: `BackendAlwaysSelectPlugin`,
`ScoreComparisonPlugin`, `GEPAPlugin`, `MetaHarnessPlugin`.
A method that measures for itself mixes a decide mixin into its own plugin and
writes `evaluate()` directly, which is what the OpenClaw-RL regression gate
needs and what the old split made awkward.
Recipes now choose a plugin class rather than a selector: cordis's
`evolution.selection` resolves to a `CandidatePluginFactory` (a named Protocol
over `backend -> CandidateEvaluationPlugin`), and a dotted path in YAML must
name a plugin class. `CordisRecipe.candidate_selector` becomes
`candidate_plugin`; gepa and meta-harness bind theirs in `build`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SuEa3FyrkW1xc3sjmAJSEB
BobbyZhouZijian
requested review from
Benjamin-eecs and
simonucl
as code owners
September 11, 2026 11:51
A mixin that can be instantiated on its own is a mixin in name only: nothing
stopped `AlwaysSelectMixin()` from being built and used as if it were a plugin,
even though it has no `evaluate` and would have failed at the first call.
`CandidateSelector.decide` and `CandidateEvaluator.evaluate` are now
`@abstractmethod`, and every mixin inherits `CandidateEvaluationPlugin` and
implements one half of it. The half it does not implement stays abstract, so
each mixin is abstract on exactly the method it is missing and only the pairing
is concrete:
AlwaysSelectMixin abstract={evaluate}
RegressionGateMixin abstract={evaluate}
ScoreComparisonMixin abstract={evaluate}
BackendEvaluateMixin abstract={decide}
BackendAlwaysSelectPlugin / ScoreComparisonPlugin / GEPAPlugin /
MetaHarnessPlugin abstract={}
Declaring the abstract members on the protocols rather than on each mixin is
what makes this compose: they sit last in every plugin's MRO, so a mixin's
concrete half always wins over the inherited abstract one. Plugins therefore no
longer name `CandidateEvaluationPlugin` explicitly - they inherit it twice over.
The contracts stay `runtime_checkable` Protocols, so structural typing and the
`isinstance`/`issubclass` checks are unchanged; only explicit subclasses gain
the enforcement. Verified identical on Python 3.10, 3.11, 3.12 and 3.13.
Tests that built a bare mixin now build the real plugin with a backend stub
that refuses `evaluate`, which is honest about their exercising `decide` only,
and the contract test asserts the abstractness directly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SuEa3FyrkW1xc3sjmAJSEB
…ambda The min_win_margin branch assigned a lambda to a name and carried a `# noqa: E731` to get past the linter. `functools.partial` says the same thing without the suppression. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SuEa3FyrkW1xc3sjmAJSEB
BobbyZhouZijian
added a commit
that referenced
this pull request
Sep 11, 2026
#380 landed the candidate-evaluation refactor on main, which conflicts with the copy of the pre-refactor mixin this branch was carrying. Both conflicted files (`reef/train/evaluation/__init__.py`, `reef/train/evaluation/evaluators.py`) resolve to main's version, and OpenClaw-RL's plugin adapts to it: - `RegressionGateMixin` now inherits `CandidateEvaluationPlugin` and leaves `evaluate` abstract, so `OpenClawRLCandidateEvaluationPlugin` no longer names the contract a second time. It supplies `evaluate`, so it resolves concrete. - The module docstring names `BackendAlwaysSelectPlugin`, the default that replaced `AlwaysSelect`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SuEa3FyrkW1xc3sjmAJSEB
13 tasks
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.
Why
Candidate evaluation was two objects glued together: a
CandidateEvaluatorthat measured a candidate and aCandidateSelectorthat decided whether to publish it, wrapped at runtime byDefaultCandidateEvaluationPlugin. A method that wanted to own both halves had to write two classes, and a recipe had to know how to pair them — so "the selection policy" was never a thing you could point at, and a method that measures for itself (rather than through the training backend) had nowhere natural to put that code.That came up while building a regression gate for OpenClaw-RL, which needs exactly that shape: its own measurement plus a best-checkpoint decision.
What
A plugin is one class that mixes in the methods it needs.
AlwaysSelectMixindecide()— publish every evaluated candidateRegressionGateMixindecide()— best-checkpoint selection over one scalar metric (new)ScoreComparisonMixin(cordis)decide()— more task wins than lossesGEPASelectorMixin,MetaHarnessSelectorMixindecide()— archive / population policiesBackendEvaluateMixinevaluate()— delegate to the training backendThe mixins are abstract.
CandidateSelector.decideandCandidateEvaluator.evaluateare@abstractmethod, and each mixin inheritsCandidateEvaluationPluginand implements one half of it — so the half it does not implement stays abstract and the mixin cannot be instantiated on its own:Declaring the abstract members on the protocols rather than on each mixin is what makes this compose: they sit last in every plugin's MRO, so a mixin's concrete half always wins over the inherited abstract one. The contracts stay
runtime_checkableProtocols, so structural typing and theisinstance/issubclasschecks are unchanged — only explicit subclasses gain the enforcement. Verified identical on Python 3.10, 3.11, 3.12 and 3.13.A shipped plugin is the pairing:
BackendAlwaysSelectPlugin,ScoreComparisonPlugin,GEPAPlugin,MetaHarnessPlugin. A method that measures for itself mixes a decide mixin into its own plugin and writesevaluate()directly.RegressionGateMixinreads one scalar metric from the evaluation and selects only while the score stays withinmarginof the best selected so far; otherwise it rejects and serving holds the last selected weights. It's best-checkpoint selection made online: an objective past its peak cannot compound regressing steps into serving. The first candidate seeds the bar, so the initial climb is always admitted and the gate only bites once there is a peak to regress from.higher_is_better=Falsegates a metric that improves as it falls.Contract change
Recipes now choose a plugin class, not a selector.
CordisRecipe.candidate_selector→candidate_plugin, typed by a namedCandidatePluginFactoryProtocol (backend -> CandidateEvaluationPlugin).evolution.selectionmust now name a plugin class, not a selector.ScoreComparisonSelector→ScoreComparisonMixin+ScoreComparisonPlugin;GEPASelector→GEPASelectorMixin+GEPAPlugin;MetaHarnessSelector→MetaHarnessSelectorMixin+MetaHarnessPlugin.AlwaysSelectandDefaultCandidateEvaluationPluginare gone, replaced byAlwaysSelectMixinandBackendAlwaysSelectPlugin.CandidateEvaluator/CandidateSelectorremain as the published protocols; only the built-ins changed shape.Evidence
CI green on all 15 checks (lint, docs-gate, test and package on 3.10/3.11/3.12, the five real-harness jobs). Local: 2590 passed, 52 skipped (slime-dependency files excluded — no
slimeon this machine). mypy, ruff 0.14.7, isort 5.13.2, black 24.3.0, the Python design/statement policies, the README i18n check and the docs contract gate all pass.The gate's effect on a real run is in the OpenClaw-RL MLX experiment on #325: ungated, the policy collapsed into an unbroken no-reply wall from session 51 to 71; gated, it kept answering through 17 of the 18 sessions in the same stretch.
🤖 Generated with Claude Code
https://claude.ai/code/session_01SuEa3FyrkW1xc3sjmAJSEB