vllm
vllm copied to clipboard
[Frontend] Support guidance:no-additional-properties for compatibility with xgrammar
The guidance guided decoding backend, has a change in behavior around additionalProperties for JSON object schemas:
guidancefollows the JSON Schema spec and defaults to allowing additional propertiesoutlinesandxgrammarboth do not allow additional properties on objects by default
The main change in this PR is to implement the no-additional-properties backend option to default inject "additionalProperties": false when using guidance to better align the behaviors. This is implemented for V1 and for V0, including re-enabling guidance support for V0. The PR also attempts to simplify how the supported guideded decoding backends and their options are handled.
👋 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.
🚀
Nice PR :-) But I think this commit should be cherry-picked on it to make it work with vllm serve
https://github.com/vllm-project/vllm/commit/e3a9a732db34786e7825986659cc689b5dd1b2f5
PR: https://github.com/44ai-labs/vllm/tree/llguidance-additional-properties
This pull request has merge conflicts that must be resolved before it can be merged. Please rebase the PR, @tjohnson31415.
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork
This pull request has merge conflicts that must be resolved before it can be merged. Please rebase the PR, @tjohnson31415.
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork
This pull request has merge conflicts that must be resolved before it can be merged. Please rebase the PR, @tjohnson31415.
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork
I reworked the branch and rebased on the latest changes that include refactoring of the DecodingConfig (https://github.com/vllm-project/vllm/pull/16789). This overlapped with some of the changes I previously had on the branch.
The new changes have the supported backends as a Literal type, but the options strings aren't included in that type... so that caused a regressions such that using --guided-decoding-backend with options now results in an error:
main.py serve: error: argument --guided-decoding-backend: invalid choice: 'guidance:no-additional-properties' (choose from 'auto', 'outlines', 'lm-format-enforcer', 'xgrammar', 'guidance')
This pull request has merge conflicts that must be resolved before it can be merged. Please rebase the PR, @tjohnson31415.
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork