mirror of https://github.com/razor-ai/soup.git
fix(train): reward-hack mitigation review fixes (v0.71.26)
Fixes from 5 sequential ECC reviews (python/code/security/tdd/verification). python-review (2 CRITICAL + HIGH/MED/LOW): - signal/vote coherence: schema now requires the active detector in reward_hack_signals + rejects the inactive detector name (was silently dropping the primary signal from the vote). - integral_clamp is its own field (was wrongly hard-wired to beta_ceil). - task/backend gate runs before controller-config checks; EMA convention corrected; type hints; mutable-list default -> tuple + normalised compare. code-review (4 HIGH + MED/LOW): - _prune now trims _saved in sync with disk (rollback can't target a deleted checkpoint); bang-bang release_count resets after each relaxation (hysteretic descent); EMA formula uses standard convention; _escalate no longer burns a recovery attempt on a None target; max_recovery_attempts>=1 required with rollback; _action_history capped; on_step_end logs errors once; loud warning when the mitigation callback can't attach (was a silent safety-off). security-review (HIGH + MED): - restore_checkpoint / save_checkpoint refuse a SYMLINKED optimizer.pt (torch.load weights_only=False was an RCE via attacker-placed symlink); bool-before-int/float guards on all new numeric fields; reward_hack_signals max_length=4; empty-signals guard in the callback. tdd-review: +13 coverage tests (dead-band hold, shim verbatim-on-error, conservative boundary, read-only-beta dual-write, escalation postconditions, both-restore, PID D exact, log cap + concurrency, no-top-level-import, fuzz field-validity). Test count 152 -> 180 (+2 POSIX-only symlink skips).
This commit is contained in:
Alpamys
parent
eb2edb1a51
commit
fa992bf381
|
|
@ -15,8 +15,6 @@ Reward-function signature (TRL GRPO/PPO): ``fn(prompts, completions, **kwargs)
|
|||
lists of ``{"role", "content"}`` message dicts (conversational).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
|
||||
# The synthetic "correct answer" the TRUE scorer looks for. Orthogonal to both
|
||||
|
|
|
|||
|
|
@ -1736,9 +1736,11 @@ class TrainingConfig(BaseModel):
|
|||
)
|
||||
reward_hack_signals: List[str] = Field(
|
||||
default_factory=lambda: ["info_rm"],
|
||||
max_length=4,
|
||||
description=(
|
||||
"v0.71.26 — signals combined into the controller's multi-signal "
|
||||
"vote. Allowlist: info_rm, rm_ensemble, length_trend, repetition."
|
||||
"vote. Allowlist (max 4): info_rm, rm_ensemble, length_trend, "
|
||||
"repetition."
|
||||
),
|
||||
)
|
||||
# ---- v0.71.26 Stage 2 — PID-Lagrangian controller + rollback ---------
|
||||
|
|
@ -1769,6 +1771,15 @@ class TrainingConfig(BaseModel):
|
|||
"(pid_lagrangian mode)."
|
||||
),
|
||||
)
|
||||
reward_hack_integral_clamp: float = Field(
|
||||
default=1.0,
|
||||
gt=0.0,
|
||||
le=1000.0,
|
||||
description=(
|
||||
"v0.71.26 — PID anti-windup bound on the integral accumulator "
|
||||
"(pid_lagrangian mode). Independent of beta_ceil."
|
||||
),
|
||||
)
|
||||
reward_hack_rollback: bool = Field(
|
||||
default=False,
|
||||
description=(
|
||||
|
|
@ -1852,6 +1863,36 @@ class TrainingConfig(BaseModel):
|
|||
f"reward-hack bool flag must be bool, got {type(v).__name__}"
|
||||
)
|
||||
|
||||
@field_validator(
|
||||
"reward_hack_dwell_steps",
|
||||
"reward_hack_release_patience",
|
||||
"reward_hack_rollback_patience",
|
||||
"reward_hack_max_recovery_attempts",
|
||||
"reward_hack_smoothing_window",
|
||||
"reward_hack_beta_floor",
|
||||
"reward_hack_beta_ceil",
|
||||
"reward_hack_trip_band",
|
||||
"reward_hack_release_band",
|
||||
"reward_hack_kl_gain",
|
||||
"reward_hack_pid_kp",
|
||||
"reward_hack_pid_ki",
|
||||
"reward_hack_pid_kd",
|
||||
"reward_hack_signal_target",
|
||||
"reward_hack_integral_clamp",
|
||||
"reward_hack_shaping_strength",
|
||||
mode="before",
|
||||
)
|
||||
@classmethod
|
||||
def _reject_bool_on_reward_hack_numerics(cls, v):
|
||||
"""v0.71.26 — bool-before-int/float policy (security-review MEDIUM): a
|
||||
YAML ``yes`` must not silently coerce to 1 on a numeric tunable."""
|
||||
if isinstance(v, bool):
|
||||
raise ValueError(
|
||||
"reward-hack numeric tunable must not be bool "
|
||||
"(YAML on/off/yes/no coerces to a number)"
|
||||
)
|
||||
return v
|
||||
|
||||
@field_validator("reward_hack_mitigation", mode="before")
|
||||
@classmethod
|
||||
def _coerce_reward_hack_mitigation(cls, v):
|
||||
|
|
@ -3260,18 +3301,19 @@ class EvalConfig(BaseModel):
|
|||
# per stage (Stage 2/3 tunables added with their fields).
|
||||
# Stage-2 (PID-Lagrangian + rollback) tunables — meaningful only in
|
||||
# pid_lagrangian mode. Setting one under any other mode is a no-op footgun.
|
||||
_REWARD_HACK_STAGE2_DEFAULTS: dict = {
|
||||
_REWARD_HACK_STAGE2_DEFAULTS: dict[str, Any] = {
|
||||
"reward_hack_pid_kp": 0.5,
|
||||
"reward_hack_pid_ki": 0.1,
|
||||
"reward_hack_pid_kd": 0.05,
|
||||
"reward_hack_signal_target": 0.15,
|
||||
"reward_hack_integral_clamp": 1.0,
|
||||
"reward_hack_rollback": False,
|
||||
"reward_hack_rollback_patience": 3,
|
||||
"reward_hack_max_recovery_attempts": 2,
|
||||
}
|
||||
|
||||
# Stage-3 (anti-gaming) tunables — meaningful for any non-off mode.
|
||||
_REWARD_HACK_STAGE3_DEFAULTS: dict = {
|
||||
_REWARD_HACK_STAGE3_DEFAULTS: dict[str, Any] = {
|
||||
"reward_hack_signal_smoothing": "none",
|
||||
"reward_hack_smoothing_window": 8,
|
||||
"reward_hack_conservative_on_disagreement": False,
|
||||
|
|
@ -3280,7 +3322,7 @@ _REWARD_HACK_STAGE3_DEFAULTS: dict = {
|
|||
"reward_hack_shaping_strength": 0.0,
|
||||
}
|
||||
|
||||
_REWARD_HACK_TUNABLE_DEFAULTS: dict = {
|
||||
_REWARD_HACK_TUNABLE_DEFAULTS: dict[str, Any] = {
|
||||
"reward_hack_beta_floor": 0.02,
|
||||
"reward_hack_beta_ceil": 1.0,
|
||||
"reward_hack_trip_band": 0.30,
|
||||
|
|
@ -3288,22 +3330,28 @@ _REWARD_HACK_TUNABLE_DEFAULTS: dict = {
|
|||
"reward_hack_dwell_steps": 2,
|
||||
"reward_hack_release_patience": 3,
|
||||
"reward_hack_kl_gain": 1.5,
|
||||
"reward_hack_signals": ["info_rm"],
|
||||
# tuple (not list) so a caller cannot mutate this module-level default.
|
||||
"reward_hack_signals": ("info_rm",),
|
||||
**_REWARD_HACK_STAGE2_DEFAULTS,
|
||||
**_REWARD_HACK_STAGE3_DEFAULTS,
|
||||
}
|
||||
|
||||
|
||||
def _customized_reward_hack_tunables(tcfg) -> list:
|
||||
def _customized_reward_hack_tunables(tcfg: Any) -> list[str]:
|
||||
"""Return the reward-hack control tunables set to a non-default value."""
|
||||
offenders = []
|
||||
offenders: list[str] = []
|
||||
for field_name, default in _REWARD_HACK_TUNABLE_DEFAULTS.items():
|
||||
if getattr(tcfg, field_name, default) != default:
|
||||
current = getattr(tcfg, field_name, default)
|
||||
# Normalise list/tuple so a list value compares equal to a tuple default.
|
||||
if isinstance(default, tuple) and isinstance(current, (list, tuple)):
|
||||
if tuple(current) != default:
|
||||
offenders.append(field_name)
|
||||
elif current != default:
|
||||
offenders.append(field_name)
|
||||
return offenders
|
||||
|
||||
|
||||
def _validate_reward_hack_controller(tcfg) -> None:
|
||||
def _validate_reward_hack_controller(tcfg: Any) -> None:
|
||||
"""Validate the mitigation-controller config (only when a mode is active).
|
||||
|
||||
Numeric consistency (β floor < ceil, release < trip band), the signal
|
||||
|
|
@ -3325,12 +3373,29 @@ def _validate_reward_hack_controller(tcfg) -> None:
|
|||
)
|
||||
from soup_cli.utils.reward_hack_control import SIGNAL_NAMES
|
||||
|
||||
for name in tcfg.reward_hack_signals or []:
|
||||
# The controller votes on the ACTIVE detector's signal plus the auxiliary
|
||||
# signals. Listing the other detector's name (never produced) or omitting
|
||||
# the active detector silently drops the primary signal from the vote —
|
||||
# reject both so the config is coherent (python-review CRITICAL #1).
|
||||
signals = list(tcfg.reward_hack_signals or [])
|
||||
allowed = {tcfg.reward_hack_detector, "length_trend", "repetition"}
|
||||
for name in signals:
|
||||
if name not in SIGNAL_NAMES:
|
||||
raise ValueError(
|
||||
f"reward_hack_signals contains unknown signal {name!r}; "
|
||||
f"valid: {sorted(SIGNAL_NAMES)}"
|
||||
)
|
||||
if name not in allowed:
|
||||
raise ValueError(
|
||||
f"reward_hack_signals contains {name!r}, but the active "
|
||||
f"detector is {tcfg.reward_hack_detector!r}; valid signals "
|
||||
f"are {sorted(allowed)}"
|
||||
)
|
||||
if tcfg.reward_hack_detector is not None and tcfg.reward_hack_detector not in signals:
|
||||
raise ValueError(
|
||||
"reward_hack_signals must include the active detector "
|
||||
f"{tcfg.reward_hack_detector!r} (its signal is the primary vote)"
|
||||
)
|
||||
# A control mode drives the KL/ref dynamics; a competing β schedule
|
||||
# (ref_model_ema_alpha regenerates the reference) fights it — reject.
|
||||
if tcfg.reward_hack_mitigation in ("kl_control", "pid_lagrangian"):
|
||||
|
|
@ -3358,6 +3423,14 @@ def _validate_reward_hack_controller(tcfg) -> None:
|
|||
"reward_hack_rollback=True requires rl_checkpoint_save_every_steps "
|
||||
"to be set (a cadence to roll back to)"
|
||||
)
|
||||
# max_recovery_attempts=0 with rollback would early-stop on the first HACK
|
||||
# streak WITHOUT a single rollback — a footgun (code-review MEDIUM).
|
||||
if tcfg.reward_hack_rollback and tcfg.reward_hack_max_recovery_attempts < 1:
|
||||
raise ValueError(
|
||||
"reward_hack_rollback=True requires "
|
||||
"reward_hack_max_recovery_attempts >= 1 (0 would early-stop "
|
||||
"before any rollback)"
|
||||
)
|
||||
# v0.71.26 Stage 3 — reward shaping MUTATES rewards, so it is only valid
|
||||
# for a control mode (log_only must stay observe-only).
|
||||
if tcfg.reward_hack_reward_shaping:
|
||||
|
|
@ -4965,10 +5038,9 @@ class SoupConfig(BaseModel):
|
|||
f"reward_hack_mitigation={mitigation!r} requires "
|
||||
"reward_hack_detector to be set (the signal source)"
|
||||
)
|
||||
# v0.71.26 — controller config (numeric bounds, signal allowlist,
|
||||
# β-schedule mutual exclusion) only when a mode is active.
|
||||
if mitigation != "off":
|
||||
_validate_reward_hack_controller(tcfg)
|
||||
# v0.71.26 — the task / backend gate runs BEFORE the controller-config
|
||||
# checks so a task mismatch surfaces the actionable error (not a
|
||||
# numeric-bounds error) — python-review HIGH #3.
|
||||
if self.task not in ("grpo", "ppo"):
|
||||
raise ValueError(
|
||||
"reward_hack_detector / reward_hack_halt / "
|
||||
|
|
@ -4980,6 +5052,10 @@ class SoupConfig(BaseModel):
|
|||
"reward_hack_detector / reward_hack_mitigation are not "
|
||||
"supported on backend=mlx (RL detectors are transformers-only)"
|
||||
)
|
||||
# Controller config (numeric bounds, signal allowlist, β-schedule
|
||||
# mutual exclusion) only when a mode is active.
|
||||
if mitigation != "off":
|
||||
_validate_reward_hack_controller(tcfg)
|
||||
return self
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -269,7 +269,7 @@ def _attach_reward_hack(
|
|||
signal_target=tcfg.reward_hack_signal_target,
|
||||
beta_floor=tcfg.reward_hack_beta_floor,
|
||||
beta_ceil=tcfg.reward_hack_beta_ceil,
|
||||
integral_clamp=tcfg.reward_hack_beta_ceil,
|
||||
integral_clamp=tcfg.reward_hack_integral_clamp,
|
||||
)
|
||||
callback = RewardHackMitigationCallback(
|
||||
mode=mitigation,
|
||||
|
|
@ -301,7 +301,15 @@ def _attach_reward_hack(
|
|||
callback.attach(trainer)
|
||||
return 1
|
||||
except (TypeError, ValueError, OSError) as exc:
|
||||
logger.debug("attach reward-hack mitigation callback rejected: %s", exc)
|
||||
# A user explicitly enabled mitigation — a silent drop would leave
|
||||
# them believing a safety controller is active when it is not.
|
||||
# Warn LOUDLY (e.g. output dir outside cwd fails the log writer).
|
||||
logger.warning(
|
||||
"reward-hack mitigation callback NOT attached (%s): %s. "
|
||||
"Training will proceed WITHOUT mitigation.",
|
||||
type(exc).__name__,
|
||||
exc,
|
||||
)
|
||||
return 0
|
||||
if detector is not None:
|
||||
from soup_cli.utils.reward_hacking import build_reward_hack_callback
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ Security:
|
|||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import logging
|
||||
import math
|
||||
import os
|
||||
import stat
|
||||
|
|
@ -43,6 +44,10 @@ from typing import Any
|
|||
from soup_cli.monitoring.trace_logger import redact_value
|
||||
from soup_cli.utils.paths import is_under_cwd
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_MAX_ACTION_HISTORY = 1000 # cap the in-memory action log for long runs
|
||||
|
||||
_DEFAULT_CAP_MB = 100
|
||||
_MIN_CAP_MB = 1
|
||||
_MAX_CAP_MB = 10_000
|
||||
|
|
@ -57,7 +62,7 @@ SIGNAL_NAMES: frozenset[str] = frozenset(
|
|||
SMOOTHING_METHODS: frozenset[str] = frozenset({"none", "ema", "median"})
|
||||
SHAPING_KINDS: frozenset[str] = frozenset({"length", "repetition", "sentinel"})
|
||||
|
||||
_EMA_ALPHA = 0.5 # fixed EMA weight on the new sample (documented, Stage 3)
|
||||
_EMA_ALPHA = 0.5 # EMA smoothing factor: weight on the NEW sample (1 - it on prev)
|
||||
_CONSERVATIVE_DISAGREE_TOL = 0.2 # detectors differ beyond this → stay cautious
|
||||
|
||||
|
||||
|
|
@ -150,9 +155,10 @@ def combine_signals(signals: Mapping[str, Any], names: Sequence[str]) -> float:
|
|||
|
||||
|
||||
def smooth_signal(new: float, window: Sequence[float], *, method: str) -> float:
|
||||
"""Smooth a scalar signal. ``none`` → new; ``ema`` → 0.5·prev + 0.5·new
|
||||
(prev = ``window[-1]``, or ``new`` when the window is empty); ``median`` →
|
||||
median of ``window + [new]``.
|
||||
"""Smooth a scalar signal. ``none`` → new; ``ema`` → ``alpha·new +
|
||||
(1-alpha)·prev`` with alpha = ``_EMA_ALPHA`` = 0.5 (prev = ``window[-1]``,
|
||||
or ``new`` when the window is empty); ``median`` → median of
|
||||
``window + [new]``.
|
||||
"""
|
||||
if method not in SMOOTHING_METHODS:
|
||||
raise ValueError(
|
||||
|
|
@ -165,7 +171,8 @@ def smooth_signal(new: float, window: Sequence[float], *, method: str) -> float:
|
|||
if method == "ema":
|
||||
if not win:
|
||||
return fnew
|
||||
return _EMA_ALPHA * win[-1] + (1.0 - _EMA_ALPHA) * fnew
|
||||
# Standard EMA convention: alpha weights the NEW sample.
|
||||
return _EMA_ALPHA * fnew + (1.0 - _EMA_ALPHA) * win[-1]
|
||||
return float(statistics.median(win + [fnew]))
|
||||
|
||||
|
||||
|
|
@ -239,7 +246,7 @@ def _completion_text(completion: Any) -> str:
|
|||
return _completion_to_text(completion)
|
||||
|
||||
|
||||
def _extract_completions(args: tuple, kwargs: dict) -> Any:
|
||||
def _extract_completions(args: tuple[Any, ...], kwargs: dict[str, Any]) -> Any:
|
||||
"""Pull the ``completions`` arg from a TRL reward-fn call."""
|
||||
completions = kwargs.get("completions")
|
||||
if completions is None:
|
||||
|
|
@ -279,6 +286,10 @@ def shape_reward_fn(
|
|||
never corrupt training: on any exception the verbatim reward is returned.
|
||||
``__name__`` is preserved so TRL's per-function logging keys stay correct.
|
||||
"""
|
||||
if not callable(inner):
|
||||
raise TypeError(
|
||||
f"shape_reward_fn inner must be callable, got {type(inner).__name__}"
|
||||
)
|
||||
if kind not in SHAPING_KINDS:
|
||||
raise ValueError(f"kind must be one of {sorted(SHAPING_KINDS)}, got {kind!r}")
|
||||
strength_val = _check_finite_float(strength, "strength", nonneg=True)
|
||||
|
|
@ -343,7 +354,7 @@ def explain_giveup(
|
|||
f"The '{signal_name}' signal stayed elevated (last smoothed drop_pct="
|
||||
f"{state.last_signal:.3f}).",
|
||||
]
|
||||
recent = [str(a) for a in list(action_history)[-5:]]
|
||||
recent = [str(a) for a in action_history[-5:]]
|
||||
if recent:
|
||||
lines.append("Recent actions tried: " + " | ".join(recent))
|
||||
lines.append(
|
||||
|
|
@ -517,9 +528,13 @@ def bang_bang_step(
|
|||
if new_beta != beta:
|
||||
reason = f"relax beta to {new_beta:.4f} (vote={fvote:.3f})"
|
||||
beta = new_beta
|
||||
# Reset the patience counter after EACH relaxation so the
|
||||
# descent is hysteretic — β must see release_patience fresh
|
||||
# below-band steps before the next geometric relaxation
|
||||
# (v0.71.26 code-review HIGH: raise fast, relax cautiously).
|
||||
release = 0
|
||||
if beta <= policy.beta_floor:
|
||||
tripped = False
|
||||
release = 0
|
||||
else:
|
||||
release = 0
|
||||
else:
|
||||
|
|
@ -775,6 +790,10 @@ class RewardHackMitigationCallback(_TrainerCallbackBase): # type: ignore[misc,
|
|||
self.detector = validate_hack_detector(detector)
|
||||
self.log_writer = log_writer
|
||||
self.signals = tuple(signals)
|
||||
if not self.signals:
|
||||
# An empty signal set makes the vote always 0.0 → the controller is
|
||||
# active but permanently inert (security-review LOW #4).
|
||||
raise ValueError("signals must be non-empty (the controller vote)")
|
||||
for name in self.signals:
|
||||
if name not in SIGNAL_NAMES:
|
||||
raise ValueError(
|
||||
|
|
@ -814,6 +833,13 @@ class RewardHackMitigationCallback(_TrainerCallbackBase): # type: ignore[misc,
|
|||
self._signal_windows: dict[str, list[float]] = {}
|
||||
self._action_history: list[str] = []
|
||||
self._last_drift = False
|
||||
self._warned_error = False
|
||||
|
||||
def _record_action(self, reason: str) -> None:
|
||||
"""Append an action reason, capping the in-memory history (LOW #9)."""
|
||||
self._action_history.append(reason)
|
||||
if len(self._action_history) > _MAX_ACTION_HISTORY:
|
||||
del self._action_history[:-_MAX_ACTION_HISTORY]
|
||||
|
||||
def attach(self, trainer: Any) -> None:
|
||||
"""Store the trainer reference (the β / kl_coef mutation target)."""
|
||||
|
|
@ -965,7 +991,7 @@ class RewardHackMitigationCallback(_TrainerCallbackBase): # type: ignore[misc,
|
|||
new_state, action = bang_bang_step(policy, self._state, vote=vote)
|
||||
self._state = new_state
|
||||
self._apply_coefficient(action.new_beta)
|
||||
self._action_history.append(action.reason)
|
||||
self._record_action(action.reason)
|
||||
telemetry["vote"] = vote
|
||||
telemetry["new_beta"] = action.new_beta
|
||||
telemetry["tripped"] = action.tripped
|
||||
|
|
@ -982,6 +1008,16 @@ class RewardHackMitigationCallback(_TrainerCallbackBase): # type: ignore[misc,
|
|||
self, model: Any, optimizer: Any, control: Any, telemetry: dict[str, Any]
|
||||
) -> Any:
|
||||
"""Escalation ladder rung: rollback to last-good, else early-stop."""
|
||||
target = self._last_good_step
|
||||
# No last-good checkpoint yet — do NOT burn a recovery attempt or
|
||||
# early-stop; keep training until a good checkpoint exists or the real
|
||||
# rollback budget is spent (v0.71.26 code-review MEDIUM).
|
||||
if target is None or self.rl_checkpoint_cb is None:
|
||||
telemetry["escalation"] = (
|
||||
"no rollback target available yet (no saved checkpoint)"
|
||||
)
|
||||
self._hack_streak = 0
|
||||
return control
|
||||
if self._state.recovery_attempts >= self.max_recovery_attempts:
|
||||
telemetry["escalation"] = "early_stop"
|
||||
telemetry["explanation"] = explain_giveup(
|
||||
|
|
@ -991,17 +1027,19 @@ class RewardHackMitigationCallback(_TrainerCallbackBase): # type: ignore[misc,
|
|||
)
|
||||
self._request_stop(control)
|
||||
return control
|
||||
target = self._last_good_step
|
||||
restored = False
|
||||
if target is not None and self.rl_checkpoint_cb is not None:
|
||||
try:
|
||||
restored = bool(
|
||||
self.rl_checkpoint_cb.restore_checkpoint(
|
||||
step=target, model=model, optimizer=optimizer
|
||||
)
|
||||
try:
|
||||
restored = bool(
|
||||
self.rl_checkpoint_cb.restore_checkpoint(
|
||||
step=target, model=model, optimizer=optimizer
|
||||
)
|
||||
except Exception: # noqa: BLE001 — rollback must never crash the run
|
||||
restored = False
|
||||
)
|
||||
except Exception: # noqa: BLE001 — rollback must never crash the run
|
||||
restored = False
|
||||
# NOTE (known limitation): the rollback restores the model weights +
|
||||
# optimizer, but the controller's β / integral state is intentionally
|
||||
# NOT reset — we keep KL elevated while recovering from hacking. The PID
|
||||
# continues from its last state, which is the conservative choice.
|
||||
self._state = replace(
|
||||
self._state, recovery_attempts=self._state.recovery_attempts + 1
|
||||
)
|
||||
|
|
@ -1026,7 +1064,7 @@ class RewardHackMitigationCallback(_TrainerCallbackBase): # type: ignore[misc,
|
|||
new_state, action = pid_step(policy, self._state, signal=vote)
|
||||
self._state = new_state
|
||||
self._apply_coefficient(action.new_beta)
|
||||
self._action_history.append(action.reason)
|
||||
self._record_action(action.reason)
|
||||
telemetry["vote"] = vote
|
||||
telemetry["new_beta"] = action.new_beta
|
||||
telemetry["tripped"] = action.tripped
|
||||
|
|
@ -1043,7 +1081,7 @@ class RewardHackMitigationCallback(_TrainerCallbackBase): # type: ignore[misc,
|
|||
control = self._escalate(model, optimizer, control, telemetry)
|
||||
return control
|
||||
|
||||
def on_step_end(self, args, state, control, **kwargs):
|
||||
def on_step_end(self, args: Any, state: Any, control: Any, **kwargs: Any) -> Any:
|
||||
"""Per-step hook — read the buffer, compute telemetry, act by mode.
|
||||
|
||||
Instrumentation must NEVER crash training: a broad except returns the
|
||||
|
|
@ -1069,5 +1107,16 @@ class RewardHackMitigationCallback(_TrainerCallbackBase): # type: ignore[misc,
|
|||
)
|
||||
self.log_writer.record(step=step, snapshot=telemetry)
|
||||
return control
|
||||
except Exception: # noqa: BLE001 — instrumentation must never crash
|
||||
except Exception as exc: # noqa: BLE001 — instrumentation must never crash
|
||||
# Training must not crash, but a persistent controller bug silently
|
||||
# disabling the safety loop must be visible — warn ONCE (code-review
|
||||
# LOW #10), not every step.
|
||||
if not self._warned_error:
|
||||
self._warned_error = True
|
||||
logger.warning(
|
||||
"reward-hack mitigation callback error (%s): %s. The "
|
||||
"controller is inactive this step; training continues.",
|
||||
type(exc).__name__,
|
||||
exc,
|
||||
)
|
||||
return control
|
||||
|
|
|
|||
|
|
@ -28,9 +28,12 @@ Security:
|
|||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import logging
|
||||
from dataclasses import dataclass
|
||||
from typing import Any, Optional
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_MAX_SAVE_EVERY_STEPS = 10_000_000
|
||||
_MIN_KEEP_LAST = 1
|
||||
_MAX_KEEP_LAST = 100
|
||||
|
|
@ -285,16 +288,19 @@ class RLCheckpointCallback(_TrainerCallbackBase): # type: ignore[misc, valid-ty
|
|||
|
||||
has_optimizer = False
|
||||
if self.config.include_optimizer_state and optimizer is not None:
|
||||
try:
|
||||
import torch
|
||||
|
||||
torch.save(
|
||||
optimizer.state_dict(),
|
||||
os.path.join(ckpt_dir, "optimizer.pt"),
|
||||
)
|
||||
has_optimizer = True
|
||||
except Exception: # noqa: BLE001 — best-effort, manifest reflects it
|
||||
opt_out = os.path.join(ckpt_dir, "optimizer.pt")
|
||||
# Refuse to write THROUGH a pre-placed symlink (write-to-arbitrary
|
||||
# path in a shared checkpoint dir; security-review LOW #6).
|
||||
if os.path.islink(opt_out):
|
||||
has_optimizer = False
|
||||
else:
|
||||
try:
|
||||
import torch
|
||||
|
||||
torch.save(optimizer.state_dict(), opt_out)
|
||||
has_optimizer = True
|
||||
except Exception: # noqa: BLE001 — best-effort, manifest reflects it
|
||||
has_optimizer = False
|
||||
|
||||
manifest = RLCheckpointState(
|
||||
step=int(step),
|
||||
|
|
@ -343,12 +349,22 @@ class RLCheckpointCallback(_TrainerCallbackBase): # type: ignore[misc, valid-ty
|
|||
pass
|
||||
if optimizer is not None:
|
||||
opt_path = os.path.join(ckpt_dir, "optimizer.pt")
|
||||
if os.path.isfile(opt_path):
|
||||
# SECURITY (review HIGH #1): torch.load(weights_only=False) executes
|
||||
# arbitrary pickle. Refuse a SYMLINKED optimizer.pt — an attacker
|
||||
# with write access to a shared checkpoint dir could swap the file
|
||||
# for a symlink to a malicious pickle between save and restore (RCE).
|
||||
# opt_path is otherwise contained (output_dir is_under_cwd-verified in
|
||||
# __init__ + int-cast step); the symlink check closes the TOCTOU.
|
||||
if os.path.islink(opt_path):
|
||||
logger.warning(
|
||||
"refusing to restore optimizer state from a symlinked "
|
||||
"optimizer.pt (%s) — possible tampering",
|
||||
opt_path,
|
||||
)
|
||||
elif os.path.isfile(opt_path):
|
||||
try:
|
||||
import torch
|
||||
|
||||
# Trusted file (we wrote it under the cwd-contained run dir);
|
||||
# weights_only=False loads the full optimizer state_dict.
|
||||
optimizer.load_state_dict(
|
||||
torch.load(
|
||||
opt_path, map_location="cpu", weights_only=False
|
||||
|
|
@ -376,11 +392,16 @@ class RLCheckpointCallback(_TrainerCallbackBase): # type: ignore[misc, valid-ty
|
|||
continue
|
||||
entries.append((_step_number(name), full))
|
||||
entries.sort(key=lambda t: t[0], reverse=True)
|
||||
for _, path in entries[self.config.keep_last:]:
|
||||
for step_num, path in entries[self.config.keep_last:]:
|
||||
try:
|
||||
shutil.rmtree(path)
|
||||
except OSError:
|
||||
pass
|
||||
continue
|
||||
# Keep the in-memory ledger in sync with disk so a rollback target
|
||||
# (max(_saved)) can never point at a deleted checkpoint (v0.71.26
|
||||
# code-review HIGH — the reward-hack rollback ladder reads _saved).
|
||||
if step_num in self._saved:
|
||||
self._saved.remove(step_num)
|
||||
|
||||
def on_step_end(self, args, state, control, model=None, **kwargs):
|
||||
"""Per-step hook — save a checkpoint on the configured cadence."""
|
||||
|
|
|
|||
|
|
@ -1807,12 +1807,27 @@ class TestControllerFuzz:
|
|||
state = ControllerState(beta=policy.beta_floor)
|
||||
prev = state.beta
|
||||
for vote in trace:
|
||||
state, _ = bang_bang_step(policy, state, vote=vote)
|
||||
state, action = bang_bang_step(policy, state, vote=vote)
|
||||
assert policy.beta_floor - 1e-9 <= state.beta <= policy.beta_ceil + 1e-9
|
||||
assert state.beta > 0.0 and math.isfinite(state.beta)
|
||||
assert state.beta / prev <= policy.kl_gain + 1e-9 # geometric only
|
||||
# field-validity invariants (tdd review LOW #8)
|
||||
assert 0.0 <= state.last_signal <= 1.0
|
||||
assert state.dwell_count >= 0 and state.release_count >= 0
|
||||
assert isinstance(state.tripped, bool)
|
||||
assert action.new_beta == state.beta
|
||||
prev = state.beta
|
||||
|
||||
def test_bang_bang_converges_on_sustained_signal(self):
|
||||
# convergence property: sustained above-band input eventually trips.
|
||||
from soup_cli.utils.reward_hack_control import ControllerState, bang_bang_step
|
||||
|
||||
policy = _bang_policy(dwell_steps=3)
|
||||
state = ControllerState(beta=0.02)
|
||||
for _ in range(5):
|
||||
state, _ = bang_bang_step(policy, state, vote=0.9)
|
||||
assert state.tripped and state.beta > 0.02
|
||||
|
||||
def test_bang_bang_no_flap_on_alternation(self):
|
||||
from soup_cli.utils.reward_hack_control import ControllerState, bang_bang_step
|
||||
|
||||
|
|
@ -1833,3 +1848,413 @@ class TestControllerFuzz:
|
|||
assert policy.beta_floor - 1e-9 <= state.beta <= policy.beta_ceil + 1e-9
|
||||
assert state.beta > 0.0 and math.isfinite(state.beta)
|
||||
assert abs(state.integral) <= policy.integral_clamp + 1e-9
|
||||
assert math.isfinite(state.integral) and math.isfinite(state.prev_error)
|
||||
assert 0.0 <= state.last_signal <= 1.0
|
||||
|
||||
|
||||
# =====================================================================
|
||||
# python-review fixes (v0.71.26)
|
||||
# =====================================================================
|
||||
|
||||
|
||||
class TestReviewFixesPython:
|
||||
"""Regression tests for the python-review findings."""
|
||||
|
||||
def _cfg(self, extra, *, mitigation="kl_control", detector="info_rm", task="grpo"):
|
||||
from soup_cli.config.loader import load_config_from_string
|
||||
|
||||
return load_config_from_string(
|
||||
_yaml(task, mitigation=mitigation, detector=detector, extra=extra)
|
||||
)
|
||||
|
||||
def test_signals_must_include_active_detector(self):
|
||||
# CRITICAL #1 — a signal set that omits the active detector silently
|
||||
# drops the primary signal from the vote; reject it.
|
||||
with pytest.raises(ValueError, match="detector"):
|
||||
self._cfg("reward_hack_signals: [length_trend]")
|
||||
|
||||
def test_signals_reject_inactive_detector_name(self):
|
||||
# detector=info_rm but signals lists rm_ensemble (never produced) → reject.
|
||||
with pytest.raises(ValueError, match="rm_ensemble|active detector"):
|
||||
self._cfg("reward_hack_signals: [info_rm, rm_ensemble]")
|
||||
|
||||
def test_integral_clamp_is_a_field(self):
|
||||
# CRITICAL #2 — integral_clamp must be its own tunable, not beta_ceil.
|
||||
from soup_cli.config.schema import TrainingConfig
|
||||
|
||||
assert TrainingConfig().reward_hack_integral_clamp == 1.0
|
||||
cfg = self._cfg(
|
||||
"reward_hack_integral_clamp: 5.0", mitigation="pid_lagrangian"
|
||||
)
|
||||
assert cfg.training.reward_hack_integral_clamp == 5.0
|
||||
|
||||
def test_integral_clamp_wired_into_pid_policy(self, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
from soup_cli.config.schema import TrainingConfig
|
||||
from soup_cli.utils.peft_wiring import attach_rl_callbacks
|
||||
from soup_cli.utils.reward_hack_control import RewardHackMitigationCallback
|
||||
|
||||
tcfg = TrainingConfig(
|
||||
reward_hack_mitigation="pid_lagrangian",
|
||||
reward_hack_detector="info_rm",
|
||||
reward_hack_integral_clamp=7.0,
|
||||
reward_hack_beta_ceil=3.0,
|
||||
)
|
||||
added: list = []
|
||||
attach_rl_callbacks(
|
||||
_fake_trainer_recording(added),
|
||||
tcfg,
|
||||
buffer=object(),
|
||||
output_dir=str(tmp_path),
|
||||
task="grpo",
|
||||
)
|
||||
mit = [c for c in added if isinstance(c, RewardHackMitigationCallback)][0]
|
||||
assert mit.pid.integral_clamp == 7.0 # not beta_ceil (3.0)
|
||||
|
||||
def test_task_gate_before_controller_checks(self):
|
||||
# HIGH #3 — a bad beta bound on an sft task should surface the task
|
||||
# error, not the numeric one.
|
||||
with pytest.raises(ValueError, match="grpo|ppo|task"):
|
||||
self._cfg(
|
||||
"reward_hack_beta_floor: 1.0\nreward_hack_beta_ceil: 0.5",
|
||||
task="sft",
|
||||
)
|
||||
|
||||
def test_shape_reward_fn_rejects_non_callable(self):
|
||||
# MEDIUM #10 — non-callable inner must fail fast, not at train time.
|
||||
from soup_cli.utils.reward_hack_control import shape_reward_fn
|
||||
|
||||
with pytest.raises((TypeError, ValueError), match="callable"):
|
||||
shape_reward_fn("not a fn", kind="length", strength=0.5)
|
||||
|
||||
|
||||
class TestReviewFixesCode:
|
||||
"""Regression tests for the code-review findings."""
|
||||
|
||||
def test_prune_trims_saved_list(self, tmp_path, monkeypatch):
|
||||
# HIGH #1 — _prune must keep _saved in sync with surviving dirs so the
|
||||
# rollback target can never point at a deleted checkpoint.
|
||||
import os
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
from soup_cli.utils.rl_checkpoint import (
|
||||
RLCheckpointConfig,
|
||||
build_rl_checkpoint_callback,
|
||||
)
|
||||
|
||||
cb = build_rl_checkpoint_callback(
|
||||
RLCheckpointConfig(save_every_steps=1, keep_last=2),
|
||||
output_dir="run",
|
||||
task="grpo",
|
||||
)
|
||||
for step in (1, 2, 3, 4):
|
||||
cb.save_checkpoint(step=step, model=_FakeSavableModel(), optimizer=None)
|
||||
root = os.path.join("run", "rl-checkpoints")
|
||||
on_disk = sorted(
|
||||
int(d.split("-")[1]) for d in os.listdir(root) if d.startswith("step-")
|
||||
)
|
||||
assert on_disk == [3, 4]
|
||||
assert sorted(cb._saved) == on_disk # in-memory list matches disk
|
||||
|
||||
def test_bang_release_requires_patience_per_relaxation(self):
|
||||
# HIGH #4 — each geometric relaxation must re-accumulate release_patience.
|
||||
from soup_cli.utils.reward_hack_control import ControllerState, bang_bang_step
|
||||
|
||||
policy = _bang_policy(dwell_steps=1, release_patience=2, kl_gain=1.5, beta_ceil=1.0)
|
||||
state = ControllerState(beta=0.02)
|
||||
for _ in range(3): # raise β three times → ~0.0675
|
||||
state, _ = bang_bang_step(policy, state, vote=0.9)
|
||||
high = state.beta
|
||||
state, _ = bang_bang_step(policy, state, vote=0.0) # release=1, no relax
|
||||
assert state.beta == pytest.approx(high)
|
||||
state, _ = bang_bang_step(policy, state, vote=0.0) # release=2 → relax
|
||||
after_first = state.beta
|
||||
assert after_first < high
|
||||
state, _ = bang_bang_step(policy, state, vote=0.0) # release reset → 1, no relax
|
||||
assert state.beta == pytest.approx(after_first)
|
||||
state, _ = bang_bang_step(policy, state, vote=0.0) # release=2 → relax again
|
||||
assert state.beta < after_first
|
||||
|
||||
def test_rollback_requires_nonzero_recovery_attempts(self):
|
||||
# MEDIUM #5 — rollback=True with max_recovery_attempts=0 is a footgun.
|
||||
from soup_cli.config.loader import load_config_from_string
|
||||
|
||||
with pytest.raises(ValueError, match="max_recovery_attempts"):
|
||||
load_config_from_string(
|
||||
_yaml(
|
||||
"grpo",
|
||||
mitigation="pid_lagrangian",
|
||||
extra=(
|
||||
"reward_hack_rollback: true\n"
|
||||
"rl_checkpoint_save_every_steps: 2\n"
|
||||
"reward_hack_max_recovery_attempts: 0"
|
||||
),
|
||||
)
|
||||
)
|
||||
|
||||
def test_escalate_no_target_does_not_waste_attempt(self, tmp_path, monkeypatch):
|
||||
# MEDIUM #6 — a rollback with no last-good checkpoint must not burn a
|
||||
# recovery attempt nor early-stop.
|
||||
monkeypatch.chdir(tmp_path)
|
||||
ckpt = _FakeCkptCb(saved=[]) # no checkpoints saved yet
|
||||
cb = _pid_callback(
|
||||
tmp_path,
|
||||
_SeqBuffer([_HEALTHY, _HACK, _HACK, _HACK, _HACK]),
|
||||
rollback=True,
|
||||
rollback_patience=2,
|
||||
max_recovery_attempts=1,
|
||||
ckpt_cb=ckpt,
|
||||
)
|
||||
cb.attach(_fake_grpo_trainer(beta=0.02))
|
||||
control = types.SimpleNamespace(should_training_stop=False)
|
||||
for step in range(1, 6):
|
||||
control = cb.on_step_end(
|
||||
None, types.SimpleNamespace(global_step=step), control,
|
||||
model=object(), optimizer=object(),
|
||||
)
|
||||
assert ckpt.restore_calls == []
|
||||
assert cb._state.recovery_attempts == 0 # not wasted on a None target
|
||||
|
||||
def test_action_history_is_bounded(self, tmp_path, monkeypatch):
|
||||
# LOW #9 — _action_history must not grow unbounded.
|
||||
monkeypatch.chdir(tmp_path)
|
||||
from itertools import repeat
|
||||
|
||||
cb = _kl_callback(tmp_path, _SeqBuffer(list(repeat(_HACK, 1))))
|
||||
cb.attach(_fake_grpo_trainer(beta=0.02))
|
||||
for step in range(1, 60):
|
||||
cb.on_step_end(None, types.SimpleNamespace(global_step=step), None)
|
||||
assert len(cb._action_history) <= 1000
|
||||
|
||||
|
||||
class TestReviewFixesSecurity:
|
||||
"""Regression tests for the security-review findings."""
|
||||
|
||||
@pytest.mark.skipif(os.name == "nt", reason="symlink needs privilege on Windows")
|
||||
def test_restore_refuses_symlink_optimizer(self, tmp_path, monkeypatch):
|
||||
# HIGH #1 — torch.load(weights_only=False) on an attacker-symlinked
|
||||
# optimizer.pt is RCE; restore must refuse a symlinked file.
|
||||
import torch
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
from soup_cli.utils.rl_checkpoint import (
|
||||
RLCheckpointConfig,
|
||||
build_rl_checkpoint_callback,
|
||||
)
|
||||
|
||||
cb = build_rl_checkpoint_callback(
|
||||
RLCheckpointConfig(save_every_steps=1), output_dir="run", task="grpo"
|
||||
)
|
||||
param = torch.nn.Parameter(torch.zeros(2))
|
||||
opt = torch.optim.SGD([param], lr=0.1)
|
||||
cb.save_checkpoint(step=1, model=_FakeSavableModel(), optimizer=opt)
|
||||
opt_path = os.path.join("run", "rl-checkpoints", "step-1", "optimizer.pt")
|
||||
evil = tmp_path / "evil.pt"
|
||||
evil.write_bytes(b"junk")
|
||||
os.remove(opt_path)
|
||||
os.symlink(str(evil), opt_path)
|
||||
# must refuse the symlinked optimizer (return False, never torch.load it)
|
||||
assert cb.restore_checkpoint(step=1, model=None, optimizer=opt) is False
|
||||
|
||||
def test_bool_rejected_on_int_fields(self):
|
||||
# MEDIUM #2 — bool-before-int policy on the new integer fields.
|
||||
from soup_cli.config.schema import TrainingConfig
|
||||
|
||||
for field in (
|
||||
"reward_hack_dwell_steps",
|
||||
"reward_hack_release_patience",
|
||||
"reward_hack_rollback_patience",
|
||||
"reward_hack_max_recovery_attempts",
|
||||
"reward_hack_smoothing_window",
|
||||
):
|
||||
with pytest.raises((ValueError, TypeError), match="bool"):
|
||||
TrainingConfig(**{field: True})
|
||||
|
||||
def test_bool_rejected_on_float_fields(self):
|
||||
from soup_cli.config.schema import TrainingConfig
|
||||
|
||||
# pid_kp has ge=0 so True→1.0 would pass the bound without a bool guard.
|
||||
with pytest.raises((ValueError, TypeError), match="bool"):
|
||||
TrainingConfig(reward_hack_pid_kp=True)
|
||||
|
||||
def test_signals_length_capped(self):
|
||||
# MEDIUM #3 — unbounded signals list is a per-step DoS.
|
||||
from soup_cli.config.schema import TrainingConfig
|
||||
|
||||
with pytest.raises((ValueError, TypeError)):
|
||||
TrainingConfig(reward_hack_signals=["info_rm"] * 100)
|
||||
|
||||
def test_callback_rejects_empty_signals(self, tmp_path, monkeypatch):
|
||||
# LOW #4 — an empty signals tuple silently disables the controller.
|
||||
monkeypatch.chdir(tmp_path)
|
||||
from soup_cli.utils.reward_hack_control import (
|
||||
MitigationLogWriter,
|
||||
RewardHackMitigationCallback,
|
||||
)
|
||||
|
||||
with pytest.raises(ValueError, match="signal"):
|
||||
RewardHackMitigationCallback(
|
||||
mode="log_only",
|
||||
detector="info_rm",
|
||||
log_writer=MitigationLogWriter("m.jsonl"),
|
||||
signals=(),
|
||||
)
|
||||
|
||||
|
||||
class TestReviewFixesTdd:
|
||||
"""Coverage gaps identified by the tdd review."""
|
||||
|
||||
def test_bang_bang_deadband_hold_while_tripped(self):
|
||||
# GAP 1 (HIGH) — the dead-band 'hold' branch while tripped must keep β
|
||||
# and reset both counters, without relaxing.
|
||||
from soup_cli.utils.reward_hack_control import ControllerState, bang_bang_step
|
||||
|
||||
policy = _bang_policy(dwell_steps=2, release_patience=2, trip_band=0.3, release_band=0.1)
|
||||
state = ControllerState(beta=0.02)
|
||||
state, _ = bang_bang_step(policy, state, vote=0.5)
|
||||
state, _ = bang_bang_step(policy, state, vote=0.5) # trip → β=0.03
|
||||
assert state.tripped and state.beta == pytest.approx(0.03)
|
||||
state, action = bang_bang_step(policy, state, vote=0.2) # dead-band
|
||||
assert state.beta == pytest.approx(0.03) and state.tripped
|
||||
assert state.release_count == 0 and state.dwell_count == 0
|
||||
assert action.reason == "hold"
|
||||
|
||||
def test_shape_reward_verbatim_on_shim_error(self, monkeypatch):
|
||||
# GAP 2 (HIGH) — a shim error must return the verbatim inner reward.
|
||||
import soup_cli.utils.reward_hack_control as rhc
|
||||
|
||||
def boom(*a, **k):
|
||||
raise RuntimeError("boom")
|
||||
|
||||
monkeypatch.setattr(rhc, "_shaping_penalty", boom)
|
||||
shaped = rhc.shape_reward_fn(_inner_reward, kind="length", strength=0.5)
|
||||
assert shaped(["p"], ["w " * 40]) == [1.0] # verbatim despite shim error
|
||||
|
||||
def test_conservative_disagreement_boundary(self):
|
||||
# GAP 3 (MEDIUM) — max-min == tol is NOT > tol → mean (not max).
|
||||
from soup_cli.utils.reward_hack_control import combine_conservative
|
||||
|
||||
assert combine_conservative([0.1, 0.3], disagree_tol=0.2) == pytest.approx(0.2)
|
||||
assert combine_conservative([0.1, 0.301], disagree_tol=0.2) == pytest.approx(0.301)
|
||||
|
||||
def test_dual_write_survives_readonly_beta(self, tmp_path, monkeypatch):
|
||||
# GAP 4 (MEDIUM) — a read-only trainer.beta must not block args.beta.
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
class _ROTrainer:
|
||||
def __init__(self):
|
||||
self.args = types.SimpleNamespace(beta=0.02)
|
||||
|
||||
@property
|
||||
def beta(self):
|
||||
return 0.02 # read-only property
|
||||
|
||||
cb = _kl_callback(tmp_path, _SeqBuffer([_HEALTHY, _HACK]))
|
||||
trainer = _ROTrainer()
|
||||
cb.attach(trainer)
|
||||
cb.on_step_end(None, types.SimpleNamespace(global_step=1), None)
|
||||
cb.on_step_end(None, types.SimpleNamespace(global_step=2), None)
|
||||
assert trainer.args.beta == pytest.approx(0.04) # args.beta still updated
|
||||
|
||||
def test_escalation_postconditions(self, tmp_path, monkeypatch):
|
||||
# GAP 5 (MEDIUM) — recovery_attempts increments to 1, hack_streak resets.
|
||||
monkeypatch.chdir(tmp_path)
|
||||
ckpt = _FakeCkptCb(saved=[10])
|
||||
cb = _pid_callback(
|
||||
tmp_path,
|
||||
_SeqBuffer([_HEALTHY, _HACK, _HACK]),
|
||||
rollback=True,
|
||||
rollback_patience=2,
|
||||
max_recovery_attempts=2,
|
||||
ckpt_cb=ckpt,
|
||||
)
|
||||
cb.attach(_fake_grpo_trainer(beta=0.02))
|
||||
control = types.SimpleNamespace(should_training_stop=False)
|
||||
for step in (1, 2, 3):
|
||||
control = cb.on_step_end(
|
||||
None, types.SimpleNamespace(global_step=step), control,
|
||||
model=object(), optimizer=object(),
|
||||
)
|
||||
assert ckpt.restore_calls == [10]
|
||||
assert cb._state.recovery_attempts == 1
|
||||
assert cb._hack_streak == 0
|
||||
assert control.should_training_stop is False # max=2 → not stopped yet
|
||||
|
||||
def test_pid_derivative_exact_spike(self):
|
||||
# GAP 2b — pin the exact D-term magnitude, not just > floor.
|
||||
states = _run_pid(
|
||||
_pid_policy(kp=0.0, ki=0.0, kd=1.0), [0.65]
|
||||
)
|
||||
# error=0.65-0.15=0.5, prev_error=0 → deriv=0.5 → β=floor+0.5=0.52
|
||||
assert states[0].beta == pytest.approx(0.52)
|
||||
assert states[0].prev_error == pytest.approx(0.5)
|
||||
|
||||
def test_drift_negative_gap(self):
|
||||
from soup_cli.utils.reward_hack_control import detect_reward_distribution_drift
|
||||
|
||||
# gap <= 0 (sorted halves can't reverse, but constant-ish → gap 0) → False
|
||||
assert detect_reward_distribution_drift([1, 1, 1, 1, 1, 1]) is False
|
||||
|
||||
def test_smoothing_window_lower_boundary_ok(self):
|
||||
from soup_cli.config.loader import load_config_from_string
|
||||
|
||||
cfg = load_config_from_string(
|
||||
_yaml("grpo", mitigation="kl_control", extra="reward_hack_smoothing_window: 2")
|
||||
)
|
||||
assert cfg.training.reward_hack_smoothing_window == 2
|
||||
|
||||
def test_log_cap_boundaries(self, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
from soup_cli.utils.reward_hack_control import MitigationLogWriter
|
||||
|
||||
assert MitigationLogWriter("a.jsonl", cap_mb=1).cap_bytes == 1024 * 1024
|
||||
assert MitigationLogWriter("b.jsonl", cap_mb=10_000).cap_bytes == 10_000 * 1024 * 1024
|
||||
with pytest.raises(ValueError):
|
||||
MitigationLogWriter("c.jsonl", cap_mb=10_001)
|
||||
|
||||
def test_log_concurrent_writes(self, tmp_path, monkeypatch):
|
||||
import threading
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
from soup_cli.utils.reward_hack_control import MitigationLogWriter
|
||||
|
||||
writer = MitigationLogWriter("cc.jsonl")
|
||||
|
||||
def worker(base):
|
||||
for i in range(50):
|
||||
writer.record(step=base + i, snapshot={"x": i})
|
||||
|
||||
threads = [threading.Thread(target=worker, args=(b,)) for b in (0, 1000, 2000)]
|
||||
for t in threads:
|
||||
t.start()
|
||||
for t in threads:
|
||||
t.join()
|
||||
lines = (tmp_path / "cc.jsonl").read_text().strip().splitlines()
|
||||
assert len(lines) == 150 # no interleaved/corrupt lines
|
||||
for line in lines:
|
||||
json.loads(line) # every line is a complete JSON object
|
||||
|
||||
def test_record_action_caps_at_max(self, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
from soup_cli.utils.reward_hack_control import _MAX_ACTION_HISTORY
|
||||
|
||||
cb = _kl_callback(tmp_path, None)
|
||||
for i in range(_MAX_ACTION_HISTORY + 100):
|
||||
cb._record_action(f"a{i}")
|
||||
assert len(cb._action_history) == _MAX_ACTION_HISTORY
|
||||
assert cb._action_history[-1] == f"a{_MAX_ACTION_HISTORY + 99}" # keeps the tail
|
||||
|
||||
def test_no_top_level_heavy_import_in_source(self):
|
||||
import inspect
|
||||
|
||||
from soup_cli.utils import reward_hack_control
|
||||
|
||||
src = inspect.getsource(reward_hack_control)
|
||||
for line in src.splitlines():
|
||||
stripped = line.strip()
|
||||
# module-scope imports have no indentation
|
||||
if line and not line[0].isspace():
|
||||
assert not stripped.startswith(("import torch", "from torch")), line
|
||||
assert not stripped.startswith(
|
||||
("import transformers", "from transformers")
|
||||
), line
|
||||
|
|
|
|||
Loading…
Reference in New Issue