Skip to content

test: Add test and usage example for mm requests#453

Open
sagearc wants to merge 3 commits intollm-d:mainfrom
sagearc:mm-test-and-example-uds
Open

test: Add test and usage example for mm requests#453
sagearc wants to merge 3 commits intollm-d:mainfrom
sagearc:mm-test-and-example-uds

Conversation

@sagearc
Copy link
Copy Markdown
Collaborator

@sagearc sagearc commented Mar 23, 2026

review fixes from #428

Added the example from #280 and mm test

Closes #451, Closes #452

@sagearc sagearc requested a review from delavet as a code owner March 23, 2026 16:30
Copilot AI review requested due to automatic review settings March 23, 2026 16:30
@sagearc sagearc marked this pull request as draft March 23, 2026 16:30
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

Adds multimodal (image) coverage to the UDS tokenizer renderer integration tests by introducing a local image fixture and new MM-specific test cases.

Changes:

  • Add local llm-d logo testdata and a fixture that exposes it as a base64 data URL.
  • Introduce MM model fixture and new RenderChatCompletion MM integration tests.
  • Add helper for building image+text chat messages for test requests.

Reviewed changes

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

File Description
services/uds_tokenizer/tests/testdata/llmd_logo.png Adds a local image asset used by MM tests.
services/uds_tokenizer/tests/test_renderer.py Adds MM request helpers and new MM integration tests (features + determinism).
services/uds_tokenizer/tests/conftest.py Adds MM model fixture and base64 data URL fixture backed by local testdata.

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

@sagearc sagearc marked this pull request as ready for review March 23, 2026 16:54
@sagearc
Copy link
Copy Markdown
Collaborator Author

sagearc commented Mar 23, 2026

@delavet CR fixes

closes #452, #451

@sagearc sagearc changed the title [MM] Add test and usage example for mm requests test: Add test and usage example for mm requests Mar 23, 2026
Copy link
Copy Markdown
Collaborator

@delavet delavet left a comment

Choose a reason for hiding this comment

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

I think we also need a Dockerfile and corresponding Make commands to build this example—what do you think?
Everything else is fantastic!

@gyliu513
Copy link
Copy Markdown
Contributor

@delavet Introducing a dockerfile maybe complex to run this program, how about follow other demos like make run-example kv_cache_index, it is just calling go build and then run the binary.

@delavet
Copy link
Copy Markdown
Collaborator

delavet commented Mar 28, 2026

@delavet Introducing a dockerfile maybe complex to run this program, how about follow other demos like make run-example kv_cache_index, it is just calling go build and then run the binary.

Perhaps you’re right—this would make verification simpler. However, I usually do verify in Kubernetes environments, so I may consider submitting a PR to include Kubernetes deployment-related content. Thank you for your feedback!

I'll approve this for now. @sagearc What do you think? Perhaps we can merge this PR now?

delavet
delavet previously approved these changes Mar 28, 2026
@sagearc
Copy link
Copy Markdown
Collaborator Author

sagearc commented Mar 28, 2026

Thanks @delavet @gyliu513, good points. Since examples are mostly meant to run locally, we could add a run-example-online-uds Make target that sets up the local venv and starts the UDS tokenizer as a background process alongside the go binary.
Happy to split if you'd prefer to land the test first.

@gyliu513
Copy link
Copy Markdown
Contributor

@sagearc +1 to land the test first, we can always have follow up PRs.

@delavet
Copy link
Copy Markdown
Collaborator

delavet commented Mar 29, 2026

@sagearc Thanks, I think we can merge this right after conflicts being resolved

@sagearc
Copy link
Copy Markdown
Collaborator Author

sagearc commented Mar 29, 2026

Opened this PR to update all examples to use the tokenization service:
#471

@sagearc sagearc force-pushed the mm-test-and-example-uds branch from 7fe0277 to 0a019ba Compare March 31, 2026 19:48
@github-actions github-actions bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 31, 2026
Signed-off-by: Sage Ahrac <sagiahrak@gmail.com>
@sagearc sagearc force-pushed the mm-test-and-example-uds branch from 0a019ba to 5845ba6 Compare March 31, 2026 19:50
@github-actions github-actions bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 31, 2026
sagearc added 2 commits March 31, 2026 22:53
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

size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add multimodal test coverage for render RPCs Add UDS tokenizer usage example

4 participants