fix(deployment): validate launcher names instead of silently defaulting

[coketaste]

Summary

• normalize_launcher() silently mapped any unrecognized launcher spelling to the docker/native sentinel, so a typo'd launcher ran as a single-process job and reported success instead of failing. This masked a real bug: the documented megatron-lm spelling was broken on both SLURM and Kubernetes because their dispatch arms compared against the literal "megatron".
• Replaces normalization with validate_launcher(), which accepts exactly one canonical spelling per launcher (plus the documented slurm-multi hyphen alias) and raises ConfigurationError with did-you-mean suggestions for anything else. Enforced at two chokepoints — cli/validators.py (CLI additional_context) and BaseDeployment.__init__ (manifest and model-card sources) — covering all three config sources on both backends.
• Fixes the megatron-lm dispatch arms in slurm.py and k8s_template_context.py, and adds sglang-disagg to the Ray GPU-visibility guard in both job templates (it was missing and would otherwise hit Ray's "Inconsistent values found" error).
• Consolidates three divergent launcher-read implementations in container_runner.py into one _resolve_launcher() helper.
• Corrects the megatron → megatron-lm launcher value and drops the dead distributed.master_port key across affected example configs (examples/k8s-configs/, examples/slurm-configs/).

Split out of coketaste/docs-rewrite — this is a behavior change (new validation error path) and belongs in its own review rather than riding along with a docs rewrite. The docs branch depends on this landing first (it documents the megatron-lm spelling and the validation error text).

Test plan

• pytest tests/unit — 646 passed
• New tests/unit/test_launcher_dispatch.py, parametrized over every valid launcher on both backends, asserts each reaches a real dispatch arm rather than falling through silently
• tests/unit/test_validators.py, tests/unit/test_deployment.py, tests/unit/test_slurm_multi.py, tests/unit/test_container_runner.py updated/passing

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR tightens distributed launcher handling across madengine’s deployment paths by replacing permissive launcher normalization with explicit validation, fixing megatron-lm dispatch on SLURM/Kubernetes, and aligning templates/tests so invalid launcher spellings fail loudly instead of silently running single-process jobs.

Changes:

• Replace silent launcher defaulting with validate_launcher() + enforce validation at CLI (cli/validators.py) and deployment initialization (BaseDeployment.__init__).
• Fix backend dispatch/template logic for megatron-lm (previously compared against "megatron"), and extend Ray GPU-visibility guards to include sglang-disagg.
• Consolidate container-runner launcher resolution and update/add unit tests and example configs to use canonical launcher/port keys.

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/unit/test_validators.py	Adds CLI-boundary launcher validation tests (accept/reject/canonicalize).
tests/unit/test_slurm_multi.py	Updates tests to validate_launcher() and asserts unknown launchers now error.
tests/unit/test_launcher_dispatch.py	New parity/dispatch tests to ensure each valid launcher reaches a real dispatch arm.
tests/unit/test_deployment.py	Replaces normalize_launcher tests with launcher_for_reporting + validate_launcher tests.
tests/unit/test_container_runner.py	Updates self-managed launcher tests to match canonical spellings only.
src/madengine/execution/container_runner.py	Consolidates launcher lookup into _resolve_launcher() and drops alias canonicalization.
src/madengine/deployment/templates/slurm/job.sh.j2	Adds sglang-disagg to Ray GPU guard; switches megatron → megatron-lm in templated checks.
src/madengine/deployment/templates/kubernetes/job.yaml.j2	Adds sglang-disagg to Ray GPU guard; updates distributed-env block to megatron-lm.
src/madengine/deployment/slurm.py	Removes normalization/canonicalization; fixes megatron-lm dispatch; uses launcher_for_reporting.
src/madengine/deployment/k8s_template_context.py	Removes alias canonicalization; fixes megatron-lm dispatch arms; treats sglang-disagg as canonical.
src/madengine/deployment/k8s_results.py	Uses launcher_for_reporting in results collection paths.
src/madengine/deployment/common.py	Introduces validate_launcher() and launcher_for_reporting(); updates self-managed detection.
src/madengine/deployment/base.py	Validates/canonicalizes launchers in __init__ across additional_context, manifest, and model cards.
src/madengine/cli/validators.py	Adds _validate_launcher_after_defaults() to fail fast on invalid launcher values.
examples/slurm-configs/README.md	Updates documented launcher list and examples to megatron-lm.
examples/slurm-configs/minimal/megatron-lm-minimal.json	Updates launcher to megatron-lm.
examples/slurm-configs/basic/09-megatron-lm-multi-node.json	Updates launcher to megatron-lm; switches master_port → port.
examples/k8s-configs/README.md	Updates docs to megatron-lm in launcher lists/descriptions.
examples/k8s-configs/minimal/megatron-lm-optimized.json	Updates launcher to megatron-lm; switches master_port → port.
examples/k8s-configs/minimal/megatron-lm-minimal.json	Updates launcher to megatron-lm.
examples/k8s-configs/minimal/megatron-lm-exclude-node.json	Updates launcher to megatron-lm.
examples/k8s-configs/basic/vllm-multi-node-basic.json	Switches master_port → port.
examples/k8s-configs/basic/torchtitan-multi-node-basic.json	Switches master_port → port.
examples/k8s-configs/basic/sglang-multi-node-basic.json	Switches master_port → port.
examples/k8s-configs/basic/sglang-disagg-multi-node-basic.json	Switches master_port → port.
examples/k8s-configs/basic/sglang-disagg-custom-split.json	Switches master_port → port.
examples/k8s-configs/basic/megatron-lm-multi-node-basic.json	Updates launcher to megatron-lm; switches master_port → port.
examples/k8s-configs/basic/06-data-provider-with-pvc.json	Switches master_port → port.
examples/k8s-configs/basic/05-torchrun-nvidia-gpu-example.json	Switches master_port → port.
examples/k8s-configs/basic/04-torchrun-multi-node-advanced.json	Switches master_port → port.
examples/k8s-configs/basic/03-torchrun-multi-node-basic.json	Switches master_port → port.
examples/k8s-configs/basic/02-torchrun-single-node-multi-gpu.json	Switches master_port → port.
examples/k8s-configs/basic/02-torchrun-single-node-multi-gpu-tools.json	Switches master_port → port.
docs/deployment.md	Updates supported launcher spelling to megatron-lm.

Suppressed comments (1)

src/madengine/deployment/common.py:83

• validate_launcher() trims whitespace into normalized, but whitespace-only values (e.g. " ") still fall through to the "unknown launcher" error. This contradicts the function docstring (“Empty values mean 'no launcher configured'”) because after stripping, the value is empty and should be treated as unset.

    normalized = launcher.strip().lower()
    if normalized in _LAUNCHER_SENTINELS:
        return normalized
    normalized = _DOCUMENTED_ALIASES.get(normalized, normalized)
    if normalized in VALID_LAUNCHERS:
        return normalized

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

src/madengine/deployment/k8s_results.py:135

• k8s_results treats only vllm/sglang as Ray-based (is_ray_launcher), but the templates and tests now treat sglang-disagg as Ray-based too. Leaving it out will skip the Ray multi-node handling branches for sglang-disagg jobs.

            launcher_type = launcher_for_reporting(launcher_type, "kubernetes")

            is_ray_launcher = launcher_type in ["vllm", "sglang"]

[Copilot]

🔵 Needs a closer look

Several Kubernetes example configs now set distributed.port, but the Kubernetes backend reads the master port from launcher.master_port, making the updated examples misleading as written.

Review details

Suppressed comments (13)

Previously missed (13) — in code that hasn't changed since the last review.

examples/k8s-configs/basic/02-torchrun-single-node-multi-gpu-tools.json:31

• On Kubernetes, the rendezvous/master port is read from the top-level launcher.master_port object (see k8s_template_context.py), not from distributed.port. Keeping distributed.port in a K8s example suggests it’s configurable there when it’s currently ignored.
  examples/k8s-configs/basic/02-torchrun-single-node-multi-gpu.json:30
• On Kubernetes, the rendezvous/master port is read from the top-level launcher.master_port object (see k8s_template_context.py), not from distributed.port. Keeping distributed.port in a K8s example suggests it’s configurable there when it’s currently ignored.
  examples/k8s-configs/basic/03-torchrun-multi-node-basic.json:31
• The distributed.port field in this Kubernetes example is not used by the Kubernetes backend; k8s_template_context.py reads the master port from the top-level launcher.master_port object instead (see docs/launchers.md note). As written, this example implies distributed.port is honored on Kubernetes when it is effectively dead config, which can mislead users trying to change the rendezvous port.
  examples/k8s-configs/basic/04-torchrun-multi-node-advanced.json:57
• On Kubernetes, the rendezvous/master port is read from the top-level launcher.master_port object (see k8s_template_context.py), not from distributed.port. Keeping distributed.port in a K8s example suggests it’s configurable there when it’s currently ignored.
  examples/k8s-configs/basic/05-torchrun-nvidia-gpu-example.json:34
• On Kubernetes, the rendezvous/master port is read from the top-level launcher.master_port object (see k8s_template_context.py), not from distributed.port. Keeping distributed.port in a K8s example suggests it’s configurable there when it’s currently ignored.
  examples/k8s-configs/basic/06-data-provider-with-pvc.json:39
• On Kubernetes, the rendezvous/master port is read from the top-level launcher.master_port object (see k8s_template_context.py), not from distributed.port. Keeping distributed.port in a K8s example suggests it’s configurable there when it’s currently ignored.
  examples/k8s-configs/basic/megatron-lm-multi-node-basic.json:24
• On Kubernetes, the rendezvous/master port is read from the top-level launcher.master_port object (see k8s_template_context.py), not from distributed.port. Keeping distributed.port in a K8s example suggests it’s configurable there when it’s currently ignored.
  examples/k8s-configs/basic/sglang-disagg-custom-split.json:33
• On Kubernetes, the rendezvous/master port is read from the top-level launcher.master_port object (see k8s_template_context.py), not from distributed.port. Keeping distributed.port in a K8s example suggests it’s configurable there when it’s currently ignored.
  examples/k8s-configs/basic/sglang-disagg-multi-node-basic.json:31
• On Kubernetes, the rendezvous/master port is read from the top-level launcher.master_port object (see k8s_template_context.py), not from distributed.port. Keeping distributed.port in a K8s example suggests it’s configurable there when it’s currently ignored.
  examples/k8s-configs/basic/sglang-multi-node-basic.json:25
• On Kubernetes, the rendezvous/master port is read from the top-level launcher.master_port object (see k8s_template_context.py), not from distributed.port. Keeping distributed.port in a K8s example suggests it’s configurable there when it’s currently ignored.
  examples/k8s-configs/basic/torchtitan-multi-node-basic.json:25
• On Kubernetes, the rendezvous/master port is read from the top-level launcher.master_port object (see k8s_template_context.py), not from distributed.port. Keeping distributed.port in a K8s example suggests it’s configurable there when it’s currently ignored.
  examples/k8s-configs/basic/vllm-multi-node-basic.json:25
• On Kubernetes, the rendezvous/master port is read from the top-level launcher.master_port object (see k8s_template_context.py), not from distributed.port. Keeping distributed.port in a K8s example suggests it’s configurable there when it’s currently ignored.
  examples/k8s-configs/minimal/megatron-lm-optimized.json:35
• On Kubernetes, the rendezvous/master port is read from the top-level launcher.master_port object (see k8s_template_context.py), not from distributed.port. Keeping distributed.port in a K8s example suggests it’s configurable there when it’s currently ignored.

• Files reviewed: 34/34 changed files
• Comments generated: 0 new
• Review effort level: Lite