Merge pull request #601 from noonghunna/hotfix/registry-emit-no-pyyaml

fix(launch): stdlib-only switch/launch table derivation (#584 ModuleNotFoundError: yaml)

AGENTS.md

@@ -70,7 +70,9 @@ When adding the first vendored patch to a previously-rolling engine: pin in the
 **Delivery model (vLLM):** patches reach the container by **volume-mounting into the pinned *stock* `vllm/vllm-openai` image** (python sidecars / site-package overlays / install scripts — see `delivery_mechanism` in `scripts/lib/profiles/patches.yml`), **not** by baking a custom image. The older baked-image path (`ghcr.io/noonghunna/vllm-club3090`, which shipped the release images through `club-v0.8.3`) is **retired** — no compose or engine-pin references it, and the `dockerfile_bake` `delivery:` block in `patches.yml` is legacy/test-only. The GHCR package is kept as historical release artifacts (users pinned to a `club-v0.8.x` tag can still pull); it is not deleted and not produced by anything in-repo.
 
 ### File encoding in scripts — always `encoding="utf-8"`
-Any Python read of a repo source file (compose YAML, profile YAML, `baselines.yml`, …) — including python heredocs inside shell scripts — MUST pass `encoding="utf-8"`. `Path.read_text()` / `open()` default to the **locale** encoding, and community rigs run non-UTF-8 locales (minimal VMs / containers with `LC_ALL=C` → `ANSI_X3.4-1968`). Repo files are full of unicode (`— × → ⚠` in compose headers), so a bare read that works on the dev machine crashes `switch.sh`/`launch.sh` on those rigs (#599). Two corollaries:
+Any Python read of a repo source file (compose YAML, profile YAML, `baselines.yml`, …) — including python heredocs inside shell scripts — MUST pass `encoding="utf-8"`. `Path.read_text()` / `open()` default to the **locale** encoding, and community rigs run non-UTF-8 locales (minimal VMs / containers with `LC_ALL=C` → `ANSI_X3.4-1968`). Repo files are full of unicode (`— × → ⚠` in compose headers), so a bare read that works on the dev machine crashes `switch.sh`/`launch.sh` on those rigs (#599). Corollaries:
+- **Writes too:** a *piped* stdout under the C locale defaults to ASCII — printing a status note with unicode raises `UnicodeEncodeError`. Python emit blocks on launcher paths pin it with `sys.stdout.reconfigure(encoding="utf-8")`.
+- **The launcher table path is python-STDLIB-ONLY** — no PyYAML, no pip deps (community VMs ship bare python3; #584's `ModuleNotFoundError: yaml`). The `--json` contract path may require PyYAML but must fail with a `Fix:` hint, not a traceback. `test-registry-emit-no-yaml` guards both plus the locale cases.
 - **Repro before claiming fixed:** modern Python coerces `C` → `C.UTF-8`, so plain `LC_ALL=C` won't reproduce — use `PYTHONUTF8=0 PYTHONCOERCECLOCALE=0 LC_ALL=C`.
 - **Don't blind-`2>/dev/null` launcher derive paths** — swallowing the traceback hid this exact class for months; capture stderr and surface it on failure instead.
 

scripts/lib/registry-emit.sh

@@ -9,11 +9,33 @@ registry_variant_rows() {
   python3 - "$root" <<'PY_EMIT'
 from __future__ import annotations
 
+import os
 import re
 import sys
 from pathlib import Path
 
-import yaml
+# PyYAML is OPTIONAL on this path (the switch.sh/launch.sh table derivation):
+# community rigs run minimal VMs without python3-yaml (#584 — ryan's Proxmox
+# box), and the ONLY thing this block used yaml for is pulling container_name
+# out of each compose — which the regex fallback below handles for our own
+# compose files. The `--json` contract path (c3 cockpit / baselines join)
+# still requires PyYAML. CLUB3090_EMIT_NO_YAML=1 forces the fallback so the
+# no-yaml guard test can exercise it on rigs where yaml IS installed.
+try:
+    import yaml
+except Exception:
+    yaml = None
+if os.environ.get("CLUB3090_EMIT_NO_YAML") == "1":
+    yaml = None
+
+# Non-UTF-8 locales (LC_ALL=C VMs, #599/#584) also break the WRITE side: a
+# piped stdout defaults to the locale codec → UnicodeEncodeError printing the
+# unicode in status notes. Reads were fixed in #599; pin the output too.
+for _stream in (sys.stdout, sys.stderr):
+    try:
+        _stream.reconfigure(encoding="utf-8")
+    except Exception:
+        pass
 
 root = Path(sys.argv[1])
 sys.path.insert(0, str(root))
@@ -35,8 +57,27 @@ def switch_engine(key: str) -> str:
     return "llamacpp" if prefix == "llamacpp" else prefix
 
 
+# First non-comment `container_name:` line — the regex fallback for rigs
+# without PyYAML. Our compose files are single-service (or first-service-wins,
+# matching the yaml path's dict-order iteration), so this is equivalent for
+# every checked-in compose; test-registry-emit-no-yaml asserts that parity.
+_CONTAINER_RX = re.compile(r"""^\s*container_name:\s*(['"]?)(.+?)\1\s*$""", re.M)
+
+
+def _unwrap_env_default(raw: str) -> str:
+    match = re.fullmatch(r"\$\{[^}:]+:-(.+)\}", raw)
+    return match.group(1) if match else raw
+
+
 def container_name(compose_path: str) -> str:
     path = root / compose_path
+    if yaml is None:
+        try:
+            text = path.read_text(encoding="utf-8")
+        except Exception as exc:
+            raise RuntimeError(f"could not read compose yaml: {exc}") from exc
+        m = _CONTAINER_RX.search(text)
+        return _unwrap_env_default(m.group(2).strip()) if m else ""
     try:
         data = yaml.safe_load(path.read_text(encoding="utf-8")) or {}
     except Exception as exc:
@@ -46,9 +87,7 @@ def container_name(compose_path: str) -> str:
         raw = service.get("container_name")
         if not raw:
             continue
-        raw = str(raw)
-        match = re.fullmatch(r"\$\{[^}:]+:-(.+)\}", raw)
-        return match.group(1) if match else raw
+        return _unwrap_env_default(str(raw))
     return ""
 
 
@@ -390,6 +429,28 @@ import os
 import sys
 from pathlib import Path
 
+# PyYAML is REQUIRED on the --json contract path (profiles + baselines join —
+# load_profiles() below imports it too, so check FIRST and fail with the fix,
+# not a bare ModuleNotFoundError traceback). The switch.sh/launch.sh table
+# path runs stdlib-only (regex container_name fallback, #584) — only
+# c3-cockpit consumers need PyYAML.
+try:
+    import yaml  # noqa: E402,F401
+except Exception:
+    print(
+        "registry-emit --json requires PyYAML (profiles/baselines join).\n"
+        "Fix: sudo apt install python3-yaml   (or: pip install pyyaml)",
+        file=sys.stderr,
+    )
+    raise SystemExit(3)
+
+# Pin output to UTF-8 regardless of locale (see the table-path note, #599/#584).
+for _stream in (sys.stdout, sys.stderr):
+    try:
+        _stream.reconfigure(encoding="utf-8")
+    except Exception:
+        pass
+
 root = Path(sys.argv[1])
 sys.path.insert(0, str(root))
 
@@ -421,7 +482,7 @@ profiles = load_profiles()
 #     BENCHMARKS.md) directly. ---
 import re as _re  # noqa: E402
 
-import yaml as _yaml  # noqa: E402
+_yaml = yaml  # required-import guard at the top of this block (#584)
 
 _bl_path = root / "scripts" / "lib" / "profiles" / "baselines.yml"
 _baselines = {}

scripts/tests/test-registry-emit-no-yaml.sh

@@ -0,0 +1,58 @@
+#!/usr/bin/env bash
+# test-registry-emit-no-yaml.sh — the switch.sh/launch.sh table derivation must
+# be python-STDLIB-ONLY and locale-proof (#584, ryan's Proxmox VM):
+#
+#   1. PyYAML absent  → the regex container_name fallback must produce output
+#      BYTE-IDENTICAL to the yaml parse (CLUB3090_EMIT_NO_YAML=1 forces the
+#      fallback on rigs where yaml IS installed — i.e. CI and this rig).
+#   2. Non-UTF-8 locale (LC_ALL=C + PYTHONUTF8=0 + PYTHONCOERCECLOCALE=0 — the
+#      #599 repro recipe; modern python silently coerces C→C.UTF-8 without the
+#      extra vars) → reads AND writes must still work: compose headers carry
+#      unicode, and a piped stdout under the C locale would otherwise
+#      UnicodeEncodeError printing status notes.
+#   3. Both at once — the actual community-rig configuration that broke.
+#
+# The --json contract path (c3 / baselines join) legitimately requires PyYAML
+# and must fail with an actionable Fix: line, not a bare traceback.
+set -euo pipefail
+ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
+# shellcheck source=../lib/registry-emit.sh
+source "$ROOT/scripts/lib/registry-emit.sh"
+
+fail() { echo "FAIL: $*" >&2; exit 1; }
+
+base="$(registry_variant_rows "$ROOT")" || fail "baseline emit errored"
+[[ -n "$base" ]] || fail "baseline emit produced no rows"
+
+# 1 — yaml-less parity
+noyaml="$(CLUB3090_EMIT_NO_YAML=1 registry_variant_rows "$ROOT")" \
+  || fail "yaml-less emit errored (regex container_name fallback broken)"
+diff <(printf '%s\n' "$base") <(printf '%s\n' "$noyaml") >/dev/null \
+  || fail "yaml-less output drifts from the yaml parse (container_name regex fallback wrong for some compose)"
+
+# 2 — C-locale (reads #599 + writes)
+clocale="$(LC_ALL=C PYTHONUTF8=0 PYTHONCOERCECLOCALE=0 registry_variant_rows "$ROOT")" \
+  || fail "C-locale emit errored (encoding regression — reads need encoding=utf-8, writes need stdout reconfigure)"
+diff <(printf '%s\n' "$base") <(printf '%s\n' "$clocale") >/dev/null \
+  || fail "C-locale output drifts from the UTF-8 run"
+
+# 3 — the ryan-rig combination: no PyYAML + ASCII locale
+ryan="$(CLUB3090_EMIT_NO_YAML=1 LC_ALL=C PYTHONUTF8=0 PYTHONCOERCECLOCALE=0 registry_variant_rows "$ROOT")" \
+  || fail "no-yaml + C-locale emit errored (the #584 community-rig configuration)"
+diff <(printf '%s\n' "$base") <(printf '%s\n' "$ryan") >/dev/null \
+  || fail "no-yaml + C-locale output drifts"
+
+# 4 — the --json path must refuse WITHOUT yaml via an actionable message.
+#     Simulate by hiding yaml with a poisoned import stub on PYTHONPATH.
+stubdir="$(mktemp -d)"
+trap 'rm -rf "$stubdir"' EXIT
+printf 'raise ImportError("PyYAML hidden by test-registry-emit-no-yaml")\n' > "$stubdir/yaml.py"
+set +e
+json_err="$(PYTHONPATH="$stubdir" bash "$ROOT/scripts/lib/registry-emit.sh" --json 2>&1 >/dev/null)"
+json_rc=$?
+set -e
+[[ $json_rc -ne 0 ]] || fail "--json without PyYAML should exit non-zero"
+grep -q "requires PyYAML" <<<"$json_err" || fail "--json no-yaml error lacks the actionable message (got: ${json_err:0:200})"
+grep -q "Fix:" <<<"$json_err" || fail "--json no-yaml error lacks a Fix: hint"
+
+echo "PASS: launcher table path is stdlib-only + locale-proof; --json fails actionably without PyYAML"