fix: remediate the MEDIUM/LOW code-review findings

- license_matrix: permissive ↔ weak-copyleft is now symmetric (MIT + LGPL no
  longer flagged incompatible regardless of order).
- formats.detect_format: check tool-calling before audio so an audio+tools row
  keeps its tool_calls instead of being classified audio.
- formats: reject null message content in the alpaca / sharegpt / vision text
  converters (routes the row to the drop path instead of literal None content).
  DPO only rejects an explicit null (chosen/rejected may be message lists).
- eval/custom.tool_call_args_subset: hallucinated args on a no-arg expected
  call now score 0.0 (was a dead `0.5 if ... else 0.5` ternary).
- monitoring/callback: SSE metric push uses `is not None` so a real 0.0 loss/lr
  is not reported as None.
- cans/schema.DeployTarget: reject Windows drive-absolute paths (C:\..., C:/...).
- commands/diagnose: reject a non-numeric evidence score with a clear
  BadParameter (was ValueError -> exit 1 with zero output).
- commands/generate: partial-save accumulated examples on a mid-run failure so
  paid API spend is not discarded.
- __init__.py: fix the byte-corrupted em dash in the package docstring.

Two MEDIUM/LOW items reverted to documented known limitations after they broke
existing behaviour locked by tests: (1) the reward_hack EMA smoother is a
recursive 2-tap by design — smoothing_window only affects `median`; (2)
`soup train`'s MoD compute-savings and the hardware_fit OOM preflight wiring
are architectural, GPU-validation work left as follow-ups.

Adds tests/test_code_review_medium_low.py (10 tests). ruff clean; full suite
14861 passed / 120 skipped.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

src/soup_cli/__init__.py

@@ -1,3 +1,3 @@
-"""Soup CLI — Fine-tune and post-train LLMs in one command."""
+"""Soup CLI — Fine-tune and post-train LLMs in one command."""
 
 __version__ = "0.71.26"

src/soup_cli/cans/schema.py

@@ -82,6 +82,10 @@ class DeployTarget(BaseModel):
             raise ValueError("deploy path contains null byte")
         if value.startswith("/") or value.startswith("\\"):
             raise ValueError(f"deploy path '{value}' must be relative")
+        # Windows drive-absolute (``C:\...`` / ``C:/...``) is absolute too but
+        # starts with a letter, so it slipped past the ``/`` / ``\`` check.
+        if len(value) >= 2 and value[1] == ":" and value[0].isalpha():
+            raise ValueError(f"deploy path '{value}' must be relative")
         # Normalise separators first, then split — a mixed-separator path
         # like ``foo/..\\bar`` would otherwise slip past a single-separator
         # split because ``"..\\bar"`` != ``".."``.

src/soup_cli/commands/diagnose.py

@@ -109,6 +109,13 @@ def _scores_from_evidence(payload: dict) -> dict:
         if not isinstance(entry, dict):
             raise typer.BadParameter(f"scores.{mode} must be an object")
         score = entry.get("score", 1.0)
