mirror of https://github.com/razor-ai/soup.git
fix: resolve full-tree code-review findings (6 HIGH + MEDIUM/LOW)
Address every finding from the v0.71.32 review round. No version bump —
lands under CHANGELOG [Unreleased] for the next patch.
HIGH:
- serve: vLLM no longer forces trust_remote_code=True; resolved once for
all backends via the v0.36.0 default-deny gate + warning panel.
- serve: multi-adapter serving actually loads + switches adapters
(_load_named_adapters + lock-guarded _adapter_scope on both the
non-stream and stream generation paths); was a silent no-op.
- data/loader: vision image paths are containment-checked against
image_dir (mirrors the audio loader) — {"image": "/etc/passwd"} dropped.
- train: --dry-run skips the accelerate re-exec; also forwards
--minillm-on-policy / --capture-activations / --capture-prompts.
- trainer/mlx_sft: build a real AdamW instead of optimizer=None.
- commands/data: escape dataset/HF-derived strings in inspect/preview/search.
ASR (v0.71.32 review): metric calls moved inside the per-row try (no
batch-crash on an oversized reference), control-strip + escape on skip
warnings, transcript-label cap to Whisper's 448 + >30s audio warning,
upfront --asr-task validation, exit 2 on all-skipped, fp16 fallback on
pre-Ampere GPUs.
MEDIUM/LOW: distill KD term causal-shift to match the CE term; constant-
time UI Bearer-token compare + locked _train_process read;
load_config_from_string raises ValueError (not TypeError) on non-mapping
YAML; soup runs escapes config-derived fields; vscode_setup + lr_finder
use the centralised atomic, symlink-rejecting writer.
Tests: +28 in tests/test_review_fixes_v07133.py; updated 4 existing tests
that encoded pre-fix behavior (2 vision, 1 distill, 1 vLLM). Full suite
15569 passed, 0 failed, 123 skipped; ruff clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alpamys
parent
a9e17b6add
commit
3a549d3366
36
CHANGELOG.md
36
CHANGELOG.md
|
|
@ -12,6 +12,42 @@ reproducing 70+ versions of notes.
|
|||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
- **`soup serve --backend vllm` no longer force-enables `trust_remote_code`.** The
|
||||
vLLM path now goes through the same `--trust-remote-code` default-deny gate (and
|
||||
warning panel) as the transformers backend, so serving an untrusted repo never
|
||||
executes its code silently.
|
||||
- **Multi-adapter serving (`soup serve --adapters name=path`) now actually switches
|
||||
adapters.** The named adapters are loaded into the model and selected per request
|
||||
(via `POST /v1/adapters/activate/{name}` or the request `adapter` field);
|
||||
previously every request silently ran the startup model. The base model is served
|
||||
when no adapter is selected.
|
||||
- **Vision datasets reject out-of-directory image paths.** `llava` / `sharegpt4v`
|
||||
rows are containment-checked against `image_dir` (mirroring the audio loader), so a
|
||||
crafted `{"image": "/etc/passwd"}` row can no longer read arbitrary local files.
|
||||
- **`soup train --dry-run --gpus N` no longer launches a real multi-GPU run.** The
|
||||
accelerate re-exec is skipped under `--dry-run`. The re-exec also now forwards
|
||||
`--minillm-on-policy`, `--capture-activations`, and `--capture-prompts` (previously
|
||||
dropped on multi-GPU runs).
|
||||
- **MLX SFT now builds a real optimizer** (`AdamW` from the configured LR) instead of
|
||||
passing `optimizer=None`, which left the model untrained.
|
||||
- **`soup data inspect` / `preview` / `search` escape dataset- and Hub-derived text**
|
||||
so a stray `[/]` no longer crashes the command and a crafted `[link=…]` tag can't
|
||||
render a phishing hyperlink. `soup runs` list/show escape config-derived fields too.
|
||||
- **`soup infer --task asr` hardening** — an oversized reference no longer crashes the
|
||||
whole batch after transcription (that row's metric is skipped); an all-skipped run
|
||||
exits non-zero instead of reporting success; `--asr-task` is validated upfront; and
|
||||
dataset-derived filenames are control-stripped before printing. ASR training now
|
||||
caps transcript labels to Whisper's decoder limit, warns on >30 s audio, and picks
|
||||
fp16 on pre-Ampere GPUs instead of hardcoding bf16.
|
||||
- **Knowledge-distillation KD term aligns with the CE term.** The token-level
|
||||
divergence is now computed over causal-shifted positions, so the distillation signal
|
||||
covers exactly the trained tokens (previously off by one).
|
||||
- **Miscellaneous robustness** — `load_config_from_string` raises `ValueError` (not
|
||||
`TypeError`) on a non-mapping YAML document; the Web UI Bearer-token check is
|
||||
constant-time; and `soup doctor --vscode` / the LR-finder report use the centralised
|
||||
atomic, symlink-rejecting writer.
|
||||
|
||||
## [0.71.32] - 2026-07-07
|
||||
|
||||
### Added
|
||||
|
|
|
|||
|
|
@ -53,12 +53,20 @@ def inspect(
|
|||
|
||||
# Print sample rows
|
||||
if rows > 0 and len(data) > 0:
|
||||
# Escape dataset-derived cell content + column names: a stray '[/]' in
|
||||
# ordinary data crashes Rich with MarkupError; a crafted '[link=...]'
|
||||
# renders a phishing hyperlink. Mirrors `soup data review`.
|
||||
from rich.markup import escape as _escape
|
||||
|
||||
console.print(f"\n[bold]Sample rows ({min(rows, len(data))}):[/]")
|
||||
sample_table = Table(show_lines=True)
|
||||
for col in result["columns"][:5]: # max 5 columns
|
||||
sample_table.add_column(col, max_width=60)
|
||||
sample_table.add_column(_escape(str(col)), max_width=60)
|
||||
for row in data[: min(rows, len(data))]:
|
||||
values = [str(row.get(col, ""))[:60] for col in result["columns"][:5]]
|
||||
values = [
|
||||
_escape(str(row.get(col, ""))[:60])
|
||||
for col in result["columns"][:5]
|
||||
]
|
||||
sample_table.add_row(*values)
|
||||
console.print(sample_table)
|
||||
|
||||
|
|
@ -1102,16 +1110,20 @@ def search_datasets(
|
|||
table.add_column("Likes", justify="right")
|
||||
table.add_column("Tags", max_width=30)
|
||||
|
||||
# HF-hub metadata (ids, tags) is attacker-authored — escape before it
|
||||
# reaches a Rich table (MarkupError crash / phishing-link injection).
|
||||
from rich.markup import escape as _escape
|
||||
|
||||
for ds_item in datasets[:limit]:
|
||||
ds_tags = getattr(ds_item, "tags", []) or []
|
||||
tag_str = ", ".join(ds_tags[:5])
|
||||
tag_str = ", ".join(str(t) for t in ds_tags[:5])
|
||||
if len(ds_tags) > 5:
|
||||
tag_str += "..."
|
||||
table.add_row(
|
||||
ds_item.id,
|
||||
_escape(str(ds_item.id)),
|
||||
_format_count(getattr(ds_item, "downloads", 0) or 0),
|
||||
_format_count(getattr(ds_item, "likes", 0) or 0),
|
||||
tag_str,
|
||||
_escape(tag_str),
|
||||
)
|
||||
|
||||
console.print(table)
|
||||
|
|
@ -1137,22 +1149,26 @@ def preview_dataset(
|
|||
)
|
||||
raise typer.Exit(1)
|
||||
|
||||
table = Table(title=f"Dataset: {info['id']}")
|
||||
# HF-hub metadata (id, description, tags, feature/split names) is
|
||||
# attacker-authored — escape before it reaches a Rich table.
|
||||
from rich.markup import escape as _escape
|
||||
|
||||
table = Table(title=f"Dataset: {_escape(str(info['id']))}")
|
||||
table.add_column("Field", style="bold")
|
||||
table.add_column("Value", max_width=80)
|
||||
|
||||
table.add_row("ID", info["id"])
|
||||
table.add_row("ID", _escape(str(info["id"])))
|
||||
desc = info["description"]
|
||||
if len(desc) > 200:
|
||||
desc = desc[:200] + "..."
|
||||
table.add_row("Description", desc or "[dim]No description[/]")
|
||||
table.add_row("Description", _escape(desc) if desc else "[dim]No description[/]")
|
||||
table.add_row("Downloads", _format_count(info["downloads"]))
|
||||
table.add_row("Likes", _format_count(info["likes"]))
|
||||
table.add_row("Size", _format_size_bytes(info["size_bytes"]))
|
||||
|
||||
if info["splits"]:
|
||||
splits_str = ", ".join(
|
||||
f"{name} ({_format_count(count)})"
|
||||
f"{_escape(str(name))} ({_format_count(count)})"
|
||||
for name, count in info["splits"].items()
|
||||
)
|
||||
table.add_row("Splits", splits_str)
|
||||
|
|
@ -1160,10 +1176,10 @@ def preview_dataset(
|
|||
table.add_row("Splits", "[dim]Not available (use streaming to explore)[/]")
|
||||
|
||||
if info["features"]:
|
||||
table.add_row("Features", ", ".join(info["features"]))
|
||||
table.add_row("Features", ", ".join(_escape(str(f)) for f in info["features"]))
|
||||
|
||||
if info["tags"]:
|
||||
table.add_row("Tags", ", ".join(info["tags"][:10]))
|
||||
table.add_row("Tags", ", ".join(_escape(str(t)) for t in info["tags"][:10]))
|
||||
|
||||
console.print(table)
|
||||
|
||||
|
|
|
|||
|
|
@ -164,6 +164,15 @@ def infer(
|
|||
# v0.71.32 — ASR (Whisper) transcription branch. Diverts before the chat
|
||||
# model-resolution path; _infer_asr owns its own Whisper load + output.
|
||||
if task == "asr":
|
||||
# Validate --asr-task up front: a typo would otherwise be passed to
|
||||
# whisper.generate(task=...) and fail INSIDE every row (100k confusing
|
||||
# per-row skips instead of one upfront rejection).
|
||||
if asr_task is not None and asr_task not in ("transcribe", "translate"):
|
||||
console.print(
|
||||
f"[red]--asr-task must be 'transcribe' or 'translate', "
|
||||
f"got {asr_task!r}.[/]"
|
||||
)
|
||||
raise typer.Exit(2)
|
||||
_infer_asr(
|
||||
model=model,
|
||||
base=base,
|
||||
|
|
@ -301,6 +310,19 @@ _ASR_TRANSCRIBER_OVERRIDE = None
|
|||
# vector (mirrors the project's 10k custom-eval / 1e6 HF-download caps).
|
||||
_MAX_ASR_ROWS: int = 100_000
|
||||
|
||||
# C0 control bytes (keep tab/newline/CR) + DEL, stripped from dataset-derived
|
||||
# strings before they reach the terminal. rich.markup.escape() neutralises Rich
|
||||
# '[...]' tags but NOT raw ESC bytes, and a row's audio path / an exception
|
||||
# carrying it is untrusted (title-bar / OSC-8 spoofing). Mirrors v0.71.27
|
||||
# data_doctor._for_terminal.
|
||||
_CONTROL_STRIP_TABLE = {i: None for i in range(0x20) if i not in (0x09, 0x0A, 0x0D)}
|
||||
_CONTROL_STRIP_TABLE[0x7F] = None
|
||||
|
||||
|
||||
def _for_terminal(text: str) -> str:
|
||||
"""Strip C0/ESC/DEL control bytes from a dataset-derived string."""
|
||||
return str(text).translate(_CONTROL_STRIP_TABLE)
|
||||
|
||||
|
||||
def _read_asr_rows(path: Path) -> list[dict]:
|
||||
"""Read ASR rows ``{"audio": path[, "text": reference]}`` from JSONL.
|
||||
|
|
@ -537,6 +559,8 @@ def _infer_asr(
|
|||
console.print(f"[red]{exc}[/]")
|
||||
raise typer.Exit(2) from exc
|
||||
|
||||
from rich.markup import escape as _escape
|
||||
|
||||
refs: list[str] = []
|
||||
hyps: list[str] = []
|
||||
out_lines: list[str] = []
|
||||
|
|
@ -548,20 +572,47 @@ def _infer_asr(
|
|||
hyp = transcribe(resolved)
|
||||
except (ValueError, OSError, ImportError) as exc:
|
||||
skipped += 1
|
||||
console.print(f"[yellow]Skipped {Path(str(audio)).name!r}: {exc}[/]")
|
||||
# Escape + control-strip the dataset-derived filename AND the
|
||||
# exception (whose message embeds that filename) before printing.
|
||||
name = _escape(_for_terminal(Path(str(audio)).name))
|
||||
console.print(
|
||||
f"[yellow]Skipped {name!r}: {_escape(_for_terminal(str(exc)))}[/]"
|
||||
)
|
||||
continue
|
||||
rec = {"audio": audio, "transcription": hyp}
|
||||
ref = row.get("text")
|
||||
if isinstance(ref, str):
|
||||
rec["reference"] = ref
|
||||
rec["wer"] = wer(ref, hyp)
|
||||
rec["cer"] = cer(ref, hyp)
|
||||
refs.append(ref)
|
||||
hyps.append(hyp)
|
||||
# WER/CER can raise ValueError (the _MAX_RAW_CHARS DoS guard) on an
|
||||
# oversized reference. Keep it INSIDE a try so one hostile row is
|
||||
# skipped-unscored, not an uncaught crash that loses every already
|
||||
# transcribed row's output (transcription cost is already paid).
|
||||
try:
|
||||
row_wer = wer(ref, hyp)
|
||||
row_cer = cer(ref, hyp)
|
||||
except ValueError as exc:
|
||||
name = _escape(_for_terminal(Path(str(audio)).name))
|
||||
console.print(
|
||||
f"[yellow]Metric skipped for {name!r}: "
|
||||
f"{_escape(_for_terminal(str(exc)))}[/]"
|
||||
)
|
||||
else:
|
||||
rec["reference"] = ref
|
||||
rec["wer"] = row_wer
|
||||
rec["cer"] = row_cer
|
||||
refs.append(ref)
|
||||
hyps.append(hyp)
|
||||
out_lines.append(json.dumps(rec, ensure_ascii=False))
|
||||
|
||||
atomic_write_text("\n".join(out_lines) + ("\n" if out_lines else ""), output_file,
|
||||
field="--output")
|
||||
# All rows failed to transcribe — do not write an empty file and claim
|
||||
# success; a scripted pipeline (soup ship, CI) must see a non-zero exit.
|
||||
if not out_lines:
|
||||
console.print(
|
||||
f"[red]No clips transcribed ({skipped} skipped). "
|
||||
"Check --audio-dir and the input audio paths.[/]"
|
||||
)
|
||||
raise typer.Exit(2)
|
||||
|
||||
atomic_write_text("\n".join(out_lines) + "\n", output_file, field="--output")
|
||||
|
||||
summary = f"Transcribed [bold]{len(out_lines)}[/] clip(s) -> {output_file}"
|
||||
if skipped:
|
||||
|
|
|
|||
|
|
@ -119,11 +119,14 @@ def list_runs(
|
|||
# Shorten run_id for display
|
||||
short_id = run["run_id"]
|
||||
|
||||
# experiment_name / base_model / task are config-derived — escape so a
|
||||
# crafted value can't inject Rich markup (registry.py escapes the same
|
||||
# fields).
|
||||
table.add_row(
|
||||
short_id,
|
||||
run.get("experiment_name") or "",
|
||||
run.get("base_model") or "",
|
||||
run.get("task") or "",
|
||||
markup_escape(str(short_id)),
|
||||
markup_escape(str(run.get("experiment_name") or "")),
|
||||
markup_escape(str(run.get("base_model") or "")),
|
||||
markup_escape(str(run.get("task") or "")),
|
||||
status_str,
|
||||
loss_str,
|
||||
str(run.get("total_steps") or ""),
|
||||
|
|
@ -167,23 +170,25 @@ def show(
|
|||
minutes = int((secs % 3600) // 60)
|
||||
duration_str = f"{hours}h {minutes}m" if hours > 0 else f"{minutes}m"
|
||||
|
||||
# Build info panel
|
||||
# Build info panel — config-derived fields escaped (mirrors registry.py).
|
||||
_esc = markup_escape
|
||||
info_lines = [
|
||||
f"Run ID: [bold]{run['run_id']}[/]",
|
||||
f"Name: {run.get('experiment_name') or '-'}",
|
||||
f"Run ID: [bold]{_esc(str(run['run_id']))}[/]",
|
||||
f"Name: {_esc(str(run.get('experiment_name') or '-'))}",
|
||||
f"Status: {status_str}",
|
||||
f"Date: {run['created_at'][:19].replace('T', ' ')}",
|
||||
"",
|
||||
f"Model: [bold]{run.get('base_model') or '-'}[/]",
|
||||
f"Task: {run.get('task') or '-'}",
|
||||
f"Device: {run.get('device_name') or '-'} ({run.get('device') or '-'})",
|
||||
f"GPU Memory: {run.get('gpu_memory') or '-'}",
|
||||
f"Model: [bold]{_esc(str(run.get('base_model') or '-'))}[/]",
|
||||
f"Task: {_esc(str(run.get('task') or '-'))}",
|
||||
f"Device: {_esc(str(run.get('device_name') or '-'))} "
|
||||
f"({_esc(str(run.get('device') or '-'))})",
|
||||
f"GPU Memory: {_esc(str(run.get('gpu_memory') or '-'))}",
|
||||
"",
|
||||
f"Loss: {_fmt_loss(run)}",
|
||||
f"Steps: {run.get('total_steps') or '-'}",
|
||||
f"Duration: {duration_str}",
|
||||
f"Cost: {_fmt_cost(run)}",
|
||||
f"Output: {run.get('output_dir') or '-'}",
|
||||
f"Output: {_esc(str(run.get('output_dir') or '-'))}",
|
||||
]
|
||||
console.print(Panel("\n".join(info_lines), title="Run Details"))
|
||||
|
||||
|
|
|
|||
|
|
@ -1,8 +1,10 @@
|
|||
"""soup serve — local inference server with OpenAI-compatible API."""
|
||||
|
||||
import contextlib
|
||||
import json
|
||||
import logging
|
||||
import re
|
||||
import threading
|
||||
import time
|
||||
import uuid
|
||||
from collections.abc import Mapping
|
||||
|
|
@ -722,6 +724,25 @@ def serve(
|
|||
console.print(f"[red]{exc}[/]")
|
||||
raise typer.Exit(1)
|
||||
|
||||
# v0.36.0 Part B: --trust-remote-code default-deny, resolved ONCE for
|
||||
# every backend. vLLM previously loaded with an unconditional
|
||||
# trust_remote_code=True (arbitrary repo code, zero notice) — resolve the
|
||||
# same gate + warning panel the transformers path uses so no backend
|
||||
# silently executes an untrusted repo's code.
|
||||
from soup_cli.utils.trust_remote import (
|
||||
model_requires_trust_remote_code,
|
||||
resolve_trust_remote_code,
|
||||
)
|
||||
|
||||
_trust_probe_target = base_model or str(model_path)
|
||||
_trust_requires = model_requires_trust_remote_code(str(model_path)) or False
|
||||
resolved_trust = resolve_trust_remote_code(
|
||||
_trust_probe_target,
|
||||
requested=trust_remote_code,
|
||||
console=console,
|
||||
requires_remote_code=_trust_requires,
|
||||
)
|
||||
|
||||
if backend == "vllm":
|
||||
if speculative_model:
|
||||
console.print(
|
||||
|
|
@ -741,6 +762,7 @@ def serve(
|
|||
num_speculative_tokens=num_speculative_tokens,
|
||||
enable_prefix_caching=prefix_cache,
|
||||
quantization=auto_quant_kwargs.get("quantization"),
|
||||
trust_remote_code=resolved_trust,
|
||||
)
|
||||
elif backend == "sglang":
|
||||
app = _serve_sglang(
|
||||
|
|
@ -752,21 +774,8 @@ def serve(
|
|||
gpu_memory_utilization=gpu_memory_utilization,
|
||||
)
|
||||
else:
|
||||
# Transformers backend (original).
|
||||
# v0.36.0 Part B: --trust-remote-code default-deny.
|
||||
from soup_cli.utils.trust_remote import (
|
||||
model_requires_trust_remote_code,
|
||||
resolve_trust_remote_code,
|
||||
)
|
||||
|
||||
probe_target = base_model or str(model_path)
|
||||
requires = model_requires_trust_remote_code(str(model_path)) or False
|
||||
resolved_trust = resolve_trust_remote_code(
|
||||
probe_target,
|
||||
requested=trust_remote_code,
|
||||
console=console,
|
||||
requires_remote_code=requires,
|
||||
)
|
||||
# Transformers backend (original). ``resolved_trust`` was computed
|
||||
# once above (v0.36.0 Part B default-deny) and shared across backends.
|
||||
|
||||
# v0.71.17 #259 — serve-time MoLE loads its OWN base + N task LoRAs +
|
||||
# gate; the `model` CLI arg is the base, the manifest supplies adapters
|
||||
|
|
@ -815,6 +824,27 @@ def serve(
|
|||
f"— {resolved_kv_runtime.note}"
|
||||
)
|
||||
|
||||
# v0.71.33 — actually load the --adapters map into the model so
|
||||
# /v1/adapters/activate + the per-request `adapter` field switch the
|
||||
# served weights (previously validated + tracked but never applied).
|
||||
peft_adapter_names: set = set()
|
||||
if adapter_map:
|
||||
from rich.markup import escape as _esc
|
||||
|
||||
try:
|
||||
model_obj, peft_adapter_names = _load_named_adapters(
|
||||
model_obj, adapter_map
|
||||
)
|
||||
except Exception as exc: # noqa: BLE001 — surface any PEFT error
|
||||
console.print(
|
||||
f"[red]Failed to load --adapters:[/] {_esc(str(exc))}"
|
||||
)
|
||||
raise typer.Exit(1) from exc
|
||||
console.print(
|
||||
"[green]Adapters ready:[/] "
|
||||
+ ", ".join(sorted(peft_adapter_names))
|
||||
)
|
||||
|
||||
# v0.71.10 #201 — install the activation-steering decode hook. The
|
||||
# handle persists for the server's lifetime (process-global model).
|
||||
if steer is not None:
|
||||
|
|
@ -986,6 +1016,7 @@ def serve(
|
|||
draft_model=draft_model,
|
||||
num_speculative_tokens=num_speculative_tokens,
|
||||
adapter_map=adapter_map if adapter_map else None,
|
||||
peft_adapter_names=peft_adapter_names,
|
||||
output_constraint=constraint,
|
||||
enable_dashboard=dashboard,
|
||||
tracer=tracer,
|
||||
|
|
@ -1035,6 +1066,7 @@ def _serve_vllm(
|
|||
num_speculative_tokens: int = 5,
|
||||
enable_prefix_caching: bool = False,
|
||||
quantization: Optional[str] = None,
|
||||
trust_remote_code: bool = False,
|
||||
):
|
||||
"""Set up vLLM engine and create FastAPI app."""
|
||||
from soup_cli.utils.vllm import create_vllm_app, create_vllm_engine
|
||||
|
|
@ -1050,6 +1082,7 @@ def _serve_vllm(
|
|||
num_speculative_tokens=num_speculative_tokens,
|
||||
enable_prefix_caching=enable_prefix_caching,
|
||||
quantization=quantization,
|
||||
trust_remote_code=trust_remote_code,
|
||||
)
|
||||
console.print("[bold green]vLLM engine ready![/]")
|
||||
|
||||
|
|
@ -1172,6 +1205,67 @@ def _load_model(
|
|||
return model_obj, tokenizer
|
||||
|
||||
|
||||
def _load_named_adapters(model_obj, adapter_map: Dict[str, str]):
|
||||
"""Load the ``--adapters name=path`` map into ``model_obj`` for hot-swap.
|
||||
|
||||
Returns ``(model_obj, adapter_names)``. The returned model is a PeftModel
|
||||
carrying every named adapter; ``adapter_names`` is the set actually loaded.
|
||||
Request-time selection is done by :func:`_adapter_scope`.
|
||||
|
||||
Without this, ``--adapters`` / ``POST /v1/adapters/activate`` / the
|
||||
per-request ``adapter`` field were validated + tracked but NEVER applied —
|
||||
every request silently ran the startup model (v0.71.33 fix).
|
||||
"""
|
||||
from peft import PeftModel
|
||||
|
||||
names = list(adapter_map)
|
||||
already_peft = isinstance(model_obj, PeftModel)
|
||||
for idx, name in enumerate(names):
|
||||
path = adapter_map[name]
|
||||
if idx == 0 and not already_peft:
|
||||
# Wrap the plain base model into a multi-adapter PeftModel.
|
||||
model_obj = PeftModel.from_pretrained(
|
||||
model_obj, path, adapter_name=name
|
||||
)
|
||||
else:
|
||||
model_obj.load_adapter(path, adapter_name=name)
|
||||
console.print(f"[dim]Loaded adapter '{name}' from {path}[/]")
|
||||
model_obj.eval()
|
||||
return model_obj, set(names)
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
def _adapter_scope(model, lock, names, requested, active):
|
||||
"""Select the LoRA adapter for one generation, serialized by ``lock``.
|
||||
|
||||
``requested`` (request body ``adapter`` field) overrides ``active`` (the
|
||||
``/v1/adapters/activate`` selection). A name not in ``names`` (or ``None``)
|
||||
runs the base model with adapters disabled. No-op when no named adapters
|
||||
were loaded (``names`` empty), so the ordinary single-model serve path is
|
||||
completely unaffected.
|
||||
|
||||
The lock spans the whole generation because the PeftModel is process-global
|
||||
and ``set_adapter`` mutates shared state — two concurrent requests on
|
||||
different adapters would otherwise race. Generation is one blocking call in
|
||||
every path (chat / stream / completions all generate-then-return), so this
|
||||
serializes adapter-selected requests but never holds across true streaming.
|
||||
"""
|
||||
if not names or lock is None:
|
||||
yield
|
||||
return
|
||||
name = requested or active
|
||||
with lock:
|
||||
if name and name in names and hasattr(model, "set_adapter"):
|
||||
model.set_adapter(name)
|
||||
yield
|
||||
elif hasattr(model, "disable_adapter"):
|
||||
# No (or unknown) adapter selected → base model for this request.
|
||||
with model.disable_adapter():
|
||||
yield
|
||||
else:
|
||||
yield
|
||||
|
||||
|
||||
def _load_draft_model(speculative_model: str, device: str):
|
||||
"""Load a smaller draft model for speculative decoding."""
|
||||
import re
|
||||
|
|
@ -1313,6 +1407,7 @@ def _create_app(
|
|||
draft_model=None,
|
||||
num_speculative_tokens: int = 5,
|
||||
adapter_map: Optional[Dict[str, str]] = None,
|
||||
peft_adapter_names: Optional[set] = None,
|
||||
output_constraint: Optional[Dict] = None,
|
||||
enable_dashboard: bool = False,
|
||||
tracer=None,
|
||||
|
|
@ -1398,6 +1493,10 @@ def _create_app(
|
|||
|
||||
# Resolved adapter map (name → path)
|
||||
_adapter_map = adapter_map or {}
|
||||
# v0.71.33 — adapters actually loaded into the PeftModel + the lock that
|
||||
# serializes set_adapter + generate (the model is process-global).
|
||||
_peft_adapter_names = peft_adapter_names or set()
|
||||
_generation_lock = threading.Lock()
|
||||
|
||||
# v0.71.12 #221 — VeRA / VB-LoRA bank loaded at startup (or None). The
|
||||
# active user is selected per request via the X-User-Id header.
|
||||
|
|
@ -1520,6 +1619,10 @@ def _create_app(
|
|||
mole_runtime=_mole_runtime,
|
||||
loaded_bank=_loaded_bank,
|
||||
x_user_id=x_user_id,
|
||||
adapter_lock=_generation_lock,
|
||||
adapter_names=_peft_adapter_names,
|
||||
requested_adapter=requested_adapter,
|
||||
active_adapter=_active_snapshot(),
|
||||
),
|
||||
media_type="text/event-stream",
|
||||
)
|
||||
|
|
@ -1558,17 +1661,28 @@ def _create_app(
|
|||
top_p=request.top_p,
|
||||
)
|
||||
else:
|
||||
response_text, prompt_tokens, completion_tokens = _generate_response(
|
||||
model_obj, tokenizer, messages,
|
||||
max_tokens=max_tokens,
|
||||
temperature=request.temperature,
|
||||
top_p=request.top_p,
|
||||
assistant_model=draft_model,
|
||||
num_assistant_tokens=num_speculative_tokens,
|
||||
logits_processor=processors or None,
|
||||
ngram_config=ngram_config,
|
||||
kv_cache_generate_kwargs=kv_cache_generate_kwargs,
|
||||
)
|
||||
# v0.71.33 — select the request's LoRA adapter (base =
|
||||
# disabled) under the generation lock for the duration
|
||||
# of generate().
|
||||
with _adapter_scope(
|
||||
model_obj, _generation_lock, _peft_adapter_names,
|
||||
requested_adapter, _active_snapshot(),
|
||||
):
|
||||
(
|
||||
response_text,
|
||||
prompt_tokens,
|
||||
completion_tokens,
|
||||
) = _generate_response(
|
||||
model_obj, tokenizer, messages,
|
||||
max_tokens=max_tokens,
|
||||
temperature=request.temperature,
|
||||
top_p=request.top_p,
|
||||
assistant_model=draft_model,
|
||||
num_assistant_tokens=num_speculative_tokens,
|
||||
logits_processor=processors or None,
|
||||
ngram_config=ngram_config,
|
||||
kv_cache_generate_kwargs=kv_cache_generate_kwargs,
|
||||
)
|
||||
except Exception:
|
||||
logger.exception("Generation error")
|
||||
raise HTTPException(status_code=500, detail="Internal server error")
|
||||
|
|
@ -1975,6 +2089,8 @@ def _stream_response(
|
|||
kv_cache_generate_kwargs=None,
|
||||
mole_runtime=None,
|
||||
loaded_bank=None, x_user_id=None,
|
||||
adapter_lock=None, adapter_names=None,
|
||||
requested_adapter=None, active_adapter=None,
|
||||
):
|
||||
"""Generator that yields SSE chunks for streaming responses."""
|
||||
chat_id = f"chatcmpl-{uuid.uuid4().hex[:8]}"
|
||||
|
|
@ -2002,15 +2118,21 @@ def _stream_response(
|
|||
top_p=top_p,
|
||||
)
|
||||
else:
|
||||
response_text, _, completion_tokens_for_log = _generate_response(
|
||||
model, tokenizer, messages,
|
||||
max_tokens=max_tokens,
|
||||
temperature=temperature,
|
||||
top_p=top_p,
|
||||
assistant_model=assistant_model,
|
||||
num_assistant_tokens=num_assistant_tokens,
|
||||
kv_cache_generate_kwargs=kv_cache_generate_kwargs,
|
||||
)
|
||||
# v0.71.33 — select the request's LoRA adapter under the generation
|
||||
# lock (resolved in the endpoint, applied here where generate runs).
|
||||
with _adapter_scope(
|
||||
model, adapter_lock, adapter_names,
|
||||
requested_adapter, active_adapter,
|
||||
):
|
||||
response_text, _, completion_tokens_for_log = _generate_response(
|
||||
model, tokenizer, messages,
|
||||
max_tokens=max_tokens,
|
||||
temperature=temperature,
|
||||
top_p=top_p,
|
||||
assistant_model=assistant_model,
|
||||
num_assistant_tokens=num_assistant_tokens,
|
||||
kv_cache_generate_kwargs=kv_cache_generate_kwargs,
|
||||
)
|
||||
except Exception:
|
||||
logger.exception("Stream generation error")
|
||||
yield 'data: {"error": "Internal server error"}\n\n'
|
||||
|
|
|
|||
|
|
@ -785,7 +785,16 @@ def train(
|
|||
is_in_distributed,
|
||||
)
|
||||
|
||||
if not is_in_distributed():
|
||||
if dry_run and not is_in_distributed():
|
||||
# --dry-run must NEVER os.execvp into a real multi-GPU run.
|
||||
# Without this guard the re-exec fired before the dry_run check
|
||||
# (~350 lines below), so `soup train --dry-run --gpus N` launched
|
||||
# a full accelerate run instead of just validating.
|
||||
console.print(
|
||||
f"[dim]--dry-run: skipping accelerate re-exec "
|
||||
f"({num_gpus} GPUs, {topo['interconnect']}).[/]"
|
||||
)
|
||||
elif not is_in_distributed():
|
||||
# v0.33.0 #37 — auto-reexec under accelerate launch unless
|
||||
# --no-reexec was passed. Reexec uses os.execvp so the new
|
||||
# accelerate process replaces this process; no leftover PID
|
||||
|
|
@ -869,6 +878,15 @@ def train(
|
|||
script_args.extend(["--energy-out", energy_out])
|
||||
if yes:
|
||||
script_args.append("--yes")
|
||||
# Distillation / activation-capture flags — same drop-on-reexec
|
||||
# bug class as the block above: a multi-GPU run silently ignored
|
||||
# them (MiniLLM stayed offline, no activation snapshot written).
|
||||
if minillm_on_policy:
|
||||
script_args.append("--minillm-on-policy")
|
||||
if capture_activations:
|
||||
script_args.extend(["--capture-activations", capture_activations])
|
||||
if capture_prompts:
|
||||
script_args.extend(["--capture-prompts", capture_prompts])
|
||||
argv = build_accelerate_argv(
|
||||
num_processes=num_gpus, script_args=script_args,
|
||||
)
|
||||
|
|
@ -890,22 +908,25 @@ def train(
|
|||
"the launch command for manual execution."
|
||||
)
|
||||
raise typer.Exit(1) from exc
|
||||
console.print(
|
||||
f"[green]Distributed run detected[/] "
|
||||
f"({num_gpus} procs, {topo['interconnect']} interconnect)"
|
||||
)
|
||||
# Apply NCCL env hints. All current keys (``NCCL_P2P_DISABLE`` /
|
||||
# ``NCCL_IB_DISABLE`` / ``NCCL_NVLS_ENABLE``) are rank-idempotent
|
||||
# string literals so it is safe to run on every rank. If a
|
||||
# rank-sensitive key is ever added to ``suggest_nccl_env``, this
|
||||
# loop must be gated to ``LOCAL_RANK == 0``. ``setdefault`` keeps
|
||||
# user / launcher overrides winning over our suggestions.
|
||||
from soup_cli.utils.topology import suggest_nccl_env
|
||||
elif is_in_distributed():
|
||||
# Already a launched rank — announce + apply NCCL hints. (The
|
||||
# dry_run branch above intentionally does neither.)
|
||||
console.print(
|
||||
f"[green]Distributed run detected[/] "
|
||||
f"({num_gpus} procs, {topo['interconnect']} interconnect)"
|
||||
)
|
||||
# Apply NCCL env hints. All current keys (``NCCL_P2P_DISABLE`` /
|
||||
# ``NCCL_IB_DISABLE`` / ``NCCL_NVLS_ENABLE``) are rank-idempotent
|
||||
# string literals so it is safe to run on every rank. If a
|
||||
# rank-sensitive key is ever added to ``suggest_nccl_env``, this
|
||||
# loop must be gated to ``LOCAL_RANK == 0``. ``setdefault`` keeps
|
||||
# user / launcher overrides winning over our suggestions.
|
||||
from soup_cli.utils.topology import suggest_nccl_env
|
||||
|
||||
for key, val in suggest_nccl_env(
|
||||
gpu_count=num_gpus, interconnect=topo["interconnect"]
|
||||
).items():
|
||||
os.environ.setdefault(key, val)
|
||||
for key, val in suggest_nccl_env(
|
||||
gpu_count=num_gpus, interconnect=topo["interconnect"]
|
||||
).items():
|
||||
os.environ.setdefault(key, val)
|
||||
|
||||
# Detect hardware
|
||||
device, device_name = detect_device()
|
||||
|
|
|
|||
|
|
@ -41,6 +41,13 @@ def load_config_from_string(yaml_str: str) -> SoupConfig:
|
|||
raw = yaml.safe_load(yaml_str)
|
||||
if raw is None:
|
||||
raise ValueError("Config is empty")
|
||||
if not isinstance(raw, dict):
|
||||
# A non-mapping document (e.g. a bare list "- a") would make
|
||||
# SoupConfig(**raw) raise TypeError, breaking this function's
|
||||
# ValueError-only contract (API/UI callers only catch ValueError).
|
||||
raise ValueError(
|
||||
f"Config must be a YAML mapping, got {type(raw).__name__}"
|
||||
)
|
||||
|
||||
try:
|
||||
return SoupConfig(**raw)
|
||||
|
|
|
|||
|
|
@ -170,11 +170,17 @@ def load_dataset(data_config: DataConfig) -> dict:
|
|||
def _validate_vision_images(data: list[dict], image_dir: Path) -> list[dict]:
|
||||
"""Validate and resolve image paths in vision dataset rows.
|
||||
|
||||
Each row must have an 'image' key with a filename or path.
|
||||
Resolves relative paths against image_dir.
|
||||
Each row must have an 'image' key with a filename or path. Resolves
|
||||
relative paths against image_dir and rejects path traversal — a crafted
|
||||
llava/sharegpt4v row like ``{"image": "/etc/passwd"}`` must not be handed
|
||||
to ``PIL.Image.open``. Mirrors :func:`_validate_audio_files` (the sibling
|
||||
audio path got this fix in v0.71.32; the vision path was missed).
|
||||
"""
|
||||
from soup_cli.utils.paths import is_under
|
||||
|
||||
valid = []
|
||||
missing = 0
|
||||
traversal = 0
|
||||
for row in data:
|
||||
if "image" not in row or not row["image"]:
|
||||
missing += 1
|
||||
|
|
@ -182,11 +188,21 @@ def _validate_vision_images(data: list[dict], image_dir: Path) -> list[dict]:
|
|||
image_path = Path(row["image"])
|
||||
if not image_path.is_absolute():
|
||||
image_path = image_dir / image_path
|
||||
row["image"] = str(image_path)
|
||||
valid.append(row)
|
||||
# Path traversal protection: resolved path must stay under image_dir.
|
||||
# realpath + commonpath (is_under) — Path.is_relative_to() breaks on
|
||||
# Windows 8.3 short names.
|
||||
if not is_under(image_path, image_dir):
|
||||
traversal += 1
|
||||
continue
|
||||
valid.append({**row, "image": str(image_path.resolve())})
|
||||
|
||||
if missing > 0:
|
||||
console.print(f"[yellow]Warning: {missing} rows skipped (missing image path)[/]")
|
||||
if traversal > 0:
|
||||
console.print(
|
||||
f"[yellow]Warning: {traversal} rows skipped "
|
||||
f"(image path outside {image_dir})[/]"
|
||||
)
|
||||
return valid
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -257,13 +257,34 @@ class AsrTrainerWrapper:
|
|||
|
||||
from soup_cli.utils.tts_codec import load_audio_mono
|
||||
|
||||
# Whisper's decoder is capped at max_target_positions (448 for every
|
||||
# size). A transcript that tokenizes longer would crash mid-training
|
||||
# with an opaque positional-embedding index error, so cap the labels —
|
||||
# honouring a smaller data.max_length when the user set one (previously
|
||||
# ignored). Audio > 30 s is silently truncated to 3000 mel frames by the
|
||||
# feature extractor while the full transcript stays in the labels (that
|
||||
# teaches hallucination), so count both and warn once after the map.
|
||||
max_target_positions = int(
|
||||
getattr(self._unwrapped_model().config, "max_target_positions", 448)
|
||||
)
|
||||
label_cap = max_target_positions
|
||||
if cfg.data.max_length and int(cfg.data.max_length) < label_cap:
|
||||
label_cap = int(cfg.data.max_length)
|
||||
max_audio_samples = _ASR_SAMPLE_RATE * 30
|
||||
trunc = {"labels": 0, "audio": 0}
|
||||
|
||||
def encode(row: dict) -> dict:
|
||||
audio_path, text = _validate_asr_row(row)
|
||||
wave = load_audio_mono(audio_path, target_sr=_ASR_SAMPLE_RATE)
|
||||
if len(wave) > max_audio_samples:
|
||||
trunc["audio"] += 1
|
||||
features = feature_extractor(
|
||||
wave, sampling_rate=_ASR_SAMPLE_RATE
|
||||
).input_features[0]
|
||||
labels = tokenizer(text).input_ids
|
||||
if len(labels) > label_cap:
|
||||
labels = labels[:label_cap]
|
||||
trunc["labels"] += 1
|
||||
return {"input_features": features, "labels": labels}
|
||||
|
||||
raw_train = Dataset.from_list(dataset["train"])
|
||||
|
|
@ -272,6 +293,16 @@ class AsrTrainerWrapper:
|
|||
if dataset.get("val"):
|
||||
raw_val = Dataset.from_list(dataset["val"])
|
||||
eval_ds = raw_val.map(encode, remove_columns=raw_val.column_names)
|
||||
if trunc["labels"]:
|
||||
console.print(
|
||||
f"[yellow]{trunc['labels']} row(s) had transcripts longer than "
|
||||
f"{label_cap} tokens; labels truncated (Whisper decoder limit).[/]"
|
||||
)
|
||||
if trunc["audio"]:
|
||||
console.print(
|
||||
f"[yellow]{trunc['audio']} row(s) had audio >30s; the feature "
|
||||
f"extractor truncates to 30s — the transcript may not align.[/]"
|
||||
)
|
||||
|
||||
output_dir = Path(cfg.output)
|
||||
if cfg.experiment_name:
|
||||
|
|
@ -285,6 +316,18 @@ class AsrTrainerWrapper:
|
|||
)
|
||||
warmup_steps = int(total_steps * tcfg.warmup_ratio)
|
||||
|
||||
# Mixed precision by GPU capability — bf16=cuda was hardcoded, which
|
||||
# crashes on pre-Ampere cards (T4 / GTX 16xx) that lack bf16. Fall back
|
||||
# to fp16 there; fp32 on CPU.
|
||||
use_bf16 = use_fp16 = False
|
||||
if self.device == "cuda":
|
||||
import torch
|
||||
|
||||
if torch.cuda.is_bf16_supported():
|
||||
use_bf16 = True
|
||||
else:
|
||||
use_fp16 = True
|
||||
|
||||
args = Seq2SeqTrainingArguments(
|
||||
output_dir=str(output_dir),
|
||||
num_train_epochs=tcfg.epochs,
|
||||
|
|
@ -299,7 +342,8 @@ class AsrTrainerWrapper:
|
|||
logging_steps=tcfg.logging_steps,
|
||||
save_steps=tcfg.save_steps,
|
||||
save_total_limit=3,
|
||||
bf16=self.device == "cuda",
|
||||
bf16=use_bf16,
|
||||
fp16=use_fp16,
|
||||
report_to=self.report_to,
|
||||
deepspeed=self.deepspeed_config,
|
||||
predict_with_generate=True,
|
||||
|
|
|
|||
|
|
@ -77,6 +77,21 @@ def _compute_distill_term(
|
|||
raise ValueError(
|
||||
f"temperature must be finite and positive, got {temperature!r}"
|
||||
)
|
||||
|
||||
# Causal-LM alignment: logits at position i predict token i+1, so the CE
|
||||
# term shifts (logits[:, :-1] vs labels[:, 1:]). The KD term must shift the
|
||||
# SAME way — otherwise the trained-token mask (labels != -100) is applied
|
||||
# one position off: it drops each assistant span's first predicted token and
|
||||
# leaks the boundary token just before the span. Shift here so both terms
|
||||
# measure the same positions.
|
||||
if labels is not None or attention_mask is not None:
|
||||
student_logits = student_logits[:, :-1, :]
|
||||
teacher_logits = teacher_logits[:, :-1, :]
|
||||
if labels is not None:
|
||||
labels = labels[:, 1:]
|
||||
if attention_mask is not None:
|
||||
attention_mask = attention_mask[:, 1:]
|
||||
|
||||
temp = float(temperature)
|
||||
s = student_logits / temp
|
||||
t = teacher_logits / temp
|
||||
|
|
|
|||
|
|
@ -101,10 +101,17 @@ class MLXSFTTrainerWrapper:
|
|||
adapter_file=str(output_dir / "adapters.safetensors"),
|
||||
)
|
||||
|
||||
# mlx_lm.tuner.trainer.train requires a real optimizer — passing None
|
||||
# left the model untrained (AttributeError on optimizer.update, or a
|
||||
# silent no-op checkpoint). Build an AdamW from the configured LR.
|
||||
import mlx.optimizers as optim # type: ignore
|
||||
|
||||
optimizer = optim.AdamW(learning_rate=float(cfg.training.lr))
|
||||
|
||||
train(
|
||||
model=self.model,
|
||||
tokenizer=self.tokenizer,
|
||||
optimizer=None,
|
||||
optimizer=optimizer,
|
||||
train_dataset=self._dataset.get("train", []),
|
||||
val_dataset=self._dataset.get("val", []),
|
||||
training_callback=None,
|
||||
|
|
|
|||
|
|
@ -114,7 +114,9 @@ def create_app(host: str = "127.0.0.1", port: int = 7860):
|
|||
auth = request.headers.get("Authorization", "")
|
||||
with _auth_token_lock:
|
||||
expected = f"Bearer {_auth_token}"
|
||||
if auth != expected:
|
||||
# Constant-time compare — a plain != leaks the token byte-by-byte via
|
||||
# response timing when `soup ui --public` is exposed on a LAN.
|
||||
if not secrets.compare_digest(auth, expected):
|
||||
raise HTTPException(status_code=401, detail="Unauthorized")
|
||||
|
||||
# --- Static files ---
|
||||
|
|
@ -504,7 +506,11 @@ def create_app(host: str = "127.0.0.1", port: int = 7860):
|
|||
run_id: Optional[str] = Query(default=None),
|
||||
):
|
||||
"""Return current training progress snapshot."""
|
||||
proc = _train_process
|
||||
# Read the shared process handle under the lock, like every sibling
|
||||
# endpoint (start/status/stop) — avoids a torn read racing a concurrent
|
||||
# start/stop.
|
||||
with _train_lock:
|
||||
proc = _train_process
|
||||
is_running = proc is not None and proc.poll() is None
|
||||
|
||||
if not is_running and run_id is None:
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ import math
|
|||
from pathlib import Path
|
||||
from typing import Optional, Sequence, TypedDict
|
||||
|
||||
from soup_cli.utils.paths import is_under_cwd
|
||||
from soup_cli.utils.paths import atomic_write_text, is_under_cwd
|
||||
|
||||
|
||||
class LRFinderResult(TypedDict):
|
||||
|
|
@ -217,11 +217,11 @@ def save_lr_finder_report(
|
|||
"min_loss_lr": summary["min_loss_lr"],
|
||||
"diverged_at": summary["diverged_at"],
|
||||
}
|
||||
output.parent.mkdir(parents=True, exist_ok=True)
|
||||
# ``allow_nan=False`` is belt-and-braces: report_* are already finite,
|
||||
# but ``smoothed_losses`` could carry a non-finite if the input loss
|
||||
# somehow drifted. Reject rather than emit ``NaN`` (invalid JSON).
|
||||
output.write_text(
|
||||
json.dumps(payload, indent=2, allow_nan=False),
|
||||
encoding="utf-8",
|
||||
)
|
||||
# somehow drifted. Reject (raises ValueError) rather than emit ``NaN``
|
||||
# (invalid JSON) — computed before the write so nothing partial lands.
|
||||
body = json.dumps(payload, indent=2, allow_nan=False)
|
||||
# Atomic write + symlink rejection (centralised TOCTOU defence) instead of
|
||||
# a plain write_text with no symlink check.
|
||||
atomic_write_text(body, str(output), field="output_path")
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ def create_vllm_engine(
|
|||
enable_prefix_caching: bool = False,
|
||||
quantization: Optional[str] = None,
|
||||
sleep_mode: bool = False,
|
||||
trust_remote_code: bool = False,
|
||||
):
|
||||
"""Create a vLLM AsyncLLMEngine for serving.
|
||||
|
||||
|
|
@ -57,6 +58,10 @@ def create_vllm_engine(
|
|||
win for RAG / agent workloads with shared system prompts.
|
||||
sleep_mode: Enable vLLM sleep/standby support (v0.71.21 #124 —
|
||||
requires vLLM >= 0.7; raises a friendly RuntimeError otherwise).
|
||||
trust_remote_code: Execute custom repo code on model load. Default
|
||||
False — the caller (``serve``) resolves this through the shared
|
||||
v0.36.0 ``resolve_trust_remote_code`` gate (flag + warning panel)
|
||||
so an untrusted HF repo never runs code silently.
|
||||
|
||||
Returns:
|
||||
(engine, engine_model_name) tuple.
|
||||
|
|
@ -72,7 +77,7 @@ def create_vllm_engine(
|
|||
tensor_parallel_size=tensor_parallel_size,
|
||||
gpu_memory_utilization=gpu_memory_utilization,
|
||||
dtype=dtype,
|
||||
trust_remote_code=True,
|
||||
trust_remote_code=trust_remote_code,
|
||||
enable_prefix_caching=enable_prefix_caching,
|
||||
)
|
||||
if max_model_len is not None:
|
||||
|
|
@ -84,7 +89,7 @@ def create_vllm_engine(
|
|||
tensor_parallel_size=tensor_parallel_size,
|
||||
gpu_memory_utilization=gpu_memory_utilization,
|
||||
dtype=dtype,
|
||||
trust_remote_code=True,
|
||||
trust_remote_code=trust_remote_code,
|
||||
enable_prefix_caching=enable_prefix_caching,
|
||||
)
|
||||
if max_model_len is not None:
|
||||
|
|
|
|||
|
|
@ -9,9 +9,8 @@ from __future__ import annotations
|
|||
|
||||
import json
|
||||
import os
|
||||
import stat
|
||||
|
||||
from soup_cli.utils.paths import is_under_cwd
|
||||
from soup_cli.utils.paths import atomic_write_text, is_under_cwd
|
||||
|
||||
|
||||
def build_launch_json(*, config_path: str = "soup.yaml") -> dict:
|
||||
|
|
@ -81,24 +80,15 @@ def write_vscode_launch(
|
|||
payload = build_launch_json(config_path=config_path)
|
||||
os.makedirs(real, exist_ok=True)
|
||||
out_path = os.path.join(real, "launch.json")
|
||||
# TOCTOU defence: reject symlinks at the target path regardless of
|
||||
# `force` (mirrors v0.33.0 #22 prune_checkpoints policy). Without this
|
||||
# guard, force=True would follow a pre-placed symlink and overwrite a
|
||||
# file outside cwd.
|
||||
try:
|
||||
st = os.lstat(out_path)
|
||||
except FileNotFoundError:
|
||||
st = None
|
||||
if st is not None:
|
||||
if stat.S_ISLNK(st.st_mode):
|
||||
raise ValueError(
|
||||
"launch.json target is a symlink; aborting"
|
||||
)
|
||||
if not force:
|
||||
raise FileExistsError(
|
||||
f"{out_path} already exists; pass force=True to overwrite"
|
||||
)
|
||||
with open(out_path, "w", encoding="utf-8") as f:
|
||||
json.dump(payload, f, indent=2, ensure_ascii=False)
|
||||
f.write("\n")
|
||||
# Refuse to clobber an existing non-symlink file unless force. A symlink at
|
||||
# the target (regardless of force) is rejected by atomic_write_text's
|
||||
# enforce_under_cwd_and_no_symlink, which also closes the lstat-then-open
|
||||
# TOCTOU window: bytes go to a fresh mkstemp file + os.replace, never
|
||||
# through a swapped-in symlink.
|
||||
if not force and os.path.lexists(out_path) and not os.path.islink(out_path):
|
||||
raise FileExistsError(
|
||||
f"{out_path} already exists; pass force=True to overwrite"
|
||||
)
|
||||
body = json.dumps(payload, indent=2, ensure_ascii=False) + "\n"
|
||||
atomic_write_text(body, out_path, field="launch.json")
|
||||
return out_path
|
||||
|
|
|
|||
|
|
@ -102,24 +102,39 @@ def test_beta_schedule_callback_sets_beta_from_start():
|
|||
|
||||
|
||||
def test_distill_term_masks_padding_and_prompt():
|
||||
"""The default distillation divergence must be measured only over trained
|
||||
tokens (labels != -100), not padding/prompt."""
|
||||
"""The default distillation divergence is measured only over trained tokens,
|
||||
CAUSALLY ALIGNED with the CE term: logit position i is trained iff
|
||||
labels[i+1] != -100 (v0.71.33 off-by-one fix — the mask was previously
|
||||
applied to the unshifted labels, dropping each span's first predicted token
|
||||
and leaking the boundary token past it)."""
|
||||
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
|
||||
# Divergence localised at logit position 1, which predicts token index 2.
|
||||
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))
|
||||
# Token index 2 is the only trained target → logit position 1 is trained →
|
||||
# the divergence there IS measured.
|
||||
labels_trained = torch.tensor([[-100, -100, 5]])
|
||||
masked = float(
|
||||
_compute_distill_term(student, teacher, "forward_kl", 1.0, labels=labels_trained)
|
||||
)
|
||||
assert masked > 0.1, masked
|
||||
|
||||
# Token index 1 trained instead → logit position 0 is the trained one, but
|
||||
# the divergence lives at position 1 → excluded → ~0.
|
||||
labels_other = torch.tensor([[-100, 5, -100]])
|
||||
masked_other = float(
|
||||
_compute_distill_term(student, teacher, "forward_kl", 1.0, labels=labels_other)
|
||||
)
|
||||
assert masked_other < 1e-5, masked_other
|
||||
|
||||
# Unmasked averages over every position → the divergent one still shows.
|
||||
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
|
||||
assert unmasked > 0.05, unmasked
|
||||
|
||||
|
||||
def test_kto_negative_one_label_is_undesirable():
|
||||
|
|
|
|||
|
|
@ -0,0 +1,662 @@
|
|||
"""Regression tests for the v0.71.32 full-tree review findings.
|
||||
|
||||
Covers the 6 HIGH + MEDIUM/LOW findings surfaced by the review round:
|
||||
vLLM trust_remote_code gate, vision image path containment, functional
|
||||
multi-adapter serving, --dry-run/--gpus re-exec guard + dropped-flag forwarding,
|
||||
MLX optimizer construction, Rich-markup escaping in `soup data`, the ASR infer
|
||||
metric-guard / control-strip / task-validation / exit-code fixes, the distill KD
|
||||
off-by-one shift, the load_config_from_string ValueError contract, `soup runs`
|
||||
escaping, the constant-time UI token compare, and the atomic-write centralisation.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import sys
|
||||
import types
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from typer.testing import CliRunner
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# HIGH-1 — vLLM serve must not force trust_remote_code=True
|
||||
# --------------------------------------------------------------------------
|
||||
class TestVllmTrustRemoteCode:
|
||||
def _install_fake_vllm(self, monkeypatch):
|
||||
captured: dict = {}
|
||||
|
||||
class _FakeEngineArgs:
|
||||
def __init__(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
|
||||
class _FakeEngine:
|
||||
@classmethod
|
||||
def from_engine_args(cls, args):
|
||||
return object()
|
||||
|
||||
fake = types.ModuleType("vllm")
|
||||
fake.AsyncEngineArgs = _FakeEngineArgs
|
||||
fake.AsyncLLMEngine = _FakeEngine
|
||||
monkeypatch.setitem(sys.modules, "vllm", fake)
|
||||
return captured
|
||||
|
||||
def test_default_is_false(self, monkeypatch):
|
||||
from soup_cli.utils.vllm import create_vllm_engine
|
||||
|
||||
captured = self._install_fake_vllm(monkeypatch)
|
||||
create_vllm_engine(model_path="some/model")
|
||||
assert captured["trust_remote_code"] is False
|
||||
|
||||
def test_true_is_threaded_through(self, monkeypatch):
|
||||
from soup_cli.utils.vllm import create_vllm_engine
|
||||
|
||||
captured = self._install_fake_vllm(monkeypatch)
|
||||
create_vllm_engine(model_path="some/model", trust_remote_code=True)
|
||||
assert captured["trust_remote_code"] is True
|
||||
|
||||
def test_serve_vllm_passes_resolved_trust(self, monkeypatch):
|
||||
# _serve_vllm must forward its trust_remote_code down to the engine.
|
||||
import soup_cli.commands.serve as serve
|
||||
|
||||
seen: dict = {}
|
||||
|
||||
def _fake_engine(**kwargs):
|
||||
seen.update(kwargs)
|
||||
return object(), "m"
|
||||
|
||||
monkeypatch.setattr(
|
||||
"soup_cli.utils.vllm.create_vllm_engine", _fake_engine
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"soup_cli.utils.vllm.create_vllm_app",
|
||||
lambda **kw: object(),
|
||||
)
|
||||
serve._serve_vllm(
|
||||
model_path=Path("m"),
|
||||
base_model=None,
|
||||
is_adapter=False,
|
||||
max_tokens_default=128,
|
||||
tensor_parallel=1,
|
||||
gpu_memory_utilization=0.9,
|
||||
trust_remote_code=True,
|
||||
)
|
||||
assert seen.get("trust_remote_code") is True
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# HIGH-2 — vision dataset image paths must be containment-checked
|
||||
# --------------------------------------------------------------------------
|
||||
class TestVisionImageContainment:
|
||||
def test_absolute_traversal_dropped(self, tmp_path):
|
||||
from soup_cli.data.loader import _validate_vision_images
|
||||
|
||||
img_dir = tmp_path / "imgs"
|
||||
img_dir.mkdir()
|
||||
(img_dir / "ok.png").write_bytes(b"x")
|
||||
rows = [
|
||||
{"image": "ok.png", "messages": []},
|
||||
{"image": "/etc/passwd", "messages": []},
|
||||
]
|
||||
out = _validate_vision_images([dict(r) for r in rows], img_dir)
|
||||
kept = [Path(r["image"]).name for r in out]
|
||||
assert "ok.png" in kept
|
||||
assert "passwd" not in kept
|
||||
assert len(out) == 1
|
||||
|
||||
def test_relative_traversal_dropped(self, tmp_path):
|
||||
from soup_cli.data.loader import _validate_vision_images
|
||||
|
||||
img_dir = tmp_path / "imgs"
|
||||
img_dir.mkdir()
|
||||
secret = tmp_path / "secret.png"
|
||||
secret.write_bytes(b"x")
|
||||
out = _validate_vision_images(
|
||||
[{"image": "../secret.png", "messages": []}], img_dir
|
||||
)
|
||||
assert out == []
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# HIGH-3 — multi-adapter serving actually switches adapters
|
||||
# --------------------------------------------------------------------------
|
||||
class _FakeAdapterModel:
|
||||
"""Records set_adapter + disable_adapter usage."""
|
||||
|
||||
def __init__(self):
|
||||
self.set_calls: list = []
|
||||
self.disable_calls = 0
|
||||
self.active = None
|
||||
|
||||
def set_adapter(self, name):
|
||||
self.set_calls.append(name)
|
||||
self.active = name
|
||||
|
||||
def disable_adapter(self):
|
||||
model = self
|
||||
|
||||
class _Ctx:
|
||||
def __enter__(self):
|
||||
model.disable_calls += 1
|
||||
return self
|
||||
|
||||
def __exit__(self, *exc):
|
||||
return False
|
||||
|
||||
return _Ctx()
|
||||
|
||||
|
||||
class TestAdapterScope:
|
||||
def _run(self, requested, active, names):
|
||||
import threading
|
||||
|
||||
from soup_cli.commands.serve import _adapter_scope
|
||||
|
||||
model = _FakeAdapterModel()
|
||||
lock = threading.Lock()
|
||||
with _adapter_scope(model, lock, names, requested, active):
|
||||
pass
|
||||
return model
|
||||
|
||||
def test_requested_overrides_active(self):
|
||||
model = self._run("code", "chat", {"chat", "code"})
|
||||
assert model.set_calls == ["code"]
|
||||
assert model.disable_calls == 0
|
||||
|
||||
def test_falls_back_to_active(self):
|
||||
model = self._run(None, "chat", {"chat", "code"})
|
||||
assert model.set_calls == ["chat"]
|
||||
|
||||
def test_unknown_name_uses_base(self):
|
||||
model = self._run("nope", None, {"chat"})
|
||||
assert model.set_calls == []
|
||||
assert model.disable_calls == 1
|
||||
|
||||
def test_none_selects_base(self):
|
||||
model = self._run(None, None, {"chat"})
|
||||
assert model.disable_calls == 1
|
||||
|
||||
def test_no_adapters_is_noop(self):
|
||||
model = self._run("chat", "chat", set())
|
||||
assert model.set_calls == []
|
||||
assert model.disable_calls == 0
|
||||
|
||||
|
||||
class TestLoadNamedAdapters:
|
||||
def test_wraps_plain_model_then_loads_rest(self, monkeypatch):
|
||||
import soup_cli.commands.serve as serve
|
||||
|
||||
loaded: dict = {"from_pretrained": [], "load_adapter": []}
|
||||
|
||||
class _FakePeft:
|
||||
def __init__(self, name="default"):
|
||||
self.name = name
|
||||
|
||||
@classmethod
|
||||
def from_pretrained(cls, model, path, adapter_name=None):
|
||||
loaded["from_pretrained"].append((path, adapter_name))
|
||||
return cls(adapter_name)
|
||||
|
||||
def load_adapter(self, path, adapter_name=None):
|
||||
loaded["load_adapter"].append((path, adapter_name))
|
||||
|
||||
def eval(self):
|
||||
return self
|
||||
|
||||
fake_peft = types.ModuleType("peft")
|
||||
fake_peft.PeftModel = _FakePeft
|
||||
monkeypatch.setitem(sys.modules, "peft", fake_peft)
|
||||
|
||||
plain = object() # not a PeftModel → must be wrapped
|
||||
model, names = serve._load_named_adapters(
|
||||
plain, {"chat": "a/path", "code": "b/path"}
|
||||
)
|
||||
assert names == {"chat", "code"}
|
||||
# First adapter wraps the base; the rest are load_adapter'd.
|
||||
assert loaded["from_pretrained"] == [("a/path", "chat")]
|
||||
assert loaded["load_adapter"] == [("b/path", "code")]
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# HIGH-4 — --dry-run must NOT re-exec, and dropped flags must forward
|
||||
# --------------------------------------------------------------------------
|
||||
def _write_sft_config(tmp_path):
|
||||
(tmp_path / "soup.yaml").write_text(
|
||||
"base: test/model\n"
|
||||
"task: sft\n"
|
||||
"data: {train: data.jsonl, format: alpaca}\n"
|
||||
"training: {epochs: 1, lr: 1e-4, batch_size: 1}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
|
||||
def _force_two_gpus(monkeypatch):
|
||||
from soup_cli.commands import train as train_cmd
|
||||
from soup_cli.utils import launcher as launcher_mod
|
||||
from soup_cli.utils import topology as topo_mod
|
||||
|
||||
for var in ("RANK", "WORLD_SIZE", "LOCAL_RANK"):
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
topo = {"gpu_count": 2, "interconnect": "PCIe"}
|
||||
monkeypatch.setattr(topo_mod, "detect_topology", lambda: topo)
|
||||
monkeypatch.setattr(topo_mod, "resolve_num_gpus", lambda spec: 2)
|
||||
monkeypatch.setattr(train_cmd, "detect_topology", lambda: topo, raising=False)
|
||||
monkeypatch.setattr(
|
||||
train_cmd, "resolve_num_gpus", lambda spec: 2, raising=False
|
||||
)
|
||||
monkeypatch.setattr(launcher_mod, "is_in_distributed", lambda: False)
|
||||
|
||||
|
||||
class TestDryRunNoReexec:
|
||||
def test_dry_run_does_not_execvp(self, tmp_path, monkeypatch):
|
||||
from soup_cli.cli import app
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
_write_sft_config(tmp_path)
|
||||
_force_two_gpus(monkeypatch)
|
||||
|
||||
called = {"execvp": False}
|
||||
|
||||
def _fake_execvp(file, argv):
|
||||
called["execvp"] = True
|
||||
raise SystemExit(99)
|
||||
|
||||
monkeypatch.setattr("os.execvp", _fake_execvp)
|
||||
|
||||
CliRunner().invoke(
|
||||
app, ["train", "--config", "soup.yaml", "--gpus", "2", "--dry-run"]
|
||||
)
|
||||
assert called["execvp"] is False, (
|
||||
"os.execvp was called on a --dry-run multi-GPU invocation"
|
||||
)
|
||||
|
||||
|
||||
class TestReexecForwardsFlags:
|
||||
def test_capture_flags_forwarded(self, tmp_path, monkeypatch):
|
||||
from soup_cli.cli import app
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
_write_sft_config(tmp_path)
|
||||
_force_two_gpus(monkeypatch)
|
||||
|
||||
captured: dict = {}
|
||||
|
||||
def _fake_execvp(file, argv):
|
||||
captured["argv"] = list(argv)
|
||||
raise SystemExit(99)
|
||||
|
||||
monkeypatch.setattr("os.execvp", _fake_execvp)
|
||||
|
||||
CliRunner().invoke(
|
||||
app,
|
||||
[
|
||||
"train", "--config", "soup.yaml", "--gpus", "2", "--yes",
|
||||
"--capture-activations", "model.layers.5",
|
||||
"--capture-prompts", "p.jsonl",
|
||||
],
|
||||
)
|
||||
argv = captured.get("argv", [])
|
||||
assert "--capture-activations" in argv
|
||||
assert "model.layers.5" in argv
|
||||
assert "--capture-prompts" in argv
|
||||
assert "p.jsonl" in argv
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# HIGH-5 — MLX SFT must build a real optimizer (not pass None)
|
||||
# --------------------------------------------------------------------------
|
||||
class TestMlxOptimizer:
|
||||
def test_train_passes_non_none_optimizer(self, tmp_path, monkeypatch):
|
||||
from soup_cli.config.loader import load_config_from_string
|
||||
from soup_cli.trainer.mlx_sft import MLXSFTTrainerWrapper
|
||||
|
||||
cfg = load_config_from_string(
|
||||
"base: mlx-community/tiny\n"
|
||||
"task: sft\n"
|
||||
"backend: mlx\n"
|
||||
"data: {train: d.jsonl, format: chatml}\n"
|
||||
"training: {epochs: 1, lr: 2e-4, batch_size: 1}\n"
|
||||
f"output: {json.dumps(str(tmp_path / 'out'))}\n"
|
||||
)
|
||||
wrapper = MLXSFTTrainerWrapper(cfg)
|
||||
wrapper.model = object()
|
||||
wrapper.tokenizer = object()
|
||||
wrapper._dataset = {"train": [{"messages": []}], "val": []}
|
||||
monkeypatch.setattr(wrapper, "_require_mlx", lambda: None)
|
||||
|
||||
seen: dict = {}
|
||||
|
||||
def _fake_train(**kwargs):
|
||||
seen.update(kwargs)
|
||||
|
||||
class _FakeArgs:
|
||||
def __init__(self, **kwargs):
|
||||
pass
|
||||
|
||||
sentinel = object()
|
||||
|
||||
trainer_mod = types.ModuleType("mlx_lm.tuner.trainer")
|
||||
trainer_mod.TrainingArgs = _FakeArgs
|
||||
trainer_mod.train = _fake_train
|
||||
tuner_mod = types.ModuleType("mlx_lm.tuner")
|
||||
mlx_lm_mod = types.ModuleType("mlx_lm")
|
||||
opt_mod = types.ModuleType("mlx.optimizers")
|
||||
opt_mod.AdamW = lambda learning_rate=None: sentinel
|
||||
mlx_root = types.ModuleType("mlx")
|
||||
|
||||
monkeypatch.setitem(sys.modules, "mlx", mlx_root)
|
||||
monkeypatch.setitem(sys.modules, "mlx.optimizers", opt_mod)
|
||||
monkeypatch.setitem(sys.modules, "mlx_lm", mlx_lm_mod)
|
||||
monkeypatch.setitem(sys.modules, "mlx_lm.tuner", tuner_mod)
|
||||
monkeypatch.setitem(sys.modules, "mlx_lm.tuner.trainer", trainer_mod)
|
||||
|
||||
wrapper.train()
|
||||
assert "optimizer" in seen
|
||||
assert seen["optimizer"] is sentinel
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# HIGH-6 — `soup data inspect` must escape dataset-derived Rich markup
|
||||
# --------------------------------------------------------------------------
|
||||
class TestDataInspectEscape:
|
||||
def test_bracket_slash_does_not_crash(self, tmp_path, monkeypatch):
|
||||
from soup_cli.commands.data import app as data_app
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
data_file = tmp_path / "d.jsonl"
|
||||
# A stray '[/]' + a crafted link tag in ordinary data.
|
||||
rows = [
|
||||
{"instruction": "hi [/] there", "output": "[link=http://x]clickme[/]"},
|
||||
{"instruction": "normal", "output": "fine"},
|
||||
]
|
||||
data_file.write_text(
|
||||
"\n".join(json.dumps(r) for r in rows), encoding="utf-8"
|
||||
)
|
||||
result = CliRunner().invoke(data_app, ["inspect", "d.jsonl"])
|
||||
assert result.exit_code == 0, (result.output, repr(result.exception))
|
||||
# Literal markup survives as text (escaped), not interpreted.
|
||||
assert "[/]" in result.output
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# ASR — infer.py metric guard / control-strip / task-validate / exit code
|
||||
# --------------------------------------------------------------------------
|
||||
def _write_asr_input(tmp_path, rows):
|
||||
p = tmp_path / "in.jsonl"
|
||||
p.write_text("\n".join(json.dumps(r) for r in rows), encoding="utf-8")
|
||||
return p
|
||||
|
||||
|
||||
class TestAsrInferMetricGuard:
|
||||
def test_oversized_reference_does_not_crash(self, tmp_path, monkeypatch):
|
||||
import soup_cli.commands.infer as infer
|
||||
from soup_cli.cli import app as cli_app
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
(tmp_path / "a.wav").write_bytes(b"x")
|
||||
(tmp_path / "b.wav").write_bytes(b"x")
|
||||
big = "word " * 300_000 # > _MAX_RAW_CHARS
|
||||
_write_asr_input(
|
||||
tmp_path,
|
||||
[
|
||||
{"audio": "a.wav", "text": big}, # metric must be skipped
|
||||
{"audio": "b.wav", "text": "hello"}, # metric ok
|
||||
],
|
||||
)
|
||||
monkeypatch.setattr(infer, "_ASR_TRANSCRIBER_OVERRIDE", lambda p: "hello")
|
||||
|
||||
result = CliRunner().invoke(
|
||||
cli_app,
|
||||
["infer", "--task", "asr", "--model", "whatever",
|
||||
"--input", "in.jsonl", "--output", "out.jsonl"],
|
||||
)
|
||||
assert result.exit_code == 0, (result.output, repr(result.exception))
|
||||
lines = [
|
||||
json.loads(x)
|
||||
for x in (tmp_path / "out.jsonl").read_text().splitlines() if x
|
||||
]
|
||||
assert len(lines) == 2 # both rows still transcribed
|
||||
# Oversized row present but unscored; the small row scored.
|
||||
assert "wer" not in lines[0]
|
||||
assert "wer" in lines[1]
|
||||
|
||||
|
||||
class TestAsrSkipControlStrip:
|
||||
def test_hostile_filename_is_stripped(self, tmp_path, monkeypatch):
|
||||
import soup_cli.commands.infer as infer
|
||||
from soup_cli.cli import app as cli_app
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
# A filename carrying a raw ESC byte; transcriber raises so the skip
|
||||
# warning path (which prints the name) runs.
|
||||
(tmp_path / "clip.wav").write_bytes(b"x")
|
||||
|
||||
def _boom(_p):
|
||||
raise ValueError("bad \x1b[31maudio")
|
||||
|
||||
monkeypatch.setattr(infer, "_ASR_TRANSCRIBER_OVERRIDE", _boom)
|
||||
_write_asr_input(tmp_path, [{"audio": "clip.wav"}])
|
||||
result = CliRunner().invoke(
|
||||
cli_app,
|
||||
["infer", "--task", "asr", "--model", "m",
|
||||
"--input", "in.jsonl", "--output", "out.jsonl"],
|
||||
)
|
||||
# All rows skipped → exit 2 (L2), and no raw ESC reaches the terminal.
|
||||
assert result.exit_code == 2
|
||||
assert "\x1b" not in result.output
|
||||
|
||||
|
||||
class TestAsrTaskValidation:
|
||||
def test_bad_asr_task_rejected_upfront(self, tmp_path, monkeypatch):
|
||||
from soup_cli.cli import app as cli_app
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
_write_asr_input(tmp_path, [{"audio": "a.wav"}])
|
||||
result = CliRunner().invoke(
|
||||
cli_app,
|
||||
["infer", "--task", "asr", "--model", "m", "--input", "in.jsonl",
|
||||
"--output", "out.jsonl", "--asr-task", "translat"],
|
||||
)
|
||||
assert result.exit_code == 2
|
||||
assert "transcribe" in result.output
|
||||
|
||||
|
||||
class TestAsrAllSkippedExit:
|
||||
def test_all_rows_skipped_exit_2(self, tmp_path, monkeypatch):
|
||||
import soup_cli.commands.infer as infer
|
||||
from soup_cli.cli import app as cli_app
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
(tmp_path / "a.wav").write_bytes(b"x")
|
||||
|
||||
def _boom(_p):
|
||||
raise OSError("cannot decode")
|
||||
|
||||
monkeypatch.setattr(infer, "_ASR_TRANSCRIBER_OVERRIDE", _boom)
|
||||
_write_asr_input(tmp_path, [{"audio": "a.wav"}])
|
||||
result = CliRunner().invoke(
|
||||
cli_app,
|
||||
["infer", "--task", "asr", "--model", "m",
|
||||
"--input", "in.jsonl", "--output", "out.jsonl"],
|
||||
)
|
||||
assert result.exit_code == 2
|
||||
assert not (tmp_path / "out.jsonl").exists()
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# MEDIUM — distill KD term must shift like the CE term
|
||||
# --------------------------------------------------------------------------
|
||||
class TestDistillKdShift:
|
||||
def test_mask_aligns_to_predicted_token(self):
|
||||
import torch
|
||||
|
||||
from soup_cli.trainer.distill import _compute_distill_term
|
||||
|
||||
# seq=4, vocab=3. Teacher/student differ ONLY at position 1 (which,
|
||||
# after the causal shift, predicts token at index 2).
|
||||
student = torch.zeros(1, 4, 3)
|
||||
teacher = torch.zeros(1, 4, 3)
|
||||
teacher[0, 1, 0] = 10.0 # divergence localised at position 1
|
||||
|
||||
# Case A: only the PREDICTED token (index 2) is trained → after the
|
||||
# shift, position 1 is included → non-zero divergence.
|
||||
labels_a = torch.tensor([[-100, -100, 5, -100]])
|
||||
loss_a = _compute_distill_term(
|
||||
student, teacher, "forward_kl", 1.0, labels=labels_a
|
||||
)
|
||||
assert float(loss_a) > 0.0
|
||||
|
||||
# Case B: only the INPUT token at index 1 is trained (its prediction,
|
||||
# index 1's target = index 2, is NOT) → position 1 excluded → ~0.
|
||||
labels_b = torch.tensor([[-100, 5, -100, -100]])
|
||||
loss_b = _compute_distill_term(
|
||||
student, teacher, "forward_kl", 1.0, labels=labels_b
|
||||
)
|
||||
assert float(loss_b) == pytest.approx(0.0, abs=1e-6)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# MEDIUM — load_config_from_string ValueError-only contract
|
||||
# --------------------------------------------------------------------------
|
||||
class TestLoaderNonMapping:
|
||||
def test_bare_list_raises_valueerror(self):
|
||||
from soup_cli.config.loader import load_config_from_string
|
||||
|
||||
with pytest.raises(ValueError, match="mapping"):
|
||||
load_config_from_string("- a\n- b\n")
|
||||
|
||||
def test_scalar_raises_valueerror(self):
|
||||
from soup_cli.config.loader import load_config_from_string
|
||||
|
||||
with pytest.raises(ValueError, match="mapping"):
|
||||
load_config_from_string("42\n")
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# MEDIUM — `soup runs` escapes config-derived strings
|
||||
# --------------------------------------------------------------------------
|
||||
class TestRunsEscape:
|
||||
def _install_fake_tracker(self, monkeypatch, run):
|
||||
class _FakeTracker:
|
||||
def __init__(self, *a, **k):
|
||||
pass
|
||||
|
||||
def list_runs(self, limit=50):
|
||||
return [run]
|
||||
|
||||
def get_run(self, run_id):
|
||||
return run
|
||||
|
||||
def get_eval_results(self, run_id=None):
|
||||
return []
|
||||
|
||||
def get_metrics(self, run_id):
|
||||
return []
|
||||
|
||||
def close(self):
|
||||
pass
|
||||
|
||||
monkeypatch.setattr(
|
||||
"soup_cli.experiment.tracker.ExperimentTracker", _FakeTracker
|
||||
)
|
||||
|
||||
def _crafted_run(self):
|
||||
return {
|
||||
"run_id": "abcd1234",
|
||||
"experiment_name": "exp [/] boom",
|
||||
"base_model": "org/[link=http://x]evil[/]",
|
||||
"task": "sft",
|
||||
"status": "completed",
|
||||
"created_at": "2026-07-08T10:00:00",
|
||||
"total_steps": 10,
|
||||
"duration_secs": 60,
|
||||
"device_name": "GPU",
|
||||
"device": "cuda",
|
||||
"gpu_memory": "4GB",
|
||||
"output_dir": "out",
|
||||
"config_json": None,
|
||||
}
|
||||
|
||||
def test_list_does_not_crash(self, monkeypatch):
|
||||
from soup_cli.commands.runs import app as runs_app
|
||||
|
||||
self._install_fake_tracker(monkeypatch, self._crafted_run())
|
||||
result = CliRunner().invoke(runs_app, [])
|
||||
assert result.exit_code == 0, (result.output, repr(result.exception))
|
||||
|
||||
def test_show_does_not_crash(self, monkeypatch):
|
||||
from soup_cli.commands.runs import app as runs_app
|
||||
|
||||
self._install_fake_tracker(monkeypatch, self._crafted_run())
|
||||
result = CliRunner().invoke(runs_app, ["show", "abcd1234", "--no-plot"])
|
||||
assert result.exit_code == 0, (result.output, repr(result.exception))
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# MEDIUM — constant-time UI token compare (behaviour preserved)
|
||||
# --------------------------------------------------------------------------
|
||||
class TestConstantTimeToken:
|
||||
def test_wrong_token_401_right_token_ok(self):
|
||||
pytest.importorskip("fastapi")
|
||||
from starlette.testclient import TestClient
|
||||
|
||||
from soup_cli.ui.app import create_app, set_auth_token
|
||||
|
||||
set_auth_token("a" * 32)
|
||||
app = create_app()
|
||||
client = TestClient(app)
|
||||
# A mutating endpoint requires the Bearer token.
|
||||
bad = client.post(
|
||||
"/api/train/start",
|
||||
headers={"Authorization": "Bearer wrong"},
|
||||
json={"config": "base: x"},
|
||||
)
|
||||
assert bad.status_code == 401
|
||||
# Correct token gets past the auth gate (may fail later on validation,
|
||||
# but never with 401).
|
||||
ok = client.post(
|
||||
"/api/train/start",
|
||||
headers={"Authorization": "Bearer " + "a" * 32},
|
||||
json={"config": "not: valid: config"},
|
||||
)
|
||||
assert ok.status_code != 401
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# MEDIUM — atomic writes for vscode_setup + lr_finder
|
||||
# --------------------------------------------------------------------------
|
||||
class TestAtomicWrites:
|
||||
def test_vscode_writes_and_honours_force(self, tmp_path, monkeypatch):
|
||||
from soup_cli.utils.vscode_setup import write_vscode_launch
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
out = write_vscode_launch(config_path="soup.yaml", target_dir=".vscode")
|
||||
assert Path(out).exists()
|
||||
payload = json.loads(Path(out).read_text())
|
||||
assert "configurations" in payload
|
||||
# Second write without force must refuse.
|
||||
with pytest.raises(FileExistsError):
|
||||
write_vscode_launch(config_path="soup.yaml", target_dir=".vscode")
|
||||
# force=True overwrites atomically.
|
||||
write_vscode_launch(
|
||||
config_path="other.yaml", target_dir=".vscode", force=True
|
||||
)
|
||||
assert Path(out).exists()
|
||||
|
||||
def test_lr_finder_writes_under_cwd(self, tmp_path, monkeypatch):
|
||||
from soup_cli.utils.lr_finder import save_lr_finder_report
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
out = tmp_path / "lr.json"
|
||||
save_lr_finder_report([1e-5, 1e-4, 1e-3, 1e-2], [3.0, 2.0, 2.5, 4.0], out)
|
||||
payload = json.loads(out.read_text())
|
||||
assert "recommended_lr" in payload
|
||||
|
||||
def test_lr_finder_rejects_outside_cwd(self, tmp_path, monkeypatch):
|
||||
from soup_cli.utils.lr_finder import save_lr_finder_report
|
||||
|
||||
monkeypatch.chdir(tmp_path)
|
||||
outside = tmp_path.parent / "escape.json"
|
||||
with pytest.raises(ValueError):
|
||||
save_lr_finder_report([1e-5, 1e-4], [3.0, 2.0], outside)
|
||||
|
|
@ -296,14 +296,18 @@ class TestVisionDataLoader:
|
|||
"""Test vision image validation in data loader."""
|
||||
|
||||
def test_validate_vision_images_resolves_relative(self):
|
||||
"""Relative image paths should be resolved against image_dir."""
|
||||
"""Relative image paths are resolved against image_dir (realpath).
|
||||
|
||||
v0.71.33: the validator now stores the resolved path (mirrors
|
||||
_validate_audio_files) after a containment check.
|
||||
"""
|
||||
from soup_cli.data.loader import _validate_vision_images
|
||||
|
||||
data = [{"messages": [{"role": "user", "content": "Hi"}], "image": "photo.jpg"}]
|
||||
image_dir = Path("/data/images")
|
||||
result = _validate_vision_images(data, image_dir)
|
||||
assert len(result) == 1
|
||||
assert result[0]["image"] == str(image_dir / "photo.jpg")
|
||||
assert result[0]["image"] == str((image_dir / "photo.jpg").resolve())
|
||||
|
||||
def test_validate_vision_images_skips_missing(self):
|
||||
"""Rows without image field should be skipped."""
|
||||
|
|
@ -326,15 +330,29 @@ class TestVisionDataLoader:
|
|||
result = _validate_vision_images(data, image_dir)
|
||||
assert len(result) == 0
|
||||
|
||||
def test_validate_vision_images_absolute_path(self):
|
||||
"""Absolute image paths should be kept as-is."""
|
||||
def test_validate_vision_images_absolute_inside_kept(self, tmp_path):
|
||||
"""An absolute image path INSIDE image_dir is kept (resolved)."""
|
||||
from soup_cli.data.loader import _validate_vision_images
|
||||
|
||||
abs_path = str(Path("/absolute/path/photo.jpg"))
|
||||
data = [{"messages": [{"role": "user", "content": "Hi"}], "image": abs_path}]
|
||||
image_dir = Path("/different/dir")
|
||||
result = _validate_vision_images(data, image_dir)
|
||||
assert result[0]["image"] == abs_path
|
||||
img_dir = tmp_path / "imgs"
|
||||
img_dir.mkdir()
|
||||
inside = img_dir / "photo.jpg"
|
||||
inside.write_bytes(b"x")
|
||||
data = [{"messages": [{"role": "user", "content": "Hi"}], "image": str(inside)}]
|
||||
result = _validate_vision_images(data, img_dir)
|
||||
assert len(result) == 1
|
||||
assert Path(result[0]["image"]).name == "photo.jpg"
|
||||
|
||||
def test_validate_vision_images_rejects_out_of_dir(self, tmp_path):
|
||||
"""v0.71.33 security fix: an absolute path OUTSIDE image_dir is dropped
|
||||
(previously it was handed straight to PIL.Image.open — arbitrary read)."""
|
||||
from soup_cli.data.loader import _validate_vision_images
|
||||
|
||||
img_dir = tmp_path / "imgs"
|
||||
img_dir.mkdir()
|
||||
data = [{"messages": [{"role": "user", "content": "Hi"}], "image": "/etc/passwd"}]
|
||||
result = _validate_vision_images(data, img_dir)
|
||||
assert result == []
|
||||
|
||||
def test_load_dataset_with_vision_format(self):
|
||||
"""load_dataset should handle llava format data files."""
|
||||
|
|
|
|||
|
|
@ -505,6 +505,7 @@ class TestServeVllmIntegration:
|
|||
num_speculative_tokens=5,
|
||||
enable_prefix_caching=False,
|
||||
quantization=None, # v0.35.0 #61 — auto-quant default
|
||||
trust_remote_code=False, # v0.71.33 — default-deny gate
|
||||
)
|
||||
mock_create_app.assert_called_once()
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue