Skip to content

[Model] Support Hy3 token suffix and JSON Schema array types - #47192

Merged
jeejeelee merged 1 commit into
vllm-project:mainfrom
stevenkuang-tencent:feature/update_hy_v3_parser_md5
Jul 1, 2026
Merged

jeejeelee merged 1 commit into
vllm-project:mainfrom
stevenkuang-tencent:feature/update_hy_v3_parser_md5

Conversation

@stevenkuang-tencent

@stevenkuang-tencent stevenkuang-tencent commented Jun 30, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Support Hy3 token suffix and JSON Schema array types.

Test Plan

Tests for the hy_v3 reasoning parser and tool parser.​

Test Result

Passed


Add two enhancements to the hy_v3 reasoning parser and tool parser
  • Hy3 tokenizers append a suffix to the special tokens. Read the suffix from tokenizer.init_kwargs["token_suffix"] and apply it to the parser's special tokens.
  • Support JSON Schema array types, i.e. union-type declarations such as "type": ["string", "object"].

Signed-off-by: stevenkuang-tencent <stevenkuang@tencent.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

self.tool_calls_start_token: str = "<tool_calls>"
self.tool_calls_end_token: str = "</tool_calls>"
init_kwargs = getattr(tokenizer, "init_kwargs", None) or {}
self.suffix: str = init_kwargs.get("token_suffix") or ""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚪ Severity: LOW

The token_suffix value from the tokenizer config is interpolated into regex patterns (lines 289-303) without re.escape(). A malicious model's tokenizer_config.json could supply regex metacharacters to alter pattern matching of tool calls. Other parsers in this codebase (e.g., deepseekv32_tool_parser.py) correctly use re.escape().
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: Apply re.escape() to the token strings when interpolating them into regex patterns on lines 289-303. The token values (which contain the unescaped suffix) should be escaped before being used in regex compilation. For example, change the regex constructions to use re.escape(self.tool_call_start_token) instead of directly interpolating self.tool_call_start_token. Specifically:

self.tool_call_regex = re.compile(
    re.escape(self.tool_call_start_token) + r"(.*?)"
    + re.escape(self.tool_sep_token) + r"(.*?)"
    + re.escape(self.tool_call_end_token),
    re.DOTALL,
)

self.tool_call_portion_regex = re.compile(
    re.escape(self.tool_call_start_token) + r"(.*?)"
    + re.escape(self.tool_sep_token) + r"(.*)",
    re.DOTALL,
)

self.func_args_regex = re.compile(
    re.escape(self.arg_key_start_token) + r"(.*?)"
    + re.escape(self.arg_key_end_token) + r"\s*"
    + re.escape(self.arg_value_start_token) + r"(.*?)"
    + re.escape(self.arg_value_end_token),
    re.DOTALL,
)

This follows the same pattern used in deepseekv32_tool_parser.py and ensures that any regex metacharacters in the suffix from the tokenizer config are treated as literals.

@sfeng33 sfeng33 left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for these fixes!

For context, there's an ongoing effort to migrate parsers onto the declarative parser engine (https://github.com/vllm-project/vllm/tree/main/vllm/parser/engine) (GLM-4.7, MiniMax-M2, Qwen3 already there), and hy_v3 is a good candidate. If hv_v3 is ported over, the JSON Schema union types crash would be fixed structurally, and the issue with token suffix not recognized becomes a one-place config parameter.

Would you be willing to help with migrating hv over?

@BugenZhao BugenZhao added the ready ONLY add when PR is ready to merge/full CI is needed label Jul 1, 2026

@BugenZhao BugenZhao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the work! Agree with @sfeng33 that we could port this to the parser engine in a longer term, but this PR still looks good as a quick unblock

@jeejeelee
jeejeelee enabled auto-merge (squash) July 1, 2026 06:56
@jeejeelee
jeejeelee merged commit cc56379 into vllm-project:main Jul 1, 2026
60 of 62 checks passed
philippesic pushed a commit to philippesic/vllm-semantic-cache that referenced this pull request Jul 19, 2026
…oject#47192)

Signed-off-by: stevenkuang-tencent <stevenkuang@tencent.com>
michael-gpu pushed a commit to practical-inference/vllm-ampere-extended that referenced this pull request Oct 20, 2026
…oject#47192)

Signed-off-by: stevenkuang-tencent <stevenkuang@tencent.com>
@arpera arpera removed this from Tool Calling Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed tool-calling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants