Skip to content

Conversation

@jreiml
Copy link
Contributor

@jreiml jreiml commented Jan 10, 2026

What does this PR do?

Fixes CLI argument serialization for list-type config values in async vLLM server. Lists like cudagraph_capture_sizes=[1, 2, 4, 8] are now correctly expanded to separate CLI arguments.

Checklist Before Starting

Test

Added CPU unit tests in tests/workers/rollout/test_vllm_cli_args_on_cpu.py covering:

  • String, int, float values → ['--key', 'value']
  • Bool True → ['--key'], Bool False → skipped
  • None values → skipped
  • List expansion → ['--key', '1', '2', '3']
  • Empty lists → skipped (vLLM nargs="+" requires at least one value)
  • Dict values → JSON serialized
  • Mixed configs, order preservation, edge cases

API and Usage Example

No API changes.

Design & Code Changes

Problem:
When passing list-type configuration values (e.g., cudagraph_capture_sizes) to the vLLM async server, the old code used str(list) which produces "[1, 2, 4, 8]" as a single argument.

However, vLLM's argparse uses nargs="+" for list arguments, which expects multiple space-separated values like --cudagraph-capture-sizes 1 2 4 8.

This caused argparse to fail with: invalid int value: '[1, 2, 4, 8]'

Solution:

  1. Extract build_cli_args_from_config() helper function for testability
  2. Expand list values into multiple separate CLI arguments:
# Before: ['--cudagraph-capture-sizes', '[1, 2, 4, 8]']  # FAILS
# After:  ['--cudagraph-capture-sizes', '1', '2', '4', '8']  # WORKS
  1. Handle edge case of empty lists (skip them since nargs="+" requires at least one value)

Checklist Before Submitting

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly fixes an issue with serializing list-type CLI arguments for the vLLM async server. The changes ensure that lists are expanded into space-separated values as expected by argparse. I've included one suggestion to improve the code structure for better readability and maintainability.

@jreiml jreiml force-pushed the fix/vllm-list-cli-args branch 3 times, most recently from 0c07062 to 34a13df Compare January 10, 2026 14:52
jreiml and others added 2 commits January 10, 2026 15:54
Lists need to be expanded as multiple separate arguments for argparse
nargs="+". The old code used str(list) which produces "[1, 2, 4, 8]"
but argparse expects "1 2 4 8" as separate arguments.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Extract build_cli_args_from_config helper function for testability
- Add comprehensive tests covering all value types (string, int, bool, list, dict, None)
- Tests verify correct handling of empty lists and bool False (skipped)
@jreiml jreiml force-pushed the fix/vllm-list-cli-args branch from 34a13df to 0d1baca Compare January 10, 2026 14:54
@vermouth1992 vermouth1992 merged commit 7c7f3c6 into volcengine:main Jan 12, 2026
69 of 71 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants