Unverified Commit cd3f9bbd authored by KrishnanPrash's avatar KrishnanPrash Committed by GitHub
Browse files

fix: treat `--tool-call-parser` and `--dyn-tool-call-parser` independently for SGLang (#5849)


Signed-off-by: default avatarKrishnan Prashanth <kprashanth@nvidia.com>
parent 4fcee92f
...@@ -239,47 +239,13 @@ def _preprocess_for_encode_config( ...@@ -239,47 +239,13 @@ def _preprocess_for_encode_config(
} }
def _set_parser( def _validate_parser_flags(
sglang_str: Optional[str], sglang_val: Optional[str], dynamo_val: Optional[str], name: str
dynamo_str: Optional[str], ) -> None:
arg_name: str = "tool-call-parser", """Validate that --{name} (SGLang) and --dyn-{name} (Dynamo) are not both set."""
) -> Optional[str]: if sglang_val and dynamo_val:
"""Resolve parser name from SGLang and Dynamo arguments. logging.error(f"Cannot use both --{name} and --dyn-{name}.")
sys.exit(1)
Args:
sglang_str: Parser value from SGLang argument.
dynamo_str: Parser value from Dynamo argument.
arg_name: Name of the parser argument for logging.
Returns:
Resolved parser name, preferring Dynamo's value if both set.
Raises:
ValueError: If parser name is not valid.
"""
# If both are present, give preference to dynamo_str
if sglang_str is not None and dynamo_str is not None:
logging.warning(
f"--dyn-{arg_name} and --{arg_name} are both set. Giving preference to --dyn-{arg_name}"
)
return dynamo_str
# If dynamo_str is not set, use try to use sglang_str if it matches with the allowed parsers
elif sglang_str is not None:
logging.warning(f"--dyn-{arg_name} is not set. Using --{arg_name}.")
if arg_name == "tool-call-parser" and sglang_str not in get_tool_parser_names():
raise ValueError(
f"--{arg_name} is not a valid tool call parser. Valid parsers are: {get_tool_parser_names()}"
)
elif (
arg_name == "reasoning-parser"
and sglang_str not in get_reasoning_parser_names()
):
raise ValueError(
f"--{arg_name} is not a valid reasoning parser. Valid parsers are: {get_reasoning_parser_names()}"
)
return sglang_str
else:
return dynamo_str
def _extract_config_section( def _extract_config_section(
...@@ -498,16 +464,20 @@ async def parse_args(args: list[str]) -> Config: ...@@ -498,16 +464,20 @@ async def parse_args(args: list[str]) -> Config:
parsed_namespace, parsed_component_name, parsed_endpoint_name = endpoint_parts parsed_namespace, parsed_component_name, parsed_endpoint_name = endpoint_parts
tool_call_parser = _set_parser( # Validate parser flags: error if both --{name} and --dyn-{name} are set.
# --dyn-{name} choices are validated by argparse; --{name} by SGLang.
_validate_parser_flags(
parsed_args.tool_call_parser, parsed_args.tool_call_parser,
parsed_args.dyn_tool_call_parser, parsed_args.dyn_tool_call_parser,
"tool-call-parser", "tool-call-parser",
) )
reasoning_parser = _set_parser( _validate_parser_flags(
parsed_args.reasoning_parser, parsed_args.reasoning_parser,
parsed_args.dyn_reasoning_parser, parsed_args.dyn_reasoning_parser,
"reasoning-parser", "reasoning-parser",
) )
tool_call_parser = parsed_args.dyn_tool_call_parser
reasoning_parser = parsed_args.dyn_reasoning_parser
if parsed_args.custom_jinja_template and parsed_args.use_sglang_tokenizer: if parsed_args.custom_jinja_template and parsed_args.use_sglang_tokenizer:
logging.error( logging.error(
......
...@@ -75,3 +75,48 @@ async def test_custom_jinja_template_env_var_expansion(monkeypatch, mock_sglang_ ...@@ -75,3 +75,48 @@ async def test_custom_jinja_template_env_var_expansion(monkeypatch, mock_sglang_
f"Expected custom_jinja_template value to be {JINJA_TEMPLATE_PATH}, " f"Expected custom_jinja_template value to be {JINJA_TEMPLATE_PATH}, "
f"got {config.dynamo_args.custom_jinja_template}" f"got {config.dynamo_args.custom_jinja_template}"
) )
# --- Tool Call Parser Validation Tests ---
@pytest.mark.asyncio
async def test_tool_call_parser_valid_with_dynamo_tokenizer(mock_sglang_cli):
"""Valid parser name works when using Dynamo's tokenizer."""
mock_sglang_cli(
"--model",
"Qwen/Qwen3-0.6B",
"--dyn-tool-call-parser",
"hermes", # supported by Dynamo
)
config = await parse_args(sys.argv[1:])
assert config.dynamo_args.tool_call_parser == "hermes"
@pytest.mark.asyncio
async def test_tool_call_parser_invalid_with_dynamo_tokenizer(mock_sglang_cli):
"""Invalid parser name exits when using Dynamo's tokenizer."""
mock_sglang_cli(
"--model", "Qwen/Qwen3-0.6B", "--dyn-tool-call-parser", "nonexistent_parser"
)
with pytest.raises(SystemExit):
await parse_args(sys.argv[1:])
@pytest.mark.asyncio
async def test_tool_call_parser_both_flags_error(mock_sglang_cli):
"""Setting both --dyn-tool-call-parser and --tool-call-parser exits with error."""
mock_sglang_cli(
"--model",
"Qwen/Qwen3-0.6B",
"--dyn-tool-call-parser",
"hermes",
"--tool-call-parser",
"qwen25",
)
with pytest.raises(SystemExit):
await parse_args(sys.argv[1:])
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment