mirror of https://github.com/razor-ai/soup.git
fix: remediate the HIGH-tier code-review findings
Training correctness (silent wrong results): - ppo: refuse a randomly-initialised reward head when only reward_fn is set (trl 0.19.1 PPO can't use a reward_fn) instead of training against noise. - edit_kernels (AlphaEdit): reject a non-finite key-norm (NaN <= 0.0 is False). - preference_combine (ORPO): length-normalise log-probs so exp() doesn't underflow and kill the odds-ratio correction (+_read_lens caller wiring). - ipo: anneal the beta schedule from ipo_tau, not the DPO default dpo_beta. - distill: mask padding + prompt tokens in the default KL term (labels!=-100). - block_expansion: freeze all-but-the-ACTUAL-added blocks (clamp over-request). - formats (KTO): map a -1 label to False (bool(-1) was silently True). Features that silently did nothing: - sft: actually install the LongLoRA S² attention override (defensive). - train --gpus re-exec: pass through --gate/--push-as/--trust-remote-code/ --tracker/--diagnose-gate/--annex-xi/--repro-receipt/--profile/energy flags. - eval gate-install hook: pass $GATE_SUITE to `soup eval against`, which now validates the locked suite as a precondition (block on missing/tampered). - deploy_measure: fold the candidate list into the cache key. Security: - sglang: loopback-only CORS (was wildcard). - fetch: lstat the ORIGINAL path (realpath resolved the symlink -> S_ISLNK never fired -> write followed the link). - ui /api/data/inspect: is_under_cwd (commonpath) instead of str.startswith. - registry lineage: unbounded cycle check (the depth-10 cap accepted a far-away cycle-closing edge). - gguf calib: read from the O_NOFOLLOW fd (no close+reopen TOCTOU window). - namespace_pin: flag ANY created_at drift (repo-recreation moves it forward). Robustness / cross-platform: - bench: re-raise typer.Exit (RuntimeError subclass) instead of masking it. - eval auto: catch typer.Exit so a benchmark failure falls through. - data split: reject negative --val/--test (negative slice inverted the split). - trace parser: read utf-8-sig so a BOM'd first record isn't dropped. - terraform plan: tolerate batch_size="auto" in the runtime estimate. - rl_checkpoint: only rank-0 writes; atomic optimizer save. Adds tests/test_code_review_high.py (29 regression tests). ruff clean; full suite 14844 passed / 120 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alpamys
parent
b587deea94
commit
58d280cb6f
|
|
@ -254,6 +254,15 @@ def register(app: typer.Typer, console: Console) -> None:
|
|||
False, "--json-only",
|
||||
help="Suppress Rich output; emit a single JSON verdict line.",
|
||||
),
|
||||
suite: str = typer.Option(
|
||||
None, "--suite",
|
||||
help=(
|
||||
"Locked eval suite (from `soup eval lock`) to validate as a gate "
|
||||
"precondition — a missing / unparseable suite BLOCKS the check "
|
||||
"(exit 1). cwd-contained. The generated pre-push hook passes "
|
||||
"$GATE_SUITE here so the locked suite is actually enforced."
|
||||
),
|
||||
),
|
||||
) -> None:
|
||||
"""Run-vs-run regression check (paired-bootstrap CI).
|
||||
|
||||
|
|
@ -265,6 +274,26 @@ def register(app: typer.Typer, console: Console) -> None:
|
|||
"""
|
||||
import json as _json
|
||||
|
||||
# The locked suite is a hard precondition when supplied: validate it
|
||||
# exists, is under cwd, and parses. Previously the hook wrote $GATE_SUITE
|
||||
# but never used it, so a deleted / tampered locked suite silently
|
||||
# passed the gate.
|
||||
if suite:
|
||||
from soup_cli.utils.eval_lock_coverage import load_locked_suite
|
||||
from soup_cli.utils.paths import is_under_cwd
|
||||
|
||||
if not is_under_cwd(suite):
|
||||
console.print(f"[red]--suite is outside cwd:[/] {escape(str(suite))}")
|
||||
raise typer.Exit(1)
|
||||
try:
|
||||
load_locked_suite(suite)
|
||||
except (FileNotFoundError, ValueError, OSError) as exc:
|
||||
console.print(
|
||||
f"[red]Locked eval suite invalid — gate blocked:[/] "
|
||||
f"{escape(str(exc))}"
|
||||
)
|
||||
raise typer.Exit(1) from exc
|
||||
|
||||
from soup_cli.experiment.tracker import ExperimentTracker
|
||||
from soup_cli.utils.eval_gate_hook import (
|
||||
GateThresholds,
|
||||
|
|
|
|||
|
|
@ -194,6 +194,11 @@ def bench(
|
|||
start_load = time.time()
|
||||
try:
|
||||
model_obj, tokenizer = _load_model(str(model_path), base, device)
|
||||
except typer.Exit:
|
||||
# typer.Exit subclasses RuntimeError, so the broad except below would
|
||||
# swallow an already-reported CLI exit and mis-print "Failed to load
|
||||
# model: 1". Let it propagate unchanged.
|
||||
raise
|
||||
except (OSError, ImportError, RuntimeError, ValueError) as exc:
|
||||
console.print(f"[red]Failed to load model:[/] {exc}")
|
||||
raise typer.Exit(1) from exc
|
||||
|
|
|
|||
|
|
@ -812,6 +812,13 @@ def split_data(
|
|||
console.print("[red]Specify at least one of --val or --test.[/]")
|
||||
raise typer.Exit(1)
|
||||
|
||||
# Reject negatives: a negative val/test slipped past the `>= total` check
|
||||
# and produced a negative slice (e.g. --val -10 sent 90 rows to val, 10 to
|
||||
# train — a silently inverted split).
|
||||
if (val is not None and val < 0) or (test is not None and test < 0):
|
||||
console.print("[red]--val and --test must be non-negative.[/]")
|
||||
raise typer.Exit(1)
|
||||
|
||||
data = load_raw_data(file_path)
|
||||
if not data:
|
||||
console.print("[red]Dataset is empty.[/]")
|
||||
|
|
|
|||
|
|
@ -516,7 +516,11 @@ def auto(
|
|||
run_id=None,
|
||||
device=None,
|
||||
)
|
||||
except SystemExit:
|
||||
except (typer.Exit, SystemExit):
|
||||
# benchmark() signals failure with typer.Exit (a RuntimeError, NOT
|
||||
# SystemExit), so the old `except SystemExit` never caught it and a
|
||||
# benchmark failure aborted the whole auto-eval instead of falling
|
||||
# through to the custom eval below.
|
||||
console.print("[yellow]Benchmark eval skipped (see above).[/]")
|
||||
|
||||
# Run custom eval if specified
|
||||
|
|
|
|||
|
|
@ -94,8 +94,11 @@ def fetch(
|
|||
# Symlink-at-target rejection (TOCTOU defence) — matches v0.33.0 #22 /
|
||||
# v0.40.2 #51 / v0.43.0 Part C policy. Apply BEFORE the existence check
|
||||
# so a symlink-with-no-real-file can never be silently overwritten.
|
||||
# lstat the ORIGINAL path — os.path.realpath() already resolved any symlink,
|
||||
# so lstat(real_target) inspected the link's TARGET and S_ISLNK was never
|
||||
# true, letting the write follow the link and clobber its target.
|
||||
try:
|
||||
link_stat = os.lstat(real_target)
|
||||
link_stat = os.lstat(target_path)
|
||||
except FileNotFoundError:
|
||||
link_stat = None
|
||||
except OSError as exc:
|
||||
|
|
|
|||
|
|
@ -717,6 +717,34 @@ def train(
|
|||
script_args.extend(
|
||||
["--reward-hack-mitigation", reward_hack_mitigation]
|
||||
)
|
||||
# Pass through the remaining run-shaping flags — these were
|
||||
# silently dropped on re-exec, so a multi-GPU run ignored the
|
||||
# eval gate, HF push, trust-remote-code, tracker, diagnose gate,
|
||||
# and the governance/energy artifacts the user asked for.
|
||||
if gate:
|
||||
script_args.extend(["--gate", gate])
|
||||
if push_as:
|
||||
script_args.extend(["--push-as", push_as])
|
||||
if hf_resume:
|
||||
script_args.append("--hf-resume")
|
||||
if trust_remote_code:
|
||||
script_args.append("--trust-remote-code")
|
||||
if tracker:
|
||||
script_args.extend(["--tracker", tracker])
|
||||
if diagnose_gate:
|
||||
script_args.extend(["--diagnose-gate", diagnose_gate])
|
||||
if annex_xi:
|
||||
script_args.extend(["--annex-xi", annex_xi])
|
||||
if repro_receipt:
|
||||
script_args.extend(["--repro-receipt", repro_receipt])
|
||||
if profile_run:
|
||||
script_args.append("--profile")
|
||||
if track_energy:
|
||||
script_args.append("--track-energy")
|
||||
if energy_country:
|
||||
script_args.extend(["--energy-country", energy_country])
|
||||
if energy_out:
|
||||
script_args.extend(["--energy-out", energy_out])
|
||||
if yes:
|
||||
script_args.append("--yes")
|
||||
argv = build_accelerate_argv(
|
||||
|
|
|
|||
|
|
@ -184,6 +184,13 @@ def _convert_kto(row: dict) -> dict:
|
|||
raise ValueError(
|
||||
f"KTO label must be true/false, got string: {raw_label!r}"
|
||||
)
|
||||
elif isinstance(raw_label, bool):
|
||||
label = raw_label
|
||||
elif isinstance(raw_label, (int, float)):
|
||||
# Both the ±1 convention (+1 desirable / -1 undesirable) and the 0/1
|
||||
# convention map "positive == desirable". `bool(-1)` is True, which
|
||||
# would silently INVERT a -1 "bad" label — flip it to False here.
|
||||
label = raw_label > 0
|
||||
else:
|
||||
label = bool(raw_label)
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -178,7 +178,10 @@ def parse_soup_serve(path: str) -> Iterator[Trace]:
|
|||
if not dir_path.is_dir():
|
||||
return
|
||||
for file_path in sorted(dir_path.glob("*.jsonl")):
|
||||
for line in file_path.read_text(encoding="utf-8").splitlines():
|
||||
# utf-8-sig strips a leading BOM (common on Windows-written JSONL);
|
||||
# plain utf-8 left the BOM on line 1, so json.loads failed and the
|
||||
# first record of every BOM'd file was silently dropped.
|
||||
for line in file_path.read_text(encoding="utf-8-sig").splitlines():
|
||||
if not line.strip():
|
||||
continue
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -534,14 +534,15 @@ class RegistryStore:
|
|||
if self.get(parent_id) is None:
|
||||
raise ValueError(f"parent entry not found: {parent_id}")
|
||||
# Cycle check: if parent already has child as an ancestor, adding this
|
||||
# edge would close a loop. Walk from parent_id upward and reject if
|
||||
# child_id is reachable.
|
||||
for anc in self.get_ancestors(parent_id):
|
||||
if anc["id"] == child_id:
|
||||
raise ValueError(
|
||||
"lineage would introduce a cycle "
|
||||
f"({child_id} -> {parent_id} -> ... -> {child_id})"
|
||||
)
|
||||
# edge would close a loop. Use an UNBOUNDED reachability walk — the old
|
||||
# code walked get_ancestors(parent_id) whose max_depth=10 cap silently
|
||||
# accepted a cycle-closing edge >10 hops away (the `soup loop watch`
|
||||
# daemon builds long chains), corrupting the DAG.
|
||||
if self._reaches_ancestor(parent_id, child_id):
|
||||
raise ValueError(
|
||||
"lineage would introduce a cycle "
|
||||
f"({child_id} -> {parent_id} -> ... -> {child_id})"
|
||||
)
|
||||
now = datetime.now().isoformat()
|
||||
conn = self._get_conn()
|
||||
conn.execute(
|
||||
|
|
@ -580,6 +581,36 @@ class RegistryStore:
|
|||
frontier = new_frontier
|
||||
return ancestors
|
||||
|
||||
def _reaches_ancestor(self, start_id: str, target_id: str) -> bool:
|
||||
"""True iff ``target_id`` is an ancestor of ``start_id`` (any depth).
|
||||
|
||||
Unbounded BFS terminated by a ``seen`` set — used for cycle detection in
|
||||
:meth:`add_lineage`, where a depth cap would let a far-away cycle-closing
|
||||
edge slip through. Queries only ``parent_id`` (no hydrate) so it stays
|
||||
cheap even on long lineage chains.
|
||||
"""
|
||||
conn = self._get_conn()
|
||||
seen: set[str] = set()
|
||||
frontier = [start_id]
|
||||
while frontier:
|
||||
placeholders = ",".join("?" * len(frontier))
|
||||
rows = conn.execute(
|
||||
f"""SELECT parent_id FROM registry_lineage
|
||||
WHERE child_id IN ({placeholders})""",
|
||||
tuple(frontier),
|
||||
).fetchall()
|
||||
new_frontier: list[str] = []
|
||||
for row in rows:
|
||||
pid = row["parent_id"]
|
||||
if pid == target_id:
|
||||
return True
|
||||
if pid in seen:
|
||||
continue
|
||||
seen.add(pid)
|
||||
new_frontier.append(pid)
|
||||
frontier = new_frontier
|
||||
return False
|
||||
|
||||
def get_descendants(self, entry_id: str, *, max_depth: int = 10) -> list[dict]:
|
||||
"""BFS walk downwards from entry_id across lineage children."""
|
||||
conn = self._get_conn()
|
||||
|
|
|
|||
|
|
@ -47,12 +47,18 @@ def _compute_distill_term(
|
|||
teacher_logits: "_torch_typ.Tensor",
|
||||
divergence: str,
|
||||
temperature: float,
|
||||
labels: "_torch_typ.Tensor | None" = None,
|
||||
attention_mask: "_torch_typ.Tensor | None" = None,
|
||||
) -> "_torch_typ.Tensor":
|
||||
"""Pure tensor kernel: divergence between student and teacher logits.
|
||||
|
||||
Both logits are ``(batch, seq, vocab)``. Temperature softens the
|
||||
distributions before the divergence is computed (Hinton). The result is
|
||||
a scalar mean over the token-level divergences.
|
||||
a scalar mean over the token-level divergences, restricted to the trained
|
||||
tokens: ``labels != -100`` when ``labels`` is given (excludes padding AND
|
||||
prompt), else ``attention_mask`` (excludes padding), else all positions.
|
||||
Averaging over padding/prompt tokens (the pre-fix behaviour) diluted the
|
||||
signal with the divergence on positions the student is not trained on.
|
||||
|
||||
Raises:
|
||||
TypeError: ``temperature`` not numeric or is bool.
|
||||
|
|
@ -74,20 +80,32 @@ def _compute_distill_term(
|
|||
temp = float(temperature)
|
||||
s = student_logits / temp
|
||||
t = teacher_logits / temp
|
||||
|
||||
def _masked_mean(per_token: "_torch_typ.Tensor") -> "_torch_typ.Tensor":
|
||||
"""Mean of a ``(batch, seq)`` per-token divergence over trained tokens."""
|
||||
if labels is not None:
|
||||
mask = labels != -100
|
||||
elif attention_mask is not None:
|
||||
mask = attention_mask.bool()
|
||||
else:
|
||||
return per_token.mean()
|
||||
mask = mask.to(per_token.dtype)
|
||||
denom = mask.sum().clamp(min=1.0)
|
||||
return (per_token * mask).sum() / denom
|
||||
|
||||
kl_div = torch.nn.functional.kl_div
|
||||
if divergence == "forward_kl":
|
||||
# KL(teacher || student). Use kl_div which expects log-probs of the
|
||||
# student and probs of the teacher.
|
||||
# KL(teacher || student): student log-probs, teacher probs. reduction=
|
||||
# "none" keeps per-token so we can mask before averaging.
|
||||
log_s = torch.log_softmax(s, dim=-1)
|
||||
p_t = torch.softmax(t, dim=-1)
|
||||
return torch.nn.functional.kl_div(
|
||||
log_s, p_t, reduction="batchmean"
|
||||
) * (temp * temp)
|
||||
per_token = kl_div(log_s, p_t, reduction="none").sum(dim=-1)
|
||||
return _masked_mean(per_token) * (temp * temp)
|
||||
if divergence == "reverse_kl":
|
||||
log_t = torch.log_softmax(t, dim=-1)
|
||||
p_s = torch.softmax(s, dim=-1)
|
||||
return torch.nn.functional.kl_div(
|
||||
log_t, p_s, reduction="batchmean"
|
||||
) * (temp * temp)
|
||||
per_token = kl_div(log_t, p_s, reduction="none").sum(dim=-1)
|
||||
return _masked_mean(per_token) * (temp * temp)
|
||||
if divergence == "js":
|
||||
# Jensen-Shannon: 0.5 (KL(p||m) + KL(q||m)), m = 0.5 (p + q).
|
||||
log_s = torch.log_softmax(s, dim=-1)
|
||||
|
|
@ -96,9 +114,9 @@ def _compute_distill_term(
|
|||
p_t = log_t.exp()
|
||||
m = 0.5 * (p_s + p_t)
|
||||
log_m = m.clamp(min=1e-12).log()
|
||||
kl_pm = torch.nn.functional.kl_div(log_m, p_s, reduction="batchmean")
|
||||
kl_qm = torch.nn.functional.kl_div(log_m, p_t, reduction="batchmean")
|
||||
return 0.5 * (kl_pm + kl_qm) * (temp * temp)
|
||||
kl_pm = kl_div(log_m, p_s, reduction="none").sum(dim=-1)
|
||||
kl_qm = kl_div(log_m, p_t, reduction="none").sum(dim=-1)
|
||||
return 0.5 * (_masked_mean(kl_pm) + _masked_mean(kl_qm)) * (temp * temp)
|
||||
raise ValueError(f"Unknown divergence {divergence!r}")
|
||||
|
||||
|
||||
|
|
@ -605,8 +623,12 @@ class DistillTrainerWrapper:
|
|||
)
|
||||
anchor = _minillm_cb.anchor_term(model)
|
||||
else:
|
||||
# Mask padding + prompt tokens so the divergence is measured
|
||||
# only over the completion tokens (parity with the ULD path).
|
||||
distill_loss = _compute_distill_term(
|
||||
student_logits, teacher_logits, divergence, temperature
|
||||
student_logits, teacher_logits, divergence, temperature,
|
||||
labels=labels,
|
||||
attention_mask=inputs.get("attention_mask"),
|
||||
)
|
||||
total = _CE_WEIGHT * ce_loss + _DISTILL_WEIGHT * distill_loss
|
||||
if anchor is not None:
|
||||
|
|
|
|||
|
|
@ -292,7 +292,10 @@ class IPOTrainerWrapper:
|
|||
|
||||
tcfg = self.config.training
|
||||
variant_cbs = build_dpo_variant_callbacks(
|
||||
beta_start=tcfg.dpo_beta,
|
||||
# IPO's regularization strength is `ipo_tau` (passed as trl's beta,
|
||||
# see setup()); the β-schedule must anneal from that, not the DPO
|
||||
# default `dpo_beta`, which would overwrite the user's τ at step 0.
|
||||
beta_start=tcfg.ipo_tau,
|
||||
beta_end=tcfg.dpo_beta_end,
|
||||
schedule=tcfg.dpo_beta_schedule,
|
||||
total_steps=0,
|
||||
|
|
|
|||
|
|
@ -262,7 +262,9 @@ class PPOTrainerWrapper:
|
|||
trainer_kwargs["ref_model"] = None
|
||||
if "reward_model" in ppo_trainer_params:
|
||||
trainer_kwargs["reward_model"] = self._get_or_create_reward_model(
|
||||
cfg, tcfg
|
||||
cfg, tcfg,
|
||||
reward_funcs_supplied=bool(reward_funcs)
|
||||
and "reward_funcs" in ppo_trainer_params,
|
||||
)
|
||||
if "value_model" in ppo_trainer_params:
|
||||
trainer_kwargs["value_model"] = self._create_value_model(cfg, tcfg)
|
||||
|
|
@ -313,13 +315,17 @@ class PPOTrainerWrapper:
|
|||
self._num_epochs = tcfg.epochs
|
||||
self._max_length = cfg.data.max_length
|
||||
|
||||
def _get_or_create_reward_model(self, cfg, tcfg):
|
||||
"""Get existing reward model or create one for trl experimental PPO API.
|
||||
def _get_or_create_reward_model(self, cfg, tcfg, *, reward_funcs_supplied=False):
|
||||
"""Get the reward model for trl's PPO API (an nn.Module, not a callable).
|
||||
|
||||
The experimental PPOTrainer requires an nn.Module reward model (not a callable).
|
||||
If we have a loaded reward model instance, use it. Otherwise, load one from
|
||||
the configured reward_model path, or create a fresh AutoModelForSequenceClassification
|
||||
from the base model.
|
||||
Uses a loaded instance, else loads the configured ``reward_model`` path.
|
||||
If neither exists, PPO has NO real reward signal: a fresh
|
||||
``AutoModelForSequenceClassification`` has a randomly-initialised
|
||||
regression head, so optimising the policy against it trains toward
|
||||
noise (silent, GPU-hours wasted). A ``reward_fn`` (callable) cannot fill
|
||||
this nn.Module slot on trl's PPO API. Fail loudly instead — unless the
|
||||
trainer is separately consuming ``reward_funcs`` (a newer trl API), in
|
||||
which case the reward model is unused and a neutral head is fine.
|
||||
"""
|
||||
if self.reward_model_instance is not None:
|
||||
return self.reward_model_instance
|
||||
|
|
@ -331,12 +337,23 @@ class PPOTrainerWrapper:
|
|||
tcfg=tcfg,
|
||||
)
|
||||
|
||||
# Fallback: create a sequence classification model from the base model
|
||||
if not reward_funcs_supplied:
|
||||
raise RuntimeError(
|
||||
"PPO requires a trained reward_model (set training.reward_model "
|
||||
"to a path or HF id). trl's PPO reward-model slot needs an "
|
||||
"nn.Module; a reward_fn cannot drive it, and creating one from "
|
||||
"the base model would train the policy against a randomly-"
|
||||
"initialised reward head. Use `task: grpo` for reward-function-"
|
||||
"based RL instead."
|
||||
)
|
||||
|
||||
# reward_funcs handles the reward signal on this trl API; the reward
|
||||
# model is a required-but-unused formality (neutral head is fine).
|
||||
from transformers import AutoModelForSequenceClassification
|
||||
|
||||
console.print(
|
||||
"[yellow]No reward_model path specified. Creating reward model "
|
||||
f"from base model: {cfg.base}[/]"
|
||||
"[dim]reward_funcs active; creating a neutral reward-model "
|
||||
f"placeholder from base model: {cfg.base}[/]"
|
||||
)
|
||||
reward_model = AutoModelForSequenceClassification.from_pretrained(
|
||||
cfg.base,
|
||||
|
|
|
|||
|
|
@ -1203,9 +1203,31 @@ class SFTTrainerWrapper:
|
|||
f"{self._output_dir!r}"
|
||||
)
|
||||
offload_save_dir = candidate
|
||||
with offload_context(
|
||||
tcfg.activation_offloading, save_dir=offload_save_dir
|
||||
):
|
||||
import contextlib
|
||||
|
||||
with contextlib.ExitStack() as _train_ctx:
|
||||
_train_ctx.enter_context(
|
||||
offload_context(tcfg.activation_offloading, save_dir=offload_save_dir)
|
||||
)
|
||||
# LongLoRA S² shifted-sparse attention (v0.49.0 schema). The override
|
||||
# monkeypatches attention.forward for the duration of training and
|
||||
# was previously never installed (use_longlora validated but shipped
|
||||
# plain attention). Enter defensively: an install failure on the
|
||||
# current transformers degrades to plain attention with a warning
|
||||
# instead of crashing the run. Arch compat is already schema-gated.
|
||||
if getattr(tcfg, "use_longlora", False) and self.config.backend == "transformers":
|
||||
from soup_cli.utils.longlora import apply_longlora_forward_override
|
||||
|
||||
try:
|
||||
_train_ctx.enter_context(
|
||||
apply_longlora_forward_override(self.model)
|
||||
)
|
||||
console.print("[green]LongLoRA S² attention override active[/]")
|
||||
except Exception as exc: # noqa: BLE001 — fall back to plain attn
|
||||
console.print(
|
||||
"[yellow]LongLoRA override could not be installed "
|
||||
f"({exc}); training with plain attention.[/]"
|
||||
)
|
||||
self.trainer.train(resume_from_checkpoint=resume_from_checkpoint)
|
||||
duration = time.time() - start
|
||||
|
||||
|
|
|
|||
|
|
@ -334,15 +334,17 @@ def create_app(host: str = "127.0.0.1", port: int = 7860):
|
|||
@app.post("/api/data/inspect", dependencies=[Depends(_verify_token)])
|
||||
def inspect_data(req: DataInspectRequest):
|
||||
from soup_cli.data.loader import load_raw_data
|
||||
from soup_cli.utils.paths import is_under_cwd
|
||||
|
||||
# Path traversal protection: resolve and check against cwd
|
||||
allowed_root = Path.cwd().resolve()
|
||||
# Path traversal protection. Use realpath + commonpath containment
|
||||
# (is_under_cwd) — the old str.startswith check let a sibling like
|
||||
# ".../project-secrets" pass as under ".../project".
|
||||
try:
|
||||
resolved = Path(req.path).resolve()
|
||||
except (ValueError, OSError):
|
||||
raise HTTPException(status_code=400, detail="Invalid path")
|
||||
|
||||
if not str(resolved).startswith(str(allowed_root)):
|
||||
if not is_under_cwd(req.path):
|
||||
raise HTTPException(
|
||||
status_code=403, detail="Access denied: path outside working directory"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -197,6 +197,11 @@ def apply_llama_pro_freeze(model: Any, num_new_blocks: int) -> int:
|
|||
if layers is None:
|
||||
return 0
|
||||
total = len(layers)
|
||||
# Defence-in-depth: never treat more than `total` layers as "new" — an
|
||||
# unclamped over-request would push `new_start` to 0 and unfreeze the whole
|
||||
# model. Callers should pass the actual appended count (see the shared
|
||||
# helper), but clamp here too so direct callers stay safe.
|
||||
n = min(n, total)
|
||||
new_start = max(0, total - n)
|
||||
# Freeze everything first.
|
||||
for param in model.parameters():
|
||||
|
|
@ -278,22 +283,28 @@ def apply_block_expansion_if_configured(
|
|||
n = getattr(tcfg, "expand_layers", None)
|
||||
if not n:
|
||||
return _count_layers(model)
|
||||
orig_total = _count_layers(model)
|
||||
new_total = expand_model_blocks(model, n)
|
||||
# expand_model_blocks CLAMPS the requested count to the available base
|
||||
# blocks, so the number actually appended can be < n. The freeze below must
|
||||
# target that real count — passing the raw request would leave original
|
||||
# layers trainable when over-requested, defeating LLaMA Pro entirely.
|
||||
added = int(new_total) - int(orig_total)
|
||||
if console is not None:
|
||||
console.print(
|
||||
f"[green]LLaMA Pro:[/] expanded to {int(new_total)} layers "
|
||||
f"(+{int(n)} zero-init blocks)"
|
||||
f"(+{added} zero-init blocks)"
|
||||
)
|
||||
freeze = getattr(tcfg, "freeze_trainable_layers", None)
|
||||
# Project policy ``is None`` over falsy — but a value of 0 means "no
|
||||
# positive freeze direction", so the canonical "train only new blocks"
|
||||
# path runs iff the user opted in with a positive ``freeze_trainable_layers``.
|
||||
if freeze is not None and freeze > 0:
|
||||
trainable = apply_llama_pro_freeze(model, n)
|
||||
if freeze is not None and freeze > 0 and added > 0:
|
||||
trainable = apply_llama_pro_freeze(model, added)
|
||||
if console is not None:
|
||||
console.print(
|
||||
f"[green]LLaMA Pro freeze:[/] {trainable:,} parameters "
|
||||
f"trainable (only the {int(n)} new blocks)"
|
||||
f"trainable (only the {added} new blocks)"
|
||||
)
|
||||
return new_total
|
||||
|
||||
|
|
|
|||
|
|
@ -64,7 +64,13 @@ class MeasureResult:
|
|||
verdict: str # "OK" | "MINOR" | "MAJOR"
|
||||
|
||||
|
||||
def compute_cache_key(*, base_sha: str, profile_name: str, tasks_sha: str) -> str:
|
||||
def compute_cache_key(
|
||||
*,
|
||||
base_sha: str,
|
||||
profile_name: str,
|
||||
tasks_sha: str,
|
||||
candidates: "Optional[Sequence[str]]" = None,
|
||||
) -> str:
|
||||
"""Build a deterministic cache key from the input tuple.
|
||||
|
||||
Callers should pass FULL SHA-256 hex strings (64 chars) for the two
|
||||
|
|
@ -74,6 +80,13 @@ def compute_cache_key(*, base_sha: str, profile_name: str, tasks_sha: str) -> st
|
|||
probability ≈ 1 in 2³² across ~4 billion cache entries) but is the
|
||||
operator-facing policy, not a property enforced here. Future callers
|
||||
that truncate further should expect collisions in proportion.
|
||||
|
||||
``candidates`` MUST be included in the key: the cached rows are one
|
||||
:class:`MeasureResult` per candidate, so a run measuring ``["4bit"]``
|
||||
must not be served for a later ``["4bit", "8bit"]`` request (which would
|
||||
silently drop the 8bit measurement). Omitting ``candidates`` reproduces
|
||||
the pre-fix key for back-compat, but the live orchestrator always passes
|
||||
the list.
|
||||
"""
|
||||
for name, value in (
|
||||
("base_sha", base_sha),
|
||||
|
|
@ -96,6 +109,17 @@ def compute_cache_key(*, base_sha: str, profile_name: str, tasks_sha: str) -> st
|
|||
hasher.update(profile_name.encode("utf-8"))
|
||||
hasher.update(b"\x1f")
|
||||
hasher.update(tasks_sha.encode("utf-8"))
|
||||
if candidates is not None:
|
||||
if isinstance(candidates, (str, bytes)) or not isinstance(candidates, Sequence):
|
||||
raise TypeError("candidates must be a sequence of strings")
|
||||
hasher.update(b"\x1f")
|
||||
for cand in candidates:
|
||||
if not isinstance(cand, str):
|
||||
raise TypeError(
|
||||
f"each candidate must be str, got {type(cand).__name__}"
|
||||
)
|
||||
hasher.update(cand.encode("utf-8"))
|
||||
hasher.update(b"\x1e")
|
||||
return hasher.hexdigest()[:32]
|
||||
|
||||
|
||||
|
|
@ -560,6 +584,7 @@ def run_measure(
|
|||
tasks_sha = sha_of_file(tasks_file)
|
||||
key = compute_cache_key(
|
||||
base_sha=base_sha, profile_name=profile_name, tasks_sha=tasks_sha,
|
||||
candidates=list(candidates),
|
||||
)
|
||||
|
||||
cache = load_cache(cache_path)
|
||||
|
|
|
|||
|
|
@ -520,7 +520,10 @@ def apply_alphaedit_edit(
|
|||
)
|
||||
key_t = key.to(down.weight.dtype)
|
||||
denom = float(torch.dot(key_t, key_t).item())
|
||||
if denom <= 0.0:
|
||||
# NaN <= 0.0 is False, so a non-finite denom would slip past a bare
|
||||
# `denom <= 0.0` and corrupt the weights in place (matches the sibling
|
||||
# `_rank1_update` guard). Reject non-finite AND non-positive.
|
||||
if not math.isfinite(denom) or denom <= 0.0:
|
||||
raise ValueError("key vector has zero norm; cannot apply AlphaEdit update")
|
||||
# Logical [out, in] ROME update (delta is OUTPUT-space, key is INPUT-space).
|
||||
rome_update = torch.outer(delta.to(down.weight.dtype), key_t) / denom
|
||||
|
|
|
|||
|
|
@ -296,7 +296,8 @@ if [ -z "$CANDIDATE_RUN_ID" ]; then
|
|||
exit 0
|
||||
fi
|
||||
|
||||
soup eval against "$BASELINE_RUN_ID" --candidate "$CANDIDATE_RUN_ID" --json-only \\
|
||||
soup eval against "$BASELINE_RUN_ID" --candidate "$CANDIDATE_RUN_ID" \\
|
||||
--suite "$GATE_SUITE" --json-only \\
|
||||
|| {{
|
||||
echo "[soup] pre-push gate blocked: regression vs $BASELINE_RUN_ID" >&2
|
||||
exit 1
|
||||
|
|
|
|||
|
|
@ -393,7 +393,14 @@ def _prepare_calibration_text(calibration_data: str, staged_dir: Path) -> Path:
|
|||
"calibration_data became a symlink during the export "
|
||||
"(TOCTOU defence): refusing to open."
|
||||
) from exc
|
||||
os.close(fd)
|
||||
# Read from THIS fd. The previous code closed it and re-opened `src`
|
||||
# with a plain open(), re-introducing the symlink-swap window between
|
||||
# the O_NOFOLLOW check and the actual read.
|
||||
src_fh = os.fdopen(fd, encoding="utf-8")
|
||||
else:
|
||||
# No O_NOFOLLOW (Windows): enforce_under_cwd_and_no_symlink's
|
||||
# dispatch-time check is the portable backstop.
|
||||
src_fh = open(src, encoding="utf-8")
|
||||
out = staged_dir / "calib.txt"
|
||||
line_count = 0
|
||||
total_bytes = 0
|
||||
|
|
@ -405,9 +412,7 @@ def _prepare_calibration_text(calibration_data: str, staged_dir: Path) -> Path:
|
|||
sanitised = text.replace("\x00", "").replace("\n", " ")
|
||||
return sanitised[:max_per_line]
|
||||
|
||||
with open(src, encoding="utf-8") as fh_in, open(
|
||||
out, "w", encoding="utf-8"
|
||||
) as fh_out:
|
||||
with src_fh as fh_in, open(out, "w", encoding="utf-8") as fh_out:
|
||||
for raw_line in fh_in:
|
||||
line = raw_line.strip()
|
||||
if not line:
|
||||
|
|
|
|||
|
|
@ -297,6 +297,22 @@ def _is_backward(current: str, recorded: str) -> bool:
|
|||
return current < recorded
|
||||
|
||||
|
||||
def _created_at_differs(current: str, recorded: str) -> bool:
|
||||
"""Return True when ``current`` is a DIFFERENT moment than ``recorded``.
|
||||
|
||||
A pinned repo's ``created_at`` is immutable, so ANY drift — forward OR
|
||||
backward — means the namespace was re-created (the AI-Jacking / repo-
|
||||
recreation attack this pin targets). The previous gate only flagged a
|
||||
*backward* jump, but a recreated repo gets a *later* timestamp and sailed
|
||||
straight through. Offset-aware equality via ``fromisoformat``; falls back
|
||||
to string inequality when either value fails to parse (legacy rows).
|
||||
"""
|
||||
try:
|
||||
return datetime.fromisoformat(current) != datetime.fromisoformat(recorded)
|
||||
except ValueError:
|
||||
return current != recorded
|
||||
|
||||
|
||||
def record_repo_first_seen(
|
||||
store: NamespacePinStore,
|
||||
*,
|
||||
|
|
@ -386,13 +402,13 @@ def verify_namespace(
|
|||
)
|
||||
|
||||
author_match = existing.author == current_author
|
||||
# Compare timestamps numerically rather than lexicographically — the
|
||||
# latter works for fixed-format UTC strings but is fragile if a non-Z
|
||||
# offset arrives from HF Hub. Fall back to a lexicographic compare
|
||||
# only when parsing fails (legacy data).
|
||||
created_at_backward = _is_backward(current_created_at, existing.created_at)
|
||||
# Flag ANY created_at drift (forward or backward): a pinned repo's creation
|
||||
# time is immutable, so a changed timestamp means the namespace was
|
||||
# re-created. The old `_is_backward`-only check missed the repo-recreation
|
||||
# attack (which produces a LATER timestamp).
|
||||
created_at_changed = _created_at_differs(current_created_at, existing.created_at)
|
||||
|
||||
if author_match and not created_at_backward:
|
||||
if author_match and not created_at_changed:
|
||||
return NamespaceVerifyReport(
|
||||
repo_id=repo_id,
|
||||
ok=True,
|
||||
|
|
@ -400,8 +416,8 @@ def verify_namespace(
|
|||
recorded=existing,
|
||||
)
|
||||
|
||||
# Mismatch — either author changed or created_at jumped backward.
|
||||
# Author comparison is case-insensitive: HF Hub authors are
|
||||
# Mismatch — either author changed or created_at drifted (in either
|
||||
# direction). Author comparison is case-insensitive: HF Hub authors are
|
||||
# case-insensitive, and a user typing the wrong case should not
|
||||
# fail the gate (security-review LOW fix).
|
||||
if override is not None and override.lower() == current_author.lower():
|
||||
|
|
|
|||
|
|
@ -128,19 +128,41 @@ def compute_simpo_term(
|
|||
return -_logsigmoid(logits).mean()
|
||||
|
||||
|
||||
def compute_orpo_term(pol_chosen, pol_rejected, alpha: float):
|
||||
def compute_orpo_term(
|
||||
pol_chosen,
|
||||
pol_rejected,
|
||||
alpha: float,
|
||||
chosen_lens=None,
|
||||
rejected_lens=None,
|
||||
):
|
||||
"""Reference-free odds-ratio preference loss (ORPO).
|
||||
|
||||
Uses the response-log-prob formulation ``-log σ(log(p_w) - log(p_l) +
|
||||
log(1-p_l) - log(1-p_w))`` scaled by ``alpha``. Approximates the full
|
||||
ORPO loss without the SFT term — caller is expected to mix in SFT via
|
||||
its own weight if desired.
|
||||
|
||||
``pol_chosen`` / ``pol_rejected`` are *summed* sequence log-probs. On a real
|
||||
sequence the summed log-prob is ≈ −45, so ``exp()`` underflows to 0 and the
|
||||
``log(1 − p)`` odds-ratio correction collapses to 0 — degenerating the loss
|
||||
to a plain log-prob difference. TRL avoids this with ``average_log_prob``;
|
||||
pass ``chosen_lens`` / ``rejected_lens`` (response token counts) to
|
||||
length-normalise the log-probs here so ``exp()`` yields a real per-token
|
||||
probability and the odds-ratio term is meaningful.
|
||||
"""
|
||||
import torch
|
||||
|
||||
log_odds_chosen = pol_chosen - torch.log1p(-torch.exp(pol_chosen).clamp(max=1 - 1e-7))
|
||||
log_odds_rejected = pol_rejected - torch.log1p(
|
||||
-torch.exp(pol_rejected).clamp(max=1 - 1e-7)
|
||||
if chosen_lens is not None and rejected_lens is not None:
|
||||
chosen_lens = torch.clamp(chosen_lens.float(), min=1.0)
|
||||
rejected_lens = torch.clamp(rejected_lens.float(), min=1.0)
|
||||
lp_chosen = pol_chosen / chosen_lens
|
||||
lp_rejected = pol_rejected / rejected_lens
|
||||
else:
|
||||
lp_chosen = pol_chosen
|
||||
lp_rejected = pol_rejected
|
||||
log_odds_chosen = lp_chosen - torch.log1p(-torch.exp(lp_chosen).clamp(max=1 - 1e-7))
|
||||
log_odds_rejected = lp_rejected - torch.log1p(
|
||||
-torch.exp(lp_rejected).clamp(max=1 - 1e-7)
|
||||
)
|
||||
sigm_term = _logsigmoid(log_odds_chosen - log_odds_rejected)
|
||||
return (-alpha * sigm_term).mean()
|
||||
|
|
@ -271,8 +293,13 @@ def attach_weighted_preference_combine(trainer: object, weights: Mapping[str, fl
|
|||
alpha = (
|
||||
float(alpha_attr) if alpha_attr is not None else 1.0
|
||||
)
|
||||
# Length-normalise when the batch carries response
|
||||
# lengths/labels — otherwise summed log-probs underflow
|
||||
# exp() and the odds-ratio correction degenerates.
|
||||
terms["orpo"] = compute_orpo_term(
|
||||
pol_chosen, pol_rejected, alpha
|
||||
pol_chosen, pol_rejected, alpha,
|
||||
chosen_lens=_read_lens(inputs, "chosen"),
|
||||
rejected_lens=_read_lens(inputs, "rejected"),
|
||||
)
|
||||
elif name == "bco":
|
||||
# BCO is data-format-incompatible — already rejected
|
||||
|
|
@ -311,6 +338,30 @@ def _read_logps(obj, name: str):
|
|||
return getattr(obj, name, None)
|
||||
|
||||
|
||||
def _read_lens(obj, prefix: str):
|
||||
"""Best-effort per-side response-length tensor for length-normalised terms.
|
||||
|
||||
Tries an explicit ``<prefix>_lens`` key first, then derives the count from
|
||||
``<prefix>_labels`` (non-``-100`` tokens = the loss-masked response). Returns
|
||||
None when neither is present, in which case the caller keeps the (degenerate)
|
||||
summed-log-prob behaviour rather than crashing.
|
||||
"""
|
||||
lens = _read_logps(obj, f"{prefix}_lens")
|
||||
if lens is not None:
|
||||
return lens
|
||||
labels = _read_logps(obj, f"{prefix}_labels")
|
||||
if labels is None:
|
||||
return None
|
||||
try:
|
||||
import torch
|
||||
|
||||
if not isinstance(labels, torch.Tensor):
|
||||
return None
|
||||
return (labels != -100).sum(dim=-1)
|
||||
except Exception: # noqa: BLE001 — best-effort; degrade to no normalisation
|
||||
return None
|
||||
|
||||
|
||||
def _pick_primary(weights: Mapping[str, float]) -> str:
|
||||
"""Return the loss with the highest weight (deterministic on ties)."""
|
||||
return max(sorted(weights), key=lambda k: weights[k])
|
||||
|
|
|
|||
|
|
@ -217,6 +217,33 @@ def _step_number(name: str) -> int:
|
|||
return -1
|
||||
|
||||
|
||||
def _is_main_process() -> bool:
|
||||
"""True on the rank-0 / non-distributed process.
|
||||
|
||||
Only the main process may write RL checkpoints — concurrent writes from
|
||||
every rank to the same directory race and corrupt the (non-atomic) saves.
|
||||
Prefers ``torch.distributed`` when initialised; otherwise reads the
|
||||
launcher env vars (accelerate / torchrun set ``RANK`` / ``LOCAL_RANK``).
|
||||
"""
|
||||
import os
|
||||
|
||||
try:
|
||||
import torch.distributed as dist
|
||||
|
||||
if dist.is_available() and dist.is_initialized():
|
||||
return dist.get_rank() == 0
|
||||
except Exception: # noqa: BLE001 — torch missing / not initialised
|
||||
pass
|
||||
for var in ("RANK", "LOCAL_RANK"):
|
||||
raw = os.environ.get(var)
|
||||
if raw is not None:
|
||||
try:
|
||||
return int(raw) == 0
|
||||
except ValueError:
|
||||
return True
|
||||
return True
|
||||
|
||||
|
||||
class RLCheckpointCallback(_TrainerCallbackBase): # type: ignore[misc, valid-type]
|
||||
"""Live HF TrainerCallback for mid-epoch RL checkpoints (v0.71.11 #238).
|
||||
|
||||
|
|
@ -281,6 +308,15 @@ class RLCheckpointCallback(_TrainerCallbackBase): # type: ignore[misc, valid-ty
|
|||
|
||||
root = self._ckpt_root()
|
||||
ckpt_dir = os.path.join(root, f"step-{int(step)}")
|
||||
|
||||
# Multi-process guard: only the main (rank-0) process writes. Every rank
|
||||
# would otherwise race on the SAME paths — the non-atomic
|
||||
# torch.save(optimizer.pt) + save_pretrained can interleave and corrupt
|
||||
# the checkpoint, after which the rollback ladder silently restores
|
||||
# nothing. Non-main ranks return the (rank-0-written) path unchanged.
|
||||
if not _is_main_process():
|
||||
return ckpt_dir
|
||||
|
||||
os.makedirs(ckpt_dir, exist_ok=True)
|
||||
|
||||
if model is not None and hasattr(model, "save_pretrained"):
|
||||
|
|
@ -297,7 +333,11 @@ class RLCheckpointCallback(_TrainerCallbackBase): # type: ignore[misc, valid-ty
|
|||
try:
|
||||
import torch
|
||||
|
||||
torch.save(optimizer.state_dict(), opt_out)
|
||||
# Atomic write: torch.save straight to opt_out could leave a
|
||||
# truncated file if interrupted. Stage to .tmp then rename.
|
||||
tmp_out = opt_out + ".tmp"
|
||||
torch.save(optimizer.state_dict(), tmp_out)
|
||||
os.replace(tmp_out, opt_out)
|
||||
has_optimizer = True
|
||||
except Exception: # noqa: BLE001 — best-effort, manifest reflects it
|
||||
has_optimizer = False
|
||||
|
|
|
|||
|
|
@ -112,10 +112,13 @@ def create_sglang_app(
|
|||
|
||||
app = FastAPI(title="Soup Inference Server (SGLang)", version="1.0.0")
|
||||
|
||||
# Loopback-only CORS (parity with the transformers / vLLM servers). The
|
||||
# wildcard the loopback fix never reached let any web page read this local
|
||||
# server's responses.
|
||||
app.add_middleware(
|
||||
CORSMiddleware,
|
||||
allow_origins=["*"],
|
||||
allow_methods=["*"],
|
||||
allow_origin_regex=r"^https?://(localhost|127\.0\.0\.1)(:\d+)?$",
|
||||
allow_methods=["GET", "POST", "OPTIONS"],
|
||||
allow_headers=["*"],
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -219,10 +219,25 @@ _DEFAULT_SPOT_PRICE = 0.30 # ~$/hr for a 24 GB consumer GPU
|
|||
_DEFAULT_PEAK_VRAM = 8.0
|
||||
|
||||
|
||||
def _to_float(value: Any, default: float) -> float:
|
||||
"""Coerce a config number to float, tolerating ``batch_size: "auto"``.
|
||||
|
||||
``soup train`` accepts ``batch_size: "auto"`` (int-or-"auto" per schema), so
|
||||
the plan estimator must not crash on ``float("auto")`` — fall back to the
|
||||
baseline for the heuristic instead.
|
||||
"""
|
||||
try:
|
||||
return float(value)
|
||||
except (TypeError, ValueError):
|
||||
return default
|
||||
|
||||
|
||||
def _estimate_runtime_minutes(config: Mapping[str, Any]) -> float:
|
||||
training = config.get("training", {}) if isinstance(config, Mapping) else {}
|
||||
epochs = float(training.get("epochs", 1) if isinstance(training, Mapping) else 1)
|
||||
batch_size = float(training.get("batch_size", 4) if isinstance(training, Mapping) else 4)
|
||||
if not isinstance(training, Mapping):
|
||||
training = {}
|
||||
epochs = _to_float(training.get("epochs", 1), 1.0)
|
||||
batch_size = _to_float(training.get("batch_size", 4), 4.0)
|
||||
# Soft heuristic: 5 minutes per epoch at batch_size=4 baseline.
|
||||
base = 5.0 * epochs
|
||||
if batch_size > 0:
|
||||
|
|
|
|||
|
|
@ -0,0 +1,413 @@
|
|||
"""Regression tests for the HIGH findings in CODE_REVIEW.md.
|
||||
|
||||
Grouped by the review's own subsections. Each asserts the specific broken
|
||||
behavior is now correct.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import types
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
import soup_cli
|
||||
|
||||
# ═══════════════════════ HIGH — training correctness ═══════════════════════
|
||||
|
||||
|
||||
def test_ppo_refuses_silent_random_reward_head():
|
||||
"""PPO with only a reward_fn (no reward_model) must fail loudly, not build
|
||||
a randomly-initialised reward head and train the policy against noise."""
|
||||
from soup_cli.trainer.ppo import PPOTrainerWrapper
|
||||
|
||||
wrapper = object.__new__(PPOTrainerWrapper)
|
||||
wrapper.reward_model_instance = None
|
||||
wrapper.device = "cpu"
|
||||
wrapper.trust_remote_code = False
|
||||
wrapper._trust_remote_code = False
|
||||
cfg = types.SimpleNamespace(base="hf-internal-testing/tiny-random-gpt2")
|
||||
tcfg = types.SimpleNamespace(reward_model=None)
|
||||
with pytest.raises(RuntimeError, match="reward_model"):
|
||||
wrapper._get_or_create_reward_model(cfg, tcfg)
|
||||
|
||||
|
||||
def test_alphaedit_rejects_nonfinite_denominator(monkeypatch):
|
||||
"""A NaN key-norm must be rejected (NaN <= 0.0 is False, so the old bare
|
||||
`denom <= 0.0` guard let it corrupt weights in place)."""
|
||||
torch = pytest.importorskip("torch")
|
||||
import soup_cli.utils.edit_kernels as ek
|
||||
|
||||
class _Down:
|
||||
def __init__(self):
|
||||
self.weight = torch.zeros(2, 2)
|
||||
|
||||
monkeypatch.setattr(ek, "_locate_decoder_layers", lambda m: ["layer0"])
|
||||
monkeypatch.setattr(ek, "_down_proj", lambda layers, layer: _Down())
|
||||
monkeypatch.setattr(
|
||||
ek, "_capture_key", lambda *a, **k: torch.tensor([float("nan"), float("nan")])
|
||||
)
|
||||
monkeypatch.setattr(ek, "_optimise_residual", lambda *a, **k: torch.tensor([1.0, 1.0]))
|
||||
|
||||
with pytest.raises(ValueError, match="zero norm"):
|
||||
ek.apply_alphaedit_edit(
|
||||
object(), object(), subject="s", target="t", layer=0, device="cpu"
|
||||
)
|
||||
|
||||
|
||||
def test_orpo_length_normalization_restores_odds_ratio():
|
||||
"""With summed sequence log-probs exp() underflows and the odds-ratio
|
||||
correction collapses; passing lengths length-normalises and restores it."""
|
||||
torch = pytest.importorskip("torch")
|
||||
from soup_cli.utils.preference_combine import compute_orpo_term
|
||||
|
||||
pol_chosen = torch.tensor([-40.0])
|
||||
pol_rejected = torch.tensor([-50.0])
|
||||
lens = torch.tensor([20.0])
|
||||
|
||||
degenerate = float(compute_orpo_term(pol_chosen, pol_rejected, 1.0))
|
||||
normalized = float(
|
||||
compute_orpo_term(
|
||||
pol_chosen, pol_rejected, 1.0, chosen_lens=lens, rejected_lens=lens
|
||||
)
|
||||
)
|
||||
import math as _math
|
||||
|
||||
assert _math.isfinite(normalized)
|
||||
# The summed-logp version underflows to ~0 loss; the length-normalised
|
||||
# version has a meaningful (larger) odds-ratio loss.
|
||||
assert normalized - degenerate > 1e-2
|
||||
|
||||
|
||||
def test_ipo_beta_schedule_uses_ipo_tau_not_dpo_beta():
|
||||
src = (Path(soup_cli.__file__).parent / "trainer" / "ipo.py").read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
assert "beta_start=tcfg.ipo_tau" in src
|
||||
assert "beta_start=tcfg.dpo_beta" not in src
|
||||
|
||||
|
||||
def test_beta_schedule_callback_sets_beta_from_start():
|
||||
"""At step 0 the schedule must apply beta_start — so passing ipo_tau (not
|
||||
the DPO default) is what keeps the user's τ intact."""
|
||||
from soup_cli.utils.dpo_variants import BetaScheduleCallback
|
||||
|
||||
trainer = types.SimpleNamespace(beta=0.5) # e.g. the user's ipo_tau
|
||||
cb = BetaScheduleCallback(beta_start=0.5, beta_end=0.1, total_steps=10, schedule="linear")
|
||||
cb.attach(trainer)
|
||||
state = types.SimpleNamespace(max_steps=10, global_step=0)
|
||||
cb.on_train_begin(None, state, None)
|
||||
cb.on_step_begin(None, state, None)
|
||||
assert abs(trainer.beta - 0.5) < 1e-9
|
||||
|
||||
|
||||
def test_distill_term_masks_padding_and_prompt():
|
||||
"""The default distillation divergence must be measured only over trained
|
||||
tokens (labels != -100), not padding/prompt."""
|
||||
torch = pytest.importorskip("torch")
|
||||
from soup_cli.trainer.distill import _compute_distill_term
|
||||
|
||||
student = torch.zeros(1, 3, 4)
|
||||
teacher = torch.zeros(1, 3, 4)
|
||||
# Positions 0,1 diverge wildly; position 2 is identical (KL 0).
|
||||
student[0, 0, 0] = 10.0
|
||||
teacher[0, 0, 3] = 10.0
|
||||
student[0, 1, 1] = 10.0
|
||||
teacher[0, 1, 2] = 10.0
|
||||
labels = torch.tensor([[-100, -100, 5]]) # only the last token is trained
|
||||
|
||||
masked = float(_compute_distill_term(student, teacher, "forward_kl", 1.0, labels=labels))
|
||||
unmasked = float(_compute_distill_term(student, teacher, "forward_kl", 1.0))
|
||||
assert masked < 1e-5, masked # only the identical (trained) position counts
|
||||
assert unmasked > 0.1, unmasked # divergent prompt positions dominate unmasked
|
||||
|
||||
|
||||
def test_kto_negative_one_label_is_undesirable():
|
||||
"""`bool(-1)` is True; a -1 label in the ±1 convention must map to False."""
|
||||
from soup_cli.data.formats import _convert_kto
|
||||
|
||||
base = {"prompt": "p", "completion": "c"}
|
||||
assert _convert_kto({**base, "label": -1})["label"] is False
|
||||
assert _convert_kto({**base, "label": 1})["label"] is True
|
||||
assert _convert_kto({**base, "label": 0})["label"] is False
|
||||
assert _convert_kto({**base, "label": True})["label"] is True
|
||||
assert _convert_kto({**base, "label": False})["label"] is False
|
||||
assert _convert_kto({**base, "label": "false"})["label"] is False
|
||||
|
||||
|
||||
def test_apply_llama_pro_freeze_freezes_all_but_new_blocks():
|
||||
pytest.importorskip("torch")
|
||||
import torch.nn as nn
|
||||
|
||||
from soup_cli.utils.block_expansion import apply_llama_pro_freeze
|
||||
|
||||
class _M(nn.Module):
|
||||
def __init__(self):
|
||||
super().__init__()
|
||||
self.model = nn.Module()
|
||||
self.model.layers = nn.ModuleList([nn.Linear(2, 2) for _ in range(4)])
|
||||
|
||||
model = _M()
|
||||
apply_llama_pro_freeze(model, 2) # keep only the last 2 blocks trainable
|
||||
trainable = [
|
||||
all(p.requires_grad for p in layer.parameters()) for layer in model.model.layers
|
||||
]
|
||||
assert trainable == [False, False, True, True]
|
||||
|
||||
|
||||
def test_block_expansion_freeze_uses_actual_added_not_requested(monkeypatch):
|
||||
"""When expand_layers over-requests, the freeze must target the ACTUAL
|
||||
appended count (clamped), else original layers stay trainable."""
|
||||
import soup_cli.utils.block_expansion as be
|
||||
|
||||
class _Inner:
|
||||
def __init__(self, n):
|
||||
self.layers = list(range(n))
|
||||
|
||||
class _Model:
|
||||
def __init__(self, n):
|
||||
self.model = _Inner(n)
|
||||
|
||||
model = _Model(4) # 4 base layers
|
||||
|
||||
def fake_expand(m, n):
|
||||
added = min(n, 4) # expand_model_blocks clamps to available base blocks
|
||||
m.model.layers = list(range(4 + added))
|
||||
return 4 + added
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_freeze(m, count):
|
||||
captured["count"] = count
|
||||
return 123
|
||||
|
||||
monkeypatch.setattr(be, "expand_model_blocks", fake_expand)
|
||||
monkeypatch.setattr(be, "apply_llama_pro_freeze", fake_freeze)
|
||||
|
||||
tcfg = types.SimpleNamespace(expand_layers=10, freeze_trainable_layers=1)
|
||||
be.apply_block_expansion_if_configured(model, tcfg)
|
||||
assert captured["count"] == 4 # actual added, NOT the over-requested 10
|
||||
|
||||
|
||||
def _src(rel: str) -> str:
|
||||
return (Path(soup_cli.__file__).parent / rel).read_text(encoding="utf-8")
|
||||
|
||||
|
||||
# ═══════════════════════ HIGH — features that silently do nothing ═══════════
|
||||
|
||||
|
||||
def test_longlora_wired_into_sft_train():
|
||||
src = _src("trainer/sft.py")
|
||||
assert "apply_longlora_forward_override" in src, "LongLoRA override never installed"
|
||||
|
||||
|
||||
def test_gpus_reexec_passes_run_shaping_flags():
|
||||
src = _src("commands/train.py")
|
||||
for flag in ('"--gate"', '"--push-as"', '"--trust-remote-code"', '"--tracker"',
|
||||
'"--diagnose-gate"', '"--annex-xi"', '"--repro-receipt"'):
|
||||
assert "script_args" in src and flag in src, f"re-exec drops {flag}"
|
||||
|
||||
|
||||
def test_pre_push_hook_enforces_gate_suite():
|
||||
from soup_cli.utils.eval_gate_hook import render_pre_push_hook
|
||||
|
||||
hook = render_pre_push_hook(baseline_run_id="run-abc123", suite_path="evals/locked.json")
|
||||
assert '--suite "$GATE_SUITE"' in hook
|
||||
|
||||
|
||||
def test_eval_against_blocks_on_unloadable_locked_suite(tmp_path, monkeypatch):
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from soup_cli.commands.eval import app
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
result = CliRunner().invoke(
|
||||
app,
|
||||
["against", "base-run", "--candidate", "cand-run", "--suite", "missing.json"],
|
||||
)
|
||||
assert result.exit_code == 1, (result.output, repr(result.exception))
|
||||
|
||||
|
||||
def test_deploy_measure_cache_key_includes_candidates():
|
||||
from soup_cli.utils.deploy_measure import compute_cache_key
|
||||
|
||||
common = dict(base_sha="a" * 16, profile_name="p", tasks_sha="b" * 64)
|
||||
k_one = compute_cache_key(**common, candidates=["4bit"])
|
||||
k_two = compute_cache_key(**common, candidates=["4bit", "8bit"])
|
||||
k_none = compute_cache_key(**common) # back-compat: same as pre-fix key
|
||||
assert k_one != k_two, "different candidate sets must not share a cache key"
|
||||
assert k_none not in (k_one, k_two)
|
||||
|
||||
|
||||
# ═══════════════════════ HIGH — security ═══════════════════════
|
||||
|
||||
|
||||
def test_sglang_cors_is_loopback_only():
|
||||
src = _src("utils/sglang.py")
|
||||
assert 'allow_origins=["*"]' not in src
|
||||
assert "localhost|127" in src # loopback-only regex
|
||||
|
||||
|
||||
def test_fetch_lstats_original_path_not_realpath():
|
||||
src = _src("commands/fetch.py")
|
||||
assert "os.lstat(target_path)" in src
|
||||
assert "os.lstat(real_target)" not in src
|
||||
|
||||
|
||||
def test_ui_inspect_uses_commonpath_containment():
|
||||
src = _src("ui/app.py")
|
||||
assert "is_under_cwd(req.path)" in src
|
||||
|
||||
|
||||
def test_is_under_cwd_rejects_sibling_prefix(tmp_path, monkeypatch):
|
||||
from soup_cli.utils.paths import is_under_cwd
|
||||
|
||||
proj = tmp_path / "project"
|
||||
proj.mkdir()
|
||||
(tmp_path / "project-secrets").mkdir()
|
||||
monkeypatch.chdir(proj)
|
||||
assert is_under_cwd(str(proj / "d.jsonl")) is True
|
||||
# The sibling shares the "project" prefix but is NOT under cwd.
|
||||
assert is_under_cwd(str(tmp_path / "project-secrets" / "d.jsonl")) is False
|
||||
|
||||
|
||||
def test_registry_lineage_cycle_detected_beyond_depth_10(tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("SOUP_REGISTRY_DB_PATH", str(tmp_path / "reg.db"))
|
||||
from soup_cli.registry.store import RegistryStore
|
||||
|
||||
with RegistryStore() as store:
|
||||
ids = [
|
||||
store.push(
|
||||
name=f"m{i}", tag="v1", base_model="b", task="sft",
|
||||
run_id=None, config={},
|
||||
)
|
||||
for i in range(15)
|
||||
]
|
||||
# Chain child->parent 14 deep: ids[0] -> ids[1] -> ... -> ids[14].
|
||||
for i in range(14):
|
||||
store.add_lineage(
|
||||
child_id=ids[i], parent_id=ids[i + 1], relation="forked_from"
|
||||
)
|
||||
# ids[14] is an ancestor of ids[0] 14 hops away — past the old depth-10
|
||||
# cap. The unbounded walk must still catch the cycle.
|
||||
with pytest.raises(ValueError, match="cycle"):
|
||||
store.add_lineage(
|
||||
child_id=ids[14], parent_id=ids[0], relation="forked_from"
|
||||
)
|
||||
|
||||
|
||||
def test_gguf_calib_reads_from_nofollow_fd_no_reopen():
|
||||
src = _src("utils/gguf_quant.py")
|
||||
assert "os.fdopen(fd" in src
|
||||
assert "os.close(fd)" not in src # the close+reopen TOCTOU window is gone
|
||||
|
||||
|
||||
def test_namespace_created_at_differs_both_directions():
|
||||
from soup_cli.utils.namespace_pin import _created_at_differs
|
||||
|
||||
assert _created_at_differs("2026-01-02T00:00:00", "2026-01-01T00:00:00") is True
|
||||
assert _created_at_differs("2026-01-01T00:00:00", "2026-01-02T00:00:00") is True
|
||||
assert _created_at_differs("2026-01-01T00:00:00", "2026-01-01T00:00:00") is False
|
||||
|
||||
|
||||
def test_namespace_pin_flags_forward_created_at_drift(tmp_path):
|
||||
from soup_cli.utils.namespace_pin import NamespacePinStore, verify_namespace
|
||||
|
||||
store = NamespacePinStore(str(tmp_path / "pins.db"))
|
||||
first = verify_namespace(
|
||||
store, repo_id="org/model", current_author="alice",
|
||||
current_created_at="2026-01-01T00:00:00",
|
||||
)
|
||||
assert first.ok is True # trust on first use
|
||||
# Same author, LATER created_at (repo re-created / AI-Jacking) must be flagged.
|
||||
recreated = verify_namespace(
|
||||
store, repo_id="org/model", current_author="alice",
|
||||
current_created_at="2026-06-01T00:00:00",
|
||||
)
|
||||
assert recreated.ok is False
|
||||
|
||||
|
||||
# ═══════════════════════ HIGH — robustness / cross-platform ═══════════════════
|
||||
|
||||
|
||||
def test_typer_exit_is_a_runtimeerror():
|
||||
import typer
|
||||
|
||||
# This is WHY the bench.py `except RuntimeError` swallowed it and the
|
||||
# eval.py `except SystemExit` missed it.
|
||||
assert issubclass(typer.Exit, RuntimeError)
|
||||
assert not issubclass(typer.Exit, SystemExit)
|
||||
|
||||
|
||||
def test_bench_reraises_typer_exit():
|
||||
assert "except typer.Exit:" in _src("commands/bench.py")
|
||||
|
||||
|
||||
def test_eval_auto_catches_typer_exit():
|
||||
assert "except (typer.Exit, SystemExit):" in _src("commands/eval.py")
|
||||
|
||||
|
||||
def test_data_split_rejects_negative_val(tmp_path, monkeypatch):
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from soup_cli.commands.data import app
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
(tmp_path / "d.jsonl").write_text(
|
||||
'{"messages": [{"role": "user", "content": "x"}]}\n' * 20, encoding="utf-8"
|
||||
)
|
||||
result = CliRunner().invoke(
|
||||
app, ["split", "d.jsonl", "--val", "-10", "--absolute"]
|
||||
)
|
||||
assert result.exit_code == 1, (result.output, repr(result.exception))
|
||||
assert "non-negative" in result.output
|
||||
|
||||
|
||||
def test_trace_parser_reads_bom_first_record(tmp_path):
|
||||
from soup_cli.data.traces.parsers import parse_soup_serve
|
||||
|
||||
trace_dir = tmp_path / "traces"
|
||||
trace_dir.mkdir()
|
||||
# utf-8-sig writes a BOM before line 1.
|
||||
(trace_dir / "t.jsonl").write_text(
|
||||
'{"prompt": "p1", "response": "r1"}\n{"prompt": "p2", "response": "r2"}\n',
|
||||
encoding="utf-8-sig",
|
||||
)
|
||||
traces = list(parse_soup_serve(str(trace_dir)))
|
||||
assert len(traces) == 2, "BOM dropped the first record"
|
||||
assert traces[0].prompt == "p1"
|
||||
|
||||
|
||||
def test_plan_estimate_handles_batch_size_auto():
|
||||
from soup_cli.utils.terraform_plan import _estimate_runtime_minutes
|
||||
|
||||
minutes = _estimate_runtime_minutes(
|
||||
{"training": {"epochs": 2, "batch_size": "auto"}}
|
||||
)
|
||||
assert minutes > 0 # float("auto") no longer crashes
|
||||
|
||||
|
||||
def test_rl_checkpoint_only_main_process_writes(tmp_path, monkeypatch):
|
||||
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), output_dir="run", task="grpo"
|
||||
)
|
||||
|
||||
class _M:
|
||||
def save_pretrained(self, path):
|
||||
os.makedirs(path, exist_ok=True)
|
||||
|
||||
monkeypatch.setenv("RANK", "1") # non-main rank
|
||||
path = cb.save_checkpoint(step=1, model=_M(), optimizer=None)
|
||||
assert not os.path.exists(path), "non-main rank must not write the checkpoint"
|
||||
|
||||
monkeypatch.setenv("RANK", "0") # main rank
|
||||
path2 = cb.save_checkpoint(step=2, model=_M(), optimizer=None)
|
||||
assert os.path.exists(os.path.join(path2, "manifest.json"))
|
||||
Loading…
Reference in New Issue