Skip to content

[Feat][Frontend] Added support for HermesToolParser for models without special tokens #16890

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

minpeter
Copy link

@minpeter minpeter commented Apr 20, 2025

Upstream the implementation implemented in the https://github.com/minpeter/hermes-llama-parse repo,

Modify the hermes parser to be compatible with models that do not have separate "<tool_call>", "</tool_call>" tags (e.g. llama),

*I know there is a llama-specific parser, but it is useful in fine-tuning llama-based models.

@minpeter minpeter marked this pull request as draft April 20, 2025 15:39
Copy link

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

@minpeter minpeter changed the title Refactor HermesToolParser for improved readability and functionality Added support for HermesToolParser for models without special tokens Apr 20, 2025
@mergify mergify bot added the frontend label Apr 20, 2025
@minpeter minpeter marked this pull request as ready for review April 20, 2025 15:44
@DarkLight1337 DarkLight1337 requested a review from mgoin April 22, 2025 08:28
@minpeter minpeter requested a review from aarnphm as a code owner June 11, 2025 08:29
@aarnphm aarnphm changed the title Added support for HermesToolParser for models without special tokens [Feat][Frontend] Added support for HermesToolParser for models without special tokens Jun 12, 2025
Copy link
Collaborator

@aarnphm aarnphm left a comment

Choose a reason for hiding this comment

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

Is there a test fine-tuned model that we can use to test this?

maybe a very small finetune llama3.2-1b would work here.

@minpeter
Copy link
Author

minpeter commented Jun 12, 2025

Is there a test fine-tuned model that we can use to test this?

maybe a very small finetune llama3.2-1b would work here.

https://huggingface.co/minpeter/LoRA-Llama-3b-xlam-vllm-test-ci

According to the project's naming convention,
'Buffered_delta_text' will be changed to 'buffered_delta_text'.
This will improve the consistency and readability of the code.
@minpeter minpeter requested a review from aarnphm June 12, 2025 09:56
@aarnphm
Copy link
Collaborator

aarnphm commented Jun 13, 2025

Can you add a testcase using that model?

@minpeter
Copy link
Author

Can you add a testcase using that model?

Let me try, is there any other test code I can refer to?

@aarnphm
Copy link
Collaborator

aarnphm commented Jun 15, 2025

iirc there are a few tool parser tests under tests/entrypoints/openai/tool_parsers

@minpeter minpeter marked this pull request as draft June 20, 2025 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants