[Plugin] Simplify IO Processor Plugin interface#34236
[Plugin] Simplify IO Processor Plugin interface#34236vllm-bot merged 12 commits intovllm-project:mainfrom
Conversation
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
|
Documentation preview: https://vllm--34236.org.readthedocs.build/en/34236/ |
There was a problem hiding this comment.
Code Review
This pull request simplifies the IOProcessor plugin interface by renaming parse_request to parse_data, deprecating output_to_response, and splitting validate_or_generate_params. The changes are aimed at unifying the online and offline APIs and improving type annotations, with backward compatibility in mind. However, I've identified a critical issue where removing request_id from the post_process method signature breaks the ability for plugins to maintain context between pre-processing and post-processing steps for a given request.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request simplifies the IOProcessor plugin interface by renaming, deprecating, and splitting methods for better clarity and type safety. The changes are well-documented and applied to the test plugin. However, I've identified a critical issue with the backward compatibility shims for merge_sampling_params and merge_pooling_params. These shims are not type-safe and could lead to runtime errors by returning objects of an incorrect type. I've included suggestions to make these shims robust and ensure true backward compatibility.
|
Hi @DarkLight1337, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
|
Thanks @DarkLight1337, let me run a few tests. |
| assert self.io_processor is not None | ||
|
|
||
| pooling_params = self.io_processor.merge_pooling_params() | ||
| if pooling_params.task is None: |
There was a problem hiding this comment.
This is nice, letting the plugin decide which task it will be instead of just setting "plugin". This is useful for plugins that just want to post process the output for existing tasks, for example.
|
PTAL #34214 We need to make the renderer easily accessible to the IO Processor. |
I was about to make the same comment :) |
|
Yeah, this is the first step towards making that happen. I am planning to initialize IOProcessor and use it to process the input prompt regardless of plugin, with the default one being a no-op. Then plugins can have their custom IOProcessor subclass. |
|
Hi @DarkLight1337, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk> Signed-off-by: Eldar Kurtic <research@neuralmagic.com>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Purpose
parse_request -> parse_dataand passrequest.datato it so there is no need to distinguish between offline and online APIs.output_to_response, since we can construct the response automatically from the output data and provided request ID.validate_or_generate_paramsintomerge_sampling_paramsandmerge_pooling_paramsto make type annotation easier.All changes are back-compatible until v0.19.
cc @maxdebayser @christian-pinto
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.