From 8a6f2978a2a062e433fc6430b75047232c86bc1e Mon Sep 17 00:00:00 2001 From: michael <78113554+chowhsu@users.noreply.github.com> Date: Sat, 10 Oct 2026 09:13:28 +0000 Subject: [PATCH 1/2] critic family: refuse positive_lm_source=success when the reward function is not declared to write metadata['success'] S19 #6 VAPO G3 s19-vapo-g3-20261010a failed at the first critic step on both islands: the out-of-tree gsm8k_reward:score writes no success flag. S13 G1 passed with the in-package yeto.rl.gsm8k_reward:score, which does. - yeto.rl.math_reward.writes_success declares a reward function; the in-package gsm8k and math rewards carry it. - launch check positive_lm_success (launcher dry-run + island config) rejects an undeclared or unimportable reward function. Co-Authored-By: Claude Opus 5.5 --- tests/test_rl_positive_lm_success_check.py | 77 ++++++++++++++++++++++ yeto/launcher.py | 1 + yeto/rl/adapters/miles/config.py | 1 + yeto/rl/algos/critic.py | 45 +++++++++++++ yeto/rl/gsm8k_reward.py | 3 +- yeto/rl/math_reward.py | 17 +++++ 6 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 tests/test_rl_positive_lm_success_check.py diff --git a/tests/test_rl_positive_lm_success_check.py b/tests/test_rl_positive_lm_success_check.py new file mode 100644 index 000000000..56cdb43ad --- /dev/null +++ b/tests/test_rl_positive_lm_success_check.py @@ -0,0 +1,77 @@ +"""positive_lm_success launch check + gsm8k success flag (S19 #6 s19-vapo-g3-20261010a). + +The G3 run loaded the out-of-tree ``gsm8k_reward:score`` (no success flag) with a +VAPO spec using ``positive_lm_source='success'``; the Miles fork raised on both +islands at the first critic step. S13 G1 passed with ``yeto.rl.gsm8k_reward:score``. +""" + +from __future__ import annotations + +import asyncio +import sys +import types + +from yeto.rl.algos.critic import positive_lm_success_problems +from yeto.rl.algos.vapo import vapo_spec +from yeto.rl.engine.algorithm import launch_problems +from yeto.rl.math_reward import SUCCESS_DECLARATION_ATTR + +_VALS = {"rollout_batch_size": 4, "rollout_max_response_len": 384, + "context_parallel_size": 1, "multi_lora": False} + + +def _spec(source="success"): + if source == "success": + return vapo_spec() + return vapo_spec(loss={"positive_lm_source": source, "positive_lm_reward_threshold": 0.0}) + + +def _install_undeclared(monkeypatch): + mod = types.ModuleType("_oot_gsm8k_reward") + + async def score(args, sample, **kw): # out-of-tree copy: reward only + return 1.0 + + mod.score = score + monkeypatch.setitem(sys.modules, "_oot_gsm8k_reward", mod) + + +def test_vapo_spec_uses_success_source(): + assert _spec().loss.positive_lm_source == "success" + + +def test_undeclared_reward_is_refused(monkeypatch): + _install_undeclared(monkeypatch) + vals = dict(_VALS, reward_function="_oot_gsm8k_reward:score") + probs = positive_lm_success_problems(_spec(), vals) + assert probs and "writes_success" in probs[0] + assert any(p.startswith("[positive_lm_success]") for p in launch_problems(_spec(), vals)) + + +def test_unimportable_reward_is_refused(): + probs = positive_lm_success_problems(_spec(), dict(_VALS, reward_function="no_such_mod_xyz:score")) + assert probs and "cannot be imported" in probs[0] + + +def test_declared_in_package_rewards_pass(): + for rf in ("yeto.rl.gsm8k_reward:score", "yeto.rl.math_reward:reward_func"): + assert positive_lm_success_problems(_spec(), dict(_VALS, reward_function=rf)) == [] + + +def test_skips_without_reward_or_other_source(monkeypatch): + _install_undeclared(monkeypatch) + assert positive_lm_success_problems(_spec(), _VALS) == [] + vals = dict(_VALS, reward_function="_oot_gsm8k_reward:score") + assert positive_lm_success_problems(_spec("reward"), vals) == [] + from yeto.rl.engine.algorithm import AlgorithmSpec + assert positive_lm_success_problems(AlgorithmSpec(), vals) == [] + + +def test_gsm8k_score_writes_success_matching_reward(): + from yeto.rl.gsm8k_reward import score + assert getattr(score, SUCCESS_DECLARATION_ATTR) is True + for response, want in (("so \\boxed{72}", 1.0), ("so \\boxed{71}", 0.0), ("", 0.0)): + sample = types.SimpleNamespace(response=response, label="... #### 72", metadata={}) + value = asyncio.run(score(None, sample)) + assert value == want + assert sample.metadata["success"] is (want == 1.0) diff --git a/yeto/launcher.py b/yeto/launcher.py index 8e83f4bf9..00ca43691 100644 --- a/yeto/launcher.py +++ b/yeto/launcher.py @@ -1648,6 +1648,7 @@ def _prepare_ports_algorithm(args, rl_engine: str) -> None: # rl-algo-critic-family 2.3 (critic run-level rejections) "sync_preset": getattr(args, "rl_sync_preset", "strict-avg"), "elastic": bool(getattr(args, "rl_elastic", False)), + "reward_function": getattr(args, "reward_function", None), }) if problems: raise AlgorithmSpecError("algorithm spec rejected: " + "; ".join(problems)) diff --git a/yeto/rl/adapters/miles/config.py b/yeto/rl/adapters/miles/config.py index 1c7d6148a..89ab3f8d2 100644 --- a/yeto/rl/adapters/miles/config.py +++ b/yeto/rl/adapters/miles/config.py @@ -674,6 +674,7 @@ def translate_run_config( "multi_lora": any(t.split("=", 1)[0] == "--multi-lora" for t in extra_argv), # rl-algo-critic-family 2.3: critic GPU counts / deploy-component / indep-dp "extra_argv": tuple(extra_argv), + "reward_function": config.algorithm.reward_function, "actor_num_nodes": config.parallel.actor_num_nodes, "actor_num_gpus_per_node": config.parallel.actor_num_gpus_per_node, "elastic": bool(config.use_miles_router), # set iff --rl-elastic (run_config) diff --git a/yeto/rl/algos/critic.py b/yeto/rl/algos/critic.py index 8ed9cb263..53bb6c1cf 100644 --- a/yeto/rl/algos/critic.py +++ b/yeto/rl/algos/critic.py @@ -99,6 +99,51 @@ def _reject_positive_lm(spec: AlgorithmSpec) -> str | None: register_rejection("positive_lm_threshold", _reject_positive_lm) + +def _reward_declares_success(reward_function: str) -> bool | None: + """True/False: the ``module:function`` carries the ``writes_success`` + declaration; None: it cannot be imported here.""" + + import importlib + + from yeto.rl.math_reward import SUCCESS_DECLARATION_ATTR + + module, _, name = str(reward_function).partition(":") + try: + fn = getattr(importlib.import_module(module), name) + except Exception: # noqa: BLE001 - any import failure means "not confirmed" + return None + return bool(getattr(fn, SUCCESS_DECLARATION_ATTR, False)) + + +def positive_lm_success_problems(spec: AlgorithmSpec, values: Mapping[str, Any]) -> list[str]: + """``loss.positive_lm_source='success'`` needs a reward function declared + (``yeto.rl.math_reward.writes_success``) to write ``sample.metadata['success']``. + + Without it the Miles fork raises at the first critic step on the island + (S19 #6 s19-vapo-g3-20261010a: out-of-tree ``gsm8k_reward:score`` wrote no + flag; S13 G1 passed with ``yeto.rl.gsm8k_reward:score``). Skipped when the + caller passes no ``reward_function``. + """ + + if spec.loss.positive_lm_coef is None or spec.loss.positive_lm_source != "success": + return [] + reward_function = values.get("reward_function") + if not reward_function: + return [] + declared = _reward_declares_success(reward_function) + if declared: + return [] + why = ("cannot be imported to confirm it" if declared is None + else "is not declared with yeto.rl.math_reward.writes_success") + return [f"loss.positive_lm_source='success' needs a reward function that writes " + f"sample.metadata['success']; {reward_function} {why}. Use e.g. " + "yeto.rl.gsm8k_reward:score, or positive_lm_source='reward' only when a " + "positive reward always means complete success"] + + +register_launch_check("positive_lm_success", positive_lm_success_problems) + # -------------------------------------------------------------------------- # Stage-W value-quality gate (user decision B, design D5/D7). Absent (None) = # record only, never block; absent from the canonical JSON while None, so no diff --git a/yeto/rl/gsm8k_reward.py b/yeto/rl/gsm8k_reward.py index 4ed2d1f4a..073831e7a 100644 --- a/yeto/rl/gsm8k_reward.py +++ b/yeto/rl/gsm8k_reward.py @@ -15,13 +15,14 @@ # Neutral form: yeto.rl.rewards.builtin.gsm8k_reward (decoupling 3.2); this # module keeps the Miles entry point and its signature. -from yeto.rl.math_reward import set_success +from yeto.rl.math_reward import set_success, writes_success from yeto.rl.rewards.builtin import _num, gsm8k_grade as grade, gsm8k_reward from yeto.rl.rewards.types import Trajectory __all__ = ["_num", "grade", "score"] +@writes_success async def score(args, sample, **kwargs): result = gsm8k_reward(Trajectory(response=sample.response or "", label=sample.label)) set_success(sample, result.metadata["success"]) diff --git a/yeto/rl/math_reward.py b/yeto/rl/math_reward.py index fc97d0d60..621231d3b 100644 --- a/yeto/rl/math_reward.py +++ b/yeto/rl/math_reward.py @@ -39,6 +39,22 @@ def score(response: str, label) -> float: return 1.0 if grade_answer_mathd(answer, truth) or grade_answer_sympy(answer, truth) else 0.0 +SUCCESS_DECLARATION_ATTR = "yeto_writes_success" + + +def writes_success(fn): + """Declare that reward function ``fn`` writes ``sample.metadata['success']``. + + The launch check ``positive_lm_success`` (rl-algo-critic-family) rejects + ``loss.positive_lm_source='success'`` when the run's reward function lacks + this declaration: the Miles fork otherwise fails on the island at the first + critic step (S19 #6 VAPO G3 s19-vapo-g3-20261010a). + """ + + setattr(fn, SUCCESS_DECLARATION_ATTR, True) + return fn + + def set_success(sample, success: bool) -> None: """Write the boolean full-success flag into ``sample.metadata`` (in place).""" metadata = getattr(sample, "metadata", None) @@ -48,6 +64,7 @@ def set_success(sample, success: bool) -> None: metadata["success"] = bool(success) +@writes_success async def reward_func(args, sample, **kwargs) -> float: value = score(sample.response or "", sample.label) set_success(sample, value == 1.0) From 4df669fae73daafdb7325e60bf1f9e409af52d43 Mon Sep 17 00:00:00 2001 From: michael <78113554+chowhsu@users.noreply.github.com> Date: Sat, 10 Oct 2026 09:23:48 +0000 Subject: [PATCH 2/2] positive_lm_success: keep reward modules untouched (golden source hashes); declare via yeto.rl.reward_declarations; unimportable reward gets no verdict Co-Authored-By: Claude Opus 5.5 --- tests/test_rl_positive_lm_success_check.py | 21 ++++++++--- yeto/rl/algos/critic.py | 32 +++++------------ yeto/rl/gsm8k_reward.py | 3 +- yeto/rl/math_reward.py | 17 --------- yeto/rl/reward_declarations.py | 42 ++++++++++++++++++++++ 5 files changed, 68 insertions(+), 47 deletions(-) create mode 100644 yeto/rl/reward_declarations.py diff --git a/tests/test_rl_positive_lm_success_check.py b/tests/test_rl_positive_lm_success_check.py index 56cdb43ad..ab1fafd08 100644 --- a/tests/test_rl_positive_lm_success_check.py +++ b/tests/test_rl_positive_lm_success_check.py @@ -14,7 +14,7 @@ from yeto.rl.algos.critic import positive_lm_success_problems from yeto.rl.algos.vapo import vapo_spec from yeto.rl.engine.algorithm import launch_problems -from yeto.rl.math_reward import SUCCESS_DECLARATION_ATTR +from yeto.rl.reward_declarations import writes_success _VALS = {"rollout_batch_size": 4, "rollout_max_response_len": 384, "context_parallel_size": 1, "multi_lora": False} @@ -48,9 +48,21 @@ def test_undeclared_reward_is_refused(monkeypatch): assert any(p.startswith("[positive_lm_success]") for p in launch_problems(_spec(), vals)) -def test_unimportable_reward_is_refused(): - probs = positive_lm_success_problems(_spec(), dict(_VALS, reward_function="no_such_mod_xyz:score")) - assert probs and "cannot be imported" in probs[0] +def test_unimportable_reward_gets_no_verdict(): + assert positive_lm_success_problems(_spec(), dict(_VALS, reward_function="no_such_mod_xyz:score")) == [] + + +def test_user_reward_with_declaration_passes(monkeypatch): + mod = types.ModuleType("_user_reward") + + @writes_success + async def score(args, sample, **kw): + sample.metadata["success"] = True + return 1.0 + + mod.score = score + monkeypatch.setitem(sys.modules, "_user_reward", mod) + assert positive_lm_success_problems(_spec(), dict(_VALS, reward_function="_user_reward:score")) == [] def test_declared_in_package_rewards_pass(): @@ -69,7 +81,6 @@ def test_skips_without_reward_or_other_source(monkeypatch): def test_gsm8k_score_writes_success_matching_reward(): from yeto.rl.gsm8k_reward import score - assert getattr(score, SUCCESS_DECLARATION_ATTR) is True for response, want in (("so \\boxed{72}", 1.0), ("so \\boxed{71}", 0.0), ("", 0.0)): sample = types.SimpleNamespace(response=response, label="... #### 72", metadata={}) value = asyncio.run(score(None, sample)) diff --git a/yeto/rl/algos/critic.py b/yeto/rl/algos/critic.py index 53bb6c1cf..32e3a365d 100644 --- a/yeto/rl/algos/critic.py +++ b/yeto/rl/algos/critic.py @@ -100,30 +100,14 @@ def _reject_positive_lm(spec: AlgorithmSpec) -> str | None: register_rejection("positive_lm_threshold", _reject_positive_lm) -def _reward_declares_success(reward_function: str) -> bool | None: - """True/False: the ``module:function`` carries the ``writes_success`` - declaration; None: it cannot be imported here.""" - - import importlib - - from yeto.rl.math_reward import SUCCESS_DECLARATION_ATTR - - module, _, name = str(reward_function).partition(":") - try: - fn = getattr(importlib.import_module(module), name) - except Exception: # noqa: BLE001 - any import failure means "not confirmed" - return None - return bool(getattr(fn, SUCCESS_DECLARATION_ATTR, False)) - - def positive_lm_success_problems(spec: AlgorithmSpec, values: Mapping[str, Any]) -> list[str]: """``loss.positive_lm_source='success'`` needs a reward function declared - (``yeto.rl.math_reward.writes_success``) to write ``sample.metadata['success']``. + (``yeto.rl.reward_declarations``) to write ``sample.metadata['success']``. Without it the Miles fork raises at the first critic step on the island (S19 #6 s19-vapo-g3-20261010a: out-of-tree ``gsm8k_reward:score`` wrote no flag; S13 G1 passed with ``yeto.rl.gsm8k_reward:score``). Skipped when the - caller passes no ``reward_function``. + caller passes no ``reward_function`` or it cannot be imported here. """ if spec.loss.positive_lm_coef is None or spec.loss.positive_lm_source != "success": @@ -131,13 +115,15 @@ def positive_lm_success_problems(spec: AlgorithmSpec, values: Mapping[str, Any]) reward_function = values.get("reward_function") if not reward_function: return [] - declared = _reward_declares_success(reward_function) - if declared: + from yeto.rl.reward_declarations import reward_declares_success + + # None (not importable here): no verdict; the Miles fork still refuses on + # the island. Sub-agent decision: test fixtures use placeholder references. + if reward_declares_success(reward_function) is not False: return [] - why = ("cannot be imported to confirm it" if declared is None - else "is not declared with yeto.rl.math_reward.writes_success") return [f"loss.positive_lm_source='success' needs a reward function that writes " - f"sample.metadata['success']; {reward_function} {why}. Use e.g. " + f"sample.metadata['success']; {reward_function} is not declared with " + "yeto.rl.reward_declarations.writes_success. Use e.g. " "yeto.rl.gsm8k_reward:score, or positive_lm_source='reward' only when a " "positive reward always means complete success"] diff --git a/yeto/rl/gsm8k_reward.py b/yeto/rl/gsm8k_reward.py index 073831e7a..4ed2d1f4a 100644 --- a/yeto/rl/gsm8k_reward.py +++ b/yeto/rl/gsm8k_reward.py @@ -15,14 +15,13 @@ # Neutral form: yeto.rl.rewards.builtin.gsm8k_reward (decoupling 3.2); this # module keeps the Miles entry point and its signature. -from yeto.rl.math_reward import set_success, writes_success +from yeto.rl.math_reward import set_success from yeto.rl.rewards.builtin import _num, gsm8k_grade as grade, gsm8k_reward from yeto.rl.rewards.types import Trajectory __all__ = ["_num", "grade", "score"] -@writes_success async def score(args, sample, **kwargs): result = gsm8k_reward(Trajectory(response=sample.response or "", label=sample.label)) set_success(sample, result.metadata["success"]) diff --git a/yeto/rl/math_reward.py b/yeto/rl/math_reward.py index 621231d3b..fc97d0d60 100644 --- a/yeto/rl/math_reward.py +++ b/yeto/rl/math_reward.py @@ -39,22 +39,6 @@ def score(response: str, label) -> float: return 1.0 if grade_answer_mathd(answer, truth) or grade_answer_sympy(answer, truth) else 0.0 -SUCCESS_DECLARATION_ATTR = "yeto_writes_success" - - -def writes_success(fn): - """Declare that reward function ``fn`` writes ``sample.metadata['success']``. - - The launch check ``positive_lm_success`` (rl-algo-critic-family) rejects - ``loss.positive_lm_source='success'`` when the run's reward function lacks - this declaration: the Miles fork otherwise fails on the island at the first - critic step (S19 #6 VAPO G3 s19-vapo-g3-20261010a). - """ - - setattr(fn, SUCCESS_DECLARATION_ATTR, True) - return fn - - def set_success(sample, success: bool) -> None: """Write the boolean full-success flag into ``sample.metadata`` (in place).""" metadata = getattr(sample, "metadata", None) @@ -64,7 +48,6 @@ def set_success(sample, success: bool) -> None: metadata["success"] = bool(success) -@writes_success async def reward_func(args, sample, **kwargs) -> float: value = score(sample.response or "", sample.label) set_success(sample, value == 1.0) diff --git a/yeto/rl/reward_declarations.py b/yeto/rl/reward_declarations.py new file mode 100644 index 000000000..d40b15bbb --- /dev/null +++ b/yeto/rl/reward_declarations.py @@ -0,0 +1,42 @@ +"""Declarations about what a reward function writes into ``sample.metadata``. + +``loss.positive_lm_source='success'`` (VAPO positive-example LM loss) needs the +reward function to write the boolean ``sample.metadata['success']``. The launch +check ``positive_lm_success`` (yeto.rl.algos.critic) asks this module. + +Kept apart from the reward modules on purpose: their source hashes are part of +the decoupling golden samples (tests/decoupling_golden.py). +""" + +from __future__ import annotations + +import importlib + +SUCCESS_DECLARATION_ATTR = "yeto_writes_success" + +# In-package Miles reward entry points that call yeto.rl.math_reward.set_success. +SUCCESS_WRITING_REWARDS = frozenset({ + "yeto.rl.gsm8k_reward:score", + "yeto.rl.math_reward:reward_func", +}) + + +def writes_success(fn): + """Declare that reward function ``fn`` writes ``sample.metadata['success']``.""" + + setattr(fn, SUCCESS_DECLARATION_ATTR, True) + return fn + + +def reward_declares_success(reward_function: str) -> bool | None: + """True: declared; False: importable but undeclared; None: cannot import here.""" + + ref = str(reward_function) + if ref in SUCCESS_WRITING_REWARDS: + return True + module, _, name = ref.partition(":") + try: + fn = getattr(importlib.import_module(module), name) + except Exception: # noqa: BLE001 - any import failure means "cannot confirm" + return None + return bool(getattr(fn, SUCCESS_DECLARATION_ATTR, False))