feat: implement ONNX and OpenVINO native adapters (Phase 3)#2533
feat: implement ONNX and OpenVINO native adapters (Phase 3)#2533Paramveersingh-S wants to merge 13 commits into
Conversation
✅ Deploy Preview for vllm-semantic-router ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
👥 vLLM Semantic Team NotificationThe following members have been identified for the changed files in this PR and have been automatically assigned when their GitHub accounts are assignable in this repository: 📁
|
Signed-off-by: Param <param15.veer.singh@gmail.com>
Signed-off-by: Param <param15.veer.singh@gmail.com>
Signed-off-by: Param <param15.veer.singh@gmail.com>
8d4484a to
e74cdca
Compare
Signed-off-by: Param <param15.veer.singh@gmail.com>
✅ Supply Chain Security Report — All Clear
Scanned at |
Xunzhuo
left a comment
There was a problem hiding this comment.
Requesting changes because Phase 3 still exposes placeholder adapters as real backends. This repeats the main concern from #2458: a backend should enter the native layer only when it performs real inference and has backend-specific lifecycle/inference tests. This PR also inherits the Candle compile failure (fmt imported and unused), adds no ONNX/OpenVINO tests, is based on main instead of Phase 2, and says Fixes #2396 even though later acceptance phases remain. Please make this a true stack (base: Phase 2), use Part of #2396, and land only truthful, executable backend support.
| func (a *Adapter) LoadModel(ctx context.Context, req native.LoadRequest) (native.ModelHandle, error) { | ||
| // For Phase 3, this implements the contract but delegates down. | ||
| // In the future this calls the actual onnx-binding init logic. | ||
| return &onnxHandle{id: req.ModelRef}, nil |
There was a problem hiding this comment.
This returns a successful handle without loading an ONNX session, validating the artifact/provider, or allocating anything that UnloadModel can release. Callers therefore cannot distinguish a usable model from a placeholder. Please wire this to the actual ONNX runtime binding (including provider selection), return explicit unsupported/unavailable errors where necessary, and add backend tests that exercise load, inference, dimensions, and unload.
|
|
||
| func (a *Adapter) LoadModel(ctx context.Context, req native.LoadRequest) (native.ModelHandle, error) { | ||
| // For Phase 3, this implements the contract but delegates down. | ||
| return &openvinoHandle{id: req.ModelRef}, nil |
There was a problem hiding this comment.
OpenVINO has the same false-success behavior: every model reference is reported as loaded even though no compiled model exists and UnloadModel is a no-op. This needs a real OpenVINO lifecycle and inference path, plus backend-specific tests; otherwise the adapter should not be registered or advertise these capabilities.
| } | ||
|
|
||
| func init() { | ||
| native.Registry.Register(NewAdapter()) |
There was a problem hiding this comment.
Registration through package init() is ineffective unless some production package imports this adapter, and there are currently no such imports. It also advertises ONNX/CUDA/ROCm capabilities unconditionally, irrespective of build tags or runtime/provider availability. Please compose available adapters explicitly at the runtime assembly seam and make capability reporting reflect what this binary/runtime can actually execute; apply the same fix to OpenVINO.
Signed-off-by: Param <param15.veer.singh@gmail.com>
Signed-off-by: Param <param15.veer.singh@gmail.com>
Signed-off-by: Param <param15.veer.singh@gmail.com>
Signed-off-by: Param <param15.veer.singh@gmail.com>
Signed-off-by: Param <param15.veer.singh@gmail.com>
Signed-off-by: Param <param15.veer.singh@gmail.com>
Signed-off-by: Param <param15.veer.singh@gmail.com>
20daac4 to
021ced9
Compare
Signed-off-by: Param <param15.veer.singh@gmail.com>
|
You're completely right about the placeholder adapters. I've updated the branch:
|

Overview
This is Phase 3 of the staged rollout for Issue #2396. This PR expands the native contract to explicitly encompass the remaining native backends: ONNX Runtime and OpenVINO.
Key Additions
native/onnx/adapter.go): Implemented theBackendAdapterfor ONNX. Explicitly declared its hardware capability set (provider_selectionvia CPU/CUDA/ROCm) and unique features likematryoshka_2dextraction.native/openvino/adapter.go): Implemented theBackendAdapterfor OpenVINO, explicitly restricting its supported families toFamilyModernBERTto fail-fast during config validation if mismatched.Fixes #2396
part of it