+        # Validate numeric before float() — a non-numeric score otherwise raised
+        # ValueError that exited 1 with zero output. BadParameter prints a clear
+        # message.
+        if isinstance(score, bool) or not isinstance(score, (int, float)):
+            raise typer.BadParameter(
+                f"scores.{mode}.score must be a number, got {score!r}"
+            )
         evidence = entry.get("evidence", "supplied by --evidence")
         verdict = entry.get("verdict") or classify_score(score)
         out[mode] = FailureScore(

src/soup_cli/commands/generate.py

@@ -300,6 +300,21 @@ def generate(
                 )
             except Exception as exc:
                 console.print(f"[red]Generation error: {exc}[/]")
+                # Partial save: don't discard paid API spend on a mid-run
+                # failure — persist whatever was generated so far.
+                if all_examples:
+                    try:
+                        partial_path = Path(output).resolve()
+                        if _path_within_cwd(partial_path, cwd):
+                            with open(partial_path, "w", encoding="utf-8") as f:
+                                for row in all_examples:
+                                    f.write(json.dumps(row, ensure_ascii=False) + "\n")
+                            console.print(
+                                f"[yellow]Saved {len(all_examples)} examples "
+                                f"generated before the error to {partial_path}[/]"
+                            )
+                    except OSError:
+                        pass
                 raise typer.Exit(1)
 
             last_request_time = time.monotonic()

src/soup_cli/data/formats.py

@@ -46,11 +46,13 @@ def detect_format(data: list[dict]) -> str:
     keys = set(sample.keys())
 
     # Check more specific formats first (llava/sharegpt4v before sharegpt).
-    # tool-calling checked before chatml (signature is a superset of chatml).
-    # plaintext ("text" key only) checked last to avoid false matches.
+    # tool-calling (messages+tools+tool_calls) is checked BEFORE audio
+    # (audio+messages): a row carrying both would otherwise match audio first
+    # and silently drop its tools/tool_calls. tool-calling before chatml
+    # (signature is a superset of chatml). plaintext ("text" key only) last.
     check_order = [
         "alpaca", "llava", "sharegpt4v", "kto", "dpo", "embedding",
-        "audio", "tool-calling", "sharegpt", "chatml", "plaintext",
+        "tool-calling", "audio", "sharegpt", "chatml", "plaintext",
     ]
     for fmt in check_order:
         required_keys = FORMAT_SIGNATURES[fmt]
@@ -127,10 +129,23 @@ def format_to_messages(row: dict, fmt: str) -> Optional[dict]:
         return None
 
 
+def _require_str_content(value: object, field: str) -> str:
+    """Reject non-string message content (e.g. a JSON ``null``).
+
+    The older converters passed row values through verbatim, so a JSON
+    ``null`` became literal ``None`` message content. Raising here routes the
+    row to ``format_to_messages``'s drop path instead of silently corrupting
+    the dataset with a ``None``-content turn.
+    """
+    if not isinstance(value, str):
+        raise TypeError(f"{field} must be a string, got {type(value).__name__}")
+    return value
+
+
 def _convert_alpaca(row: dict) -> dict:
-    instruction = row["instruction"]
-    input_text = row.get("input", "")
-    output = row["output"]
+    instruction = _require_str_content(row["instruction"], "alpaca.instruction")
+    input_text = row.get("input") or ""  # missing / null -> ""
+    output = _require_str_content(row["output"], "alpaca.output")
 
     user_content = f"{instruction}\n{input_text}".strip() if input_text else instruction
 
@@ -152,7 +167,9 @@ def _convert_sharegpt(row: dict) -> dict:
     messages = []
     for turn in conversations:
         role = role_map.get(turn["from"], turn["from"])
-        messages.append({"role": role, "content": turn["value"]})
+        messages.append(
+            {"role": role, "content": _require_str_content(turn["value"], "sharegpt.value")}
+        )
 
     return {"messages": messages}
 
@@ -163,7 +180,15 @@ def _convert_chatml(row: dict) -> dict:
 
 
 def _convert_dpo(row: dict) -> dict:
-    """Convert DPO preference row to {prompt, chosen, rejected} for trl.DPOTrainer."""
+    """Convert DPO preference row to {prompt, chosen, rejected} for trl.DPOTrainer.
+
+    Note: chosen/rejected may legitimately be message LISTS (conversational
+    DPO), so this converter only rejects an explicit null rather than requiring
+    a plain string (unlike the alpaca / sharegpt / vision text converters).
+    """
+    for field in ("prompt", "chosen", "rejected"):
+        if row.get(field) is None:
+            raise TypeError(f"dpo.{field} must not be null")
     return {
         "prompt": row["prompt"],
         "chosen": row["chosen"],
@@ -258,7 +283,9 @@ def _convert_vision(row: dict) -> dict:
     messages = []
     for turn in conversations:
         role = role_map.get(turn["from"], turn["from"])
-        messages.append({"role": role, "content": turn["value"]})
+        messages.append(
+            {"role": role, "content": _require_str_content(turn["value"], "vision.value")}
+        )
 
     result = {"messages": messages, "image": row["image"]}
     # Preserve optional id field

src/soup_cli/eval/custom.py

@@ -268,7 +268,10 @@ def tool_call_args_subset(output: str, expected: str) -> float:
     exp_args = _parse_args(exp_func) or {}
 
     if not exp_args:
-        args_score = 0.5 if not out_args else 0.5
+        # No args expected: full credit only if the model also produced none.
+        # Hallucinated args must NOT score full (the old `0.5 if ... else 0.5`
+        # dead ternary gave them full credit).
+        args_score = 0.5 if not out_args else 0.0
     else:
         matched = sum(
             1 for k, v in exp_args.items() if k in out_args and out_args[k] == v

src/soup_cli/monitoring/callback.py

@@ -195,9 +195,11 @@ class SoupTrainerCallback(TrainerCallback):
                     type="metric",
                     step=int(step) if step is not None else None,
                     epoch=float(epoch) if epoch is not None else None,
-                    loss=float(loss) if loss else None,
-                    lr=float(lr) if lr else None,
-                    grad_norm=float(grad_norm) if grad_norm else None,
+                    # `is not None`, not truthiness — a real 0.0 loss / lr (e.g.
+                    # end of an LR schedule) must not be reported as None.
+                    loss=float(loss) if loss is not None else None,
+                    lr=float(lr) if lr is not None else None,
+                    grad_norm=float(grad_norm) if grad_norm is not None else None,
                 )
             )
         except Exception:

src/soup_cli/utils/license_matrix.py

@@ -88,7 +88,11 @@ LICENSE_KINDS = types.MappingProxyType(dict(_LICENSE_KINDS_RAW))
 # be safely combined with. Conservative-by-design — when legal counsel is
 # uncertain, flag the operator (they can `--license-override <reason>`).
 _COMPAT_RAW: dict[str, Tuple[str, ...]] = {
-    _PERMISSIVE: (_PERMISSIVE,),
+    # Permissive ↔ weak-copyleft is symmetric (e.g. MIT + LGPL combine fine).
+    # Listing WEAK_COPYLEFT here mirrors the WEAK_COPYLEFT entry below —
+    # without it the (permissive, weak) ordered pair failed the pairwise check
+    # and flagged the merge regardless of input order.
+    _PERMISSIVE: (_PERMISSIVE, _WEAK_COPYLEFT),
     _WEAK_COPYLEFT: (_WEAK_COPYLEFT, _PERMISSIVE),
     _STRONG_COPYLEFT: (_STRONG_COPYLEFT,),
     _NON_COMMERCIAL: (_NON_COMMERCIAL,),

src/soup_cli/utils/reward_hack_control.py

@@ -171,7 +171,10 @@ def smooth_signal(new: float, window: Sequence[float], *, method: str) -> float:
     if method == "ema":
         if not win:
             return fnew
-        # Standard EMA convention: alpha weights the NEW sample.
+        # Standard recursive EMA: alpha weights the NEW sample against the
+        # previous value (window[-1]). Note: a true EMA is inherently
+        # window-size-independent, so `reward_hack_smoothing_window` only
+        # affects the `median` method — see the v0.71.26 known limitation.
         return _EMA_ALPHA * fnew + (1.0 - _EMA_ALPHA) * win[-1]
     return float(statistics.median(win + [fnew]))
 

tests/test_code_review_medium_low.py

@@ -0,0 +1,94 @@
+"""Regression tests for the MEDIUM/LOW findings in CODE_REVIEW.md."""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+import soup_cli
+
+
+def _src(rel: str) -> str:
+    return (Path(soup_cli.__file__).parent / rel).read_text(encoding="utf-8")
+
+
+def test_license_matrix_permissive_weak_symmetric():
+    from soup_cli.utils.license_matrix import (
+        _PERMISSIVE,
+        _WEAK_COPYLEFT,
+        LICENSE_MATRIX,
+    )
+
+    assert _WEAK_COPYLEFT in LICENSE_MATRIX[_PERMISSIVE]
+    assert _PERMISSIVE in LICENSE_MATRIX[_WEAK_COPYLEFT]
+
+
+def test_detect_format_prefers_tool_calling_over_audio():
+    from soup_cli.data.formats import detect_format
+
+    row = {
+        "messages": [{"role": "user", "content": "x"}],
+        "tools": [{"name": "f"}],
+        "tool_calls": [{"name": "f", "arguments": "{}"}],
+        "audio": "a.wav",
+    }
+    assert detect_format([row]) == "tool-calling"
+
+
+def test_converters_reject_null_content():
+    from soup_cli.data.formats import format_to_messages
+
+    # A JSON null in a required content field routes the row to the drop path
+    # (returns None) instead of producing literal None content.
+    assert format_to_messages({"instruction": "hi", "output": None}, "alpaca") is None
+    assert (
+        format_to_messages(
+            {"prompt": "p", "chosen": None, "rejected": "r"}, "dpo"
+        )
+        is None
+    )
+    # A well-formed row still converts.
+    ok = format_to_messages({"instruction": "hi", "output": "yo"}, "alpaca")
+    assert ok["messages"][-1]["content"] == "yo"
+
+
+def test_tool_call_args_subset_penalizes_hallucinated_args():
+    # The dead ternary `0.5 if not out_args else 0.5` gave hallucinated args
+    # full credit; the fix scores 0.0 for the args portion in that branch.
+    src = _src("eval/custom.py")
+    assert "args_score = 0.5 if not out_args else 0.0" in src
+    assert "0.5 if not out_args else 0.5" not in src
+
+
+def test_median_smoothing_uses_window():
+    from soup_cli.utils.reward_hack_control import smooth_signal
+
+    # `median` genuinely uses the retained window (smoothing_window has effect
+    # here). EMA is recursive/window-independent by design — see the v0.71.26
+    # known-limitation note; its 2-tap form is asserted by test_v07126.
+    assert smooth_signal(10.0, [1.0, 2.0], method="median") == 2.0
+    assert smooth_signal(1.0, [0.0], method="ema") == 0.5
+
+
+def test_sse_metric_push_preserves_zero():
+    assert "float(loss) if loss is not None else None" in _src("monitoring/callback.py")
+
+
+def test_deploy_target_rejects_windows_drive_absolute():
+    src = _src("cans/schema.py")
+    assert 'value[1] == ":"' in src  # drive-absolute (C:\...) now rejected
+
+
+def test_diagnose_rejects_non_numeric_score():
+    src = _src("commands/diagnose.py")
+    assert "must be a number" in src
+
+
+def test_generate_partial_save_present():
+    src = _src("commands/generate.py")
+    assert "Partial save" in src and "generated before the error" in src
+
+
+def test_package_docstring_has_no_mojibake():
+    assert soup_cli.__doc__ is not None
+    assert "вЂ" not in soup_cli.__doc__
+    assert "—" in soup_cli.__doc__