fix(vllm): per-engine TP + strict external config check (harden #68)

[aoshen02]

Hardening on top of #68 (feature/multi_nodes). Two fixes of one root cause: TP / parallel sizing read the global rollout_num_gpus_per_engine instead of the per-engine value, in the two engine-launch paths. No new features.

P2 — managed launch: TP was not actually per-engine

validate_args unconditionally set a global args.vllm_tp_size = global rollout_num_gpus_per_engine // pp, and _resolve_vllm_parallel_sizes preferred it — so the per-engine tp = gpus_per_engine // pp branch was dead code in real runs. A heterogeneous per-group engine (e.g. a group with num_gpus_per_engine=2, tp=2) therefore launched with the global TP, while the trainer sized the NCCL weight-transfer rendezvous from the per-group engine_gpu_counts. They disagreed → the rendezvous hung 300 s (3/4 clients joined / DistStoreError).

• TP is now derived per engine in _resolve_vllm_parallel_sizes (no global shadow), matching upstream slime's sglang_engine (tp = _gpus_per_engine // pp).
• The global vllm_tp_size computation in validate_args is removed.
• dp>1 raises NotImplementedError (DP/EP wiring is a follow-up PR); pp divisibility is validated per engine.

P1 — external engine: weak, wrong-baseline, not node-rank aware

_wait_external_config_ready compared the engine's reported TP against the same global flag and only warned, and it ran on headless workers (node_rank>0) that own no HTTP. Replaced with _sanity_check_external_server_args:

• compares tp/pp/dp/nnodes against the per-engine expectation (self._server_args) and raises on mismatch (fail fast at init instead of a 300 s rendezvous hang later);
• gated to node_rank == 0;
• skips fields /server_info does not report (vLLM parallel_config may omit nnodes), so it stays strict without false-failing.

Note on #66

#66 (aoshen/vllm-mirror-sglang-arch) carries the identical global-TP shadow (_get_vllm_tp_size + its validate_args). This is a fix both branches need, not a port from #66.

Tests

• New unit tests: per-engine TP, heterogeneous per-group TP via compute_vllm_engine_topology, dp>1 guard, and the strict external check (match / mismatch-raises / unreported-field-skipped / missing-parallel-config).
• Existing [Feature]vLLM multi-node rollout engine topology #68 topology unit tests are unchanged and still pass (they set pp consistently, so gpus//pp equals their old vllm_tp_size).
• Verified the new logic in a fresh r3 container (py_compile + inline drivers for all cases). E2E confirmation on test_qwen2.5_0.5B_vllm_config_distributed (heterogeneous tp=2/tp=1) to follow.

AI assistance (Claude Code) was used for this change.

[gemini-code-assist]

Code Review

This pull request refactors how parallel sizes (specifically tensor parallel size) are resolved for vLLM engines, moving from a global precomputation to a per-engine resolution to support heterogeneous per-group engines. It also introduces strict verification of external engine configurations against expected parameters during initialization, raising an error on mismatch instead of just logging a warning. Additionally, only the head node (node_rank 0) now performs external engine initialization checks. Unit tests have been added to cover these changes.

Feedback on the PR highlights two potential issues in _sanity_check_external_server_args:

1. A potential AttributeError if vllm_config is explicitly null in the JSON response, which can be avoided by using or {} instead of .get(..., {}).
2. Raw requests.RequestException errors being raised directly on network or endpoint failures, which should be wrapped in a try-except block to provide a more descriptive error message.

[gemini-code-assist]

If vllm_config is explicitly null (None) in the JSON response, body.get(\"vllm_config\", {}) will return None. Chaining .get(\"parallel_config\", {}) on None will then raise an AttributeError: 'NoneType' object has no attribute 'get', crashing the initialization instead of raising the intended RuntimeError on line 738. Using or {} handles this safely.

        vllm_cfg = body.get(\"vllm_config\") or {}
        parallel_cfg = vllm_cfg.get(\"parallel_config\") or {}

[gemini-code-assist]

If the external server does not support /server_info or if there is a transient network issue, requests.get or response.raise_for_status() will raise a raw requests.RequestException. Wrapping this call in a try-except block and raising a descriptive RuntimeError will significantly improve usability and debuggability.

        try:
            response = requests.get(f\"{self._http_base()}/server_info\", params={\"config_format\": \"json\"}, timeout=30)
            response.raise_for_status()
        except requests.RequestException as e:
            raise RuntimeError(
                f\"Failed to fetch server configuration from external vLLM server at {self._http_base()}/server_info: {e}\"
            ) from e