refactor(arguments): align with slime + revert spurious dist.gather_object patch

[aoshen02]

Summary

• vime/backends/vllm_utils/arguments.py (415→361 lines): trim verbose docstrings/comments, slim SKIPPED_DESTS to names-only, add mutual-exclusion assertions in validate_args mirroring slime (vllm_config / prefill_num_servers / rollout_external conflict checks). _make_add_argument_wrapper stays public (5 unit tests reference it by name); validate_args intentionally omits vllm_tp_size (global TP broke heterogeneous per-group engines — per-engine resolver owns it, pinned by test).

• vime/utils/reloadable_process_group.py: drop dist.gather_object monkey-patch (added by PR [feat] Support Colocated Weight Sync via CUDA IPC for vime #22, never in slime; unused in vime — callers explicitly use all_gather_object to stay on the patched path).

Test plan

• pytest tests/unit/backends/vllm_utils/test_arguments.py -m unit — all argument contract tests pass
• pytest tests/ -m unit — full unit suite

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request refactors docstrings and comments in vime/backends/vllm_utils/arguments.py, introduces mutual exclusion validation checks for prefill_num_servers, vllm_config, and rollout_external, and removes the patching of dist.gather_object in vime/utils/reloadable_process_group.py. The reviewer feedback suggests converting SKIPPED_DESTS from a list to a frozenset for more efficient O(1) lookups, and replacing the runtime assert statements with explicit ValueError exceptions to ensure validation is not bypassed under Python optimization flags.

[gemini-code-assist]

Using a frozenset instead of a list for SKIPPED_DESTS improves lookup efficiency from $O(N)$ to $O(1)$ when performing membership checks (e.g., canonical in SKIPPED_DESTS). Since this collection is only used for membership tests, a set-based structure is more idiomatic and performant.

SKIPPED_DESTS = frozenset({
    "model",
    "served_model_name",
    "config",
    "tokenizer",
    "tokenizer_mode",
    "tokenizer_revision",
    "trust_remote_code",
    "seed",
    "dtype",
    # TP is orchestrator-owned; PP/DP remain user-controllable and auto-forward.
    "tensor_parallel_size",
    "nnodes",
    "node_rank",
    "master_addr",
    "master_port",
    "data_parallel_backend",
    "distributed_executor_backend",
    "port",
    "host",
    "enable_return_routed_experts",
})

[gemini-code-assist]

Avoid using assert statements for runtime configuration validation. In Python, assertions can be globally disabled when the interpreter is run with optimization flags (e.g., python -O), which would bypass these mutual-exclusion checks entirely. Use explicit if conditions and raise ValueError instead.

Suggested change

	assert not (
	getattr(args, "prefill_num_servers", None) is not None and getattr(args, "rollout_external", False)
	), "prefill_num_servers cannot be set with --rollout-external-engine-addrs."
	assert not (
	getattr(args, "vllm_config", None) is not None and getattr(args, "rollout_external", False)
	), "vllm_config cannot be set with --rollout-external-engine-addrs."
	assert not (
	getattr(args, "vllm_config", None) is not None and getattr(args, "prefill_num_servers", None) is not None
	), "vllm_config and prefill_num_servers are mutually exclusive. Use server_groups in the YAML config instead."
	if getattr(args, "prefill_num_servers", None) is not None and getattr(args, "rollout_external", False):
	raise ValueError("prefill_num_servers cannot be set with --rollout-external-engine-addrs.")
	if getattr(args, "vllm_config", None) is not None and getattr(args, "rollout_external", False):
	raise ValueError("vllm_config cannot be set with --rollout-external-engine-addrs.")
	if getattr(args, "vllm_config", None) is not None and getattr(args, "prefill_num_servers", None) is not None:
	raise ValueError("vllm_config and prefill_num_servers are mutually exclusive. Use server_groups in the YAML config instead.")

[aoshen02]

Merging into PR #232 instead.