Skip to content

feat(tokenization): replace request_json with native proto fields in render RPCs#461

Merged
kfirtoledo merged 11 commits intollm-d:mainfrom
sagearc:native-grpc-protos
Mar 25, 2026
Merged

feat(tokenization): replace request_json with native proto fields in render RPCs#461
kfirtoledo merged 11 commits intollm-d:mainfrom
sagearc:native-grpc-protos

Conversation

@sagearc
Copy link
Copy Markdown
Collaborator

@sagearc sagearc commented Mar 25, 2026

RenderChatCompletionRequest and RenderCompletionRequest previously used a request_json string field, requiring callers to manually marshal OpenAI types to JSON. This replaces those fields with native proto types so callers work directly with structured proto messages.

RenderChatCompletionRequest now has messages, tools, chat_template, add_generation_prompt, continue_final_message, and chat_template_kwargs. RenderCompletionRequest now has prompt and model_name. ChatMessage is also extended to support multimodal content via content_parts (text + image_url blocks), consistent with the multimodal work in #219.

Ref: vllm-project/vllm#36102
Closes #449

Copilot AI review requested due to automatic review settings March 25, 2026 12:17
@github-actions
Copy link
Copy Markdown

Unsigned commits detected! Please sign your commits.

For instructions on how to set up GPG/SSH signing and verify your commits, please see GitHub Documentation.

…render RPCs

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
@sagearc sagearc force-pushed the native-grpc-protos branch from 6d74825 to 8532ac7 Compare March 25, 2026 12:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the tokenizer gRPC API to replace request_json inputs for render RPCs with structured protobuf fields, and extends ChatMessage to represent multimodal content (text + image blocks) directly in proto.

Changes:

  • Replace request_json in RenderChatCompletionRequest/RenderCompletionRequest with native proto fields (messages/tools/template options, prompt, etc.).
  • Extend ChatMessage with content_parts plus new ContentPart/ImageUrl messages to support multimodal inputs.
  • Regenerate protobuf Go bindings and update tokenizer UDS tests/code to use the new optional-string content accessor.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
pkg/tokenization/uds_tokenizer_test.go Updates mock rendering to use GetContent() for optional proto field access.
pkg/tokenization/uds_tokenizer.go Updates ChatMessage.Content assignment for new optional string field (currently introduces a pointer-to-range-variable bug).
api/tokenizerpb/tokenizer.proto Adds multimodal message types; replaces JSON request fields with structured fields; adds render RPCs.
api/tokenizerpb/tokenizer.pb.go Regenerated Go protobuf bindings reflecting the updated proto schema.
api/tokenizerpb/tokenizer_grpc.pb.go Regenerated Go gRPC bindings; adds new RPC client/server methods.
api/indexerpb/indexer_grpc.pb.go Regenerated header/version metadata only.
api/indexerpb/indexer.pb.go Regenerated header/version metadata only.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sagearc added 8 commits March 25, 2026 14:30
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
sagearc added 2 commits March 25, 2026 15:21
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
@vMaroon
Copy link
Copy Markdown
Member

vMaroon commented Mar 25, 2026

/lgtm
/approve

@github-actions github-actions bot added the lgtm Looks good to me, indicates that a PR is ready to be merged. label Mar 25, 2026
@kfirtoledo kfirtoledo merged commit 665ec28 into llm-d:main Mar 25, 2026
11 checks passed
@sagearc sagearc deleted the native-grpc-protos branch March 25, 2026 13:47
sagearc added a commit to sagearc/llm-d-kv-cache-manager that referenced this pull request Mar 25, 2026
…render RPCs (llm-d#461)

* feat(tokenization): replace request_json with native proto fields in render RPCs

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* add multimodal content type

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* populate content parts in chat message proto

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* add ContentPartList wrapper and use oneof for ChatMessage content

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* update uds_tokenizer to use oneof content types

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* remove detail field from ImageUrl

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* remove Detail from ImageBlock, add PlainText helper, fix MarshalJSON

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* remove deprecated json string

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* fix indices in proto

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* cr review

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* trace verbosity

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

---------

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
sagearc added a commit to sagearc/llm-d-kv-cache-manager that referenced this pull request Mar 25, 2026
…render RPCs (llm-d#461)

* feat(tokenization): replace request_json with native proto fields in render RPCs

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* add multimodal content type

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* populate content parts in chat message proto

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* add ContentPartList wrapper and use oneof for ChatMessage content

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* update uds_tokenizer to use oneof content types

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* remove detail field from ImageUrl

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* remove Detail from ImageBlock, add PlainText helper, fix MarshalJSON

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* remove deprecated json string

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* fix indices in proto

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* cr review

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

* trace verbosity

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>

---------

Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Looks good to me, indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switch render RPCs from request_json to vLLM native gRPC API

4 participants